Skip to content

fix: reject key writes whose secret is already used by another key entity #1979 - #1980

Open
KirylKurnosenka wants to merge 1 commit into
developmentfrom
fix/key-secret-uniqueness
Open

KirylKurnosenka wants to merge 1 commit into
developmentfrom
fix/key-secret-uniqueness

Conversation

@KirylKurnosenka

@KirylKurnosenka KirylKurnosenka commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

ApiKeyStore.keys is keyed by the plaintext secret, so two Key entities created with the same secret silently collapsed: auth, roles, rate-limit project, and author attribution flipped to whichever entity was written last.

Applicable issues

Description of changes

  • Strict duplicate-secret guard on all key write paths, mirroring the schema duplicate-$id guard. Shared predicate ConfigPostProcessor.isKeySecretTakenByAnotherKey scans the folded Config.keys map, skipping the written entity's own entry.
  • PUT /v1/keys/platform/{name} returns 409 before anything is written; /v1/admin/apply and /v1/admin/validate mark the entity FAILED (within-batch duplicates are caught too, via the scratch).
  • The config-file migration endpoint pre-removes the file twin from the scratch (scratch.getKeys().remove(secret)), the same handoff scratchSchemas.remove(id) gives schemas — migration stays the only supported file→blob handoff for keys.
  • Updates that keep the entity's own prior secret stay allowed, so a migrated key can still be updated with its unchanged secret.
  • Replica events log a warning instead of rejecting (the blob is already written on the origin pod).
  • Rebuild-time detection of legacy blob-vs-blob duplicate pairs is out of scope.
  • 11 new tests: 409/duplicate-secret cases on PUT, apply, and validate (including rotation, unchanged-secret update, and file-key-secret cases) plus predicate unit tests; migration and ApiKeyStore/MergedConfigStore regression suites verified locally.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

🤖 Generated with Claude Code

…tity #1979

ApiKeyStore.keys is keyed by the plaintext secret, so two Key entities
created with the same secret silently collapsed: auth, roles, rate-limit
project, and author attribution flipped to whichever entity was written
last. Reject such writes on all write paths (409 on PUT, FAILED per
entity on apply/validate), mirroring the schema duplicate-$id guard;
the config-file migration endpoint pre-removes the file twin from the
scratch so it remains the supported handoff for file-defined keys.
Updates that keep the entity's own prior secret stay allowed.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@ai-dial-actions

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

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.

Admin API: key entities with the same secret silently overwrite each other

2 participants