Repository navigation
Conversation
…ating (M2, #2576) - Create crove_meetings schema with 5 tables: meeting, meeting_reader, audio_segment, meeting_video, media_key - Enforce reader-gated restrictive RLS and pinned SECURITY DEFINER functions - Implement AES-256-GCM chunked blob cipher with HKDF wrap subkey in @crove/kernel-runtime - Wire credential reseal rotation in apps/kernel/src/credential-reseal.ts - Create @crove/mod-meetings module with manifest, commands, queries, and orpc endpoints - Expand staged upload purposes: meeting_audio (300 MB) and meeting_video (2 GB) - Implement authenticated media playback endpoints with HTTP 206 Range support and SHA-256 verification - Add key shredding on meeting deletion - Add Cuoc hop sub-tab under Ludy Notes with consent gate modal and player - Update crove_kernel.org_purge and crove_kernel.health for meetings module - Closes #2576, Refs #932
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 21 minutes. View limit details
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. |
6 tasks
![dos[bot]](https://avatars.githubusercontent.com/in/4711427?s=60&v=4){kind=small}
There was a problem hiding this comment.
⏱️ Code Review completed (50 eligible files · 199,008 chars · 4 model call(s) · 7 PR unit(s))
Whole diff with bounded context at base e7bb674 / head aa22c8e.
Excluded by noise policy: pnpm-lock.yaml.
🔍 Verified Adversarial Review Findings
🟡 IMPORTANT
apps/kernel/src/http.ts:1300: Authorization header bypassed by ?token= query parameter on all /api/* routes- Source Side: RIGHT (head)
- Failure Trace: A request to GET /api/meetings//audio?token= with no Authorization header now authenticates: c.req.header('authorization') is null, so the new fallback builds
Bearer <token>from the query string and calls authenticate(). Previously the same request was rejected. This exposes the bearer token to URL logs, proxy/CDN logs, Referer headers, and browser history, and lets any endpoint under /api/* accept a token in the query string. The meeting media endpoints are the new sensitive consumers of this widened surface. - Evidence:
apps/kernel/src/http.ts:1300ataa22c8ed4c91ec8afeffea0d5631e368805c6368,apps/kernel/src/http.ts:1294ate7bb674c459f0ab1aa37ec330198ff7e9eb53264
🟡 IMPORTANT
modules/meetings/src/meetings.ts:250: Media key sealed with a hardcoded fallback key id when broker has no keyId- Source Side: RIGHT (head)
- Failure Trace: In buildMeetingClaimMedia,
const keyId = (deps.broker as { keyId?: string }).keyId ?? 'k1-default';is used when the broker does not expose a keyId property. The wrapped key is then sealed under key_id='k1-default' in crove_meetings.media_key. At playback time (http.ts line 1665) the broker derives the wrap subkey via broker.deriveKey(media.keyId, ...) or broker.derive(...). If the real broker's current key id differs from 'k1-default', deriveKey('k1-default', ...) throws CredentialError('unknown key') and every meeting created through this path is permanently undecryptable (MEDIA_KEY_UNAVAILABLE 503). The fallback silently produces an envelope that no rotation or playback path can open. - Evidence:
modules/meetings/src/meetings.ts:250ataa22c8ed4c91ec8afeffea0d5631e368805c6368,apps/kernel/src/http.ts:1665ataa22c8ed4c91ec8afeffea0d5631e368805c6368
1 possible issue(s) could not be grounded in or verified against the snapshot source, so they are not published.
Context investigation was bounded to the explicitly requested snapshot paths.
🔒 Dependency check
✅ Passed. Every new package version comes from the public registry or a source this lockfile already used for it, its checksum is pinned where the format records one, and OSV lists no new malicious package or high or critical vulnerability for the versions it was asked about.
Contributor
Author
Security review (session)
Blocking findings to fix before merge
Also folding into the same fix commit: key buffer zeroization and fail-closed |
…nd MFA compliance - P0-1 and P0-2: pin is_meeting_recorder and is_meeting_reader definers (SECURITY DEFINER, search_path = pg_catalog, pg_temp, crove_tenant only) and break recursive RLS - P0-3: enable RLS on media_key with zero login grants and install agent_write_guard - P0-4 and P1-1: strict bearer auth, 404 on uninstalled module and deleted meetings, HTTP 206 Range chunk streaming with deriveKey subkeys and security headers - P1-2 and P1-3: require consent selection before meeting creation, validate active non-viewer members for readers, validate magic container bytes and staged max_bytes - P1-4 and P3-1: split recorder delete (R1) and owner compliance delete (R3 with step-up MFA) - P2-1 and P2-2: zeroize plaintext keys in finally, check generation in file storage, add CAS check in credential reseal - P3-2: real PostgreSQL 17 integration test suite covering 7 security scenarios
There was a problem hiding this comment.
⏱️ Code Review completed (53 eligible files · 247,973 chars · 2 model call(s) · 9 PR unit(s))
Whole diff with bounded context at base e7bb674 / head 1b8ce1f.
Excluded by noise policy: pnpm-lock.yaml.
✅ No verified defects
1 possible issue(s) could not be grounded in or verified against the snapshot source, so they are not published.
🔒 Dependency check
✅ Passed. Every new package version comes from the public registry or a source this lockfile already used for it, its checksum is pinned where the format records one, and OSV lists no new malicious package or high or critical vulnerability for the versions it was asked about.
Contributor
Author
Security review (session)Reviewed commit: Round 1 blockers verified resolved in
Remaining blockers requested for commit 3:
|
4 tasks
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2576
Refs #932
Summary
This PR implements Milestone M2 for AI Meeting Notes: encrypted meeting media (audio and video), consent records, attendee reader gating, authenticated playback, and key shredding.
Database (crove_meetings):
Cryptography and Key Management (@crove/kernel-runtime):
Module @crove/mod-meetings and Kernel Wiring:
Web UI (apps/app):
Acceptance
User-Visible Changes
Evidence Against Acceptance Criteria
Review Results
Risk Left
📌 TL;DR
This PR introduces the "Meetings" module, enabling users to record, upload, and manage encrypted audio/video meeting recordings with mandatory consent tracking. It implements a new sub-navigation system for the Notes/Meetings/My-Work cluster and integrates media key management into the kernel's credential resealing workflow.
🎯 Type of Change
🔍 Changes Walkthrough
apps/app/src/components/notes-subnav.tsxapps/app/src/i18n/messages/en.tsapps/app/src/i18n/messages/vi.tsapps/app/src/lib/api.tsmeeting_audioandmeeting_videopurposes with specific size limits. Adds astageMediaUploadfunction to handle presigned uploads for large media files.apps/app/src/lib/kernel-errors.tsMEETING_NOT_FOUNDas a known kernel error code.apps/app/src/router.tsx/o/$slug/meetings(list) and/o/$slug/meetings/$id(detail) under the organization route tree.apps/app/src/routes/meeting-detail.tsxapps/app/src/routes/meetings.tsxapps/app/src/routes/my-work.tsxNotesSubnavcomponent into the My Work page.apps/app/src/routes/notes.tsxNotesSubnavcomponent into the Notes page.apps/app/test/kernel-errors.test.tsapps/kernel/package.json@crove/mod-meetingsas a workspace dependency.apps/kernel/src/config.tsmeetingsto the list of available modules.apps/kernel/src/credential-reseal.tsapps/kernel/src/http.ts📊 Architectural Flow
sequenceDiagram participant User participant App as Frontend (React) participant API as Kernel API participant Broker as Credential Broker participant Store as Blob Store User->>App: Click "Record New Meeting" App->>User: Display Consent Gate User->>App: Confirm Consent & Select Files App->>API: POST /api/files/uploads (purpose: meeting_audio) API-->>App: Return Presigned URL & Upload ID App->>Store: Upload Media File (Direct) App->>API: POST /api/meetings (Create Meeting) API->>Broker: Generate & Wrap Media Key API-->>App: Return Meeting ID App->>API: POST /api/meetings/:id/claim-media API->>Store: Verify & Link Media to Meeting Note over User,Store: Playback Flow User->>App: Click Play Audio App->>API: GET /api/meetings/:id/audio API->>Broker: Unwrap Media Key API->>Store: Fetch Encrypted Blob API->>API: Decrypt Blob API-->>App: Return Decrypted Audio Stream