Skip to content

fix(agent): pin every base image by digest so an upstream tag move can't rebuild the world - #704

Merged
scottschreckengaust merged 3 commits into
mainfrom
fix/pin-agent-image-digests
Aug 5, 2026
Merged

fix(agent): pin every base image by digest so an upstream tag move can't rebuild the world#704
scottschreckengaust merged 3 commits into
mainfrom
fix/pin-agent-image-digests

Conversation

@isadeks

@isadeks isadeks commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Problem

Closes #715

Three of the four external images in agent/Dockerfile were not pinned by digest:

FROM jdxcode/mise:latest AS mise            # no version at all
FROM golang:1.26.4-bookworm AS gh-builder   # tag; can be re-pushed
COPY --from=ghcr.io/astral-sh/uv:0.11.14 …  # tag; can be re-pushed
FROM python:3.13-slim@sha256:dc1546ee…      # correctly pinned

A tag is a moving pointer, and the mise COPY sits near the top of the final stage. When upstream republishes, that 149 MB layer changes — and so does every layer built after it: the ~413 MB apt install, the node install, the 362 MB dependency sync. The image is rebuilt and re-pushed in full for a commit that touched only a doc.

That is roughly 1.5 GB of avoidable transfer, triggered by upstream's release schedule rather than by any change of ours. It is wasteful everywhere and prohibitive on a modest uplink, where it turns a two-minute deploy into well over an hour. mise:latest was the worst case, carrying no version to reason about at all.

The quieter half matters more for a sample repo: two builds of the same commit could produce different images, because what they resolved to depended on when they ran.

Change

All four references are now image:tag@sha256:….

The tag is kept alongside the digest deliberately. image@sha256:… alone is valid but opaque; keeping both lets a reviewer tell an intentional version bump from a digest refresh of the same version, and gives the next person something to imagetools inspect.

Each digest is a multi-platform index confirmed to include linux/arm64, so --platform=$TARGETPLATFORM still selects the right architecture.

To bump one later: docker buildx imagetools inspect <image>:<tag> and copy the Digest — noted in a comment at the top of the file.

Verification

Measured, not assumed:

Check Result
Builds with all pins clean, 630 MB
Rebuild of same tree 18/18 cached
Layers differing 0 of 21
CDK asset hash stable across synths
Docs-only edit zero layer churn

The last row is the point: previously a docs-only edit invalidated the four large layers above. (The image ID still differs by build metadata, which pushes nothing — layer identity is what determines transfer cost.)

Guard

Nothing in the repo prevented this, and nothing would prevent it returning — there is no Dockerfile lint or pin check in CI. Added cdk/test/constructs/agent-image-pins.test.ts, asserting that:

  • every registry reference carries a digest;
  • each keeps a human-readable tag alongside it;
  • the parser finds all four refs and does not mistake COPY --from=<stage> for a registry image, so a green run cannot mean "matched nothing".

Each assertion was confirmed to fail against a deliberately broken version — including the exact shape of the original bug (unpinned mise:latest), a digest-without-tag ref, and an unpinned COPY --from= registry ref.

Scope

Independent of the log-delivery pin work in #703; this touches only the Dockerfile and adds a test. Full mise run build green: 3477 CDK tests (174 suites) and 681 CLI tests. No build mutation.

One thing I did not verify: that a real cdk deploy now skips the image push end to end. The layer-identity evidence above is direct, but the deploy-level win is inference from it.

…n't rebuild the world

`jdxcode/mise:latest` was pinned to no version at all, and `golang:1.26.4-bookworm`
and `ghcr.io/astral-sh/uv:0.11.14` were pinned by tag, which a publisher can re-push.
Only the python base carried a digest.

A tag is a moving pointer, and the mise COPY sits near the top of the final stage.
When upstream republishes, that 149 MB layer changes and so does every layer after
it — the ~413 MB apt install, the node install, the 362 MB dependency sync. The
image is then rebuilt and re-pushed in full for a commit that touched only a doc.
That is ~1.5 GB of avoidable transfer, slow on any connection and prohibitive on a
modest uplink, where it turns a two-minute deploy into well over an hour.

The quieter half matters more: two builds of the same commit could produce
different images, because what they resolved to depended on when they ran.

All four external references are now `image:tag@sha256:…`. The tag stays alongside
the digest deliberately — `image@sha256:…` alone is valid but opaque, and keeping
both lets a reviewer tell an intentional version bump from a digest refresh of the
same version. Each digest is a multi-platform index verified to include
linux/arm64, so `--platform=$TARGETPLATFORM` still selects the right architecture.

Verified rather than assumed: the image builds clean at every pin; a rebuild of the
same tree is fully cached with 0 of 21 layers differing; the CDK asset hash is
stable across repeated synths; and a docs-only edit now produces zero layer churn,
where before it invalidated the four large layers above.

Adds a guard, because nothing in the repo prevented this and nothing would prevent
it returning. It asserts every registry reference carries a digest, that each keeps
its tag, and — so a green run cannot mean "matched nothing" — that the parser finds
all four and does not mistake `COPY --from=<stage>` for a registry image. Each
assertion was confirmed to fail against a deliberately unpinned version, including
the exact shape of the original bug.
@isadeks
isadeks requested review from a team as code owners August 3, 2026 22:08
The dependency scan fails on this branch, and on main at 4357c35 with exactly the
same 16 findings across the same 6 packages. These are advisories published against
versions already in the lockfiles, not anything introduced here — this branch
changes only the agent Dockerfile and adds a test, and every flagged version matches
main's.

  cryptography  49.0.0 → 50.0.0   (PyPI, transitive via pyjwt)
  fast-uri      3.1.4  → 3.1.5
  undici        7.28.0 → 7.29.0
  ip-address    10.2.0 → 10.4.0

Bumped through the mechanisms already used for this in both lockfiles: yarn
`resolutions` at the root, npm `overrides` in the Forge app, and a targeted
`uv lock --upgrade-package` for the Python one. That keeps each package transitive.
Running `yarn upgrade` or `npm install <pkg>` instead would add these as direct
dependencies of packages that do not use them.

`ip-address` resolves to 10.4.0 rather than the 10.3.1 the advisory names as fixed,
because the range admits it and the newer release is also past the fix.

osv-scanner now reports no issues. Suites green after the bumps: 3477 cdk, 681 cli,
1436 agent, and the agent typecheck passes — worth checking rather than assuming,
since a lockfile bump is exactly where a break would surface.

@scottschreckengaust scottschreckengaust left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Principal Architect Review — PR #704

1. Verdict: Request changes

The engineering here is genuinely good — this is exactly the kind of supply-chain hardening the platform should welcome, and the test is one of the better-defended assertions I've reviewed lately. But I'm blocking on two non-code, cheap-to-fix process issues: there is no approved backing issue (ADR-003 gate) and the PR body materially misrepresents the diff (it claims Dockerfile-only while HEAD carries a cryptography major bump and three other dependency changes). Fix those two and this is an easy approve.

2. Vision & tenet alignment

Strongly aligned with bounded blast radius & cost and reviewable outcomes (VISION.md). Digest-pinning makes a rebuild depend on our changes rather than upstream's release schedule — it makes the agent image reproducible and bounds transfer cost, which directly serves fire-and-forget reliability. No tenet is traded away; no ADR is required. The one tension is with reviewable outcomes: see blocking issue #2 — an inaccurate PR body defeats informed review of the very change it describes.

3. Blocking issues

  1. No approved backing issue (ADR-003 governance gate). closingIssuesReferences is empty, the branch is fix/pin-agent-image-digests (no issue number), and no issue carries the approved label for this work. Per ADR-003 and the standing review rule, the gate is an approved backing issue; a branch with no issue reference is unauthorized work. Fix: file an issue for the image-pinning + advisory-clearing maintenance with an appropriate priority label (P1/P2), get it approved, and reference it from the PR. This is the governance gate, not a code defect — the code is fine.

  2. PR body misrepresents the diff — hides a cryptography major bump. The body states: "this touches only the Dockerfile and adds a test." That is false for the current branch. HEAD (a2c06925, fix(deps): clear 16 osv-scanner advisories) sits on top of the pin commit and changes agent/uv.lock (cryptography 49.0.0 → 50.0.0, a major version jump), package.json/yarn.lock (fast-uri, undici, ip-address), and the jira-forge-app manifests. A reviewer approving a "Dockerfile-only" PR would unknowingly ship a crypto-library major upgrade into the agent runtime. Fix: either (a) split the fix(deps) commit into its own PR (cleaner, matches the body's own stated scope and the #703-independence note), or (b) rewrite the PR body to disclose and justify the dependency bumps — especially the cryptography major — and confirm the agent runtime (cedarpy, etc.) is compatible. CI build (agentcore) is green, which is reassuring but not a substitute for disclosure.

4. Non-blocking suggestions / nits

  • Dockerfile (agent/Dockerfile:22,25,29,99) — Pinning is correct and the inline "how to bump" comment is excellent. Consider a follow-up to teach the digest-bump procedure to the dependency-update automation (Dependabot/renovate) so pins don't silently rot; the new test will catch un-pinning, but nothing keeps digests current.
  • Test parser (cdk/test/constructs/agent-image-pins.test.ts:189) — The COPY --from=(\S*[/:@]\S*) heuristic correctly excludes local stage names (mise, gh-builder) and correctly includes ghcr.io/astral-sh/uv:.... It would also treat a hypothetical registry:5000/img:tag@sha256:... (registry-with-port) fine under the tag regex. No change needed — noting I verified the edge cases.

5. Documentation

No docs/ changes in the real diff (the large doc list from a stale local main is not part of this PR — verified against origin/main..HEAD, 7 files). The digest-bump workflow is self-documented inline in the Dockerfile, which is sufficient; no guide/mirror update is required. Missing: an issue-tracking entry for this maintenance work (ties to blocking #1). Starlight mirror sync: N/A (no guide/design edits).

6. Tests & CI

  • New test is high quality. agent-image-pins.test.ts asserts (a) every registry ref carries a digest, (b) each keeps a human-readable tag, (c) the parser finds ≥4 refs so a green run can't mean "matched nothing", and (d) local build stages aren't mistaken for registry images. I traced all four FROM/COPY lines and the ^[^@]+:[^@:]+@sha256:[a-f0-9]{64}$ regex by hand against the current Dockerfile — all pass, and the negative guards are real. This is the AI005-correct shape (asserts what the code should do). I could not execute the suite in this review env (no node_modules in the worktree), but CI reports the full CDK suite green.
  • CI: all checks pass (Analyze x3, CodeQL, secrets/deps/workflow scan, PR-title, build (agentcore) 10m).
  • Bootstrap synth-coverage: Not applicable — the diff adds no CDK constructs/stacks/handlers and no new CloudFormation resource types, so no cdk/src/bootstrap/* policy, action-map, BOOTSTRAP_VERSION, or golden-baseline update is required.

7. Review agents run

Note: this review ran headless without the worktree toolchain, so the pr-review-toolkit agents could not be spawned as isolated subagents; I applied each agent's review dimension analytically over the (small, fully-read) diff and report per-dimension findings below.

  • code-reviewer — Ran (Dockerfile, TS test, package manifests). Findings: style/quality clean; pinning idiomatic; test well-structured. No guideline violations.
  • silent-failure-hunter — Ran (the test's file-read + regex parser is the only error-handling surface). The ≥4 refs guard specifically defends against the classic silent-pass-on-empty-match failure. No hidden fallbacks. No new runtime error handling in product code.
  • type-design-analyzer — Ran. No new types/interfaces introduced (string[] helper only). Nothing to flag.
  • comment-analyzer — Ran. Dockerfile and test comments are accurate and match behavior (verified the arm64 multi-platform-index and digest-bump claims against the actual refs). Accurate.
  • pr-test-analyzer — Ran. Coverage matches the change: the pin behavior is directly tested with negative guards. The fix(deps) commit ships no test, which is acceptable for lockfile/advisory bumps but reinforces blocking #2 (it should be disclosed/split).
  • /security-review — Ran as a lens. The change is net-positive supply-chain hardening (digest pinning + advisory clearing). No IAM, Cedar, network, secrets, or input-gateway code is touched, so no policy/least-privilege surface to assess. The cryptography major bump is the only security-relevant item and is advisory-driven (see blocking #2 for the disclosure ask).
  • Omitted: none whose scope the diff genuinely touches.

8. Human heuristics

  • Proportionality — Pass. A one-file Dockerfile fix plus a focused test; no over-abstraction.
  • Coherence — Concern (agent/uv.lock, HEAD commit). The fix(deps) work is coherent on its own but does not belong under a PR titled/described as image-pinning; it belongs in its own PR or an honest body (blocking #2).
  • Clarity — Pass. Names and comments communicate intent well; the digest-bump instructions are exemplary.
  • Appropriateness — Pass. Maintainable by this team; the test asserts intended behavior (AI005-correct) and is verified against the real Dockerfile, not a self-written mock (AI001-clean).

Summary: excellent, low-risk hardening blocked only on governance (no approved issue) and an inaccurate PR body that hides a cryptography major bump. Both are cheap to resolve.

@scottschreckengaust
scottschreckengaust requested review from a team August 4, 2026 16:52
@scottschreckengaust

Copy link
Copy Markdown
Contributor

3. Blocking issues

  1. No approved backing issue (ADR-003 governance gate). closingIssuesReferences is empty, the branch is fix/pin-agent-image-digests (no issue number), and no issue carries the approved label for this work. Per ADR-003 and the standing review rule, the gate is an approved backing issue; a branch with no issue reference is unauthorized work. Fix: file an issue for the image-pinning + advisory-clearing maintenance with an appropriate priority label (P1/P2), get it approved, and reference it from the PR. This is the governance gate, not a code defect — the code is fine.

I added #715 so unblocking above.

  1. PR body misrepresents the diff — hides a cryptography major bump. The body states: "this touches only the Dockerfile and adds a test." That is false for the current branch. HEAD (a2c06925, fix(deps): clear 16 osv-scanner advisories) sits on top of the pin commit and changes agent/uv.lock (cryptography 49.0.0 → 50.0.0, a major version jump), package.json/yarn.lock (fast-uri, undici, ip-address), and the jira-forge-app manifests. A reviewer approving a "Dockerfile-only" PR would unknowingly ship a crypto-library major upgrade into the agent runtime. Fix: either (a) split the fix(deps) commit into its own PR (cleaner, matches the body's own stated scope and the #703-independence note), or (b) rewrite the PR body to disclose and justify the dependency bumps — especially the cryptography major — and confirm the agent runtime (cedarpy, etc.) is compatible. CI build (agentcore) is green, which is reassuring but not a substitute for disclosure.

Will update this PR with a rebased version that clear vulnerabilities out of scope

@scottschreckengaust
scottschreckengaust self-requested a review August 4, 2026 20:17
@scottschreckengaust
scottschreckengaust dismissed their stale review August 4, 2026 20:20

Dismissing my own review

@scottschreckengaust scottschreckengaust left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Principal Architect Review — PR #704 (re-review at c2b66188)

1. Verdict: Approve

Both blockers from my prior review (now DISMISSED) are resolved on the current head, and I independently verified all four digests against the live registries. This is clean, well-evidenced supply-chain hardening with a real guard test. Approving.

Prior blockers — verified resolved, not re-raised:

Prior blocker Status at c2b66188
#1 No approved backing issue (ADR-003) Resolved. #715 exists, carries the approved label (verified via gh issue view 715 --json labels), is self-assigned to @isadeks, and is linked via Closes #715 in the body.
#2 PR body misrepresented the diff — hid a cryptography 49→50 major bump Resolved. The fix(deps) commit a2c06925 is now subsumed by e1dec8d1 ("remediate all osv-scanner advisories on main", #711), which is an ancestor of this head. git diff --stat origin/main...HEAD is exactly 2 files, +111/−4agent/Dockerfile and cdk/test/constructs/agent-image-pins.test.ts. No lockfile, package.json, or integrations/jira-forge-app change remains in this PR's contribution. The body's "touches only the Dockerfile and adds a test" is now accurate.

2. Vision & tenet alignment

Strongly aligned with bounded blast radius & cost and reviewable outcomes (VISION.md). Digest pinning makes an agent-image rebuild a function of our commits rather than upstream's release cadence — that is reproducibility of the artifact that actually executes untrusted cloned repos, which is a security property as much as a cost one. It also removes a real supply-chain hole: jdxcode/mise:latest gave an upstream publisher unilateral, unreviewed write access to the top layer of the agent runtime. No tenet is traded; no ADR required.

agent/AGENTS.md:86 lists "Dockerfile base image" under ⚠️ Ask first. Satisfied: #715 is the approved ask.

3. Blocking issues

None.

4. Non-blocking suggestions / nits

  1. agent/Dockerfile:22mise:latest@sha256:… keeps the least informative tag available, and the repo does publish versioned tags. I resolved the pinned digest against Docker Hub: sha256:b2297770… is exactly the digest of tag 2026.8.1; latest has already moved on to sha256:010e6829… (= 2026.8.2). So the human-readable half of the pin — the part the second test exists to preserve, and which the top-of-file comment says "documents what the digest is meant to be" — currently says only "whatever latest was at some point," which is precisely the information the PR sets out to recover. Suggest jdxcode/mise:2026.8.1@sha256:b2297770…. The digest itself is already immutable, so this is documentation quality, not correctness — hence a nit rather than a blocker. It also makes Dependabot's docker updates legible: a latest pin gives it no version to reason about.

  2. agent/Dockerfile:29python:3.13-slim digest is now stale relative to the tag. 3.13-slim currently resolves to sha256:bf503bb2…; the pinned sha256:dc1546ee… is an older rebuild. This is pre-existing (the python pin predates this PR) and is the intended behavior of pinning — but it interacts with the apt-get upgrade -y at agent/Dockerfile:47, whose own comment explains it exists because "system-library CVEs ride the base python:3.13-slim tag until upstream rebuilds." Upstream has rebuilt; the pin now holds us on the older base and shifts the CVE-patching burden entirely onto the in-image apt-get upgrade. That is a legitimate trade, but worth a Dependabot-cadence check (see #3). The golang:1.26.4-bookworm and uv:0.11.14 digests I verified are still current for their tags.

  3. Digest freshness has no owner. .github/dependabot.yml does configure package-ecosystem: docker over directories: ["**/*"] weekly with a 7-day cooldown, so Dependabot should propose digest refreshes — good. Worth a follow-up issue to confirm empirically that it actually opens a PR against a tag@sha256: ref in agent/Dockerfile (note open-pull-requests-limit: 1 for the whole all-docker group). The new test guards against un-pinning; nothing guards against pins rotting, and rot is now the failure mode with teeth given nit #2.

  4. Parser blind spots in the new test (cdk/test/constructs/agent-image-pins.test.ts:28-42). I executed the parser logic against the real Dockerfile — 4 refs found, all 4 pass both assertions, and the stage-name exclusions (mise, gh-builder) work. I then probed it for false negatives, i.e. shapes where an unpinned image would slip through green:

    • COPY --chown=… --from=ghcr.io/x:1.0missed (the regex requires --from= to immediately follow COPY ; any intervening flag defeats it). Same for COPY --link --from=….
    • lowercase from python:3.13-slimmissed (no i flag; Dockerfile keywords are case-insensitive).
    • FROM ${BASE} with ARG BASE=python:3.13-slimmissed (no ARG expansion).
    • Multi-flag FROM --platform=… --foo img → missed.
      Every miss is a false negative (silent pass), which is the failure class the third test was written to defend against — it defends against "matched nothing at all," not "matched 4 of 5." Cheap hardening: case-insensitive anchors and ^COPY\s+(?:--\S+\s+)*--from=. Not blocking: none of these shapes exist in the file today, and the >= 4 + per-image-name assertions mean the current four cannot silently drop out.
      Note also that refs).not.toContain('mise') at line 84 is weaker than it reads — the /[/:@]/ filter already makes a bare mise unrepresentable in the output, so that assertion can never fail. Harmless, but it is not testing what its comment claims.
  5. Placement. cdk/test/constructs/ is a slightly odd home for a test whose subject is agent/Dockerfile and which instantiates no construct. There is precedent for the cross-package reach (cdk/test/handlers/shared/builtin-policies.test.ts:28 reads agent/policies/, cdk/test/constructs/ecs-agent-cluster.test.ts:54 points a DockerImageAsset at agent/), and putting it in cdk/ is defensible since cdk/src/stacks/agent.ts:660 is what consumes the Dockerfile. cdk/test/contracts/ would fit better — it is a cross-language contract assertion, same family as cedar-parity / pdf-parse-bundling. Cosmetic.

5. Documentation

  • Required docs: none missing. The change alters no contract, env var, command, or user-facing behavior. The digest-bump procedure is documented inline at agent/Dockerfile:20-21 (docker buildx imagetools inspect <image>:<tag>), which I verified is the correct invocation and the right place for it — a contributor editing a pin reads the file, not a guide.
  • Starlight mirror: N/A. No edits under docs/guides/, docs/design/, or CONTRIBUTING.md, so mise //docs:sync is not required and there is no stale-mirror mutation risk.
  • AGENTS.md: no update needed — routing and commands are unchanged, and agent/AGENTS.md:86 already flags base-image changes as ask-first.
  • Issue tracking: #715 exists, approved, and is closed by this PR. It carries no priority label (P0/P1/…) — minor gap against Stage 4's tracking requirement, worth adding for consistency but not worth blocking a governance-cleared change over.

6. Tests & CI

  • Coverage is proportionate and the assertions are honest. Four tests: digest presence, tag-alongside-digest shape, a parser-liveness floor (>= 4 refs plus per-image-name checks) so a green run cannot mean "matched nothing," and a stage-name-exclusion negative. I re-implemented the parser and ran it against the real agent/Dockerfile: all four refs found and passing. This is AI005-correct (asserts what the code should do) and AI001-clean (reads the real Dockerfile, not a fixture). Author states each assertion was confirmed to fail against a deliberately broken variant, including the exact shape of the original bug — consistent with what I see. Gaps are the false negatives in nit #4.
  • Test performance (#366): no concern. The test does one readFileSync at module load, never calls new App() or Template.fromStack(), and does not touch aws:cdk:bundling-stacks, so test/setup/disable-bundling.ts is unaffected.
  • CI: all 8 checks green at c2b66188 — Analyze ×3, CodeQL, Dead-code (advisory), "Secrets, deps, and workflow scan", Validate PR title, build (agentcore) 12m30s. Notably the secrets/deps scan is green, corroborating that the advisory work is now on main rather than in this PR. mergeStateStatus: BLOCKED is REVIEW_REQUIRED (my prior review was dismissed), not a failing check.
  • Base freshness: merge-base is e1dec8d1; origin/main has since advanced to df1ebac6 (+3 commits: #345 UA attribution, #718 pin-drift guard, #717 resolutions prune). None touch agent/Dockerfile; the three that touch cdk/test/constructs/ touch different files. No conflict (mergeable: MERGEABLE) and no semantic interaction — the new check:transitive-pin-sync guard from #718 concerns npm lockfiles, which this PR no longer modifies. Per AGENTS.md, run mise //cdk:eslint + mise //cli:eslint and commit any auto-fixes after the next main merge so "Fail build on mutation" stays green.
  • Bootstrap synth-coverage: not applicable. No constructs, stacks, or handlers changed; no new CloudFormation resource types. No cdk/src/bootstrap/policies/*, resource-action-map.ts, BOOTSTRAP_VERSION, or DEPLOYMENT_ROLES.md update is required (ADR-002).
  • Unverified claim, acknowledged by the author: that a real cdk deploy now skips the image push end to end. The layer-identity evidence is direct; the deploy-level win is inference. Correctly disclosed — I am not treating an honestly-flagged inference as a defect.

7. Review agents run

Process disclosure: this review executed as a headless subagent with no Agent/Task dispatch tool exposed (ToolSearch for Agent/Task/pr-review-toolkit:* returns nothing), so the pr-review-toolkit agents could not be spawned as isolated subagents. Rather than hand-wave, I read each agent's definition from ~/.claude/plugins/cache/claude-plugins-official/pr-review-toolkit/unknown/agents/*.md and applied its own published rubric to the diff, plus independent empirical verification (live registry manifest resolution; executing the test's parser against the real Dockerfile and against adversarial probes). Findings per rubric:

  • code-reviewer (confidence-≥80 rubric) — Ran. No findings at ≥80. License header matches sibling tests verbatim; node: import prefixes are idiomatic for this tree; no AGENTS.md rule violated. The not.toContain('mise') tautology (nit #4) scores ~55.
  • silent-failure-hunter — Ran. No production error-handling code in the diff. Its concern maps onto the test's own silent-pass risk, which is the crux of nit #4: the parser's false negatives are exactly the "green means nothing was checked" class this agent exists to catch, and the >= 4 floor only partially closes it. No hidden fallbacks; no broad catch; readFileSync is deliberately unguarded so a missing Dockerfile fails loudly.
  • type-design-analyzer — Ran, minimal surface. One local helper externalImageRefs(): string[]; no new type, interface, or domain model. Nothing to rate.
  • comment-analyzer — Ran, and this is where I disagree most with the diff. Two accuracy problems: (a) agent/Dockerfile:20-21 says "Keep the tag alongside the digest; it documents what the digest is meant to be" while line 22's tag is latest, which documents nothing — the comment's own standard is not met by the line it annotates (nit #1). (b) the comment at cdk/test/constructs/agent-image-pins.test.ts:34-35 asserts a COPY --from=<stage> has "no slash, no colon, no digest," which is true of this Dockerfile but is not a Docker guarantee. The ~400 MB apt install and multi-platform/arm64 claims I verified as accurate (all four digests are OCI image indexes; mise and uv both include linux/arm64, golang and python include linux/arm64/v8). Volume is high for a 28-line change — the top-of-file block duplicates the PR body and is re-duplicated in the test comments — but for a file whose failure mode is "someone unpins this in two years," I'd rather over-document than under.
  • pr-test-analyzer (criticality 1-10) — Ran. Coverage matches the change; no critical gap. Highest-value additions: case-insensitive FROM/COPY anchors and (?:--\S+\s+)* before --from= (criticality 4 — closes the silent-pass shapes in nit #4); replacing the tautological not.toContain('mise') with a fixture-string assertion (criticality 2).
  • /security-review — Ran as a lens; skipped as a formal skill invocation because the diff touches no IAM, Cedar, network, secrets, or input-gateway surface (verified: no cdk/src/ change at all, no policy files, no handler). Net-positive supply-chain hardening: it removes an unreviewed upstream write path into the agent runtime image (mise:latest). I did the security work that mattered here empirically instead — resolving all four digests against Docker Hub / GHCR to confirm they are real, multi-platform, and arm64-capable rather than trusting the PR body.
  • Omitted entirely: none.

8. Human heuristics

  • Proportionality — Pass. 24 added Dockerfile lines (mostly rationale) plus one 87-line focused test. No new abstraction, factory, or engine; the guard is a regex over a file, which is the right weight for the problem.
  • Coherence — Pass, now that the fix(deps) work has landed via #711 on main. The PR does one thing and its body describes that one thing. Minor: cdk/test/constructs/ is not the most coherent home for a Dockerfile contract test (nit #5).
  • Clarity — Concern, minor: agent/Dockerfile:22. mise:latest@sha256:… is self-contradictory against the comment three lines above it, and the tag that digest actually corresponds to (2026.8.1) is knowable and would communicate intent (nit #1).
  • Appropriateness — Pass. Maintainable by this team; the verification is measured (18/18 cached, 0/21 layers differing, stable CDK asset hash) rather than asserted, and the one thing not verified end-to-end is explicitly called out in the body. That is the standard I want on this repo.

Summary: prior blockers cleared and independently re-verified — #715 is approved, and the head no longer carries the lockfile bumps. All four digests confirmed real, current-or-intentionally-pinned, and arm64-capable. Approving; the one thing I'd fix on the way past is mise:latest@sha256:…mise:2026.8.1@sha256:…, since that digest is exactly tag 2026.8.1 and latest has already moved to 2026.8.2.

@scottschreckengaust
scottschreckengaust added this pull request to the merge queue Aug 5, 2026
Merged via the queue into main with commit a94be94 Aug 5, 2026
9 checks passed
@scottschreckengaust
scottschreckengaust deleted the fix/pin-agent-image-digests branch August 5, 2026 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pin every base image by digest

2 participants