Uh oh!
There was an error while loading. Please reload this page.
Add ignore-images to .grant.yaml and exclude the Serena MCP image from license scanning - #53369
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
ignore-images to .grant.yaml and exclude the Serena MCP image from license scanning✅ Design Decision Gate 🏗️ completed the design decision gate check.
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
✅ Ponytail Reviewer completed successfully!
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
✅ PR Code Quality Reviewer completed the code quality review. Reviewed PR #53369 and found no actionable blocking issues or non-duplicative line comments to submit.
|
There was a problem hiding this comment.
Pull request overview
Adds image-level license-scan exclusions while retaining vulnerability scanning.
Changes:
- Parses and applies
ignore-imagesglob patterns. - Excludes the Serena MCP image.
- Adds documentation, tests, and a changeset.
Show a summary per file
| File | Description |
|---|---|
pkg/cli/grant.go | Filters ignored images before Grant scanning. |
pkg/cli/grant_test.go | Tests policy parsing and matching. |
.grant.yaml | Excludes the Serena image. |
CONTRIBUTING.md | Documents image exclusions. |
.changeset/grant-ignore-images-serena.md | Records the user-facing change. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Balanced
| if ref == pattern { | ||
| return true | ||
| } | ||
| if matched, err := filepath.Match(pattern, ref); err == nil && matched { |
There was a problem hiding this comment.
Addressed in 4ca2cf3: image references now use path.Match, and TestGrantIsImageIgnored_GlobDoesNotMatchDeeperNamespace verifies a single-segment glob does not match a deeper namespace.
There was a problem hiding this comment.
The implementation is clean and correct. grantIsImageIgnored uses filepath.Match (shell globbing), which correctly handles pinned digest refs like serena:1.7.0@sha256:... since * does not cross path separators. Test coverage is thorough, including an integration test against the live .grant.yaml. No blocking issues found.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 12.9 AIC · ⌖ 7.89 AIC · ⊞ 5.6K
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.
|
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs — approving with one minor suggestion on silent error handling.
📋 Key Themes & Highlights
Positive Highlights
- ✅ Clean, minimal Go implementation —
grantIgnoredImagePatterns+grantIsImageIgnoredare well-separated and testable - ✅ Good test coverage: key present, absent, malformed YAML, glob matching, negative cases, and the repository policy guard test
- ✅
ignore-imagesis forward-compatible — grant ignores unknown YAML keys, so the policy file remains valid - ✅ Verbose logging path is wired correctly; early-exit when all images are excluded is a nice touch
- ✅ PR description clearly explains why package-level exceptions would be worse than a full image exclusion
One Finding
- Silent bad-pattern handling — invalid glob syntax in
ignore-imagesis swallowed silently (see inline comment). Low risk given the current single-entry policy, but worth hardening.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 21.3 AIC · ⌖ 8.53 AIC · ⊞ 7.7K
Comment /matt to run again
| } | ||
| if ref == pattern { | ||
| return true | ||
| } |
There was a problem hiding this comment.
[/diagnosing-bugs] Malformed glob patterns (e.g. [z-a]) are silently skipped — filepath.Match returns an error and the image is treated as a non-match without any warning.
💡 Suggestion
Log a warning when a pattern errors:
ifmatched, err:=filepath.Match(pattern, ref); err!=nil {
grantLog.Printf("invalid ignore-images pattern %q: %v", pattern, err)
continue
}
ifmatched {
returntrue
}This surfaces misconfigured entries at scan time rather than silently passing an image through.
@copilot please address this.
There was a problem hiding this comment.
Addressed in 4ca2cf3: invalid ignore-images patterns now emit a grant log message and are skipped; TestGrantIsImageIgnored_InvalidPattern covers the malformed-pattern path.
There was a problem hiding this comment.
Generated by ✂️ Ponytail Reviewer for #53369 · auto · 21 AIC · ⌖ 3.61 AIC · ⊞ 7.2K
Comment /ponytail to run again
| if ref == "" { | ||
| continue | ||
| } | ||
| if ref == pattern { |
There was a problem hiding this comment.
L221: delete: redundant literal-equality check before filepath.Match. filepath.Match already matches non-glob strings exactly; drop if ref == pattern { return true }.
There was a problem hiding this comment.
Addressed in 4ca2cf3: removed the redundant literal-equality check; path.Match handles literal patterns exactly.
… extension Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (174 new lines in 📄 Draft ADR committed:
📋 What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. ❓ Why ADRs Matter
ADRs 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 References:§32030801371
|
pelikhan
commented
Aug 17, 2026
@copilot run pr-finisher skill |
gh-aw-bot
commented
Aug 17, 2026
@copilot this PR is ready for the next finishing pass. Please address these items, newest first:
Run: https://github.com/github/gh-aw/actions/runs/32032749148
|
Hey What this doesAdds ignore-images support to .grant.yaml to exclude container images by glob pattern from license scanning — in this case, the Serena MCP development environment that ships with 677 license-policy violations (mostly metadata gaps in Rust crates, not actual license risks). The feature maintains vulnerability scanning coverage while unblocking policy compliance. Quality checklist ✅
All green for review! 🚀
|
…53383) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…ge-scan-findings-another-one # Conflicts: # .github/workflows/mcp-inspector.lock.yml Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
pelikhan
commented
Aug 17, 2026
@copilot resolve the merge conflicts on this branch. |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
gh-aw-bot
commented
Aug 17, 2026
@copilot please take another pass on this PR. There are unresolved review threads to address:
Please respond on each open thread, refresh the branch if needed, and run the
|
All three open review threads now have responses referencing |
🎉 This pull request is included in a new release. Release: |
The daily container scan reports 677 license-policy violations for
ghcr.io/oraios/serena:latest— 566 of them cargo crates that ship no SPDX metadata, plus 111 named-license flags from its Debian/GCC/Perl toolchain. Grant's policy schema only scopes exceptions by package name, so suppressing these would mean adding ~700ignore-packagesentries that silently weaken the allowlist for every other scanned image.Changes
pkg/cli/grant.go—.grant.yamlmay now carry anignore-imageslist of glob patterns, matched against both the tagged and pinned (@sha256:) references of each image collected from lock manifests. Matching images are skipped bygh aw compile --grant. The key is gh-aw specific; grant unmarshals its policy non-strictly, so the file remains a valid grant config..grant.yaml— excludesghcr.io/oraios/serena:*with the rationale inline: it is a third-party development image that gh-aw executes as an MCP server and never links or redistributes.CONTRIBUTING.md— documents the key next to the existingignore-packagesand.grype.yamlsections.Scope
Vulnerability scanning is untouched —
--grypestill covers this image. The 50 Critical / 128 High findings are either unfixable in the Debian trixie base (no fixed version published forperl,curl,libc6,openssh) or require serena to rebuild with patchednode/tar; neither is actionable from this repository, so they stay visible rather than being risk-accepted here.\nrun: https://github.com/github/gh-aw/actions/runs/32032749148> Generated by 👨🍳 PR Sous Chef · gpt54 · 6.96 AIC · ⌖ 5.94 AIC · ⊞ 8.8K · ◷