Skip to content

fix(ci): pin Node releases with incident key rejection - #32

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

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

Conversation

@joshavant

Copy link
Copy Markdown
Contributor

Select a fixed Node patch within each existing workflow release line so managed jobs reject the compromised eMudhra keys and cannot reuse an older cached patch.

Changed workflow YAML parsed successfully and the patch passes git diff --check; application source and dependency locks are unchanged. Independent review passed. The official Linux arm64 runtimes rejected all 12 incident chains in eight trust modes, with ordinary HTTPS controls passing. Native platform execution, the Actions runner and installed/deployed products remain separate proof and update obligations.

@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. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. 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 real behavior proof before merge.

What this changes

This PR pins CI, documentation, release, and Windows smoke runtimes to Node versions containing compromised-key rejection.

Example: A workflow runner has Node 24.22.0 cached.

  • Before: The broad Node 24 selector can reuse that older patch.
  • After: The workflow requires Node 24.22.1.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The patch is focused and has passing CI, but the security behavior is asserted without inspectable final-effect evidence.
Proof confidence 🧂 unranked krab (1/6) Real behavior proof is necessary before merge. See Before merge.
Patch quality 🦐 gold shrimp (3/6) Security review found an item that needs attention.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Managed jobs can use a Node runtime lacking rejection of compromised CA keys.
Reason: The maintainer-directed security update preserves existing release lines and narrowly selects patched runtimes.

Merge readiness

⛔ Blocked before merge - 3 items remain

Keep this PR open: the focused security update is absent from main, preserves the supported Node release lines, and introduces no identified code defect.

Priority: P2
Reviewed head: 41fbc4ae9f56b2e423de1621e7e767cee6bd17d4

Before merge

  • Add real behavior proof - Authority-chain proof required: the body reports official Linux arm64 runtimes rejecting incident chains, but supplies no inspectable output showing runtime versions, trusted-peer success, and compromised-intermediate rejection before application HTTPS traffic; attach those existing logs or terminal screenshots and redact credentials, IPs, and non-public endpoints. After adding proof, update the PR body, then request a new review with @clawsweeper re-review. The PR author or someone with repository write access can request it.
  • Resolve security review attention item - No introduced bypass was found, but final-effect evidence for the changed TLS trust boundary remains unverified.
  • Resolve merge risk - Before merge, establish that the selected runtime rejects a peer using a compromised intermediate before application HTTPS traffic while accepting an ordinary trusted peer.

Findings

None.

Tests

  • Missing end-to-end proof: Attach the claimed real-runtime incident-chain rejection and ordinary HTTPS control output; passing CI does not demonstrate that security scenario.
Agent review details

How this fits together

GitHub workflows and the Windows smoke script select Node before installing dependencies and running package, documentation, or publication commands.

flowchart TD
A[Workflow or Windows smoke] --> B[Exact Node version]
B --> C[Runtime cache or official download]
C --> D[Node TLS verification]
D --> E[Dependency and registry HTTPS]
E --> F[Build test or publish]
Loading

Technical review

Best possible solution:

Retain the exact patch pins and attach the claimed runtime rejection evidence.

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

Main's broad selectors can match an older cached patch, and the Windows script explicitly requests Node 24.15.0; no incident-chain reproduction was run during this review.

Is this the best way to solve the issue?

Exact versions use the existing runtime-selection owners and address cached older patches without changing package compatibility or adding machinery.

AGENTS.md: not found in the target repository.

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

Merge-risk options

Maintainer options:

  1. Provide the runtime trust-boundary trace (recommended)
    Attach the existing incident-chain results with runtime identities, an ordinary HTTPS control, and evidence that rejection occurs before application traffic.

Provenance checked

Testing

Proof path: none.

Security

Needs attention: No introduced bypass was found, but final-effect evidence for the changed TLS trust boundary remains unverified.

Evidence

What I checked:

  • Verified introduced change: The pinned comparison contains only six version substitutions across four files; main retains broad workflow selectors and Windows Node 24.15.0. (.github/workflows/ci.yml:20, 41fbc4ae9f56)
  • Current-head validation: All three exact-version CI lanes passed; publication and Pages deployment are separate workflow entry points. (.github/workflows/ci.yml:14, 41fbc4ae9f56)
  • Runtime-selection contract: The changed workflows execute this pinned action; its documentation confirms that check-latest defaults to false and cached semver matches are preferred, explaining why exact patch selectors prevent older-patch reuse. (docs/advanced-usage.md, 820762786026)
  • Upstream security contract: The selected runtime is the security implementation: tls: reject chains containing blocked public keys nodejs/node#66871 adds SPKI rejection through certificate verification, reports CERT_REVOKED, and honors rejectUnauthorized; its tests cover allowed and blocked peers. The official Node 24.22.1 release and Node 26.12.1 release include that change. (src/crypto/crypto_context.cc, d9a3f3d150f7)
  • Proof supplied: The fully read body claims Linux arm64 rejection of 12 incident chains in eight trust modes, but provides no trace or linked artifact; both discussion comments are bot status updates. (41fbc4ae9f56)
  • Feature history and policy search: Main history identifies Peter Steinberger's sustained CI, Pages, and Windows work and Vincent Koc's trusted-release workflow; no AGENTS.md, maintainer notes, or pr-simplify skill exists in the reviewed tree. Some follow-history reads reached unavailable historical blobs; introduction claims rely on the supplied provenance and readable history. (.github/workflows/release.yml:16, 4d585841a512)

Labels

Label changes:

  • add P2: This is a narrow managed-runtime security update; no current user outage or demonstrated exploitation is established.
  • add merge-risk: 🚨 security-boundary: The change selects a different TLS trust policy, and the claimed forbidden-peer rejection has no inspectable runtime trace.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Label justifications:

  • P2: This is a narrow managed-runtime security update; no current user outage or demonstrated exploitation is established.
  • merge-risk: 🚨 security-boundary: The change selects a different TLS trust policy, and the claimed forbidden-peer rejection has no inspectable runtime trace.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Rank-up moves

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

  • Attach the claimed runtime trace showing ordinary HTTPS success and compromised-intermediate rejection before application traffic.

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, 5:56 PM ET / 21:56 UTC.

@joshavant
joshavant merged commit af42c87 into main Oct 22, 2026
14 checks passed
@joshavant
joshavant deleted the security/emudhra-node-release branch October 22, 2026 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant