feat(vault): add read-secret CLI command (swamp-club#238)

[stack72]

Summary

• Add swamp vault read-secret <vault> <key> CLI command that retrieves and displays secret values from vaults
• Wires existing VaultService.get() to a CLI surface — no VaultProvider interface changes, every vault already implements get() for CEL expression resolution
• Adds VaultSecretRead domain event for audit trail, --force flag for non-interactive use, and skips confirmation in --json mode for agent consumption
• Fixes vault skill quick reference table that incorrectly referenced swamp vault get <vault> <key> (which doesn't exist)

Test plan

• 6 unit tests for libswamp generator (happy path, vault not found, no vaults configured, empty vault name, empty key, secret not found)
• deno check passes
• deno lint passes
• deno fmt passes
• deno run compile succeeds
• UAT issue to be filed in swamp-uat for CLI coverage
• Docs issue to be filed in swamp-club for content/manual/reference/vaults.md

Closes swamp-club#238

🤖 Generated with Claude Code

[github-actions]

CLI UX Review

Blocking

None.

Suggestions

1. Log-mode output has no label for the secret value — src/presentation/renderers/vault_read_secret.ts (line 37) emits only the raw value via logger.info${e.data.value}``. After confirming the interactive prompt, the user sees just e.g. sk-1234567890 with no indication of which key or vault it came from. This is intentionally pipeable, but every other vault renderer includes context (`vault_put`: "Stored secret API_KEY in vault my-vault", `vault_list_keys`: "Vault dev-secrets (local_encryption): 3 key(s)"). Consider a format like `API_KEY (my-vault): sk-1234567890` in log mode only — or accept the tradeoff since bare-value output is useful for `export KEY=$(swamp vault read-secret ...)`.

2. Quick-reference table lists --force --json together — .claude/skills/swamp-vault/SKILL.md table row shows swamp vault read-secret <vault> <key> --force --json but --force is redundant with --json (JSON mode already skips the prompt). The pairing implies --force is required for JSON mode, which is misleading. Consider splitting into two rows or using just --json for the agent pattern.

3. Cancelled path exits with 0 — when the user declines the confirmation in src/cli/commands/vault_read_secret.ts (line 83), the command returns normally with cliCtx.logger.infoCancelled.``. Compare vault_put, which emits `{ cancelled: true }` in JSON mode and a distinct log message. Here cancellation only occurs in log mode so it's functionally correct, but logging "Cancelled." and exiting 0 means a script wrapping the command cannot distinguish a successful read from a user abort without inspecting stdout. Not blocking, but worth noting.

Verdict

PASS — No blocking issues. The command is well-structured, error messages are clear and actionable (vault-not-found shows available vaults, key-not-found links to list-keys), -f/--force and JSON auto-skip are consistent with vault_migrate and vault_put patterns, and the JSON output shape is complete and documented.

[github-actions]

Code Review

PR adds swamp vault read-secret <vault> <key> CLI command that wires the existing VaultService.get() to a CLI surface with an audit trail and safety prompt.

Blocking Issues

None.

DDD Assessment

• Domain Event: VaultSecretRead is properly modeled in the domain events layer, capturing vault identity and secret key for audit. Correctly omits the secret value from the event — good security practice.
• Application Service: The vaultReadSecret async generator in libswamp follows the application service pattern, orchestrating vault config lookup, secret retrieval, and event publishing. Properly lives in the application layer.
• Dependency Injection: VaultReadSecretDeps abstracts infrastructure behind a narrow interface, enabling the clean test doubles in the test file.
• Aggregate Boundaries: Respected — uses VaultService and VaultConfigRepository without crossing aggregate boundaries.

Convention Compliance

• ✅ AGPLv3 license headers on all new .ts files
• ✅ CLI and presentation layers import from src/libswamp/mod.ts (not internal paths)
• ✅ Both log and json output modes supported
• ✅ LogTape tagged templates used correctly (bare interpolation for values, function call for static strings)
• ✅ 6 unit tests covering happy path, vault not found, no vaults configured, empty vault name, empty key, secret not found
• ✅ AnyOptions pattern matches established convention across vault commands
• ✅ CI passing on both Ubuntu and Windows
• ✅ VaultSecretRead properly added to RepositoryEvent union type

Suggestions

1. promptConfirmation duplication — This is now copy-pasted in 4 files (vault_put.ts, vault_read_secret.ts, extension_pull.ts, data_delete.ts). A shared utility in src/cli/ would reduce drift. Non-blocking since it matches current codebase convention.

2. Local VaultConfigInfo interface — read_secret.ts defines its own VaultConfigInfo with {id, name, type}, while get.ts already exports a superset. Could import and reuse it, though the narrow local interface is a valid interface-segregation choice. Non-blocking.

[github-actions]

Adversarial Review

Critical / High

None found.

Medium

1. src/libswamp/vaults/read_secret.ts:162-175 — All readSecret errors mapped to notFound: The catch block wraps every exception from deps.readSecret() as notFound("Secret", ...). For cloud-backed vault providers (e.g., AWS Secrets Manager), VaultService.get() can throw network timeouts, authentication failures, or rate-limit errors — none of which mean "secret not found." A user seeing Secret not found: 'MY_KEY' in vault 'prod' when the actual problem is an expired AWS credential would waste time debugging the wrong thing.

   Breaking example: Vault prod uses @swamp/aws-sm. AWS STS token is expired → provider.get() throws ExpiredTokenException → user sees "Secret not found" instead of an auth error.

   Suggested fix: Distinguish provider errors from key-not-found errors. Either check for a well-known "not found" pattern in the error message and use a generic operationFailed (or similar) error for everything else, or let the underlying error propagate with its original message and code. This mirrors how vault_put.ts delegates put errors without catch-all mapping. That said, this follows the same pattern as the rest of the vault commands and is not a regression — it's an inherited design gap.

Low

1. src/libswamp/vaults/read_secret.ts:177-182 — publishSecretRead failure is unhandled: If eventBus.publish() throws (e.g., a subscriber throws), the error propagates out of the async generator uncaught, and the user gets an opaque stack trace rather than a user-friendly error. The secret was already read successfully at this point, so the user gets neither the value nor a clear error. However, this is the exact same pattern used in put.ts:184-189, so it's consistent with existing code and unlikely in practice (event handlers are local/synchronous).

2. src/libswamp/vaults/read_secret.ts:135+161 — TOCTOU between findVault and readSecret: findVault checks vault existence via YamlVaultConfigRepository, then readSecret re-resolves the vault via a separate VaultService.fromRepository() call. If the vault config is deleted between the two calls, VaultService.get() throws a vault-not-found error that gets misclassified as secret-not-found (see Medium #1). Window is tiny and this is the same pattern as put.ts — purely theoretical.

Verdict

PASS — Clean implementation that follows established patterns (put.ts, list_keys.ts). The error categorization in the catch-all is imprecise but not a regression from existing vault commands. No security vulnerabilities, no data corruption paths, no resource leaks.