Skip to content
This repository was archived by the owner on Jul 16, 2026. It is now read-only.

feat(vault): bridge-level secret interception + vault CLI skill - #65

Open
bassilkhilo-ag2 wants to merge 7 commits into
mainfrom
fix/vault-stando-pr
Open

bassilkhilo-ag2 wants to merge 7 commits into
mainfrom
fix/vault-stando-pr

Conversation

@bassilkhilo-ag2

Copy link
Copy Markdown

Summary

2 commits only — vault feature on top of stando's current main.

  • Bridge-level secret interception: vault set KEY VALUE commands sent via Slack or Discord are intercepted before the task file is written. The actual value goes straight to macOS Keychain. Task files only ever see [STORED-IN-KEYCHAIN].
  • Key manifest (~/.sutando-vault/keys.json): tracks stored key names (never values) so keys are enumerable.
  • vault CLI skill (skills/vault/vault.py): three subcommands:
    • vault list — show all stored key names
    • vault get KEY — retrieve value from Keychain
    • vault env KEY... -- CMD — run CMD with vault keys injected as env vars
  • get_vault_key(KEY) helper in vault_intercept.py — for any skill that needs a secret at runtime.

Use case

Bassil is on a walk. Opens Apollo on his iPhone, generates an API key, sends it to Sutando via Slack. The bridge intercepts it to Keychain. Sutando builds the GTM agent using get_vault_key('APOLLO_API_KEY'). He passes his Railway key, Sutando deploys — he can see the app live while sitting by the lake.

Secret lifecycle: Slack/Discord API → bridge (in-memory, SSL) → Keychain (encrypted at rest). Disk never sees plaintext.

Test plan

  • 16 tests in tests/vault-skill.test.py — all passing
  • 15 tests in tests/vault-intercept.test.py — all passing
  • Task files after vault set show [STORED-IN-KEYCHAIN] placeholder
  • vault list shows stored key names
  • vault env KEY -- CMD injects key as env var

🤖 Generated with Claude Code

Bassil Khilo and others added 6 commits May 24, 2026 18:08
…ored to Keychain before task file write

Adds src/vault_intercept.py: intercepts 'vault set KEY VALUE' patterns
in incoming Slack/Discord messages BEFORE writing to task files on disk.
Secrets go to macOS Keychain (security add-generic-password); task file
gets 'vault set KEY [STORED-IN-KEYCHAIN]' placeholder.

Secret lifecycle: Slack/Discord API → bridge (in-memory, SSL in transit)
→ Keychain (encrypted at rest). Disk never sees plaintext.

- src/vault_intercept.py: shared intercept module (regex, Keychain write,
  NamedTuple result). Supports bare, double-quoted, and single-quoted values.
  Multiple vault-set commands in one message all intercepted in a single pass.
- src/slack-bridge.py: calls intercept_vault_commands() in _write_task()
  before user_task_text is built. Logs stored key names; logs but does not
  crash on Keychain failure (task still goes through sanitized).
- src/discord-bridge.py: same pattern before user_task_text is built.
- tests/vault-intercept.test.py: 15 tests (no real Keychain writes — mocked).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extends vault_intercept.py with:
- Key manifest at ~/.sutando-vault/keys.json (names only, no values)
  updated on every store so keys are enumerable without Keychain dump
- get_vault_key(key) helper for scripts and skills that need a secret
- list_vault_keys() returns sorted key names from the manifest

Adds skills/vault/vault.py — a CLI with three subcommands:
  vault list                   — show all stored key names
  vault get KEY                — retrieve value from Keychain
  vault env KEY... -- CMD      — run CMD with vault keys as env vars

Use case: Bassil passes an API key via Slack, bridge intercepts it to
Keychain, then any skill can call `get_vault_key('KEY')` or spawn a
subprocess with `vault env KEY -- python3 my_script.py` — key never
touches a config file or task file.

16 tests in tests/vault-skill.test.py, all passing.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Documents the vault secret store so every future session knows to call
get_vault_key("KEY") instead of asking for .env entries or hardcoding
secrets. Covers list, get, and env CLI subcommands.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…iew)

- _replacer now catches RuntimeError internally — plaintext is always
  redacted from the returned text even when Keychain write fails.
  On failure, inserts [VAULT-STORE-FAILED] placeholder instead of
  [STORED-IN-KEYCHAIN] — secret never reaches disk either way.
- InterceptResult gains a third field: failed: list[str] — keys that
  could not be stored. Bridges log these without re-exposing values.
- _register_key uses temp-file + os.replace for atomic manifest writes
  — concurrent Slack + Discord intercepts can no longer corrupt keys.json.
- Added ps-visibility comment on security argv call (known limitation).
- Updated bridges to use attribute access (result.text / result.stored /
  result.failed) instead of tuple unpacking.
- Replaced test_keychain_failure_raises_runtime_error with two new tests:
  test_store_failure_still_redacts and test_partial_failure_redacts_all.
  All 32 vault tests pass.
… + telegram bridge

Addresses Maddy's review on sutando OSS PR sonichi#1084 (blocking + 3 non-blocking):

BLOCKING fixed:
- discord-bridge: vault interception now guarded by `access_tier == "owner"` check.
  Previously intercept_vault_commands() fired before the team/other sandbox gate,
  letting untrusted Discord senders write arbitrary keys to the owner's Keychain.
  Non-owner messages get redact_vault_commands() instead (pattern scrubbed, no write).
- slack-bridge: vault interception moved to AFTER access_tier is resolved from
  tierMap (was called 40+ lines too early, before tier_map lookup). Same owner-only
  gate applied.

Non-blocking fixed:
- vault_intercept: regex extended to match backtick-quoted values (`secret`).
  Discord users typing code-formatted secrets no longer get backticks stored verbatim.
- vault_intercept: empty value rejected with [VAULT-EMPTY-VALUE] placeholder.
- telegram-bridge: vault interception added (was the only bridge without it).
  All Telegram senders are owner-tier (bot token allowlist); no tier gate needed yet.
- redact_vault_commands() added to vault_intercept.py for non-owner scrubbing.

Tests: 26/26 passing (10 new tests covering backtick, empty value, redact variants).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@bassilkhilo-ag2

Copy link
Copy Markdown
Author

Security review addressed (f52525c cherry-picked from sonichi#1084 review by @liususan091219):

Blocking fixed:

  • access_tier gate — discord-bridge: vault interception now guarded by access_tier == "owner". Non-owner Discord senders previously could write arbitrary keys to the owner's Keychain. Non-owner messages get redact_vault_commands() — pattern scrubbed, no Keychain write.
  • access_tier gate — slack-bridge: vault interception moved to after access_tier is resolved from tierMap (was called 40+ lines before the tier lookup). Same owner-only gate.

Non-blocking also fixed:

  • Backtick-quoted values (secret) now matched and backticks stripped before Keychain write
  • Empty values (vault set FOO "") rejected with [VAULT-EMPTY-VALUE]
  • Telegram bridge: vault interception added (was the only bridge without it)
  • redact_vault_commands() added for non-owner scrubbing without Keychain write

Tests: 26/26 passing, 10 new tests.

@bassilkhilo-ag2

Copy link
Copy Markdown
Author

Mirrors sutando OSS sonichi#1084 — same implementation.

LGTM:

  • Intercept point is AFTER access_tier resolution — non-owner messages get redact_vault_commands (Keychain untouched), owner messages get full intercept_vault_commands. Correct ordering.
  • The ps visibility note for the security subprocess is accurate and acceptable on single-user Mac.
  • Keychain store is atomic + -U handles updates idempotently.
  • Manifest at ~/.sutando-vault/keys.json stores key names only, never values.
  • Test suite mocks the Keychain subprocess so CI stays clean.
  • All 3 bridges (Discord/Slack/Telegram) covered.

…lt intercept block

The vault PR adds ~1.2k chars to _write_task. The {0,6000}? capture budget
no longer reaches the terminating double-blank-line, causing
'_write_task function not found'. Bump to 8000.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@bassilkhilo-ag2

Copy link
Copy Markdown
Author

Fixed the CI failure. Root cause:

slack-bridge-access.test.py was added to main after this PR branch was created. The test's _write_task regex uses a {0,6000}? capture budget. The vault PR adds ~1.2k chars to _write_task (owner/non-owner intercept branch), pushing the function body from ~5.4k to ~6.6k chars — past the budget. The regex engine can't reach the terminating \n\ndef boundary and reports '_write_task function not found'.

Fix: committed {0,6000}? → {0,8000}? directly to this branch. New CI run started.

@bassilkhilo-ag2

Copy link
Copy Markdown
Author

Tagging @VasiliyRad for review — this is the Stando-side counterpart to sonichi#1084 (vault bridge interception). CI is passing. The key security invariant: vault values never touch disk in task files — bridge intercepts vault set KEY VALUE, writes to Keychain, task file only sees [STORED-IN-KEYCHAIN].

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant