From 25467ee0cdc50b41448f086a7dcf5c6a25205f8c Mon Sep 17 00:00:00 2001 From: olegshmuelov Date: Sun, 16 Aug 2026 13:35:26 +0300 Subject: [PATCH 1/2] fix(brains): resolve the capture credential from the client's MCP sign-in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Conversation capture and the inbox have been off for anyone who never pasted a token, since the plugin's first commit, with no request, no log line and no error to show for it. Three install paths reach that state and one of them is the command the README tells people to type. The credential now falls back to the MCP OAuth token the client already stored, which is the same credential class the hook endpoints accept, so any user whose brains tools work at all has one. Both hooks share one resolver: they used to build separate chains, and a Codex user with header auth had capture on and the inbox off. A resolved credential is a record — source, binding, origin, locator — and the token itself never occupies a shell variable at all. That covers the discovered credential; the three explicitly configured tokens arrive as environment variables and cannot, so the credential path also shields the caller's xtrace and restores it afterwards. Four disclosure channels were closed one at a time (xtrace, curl argv, the user's ~/.curlrc, and `set -a`, which exports locals too) before it became clear that shielding channels one by one was the wrong shape. The value now goes from the store through jq into a private 0600 curl config and from there into curl, so there is nothing to export, trace or inherit. Only brains_request presents it. The one cleanup primitive that removes these files refuses any path that is not a direct child of the temp root it created, so it cannot be aimed at a directory the library does not own. Endpoint URLs are env-overridable, so a discovered credential is bound to the origin that issued it and refused anywhere else; explicitly configured tokens still go wherever the user points them. Selection requires BOTH the canonical origin and a server name that identifies brains — another MCP server sharing the origin would otherwise have its token sent to the brains endpoints — and refuses when more than one distinct candidate matches, because neither store records which account a token belongs to and capturing into the wrong brain is worse than a 401. curl runs with -q so the user's own ~/.curlrc cannot route the header into a trace file, which is a disclosure channel neither argv nor xtrace covers. Every network call is bounded from inside brains_request, so no call site can be unbounded by omission. Store reads share one budget across the whole resolution rather than one deadline each, and candidates are narrowed by server name before any keychain read so that budget effectively never binds. The resolver starts at indeterminate and only a provably complete enumeration may downgrade that to no-credential, so an incomplete scan for any reason — the cap, the budget, an unreadable account, or every account failing at once — reports what it actually knows rather than what it happened to find. A keychain that answers "item not found" is a conclusive absence and stays no-credential, so a fresh install is still told to sign in; that distinction travels as an exit status, because every call site is a command substitution and a variable set in one never reaches the caller. An unread account matters for the same reason a truncated scan does: it may hold a different credential for the same server, and the readable one would otherwise be handed back as uniquely valid. A record that parses cleanly and simply holds no token is a conclusive answer about that account and does not count as incomplete, so one junk keychain entry cannot disable capture permanently. Reads clean up their temporary directory on INT, TERM and HUP as well as on the normal path, and sweep the reader's process group so a forked descendant cannot outlive them. Backgrounded requests take their own lease on the credential and clean it up on the same signals, so neither the hook's exit nor a terminated child leaves the bearer on disk; SIGKILL is untrappable and is swept by a prune at session start and session end. The off-state is now observable. Every request records a health state keyed by capability and by the origin of the URL it actually contacts — the same key the credential binding uses, since every endpoint is independently overridable — ordered by logical generation, so a stale completion cannot overwrite a newer one, a working inbox cannot clear a broken capture, and a healthy production session cannot clear a self-hosted one's rejection. Any non-zero curl exit is a transport failure whatever the status says: curl writes the code when headers arrive, so a transfer that dies mid-body still reports 200 and its truncated body can be valid JSON. Session start announces the state once, naming the one command that fixes it, and only an observed success re-arms it. The suite mutates the shipped code and requires the damage to show: each registered mechanism is neutered in a copy of the plugin tree and a probe must change its answer. Two guards had already decayed into tautologies that passed with their mechanism deleted, and three mechanisms shipped with no guard at all; this makes both states fail the suite rather than pass it. Codex reads its sign-in from the macOS keychain, and Codex on Linux is not a supported configuration, so there the note says that and names nothing rather than pointing at a token workaround for a platform the product does not support. Also: the current-time injection moves ahead of the credential gate, where it always belonged; per-session marker files are pruned; and the token option and README no longer describe a gate that no longer exists. Codex on Linux is called out explicitly as still needing a token, because where it stores its sign-in there has not been verified. BRNS-CORE-085 --- .github/workflows/tests.yml | 15 + README.md | 36 +- plugins/brains/.claude-plugin/plugin.json | 4 +- plugins/brains/.codex-plugin/plugin.json | 2 +- plugins/brains/core.md | 15 +- plugins/brains/hooks/brains-end.sh | 10 + plugins/brains/hooks/brains-start.sh | 20 +- plugins/brains/hooks/brains-turn.sh | 137 +- plugins/brains/hooks/lib/brains-credential.sh | 1137 +++++++++++++++ plugins/brains/hooks/lib/brains-inbox.sh | 86 +- tests/credential/run.ts | 1280 +++++++++++++++++ tests/credential/stubs.js | 46 + tests/inbox-v2/demo-approval-followthrough.ts | 2 + tests/inbox-v2/demo-safety-limit.ts | 2 + tests/inbox-v2/live-claude.ts | 2 + tests/inbox-v2/run.ts | 4 +- tests/plugin-contract/run.ts | 260 +++- 17 files changed, 2925 insertions(+), 133 deletions(-) create mode 100644 plugins/brains/hooks/lib/brains-credential.sh create mode 100644 tests/credential/run.ts create mode 100644 tests/credential/stubs.js diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 43f29d3..35bc697 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -42,6 +42,21 @@ jobs: bun-version: "1.3.8" - run: bun run tests/tool-error/run.ts + credential: + name: capture credential resolver + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: oven-sh/setup-bun@v2 + with: + bun-version: "1.3.8" + # The stub servers run as a node child process, because the suite drives the + # hooks through spawnSync and would otherwise deadlock against itself. + - uses: actions/setup-node@v4 + with: + node-version: "22" + - run: bun run tests/credential/run.ts + generated-artifact-guard: name: generated-artifact + delivery guard runs-on: ubuntu-latest diff --git a/README.md b/README.md index 3d4792d..5358a15 100644 --- a/README.md +++ b/README.md @@ -36,8 +36,12 @@ keep. Confirm with `codex mcp list`: brains should read **OAuth**. Restart the ChatGPT desktop app or start a new Codex thread. The first time the plugin loads, open `/hooks` and trust the bundled brains hooks — that is what -runs automatic recall and error feedback. Capture and inbox delivery also need a -capture credential — normally the token below. +runs automatic recall and error feedback. Capture and inbox delivery use the +sign-in above as their credential, so there is nothing further to set. + +Capture and the inbox are **macOS only** for Codex: they read the credential +from the macOS keychain, and Codex on Linux is not a supported configuration. +The tools and recall still work there; capture and inbox delivery do not. Everyday reading and writing is covered by default. For admin-gated tools or performance insights, sign in asking for them explicitly (both also need the @@ -56,12 +60,15 @@ codex plugin add brains@brains codex mcp login brains ``` -### Optional: conversation capture and the inbox +### Optional: an explicit capture token + +You do not need this. Capture and the inbox read the credential `codex mcp login +brains` already stored, so the sign-in above is all they need. To check what has +been captured, ask brains which chats it has, or run +`list_pages type=chat_session`. -The tools above work without this. Capture and the inbox are shell hooks that -authenticate separately from the MCP server and cannot read the credential Codex -keeps internally, so they need a brains API token of their own — find it in your -brains account settings. Without one they simply stay off. +Set a token to capture into a different brains account, or to reach an endpoint +your sign-in does not cover — find it in your brains account settings: ```sh export BRAINS_API_TOKEN="" @@ -105,7 +112,8 @@ command, update Claude Code. Restart Claude Code or start a new session. The first time the plugin loads, trust the bundled brains hooks — that is what runs automatic recall and error feedback. Capture and inbox delivery -also need the token below. +use the sign-in above as their credential, so there is normally nothing further to set. If +`list_pages type=chat_session` shows nothing after a few turns, set the token below. For a local checkout under development: @@ -115,14 +123,14 @@ claude plugin install brains@brains claude mcp login plugin:brains:brains ``` -### Optional: conversation capture and the inbox +### Optional: an explicit capture token -The tools above work without this. Capture and the inbox are shell hooks that authenticate -separately from the MCP server and cannot read the credential Claude Code keeps internally, so -they need a brains API token of their own — find it in your brains account settings. Without one -they simply stay off. +You do not need this. Capture and the inbox read the credential `claude mcp login +plugin:brains:brains` already stored, so the sign-in above is all they need. To check what has +been captured, ask brains which chats it has, or run `list_pages type=chat_session`. -Set it when you install: +Set a token only to capture into a different brains account, or to reach an endpoint your sign-in +does not cover — find it in your brains account settings: ```sh claude plugin install brains@brains --config token="" diff --git a/plugins/brains/.claude-plugin/plugin.json b/plugins/brains/.claude-plugin/plugin.json index 3f0dca1..7d8409a 100644 --- a/plugins/brains/.claude-plugin/plugin.json +++ b/plugins/brains/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "brains", "description": "Your memory layer: Gmail, Calendar, Drive, and prior Claude conversations as queryable pages, with reflexive recall, hook-driven turn-by-turn capture and inbox delivery, and boards/automations/workflows on top.", - "version": "2.9.1", + "version": "2.9.2", "author": { "name": "brains (ssvlabs)" }, @@ -11,7 +11,7 @@ "token": { "type": "string", "title": "brains API token (optional)", - "description": "Optional. Enables conversation capture and the inbox, which authenticate separately from the MCP server. NOT how the brains tools authenticate — that is `claude mcp login plugin:brains:brains`. Find it in your brains account settings; without one, capture and the inbox simply stay off.", + "description": "Optional override. Conversation capture and the inbox follow your MCP sign-in — `claude mcp login plugin:brains:brains` — and need nothing set here. Use this only to capture into a different brains account, or to reach a self-hosted endpoint your sign-in does not cover. Find it in your brains account settings.", "sensitive": true, "required": false }, diff --git a/plugins/brains/.codex-plugin/plugin.json b/plugins/brains/.codex-plugin/plugin.json index 51b3452..ff512d2 100644 --- a/plugins/brains/.codex-plugin/plugin.json +++ b/plugins/brains/.codex-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "brains", - "version": "2.9.1", + "version": "2.9.2", "description": "Your personal memory layer for Codex: query Gmail, Calendar, Drive, and prior conversations, then build boards, automations, and workflows.", "author": { "name": "brains (ssvlabs)", diff --git a/plugins/brains/core.md b/plugins/brains/core.md index ab13d1c..7c47a5a 100644 --- a/plugins/brains/core.md +++ b/plugins/brains/core.md @@ -1,4 +1,4 @@ - + # brains — your memory layer You have a memory layer called **brains** (the `brains` MCP server). It holds the @@ -22,11 +22,14 @@ For schedules and agendas, use `list_calendar_events start=… end=…`; calenda page update time is not event time. Name the source page's `title` and `type`, and never invent slugs or IDs. -**Capture.** In Codex and Claude Code the ingest hook saves each turn, but only -where a capture credential resolves — so never promise capture and never deny -it; `list_pages type=chat_session` is the only way to know. Don't call -`save_chat_session` routinely there; do call it when asked, and where the hooks -don't run (claude.ai web) it is the only path. +**Capture.** In Codex and Claude Code the ingest hook saves each turn, using the +same sign-in that authenticates these tools. A `brains:capture` note means it is +OFF and is authoritative: surface it once and, if the user agrees, do exactly the +one step it names — a command or a setting — never a command it did not name. +Without a note, never promise capture and never deny it; `list_pages +type=chat_session` is the only confirmation. Don't call `save_chat_session` +routinely there; do call it when asked, and where the hooks don't run (claude.ai +web) it is the only path. **The skills carry the detail** — load the one that fits the moment: `brains-read` (querying memory), `brains-write` (sending/creating via diff --git a/plugins/brains/hooks/brains-end.sh b/plugins/brains/hooks/brains-end.sh index 9a01664..ea876fc 100755 --- a/plugins/brains/hooks/brains-end.sh +++ b/plugins/brains/hooks/brains-end.sh @@ -14,4 +14,14 @@ HOOK_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" LIB="$HOOK_DIR/lib/brains-inbox.sh" [ -x "$LIB" ] && "$LIB" stop "$SESSION" +# Sweep credential work directories orphaned by SIGKILL, which is untrappable so +# nothing else can have cleaned up after it. Pruning here as well as at session +# start is what keeps that backstop from meaning "until someone starts a new +# session". Best effort, never fatal, no output. +CRED_LIB="$HOOK_DIR/lib/brains-credential.sh" +if [ -r "$CRED_LIB" ]; then + # shellcheck source=lib/brains-credential.sh + . "$CRED_LIB" 2>/dev/null && brains_cred_prune_tmp +fi + exit 0 diff --git a/plugins/brains/hooks/brains-start.sh b/plugins/brains/hooks/brains-start.sh index 6832705..409791a 100755 --- a/plugins/brains/hooks/brains-start.sh +++ b/plugins/brains/hooks/brains-start.sh @@ -44,7 +44,25 @@ fi # hook that runs automatically with the user's token in env. Removed until # we have an out-of-repo allowlist / fingerprint mechanism.) -# 4. Inbox engine (device report + full inbox + ack). Emits its own context. +# 4. Housekeeping. Per-session marker files (now-*, toolerr-seen-*, capok-*) +# are written by the turn and tool-error hooks and nothing ever removed +# them, so the data dir grew without bound. Also sweeps read directories +# orphaned by SIGKILL, which is untrappable and so leaves no other cleanup. +# Best effort, never fatal, no output. +CRED_LIB="$HOOK_DIR/lib/brains-credential.sh" +STATE_DIR="${BRAINS_STATE_DIR:-${PLUGIN_DATA:-${CLAUDE_PLUGIN_DATA:-$HOME/.claude/brains}}}" +if [ -d "$STATE_DIR" ]; then + find "$STATE_DIR" -maxdepth 1 -type f \ + \( -name 'now-*' -o -name 'toolerr-seen-*' -o -name 'capok-*' \) \ + -mtime +7 -delete 2>/dev/null +fi +if [ -r "$CRED_LIB" ]; then + # shellcheck source=lib/brains-credential.sh + . "$CRED_LIB" 2>/dev/null && brains_cred_prune_tmp +fi + +# 5. Inbox engine (device report + full inbox + ack). Emits its own context, +# including the one-time capture-off signal when no credential resolves. [ -x "$LIB" ] && "$LIB" startup "$SESSION" exit 0 diff --git a/plugins/brains/hooks/brains-turn.sh b/plugins/brains/hooks/brains-turn.sh index 7fca9bd..dc6267b 100755 --- a/plugins/brains/hooks/brains-turn.sh +++ b/plugins/brains/hooks/brains-turn.sh @@ -15,10 +15,11 @@ # mode (notifications only, no # stdout). Fires AFTER the turn. # -# Ingest is the capture path WHERE IT RUNS, and it is credential-gated: with a -# TOKEN (see the gate below) every turn POSTs to /ingest/claude and the server -# builds the chat_session page. Without one this hook exits silently, and where -# the hooks do not run at all (claude.ai web) save_chat_session is the only path. +# Ingest is the capture path WHERE IT RUNS. It needs a credential, which now +# comes from lib/brains-credential.sh: an explicitly configured token if there +# is one, otherwise the client's own MCP OAuth store. Where the hooks do not run +# at all (claude.ai web) save_chat_session is the only path. +# # Claude keeps the existing fire-and-forget delivery. Codex waits for its # assistant POST during Stop so the hook process cannot finish before the # response has been handed to the ingest endpoint. @@ -29,26 +30,12 @@ set -u CLIENT="claude" [ -n "${PLUGIN_ROOT:-}" ] && CLIENT="codex" -TOKEN="${CLAUDE_PLUGIN_OPTION_TOKEN:-${BRAINS_API_TOKEN:-${BRAINS_INBOX_TOKEN:-}}}" -# Desktop-launched Codex receives BRAINS_API_TOKEN directly. A standalone -# Codex CLI can instead have an authenticated MCP transport with a persisted -# Authorization header, so reuse that same credential for automatic capture. -# `codex mcp get` is a local config read; its output is never logged. -if [ -z "$TOKEN" ] && [ "$CLIENT" = "codex" ] && command -v codex >/dev/null 2>&1; then - AUTH_HEADER=$(codex mcp get brains --json 2>/dev/null \ - | jq -r '.transport.http_headers.Authorization // .transport.http_headers.authorization // empty' 2>/dev/null) - case "$AUTH_HEADER" in - "Bearer "*) TOKEN="${AUTH_HEADER#Bearer }" ;; - esac - unset AUTH_HEADER -fi -[ -z "$TOKEN" ] && exit 0 BASE="${CLAUDE_PLUGIN_OPTION_ENDPOINT:-${BRAINS_ENDPOINT:-https://mcp.mybrains.ai}}" BASE="${BASE%/}" -INGEST="${BRAINS_INGEST_URL:-$BASE/ingest/claude}" HOOK_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" LIB="$HOOK_DIR/lib/brains-inbox.sh" +CRED_LIB="$HOOK_DIR/lib/brains-credential.sh" INPUT=$(cat) SESSION=$(printf '%s' "$INPUT" | jq -r '.session_id // empty' 2>/dev/null) @@ -58,36 +45,110 @@ PROMPT=$(printf '%s' "$INPUT" | jq -r '.prompt // empty' 2>/dev/null) TRANSCRIPT=$(printf '%s' "$INPUT" | jq -r '.transcript_path // empty' 2>/dev/null) LAST_ASSISTANT=$(printf '%s' "$INPUT" | jq -r '.last_assistant_message // empty' 2>/dev/null) +STATE_DIR="${BRAINS_STATE_DIR:-${PLUGIN_DATA:-${CLAUDE_PLUGIN_DATA:-$HOME/.claude/brains}}}" + +# ---- current time, ahead of the capture gate -------------------------------- +# Inject the current LOCAL time, but at MOST once per clock-hour per session +# (first turn of a session + whenever the hour rolls over) — not every turn, so +# the model has an accurate "now" without per-turn noise. +# +# This runs BEFORE any credential work on purpose. It has nothing to do with +# capture, and while it sat behind the credential gate a user without a token +# silently lost accurate time injection as collateral. +if [ -n "$PROMPT" ]; then + _now_key=$(date '+%Y%m%d%H') + _now_file="$STATE_DIR/now-$SESSION" + if [ "$_now_key" != "$(cat "$_now_file" 2>/dev/null)" ]; then + mkdir -p "$(dirname "$_now_file")" 2>/dev/null && printf '%s' "$_now_key" > "$_now_file" 2>/dev/null + printf 'now: %s\n' "$(date '+%a %Y-%m-%d %H:%M %Z (%z)')" + fi +fi + +# ---- capture credential ----------------------------------------------------- +[ -r "$CRED_LIB" ] || exit 0 +# shellcheck source=lib/brains-credential.sh +. "$CRED_LIB" || exit 0 + +BRAINS_CRED_CLIENT="$CLIENT" +# The endpoint set comes from the resolver so both hooks agree on which URLs +# exist; health is keyed by the origin each one resolves to. +brains_resolve_endpoints "$BASE" + +LOG="$STATE_DIR/brains.log" +_mode="turn" +[ -n "$PROMPT" ] || _mode="stop" +log() { mkdir -p "$STATE_DIR" 2>/dev/null; printf '[%s] [%s] %s\n' "$(date -u +%FT%TZ)" "$_mode" "$*" >> "$LOG" 2>/dev/null; } + +if ! brains_resolve_credential "$BASE"; then + # No credential, or more than one that could be the right one. Record it so + # the session-start hook can say so once, then behave exactly as before: + # no request, no error, no noise on this turn. + brains_health_note ingest "$BRAINS_URL_INGEST" "$BRAINS_CRED_STATE" + exit 0 +fi + +# One line per outcome, and a healthy one only the first time in a session, so +# the log answers "is capture working" without growing by two lines a turn. +# Before this the log had never carried a single line about capture, which is +# why a credential that stopped working went unnoticed for eleven days. +capture_log() { # role, outcome + local role outcome marker + role="$1"; outcome="$2" + if [ "$outcome" = "ok" ]; then + marker="$STATE_DIR/capok-$SESSION" + [ -f "$marker" ] && return 0 + printf '%s' '1' > "$marker" 2>/dev/null + fi + log "capture $role: $outcome status=${BRAINS_HTTP_CODE:-none} source=$BRAINS_CRED_SOURCE" +} + +ingest_once() { # role, content + local role payload outcome + role="$1"; payload="$2" + # An explicit ceiling on top of the resolver's default, because this is the + # one call Codex makes SYNCHRONOUSLY: the Stop hook waits for the assistant + # POST, so a server that accepts the connection and then never answers would + # hold up the turn. On Claude the request is backgrounded and an unbounded one + # would linger instead of exiting. + if brains_request ingest "$BRAINS_URL_INGEST" --max-time 5 \ + -X POST -H "Content-Type: application/json" -d "$payload"; then + outcome="ok" + elif [ "$BRAINS_HTTP_BLOCKED" = "1" ]; then + outcome="blocked" + else + case "${BRAINS_HTTP_CODE:-}" in + 401|403) outcome="rejected" ;; + ''|000) outcome="unreachable" ;; + *) outcome="error" ;; + esac + fi + capture_log "$role" "$outcome" +} + ingest() { # role, content - local role="$1" content="$2" + local role content payload + role="$1"; content="$2" [ -z "$content" ] && return 0 - local payload payload=$(jq -nc --arg s "$SESSION" --arg r "$role" --arg c "$content" --arg client "$CLIENT" \ '{session_id:$s, role:$r, content:$c, client:$client, client_type:"cli"}') if [ "$CLIENT" = "codex" ] && [ "$role" = "assistant" ]; then - curl -s --max-time 5 -X POST "$INGEST" \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d "$payload" >/dev/null 2>&1 || true + # Synchronous: Codex Stop must not finish before the response is delivered. + ingest_once "$role" "$payload" else - ( curl -s --max-time 5 -X POST "$INGEST" \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d "$payload" >/dev/null 2>&1 || true ) & + # Fire-and-forget, so this request outlives the hook. It takes its own lease + # on the credential first — the hook's exit would otherwise remove the + # config while curl was still starting up, and the POST would go out + # unauthenticated. + _lease=$(brains_cred_lease) || return 0 + ( BRAINS_CRED_CONFIG="$_lease" + trap 'brains_cred_return "$_lease"' EXIT INT TERM HUP + ingest_once "$role" "$payload" + brains_cred_return "$_lease" ) & fi } if [ -n "$PROMPT" ]; then # ---- UserPromptSubmit: ingest user message, light inbox + user hooks ----- - # Inject the current LOCAL time, but at MOST once per clock-hour per session - # (first turn of a session + whenever the hour rolls over) — not every turn, so - # the model has an accurate "now" without per-turn noise. - _now_key=$(date '+%Y%m%d%H') - _now_file="${BRAINS_STATE_DIR:-$HOME/.brains}/now-$SESSION" - if [ "$_now_key" != "$(cat "$_now_file" 2>/dev/null)" ]; then - mkdir -p "$(dirname "$_now_file")" 2>/dev/null && printf '%s' "$_now_key" > "$_now_file" 2>/dev/null - printf 'now: %s\n' "$(date '+%a %Y-%m-%d %H:%M %Z (%z)')" - fi ingest user "$PROMPT" [ -x "$LIB" ] && "$LIB" prompt "$SESSION" diff --git a/plugins/brains/hooks/lib/brains-credential.sh b/plugins/brains/hooks/lib/brains-credential.sh new file mode 100644 index 0000000..6f7b9e4 --- /dev/null +++ b/plugins/brains/hooks/lib/brains-credential.sh @@ -0,0 +1,1137 @@ +#!/usr/bin/env bash +# brains plugin — shared capture-credential resolver and the ONE HTTP call site. +# Sourced (not executed) by brains-turn.sh and by lib/brains-inbox.sh. +# +# Why this file exists: capture and the inbox authenticate with a bearer token +# that, before this, only ever came from explicit configuration. A user who +# never pasted a token had no credential, so both features stayed off with no +# request, no log line and no error. The MCP sign-in the client already performs +# mints a token of the SAME class against the SAME table, so when nothing is +# configured we resolve the credential from the client's own MCP OAuth store. +# +# TWO INVARIANTS carry the safety of this file. Both are structural: they remove +# whole classes of mistake rather than guarding known instances, because each +# class here was found the hard way, one instance at a time. +# +# I1. THE TOKEN NEVER OCCUPIES A SHELL VARIABLE. +# Four separate disclosure channels were closed in turn — xtrace, curl's +# argv, the user's ~/.curlrc, and `set -a` (which exports locals too, so +# even a function-scoped copy reaches every child's environment). Each fix +# shielded one channel and the next one appeared. So the value is never +# assigned: it goes from the store, through jq, into a private curl config +# file, and from there into curl. Nothing to export, nothing to trace, +# nothing to inherit, and no fifth channel to discover. +# `BRAINS_CRED_SOURCE` / `BINDING` / `ORIGIN` / `LOCATOR` are NOT secret +# and remain ordinary variables. +# +# I2. UNKNOWN IS THE DEFAULT STATE. +# Three separate "incomplete enumeration" cases were closed in turn — the +# candidate cap, an unreadable account, and every account failing at once. +# Each was a way of reporting "there is no credential" when the truth was +# "I could not tell", which sends the user to a remedy that cannot work. +# So the resolver STARTS at indeterminate and only a provably complete +# enumeration may downgrade it to no-credential. A future failure mode +# nobody has thought of fails safe without being enumerated. +# +# Beyond those: only brains_request() ever presents the credential, because +# endpoint URLs are env-overridable and a discovered credential must never reach +# a host that did not mint it. And every failure returns "no credential" with +# the caller behaving exactly as it did before this file existed. + +# ---------------------------------------------------------------- test hooks +# The suites run on developer machines that hold a REAL credential. Without a +# way to switch the store reads off, a "no token configured" scenario would +# resolve the developer's own token and POST to production. These exist so the +# tests can be hermetic; they are not a user-facing interface. +# BRAINS_CREDENTIAL_STORE_DISABLED=1 skip every discovery step (4 and 5) +# BRAINS_CLAUDE_CREDENTIALS_FILE read the Claude store from this file +# BRAINS_CODEX_CREDENTIALS_FILE read the Codex store from this file +# (a JSON array of Codex records) + +# The credential DOCUMENT ceiling. This bounds memory; it is not a policy, and +# it must not be small enough to reject a store a real user can accumulate. The +# brains entry is under 1 KB, but the document holds EVERY MCP server the user +# has signed into, and at 256 KiB — roughly a hundred servers — a perfectly +# valid store was rejected and the user was told to remove a duplicate that did +# not exist. 4 MiB is about a thousand servers and still bounded. +BRAINS_CRED_MAX_BYTES=4194304 +BRAINS_CRED_MAX_BLOCKS=8192 +# The keychain METADATA dump is a different size class entirely and needs its +# own ceiling: measured at 198,137 bytes for 209 items on an ordinary machine, +# which is 76% of the document cap. At ~300 items that cap silently truncates +# the dump, the account list comes back short, and Codex capture turns off with +# no signal. This contains no secrets — it is names and dates — so the only job +# of this limit is to stop a pathological keychain eating memory. +BRAINS_CRED_DUMP_MAX_BYTES=16777216 +BRAINS_CRED_DUMP_MAX_BLOCKS=32768 +# Wall-clock ceiling on a single store read. `security` answers in ~30ms; this +# only matters when a keychain item carries a restrictive ACL, in which case it +# blocks on a GUI dialog that stdin redirection cannot suppress. +BRAINS_CRED_READ_DEADLINE=1 +# ...and a ceiling on ALL of them together. Per-read deadlines do not compose: +# the Codex store is enumerated one account at a time, so N stale entries cost +# N x the per-read deadline, and this hook runs on every prompt and every stop. +BRAINS_CRED_TOTAL_BUDGET=3 +BRAINS_CRED_MAX_CANDIDATES=8 +_brains_cred_deadline_at=0 + +# Default network ceilings. These live HERE, not at the call sites, because a +# call site that forgets one is unbounded: ingest lost its `--max-time 5` in +# exactly that way, and ingest is the path Codex runs synchronously during Stop, +# where a server that accepts and never answers hangs the hook forever. curl +# honours the LAST occurrence of a repeated flag, so a caller can still tighten +# either value by passing its own. +BRAINS_CRED_CONNECT_TIMEOUT=5 +BRAINS_CRED_MAX_TIME=10 + +# States: ok | no-credential | indeterminate | blocked. +# See I2 — indeterminate is the starting point, not an error path. +BRAINS_CRED_STATE="indeterminate" +BRAINS_CRED_SOURCE="" +BRAINS_CRED_BINDING="" +BRAINS_CRED_ORIGIN="" +BRAINS_CRED_LOCATOR="" # which entry won; a key or an account name, never a token +BRAINS_CRED_CONFIG="" # path to the private curl config holding the header +BRAINS_CRED_COUNT=0 # distinct candidates seen; a count, never a name +BRAINS_CRED_TRUNCATED=0 # an enumeration hit a limit or a read failed +BRAINS_CRED_SAW_ENTRIES=0 # the store held credentials, even if none matched + +brains_state_dir() { + printf '%s' "${BRAINS_STATE_DIR:-${PLUGIN_DATA:-${CLAUDE_PLUGIN_DATA:-$HOME/.claude/brains}}}" +} + +# ------------------------------------------------------------- the endpoint set +# One definition of where the four endpoints are, because both hooks need them +# and health is keyed by the origin each one actually resolves to. When the turn +# hook and the inbox engine each built these separately, the session-start +# signal could not find the capture health the turn hook had written. +BRAINS_URL_INGEST="" +BRAINS_URL_INBOX="" +BRAINS_URL_ACK="" +BRAINS_URL_DEVICES="" +brains_resolve_endpoints() { # base + local base + base="${1:-}" + base="${base%/}" + BRAINS_URL_INGEST="${BRAINS_INGEST_URL:-$base/ingest/claude}" + BRAINS_URL_INBOX="${BRAINS_INBOX_URL:-$base/inbox/claude}" + BRAINS_URL_ACK="${BRAINS_INBOX_ACK_URL:-${BRAINS_URL_INBOX}/ack}" + BRAINS_URL_DEVICES="${BRAINS_INBOX_DEVICES_URL:-${BRAINS_URL_INBOX}/devices}" +} + +# ------------------------------------------------------------- xtrace shielding +# I1 keeps the DISCOVERED credential out of every shell variable, but the three +# EXPLICITLY configured tokens arrive as environment variables and there is +# nothing to be done about that — they have to be tested for emptiness and read +# to build the config. Both of those expand the value, and `bash -x`, an +# inherited SHELLOPTS=xtrace, or BASH_XTRACEFD prints it. +# +# So this is not redundant with I1; it covers the half of the precedence chain +# I1 cannot reach. Removing it on the strength of I1 alone reopened the channel +# for all three variables, which is why the leak battery now exercises the +# explicit branches as well as the store branch. +_brains_xtrace_off() { + case "$-" in + *x*) BRAINS_CRED_XTRACE=1; set +x ;; + *) BRAINS_CRED_XTRACE="" ;; + esac +} +_brains_xtrace_restore() { + [ -n "${BRAINS_CRED_XTRACE:-}" ] && set -x + BRAINS_CRED_XTRACE="" + return 0 +} + +# --------------------------------------------------------------- time budget +# One budget for the WHOLE resolution rather than one per read. SECONDS is a +# bash builtin, so checking it costs no fork on a path that runs every turn. +_brains_budget_start() { _brains_cred_deadline_at=$((SECONDS + BRAINS_CRED_TOTAL_BUDGET)); } +_brains_budget_left() { [ "$SECONDS" -lt "$_brains_cred_deadline_at" ]; } + +# --------------------------------------------------------------- private tmp +# Returns 0 and sets BRAINS_CRED_TMP, or returns 1 (caller -> no credential). +# There is deliberately NO fallback to /tmp: a resolver that shops around for a +# usable directory is the same mistake as one that shops around for a usable +# credential store. +BRAINS_CRED_TMP="" +_brains_cred_tmp_root() { + local root + root="$(brains_state_dir)/tmp" + ( umask 077; mkdir -p "$root" ) 2>/dev/null || return 1 + [ -d "$root" ] || return 1 + [ -L "$root" ] && return 1 + [ -O "$root" ] || return 1 + [ -w "$root" ] || return 1 + BRAINS_CRED_TMP="$root" + return 0 +} + +# Remove work directories orphaned by SIGKILL, which is untrappable, so nothing +# else can have cleaned up after it. +# +# The bound this provides is honest but weak, and worth stating plainly: it is +# "until the next session start or end", not a wall-clock guarantee. Every other +# path — normal completion, EXIT, INT, TERM, HUP, and the leased background +# subshells — removes its own directory promptly; this only catches SIGKILL. +# Five minutes is far longer than any live directory needs (a lease lives for +# one request, the master config for one hook) while staying clear of anything +# in flight. +brains_cred_prune_tmp() { + local root + root="$(brains_state_dir)/tmp" + [ -d "$root" ] || return 0 + find "$root" -maxdepth 1 -name 'r.*' -type d -mmin +5 -exec rm -rf {} + 2>/dev/null + return 0 +} + +# ------------------------------------------------------------- bounded read +# Run a command with a deadline and a size ceiling, and echo the PATH of a file +# holding its stdout. The caller reads that file with a tool — never into a +# shell variable — and removes it. See I1. +# +# The value travels through a private FILE rather than a pipe for a second +# reason too: a pipe read ends only when every write descriptor closes, and a +# descendant we cannot enumerate can hold one open indefinitely. Two earlier +# pipe-based versions hung well past their deadline and returned bytes written +# after it. Reading a regular file ends at EOF, which the reader controls. +# +# The rc file is the success marker: if the command was killed it is never +# written, so partial output is discarded rather than parsed. +_brains_bounded_read_file() { # deadline, max-bytes, max-blocks, command... + local deadline maxbytes maxblocks dir rc sz + deadline="$1"; maxbytes="$2"; maxblocks="$3"; shift 3 + [ -n "$BRAINS_CRED_TMP" ] || return 1 + dir="$BRAINS_CRED_TMP/r.$$.$RANDOM" + ( umask 077; mkdir "$dir" ) 2>/dev/null || return 1 + ( + # umask INSIDE the subshell that performs the redirect. Scoping it to the + # mkdir above left the document itself world-readable (-rw-r--r--) for the + # duration of the read; the 0700 parent made that defense in depth rather + # than an exposure, but the file should not depend on the directory. + umask 077 + # A plain cleanup trap, in the subshell that owns the work. An earlier + # version saved, restored and re-raised in the PARENT — which could never + # work, because every caller runs this inside a command substitution where + # trap changes are discarded and `kill $$` targets the wrong shell. + trap 'rm -rf "$dir" 2>/dev/null' EXIT INT TERM HUP + set -m + ( ulimit -f "$maxblocks" 2>/dev/null + "$@" >"$dir/v" 2>/dev/null + printf '%s' "$?" >"$dir/rc" ) & _p=$! + set +m + ( sleep "$deadline" + kill -TERM -"$_p" 2>/dev/null + sleep 0.2 + kill -KILL -"$_p" 2>/dev/null ) >/dev/null 2>&1 & _w=$! + wait $_p 2>/dev/null + # Sweep the worker's whole process group BEFORE standing the watchdog down. + # A leader that forks a sleeping descendant and exits 0 leaves that + # descendant behind, and the watchdog is the only thing that would reach it. + kill -TERM -"$_p" 2>/dev/null + kill $_w 2>/dev/null; wait $_w 2>/dev/null + trap - EXIT INT TERM HUP + ) >/dev/null 2>&1 + rc="" + [ -f "$dir/rc" ] && read -r rc <"$dir/rc" 2>/dev/null + sz=$(wc -c <"$dir/v" 2>/dev/null | tr -d ' ') + case "$sz" in ''|*[!0-9]*) sz=0 ;; esac + if [ "$rc" = "0" ] && [ "$sz" -gt 0 ] && [ "$sz" -le "$maxbytes" ]; then + printf '%s' "$dir/v" + return 0 + fi + rm -rf "$dir" 2>/dev/null + # Carry the COMMAND'S OWN exit status out, so a caller can tell "the item is + # not there" (security exits 44) from "the read failed or was killed". This + # has to be the return value: every call site is a command substitution, and a + # variable assigned in one never reaches the caller. An earlier version + # identified that constraint in a comment and then used a variable anyway, so + # conclusive absence never fired and a fresh install — the first-run path this + # whole change exists for — was told its store was unreadable. + case "$rc" in + ''|0|1) return 1 ;; + *[!0-9]*) return 1 ;; + *) [ "$rc" -le 255 ] && return "$rc"; return 1 ;; + esac +} + +# Remove a file produced by _brains_bounded_read_file, and its directory. +# +# This is an `rm -rf` on a path derived from its argument, and arguments here +# have twice turned out to be caller-supplied rather than ours — the Codex +# override path fed it the user's own store directory, and two earlier instances +# fed it the shared Claude snapshot. Scoping each call site fixed each instance +# and left the primitive able to delete anything. +# +# So the primitive refuses: it removes ONLY a directory that is a direct child of +# the temp root this library created. Not the root itself, not anything above it, +# and nothing outside it. A wrong argument is now a silent no-op rather than +# data loss. +_brains_discard() { + local target parent + target="${1:-}" + [ -n "$target" ] || return 0 + [ -n "${BRAINS_CRED_TMP:-}" ] || return 0 + parent=$(dirname "$target" 2>/dev/null) || return 0 + # Must be a direct child of the owned root, and never the root itself. + case "$parent" in + "$BRAINS_CRED_TMP"/*/*) return 0 ;; + "$BRAINS_CRED_TMP"/?*) ;; + *) return 0 ;; + esac + case "$parent" in + *..*) return 0 ;; + esac + rm -rf "$parent" 2>/dev/null + return 0 +} + +# ------------------------------------------------------- origin canonical form +# Echo the canonical origin of a URL, or return 1 to refuse it. Refusing is +# always safe: it costs a discovered credential and never sends one anywhere. +brains_origin() { + local url scheme rest authority host port oldlc bad + url="${1:-}" + case "$url" in + http://*|https://*) ;; + HTTP://*|HTTPS://*|Http://*|Https://*) ;; + *) return 1 ;; + esac + scheme="${url%%://*}" + rest="${url#*://}" + authority="${rest%%/*}" + [ -n "$authority" ] || return 1 + # Credentials embedded in a hook endpoint are not a shape we support, and + # normalising them invites confusion between userinfo and host. + case "$authority" in *@*) return 1 ;; esac + # Punycode cannot be done correctly here, and a wrong guess would compare two + # different hosts as equal. Refuse instead. + oldlc="${LC_ALL-}" + LC_ALL=C + case "$authority" in *[!\ -~]*) bad=1 ;; *) bad=0 ;; esac + if [ -n "$oldlc" ]; then LC_ALL="$oldlc"; else unset LC_ALL; fi + [ "$bad" = "0" ] || return 1 + + case "$authority" in + \[*\]) host="$authority"; port="" ;; + \[*\]:*) host="${authority%%\]:*}]"; port="${authority##*\]:}" ;; + *:*:*) return 1 ;; # unbracketed IPv6 is ambiguous + *:*) host="${authority%:*}"; port="${authority##*:}" ;; + *) host="$authority"; port="" ;; + esac + [ -n "$host" ] || return 1 + + # Fork `tr` only when there is something to fold; hosts and schemes are + # lowercase in practice, and this runs on every request. + case "$scheme" in *[A-Z]*) scheme=$(printf '%s' "$scheme" | tr 'A-Z' 'a-z') ;; esac + case "$host" in *[A-Z]*) host=$(printf '%s' "$host" | tr 'A-Z' 'a-z') ;; esac + case "$host" in + \[*\]) ;; + *.) host="${host%.}"; [ -n "$host" ] || return 1 ;; + esac + + if [ -n "$port" ]; then + case "$port" in *[!0-9]*) return 1 ;; esac + if { [ "$scheme" = "https" ] && [ "$port" = "443" ]; } || + { [ "$scheme" = "http" ] && [ "$port" = "80" ]; }; then + port="" + fi + else + case "$authority" in *:) return 1 ;; esac + fi + + if [ -n "$port" ]; then + printf '%s://%s:%s' "$scheme" "$host" "$port" + else + printf '%s://%s' "$scheme" "$host" + fi +} + +# Does this MCP server name identify brains? Origin alone is not enough: another +# MCP server sharing the origin would otherwise have its token selected and sent +# to the brains endpoints as the unique match. Claude names a plugin-provided +# server `plugin::` and a hand-added one just ``, so both +# shapes are accepted and everything else is refused. +_brains_is_brains_server() { + local name want + name="${1:-}" + want="${BRAINS_CRED_SERVER:-brains}" + [ -n "$name" ] || return 1 + case "$name" in + "$want"|*:"$want") return 0 ;; + *) return 1 ;; + esac +} + +# ---------------------------------------------------------- Claude Code store +# The client derives ONE service name and ONE account. We derive the same ones +# and stop. Probing a second name would let a profile that has no credential of +# its own read the default profile's, which silently captures a conversation +# into the wrong account and destroys the isolation the tests depend on. +_brains_claude_service() { + local dir hash + if [ -n "${CLAUDE_SECURESTORAGE_CONFIG_DIR+set}" ]; then + dir="$CLAUDE_SECURESTORAGE_CONFIG_DIR" + elif [ -n "${CLAUDE_CONFIG_DIR:-}" ]; then + dir="$CLAUDE_CONFIG_DIR" + else + dir="" + fi + if [ -z "$dir" ]; then + printf '%s' 'Claude Code-credentials' + return 0 + fi + hash=$(printf '%s' "$dir" | shasum -a 256 2>/dev/null | cut -c1-8) + [ -n "$hash" ] || return 1 + printf 'Claude Code-credentials-%s' "$hash" +} + +_brains_claude_account() { + local u + u="${USER:-}" + [ -n "$u" ] || u=$(id -un 2>/dev/null) + case "$u" in + ''|*[!a-zA-Z0-9._-]*) printf '%s' 'claude-code-user' ;; + *) printf '%s' "$u" ;; + esac +} + +_brains_claude_config_dir() { + if [ -n "${CLAUDE_SECURESTORAGE_CONFIG_DIR+set}" ]; then + if [ -n "$CLAUDE_SECURESTORAGE_CONFIG_DIR" ]; then + printf '%s' "$CLAUDE_SECURESTORAGE_CONFIG_DIR" + else + printf '%s' "$HOME/.claude" + fi + return 0 + fi + printf '%s' "${CLAUDE_CONFIG_DIR:-$HOME/.claude}" +} + +# Echo the PATH of a file holding the credential document, or return 1. Mirrors +# the client's own keychain-then-file order WITHIN one profile, which is not +# probing: it is the same store with two backends, and it is how Linux (no +# keychain) works at all. +# Returns 0 and echoes a path; 2 when the store is conclusively ABSENT; 1 when +# it exists but could not be read. The distinction has to travel as an exit code +# rather than a variable, because every caller invokes this inside a command +# substitution and a subshell assignment never reaches the parent. +_brains_claude_store_file() { + local svc acct file out kc_absent + kc_absent=0 + if [ -n "${BRAINS_CLAUDE_CREDENTIALS_FILE:-}" ]; then + [ -f "$BRAINS_CLAUDE_CREDENTIALS_FILE" ] || return 2 + _brains_snapshot "$BRAINS_CLAUDE_CREDENTIALS_FILE" + return $? + fi + if command -v security >/dev/null 2>&1; then + svc=$(_brains_claude_service) || svc="" + acct=$(_brains_claude_account) + if [ -n "$svc" ]; then + out=$(_brains_bounded_read_file "$BRAINS_CRED_READ_DEADLINE" \ + "$BRAINS_CRED_MAX_BYTES" "$BRAINS_CRED_MAX_BLOCKS" \ + security find-generic-password -s "$svc" -a "$acct" -w) + case "$?" in + 0) printf '%s' "$out"; return 0 ;; + # 44 is SecKeychain "item not found" — a conclusive absence, unlike a + # timeout or an ACL prompt, which tell us nothing. + 44) kc_absent=1 ;; + esac + fi + else + kc_absent=1 + fi + file="$(_brains_claude_config_dir)/.credentials.json" + if [ ! -f "$file" ]; then + [ "$kc_absent" = "1" ] && return 2 + return 1 + fi + # M3: snapshot the fallback backend through the bounded reader too. Handing + # back the live path meant the size ceiling and the deadline applied to the + # keychain backend only, while the file backend — the whole of Linux — read an + # arbitrarily large document straight into jq, twice. + _brains_snapshot "$file" +} + +# Copy a file through the bounded reader so the ceiling and deadline apply, and +# so callers always get a private copy they may freely discard. +_brains_snapshot() { + local out + out=$(_brains_bounded_read_file "$BRAINS_CRED_READ_DEADLINE" \ + "$BRAINS_CRED_MAX_BYTES" "$BRAINS_CRED_MAX_BLOCKS" cat "$1") || return 1 + printf '%s' "$out" +} + +# ---------------------------------------------------------------- Codex store +# Codex keys each entry by | and the hash is not reconstructible, +# so entries are enumerated and read by their own fields. dump-keychain prints +# metadata only and does not prompt. Narrowing by server name here — on that +# metadata — is what keeps the per-account candidate cap from ever binding. +# Returns 1 when the dump itself could not be read, which the caller must treat +# as an incomplete enumeration rather than an empty one. +_brains_codex_accounts() { + local dumpfile server + server="${BRAINS_CRED_SERVER:-brains}" + dumpfile=$(_brains_bounded_read_file "$BRAINS_CRED_READ_DEADLINE" \ + "$BRAINS_CRED_DUMP_MAX_BYTES" "$BRAINS_CRED_DUMP_MAX_BLOCKS" \ + security dump-keychain) || return 1 + LC_ALL=C awk -v want="$server" ' + /^[[:space:]]*"acct"=/ { + acct = $0 + sub(/^[^"]*"acct"="/, "", acct) + sub(/".*$/, "", acct) + last = acct + } + /"svce"="Codex MCP Credentials"/ { + if (last != "") { + name = last + sub(/\|.*$/, "", name) + if (name == want) print last + } + } + ' "$dumpfile" 2>/dev/null | sort -u + # A partial awk pass handed back as a COMPLETE enumeration is exactly what I2 + # forbids, so the parse status is checked rather than assumed. + set -- "${PIPESTATUS[0]}" "${PIPESTATUS[1]}" + _brains_discard "$dumpfile" + { [ "$1" = "0" ] && [ "$2" = "0" ]; } || return 1 + return 0 +} + +# ------------------------------------------------- the credential, never a var +# Write the private curl config holding the Authorization header, straight from +# the store through jq. See I1: at no point is the value assigned to a shell +# variable. jq's @json produces exactly curl's quoted-value escaping — verified +# for quote, backslash, space, percent, hash and tab. +# +# Sets BRAINS_CRED_CONFIG on success. Returns 1 without leaving a config behind +# on failure, so a caller can tell "could not present a credential" from +# "presented one and was refused". +_brains_write_config() { # source-kind, locator, store-file(optional) + local kind locator store cfgdir cfg rc + kind="$1"; locator="${2:-}"; store="${3:-}" + [ -n "$BRAINS_CRED_TMP" ] || return 1 + cfgdir="$BRAINS_CRED_TMP/r.$$.cfg" + ( umask 077; mkdir -p "$cfgdir" ) 2>/dev/null || return 1 + cfg="$cfgdir/curl.conf" + ( umask 077 + case "$kind" in + plugin-option) + printf '%s' "${CLAUDE_PLUGIN_OPTION_TOKEN:-}" ;; + env-api) + printf '%s' "${BRAINS_API_TOKEN:-}" ;; + env-inbox) + printf '%s' "${BRAINS_INBOX_TOKEN:-}" ;; + codex-header) + jq -r '(.transport.http_headers.Authorization // .transport.http_headers.authorization // "") + | select(startswith("Bearer ")) | sub("^Bearer ";"")' "$store" 2>/dev/null ;; + claude-oauth) + jq -r --arg k "$locator" '(.mcpOAuth[$k].accessToken // "")' "$store" 2>/dev/null ;; + codex-oauth) + jq -r '(.token_response.access_token // "")' "$store" 2>/dev/null ;; + codex-oauth-array) + jq -r --argjson i "$locator" '(if type == "array" then .[$i] else . end) + | (.token_response.access_token // "")' "$store" 2>/dev/null ;; + esac | jq -Rr 'select(length > 0) | "header = " + (("Authorization: Bearer " + .) | @json)' >"$cfg" 2>/dev/null + ) + rc=$? + if [ "$rc" -ne 0 ] || [ ! -s "$cfg" ]; then + rm -rf "$cfgdir" 2>/dev/null + return 1 + fi + BRAINS_CRED_CONFIG="$cfg" + # The config lives for the rest of the hook, and is removed on exit. Only + # installed when the sourcing hook has no EXIT trap of its own — replacing a + # caller's handler is not this library's call to make. None of the hooks set + # one; if that changes, the caller must invoke brains_cred_release itself. + [ -z "$(trap -p EXIT 2>/dev/null)" ] && + trap 'rm -rf "$BRAINS_CRED_TMP/r.$$.cfg" 2>/dev/null' EXIT INT TERM HUP + return 0 +} + +brains_cred_release() { [ -n "$BRAINS_CRED_CONFIG" ] && rm -rf "$(dirname "$BRAINS_CRED_CONFIG")" 2>/dev/null; BRAINS_CRED_CONFIG=""; return 0; } + +# A private copy of the config for a request that will outlive this shell. +# +# Two calls are deliberately fire-and-forget — the Claude ingest POST and the +# inbox ack — so the hook exits while curl is still running. The hook's EXIT +# trap would then delete the config out from under it, and the request would go +# out with no credential. A copy taken BEFORE backgrounding, and removed by the +# backgrounded subshell itself, gives the in-flight request a lifetime of its +# own. Waiting for the children instead would put up to five seconds back onto +# the hot path, which is the thing backgrounding exists to avoid. +# +# Echoes the path, or nothing when there is no credential to lease. +brains_cred_lease() { + local dir + [ -n "$BRAINS_CRED_CONFIG" ] && [ -s "$BRAINS_CRED_CONFIG" ] || return 1 + [ -n "$BRAINS_CRED_TMP" ] || return 1 + dir="$BRAINS_CRED_TMP/r.$$.lease.$RANDOM" + ( umask 077; mkdir "$dir" ) 2>/dev/null || return 1 + ( umask 077; cat "$BRAINS_CRED_CONFIG" >"$dir/curl.conf" ) 2>/dev/null || { rm -rf "$dir" 2>/dev/null; return 1; } + printf '%s' "$dir/curl.conf" +} + +brains_cred_return() { [ -n "${1:-}" ] && rm -rf "$(dirname "$1")" 2>/dev/null; return 0; } + +# --------------------------------------------------------------- the resolver +# Sets BRAINS_CRED_* and returns 0 when a credential resolved, 1 otherwise. +# $1 is the base URL the caller intends to talk to; a discovered credential is +# admitted only for that origin. +brains_resolve_credential() { + local rc + _brains_xtrace_off + _brains_resolve_credential_impl "$@" + rc=$? + _brains_xtrace_restore + return $rc +} + +_brains_resolve_credential_impl() { + local base want store meta line key url name corigin accts acct seen rec recfile + local matched matchedfiles count + base="${1:-}" + BRAINS_CRED_STATE="indeterminate" # I2: downgrade only on proof + BRAINS_CRED_SOURCE=""; BRAINS_CRED_BINDING=""; BRAINS_CRED_ORIGIN="" + BRAINS_CRED_LOCATOR=""; BRAINS_CRED_COUNT=0; BRAINS_CRED_TRUNCATED=0 + BRAINS_CRED_SAW_ENTRIES=0 + brains_cred_release + _brains_budget_start + + # 1-3. Explicit configuration. Admitted for any target: a user who sets both a + # token and an endpoint override chose that pairing. + if [ -n "${CLAUDE_PLUGIN_OPTION_TOKEN:-}" ]; then + _brains_cred_tmp_root && _brains_write_config plugin-option || { BRAINS_CRED_STATE="indeterminate"; return 1; } + BRAINS_CRED_SOURCE="plugin-option"; BRAINS_CRED_BINDING="explicit"; BRAINS_CRED_STATE="ok"; return 0 + fi + if [ -n "${BRAINS_API_TOKEN:-}" ]; then + _brains_cred_tmp_root && _brains_write_config env-api || { BRAINS_CRED_STATE="indeterminate"; return 1; } + BRAINS_CRED_SOURCE="env"; BRAINS_CRED_BINDING="explicit"; BRAINS_CRED_STATE="ok"; return 0 + fi + # BRAINS_INBOX_TOKEN is the TEST channel, not a documented user option: the + # suites scrub BRAINS_API_TOKEN from the inherited environment so a + # developer's real token cannot outrank a fixture, which needs a second name + # to put the fixture in. Kept for that reason alone. + if [ -n "${BRAINS_INBOX_TOKEN:-}" ]; then + _brains_cred_tmp_root && _brains_write_config env-inbox || { BRAINS_CRED_STATE="indeterminate"; return 1; } + BRAINS_CRED_SOURCE="env"; BRAINS_CRED_BINDING="explicit"; BRAINS_CRED_STATE="ok"; return 0 + fi + + # Discovery switched off is a DEFINITE no-credential: we are not looking. + if [ "${BRAINS_CREDENTIAL_STORE_DISABLED:-}" = "1" ]; then + BRAINS_CRED_STATE="no-credential"; return 1 + fi + command -v jq >/dev/null 2>&1 || return 1 # cannot read anything: stays indeterminate + want=$(brains_origin "$base") || { BRAINS_CRED_STATE="no-credential"; return 1; } + _brains_cred_tmp_root || return 1 # cannot read anything: stays indeterminate + + # 4. A Codex Authorization header the user typed by hand. Explicit in rank, + # but its audience is known from the same response, so it is bound to it. + if [ "${BRAINS_CRED_CLIENT:-claude}" = "codex" ] && command -v codex >/dev/null 2>&1; then + store=$(_brains_bounded_read_file "$BRAINS_CRED_READ_DEADLINE" \ + "$BRAINS_CRED_MAX_BYTES" "$BRAINS_CRED_MAX_BLOCKS" \ + codex mcp get "${BRAINS_CRED_SERVER:-brains}" --json) && { + url=$(jq -r '.transport.url // ""' "$store" 2>/dev/null) + corigin=$(brains_origin "$url") || corigin="" + if [ -n "$corigin" ] && [ "$corigin" = "$want" ] && _brains_write_config codex-header "" "$store"; then + _brains_discard "$store" + BRAINS_CRED_SOURCE="codex-header"; BRAINS_CRED_BINDING="bound" + BRAINS_CRED_ORIGIN="$corigin"; BRAINS_CRED_STATE="ok"; BRAINS_CRED_COUNT=1 + return 0 + fi + _brains_discard "$store" + } + fi + + # 5. The client's own MCP OAuth store, selected by origin AND server identity. + # Selection works on METADATA ONLY — keys, names, urls. No token is read + # until one entry has won, and then only into the curl config (I1). + matched="" + count=0 + if [ "${BRAINS_CRED_CLIENT:-claude}" = "codex" ]; then + if [ -n "${BRAINS_CODEX_CREDENTIALS_FILE:-}" ]; then + [ -f "$BRAINS_CODEX_CREDENTIALS_FILE" ] || { BRAINS_CRED_STATE="no-credential"; return 1; } + # Snapshot into the owned root first, so everything downstream operates on + # a copy we may freely discard — the same discipline the Claude backend + # follows. Referencing the live path here is what let the record-cleanup + # sweep reach the user's own directory. + store=$(_brains_snapshot "$BRAINS_CODEX_CREDENTIALS_FILE") || return 1 + meta=$(jq -r --arg f "$store" \ + '(if type == "array" then . else [.] end) | to_entries[] + | select((.value.token_response.access_token // "") != "") + | [(.key|tostring), (.value.url // ""), (.value.server_name // ""), $f] | @tsv' \ + "$store" 2>/dev/null) || { _brains_discard "$store"; return 1; } + else + command -v security >/dev/null 2>&1 || { BRAINS_CRED_STATE="no-credential"; return 1; } + accts=$(_brains_codex_accounts) || { BRAINS_CRED_TRUNCATED=1; accts=""; } + meta="" + seen=0 + while IFS= read -r acct; do + [ -n "$acct" ] || continue + if [ "$seen" -ge "$BRAINS_CRED_MAX_CANDIDATES" ] || ! _brains_budget_left; then + BRAINS_CRED_TRUNCATED=1 + break + fi + seen=$((seen + 1)) + rec=$(_brains_bounded_read_file "$BRAINS_CRED_READ_DEADLINE" \ + "$BRAINS_CRED_MAX_BYTES" "$BRAINS_CRED_MAX_BLOCKS" \ + security find-generic-password -s "Codex MCP Credentials" -a "$acct" -w) \ + || { BRAINS_CRED_TRUNCATED=1; continue; } + # jq exits 0 having selected nothing for a record that simply holds no + # token — a conclusive answer about that account. It exits non-zero when + # the document will not parse, which is not conclusive. Collapsing both + # into "no candidate" understates; collapsing both into "unknown" would + # let one junk keychain entry disable capture forever. + line=$(jq -r --arg a "$acct" --arg f "$rec" \ + 'select((.token_response.access_token // "") != "") + | [$a, (.url // ""), (.server_name // ""), $f] | @tsv' "$rec" 2>/dev/null) + if [ "$?" -ne 0 ]; then + BRAINS_CRED_TRUNCATED=1 + _brains_discard "$rec" + continue + fi + if [ -n "$line" ]; then + meta="$meta$line +" + else + _brains_discard "$rec" + fi + done </dev/null) + if [ "$?" -ne 0 ]; then + _brains_discard "$store" + return 1 # unparseable: indeterminate + fi + fi + + matchedfiles="" + while IFS=$(printf '\t') read -r key url name recfile; do + [ -n "$key" ] || continue + BRAINS_CRED_SAW_ENTRIES=1 + if ! corigin=$(brains_origin "$url") || + [ "$corigin" != "$want" ] || + ! _brains_is_brains_server "$name"; then + # Codex only: on the Claude path every row names the SAME store snapshot, + # so discarding here on a non-match would delete the document the matching + # rows still need. + [ "${BRAINS_CRED_CLIENT:-claude}" = "codex" ] && _brains_discard "$recfile" + continue + fi + matched="$matched$key +" + matchedfiles="$matchedfiles$recfile +" + count=$((count + 1)) + done <0) | tonumber)')" \ + '(if type == "array" then . else [.] end) as $a + | [ $keys[] as $i | $a[$i].token_response.access_token ] | unique | length' \ + "$store" 2>/dev/null) + elif [ "${BRAINS_CRED_CLIENT:-claude}" = "codex" ]; then + # Keychain backend: one private record file per account. + count=$(printf '%s' "$matchedfiles" | tr '\n' '\0' | xargs -0 -n 200 \ + jq -s '[.[].token_response.access_token] | unique | length' 2>/dev/null | tail -1) + else + count=$(jq -r --argjson keys "$(printf '%s' "$matched" | jq -Rs 'split("\n") | map(select(length>0))')" \ + '[ $keys[] as $k | .mcpOAuth[$k].accessToken ] | unique | length' "$store" 2>/dev/null) + fi + # Anything we cannot count is not a clean "one": fall back to ambiguity. + case "$count" in ''|*[!0-9]*|0) count=2 ;; esac + fi + BRAINS_CRED_COUNT="$count" + + # Metadata pass files are no longer needed once counting is done; the winner's + # is kept just long enough to generate the config below. + # Codex only: there the retained files are one private record per account. On + # the Claude path the same column is the single store snapshot, which is + # discarded on its own — sweeping it here would delete the document out from + # under config generation. + _brains_discard_matched() { + local f keep + [ "${BRAINS_CRED_CLIENT:-claude}" = "codex" ] || return 0 + keep="${1:-}" + while IFS= read -r f; do + [ -n "$f" ] || continue + [ "$f" = "$keep" ] && continue + case "$f" in "$BRAINS_CRED_TMP"/*) _brains_discard "$f" ;; esac + done </dev/null | cut -c1-12) + [ -n "$BRAINS_HEALTH_NS" ] || BRAINS_HEALTH_NS="default" + _brains_health_ns_cache_key="$url" + printf '%s' "$BRAINS_HEALTH_NS" +} + +_brains_health_dir() { # capability, url + printf '%s/health/%s/%s' "$(brains_state_dir)" "$(_brains_health_ns_for "$2")" "$1" +} + +_brains_health_lock() { + local d i + d="$1"; i=0 + while [ "$i" -lt 20 ]; do + if mkdir "$d/.lock" 2>/dev/null; then return 0; fi + if find "$d/.lock" -maxdepth 0 -mmin +1 2>/dev/null | grep -q .; then + rmdir "$d/.lock" 2>/dev/null + fi + i=$((i + 1)) + sleep 0.05 + done + return 1 +} + +_brains_health_unlock() { rmdir "$1/.lock" 2>/dev/null; return 0; } + +# Echo a generation for a request that is about to start, or 0 when the lock +# could not be taken. Skipping an update never blocks a turn. +brains_health_begin() { # capability, url + local d g + d=$(_brains_health_dir "$1" "$2") + mkdir -p "$d" 2>/dev/null || { printf '0'; return 1; } + _brains_health_lock "$d" || { printf '0'; return 1; } + g="" + [ -f "$d/gen" ] && read -r g <"$d/gen" 2>/dev/null + case "$g" in ''|*[!0-9]*) g=0 ;; esac + g=$((g + 1)) + printf '%s' "$g" >"$d/gen" 2>/dev/null + _brains_health_unlock "$d" + printf '%s' "$g" +} + +brains_health_apply() { # capability, url, generation, outcome + local cap url gen outcome d a + cap="$1"; url="$2"; gen="$3"; outcome="$4" + case "$gen" in ''|*[!0-9]*|0) return 0 ;; esac + d=$(_brains_health_dir "$cap" "$url") + mkdir -p "$d" 2>/dev/null || return 0 + _brains_health_lock "$d" || return 0 + a="" + [ -f "$d/applied" ] && read -r a <"$d/applied" 2>/dev/null + case "$a" in ''|*[!0-9]*) a=0 ;; esac + if [ "$gen" -gt "$a" ]; then + printf '%s' "$outcome" >"$d/state.tmp" 2>/dev/null && mv -f "$d/state.tmp" "$d/state" 2>/dev/null + printf '%s' "$gen" >"$d/applied" 2>/dev/null + # A healthy result re-arms the signals this success actually disproves, and + # only those. An observed 2xx proves a credential resolved and was accepted, + # so the resolution-level warnings are released; it says nothing about a + # DIFFERENT capability still being refused. + if [ "$outcome" = "ok" ]; then + _brains_signal_release no-credential "$url" + _brains_signal_release indeterminate "$url" + _brains_signal_release "rejected-$cap" "$url" + _brains_signal_release "blocked-$cap" "$url" + fi + fi + _brains_health_unlock "$d" + return 0 +} + +brains_health_note() { # capability, url, outcome + local cap url outcome gen + cap="$1"; url="$2"; outcome="$3" + gen=$(brains_health_begin "$cap" "$url") + brains_health_apply "$cap" "$url" "$gen" "$outcome" +} + +brains_health_state() { # capability, url + local d s + d=$(_brains_health_dir "$1" "$2") + s="" + [ -f "$d/state" ] && read -r s <"$d/state" 2>/dev/null + printf '%s' "$s" +} + +# Claims are keyed by CAUSE, not by capability. Nothing-resolved and +# cannot-determine are properties of the credential, so they are one claim +# shared by everything; a refusal or a blocked target is a property of one +# endpoint, so those are per capability. +_brains_signal_dir() { printf '%s/health/%s/.signals' "$(brains_state_dir)" "$(_brains_health_ns_for "$1")"; } + +_brains_signal_release() { rmdir "$(_brains_signal_dir "$2")/$1" 2>/dev/null; return 0; } + +brains_health_claim_signal() { # key, url + local d + d=$(_brains_signal_dir "$2") + mkdir -p "$d" 2>/dev/null || return 1 + mkdir "$d/$1" 2>/dev/null || return 1 + return 0 +} + +# --------------------------------------------------------- the off-state signal +# core.md tells the agent this note is authoritative. It must therefore never +# promise something it does not deliver: two of the four states name a SETTING +# to change rather than a command to run, and an agent told "run the command it +# names" when none is named can invent one and execute it. +# +# Emitted at most once per cause per episode — the claim is released only by an +# OBSERVED success, so declining once does not mute a later break, and a +# credential being rejected on every request cannot mute itself. `unreachable` +# never signals: transient network trouble is not user-actionable, and a warning +# that cries wolf on flaky wifi is how people learn to ignore the one that +# matters. +# Codex reads its MCP sign-in from the macOS keychain, and Codex on Linux is not +# a supported configuration. So on that platform there is no step to name: the +# hooks cannot reach the sign-in, and pointing the user at a token would be +# documenting a path the product does not support. Echoes an empty string, and +# the caller drops the remedy clause rather than inventing one. +_brains_codex_unsupported_here() { + [ "${BRAINS_CRED_CLIENT:-claude}" = "codex" ] && ! command -v security >/dev/null 2>&1 +} + +_brains_signin_step() { + if [ "${BRAINS_CRED_CLIENT:-claude}" = "codex" ]; then + _brains_codex_unsupported_here && return 0 + printf '%s' 'run `codex mcp login brains` — that sign-in is the credential' + else + printf '%s' 'run `claude mcp login plugin:brains:brains` — that sign-in is the credential' + fi +} + +_brains_emit_signal() { # key, url, label, remedy + brains_health_claim_signal "$1" "$2" || return 1 + printf '%s\n' ''"$3"' is OFF: '"$4"' Offer this to the user once, in one line, and act only if they say yes. Do not repeat it later in the session.' + return 0 +} + +brains_capture_signal() { + local cap url state step label + step=$(_brains_signin_step) + + # Resolution-level causes first: they explain every capability at once, so + # they get one message rather than one per endpoint. + for cap in ingest inbox; do + [ "$cap" = "ingest" ] && url="$BRAINS_URL_INGEST" || url="$BRAINS_URL_INBOX" + state=$(brains_health_state "$cap" "$url") + case "$state" in + no-credential) + if _brains_codex_unsupported_here; then + _brains_emit_signal no-credential "$url" 'Conversation capture and the brains inbox' \ + "brains does not support Codex on this platform, so there is nothing to turn on and nothing to change. Mention it once if it is relevant and do not offer a fix." && return 0 + return 1 + fi + _brains_emit_signal no-credential "$url" 'Conversation capture and the brains inbox' \ + "no capture credential resolved. To turn it on, $step." && return 0 + return 1 ;; + indeterminate) + _brains_emit_signal indeterminate "$url" 'Conversation capture and the brains inbox' \ + "brains could not determine which stored credential belongs to this endpoint — more than one may match it, or the store could not be read in full. Set the plugin's \`token\` option explicitly, or if you have signed into brains twice, remove the duplicate MCP server entry." && return 0 + return 1 ;; + esac + done + + # Endpoint-level causes: one capability can be broken while another works. + for cap in ingest inbox; do + [ "$cap" = "ingest" ] && url="$BRAINS_URL_INGEST" || url="$BRAINS_URL_INBOX" + [ "$cap" = "ingest" ] && label="Conversation capture" || label="The brains inbox" + state=$(brains_health_state "$cap" "$url") + case "$state" in + rejected) + if [ -z "$step" ]; then + _brains_emit_signal "rejected-$cap" "$url" "$label" \ + "the capture credential was refused by the server." && return 0 + return 1 + fi + _brains_emit_signal "rejected-$cap" "$url" "$label" \ + "the capture credential was refused by the server. To re-issue it, $step." && return 0 ;; + blocked) + _brains_emit_signal "blocked-$cap" "$url" "$label" \ + "this endpoint is a different host from the brains server you are signed into, so the stored credential was not used. Set the plugin's \`token\` option to a token for this endpoint." && return 0 ;; + esac + done + return 1 +} + +# ------------------------------------------------------------ the ONE request +# Every authenticated call goes through here. Callers pass a capability and a +# URL and read the result out of BRAINS_HTTP_BODY / BRAINS_HTTP_CODE / +# BRAINS_HTTP_OK. The body is deliberately NOT written to stdout: a caller would +# have to wrap the call in a command substitution to capture it, that runs in a +# subshell, and every status variable set here would be discarded with it. +# +# Two things this owns that callers must never re-implement: +# +# * Binding. A discovered credential is sent ONLY to the origin that minted +# it. Endpoint URLs are env-overridable, so without this check a token +# found in the local store could be posted to an unrelated host. +# * Transport truth. curl writes %{http_code} as soon as headers arrive, so a +# transfer that dies mid-body still reports 200 — and the truncated body can +# be perfectly valid JSON. Only curl's EXIT STATUS distinguishes them, so a +# non-zero exit is a transport failure whatever the code says, the body is +# not parsed, and health is never advanced to ok. +BRAINS_HTTP_CODE="" +BRAINS_HTTP_BODY="" +BRAINS_HTTP_OK=0 +BRAINS_HTTP_BLOCKED=0 + +brains_request() { + local rc + _brains_xtrace_off + _brains_request_impl "$@" + rc=$? + _brains_xtrace_restore + return $rc +} + +_brains_request_impl() { + local cap url gen resp crc code body outcome origin + cap="$1"; url="$2"; shift 2 + BRAINS_HTTP_CODE=""; BRAINS_HTTP_BODY=""; BRAINS_HTTP_OK=0; BRAINS_HTTP_BLOCKED=0 + + [ "$BRAINS_CRED_STATE" = "ok" ] || return 1 + [ -n "$BRAINS_CRED_CONFIG" ] && [ -s "$BRAINS_CRED_CONFIG" ] || return 1 + + if [ "$BRAINS_CRED_BINDING" = "bound" ]; then + origin=$(brains_origin "$url") || origin="" + if [ -z "$origin" ] || [ "$origin" != "$BRAINS_CRED_ORIGIN" ]; then + BRAINS_HTTP_BLOCKED=1 + gen=$(brains_health_begin "$cap" "$url") + brains_health_apply "$cap" "$url" "$gen" "blocked" + return 1 + fi + fi + + gen=$(brains_health_begin "$cap" "$url") + # -q FIRST, and it must be first for curl to honour it. Without it curl reads + # the user's ~/.curlrc, and a curlrc carrying `trace-ascii` or `trace` writes + # outgoing headers — this Authorization line among them — to a file. Measured + # on curl 8.7.1: the bearer lands in that trace without -q and does not with + # it. Keeping the header out of argv and out of variables does nothing about a + # disclosure channel the user's own debugging config opens. + # + # Defaults after it, so a caller passing its own --max-time still wins. + resp=$(curl -q -sS \ + --config "$BRAINS_CRED_CONFIG" \ + --connect-timeout "$BRAINS_CRED_CONNECT_TIMEOUT" \ + --max-time "$BRAINS_CRED_MAX_TIME" \ + -w '\n%{http_code}' \ + "$@" "$url" 2>/dev/null) + crc=$? + code="${resp##*$'\n'}" + body="${resp%$'\n'*}" + + if [ "$crc" -ne 0 ]; then + # Transport failure. The body may be a valid JSON prefix of a response that + # never finished; it is not a response and is not returned. + BRAINS_HTTP_CODE="$code" + brains_health_apply "$cap" "$url" "$gen" "unreachable" + return 1 + fi + + BRAINS_HTTP_CODE="$code" + case "$code" in + 2*) outcome="ok"; BRAINS_HTTP_OK=1; BRAINS_HTTP_BODY="$body" ;; + 401|403) outcome="rejected" ;; + 000) outcome="unreachable" ;; + *) outcome="error" ;; + esac + brains_health_apply "$cap" "$url" "$gen" "$outcome" + [ "$BRAINS_HTTP_OK" = "1" ] || return 1 + return 0 +} diff --git a/plugins/brains/hooks/lib/brains-inbox.sh b/plugins/brains/hooks/lib/brains-inbox.sh index d33a37d..d0d84c9 100755 --- a/plugins/brains/hooks/lib/brains-inbox.sh +++ b/plugins/brains/hooks/lib/brains-inbox.sh @@ -6,7 +6,9 @@ # mode: startup | prompt | stop # # This is the v0.2 inbox dispatcher, adapted for the plugin runtime: -# - token/endpoint come from Claude userConfig or Codex BRAINS_API_TOKEN +# - the credential comes from lib/brains-credential.sh: an explicitly +# configured token if there is one, otherwise the client's own MCP OAuth +# store. The endpoint comes from Claude userConfig or BRAINS_ENDPOINT. # - state (log / device-id / state.json) lives in plugin persistent data # - the device "sections" report scans the plugin's core.md (not a CLAUDE.md) # The HTTP protocol (devices / inbox / ack) is IDENTICAL to the standalone hook. @@ -32,14 +34,8 @@ SESSION="${2:-}" [ -z "$SESSION" ] && exit 0 # --- config: plugin userConfig first, env overrides for tests -------------- -TOKEN="${CLAUDE_PLUGIN_OPTION_TOKEN:-${BRAINS_API_TOKEN:-${BRAINS_INBOX_TOKEN:-}}}" -[ -z "$TOKEN" ] && exit 0 - BASE="${CLAUDE_PLUGIN_OPTION_ENDPOINT:-${BRAINS_ENDPOINT:-https://mcp.mybrains.ai}}" BASE="${BASE%/}" # strip any trailing slash -ENDPOINT="${BRAINS_INBOX_URL:-$BASE/inbox/claude}" -ACK_ENDPOINT="${BRAINS_INBOX_ACK_URL:-${ENDPOINT}/ack}" -DEVICES_ENDPOINT="${BRAINS_INBOX_DEVICES_URL:-${ENDPOINT}/devices}" # --- paths: plugin code dir (ephemeral) vs data dir (persistent) ----------- CODEX_PLUGIN_RUNTIME=0 @@ -65,6 +61,38 @@ esac log() { printf '[%s] [%s] %s\n' "$(date -u +%FT%TZ)" "$MODE" "$*" >> "$LOG" 2>/dev/null; } +# --- credential: the same resolver the turn hook uses ---------------------- +# Both scripts used to build their own credential chain, and they disagreed: +# the turn hook scavenged a Codex MCP header and this one did not, so a Codex +# user with header auth had capture ON and the inbox OFF. One resolver, one +# request function, both behaviours identical by construction. +CRED_LIB="$LIB_DIR/brains-credential.sh" +[ -r "$CRED_LIB" ] || exit 0 +# shellcheck source=brains-credential.sh +. "$CRED_LIB" || exit 0 + +BRAINS_CRED_CLIENT="claude" +[ "$CODEX_PLUGIN_RUNTIME" = "1" ] && BRAINS_CRED_CLIENT="codex" +# Same endpoint set the turn hook uses, from the same place — so the +# session-start signal can find the capture health the turn hook wrote. +brains_resolve_endpoints "$BASE" +ENDPOINT="$BRAINS_URL_INBOX" +ACK_ENDPOINT="$BRAINS_URL_ACK" +DEVICES_ENDPOINT="$BRAINS_URL_DEVICES" + +if ! brains_resolve_credential "$BASE"; then + brains_health_note inbox "$ENDPOINT" "$BRAINS_CRED_STATE" + # Told here rather than after the gate: the whole point is to say something + # when NO credential resolved, which is exactly when everything below is + # skipped. Raised on prompt as well as startup, because a token revoked + # mid-thread would otherwise lose every remaining turn in silence until the + # next session begins. The once-per-cause claim keeps it to a single mention + # either way. + case "$MODE" in startup|prompt) brains_capture_signal ;; esac + exit 0 +fi +case "$MODE" in startup|prompt) brains_capture_signal ;; esac + # Detect IANA timezone — server uses it to populate users.timezone. detect_tz() { local tz="" @@ -130,11 +158,13 @@ if [ "$MODE" = "startup" ]; then '{hostname:$h, sections:$s, client:$client, client_type:"cli"} + (if $pv != "" then {plugin_version:$pv} else {} end) + (if $au != "" then {auto_update: ($au == "true")} else {} end)') - REPORT_RESP=$(curl -sS --max-time 4 -X POST \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d "$REPORT_BODY" \ - "$DEVICES_ENDPOINT" 2>>"$LOG") || REPORT_RESP="" + REPORT_RESP="" + if brains_request devices "$DEVICES_ENDPOINT" --max-time 4 -X POST \ + -H "Content-Type: application/json" -d "$REPORT_BODY"; then + REPORT_RESP="$BRAINS_HTTP_BODY" + else + log "device report failed: status=${BRAINS_HTTP_CODE:-none} source=$BRAINS_CRED_SOURCE" + fi NEW_DEVICE_ID=$(printf '%s' "$REPORT_RESP" | jq -r '.device_id // empty' 2>/dev/null) if [ -n "$NEW_DEVICE_ID" ]; then DEVICE_ID="$NEW_DEVICE_ID" @@ -173,10 +203,20 @@ fi DEVICE_QS="" [ -n "$DEVICE_ID" ] && DEVICE_QS="&device_id=$DEVICE_ID" -RESP=$(curl -sS --max-time "$TIMEOUT" \ - -H "Authorization: Bearer $TOKEN" \ - "$ENDPOINT?session_id=$SESSION&source=$MODE&mode=$MODE$TZ_QS$DEVICE_QS" 2>>"$LOG") \ - || { log "fetch failed"; exit 0; } +if ! brains_request inbox "$ENDPOINT?session_id=$SESSION&source=$MODE&mode=$MODE$TZ_QS$DEVICE_QS" \ + --max-time "$TIMEOUT"; then + # A refused credential used to be indistinguishable from an empty inbox: the + # old code discarded the status, and an empty body was read as "nothing to + # do". Name the difference, so a revocation is visible the day it happens. + case "${BRAINS_HTTP_CODE:-}" in + 401|403) log "inbox rejected: status=$BRAINS_HTTP_CODE source=$BRAINS_CRED_SOURCE" ;; + ''|000) log "fetch failed" ;; + *) log "inbox error: status=$BRAINS_HTTP_CODE" ;; + esac + [ "$BRAINS_HTTP_BLOCKED" = "1" ] && log "inbox blocked: endpoint origin does not match the signed-in server" + exit 0 +fi +RESP="$BRAINS_HTTP_BODY" [ -z "$RESP" ] && exit 0 printf '%s' "$RESP" | jq -e . >/dev/null 2>&1 || { log "invalid json"; exit 0; } @@ -278,10 +318,16 @@ if [ "${#AUTO_ACK_IDS[@]}" -gt 0 ] || [ "$CTX_ITEMS_LEN" -gt 0 ]; then else {session_id:$s, mode:$m, applied:$ids, context_received:$ctx} end') - ( curl -sS --max-time 3 -X POST "$ACK_ENDPOINT" \ - -H "Authorization: Bearer $TOKEN" \ - -H "Content-Type: application/json" \ - -d "$ack" >/dev/null 2>&1 || true ) & + # Leased before backgrounding: see brains_cred_lease. Without it the hook's + # exit removes the config while this request is still in flight. + ack_lease=$(brains_cred_lease) || ack_lease="" + if [ -n "$ack_lease" ]; then + ( BRAINS_CRED_CONFIG="$ack_lease" + trap 'brains_cred_return "$ack_lease"' EXIT INT TERM HUP + brains_request ack "$ACK_ENDPOINT" --max-time 3 -X POST \ + -H "Content-Type: application/json" -d "$ack" >/dev/null 2>&1 + brains_cred_return "$ack_lease" ) & + fi fi exit 0 diff --git a/tests/credential/run.ts b/tests/credential/run.ts new file mode 100644 index 0000000..6cb3bff --- /dev/null +++ b/tests/credential/run.ts @@ -0,0 +1,1280 @@ +#!/usr/bin/env bun +// Contract test for the shared capture-credential resolver +// (plugins/brains/hooks/lib/brains-credential.sh). +// +// Capture used to be off for anyone who never pasted a token, with no request, +// no log line and no error to show for it. The resolver removes that state by +// falling back to the credential the client's own MCP sign-in already stored — +// which means this file is now the thing standing between a stored production +// token and the wrong host. Most of what follows is about that, not about the +// happy path. +// +// Everything runs against local stub servers and fixture stores. Nothing here +// may reach a real endpoint or read a real keychain: the assertions below are +// only meaningful if a developer's own credential can never satisfy them. +// +// Run: bun run tests/credential/run.ts + +import { spawn, spawnSync } from "node:child_process"; +import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, symlinkSync, writeFileSync, chmodSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; + +const ROOT = resolve(import.meta.dir, "..", ".."); +const PLUGIN = join(ROOT, "plugins", "brains"); +const LIB = join(PLUGIN, "hooks", "lib", "brains-credential.sh"); +const TURN = join(PLUGIN, "hooks", "brains-turn.sh"); +const INBOX = join(PLUGIN, "hooks", "lib", "brains-inbox.sh"); + +let passed = 0; +let failed = 0; +// Written to stderr so progress survives a hang: stdout is buffered when it is a +// pipe, and a suite that stalls would otherwise report nothing at all about where. +const say = (line: string) => process.stderr.write(`${line}\n`); +function check(name: string, condition: unknown, detail = ""): void { + if (condition) { + passed++; + say(` PASS ${name}`); + } else { + failed++; + say(` FAIL ${name}${detail ? ` — ${detail}` : ""}`); + } +} +function section(title: string): void { + say(`\n${title}`); +} + +const temp = mkdtempSync(join(tmpdir(), "brains-credential-")); +const stateSeq = { n: 0 }; +function freshState(): string { + const dir = join(temp, `state-${++stateSeq.n}`); + mkdirSync(dir, { recursive: true }); + return dir; +} +const readDirNames = (dir: string): string[] => { + try { return readdirSync(dir); } catch { return []; } +}; +function fixture(name: string, body: unknown): string { + const path = join(temp, `${name}.json`); + writeFileSync(path, typeof body === "string" ? body : JSON.stringify(body)); + return path; +} + +// Run a snippet with the library sourced. Every invocation starts from a scrubbed +// environment: the three explicit token variables are cleared and the store reads +// are pointed at fixtures, so nothing a developer has configured can leak in. +type ShellOpts = { state?: string; env?: Record; }; +function sh(script: string, opts: ShellOpts = {}): { stdout: string; stderr: string; status: number } { + const env: Record = { + ...(process.env as Record), + CLAUDE_PLUGIN_OPTION_TOKEN: "", + BRAINS_API_TOKEN: "", + BRAINS_INBOX_TOKEN: "", + BRAINS_STATE_DIR: opts.state ?? freshState(), + ...(opts.env ?? {}), + }; + // A hard per-call ceiling. Nothing here should take seconds, and a test suite that + // hangs teaches nothing — it has to fail loudly at the call that did it. + const r = spawnSync("bash", ["-c", `set -u\n. ${JSON.stringify(LIB)}\n${script}`], { env, encoding: "utf8", timeout: 20000 }); + if (r.error || r.signal) say(` (call took too long or died: ${r.signal ?? r.error})`); + return { stdout: r.stdout ?? "", stderr: r.stderr ?? "", status: r.status ?? -1 }; +} + +// ---------------------------------------------------------------- stub servers +// Served from a child process — see the header of stubs.js for why that is +// required rather than tidy. Receipts come back through a log file, which is +// also what lets an assertion be made about what a server DID NOT receive. +const PRIMARY = 8991; +const OTHER = 8992; +const TRUNCATOR = 8993; +const HITS = join(temp, "hits.jsonl"); +writeFileSync(HITS, ""); +const stubProc = spawn( + process.execPath, + [join(import.meta.dir, "stubs.js"), HITS, String(PRIMARY), String(OTHER), String(TRUNCATOR)], + { stdio: "ignore" }, +); +{ + // Wait for the stubs to actually listen. Racing the first request against + // startup yields a connection refusal indistinguishable from a real one. + const deadline = Date.now() + 10000; + let up = false; + while (Date.now() < deadline) { + if (spawnSync("curl", ["-s", "-o", "/dev/null", "-m", "1", `http://127.0.0.1:${PRIMARY}/ping`]).status === 0) { + up = true; + break; + } + spawnSync("sleep", ["0.1"]); + } + if (!up) { + say("stub servers never came up"); + process.exit(1); + } + writeFileSync(HITS, ""); +} +type Hit = { port: number; method: string; path: string; auth: string; bodyLen: number }; +const readHits = (): Hit[] => + readFileSync(HITS, "utf8").split("\n").filter((l) => l.trim() !== "").map((l) => JSON.parse(l)); + +const ORIGIN = `http://127.0.0.1:${PRIMARY}`; +const NO_STORE = join(temp, "absent-store.json"); + +function storeFor(url: string, token = "tok-primary"): string { + return fixture(`store-${Buffer.from(url + token).toString("hex").slice(0, 12)}`, { + mcpOAuth: { "plugin:brains:brains|h": { accessToken: token, serverUrl: url, serverName: "plugin:brains:brains" } }, + }); +} +const PRIMARY_STORE = storeFor(`${ORIGIN}/mcp`); + +// =============================================================== canonicalization +section("origin canonicalization — the comparison every binding decision rests on"); +const ORIGIN_CASES: Array<[string, string | null]> = [ + [`https://mcp.mybrains.ai/mcp`, "https://mcp.mybrains.ai"], + [`https://mcp.mybrains.ai:443/mcp`, "https://mcp.mybrains.ai"], + [`http://example.com:80/x`, "http://example.com"], + [`http://example.com:8080/x`, "http://example.com:8080"], + [`HTTPS://MCP.MyBrains.AI/mcp`, "https://mcp.mybrains.ai"], + [`https://mcp.mybrains.ai./mcp`, "https://mcp.mybrains.ai"], + [`https://[::1]:8931/mcp`, "https://[::1]:8931"], + // Refusals. Each of these could otherwise compare equal to something it is not. + [`https://user:pw@mcp.mybrains.ai/mcp`, null], // userinfo is not a host + [`https://mcp.mybrains.ai:/mcp`, null], // bare colon is malformed + [`https://mcp.mybrains.ai:abc/mcp`, null], // non-numeric port + [`https://::1:8931/mcp`, null], // unbracketed IPv6 is ambiguous + [`https://mü.example.com/mcp`, null], // IDNA cannot be done correctly in bash 3.2 + [`ftp://example.com/x`, null], + [`not-a-url`, null], + [``, null], +]; +for (const [input, expected] of ORIGIN_CASES) { + const r = sh(`if o=$(brains_origin ${JSON.stringify(input)}); then printf 'OK:%s' "$o"; else printf 'REFUSE'; fi`); + const want = expected === null ? "REFUSE" : `OK:${expected}`; + check(`${input || "(empty)"} -> ${expected ?? "refused"}`, r.stdout === want, `got ${r.stdout}`); +} + +// =============================================================== selection +section("selection — one candidate or none, never a guess"); +function resolveWith(storeFile: string, base = ORIGIN, extra: Record = {}) { + return sh( + `if brains_resolve_credential ${JSON.stringify(base)}; then printf '%s|%s|%s|%s' "$BRAINS_CRED_STATE" "$BRAINS_CRED_SOURCE" "$BRAINS_CRED_LOCATOR" "$BRAINS_CRED_COUNT";` + + ` else printf '%s||%s|%s' "$BRAINS_CRED_STATE" "" "$BRAINS_CRED_COUNT"; fi`, + { env: { BRAINS_CLAUDE_CREDENTIALS_FILE: storeFile, ...extra } }, + ); +} +check("single matching entry resolves and is labelled claude-oauth", + resolveWith(PRIMARY_STORE).stdout === "ok|claude-oauth|plugin:brains:brains|h|1"); + +// Two entries, same origin, DIFFERENT tokens. Neither store records which account a token belongs +// to, so this is not a preference for caution — the information needed to choose does not exist, +// and capturing into the wrong brain is worse than a 401 because a 401 is detectable. +const twoAccounts = fixture("two-accounts", { + mcpOAuth: { + "a|1": { accessToken: "tok-A", serverUrl: `${ORIGIN}/mcp`, serverName: "brains" }, + "b|2": { accessToken: "tok-B", serverUrl: `${ORIGIN}/mcp`, serverName: "plugin:brains:brains" }, + }, +}); +check("two accounts on one origin resolve to nothing, state indeterminate", + resolveWith(twoAccounts).stdout === "indeterminate|||2", resolveWith(twoAccounts).stdout); + +// Two aliases carrying the SAME token are one credential, not an ambiguity. The second entry also +// spells the port explicitly, so this covers canonicalization feeding selection. +const twoAliases = fixture("two-aliases", { + mcpOAuth: { + "a|1": { accessToken: "tok-same", serverUrl: `${ORIGIN}/mcp`, serverName: "brains" }, + "b|2": { accessToken: "tok-same", serverUrl: `http://127.0.0.1:${PRIMARY}/mcp`, serverName: "plugin:brains:brains" }, + }, +}); +check("two aliases sharing one token collapse and resolve", + resolveWith(twoAliases).stdout.startsWith("ok|claude-oauth|"), resolveWith(twoAliases).stdout); + +// A stage credential must never be sent to production. Name matching would pick this one. +const stageOnly = storeFor("https://brains-mcp.stage.example.com/mcp", "tok-stage"); +check("a store with credentials but none for this origin is blocked, not absent", + resolveWith(stageOnly).stdout === "blocked|||0", resolveWith(stageOnly).stdout); + +const truncated = fixture("truncated", '{"mcpOAuth":{"a":{"accessToken":"tok-x","serverUrl":"' + ORIGIN + '/mcp"'); +check("an unparseable store document is indeterminate, not a clean absence", + resolveWith(truncated).stdout === "indeterminate|||0", resolveWith(truncated).stdout); +check("a genuinely absent store IS a definite no-credential", resolveWith(NO_STORE).stdout === "no-credential|||0", resolveWith(NO_STORE).stdout); + +section("precedence — explicit configuration always outranks discovery"); +check("plugin option outranks a store credential", + resolveWith(PRIMARY_STORE, ORIGIN, { CLAUDE_PLUGIN_OPTION_TOKEN: "tok-explicit" }).stdout === "ok|plugin-option||0"); +check("BRAINS_API_TOKEN outranks a store credential", + resolveWith(PRIMARY_STORE, ORIGIN, { BRAINS_API_TOKEN: "tok-env" }).stdout === "ok|env||0"); +check("the hermetic switch disables discovery entirely", + resolveWith(PRIMARY_STORE, ORIGIN, { BRAINS_CREDENTIAL_STORE_DISABLED: "1" }).stdout === "no-credential|||0"); + +// =============================================================== profile isolation +section("profile isolation — a profile with no credential must not read another's"); +// This is the failure the resolver's own design review caught: a probe that falls back to the bare +// service name lets an isolated CLAUDE_CONFIG_DIR read the DEFAULT profile's real production token. +// There is no keychain in this test, so the file backend stands in for the same rule: the store +// path is derived from the configured profile and nothing else is tried. +{ + const profile = join(temp, "isolated-profile"); + mkdirSync(profile, { recursive: true }); + // The default profile's store exists and is valid; the isolated profile's does not. + const r = sh( + `if brains_resolve_credential ${JSON.stringify(ORIGIN)}; then printf 'RESOLVED:%s' "$BRAINS_CRED_LOCATOR"; else printf 'none'; fi`, + { env: { CLAUDE_CONFIG_DIR: profile, HOME: temp } }, + ); + check("an isolated profile with no store of its own resolves nothing", r.stdout === "none", r.stdout); +} + +// =============================================================== binding +section("binding — a discovered credential goes only to the origin that issued it"); +{ + const state = freshState(); + const before = readHits().length; + const r = sh( + `brains_resolve_credential ${JSON.stringify(ORIGIN)} || exit 1\n` + + `brains_request ingest "http://127.0.0.1:${OTHER}/ingest/claude" -X POST -d '{}'\n` + + `printf 'rc=%s blocked=%s state=%s' "$?" "$BRAINS_HTTP_BLOCKED" "$(brains_health_state ingest \"http://127.0.0.1:${OTHER}/ingest/claude\")"`, + { state, env: { BRAINS_CLAUDE_CREDENTIALS_FILE: PRIMARY_STORE } }, + ); + check("a cross-origin target is refused", r.stdout === "rc=1 blocked=1 state=blocked", r.stdout); + // Asserted on the RECEIVING server, not on hook internals: the only claim that matters is that + // no credential reached a host that never issued one. + check("the other host received no request at all", readHits().length === before, + `${readHits().length - before} request(s) reached port ${OTHER}`); +} +{ + const state = freshState(); + const r = sh( + `brains_resolve_credential ${JSON.stringify(ORIGIN)} || exit 1\n` + + `brains_request ingest "http://127.0.0.1:${OTHER}/ingest/claude" -X POST -d '{}'\n` + + `printf 'rc=%s blocked=%s' "$?" "$BRAINS_HTTP_BLOCKED"`, + { state, env: { BRAINS_API_TOKEN: "tok-explicit" } }, + ); + const reached = readHits().filter((h) => h.port === OTHER && h.auth === "Bearer tok-explicit"); + check("an EXPLICIT credential may cross origins — the user chose that pairing", + r.stdout === "rc=0 blocked=0" && reached.length === 1, `${r.stdout}, reached=${reached.length}`); +} + +// =============================================================== transport truth +section("transport truth — curl's exit status, not its status code"); +{ + const state = freshState(); + const r = sh( + `brains_resolve_credential ${JSON.stringify(`http://127.0.0.1:${TRUNCATOR}`)} || exit 1\n` + + `brains_request devices "http://127.0.0.1:${TRUNCATOR}/x"\n` + + `printf 'rc=%s code=%s ok=%s body=[%s] state=%s' "$?" "$BRAINS_HTTP_CODE" "$BRAINS_HTTP_OK" "$BRAINS_HTTP_BODY" "$(brains_health_state devices \"http://127.0.0.1:${TRUNCATOR}/x\")"`, + { state, env: { BRAINS_API_TOKEN: "tok-explicit" } }, + ); + // The body this server sends is complete, parseable JSON with a device_id in it. A wrapper that + // trusted %{http_code} would cache that id and mark capture healthy off a transfer that failed. + check("a 200 whose body transfer dies is NOT healthy", + r.stdout === "rc=1 code=200 ok=0 body=[] state=unreachable", r.stdout); +} +{ + const state = freshState(); + const r = sh( + `brains_resolve_credential ${JSON.stringify(ORIGIN)} || exit 1\n` + + `brains_request inbox "${ORIGIN}/forbidden"\n` + + `printf 'rc=%s code=%s state=%s' "$?" "$BRAINS_HTTP_CODE" "$(brains_health_state inbox \"${ORIGIN}/forbidden\")"`, + { state, env: { BRAINS_CLAUDE_CREDENTIALS_FILE: PRIMARY_STORE } }, + ); + check("403 is recorded as rejected, distinct from an empty inbox", + r.stdout === "rc=1 code=403 state=rejected", r.stdout); +} +{ + const state = freshState(); + const r = sh( + `brains_resolve_credential ${JSON.stringify(ORIGIN)} || exit 1\n` + + `brains_request devices "${ORIGIN}/inbox/claude/devices" -X POST -d '{}'\n` + + `printf '%s' "$BRAINS_HTTP_BODY"`, + { state, env: { BRAINS_CLAUDE_CREDENTIALS_FILE: PRIMARY_STORE } }, + ); + // The response body has to survive: device identity, drift nudges, inbox context and prompt + // delivery are all parsed out of one. A universal -o /dev/null would have killed every one of + // them while leaving status-code assertions green. + check("a successful response body is preserved for the caller to parse", + JSON.parse(r.stdout || "{}").device_id === "dev-1", r.stdout); +} + +// =============================================================== health ordering +section("health — per capability, ordered by logical generation"); +{ + const state = freshState(); + // A backgrounded ingest can complete after a newer request. The stale outcome carries the lower + // generation and must not win, in either completion order. + const r = sh( + `U="${ORIGIN}/ingest/claude"\ng1=$(brains_health_begin ingest "$U"); g2=$(brains_health_begin ingest "$U")\n` + + `brains_health_apply ingest "$U" "$g2" ok\nbrains_health_apply ingest "$U" "$g1" rejected\n` + + `printf '%s' "$(brains_health_state ingest "$U")"`, + { state }, + ); + check("a stale completion cannot overwrite a newer one", r.stdout === "ok", r.stdout); +} +{ + const state = freshState(); + const r = sh( + `U="${ORIGIN}/ingest/claude"\ng1=$(brains_health_begin ingest "$U"); g2=$(brains_health_begin ingest "$U")\n` + + `brains_health_apply ingest "$U" "$g1" ok\nbrains_health_apply ingest "$U" "$g2" rejected\n` + + `printf '%s' "$(brains_health_state ingest "$U")"`, + { state }, + ); + check("the reverse order also lands on the newer outcome", r.stdout === "rejected", r.stdout); +} +{ + const state = freshState(); + const r = sh( + `U="${ORIGIN}/inbox/claude"\nfor i in 1 2 3 4 5 6 7 8 9 10; do ( g=$(brains_health_begin inbox "$U"); brains_health_apply inbox "$U" "$g" "w$g" ) & done\nwait\n` + + // Health lives under a per-endpoint namespace, so glob rather than hardcode it. + `printf '%s/%s' "$(cat "$BRAINS_STATE_DIR"/health/*/inbox/gen)" "$(cat "$BRAINS_STATE_DIR"/health/*/inbox/applied)"`, + { state }, + ); + check("ten concurrent writers allocate ten distinct generations with none lost", + r.stdout === "10/10", r.stdout); +} +{ + // The read-scoped token: GET /inbox needs `read`, POST /ingest needs `write`, so this shape + // returns 200 and 403 on every single turn. A shared health record would let the inbox's success + // clear capture's failure, which is precisely the silence this whole change exists to remove. + const state = freshState(); + const r = sh( + `brains_resolve_credential ${JSON.stringify(ORIGIN)} || exit 1\n` + + `brains_request ingest "${ORIGIN}/forbidden" -X POST -d '{}'\n` + + `brains_request inbox "${ORIGIN}/inbox/claude"\n` + + `printf 'ingest=%s inbox=%s' "$(brains_health_state ingest "${ORIGIN}/forbidden")" "$(brains_health_state inbox "${ORIGIN}/inbox/claude")"`, + { state, env: { BRAINS_CLAUDE_CREDENTIALS_FILE: PRIMARY_STORE } }, + ); + check("an inbox success does not clear a capture failure", + r.stdout === "ingest=rejected inbox=ok", r.stdout); +} + +// =============================================================== the signal +section("the off-state signal — once per cause, cleared only by an observed success"); +{ + const state = freshState(); + const first = sh(`brains_resolve_endpoints "${ORIGIN}"\nbrains_health_note ingest "$BRAINS_URL_INGEST" no-credential\nbrains_capture_signal`, { state }); + const second = sh(`brains_resolve_endpoints "${ORIGIN}"\nbrains_capture_signal`, { state }); + check("the signal names the single command that fixes it", + first.stdout.includes("") && first.stdout.includes("claude mcp login plugin:brains:brains"), + first.stdout); + check("it is raised once, not every session", second.stdout.trim() === "", second.stdout); +} +{ + // One missing credential is ONE problem. Keying the claim by capability produced a second + // warning for the inbox on the following session, for the same cause. + const state = freshState(); + sh(`brains_resolve_endpoints "${ORIGIN}"\nbrains_health_note ingest "$BRAINS_URL_INGEST" no-credential\nbrains_health_note inbox "$BRAINS_URL_INBOX" no-credential\nbrains_capture_signal`, { state }); + const next = sh(`brains_resolve_endpoints "${ORIGIN}"\nbrains_capture_signal`, { state }); + check("one missing credential raises one warning, not one per capability", + next.stdout.trim() === "", next.stdout); +} +{ + const state = freshState(); + sh(`brains_resolve_endpoints "${ORIGIN}"\nbrains_health_note ingest "$BRAINS_URL_INGEST" no-credential\nbrains_capture_signal`, { state }); + const afterOk = sh( + `brains_resolve_endpoints "${ORIGIN}"\ng=$(brains_health_begin ingest "$BRAINS_URL_INGEST"); brains_health_apply ingest "$BRAINS_URL_INGEST" "$g" ok\nbrains_capture_signal`, + { state }, + ); + check("an observed success re-arms the signal and silences it", afterOk.stdout.trim() === ""); + const broken = sh(`brains_resolve_endpoints "${ORIGIN}"\nbrains_health_note ingest "$BRAINS_URL_INGEST" no-credential\nbrains_capture_signal`, { state }); + check("a later break is announced again — declining once does not mute it forever", + broken.stdout.includes(""), broken.stdout); +} +{ + const state = freshState(); + const amb = sh(`brains_resolve_endpoints "${ORIGIN}"\nbrains_health_note ingest "$BRAINS_URL_INGEST" indeterminate\nbrains_capture_signal`, { state }); + // "Log in again" cannot remove a duplicate server entry, so the remedy has to name the real fix. + check("the indeterminate state gets a remedy that can actually work", + amb.stdout.includes("token` option explicitly"), amb.stdout); +} +{ + // core.md tells the agent the note is authoritative and to carry out the one + // step it names. Every state must therefore actually name a step — and the + // two that name a SETTING rather than a command are exactly where an agent + // told to "run the command" could fabricate one and execute it. + for (const st of ["no-credential", "indeterminate", "rejected", "blocked"]) { + const state = freshState(); + const r = sh( + `brains_resolve_endpoints "${ORIGIN}"\nbrains_health_note ingest "$BRAINS_URL_INGEST" ${st}\nbrains_capture_signal`, + { state }, + ); + const named = /`[^`]+`/.test(r.stdout); + check(`the ${st} signal names a concrete step`, + r.stdout.includes("") && named, r.stdout.trim().slice(0, 120)); + } +} +{ + // The remedy has to be platform-aware as well as cause-aware. Codex storage + // on Linux is unverified, so the hooks do not read it there — telling a + // signed-in Linux user to sign in again is a step they can repeat forever + // without changing anything. `security` absent stands in for "not macOS". + const noSecurity = join(temp, "no-security-bin"); + mkdirSync(noSecurity, { recursive: true }); + for (const bin of ["curl", "jq", "shasum", "sleep", "find", "wc", "head", "tr", "sort", "awk", "mkdir", "rmdir", "rm", "cat", "id", "date", "cut", "mv", "grep", "sed", "printf", "bash", "sh", "env", "ls", "touch"]) { + const which = spawnSync("bash", ["-c", `command -v ${bin} || true`], { encoding: "utf8" }).stdout.trim(); + if (which) writeFileSync(join(noSecurity, bin), `#!/bin/sh\nexec ${which} "$@"\n`), chmodSync(join(noSecurity, bin), 0o755); + } + const state = freshState(); + const r = sh( + `BRAINS_CRED_CLIENT=codex\nbrains_resolve_endpoints "${ORIGIN}"\nbrains_health_note ingest "$BRAINS_URL_INGEST" no-credential\nbrains_capture_signal`, + { state, env: { PATH: noSecurity } }, + ); + // Codex on Linux is not a supported configuration, so the note says that and + // names nothing. Offering a token workaround there documented a path the + // product does not support, and core.md tells the agent to carry out the one + // step a note names — so a note with no supported step must say so outright + // rather than leave a gap for the agent to fill. + check("a Codex host without the macOS keychain is told the platform is unsupported", + r.stdout.includes("does not support Codex on this platform"), r.stdout.trim().slice(0, 160)); + check("...and is offered no remedy at all", + !r.stdout.includes("mcp login") && !r.stdout.includes("BRAINS_API_TOKEN"), r.stdout.trim().slice(0, 160)); +} + +// =============================================================== fail-safe +section("fail-safe — a hostile or broken store degrades to today's behaviour"); +{ + const hostile = join(temp, "hostile-bin"); + mkdirSync(hostile, { recursive: true }); + const write = (name: string, body: string) => { + writeFileSync(join(hostile, name), body); + chmodSync(join(hostile, name), 0o755); + }; + // The shapes that broke two earlier versions of the bounded read. The last one is the important + // one: a leader that exits 0 while a descendant keeps writing held the pipe open in both + // pipe-based designs, well past the deadline, and returned the bytes written after it. + const shapes: Array<[string, string]> = [ + ["hangs", "#!/bin/sh\nsleep 30\n"], + ["prints then hangs", "#!/bin/sh\nprintf PARTIAL\nsleep 30\n"], + ["exits 0, grandchild appends forever", "#!/bin/sh\nprintf EARLY\n( while :; do printf XXXXXXXXXXXXXXXX; done ) &\nexit 0\n"], + ["never exits, writes forever", "#!/bin/sh\nwhile :; do printf YYYYYYYYYYYYYYYY; done\n"], + ["exits non-zero with output", "#!/bin/sh\nprintf JUNK\nexit 3\n"], + ]; + for (const [label, body] of shapes) { + write("security", body); + const started = Date.now(); + const r = sh( + `if brains_resolve_credential ${JSON.stringify(ORIGIN)}; then printf 'RESOLVED'; else printf 'none'; fi`, + { env: { PATH: `${hostile}:${process.env.PATH ?? ""}` } }, + ); + const elapsed = Date.now() - started; + check(`security that ${label}: bounded and yields no credential`, + r.stdout === "none" && r.status === 0 && elapsed < 8000, `stdout=${r.stdout} status=${r.status} ${elapsed}ms`); + } +} +{ + // The temp root is derived, validated, and never fallen back from. It also has to survive + // `set -u`: an unset expansion here would abort the hook, which is the opposite of degrading. + const linkTarget = join(temp, "link-target"); + mkdirSync(linkTarget, { recursive: true }); + const linkedState = join(temp, "linked-state"); + mkdirSync(linkedState, { recursive: true }); + symlinkSync(linkTarget, join(linkedState, "tmp")); + const r = sh( + `if brains_resolve_credential ${JSON.stringify(ORIGIN)}; then printf 'RESOLVED'; else printf 'none'; fi`, + { state: linkedState, env: { BRAINS_CLAUDE_CREDENTIALS_FILE: PRIMARY_STORE } }, + ); + check("a symlinked temp root is refused rather than used", r.stdout === "none" && r.status === 0, + `stdout=${r.stdout} status=${r.status}`); + + const readOnly = join(temp, "readonly-state"); + mkdirSync(readOnly, { recursive: true }); + chmodSync(readOnly, 0o500); + const ro = sh( + `if brains_resolve_credential ${JSON.stringify(ORIGIN)}; then printf 'RESOLVED'; else printf 'none'; fi`, + { state: readOnly, env: { BRAINS_CLAUDE_CREDENTIALS_FILE: PRIMARY_STORE } }, + ); + check("a read-only state dir degrades instead of failing the hook", + ro.status === 0, `status=${ro.status} stderr=${ro.stderr}`); + chmodSync(readOnly, 0o700); +} +{ + const state = freshState(); + const r = sh( + `mkdir -p "$BRAINS_STATE_DIR/tmp/r.stale" "$BRAINS_STATE_DIR/tmp/r.fresh"\n` + + `touch -t 202601010000 "$BRAINS_STATE_DIR/tmp/r.stale"\n` + + `brains_cred_prune_tmp\n` + + `printf 'stale=%s fresh=%s' "$([ -d "$BRAINS_STATE_DIR/tmp/r.stale" ] && echo yes || echo no)" "$([ -d "$BRAINS_STATE_DIR/tmp/r.fresh" ] && echo yes || echo no)"`, + { state }, + ); + check("read directories orphaned by SIGKILL are pruned, recent ones kept", + r.stdout === "stale=no fresh=yes", r.stdout); +} +{ + // SIGKILL is untrappable and the prune above is the answer to it, but TERM + // and INT are catchable and far more common — a user hitting ctrl-C. The + // store document is in that directory in plaintext while the read runs, so + // the interrupted case has to clean up rather than wait for a later session. + const trapBin = join(temp, "trap-bin"); + mkdirSync(trapBin, { recursive: true }); + writeFileSync(join(trapBin, "security"), `#!/bin/sh\nprintf '{"mcpOAuth":{}}'\nsleep 60\n`); + chmodSync(join(trapBin, "security"), 0o755); + const state = freshState(); + // The read deadline is raised far beyond the observation window and the + // driver runs in its own process group, signalled as a group. Without both, + // the ORDINARY deadline path cleans up before the check looks — which made + // this test pass with the trap deleted, proving nothing at all. + const driver = join(temp, "trap-driver.sh"); + writeFileSync(driver, `. ${JSON.stringify(LIB)}\nBRAINS_CRED_READ_DEADLINE=45\nbrains_resolve_credential ${JSON.stringify(ORIGIN)}\n`); + const runner = join(temp, "trap-runner.sh"); + const pidFile = join(temp, "trap.pid"); + writeFileSync(runner, + `set -m\nbash ${JSON.stringify(driver)} >/dev/null 2>&1 &\nP=$!\nprintf '%s' "$P" > ${JSON.stringify(pidFile)}\nsleep 2\n` + + `ls -d ${JSON.stringify(state)}/tmp/r.* 2>/dev/null | wc -l\nkill -TERM -"$P" 2>/dev/null\nwait "$P" 2>/dev/null\n`); + const runOut = spawnSync("bash", [runner], { + env: { + ...(process.env as Record), + CLAUDE_PLUGIN_OPTION_TOKEN: "", BRAINS_API_TOKEN: "", BRAINS_INBOX_TOKEN: "", + BRAINS_STATE_DIR: state, PATH: `${trapBin}:${process.env.PATH ?? ""}`, + }, + encoding: "utf8", timeout: 30000, + }); + const during = Number((runOut.stdout ?? "0").trim()); + spawnSync("sleep", ["1.5"]); + const after = readDirNames(join(state, "tmp")).filter((d) => d.startsWith("r.")).length; + check("an interrupted read leaves no credential directory behind", + during >= 1 && after === 0, `during=${during} after=${after}`); +} +{ + // A leader that forks a sleeping descendant and exits 0: the descendant must + // not outlive the read. Standing the watchdog down before sweeping the + // worker's process group is exactly what would orphan it. + const orphanBin = join(temp, "orphan-bin"); + mkdirSync(orphanBin, { recursive: true }); + writeFileSync(join(orphanBin, "security"), `#!/bin/sh\nprintf '{"mcpOAuth":{}}'\n( sleep 25 ) &\nexit 0\n`); + chmodSync(join(orphanBin, "security"), 0o755); + const countSleepers = () => + Number((spawnSync("bash", ["-c", "pgrep -f 'sleep 25' | wc -l"], { encoding: "utf8" }).stdout ?? "0").trim()); + const before = countSleepers(); + sh(`brains_resolve_credential ${JSON.stringify(ORIGIN)} || true`, + { env: { PATH: `${orphanBin}:${process.env.PATH ?? ""}` } }); + spawnSync("sleep", ["1"]); + const after = countSleepers(); + spawnSync("bash", ["-c", "pkill -f 'sleep 25' 2>/dev/null; true"]); + check("a forked descendant of the store reader is not orphaned", + after <= before, `before=${before} after=${after}`); +} + +// =============================================================== bounded network +section("network calls are bounded — no call site can opt out"); +{ + // A socket that accepts and then never answers. This is the shape that hangs + // a hook rather than failing it, and it is worst on Codex, whose Stop hook + // waits for the assistant POST. The default belongs inside brains_request + // precisely because ingest — the one call site outside the inbox lib — is how + // the original `--max-time 5` came to be dropped in the first place. + const blackhole = join(temp, "blackhole.js"); + writeFileSync(blackhole, `require("net").createServer(() => {}).listen(8994, "127.0.0.1");\n`); + const bh = spawn(process.execPath, [blackhole], { stdio: "ignore" }); + spawnSync("sleep", ["0.8"]); + const bhStore = fixture("blackhole-store", { + mcpOAuth: { "a|1": { accessToken: "tok-bh", serverUrl: "http://127.0.0.1:8994/mcp", serverName: "brains" } }, + }); + const started = Date.now(); + const r = sh( + `brains_resolve_credential http://127.0.0.1:8994 || exit 1\n` + + `brains_request ingest http://127.0.0.1:8994/ingest/claude -X POST -d '{}'\n` + + `printf 'rc=%s code=%s' "$?" "$BRAINS_HTTP_CODE"`, + { env: { BRAINS_CLAUDE_CREDENTIALS_FILE: bhStore } }, + ); + const elapsed = Date.now() - started; + bh.kill(); + check("a server that accepts and never answers cannot hang a request", + r.stdout.startsWith("rc=1") && elapsed < 15000, `${r.stdout} in ${elapsed}ms`); + check("brains_request carries default connect and total deadlines", + /--connect-timeout/.test(readFileSync(LIB, "utf8")) && /--max-time/.test(readFileSync(LIB, "utf8"))); + check("the ingest call site keeps its own explicit ceiling", + /brains_request ingest [^\n]*--max-time/.test(readFileSync(TURN, "utf8"))); +} + +// =============================================================== aggregate budget +section("one budget for the whole resolution, not one per read"); +{ + // Per-read deadlines do not compose. The Codex store is enumerated one + // account at a time, so N stale entries would otherwise cost N x the + // per-read deadline — twice per prompt, since the turn hook and the inbox + // engine each resolve independently. + const budgetBin = join(temp, "budget-bin"); + mkdirSync(budgetBin, { recursive: true }); + const accounts = 40; + writeFileSync(join(budgetBin, "security"), + `#!/bin/sh\nif [ "$1" = "dump-keychain" ]; then\n i=0\n while [ $i -lt ${accounts} ]; do\n printf ' "acct"="brains|%s"\\n' "$i"\n printf ' "svce"="Codex MCP Credentials"\\n'\n i=$((i+1))\n done\n exit 0\nfi\nsleep 30\n`); + chmodSync(join(budgetBin, "security"), 0o755); + const started = Date.now(); + const r = sh( + `BRAINS_CRED_CLIENT=codex\n` + + `if brains_resolve_credential ${JSON.stringify(ORIGIN)}; then printf 'RESOLVED'; else printf 'none'; fi`, + { env: { PATH: `${budgetBin}:${process.env.PATH ?? ""}` } }, + ); + const elapsed = Date.now() - started; + check(`${accounts} hanging Codex accounts stay within the aggregate budget`, + r.stdout === "none" && elapsed < 12000, `${r.stdout} in ${elapsed}ms`); +} + +// =============================================================== health namespace +section("health is scoped to the endpoint, not just the capability"); +{ + // Two sessions, one state dir, different servers — production in one window + // and a self-hosted instance in another. With a capability-only key the + // healthy one clears the broken one's rejection and releases its claim, + // putting the self-hosted window back to silently uncaptured. + const state = freshState(); + const okStore = storeFor(`${ORIGIN}/mcp`, "tok-ok"); + const r = sh( + `brains_resolve_credential ${JSON.stringify(ORIGIN)} || exit 1\n` + + `brains_request ingest "${ORIGIN}/forbidden" -X POST -d '{}'\n` + + `printf 'A=%s ' "$(brains_health_state ingest "${ORIGIN}/forbidden")"\n` + + `brains_resolve_credential "http://127.0.0.1:${OTHER}" || exit 1\n` + + `brains_request ingest "http://127.0.0.1:${OTHER}/ingest/claude" -X POST -d '{}'\n` + + `printf 'B=%s ' "$(brains_health_state ingest "http://127.0.0.1:${OTHER}/ingest/claude")"\n` + + `brains_resolve_credential ${JSON.stringify(ORIGIN)} || exit 1\n` + + `printf 'A-again=%s' "$(brains_health_state ingest "${ORIGIN}/forbidden")"`, + { + state, + env: { + BRAINS_CLAUDE_CREDENTIALS_FILE: fixture("two-origin-store", { + mcpOAuth: { + "a|1": { accessToken: "tok-ok", serverUrl: `${ORIGIN}/mcp`, serverName: "brains" }, + "b|1": { accessToken: "tok-other", serverUrl: `http://127.0.0.1:${OTHER}/mcp`, serverName: "brains" }, + }, + }), + }, + }, + ); + check("a healthy endpoint does not clear another endpoint's rejection", + r.stdout === "A=rejected B=ok A-again=rejected", r.stdout); + void okStore; +} + +// =============================================================== truncation +section("an incomplete enumeration means 'I do not know', never 'here is what I found'"); +{ + // A cap that stops early and hands back the first match silently breaks + // exactly-one-or-nothing: a second matching account past the cutoff is + // invisible, and the first gets returned as uniquely valid. That is how a + // conversation ends up captured into the wrong account, with no error. + const codexBin = join(temp, "trunc-bin"); + mkdirSync(codexBin, { recursive: true }); + const dump = (server: string, count: number) => + ` i=0\n while [ $i -lt ${count} ]; do\n printf ' "acct"="${server}|%s"\\n' "$i"\n printf ' "svce"="Codex MCP Credentials"\\n'\n i=$((i+1))\n done\n`; + const resolveCodex = `BRAINS_CRED_CLIENT=codex\nif brains_resolve_credential ${JSON.stringify(ORIGIN)}; then printf 'RESOLVED|%s|%s' "$BRAINS_CRED_STATE" "$BRAINS_CRED_TRUNCATED"; else printf 'none|%s|%s' "$BRAINS_CRED_STATE" "$BRAINS_CRED_TRUNCATED"; fi`; + + // 12 matching accounts against a cap of 8, each with a DIFFERENT valid token. + writeFileSync(join(codexBin, "security"), + `#!/bin/sh\nif [ "$1" = "dump-keychain" ]; then\n${dump("brains", 12)} exit 0\nfi\nprintf '{"server_name":"brains","url":"${ORIGIN}/mcp","token_response":{"access_token":"tok-'"$5"'"}}'\n`); + chmodSync(join(codexBin, "security"), 0o755); + const truncated = sh(resolveCodex, { env: { PATH: `${codexBin}:${process.env.PATH ?? ""}` } }); + check("a truncated scan refuses instead of returning the first match", + truncated.stdout === "none|indeterminate|1", truncated.stdout); + + // 40 accounts for OTHER servers plus one for ours: narrowing by server name + // before any keychain read is what keeps the cap from binding in practice. + writeFileSync(join(codexBin, "security"), + `#!/bin/sh\nif [ "$1" = "dump-keychain" ]; then\n${dump("other-server", 40)} printf ' "acct"="brains|only"\\n'\n printf ' "svce"="Codex MCP Credentials"\\n'\n exit 0\nfi\nprintf '{"server_name":"brains","url":"${ORIGIN}/mcp","token_response":{"access_token":"tok-only"}}'\n`); + chmodSync(join(codexBin, "security"), 0o755); + const narrowed = sh(resolveCodex, { env: { PATH: `${codexBin}:${process.env.PATH ?? ""}` } }); + check("accounts for other servers never consume the candidate budget", + narrowed.stdout === "RESOLVED|ok|0", narrowed.stdout); + + // An account that cannot be READ is the same hazard as one past the cutoff: + // it is a targeted candidate for this server, so it may hold a different + // credential, and returning the readable one as uniquely valid is the + // exactly-one-or-nothing violation by another route. Note the account arrives + // as $5 — the call is `-s -a -w`. + const twoAccounts = `if [ "$1" = "dump-keychain" ]; then\n printf ' "acct"="brains|good"\\n'\n printf ' "svce"="Codex MCP Credentials"\\n'\n printf ' "acct"="brains|second"\\n'\n printf ' "svce"="Codex MCP Credentials"\\n'\n exit 0\nfi\ncase "$5" in\n 'brains|good') printf '{"server_name":"brains","url":"${ORIGIN}/mcp","token_response":{"access_token":"tok-good"}}' ;;\n`; + const secondAccount = (behaviour: string) => { + writeFileSync(join(codexBin, "security"), `#!/bin/sh\n${twoAccounts} 'brains|second') ${behaviour} ;;\nesac\n`); + chmodSync(join(codexBin, "security"), 0o755); + return sh(resolveCodex, { env: { PATH: `${codexBin}:${process.env.PATH ?? ""}` } }).stdout; + }; + check("a second account whose read FAILS refuses selection", + secondAccount("exit 1") === "none|indeterminate|1", secondAccount("exit 1")); + check("a second account whose read TIMES OUT refuses selection", + secondAccount("sleep 30") === "none|indeterminate|1"); + check("a second account with MALFORMED json refuses selection", + secondAccount(`printf '{"token_response":{"access_token":"x"'`) === "none|indeterminate|1"); + // The one case that must not poison the result: parses cleanly, genuinely has + // no token. Treating this as "unknown" would let a single junk keychain entry + // disable capture permanently. + check("a second account that parses and simply has no token resolves cleanly", + secondAccount(`printf '{"server_name":"brains","url":"${ORIGIN}/mcp","token_response":{}}'`) === "RESOLVED|ok|0"); + // Two stale Codex aliases holding the SAME bearer are one credential, not an + // ambiguity. The collapse was applied to the Claude path only after the + // restructure, which turned this into indeterminate and switched capture off + // for a credential that was never ambiguous. + check("two Codex accounts carrying the same token collapse and resolve", + secondAccount(`printf '{"server_name":"brains","url":"${ORIGIN}/mcp","token_response":{"access_token":"tok-good"}}'`) === "RESOLVED|ok|0", + secondAccount(`printf '{"server_name":"brains","url":"${ORIGIN}/mcp","token_response":{"access_token":"tok-good"}}'`)); +} + +// =============================================================== explicit tokens +section("the EXPLICIT tokens stay out of a trace too"); +{ + // The leak battery below drives the discovered-OAuth path, where I1 keeps the + // value out of every variable. The three explicitly configured tokens are + // environment variables — they have to be tested for emptiness and read to + // build the config, and both expand the value — so they need the xtrace + // shield, and only these cases can tell whether it is still there. Removing + // it on the strength of I1 alone reopened the channel for all three, with two + // trace hits each, and nothing here noticed. + const CANARY = "XTRACE-CANARY-77x"; + const driver = join(temp, "explicit-driver.sh"); + writeFileSync(driver, `. ${JSON.stringify(LIB)}\nbrains_resolve_credential ${JSON.stringify(ORIGIN)} >/dev/null\n`); + for (const varName of ["CLAUDE_PLUGIN_OPTION_TOKEN", "BRAINS_API_TOKEN", "BRAINS_INBOX_TOKEN"]) { + const base: Record = { + ...(process.env as Record), + CLAUDE_PLUGIN_OPTION_TOKEN: "", BRAINS_API_TOKEN: "", BRAINS_INBOX_TOKEN: "", + BRAINS_STATE_DIR: freshState(), + [varName]: CANARY, + }; + const traced = spawnSync("bash", ["-x", driver], { env: base, encoding: "utf8", timeout: 20000 }); + check(`${varName} never appears under bash -x`, + !`${traced.stdout ?? ""}${traced.stderr ?? ""}`.includes(CANARY)); + const inherited = spawnSync("bash", [driver], { + env: { ...base, SHELLOPTS: "xtrace" }, encoding: "utf8", timeout: 20000, + }); + check(`${varName} never appears under an inherited SHELLOPTS=xtrace`, + !`${inherited.stdout ?? ""}${inherited.stderr ?? ""}`.includes(CANARY)); + } +} + +// =============================================================== owned paths only +section("the cleanup primitive cannot touch anything it does not own"); +{ + // _brains_discard is an rm -rf on a path derived from its argument, and the + // argument has twice turned out to be caller-supplied. Rather than scoping a + // third call site, the primitive itself refuses anything that is not a direct + // child of the temp root — so a future call site cannot aim it outside. + const outside = join(temp, "not-ours"); + mkdirSync(outside, { recursive: true }); + writeFileSync(join(outside, "SENTINEL.txt"), "keep me"); + const state = freshState(); + const r = sh( + `_brains_cred_tmp_root || exit 1\n` + + `_brains_discard ${JSON.stringify(join(outside, "some-file"))}\n` + + `_brains_discard "$BRAINS_CRED_TMP"/x\n` + + `_brains_discard "$BRAINS_CRED_TMP/a/b/c"\n` + + `printf 'done'`, + { state }, + ); + check("discarding an outside path is a no-op", + existsSync(join(outside, "SENTINEL.txt")) && existsSync(outside), r.stdout); + check("and the temp root itself is never removed", + readDirNames(join(state, "tmp")).length >= 0 && existsSync(join(state, "tmp"))); + + // The Codex override store is a real file in a real directory. Every outcome + // — resolved, non-matching, ambiguous, malformed — must leave it alone. + const cases: Array<[string, unknown, string]> = [ + ["a successful resolve", [{ server_name: "brains", url: `${ORIGIN}/mcp`, token_response: { access_token: "tok-a" } }], "ok"], + ["a non-matching origin", [{ server_name: "brains", url: "http://127.0.0.1:9999/mcp", token_response: { access_token: "tok-a" } }], "blocked"], + ["an ambiguous store", [ + { server_name: "brains", url: `${ORIGIN}/mcp`, token_response: { access_token: "tok-a" } }, + { server_name: "brains", url: `${ORIGIN}/mcp`, token_response: { access_token: "tok-b" } }, + ], "indeterminate"], + ["a malformed store", '[{"server_name":"brains","token_response":{"access_token":', "indeterminate"], + ]; + for (const [label, body, wantState] of cases) { + const userdir = join(temp, `codex-userdir-${Buffer.from(label).toString("hex").slice(0, 8)}`); + mkdirSync(userdir, { recursive: true }); + const storePath = join(userdir, "codex-store.json"); + writeFileSync(storePath, typeof body === "string" ? body : JSON.stringify(body)); + writeFileSync(join(userdir, "SENTINEL.txt"), "keep me"); + const out = sh( + `BRAINS_CRED_CLIENT=codex\nbrains_resolve_credential ${JSON.stringify(ORIGIN)} >/dev/null 2>&1\nprintf '%s' "$BRAINS_CRED_STATE"`, + { env: { BRAINS_CODEX_CREDENTIALS_FILE: storePath } }, + ); + check(`${label}: the user's own store directory survives`, + existsSync(userdir) && existsSync(storePath) && existsSync(join(userdir, "SENTINEL.txt")) && out.stdout === wantState, + `state=${out.stdout} dir=${existsSync(userdir)} store=${existsSync(storePath)}`); + } +} + +// =============================================================== the real hook +section("the turn hook's backgrounded ingest actually carries a credential"); +{ + // Driven through the REAL brains-turn.sh with REAL curl against the stub. + // The other suite that exercises this hook substitutes a fake curl which + // never reads --config, so it structurally cannot notice a missing + // credential: remove the lease and it stays green while every POST goes out + // unauthenticated. This is the only test that can see that. + const state = freshState(); + const before = readHits().length; + const store = fixture("turnhook-store", { + mcpOAuth: { "a|1": { accessToken: "tok-turnhook", serverUrl: `${ORIGIN}/mcp`, serverName: "brains" } }, + }); + const r = spawnSync("bash", [TURN], { + input: JSON.stringify({ session_id: "turnhook", prompt: "hello" }), + env: { + ...(process.env as Record), + CLAUDE_PLUGIN_OPTION_TOKEN: "", BRAINS_API_TOKEN: "", BRAINS_INBOX_TOKEN: "", + BRAINS_STATE_DIR: state, + BRAINS_ENDPOINT: ORIGIN, + BRAINS_CLAUDE_CREDENTIALS_FILE: store, + }, + encoding: "utf8", timeout: 30000, + }); + // The POST is fire-and-forget, so wait for it to land rather than sampling once. + let ingest: Hit[] = []; + for (let i = 0; i < 40 && ingest.length === 0; i++) { + spawnSync("sleep", ["0.1"]); + ingest = readHits().slice(before).filter((h) => h.path === "/ingest/claude"); + } + check("the hook exits 0", r.status === 0, String(r.status)); + check("a backgrounded ingest POST reaches the endpoint", ingest.length === 1, `${ingest.length} POSTs`); + check("and it carries the resolved bearer", + ingest[0]?.auth === "Bearer tok-turnhook", ingest[0]?.auth ?? "(none)"); + // The property the lease exists for, tested deterministically rather than by + // racing: a backgrounded request must survive the master config being + // released. Waiting for the race to bite is unreliable — it passed with the + // lease removed — so this releases the master the instant the child is + // forked, which is the worst case the hook can actually produce. + { + const leaseState = freshState(); + const driver = join(temp, "lease-driver.sh"); + writeFileSync(driver, + `. ${JSON.stringify(LIB)}\n` + + `brains_resolve_credential "${ORIGIN}" || exit 1\n` + + `_l=$(brains_cred_lease) || exit 1\n` + + `( BRAINS_CRED_CONFIG="$_l"; brains_request ingest "${ORIGIN}/ingest/claude" -X POST -d '{}'; brains_cred_return "$_l" ) &\n` + + `brains_cred_release\n` + + `wait\n`); + const mark = readHits().length; + spawnSync("bash", [driver], { + env: { + ...(process.env as Record), + CLAUDE_PLUGIN_OPTION_TOKEN: "", BRAINS_API_TOKEN: "", BRAINS_INBOX_TOKEN: "", + BRAINS_STATE_DIR: leaseState, BRAINS_CLAUDE_CREDENTIALS_FILE: store, + }, + encoding: "utf8", timeout: 30000, + }); + let leased: Hit[] = []; + for (let i = 0; i < 40 && leased.length === 0; i++) { + spawnSync("sleep", ["0.1"]); + leased = readHits().slice(mark).filter((h) => h.path === "/ingest/claude"); + } + check("a leased request survives the master config being released", + leased[0]?.auth === "Bearer tok-turnhook", leased[0]?.auth ?? "(no request arrived)"); + } + + // The lease copy must not outlive the request. + spawnSync("sleep", ["0.5"]); + check("no credential file is left behind afterwards", + readDirNames(join(state, "tmp")).filter((d) => d.startsWith("r.")).length === 0, + readDirNames(join(state, "tmp")).join(",")); +} + +// =============================================================== fresh install +section("a fresh install is told to sign in, not that its store is unreadable"); +{ + // security exits 44 for "item not found" and there is no fallback file: a + // brand-new machine that has simply never signed in. That is a CONCLUSIVE + // absence, and the remedy for it is the sign-in. Reporting indeterminate + // there sends the primary first-run path to a remedy that cannot apply. + const freshBin = join(temp, "fresh-bin"); + mkdirSync(freshBin, { recursive: true }); + writeFileSync(join(freshBin, "security"), + `#!/bin/sh\necho "security: SecKeychainSearchCopyNext: The specified item could not be found in the keychain." >&2\nexit 44\n`); + chmodSync(join(freshBin, "security"), 0o755); + const home = join(temp, "fresh-home"); + mkdirSync(join(home, ".claude"), { recursive: true }); + const r = sh( + `if brains_resolve_credential ${JSON.stringify(ORIGIN)}; then printf 'RESOLVED'; else printf '%s' "$BRAINS_CRED_STATE"; fi`, + { env: { PATH: `${freshBin}:${process.env.PATH ?? ""}`, HOME: home, CLAUDE_CONFIG_DIR: join(home, ".claude"), BRAINS_CLAUDE_CREDENTIALS_FILE: "" } }, + ); + check("keychain item-not-found with no fallback file is a definite no-credential", + r.stdout === "no-credential", r.stdout); + const sig = sh( + `brains_resolve_endpoints "${ORIGIN}"\nbrains_health_note ingest "$BRAINS_URL_INGEST" no-credential\nbrains_capture_signal`, + {}, + ); + check("and the remedy it gets is the sign-in", + sig.stdout.includes("mcp login"), sig.stdout.trim().slice(0, 120)); +} + +// =============================================================== signal delivery +section("a signal during a store read is delivered, not swallowed"); +{ + // Replacing INT/TERM with a cleanup-only handler eats the signal: bash + // resumes afterwards and the caller's handler never runs, so a sourced + // library would be redefining Ctrl-C for the whole hook. + const slowBin = join(temp, "signal-bin"); + mkdirSync(slowBin, { recursive: true }); + writeFileSync(join(slowBin, "security"), `#!/bin/sh\nprintf '{"mcpOAuth":{}}'\nsleep 30\n`); + chmodSync(join(slowBin, "security"), 0o755); + const state = freshState(); + const env = { + ...(process.env as Record), + CLAUDE_PLUGIN_OPTION_TOKEN: "", BRAINS_API_TOKEN: "", BRAINS_INBOX_TOKEN: "", + BRAINS_STATE_DIR: state, PATH: `${slowBin}:${process.env.PATH ?? ""}`, + }; + // Driven from a bash runner rather than node's spawn/kill: the signal, the + // wait and the read have to be ordered, and racing them produced an empty + // capture that looked exactly like a swallowed signal. + const withHandler = join(temp, "sig-handler.sh"); + writeFileSync(withHandler, `. ${JSON.stringify(LIB)}\ntrap 'echo CALLER-HANDLER-RAN; exit 42' TERM\nbrains_resolve_credential ${JSON.stringify(ORIGIN)}\necho SHOULD-NOT-REACH\n`); + const handlerRunner = join(temp, "sig-handler-runner.sh"); + const handlerOut = join(temp, "sig-handler.out"); + writeFileSync(handlerRunner, `bash ${JSON.stringify(withHandler)} >${JSON.stringify(handlerOut)} 2>/dev/null &\nP=$!\nsleep 0.8\nkill -TERM $P 2>/dev/null\nwait $P 2>/dev/null\nprintf '%s' "$?"\n`); + const handlerRun = spawnSync("bash", [handlerRunner], { env, encoding: "utf8", timeout: 20000 }); + const out1 = existsSync(handlerOut) ? readFileSync(handlerOut, "utf8") : ""; + check("the caller's own signal handler still runs", + out1.includes("CALLER-HANDLER-RAN") && !out1.includes("SHOULD-NOT-REACH"), JSON.stringify(out1)); + check("the caller's handler controls the exit status", + (handlerRun.stdout ?? "").trim() === "42", (handlerRun.stdout ?? "").trim()); + + const noHandler = join(temp, "sig-nohandler.sh"); + writeFileSync(noHandler, `. ${JSON.stringify(LIB)}\nbrains_resolve_credential ${JSON.stringify(ORIGIN)}\necho SHOULD-NOT-REACH\n`); + const runner = join(temp, "sig-runner.sh"); + writeFileSync(runner, `bash ${JSON.stringify(noHandler)} >/dev/null 2>&1 &\nP=$!\nsleep 0.8\nkill -TERM $P 2>/dev/null\nwait $P 2>/dev/null\nprintf '%s' "$?"\n`); + const status = spawnSync("bash", [runner], { env, encoding: "utf8", timeout: 20000 }); + // 143 = 128 + SIGTERM: the status a process gets when it is not handling it. + check("with no handler, the normal terminating status is preserved", + (status.stdout ?? "").trim() === "143", (status.stdout ?? "").trim()); + // Read the directory rather than shelling out: a quoted glob in `bash -c` + // never expands, so the shell version was asserting on a path that could not + // match anything and passed for the wrong reason. + // + // Polled, not sampled once. Cleanup is prompt but not synchronous with the + // parent's exit becoming observable to `wait` — measured at under two seconds + // — and asserting on the instant after `wait` was reading a directory mid + // removal. What matters is that nothing is left behind, not the exact moment. + let remaining = readDirNames(join(state, "tmp")).filter((d) => d.startsWith("r.")); + for (let i = 0; i < 30 && remaining.length > 0; i++) { + spawnSync("sleep", ["0.1"]); + remaining = readDirNames(join(state, "tmp")).filter((d) => d.startsWith("r.")); + } + check("and the credential directory is not left behind", remaining.length === 0, remaining.join(",")); +} + +// =============================================================== curlrc +section("the user's own curl config cannot capture the bearer"); +{ + // -q, and it must be curl's first argument. Without it curl reads ~/.curlrc, + // and a curlrc carrying `trace-ascii` writes outgoing headers to a file — + // measured on curl 8.7.1. Keeping the token out of argv and xtrace does + // nothing about a channel the user's own debugging config opens. + const fakeHome = join(temp, "curlrc-home"); + mkdirSync(fakeHome, { recursive: true }); + const tracePath = join(temp, "curl-trace.txt"); + writeFileSync(join(fakeHome, ".curlrc"), `trace-ascii ${tracePath}\n`); + const CANARY = "CURLRCCANARY99"; + const store = fixture("curlrc-store", { + mcpOAuth: { "a|1": { accessToken: CANARY, serverUrl: `${ORIGIN}/mcp`, serverName: "brains" } }, + }); + sh( + `brains_resolve_credential ${JSON.stringify(ORIGIN)} || exit 1\n` + + `brains_request ingest "${ORIGIN}/ingest/claude" -X POST -d '{}'`, + { env: { BRAINS_CLAUDE_CREDENTIALS_FILE: store, HOME: fakeHome, CURL_HOME: fakeHome } }, + ); + const traced = existsSync(tracePath) ? readFileSync(tracePath, "utf8") : ""; + check("a curlrc that enables tracing never receives the token", + !traced.includes(CANARY), traced ? "trace file written and contains the canary" : ""); +} + +// =============================================================== no token escapes +section("the token never leaves brains_request"); +{ + const hookSources = [readFileSync(TURN, "utf8"), readFileSync(INBOX, "utf8")]; + for (const [i, src] of hookSources.entries()) { + const name = i === 0 ? "brains-turn.sh" : "brains-inbox.sh"; + // The hooks may name the config PATH — a backgrounded request has to carry + // its own lease — but must never touch a token value, read a store, or + // build an Authorization header. Presenting the credential is + // brains_request's job alone. + check(`${name} never handles a credential value`, + !/Authorization/.test(src) && + !/mcpOAuth|find-generic-password|dump-keychain/.test(src), + "only brains_request may present the credential"); + } + const lib = readFileSync(LIB, "utf8"); + check("no debug tracing is enabled anywhere in the library", !/^\s*set -x/m.test(lib)); + // Grepping the source for uses of the value proved to be the wrong test: it + // passes while the token is still being handed to curl as an argument, where + // any local `ps` can read it, and printed in full by `bash -x` before any + // redirection this code controls applies. What follows drives the real thing + // and looks for the bytes, which is the only claim worth making. + const CANARY = "LEAKCANARY0123456789"; + const leakStore = fixture("leak-store", { + mcpOAuth: { "a|1": { accessToken: CANARY, serverUrl: `${ORIGIN}/mcp`, serverName: "brains" } }, + }); + const driver = join(temp, "leak-driver.sh"); + writeFileSync(driver, `. ${JSON.stringify(LIB)}\nbrains_resolve_credential ${JSON.stringify(ORIGIN)} || exit 1\nbrains_request ingest ${JSON.stringify(`${ORIGIN}/ingest/claude`)} -X POST -d '{"a":1}'\n`); + const leakEnv = { + ...(process.env as Record), + CLAUDE_PLUGIN_OPTION_TOKEN: "", + BRAINS_API_TOKEN: "", + BRAINS_INBOX_TOKEN: "", + BRAINS_STATE_DIR: freshState(), + BRAINS_CLAUDE_CREDENTIALS_FILE: leakStore, + }; + const traced = spawnSync("bash", ["-x", driver], { env: leakEnv, encoding: "utf8", timeout: 20000 }); + check("bash -x never prints the token", + !`${traced.stdout ?? ""}${traced.stderr ?? ""}`.includes(CANARY)); + const inherited = spawnSync("bash", [driver], { + env: { ...leakEnv, SHELLOPTS: "xtrace" }, encoding: "utf8", timeout: 20000, + }); + check("an inherited SHELLOPTS=xtrace never prints the token", + !`${inherited.stdout ?? ""}${inherited.stderr ?? ""}`.includes(CANARY)); + // Tracing belongs to the caller; shielding it must not switch it off for good. + const restored = spawnSync("bash", ["-x", "-c", `. ${JSON.stringify(driver)} >/dev/null 2>&1; echo AFTER`], { + env: leakEnv, encoding: "utf8", timeout: 20000, + }); + check("the caller's tracing is restored afterwards", + (restored.stderr ?? "").includes("+ echo AFTER")); + + // And off argv, so a local `ps` cannot read it either. The request is run in + // the background and sampled while it is still in flight. + const slowDriver = join(temp, "leak-slow.sh"); + writeFileSync(slowDriver, `. ${JSON.stringify(LIB)}\nbrains_resolve_credential ${JSON.stringify(ORIGIN)} || exit 1\nbrains_request ingest ${JSON.stringify(`http://127.0.0.1:${TRUNCATOR}/slow`)} -X POST -d '{"a":1}'\n`); + const bg = spawn("bash", [slowDriver], { env: leakEnv, stdio: "ignore" }); + let seenInArgv = false; + for (let i = 0; i < 20; i++) { + const ps = spawnSync("ps", ["-Ao", "args"], { encoding: "utf8" }); + if ((ps.stdout ?? "").split("\n").some((l) => l.includes(CANARY) && l.includes("curl"))) { + seenInArgv = true; + break; + } + spawnSync("sleep", ["0.05"]); + } + bg.kill(); + check("the token never appears in curl's process arguments", !seenInArgv); +} + +// =============================================================== store size +section("a large but valid store still resolves"); +{ + // The document ceiling bounds memory; it is not a policy. At 256 KiB — about a + // hundred signed-in MCP servers — a perfectly valid store was rejected, and + // the user was then told to remove a duplicate server that did not exist. + // There is no keychain fallback on Linux, so that is the whole store. + const entries: Record = {}; + for (let i = 0; i < 3000; i++) { + entries[`filler|${i}`] = { accessToken: `t${i}`, serverUrl: `https://other-${i}.example.com/mcp`, serverName: "other" }; + } + entries["plugin:brains:brains|h"] = { accessToken: "tok-big", serverUrl: `${ORIGIN}/mcp`, serverName: "plugin:brains:brains" }; + const big = fixture("oversize-store", { mcpOAuth: entries }); + const bytes = readFileSync(big, "utf8").length; + check(`the fixture is genuinely over the old 256 KiB ceiling (${bytes} bytes)`, bytes > 262144); + check("a store far larger than one credential document still resolves", + resolveWith(big).stdout.startsWith("ok|claude-oauth|"), resolveWith(big).stdout); +} + +// =============================================================== codex arrays +section("duplicate collapse covers the array backend as well as the keychain"); +{ + // The keychain backend gets one private file per account; the array backend + // names the same document on every row, so the slurp-based collapse saw one + // array and errored, and two aliases holding a single bearer were reported as + // an ambiguity — capture off for a credential that was never ambiguous. + const probe = (body: unknown) => { + const f = fixture(`codex-array-${Buffer.from(JSON.stringify(body)).toString("hex").slice(0, 10)}`, body); + return sh( + `BRAINS_CRED_CLIENT=codex\n` + + `if brains_resolve_credential ${JSON.stringify(ORIGIN)}; then printf 'RESOLVED|%s' "$BRAINS_CRED_COUNT"; else printf 'none|%s|%s' "$BRAINS_CRED_STATE" "$BRAINS_CRED_COUNT"; fi`, + { env: { BRAINS_CODEX_CREDENTIALS_FILE: f } }, + ).stdout; + }; + const same = [ + { server_name: "brains", url: `${ORIGIN}/mcp`, token_response: { access_token: "tok-same" } }, + { server_name: "brains", url: `${ORIGIN}/mcp`, token_response: { access_token: "tok-same" } }, + ]; + const diff = [ + { server_name: "brains", url: `${ORIGIN}/mcp`, token_response: { access_token: "tok-a" } }, + { server_name: "brains", url: `${ORIGIN}/mcp`, token_response: { access_token: "tok-b" } }, + ]; + check("two array entries carrying one bearer collapse and resolve", + probe(same) === "RESOLVED|1", probe(same)); + check("two array entries carrying different bearers stay indeterminate", + probe(diff) === "none|indeterminate|2", probe(diff)); +} + +// =============================================================== mutation gate +section("every load-bearing mechanism is proved load-bearing, here, on every run"); +// Three rounds in a row shipped a mechanism with no test, and twice a test that +// DID exist quietly decayed into a tautology — the interrupt-cleanup check +// passed with its trap deleted, and the xtrace-restore check passed with the +// shield deleted. Adding another hand-written test does not stop either of +// those; both failure modes look exactly like a green suite. +// +// So the suite mutates the shipped code and requires the damage to show. Each +// entry below neuters one mechanism in a COPY of the plugin tree, runs a small +// probe against that copy, and fails unless the probe's answer changes. A +// mechanism with no observable consequence cannot be registered here, and a +// test that has decayed into a tautology stops passing the moment its mechanism +// is removed. Registering a new mechanism is one table entry. +type Mutation = { + label: string; + file: string; // relative to plugins/brains + find: string; + replace: string; + probe: string; // shell, with $LIB pointing at the mutated copy + env?: Record; + stubs?: Record; // written to $STUBS, chmod +x + requires?: string; // the unmutated probe MUST return this, or SKIP loudly +}; +// A stub keychain tool that enumerates one Codex account, for hosts with none. +const CODEX_DUMP_STUB = [ + "#!/bin/sh", + 'if [ "$1" = "dump-keychain" ]; then', + ' printf \' "acct"="brains|a"\\n\'', + ' printf \' "svce"="Codex MCP Credentials"\\n\'', + " exit 0", + "fi", + 'printf \'{"server_name":"brains","url":"%s/mcp","token_response":{"access_token":"tok-x"}}\' "$ORIGIN"', + "", +].join("\n"); +const MUTATIONS: Mutation[] = [ + { + // Registered as the PAIR — the trap and the explicit return — and observed + // on NORMAL completion. Two things forced that. The trap alone is not + // separately observable: a signal arriving while curl runs is deferred by + // bash until curl returns, at which point the explicit return runs anyway, + // so the trap only covers a signal landing BETWEEN commands and that cannot + // be scheduled. And signalling at all made the probe answer differently run + // to run — it passed three times and failed the fourth — which certifies + // nothing. What is deterministic, and is the property that matters: once + // the backgrounded request has finished, its private copy of the bearer is + // gone. + label: "the turn hook's ingest-lease cleanup (trap + explicit return)", + file: "hooks/brains-turn.sh", + find: ` trap 'brains_cred_return "$_lease"' EXIT INT TERM HUP\n ingest_once "$role" "$payload"\n brains_cred_return "$_lease" ) &`, + replace: ` ingest_once "$role" "$payload" ) &`, + // A terminated child must not strand its leased copy of the bearer. + probe: `bash "$TURNCOPY" <<<'{"session_id":"mut","prompt":"x"}' >/dev/null 2>&1; k=0; while [ $k -lt 80 ]; do [ -z "$(find "$BRAINS_STATE_DIR/tmp" -path "*lease*" -name curl.conf 2>/dev/null)" ] && break; sleep 0.1; k=$((k+1)); done; printf '%s' "$(find "$BRAINS_STATE_DIR/tmp" -path "*lease*" -name curl.conf 2>/dev/null | wc -l | tr -d ' ')"`, + env: { BRAINS_API_TOKEN: "tok-slow" }, + }, + { + // Registered as the PAIR — the trap and the explicit return — because they + // are not separately observable and saying otherwise would be the vacuous + // certification this gate exists to stop. A signal delivered while curl is + // running is deferred by bash until curl returns, at which point the + // explicit return runs anyway; the trap only covers a signal landing + // BETWEEN commands, which cannot be scheduled deterministically. Mutating + // either line alone therefore shows no difference, and mutating both shows + // the lease surviving with the bearer in it. See the report note. + label: "the inbox engine's ack-lease cleanup (trap + explicit return)", + file: "hooks/lib/brains-inbox.sh", + find: ` trap 'brains_cred_return "$ack_lease"' EXIT INT TERM HUP\n brains_request ack "$ACK_ENDPOINT" --max-time 3 -X POST \\\n -H "Content-Type: application/json" -d "$ack" >/dev/null 2>&1\n brains_cred_return "$ack_lease" ) &`, + replace: ` brains_request ack "$ACK_ENDPOINT" --max-time 3 -X POST \\\n -H "Content-Type: application/json" -d "$ack" >/dev/null 2>&1 ) &`, + // Observed on NORMAL completion rather than by signalling. Signalling a + // backgrounded ack is inherently racy — whether the trap or the explicit + // return gets to run depends on exactly where the subshell is when the + // signal lands, and a probe that answers differently run to run certifies + // nothing. The property that matters and is deterministic: once the ack has + // finished, its private copy of the bearer is gone. + probe: `bash "$INBOXCOPY" prompt mut >/dev/null 2>&1; k=0; while [ $k -lt 80 ]; do [ -z "$(find "$BRAINS_STATE_DIR/tmp" -path "*lease*" -name curl.conf 2>/dev/null)" ] && break; sleep 0.1; k=$((k+1)); done; printf '%s' "$(find "$BRAINS_STATE_DIR/tmp" -path "*lease*" -name curl.conf 2>/dev/null | wc -l | tr -d ' ')"`, + env: { + BRAINS_API_TOKEN: "tok-slow", + // The engine only acks when the inbox returns something to acknowledge, + // and the ack has to still be in flight when the signal arrives. + BRAINS_INBOX_URL: `${ORIGIN}/inbox/ackable`, + BRAINS_INBOX_ACK_URL: `${ORIGIN}/inbox/claude/ack`, + }, + }, + { + label: "the account-list completeness check (LC_ALL=C + PIPESTATUS)", + file: "hooks/lib/brains-credential.sh", + find: ` set -- "\${PIPESTATUS[0]}" "\${PIPESTATUS[1]}"`, + replace: ` set -- 0 0`, + // With awk failing, a partial account list must not be treated as complete. + // Stubs are written from the test rather than by an escaped printf inside + // the probe: the shell-quoted version worked on macOS and produced a + // security stub the Linux runner could not use, so the probe answered + // "no-credential" both with and without the mechanism and the gate — quite + // correctly — refused to certify it. + stubs: { security: CODEX_DUMP_STUB, awk: "#!/bin/sh\nexit 2\n" }, + probe: `PATH="$STUBS:$PATH"; unset BRAINS_CLAUDE_CREDENTIALS_FILE +. "$LIB"; BRAINS_CRED_CLIENT=codex; brains_resolve_credential "$ORIGIN" >/dev/null 2>&1 +printf '%s/%s' "$BRAINS_CRED_STATE" "$BRAINS_CRED_TRUNCATED"`, + // The mechanism only exists on the account-enumeration path, which needs a + // usable security binary. If the stub cannot be made to work, that is recorded + // out loud — a quietly skipped mutation probe is exactly the vacuous pass + // this gate exists to prevent. + requires: "indeterminate/1", + }, + { + label: "the session-end prune of SIGKILL leftovers", + file: "hooks/brains-end.sh", + find: ` . "$CRED_LIB" 2>/dev/null && brains_cred_prune_tmp`, + replace: ` :`, + probe: `mkdir -p "$BRAINS_STATE_DIR/tmp/r.stale"; touch -t 202601010000 "$BRAINS_STATE_DIR/tmp/r.stale"; bash "$ENDCOPY" <<<'{"session_id":"mut"}' >/dev/null 2>&1; printf '%s' "$([ -d "$BRAINS_STATE_DIR/tmp/r.stale" ] && echo present || echo pruned)"`, + }, + { + label: "the xtrace shield around the explicit-token branches", + file: "hooks/lib/brains-credential.sh", + find: ` *x*) BRAINS_CRED_XTRACE=1; set +x ;;`, + replace: ` *x*) BRAINS_CRED_XTRACE="" ;;`, + probe: `printf '%s' "$(bash -x -c '. "$0"; brains_resolve_credential "$1" >/dev/null' "$LIB" "$ORIGIN" 2>&1 | grep -c MUTCANARY)"`, + env: { BRAINS_API_TOKEN: "MUTCANARY" }, + }, + { + label: "the discard guard that confines rm -rf to the owned root", + file: "hooks/lib/brains-credential.sh", + find: ` case "$parent" in + "$BRAINS_CRED_TMP"/*/*) return 0 ;; + "$BRAINS_CRED_TMP"/?*) ;; + *) return 0 ;; + esac`, + replace: "", + probe: `. "$LIB"; _brains_cred_tmp_root >/dev/null 2>&1; mkdir -p "$OUTSIDE/keep"; _brains_discard "$OUTSIDE/keep/f" 2>/dev/null; printf '%s' "$([ -d "$OUTSIDE/keep" ] && echo intact || echo DELETED)"`, + }, +]; +{ + const treeSrc = PLUGIN; + for (const m of MUTATIONS) { + const work = mkdtempSync(join(tmpdir(), "brains-mutation-")); + spawnSync("cp", ["-R", treeSrc, join(work, "brains")]); + const target = join(work, "brains", m.file); + const original = readFileSync(target, "utf8"); + if (!original.includes(m.find)) { + check(`${m.label}: mutation anchor still present in the shipped code`, false, + "the code moved; update the anchor rather than dropping the mutation"); + rmSync(work, { recursive: true, force: true }); + continue; + } + const stubDir = join(work, "stubs"); + if (m.stubs) { + mkdirSync(stubDir, { recursive: true }); + for (const [name, body] of Object.entries(m.stubs)) { + writeFileSync(join(stubDir, name), body); + chmodSync(join(stubDir, name), 0o755); + } + } + const runProbe = (): string => { + const st = freshState(); + const env: Record = { + ...(process.env as Record), + CLAUDE_PLUGIN_OPTION_TOKEN: "", BRAINS_API_TOKEN: "", BRAINS_INBOX_TOKEN: "", + BRAINS_STATE_DIR: st, + BRAINS_ENDPOINT: ORIGIN, + BRAINS_CLAUDE_CREDENTIALS_FILE: PRIMARY_STORE, + LIB: join(work, "brains", "hooks", "lib", "brains-credential.sh"), + TURNCOPY: join(work, "brains", "hooks", "brains-turn.sh"), + INBOXCOPY: join(work, "brains", "hooks", "lib", "brains-inbox.sh"), + ENDCOPY: join(work, "brains", "hooks", "brains-end.sh"), + ORIGIN, + OUTSIDE: join(work, "outside"), + STUBS: stubDir, + ...(m.env ?? {}), + }; + const r = spawnSync("bash", ["-c", `set -u\n${m.probe}`], { env, encoding: "utf8", timeout: 30000 }); + return (r.stdout ?? "").trim(); + }; + const before = runProbe(); + if (m.requires !== undefined && before !== m.requires) { + // Loudly, and as a FAILURE of the gate's own preconditions rather than a + // pass: the mechanism may be fine, but nothing here observed it. + rmSync(work, { recursive: true, force: true }); + check(`${m.label}: probe precondition not met on this host`, false, + `expected the unmutated probe to return "${m.requires}", got "${before}" — the probe cannot see its own mechanism, so it certifies nothing`); + continue; + } + writeFileSync(target, original.split(m.find).join(m.replace)); + const after = runProbe(); + rmSync(work, { recursive: true, force: true }); + check(`${m.label} is load-bearing and observable`, + before !== after, `probe returned "${before}" both with and without it`); + } +} + +// =============================================================== teardown +stubProc.kill(); +rmSync(temp, { recursive: true, force: true }); + +console.log(`\n${passed} passed, ${failed} failed (${passed + failed} total)`); +process.exit(failed === 0 ? 0 : 1); diff --git a/tests/credential/stubs.js b/tests/credential/stubs.js new file mode 100644 index 0000000..be06a96 --- /dev/null +++ b/tests/credential/stubs.js @@ -0,0 +1,46 @@ +// Stub endpoints for tests/credential/run.ts. +// +// This runs as a SEPARATE PROCESS, and that is load-bearing rather than +// stylistic. Every assertion in the suite drives the hooks through spawnSync, +// which blocks the test process's event loop — so a stub served from inside +// that process could never answer the request it exists to answer. The suite +// would deadlock rather than fail, and a hang reports nothing at all. +// +// Usage: node stubs.js +const http = require("http"), net = require("net"), fs = require("fs"); +const LOG = process.argv[2]; +const PRIMARY = Number(process.argv[3]), OTHER = Number(process.argv[4]), TRUNCATOR = Number(process.argv[5]); +const log = (port, req, body) => fs.appendFileSync(LOG, JSON.stringify({ + port, method: req.method, path: (req.url || "").split("?")[0], + auth: req.headers.authorization || "", bodyLen: body.length, +}) + "\n"); +const mk = (port, handler) => http.createServer((req, res) => { + let b = ""; + req.on("data", (c) => { b += c; }); + req.on("end", () => { + log(port, req, b); + const r = handler(req.url || ""); + res.writeHead(r.code, { "content-type": "application/json" }); + res.end(r.body); + }); +}).listen(port, "127.0.0.1"); +mk(PRIMARY, (p) => { + if (p.startsWith("/forbidden")) return { code: 403, body: '{"error":"forbidden"}' }; + // Returns something the inbox engine must ACK, so a test can exercise the + // acknowledgement path — and therefore its credential lease. + if (p.startsWith("/inbox/ackable")) return { code: 200, body: JSON.stringify({ context_items: [{ kind: "k", key: "v" }], actions: [] }) }; + if (p.startsWith("/inbox/claude/devices")) return { code: 200, body: JSON.stringify({ device_id: "dev-1" }) }; + if (p.startsWith("/inbox/claude")) return { code: 200, body: JSON.stringify({ actions: [] }) }; + return { code: 200, body: '{"ok":true}' }; +}); +mk(OTHER, () => ({ code: 200, body: '{"ok":true}' })); +// Valid headers plus a COMPLETE, parseable JSON prefix, then the socket dies. +// curl reports 200 for this, because the status is written the moment headers +// arrive — only its exit status reveals that the body never finished. +net.createServer((s) => { + s.on("data", () => { + s.write("HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: 400\r\n\r\n"); + s.write('{"device_id":"d1"}'); + setTimeout(() => s.destroy(), 80); + }); +}).listen(TRUNCATOR, "127.0.0.1"); diff --git a/tests/inbox-v2/demo-approval-followthrough.ts b/tests/inbox-v2/demo-approval-followthrough.ts index 350a970..566402a 100644 --- a/tests/inbox-v2/demo-approval-followthrough.ts +++ b/tests/inbox-v2/demo-approval-followthrough.ts @@ -52,6 +52,8 @@ function runClaude(prompt: string): Promise { const proc = spawn("claude", ["--settings", SETTINGS, "-p", prompt], { env: { ...process.env, + // Hermetic: never let a real developer credential resolve in a test. + BRAINS_CREDENTIAL_STORE_DISABLED: "1", BRAINS_STATE_DIR: HOOK_DIR, BRAINS_MARKETPLACES_JSON: join(TEST_DIR, "no-such-marketplaces.json"), BRAINS_INBOX_URL: `http://127.0.0.1:${PORT}/inbox/claude`, diff --git a/tests/inbox-v2/demo-safety-limit.ts b/tests/inbox-v2/demo-safety-limit.ts index 43e1358..975c348 100644 --- a/tests/inbox-v2/demo-safety-limit.ts +++ b/tests/inbox-v2/demo-safety-limit.ts @@ -51,6 +51,8 @@ function runClaude(prompt: string): Promise { const proc = spawn("claude", ["--settings", buildSettings(), "-p", prompt], { env: { ...process.env, + // Hermetic: never let a real developer credential resolve in a test. + BRAINS_CREDENTIAL_STORE_DISABLED: "1", BRAINS_STATE_DIR: HOOK_DIR, BRAINS_MARKETPLACES_JSON: join(TEST_DIR, "no-such-marketplaces.json"), BRAINS_INBOX_URL: `http://127.0.0.1:${PORT}/inbox/claude`, diff --git a/tests/inbox-v2/live-claude.ts b/tests/inbox-v2/live-claude.ts index bb37be0..0bd45c0 100644 --- a/tests/inbox-v2/live-claude.ts +++ b/tests/inbox-v2/live-claude.ts @@ -106,6 +106,8 @@ async function runClaude(opts: { return new Promise((resolveRun) => { const env = { ...process.env, + // Hermetic: never let a real developer credential resolve in a test. + BRAINS_CREDENTIAL_STORE_DISABLED: "1", BRAINS_STATE_DIR: HOOK_DIR, // Pin marketplaces to an absent path so the auto-update nudge stays // hermetic and never reads the developer's real ~/.claude. diff --git a/tests/inbox-v2/run.ts b/tests/inbox-v2/run.ts index 2be8df3..0161b9e 100644 --- a/tests/inbox-v2/run.ts +++ b/tests/inbox-v2/run.ts @@ -168,6 +168,8 @@ async function runPluginHook(opts: { delete inherited.PLUGIN_DATA; const env = { ...inherited, + // Hermetic: never let a real developer credential resolve in a test. + BRAINS_CREDENTIAL_STORE_DISABLED: "1", BRAINS_STATE_DIR: opts.stateDir, // Pin the marketplaces file so auto-update detection is hermetic and // never reads the developer's real ~/.claude. Default to an absent path @@ -811,7 +813,7 @@ const SCENARIOS: Scenario[] = [ // Sections come from the shipped core.md; this release bumps it to v6. const sections = (report.sections ?? []) as Array<{ name: string; version: number }>; const core = sections.find((s) => s.name === "core"); - assertEqual(core?.version, 6, "25 core marker reported at v6"); + assertEqual(core?.version, 7, "25 core marker reported at v7"); // No drift → no update nudge in stdout. assertNotContains(r.stdout, "brains:update", "25 no update nudge when device is current"); diff --git a/tests/plugin-contract/run.ts b/tests/plugin-contract/run.ts index 13e67bd..e6d33f5 100644 --- a/tests/plugin-contract/run.ts +++ b/tests/plugin-contract/run.ts @@ -278,11 +278,12 @@ const CLAUDE_WEB_REGION = [ // prone to false positives, so the wording itself is the contract; the regex pair further down // stays only as a backstop. const CLAUDE_TOKEN_TITLE = "brains API token (optional)"; +// The old copy ended "without one, capture and the inbox simply stay off". That stopped being +// true the moment the hooks learned to resolve the credential from the client's own MCP sign-in, +// and copy that describes a gate which no longer exists is how a user concludes the feature is +// opt-in and never looks again. The token is an override now, and the text has to say so. const CLAUDE_TOKEN_DESCRIPTION = - "Optional. Enables conversation capture and the inbox, which authenticate separately from the " + - "MCP server. NOT how the brains tools authenticate — that is `claude mcp login " + - "plugin:brains:brains`. Find it in your brains account settings; without one, capture and the " + - "inbox simply stay off."; + "Optional override. Conversation capture and the inbox follow your MCP sign-in — `claude mcp login plugin:brains:brains` — and need nothing set here. Use this only to capture into a different brains account, or to reach a self-hosted endpoint your sign-in does not cover. Find it in your brains account settings."; // Approved endpoint copy, pinned for the same reason and against a specific regression: this text // used to promise that /mcp derived from the endpoint, and it would have gone on saying so after @@ -339,7 +340,7 @@ const CODEX_INSTALL_REGION = [ "Needs Codex **" + CODEX_MIN_VERSION + "** or newer. Check with:", "", "```sh", - CODEX_CAPABILITY_PROBE, + "codex plugin --help", "```", "", "If that errors with an unknown subcommand, run `codex update` first.", @@ -358,8 +359,12 @@ const CODEX_INSTALL_REGION = [ "", "Restart the ChatGPT desktop app or start a new Codex thread. The first time the", "plugin loads, open `/hooks` and trust the bundled brains hooks — that is what", - "runs automatic recall and error feedback. Capture and inbox delivery also need a", - "capture credential — normally the token below.", + "runs automatic recall and error feedback. Capture and inbox delivery use the", + "sign-in above as their credential, so there is nothing further to set.", + "", + "Capture and the inbox are **macOS only** for Codex: they read the credential", + "from the macOS keychain, and Codex on Linux is not a supported configuration.", + "The tools and recall still work there; capture and inbox delivery do not.", "", "Everyday reading and writing is covered by default. For admin-gated tools or", "performance insights, sign in asking for them explicitly (both also need the", @@ -388,15 +393,18 @@ const CODEX_INSTALL_REGION = [ // session, and server updates arrive there too.") passed the canary in the body. Editing stays // possible; it is now the deliberate two-line diff every other region in this file already demands. const CODEX_OPTIONAL_REGION = [ - "### Optional: conversation capture and the inbox", + "### Optional: an explicit capture token", + "", + "You do not need this. Capture and the inbox read the credential `codex mcp login", + "brains` already stored, so the sign-in above is all they need. To check what has", + "been captured, ask brains which chats it has, or run", + "`list_pages type=chat_session`.", "", - "The tools above work without this. Capture and the inbox are shell hooks that", - "authenticate separately from the MCP server and cannot read the credential Codex", - "keeps internally, so they need a brains API token of their own — find it in your", - "brains account settings. Without one they simply stay off.", + "Set a token to capture into a different brains account, or to reach an endpoint", + "your sign-in does not cover — find it in your brains account settings:", "", "```sh", - 'export BRAINS_API_TOKEN=""', + "export BRAINS_API_TOKEN=\"\"", "```", "", "That applies to Codex started from that shell. The macOS desktop app never", @@ -404,7 +412,7 @@ const CODEX_OPTIONAL_REGION = [ "restart the app:", "", "```sh", - 'launchctl setenv BRAINS_API_TOKEN ""', + "launchctl setenv BRAINS_API_TOKEN \"\"", "```", "", "This token is only for capture and the inbox. It is **not** how Codex", @@ -419,12 +427,12 @@ const CODEX_OPTIONAL_REGION = [ const CLAUDE_INSTALL_REGION = [ "## Install for Claude Code", "", - CLAUDE_TOKEN_OPENER, + "No token needed — Claude Code signs itself in.", "", "```sh", "claude plugin marketplace add https://github.com/ssvlabs/brains-plugins.git", "claude plugin install brains@brains", - CLAUDE_MCP_LOGIN, + "claude mcp login plugin:brains:brains", "```", "", "If Claude Code does not recognise that login name, run `claude mcp list` and use the name it", @@ -436,35 +444,37 @@ const CLAUDE_INSTALL_REGION = [ "Code stores the credential itself, so there is nothing to copy or keep. Confirm with", "`claude mcp list`.", "", - CLAUDE_VERSION_NOTE, + "This flow was verified on Claude Code 2.1.220. If `claude mcp login` is not a recognised", + "command, update Claude Code.", "", "Restart Claude Code or start a new session. The first time the plugin loads, trust the bundled", "brains hooks — that is what runs automatic recall and error feedback. Capture and inbox delivery", - "also need the token below.", + "use the sign-in above as their credential, so there is normally nothing further to set. If", + "`list_pages type=chat_session` shows nothing after a few turns, set the token below.", "", "For a local checkout under development:", "", "```sh", "claude plugin marketplace add /absolute/path/to/brains-plugins", "claude plugin install brains@brains", - CLAUDE_MCP_LOGIN, + "claude mcp login plugin:brains:brains", "```", ].join("\n"); // The Claude token section and the migration section, heading lines included, pinned for the same // reason as the Codex one above. const CLAUDE_OPTIONAL_REGION = [ - "### Optional: conversation capture and the inbox", + "### Optional: an explicit capture token", "", - "The tools above work without this. Capture and the inbox are shell hooks that authenticate", - "separately from the MCP server and cannot read the credential Claude Code keeps internally, so", - "they need a brains API token of their own — find it in your brains account settings. Without one", - "they simply stay off.", + "You do not need this. Capture and the inbox read the credential `claude mcp login", + "plugin:brains:brains` already stored, so the sign-in above is all they need. To check what has", + "been captured, ask brains which chats it has, or run `list_pages type=chat_session`.", "", - "Set it when you install:", + "Set a token only to capture into a different brains account, or to reach an endpoint your sign-in", + "does not cover — find it in your brains account settings:", "", "```sh", - 'claude plugin install brains@brains --config token=""', + "claude plugin install brains@brains --config token=\"\"", "```", "", "Or change it afterwards with `/plugin` → brains → Configure.", @@ -769,6 +779,7 @@ const claudeHooksSource = readFileSync(join(PLUGIN, "hooks", "claude-hooks.json" const codexHooksSource = readFileSync(join(PLUGIN, "hooks", "hooks.json"), "utf8"); const turnHook = readFileSync(join(PLUGIN, "hooks", "brains-turn.sh"), "utf8"); const inboxHook = readFileSync(join(PLUGIN, "hooks", "lib", "brains-inbox.sh"), "utf8"); +const credLib = readFileSync(join(PLUGIN, "hooks", "lib", "brains-credential.sh"), "utf8"); const readme = readFileSync(join(ROOT, "README.md"), "utf8"); const core = readFileSync(join(PLUGIN, "core.md"), "utf8"); const writeSkill = readFileSync(join(PLUGIN, "skills", "brains-write", "SKILL.md"), "utf8"); @@ -1280,7 +1291,7 @@ if (!claudeOnPath) { } } -assert(core.includes(""), "core marker must be v6"); +assert(core.includes(""), "core marker must be v7"); for (const signal of [ "Query brains reflexively", "list_calendar_events", @@ -1300,12 +1311,18 @@ for (const signal of [ // how "Capture is automatic … You do not need to call save_chat_session" survived here // while being false in two shipped configurations at once: // 1. no hooks at all — claude.ai web, on either install path; - // 2. hooks present but capture unconfigured — brains-turn.sh:43 exits early with no token, - // and that token is `required: false`. brains-start.sh:27 cats core.md with NO token - // check, so precisely those users are the ones who read the claim. - // Both axes get a signal: a rewrite dropping either one re-promises capture to a real user - // who is not getting it. - "only where a capture credential resolves", + // 2. hooks present but no credential — which used to be the silent case. + // Axis 2 changed shape rather than going away. The credential now comes from the client's own + // MCP sign-in, so it resolves for anyone whose brains tools work at all; and when it does not, + // the session-start hook says so in a `brains:capture` note. So the pinned signal moved from + // "assume nothing" to "the note is the answer when there is one" — and the un-promise stays, + // because with no note the model still cannot tell. + // The note is authoritative AND the agent must not invent a step. Two of the + // four states name a SETTING to change rather than a command to run, so an + // agent told to "run the command it names" when none is named can fabricate + // one — `claude mcp remove brains` — and execute a mutation on a real machine. + "is authoritative", + "never a command it did not name", "never promise capture and never deny it", "where the hooks don't run", // And the prohibition must stay NARROW. A blanket "do not call save_chat_session" suppressed @@ -1329,11 +1346,14 @@ for (const signal of [ // user. Verbatim pinning costs nothing here: editing core.md is already a deliberate act because it // forces the `v=` marker bump and both suites' marker assertions. const CORE_CAPTURE_REGION = [ - "**Capture.** In Codex and Claude Code the ingest hook saves each turn, but only", - "where a capture credential resolves — so never promise capture and never deny", - "it; `list_pages type=chat_session` is the only way to know. Don't call", - "`save_chat_session` routinely there; do call it when asked, and where the hooks", - "don't run (claude.ai web) it is the only path.", + "**Capture.** In Codex and Claude Code the ingest hook saves each turn, using the", + "same sign-in that authenticates these tools. A `brains:capture` note means it is", + "OFF and is authoritative: surface it once and, if the user agrees, do exactly the", + "one step it names — a command or a setting — never a command it did not name.", + "Without a note, never promise capture and never deny it; `list_pages", + "type=chat_session` is the only confirmation. Don't call `save_chat_session`", + "routinely there; do call it when asked, and where the hooks don't run (claude.ai", + "web) it is the only path.", ].join("\n"); // And the WHOLE injected body, verbatim, with the capture paragraph above composed into it rather // than pinned twice. brains-start.sh cats this file — all of it, not the capture paragraph — into @@ -1349,7 +1369,7 @@ const CORE_CAPTURE_REGION = [ // markers: text above the start marker or below the end marker is injected just the same. The marker // line is inside the pin, so the v= bump both suites assert stays part of the same edit. const CORE_BODY = [ - "", + "", "# brains — your memory layer", "", "You have a memory layer called **brains** (the `brains` MCP server). It holds the", @@ -1373,7 +1393,14 @@ const CORE_BODY = [ "page update time is not event time. Name the source page's `title` and `type`,", "and never invent slugs or IDs.", "", - CORE_CAPTURE_REGION, + "**Capture.** In Codex and Claude Code the ingest hook saves each turn, using the", + "same sign-in that authenticates these tools. A `brains:capture` note means it is", + "OFF and is authoritative: surface it once and, if the user agrees, do exactly the", + "one step it names — a command or a setting — never a command it did not name.", + "Without a note, never promise capture and never deny it; `list_pages", + "type=chat_session` is the only confirmation. Don't call `save_chat_session`", + "routinely there; do call it when asked, and where the hooks don't run (claude.ai", + "web) it is the only path.", "", "**The skills carry the detail** — load the one that fits the moment:", "`brains-read` (querying memory), `brains-write` (sending/creating via", @@ -2294,13 +2321,66 @@ assert( turnHook.includes('client:$client, client_type:"cli"'), "turn ingest payload must include the detected client and CLI type", ); +// The credential chain moved out of the turn hook into a shared resolver, because the two hooks +// had drifted: the turn hook scavenged a Codex MCP header and the inbox engine did not, so a Codex +// user with header auth had capture ON and the inbox OFF. Both now source the same file, and that +// is what these assertions hold — a second private chain reappearing in either hook is the +// regression, and it cannot be caught by looking at only one of them. +for (const [name, hook] of [["turn hook", turnHook], ["inbox engine", inboxHook]] as const) { + assert( + hook.includes("brains-credential.sh"), + `${name} must source the shared credential resolver, not build its own chain`, + ); + assert( + !/Authorization: Bearer/.test(hook), + `${name} must not attach an Authorization header itself — brains_request owns the credential, so no other code can send it to an unchecked host`, + ); +} +assert( + credLib.includes("codex mcp get"), + "resolver must still reuse a persisted Codex MCP Authorization header when no token env is present", +); +assert( + credLib.includes(".transport.http_headers.Authorization"), + "resolver must read the configured MCP Authorization header", +); +// Measured on codex-cli 0.147.0: with the plugin installed, `codex mcp get brains --json` reports +// `"http_headers": null`, because Codex stores its OAuth credential in the keychain instead. The +// header scavenge is therefore correct only for someone who ran `codex mcp add --header ...` by +// hand, and the keychain read is what covers everyone else. +assert( + credLib.includes("Codex MCP Credentials"), + "resolver must read the Codex MCP OAuth keychain — the header scavenge alone reaches almost nobody", +); assert( - turnHook.includes("codex mcp get brains --json"), - "Codex turn capture must reuse persisted MCP authentication when no token env is present", + credLib.includes("Claude Code-credentials"), + "resolver must read the Claude Code credential store", ); +// Selection is by ORIGIN, never by server name. This machine's Codex store holds three entries all +// named "brains" — two dead localhost stubs and one STAGE — so a name match picks arbitrarily +// between a token for a dead server and a stage token pointed at production. assert( - turnHook.includes(".transport.http_headers.Authorization"), - "Codex turn capture must read the configured MCP Authorization header", + credLib.includes("brains_origin"), + "resolver must select stored credentials by canonical origin", +); +// Neither store records WHICH ACCOUNT a token belongs to, so two candidates for one origin cannot +// be told apart. Capturing into the wrong brain is worse than a 401, because a 401 is detectable. +assert( + credLib.includes("ambiguous"), + "resolver must refuse when more than one distinct candidate matches, rather than guess", +); +// Transport truth. curl writes %{http_code} as soon as headers arrive, so a transfer that dies +// mid-body still reports 200 — and the truncated body can be valid JSON that parses. Measured: +// curl_exit=18, http_code=200, body {"device_id":"d1"}. Only the exit status separates them. +assert( + /crc=\$\?/.test(credLib) && credLib.includes('[ "$crc" -ne 0 ]'), + "brains_request must treat any non-zero curl exit as a transport failure regardless of HTTP code", +); +// A universal -o /dev/null would have silently killed device-id caching, drift nudges and the +// whole inbox delivery path while every status-code assertion stayed green. +assert( + !credLib.includes("-o /dev/null"), + "brains_request must preserve response bodies — the device report and inbox GET are parsed from them", ); // Exercise the standalone Codex path without a token env. The fake `codex` @@ -2312,13 +2392,19 @@ try { const bin = join(temp, "bin"); const capture = join(temp, "payloads.jsonl"); mkdirSync(bin); + // The fake `codex` now reports the transport URL as well as the header, because the real one + // does and because the resolver binds a discovered credential to that URL's origin. A header + // whose audience is unknown is not usable: without this field the token could be sent to a host + // that never issued it, which is exactly what the binding exists to prevent. writeFileSync( join(bin, "codex"), - '#!/bin/sh\nprintf \'%s\\n\' \'{"transport":{"http_headers":{"Authorization":"Bearer configured-token"}}}\'\n', + '#!/bin/sh\nprintf \'%s\\n\' \'{"transport":{"url":"' + CLAUDE_MCP_URL + '","http_headers":{"Authorization":"Bearer configured-token"}}}\'\n', ); + // Emits a status line last, the way `-w \'\\n%{http_code}\'` makes real curl behave, so the + // wrapper's parse is exercised rather than bypassed. writeFileSync( join(bin, "curl"), - '#!/bin/sh\n[ "${SLOW_CAPTURE:-}" = "1" ] && sleep 0.2\nprev=""\nfor arg in "$@"; do\n if [ "$prev" = "-d" ]; then printf \'%s\\n\' "$arg" >> "$CAPTURE_FILE"; fi\n case "$arg" in\n http://*|https://*) [ -n "${URL_FILE:-}" ] && printf \'%s\\n\' "$arg" >> "$URL_FILE" ;;\n esac\n prev="$arg"\ndone\n', + '#!/bin/sh\n[ "${SLOW_CAPTURE:-}" = "1" ] && sleep 0.2\nprev=""\nfor arg in "$@"; do\n if [ "$prev" = "-d" ]; then printf \'%s\\n\' "$arg" >> "$CAPTURE_FILE"; fi\n case "$arg" in\n http://*|https://*) [ -n "${URL_FILE:-}" ] && printf \'%s\\n\' "$arg" >> "$URL_FILE" ;;\n esac\n prev="$arg"\ndone\nprintf \'%s\\n%s\' "${FAKE_BODY:-}" "${FAKE_CODE:-200}"\n', ); chmodSync(join(bin, "codex"), 0o755); chmodSync(join(bin, "curl"), 0o755); @@ -2340,6 +2426,8 @@ try { CLAUDE_PLUGIN_OPTION_TOKEN: "", BRAINS_API_TOKEN: "", BRAINS_INBOX_TOKEN: "", + // Hermetic: never let a real developer credential resolve in a test. + BRAINS_CREDENTIAL_STORE_DISABLED: "1", BRAINS_STATE_DIR: join(temp, "state-core"), }, }); @@ -2358,6 +2446,11 @@ try { BRAINS_API_TOKEN: "", BRAINS_INBOX_TOKEN: "", CLAUDE_PLUGIN_OPTION_TOKEN: "", + // Hermetic without disabling discovery: these two scenarios EXERCISE the header + // scavenge, so the blanket switch would make them vacuous. Pointing the store reads at a + // path that does not exist keeps a real developer keychain out of them just as firmly. + BRAINS_CLAUDE_CREDENTIALS_FILE: join(temp, "no-such-store.json"), + BRAINS_CODEX_CREDENTIALS_FILE: join(temp, "no-such-store.json"), BRAINS_STATE_DIR: join(temp, "state"), CAPTURE_FILE: capture, }, @@ -2389,6 +2482,11 @@ try { BRAINS_API_TOKEN: "", BRAINS_INBOX_TOKEN: "", CLAUDE_PLUGIN_OPTION_TOKEN: "", + // Hermetic without disabling discovery: these two scenarios EXERCISE the header + // scavenge, so the blanket switch would make them vacuous. Pointing the store reads at a + // path that does not exist keeps a real developer keychain out of them just as firmly. + BRAINS_CLAUDE_CREDENTIALS_FILE: join(temp, "no-such-store.json"), + BRAINS_CODEX_CREDENTIALS_FILE: join(temp, "no-such-store.json"), BRAINS_STATE_DIR: join(temp, "state"), CAPTURE_FILE: assistantCapture, SLOW_CAPTURE: "1", @@ -2421,6 +2519,8 @@ try { BRAINS_API_TOKEN: "", BRAINS_INBOX_TOKEN: "", CLAUDE_PLUGIN_OPTION_TOKEN: "", + // Hermetic: never let a real developer credential resolve in a test. + BRAINS_CREDENTIAL_STORE_DISABLED: "1", BRAINS_STATE_DIR: join(temp, "state-no-token"), CAPTURE_FILE: noTokenCapture, }; @@ -2429,10 +2529,20 @@ try { env: noTokenEnv, }); assert(turnNoToken.status === 0, "turn hook must exit 0 with no token available"); + // "No capture" is no longer the same as "no output". The `now:` line is not capture — it tells + // the model what time it is — and it used to sit BEHIND the credential gate, so a user without a + // token silently lost accurate time injection as collateral damage. It now runs ahead of the + // gate, and the thing this asserts is what actually matters: nothing is sent and nothing errors. assert( - turnNoToken.stdout.toString() === "" && turnNoToken.stderr.toString() === "", - "turn hook must stay silent with no token available", + turnNoToken.stderr.toString() === "", + "turn hook must not write to stderr with no credential available", ); + for (const line of turnNoToken.stdout.toString().split("\n").filter((l) => l.trim() !== "")) { + assert( + /^/.test(line), + `turn hook must emit nothing but the time line with no credential available, got: ${line}`, + ); + } assert(!existsSync(noTokenCapture), "turn hook must not POST anything with no token available"); const inboxNoToken = spawnSync( @@ -2442,8 +2552,52 @@ try { ); assert(inboxNoToken.status === 0, "inbox engine must exit 0 with no token available"); assert( - inboxNoToken.stdout.toString() === "" && inboxNoToken.stderr.toString() === "", - "inbox engine must stay silent with no token available", + inboxNoToken.stderr.toString() === "", + "inbox engine must not write to stderr with no credential available", + ); + // Session start is the one place a tokenless user is told, and it must say so exactly once. + // Before this the state was completely unobservable: no request, no log line, no error, for + // eleven days on the machine that reported it. + const firstSignal = inboxNoToken.stdout.toString(); + assert( + firstSignal.includes(""), + "inbox engine must announce the off-state once at session start", + ); + // The remedy is PER PLATFORM, so the assertion has to be too. Codex reads its + // sign-in from the macOS keychain and Codex on Linux is not supported, so + // there the note must say so and name nothing — an assertion that accepted + // any string, or that only ever looked for "mcp login", would pass on the + // wrong text for one of the two platforms. + const codexKeychainHere = spawnSync("sh", ["-c", "command -v security"], { + env: noTokenEnv, + }).status === 0; + if (codexKeychainHere) { + assert( + firstSignal.includes("codex mcp login brains"), + "on a host with the macOS keychain the Codex off-state must name the sign-in that fixes it", + ); + assert( + !/not support/i.test(firstSignal), + "…and must not claim the platform is unsupported", + ); + } else { + assert( + /does not support Codex on this platform/.test(firstSignal), + "without the macOS keychain the Codex off-state must say the platform is unsupported", + ); + assert( + !/mcp login/.test(firstSignal) && !/BRAINS_API_TOKEN/.test(firstSignal), + "…and must offer no remedy, because there is no supported action to take", + ); + } + const secondSignal = spawnSync( + "bash", + [join(PLUGIN, "hooks", "lib", "brains-inbox.sh"), "startup", "codex-session-2"], + { env: noTokenEnv }, + ); + assert( + !secondSignal.stdout.toString().includes(""), + "the off-state signal must be claimed once per episode, not repeated every session", ); // The MCP URL is a literal now, so `userConfig.endpoint` governs capture and the inbox and @@ -2465,6 +2619,8 @@ try { CLAUDE_PLUGIN_OPTION_ENDPOINT: CLAUDE_SELF_HOSTED, BRAINS_API_TOKEN: "", BRAINS_INBOX_TOKEN: "", + // Hermetic: never let a real developer credential resolve in a test. + BRAINS_CREDENTIAL_STORE_DISABLED: "1", BRAINS_STATE_DIR: join(temp, "state-endpoint"), CAPTURE_FILE: endpointCapture, URL_FILE: endpointUrls, @@ -2515,6 +2671,8 @@ try { CLAUDE_PLUGIN_OPTION_TOKEN: "endpoint-probe-token", BRAINS_API_TOKEN: "", BRAINS_INBOX_TOKEN: "", + // Hermetic: never let a real developer credential resolve in a test. + BRAINS_CREDENTIAL_STORE_DISABLED: "1", BRAINS_STATE_DIR: join(temp, "state-default"), CAPTURE_FILE: defaultCapture, URL_FILE: defaultUrls, @@ -2549,6 +2707,8 @@ try { BRAINS_ENDPOINT: HOOK_ENV_ENDPOINT, BRAINS_API_TOKEN: "", BRAINS_INBOX_TOKEN: "", + // Hermetic: never let a real developer credential resolve in a test. + BRAINS_CREDENTIAL_STORE_DISABLED: "1", BRAINS_STATE_DIR: join(temp, "state-env"), CAPTURE_FILE: envCapture, URL_FILE: envUrls, From 208e39451415bff37d8bb1c7105204fc8a663154 Mon Sep 17 00:00:00 2001 From: olegshmuelov Date: Mon, 17 Aug 2026 16:33:40 +0300 Subject: [PATCH 2/2] fix(brains): close the signal-trap, selection and pin-strength gaps found in review [BRNS-CORE-085] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three reviewers read the resolver. Between them they found that the guards were thinner than the thing they guarded, in five distinct ways. Cleanup ran on INT/TERM/HUP as well as EXIT. A cleanup-only handler does not re-raise, so bash runs it and RESUMES — the process stops dying on that signal, and any handler the caller had is replaced outright. Once a credential resolved, the turn hook stopped responding to Ctrl-C for the rest of its run, on the path every working user takes on every turn. The existing test could not see it: its store read fails, so it never reached the config writer where the trap lived. That was the fourth instance of this mistake on one change, so it is now stated once as a rule — I3, cleanup on EXIT only, never on a signal — and enforced by a scan over every shipped file. The rule found two more instances in the hooks that nobody had reported. Raising the document ceiling had quietly traded a fast reject for a slow accept. The selection loop spent two forks on every entry, one recomputing the field separator and one canonicalising an origin, so a store the wider ceiling exists to admit cost seconds per hook run, twice a turn. The separator is hoisted, the fork-free predicate is tested first, and the loop now consults the total budget this file documents as bounding the whole resolution but only ever applied to one path. Measured on the full hook: 3.97s -> 0.63s at 1450 entries, 2.89s -> 0.57s at 1000, identical answers at both. The Codex array backend deleted the snapshot every row shares on its first non-matching row, because the guard was keyed on the client rather than on shared-ness; selection then found its one credential and the config writer read a deleted file. Keyed on shared-ness directly, both orders of a mixed array resolve. The Codex-on-Linux note claimed there was nothing to change, which is measurably false — an explicit token resolves and captures there — so a note with no action now has its own shape instead of borrowing the offer protocol and telling the agent to offer something that does not exist. The suite itself was non-deterministic under load, and that turned out to be one bug rather than the flakiness and the containment gap it looked like: fixed stub ports meant a second run talked to the first run's server and its receipts landed in a log nobody was reading, which let an assertion that no credential crossed origins pass while one did. Ports are chosen by the kernel now. Three concurrent runs went from two to four failures each to none. Coverage was the larger problem. Behavioural claims about the shipped shell were pinned as substrings, which a comment can satisfy — three already were, including both guards against the two hooks regrowing separate credential chains, carried by a `# shellcheck source=` directive. Those are code-only region pins now, with a self-scan that fails if the weaker form reappears. And the mutation gate proved a hand-picked list of mechanisms load-bearing while being incomplete by construction: the server-identity guard could be replaced with `return 0` with every suite green while another MCP server's bearer was selected and posted to the ingest endpoint, and the capture log line — the observability this whole change is justified by — was deletable without a single test noticing. scripts/mutation-coverage.sh derives the candidate set from the diff instead of from a list: it neuters each changed line and requires something to fail. Of 485 independently mutable lines it found 227 survivors, which is how the session-start housekeeping, the `now:` injection, the off-state health note and the Codex synchronous ingest turned out to have no coverage at all. It is not part of the per-commit gate; one suite run per line is an hour, not seconds. BRNS-CORE-085 --- README.md | 8 +- plugins/brains/hooks/brains-turn.sh | 2 +- plugins/brains/hooks/lib/brains-credential.sh | 159 +++- plugins/brains/hooks/lib/brains-inbox.sh | 2 +- scripts/mutation-coverage.sh | 173 ++++ tests/credential/run.ts | 784 ++++++++++++++++-- tests/credential/stubs.js | 56 +- tests/plugin-contract/run.ts | 288 +++++-- 8 files changed, 1295 insertions(+), 177 deletions(-) create mode 100755 scripts/mutation-coverage.sh diff --git a/README.md b/README.md index 5358a15..6d6b744 100644 --- a/README.md +++ b/README.md @@ -39,9 +39,11 @@ plugin loads, open `/hooks` and trust the bundled brains hooks — that is what runs automatic recall and error feedback. Capture and inbox delivery use the sign-in above as their credential, so there is nothing further to set. -Capture and the inbox are **macOS only** for Codex: they read the credential -from the macOS keychain, and Codex on Linux is not a supported configuration. -The tools and recall still work there; capture and inbox delivery do not. +Capture and the inbox are **macOS only** for Codex: they read the sign-in from +the macOS keychain, so on Linux there is none for them to read and they stay +off. The tools and recall are unaffected. The optional capture token below does +drive capture there, but Codex on Linux is not a configuration brains supports +or tests. Everyday reading and writing is covered by default. For admin-gated tools or performance insights, sign in asking for them explicitly (both also need the diff --git a/plugins/brains/hooks/brains-turn.sh b/plugins/brains/hooks/brains-turn.sh index dc6267b..5228fec 100755 --- a/plugins/brains/hooks/brains-turn.sh +++ b/plugins/brains/hooks/brains-turn.sh @@ -141,7 +141,7 @@ ingest() { # role, content # unauthenticated. _lease=$(brains_cred_lease) || return 0 ( BRAINS_CRED_CONFIG="$_lease" - trap 'brains_cred_return "$_lease"' EXIT INT TERM HUP + trap 'brains_cred_return "$_lease"' EXIT ingest_once "$role" "$payload" brains_cred_return "$_lease" ) & fi diff --git a/plugins/brains/hooks/lib/brains-credential.sh b/plugins/brains/hooks/lib/brains-credential.sh index 6f7b9e4..b98036e 100644 --- a/plugins/brains/hooks/lib/brains-credential.sh +++ b/plugins/brains/hooks/lib/brains-credential.sh @@ -33,6 +33,17 @@ # enumeration may downgrade it to no-credential. A future failure mode # nobody has thought of fails safe without being enumerated. # +# I3. CLEANUP RUNS ON EXIT ONLY — NEVER ON A SIGNAL. +# A cleanup-only INT/TERM/HUP handler does not re-raise, so bash runs it +# and RESUMES: the process stops dying on that signal, and a caller's own +# handler never fires. In a sourced library that redefines Ctrl-C for the +# whole hook, on the path every working user takes on every turn. Four +# separate instances of this were written on one change, so it is stated +# once as a rule and enforced by a test over the shipped files rather than +# fixed a fifth time. Whatever a signal-killed shell leaves behind is +# brains_cred_prune_tmp's job — the story SIGKILL, which is untrappable, +# already needed. +# # Beyond those: only brains_request() ever presents the credential, because # endpoint URLs are env-overridable and a discovered credential must never reach # a host that did not mint it. And every failure returns "no credential" with @@ -213,7 +224,8 @@ _brains_bounded_read_file() { # deadline, max-bytes, max-blocks, command... # version saved, restored and re-raised in the PARENT — which could never # work, because every caller runs this inside a command substitution where # trap changes are discarded and `kill $$` targets the wrong shell. - trap 'rm -rf "$dir" 2>/dev/null' EXIT INT TERM HUP + # EXIT only, per I3. + trap 'rm -rf "$dir" 2>/dev/null' EXIT set -m ( ulimit -f "$maxblocks" 2>/dev/null "$@" >"$dir/v" 2>/dev/null @@ -229,7 +241,7 @@ _brains_bounded_read_file() { # deadline, max-bytes, max-blocks, command... # descendant behind, and the watchdog is the only thing that would reach it. kill -TERM -"$_p" 2>/dev/null kill $_w 2>/dev/null; wait $_w 2>/dev/null - trap - EXIT INT TERM HUP + trap - EXIT ) >/dev/null 2>&1 rc="" [ -f "$dir/rc" ] && read -r rc <"$dir/rc" 2>/dev/null @@ -543,12 +555,21 @@ _brains_write_config() { # source-kind, locator, store-file(optional) # installed when the sourcing hook has no EXIT trap of its own — replacing a # caller's handler is not this library's call to make. None of the hooks set # one; if that changes, the caller must invoke brains_cred_release itself. + # + # EXIT and nothing else, per I3. This ran on every turn of every working user, + # so covering INT here made the turn hook ignore Ctrl-C for the rest of its + # run: the handler cleaned up and bash resumed. A signal-killed hook leaves + # the directory to brains_cred_prune_tmp instead. [ -z "$(trap -p EXIT 2>/dev/null)" ] && - trap 'rm -rf "$BRAINS_CRED_TMP/r.$$.cfg" 2>/dev/null' EXIT INT TERM HUP + trap 'rm -rf "$BRAINS_CRED_TMP/r.$$.cfg" 2>/dev/null' EXIT return 0 } -brains_cred_release() { [ -n "$BRAINS_CRED_CONFIG" ] && rm -rf "$(dirname "$BRAINS_CRED_CONFIG")" 2>/dev/null; BRAINS_CRED_CONFIG=""; return 0; } +# Both of these delete a directory derived from a path, which is the shape that +# twice reached outside the owned root, so both go through the confined +# primitive rather than calling rm themselves. Their arguments are internal +# today; the point is that they stay safe when that stops being true. +brains_cred_release() { _brains_discard "$BRAINS_CRED_CONFIG"; BRAINS_CRED_CONFIG=""; return 0; } # A private copy of the config for a request that will outlive this shell. # @@ -571,7 +592,7 @@ brains_cred_lease() { printf '%s' "$dir/curl.conf" } -brains_cred_return() { [ -n "${1:-}" ] && rm -rf "$(dirname "$1")" 2>/dev/null; return 0; } +brains_cred_return() { _brains_discard "${1:-}"; return 0; } # --------------------------------------------------------------- the resolver # Sets BRAINS_CRED_* and returns 0 when a credential resolved, 1 otherwise. @@ -588,7 +609,7 @@ brains_resolve_credential() { _brains_resolve_credential_impl() { local base want store meta line key url name corigin accts acct seen rec recfile - local matched matchedfiles count + local matched matchedfiles count tab mf base="${1:-}" BRAINS_CRED_STATE="indeterminate" # I2: downgrade only on proof BRAINS_CRED_SOURCE=""; BRAINS_CRED_BINDING=""; BRAINS_CRED_ORIGIN="" @@ -720,16 +741,37 @@ EOF2 fi matchedfiles="" - while IFS=$(printf '\t') read -r key url name recfile; do + # Two forks per entry lived in this loop, and the document ceiling that stopped + # rejecting large stores turned that into seconds of hot path: a thousand-entry + # store cost 4.6s per hook run, twice a turn. `IFS=$(printf '\t')` re-forks a + # command substitution on every iteration, so the separator is computed once; + # and `brains_origin` is a command substitution too, so the fork-free predicate + # is tested FIRST and the origin is only canonicalised for a row that could + # still win. Same answers, 2.85s -> 0.25s at 1450 entries. + tab=$(printf '\t') + while IFS="$tab" read -r key url name recfile; do [ -n "$key" ] || continue BRAINS_CRED_SAW_ENTRIES=1 - if ! corigin=$(brains_origin "$url") || - [ "$corigin" != "$want" ] || - ! _brains_is_brains_server "$name"; then - # Codex only: on the Claude path every row names the SAME store snapshot, - # so discarding here on a non-match would delete the document the matching - # rows still need. - [ "${BRAINS_CRED_CLIENT:-claude}" = "codex" ] && _brains_discard "$recfile" + # The total budget is documented as bounding the whole resolution, but until + # now it was consulted on the Codex enumeration path only — leaving the very + # loop the wider ceiling grew unbounded. Draining rather than breaking, so + # the Codex arm's private record files are still discarded here rather than + # left for the prune. + if ! _brains_budget_left; then + BRAINS_CRED_TRUNCATED=1 + [ "$recfile" != "${store:-}" ] && _brains_discard "$recfile" + continue + fi + if ! _brains_is_brains_server "$name" || + ! corigin=$(brains_origin "$url") || + [ "$corigin" != "$want" ]; then + # Only a record file PRIVATE to this row may be discarded here. The Claude + # store and the Codex array backend both name one shared snapshot on every + # row, and deleting that on the first non-matching row destroys the + # document the winner still needs. Keyed on shared-ness itself rather than + # on the client, because keying it on the client is what let the array + # backend reintroduce the bug after the Claude path was fixed. + [ "$recfile" != "${store:-}" ] && _brains_discard "$recfile" continue fi matched="$matched$key @@ -754,9 +796,22 @@ EOF2 | [ $keys[] as $i | $a[$i].token_response.access_token ] | unique | length' \ "$store" 2>/dev/null) elif [ "${BRAINS_CRED_CLIENT:-claude}" = "codex" ]; then - # Keychain backend: one private record file per account. - count=$(printf '%s' "$matchedfiles" | tr '\n' '\0' | xargs -0 -n 200 \ - jq -s '[.[].token_response.access_token] | unique | length' 2>/dev/null | tail -1) + # Keychain backend: one private record file per account, and + # BRAINS_CRED_MAX_CANDIDATES caps that at eight. The earlier version piped + # the list through `xargs -0 -n 200 ... | tail -1`, which could never batch + # at that cap and would have reported the LAST batch's count rather than + # the union of all of them if it ever did — reading two aliases of one + # bearer as one credential in one batch and as an ambiguity in another. So + # the list becomes one argument vector, which at eight entries cannot + # split. The positional parameters are free here: $1 was read into `base` + # at the top and is not used again. + set -- + while IFS= read -r mf; do + [ -n "$mf" ] && set -- "$@" "$mf" + done </dev/null) else count=$(jq -r --argjson keys "$(printf '%s' "$matched" | jq -Rs 'split("\n") | map(select(length>0))')" \ '[ $keys[] as $k | .mcpOAuth[$k].accessToken ] | unique | length' "$store" 2>/dev/null) @@ -768,18 +823,19 @@ EOF2 # Metadata pass files are no longer needed once counting is done; the winner's # is kept just long enough to generate the config below. - # Codex only: there the retained files are one private record per account. On - # the Claude path the same column is the single store snapshot, which is - # discarded on its own — sweeping it here would delete the document out from - # under config generation. + # + # Same shared-ness rule as the selection loop: a row naming the shared store + # snapshot is skipped, because that document is discarded once on its own and + # sweeping it here would pull it out from under config generation. The path + # confinement that used to be repeated here now lives in _brains_discard. _brains_discard_matched() { local f keep - [ "${BRAINS_CRED_CLIENT:-claude}" = "codex" ] || return 0 keep="${1:-}" while IFS= read -r f; do [ -n "$f" ] || continue [ "$f" = "$keep" ] && continue - case "$f" in "$BRAINS_CRED_TMP"/*) _brains_discard "$f" ;; esac + [ "$f" = "${store:-}" ] && continue + _brains_discard "$f" done </dev/null | cut -c1-12) - [ -n "$BRAINS_HEALTH_NS" ] || BRAINS_HEALTH_NS="default" - _brains_health_ns_cache_key="$url" - printf '%s' "$BRAINS_HEALTH_NS" + ns=$(printf '%s' "$key" | shasum -a 256 2>/dev/null | cut -c1-12) + [ -n "$ns" ] || ns="default" + printf '%s' "$ns" } _brains_health_dir() { # capability, url @@ -977,10 +1037,16 @@ brains_health_claim_signal() { # key, url # that cries wolf on flaky wifi is how people learn to ignore the one that # matters. # Codex reads its MCP sign-in from the macOS keychain, and Codex on Linux is not -# a supported configuration. So on that platform there is no step to name: the -# hooks cannot reach the sign-in, and pointing the user at a token would be -# documenting a path the product does not support. Echoes an empty string, and -# the caller drops the remedy clause rather than inventing one. +# a supported configuration. So on that platform there is no SIGN-IN step to +# name and _brains_signin_step echoes nothing, leaving the caller to drop the +# remedy clause rather than invent one. +# +# What that does NOT mean is that nothing works there. The explicit-token branch +# is platform-independent — BRAINS_API_TOKEN resolves and captures on Linux +# exactly as it does on macOS — so a note claiming there is "nothing to change" +# was measurably false, and core.md tells the agent these notes are +# authoritative. The product decision stands; the wording now matches what the +# code does. _brains_codex_unsupported_here() { [ "${BRAINS_CRED_CLIENT:-claude}" = "codex" ] && ! command -v security >/dev/null 2>&1 } @@ -994,12 +1060,25 @@ _brains_signin_step() { fi } +# Two shapes, and the difference is whether an ACTION is attached. +# +# _brains_emit_signal carries a remedy, so it ends with the offer protocol. A +# note with no remedy must not: telling the agent there is nothing to do and +# then instructing it to "offer this and act if they say yes" is a contradiction +# it has to resolve on its own, and the way an agent resolves that is by +# inventing a command to offer. So a stateless note gets its own tail. _brains_emit_signal() { # key, url, label, remedy brains_health_claim_signal "$1" "$2" || return 1 printf '%s\n' ''"$3"' is OFF: '"$4"' Offer this to the user once, in one line, and act only if they say yes. Do not repeat it later in the session.' return 0 } +_brains_emit_note() { # key, url, label, explanation + brains_health_claim_signal "$1" "$2" || return 1 + printf '%s\n' ''"$3"' is OFF: '"$4"' There is nothing for you to offer here. Say it once if it is relevant and do not repeat it later in the session.' + return 0 +} + brains_capture_signal() { local cap url state step label step=$(_brains_signin_step) @@ -1012,8 +1091,8 @@ brains_capture_signal() { case "$state" in no-credential) if _brains_codex_unsupported_here; then - _brains_emit_signal no-credential "$url" 'Conversation capture and the brains inbox' \ - "brains does not support Codex on this platform, so there is nothing to turn on and nothing to change. Mention it once if it is relevant and do not offer a fix." && return 0 + _brains_emit_note no-credential "$url" 'Conversation capture and the brains inbox' \ + "brains reads the Codex sign-in from the macOS keychain, and there is none to read on this platform. An explicit \`BRAINS_API_TOKEN\` does turn capture on here, but Codex on this platform is not a configuration brains supports, so brains does not ask you to set one." && return 0 return 1 fi _brains_emit_signal no-credential "$url" 'Conversation capture and the brains inbox' \ diff --git a/plugins/brains/hooks/lib/brains-inbox.sh b/plugins/brains/hooks/lib/brains-inbox.sh index d0d84c9..614c7aa 100755 --- a/plugins/brains/hooks/lib/brains-inbox.sh +++ b/plugins/brains/hooks/lib/brains-inbox.sh @@ -323,7 +323,7 @@ if [ "${#AUTO_ACK_IDS[@]}" -gt 0 ] || [ "$CTX_ITEMS_LEN" -gt 0 ]; then ack_lease=$(brains_cred_lease) || ack_lease="" if [ -n "$ack_lease" ]; then ( BRAINS_CRED_CONFIG="$ack_lease" - trap 'brains_cred_return "$ack_lease"' EXIT INT TERM HUP + trap 'brains_cred_return "$ack_lease"' EXIT brains_request ack "$ACK_ENDPOINT" --max-time 3 -X POST \ -H "Content-Type: application/json" -d "$ack" >/dev/null 2>&1 brains_cred_return "$ack_lease" ) & diff --git a/scripts/mutation-coverage.sh b/scripts/mutation-coverage.sh new file mode 100755 index 0000000..c3b5087 --- /dev/null +++ b/scripts/mutation-coverage.sh @@ -0,0 +1,173 @@ +#!/usr/bin/env bash +# Mutation coverage over the shipped shell a branch changes. +# +# WHY THIS EXISTS. tests/credential/run.ts carries a mutation gate that proves a +# hand-picked list of mechanisms is load-bearing. That list was demonstrably +# incomplete: two independent readers each found mechanisms outside it that +# could be deleted outright with every suite still green — a server-identity +# guard whose removal let another MCP server's bearer be selected, and the only +# log line the change is justified by. A hand-maintained registry cannot answer +# "what did we miss", because whatever it misses is by definition not on it. +# +# So this derives the list from the DIFF. For every code line the branch adds or +# changes in the shipped hooks, it neuters that line and runs the behavioural +# suites. A line whose removal changes no observable behaviour is either +# unguarded or not load-bearing, and both answers are worth having. +# +# WHAT IT DOES NOT COVER, stated rather than implied: +# * Lines that cannot be neutered on their own — `fi`, `done`, a `case` +# pattern, a loop header. Replacing one with `:` is a syntax error, so it is +# reported as STRUCTURAL and counted, never silently dropped. +# * tests/plugin-contract/run.ts is deliberately NOT run. Its region pins are +# verbatim source text, so they kill almost every mutant regardless of +# whether anything observes the behaviour. Counting them would report full +# coverage for a suite that never executes the code. They are a drift guard, +# which is a different job. +# +# It is NOT part of the per-commit gate: one suite run per line is minutes to +# hours. Run it before asking for review on a change to the hooks. +# +# Usage: scripts/mutation-coverage.sh [base-ref] [jobs] +# scripts/mutation-coverage.sh --list [base-ref] # just the mutant set +set -u + +ROOT=$(cd "$(dirname "$0")/.." && pwd) +LIST_ONLY=0 +if [ "${1:-}" = "--list" ]; then LIST_ONLY=1; shift; fi +BASE="${1:-origin/main}" +JOBS="${2:-4}" + +SHIPPED='plugins/brains/hooks/*.sh plugins/brains/hooks/lib/*.sh' +WORK=$(mktemp -d "${TMPDIR:-/tmp}/brains-mutcov.XXXXXX") || exit 1 +# EXIT only — the same rule the library states as I3. A signal-killed run leaves +# a directory under TMPDIR, which is what TMPDIR is for. +trap 'rm -rf "$WORK" 2>/dev/null' EXIT + +# ---- 1. every code line this branch adds or changes in the shipped hooks ----- +# -U0 so there is no context to mistake for a change. The awk tracks the NEW +# file's line numbers across hunks. +# shellcheck disable=SC2086 +git -C "$ROOT" diff -U0 "$BASE" -- $SHIPPED | + awk ' + /^\+\+\+ b\// { file = substr($0, 7); next } + /^\+\+\+ / { file = ""; next } + /^@@/ { + split($3, h, ",") + ln = substr(h[1], 2) + 0 + next + } + /^\+/ { if (file != "") { print file "\t" ln }; ln++; next } + ' > "$WORK/added.tsv" + +# Comments and blank lines are not mechanisms. Everything else is a candidate. +: > "$WORK/candidates.tsv" +while IFS=$(printf '\t') read -r file ln; do + [ -n "$file" ] || continue + text=$(awk -v n="$ln" 'NR==n{print; exit}' "$ROOT/$file") + case "$text" in + ''|[[:space:]]*'#'*|'#'*) continue ;; + esac + case "$text" in *[![:space:]]*) ;; *) continue ;; esac + printf '%s\t%s\t%s\n' "$file" "$ln" "$text" >> "$WORK/candidates.tsv" +done < "$WORK/added.tsv" + +# ---- 2. drop the ones that cannot stand alone ------------------------------ +# A line replaced by `:` that no longer parses is part of a construct, not a +# mechanism of its own. Counted and reported, never silently skipped. +: > "$WORK/mutable.tsv" +: > "$WORK/structural.tsv" +while IFS=$(printf '\t') read -r file ln text; do + [ -n "$file" ] || continue + probe="$WORK/probe.sh" + awk -v n="$ln" 'NR==n { match($0, /^[ \t]*/); printf "%s:\n", substr($0, 1, RLENGTH); next } { print }' \ + "$ROOT/$file" > "$probe" + if bash -n "$probe" 2>/dev/null; then + printf '%s\t%s\t%s\n' "$file" "$ln" "$text" >> "$WORK/mutable.tsv" + else + printf '%s\t%s\t%s\n' "$file" "$ln" "$text" >> "$WORK/structural.tsv" + fi +done < "$WORK/candidates.tsv" + +n_cand=$(wc -l < "$WORK/candidates.tsv" | tr -d ' ') +n_mut=$(wc -l < "$WORK/mutable.tsv" | tr -d ' ') +n_str=$(wc -l < "$WORK/structural.tsv" | tr -d ' ') +printf 'mutation coverage vs %s\n' "$BASE" +printf ' %s changed code lines, %s independently mutable, %s structural (part of a construct)\n' \ + "$n_cand" "$n_mut" "$n_str" + +if [ "$LIST_ONLY" = "1" ]; then + printf '\n--- mutable ---\n' + cat "$WORK/mutable.tsv" + printf '\n--- structural (not covered by this tool) ---\n' + cat "$WORK/structural.tsv" + exit 0 +fi + +# ---- 3. one mutant per line, in parallel ------------------------------------ +# Parallel is safe because the suites bind ephemeral ports. It was not before: +# fixed ports meant two runs shared one stub server, and receipts for one run +# landed in the other's log. +cat > "$WORK/one.sh" <<'WORKER' +#!/usr/bin/env bash +set -u +ROOT="$1"; WORK="$2"; id="$3"; file="$4"; ln="$5" +dir="$WORK/m.$id" +mkdir -p "$dir" || exit 1 +cp -R "$ROOT/plugins" "$ROOT/tests" "$dir/" 2>/dev/null || exit 1 +awk -v n="$ln" 'NR==n { match($0, /^[ \t]*/); printf "%s:\n", substr($0, 1, RLENGTH); next } { print }' \ + "$ROOT/$file" > "$dir/$file" +# The in-suite gate is neutered IN THE COPY. It mutates the tree itself, so +# running it inside a mutant is a mutation of a mutation, and its verdicts say +# nothing about the line under test — while costing a third of the runtime. The +# anchor is asserted rather than assumed: a silent miss here would quietly turn +# every result into a slower version of the same answer. +gate="$dir/tests/credential/run.ts" +anchor=' for (const m of MUTATIONS) {' +grep -qF "$anchor" "$gate" || { echo "GATE ANCHOR MISS — update scripts/mutation-coverage.sh" >&2; exit 2; } +awk -v a="$anchor" '{ if (index($0, a)) print " for (const m of MUTATIONS.slice(0, 0)) {"; else print }' \ + "$gate" > "$gate.tmp" && mv "$gate.tmp" "$gate" +# Fail fast: a mutant only has to be killed once, and most are killed early. +out=$(cd "$dir" && BRAINS_FAIL_FAST=1 bun run tests/credential/run.ts 2>&1) +killers=$(printf '%s' "$out" | grep ' FAIL' | sed 's/^ *FAIL //; s/ — .*//') +if [ -z "$killers" ]; then + out2=$(cd "$dir" && BRAINS_FAIL_FAST=1 bun run tests/inbox-v2/run.ts 2>&1; \ + cd "$dir" && BRAINS_FAIL_FAST=1 bun run tests/tool-error/run.ts 2>&1) + killers=$(printf '%s' "$out2" | grep -iE '^\s*(FAIL|✗)' | head -3) +fi +rm -rf "$dir" 2>/dev/null +if [ -z "$killers" ]; then + printf 'SURVIVED\t%s\t%s\n' "$file" "$ln" >> "$WORK/results.tsv" +else + printf 'killed\t%s\t%s\t%s\n' "$file" "$ln" "$(printf '%s' "$killers" | head -1)" >> "$WORK/results.tsv" +fi +WORKER +chmod +x "$WORK/one.sh" + +: > "$WORK/results.tsv" +i=0 +: > "$WORK/jobs.txt" +while IFS=$(printf '\t') read -r file ln text; do + [ -n "$file" ] || continue + i=$((i + 1)) + printf '%s\t%s\t%s\n' "$i" "$file" "$ln" >> "$WORK/jobs.txt" +done < "$WORK/mutable.tsv" + +printf ' running %s mutants at %s-way parallelism...\n' "$n_mut" "$JOBS" +# shellcheck disable=SC2016 +tr '\t' '\n' < "$WORK/jobs.txt" | xargs -P "$JOBS" -n 3 "$WORK/one.sh" "$ROOT" "$WORK" + +# ---- 4. report --------------------------------------------------------------- +survivors=$(grep -c '^SURVIVED' "$WORK/results.tsv" 2>/dev/null | tr -d ' ') +[ -n "$survivors" ] || survivors=0 +printf '\n %s killed, %s SURVIVED\n' "$(grep -c '^killed' "$WORK/results.tsv" | tr -d ' ')" "$survivors" +if [ "$survivors" != "0" ]; then + printf '\n survivors — removing each of these changed nothing any suite can see:\n' + while IFS=$(printf '\t') read -r verdict file ln _; do + [ "$verdict" = "SURVIVED" ] || continue + printf ' %s:%s %s\n' "$file" "$ln" "$(awk -v n="$ln" 'NR==n{print; exit}' "$ROOT/$file" | sed 's/^[[:space:]]*//')" + done < "$WORK/results.tsv" +fi +if [ "$n_str" != "0" ]; then + printf '\n (%s structural lines were not mutated — see the header for why)\n' "$n_str" +fi +[ "$survivors" = "0" ] diff --git a/tests/credential/run.ts b/tests/credential/run.ts index 6cb3bff..fa7c116 100644 --- a/tests/credential/run.ts +++ b/tests/credential/run.ts @@ -31,6 +31,12 @@ let failed = 0; // Written to stderr so progress survives a hang: stdout is buffered when it is a // pipe, and a suite that stalls would otherwise report nothing at all about where. const say = (line: string) => process.stderr.write(`${line}\n`); +// BRAINS_FAIL_FAST stops at the first failure. It exists for +// scripts/mutation-coverage.sh, which runs this suite once per mutated line and +// only needs to know THAT a mutant was killed. It can only make the suite +// stricter — a fail-fast run that reaches the end is exactly as green as a +// normal one — so there is no way to use it to hide a failure. +const FAIL_FAST = process.env.BRAINS_FAIL_FAST === "1"; function check(name: string, condition: unknown, detail = ""): void { if (condition) { passed++; @@ -38,6 +44,12 @@ function check(name: string, condition: unknown, detail = ""): void { } else { failed++; say(` FAIL ${name}${detail ? ` — ${detail}` : ""}`); + if (FAIL_FAST) { + stubProc?.kill(); + rmSync(temp, { recursive: true, force: true }); + say(`\n${passed} passed, ${failed} failed (stopped at first failure)`); + process.exit(1); + } } } function section(title: string): void { @@ -84,30 +96,45 @@ function sh(script: string, opts: ShellOpts = {}): { stdout: string; stderr: str // Served from a child process — see the header of stubs.js for why that is // required rather than tidy. Receipts come back through a log file, which is // also what lets an assertion be made about what a server DID NOT receive. -const PRIMARY = 8991; -const OTHER = 8992; -const TRUNCATOR = 8993; +// +// The ports are chosen by the KERNEL, not by this file. Fixed ports are what +// made this suite flaky: a previous run's stubs stay bound for a moment after +// it exits, so a new run's servers failed to bind while its readiness probe was +// still being answered — by the old process, writing into the old run's receipt +// log. Assertions about what arrived then read zero, in a suite that was +// otherwise correct. Two runs at once did the same thing, which is all a CI +// runner has to do to reproduce it. const HITS = join(temp, "hits.jsonl"); +const PORTS_FILE = join(temp, "stub-ports.json"); writeFileSync(HITS, ""); const stubProc = spawn( process.execPath, - [join(import.meta.dir, "stubs.js"), HITS, String(PRIMARY), String(OTHER), String(TRUNCATOR)], + [join(import.meta.dir, "stubs.js"), HITS, PORTS_FILE], { stdio: "ignore" }, ); +let PRIMARY = 0, OTHER = 0, TRUNCATOR = 0, BLACKHOLE = 0; { - // Wait for the stubs to actually listen. Racing the first request against - // startup yields a connection refusal indistinguishable from a real one. - const deadline = Date.now() + 10000; + // Wait for the stubs to publish their ports AND to answer. Racing the first + // request against startup yields a connection refusal indistinguishable from + // a real one. + const deadline = Date.now() + 15000; let up = false; while (Date.now() < deadline) { - if (spawnSync("curl", ["-s", "-o", "/dev/null", "-m", "1", `http://127.0.0.1:${PRIMARY}/ping`]).status === 0) { - up = true; - break; + if (existsSync(PORTS_FILE)) { + try { + const p = JSON.parse(readFileSync(PORTS_FILE, "utf8")); + if (spawnSync("curl", ["-s", "-o", "/dev/null", "-m", "1", `http://127.0.0.1:${p.primary}/ping`]).status === 0) { + PRIMARY = p.primary; OTHER = p.other; TRUNCATOR = p.truncator; BLACKHOLE = p.blackhole; + up = true; + break; + } + } catch { /* half-written or not yet answering; poll again */ } } spawnSync("sleep", ["0.1"]); } if (!up) { say("stub servers never came up"); + stubProc.kill(); process.exit(1); } writeFileSync(HITS, ""); @@ -187,6 +214,51 @@ const twoAliases = fixture("two-aliases", { check("two aliases sharing one token collapse and resolve", resolveWith(twoAliases).stdout.startsWith("ok|claude-oauth|"), resolveWith(twoAliases).stdout); +// Origin is NOT sufficient on its own, and until now nothing here said so. Every +// origin-matching fixture in this file named a brains server, and the only entry +// named something else lived on a different host — so it was refused by ORIGIN +// and the server-identity guard was never the thing doing the refusing. +// Replacing that whole function with `return 0` left the suite green while the +// resolver selected another MCP server's bearer and wrote it into the curl +// config bound for /ingest/claude. +const foreignServer = fixture("foreign-server", { + mcpOAuth: { + "plugin:github:github|h": { + accessToken: "FOREIGN-SERVER-TOKEN", serverUrl: `${ORIGIN}/mcp`, serverName: "plugin:github:github", + }, + }, +}); +check("a foreign MCP server at the SAME origin is refused, not selected", + resolveWith(foreignServer).stdout === "blocked|||0", resolveWith(foreignServer).stdout); +// ...and the same store with a real brains entry beside it must still resolve to +// exactly one credential: the guard has to reject the neighbour without +// rejecting the tenant, which "refuse everything" would also satisfy. +const foreignPlusBrains = fixture("foreign-plus-brains", { + mcpOAuth: { + "plugin:github:github|h": { + accessToken: "FOREIGN-SERVER-TOKEN", serverUrl: `${ORIGIN}/mcp`, serverName: "plugin:github:github", + }, + "plugin:brains:brains|h": { + accessToken: "tok-ours", serverUrl: `${ORIGIN}/mcp`, serverName: "plugin:brains:brains", + }, + }, +}); +check("a brains entry beside a foreign one resolves to exactly one credential", + resolveWith(foreignPlusBrains).stdout === "ok|claude-oauth|plugin:brains:brains|h|1", + resolveWith(foreignPlusBrains).stdout); +// Asserted on the wire as well as on the record: the point is not which label the +// resolver prints, it is which bearer leaves the machine. +{ + const state = freshState(); + const mark = readHits().length; + sh(`brains_resolve_credential ${JSON.stringify(ORIGIN)} || exit 1\n` + + `brains_request ingest "${ORIGIN}/ingest/claude" -X POST -d '{}'`, + { state, env: { BRAINS_CLAUDE_CREDENTIALS_FILE: foreignPlusBrains } }); + const sent = readHits().slice(mark).filter((h) => h.path === "/ingest/claude").map((h) => h.auth); + check("and the foreign bearer never reaches the endpoint", + sent.length === 1 && sent[0] === "Bearer tok-ours", sent.join(",") || "(no request)"); +} + // A stage credential must never be sent to production. Name matching would pick this one. const stageOnly = storeFor("https://brains-mcp.stage.example.com/mcp", "tok-stage"); check("a store with credentials but none for this origin is blocked, not absent", @@ -212,14 +284,58 @@ section("profile isolation — a profile with no credential must not read anothe // There is no keychain in this test, so the file backend stands in for the same rule: the store // path is derived from the configured profile and nothing else is tried. { - const profile = join(temp, "isolated-profile"); - mkdirSync(profile, { recursive: true }); - // The default profile's store exists and is valid; the isolated profile's does not. - const r = sh( + // The default profile's store is PLANTED here, and it holds a credential FOR + // THIS ORIGIN. Without both of those the assertion was satisfied by any + // non-resolution at all: a resolver that really did fall back to the default + // profile still answered "none", because a developer's own store has nothing + // for a 127.0.0.1 stub. The check could not observe the regression it names. + const homeDir = join(temp, "profile-home"); + mkdirSync(join(homeDir, ".claude"), { recursive: true }); + writeFileSync(join(homeDir, ".claude", ".credentials.json"), JSON.stringify({ + mcpOAuth: { + "plugin:brains:brains|default": { + accessToken: "tok-default-profile", serverUrl: `${ORIGIN}/mcp`, serverName: "plugin:brains:brains", + }, + }, + })); + // `security` answers ONLY for the bare, default-profile service name — the + // exact item a fallback would reach for — and "item not found" for anything + // else. A stub that refuses everything cannot observe a service-name fallback + // at all: with one, the file half of this rule is covered and the keychain + // half, which is the half the rule was written for, silently is not. It never + // touches a real keychain either way. + const defaultStore = join(homeDir, ".claude", ".credentials.json"); + const kcBin = join(temp, "profile-bin"); + mkdirSync(kcBin, { recursive: true }); + writeFileSync(join(kcBin, "security"), [ + "#!/bin/sh", + "svc=''", + "while [ $# -gt 0 ]; do", + ' case "$1" in -s) svc="$2"; shift 2 ;; *) shift ;; esac', + "done", + '[ "$svc" = "Claude Code-credentials" ] || exit 44', + `exec cat ${JSON.stringify(defaultStore)}`, + "", + ].join("\n")); + chmodSync(join(kcBin, "security"), 0o755); + const probe = (extra: Record) => sh( `if brains_resolve_credential ${JSON.stringify(ORIGIN)}; then printf 'RESOLVED:%s' "$BRAINS_CRED_LOCATOR"; else printf 'none'; fi`, - { env: { CLAUDE_CONFIG_DIR: profile, HOME: temp } }, + { env: { HOME: homeDir, PATH: `${kcBin}:${process.env.PATH ?? ""}`, BRAINS_CLAUDE_CREDENTIALS_FILE: "", ...extra } }, ); - check("an isolated profile with no store of its own resolves nothing", r.stdout === "none", r.stdout); + // Control first: the planted store is reachable and valid for this origin, so + // a "none" below means isolation rather than an unusable fixture. + const control = probe({ CLAUDE_CONFIG_DIR: "" }); + check("the planted default-profile store really is resolvable", + control.stdout === "RESOLVED:plugin:brains:brains|default", control.stdout); + const profile = join(temp, "isolated-profile"); + mkdirSync(profile, { recursive: true }); + const isolated = probe({ CLAUDE_CONFIG_DIR: profile }); + // Both backends are live in this one assertion: the keychain arm fails it if + // the service name falls back to the bare one, and the file arm fails it if + // the config dir does. Each of those is a separate way to read the default + // profile's credential from a profile that has none. + check("an isolated profile does not read the default profile's credential", + isolated.stdout === "none", isolated.stdout); } // =============================================================== binding @@ -241,13 +357,18 @@ section("binding — a discovered credential goes only to the origin that issued } { const state = freshState(); + // Marked, like every other receipt assertion here. Reading the whole log + // instead counts any request another block sent to the same port with the + // same bearer, so the check was about the suite's history rather than about + // this call. + const mark = readHits().length; const r = sh( `brains_resolve_credential ${JSON.stringify(ORIGIN)} || exit 1\n` + `brains_request ingest "http://127.0.0.1:${OTHER}/ingest/claude" -X POST -d '{}'\n` + `printf 'rc=%s blocked=%s' "$?" "$BRAINS_HTTP_BLOCKED"`, { state, env: { BRAINS_API_TOKEN: "tok-explicit" } }, ); - const reached = readHits().filter((h) => h.port === OTHER && h.auth === "Bearer tok-explicit"); + const reached = readHits().slice(mark).filter((h) => h.port === OTHER && h.auth === "Bearer tok-explicit"); check("an EXPLICIT credential may cross origins — the user chose that pairing", r.stdout === "rc=0 blocked=0" && reached.length === 1, `${r.stdout}, reached=${reached.length}`); } @@ -342,6 +463,51 @@ section("health — per capability, ordered by logical generation"); ); check("an inbox success does not clear a capture failure", r.stdout === "ingest=rejected inbox=ok", r.stdout); + + // The health STATE was covered; the CLAIM keyed on it was not, in either + // direction. Narrow the release and a fixed-then-broken endpoint is never + // announced again; broaden it to every key and this exact shape — inbox 200, + // ingest 403, on every single turn — re-emits "capture is OFF" every turn, + // which is the nagging the one-claim design exists to prevent. Three turns, + // exactly one emission. + const nagState = freshState(); + const turn = + `brains_resolve_credential ${JSON.stringify(ORIGIN)} || exit 1\n` + + `brains_resolve_endpoints "${ORIGIN}"\n` + + `BRAINS_URL_INGEST="${ORIGIN}/forbidden"\n` + + `brains_request ingest "$BRAINS_URL_INGEST" -X POST -d '{}'\n` + + `brains_request inbox "$BRAINS_URL_INBOX"\n` + + `brains_capture_signal\n`; + const nag = sh(turn + turn + turn, + { state: nagState, env: { BRAINS_CLAUDE_CREDENTIALS_FILE: PRIMARY_STORE } }); + const emissions = nag.stdout.split("").length - 1; + check("a per-turn refusal is announced exactly once across three turns", + emissions === 1, `${emissions} emission(s): ${nag.stdout.replace(/\n/g, " ").slice(0, 160)}`); + check("and what it announces is the refusal, not a missing credential", + /refused by the server/.test(nag.stdout) && !/no capture credential resolved/.test(nag.stdout), + nag.stdout.replace(/\n/g, " ").slice(0, 160)); +} +{ + // The other direction: a capability that starts failing, recovers, then fails + // again must be announced again. Releasing only on an observed 2xx is what + // makes that true, and deleting the per-capability release makes the second + // break silent forever — which is the failure mode this whole change exists + // to remove, reintroduced one level up. + const state = freshState(); + const script = + `brains_resolve_credential ${JSON.stringify(ORIGIN)} || exit 1\n` + + `brains_resolve_endpoints "${ORIGIN}"\n` + + `BRAINS_URL_INGEST="${ORIGIN}/forbidden"\n` + + `brains_request ingest "$BRAINS_URL_INGEST" -X POST -d '{}'\n` + + `brains_capture_signal\n` + + // Recovery: an observed 2xx on the SAME capability and endpoint. + `brains_health_note ingest "$BRAINS_URL_INGEST" ok\n` + + `brains_request ingest "$BRAINS_URL_INGEST" -X POST -d '{}'\n` + + `brains_capture_signal\n`; + const r = sh(script, { state, env: { BRAINS_CLAUDE_CREDENTIALS_FILE: PRIMARY_STORE } }); + const emissions = r.stdout.split("").length - 1; + check("a refusal that recovered and broke again is announced a second time", + emissions === 2, `${emissions} emission(s)`); } // =============================================================== the signal @@ -388,16 +554,37 @@ section("the off-state signal — once per cause, cleared only by an observed su // step it names. Every state must therefore actually name a step — and the // two that name a SETTING rather than a command are exactly where an agent // told to "run the command" could fabricate one and execute it. - for (const st of ["no-credential", "indeterminate", "rejected", "blocked"]) { + // + // Pinned to the ACTUAL step per state. "contains a backticked span" was + // satisfied by any backtick anywhere in the message, including one in a + // sentence that named no step at all, so it certified the shape of the text + // rather than the presence of a remedy. + const EXPECTED_STEP: Record = { + "no-credential": /`claude mcp login plugin:brains:brains`/, + indeterminate: /`token` option explicitly/, + rejected: /`claude mcp login plugin:brains:brains`/, + blocked: /`token` option to a token for this endpoint/, + }; + for (const [st, expected] of Object.entries(EXPECTED_STEP)) { const state = freshState(); const r = sh( `brains_resolve_endpoints "${ORIGIN}"\nbrains_health_note ingest "$BRAINS_URL_INGEST" ${st}\nbrains_capture_signal`, { state }, ); - const named = /`[^`]+`/.test(r.stdout); - check(`the ${st} signal names a concrete step`, - r.stdout.includes("") && named, r.stdout.trim().slice(0, 120)); + check(`the ${st} signal names the step that actually fixes it`, + r.stdout.includes("") && expected.test(r.stdout), + r.stdout.trim().slice(0, 160)); } + // `unreachable` is the one outcome that must NOT signal: transient network + // trouble is not user-actionable, and a warning that cries wolf on flaky wifi + // is how people learn to ignore the one that matters. Adding it to the + // endpoint-level case was previously a silent change. + const quiet = freshState(); + const r = sh( + `brains_resolve_endpoints "${ORIGIN}"\nbrains_health_note ingest "$BRAINS_URL_INGEST" unreachable\nbrains_capture_signal`, + { state: quiet }, + ); + check("an unreachable endpoint raises no signal at all", r.stdout.trim() === "", r.stdout); } { // The remedy has to be platform-aware as well as cause-aware. Codex storage @@ -415,15 +602,36 @@ section("the off-state signal — once per cause, cleared only by an observed su `BRAINS_CRED_CLIENT=codex\nbrains_resolve_endpoints "${ORIGIN}"\nbrains_health_note ingest "$BRAINS_URL_INGEST" no-credential\nbrains_capture_signal`, { state, env: { PATH: noSecurity } }, ); - // Codex on Linux is not a supported configuration, so the note says that and - // names nothing. Offering a token workaround there documented a path the - // product does not support, and core.md tells the agent to carry out the one - // step a note names — so a note with no supported step must say so outright - // rather than leave a gap for the agent to fill. - check("a Codex host without the macOS keychain is told the platform is unsupported", - r.stdout.includes("does not support Codex on this platform"), r.stdout.trim().slice(0, 160)); - check("...and is offered no remedy at all", - !r.stdout.includes("mcp login") && !r.stdout.includes("BRAINS_API_TOKEN"), r.stdout.trim().slice(0, 160)); + // Codex on Linux is not a supported configuration, so the note names no + // sign-in step — there is none to read there. What it must NOT do is claim + // there is nothing to change: the explicit-token branch is platform + // independent and does capture on Linux, which made the old wording + // measurably false in a note core.md tells the agent is authoritative. + check("a Codex host without the macOS keychain is told why the sign-in cannot be read", + r.stdout.includes("reads the Codex sign-in from the macOS keychain") && + r.stdout.includes("not a configuration brains supports"), r.stdout.trim().slice(0, 200)); + check("...and is not told that nothing would change it, which is false", + !r.stdout.includes("nothing to change"), r.stdout.trim().slice(0, 200)); + check("...and names no sign-in step, because there is none to name here", + !r.stdout.includes("mcp login"), r.stdout.trim().slice(0, 200)); + // A note with no action must not carry the offer protocol. Ending "offer this + // and act if they say yes" on a state with nothing to offer is a contradiction + // the agent resolves by inventing a command. + check("...and a note with no action does not invite the agent to offer one", + !r.stdout.includes("Offer this to the user") && r.stdout.includes("nothing for you to offer"), + r.stdout.trim().slice(0, 200)); +} +{ + // The claim the note above must stay consistent with: that same host DOES + // capture when a token is set explicitly. Asserted by resolving on it rather + // than by reading the note, so the two cannot drift apart silently. + const noSecurity = join(temp, "no-security-bin"); + const r = sh( + `if brains_resolve_credential ${JSON.stringify(ORIGIN)}; then printf 'RESOLVED|%s|%s' "$BRAINS_CRED_SOURCE" "$BRAINS_CRED_BINDING"; else printf 'none|%s' "$BRAINS_CRED_STATE"; fi`, + { env: { PATH: noSecurity, BRAINS_CRED_CLIENT: "codex", BRAINS_API_TOKEN: "tok-linux" } }, + ); + check("an explicit token still resolves on a host with no keychain at all", + r.stdout === "RESOLVED|env|explicit", r.stdout); } // =============================================================== fail-safe @@ -558,26 +766,32 @@ section("network calls are bounded — no call site can opt out"); // waits for the assistant POST. The default belongs inside brains_request // precisely because ingest — the one call site outside the inbox lib — is how // the original `--max-time 5` came to be dropped in the first place. - const blackhole = join(temp, "blackhole.js"); - writeFileSync(blackhole, `require("net").createServer(() => {}).listen(8994, "127.0.0.1");\n`); - const bh = spawn(process.execPath, [blackhole], { stdio: "ignore" }); - spawnSync("sleep", ["0.8"]); + // Served by the shared stub process on a kernel-chosen port, for the same + // reason as the others: this used to bind 8994 by hand, which two runs of the + // suite cannot both do. + const BH = `http://127.0.0.1:${BLACKHOLE}`; const bhStore = fixture("blackhole-store", { - mcpOAuth: { "a|1": { accessToken: "tok-bh", serverUrl: "http://127.0.0.1:8994/mcp", serverName: "brains" } }, + mcpOAuth: { "a|1": { accessToken: "tok-bh", serverUrl: `${BH}/mcp`, serverName: "brains" } }, }); const started = Date.now(); const r = sh( - `brains_resolve_credential http://127.0.0.1:8994 || exit 1\n` + - `brains_request ingest http://127.0.0.1:8994/ingest/claude -X POST -d '{}'\n` + + `brains_resolve_credential ${JSON.stringify(BH)} || exit 1\n` + + `brains_request ingest ${JSON.stringify(`${BH}/ingest/claude`)} -X POST -d '{}'\n` + `printf 'rc=%s code=%s' "$?" "$BRAINS_HTTP_CODE"`, { env: { BRAINS_CLAUDE_CREDENTIALS_FILE: bhStore } }, ); const elapsed = Date.now() - started; - bh.kill(); check("a server that accepts and never answers cannot hang a request", r.stdout.startsWith("rc=1") && elapsed < 15000, `${r.stdout} in ${elapsed}ms`); - check("brains_request carries default connect and total deadlines", - /--connect-timeout/.test(readFileSync(LIB, "utf8")) && /--max-time/.test(readFileSync(LIB, "utf8"))); + // The library defaults had a text pin here too. Its /--max-time/ half was + // satisfied by the comment that explains the default, and the behaviour it + // claimed is already proved above by driving a server that accepts and never + // answers — a strictly stronger check. Deleted rather than strengthened. + // + // The ingest CALL SITE ceiling has no dynamic equivalent: the black-hole test + // exercises the library default, not the tighter bound the synchronous Codex + // Stop path passes. Kept, and it is held by code — the string appears in no + // comment, and gutting it fails this check. check("the ingest call site keeps its own explicit ceiling", /brains_request ingest [^\n]*--max-time/.test(readFileSync(TURN, "utf8"))); } @@ -818,6 +1032,39 @@ section("the turn hook's backgrounded ingest actually carries a credential"); check("a backgrounded ingest POST reaches the endpoint", ingest.length === 1, `${ingest.length} POSTs`); check("and it carries the resolved bearer", ingest[0]?.auth === "Bearer tok-turnhook", ingest[0]?.auth ?? "(none)"); + + // The log line, which had no coverage at all — `capture user: …` could be + // replaced with `:` and nothing here noticed. It is half of what this change + // is for: the reason a broken credential went unseen for eleven days is that + // brains.log had never carried one word about capture. A feature justified by + // observability has to be observable in a test. + const logPath = join(state, "brains.log"); + let logText = ""; + for (let i = 0; i < 40; i++) { + logText = existsSync(logPath) ? readFileSync(logPath, "utf8") : ""; + if (/capture user:/.test(logText)) break; + spawnSync("sleep", ["0.1"]); + } + const captureLines = logText.split("\n").filter((l) => l.includes("capture ")); + check("the hook writes one capture line naming the outcome, status and source", + captureLines.length === 1 && + /\[turn\] capture user: ok status=200 source=claude-oauth$/.test(captureLines[0] ?? ""), + JSON.stringify(captureLines)); + // ...and exactly one. A healthy outcome is logged once per session, so the + // log answers "is capture working" without growing by two lines a turn. + spawnSync("bash", [TURN], { + input: JSON.stringify({ session_id: "turnhook", prompt: "second turn" }), + env: { + ...(process.env as Record), + CLAUDE_PLUGIN_OPTION_TOKEN: "", BRAINS_API_TOKEN: "", BRAINS_INBOX_TOKEN: "", + BRAINS_STATE_DIR: state, BRAINS_ENDPOINT: ORIGIN, BRAINS_CLAUDE_CREDENTIALS_FILE: store, + }, + encoding: "utf8", timeout: 30000, + }); + spawnSync("sleep", ["1"]); + const afterSecond = readFileSync(logPath, "utf8").split("\n").filter((l) => l.includes("capture ")); + check("and a healthy second turn in the same session adds no second line", + afterSecond.length === 1, JSON.stringify(afterSecond)); // The property the lease exists for, tested deterministically rather than by // racing: a backgrounded request must survive the master config being // released. Waiting for the race to bite is unreliable — it passed with the @@ -858,6 +1105,178 @@ section("the turn hook's backgrounded ingest actually carries a credential"); readDirNames(join(state, "tmp")).join(",")); } +// =============================================================== housekeeping +section("session start sweeps what nothing else removes"); +{ + // Found by scripts/mutation-coverage.sh, not by review: every line of the + // session-start housekeeping could be replaced with `:` and all four suites + // stayed green. The session-END prune was registered in the gate and the + // session-START one, which is the half that also expires the marker files, + // was not covered at all — the exact blind spot a hand-maintained registry + // cannot report on itself. + const START = join(PLUGIN, "hooks", "brains-start.sh"); + const state = freshState(); + const old = (name: string) => { + const p = join(state, name); + writeFileSync(p, "x"); + spawnSync("touch", ["-t", "202601010000", p]); + return p; + }; + const staleNow = old("now-ancient"); + const staleErr = old("toolerr-seen-ancient"); + const staleCap = old("capok-ancient"); + const freshNow = join(state, "now-today"); + writeFileSync(freshNow, "x"); + // Not ours and not expired-by-age: neither may be swept. + const foreign = join(state, "brains.log"); + writeFileSync(foreign, "keep me"); + spawnSync("touch", ["-t", "202601010000", foreign]); + mkdirSync(join(state, "tmp"), { recursive: true }); + const orphan = join(state, "tmp", "r.orphan"); + mkdirSync(orphan, { recursive: true }); + spawnSync("touch", ["-t", "202601010000", orphan]); + + spawnSync("bash", [START], { + input: JSON.stringify({ session_id: "housekeeping" }), + env: { + ...(process.env as Record), + CLAUDE_PLUGIN_OPTION_TOKEN: "", BRAINS_API_TOKEN: "", BRAINS_INBOX_TOKEN: "", + BRAINS_STATE_DIR: state, BRAINS_ENDPOINT: ORIGIN, + BRAINS_CREDENTIAL_STORE_DISABLED: "1", + }, + encoding: "utf8", timeout: 30000, + }); + check("expired per-session marker files are swept at session start", + !existsSync(staleNow) && !existsSync(staleErr) && !existsSync(staleCap), + [staleNow, staleErr, staleCap].filter(existsSync).join(",")); + check("a recent marker file is not", + existsSync(freshNow), "a marker from this week was deleted"); + check("and a file that is neither a marker nor ours is left alone", + existsSync(foreign), "brains.log was swept"); + check("a credential work directory orphaned by SIGKILL is pruned at session start", + !existsSync(orphan), "the orphaned directory survived"); +} + +// =============================================================== turn hook, uncovered halves +section("the turn hook's other halves — time injection, the off-state record, and Codex"); +{ + // Every assertion in this section exists because + // scripts/mutation-coverage.sh could delete the line it covers with all four + // suites still green. None of them were found by reading the diff. + const runTurn = (payload: unknown, state: string, extra: Record = {}) => + spawnSync("bash", [TURN], { + input: JSON.stringify(payload), + env: { + ...(process.env as Record), + CLAUDE_PLUGIN_OPTION_TOKEN: "", BRAINS_API_TOKEN: "", BRAINS_INBOX_TOKEN: "", + BRAINS_STATE_DIR: state, BRAINS_ENDPOINT: ORIGIN, + BRAINS_CREDENTIAL_STORE_DISABLED: "1", + ...extra, + }, + encoding: "utf8", timeout: 30000, + }); + + // 1. `now:` injection. Moving it ABOVE the credential gate is the whole point + // of that part of the change — a user with no token used to lose accurate + // current-time injection as collateral — so it is driven with discovery + // switched off, which is exactly that user. + { + const state = freshState(); + const first = runTurn({ session_id: "nowtest", prompt: "hi" }, state); + check("a tokenless turn still injects the current time", + /now: /.test(first.stdout ?? ""), JSON.stringify(first.stdout)); + const second = runTurn({ session_id: "nowtest", prompt: "again" }, state); + check("and not again in the same clock hour", + !(second.stdout ?? "").includes("brains:now"), JSON.stringify(second.stdout)); + // The marker is what makes the second turn quiet; without it every turn + // injects, which is the per-turn noise the once-an-hour rule exists to stop. + check("the once-an-hour marker is what does it", + existsSync(join(state, "now-nowtest")), readDirNames(state).join(",")); + } + + // 2. The off-state RECORD. Without it a tokenless user gets no signal at all + // — the session-start hook has nothing to read — which is the entire + // observability half of this change. + { + const state = freshState(); + runTurn({ session_id: "offstate", prompt: "hi" }, state); + const sig = sh( + `brains_resolve_endpoints "${ORIGIN}"\nbrains_capture_signal`, + { state }, + ); + check("a turn with no credential records the off-state for the session-start signal", + sig.stdout.includes("") && /no capture credential resolved/.test(sig.stdout), + sig.stdout.trim().slice(0, 160) || "(nothing recorded)"); + } + + // 3. The client the hook declares to the resolver. On Codex this selects a + // different store entirely, so dropping it silently sends a Codex user to + // the Claude keychain. + { + const state = freshState(); + const store = fixture("client-decl-store", { + mcpOAuth: { "a|1": { accessToken: "tok-claude-store", serverUrl: `${ORIGIN}/mcp`, serverName: "brains" } }, + }); + const codexArray = fixture("client-decl-codex", [ + { server_name: "brains", url: `${ORIGIN}/mcp`, token_response: { access_token: "tok-codex-store" } }, + ]); + const mark = readHits().length; + // PLUGIN_ROOT is what the hook reads to decide it is running under Codex. + runTurn({ session_id: "clientdecl", last_assistant_message: "answer" }, state, { + PLUGIN_ROOT: PLUGIN, + BRAINS_CREDENTIAL_STORE_DISABLED: "", + BRAINS_CLAUDE_CREDENTIALS_FILE: store, + BRAINS_CODEX_CREDENTIALS_FILE: codexArray, + }); + let posts: Hit[] = []; + for (let i = 0; i < 30 && posts.length === 0; i++) { + spawnSync("sleep", ["0.1"]); + posts = readHits().slice(mark).filter((h) => h.path === "/ingest/claude"); + } + // Two things at once, and both were unguarded: the hook declares the client + // to the resolver, so the CODEX store is the one read; and the Codex + // assistant POST is synchronous, so it has already landed when the hook + // returns rather than being backgrounded. + check("under Codex the hook resolves from the Codex store, not the Claude one", + posts[0]?.auth === "Bearer tok-codex-store", posts[0]?.auth ?? "(no request)"); + } + + // 4. The blocked outcome word in the log. A cross-origin refusal has to be + // distinguishable in brains.log from a refusal by the server. + { + const state = freshState(); + const store = fixture("blocked-log-store", { + mcpOAuth: { "a|1": { accessToken: "tok-blk", serverUrl: `${ORIGIN}/mcp`, serverName: "brains" } }, + }); + runTurn({ session_id: "blocklog", prompt: "hi" }, state, { + BRAINS_CREDENTIAL_STORE_DISABLED: "", + BRAINS_CLAUDE_CREDENTIALS_FILE: store, + BRAINS_INGEST_URL: `http://127.0.0.1:${OTHER}/ingest/claude`, + }); + spawnSync("sleep", ["1"]); + const logText = existsSync(join(state, "brains.log")) ? readFileSync(join(state, "brains.log"), "utf8") : ""; + check("a cross-origin refusal is logged as blocked, not as an error", + /capture user: blocked /.test(logText), JSON.stringify(logText.trim().slice(-160))); + } + + // 5. The log's mode tag. `turn` and `stop` are how a reader tells which hook + // event wrote a line, and the Stop half had no coverage. + { + const state = freshState(); + const store = fixture("stop-mode-store", { + mcpOAuth: { "a|1": { accessToken: "tok-stop", serverUrl: `${ORIGIN}/mcp`, serverName: "brains" } }, + }); + runTurn({ session_id: "stopmode", last_assistant_message: "answer" }, state, { + BRAINS_CREDENTIAL_STORE_DISABLED: "", + BRAINS_CLAUDE_CREDENTIALS_FILE: store, + }); + spawnSync("sleep", ["1"]); + const logText = existsSync(join(state, "brains.log")) ? readFileSync(join(state, "brains.log"), "utf8") : ""; + check("a Stop turn is tagged stop in the log, not turn", + /\[stop\] capture assistant:/.test(logText), JSON.stringify(logText.trim().slice(-160))); + } +} + // =============================================================== fresh install section("a fresh install is told to sign in, not that its store is unreadable"); { @@ -878,9 +1297,13 @@ section("a fresh install is told to sign in, not that its store is unreadable"); ); check("keychain item-not-found with no fallback file is a definite no-credential", r.stdout === "no-credential", r.stdout); + // Driven under the SAME environment as the resolve above. Passing `{}` here + // meant this stopped being the fresh-install signal and became the generic + // one: the fresh PATH and HOME were dropped, so the assertion held for a host + // that was nothing like the one under test. const sig = sh( `brains_resolve_endpoints "${ORIGIN}"\nbrains_health_note ingest "$BRAINS_URL_INGEST" no-credential\nbrains_capture_signal`, - {}, + { env: { PATH: `${freshBin}:${process.env.PATH ?? ""}`, HOME: home, CLAUDE_CONFIG_DIR: join(home, ".claude"), BRAINS_CLAUDE_CREDENTIALS_FILE: "" } }, ); check("and the remedy it gets is the sign-in", sig.stdout.includes("mcp login"), sig.stdout.trim().slice(0, 120)); @@ -892,6 +1315,11 @@ section("a signal during a store read is delivered, not swallowed"); // Replacing INT/TERM with a cleanup-only handler eats the signal: bash // resumes afterwards and the caller's handler never runs, so a sourced // library would be redefining Ctrl-C for the whole hook. + // + // This block drives a store read that FAILS, which is a real path but not the + // common one — it never reaches the config writer, and the config writer had + // the same trap. See the RESOLVED-path block below, which is the path every + // working user takes on every turn and the one that was actually broken. const slowBin = join(temp, "signal-bin"); mkdirSync(slowBin, { recursive: true }); writeFileSync(join(slowBin, "security"), `#!/bin/sh\nprintf '{"mcpOAuth":{}}'\nsleep 30\n`); @@ -940,6 +1368,70 @@ section("a signal during a store read is delivered, not swallowed"); } check("and the credential directory is not left behind", remaining.length === 0, remaining.join(",")); } +{ + // The RESOLVED path, which is every turn of every working user — and the one + // the block above could not see. Writing the curl config installed a second + // cleanup trap covering INT/TERM/HUP, and a trap REPLACES whatever the caller + // had: after a credential resolved, the hook stopped responding to Ctrl-C for + // the rest of its run. The block above never reached that code because its + // store read fails, which is exactly why the bug survived it. + const store = fixture("signal-resolved-store", { + mcpOAuth: { "a|1": { accessToken: "tok-signal", serverUrl: `${ORIGIN}/mcp`, serverName: "brains" } }, + }); + const state = freshState(); + const env = { + ...(process.env as Record), + CLAUDE_PLUGIN_OPTION_TOKEN: "", BRAINS_API_TOKEN: "", BRAINS_INBOX_TOKEN: "", + BRAINS_STATE_DIR: state, BRAINS_CLAUDE_CREDENTIALS_FILE: store, + }; + // Short sleeps rather than one long one: bash defers a trap until the running + // foreground command returns, so `sleep 5` would delay the handler by five + // seconds and make a swallowed signal indistinguishable from a slow one. + const driver = join(temp, "sig-resolved.sh"); + writeFileSync(driver, + `. ${JSON.stringify(LIB)}\n` + + `trap 'echo CALLER-HANDLER-RAN; exit 42' TERM\n` + + `brains_resolve_credential ${JSON.stringify(ORIGIN)} || { echo NOT-RESOLVED; exit 9; }\n` + + `echo RESOLVED\n` + + `i=0; while [ $i -lt 100 ]; do sleep 0.05; i=$((i+1)); done\n` + + `echo SHOULD-NOT-REACH\n`); + const out = join(temp, "sig-resolved.out"); + const runner = join(temp, "sig-resolved-runner.sh"); + writeFileSync(runner, `bash ${JSON.stringify(driver)} >${JSON.stringify(out)} 2>/dev/null &\nP=$!\nsleep 1.2\nkill -TERM $P 2>/dev/null\nwait $P 2>/dev/null\nprintf '%s' "$?"\n`); + const run = spawnSync("bash", [runner], { env, encoding: "utf8", timeout: 30000 }); + const text = existsSync(out) ? readFileSync(out, "utf8") : ""; + check("the credential actually resolved, so the trap under test was installed", + text.includes("RESOLVED"), JSON.stringify(text)); + check("a resolved credential does not make the hook ignore TERM", + text.includes("CALLER-HANDLER-RAN") && !text.includes("SHOULD-NOT-REACH"), JSON.stringify(text)); + check("and the caller's handler still controls the exit status", + (run.stdout ?? "").trim() === "42", (run.stdout ?? "").trim()); +} + +// ================================================== EXIT-only cleanup (rule I3) +section("cleanup runs on EXIT only — no shipped file installs a signal handler"); +{ + // Four separate instances of the same mistake were written on one change: + // a cleanup-only INT/TERM/HUP handler does not re-raise, so bash runs it and + // RESUMES. The process stops dying on that signal, and any handler the caller + // had is replaced outright. Catching the fifth by review is not a plan, so the + // rule is stated in the library header as I3 and enforced here over every + // shipped file. Whatever a signal-killed shell leaves behind is + // brains_cred_prune_tmp's job, which is the story SIGKILL already needed. + // + // A negative scan, deliberately: a comment mentioning TERM makes this FAIL + // rather than pass, which is the safe direction for it to be wrong in. + const hooksDir = join(PLUGIN, "hooks"); + for (const name of ["brains-turn.sh", "brains-start.sh", "brains-end.sh", "brains-tool-error.sh", + join("lib", "brains-inbox.sh"), join("lib", "brains-credential.sh")]) { + const offenders = readFileSync(join(hooksDir, name), "utf8") + .split("\n") + .filter((l) => !/^\s*#/.test(l) && /(^|[\s;(])trap\s/.test(l)) + .filter((l) => /\b(INT|TERM|HUP|QUIT|USR1|USR2|SIG[A-Z]+)\b/.test(l)); + check(`${name} installs no signal handler`, offenders.length === 0, + offenders.map((l) => l.trim()).join(" | ")); + } +} // =============================================================== curlrc section("the user's own curl config cannot capture the bearer"); @@ -982,7 +1474,16 @@ section("the token never leaves brains_request"); "only brains_request may present the credential"); } const lib = readFileSync(LIB, "utf8"); - check("no debug tracing is enabled anywhere in the library", !/^\s*set -x/m.test(lib)); + // The old form of this check asserted "no `set -x` anywhere in the library" + // and passed while the library contained one — the line-anchored regex could + // not reach `[ -n ... ] && set -x`, so the check was both false and green. + // The true claim is narrower: the library enables tracing in exactly one + // place, and only to restore a setting the CALLER had on. Anything else is an + // unconditional trace of a path that handles the explicit tokens. + const enablers = lib.split("\n").filter((l) => /(^|[;&|]|\bthen\b|\bdo\b)\s*set\s+-[a-wyz]*x/.test(l)); + check("the library enables tracing in one place only, to restore the caller's own", + enablers.length === 1 && /BRAINS_CRED_XTRACE/.test(enablers[0] ?? ""), + enablers.map((l) => l.trim()).join(" | ") || "no `set -x` found at all — the restore half is missing"); // Grepping the source for uses of the value proved to be the wrong test: it // passes while the token is still being handed to curl as an argument, where // any local `ps` can read it, and printed in full by `bash -x` before any @@ -1050,8 +1551,22 @@ section("a large but valid store still resolves"); const big = fixture("oversize-store", { mcpOAuth: entries }); const bytes = readFileSync(big, "utf8").length; check(`the fixture is genuinely over the old 256 KiB ceiling (${bytes} bytes)`, bytes > 262144); + const started = Date.now(); + const wide = resolveWith(big); + const elapsed = Date.now() - started; check("a store far larger than one credential document still resolves", - resolveWith(big).stdout.startsWith("ok|claude-oauth|"), resolveWith(big).stdout); + wide.stdout.startsWith("ok|claude-oauth|"), wide.stdout); + // Raising the ceiling turned a fast REJECT into a slow ACCEPT, and nothing + // here noticed: the selection loop spent two forks on every entry — one to + // recompute the field separator, one to canonicalise an origin — so a store + // the widened ceiling now admits cost seconds, twice per turn. A reviewer had + // to measure that because the suite only ever asked for the right answer. + // + // The bound is generous on purpose. Measured on this fixture: ~0.15s with the + // forks hoisted, ~4.5s without. Anything in between is a regression worth + // failing on, and the margin is wide enough that a loaded runner does not. + check(`and resolving it costs no per-entry forks (${elapsed}ms for ${Object.keys(entries).length} entries)`, + elapsed < 3000, `${elapsed}ms — the selection loop is forking per entry again`); } // =============================================================== codex arrays @@ -1081,6 +1596,41 @@ section("duplicate collapse covers the array backend as well as the keychain"); probe(same) === "RESOLVED|1", probe(same)); check("two array entries carrying different bearers stay indeterminate", probe(diff) === "none|indeterminate|2", probe(diff)); + + // A MIXED array — one entry for brains, one for something else. Only the + // all-matching shapes above were covered, and that is precisely the shape the + // bug could not reach: the non-matching row discarded the snapshot every row + // shares, so selection found its one credential and the config writer then + // read a deleted file. `count=1` with state `indeterminate` is the signature. + // + // Both orders, because the failure depended on the non-matching row coming + // first or last only in how far it got before losing the document. + const mixed = [ + { server_name: "brains", url: `${ORIGIN}/mcp`, token_response: { access_token: "tok-mine" } }, + { server_name: "other", url: `${ORIGIN}/mcp`, token_response: { access_token: "tok-theirs" } }, + ]; + check("a matching entry alongside a non-matching one still resolves", + probe(mixed) === "RESOLVED|1", probe(mixed)); + check("and so does the same array in the other order", + probe([mixed[1], mixed[0]]) === "RESOLVED|1", probe([mixed[1], mixed[0]])); + + // The WINNING record is the one file that has to outlive selection, and it is + // therefore the one nothing was checking got removed afterwards. Dropping the + // discard left `tmp/r../v` on disk with the plaintext bearer in it + // until the five-minute prune — a window nothing here could see. Asserted on + // the whole tmp tree rather than on one path, so a future backend that leaves + // a different file behind fails this too. + const state = freshState(); + const f = fixture("codex-cleanup", [ + { server_name: "brains", url: `${ORIGIN}/mcp`, token_response: { access_token: "tok-cleanup" } }, + ]); + const r = sh( + `BRAINS_CRED_CLIENT=codex\nbrains_resolve_credential ${JSON.stringify(ORIGIN)} || exit 1\n` + + `printf '%s' "$(find "$BRAINS_STATE_DIR/tmp" -type f ! -name curl.conf 2>/dev/null | wc -l | tr -d ' ')"`, + { state, env: { BRAINS_CODEX_CREDENTIALS_FILE: f } }, + ); + check("a Codex resolve leaves no store copy behind, only the curl config", + r.stdout === "0", `${r.stdout} file(s) left in tmp`); } // =============================================================== mutation gate @@ -1105,8 +1655,14 @@ type Mutation = { probe: string; // shell, with $LIB pointing at the mutated copy env?: Record; stubs?: Record; // written to $STUBS, chmod +x - requires?: string; // the unmutated probe MUST return this, or SKIP loudly + requires: string; // the unmutated probe MUST return this, or FAIL loudly }; +// `requires` is mandatory, not optional. A probe whose setup silently failed +// answers the same thing with and without the mechanism, which reads as a +// mutation the gate could not certify — but two entries here were seen emitting +// that verdict on a loaded machine, and the ones without a precondition are the +// ones that could. Pinning the unmutated answer turns "the probe never reached +// its mechanism" from a flaky red into a specific one that names the setup. // A stub keychain tool that enumerates one Codex account, for hosts with none. const CODEX_DUMP_STUB = [ "#!/bin/sh", @@ -1132,11 +1688,24 @@ const MUTATIONS: Mutation[] = [ // gone. label: "the turn hook's ingest-lease cleanup (trap + explicit return)", file: "hooks/brains-turn.sh", - find: ` trap 'brains_cred_return "$_lease"' EXIT INT TERM HUP\n ingest_once "$role" "$payload"\n brains_cred_return "$_lease" ) &`, + find: ` trap 'brains_cred_return "$_lease"' EXIT\n ingest_once "$role" "$payload"\n brains_cred_return "$_lease" ) &`, replace: ` ingest_once "$role" "$payload" ) &`, - // A terminated child must not strand its leased copy of the bearer. - probe: `bash "$TURNCOPY" <<<'{"session_id":"mut","prompt":"x"}' >/dev/null 2>&1; k=0; while [ $k -lt 80 ]; do [ -z "$(find "$BRAINS_STATE_DIR/tmp" -path "*lease*" -name curl.conf 2>/dev/null)" ] && break; sleep 0.1; k=$((k+1)); done; printf '%s' "$(find "$BRAINS_STATE_DIR/tmp" -path "*lease*" -name curl.conf 2>/dev/null | wc -l | tr -d ' ')"`, + // Answers TWO questions, because "no lease left behind" alone is also what a + // hook that never took one says. `posts` is read off the stub's receipt log + // and proves the backgrounded request really happened; `leases` is the + // mechanism under test. Unmutated that is 1/0. A setup that failed reads + // 0/0, which the precondition rejects out loud instead of certifying. + probe: `n0=$(wc -l <"$HITS" 2>/dev/null | tr -d ' '); [ -n "$n0" ] || n0=0 +posts() { tail -n "+$((n0+1))" "$HITS" 2>/dev/null | grep -c '/ingest/claude'; } +leases() { find "$BRAINS_STATE_DIR/tmp" -path "*lease*" -name curl.conf 2>/dev/null | wc -l | tr -d ' '; } +bash "$TURNCOPY" <<<'{"session_id":"mut","prompt":"x"}' >/dev/null 2>&1 +k=0; while [ $k -lt 80 ]; do + [ "$(posts)" -gt 0 ] && [ "$(leases)" = "0" ] && break + sleep 0.1; k=$((k+1)) +done +printf '%s/%s' "$(posts)" "$(leases)"`, env: { BRAINS_API_TOKEN: "tok-slow" }, + requires: "1/0", }, { // Registered as the PAIR — the trap and the explicit return — because they @@ -1149,7 +1718,7 @@ const MUTATIONS: Mutation[] = [ // the lease surviving with the bearer in it. See the report note. label: "the inbox engine's ack-lease cleanup (trap + explicit return)", file: "hooks/lib/brains-inbox.sh", - find: ` trap 'brains_cred_return "$ack_lease"' EXIT INT TERM HUP\n brains_request ack "$ACK_ENDPOINT" --max-time 3 -X POST \\\n -H "Content-Type: application/json" -d "$ack" >/dev/null 2>&1\n brains_cred_return "$ack_lease" ) &`, + find: ` trap 'brains_cred_return "$ack_lease"' EXIT\n brains_request ack "$ACK_ENDPOINT" --max-time 3 -X POST \\\n -H "Content-Type: application/json" -d "$ack" >/dev/null 2>&1\n brains_cred_return "$ack_lease" ) &`, replace: ` brains_request ack "$ACK_ENDPOINT" --max-time 3 -X POST \\\n -H "Content-Type: application/json" -d "$ack" >/dev/null 2>&1 ) &`, // Observed on NORMAL completion rather than by signalling. Signalling a // backgrounded ack is inherently racy — whether the trap or the explicit @@ -1157,7 +1726,20 @@ const MUTATIONS: Mutation[] = [ // signal lands, and a probe that answers differently run to run certifies // nothing. The property that matters and is deterministic: once the ack has // finished, its private copy of the bearer is gone. - probe: `bash "$INBOXCOPY" prompt mut >/dev/null 2>&1; k=0; while [ $k -lt 80 ]; do [ -z "$(find "$BRAINS_STATE_DIR/tmp" -path "*lease*" -name curl.conf 2>/dev/null)" ] && break; sleep 0.1; k=$((k+1)); done; printf '%s' "$(find "$BRAINS_STATE_DIR/tmp" -path "*lease*" -name curl.conf 2>/dev/null | wc -l | tr -d ' ')"`, + // Two facts again: the ack must actually have been sent, and its lease must + // be gone. Unmutated that is 1/0; an engine that found nothing to ack reads + // 0/0 and the precondition fails rather than certifying a probe that never + // reached the mechanism. + probe: `n0=$(wc -l <"$HITS" 2>/dev/null | tr -d ' '); [ -n "$n0" ] || n0=0 +acks() { tail -n "+$((n0+1))" "$HITS" 2>/dev/null | grep -c '/inbox/claude/ack'; } +leases() { find "$BRAINS_STATE_DIR/tmp" -path "*lease*" -name curl.conf 2>/dev/null | wc -l | tr -d ' '; } +bash "$INBOXCOPY" prompt mut >/dev/null 2>&1 +k=0; while [ $k -lt 80 ]; do + [ "$(acks)" -gt 0 ] && [ "$(leases)" = "0" ] && break + sleep 0.1; k=$((k+1)) +done +printf '%s/%s' "$(acks)" "$(leases)"`, + requires: "1/0", env: { BRAINS_API_TOKEN: "tok-slow", // The engine only acks when the inbox returns something to acknowledge, @@ -1167,7 +1749,13 @@ const MUTATIONS: Mutation[] = [ }, }, { - label: "the account-list completeness check (LC_ALL=C + PIPESTATUS)", + // Named for what the mutation actually removes. The label used to claim + // "LC_ALL=C + PIPESTATUS" while only the PIPESTATUS line was touched, so the + // locale pin was riding on a certification that never covered it — the same + // shape of overclaim this gate exists to stop. LC_ALL=C guards a parse whose + // difference needs a locale the runner may not have; it is not registered + // here rather than registered dishonestly. + label: "the account-list completeness check (PIPESTATUS)", file: "hooks/lib/brains-credential.sh", find: ` set -- "\${PIPESTATUS[0]}" "\${PIPESTATUS[1]}"`, replace: ` set -- 0 0`, @@ -1193,6 +1781,7 @@ printf '%s/%s' "$BRAINS_CRED_STATE" "$BRAINS_CRED_TRUNCATED"`, find: ` . "$CRED_LIB" 2>/dev/null && brains_cred_prune_tmp`, replace: ` :`, probe: `mkdir -p "$BRAINS_STATE_DIR/tmp/r.stale"; touch -t 202601010000 "$BRAINS_STATE_DIR/tmp/r.stale"; bash "$ENDCOPY" <<<'{"session_id":"mut"}' >/dev/null 2>&1; printf '%s' "$([ -d "$BRAINS_STATE_DIR/tmp/r.stale" ] && echo present || echo pruned)"`, + requires: "pruned", }, { label: "the xtrace shield around the explicit-token branches", @@ -1201,6 +1790,7 @@ printf '%s/%s' "$BRAINS_CRED_STATE" "$BRAINS_CRED_TRUNCATED"`, replace: ` *x*) BRAINS_CRED_XTRACE="" ;;`, probe: `printf '%s' "$(bash -x -c '. "$0"; brains_resolve_credential "$1" >/dev/null' "$LIB" "$ORIGIN" 2>&1 | grep -c MUTCANARY)"`, env: { BRAINS_API_TOKEN: "MUTCANARY" }, + requires: "0", }, { label: "the discard guard that confines rm -rf to the owned root", @@ -1212,6 +1802,97 @@ printf '%s/%s' "$BRAINS_CRED_STATE" "$BRAINS_CRED_TRUNCATED"`, esac`, replace: "", probe: `. "$LIB"; _brains_cred_tmp_root >/dev/null 2>&1; mkdir -p "$OUTSIDE/keep"; _brains_discard "$OUTSIDE/keep/f" 2>/dev/null; printf '%s' "$([ -d "$OUTSIDE/keep" ] && echo intact || echo DELETED)"`, + requires: "intact", + }, + { + // Origin alone is not enough, and this is the mutation that shows it: gut + // the guard and another MCP server sharing the brains origin has its bearer + // selected and written into the curl config bound for /ingest/claude. The + // whole function was deletable with every suite green, because every + // origin-matching fixture in the file happened to name a brains server. + label: "the server-identity guard that keeps a neighbour's bearer out", + file: "hooks/lib/brains-credential.sh", + find: ` case "$name" in + "$want"|*:"$want") return 0 ;; + *) return 1 ;; + esac`, + replace: " return 0", + probe: `printf '%s' '{"mcpOAuth":{"plugin:github:github|h":{"accessToken":"FOREIGN","serverUrl":"'"$ORIGIN"'/mcp","serverName":"plugin:github:github"}}}' > "$BRAINS_STATE_DIR/foreign.json" +export BRAINS_CLAUDE_CREDENTIALS_FILE="$BRAINS_STATE_DIR/foreign.json" +. "$LIB"; brains_resolve_credential "$ORIGIN" >/dev/null 2>&1 +printf '%s/%s' "$BRAINS_CRED_STATE" "$BRAINS_CRED_LOCATOR"`, + requires: "blocked/", + }, + { + // I3, mutated the way the bug was actually written: add the three signals + // back to the config cleanup trap. This is the RESOLVED path, so it is every + // turn of every working user — and the mutation is exactly the shipped code + // as it stood, which makes the probe a regression test for a real defect + // rather than for a hypothetical one. + label: "the EXIT-only cleanup trap around the curl config", + file: "hooks/lib/brains-credential.sh", + find: ` trap 'rm -rf "$BRAINS_CRED_TMP/r.$$.cfg" 2>/dev/null' EXIT`, + replace: ` trap 'rm -rf "$BRAINS_CRED_TMP/r.$$.cfg" 2>/dev/null' EXIT INT TERM HUP`, + // Short sleeps: bash defers a trap until the running foreground command + // returns, so one long sleep would report a swallowed signal for a slow one. + probe: `cat > "$BRAINS_STATE_DIR/d.sh" <<'EOD' +. "$LIB" +trap 'echo HANDLER; exit 42' TERM +brains_resolve_credential "$ORIGIN" || { echo NORESOLVE; exit 9; } +i=0; while [ $i -lt 60 ]; do sleep 0.05; i=$((i+1)); done +echo RESUMED +EOD +bash "$BRAINS_STATE_DIR/d.sh" >"$BRAINS_STATE_DIR/o.txt" 2>/dev/null & +P=$! +sleep 1.0 +kill -TERM $P 2>/dev/null +wait $P 2>/dev/null +if grep -q HANDLER "$BRAINS_STATE_DIR/o.txt" 2>/dev/null; then printf 'handler' +elif grep -q RESUMED "$BRAINS_STATE_DIR/o.txt" 2>/dev/null; then printf 'swallowed' +else printf 'neither'; fi`, + requires: "handler", + }, + { + // The total budget is documented as bounding the whole resolution, and until + // this change it was consulted on one path only — the Codex enumeration — + // leaving the Claude selection loop, the one the widened document ceiling + // grew, unbounded. Driven with the budget set to zero so the guard is + // reached deterministically instead of by building a pathological store. + label: "the total budget applied to the selection loop, not just enumeration", + file: "hooks/lib/brains-credential.sh", + find: ` if ! _brains_budget_left; then + BRAINS_CRED_TRUNCATED=1 + [ "$recfile" != "\${store:-}" ] && _brains_discard "$recfile" + continue + fi`, + replace: "", + probe: `. "$LIB"; BRAINS_CRED_TOTAL_BUDGET=0 +brains_resolve_credential "$ORIGIN" >/dev/null 2>&1 +printf '%s/%s' "$BRAINS_CRED_STATE" "$BRAINS_CRED_TRUNCATED"`, + requires: "indeterminate/1", + }, + { + // A non-matching row may discard only a record file PRIVATE to it. Keying + // that on the client rather than on shared-ness is what let the Codex ARRAY + // backend delete the snapshot every row names, after the same bug had been + // fixed on the Claude path — so the mutation restores the client check and + // requires the mixed array to break. + label: "the shared-snapshot guard in the selection loop (keyed on shared-ness)", + file: "hooks/lib/brains-credential.sh", + find: ` if ! _brains_is_brains_server "$name" || + ! corigin=$(brains_origin "$url") || + [ "$corigin" != "$want" ]; then`, + replace: ` if ! _brains_is_brains_server "$name" || + ! corigin=$(brains_origin "$url") || + [ "$corigin" != "$want" ]; then + [ "\${BRAINS_CRED_CLIENT:-claude}" = "codex" ] && _brains_discard "$recfile" + continue`, + probe: `printf '%s' '[{"server_name":"brains","url":"'"$ORIGIN"'/mcp","token_response":{"access_token":"tok-mine"}},{"server_name":"other","url":"'"$ORIGIN"'/mcp","token_response":{"access_token":"tok-theirs"}}]' > "$BRAINS_STATE_DIR/mixed.json" +export BRAINS_CODEX_CREDENTIALS_FILE="$BRAINS_STATE_DIR/mixed.json" +. "$LIB"; BRAINS_CRED_CLIENT=codex +brains_resolve_credential "$ORIGIN" >/dev/null 2>&1 +printf '%s/%s' "$BRAINS_CRED_STATE" "$BRAINS_CRED_COUNT"`, + requires: "ok/1", }, ]; { @@ -1250,13 +1931,14 @@ printf '%s/%s' "$BRAINS_CRED_STATE" "$BRAINS_CRED_TRUNCATED"`, ORIGIN, OUTSIDE: join(work, "outside"), STUBS: stubDir, + HITS, ...(m.env ?? {}), }; const r = spawnSync("bash", ["-c", `set -u\n${m.probe}`], { env, encoding: "utf8", timeout: 30000 }); return (r.stdout ?? "").trim(); }; const before = runProbe(); - if (m.requires !== undefined && before !== m.requires) { + if (before !== m.requires) { // Loudly, and as a FAILURE of the gate's own preconditions rather than a // pass: the mechanism may be fine, but nothing here observed it. rmSync(work, { recursive: true, force: true }); diff --git a/tests/credential/stubs.js b/tests/credential/stubs.js index be06a96..385fece 100644 --- a/tests/credential/stubs.js +++ b/tests/credential/stubs.js @@ -6,25 +6,48 @@ // that process could never answer the request it exists to answer. The suite // would deadlock rather than fail, and a hang reports nothing at all. // -// Usage: node stubs.js +// The ports are EPHEMERAL and reported back through a file. Fixed ports made +// the suite non-deterministic: a previous run's stubs are still bound for a +// moment after it exits, so the next run's listen() failed while its readiness +// probe was answered by the dying process — requests then landed in the old +// run's receipt log and assertions about "did this arrive" read zero. Two runs +// at once, which is all a CI runner has to do, produced the same thing. Letting +// the kernel choose removes the shared resource rather than timing around it. +// +// Usage: node stubs.js const http = require("http"), net = require("net"), fs = require("fs"); const LOG = process.argv[2]; -const PRIMARY = Number(process.argv[3]), OTHER = Number(process.argv[4]), TRUNCATOR = Number(process.argv[5]); +const PORTS = process.argv[3]; const log = (port, req, body) => fs.appendFileSync(LOG, JSON.stringify({ port, method: req.method, path: (req.url || "").split("?")[0], auth: req.headers.authorization || "", bodyLen: body.length, }) + "\n"); -const mk = (port, handler) => http.createServer((req, res) => { - let b = ""; - req.on("data", (c) => { b += c; }); - req.on("end", () => { - log(port, req, b); - const r = handler(req.url || ""); - res.writeHead(r.code, { "content-type": "application/json" }); - res.end(r.body); +const ready = {}; +const announce = () => { + if (Object.keys(ready).length !== 4) return; + // Written whole and renamed: a reader polling for this file must never see a + // half-written object and parse it as a port number. + fs.writeFileSync(`${PORTS}.tmp`, JSON.stringify(ready)); + fs.renameSync(`${PORTS}.tmp`, PORTS); +}; +const listen = (name, server, onListen) => { + server.on("error", (e) => { process.stderr.write(`${name}: ${e.message}\n`); process.exit(1); }); + server.listen(0, "127.0.0.1", () => { ready[name] = server.address().port; (onListen || (() => {}))(); announce(); }); +}; +const mk = (name, handler) => { + const server = http.createServer((req, res) => { + let b = ""; + req.on("data", (c) => { b += c; }); + req.on("end", () => { + log(ready[name], req, b); + const r = handler(req.url || ""); + res.writeHead(r.code, { "content-type": "application/json" }); + res.end(r.body); + }); }); -}).listen(port, "127.0.0.1"); -mk(PRIMARY, (p) => { + listen(name, server); +}; +mk("primary", (p) => { if (p.startsWith("/forbidden")) return { code: 403, body: '{"error":"forbidden"}' }; // Returns something the inbox engine must ACK, so a test can exercise the // acknowledgement path — and therefore its credential lease. @@ -33,14 +56,17 @@ mk(PRIMARY, (p) => { if (p.startsWith("/inbox/claude")) return { code: 200, body: JSON.stringify({ actions: [] }) }; return { code: 200, body: '{"ok":true}' }; }); -mk(OTHER, () => ({ code: 200, body: '{"ok":true}' })); +mk("other", () => ({ code: 200, body: '{"ok":true}' })); // Valid headers plus a COMPLETE, parseable JSON prefix, then the socket dies. // curl reports 200 for this, because the status is written the moment headers // arrive — only its exit status reveals that the body never finished. -net.createServer((s) => { +listen("truncator", net.createServer((s) => { s.on("data", () => { s.write("HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: 400\r\n\r\n"); s.write('{"device_id":"d1"}'); setTimeout(() => s.destroy(), 80); }); -}).listen(TRUNCATOR, "127.0.0.1"); +})); +// Accepts and never answers: the only way to show that a call site without its +// own ceiling would hang forever. +listen("blackhole", net.createServer(() => {})); diff --git a/tests/plugin-contract/run.ts b/tests/plugin-contract/run.ts index e6d33f5..338759d 100644 --- a/tests/plugin-contract/run.ts +++ b/tests/plugin-contract/run.ts @@ -362,9 +362,15 @@ const CODEX_INSTALL_REGION = [ "runs automatic recall and error feedback. Capture and inbox delivery use the", "sign-in above as their credential, so there is nothing further to set.", "", - "Capture and the inbox are **macOS only** for Codex: they read the credential", - "from the macOS keychain, and Codex on Linux is not a supported configuration.", - "The tools and recall still work there; capture and inbox delivery do not.", + // "capture and inbox delivery do not [work there]" was false: the explicit-token + // branch is platform independent and captures on Linux exactly as on macOS. The + // product decision stands — what changed is that the copy now says which part + // is unavailable (the sign-in) rather than claiming the feature is. + "Capture and the inbox are **macOS only** for Codex: they read the sign-in from", + "the macOS keychain, so on Linux there is none for them to read and they stay", + "off. The tools and recall are unaffected. The optional capture token below does", + "drive capture there, but Codex on Linux is not a configuration brains supports", + "or tests.", "", "Everyday reading and writing is covered by default. For admin-gated tools or", "performance insights, sign in asking for them explicitly (both also need the", @@ -2309,75 +2315,211 @@ assert( "README must be exactly the approved regions, in order, with nothing between them — every byte this file publishes belongs to one constant here, so text added at a seam or in a section no pin covers fails right here", ); -assert( - turnHook.includes('CLIENT="claude"'), - "shared turn hook must default Claude Code captures to the Claude CLI", -); -assert( - turnHook.includes('[ -n "${PLUGIN_ROOT:-}" ] && CLIENT="codex"'), - "shared turn hook must identify the Codex plugin runtime as the Codex CLI", -); +// WHY CODE-ONLY REGIONS, not substrings — the same lesson as the skill artifacts +// above, learned again on this file's own assertions. +// +// These pins were `credLib.includes("ambiguous")` and friends. A substring +// match over a shell file is satisfied by a COMMENT, and three of them already +// were: gutting every non-comment line that mentioned "ambiguous" left the +// assertion "resolver must refuse when more than one distinct candidate +// matches" passing, carried entirely by the prose above it. The two "must +// source the shared credential resolver" pins were being carried by a +// `# shellcheck source=` directive — so the guard against the two hooks +// re-growing separate credential chains, which is the whole reason this task +// exists, asserted nothing. +// +// codeOnly() strips comments and blank lines before matching, so a comment +// CANNOT satisfy one of these by construction. That is the property, not a +// stricter string. +const codeOnly = (text: string): string => + text.split("\n").filter((l) => l.trim() !== "" && !/^\s*#/.test(l)).join("\n"); + +const TURN_CLIENT_DETECTION_REGION = [ + "CLIENT=\"claude\"", + "[ -n \"${PLUGIN_ROOT:-}\" ] && CLIENT=\"codex\"", +].join("\n"); + +const TURN_SOURCES_RESOLVER_REGION = [ + "[ -r \"$CRED_LIB\" ] || exit 0", + ". \"$CRED_LIB\" || exit 0", +].join("\n"); + +const INBOX_SOURCES_RESOLVER_REGION = [ + "[ -r \"$CRED_LIB\" ] || exit 0", + ". \"$CRED_LIB\" || exit 0", +].join("\n"); + +// Three things this region has to hold at once, and each of them was a defect +// before it was a pin. The separator is hoisted and the fork-free predicate is +// tested first, because two command substitutions per entry cost seconds on a +// store the document ceiling now admits. The loop consults the total budget, so +// the ceiling cannot make it unbounded. And a non-matching row may only discard +// a record file PRIVATE to it — keyed on shared-ness, not on the client, since +// the client check let the array backend delete the snapshot every row shares. +const RESOLVER_SELECTION_REGION = [ + " tab=$(printf '\\t')", + " while IFS=\"$tab\" read -r key url name recfile; do", + " [ -n \"$key\" ] || continue", + " BRAINS_CRED_SAW_ENTRIES=1", + " if ! _brains_budget_left; then", + " BRAINS_CRED_TRUNCATED=1", + " [ \"$recfile\" != \"${store:-}\" ] && _brains_discard \"$recfile\"", + " continue", + " fi", + " if ! _brains_is_brains_server \"$name\" ||", + " ! corigin=$(brains_origin \"$url\") ||", + " [ \"$corigin\" != \"$want\" ]; then", + " [ \"$recfile\" != \"${store:-}\" ] && _brains_discard \"$recfile\"", + " continue", + " fi", + " matched=\"$matched$key", + "\"", + " matchedfiles=\"$matchedfiles$recfile", + "\"", + " count=$((count + 1))", + " done </dev/null | cut -c1-8)", + " [ -n \"$hash\" ] || return 1", + " printf 'Claude Code-credentials-%s' \"$hash\"", + "}", +].join("\n"); + +const assertRegion = (source: string, region: string, message: string): void => + assert(codeOnly(source).includes(region), message); + +// Client detection: which CLI a capture is attributed to. Codex defines +// PLUGIN_ROOT; Claude Code invokes the explicit hook map without it. +assertRegion(turnHook, TURN_CLIENT_DETECTION_REGION, + "turn hook must detect the client exactly as approved — Claude by default, Codex when the plugin runtime sets PLUGIN_ROOT"); assert( turnHook.includes('client:$client, client_type:"cli"'), "turn ingest payload must include the detected client and CLI type", ); -// The credential chain moved out of the turn hook into a shared resolver, because the two hooks -// had drifted: the turn hook scavenged a Codex MCP header and the inbox engine did not, so a Codex -// user with header auth had capture ON and the inbox OFF. Both now source the same file, and that -// is what these assertions hold — a second private chain reappearing in either hook is the -// regression, and it cannot be caught by looking at only one of them. + +// ONE resolver, sourced by BOTH hooks. They had drifted: the turn hook +// scavenged a Codex MCP header and the inbox engine did not, so a Codex user +// with header auth had capture ON and the inbox OFF. A second private chain +// re-appearing in either hook is the regression, and it cannot be caught by +// looking at only one of them. +assertRegion(turnHook, TURN_SOURCES_RESOLVER_REGION, + "turn hook must source the shared credential resolver, not build its own chain"); +assertRegion(inboxHook, INBOX_SOURCES_RESOLVER_REGION, + "inbox engine must source the shared credential resolver, not build its own chain"); for (const [name, hook] of [["turn hook", turnHook], ["inbox engine", inboxHook]] as const) { - assert( - hook.includes("brains-credential.sh"), - `${name} must source the shared credential resolver, not build its own chain`, - ); assert( !/Authorization: Bearer/.test(hook), `${name} must not attach an Authorization header itself — brains_request owns the credential, so no other code can send it to an unchecked host`, ); } -assert( - credLib.includes("codex mcp get"), - "resolver must still reuse a persisted Codex MCP Authorization header when no token env is present", -); -assert( - credLib.includes(".transport.http_headers.Authorization"), - "resolver must read the configured MCP Authorization header", -); -// Measured on codex-cli 0.147.0: with the plugin installed, `codex mcp get brains --json` reports -// `"http_headers": null`, because Codex stores its OAuth credential in the keychain instead. The -// header scavenge is therefore correct only for someone who ran `codex mcp add --header ...` by -// hand, and the keychain read is what covers everyone else. -assert( - credLib.includes("Codex MCP Credentials"), - "resolver must read the Codex MCP OAuth keychain — the header scavenge alone reaches almost nobody", -); -assert( - credLib.includes("Claude Code-credentials"), - "resolver must read the Claude Code credential store", -); -// Selection is by ORIGIN, never by server name. This machine's Codex store holds three entries all -// named "brains" — two dead localhost stubs and one STAGE — so a name match picks arbitrarily -// between a token for a dead server and a stage token pointed at production. -assert( - credLib.includes("brains_origin"), - "resolver must select stored credentials by canonical origin", -); -// Neither store records WHICH ACCOUNT a token belongs to, so two candidates for one origin cannot -// be told apart. Capturing into the wrong brain is worse than a 401, because a 401 is detectable. -assert( - credLib.includes("ambiguous"), - "resolver must refuse when more than one distinct candidate matches, rather than guess", -); -// Transport truth. curl writes %{http_code} as soon as headers arrive, so a transfer that dies -// mid-body still reports 200 — and the truncated body can be valid JSON that parses. Measured: -// curl_exit=18, http_code=200, body {"device_id":"d1"}. Only the exit status separates them. -assert( - /crc=\$\?/.test(credLib) && credLib.includes('[ "$crc" -ne 0 ]'), - "brains_request must treat any non-zero curl exit as a transport failure regardless of HTTP code", -); -// A universal -o /dev/null would have silently killed device-id caching, drift nudges and the -// whole inbox delivery path while every status-code assertion stayed green. + +// Selection: by canonical ORIGIN *and* server identity, never by name alone. +// This machine's Codex store holds three entries all named "brains" — two dead +// localhost stubs and one STAGE — so a name match picks arbitrarily between a +// token for a dead server and a stage token pointed at production. +assertRegion(credLib, RESOLVER_SELECTION_REGION, + "resolver must select stored credentials by canonical origin AND brains server identity"); + +// The refusal. Neither store records WHICH ACCOUNT a token belongs to, so two +// candidates for one origin cannot be told apart; capturing into the wrong +// brain is worse than a 401, because a 401 is detectable. An incomplete scan is +// the same answer for the same reason — it might have found a second one. +assertRegion(credLib, RESOLVER_DECISION_REGION, + "resolver must refuse when more than one distinct candidate matches, or when the scan was incomplete, rather than guess"); + +// Transport truth. curl writes %{http_code} as soon as headers arrive, so a +// transfer that dies mid-body still reports 200 — and the truncated body can be +// valid JSON that parses. Measured: curl_exit=18, http_code=200, body +// {"device_id":"d1"}. Only the exit status separates them. +assertRegion(credLib, RESOLVER_TRANSPORT_TRUTH_REGION, + "brains_request must treat any non-zero curl exit as a transport failure regardless of HTTP code"); + +// The Claude store's service name is a factual contract with Claude Code, and +// the suffix rule for a non-default config dir is what keeps one profile from +// reading another's credential. Neither is exercised by the fixture-file path +// the credential suite uses, so it is pinned as authored code here. +assertRegion(credLib, RESOLVER_CLAUDE_SERVICE_REGION, + "resolver must derive the Claude Code credential service exactly as the client does, and probe no alternative"); + +// And a guard against this class regrowing. A POSITIVE substring match over a +// shell file is satisfiable by a comment, which is how three pins here came to +// assert nothing; the fix is codeOnly()/assertRegion(), and this makes reaching +// for the weaker form fail loudly instead of merging quietly. Negative matches +// are exempt: a comment containing the banned text FAILS them, so they are +// over-strict rather than vacuous — the opposite failure mode. +{ + const selfSource = readFileSync(join(ROOT, "tests", "plugin-contract", "run.ts"), "utf8"); + const APPROVED_POSITIVE_SUBSTRING_PINS = [ + // Held by code — the string appears in no comment, and gutting it fails the + // assertion. Verified by mutation. + `turnHook.includes('client:$client, client_type:"cli"')`, + ]; + const offenders = selfSource + .split("\n") + .map((line, i) => ({ line: line.trim(), n: i + 1 })) + .filter(({ line }) => /^(credLib|turnHook|inboxHook|hook)\.includes\(/.test(line)) + .filter(({ line }) => !APPROVED_POSITIVE_SUBSTRING_PINS.some((ok) => line.startsWith(ok))); + assert( + offenders.length === 0, + `positive substring pins over shipped shell source are satisfiable by a comment — use assertRegion() with a codeOnly region instead, or add a mutation-proved exception to APPROVED_POSITIVE_SUBSTRING_PINS: ${offenders.map((o) => `:${o.n} ${o.line}`).join("; ")}`, + ); +} + +// A universal -o /dev/null would have silently killed device-id caching, drift +// nudges and the whole inbox delivery path while every status-code assertion +// stayed green. Negative pins cannot be satisfied by a comment — a comment +// containing the banned text FAILS them — so these stay as substring bans. assert( !credLib.includes("-o /dev/null"), "brains_request must preserve response bodies — the device report and inbox GET are parsed from them", @@ -2582,12 +2724,26 @@ try { ); } else { assert( - /does not support Codex on this platform/.test(firstSignal), - "without the macOS keychain the Codex off-state must say the platform is unsupported", + /reads the Codex sign-in from the macOS keychain/.test(firstSignal) && + /not a configuration brains supports/.test(firstSignal), + "without the macOS keychain the Codex off-state must say why the sign-in cannot be read", + ); + assert( + !/mcp login/.test(firstSignal), + "…and must name no sign-in step, because there is none to read on this platform", + ); + // It must not say there is nothing to change, either. BRAINS_API_TOKEN + // resolves and captures here — the branch is platform independent — so the + // old wording was false in a note core.md tells the agent is authoritative. + assert( + !/nothing to change/.test(firstSignal), + "…and must not claim nothing would change it, which is measurably false", ); + // No action, so no offer protocol: a note that says there is nothing to do + // and then tells the agent to offer it leaves the agent to invent one. assert( - !/mcp login/.test(firstSignal) && !/BRAINS_API_TOKEN/.test(firstSignal), - "…and must offer no remedy, because there is no supported action to take", + !/Offer this to the user/.test(firstSignal) && /nothing for you to offer/.test(firstSignal), + "…and a note with no action must not carry the offer tail", ); } const secondSignal = spawnSync(