Repository navigation
feat: credential custody under a passphrase and a verifiable trust surface (v1.78.0) - #244
Merged
Merged
Conversation
…he result The four model-answer verbs (extract-signals, design-note, skill-proposals page-draft, distill) share src/core/brain/payload-json.ts: plain parse first; only on failure strip ONE leading <think>...</think> block (fenced-unwrap per the decision-model/llm-emulation.ts precedent) and retry. Every strip is named on the result - a note on the ok envelope in --json mode, a note line in text mode - and a payload that still refuses after the strip keeps the named exit-1 refusal with the attempt named. Clean JSON output is byte-identical; genuinely malformed JSON is never rescued. Distill reports the payload refusal on its own unprefixed stderr line, matching its operator-input idiom.
sealWithDigest(body) and digestVerifies(body, digest) are now exported from src/core/integrity/digest.ts over the module's own sha256Hex(canonicalJson(body)) encoding, with the persisted-digest doctrine documented beside them: a seal re-checked across processes keeps wall-clock fields out of its body; a point-in-time record may bind its timestamp. migrate.ts keeps sealManifest/manifestVerifies as thin aliases over the lifted primitives - behavior untouched, pinned by a new migrate test asserting the sealed manifest equals sealWithDigest of the same bound body. The digest suite gains the seal contract: round-trip, key-order insensitivity, tamper refusal on any body mutation, undefined-entry omission matching canonicalJson.
The dry run now seals the adoption plan - plans, skipped entries, conflicts, unchanged rows - with the integrity module's sealWithDigest, and the result carries that digest. Wall-clock fields (imported_at, localDate, the trial window) stay outside the sealed body, so the same content seals identically whenever it is planned. Apply accepts approvalDigest and checks it against the freshly computed plan BEFORE the snapshot/write loop; drift raises the typed ApprovalDigestError naming approved and current digests, stating that nothing was written, and pointing at a fresh dry run. Without the option, apply is byte-identical to today. The CLI gains --approval-digest; non-interactive --apply requires it (mirroring the --yes guard, exit 2), interactive TTY keeps the human escape hatch, and dry-run text output prints approval-digest for capture. The pre-existing non-interactive apply CLI test moves to the two-step dry-run-then-apply flow the new contract mandates. Failing-first: orchestrator suite could not even load (no ApprovalDigestError export, no digest on results); the CLI guard and stale-digest tests asserted refusals that did not exist. Defect found while testing: the bit-identity check originally compared snapshot archive bytes, which encode the vault's absolute path and host file metadata - the comparison now covers the written trees and asserts snapshot existence instead.
A manifest-wide `contract: { fingerprint }` field, schema bumped to 2 with
a v1 read path (a v1 manifest reads back with contract: null, the
changed-contract marker; unknown versions still refuse by name), and
source-cleanup's direct write round-trips the field so a cleanup cannot
re-stamp entries recorded under an older contract.
computeExtractionContractFingerprint(vault) is pure over the schema
extractable allowlist (sorted) and the hand-bumped EXTRACT_CONTRACT_VERSION;
per-call options stay out.
Defect fix in the new test file: the vault fixtures now mint through
tests/helpers/temp-dir.ts tempDirs() instead of an unowned mkdtempSync, which
left nine temp entries per run for tests/setup.ts to refuse.
planBatches compares the manifest's recorded fingerprint against the live one
immediately after readManifest: on a mismatch - including a v1 manifest, which
recorded none - byte-identical sources reprocess with the distinct
contract-changed token ("modified" stays bytes-changed), the skip list empties,
and the wire gains contract_changed plus contract_changed_files only when the
contract changed, so an unchanged contract serializes byte-identically.
The fingerprint folds into computePlanId (the computeSessionImportId pattern),
so a stale checkpoint recorded under the old contract cannot resume-skip work
extracted under the new one; the old checkpoint file orphans harmlessly.
Reconcile consequently names those sources as the gap, and the batch-plan CLI
reports the change in text and JSON mode. The git-discovery plan-id pin gains
the fingerprint term its derivation now has.
Opt-in scrypt envelope (N=2^15 r=8 p=1, explicit maxmem, per-envelope random salt) replaces the raw 32-byte keyfile at the same path; the passphrase-derived key lives only in a process-local holder with a lock/unlock lifecycle and named refusals (closed code table, GCM-tag wrong-passphrase detection, timingSafeEqual holder-consistency check). - secret unlock wraps a raw keyfile on first use and warns with the wrapped-keyfile sync-exposure wording; secret lock clears the holder and refuses a never-wrapped store by name instead of recording a protection that does not exist. - loadOrCreateKey unwraps via the holder; with an empty holder it raises SecretStoreLockedError rather than minting a key that would orphan every stored value. - new read-only resolveSecretReadOnly export decrypts without the last_used_at stamp, the exec audit record, or the vault-identity guard (Lane B resolver contract, t_e5807974). - new vault-level writers (unlock/lock) sit behind assertVaultIdentityForWrite ahead of their first byte; envelope.ts carries the census exclusions its keyfile-path custody writes owe. - help and manifest state the loss warning: a lost passphrase is unrecoverable.
secretProvider/resolveNamedSecret compose parseSecretReference with Lane A's read-only resolveSecretReadOnly export: store first, env fallback, and a store-held name under a locked envelope surfaces the named locked-store refusal instead of silently answering from the environment. Resolution is read-only - no last_used_at stamp, no exec custody record, no store-file mutation - and a vault with no custody store gains no state from it. listNamedSecretAvailability reports metadata-only availability for o2b secrets list, namedSecretAvailable backs o2b secrets status. Also fixes a typecheck defect in the egress resolved-literals suite (tsc rejected toEqual on StructuredRedaction.value, which is typed unknown); the assertion now narrows the released verdict first.
…solver Every credential consumer now resolves a $secret:NAME value through the custody-store-backed provider at its use site, with byte-identical behavior when no reference is used (each routing is opt-in via the vault or option that joins the custody store): - expandRegisteredProvider probes envKey names through the merged provider (store first) and resolves a reference-shaped probe entry, behind a new optional secretsVault option. - decision-model config resolution resolves a reference-shaped key value when secretsVault is passed; an unresolvable reference or a locked store lands in errors as the named refusal (status invalid) instead of a silent empty key - resolution still never throws. - resolveResearchPoolEnv resolves reference-shaped provider keys with the custody vault passed; an unresolvable reference throws the named SecretReferenceError instead of quietly yielding an empty pool. - resolveTelegramBotToken and resolveInstallationSecret resolve a reference token/key through the passed vault; the installation secret never self-heals over an unresolvable reference, and vaultStoreReference threads the vault it references. - o2b secrets list/status accept --vault and report availability from the merged provider (metadata only, locked still counts); without it the env-only lookup and the status exit-code contract are unchanged.
UpgradePlan gains a digest, sealed inside planUpgrade with the integrity module's sealWithDigest over the rows, pending and errors. The body carries no wall-clock field, so the same managed-file state seals identically on every run. applyUpgrade verifies the seal FIRST - before the error rows, the no-op return, the byte-drift check and the snapshot. A caller-supplied plan that fails its own digest gets the drift refusal shape (no run id, no snapshot, nothing written) with a distinct message: the plan object was edited after it was sealed, and a plan nobody measured is not applied. A self-computed plan verifies trivially, so the CLI and the self-heal worker - both applying the plan they just planned - are unchanged; the worker's suites stay green untouched. renderUpgradePlanJson now emits the digest. The row projection lives in upgrade-render.ts (outside this lane's paths), so the emission is wrapped at the barrel every verb imports its renderers through. Failing-first: plan.digest was undefined in the core and CLI suites, and a hand-tampered plan was trusted and applied instead of refused. The pre-existing tamper test (apply writes the bytes it was handed) now re-seals via reseal() - an honest caller recomputes the seal over its own rows - keeping its intent under the sealed regime.
export re-encrypts every entry under a key the passphrase derives through A1's envelope KDF (fresh salt, parameters recorded inside the bundle); import verifies and decrypts EVERY value before writing anything, reuses the store's name/env-var validation, refuses existing names exactly (removeSecret exactness) unless --replace, and writes under the store's lock through writeStore. Both ops land no-values custody records (secret_bundle_exported/imported). - export --out is the wave's one new egress site: flag, registry entry and census rows land in this single commit. The guard runs over the metadata inventory only, and the inventory is shaped so the scan can only help: entries as an ARRAY (a name like api-key as a mapping key is the redactor's credential-assignment shape and replaced the whole entry), the KDF block excluded (its random salt is exactly the high-entropy shape the token pass catches), allow patterns scanned in full, and a REWRITTEN entry identifier refuses instead of merging. Defect found and fixed during A2: the first cut scanned the raw metadata tree, which mangled entry objects and salts; the census- declared status (shared_redactor) stays honest to what the module actually scans. - vault-identity: export and import carry the guard ahead of their first byte; store.ts gains the shared readStore/writeStore/lock exports and the name/env-var predicates the importer reuses.
Lane B's resolver routing made src/core/config.ts import secret-resolver.ts, which reaches the secrets custody store, which appends through reliability/audit.ts, which reads JSONL_LEDGER_EXT from brain/ledger-shards.ts at module-evaluation time - and ledger-shards.ts imports config.ts for the device id. With ledger-shards.ts (or anything entering through it) evaluated first, audit.ts resumed mid-cycle and hit a TDZ ReferenceError on JSONL_LEDGER_EXT, red-ing every suite that loads the config module. Move the constant into the leaf path-constants.ts (the Brain layer's declared no-imports constants module) and re-export it from ledger-shards.ts so no consumer changes; audit.ts now reads it from the leaf directly.
… pin Lane C's a7a34df added the frontmatter-tags detector to HYGIENE_DETECTOR_IDS; the literal tuple this suite pins stayed at seven members, so the lockstep test failed. Add the id after "tags", matching the source tuple's order.
Lane B's routing and the passphrase envelope each closed a module cycle the architecture gate refuses, and the six-module one aborted audit.ts at module scope with a TDZ error (2f57dd6 removed the abort; the cycle itself remained): config -> secret-resolver -> custody store -> audit -> ledger-shards -> config The store is joined by the resolver at CALL time now (lazy require, the cycle cure the gate sanctions) - it is needed only when a reference or a probe actually resolves. The same cure cuts crypto -> envelope, whose wrap path encrypts through the crypto kernel. audit.ts reads JSONL_LEDGER_EXT through the ledger-shards re-export explicitly: a re-exported binding resolves to the declaring leaf module, so it stays initialized mid-cycle.
…tructive census The passphrase envelope (fee2856) swaps the plaintext keyfile for its wrapped form through a tmp-plus-rename and unlinks the tmp on a failed wrap. The census saw two ungated removal sites with no declared recovery story and failed, cascading into every 'the census can fail' fixture that asserts the report is exactly one intruder. Declare the site: the displaced bytes are the keyfile the envelope just replaced, the wrapping passphrase re-derives the key they protected, and the custody state sits outside every snapshot archive by design.
readAllRecords tie-broke same-timestamp rows by content-hash id, and a content hash says nothing about which of two same-instant writes landed first - so 'latest wins' readers answered arbitrarily, and Lane A's divergent-session-summary read (cd8378c) carried the FIRST take after two differing writes. Within one shard the append-only file is the authority: rows now read in append order, with cross-shard ties still ordered by shard id because two devices' arrival orders genuinely cannot be known. session-summary's re-sort keeps that order with a stable created_at sort instead of re-breaking ties by id.
Lane B routed config RESOLUTION through the named-secret resolver but left the use sites reading raw values. Complete card t_e5807974's intent at those sites, each opt-in via the vault that joins the custody store and byte-identical when no reference is used: - makeDecisionProvider takes an optional vault and resolves a $secret:NAME key value through it at call time (config resolution only probes the reference for presence; the factory sends the value). Named refusals propagate, as research.ts already does. The runDecision option threads it, and every use with a vault in scope passes it: the extract pre-filter, labels, recall-inject, skills (both stages) and the decision-model rerank. - resolveSearchConfig's registry expansion passes the vault, so a registered embedding provider probes the custody store and resolves a reference-shaped envKey. - the telegram-capture verb passes the vault to resolveTelegramBotToken; an unresolvable reference now fails the run naming the secret instead of the raw reference reaching the Telegram API as a token. - the vault_path MCP field degrades to named, path-free reasons when the installation secret is a reference the custody store cannot resolve or answers under a locked envelope - both named errors carry paths or raw values that must not travel in model context. Tests: the decision-model config/provider suite witnesses the resolved authorization header on a loopback fake (and the raw-value pin without a vault), the registry suite resolves through resolveSearchConfig, the telegram-capture CLI suite proves the named refusal before any transport is built, and a new vault-path-field suite pins both degraded reasons plus the plain-secret pass-through.
The batch-plan reconcile and claude-memory dry-run tests cast the parsed JSON to shapes narrower than their own toEqual assertions (bytes on the batch file row; basename and prefId on an import plan), which TS2769-rejected the toEqual overloads. Widen the casts to the shapes the wire actually carries.
configWithProvider captures nothing from its describe scope, which the consistent-function-scoping rule flags; the other fixtures in this suite live at module scope, so this one does too.
…docs The wave's planning docs named the recon cache and the worktree by their absolute /home paths, which the repo's hardcoded-home-paths gate refuses on shipped surfaces. Tilde-fold them; the session-scoped cache directory they name is gone either way.
src changed across the trust-surface-hardening wave (resolver plane, session summaries, hygiene detectors, sealed plans, capture vocabulary); the committed bundle was stale. Rebuilt with bun 1.4.0 (the version CI pins in .github/workflows/ci.yml) via bun run build:openclaw.
A deleteBySource on a not-yet-reprocessed v1 manifest used to rewrite the file as v2 under the live contract: readManifest handed back contract null and writeManifestAtomic coalesced that null into the live fingerprint. The surviving v1-extracted entries then classify unchanged forever - the owed one-time reprocess forfeited, silently, for exactly the vaults that ship v1 manifests. writeManifestAtomic now treats the three contract spellings distinctly: omitted records the live contract (the post-extraction upgrade, legitimate because the named paths were reprocessed); a contract object round-trips verbatim; an explicit null serializes the v1 shape, contract-less, so the changed-contract marker survives until a real reprocessing write lands it as v2. readManifest also reports the schema version actually on disk instead of the write constant, so a v1 file answers '1' to any diagnostic. Pinned: v1 + deleteBySource still reads contract-null and the next batch-plan reprocesses the survivor as contract-changed; the on-disk version round-trips; the omitted form still upgrades.
One legacy MEMORY file whose name slugifies to a purely numeric topic used to abort the ENTIRE import: the renderer's TagSyntaxError escaped the entries loop and refused dry run and apply alike, with a message naming only the field - no file an operator could rename. Every other entry-level data problem in the same loop (a non-feedback parse, a duplicate target id) already lands a named skip row and lets the import continue. The orchestrator now renders BEFORE the plan row exists and catches the tag-rule refusal per entry: the entry lands a skip row carrying its basename and the shared rule text, no contradictory plan row reaches the sealed approval body, and the good entries land. The duplicate-id guard only claims an id for entries that actually rendered. Tests wrap their temp-dir cleanup in try/finally so a failing assertion no longer leaks dirs and masks itself.
…usal drain --dry-run classified the idea route as routable while apply refused it per item: the declared page_types vocabulary gate lived in writeIdeaNote, which only fires on execute. The gate now sits in the idea classifier, so a vault that declares page_types without captured-idea gets the same typed UnroutableCapture - same refusal text, capture left staged - in BOTH modes, and the dry-run report is a preview an operator can act on. import-claude-memory silently dropped --approval-digest under --dry-run (the flag was parsed and only consulted on apply), so a mis-piped script read a successful exit code as 'digest honored'. The pairing now refuses by name - a dry run computes a digest, it does not consume one - exit 2, before any work.
The divergent response was pinned key-by-key but never as a closed set, so a future key could appear or disappear without a test noticing (the single-record envelope already had an exact key-set pin in the core suite). Object.keys(...).toSorted() equality freezes the additive shape: found/digest/digest_count, plus exactly divergent/records when the session diverges.
…ap exclusion, path-free refusal
…hanged reporting The two operator-facing behaviors this review round owned were absent from the CLI reference: the import-claude-memory row now names --approval-digest (the non-interactive apply seal, the stale-digest remedy, the per-entry skip for unrenderable names, and the digest+dry-run refusal), and the batch-plan row names the changed-contract reporting (contract-changed status, the contract_changed/contract_changed_files keys, v1 manifests included). Only those two rows touched.
… credential reads
…tore writer refuses them
…ade the ping by name
…eference inspection
…ait bound off the deadline The lock-lifecycle racer test called unlockSecretKeyfile twice sequentially, so its name claimed an interleaving it never constructed; the rename states the loser-path contract it actually pins (re-check under the lock takes the unlock branch, never a locked refusal). The systemone retry-after bound sat exactly at the decide deadline, where scheduler jitter alone could flake it; the named 429 reason plus the single recorded request are the structural witnesses, and the elapsed bound now only has to separate a 30s retry-after wait from a slow machine.
…tead of minting resolveSecretReadOnly composed loadOrCreateKey, which mints a fresh keyfile when the file is missing. Over a store that still holds entries, a $secret: resolution therefore minted a fresh key over the surviving ciphertext: the resolve answered a raw cipher error, left a new keyfile on disk, and silently orphaned every stored value - the same self-heal-over-persisted-state shape the installation-secret fix removed, on the one resolve whose contract says it writes nothing. The read-only resolve now refuses the missing-keyfile state by name (secret_store_keyfile_missing, path-free prose, the locked refusal's sibling state), and every named-degrade site that already carries the locked and reference refusals carries this one beside them: the decision-model key resolution and the ping check, the search registry probe, and the vault_path field's degradation contract. The egress literal scan's degradation is unchanged in behavior - a missing keyfile still contributes nothing and still mints nothing - now enforced in the store itself. Pinned: the store-level refusal leaves no custody state behind (no keyfile minted, ciphertext byte-identical); the registry probe surfaces the named refusal instead of 'not a registered provider'; the decision-model check degrades it into errors and a not-sent ping with no request leaving; the vault_path field degrades to the named reason; the MCP error boundary answers byte-identically to the bare redactor.
The stdin ingestion refused a passphrase that is empty after trim; the --passphrase-from-env ingestion accepted one, so a blank env value (the classic unset-variable accident) could wrap the keyfile under what is almost certainly not the intended passphrase. Both routes now enforce the same rule, the refusal names the blank case, and a test pins that the refusal precedes any custody state. Also gives storeValue its own docblock instead of the merged provider's copy-pasted one.
|
Important Review skippedToo many files! This PR contains 157 files, which is 57 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configuration
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (157)
You can disable this status message by setting the
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The CI plugin scanner reads a quoted config line whose value carries a dollar-secret reference as hardcoded material when it follows a credential-shaped key inline. The four flagged fixtures now assemble the same bytes from named constants, so the rendered config is byte-identical and no source line carries the flagged shape.
The suite exercises the per-cause degradations of a reference-valued installation secret; config resolves references through the named resolver port, so the suite now registers it like the sibling custody-store suites do. Without the registration the branch takes the unwired generic refusal and the locked and missing-keyfile degradations cannot be exercised.
Owner
Author
|
CodeRabbit skipped this change set (157 files, above its limit). In its place this PR carried four in-run review passes over the full branch: a self-review round (four read-only reviewers across the secrets, validation, sessions and ingest clusters, with two fixer rounds), a test audit (every changed test file proven to fail on pre-fix code), a security review round (focused on the new custody boundaries), and a delegated OpenCodeReview pass covering all 84 reviewable files. Findings from each pass are fixed in the branch with regression tests. |
…ile restriction Two failures from the Windows CI job of this branch: The default memory location in the import refusal rendered with the platform separator, so Windows operators saw backslashes where every sibling refusal spells ~/.claude/projects/. The home-relative display form now always uses forward slashes. The wrap path replaces the keyfile by rename, and the renamed file inherits the directory's ACL; restrictToOwner's idempotence memo keyed on the path skipped the re-restriction, leaving inherited entries on the wrapped keyfile. The wrap now forces the restriction against the memo, matching the mint path's owner-only guarantee.
The unlock lifecycle test spawns five CLI processes and the approval digest tests four; on a slow hosted runner they brush the default per-test timeout while passing everywhere with headroom. Each now carries an explicit budget instead of relying on the default.
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
1 participant
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.
Summary
Open Second Brain 1.78.0 puts credential custody under the operator's passphrase and makes the trust surface verifiable end to end. The secrets keyfile wraps into an opt-in scrypt envelope behind a lock/unlock lifecycle;
$secret:NAMEreferences resolve through the custody store at every credential use site; one passphrase-encrypted bundle moves the store between installs; resolved credential literals are redacted at the error and config-mapping boundaries. The vault binds its own writers (Obsidian-parseable tags, declared page vocabulary on capture), and the plans an operator approves are sealed - import and upgrade apply exactly the approved plan or nothing.flowchart LR P["Operator passphrase<br/>memory only, never persisted"] K["Wrapped keyfile<br/>scrypt envelope"] S["Custody store<br/>ciphertext at rest"] R["Config values<br/>dollar-secret-NAME references"] C["Consumers<br/>embeddings, decision model,<br/>research, Telegram"] E["Egress boundaries<br/>MCP errors, config mappings"] P -- "unlock, lock" --> K K --> S S --> R R --> C C -- "resolved values" --> E E -- "literals scrubbed,<br/>named refusals" --> O["Caller output"]What changed
--passphrase-from-envingestion, path-free named refusals, a stated loss warning, and a read-only resolve that refuses a missing keyfile instead of minting over surviving ciphertext.secret export/importre-encrypt every entry under the same KDF, validate names, env-var mappings and allow patterns exactly assetwould, and refuse held names without--replace.$secret:NAME; store-first with env fallback, a locked store refuses instead of falling back, unresolvable references surface by name, ando2b secrets list/status --vaultreports merged availability.import-claude-memory --approval-digest(non-interactive apply requires it; stale digest refuses before any write) and brain-upgrade plans verified at the top of apply; the self-heal worker is untouched.frontmatter-tagsdoctor detector) and capture writes consult the declared page vocabulary before any filesystem effect.digest_count, hash-only sample), the CLI answer verbs strip and name a leading think block, and the source-ingest manifest records the extraction contract so changed settings reprocess sources (schema 2, v1 manifests preserved through cleanup).docs/cli-reference.mdanddocs/how-it-works.mdcover the new surface; CHANGELOG entry and version 1.78.0 with all mirror targets synced.Verification
bun test16,579 pass, 20 skip, 1 fail at the reviewed head; the one failure is byte-identical tomainand green under the CI-pinned Bun (a bun-version JSON-message pin, adjudicated, untouched); Python tests and the anti-drift test pass; hermes plugin scan reports critical 0; the OpenClaw bundle is rebuilt with Bun 1.4.0.