Skip to content

Key-custody seam: file + injected secret-key providers (closes #166)#512

Merged
kriszyp merged 12 commits into
mainfrom
kris/secret-custody-166
Jul 10, 2026
Merged

Key-custody seam: file + injected secret-key providers (closes #166)#512
kriszyp merged 12 commits into
mainfrom
kris/secret-custody-166

Conversation

@kriszyp

@kriszyp kriszyp commented Jul 2, 2026

Copy link
Copy Markdown
Member

Summary

Closes #166.

Implements the KeyCustody seam for cluster secret keys (#166): a Pro secretCustody component that selects a key-custody provider at startup and registers it with core's registerSecretCustody (from harper#1554), which installs the .env decryptor and backs the secret operations. Core owns the get_secrets_public_key operation — this component registers no operations (regression-tested: Pro startup never touches that op-map entry).

Supersedes #505 (feat/env-secret-encryption). The file tier is #505's provider, ported (with credit to @dawsontoth via Co-authored-by on the ported commit) onto the core seam: the envelope crypto now lives in core (core/utility/secretEnvelope.ts, itself the ported envSecretCrypto.ts), the get_secrets_public_key registration is dropped (core's), and on-disk key files are per-kid.

Tiers and trade-offs

file injected
Key at rest on disk, keys/envSecrets.<kid8>.private.pem (0600) memory only — never persisted
Provisioning leader generates on first boot; clone_node fetches via get_key host/orchestrator (Fabric) provisions every node
get_key exposure registered (restricted — see below) never in the key store
Worker delivery each thread (incl. job workers) loads from disk via core's workerData provider hook (harper#1559), http workers only, never job workers; the worker deletes the workerData property as early as it runs
Ingestion n/a fd-first: HARPER_SECRETS_KEY_FD read to EOF then closed (key bytes never in the environ); fallback HARPER_SECRETS_PRIVATE_KEY_B64, deleted immediately — but the environ residue at spawn time (/proc/<pid>/environ) is why fd is preferred. Both vars are scrubbed during main-thread component load, before any worker/job spawns (job workers copy process.env explicitly).

Provider selection (explicit-when-ambiguous)

Auto only when exactly one source exists: injected material → injected; none → file (generates on first boot). If injected material AND an on-disk key are both present, custody refuses to activatelogger.error demands an explicit secretCustody: provider: file|injected config block, and a disabled marker is delivered to workers via workerData so they don't silently activate the file tier while the main thread refused.

get_key restriction (the exfiltration hole)

keyService.ts's get_key documents "must have bypass_auth" but never enforced it — any super-user API call could read any getPrivateKeys() entry remotely. Registering the custody key there would have made the cluster's secrets key exfiltrable with one SU HTTP call. This PR gates custody-named keys (envSecrets.*private.pem) on req.bypass_auth === true, which external HTTP requests can never present (the server strips it) and replication OPERATION_REQUESTs from authorized node identities do (server.operation(data, {user}, !isAuthorizedNode) in replicationConnection.ts). Consequence: cert-auth clone_node still clones the custody key; credential/token-auth clones cannot — they get a 403, log clear guidance, and the node will generate its own (divergent) keypair on first boot. This is deliberate fail-closed behavior; flagging it here for review. (JWT keys keep their existing open-path behavior — out of scope.)

kid-map rotation shape

Decrypt is a kid→privateKey map lookup by the envelope's own kid, falling back to the active key for kid-less envelopes (unknown kid → the standard mismatch error). Encrypt-side (getPublicKey) exposes the single ACTIVE key. File tier persists per-kid filenames with the newest file as active, plus a stable key-store alias (envSecrets.private.pem) so a cloning node can fetch the active key without kid discovery; the cloned key is re-persisted under its per-kid name derived from the key material. Single key today; the rotation walk (P5) consumes the map.

Ordering

Registration order vs .env loading is not load-bearing: core queues encrypted entries it can't decrypt yet and registerSecretDecryptor replays them (harper#1559), so late custody registration heals rather than fails. (For reference, the normal order is still fine: startHTTPThreads awaits loadRootComponents() before spawning workers, and workers load root components — including this one — before user apps' .env entries.)

Dependencies

Builds on harper#1554 (registerSecretCustody + hdb_secret store) and harper#1559 (workerData providers + deferred decrypt replay). The core submodule pointer in this PR references the integration branch kris/secret-custody-core (= #1554 branch + #1559 merged); the pointer moves to upstream main at merge time, after both core PRs land. Merge order: harper#1554, harper#1559, then this.

Where to focus

  • security/keyCustody.ts — selection + worker start() fallback ladder (workerData → env ingest → disk). The worker env-ingest fallback exists for spawn paths that copy the environment before main-thread component load (dynamic threads); it also removes raw key material from a worker's process.env rather than leaving it readable.
  • security/custodyState.ts + the clone gate in keyCustody.ts / cloneNode.ts — the fix for the clone-ordering hole (see Cross-model review below); the fail-closed latch when no key can be fetched is a deliberate trade-off.
  • security/keyService.ts — the 4-line gate carries the security weight of the file tier.
  • The credential-auth clone limitation above (fail-closed vs. silently-open was the judgment call).
  • Ambiguity refusal: the disabled workerData marker is supplied to every worker type (it is a marker, not key material), so workers — including job workers — cannot silently activate the on-disk key while the main thread refused.

Testing

unitTests/security/: file tier (generate/persist 0600/reload round-trip, decrypt of a core-encryptEnvelope envelope, fingerprint match, key-store registration + cleanup); injected tier (real-fd ingestion + close + scrub, b64 fallback + scrub, invalid-fd rejection, kid-map decrypt incl. non-active kid / kid-less / unknown-kid, worker-type filtering, selection matrix); component-level against the real core submodule (injected activation flushes core's deferred .env queue, ambiguity refusal, explicit override, dormancy, get_key 403/served/unaffected, get_secrets_public_key handler identity unchanged by Pro startup). Full Pro unit suite: 264 passing. Build (tsc, includes dist/core from the integration branch) emits with the same pre-existing type-error baseline as main (36 lines, none in touched files).

Generated by Claude (Fable 5).

Cross-model review (Codex + Gemini, adjudicated)

Found and fixed (commit ba65c15):

  • HIGH — clone ordering hole (Codex): cloneNode starts Harper before cloneEnvSecretsKeys, so a fresh clone would self-generate a divergent file key, register it, and could accept set_secret under a clone-only key before the leader's key arrived — with no re-registration after the fetch. Chosen fix: path (b), generation suppression. Pre-start fetch (path a) is not viable: cert auth "has no way to talk to the leader before replication is up" (cloneNode's own constraint), and credential auth is barred from custody keys by the get_key gate — so no auth mode can fetch the key pre-start. Instead, cloneNode sets a clone-bootstrap gate (security/custodyState.ts, dependency-free so it loads pre-start) before main(); while gated, the file tier never GENERATES (loading an existing on-disk key stays allowed for re-clones) and custody runs dormant — set_secret with plaintext is correctly rejected, encrypted .env values queue in core. After cloneEnvSecretsKeys persists the fetched key, the gate lifts and activateFileCustodyFromDisk() registers it (core's deferred replay flushes queued values on the main thread; workers that started dormant pick the key up from disk on the next restart — same residual feat(security): Pro env-secrets component for .env encryption #505 had). If no key can be fetched, the gate stays latched for the session: fail-closed, no divergent generation.
  • Single-thread bypass (Codex): disableCustody now latches module state; with threadCount === 0, the same-process start() honors the latch instead of falling through to loadFileKeys() and activating the very key the ambiguous selection refused. Test added.
  • Unreadable key files (Gemini): hasFileKeys() && !loadFileKeys() (permission/disk fault) now ABORTS ensureFileKeys with a clear error instead of generating a replacement keypair (which would split the cluster key and orphan every existing enc:v1: value); worker threads log an error instead of running silently dormant. Test added.
  • Unknown-kid fail-fast (Gemini): the active-key fallback now applies only to kid-LESS envelopes; an unknown kid throws no secrets key for kid <kid> immediately instead of attempting RSA with the wrong key and surfacing an opaque OpenSSL padding error.
  • workerData in guard: start() guards against non-object workerData before the in check.

Verified, no action needed: the claimed missing LICENSE_KEY_DIR_NAME import in cloneNode.ts exists (pre-existing import block, line 28). Dismissed: .ts import-extension claims (repo TypeStrip/dist convention).

kriszyp and others added 3 commits July 1, 2026 22:23
…ta-hook)

Integration branch for the custody seam: core #1554 (registerSecretCustody +
hdb_secret store) merged with harper#1559 (workerData providers + deferred
decrypt replay). The pointer moves to the merged upstream branches at merge
time, after both core PRs land.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Cluster-shared RSA-4096 keypair persisted under keys/: the leader generates
it on first boot, cloning nodes fetch the private key from the leader during
clone_node, and every thread loads it from disk. Ported from
feat/env-secret-encryption (#505) with two changes: the envelope crypto now
lives in core (utility/secretEnvelope.ts, itself ported from #505's
envSecretCrypto.ts) instead of being duplicated here, and on-disk filenames
are per-kid (envSecrets.<kid8>.private.pem) so a future rotation walk can
stage multiple keys. The key store keeps a stable active-key alias
(envSecrets.private.pem) so cloneNode can get_key it without discovering the
kid first; the cloned key is persisted under its per-kid name derived from
the key material.

Co-authored-by: Dawson Toth <dawson@harperdb.io>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…riction

Closes #166. The secretCustody component selects a custody provider at
startup and registers it with core's registerSecretCustody (which installs
the loadEnv decryptor and backs the secret operations; core owns the
get_secrets_public_key operation — Pro registers none).

- injected tier: the host provisions the private key into the main thread,
  fd-first (HARPER_SECRETS_KEY_FD read to EOF and closed) with a base64 env
  fallback (HARPER_SECRETS_PRIVATE_KEY_B64); both variables are scrubbed
  before any worker or job spawns. Memory-only: nothing persisted, nothing
  in the get_key store. Workers receive the key via core's workerData
  provider hook — request (http) workers only, never job workers — and the
  worker deletes the workerData property as early as it runs.
- selection: auto only when exactly one source exists; injected material
  plus an on-disk key refuses to activate (main thread and, via a disabled
  workerData marker, all workers) until secretCustody.provider is set.
- get_key restriction: custody key names are served only to bypass_auth
  (internal / node-identity) requests, closing the remote-exfiltration path
  through the operations API; replication cert-auth cloning still works,
  credential-auth clones cannot fetch the custody key.
- decrypt is kid-map based (envelope kid lookup, active-key fallback for
  kid-less envelopes); getPublicKey exposes the single active key.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a secret-custody component supporting two tiers of key custody for cluster-shared secrets: a file-based tier that persists RSA-4096 keys on disk and clones them from the leader, and an injected tier that ingests memory-only keys via file descriptors or environment variables. It also restricts custody key access to node-identity requests and adds comprehensive tests. The review feedback highlights a potential file-writing failure in cloneNode.ts if the target directory does not exist, and recommends wrapping key-parsing operations in try-catch blocks within keyCustody.ts to prevent unhandled exceptions from crashing the main process or worker threads when dealing with malformed key material.

Comment thread cloneNode/cloneNode.ts
Comment thread security/keyCustody.ts Outdated
Comment thread security/keyCustody.ts Outdated
@claude

claude Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Reviewed; no blockers found.

kriszyp and others added 2 commits July 1, 2026 22:56
kris/secret-custody-core now includes harper#1559's review fixes (reserved
workerData keys incl. noServerStart, structuredClone detachment, redacted
deferred-queue accessor, identical-value replay no-op).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…bort, kid fail-fast

- Clone ordering hole (Codex): cloneNode starts Harper before the leader's
  env-secrets key is cloned, so a fresh clone would self-generate a
  divergent file key and could even encrypt new secrets under it. cloneNode
  now sets a clone-bootstrap gate (security/custodyState.ts, dependency-free
  so it can load pre-start) before main(); the file tier stays dormant
  instead of generating while gated (loading an existing on-disk key is
  still allowed for re-clones), and cloneEnvSecretsKeys lifts the gate and
  registers the cloned key via activateFileCustodyFromDisk once persisted
  (core's deferred replay heals queued .env values). If no key can be
  fetched, the gate stays latched for the session — fail-closed, no
  divergent generation. Pre-start fetch was not viable: cert auth cannot
  reach the leader before replication is up, and credential auth is barred
  from custody keys by the get_key gate.
- Single-thread bypass (Codex): disableCustody now latches; start() honors
  the latch so threadCount=0 cannot fall through to the on-disk key an
  ambiguous selection just refused.
- Unreadable key files (Gemini): files-exist-but-unreadable now aborts
  ensureFileKeys with a clear error instead of generating a replacement
  keypair (cluster key split); workers log an error instead of running
  silently dormant.
- Unknown-kid fail-fast (Gemini): the active-key fallback applies only to
  kid-less envelopes; an unknown kid throws `no secrets key for kid` rather
  than surfacing an OpenSSL padding error from the wrong key.
- Guard the workerData `in` check against non-object workerData.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@kriszyp kriszyp requested a review from dawsontoth July 2, 2026 04:58
@kriszyp kriszyp marked this pull request as ready for review July 2, 2026 05:03
@kriszyp kriszyp requested a review from a team as a code owner July 2, 2026 05:03
Comment thread security/keyService.ts
// requests have it stripped) — otherwise one super-user API call could exfiltrate the
// cluster's secrets key (#166). Credential/token-auth cloning therefore cannot fetch these.
if (isCustodyKeyName(name) && req.bypass_auth !== true) {
throw new ClientError('This key is restricted to node-identity requests', 403);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we list these keys in the SSH keys list operation that the UI calls? To avoid innocent errors, if we do, filtering them out of that operation would be helpful. A core change... double checking here though.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great instinct to check — I dug into both listing surfaces the Studio calls, and the good news is the custody keys don't show up in either, so no filter (core or pro) is needed:

  • list_ssh_keys (security/sshKeyOperations.tslistSSHKeys) enumerates <rootPath>/ssh/ only (with known_hosts/config excluded). The custody keys live in <rootPath>/keys/ (envSecrets.<kid8>.private.pem), a different directory, so they can never appear there.
  • list_certificates (core security/keys.tslistCertificates) reads rows from the hdb_certificate table. Custody keys are never inserted there — they exist only as files under keys/ plus entries in core's in-memory getPrivateKeys() map, and I verified nothing enumerates either of those into any operation response (the only readdir surfaces in core + pro are the ssh/ dir and the custody module's own scan).

So the only operation that can return custody key material at all is get_key by exact name — which is exactly the path this PR gates to node-identity/bypass_auth requests. If a future op ever enumerates the keys/ dir or the private-key map, isCustodyKeyName() is exported and ready to be that filter.

Leaving this thread open for you in case you had a different listing surface in mind!

🤖 (Claude, Fable 5)

kriszyp and others added 2 commits July 2, 2026 09:37
…esults)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ial never crashes

- cloneEnvSecretsKeys creates the keys dir before persisting the cloned key
  (a fresh clone cannot depend on boot having created it).
- Malformed injected key material (bad PEM/base64, fd garbage) no longer
  throws out of startOnMainThread/start: custody logs a clear error (never
  the material), latches disabled, and the node keeps booting — plaintext
  set_secret stays rejected and encrypted .env values stay queued.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…wns (#530)

The entrypoint hands the cluster custody private key to the container over
fd 3 via `exec 3<`, which cannot set close-on-exec. Until the consumer closes
that fd, it is inherited by every descendant that shares the process fd table
(worker threads) or that the process forks, so a compromised component could
read the key straight off /proc/self/fd/3 (raised in host-manager#130 review).

- Move the fd read/scrub/close into the dependency-free custodyState.ts
  (node:fs only) as consumeInjectedKeyMaterial(), and call it at the very top
  of bin/harper.js — before cloneNode()/harper() spawn any worker thread or
  subprocess. Because the module pulls in no config/logging graph, this runs
  before the base path is even set.
- Make ingestion idempotent (read-once + cached): the top-of-boot consume and
  the later secretCustody component call now share one cached PEM; the fd is
  read exactly once. Deferred read/misconfig errors are stashed and logged by
  the logging-capable front door (injectedKeyCustody.ingestInjectedMaterial).
- Keep the HARPER_SECRETS_PRIVATE_KEY_B64 env fallback scrub path unchanged.

Tests: a worker thread can read the key off /proc while the fd is open and
cannot after ingestion (fails if the close regresses); an end-to-end subprocess
spot-check of /proc/self/fd/3; and an idempotency (read-once) check.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@kriszyp kriszyp requested a review from DavidCockerill July 7, 2026 02:27
@kriszyp

kriszyp commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

Folded in a follow-up commit addressing #530 (fd-custody hardening) — this is the consumer half of the delivery contract @DavidCockerill flagged in host-manager#130 review.

What it does

  • The entrypoint hands us fd 3 via exec 3<, which can't set close-on-exec, so until we close it the key is readable off /proc/self/fd/3 by any descendant that shares the fd table (worker threads) or that we fork.
  • The read/scrub/close now lives in the dependency-free custodyState.ts (node:fs only) as consumeInjectedKeyMaterial(), called at the top of bin/harper.js — before cloneNode()/harper() spawn any thread or subprocess, and before the logging/config graph loads. Ingestion is idempotent (read-once + cached), so the later secretCustody component retrieves the same PEM without re-reading the (now-closed) fd. The HARPER_SECRETS_PRIVATE_KEY_B64 env fallback scrub is unchanged.

On the test: the acceptance asks for a /proc/<child>/fd spot-check. Worth noting that libuv already strips non-stdio fds from spawned subprocesses, so a subprocess /proc check passes regardless of our close — it can't witness the fix. The vector the close actually closes is worker threads (shared fd table, no exec). So the primary test is worker-thread-based and verified to go red if the close regresses; a subprocess spot-check is kept as an end-to-end check with that caveat commented.

— KrAIs 🤖 (Opus 4.8)

@kriszyp kriszyp merged commit 9106e77 into main Jul 10, 2026
32 checks passed
@kriszyp kriszyp deleted the kris/secret-custody-166 branch July 10, 2026 04:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Secrets / env-var management for Fabric-deployed applications

2 participants