Security hardening: OS credential store, human consent for writes, encrypted cache

[AdelsJarlen]

Security hardening, not remediation. Upstream was audited and found clean — no
malware, no exfiltration, a single pinned endpoint. This closes a mundane but
real exposure and adds a consent gate on writes.

What changed

The token moves out of plaintext config. The model never had access to it —
this server is a local subprocess and the LLM only sees tool return values. The
actual leak was claude mcp add-json ... "env":{"YNAB_API_KEY":"..."}, which
writes the token in plaintext into ~/.claude.json or
claude_desktop_config.json. Any agent with a file-read tool, including Claude
Code itself, can open that file. It now lives in the OS credential store via
keyring, with mcp-ynab auth login to put it there.

Every write requires human confirmation, with a real diff. All 16 mutating
tools now elicit approval before sending anything to YNAB. The security property
is that the "before" shown to the human comes from the YNAB API, not from
anything the model said. Two things hold that up:

• A tool parameter annotated Context is injected by the SDK and does not
  appear in the tool's input schema, so the model cannot supply, forge or
  suppress it. Consent cannot be satisfied by anything the model emits.
• Pre-images are read through _shared.client, bypassing the cache — a stale
  row would show a false "before". Each call site fingerprints the resolved
  payload before eliciting and asserts it afterwards, so nothing is re-resolved
  between the prompt and the write.

Bulk operations itemise every row with its own diff rather than summarising a
count, because "12 transactions" is exactly the prompt a person waves through
unread. import_transactions has no knowable diff, so it names the linked
accounts and says plainly that the result cannot be previewed.

The cache is encrypted at rest. It holds payees, memos, amounts and category
names. Payload columns are AES-256-GCM with the key in the credential store,
implemented as a SQLAlchemy TypeDecorator so src/cache/service.py is
untouched.

Opt-in read-only mode. YNAB_READ_ONLY=1 drops the 16 write tools at
registration time, so they never reach the model's tool list. Off by default.

Honest limits (all stated in SECURITY.md)

• DPAPI/Keychain/Secret Service scope secrets to the logged-in user, not to
  a program. Code running as the same user can still retrieve the token. Closing
  that needs a separate OS account, a hardware token, or a broker process —
  none of which a pip-installable server can deliver.
• The cache AAD is mcp-ynab:<table>.<column>, which binds ciphertext to its
  table and column but not to its row. Someone with DB write access could
  swap ciphertexts between rows of one table. Acceptable (they already have
  write access) and documented rather than hidden.
• Lookup and delta-sync columns stay plaintext, leaking entity counts and sync
  timing but no amounts, payees, memos or category names.
• 0600 on POSIX only; on Windows the file inherits the user-profile ACL.

Reviewer notes

The consent gate is only as strong as the client's elicitation support. On a
client that does not implement elicitation/create, writes now refuse outright
rather than degrading. Verified in practice: Claude Code's terminal CLI supports
it (2.1.76+); Claude Desktop does not (anthropics/claude-code#41110), and the
desktop Code tab advertises the capability then auto-declines without rendering
a prompt (anthropics/claude-code#94450). YNAB_READ_ONLY=1 is the clean
configuration for those.

Two deviations from the original brief, both deliberate:

• allow_env_key defaults to true, not false. The brief specified = False
  while also requiring the env var be permitted with a deprecation warning;
  those contradict. Defaulting it off would break scripts/smoke_test.py and
  every existing install. YNAB_ALLOW_ENV_KEY=0 gives the strict behaviour.
• crypto.encrypt() produces exactly nonce(12) || ciphertext || tag as
  specified, but the storage layer prefixes one framing byte so
  YNAB_CACHE_ENCRYPTION=0 can be toggled without stored rows decoding as
  garbage.

auth login --stdin also widens the brief, which said the token must only come
from getpass. It keeps the token out of argv and shell history — the
property that actually mattered — and exists because pasting into a hidden
prompt is broken on the Windows console (see below).

A migration bug worth knowing about. The first version of the cache
migration dropped cached_entity but left server_knowledge. Delta sync would
then have believed it was current, fetched only changes since that knowledge,
and the dropped entities would never have returned. The two now drop together.

Tool surface is unchanged: 47 tools, identical names and parameter schemas,
no ctx leaking into any input schema. exclude_fields behaviour and
FIELDS.md semantics are preserved.

Verification

• uv run pytest — 153 passed (105 pre-existing, 48 new in
  tests/test_security.py). Existing tests keep their assertions on exactly
  which payload reaches the cache service; they only gained the fixtures the
  gate requires.
• python scripts/smoke_test.py — passes on highest (resolves mcp 2.2.0) and
  --resolution lowest-direct (mcp 2.0.0, keyring 25.0, cryptography 43.0).
• Driven end to end over real stdio as a client advertising elicitation: the
  prompt renders, decline blocks the write, a client without elicitation is
  refused, and YNAB_READ_ONLY=1 serves 31 tools instead of 47.
• Encrypted round trip verified against a real SQLite file: payload stored as an
  opaque BLOB, lookup columns intact, plaintext absent from the bytes on disk.

The last two commits fix bugs found while setting this up on Windows for the
first time: a Ctrl+V control character reaching the Authorization header and
producing an opaque HTTP 400, a sys.stdin.isatty() check that returns true
under Git Bash even when redirected (so auth login hung forever instead of
erroring), and PowerShell's UTF-8 BOM breaking --stdin.

🤖 Generated with Claude Code

[AdelsJarlen]

Opened against the wrong repository by mistake — this belongs on the fork, not upstream. Closing immediately; apologies for the noise.