Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 73 additions & 4 deletions supabase/functions/_backend/public/build/upload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@
// Use authenticated client for data queries - RLS will enforce access
const supabase = supabaseApikey(c, apikey.key)

// Get build request to verify ownership
// Get build request to verify ownership and that the upload window is still open
const { data: buildRequest, error: buildRequestError } = await supabase
.from('build_requests')
.select('app_id, owner_org, builder_job_id, upload_path')
.select('app_id, owner_org, builder_job_id, upload_path, upload_expires_at, status')
.eq('builder_job_id', jobId)
.single()

Expand All @@ -58,6 +58,38 @@
throw simpleError('not_found', 'Build request not found')
}

// Fail closed: only `pending` is the upload window (set in request.ts).
// After /build/start the row moves to running/terminal and must not accept more bytes.
if (!buildRequest.status || buildRequest.status !== 'pending') {
cloudlogErr({
requestId: c.get('requestId'),
message: 'TUS upload rejected for non-pending build status',
job_id: jobId,
status: buildRequest.status,
})
throw quickError(403, 'upload_not_allowed', 'Upload is not allowed for this build status')
}

if (!buildRequest.upload_expires_at) {
cloudlogErr({
requestId: c.get('requestId'),
message: 'TUS upload rejected: missing upload_expires_at',
job_id: jobId,
})
throw quickError(403, 'upload_expired', 'Upload window has expired')
}

const uploadExpiresAt = new Date(buildRequest.upload_expires_at)
if (Number.isNaN(uploadExpiresAt.getTime()) || uploadExpiresAt.getTime() <= Date.now()) {
cloudlogErr({
requestId: c.get('requestId'),
message: 'TUS upload rejected: upload window expired',
job_id: jobId,
upload_expires_at: buildRequest.upload_expires_at,
})
throw quickError(403, 'upload_expired', 'Upload window has expired')
}

// Check if user has permission to upload for this build (auth context set by middlewareKey)
if (!(await checkPermission(c, 'app.build_native', { appId: buildRequest.app_id }))) {
cloudlogErr({
Expand Down Expand Up @@ -115,7 +147,7 @@
upload_path: buildRequest.upload_path,
})

// Extract the path after /upload/:jobId/ and forward to builder
// Extract the path after /upload/:jobId/ and bind it to this job's upload resource
// Example: /build/upload/abc123/myfile.zip -> /upload/myfile.zip
// Example: /build/upload/abc123 -> /upload/
const requestUrl = c.req.raw.url
Expand Down Expand Up @@ -161,8 +193,45 @@
throw quickError(400, 'invalid_path', 'Invalid upload path')
}

const clientSuffix = decodedTusPath.replace(/^\/+|\/+$/g, '')

Check warning on line 196 in supabase/functions/_backend/public/build/upload.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Simplify this regular expression to reduce its runtime, as it has super-linear performance due to backtracking.

See more on https://sonarcloud.io/project/issues?id=Cap-go_capgo&issues=AaAMPmo241jXsmVC1LCt&open=AaAMPmo241jXsmVC1LCt&pullRequest=3091
const jobUploadResource = buildRequest.upload_path.split('/').filter(Boolean).at(-1)

Check warning on line 197 in supabase/functions/_backend/public/build/upload.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Prefer `.findLast(…)` over `.filter(…).at(-1)`.

See more on https://sonarcloud.io/project/issues?id=Cap-go_capgo&issues=AaAMPmo241jXsmVC1LCu&open=AaAMPmo241jXsmVC1LCu&pullRequest=3091
if (!jobUploadResource) {
throw simpleError('invalid_request', 'Invalid upload path format')
}

// POST creates this job's upload only. Never forward a client-chosen extra segment.
if (forwardMethod === 'POST' && clientSuffix) {
cloudlogErr({
requestId: c.get('requestId'),
message: 'Rejected POST with extra TUS path segment',
job_id: jobId,
original_path: originalPath,
upload_path: decodedTusPath,
})
throw quickError(400, 'invalid_path', 'Upload create path must not include a TUS resource id.')
}

// PATCH/HEAD/OPTIONS may send a Location suffix. Bind it to this job's upload_path
// (last segment or the full stored path). Anything else is another job's resource.
if (forwardMethod !== 'POST' && clientSuffix
&& clientSuffix !== jobUploadResource
&& clientSuffix !== buildRequest.upload_path) {
cloudlogErr({
requestId: c.get('requestId'),
message: 'Rejected TUS path that does not belong to this job',
job_id: jobId,
original_path: originalPath,
upload_path: decodedTusPath,
})
throw quickError(400, 'invalid_path', 'Upload path does not match this build request')
}

const boundTusPath = forwardMethod === 'POST'
? '/'
: `/${clientSuffix === buildRequest.upload_path ? buildRequest.upload_path : jobUploadResource}`

Check warning on line 231 in supabase/functions/_backend/public/build/upload.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Extract this nested ternary operation into an independent statement.

See more on https://sonarcloud.io/project/issues?id=Cap-go_capgo&issues=AaAMPmo241jXsmVC1LCv&open=AaAMPmo241jXsmVC1LCv&pullRequest=3091

const baseUploadUrl = new URL(`${builderUrl}/upload/`)
const resolvedTusUrl = new URL(`.${tusPath}`, baseUploadUrl)
const resolvedTusUrl = new URL(`.${boundTusPath}`, baseUploadUrl)
if (!resolvedTusUrl.pathname.startsWith(baseUploadUrl.pathname)) {
cloudlogErr({
requestId: c.get('requestId'),
Expand Down
172 changes: 158 additions & 14 deletions tests/build-upload-security.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,24 @@ describe('build upload proxy security', () => {
const appId = 'com.test.traversal.app'
const orgId = 'org-traversal'
const validUploadPath = `orgs/${orgId}/apps/${appId}/native-builds/file.zip`
const buildRequestQuery = {
data: {
app_id: appId,
owner_org: orgId,
builder_job_id: jobId,
upload_path: validUploadPath,
},
error: null,
const jobUploadResource = 'file.zip'
const futureExpiry = () => new Date(Date.now() + 60 * 60 * 1000).toISOString()
const defaultBuildRequest = {
app_id: appId,
owner_org: orgId,
builder_job_id: jobId,
upload_path: validUploadPath,
upload_expires_at: futureExpiry(),
status: 'pending',
}

const createQueryBuilder = () => ({
const createQueryBuilder = (data: Record<string, unknown> = defaultBuildRequest) => ({
select: vi.fn().mockReturnThis(),
eq: vi.fn().mockReturnThis(),
single: vi.fn().mockResolvedValue(buildRequestQuery),
single: vi.fn().mockResolvedValue({
data,
error: null,
}),
})
let queryBuilder: ReturnType<typeof createQueryBuilder>

Expand Down Expand Up @@ -138,12 +142,12 @@ describe('build upload proxy security', () => {
}))

try {
const context = fakeContext(`http://localhost/build/upload/${jobId}/artifact.zip`, 'PATCH')
const context = fakeContext(`http://localhost/build/upload/${jobId}/${jobUploadResource}`, 'PATCH')
const response = await tusProxy(context as any, jobId, { user_id: 'user-test', key: 'api-test' } as any)

expect(response.status).toBe(201)
expect(fetchMock).toHaveBeenCalledWith(
'https://builder.capgo.app/upload/artifact.zip',
`https://builder.capgo.app/upload/${jobUploadResource}`,
expect.anything(),
)
}
Expand Down Expand Up @@ -177,7 +181,7 @@ describe('build upload proxy security', () => {
}))

try {
const uploadUrl = `http://localhost/build/upload/${jobId}/artifact.zip`
const uploadUrl = `http://localhost/build/upload/${jobId}/${jobUploadResource}`
const apikey = { user_id: 'user-test', key: 'api-test' } as any
const firstPatchResponse = await tusProxy(fakeContext(uploadUrl, 'PATCH') as any, jobId, apikey)
const headResponse = await tusProxy(fakeContext(uploadUrl, 'HEAD') as any, jobId, apikey, 'HEAD')
Expand Down Expand Up @@ -210,7 +214,7 @@ describe('build upload proxy security', () => {
}))

try {
const context = fakeContext(`http://localhost/build/upload/${jobId}/artifact.zip`, 'HEAD')
const context = fakeContext(`http://localhost/build/upload/${jobId}/${jobUploadResource}`, 'HEAD')
const response = await tusProxy(context as any, jobId, { user_id: 'user-test', key: 'api-test' } as any, 'HEAD')
const [, init] = fetchMock.mock.calls[0] as [string, RequestInit]

Expand All @@ -224,4 +228,144 @@ describe('build upload proxy security', () => {
fetchMock.mockRestore()
}
})

it('rejects PATCH to a different TUS suffix than this job upload_path', async () => {
const fetchMock = vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response(null, { status: 204 }))

let error: HTTPException | undefined
try {
await tusProxy(
fakeContext(`http://localhost/build/upload/${jobId}/other-job-id`, 'PATCH') as any,
jobId,
{ user_id: 'user-test', key: 'api-test' } as any,
)
}
catch (err) {
expect(err).toBeInstanceOf(HTTPException)
if (!(err instanceof HTTPException)) {
throw err
}
error = err
}
finally {
const forwardedUrls = fetchMock.mock.calls.map(([url]) => String(url))
expect(forwardedUrls.some(url => url.includes('/upload/other-job-id'))).toBe(false)
expect(fetchMock).not.toHaveBeenCalled()
fetchMock.mockRestore()
}

if (!error) {
throw new Error('Expected tusProxy to reject with HTTPException')
}
expect(error.status).toBe(400)
expect(error.cause).toMatchObject({
error: 'invalid_path',
message: 'Upload path does not match this build request',
})
})

it('rejects expired upload_expires_at before forwarding', async () => {
queryBuilder = createQueryBuilder({
...defaultBuildRequest,
upload_expires_at: new Date(Date.now() - 60 * 1000).toISOString(),
})
mockSupabaseApikey.mockReturnValue({
from: vi.fn().mockReturnValue(queryBuilder),
})

const fetchMock = vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response(null, { status: 204 }))

let error: HTTPException | undefined
try {
await tusProxy(
fakeContext(`http://localhost/build/upload/${jobId}/${jobUploadResource}`, 'PATCH') as any,
jobId,
{ user_id: 'user-test', key: 'api-test' } as any,
)
}
catch (err) {
expect(err).toBeInstanceOf(HTTPException)
if (!(err instanceof HTTPException)) {
throw err
}
error = err
}
finally {
expect(fetchMock).not.toHaveBeenCalled()
fetchMock.mockRestore()
}

if (!error) {
throw new Error('Expected tusProxy to reject with HTTPException')
}
expect(error.status).toBe(403)
expect(error.cause).toMatchObject({
error: 'upload_expired',
message: 'Upload window has expired',
})
})

it('rejects non-pending terminal status before forwarding', async () => {
queryBuilder = createQueryBuilder({
...defaultBuildRequest,
status: 'failed',
})
mockSupabaseApikey.mockReturnValue({
from: vi.fn().mockReturnValue(queryBuilder),
})

const fetchMock = vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response(null, { status: 204 }))

let error: HTTPException | undefined
try {
await tusProxy(
fakeContext(`http://localhost/build/upload/${jobId}/${jobUploadResource}`, 'PATCH') as any,
jobId,
{ user_id: 'user-test', key: 'api-test' } as any,
)
}
catch (err) {
expect(err).toBeInstanceOf(HTTPException)
if (!(err instanceof HTTPException)) {
throw err
}
error = err
}
finally {
expect(fetchMock).not.toHaveBeenCalled()
fetchMock.mockRestore()
}

if (!error) {
throw new Error('Expected tusProxy to reject with HTTPException')
}
expect(error.status).toBe(403)
expect(error.cause).toMatchObject({
error: 'upload_not_allowed',
message: 'Upload is not allowed for this build status',
})
})

it('forwards valid POST without extra suffix to builder upload root', async () => {
const fetchMock = vi.spyOn(globalThis, 'fetch').mockResolvedValue(new Response(null, {
status: 201,
headers: {
Location: 'https://builder.capgo.app/upload/file.zip',
},
}))

try {
const context = fakeContext(`http://localhost/build/upload/${jobId}`, 'POST')
const response = await tusProxy(context as any, jobId, { user_id: 'user-test', key: 'api-test' } as any)

expect(response.status).toBe(201)
expect(fetchMock).toHaveBeenCalledWith(
'https://builder.capgo.app/upload/',
expect.anything(),
)
}
finally {
fetchMock.mockRestore()
}
})
})
Loading