feat(files): public share links for workspace files#5130
feat(files): public share links for workspace files#5130TheodoreSpeaks wants to merge 9 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview Backend: New Frontend: Share modal and Share entry points in the files UI; React Query hooks for share state. Public page reuses Reviewed by Cursor Bugbot for commit f2fe99e. 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 file sharing to the workspace, introducing a
Confidence Score: 5/5The change is safe to merge — all previously raised issues have been resolved and no new defects were found. All the concerns raised in the prior review round (draftActive stale-initialShare bug, createdBy CASCADE, download anchor reliability, deletedAt filter, rate limiting on the content route, and the CSV import toast firing on unauthenticated public paths) are addressed in this iteration. Token entropy is strong (~126 bits). The FileContentSource seam cleanly isolates the public URL substitution without touching existing workspace rendering paths. The migration is additive with no destructive changes. No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant U as Workspace User
participant FE as Files UI
participant ShareAPI as PUT /workspaces/.../share
participant ShareMgr as ShareManager
participant DB as public_share table
participant PubPage as /f/[token] SSR
participant PubAPI as GET /api/files/public/[token]/content
participant RateLimit as Rate Limiter
U->>FE: Open ShareModal, toggle Public, Save
FE->>ShareAPI: PUT isActive:true
ShareAPI->>ShareMgr: upsertFileShare()
ShareMgr->>DB: INSERT ON CONFLICT UPDATE isActive
DB-->>ShareMgr: row with stable token
ShareMgr-->>ShareAPI: ShareRecord + URL
ShareAPI-->>FE: 200 share
FE->>U: Toast with Copy action
U->>PubPage: GET /f/abc123
PubPage->>ShareMgr: resolveActiveShareByToken(token)
ShareMgr->>DB: JOIN workspaceFiles WHERE token AND isActive AND deletedAt IS NULL
DB-->>ShareMgr: file metadata + provenance
ShareMgr-->>PubPage: ResolvedShare
PubPage->>U: Render PublicFileView
U->>PubAPI: GET /api/files/public/token/content
PubAPI->>RateLimit: enforcePublicFileRateLimit
alt rate limit exceeded
RateLimit-->>U: 429 Retry-After
else allowed
PubAPI->>ShareMgr: resolveActiveShareByToken
ShareMgr-->>PubAPI: file record
PubAPI->>PubAPI: loadServableDocArtifact
PubAPI-->>U: bytes Cache-Control private no-cache
end
%%{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 Workspace User
participant FE as Files UI
participant ShareAPI as PUT /workspaces/.../share
participant ShareMgr as ShareManager
participant DB as public_share table
participant PubPage as /f/[token] SSR
participant PubAPI as GET /api/files/public/[token]/content
participant RateLimit as Rate Limiter
U->>FE: Open ShareModal, toggle Public, Save
FE->>ShareAPI: PUT isActive:true
ShareAPI->>ShareMgr: upsertFileShare()
ShareMgr->>DB: INSERT ON CONFLICT UPDATE isActive
DB-->>ShareMgr: row with stable token
ShareMgr-->>ShareAPI: ShareRecord + URL
ShareAPI-->>FE: 200 share
FE->>U: Toast with Copy action
U->>PubPage: GET /f/abc123
PubPage->>ShareMgr: resolveActiveShareByToken(token)
ShareMgr->>DB: JOIN workspaceFiles WHERE token AND isActive AND deletedAt IS NULL
DB-->>ShareMgr: file metadata + provenance
ShareMgr-->>PubPage: ResolvedShare
PubPage->>U: Render PublicFileView
U->>PubAPI: GET /api/files/public/token/content
PubAPI->>RateLimit: enforcePublicFileRateLimit
alt rate limit exceeded
RateLimit-->>U: 429 Retry-After
else allowed
PubAPI->>ShareMgr: resolveActiveShareByToken
ShareMgr-->>PubAPI: file record
PubAPI->>PubAPI: loadServableDocArtifact
PubAPI-->>U: bytes Cache-Control private no-cache
end
Reviews (5): Last reviewed commit: "refactor(files): drive CSV preview impor..." | 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.
| ? await loadServableDocArtifact(file.workspaceId, raw, file.originalName) | ||
| : null | ||
| const buffer = artifact?.buffer ?? raw | ||
| const contentType = artifact?.contentType ?? file.contentType |
There was a problem hiding this comment.
Public route serves doc source
Medium Severity
When loadServableDocArtifact finds no compiled artifact for a generated office/PDF file, the public content handler falls back to raw stored bytes while keeping the file’s binary contentType. Authenticated serve returns 409 until the artifact exists, so shared generated docs can download or preview as corrupt source instead of the compiled document.
Reviewed by Cursor Bugbot for commit f2fe99e. Configure here.
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