promote: IAM registration and replay hardening - #48
Merged
Conversation
…ical-20260804 fix: harden IAM registration and service-account replay
|
Warning Review limit reached
Next review available in: 8 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (51)
📝 WalkthroughWalkthroughThis PR hardens IAM registration, recovery, invitations, and service-account creation. It adds audit replay handling, authoritative entitlement verification, encrypted secret envelopes, database uniqueness constraints, generic registration responses, required MFA response fields, and stricter API schemas. ChangesAudit attestation and seal access
Entitlement and invitation domain behavior
IAM flows
API contracts and persistence constraints
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Client
participant RegistrationController
participant AdmissionAdapters
participant RegistrationService
participant IAMRepository
Client->>RegistrationController: submit registration
RegistrationController->>AdmissionAdapters: check IP and email digests
AdmissionAdapters-->>RegistrationController: allow or reject
RegistrationController->>RegistrationService: register accepted request
RegistrationService->>IAMRepository: persist registration
IAMRepository-->>RegistrationService: registration result
RegistrationService-->>RegistrationController: accepted result
RegistrationController-->>Client: 202 Accepted
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
services/api/src/features/iam/application/invitation.service.ts (1)
174-239: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftAdd durable recovery for delivery failures.
After
saveInvitationcommits, a delivery exception returnsDELIVERY_UNAVAILABLE. A retry then finds the active token and returnsCONFLICT. The raw token exists only inpendingDelivery, whiledocs/operations/iam-010-invitation-token-2026-08-03.mdstates that only digests persist. The later resend described on Lines 230-231 cannot use the same bearer.Persist an encrypted delivery outbox payload and retry the same token, or add a safe replacement and revocation flow. This prevents a transient delivery failure from permanently blocking the invited membership.
🤖 Prompt for 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. In `@services/api/src/features/iam/application/invitation.service.ts` around lines 174 - 239, Update the invitation flow around saveInvitation and delivery.deliver to persist a durable, encrypted outbox payload containing the raw bearer token before the transaction commits, then remove or mark it delivered after successful delivery and retry it on subsequent attempts. Ensure the existing active token can be recovered for resend without persisting plaintext token data, and preserve token reuse and revocation semantics so delivery failures do not return CONFLICT permanently.
🧹 Nitpick comments (9)
services/api/test/http-contract.test.ts (1)
336-336: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the
trueMFA re-enrollment path.These assertions cover only the omitted-value fallback to
false. Ensure the contract tests also passmfaReenrollmentRequired: truethrough sign-in and current-session responses, then asserttrue. This detects a regression that drops a required re-enrollment state.Also applies to: 678-678
🤖 Prompt for 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. In `@services/api/test/http-contract.test.ts` at line 336, Extend the contract tests around the sign-in and current-session response assertions to provide mfaReenrollmentRequired: true and verify the responses preserve true, while retaining the existing omitted-value fallback assertion for false.services/api/test/prisma-foundation.test.mjs (1)
573-593: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the complete migration contract.
The current checks validate only selected names and columns. They do not prove that the invitation index uses
membership_idwithWHERE "status" = 'ACTIVE'.The service-account checks also omit
create_idempotency_key,create_request_hash, andservice_accounts_create_idempotency_key. They do not verify the required index columns or partial predicates. Add assertions for these exact SQL clauses from both migration files.🤖 Prompt for 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. In `@services/api/test/prisma-foundation.test.mjs` around lines 573 - 593, Expand the migration assertions in the test covering active invitations and service-account idempotency to validate the complete SQL contract, not just object names. Assert the invitation index includes membership_id and the ACTIVE status predicate, and add checks for create_idempotency_key, create_request_hash, service_accounts_create_idempotency_key, each required index column list, and their partial predicates using the existing migration-content assertion pattern.services/api/src/features/iam/application/service-account.service.ts (1)
208-231: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCanonicalize
permissionsbefore hashing.
createRequestHashnormalizesnameandsecretExpiresAt, but it hashesinput.permissionsin the order received. A retry that sends the same permissions in a different order produces a different hash.replayCreatethen returnsCONFLICTfor a semantically identical retry. Sort the permission list in the hash input to keep the hash order-independent.♻️ Proposed change
- JSON.stringify({ - name, - workspaceId: workspaceId ?? null, - permissions: input.permissions, - secretExpiresAt: expiry ?? null, - }), + JSON.stringify({ + name, + workspaceId: workspaceId ?? null, + permissions: [...input.permissions].sort(), + secretExpiresAt: expiry ?? null, + }),🤖 Prompt for 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. In `@services/api/src/features/iam/application/service-account.service.ts` around lines 208 - 231, Update createRequestHash to canonicalize permissions before hashing by sorting the validated permission list in the JSON payload. Preserve the existing validation and ensure semantically identical permission sets produce the same order-independent hash for replayCreate.services/api/test/features/iam/service-account-composition.test.ts (1)
34-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that composition wires the secret envelope.
createnow returnsUNAVAILABLEwhensecretEnvelopeis absent. This test only asserts that the provider resolves to aServiceAccountServiceinstance, so it passes even when the envelope is unwired and every create fails. Add an assertion that exercises the wired envelope, for example acreatecall against a stub database, or an assertion on the envelope provider registered byIamModule.register.🤖 Prompt for 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. In `@services/api/test/features/iam/service-account-composition.test.ts` around lines 34 - 46, Extend the IAM composition test around IamModule.register and the SERVICE_ACCOUNT_SERVICE provider to verify the secret envelope is wired, not only that ServiceAccountService is instantiated. Exercise the service’s create path with a stub database and assert it does not return UNAVAILABLE, or assert the envelope provider registered by IamModule.register is present and correctly configured.services/api/prisma/migrations/20260804010000_iam_service_account_create_idempotency/migration.sql (1)
25-34: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
service_accounts_create_idempotency_workspace_keyis redundant.This index uses the same column list as
service_accounts_create_idempotency_keyon lines 9-15, and its predicate only narrows the rows covered. Any duplicate pair rejected by this partial index is already rejected by the full index, because none of the four columns is NULL under the predicate. The workspace-scope race is therefore already closed by the full index.The organization-scope partial index on lines 19-23 remains necessary, because the full index treats NULL
workspace_idvalues as distinct.Dropping the redundant index removes one index maintenance cost per insert and update.
♻️ Proposed removal of the redundant index
-CREATE UNIQUE INDEX "service_accounts_create_idempotency_workspace_key" -ON "iam"."service_accounts"( - "organization_id", - "workspace_id", - "created_by_actor_id", - "create_idempotency_key" -) -WHERE "workspace_id" IS NOT NULL - AND "created_by_actor_id" IS NOT NULL - AND "create_idempotency_key" IS NOT NULL;🤖 Prompt for 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. In `@services/api/prisma/migrations/20260804010000_iam_service_account_create_idempotency/migration.sql` around lines 25 - 34, Remove the redundant CREATE UNIQUE INDEX statement named service_accounts_create_idempotency_workspace_key from the migration, while retaining service_accounts_create_idempotency_key and the organization-scope partial index unchanged.services/api/src/features/iam/adapter/service-account-secret-envelope.adapter.ts (2)
64-67: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument the durability effect of the process-local fallback key.
randomServiceAccountSecretEnvelopeAdaptergenerates a new key for each process. Every storedcreate_secret_envelopevalue becomes undecryptable after a restart, and a second instance cannot open envelopes sealed by the first. A retry with the originalIdempotency-Keythen returnsUNAVAILABLEinstead of the original secret, becauseopenfails closed.The behavior is safe. The operational consequence is that any deployment with more than one instance, or with restarts, must configure a durable key. Add that constraint to the doc comment or to the operations documentation so the fallback is not selected by accident.
🤖 Prompt for 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. In `@services/api/src/features/iam/adapter/service-account-secret-envelope.adapter.ts` around lines 64 - 67, Update the doc comment for randomServiceAccountSecretEnvelopeAdapter to explicitly state that its process-local random key cannot decrypt stored envelopes after restarts or across instances, and that multi-instance or restart-prone deployments must configure a durable key.
40-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReject envelopes that carry extra framing segments.
envelope.split('.')returns every segment, and the destructuring pattern drops any segment after the fourth. An envelope such asv1.<iv>.<tag>.<ciphertext>.<extra>therefore opens successfully, because AES-GCM authenticates only the three decoded fields. The trailing segment is unauthenticated and silently ignored.The current test at
services/api/test/features/iam/service-account-secret-envelope.adapter.test.tsline 37 passes only because the IV length check rejects that specific input, not because the framing is strict.Check the segment count so the framing stays exact.
♻️ Proposed strict framing check
public open(envelope: string): string | undefined { if (typeof envelope !== 'string') return undefined; - const [version, ivEncoded, tagEncoded, ciphertextEncoded] = envelope.split('.'); - if (version !== 'v1' || !ivEncoded || !tagEncoded || !ciphertextEncoded) return undefined; + const parts = envelope.split('.'); + if (parts.length !== 4) return undefined; + const [version, ivEncoded, tagEncoded, ciphertextEncoded] = parts; + if (version !== 'v1' || !ivEncoded || !tagEncoded || !ciphertextEncoded) return undefined;🤖 Prompt for 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. In `@services/api/src/features/iam/adapter/service-account-secret-envelope.adapter.ts` around lines 40 - 43, Update the open method in the service-account secret envelope adapter to reject envelopes whose split result contains anything other than exactly four segments before decoding or decrypting. Preserve the existing validation for the v1 version and required encoded fields, while ensuring trailing framing segments cannot be ignored.services/api/test/features/iam/service-account.service.test.ts (1)
71-92: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse the module-level
digestSecrethelper.Line 76 defines a local
digestarrow that repeatsdigestSecretfrom line 26 of this file. Call the existing helper instead.♻️ Proposed deduplication
const iam = new InMemoryIamRepositoryAdapter(); iam.seed([membership()]); - const digest = (secret: string) => createHash('sha256').update(secret, 'utf8').digest('hex'); const secrets = [ - { secret: 'dbsa_first', digest: digest('dbsa_first') }, - { secret: 'dbsa_second', digest: digest('dbsa_second') }, + { secret: 'dbsa_first', digest: digestSecret('dbsa_first') }, + { secret: 'dbsa_second', digest: digestSecret('dbsa_second') }, ];🤖 Prompt for 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. In `@services/api/test/features/iam/service-account.service.test.ts` around lines 71 - 92, In the service helper, remove the local digest arrow and use the existing module-level digestSecret helper when constructing the secrets array. Keep the generated secret values and resulting digests unchanged.services/api/test/features/iam/prisma-service-account-repository.test.ts (1)
213-213: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThis assertion cannot fail.
The test never supplies a raw secret.
secretEnvelopeis the literal'v1.encrypted-envelope', and no value passed intosaveServiceAccountcontains'dbsa'. The assertion therefore passes regardless of what the adapter writes, so it does not prove that raw secrets stay out of the row.Assert the property directly instead: confirm that the persisted row carries the envelope value and no plaintext marker that was supplied separately.
♻️ Proposed assertion that can fail
await repository.saveServiceAccount(organizationContext, value, { actorId: organizationContext.actorId, idempotencyKey: 'create-key', requestHash: 'b'.repeat(64), - secretEnvelope: 'v1.encrypted-envelope', + secretEnvelope: `v1.${Buffer.from('dbsa_raw_secret').toString('base64url')}`, });- assert.equal(JSON.stringify(rows[0]).includes('dbsa'), false); + // The row stores only the opaque envelope, never the raw secret text. + assert.equal(JSON.stringify(rows[0]).includes('dbsa_raw_secret'), false); + assert.equal(rows[0]?.createSecretEnvelope, envelope);Adjust the
secretEnvelopeandenvelopebindings to a single constant so both assertions reference the same value.🤖 Prompt for 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. In `@services/api/test/features/iam/prisma-service-account-repository.test.ts` at line 213, Update the service-account persistence test around saveServiceAccount to use a distinct plaintext secret marker alongside a shared secretEnvelope/envelope constant. Assert directly that the persisted row contains the envelope value and does not contain the separately supplied plaintext marker, replacing the ineffective JSON.stringify(...).includes('dbsa') check.
🤖 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 `@services/api/src/features/bua/api/entitlement-lease.dto.ts`:
- Around line 13-24: Update the OpenAPI schemas for the snapshotRevision and
securityEpoch query parameters to use type integer and maximum 9007199254740991,
matching the validation decorators in the corresponding DTO fields and
preventing fractional client values.
In
`@services/api/src/features/iam/adapter/prisma-service-account-repository.adapter.ts`:
- Around line 161-193: The persistedReplay flow must replay the original
create-time account rather than the current row after rotate or revoke. Update
the persistence model and lifecycle writes around persistedReplay to store and
preserve a create-time account snapshot with the replay envelope, or invalidate
replay metadata when lifecycle changes make it stale; ensure replayCreate
returns UNAVAILABLE for invalidated metadata and never returns the revoked
current account. Add a Prisma regression test covering rotate and revoke
behavior.
In `@services/api/src/features/iam/api/authentication.controller.ts`:
- Line 71: Make mfaReenrollmentRequired required in both
AuthenticatedPrincipalV1 and IamTenantContextV1, then validate authentication
inputs before the mappings used by me and signIn so omitted values are rejected
rather than defaulted to false. Remove the nullish fallback in the
mfaReenrollmentRequired mapping and preserve explicit boolean values.
In `@services/api/src/features/iam/api/registration.controller.ts`:
- Around line 26-29: Update admissionDigest to use HMAC-SHA-256 with a validated
registration-specific secret instead of unkeyed SHA-256, while preserving the
existing databreeze:iam:registration:{kind}:v1 namespace. Load and validate the
HMAC key through the existing configuration mechanism, keep it out of Redis
values, and support key rotation by accepting the configured current/previous
key set during admission checks.
In `@services/api/src/features/iam/application/recovery.service.ts`:
- Around line 171-173: Update the compensating-revocation handling in the
recovery flow around delivery and saveChallenge so a saveChallenge failure is
not swallowed while the challenge remains ACTIVE. Persist a retryable revocation
task or fail closed until revocation succeeds, and add a test covering delivery
failure followed by compensating-revocation failure; ensure complete() cannot
use the raw token in that state.
In `@services/api/src/features/iam/application/service-account.service.ts`:
- Around line 315-326: Bound one-time secret retention across idempotent replay
and rotation. Add and persist createIdempotencyExpiresAt when creating the
service account, reject replay in findServiceAccountByIdempotency once that
timestamp has expired, and clear createSecretEnvelope when the window expires.
Update replaceServiceAccount to clear the envelope during rotation as well.
- Around line 274-275: Update IamModule.register and the production startup
configuration to require and provide a durable serviceAccountSecretEnvelopeKey
instead of generating a random process-local key. Ensure the same configured key
is reused across restarts and replicas so service-account secret storage remains
readable; preserve the existing UNAVAILABLE behavior only when the durable key
is unavailable.
In `@services/api/src/features/iam/iam.module.ts`:
- Around line 489-504: Update the service-account initialization around
serviceAccountSecretEnvelope and serviceAccountService so durable
serviceAccountDatabase configurations require an explicit
serviceAccountSecretEnvelope or serviceAccountSecretEnvelopeKey, rejecting
missing stable secrets instead of using
randomServiceAccountSecretEnvelopeAdapter(). Retain the random adapter only for
in-memory repositories, and add a restart replay test using shared durable
storage to verify persisted envelopes remain readable.
- Around line 404-423: Update the registration admission setup in
IamModule.register so missing
registrationIpAdmission/registrationIpAdmissionCounter or
registrationEmailAdmission/registrationEmailAdmissionCounter rejects
configuration instead of creating an implicit InMemoryRecoveryAdmissionAdapter.
Retain RedisRecoveryAdmissionAdapter for configured durable counters and allow
InMemoryRecoveryAdmissionAdapter only when explicitly supplied via the admission
provider options.
---
Outside diff comments:
In `@services/api/src/features/iam/application/invitation.service.ts`:
- Around line 174-239: Update the invitation flow around saveInvitation and
delivery.deliver to persist a durable, encrypted outbox payload containing the
raw bearer token before the transaction commits, then remove or mark it
delivered after successful delivery and retry it on subsequent attempts. Ensure
the existing active token can be recovered for resend without persisting
plaintext token data, and preserve token reuse and revocation semantics so
delivery failures do not return CONFLICT permanently.
---
Nitpick comments:
In
`@services/api/prisma/migrations/20260804010000_iam_service_account_create_idempotency/migration.sql`:
- Around line 25-34: Remove the redundant CREATE UNIQUE INDEX statement named
service_accounts_create_idempotency_workspace_key from the migration, while
retaining service_accounts_create_idempotency_key and the organization-scope
partial index unchanged.
In
`@services/api/src/features/iam/adapter/service-account-secret-envelope.adapter.ts`:
- Around line 64-67: Update the doc comment for
randomServiceAccountSecretEnvelopeAdapter to explicitly state that its
process-local random key cannot decrypt stored envelopes after restarts or
across instances, and that multi-instance or restart-prone deployments must
configure a durable key.
- Around line 40-43: Update the open method in the service-account secret
envelope adapter to reject envelopes whose split result contains anything other
than exactly four segments before decoding or decrypting. Preserve the existing
validation for the v1 version and required encoded fields, while ensuring
trailing framing segments cannot be ignored.
In `@services/api/src/features/iam/application/service-account.service.ts`:
- Around line 208-231: Update createRequestHash to canonicalize permissions
before hashing by sorting the validated permission list in the JSON payload.
Preserve the existing validation and ensure semantically identical permission
sets produce the same order-independent hash for replayCreate.
In `@services/api/test/features/iam/prisma-service-account-repository.test.ts`:
- Line 213: Update the service-account persistence test around
saveServiceAccount to use a distinct plaintext secret marker alongside a shared
secretEnvelope/envelope constant. Assert directly that the persisted row
contains the envelope value and does not contain the separately supplied
plaintext marker, replacing the ineffective JSON.stringify(...).includes('dbsa')
check.
In `@services/api/test/features/iam/service-account-composition.test.ts`:
- Around line 34-46: Extend the IAM composition test around IamModule.register
and the SERVICE_ACCOUNT_SERVICE provider to verify the secret envelope is wired,
not only that ServiceAccountService is instantiated. Exercise the service’s
create path with a stub database and assert it does not return UNAVAILABLE, or
assert the envelope provider registered by IamModule.register is present and
correctly configured.
In `@services/api/test/features/iam/service-account.service.test.ts`:
- Around line 71-92: In the service helper, remove the local digest arrow and
use the existing module-level digestSecret helper when constructing the secrets
array. Keep the generated secret values and resulting digests unchanged.
In `@services/api/test/http-contract.test.ts`:
- Line 336: Extend the contract tests around the sign-in and current-session
response assertions to provide mfaReenrollmentRequired: true and verify the
responses preserve true, while retaining the existing omitted-value fallback
assertion for false.
In `@services/api/test/prisma-foundation.test.mjs`:
- Around line 573-593: Expand the migration assertions in the test covering
active invitations and service-account idempotency to validate the complete SQL
contract, not just object names. Assert the invitation index includes
membership_id and the ACTIVE status predicate, and add checks for
create_idempotency_key, create_request_hash,
service_accounts_create_idempotency_key, each required index column list, and
their partial predicates using the existing migration-content assertion pattern.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5281b260-7016-41d1-aac3-123da13b8514
📒 Files selected for processing (75)
docs/operations/iam-010-invitation-token-2026-08-03.mddocs/operations/iam-registration-2026-08-03.mdpackages/domain/src/entitlements/v1.tspackages/domain/src/invitation/v1.tspackages/domain/test/entitlement-lease-issuance-v1.test.mjspackages/domain/test/invitation-v1.test.mjsservices/api/openapi/v1.jsonservices/api/prisma/migrations/20260804000000_iam_invitation_active_membership_unique/migration.sqlservices/api/prisma/migrations/20260804010000_iam_service_account_create_idempotency/migration.sqlservices/api/prisma/schema/iam.prismaservices/api/src/features/aud/adapter/in-memory-audit-attestation-repository.adapter.tsservices/api/src/features/aud/adapter/in-memory-audit-repository.adapter.tsservices/api/src/features/aud/adapter/prisma-audit-attestation-repository.adapter.tsservices/api/src/features/aud/adapter/prisma-audit-repository.adapter.tsservices/api/src/features/aud/api/audit-attestation.dto.tsservices/api/src/features/aud/application/audit-attestation-repository.port.tsservices/api/src/features/aud/application/audit-attestation.service.tsservices/api/src/features/aud/application/audit-repository.port.tsservices/api/src/features/bua/adapter/in-memory-entitlement-lease-repository.adapter.tsservices/api/src/features/bua/adapter/prisma-entitlement-lease-repository.adapter.tsservices/api/src/features/bua/api/entitlement-lease.dto.tsservices/api/src/features/bua/api/entitlement.controller.tsservices/api/src/features/bua/application/entitlement-equality.tsservices/api/src/features/bua/application/entitlement-lease.service.tsservices/api/src/features/iae/api/artifact-admission.dto.tsservices/api/src/features/iae/api/artifact-retention.dto.tsservices/api/src/features/iae/api/inbox-item.dto.tsservices/api/src/features/iam/adapter/iam-invitation-crypto.adapter.tsservices/api/src/features/iam/adapter/iam-recovery-crypto.adapter.tsservices/api/src/features/iam/adapter/in-memory-service-account-repository.adapter.tsservices/api/src/features/iam/adapter/prisma-mfa-repository.adapter.tsservices/api/src/features/iam/adapter/prisma-recovery-repository.adapter.tsservices/api/src/features/iam/adapter/prisma-service-account-repository.adapter.tsservices/api/src/features/iam/adapter/service-account-secret-envelope.adapter.tsservices/api/src/features/iam/api/auth-session.dto.tsservices/api/src/features/iam/api/authentication.controller.tsservices/api/src/features/iam/api/current-session.dto.tsservices/api/src/features/iam/api/registration.controller.tsservices/api/src/features/iam/api/registration.dto.tsservices/api/src/features/iam/api/service-account.controller.tsservices/api/src/features/iam/application/invitation.service.tsservices/api/src/features/iam/application/mfa-repository.port.tsservices/api/src/features/iam/application/recovery.service.tsservices/api/src/features/iam/application/registration-repository.port.tsservices/api/src/features/iam/application/registration.service.tsservices/api/src/features/iam/application/service-account-repository.port.tsservices/api/src/features/iam/application/service-account.service.tsservices/api/src/features/iam/iam.module.tsservices/api/src/features/sa/api/spreadsheet-audit.dto.tsservices/api/test/features/aud/audit-attestation-repository.test.tsservices/api/test/features/aud/audit-attestation.service.test.tsservices/api/test/features/aud/prisma-audit-attestation-repository.test.tsservices/api/test/features/aud/prisma-audit-repository.test.tsservices/api/test/features/bua/entitlement-equality.test.tsservices/api/test/features/bua/entitlement-lease.service.test.tsservices/api/test/features/bua/entitlement.controller.test.tsservices/api/test/features/iam/iam-invitation-crypto.adapter.test.tsservices/api/test/features/iam/invitation-service.test.tsservices/api/test/features/iam/prisma-recovery-repository.test.tsservices/api/test/features/iam/prisma-service-account-repository.test.tsservices/api/test/features/iam/recovery-composition.test.tsservices/api/test/features/iam/recovery-crypto.test.tsservices/api/test/features/iam/recovery-http.test.tsservices/api/test/features/iam/recovery.service.test.tsservices/api/test/features/iam/registration-composition.test.tsservices/api/test/features/iam/registration-controller.test.tsservices/api/test/features/iam/registration-http.test.tsservices/api/test/features/iam/registration.service.test.tsservices/api/test/features/iam/service-account-composition.test.tsservices/api/test/features/iam/service-account-repository.test.tsservices/api/test/features/iam/service-account-secret-envelope.adapter.test.tsservices/api/test/features/iam/service-account.controller.test.tsservices/api/test/features/iam/service-account.service.test.tsservices/api/test/http-contract.test.tsservices/api/test/prisma-foundation.test.mjs
💤 Files with no reviewable changes (1)
- services/api/src/features/bua/api/entitlement.controller.ts
Comment on lines
+171
to
+173
| } catch { | ||
| // The challenge remains unusable only if the compensating revocation also fails. | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline services/api/src/features/iam/application/recovery.service.ts --match RecoveryService --view expanded
rg -n -C 12 'delivery\.deliver|findChallengeByTokenDigest|saveChallenge|IAM_RECOVERY_REVOKE_INVALID' \
services/api/src/features/iam/application/recovery.service.ts
rg -n -C 10 'delivery failures|saveChallenge|REVOKED|ACTIVE' \
services/api/test/features/iam/recovery.service.test.tsRepository: DatabreezeService/databreeze-platform
Length of output: 10319
Improper Credential Lifecycle (CWE-664)
Reachability: External · Exploitability: Moderate
Reachability path
● Entry
services/api/src/features/iam/adapter/iam-invitation-crypto.adapter.ts:7
validKey
│
▼
● Hop
services/api/src/features/iam/adapter/iam-recovery-crypto.adapter.ts
│
▼
● Sink
services/api/src/features/iam/application/recovery.service.ts
Do not ignore compensating-revocation failures.
If delivery.deliver throws after partial delivery and the compensating saveChallenge fails, the challenge remains ACTIVE. complete() can then use the raw token to reset the password. Persist a retryable revocation task or fail closed until revocation succeeds. Add a test for this failure path.
🤖 Prompt for 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.
In `@services/api/src/features/iam/application/recovery.service.ts` around lines
171 - 173, Update the compensating-revocation handling in the recovery flow
around delivery and saveChallenge so a saveChallenge failure is not swallowed
while the challenge remains ACTIVE. Persist a retryable revocation task or fail
closed until revocation succeeds, and add a test covering delivery failure
followed by compensating-revocation failure; ensure complete() cannot use the
raw token in that state.
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
1 participant
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.
Promotion
Promotes the reviewed IAM security completion and the follow-up IAM hardening batch from
devtomain.Included
Verification
corepack pnpm repo:checkcorepack pnpm repo:builddevwith all checks passing@coderabbitai full review
Summary by CodeRabbit
202 Acceptedresponse with admission controls.