diff --git a/packages/coding-agent/.changes/kernel-stderr-log-owner-only.md b/packages/coding-agent/.changes/kernel-stderr-log-owner-only.md new file mode 100644 index 0000000000..442591b395 --- /dev/null +++ b/packages/coding-agent/.changes/kernel-stderr-log-owner-only.md @@ -0,0 +1 @@ +- Fixed the kernel stderr log (`kernel-stderr.log` in the session artifact directory) being created world-readable: the log is now owner-only (0600), and its directory is created owner-only (0700) when the kernel manager creates it, matching the kernel state snapshot and the other private session artifacts, because kernel stderr can carry exception payloads. diff --git a/packages/coding-agent/src/core/kernel/repl-manager.ts b/packages/coding-agent/src/core/kernel/repl-manager.ts index 31a1408f30..9413847708 100644 --- a/packages/coding-agent/src/core/kernel/repl-manager.ts +++ b/packages/coding-agent/src/core/kernel/repl-manager.ts @@ -2,7 +2,18 @@ // (`python -m rlm.repl`) — requests on stdin, events on stdout, stderr kept as // a diagnostics tail. The protocol is documented in prime-agent-runtime/src/rlm/repl.md. import type { ChildProcess } from "node:child_process"; -import { closeSync, existsSync, mkdirSync, openSync, renameSync, rmSync, statSync, writeSync } from "node:fs"; +import { + chmodSync, + closeSync, + existsSync, + fchmodSync, + mkdirSync, + openSync, + renameSync, + rmSync, + statSync, + writeSync, +} from "node:fs"; import { dirname } from "node:path"; import { StringDecoder } from "node:string_decoder"; import { v4 as uuid } from "uuid"; @@ -63,6 +74,10 @@ const MAX_BACKGROUND_OUTPUT_CHARS = 64 * 1024; const MAX_KERNEL_STDERR_CHARS = 8 * 1024; const MAX_KERNEL_STDERR_LOG_BYTES = 5 * 1024 * 1024; const KERNEL_STDERR_LOG_BUDGET_MARKER = "[stderr log budget exhausted]\n"; +// Owner-only directory for the kernel stderr log, matching the other private session artifacts. +const KERNEL_STDERR_LOG_DIR_MODE = 0o700; +// Owner-only file bits; kernel stderr can carry exception payloads. +const KERNEL_STDERR_LOG_MODE = 0o600; /** fs.writeSync may write fewer bytes than asked (partial ENOSPC, signals); loop until done. */ function writeFullySync(fd: number, data: Buffer): void { @@ -247,10 +262,13 @@ export class ReplKernelManager { const path = this.options.stderrLogPath; if (!path) return undefined; try { - mkdirSync(dirname(path), { recursive: true }); + mkdirSync(dirname(path), { recursive: true, mode: KERNEL_STDERR_LOG_DIR_MODE }); let size = existsSync(path) ? statSync(path).size : 0; if (size > MAX_KERNEL_STDERR_LOG_BYTES) { try { + // Tighten before the move: a renamed log keeps its mode, and the + // rotated file holds the exception payloads worth protecting. + chmodSync(path, KERNEL_STDERR_LOG_MODE); // Drop any prior .old first: rename fails on Windows if it exists. rmSync(`${path}.old`, { force: true }); renameSync(path, `${path}.old`); @@ -260,7 +278,15 @@ export class ReplKernelManager { this.appendKernelDiagnostic(`cannot rotate kernel stderr log: ${errorMessage(error)}`); } } - return { fd: openSync(path, "a"), budget: Math.max(0, MAX_KERNEL_STDERR_LOG_BYTES - size) }; + const fd = openSync(path, "a", KERNEL_STDERR_LOG_MODE); + // Exact bits despite the umask; tightens a pre-existing loose log. + try { + fchmodSync(fd, KERNEL_STDERR_LOG_MODE); + } catch (error) { + closeSync(fd); + throw error; + } + return { fd, budget: Math.max(0, MAX_KERNEL_STDERR_LOG_BYTES - size) }; } catch (error) { this.appendKernelDiagnostic(`cannot open kernel stderr log: ${errorMessage(error)}`); return undefined; diff --git a/packages/coding-agent/test/repl-kernel-startup.test.ts b/packages/coding-agent/test/repl-kernel-startup.test.ts index b8367fbadb..18db4777fd 100644 --- a/packages/coding-agent/test/repl-kernel-startup.test.ts +++ b/packages/coding-agent/test/repl-kernel-startup.test.ts @@ -1,4 +1,4 @@ -import { chmodSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { chmodSync, mkdtempSync, rmSync, statSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; @@ -6,9 +6,31 @@ import { ReplKernelManager } from "../src/core/kernel/index.js"; let tempDir = ""; -function writeExecutable(filePath: string, content: string): void { - writeFileSync(filePath, content); - chmodSync(filePath, 0o755); +// Failing-start tests settle well inside the 30s ready timeout. +const START_FAILURE_TIMEOUT_MS = 15_000; + +/** A fake kernel runtime (usually one that dies before ready) at the manager's python path. */ +function fakeRuntime(...lines: string[]): string { + const python = join(tempDir, "python"); + writeFileSync(python, lines.join("\n")); + chmodSync(python, 0o755); + return python; +} + +/** Runs a manager against a fake runtime with console noise muted, then always shuts it down. */ +async function withManager( + python: string, + body: (manager: ReplKernelManager) => Promise, + stderrLogPath?: string, +): Promise { + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const manager = new ReplKernelManager({ python, cwd: tempDir, stderrLogPath }); + try { + await body(manager); + } finally { + errorSpy.mockRestore(); + await manager.shutdown({ snapshot: true, drainHostRequests: true }); + } } describe("ReplKernelManager startup", () => { @@ -24,97 +46,87 @@ describe("ReplKernelManager startup", () => { }); it("surfaces kernels that exit before ready with the stderr tail", async () => { - const python = join(tempDir, "python"); - writeExecutable(python, ["#!/bin/sh", 'echo "fake runtime died before ready" >&2', "exit 42", ""].join("\n")); - const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - const manager = new ReplKernelManager({ python, cwd: tempDir }); - - try { + const python = fakeRuntime("#!/bin/sh", 'echo "fake runtime died before ready" >&2', "exit 42"); + await withManager(python, async (manager) => { await expect(manager.execute("print(1)")).rejects.toThrow( /Kernel exited before ready[\s\S]*fake runtime died before ready/, ); - } finally { - errorSpy.mockRestore(); - await manager.shutdown({ snapshot: true, drainHostRequests: true }); - } + }); }); - it("completes teardown while an inherited grandchild keeps writing stderr", async () => { - const python = join(tempDir, "python"); - writeExecutable( - python, - [ - "#!/bin/sh", - // A busy writer inherits fd 2 and survives the kernel: its stream - // never goes quiet and never EOFs. - "sh -c 'while :; do echo post-mortem noise; done' >&2 &", - "exit 42", - "", - ].join("\n"), - ); - const stderrLogPath = join(tempDir, "kernel-stderr.log"); - const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - const manager = new ReplKernelManager({ python, cwd: tempDir, stderrLogPath }); + it( + "completes teardown while an inherited grandchild keeps writing stderr", + async () => { + // A busy writer inherits fd 2 and survives the kernel: its stream never + // goes quiet and never EOFs. + const python = fakeRuntime("#!/bin/sh", "sh -c 'while :; do echo post-mortem noise; done' >&2 &", "exit 42"); + const stderrLogPath = join(tempDir, "artifacts", "kernel-stderr.log"); + await withManager( + python, + async (manager) => { + // Well under the ready timeout: teardown must not wait for the + // grandchild (destroying the pipe kills it with SIGPIPE). + await expect(manager.execute("print(1)")).rejects.toThrow(/Kernel exited before ready/); + expect(statSync(stderrLogPath).mode & 0o777).toBe(0o600); + expect(statSync(join(tempDir, "artifacts")).mode & 0o777).toBe(0o700); + }, + stderrLogPath, + ); + }, + START_FAILURE_TIMEOUT_MS, + ); - try { - // Well under the 30s ready timeout: teardown must not wait for the - // grandchild (destroying the pipe kills it with SIGPIPE). - await expect(manager.execute("print(1)")).rejects.toThrow(/Kernel exited before ready/); - } finally { - errorSpy.mockRestore(); - await manager.shutdown({ snapshot: true, drainHostRequests: true }); - } - }, 15000); + it( + "tightens a rotated kernel stderr log that was world-readable", + async () => { + const stderrLogPath = join(tempDir, "kernel-stderr.log"); + writeFileSync(stderrLogPath, Buffer.alloc(5 * 1024 * 1024 + 1), { mode: 0o644 }); + const python = fakeRuntime("#!/bin/sh", "exit 42"); + await withManager( + python, + async (manager) => { + await expect(manager.execute("print(1)")).rejects.toThrow(/Kernel exited before ready/); + // The rotated file holds the historical exception payloads. + expect(statSync(`${stderrLogPath}.old`).mode & 0o777).toBe(0o600); + expect(statSync(stderrLogPath).mode & 0o777).toBe(0o600); + }, + stderrLogPath, + ); + }, + START_FAILURE_TIMEOUT_MS, + ); it("fails a runtime announcing an unexpected protocol version", async () => { - const python = join(tempDir, "python"); - writeExecutable( - python, - ["#!/bin/sh", `echo '{"event":"ready","protocol":1,"python":"3.13.0"}'`, "exec sleep 60", ""].join("\n"), + const python = fakeRuntime( + "#!/bin/sh", + `echo '{"event":"ready","protocol":1,"python":"3.13.0"}'`, + "exec sleep 60", ); - const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - const manager = new ReplKernelManager({ python, cwd: tempDir }); - - try { + await withManager(python, async (manager) => { await expect(manager.execute("print(1)")).rejects.toThrow(/speaks protocol 1, expected 3/); - } finally { - errorSpy.mockRestore(); - await manager.shutdown({ snapshot: true, drainHostRequests: true }); - } + }); }); it("rejects promptly when the kernel process fails to spawn", async () => { - const python = join(tempDir, "does-not-exist"); - const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - const manager = new ReplKernelManager({ python, cwd: tempDir }); - - try { - // Without prompt rejection this would ride out the 30s ready timeout. + // Without prompt rejection this would ride out the 30s ready timeout. + await withManager(join(tempDir, "does-not-exist"), async (manager) => { await expect(manager.start()).rejects.toThrow(/ENOENT/); - } finally { - errorSpy.mockRestore(); - await manager.shutdown({ snapshot: true, drainHostRequests: true }); - } + }); }); it("times out a runtime that never sends ready", async () => { vi.useFakeTimers(); - const python = join(tempDir, "python"); - writeExecutable(python, ["#!/bin/sh", "exec sleep 120", ""].join("\n")); - const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); - const manager = new ReplKernelManager({ python, cwd: tempDir }); - try { - const startPromise = manager.start(); - const expectation = expect(startPromise).rejects.toThrow(/did not become ready within 30000ms/); - await vi.advanceTimersByTimeAsync(30_000); - // The failure path runs a graceful shutdown bounded by its own deadline. - await vi.advanceTimersByTimeAsync(5_000); - await expectation; + await withManager(fakeRuntime("#!/bin/sh", "exec sleep 120"), async (manager) => { + const startPromise = manager.start(); + const expectation = expect(startPromise).rejects.toThrow(/did not become ready within 30000ms/); + await vi.advanceTimersByTimeAsync(30_000); + // The failure path runs a graceful shutdown bounded by its own deadline. + await vi.advanceTimersByTimeAsync(5_000); + await expectation; + }); } finally { vi.useRealTimers(); - errorSpy.mockRestore(); - await manager.shutdown({ snapshot: true, drainHostRequests: true }); } }); });