feat(habit): expose trusted data-rights contributor transport - #192
Conversation
|
Warning Review limit reached
Next review available in: 21 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 (2)
📝 WalkthroughWalkthroughHabit 데이터 권리 요청의 HTTP 신뢰 경계와 HMAC 인증을 추가했습니다. 인증된 요청만 내부 contributor로 전달합니다. Destructive 요청은 PostgreSQL replay guard로 한 번만 처리합니다. 요청 오류와 contributor 오류를 전용 HTTP 예외로 변환합니다. 관련 테스트와 시크릿 설정 예시도 추가했습니다. ChangesHabit 데이터 권리 요청
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant HabitDataRightsController
participant HabitDataRightsHttpBoundary
participant HabitRuntime
participant PostgreSQL
participant DataRightsContributor
Client->>HabitDataRightsController: POST /v1/internal/data-rights/contributor
HabitDataRightsController->>HabitDataRightsHttpBoundary: 요청 본문과 HMAC 서명 검증
HabitDataRightsHttpBoundary->>HabitRuntime: destructive 요청의 replay 증거 소비
HabitRuntime->>PostgreSQL: digest 원자적 소비
PostgreSQL-->>HabitRuntime: 최초 소비 여부 반환
HabitRuntime-->>HabitDataRightsHttpBoundary: 소비 결과 반환
HabitDataRightsHttpBoundary-->>HabitDataRightsController: 정규화된 요청 반환
HabitDataRightsController->>DataRightsContributor: 검증된 요청 전달
DataRightsContributor-->>HabitDataRightsController: 처리 결과 반환
HabitDataRightsController-->>Client: HTTP 응답 반환
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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: 3
🤖 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/habit-service/src/habit-data-rights-http-boundary.ts`:
- Around line 195-239: Update parseTrustedHabitDataRightsRequest in
apps/habit-service/src/habit-data-rights-http-boundary.ts (lines 195-239) to
atomically consume a replay identifier in the habit owner’s persistence store
before destructive erase persistence, rejecting duplicate signed authority. In
apps/habit-service/src/habit-data-rights-http-boundary.test.ts (lines 104-127),
submit the same signed erase request twice and verify the contributor is not
invoked on the second submission.
- Around line 20-25: 문제 응답 계약과 요청 정규화·검증·digest 생성 규칙이 문서화되지 않았습니다.
apps/habit-service/src/habit-data-rights-http-boundary.ts:20-25의
HabitDataRightsProblemDetails에 외부 응답 형식을, :36-41의 NormalizedRequest에 정규화 보장을,
:43-188의 검증 helper와 requestDigest에 실패 조건 및 canonical digest 입력 순서를 설명하는
docstring을 추가하세요.
- Around line 243-245: Update toHabitDataRightsHttpException so only
permission-parsing errors are returned unchanged; do not pass through arbitrary
HttpException instances raised by dataRightsContributor.handle(). Convert every
contributor error, including HttpException, into the bounded credential-free 503
data_rights_unavailable problem response, and add a regression test covering a
contributor that throws HttpException.
🪄 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: 856bfe4a-7f07-4014-9942-abd4787e0086
📒 Files selected for processing (5)
.env.exampleapps/habit-service/src/habit-data-rights-controller-authority.test.tsapps/habit-service/src/habit-data-rights-http-boundary.test.tsapps/habit-service/src/habit-data-rights-http-boundary.tsapps/habit-service/src/main.ts
|
@coderabbitai review |
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
apps/habit-service/src/habit-data-rights-controller-authority.test.ts (1)
27-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win정규 HMAC 필드 순서의 테스트 사본을 하나로 합치세요.
이
signature헬퍼는habit-data-rights-http-boundary.test.ts의 서명 헬퍼와 동일한 필드 순서를 반복합니다. productionrequestDigest까지 포함하면 같은 계약이 세 곳에 존재합니다. 필드 순서가 바뀌면 두 테스트 파일을 모두 수정해야 합니다. 수정 누락 시 한쪽 테스트만 통과합니다.공유 테스트 헬퍼 모듈로 추출하세요.
🤖 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/habit-service/src/habit-data-rights-controller-authority.test.ts` around lines 27 - 45, Extract the duplicated canonical HMAC field ordering from signature in habit-data-rights-controller-authority.test.ts and the corresponding helper in habit-data-rights-http-boundary.test.ts into one shared test helper module, reusing the production requestDigest contract rather than maintaining separate field lists. Update both test files to call the shared helper and remove their local signature implementations.apps/habit-service/src/habit-data-rights-http-boundary.ts (1)
299-307: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win오류 원인을 관측 가능하게 남기세요.
toHabitDataRightsHttpException은 모든 오류를 폐기합니다. 응답을 bounded, credential-free로 유지하는 결정은 올바릅니다. 다만 원인이 완전히 사라지면 운영 중 장애 원인 분석이 불가능합니다. 오류 종류와requestId같은 비민감 식별자만 구조화 로그로 남기세요. 원문 메시지와 stack trace는 계속 응답과 보존 아티팩트에서 제외하세요.🤖 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/habit-service/src/habit-data-rights-http-boundary.ts` around lines 299 - 307, The toHabitDataRightsHttpException function currently discards all error context; add structured logging there that records only the error type and a non-sensitive requestId when available. Keep the existing bounded, credential-free response unchanged, and do not include the original message or stack trace in the response or preserved artifacts.Source: Coding guidelines
apps/habit-service/src/habit-data-rights-authority-replay.test.ts (1)
58-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win형식은 맞지만 존재하지 않는 시각 케이스를 추가하세요.
현재 malformed 케이스는 패턴 자체가 틀린
'2026-08-12'뿐입니다.habit-data-rights-authority-replay.ts51행의parsed.toISOString() !== value분기는 검증되지 않습니다. 패턴은 통과하지만 정규 형식이 아닌 값을 추가하세요. 이 분기는 커버리지 게이트 대상입니다.💚 제안 변경
for (const evidence of [ { evidenceDigest: 'not-a-digest', expiresAt: EXPIRES_AT }, { evidenceDigest: DIGEST, expiresAt: '2026-08-12' }, + { evidenceDigest: DIGEST, expiresAt: '2026-02-30T00:00:00.000Z' }, ]) {🤖 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/habit-service/src/habit-data-rights-authority-replay.test.ts` around lines 58 - 61, Extend the evidence cases in the replay test around the existing expiresAt validation to include a timestamp that matches the accepted pattern but fails the canonical ISO representation check, so the parsed.toISOString() !== value branch in the replay implementation is exercised while retaining the current malformed cases.Source: Coding guidelines
apps/habit-service/src/habit-data-rights-authority-replay.ts (1)
78-92: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoff만료 정리를 요청 경로에서 분리하는 방안을 검토하세요.
consume는 요청마다DELETE를 먼저 실행합니다. 정확성은 유지됩니다.INSERT가WHERE $2::timestamptz >= now()로 만료를 다시 확인하기 때문입니다. 다만 모든 destructive 요청이 추가 쓰기를 발생시킵니다. 이는 write amplification과 vacuum 부하를 만듭니다.만료 정리를 주기 작업이나 파티션 삭제로 옮기는 방안을 검토하세요. 요청 경로에는
INSERT만 남기세요.🤖 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/habit-service/src/habit-data-rights-authority-replay.ts` around lines 78 - 92, Remove the per-request DELETE cleanup from the consume flow in habit-data-rights-authority-replay.ts, leaving the INSERT query’s expiresAt validation and conflict handling unchanged. Move expired-record cleanup to an appropriate periodic maintenance job or partition-retention mechanism, and ensure the request path performs only the INSERT operation.apps/habit-service/src/habit-data-rights-http-boundary.test.ts (1)
147-158: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win응답 본문이 저장소 오류 세부정보를 포함하지 않음을 검증하세요.
이 테스트는
'database detail'메시지를 가진 오류를 주입합니다. 그러나 상태 코드만 확인합니다. 문제 응답 본문에 해당 문자열이 없음을 함께 검증하세요. 이 검증은 저장소 세부정보 유출 회귀를 직접 차단합니다.🛡️ 제안 변경
- await expectHttpStatus( - () => - parseTrustedHabitDataRightsRequest( - ERASE_REQUEST, - { issuedAt, signature }, - CONTEXT_SECRET, - BINDING, - NOW_SECONDS, - { consume: vi.fn().mockRejectedValue(new Error('database detail')) }, - ), - 503, - ); + let thrown: unknown; + try { + await parseTrustedHabitDataRightsRequest( + ERASE_REQUEST, + { issuedAt, signature }, + CONTEXT_SECRET, + BINDING, + NOW_SECONDS, + { consume: vi.fn().mockRejectedValue(new Error('database detail')) }, + ); + } catch (error) { + thrown = error; + } + expect(thrown).toBeInstanceOf(HttpException); + expect((thrown as HttpException).getStatus()).toBe(503); + expect(JSON.stringify((thrown as HttpException).getResponse())).not.toContain( + 'database detail', + );🤖 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/habit-service/src/habit-data-rights-http-boundary.test.ts` around lines 147 - 158, Update the test around parseTrustedHabitDataRightsRequest to capture the full HTTP response from the injected consume rejection, assert status 503, and additionally verify the response body does not contain “database detail”.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/habit-service/src/habit-data-rights-authority-replay.integration.test.ts`:
- Around line 71-87: Update the replay-consumption test around guard.consume so
evidenceDigest is deterministically derived from rawSignature using the
production digesting path or equivalent domain helper, then pass that digest to
both consume calls. Keep the assertions verifying the stored digest and absence
of rawSignature, so the test can fail if the original signature is persisted.
In `@apps/habit-service/src/habit-runtime.ts`:
- Line 3: HabitRuntime의 dataRightsAuthorityReplayGuard 필드 타입을 구체 구현체인
PostgresHabitDataRightsAuthorityReplayGuard가 아닌
HabitDataRightsAuthorityReplayGuardPort로 변경하세요.
parseTrustedHabitDataRightsRequest가 요구하는 포트 계약을 사용하도록 관련 import와 타입 참조를 정리하고,
createHabitRuntime에서는 기존처럼 PostgreSQL 구현을 생성해 주입하세요.
---
Nitpick comments:
In `@apps/habit-service/src/habit-data-rights-authority-replay.test.ts`:
- Around line 58-61: Extend the evidence cases in the replay test around the
existing expiresAt validation to include a timestamp that matches the accepted
pattern but fails the canonical ISO representation check, so the
parsed.toISOString() !== value branch in the replay implementation is exercised
while retaining the current malformed cases.
In `@apps/habit-service/src/habit-data-rights-authority-replay.ts`:
- Around line 78-92: Remove the per-request DELETE cleanup from the consume flow
in habit-data-rights-authority-replay.ts, leaving the INSERT query’s expiresAt
validation and conflict handling unchanged. Move expired-record cleanup to an
appropriate periodic maintenance job or partition-retention mechanism, and
ensure the request path performs only the INSERT operation.
In `@apps/habit-service/src/habit-data-rights-controller-authority.test.ts`:
- Around line 27-45: Extract the duplicated canonical HMAC field ordering from
signature in habit-data-rights-controller-authority.test.ts and the
corresponding helper in habit-data-rights-http-boundary.test.ts into one shared
test helper module, reusing the production requestDigest contract rather than
maintaining separate field lists. Update both test files to call the shared
helper and remove their local signature implementations.
In `@apps/habit-service/src/habit-data-rights-http-boundary.test.ts`:
- Around line 147-158: Update the test around parseTrustedHabitDataRightsRequest
to capture the full HTTP response from the injected consume rejection, assert
status 503, and additionally verify the response body does not contain “database
detail”.
In `@apps/habit-service/src/habit-data-rights-http-boundary.ts`:
- Around line 299-307: The toHabitDataRightsHttpException function currently
discards all error context; add structured logging there that records only the
error type and a non-sensitive requestId when available. Keep the existing
bounded, credential-free response unchanged, and do not include the original
message or stack trace in the response or preserved artifacts.
🪄 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: 1421dc98-3cc4-4181-ac63-66135e6b204a
📒 Files selected for processing (9)
apps/habit-service/migrations/0003_data_rights_authority_replay.sqlapps/habit-service/src/habit-data-rights-authority-replay.integration.test.tsapps/habit-service/src/habit-data-rights-authority-replay.test.tsapps/habit-service/src/habit-data-rights-authority-replay.tsapps/habit-service/src/habit-data-rights-controller-authority.test.tsapps/habit-service/src/habit-data-rights-http-boundary.test.tsapps/habit-service/src/habit-data-rights-http-boundary.tsapps/habit-service/src/habit-runtime.tsapps/habit-service/src/main.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/habit-service/src/main.ts
1 participant
Summary
life-os.data-rights-contributor.v1participant over an internal service-authenticated HTTP boundaryScope / maturity
This advances #55 but does not claim the end-to-end data-rights workflow is complete. Identity orchestration still needs a production client/registration path for this contributor, followed by the remaining contributor coverage, artifact delivery, lifecycle/audit, and recovery evidence described by the canonical data-rights plan.
Validation
CI and required security/review workflows must validate exact head; no pending/skipped/stale result is treated as passing.
Refs #55
Summary by CodeRabbit
새 기능
테스트