feat(daemon): namespaced memory + keychain secret stores - #113
Merged
cidrblock merged 19 commits intoAug 13, 2026
Conversation
Wire DualSecretStore so clients can SetSecret with store=MEMORY without persisting to the OS keychain. Enforces move-on-write exclusivity and fails closed if the other backend cannot be cleared (DR-047).
|
Warning Review limit reached
Next review available in: 37 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (34)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe daemon now uses a composite memory and keychain secret store. gRPC and HTTP APIs support backend selection, report secret locations, reject environment-backed writes, and include integration and unit test coverage. ChangesDual secret storage
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant SecretAPI
participant DualSecretStore
participant MemorySecretStore
participant KeychainSecretStore
Client->>SecretAPI: Submit secret and store selection
SecretAPI->>DualSecretStore: Validate and write secret
DualSecretStore->>MemorySecretStore: Update memory backend
DualSecretStore->>KeychainSecretStore: Update keychain backend
SecretAPI-->>Client: Return secret backend metadata
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #113 +/- ##
==========================================
+ Coverage 78.02% 78.58% +0.55%
==========================================
Files 38 38
Lines 4979 5276 +297
Branches 1569 1709 +140
==========================================
+ Hits 3885 4146 +261
- Misses 542 557 +15
- Partials 552 573 +21
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR adds a process-lifetime in-memory secret backend alongside the existing OS keychain backend, allowing clients to choose where secrets are stored (with move-on-write mutual exclusivity) and rejecting writable ENV secrets.
Changes:
- Introduces
DualSecretStore(memory + keychain) with mutual exclusivity and “fail closed” semantics when clearing the other backend fails. - Extends HTTP and gRPC secret-setting APIs to accept a
storeselector (memory/keychain) and rejectenvwrites. - Updates integration/unit tests and security/configuration/architecture docs to describe DR-047 and the new behavior.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/daemon/tests/integration/web-sse.test.ts | Adds HTTP integration coverage for store: "memory" and rejecting store: "env". |
| packages/daemon/tests/integration/grpc-real-service.test.ts | Adds gRPC integration coverage for memory-only storage, move-on-write behavior, and rejecting ENV writes. |
| packages/daemon/src/daemon/web/server.ts | Adds store selection for HTTP secret writes and reports backend in GET /api/secrets. |
| packages/daemon/src/daemon/web/api-schemas.ts | Extends request schemas to accept optional store field. |
| packages/daemon/src/daemon/web/api-schemas.test.ts | Adds schema tests for accepted store values. |
| packages/daemon/src/daemon/state.ts | Switches daemon default secret store to DualSecretStore(memory, keychain). |
| packages/daemon/src/daemon/server/abbenay-service.ts | Adds gRPC store parsing/dispatch and reports backend in ListSecrets. |
| packages/daemon/src/daemon/secrets/dual.ts | Implements DualSecretStore, store parsing, and proto mapping helpers. |
| packages/daemon/src/daemon/secrets/dual.test.ts | Adds unit tests for dual-store semantics and store parsing/mapping. |
| packages/daemon/src/core/secrets.ts | Updates documentation/comments to reflect dual-store usage and audit sources. |
| docs/SECURITY.md | Documents in-memory secret implications and threat model considerations. |
| docs/decisions.md | Adds DR-047 decision record for process-lifetime secrets. |
| docs/CONFIGURATION.md | Documents configuration semantics and how to use the in-memory backend via secrets APIs. |
| docs/ARCHITECTURE.md | Updates architecture docs to reference DualSecretStore and its role. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/decisions.md`:
- Around line 837-847: Update DualSecretStore.setIn to prevent or explicitly
handle dual residency when rollback fails, including defined failure and
recovery behavior. Revise docs/decisions.md lines 837-847 and
docs/CONFIGURATION.md lines 407-408 so both guarantees accurately describe the
implementation and rollback failure outcome.
In `@packages/daemon/src/daemon/secrets/dual.ts`:
- Around line 78-87: Update the verification flow around other.has(key) to catch
rejected verification calls, delete the newly written primary secret as
rollback, ignore rollback deletion errors, and throw the same clear-failure
error used when the other backend still contains the key. Preserve the existing
successful verification and fail-closed behavior.
- Around line 56-88: Serialize all per-key mutations in the dual secret backend:
update setIn and the corresponding delete operation to use the same keyed mutex
or promise queue, holding it across every await until the operation completes or
rolls back. Preserve existing rollback and error behavior, and add a parallel
move test demonstrating concurrent operations cannot leave the key absent or
resident in both backends.
In `@packages/daemon/src/daemon/state.ts`:
- Around line 87-89: Update the HTTP and gRPC GetKeyStatus handlers to preserve
keychain-only semantics: when secretStore is the dual store, determine presence
with await dual.locate(name) === 'keychain' instead of has(name); retain
has(name) for non-dual stores. Use the DualSecretStore instance configured in
the daemon state initialization.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 42712a66-4ecf-4d2b-9242-0c805e62117e
📒 Files selected for processing (14)
docs/ARCHITECTURE.mddocs/CONFIGURATION.mddocs/SECURITY.mddocs/decisions.mdpackages/daemon/src/core/secrets.tspackages/daemon/src/daemon/secrets/dual.test.tspackages/daemon/src/daemon/secrets/dual.tspackages/daemon/src/daemon/server/abbenay-service.tspackages/daemon/src/daemon/state.tspackages/daemon/src/daemon/web/api-schemas.test.tspackages/daemon/src/daemon/web/api-schemas.tspackages/daemon/src/daemon/web/server.tspackages/daemon/tests/integration/grpc-real-service.test.tspackages/daemon/tests/integration/web-sse.test.ts
Add api_key_keychain_name so providers reference an existing SetSecret key (N:1). Raw api_key alone keeps the legacy invent-name shortcut.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 20 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- packages/python/src/abbenay_grpc/abbenay/v1/service_pb2.py: Generated file
Suppressed comments (2)
packages/daemon/src/daemon/secrets/dual.ts:144
- The ENV-write rejection message is gRPC-specific (mentions
SetSecretandapi_key_env_var_name). This same string is returned by the HTTP secrets endpoints too, so HTTP callers will get a confusing error that references the wrong API shape.
if (store === 'env' || store === 'SECRET_STORE_ENV' || store === 2 || store === '2') {
return {
ok: false,
error:
'SECRET_STORE_ENV is not writable via SetSecret; use api_key_env_var_name in provider config',
};
packages/daemon/tests/integration/consumer-auth.test.ts:295
- This test title says it rejects a missing
api_key_keychain_name, but the request includesapi_key_keychain_nameand the actual failure case is that the referenced secret does not exist. Renaming the test would make the intent clearer and easier to diagnose when it fails.
it('ConfigureProvider rejects missing api_key_keychain_name', async () => {
Rename the logical secret lookup to secret_name (YAML still accepts api_key_keychain_name). ConfigureProvider/HTTP use secret_name plus optional secret_store when writing a raw api_key.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- packages/python/src/abbenay_grpc/abbenay/v1/service_pb2.py: Generated file
Suppressed comments (3)
packages/daemon/src/daemon/secrets/dual.ts:143
- parseSecretStoreChoice() is used from multiple entry points (gRPC SetSecret, HTTP secrets endpoints, and provider configuration). The ENV error message currently mentions “via SetSecret”, which can be confusing when returned from non-SetSecret code paths. Consider making the message backend-agnostic (e.g., “not writable via secrets APIs”).
return {
ok: false,
error:
'SECRET_STORE_ENV is not writable via SetSecret; use api_key_env_var_name in provider config',
};
packages/daemon/src/daemon/secrets/dual.ts:87
- DualSecretStore.setIn doesn’t check the boolean return from other.delete(). For backends that signal failure via a false return (e.g., keytar deletePassword failures are caught and return false), the current code may treat the key as cleared as long as other.has() returns false, which can violate the “fails closed / mutual exclusivity” guarantee. Capture the delete result and fail/rollback when the other backend previously had the key but deletion reports false.
try {
await other.delete(key);
} catch (error: unknown) {
// Roll back primary so we do not leave a dual-resident key.
try {
packages/daemon/src/core/secrets.ts:36
- auditSecretChange() source documentation is missing the newly introduced "*-configure-memory" sources used by the HTTP/gRPC configure paths (e.g., "http-configure-memory" and "grpc-configure-memory"), which makes the comment misleading for operators grepping logs.
op: 'set' | 'delete';
/** http-secrets | http-secrets-memory | grpc-secrets | grpc-secrets-memory | http-configure | grpc-configure | core-add */
source: string;
secret_name + secret_store=env points at a process env var (resolved at request time). SetSecret still rejects ENV writes; legacy env_var_name maps to the same shape.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- packages/python/src/abbenay_grpc/abbenay/v1/service_pb2.py: Generated file
Suppressed comments (4)
packages/daemon/src/daemon/server/abbenay-service.ts:1947
- RemoveProvider currently deletes providerSecretName() unconditionally. With the new secret_name/secret_store model this can (1) try to delete env-based provider “secrets” (env var names) and (2) delete shared secrets when removing one provider that references a shared key. Consider only deleting provider-owned secrets (legacy
${PROVIDER_ID}_API_KEYorabbenay.${providerId}) and skip deletion when secret_store is "env".
const secretName = providerSecretName(config.providers[providerId]);
if (secretName) {
try {
await state.secretStore.delete(secretName);
auditSecretChange({ key: secretName, op: 'delete', source: 'grpc-configure' });
packages/daemon/src/daemon/web/server.ts:1109
- Same type mismatch as above: DualSecretStore.setIn only accepts "memory" | "keychain", but storeChoice.backend is typed as SecretStoreKind (includes "env"). Cast/narrow before calling setIn so this continues to typecheck and cannot accidentally treat "env" as "keychain" at runtime.
const dual = isDualSecretStore(state.secretStore) ? state.secretStore : null;
if (dual) {
await dual.setIn(storeChoice.backend, parsed.data.key, parsed.data.value);
} else {
await state.secretStore.set(parsed.data.key, parsed.data.value);
}
packages/daemon/src/daemon/web/server.ts:1073
- Type mismatch: parseSecretStoreChoice() returns backend as SecretStoreKind (includes "env"), but DualSecretStore.setIn expects only "memory" | "keychain". Even though ENV is rejected for this route, the call currently passes storeChoice.backend directly. Narrow/cast after asserting it cannot be "env" to keep the type-safety (and avoid accidental behavior changes if parsing rules change).
This issue also appears on line 1104 of the same file.
const dual = isDualSecretStore(state.secretStore) ? state.secretStore : null;
if (dual) {
await dual.setIn(storeChoice.backend, key, parsed.data.value);
} else {
await state.secretStore.set(key, parsed.data.value);
packages/daemon/src/daemon/server/abbenay-service.ts:842
- Type mismatch: parseSecretStoreChoice() returns backend as SecretStoreKind (includes "env"), but DualSecretStore.setIn expects only "memory" | "keychain". ENV is rejected here, but the call still passes parsed.backend directly; narrow/cast after validation to keep this code type-safe and prevent "env" from ever being treated as "keychain" by DualSecretStore.
This issue also appears on line 1943 of the same file.
const dual = isDualSecretStore(state.secretStore) ? state.secretStore : null;
const write = dual
? dual.setIn(parsed.backend, key, value)
: state.secretStore.set(key, value);
const auditSource =
parseSecretStoreChoice overloads so SetSecret/configure writes type as SecretBackend; env remains configure/reference-only and CI builds again.
This was referenced Aug 12, 2026
Collaborator
Author
|
Follow-up issues filed:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 24 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- packages/python/src/abbenay_grpc/abbenay/v1/service_pb2.py: Generated file
Suppressed comments (2)
packages/daemon/src/daemon/web/server.ts:1554
- When removing a provider, this deletes
providerSecretName(...)from the daemon secret store unconditionally. Forsecret_store: "env",providerSecretNamereturns the env var name, so this would attempt to delete a secret-store entry with the same name (and could remove a real stored secret shared by other providers). Skip secretStore deletion when the provider is configured to use env.
const secretName = providerSecretName(config.providers[providerId]);
if (secretName) {
try {
await state.secretStore.delete(secretName);
auditSecretChange({ key: secretName, op: 'delete', source: 'http-configure' });
packages/daemon/src/daemon/web/server.ts:1052
- This route supports both
secret_storeand legacystorefields (see parsing below), but the comment only documentsstore. Updating the docstring avoids confusion for API consumers.
/**
* POST /api/secrets/:key - Set a specific secret (API key)
* Body: { value: string, store?: "memory" | "keychain" }
*/
…egistry Drop move-on-write DualSecretStore so memory and keychain stay discrete; providers resolve only their configured backend, positioning N pluggable stores.
cidrblock
changed the title
3 tasks
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- packages/python/src/abbenay_grpc/abbenay/v1/service_pb2.py: Generated file
Suppressed comments (3)
packages/daemon/src/daemon/web/api-schemas.ts:150
- PostProviderConfigureBodySchema currently allows
apiKeytogether withsecretStore: "env", but the configure route rejects ENV for writes (parseSecretStoreChoice without allowEnv). Consider rejecting this combination at schema-validation time so HTTP clients get a clear, consistent 400 error before the route logic runs.
if (data.apiKey && data.envVarName) {
ctx.addIssue({
code: z.ZodIssueCode.custom,
message: 'Provide only one of apiKey or envVarName',
path: ['apiKey'],
packages/daemon/src/daemon/web/api-schemas.ts:46
- PostSecretBodySchema / PostSecretByKeyBodySchema accept
secret_store: "env", but the HTTP secrets endpoints reject ENV via parseSecretStoreChoice (only memory|keychain are writable). This mismatch means invalid requests pass schema validation and fail later with a less-specific error; it also contradicts the endpoint docs.
const SecretStoreFieldSchema = z.enum(['memory', 'keychain', 'env']).optional();
packages/daemon/src/daemon/web/api-schemas.test.ts:190
- This test asserts that the secrets POST schemas accept
secret_store: "env", but the secrets endpoints reject ENV writes (only memory|keychain are writable). If the schemas are tightened to match the runtime contract, this expectation should be updated.
it('accepts optional secret_store memory|keychain|env (and legacy store)', () => {
expect(
PostSecretBodySchema.safeParse({ key: 'K', value: 'v', secret_store: 'memory' }).success,
).toBe(true);
expect(
PostSecretByKeyBodySchema.safeParse({ value: 'v', store: 'keychain' }).success,
).toBe(true);
expect(
PostSecretBodySchema.safeParse({ key: 'K', value: 'v', secret_store: 'env' }).success,
).toBe(true);
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 33 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- packages/python/src/abbenay_grpc/abbenay/v1/service_pb2.py: Generated file
Suppressed comments (4)
packages/daemon/src/daemon/web/api-schemas.ts:45
PostSecretBodySchema/PostSecretByKeyBodySchemacurrently acceptsecret_store: "env"(viaSecretStoreFieldSchema), but the HTTP secrets endpoints reject ENV writes viaparseSecretStoreChoice. This makes the request schema disagree with the actual API contract (memory|keychain only) and can mislead clients.
const SecretStoreFieldSchema = z.enum(['memory', 'keychain', 'env']).optional();
packages/daemon/src/daemon/web/server.ts:1058
- The comment for
POST /api/secrets/:keydocuments onlystore?: "memory" | "keychain", but the implementation (and request schema) usessecret_storeas the preferred field andstoreas a legacy alias. Updating the comment avoids confusion for callers and keeps it aligned with the code.
/**
* POST /api/secrets/:key - Set a specific secret (API key)
* Body: { value: string, store?: "memory" | "keychain" }
*/
packages/daemon/src/daemon/web/api-schemas.test.ts:186
- This test asserts that
PostSecretBodySchemaacceptssecret_store: "env", but the daemon's secrets write endpoints reject ENV (it’s a credential source, not a writable store). Keeping this expectation will codify behavior that contradicts the API contract.
expect(
PostSecretBodySchema.safeParse({ key: 'K', value: 'v', secret_store: 'env' }).success,
).toBe(true);
packages/daemon/src/daemon/web/server.ts:1188
- The key-status handler now supports
source=memory, but the route-level comment above still documents onlysource=keychain|env. Update the doc comment to includememoryso it matches the supported sources.
if (source === 'keychain' || source === 'memory') {
const registry = isSecretStoreRegistry(state.secretStore) ? state.secretStore : null;
exists = registry
? await registry.hasIn(source, name)
: source === 'keychain'
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 33 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- packages/python/src/abbenay_grpc/abbenay/v1/service_pb2.py: Generated file
Suppressed comments (4)
packages/daemon/src/core/secrets.ts:68
- The auditSecretChange JSDoc list of allowed sources should include the new "http-configure-memory" / "grpc-configure-memory" values to match actual call sites.
* Sources include: http-secrets | http-secrets-memory | grpc-secrets |
* grpc-secrets-memory | http-configure | grpc-configure | core-add
packages/daemon/src/daemon/web/server.ts:1609
- Provider delete audits all secret deletions as "http-configure" even when the owned secret was stored in the memory backend. This loses the backend distinction that ConfigureProvider already records via "http-configure-memory".
auditSecretChange({ key: secretName, op: 'delete', source: 'http-configure' });
packages/daemon/src/daemon/server/abbenay-service.ts:2013
- RemoveProvider audits all secret deletions as "grpc-configure" even when the owned secret was stored in the memory backend. This makes audit logs inconsistent with ConfigureProvider, which emits "grpc-configure-memory" for memory writes.
auditSecretChange({ key: secretName, op: 'delete', source: 'grpc-configure' });
packages/daemon/src/core/secrets.ts:59
- The SecretAuditEvent.source docstring omits the "*-configure-memory" sources that are now emitted (e.g. http-configure-memory / grpc-configure-memory). Update the list so it stays accurate for log/audit consumers.
This issue also appears on line 67 of the same file.
/** http-secrets | http-secrets-memory | grpc-secrets | grpc-secrets-memory | http-configure | grpc-configure | core-add */
Optional store fields regenerated packages/vscode/src/proto, and Sonar counted ~1.6k uncovered generated lines against new-code coverage. Co-authored-by: Cursor <cursoragent@cursor.com>
Parallel CI files can clobber a shared console.warn spy, making the short-circuit test flake. Assert the sticky loadError instead. Co-authored-by: Cursor <cursoragent@cursor.com>
Clients could request memory while a plain store silently persisted to keychain and audited as memory. Require a namespaced registry for memory on gRPC/HTTP secrets and configure paths. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- packages/python/src/abbenay_grpc/abbenay/v1/service_pb2.py: Generated file
Suppressed comments (3)
packages/daemon/src/daemon/web/api-schemas.ts:49
- The secrets write body schemas allow
secret_store: "env", but the corresponding HTTP routes callparseSecretStoreChoice(...)withoutallowEnvand will always reject ENV writes (400). This makes the schema contract inconsistent with runtime behavior and the PR summary (“env rejected — credential source only”). Consider restricting the secrets write schemas to onlymemory|keychain(keepenvfor provider config/reference).
const SecretStoreFieldSchema = z.enum(['memory', 'keychain', 'env']).optional();
export const PostSecretByKeyBodySchema = z
.object({
value: z.string().min(1),
packages/daemon/src/daemon/web/api-schemas.test.ts:190
- This test currently asserts that secret write bodies accept
secret_store: "env", but the secrets routes intentionally reject ENV writes (env is reference-only). Once the schema is tightened tomemory|keychain, this expectation should flip tofalseto match the intended contract.
it('accepts optional secret_store memory|keychain|env (and legacy store)', () => {
expect(
PostSecretBodySchema.safeParse({ key: 'K', value: 'v', secret_store: 'memory' }).success,
).toBe(true);
expect(
PostSecretByKeyBodySchema.safeParse({ value: 'v', store: 'keychain' }).success,
).toBe(true);
expect(
PostSecretBodySchema.safeParse({ key: 'K', value: 'v', secret_store: 'env' }).success,
).toBe(true);
expect(
packages/daemon/src/core/secrets.ts:60
auditSecretChangesource docs don’t mention the newly introduced*-configure-memorysources, even though web/gRPC provider configure now emithttp-configure-memory/grpc-configure-memory. Updating the inline docs helps keep audit-source semantics discoverable/consistent.
export interface SecretAuditEvent {
/** Secret key name only — never the value */
key: string;
op: 'set' | 'delete';
/** http-secrets | http-secrets-memory | grpc-secrets | grpc-secrets-memory | http-configure | grpc-configure | core-add */
source: string;
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 34 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- packages/python/src/abbenay_grpc/abbenay/v1/service_pb2.py: Generated file
Suppressed comments (1)
packages/daemon/src/daemon/web/server.ts:1502
- When referencing an existing
secretName(noapiKey), an invalidsecretStorevalue currently falls through to the default keychain lookup branch. That means requests like{ secretName: "X", secretStore: "vault" }are silently treated as keychain instead of failing closed with a 400, which is inconsistent with the rest of the API (and with the gRPC handler). Add explicit validation for unsupportedsecretStorevalues before theenv/memory/keychainbranching.
const storeRaw = secretStoreBody;
if (storeRaw === 'env') {
|
Hrithik-Gavankar
self-requested a review
August 13, 2026 14:21
7 tasks
cidrblock
added a commit
to cidrblock/abbenay
that referenced
this pull request
Aug 13, 2026
Hard-break the miss from redhat-developer#113 — no deprecated snake_case or bare store fields on secrets request bodies or DELETE query. Co-authored-by: Cursor <cursoragent@cursor.com>
cidrblock
added a commit
that referenced
this pull request
Aug 13, 2026
…retStore fix(daemon): camelCase HTTP secretStore (missed in #113)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SecretStoreRegistry(MemorySecretStore+KeychainSecretStore); values addressed by(secret_store, secret_name)— overlapping names allowed across backends (no overlay)secret_name+secret_store(memory|keychain|env); keepapi_key_keychain_name/api_key_env_var_nameas YAML legacy aliasesNamespacedSecretStore.getFrom); omitted store defaults to keychainSetSecret/ HTTP secrets writememory|keychainonly (envrejected — env is a credential source, not a registered store); Get/Delete take optionalstore(default keychain); List emits one row per backend that holds a keyConfigureProvider/ HTTP configure pick an existing named secret in a specific store (N providers → 1 key); rawapi_keyalone keeps the legacy invent-name shortcut${PROVIDER}_API_KEY/abbenay.<id>); skipssecret_store: envand shared picksOut of scope (follow-up)
SecretStoreRegistry/NamespacedSecretStore)Commits
feat(daemon): add process-lifetime in-memory secret storefeat(daemon): let ConfigureProvider pick a named secretrefactor(daemon): prefer secret_name and secret_store vocabularyfeat(daemon): allow secret_store=env for provider referencesfix(daemon): narrow secret_store writes to memory|keychain for tscrefactor(daemon): namespace secrets by (store, name) via SecretStoreRegistryfix(daemon): align secret remove/status with namespaced storesfix(daemon): normalize keytar missing values to nulltest(daemon): cover namespaced secret store resolve and remove pathsTest plan
SetSecretMEMORY +GetSecretstore=MEMORY; keychain Get does not see itConfigureProviderwithsecret_name+secret_store=memoryresolves that backend onlysecret_name+secret_store=envconfigures without writing the secret storeSetSecret/ HTTP withsecret_store=envreturns INVALID_ARGUMENT / 400providerCredentialSource/isProviderOwnedSecretNameresolveApiKeyreads only the configured namespaced backendsecret_store=memoryleaves keychain intact; key-status reports backends independentlyListSecretsemits one row per backend holding an engine key