Add Grant license scanning to gh aw compile#47516
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
gh aw compile
There was a problem hiding this comment.
Pull request overview
Adds opt-in Grant license-policy scanning for container images during workflow compilation.
Changes:
- Adds
--grantto CLI, compile, and MCP paths. - Implements Grant execution, policy loading, result reporting, and Docker preparation.
- Adds tests and documentation for the new scanner.
Show a summary per file
| File | Description |
|---|---|
cmd/gh-aw/main.go |
Registers and configures --grant. |
cmd/gh-aw/compile_flags_test.go |
Tests Grant flag registration. |
pkg/cli/compile_config.go |
Adds Grant configuration. |
pkg/cli/compile_pipeline.go |
Integrates Grant into batch compilation. |
pkg/cli/compile_external_tools.go |
Exposes the Grant runner. |
pkg/cli/grant.go |
Implements Grant scanning and reporting. |
pkg/cli/grant_test.go |
Tests Grant helpers. |
pkg/cli/docker_images.go |
Registers the Grant Docker image. |
pkg/cli/docker_images_test.go |
Updates Docker preparation tests. |
pkg/cli/mcp_tools_readonly.go |
Adds Grant to MCP compile requests. |
pkg/cli/mcp_server_defaults_test.go |
Updates the compile schema fixture. |
docs/src/content/docs/setup/cli.md |
Documents the CLI flag. |
docs/src/content/docs/reference/gh-aw-as-mcp-server.md |
Documents MCP support. |
docs/src/content/docs/reference/compilation-process.md |
Adds Grant to compilation examples. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (2)
pkg/cli/compile_pipeline.go:466
- The all-workflows path also places Grant inside the Grype conditional. As a result,
gh aw compile --grantwithout explicit workflow arguments silently skips license scanning unless--grypeis enabled.
// Run grant license scanner on container images referenced in the compiled lock files.
if config.Grant && !config.NoEmit && len(lockFilesForGrant) > 0 {
pkg/cli/compile_pipeline.go:472
- The directory compilation path has the same strict JSON failure: a Grant policy violation returns before
outputResults, causing MCP compile-all requests to receive a protocol error with no JSON result. Preserve the scanner findings invalidationResultsand emit the JSON response before returning the strict-mode failure.
if err := RunGrantOnLockFiles(lockFilesForGrant, config.Verbose && !config.JSONOutput, config.Strict); err != nil {
if config.Strict {
return workflowDataList, err
- Files reviewed: 14/14 changed files
- Comments generated: 2
- Review effort level: Medium
|
|
||
| // Run grant license scanner on container images referenced in the compiled lock files. | ||
| if config.Grant && !config.NoEmit && len(lockFilesForGrant) > 0 { | ||
| if err := ctx.Err(); err != nil { | ||
| return workflowDataList, err | ||
| } | ||
| if err := RunGrantOnLockFiles(lockFilesForGrant, config.Verbose && !config.JSONOutput, config.Strict); err != nil { | ||
| if config.Strict { | ||
| return workflowDataList, err | ||
| } | ||
| } | ||
| } | ||
| } |
| if err := RunGrantOnLockFiles(lockFilesForGrant, config.Verbose && !config.JSONOutput, config.Strict); err != nil { | ||
| if config.Strict { | ||
| return workflowDataList, err |
|
✅ Test Quality Sentinel completed test quality analysis. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ PR Code Quality Reviewer completed the code quality review. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
REQUEST_CHANGES — two correctness issues in grant.go and a misclassified test need fixes before merge.
Blocking findings summary
1. runErr silently discarded (grant.go:559)
When grant exits non-zero but emits parseable JSON with at least one non-error target, grantRunOnImage discards the exit code and returns success. A transient Docker failure or OOM that produces partial output would be silently swallowed.
2. Total scan failure returns nil in non-strict mode (grant.go:498)
If every image scan fails (all end up in scanErrors), non-strict mode prints a warning and returns nil. The compile pipeline sees a clean result. A 100% failure rate should still return an error regardless of strict mode.
3. TestGrantPolicyFile misclassified as unit test (grant_test.go:716)
Tagged !integration but calls grantPolicyFile() which requires .grant.yaml to exist on disk. This will cause flaky failures in forks and sandboxes. Should be //go:build integration or refactored to use a temp file.
🔎 Code quality review by PR Code Quality Reviewer · sonnet46 39.9 AIC · ⌖ 4.95 AIC · ⊞ 5.7K
Comment /review to run again
Comments that could not be inline-anchored
pkg/cli/grant.go:559
runErr is silently discarded when JSON output is parseable: if grant exits non-zero (e.g. transient Docker OOM or unexpected error) but still emits partial JSON with at least one non-error target, grantRunOnImage returns the partial output as success. The caller treats the image as cleanly scanned, hiding the failure.
<details>
<summary>💡 Suggested fix</summary>
After parsing JSON and walking targets, check whether runErr != nil and no target explains the exit:
runErr := cmd…
</details>
<details><summary>pkg/cli/grant.go:498</summary>
**Total scan failure returns `nil` (success) in non-strict mode**: when every image scan fails and `scanErrors` is non-empty but `strict` is false, the function prints a warning and returns `nil`. The caller in `compile_pipeline.go` has no way to distinguish "scanned cleanly" from "every scan failed" — it always sees success.
<details>
<summary>💡 Suggested fix</summary>
Return a sentinel or non-nil error even in non-strict mode when *all* images failed (i.e., `len(scanErrors) == len(images)`…
</details>
<details><summary>pkg/cli/grant_test.go:716</summary>
**`TestGrantPolicyFile` is misclassified as a unit test**: it has the `!integration` build tag but makes a live filesystem call to `grantPolicyFile()`, which requires `.grant.yaml` to exist at the repository root. This will hard-fail in any fork, CI environment, or Docker-based sandbox that doesn't have that file. It should use `//go:build integration` or be refactored to not touch the real filesystem.
<details>
<summary>💡 Suggested fix</summary>
Either:
1. Change the build tag to `integrati…
</details>
There was a problem hiding this comment.
Skills-Based Review
Applied /diagnosing-bugs, /tdd, and /codebase-design — requesting changes on two blocking bugs and three correctness/reliability issues.
Key Themes
Blocking Issues (already noted in existing comments)
- Grant nested inside Grype block (
compile_pipeline.go:230):--grantonly runs when--grypeis also supplied — the block must be closed and placed at the same level as Grype. - Strict-mode early return before
outputResults(compile_pipeline.go:236): MCP callers in strict mode receive empty stdout JSON when a license violation is found.
New Issues
/tmp/gh-aw-grant-policy.yamlrace (grant.go:148): concurrent compile runs on the same host share a single container path — add a PID suffix.grantDisplayFindingsignoresJSONOutputmode (grant.go:246): stderr findings pollute MCP responses; needs anio.Writerparameter gated on JSON mode.grantPolicyFile()hard-errors with no guidance (grant.go:72): users in repos without.grant.yamlget an opaque error; improve the message with a hint.TestGrantPolicyFileis environment-coupled (grant_test.go:67): fails in forks/CI without.grant.yaml; inject the root path for testability.
Positive Highlights
- Clean reuse of
collectContainerImagesFromLockFiles— no duplication with the Grype path. - Thorough
grantRunOnImageerror handling (exit code, empty targets, evaluation errors). - Good use of
runBatchLockFileToolfor consistent batch semantics.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 45.5 AIC · ⌖ 4.94 AIC · ⊞ 6.7K
Comment /matt to run again
Comments that could not be inline-anchored
pkg/cli/docker_images.go:225
[/codebase-design] GrantImage uses :latest — confirm Dependabot tracks anchore/grant Docker images, otherwise scan results may silently drift.
<details>
<summary>Suggestion</summary>
Verify .github/dependabot.yml includes anchore/grant. If not, pin to a semver tag to match other scanner images when they get pinned.
</details>
@copilot please address this.
pkg/cli/grant.go:72
[/diagnosing-bugs] grantPolicyFile() returns a hard error when .grant.yaml is absent, so gh aw compile --grant always fails in repos without a policy file — with no actionable guidance.
<details>
<summary>Suggestion</summary>
Improve the error message to include a link to Grant policy docs or a hint to create the file:
return "", fmt.Errorf("grant requires %s at the repository root (see https://github.com/anchore/grant#policy): %w", grantPolicyFilename, err)</details>
…
pkg/cli/grant.go:148
[/codebase-design] The hardcoded container path /tmp/gh-aw-grant-policy.yaml is a race if two gh aw compile --grant runs execute concurrently on the same host (e.g. parallel matrix CI jobs).
<details>
<summary>Suggestion</summary>
Add a PID or random suffix to the in-container path:
containerPolicyPath := fmt.Sprintf("/tmp/gh-aw-grant-policy-%d.yaml", os.Getpid())</details>
@copilot please address this.
pkg/cli/grant.go:246
[/tdd] grantDisplayFindings writes directly to os.Stderr even when config.JSONOutput is true (MCP mode). In JSON mode, callers expect clean stdout — this stderr output will appear as noise in MCP responses.
<details>
<summary>Suggestion</summary>
Thread an io.Writer through grantDisplayFindings and conditionally suppress or redirect findings in JSON mode, mirroring the config.Verbose && !config.JSONOutput guard used on line 78.
</details>
@copilot please address this.
pkg/cli/grant_test.go:67
[/tdd] TestGrantPolicyFile is environment-coupled: it passes only when .grant.yaml exists at the git root. This will fail in forks or CI environments where the file is absent.
<details>
<summary>Suggestion</summary>
Refactor grantPolicyFile to accept a root directory param, then test with t.TempDir() for both missing-file and present-file scenarios. The current test can remain as an integration test gated on the real repository.
</details>
@copilot please address this.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (409 new lines in Draft ADR committed: This PR cannot merge until an ADR is linked in the PR body. What to do next
ADR: ADR-47516: Add Grant Container Image License Scanning to Compile Pipeline Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. Why ADRs MatterADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
|
There was a problem hiding this comment.
Review: Add Grant license scanning to gh aw compile
Good implementation overall — the architecture mirrors Grype cleanly and the test coverage is reasonable. Two blocking issues found.
Blocking
-
Grant block nested inside Grype guard (
compile_pipeline.golines 230 and 466 — bothcompileSpecificFilesandcompileAllFilesInDirectory):--grantonly runs when--grypeis also supplied. Both functions need the Grant block moved out as an independent top-levelif config.Grantblock. -
Hard failure when
.grant.yamlis absent (grant.go:138):grantPolicyFile()returns an opaque error for any repo that does not already have.grant.yaml. This makes--grantimmediately unusable without prior setup. Either skip scanning gracefully with a helpful message, or gate the error onos.IsNotExistand print a setup hint.
Non-blocking (noted by earlier review)
- Strict-mode early return at line 236 skips
outputResultsin JSON mode.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 66.1 AIC · ⌖ 10 AIC · ⊞ 5K
| } | ||
|
|
||
| // Run grant license scanner on container images referenced in the compiled lock files. | ||
| if config.Grant && !config.NoEmit && len(lockFilesForGrant) > 0 { |
There was a problem hiding this comment.
Same nesting bug as in compileSpecificFiles: the Grant block (lines 466–475) is inside the if config.Grype block, so --grant without --grype is silently skipped in compileAllFilesInDirectory. Move this block out as a top-level if config.Grant block, parallel to the Grype block at line 455.
@copilot please address this.
| policyFile := filepath.Join(repoRoot, grantPolicyFilename) | ||
| info, err := os.Stat(policyFile) | ||
| if err != nil { | ||
| return "", fmt.Errorf("grant requires %s at the repository root: %w", grantPolicyFilename, err) |
There was a problem hiding this comment.
grantPolicyFile() returns a hard error when .grant.yaml is absent from the repo root. This turns --grant into a compile-breaking flag for every repository that hasn't created that file, even if the user just wants to try the feature. Consider returning a nil policy path (skip scanning gracefully) or at minimum printing a friendly setup hint instead of an opaque os.Stat error.
@copilot please address this.
🧪 Test Quality Sentinel Report✅ Test Quality Score: 80/100 — Excellent
📊 Metrics (5 tests)
|
|
@copilot run pr-finisher skill |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done. I ran the PR finisher flow and pushed fixes in |
|
@copilot please run the
|
Addressed in |
|
@copilot resolve the merge conflicts on this branch. |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Resolved by merging |
|
@copilot resolve the merge conflicts on this branch. |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Resolved in |
|
🎉 This pull request is included in a new release. Release: |
This adds Grant as an opt-in compile-time scanner for container images referenced by compiled workflow manifests. It extends the existing
compilescanner model so license-policy checks can run alongside workflow lint and security analysis.CLI and compile plumbing
--granttogh aw compile.CompileConfig, the compile pipeline, and the MCPcompiletool schema/dispatch path.Container image license scanning
gh-aw-manifestheaders in generated.lock.ymlfiles.Docker tool preparation
Policy source
.grant.yamlas the license policy input for Grant runs, keeping compile-time behavior aligned with the repository’s existing license rules.Docs and surface area
--grantanywhere compile scanner options are described.