feat(wallet): key custody through an external secret manager - #33
Merged
Conversation
Adds a third custody mode alongside a raw key and a Foundry keystore: the config holds a <provider>:<reference> pointer and the key is fetched into memory per command, so nothing is ever at rest. This is the same shape the keystore path already uses -- config names an external source, and a helper is run at use time to produce the key on stdout -- so it introduces no new concept, only a second instance of an existing one. It closes the gap keystore-setup.md documents: keystore mode prompts on a tty, so MCP and CI have had no custody option but a key in the config file. A key reference prompts for nothing, which makes it the first mode that works under automation with no key on disk. After wallet init --keyRef, every command is ordinary foc-cli usage -- no wrapper, no prefix, no environment to prepare. Design notes: Providers are a closed set. The executable is chosen by code and only the reference comes from config, so a tampered config cannot turn key resolution into arbitrary command execution -- the property the keystore path has, and the reason there is no general "run this to get my key" field. keyRef is absent on every existing install, so the branches below it are reached unchanged and there is nothing to migrate. Only one custody mode is ever live: setting any of them clears the others. Without that, a stale privateKey would sit at rest where nothing reads it. execFileSync does not apply PATHEXT on Windows, where an npm-installed helper is clawdi.cmd. Candidates are probed and stat'd rather than reaching for shell: true, which would put a config-supplied string through a shell. Errors never echo what was resolved: a reference pointing at the wrong field must not print that field's contents. Resolution stays lazy, so read-only commands never pay for it. wallet balance now reports keySource, so a vault-backed setup is verifiable at a glance -- the address proves which key signed, this proves where it came from.
…fering them Three related gaps around the key-reference mode, all of them about not misleading or surprising the caller. Never suggest a tool the machine does not have. `wallet init` guidance offered `--keyRef clawdi:...` unconditionally, which on a machine without clawdi is a dead end an agent will walk into. Providers are now probed on PATH -- a stat, no process, no network -- and only offered where they would work. The reference docs still describe every provider; a call to action describes what you can do right now. Refuse to discard a configured key. An explicit method replaced whatever was configured, silently and unrecoverably. It now names what would be lost -- the derived address for a private key, the path for a keystore, the reference for a key reference, never the key itself -- and asks on a terminal, or fails with WALLET_ALREADY_CONFIGURED and a --force call to action in agent mode. Only a change that actually replaces something triggers it: re-running the same reference is idempotent, and prompting for that would train agents to pass --force reflexively, defeating the guard. Check the cheap things first. Every wallet-touching command now runs a preflight before constructing a client, so "no wallet" and "provider not installed" arrive as WALLET_NOT_CONFIGURED / KEY_REF_PROVIDER_MISSING with actionable next steps, instead of escaping as an untyped throw from inside key resolution. The preflight deliberately does not resolve the key: that needs an authenticated provider and a round trip, and belongs at use time. Configuring a reference before installing its provider stays legal -- image layers and provisioning scripts run in an order you do not control -- but `wallet init` now reports providerAvailable and warns, so the next command cannot fail confusingly.
Adds external key custody ([#33]) — a config-held reference to a key kept in a secret manager, resolved per command. Contains one behaviour change, so the next publish must not ship as a patch: wallet init no longer silently replaces a configured wallet. Atomic bump: package version, both skill frontmatters, version pins in skill examples, and the changelog Unreleased retitle — CI pins them together.
Accept a resolved value only when it is a 0x + 64 hex token standing on its own, and refuse output holding more than one. Any 32 bytes form a valid secp256k1 key, so the old unanchored first-match took the leading half of a longer blob and signed as a different address instead of failing. Run .cmd/.bat helpers through cmd.exe. npm installs clawdi.cmd on Windows, which CreateProcess cannot launch and Node has refused to since the fix for CVE-2024-27980: the PATH probe found it, the launch failed with EINVAL, and that was reported as "not logged in / wrong project". References and project scopes are restricted to characters a shell treats literally, so a tampered config still cannot become command execution. Cache PATH hits so a signing command probes once rather than twice, and expose the probe and the key-ref call to action for reuse.
The WALLET_ALREADY_CONFIGURED call to action echoed the caller's whole option set, so a --privateKey passed on the refused invocation came back inside the error envelope — into the MCP result, the agent's context and every log downstream. Replay an allowlist instead, with the key as 0x... Adding or changing --keyProject on a configured reference re-scopes the same lookup and replaces nothing, but was refused as destructive; it now passes through. State the consequence that actually applies, too: only a stored private key is destroyed by a swap, while a keystore file stays on disk and a vault key stays in the vault. Update the description and MCP title, which still promised that an explicit method replaces any configured wallet with no mention of --force, and take the key-ref call to action from key-ref.ts rather than from a second copy of it.
The preflight covered the key-reference mode only, so a keystore install still died inside cast as an untyped throw from outside the command's try block — the shape the guard exists to remove. It now also reports KEYSTORE_INTERACTIVE_ONLY under an agent and KEYSTORE_TOOL_MISSING when Foundry is unreachable, plus MALFORMED_KEY_REF for a reference with no provider prefix, which previously escaped as UNKNOWN with no code and no way to act on it. KEY_REF_PROVIDER_MISSING no longer suggests `wallet init --auto --force`. A missing provider is usually a short PATH under an agent, and agents follow calls to action: the only one offered would have replaced a funded vault-backed wallet with a throwaway testnet key. It now carries no command and is marked retryable. Collapse the fifteen pasted copies of the guard into requireWallet(), and assert structurally that every command building a signing client calls it first.
The identification table still told readers an unconfigured wallet surfaces as "Private key not found", contradicting the error catalog forty lines below it. Catalog the new codes, say that a missing provider is retryable and must not be "fixed" by re-initializing, and note that --keyProject on a configured reference needs no --force.
Six findings from review of this branch, all behavioural — the build and the suite were already clean, which is the point: none of these announce themselves. Silent wrong-key hazards: - `wallet init --keyRef X` on a wallet already pinned to `--keyProject` dropped the pin. The force guard correctly reads an identical reference as a no-op and never asks, so the delete ran unconfirmed and the next command re-resolved against the provider's default project — a different key, a different signing address, nothing on screen. The scope is now cleared only when it stops applying: an explicit `--keyProject ""`, or a reference that actually changed. - A configured keystore was invisible to the already-configured checks, so bare `wallet init` fell past them to the interactive prompt, which writes a private key and deletes the keystore. `--force` bypassed entirely. It now reports `already_configured` like the other two modes. - The keystore key scrape kept an unbounded `search()` + `slice(+66)` while the key-reference path in the same branch refused exactly that shape. Every 32-byte value is a valid secp256k1 key, so the first 64 hex digits of a longer blob would have signed as a different address. Both paths now share `findPrivateKeys`, and the ambiguous case is refused rather than guessed. - `--keyRef 0x<64 hex>` — plausible, since it sits beside `--privateKey` in help — was echoed verbatim into `INVALID_KEY_REF` and replayed into the `WALLET_ALREADY_CONFIGURED` CTA, past the allowlist that exists to keep secrets out of envelopes. Redacted by shape now, in the message and in the replay, and the message names `--private-key` instead. Setups that had stopped working, and errors that were not what they said: - Keystore mode was gated on `isAgent()`, which is true whenever stdout is not a TTY — so `wallet balance --json | jq` and `wallet costs > costs.json` began refusing on installs where they had always worked. `cast` reads the password from /dev/tty; a pipe takes nothing away. New `canPrompt()` asks the question actually being asked. - An unrecognized provider prefix reached `isProviderAvailable()`, which cannot tell "does not exist" from "not installed", and was reported as a retryable PATH gap with a null install hint. An agent would retry a permanent misconfiguration forever. Now `UNKNOWN_KEY_REF_PROVIDER`, not retryable, listing the providers that do exist. Also narrows the keystore `try` to the `cast` call it diagnoses — the scrape sat inside it, so "no key found in the output" was reported as "Mac Mismatch means the password was wrong", which is the one thing it is not. Docs updated where they described the old behaviour.
Finding 6 and the validation trio.
**Key resolution failures are typed at the throw.**
`resolveKeyRef` and the keystore decrypt run at *use* time, from inside
the client construction every signing command performs before its try
block — so a plain Error reached incur's top-level handler and rendered
as `{ code: 'UNKNOWN' }`, no code and no `retryable`. That is what an
agent saw for the most common failure a vault-backed wallet has:
installed, but not logged in.
The obvious fix — move construction inside each command's try — would
have made it worse. Those catches end in `out.fail('UPLOAD_FAILED', …)`,
`'COSTS_FAILED'`, `'DATASET_LIST_FAILED'`, so a key that could not be
fetched would be reported as an upload that failed: typed, and wrong.
Throwing `Errors.IncurError` instead fixes all 15 commands at once,
plus any command that does not exist yet, and needs no command edits.
Codes are shared with the ones `walletPreflight` already emits for the
same conditions — whether the guard caught it or the resolver did is an
implementation detail, and the fix is identical either way. New at use
time: `KEY_REF_RESOLUTION_FAILED` (provider ran and refused — explicitly
not retryable, since every cause needs a deliberate act),
`KEY_REF_NOT_A_KEY`, `KEY_REF_AMBIGUOUS`, `KEYSTORE_DECRYPT_FAILED`,
`KEYSTORE_NOT_A_KEY`, `KEYSTORE_AMBIGUOUS`.
**A leading `-` is no longer a legal reference.**
`clawdi:--project` passed SAFE_REF and reached argv as `clawdi vault
resolve --project`, so config steered the helper's own option parsing
rather than naming a secret. Not arbitrary execution, but more than
"only the reference comes from config" allows. A dash anywhere else
stays legal — `FILECOIN-PRIVATE-KEY` still resolves.
**`wallet init` refuses what no command could resolve.**
It validated the `<provider>:<ref>` shape and nothing else, so
`clawdi:MY KEY&touch x` returned `configured`, cleared the previous
wallet, and left every later command failing with "re-run `foc-cli
wallet init`" — pointing back at the command that had just accepted it.
The check is shared with the resolver via `unsafeRefReason`, and the
preflight applies it too so a config already holding one gets a CTA
instead of a bare throw mid-command.
**`--keyProject` on its own now does what both docs promise.**
No branch consumed it: the command fell through to `already_configured`,
wrote nothing, and reported success, so the caller believed a scope was
pinned that never was and every command kept resolving against the
provider's default project. It now re-scopes the configured reference.
Without one it fails `KEY_PROJECT_WITHOUT_KEY_REF`, and combined with
`--auto`/`--privateKey`/`--keystore` it is refused rather than shadowing
them — answering a contradiction by quietly picking one is the same
silent-ignore this removes.
incur renders an example option whose value is `true` as `--withCDN true`, and the parser never reads a boolean's value from the next token — so the flag is enabled and `true` is left over as a stray positional. Nine examples taught that form, and `--prompt upload files` searched for "upload" while silently dropping "files". Drop the boolean options from examples, which cannot express a bare switch, and state the correct syntax in each command's hint instead.
Every CTA offering a switch was unrunnable: incur renders `{ force: true }`
as `--force <force>`, a placeholder a shell reads as a redirect, so the
escape hatch for the whole --force guard could not be run. Switches now
travel in the command string via ctaFlags, and Problem.cta is typed CTA
rather than any — the missing type is how this shipped.
Both external key tools ran execFileSync with no timeout. They are
synchronous, so a hung vault call or a password prompt with nobody to
answer it froze the event loop, and with it the MCP server. Both are now
bounded and report typed retryable failures.
Also: redact a key-shaped keyRef before quoting it into a refusal;
validate the whole init request before writing anything, so a refused
init leaves config untouched and a keystore that cannot work here is
refused on the first call; refuse conflicting custody methods instead of
letting --keyRef win silently; type a malformed stored private key in the
preflight; treat an empty key project as absent, matching the argv
builder; fold dropped IncurError hints into the message, which is the
only field the envelope carries.
Checked every documented command, flag, and error code against the built CLI's help and --schema output. Fixes: --chain and --debug were claimed as global but four commands define neither; `piece list` was shown without its required <dataSetId>; download's --force and refusal to overwrite, wallet costs' --withCDN, and wallet init's --keyProject and --force were undocumented; MCP tools were described as underscore-separated, but `multi-upload` keeps its hyphen, so an agent generalising the rule calls multi_upload and fails. Both skills also had the boolean rule backwards: a value is never read from the next token, so `--flag false` enables the flag. Only `--flag=false` disables it. Documents the codes added alongside: CONFLICTING_INIT_METHODS, KEY_REF_TIMED_OUT, KEYSTORE_TIMED_OUT.
Every drift fixed in the previous commits was only findable by running the example, which is the moment it costs the most. This checks each one against the imported Zod schemas — unknown flags, missing required args, booleans passed a value, extra positionals — so it needs no build and fails on the commit that introduces the drift. Also asserts COMMANDS covers every command module, since a new command nobody registers would be silently exempt.
A key is pure hex, so `--key-ref clawdi:0x<key>` passed the character allowlist: the key was stored under a field documented as safe to display, sent to the provider as a lookup name, and quoted verbatim in the use-time error envelope bound for the MCP result and the logs. Validation now refuses a value that is itself a private key — init and the preflight both, via the shared rule — and every surface that quotes a reference (resolver failures, the unknown-provider message, init's own results) redacts key-like runs first, a no-op for every legitimate reference.
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.
Adds a third custody mode alongside a raw private key and a Foundry keystore: the config holds a
<provider>:<reference>pointer, and the key is fetched into memory per command.foc-cli wallet init --keyRef clawdi:FILECOIN_PRIVATE_KEY foc-cli upload ./blob.json # ordinary usage from here — no wrapper, no prefixWhy
references/keystore-setup.mdalready documents a gap it cannot close: keystore mode prompts for its password on a tty at use time, so under MCP or CI it simply fails, and the only remaining option is a raw key sitting in the config file. That is the mode agents run in.A key reference prompts for nothing. It is the first custody mode that works under automation with no key at rest — and after
wallet init, every command behaves exactly as it does with a raw key or a keystore.Shape
This is not a new concept in the CLI. The keystore path already names an external source in config and runs a helper at use time to produce the key on stdout; this is a second instance of that pattern, with
clawdi vault resolvein place ofcast wallet decrypt-keystore.foc-clialready hardcodes one vendor's binary (Foundry'scast), so a second named provider is not a new kind of coupling.Providers are a closed set: the executable is chosen by code and only the reference comes from config, so a tampered config cannot turn key resolution into arbitrary command execution. That is deliberately not a general "run this command to get my key" field.
What is in here
Key reference mode —
wallet init --keyRef <provider>:<reference>, optional--keyProject(omitted, the provider picks its own default).clawdiis the first provider. Resolution stays lazy, so read-only commands (docs,provider list) never pay for it.wallet initno longer silently replaces a configured wallet. This is a behaviour change and the reason for a minor bump. An explicit method used to overwrite whatever was configured, discarding a key that may have been the only copy. It now names what would be lost — the derived address for a private key, the path for a keystore, the reference for a key reference, never the key itself — and asks on a terminal, or fails withWALLET_ALREADY_CONFIGUREDplus a--forceCTA in agent mode. Re-running the same method with the same value replaces nothing and is never blocked; confirming idempotent calls would only train agents to pass--forcereflexively.Guidance never names a tool you do not have. Key-reference CTAs appear only when that provider's CLI is actually on PATH — a stat, no process, no network. The reference docs still describe every provider; a CTA describes what works on this machine.
Typed preflight. Every wallet-touching command checks the cheap things before constructing a client, so
WALLET_NOT_CONFIGUREDandKEY_REF_PROVIDER_MISSINGarrive as typed errors with next steps instead of escaping as an untyped throw from inside key resolution. The preflight deliberately does not resolve the key: that needs an authenticated provider and a round trip, and belongs at use time.wallet balancereportskeySource. The address proves which key signed; this proves where it came from. Never the value.Deliberate choices worth reviewing
providerAvailable: falsewith a warning, rather than failing. Image layers and provisioning scripts run in an order you do not control, and nothing is at risk until a command signs. The hard failure stays where it is actionable.execFileSyncdoes not apply PATHEXT, and an npm-installed helper isclawdi.cmdthere. Candidates are probed andstated rather than reaching forshell: true, which would put a config-supplied string through a shell.Compatibility
keyRefis absent on every existing install, so the branches below it are reached unchanged and there is nothing to migrate. The one caller-visible change is thewallet initguard: automation that re-runswallet init --autoexpecting a fresh key must now pass--force.Verification
bun test127 pass (18 new),biome checkclean,tsc --noEmitclean,bun run buildclean.Exercised end to end against a real
clawdiand against a stub provider: correct address derived from the resolved key,keySource: keyRef, CTA gating confirmed with the provider on and off PATH, the overwrite guard refusing and then accepting--force, and an idempotent re-set passing through untouched.Not yet measured:
clawdi vault resolvelatency against the live service in a long MCP session.