Repository navigation
feat: enforce organization-only access for all recordings - #2355
richiemcilroy wants to merge 11 commits into
Conversation
|
hey @greptileai, please re-review the PR |
|
hey @greptileai, please re-review the PR |
|
hey @greptileai, please re-review the PR |
|
hey @greptileai, please re-review the PR |
|
hey @greptileai, please re-review the PR |
|
hey @greptileai, please re-review the PR |
| .filter( | ||
| (entry) => | ||
| entry.length > 0 && entry === entry.trim() && !entry.includes(","), | ||
| ) |
There was a problem hiding this comment.
Exact email matches are dropped
For member,team@example.invalid with that same address saved as the restriction, the direct access check accepts the exact match, but this filter removes the address before building its match pattern and rejects it. The new seed case therefore fails its parity assertion before the browser walkthrough can run. The collection filter also hides the recording from a viewer the direct access check allows.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/database/video-organization-access.ts
Line: 60-63
Comment:
**Exact email matches are dropped**
For `member,team@example.invalid` with that same address saved as the restriction, the direct access check accepts the exact match, but this filter removes the address before building its match pattern and rejects it. The new seed case therefore fails its parity assertion before the browser walkthrough can run. The collection filter also hides the recording from a viewer the direct access check allows.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.|
hey @greptileai, please re-review the PR |
|
hey @greptileai, please re-review the PR |
Demo: Watch the 50-second walkthrough. Recorded and exported with Cap on macOS at 1920×1080, using synthetic data. Public playback was verified through the end, and the recording is in the personal PR's folder.
Organizations can enable Only members of this organization to restrict every existing and new recording to signed-in members of its owning organization. The rule takes precedence over public links, guest invitations, passwords, and shares into other organizations or spaces. Turning it off restores the saved per-recording settings. Only owners and admins can change the rule.
Access checks cover playback, downloads, embeds, thumbnails, rich previews, API listings, and collections. Members can open protected collections in their organization, eligible externally shared recordings remain visible, and mobile details reflect the enforced policy. The recorded walkthrough demonstrates existing and new recordings, member playback, outsider denial, sharing controls, and restoration of saved settings.
Validation on the current commit: 311 focused tests across 14 files, web TypeScript, scoped formatting, and the recorded browser walkthrough passed. Isolated database checks verified membership and email-restriction parity. Generated migration validation and secret scanning also passed. The macOS capture uses a tracked recipe and a dedicated synthetic browser window after the Linux recording environment repeatedly failed to restore its prepared snapshot. This verifies the web feature on macOS; native app UI and Windows-specific behavior were not exercised.
Review: Greptile scored the current commit 5/5. The historical “Exact email matches are dropped” finding below is contradicted by regression tests in
videos-policy.test.tsand the executed database parity checks. Restrictions are comma-separated entries:member,team@example.invaliddoes not match that entire value as one email address;team@example.invalidmatches its second entry, and theexample.invaliddomain matches the former address. All three cases pass through the full direct-access policy. The historical thread remains open; no access rule was weakened to satisfy it.Rollout: apply generated migration 0050 before deploying the application. It adds one boolean column, defaulting to false. The column and fixture queries were verified on an isolated development database; the existing schema-push command also reported an unrelated prefix-index syntax error. Production is unchanged. Already downloaded copies and temporary media URLs issued before restriction cannot be recalled.
CI: web typechecking, formatting, linting, mobile checks, migration validation, and recording reliability checks passed. Desktop builds and Rust analysis are still running for the current commit, so this remains a draft. The preceding commit's three desktop jobs failed the existing GPUI unused-icon test in unchanged code.
The reviewed code appears safe to merge, subject to completing the draft PR’s required recorded demonstration.
Findings
Fix with agent prompt
Summary
The PR adds an organization-wide recording access setting and applies it across playback, downloads, previews, listings, collections, and mobile responses.
Reviews (8) · Last reviewed commit: "test: record organization access with a ..."