From 18a4ccf23e49bd61c0c9847bf97258badea73904 Mon Sep 17 00:00:00 2001 From: Seth Karten <32787133+sethkarten@users.noreply.github.com> Date: Mon, 10 Aug 2026 10:14:50 -0700 Subject: [PATCH 1/6] fix(security): contain private session artifacts (cherry picked from commit b0ce5adf20ec8c891dd44e8b39bdb0ab453a5eba) --- packages/coding-agent/CHANGELOG.md | 1 + packages/coding-agent/src/config.ts | 20 +-- .../coding-agent/src/core/auth-storage.ts | 28 +--- .../src/core/kernel/state-snapshot.ts | 39 ++++- .../coding-agent/src/core/session-manager.ts | 133 +++++++++--------- .../coding-agent/src/utils/private-files.ts | 108 ++++++++++++++ .../test/kernel-state-snapshot.test.ts | 11 +- .../1105-session-storage-security.test.ts | 127 +++++++++++++++++ 8 files changed, 355 insertions(+), 112 deletions(-) create mode 100644 packages/coding-agent/src/utils/private-files.ts create mode 100644 packages/coding-agent/test/suite/regressions/1105-session-storage-security.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index b8e4bb0171..1d868ad3d6 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -6,6 +6,7 @@ - Added `app.messages.expand` (`ctrl+p`) to collapse or expand agent-to-agent messages separately from `ctrl+o` tool output. - Added a `ctrl+t` expand hint to collapsed thinking blocks, matching the tool output hint. - Changed expand/collapse hints to a consistent bracketed `(Ctrl+O to expand)` style across tool, message, summary, and error rows. +- Fixed session and artifact storage accepting traversal IDs, symlinked paths, permissive modes, and unsafe snapshot files ([#1105](https://github.com/PrimeIntellect-ai/prime-agent/pull/1105)). - Added a configurable copy action to login dialogs so raw sign-in URLs can be copied without selecting wrapped text ([#643](https://github.com/PrimeIntellect-ai/prime-agent/issues/643)). - Added privacy-safe pseudonymous product analytics for onboarding, command use, execution modes, run outcomes, TTFT, latency, usage, tools, retries, and compactions, with disclosure and opt-out controls ([ENG-4682](https://linear.app/primeintellect/issue/ENG-4682/add-privacy-safe-posthog-analytics-to-prime-agent)). - Changed sent agent messages in the IPython cell UI to show only the message text with a `╰─` gutter when expanded, matching received messages, and hid the raw `agent_message.send` receipt dictionary. diff --git a/packages/coding-agent/src/config.ts b/packages/coding-agent/src/config.ts index b709ab10e9..62a0a3fb3b 100644 --- a/packages/coding-agent/src/config.ts +++ b/packages/coding-agent/src/config.ts @@ -1,21 +1,11 @@ import { spawnSync } from "child_process"; import { createHash } from "crypto"; -import { - accessSync, - appendFileSync, - constants, - existsSync, - mkdirSync, - readFileSync, - realpathSync, - renameSync, - rmSync, - statSync, -} from "fs"; +import { accessSync, constants, existsSync, readFileSync, realpathSync, renameSync, rmSync, statSync } from "fs"; import { homedir } from "os"; import { basename, dirname, join, resolve, sep, win32 } from "path"; import { fileURLToPath } from "url"; import { shouldUseWindowsShell } from "./utils/child-process.js"; +import { appendPrivateFile, ensurePrivateFile } from "./utils/private-files.js"; // ============================================================================= // Package Detection @@ -593,9 +583,9 @@ const MAX_LOG_BYTES = 5 * 1024 * 1024; */ export function appendRotatingLog(logPath: string, message: string, maxBytes: number = MAX_LOG_BYTES): void { try { - mkdirSync(dirname(logPath), { recursive: true }); + ensurePrivateFile(logPath); try { - if (existsSync(logPath) && statSync(logPath).size > maxBytes) { + if (statSync(logPath).size > maxBytes) { // Drop any prior .old first: renameSync fails on Windows if it exists. rmSync(`${logPath}.old`, { force: true }); renameSync(logPath, `${logPath}.old`); @@ -603,7 +593,7 @@ export function appendRotatingLog(logPath: string, message: string, maxBytes: nu } catch { // Keep appending rather than dropping the log on a rotation failure. } - appendFileSync(logPath, `${message}\n`); + appendPrivateFile(logPath, `${message}\n`); } catch { // A read-only or missing log dir must never break the caller. } diff --git a/packages/coding-agent/src/core/auth-storage.ts b/packages/coding-agent/src/core/auth-storage.ts index 493c9c964d..83dd36eeec 100644 --- a/packages/coding-agent/src/core/auth-storage.ts +++ b/packages/coding-agent/src/core/auth-storage.ts @@ -15,10 +15,10 @@ import { type OAuthProviderId, } from "@earendil-works/pi-ai"; import { getOAuthApiKey, getOAuthProvider, getOAuthProviders } from "@earendil-works/pi-ai/oauth"; -import { chmodSync, existsSync, mkdirSync, readFileSync, writeFileSync } from "fs"; -import { dirname, join } from "path"; +import { join } from "path"; import lockfile from "proper-lockfile"; import { getAgentDir } from "../config.js"; +import { ensurePrivateFile, readPrivateFile, writePrivateFileAtomic } from "../utils/private-files.js"; import { clearPrimeCliCredentials, getPrimeCliConfigPath, @@ -108,18 +108,8 @@ export interface AuthStorageBackend { export class FileAuthStorageBackend implements AuthStorageBackend { constructor(private authPath: string = join(getAgentDir(), "auth.json")) {} - private ensureParentDir(): void { - const dir = dirname(this.authPath); - if (!existsSync(dir)) { - mkdirSync(dir, { recursive: true, mode: 0o700 }); - } - } - private ensureFileExists(): void { - if (!existsSync(this.authPath)) { - writeFileSync(this.authPath, "{}", "utf-8"); - chmodSync(this.authPath, 0o600); - } + ensurePrivateFile(this.authPath, "{}"); } private acquireLockSyncWithRetry(path: string): () => void { @@ -150,17 +140,15 @@ export class FileAuthStorageBackend implements AuthStorageBackend { } withLock(fn: (current: string | undefined) => LockResult): T { - this.ensureParentDir(); this.ensureFileExists(); let release: (() => void) | undefined; try { release = this.acquireLockSyncWithRetry(this.authPath); - const current = existsSync(this.authPath) ? readFileSync(this.authPath, "utf-8") : undefined; + const current = readPrivateFile(this.authPath, "utf-8"); const { result, next } = fn(current); if (next !== undefined) { - writeFileSync(this.authPath, next, "utf-8"); - chmodSync(this.authPath, 0o600); + writePrivateFileAtomic(this.authPath, next); } return result; } finally { @@ -171,7 +159,6 @@ export class FileAuthStorageBackend implements AuthStorageBackend { } async withLockAsync(fn: (current: string | undefined) => Promise>): Promise { - this.ensureParentDir(); this.ensureFileExists(); let release: (() => Promise) | undefined; @@ -200,12 +187,11 @@ export class FileAuthStorageBackend implements AuthStorageBackend { }); throwIfCompromised(); - const current = existsSync(this.authPath) ? readFileSync(this.authPath, "utf-8") : undefined; + const current = readPrivateFile(this.authPath, "utf-8"); const { result, next } = await fn(current); throwIfCompromised(); if (next !== undefined) { - writeFileSync(this.authPath, next, "utf-8"); - chmodSync(this.authPath, 0o600); + writePrivateFileAtomic(this.authPath, next); } throwIfCompromised(); return result; diff --git a/packages/coding-agent/src/core/kernel/state-snapshot.ts b/packages/coding-agent/src/core/kernel/state-snapshot.ts index 6de1219277..6080572501 100644 --- a/packages/coding-agent/src/core/kernel/state-snapshot.ts +++ b/packages/coding-agent/src/core/kernel/state-snapshot.ts @@ -99,12 +99,22 @@ def _prime_agent_snapshot_state(): payload[name] = blob total += _b.len(blob) - os.makedirs(os.path.dirname(${pyStr(outPath)}), exist_ok=True) - tmp = ${pyStr(outPath)} + ".tmp" + out_dir = os.path.dirname(${pyStr(outPath)}) + os.makedirs(out_dir, mode=0o700, exist_ok=True) try: - with _b.open(tmp, "wb") as fh: + os.chmod(out_dir, 0o700) + except _b.Exception: + pass + tmp = ${pyStr(outPath)} + ".tmp." + _b.str(os.getpid()) + "." + os.urandom(8).hex() + try: + flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL | _b.getattr(os, "O_NOFOLLOW", 0) + fd = os.open(tmp, flags, 0o600) + with os.fdopen(fd, "wb") as fh: dill.dump(payload, fh) + fh.flush() + os.fsync(fh.fileno()) os.replace(tmp, ${pyStr(outPath)}) + os.chmod(${pyStr(outPath)}, 0o600) except _b.Exception as _err: try: os.remove(tmp) @@ -124,10 +134,20 @@ def _prime_agent_snapshot_state(): "timestamp": datetime.datetime.now(datetime.timezone.utc).isoformat(), } try: - with _b.open(${pyStr(manifestPath)}, "w") as fh: + manifest_tmp = ${pyStr(manifestPath)} + ".tmp." + _b.str(os.getpid()) + "." + os.urandom(8).hex() + flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL | _b.getattr(os, "O_NOFOLLOW", 0) + fd = os.open(manifest_tmp, flags, 0o600) + with os.fdopen(fd, "w") as fh: json.dump(manifest, fh) + fh.flush() + os.fsync(fh.fileno()) + os.replace(manifest_tmp, ${pyStr(manifestPath)}) + os.chmod(${pyStr(manifestPath)}, 0o600) except _b.Exception: - pass + try: + os.remove(manifest_tmp) + except _b.Exception: + pass _b.print(${pyStr(RESULT_MARKER)} + json.dumps({"saved": saved, "skipped": skipped, "bytes": bytes_written})) @@ -159,7 +179,14 @@ def _prime_agent_restore_state(): return try: - with _b.open(${pyStr(inPath)}, "rb") as fh: + import stat as _stat + snapshot_stat = os.lstat(${pyStr(inPath)}) + if not _stat.S_ISREG(snapshot_stat.st_mode): + _b.print(${pyStr(RESULT_MARKER)} + json.dumps({"restored": [], "failed": [], "error": "load failed: snapshot is not a regular file"})) + return + flags = os.O_RDONLY | _b.getattr(os, "O_NOFOLLOW", 0) + fd = os.open(${pyStr(inPath)}, flags) + with os.fdopen(fd, "rb") as fh: payload = dill.load(fh) except _b.Exception as _err: _b.print(${pyStr(RESULT_MARKER)} + json.dumps({"restored": [], "failed": [], "error": "load failed: " + _b.str(_err)})) diff --git a/packages/coding-agent/src/core/session-manager.ts b/packages/coding-agent/src/core/session-manager.ts index f9161168e5..52b15ae59b 100644 --- a/packages/coding-agent/src/core/session-manager.ts +++ b/packages/coding-agent/src/core/session-manager.ts @@ -1,26 +1,19 @@ import type { AgentMessage } from "@earendil-works/pi-agent-core"; import type { AssistantMessage, ImageContent, Message, ServiceTier, TextContent, Usage } from "@earendil-works/pi-ai"; import { randomUUID } from "crypto"; -import { - appendFileSync, - chmodSync, - chownSync, - existsSync, - mkdirSync, - readdirSync, - readFileSync, - realpathSync, - renameSync, - rmSync, - statSync, - writeFileSync, -} from "fs"; +import { existsSync, readdirSync, readFileSync, realpathSync, statSync } from "fs"; import { readdir, readFile, stat } from "fs/promises"; -import { basename, dirname, join, resolve } from "path"; +import { dirname, isAbsolute, join, relative, resolve } from "path"; import { v7 as uuidv7 } from "uuid"; import { getAgentDir as getDefaultAgentDir, getSessionsDir } from "../config.js"; import { readFirstLineSync, readLinesAsBuffers } from "../utils/file-lines.js"; import { captureGitContext, type GitContext, gitContextsEqual } from "../utils/git.js"; +import { + appendPrivateFile, + assertRegularFileNoSymlink, + ensurePrivateDirectory, + writePrivateFileAtomic, +} from "../utils/private-files.js"; import { type BashExecutionMessage, type CustomMessage, @@ -53,25 +46,6 @@ const CONTENT_ENTRY_TYPES = new Set([ "branch_summary", ]); -function realpathIfPresent(path: string): string { - try { - return realpathSync(path); - } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT") return path; - throw error; - } -} - -function statMetadataIfPresent(path: string): { mode: number; uid: number; gid: number } | undefined { - try { - const { mode, uid, gid } = statSync(path); - return { mode: mode & 0o777, uid, gid }; - } catch (error) { - if ((error as NodeJS.ErrnoException).code === "ENOENT") return undefined; - throw error; - } -} - export interface SessionHeader { type: "session"; version?: number; // v1 sessions don't have this @@ -324,11 +298,22 @@ export type ReadonlySessionManager = Pick< | "getSessionName" >; +export const SESSION_ID_PATTERN = /^[A-Za-z0-9](?:[A-Za-z0-9._-]{0,127})$/; + +export function assertValidSessionId(sessionId: string): void { + if (!SESSION_ID_PATTERN.test(sessionId)) { + throw new Error( + "Invalid session id: expected 1-128 ASCII letters, digits, dots, underscores, or hyphens, starting with a letter or digit", + ); + } +} + function createSessionId(): string { return uuidv7(); } function getSessionFilePath(sessionDir: string, sessionId: string): string { + assertValidSessionId(sessionId); return join(sessionDir, `${sessionId}.jsonl`); } @@ -344,7 +329,23 @@ function createUniqueSessionFileTarget(sessionDir: string): { sessionId: string; } function getSessionArtifactPath(sessionDir: string, sessionId: string): string { - return join(dirname(sessionDir), "session-artifacts", sessionId); + assertValidSessionId(sessionId); + const artifactRoot = resolve(dirname(sessionDir), "session-artifacts"); + const artifactPath = resolve(artifactRoot, sessionId); + const lexicalRelativePath = relative(artifactRoot, artifactPath); + if (lexicalRelativePath.startsWith("..") || isAbsolute(lexicalRelativePath)) { + throw new Error(`Session artifact path escapes its root: ${artifactPath}`); + } + + ensurePrivateDirectory(artifactRoot); + ensurePrivateDirectory(artifactPath); + const canonicalRoot = realpathSync(artifactRoot); + const canonicalArtifactPath = realpathSync(artifactPath); + const canonicalRelativePath = relative(canonicalRoot, canonicalArtifactPath); + if (canonicalRelativePath.startsWith("..") || isAbsolute(canonicalRelativePath)) { + throw new Error(`Session artifact path escapes its canonical root: ${artifactPath}`); + } + return artifactPath; } /** Generate a unique short ID (8 hex chars, collision-checked) */ @@ -602,9 +603,7 @@ export function buildSessionContext( */ export function getDefaultSessionDir(_cwd: string, agentDir: string = getDefaultAgentDir()): string { const sessionDir = getSessionsDir(agentDir); - if (!existsSync(sessionDir)) { - mkdirSync(sessionDir, { recursive: true }); - } + ensurePrivateDirectory(sessionDir); return sessionDir; } @@ -652,7 +651,7 @@ async function parseEntriesFromBufferAsync(buffer: Buffer): Promise function finalizeLoadedEntries(entries: FileEntry[]): FileEntry[] { if (entries.length === 0) return entries; const header = entries[0]; - if (header.type !== "session" || typeof (header as any).id !== "string") { + if (header.type !== "session" || typeof header.id !== "string" || !SESSION_ID_PATTERN.test(header.id)) { return []; } applyChildUsageAttributions(entries); @@ -662,6 +661,7 @@ function finalizeLoadedEntries(entries: FileEntry[]): FileEntry[] { /** Exported for testing */ export function loadEntriesFromFile(filePath: string): FileEntry[] { if (!existsSync(filePath)) return []; + assertRegularFileNoSymlink(filePath); return finalizeLoadedEntries(parseEntriesFromBuffer(readFileSync(filePath))); } @@ -673,6 +673,7 @@ export async function loadEntriesFromFileAsync( options: { streamThresholdBytes?: number } = {}, ): Promise { if (!existsSync(filePath)) return []; + assertRegularFileNoSymlink(filePath); const streamThresholdBytes = options.streamThresholdBytes ?? SESSION_STREAMING_LOAD_THRESHOLD_BYTES; if ((await stat(filePath)).size < streamThresholdBytes) { return finalizeLoadedEntries(await parseEntriesFromBufferAsync(await readFile(filePath))); @@ -692,6 +693,7 @@ export async function loadEntriesFromFileAsync( } function readSessionHeader(filePath: string): Partial | undefined { + assertRegularFileNoSymlink(filePath); const firstLine = readFirstLineSync(filePath); if (!firstLine) { return undefined; @@ -779,7 +781,7 @@ function rootRlmDepthFromEnv(): number { function isValidSessionFile(filePath: string): boolean { try { const header = readSessionHeader(filePath); - return header?.type === "session" && typeof header.id === "string"; + return header?.type === "session" && typeof header.id === "string" && SESSION_ID_PATTERN.test(header.id); } catch { return false; } @@ -813,6 +815,7 @@ function sessionHeaderMatchesCwd(header: Partial | undefined, cwd return ( header?.type === "session" && typeof header.id === "string" && + SESSION_ID_PATTERN.test(header.id) && typeof header.cwd === "string" && normalizeCwd(header.cwd) === normalizeCwd(cwd) ); @@ -1082,6 +1085,9 @@ async function scanSessionInfo(filePath: string, stats: Awaited e.type === "session") as SessionHeader | undefined; - this.sessionId = header?.id ?? createSessionId(); + if (!header) { + throw new Error(`Session file is missing a valid header: ${this.sessionFile}`); + } + assertValidSessionId(header.id); + this.sessionId = header.id; let shouldRewrite = migrateToCurrentVersion(this.fileEntries); if (header?.parentSession && !isValidRlmDepth(header.rlmDepth)) { @@ -1267,6 +1281,7 @@ export class SessionManager { newSession(options?: NewSessionOptions): string | undefined { let sessionId = options?.id ?? createSessionId(); + assertValidSessionId(sessionId); let sessionFile: string | undefined; const hasExplicitRlmDepth = options !== undefined && Object.hasOwn(options, "rlmDepth"); let parentHeader: Partial | undefined; @@ -1345,21 +1360,7 @@ export class SessionManager { private _rewriteFile(): void { if (!this.persist || !this.sessionFile) return; const content = `${this.fileEntries.map((e) => JSON.stringify(e)).join("\n")}\n`; - const targetPath = realpathIfPresent(this.sessionFile); - const directory = dirname(targetPath); - mkdirSync(directory, { recursive: true }); - const tempPath = join(directory, `.${basename(targetPath)}.${process.pid}.${randomUUID()}.tmp`); - try { - const metadata = statMetadataIfPresent(targetPath); - writeFileSync(tempPath, content, metadata === undefined ? undefined : { mode: metadata.mode }); - if (metadata !== undefined) { - chownSync(tempPath, metadata.uid, metadata.gid); - chmodSync(tempPath, metadata.mode); - } - renameSync(tempPath, targetPath); - } finally { - rmSync(tempPath, { force: true }); - } + writePrivateFileAtomic(this.sessionFile, content); this._notifyPersistListeners(); } @@ -1408,9 +1409,7 @@ export class SessionManager { return this.sessionFile; } const dir = sessionDir ?? (this.sessionDir || getDefaultSessionDir(this.cwd)); - if (!existsSync(dir)) { - mkdirSync(dir, { recursive: true }); - } + ensurePrivateDirectory(dir); const previousHeader = this.getHeader(); const target = createUniqueSessionFileTarget(dir); this.sessionDir = dir; @@ -1468,8 +1467,7 @@ export class SessionManager { this._rewriteFile(); this.flushed = true; } else { - mkdirSync(dirname(this.sessionFile), { recursive: true }); - appendFileSync(this.sessionFile, `${JSON.stringify(entry)}\n`); + appendPrivateFile(this.sessionFile, `${JSON.stringify(entry)}\n`); this._notifyPersistListeners(); } } @@ -2241,9 +2239,7 @@ export class SessionManager { migrateToCurrentVersion(sourceEntries); const dir = sessionDir ?? getDefaultSessionDir(targetCwd); - if (!existsSync(dir)) { - mkdirSync(dir, { recursive: true }); - } + ensurePrivateDirectory(dir); // Create new session file with new ID but forked content const target = createUniqueSessionFileTarget(dir); @@ -2262,7 +2258,7 @@ export class SessionManager { rlmDepth: resolveSessionRlmDepth(sourceHeader, sourcePath), git: captureGitContext(targetCwd) ?? undefined, }; - appendFileSync(newSessionFile, `${JSON.stringify(newHeader)}\n`); + const forkedEntries: FileEntry[] = [newHeader]; // Drop the source's git_state entries (re-linking children): they describe the source repo, // so the fork would otherwise report the source's git instead of its own target context. @@ -2279,8 +2275,9 @@ export class SessionManager { if (entry.type === "session" || entry.type === "git_state") continue; const parentId = liveParent(entry.parentId); const out = parentId === entry.parentId ? entry : { ...entry, parentId }; - appendFileSync(newSessionFile, `${JSON.stringify(out)}\n`); + forkedEntries.push(out); } + writePrivateFileAtomic(newSessionFile, `${forkedEntries.map((entry) => JSON.stringify(entry)).join("\n")}\n`); return new SessionManager(targetCwd, dir, newSessionFile, true); } diff --git a/packages/coding-agent/src/utils/private-files.ts b/packages/coding-agent/src/utils/private-files.ts new file mode 100644 index 0000000000..82df2435b4 --- /dev/null +++ b/packages/coding-agent/src/utils/private-files.ts @@ -0,0 +1,108 @@ +import { randomUUID } from "node:crypto"; +import { + chmodSync, + closeSync, + constants, + existsSync, + fsyncSync, + lstatSync, + mkdirSync, + openSync, + readFileSync, + renameSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { basename, dirname, join } from "node:path"; + +const PRIVATE_DIRECTORY_MODE = 0o700; +const PRIVATE_FILE_MODE = 0o600; +const NOFOLLOW_FLAG = constants.O_NOFOLLOW ?? 0; + +export function assertRegularFileNoSymlink(path: string): void { + const stats = lstatSync(path); + if (stats.isSymbolicLink() || !stats.isFile()) { + throw new Error(`Refusing to use non-regular private file: ${path}`); + } +} + +export function ensurePrivateDirectory(path: string): void { + mkdirSync(path, { recursive: true, mode: PRIVATE_DIRECTORY_MODE }); + const stats = lstatSync(path); + if (stats.isSymbolicLink() || !stats.isDirectory()) { + throw new Error(`Refusing to use non-directory private path: ${path}`); + } + chmodSync(path, PRIVATE_DIRECTORY_MODE); +} + +export function ensurePrivateFile(path: string, initialContent = ""): void { + ensurePrivateDirectory(dirname(path)); + if (!existsSync(path)) { + let fd: number | undefined; + try { + fd = openSync( + path, + constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL | NOFOLLOW_FLAG, + PRIVATE_FILE_MODE, + ); + writeFileSync(fd, initialContent); + } finally { + if (fd !== undefined) closeSync(fd); + } + } + assertRegularFileNoSymlink(path); + chmodSync(path, PRIVATE_FILE_MODE); +} + +export function readPrivateFile(path: string, encoding: BufferEncoding): string { + assertRegularFileNoSymlink(path); + chmodSync(path, PRIVATE_FILE_MODE); + const fd = openSync(path, constants.O_RDONLY | NOFOLLOW_FLAG); + try { + return readFileSync(fd, encoding); + } finally { + closeSync(fd); + } +} + +export function writePrivateFileAtomic(path: string, content: string | Uint8Array): void { + ensurePrivateDirectory(dirname(path)); + if (existsSync(path)) { + assertRegularFileNoSymlink(path); + } + const tempPath = join(dirname(path), `.${basename(path)}.${process.pid}.${randomUUID()}.tmp`); + let fd: number | undefined; + try { + fd = openSync( + tempPath, + constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL | NOFOLLOW_FLAG, + PRIVATE_FILE_MODE, + ); + writeFileSync(fd, content); + fsyncSync(fd); + closeSync(fd); + fd = undefined; + renameSync(tempPath, path); + chmodSync(path, PRIVATE_FILE_MODE); + } finally { + if (fd !== undefined) closeSync(fd); + rmSync(tempPath, { force: true }); + } +} + +export function appendPrivateFile(path: string, content: string): void { + ensurePrivateDirectory(dirname(path)); + let flags = constants.O_WRONLY | constants.O_APPEND | NOFOLLOW_FLAG; + if (existsSync(path)) { + assertRegularFileNoSymlink(path); + chmodSync(path, PRIVATE_FILE_MODE); + } else { + flags |= constants.O_CREAT | constants.O_EXCL; + } + const fd = openSync(path, flags, PRIVATE_FILE_MODE); + try { + writeFileSync(fd, content); + } finally { + closeSync(fd); + } +} diff --git a/packages/coding-agent/test/kernel-state-snapshot.test.ts b/packages/coding-agent/test/kernel-state-snapshot.test.ts index 5aee07f717..d0663c4c7b 100644 --- a/packages/coding-agent/test/kernel-state-snapshot.test.ts +++ b/packages/coding-agent/test/kernel-state-snapshot.test.ts @@ -92,9 +92,13 @@ describe("buildSnapshotCode", () => { expect(code).toContain(String(DEFAULT_SNAPSHOT_MAX_BYTES)); }); - it("uses dill, an atomic write, and skips internal handles", () => { + it("uses dill with private exclusive atomic writes and skips internal handles", () => { expect(code).toContain("import dill"); + expect(code).toContain("os.O_EXCL"); + expect(code).toContain('getattr(os, "O_NOFOLLOW", 0)'); expect(code).toContain("os.replace"); + expect(code).toContain("os.chmod(out_dir, 0o700)"); + expect(code).toContain('os.chmod("/state/sess.dill", 0o600)'); // rlm and the IPython display names must never be serialized. expect(code).toContain('"rlm"'); expect(code).toContain(`print(${JSON.stringify(MARKER)}`); @@ -104,9 +108,12 @@ describe("buildSnapshotCode", () => { describe("buildRestoreCode", () => { const code = buildRestoreCode("/state/sess.dill"); - it("embeds the input path and no-ops when the file is missing", () => { + it("embeds the input path and rejects non-regular or symlinked snapshot files", () => { expect(code).toContain('"/state/sess.dill"'); expect(code).toContain("os.path.exists"); + expect(code).toContain("os.lstat"); + expect(code).toContain("S_ISREG"); + expect(code).toContain('getattr(os, "O_NOFOLLOW", 0)'); expect(code).toContain("dill.loads"); }); }); diff --git a/packages/coding-agent/test/suite/regressions/1105-session-storage-security.test.ts b/packages/coding-agent/test/suite/regressions/1105-session-storage-security.test.ts new file mode 100644 index 0000000000..adb8882cf5 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/1105-session-storage-security.test.ts @@ -0,0 +1,127 @@ +import { + chmodSync, + lstatSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + statSync, + symlinkSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { AuthStorage } from "../../../src/core/auth-storage.js"; +import { SessionManager } from "../../../src/core/session-manager.js"; + +const describePosix = process.platform === "win32" ? describe.skip : describe; + +describe("issue #1105 session storage security", () => { + let tempRoot: string; + + beforeEach(() => { + tempRoot = mkdtempSync(join(tmpdir(), "prime-1105-security-")); + }); + + afterEach(() => { + rmSync(tempRoot, { recursive: true, force: true }); + }); + + it.each(["../escape", "../../escape", "nested/id", "nested\\id", ".", "..", "x".repeat(129)])( + "rejects unsafe explicit session id %j", + (sessionId) => { + const sessionDir = join(tempRoot, "sessions"); + const manager = SessionManager.create(tempRoot, sessionDir); + + expect(() => manager.newSession({ id: sessionId })).toThrow("Invalid session id"); + expect(() => manager.getSessionArtifactDir()).not.toThrow(); + }, + ); + + it("rejects a traversal id from a persisted session header", () => { + const sessionDir = join(tempRoot, "sessions"); + mkdirSync(sessionDir); + const sessionFile = join(sessionDir, "attacker.jsonl"); + writeFileSync( + sessionFile, + `${JSON.stringify({ + type: "session", + version: 3, + id: "../../escaped-artifacts", + timestamp: new Date().toISOString(), + cwd: tempRoot, + })}\n`, + ); + + expect(() => SessionManager.open(sessionFile, sessionDir)).toThrow("Invalid session id"); + expect(readFileSync(sessionFile, "utf8")).toContain('"id":"../../escaped-artifacts"'); + }); + + describePosix("POSIX containment and permissions", () => { + it("rejects a symlinked session transcript", () => { + const sessionDir = join(tempRoot, "sessions"); + mkdirSync(sessionDir); + const target = join(tempRoot, "outside-session.jsonl"); + writeFileSync( + target, + `${JSON.stringify({ + type: "session", + version: 3, + id: "safe-session", + timestamp: new Date().toISOString(), + cwd: tempRoot, + })}\n`, + ); + const sessionFile = join(sessionDir, "safe-session.jsonl"); + symlinkSync(target, sessionFile); + + expect(() => SessionManager.open(sessionFile, sessionDir)).toThrow("non-regular private file"); + }); + + it("rejects a symlinked per-session artifact directory", () => { + const sessionDir = join(tempRoot, "sessions"); + const manager = SessionManager.create(tempRoot, sessionDir); + manager.newSession({ id: "safe-session" }); + const artifactRoot = join(tempRoot, "session-artifacts"); + const outside = join(tempRoot, "outside"); + mkdirSync(artifactRoot); + mkdirSync(outside); + symlinkSync(outside, join(artifactRoot, "safe-session"), "dir"); + + expect(() => manager.getSessionArtifactDir()).toThrow("Refusing to use non-directory private path"); + expect(lstatSync(join(artifactRoot, "safe-session")).isSymbolicLink()).toBe(true); + }); + + it("creates private session and artifact storage and repairs an existing transcript mode", () => { + const sessionDir = join(tempRoot, "sessions"); + const manager = SessionManager.create(tempRoot, sessionDir); + manager.appendSessionState({ status: "active" }); + manager.flushNow(); + const sessionFile = manager.getSessionFile()!; + chmodSync(sessionFile, 0o644); + manager.appendSessionInfo("private"); + const artifactDir = manager.getSessionArtifactDir()!; + + expect(statSync(sessionDir).mode & 0o777).toBe(0o700); + expect(statSync(sessionFile).mode & 0o777).toBe(0o600); + expect(statSync(join(tempRoot, "session-artifacts")).mode & 0o777).toBe(0o700); + expect(statSync(artifactDir).mode & 0o777).toBe(0o700); + }); + + it("rejects a symlinked auth file without modifying its target", () => { + const authDir = join(tempRoot, "auth"); + mkdirSync(authDir); + const target = join(tempRoot, "target.json"); + writeFileSync(target, '{"sentinel":true}'); + const authPath = join(authDir, "auth.json"); + symlinkSync(target, authPath); + + const storage = AuthStorage.create(authPath, { usePrimeCliConfig: false }); + storage.set("anthropic", { type: "api_key", key: "not-written" }); + + expect(storage.drainErrors().some((error) => error.message.includes("non-regular private file"))).toBe(true); + expect(readFileSync(target, "utf8")).toBe('{"sentinel":true}'); + }); + }); +}); From d0b6320260ab7772d162e0d4f2117d78d240c3eb Mon Sep 17 00:00:00 2001 From: Seth Karten <32787133+sethkarten@users.noreply.github.com> Date: Mon, 10 Aug 2026 10:53:51 -0700 Subject: [PATCH 2/6] fix(security): harden remaining private file sinks (cherry picked from commit 4ede067b97f803957af36aefe16487e15851c363) --- .../src/core/export-html/index.ts | 7 +- .../src/core/refinement/refinement.ts | 32 +----- .../components/extension-editor.ts | 15 +-- .../src/modes/interactive/interactive-mode.ts | 30 ++--- .../coding-agent/src/utils/private-files.ts | 108 +++++++++++++++--- .../interactive-mode-debug-command.test.ts | 26 ++++- .../1105-named-sinks-security.test.ts | 103 +++++++++++++++++ prime-agent-runtime/src/rlm/harness.py | 86 ++++++++++++-- prime-agent-runtime/test/test_harness.py | 21 ++++ 9 files changed, 348 insertions(+), 80 deletions(-) create mode 100644 packages/coding-agent/test/suite/regressions/1105-named-sinks-security.test.ts diff --git a/packages/coding-agent/src/core/export-html/index.ts b/packages/coding-agent/src/core/export-html/index.ts index e667366163..846e2a930b 100644 --- a/packages/coding-agent/src/core/export-html/index.ts +++ b/packages/coding-agent/src/core/export-html/index.ts @@ -1,8 +1,9 @@ import type { AgentState } from "@earendil-works/pi-agent-core"; -import { existsSync, readFileSync, writeFileSync } from "fs"; +import { existsSync, readFileSync } from "fs"; import { basename, join } from "path"; import { APP_NAME, getExportTemplateDir } from "../../config.js"; import { getResolvedThemeColors, getThemeExportColors } from "../../modes/interactive/theme/theme.js"; +import { writePrivateFileAtomic } from "../../utils/private-files.js"; import type { ToolDefinition } from "../extensions/types.js"; import type { SessionEntry } from "../session-manager.js"; import { SessionManager } from "../session-manager.js"; @@ -276,7 +277,7 @@ export async function exportSessionToHtml( outputPath = `${APP_NAME}-session-${sessionBasename}.html`; } - writeFileSync(outputPath, html, "utf8"); + writePrivateFileAtomic(outputPath, html, { privateParent: false }); return outputPath; } @@ -309,6 +310,6 @@ export async function exportFromFile(inputPath: string, options?: ExportOptions outputPath = `${APP_NAME}-session-${inputBasename}.html`; } - writeFileSync(outputPath, html, "utf8"); + writePrivateFileAtomic(outputPath, html, { privateParent: false }); return outputPath; } diff --git a/packages/coding-agent/src/core/refinement/refinement.ts b/packages/coding-agent/src/core/refinement/refinement.ts index b19b33db63..8d462c5c40 100644 --- a/packages/coding-agent/src/core/refinement/refinement.ts +++ b/packages/coding-agent/src/core/refinement/refinement.ts @@ -1,19 +1,10 @@ -import { randomUUID } from "node:crypto"; -import { - appendFileSync, - existsSync, - mkdirSync, - readFileSync, - renameSync, - statSync, - unlinkSync, - writeFileSync, -} from "node:fs"; +import { existsSync } from "node:fs"; import { join } from "node:path"; import type { AgentMessage, ThinkingLevel } from "@earendil-works/pi-agent-core"; import type { Model } from "@earendil-works/pi-ai"; import { completeSimple } from "@earendil-works/pi-ai"; import { getAgentDir } from "../../config.js"; +import { appendPrivateFile, readPrivateFile, writePrivateFileAtomic } from "../../utils/private-files.js"; import { serializeConversation } from "../compaction/utils.js"; import { convertToLlm } from "../messages.js"; import type { CustomEntry } from "../session-manager.js"; @@ -288,7 +279,7 @@ export function loadHarnessState( } let parsed: Partial; try { - const raw = JSON.parse(readFileSync(statePath, "utf8")); + const raw = JSON.parse(readPrivateFile(statePath, "utf8")); // loadHarnessState runs on every system-prompt build and before each /refine, so // a corrupt or unreadable (or non-object) state file must degrade to empty rather // than throw and break the session. The next saveHarnessState rewrites it cleanly. @@ -344,17 +335,7 @@ export function mergeHarnessStates(globalState: HarnessState, localState?: Harne export function saveHarnessState(harnessStateDir: string, state: HarnessState): string { const statePath = getHarnessStatePath(harnessStateDir); - const tempPath = `${statePath}.${process.pid}.${randomUUID()}.tmp`; - mkdirSync(harnessStateDir, { recursive: true }); - try { - const mode = existsSync(statePath) ? statSync(statePath).mode & 0o777 : 0o600; - writeFileSync(tempPath, `${JSON.stringify(state, null, 2)}\n`, { encoding: "utf8", mode }); - renameSync(tempPath, statePath); - } finally { - if (existsSync(tempPath)) { - unlinkSync(tempPath); - } - } + writePrivateFileAtomic(statePath, `${JSON.stringify(state, null, 2)}\n`); return statePath; } @@ -373,8 +354,7 @@ function isRefinementResult(data: unknown): data is RefinementResult { */ export function appendGlobalRefinement(harnessStateDir: string, result: RefinementResult): string { const historyPath = getRefinementHistoryPath(harnessStateDir); - mkdirSync(harnessStateDir, { recursive: true }); - appendFileSync(historyPath, `${JSON.stringify(result)}\n`, "utf8"); + appendPrivateFile(historyPath, `${JSON.stringify(result)}\n`); return historyPath; } @@ -384,7 +364,7 @@ export function loadGlobalRefinementHistory(harnessStateDir: string = getGlobalH return []; } const results: RefinementResult[] = []; - for (const line of readFileSync(historyPath, "utf8").split("\n")) { + for (const line of readPrivateFile(historyPath, "utf8").split("\n")) { const trimmed = line.trim(); if (!trimmed) continue; try { diff --git a/packages/coding-agent/src/modes/interactive/components/extension-editor.ts b/packages/coding-agent/src/modes/interactive/components/extension-editor.ts index e1134ce964..8d548ddd37 100644 --- a/packages/coding-agent/src/modes/interactive/components/extension-editor.ts +++ b/packages/coding-agent/src/modes/interactive/components/extension-editor.ts @@ -5,8 +5,6 @@ import { spawnSync } from "node:child_process"; import * as fs from "node:fs"; -import * as os from "node:os"; -import * as path from "node:path"; import { Container, Editor, @@ -18,6 +16,7 @@ import { type TUI, } from "@earendil-works/pi-tui"; import type { KeybindingsManager } from "../../../core/keybindings.js"; +import { createPrivateTempFile, readPrivateFile } from "../../../utils/private-files.js"; import { getEditorTheme, theme } from "../theme/theme.js"; import { DynamicBorder } from "./dynamic-border.js"; import { keyHint } from "./keybinding-hints.js"; @@ -117,10 +116,10 @@ export class ExtensionEditorComponent extends Container implements Focusable { } const currentText = this.editor.getText(); - const tmpFile = path.join(os.tmpdir(), `pi-extension-editor-${Date.now()}.md`); + const temp = createPrivateTempFile("pi-extension-editor-", ".md", currentText); + const tmpFile = temp.path; try { - fs.writeFileSync(tmpFile, currentText, "utf-8"); this.tui.stop(); const [editor, ...editorArgs] = editorCmd.split(" "); @@ -130,15 +129,11 @@ export class ExtensionEditorComponent extends Container implements Focusable { }); if (result.status === 0) { - const newContent = fs.readFileSync(tmpFile, "utf-8").replace(/\n$/, ""); + const newContent = readPrivateFile(tmpFile, "utf-8").replace(/\n$/, ""); this.editor.setText(newContent); } } finally { - try { - fs.unlinkSync(tmpFile); - } catch { - // Ignore cleanup errors - } + fs.rmSync(temp.directory, { recursive: true, force: true }); this.tui.start(); // Force full re-render since external editor uses alternate screen this.tui.requestRender(true); diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index b0585156ff..8f1831cc46 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts @@ -137,6 +137,7 @@ import { readClipboardImage } from "../../utils/clipboard-image.js"; import { parseGitUrl } from "../../utils/git.js"; import { resizeImage } from "../../utils/image-resize.js"; import { getCwdRelativePath } from "../../utils/paths.js"; +import { createPrivateTempFile, readPrivateFile, writePrivateFileAtomic } from "../../utils/private-files.js"; import { killTrackedDetachedChildren } from "../../utils/shell.js"; import { ensureTool, ensureToolWithStatus, formatMissingRipgrepMessage } from "../../utils/tools-manager.js"; import { checkForNewPiVersion } from "../../utils/version-check.js"; @@ -7310,12 +7311,10 @@ export class InteractiveMode { } const currentText = this.editor.getExpandedText?.() ?? this.editor.getText(); - const tmpFile = path.join(os.tmpdir(), `pi-editor-${Date.now()}.pi.md`); + const temp = createPrivateTempFile("pi-editor-", ".pi.md", currentText); + const tmpFile = temp.path; try { - // Write current content to temp file - fs.writeFileSync(tmpFile, currentText, "utf-8"); - // Stop TUI to release terminal this.ui.stop(); @@ -7330,17 +7329,13 @@ export class InteractiveMode { // On successful exit (status 0), replace editor content if (result.status === 0) { - const newContent = fs.readFileSync(tmpFile, "utf-8").replace(/\n$/, ""); + const newContent = readPrivateFile(tmpFile, "utf-8").replace(/\n$/, ""); this.editor.setText(newContent); } // On non-zero exit, keep original text (no action needed) } finally { // Clean up temp file - try { - fs.unlinkSync(tmpFile); - } catch { - // Ignore cleanup errors - } + fs.rmSync(temp.directory, { recursive: true, force: true }); // Restart TUI this.ui.start(); @@ -8955,11 +8950,13 @@ export class InteractiveMode { return; } - // Export to a temp file - const tmpFile = path.join(os.tmpdir(), "session.html"); + // Export to a private, unpredictable temp file. + const temp = createPrivateTempFile("prime-agent-share-", ".html"); + const tmpFile = temp.path; try { await this.agentConnection.exportToHtml(tmpFile); } catch (error: unknown) { + fs.rmSync(temp.directory, { recursive: true, force: true }); this.showError(`Failed to export session: ${error instanceof Error ? error.message : "Unknown error"}`); return; } @@ -8976,11 +8973,7 @@ export class InteractiveMode { this.editorContainer.clear(); this.editorContainer.addChild(this.editor); this.ui.setFocus(this.editor); - try { - fs.unlinkSync(tmpFile); - } catch { - // Ignore cleanup errors - } + fs.rmSync(temp.directory, { recursive: true, force: true }); }; // Create a secret gist asynchronously @@ -9909,8 +9902,7 @@ ${interrupt ? `| \`${interrupt}\` | Interrupt current operation |\n` : ""}${shor "", ].join("\n"); - fs.mkdirSync(path.dirname(debugLogPath), { recursive: true }); - fs.writeFileSync(debugLogPath, debugData); + writePrivateFileAtomic(debugLogPath, debugData); this.chatContainer.addChild(new Spacer(1)); this.chatContainer.addChild( diff --git a/packages/coding-agent/src/utils/private-files.ts b/packages/coding-agent/src/utils/private-files.ts index 82df2435b4..32148b79a2 100644 --- a/packages/coding-agent/src/utils/private-files.ts +++ b/packages/coding-agent/src/utils/private-files.ts @@ -3,21 +3,54 @@ import { chmodSync, closeSync, constants, - existsSync, + fchmodSync, + fstatSync, fsyncSync, lstatSync, mkdirSync, + mkdtempSync, openSync, readFileSync, renameSync, rmSync, writeFileSync, } from "node:fs"; +import { tmpdir } from "node:os"; import { basename, dirname, join } from "node:path"; const PRIVATE_DIRECTORY_MODE = 0o700; const PRIVATE_FILE_MODE = 0o600; const NOFOLLOW_FLAG = constants.O_NOFOLLOW ?? 0; +const DIRECTORY_FLAG = constants.O_DIRECTORY ?? 0; + +function pathExistsLexical(path: string): boolean { + try { + lstatSync(path); + return true; + } catch (error) { + if (error instanceof Error && "code" in error && error.code === "ENOENT") return false; + throw error; + } +} + +function setPrivateFileMode(fd: number, path: string, mode: number): void { + if (process.platform === "win32") { + chmodSync(path, mode); + } else { + fchmodSync(fd, mode); + } +} + +function openRegularFileNoSymlink(path: string, flags: number): number { + assertRegularFileNoSymlink(path); + const fd = openSync(path, flags | NOFOLLOW_FLAG); + const stats = fstatSync(fd); + if (!stats.isFile()) { + closeSync(fd); + throw new Error(`Refusing to use non-regular private file: ${path}`); + } + return fd; +} export function assertRegularFileNoSymlink(path: string): void { const stats = lstatSync(path); @@ -32,12 +65,22 @@ export function ensurePrivateDirectory(path: string): void { if (stats.isSymbolicLink() || !stats.isDirectory()) { throw new Error(`Refusing to use non-directory private path: ${path}`); } - chmodSync(path, PRIVATE_DIRECTORY_MODE); + if (process.platform === "win32") { + chmodSync(path, PRIVATE_DIRECTORY_MODE); + return; + } + const fd = openSync(path, constants.O_RDONLY | DIRECTORY_FLAG | NOFOLLOW_FLAG); + try { + if (!fstatSync(fd).isDirectory()) throw new Error(`Refusing to use non-directory private path: ${path}`); + setPrivateFileMode(fd, path, PRIVATE_DIRECTORY_MODE); + } finally { + closeSync(fd); + } } export function ensurePrivateFile(path: string, initialContent = ""): void { ensurePrivateDirectory(dirname(path)); - if (!existsSync(path)) { + if (!pathExistsLexical(path)) { let fd: number | undefined; try { fd = openSync( @@ -50,14 +93,17 @@ export function ensurePrivateFile(path: string, initialContent = ""): void { if (fd !== undefined) closeSync(fd); } } - assertRegularFileNoSymlink(path); - chmodSync(path, PRIVATE_FILE_MODE); + const privateFd = openRegularFileNoSymlink(path, constants.O_RDONLY); + try { + setPrivateFileMode(privateFd, path, PRIVATE_FILE_MODE); + } finally { + closeSync(privateFd); + } } export function readPrivateFile(path: string, encoding: BufferEncoding): string { - assertRegularFileNoSymlink(path); - chmodSync(path, PRIVATE_FILE_MODE); - const fd = openSync(path, constants.O_RDONLY | NOFOLLOW_FLAG); + const fd = openRegularFileNoSymlink(path, constants.O_RDONLY); + setPrivateFileMode(fd, path, PRIVATE_FILE_MODE); try { return readFileSync(fd, encoding); } finally { @@ -65,9 +111,24 @@ export function readPrivateFile(path: string, encoding: BufferEncoding): string } } -export function writePrivateFileAtomic(path: string, content: string | Uint8Array): void { - ensurePrivateDirectory(dirname(path)); - if (existsSync(path)) { +export function writePrivateFileAtomic( + path: string, + content: string | Uint8Array, + options: { privateParent?: boolean } = {}, +): void { + const parent = dirname(path); + if (options.privateParent === false) { + const parentExisted = pathExistsLexical(parent); + mkdirSync(parent, { recursive: true, mode: PRIVATE_DIRECTORY_MODE }); + const parentStats = lstatSync(parent); + if (parentStats.isSymbolicLink() || !parentStats.isDirectory()) { + throw new Error(`Refusing to use non-directory private path: ${parent}`); + } + if (!parentExisted) chmodSync(parent, PRIVATE_DIRECTORY_MODE); + } else { + ensurePrivateDirectory(parent); + } + if (pathExistsLexical(path)) { assertRegularFileNoSymlink(path); } const tempPath = join(dirname(path), `.${basename(path)}.${process.pid}.${randomUUID()}.tmp`); @@ -83,7 +144,6 @@ export function writePrivateFileAtomic(path: string, content: string | Uint8Arra closeSync(fd); fd = undefined; renameSync(tempPath, path); - chmodSync(path, PRIVATE_FILE_MODE); } finally { if (fd !== undefined) closeSync(fd); rmSync(tempPath, { force: true }); @@ -93,16 +153,36 @@ export function writePrivateFileAtomic(path: string, content: string | Uint8Arra export function appendPrivateFile(path: string, content: string): void { ensurePrivateDirectory(dirname(path)); let flags = constants.O_WRONLY | constants.O_APPEND | NOFOLLOW_FLAG; - if (existsSync(path)) { + const exists = pathExistsLexical(path); + if (exists) { assertRegularFileNoSymlink(path); - chmodSync(path, PRIVATE_FILE_MODE); } else { flags |= constants.O_CREAT | constants.O_EXCL; } const fd = openSync(path, flags, PRIVATE_FILE_MODE); try { + if (!fstatSync(fd).isFile()) throw new Error(`Refusing to use non-regular private file: ${path}`); + setPrivateFileMode(fd, path, PRIVATE_FILE_MODE); writeFileSync(fd, content); } finally { closeSync(fd); } } + +export interface PrivateTempFile { + path: string; + directory: string; +} + +export function createPrivateTempFile(prefix: string, suffix: string, content = ""): PrivateTempFile { + const directory = mkdtempSync(join(tmpdir(), prefix)); + chmodSync(directory, PRIVATE_DIRECTORY_MODE); + const path = join(directory, `${randomUUID()}${suffix}`); + try { + ensurePrivateFile(path, content); + return { path, directory }; + } catch (error) { + rmSync(directory, { recursive: true, force: true }); + throw error; + } +} diff --git a/packages/coding-agent/test/interactive-mode-debug-command.test.ts b/packages/coding-agent/test/interactive-mode-debug-command.test.ts index 3388fcfc22..4c415dc44b 100644 --- a/packages/coding-agent/test/interactive-mode-debug-command.test.ts +++ b/packages/coding-agent/test/interactive-mode-debug-command.test.ts @@ -1,4 +1,4 @@ -import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { existsSync, mkdtempSync, readFileSync, rmSync, statSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import type { AgentMessage } from "@earendil-works/pi-agent-core"; @@ -70,10 +70,34 @@ describe("InteractiveMode /debug", () => { expect(debugLog).toContain("Terminal: 80x24"); expect(debugLog).toContain("rendered line"); expect(debugLog).toContain(JSON.stringify(message)); + expect(statSync(getDebugLogPath()).mode & 0o777).toBe(0o600); + expect(statSync(tempAgentDir).mode & 0o777).toBe(0o700); expect(context.showError).not.toHaveBeenCalled(); expect(context.ui.requestRender).toHaveBeenCalledWith(); }); + it("refuses a symlinked debug log without modifying its target", async () => { + const target = join(tempAgentDir, "outside.log"); + writeFileSync(target, "sentinel"); + symlinkSync(target, getDebugLogPath()); + const context: DebugCommandContext = { + ui: { + terminal: { columns: 80, rows: 24 }, + render: vi.fn(() => ["rendered line"]), + requestRender: vi.fn(), + }, + agentConnection: { getMessages: vi.fn(async () => []) }, + chatContainer: new Container(), + showError: vi.fn(), + }; + + await interactiveModePrototype.handleDebugCommand.call(context); + + expect(context.showError).toHaveBeenCalledWith(expect.stringContaining("non-regular private file")); + expect(readFileSync(target, "utf8")).toBe("sentinel"); + expect(context.ui.requestRender).not.toHaveBeenCalled(); + }); + it("reports connection failures instead of falling back to local UI services", async () => { const context: DebugCommandContext = { ui: { diff --git a/packages/coding-agent/test/suite/regressions/1105-named-sinks-security.test.ts b/packages/coding-agent/test/suite/regressions/1105-named-sinks-security.test.ts new file mode 100644 index 0000000000..860df59773 --- /dev/null +++ b/packages/coding-agent/test/suite/regressions/1105-named-sinks-security.test.ts @@ -0,0 +1,103 @@ +import { + lstatSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + statSync, + symlinkSync, + unlinkSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { exportFromFile } from "../../../src/core/export-html/index.js"; +import { + appendGlobalRefinement, + getHarnessStatePath, + getRefinementHistoryPath, + loadHarnessState, + saveHarnessState, +} from "../../../src/core/refinement/refinement.js"; +import { SessionManager } from "../../../src/core/session-manager.js"; +import { createPrivateTempFile } from "../../../src/utils/private-files.js"; + +const describePosix = process.platform === "win32" ? describe.skip : describe; + +describePosix("issue #1105 named sink security", () => { + let tempRoot: string; + + beforeEach(() => { + tempRoot = mkdtempSync(join(tmpdir(), "prime-1105-named-sinks-")); + }); + + afterEach(() => { + rmSync(tempRoot, { recursive: true, force: true }); + }); + + it("creates unpredictable private editor/share temp files", () => { + const first = createPrivateTempFile("prime-agent-security-test-", ".html", "secret"); + const second = createPrivateTempFile("prime-agent-security-test-", ".html", "secret"); + try { + expect(first.path).not.toBe(second.path); + expect(lstatSync(first.path).isSymbolicLink()).toBe(false); + expect(statSync(first.directory).mode & 0o777).toBe(0o700); + expect(statSync(first.path).mode & 0o777).toBe(0o600); + expect(readFileSync(first.path, "utf8")).toBe("secret"); + } finally { + rmSync(first.directory, { recursive: true, force: true }); + rmSync(second.directory, { recursive: true, force: true }); + } + }); + + it("writes HTML exports privately and refuses a symlink destination", async () => { + const manager = SessionManager.create(tempRoot, join(tempRoot, "sessions")); + manager.appendMessage({ role: "user", content: "secret export", timestamp: Date.now() }); + manager.flushNow(); + const sessionFile = manager.getSessionFile(); + if (!sessionFile) throw new Error("Missing test session file"); + const output = join(tempRoot, "exports", "session.html"); + + await exportFromFile(sessionFile, output); + expect(statSync(output).mode & 0o777).toBe(0o600); + expect(readFileSync(output, "utf8")).toContain('