[antigenius0910]

What problem does this solve?

No job notices when Node.js ships a security release. #1567 found stable-* images behind two consecutive Node.js security releases (22.23.2 in July, 22.23.4 in October). The maintainers asked for a daily check that keeps one labeled issue open while anything is behind, without failing the workflow every day, for both the Node.js 22 and Node.js 24 (kimi) lines.

Part of #1567 (step 3, second PR). Merge order: #1570, then #1571, then #1572. The script reads the NODE<major>_VERSION ARGs from #1570, so the PR-time dry-run job fails on this PR until #1570 is on main. That red check is the merge gate. The issue body links the RELEASING.md runbook anchor from #1572. Until #1572 merges, the link opens the top of RELEASING.md.

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

Review Contract

Goal

Within a day of a Node.js security release, one open issue labeled node-security lists each Node.js pin and each published stable-<agent>/beta-<agent> image that is behind. The same issue is updated while the gap persists and closed when everything is current.

Non-goals

• Bumping pins or cutting releases automatically. A maintainer follows the RELEASING.md runbook (companion PR).
• Tracking non-security Node.js patch releases, Debian CVEs, or agent CLI versions.
• Checking nightly-* or pre-beta-* images, which are not release channels.

Accepted Residual Risks

• The check depends on nodejs.org/dist/index.json and anonymous ghcr.io reads. If either is down, the run fails (red) for that day, and the next run recovers. A broken check is red on purpose, and stale data is not.
• Only linux/amd64 image configs are read. Both architectures are built from the same FROM line, so a mismatch is unlikely.
• A new intermediate Node.js stage whose name does not start with base- would be checked as an image tag and reported not published. It shows up in the table and does not break the run.
• The issue body carries a "Last checked" timestamp, so the body is edited daily while the issue is open. Body edits do not notify.

Acceptance Criteria

• actionlint and shellcheck are clean on the new files.
• DRY_RUN=1 against a pinned Dockerfile reports pins current and today's stable-*/beta-* images stale, and exits 0.
• A Dockerfile without NODE<major>_VERSION ARGs exits 1 with an ::error:: line.
• The create, update, close, and no-op decisions are each exercised with a stubbed gh.
• After merge, a manual workflow_dispatch run on openabdev/openab creates or updates exactly one node-security issue.

Follow-ups

• A row for node-security-check.yml in the RELEASING.md workflow table once both PRs are merged.

At a Glance

daily cron / workflow_dispatch (openabdev/openab only)
        │
        ▼
node_security_check.sh
  ├─ Dockerfile.unified ──► pins {22: 22.23.4, 24: 24.22.1} + Node agents per major
  ├─ nodejs.org/dist/index.json ──► latest security release per major
  └─ ghcr.io configs ──► NODE_VERSION of stable-<agent>, beta-<agent> (404 = not published)
        │
        ▼
 anything stale? × open node-security issue?
   yes × no  ──► create          yes × yes ──► update in place
   no  × yes ──► comment + close no  × no  ──► nothing
 exit 0 in all four; non-zero only if the check itself breaks

Prior Art & Industry Research

Not applicable. This is a repository CI job for release hygiene. It does not change agent runtime behavior.

Proposed Solution

• .github/workflows/node-security-check.yml runs daily at 07:47 UTC and on workflow_dispatch, with contents: read and issues: write. It is guarded with if: github.repository == 'openabdev/openab' so forks stay quiet. A concurrency group prevents a cron run and a dispatch from both creating an issue.
• .github/scripts/node_security_check.sh (bash, curl, jq):
  • It derives each Node.js major and its agents from Dockerfile.unified. FROM base-node AS <agent> inherits base-node's major. A Node.js stage that is not pinned through an ARG is an error.
  • It finds the latest security: true release per major in index.json.
  • It reads NODE_VERSION from each published image config.
  • It reconciles one issue through an explicit four-state decision.
  • It writes the same tables to the job summary.
• A pull_request dry-run job (contents: read, DRY_RUN=1) runs when the script, the workflow, or Dockerfile.unified changes. A parser break fails in its own PR.
• The tracker issue carries a <!-- node-security-check --> marker. Only labeled issues with that marker are updated or closed. An image or pin on a different Node major than its line counts as major mismatch. Every curl call has timeouts.
• issue-check.yml and close-unlinked-stale-issues.yml skip node-security issues. Without that, the stale bot would close the tracking issue after 14 days, and the next daily run would open a new one.

Why this approach?

An issue that stays open and updates itself is visible and actionable. A daily red check gets ignored, as the maintainers noted. Deriving the agent list from the Dockerfile means a new Node.js agent or a new Node.js major is covered without editing the script. Reading image configs through the registry API costs a few small HTTP requests and needs no image pulls.

Alternatives Considered

• Fail the workflow while stale. Rejected by the maintainers in RFC: rebuild agent images for Node.js security releases (Node 22.23.4 / 24.22.1) #1567.
• Dependabot or Renovate. Dependabot cannot parse an ARG-templated FROM and does not detect published images that lag behind. Renovate would add an app dependency for the same partial coverage.
• A hard-coded agent list. It would drift from Dockerfile.unified.

Validation

All runs used ubuntu:24.04 (bash 5.2, jq 1.7) in Docker as a stand-in for ubuntu-latest, against live nodejs.org and ghcr.io data.

• Pinned copy of Dockerfile.unified (22.23.4 / 24.22.1). Both pins are current. Every stable-* image is 22.23.1 stale, every beta-* image is 22.23.3 stale, beta-kimi is 24.21.0 stale, and stable-kimi is not published. The decision is "create or update issue" and the exit code is 0.
• Pin at 22.23.3. The 22 row is reported stale, and the exit code is 0.
• Unmodified upstream Dockerfile.unified. Exits 1 with ::error::no 'ARG NODE<major>_VERSION=<x.y.z>' found in Dockerfile.unified.
• An unpinned extra FROM node:24-trixie-slim AS foo exits 1. No network (--network none) exits 1 after retries.
• With gh stubbed, the decisions came out as follows. Not stale with issue feat: /close slash command to manually terminate a thread session #42 open gives close (#42). Stale with feat: /close slash command to manually terminate a thread session #42 open gives update (#42). Stale with no issue gives create, plus a label create when the label is missing. Not stale with no issue gives none.
• Read-only gh issue list and gh label list against openabdev/openab behave as the script expects.
• shellcheck (stable) and actionlint 1.7.7 are clean, including on close-unlinked-stale-issues.yml.

🤖 Generated with Claude Code

[chaodu-obk]

Suggestion (not a formal review): please widen what the check covers. Two cases this week would not be caught with CHANNELS=(stable beta) and the fixed REPO=openabdev/openab:

1. pre-beta-<agent>: openab:pre-beta-* is still on Node 22.23.3 / 24.21.0, last built 2026-10-03. The cause is the pre-beta schedule gate (fix in ci(pre-beta): gate scheduled builds on main HEAD SHA #1573; a manual rebuild is running). Adding pre-beta to CHANNELS would have flagged it.
2. Downstream ghcr.io/openabdev/openab-pty: every openab-pty image, nightly included, is built FROM openab:pre-beta-<variant>. So openab-pty:pre-beta-claude and openab-pty:nightly-claude (built 2026-10-22) are both on 22.23.3. This would need the registry repo to be a per-row parameter rather than the global REPO. Its channels are pre-beta and nightly, and it has no kimi variant. A tag that is not published already renders as "not published", which fits here.

Both are cheap additions to assess(). If you'd rather keep this PR scoped to the release channels, a follow-up is fine too. Thanks for turning these PRs around so quickly.

[chaodu-agent]

Note

LGTM ✅ - All four round-1 blockers are resolved in d9566724. Merge only in the declared order #1570 -> #1572 -> #1571; the new dry-run check stays red on this PR until #1570 is on main.

What This PR Does

Nothing tells maintainers when Node.js publishes a security release that openab's pins or published images are behind on. #1567 found stable-* images behind two consecutive Node.js security releases. This PR adds a daily workflow that keeps one node-security issue open while any pin or published stable-*/beta-* image is behind, and closes it when everything is current. A behind image does not fail the workflow; only a broken check does.

How It Works

• node_security_check.sh reads the ARG NODE<major>_VERSION pins and Node stages from Dockerfile.unified. Child stages inherit their parent's major, and base-* stages are excluded.
• It takes the latest security: true release per major from nodejs.org/dist/index.json.
• It reads NODE_VERSION from the linux/amd64 config of each stable-<agent>/beta-<agent> image through an anonymous ghcr.io token. A 404 means not published.
• It reconciles a single issue through four states: create, update, close, none.
• New in this round: a dry-run job runs the script with DRY_RUN=1 on pull requests that touch the script, the workflow, or Dockerfile.unified. It has only contents: read and makes no gh calls.
• The reconcile check job keeps the schedule and dispatch triggers, the repository guard, and a job-scoped issues: write.
• close-unlinked-stale-issues.yml skips node-security issues.

Verified at d9566724:

• actionlint 1.7.7 reports no issues on both workflows, and bash -n passes on the script.
• DRY_RUN=1 against chore(docker): pin Node.js base image via NODE22_VERSION/NODE24_VERSION #1570's Dockerfile.unified and live data exits 0:
  • pins 22.23.4/24.22.1 are current;
  • all seven stable-* images are 22.23.1 stale;
  • all beta-* images are current;
  • stable-kimi is not published.
• Against main's Dockerfile the script exits 1 with ::error::.
• The PR dry-run check (run 39889877367) fails for exactly that reason, as intended.

Findings

#	Severity	Finding	Location
1	🟢	Resolved: merge order is now declared in the PR body and enforced by the red dry-run check until #1570 lands	PR body, .github/workflows/node-security-check.yml
2	🟢	Resolved: the #1572 dependency for the RELEASING.md runbook anchor is now declared	PR body
3	🟢	Resolved: PR-time DRY_RUN=1 regression gate with least-privilege permissions	.github/workflows/node-security-check.yml:9-34
4	🟢	Resolved: remediation hint derives majors (NODE[0-9]+_VERSION)	.github/scripts/node_security_check.sh:149
5	🟢	Follow-up suggestions carried from round 1 (non-blocking)	-
6	🟢	Optional: move concurrency to the check job to simplify the workflow-level expression	.github/workflows/node-security-check.yml:18-22
7	🟢	Sound design and correct permission isolation	-

Finding Details

🟢 F1: Merge order with #1570 - resolved

The PR body now has an [!IMPORTANT] block that states "#1570 -> #1572 -> #1571 (this PR last), or all three together" and gives the reason for each dependency. The new dry-run job makes the dependency visible on GitHub. On a merge ref without #1570's ARGs it fails with no 'ARG NODE<major>_VERSION=<x.y.z>' found, and it turns green on re-run once #1570 is on main. A pull_request workflow runs from the PR's merge commit, so the gate is already active on this PR.

🟢 F2: RELEASING.md anchor - resolved

The link to RELEASING.md#base-image-security-release is unchanged. The PR body now declares #1572, which adds that section, as a prerequisite. With the declared order, the link is live before this workflow can create an issue.

🟢 F3: PR-time regression gate - resolved

The trigger, permissions, and concurrency were checked event by event:

• Trigger: pull_request, not pull_request_target, with paths: covering the script, the workflow, and Dockerfile.unified, which is everything the parser reads.
• dry-run permissions: It inherits workflow-level contents: read and sets no GH_TOKEN. The DRY_RUN branch returns before reconcile/ensure_label, so it makes no gh calls.
• check permissions: check holds issues: write at job level and is guarded by github.event_name != 'pull_request' && github.repository == 'openabdev/openab'. No PR event, including one from a fork, can reach it.
• Concurrency:
  • PR runs use node-security-check-pr-<N> with cancel-in-progress: true.
  • Schedule and dispatch runs share node-security-check-reconcile without cancellation, which keeps the original create-race guard.
  • The &&/|| expression short-circuits safely on non-PR events.

Known trade-offs:

• A PR touching Dockerfile.unified now depends on nodejs.org and ghcr.io being reachable. A brief outage fails that PR's check until a re-run, which is acceptable.
• The gate exercises parsing and assessment only. The decide/reconcile paths remain covered by the stubbed-gh validation in the PR description, not by CI.

🟢 F4: Derived majors in the remediation hint - resolved

Line 149 now renders git grep -lE 'ARG NODE[0-9]+_VERSION' -- 'Dockerfile*'. The pattern is not expanded inside the unquoted heredoc. On #1570's tree it matches exactly the 10 Dockerfiles that declare the ARG.

🟢 F5: Follow-up suggestions (non-blocking)

Carried from round 1 for a follow-up PR or issue:

• Cross-major mismatch. Compare ${node%%.*} with the expected major and report major mismatch. Also extend the STALE_MAJOR condition at line 112, or decide still returns none.
• Expected-but-absent tags. Treat a beta-<agent> 404 as missing. A stable-<agent> 404 stays not published. Whether missing opens the issue is a maintainer policy choice. Opening it catches deleted tags, at the cost of a short false positive after a new Node agent is added.
• Timeouts. Add --connect-timeout/--max-time to curl.
• Ownership. Add an ownership marker or --author filter in reconcile, a header comment documenting the ARG/stage/tag/base-* contract, and a LABEL cross-reference to close-unlinked-stale-issues.yml.
• Issue checks. Exempt node-security in issue-check.yml for human edits.
• Simplification. Optional: drop the daily "Last checked" edit, use plain if branches instead of the decide() code concatenation, and label the image table "stable/beta".

🟢 F6: Optional concurrency simplification

Only check touches issues. Moving concurrency: { group: node-security-check } onto that job would replace the workflow-level ternary group: and the cancel-in-progress: expression. The trade-off is that PR runs are no longer cancelled on a new push. Not required.

🟢 F7: What is done well

See "What's Good" below.

Addressing External Reviewer Feedback

chaodu-obk[bot] (issue comment, 2026-10-23)

Widen coverage: (1) add pre-beta-<agent> to CHANNELS, since openab:pre-beta-* is still on Node 22.23.3 / 24.21.0; (2) check downstream ghcr.io/openabdev/openab-pty, which is built FROM openab:pre-beta-<variant>, by making the registry repo a per-row parameter.

ℹ️ Accepted as a follow-up; unchanged from round 1. This PR's Non-goals exclude pre-beta-* and nightly-*. Its goal is met without them. Because openab-pty ships on top of pre-beta, its staleness does reach users, so both additions are worth a follow-up issue linked to #1567 and #1573. They fit the existing design, since unpublished tags already render as not published.

Round 1 -> Round 2 Resolution

Round 1	Status	Resolution
F1 merge order with #1570 not enforced	✅ Resolved	Declared in the PR body and enforced by the red dry-run check
F2 runbook anchor depends on undeclared #1572	✅ Resolved	#1572 declared as a prerequisite in the PR body
F3 no PR-time regression gate	✅ Resolved	pull_request + paths + DRY_RUN=1 job, contents: read only (commit d9566724)
F4 hard-coded majors in remediation hint	✅ Resolved	NODE[0-9]+_VERSION (commit d9566724)
F5-F10 suggestions	Open, non-blocking	Carried as F5 above

Baseline Check

• PR opened: 2026-10-23. Base: main. Head: d95667249bef8e54461fcdd6d893d2886c60e146. Round-2 delta f895ac98..d9566724: 2 files, +26/-4.
• main already has: no Node security check, no node-security label, no ARG NODE<major>_VERSION pins, and no RELEASING.md security runbook.
• Prerequisites: chore(docker): pin Node.js base image via NODE22_VERSION/NODE24_VERSION #1570 adds the pins and ARG-templated FROM lines. docs(release): add SECURITY.md and base image security runbook #1572 adds the RELEASING.md runbook and SECURITY.md. Both are open.
• Net-new value: daily detection that covers both the pins and the published stable-*/beta-* images. Only a published-image check sees stable tags that were re-tagged from an older beta.

What's Good (🟢)

• The check fails closed: set -euo pipefail with inherit_errexit, ::error:: annotations, and a 404-versus-other-error split. A broken check goes red, while a behind image never does.
• Agents and majors are derived from Dockerfile.unified, so new Node agents and majors are covered without editing the script.
• Image configs are read through the registry API, with no image pulls and no stored credentials.
• Permissions are least privilege and improved in this round: contents: read at the workflow level, issues: write only on the guarded check job, and pull_request rather than pull_request_target.
• The issue lifecycle is a small, explicit four-state reconcile, and DRY_RUN and the job summary make it easy to inspect.

5️⃣ Three Reasons We Might Not Need This PR

1. Detection is not remediation. Stable images are still re-tagged rather than rebuilt, so a human must act on the issue. A tracker that stays open for weeks can be ignored.
2. Another source of truth. chore(docker): pin Node.js base image via NODE22_VERSION/NODE24_VERSION #1570's CI assertions, the runbook, and this issue all describe whether Node.js is current. The script is about 226 lines of bash, including a small OCI client.
3. External coupling. The check depends on the ARG convention, nodejs.org, and anonymous ghcr.io reads. A change to any of them turns the check red until someone fixes it.

All three are outweighed by two consecutive security releases that went unnoticed, and by the fact that no off-the-shelf tool sees re-tagged stale images.

[antigenius0910]

Thanks for the review. The branch now has your d956672 (F3 dry-run gate, F4 generic hint) and acf845a on top of it.

Finding	Status
F1 merge order with #1570	Declared in the body: #1570, then #1571, then #1572. The dry-run job reads this branch's Dockerfile.unified, so it stays red until #1570 is on main. That red check is the gate.
F2 runbook anchor from #1572	Declared in the body. Until #1572 merges, the link opens the top of RELEASING.md, which is harmless.
F3 PR-time gate	d956672.
F4 hard-coded majors	d956672. acf845a also states that each Dockerfile declares only its own major and that a bump needs no workflow edit.
F5 cross-major	acf845a. An image or pin on a different major reports major mismatch and counts as behind. Tested with a beta-claude on 24.x: HEAD reported current, the fix reports major mismatch.
F6 missing tags	Deferred to a follow-up, together with the pre-beta-* / openab-pty coverage that chaodu-obk suggested.
F7 curl timeouts	acf845a (--connect-timeout 10 --max-time 30).
F8 ownership and contracts	acf845a. The issue body carries <!-- node-security-check -->, and reconcile acts only on labeled issues with that marker. The label and Dockerfile contracts are documented in the script.
F9 issue-check.yml	acf845a. Tracker issues are skipped. A stubbed run confirmed that without the exemption the tracker gets incomplete/needs-triage and a comment.
F10 simplifications	Not taken in this PR.

Verification of acf845a: shellcheck and actionlint are clean. A DRY_RUN=1 run against a pinned Dockerfile.unified exited 0, reported stable-* as 22.23.1 stale and beta-* as current, and printed the marker. A reconcile test with stubbed gh covered marked, unmarked, and mixed issues.

[antigenius0910]

Correction to my earlier comment: this PR was merged at d956672, before acf845a reached the branch. So F5, F7, F8, and F9 are not on main. They are now in #1574. Merge it after #1570.