diff --git a/work-items/open/20260529-145355-manifest-profile-encrypted-secrets/artifacts/review-20260531-secrets-implementation.md b/work-items/open/20260529-145355-manifest-profile-encrypted-secrets/artifacts/review-20260531-secrets-implementation.md new file mode 100644 index 00000000..0adf9ab5 --- /dev/null +++ b/work-items/open/20260529-145355-manifest-profile-encrypted-secrets/artifacts/review-20260531-secrets-implementation.md @@ -0,0 +1,34 @@ +# Review: local key-value secret store implementation + +Implementation reviewed on branch `manifest-profile-encrypted-secrets`. + +Reviewed commits: + +- `cc2c9a2 secrets: add local key store` +- `7ddf745 secrets: polish key manager and docs` + +Verdict: approve. + +Summary: + +- Core provider-independent `id -> value` local secret store satisfies the ticket model. +- Store values are not persisted as casual plaintext and error/debug surfaces avoid secret values within the stated modest protection boundary. +- Provider auth now resolves explicit `secret_ref` values through the local store without env credential fallback. +- WebSearch uses explicit `api_key_secret` and no longer depends on `BRAVE_SEARCH_API_KEY` / `api_key_env` in the normal path. +- `insomnia keys` provides interactive list/add-set/delete management without displaying plaintext values. +- Codex OAuth behavior remains separate and unchanged. +- Follow-up review confirmed the docs credential example is schema-valid and key-manager terminal setup cleanup was added. + +Validation reported by coder/reviewer: + +- `cargo fmt --check` +- `cargo test -p secrets` +- focused manifest/provider/tools/tui/insomnia tests +- `cargo check -p manifest -p provider -p tools -p tui -p insomnia` +- `./tickets.sh doctor` +- `git diff --check` +- credential/env greps confirming `api_key_env`, `BRAVE_SEARCH_API_KEY`, `INSOMNIA_API_KEY`, and `default_env_var` are absent from crates/docs/resources + +Remaining caveat: + +- Continue to describe this as local obfuscation / limited at-rest protection, not a high-assurance password manager or OS-keychain-backed vault. diff --git a/work-items/open/20260529-145355-manifest-profile-encrypted-secrets/item.md b/work-items/open/20260529-145355-manifest-profile-encrypted-secrets/item.md index b1d1d534..6eecf333 100644 --- a/work-items/open/20260529-145355-manifest-profile-encrypted-secrets/item.md +++ b/work-items/open/20260529-145355-manifest-profile-encrypted-secrets/item.md @@ -7,7 +7,7 @@ kind: feature priority: P2 labels: [manifest, profiles, secrets, security, cli, tui] created_at: 2026-05-29T14:53:55Z -updated_at: 2026-05-31T21:23:46Z +updated_at: 2026-05-31T22:21:04Z assignee: null legacy_ticket: null --- diff --git a/work-items/open/20260529-145355-manifest-profile-encrypted-secrets/thread.md b/work-items/open/20260529-145355-manifest-profile-encrypted-secrets/thread.md index f9de53a7..a6f5201c 100644 --- a/work-items/open/20260529-145355-manifest-profile-encrypted-secrets/thread.md +++ b/work-items/open/20260529-145355-manifest-profile-encrypted-secrets/thread.md @@ -203,4 +203,46 @@ Codex OAuth relationship: - If Insomnia later owns Codex login/token storage, design it as a separate OAuth token-store feature, not as an implicit use of the simple key-value store. +--- + + + +## Review: approve + +# Review: local key-value secret store implementation + +Implementation reviewed on branch `manifest-profile-encrypted-secrets`. + +Reviewed commits: + +- `cc2c9a2 secrets: add local key store` +- `7ddf745 secrets: polish key manager and docs` + +Verdict: approve. + +Summary: + +- Core provider-independent `id -> value` local secret store satisfies the ticket model. +- Store values are not persisted as casual plaintext and error/debug surfaces avoid secret values within the stated modest protection boundary. +- Provider auth now resolves explicit `secret_ref` values through the local store without env credential fallback. +- WebSearch uses explicit `api_key_secret` and no longer depends on `BRAVE_SEARCH_API_KEY` / `api_key_env` in the normal path. +- `insomnia keys` provides interactive list/add-set/delete management without displaying plaintext values. +- Codex OAuth behavior remains separate and unchanged. +- Follow-up review confirmed the docs credential example is schema-valid and key-manager terminal setup cleanup was added. + +Validation reported by coder/reviewer: + +- `cargo fmt --check` +- `cargo test -p secrets` +- focused manifest/provider/tools/tui/insomnia tests +- `cargo check -p manifest -p provider -p tools -p tui -p insomnia` +- `./tickets.sh doctor` +- `git diff --check` +- credential/env greps confirming `api_key_env`, `BRAVE_SEARCH_API_KEY`, `INSOMNIA_API_KEY`, and `default_env_var` are absent from crates/docs/resources + +Remaining caveat: + +- Continue to describe this as local obfuscation / limited at-rest protection, not a high-assurance password manager or OS-keychain-backed vault. + + ---