feat: implement evidence-to-admissibility enforcement pipeline#22299
feat: implement evidence-to-admissibility enforcement pipeline#22299BrianCLong wants to merge 1 commit intomainfrom
Conversation
BrianCLong
added
the
codex
ChatGPT Codex Connector
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a supply chain security framework featuring a TypeScript-based admissibility evaluation engine, CI scripts for evidence generation, and Kubernetes resources for enforcement. Key components include a Kyverno ClusterPolicy for image signature and SLSA attestation verification, alongside an ArgoCD PreSync hook to gate deployments. Feedback identifies several critical issues: the Kyverno policies and ArgoCD checks use overly broad scopes (such as wildcard image references and permissive GitHub subjects) that could disrupt cluster operations or bypass security gates. Furthermore, hardcoded 'PASS' values in manifests and missing ServiceAccount permissions for the PreSync Job must be resolved to ensure the security framework is effective and functional.
| - Pod | ||
| verifyImages: | ||
| - imageReferences: | ||
| - "*" |
There was a problem hiding this comment.
Using * for imageReferences in a ClusterPolicy will subject every container in the cluster to these verification rules, including critical system components (e.g., kube-proxy, coredns, or Kyverno itself). Unless all images in the cluster are signed by the specified issuer, this will block system pods from starting, potentially breaking the cluster.
- "ghcr.io/your-org/*"| entries: | ||
| - keyless: | ||
| issuer: https://token.actions.githubusercontent.com | ||
| subject: https://github.com/*/* |
There was a problem hiding this comment.
The keyless attestation subject is overly permissive. Setting it to https://github.com/*/* allows any GitHub repository to produce valid signatures that satisfy this policy. This should be restricted to your specific organization or repository to ensure only trusted builds are admitted.
subject: https://github.com/your-org/*| - Pod | ||
| verifyImages: | ||
| - imageReferences: | ||
| - "*" |
There was a problem hiding this comment.
Similar to the signature check, using * for SLSA attestation verification will block all images in the cluster that lack these specific attestations. This should be scoped to your specific container registry to avoid breaking third-party or system components.
- "ghcr.io/your-org/*"| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: admissibility-status | ||
| namespace: argocd | ||
| data: | ||
| status: "PASS" |
There was a problem hiding this comment.
Including the admissibility-status ConfigMap with a hardcoded PASS value in the same manifest as the PreSync check defeats the purpose of the gate. Since ArgoCD applies resources during the sync process, this manifest will overwrite any existing status in the cluster with PASS, effectively bypassing the check. Consider removing this ConfigMap from the base manifest and managing it via an external process (e.g., a CI runner) that reflects actual build admissibility.
| spec: | ||
| restartPolicy: Never |
There was a problem hiding this comment.
The Job does not specify a serviceAccountName. By default, it uses the default ServiceAccount, which typically lacks permissions to get ConfigMaps in the argocd namespace. This will cause the kubectl command to fail with a Forbidden error, blocking the sync even if the status is correct.
spec:
serviceAccountName: admissibility-checker-sa
restartPolicy: Never| metadata: | ||
| labels: | ||
| app: summit-server | ||
| security.summit.io/admissibility: "PASS" |
There was a problem hiding this comment.
Hardcoding the admissibility label to PASS in the deployment manifest allows any deployment to satisfy the Kyverno label requirement regardless of whether the CI pipeline actually verified the build. To make this an effective gate, this label should be injected dynamically by the CI process or a mutating webhook only after verification is complete.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2b0ca7c69a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| kinds: | ||
| - Pod |
There was a problem hiding this comment.
This rule currently matches all Pod objects cluster-wide, so the new enforcement applies to system namespaces and ArgoCD hook Pods as well as Summit workloads. Because most existing Pods (including k8s/argo/admissibility-presync-check.yaml's hook Pod) do not carry security.summit.io/admissibility: "PASS" or matching GitHub keyless signatures/attestations, admissions will be denied broadly and can halt routine sync/deploy activity. Scope this policy to the intended namespaces/images/workloads (or add explicit excludes) before enforcing.
Useful? React with 👍 / 👎.
| name: admissibility-status | ||
| namespace: argocd | ||
| data: | ||
| status: "PASS" |
There was a problem hiding this comment.
The PreSync job reads admissibility-status and fails only when it is not PASS, but this manifest creates that ConfigMap with status: "PASS" unconditionally. Since no CI output is wired into this value, the hook always succeeds and cannot block sync when admissibility evidence fails, making the runtime gate effectively non-enforcing.
Useful? React with 👍 / 👎.
WalkthroughThis PR introduces a comprehensive admissibility supply chain security gate system. It includes a GitHub Actions workflow that generates deterministic build evidence (SBOM, provenance, signatures), Kubernetes enforcement policies via Kyverno, TypeScript evaluation logic, verification scripts, and supporting tests to enforce supply chain integrity checks on container images and deployments. Changes
Sequence DiagramsequenceDiagram
participant GHA as GitHub Actions
participant Build as Build Process
participant SBOM as SBOM Generator<br/>(syft)
participant Attest as Build Attestor
participant Cosign as Cosign Signer
participant Script as Evidence Script
participant Artifact as Artifact Storage
GHA->>Build: Checkout & Setup
GHA->>SBOM: Generate SBOM
SBOM->>SBOM: Parse components
SBOM-->>GHA: sbom.cdx.json
GHA->>Attest: Create provenance
Attest->>Attest: Embed SHA-256 digest
Attest-->>GHA: provenance.json
GHA->>Cosign: Sign artifact
Cosign->>Cosign: Generate key pair
Cosign-->>GHA: artifact.sig
GHA->>Script: Run evidence builder
Script->>Script: Read SBOM/provenance
Script->>Script: Extract components<br/>& subjects
Script->>Script: Determine signature<br/>validity
Script->>Script: Build dependency list
Script-->>GHA: report.json<br/>metrics.json<br/>stamp.json
GHA->>Artifact: Upload evidence<br/>bundle
participant K8s as Kubernetes
participant Kyverno as Kyverno Policy
participant Pod as Pod Admission
K8s->>Kyverno: Pod creation event
Kyverno->>Kyverno: Check admissibility label
Kyverno->>Kyverno: Verify image signatures<br/>(keyless attestor)
Kyverno->>Kyverno: Verify SLSA<br/>provenance
Kyverno-->>Pod: Enforce/Block
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (6)
scripts/ci/build_admissibility_evidence.mjs (2)
18-19: Add error handling for SBOM/provenance JSON parsing.If
sbomPathorprovenancePathcontain invalid JSON or don't exist, the script will crash with an unhelpful error. For CI reliability, wrap these in try/catch with clear error messages.♻️ Proposed improvement
-const sbom = JSON.parse(readFileSync(sbomPath, 'utf8')); -const provenance = JSON.parse(readFileSync(provenancePath, 'utf8')); +let sbom, provenance; +try { + sbom = JSON.parse(readFileSync(sbomPath, 'utf8')); +} catch (err) { + console.error(`Failed to parse SBOM from ${sbomPath}:`, err.message); + process.exit(1); +} +try { + provenance = JSON.parse(readFileSync(provenancePath, 'utf8')); +} catch (err) { + console.error(`Failed to parse provenance from ${provenancePath}:`, err.message); + process.exit(1); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/ci/build_admissibility_evidence.mjs` around lines 18 - 19, Wrap the JSON parsing of sbom and provenance in try/catch blocks to handle file read or parse failures: when calling JSON.parse(readFileSync(sbomPath, 'utf8')) and JSON.parse(readFileSync(provenancePath, 'utf8')), catch errors from readFileSync/JSON.parse and log/throw a clear CI-friendly message that includes the path (sbomPath or provenancePath) and the original error message; ensure the script exits non-zero after logging so failures are surfaced in CI.
35-40: SBOM completeness check may be too permissive.
complete: components.length > 0only verifies that at least one component exists. A partial or incomplete SBOM (e.g., missing transitive dependencies) would still be marked complete. If stricter SBOM validation is intended, consider validating against expected component counts or required fields.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/ci/build_admissibility_evidence.mjs` around lines 35 - 40, The current sbom object sets complete: components.length > 0 which only ensures one component exists; update the completeness logic in the sbom construction to perform stricter validation (e.g., require components.length >= expectedCount when available, or validate that every item in components contains required fields like name, version, and purl, and that transitive dependencies are present) by replacing the simple length check with a predicate that uses components.every(...) for required fields and/or compares against an expected components count; reference the sbom object, the complete property, and the components variable to locate and update the check.scripts/ci/verify-admissibility.ts (1)
9-11: Add error handling for file read and JSON parse failures.The script will crash with an unhelpful stack trace if the file doesn't exist or contains invalid JSON. For a CI gating script, clearer error messages would aid debugging.
♻️ Proposed improvement
+let parsed: AdmissibilityEvidenceBundle; +try { + const content = readFileSync(reportPath, "utf8"); + parsed = JSON.parse(content) as AdmissibilityEvidenceBundle; +} catch (err) { + console.error(`Failed to load evidence from ${reportPath}:`, err instanceof Error ? err.message : err); + process.exit(1); +} -const reportPath = resolve(process.cwd(), inputPath); -const parsed = JSON.parse(readFileSync(reportPath, "utf8")) as AdmissibilityEvidenceBundle; -const verdict = evaluateAdmissibility(parsed); +const verdict = evaluateAdmissibility(parsed);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/ci/verify-admissibility.ts` around lines 9 - 11, Wrap the file reading and JSON parsing around a try/catch and add explicit checks so failures produce clear CI-friendly errors: verify the file exists (use existsSync or fs.stat) for reportPath before calling readFileSync, then catch JSON.parse errors when constructing parsed as AdmissibilityEvidenceBundle and log a descriptive error with the file path and error.message, call process.exit(1) on failure, and only call evaluateAdmissibility(parsed) when parsing succeeds; reference symbols to change: reportPath, inputPath, resolve/process.cwd, readFileSync, JSON.parse, parsed, AdmissibilityEvidenceBundle, and evaluateAdmissibility.lib/admissibility.ts (1)
76-80: Boolean namingprohibitedDependenciesis semantically inverted.
prohibitedDependencies = truemeans no prohibited dependencies were found, which is counterintuitive. Consider renaming tonoprohibitedDependenciesordependenciesCleanfor clarity:♻️ Suggested renaming for clarity
In the
checksinterface (line 44) and usage:checks: { signatureValid: boolean; sbomComplete: boolean; provenanceIntact: boolean; - prohibitedDependencies: boolean; + noProhibitedDependencies: boolean; };And in the function:
- const prohibitedDependencies = evidenceBundle.dependencies.prohibitedFound.length === 0; - if (!prohibitedDependencies) { + const noProhibitedDependencies = evidenceBundle.dependencies.prohibitedFound.length === 0; + if (!noProhibitedDependencies) { const blocked = sortUnique(evidenceBundle.dependencies.prohibitedFound).join(","); reasons.push(`PROHIBITED_DEPENDENCIES:${blocked}`); }This aligns with the other check names where
truemeans "passing."Also applies to: 44-44
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/admissibility.ts` around lines 76 - 80, The boolean variable prohibitedDependencies is inverted (true means no prohibited deps) and should be renamed to reflect a passing state; change prohibitedDependencies to a clearer name such as dependenciesClean or noProhibitedDependencies in both the checks interface and its usage in the function so that true means "passing" (update the declaration using evidenceBundle.dependencies.prohibitedFound.length === 0 and the conditional that pushes PROHIBITED_DEPENDENCIES to use the new name, and adjust any references to prohibitedDependencies elsewhere in the checks interface and code)..github/workflows/admissibility-gate.yml (2)
85-96: Verifycosign.keyis not inadvertently uploaded.The artifact upload includes
cosign.pubbutcosign.key(the private key) remains in the workspace. While it's not explicitly listed, confirm the path patterns don't capture it. The current explicit paths look safe, but if the step is ever changed to use a glob pattern, the private key could leak.Consider adding explicit cleanup of the private key before upload:
🧹 Optional: explicit cleanup
+ - name: Remove ephemeral private key + run: rm -f cosign.key + - name: Upload evidence artifacts uses: actions/upload-artifact@v4🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/admissibility-gate.yml around lines 85 - 96, The "Upload evidence artifacts" step currently lists cosign.pub but leaves cosign.key in the workspace; add an explicit pre-upload cleanup or exclusion to ensure cosign.key (the private key) is removed or never present when artifacts are uploaded: update the job step named "Upload evidence artifacts" to run a short cleanup step (or an exclude pattern) that securely deletes or moves evidence/cosign.key before the actions/upload-artifact@v4 invocation, and ensure only the intended files (evidence/report.json, evidence/metrics.json, evidence/stamp.json, evidence/sbom.cdx.json, evidence/provenance.json, evidence/artifact.sig, cosign.pub) are retained for upload.
36-39: Artifact payload may not be deterministic across environments.Line 39 computes hashes then sorts the hash values. Since
sha256sumoutputs<hash> <filename>andawk '{print $1}'extracts only the hash, the sort operates on hashes rather than filenames. If the shell glob orsha256sumreturns files in a different order across runs, sorting the hashes (not the filename-hash pairs) could yield different orderings for files with lexicographically ordered hashes.Consider sorting by filename first, then hashing:
♻️ Proposed fix for deterministic ordering
- name: Build deterministic artifact payload run: | mkdir -p dist - sha256sum package.json pnpm-lock.yaml | awk '{print $1}' | sort > dist/admissible-artifact.txt + sha256sum package.json pnpm-lock.yaml | sort -k2 | awk '{print $1}' > dist/admissible-artifact.txtThis ensures the file order is consistent before extracting hashes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/admissibility-gate.yml around lines 36 - 39, The current pipeline extracts only hashes then sorts them, which can produce non-deterministic ordering; run sha256sum for the target files (package.json and pnpm-lock.yaml), sort the filename-hash pairs by filename (e.g. using sort keyed on the filename column) before extracting the hash with awk, and write the resulting ordered hashes to dist/admissible-artifact.txt so the artifact payload is deterministic; modify the invocation that currently uses sha256sum, awk, and sort to sort by filename (column 2) prior to printing the hash.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/admissibility-gate.yml:
- Around line 68-72: The workflow currently generates a transient keypair with
cosign (cosign generate-key-pair) and signs with cosign.key/cosign.pub, which
prevents stable verification and risks leaking the private key; replace this
with keyless signing by removing the generate-key-pair step and invoking cosign
sign-blob without --key (using OIDC) and verify with the corresponding cosign
verify-blob call, or if keyless is not acceptable, load a persistent private key
from a GitHub Secret instead of writing cosign.key to the workspace and use that
secret-provided key for cosign sign-blob and verify-blob (ensure the secret
never gets persisted to disk).
In `@k8s/argo/admissibility-presync-check.yaml`:
- Around line 6-7: The ConfigMap currently hardcodes data.status: "PASS", which
defeats the PreSync gate; change the manifest so it does not contain a static
"PASS" (remove the fixed value or replace it with a placeholder like an
unset/templated value) and update the CI/CD pipeline to create/update the
ConfigMap's data.status at runtime with the real admissibility verdict;
specifically modify the ConfigMap's data.status entry in
admissibility-presync-check.yaml and ensure your pipeline task writes the actual
verdict into that key before the PreSync gate evaluates it.
- Around line 19-33: The Job currently runs kubectl without a specific
serviceAccountName so it may lack RBAC to read the admissibility-status
ConfigMap; add serviceAccountName: admissibility-checker to the Job spec and
create a ServiceAccount named admissibility-checker plus a Role named
admissibility-configmap-reader (namespace: argocd) that grants get on the
configmap resourceNames: ["admissibility-status"], then bind them with a
RoleBinding named admissibility-checker-binding that references the Role and the
admissibility-checker ServiceAccount in the argocd namespace so the kubectl
command in the container can read the ConfigMap.
In `@k8s/policies/kyverno-admissibility-enforcement.yaml`:
- Around line 40-42: The keyless subject pattern is too broad—replace the
subject value `https://github.com/*/*` under the `keyless` block with a more
restrictive pattern that limits signatures to your org or specific repo (for
example `https://github.com/your-org/*` or
`https://github.com/your-org/your-repo`) so only workflows from trusted GitHub
repositories are accepted; update the `subject` field next to `issuer:
https://token.actions.githubusercontent.com` accordingly.
- Around line 33-34: The policy currently uses imageReferences: "*" which will
reject all images without GitHub Actions signatures or SLSA attestations; update
the Kyverno rules (look for imageReferences and validationFailureAction in the
policy) to a scoped allowlist and/or exclusions: replace "*" with your registry
patterns (e.g., ghcr.io/brianlong/* or intelgraph/*) in imageReferences, add
exclude blocks listing required third‑party images (e.g., alpine,
bitnami/kubectl, neo4j:5) or add exclude/namespace rules for system namespaces,
and ensure validationFailureAction: Enforce remains only for workloads that must
be gated by attestations.
In `@k8s/server-deployment.yaml`:
- Line 13: Remove the hardcoded label key "security.summit.io/admissibility:
\"PASS\"" from the Deployment manifest and instead expose a templated label
placeholder (e.g., a Helm value like .Values.admissibility or a kustomize patch
variable) so the CI/CD pipeline can inject the actual evaluateAdmissibility
verdict; update the pipeline to set that label to "PASS" only when the CI check
succeeds (or omit/set "FAIL" otherwise) via a kustomize patch or Helm values
override applied at deploy time.
In `@scripts/ci/verify-admissibility.ts`:
- Around line 5-7: The code that computes inputPath using args, inputFlagIndex
and args[inputFlagIndex + 1] can produce undefined when "--input" is the last
CLI arg; update the logic around args/inputFlagIndex/inputPath to explicitly
check that inputFlagIndex >= 0 and inputFlagIndex + 1 < args.length and that
args[inputFlagIndex + 1] is a non-empty string before using it, otherwise fall
back to the default "evidence/report.json" (or explicitly throw a clear error
about the missing value); change the assignment that references
args[inputFlagIndex + 1] (the inputPath calculation) to perform this guard so
resolve(process.cwd(), ...) never receives undefined.
---
Nitpick comments:
In @.github/workflows/admissibility-gate.yml:
- Around line 85-96: The "Upload evidence artifacts" step currently lists
cosign.pub but leaves cosign.key in the workspace; add an explicit pre-upload
cleanup or exclusion to ensure cosign.key (the private key) is removed or never
present when artifacts are uploaded: update the job step named "Upload evidence
artifacts" to run a short cleanup step (or an exclude pattern) that securely
deletes or moves evidence/cosign.key before the actions/upload-artifact@v4
invocation, and ensure only the intended files (evidence/report.json,
evidence/metrics.json, evidence/stamp.json, evidence/sbom.cdx.json,
evidence/provenance.json, evidence/artifact.sig, cosign.pub) are retained for
upload.
- Around line 36-39: The current pipeline extracts only hashes then sorts them,
which can produce non-deterministic ordering; run sha256sum for the target files
(package.json and pnpm-lock.yaml), sort the filename-hash pairs by filename
(e.g. using sort keyed on the filename column) before extracting the hash with
awk, and write the resulting ordered hashes to dist/admissible-artifact.txt so
the artifact payload is deterministic; modify the invocation that currently uses
sha256sum, awk, and sort to sort by filename (column 2) prior to printing the
hash.
In `@lib/admissibility.ts`:
- Around line 76-80: The boolean variable prohibitedDependencies is inverted
(true means no prohibited deps) and should be renamed to reflect a passing
state; change prohibitedDependencies to a clearer name such as dependenciesClean
or noProhibitedDependencies in both the checks interface and its usage in the
function so that true means "passing" (update the declaration using
evidenceBundle.dependencies.prohibitedFound.length === 0 and the conditional
that pushes PROHIBITED_DEPENDENCIES to use the new name, and adjust any
references to prohibitedDependencies elsewhere in the checks interface and
code).
In `@scripts/ci/build_admissibility_evidence.mjs`:
- Around line 18-19: Wrap the JSON parsing of sbom and provenance in try/catch
blocks to handle file read or parse failures: when calling
JSON.parse(readFileSync(sbomPath, 'utf8')) and
JSON.parse(readFileSync(provenancePath, 'utf8')), catch errors from
readFileSync/JSON.parse and log/throw a clear CI-friendly message that includes
the path (sbomPath or provenancePath) and the original error message; ensure the
script exits non-zero after logging so failures are surfaced in CI.
- Around line 35-40: The current sbom object sets complete: components.length >
0 which only ensures one component exists; update the completeness logic in the
sbom construction to perform stricter validation (e.g., require
components.length >= expectedCount when available, or validate that every item
in components contains required fields like name, version, and purl, and that
transitive dependencies are present) by replacing the simple length check with a
predicate that uses components.every(...) for required fields and/or compares
against an expected components count; reference the sbom object, the complete
property, and the components variable to locate and update the check.
In `@scripts/ci/verify-admissibility.ts`:
- Around line 9-11: Wrap the file reading and JSON parsing around a try/catch
and add explicit checks so failures produce clear CI-friendly errors: verify the
file exists (use existsSync or fs.stat) for reportPath before calling
readFileSync, then catch JSON.parse errors when constructing parsed as
AdmissibilityEvidenceBundle and log a descriptive error with the file path and
error.message, call process.exit(1) on failure, and only call
evaluateAdmissibility(parsed) when parsing succeeds; reference symbols to
change: reportPath, inputPath, resolve/process.cwd, readFileSync, JSON.parse,
parsed, AdmissibilityEvidenceBundle, and evaluateAdmissibility.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b66e8a6c-7756-4dbf-8e78-6fb0f93b8d6b
📒 Files selected for processing (10)
.github/workflows/admissibility-gate.ymlk8s/argo/admissibility-presync-check.yamlk8s/base/kustomization.yamlk8s/policies/kyverno-admissibility-enforcement.yamlk8s/server-deployment.yamllib/admissibility.tspackage.jsonscripts/ci/build_admissibility_evidence.mjsscripts/ci/verify-admissibility.tstests/ci/admissibility.test.mts
| - name: Sign and verify artifact signature | ||
| run: | | ||
| cosign generate-key-pair | ||
| cosign sign-blob --yes --key cosign.key --output-signature evidence/artifact.sig dist/admissible-artifact.txt | ||
| cosign verify-blob --key cosign.pub --signature evidence/artifact.sig dist/admissible-artifact.txt |
There was a problem hiding this comment.
Ephemeral key pair compromises long-term signature verification.
Generating a new key pair on every workflow run means:
- Signatures cannot be verified against a stable, trusted key outside this workflow run.
- The private key (
cosign.key) is created in the workspace and could be accidentally exposed.
For a supply-chain security gate, consider using Sigstore's keyless signing (cosign sign-blob --yes without --key) which leverages OIDC identity, or store a persistent signing key in GitHub Secrets.
🔑 Suggested approach using keyless signing
- name: Sign and verify artifact signature
run: |
- cosign generate-key-pair
- cosign sign-blob --yes --key cosign.key --output-signature evidence/artifact.sig dist/admissible-artifact.txt
- cosign verify-blob --key cosign.pub --signature evidence/artifact.sig dist/admissible-artifact.txt
+ cosign sign-blob --yes --output-signature evidence/artifact.sig --output-certificate evidence/artifact.crt dist/admissible-artifact.txt
+ cosign verify-blob --signature evidence/artifact.sig --certificate evidence/artifact.crt \
+ --certificate-identity-regexp ".*" --certificate-oidc-issuer-regexp ".*" dist/admissible-artifact.txtIf keyless signing isn't suitable, ensure the private key is stored securely in secrets and never written to disk in the workflow directory.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Sign and verify artifact signature | |
| run: | | |
| cosign generate-key-pair | |
| cosign sign-blob --yes --key cosign.key --output-signature evidence/artifact.sig dist/admissible-artifact.txt | |
| cosign verify-blob --key cosign.pub --signature evidence/artifact.sig dist/admissible-artifact.txt | |
| - name: Sign and verify artifact signature | |
| run: | | |
| cosign sign-blob --yes --output-signature evidence/artifact.sig --output-certificate evidence/artifact.crt dist/admissible-artifact.txt | |
| cosign verify-blob --signature evidence/artifact.sig --certificate evidence/artifact.crt \ | |
| --certificate-identity-regexp ".*" --certificate-oidc-issuer-regexp ".*" dist/admissible-artifact.txt |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/admissibility-gate.yml around lines 68 - 72, The workflow
currently generates a transient keypair with cosign (cosign generate-key-pair)
and signs with cosign.key/cosign.pub, which prevents stable verification and
risks leaking the private key; replace this with keyless signing by removing the
generate-key-pair step and invoking cosign sign-blob without --key (using OIDC)
and verify with the corresponding cosign verify-blob call, or if keyless is not
acceptable, load a persistent private key from a GitHub Secret instead of
writing cosign.key to the workspace and use that secret-provided key for cosign
sign-blob and verify-blob (ensure the secret never gets persisted to disk).
| data: | ||
| status: "PASS" |
There was a problem hiding this comment.
Hardcoded "PASS" in ConfigMap defeats the PreSync gate purpose.
Similar to the deployment label issue, the ConfigMap's status: "PASS" is statically defined. The CI pipeline should dynamically create/update this ConfigMap with the actual admissibility verdict, otherwise this check will always pass.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@k8s/argo/admissibility-presync-check.yaml` around lines 6 - 7, The ConfigMap
currently hardcodes data.status: "PASS", which defeats the PreSync gate; change
the manifest so it does not contain a static "PASS" (remove the fixed value or
replace it with a placeholder like an unset/templated value) and update the
CI/CD pipeline to create/update the ConfigMap's data.status at runtime with the
real admissibility verdict; specifically modify the ConfigMap's data.status
entry in admissibility-presync-check.yaml and ensure your pipeline task writes
the actual verdict into that key before the PreSync gate evaluates it.
| spec: | ||
| restartPolicy: Never | ||
| containers: | ||
| - name: gate | ||
| image: bitnami/kubectl:1.30 | ||
| command: | ||
| - /bin/sh | ||
| - -ec | ||
| - | | ||
| STATUS="$(kubectl -n argocd get configmap admissibility-status -o jsonpath='{.data.status}')" | ||
| if [ "$STATUS" != "PASS" ]; then | ||
| echo "ArgoCD sync blocked: admissibility status is $STATUS" | ||
| exit 1 | ||
| fi | ||
| echo "ArgoCD sync permitted: admissibility status is PASS" |
There was a problem hiding this comment.
Missing ServiceAccount and RBAC for kubectl access.
The Job uses kubectl to read the ConfigMap but doesn't specify a serviceAccountName. The default service account in the argocd namespace may not have permission to read ConfigMaps, causing the Job to fail with a permissions error.
🛠️ Proposed fix
template:
spec:
restartPolicy: Never
+ serviceAccountName: argocd-application-controller # or create a dedicated SA with ConfigMap read permissions
containers:
- name: gateAlternatively, create a dedicated ServiceAccount and RoleBinding:
apiVersion: v1
kind: ServiceAccount
metadata:
name: admissibility-checker
namespace: argocd
---
apiVersion: rbac.authorization.k8s.io/v1
kind: Role
metadata:
name: admissibility-configmap-reader
namespace: argocd
rules:
- apiGroups: [""]
resources: ["configmaps"]
resourceNames: ["admissibility-status"]
verbs: ["get"]
---
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
name: admissibility-checker-binding
namespace: argocd
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: admissibility-configmap-reader
subjects:
- kind: ServiceAccount
name: admissibility-checker
namespace: argocd📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| spec: | |
| restartPolicy: Never | |
| containers: | |
| - name: gate | |
| image: bitnami/kubectl:1.30 | |
| command: | |
| - /bin/sh | |
| - -ec | |
| - | | |
| STATUS="$(kubectl -n argocd get configmap admissibility-status -o jsonpath='{.data.status}')" | |
| if [ "$STATUS" != "PASS" ]; then | |
| echo "ArgoCD sync blocked: admissibility status is $STATUS" | |
| exit 1 | |
| fi | |
| echo "ArgoCD sync permitted: admissibility status is PASS" | |
| spec: | |
| restartPolicy: Never | |
| serviceAccountName: argocd-application-controller # or create a dedicated SA with ConfigMap read permissions | |
| containers: | |
| - name: gate | |
| image: bitnami/kubectl:1.30 | |
| command: | |
| - /bin/sh | |
| - -ec | |
| - | | |
| STATUS="$(kubectl -n argocd get configmap admissibility-status -o jsonpath='{.data.status}')" | |
| if [ "$STATUS" != "PASS" ]; then | |
| echo "ArgoCD sync blocked: admissibility status is $STATUS" | |
| exit 1 | |
| fi | |
| echo "ArgoCD sync permitted: admissibility status is PASS" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@k8s/argo/admissibility-presync-check.yaml` around lines 19 - 33, The Job
currently runs kubectl without a specific serviceAccountName so it may lack RBAC
to read the admissibility-status ConfigMap; add serviceAccountName:
admissibility-checker to the Job spec and create a ServiceAccount named
admissibility-checker plus a Role named admissibility-configmap-reader
(namespace: argocd) that grants get on the configmap resourceNames:
["admissibility-status"], then bind them with a RoleBinding named
admissibility-checker-binding that references the Role and the
admissibility-checker ServiceAccount in the argocd namespace so the kubectl
command in the container can read the ConfigMap.
| - imageReferences: | ||
| - "*" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there are other third-party images in k8s manifests that would be affected
rg -n 'image:' k8s/ --type yaml | grep -v 'summit/' | head -20Repository: BrianCLong/summit
Length of output: 1551
🏁 Script executed:
cat -n k8s/policies/kyverno-admissibility-enforcement.yaml | head -60Repository: BrianCLong/summit
Length of output: 2071
imageReferences: "*" will reject all third-party images lacking GitHub Actions signatures and SLSA attestations.
The wildcard in both rules (lines 33-34 and 51-52) applies to every container image in the cluster. With validationFailureAction: Enforce, the policy will block any Pod using images without:
- GitHub Actions OIDC signatures (from
https://token.actions.githubusercontent.comwith subjecthttps://github.com/*/*), OR - SLSA provenance attestations
Third-party images in your manifests—alpine, node:18-alpine, bitnami/kubectl, neo4j:5, amazon/aws-cli:2, trufflesecurity/trufflehog, aquasec/kube-bench, and others—do not carry these attestations and will be rejected.
To fix:
- Restrict
imageReferencesto your own registry patterns (e.g.,ghcr.io/brianlong/*,intelgraph/*) - Add
excluderules for third-party images your workloads require (e.g.,alpine,bitnami/kubectl,neo4j:5) - Or add namespace exclusions for system workloads that use third-party images
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@k8s/policies/kyverno-admissibility-enforcement.yaml` around lines 33 - 34,
The policy currently uses imageReferences: "*" which will reject all images
without GitHub Actions signatures or SLSA attestations; update the Kyverno rules
(look for imageReferences and validationFailureAction in the policy) to a scoped
allowlist and/or exclusions: replace "*" with your registry patterns (e.g.,
ghcr.io/brianlong/* or intelgraph/*) in imageReferences, add exclude blocks
listing required third‑party images (e.g., alpine, bitnami/kubectl, neo4j:5) or
add exclude/namespace rules for system namespaces, and ensure
validationFailureAction: Enforce remains only for workloads that must be gated
by attestations.
| - keyless: | ||
| issuer: https://token.actions.githubusercontent.com | ||
| subject: https://github.com/*/* |
There was a problem hiding this comment.
Overly permissive keyless subject pattern.
The subject https://github.com/*/* accepts signatures from any GitHub repository's Actions workflow. Consider restricting to your organization or specific repository to prevent accepting signatures from untrusted sources:
- keyless:
issuer: https://token.actions.githubusercontent.com
- subject: https://github.com/*/*
+ subject: https://github.com/BrianCLong/summit/*📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - keyless: | |
| issuer: https://token.actions.githubusercontent.com | |
| subject: https://github.com/*/* | |
| - keyless: | |
| issuer: https://token.actions.githubusercontent.com | |
| subject: https://github.com/BrianCLong/summit/* |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@k8s/policies/kyverno-admissibility-enforcement.yaml` around lines 40 - 42,
The keyless subject pattern is too broad—replace the subject value
`https://github.com/*/*` under the `keyless` block with a more restrictive
pattern that limits signatures to your org or specific repo (for example
`https://github.com/your-org/*` or `https://github.com/your-org/your-repo`) so
only workflows from trusted GitHub repositories are accepted; update the
`subject` field next to `issuer: https://token.actions.githubusercontent.com`
accordingly.
| metadata: | ||
| labels: | ||
| app: summit-server | ||
| security.summit.io/admissibility: "PASS" |
There was a problem hiding this comment.
Hardcoded "PASS" label undermines the admissibility enforcement model.
The security.summit.io/admissibility label is statically set to "PASS" in the manifest. This means the Kyverno policy's label check will always pass regardless of whether the CI admissibility verification actually succeeded.
For the enforcement to be meaningful, this label should be injected dynamically by CI/CD based on the actual evaluateAdmissibility verdict—not baked into the source manifest. Otherwise, an attacker or misconfiguration can deploy without passing the admissibility checks.
Consider using a Kustomize patch or Helm values injection in the deployment pipeline that sets this label only when CI verification passes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@k8s/server-deployment.yaml` at line 13, Remove the hardcoded label key
"security.summit.io/admissibility: \"PASS\"" from the Deployment manifest and
instead expose a templated label placeholder (e.g., a Helm value like
.Values.admissibility or a kustomize patch variable) so the CI/CD pipeline can
inject the actual evaluateAdmissibility verdict; update the pipeline to set that
label to "PASS" only when the CI check succeeds (or omit/set "FAIL" otherwise)
via a kustomize patch or Helm values override applied at deploy time.
| const args = process.argv.slice(2); | ||
| const inputFlagIndex = args.indexOf("--input"); | ||
| const inputPath = inputFlagIndex >= 0 ? args[inputFlagIndex + 1] : "evidence/report.json"; |
There was a problem hiding this comment.
Handle missing --input argument value.
If --input is provided as the last argument without a following value, args[inputFlagIndex + 1] will be undefined, causing resolve(process.cwd(), undefined) to throw or produce unexpected behavior.
🛠️ Proposed fix
const args = process.argv.slice(2);
const inputFlagIndex = args.indexOf("--input");
-const inputPath = inputFlagIndex >= 0 ? args[inputFlagIndex + 1] : "evidence/report.json";
+const inputPath = inputFlagIndex >= 0 && args[inputFlagIndex + 1]
+ ? args[inputFlagIndex + 1]
+ : "evidence/report.json";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/ci/verify-admissibility.ts` around lines 5 - 7, The code that
computes inputPath using args, inputFlagIndex and args[inputFlagIndex + 1] can
produce undefined when "--input" is the last CLI arg; update the logic around
args/inputFlagIndex/inputPath to explicitly check that inputFlagIndex >= 0 and
inputFlagIndex + 1 < args.length and that args[inputFlagIndex + 1] is a
non-empty string before using it, otherwise fall back to the default
"evidence/report.json" (or explicitly throw a clear error about the missing
value); change the assignment that references args[inputFlagIndex + 1] (the
inputPath calculation) to perform this guard so resolve(process.cwd(), ...)
never receives undefined.
|
Superseded by #22309. The evidence-to-admissibility enforcement core now rides the golden-path branch. |
1 participant
Motivation
Description
lib/admissibility.tsexposingevaluateAdmissibility(evidenceBundle) → PASS|FAILthat checks signature validity, SBOM completeness, provenance integrity, and prohibited dependencies and returns a structured JSON verdict.scripts/ci/build_admissibility_evidence.mjsto assemble deterministicevidence/report.jsonandevidence/metrics.json(andevidence/stamp.jsonfor non-deterministic metadata), plusscripts/ci/verify-admissibility.tsandnpm run verify:admissibilityto evaluate and fail onFAILverdicts..github/workflows/admissibility-gate.ymlto generate SBOM (Syft), create SLSA provenance attestations, sign with Cosign, materialize deterministic evidence, and run the admissibility verifier as a blocking job.k8s/policies/kyverno-admissibility-enforcement.yamlto require signed images, SLSA attestations, andsecurity.summit.io/admissibility: "PASS"label, plus an ArgoCD PreSync hookk8s/argo/admissibility-presync-check.yamland wire both intok8s/base/kustomization.yaml; updatek8s/server-deployment.yamlto include the admissibility label.tests/ci/admissibility.test.mtscovering PASS and FAIL scenarios and updatepackage.jsonwithverify:admissibilityandtest:admissibilityscripts.Testing
node --test --import tsx tests/ci/admissibility.test.mts, which passed.pnpm verify:admissibility --input evidence/report.json, which returnedPASSand exited 0.node scripts/check-boundaries.cjsto ensure changes respect zone boundaries, which passed.Codex Task
Summary by CodeRabbit
Release Notes
New Features
Chores