diff --git a/src/host-core.test.ts b/src/host-core.test.ts index 2bb72d4b6..cbbaf27ba 100644 --- a/src/host-core.test.ts +++ b/src/host-core.test.ts @@ -23,6 +23,8 @@ import { sessionDir, inboundDbPath, outboundDbPath, + readOutboxFiles, + clearOutbox, } from './session-manager.js'; import { getSession, findSession } from './db/sessions.js'; import type { InboundEvent } from './channels/adapter.js'; @@ -108,6 +110,58 @@ describe('session manager', () => { outDb.close(); }); + it('should reject outbound attachment filenames that escape the message outbox', () => { + initSessionFolder('ag-1', 'sess-test'); + const dir = sessionDir('ag-1', 'sess-test'); + const msgOutbox = path.join(dir, 'outbox', 'msg-1'); + fs.mkdirSync(msgOutbox, { recursive: true }); + + const outside = path.join(TEST_DIR, 'outside.txt'); + fs.writeFileSync(outside, 'outside secret'); + + expect(readOutboxFiles('ag-1', 'sess-test', 'msg-1', ['../../../../../outside.txt'])).toBeUndefined(); + }); + + it('should reject outbound attachment symlinks that escape the message outbox', () => { + initSessionFolder('ag-1', 'sess-test'); + const dir = sessionDir('ag-1', 'sess-test'); + const msgOutbox = path.join(dir, 'outbox', 'msg-1'); + fs.mkdirSync(msgOutbox, { recursive: true }); + + const outside = path.join(TEST_DIR, 'outside.txt'); + fs.writeFileSync(outside, 'outside secret'); + fs.symlinkSync('../../../../../outside.txt', path.join(msgOutbox, 'safe-name.txt')); + + expect(readOutboxFiles('ag-1', 'sess-test', 'msg-1', ['safe-name.txt'])).toBeUndefined(); + }); + + it('should not recursively delete outside the outbox for unsafe message ids', () => { + initSessionFolder('ag-1', 'sess-test'); + const victimDir = path.join(TEST_DIR, 'victim-dir'); + fs.mkdirSync(victimDir, { recursive: true }); + fs.writeFileSync(path.join(victimDir, 'keep.txt'), 'do not delete'); + + clearOutbox('ag-1', 'sess-test', '../../../../victim-dir'); + + expect(fs.existsSync(path.join(victimDir, 'keep.txt'))).toBe(true); + }); + + it('should still read and clear normal basename outbox files', () => { + initSessionFolder('ag-1', 'sess-test'); + const dir = sessionDir('ag-1', 'sess-test'); + const msgOutbox = path.join(dir, 'outbox', 'msg-1'); + fs.mkdirSync(msgOutbox, { recursive: true }); + fs.writeFileSync(path.join(msgOutbox, 'result.txt'), 'ok'); + + const files = readOutboxFiles('ag-1', 'sess-test', 'msg-1', ['result.txt']); + expect(files).toHaveLength(1); + expect(files?.[0]?.filename).toBe('result.txt'); + expect(files?.[0]?.data.toString()).toBe('ok'); + + clearOutbox('ag-1', 'sess-test', 'msg-1'); + expect(fs.existsSync(msgOutbox)).toBe(false); + }); + it('should resolve to existing session (shared mode)', () => { const { session: s1, created: c1 } = resolveSession('ag-1', 'mg-1', null, 'shared'); expect(c1).toBe(true); diff --git a/src/session-manager.ts b/src/session-manager.ts index 96bca9624..2772443c5 100644 --- a/src/session-manager.ts +++ b/src/session-manager.ts @@ -21,7 +21,6 @@ import { DATA_DIR } from './config.js'; import { getMessagingGroup } from './db/messaging-groups.js'; import { createSession, - findSession, findSessionByAgentGroup, findSessionForAgent, getSession, @@ -38,6 +37,11 @@ import { import { log } from './log.js'; import type { Session } from './types.js'; +function isPathInside(parent: string, child: string): boolean { + const relative = path.relative(parent, child); + return relative === '' || (!relative.startsWith('..') && !path.isAbsolute(relative)); +} + /** Root directory for all session data. */ export function sessionsBaseDir(): string { return path.join(DATA_DIR, 'v2-sessions'); @@ -369,19 +373,48 @@ export function readOutboxFiles( messageId: string, filenames: string[], ): OutboundFile[] | undefined { + if (!isSafeAttachmentName(messageId)) { + log.warn('Rejecting unsafe outbox message id', { messageId }); + return undefined; + } + const outboxDir = path.join(sessionDir(agentGroupId, sessionId), 'outbox', messageId); if (!fs.existsSync(outboxDir)) return undefined; + + let realOutboxDir: string; + try { + const stat = fs.lstatSync(outboxDir); + if (!stat.isDirectory() || stat.isSymbolicLink()) { + log.warn('Rejecting unsafe outbox directory', { messageId, outboxDir }); + return undefined; + } + realOutboxDir = fs.realpathSync(outboxDir); + } catch (err) { + log.warn('Failed to inspect outbox directory', { messageId, err }); + return undefined; + } + const files: OutboundFile[] = []; for (const filename of filenames) { - // Reject any name that isn't a bare basename before touching the filesystem. if (!isSafeAttachmentName(filename)) { - log.warn('Refused unsafe outbox filename — would escape outbox', { messageId, filename }); + log.warn('Refused unsafe outbox filename, would escape outbox', { messageId, filename }); continue; } + const filePath = path.join(outboxDir, filename); - if (fs.existsSync(filePath)) { - files.push({ filename, data: fs.readFileSync(filePath) }); - } else { + try { + const stat = fs.lstatSync(filePath); + if (!stat.isFile() || stat.isSymbolicLink()) { + log.warn('Rejecting unsafe outbox file', { messageId, filename }); + continue; + } + const realFilePath = fs.realpathSync(filePath); + if (!isPathInside(realOutboxDir, realFilePath)) { + log.warn('Rejecting outbox file outside message directory', { messageId, filename }); + continue; + } + files.push({ filename, data: fs.readFileSync(realFilePath) }); + } catch { log.warn('Outbox file not found', { messageId, filename }); } } @@ -395,10 +428,26 @@ export function readOutboxFiles( * thrown error would trigger the delivery retry path and deliver twice. */ export function clearOutbox(agentGroupId: string, sessionId: string, messageId: string): void { + if (!isSafeAttachmentName(messageId)) { + log.warn('Rejecting unsafe outbox cleanup message id', { messageId }); + return; + } + const outboxDir = path.join(sessionDir(agentGroupId, sessionId), 'outbox', messageId); if (!fs.existsSync(outboxDir)) return; try { - fs.rmSync(outboxDir, { recursive: true, force: true }); + const stat = fs.lstatSync(outboxDir); + if (!stat.isDirectory() || stat.isSymbolicLink()) { + log.warn('Rejecting unsafe outbox cleanup directory', { messageId, outboxDir }); + return; + } + const realOutboxBase = fs.realpathSync(path.join(sessionDir(agentGroupId, sessionId), 'outbox')); + const realOutboxDir = fs.realpathSync(outboxDir); + if (!isPathInside(realOutboxBase, realOutboxDir)) { + log.warn('Rejecting outbox cleanup outside session outbox', { messageId, outboxDir }); + return; + } + fs.rmSync(realOutboxDir, { recursive: true, force: true }); } catch (err) { log.warn('Outbox cleanup failed (message already delivered)', { messageId, err }); }