diff --git a/src/daemon/__tests__/request-execution-scope.test.ts b/src/daemon/__tests__/request-execution-scope.test.ts index f0298db1f..2dfe85f2a 100644 --- a/src/daemon/__tests__/request-execution-scope.test.ts +++ b/src/daemon/__tests__/request-execution-scope.test.ts @@ -91,6 +91,29 @@ test('createRequestExecutionScope resolves session-scoped request and runner log expect(scope.runnerLogPath).toMatch(/cwd_[a-f0-9]{16}_default\/runner\.log$/); }); +test('a relative session name is rejected before any session artifact path is written', async () => { + const sessionStore = makeSessionStore('agent-device-request-scope-'); + const stateDir = sessionStore.resolveDaemonStateDir(); + const requestId = 'relative-session-1'; + + await withDiagnosticsScope({ command: 'snapshot', requestId, logPath: LOG_PATH }, async () => { + await expect( + createRequestExecutionScope({ + req: makeRequest({ session: '..', meta: { requestId } }), + sessionStore, + leaseRegistry: new LeaseRegistry(), + }), + ).rejects.toMatchObject({ + code: 'INVALID_ARGS', + message: expect.stringMatching(/session name/i), + }); + flushDiagnosticsToSessionFile({ force: true }); + }); + + // Nothing landed in the state dir itself (where `sessions/..` resolves to). + expect(fs.readdirSync(stateDir).filter((entry) => entry !== 'sessions')).toEqual([]); +}); + test('request diagnostics flush into the effective session request log', async () => { const sessionStore = makeSessionStore('agent-device-request-scope-'); const cwd = fs.mkdtempSync(path.join(TEST_ROOT, 'diag-scope-')); diff --git a/src/daemon/__tests__/session-store.test.ts b/src/daemon/__tests__/session-store.test.ts index 447f43482..32b4b03d9 100644 --- a/src/daemon/__tests__/session-store.test.ts +++ b/src/daemon/__tests__/session-store.test.ts @@ -3,6 +3,7 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; +import { AppError } from '@agent-device/kernel/errors'; import { SessionStore } from '../session-store.ts'; import type { SessionState } from '../types.ts'; import { buildRequestFinishedEvent } from '../session-event-log.ts'; @@ -114,6 +115,26 @@ test('defaultTracePath sanitizes session name', () => { assert.match(tracePath, /\.trace\.log$/); }); +test('resolveSessionDir keeps every session dir beneath the sessions dir', () => { + const sessionsDir = path.join(os.tmpdir(), 'agent-device-tests', 'sessions'); + const store = new SessionStore(sessionsDir); + assert.equal(store.resolveSessionDir('a/b:c d'), path.join(sessionsDir, 'a_b_c_d')); + // `.` and `..` survive `safeSessionName` unchanged, so without an explicit + // refusal `path.join` resolves them to the sessions dir itself and its parent + // (the daemon state dir): a remote caller's `--session ..` would then land + // app.log / runner.log / requests/*.ndjson outside the sessions tree. + for (const name of ['.', '..', '']) { + assert.throws( + () => store.resolveSessionDir(name), + (error: unknown) => + error instanceof AppError && + error.code === 'INVALID_ARGS' && + /session name/i.test(error.message), + `expected resolveSessionDir(${JSON.stringify(name)}) to reject`, + ); + } +}); + test('session lease metadata round-trips through the store', () => { const { store, session } = makeFixture('agent-device-session-lease-'); session.lease = { diff --git a/src/daemon/session-paths.ts b/src/daemon/session-paths.ts index dc54376bb..4ac0f3a76 100644 --- a/src/daemon/session-paths.ts +++ b/src/daemon/session-paths.ts @@ -4,6 +4,20 @@ export function safeSessionName(name: string): string { return name.replace(/[^a-zA-Z0-9._-]/g, '_'); } +/** + * Whether `name` addresses exactly one filesystem entry under a session dir. + * `safeSessionName` already strips separators, so what is left to reject is the + * empty name and the two relative-directory names — the shapes that would make + * `path.join` leave the directory it was given. `SessionStore.resolveSessionDir` + * refuses such a name (so no request can steer session artifacts outside the + * sessions tree), and callers that accept a session or request id from the + * network refuse it before addressing a record. + */ +export function isSafeSessionSegment(name: string): boolean { + const safe = safeSessionName(name); + return safe.length > 0 && safe !== '.' && safe !== '..'; +} + export function expandSessionPath(filePath: string, cwd?: string): string { return resolveUserPath(filePath, { cwd }); } diff --git a/src/daemon/session-store.ts b/src/daemon/session-store.ts index 63919fc97..ec04bbfb5 100644 --- a/src/daemon/session-store.ts +++ b/src/daemon/session-store.ts @@ -1,9 +1,10 @@ import path from 'node:path'; import fs from 'node:fs'; +import { AppError } from '@agent-device/kernel/errors'; import { emitDiagnostic } from '../utils/diagnostics.ts'; import type { SessionRuntimeHints, SessionState } from './types.ts'; import { recordActionEntry, type RecordActionEntry } from './session-action-recorder.ts'; -import { expandSessionPath, safeSessionName } from './session-paths.ts'; +import { expandSessionPath, isSafeSessionSegment, safeSessionName } from './session-paths.ts'; import { NO_SCRIPT_PUBLICATION, isRepairCommittable } from './session-script-publication-state.ts'; import { effectiveWriteForce } from './session-script-publication-capability.ts'; import { @@ -276,7 +277,19 @@ export class SessionStore { return path.join(this.sessionsDir, `${safeName}-${timestamp}.trace.log`); } + /** + * The one place a session name becomes a directory, so the invariant that every + * session dir lies beneath `sessionsDir` is enforced here rather than by each + * caller: `.` and `..` survive `safeSessionName` and would resolve to the + * sessions dir itself or the daemon state dir above it. + */ resolveSessionDir(sessionName: string): string { + if (!isSafeSessionSegment(sessionName)) { + throw new AppError( + 'INVALID_ARGS', + `Invalid session name ${JSON.stringify(sessionName)}: a session name cannot be empty, ".", or "..".`, + ); + } return path.join(this.sessionsDir, safeSessionName(sessionName)); }