diff --git a/.github/workflows/README.md b/.github/workflows/README.md index f4113c740e..3c694c6626 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -12,11 +12,12 @@ Use `false` unless a later command in the same job performs an authenticated Git write. The small number of write-path exceptions use `true` with the machine-checked `# required: authenticated git writes` rationale. -Each workflow declares its default `GITHUB_TOKEN` permissions explicitly and -enumerates scopes instead of using `read-all` or `write-all`. Workflow inputs, -secrets, `github.token`, and attacker-controlled GitHub event metadata are -passed to `run` steps through `env`; they are data and must never be -interpolated into the generated shell program. +Each workflow declares its default `GITHUB_TOKEN` permissions explicitly, and +both workflow defaults and job-level overrides enumerate scopes instead of +using `read-all`, `write-all`, or dynamic grants. Workflow inputs, secrets, +`github.token`, and attacker-controlled GitHub event metadata are passed to +`run` steps through `env`; they are data and must never be interpolated into +the generated shell program. Workflows triggered by `pull_request` cannot reference confidential repository secrets. Canonical governance therefore keeps its pull-request checks local to diff --git a/scripts/check_workflow_trust.py b/scripts/check_workflow_trust.py index 03ecf339c6..fc7dee9635 100644 --- a/scripts/check_workflow_trust.py +++ b/scripts/check_workflow_trust.py @@ -44,6 +44,7 @@ SECRET_CONTEXT_TOKEN_RE = re.compile(r"(? list[Finding]: ) permission_declarations = [ - index + (index, match) for index, line in enumerate(lines) - if re.match(r"^permissions\s*:", line) + if (match := PERMISSIONS_RE.match(line.split("#", 1)[0])) ] - if len(permission_declarations) != 1: + top_level_permissions = [ + (index, match) + for index, match in permission_declarations + if not match.group(1) + ] + if len(top_level_permissions) != 1: findings.append( Finding( path, @@ -166,17 +172,20 @@ def audit_workflow(path: Path) -> list[Finding]: "workflow must declare top-level permissions explicitly exactly once", ) ) - elif lines[permission_declarations[0]].split("#", 1)[0].strip() not in { - "permissions:", - "permissions: {}", - }: - findings.append( - Finding( - path, - permission_declarations[0] + 1, - "workflow permissions must use a scope mapping or explicit empty mapping", + for index, match in permission_declarations: + # GitHub applies job-level permissions after the workflow default. Audit + # every declaration so a job cannot reintroduce read-all, write-all, or + # a dynamic grant beneath an otherwise least-privilege workflow. + inline_value = match.group(2) + if inline_value not in {"", "{}"}: + scope = "workflow" if not match.group(1) else "job" + findings.append( + Finding( + path, + index + 1, + f"{scope} permissions must use a scope mapping or explicit empty mapping", + ) ) - ) for index, line in enumerate(lines): line_number = index + 1 diff --git a/scripts/tests/test_workflow_trust.py b/scripts/tests/test_workflow_trust.py index 9cde5aa300..fc657d15bd 100644 --- a/scripts/tests/test_workflow_trust.py +++ b/scripts/tests/test_workflow_trust.py @@ -113,6 +113,41 @@ steps: dynamic = self.audit("permissions: ${{ inputs.permissions }}\njobs: {}\n") self.assertTrue(any("scope mapping" in finding for finding in dynamic)) + def test_rejects_broad_or_dynamic_job_permission_overrides(self) -> None: + broad = self.audit( + """permissions: + contents: read +jobs: + unsafe: + permissions: write-all +""" + ) + self.assertTrue(any("job permissions" in finding for finding in broad)) + + dynamic = self.audit( + """permissions: {} +jobs: + unsafe: + permissions: ${{ inputs.permissions }} +""" + ) + self.assertTrue(any("job permissions" in finding for finding in dynamic)) + + def test_accepts_explicit_job_permission_mappings(self) -> None: + findings = self.audit( + """permissions: + contents: read +jobs: + read_only: + permissions: {} + publisher: + permissions: + contents: write + id-token: write +""" + ) + self.assertEqual(findings, []) + def test_rejects_shell_template_data_but_accepts_env_data(self) -> None: findings = self.audit( """permissions: {}