Add --syft container scan support to gh aw compile#47515
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
--syft container scan support to gh aw compile
|
@copilot update static analysis agentic workflow |
There was a problem hiding this comment.
Pull request overview
Adds Syft-based container SBOM scanning to the compile pipeline and MCP interface.
Changes:
- Adds
--syftconfiguration and command wiring. - Runs Syft through Docker against manifest container images.
- Extends Docker preparation and scanner argument tests.
Show a summary per file
| File | Description |
|---|---|
cmd/gh-aw/main.go |
Defines and wires --syft. |
pkg/cli/compile_config.go |
Adds Syft compile configuration. |
pkg/cli/compile_external_tools.go |
Exposes the Syft batch runner. |
pkg/cli/compile_pipeline.go |
Executes Syft after compilation. |
pkg/cli/docker_images.go |
Adds Syft image preparation. |
pkg/cli/docker_images_test.go |
Updates Docker image tests. |
pkg/cli/mcp_argument_validation_test.go |
Adds Syft to MCP parameter tests. |
pkg/cli/mcp_tools_readonly.go |
Adds MCP Syft support. |
pkg/cli/syft.go |
Implements Docker-based Syft scanning. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 9/9 changed files
- Comments generated: 4
- Review effort level: Medium
| imageRef = img.Image | ||
| } | ||
|
|
||
| if _, err := runSyftOnImage(imageRef, verbose); err != nil { |
There was a problem hiding this comment.
Addressed in 8af308f and verified in latest commit. SBOM files are now persisted to /tmp/gh-aw-syft-sboms/sbom-{image}.json with paths returned in SyftScanResult.SBOMPath. The compile result includes SBOM paths in verbose output (lines 84-88 of syft.go).
| if err := cmd.Run(); err != nil { | ||
| stderrStr := strings.TrimSpace(stderr.String()) | ||
| if stderrStr != "" { | ||
| syftLog.Printf("syft stderr for %s: %s", imageRef, stderrStr) | ||
| } | ||
| return nil, fmt.Errorf("syft failed on %s: %w", imageRef, err) | ||
| } |
There was a problem hiding this comment.
Addressed in 8af308f. Non-empty stderr is now captured and included in the returned error message (lines 127-133 of syft.go): "syft failed on %s: %w\nstderr: %s" format provides registry authentication and image resolution failures.
| cmd := exec.Command( | ||
| "docker", | ||
| "run", | ||
| "--rm", | ||
| SyftImage, | ||
| imageRef, | ||
| "-o", "syft-json", | ||
| ) |
There was a problem hiding this comment.
Addressed in 8af308f. Context is now threaded through runSyftOnImage signature and exec.CommandContext is used (line 108 of syft.go), allowing cancellation during long registry operations.
| return nil | ||
| } | ||
|
|
||
| func runSyftOnImage(imageRef string, verbose bool) (*syftOutput, error) { |
There was a problem hiding this comment.
Addressed in 8af308f. Added comprehensive test suite in syft_test.go with 8 test functions covering: context cancellation, JSON parsing, SBOM persistence, safe filename generation, strict/non-strict modes, and empty lock file handling.
|
@copilot please run the
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Test Quality Sentinel completed test quality analysis. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. |
|
✅ 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.
Skills-Based Review 🧠
Applied /tdd and /codebase-design — requesting changes on correctness and test coverage gaps.
📋 Key Themes & Highlights
Key Themes
- No unit tests for
syft.go:grype.gohas extensive coverage ingrype_test.go; the analogoussyft.gohas none. Missing tests for image selection, JSON parsing, command failure, and strict/non-strict paths. - Context not propagated to Docker subprocess:
cmd.Run()is not bound to the compile context — cancellation during a slow registry pull is silently ignored. - Stderr swallowed on failure: only the exit-status string is surfaced; actionable Docker/registry errors remain hidden from the user.
- SBOM discarded:
runSyftOnImagereturns a parsed*syftOutputthat is never used — the only observable output is a package count, making--syftless useful than--grypein non-verbose mode.
Positive Highlights
- ✅ Clean structural parity with
grype.go— the batch-runner abstraction is well reused. - ✅ Consistent strict/non-strict handling and pinned-vs-floating image selection.
- ✅ MCP wiring (args struct, validation list, Docker preflight, log line) is complete and correct.
- ✅
#nosec G204annotation is present and justified.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 25 AIC · ⌖ 7.39 AIC · ⊞ 6.7K
Comment /matt to run again
There was a problem hiding this comment.
Review: Add --syft container scan support
The implementation is well-structured and follows the existing scanner patterns. Four blocking issues were identified by a prior review pass; I am adding one more:
Blocking issues (5 total):
-
Unpinned
anchore/syft:latestimage (docker_images.go:36) — floating tag breaks reproducibility and is a supply-chain risk. Pin to a versioned tag likev1.x.yto matchrhysd/actionlint:1.7.12. -
SBOM output is discarded (
syft.go:54) —runSyftOnImagereturns*syftOutputbut the caller ignores it. Users get only a package count; no SBOM data is surfaced or stored. -
Scan errors hide actionable stderr (
syft.go:101) — on failure, onlyexit status Nis returned to the caller; Syft/Docker stderr (e.g. auth errors, unknown image) is sent only to the debug logger. -
No context cancellation propagation (
syft.go:84) —cmd.Run()is not bound to the compile context, so long registry operations cannot be cancelled. -
No tests for
syft.go— analogousgrype.gohas comprehensive test coverage; this new file has none.
Non-blocking notes
- The
--syftflag help text could mention that an SBOM is not currently persisted. GrypeImagehas the same:latestfloating-tag issue; consider addressing both together.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 27 AIC · ⌖ 4.66 AIC · ⊞ 5K
| PoutineImage = "ghcr.io/boostsecurityio/poutine:latest" | ||
| ActionlintImage = "rhysd/actionlint:1.7.12" | ||
| RunnerGuardImage = "ghcr.io/vigilant-llc/runner-guard:latest" | ||
| SyftImage = "anchore/syft:latest" |
There was a problem hiding this comment.
Unpinned image tag — anchore/syft:latest uses a floating tag. All other scanners with their own tag use a pinned version (e.g. rhysd/actionlint:1.7.12). A floating :latest tag makes scans non-reproducible and creates a supply-chain risk if the upstream image is compromised.
Pin to a specific release tag or digest, e.g.:
SyftImage = "anchore/syft:v1.x.y"@copilot please address this.
There was a problem hiding this comment.
Addressed in latest commit. Syft image is now pinned to v1.48.0 instead of latest tag in docker_images.go:36.
Test Quality Sentinel Report
|
There was a problem hiding this comment.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (185 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
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
REQUEST_CHANGES — syft.go has several correctness and performance issues that must be fixed before merge.
Blocking issues (5)
1. SBOM output is discarded (high)
The *syftOutput returned by runSyftOnImage is ignored on every call. --syft currently produces only a count message; no SBOM data is stored, returned, or written to disk. The flag is not useful until this is resolved.
2. Context not propagated — Docker ignores cancellation (high)
exec.Command is used instead of exec.CommandContext. A cancelled compile context cannot stop a running Docker pull or catalog operation. The full call chain (runSyftOnImage → runSyftOnLockFiles → RunSyftOnLockFiles) must accept and forward ctx.
3. Unconditional json.Unmarshal on potentially empty stdout (medium)
If syft exits 0 but writes nothing, the unmarshal returns a misleading unexpected end of JSON input. Add a stdout.Len() == 0 guard.
4. No scan-result cache (medium)
grype.go deduplicates per-image runs with a mutex-protected map. syft.go has none — N lock files referencing the same image trigger N Docker pulls.
5. Missing scan subcommand in Docker invocation (medium)
Syft deprecated the implicit default command in favor of syft scan <source> (v0.60+). The current invocation (syft <image> -o syft-json) may silently break as anchore/syft:latest advances.
🔎 Code quality review by PR Code Quality Reviewer · sonnet46 70.8 AIC · ⌖ 5.13 AIC · ⊞ 5.7K
Comment /review to run again
Comments that could not be inline-anchored
pkg/cli/syft.go:437
runSyftOnImage result discarded — SBOM is silently dropped: the parsed package list is never stored, returned, or written to disk, so --syft currently produces only a count message with no persistent artifact.
<details>
<summary>💡 Suggested fix</summary>
Either write the SBOM to a sidecar file (e.g. <lockfile>.syft.json) or at minimum log the full result. As written, the scanner produces no actionable output — the *syftOutput return value is discarded:
// current:
if _, er…
</details>
<details><summary>pkg/cli/syft.go:460</summary>
**`exec.Command` used instead of `exec.CommandContext` — Docker process ignores context cancellation**: if the compile context is cancelled (timeout or interrupt) while syft is pulling or cataloging, `cmd.Run()` blocks indefinitely.
<details>
<summary>💡 Suggested fix</summary>
Thread `ctx` through the full call chain and use `exec.CommandContext`:
```go
// runSyftOnImage(ctx context.Context, imageRef string, verbose bool) (*syftOutput, error)
cmd := exec.CommandContext(ctx, "docker", "run",…
</details>
<details><summary>pkg/cli/syft.go:487</summary>
**`json.Unmarshal` called unconditionally — empty stdout causes a misleading parse error**: if syft succeeds but writes no JSON (empty image, startup warning on stdout), this returns `unexpected end of JSON input` instead of a useful diagnostic.
<details>
<summary>💡 Suggested fix</summary>
Guard the unmarshal, matching the defensive pattern in `grype.go`:
```go
if stdout.Len() == 0 {
return nil, fmt.Errorf("syft produced no output for %s", imageRef)
}
if err := json.Unmarshal(stdout.Byt…
</details>
<details><summary>pkg/cli/syft.go:1</summary>
**No scan-result cache — same image scanned N times**: `grype.go` deduplicates runs with a `grypeCache` (`sync.Mutex` + `map[string]*grypeOutput`). `syft.go` has no equivalent, so N lock files referencing the same image trigger N sequential Docker pulls and catalog runs.
<details>
<summary>💡 Suggested fix</summary>
Add a package-level cache matching `grypeCache`:
```go
var (
syftCacheMu sync.Mutex
syftCache = map[string]*syftOutput{}
)Then check and populate in `runSyftOnIma…
pkg/cli/syft.go:461
docker run invocation missing scan subcommand — may break on current syft versions: syft deprecated the implicit default command in favor of syft scan <source> (stable since v0.60+). Running without the subcommand may silently fall back to legacy behavior or fail as anchore/syft:latest advances.
<details>
<summary>💡 Suggested fix</summary>
cmd := exec.CommandContext(ctx, "docker", "run", "--rm",
SyftImage,
"scan", imageRef,
"-o", "syft-json",
)Pin `SyftIma…
|
@copilot run pr-finisher skill |
…, and tests Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Completed pr-finisher task. Addressed all review feedback in commits 8af308f, e68aa07, dca8f46, and fb4d3ed:
All tests passing, lint clean, workflows validated. |
|
@copilot please run the
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
|
🎉 This pull request is included in a new release. Release: |
This change adds Syft as a first-class compile-time scanner, aligned with existing optional scanners (
actionlint,poutine,zizmor,runner-guard,grype). The compile command and MCP compile interface can now request SBOM scanning for container images referenced by compiled workflows.CLI and compile config surface
--syfttogh aw compile.CompileConfigand command construction paths.Compile pipeline integration
Syft scanner implementation
pkg/cli/syft.gowith Docker-based Syft execution against referenced images.Docker image preparation + MCP compile tool
SyftImageto Docker scanner image constants.syft.Targeted test updates