feat(folders): public sharing of a folder via a signed link#1890
feat(folders): public sharing of a folder via a signed link#1890alexis-morain wants to merge 2 commits into
Conversation
Adds a per-folder "Share" action that produces a public URL anyone with the link can open to browse every video in the folder. No client account required. How it works ------------ - New boolean column folders.publicShared (default false). - Slug is an HMAC of the folder id signed with NEXTAUTH_SECRET (no DB row per share, no extra table). Rotating NEXTAUTH_SECRET revokes all outstanding folder share links. - Enabling sharing flips folders.publicShared=true AND sets every video in that folder to videos.public=true so the existing /s/<videoId> share page keeps working unauthenticated. Disabling reverts both. - Public landing page lives at /share/f/[slug], rendered as a server component. It validates the slug, refuses if publicShared=false, resolves S3-signed thumbnail URLs server-side, and links each card to the existing /s/<videoId> player. - Proxy whitelist gains /share/ so the route is reachable on self-hosted instances (matches the existing /embed/ entry). UI -- - New Share folder item in the per-folder kebab dropdown (caps page only; suppressed for folders owned by a space). - ShareFolderDialog: state hydration, Enable / Disable buttons, read-only input + Copy button for the public URL. Notes for reviewers ------------------- - The folder share affects every video in the folder. The dialog copy states this explicitly. There is no per-video override yet. - I did not regenerate the Drizzle migration (no MySQL on the dev machine that pushed this branch); please run `pnpm db:generate` to produce the migration file and journal entry. - A follow-up could swap "set videos.public=true on share" for a signed query param the /s/<videoId> route checks against the folder slug; that lets shared videos stay private to direct guesses. Kept out of this PR to limit the blast radius.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| await db() | ||
| .update(videos) | ||
| .set({ public: false }) | ||
| .where(eq(videos.folderId, folderId)); |
There was a problem hiding this comment.
disableFolderSharing permanently silences pre-existing public videos
disableFolderSharing unconditionally sets public = false on every video in the folder, regardless of whether those videos were already public before folder sharing was enabled. A user who individually shared a video via /s/<videoId> before ever touching the folder-share feature will silently find that video made private the moment they disable folder sharing — with no way to know it happened.
enableFolderSharing should record which videos it flipped (e.g., by filtering on public = false before updating, or by storing a flag like sharedViaFolder), so disableFolderSharing can revert only those videos rather than all of them.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/actions/folders/share-folder.ts
Line: 59-62
Comment:
**`disableFolderSharing` permanently silences pre-existing public videos**
`disableFolderSharing` unconditionally sets `public = false` on every video in the folder, regardless of whether those videos were already public before folder sharing was enabled. A user who individually shared a video via `/s/<videoId>` before ever touching the folder-share feature will silently find that video made private the moment they disable folder sharing — with no way to know it happened.
`enableFolderSharing` should record which videos it flipped (e.g., by filtering on `public = false` before updating, or by storing a flag like `sharedViaFolder`), so `disableFolderSharing` can revert only those videos rather than all of them.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| @@ -0,0 +1,39 @@ | |||
| import { createHmac } from "node:crypto"; | |||
There was a problem hiding this comment.
timingSafeEqual import is missing when switching to constant-time comparison.
| import { createHmac } from "node:crypto"; | |
| import { createHmac, timingSafeEqual } from "node:crypto"; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/lib/folder-share.ts
Line: 1
Comment:
The `timingSafeEqual` import is missing when switching to constant-time comparison.
```suggestion
import { createHmac, timingSafeEqual } from "node:crypto";
```
How can I resolve this? If you propose a fix, please make it concise.| } catch { | ||
| return null; | ||
| } | ||
| if (sign(folderId) !== sig) return null; |
There was a problem hiding this comment.
sign(folderId) !== sig is a standard string comparison that short-circuits on the first differing byte, leaking timing information. Since the HMAC check runs before any DB lookup, an attacker can probe it freely. The fix is to use timingSafeEqual from node:crypto, which always runs in constant time regardless of where the strings differ.
| if (sign(folderId) !== sig) return null; | |
| const expected = Buffer.from(sign(folderId)); | |
| const actual = Buffer.from(sig); | |
| if (expected.length !== actual.length || !timingSafeEqual(expected, actual)) | |
| return null; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/lib/folder-share.ts
Line: 37
Comment:
**Non-constant-time HMAC verification is susceptible to timing attacks**
`sign(folderId) !== sig` is a standard string comparison that short-circuits on the first differing byte, leaking timing information. Since the HMAC check runs before any DB lookup, an attacker can probe it freely. The fix is to use `timingSafeEqual` from `node:crypto`, which always runs in constant time regardless of where the strings differ.
```suggestion
const expected = Buffer.from(sign(folderId));
const actual = Buffer.from(sig);
if (expected.length !== actual.length || !timingSafeEqual(expected, actual))
return null;
```
How can I resolve this? If you propose a fix, please make it concise.| async function resolveThumbnailUrl( | ||
| videoId: string, | ||
| ownerId: string, | ||
| bucketId: string | null, | ||
| ): Promise<string | null> { | ||
| try { | ||
| const [bucket] = await S3Buckets.getBucketAccess( | ||
| Option.fromNullable(bucketId), | ||
| ).pipe(runPromise); | ||
| const listResponse = await bucket | ||
| .listObjects({ prefix: `${ownerId}/${videoId}/` }) | ||
| .pipe(runPromise); | ||
| const contents = listResponse.Contents || []; | ||
| const thumbnailKey = contents.find((item) => | ||
| item.Key?.endsWith("screen-capture.jpg"), | ||
| )?.Key; | ||
| if (!thumbnailKey) return null; | ||
| const url = await bucket | ||
| .getSignedObjectUrl(thumbnailKey) | ||
| .pipe(runPromise); | ||
| return url; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
resolveThumbnailUrl makes two sequential S3 API calls per video (listObjects then getSignedObjectUrl). For a folder with 30 videos, the page issues 60 S3 calls concurrently via Promise.all — each of which fans out to the cloud. This will cause noticeably slow page loads and may hit S3 rate limits on large folders. If the thumbnail key pattern is stable (e.g., {ownerId}/{videoId}/screen-capture.jpg), the listObjects call can be skipped and the signed URL generated directly from the known key.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/app/share/f/[slug]/page.tsx
Line: 47-71
Comment:
**N+1 S3 API calls on page load**
`resolveThumbnailUrl` makes two sequential S3 API calls per video (`listObjects` then `getSignedObjectUrl`). For a folder with 30 videos, the page issues 60 S3 calls concurrently via `Promise.all` — each of which fans out to the cloud. This will cause noticeably slow page loads and may hit S3 rate limits on large folders. If the thumbnail key pattern is stable (e.g., `{ownerId}/{videoId}/screen-capture.jpg`), the `listObjects` call can be skipped and the signed URL generated directly from the known key.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| const [folder] = await db() | ||
| .select({ name: folders.name, publicShared: folders.publicShared }) | ||
| .from(folders) | ||
| .where(eq(folders.id, folderId as any)); |
There was a problem hiding this comment.
as any casts suppress Drizzle type safety
Both WHERE clauses cast folderId to any (lines 24, 88, 105). This suggests a branded-type mismatch between the decoded string returned by verifyFolderShareSlug and the column's declared type (Folder.FolderId). The right fix is to cast the return value of verifyFolderShareSlug to the correct branded type once, or to type the function return as Folder.FolderId | null, instead of silencing the type checker at every call site.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/web/app/share/f/[slug]/page.tsx
Line: 24
Comment:
**`as any` casts suppress Drizzle type safety**
Both WHERE clauses cast `folderId` to `any` (lines 24, 88, 105). This suggests a branded-type mismatch between the decoded string returned by `verifyFolderShareSlug` and the column's declared type (`Folder.FolderId`). The right fix is to cast the return value of `verifyFolderShareSlug` to the correct branded type once, or to type the function return as `Folder.FolderId | null`, instead of silencing the type checker at every call site.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
disable preserves pre-existing public videos, skip listObjects) - folder-share.ts: switch HMAC comparison to crypto.timingSafeEqual and return Folder.FolderId | null instead of string | null so the Drizzle WHERE clauses no longer need `as any`. - share-folder.ts: track which videos the share itself flipped via a new videos.publicSourcedFromFolderShare boolean. enableFolderSharing only flips videos that were public=false (and stamps the flag); disableFolderSharing only reverts videos with the flag, so a video the user had individually marked public stays public after the folder is unshared. - share/f/[slug]/page.tsx: drop the listObjects probe — the thumbnail key pattern is stable (ownerId/videoId/screenshot/screen-capture.jpg) so we can sign it directly. Halves the S3 round-trips per video. - packages/database/schema.ts: new boolean column videos.publicSourcedFromFolderShare (default false).
1 participant
Summary
Adds a per-folder Share folder action that produces a public URL anyone with the link can open. The visitor sees a clean index of the folder's videos (thumbnails, names, durations, dates) and clicks through to the existing
/s/<videoId>player — no account, no signup.I built this for a self-hosted instance where I needed to hand a client all the videos in one project folder without giving them a Cap account. Felt generally useful so I'm upstreaming it.
Architecture
folders.publicShared boolean default false. No new table; the slug is derived, not stored.base64url(folderId) + \".\" + hmac(folderId, NEXTAUTH_SECRET). Stateless. RotatingNEXTAUTH_SECRETrevokes every outstanding folder-share link.enableFolderSharing(folderId)flipsfolders.publicShared = trueANDvideos.public = truefor every video in the folder so the existing share page keeps working unauthenticated.disableFolderSharingreverts both./share/f/[slug]/page.tsx(Server Component): verifies the slug, refuses ifpublicShared = false, resolves S3 signed thumbnail URLs server-side (the existing/api/thumbnailreturns JSON, not the image), renders a responsive grid linking each card to/s/<videoId>./share/added to the self-hosted whitelist next to/embed/so the route is reachable without auth.FoldersDropdownkebab menu (hidden for folders owned by a Space — out of scope for this PR). NewShareFolderDialogwith state hydration, Enable / Disable buttons, copy-to-clipboard.Test plan
/dashboard/caps, kebab on any folder → Share folder → dialog opens, shows "Enable sharing"./s/<videoId>plays without login./s/<videoId>from that folder requires auth again (videos flipped back to private).NEXTAUTH_SECRET→ previously-generated links return 404. Generating a new link produces a different slug.Notes for reviewers
pnpm db:generateto emit the migration file and journal entry; the schema change is just the new boolean column./s/<videoId>work for guests without touching the share page. Downside: someone who guesses a video id while a folder is shared can hit it directly. A follow-up could swap this for a signed query param/s/<videoId>?fs=<slug>checked against the folder slug, keeping the videos technically private. Kept out of this PR to limit the blast radius.setShareDialogOpen={spaceId ? undefined : ...}. The Space sharing model is a separate question and I didn't want to overload it here.Greptile Summary
This PR adds stateless public folder sharing: a signed slug (
base64url(folderId) + \".\" + truncated-HMAC) is generated fromNEXTAUTH_SECRET, and a new/share/f/[slug]page renders the folder's video grid for unauthenticated visitors.disableFolderSharingis destructive: it unconditionally bulk-sets all videos in the folder topublic = false, which permanently makes private any video that was already individually shared before folder sharing was ever enabled. Only videos whosepublicflag was flipped byenableFolderSharingshould be reverted.verifyFolderShareSluguses!==to compare the expected and received HMAC signatures;crypto.timingSafeEqualshould be used instead.listObjects+getSignedObjectUrl) for every video; for large folders this causes meaningful latency and could be reduced by constructing the key directly.Confidence Score: 3/5
Not safe to merge as-is: disabling folder sharing will silently revoke individually-shared videos, and the HMAC verification is not constant-time.
Two issues need resolution before merge. The
disableFolderSharingaction bulk-sets every video in the folder to private without checking whether those videos were already public before folder sharing was enabled — any video a user had independently shared will silently lose its public access. The HMAC verification infolder-share.tsuses a plain string comparison instead oftimingSafeEqual, introducing a timing oracle on a security-critical path.apps/web/actions/folders/share-folder.ts(destructive video visibility reset) andapps/web/lib/folder-share.ts(non-constant-time signature check) both need fixes before this lands.Security Review
apps/web/lib/folder-share.tsline 37):sign(folderId) !== siguses a short-circuit string comparison, leaking byte-position timing information. Because the HMAC check runs before any database query, an attacker can probe it freely without needing a real folder ID. Replacing it withcrypto.timingSafeEqualeliminates the timing oracle.Important Files Changed
verifyFolderShareSlug, which should usetimingSafeEqualinstead.disableFolderSharingunconditionally sets all folder videos to private, destroying the pre-existing public state of videos that were already shared individually before folder sharing was enabled.as anycasts to work around Drizzle branded-type mismatches.handleEnable.undefinedprop guard.publicShared boolean NOT NULL DEFAULT falsecolumn tofolderstable; schema change is straightforward, migration not yet generated (noted in PR)./share/to the self-hosted auth bypass whitelist alongside/embed/; minimal and correct change.ShareFolderDialoginto the folder card; correctly gates the share action behind!spaceId.Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat(folders): public sharing of a folde..." | Re-trigger Greptile