Skip to content

Commit b26996c

Browse files
committed
fix(project-files): enforce public delivery and share policy safety
1 parent 8e82431 commit b26996c

10 files changed

Lines changed: 190 additions & 67 deletions

File tree

‎apps/sim/app/api/files/public/[token]/inline/route.ts‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { createLogger } from '@sim/logger'
2-
import { type NextRequest, NextResponse } from 'next/server'
2+
import type { NextRequest } from 'next/server'
33
import { getPublicInlineFileContract } from '@/lib/api/contracts/public-shares'
44
import { parseRequest } from '@/lib/api/server'
55
import { getClientIp } from '@/lib/core/utils/request'
@@ -33,16 +33,6 @@ export const GET = withRouteHandler(
3333
credential: await publicFileShareCredential(request.cookies.getAll(), getClientIp(request)),
3434
})
3535
if (!auth.authorized) return publicFileAuthDenied(auth)
36-
if (request.method === 'HEAD') {
37-
return new NextResponse(null, {
38-
status: 405,
39-
headers: {
40-
Allow: 'GET',
41-
'Cache-Control': FILE_CACHE_CONTROL.noStore,
42-
'X-Content-Type-Options': 'nosniff',
43-
},
44-
})
45-
}
4636
const result = await readPublicFileShareInline({
4737
grant: auth.grant,
4838
...parsed.data.query,

‎apps/sim/lib/projects/files/__integration__/content-sharing.integration.ts‎

Lines changed: 114 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { mkdtempSync } from 'node:fs'
22
import { mkdir, rm, writeFile } from 'node:fs/promises'
33
import { tmpdir } from 'node:os'
44
import { dirname, join } from 'node:path'
5-
import { db } from '@sim/db'
5+
import { db, runOutsideTransactionContext } from '@sim/db'
66
import {
77
folder,
88
idempotencyKey,
@@ -17,6 +17,7 @@ import {
1717
subscription,
1818
user,
1919
workspace,
20+
workspaceFileSecretProvenance,
2021
workspaceFiles,
2122
} from '@sim/db/schema'
2223
import { deleteWorkspaceFixture, insertWorkspaceFixture } from '@sim/db/testing/workspace-fixtures'
@@ -38,6 +39,7 @@ import {
3839
createProjectFile,
3940
updateProjectFileContent,
4041
} from '@/lib/projects/files/application/content'
42+
import * as shareManager from '@/lib/public-shares/manager'
4143
import * as storage from '@/lib/uploads/core/storage-service'
4244

4345
vi.mock('@/lib/core/config/feature-flags', () => featureFlagsMock)
@@ -139,6 +141,84 @@ function createInput(projectId: string, content = 'Project architecture') {
139141
}
140142

141143
describe('Public file shares against PostgreSQL and private storage', () => {
144+
check(
145+
'public delivery rejects unknown and secret-bearing provenance before storage',
146+
async () => {
147+
const { updateProjectFileShare } = await import('@/lib/projects/files/application/shares')
148+
const { authorizePublicFileShare, readPublicFileShareContent } = await import(
149+
'@/lib/public-shares/application'
150+
)
151+
const f = await fixture()
152+
for (const secretProvenance of [
153+
{ status: 'unknown' as const },
154+
{
155+
status: 'exact' as const,
156+
entries: [{ encryptedValue: 'private-fixture', sourceUserId: f.editorId }],
157+
},
158+
]) {
159+
const { file } = await createProjectFile.execute({
160+
principal: f.principal,
161+
input: {
162+
...createInput(f.projectId),
163+
name: `${generateId()}.md`,
164+
secretProvenance,
165+
},
166+
})
167+
const { share } = await updateProjectFileShare.execute({
168+
principal: f.principal,
169+
input: {
170+
projectId: f.projectId,
171+
fileId: file.id,
172+
isActive: true,
173+
},
174+
})
175+
const access = await authorizePublicFileShare({
176+
token: share.token,
177+
credential: { method: 'GET' },
178+
})
179+
if (!access.authorized) throw new Error('Public fixture rejected')
180+
const download = vi.spyOn(storage, 'downloadFile')
181+
await expect(readPublicFileShareContent({ grant: access.grant })).rejects.toMatchObject({
182+
code: 'forbidden',
183+
})
184+
expect(download).not.toHaveBeenCalled()
185+
download.mockRestore()
186+
}
187+
}
188+
)
189+
190+
check('non-SSO shares do not consult an unavailable session service', async () => {
191+
const { updateProjectFileShare } = await import('@/lib/projects/files/application/shares')
192+
const { authorizePublicFileShare } = await import('@/lib/public-shares/application')
193+
const f = await fixture()
194+
const { file } = await createProjectFile.execute({
195+
principal: f.principal,
196+
input: createInput(f.projectId),
197+
})
198+
const resolveSessionPrincipal = vi
199+
.fn()
200+
.mockRejectedValue(new Error('Session service unavailable'))
201+
for (const authType of ['public', 'password', 'email'] as const) {
202+
const { share } = await updateProjectFileShare.execute({
203+
principal: f.principal,
204+
input: {
205+
projectId: f.projectId,
206+
fileId: file.id,
207+
isActive: true,
208+
authType,
209+
password: 'fixture-password',
210+
allowedEmails: ['reader@shared.invalid'],
211+
},
212+
})
213+
const result = await authorizePublicFileShare({
214+
token: share.token,
215+
credential: { method: 'GET', resolveSessionPrincipal },
216+
})
217+
expect(result.authorized).toBe(authType === 'public')
218+
}
219+
expect(resolveSessionPrincipal).not.toHaveBeenCalled()
220+
})
221+
142222
check(
143223
'public share configuration requires Project write and the canonical organization sharing policy',
144224
async () => {
@@ -274,6 +354,21 @@ describe('Public file shares against PostgreSQL and private storage', () => {
274354
input: { ...input, authType: 'password', password: 'a-valid-share-password' },
275355
})
276356
expect(allowed.share.authType).toBe('password')
357+
const upsert = shareManager.upsertOwnedFileShare
358+
const interleave = vi
359+
.spyOn(shareManager, 'upsertOwnedFileShare')
360+
.mockImplementationOnce(async (...args) => {
361+
await runOutsideTransactionContext(() =>
362+
db
363+
.update(publicShare)
364+
.set({ authType: 'public', isActive: false })
365+
.where(eq(publicShare.id, allowed.share.id))
366+
)
367+
return upsert(...args)
368+
})
369+
const reenabled = await updateProjectFileShare.execute({ principal: f.principal, input })
370+
expect(reenabled.share).toMatchObject({ authType: 'password', isActive: true })
371+
interleave.mockRestore()
277372
} finally {
278373
await deleteWorkspaceFixture(db, eq(workspace.id, secondId))
279374
}
@@ -467,6 +562,17 @@ describe('Public file shares against PostgreSQL and private storage', () => {
467562
await requestPublicFileShareOtp({ token: share.token, email: 'outsider@shared.invalid' })
468563
).deliver()
469564
expect(await otpStore.getOTP('file', share.id, 'outsider@shared.invalid')).toBeNull()
565+
const failure = { authorized: false, status: 400, error: 'Invalid verification code' }
566+
expect(
567+
await verifyPublicFileShareOtp({
568+
token: share.token,
569+
email: 'outsider@shared.invalid',
570+
otp: '000000',
571+
})
572+
).toEqual(failure)
573+
expect(await verifyPublicFileShareOtp({ token: share.token, email, otp: '000000' })).toEqual(
574+
failure
575+
)
470576
await (await requestPublicFileShareOtp({ token: share.token, email })).deliver()
471577
const stored = await otpStore.getOTP('file', share.id, email)
472578
if (!stored) throw new Error('Allowed email received no code')
@@ -613,7 +719,7 @@ describe('Public file shares against PostgreSQL and private storage', () => {
613719
'@/lib/public-shares/application'
614720
)
615721
const { encryptSecret } = await import('@/lib/core/security/encryption')
616-
for (const change of ['disabled', 'password', 'archived'] as const) {
722+
for (const change of ['disabled', 'password', 'archived', 'provenance'] as const) {
617723
const f = await fixture()
618724
const { file } = await createProjectFile.execute({
619725
principal: f.principal,
@@ -648,6 +754,11 @@ describe('Public file shares against PostgreSQL and private storage', () => {
648754
.update(publicShare)
649755
.set({ password: (await encryptSecret('replacement-password')).encrypted })
650756
.where(eq(publicShare.id, share.id))
757+
if (change === 'provenance')
758+
await db
759+
.update(workspaceFileSecretProvenance)
760+
.set({ status: 'unknown' })
761+
.where(eq(workspaceFileSecretProvenance.fileId, file.id))
651762
if (change === 'archived')
652763
await db
653764
.update(workspaceFiles)
@@ -657,7 +768,7 @@ describe('Public file shares against PostgreSQL and private storage', () => {
657768
return result
658769
})
659770
await expect(readPublicFileShareContent({ grant: access.grant })).rejects.toMatchObject({
660-
code: 'not_found',
771+
code: change === 'provenance' ? 'forbidden' : 'not_found',
661772
})
662773
spy.mockRestore()
663774
}

‎apps/sim/lib/projects/files/application/shares.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ export const updateProjectFileShare = defineAuthorizedProjectFileUseCase<
117117
return {
118118
share: await upsertOwnedFileShare(tx, context.owner, {
119119
...input,
120+
authType: effectiveAuthType,
120121
userId: requirePrincipalSubjectUserId(principal),
121122
}),
122123
}

‎apps/sim/lib/public-shares/api/credentials.ts‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,19 @@ export async function publicFileShareCredential(
66
cookies: readonly { name: string; value: string }[],
77
clientIp?: string | null
88
) {
9-
const session = await getSession()
10-
let sessionPrincipal: SessionPrincipal | undefined
11-
if (session?.user?.id) {
12-
const sessionId = session.session?.id
13-
if (!sessionId) throw new Error('Authenticated session is missing its session ID')
14-
sessionPrincipal = { kind: 'session', userId: session.user.id, sessionId }
15-
}
169
return {
1710
method: 'GET' as const,
1811
cookies: Object.fromEntries(cookies.map(({ name, value }) => [name, value])),
19-
sessionPrincipal,
12+
async resolveSessionPrincipal() {
13+
const session = await getSession()
14+
let sessionPrincipal: SessionPrincipal | undefined
15+
if (session?.user?.id) {
16+
const sessionId = session.session?.id
17+
if (!sessionId) throw new Error('Authenticated session is missing its session ID')
18+
sessionPrincipal = { kind: 'session', userId: session.user.id, sessionId }
19+
}
20+
return sessionPrincipal
21+
},
2022
clientIp,
2123
}
2224
}

‎apps/sim/lib/public-shares/application/authorization.ts‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ interface PublicFileCredential {
2828
password?: string
2929
email?: string
3030
cookies?: Readonly<Record<string, string>>
31+
resolveSessionPrincipal?: () => Promise<Principal | undefined>
3132
sessionPrincipal?: Principal
3233
clientIp?: string | null
3334
}
@@ -151,14 +152,16 @@ export async function authorizePublicFileShare({
151152
> {
152153
try {
153154
if (!/^[A-Za-z0-9_-]{16,64}$/.test(token)) unavailable()
154-
const initial = await db.transaction(async (tx) => {
155-
const snapshot = await loadSnapshot(tx, token)
156-
const session =
157-
snapshot.share.authType === 'sso' && credential.sessionPrincipal?.kind === 'session'
158-
? await loadSession(tx, credential.sessionPrincipal.userId)
159-
: undefined
160-
return { snapshot, session }
161-
})
155+
const snapshot = await db.transaction((tx) => loadSnapshot(tx, token))
156+
const principal =
157+
snapshot.share.authType === 'sso'
158+
? (credential.sessionPrincipal ?? (await credential.resolveSessionPrincipal?.()))
159+
: undefined
160+
const session =
161+
principal?.kind === 'session'
162+
? await db.transaction((tx) => loadSession(tx, principal.userId))
163+
: undefined
164+
const initial = { snapshot, session }
162165
const result = await validateDeploymentCredentials(
163166
generateShortId(),
164167
initial.snapshot.share,

‎apps/sim/lib/public-shares/application/credentials.ts‎

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ const SSO_RESOURCE_RATE_LIMIT: TokenBucketConfig = {
2828
refillRate: 100,
2929
refillIntervalMs: 15 * 60_000,
3030
}
31-
const OTP_LOCKED_MESSAGE = 'Too many failed attempts. Please request a new code.'
31+
const OTP_FAILURE = { authorized: false as const, error: 'Invalid verification code', status: 400 }
3232

3333
/** Exchanges a verified password for a cookie bound to the share's current password slot. */
3434
export async function authenticatePublicFileSharePassword({
@@ -113,27 +113,18 @@ export async function verifyPublicFileShareOtp({
113113
const allowed = await challenge.withCurrentPolicy(({ share }) =>
114114
isEmailAllowed(normalizedEmail, share.allowedEmails)
115115
)
116-
if (!allowed) return { authorized: false as const, error: 'Email not authorized', status: 403 }
116+
if (!allowed) return OTP_FAILURE
117117
const value = await getOTP('file', challenge.shareId, normalizedEmail)
118118
await challenge.withCurrentPolicy(() => undefined)
119-
if (!value)
120-
return {
121-
authorized: false as const,
122-
error: 'No verification code found, request a new one',
123-
status: 400,
124-
}
119+
if (!value) return OTP_FAILURE
125120
const stored = decodeOTPValue(value)
126121
if (stored.attempts >= MAX_OTP_ATTEMPTS) {
127122
await deleteOTP('file', challenge.shareId, normalizedEmail)
128-
return { authorized: false as const, error: OTP_LOCKED_MESSAGE, status: 429 }
123+
return OTP_FAILURE
129124
}
130125
if (!safeCompare(stored.otp, otp)) {
131-
const result = await incrementOTPAttempts('file', challenge.shareId, normalizedEmail, value)
132-
return {
133-
authorized: false as const,
134-
error: result === 'locked' ? OTP_LOCKED_MESSAGE : 'Invalid verification code',
135-
status: result === 'locked' ? 429 : 400,
136-
}
126+
await incrementOTPAttempts('file', challenge.shareId, normalizedEmail, value)
127+
return OTP_FAILURE
137128
}
138129
await deleteOTP('file', challenge.shareId, normalizedEmail)
139130
const resource = await challenge.withCurrentPolicy(({ share }) => share)

‎apps/sim/lib/public-shares/application/owner-adapters.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { project, type WorkspaceFileRow, workspace } from '@sim/db/schema'
22
import { eq } from 'drizzle-orm'
33
import { OrchestrationError } from '@/lib/core/orchestration/types'
44
import type { DbTransaction } from '@/lib/db/types'
5-
import { lockProject, lockWorkspaceProject } from '@/lib/projects/membership'
5+
import { lockProjectShared, lockWorkspaceProject } from '@/lib/projects/membership'
66
import { requireProjectApiEnabled } from '@/lib/projects/rollout.server'
77
import {
88
type E2BDocFormat,
@@ -70,7 +70,7 @@ const adapters: FileOwnerAdapters<PublicFileOwnerAdapter> = {
7070
project: {
7171
async load(tx, entityId) {
7272
await requireProjectApiEnabled()
73-
await lockProject(tx, entityId)
73+
await lockProjectShared(tx, entityId)
7474
const [row] = await tx.select().from(project).where(eq(project.id, entityId)).for('share')
7575
if (!row || row.archivedAt) throw new OrchestrationError('not_found', 'File share not found')
7676
return {

0 commit comments

Comments
 (0)