[antigenius0910]

What problem does this solve?

Every Node.js-based agent image uses a floating FROM node:22-trixie-slim (kimi: node:24-trixie-slim). The repo does not record which Node.js version an image ships, and a Node.js security release reaches users only when an image happens to be rebuilt. #1567 measured stable-* on Node.js 22.23.1 and beta-* on 22.23.3, both behind the October 2026 security release (22.23.4 / 24.22.1). The maintainers decided to pin the version with a build ARG and keep nightly floating.

Part of #1567 (step 3, first PR). Merge order: #1570, then #1571, then #1572.

Discord Discussion URL: https://discord.com/channels/1491295327620169908/1491365158868619404/1562955032012398712

Review Contract

Goal

Every release, pre-beta, and per-agent Dockerfile builds its Node.js base from one explicit full version per major. Changing that version takes a reviewed one-line bump per file. CI fails if an image's node --version differs from the pin, or if the pin differs between files.

Non-goals

• Cutting a release or rebuilding published images. Maintainers do that in steps 1 and 2 of RFC: rebuild agent images for Node.js security releases (Node 22.23.4 / 24.22.1) #1567.
• Detecting new Node.js security releases. The companion PR adds node-security-check.yml for that.
• Pinning by digest. A version tag keeps Debian package fixes on rebuild, as weighed in RFC: rebuild agent images for Node.js security releases (Node 22.23.4 / 24.22.1) #1567.

Accepted Residual Risks

• The upstream node:<version>-trixie-slim tag is re-published by Docker for Debian updates, so a rebuild is not byte-reproducible. The Node.js version is. This is the intended trade-off.
• Nightly does not assert the Node.js version. It floats on purpose and serves as the canary.
• Ten Dockerfiles each declare the ARG. The new assert_line in docker-smoke-test.yml keeps the values identical, so a partial bump fails CI rather than shipping mixed versions.

Acceptance Criteria

• git grep -n 'FROM node:' -- 'Dockerfile*' shows only node:${NODE22_VERSION}-... or node:${NODE24_VERSION}-....
• Docker Smoke Test and Docker Smoke Test (Unified) pass, including the new "Verify Node.js runtime version" step for claude, codex, copilot, gemini, kimi, mimocode, opencode, and pi.
• validate-packaged-pins passes with the new NODE22_VERSION and NODE24_VERSION assert_line checks.
• The nightly build-args contain NODE22_VERSION=22 and NODE24_VERSION=24.

Follow-ups

• node-security-check.yml (companion PR) and the RELEASING.md runbook plus SECURITY.md (companion PR).
• An optional warn-only step that reports nightly's Node.js version in the job summary, if maintainers want the canary result visible there.

At a Glance

ARG NODE22_VERSION=22.23.4 ──► FROM node:${NODE22_VERSION}-trixie-slim AS base-node ──► claude, codex, copilot,
ARG NODE24_VERSION=24.22.1 ──► FROM node:${NODE24_VERSION}-trixie-slim AS kimi          gemini, mimocode, opencode, pi

release / pre-beta / snapshot ── use the ARG default (pinned)
nightly ── --build-arg NODE22_VERSION=22 NODE24_VERSION=24 ──► node:22-trixie-slim / node:24-trixie-slim (floating canary)
smoke tests ── node --version == v<ARG default> ; assert_line keeps the ARG identical in every Dockerfile

Prior Art & Industry Research

Not applicable. This is a build and CI change to how the base image version is declared. It does not change agent runtime behavior.

Proposed Solution

• Declare global ARGs NODE22_VERSION=22.23.4 and NODE24_VERSION=24.22.1 before the first FROM in Dockerfile.unified, Dockerfile.package, and the per-agent Dockerfiles (claude, codex, copilot, gemini, mimocode, opencode, pi, kimi). Each file declares only the ARG it uses.
• docker-smoke-test.yml and docker-smoke-test-unified.yml gain a runtime step that compares docker run --entrypoint node <image> --version with the pinned ARG. After review round 1, .github/scripts/node_base_arg.sh resolves that ARG from the stage's base image instead of a hard-coded agent list. Non-Node images skip the step, and an unpinned FROM node: fails it.
• validate-packaged-pins checks that every Dockerfile built FROM node:${NODE<n>_VERSION} carries the same pin as Dockerfile.unified. A Node bump therefore edits only Dockerfiles.
• Nightly writes the Node.js version it actually shipped to the job summary. This is warn-only.
• nightly-build.yml appends NODE22_VERSION=22 NODE24_VERSION=24 to the resolved build-args, so node:${NODE22_VERSION}-trixie-slim resolves to the floating node:22-trixie-slim.
• docs/opencode.md named node:22-bookworm-slim. It now describes the pinned trixie base.

Why this approach?

A full-version ARG puts the shipped Node.js version in the diff and makes the security bump a reviewable PR. Because the major alone is also a valid tag, nightly floats with a single build-arg and no second Dockerfile. The maintainers chose this in #1567.

Alternatives Considered

• Keep floating. Rejected in RFC: rebuild agent images for Node.js security releases (Node 22.23.4 / 24.22.1) #1567. Nothing records the version, and nothing triggers a rebuild.
• Pin by digest. Rejected in RFC: rebuild agent images for Node.js security releases (Node 22.23.4 / 24.22.1) #1567. Every Debian rebuild of the tag would need a bump, and the Node.js version would still not be visible.
• Dependabot for the base image. It cannot parse an ARG-templated FROM, and the gap is the release step. RFC: rebuild agent images for Node.js security releases (Node 22.23.4 / 24.22.1) #1567 has the details.

Validation

• docker buildx build --check on all 10 edited Dockerfiles. Warning counts match upstream main (only the existing SecretsUsedInArgOrEnv).
• Dockerfile.unified --target base-node built and printed v22.23.4. With --build-arg NODE22_VERSION=22 it resolved node:22-trixie-slim.
• Dockerfile.package --target kimi built with a stub bins context and printed v24.22.1. With the nightly args it resolved node:24-trixie-slim.
• The new smoke-test run: blocks were extracted from the YAML and run against the built images. claude and kimi, per-agent and unified, passed. A forced mismatch (ARG 22.0.0, 24.0.0) failed with ::error::Node.js version mismatch: expected v22.0.0 (NODE22_VERSION), got v22.23.4. A missing ARG failed with ::error::Failed to extract NODE24_VERSION from Dockerfile.kimi.
• The extracted validate-packaged-pins script passed as committed and failed after Dockerfile.pi was changed to 22.23.3.
• actionlint 1.7.7 on the three edited workflows reports no new findings. The five SC2016 notes predate this PR.

🤖 Generated with Claude Code

[antigenius0910]

Thanks for the review. 5ae1753 covers all three findings (F1, F2, F3), so I kept it as the round-1 fix. I had a parallel fix in progress and dropped it in favor of yours, so the branch carries a single implementation.

• Merge order. chore(docker): pin Node.js base image via NODE22_VERSION/NODE24_VERSION #1570, then chore(ci): add daily node-security-check workflow #1571, then docs(release): add SECURITY.md and base image security runbook #1572. Each PR body now states this order. chore(ci): add daily node-security-check workflow #1571's PR-time dry-run job fails until this PR is on main, which enforces the order.
• Nightly build-arg duplication. Left as is. No resolver emits NODE2x_VERSION today, so a duplicate key cannot occur.
• "Three reasons we might not need this PR". The pin makes the shipped Node.js version visible and reviewable. chore(ci): add daily node-security-check workflow #1571 provides the patch-uptake signal that floating used to give implicitly. Renovate stays a possible follow-up.

[antigenius0910]

Round 2, F1: 1f1fec4 removes the two hard-coded assert_line 'ARG NODE2x_VERSION=...' lines. The derived loop is now the only parity check, and its comment is updated. I ran the extracted step script on three cases. The tree as committed passes. Bumping all nine NODE22_VERSION Dockerfiles to 22.23.5 passes. Bumping only Dockerfile.pi fails. actionlint shows the same six findings as before, all pre-existing SC2013 and SC2016.

#1571 is already on main, so its scheduled run (07:47 UTC) fails until this PR merges.

[antigenius0910]

Round 3, F1: f18717d applies option 2. node_base_arg.sh --all <dockerfile> checks every FROM line, skips --platform, matches FROM in any case, prints the NODE ARGs in use, and fails on any unpinned node: base. The parity gate runs it on every tracked Dockerfile, including those in subdirectories, so it has no regex of its own. The file list is assigned before the loop, so a git failure stops the step. Before this change, a failing git ls-files in the process substitution skipped every check. Inline fixtures cover FROM --platform=$TARGETPLATFORM node:22-..., from node:22-..., and a pinned --platform base.

I extracted the step and ran it in ubuntu:24.04. It passes as committed and after a full bump. It fails on a partial bump, a missing unified pin, the two reported Dockerfile.package forms, an unpinned openshell/Dockerfile base, and outside a git repo. Default resolver output is unchanged: claude, pi → NODE22_VERSION, kimi → NODE24_VERSION, kiro, cursor → none. actionlint shows two more SC2016 infos, from the new fixtures' intentional literal $TARGETPLATFORM / ${NODE22_VERSION}.

[chaodu-agent]

Note

LGTM ✅ - Round 3 F1 is fixed. node_base_arg.sh is now the only Dockerfile parser for Node.js bases, and it fails closed on every unpinned or unresolvable form found during review. A 39-fixture regression suite runs in CI, and all checks pass.

What This PR Does

Node-based agent images were built from floating node:22-trixie-slim / node:24-trixie-slim tags, so nothing recorded which Node.js version shipped. This PR pins NODE22_VERSION=22.23.4 and NODE24_VERSION=24.22.1 as global ARGs in every Node-based Dockerfile. Nightly overrides them with the major version only and keeps floating as a canary. CI verifies the runtime version and that the pins match across files (#1567, step 3). It also provides the pins that the already-merged node_security_check.sh (#1571) reads.

How It Works

• .github/scripts/node_base_arg.sh is the only Dockerfile parser for Node.js bases. It joins \ continuation lines as Docker does, then classifies every FROM in the file:
  • A stage reference inherits the class of the referenced stage. Stage names are compared case-insensitively.
  • An image whose repository name is node, under any registry or namespace, must be written as node:${NODE<n>_VERSION}-<suffix>. Anything else fails. A missing tag suffix has its own error message.
  • A FROM ${VAR} base is allowed only for allowlisted build args (BUILDER_IMAGE), and the arg's declared default is classified as well. Any other build arg inside an image reference fails, because it cannot be resolved statically.
  • A FROM with no image, and an # escape= parser directive, are rejected.
• node_base_arg.sh <dockerfile> [target] prints the ARG a stage builds on. Both smoke-test workflows use it to decide which images get the runtime node --version check, and nightly uses it to report the version that actually shipped.
• node_base_arg.sh --check <files...> validates every stage and prints the pins each file uses. validate-packaged-pins runs it over every tracked Dockerfile, including nested ones. It then requires each pin to equal the one in Dockerfile.unified, requires that pin to be a full x.y.z, and fails if no Node-based Dockerfile is found.
• .github/scripts/test_node_base_arg.sh holds 39 fixtures plus the current repository's target mapping, and runs in validate-packaged-pins.
• docker-smoke-test.yml now also triggers on '**/Dockerfile*', so a PR that only changes a nested Dockerfile still runs the parity check.

Findings

#	Severity	Finding	Location
1	🟢	Round 3 F1 fixed: one fail-closed parser shared by the parity gate, both smoke tests, and the nightly report	.github/scripts/node_base_arg.sh
2	🟢	Parity gate verified fail-closed: a Dockerfile-only bump passes, while a partial bump, a bare-major pin, a missing pin, or an unpinned base in any form fails with a file-specific message	.github/workflows/docker-smoke-test.yml (validate-packaged-pins)
3	🟢	Regression suite (39 fixtures plus repository targets) runs in CI; the repository-target checks also assert the exit status, so a crash cannot pass as "not Node-based"	.github/scripts/test_node_base_arg.sh
4	🟢	Trigger coverage matches scan coverage: '**/Dockerfile*' added to the smoke-test paths	.github/workflows/docker-smoke-test.yml:7-10
5	🟢	Round 1 and round 2 fixes still hold: derived runtime check, nightly Node version record, no hard-coded version literals in the workflow	-
6	🟢	CI green at f44623d (40 check runs: 36 passed, 4 skipped by design), including validate-packaged-pins, all smoke-test and unified smoke-test jobs, and the #1571 Node security check dry run	-

Finding Details

🟢 F1-F2: Unpinned and unresolvable Node.js bases now fail closed

Round 3 found that the parity gate used its own ^FROM node: regex, so some forms passed silently. Review of the fix found more such forms. All of the following now exit non-zero in both --check and target mode, and each has a fixture:

Form	Example
Literal tag or digest	node:22-trixie-slim, node@sha256:..., node
Flags and case	FROM --platform=$TARGETPLATFORM node:22-..., from node:22-...
Any registry or namespace	docker.io/library/node:22, index.docker.io/..., mirror.gcr.io/library/node:22, public.ecr.aws/docker/library/node:22
Hidden behind a stage	FROM node:22-slim AS Base then FROM base
Build-arg base or composite	FROM ${BASE}, FROM $BASE, ${BASE:-node:22}, $BASE-node, node${TAG}-slim, ${IMG}:22, ${REG}/node:..., node:${TAG}
Allowlisted arg with a Node default	ARG BUILDER_IMAGE=node:22 then FROM ${BUILDER_IMAGE}
Continuation and directives	FROM \ followed by node:22-... (including comment lines inside the continuation), FROM with no image, # escape=
Missing tag suffix	node:${NODE22_VERSION} (reported as "has no tag suffix")

The pinned forms still resolve: node:${NODE22_VERSION}-trixie-slim (also behind a registry prefix, a digest, --platform, or a continuation), and FROM ${BUILDER_IMAGE}, whose default local_builder is a stage. Every current repository Dockerfile passes --check.

The validate-packaged-pins step was extracted and run locally at the final tree:

Scenario	Result
As committed	rc=0
All 9 NODE22_VERSION Dockerfiles bumped to 22.23.5 (runbook path)	rc=0
Only Dockerfile.pi bumped	rc=1, Dockerfile.pi must declare 'ARG NODE22_VERSION=22.23.4' to match Dockerfile.unified
All pins set to bare major 22	rc=1, Dockerfile.unified must pin a full Node.js version
ARG NODE22_VERSION removed from Dockerfile.unified	rc=1
Dockerfile.package base rewritten in each unpinned form above	rc=1, with the line annotated

🟢 F3: Regression suite

test_node_base_arg.sh covers positive resolution, every fail-closed form above, --check output, and the exact target mapping of Dockerfile.unified and Dockerfile.package. That mapping must match the old hand-written allowlist: claude, codex, copilot, gemini, mimocode, opencode and pi resolve to NODE22_VERSION, kimi resolves to NODE24_VERSION, and the 7 non-Node targets resolve to none. It passes under mawk 1.3.4, the runner's default awk. The CI log for validate-packaged-pins at f44623d shows all 39 fixtures, the repository targets, and "all node_base_arg.sh tests passed".

🟢 F6: CI

At f44623d, every check is green: validate-packaged-pins, all smoke-test and smoke-test-unified jobs including the runtime Node.js version step, CI, Review Contract, PR Discussion URL Check, and the #1571 Node security check dry run.

Non-blocking Notes

• The base-image policy covers non-Node images too. Rejecting unresolvable build args also rejects forms like FROM debian:${DEB}. No Dockerfile uses such a form today. If one is ever needed, extend the allowlist and say so in the script header.
• Known false-error cases. A FROM line inside a RUN heredoc, a blank line inside a \ continuation (which BuildKit warns about as NoEmptyContinuation), and an # escape= comment anywhere in the file are all rejected. Each can only add an error, never hide a Node.js base, and none occurs in the repository.
• Optional runtime backstop. The smoke tests could also fail when a supposedly non-Node image has node on PATH. Whether any current non-Node image does was not verified, so this is left as a follow-up. The static gate is the control that covers Dockerfile.package, which has no PR-time runtime test.
• Nightly build-arg duplication. As discussed after round 1, no resolver emits NODE2x_VERSION, so no duplicate key can occur today.
• Nightly report cost. The report step pulls each Node-based image. docker buildx imagetools inspect on the digest can read the base image's NODE_VERSION env without a pull.
• docs/opencode.md placeholder. Keeping node:<NODE22_VERSION>-trixie-slim avoids one more location to edit on every bump.
• Late non-blocking observations at this head. node:${NODE22_VERSION}-${NODE24_VERSION} passes and resolves to the first ARG; it cannot be exploited, because no such tag exists and the build fails. Continuation lines are joined with a space, whereas Docker removes \ and the newline outright, so a token split across lines is reported as unpinned (a false error only). Nested Dockerfiles that are not in the smoke matrix also trigger the full smoke matrix, which costs CI time but does not affect correctness. When the test suite fails, it still prints ok repository targets, although the exit status is correct.
• Follow-ups. chore(ci): add daily node-security-check workflow #1571's node_security_check.sh still parses Dockerfiles with its own regex and could reuse node_base_arg.sh. It is also still unanswered whether the per-agent snapshot images have external consumers; that decides whether they need the pin.

Baseline Check

• PR opened: 2026-10-23
• Head SHA: f44623d2ff633af84ae3ee18e8264764e1f7c1d2. Base: main (not stacked). Merge-base: 8b6bf293. Diff: 16 files, +368 / -17.
• Commits since round 3 (1f1fec4): f18717d (contributor, --all parser mode), then 5cb31c3, b8c46ed, a91ddc8, 08f8ca7 and f44623d. The last five were pushed by the reviewer as maintainer fix-ups on the owner's instruction, and were reviewed again by the full team before this round.
• Main already has: floating FROM node:22-trixie-slim / node:24-trixie-slim, and the merged chore(ci): add daily node-security-check workflow #1571 security check, which expects the ARGs this PR adds.
• Net-new value: an explicit Node.js pin, a derived runtime check, cross-file pin parity enforced by a single fail-closed parser with a regression suite, and a nightly float that records its version.

Addressing External Reviewer Feedback

@thepagent

✅ Approved at f44623d (submitted review). No concerns were raised that need a response.

@antigenius0910 (f18717d)

chore(docker): make node_base_arg.sh the only Node FROM parser (adds --all)

✅ Superseded in 5cb31c3: The direction was the same, a single parser. The --all mode covered --platform and lowercase from, but digest-only, registry-qualified, build-arg and case-mismatched stage bases still passed. 5cb31c3 replaced it with the --check mode described above, rebased on top of f18717d without a force-push. Nothing else in the repository used --all.

The file list is assigned before the loop, so a git failure stops the step. Before this change, a failing git ls-files in the process substitution skipped every check.

✅ Still covered: In the final version, mapfile reads git ls-files through a process substitution, so git's exit status is not checked directly. A failing git ls-files yields an empty file list, node_base_arg.sh --check then exits 2 with a usage error, and the step fails. This was reproduced locally with a stub git that exits 128. Failing on an explicit git ls-files status, with a clearer message, would be a small optional improvement.

@antigenius0910 (earlier rounds)

✅ Round 2 F1 (hard-coded pin assertions) was addressed in 1f1fec4, and that change is preserved. The merge-order and nightly duplication replies were acknowledged in round 3 and remain accepted.

Three Reasons We Might Not Need This PR

1. A different long-term route would retire this machinery. Literal tags with Renovate or Dependabot bump PRs, or a shared node-base image, would remove the ARG layer, the parser, and the parity gate. Rebuttal: RFC: rebuild agent images for Node.js security releases (Node 22.23.4 / 24.22.1) #1567 chose the ARG design so the shipped Node.js version is explicit, and chore(ci): add daily node-security-check workflow #1571 now consumes this ARG contract.
2. The parser is now real CI code to maintain. About 140 lines of awk plus 39 fixtures guard one convention. Rebuttal: each piece closes a demonstrated silent-skip path for Dockerfile.package, which has no other PR-time check, and the fixtures make future changes safe.
3. Pinning slows automatic patch uptake. A floating tag picked up Node.js patches on any rebuild; now a bump is a reviewed edit. Rebuttal: that is the trade-off RFC: rebuild agent images for Node.js security releases (Node 22.23.4 / 24.22.1) #1567 accepted, with chore(ci): add daily node-security-check workflow #1571 detecting stale pins and the release runbook handling the rebuild.