Skip to content

fix: generate CI with a Node patch that rejects compromised keys - #121

Merged
joshavant merged 1 commit into
mainfrom
security/emudhra-node-runtime
Oct 22, 2026
Merged

joshavant merged 1 commit into
mainfrom
security/emudhra-node-runtime

Conversation

@joshavant

Copy link
Copy Markdown
Contributor

What Problem This Solves

Generated plugin CI, supplied CI examples, and this repository’s workflows can select Node runtimes that accept the compromised eMudhra keys.

User Impact

Newly generated workflows select Node24.22.1. Existing supported Node majors select their fixed patches; the hydration workflow also selects native pnpm12.15.0, whose independent TLS verifier rejects the affected keys. Existing generated workflows and installed consumer runtimes require their own updates.

Why This Change Was Made

Update the existing generator and all supplied examples together, including immutable GitLab and CircleCI images. Keep the public engine compatibility range and existing release gates.

Evidence

The real CLI initialization regression fails on the base and passes with this change. npm run check, package contents, workflow actionlint, and git diff --check pass in the isolated Linux arm64 worker. Independent review is clean through P2. Official Node and native pnpm releases were independently qualified against all disclosed incident keys, including explicit old-CA modes and positive controls; the CircleCI image’s Node binary matches the qualified release. Native macOS/Windows and provider-owned runner/action runtimes remain separate coverage obligations. Package publication and Crabpot’s source-pin followthrough are separate release gates; this PR does not publish a new package.

@joshavant
joshavant requested a review from a team as a code owner October 21, 2026 22:15
@clawsweeper

clawsweeper Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 21, 2026
@clawsweeper

clawsweeper Bot commented Oct 21, 2026

Copy link
Copy Markdown

Codex review: needs changes before merge.

What this changes

This PR pins generated CI, supplied examples, and repository workflows to patched Node releases and updates the hydration pnpm version.

Example: Run plugin-inspector init --ci --package-manager pnpm.

  • Before: The generated workflow requests Node 24 without a patch version.
  • After: The generated workflow requests Node 24.22.1.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The patch is narrow and source-consistent, with focused regression coverage; reported runtime qualification lacks inspectable artifacts.
Proof confidence 🦐 gold shrimp (3/6) The supplied maintainer-authored classification places this PR outside the ordinary contributor-proof gate; its body reports Linux arm64 CLI and runtime qualification, while inspectable support consists of the focused CLI regression and successful checks.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Supplied CI configurations can select runtimes whose certificate verification accepts compromised intermediate keys.
Reason: The supplied maintainer-authored classification establishes direction, and the narrow patch updates the existing runtime-selection owners without expanding the public contract.

Merge readiness

⛔ Needs changes before merge - 1 item remains

Keep this PR open: it makes a focused security update that is absent from main, and inspection found no introduced correctness defect.

Priority: P1
Reviewed head: e1d8632b2fb39c942d6cb8b60208a97466b168c3

Before merge

  • Complete next step - Run Crabpot's source-mode plugin-inspector smoke against this change and assess any needed pin/docs followthrough before calling the behavior update complete, as required by AGENTS.md.

Findings

None.

Tests

  • Missing end-to-end proof: The reported standalone CLI and TLS qualification runs have no attached output, and the repository-required Crabpot smoke is not supplied.
Agent review details

How this fits together

The init CLI renders a GitHub Actions workflow for plugin authors; repository workflows and checked-in CI examples separately select runtimes for installation and inspection.

flowchart LR
  A[Plugin author] --> B[Init CLI]
  B --> C[Workflow renderer]
  C --> D[Generated workflow]
  E[Supplied CI examples] --> F[Patched Node runtime]
  D --> F
  F --> G[Plugin inspection]
Loading

Technical review

Best possible solution:

Land the focused runtime-selection update after the required consumer smoke, keeping publication and existing consumer-runtime updates in their documented release paths.

Do we have a high-confidence way to reproduce the issue?

Source establishes the old runtime selections and the exact generated-output change; official Node changelogs confirm the selected patches contain the key-blocking fix, but no target code or TLS reproduction was run during this review.

Is this the best way to solve the issue?

Updating the existing renderer and corresponding examples is the smallest complete repository-level repair, while preserving package-manager preparation, supported engine ranges, and consumer overwrite protection.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against c9f7110819b2.

Provenance checked

  • Generated workflow runtime changes intended behavior with a stated reason (0fc9697: The plugin-root CLI work introduced generated CI using Node 24; later action and package-manager repairs retained that selection.)
  • Repository CI and hydration toolchain changes intended behavior with a stated reason (6a75c55: Existing workflows selected supported Node majors, while the hydration refresh selected pnpm 12.5.1; this PR explicitly replaces those selections for the disclosed certificate-key incident.)
  • Supplied CI examples changes intended behavior with a stated reason (c89fb0b: The examples demonstrate plugin inspection and report collection; patched runtime versions and immutable image references preserve those purposes.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

  • Exact introduced change: The pinned diff changes runtime selections and adds one assertion to the existing CLI initialization regression; it adds no dependencies, permissions, execution hooks, or parallel implementation. (src/init.js:114, e1d8632b2fb3)
  • Current main and existing workflows: Main emits node-version: 24. Existing workflow files remain protected by the unchanged refusal to overwrite without --force, so this update does not migrate existing consumer workflows. (src/init.js:25, c9f7110819b2)
  • Upstream security contract: The official v24.22.1 and v22.23.4 changelogs include the blocked-public-key verification fix from tls: reject chains containing blocked public keys nodejs/node#66871. That merged PR describes rejecting chains containing the eight compromised intermediate keys. This contract applies because the patch selects these Node runtimes. (d9a3f3d150f7)
  • Repository-required consumer validation: AGENTS.md requires the Crabpot smoke for behavior changes. The PR explicitly leaves source-pin followthrough to a separate gate and supplies no Crabpot smoke result. The analogous generator repair at fix: prepare package managers in generated CI workflows #93 ran that smoke and established that workflow generation required no pin/schema migration. (AGENTS.md:12, e1d8632b2fb3)
  • Discussion and validation: The complete body reports a base-failing/head-passing CLI regression and Linux arm64 qualification, but attaches no terminal trace or qualification artifact. Both comments are review-status messages. Exact-head checks show six successes and two skipped dispatch jobs. (test/cli.test.js:401, e1d8632b2fb3)
  • Feature history and inspection limit: Scoped history identifies Vincent Koc's original generator work and Peter Steinberger's action refresh and package-manager repair. Some historical blob reads failed because lazy fetching is disabled; current source, pinned diff, supplied provenance, commit metadata, and current-main blame remained readable. No matching maintainer notes, nested AGENTS.md, or pr-simplify skill were present. (src/init.js:115, c9f7110819b2)

Likely related people:

  • Peter Steinberger: Raw commit 6b7bad3 adds src/init.js:115 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 6b7bad3d0c1d; files: src/init.js)

Labels

Label changes:

  • add P1: The update selects security releases that reject known compromised certificate keys in CI runtimes.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

Label justifications:

  • P1: The update selects security releases that reject known compromised certificate keys in CI runtimes.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Attach the existing CLI and certificate-rejection qualification output, including positive controls, to make the reported security verification inspectable.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 21, 2026, 6:28 PM ET / 22:28 UTC.

@joshavant
joshavant merged commit 325f706 into main Oct 22, 2026
12 checks passed
@joshavant
joshavant deleted the security/emudhra-node-runtime branch October 22, 2026 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant