feat(files): public share links for workspace files#5130
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview Backend: new Public UI: Viewer plumbing: binary/text fetch hooks and image preview go through the content-source seam; read-only previews skip CSV “import as table” and treat oversized CSVs as download-only on the public path. Reviewed by Cursor Bugbot for commit d939f0c. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
@greptile review |
# Conflicts: # apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/file-viewer.tsx # apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/index.ts # packages/db/migrations/meta/0241_snapshot.json # packages/db/migrations/meta/_journal.json # scripts/check-api-validation-contracts.ts
Greptile SummaryThis PR adds public share links for workspace files: a new
Confidence Score: 5/5Safe to merge — the feature is well-isolated, public endpoints are rate-limited and return consistent 404s without leaking workspace data, and all issues raised in prior review threads have been addressed. All previously flagged issues (stale draftActive, download anchor, soft-delete filter, content-endpoint rate limiting, CSV import toast on public page, createdBy cascade) have been resolved. The two remaining observations are both low-likelihood: the token-unique conflict path in upsertFileShare is practically unreachable with 126-bit nanoid tokens, and the close-animation skip in the share modal is cosmetic. No files require special attention. The share-manager.ts upsert and share-modal.tsx unmount pattern are the only areas worth a second glance, but neither poses a correctness risk in production. Important Files Changed
Reviews (9): Last reviewed commit: "feat(files): gate public sharing behind ..." | Re-trigger Greptile |
Greptile SummaryThis PR adds public share links for workspace files — write/admin members toggle sharing per-file via a new
Confidence Score: 3/5Two concrete defects should be addressed before merge: the share modal can silently overwrite a concurrent share state on Save, and the ON DELETE CASCADE on created_by will break all public share links when a user account is removed. The share modal initializes draftActive from potentially stale list-cache data; when the fresh server fetch resolves with a different isActive value, Save becomes active without any user action — clicking it would overwrite the actual server state with the stale value. Separately, the created_by foreign key uses ON DELETE CASCADE, meaning any user deletion silently removes all shares they ever created across all workspaces, which is likely to surprise both admins and external link holders. apps/sim/app/workspace/[workspaceId]/files/components/share-modal/share-modal.tsx (isDirty logic) and packages/db/schema.ts (created_by cascade behavior).
|
| Filename | Overview |
|---|---|
| apps/sim/app/workspace/[workspaceId]/files/components/share-modal/share-modal.tsx | Share modal — draftActive is initialized from stale initialShare and compared against freshly-loaded share, so isDirty can flip true without user input, enabling an accidental silent write. |
| packages/db/schema.ts | New public_share table is well-indexed and polymorphic; the createdBy ON DELETE CASCADE will silently break existing share links when the creating user is removed, which is likely unintentional. |
| apps/sim/app/api/files/public/[token]/content/route.ts | Unauthenticated byte-serving route; share validity is properly checked; loadServableDocArtifact serves prebuilt artifacts without compiling; no rate limiting is applied. |
| apps/sim/app/api/workspaces/[id]/files/[fileId]/share/route.ts | Auth and permission gating (401/403 checks) are correct; write-gated PUT and read-gated GET match the spec; audit events recorded on both enable and disable. |
| apps/sim/lib/public-shares/share-manager.ts | Core share CRUD — clean upsert with stable token semantics; resolveActiveShareByToken correctly gates on isActive + deletedAt; generateShortId uses crypto.getRandomValues so entropy is adequate. |
| apps/sim/app/f/[token]/public-file-view.tsx | FileContentSourceProvider seam correctly replaces workspace serve URLs with the token-scoped public endpoint; synthetic WorkspaceFileRecord is cleanly built; header includes noindex robots and provenance. |
| apps/sim/hooks/use-file-content-source.tsx | New context seam cleanly abstracts the URL construction; workspaceFileContentSource is the unchanged default so existing callers are unaffected. |
| apps/sim/hooks/queries/workspace-files.ts | fetchWorkspaceFileContent and fetchWorkspaceFileBinary now accept a pre-built URL from the content source, removing hardcoded workspace paths; backwards-compatible change. |
| apps/sim/app/workspace/[workspaceId]/files/files.tsx | ShareModal is wired into the context menu and toolbar correctly; shareModal is rendered in each conditional branch (selectedFile view and main list view) separately, not duplicated simultaneously. |
| apps/sim/lib/copilot/tools/server/files/doc-compile.ts | loadServableDocArtifact cleanly detects pre-compiled binaries via magic bytes and never triggers E2B compilation on the public path. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant U as User (Writer)
participant FM as FileViewer / ShareModal
participant SA as PUT /workspaces/[id]/files/[fileId]/share
participant SM as ShareManager (DB)
participant PU as Public Visitor
participant PA as GET /files/public/[token]
participant PC as GET /files/public/[token]/content
U->>FM: Toggle share ON → Save
FM->>SA: "PUT { isActive: true }"
SA->>SM: upsertFileShare()
SM-->>SA: "{ token, url, isActive: true }"
SA-->>FM: "200 { share }"
FM-->>U: Toast with link (copy action)
PU->>PA: "GET /api/files/public/{token}"
PA->>SM: resolveActiveShareByToken(token)
SM-->>PA: "{ file, workspaceName, ownerName }"
PA-->>PU: "{ name, type, size, workspaceName, ownerName }"
PU->>PC: "GET /api/files/public/{token}/content"
PC->>SM: resolveActiveShareByToken(token)
SM-->>PC: "{ file }"
PC-->>PU: file bytes (or compiled artifact)
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant U as User (Writer)
participant FM as FileViewer / ShareModal
participant SA as PUT /workspaces/[id]/files/[fileId]/share
participant SM as ShareManager (DB)
participant PU as Public Visitor
participant PA as GET /files/public/[token]
participant PC as GET /files/public/[token]/content
U->>FM: Toggle share ON → Save
FM->>SA: "PUT { isActive: true }"
SA->>SM: upsertFileShare()
SM-->>SA: "{ token, url, isActive: true }"
SA-->>FM: "200 { share }"
FM-->>U: Toast with link (copy action)
PU->>PA: "GET /api/files/public/{token}"
PA->>SM: resolveActiveShareByToken(token)
SM-->>PA: "{ file, workspaceName, ownerName }"
PA-->>PU: "{ name, type, size, workspaceName, ownerName }"
PU->>PC: "GET /api/files/public/{token}/content"
PC->>SM: resolveActiveShareByToken(token)
SM-->>PC: "{ file }"
PC-->>PU: file bytes (or compiled artifact)
Reviews (2): Last reviewed commit: "Merge remote-tracking branch 'origin/sta..." | Re-trigger Greptile
|
Fixed the share-modal init bug Greptile flagged: @greptile review |
…FK, soft-delete filter, download anchor
|
Thanks for the reviews — addressed all findings: Fixed in this PR
Acknowledged, not changing
@greptile review |
|
Fixed the public-CSV import toast (Greptile P1): threaded a @greptile review |
|
Addressed the synthetic-record staleness: the page now threads the file's real @greptile review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f2fe99e. Configure here.
…no compiled artifact
|
Two Bugbot items:
@greptile review |
|
@greptile review |
|
@greptile review |

Summary
/f/{token}; default private, write/admin onlypublic_sharetable (resourceType/resourceId, token, isActive) — shaped for future folder sharing + password/email gating (reserved columns)/f/{token}reuses the in-appFileViewervia a content-source seam +readOnlymode, so images/PDF/markdown/code/docx/pptx/xlsx all render (generated docs load their prebuilt compiled artifact, never compile on the public path)noindexType of Change
Testing
Tested manually.
bun run lint,check:api-validation:strict,check:migrations origin/staging(backward-compatible — additive CREATE TABLE), and route tests (share auth gates + public 404/no-leak) all pass.Checklist