Commit Graph
4 Commits
Author SHA1 Message Date
thePR0M3TH3ANandClaude Opus 5 7f6edf10cc fix(agent): close two ways a local caller could subvert the session daemon
Both were found by the 2026-08-17 self-review and confirmed by speaking to
the socket directly. The daemon is the enforcement point for the whole token
model and treats every caller as untrusted, so neither fix leans on the CLI's
own validation.

1. A `put` reported as FAILED could leave the seed resident and unexpiring.
   State was committed to `held` before the audit append was awaited. A
   non-numeric ttl made `expiresAt` NaN; the audit write then threw on the
   non-finite value, so the caller saw ok:false while the seed stayed in
   memory forever (`NaN <= now` is always false) with no record it was ever
   unlocked. Now ttl is validated as a positive integer at the boundary, and
   an audit failure rolls the seed back out — ok:false means genuinely locked.

2. A `use`-scoped token could crash the whole agent. `stdinSink` wrote to the
   child's stdin with no error listener, so a command that exits before
   draining raised an unhandled EPIPE; the agent runs sinks in-process and
   installs no uncaughtException handler, so it exited and dropped every held
   seed. The least-privileged principal could take down the enforcement point
   by naming a command that exits early. stdin errors are now swallowed and
   the child's exit code carries the result, as feedClipboardTool already did.

Also hardens token-issue: scopes/kinds are shape-checked before the array
methods run against them (a bare-string kinds turned an exact match into a
substring match), and ttl/uses go through the same positive-integer boundary
so a token can no longer be minted with a NaN, never-expiring lifetime.

Three regression tests speak the wire protocol directly. Verified as negative
controls: with the fixes stashed, the stdin test reproduces the EPIPE uncaught
exception at sinks.ts:143 and two of the three fail; with them restored all
131 CLI tests pass.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Tkj4rzKBA3uhfgk7kqjsb8
2026-08-17 22:15:52 -04:00
thePR0M3TH3ANandClaude Fable 5 dafae50ca4 security: fix agent-boundary findings from the fourth reviewer
The reviewer that took longest found the most, including one bug that lies
to the user about a security-relevant outcome.

- `agent stop` never sent the capability and never checked the reply. The
  daemon refused the shutdown, the CLI printed {"agent":"stopped"} anyway,
  and the seed stayed in memory and still served owner-mnemonic. The command
  documents itself as wiping held seeds. It now authenticates and reports
  what actually happened.

- The owner capability lived at <appDir>/agent.sock.cap -- inside the very
  directory a scoped agent must be given to find fingerprints.json and the
  socket. A token holder could `unset SEEDPASS_TOKEN` and the CLI would read
  the capability on its behalf. It now lives under XDG_RUNTIME_DIR in a 0700
  parent, created with wx so a pre-planted file is never adopted. This raises
  the bar; it does not make a token a sandbox, and capabilities.ts no longer
  claims otherwise.

- Redaction was a three-key denylist over deliberately loose schemas, so
  `custom_fields` -- including values Python masks as is_hidden -- went to
  any read-scoped token verbatim. It is now an allowlist that recurses into
  custom_fields, reporting labels and presence but never values.

- Authorization ran after the vault was decrypted and after the entry
  existence check, giving an unauthenticated caller an entry-id enumeration
  oracle, no audit record for the misses, and a full PBKDF2 unlock per probe.
  Tokens are now checked first, denials are indistinguishable for missing
  and forbidden entries, and every denial is audited.

- The audit chain had no anchor: deleting the log, or a suffix of it, left
  something that verified perfectly. A signed head now pins the record count
  and tail signature. Separately, `loaded` was latched before the read that
  could throw, so one transient EACCES made every later append sign from
  genesis and broke the chain permanently.

- The exec allowlist -- documented as the only real containment for a `use`
  grant -- was bypassable via the clipboard (a session-wide channel) and had
  no CLI flag at all, so no shipped token could carry one. Both fixed.

- Unauthenticated connections could buffer without bound, growing to
  gigabytes while each chunk rescanned the accumulated string. Line size and
  concurrent connections are now capped.

- A token-supplied TOTP timestamp was unbounded, so one use minted codes
  valid decades ahead, outliving the token's TTL. Clamped to a small window.

- Prototype keys ("__proto__", "constructor") resolved to inherited
  properties, passed constraint checks against undefined values and burned a
  use; now an own-property check. The socket was created with the umask mode
  and chmod'd afterwards, leaving a connectable window; it is now created
  under a restrictive umask. TTL was enforced only when a request arrived, so
  an idle agent held the seed indefinitely past expiry; there is now a timer.

220 core + 78 CLI tests green; 36/36 cross-implementation checks green.

Co-Authored-By: Claude Fable 5 <[email protected]>
2026-08-14 23:58:58 -04:00
thePR0M3TH3ANandClaude Fable 5 6b862baa71 security: reject non-string credentials on the agent protocol
Found by running attacks against the socket rather than reading the code:
a token supplied as a single-element array was accepted, because
String(["abc"]) is "abc" and the comparison went through String() coercion.

No privilege was gained -- the attacker still needed the real token -- but
type coercion on a credential path is the kind of sloppiness that becomes a
bug the moment the surrounding code changes. The handler now requires op,
cap, token, fingerprint, id and sink to be strings, and command to be an
array of strings, rejecting anything else outright.

The same run confirmed the rest of the boundary behaves: by-id access to
entries excluded by a token's label regex or kind constraint is refused
server-side with the correct reason, reveal without the reveal scope is
refused, denials do not consume uses, concurrent requests cannot overrun a
one-use token, and a sink child spawned by the agent sees exactly one
SEEDPASS_ variable -- the secret it was given.

Co-Authored-By: Claude Fable 5 <[email protected]>
2026-08-14 23:24:08 -04:00
thePR0M3TH3ANandClaude Fable 5 9926b48e16 security: fix agent trust boundary and secret-environment leaks
A security review found the agent's authorization model was enforced in the
client, not the server: any local process that could open the 0600 socket
could retrieve the parent seed, mint tokens, unlock/lock, or shut the agent
down, because the daemon accepted privileged operations unauthenticated. It
also found that scoped tokens received far more than their grant, and that
sink children inherited the caller's entire environment.

Server-side authorization:
- Privileged operations (put, owner-mnemonic, token issue/list/revoke, lock,
  status, shutdown) now require a capability secret written to a 0600 file
  beside the socket at startup, compared in constant time. Socket permissions
  alone cannot distinguish the owner's CLI from a scoped agent holding a
  token. The residual risk is documented in the code: a same-uid attacker who
  can read that file, or ptrace the daemon, still wins -- that needs an
  OS-level boundary this process cannot provide.
- The mnemonic-returning op is renamed owner-mnemonic and is owner-gated. It
  is unreachable with a token.

Token containment:
- vault-index returned the decrypted index to any read-scoped token, handing
  over every imported TOTP secret, key-value value and document body. It now
  returns redacted metadata, filtered to entries the token's kind and label
  constraints actually cover, so out-of-scope entries are not even disclosed.
- The `secret` op now requires the reveal scope specifically. A use-scoped
  token can no longer pull plaintext back across the socket.
- `use` is served by a new use-sink op: the agent runs the sink itself and
  returns a receipt, so plaintext never crosses back to the token holder.
  Tokens gained an optional exec allowlist, because a holder who chooses the
  command can always read the secret from inside it -- the allowlist is what
  contains a `use` grant, the audit record is what makes it accountable.

Secret hygiene and state:
- Sink children get a minimal allowlisted environment. SEEDPASS_MNEMONIC,
  SEEDPASS_PASSWORD, SEEDPASS_TOKEN, the agent socket and app dir are never
  inherited: a helper invoked to receive one password could previously have
  taken the whole vault.
- Locking (and TTL expiry) now clears that profile's tokens and its audit
  logger, so tokens cannot spring back to life after a later unlock and the
  seed-derived audit key does not stay resident.
- Audit appends are serialized, so concurrent requests cannot interleave and
  break the HMAC chain.
- Numeric CLI options are validated before anything is persisted. A single
  `--length not-a-number` previously stored NaN as JSON null and made every
  later vault open fail schema validation.

test/agentSecurity.test.ts speaks the wire protocol directly, bypassing the
CLI, and asserts each of the review's reproductions now fails: no seed, no
token minting, no lock/shutdown/status/put, no raw index for a read token, no
plaintext for a use token, no owner ops via a token, and no SEEDPASS_* in a
sink child. Also verifies the capability file is 0600 and that tokens die
with a lock.

193 core + 60 CLI tests green; 36/36 cross-implementation checks green.

Co-Authored-By: Claude Fable 5 <[email protected]>
2026-08-14 23:02:07 -04:00