fix(security): consume tenant-claim signing key through credential registry - #433
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
@opencode-agent review Review exact stacked head Review read-only; preserve independent reviewer identity. Do not mutate the branch, synthesize author approval, weaken protection, or merge. |
Rate Limit Exceeded
|
Objective
Advance issue #319 on the exact credential-registry stack by removing tenant-claim signing key authority from raw Spring secret injection. This child is intentionally stacked on #354 (
DirectoryCredentialRegistry) and remains Draft.Exact stack identity
d7321e0c29c38553f6e0fa089a9e659d91d08b8a;85ad5285075432ce1cdee9fd37299cbbeb0d44b8;4071808854b98731542f44f724242f95f2fcb4c7;7a6a1722d15c7e985ed1fb29dc025ee97b6a01a8;TenantAccessService, its registry-focused regression, and the existing null-secret compatibility test made overload-explicit;RED → implementation → compile repair
The RED regression required
TenantAccessServiceto accept the provider-neutralCredentialRegistry, resolve the server-ownedtenant-claims-signingreference forTENANT_CLAIMS_SIGNING, consume opaque snapshot bytes for HMAC verification, and reject credential identity or purpose mismatch.The implementation added that constructor and exact identity/purpose checks. Exact-head CI on
4071808854b98731542f44f724242f95f2fcb4c7then exposed one deterministic Java compile failure: the existingnew TenantAccessService(null, ...)fixture became ambiguous between theStringandCredentialRegistryoverloads. Current head keeps production behavior unchanged and casts the legacy null-secret fixture explicitly toString. Exact current-head CI and fuzz now pass.Contract
Registry-backed tenant verification fails closed unless the returned snapshot has credential identifier
tenant-claims-signingand purposeTENANT_CLAIMS_SIGNING. Secret bytes are converted only at the HMAC boundary and the local copy is cleared after constructor initialization. Existing direct-string constructors remain bounded compatibility surfaces for tests and legacy demo wiring; production Spring authority migration remains part of the ordered issue-#319 stack.Scope boundary
Tenant-claims runtime consumption only. Artifact-token signing, production registry bootstrap, rotation compatibility windows, KMS custody, distributed adapters, and
ProductionAuthReadinessConfigmigration remain follow-on work. No raw credential material enters PR metadata, logs, telemetry, browser code, or exceptions.Merge gate
Keep Draft while #354/#344 remain unmerged. Child CI/fuzz success is review evidence, not protected-main merge evidence. After the parent stack reaches protected
main, rebuild or retarget this bounded delta onto the new exact protected base and regenerate main-targeted CI, Security Scan, SAST, fuzz, current-head reviews, and qualifying independent approval. No predecessor evidence transfers.