Repository navigation
Use caller-scoped job token for release assets #167
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -90,6 +90,7 @@ on: | |
| secrets: | ||
| RELENG_GITHUB_TOKEN: | ||
| required: true | ||
| description: "Temporary Homebrew tap write token only; release assets use the caller job token." | ||
| APPLE_SIGNING_KEY_P12: | ||
| required: true | ||
| APPLE_SIGNING_KEY_P12_PASSWORD: | ||
|
|
@@ -396,7 +397,7 @@ jobs: | |
| GOPROXY: https://proxy.golang.org | ||
| GOSUMDB: sum.golang.org | ||
| permissions: | ||
| contents: read | ||
| contents: write # Create/upload the caller repository's GitHub release assets. | ||
| id-token: write # <-- needed for cosign keyless (OIDC) | ||
| outputs: | ||
| s3_directory: ${{ steps.s3-directory.outputs.S3_DIRECTORY }} | ||
|
|
@@ -511,13 +512,14 @@ jobs: | |
| working-directory: _caller | ||
| run: goreleaser release --clean --config ../_workflows/_generated/.goreleaser.binaries.yaml | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.RELENG_GITHUB_TOKEN }} | ||
| GITHUB_TOKEN: ${{ github.token }} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Suggestion (low-medium confidence): For plain tag pushes, |
||
| GORELEASER_CURRENT_TAG: ${{ inputs.tag }} | ||
| AC_PASSWORD: ${{ secrets.AC_PASSWORD }} | ||
| AC_PROVIDER: ${{ secrets.AC_PROVIDER }} | ||
|
|
||
| # GoReleaser renders the formula (skip_upload) and this step writes it, | ||
| # retrying when a concurrent release moved the tap's branch. | ||
| # The tap-scoped replacement token is deferred; this is the only legacy-token consumer. | ||
| - name: Publish Homebrew formula | ||
| if: inputs.brew == true | ||
| working-directory: _workflows | ||
|
|
@@ -847,7 +849,7 @@ jobs: | |
| working-directory: _caller | ||
| run: goreleaser release --clean --skip=publish --config ../_workflows/_generated/.goreleaser.windows.yaml | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.RELENG_GITHUB_TOKEN }} | ||
| GITHUB_TOKEN: ${{ github.token }} | ||
| GORELEASER_CURRENT_TAG: ${{ inputs.tag }} | ||
| RELEASE_TAG: ${{ inputs.tag }} | ||
| GORELEASER_KEY: ${{ secrets.GORELEASER_PRO_KEY }} | ||
|
|
@@ -1173,7 +1175,7 @@ jobs: | |
| working-directory: _caller | ||
| run: goreleaser release --clean --config ../_workflows/_generated/.goreleaser.docker.yaml | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.RELENG_GITHUB_TOKEN }} | ||
| GITHUB_TOKEN: ${{ github.token }} | ||
| GORELEASER_CURRENT_TAG: ${{ inputs.tag }} | ||
| COSIGN_EXPERIMENTAL: "1" | ||
|
|
||
|
|
@@ -1210,7 +1212,7 @@ jobs: | |
| working-directory: _caller | ||
| run: goreleaser release --clean --config ../_workflows/_generated/.goreleaser.lambda.yaml | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.RELENG_GITHUB_TOKEN }} | ||
| GITHUB_TOKEN: ${{ github.token }} | ||
| GORELEASER_CURRENT_TAG: ${{ inputs.tag }} | ||
|
|
||
| - name: Extract image digests from GoReleaser assets | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| #!/usr/bin/env python3 | ||
| """Check least-privilege token wiring without release credentials or network.""" | ||
| import copy | ||
| import json | ||
| from pathlib import Path | ||
| import subprocess | ||
| import unittest | ||
|
|
||
| ROOT = Path(__file__).resolve().parent.parent | ||
|
|
||
|
|
||
| def load_yaml(path): | ||
| return json.loads(subprocess.check_output(["yq", ".", str(ROOT / path)], text=True)) | ||
|
|
||
|
|
||
| def token_errors(workflow): | ||
| errors = [] | ||
| if workflow["permissions"] != {}: | ||
| errors.append("workflow-wide permissions must remain empty") | ||
| residuals = [] | ||
| for name, job in workflow["jobs"].items(): | ||
| expected = "write" if name == "goreleaser-binaries" else "read" | ||
| if job["permissions"].get("contents") != expected: | ||
| errors.append(f"{name}: unexpected contents permission") | ||
| for step in job.get("steps", []): | ||
| if "secrets.RELENG_GITHUB_TOKEN" in json.dumps(step): | ||
| residuals.append((name, step["name"])) | ||
| if step.get("run", "").startswith("goreleaser release "): | ||
| if step.get("env", {}).get("GITHUB_TOKEN") != "${{ github.token }}": | ||
| errors.append(f"{name}: GoReleaser must use the job token") | ||
| if "gh api" in step.get("run", ""): | ||
| if step.get("env", {}).get("GH_TOKEN") != "${{ github.token }}": | ||
| errors.append(f"{name}: metadata reads must use the job token") | ||
| if residuals != [("goreleaser-binaries", "Publish Homebrew formula")]: | ||
| errors.append("only the deferred Homebrew tap step may consume the legacy token") | ||
| return errors | ||
|
|
||
|
|
||
| class ReleaseTokenTest(unittest.TestCase): | ||
| def setUp(self): | ||
| self.workflow = load_yaml(".github/workflows/release.yaml") | ||
|
|
||
| def test_release_job_permissions_and_tokens(self): | ||
| self.assertEqual(token_errors(self.workflow), []) | ||
| release_steps = [step for job in self.workflow["jobs"].values() for step in job.get("steps", []) | ||
| if step.get("run", "").startswith("goreleaser release ")] | ||
| self.assertEqual(len(release_steps), 4) | ||
| tap = next(step for step in self.workflow["jobs"]["goreleaser-binaries"]["steps"] | ||
| if step["name"] == "Publish Homebrew formula") | ||
| self.assertEqual(tap["if"], "inputs.brew == true") | ||
| self.assertEqual(tap["env"]["GITHUB_TOKEN"], "${{ secrets.RELENG_GITHUB_TOKEN }}") | ||
|
|
||
| def test_only_binaries_publish_github_release(self): | ||
| binaries = load_yaml("templates/.goreleaser-binaries-template.yaml.tmpl") | ||
| self.assertNotEqual(binaries["release"].get("disable"), True) | ||
| self.assertEqual(binaries["brews"][0]["skip_upload"], "true") | ||
| for template in ("windows", "docker-oci", "docker-lambda"): | ||
| config = load_yaml(f"templates/.goreleaser-{template}-template.yaml.tmpl") | ||
| self.assertTrue(config["release"]["disable"]) | ||
|
|
||
| def test_read_only_publisher_negative_control(self): | ||
| changed = copy.deepcopy(self.workflow) | ||
| changed["jobs"]["goreleaser-binaries"]["permissions"]["contents"] = "read" | ||
| self.assertIn("goreleaser-binaries: unexpected contents permission", token_errors(changed)) | ||
|
|
||
| def test_no_broader_grants_negative_controls(self): | ||
| for name in self.workflow["jobs"]: | ||
| if name == "goreleaser-binaries": | ||
| continue | ||
| changed = copy.deepcopy(self.workflow) | ||
| changed["jobs"][name]["permissions"]["contents"] = "write" | ||
| self.assertTrue(token_errors(changed), name) | ||
| changed = copy.deepcopy(self.workflow) | ||
| changed["permissions"] = {"contents": "write"} | ||
| self.assertTrue(token_errors(changed)) | ||
|
|
||
| def test_legacy_release_token_negative_control(self): | ||
| for name, job in self.workflow["jobs"].items(): | ||
| for index, step in enumerate(job.get("steps", [])): | ||
| if not step.get("run", "").startswith("goreleaser release "): | ||
| continue | ||
| changed = copy.deepcopy(self.workflow) | ||
| changed["jobs"][name]["steps"][index]["env"]["GITHUB_TOKEN"] = "${{ secrets.RELENG_GITHUB_TOKEN }}" | ||
| self.assertTrue(token_errors(changed), name) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| unittest.main() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Suggestion (medium confidence): If a caller grants only
contents: read, for example through a read-only org default, GitHub rejects the whole called workflow at startup. The error is "nested job is requesting 'contents: write'", so this is not limited to the binaries job, and it affects every caller that tracks this ref as soon as it merges. The PR body covers this, but the guard is only prose. Consider merging only after the caller-template PR lands, or tagging this as a breaking release so callers that pin a ref opt in.