Add step shell script validator: gh CLI without GH_TOKEN heuristic#44661
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot run pr-finisher skill |
There was a problem hiding this comment.
Pull request overview
Adds a new compile-time heuristic validator to detect run: scripts invoking the gh CLI without GH_TOKEN set in env: (error in strict mode, warning otherwise), wires it into the compiler’s validation chain, and updates the craft workflow to satisfy the new rule.
Changes:
- Introduce
validateStepShellScripts/step_shell_validator.goto detectghusage in steprun:scripts and requireGH_TOKEN. - Add unit tests covering strict vs non-strict behavior and step section coverage.
- Update
.github/workflows/craft.md(and regenerated lock) to includeGH_TOKENfor theghextension install step.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/step_shell_validator.go | New heuristic validator for gh usage without GH_TOKEN in step env:. |
| pkg/workflow/step_shell_validator_test.go | Unit tests for strict/non-strict behavior and section scanning. |
| pkg/workflow/compiler_orchestrator_engine.go | Hooks the new validator into the existing validation chain. |
| .github/workflows/craft.md | Adds GH_TOKEN to a step that runs gh. |
| .github/workflows/craft.lock.yml | Regenerated compiled workflow reflecting the craft.md change. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 5/5 changed files
- Comments generated: 3
- Review effort level: Low
| var violations []string | ||
| for _, step := range steps { | ||
| if v := checkStepGHToken(step); v != "" { | ||
| violations = append(violations, v) | ||
| } | ||
| } |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed the finisher pass in bedc162: fixed the validator review feedback, added the missing regression coverage, and reran local validation ( |
|
@copilot please run the Unresolved review threads:
|
…piler-validator # Conflicts: # .github/workflows/craft.lock.yml Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
🤖 PR Triage
Score breakdown: impact 38 + urgency 22 + quality 12 Rationale: Adds compile-time validator catching gh CLI usage without GH_TOKEN, preventing runtime auth failures. Touches core compiler orchestrator and new validator. High value — catches a real class of silent failures. Recommend expedited human review.
|
|
@copilot validate container shas |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed in |
|
Run: https://github.com/github/gh-aw/actions/runs/29079112957
|
…update-go-compiler-validator Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Addressed on |
|
Hey The PR is well-structured: the validator is cleanly wired into the orchestrator alongside
|
|
@copilot please run the
|
Checked the branch on |
|
@copilot please run the
|
|
🎉 This pull request is included in a new release. Release: |
Steps using the
ghCLI withoutGH_TOKENinenv:fail at runtime with opaque auth errors. This adds a compile-time heuristic to catch it early.New validator:
step_shell_validator.gopre-steps,steps,pre-agent-steps,post-stepsforrun:scripts invokingghghas a command token at line-start or after&&/||/;/|$(gh ...)substitutions are not detectedExample violation caught:
Fix suggested in the error message:
Wiring
validateStepShellScriptsadded to the validation chain incompiler_orchestrator_engine.go, alongsidevalidateEnvSecretsandvalidateStepsSecrets.Incidental fix
craft.md"Install gh-aw extension" step was genuinely missingGH_TOKEN— caught and fixed by the new validator.