feat(integration): bind plugin operator authority to exact request - #191
Conversation
|
Warning Review limit reached
Next review available in: 14 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough서명된 플러그인 운영자 요청의 경로, 메서드, UUID, 발급 시각, 공유 비밀값 및 HMAC 서명을 검증하는 모듈을 추가했다. Replay evidence를 PostgreSQL에 원자적으로 소비하는 guard와 설치·자격 증명 작업을 위임하는 애플리케이션 계층을 추가했다. Changes플러그인 운영자 권한
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant PluginOperatorApplication
participant requireVerifiedPluginOperatorContext
participant PluginOperatorReplayGuard
participant HostPort
Operator->>PluginOperatorApplication: 서명된 운영자 요청 제출
PluginOperatorApplication->>requireVerifiedPluginOperatorContext: 헤더와 요청 바인딩 검증
requireVerifiedPluginOperatorContext-->>PluginOperatorApplication: trustedContext와 evidence 반환
PluginOperatorApplication->>PluginOperatorReplayGuard: evidence 소비
PluginOperatorReplayGuard-->>PluginOperatorApplication: 소비 결과 반환
PluginOperatorApplication->>HostPort: trustedContext와 작업 데이터 전달
HostPort-->>PluginOperatorApplication: 작업 결과 반환
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/integration-service/src/plugin-operator-context.test.ts`:
- Around line 51-69: Update the rejection assertions in the cross-user,
cross-workspace, method, and path replay cases within
requireTrustedPluginOperatorContext tests, plus the timestamp and signature
failure cases around the referenced range, to assert the thrown
IntegrationOperatorContextError has kind: 'invalid'. Preserve the existing
error-type assertions while verifying each invalid evidence scenario is not
classified as unavailable.
In `@apps/integration-service/src/plugin-operator-context.ts`:
- Around line 19-24: In apps/integration-service/src/plugin-operator-context.ts
lines 19-24, add a signed unique UUIDv4 evidence identifier to
IntegrationOperatorContextHeaders; in lines 115-163, after HMAC validation
atomically consume that identifier through a service-owned TTL store and return
invalid for identifiers already consumed. In
apps/integration-service/src/plugin-operator-context.test.ts lines 39-49, add
coverage submitting the same valid context twice and asserting the second call
fails.
- Around line 4-16: 문서화되지 않은 내부 선언에 계약 설명이 부족합니다. UUID·시간·해시·라우트 패턴, 제한값과 컬렉션 경로
상수 및 invalid, unavailable, requireUuidV4, requireOperatorRoute를 대상으로 JSDoc을 추가해
입력 형식, 정규화 규칙, 허용되는 요청 표면, 검증 성공 조건과 각 실패 시 오류 분류를 명시하십시오.
- Around line 82-104: Bind signatures to the exact received route by removing
the case-insensitive flag from the UUID route pattern and returning the
validated path without applying toLowerCase in the relevant route handling in
apps/integration-service/src/plugin-operator-context.ts lines 82-104. Add
coverage in apps/integration-service/src/plugin-operator-context.test.ts lines
71-112 verifying that paths containing uppercase UUID characters are rejected as
invalid.
🪄 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: dc945377-04a5-4177-a0ef-db5f89ccf7c5
📒 Files selected for processing (2)
apps/integration-service/src/plugin-operator-context.test.tsapps/integration-service/src/plugin-operator-context.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/integration-service/src/plugin-operator-application.test.ts (1)
258-282: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLine 274 어서션은 프로덕션 동작을 증명하지 않습니다.
credentials.bind는 고정 픽스처CREDENTIAL_VIEW를 반환합니다. 따라서not.toHaveProperty('secretReference')는 픽스처의 형태만 확인합니다. 애플리케이션이 비밀 참조를 제거한다는 사실은 증명하지 않습니다.목이
secretReference같은 필드를 포함한 값을 반환하도록 만든 뒤, 경계가 이를 노출하지 않는지 검증하십시오. 경계가 응답을 변형하지 않는 설계라면 이 어서션을 제거하고 자격 증명 뷰 계약 테스트로 옮기십시오.가이드라인에 따라 "Tests must prove realistic domain accuracy and failure behavior, not only mocked call counts."
🤖 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/integration-service/src/plugin-operator-application.test.ts` around lines 258 - 282, Update the test around application.bindCredential so credentials.bind returns a credential view containing secretReference, then assert the application result omits that field while preserving the trustedContext verification. If the application intentionally does not sanitize the response, remove this assertion and move the secretReference contract check to the credential-view boundary test instead.Source: Coding guidelines
🤖 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/integration-service/src/plugin-operator-application.ts`:
- Around line 148-160: The requireContext method must enforce one-time use of
each signed (method, path, signature) combination. Record consumed signatures
with their expiration time, reject reuse with a kind: 'invalid' error, and
remove expired entries to prevent unbounded memory growth. In
apps/integration-service/src/plugin-operator-application.test.ts lines 165-183,
retain the replay-prevention test and add coverage confirming new evidence
succeeds after the validity window expires.
---
Nitpick comments:
In `@apps/integration-service/src/plugin-operator-application.test.ts`:
- Around line 258-282: Update the test around application.bindCredential so
credentials.bind returns a credential view containing secretReference, then
assert the application result omits that field while preserving the
trustedContext verification. If the application intentionally does not sanitize
the response, remove this assertion and move the secretReference contract check
to the credential-view boundary test instead.
🪄 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: 9a12155d-c46c-4fe2-82f4-59c22d95254e
📒 Files selected for processing (4)
apps/integration-service/src/plugin-operator-application.test.tsapps/integration-service/src/plugin-operator-application.tsapps/integration-service/src/plugin-operator-context.test.tsapps/integration-service/src/plugin-operator-context.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/integration-service/src/plugin-operator-context.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
apps/integration-service/src/plugin-operator-replay-migration.test.ts (1)
150-160: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win동일 시각 경계 케이스를 추가하십시오.
CHECK (expires_at >= consumed_at)는 두 값이 같은 경우를 허용합니다. 현재 테스트는 만료가 더 이른 경우만 거부를 검증합니다.expires_at = consumed_at인 삽입이 성공하는지 확인하는 테스트를 추가하십시오. 이렇게 하면 제약의 경계 동작이 고정됩니다.🤖 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/integration-service/src/plugin-operator-replay-migration.test.ts` around lines 150 - 160, In the plugin operator replay migration tests, add a case alongside the existing expiration-order test that inserts identical consumed_at and expires_at timestamps and verifies the insertion succeeds. Preserve the current rejection test for expires_at earlier than consumed_at, covering both sides of the CHECK constraint boundary.apps/integration-service/src/plugin-operator-application.ts (1)
185-190: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winreplay 저장소 실패를 관측 가능하게 만드십시오.
catch가 원인 오류를 완전히 버립니다. 그 결과 데이터베이스 장애와 설정 누락을 운영 중에 구분할 수 없습니다. 오류 원인을 노출하지 않는 범위에서 구조화된 로그나 메트릭 훅을 추가하십시오. 스택 트레이스와 자격 증명은 보존 아티팩트에 남기지 마십시오.cause옵션으로 원인을 전달하고 경계에서 정제하는 방법도 가능합니다.🤖 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/integration-service/src/plugin-operator-application.ts` around lines 185 - 190, Update the replayGuard.consume error handling in the surrounding method to make failures observable without exposing credentials or retaining raw stack traces: add a structured log or metric with sanitized error classification/details, and propagate the original failure via the IntegrationOperatorContextError cause option so the boundary can redact it. Preserve the existing 'unavailable' context error behavior.Source: Coding guidelines
apps/integration-service/src/plugin-operator-application.test.ts (1)
169-196: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win증거 만료 시각이 발급 시각에서 유도되는지 검증하십시오.
현재 테스트는
issuedAt과 현재 시각이 같습니다. 따라서expiresAt이issuedAt + 60에서 나오는지, 아니면now + 60에서 나오는지 구분되지 않습니다. 시간창 안에 있지만 더 이른issuedAt을 사용하는 케이스를 추가하십시오. 그 케이스에서consumedAt은 현재 시각이고expiresAt은 발급 시각 기준이어야 합니다. 이렇게 하면 재생 방지 창의 실제 도메인 동작이 고정됩니다.✅ 제안 추가 테스트
+ it('derives evidence expiry from the signed issuance time, not the consumption time', async () => { + const installations = installationPort(); + const replay = replayGuard(); + const app = application(installations, credentialPort(), replay); + const issuedAt = String(NOW_SECONDS - 30); + const headers = signedHeaders( + 'POST', + '/v1/plugins/installations', + issuedAt, + ); + + await app.install(headers, { + installationId: INSTALLATION_ID, + manifest: MANIFEST, + grantedCapabilities: ['lifeos.calendar.event.v1'], + }); + + expect(replay.consume).toHaveBeenCalledWith({ + evidenceId: headers.evidenceId, + consumedAt: NOW, + expiresAt: new Date((Number(issuedAt) + 60) * 1_000).toISOString(), + }); + });As per coding guidelines, "Tests must model realistic domain outcomes, not only mocked implementation calls."
🤖 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/integration-service/src/plugin-operator-application.test.ts` around lines 169 - 196, Extend the test around application.install and signedHeaders to use an issuedAt earlier than the current time while remaining within the valid window, then assert that replay.consume receives consumedAt as NOW but expiresAt derived from that issuedAt plus the configured lifetime rather than NOW plus the lifetime. Preserve the existing installation forwarding assertions and use the established timestamp constants or helpers.Source: Coding guidelines
apps/integration-service/src/plugin-operator-replay.test.ts (1)
108-133: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win미검증 분기를 추가로 다루십시오.
현재 테스트는
rows.length === 0이면서rowCount !== 0인 결과를 다루지 않습니다. 이 조합은plugin-operator-replay.ts의 121-123행 분기를 실행하며, 모호한 결과를PluginOperatorReplayValidationError로 거부해야 합니다. 대문자 UUID 증거가 소문자로 정규화되어 SQL 파라미터에 전달되는지도 검증하십시오. 커버리지 게이트가 100% 분기 커버리지를 요구합니다.✅ 제안 추가 케이스
for (const inserted of [ { rows: [{ evidence_id: EVIDENCE_ID }], rowCount: null }, + { rows: [], rowCount: 1 }, {As per coding guidelines, "Packages that enforce coverage gates must retain 100% statement, branch, function, and line coverage."
🤖 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/integration-service/src/plugin-operator-replay.test.ts` around lines 108 - 133, Extend the test in “rejects ambiguous or corrupted INSERT evidence instead of granting authority” with a result where rows is empty and rowCount is nonzero, asserting PluginOperatorReplayValidationError. Also add coverage verifying uppercase evidence UUID input is normalized to lowercase before being passed as the SQL parameter, while preserving the existing rejection cases.Source: Coding guidelines
apps/integration-service/migrations/0003_plugin_operator_context_replay_record.sql (1)
1-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value테이블 계약을 설명하는 주석을 추가하십시오.
이 마이그레이션은 프로덕션 선언입니다. 현재 파일에는 각 컬럼의 의미와 보존 규칙에 대한 설명이 없습니다.
COMMENT ON TABLE과COMMENT ON COLUMN으로 증거 식별자의 일회성,consumed_at의 의미,expires_at의 정리 규칙을 명시하십시오.📝 제안 추가
CREATE INDEX plugin_operator_context_replay_expiry_index ON plugin_integration.plugin_operator_context_replay_record (expires_at); + +COMMENT ON TABLE plugin_integration.plugin_operator_context_replay_record IS + 'Durable one-time operator evidence identities consumed by the Integration service.'; +COMMENT ON COLUMN plugin_integration.plugin_operator_context_replay_record.evidence_id IS + 'Signed UUIDv4 evidence identity; the primary key enforces single consumption.'; +COMMENT ON COLUMN plugin_integration.plugin_operator_context_replay_record.consumed_at IS + 'Instant at which the winning service instance consumed this evidence.'; +COMMENT ON COLUMN plugin_integration.plugin_operator_context_replay_record.expires_at IS + 'Instant after which the record is prunable because the signature window closed.';As per coding guidelines, "Production declarations must include explanatory docstrings sufficient for a new contributor to understand the contract without reconstructing the implementation."
🤖 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/integration-service/migrations/0003_plugin_operator_context_replay_record.sql` around lines 1 - 10, Add explanatory SQL comments for the plugin_operator_context_replay_record table contract using COMMENT ON TABLE and COMMENT ON COLUMN. Document that evidence_id identifies a one-time replay record, consumed_at records when the evidence was consumed, and expires_at defines the retention and cleanup deadline; keep the existing schema and constraints unchanged.Source: Coding guidelines
apps/integration-service/src/plugin-operator-context.ts (1)
201-220: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win호환 래퍼를 제거하십시오.
requireTrustedPluginOperatorContext의 비테스트 호출자는 없습니다. 이 함수는evidenceId를 버리므로 replay guard를 적용할 수 없습니다. 함수와 호환 래퍼 전용 테스트를 제거하고requireVerifiedPluginOperatorContext만 사용하십시오.🤖 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/integration-service/src/plugin-operator-context.ts` around lines 201 - 220, Remove the unused requireTrustedPluginOperatorContext compatibility wrapper and its dedicated tests. Update any remaining references to call requireVerifiedPluginOperatorContext directly, preserving access to evidenceId so callers can apply the required replay guard.
🤖 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/integration-service/src/plugin-operator-replay.ts`:
- Around line 104-108: Update the expiration cleanup query in the consume flow
to compare expires_at against the database’s now() rather than safe.consumedAt,
and remove the application timestamp parameter. Move this DELETE out of every
consume invocation into the service’s periodic cleanup job, preserving the same
expired-record criteria.
---
Nitpick comments:
In
`@apps/integration-service/migrations/0003_plugin_operator_context_replay_record.sql`:
- Around line 1-10: Add explanatory SQL comments for the
plugin_operator_context_replay_record table contract using COMMENT ON TABLE and
COMMENT ON COLUMN. Document that evidence_id identifies a one-time replay
record, consumed_at records when the evidence was consumed, and expires_at
defines the retention and cleanup deadline; keep the existing schema and
constraints unchanged.
In `@apps/integration-service/src/plugin-operator-application.test.ts`:
- Around line 169-196: Extend the test around application.install and
signedHeaders to use an issuedAt earlier than the current time while remaining
within the valid window, then assert that replay.consume receives consumedAt as
NOW but expiresAt derived from that issuedAt plus the configured lifetime rather
than NOW plus the lifetime. Preserve the existing installation forwarding
assertions and use the established timestamp constants or helpers.
In `@apps/integration-service/src/plugin-operator-application.ts`:
- Around line 185-190: Update the replayGuard.consume error handling in the
surrounding method to make failures observable without exposing credentials or
retaining raw stack traces: add a structured log or metric with sanitized error
classification/details, and propagate the original failure via the
IntegrationOperatorContextError cause option so the boundary can redact it.
Preserve the existing 'unavailable' context error behavior.
In `@apps/integration-service/src/plugin-operator-context.ts`:
- Around line 201-220: Remove the unused requireTrustedPluginOperatorContext
compatibility wrapper and its dedicated tests. Update any remaining references
to call requireVerifiedPluginOperatorContext directly, preserving access to
evidenceId so callers can apply the required replay guard.
In `@apps/integration-service/src/plugin-operator-replay-migration.test.ts`:
- Around line 150-160: In the plugin operator replay migration tests, add a case
alongside the existing expiration-order test that inserts identical consumed_at
and expires_at timestamps and verifies the insertion succeeds. Preserve the
current rejection test for expires_at earlier than consumed_at, covering both
sides of the CHECK constraint boundary.
In `@apps/integration-service/src/plugin-operator-replay.test.ts`:
- Around line 108-133: Extend the test in “rejects ambiguous or corrupted INSERT
evidence instead of granting authority” with a result where rows is empty and
rowCount is nonzero, asserting PluginOperatorReplayValidationError. Also add
coverage verifying uppercase evidence UUID input is normalized to lowercase
before being passed as the SQL parameter, while preserving the existing
rejection cases.
🪄 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: 5ad00e0f-533f-47a6-a567-cc3d041e2495
📒 Files selected for processing (8)
apps/integration-service/migrations/0003_plugin_operator_context_replay_record.sqlapps/integration-service/src/plugin-operator-application.test.tsapps/integration-service/src/plugin-operator-application.tsapps/integration-service/src/plugin-operator-context.test.tsapps/integration-service/src/plugin-operator-context.tsapps/integration-service/src/plugin-operator-replay-migration.test.tsapps/integration-service/src/plugin-operator-replay.test.tsapps/integration-service/src/plugin-operator-replay.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/integration-service/src/plugin-operator-context.test.ts
|
@coderabbitai review |
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/integration-service/src/plugin-operator-replay-migration.test.ts (1)
122-128: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win데이터베이스 테스트 게이트를 fail closed로 바꾸십시오.
PostgreSQL 연결 설정이 없으면 replay migration 계약 테스트 전체가 건너뛰어 CI가 마이그레이션 오류를 보고하지 않을 수 있습니다. 연결 설정이 있으면
beforeEach의 스키마 초기화가 임의의 데이터베이스에 실행될 수 있습니다. CI에서는 연결 누락을 실패로 처리하고, 실행 전에 전용 테스트 데이터베이스의 호스트와 이름을 검증하십시오. 로컬 skip은 명시적 opt-in으로 제한하십시오.As per coding guidelines, 환경값은 신뢰하지 말고 malformed provider configuration에서는 fail closed해야 합니다.
🤖 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/integration-service/src/plugin-operator-replay-migration.test.ts` around lines 122 - 128, Update the PostgreSQL gate around describeWithPostgres and its beforeEach setup to fail closed when connection configuration is missing or malformed, rather than silently skipping the replay migration contract tests. Require an explicit local opt-in for any skip behavior, and validate that the configured host and database name identify the dedicated test database before executing schema reset or migration SQL.Source: Coding guidelines
🧹 Nitpick comments (1)
apps/integration-service/src/plugin-operator-replay.test.ts (1)
103-111: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftPostgreSQL의 중복 소비 원자성을 실제로 검증하세요.
ScriptedSqlClient가rowCount: 0을 미리 반환하므로, 이 테스트는apps/integration-service/src/plugin-operator-replay.ts:102-132의ON CONFLICT (evidence_id) DO NOTHING과 실제 unique constraint 동작을 증명하지 않습니다. 동일한evidenceId를 동시에 두 번 소비하고 정확히 한 호출만true를 반환하는 PostgreSQL 통합 테스트를 추가하세요. 기존 migration 테스트가 실제 PostgreSQL을 사용한다면 해당 테스트에 이 경계를 추가하면 됩니다.As per coding guidelines: Tests must prove realistic domain accuracy and failure behavior, not only mocked call counts.
🤖 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/integration-service/src/plugin-operator-replay.test.ts` around lines 103 - 111, Replace the mocked duplicate-consumption assertion in the PostgreSQL replay guard tests with an integration test using a real PostgreSQL database and schema migration, exercising two concurrent consume calls for the same evidenceId. Verify that exactly one call resolves true and the other false, proving the ON CONFLICT (evidence_id) DO NOTHING path and unique constraint behavior in Postgres; reuse the existing migration-test setup if available.Source: Coding guidelines
🤖 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/integration-service/src/plugin-operator-replay-migration.test.ts`:
- Around line 116-117: Replace the sensitive-field regex assertion in the
migration test with an allowlist validation after applying the migration: query
information_schema.columns and assert that only evidence_id, consumed_at, and
expires_at exist, with their exact expected types and NOT NULL constraints.
Ensure the assertions cover quoted identifiers and columns added through ALTER
TABLE ... ADD COLUMN, and retain failure behavior for any credential or
bearer-material columns such as api_key, access_token, or client_secret.
---
Outside diff comments:
In `@apps/integration-service/src/plugin-operator-replay-migration.test.ts`:
- Around line 122-128: Update the PostgreSQL gate around describeWithPostgres
and its beforeEach setup to fail closed when connection configuration is missing
or malformed, rather than silently skipping the replay migration contract tests.
Require an explicit local opt-in for any skip behavior, and validate that the
configured host and database name identify the dedicated test database before
executing schema reset or migration SQL.
---
Nitpick comments:
In `@apps/integration-service/src/plugin-operator-replay.test.ts`:
- Around line 103-111: Replace the mocked duplicate-consumption assertion in the
PostgreSQL replay guard tests with an integration test using a real PostgreSQL
database and schema migration, exercising two concurrent consume calls for the
same evidenceId. Verify that exactly one call resolves true and the other false,
proving the ON CONFLICT (evidence_id) DO NOTHING path and unique constraint
behavior in Postgres; reuse the existing migration-test setup if available.
🪄 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: 7f087c88-0283-40bb-9346-2cabb5943409
📒 Files selected for processing (5)
apps/integration-service/migrations/0003_plugin_operator_context_replay_record.sqlapps/integration-service/src/plugin-operator-application.test.tsapps/integration-service/src/plugin-operator-replay-migration.test.tsapps/integration-service/src/plugin-operator-replay.test.tsapps/integration-service/src/plugin-operator-replay.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/integration-service/migrations/0003_plugin_operator_context_replay_record.sql
- apps/integration-service/src/plugin-operator-replay.ts
- apps/integration-service/src/plugin-operator-application.test.ts
1 participant
Buyer/security outcome
Advance #130 by introducing a trusted Integration operator authority that binds workspace + authenticated user + exact HTTP method/path before plugin installation or credential lifecycle can become application authority. This prevents browser-selected tenant/user identifiers, route replay, or plugin-controlled metadata from self-authorizing host lifecycle operations.
Test-first state
The first commit intentionally defines the RED contract only. It imports the not-yet-implemented
plugin-operator-contextboundary so exact-head CI must fail before implementation. No historical check/review evidence transfers.Planned narrow implementation
Refs #130.
Summary by CodeRabbit
새로운 기능
테스트