Require explicit scopes for GitHub App tokens at compile time - #64976
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
|
✅ Ponytail Reviewer completed successfully! Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
hello
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 58.4 AIC · ⌖ 7.2 AIC · ⊞ 19.2K
Comment /review to run again
| continue | ||
| } | ||
| level, ok := value.(string) | ||
| if ok && (level == "read" || level == "write") { |
There was a problem hiding this comment.
This appears to be a placeholder comment and has no actionable finding; no code change was needed.
| continue | ||
| } | ||
| level, ok := value.(string) | ||
| if ok && (level == "read" || level == "write") { |
There was a problem hiding this comment.
This only counts read/write permission inputs as scoped, so permission inputs set to none are wrongly treated as unscoped and can still fail strict mode.
There was a problem hiding this comment.
The validator now recognizes none as an explicit permission level, with regression coverage. Updated in 6ac9bf6.
🏗️ ADR required — draft added for PR #64976I added a draft ADR at Evidence used
Next actionPlease review the draft ADR, confirm the decision wording and trade-offs, and update it if needed before merge.
|
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The guard is bypassable by case variants, mishandles permission inputs, and includes undocumented generated changes.
Review effort: Balanced
Findings: 2
Open (5)
What changed in this PR
Adds compile-time enforcement for explicitly scoped GitHub App tokens.
Changes:
- Validates compiled token-creation steps, warning or failing by strict mode.
- Adds tests and documentation.
- Scopes Squad bootstrap tokens and refreshes generated workflow artifacts.
| File | Description |
|---|---|
pkg/workflow/compiler.go |
Invokes compiled-token validation. |
pkg/workflow/app_token_permissions_validation.go |
Implements permission checks. |
pkg/workflow/app_token_permissions_validation_test.go |
Adds regression tests. |
docs/src/content/docs/reference/steps-jobs.md |
Documents explicit scopes. |
.github/workflows/shared/squad.md |
Restricts bootstrap token to contents read. |
.github/workflows/squad.lock.yml |
Regenerates scoped Squad workflow. |
.github/workflows/squad-plan.lock.yml |
Regenerates scoped planning workflow. |
.github/workflows/squad-implement-worker.lock.yml |
Regenerates scoped worker workflow. |
.github/workflows/squad-game-planner.lock.yml |
Regenerates scoped game-planner workflow. |
.github/workflows/visual-regression-checker.lock.yml |
Refreshes unrelated firewall output. |
.github/workflows/spec-enforcer.lock.yml |
Refreshes coverage action pin. |
.github/workflows/purelock.lock.yml |
Refreshes coverage action pin. |
.github/aw/actions-lock.json |
Updates cached action versions. |
| if !ok { | ||
| continue | ||
| } | ||
| if !strings.HasPrefix(uses, "actions/create-github-app-token@") { |
There was a problem hiding this comment.
The validator now normalizes the action reference and permission-input key before matching, with case-variant coverage. Updated in 6ac9bf6.
| // fast-path checks avoid an unnecessary yaml.Unmarshal. | ||
| needsSchemaCheck := !c.skipValidation | ||
| needsWorkflowParse := needsSchemaCheck || requireSelfHostedRunners | ||
| needsAppTokenCheck := strings.Contains(yamlContent, "actions/create-github-app-token@") |
There was a problem hiding this comment.
The compiler prefilter now normalizes the compiled YAML before detection, and the validator matches the action reference case-insensitively. Covered by the case-variant compile test in 6ac9bf6.
| - name: Upload code coverage report | ||
| id: upload_code_coverage | ||
| uses: actions/upload-code-coverage@bfa741d815a28cb064a8e3a0837e577457a017d5 # v1.4.3 | ||
| uses: actions/upload-code-coverage@2b21a77928be8d5168c2b9581a67f2adbebacc52 # v1.4.4 |
There was a problem hiding this comment.
The current PR diff does not modify purelock.lock.yml; the unrelated coverage-action drift is absent. No lock change was included in 6ac9bf6.
| - name: Upload code coverage report | ||
| id: upload_code_coverage | ||
| uses: actions/upload-code-coverage@bfa741d815a28cb064a8e3a0837e577457a017d5 # v1.4.3 | ||
| uses: actions/upload-code-coverage@2b21a77928be8d5168c2b9581a67f2adbebacc52 # v1.4.4 |
There was a problem hiding this comment.
The current PR diff does not modify spec-enforcer.lock.yml; the unrelated coverage-action drift is absent. No lock change was included in 6ac9bf6.
| GH_AW_MAX_AI_CREDITS="1000" | ||
| fi | ||
| printf '%s\n' "{\"\$schema\":\"https://github.com/github/gh-aw-firewall/releases/download/v0.28.31/awf-config.schema.json\",\"network\":{\"allowDomains\":[\"*.grafana.net\",\"*.sentry.io\",\"127.0.0.1\",\"::1\",\"api.npms.io\",\"api.snapcraft.io\",\"archive.ubuntu.com\",\"azure.archive.ubuntu.com\",\"bun.sh\",\"cdn.jsdelivr.net\",\"cdn.playwright.dev\",\"crl.geotrust.com\",\"crl.globalsign.com\",\"crl.identrust.com\",\"crl.sectigo.com\",\"crl.thawte.com\",\"crl.usertrust.com\",\"crl.verisign.com\",\"crl3.digicert.com\",\"crl4.digicert.com\",\"crls.ssl.com\",\"deb.nodesource.com\",\"deno.land\",\"esm.sh\",\"get.pnpm.io\",\"googleapis.deno.dev\",\"googlechromelabs.github.io\",\"json-schema.org\",\"json.schemastore.org\",\"jsr.io\",\"keyserver.ubuntu.com\",\"localhost\",\"nodejs.org\",\"npm.pkg.github.com\",\"npmjs.com\",\"npmjs.org\",\"ocsp.digicert.com\",\"ocsp.geotrust.com\",\"ocsp.globalsign.com\",\"ocsp.identrust.com\",\"ocsp.sectigo.com\",\"ocsp.ssl.com\",\"ocsp.thawte.com\",\"ocsp.usertrust.com\",\"ocsp.verisign.com\",\"packagecloud.io\",\"packages.cloud.google.com\",\"packages.microsoft.com\",\"playwright.download.prss.microsoft.com\",\"ppa.launchpad.net\",\"registry.bower.io\",\"registry.npmjs.com\",\"registry.npmjs.org\",\"registry.yarnpkg.com\",\"repo.yarnpkg.com\",\"s.symcb.com\",\"s.symcd.com\",\"security.ubuntu.com\",\"skimdb.npmjs.com\",\"storage.googleapis.com\",\"telemetry.vercel.com\",\"ts-crl.ws.symantec.com\",\"ts-ocsp.ws.symantec.com\",\"www.googleapis.com\",\"www.npmjs.com\",\"www.npmjs.org\",\"yarnpkg.com\"],\"isolation\":true,\"topologyAttach\":[\"awmg-mcpg\"]},\"apiProxy\":{\"enabled\":true,\"enableTokenSteering\":true,\"maxRuns\":500,\"maxAiCredits\":${GH_AW_MAX_AI_CREDITS},\"maxCacheMisses\":5,\"models\":{\"agent\":[\"sonnet-6x\",\"gpt-6\",\"gpt-5.4\",\"gpt-5.5\",\"gpt-5.6\",\"gpt-5.3\",\"gemini-pro\",\"any\"],\"antigravity\":[\"copilot/antigravity*\",\"google/antigravity*\",\"gemini/antigravity*\"],\"any\":[\"copilot/*\",\"anthropic/*\",\"openai/*\",\"google/*\",\"gemini/*\"],\"auto\":[\"copilot/auto\",\"large\"],\"claude\":[\"agent\"],\"codex\":[\"agent\"],\"coding\":[\"copilot/gpt-5*codex*\",\"openai/gpt-5*codex*\",\"gpt-5-codex\",\"kimi\"],\"computer-use\":[\"copilot/*computer-use*\",\"google/*computer-use*\",\"gemini/*computer-use*\",\"openai/*computer-use*\"],\"copilot\":[\"agent\"],\"deep-research\":[\"copilot/deep-research*\",\"copilot/o3-deep-research*\",\"copilot/o4-mini-deep-research*\",\"google/deep-research*\",\"gemini/deep-research*\",\"openai/o3-deep-research*\",\"openai/o4-mini-deep-research*\"],\"detection\":[\"small\"],\"evals\":[\"small\"],\"fable\":[\"copilot/*fable*\",\"anthropic/*fable*\"],\"gemini\":[\"agent\"],\"gemini-3-flash\":[\"copilot/gemini-3*flash*\",\"google/gemini-3*flash*\",\"gemini/gemini-3*flash*\"],\"gemini-3-pro\":[\"copilot/gemini-3*pro*\",\"google/gemini-3*pro*\",\"google/nano-banana*\",\"gemini/gemini-3*pro*\"],\"gemini-3.1-flash\":[\"copilot/gemini-3.1*flash*\",\"google/gemini-3.1*flash*\",\"gemini/gemini-3.1*flash*\"],\"gemini-3.1-pro\":[\"copilot/gemini-3.1*pro*\",\"google/gemini-3.1*pro*\",\"gemini/gemini-3.1*pro*\"],\"gemini-3.5-flash\":[\"copilot/gemini-3.5*flash*\",\"google/gemini-3.5*flash*\",\"gemini/gemini-3.5*flash*\"],\"gemini-3.6-flash\":[\"copilot/gemini-3.6*flash*\",\"google/gemini-3.6*flash*\",\"gemini/gemini-3.6*flash*\"],\"gemini-3.7-flash\":[\"copilot/gemini-3.7*flash*\",\"google/gemini-3.7*flash*\",\"gemini/gemini-3.7*flash*\"],\"gemini-3.8-flash\":[\"copilot/gemini-3.8*flash*\",\"google/gemini-3.8*flash*\",\"gemini/gemini-3.8*flash*\"],\"gemini-flash\":[\"copilot/gemini-*flash*\",\"google/gemini-*flash*\",\"gemini/gemini-*flash*\"],\"gemini-flash-lite\":[\"copilot/gemini-*flash*lite*\",\"google/gemini-*flash*lite*\",\"gemini/gemini-*flash*lite*\"],\"gemini-omni\":[\"copilot/gemini-omni*\",\"google/gemini-omni*\",\"gemini/gemini-omni*\"],\"gemini-pro\":[\"copilot/gemini-*pro*\",\"google/gemini-*pro*\",\"gemini/gemini-*pro*\"],\"gemma\":[\"copilot/gemma*\",\"google/gemma*\",\"gemini/gemma*\"],\"gpt-5\":[\"copilot/gpt-5*\",\"openai/gpt-5*\"],\"gpt-5-codex\":[\"copilot/gpt-5*codex*\",\"openai/gpt-5*codex*\"],\"gpt-5-mini\":[\"copilot/gpt-5*mini*\",\"openai/gpt-5*mini*\"],\"gpt-5-nano\":[\"copilot/gpt-5*nano*\",\"openai/gpt-5*nano*\"],\"gpt-5-pro\":[\"copilot/gpt-5*pro*\",\"openai/gpt-5*pro*\"],\"gpt-5.1\":[\"copilot/gpt-5.1*\",\"openai/gpt-5.1*\"],\"gpt-5.2\":[\"copilot/gpt-5.2*\",\"openai/gpt-5.2*\"],\"gpt-5.3\":[\"copilot/gpt-5.3*\",\"openai/gpt-5.3*\"],\"gpt-5.4\":[\"copilot/gpt-5.4*\",\"openai/gpt-5.4*\"],\"gpt-5.5\":[\"copilot/gpt-5.5*\",\"openai/gpt-5.5*\"],\"gpt-5.6\":[\"copilot/gpt-5.6*\",\"openai/gpt-5.6*\"],\"gpt-6\":[\"copilot/gpt-6*\",\"openai/gpt-6*\"],\"grok\":[\"copilot/*grok*\",\"openai/*grok*\"],\"haiku\":[\"copilot/*haiku*\",\"anthropic/*haiku*\"],\"image-generation\":[\"copilot/gpt-image*\",\"openai/gpt-image*\",\"openai/chatgpt-image*\",\"copilot/gemini-*image*\",\"google/gemini-*image*\",\"gemini/gemini-*image*\",\"google/imagen*\"],\"kimi\":[\"copilot/kimi*\",\"openai/kimi*\"],\"kiwi\":[\"copilot/kiwi*\",\"openai/kiwi*\"],\"large\":[\"sonnet\",\"gpt-6\",\"gpt-5-pro\",\"gpt-5\",\"gemini-pro\"],\"lyria\":[\"google/lyria*\",\"gemini/lyria*\",\"copilot/lyria*\"],\"mai-code\":[\"copilot/MAI-Code*\",\"copilot/mai-code*\",\"openai/MAI-Code*\"],\"mai-code-1-flash-picker\":[\"copilot/MAI-Code-1-Flash-picker*\",\"copilot/mai-code-1-flash-picker*\",\"openai/MAI-Code-1-Flash-picker*\"],\"mini\":[\"haiku\",\"gpt-5-mini\",\"gpt-5-nano\",\"gemini-flash-lite\"],\"nano-banana\":[\"copilot/nano-banana*\",\"google/nano-banana*\",\"gemini/nano-banana*\"],\"opus\":[\"copilot/*opus*\",\"anthropic/*opus*\"],\"opusplan\":[\"opus?effort=high\"],\"raptor-mini\":[\"copilot/raptor*\",\"openai/raptor*\"],\"reasoning\":[\"copilot/o1*\",\"copilot/o3*\",\"copilot/o4*\",\"openai/o1*\",\"openai/o3*\",\"openai/o4*\"],\"robotics\":[\"copilot/*robotics*\",\"google/*robotics*\",\"gemini/*robotics*\"],\"small\":[\"mini\"],\"small-agent\":[\"haiku\",\"gpt-5-mini\",\"gemini-flash\"],\"sonnet\":[\"copilot/*sonnet*\",\"anthropic/*sonnet*\"],\"sonnet-6x\":[\"copilot/*sonnet-4.5*\",\"copilot/*sonnet-4.6*\",\"copilot/*sonnet-5*\",\"copilot/*sonnet-4-5-*\",\"anthropic/*sonnet-4-5-*\",\"copilot/*sonnet-4-6*\",\"anthropic/*sonnet-4-6*\",\"anthropic/*sonnet-5*\"],\"summarization\":[\"haiku\",\"gpt-5-mini\",\"gemini-flash-lite\",\"mini\"],\"veo\":[\"google/veo*\",\"gemini/veo*\"],\"vision\":[\"copilot/gemini-*image*\",\"google/gemini-*image*\",\"gemini/gemini-*image*\",\"copilot/gemini-*flash*\",\"google/gemini-*flash*\",\"gemini/gemini-*flash*\"]}},\"container\":{\"imageTag\":\"0.28.31,squid=sha256:90a46d2e6e910ace2c09c2dcf6b56dc374ed513d9da642a4e9fcfcd6e9248a9d,agent=sha256:c4ab1d48d533cc7daaa5f2e1193d1a644888242fb8813d49c6de46d129224b76,api-proxy=sha256:a5a37489635109334a5e2b5cb2eaba32e8c8f1a89d8dea6009baa962a8904c4a,cli-proxy=sha256:505df66052e6688cf419ad248a511d17093475bd66a958618078d3c9cdf95083\"},\"logging\":{\"proxyLogsDir\":\"/tmp/gh-aw/sandbox/firewall/logs\",\"auditDir\":\"/tmp/gh-aw/sandbox/firewall/audit\"}}" > "${RUNNER_TEMP}/gh-aw/awf-config.json" | ||
| printf '%s\n' "{\"\$schema\":\"https://github.com/github/gh-aw-firewall/releases/download/v0.28.31/awf-config.schema.json\",\"network\":{\"allowDomains\":[\"*.grafana.net\",\"*.sentry.io\",\"127.0.0.1\",\"::1\",\"api.npms.io\",\"api.snapcraft.io\",\"archive.ubuntu.com\",\"azure.archive.ubuntu.com\",\"bun.sh\",\"cdn.jsdelivr.net\",\"cdn.playwright.dev\",\"crl.geotrust.com\",\"crl.globalsign.com\",\"crl.identrust.com\",\"crl.sectigo.com\",\"crl.thawte.com\",\"crl.usertrust.com\",\"crl.verisign.com\",\"crl3.digicert.com\",\"crl4.digicert.com\",\"crls.ssl.com\",\"deb.nodesource.com\",\"deno.land\",\"esm.sh\",\"get.pnpm.io\",\"googleapis.deno.dev\",\"googlechromelabs.github.io\",\"host.docker.internal\",\"json-schema.org\",\"json.schemastore.org\",\"jsr.io\",\"keyserver.ubuntu.com\",\"localhost\",\"nodejs.org\",\"npm.pkg.github.com\",\"npmjs.com\",\"npmjs.org\",\"ocsp.digicert.com\",\"ocsp.geotrust.com\",\"ocsp.globalsign.com\",\"ocsp.identrust.com\",\"ocsp.sectigo.com\",\"ocsp.ssl.com\",\"ocsp.thawte.com\",\"ocsp.usertrust.com\",\"ocsp.verisign.com\",\"packagecloud.io\",\"packages.cloud.google.com\",\"packages.microsoft.com\",\"playwright.download.prss.microsoft.com\",\"ppa.launchpad.net\",\"registry.bower.io\",\"registry.npmjs.com\",\"registry.npmjs.org\",\"registry.yarnpkg.com\",\"repo.yarnpkg.com\",\"s.symcb.com\",\"s.symcd.com\",\"security.ubuntu.com\",\"skimdb.npmjs.com\",\"storage.googleapis.com\",\"telemetry.vercel.com\",\"ts-crl.ws.symantec.com\",\"ts-ocsp.ws.symantec.com\",\"www.googleapis.com\",\"www.npmjs.com\",\"www.npmjs.org\",\"yarnpkg.com\"],\"isolation\":true,\"topologyAttach\":[\"awmg-mcpg\"]},\"apiProxy\":{\"enabled\":true,\"enableTokenSteering\":true,\"maxRuns\":500,\"maxAiCredits\":${GH_AW_MAX_AI_CREDITS},\"maxCacheMisses\":5,\"models\":{\"agent\":[\"sonnet-6x\",\"gpt-6\",\"gpt-5.4\",\"gpt-5.5\",\"gpt-5.6\",\"gpt-5.3\",\"gemini-pro\",\"any\"],\"antigravity\":[\"copilot/antigravity*\",\"google/antigravity*\",\"gemini/antigravity*\"],\"any\":[\"copilot/*\",\"anthropic/*\",\"openai/*\",\"google/*\",\"gemini/*\"],\"auto\":[\"copilot/auto\",\"large\"],\"claude\":[\"agent\"],\"codex\":[\"agent\"],\"coding\":[\"copilot/gpt-5*codex*\",\"openai/gpt-5*codex*\",\"gpt-5-codex\",\"kimi\"],\"computer-use\":[\"copilot/*computer-use*\",\"google/*computer-use*\",\"gemini/*computer-use*\",\"openai/*computer-use*\"],\"copilot\":[\"agent\"],\"deep-research\":[\"copilot/deep-research*\",\"copilot/o3-deep-research*\",\"copilot/o4-mini-deep-research*\",\"google/deep-research*\",\"gemini/deep-research*\",\"openai/o3-deep-research*\",\"openai/o4-mini-deep-research*\"],\"detection\":[\"small\"],\"evals\":[\"small\"],\"fable\":[\"copilot/*fable*\",\"anthropic/*fable*\"],\"gemini\":[\"agent\"],\"gemini-3-flash\":[\"copilot/gemini-3*flash*\",\"google/gemini-3*flash*\",\"gemini/gemini-3*flash*\"],\"gemini-3-pro\":[\"copilot/gemini-3*pro*\",\"google/gemini-3*pro*\",\"google/nano-banana*\",\"gemini/gemini-3*pro*\"],\"gemini-3.1-flash\":[\"copilot/gemini-3.1*flash*\",\"google/gemini-3.1*flash*\",\"gemini/gemini-3.1*flash*\"],\"gemini-3.1-pro\":[\"copilot/gemini-3.1*pro*\",\"google/gemini-3.1*pro*\",\"gemini/gemini-3.1*pro*\"],\"gemini-3.5-flash\":[\"copilot/gemini-3.5*flash*\",\"google/gemini-3.5*flash*\",\"gemini/gemini-3.5*flash*\"],\"gemini-3.6-flash\":[\"copilot/gemini-3.6*flash*\",\"google/gemini-3.6*flash*\",\"gemini/gemini-3.6*flash*\"],\"gemini-3.7-flash\":[\"copilot/gemini-3.7*flash*\",\"google/gemini-3.7*flash*\",\"gemini/gemini-3.7*flash*\"],\"gemini-3.8-flash\":[\"copilot/gemini-3.8*flash*\",\"google/gemini-3.8*flash*\",\"gemini/gemini-3.8*flash*\"],\"gemini-flash\":[\"copilot/gemini-*flash*\",\"google/gemini-*flash*\",\"gemini/gemini-*flash*\"],\"gemini-flash-lite\":[\"copilot/gemini-*flash*lite*\",\"google/gemini-*flash*lite*\",\"gemini/gemini-*flash*lite*\"],\"gemini-omni\":[\"copilot/gemini-omni*\",\"google/gemini-omni*\",\"gemini/gemini-omni*\"],\"gemini-pro\":[\"copilot/gemini-*pro*\",\"google/gemini-*pro*\",\"gemini/gemini-*pro*\"],\"gemma\":[\"copilot/gemma*\",\"google/gemma*\",\"gemini/gemma*\"],\"gpt-5\":[\"copilot/gpt-5*\",\"openai/gpt-5*\"],\"gpt-5-codex\":[\"copilot/gpt-5*codex*\",\"openai/gpt-5*codex*\"],\"gpt-5-mini\":[\"copilot/gpt-5*mini*\",\"openai/gpt-5*mini*\"],\"gpt-5-nano\":[\"copilot/gpt-5*nano*\",\"openai/gpt-5*nano*\"],\"gpt-5-pro\":[\"copilot/gpt-5*pro*\",\"openai/gpt-5*pro*\"],\"gpt-5.1\":[\"copilot/gpt-5.1*\",\"openai/gpt-5.1*\"],\"gpt-5.2\":[\"copilot/gpt-5.2*\",\"openai/gpt-5.2*\"],\"gpt-5.3\":[\"copilot/gpt-5.3*\",\"openai/gpt-5.3*\"],\"gpt-5.4\":[\"copilot/gpt-5.4*\",\"openai/gpt-5.4*\"],\"gpt-5.5\":[\"copilot/gpt-5.5*\",\"openai/gpt-5.5*\"],\"gpt-5.6\":[\"copilot/gpt-5.6*\",\"openai/gpt-5.6*\"],\"gpt-6\":[\"copilot/gpt-6*\",\"openai/gpt-6*\"],\"grok\":[\"copilot/*grok*\",\"openai/*grok*\"],\"haiku\":[\"copilot/*haiku*\",\"anthropic/*haiku*\"],\"image-generation\":[\"copilot/gpt-image*\",\"openai/gpt-image*\",\"openai/chatgpt-image*\",\"copilot/gemini-*image*\",\"google/gemini-*image*\",\"gemini/gemini-*image*\",\"google/imagen*\"],\"kimi\":[\"copilot/kimi*\",\"openai/kimi*\"],\"kiwi\":[\"copilot/kiwi*\",\"openai/kiwi*\"],\"large\":[\"sonnet\",\"gpt-6\",\"gpt-5-pro\",\"gpt-5\",\"gemini-pro\"],\"lyria\":[\"google/lyria*\",\"gemini/lyria*\",\"copilot/lyria*\"],\"mai-code\":[\"copilot/MAI-Code*\",\"copilot/mai-code*\",\"openai/MAI-Code*\"],\"mai-code-1-flash-picker\":[\"copilot/MAI-Code-1-Flash-picker*\",\"copilot/mai-code-1-flash-picker*\",\"openai/MAI-Code-1-Flash-picker*\"],\"mini\":[\"haiku\",\"gpt-5-mini\",\"gpt-5-nano\",\"gemini-flash-lite\"],\"nano-banana\":[\"copilot/nano-banana*\",\"google/nano-banana*\",\"gemini/nano-banana*\"],\"opus\":[\"copilot/*opus*\",\"anthropic/*opus*\"],\"opusplan\":[\"opus?effort=high\"],\"raptor-mini\":[\"copilot/raptor*\",\"openai/raptor*\"],\"reasoning\":[\"copilot/o1*\",\"copilot/o3*\",\"copilot/o4*\",\"openai/o1*\",\"openai/o3*\",\"openai/o4*\"],\"robotics\":[\"copilot/*robotics*\",\"google/*robotics*\",\"gemini/*robotics*\"],\"small\":[\"mini\"],\"small-agent\":[\"haiku\",\"gpt-5-mini\",\"gemini-flash\"],\"sonnet\":[\"copilot/*sonnet*\",\"anthropic/*sonnet*\"],\"sonnet-6x\":[\"copilot/*sonnet-4.5*\",\"copilot/*sonnet-4.6*\",\"copilot/*sonnet-5*\",\"copilot/*sonnet-4-5-*\",\"anthropic/*sonnet-4-5-*\",\"copilot/*sonnet-4-6*\",\"anthropic/*sonnet-4-6*\",\"anthropic/*sonnet-5*\"],\"summarization\":[\"haiku\",\"gpt-5-mini\",\"gemini-flash-lite\",\"mini\"],\"veo\":[\"google/veo*\",\"gemini/veo*\"],\"vision\":[\"copilot/gemini-*image*\",\"google/gemini-*image*\",\"gemini/gemini-*image*\",\"copilot/gemini-*flash*\",\"google/gemini-*flash*\",\"gemini/gemini-*flash*\"]}},\"container\":{\"imageTag\":\"0.28.31,squid=sha256:90a46d2e6e910ace2c09c2dcf6b56dc374ed513d9da642a4e9fcfcd6e9248a9d,agent=sha256:c4ab1d48d533cc7daaa5f2e1193d1a644888242fb8813d49c6de46d129224b76,api-proxy=sha256:a5a37489635109334a5e2b5cb2eaba32e8c8f1a89d8dea6009baa962a8904c4a,cli-proxy=sha256:505df66052e6688cf419ad248a511d17093475bd66a958618078d3c9cdf95083\"},\"logging\":{\"proxyLogsDir\":\"/tmp/gh-aw/sandbox/firewall/logs\",\"auditDir\":\"/tmp/gh-aw/sandbox/firewall/audit\"}}" > "${RUNNER_TEMP}/gh-aw/awf-config.json" |
There was a problem hiding this comment.
The current PR diff does not modify visual-regression-checker.lock.yml; the unrelated firewall allowlist drift is absent. No lock change was included in 6ac9bf6.
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /tdd — requesting changes: the new strict-mode validator has an unhandled false-positive path.
📋 Key Themes & Highlights
Key Themes
- Real regression, not edge case:
buildPreActivationAppTokenMintStep(used whengithub-appis combined withskip-if-match/skip-if-no-match) never emitspermission-*inputs, unlike the other two app-token generators (buildActivationAppTokenMintStep,buildDailyAICAppTokenMintStep) which both callconvertPermissionsToAppTokenFields. Verified by compiling a minimal repro workflow in this repo (strict mode is enforced here viaaw.json): compilation fails with the exact error this PR introduces, for a workflow author who did nothing wrong. - Test gap: the new test file covers the warning/strict toggle and a synthetic unscoped job, but never exercises the
github-app+skip-if-matchcombination that triggers the gap above. A regression test here would have caught it. - Scope of the fix is good: checking imported/generated steps (not just user-authored ones) is the right call given the four Squad lock files this PR already had to patch — it shows the detection genuinely works on compiler-emitted steps. The gap is that one more compiler-emitted generator wasn't updated to match.
Positive Highlights
- ✅ The compiler now validates on the compiled YAML rather than the markdown source, catching both imported and generated steps — this is the correct approach and matches the PR description's stated goal.
- ✅ Docs update in
steps-jobs.mdis clear and actionable, with a concrete code example. - ✅
needsAppTokenCheckshort-circuits the YAML parse/validation path when nocreate-github-app-tokenusage exists, avoiding unnecessary work on unrelated workflows.
Unrelated note: .github/aw/actions-lock.json also downgrades github/stale-repos and super-linter/super-linter pins — worth confirming that's intentional and not an artifact of a stale rebase/merge.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 117 AIC · ⌖ 15.8 AIC · ⊞ 10K
Comment /matt to run again
| } | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
[/diagnosing-bugs] This new strict-mode check has an untested blind spot: buildPreActivationAppTokenMintStep (pkg/workflow/compiler_pre_activation_job.go) mints an actions/create-github-app-token step for skip-if checks but never emits any permission-* input. Any workflow combining github-app with skip-if-match/skip-if-no-match will now fail compilation in strict mode with this exact validator's error, even though the author did nothing wrong.
💡 Repro
---
on:
workflow_dispatch:
skip-if-match: "is:issue is:open label:test"
github-app:
app-id: ${{ vars.APP_ID }}
private-key: ${{ secrets.APP_KEY }}
---Compiling this in this repo (strict mode is enforced via aw.json) fails with:
strict mode: actions/create-github-app-token in job "pre_activation" (Generate GitHub App token for skip-if checks) has no explicit permission-* inputs
buildDailyAICAppTokenMintStep and buildActivationAppTokenMintStep both call convertPermissionsToAppTokenFields to emit scoped permission-* fields, but buildPreActivationAppTokenMintStep doesn't — it's the one generator this PR's validator doesn't account for. Either (a) add a minimal permission-* scope (e.g. the scopes needed for the skip-if search query, likely permission-issues/permission-pull-requests: read) to that step, or (b) add a regression test asserting the compiler still succeeds for a github-app + skip-if-match workflow in strict mode, since none of the current .github/workflows/*.lock.yml files exercise this combination today.
@copilot please address this.
There was a problem hiding this comment.
The pre-activation skip-if token now receives issues: read and pull-requests: read scopes. A strict-mode compile regression covers github-app with skip-if-match. Updated in 6ac9bf6.
| map[string]any{"uses": "actions/create-github-app-token@sha", "with": map[string]any{"private-key": "secret"}}, | ||
| }}, | ||
| }} | ||
| if err := compiler.validateAppTokenPermissions(workflow, true); err == nil || !strings.Contains(err.Error(), `job "custom"`) { |
There was a problem hiding this comment.
[/tdd] Good coverage of the warning/strict toggle and empty-string edge case, but the test suite only exercises the on.github-app + arbitrary job path and the synthetic validateAppTokenPermissions custom-job case. It never compiles a real github-app + skip-if-match/skip-if-no-match workflow, which is exactly the combination that currently breaks (see the sibling comment on app_token_permissions_validation.go). Adding that case here would have caught the regression before merge.
@copilot please address this.
There was a problem hiding this comment.
Added a regression test that compiles a strict-mode github-app plus skip-if-match workflow and checks the generated read scopes. Updated in 6ac9bf6.
There was a problem hiding this comment.
Summary
This PR adds a compile-time guard that rejects (strict mode) or warns about any actions/create-github-app-token step lacking an explicit permission-* input. The intent — avoiding fully-scoped installation tokens — is good, but the guard is applied globally to all compiled jobs, including several compiler-generated token-mint steps that currently never set permission-*:
buildPreActivationAppTokenMintStep(skip-if checks viaon.github-app)generateOTLPOIDCMintStep(observability.otlp.github-app)sideRepoAppTokenMintStepYAML(cross-repo maintenance checkout)
I verified by compiling a minimal workflow with strict: true and observability.otlp.github-app configured — it now fails compilation with no documented way to add a permission-* field for that schema. This is a correctness regression for any workflow using these existing, legitimate features in strict mode (the default), not just the four Squad workflows this PR explicitly fixed.
Test coverage also doesn't exercise these compiler-generated paths — only a hand-authored step in a synthetic workflow.
Details and suggested fixes are in the inline comments.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · copilot · sonnet50 · 256.2 AIC · ⌖ 13.2 AIC · ⊞ 8.1K
| } | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
This new strict-mode gate scans every compiled job for uses: actions/create-github-app-token@... and requires a permission-* field, but several existing compiler-generated token-mint call sites never emit one — so this PR will break those workflows the moment strict mode (the default) is on:
buildPreActivationAppTokenMintStep(compiler_pre_activation_job.go:776-815, used for skip-if checks) never appends anypermission-*field.generateOTLPOIDCMintStep→buildGitHubAppTokenMintStepWithMeta(app, nil, ...)(compiler_yaml_step_generation.go:150) passespermissions=nilforobservability.otlp.github-app.sideRepoAppTokenMintStepYAML(side_repo_maintenance.go:143-152) explicitly passesnil"no additional permission scoping".
I confirmed this by compiling a minimal workflow with strict: true + observability.otlp.github-app:
error: strict mode: actions/create-github-app-token in job "safe_outputs" (Mint OTLP GitHub App token) has no explicit permission-* inputs
This will fail compilation for any existing workflow using OTLP GitHub App auth, skip-if checks with on.github-app, or side-repo maintenance checkout, with no way to set a permission-* input (none of those schemas expose one). Please either:
- Skip/allowlist these compiler-generated steps (they're already scoped by the App's installation, not by job permissions), or
- Plumb a permission-scoping mechanism through to these call sites before enabling the check in strict mode.
@copilot please address this.
There was a problem hiding this comment.
Generated tokens now receive explicit scopes: skip-if gets issue/PR read, nil generic tokens default to contents read, head tokens get contents/PR write, and side-repo maintenance gets its required permission union. Regression tests cover these paths. Updated in 6ac9bf6.
| if err := compiler.validateAppTokenPermissions(workflow, true); err == nil || !strings.Contains(err.Error(), `job "custom"`) { | ||
| t.Fatalf("expected custom job permission error, got %v", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
The new regression test only covers a synthetic workflow with a manually-authored create-github-app-token step. It doesn't exercise any of the compiler-generated token-mint paths (pre-activation skip-if token, OTLP App auth, side-repo maintenance, plugin installs) that this change actually affects in production workflows — several of which, as noted in the other comment, don't set permission-* and will now fail strict-mode compilation. Consider adding a test that compiles a workflow using e.g. on.github-app + skip-if-match, or observability.otlp.github-app, to catch this class of regression.
@copilot please address this.
There was a problem hiding this comment.
Added tests for generated pre-activation, default/app-configured, OTLP, and side-repository token scopes; the existing workflow package tests also pass. Updated in 6ac9bf6.
|
@copilot Quick triage nudge: branch refresh was attempted by sous-chef. Please review the current state of this PR and run the
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Ran the requested PR-finisher pass and pushed review fixes in |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
The compiler now requires a non-empty |
|
@copilot ensure Is a best secure practices |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Completed the PR-finisher pass and pushed |
|
@copilot fix compilation errors in agentic workflows (squad) |
|
@copilot This PR still needs forward progress. Please address the outstanding blocking review, rerun any needed checks, refresh the branch if appropriate, and finish with the
|
I couldn't reproduce a Squad compilation error at current HEAD ( |
…oken-permissions Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Merged current |
|
🎉 This pull request is included in a new release. Release: |
PR #64976 added compile-time validation requiring manually-authored actions/create-github-app-token steps to declare explicit repositories and permission-* inputs (error in strict mode, warning otherwise). Declarative github-app: blocks already compute these automatically and are unaffected; syntax-core.md didn't mention the new custom-step requirement. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Four Squad workflows mint GitHub App tokens without permission inputs, leaving each token with the App installation’s full scope.
permission-*inputs fail compilation in strict mode and warn otherwise.permission-contents: readand regenerate the four affected lock files.