fix(runner): validate remote workspace at runner boundary
This commit is contained in:
parent
73b496edad
commit
d7faaafa6d
|
|
@ -6,12 +6,15 @@ import type {
|
|||
} from "../contracts/native-session-backend.js";
|
||||
import type { CodexAppServerTransport } from "../drivers/codex/app-server-transport.js";
|
||||
import { CodexAppServerDriver } from "../drivers/codex/codex-app-server-driver.js";
|
||||
import type { CodexWorkingDirectoryAuthority } from "../drivers/codex/codex-boundaries.js";
|
||||
import { HarnessDriverBackend } from "./harness-driver-backend.js";
|
||||
import { nativeSystemInstructions, nativeTaskConstraints } from "./runtime-context.js";
|
||||
|
||||
export interface CodexNativeSessionBackendOptions {
|
||||
/** Effective provider environment, including the assigned workspace boundary. */
|
||||
environment?: NodeJS.ProcessEnv;
|
||||
/** Filesystem that authoritatively admits the workspace path. */
|
||||
workingDirectoryAuthority?: CodexWorkingDirectoryAuthority;
|
||||
runnerInstanceId?: string;
|
||||
onSpawn?: (meta: {
|
||||
pid: number;
|
||||
|
|
@ -90,6 +93,14 @@ function createTransportBackedNativeSessionBackend(
|
|||
input: NativeExecutionInput,
|
||||
options: CodexNativeSessionBackendOptions,
|
||||
): NativeSessionBackend {
|
||||
if (
|
||||
options.workingDirectoryAuthority === "remote_runner" &&
|
||||
!options.transportFactory
|
||||
) {
|
||||
throw new Error(
|
||||
"Remote runner workspace authority requires a runnerd transport",
|
||||
);
|
||||
}
|
||||
const driverIdentity = transportDriverIdentity(input);
|
||||
const isCodex = input.provider.kind === "codex";
|
||||
const supportsCollaborativePlanning =
|
||||
|
|
@ -147,6 +158,7 @@ function createTransportBackedNativeSessionBackend(
|
|||
dynamicTools: options.dynamicTools,
|
||||
dynamicToolHandler: options.dynamicToolHandler,
|
||||
environment: options.environment,
|
||||
workingDirectoryAuthority: options.workingDirectoryAuthority,
|
||||
driverIdentity,
|
||||
capabilities: isCodex
|
||||
? {}
|
||||
|
|
|
|||
|
|
@ -277,6 +277,62 @@ describe("native backend factory", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("defers remote ACPX workspace admission to the runner filesystem", async () => {
|
||||
const remoteWorkspace = "/home/daytona/paperclip-workspace";
|
||||
const transport = new FakeCodexTransport();
|
||||
const request = transport.request.bind(transport);
|
||||
transport.request = async (method, params) => {
|
||||
const response = await request(method, params);
|
||||
if (method !== "thread/start" && method !== "thread/resume") {
|
||||
return response;
|
||||
}
|
||||
return {
|
||||
...response,
|
||||
cwd: remoteWorkspace,
|
||||
thread: {
|
||||
...(response.thread as Record<string, unknown>),
|
||||
cwd: remoteWorkspace,
|
||||
},
|
||||
};
|
||||
};
|
||||
const backend = createNativeSessionBackend(acpxExecution("claude"), {
|
||||
codexTransportFactory: () => transport,
|
||||
workingDirectoryAuthority: "remote_runner",
|
||||
environment: {
|
||||
HOME: remoteWorkspace,
|
||||
CODEX_HOME: `${remoteWorkspace}/.codex`,
|
||||
PAPERCLIP_WORKSPACE_CWD: remoteWorkspace,
|
||||
},
|
||||
});
|
||||
|
||||
const session = await backend.openSession({
|
||||
identity: {
|
||||
runId: "run",
|
||||
sessionId: "session",
|
||||
companyId: "company",
|
||||
issueId: "issue",
|
||||
agentId: "agent",
|
||||
},
|
||||
workingDirectory: remoteWorkspace,
|
||||
});
|
||||
|
||||
expect(
|
||||
transport.calls.find((call) => call.method === "thread/start")?.params,
|
||||
).toMatchObject({ cwd: remoteWorkspace });
|
||||
await session.close({ reason: "test complete" });
|
||||
});
|
||||
|
||||
it("does not allow remote workspace authority without runnerd", () => {
|
||||
expect(() =>
|
||||
createNativeSessionBackend(execution(), {
|
||||
workingDirectoryAuthority: "remote_runner",
|
||||
environment: {
|
||||
PAPERCLIP_WORKSPACE_CWD: "/home/daytona/paperclip-workspace",
|
||||
},
|
||||
}),
|
||||
).toThrow("requires a runnerd transport");
|
||||
});
|
||||
|
||||
it.each([
|
||||
["OpenCode", opencodeExecution()],
|
||||
["ACPX Codex", acpxExecution("codex")],
|
||||
|
|
|
|||
|
|
@ -47,6 +47,7 @@ export function createNativeSessionBackend(
|
|||
dynamicTools: options.dynamicTools,
|
||||
dynamicToolHandler: options.dynamicToolHandler,
|
||||
environment: options.environment,
|
||||
workingDirectoryAuthority: options.workingDirectoryAuthority,
|
||||
transportFactory: options.codexTransportFactory,
|
||||
});
|
||||
}
|
||||
|
|
@ -100,6 +101,7 @@ export function createNativeSessionBackend(
|
|||
dynamicTools: options.dynamicTools,
|
||||
dynamicToolHandler: options.dynamicToolHandler,
|
||||
environment: options.environment,
|
||||
workingDirectoryAuthority: options.workingDirectoryAuthority,
|
||||
transportFactory: options.codexTransportFactory,
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -199,6 +199,7 @@ export class CodexAppServerDriver implements HarnessDriver {
|
|||
const workingDirectory = validateWorkingDirectory(
|
||||
input.workingDirectory,
|
||||
this.#options.environment,
|
||||
this.#options.workingDirectoryAuthority,
|
||||
);
|
||||
const transport = this.#transport();
|
||||
const cancellation = bootstrapCancellation(transport, input.signal);
|
||||
|
|
@ -326,6 +327,7 @@ export class CodexAppServerDriver implements HarnessDriver {
|
|||
const workingDirectory = validateWorkingDirectory(
|
||||
text(existingThread.cwd),
|
||||
this.#options.environment,
|
||||
this.#options.workingDirectoryAuthority,
|
||||
);
|
||||
const response = await cancellation.wait(transport.request("thread/resume", {
|
||||
threadId: snapshot.driverSessionId,
|
||||
|
|
|
|||
|
|
@ -120,6 +120,47 @@ describe("Codex value and workspace boundaries", () => {
|
|||
}
|
||||
});
|
||||
|
||||
it("defers provider-owned workspace existence without weakening its assignment", () => {
|
||||
const remoteWorkspace = "/home/daytona/paperclip-workspace";
|
||||
const remoteEnvironment = {
|
||||
HOME: remoteWorkspace,
|
||||
CODEX_HOME: `${remoteWorkspace}/.codex`,
|
||||
PAPERCLIP_WORKSPACE_CWD: remoteWorkspace,
|
||||
};
|
||||
|
||||
expect(
|
||||
validateCodexWorkingDirectory(
|
||||
remoteWorkspace,
|
||||
remoteEnvironment,
|
||||
"remote_runner",
|
||||
),
|
||||
).toBe(remoteWorkspace);
|
||||
expect(() =>
|
||||
validateCodexWorkingDirectory(remoteWorkspace, remoteEnvironment),
|
||||
).toThrow("must exist before provider admission");
|
||||
expect(() =>
|
||||
validateCodexWorkingDirectory(
|
||||
`${remoteWorkspace}/nested`,
|
||||
remoteEnvironment,
|
||||
"remote_runner",
|
||||
),
|
||||
).toThrow("does not match the assigned workspace");
|
||||
expect(() =>
|
||||
validateCodexWorkingDirectory(
|
||||
`${remoteWorkspace}/../escape`,
|
||||
remoteEnvironment,
|
||||
"remote_runner",
|
||||
),
|
||||
).toThrow("must be a normalized absolute path");
|
||||
expect(() =>
|
||||
validateCodexWorkingDirectory(
|
||||
"/",
|
||||
{ PAPERCLIP_WORKSPACE_CWD: "/" },
|
||||
"remote_runner",
|
||||
),
|
||||
).toThrow("filesystem root");
|
||||
});
|
||||
|
||||
it("bounds retained values and redacts protected diagnostics", () => {
|
||||
const bounded = boundedCodexPayload({
|
||||
short: "ok",
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import {
|
|||
dirname,
|
||||
isAbsolute,
|
||||
parse,
|
||||
posix,
|
||||
relative,
|
||||
resolve,
|
||||
sep,
|
||||
|
|
@ -28,6 +29,9 @@ const SENSITIVE_HOST_HOME_DIRECTORIES = [
|
|||
".ssh",
|
||||
] as const;
|
||||
|
||||
export type CodexWorkingDirectoryAuthority =
|
||||
"local_filesystem" | "remote_runner";
|
||||
|
||||
function record(value: unknown): Record<string, unknown> {
|
||||
return typeof value === "object" && value !== null && !Array.isArray(value)
|
||||
? (value as Record<string, unknown>)
|
||||
|
|
@ -37,10 +41,14 @@ function record(value: unknown): Record<string, unknown> {
|
|||
export function validateCodexWorkingDirectory(
|
||||
workingDirectory: string,
|
||||
environment: NodeJS.ProcessEnv = process.env,
|
||||
authority: CodexWorkingDirectoryAuthority = "local_filesystem",
|
||||
): string {
|
||||
if (workingDirectory.trim().length === 0) {
|
||||
throw new Error("Codex working directory is required");
|
||||
}
|
||||
if (authority === "remote_runner") {
|
||||
return validateRemoteRunnerWorkingDirectory(workingDirectory, environment);
|
||||
}
|
||||
const requested = resolve(workingDirectory);
|
||||
let resolved: string;
|
||||
try {
|
||||
|
|
@ -109,6 +117,47 @@ export function validateCodexWorkingDirectory(
|
|||
return resolved;
|
||||
}
|
||||
|
||||
function validateRemoteRunnerWorkingDirectory(
|
||||
workingDirectory: string,
|
||||
environment: NodeJS.ProcessEnv,
|
||||
): string {
|
||||
if (
|
||||
!posix.isAbsolute(workingDirectory) ||
|
||||
posix.normalize(workingDirectory) !== workingDirectory ||
|
||||
/[\u0000-\u001f\u007f]/u.test(workingDirectory)
|
||||
) {
|
||||
throw new Error(
|
||||
"Remote Codex working directory must be a normalized absolute path",
|
||||
);
|
||||
}
|
||||
if (workingDirectory === posix.parse(workingDirectory).root) {
|
||||
throw new Error("Codex working directory cannot be a filesystem root");
|
||||
}
|
||||
const configuredRoot = environment.PAPERCLIP_WORKSPACE_CWD?.trim();
|
||||
if (!configuredRoot) {
|
||||
throw new Error(
|
||||
"Remote Codex working directory requires an assigned workspace",
|
||||
);
|
||||
}
|
||||
if (
|
||||
!posix.isAbsolute(configuredRoot) ||
|
||||
posix.normalize(configuredRoot) !== configuredRoot
|
||||
) {
|
||||
throw new Error(
|
||||
"Assigned remote workspace must be a normalized absolute path",
|
||||
);
|
||||
}
|
||||
// The controller cannot inspect a provider-owned filesystem. Pin the facade
|
||||
// to the exact remote workspace while runnerd validates existence, type, and
|
||||
// canonical identity inside the authoritative filesystem before launch.
|
||||
if (workingDirectory !== configuredRoot) {
|
||||
throw new Error(
|
||||
"Remote Codex working directory does not match the assigned workspace",
|
||||
);
|
||||
}
|
||||
return workingDirectory;
|
||||
}
|
||||
|
||||
function canonicalConfiguredPath(value: string | undefined): string | null {
|
||||
const configured = value?.trim();
|
||||
if (!configured) return null;
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import type {
|
|||
CodexTaskEnvelope,
|
||||
} from "../../contracts/codex.js";
|
||||
import type { CodexAppServerTransport } from "./app-server-transport.js";
|
||||
import type { CodexWorkingDirectoryAuthority } from "./codex-boundaries.js";
|
||||
import type { CodexQuestionResponseContext } from "./codex-question-adapter.js";
|
||||
|
||||
export interface CodexAppServerDriverOptions {
|
||||
|
|
@ -40,6 +41,8 @@ export interface CodexAppServerDriverOptions {
|
|||
arguments: unknown;
|
||||
}) => Promise<unknown>;
|
||||
environment?: NodeJS.ProcessEnv;
|
||||
/** Filesystem that authoritatively admits the workspace path. */
|
||||
workingDirectoryAuthority?: CodexWorkingDirectoryAuthority;
|
||||
now?: () => Date;
|
||||
runnerInstanceId?: string;
|
||||
onDiagnostic?: (message: string) => void;
|
||||
|
|
|
|||
|
|
@ -31,6 +31,7 @@ import { nativeRuntimeContextFixture } from "./runtime-context.test-fixture.js";
|
|||
type BackendFactoryOptions = {
|
||||
runnerInstanceId?: string;
|
||||
acpxRuntimeDirectory?: string;
|
||||
workingDirectoryAuthority?: "local_filesystem" | "remote_runner";
|
||||
codexTransportFactory?: () => unknown;
|
||||
dynamicToolHandler?: (call: unknown) => Promise<unknown>;
|
||||
onSpawn?: (meta: {
|
||||
|
|
@ -4601,7 +4602,9 @@ describe("runnerd provider runtime wiring", () => {
|
|||
expect.objectContaining({
|
||||
workspace: expect.objectContaining({ cwd: remoteCwd }),
|
||||
}),
|
||||
expect.any(Object),
|
||||
expect.objectContaining({
|
||||
workingDirectoryAuthority: "remote_runner",
|
||||
}),
|
||||
);
|
||||
expect(state.execute).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
|
|
|
|||
|
|
@ -6549,6 +6549,9 @@ async function createRunnerdBackendWithinSessionClaim(
|
|||
const backend = createNativeSessionBackend(runnerExecution, {
|
||||
runnerInstanceId: input.runnerInstanceId,
|
||||
environment: effectiveRunnerEnvironment,
|
||||
workingDirectoryAuthority: remoteTarget
|
||||
? "remote_runner"
|
||||
: "local_filesystem",
|
||||
onSpawn: input.onSpawn,
|
||||
dynamicTools,
|
||||
dynamicToolHandler: (call) => authorityEpoch.execute(call),
|
||||
|
|
|
|||
Loading…
Reference in New Issue