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

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
@scottschreckengaust scottschreckengaust added the v1 Version 1 label Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v1 Version 1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pin every base image by digest

2 participants