feat(file): add Manage Sharing operation to the File block#5177
feat(file): add Manage Sharing operation to the File block#5177TheodoreSpeaks wants to merge 5 commits into
Conversation
Adds a new file_set_sharing operation to the File block (file_v5) that idempotently enables/disables a file's public share link and sets its access mode (public, password, email, SSO). The set_sharing route case reuses upsertFileShare, requires write/admin, gates enabling through the EE public-sharing policy, and records a share audit. Returns an empty url when set to private so a disabled link isn't handed back as a dead link.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview A new The Reviewed by Cursor Bugbot for commit 80bb8ed. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
@greptile review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryAdds a new Manage Sharing operation to the
Confidence Score: 5/5Safe to merge; all three issues raised in prior review rounds have been addressed in this revision. The new operation correctly gates enabling on the EE policy, resolves the same authType fallback used by upsertFileShare to prevent bypass, and performs the permission check before the file lookup to avoid an existence side-channel. Tool param visibility is correct across the board. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["POST /api/tools/file/manage\noperation: manage_sharing"] --> B["Validate body\nfileManageSharingBodySchema"]
B --> C{"workspaceId?"}
C -- No --> ERR1["400 workspaceId required"]
C -- Yes --> D["assertActiveWorkspaceAccess"]
D --> E["getUserEntityPermissions"]
E -- read --> ERR2["403 Insufficient permissions"]
E -- write/admin --> F["getWorkspaceFile(workspaceId, fileId)"]
F -- not found --> ERR3["404 File not found"]
F -- found --> G{"isActive?"}
G -- false --> I["upsertFileShare\nisActive: false"]
G -- true --> H["getShareForResource\nresolve authType fallback"]
H --> J["validatePublicFileSharing\nEE policy gate"]
J -- not allowed --> ERR4["403 Policy violation"]
J -- allowed --> I
I --> K["recordAudit\nFILE_SHARED / FILE_SHARE_DISABLED"]
K --> L{"share.isActive?"}
L -- Yes --> M["200 {share}"]
L -- No --> N["200 {share, url: ''}"]
%%{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"}}}%%
flowchart TD
A["POST /api/tools/file/manage\noperation: manage_sharing"] --> B["Validate body\nfileManageSharingBodySchema"]
B --> C{"workspaceId?"}
C -- No --> ERR1["400 workspaceId required"]
C -- Yes --> D["assertActiveWorkspaceAccess"]
D --> E["getUserEntityPermissions"]
E -- read --> ERR2["403 Insufficient permissions"]
E -- write/admin --> F["getWorkspaceFile(workspaceId, fileId)"]
F -- not found --> ERR3["404 File not found"]
F -- found --> G{"isActive?"}
G -- false --> I["upsertFileShare\nisActive: false"]
G -- true --> H["getShareForResource\nresolve authType fallback"]
H --> J["validatePublicFileSharing\nEE policy gate"]
J -- not allowed --> ERR4["403 Policy violation"]
J -- allowed --> I
I --> K["recordAudit\nFILE_SHARED / FILE_SHARE_DISABLED"]
K --> L{"share.isActive?"}
L -- Yes --> M["200 {share}"]
L -- No --> N["200 {share, url: ''}"]
Reviews (6): Last reviewed commit: "fix(file): require isActive in tool para..." | Re-trigger Greptile |
Greptile SummaryThis PR adds a Set File Sharing operation to the
Confidence Score: 3/5The block, tool, and contract changes are safe to merge; the route's set_sharing handler needs a fix before shipping to production. The set_sharing route validates the effective sharing auth type against the org's EE policy using 'public' as a hard-coded fallback when authType is absent. upsertFileShare independently computes the real effective auth type from the existing share row, so the two values can diverge: a user on a workspace that restricts sharing to 'public' only can re-enable a previously-disabled 'sso' share by omitting authType, bypassing the allowedFileShareAuthTypes gate. The rest of the PR — tool definition, block wiring, contract schema, and tests — is well-implemented and low-risk. apps/sim/app/api/tools/file/manage/route.ts — the set_sharing switch case needs the effective auth type resolved (from the existing share record) before calling validatePublicFileSharing.
|
| Filename | Overview |
|---|---|
| apps/sim/app/api/tools/file/manage/route.ts | Adds set_sharing case with write/admin guard, EE policy validation, and audit logging; two issues: policy check uses wrong fallback authType for existing shares (allowing auth-type bypass), and file existence is probed before the permission check. |
| apps/sim/tools/file/set-sharing.ts | New tool definition for file_set_sharing; well-structured with correct visibility settings and proper response transformation. |
| apps/sim/blocks/blocks/file.ts | Adds file_set_sharing operation to FileV5Block with correct conditional subblocks, params mapping, and output declarations; password/email/SSO conditional fields are wired correctly. |
| apps/sim/lib/api/contracts/tools/file.ts | New fileManageSetSharingBodySchema added to the union; field constraints (min lengths, max 200 emails, max 1024 password) look sensible. |
| apps/sim/blocks/blocks/file.test.ts | Good coverage of public/private/password/email visibility mappings and file-object resolution; missing a test case for the sso visibility path. |
| apps/sim/tools/registry.ts | Registers file_set_sharing tool in the correct alphabetical position. |
| apps/sim/blocks/blocks.test.ts | Adds file_set_sharing to the known-tools assertion; straightforward and correct. |
| apps/sim/tools/file/index.ts | Exports the new fileSetSharingTool following the existing barrel pattern. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Block as FileV5Block (file.ts)
participant Tool as fileSetSharingTool (set-sharing.ts)
participant Route as /api/tools/file/manage
participant Auth as checkInternalAuth
participant Perm as getUserEntityPermissions
participant Policy as validatePublicFileSharing
participant DB as upsertFileShare (share-manager)
participant Audit as recordAudit
Block->>Tool: "buildParams() → { fileId, isActive, authType, password, allowedEmails }"
Tool->>Route: "POST { operation: 'set_sharing', ... }"
Route->>Auth: checkInternalAuth(request)
Auth-->>Route: "{ userId }"
Route->>Route: assertActiveWorkspaceAccess(workspaceId, userId)
Route->>DB: getWorkspaceFile(workspaceId, fileId)
DB-->>Route: "file | null (404 if missing)"
Route->>Perm: getUserEntityPermissions(userId, 'workspace', workspaceId)
Perm-->>Route: "'admin' | 'write' | 'read' (403 if read)"
alt "isActive = true"
Route->>Policy: validatePublicFileSharing(userId, workspaceId, authType ?? 'public')
Note over Route,Policy: uses 'public' fallback, not existing share's authType
Policy-->>Route: throws PublicFileSharingNotAllowedError on policy violation
end
Route->>DB: "upsertFileShare({ workspaceId, fileId, isActive, authType, ... })"
Note over DB: finalAuthType = authType ?? existing.authType ?? 'public'
DB-->>Route: ShareRecord
Route->>Audit: "recordAudit(FILE_SHARED | FILE_SHARE_DISABLED) fire-and-forget"
Route-->>Tool: "{ success: true, data: { share } }"
Tool-->>Block: "{ url, isActive, authType, hasPassword, allowedEmails }"
%%{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 Block as FileV5Block (file.ts)
participant Tool as fileSetSharingTool (set-sharing.ts)
participant Route as /api/tools/file/manage
participant Auth as checkInternalAuth
participant Perm as getUserEntityPermissions
participant Policy as validatePublicFileSharing
participant DB as upsertFileShare (share-manager)
participant Audit as recordAudit
Block->>Tool: "buildParams() → { fileId, isActive, authType, password, allowedEmails }"
Tool->>Route: "POST { operation: 'set_sharing', ... }"
Route->>Auth: checkInternalAuth(request)
Auth-->>Route: "{ userId }"
Route->>Route: assertActiveWorkspaceAccess(workspaceId, userId)
Route->>DB: getWorkspaceFile(workspaceId, fileId)
DB-->>Route: "file | null (404 if missing)"
Route->>Perm: getUserEntityPermissions(userId, 'workspace', workspaceId)
Perm-->>Route: "'admin' | 'write' | 'read' (403 if read)"
alt "isActive = true"
Route->>Policy: validatePublicFileSharing(userId, workspaceId, authType ?? 'public')
Note over Route,Policy: uses 'public' fallback, not existing share's authType
Policy-->>Route: throws PublicFileSharingNotAllowedError on policy violation
end
Route->>DB: "upsertFileShare({ workspaceId, fileId, isActive, authType, ... })"
Note over DB: finalAuthType = authType ?? existing.authType ?? 'public'
DB-->>Route: ShareRecord
Route->>Audit: "recordAudit(FILE_SHARED | FILE_SHARE_DISABLED) fire-and-forget"
Route-->>Tool: "{ success: true, data: { share } }"
Tool-->>Block: "{ url, isActive, authType, hasPassword, allowedEmails }"
Reviews (1): Last reviewed commit: "feat(file): add Set File Sharing operati..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
… params, policy gate + perm-check ordering Addresses review findings: - Make isActive explicit/required so a bare call no longer silently enables a public link - Expose isActive/authType/allowedEmails as user-or-llm so agents can disable/configure shares (password stays user-only) - Resolve authType from the existing share before the EE policy gate to close a re-enable bypass - Run the write/admin permission check before the file lookup to remove a file-existence side channel
|
@greptile review |
|
Addressed the review findings in
|
Renames the file_set_sharing operation to file_manage_sharing (route literal manage_sharing, tool Manage Sharing) across the contract, route, tool, block, registry, and tests.
TheodoreSpeaks
changed the title
|
Renamed the operation to Manage Sharing ( |
|
@greptile review |
Prior commit's lint-staged dropped the barrel re-export update, leaving index.ts importing the deleted set-sharing module and breaking the block registry check. Point the barrel at manage-sharing.
|
@greptile review |
…s in manage sharing - FileManageSharingParams.isActive is now required, matching the tool param and contract (no compile-time gap that 400s at runtime) - manage_sharing rejects multiple canonical file IDs instead of silently sharing only the first, matching decompress
|
@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 80bb8ed. Configure here.
| fileId = (file?.id as string) ?? '' | ||
| } | ||
| if (!fileId) { | ||
| throw new Error('Could not determine the file to share') |
There was a problem hiding this comment.
Picker files lack share ID
Medium Severity
Manage Sharing resolves the target file only from shareInput.id, but the basic file-upload control stores workspace picks as { name, path, key, size, type } without an id. Those runs fail with “Could not determine the file to share,” while Read/Compress accept the same shape via fileInput and the manage route only accepts fileId.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 80bb8ed. Configure here.
1 participant
Summary
file_v5) that idempotently enables/disables a file's public share link and sets its access mode (public, password, email, SSO)set_sharingcase on/api/tools/file/managereuses the existingupsertFileSharedomain logic, requires write/admin, gates enabling through the EE public-sharing policy, and records a FILE_SHARED/FILE_SHARE_DISABLED auditfile_set_sharingtool + contract schema; wired into the block via a Visibility dropdown with conditional password / allowed-emails fieldsurlwhen set to private so a disabled link isn't handed back as a dead linkType of Change
Testing
bun run lint,bun run check:api-validation:strict,tsc --noEmit, and block tests all passChecklist