Skip to content

feat: enforce organization-only access for all recordings - #2355

Draft
richiemcilroy wants to merge 11 commits into
mainfrom
building/org-video-lock-23154473
Draft

richiemcilroy wants to merge 11 commits into
mainfrom
building/org-video-lock-23154473

Conversation

@richiemcilroy

@richiemcilroy richiemcilroy commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

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.ts and the executed database parity checks. Restrictions are comma-separated entries: member,team@example.invalid does not match that entire value as one email address; team@example.invalid matches its second entry, and the example.invalid domain 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.

RetriggerConfidence Score: 5/5

The reviewed code appears safe to merge, subject to completing the draft PR’s required recorded demonstration.

Findings

  1. P1 Exact email matches are dropped ▶
Fix with agent prompt
### Issue 1
packages/database/video-organization-access.ts:60-63
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.

Summary

The PR adds an organization-wide recording access setting and applies it across playback, downloads, previews, listings, collections, and mobile responses.

  • Saved per-recording settings remain available when the organization restriction is disabled.
  • The latest changes add policy tests and a macOS demonstration-capture recipe; no new actionable issue was identified.

Reviews (8) · Last reviewed commit: "test: record organization access with a ..."

@richiemcilroy

Copy link
Copy Markdown
Member Author

hey @greptileai, please re-review the PR

@richiemcilroy

Copy link
Copy Markdown
Member Author

hey @greptileai, please re-review the PR

Comment thread apps/web/lib/public-collections.ts Outdated
Comment thread apps/web/lib/public-collections.ts Outdated
Comment thread apps/web/app/api/mobile/[...route]/route.ts
Comment thread apps/web/lib/og/video-og.tsx
@richiemcilroy

Copy link
Copy Markdown
Member Author

hey @greptileai, please re-review the PR

Comment thread apps/web/lib/public-collections.ts Outdated
@richiemcilroy

Copy link
Copy Markdown
Member Author

hey @greptileai, please re-review the PR

@richiemcilroy

Copy link
Copy Markdown
Member Author

hey @greptileai, please re-review the PR

Comment thread apps/web/lib/public-collections.ts Outdated
@richiemcilroy

Copy link
Copy Markdown
Member Author

hey @greptileai, please re-review the PR

Comment on lines +60 to +63
.filter(
(entry) =>
entry.length > 0 && entry === entry.trim() && !entry.includes(","),
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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.

@richiemcilroy

Copy link
Copy Markdown
Member Author

hey @greptileai, please re-review the PR

@richiemcilroy

Copy link
Copy Markdown
Member Author

hey @greptileai, please re-review the PR

This branch was successfully deployed

1 active deployment
Preview — e607d115 Deployed Sep 26, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant