feat(security): prototype — trusted secrets store + config-declared scope.secrets#509
Draft
dawsontoth wants to merge 4 commits into
Draft
feat(security): prototype — trusted secrets store + config-declared scope.secrets#509dawsontoth wants to merge 4 commits into
dawsontoth wants to merge 4 commits into
Conversation
Explores hardening the env-secret private key and env vars against the two attack surfaces raised in review of #505: a compromised control-plane node, and compromised customer code (a malicious plugin, or an RCE in e.g. Next SSR). Two ideas, both leaning on the fact that Harper owns the sandbox boundary: 1. KeyCustody (security/keyCustody.ts) — an interface that exposes only an `unwrapKey` operation, never the private key. Backends: - FileKeyCustody: key on disk, loaded into the trusted Pro process (never into sandboxed component/plugin code). Default; refactored out of the existing envSecrets logic. Keeps a sync path for loadEnv → process.env. - KmsKeyCustody (sketch): key lives in AWS KMS and never enters the process; `unwrapKey` is a KMS.Decrypt (auditable, rate-limitable, revocable). Turns key theft into bounded oracle access. Same interface leaves room for PKCS#11/TPM later. 2. Per-component secrets accessor (security/secretsAccessor.ts) — the mediated `scope.secrets.get(name)` surface. A component may read only the secret names it declared; decryption happens on the trusted side via KeyCustody, so the key never crosses into sandboxed code and there's no ambient process.env to scrape. Least-authority + key isolation. envSecretCrypto.ts is split into reusable primitives (parseEnvelope / unwrapWithPrivateKey / openEnvelope) so both the file fast-path and the custody-agnostic async path share one implementation. envSecrets.ts is rewired onto KeyCustody with the loadEnv/process.env compat path preserved (file backend) and createScopeSecrets() added for the accessor path. Prototype for team discussion — not wired into core/scope yet (see the INTEGRATION note in envSecrets.ts). 18 unit tests cover the crypto round-trips, custody load/deny paths, key non-exposure, and the accessor allow-list. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
Contributor
|
Reviewed; no blockers found. |
Reframes the custody prototype around how Harper actually isolates customer
code, and makes the customer-facing `secrets:` config the centerpiece.
Harper runs component/plugin code in node:vm contexts (hardened by SES / frozen
intrinsics) inside a worker thread — not in separate processes, and not with
isolated-vm. The boundary that contains a compromised plugin or an SSR JS-RCE
is therefore reference-reachability inside the isolate, governed by the curated
globals + `harper` exports Harper hands the context. Two consequences:
- A separate broker *process* buys ~nothing against that (JS-level) threat and
can't beat host-root either, so it's dropped. Keeping the key in HOST SCOPE —
off the sandbox global and the `harper` module — is what isolates it, for
free. KMS stays only as an optional off-box tier for the host-root/native-RCE
threat, behind the same interface.
- The real leak is `process.env`: `process` is reachable in the sandbox and
loadEnv writes decrypted values there, so encryption is undone at runtime.
scope.secrets resolves on demand and never populates process.env.
Customer-facing config (the crux): a component declares what it consumes in
`config.yaml`, and that declaration IS the authorization list — same names to
use a secret and to be granted it.
secrets:
DATABASE_URL: { required: true, description: Postgres DSN }
DEBUG_WEBHOOK_URL: { required: false }
# or shorthand: secrets: [DATABASE_URL, STRIPE_API_KEY]
Changes:
- security/secretsConfig.ts (new): parseSecretsConfig — list + object forms,
required (default true), description, name validation, dedupe.
- security/secretsAccessor.ts: declarations instead of bare names; adds
describe() (operator tooling) and ensureRequired() (fail-loud at load).
- security/envSecrets.ts: createScopeSecrets takes declarations + two-tier
sources (cluster secret overrides app .env); header + INTEGRATION notes
rewritten around host-scope key and the loader binding.
- security/keyCustody.ts: header reframed (host scope; File is the mechanism;
KMS demoted to optional off-box tier); removed the broker-process backend.
- docs/component-secrets.md (new): stipulates the config surface and why the
declaration is, by construction, the protection.
- tests: secretsConfig + secretsAccessor split out and expanded (28 passing).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fixes the self-granting hole: the previous cut used a component's own
`config.yaml` `secrets:` block as the allow-list, but the component (and whoever
deploys its config) controls that file — so it authorized itself. Authority now
lives in a store the component can't write.
Model (mirrors how Harper already replicates hdb_user/hdb_role):
- system.hdb_secret — a table in the replicated `system` database:
name (PK) · value (enc:v1 ciphertext) · grants[] (authorized components) ·
description/updatedBy/updatedAt. Writable only by super_user.
- Identity is Harper-asserted: grants key off the component name the loader
stamps (basename(componentDirectory)); a component can't claim another's name.
- Resolution: scope.secrets.get('X') from component C → look up X → allow only
if C ∈ X.grants → decrypt X.value on the trusted side via KeyCustody. Never
touches process.env; key never enters the sandbox.
- The component's `secrets:` config is now a NON-AUTHORITATIVE manifest: it
states needs (for ensureRequired fail-fast + operator UX) and can never widen
access beyond the store's grants.
Changes:
- security/secretsStore.ts (new): SecretsStore interface, InMemorySecretsStore
(tested), isGranted authority check, operator ops (setSecret/grantSecret/
revokeSecret/listSecrets — ciphertext + metadata only, never plaintext), and
a TableSecretsStore sketch binding to system.hdb_secret.
- security/secretsAccessor.ts: authority from the store keyed by the
Harper-asserted componentName; has/list now reflect grants; ensureRequired
distinguishes not-granted from granted-but-unset; manifest is advisory only.
- security/envSecrets.ts: createScopeSecrets(componentName, store, {manifest}).
- security/secretsConfig.ts: re-documented as a non-authoritative manifest.
- docs/component-secrets.md: rewritten around the trusted store + grants.
- tests: secretsStore + store-backed accessor (36 passing) incl. "a component
cannot widen access via its own manifest".
Chosen with the user: keep the manifest (non-authoritative), embedded grants
array on each record, component-name identity.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…leton getter) Addresses review on #509: the `generate` flag flowed through the `getCustody` singleton getter, so it was silently dropped once `custody` was set. If exported `createScopeSecrets` (or any accessor path) ran before `startOnMainThread`, `custody` locked to generate=false and the leader's later `fileCustody(true)` hit the `if (!custody)` guard and returned the non-generating instance — on a fresh node, the shared key would never be generated. Fix: key generation is no longer a construction/getter concern. FileKeyCustody is load-only by default (accessors throw if the key is absent); the leader calls an explicit, idempotent `ensureKey()` in startOnMainThread. Dropped `generate` from getCustody/fileCustody/getKeyCustody/KeyCustodyConfig. Ordering can no longer skip key creation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment on lines
+146
to
+150
| async #kms(): Promise<any> { | ||
| const pkg = '@aws-sdk/client-kms'; | ||
| const { KMSClient } = await import(pkg); | ||
| return new KMSClient({ region: this.#region }); | ||
| } |
Contributor
There was a problem hiding this comment.
Suggestion (non-blocking): Cache the KMSClient instance rather than constructing one per call. unwrapKey is called on every scope.secrets.get(), so each resolution currently opens a fresh TCP+TLS connection to KMS. Cache as a field:
Suggested change
| async #kms(): Promise<any> { | |
| const pkg = '@aws-sdk/client-kms'; | |
| const { KMSClient } = await import(pkg); | |
| return new KMSClient({ region: this.#region }); | |
| } | |
| #client?: any; | |
| async #kms(): Promise<any> { | |
| if (this.#client) return this.#client; | |
| const pkg = '@aws-sdk/client-kms'; | |
| const { KMSClient } = await import(pkg); | |
| this.#client = new KMSClient({ region: this.#region }); | |
| return this.#client; | |
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed since the first cut (and why)
The first version used a component's own
config.yamlsecrets:block as the allow-list. That's not a control: the component (and whoever deploys its config) controls that file, so it authorizes itself. Authority moved to a store the component can't write.I also grounded this in how Harper actually isolates code (core:
server/threads/manageThreads.js,security/jsLoader.ts):node:vmcontexts hardened by SES / frozen intrinsics — notisolated-vm, not separate processes. So a separate broker process buys ~nothing against a JS-level compromise; keeping the key in host scope is what isolates it. (Broker dropped; KMS kept only as an optional off-box tier.)processis reachable in the sandbox andloadEnvwrites decrypted values toprocess.env, so that path is ambient.scope.secretsnever populatesprocess.env.The authority:
system.hdb_secret(trusted, replicated)A table in Harper's
systemdatabase — the same replicated store behindhdb_user/hdb_role, so grants + encrypted values are consistent cluster-wide and survive an instance being down. Super_user-writable only.name(PK)valueenc:v1:ciphertext (decryptable only by the host-scope key)grantsOperator ops (ciphertext + metadata only; plaintext never passes through the server):
set_secret,grant_secret,revoke_secret,list_secrets.Identity: asserted by Harper, not the component
Grants key off the component name the loader stamps (
new ApplicationScope(basename(componentDirectory), …)), so a component can't claim another's identity. Caveat, stated honestly: trust in the name is only as strong as who can deploy under it — operator-controlled in managed/Fabric, a deploy-time assumption on self-hosted.Resolution
scope.secrets.get('X')from component C → look upX→ allow only ifC ∈ X.grants(else throw, pointing atgrant_secret) → decryptX.valueon the trusted side via KeyCustody → return plaintext. Neverprocess.env; key never enters the sandbox; C can't read another component's secret.The manifest: non-authoritative
A component may still declare needs in
config.yaml— but only as a request:Used only for
ensureRequired()fail-fast (distinguishing not granted from granted but unset) and operator UX. It can never widen access beyond the store's grants — there's a test for exactly that.Changes
security/secretsStore.ts(new) —SecretsStore+InMemorySecretsStore(tested),isGranted, operator ops, and aTableSecretsStoresketch binding tosystem.hdb_secret.security/secretsAccessor.ts— authority from the store keyed by the Harper-assertedcomponentName;has/listreflect grants;ensureRequireddistinguishes not-granted vs granted-unset; manifest advisory only.security/envSecrets.ts—createScopeSecrets(componentName, store, {manifest}).security/secretsConfig.ts— re-documented as a non-authoritative manifest.security/keyCustody.ts— host-scope framing; broker removed; KMS demoted to optional off-box tier.docs/component-secrets.md— full design.Threat model, honestly
process.env, or self-granting via config. No extra process needed.Not wired into core yet (deliberately)
system.hdb_secretin core (super_user-writable, replicated); bindTableSecretsStore.set_secret/grant_secret/revoke_secret/list_secrets(handlers delegate to the store fns).componentName+ shared store, bind to per-componentscope.secrets(+harpersecretsexport), pass the manifest, callensureRequired()at load.Related
enc:v1+ dormant decrypt hook)🤖 Generated with Claude Code