feat(calendar): add encrypted self-hosted credential store - #203
Conversation
|
Warning Review limit reached
Next review available in: 6 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)
📝 WalkthroughWalkthroughAES-256-GCM 기반 파일 시크릿 저장소를 추가했습니다. 자격 증명은 opaque handle에 바인딩된 암호문으로 저장됩니다. 조회·삭제·입력 검증과 환경 변수 기반 생성 기능을 구현했습니다. 관련 통합 테스트도 추가했습니다. Changes암호화 파일 시크릿 저장소
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant CalendarEncryptedFileSecretStore
participant FileSystem
participant AES256GCM
Caller->>CalendarEncryptedFileSecretStore: writeSecret(credential)
CalendarEncryptedFileSecretStore->>AES256GCM: encrypt(payload, handle)
AES256GCM-->>CalendarEncryptedFileSecretStore: ciphertext
CalendarEncryptedFileSecretStore->>FileSystem: create encrypted file
Caller->>CalendarEncryptedFileSecretStore: readSecret(handle)
CalendarEncryptedFileSecretStore->>FileSystem: read and validate file
CalendarEncryptedFileSecretStore->>AES256GCM: decrypt(ciphertext, handle)
AES256GCM-->>CalendarEncryptedFileSecretStore: payload
CalendarEncryptedFileSecretStore-->>Caller: secret
🚥 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: 2
🧹 Nitpick comments (5)
apps/integration-calendar-service/src/calendar-encrypted-file-secret-store.ts (4)
16-17: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUUID 패턴의
i플래그는 실제로 동작하지 않습니다.
parseHandle은requireUuidV4가 소문자로 변환한 결과와 원본 문자열을 엄격히 비교합니다. 따라서 대문자 UUID 핸들은 항상 거부됩니다.i플래그는 핸들 경로에서 의미가 없습니다.핸들을 정규 소문자 형식만 허용하는 의도라면, 패턴에서
i를 제거하여 계약을 명시하십시오.requireUuidV4가 다른 곳에서 대소문자 무관 입력을 허용해야 한다면, 현재 코드를 유지하고 주석으로 이유를 기록하십시오.♻️ 제안 변경
-const UUID_V4_PATTERN = - /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/iu; +/** Canonical lowercase UUIDv4 form used for opaque handle identifiers. */ +const UUID_V4_PATTERN = + /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/u;Also applies to: 146-148
🤖 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-calendar-service/src/calendar-encrypted-file-secret-store.ts` around lines 16 - 17, Remove the unused case-insensitive `i` flag from `UUID_V4_PATTERN` so UUID handles explicitly require lowercase input, matching the strict comparison performed by `parseHandle` after `requireUuidV4` normalization. Keep the existing UUID validation behavior otherwise unchanged.
311-323: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
isSymbolicLink()검사는 중복입니다.
lstat의 결과에서isFile()이 참이면isSymbolicLink()는 항상 거짓입니다. 두 검사는 상호 배타적입니다. 심볼릭 링크는isFile()검사만으로 이미 차단됩니다.의도를 문서화하려면 조건을 제거하고 주석으로 남기십시오. dev/ino 재검증 로직은 그대로 유지하십시오.
♻️ 제안 변경
const before = await lstat(path); + // lstat + isFile() rejects symlinks; open() below is re-verified by dev/ino. if ( !before.isFile() || - before.isSymbolicLink() || before.size <= 0 || before.size > MAXIMUM_ENVELOPE_BYTES ) {🤖 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-calendar-service/src/calendar-encrypted-file-secret-store.ts` around lines 311 - 323, Remove the redundant before.isSymbolicLink() check from readSecret’s file validation condition, since lstat’s before.isFile() check already rejects symbolic links. Preserve the existing size checks and dev/ino revalidation logic; add a comment only if needed to document that symbolic links are intentionally rejected.
400-413: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value팩토리에서
requireDirectory호출이 중복됩니다.생성자(Line 234)가 이미
requireDirectory를 실행합니다. 팩토리의 추가 호출은 동일한 검증을 반복합니다. 동작은 같으므로 결함은 아닙니다.키 인자와 대칭을 맞추려면 두 값을 모두 원시 환경 값으로 전달하고 검증은 생성자에 위임하십시오.
unavailable()은never를 반환하므로 현재 삼항 표현식은 유효합니다.♻️ 제안 변경
try { return new CalendarEncryptedFileSecretStore( - requireDirectory(environment.CALENDAR_SECRET_STORE_DIRECTORY), + environment.CALENDAR_SECRET_STORE_DIRECTORY as string, typeof environment.CALENDAR_SECRET_STORE_KEY === 'string' ? environment.CALENDAR_SECRET_STORE_KEY : unavailable(), );🤖 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-calendar-service/src/calendar-encrypted-file-secret-store.ts` around lines 400 - 413, Remove the redundant requireDirectory call from createCalendarEncryptedFileSecretStoreFromEnvironment and pass environment.CALENDAR_SECRET_STORE_DIRECTORY directly to CalendarEncryptedFileSecretStore, keeping the existing key expression and catch behavior unchanged so validation remains delegated to the constructor.
238-245: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value비공개 헬퍼에 짧은 docstring을 추가하십시오.
ensureDirectory와pathForId는 저장소의 파일 배치와 권한 계약을 결정합니다. 새 기여자가 구현을 읽지 않고 계약을 이해하도록 설명을 추가하십시오. 특히mkdir뒤의 명시적chmod가 기존 디렉터리 권한을 강제하는 이유를 기록하십시오.♻️ 제안 변경
+ /** + * Creates the store root if absent and enforces owner-only permissions on + * pre-existing directories, because `mkdir` mode applies to creation only. + */ private async ensureDirectory(): Promise<void> { await mkdir(this.directory, { recursive: true, mode: 0o700 }); await chmod(this.directory, 0o700); } + /** Maps one validated UUIDv4 identifier to its envelope file path. */ private pathForId(id: string): string {코딩 가이드라인에 따라: "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-calendar-service/src/calendar-encrypted-file-secret-store.ts` around lines 238 - 245, 파일 배치와 권한 계약을 설명하는 짧은 docstring을 private 헬퍼 ensureDirectory와 pathForId에 추가하십시오. ensureDirectory 문서에는 디렉터리를 재귀적으로 생성하고 0o700 권한을 보장하며, 기존 디렉터리에도 명시적으로 chmod를 적용하는 이유를 기록하십시오. pathForId 문서에는 ID가 디렉터리 아래의 JSON 파일 경로로 변환된다는 계약을 설명하십시오.Source: Coding guidelines
apps/integration-calendar-service/src/calendar-encrypted-file-secret-store.test.ts (1)
152-174: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win커버리지 게이트를 충족하려면 분기 테스트를 추가하십시오.
현재 테스트는 정상 경로와 일부 실패 경로만 다룹니다. 다음 분기는 검증되지 않습니다.
writeSecret의EEXIST재시도 경로와MAXIMUM_WRITE_ATTEMPTS소진 경로.readSecret의MAXIMUM_ENVELOPE_BYTES초과 파일 거부.readSecret의dev/ino불일치 거부.requireEnvelope의schemaVersion/algorithm불일치 거부와requirePayload의 필드 거부.- 팩토리에서
CALENDAR_SECRET_STORE_DIRECTORY누락 및CALENDAR_SECRET_STORE_KEY누락.패키지가 커버리지 게이트를 적용하면 위 분기는 통과에 필요합니다. 필요하면 해당 테스트를 생성해 드릴 수 있습니다.
코딩 가이드라인에 따라: "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-calendar-service/src/calendar-encrypted-file-secret-store.test.ts` around lines 152 - 174, Expand the tests around createCalendarEncryptedFileSecretStoreFromEnvironment and the store’s writeSecret/readSecret flows to cover all uncovered branches: EEXIST retries through MAXIMUM_WRITE_ATTEMPTS exhaustion, oversized envelopes, dev/ino mismatches, invalid schemaVersion or algorithm in requireEnvelope, rejected payload fields in requirePayload, and missing CALENDAR_SECRET_STORE_DIRECTORY or CALENDAR_SECRET_STORE_KEY. Assert each branch’s expected error or rejection while preserving the existing success-path coverage.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-calendar-service/src/calendar-encrypted-file-secret-store.test.ts`:
- Around line 131-150: Update the test “deletes idempotently and rejects
malformed handles without path traversal” to also call deleteSecret with the
malformed traversal handle and assert it rejects with
CalendarEncryptedFileSecretStoreError. Keep the existing readSecret assertion
and idempotent deletion checks unchanged.
- Around line 99-108: Update the tampering setup in the encrypted payload test
around store.readSecret to decode payload.ciphertext, flip a bit in the decoded
bytes, re-encode the modified bytes as canonical base64, and assign that value
back before writing the file. Preserve the existing rejection assertions while
ensuring the mutation always changes the ciphertext and reaches GCM
authentication failure rather than canonical-base64 validation.
---
Nitpick comments:
In
`@apps/integration-calendar-service/src/calendar-encrypted-file-secret-store.test.ts`:
- Around line 152-174: Expand the tests around
createCalendarEncryptedFileSecretStoreFromEnvironment and the store’s
writeSecret/readSecret flows to cover all uncovered branches: EEXIST retries
through MAXIMUM_WRITE_ATTEMPTS exhaustion, oversized envelopes, dev/ino
mismatches, invalid schemaVersion or algorithm in requireEnvelope, rejected
payload fields in requirePayload, and missing CALENDAR_SECRET_STORE_DIRECTORY or
CALENDAR_SECRET_STORE_KEY. Assert each branch’s expected error or rejection
while preserving the existing success-path coverage.
In
`@apps/integration-calendar-service/src/calendar-encrypted-file-secret-store.ts`:
- Around line 16-17: Remove the unused case-insensitive `i` flag from
`UUID_V4_PATTERN` so UUID handles explicitly require lowercase input, matching
the strict comparison performed by `parseHandle` after `requireUuidV4`
normalization. Keep the existing UUID validation behavior otherwise unchanged.
- Around line 311-323: Remove the redundant before.isSymbolicLink() check from
readSecret’s file validation condition, since lstat’s before.isFile() check
already rejects symbolic links. Preserve the existing size checks and dev/ino
revalidation logic; add a comment only if needed to document that symbolic links
are intentionally rejected.
- Around line 400-413: Remove the redundant requireDirectory call from
createCalendarEncryptedFileSecretStoreFromEnvironment and pass
environment.CALENDAR_SECRET_STORE_DIRECTORY directly to
CalendarEncryptedFileSecretStore, keeping the existing key expression and catch
behavior unchanged so validation remains delegated to the constructor.
- Around line 238-245: 파일 배치와 권한 계약을 설명하는 짧은 docstring을 private 헬퍼
ensureDirectory와 pathForId에 추가하십시오. ensureDirectory 문서에는 디렉터리를 재귀적으로 생성하고 0o700
권한을 보장하며, 기존 디렉터리에도 명시적으로 chmod를 적용하는 이유를 기록하십시오. pathForId 문서에는 ID가 디렉터리 아래의
JSON 파일 경로로 변환된다는 계약을 설명하십시오.
🪄 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: d815ef4e-c5ee-482d-9377-6ba5eb8c76d6
📒 Files selected for processing (2)
apps/integration-calendar-service/src/calendar-encrypted-file-secret-store.test.tsapps/integration-calendar-service/src/calendar-encrypted-file-secret-store.ts
1 participant
Scope
Advance #129 with one bounded calendar-owned credential-storage slice. This draft starts with a test-first contract for a concrete self-hostable encrypted file secret store that implements the existing calendar create/materialization credential ports without persisting plaintext tokens in PostgreSQL.
Required properties
lifeos-calendar-secret://handlesThis intentionally does not yet claim end-to-end OAuth/KMS/runtime composition or retire the deployment-wide Google token. Those remain in #129.
Test-first state
Head
7d1974184df9cdedebe88b0f898acc2e7a0ce382is the intentional RED contract: it imports the not-yet-implemented production adapter and specifies encryption, tamper/wrong-key failure, deletion, path validation, and environment-key behavior. Production implementation follows only after the RED boundary is observed.Refs #129, #21.
Summary by CodeRabbit