feat: 006 AI advisory output authority 격리 구현 - #287
Conversation
📝 WalkthroughWalkthroughThis change implements T044 AI advisory authority proofs. It adds shared proof contracts, immutable Prisma persistence, transactional state verification, guarded API creation, display-only policy references, strict lifecycle payload validation, regression tests, and updated T044 design and completion checks. ChangesAI advisory authority-proof isolation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AiAdvisoryController
participant AiAdvisoryAuthorityService
participant PrismaSastAiAdvisoryAuthorityStore
participant PrismaDatabase
Client->>AiAdvisoryController: POST authority proof intent
AiAdvisoryController->>AiAdvisoryAuthorityService: createProof(intent)
AiAdvisoryAuthorityService->>PrismaSastAiAdvisoryAuthorityStore: createProof(tenantId, advisoryId, verifiedAt)
PrismaSastAiAdvisoryAuthorityStore->>PrismaDatabase: read advisory and authoritative state
PrismaDatabase-->>PrismaSastAiAdvisoryAuthorityStore: scoped rows and state digests
PrismaSastAiAdvisoryAuthorityStore->>PrismaDatabase: insert immutable proof
PrismaDatabase-->>PrismaSastAiAdvisoryAuthorityStore: persisted proof or replay
PrismaSastAiAdvisoryAuthorityStore-->>AiAdvisoryAuthorityService: persisted proof
AiAdvisoryAuthorityService-->>AiAdvisoryController: proof and policy reference
AiAdvisoryController-->>Client: authority-proof response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13593d54a8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| tx.policyDecision.findMany({ | ||
| where: { | ||
| tenantId: scope.tenantId, | ||
| findingId: scope.normalizedFindingId | ||
| }, |
There was a problem hiding this comment.
Snapshot the state used by policy lifecycle services
When policy, waiver, or suppression changes are made through the current APIs, these Prisma queries cannot observe them: PolicyEngineService and PolicyLifecycleService store their authoritative objects only in in-memory arrays and never populate the tables queried by captureAuthorityState. Consequently, a concurrent evaluation, waiver update, or suppression creation can leave both database snapshots identical and produce a proof claiming zero authoritative writes even though application-visible state changed. Persist those services through the same database transaction model, or snapshot their actual stores before treating this proof as valid.
AGENTS.md reference: AGENTS.md:L79-L79
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (8)
apps/api/src/ai-plane/prisma-sast-ai-advisory-authority.store.ts (4)
527-547: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the zero-authority columns from the validated proof instead of repeating literals.
buildSastAiAdvisoryAuthorityProofalready validatesproof.authorityandproof.auditagainst the sharedZERO_AUTHORITYandAUDITconstants. The literal list here duplicates that contract. If the shared contract gains a field, this list drifts silently and the new column is written with a default or omitted.Spreading the validated objects keeps the same guarantee and removes the duplication.
♻️ Proposed change
- findingCreateAuthority: false, - findingStatusMutationAuthority: false, - findingSeverityMutationAuthority: false, - lifecycleMutationAuthority: false, - waiverMutationAuthority: false, - suppressionMutationAuthority: false, - policyOverrideAuthority: false, - blockDecisionAuthority: false, - publicationAuthority: false, - scmWriteAuthority: false, - advisoryOnly: true, - proofLedgerWritten: true, - authoritativeFindingWritten: false, - lifecycleStateWritten: false, - policyDecisionWritten: false, - waiverWritten: false, - suppressionWritten: false, - callerAuthorityFieldsAccepted: false, - advisoryContentStored: false, - sourceContentStored: false, - secretValueStored: false, + ...proof.authority, + ...proof.audit,🤖 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 `@apps/api/src/ai-plane/prisma-sast-ai-advisory-authority.store.ts` around lines 527 - 547, Update the authority result construction in buildSastAiAdvisoryAuthorityProof to spread the validated proof.authority and proof.audit objects instead of repeating individual zero-authority and audit field literals. Preserve the existing derived fields and ensure all shared contract fields are persisted automatically.
593-595: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared version constant instead of a literal.
Line 594 hardcodes
'sast-ai-advisory-authority-proof-v1'. The shared package exportsSAST_AI_ADVISORY_AUTHORITY_PROOF_VERSION, andbuildSastAiAdvisoryAuthorityProofuses that constant. If the shared contract version changes, this literal does not change with it, andisSastAiAdvisoryAuthorityProofShapeValidthen rejects every stored proof at line 647.♻️ Proposed change
import { SAST_AI_ADVISORY_AUTHORITY_LIMITS, + SAST_AI_ADVISORY_AUTHORITY_PROOF_VERSION, buildSastAiAdvisoryAuthorityProof,const proof: SastAiAdvisoryAuthorityProof = { - version: 'sast-ai-advisory-authority-proof-v1', + version: SAST_AI_ADVISORY_AUTHORITY_PROOF_VERSION, proofId: row.id,🤖 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 `@apps/api/src/ai-plane/prisma-sast-ai-advisory-authority.store.ts` around lines 593 - 595, Update the proof construction in the visible proof-mapping logic to set version from the shared SAST_AI_ADVISORY_AUTHORITY_PROOF_VERSION constant instead of the hardcoded string, matching buildSastAiAdvisoryAuthorityProof and keeping stored proofs compatible with isSastAiAdvisoryAuthorityProofShapeValid.
444-457: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReport a missing lifecycle state separately from an over-broad state.
Line 447 treats
lifecycleStates.length !== 1asSTATE_TOO_BROAD. When the count is zero, the cause is a missing lifecycle row, not an over-broad state. The shared contract pinslifecycleStateCount: 1, so zero must still fail. Only the reported category is wrong, and it misdirects triage.♻️ Proposed change
+ if (lifecycleStates.length === 0) { + throw new SastAiAdvisoryAuthorityPersistenceError( + 'CONTEXT_DRIFT' + ); + } if ( findings.length > SAST_AI_ADVISORY_AUTHORITY_LIMITS.maximumNormalizedFindings || - lifecycleStates.length !== 1 || + lifecycleStates.length > 1 || policyDecisions.length >🤖 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 `@apps/api/src/ai-plane/prisma-sast-ai-advisory-authority.store.ts` around lines 444 - 457, Update the validation around lifecycleStates in the persistence flow to distinguish zero lifecycle states from multiple states: throw the contract’s missing-lifecycle-state error when lifecycleStates.length is zero, while retaining STATE_TOO_BROAD only for counts above one and preserving the existing limit checks.
682-684: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winThe canonical digester is duplicated in two files. Both files define an identical
digestfunction and pass it to the shared builders as theSastAiAdvisoryAuthorityCanonicalDigester. The store uses it to compute and persistproofDigest. The service uses it to validate the same proof and to build the policy reference. If the two definitions ever diverge, the service rejects every persisted proof andverifyPolicyReferencefails for all requests. The digester belongs to the proof contract surface, so it should live beside the contract.
apps/api/src/ai-plane/prisma-sast-ai-advisory-authority.store.ts#L682-L684: remove the localdigestfunction and import the shared canonical digester.apps/api/src/ai-plane/ai-advisory-authority.service.ts#L121-L123: remove the localdigestfunction and import the same shared canonical digester.As per coding guidelines: "Place shared API contracts in
packages/shared."🤖 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 `@apps/api/src/ai-plane/prisma-sast-ai-advisory-authority.store.ts` around lines 682 - 684, The canonical digest implementation is duplicated across the store and service. Move the shared `digest` implementation to the proof contract surface in `packages/shared`, then remove the local functions and import that canonical digester in `apps/api/src/ai-plane/prisma-sast-ai-advisory-authority.store.ts` lines 682-684 and `apps/api/src/ai-plane/ai-advisory-authority.service.ts` lines 121-123, preserving the existing `SastAiAdvisoryAuthorityCanonicalDigester` usage in both files.Source: Coding guidelines
apps/api/src/ai-plane/ai-advisory-authority.service.ts (2)
73-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog the failure category before returning false.
The empty catch fails closed, which is correct for an authority check. It also silently hides database outages from the policy path. The class already has a logger, and
safeErrorCategoryproduces a leakage-safe value.♻️ Proposed change
try { return await this.store.verifyPolicyReference(input); - } catch { + } catch (error) { + this.logger.error( + `AI advisory policy reference verification failed (${safeErrorCategory(error)}).` + ); return false; }🤖 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 `@apps/api/src/ai-plane/ai-advisory-authority.service.ts` around lines 73 - 77, Update the catch block around store.verifyPolicyReference in the policy authority check to log the failure category using the class logger and safeErrorCategory, then continue returning false to preserve fail-closed behavior. Keep the logged value leakage-safe and avoid exposing raw error details.
53-58: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMap persistence categories to distinct HTTP statuses.
The catch block converts every failure into a 404. That includes
REPLAY_CONFLICT,STATE_DRIFT, serialization conflicts after the retry budget, and database connectivity failures. A caller cannot distinguish "no such advisory" from "retry later", and a database outage is reported as a normal 404, which hides the incident from status-code-based alerting.Keep the opaque message to avoid leakage. Select the status from
safeErrorCategoryor from the persistence error category.🤖 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 `@apps/api/src/ai-plane/ai-advisory-authority.service.ts` around lines 53 - 58, Update the catch block in the AI advisory authority proof flow to map safeErrorCategory(error) or the persistence error category to distinct HTTP statuses: preserve 404 only for missing advisories, use conflict semantics for REPLAY_CONFLICT and STATE_DRIFT, and a retryable server/unavailable status for exhausted serialization conflicts or database connectivity failures. Keep the opaque error message and existing logging while replacing the unconditional unavailable() response.apps/api/prisma/schema.prisma (1)
1735-1784: 🔒 Security & Privacy | 🔵 TrivialPlan the tenant purge path for the immutable proof ledger.
tenant,repositoryBinding,scanRequest,handoff,advisory,occurrence, andnormalizedFindingall useonDelete: Restrict, and the migration adds aBEFORE DELETEtrigger. Tenant deletion and scan-data retention jobs will now fail once a proof row exists. Document the privileged maintenance procedure that disables the trigger, and add an operational runbook step for tenant offboarding.🤖 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 `@apps/api/prisma/schema.prisma` around lines 1735 - 1784, Document the privileged maintenance procedure for purging SastAiAdvisoryAuthorityProof rows, including temporarily disabling and re-enabling the BEFORE DELETE trigger while preserving referential integrity. Add an operational runbook step for tenant offboarding and scan-data retention that invokes this procedure before deleting restricted parent records, referencing SastAiAdvisoryAuthorityProof and its tenantId field.packages/shared/test/sast-ai-advisory-authority.test.mjs (1)
82-133: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd negative cases for digest tampering and bounded sets.
The tests cover state drift, non-exact intent, and authority tampering. Two contract invariants remain untested:
- A mutated
proofId,scope, orproofDigestmust failisSastAiAdvisoryAuthorityProofShapeValid. This proves the digest binding, not only the authority bits.buildSastAiAdvisoryAuthorityStateSnapshotmust returnnullfor unsorted or duplicate digest arrays, for an emptynormalizedFindingDigests, and whentargetFindingDigestis absent fromnormalizedFindingDigests.These paths carry the zero-authority guarantee described in the PR objectives.
🧪 Proposed additional cases
+test('T044 rejects tampered proof identity and unsorted snapshot sets', () => { + const snapshot = authoritySnapshot(); + const proof = buildSastAiAdvisoryAuthorityProof({ + scope: authorityScope(), + before: snapshot, + after: snapshot, + verifiedAt: '2026-08-11T05:30:00.000Z', + digestCanonical: digest + }); + assert.ok(proof); + assert.equal( + isSastAiAdvisoryAuthorityProofShapeValid( + { ...proof, proofDigest: digest('tampered') }, + digest + ), + false + ); + assert.equal( + isSastAiAdvisoryAuthorityProofShapeValid( + { ...proof, scope: { ...proof.scope, tenantId: 'tenant-other' } }, + digest + ), + false + ); + const unsorted = [digest('finding-b'), digest('finding-a')].sort().reverse(); + assert.equal( + buildSastAiAdvisoryAuthorityStateSnapshot({ + normalizedFindingDigests: unsorted, + targetFindingDigest: unsorted[0], + lifecycleStateDigests: [digest('lifecycle-open')], + policyDecisionDigests: [], + waiverDigests: [], + suppressionDigests: [], + digestCanonical: digest + }), + null + ); + assert.equal( + buildSastAiAdvisoryAuthorityStateSnapshot({ + normalizedFindingDigests: [digest('finding-a')], + targetFindingDigest: digest('finding-missing'), + lifecycleStateDigests: [digest('lifecycle-open')], + policyDecisionDigests: [], + waiverDigests: [], + suppressionDigests: [], + digestCanonical: digest + }), + 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 `@packages/shared/test/sast-ai-advisory-authority.test.mjs` around lines 82 - 133, Extend the T044 test coverage with negative cases proving digest binding: mutate proofId, scope, and proofDigest independently and assert isSastAiAdvisoryAuthorityProofShapeValid returns false. Also assert buildSastAiAdvisoryAuthorityStateSnapshot returns null for unsorted or duplicate digest arrays, an empty normalizedFindingDigests array, and a targetFindingDigest not present in that array.
🤖 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
`@apps/api/prisma/migrations/20260811140000_sast_ai_advisory_authority_proof/migration.sql`:
- Around line 144-166: Replace ON UPDATE CASCADE with ON UPDATE RESTRICT for all
five SastAiAdvisoryAuthorityProof foreign keys in
apps/api/prisma/migrations/20260811140000_sast_ai_advisory_authority_proof/migration.sql
lines 144-166. Apply the same change to the three corresponding constraint
definitions in apps/api/scripts/apply-online-sast-runtime-schema.mjs lines
664-684, and align the SastAiAdvisoryAuthorityProof relation attributes in
apps/api/prisma/schema.prisma.
In `@apps/api/src/ai-plane/ai-advisory.controller.ts`:
- Around line 26-32: Update createAuthorityProof in the AI advisory controller
so it does not trust body.tenantId after InternalServiceGuard authentication.
Obtain the caller’s tenant from tenant-scoped internal credentials or another
trusted authenticated binding, validate or override the request tenant
accordingly, and pass only that authorized tenant to
authorityService.createProof.
In `@apps/api/src/ai-plane/prisma-sast-ai-advisory-authority.store.ts`:
- Around line 140-145: Remove the redundant post-write captureAuthorityState
call and its stateDigest comparison from the transaction callback, while
retaining the initial snapshot and existing serializable transaction protection.
Ensure proof creation no longer performs the unnecessary second state read or
claims in-transaction verification.
- Around line 109-120: Replace the ineffective current-state comparison in the
existing-proof branch of runSerializable with a database fence, version
predicate, or lock that covers the full authoritative scope while keeping proof
insertion atomic. Preserve replay drift rejection and only return replayed from
replayProof when the stored authoritative state still matches; otherwise throw
SastAiAdvisoryAuthorityPersistenceError with STATE_DRIFT.
In `@apps/api/test/ai-plane/ai-advisory-authority.service.e2e-spec.ts`:
- Around line 74-96: Extend the test for
AiAdvisoryAuthorityService.verifyPolicyReference to cover persisted-reference
failures: configure store.verifyPolicyReference to return false and separately
to reject or otherwise represent an unavailable result, then assert both service
calls resolve to false. Preserve the existing valid and locally invalid
reference assertions and update call-count expectations to account for the added
cases.
In `@apps/api/test/ai-plane/sast-ai-advisory-authority-persistence.e2e-spec.ts`:
- Around line 113-146: Extend the test “verifies only a tenant and finding-bound
exact policy reference” to assert verifyPolicyReference returns false for a
foreign normalizedFindingId and for a tampered authority proof reference,
including changed authorityProofDigest or advisoryId. Keep the existing valid
and foreign-tenant assertions unchanged.
- Around line 77-111: Update the transaction fixture and the createProof test to
capture the second $transaction options argument. Assert that runSerializable
invokes $transaction with isolationLevel set to
Prisma.TransactionIsolationLevel.Serializable, plus the expected maxWait and
timeout values.
---
Nitpick comments:
In `@apps/api/prisma/schema.prisma`:
- Around line 1735-1784: Document the privileged maintenance procedure for
purging SastAiAdvisoryAuthorityProof rows, including temporarily disabling and
re-enabling the BEFORE DELETE trigger while preserving referential integrity.
Add an operational runbook step for tenant offboarding and scan-data retention
that invokes this procedure before deleting restricted parent records,
referencing SastAiAdvisoryAuthorityProof and its tenantId field.
In `@apps/api/src/ai-plane/ai-advisory-authority.service.ts`:
- Around line 73-77: Update the catch block around store.verifyPolicyReference
in the policy authority check to log the failure category using the class logger
and safeErrorCategory, then continue returning false to preserve fail-closed
behavior. Keep the logged value leakage-safe and avoid exposing raw error
details.
- Around line 53-58: Update the catch block in the AI advisory authority proof
flow to map safeErrorCategory(error) or the persistence error category to
distinct HTTP statuses: preserve 404 only for missing advisories, use conflict
semantics for REPLAY_CONFLICT and STATE_DRIFT, and a retryable
server/unavailable status for exhausted serialization conflicts or database
connectivity failures. Keep the opaque error message and existing logging while
replacing the unconditional unavailable() response.
In `@apps/api/src/ai-plane/prisma-sast-ai-advisory-authority.store.ts`:
- Around line 527-547: Update the authority result construction in
buildSastAiAdvisoryAuthorityProof to spread the validated proof.authority and
proof.audit objects instead of repeating individual zero-authority and audit
field literals. Preserve the existing derived fields and ensure all shared
contract fields are persisted automatically.
- Around line 593-595: Update the proof construction in the visible
proof-mapping logic to set version from the shared
SAST_AI_ADVISORY_AUTHORITY_PROOF_VERSION constant instead of the hardcoded
string, matching buildSastAiAdvisoryAuthorityProof and keeping stored proofs
compatible with isSastAiAdvisoryAuthorityProofShapeValid.
- Around line 444-457: Update the validation around lifecycleStates in the
persistence flow to distinguish zero lifecycle states from multiple states:
throw the contract’s missing-lifecycle-state error when lifecycleStates.length
is zero, while retaining STATE_TOO_BROAD only for counts above one and
preserving the existing limit checks.
- Around line 682-684: The canonical digest implementation is duplicated across
the store and service. Move the shared `digest` implementation to the proof
contract surface in `packages/shared`, then remove the local functions and
import that canonical digester in
`apps/api/src/ai-plane/prisma-sast-ai-advisory-authority.store.ts` lines 682-684
and `apps/api/src/ai-plane/ai-advisory-authority.service.ts` lines 121-123,
preserving the existing `SastAiAdvisoryAuthorityCanonicalDigester` usage in both
files.
In `@packages/shared/test/sast-ai-advisory-authority.test.mjs`:
- Around line 82-133: Extend the T044 test coverage with negative cases proving
digest binding: mutate proofId, scope, and proofDigest independently and assert
isSastAiAdvisoryAuthorityProofShapeValid returns false. Also assert
buildSastAiAdvisoryAuthorityStateSnapshot returns null for unsorted or duplicate
digest arrays, an empty normalizedFindingDigests array, and a
targetFindingDigest not present in that array.
🪄 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: 5573f27f-b1c3-42f8-b920-7047371cf71b
📒 Files selected for processing (33)
apps/api/prisma/migrations/20260811140000_sast_ai_advisory_authority_proof/migration.sqlapps/api/prisma/schema.prismaapps/api/scripts/apply-online-sast-runtime-schema.mjsapps/api/src/ai-plane/ai-advisory-authority.service.tsapps/api/src/ai-plane/ai-advisory.controller.tsapps/api/src/ai-plane/ai-plane.module.tsapps/api/src/ai-plane/prisma-sast-ai-advisory-authority.store.tsapps/api/src/ai-plane/sast-ai-advisory-authority.store.tsapps/api/src/policy/policy-engine.service.tsapps/api/src/policy/policy-lifecycle.service.tsapps/api/src/policy/policy.module.tsapps/api/test/ai-plane/ai-advisory-authority.service.e2e-spec.tsapps/api/test/ai-plane/ai-advisory.e2e-spec.tsapps/api/test/ai-plane/sast-ai-advisory-authority-persistence.e2e-spec.tsapps/api/test/policy/policy-decisions.e2e-spec.tsapps/api/test/policy/policy-engine.service.e2e-spec.tsapps/api/test/policy/waiver-suppression-lifecycle.e2e-spec.tsapps/api/test/support/sast-ai-advisory-fixture.tspackages/shared/src/index.tspackages/shared/src/types/production-architecture.tspackages/shared/src/types/sast-ai-advisory-authority.tspackages/shared/test/sast-ai-advisory-authority.test.mjsspecs/006-production-sast-runtime-design/contracts/sast-runtime.mdspecs/006-production-sast-runtime-design/data-model.mdspecs/006-production-sast-runtime-design/plan.mdspecs/006-production-sast-runtime-design/quality-gates.mdspecs/006-production-sast-runtime-design/quickstart.mdspecs/006-production-sast-runtime-design/research.mdspecs/006-production-sast-runtime-design/spec.mdspecs/006-production-sast-runtime-design/tasks.mdspecs/006-production-sast-runtime-design/threat-model.mdtest/github-actions/active-feature.test.mjstest/github-actions/ontology.test.mjs
| ALTER TABLE "SastAiAdvisoryAuthorityProof" | ||
| ADD CONSTRAINT "SastAiAdvisoryAuthorityProof_tenantId_fkey" | ||
| FOREIGN KEY ("tenantId") REFERENCES "Tenant"("id") | ||
| ON DELETE RESTRICT ON UPDATE CASCADE; | ||
| ALTER TABLE "SastAiAdvisoryAuthorityProof" | ||
| ADD CONSTRAINT "SastAiAdvisoryAuthorityProof_repository_scope_fkey" | ||
| FOREIGN KEY ("repositoryBindingId", "tenantId") | ||
| REFERENCES "RepositoryBinding"("id", "tenantId") | ||
| ON DELETE RESTRICT ON UPDATE CASCADE; | ||
| ALTER TABLE "SastAiAdvisoryAuthorityProof" | ||
| ADD CONSTRAINT "SastAiAdvisoryAuthorityProof_scan_scope_fkey" | ||
| FOREIGN KEY ("scanRequestId", "tenantId", "repositoryBindingId") | ||
| REFERENCES "ScanRequest"("id", "tenantId", "repositoryBindingId") | ||
| ON DELETE RESTRICT ON UPDATE CASCADE; | ||
| ALTER TABLE "SastAiAdvisoryAuthorityProof" | ||
| ADD CONSTRAINT "SastAiAdvisoryAuthorityProof_handoff_scope_fkey" | ||
| FOREIGN KEY ("handoffId", "tenantId") | ||
| REFERENCES "SastAiAdvisoryHandoff"("id", "tenantId") | ||
| ON DELETE RESTRICT ON UPDATE CASCADE; | ||
| ALTER TABLE "SastAiAdvisoryAuthorityProof" | ||
| ADD CONSTRAINT "SastAiAdvisoryAuthorityProof_advisoryId_fkey" | ||
| FOREIGN KEY ("advisoryId") REFERENCES "AiAdvisoryMetadata"("id") | ||
| ON DELETE RESTRICT ON UPDATE CASCADE; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
ON UPDATE CASCADE contradicts the immutable proof ledger. All eight foreign keys on SastAiAdvisoryAuthorityProof declare ON UPDATE CASCADE, but the SastAiAdvisoryAuthorityProof_immutable_update trigger rejects every UPDATE on that table with SQLSTATE 55000. A cascaded parent-key update issues an UPDATE on the proof row and therefore aborts the parent update with a confusing immutability error.
apps/api/prisma/migrations/20260811140000_sast_ai_advisory_authority_proof/migration.sql#L144-L166: change the five foreign keys toON DELETE RESTRICT ON UPDATE RESTRICT.apps/api/scripts/apply-online-sast-runtime-schema.mjs#L664-L684: change the three constraint definitions toON DELETE RESTRICT ON UPDATE RESTRICT, and keepapps/api/prisma/schema.prismarelation attributes aligned.
📍 Affects 2 files
apps/api/prisma/migrations/20260811140000_sast_ai_advisory_authority_proof/migration.sql#L144-L166(this comment)apps/api/scripts/apply-online-sast-runtime-schema.mjs#L664-L684
🤖 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
`@apps/api/prisma/migrations/20260811140000_sast_ai_advisory_authority_proof/migration.sql`
around lines 144 - 166, Replace ON UPDATE CASCADE with ON UPDATE RESTRICT for
all five SastAiAdvisoryAuthorityProof foreign keys in
apps/api/prisma/migrations/20260811140000_sast_ai_advisory_authority_proof/migration.sql
lines 144-166. Apply the same change to the three corresponding constraint
definitions in apps/api/scripts/apply-online-sast-runtime-schema.mjs lines
664-684, and align the SastAiAdvisoryAuthorityProof relation attributes in
apps/api/prisma/schema.prisma.
| @Post('authority-proofs') | ||
| @UseGuards(InternalServiceGuard) | ||
| createAuthorityProof( | ||
| @Body() body: SastAiAdvisoryAuthorityProofIntent | ||
| ) { | ||
| return this.authorityService.createProof(body); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect InternalServiceGuard to determine whether it binds a tenant.
fd -g 'internal-service.guard.ts' --exec cat -n {}
rg -n -C 3 --type=ts 'CurrentTenant|tenantId' -g '**/internal-service*'Repository: AigisAI/AegisAI_v2
Length of output: 1675
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- candidate files ---'
fd -t f | rg '(^|/)(ai-advisory|authority|internal-service|tenant).*\\.(ts|tsx)$' | head -80
printf '%s\n' '--- relevant symbols and routes ---'
rg -n -C 4 --type=ts 'createAuthorityProof|createProof|SastAiAdvisoryAuthorityProofIntent|InternalServiceGuard|CurrentTenant' apps
printf '%s\n' '--- tenant checks in authority-proof paths ---'
rg -n -C 4 --type=ts 'authority.?proof|tenantId|tenant_id' apps/api/src | head -300Repository: AigisAI/AegisAI_v2
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- authority service ---'
sed -n '1,180p' apps/api/src/ai-plane/ai-advisory-authority.service.ts
printf '%s\n' '--- authority store ---'
sed -n '1,240p' apps/api/src/ai-plane/prisma-sast-ai-advisory-authority.store.ts
printf '%s\n' '--- controller and tenant decorator ---'
sed -n '1,100p' apps/api/src/ai-plane/ai-advisory.controller.ts
sed -n '1,80p' apps/api/src/auth/decorators/current-tenant.decorator.ts
printf '%s\n' '--- endpoint tests ---'
sed -n '1,180p' apps/api/test/ai-plane/ai-advisory.e2e-spec.tsRepository: AigisAI/AegisAI_v2
Length of output: 17210
🏁 Script executed:
#!/bin/bash
python3 - <<'PY'
from pathlib import Path
import re
root = Path("apps/api/src")
guard = (root / "common/security/internal-service.guard.ts").read_text()
controller = (root / "ai-plane/ai-advisory.controller.ts").read_text()
service = (root / "ai-plane/ai-advisory-authority.service.ts").read_text()
store = (root / "ai-plane/prisma-sast-ai-advisory-authority.store.ts").read_text()
print("guard_reads_tenant:", bool(re.search(r"\btenantId\b|\bCurrentTenant\b", guard)))
print("controller_uses_internal_guard:", "InternalServiceGuard" in controller)
print("controller_body_to_createProof:",
bool(re.search(r"createProof\s*\(\s*body\s*\)", controller)))
print("service_forwards_body_tenant:",
bool(re.search(r"tenantId\s*:\s*input\.tenantId", service)))
print("store_advisory_tenant_filter:",
bool(re.search(r"where:\s*\{\s*id:\s*input\.advisoryId,\s*tenantId:\s*input\.tenantId", store, re.S)))
print("store_authenticated_tenant_reference:",
bool(re.search(r"request|user|CurrentTenant|session|principal", store, re.I)))
assert not re.search(r"\btenantId\b|\bCurrentTenant\b", guard)
assert "InternalServiceGuard" in controller
assert re.search(r"createProof\s*\(\s*body\s*\)", controller)
assert re.search(r"tenantId\s*:\s*input\.tenantId", service)
assert re.search(
r"where:\s*\{\s*id:\s*input\.advisoryId,\s*tenantId:\s*input\.tenantId",
store, re.S
)
PY
printf '%s\n' '--- proof return construction ---'
rg -n -C 5 'return replayProof|function replayProof|proofData|buildSastAiAdvisoryAuthorityProof' \
apps/api/src/ai-plane/prisma-sast-ai-advisory-authority.store.tsRepository: AigisAI/AegisAI_v2
Length of output: 2449
🏁 Script executed:
#!/bin/bash
set -e
sed -n '200,360p' apps/api/src/ai-plane/prisma-sast-ai-advisory-authority.store.ts
sed -n '490,610p' apps/api/src/ai-plane/prisma-sast-ai-advisory-authority.store.tsRepository: AigisAI/AegisAI_v2
Length of output: 9141
Bind tenantId to the authenticated internal caller. InternalServiceGuard validates only the shared bearer secret, and createProof forwards the body-supplied tenantId to the store. The store validates the advisory and tenant pair but does not authorize the caller for that tenant. Use tenant-scoped internal credentials or another trusted tenant binding before creating the proof.
🤖 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 `@apps/api/src/ai-plane/ai-advisory.controller.ts` around lines 26 - 32, Update
createAuthorityProof in the AI advisory controller so it does not trust
body.tenantId after InternalServiceGuard authentication. Obtain the caller’s
tenant from tenant-scoped internal credentials or another trusted authenticated
binding, validate or override the request tenant accordingly, and pass only that
authorized tenant to authorityService.createProof.
| if (existing) { | ||
| const replayed = replayProof(existing, context); | ||
| const current = await captureAuthorityState(tx, context); | ||
| if ( | ||
| current.stateDigest !== replayed.proof.before.stateDigest | ||
| ) { | ||
| throw new SastAiAdvisoryAuthorityPersistenceError( | ||
| 'STATE_DRIFT' | ||
| ); | ||
| } | ||
| return replayed; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Locate the replay and drift requirements for T044/FR-052 in the spec files.
fd -e md . specs/006-production-sast-runtime-design | while IFS= read -r file; do
rg -n -C 5 'FR-052|T044|replay|REPLAY_CONFLICT|STATE_DRIFT' "$file"
doneRepository: AigisAI/AegisAI_v2
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- Relevant specification text ---'
fd -e md . specs/006-production-sast-runtime-design | while IFS= read -r file; do
if rg -q 'T044|FR-052|exact advisory replay|changed state|STATE_DRIFT|identical before/after' "$file"; then
printf '\nFILE: %s\n' "$file"
rg -n -C 4 --max-count 20 'T044|FR-052|exact advisory replay|changed state|STATE_DRIFT|identical before/after' "$file"
fi
done
printf '%s\n' '--- Implementation and callers ---'
rg -n -C 8 'replayProof|captureAuthorityState|STATE_DRIFT|createProof' apps/api/src/ai-planeRepository: AigisAI/AegisAI_v2
Length of output: 31104
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- Replay validation and error mapping ---'
sed -n '553,590p' apps/api/src/ai-plane/prisma-sast-ai-advisory-authority.store.ts
sed -n '35,75p' apps/api/src/ai-plane/ai-advisory-authority.service.ts
printf '%s\n' '--- State digest fields ---'
sed -n '334,430p' apps/api/src/ai-plane/prisma-sast-ai-advisory-authority.store.ts
rg -n -C 5 'updatedAt|reconciliation|lifecycleState.*update|waiver.*update|suppression.*update' apps/api/src packagesRepository: AigisAI/AegisAI_v2
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
import re
path = Path("apps/api/src/ai-plane/prisma-sast-ai-advisory-authority.store.ts")
text = path.read_text()
create = re.search(
r"const before = await captureAuthorityState\(tx, context\);(?P<body>.*?)"
r"const after = await captureAuthorityState\(tx, context\);",
text,
re.S,
)
if not create:
raise SystemExit("createProof before/after sequence not found")
body = create.group("body")
writes = re.findall(r"\btx\.([A-Za-z0-9_]+)\.(create|createMany|update|updateMany|upsert|delete|deleteMany)\b", body)
print("writes_between_authority_captures:", writes)
print("only_proof_ledger_write:",
writes == [("sastAiAdvisoryAuthorityProof", "create")])
run = re.search(
r"async function runSerializable|private async runSerializable",
text,
)
print("runSerializable_declared_in_file:", bool(run))
print("\nrelevant_sequence:")
for i, line in enumerate(text.splitlines(), 1):
if 120 <= i <= 146:
print(f"{i}: {line}")
PYRepository: AigisAI/AegisAI_v2
Length of output: 1304
🏁 Script executed:
#!/bin/bash
set -e
rg -n -C 12 'runSerializable|isolationLevel|SastAiAdvisoryAuthorityProof' \
apps/api/src/ai-plane/prisma-sast-ai-advisory-authority.store.ts \
apps/api/src/ai-plane/prisma.service.ts \
apps/api/prisma/schema.prisma prisma/schema.prisma 2>/dev/null || trueRepository: AigisAI/AegisAI_v2
Length of output: 48486
Replace the tautological post-write drift check
runSerializable uses one SERIALIZABLE transaction snapshot. The only intervening write is SastAiAdvisoryAuthorityProof.create, which captureAuthorityState does not read. Therefore, before.stateDigest !== after.stateDigest cannot detect authoritative drift. Use a database fence, version predicate, or locking strategy that covers the complete authoritative scope while preserving atomic proof insertion. Keep replay drift rejection because changed state must fail closed; exact retry applies only when the stored state still matches.
🤖 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 `@apps/api/src/ai-plane/prisma-sast-ai-advisory-authority.store.ts` around
lines 109 - 120, Replace the ineffective current-state comparison in the
existing-proof branch of runSerializable with a database fence, version
predicate, or lock that covers the full authoritative scope while keeping proof
insertion atomic. Preserve replay drift rejection and only return replayed from
replayProof when the stored authoritative state still matches; otherwise throw
SastAiAdvisoryAuthorityPersistenceError with STATE_DRIFT.
| const after = await captureAuthorityState(tx, context); | ||
| if (before.stateDigest !== after.stateDigest) { | ||
| throw new SastAiAdvisoryAuthorityPersistenceError( | ||
| 'STATE_DRIFT' | ||
| ); | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
The after snapshot cannot differ from before inside the same serializable transaction.
Lines 122 and 140 capture state twice within one transaction. PostgreSQL SERIALIZABLE uses a single consistent snapshot per transaction, and the transaction writes only to sastAiAdvisoryAuthorityProof. Therefore after.stateDigest always equals before.stateDigest, and the check at line 141 can never fail.
Two consequences:
- The proof claims "authoritative state unchanged during proof creation", but the in-transaction re-read provides no additional evidence.
- Each proof creation runs five extra
findManyqueries for no benefit.
The real protection comes from serializable conflict detection plus the replay-time comparison at lines 112-118. Consider removing the second capture, or move the second capture outside the transaction if a genuine post-commit verification is required.
♻️ Proposed simplification
const created =
await tx.sastAiAdvisoryAuthorityProof.create({
data: proofData(proof)
});
- const after = await captureAuthorityState(tx, context);
- if (before.stateDigest !== after.stateDigest) {
- throw new SastAiAdvisoryAuthorityPersistenceError(
- 'STATE_DRIFT'
- );
- }
return replayProof(created, context, false);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const after = await captureAuthorityState(tx, context); | |
| if (before.stateDigest !== after.stateDigest) { | |
| throw new SastAiAdvisoryAuthorityPersistenceError( | |
| 'STATE_DRIFT' | |
| ); | |
| } |
🤖 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 `@apps/api/src/ai-plane/prisma-sast-ai-advisory-authority.store.ts` around
lines 140 - 145, Remove the redundant post-write captureAuthorityState call and
its stateDigest comparison from the transaction callback, while retaining the
initial snapshot and existing serializable transaction protection. Ensure proof
creation no longer performs the unnecessary second state read or claims
in-transaction verification.
| it('fails closed when a policy reference is missing, drifted, or unavailable', async () => { | ||
| const store = { | ||
| createProof: jest.fn(), | ||
| verifyPolicyReference: jest.fn().mockResolvedValue(true) | ||
| }; | ||
| const service = new AiAdvisoryAuthorityService(store as never); | ||
| const reference = aiPolicyReference(); | ||
|
|
||
| await expect( | ||
| service.verifyPolicyReference({ | ||
| tenantId: 'tenant-ai', | ||
| normalizedFindingId: 'normalized-finding-ai', | ||
| reference | ||
| }) | ||
| ).resolves.toBe(true); | ||
| await expect( | ||
| service.verifyPolicyReference({ | ||
| tenantId: 'tenant-ai', | ||
| normalizedFindingId: 'normalized-finding-ai', | ||
| reference: { ...reference, suggestedAction: 'BLOCK' } as never | ||
| }) | ||
| ).resolves.toBe(false); | ||
| expect(store.verifyPolicyReference).toHaveBeenCalledTimes(1); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add missing and unavailable reference cases.
This test verifies one valid reference and one locally invalid reference. It does not make store.verifyPolicyReference return false or reject. A service that accepts a missing, drifted, or unavailable persisted proof can pass this test.
Add cases for a false result and an unavailable store result. Assert that both resolve to false.
🤖 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 `@apps/api/test/ai-plane/ai-advisory-authority.service.e2e-spec.ts` around
lines 74 - 96, Extend the test for
AiAdvisoryAuthorityService.verifyPolicyReference to cover persisted-reference
failures: configure store.verifyPolicyReference to return false and separately
to reject or otherwise represent an unavailable result, then assert both service
calls resolve to false. Preserve the existing valid and locally invalid
reference assertions and update call-count expectations to account for the added
cases.
| it('uses one serializable proof write and no authoritative model writes', async () => { | ||
| const fixture = prismaFixture(); | ||
| const store = new PrismaSastAiAdvisoryAuthorityStore( | ||
| fixture.prisma as never | ||
| ); | ||
| const handoff = aiHandoff(); | ||
|
|
||
| const first = await store.createProof({ | ||
| tenantId: handoff.tenantId, | ||
| advisoryId: handoff.advisoryId, | ||
| verifiedAt: '2026-08-11T05:30:00.000Z' | ||
| }); | ||
| expect(first.replayed).toBe(false); | ||
| expect(first.proof.before.stateDigest).toBe( | ||
| first.proof.after.stateDigest | ||
| ); | ||
| expect(fixture.proof.create).toHaveBeenCalledTimes(1); | ||
| expect(fixture.finding.findMany).toHaveBeenCalledTimes(2); | ||
| expect(fixture.lifecycle.findMany).toHaveBeenCalledTimes(2); | ||
| expect(fixture.policy.findMany).toHaveBeenCalledTimes(2); | ||
| expect(fixture.waiver.findMany).toHaveBeenCalledTimes(2); | ||
| expect(fixture.suppression.findMany).toHaveBeenCalledTimes(2); | ||
| expect(storeSource).not.toMatch( | ||
| /\b(?:normalizedFinding|sastFindingLifecycleState|policyDecision|waiver|suppression)\.(?:create|createMany|update|updateMany|upsert|delete|deleteMany)\b/u | ||
| ); | ||
|
|
||
| await expect( | ||
| store.createProof({ | ||
| tenantId: handoff.tenantId, | ||
| advisoryId: handoff.advisoryId, | ||
| verifiedAt: '2026-08-11T06:00:00.000Z' | ||
| }) | ||
| ).resolves.toMatchObject({ replayed: true, proof: first.proof }); | ||
| expect(fixture.proof.create).toHaveBeenCalledTimes(1); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 5 'runSerializable|\$transaction|isolationLevel' \
apps/api/src/ai-plane/prisma-sast-ai-advisory-authority.store.ts \
apps/api/test/ai-plane/sast-ai-advisory-authority-persistence.e2e-spec.tsRepository: AigisAI/AegisAI_v2
Length of output: 5397
Assert the $transaction options.
The fixture ignores the second $transaction argument. Capture it and assert isolationLevel: Prisma.TransactionIsolationLevel.Serializable, maxWait, and timeout from runSerializable.
🤖 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 `@apps/api/test/ai-plane/sast-ai-advisory-authority-persistence.e2e-spec.ts`
around lines 77 - 111, Update the transaction fixture and the createProof test
to capture the second $transaction options argument. Assert that runSerializable
invokes $transaction with isolationLevel set to
Prisma.TransactionIsolationLevel.Serializable, plus the expected maxWait and
timeout values.
| it('verifies only a tenant and finding-bound exact policy reference', async () => { | ||
| const fixture = prismaFixture(); | ||
| const store = new PrismaSastAiAdvisoryAuthorityStore( | ||
| fixture.prisma as never | ||
| ); | ||
| const handoff = aiHandoff(); | ||
| const persisted = await store.createProof({ | ||
| tenantId: handoff.tenantId, | ||
| advisoryId: handoff.advisoryId, | ||
| verifiedAt: '2026-08-11T05:30:00.000Z' | ||
| }); | ||
| const reference = { | ||
| ...aiPolicyReference(), | ||
| authorityProofId: persisted.proof.proofId, | ||
| authorityProofDigest: persisted.proof.proofDigest | ||
| }; | ||
|
|
||
| await expect( | ||
| store.verifyPolicyReference({ | ||
| tenantId: handoff.tenantId, | ||
| normalizedFindingId: | ||
| handoff.normalizedFinding.normalizedFindingId, | ||
| reference | ||
| }) | ||
| ).resolves.toBe(true); | ||
| await expect( | ||
| store.verifyPolicyReference({ | ||
| tenantId: 'foreign-tenant', | ||
| normalizedFindingId: | ||
| handoff.normalizedFinding.normalizedFindingId, | ||
| reference | ||
| }) | ||
| ).resolves.toBe(false); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Test the finding and digest bindings.
The negative case changes only tenantId. A verifier that ignores normalizedFindingId, authorityProofDigest, or advisoryId can still pass this test.
Add rejection cases for a foreign finding and a tampered proof reference.
🤖 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 `@apps/api/test/ai-plane/sast-ai-advisory-authority-persistence.e2e-spec.ts`
around lines 113 - 146, Extend the test “verifies only a tenant and
finding-bound exact policy reference” to assert verifyPolicyReference returns
false for a foreign normalizedFindingId and for a tampered authority proof
reference, including changed authorityProofDigest or advisoryId. Keep the
existing valid and foreign-tenant assertions unchanged.
🎋 작업 중인 브랜치 및 이슈
feat/286-006-ai-output-authority🔎 주요 변경 사항
sast-ai-advisory-authority-proof-v1shared 계약으로 exact intent, advisory/handoff binding, authoritative before/after snapshot 및 zero-authority proof digest를 정의했습니다.advisoryOnly=true만 허용하고 enforcement/reason/block/ticket 결과는 deterministic finding/coverage 입력으로만 계산합니다.✅ 컨벤션 확인
type/issue-number-short-feature형식을 따르나요?<type>: <description>형식을 따르나요?Check List
검증
git diff --check통과006 진행 상태
Closes #286
Summary by CodeRabbit