Skip to content
Merged
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
23 changes: 23 additions & 0 deletions src/daemon/__tests__/request-execution-scope.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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-'));
Expand Down
21 changes: 21 additions & 0 deletions src/daemon/__tests__/session-store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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 = {
Expand Down
14 changes: 14 additions & 0 deletions src/daemon/session-paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}
15 changes: 14 additions & 1 deletion src/daemon/session-store.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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));
}

Expand Down
Loading