diff --git a/service/src/service/file-authorization.test.ts b/service/src/service/file-authorization.test.ts index 4d98c59f..fa60d4c8 100644 --- a/service/src/service/file-authorization.test.ts +++ b/service/src/service/file-authorization.test.ts @@ -304,6 +304,106 @@ describe('resolveOutputBucketSessionKey', () => { }); describe('authorizeRequestedFiles', () => { + test('coalesces repeated destinations while preserving distinct names for one object', async () => { + const req = request({ tenantId: TENANT_ID, userId: USER_ID }); + const sessionKey = resolveSessionKey(req, { kind: 'user', id: USER_ID }); + const alias = validFile({ name: 'extract/Transport_Phenomena/appendix-a1b2c3.pdf' }); + const inherited = { ...alias, name: 'extract/Transport Phenomena/appendix.pdf' }; + await expect(authorizeRequestedFiles({ + req, files: [alias, inherited, { ...alias }, { ...inherited }], store: ownedStore(sessionKey), + })).resolves.toEqual([alias, inherited]); + }); + + test('keeps distinct destinations even when their object ids match', async () => { + const req = request({ tenantId: TENANT_ID, userId: USER_ID }); + const sessionKey = resolveSessionKey(req, { kind: 'user', id: USER_ID }); + const files = [ + validFile(), + validFile({ id: 'file_abcdefghijklmnop', name: 'inputs/second.csv' }), + validFile({ storage_session_id: 'sess_abcdefghijklmnop', name: 'inputs/third.csv' }), + ]; + const store = ownedStore(sessionKey); + for (const file of files) { + store.set(`session:${file.storage_session_id}`, sessionKey); + store.set(`upload:${sessionKey}${file.storage_session_id}${file.id}`, 'true'); + } + await expect(authorizeRequestedFiles({ req, files, store })).resolves.toEqual(files); + }); + + test('selects the last distinct user ref per path without letting an earlier echo replace it', async () => { + const req = request({ tenantId: TENANT_ID, userId: USER_ID }); + const sessionKey = resolveSessionKey(req, { kind: 'user', id: USER_ID }); + const earlier = validFile({ name: 'report.py' }); + const replacement = validFile({ + name: earlier.name, id: 'file_abcdefghijklmnop', storage_session_id: 'sess_abcdefghijklmnop', + }); + const unrelated = validFile({ id: 'file_0000000000000000', name: 'report.csv' }); + const store = ownedStore(sessionKey, earlier); + for (const file of [replacement, unrelated]) { + store.set(`session:${file.storage_session_id}`, sessionKey); + store.set(`upload:${sessionKey}${file.storage_session_id}${file.id}`, 'true'); + } + for (const files of [ + [earlier, unrelated, replacement], + [earlier, unrelated, replacement, { ...earlier }], + ]) { + await expect(authorizeRequestedFiles({ req, files, store })).resolves.toEqual([unrelated, replacement]); + } + await expect(authorizeRequestedFiles({ + req, files: [replacement, unrelated, earlier], store, + })).resolves.toEqual([unrelated, earlier]); + const alias = { ...earlier, name: 'original-report.py' }; + await expect(authorizeRequestedFiles({ + req, files: [earlier, alias, replacement, { ...earlier }], store, + })).resolves.toEqual([alias, replacement]); + }); + + test('authorizes superseded user refs before selecting a destination', async () => { + const req = request({ tenantId: TENANT_ID, userId: USER_ID }); + const sessionKey = resolveSessionKey(req, { kind: 'user', id: USER_ID }); + for (const unauthorized of [ + validFile({ storage_session_id: 'sess_abcdefghijklmnop' }), + validFile({ id: 'file_abcdefghijklmnop' }), + ]) { + for (const files of [[unauthorized, validFile()], [validFile(), unauthorized]]) { + await expectAuthError(authorizeRequestedFiles({ + req, files, store: ownedStore(sessionKey), + }), 403); + } + } + }); + + test('preserves shared input collisions for the sandbox to reject', async () => { + const req = request({ tenantId: TENANT_ID, userId: USER_ID }); + const userSessionKey = resolveSessionKey(req, { kind: 'user', id: USER_ID }); + for (const scope of [ + { kind: 'skill' as const, resource_id: SKILL_ID, version: 1 }, + { kind: 'agent' as const, resource_id: AGENT_ID }, + ]) { + const shared = validFile({ ...scope, storage_session_id: 'sess_abcdefghijklmnop' }); + const otherShared = { ...shared, id: 'file_abcdefghijklmnop' }; + const sharedKey = resolveSessionKey(req, { kind: scope.kind, id: scope.resource_id, version: scope.version }); + const store = ownedStore(userSessionKey); + store.set(`session:${shared.storage_session_id}`, sharedKey); + for (const file of [shared, otherShared]) { + store.set(`upload:${sharedKey}${file.storage_session_id}${file.id}`, 'true'); + } + for (const files of [[shared, validFile()], [validFile(), shared], [shared, otherShared]]) { + await expect(authorizeRequestedFiles({ req, files, store })).resolves.toEqual(files); + } + } + }); + + test('still rejects an unauthorized scope on a duplicate object', async () => { + const req = request({ tenantId: TENANT_ID, userId: USER_ID }); + const sessionKey = resolveSessionKey(req, { kind: 'user', id: USER_ID }); + await expectAuthError(authorizeRequestedFiles({ + req, + files: [validFile(), validFile({ kind: 'agent', resource_id: AGENT_ID })], + store: ownedStore(sessionKey), + }), 403); + }); + test('allows files owned by the resolved user sessionKey', async () => { const req = request({ tenantId: TENANT_ID, userId: USER_ID }); const sessionKey = resolveSessionKey(req, { kind: 'user', id: USER_ID }); diff --git a/service/src/service/file-authorization.ts b/service/src/service/file-authorization.ts index 4afa2c85..1b46ade0 100644 --- a/service/src/service/file-authorization.ts +++ b/service/src/service/file-authorization.ts @@ -269,5 +269,22 @@ export async function authorizeRequestedFiles(args: { } } - return requestedFiles; + // Deduplicate authorized refs by object and destination, preserving aliases. + const seenReferences = new Set(); + const uniqueReferences = requestedFiles.filter(file => { + const identity = `${file.storage_session_id}\0${file.id}\0${file.name}`; + if (seenReferences.has(identity)) return false; + seenReferences.add(identity); + return true; + }); + + // Last distinct user ref wins each path; exact echoes cannot undo replacements. + // Shared skill/agent inputs retain the sandbox's destination conflict checks. + const selectedUserFiles = new Map(); + for (const file of uniqueReferences) { + if (file.kind === 'user') selectedUserFiles.set(file.name, file); + } + return uniqueReferences.filter(file => + file.kind !== 'user' || selectedUserFiles.get(file.name) === file, + ); } diff --git a/service/src/service/programmatic-input-limit.test.ts b/service/src/service/programmatic-input-limit.test.ts new file mode 100644 index 00000000..fe7ba594 --- /dev/null +++ b/service/src/service/programmatic-input-limit.test.ts @@ -0,0 +1,96 @@ +import { expect, test } from 'bun:test'; +import { resolve } from 'path'; + +test('selected-workspace replay caps authorized, coalesced inputs', async () => { + // Keep infrastructure mocks isolated from other suites. Cancellation stops + // accepted requests after validation, before any execution state is written. + const probe = Bun.spawn([process.execPath, '-e', ` + import { mock } from 'bun:test'; + import assert from 'node:assert/strict'; + import { BRIDGE_WORKSPACE_PROGRAMMATIC_MAX_INPUT_FILES as limit, + BRIDGE_WORKSPACE_PROGRAMMATIC_MAX_FILES as totalLimit } from '../packages/code/src/protocol'; + const passthrough = (_req, _res, next) => next(); + mock.module('./src/middleware/limits', () => ({ + executionLimiter: passthrough, cancellationLimiter: passthrough, + })); + mock.module('./src/lifecycle', () => ({ + checkServiceStartUp: () => false, checkServiceShutDown: () => false, + })); + mock.module('./src/request-disconnect', () => ({ + observeRequestDisconnect: () => ({ + signal: AbortSignal.abort(), isDisconnected: () => false, dispose() {}, + }), + })); + const stored = new Map(); + mock.module('./src/queue', () => ({ + pyQueue: {}, pyQueueEvents: {}, jobCancellationRegistry: {}, + getExecutionQueueBinding() { throw new Error('must not enqueue'); }, + getExistingExecutionJob() { throw new Error('must not look up jobs'); }, + connection: { + defineCommand() {}, + get: async key => stored.get(key) ?? null, + exists: async key => stored.has(key) ? 1 : 0, + }, + })); + const { env } = await import('./src/config'); + env.PTC_MODE = 'replay'; + env.SANDBOX_BACKEND = 'remote-bridge'; + env.BRIDGE_DYNAMIC_WORKERS = true; + const { resolveSessionKey } = await import('./src/session-key'); + const { default: router } = await import('./src/service/programmatic-router'); + const handler = router.stack.find(layer => layer.route?.path === '/exec/programmatic').route.stack.at(-1).handle; + const auth = { userId: 'user_123', tenantId: 'tenant_abc' }; + const sessionKey = resolveSessionKey({ codeApiAuthContext: auth }, { kind: 'user', id: auth.userId }); + function file(index, name = 'input-' + index + '.txt') { + const result = { + id: 'file_' + String(index).padStart(16, '0'), resource_id: 'rsrc_1234567890123456', + storage_session_id: 'sess_1234567890123456', name, kind: 'user', + }; + stored.set('session:' + result.storage_session_id, sessionKey); + stored.set('upload:' + sessionKey + result.storage_session_id + result.id, 'true'); + return result; + } + async function request(files) { + const req = { + body: { code: 'ls', language: 'bash', tools: [], files }, + header: name => name === 'X-LibreChat-Code-Workspace-ID' ? 'workspace-1' : undefined, + codeApiPrincipal: { ...auth, principalSource: 'none', codeWorkerId: 'worker-1' }, + codeApiAuthContext: auth, + }; + const res = { status(code) { this.statusCode = code; return this; }, json(body) { this.body = body; return this; } }; + await handler(req, res); + return { req, res }; + } + assert.equal(limit + 2, totalLimit, 'main and history reserve two slots'); + const boundary = Array.from({ length: limit }, (_, index) => file(index)); + const replacement = file(limit, boundary[0].name); + for (const [inputs, expected] of [ + [boundary, boundary], + [[...boundary, boundary[0]], boundary], + [[...boundary, replacement, boundary[0]], [...boundary.slice(1), replacement]], + ]) { + const { req, res } = await request(inputs); + assert.equal(res.statusCode, 200, JSON.stringify(res.body)); + assert.equal(res.body.error, 'Programmatic execution request cancelled'); + assert.deepEqual(req.body.files, expected); + } + const { res: oversized } = await request([...boundary, file(limit + 1)]); + assert.equal(oversized.statusCode, 400); + assert.equal(oversized.body.error, 'Selected-workspace execution allows at most ' + limit + ' input files; main and replay history occupy two reserved slots'); + for (const unauthorized of [ + { ...boundary[0], kind: 'agent' }, + { ...boundary[0], storage_session_id: 'sess_abcdefghijklmnop' }, + ]) { + const { res } = await request([unauthorized, ...boundary]); + assert.equal(res.statusCode, 403); + assert.equal(res.body.error, 'Unauthorized file reference'); + } + console.log('PROGRAMMATIC_INPUT_LIMIT_OK'); + process.exit(0); + `], { cwd: resolve(__dirname, '../..'), stdout: 'pipe', stderr: 'pipe' }); + const [exitCode, stdout, stderr] = await Promise.all([ + probe.exited, new Response(probe.stdout).text(), new Response(probe.stderr).text(), + ]); + expect(exitCode, `${stdout}\n${stderr}`).toBe(0); + expect(stdout).toContain('PROGRAMMATIC_INPUT_LIMIT_OK'); +}, 15000); diff --git a/service/src/service/programmatic-router.ts b/service/src/service/programmatic-router.ts index 1063fbfe..85d7e2c8 100644 --- a/service/src/service/programmatic-router.ts +++ b/service/src/service/programmatic-router.ts @@ -509,9 +509,6 @@ async function handleReplayInitial( req.body as t.ProgrammaticRequestBody; let timeout: number; try { - if (workspaceId != null && Array.isArray(files) && files.length > BRIDGE_WORKSPACE_PROGRAMMATIC_MAX_INPUT_FILES) { - throw new Error(`Selected-workspace execution allows at most ${BRIDGE_WORKSPACE_PROGRAMMATIC_MAX_INPUT_FILES} input files; main and replay history occupy two reserved slots`); - } timeout = workspaceId != null ? normalizeSelectedWorkspaceProgrammaticTimeoutMs( (req.body as t.ProgrammaticRequestBody).timeout, @@ -602,6 +599,13 @@ async function handleReplayInitial( return; } + if (workspaceId != null && authorizedFiles.length > BRIDGE_WORKSPACE_PROGRAMMATIC_MAX_INPUT_FILES) { + res.status(400).json({ + error: `Selected-workspace execution allows at most ${BRIDGE_WORKSPACE_PROGRAMMATIC_MAX_INPUT_FILES} input files; main and replay history occupy two reserved slots`, + }); + return; + } + /* Output bucket: hardcoded user-private. See router.ts /exec for the * full rationale; same gate, same shape. */ let sessionKey: string;