diff --git a/.github/workflows/attest-release.yml b/.github/workflows/attest-release.yml index 2bf7710..f441e4a 100644 --- a/.github/workflows/attest-release.yml +++ b/.github/workflows/attest-release.yml @@ -45,6 +45,8 @@ jobs: attestations: write steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - name: fetch what the maintainer signed shell: bash diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2764959..23f0aab 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,7 +67,16 @@ jobs: # holds this job to setting it and the imports job to not setting it. TFG_IMPORT_TABLE_JOB: "1" steps: + # Every checkout in these workflows turns the token off. The checkout + # keeps the job's token in .git/config unless told not to, and the jobs + # go on to run code from the pull request under test. Nothing after a + # checkout here pushes or fetches - the release and the pages talk to + # GitHub through gh and through actions that carry their own token - so + # the credential has no use once the tree is on disk. A guard asks each + # checkout (checkoutcredentials_test.go), so a new one cannot forget. - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: @@ -353,11 +362,6 @@ jobs: timeout-minutes: 15 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - # The checkout keeps the job's token in .git/config unless told not - # to, and this job goes on to run go test over the pull request's - # own code. Nothing here pushes, so the token has no use after the - # checkout. Asked for by an outside review of #117, and the other - # checkouts in these workflows are O230. with: persist-credentials: false @@ -383,6 +387,8 @@ jobs: timeout-minutes: 15 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: @@ -425,6 +431,8 @@ jobs: timeout-minutes: 15 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: @@ -468,6 +476,8 @@ jobs: timeout-minutes: 15 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: @@ -512,6 +522,8 @@ jobs: timeout-minutes: 15 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: @@ -576,6 +588,8 @@ jobs: timeout-minutes: 20 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: @@ -646,6 +660,7 @@ jobs: steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: + persist-credentials: false # The comparison needs the earlier commit, and the default checkout # fetches one. fetch-depth: 0 @@ -742,6 +757,8 @@ jobs: CGO_ENABLED: "1" steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: @@ -817,6 +834,8 @@ jobs: CGO_ENABLED: "0" steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: @@ -893,6 +912,8 @@ jobs: CGO_ENABLED: "0" steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: @@ -948,6 +969,8 @@ jobs: CGO_ENABLED: "0" steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: diff --git a/.github/workflows/dependency-review.yml b/.github/workflows/dependency-review.yml index fed68d5..f5b8ab2 100644 --- a/.github/workflows/dependency-review.yml +++ b/.github/workflows/dependency-review.yml @@ -26,6 +26,8 @@ jobs: timeout-minutes: 10 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - uses: actions/dependency-review-action@a1d282b36b6f3519aa1f3fc636f609c47dddb294 # v5.0.0 with: diff --git a/.github/workflows/dev-build.yml b/.github/workflows/dev-build.yml index 0e376cc..c45562f 100644 --- a/.github/workflows/dev-build.yml +++ b/.github/workflows/dev-build.yml @@ -69,6 +69,8 @@ jobs: CGO_ENABLED: "0" steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: @@ -153,6 +155,8 @@ jobs: shell: bash steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: diff --git a/.github/workflows/pages.yml b/.github/workflows/pages.yml index 818310e..c58709d 100644 --- a/.github/workflows/pages.yml +++ b/.github/workflows/pages.yml @@ -65,6 +65,7 @@ jobs: # nosemgrep: yaml.github-actions.security.workflow-run-target-code-checkout.workflow-run-target-code-checkout - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: + persist-credentials: false # The commit CI passed on, not whatever main happens to be now. On a # dispatch there is no such commit and the default ref is right. ref: ${{ github.event.workflow_run.head_sha || github.ref }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 583d9ff..ecc66b4 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -42,6 +42,8 @@ jobs: CGO_ENABLED: "0" steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: @@ -151,6 +153,8 @@ jobs: CGO_ENABLED: "0" steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: @@ -258,6 +262,8 @@ jobs: shell: bash steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: @@ -363,6 +369,8 @@ jobs: attestations: write steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: diff --git a/.github/workflows/tool-versions.yml b/.github/workflows/tool-versions.yml index 45f7b2c..68dc02b 100644 --- a/.github/workflows/tool-versions.yml +++ b/.github/workflows/tool-versions.yml @@ -39,6 +39,8 @@ jobs: timeout-minutes: 10 steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - name: compare each pin with its latest release env: diff --git a/.github/workflows/verify-release.yml b/.github/workflows/verify-release.yml index cdc5141..e14511e 100644 --- a/.github/workflows/verify-release.yml +++ b/.github/workflows/verify-release.yml @@ -41,6 +41,8 @@ jobs: contents: read steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - name: which release this is id: which @@ -151,6 +153,8 @@ jobs: contents: read steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - name: which release this is id: which diff --git a/internal/guard/checkoutcredentials_test.go b/internal/guard/checkoutcredentials_test.go new file mode 100644 index 0000000..8f0fb0c --- /dev/null +++ b/internal/guard/checkoutcredentials_test.go @@ -0,0 +1,135 @@ +package guard + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/goccy/go-yaml" +) + +// Every checkout in a workflow turns the job's token off once the tree is on +// disk. +// +// actions/checkout writes the token it cloned with into .git/config and leaves +// it there, so that a later step can push. No step in these workflows pushes: +// the release, the attestation and the pages talk to GitHub through gh with a +// token in the environment, or through actions that carry their own. What the +// jobs do after a checkout is run go test over the code of the pull request +// under test - and that code can read .git/config. The token of a pull request +// from a fork is read only, so the class is real and the weight is small, and +// it is the same class on every one of the twenty four checkouts, which is why +// the answer is every one of them and a guard, not a fix at the job somebody +// happened to review (O230). +// +// Asked through the YAML parser rather than by searching the text, because +// `with:` can sit under a comment block and the key can sit anywhere inside +// it - a regular expression tying the key to the line after `uses:` would read +// pages.yml wrong today. The text is still read, once, for a second count of +// the checkouts: a parser that stopped seeing some of the steps would +// otherwise report a clean tree over the ones it dropped. Two readers that +// have to agree replace a number somebody would keep by hand - an outside +// review of #118 asked for an exact count of twenty four, and that constant +// would go stale with the next workflow while proving nothing the agreement +// does not. +func TestEveryCheckoutTurnsItsTokenOff(t *testing.T) { + // Asking the predicate about shapes the tree does not currently contain. + // Every checkout DOES turn its token off today, so a change that weakened + // the rule would find nothing to let through and stay green. These cases + // keep a hold on the rule itself. + for _, c := range []struct { + with map[string]any + off bool + why string + }{ + {map[string]any{"persist-credentials": false}, true, "the key is there and false"}, + {map[string]any{"persist-credentials": "false"}, true, "the action reads its inputs as strings, so a quoted false is the same answer"}, + {nil, false, "no with block at all means the default, which keeps the token"}, + {map[string]any{"fetch-depth": 0}, false, "a with block that says nothing about the token keeps it"}, + {map[string]any{"persist-credentials": true}, false, "the key is there and true"}, + {map[string]any{"persist-credentials": "no"}, false, "a word that is not false is not false"}, + } { + if tokenTurnedOff(c.with) != c.off { + t.Errorf("tokenTurnedOff(%v) should be %v, because %s", c.with, c.off, c.why) + } + } + + dir := filepath.Join(repoRoot(t), ".github", "workflows") + entries, err := os.ReadDir(dir) + if err != nil { + t.Skipf("the workflows are not here: %v", err) + } + + seen, inText := 0, 0 + for _, e := range entries { + name := e.Name() + if e.IsDir() || (!strings.HasSuffix(name, ".yml") && !strings.HasSuffix(name, ".yaml")) { + continue + } + body, err := os.ReadFile(filepath.Join(dir, name)) + if err != nil { + t.Fatalf("reading %s: %v", name, err) + } + inText += strings.Count(withoutYamlComments(string(body)), "uses: actions/checkout@") + var workflow struct { + Jobs map[string]struct { + Steps []struct { + Uses string `yaml:"uses"` + With map[string]any `yaml:"with"` + } `yaml:"steps"` + } `yaml:"jobs"` + } + if err := yaml.Unmarshal(body, &workflow); err != nil { + t.Fatalf("reading %s: %v", name, err) + } + for jobName, job := range workflow.Jobs { + for _, step := range job.Steps { + if !strings.HasPrefix(step.Uses, "actions/checkout@") { + continue + } + seen++ + if tokenTurnedOff(step.With) { + continue + } + t.Errorf("%s, job %q checks out with the token left in .git/config, and the steps "+ + "after it run the pull request's own code. Nothing in these workflows pushes, "+ + "so turn it off:\n"+ + " with:\n"+ + " persist-credentials: false", + name, jobName) + } + } + } + + // The two readings have to agree, and the text has to have found + // something. A parser that stopped seeing steps - a renamed key, a + // changed suffix, a shape it does not decode - would otherwise report a + // clean tree over the checkouts it dropped, which is the way this guard + // is most likely to break. Measured 2026-09-22: twenty four, by both. + if inText == 0 { + t.Errorf("no checkout was found in the text under %s, so this guard checked nothing. "+ + "Either the workflows moved or the way this reads them stopped working", dir) + } + if seen != inText { + t.Errorf("the YAML walk found %d checkouts and the text holds %d. The walk is the one "+ + "that judges them, so every checkout it does not see is one it does not ask "+ + "- find out which shape it stopped decoding", seen, inText) + } +} + +// tokenTurnedOff reports whether a checkout's with block says +// persist-credentials: false. +// +// A bare false parses as a boolean and a quoted one as a string, and the +// action reads either as false, so both are accepted. Anything else - the key +// missing, true, or a word - leaves the token where the action puts it. +func tokenTurnedOff(with map[string]any) bool { + switch v := with["persist-credentials"].(type) { + case bool: + return !v + case string: + return v == "false" + } + return false +}