cherry-pick: Publish TLS state transactionally so a failed rebuild never downgrades below last-good (#2384 → v5.2)

[kriszyp]

Cherry-pick of #2384 onto v5.2: a TLS rebuild pass never publishes a state worse than the one being served. updateTLS builds the whole replacement state into pass-local candidates and reconciles the live maps only after the pass completes; a record that is still in the table but fails to build keeps every live entry it owns and its default candidacy, so a cert renewal whose hdb_certificate write reaches a worker before the matching on-disk key no longer drops that worker to the self-signed default until the 5-minute re-read heals it. A failed pass arms a self-retry with per-signature backoff (1.5 s doubling to 5 min), and loadAndWatch's mtime latch now means "last successfully applied": it rolls back on a synchronous throw or a rejected callback promise, so the periodic poll can heal a lost table write instead of deduplicating it forever.

Why now: on 5.2 today, a renewal whose key lands on a worker before the matching cert makes that worker serve the self-signed certificate until the next 5-minute re-read (#1394); production nodes served the wrong certificate for days with nothing retrying (#2382). The fails-on-base run below reproduces exactly that on unmodified v5.2.

For the human reviewer

1. Design assessment: no new design. This is a backport of a merged, reviewed change; the invariant it enforces (a rebuild pass never publishes below last-good, and only deleting a record drops its contexts) is unchanged from main. The planning gate was not run for that reason, matching the task brief.
2. Two conflict resolutions in security/keys.ts, no extra main commit ported. Five main-side keys.ts commits sit between the v5.2 branch point and Publish TLS state transactionally so a failed rebuild never downgrades below last-good #2384; only two of them reach the squash commit's parent (Canonicalize watch paths so a Windows 8.3 short path cannot abort the process #2309, watch-path canonicalization with polling fallback; and the IPv6 node-identity change to getHost). Neither is a dependency of Publish TLS state transactionally so a failed rebuild never downgrades below last-good #2384's hunks, and neither is pulled in:
   • loadAndWatch: main spells the reload closure (path, stats?) because Canonicalize watch paths so a Windows 8.3 short path cannot abort the process #2309 calls it without stats from its canonicalized watcher. v5.2 still registers loadFile directly on the chokidar watcher, so the closure keeps v5.2's (path, stats) signature and takes Publish TLS state transactionally so a failed rebuild never downgrades below last-good #2384's latch-rollback body unchanged. The stats ?? statSync(path) fallback was already on v5.2.
   • Publish point: v5.2's cherry-pick: Reject mTLS clients whose revocation status cannot be checked under fail-closed, and recover the issuer from Harper's trusted CAs when the socket chain lacks it (conflicts → v5.2) (#2457 → v5.2) #2827 backport of Reject mTLS clients whose revocation status cannot be checked under fail-closed, and recover the issuer from Harper's trusted CAs when the socket chain lacks it #2457 inserted publishTrustedAuthorities(caCerts.values()) right after the zero-certificate retry guard, where main at that time had nothing. Publish TLS state transactionally so a failed rebuild never downgrades below last-good #2384 replaces that spot with the candidate-map swap, so the publish line now sits after caCerts is rebuilt from candidateCAs, identical to where main's keys.ts holds it today. Look hardest here: publishing before the swap would hand the revocation checker the previous pass's CA set.
   • Everything else in keys.ts is byte-identical to the squash commit's hunks; the remaining diff against main's current keys.ts is exactly the two unrelated commits above plus later main work.
3. Workflow hunk kept. v5.2's integration-tests.yml has the same job shape as main, so HARPER_TEST_REQUIRE_FILE_WATCHERS: '1' is added to the integration job env, making cert-key-reload fail instead of skip when a file change never reaches hdb_certificate on Linux runners. Pushing a workflow file needs a token with workflow scope, which is why cherry-pick-patch.yml could not open this PR itself.
4. Branch rebuild caveat. cherry-pick-patch.yml rebuilds cherry-pick/<rel>/pr-<N> branches from the release branch on PR pushes and at merge, without rerere; expect the keys.ts resolution to need re-applying if that workflow touches this branch. Test this branch by dispatching integration-tests.yml / unit-test.yml with --ref cherry-pick/v5.2/pr-2384, not through the cherry-pick workflow.
5. Related root-cause issue, out of scope here: Cert renewal reaches workers as two separate reloads (key from disk, cert via table), so every renewal builds a mismatched pair #2978.
6. Independent review findings, all declined here because they are Publish TLS state transactionally so a failed rebuild never downgrades below last-good #2384's design on main, byte-identical in this diff. Codex, Gemini, Cursor Composer and the Harper-domain adjudicator found nothing in the two conflict resolutions; the adjudicator's own ruling was "fix on main first and re-pick", since a v5.2-only change would fork the line. Carried to the dispatch findings for main: (a) one-shot selectors (liveReload=false, getReplicationCert) now arm the self-rearming failure-retry timer too, leaving an orphan rebuild chain per boot/restart while a record keeps failing (log noise and periodic scans, no serving impact); (b) deleting the last leaf record trips the zero-certificate guard, so its hostnames and default stay served while scheduleRebuild loops, which contradicts the new DESIGN.md sentence (the default half of that was already true before Publish TLS state transactionally so a failed rebuild never downgrades below last-good #2384); (c) a leaf record with an unparseable PEM is pushed to failedThisPass by both loops; (d) the tie regression test's waitFor predicate is already true at baseline; (e) new sinon/rewire uses in keys.test.js against AGENTS.md. None of these is changed on this branch.

Changes

• security/keys.ts: pass-local candidate maps and default; per-record build failure retains the record's live entries and default candidacy; retained mTLS contexts are rebuilt against the current CA set and dropped if that rebuild fails (unless nothing else is servable); failure-retry backoff and signature-throttled logging; a pass-level throw after .ready settled leaves live state untouched; loadAndWatch latch rollback; the cert watcher returns its certificateTable.put so a rejected write unlatches and is retried by the poll.
• DESIGN.md: the transactional-publication note, verbatim from main.
• .github/workflows/integration-tests.yml and .gitignore (ignore node_modules under any integrationTests/ fixture, which the new test's fixture dir creates on a dev-mode boot): applied cleanly.

Verification

Node v26.2.0 locally, npm ci against v5.2's lockfile, dist/ rebuilt. All runs under a private HOME (v5.2's mocha.init has no per-PID system-database isolation).

• Fails on base: unmodified origin/v5.2 (78e528445) built in a side tree with the new cert-key-reload.test.ts copied in, HARPER_TEST_REQUIRE_FILE_WATCHERS=1 npm run test:integration -- integrationTests/security/cert-key-reload.test.ts: 0 pass / 1 fail, a worker stopped serving the last-good cert while the renewed cert had no matching key (served serial 8353068887037099000, the self-signed default, instead of 3001), with Error applying TLS ... key values mismatch logged by both HTTP workers. That is the A cert renewal whose table write misses the rebuild window downgrades TLS to the self-signed cert and never retries #2382 fallback the task describes.
• Same command on this branch: 1 pass / 0 fail / 0 skipped (the retain-last-good assertion ran, not skipped, so file-watch delivery works on this box with the env gate on).
• npx mocha unitTests/security/keys.test.js: 70 passing, including the new retain-last-good, whole-pass failure and latch rollback suites.
• npm run test:unit:main, npm run test:unit:resources: 4579 passing / 2 failing and 1817 passing / 0 failing. The two test:unit:main failures are this box, not the diff: gitCredentials sees the dispatch daemon's own GIT_CONFIG_GLOBAL/GIT_EDITOR in the spawn env, and tokenAuthentication rsa_keys passes when re-run alone (load flake). Neither file imports security/keys.ts.
• npm run build, npm run lint:required, prettier --check on the changed files: clean.
• test:integration:all is left to this PR's CI (the local box cannot run the Ollama suite); the changed code is exercised end-to-end by cert-key-reload above.

Refs #2384
Refs #2382

Complexity: medium

Signed: Claude Fable 5.1

🤖 Generated with Claude Code

https://claude.ai/code/session_01LSCvLgqVLi4PMaUr6ZgeP4

Origin — the dispatch brief this PR was written from

Backport harper PR #2384 ('Publish TLS state transactionally so a failed rebuild never downgrades below last-good', squash-merged to main as 1e4c163, closes #2382) onto the v5.2 release branch. Open a PR from branch cherry-pick/v5.2/pr-2384 against base v5.2, titled 'cherry-pick: Publish TLS state transactionally so a failed rebuild never downgrades below last-good (#2384 → v5.2)', following the shape of the earlier manual backport PR #2827. On 5.2, a cert renewal whose key reaches a worker before the matching cert currently makes that worker fall back to the self-signed cert until the 5-minute re-read (#1394) heals it; with #2384 the worker keeps serving the last-good pair instead.

Acceptance

1. Branch cherry-pick/v5.2/pr-2384 based on origin/v5.2 contains the full Publish TLS state transactionally so a failed rebuild never downgrades below last-good #2384 change, conflicts resolved, with no conflict markers left. 2) unitTests/security/keys.test.js passes on the branch (build first; mocha loads dist). 3) integrationTests/security/cert-key-reload.test.ts passes locally on Linux with HARPER_TEST_REQUIRE_FILE_WATCHERS=1, so it runs instead of skipping. 4) Prove the test can fail: run cert-key-reload against unmodified origin/v5.2 sources after a forced rebuild, and confirm it fails or falls back to the self-signed cert there. Report the result in the PR body. 5) The PR body lists every conflict resolution in security/keys.ts and any extra main commit you ported, with the reason. 6) Open as a draft PR against v5.2 with milestone v5.2. Do not touch main, and do not merge.

_{Dispatch: task harper-2384-backport-v52 · queued by kris-session · ran by claude/fable/high · worker kzyp-xps-1}

_{Review-Coverage: authored=claude; ran=cursor-composer,gemini,codex; adjudicated=domain; declined=cursor-grok,cursor-kimi,cursor-muse; rounds=1; full=1 @ 2fb147d}

_{Human-Review-Need: 4 (decisions: retain-over-revoke-on-replace, retain-past-expiry, backoff-before-ready, error-level-summary, ci-fail-not-skip, backport-with-known-gaps) @ 2fb147d}

[gemini-code-assist]

Code Review

This pull request introduces transactional publication for TLS configuration updates to resolve issue #2382. By building the replacement state off-to-side and only updating the live maps upon successful completion, the system ensures that transient configuration mismatches or build failures do not downgrade the active serving state below the last-good configuration. Additionally, it implements self-retry with backoff, throttled logging for failed passes, and mtime latch rollback on failed file applies. The feedback suggests minor improvements to the new unit tests, specifically guarding the afterEach cleanup hooks to prevent secondary TypeErrors if the beforeEach hooks fail before initializing key variables.