diff --git a/claims.json b/claims.json index 6e93080..7fce028 100644 --- a/claims.json +++ b/claims.json @@ -106,16 +106,16 @@ }, { "id": "test-count", - "claim": "349 tests pass on Node's built-in test runner, the count docs/ROADMAP.md records for `npm test`.", - "value": "349", - "metric": "passing node:test cases (# tests 349 / # pass 349 / # fail 0)", - "method": "npm test → node --test test/*.test.js, run in the claims job after npm ci, and the TAP summary is asserted. The roadmap sentence ('349 tests on Node's built-in runner (`npm test`, 2026-10-17)') is verified by the run, not read back from the prose.", + "claim": "356 tests pass on Node's built-in test runner, the count docs/ROADMAP.md records for `npm test`.", + "value": "356", + "metric": "passing node:test cases (# tests 356 / # pass 356 / # fail 0)", + "method": "npm test → node --test test/*.test.js, run in the claims job after npm ci, and the TAP summary is asserted. The roadmap sentence ('356 tests on Node's built-in runner (`npm test`, 2026-10-17)') is verified by the run, not read back from the prose.", "repro": "npm test 2>&1 | grep -E '^# (tests|pass|fail)'", "evidence": "docs/ROADMAP.md", "as_of": "2026-10-17", "check": { "cmd": "npm test 2>&1 | grep -E '^# (tests|pass|fail)'", - "expect": { "contains": ["# tests 349", "# pass 349", "# fail 0"] }, + "expect": { "contains": ["# tests 356", "# pass 356", "# fail 0"] }, "timeout": 120 } }, diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index f96b93d..5a98043 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -8,7 +8,7 @@ - **Session browser** with tool-call inspection, trace/waterfall view, spawn tracking and message timeline - **Prompt tooling** — extraction (noise filtered), template clustering with outcome attribution, Claude-powered rewrites, and a prompt library that installs entries as native slash commands - **Global search** across all platforms, insights dashboard, incremental session backup -- **React + Vite frontend** served by an Express backend; 349 tests on Node's built-in runner (`npm test`, 2026-10-17), CI on Node 22 +- **React + Vite frontend** served by an Express backend; 356 tests on Node's built-in runner (`npm test`, 2026-10-17), CI on Node 22 - **Evidence-backed failure events and local review** with full-result invalidation, evidence navigation and narrow-screen session layout ## Current priorities diff --git a/docs/usage.md b/docs/usage.md index 2915ae5..cabd52b 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -176,7 +176,7 @@ GEMINI_DIR=/custom/path/gemini/tmp \ npm start ``` -**Via API:** Pass `?dir=/absolute/path` query parameter to any API endpoint. +**Via API:** Pass `?dir=/absolute/path` query parameter to any API endpoint. The override must be an absolute path without `..` whose real path (symlinks resolved) sits inside `$HOME`, one of the configured platform/storage dirs above, or a dir listed in `AGENTXRAY_ALLOWED_DIRS` (`:`-separated); anything else is rejected with `403`. --- @@ -218,7 +218,7 @@ npm start | `POST /api/backup` | Run an incremental backup into `~/.agentxray/archive` | | `GET /api/backup/status` | Archive stats: file count, total bytes, last backup time | -All list/detail endpoints accept an optional `?dir=` parameter to override the default directory. +All list/detail endpoints accept an optional `?dir=` parameter to override the default directory (same allowed-roots rule as above; out-of-bounds, relative, `..` or NUL-containing values get `403`, and agent names / session ids that are only dots get `400`). --- diff --git a/lib/config.js b/lib/config.js index 2b93aaf..7ba73f3 100644 --- a/lib/config.js +++ b/lib/config.js @@ -1,5 +1,6 @@ const path = require('path'); const fsp = require('fs/promises'); +const { resolveAllowedDir } = require('./safe-path'); const HOME = process.env.HOME || '/root'; const DATA_DIR = process.env.OPENCLAW_DIR || path.join(HOME, '.openclaw', 'agents'); @@ -25,11 +26,25 @@ const AGENT_NAME_RE = /^[A-Za-z0-9._-]+$/; // Key: absolute file path → { mtime: number, data: sessionMetadataObject } const sessionMetaCache = new Map(); +// Directory override from a request (?dir= and friends). No override → the +// default. Otherwise it must be an absolute path whose realpath lies inside an +// allowed root (lib/safe-path.js); anything else throws PathEscapeError (403) +// instead of silently reading another directory. function resolveDir(queryDir, defaultDir) { - if (!queryDir || typeof queryDir !== 'string') return defaultDir; - if (!path.isAbsolute(queryDir)) return defaultDir; - if (queryDir.includes('..')) return defaultDir; - return queryDir; + if (queryDir === undefined || queryDir === null || queryDir === '') return defaultDir; + return resolveAllowedDir(queryDir, [ + HOME, + DATA_DIR, + CODEX_DIR, + CLAUDE_CODE_DIR, + HERMES_DIR, + OMP_DIR, + DSH_DIR, + GEMINI_DIR, + ANALYSIS_DIR, + LIBRARY_DIR, + ARCHIVE_DIR, + ]); } function isArchivedFile(fileName) { @@ -40,12 +55,16 @@ function isSessionLogFile(fileName) { return fileName.endsWith('.jsonl') || isArchivedFile(fileName); } +// Both character sets allow '.', so a name made only of dots ('.', '..') would +// pass the regex and address the parent directory once joined; reject those. +const DOTS_ONLY_RE = /^\.+$/; + function sanitizeAgentName(name) { - return AGENT_NAME_RE.test(name) ? name : null; + return typeof name === 'string' && AGENT_NAME_RE.test(name) && !DOTS_ONLY_RE.test(name) ? name : null; } function sanitizeSessionId(id) { - return SESSION_ID_RE.test(id) ? id : null; + return typeof id === 'string' && SESSION_ID_RE.test(id) && !DOTS_ONLY_RE.test(id) ? id : null; } async function ensureDirectory(dirPath) { diff --git a/lib/library.js b/lib/library.js index 0f38cac..f913588 100644 --- a/lib/library.js +++ b/lib/library.js @@ -1,6 +1,7 @@ const fs = require('fs'); const fsp = require('fs/promises'); const path = require('path'); +const { resolveInside } = require('./safe-path'); const readline = require('readline'); const { execFile } = require('child_process'); const { HOME, LIBRARY_DIR, CODEX_DIR, CLAUDE_CODE_DIR, OMP_DIR } = require('./config'); @@ -22,11 +23,11 @@ function sanitizeLibraryName(name) { } function libraryFilePath(name) { - return path.join(LIBRARY_DIR, `${name}.md`); + return resolveInside(LIBRARY_DIR, `${name}.md`); } function installedFilePath(target, name) { - return path.join(INSTALL_TARGETS[target], `${name}.md`); + return resolveInside(INSTALL_TARGETS[target], `${name}.md`); } function normalizeLibraryTags(tags) { diff --git a/lib/platforms/claude.js b/lib/platforms/claude.js index f7e637e..c61c8b1 100644 --- a/lib/platforms/claude.js +++ b/lib/platforms/claude.js @@ -1,6 +1,7 @@ const fs = require('fs'); const fsp = require('fs/promises'); const path = require('path'); +const { resolveInside } = require('../safe-path'); const readline = require('readline'); const { CLAUDE_CODE_DIR } = require('../config'); const { withMetadataCache, makeMessage, sortSessionsByTimestampDesc, topToolsOf } = require('./shared'); @@ -23,23 +24,23 @@ async function findClaudeCodeSessionFile(baseDir, sessionId) { const dir = baseDir || CLAUDE_CODE_DIR; const projects = await listClaudeCodeProjects(dir); for (const project of projects) { - const dirPath = path.join(dir, project); + const dirPath = resolveInside(dir, project); const files = await fsp.readdir(dirPath, { withFileTypes: true }).catch(() => []); for (const f of files) { if (!f.isFile() || !f.name.endsWith('.jsonl')) continue; const base = f.name.replace(/\.jsonl$/, ''); if (base === sessionId || base.endsWith(sessionId)) { - return path.join(dirPath, f.name); + return resolveInside(dirPath, f.name); } } // Check subagents subdirectory - const subDir = path.join(dirPath, 'subagents'); + const subDir = resolveInside(dirPath, 'subagents'); const subFiles = await fsp.readdir(subDir, { withFileTypes: true }).catch(() => []); for (const f of subFiles) { if (!f.isFile() || !f.name.endsWith('.jsonl')) continue; const base = f.name.replace(/\.jsonl$/, ''); if (base === sessionId || base.endsWith(sessionId)) { - return path.join(subDir, f.name); + return resolveInside(subDir, f.name); } } } @@ -177,11 +178,11 @@ async function listClaudeCodeSessions(baseDir) { const projects = await listClaudeCodeProjects(dir); for (const project of projects) { - const dirPath = path.join(dir, project); + const dirPath = resolveInside(dir, project); const files = await fsp.readdir(dirPath, { withFileTypes: true }).catch(() => []); for (const f of files) { if (!f.isFile() || !f.name.endsWith('.jsonl')) continue; - sessions.push(parseClaudeCodeSessionMetadata(path.join(dirPath, f.name), f.name)); + sessions.push(parseClaudeCodeSessionMetadata(resolveInside(dirPath, f.name), f.name)); } } @@ -336,7 +337,7 @@ async function parseClaudeCodeSessionFile(filePath) { async function findClaudeSpawnDir(baseDir, sessionId) { const filePath = await findClaudeCodeSessionFile(baseDir, sessionId); if (!filePath) return null; - const spawnDir = path.join(path.dirname(filePath), sessionId, 'subagents'); + const spawnDir = resolveInside(path.dirname(filePath), sessionId, 'subagents'); try { const st = await fsp.stat(spawnDir); return st.isDirectory() ? spawnDir : null; diff --git a/lib/platforms/hermes.js b/lib/platforms/hermes.js index baf6c37..e1797f4 100644 --- a/lib/platforms/hermes.js +++ b/lib/platforms/hermes.js @@ -1,5 +1,6 @@ const fs = require('fs'); const path = require('path'); +const { resolveInside } = require('../safe-path'); // node:sqlite still emits an ExperimentalWarning on 22.x; it is the only warning // we swallow, and stderr is the operator's console for this server. const originalEmitWarning = process.emitWarning; @@ -14,7 +15,7 @@ const { makeMessage } = require('./shared'); function getHermesDbPath(dir) { const base = dir || HERMES_DIR; - return path.join(base, 'state.db'); + return resolveInside(base, 'state.db'); } // node:sqlite (Node >= 22.13): no native build step, so `npm i` never fails on a diff --git a/lib/platforms/omp.js b/lib/platforms/omp.js index 733bd13..ab1d454 100644 --- a/lib/platforms/omp.js +++ b/lib/platforms/omp.js @@ -1,6 +1,7 @@ const fs = require('fs'); const fsp = require('fs/promises'); const path = require('path'); +const { resolveInside } = require('../safe-path'); const readline = require('readline'); const { OMP_DIR } = require('../config'); const { ompOutcome } = require('./omp-outcome'); @@ -19,13 +20,13 @@ async function findOmpSessionFile(baseDir, sessionId) { const slugs = await fsp.readdir(dir, { withFileTypes: true }).catch(() => []); for (const s of slugs) { if (!s.isDirectory()) continue; - const slugDir = path.join(dir, s.name); + const slugDir = resolveInside(dir, s.name); const files = await fsp.readdir(slugDir, { withFileTypes: true }).catch(() => []); for (const f of files) { if (!f.isFile() || !f.name.endsWith('.jsonl')) continue; const base = f.name.replace(/\.jsonl$/, ''); if (base.endsWith('_' + sessionId) || base === sessionId) { - return path.join(slugDir, f.name); + return resolveInside(slugDir, f.name); } } } @@ -148,11 +149,11 @@ async function listOmpSessions(baseDir) { for (const s of entries) { if (!s.isDirectory()) continue; - const slugDir = path.join(dir, s.name); + const slugDir = resolveInside(dir, s.name); const files = await fsp.readdir(slugDir, { withFileTypes: true }).catch(() => []); for (const f of files) { if (!f.isFile() || !f.name.endsWith('.jsonl')) continue; - sessions.push(parseOmpSessionMetadata(path.join(slugDir, f.name), f.name)); + sessions.push(parseOmpSessionMetadata(resolveInside(slugDir, f.name), f.name)); } } diff --git a/lib/platforms/openclaw.js b/lib/platforms/openclaw.js index 93d4db2..53edf87 100644 --- a/lib/platforms/openclaw.js +++ b/lib/platforms/openclaw.js @@ -1,6 +1,7 @@ const fs = require('fs'); const fsp = require('fs/promises'); const path = require('path'); +const { resolveInside } = require('../safe-path'); const readline = require('readline'); const { DATA_DIR, HERMES_DIR, isArchivedFile, isSessionLogFile, ensureDirectory, readAgents } = require('../config'); const { withMetadataCache, sortSessionsByTimestampDesc, topToolsOf } = require('./shared'); @@ -140,7 +141,7 @@ async function _parseSessionMetadataRaw(filePath, fileName) { async function listSessionsForAgent(baseDir, agentName, includeArchived) { const dir = baseDir || DATA_DIR; - const agentDir = path.join(dir, agentName, 'sessions'); + const agentDir = resolveInside(dir, agentName, 'sessions'); await ensureDirectory(agentDir); const entries = await fsp.readdir(agentDir, { withFileTypes: true }); const sessionFiles = entries @@ -149,7 +150,7 @@ async function listSessionsForAgent(baseDir, agentName, includeArchived) { .map((entry) => entry.name); const sessions = await Promise.all( - sessionFiles.map((fileName) => parseSessionMetadata(path.join(agentDir, fileName), fileName)) + sessionFiles.map((fileName) => parseSessionMetadata(resolveInside(agentDir, fileName), fileName)) ); return sortSessionsByTimestampDesc(sessions); @@ -157,7 +158,7 @@ async function listSessionsForAgent(baseDir, agentName, includeArchived) { async function resolveSessionFile(baseDir, agentName, sessionId) { const dir = baseDir || DATA_DIR; - const agentDir = path.join(dir, agentName, 'sessions'); + const agentDir = resolveInside(dir, agentName, 'sessions'); await ensureDirectory(agentDir); const entries = await fsp.readdir(agentDir, { withFileTypes: true }); const candidates = entries @@ -178,7 +179,7 @@ async function resolveSessionFile(baseDir, agentName, sessionId) { return null; } - return path.join(agentDir, candidates[0]); + return resolveInside(agentDir, candidates[0]); } function normalizeMessage(record) { @@ -242,7 +243,7 @@ async function buildSpawnMap(baseDir) { const agents = await readAgents(dir); for (const agentName of agents) { - const agentDir = path.join(dir, agentName, 'sessions'); + const agentDir = resolveInside(dir, agentName, 'sessions'); let entries; try { entries = await fsp.readdir(agentDir, { withFileTypes: true }); @@ -256,7 +257,7 @@ async function buildSpawnMap(baseDir) { for (const fileName of sessionFiles) { const sessionId = fileName.split('.jsonl')[0]; - const filePath = path.join(agentDir, fileName); + const filePath = resolveInside(agentDir, fileName); const stream = fs.createReadStream(filePath, { encoding: 'utf8' }); const rl = readline.createInterface({ input: stream, crlfDelay: Infinity }); @@ -331,7 +332,7 @@ async function buildSpawnTree(baseDir) { const spawnCalls = []; for (const agentName of agents) { - const agentDir = path.join(dir, agentName, 'sessions'); + const agentDir = resolveInside(dir, agentName, 'sessions'); let entries; try { entries = await fsp.readdir(agentDir, { withFileTypes: true }); @@ -343,7 +344,7 @@ async function buildSpawnTree(baseDir) { for (const fileName of sessionFiles) { const sessionId = fileName.split('.jsonl')[0]; - const filePath = path.join(agentDir, fileName); + const filePath = resolveInside(agentDir, fileName); const stream = fs.createReadStream(filePath, { encoding: 'utf8' }); const rl = readline.createInterface({ input: stream, crlfDelay: Infinity }); diff --git a/lib/prompts.js b/lib/prompts.js index 26b5ea1..7ef8e1f 100644 --- a/lib/prompts.js +++ b/lib/prompts.js @@ -1,6 +1,7 @@ const fs = require('fs'); const fsp = require('fs/promises'); const path = require('path'); +const { resolveInside } = require('./safe-path'); const readline = require('readline'); const { normalizePromptText, hashPromptText } = require('./text-utils'); const { parseLlmJson } = require('./llm-json'); @@ -552,7 +553,7 @@ suggestions 数组按模板顺序,每个模板一项。用中文回答。JSON // platform (+ '-agentName') sanitized to [a-z0-9-]. Survives server restarts. function analysisFilePath(platform, agentName) { const key = `${platform}${agentName ? `-${agentName}` : ''}`.toLowerCase().replace(/[^a-z0-9-]/g, '-'); - return path.join(ANALYSIS_DIR, `${key}.json`); + return resolveInside(ANALYSIS_DIR, `${key}.json`); } async function loadPersistedAnalysis(platform, agentName) { diff --git a/lib/routes/sessions.js b/lib/routes/sessions.js index 3c1fc56..1345886 100644 --- a/lib/routes/sessions.js +++ b/lib/routes/sessions.js @@ -1,5 +1,6 @@ const fsp = require('fs/promises'); const path = require('path'); +const { resolveInside } = require('../safe-path'); const { DATA_DIR, OMP_DIR, @@ -188,7 +189,7 @@ module.exports = function mountSessionRoutes(app) { const children = []; for (const e of entries) { if (!e.isFile() || !e.name.endsWith('.jsonl')) continue; - const meta = await parseOmpSessionMetadata(path.join(spawnDir, e.name), e.name).catch(() => null); + const meta = await parseOmpSessionMetadata(resolveInside(spawnDir, e.name), e.name).catch(() => null); children.push({ name: e.name.replace(/\.jsonl$/, ''), file: e.name, @@ -214,7 +215,7 @@ module.exports = function mountSessionRoutes(app) { const dir = resolveDir(req.query.dir, OMP_DIR); const spawnDir = await findOmpSpawnDir(dir, sessionId); if (!spawnDir) return res.status(404).json({ error: 'No subagents for this session' }); - const payload = await parseOmpSessionFile(path.join(spawnDir, name + '.jsonl')); + const payload = await parseOmpSessionFile(resolveInside(spawnDir, `${name}.jsonl`)); res.json(payload); } catch (error) { if (error.code === 'ENOENT') return res.status(404).json({ error: 'Subagent not found' }); @@ -236,10 +237,10 @@ module.exports = function mountSessionRoutes(app) { for (const e of entries) { if (!e.isFile() || !e.name.startsWith('agent-') || !e.name.endsWith('.jsonl')) continue; const stem = e.name.replace(/\.jsonl$/, ''); - const meta = await parseClaudeCodeSessionMetadata(path.join(spawnDir, e.name), e.name).catch(() => null); + const meta = await parseClaudeCodeSessionMetadata(resolveInside(spawnDir, e.name), e.name).catch(() => null); let agentMeta = null; try { - agentMeta = JSON.parse(await fsp.readFile(path.join(spawnDir, stem + '.meta.json'), 'utf8')); + agentMeta = JSON.parse(await fsp.readFile(resolveInside(spawnDir, `${stem}.meta.json`), 'utf8')); } catch { /* absent or corrupt meta.json */ } @@ -270,7 +271,7 @@ module.exports = function mountSessionRoutes(app) { const dir = resolveDir(req.query.dir, CLAUDE_CODE_DIR); const spawnDir = await findClaudeSpawnDir(dir, sessionId); if (!spawnDir) return res.status(404).json({ error: 'No subagents for this session' }); - const payload = await parseClaudeCodeSessionFile(path.join(spawnDir, name + '.jsonl')); + const payload = await parseClaudeCodeSessionFile(resolveInside(spawnDir, `${name}.jsonl`)); res.json(payload); } catch (error) { if (error.code === 'ENOENT') return res.status(404).json({ error: 'Subagent not found' }); diff --git a/lib/safe-path.js b/lib/safe-path.js new file mode 100644 index 0000000..63bdd08 --- /dev/null +++ b/lib/safe-path.js @@ -0,0 +1,81 @@ +// Central path guards for every filesystem path built from request input. +// +// AgentXRay reads agent session directories, and the API lets the UI point a +// platform at another directory (?dir=, dirCodex=, …). Those overrides and every +// name joined under a base (agent, session id, library prompt, analysis key) go +// through here so a request can never reach outside the directories this tool is +// meant to read: +// - resolveInside(base, ...parts): resolve under a trusted base and require the +// result to stay inside it (rejects '..', absolute segments, NUL bytes). +// - resolveAllowedDir(input): an absolute directory override; its realpath must +// sit inside an allowed root (HOME, the configured platform/storage dirs, or +// AGENTXRAY_ALLOWED_DIRS), so a symlink cannot lead outside either. +// Violations throw PathEscapeError (HTTP 403 via its `status`). +const fs = require('fs'); +const path = require('path'); + +class PathEscapeError extends Error { + constructor(message) { + super(message); + this.name = 'PathEscapeError'; + this.code = 'EPATHESCAPE'; + this.status = 403; + } +} + +function isInside(root, candidate) { + return candidate === root || candidate.startsWith(root.endsWith(path.sep) ? root : root + path.sep); +} + +function assertCleanSegment(part) { + if (typeof part !== 'string') throw new PathEscapeError('path segment must be a string'); + if (part.includes('\0')) throw new PathEscapeError('path segment contains a NUL byte'); +} + +function resolveInside(base, ...parts) { + const root = path.resolve(base); + for (const part of parts) { + assertCleanSegment(part); + if (path.isAbsolute(part)) throw new PathEscapeError('absolute path segment not allowed'); + } + const target = path.resolve(root, ...parts); + if (!isInside(root, target)) { + throw new PathEscapeError('path escapes its base directory'); + } + return target; +} + +function realpathOrSelf(p) { + try { + return fs.realpathSync(p); + } catch { + return p; + } +} + +function allowedRoots(extraRoots) { + const listed = (process.env.AGENTXRAY_ALLOWED_DIRS || '').split(path.delimiter).filter(Boolean); + const roots = new Set(); + for (const r of [...extraRoots, ...listed]) { + if (typeof r !== 'string' || !path.isAbsolute(r)) continue; + const resolved = path.resolve(r); + roots.add(resolved); + roots.add(realpathOrSelf(resolved)); + } + return [...roots]; +} + +// Validate a directory override. Returns the realpath (or the resolved path when +// it does not exist yet) once it is proven to sit inside an allowed root. +function resolveAllowedDir(input, extraRoots) { + assertCleanSegment(input); + if (!path.isAbsolute(input)) throw new PathEscapeError('dir must be an absolute path'); + if (input.split(/[\\/]/).includes('..')) throw new PathEscapeError("dir must not contain '..'"); + const real = realpathOrSelf(path.resolve(input)); + for (const root of allowedRoots(extraRoots)) { + if (isInside(root, real)) return resolveInside(root, path.relative(root, real)); + } + throw new PathEscapeError('dir is outside the allowed roots (HOME, configured dirs, AGENTXRAY_ALLOWED_DIRS)'); +} + +module.exports = { PathEscapeError, resolveInside, resolveAllowedDir, isInside }; diff --git a/lib/search.js b/lib/search.js index a877ae0..40d4f29 100644 --- a/lib/search.js +++ b/lib/search.js @@ -1,6 +1,7 @@ const fs = require('fs'); const fsp = require('fs/promises'); const path = require('path'); +const { resolveInside } = require('./safe-path'); const readline = require('readline'); const { CLAUDE_CODE_DIR, HERMES_DIR, resolveDir } = require('./config'); const { collectSessionFiles } = require('./platforms'); @@ -175,7 +176,7 @@ async function searchGeminiFile(sf, keywords) { // sessions were removed by Claude's cleanupPeriodDays retention. Grouped by // project; prompts whose snippet already appears in a live hit are skipped. async function searchClaudeHistory(claudeDir, keywords, liveSnippets) { - const historyPath = path.join(path.dirname(claudeDir), 'history.jsonl'); + const historyPath = resolveInside(path.dirname(claudeDir), 'history.jsonl'); const byProject = new Map(); // project → matches[] const results = []; try { diff --git a/server.js b/server.js index 141c9b6..40a9380 100644 --- a/server.js +++ b/server.js @@ -13,6 +13,8 @@ const mountWatchRoutes = require('./lib/routes/watch'); const mountLlmRoutes = require('./lib/routes/llm'); const { csrfGuard } = require('./lib/csrf'); const { limiter } = require('./lib/rate-limit'); +const { resolveDir } = require('./lib/config'); +const { PathEscapeError } = require('./lib/safe-path'); const app = express(); const PORT = process.env.PORT || 3800; @@ -40,6 +42,21 @@ if (HAS_DIST) { } else { app.use(express.static(PUBLIC_DIR, { maxAge: 0, etag: false, lastModified: false })); } +// Directory overrides (?dir=, ?dirCodex=, …) must stay inside the allowed roots +// (lib/safe-path.js). Reject an escaping one with 403 before any route reads it. +const DIR_PARAM_RE = /^dir([A-Z][A-Za-z]*)?$/; +app.use('/api', (req, res, next) => { + for (const [key, value] of Object.entries(req.query || {})) { + if (!DIR_PARAM_RE.test(key) || value === '') continue; + try { + resolveDir(value, ''); + } catch (error) { + if (error instanceof PathEscapeError) return res.status(403).json({ error: error.message }); + return next(error); + } + } + next(); +}); // Reject cross-site POST/PUT/DELETE before the body is parsed (lib/csrf.js). app.use('/api', csrfGuard); app.use(express.json({ limit: '256kb' })); diff --git a/test/api.test.js b/test/api.test.js index 13037a7..b278bd5 100644 --- a/test/api.test.js +++ b/test/api.test.js @@ -6,6 +6,7 @@ const { describe, it, before, after } = require('node:test'); const assert = require('node:assert/strict'); const fsp = require('node:fs/promises'); +const http = require('node:http'); const path = require('node:path'); const { startServer, getJson, sendJson } = require('./helpers.js'); @@ -503,3 +504,67 @@ describe('csrf guard', () => { assert.equal(res.status, 200); }); }); + +describe('path guards on ?dir= and route params', () => { + let srv; + before(async () => { + srv = await startServer(); + }); + after(async () => { + await srv.stop(); + }); + + const status = async (pathname) => (await fetch(srv.base + pathname)).status; + const q = (value) => encodeURIComponent(value); + + it('rejects ?dir= outside the allowed roots with 403', async () => { + assert.equal(await status(`/api/codex/sessions?dir=${q('/etc')}`), 403); + assert.equal(await status(`/api/agents?dir=${q(path.dirname(srv.home))}`), 403); + const res = await fetch(`${srv.base}/api/codex/sessions?dir=${q('/etc')}`); + assert.match((await res.json()).error, /outside the allowed roots/); + }); + + it('rejects ../ escapes, relative dirs and NUL bytes with 403', async () => { + const dotdot = `${path.join(srv.home, '.codex', 'sessions')}/../../..`; + assert.equal(await status(`/api/codex/sessions?dir=${q(dotdot)}`), 403); + assert.equal(await status(`/api/codex/sessions?dir=${q(`${srv.home}/../${path.basename(srv.home)}`)}`), 403); + assert.equal(await status(`/api/codex/sessions?dir=${q('.codex/sessions')}`), 403); + assert.equal(await status(`/api/codex/sessions?dir=${q(`${srv.home}\0/x`)}`), 403); + assert.equal(await status(`/api/search?q=x&platform=codex&dirCodex=${q('/etc')}`), 403); + }); + + it('rejects a symlink inside HOME that points outside it', async () => { + const link = path.join(srv.home, 'escape-link'); + await fsp.symlink(path.dirname(srv.home), link); + assert.equal(await status(`/api/codex/sessions?dir=${q(link)}`), 403); + }); + + it('still serves an explicit ?dir= inside HOME, same as the default', async () => { + const dflt = await getJson(srv.base, '/api/codex/sessions'); + const explicit = await getJson(srv.base, `/api/codex/sessions?dir=${q(path.join(srv.home, '.codex', 'sessions'))}`); + assert.deepEqual( + explicit.map((s) => s.id), + dflt.map((s) => s.id) + ); + assert.ok(dflt.length > 0); + assert.equal(await status('/api/codex/sessions?dir='), 200); + }); + + // fetch() would normalise %2E%2E away per the URL spec, so send the raw path. + const rawStatus = (rawPath) => + new Promise((resolve, reject) => { + const { hostname, port } = new URL(srv.base); + http + .get({ hostname, port, path: rawPath }, (res) => { + res.resume(); + resolve(res.statusCode); + }) + .on('error', reject); + }); + + it('rejects dot-only agent names and session ids with 400', async () => { + assert.equal(await rawStatus('/api/agents/%2E%2E/sessions'), 400); + assert.equal(await rawStatus('/api/agents/main/sessions/%2E%2E'), 400); + assert.equal(await rawStatus('/api/agents/main/sessions/%2E%2E%2E'), 400); + }); +}); diff --git a/test/unit.test.js b/test/unit.test.js index e58f8bb..a1220ab 100644 --- a/test/unit.test.js +++ b/test/unit.test.js @@ -408,3 +408,24 @@ test('buildTurnLedger without usage still yields time rows and reports hasUsage= assert.equal(l.hasUsage, false); assert.equal(l.hasCost, false); }); + +const safePath = require(path.join(__dirname, '..', 'lib', 'safe-path')); + +test('resolveInside keeps joins inside the base and rejects escapes', () => { + const base = path.join(path.sep, 'srv', 'data'); + assert.equal(safePath.resolveInside(base, 'a', 'b.json'), path.join(base, 'a', 'b.json')); + assert.equal(safePath.resolveInside(base, 'a/../b'), path.join(base, 'b')); + for (const bad of [['..'], ['a', '../../etc'], [path.sep + 'etc'], ['a\0b'], [42]]) { + assert.throws(() => safePath.resolveInside(base, ...bad), { code: 'EPATHESCAPE' }); + } + assert.ok(safePath.isInside(base, base)); + assert.ok(!safePath.isInside(base, `${base}-evil`)); +}); + +test('resolveAllowedDir accepts absolute dirs under a root and rejects the rest', () => { + const root = path.join(path.sep, 'srv', 'data'); + assert.equal(safePath.resolveAllowedDir(path.join(root, 'x'), [root]), path.join(root, 'x')); + for (const bad of ['relative/x', `${root}/../etc`, path.join(path.sep, 'etc'), `${root}\0`]) { + assert.throws(() => safePath.resolveAllowedDir(bad, [root]), { name: 'PathEscapeError' }); + } +});