Skip to content

feat: 006 normalized finding 및 AI advisory handoff 구현 - #285

Merged
goodtu02 merged 3 commits into
devfrom
feat/284-006-ai-advisory-handoff
Aug 11, 2026
Merged

feat: 006 normalized finding 및 AI advisory handoff 구현#285
goodtu02 merged 3 commits into
devfrom
feat/284-006-ai-advisory-handoff

Conversation

@goodtu02

@goodtu02 goodtu02 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

🎋 작업 중인 브랜치 및 이슈

🔎 주요 변경 사항

  • sast-ai-advisory-handoff-v1 shared 계약으로 caller intent, durable normalized finding projection, T042 reduced evidence reference, canonical digest 및 zero-authority audit shape를 정의했습니다.
  • 내부 advisory endpoint를 tenant/repository/evidence/model intent만 받는 exact-shape 경계로 제한하고 caller-provided finding, evidence, prompt, content 및 provider authority를 거부합니다.
  • SastEvidenceAccessService.classifyForAi 전후로 durable source와 access decision을 재결속해 tenant/repository/scan/attempt/occurrence/evidence/fingerprint drift, expiry 및 deletion race를 fail closed 처리합니다.
  • normalized finding은 durable redacted metadata의 bounded projection만 전달하고 raw scanner payload, repository source, code fragment, secret, retrieval 또는 tool 입력을 포함하지 않습니다.
  • AI runtime 요청과 응답을 T043 exact shape로 검증하고 retrieval, tools, policy, publication, lifecycle mutation 및 SCM write authority를 모두 false로 고정했습니다.
  • reference-only SastAiAdvisoryHandoff immutable ledger와 advisory result binding을 추가해 payload를 저장하지 않으며 exact retry는 동일 handoff/result를 재사용합니다.
  • Prisma schema/migration에 T042 access decision, T037 occurrence/source 및 normalized finding을 결속하는 composite FK, expiry/check, immutable update/delete 제약을 추가했습니다.
  • legacy direct AiAdvisoryRequest 경로를 제거하고 API/AI runtime/controller/module boundary를 T043 handoff만 허용하도록 변경했습니다.
  • cross-scope, caller injection, expiry, replay, persistence, malformed runtime output 및 zero-authority 회귀 테스트를 추가했습니다.
  • 006 contracts/data-model/research/threat-model/quality-gates/spec/plan/quickstart/tasks와 active-feature/ontology guard를 T043 완료 및 T044 다음 진입점으로 동기화했습니다.

✅ 컨벤션 확인

  • 브랜치명이 type/issue-number-short-feature 형식을 따르나요?
  • 이슈 제목과 PR 제목을 동일하게 작성했나요?
  • 커밋 메시지가 <type>: <description> 형식을 따르나요?

Check List

  • Assignees 등록을 하였나요?
  • 라벨(Label) 등록을 하였나요?
  • PR 머지 전 반드시 CI가 정상적으로 작동하는지 확인했나요?

검증

  • Prisma format, validate 및 client generate 통과
  • 전체 lint, typecheck 및 production build 통과
  • shared 104/104 tests 통과
  • API 101 suites / 653 tests 통과
  • AI runtime 16/16 tests 통과
  • T043 API focused 4 suites / 18 tests 통과
  • GitHub Actions guards 34/34 tests 통과
  • runtime guard 1/1 test 통과
  • git diff --check 통과
  • 자동 리뷰 inline thread 9/9 해결
  • CI run #336 전체 성공 (clean DB migration 포함)

006 진행 상태

  • T043 완료
  • 다음 작업: T044 authoritative AI output proof와 advisory-only 소비 경계 구현

Closes #284

Summary by CodeRabbit

  • New Features

    • Added secure AI advisory handoffs for SAST findings using validated, scoped, expiring evidence references.
    • Added deterministic advisory processing with safe replay behavior and durable advisory results.
    • Advisory responses now include request correlation and digest metadata when available.
  • Bug Fixes

    • Rejected legacy, malformed, unknown, tampered, expired, or authority-expanding requests.
    • Prevented sensitive source content, prompts, secrets, snippets, and payloads from reaching AI processing or persistence.
  • Documentation

    • Documented the new advisory handoff contract, security controls, quality gates, and completion status.

@goodtu02 goodtu02 self-assigned this Aug 11, 2026
@goodtu02 goodtu02 added the ✨ feat 새로운 기능 추가 label Aug 11, 2026 — with ChatGPT Codex Connector
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

T043 adds a validated SAST AI advisory handoff. Durable evidence access produces normalized findings and opaque reduced references. Immutable persistence stores digests and metadata only. The AI runtime rejects legacy, unknown, sensitive, or authority-bearing payloads.

Changes

Shared handoff contract

Layer / File(s) Summary
Contract, validation, and design synchronization
packages/shared/src/types/sast-ai-advisory-handoff.ts, packages/shared/src/index.ts, packages/shared/src/types/production-architecture.ts, packages/shared/test/*, specs/006-production-sast-runtime-design/*
Adds deterministic handoff construction, normalized finding and reduced-reference validation, fixed zero-authority metadata, canonical digests, and T043 design documentation.

Durable persistence

Layer / File(s) Summary
Immutable handoff ledger and Prisma store
apps/api/prisma/schema.prisma, apps/api/prisma/migrations/*, apps/api/src/ai-plane/sast-ai-advisory.store.ts, apps/api/src/ai-plane/prisma-sast-ai-advisory.store.ts, apps/api/test/ai-plane/sast-ai-advisory-persistence.e2e-spec.ts
Adds scoped handoff relations, restrictive foreign keys, uniqueness constraints, immutable triggers, serializable transactions, replay checks, and reference-only advisory persistence.

Advisory service flow

Layer / File(s) Summary
Intent validation and handoff orchestration
apps/api/src/ai-plane/ai-advisory.controller.ts, apps/api/src/ai-plane/ai-advisory.service.ts, apps/api/src/ai-plane/ai-plane.module.ts, apps/api/test/support/sast-ai-advisory-fixture.ts
The API accepts SastAiAdvisoryIntent. The service reclassifies evidence access, reloads normalized findings, builds and persists handoffs, invokes runtime inference, stores results, and replays existing advisories.

Runtime boundary

Layer / File(s) Summary
Handoff-derived inference requests and strict responses
apps/api/src/ai-plane/ai-advisory-runtime.client.ts, apps/ai/src/advisory-runtime.ts, apps/ai/src/model-gateway.ts, apps/ai/src/index.ts, apps/ai/test/t043-inference.fixture.ts
The runtime client derives metadata-only requests from validated handoffs. The AI runtime accepts only exact reduced-reference requests. Gateway validation checks scope, digests, capabilities, authority, evidence, and expiry.

Validation coverage

Layer / File(s) Summary
Boundary, fallback, audit, and completion tests
apps/api/test/ai-plane/ai-advisory-runtime.client.e2e-spec.ts, apps/api/test/ai-plane/ai-advisory.e2e-spec.ts, apps/api/test/ai-plane/ai-advisory.service.e2e-spec.ts, apps/ai/test/advisory-runtime.test.ts, apps/ai/test/model-gateway.test.ts, test/github-actions/*
Tests reject caller-supplied payloads, tampered or cross-scope data, sensitive fields, authority expansion, and malformed responses. Tests cover deterministic fallback, audit sequencing, replay behavior, and T043-to-T044 synchronization.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant AdvisoryController
  participant AiAdvisoryService
  participant SastEvidenceAccessService
  participant SastAiAdvisoryStore
  participant AiAdvisoryRuntimeClient
  participant AiRuntime

  Caller->>AdvisoryController: Submit SastAiAdvisoryIntent
  AdvisoryController->>AiAdvisoryService: createAdvisory(intent)
  AiAdvisoryService->>SastEvidenceAccessService: Classify durable evidence scope
  SastEvidenceAccessService-->>AiAdvisoryService: Return allowed access decision
  AiAdvisoryService->>SastAiAdvisoryStore: Persist validated handoff
  AiAdvisoryService->>AiAdvisoryRuntimeClient: createAdvisory(handoff)
  AiAdvisoryRuntimeClient->>AiRuntime: Send metadata and opaque reduced reference
  AiRuntime-->>AiAdvisoryRuntimeClient: Return validated inference response
  AiAdvisoryRuntimeClient-->>AiAdvisoryService: Return advisory result
  AiAdvisoryService->>SastAiAdvisoryStore: Persist advisory result
Loading

Possibly related PRs

  • AigisAI/AegisAI_v2#283: Provides the evidence-access service, access-decision model, and reduced-evidence contracts consumed by T043.
  • AigisAI/AegisAI_v2#271: Provides durable finding-lineage entities used to bind advisory handoffs.
  • AigisAI/AegisAI_v2#150: Introduced the earlier advisory runtime and validation flow replaced by the T043 contract.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes implement the linked issue objectives for T043 contracts, validation, secure handoffs, persistence, replay, tests, and documentation.
Out of Scope Changes check ✅ Passed The code, schema, tests, migrations, documentation, and feature guards are directly related to the T043 implementation objectives.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main changes: normalized findings and the AI advisory handoff implementation.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f8790da311

ℹ️ 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".

Comment thread apps/api/src/ai-plane/ai-advisory-runtime.client.ts Outdated
Comment thread apps/api/prisma/migrations/20260811040000_sast_ai_advisory_handoff/migration.sql Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🧹 Nitpick comments (12)
apps/ai/test/model-gateway.test.ts (1)

129-178: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Negative T043 boundary tests do not bind assertions to a specific rejection reason. Both sites accept any failure, so a change in the validation class stays undetected while the tests still pass.

  • apps/ai/test/model-gateway.test.ts#L129-L178: pair each invalid candidate with its expected AiInferenceValidationError reason code and assert that reason instead of the shared /reduced evidence|T043|tenant attribution/i regex.
  • apps/ai/test/advisory-runtime.test.ts#L63-L93: read the response body in the loop and assert the error message, in addition to the 400 status.
🤖 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/ai/test/model-gateway.test.ts` around lines 129 - 178, Update the
invalid T043 cases in apps/ai/test/model-gateway.test.ts#L129-L178 to pair each
candidate with its expected AiInferenceValidationError reason code and assert
that specific reason instead of the shared regex. In
apps/ai/test/advisory-runtime.test.ts#L63-L93, read each response body in the
loop and assert its error message alongside the existing 400 status check.
test/github-actions/ontology.test.mjs (1)

85-85: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Bound the wildcard gaps in this plan assertion.

The pattern uses unbounded [\s\S]* between each token. The assertion passes when T043 and T044 appear in unrelated sections of the document. test/github-actions/active-feature.test.mjs Line 1714 uses bounded gaps for the equivalent check. Use bounded gaps here for the same strictness.

♻️ Proposed change
-    /did not advance or satisfy T040[\s\S]*completed[\s\S]*T043[\s\S]*proceeds to T044/
+    /did not advance or satisfy T040[\s\S]{0,200}completed[\s\S]{0,200}T043[\s\S]{0,200}proceeds to T044/
🤖 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 `@test/github-actions/ontology.test.mjs` at line 85, Update the ontology plan
assertion around the pattern containing “did not advance or satisfy T040” so
each wildcard gap is bounded, matching the bounded-gap structure used by the
equivalent active-feature assertion. Keep the required token order and
completion/progression checks while preventing matches that span unrelated
document sections.
apps/ai/src/advisory-runtime.ts (2)

39-47: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Construct the model gateway once at module scope.

createModelGateway runs on every request and re-executes validateModelGatewayConfig with a constant configuration. The gateway holds no per-request state.

♻️ Proposed hoist
+const gateway = createModelGateway({
+  config: {
+    providerId: 'deterministic',
+    model: 'detector-planner-fallback',
+    version: 'v1',
+    allowFallback: true
+  },
+  fallbackProvider: createDeterministicFallbackProvider()
+});
+
 export async function handleAiAdvisoryRequest(
   request: Request
 ): Promise<Response> {
-    const gateway = createModelGateway({
-      config: {
-        providerId: 'deterministic',
-        model: 'detector-planner-fallback',
-        version: 'v1',
-        allowFallback: true
-      },
-      fallbackProvider: createDeterministicFallbackProvider()
-    });
     return jsonResponse(await gateway.infer(body), 200);
🤖 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/ai/src/advisory-runtime.ts` around lines 39 - 47, Move the
createModelGateway invocation out of the request-handling path and initialize
the gateway once at module scope. Preserve the existing deterministic
configuration and createDeterministicFallbackProvider setup, then reuse the
module-level gateway for every request.

49-59: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Do not echo arbitrary error messages from the AI runtime boundary.

The catch block returns error.message for every failure. Validation errors from validateAiInferenceRequest are safe. Unexpected internal errors are not. A JSON parse failure or a Node system error returns internal detail to the caller.

Return a stable code for known validation errors and a fixed message otherwise.

🛡️ Proposed fix
   } catch (error) {
+    const reasonCode =
+      error instanceof AiInferenceValidationError
+        ? error.reasonCode
+        : 'MALFORMED_REQUEST';
     return jsonResponse(
       {
-        error:
-          error instanceof Error
-            ? error.message
-            : 'AI advisory request is malformed.'
+        error: 'AI advisory request was rejected.',
+        reasonCode
       },
       400
     );
   }

Export AiInferenceValidationError from ./model-gateway if it is not already exported.

🤖 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/ai/src/advisory-runtime.ts` around lines 49 - 59, Update the catch block
in the advisory runtime handler to return a stable validation error code only
when the caught error is an AiInferenceValidationError, importing or exporting
that class from ./model-gateway as needed. For all other failures, return a
fixed generic message instead of exposing error.message, while preserving the
existing 400 response status.
apps/api/src/ai-plane/ai-advisory.service.ts (2)

305-326: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Reorder the null check in sameAccess for clarity.

Line 324 checks right.reducedEvidenceReference !== null after five optional-chained comparisons. If both sides are null, each comparison yields undefined === undefined, which is true, and only the final check rejects the pair. The logic is correct, but the guard is easy to break during a later edit.

♻️ Proposed reorder
 ): boolean {
+  if (
+    left.reducedEvidenceReference === null ||
+    right.reducedEvidenceReference === null
+  ) {
+    return false;
+  }
   return (
     left.decision.accessDecisionId ===
       right.decision.accessDecisionId &&
     left.decision.decisionDigest ===
       right.decision.decisionDigest &&
-    left.reducedEvidenceReference?.reducedEvidenceRef ===
-      right.reducedEvidenceReference?.reducedEvidenceRef &&
-    left.reducedEvidenceReference?.redactedProjectionDigest ===
-      right.reducedEvidenceReference?.redactedProjectionDigest &&
-    left.reducedEvidenceReference?.payloadExpiresAt ===
-      right.reducedEvidenceReference?.payloadExpiresAt &&
-    right.reducedEvidenceReference !== null
+    left.reducedEvidenceReference.reducedEvidenceRef ===
+      right.reducedEvidenceReference.reducedEvidenceRef &&
+    left.reducedEvidenceReference.redactedProjectionDigest ===
+      right.reducedEvidenceReference.redactedProjectionDigest &&
+    left.reducedEvidenceReference.payloadExpiresAt ===
+      right.reducedEvidenceReference.payloadExpiresAt
   );
 }
🤖 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.service.ts` around lines 305 - 326, Reorder
the conditions in sameAccess so right.reducedEvidenceReference !== null is
checked before the optional-chained reduced-evidence comparisons, while
preserving all existing equality checks and return behavior.

158-160: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Add logging before mapping infrastructure failures to unavailable().

Four catch blocks discard the original error and return NotFoundException. A database outage, a Prisma REPLAY_CONFLICT, and a legitimate denial all produce the same 404 response with no trace. Production diagnosis of the handoff flow becomes impossible.

Keep the bounded external response. Record the cause internally.

♻️ Proposed logging addition
 `@Injectable`()
 export class AiAdvisoryService {
+  private readonly logger = new Logger(AiAdvisoryService.name);
+
   constructor(
   private async persistHandoffAndRead(
     handoff: Readonly<SastAiAdvisoryHandoff>
   ) {
     try {
       const persisted = await this.store.persistHandoff(handoff);
       const existing = await this.store.loadAdvisory({
         tenantId: handoff.tenantId,
         advisoryId: handoff.advisoryId
       });
       return { persisted, existing };
-    } catch {
+    } catch (error) {
+      this.logger.error(
+        `Handoff persistence failed for ${handoff.handoffId}.`,
+        error instanceof Error ? error.stack : undefined
+      );
       throw unavailable();
     }
   }

Do not log tenant identifiers, fingerprints, or evidence references in message bodies beyond the opaque handoff identifier.

Also applies to: 187-192, 199-204, 226-236

🤖 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.service.ts` around lines 158 - 160, Update
the catch blocks in the handoff flow, including the blocks near unavailable(),
to log the caught error internally before mapping it to the bounded
unavailable/NotFound response. Preserve the existing external response behavior,
use the opaque handoff identifier at most in log messages, and exclude tenant
identifiers, fingerprints, and evidence references.
packages/shared/src/types/sast-ai-advisory-handoff.ts (1)

473-511: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicate reduced-evidence validator creates both drift and re-export risk. The handoff module re-implements and exports isSastReducedEvidenceReferenceShapeValid, a validator for a contract owned by ./types/sast-evidence-access. That single duplication causes both a second source of truth for T042 rules and a possible ambiguous barrel re-export.

  • packages/shared/src/types/sast-ai-advisory-handoff.ts#L473-L511: import the T042 validator and SAST_REDUCED_EVIDENCE_REFERENCE_VERSION instead of re-implementing the checks and hardcoding the version string.
  • packages/shared/src/index.ts#L27-L27: confirm that no exported name is now emitted by two export * statements; TS2308 would break the shared package build.
🤖 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/src/types/sast-ai-advisory-handoff.ts` around lines 473 -
511, The handoff module duplicates the T042 reduced-evidence validator and
version constant. In packages/shared/src/types/sast-ai-advisory-handoff.ts lines
473-511, import and reuse the validator and
SAST_REDUCED_EVIDENCE_REFERENCE_VERSION from ./types/sast-evidence-access,
removing the local implementation and hardcoded version; in
packages/shared/src/index.ts line 27, confirm barrel exports do not emit the
validator name through two export-star statements and adjust exports if needed
to avoid TS2308.
apps/api/prisma/migrations/20260811040000_sast_ai_advisory_handoff/migration.sql (1)

140-141: 🚀 Performance & Scalability | 🔵 Trivial

Consider splitting the lock-taking statements for large AiAdvisoryMetadata tables.

The unique index build and the foreign key validation each block writes to AiAdvisoryMetadata for the duration of a full table scan. If that table is large in production, add the constraint with NOT VALID first and run VALIDATE CONSTRAINT in a later migration, and build the index concurrently.

Note the constraint: CREATE INDEX CONCURRENTLY and VALIDATE CONSTRAINT cannot run inside the single transaction that Prisma wraps around one migration file. Both need their own migration step. If the table is small, keep the current form.

Also applies to: 217-220

🤖 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/20260811040000_sast_ai_advisory_handoff/migration.sql`
around lines 140 - 141, For large AiAdvisoryMetadata tables, split the
AiAdvisoryMetadata_sastHandoffId unique index and foreign-key validation into
separate migration steps: create the index concurrently, add the foreign key as
NOT VALID, then validate it in a later migration outside Prisma’s transaction.
Preserve the current migration for deployments where the table is small.

Source: Linters/SAST tools

packages/shared/test/sast-evidence-access.test.mjs (1)

243-286: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider adding negative cases for digest and expiry drift.

The test covers authority widening and caller-supplied fields. It does not cover two other fail-closed paths that isSastAiAdvisoryHandoffShapeValid enforces: a tampered requestDigest/handoffDigest and a payloadExpiresAt window longer than 24 hours. Add two assertions to lock those branches.

💚 Proposed additional assertions
   assert.equal(
     isSastAiAdvisoryHandoffShapeValid({
       ...first,
       authority: { ...first.authority, toolsAllowed: true }
     }, digest),
     false
   );
+  assert.equal(
+    isSastAiAdvisoryHandoffShapeValid({
+      ...first,
+      handoffDigest: `sha256:${'f'.repeat(64)}`
+    }, digest),
+    false
+  );
+  assert.equal(
+    isSastAiAdvisoryHandoffShapeValid({
+      ...first,
+      payloadExpiresAt: '2026-08-12T05:00:01.000Z'
+    }, digest),
+    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 `@packages/shared/test/sast-evidence-access.test.mjs` around lines 243 - 286,
Extend the T043 test around isSastAiAdvisoryHandoffShapeValid with negative
assertions for a tampered requestDigest or handoffDigest and for
payloadExpiresAt exceeding the 24-hour window. Build each case from first while
changing only the targeted field, pass digest as required, and assert validation
returns false.
apps/api/test/ai-plane/sast-ai-advisory-persistence.e2e-spec.ts (1)

55-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Consider replacing source-text assertions with behavioral assertions.

These tests read .ts and .sql files and assert on substrings. Three weaknesses follow. A matching substring inside a comment satisfies the assertion. A formatting change such as a line break inside an object literal breaks a passing test. Line 93 counts this.classify(scope, clock) occurrences by regex, so a rename or an extracted helper fails the test without a behavior change.

The mock-based test at lines 117-174 already proves the persistence behavior directly. Extend that approach for the runtime and service invariants where practical, and keep source-text checks only for the migration SQL, where no runtime seam exists.

🤖 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-persistence.e2e-spec.ts` around lines
55 - 94, Replace source-text assertions in the persistence and service tests
with behavioral or mock-based assertions, reusing the existing mock setup from
the test around lines 117-174 to verify runtime persistence and classification
invariants. Remove brittle store/service substring checks and the
occurrence-count assertion for this.classify(scope, clock), while retaining
source-text assertions only for migration SQL constraints that lack a runtime
seam.
apps/api/src/ai-plane/prisma-sast-ai-advisory.store.ts (1)

40-42: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Add the tenant predicate to the access-decision lookup.

findUnique selects by id only. The tenant check happens later in isStoredDecisionBound, so the current code fails closed. The repository threat model requires a tenant predicate in the query itself for cross-tenant object access (specs/006-production-sast-runtime-design/threat-model.md, "Cross-tenant object access" row). Use the SastEvidenceAccessDecision_tenant_scope_key composite unique that this PR adds at apps/api/prisma/schema.prisma line 1587.

🔒️ Proposed defense-in-depth fix
-    const row = await this.prisma.sastEvidenceAccessDecision.findUnique({
-      where: { id: decision.accessDecisionId },
+    const row = await this.prisma.sastEvidenceAccessDecision.findUnique({
+      where: {
+        id_tenantId: {
+          id: decision.accessDecisionId,
+          tenantId: decision.scope.tenantId
+        }
+      },

Confirm the generated compound-unique input name matches the SastEvidenceAccessDecision_tenant_scope_key mapping before you apply the change.

🤖 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.store.ts` around lines 40 - 42,
Update the access-decision lookup in the surrounding method to query by the
generated compound unique input mapped from
SastEvidenceAccessDecision_tenant_scope_key, supplying both
decision.accessDecisionId and the request tenant identifier. Confirm the
generated Prisma input name and field names before applying the change, while
preserving the existing select and subsequent validation.
apps/api/prisma/schema.prisma (1)

1712-1717: 🔒 Security & Privacy | 🔵 Trivial

Define the handoff purge policy before enabling tenant deletion.

Normal offboarding is soft revocation, but no handoff purge path is defined. SastAiAdvisoryHandoff remains protected by RESTRICT foreign keys and an immutable-delete trigger. Add it to an authorized hard-purge procedure, or document tenant-tombstone retention for these digest-only rows.

🤖 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 1712 - 1717, Define the
tenant-deletion policy for SastAiAdvisoryHandoff before enabling tenant
deletion: either include this model in an authorized hard-purge procedure that
handles its RESTRICT foreign keys and immutable-delete trigger, or explicitly
document retention of its digest-only rows in the tenant-tombstone flow.
🤖 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/ai/src/model-gateway.ts`:
- Around line 140-151: Split the combined validation in the request-validation
flow around isRecord, hasExactKeys, and tenantId checks into distinct branches
with accurate AiInferenceValidationError reason codes. Keep
MISSING_TENANT_ATTRIBUTION only for missing, non-string, or blank tenantId
values, and assign a separate exact-shape reason code for non-record requests or
unexpected/missing top-level keys so createModelGateway audits the actual
rejection.

In `@apps/api/src/ai-plane/ai-advisory-runtime.client.ts`:
- Around line 149-161: Add explicit MAX_SIGNALS and MAX_TEXT_LENGTH checks
inside isDetectorAdvisory and isPlannerAdvisory, limiting signals array length
and rationale/action string lengths before parseRuntimeOutput accepts
advisories. Bound hasForbiddenRuntimeResponseKey recursion with a depth limit so
deeply nested runtime payloads are rejected or safely handled, while preserving
existing validation behavior.
- Around line 41-46: Compute the validated finite timeout once in the advisory
runtime request flow, using the existing 2500 fallback for invalid values, and
reuse that variable in both the axios.post options and the later timeout-related
logic near AI_ADVISORY_TIMEOUT_MS. Remove the duplicate raw-value conversion
while preserving the current fallback behavior.
- Around line 97-98: Sort and deduplicate both identifier arrays before joining
them in the advisory runtime client’s CWE/CVE payload construction. Preserve the
strictly ascending lexicographic ordering rule in isCommaSeparatedIdentifiers
and document it in the T043 contract; extend the T043 fixture with at least two
scanner-supplied CWE identifiers in unsorted order to cover the regression.
Apply changes in apps/api/src/ai-plane/ai-advisory-runtime.client.ts lines
97-98, apps/ai/src/model-gateway.ts lines 343-355, and
apps/ai/test/t043-inference.fixture.ts lines 45-46.

In `@apps/api/src/ai-plane/prisma-sast-ai-advisory.store.ts`:
- Around line 519-521: Update replayHandoff comparisons in
prisma-sast-ai-advisory.store.ts to compare payloadExpiresAt and createdAt as
instants using getTime() and Date.parse(), rather than comparing toISOString()
strings. In apps/api/prisma/schema.prisma lines 1671-1725, confirm
SastAiAdvisoryHandoff.createdAt and SastEvidenceAccessDecision.decidedAt use
identical timestamp precision so SastAiAdvisoryHandoff_access_scope_fkey
preserves exact joins; apply any required schema or migration precision change.

In `@packages/shared/src/types/sast-ai-advisory-handoff.ts`:
- Line 342: Update the title validation in the SAST AI advisory handoff schema
to reject all ASCII control characters, matching the rule used by
isBoundedReference rather than relying only on isBoundedText. Apply the same
validation to the additional title path identified around the other occurrence
so every scanner-supplied title is covered.
- Around line 675-687: Update stableJson’s object-key serialization to omit
entries whose values are undefined before sorting and joining keys. Preserve
existing recursive canonicalization for defined values, arrays, primitives, and
null so an undefined-valued optional property canonicalizes the same as an
absent property and the output remains valid JSON.

---

Nitpick comments:
In `@apps/ai/src/advisory-runtime.ts`:
- Around line 39-47: Move the createModelGateway invocation out of the
request-handling path and initialize the gateway once at module scope. Preserve
the existing deterministic configuration and createDeterministicFallbackProvider
setup, then reuse the module-level gateway for every request.
- Around line 49-59: Update the catch block in the advisory runtime handler to
return a stable validation error code only when the caught error is an
AiInferenceValidationError, importing or exporting that class from
./model-gateway as needed. For all other failures, return a fixed generic
message instead of exposing error.message, while preserving the existing 400
response status.

In `@apps/ai/test/model-gateway.test.ts`:
- Around line 129-178: Update the invalid T043 cases in
apps/ai/test/model-gateway.test.ts#L129-L178 to pair each candidate with its
expected AiInferenceValidationError reason code and assert that specific reason
instead of the shared regex. In apps/ai/test/advisory-runtime.test.ts#L63-L93,
read each response body in the loop and assert its error message alongside the
existing 400 status check.

In
`@apps/api/prisma/migrations/20260811040000_sast_ai_advisory_handoff/migration.sql`:
- Around line 140-141: For large AiAdvisoryMetadata tables, split the
AiAdvisoryMetadata_sastHandoffId unique index and foreign-key validation into
separate migration steps: create the index concurrently, add the foreign key as
NOT VALID, then validate it in a later migration outside Prisma’s transaction.
Preserve the current migration for deployments where the table is small.

In `@apps/api/prisma/schema.prisma`:
- Around line 1712-1717: Define the tenant-deletion policy for
SastAiAdvisoryHandoff before enabling tenant deletion: either include this model
in an authorized hard-purge procedure that handles its RESTRICT foreign keys and
immutable-delete trigger, or explicitly document retention of its digest-only
rows in the tenant-tombstone flow.

In `@apps/api/src/ai-plane/ai-advisory.service.ts`:
- Around line 305-326: Reorder the conditions in sameAccess so
right.reducedEvidenceReference !== null is checked before the optional-chained
reduced-evidence comparisons, while preserving all existing equality checks and
return behavior.
- Around line 158-160: Update the catch blocks in the handoff flow, including
the blocks near unavailable(), to log the caught error internally before mapping
it to the bounded unavailable/NotFound response. Preserve the existing external
response behavior, use the opaque handoff identifier at most in log messages,
and exclude tenant identifiers, fingerprints, and evidence references.

In `@apps/api/src/ai-plane/prisma-sast-ai-advisory.store.ts`:
- Around line 40-42: Update the access-decision lookup in the surrounding method
to query by the generated compound unique input mapped from
SastEvidenceAccessDecision_tenant_scope_key, supplying both
decision.accessDecisionId and the request tenant identifier. Confirm the
generated Prisma input name and field names before applying the change, while
preserving the existing select and subsequent validation.

In `@apps/api/test/ai-plane/sast-ai-advisory-persistence.e2e-spec.ts`:
- Around line 55-94: Replace source-text assertions in the persistence and
service tests with behavioral or mock-based assertions, reusing the existing
mock setup from the test around lines 117-174 to verify runtime persistence and
classification invariants. Remove brittle store/service substring checks and the
occurrence-count assertion for this.classify(scope, clock), while retaining
source-text assertions only for migration SQL constraints that lack a runtime
seam.

In `@packages/shared/src/types/sast-ai-advisory-handoff.ts`:
- Around line 473-511: The handoff module duplicates the T042 reduced-evidence
validator and version constant. In
packages/shared/src/types/sast-ai-advisory-handoff.ts lines 473-511, import and
reuse the validator and SAST_REDUCED_EVIDENCE_REFERENCE_VERSION from
./types/sast-evidence-access, removing the local implementation and hardcoded
version; in packages/shared/src/index.ts line 27, confirm barrel exports do not
emit the validator name through two export-star statements and adjust exports if
needed to avoid TS2308.

In `@packages/shared/test/sast-evidence-access.test.mjs`:
- Around line 243-286: Extend the T043 test around
isSastAiAdvisoryHandoffShapeValid with negative assertions for a tampered
requestDigest or handoffDigest and for payloadExpiresAt exceeding the 24-hour
window. Build each case from first while changing only the targeted field, pass
digest as required, and assert validation returns false.

In `@test/github-actions/ontology.test.mjs`:
- Line 85: Update the ontology plan assertion around the pattern containing “did
not advance or satisfy T040” so each wildcard gap is bounded, matching the
bounded-gap structure used by the equivalent active-feature assertion. Keep the
required token order and completion/progression checks while preventing matches
that span unrelated document sections.
🪄 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: 10a32eaa-b9be-4196-8d8d-c0adee6ea711

📥 Commits

Reviewing files that changed from the base of the PR and between dcbca91 and f8790da.

📒 Files selected for processing (34)
  • apps/ai/src/advisory-runtime.ts
  • apps/ai/src/index.ts
  • apps/ai/src/model-gateway.ts
  • apps/ai/test/advisory-runtime.test.ts
  • apps/ai/test/model-gateway.test.ts
  • apps/ai/test/t043-inference.fixture.ts
  • apps/api/prisma/migrations/20260811040000_sast_ai_advisory_handoff/migration.sql
  • apps/api/prisma/schema.prisma
  • apps/api/src/ai-plane/ai-advisory-runtime.client.ts
  • apps/api/src/ai-plane/ai-advisory.controller.ts
  • apps/api/src/ai-plane/ai-advisory.service.ts
  • apps/api/src/ai-plane/ai-plane.module.ts
  • apps/api/src/ai-plane/prisma-sast-ai-advisory.store.ts
  • apps/api/src/ai-plane/sast-ai-advisory.store.ts
  • apps/api/test/ai-plane/ai-advisory-runtime.client.e2e-spec.ts
  • apps/api/test/ai-plane/ai-advisory.e2e-spec.ts
  • apps/api/test/ai-plane/ai-advisory.service.e2e-spec.ts
  • apps/api/test/ai-plane/sast-ai-advisory-persistence.e2e-spec.ts
  • apps/api/test/support/sast-ai-advisory-fixture.ts
  • packages/shared/src/index.ts
  • packages/shared/src/types/production-architecture.ts
  • packages/shared/src/types/sast-ai-advisory-handoff.ts
  • packages/shared/test/sast-evidence-access.test.mjs
  • specs/006-production-sast-runtime-design/contracts/sast-runtime.md
  • specs/006-production-sast-runtime-design/data-model.md
  • specs/006-production-sast-runtime-design/plan.md
  • specs/006-production-sast-runtime-design/quality-gates.md
  • specs/006-production-sast-runtime-design/quickstart.md
  • specs/006-production-sast-runtime-design/research.md
  • specs/006-production-sast-runtime-design/spec.md
  • specs/006-production-sast-runtime-design/tasks.md
  • specs/006-production-sast-runtime-design/threat-model.md
  • test/github-actions/active-feature.test.mjs
  • test/github-actions/ontology.test.mjs

Comment thread apps/ai/src/model-gateway.ts Outdated
Comment thread apps/api/src/ai-plane/ai-advisory-runtime.client.ts
Comment thread apps/api/src/ai-plane/ai-advisory-runtime.client.ts
Comment thread apps/api/src/ai-plane/ai-advisory-runtime.client.ts
Comment thread apps/api/src/ai-plane/prisma-sast-ai-advisory.store.ts
Comment thread packages/shared/src/types/sast-ai-advisory-handoff.ts
Comment thread packages/shared/src/types/sast-ai-advisory-handoff.ts

Copy link
Copy Markdown
Contributor Author

자동 리뷰 후속 반영을 d1980d5에 게시했습니다.

  • exact-shape/tenant rejection reason을 분리하고 외부 오류 및 fallback reason을 안정된 코드로 고정했습니다.
  • timeout을 한 번만 검증·재사용하고 runtime response의 배열, 텍스트, latency, depth/breadth를 제한했습니다.
  • title ASCII control 및 canonical JSON의 undefined key를 거부/생략하고 digest·expiry·JSON round-trip 음수 테스트를 보강했습니다.
  • access decision을 tenant compound unique로 조회하고 timestamp를 instant로 비교하며, 내부 오류 로그는 coarse category와 opaque handoff ID만 남깁니다.
  • populated AiAdvisoryMetadata unique index/FK를 mandatory online-schema의 concurrent index → NOT VALID → validate 흐름으로 이동했습니다.
  • T042 reduced-reference validator를 단일 구현으로 재사용하고, tenant tombstone/authorized hard-purge 정책과 hostile runtime output 한계를 문서화했습니다.

검토 후 적용하지 않은 제안:

  • CWE/CVE 재정렬/중복 제거: shared handoff가 이미 strict ascending + duplicate-free를 강제하므로 클라이언트의 자동 보정은 immutable contract drift를 숨깁니다. 두 개 이상의 canonical ID 전송 테스트와 계약 설명으로 보강했습니다.
  • gateway module-scope hoist: handoff별 modelVersion이 gateway config를 선택하므로 단일 정적 gateway는 model binding을 깨뜨립니다. 임의 model-version cache도 무제한 상태 증가 위험이 있어 요청 범위 구성을 유지했습니다.
  • persistence source-text guard 전면 제거: migration/architecture seam이 없는 정적 불변조건은 executable guard로 유지했고, 실제 replay/persistence/classification은 별도 mock 기반 행동 테스트로 검증됩니다.

검증: lint, typecheck, build 성공; shared 104/104, AI 16/16, API 101 suites/653, GitHub guards 34/34, runtime guard 1/1, git diff --check 성공.

@goodtu02
goodtu02 merged commit 0bbf04e into dev Aug 11, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

✨ feat 새로운 기능 추가

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: 006 normalized finding 및 AI advisory handoff 구현

1 participant