Skip to content

Commit 75f229f

Browse files
fix(file): harden set_sharing — explicit isActive, agent-controllable 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
1 parent 0d59177 commit 75f229f

3 files changed

Lines changed: 28 additions & 17 deletions

File tree

apps/sim/app/api/tools/file/manage/route.ts

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,11 @@ import { generateRequestId } from '@/lib/core/utils/request'
1515
import { ensureAbsoluteUrl } from '@/lib/core/utils/urls'
1616
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1717
import { isSupportedFileType, parseBuffer } from '@/lib/file-parsers'
18-
import { ShareValidationError, upsertFileShare } from '@/lib/public-shares/share-manager'
18+
import {
19+
getShareForResource,
20+
ShareValidationError,
21+
upsertFileShare,
22+
} from '@/lib/public-shares/share-manager'
1923
import { ensureWorkspaceFileFolderPath } from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager'
2024
import {
2125
fetchWorkspaceFileBuffer,
@@ -575,16 +579,10 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
575579
case 'set_sharing': {
576580
const { fileId, isActive, authType, password, allowedEmails } = body
577581

578-
const file = await getWorkspaceFile(workspaceId, fileId)
579-
if (!file) {
580-
return NextResponse.json(
581-
{ success: false, error: `File not found: "${fileId}"` },
582-
{ status: 404 }
583-
)
584-
}
585-
586-
// Publishing a file is more sensitive than the other mutating ops, so it
587-
// requires write/admin (not just workspace access) to match the share route.
582+
// Check permission before probing file existence so a read-only caller
583+
// can't distinguish 404 from 403 as a file-existence side channel.
584+
// Publishing is more sensitive than the other mutating ops, so it
585+
// requires write/admin (not just workspace access) like the share route.
588586
const permission = await getUserEntityPermissions(userId, 'workspace', workspaceId)
589587
if (permission !== 'admin' && permission !== 'write') {
590588
return NextResponse.json(
@@ -593,11 +591,24 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
593591
)
594592
}
595593

594+
const file = await getWorkspaceFile(workspaceId, fileId)
595+
if (!file) {
596+
return NextResponse.json(
597+
{ success: false, error: `File not found: "${fileId}"` },
598+
{ status: 404 }
599+
)
600+
}
601+
596602
// Enabling a share is gated by the org's access-control policy; disabling
597603
// is always allowed so users can un-share after the policy is turned on.
598604
if (isActive) {
605+
// Resolve the auth type the same way upsertFileShare will (falling back
606+
// to the existing share's type) so the policy gate can't be bypassed by
607+
// re-enabling a pre-existing restricted share without an explicit authType.
608+
const existingShare = await getShareForResource('file', fileId)
609+
const resolvedAuthType = authType ?? existingShare?.authType ?? 'public'
599610
try {
600-
await validatePublicFileSharing(userId, workspaceId, authType ?? 'public')
611+
await validatePublicFileSharing(userId, workspaceId, resolvedAuthType)
601612
} catch (error) {
602613
if (error instanceof PublicFileSharingNotAllowedError) {
603614
return NextResponse.json({ success: false, error: error.message }, { status: 403 })

apps/sim/lib/api/contracts/tools/file.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export const fileManageSetSharingBodySchema = z.object({
4747
operation: z.literal('set_sharing'),
4848
workspaceId: z.string().min(1).optional(),
4949
fileId: z.string().min(1, 'fileId is required for set_sharing operation'),
50-
isActive: z.boolean().optional().default(true),
50+
isActive: z.boolean({ error: 'isActive is required for set_sharing operation' }),
5151
authType: shareAuthTypeSchema.optional(),
5252
password: z.string().min(1).max(1024).optional(),
5353
allowedEmails: z.array(z.string().min(1)).max(200).optional(),

apps/sim/tools/file/set-sharing.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,14 @@ export const fileSetSharingTool: ToolConfig<FileSetSharingParams, ToolResponse>
2727
},
2828
isActive: {
2929
type: 'boolean',
30-
required: false,
31-
visibility: 'user-only',
30+
required: true,
31+
visibility: 'user-or-llm',
3232
description: 'Whether the public link is enabled. Set to false to make the file private.',
3333
},
3434
authType: {
3535
type: 'string',
3636
required: false,
37-
visibility: 'user-only',
37+
visibility: 'user-or-llm',
3838
description:
3939
'Access mode for the link: "public", "password", "email", or "sso". Defaults to "public".',
4040
},
@@ -47,7 +47,7 @@ export const fileSetSharingTool: ToolConfig<FileSetSharingParams, ToolResponse>
4747
allowedEmails: {
4848
type: 'array',
4949
required: false,
50-
visibility: 'user-only',
50+
visibility: 'user-or-llm',
5151
description:
5252
'Allowed emails or "@domain" patterns. Required when authType is "email" or "sso".',
5353
},

0 commit comments

Comments
 (0)