Pin poutine Docker image to the digest-pinned constant - #50279
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #50279 does not have the 'implementation' label and has only 15 new lines of code in business logic directories (threshold: 100). |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Test Quality Sentinel completed test quality analysis. |
There was a problem hiding this comment.
Pull request overview
Pins Poutine execution to the existing digest-pinned image constant, aligning execution with image pre-pulling.
Changes:
- Uses
PoutineImagefor directory and single-file scans. - Adds a digest-validation test, though it does not cover command construction.
Show a summary per file
| File | Description |
|---|---|
pkg/cli/poutine.go |
Replaces mutable Poutine image references. |
pkg/cli/poutine_test.go |
Validates the image constant is digest-pinned. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
| func TestPoutineImageIsPinnedByDigest(t *testing.T) { | ||
| if _, err := validateDockerImageRef(PoutineImage); err != nil { | ||
| t.Fatalf("PoutineImage %q failed docker image reference validation: %v", PoutineImage, err) | ||
| } | ||
| if !strings.Contains(PoutineImage, "@sha256:") { |
There was a problem hiding this comment.
The changes look correct and improve supply-chain security by ensuring the executed poutine image matches the digest-pinned constant used by the pre-pull logic. All four call sites are updated consistently and the regression test validates the pinning invariant.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 13.2 AIC · ⌖ 10.3 AIC · ⊞ 5.4K
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs — the fix correctly addresses the root cause (:latest tag instead of pinned digest) and adds a regression test. No blocking issues.
📋 Key Themes & Highlights
Positive Highlights
- ✅ Root cause properly fixed: all four
exec.Commandsites in both scan paths now usePoutineImage - ✅ Regression test
TestPoutineImageIsPinnedByDigestguards against future drift (validates bothvalidateDockerImageRefand@sha256:presence) - ✅ Aligns the executed image with the pre-pull constant in
docker_images.go— eliminates the execute/pull divergence - ✅ Comment strings updated to use
<PoutineImage>placeholder, keeping docs consistent
Minor Follow-up (non-blocking)
pkg/cli/README.md line 461 still documents PoutineImage as "ghcr.io/boostsecurityio/poutine:latest" — this is now stale and could mislead readers. Worth a one-line update.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 23.3 AIC · ⌖ 10.6 AIC · ⊞ 7.1K
Comment /matt to run again
There was a problem hiding this comment.
Verdict: Approve — no blocking issues
Small, correct, well-scoped fix: replaces the hardcoded ghcr.io/boostsecurityio/poutine:latest tag with the existing digest-pinned PoutineImage constant in both the directory and single-file scan paths (command args + verbose echo comments), and adds TestPoutineImageIsPinnedByDigest to guard against regression.
Review notes
- All four
exec.Commandcall sites and both verbose-echo comment strings were updated consistently — no missed occurrence of the old:latestliteral remains inpoutine.go. PoutineImage(defined indocker_images.go) is validated viavalidateDockerImageRefand is digest-pinned (@sha256:...), matching the pre-pull logic indocker_images.goso the pulled and executed images now match.- New test
TestPoutineImageIsPinnedByDigestcorrectly asserts both the reference validation and the digest requirement. - No correctness, security, or performance concerns found in the diff. Comment-only changes are accurate (comments were updated to reflect the constant instead of leaving a stale literal).
🔎 Code quality review by PR Code Quality Reviewer · auto · 14.8 AIC · ⌖ 6.25 AIC · ⊞ 7.9K
Comment /review to run again
🧪 Test Quality Sentinel Report✅ Test Quality Score: 100/100 — Excellent
📊 Metrics (1 test)
Verdict
|
|
🎉 This pull request is included in a new release. Release: |
Sighthound flagged five
exec.Commandsites inpkg/clias command injection. All five pass argument slices (no shell) and four already validate their dynamic arguments. Auditing them surfaced one genuine issue:poutine.goinvoked Docker with a mutable:latesttag rather than the digest-pinned constant used by every other scanner.Change
pkg/cli/poutine.go— replace the hardcodedghcr.io/boostsecurityio/poutine:latestliteral with thePoutineImageconstant in both scan paths (directory and single-file) and their verbose echo strings.pkg/cli/poutine_test.go— regression test assertingPoutineImageis digest-pinned and passesvalidateDockerImageRef.This also aligns poutine with the image pre-pull logic in
docker_images.go, which already referenced the pinned constant — previously the pulled image and the executed image could diverge.Findings assessed as already mitigated
runner_guard.go:114ValidateAbsolutePathon git root;buildDockerVolumeMount;buildRunnerGuardContainerScanPath(Clean + within-root +./prefix against option injection);ResolveExecutablePathgrant.go:176validateDockerImageRef(name/tag/digest allowlist, no leading-, no whitespace/control chars);buildDockerReadonlyFileMountwith regular-file statpoutine.go:109/229buildDockerVolumeMountupgrade_command.go:507ValidateExecutablePath; argv control-character screenNo changes made to those sites.