From b329c1ee599dedd3436e8dc323bda29d0277efb4 Mon Sep 17 00:00:00 2001 From: asivanadi0 Date: Fri, 4 Sep 2026 16:15:12 +0000 Subject: [PATCH 1/9] docs(adr-074): correct D2 Build Level reasoning as per-artifact L2 Keep SLSA Build L2 for binaries and images. Reframe D2 around provenance unforgeability (attest.yml reusable workflow, GITHUB_* predicate, builder_id guard) rather than requiring the build itself to run in the attestor. State the same-repo reusable-workflow caveat and defer per-artifact L3. Signed-off-by: asivanadi0 --- docs/designs/074-supply-chain-attestation.md | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/docs/designs/074-supply-chain-attestation.md b/docs/designs/074-supply-chain-attestation.md index d4d61e9e..1e13c188 100644 --- a/docs/designs/074-supply-chain-attestation.md +++ b/docs/designs/074-supply-chain-attestation.md @@ -66,11 +66,19 @@ This contract is published to users and enforced by us. Both the release-time ga The reason is decision 3. cosign uses the OIDC `job_workflow_ref` as the certificate SAN, so signing from one reusable workflow collapses every artifact onto one identity path where only the ref varies. Signing inline in each caller would give the chart, the image, and the binaries three different identities, and a `main` build a fourth that looks just as legitimate. It also isolates the signing step: caller-defined build steps run in a different job from the one holding the signing token. -**This design targets SLSA Build L2, not L3.** The distinction matters and is easy to overclaim. L3 requires the *build* to be isolated from user-defined steps, and GitHub's mechanism for that is moving the build itself into the reusable workflow. Here the builds stay in the callers — `docker buildx` in `publish.yml`, the Go cross-compile and `helm package` in `release.yml` — and `attest.yml` receives a digest and signs it. A caller that produced the wrong artifact would get a faithful signature over the wrong digest. Isolating the signer is worth having, but it is not the isolation L3 asks for. +**This design targets SLSA Build L2 for every artifact we publish today — binaries and images alike.** Claims are per-artifact, not project-wide: nothing here may state a single Build Level for "the release" as a whole. -Two consequences follow. First, no artifact or document may claim L3 — not the ADR, not the release notes, not `SECURITY.md`. Second, the provenance predicate's `runDetails.builder.id` must name the workflow that actually performed the build, not `attest.yml`. Naming the attestor as the builder would make the predicate false on its face, which is worse than claiming the wrong level. +The original L2 framing was imprecise in a way that made L3 look further away than it is. SLSA v1.0 Build L3 requires that **provenance is unforgeable by the build process**. That is not the same thing as builder isolation. On the unforgeability requirement, `attest.yml` already does most of the work: -Reaching L3 later means moving image, chart, and binary generation into the protected reusable workflow. That is a larger restructure than this record covers — it rewrites the build path rather than adding to it — and it should be its own decision once this contract is in place and stable. Recorded as deferred, not rejected. +- It is a genuine reusable workflow (`on: workflow_call`), invoked as `uses: ./.github/workflows/attest.yml` from the callers. It is not an inlined job, so the Fulcio certificate names **`attest.yml`**, not the caller. +- Every value that shapes the provenance predicate comes from trusted context **inside** that workflow — `GITHUB_REPOSITORY`, `GITHUB_REF`, `GITHUB_SHA`, `GITHUB_SERVER_URL`, `GITHUB_RUN_ID`, and the caller workflow ref GitHub itself sets — and not from a caller-supplied `inputs.*` field. A caller cannot dictate what the predicate says about origin. +- A guard refuses to proceed if `builder_id` resolves to `attest.yml`, on the grounds that the attestor cannot be the builder. Naming the attestor as the builder would make the predicate false on its face, which is worse than claiming the wrong level. + +So the build steps in `release.yml` / `publish.yml` cannot forge or tamper with the provenance today. What is genuinely absent is **builder isolation**: the builds stay in the callers — `docker buildx` in `publish.yml` / `build-image.yml`, the Go cross-compile and `helm package` in `release.yml` — and `attest.yml` receives a digest and signs it. `runDetails.builder.id` truthfully names the caller. A compromised caller would produce a bad artifact that `attest.yml` would then honestly attest. That honesty is deliberate, and any change here must not trade a true predicate for a higher number. + +Two consequences follow. First, no artifact or document may claim L3 — not the ADR, not the release notes, not `SECURITY.md` — until a recorded decision moves a specific artifact across that line. Second, the same-repo reusable-workflow caveat must be stated whenever this boundary is discussed: `uses: ./…` isolates attestation from the caller's *build steps*, not from *write access*. Caller and attestor live in one repository, so the practical strength of the boundary rests on branch protection over `.github/workflows/attest.yml`. Claiming L3 without saying so would imply stronger isolation than exists. + +A concrete path to per-artifact L3 for images exists and is recorded as deferred, not rejected: `build-image.yml` is already a reusable workflow that builds the image; if provenance were minted there, build and attestation would share one protected boundary and `builder.id` would be both honest and L3-qualifying. That would be an image-only change — `nvcrectl` binaries are still built and attested from the release job and would stay at L2 — which is why every published claim must stay per-artifact. Until that decision is taken, both binaries and images remain Build L2 under the corrected reasoning above. `attest.yml` validates every input before use: digests must match `^sha256:[0-9a-f]{64}$`, no input may contain a newline or carriage return, and the caller's authoritative `expected_digest` is compared against an independently resolved digest with a mismatch failing the job. It refuses to run on a non-tag ref unless an explicit `allow_untagged` input is set, so a test run cannot quietly produce something that looks like a release attestation. @@ -147,7 +155,7 @@ The contract lives in YAML, and the failure mode is silent: a signing step delet ## Rationale - **Exact identity over regexp** is the single highest-value decision here. Every other gap is a missing artifact, which is visibly missing. A too-permissive verification command is an artifact that appears present and correct while asserting less than the reader believes. -- **Reusable workflow** improves the security property (signing isolated from caller-defined build steps) and simplifies the consumer contract at the same time. Those usually trade against each other. It does not by itself reach Build L3 — see decision 4. +- **Reusable workflow** makes provenance unforgeable by the build process (Fulcio names `attest.yml`; the predicate is minted from trusted context) and collapses every artifact onto one pinnable identity. That is the corrected L2 claim in decision 4. It does **not** by itself provide builder isolation, and a same-repo `uses: ./…` boundary additionally depends on branch protection over `attest.yml` — see decision 4. - **Per-platform SBOM subjects** follow from what an SBOM is. Getting this wrong is not a policy choice, it is a category error, and it is already shipping. - **Verify what we produce** costs one job and converts a class of silent failure into a red release. Attestations nobody checks are decoration. - **Signing the SBOMs** closes the gap that remains after everything else is signed, at the cost of a few more bundles. @@ -158,7 +166,7 @@ The contract lives in YAML, and the failure mode is silent: a signing step delet - Every released artifact answers "who built this, from what source, containing what," with one command and one pinned identity. - The multi-platform SBOM defect is fixed, and the fail-closed digest checks prevent it from recurring silently. -- Provenance is SLSA Build L2 with a single pinnable builder identity, which is a real improvement over no provenance at all. L3 remains available as a follow-on and is not foreclosed by anything here. +- Provenance is SLSA Build L2 **per artifact** (binaries and images today), with a single pinnable attestor identity (`attest.yml`). The level is only checkable when verification pins that workflow. L3 remains available as a follow-on — per artifact, with the same-repo caveat stated — and is not foreclosed by anything here. - Admission controllers can enforce the same contract the documentation publishes, so install-time and runtime checks cannot drift. - Post-publication tampering has a bounded detection window instead of depending on a user noticing. From 04df5908f4e0311cfe8a853b9b7ef49f1e08bc7f Mon Sep 17 00:00:00 2001 From: asivanadi0 Date: Fri, 4 Sep 2026 16:15:12 +0000 Subject: [PATCH 2/9] test(releasepolicy): gate attest.yml reusable-workflow boundary Assert attest.yml is workflow_call-only and invoked via uses, that cosign sign/attest/attest-blob stay sole to attest.yml (workflows + composites), that provenance origin fields come from GITHUB_* / github.workflow_ref, and that the builder_id != attest.yml guard remains. Mutation-verified. Signed-off-by: asivanadi0 --- test/releasepolicy/attest_boundary_test.go | 370 +++++++++++++++++++++ 1 file changed, 370 insertions(+) create mode 100644 test/releasepolicy/attest_boundary_test.go diff --git a/test/releasepolicy/attest_boundary_test.go b/test/releasepolicy/attest_boundary_test.go new file mode 100644 index 00000000..db2a4b4c --- /dev/null +++ b/test/releasepolicy/attest_boundary_test.go @@ -0,0 +1,370 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package releasepolicy + +import ( + "fmt" + "os" + "path/filepath" + "regexp" + "strings" + "testing" + + "sigs.k8s.io/yaml" +) + +// cosignSignCmds are the signing invocations that may exist only inside +// attest.yml. A second home for any of them widens the published identity +// contract: consumers pin one workflow path, so a signer elsewhere is accepted +// under a different SAN with no change to the pin. +// +// Also covers the "not inlined as a job" half of ADR-074 D2: moving these +// invocations into release.yml / publish.yml as ordinary job steps would still +// compile and still green every other test, but Fulcio would stop naming +// attest.yml and identity-pinned verification would quietly fail for consumers. +var cosignSignCmds = regexp.MustCompile(`(?m)(?:^|[\s;|&])(?:retry\s+)?cosign\s+(sign|attest|attest-blob)\b`) + +var attestProvenanceAction = regexp.MustCompile(`(^|/)actions/attest-build-provenance(@|$)`) + +// Local attest.yml call shape used by publish.yml / release.yml / selftest. +var localAttestUses = regexp.MustCompile(`^\./\.github/workflows/attest\.yml(@.+)?$`) + +const attestWorkflowName = "attest.yml" + +// TestAttestIsSoleSigner keeps the published certificate identity contract true. +// +// Consumers pin +// +// .../attest.yml@refs/tags/ +// +// so any second workflow that invokes cosign sign/attest/attest-blob (or +// actions/attest-build-provenance) silently widens what that pin accepts. +// attest.yml itself must stay workflow_call-only: any other trigger makes the +// signing identity reachable from a branch push or a dispatch, and inlining +// its steps into a caller would demote the reusable-workflow boundary while +// every other test stayed green. +func TestAttestIsSoleSigner(t *testing.T) { + assertAttestIsWorkflowCallOnly(t) + assertAttestIsInvokedAsReusableWorkflow(t) + + paths := append([]string{}, workflowFiles(t)...) + paths = append(paths, compositeActionFiles(t)...) + + for _, path := range paths { + base := filepath.Base(path) + raw, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + + if isCompositeActionPath(path) { + var doc struct { + Runs struct { + Using string `json:"using"` + Steps []policyStep `json:"steps"` + } `json:"runs"` + } + if err := yaml.Unmarshal(raw, &doc); err != nil { + t.Fatalf("parse %s: %v", path, err) + } + if doc.Runs.Using != "" && doc.Runs.Using != "composite" { + continue + } + assertStepsForbidForeignSigners(t, relGithub(path), false /* allowCosign */, doc.Runs.Steps) + continue + } + + var doc struct { + Jobs map[string]struct { + Steps []policyStep `json:"steps"` + } `json:"jobs"` + } + if err := yaml.Unmarshal(raw, &doc); err != nil { + t.Fatalf("parse %s: %v", path, err) + } + + for jobName, job := range doc.Jobs { + where := fmt.Sprintf("%s: job %q", base, jobName) + assertStepsForbidForeignSigners(t, where, base == attestWorkflowName, job.Steps) + } + } +} + +// policyStep is the subset of a workflow/composite step the signer checks +// reason about. +type policyStep struct { + Name string `json:"name"` + Run string `json:"run"` + Uses string `json:"uses"` +} + +func assertStepsForbidForeignSigners(t *testing.T, where string, allowCosign bool, steps []policyStep) { + t.Helper() + + for _, step := range steps { + if attestProvenanceAction.MatchString(step.Uses) { + t.Errorf("%s step %q uses %s; provenance must be emitted by attest.yml "+ + "via cosign, not actions/attest-build-provenance", + where, step.Name, step.Uses) + } + if allowCosign { + continue + } + if m := cosignSignCmds.FindStringSubmatch(step.Run); m != nil { + t.Errorf("%s step %q invokes `cosign %s`; attest.yml must be the sole signer", + where, step.Name, m[1]) + } + } +} + +// compositeActionFiles returns every local composite action.yml. +func compositeActionFiles(t *testing.T) []string { + t.Helper() + + paths, err := filepath.Glob("../../.github/actions/*/action.yml") + if err != nil { + t.Fatalf("glob composite actions: %v", err) + } + return paths +} + +func isCompositeActionPath(path string) bool { + return strings.Contains(filepath.ToSlash(path), "/.github/actions/") +} + +func relGithub(path string) string { + slash := filepath.ToSlash(path) + if i := strings.Index(slash, ".github/"); i >= 0 { + return slash[i:] + } + return filepath.Base(path) +} + +func assertAttestIsWorkflowCallOnly(t *testing.T) { + t.Helper() + + raw, err := os.ReadFile(filepath.Join(workflowDir, attestWorkflowName)) + if err != nil { + t.Fatalf("read %s: %v", attestWorkflowName, err) + } + + triggers := workflowTriggers(raw, t) + if len(triggers) == 0 { + t.Fatalf("%s declares no triggers; it must be workflow_call-only", attestWorkflowName) + } + for name := range triggers { + if name != "workflow_call" { + t.Errorf("%s is triggered by %q; only workflow_call is allowed so the signing "+ + "identity cannot be reached from a branch or dispatch", + attestWorkflowName, name) + } + } + if _, ok := triggers["workflow_call"]; !ok { + t.Errorf("%s is missing workflow_call", attestWorkflowName) + } +} + +// assertAttestIsInvokedAsReusableWorkflow checks that every release-path caller +// reaches attest.yml through `uses: ./…`, not by inlining its jobs. Without +// this, a refactor could copy the attest steps into release.yml, keep +// workflow_call on an unused attest.yml, and demote the Fulcio identity while +// TestAttestIsSoleSigner still saw cosign only inside attest.yml — until the +// copy started signing too. +func assertAttestIsInvokedAsReusableWorkflow(t *testing.T) { + t.Helper() + + callers := []string{wfRelease, wfPublish, wfAttestSmoke} + found := 0 + for _, base := range callers { + raw, err := os.ReadFile(filepath.Join(workflowDir, base)) + if err != nil { + t.Fatalf("read %s: %v", base, err) + } + var doc struct { + Jobs map[string]struct { + Uses string `json:"uses"` + } `json:"jobs"` + } + if err := yaml.Unmarshal(raw, &doc); err != nil { + t.Fatalf("parse %s: %v", base, err) + } + for jobName, job := range doc.Jobs { + if !localAttestUses.MatchString(strings.TrimSpace(job.Uses)) { + continue + } + found++ + if !strings.HasPrefix(job.Uses, "./") { + t.Errorf("%s: job %q calls attest.yml as %q; same-repo reusable "+ + "calls must use the ./ form so the call is a workflow_call boundary", + base, jobName, job.Uses) + } + } + } + if found == 0 { + t.Fatalf("no release-path workflow calls ./.github/workflows/attest.yml; " + + "attestation must stay behind a reusable-workflow boundary") + } +} + +// workflowTriggers returns the `on:` block, tolerating YAML 1.1 turning a bare +// `on:` key into the boolean true (the same hazard workflowCallOutputs faces). +func workflowTriggers(raw []byte, t *testing.T) map[string]any { + t.Helper() + + var doc map[string]any + if err := yaml.Unmarshal(raw, &doc); err != nil { + t.Fatalf("parse workflow triggers: %v", err) + } + for _, key := range []string{"on", boolTrue} { + switch v := doc[key].(type) { + case map[string]any: + return v + case string: + return map[string]any{v: nil} + case []any: + out := map[string]any{} + for _, item := range v { + if s, ok := item.(string); ok { + out[s] = nil + } + } + return out + } + } + return nil +} + +// TestAttestPredicateUsesOnlyTrustedContext pins the half of ADR-074 D2 that +// makes provenance unforgeable by the build process: the predicate's origin +// fields are minted from GITHUB_* / github.workflow_ref inside attest.yml, never +// from a caller-supplied inputs.* value. +// +// A refactor that wired --arg repo ${{ inputs.repository }} (or passed the same +// through env) would still produce a green release whose predicate said whatever +// the caller asked. This test fails that change before it ships. +func TestAttestPredicateUsesOnlyTrustedContext(t *testing.T) { + step := provenanceStep(t) + + // The step must not pull workflow_call inputs into the provenance surface. + for envName, envVal := range step.Env { + if strings.Contains(envVal, "inputs.") { + t.Errorf("provenance step env %q expands %q; origin fields must not "+ + "be sourced from workflow_call inputs", envName, envVal) + } + } + if strings.Contains(step.Run, "inputs.") { + t.Errorf("provenance step run block references inputs.*; the predicate " + + "must be built from trusted context only") + } + + // CALLER_WORKFLOW_REF (builder.id) must come from github.workflow_ref, which + // GitHub sets to the workflow that started the run — the caller — not from + // an input a caller could forge. + callerRef, ok := step.Env["CALLER_WORKFLOW_REF"] + if !ok { + t.Fatal("provenance step is missing CALLER_WORKFLOW_REF; builder.id must " + + "be derived from github.workflow_ref") + } + if !strings.Contains(callerRef, "github.workflow_ref") { + t.Errorf("CALLER_WORKFLOW_REF is %q; it must expand github.workflow_ref "+ + "so a caller cannot name an arbitrary builder", callerRef) + } + + // arg -> required substring in the --arg value. builder is special: it is + // derived from CALLER_WORKFLOW_REF / GITHUB_SERVER_URL via builder_id. + wantByArg := map[string]string{ + "repo": "GITHUB_REPOSITORY", + "ref": "GITHUB_REF", + "sha": "GITHUB_SHA", + "server": "GITHUB_SERVER_URL", + "run_id": "GITHUB_RUN_ID", + "builder": "builder_id", + } + for arg, want := range wantByArg { + flag := "--arg " + arg + " " + idx := strings.Index(step.Run, flag) + if idx < 0 { + t.Errorf("provenance jq is missing --arg %s; the predicate must still "+ + "emit that origin field", arg) + continue + } + rest := step.Run[idx+len(flag):] + if nl := strings.IndexByte(rest, '\n'); nl >= 0 { + rest = rest[:nl] + } + rest = strings.TrimSpace(rest) + if !strings.Contains(rest, want) { + t.Errorf("--arg %s value %q must expand %s (trusted context), "+ + "not a caller-supplied input", arg, rest, want) + } + } +} + +// TestAttestBuilderIdGuardRejectsAttestorAsBuilder pins the guard that keeps +// runDetails.builder.id honest. Naming attest.yml as the builder would make the +// predicate false on its face and is exactly the trade ADR-074 D2 refuses when +// it keeps L2 rather than inflating to L3. +func TestAttestBuilderIdGuardRejectsAttestorAsBuilder(t *testing.T) { + step := provenanceStep(t) + + const needle = `"/.github/workflows/attest.yml"` + if !strings.Contains(step.Run, needle) && !strings.Contains(step.Run, "'/.github/workflows/attest.yml'") { + // Accept either quoting style used by the shell guard. + if !strings.Contains(step.Run, "/.github/workflows/attest.yml") { + t.Fatal("provenance step is missing the builder_id == attest.yml guard") + } + } + if !strings.Contains(step.Run, "builder_id") { + t.Fatal("provenance step does not compute builder_id") + } + if !strings.Contains(step.Run, "attestor cannot be the builder") && + !strings.Contains(step.Run, "builder resolved to attest.yml") { + t.Error("provenance step must refuse when builder_id resolves to attest.yml " + + "(message should name the attestor-as-builder failure)") + } +} + +type namedEnvStep struct { + Name string + Run string + Env map[string]string +} + +func provenanceStep(t *testing.T) namedEnvStep { + t.Helper() + + raw, err := os.ReadFile(filepath.Join(workflowDir, attestWorkflowName)) + if err != nil { + t.Fatalf("read %s: %v", attestWorkflowName, err) + } + + var doc struct { + Jobs map[string]struct { + Steps []struct { + Name string `json:"name"` + Run string `json:"run"` + Env map[string]string `json:"env"` + } `json:"steps"` + } `json:"jobs"` + } + if err := yaml.Unmarshal(raw, &doc); err != nil { + t.Fatalf("parse %s: %v", attestWorkflowName, err) + } + + const wantName = "Generate SLSA provenance predicate" + for jobName, job := range doc.Jobs { + for _, step := range job.Steps { + if step.Name == wantName { + if strings.TrimSpace(step.Run) == "" { + t.Fatalf("%s job %q step %q has an empty run block", + attestWorkflowName, jobName, wantName) + } + return namedEnvStep{Name: step.Name, Run: step.Run, Env: step.Env} + } + } + } + t.Fatalf("%s has no step named %q; the provenance predicate must be minted "+ + "inside the reusable workflow", attestWorkflowName, wantName) + return namedEnvStep{} +} From bc54119bec219047a26d7fd701f8158b7d69115d Mon Sep 17 00:00:00 2001 From: asivanadi0 Date: Fri, 4 Sep 2026 16:15:12 +0000 Subject: [PATCH 3/9] docs(security): state per-artifact Build L2 and identity pin Record which artifacts are Build L2 today, that pinning attest.yml is what makes the level checkable, and point at the releasepolicy tests that back those claims. No level claim without a test. Signed-off-by: asivanadi0 --- SECURITY.md | 4 ++++ docs/operations/verifying-artifacts.md | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/SECURITY.md b/SECURITY.md index 19c8079f..a877699b 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -81,6 +81,10 @@ We credit reporters of confirmed vulnerabilities in the release notes of the fix Use `--certificate-identity`, not `--certificate-identity-regexp`. An identity that names no workflow and no ref also accepts images built from branches, which are not releases and are labelled non-production when they are signed. + **Build Level (per artifact).** Every artifact this repository publishes today — the `manager` image (index provenance), the Helm chart, the `nvcrectl` binaries, and `installer` — carries SLSA Build Provenance at **Build L2**. The claim is per-artifact, not project-wide: nothing here states a single level for "the release". Provenance is unforgeable by the build process because it is minted inside the reusable `attest.yml` workflow (Fulcio names that workflow; predicate origin fields come from `GITHUB_*` context; a guard refuses to name `attest.yml` as the builder). What is absent is builder isolation — the build still runs in the caller — so we do not claim L3. A same-repo `uses: ./…` boundary additionally rests on branch protection over `.github/workflows/attest.yml`. + + Pinning the workflow identity above is what makes that level checkable. Verification that omits `--certificate-identity` (or loosens it to a regexp) can still succeed while proving a weaker claim: it no longer distinguishes an attestation minted inside `attest.yml` from one produced elsewhere. The exact pin is enforced by `TestVerificationUsesExactIdentity` and `TestPublishedVerifyCommandsAreExact` in `test/releasepolicy`; the reusable-workflow boundary itself is gated by `TestAttestIsSoleSigner`, `TestAttestPredicateUsesOnlyTrustedContext`, and `TestAttestBuilderIdGuardRejectsAttestorAsBuilder`. No level is claimed here without a test behind it. + Retrieve the provenance with `cosign verify-attestation --type slsaprovenance1` against the index digest, and a platform's SBOM with `--type cyclonedx` against that platform's manifest digest (`crane digest --platform linux/amd64 "${IMAGE}:${TAG}"`). - CLI binaries, the installer and the SBOMs are each signed, and every release asset ships with a detached Sigstore bundle (`.sigstore.json`) verified under the same identity as the image: diff --git a/docs/operations/verifying-artifacts.md b/docs/operations/verifying-artifacts.md index 7c8eeb60..d0dc7513 100644 --- a/docs/operations/verifying-artifacts.md +++ b/docs/operations/verifying-artifacts.md @@ -89,6 +89,30 @@ built on it reports success for an artifact that was never released. The exact f the workflow **and** the tag, so a signature from `v0.1.0` cannot pass as `v0.2.0`, and a branch build cannot pass as either. +## Build Level, and why the pin matters + +Provenance is **SLSA Build L2 per artifact** today — the `manager` image index, the Helm +chart, each `nvcrectl` binary, and `installer` alike. The project does not publish a +single project-wide level, and it does not claim L3: builder isolation is still missing +(the build runs in the caller; `attest.yml` attests a digest it is handed). What L2 here +does claim is that provenance is unforgeable by that build process — Fulcio names +`attest.yml`, the predicate's origin fields come from trusted `GITHUB_*` context inside +that workflow, and a guard refuses to list `attest.yml` as the builder. + +The same-repo reusable-workflow form (`uses: ./.github/workflows/attest.yml`) isolates +attestation from the caller's build steps, not from write access to the repository. The +practical strength of that boundary rests on branch protection over `attest.yml`. + +**Pinning the identity above is the check that makes the level observable.** Drop the +`--certificate-identity` flag (or replace it with a loose regexp) and verification can +still go green while proving less than Build L2: you no longer know the attestation was +minted inside the reusable workflow. The published commands keep the exact pin, and +`TestVerificationUsesExactIdentity` / `TestPublishedVerifyCommandsAreExact` fail the +build if a release-path workflow or a fenced doc command drifts to the regexp form. The +boundary itself is gated by `TestAttestIsSoleSigner`, +`TestAttestPredicateUsesOnlyTrustedContext`, and +`TestAttestBuilderIdGuardRejectsAttestorAsBuilder`. + ## Verifying the container image The image is a multi-platform index. Two different things are attested, to two different From be4209a67950f5cf7c2348aca89b8a50ca6024f7 Mon Sep 17 00:00:00 2001 From: asivanadi0 Date: Tue, 8 Sep 2026 19:07:40 +0000 Subject: [PATCH 4/9] test(releasepolicy): drop helpers overlapped by #295 Reuse sole-signer coverage from workflow_policy_test.go on main; keep only the #301-specific gates (reusable uses: ./ form, GITHUB_* predicate, builder_id != attest.yml) and name them in SECURITY.md / verifying-artifacts. Signed-off-by: asivanadi0 --- SECURITY.md | 2 +- docs/operations/verifying-artifacts.md | 1 + test/releasepolicy/attest_boundary_test.go | 191 ++------------------- 3 files changed, 13 insertions(+), 181 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index a877699b..df15e777 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -83,7 +83,7 @@ We credit reporters of confirmed vulnerabilities in the release notes of the fix **Build Level (per artifact).** Every artifact this repository publishes today — the `manager` image (index provenance), the Helm chart, the `nvcrectl` binaries, and `installer` — carries SLSA Build Provenance at **Build L2**. The claim is per-artifact, not project-wide: nothing here states a single level for "the release". Provenance is unforgeable by the build process because it is minted inside the reusable `attest.yml` workflow (Fulcio names that workflow; predicate origin fields come from `GITHUB_*` context; a guard refuses to name `attest.yml` as the builder). What is absent is builder isolation — the build still runs in the caller — so we do not claim L3. A same-repo `uses: ./…` boundary additionally rests on branch protection over `.github/workflows/attest.yml`. - Pinning the workflow identity above is what makes that level checkable. Verification that omits `--certificate-identity` (or loosens it to a regexp) can still succeed while proving a weaker claim: it no longer distinguishes an attestation minted inside `attest.yml` from one produced elsewhere. The exact pin is enforced by `TestVerificationUsesExactIdentity` and `TestPublishedVerifyCommandsAreExact` in `test/releasepolicy`; the reusable-workflow boundary itself is gated by `TestAttestIsSoleSigner`, `TestAttestPredicateUsesOnlyTrustedContext`, and `TestAttestBuilderIdGuardRejectsAttestorAsBuilder`. No level is claimed here without a test behind it. + Pinning the workflow identity above is what makes that level checkable. Verification that omits `--certificate-identity` (or loosens it to a regexp) can still succeed while proving a weaker claim: it no longer distinguishes an attestation minted inside `attest.yml` from one produced elsewhere. The exact pin is enforced by `TestVerificationUsesExactIdentity` and `TestPublishedVerifyCommandsAreExact` in `test/releasepolicy`; the reusable-workflow boundary itself is gated by `TestAttestIsSoleSigner`, `TestAttestIsInvokedAsReusableWorkflow`, `TestAttestPredicateUsesOnlyTrustedContext`, and `TestAttestBuilderIdGuardRejectsAttestorAsBuilder`. No level is claimed here without a test behind it. Retrieve the provenance with `cosign verify-attestation --type slsaprovenance1` against the index digest, and a platform's SBOM with `--type cyclonedx` against that platform's manifest digest (`crane digest --platform linux/amd64 "${IMAGE}:${TAG}"`). diff --git a/docs/operations/verifying-artifacts.md b/docs/operations/verifying-artifacts.md index d0dc7513..87054c07 100644 --- a/docs/operations/verifying-artifacts.md +++ b/docs/operations/verifying-artifacts.md @@ -110,6 +110,7 @@ minted inside the reusable workflow. The published commands keep the exact pin, `TestVerificationUsesExactIdentity` / `TestPublishedVerifyCommandsAreExact` fail the build if a release-path workflow or a fenced doc command drifts to the regexp form. The boundary itself is gated by `TestAttestIsSoleSigner`, +`TestAttestIsInvokedAsReusableWorkflow`, `TestAttestPredicateUsesOnlyTrustedContext`, and `TestAttestBuilderIdGuardRejectsAttestorAsBuilder`. diff --git a/test/releasepolicy/attest_boundary_test.go b/test/releasepolicy/attest_boundary_test.go index db2a4b4c..8912205d 100644 --- a/test/releasepolicy/attest_boundary_test.go +++ b/test/releasepolicy/attest_boundary_test.go @@ -4,7 +4,6 @@ package releasepolicy import ( - "fmt" "os" "path/filepath" "regexp" @@ -14,163 +13,23 @@ import ( "sigs.k8s.io/yaml" ) -// cosignSignCmds are the signing invocations that may exist only inside -// attest.yml. A second home for any of them widens the published identity -// contract: consumers pin one workflow path, so a signer elsewhere is accepted -// under a different SAN with no change to the pin. -// -// Also covers the "not inlined as a job" half of ADR-074 D2: moving these -// invocations into release.yml / publish.yml as ordinary job steps would still -// compile and still green every other test, but Fulcio would stop naming -// attest.yml and identity-pinned verification would quietly fail for consumers. -var cosignSignCmds = regexp.MustCompile(`(?m)(?:^|[\s;|&])(?:retry\s+)?cosign\s+(sign|attest|attest-blob)\b`) - -var attestProvenanceAction = regexp.MustCompile(`(^|/)actions/attest-build-provenance(@|$)`) - // Local attest.yml call shape used by publish.yml / release.yml / selftest. +// Complements isAttestWorkflowCall in workflow_policy_test.go: that helper +// accepts any path whose base is attest.yml; this one insists on the ./ form +// that keeps the call a same-repo workflow_call boundary. var localAttestUses = regexp.MustCompile(`^\./\.github/workflows/attest\.yml(@.+)?$`) -const attestWorkflowName = "attest.yml" - -// TestAttestIsSoleSigner keeps the published certificate identity contract true. -// -// Consumers pin -// -// .../attest.yml@refs/tags/ -// -// so any second workflow that invokes cosign sign/attest/attest-blob (or -// actions/attest-build-provenance) silently widens what that pin accepts. -// attest.yml itself must stay workflow_call-only: any other trigger makes the -// signing identity reachable from a branch push or a dispatch, and inlining -// its steps into a caller would demote the reusable-workflow boundary while -// every other test stayed green. -func TestAttestIsSoleSigner(t *testing.T) { - assertAttestIsWorkflowCallOnly(t) +// TestAttestIsInvokedAsReusableWorkflow pins the half of ADR-074 D2 that +// TestAttestIsSoleSigner (workflow_policy_test.go) does not cover: every +// release-path caller must reach attest.yml through `uses: ./…`, not by +// inlining its jobs. Without this, a refactor could copy the attest steps into +// release.yml, keep workflow_call on an unused attest.yml, and demote the +// Fulcio identity while TestAttestIsSoleSigner still saw cosign only inside +// attest.yml — until the copy started signing too. +func TestAttestIsInvokedAsReusableWorkflow(t *testing.T) { assertAttestIsInvokedAsReusableWorkflow(t) - - paths := append([]string{}, workflowFiles(t)...) - paths = append(paths, compositeActionFiles(t)...) - - for _, path := range paths { - base := filepath.Base(path) - raw, err := os.ReadFile(path) - if err != nil { - t.Fatalf("read %s: %v", path, err) - } - - if isCompositeActionPath(path) { - var doc struct { - Runs struct { - Using string `json:"using"` - Steps []policyStep `json:"steps"` - } `json:"runs"` - } - if err := yaml.Unmarshal(raw, &doc); err != nil { - t.Fatalf("parse %s: %v", path, err) - } - if doc.Runs.Using != "" && doc.Runs.Using != "composite" { - continue - } - assertStepsForbidForeignSigners(t, relGithub(path), false /* allowCosign */, doc.Runs.Steps) - continue - } - - var doc struct { - Jobs map[string]struct { - Steps []policyStep `json:"steps"` - } `json:"jobs"` - } - if err := yaml.Unmarshal(raw, &doc); err != nil { - t.Fatalf("parse %s: %v", path, err) - } - - for jobName, job := range doc.Jobs { - where := fmt.Sprintf("%s: job %q", base, jobName) - assertStepsForbidForeignSigners(t, where, base == attestWorkflowName, job.Steps) - } - } -} - -// policyStep is the subset of a workflow/composite step the signer checks -// reason about. -type policyStep struct { - Name string `json:"name"` - Run string `json:"run"` - Uses string `json:"uses"` -} - -func assertStepsForbidForeignSigners(t *testing.T, where string, allowCosign bool, steps []policyStep) { - t.Helper() - - for _, step := range steps { - if attestProvenanceAction.MatchString(step.Uses) { - t.Errorf("%s step %q uses %s; provenance must be emitted by attest.yml "+ - "via cosign, not actions/attest-build-provenance", - where, step.Name, step.Uses) - } - if allowCosign { - continue - } - if m := cosignSignCmds.FindStringSubmatch(step.Run); m != nil { - t.Errorf("%s step %q invokes `cosign %s`; attest.yml must be the sole signer", - where, step.Name, m[1]) - } - } -} - -// compositeActionFiles returns every local composite action.yml. -func compositeActionFiles(t *testing.T) []string { - t.Helper() - - paths, err := filepath.Glob("../../.github/actions/*/action.yml") - if err != nil { - t.Fatalf("glob composite actions: %v", err) - } - return paths } -func isCompositeActionPath(path string) bool { - return strings.Contains(filepath.ToSlash(path), "/.github/actions/") -} - -func relGithub(path string) string { - slash := filepath.ToSlash(path) - if i := strings.Index(slash, ".github/"); i >= 0 { - return slash[i:] - } - return filepath.Base(path) -} - -func assertAttestIsWorkflowCallOnly(t *testing.T) { - t.Helper() - - raw, err := os.ReadFile(filepath.Join(workflowDir, attestWorkflowName)) - if err != nil { - t.Fatalf("read %s: %v", attestWorkflowName, err) - } - - triggers := workflowTriggers(raw, t) - if len(triggers) == 0 { - t.Fatalf("%s declares no triggers; it must be workflow_call-only", attestWorkflowName) - } - for name := range triggers { - if name != "workflow_call" { - t.Errorf("%s is triggered by %q; only workflow_call is allowed so the signing "+ - "identity cannot be reached from a branch or dispatch", - attestWorkflowName, name) - } - } - if _, ok := triggers["workflow_call"]; !ok { - t.Errorf("%s is missing workflow_call", attestWorkflowName) - } -} - -// assertAttestIsInvokedAsReusableWorkflow checks that every release-path caller -// reaches attest.yml through `uses: ./…`, not by inlining its jobs. Without -// this, a refactor could copy the attest steps into release.yml, keep -// workflow_call on an unused attest.yml, and demote the Fulcio identity while -// TestAttestIsSoleSigner still saw cosign only inside attest.yml — until the -// copy started signing too. func assertAttestIsInvokedAsReusableWorkflow(t *testing.T) { t.Helper() @@ -207,34 +66,6 @@ func assertAttestIsInvokedAsReusableWorkflow(t *testing.T) { } } -// workflowTriggers returns the `on:` block, tolerating YAML 1.1 turning a bare -// `on:` key into the boolean true (the same hazard workflowCallOutputs faces). -func workflowTriggers(raw []byte, t *testing.T) map[string]any { - t.Helper() - - var doc map[string]any - if err := yaml.Unmarshal(raw, &doc); err != nil { - t.Fatalf("parse workflow triggers: %v", err) - } - for _, key := range []string{"on", boolTrue} { - switch v := doc[key].(type) { - case map[string]any: - return v - case string: - return map[string]any{v: nil} - case []any: - out := map[string]any{} - for _, item := range v { - if s, ok := item.(string); ok { - out[s] = nil - } - } - return out - } - } - return nil -} - // TestAttestPredicateUsesOnlyTrustedContext pins the half of ADR-074 D2 that // makes provenance unforgeable by the build process: the predicate's origin // fields are minted from GITHUB_* / github.workflow_ref inside attest.yml, never From 60f9b9354b32b379542bed97550d5fde073eaee2 Mon Sep 17 00:00:00 2001 From: asivanadi0 Date: Wed, 9 Sep 2026 03:30:18 +0000 Subject: [PATCH 5/9] test: require attest.yml call in each release-path workflow Signed-off-by: asivanadi0 --- test/releasepolicy/attest_boundary_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/releasepolicy/attest_boundary_test.go b/test/releasepolicy/attest_boundary_test.go index 8912205d..d38f1c02 100644 --- a/test/releasepolicy/attest_boundary_test.go +++ b/test/releasepolicy/attest_boundary_test.go @@ -34,7 +34,6 @@ func assertAttestIsInvokedAsReusableWorkflow(t *testing.T) { t.Helper() callers := []string{wfRelease, wfPublish, wfAttestSmoke} - found := 0 for _, base := range callers { raw, err := os.ReadFile(filepath.Join(workflowDir, base)) if err != nil { @@ -48,21 +47,22 @@ func assertAttestIsInvokedAsReusableWorkflow(t *testing.T) { if err := yaml.Unmarshal(raw, &doc); err != nil { t.Fatalf("parse %s: %v", base, err) } + foundInWorkflow := 0 for jobName, job := range doc.Jobs { if !localAttestUses.MatchString(strings.TrimSpace(job.Uses)) { continue } - found++ + foundInWorkflow++ if !strings.HasPrefix(job.Uses, "./") { t.Errorf("%s: job %q calls attest.yml as %q; same-repo reusable "+ "calls must use the ./ form so the call is a workflow_call boundary", base, jobName, job.Uses) } } - } - if found == 0 { - t.Fatalf("no release-path workflow calls ./.github/workflows/attest.yml; " + - "attestation must stay behind a reusable-workflow boundary") + if foundInWorkflow == 0 { + t.Errorf("%s does not call ./.github/workflows/attest.yml; "+ + "attestation must stay behind a reusable-workflow boundary", base) + } } } From 2fe17c58ce78fa9e76ae2f8d8d2a1899fed701ad Mon Sep 17 00:00:00 2001 From: asivanadi0 Date: Thu, 10 Sep 2026 18:36:48 +0000 Subject: [PATCH 6/9] test: execute attest builder-id guard and close boundary holes Extract-and-run the provenance step (accept, attestor-as-builder reject, fail-closed malformed ref) and assert provenance.json origin fields. Sweep every attest.yml call via isAttestWorkflowCall for the ./ form, scan workflow/job/step env for inputs./needs.*.outputs laundering, and gate cited Test* names plus SECURITY.md exact identity pins in docspolicy. Signed-off-by: asivanadi0 --- test/docspolicy/cited_gate_tests_test.go | 92 +++++++ test/docspolicy/verification_page_test.go | 52 ++++ test/releasepolicy/attest_boundary_test.go | 293 +++++++++++++++------ 3 files changed, 358 insertions(+), 79 deletions(-) create mode 100644 test/docspolicy/cited_gate_tests_test.go diff --git a/test/docspolicy/cited_gate_tests_test.go b/test/docspolicy/cited_gate_tests_test.go new file mode 100644 index 00000000..7eca3b7b --- /dev/null +++ b/test/docspolicy/cited_gate_tests_test.go @@ -0,0 +1,92 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +package docspolicy + +import ( + "go/ast" + "go/parser" + "go/token" + "os" + "path/filepath" + "regexp" + "strings" + "testing" +) + +// Pages that cite releasepolicy / docspolicy gate tests by name. Renaming a +// Test* without updating these pages would leave the prose asserting +// enforcement by a test that no longer exists; nothing else catches that. +var gateTestCitePages = []string{ + "../../SECURITY.md", + "../../docs/operations/verifying-artifacts.md", +} + +// testIdent matches a Go test function identifier cited in prose. +var testIdent = regexp.MustCompile(`\bTest[A-Z][A-Za-z0-9_]+\b`) + +// TestCitedGateTestsExist fails when SECURITY.md or verifying-artifacts.md +// names a Test* that is not defined under test/releasepolicy/ or +// test/docspolicy/. Keeps the gate-test enumeration honest without requiring +// the list to be duplicated verbatim in both pages. +func TestCitedGateTestsExist(t *testing.T) { + defined := definedTests(t, []string{"../releasepolicy", "."}) + + for _, page := range gateTestCitePages { + raw, err := os.ReadFile(page) + if err != nil { + t.Fatalf("read %s: %v", page, err) + } + seen := map[string]bool{} + for _, m := range testIdent.FindAllString(string(raw), -1) { + if seen[m] { + continue + } + seen[m] = true + if !defined[m] { + t.Errorf("%s cites %s, but no such test exists under test/releasepolicy or test/docspolicy", + filepath.Base(page), m) + } + } + if len(seen) == 0 && filepath.Base(page) == "SECURITY.md" { + t.Errorf("%s cites no Test* gate names; the Build Level paragraph must name the tests behind the claim", + filepath.Base(page)) + } + } +} + +func definedTests(t *testing.T, dirs []string) map[string]bool { + t.Helper() + out := map[string]bool{} + fset := token.NewFileSet() + for _, dir := range dirs { + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("read %s: %v", dir, err) + } + for _, e := range entries { + if e.IsDir() || !strings.HasSuffix(e.Name(), "_test.go") { + continue + } + path := filepath.Join(dir, e.Name()) + f, err := parser.ParseFile(fset, path, nil, 0) + if err != nil { + t.Fatalf("parse %s: %v", path, err) + } + for _, decl := range f.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Recv != nil || fn.Name == nil { + continue + } + name := fn.Name.Name + if strings.HasPrefix(name, "Test") { + out[name] = true + } + } + } + } + if len(out) == 0 { + t.Fatal("no Test* functions found under the scanned packages") + } + return out +} diff --git a/test/docspolicy/verification_page_test.go b/test/docspolicy/verification_page_test.go index d5276d87..07260926 100644 --- a/test/docspolicy/verification_page_test.go +++ b/test/docspolicy/verification_page_test.go @@ -189,3 +189,55 @@ func navHasPath(node any, want string) bool { } return false } + +// securityMd is the root security policy; its fenced verify commands are as +// copy-pasteable as the operations page, so a wrong-org or missing identity +// pin there is the same class of defect TestVerificationPagePinsAnExactIdentity +// catches for verifying-artifacts.md. +const securityMd = "../../SECURITY.md" + +// TestSecurityMdPinsAnExactIdentity extends the exact-identity check to +// SECURITY.md's bash fences. TestPublishedVerifyCommandsAreExact only rejects +// the regexp form; this one requires the NVIDIA attest.yml identity on every +// cosign verify* command, matching what TestVerificationPagePinsAnExactIdentity +// already enforces for the operations page. +func TestSecurityMdPinsAnExactIdentity(t *testing.T) { + raw, err := os.ReadFile(securityMd) + if err != nil { + t.Fatalf("read %s: %v", securityMd, err) + } + matches := bashFence.FindAllStringSubmatch(string(raw), -1) + if len(matches) == 0 { + t.Fatalf("%s has no ```bash blocks with verify commands", securityMd) + } + blocks := make([]string, 0, len(matches)) + for _, m := range matches { + blocks = append(blocks, m[1]) + } + joined := strings.Join(blocks, "\n") + + if strings.Contains(joined, "--certificate-identity-regexp") { + t.Error("SECURITY.md publishes --certificate-identity-regexp in a fenced command") + } + + const wantIdentity = "https://github.com/NVIDIA/cluster-readiness-engine" + + "/.github/workflows/attest.yml@refs/tags/" + + cmds := cosignVerifyCommands(joined) + if len(cmds) == 0 { + t.Fatal("SECURITY.md publishes no cosign verify commands in bash fences") + } + for _, c := range cmds { + flat := strings.Join(strings.Fields(c), " ") + if !strings.Contains(flat, "--certificate-identity") { + t.Errorf("SECURITY.md command pins no --certificate-identity: %s", flat) + continue + } + if !strings.Contains(flat, wantIdentity) && !strings.Contains(flat, "${ID}") { + t.Errorf("SECURITY.md command does not pin this repository's identity: %s", flat) + } + if !strings.Contains(flat, "--certificate-oidc-issuer") { + t.Errorf("SECURITY.md command pins no --certificate-oidc-issuer: %s", flat) + } + } +} diff --git a/test/releasepolicy/attest_boundary_test.go b/test/releasepolicy/attest_boundary_test.go index d38f1c02..f309bf7a 100644 --- a/test/releasepolicy/attest_boundary_test.go +++ b/test/releasepolicy/attest_boundary_test.go @@ -4,7 +4,10 @@ package releasepolicy import ( + "encoding/json" + "maps" "os" + "os/exec" "path/filepath" "regexp" "strings" @@ -19,13 +22,18 @@ import ( // that keeps the call a same-repo workflow_call boundary. var localAttestUses = regexp.MustCompile(`^\./\.github/workflows/attest\.yml(@.+)?$`) +// needsOutputRef matches a GitHub Actions expression that reads a job output. +// Those outputs are typically pass-throughs of workflow_call inputs (see +// validate.outputs.subject_kind), so folding one into an origin field is the +// same laundering as referencing inputs.* directly. +var needsOutputRef = regexp.MustCompile(`needs\.[A-Za-z0-9_-]+\.outputs\.[A-Za-z0-9_-]+`) + // TestAttestIsInvokedAsReusableWorkflow pins the half of ADR-074 D2 that // TestAttestIsSoleSigner (workflow_policy_test.go) does not cover: every -// release-path caller must reach attest.yml through `uses: ./…`, not by -// inlining its jobs. Without this, a refactor could copy the attest steps into -// release.yml, keep workflow_call on an unused attest.yml, and demote the -// Fulcio identity while TestAttestIsSoleSigner still saw cosign only inside -// attest.yml — until the copy started signing too. +// release-path caller must reach attest.yml through `uses: ./…`. The unique +// gap this test closes is a caller silently dropping its attest.yml call so +// nothing gets signed at all (TestAttestIsSoleSigner already rejects copying +// cosign sign/attest into a non-attest workflow). func TestAttestIsInvokedAsReusableWorkflow(t *testing.T) { assertAttestIsInvokedAsReusableWorkflow(t) } @@ -33,9 +41,15 @@ func TestAttestIsInvokedAsReusableWorkflow(t *testing.T) { func assertAttestIsInvokedAsReusableWorkflow(t *testing.T) { t.Helper() - callers := []string{wfRelease, wfPublish, wfAttestSmoke} - for _, base := range callers { - raw, err := os.ReadFile(filepath.Join(workflowDir, base)) + // Must-call list: release-path workflows that are required to invoke + // attest.yml. Form-checking sweeps every workflow (including future + // callers outside this list) via isAttestWorkflowCall below. + mustCall := []string{wfRelease, wfPublish, wfAttestSmoke} + called := map[string]bool{} + + for _, path := range workflowFiles(t) { + base := filepath.Base(path) + raw, err := os.ReadFile(path) if err != nil { t.Fatalf("read %s: %v", base, err) } @@ -47,19 +61,26 @@ func assertAttestIsInvokedAsReusableWorkflow(t *testing.T) { if err := yaml.Unmarshal(raw, &doc); err != nil { t.Fatalf("parse %s: %v", base, err) } - foundInWorkflow := 0 for jobName, job := range doc.Jobs { - if !localAttestUses.MatchString(strings.TrimSpace(job.Uses)) { + uses := strings.TrimSpace(job.Uses) + if !isAttestWorkflowCall(uses) { continue } - foundInWorkflow++ - if !strings.HasPrefix(job.Uses, "./") { + called[base] = true + // Assert the ./ form on EVERY attest.yml call, not only those that + // already match localAttestUses. An org-qualified sha-pinned call + // would otherwise keep the must-call count green while attesting + // under a stale pre-hardening attest.yml. + if !localAttestUses.MatchString(uses) || !strings.HasPrefix(uses, "./") { t.Errorf("%s: job %q calls attest.yml as %q; same-repo reusable "+ "calls must use the ./ form so the call is a workflow_call boundary", - base, jobName, job.Uses) + base, jobName, uses) } } - if foundInWorkflow == 0 { + } + + for _, base := range mustCall { + if !called[base] { t.Errorf("%s does not call ./.github/workflows/attest.yml; "+ "attestation must stay behind a reusable-workflow boundary", base) } @@ -67,32 +88,40 @@ func assertAttestIsInvokedAsReusableWorkflow(t *testing.T) { } // TestAttestPredicateUsesOnlyTrustedContext pins the half of ADR-074 D2 that -// makes provenance unforgeable by the build process: the predicate's origin -// fields are minted from GITHUB_* / github.workflow_ref inside attest.yml, never -// from a caller-supplied inputs.* value. +// makes provenance origin fields unforgeable by the build process: repository, +// ref, sha, server, run id, and builder.id are minted from GITHUB_* / +// github.workflow_ref inside attest.yml, never from a caller-supplied +// inputs.* value (including one laundered through needs.*.outputs.*). // -// A refactor that wired --arg repo ${{ inputs.repository }} (or passed the same -// through env) would still produce a green release whose predicate said whatever -// the caller asked. This test fails that change before it ships. +// subjectKind is intentionally out of scope here: it is a caller-chosen, +// enum-validated inputs.subject_kind pass-through and shapes the predicate +// without claiming origin. Origin fields are executed below so a second jq +// writer or a reformatted --arg line cannot slip past a first-occurrence scan. func TestAttestPredicateUsesOnlyTrustedContext(t *testing.T) { step := provenanceStep(t) - // The step must not pull workflow_call inputs into the provenance surface. - for envName, envVal := range step.Env { + // Reject inputs.* / needs.*.outputs.* on every env level the step can see, + // and inside the run block, for anything that could reach an origin field. + // SUBJECT_KIND may legitimately expand needs.validate.outputs.subject_kind. + for envName, envVal := range step.MergedEnv { + if envName == "SUBJECT_KIND" { + continue + } if strings.Contains(envVal, "inputs.") { t.Errorf("provenance step env %q expands %q; origin fields must not "+ "be sourced from workflow_call inputs", envName, envVal) } + if needsOutputRef.MatchString(envVal) { + t.Errorf("provenance step env %q expands %q; origin fields must not "+ + "be laundered through needs.*.outputs.* (caller inputs)", envName, envVal) + } } if strings.Contains(step.Run, "inputs.") { - t.Errorf("provenance step run block references inputs.*; the predicate " + + t.Errorf("provenance step run block references inputs.*; origin fields " + "must be built from trusted context only") } - // CALLER_WORKFLOW_REF (builder.id) must come from github.workflow_ref, which - // GitHub sets to the workflow that started the run — the caller — not from - // an input a caller could forge. - callerRef, ok := step.Env["CALLER_WORKFLOW_REF"] + callerRef, ok := step.MergedEnv["CALLER_WORKFLOW_REF"] if !ok { t.Fatal("provenance step is missing CALLER_WORKFLOW_REF; builder.id must " + "be derived from github.workflow_ref") @@ -102,64 +131,159 @@ func TestAttestPredicateUsesOnlyTrustedContext(t *testing.T) { "so a caller cannot name an arbitrary builder", callerRef) } - // arg -> required substring in the --arg value. builder is special: it is - // derived from CALLER_WORKFLOW_REF / GITHUB_SERVER_URL via builder_id. - wantByArg := map[string]string{ - "repo": "GITHUB_REPOSITORY", - "ref": "GITHUB_REF", - "sha": "GITHUB_SHA", - "server": "GITHUB_SERVER_URL", - "run_id": "GITHUB_RUN_ID", - "builder": "builder_id", - } - for arg, want := range wantByArg { - flag := "--arg " + arg + " " - idx := strings.Index(step.Run, flag) - if idx < 0 { - t.Errorf("provenance jq is missing --arg %s; the predicate must still "+ - "emit that origin field", arg) - continue - } - rest := step.Run[idx+len(flag):] - if nl := strings.IndexByte(rest, '\n'); nl >= 0 { - rest = rest[:nl] - } - rest = strings.TrimSpace(rest) - if !strings.Contains(rest, want) { - t.Errorf("--arg %s value %q must expand %s (trusted context), "+ - "not a caller-supplied input", arg, rest, want) - } + // Execute the step and assert the resulting provenance.json origin fields. + // Grepping --arg lines misses a later writer and false-fails on formatting. + dir := t.TempDir() + scriptPath := filepath.Join(dir, "provenance.sh") + if err := os.WriteFile(scriptPath, []byte(step.Run), 0o600); err != nil { + t.Fatalf("write provenance script: %v", err) + } + + const ( + wantRepo = "NVIDIA/cluster-readiness-engine" + wantRef = "refs/tags/v9.9.9" + wantSHA = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + wantServer = "https://github.com" + wantRunID = "424242" + wantKind = "image" + ) + callerWorkflow := wantRepo + "/.github/workflows/release.yml@" + wantRef + env := append(os.Environ(), + "CALLER_WORKFLOW_REF="+callerWorkflow, + "GITHUB_REPOSITORY="+wantRepo, + "GITHUB_REF="+wantRef, + "GITHUB_SHA="+wantSHA, + "GITHUB_SERVER_URL="+wantServer, + "GITHUB_RUN_ID="+wantRunID, + "SUBJECT_KIND="+wantKind, + ) + cmd := exec.Command("bash", scriptPath) + cmd.Dir = dir + cmd.Env = env + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("provenance step rejected a legitimate caller:\n%s", out) + } + + raw, err := os.ReadFile(filepath.Join(dir, "provenance.json")) + if err != nil { + t.Fatalf("read provenance.json: %v", err) + } + var pred struct { + BuildDefinition struct { + ExternalParameters struct { + Repository string `json:"repository"` + Ref string `json:"ref"` + SubjectKind string `json:"subjectKind"` + } `json:"externalParameters"` + ResolvedDependencies []struct { + Digest struct { + GitCommit string `json:"gitCommit"` + } `json:"digest"` + } `json:"resolvedDependencies"` + } `json:"buildDefinition"` + RunDetails struct { + Builder struct { + ID string `json:"id"` + } `json:"builder"` + Metadata struct { + InvocationID string `json:"invocationId"` + } `json:"metadata"` + } `json:"runDetails"` + } + if err := json.Unmarshal(raw, &pred); err != nil { + t.Fatalf("parse provenance.json: %v\n%s", err, raw) + } + ep := pred.BuildDefinition.ExternalParameters + if ep.Repository != wantRepo { + t.Errorf("externalParameters.repository = %q, want %q (trusted GITHUB_REPOSITORY)", + ep.Repository, wantRepo) + } + if ep.Ref != wantRef { + t.Errorf("externalParameters.ref = %q, want %q (trusted GITHUB_REF)", ep.Ref, wantRef) + } + if ep.SubjectKind != wantKind { + t.Errorf("externalParameters.subjectKind = %q, want %q", ep.SubjectKind, wantKind) + } + if len(pred.BuildDefinition.ResolvedDependencies) == 0 || + pred.BuildDefinition.ResolvedDependencies[0].Digest.GitCommit != wantSHA { + t.Errorf("resolvedDependencies gitCommit missing or wrong; want %q", wantSHA) + } + wantBuilder := wantServer + "/" + wantRepo + "/.github/workflows/release.yml" + if pred.RunDetails.Builder.ID != wantBuilder { + t.Errorf("runDetails.builder.id = %q, want %q", pred.RunDetails.Builder.ID, wantBuilder) + } + wantInvocation := wantServer + "/" + wantRepo + "/actions/runs/" + wantRunID + if pred.RunDetails.Metadata.InvocationID != wantInvocation { + t.Errorf("runDetails.metadata.invocationId = %q, want %q", + pred.RunDetails.Metadata.InvocationID, wantInvocation) } } // TestAttestBuilderIdGuardRejectsAttestorAsBuilder pins the guard that keeps -// runDetails.builder.id honest. Naming attest.yml as the builder would make the -// predicate false on its face and is exactly the trade ADR-074 D2 refuses when -// it keeps L2 rather than inflating to L3. +// runDetails.builder.id honest by extracting and executing it — the same +// convention attest_guards_test.go uses for the validate step. Grepping for +// the needle would stay green if exit 1 were deleted or the comparison flipped. func TestAttestBuilderIdGuardRejectsAttestorAsBuilder(t *testing.T) { step := provenanceStep(t) - - const needle = `"/.github/workflows/attest.yml"` - if !strings.Contains(step.Run, needle) && !strings.Contains(step.Run, "'/.github/workflows/attest.yml'") { - // Accept either quoting style used by the shell guard. - if !strings.Contains(step.Run, "/.github/workflows/attest.yml") { - t.Fatal("provenance step is missing the builder_id == attest.yml guard") - } + dir := t.TempDir() + scriptPath := filepath.Join(dir, "provenance.sh") + if err := os.WriteFile(scriptPath, []byte(step.Run), 0o600); err != nil { + t.Fatalf("write provenance script: %v", err) } - if !strings.Contains(step.Run, "builder_id") { - t.Fatal("provenance step does not compute builder_id") + + baseEnv := func(callerRef string) []string { + return append(os.Environ(), + "CALLER_WORKFLOW_REF="+callerRef, + "GITHUB_REPOSITORY=NVIDIA/cluster-readiness-engine", + "GITHUB_REF=refs/tags/v1.2.3", + "GITHUB_SHA=bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + "GITHUB_SERVER_URL=https://github.com", + "GITHUB_RUN_ID=1", + "SUBJECT_KIND=image", + ) } - if !strings.Contains(step.Run, "attestor cannot be the builder") && - !strings.Contains(step.Run, "builder resolved to attest.yml") { - t.Error("provenance step must refuse when builder_id resolves to attest.yml " + - "(message should name the attestor-as-builder failure)") + run := func(t *testing.T, callerRef string) (bool, string) { + t.Helper() + cmd := exec.Command("bash", scriptPath) + cmd.Dir = t.TempDir() + cmd.Env = baseEnv(callerRef) + out, err := cmd.CombinedOutput() + return err == nil, string(out) } + + t.Run("accepts caller workflow as builder", func(t *testing.T) { + ok, out := run(t, "NVIDIA/cluster-readiness-engine/.github/workflows/release.yml@refs/tags/v1.2.3") + if !ok { + t.Fatalf("guard rejected a legitimate caller:\n%s", out) + } + }) + + t.Run("rejects attest.yml as builder", func(t *testing.T) { + ok, out := run(t, "NVIDIA/cluster-readiness-engine/.github/workflows/attest.yml@refs/tags/v1.2.3") + if ok { + t.Fatal("guard accepted attest.yml as builder_id; attestor cannot be the builder") + } + if !strings.Contains(out, "attestor cannot be the builder") && + !strings.Contains(out, "builder resolved to attest.yml") { + t.Errorf("rejected, but not by the attestor-as-builder guard:\n%s", out) + } + }) + + t.Run("fail-closed on malformed workflow_ref", func(t *testing.T) { + ok, out := run(t, "NVIDIA/cluster-readiness-engine/.github/workflows/release.yml") + if ok { + t.Fatal("guard accepted a workflow_ref without @ref; must fail closed") + } + if !strings.Contains(out, "unexpected GITHUB_WORKFLOW_REF") { + t.Errorf("rejected, but not by the fail-closed case guard:\n%s", out) + } + }) } type namedEnvStep struct { - Name string - Run string - Env map[string]string + Name string + Run string + MergedEnv map[string]string // workflow + job + step, later wins } func provenanceStep(t *testing.T) namedEnvStep { @@ -171,7 +295,9 @@ func provenanceStep(t *testing.T) namedEnvStep { } var doc struct { + Env map[string]string `json:"env"` Jobs map[string]struct { + Env map[string]string `json:"env"` Steps []struct { Name string `json:"name"` Run string `json:"run"` @@ -186,12 +312,21 @@ func provenanceStep(t *testing.T) namedEnvStep { const wantName = "Generate SLSA provenance predicate" for jobName, job := range doc.Jobs { for _, step := range job.Steps { - if step.Name == wantName { - if strings.TrimSpace(step.Run) == "" { - t.Fatalf("%s job %q step %q has an empty run block", - attestWorkflowName, jobName, wantName) - } - return namedEnvStep{Name: step.Name, Run: step.Run, Env: step.Env} + if step.Name != wantName { + continue + } + if strings.TrimSpace(step.Run) == "" { + t.Fatalf("%s job %q step %q has an empty run block", + attestWorkflowName, jobName, wantName) + } + merged := map[string]string{} + for _, m := range []map[string]string{doc.Env, job.Env, step.Env} { + maps.Copy(merged, m) + } + return namedEnvStep{ + Name: step.Name, + Run: step.Run, + MergedEnv: merged, } } } From 0b15384f12e27bdf53f41012fb57f5e6b6cb3235 Mon Sep 17 00:00:00 2001 From: asivanadi0 Date: Thu, 10 Sep 2026 18:36:48 +0000 Subject: [PATCH 7/9] docs: correct ADR-074 D2 claims from ndipebot review Name tag rulesets for release identity, scope the origin-field claim around SUBJECT_KIND, record deferred L3 path costs (job_workflow_ref and sole-signer split), include chart/installer/notices/SBOMs in L2 wording, fix the attest.yml header contradiction, and point verifying-artifacts at SECURITY.md for the gate-test list. Signed-off-by: asivanadi0 --- .github/workflows/attest.yml | 35 ++++++++++---------- .github/workflows/release.yml | 9 ++--- SECURITY.md | 4 +-- docs/designs/074-supply-chain-attestation.md | 19 +++++++---- docs/operations/verifying-artifacts.md | 30 +++++++++-------- 5 files changed, 53 insertions(+), 44 deletions(-) diff --git a/.github/workflows/attest.yml b/.github/workflows/attest.yml index c2b11661..f45d4287 100644 --- a/.github/workflows/attest.yml +++ b/.github/workflows/attest.yml @@ -3,12 +3,9 @@ # Reusable workflow that signs and attests a release artifact. # -# This is intended to become the only place in the repository that invokes -# `cosign sign`, `cosign attest`, or `cosign attest-blob`. It is NOT yet: at the -# time this workflow landed, publish.yml still signs the container image inline. -# #266, #267 and #268 route the image, chart and binaries through here; #274 -# adds the test that keeps it true afterwards. Concentrating signing does two -# things (ADR-074 decision 4): +# This is the only place in the repository that invokes `cosign sign`, +# `cosign attest`, or `cosign attest-blob`. `TestAttestIsSoleSigner` keeps that +# true. Concentrating signing does two things (ADR-074 decision 4): # # 1. It gives every released artifact ONE certificate identity to pin. cosign # uses the OIDC `job_workflow_ref` as the Fulcio SAN, so signing from a @@ -17,22 +14,26 @@ # https://github.com/NVIDIA/cluster-readiness-engine/.github/workflows/attest.yml@refs/tags/ # # Signing inline in each caller would instead produce one identity per -# calling workflow and per ref, which is what makes the current -# SECURITY.md verification command accept a main-branch build as a release. +# calling workflow and per ref, which is what made the pre-epic SECURITY.md +# verification command accept a main-branch build as a release. # # 2. It isolates the signing token from caller-defined build steps. # -# This is SLSA Build **L2**, not L3, and nothing here may claim otherwise. L3 -# requires the BUILD to run inside the protected reusable workflow; the builds -# stay in the callers (`docker buildx` in publish.yml, the Go cross-compile and -# `helm package` in release.yml) and this workflow signs a digest it is handed. -# A caller that produced the wrong artifact would get a faithful signature over -# the wrong digest. Reaching L3 means moving artifact generation in here, which -# is a separate decision. +# This is SLSA Build **L2**, not L3, and nothing here may claim otherwise. +# Provenance is unforgeable by the build process today (minted here from +# trusted context), but builder isolation is absent: the builds stay in the +# callers — `docker buildx` inside the reusable `build-image.yml` (invoked by +# publish.yml / release.yml), and the Go cross-compile / `helm package` in +# release.yml — and this workflow signs a digest it is handed. A caller that +# produced the wrong artifact would get a faithful signature over the wrong +# digest. Reaching L3 means moving artifact generation behind a protected +# build+attest boundary (see ADR-074 decision 4 deferred path), which is a +# separate decision. # # Because the build happens in the caller, `runDetails.builder.id` in the -# provenance names the CALLER's workflow, not this one. Naming the attestor as -# the builder would make the predicate false on its face. +# provenance names the CALLER's top-level workflow (release.yml / publish.yml), +# not this one and not build-image.yml where image builds actually run. Naming +# the attestor as the builder would make the predicate false on its face. name: Attest (reusable) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e263c70e..019253c0 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -440,9 +440,10 @@ jobs: path: release-assets/ retention-days: 7 - # Nine published assets each need a verifying bundle: the four binaries, the - # installer, and the four SBOMs. attest.yml signs one subject per call, so - # this is a matrix over subjects rather than nine near-identical jobs. + # Ten published assets each need a verifying bundle: the four binaries, the + # installer, THIRD_PARTY_NOTICES.md, and the four SBOMs. attest.yml signs one + # subject per call, so this is a matrix over subjects rather than ten + # near-identical jobs. # # The binaries carry their SBOM as a predicate, which binds the SBOM to the # binary. The SBOM files are ALSO signed in their own right, because a user @@ -458,7 +459,7 @@ jobs: id-token: write strategy: fail-fast: false - # Nine legs, and the four binary legs sign twice, so an unbounded fan-out + # Ten legs, and the four binary legs sign twice, so an unbounded fan-out # is ~13 near-simultaneous calls to public-good Fulcio and Rekor from one # caller. attest.yml retries on a fixed 5s/10s backoff with no jitter, so # legs that fail together retry together. The gate requires every leg, so diff --git a/SECURITY.md b/SECURITY.md index df15e777..1ab020b4 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -81,9 +81,9 @@ We credit reporters of confirmed vulnerabilities in the release notes of the fix Use `--certificate-identity`, not `--certificate-identity-regexp`. An identity that names no workflow and no ref also accepts images built from branches, which are not releases and are labelled non-production when they are signed. - **Build Level (per artifact).** Every artifact this repository publishes today — the `manager` image (index provenance), the Helm chart, the `nvcrectl` binaries, and `installer` — carries SLSA Build Provenance at **Build L2**. The claim is per-artifact, not project-wide: nothing here states a single level for "the release". Provenance is unforgeable by the build process because it is minted inside the reusable `attest.yml` workflow (Fulcio names that workflow; predicate origin fields come from `GITHUB_*` context; a guard refuses to name `attest.yml` as the builder). What is absent is builder isolation — the build still runs in the caller — so we do not claim L3. A same-repo `uses: ./…` boundary additionally rests on branch protection over `.github/workflows/attest.yml`. + **Build Level (per artifact).** Artifacts this repository publishes with Build L2 provenance today include the `manager` image (index provenance), the Helm chart, the `nvcrectl` binaries, `installer`, `THIRD_PARTY_NOTICES.md`, and the standalone `nvcrectl-*.cyclonedx.json` SBOM release assets. The claim is per-artifact, not project-wide: nothing here states a single level for "the release". Provenance origin fields are unforgeable by the build process because they are minted inside the reusable `attest.yml` workflow (Fulcio names that workflow; origin fields come from `GITHUB_*` context; a guard refuses to name `attest.yml` as the builder). What is absent is builder isolation — the build still runs in the caller (for images, inside `build-image.yml` invoked by the top-level orchestrator) — so we do not claim L3. A same-repo `uses: ./…` boundary additionally rests on branch protection over `.github/workflows/attest.yml` for `main` pushes, and on tag protection / repository rulesets over `v*` tags for the release identity everyone pins. - Pinning the workflow identity above is what makes that level checkable. Verification that omits `--certificate-identity` (or loosens it to a regexp) can still succeed while proving a weaker claim: it no longer distinguishes an attestation minted inside `attest.yml` from one produced elsewhere. The exact pin is enforced by `TestVerificationUsesExactIdentity` and `TestPublishedVerifyCommandsAreExact` in `test/releasepolicy`; the reusable-workflow boundary itself is gated by `TestAttestIsSoleSigner`, `TestAttestIsInvokedAsReusableWorkflow`, `TestAttestPredicateUsesOnlyTrustedContext`, and `TestAttestBuilderIdGuardRejectsAttestorAsBuilder`. No level is claimed here without a test behind it. + Pinning the workflow identity above is what makes that level checkable. Verification that omits `--certificate-identity` (or loosens it to a regexp) can still succeed while proving a weaker claim: it no longer distinguishes an attestation minted inside `attest.yml` from one produced elsewhere. `TestVerificationUsesExactIdentity` and `TestPublishedVerifyCommandsAreExact` in `test/releasepolicy` reject the `--certificate-identity-regexp` form (and bare `gh attestation verify` in release-path workflows); the published verification page's exact NVIDIA identity pin is gated by `TestVerificationPagePinsAnExactIdentity` in `test/docspolicy`. The reusable-workflow boundary itself is gated by `TestAttestIsSoleSigner`, `TestAttestIsInvokedAsReusableWorkflow`, `TestAttestPredicateUsesOnlyTrustedContext`, and `TestAttestBuilderIdGuardRejectsAttestorAsBuilder`. No level is claimed here without a test behind it. Retrieve the provenance with `cosign verify-attestation --type slsaprovenance1` against the index digest, and a platform's SBOM with `--type cyclonedx` against that platform's manifest digest (`crane digest --platform linux/amd64 "${IMAGE}:${TAG}"`). diff --git a/docs/designs/074-supply-chain-attestation.md b/docs/designs/074-supply-chain-attestation.md index 1e13c188..35fb2a07 100644 --- a/docs/designs/074-supply-chain-attestation.md +++ b/docs/designs/074-supply-chain-attestation.md @@ -66,19 +66,24 @@ This contract is published to users and enforced by us. Both the release-time ga The reason is decision 3. cosign uses the OIDC `job_workflow_ref` as the certificate SAN, so signing from one reusable workflow collapses every artifact onto one identity path where only the ref varies. Signing inline in each caller would give the chart, the image, and the binaries three different identities, and a `main` build a fourth that looks just as legitimate. It also isolates the signing step: caller-defined build steps run in a different job from the one holding the signing token. -**This design targets SLSA Build L2 for every artifact we publish today — binaries and images alike.** Claims are per-artifact, not project-wide: nothing here may state a single Build Level for "the release" as a whole. +**This design targets SLSA Build L2 for every artifact we publish today — the `manager` image, the Helm chart, the `nvcrectl` binaries, and `installer` alike** (plus the blob subjects that also carry provenance, such as `THIRD_PARTY_NOTICES.md` and the standalone SBOM files). Claims are per-artifact, not project-wide: nothing here may state a single Build Level for "the release" as a whole. The original L2 framing was imprecise in a way that made L3 look further away than it is. SLSA v1.0 Build L3 requires that **provenance is unforgeable by the build process**. That is not the same thing as builder isolation. On the unforgeability requirement, `attest.yml` already does most of the work: - It is a genuine reusable workflow (`on: workflow_call`), invoked as `uses: ./.github/workflows/attest.yml` from the callers. It is not an inlined job, so the Fulcio certificate names **`attest.yml`**, not the caller. -- Every value that shapes the provenance predicate comes from trusted context **inside** that workflow — `GITHUB_REPOSITORY`, `GITHUB_REF`, `GITHUB_SHA`, `GITHUB_SERVER_URL`, `GITHUB_RUN_ID`, and the caller workflow ref GitHub itself sets — and not from a caller-supplied `inputs.*` field. A caller cannot dictate what the predicate says about origin. +- Provenance **origin** fields — repository, ref, commit, server, run id, and `builder.id` — come from trusted context **inside** that workflow (`GITHUB_REPOSITORY`, `GITHUB_REF`, `GITHUB_SHA`, `GITHUB_SERVER_URL`, `GITHUB_RUN_ID`, and the caller workflow ref GitHub itself sets) and not from a caller-supplied `inputs.*` field. A caller cannot dictate what the predicate says about origin. (`externalParameters.subjectKind` is an enum-validated pass-through of `inputs.subject_kind` and is out of that origin claim.) - A guard refuses to proceed if `builder_id` resolves to `attest.yml`, on the grounds that the attestor cannot be the builder. Naming the attestor as the builder would make the predicate false on its face, which is worse than claiming the wrong level. -So the build steps in `release.yml` / `publish.yml` cannot forge or tamper with the provenance today. What is genuinely absent is **builder isolation**: the builds stay in the callers — `docker buildx` in `publish.yml` / `build-image.yml`, the Go cross-compile and `helm package` in `release.yml` — and `attest.yml` receives a digest and signs it. `runDetails.builder.id` truthfully names the caller. A compromised caller would produce a bad artifact that `attest.yml` would then honestly attest. That honesty is deliberate, and any change here must not trade a true predicate for a higher number. +So the build steps in the callers cannot forge or tamper with provenance origin fields today. What is genuinely absent is **builder isolation**: the builds stay in the callers — `docker buildx` inside the reusable `build-image.yml` (invoked by `publish.yml` / `release.yml`), and the Go cross-compile and `helm package` in `release.yml` — and `attest.yml` receives a digest and signs it. `runDetails.builder.id` truthfully names the top-level caller (`release.yml` / `publish.yml`), which for images orchestrates rather than runs the build steps themselves. A compromised caller would produce a bad artifact that `attest.yml` would then honestly attest. That honesty is deliberate, and any change here must not trade a true predicate for a higher number. -Two consequences follow. First, no artifact or document may claim L3 — not the ADR, not the release notes, not `SECURITY.md` — until a recorded decision moves a specific artifact across that line. Second, the same-repo reusable-workflow caveat must be stated whenever this boundary is discussed: `uses: ./…` isolates attestation from the caller's *build steps*, not from *write access*. Caller and attestor live in one repository, so the practical strength of the boundary rests on branch protection over `.github/workflows/attest.yml`. Claiming L3 without saying so would imply stronger isolation than exists. +Two consequences follow. First, no artifact or document may claim L3 — not the ADR, not the release notes, not `SECURITY.md` — until a recorded decision moves a specific artifact across that line. Second, the same-repo reusable-workflow caveat must be stated whenever this boundary is discussed: `uses: ./…` isolates attestation from the caller's *build steps*, not from *write access*. Caller and attestor live in one repository. For the **dev-image** identity (`publish.yml` on `main`), branch protection over `.github/workflows/attest.yml` is the relevant control. For the **release** identity (`release.yml` / `publish.yml` on `v*` tags), the SAN everyone pins (`attest.yml@refs/tags/${TAG}`) is minted from whatever commit the pushed tag points at, so the control that gates release identity is **tag protection / repository rulesets over `v*` tags**, not branch protection alone. Claiming L3 without saying so would imply stronger isolation than exists. -A concrete path to per-artifact L3 for images exists and is recorded as deferred, not rejected: `build-image.yml` is already a reusable workflow that builds the image; if provenance were minted there, build and attestation would share one protected boundary and `builder.id` would be both honest and L3-qualifying. That would be an image-only change — `nvcrectl` binaries are still built and attested from the release job and would stay at L2 — which is why every published claim must stay per-artifact. Until that decision is taken, both binaries and images remain Build L2 under the corrected reasoning above. +A concrete path to per-artifact L3 for images exists and is recorded as deferred, not rejected: `build-image.yml` is already a reusable workflow that builds the image; if provenance were minted there, build and attestation would share one protected boundary. Two costs of that path must be recorded now so a follow-on does not discover them after building it: + +1. **Identity mechanics.** If provenance used the same derivation `attest.yml` uses today (`github.workflow_ref`), `builder.id` would still name the top-level `release.yml` / `publish.yml`, not `build-image.yml`. The honest, L3-qualifying identity comes from `job_workflow_ref` — the pattern in GitHub's SLSA Build L3 guide cited in References. +2. **Sole-signer split.** Signing inside `build-image.yml` moves the Fulcio SAN off `attest.yml`, splitting the single pinned identity that decision 3 and Alternative 2 exist to protect: image attestations would verify only under a `build-image.yml@…` identity while the chart and binaries stayed under `attest.yml@…`, every published image verify command would break, and `TestAttestIsSoleSigner` would fail. + +That would still be an image-only change — chart, `installer`, and `nvcrectl` binaries would stay at L2 — which is why every published claim must stay per-artifact. Until that decision is taken, the image, chart, binaries, and installer remain Build L2 under the corrected reasoning above. `attest.yml` validates every input before use: digests must match `^sha256:[0-9a-f]{64}$`, no input may contain a newline or carriage return, and the caller's authoritative `expected_digest` is compared against an independently resolved digest with a mismatch failing the job. It refuses to run on a non-tag ref unless an explicit `allow_untagged` input is set, so a test run cannot quietly produce something that looks like a release attestation. @@ -155,7 +160,7 @@ The contract lives in YAML, and the failure mode is silent: a signing step delet ## Rationale - **Exact identity over regexp** is the single highest-value decision here. Every other gap is a missing artifact, which is visibly missing. A too-permissive verification command is an artifact that appears present and correct while asserting less than the reader believes. -- **Reusable workflow** makes provenance unforgeable by the build process (Fulcio names `attest.yml`; the predicate is minted from trusted context) and collapses every artifact onto one pinnable identity. That is the corrected L2 claim in decision 4. It does **not** by itself provide builder isolation, and a same-repo `uses: ./…` boundary additionally depends on branch protection over `attest.yml` — see decision 4. +- **Reusable workflow** makes provenance **origin** fields unforgeable by the build process (Fulcio names `attest.yml`; origin fields are minted from trusted context) and collapses every artifact onto one pinnable identity. That is the corrected L2 claim in decision 4. It does **not** by itself provide builder isolation, and a same-repo `uses: ./…` boundary additionally depends on branch protection over `attest.yml` for `main` and on tag rulesets over `v*` for the release identity — see decision 4. - **Per-platform SBOM subjects** follow from what an SBOM is. Getting this wrong is not a policy choice, it is a category error, and it is already shipping. - **Verify what we produce** costs one job and converts a class of silent failure into a red release. Attestations nobody checks are decoration. - **Signing the SBOMs** closes the gap that remains after everything else is signed, at the cost of a few more bundles. @@ -166,7 +171,7 @@ The contract lives in YAML, and the failure mode is silent: a signing step delet - Every released artifact answers "who built this, from what source, containing what," with one command and one pinned identity. - The multi-platform SBOM defect is fixed, and the fail-closed digest checks prevent it from recurring silently. -- Provenance is SLSA Build L2 **per artifact** (binaries and images today), with a single pinnable attestor identity (`attest.yml`). The level is only checkable when verification pins that workflow. L3 remains available as a follow-on — per artifact, with the same-repo caveat stated — and is not foreclosed by anything here. +- Provenance is SLSA Build L2 **per artifact** (image, Helm chart, `nvcrectl` binaries, and `installer` today), with a single pinnable attestor identity (`attest.yml`). The level is only checkable when verification pins that workflow. L3 remains available as a follow-on — per artifact, with the same-repo caveat and the deferred-path costs in decision 4 stated — and is not foreclosed by anything here. - Admission controllers can enforce the same contract the documentation publishes, so install-time and runtime checks cannot drift. - Post-publication tampering has a bounded detection window instead of depending on a user noticing. diff --git a/docs/operations/verifying-artifacts.md b/docs/operations/verifying-artifacts.md index 87054c07..6bd90c25 100644 --- a/docs/operations/verifying-artifacts.md +++ b/docs/operations/verifying-artifacts.md @@ -92,27 +92,29 @@ branch build cannot pass as either. ## Build Level, and why the pin matters Provenance is **SLSA Build L2 per artifact** today — the `manager` image index, the Helm -chart, each `nvcrectl` binary, and `installer` alike. The project does not publish a +chart, each `nvcrectl` binary, `installer`, `THIRD_PARTY_NOTICES.md`, and the standalone +`nvcrectl-*.cyclonedx.json` SBOM release assets alike. The project does not publish a single project-wide level, and it does not claim L3: builder isolation is still missing -(the build runs in the caller; `attest.yml` attests a digest it is handed). What L2 here -does claim is that provenance is unforgeable by that build process — Fulcio names -`attest.yml`, the predicate's origin fields come from trusted `GITHUB_*` context inside -that workflow, and a guard refuses to list `attest.yml` as the builder. +(the build runs in the caller — for images, inside `build-image.yml` invoked by the +top-level orchestrator; `attest.yml` attests a digest it is handed). What L2 here does +claim is that provenance **origin** fields are unforgeable by that build process — Fulcio +names `attest.yml`, origin fields come from trusted `GITHUB_*` context inside that +workflow, and a guard refuses to list `attest.yml` as the builder. The same-repo reusable-workflow form (`uses: ./.github/workflows/attest.yml`) isolates -attestation from the caller's build steps, not from write access to the repository. The -practical strength of that boundary rests on branch protection over `attest.yml`. +attestation from the caller's build steps, not from write access to the repository. For +`main` (dev-image) pushes the practical control is branch protection over `attest.yml`; +for the release identity pinned above, it is tag protection / repository rulesets over +`v*` tags. **Pinning the identity above is the check that makes the level observable.** Drop the `--certificate-identity` flag (or replace it with a loose regexp) and verification can still go green while proving less than Build L2: you no longer know the attestation was -minted inside the reusable workflow. The published commands keep the exact pin, and -`TestVerificationUsesExactIdentity` / `TestPublishedVerifyCommandsAreExact` fail the -build if a release-path workflow or a fenced doc command drifts to the regexp form. The -boundary itself is gated by `TestAttestIsSoleSigner`, -`TestAttestIsInvokedAsReusableWorkflow`, -`TestAttestPredicateUsesOnlyTrustedContext`, and -`TestAttestBuilderIdGuardRejectsAttestorAsBuilder`. +minted inside the reusable workflow. The published commands keep the exact pin. The +gate-test enumeration that backs this claim lives in +[SECURITY.md](../../SECURITY.md#supply-chain) (kept in one place so renaming a test +cannot leave two pages asserting enforcement by a name that no longer exists); this page +links rather than duplicates it. ## Verifying the container image From cf4b87cdc51848df49b990d7e428d700b9d778d3 Mon Sep 17 00:00:00 2001 From: asivanadi0 Date: Fri, 11 Sep 2026 00:42:42 +0000 Subject: [PATCH 8/9] test+docs: close ndipebot 2nd-pass attest boundary holes Reject inputs[/needs[ bracket laundering in provenance env scan, assert resolvedDependencies[0].uri, add ADR-074 to gateTestCitePages, require SECURITY.md literal identity pins, and fix the release.yml ~14 / v* parenthetical / attest.yml origin-field header nits. Signed-off-by: asivanadi0 --- .github/workflows/attest.yml | 9 +++-- .github/workflows/release.yml | 2 +- docs/designs/074-supply-chain-attestation.md | 2 +- test/docspolicy/cited_gate_tests_test.go | 9 ++--- test/docspolicy/verification_page_test.go | 5 ++- test/releasepolicy/attest_boundary_test.go | 37 ++++++++++++++------ 6 files changed, 44 insertions(+), 20 deletions(-) diff --git a/.github/workflows/attest.yml b/.github/workflows/attest.yml index f45d4287..138e9b96 100644 --- a/.github/workflows/attest.yml +++ b/.github/workflows/attest.yml @@ -20,9 +20,12 @@ # 2. It isolates the signing token from caller-defined build steps. # # This is SLSA Build **L2**, not L3, and nothing here may claim otherwise. -# Provenance is unforgeable by the build process today (minted here from -# trusted context), but builder isolation is absent: the builds stay in the -# callers — `docker buildx` inside the reusable `build-image.yml` (invoked by +# Provenance **origin** fields are unforgeable by the build process today +# (minted here from trusted context: GITHUB_REPOSITORY / REF / SHA / SERVER_URL / +# RUN_ID and github.workflow_ref). `externalParameters.subjectKind` is an +# enum-validated pass-through of inputs.subject_kind and is out of that origin +# claim. Builder isolation is absent: the builds stay in the callers — +# `docker buildx` inside the reusable `build-image.yml` (invoked by # publish.yml / release.yml), and the Go cross-compile / `helm package` in # release.yml — and this workflow signs a digest it is handed. A caller that # produced the wrong artifact would get a faithful signature over the wrong diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 019253c0..b7c16a5f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -460,7 +460,7 @@ jobs: strategy: fail-fast: false # Ten legs, and the four binary legs sign twice, so an unbounded fan-out - # is ~13 near-simultaneous calls to public-good Fulcio and Rekor from one + # is ~14 near-simultaneous calls to public-good Fulcio and Rekor from one # caller. attest.yml retries on a fixed 5s/10s backoff with no jitter, so # legs that fail together retry together. The gate requires every leg, so # one correlated blip fails the whole release. Three at a time keeps the diff --git a/docs/designs/074-supply-chain-attestation.md b/docs/designs/074-supply-chain-attestation.md index 35fb2a07..b5bfa216 100644 --- a/docs/designs/074-supply-chain-attestation.md +++ b/docs/designs/074-supply-chain-attestation.md @@ -76,7 +76,7 @@ The original L2 framing was imprecise in a way that made L3 look further away th So the build steps in the callers cannot forge or tamper with provenance origin fields today. What is genuinely absent is **builder isolation**: the builds stay in the callers — `docker buildx` inside the reusable `build-image.yml` (invoked by `publish.yml` / `release.yml`), and the Go cross-compile and `helm package` in `release.yml` — and `attest.yml` receives a digest and signs it. `runDetails.builder.id` truthfully names the top-level caller (`release.yml` / `publish.yml`), which for images orchestrates rather than runs the build steps themselves. A compromised caller would produce a bad artifact that `attest.yml` would then honestly attest. That honesty is deliberate, and any change here must not trade a true predicate for a higher number. -Two consequences follow. First, no artifact or document may claim L3 — not the ADR, not the release notes, not `SECURITY.md` — until a recorded decision moves a specific artifact across that line. Second, the same-repo reusable-workflow caveat must be stated whenever this boundary is discussed: `uses: ./…` isolates attestation from the caller's *build steps*, not from *write access*. Caller and attestor live in one repository. For the **dev-image** identity (`publish.yml` on `main`), branch protection over `.github/workflows/attest.yml` is the relevant control. For the **release** identity (`release.yml` / `publish.yml` on `v*` tags), the SAN everyone pins (`attest.yml@refs/tags/${TAG}`) is minted from whatever commit the pushed tag points at, so the control that gates release identity is **tag protection / repository rulesets over `v*` tags**, not branch protection alone. Claiming L3 without saying so would imply stronger isolation than exists. +Two consequences follow. First, no artifact or document may claim L3 — not the ADR, not the release notes, not `SECURITY.md` — until a recorded decision moves a specific artifact across that line. Second, the same-repo reusable-workflow caveat must be stated whenever this boundary is discussed: `uses: ./…` isolates attestation from the caller's *build steps*, not from *write access*. Caller and attestor live in one repository. For the **dev-image** identity (`publish.yml` on `main`), branch protection over `.github/workflows/attest.yml` is the relevant control. For the **release** identity (`release.yml` on `v*` tags), the SAN everyone pins (`attest.yml@refs/tags/${TAG}`) is minted from whatever commit the pushed tag points at, so the control that gates release identity is **tag protection / repository rulesets over `v*` tags**, not branch protection alone. Claiming L3 without saying so would imply stronger isolation than exists. A concrete path to per-artifact L3 for images exists and is recorded as deferred, not rejected: `build-image.yml` is already a reusable workflow that builds the image; if provenance were minted there, build and attestation would share one protected boundary. Two costs of that path must be recorded now so a follow-on does not discover them after building it: diff --git a/test/docspolicy/cited_gate_tests_test.go b/test/docspolicy/cited_gate_tests_test.go index 7eca3b7b..2d1b731d 100644 --- a/test/docspolicy/cited_gate_tests_test.go +++ b/test/docspolicy/cited_gate_tests_test.go @@ -20,15 +20,16 @@ import ( var gateTestCitePages = []string{ "../../SECURITY.md", "../../docs/operations/verifying-artifacts.md", + "../../docs/designs/074-supply-chain-attestation.md", } // testIdent matches a Go test function identifier cited in prose. var testIdent = regexp.MustCompile(`\bTest[A-Z][A-Za-z0-9_]+\b`) -// TestCitedGateTestsExist fails when SECURITY.md or verifying-artifacts.md -// names a Test* that is not defined under test/releasepolicy/ or -// test/docspolicy/. Keeps the gate-test enumeration honest without requiring -// the list to be duplicated verbatim in both pages. +// TestCitedGateTestsExist fails when a gateTestCitePages entry names a +// Test* that is not defined under test/releasepolicy/ or test/docspolicy/. +// Keeps the gate-test enumeration honest without requiring the list to be +// duplicated verbatim across SECURITY.md, verifying-artifacts.md, and ADR-074. func TestCitedGateTestsExist(t *testing.T) { defined := definedTests(t, []string{"../releasepolicy", "."}) diff --git a/test/docspolicy/verification_page_test.go b/test/docspolicy/verification_page_test.go index 07260926..ecd6ffd3 100644 --- a/test/docspolicy/verification_page_test.go +++ b/test/docspolicy/verification_page_test.go @@ -233,7 +233,10 @@ func TestSecurityMdPinsAnExactIdentity(t *testing.T) { t.Errorf("SECURITY.md command pins no --certificate-identity: %s", flat) continue } - if !strings.Contains(flat, wantIdentity) && !strings.Contains(flat, "${ID}") { + // SECURITY.md inlines the identity; do not accept ${ID} here. A + // command that set ID to another repository could otherwise ride on a + // different fence's literal and still pass a page-wide check. + if !strings.Contains(flat, wantIdentity) { t.Errorf("SECURITY.md command does not pin this repository's identity: %s", flat) } if !strings.Contains(flat, "--certificate-oidc-issuer") { diff --git a/test/releasepolicy/attest_boundary_test.go b/test/releasepolicy/attest_boundary_test.go index f309bf7a..809db8a5 100644 --- a/test/releasepolicy/attest_boundary_test.go +++ b/test/releasepolicy/attest_boundary_test.go @@ -28,6 +28,11 @@ var localAttestUses = regexp.MustCompile(`^\./\.github/workflows/attest\.yml(@.+ // same laundering as referencing inputs.* directly. var needsOutputRef = regexp.MustCompile(`needs\.[A-Za-z0-9_-]+\.outputs\.[A-Za-z0-9_-]+`) +// Bracket-index forms (`inputs['x']`, `needs['job']…`) are valid expression +// syntax and do not contain the literal `inputs.` / `needs.` needles above. +var inputsBracketRef = regexp.MustCompile(`inputs\[`) +var needsBracketRef = regexp.MustCompile(`needs\[`) + // TestAttestIsInvokedAsReusableWorkflow pins the half of ADR-074 D2 that // TestAttestIsSoleSigner (workflow_policy_test.go) does not cover: every // release-path caller must reach attest.yml through `uses: ./…`. The unique @@ -100,25 +105,27 @@ func assertAttestIsInvokedAsReusableWorkflow(t *testing.T) { func TestAttestPredicateUsesOnlyTrustedContext(t *testing.T) { step := provenanceStep(t) - // Reject inputs.* / needs.*.outputs.* on every env level the step can see, - // and inside the run block, for anything that could reach an origin field. + // Reject inputs.* / inputs[ / needs.*.outputs.* / needs[ on every env + // level the step can see, and inside the run block, for anything that + // could reach an origin field. Bracket forms are valid expression syntax + // and would otherwise sneak past a `inputs.` / `needs.` literal scan. // SUBJECT_KIND may legitimately expand needs.validate.outputs.subject_kind. for envName, envVal := range step.MergedEnv { if envName == "SUBJECT_KIND" { continue } - if strings.Contains(envVal, "inputs.") { + if strings.Contains(envVal, "inputs.") || inputsBracketRef.MatchString(envVal) { t.Errorf("provenance step env %q expands %q; origin fields must not "+ "be sourced from workflow_call inputs", envName, envVal) } - if needsOutputRef.MatchString(envVal) { + if needsOutputRef.MatchString(envVal) || needsBracketRef.MatchString(envVal) { t.Errorf("provenance step env %q expands %q; origin fields must not "+ "be laundered through needs.*.outputs.* (caller inputs)", envName, envVal) } } - if strings.Contains(step.Run, "inputs.") { - t.Errorf("provenance step run block references inputs.*; origin fields " + - "must be built from trusted context only") + if strings.Contains(step.Run, "inputs.") || inputsBracketRef.MatchString(step.Run) { + t.Errorf("provenance step run block references inputs.*/inputs[; origin " + + "fields must be built from trusted context only") } callerRef, ok := step.MergedEnv["CALLER_WORKFLOW_REF"] @@ -176,6 +183,7 @@ func TestAttestPredicateUsesOnlyTrustedContext(t *testing.T) { SubjectKind string `json:"subjectKind"` } `json:"externalParameters"` ResolvedDependencies []struct { + URI string `json:"uri"` Digest struct { GitCommit string `json:"gitCommit"` } `json:"digest"` @@ -204,9 +212,18 @@ func TestAttestPredicateUsesOnlyTrustedContext(t *testing.T) { if ep.SubjectKind != wantKind { t.Errorf("externalParameters.subjectKind = %q, want %q", ep.SubjectKind, wantKind) } - if len(pred.BuildDefinition.ResolvedDependencies) == 0 || - pred.BuildDefinition.ResolvedDependencies[0].Digest.GitCommit != wantSHA { - t.Errorf("resolvedDependencies gitCommit missing or wrong; want %q", wantSHA) + if len(pred.BuildDefinition.ResolvedDependencies) == 0 { + t.Fatal("resolvedDependencies missing") + } + rd0 := pred.BuildDefinition.ResolvedDependencies[0] + wantURI := "git+" + wantServer + "/" + wantRepo + "@" + wantRef + if rd0.URI != wantURI { + t.Errorf("resolvedDependencies[0].uri = %q, want %q (trusted SLSA source URI)", + rd0.URI, wantURI) + } + if rd0.Digest.GitCommit != wantSHA { + t.Errorf("resolvedDependencies[0].digest.gitCommit = %q, want %q", + rd0.Digest.GitCommit, wantSHA) } wantBuilder := wantServer + "/" + wantRepo + "/.github/workflows/release.yml" if pred.RunDetails.Builder.ID != wantBuilder { From de122e62a9599730e6c006db4be10228d57b5688 Mon Sep 17 00:00:00 2001 From: asivanadi0 Date: Fri, 11 Sep 2026 15:35:53 +0000 Subject: [PATCH 9/9] docs: fix ADR-074 topology and verifying-artifacts SECURITY link Align the workflow topology diagram with publish.yml running on main (not tags), and use the same absolute SECURITY.md URL style as the page's See also section. Signed-off-by: asivanadi0 --- docs/designs/074-supply-chain-attestation.md | 2 +- docs/operations/verifying-artifacts.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/designs/074-supply-chain-attestation.md b/docs/designs/074-supply-chain-attestation.md index b5bfa216..fe071267 100644 --- a/docs/designs/074-supply-chain-attestation.md +++ b/docs/designs/074-supply-chain-attestation.md @@ -116,7 +116,7 @@ A **daily re-verification job** re-runs the same suite against the latest releas ### Workflow topology ``` -publish.yml (tag) ──┐ +publish.yml (main) ──┐ ├──> attest.yml (workflow_call, isolated signer) release.yml (tag) ──┘ │ ├─ image index -> provenance diff --git a/docs/operations/verifying-artifacts.md b/docs/operations/verifying-artifacts.md index 6bd90c25..70335c53 100644 --- a/docs/operations/verifying-artifacts.md +++ b/docs/operations/verifying-artifacts.md @@ -112,7 +112,7 @@ for the release identity pinned above, it is tag protection / repository ruleset still go green while proving less than Build L2: you no longer know the attestation was minted inside the reusable workflow. The published commands keep the exact pin. The gate-test enumeration that backs this claim lives in -[SECURITY.md](../../SECURITY.md#supply-chain) (kept in one place so renaming a test +[SECURITY.md](https://github.com/NVIDIA/cluster-readiness-engine/blob/main/SECURITY.md#supply-chain) (kept in one place so renaming a test cannot leave two pages asserting enforcement by a name that no longer exists); this page links rather than duplicates it.