Skip to content

fix(local,keychain,api): prefer keychain for secrets, disable stdio MCP by default, surface approval-record failures - #1891

Open
ra-co88 wants to merge 1 commit into
UsefulSoftwareCo:mainfrom
ra-co88:fix/secrets-provider-defaults
Open

fix(local,keychain,api): prefer keychain for secrets, disable stdio MCP by default, surface approval-record failures#1891
ra-co88 wants to merge 1 commit into
UsefulSoftwareCo:mainfrom
ra-co88:fix/secrets-provider-defaults

Conversation

@ra-co88

@ra-co88 ra-co88 commented Aug 30, 2026

Copy link
Copy Markdown

What

Three defaults change in the shipped local config:

  1. The keychain plugin registers before the file store — minted OAuth tokens default to the OS keychain where one is reachable.
  2. dangerouslyAllowStdioMCP defaults to false — stdio MCP servers are off unless explicitly enabled.
  3. Approval-record persistence failures are surfaced through the error-capture channel instead of being silently swallowed.

Why

Tokens defaulted to the plaintext file store even on platforms with a durable keychain — the weaker storage became the default by registration order, not by decision. Stdio servers spawn local processes; that is not a default a security-conscious distribution should ship. And a swallowed approval-record failure meant a pause that never recorded its approval — invisible breakage.

What changed

  • describeKeychainAvailability() encodes the platform truth (reachable/unreachable/unsupported) driving the ordering; the config consults it.
  • The MCP plugin already rejects stdio with a clear error when disabled — the default now takes the safe side.
  • recordPendingApproval failures report through the host's error capture; execution behavior is unchanged (the pause still returns paused, not a 500).

Headless/Linux behavior is unchanged: where the keychain probe fails, the file store remains the default.

Test plan

  • Availability helper platform-truth encoding (3 tests).
  • Config contract: ordering + stdio default (3 tests).
  • Approval-record failure surfacing (2 tests).

All green against current main.

…CP by default, surface approval-record failures
@ra-co88
ra-co88 force-pushed the fix/secrets-provider-defaults branch from bff2366 to 4400942 Compare August 30, 2026 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants