Repository navigation
fix(helm): collector chart namespace, service account and default-value fixes - #63
Merged
tim-thacker-nullify merged 13 commits intoSep 15, 2026
Conversation
…ue fixes - Remove the pre-install namespace hook and the bitnami/kubectl Job; the namespace now comes from --create-namespace, and a namespace helper falls back to the release namespace when serviceAccount.namespace is empty (default stays nullify for the IRSA trust). - Use the service-account-name helper in the CronJob and ServiceAccount. - Default clusterName, s3.bucket and kms.keyArn to "" and fail on empty or placeholder values, on a KMS alias instead of a key ARN, on a region that differs from the key's, and on a missing EKS role-arn or GKE role ARN. - Remove collector.dataCollection, which the binary never read. - Fix the labels helper joining custom labels onto the previous line, and apply nodeSelector/tolerations to the collector pod. - kubeVersion >=1.21, chart 0.4.0, and README/NOTES corrections. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… errors - An alias keyArn still fails, now saying to use the key ARN from the Nullify configure page and to contact Nullify if the page shows an alias. - The region mismatch error explains that --reuse-values from 0.2.0 carries us-east-1, and to upgrade with --reset-then-reuse-values or set the region. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lector role-arn guard - The EKS role-arn guard accepts arn:aws, arn:aws-us-gov and arn:aws-cn. - serviceAccount.requireRoleArn (default true) lets EKS Pod Identity, instance-profile and kube2iam/kiam installs render without IRSA; an empty role-arn annotation is no longer rendered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…values upgrades Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 14, 2026
…RN commercial - collector.kms.keyArn accepts arn:aws:kms:<region>:<account>:alias/<name> and key ARNs including multi-Region key/mrk-..., matching the connector CloudFormation AllowedPattern. The region default and check take the region from either form. - NOTES warns when an alias is used: IAM matches the key ARN, so the role needs the connector stack version that also grants key/*; the Terraform module grants only the given ARN. - The role-arn guard is back to arn:aws:iam:: only; Nullify's bucket and key are commercial-partition. serviceAccount.requireRoleArn=false stays the escape hatch. ci/eks-govcloud-role.yaml is removed. - CI: ci/eks-kms-alias.yaml and ci/eks-kms-mrk.yaml must render; a bare alias, a GovCloud alias ARN, an alias region mismatch and a GovCloud role ARN must fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- .helmignore excludes ci/ from 0.4.0, so a later edit to a CI values file does not change a published version's contents and force a version bump. - Added with the 0.4.0 bump because 0.2.0 is already published; the file changes the package contents. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e current template - NOTES, README and values.yaml say what an alias in collector.kms.keyArn needs: the IAM role's KMS statement must cover the resolved key, for example arn:aws:kms:<region>:<account-id>:key/*. - CloudFormation: connector releases that include PR #64 grant key/* in the account and region of NullifyKMSKeyArn; the template on main grants only the ARN given. Terraform: the module accepts an alias; check the module version's KMS statement covers key/*. A key ARN works with every version. - New README section "KMS alias ARNs" holds the detail; the table, 0.4.0 upgrade notes and troubleshooting link to it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…erge order main's helm-release.yml rebuilds the Pages site from the collector chart alone, so merging this chain before the publish rework unpublishes collector 0.2.0 and any other version the live index serves. - the release job compares the live index against the single package it would deploy and fails, naming the required merge order, when it would drop one - README gains a maintainer section stating that order and why Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…faults Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-defaults Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-defaults Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-defaults Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 15, 2026
tim-thacker-nullify
marked this pull request as ready for review
September 15, 2026 06:10
tim-thacker-nullify
merged commit Sep 15, 2026
d4e7a9c
into
feat/cspm-lifecycle-grants
1 check passed
Member
Author
tim-thacker-nullify
added a commit
that referenced
this pull request
Sep 15, 2026
- Revert the Helm collector clusterrole.yaml to #78's version: #98 mutated the already-published chart 0.2.0's RBAC without a version bump, which would republish 0.2.0 with different RBAC and a new digest on merge. - Revert #98's collector-role edits in both k8s-resources/main.tf trees (v5, v6) to #78's version, removing the three-file merge conflict with #58/#99/#63. Collector RBAC belongs to #58/#99 (chart 0.3.0). - Add batch/jobs (list) to managed-scan RBAC in both trees, matching Helm and the in-cluster collector role, so CronJob pod ownership attribution doesn't silently lose data on managed-scan clusters. Update the pinned k8s_resources.tftest.hcl assertions for the new rule. - Correct README:113's Secrets claim (no server-side Table fetch exists; values are redacted after collection, not before) and README:89's false "same kind list" claim for the published 0.2.0 chart. - Rewrite check_iam_policy_drift.py to compare whole statements (effect, action, resources, condition) instead of a bare union of action names, and fix the actions= regex matching inside not_actions=. - Add scripts/** to terraform-validate.yml's path filters so editing the drift script alone triggers the drift job. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Sep 15, 2026
tim-thacker-nullify
added a commit
that referenced
this pull request
Sep 17, 2026
… Kubernetes list kinds (#58) * feat: grant wafv2:Get*, ec2:GetEbsEncryptionByDefault and the missing Kubernetes list kinds The AWS collectors and the managed Kubernetes scanner read resources the connector never granted, so those reads fail closed or record a permission gap. - Add `wafv2:Get*` and `ec2:GetEbsEncryptionByDefault` to all three AWS templates (CloudFormation, terraform, terraform-v6), keeping the Allow sets byte-identical. - Reconcile the ClusterRole in both k8s-resources terraform modules and the Helm chart to the exact kinds the collector Lists: core configmaps, secrets, resourcequotas, limitranges; discovery.k8s.io endpointslices; all four admissionregistration.k8s.io kinds; storage.k8s.io csidrivers; node.k8s.io runtimeclasses; scheduling.k8s.io priorityclasses; flowcontrol.apiserver.k8s.io flowschemas; networking.k8s.io ingressclasses; certificates.k8s.io certificatesigningrequests. - Drop core `endpoints`, which every manifest granted and the collector never lists (it reads discovery.k8s.io/endpointslices instead). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(aws): grant the 45 read actions the CSPM collectors call  The connector role never granted 45 of the AWS read actions the collectors issue. Each call returns AccessDenied, the collector tolerates it and leaves the field nil, and the CSPM rule that reads the field fires on every resource: kms:GetKeyRotationStatus on every customer-managed key, ecr:GetLifecyclePolicy on every repository, sns:GetTopicAttributes on every topic. sqs, codebuild, config and opensearch drop their whole inventory instead. - part1: athena:GetWorkGroup, backup:Get*, codebuild:BatchGet*, codebuild:List*, config:Describe*, dynamodb:GetResourcePolicy, ec2:GetManagedPrefixListEntries, ec2:GetSnapshotBlockPublicAccessState, ecr:GetLifecyclePolicy, ecr:GetRepositoryPolicy, es:Describe*, es:List*, glue:Get*, iam:GenerateCredentialReport, kms:GetKeyPolicy, kms:GetKeyRotationStatus - part2: s3:GetAccountPublicAccessBlock, s3:GetBucket*, s3:GetEncryptionConfiguration, s3:GetLifecycleConfiguration, s3:GetReplicationConfiguration, sns:GetTopicAttributes, sqs:GetQueueAttributes, ssm:Describe*, ssm:GetDocument - s3:GetBucketLocation dropped as redundant under the new s3:GetBucket* - terraform-v6/data.tf gains the trailing newline it was missing - applied identically to the CloudFormation template and both Terraform modules; part1 is at 5921/6144 chars and part2 at 5593/6144, so neither needs a Part3 The 45 were derived from every *<pkg>.<Operation>Input literal in context/internal/cloudintegrations/awsintegration/processors, mapped through the IAM prefix and action renames (opensearch -> es, s3 GetBucketEncryption -> s3:GetEncryptionConfiguration, API Gateway -> apigateway:GET). A Go test in that package now re-derives the list on every run and asserts this template grants it, which is the third source check_cloud_connector_iam_drift.py cannot supply: it compares the three templates to each other, so a grant missing from all three was green. * fix(aws): scope the connector role to configuration reads only Four wildcard grants reached customer data, not just cloud configuration: wafv2:Get* covers GetSampledRequests (live HTTP requests with Cookie and Authorization headers), glue:Get* covers GetConnection (JDBC PASSWORD) and GetTable*/GetPartitions, codebuild:BatchGet* and backup:Get* both expand well past the single call each collector makes. - Replace those four with the nine actions the collectors actually call. - Deny 55 data-plane actions the Allow wildcards still reached: raw object and message content (s3-outposts:GetObject, logs:StartLiveTail, states:DescribeExecution, cognito-sync:QueryRecords, aps:QueryMetrics, rum:GetAppMonitorData, ssm:ListCommandInvocations, support:DescribeCases), cleartext credentials (lightsail:GetRelationalDatabaseMasterUserPassword, storagegateway:DescribeChapCredentials), end-user PII (cognito-idp:ListUsers and ListUsersInGroup), and the credential oracles transfer:TestIdentityProvider and verifiedpermissions:IsAuthorizedWithToken. - Delete the 12 Allow entries the new Deny statement shadowed, rather than leaving the same action in both lists. - Delete 7 Allow entries already subsumed by a sibling wildcard. - Bump the Helm chart to 0.3.0 so tenants can actually pull the new ClusterRole; helm-release.yml keys the published version off Chart.yaml, so the RBAC added in the previous commit was unreachable at 0.2.0. Verified against the collectors on monorepo ea4b7c0: 240/241 AWS calls remain covered (the one exception, sts:AssumeRole, is test-only and is the role's trust policy, not a grant). No collector calls any denied action. Real IAM-counted headroom on readonly-part1 goes 223 -> 326 chars; the drift gate under-reports by 15 chars per statement because document_size() omits "Condition". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0126R5BKvayttBu8nKmFZBVR * fix(aws): deny the data reads a second audit found still reachable An adversarial pass over the remaining Allow wildcards found reach the first narrowing missed. None of it is called by any collector. - Deny 22 actions still reachable through a wildcard: Lake Formation row data (GetWorkUnitResults, GetWorkUnits), live stream samples (kinesisanalytics:DiscoverInputSchema), Lightsail container application logs, support case attachment bytes, a plaintext OAuth client secret (cognito-idp:DescribeUserPoolClient), usable AppSync API keys (appsync:ListApiKeys), notification event bodies, and the WorkMail / IdentityStore / AppStream / Rekognition / SSO / WorkSpaces user directories. - Drop 11 Allow entries instead of shadowing them: amplify:GetWebhook (the URL embeds a bearer token and POSTing it starts a build, a write the Deny statement cannot reach), account:GetContactInformation / GetAlternateContact / GetPrimaryEmail and one:ListUsers (named-person PII), cloudtrail:LookupEvents (per-event username and source IP), and the four write actions hiding behind read-sounding names -- cloudformation:Detect* (AWS classes it Write; it schedules drift detection and mutates stack state), cloudwatch:GenerateQuery, logs:StopLiveTail, logs:StopQuery. - Drop glue:Describe*, which matches no real Glue action, and one of the two copies of notifications:List*. Verified with a from-scratch IAM evaluator: 232/232 collector actions still allowed, every action above resolves to explicitDeny or implicitDeny, and no action appears in both Allow and Deny. readonly-part1 headroom 223 -> 545. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0126R5BKvayttBu8nKmFZBVR * fix(k8s): drop the unused get verb and harden the terraform collector Under an assume-breach model the collector's RBAC and pod spec are what an attacker inherits, so both are cut to what the code actually needs. - Drop `get` from all 38 grants. `collect.go` has no `.Get(` call on any kind, including CRDs; all five list sites go through `.List`, so `get` was dead weight in the ClusterRole a compromised image would hold. - Grant `nonResourceURLs: ["/version"]`. `Discovery().ServerVersion()` is called and was never granted; it works today only because EKS, GKE and AKS bind `system:public-info-viewer` to `system:authenticated`, and under `Strict: true` a cluster without that default fails collection outright. - Give the terraform CronJob the security context the Helm chart already sets (no privilege escalation, read-only root filesystem, run as non-root, drop ALL capabilities). Terraform-deployed collectors ran with none of it. - Add the trailing newline `terraform-v6/modules/k8s-resources/main.tf` was missing, the same defect this branch already fixed in its `data.tf`. All three manifests stay byte-identical in what they grant: 38 resources at `list`, plus `/version` at `get`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0126R5BKvayttBu8nKmFZBVR * fix(aws): cut the role to the actions the collectors call The readonly policy was AWS's SecurityAudit shape minus a deny list, so it granted ~300 services the collectors never touch. If Nullify is compromised the attacker inherits this role in every onboarded account, and that unused surface is pure attacker capability with no scanning value. Rebuild the two readonly policies as an explicit allow list: 433 account-wide Allow entries -> 107. - Drop 134 services outright, none of which any collector calls: sdb, lakeformation, lightsail, support, workmail, rekognition, sso, sso-directory, identitystore, route53domains, storagegateway, servicecatalog, waf, machinelearning, the mediapackage family and the rest. - Drop cloudformation, elasticbeanstalk, amplify and serverlessrepo, which have no processor at all and leak DescribeStacks parameters, Beanstalk plaintext env config, Amplify basicAuthCredentials and presigned source archives. - Drop `iam:Simulate*`, whose only use to an attacker holding the IAM graph is testing escalation paths offline without generating AccessDenied in CloudTrail. - Narrow 9 families whose wildcard mixes configuration reads with a data or secret sibling: `lambda:Get*` (GetLayerVersion returns a presigned code URL), `s3:List*` (ListBucket enumerates object keys), `guardduty:Get*` (GetFindings), `appsync:List*` (ListApiKeys), `cognito-idp:Describe*`/`List*`, `cloudtrail:Get*`, `ssm:List*`, `states:Describe*`. - Keep `ec2:Describe*` and `iam:Get*`/`List*` as wildcards deliberately. Their families are inventory metadata, enumerating them is brittle, and `iam:Get*`/`List*` also cover the OIDC-provider reads in admin/internal/cloud/cloudpolicies that a processors-only derivation misses. Verified against 234 required actions drawn from the collectors, the admin preflight and cloudpolicies: all 234 still resolve to allowed, zero regressions. Every dropped item was confirmed to have no caller anywhere in context, admin, cspm or scpm. readonly-part1 falls from 5599 to 1355 of 6144 chars. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0126R5BKvayttBu8nKmFZBVR * fix(aws): enumerate ec2 and iam instead of wildcarding them `ec2:Describe*` and `iam:Get*`/`List*` were kept as wildcards on the argument that both families are inventory metadata. That is wrong for ec2 and too generous for iam. - `ec2:Describe*` covers 192 actions; the collectors call 24. The unused remainder includes `DescribeVpnConnections`, which returns `CustomerGatewayConfiguration` with the IPsec pre-shared key in plaintext -- a live credential into the customer's on-prem network -- plus `DescribeInstanceAttribute` (UserData), `DescribeClientVpnConnections` (client identities and source IPs) and `DescribeTags` (owner emails). - `iam:Get*`/`List*` cover 67; the collectors and admin's OIDC resolver call 25. The remainder includes `GetAccountAuthorizationDetails`, which returns the entire IAM graph in one call, `GetServiceLastAccessedDetails`, which tells an attacker which grants are stale enough to abuse unnoticed, and `GetSAMLProvider`/`GetLoginProfile` for phishing the workforce. Allow 107 -> 152: enumeration is longer than the wildcards it replaces but grants far less. - Deny 15 further actions as defence in depth against a future re-broadening, matching the discipline already applied to Cognito and WorkMail. - Deny `apigateway:GET` on `/apikeys*` and `/usageplans*` via a second, resource-scoped statement. API Gateway exposes one HTTP-verb-shaped action covering its whole control plane, including `GET /apikeys?includeValues=true`; path ARNs are the only way to separate the five calls the collectors make from plaintext API key values. 234/234 required actions still resolve to allowed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(aws): correct two invalid action names and a neptune coverage gap IAM accepts a misspelled action in a policy and simply never matches it, so both of these were silent. - `medialive:GetInputDeviceThumbnail` is not an IAM action; the real one is `DescribeInputDeviceThumbnail`. Introduced in 3a1a4fa. Inert today -- nothing grants `medialive:*` any more -- but a dead Deny that a future broad grant would sail straight past. - `neptune:` is not a valid IAM prefix. Neptune's control plane is served by the RDS API and authorised with `rds:` actions, so `neptune:Describe*` and `neptune:List*` granted nothing. Drop both. - That mattered: enumerating the policy mapped `neptune.ListTagsForResource` to the dead `neptune:List*` rather than `rds:ListTagsForResource`, and the earlier `rds:List*` wildcard was gone, so the collector's Neptune tag read would have failed. Grant `rds:ListTagsForResource`. The three Describe calls were already covered by `rds:Describe*`. Every action string in all three templates was checked against AWS's policy generator dataset: 296/297 valid before this commit, and the one miss is fixed here. 231/231 required actions resolve to allowed with Neptune mapped to `rds:`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(aws): drop collector ClusterRole grants the collector never lists whiskeylotus's review on #58 (W58-2) flagged that the collector ClusterRole grants 12 kinds main's collect.go never lists: jobs is kept since the monorepo's fix/k8s-deployment-matching-review stack (#12807, which #12830 sits on) starts collecting Jobs owned by a CronJob; cronjobs, horizontalpodautoscalers, poddisruptionbudgets, ingressclasses, storageclasses, csidrivers, runtimeclasses, priorityclasses, flowschemas, certificatesigningrequests and customresourcedefinitions are dropped -- no open collector change reads any of them. Verified against context/internal/cloudintegrations/k8sintegration/ collect/collect.go on both the monorepo's main and fix/k8s-deployment-matching-review branches. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(aws): drop collector ClusterRole grants the collector never lists (#99) whiskeylotus's review on #58 (W58-2) flagged that the collector ClusterRole grants 12 kinds main's collect.go never lists: jobs is kept since the monorepo's fix/k8s-deployment-matching-review stack (#12807, which #12830 sits on) starts collecting Jobs owned by a CronJob; cronjobs, horizontalpodautoscalers, poddisruptionbudgets, ingressclasses, storageclasses, csidrivers, runtimeclasses, priorityclasses, flowschemas, certificatesigningrequests and customresourcedefinitions are dropped -- no open collector change reads any of them. Verified against context/internal/cloudintegrations/k8sintegration/ collect/collect.go on both the monorepo's main and fix/k8s-deployment-matching-review branches. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(helm): collector chart namespace, service account and default-value fixes (#63) * fix(helm): collector chart namespace, service account and default-value fixes - Remove the pre-install namespace hook and the bitnami/kubectl Job; the namespace now comes from --create-namespace, and a namespace helper falls back to the release namespace when serviceAccount.namespace is empty (default stays nullify for the IRSA trust). - Use the service-account-name helper in the CronJob and ServiceAccount. - Default clusterName, s3.bucket and kms.keyArn to "" and fail on empty or placeholder values, on a KMS alias instead of a key ARN, on a region that differs from the key's, and on a missing EKS role-arn or GKE role ARN. - Remove collector.dataCollection, which the binary never read. - Fix the labels helper joining custom labels onto the previous line, and apply nodeSelector/tolerations to the collector pod. - kubeVersion >=1.21, chart 0.4.0, and README/NOTES corrections. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(helm): expect the role-arn guard for the collector defaults case Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(helm): give the exact fix in the collector's KMS alias and region errors - An alias keyArn still fails, now saying to use the key ARN from the Nullify configure page and to contact Nullify if the page shows an alias. - The region mismatch error explains that --reuse-values from 0.2.0 carries us-east-1, and to upgrade with --reset-then-reuse-values or set the region. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(helm): accept every AWS partition and allow opting out of the collector role-arn guard - The EKS role-arn guard accepts arn:aws, arn:aws-us-gov and arn:aws-cn. - serviceAccount.requireRoleArn (default true) lets EKS Pod Identity, instance-profile and kube2iam/kiam installs render without IRSA; an empty role-arn annotation is no longer rendered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(helm): document the alias migration, requireRoleArn and --reuse-values upgrades Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(helm): accept KMS alias ARNs in the collector and keep the role ARN commercial - collector.kms.keyArn accepts arn:aws:kms:<region>:<account>:alias/<name> and key ARNs including multi-Region key/mrk-..., matching the connector CloudFormation AllowedPattern. The region default and check take the region from either form. - NOTES warns when an alias is used: IAM matches the key ARN, so the role needs the connector stack version that also grants key/*; the Terraform module grants only the given ARN. - The role-arn guard is back to arn:aws:iam:: only; Nullify's bucket and key are commercial-partition. serviceAccount.requireRoleArn=false stays the escape hatch. ci/eks-govcloud-role.yaml is removed. - CI: ci/eks-kms-alias.yaml and ci/eks-kms-mrk.yaml must render; a bare alias, a GovCloud alias ARN, an alias region mismatch and a GovCloud role ARN must fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(helm): keep ci/ out of the collector chart package - .helmignore excludes ci/ from 0.4.0, so a later edit to a CI values file does not change a published version's contents and force a version bump. - Added with the 0.4.0 bump because 0.2.0 is already published; the file changes the package contents. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(helm): state the KMS alias requirement instead of pointing at the current template - NOTES, README and values.yaml say what an alias in collector.kms.keyArn needs: the IAM role's KMS statement must cover the resolved key, for example arn:aws:kms:<region>:<account-id>:key/*. - CloudFormation: connector releases that include PR #64 grant key/* in the account and region of NullifyKMSKeyArn; the template on main grants only the ARN given. Terraform: the module accepts an alias; check the module version's KMS statement covers key/*. A key ARN works with every version. - New README section "KMS alias ARNs" holds the detail; the table, 0.4.0 upgrade notes and troubleshooting link to it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(helm): refuse to unpublish live chart versions and document the merge order main's helm-release.yml rebuilds the Pages site from the collector chart alone, so merging this chain before the publish rework unpublishes collector 0.2.0 and any other version the live index serves. - the release job compares the live index against the single package it would deploy and fails, naming the required merge order, when it would drop one - README gains a maintainer section stating that order and why Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(terraform): finish W58-2 in both collector-role trees #99 dropped the 11 never-collected kinds (cronjobs, HPA, PDB, ingressclasses, storageclasses, csidrivers, runtimeclasses, priorityclasses, flowschemas, certificatesigningrequests, CRDs) from the Helm ClusterRole only. The Terraform collector roles in both trees still granted them, so a k8s-resources install of #58 kept read access to kinds no collector reads, and the two collector roles differed by those 11 kinds. jobs is kept. Helm (0.3.0), TF v5 and TF v6 now grant identical kinds and verbs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(aws): scope GetDocument and deny secret-shaped reads Co-authored-by: Cursor <cursoragent@cursor.com> * fix(aws): restore CSPM list kinds and put the IAM drift gate in CI Keep the 11 list-only kinds the Strict collector in monorepo #12225 reads, validate terraform-v6 and the CloudFormation template in CI, and drop the stale Helm publish merge-order section now that #60 is on main. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(aws): deny SSM document bodies and pin ClusterRole drift in CI DescribeDocument and DescribeAutomationExecutions are denied. The drift job now compares collector ClusterRole kinds across Helm and both Terraform trees. Helm defaults to k8s-collector-3.46.0; CFN IRSA uses StringEquals on :sub. * fix(k8s): drop secrets list so Secret values cannot be read Kubernetes list on secrets returns .data; omit the grant from the collector ClusterRole. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
tim-thacker-nullify
added a commit
that referenced
this pull request
Sep 17, 2026
…nning (#61) * feat(helm): add nullify-k8s-readonly-access chart for managed EKS scanning New chart nullify-k8s-readonly-access 0.1.0: a ClusterRole with list only on the 26 kinds the managed EKS scanner lists, bound to a Group (default nullify-readonly). Renders fail for an empty or system: group and for non-read extraRules verbs. grantSecretsRead and grantConfigMapsRead default to true because the strict scanner fails the cluster without them today. manifests/nullify-readonly-rbac.yaml is the default render without Helm's bookkeeping labels, and a CI job diffs the two. The chart README covers Helm, Flux and kubectl installs, impersonation-based verification, and why AmazonEKSAdminViewPolicy and AmazonEKSViewPolicy are not the recommended path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * ci(helm): fail the RBAC manifest check when either side normalises to nothing Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(helm): reject system subjects of every kind and non-read-only extra rules - extraSubjects: any name starting "system:" (trimmed, case-insensitive) fails whatever its kind; kinds are limited to User, Group and ServiceAccount; ServiceAccounts in kube-system, kube-public and kube-node-lease fail. - extraRules: reject wildcard apiGroups or resources, the exec, attach, portforward, proxy and log subresources, and nonResourceURLs other than /version. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(helm): grant get on /version to the managed scan's read-only role The strict managed scan records a ServerVersion() failure and fails the cluster, so a cluster without the default system:public-info-viewer binding could not be scanned. The raw manifest carries the same rule, and the manifest check fails if either side drops it. NOTES lists the egress IPs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(helm): correct Helm adoption, list egress IPs and drop the collector Namespace - Helm 3.2+ adopts the manifest's objects once they carry Helm ownership metadata, so moving to Helm no longer needs a delete and a scan outage. - List Nullify egress IPs per region for the EKS public endpoint. - The collector no longer creates a Namespace (#63). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * ci(helm): gate publishing the readonly-access chart on the RBAC manifest check helm-charts/nullify-k8s-readonly-access/ci/pre-publish.sh runs scripts/check-rbac-manifest.sh. The #65 follow-up's publish script runs a chart's pre-publish hook before packaging it and skips the chart when the hook fails, so manifest drift blocks the release and its tag whatever the release job's needs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(helm): allow only status and scale subresources and render the trimmed group - extraRules resources: any subresource other than status and scale is rejected, so CRD subresources such as KubeVirt's virtualmachineinstances/console and /vnc/screenshot no longer render. New must-fail cases cover both. - The ClusterRoleBinding subject and NOTES use the trimmed groupName the guard checks, not the raw value. - The Helm adoption steps take the release name and storage namespace as variables, with the Flux HelmRelease values (flux-system unless storageNamespace is set) alongside the Helm CLI ones. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(helm): keep ci/ out of the readonly-access chart package - .helmignore excludes ci/, so editing the pre-publish hook or a CI values file no longer changes the packaged contents of a published version (which forced a version bump) and the hook script is not shipped to users. - scripts/test-helm-charts.sh reads ci/*.yaml from the chart directory, so lint and render coverage is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(helm): grant list on batch/jobs in the readonly access chart The managed EKS scan now lists Jobs to attribute CronJob-run pods to their CronJob. Without the grant the list is skipped and those pods key on the Job. - ClusterRole and the rendered manifest gain a batch/jobs list rule (kept identical, as check-rbac-manifest.sh requires). - README kinds table and verify loop include jobs; note that a denied Jobs list is skipped rather than failing the scan. - values.yaml extraRules example uses cronjobs, since jobs is now granted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(helm): state 27 kinds for the readonly access grant - Manifest header, chart README and root README said 26 kinds; batch/jobs makes it 27. CI's manifest check ignores comments, so it did not catch the header. - ci/customised.yaml extraRules adds only cronjobs; jobs is now a default grant. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(helm): list-only extraRules and binding seatbelts Co-authored-by: Cursor <cursoragent@cursor.com> * test(helm): reject aggregate-to-view/edit/admin labels Co-authored-by: Cursor <cursoragent@cursor.com> * fix(helm): never grant list on secrets list returns Secret values, so the chart rejects grantSecretsRead and extraRules that name secrets. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(helm): document Table-API secrets and drop the cross-chart release gate The managed scan stores name, type, key count and age, and collectSecrets already exists, so the chart must not claim values leak or that the option is unshipped. Release stays per-chart via the pre-publish hook. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
tim-thacker-nullify
added a commit
that referenced
this pull request
Sep 17, 2026
… look up clusters in their own region (#78) * feat: grant wafv2:Get*, ec2:GetEbsEncryptionByDefault and the missing Kubernetes list kinds The AWS collectors and the managed Kubernetes scanner read resources the connector never granted, so those reads fail closed or record a permission gap. - Add `wafv2:Get*` and `ec2:GetEbsEncryptionByDefault` to all three AWS templates (CloudFormation, terraform, terraform-v6), keeping the Allow sets byte-identical. - Reconcile the ClusterRole in both k8s-resources terraform modules and the Helm chart to the exact kinds the collector Lists: core configmaps, secrets, resourcequotas, limitranges; discovery.k8s.io endpointslices; all four admissionregistration.k8s.io kinds; storage.k8s.io csidrivers; node.k8s.io runtimeclasses; scheduling.k8s.io priorityclasses; flowcontrol.apiserver.k8s.io flowschemas; networking.k8s.io ingressclasses; certificates.k8s.io certificatesigningrequests. - Drop core `endpoints`, which every manifest granted and the collector never lists (it reads discovery.k8s.io/endpointslices instead). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(aws): grant the 45 read actions the CSPM collectors call  The connector role never granted 45 of the AWS read actions the collectors issue. Each call returns AccessDenied, the collector tolerates it and leaves the field nil, and the CSPM rule that reads the field fires on every resource: kms:GetKeyRotationStatus on every customer-managed key, ecr:GetLifecyclePolicy on every repository, sns:GetTopicAttributes on every topic. sqs, codebuild, config and opensearch drop their whole inventory instead. - part1: athena:GetWorkGroup, backup:Get*, codebuild:BatchGet*, codebuild:List*, config:Describe*, dynamodb:GetResourcePolicy, ec2:GetManagedPrefixListEntries, ec2:GetSnapshotBlockPublicAccessState, ecr:GetLifecyclePolicy, ecr:GetRepositoryPolicy, es:Describe*, es:List*, glue:Get*, iam:GenerateCredentialReport, kms:GetKeyPolicy, kms:GetKeyRotationStatus - part2: s3:GetAccountPublicAccessBlock, s3:GetBucket*, s3:GetEncryptionConfiguration, s3:GetLifecycleConfiguration, s3:GetReplicationConfiguration, sns:GetTopicAttributes, sqs:GetQueueAttributes, ssm:Describe*, ssm:GetDocument - s3:GetBucketLocation dropped as redundant under the new s3:GetBucket* - terraform-v6/data.tf gains the trailing newline it was missing - applied identically to the CloudFormation template and both Terraform modules; part1 is at 5921/6144 chars and part2 at 5593/6144, so neither needs a Part3 The 45 were derived from every *<pkg>.<Operation>Input literal in context/internal/cloudintegrations/awsintegration/processors, mapped through the IAM prefix and action renames (opensearch -> es, s3 GetBucketEncryption -> s3:GetEncryptionConfiguration, API Gateway -> apigateway:GET). A Go test in that package now re-derives the list on every run and asserts this template grants it, which is the third source check_cloud_connector_iam_drift.py cannot supply: it compares the three templates to each other, so a grant missing from all three was green. * fix(aws): scope the connector role to configuration reads only Four wildcard grants reached customer data, not just cloud configuration: wafv2:Get* covers GetSampledRequests (live HTTP requests with Cookie and Authorization headers), glue:Get* covers GetConnection (JDBC PASSWORD) and GetTable*/GetPartitions, codebuild:BatchGet* and backup:Get* both expand well past the single call each collector makes. - Replace those four with the nine actions the collectors actually call. - Deny 55 data-plane actions the Allow wildcards still reached: raw object and message content (s3-outposts:GetObject, logs:StartLiveTail, states:DescribeExecution, cognito-sync:QueryRecords, aps:QueryMetrics, rum:GetAppMonitorData, ssm:ListCommandInvocations, support:DescribeCases), cleartext credentials (lightsail:GetRelationalDatabaseMasterUserPassword, storagegateway:DescribeChapCredentials), end-user PII (cognito-idp:ListUsers and ListUsersInGroup), and the credential oracles transfer:TestIdentityProvider and verifiedpermissions:IsAuthorizedWithToken. - Delete the 12 Allow entries the new Deny statement shadowed, rather than leaving the same action in both lists. - Delete 7 Allow entries already subsumed by a sibling wildcard. - Bump the Helm chart to 0.3.0 so tenants can actually pull the new ClusterRole; helm-release.yml keys the published version off Chart.yaml, so the RBAC added in the previous commit was unreachable at 0.2.0. Verified against the collectors on monorepo ea4b7c0: 240/241 AWS calls remain covered (the one exception, sts:AssumeRole, is test-only and is the role's trust policy, not a grant). No collector calls any denied action. Real IAM-counted headroom on readonly-part1 goes 223 -> 326 chars; the drift gate under-reports by 15 chars per statement because document_size() omits "Condition". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0126R5BKvayttBu8nKmFZBVR * fix(aws): deny the data reads a second audit found still reachable An adversarial pass over the remaining Allow wildcards found reach the first narrowing missed. None of it is called by any collector. - Deny 22 actions still reachable through a wildcard: Lake Formation row data (GetWorkUnitResults, GetWorkUnits), live stream samples (kinesisanalytics:DiscoverInputSchema), Lightsail container application logs, support case attachment bytes, a plaintext OAuth client secret (cognito-idp:DescribeUserPoolClient), usable AppSync API keys (appsync:ListApiKeys), notification event bodies, and the WorkMail / IdentityStore / AppStream / Rekognition / SSO / WorkSpaces user directories. - Drop 11 Allow entries instead of shadowing them: amplify:GetWebhook (the URL embeds a bearer token and POSTing it starts a build, a write the Deny statement cannot reach), account:GetContactInformation / GetAlternateContact / GetPrimaryEmail and one:ListUsers (named-person PII), cloudtrail:LookupEvents (per-event username and source IP), and the four write actions hiding behind read-sounding names -- cloudformation:Detect* (AWS classes it Write; it schedules drift detection and mutates stack state), cloudwatch:GenerateQuery, logs:StopLiveTail, logs:StopQuery. - Drop glue:Describe*, which matches no real Glue action, and one of the two copies of notifications:List*. Verified with a from-scratch IAM evaluator: 232/232 collector actions still allowed, every action above resolves to explicitDeny or implicitDeny, and no action appears in both Allow and Deny. readonly-part1 headroom 223 -> 545. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0126R5BKvayttBu8nKmFZBVR * fix(k8s): drop the unused get verb and harden the terraform collector Under an assume-breach model the collector's RBAC and pod spec are what an attacker inherits, so both are cut to what the code actually needs. - Drop `get` from all 38 grants. `collect.go` has no `.Get(` call on any kind, including CRDs; all five list sites go through `.List`, so `get` was dead weight in the ClusterRole a compromised image would hold. - Grant `nonResourceURLs: ["/version"]`. `Discovery().ServerVersion()` is called and was never granted; it works today only because EKS, GKE and AKS bind `system:public-info-viewer` to `system:authenticated`, and under `Strict: true` a cluster without that default fails collection outright. - Give the terraform CronJob the security context the Helm chart already sets (no privilege escalation, read-only root filesystem, run as non-root, drop ALL capabilities). Terraform-deployed collectors ran with none of it. - Add the trailing newline `terraform-v6/modules/k8s-resources/main.tf` was missing, the same defect this branch already fixed in its `data.tf`. All three manifests stay byte-identical in what they grant: 38 resources at `list`, plus `/version` at `get`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0126R5BKvayttBu8nKmFZBVR * fix(aws): cut the role to the actions the collectors call The readonly policy was AWS's SecurityAudit shape minus a deny list, so it granted ~300 services the collectors never touch. If Nullify is compromised the attacker inherits this role in every onboarded account, and that unused surface is pure attacker capability with no scanning value. Rebuild the two readonly policies as an explicit allow list: 433 account-wide Allow entries -> 107. - Drop 134 services outright, none of which any collector calls: sdb, lakeformation, lightsail, support, workmail, rekognition, sso, sso-directory, identitystore, route53domains, storagegateway, servicecatalog, waf, machinelearning, the mediapackage family and the rest. - Drop cloudformation, elasticbeanstalk, amplify and serverlessrepo, which have no processor at all and leak DescribeStacks parameters, Beanstalk plaintext env config, Amplify basicAuthCredentials and presigned source archives. - Drop `iam:Simulate*`, whose only use to an attacker holding the IAM graph is testing escalation paths offline without generating AccessDenied in CloudTrail. - Narrow 9 families whose wildcard mixes configuration reads with a data or secret sibling: `lambda:Get*` (GetLayerVersion returns a presigned code URL), `s3:List*` (ListBucket enumerates object keys), `guardduty:Get*` (GetFindings), `appsync:List*` (ListApiKeys), `cognito-idp:Describe*`/`List*`, `cloudtrail:Get*`, `ssm:List*`, `states:Describe*`. - Keep `ec2:Describe*` and `iam:Get*`/`List*` as wildcards deliberately. Their families are inventory metadata, enumerating them is brittle, and `iam:Get*`/`List*` also cover the OIDC-provider reads in admin/internal/cloud/cloudpolicies that a processors-only derivation misses. Verified against 234 required actions drawn from the collectors, the admin preflight and cloudpolicies: all 234 still resolve to allowed, zero regressions. Every dropped item was confirmed to have no caller anywhere in context, admin, cspm or scpm. readonly-part1 falls from 5599 to 1355 of 6144 chars. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0126R5BKvayttBu8nKmFZBVR * fix(aws): enumerate ec2 and iam instead of wildcarding them `ec2:Describe*` and `iam:Get*`/`List*` were kept as wildcards on the argument that both families are inventory metadata. That is wrong for ec2 and too generous for iam. - `ec2:Describe*` covers 192 actions; the collectors call 24. The unused remainder includes `DescribeVpnConnections`, which returns `CustomerGatewayConfiguration` with the IPsec pre-shared key in plaintext -- a live credential into the customer's on-prem network -- plus `DescribeInstanceAttribute` (UserData), `DescribeClientVpnConnections` (client identities and source IPs) and `DescribeTags` (owner emails). - `iam:Get*`/`List*` cover 67; the collectors and admin's OIDC resolver call 25. The remainder includes `GetAccountAuthorizationDetails`, which returns the entire IAM graph in one call, `GetServiceLastAccessedDetails`, which tells an attacker which grants are stale enough to abuse unnoticed, and `GetSAMLProvider`/`GetLoginProfile` for phishing the workforce. Allow 107 -> 152: enumeration is longer than the wildcards it replaces but grants far less. - Deny 15 further actions as defence in depth against a future re-broadening, matching the discipline already applied to Cognito and WorkMail. - Deny `apigateway:GET` on `/apikeys*` and `/usageplans*` via a second, resource-scoped statement. API Gateway exposes one HTTP-verb-shaped action covering its whole control plane, including `GET /apikeys?includeValues=true`; path ARNs are the only way to separate the five calls the collectors make from plaintext API key values. 234/234 required actions still resolve to allowed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(aws): correct two invalid action names and a neptune coverage gap IAM accepts a misspelled action in a policy and simply never matches it, so both of these were silent. - `medialive:GetInputDeviceThumbnail` is not an IAM action; the real one is `DescribeInputDeviceThumbnail`. Introduced in 3a1a4fa. Inert today -- nothing grants `medialive:*` any more -- but a dead Deny that a future broad grant would sail straight past. - `neptune:` is not a valid IAM prefix. Neptune's control plane is served by the RDS API and authorised with `rds:` actions, so `neptune:Describe*` and `neptune:List*` granted nothing. Drop both. - That mattered: enumerating the policy mapped `neptune.ListTagsForResource` to the dead `neptune:List*` rather than `rds:ListTagsForResource`, and the earlier `rds:List*` wildcard was gone, so the collector's Neptune tag read would have failed. Grant `rds:ListTagsForResource`. The three Describe calls were already covered by `rds:Describe*`. Every action string in all three templates was checked against AWS's policy generator dataset: 296/297 valid before this commit, and the one miss is fixed here. 231/231 required actions resolve to allowed with Neptune mapped to `rds:`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(aws): grant KMS key/* for alias ARNs, pin the collector image and look up clusters in their own region - KMS policy grants kms_key_arn and key/* in the same Nullify account and region; validation accepts key (incl. mrk-) and alias ARNs, like the CloudFormation template. - k8s-resources collector_image defaults to public.ecr.aws/w4o2j2x4/integrations:k8s-collector-3.45.0 instead of the unowned Docker Hub nullify/k8s-collector:latest. - New nullify_s3_access_point_arn grants s3:PutObject/PutObjectAcl on <ap>/object/k8s-collector/*. - v6 reads each EKS cluster in the region from its ARN; v5 fails fast with a precondition and accepts eks_oidc_issuer_urls for other regions. - multi-cluster example: region-correct lookups and exactly two clusters. - Root drops the unused kubernetes provider and the dead cronjob_schedule pass-through; cross-variable validations become preconditions so the declared Terraform >= 1.5 floor holds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(aws): fix Terraform README drift and document KMS, access point and region handling - k8s-resources file is providers.tf; basic quick start copies its own tfvars; collector_image is a k8s-resources variable, not a root one. - Module sources point at Nullify-Platform/nullify-cloud-connector. - v6 README no longer claims data.aws_eks_cluster_auth was removed. - Requirements (Terraform >= 1.5) and upgrade notes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * ci: validate terraform-v6 and run each AWS row at its declared Terraform floor Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(aws): add Terraform modules for managed EKS scan access - modules/eks-managed-scan-access (v5 and v6): one STANDARD access entry per cluster, rbac by default (group nullify-readonly) or AmazonEKSAdminViewPolicy at cluster scope as a warned opt-in. Preconditions reject CONFIG_MAP clusters and cross-account clusters (v5: clusters outside the provider region). Entries are replaced when the role's unique_id changes. A check block warns when Nullify's regional egress IPs are missing from publicAccessCidrs; the module never manages them and outputs a merge command instead. - k8s-resources: enable_collector (default true, moved blocks keep state) and enable_managed_scan_rbac, a list-only nullify-readonly ClusterRole on the 26 scanned kinds plus get /version, bound to the group. - nullify-aws-integration: role_unique_id output. - terraform test suites with mocked providers for all three modules. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(aws): wire managed EKS scan into the root and examples - Root: enable_managed_scan, managed_scan_cluster_arns, nullify_region, managed_scan_authorization (default rbac), managed_scan_kubernetes_group, and managed-scan outputs. v5 root requires AWS provider ~> 5.33. - New examples/managed-scan: one cluster, access entry plus the list-only ClusterRole, manage_rbac = false for GitOps-applied RBAC. - multi-cluster-complete: scan_mode collector (default) | managed | both. v5 uses one access-entry module per regional provider, v6 one instance. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(aws): document the managed EKS scan modules, RBAC, endpoint allowlist and upgrade notes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * ci: validate the managed-scan module and examples and run terraform test with mocked providers Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(aws): convert publicAccessCidrs to a list before building the update command terraform test caught concat() rejecting the set returned by data.aws_eks_cluster, which would fail every plan where a Nullify egress IP is missing. The test mocks aws_iam_policy_document with a valid policy so provider validation accepts the mocked JSON. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * ci: run the terraform test suites at the lowest version that can run them The modules declare required_version >= 1.5, but the validate rows only fmt and validate there, which never evaluate preconditions or check blocks, and the test job pinned 1.10.5 for every row. - terraform-test gains a version dimension: 1.8.5 and 1.10.5 - 1.8.5 is the floor for the suite itself: mock_provider is 1.7+, and the tests use strcontains, which is 1.8+ Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(terraform): name each collector's cluster and narrow the KMS grant Round-1 review of the managed EKS scan stack. - k8s-resources sets CLUSTER_NAME on the CronJob from a new cluster_name variable, so collectors stop sharing the k8s-collector/default-name-data.json upload key; a precondition requires it while enable_collector is true, and the multi-cluster examples pass each cluster's own name - eks-managed-scan-access clears kubernetes_groups with [] rather than null: the attribute is Optional+Computed, so null kept the stale group forever - nullify-aws-integration derives key/* only for an alias ARN; a key ARN is granted alone, as the CloudFormation template already does - collector_image pins k8s-collector-3.46.0, the build the Helm chart's k8s-collector-latest tag resolves to - README documents that the clusters must exist before the first plan, with the -target two-step, and the cluster_arns message says the same - docs describe the RBAC requirement instead of pointing at a chart, a manifest and a tag that exist on no merged branch - the RBAC test pins the exact kind set per API group, not just the counts Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(terraform): note the cluster_name requirement in the upgrade notes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(terraform): require KMS for the collector and narrow the key/* grant to cross-account Both published collector images refuse to upload without KMS encryption, so a Terraform deployment that left kms_key_arn empty collected the cluster and then failed every upload. cluster_name had two undocumented requirements that fail silently server-side. scan_mode = "both" registered one cluster twice. - k8s-resources: kms_key_arn required while enable_collector is true, with allow_unencrypted_upload as the only opt-out and the only setter of ALLOW_UNENCRYPTED_UPLOAD - cluster_name docs and precondition state the exact-name and registered-cluster requirements; README example uses a real-looking name - nullify-aws-integration: derive key/* for alias and key ARNs alike, only when the ARN's account is not the caller's - examples/multi-cluster-complete: reject scan_mode = "both" - use data.aws_partition for constructed ARNs, and the cluster ARN's own partition for the EKS access policy ARN - declare Terraform >= 1.8 and validate at 1.8.5, the version the test suites run - correct the upgrade note that told customers to delete an object their role is denied s3:DeleteObject* on Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(terraform): drop the unencrypted-upload opt-out and reject collector + managed scan together allow_unencrypted_upload could not work against Nullify's bucket: an empty kms_key_arn also sets enable_kms_access = false, so the role gets no KMS statement, and the bucket's SSE-KMS default encryption still requires kms:GenerateDataKey on Nullify's key. The variable traded the collector's self-explaining error for an opaque S3 AccessDenied, so it is removed and kms_key_arn is unconditionally required when enable_collector is true. - k8s-resources: remove allow_unencrypted_upload and the ALLOW_UNENCRYPTED_UPLOAD env block; the KMS precondition is now kms_key_arn != "" alone - k8s-resources: precondition rejecting enable_collector together with enable_managed_scan_rbac, matching the example's scan_mode = "both" rejection - nullify-aws-integration: kms_key_arn description now states the cross-account rule the locals actually implement - terraform/README.md: fold #88's bare-key-form note into the rewritten KMS section so merging both PRs cannot leave two near-duplicate paragraphs - multi-cluster-complete: comment out the placeholder KMS ARN and shape it so the variable regex rejects it if pasted as-is - CI: validate rows for the four remaining modules/ directories Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(terraform): move the collector/managed-scan duplicate guard to the access entry and drop phantom upgrade notes The precondition guarding against duplicate cluster registration lived on k8s-resources' ClusterRole, so authorization = admin_view_policy (which creates no Kubernetes objects) never triggered it: a direct caller running the collector and a managed scan on one cluster in that mode still plans clean. The same guard also rejected a legitimate configuration -- staging the ClusterRole ahead of a cutover with no access entry behind it yet. - Move the guard to eks-managed-scan-access's access entry (created in every authorization mode) via a new collector_cluster_arns input, keyed on cluster name; remove the now-redundant precondition from k8s-resources' ClusterRole so staging is allowed again. - Wire collector_cluster_arns through the root configs and the multi-cluster-complete examples in both terraform/ and terraform-v6/. - Add terraform test rows for both directions: rejected under admin_view_policy, and unaffected when the collector cluster differs. - Drop three Upgrade Notes bullets that describe variables (allow_unencrypted_upload, enable_managed_scan_rbac, scan_mode) absent from origin/main and both release tags -- there is no upgrade path they describe. - Add the missing Upgrade Notes section to terraform-v6/README.md with the real (non-phantom) bullets, mirrored from v5. - Fix the KMS placeholder ARN in the three remaining tfvars.example files to match the corrected multi-cluster-complete one (R3-8): the key is in Nullify's account, not the customer's. - Remove the four terraform-validate rows for module directories already covered end-to-end by the terraform-test job, and correct the comment justifying them (both matrices are hand-maintained, not auto-discovered). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(terraform): compare full cluster ARNs in the collector duplicate guard and root-derive it correctly The guard added in the previous round compared bare cluster names, so two clusters sharing a name in different regions/accounts collided with no override in the v6 root. It also derived collector_cluster_arns from the IRSA trust list, which is a prerequisite for a collector, not evidence one is deployed. - Compare full cluster ARNs instead of names in eks-managed-scan-access (both trees); add a same-name-different-region test row in both. - Add a root collector_cluster_arns override variable (both trees) so root users can correct the IRSA-trust-list proxy and decouple a collector-to-managed-scan cutover from revoking IRSA trust. - Reword the guard's error message to cover the Helm/manifest paths its own variable description already names. - Add ARN-shape validation to collector_cluster_arns, matching cluster_arns. - Remove the dead collector_cluster_arns wiring in both multi-cluster examples: scan_mode's mutual exclusivity makes it always []. - Fix terraform-v6/README.md still documenting the enable_collector + enable_managed_scan_rbac rejection this PR's base commit removed; add the missing collector_cluster_arns doc and Kubernetes-provider-removal upgrade note to match the v5 tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(terraform): tell collector-cutover instructions to stop the collector first collector_cluster_arns docs told users to override the guard during a collector-to-managed-scan cutover but never said to disable the collector first, so following them literally leaves the collector uploading while the cluster also gets managed-scan access -- the duplicate registration the guard exists to prevent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(aws): drop collector ClusterRole grants the collector never lists whiskeylotus's review on #58 (W58-2) flagged that the collector ClusterRole grants 12 kinds main's collect.go never lists: jobs is kept since the monorepo's fix/k8s-deployment-matching-review stack (#12807, which #12830 sits on) starts collecting Jobs owned by a CronJob; cronjobs, horizontalpodautoscalers, poddisruptionbudgets, ingressclasses, storageclasses, csidrivers, runtimeclasses, priorityclasses, flowschemas, certificatesigningrequests and customresourcedefinitions are dropped -- no open collector change reads any of them. Verified against context/internal/cloudintegrations/k8sintegration/ collect/collect.go on both the monorepo's main and fix/k8s-deployment-matching-review branches. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(aws): drop collector ClusterRole grants the collector never lists (#99) whiskeylotus's review on #58 (W58-2) flagged that the collector ClusterRole grants 12 kinds main's collect.go never lists: jobs is kept since the monorepo's fix/k8s-deployment-matching-review stack (#12807, which #12830 sits on) starts collecting Jobs owned by a CronJob; cronjobs, horizontalpodautoscalers, poddisruptionbudgets, ingressclasses, storageclasses, csidrivers, runtimeclasses, priorityclasses, flowschemas, certificatesigningrequests and customresourcedefinitions are dropped -- no open collector change reads any of them. Verified against context/internal/cloudintegrations/k8sintegration/ collect/collect.go on both the monorepo's main and fix/k8s-deployment-matching-review branches. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(terraform): reconcile K8s scan-mode RBAC and address #81 review feedback (#98) * fix(terraform): reconcile K8s scan-mode RBAC and fix review nits from #81 Applies dnjg's review feedback from ncc-cloud-connector #81 to #78's collapsed branch. - Align the in-cluster collector's Kubernetes RBAC (Helm chart and the Terraform k8s-resources module, v5 and v6) with the managed scan's ClusterRole and with collect.go, so switching scan mode no longer silently changes which kinds get scanned (N78-2, N78-3). - Document why both scan modes list Secrets without reading values: the managed scan reads Kubernetes' server-side Table representation, the in-cluster collector redacts before upload (N78-2). - Fix the documented terraform import address for the EKS access entry, covering the root module's count index, a direct module call and the multi-cluster example (N78-4). - Cite the platform monorepo's iac/network/vpc module as the source of the hardcoded Nullify egress IPs, in both the module comment and the README (N78-5). Serving the list from an API is left as a product follow-up, not built here. - Reword the ARN partition validation errors to say only the commercial aws partition is supported, instead of reading like a malformed-ARN typo (N78-6). - Add terraform test assertions on the KMS, S3 access point and readonly aws_iam_policy_document statement blocks: the existing mock_provider fakes every document's computed json, so no test previously asserted an actual IAM action or resource (N78-8). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * ci(terraform): add an IAM policy drift check across CFN and Terraform whiskeylotus's review on ncc #58 (W58-3) asks for a job that verifies the CloudFormation template and both Terraform trees keep granting the same Allow/Deny action sets for the customer readonly role -- nothing currently checks that the three independently maintained templates stay identical. Adds scripts/check_iam_policy_drift.py, comparing the CloudFormation template's ReadOnlyAccessPolicy(+2)/DenyActionsPolicy statements against readonly_policy_part1/part2/deny_actions_policy in both aws-integration-setup/terraform and terraform-v6, and wires it as a new job beside the existing terraform-validate matrix. Verified locally: passes against the current templates (437 Allow, 45 Deny actions matching on both trees) and correctly fails when an action is injected into one side only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(helm): collector chart namespace, service account and default-value fixes (#63) * fix(helm): collector chart namespace, service account and default-value fixes - Remove the pre-install namespace hook and the bitnami/kubectl Job; the namespace now comes from --create-namespace, and a namespace helper falls back to the release namespace when serviceAccount.namespace is empty (default stays nullify for the IRSA trust). - Use the service-account-name helper in the CronJob and ServiceAccount. - Default clusterName, s3.bucket and kms.keyArn to "" and fail on empty or placeholder values, on a KMS alias instead of a key ARN, on a region that differs from the key's, and on a missing EKS role-arn or GKE role ARN. - Remove collector.dataCollection, which the binary never read. - Fix the labels helper joining custom labels onto the previous line, and apply nodeSelector/tolerations to the collector pod. - kubeVersion >=1.21, chart 0.4.0, and README/NOTES corrections. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(helm): expect the role-arn guard for the collector defaults case Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(helm): give the exact fix in the collector's KMS alias and region errors - An alias keyArn still fails, now saying to use the key ARN from the Nullify configure page and to contact Nullify if the page shows an alias. - The region mismatch error explains that --reuse-values from 0.2.0 carries us-east-1, and to upgrade with --reset-then-reuse-values or set the region. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(helm): accept every AWS partition and allow opting out of the collector role-arn guard - The EKS role-arn guard accepts arn:aws, arn:aws-us-gov and arn:aws-cn. - serviceAccount.requireRoleArn (default true) lets EKS Pod Identity, instance-profile and kube2iam/kiam installs render without IRSA; an empty role-arn annotation is no longer rendered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(helm): document the alias migration, requireRoleArn and --reuse-values upgrades Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(helm): accept KMS alias ARNs in the collector and keep the role ARN commercial - collector.kms.keyArn accepts arn:aws:kms:<region>:<account>:alias/<name> and key ARNs including multi-Region key/mrk-..., matching the connector CloudFormation AllowedPattern. The region default and check take the region from either form. - NOTES warns when an alias is used: IAM matches the key ARN, so the role needs the connector stack version that also grants key/*; the Terraform module grants only the given ARN. - The role-arn guard is back to arn:aws:iam:: only; Nullify's bucket and key are commercial-partition. serviceAccount.requireRoleArn=false stays the escape hatch. ci/eks-govcloud-role.yaml is removed. - CI: ci/eks-kms-alias.yaml and ci/eks-kms-mrk.yaml must render; a bare alias, a GovCloud alias ARN, an alias region mismatch and a GovCloud role ARN must fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(helm): keep ci/ out of the collector chart package - .helmignore excludes ci/ from 0.4.0, so a later edit to a CI values file does not change a published version's contents and force a version bump. - Added with the 0.4.0 bump because 0.2.0 is already published; the file changes the package contents. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(helm): state the KMS alias requirement instead of pointing at the current template - NOTES, README and values.yaml say what an alias in collector.kms.keyArn needs: the IAM role's KMS statement must cover the resolved key, for example arn:aws:kms:<region>:<account-id>:key/*. - CloudFormation: connector releases that include PR #64 grant key/* in the account and region of NullifyKMSKeyArn; the template on main grants only the ARN given. Terraform: the module accepts an alias; check the module version's KMS statement covers key/*. A key ARN works with every version. - New README section "KMS alias ARNs" holds the detail; the table, 0.4.0 upgrade notes and troubleshooting link to it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(helm): refuse to unpublish live chart versions and document the merge order main's helm-release.yml rebuilds the Pages site from the collector chart alone, so merging this chain before the publish rework unpublishes collector 0.2.0 and any other version the live index serves. - the release job compares the live index against the single package it would deploy and fails, naming the required merge order, when it would drop one - README gains a maintainer section stating that order and why Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(terraform): finish W58-2 in both collector-role trees #99 dropped the 11 never-collected kinds (cronjobs, HPA, PDB, ingressclasses, storageclasses, csidrivers, runtimeclasses, priorityclasses, flowschemas, certificatesigningrequests, CRDs) from the Helm ClusterRole only. The Terraform collector roles in both trees still granted them, so a k8s-resources install of #58 kept read access to kinds no collector reads, and the two collector roles differed by those 11 kinds. jobs is kept. Helm (0.3.0), TF v5 and TF v6 now grant identical kinds and verbs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(terraform): describe today's Secrets handling and list batch/jobs in managed-scan RBAC - Drop the server-side Table fetch claim and the per-cluster opt-out claim: neither exists in the deployed collector or managed scan. - Kinds-read row: identical from k8s-resources and chart 0.3.0; 0.2.0 grants extra kinds. - Add the batch/jobs row managed_scan.tf already grants. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(terraform): match #58's networking rule formatting and state chart 0.2.0's kinds - k8s-resources main.tf (v5, v6): write the networking.k8s.io resources list the way #58 does, so a squash merge of #58 does not conflict on a formatting-only hunk. - README kinds-read row: identical from chart 0.4.0 (0.3.0 is never published); 0.2.0 grants a different set, not a superset. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(aws): drop AdminView and tighten secret-shaped IAM RBAC is the only EKS authorization mode. Scope GetDocument, deny UserData and EventBridge target Input, pin KMS key/* to alias ARNs, and allow ECR image pull for scanning. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(aws): pin ssm:GetDocument to the Session Manager document partition Co-authored-by: Cursor <cursoragent@cursor.com> * fix(ci): compare partition-scoped IAM ARNs in the CFN/Terraform drift check Fn: :Sub and the Terraform partition interpolation are the same GetDocument grant; hashing the raw CFN dict crashed the check after that action was scoped. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(aws): deny SSM document bodies and pin the collector image DescribeDocument and DescribeAutomationExecutions return runbook content and automation InputParameters; deny-wins over ssm:Describe*. Helm now defaults to the same k8s-collector-3.46.0 tag as Terraform. * fix(k8s): drop secrets list so Secret values cannot be read Kubernetes list on secrets returns .data; omit the grant from collector and managed-scan ClusterRoles. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The collector chart runs a pre-install
bitnami/kubectl:latestJob whose image no longer pulls and whoseexit 0hides failures, ships placeholder defaults that fail late at S3, and documents values the binary never reads. Merge after #60, then after #58 (stacked on it): a Helm chart change merged before #60 runs the old workflow, which replaces the Pages site and unpublishes collector 0.2.0. Review fixes are in #68, and neither PR releases until monorepo #12787 is deployed.templates/pre-install-job.yaml(unpinnedbitnami/kubectl:latest,defaultSA cannot create namespaces,exit 0hides it) andtemplates/namespace.yaml(pre-install hook that conflicts with--create-namespace/ FluxcreateNamespace). Dropnamespace.create/requireNamespace.k8s-collector.namespacehelper,serviceAccount.namespacefalling back to the release namespace. The default staysnullify, because the connector CloudFormation IRSA trust pinssystem:serviceaccount:nullify:nullify-k8s-collector-sa.serviceAccountNamehelper, as the binding already did.collector.clusterName,collector.s3.bucketandcollector.kms.keyArnbecome"", and each fails when empty or still the old placeholder.keyArnmust match^arn:aws:kms:[a-z0-9-]+:[0-9]{12}:key/; the fail message explains why an alias is denied.cloudProvider=aws, andcollector.gke.awsRoleArnis required forgcp.collector.aws.regiondefaults to the key ARN's region; any other value fails, since S3 only accepts a KMS key from the bucket's region.collector.dataCollection.*and its env vars: the binary never readsEXCLUDE_NAMESPACES/INCLUDE_RESOURCES/METADATA_ONLY. NOTES warns if they are still set.{{- toYaml }}glued customlabelsonto themanaged-byline.nodeSelector/tolerationsnow reach the CronJob pod; only the deleted Job used them.kubeVersion: ">=1.21.0-0"(batch/v1 CronJob). Chart 0.3.0 -> 0.4.0 (on feat: grant wafv2:Get*, ec2:GetEbsEncryptionByDefault and the missing Kubernetes list kinds #58's bump; fold into 0.3.0 if you prefer).s3:GetObject/ListBucketclaim (CFN denies GetObject); namesnullify-k8s-collector; IRSA via the CFN/Terraform EKS integration; region = bucket's; upgrade notes; pin an image tag once published (default unchanged).ci/eks.yaml,ci/gke.yaml, 8must-failcases for fix(helm): publish every chart version instead of replacing the Pages site #60's test script; root README andIMPLEMENTATION.mddropdataCollection.Nothing was rendered locally. #58's workflow has no pull_request trigger, so CI for this chart ran on throwaway draft #62 (this branch merged onto #60, now closed). Run 34864195067: lint/render passed with both
ci/files, all 8 must-fail cases were rejected with their expected message, and the dry-run publishes 0.4.0 next to 0.2.0.Collapsed stack
All four rounds' commits are merged into this branch (merge shas in the collapse report); round 5 adversarial review judged this stack's code converged.
Merge order (required)
helm-release.ymlwould redeploy the index with only collector 0.3.0 and unpublish 0.2.0.helm-release.ymlverbatim — it drops fix(helm): refuse to unpublish live chart versions and document the merge order #86's legacy guard.🤖 Generated with Claude Code