diff --git a/.github/workflows/attest-selftest.yml b/.github/workflows/attest-selftest.yml index 5b9e2e4c..632c1c49 100644 --- a/.github/workflows/attest-selftest.yml +++ b/.github/workflows/attest-selftest.yml @@ -30,9 +30,23 @@ # the input validation for every subject kind, but validation only -- no # signing branch. # -# The run signs under a NON-RELEASE identity: the caller ref is a branch, so the -# Fulcio SAN carries `@refs/heads/...` and attest.yml logs a loud non-production -# warning. It cannot be mistaken for, or verified as, a release attestation. +# The run signs under a NON-RELEASE identity: the smoke job's ref guard holds +# github.ref to refs/heads/main, so the Fulcio SAN carries `@refs/heads/main` +# and attest.yml logs a loud non-production warning. It cannot be mistaken for, +# or verified as, a release attestation. That guard is load-bearing — see the +# smoke job comment — because workflow_dispatch accepts a tag as its ref. +# +# Rollout: this file on `main` is not what a dispatch at an older `v*` tag runs. +# Tags cut before the ref guard (`v0.2.0`, `v0.2.0-rc.1`, `v0.2.0-rc.2`) still +# ship the repository-only gate; see RELEASE.md "Existing-tag attest-selftest +# rollout". Do not treat merge of this guard as closing that path on those refs. +# +# Repository setting: Attest Self-Test is disabled at the Actions workflow level +# (`state: disabled_manually`) so workflow_dispatch is refused at every ref — +# including those older tags — until the v0.2.0 series is out of use. Re-enabling +# this workflow reopens the old-tag path; only re-enable for a maintainer smoke +# run from refs/heads/main, then disable again. A dispatch does nothing while +# the workflow stays disabled. name: Attest Self-Test @@ -68,7 +82,19 @@ concurrency: jobs: smoke: name: Sign a non-release image through attest.yml - if: github.repository == 'NVIDIA/cluster-readiness-engine' + # The ref guard is load-bearing, not hygiene. workflow_dispatch accepts a + # TAG as its ref, and this workflow always passes allow_untagged: true. + # Dispatched from refs/tags/v*, CALLER_REF in attest.yml is the tag, so the + # non-tag refusal never fires, the non-production warning never prints, and + # Fulcio mints attest.yml@refs/tags/v* — the exact identity SECURITY.md + # tells users proves an official release — for whatever image_tag and + # expected_digest the dispatch supplied. The v* tag ruleset does not cover + # this: minting that identity creates no tag and moves no tag. allow_untagged + # cannot prevent it either; it only relaxes a check, it does not influence + # the certificate. Match publish.yml: hold the caller to refs/heads/main. + if: >- + github.repository == 'NVIDIA/cluster-readiness-engine' + && github.ref == 'refs/heads/main' permissions: contents: read packages: write @@ -89,7 +115,14 @@ jobs: report: name: Report outcome needs: [smoke] - if: always() && github.repository == 'NVIDIA/cluster-readiness-engine' + # Carries the same ref guard as smoke. Without it a workflow_dispatch from a + # non-main ref skips smoke — which is correct, that is what the guard is for + # — and then this job still runs on always() and treats the skip as a failed + # caller gate, turning a deliberate no-op into a red run. + if: >- + always() + && github.repository == 'NVIDIA/cluster-readiness-engine' + && github.ref == 'refs/heads/main' runs-on: ubuntu-latest timeout-minutes: 5 permissions: {} diff --git a/.github/workflows/attest.yml b/.github/workflows/attest.yml index c2b11661..c5c1a739 100644 --- a/.github/workflows/attest.yml +++ b/.github/workflows/attest.yml @@ -279,7 +279,19 @@ jobs: # Release attestations come from tags. A non-tag run is permitted only # when the caller says so explicitly, so a test run cannot quietly # produce something shaped like a release attestation. - if [[ "${CALLER_REF}" != refs/tags/v* ]]; then + # + # The converse is also refused: allow_untagged: true on a v* tag is + # contradictory. Callers that intend a non-production run must not + # reach here on a release ref (see attest-selftest.yml / publish.yml + # ref guards); callers that intend a release run must not pass the + # escape hatch. Without this, a future caller that forgets its own + # ref guard and passes allow_untagged: true from refs/tags/v* would + # mint the release identity while believing it asked for a test run. + if [[ "${CALLER_REF}" == refs/tags/v* ]]; then + if [[ "${IN_ALLOW_UNTAGGED}" == "true" ]]; then + fail "allow_untagged: true is contradictory on release ref ${CALLER_REF}; refuse rather than mint the release identity for a caller that claimed a non-production run" + fi + else if [[ "${IN_ALLOW_UNTAGGED}" != "true" ]]; then fail "attest.yml refuses to run on ${CALLER_REF}; pass allow_untagged: true for a non-production test run" fi diff --git a/RELEASE.md b/RELEASE.md index cd93a3b2..eb9d386d 100644 --- a/RELEASE.md +++ b/RELEASE.md @@ -216,6 +216,37 @@ if nothing published, commit your work, and tag again. **`releases/latest` returns 404.** No stable release exists yet. Use an explicit version in the download URL. +## Existing-tag attest-selftest rollout + +`attest-selftest.yml` on `main` holds `github.ref` to `refs/heads/main`, so a +`workflow_dispatch` at a `v*` ref cannot reach `attest.yml` through that caller. +GitHub selects workflow files from the ref you dispatch at, not from `main`, so +**tags that still contain the pre-fix workflow remain reachable.** + +Affected published tags today: `v0.2.0`, `v0.2.0-rc.1`, `v0.2.0-rc.2` (same +`attest-selftest.yml` blob; repository gate only, `allow_untagged: true`). Older +tags without that workflow are out of scope for this path. + +Merging the `main` fix does **not** close [#340](https://github.com/NVIDIA/cluster-readiness-engine/issues/340) +for those refs. Close the residual gap with both of the following before treating +the trust gap as closed: + +1. **Operational mitigation (immediate).** *Attest Self-Test* is disabled at the + repository Actions level (`state: disabled_manually`), so GitHub refuses + `workflow_dispatch` on it at every ref — including existing vulnerable tags — + independently of this merge. That setting is not visible to tests: if someone + re-enables the workflow for a smoke run and leaves it on, the old-tag path + reopens until the `v0.2.0` series is out of use. Re-enable only for a + maintainer smoke run from `main`, then disable again. Do not dispatch at a + `v*` ref. The `v*` tag ruleset does not block this path. +2. **Next release (durable for new tags).** Cut the next `v*` release from + `main` after the fix lands. New tags carry the guarded workflow. Do not move + or rewrite existing tags to pick up the fix. + +The acceptance criterion "dispatching at a `v*` ref does not reach `attest.yml`" +applies to refs that contain the fix; existing vulnerable tags need the +mitigation above. + ## See also - [CONTRIBUTING.md](CONTRIBUTING.md) — how to get a change into `main` before it ships diff --git a/SECURITY.md b/SECURITY.md index 19c8079f..b52c1e45 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -81,6 +81,21 @@ 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. + The release signing identity is minted only when `attest.yml` runs on a `refs/tags/v*` + caller ref. On `main`, every `workflow_dispatch` caller of that workflow carries a + ref guard (or `release.yml`'s `GITHUB_REF` check) so a dispatch at a tag cannot reach + the attestor through those callers. **Existing release tags cut before that guard + landed — notably `v0.2.0`, `v0.2.0-rc.1`, and `v0.2.0-rc.2` — still ship the older + `attest-selftest.yml`, which gates on repository alone and passes `allow_untagged: + true`.** A dispatch at one of those refs uses the workflow files *on that tag*, not + the fixed copies on `main`, and can still mint `attest.yml@refs/tags/`. + Tag protection / the `v*` ruleset does not cover this path (no tag is created or + moved). Merging the `main` fix alone does not close [#340](https://github.com/NVIDIA/cluster-readiness-engine/issues/340) + for those refs; the residual control is the repository-level disable of *Attest + Self-Test* documented in [RELEASE.md](RELEASE.md#existing-tag-attest-selftest-rollout) + (re-enabling reopens the old-tag path). + + 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/test/releasepolicy/attest_guards_test.go b/test/releasepolicy/attest_guards_test.go index 47229e3d..3c2aad02 100644 --- a/test/releasepolicy/attest_guards_test.go +++ b/test/releasepolicy/attest_guards_test.go @@ -16,6 +16,8 @@ import ( "os" "os/exec" "path/filepath" + "slices" + "sort" "strings" "testing" @@ -245,6 +247,13 @@ func TestAttestValidationRejects(t *testing.T) { inputs{callerRef: "refs/heads/main"}, "refuses to run on refs/heads/main", }, + // allow_untagged on a v* tag is contradictory: the caller claimed a + // non-production run while standing on the release ref that mints the + // identity SECURITY.md pins. Refuse rather than silently mint it. + "allow_untagged on a release tag": { + inputs{inAllowUntagged: boolTrue}, + "allow_untagged: true is contradictory on release ref", + }, "digest with non-hex characters": { inputs{inExpectedDigest: "sha256:zzzz"}, errDigestMustMatch, @@ -467,3 +476,554 @@ func TestAttestWorkflowIsGatedToThisRepository(t *testing.T) { } } } + +// Exact job-level `if:` expressions that keep workflow_dispatch off a v* tag +// from minting the release signing identity (#340). Shared by the pin test and +// the fail-closed recognizer so the allowlist cannot drift from what we assert. +const ( + exactRepoAndMainIf = "github.repository == 'NVIDIA/cluster-readiness-engine'" + + " && github.ref == 'refs/heads/main'" + exactAlwaysRepoAndMainIf = "always() && github.repository == 'NVIDIA/cluster-readiness-engine'" + + " && github.ref == 'refs/heads/main'" + // exactRepoOnlyIf is the job-level repository gate release.yml uses on + // intermediates (build-cli, helm-publish, release-tag, ...). It is not a + // ref guard, but it is the only non-empty if: we allowlist for needs-walk + // traversal so attest-binaries can still see release-tag's shell check. + exactRepoOnlyIf = "github.repository == 'NVIDIA/cluster-readiness-engine'" +) + +// Exact shell comparison release.yml's release-tag job uses to refuse a +// workflow_dispatch whose GITHUB_REF is not the release tag. Mentioning +// GITHUB_REF is not enough; this is the tested restriction. +const exactReleaseTagRefCheck = `[[ "${GITHUB_REF}" != "refs/tags/${INPUT_TAG}" ]]` + +// Fixture job names / uses for fail-closed recognition cases. +const ( + jobCaller = "caller" + jobGuarded = "guarded" + jobReleaseTag = "release-tag" + localAttestUses = "./.github/workflows/attest.yml" +) + +// TestMainBranchAttestCallersPinExactRefGuards pins the full job-level `if:` +// expressions that keep workflow_dispatch off a v* tag from minting the release +// signing identity (#340). +// +// Substring needles are not enough: appending `|| github.event_name == +// 'workflow_dispatch'` keeps both needles present while making the guard +// vacuous (`&&` binds tighter than `||`, and these workflows are +// workflow_dispatch-capable). Pinning the whitespace-normalized whole +// expression is the difference between asserting the guard is mentioned and +// asserting the guard is the condition. +// +// publish.yml carries the same load-bearing shape on `tag` / `attested`. Both +// files are tabled here so deleting either guard fails the same test. +func TestMainBranchAttestCallersPinExactRefGuards(t *testing.T) { + cases := []struct { + workflow string + job string + wantIf string + }{ + {wfAttestSmoke, "smoke", exactRepoAndMainIf}, + {wfAttestSmoke, "report", exactAlwaysRepoAndMainIf}, + {wfPublish, "tag", exactRepoAndMainIf}, + {wfPublish, "attested", exactAlwaysRepoAndMainIf}, + } + for _, tc := range cases { + t.Run(tc.workflow+"/"+tc.job, func(t *testing.T) { + got := normalizeWorkflowIf(jobIfCondition(t, tc.workflow, tc.job)) + if got != tc.wantIf { + t.Errorf("%s job %q if: = %q, want exact %q; a widened expression that still "+ + "mentions the needles would mint the release signing identity on a v* "+ + "workflow_dispatch", tc.workflow, tc.job, got, tc.wantIf) + } + }) + } +} + +// TestAttestDispatchCallersRequireRefGuards closes the class for future +// workflow_dispatch callers of attest.yml that forget their own ref guard. +// +// attest.yml's non-tag refusal only fires when allow_untagged is false; a +// caller that forgets a ref guard and does not pass the flag takes the release +// branch, hits no check, and mints the identity. Enumerating every +// workflow_dispatch caller and requiring an *effective* ref constraint on the +// path to each attest.yml call is what actually closes that class, whatever +// inputs the caller passes. +// +// Recognition is fail-closed: only the exact `if:` expressions pinned above +// and release.yml's exact GITHUB_REF comparison count. Substring mentions of +// github.ref / GITHUB_REF, non-restrictive checks, and ancestor guards behind +// an `if: always()` / `if: !cancelled()` caller do not. Unrecognized shapes +// fail this test so a new pattern must be explicitly allowlisted and covered +// before it protects anything. +// +// This covers callers whose workflow files already contain the fix. Existing +// release tags that still ship the pre-fix attest-selftest.yml are a rollout +// gap documented in SECURITY.md / RELEASE.md — merge alone does not close #340 +// for those refs. +func TestAttestDispatchCallersRequireRefGuards(t *testing.T) { + for _, path := range workflowFiles(t) { + base := filepath.Base(path) + if base == wfAttest { + continue + } + raw, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + triggers := workflowTriggers(raw, t) + if _, ok := triggers["workflow_dispatch"]; !ok { + continue + } + + jobs := loadJobsWithNeeds(t, raw, base) + var callers []string + for name, job := range jobs { + if isAttestWorkflowCall(job.Uses) { + callers = append(callers, name) + } + } + if len(callers) == 0 { + continue + } + sort.Strings(callers) + + for _, caller := range callers { + if !jobOrAncestorHasRefGuard(jobs, caller) { + t.Errorf("%s: job %q calls attest.yml and the workflow has workflow_dispatch, "+ + "but neither %q nor any needs-ancestor carries a recognized ref guard "+ + "(exact main-branch if: or release.yml's GITHUB_REF tag check); without "+ + "one a dispatch at a v* ref mints the release signing identity", + base, caller, caller) + } + } + } +} + +// TestRefGuardRecognitionFailsClosed pins the unsafe shapes that still returned +// true under substring / fail-open recognition: a non-restrictive github.ref +// check, a GITHUB_REF echo with no comparison, an always() caller that inherits +// a guarded ancestor, and a !cancelled() caller that likewise inherits (GitHub's +// documented alternative to always() for overriding skipped-needs). Unrecognized +// shapes must fail closed; only the explicit tested patterns may pass. +func TestRefGuardRecognitionFailsClosed(t *testing.T) { + t.Run("negatives", func(t *testing.T) { + cases := []struct { + name string + jobs map[string]policyJob + caller string + }{ + { + name: "nonrestrictive github.ref inequality", + jobs: map[string]policyJob{ + jobCaller: { + If: "github.ref != ''", + Uses: localAttestUses, + }, + }, + caller: jobCaller, + }, + { + name: "GITHUB_REF echo is not a restriction", + jobs: map[string]policyJob{ + jobCaller: { + Uses: localAttestUses, + Runs: []string{`echo "$GITHUB_REF"`}, + }, + }, + caller: jobCaller, + }, + { + name: "always caller does not inherit ancestor ref guard", + jobs: map[string]policyJob{ + jobGuarded: {If: exactRepoAndMainIf}, + jobCaller: { + If: "always()", + Needs: []string{jobGuarded}, + Uses: localAttestUses, + }, + }, + caller: jobCaller, + }, + { + name: "always with repo gate still does not inherit", + jobs: map[string]policyJob{ + jobGuarded: {If: exactRepoAndMainIf}, + jobCaller: { + If: "always() && github.repository == 'NVIDIA/cluster-readiness-engine'", + Needs: []string{jobGuarded}, + Uses: localAttestUses, + }, + }, + caller: jobCaller, + }, + { + name: "cancelled caller does not inherit ancestor ref guard", + jobs: map[string]policyJob{ + jobGuarded: {If: exactRepoAndMainIf}, + jobCaller: { + If: "${{ !cancelled() }}", + Needs: []string{jobGuarded}, + Uses: localAttestUses, + }, + }, + caller: jobCaller, + }, + { + name: "bare !cancelled() does not inherit ancestor ref guard", + jobs: map[string]policyJob{ + jobGuarded: {If: exactRepoAndMainIf}, + jobCaller: { + If: "!cancelled()", + Needs: []string{jobGuarded}, + Uses: localAttestUses, + }, + }, + caller: jobCaller, + }, + { + name: "release-tag comparison without exit 1 is not a guard", + jobs: map[string]policyJob{ + jobReleaseTag: { + If: exactRepoOnlyIf, + Runs: []string{"if " + exactReleaseTagRefCheck + "; then\n :\nfi"}, + }, + jobCaller: {Needs: []string{jobReleaseTag}, Uses: localAttestUses}, + }, + caller: jobCaller, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if jobHasRefGuard(tc.jobs[tc.caller]) { + t.Errorf("jobHasRefGuard(%q) = true, want false; unrecognized shapes must fail closed", tc.name) + } + if jobOrAncestorHasRefGuard(tc.jobs, tc.caller) { + t.Errorf("jobOrAncestorHasRefGuard(%q) = true, want false; unrecognized shapes must fail closed", tc.name) + } + }) + } + }) + + t.Run("positives", func(t *testing.T) { + cases := []struct { + name string + jobs map[string]policyJob + caller string + }{ + { + name: "exact main-branch if on caller", + jobs: map[string]policyJob{ + jobCaller: {If: exactRepoAndMainIf, Uses: localAttestUses}, + }, + caller: jobCaller, + }, + { + name: "exact always+main if on caller", + jobs: map[string]policyJob{ + jobCaller: {If: exactAlwaysRepoAndMainIf, Uses: localAttestUses}, + }, + caller: jobCaller, + }, + { + name: "inherit exact main-branch if from needs", + jobs: map[string]policyJob{ + jobGuarded: {If: exactRepoAndMainIf}, + jobCaller: {Needs: []string{jobGuarded}, Uses: localAttestUses}, + }, + caller: jobCaller, + }, + { + name: "release-tag GITHUB_REF comparison on ancestor", + jobs: map[string]policyJob{ + jobReleaseTag: { + If: exactRepoOnlyIf, + Runs: []string{"if " + exactReleaseTagRefCheck + "; then\n exit 1\nfi"}, + }, + jobCaller: {Needs: []string{jobReleaseTag}, Uses: localAttestUses}, + }, + caller: jobCaller, + }, + { + name: "inherit release-tag check through repo-only intermediate", + jobs: map[string]policyJob{ + jobReleaseTag: { + If: exactRepoOnlyIf, + Runs: []string{"if " + exactReleaseTagRefCheck + "; then\n exit 1\nfi"}, + }, + "build-cli": {If: exactRepoOnlyIf, Needs: []string{jobReleaseTag}}, + jobCaller: {Needs: []string{"build-cli"}, Uses: localAttestUses}, + }, + caller: jobCaller, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if !jobOrAncestorHasRefGuard(tc.jobs, tc.caller) { + t.Errorf("jobOrAncestorHasRefGuard(%q) = false, want true for an allowlisted pattern", tc.name) + } + }) + } + }) +} + +// TestReleaseTagRefCheckFailsClosed executes release.yml's Resolve tag step and +// pins that the mismatched-ref branch actually rejects. Substring recognition of +// the comparison alone stayed green when that branch's `exit 1` was stubbed with +// `:` (kaynetu #341); the recognizer and this execution table both require the +// rejection to remain effective. The step must also be mandatory: no +// continue-on-error that would swallow a failed check before attest callers run. +func TestReleaseTagRefCheckFailsClosed(t *testing.T) { + script, continueOnError := releaseTagResolveStep(t) + if continueOnError { + t.Fatalf("%s job %q step Resolve tag has continue-on-error; a failed "+ + "mismatched-ref check must fail the job so attest callers do not run", + wfRelease, jobReleaseTag) + } + if !runHasEffectiveReleaseTagRefCheck(script) { + t.Fatalf("%s Resolve tag step no longer carries an effective GITHUB_REF "+ + "mismatch rejection (comparison + exit 1 in the then-branch)", wfRelease) + } + + // Stub only the mismatched-ref branch's exit 1 (the mutation kaynetu applied). + idx := strings.Index(script, exactReleaseTagRefCheck) + if idx < 0 { + t.Fatal("Resolve tag step missing exactReleaseTagRefCheck") + } + rest := script[idx:] + exitIdx := strings.Index(rest, "exit 1") + if exitIdx < 0 { + t.Fatal("Resolve tag step missing exit 1 after ref check") + } + stubbed := script[:idx+exitIdx] + ":" + script[idx+exitIdx+len("exit 1"):] + if runHasEffectiveReleaseTagRefCheck(stubbed) { + t.Fatalf("runHasEffectiveReleaseTagRefCheck still true after stubbing " + + "mismatched-ref exit 1; recognizer must fail closed") + } + + dir := t.TempDir() + acceptEnv := []string{ + "GITHUB_EVENT_NAME=workflow_dispatch", + "GITHUB_REF=refs/tags/v1.2.3", + "INPUT_TAG=v1.2.3", + } + out, failed := runShell(t, dir, script, acceptEnv...) + if failed { + t.Fatalf("Resolve tag rejected a matching dispatch ref: %s", out) + } + + rejectDir := t.TempDir() + rejectEnv := []string{ + "GITHUB_EVENT_NAME=workflow_dispatch", + "GITHUB_REF=refs/heads/main", + "INPUT_TAG=v1.2.3", + } + out, failed = runShell(t, rejectDir, script, rejectEnv...) + if !failed { + t.Fatalf("Resolve tag accepted mismatched ref "+ + "(GITHUB_REF=refs/heads/main, INPUT_TAG=v1.2.3); want exit 1. output: %s", out) + } + if !strings.Contains(out, "dispatched with tag=v1.2.3") { + t.Fatalf("mismatched-ref rejection missing expected error annotation; output: %s", out) + } + + // Stubbed script must accept the mismatched ref (proves the mutation removed + // the rejection the recognizer is supposed to require). + stubDir := t.TempDir() + out, failed = runShell(t, stubDir, stubbed, rejectEnv...) + if failed { + t.Fatalf("stubbed Resolve tag still rejected mismatched ref; mutation did not remove the guard: %s", out) + } +} + +// releaseTagResolveStep returns the body of release.yml's release-tag +// "Resolve tag" step and whether that step sets continue-on-error. +func releaseTagResolveStep(t *testing.T) (script string, continueOnError bool) { + t.Helper() + + path := filepath.Join(workflowDir, wfRelease) + raw, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + var doc struct { + Jobs map[string]struct { + Steps []struct { + Name string `json:"name"` + Run string `json:"run"` + ContinueOnError bool `json:"continue-on-error"` + } `json:"steps"` + } `json:"jobs"` + } + if err := yaml.Unmarshal(raw, &doc); err != nil { + t.Fatalf("parse %s: %v", path, err) + } + job, ok := doc.Jobs[jobReleaseTag] + if !ok { + t.Fatalf("%s missing job %q", wfRelease, jobReleaseTag) + } + for _, step := range job.Steps { + if step.Name == "Resolve tag" { + return step.Run, step.ContinueOnError + } + } + t.Fatalf("%s job %q has no step named %q", wfRelease, jobReleaseTag, "Resolve tag") + return "", false +} + +// normalizeWorkflowIf collapses YAML folded-scalar whitespace so an exact +// expression comparison is stable across `>-` line breaks. +func normalizeWorkflowIf(s string) string { + return strings.Join(strings.Fields(s), " ") +} + +func jobIfCondition(t *testing.T, workflow, jobName string) string { + t.Helper() + + path := filepath.Join(workflowDir, workflow) + raw, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + var wf struct { + Jobs map[string]struct { + If string `json:"if"` + } `json:"jobs"` + } + if err := yaml.Unmarshal(raw, &wf); err != nil { + t.Fatalf("parse %s: %v", path, err) + } + job, ok := wf.Jobs[jobName] + if !ok { + t.Fatalf("%s is missing job %q", workflow, jobName) + } + return job.If +} + +type policyJob struct { + If string + Uses string + Needs []string + Runs []string +} + +func loadJobsWithNeeds(t *testing.T, raw []byte, base string) map[string]policyJob { + t.Helper() + + var doc struct { + Jobs map[string]struct { + If string `json:"if"` + Uses string `json:"uses"` + Needs stringOrSlice `json:"needs"` + Steps []struct { + Run string `json:"run"` + } `json:"steps"` + } `json:"jobs"` + } + if err := yaml.Unmarshal(raw, &doc); err != nil { + t.Fatalf("parse %s: %v", base, err) + } + out := make(map[string]policyJob, len(doc.Jobs)) + for name, job := range doc.Jobs { + runs := make([]string, 0, len(job.Steps)) + for _, step := range job.Steps { + if step.Run != "" { + runs = append(runs, step.Run) + } + } + out[name] = policyJob{ + If: job.If, + Uses: job.Uses, + Needs: append([]string(nil), job.Needs...), + Runs: runs, + } + } + return out +} + +// jobHasRefGuard reports whether job itself carries a recognized, effective +// ref restriction. Fail-closed: only exact allowlisted `if:` expressions and +// release.yml's release-tag shell check that both compares GITHUB_REF and +// exits non-zero on mismatch. A bare github.ref / GITHUB_REF mention, or a +// comparison whose rejection has been stubbed out, is not a guard. +func jobHasRefGuard(job policyJob) bool { + switch normalizeWorkflowIf(job.If) { + case exactRepoAndMainIf, exactAlwaysRepoAndMainIf: + return true + } + return slices.ContainsFunc(job.Runs, runHasEffectiveReleaseTagRefCheck) +} + +// runHasEffectiveReleaseTagRefCheck reports whether run contains release.yml's +// exact GITHUB_REF mismatch comparison *and* an `exit 1` in that then-branch. +// The comparison text alone is insufficient: stubbing the rejection with `:` +// left the previous substring recognizer green (kaynetu #341). +func runHasEffectiveReleaseTagRefCheck(run string) bool { + norm := normalizeWorkflowIf(run) + idx := strings.Index(norm, exactReleaseTagRefCheck) + if idx < 0 { + return false + } + rest := norm[idx:] + // Bound the then-branch at the first " fi" after the comparison so a later + // unrelated `exit 1` in the same step cannot satisfy this check. + end := strings.Index(rest, " fi") + if end < 0 { + end = len(rest) + } + return strings.Contains(rest[:end], "exit 1") +} + +// jobIfPropagatesSkippedNeeds reports whether job's if: is empty or the exact +// repository-only gate release.yml uses on intermediates. Those are the only +// conditions we allowlist for needs-walk traversal. always(), !cancelled(), and +// every other non-empty unrecognized condition can let the job run when a +// guarded ancestor is skipped, so inheritance through them fails closed +// (ndipebot #341). +func jobIfPropagatesSkippedNeeds(job policyJob) bool { + switch normalizeWorkflowIf(stripExpressionWrappers(job.If)) { + case "", exactRepoOnlyIf: + return true + default: + return false + } +} + +// stripExpressionWrappers removes a single surrounding ${{ }} so normalized +// comparisons see the inner expression. +func stripExpressionWrappers(s string) string { + s = strings.TrimSpace(s) + if strings.HasPrefix(s, "${{") && strings.HasSuffix(s, "}}") { + return strings.TrimSpace(s[3 : len(s)-2]) + } + return s +} + +// jobOrAncestorHasRefGuard walks the needs graph from name and returns true +// only when a recognized ref guard sits on the caller or on a needs-ancestor +// reached only through empty if: conditions (skip-propagating). always(), +// !cancelled(), and any other non-empty unrecognized if: block inheritance: +// those jobs can still run when the guarded ancestor is skipped. +func jobOrAncestorHasRefGuard(jobs map[string]policyJob, name string) bool { + seen := map[string]bool{} + var walk func(string) bool + walk = func(n string) bool { + if seen[n] { + return false + } + seen[n] = true + job, ok := jobs[n] + if !ok { + return false + } + if jobHasRefGuard(job) { + return true + } + if !jobIfPropagatesSkippedNeeds(job) { + return false + } + return slices.ContainsFunc(job.Needs, walk) + } + return walk(name) +} diff --git a/test/releasepolicy/shell_scope_test.go b/test/releasepolicy/shell_scope_test.go index 95c4375d..0af29217 100644 --- a/test/releasepolicy/shell_scope_test.go +++ b/test/releasepolicy/shell_scope_test.go @@ -33,21 +33,71 @@ func onReleasePath(base string) bool { return slices.Contains(releasePathWorkflows, base) } +// workflowGlobPatterns are the extensions GitHub Actions accepts for workflow +// files. Enumerating only *.yml would miss a *.yaml workflow_dispatch caller of +// attest.yml (ndipebot / CodeRabbit on #341). +var workflowGlobPatterns = []string{"*.yml", "*.yaml"} + +// globWorkflowFiles lists workflow definitions under dir for both supported +// extensions, sorted for stable output. +func globWorkflowFiles(dir string) ([]string, error) { + var paths []string + for _, pattern := range workflowGlobPatterns { + matched, err := filepath.Glob(filepath.Join(dir, pattern)) + if err != nil { + return nil, err + } + paths = append(paths, matched...) + } + sort.Strings(paths) + return paths, nil +} + // workflowFiles lists the workflow definitions, sorted for stable output. +// GitHub runs both .yml and .yaml; both must be enumerated. func workflowFiles(t *testing.T) []string { t.Helper() - paths, err := filepath.Glob(filepath.Join(workflowDir, "*.yml")) + paths, err := globWorkflowFiles(workflowDir) if err != nil { t.Fatalf("glob workflows: %v", err) } if len(paths) == 0 { t.Fatalf("no workflows found under %s", workflowDir) } - sort.Strings(paths) return paths } +// TestWorkflowFilesDiscoversYamlExtension pins that caller enumeration sees +// both extensions GitHub accepts. A *.yaml-only glob miss left +// TestAttestDispatchCallersRequireRefGuards green for an unguarded +// workflow_dispatch attest caller saved as .yaml (ndipebot #341). +func TestWorkflowFilesDiscoversYamlExtension(t *testing.T) { + dir := t.TempDir() + for _, name := range []string{"aa.yml", "bb.yaml", "cc.yml"} { + if err := os.WriteFile(filepath.Join(dir, name), []byte("name: probe\n"), 0o600); err != nil { + t.Fatalf("write %s: %v", name, err) + } + } + // Non-workflow extension must stay invisible. + if err := os.WriteFile(filepath.Join(dir, "dd.txt"), []byte("nope\n"), 0o600); err != nil { + t.Fatalf("write dd.txt: %v", err) + } + + got, err := globWorkflowFiles(dir) + if err != nil { + t.Fatalf("globWorkflowFiles: %v", err) + } + bases := make([]string, 0, len(got)) + for _, p := range got { + bases = append(bases, filepath.Base(p)) + } + want := []string{"aa.yml", "bb.yaml", "cc.yml"} + if !slices.Equal(bases, want) { + t.Fatalf("globWorkflowFiles bases = %v, want %v (both .yml and .yaml)", bases, want) + } +} + // step is one run step plus the environment it can actually see. type runStep struct { workflow string diff --git a/test/releasepolicy/workflow_graph_test.go b/test/releasepolicy/workflow_graph_test.go index 9a237336..70c97bef 100644 --- a/test/releasepolicy/workflow_graph_test.go +++ b/test/releasepolicy/workflow_graph_test.go @@ -64,7 +64,7 @@ var needsRef = regexp.MustCompile(`needs\.([A-Za-z0-9_-]+)\.outputs\.([A-Za-z0-9 func loadWorkflows(t *testing.T) map[string]wf { t.Helper() - paths, err := filepath.Glob(filepath.Join(workflowDir, "*.yml")) + paths, err := globWorkflowFiles(workflowDir) if err != nil { t.Fatalf("glob workflows: %v", err) } @@ -167,10 +167,7 @@ func TestJobOutputReferencesResolve(t *testing.T) { // `|`, so an interpolated value is executed rather than read. Values must cross // into a shell through `env:`. func TestNoExpressionInterpolationInRunBlocks(t *testing.T) { - paths, err := filepath.Glob(filepath.Join(workflowDir, "*.yml")) - if err != nil { - t.Fatalf("glob workflows: %v", err) - } + paths := workflowFiles(t) var steps struct { Jobs map[string]struct {