Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/orchestrator/agent-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ export async function assembleSystemPrompt(
const env = await detectEnvironment(projectRoot);
const toolNames =
ROLE_TOOLS[role as keyof typeof ROLE_TOOLS] ?? [];
return `${decorated}\n\n${formatEnvironmentContext(env, [...toolNames])}`;
return `${decorated}\n\n${formatEnvironmentContext(env, [...toolNames], projectRoot)}`;
} catch {
// Environment detection is best-effort — never block agent spawn.
return decorated;
Expand Down
53 changes: 53 additions & 0 deletions src/orchestrator/environment.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/**
* Unit tests for formatEnvironmentContext — the markdown block injected into
* agent system prompts. Focus on the project-root + relative-path guidance
* added to stop non-Claude models inventing absolute paths.
*/

import { describe, it, expect } from "vitest";

import { formatEnvironmentContext, type HostEnvironment } from "./environment.js";

const ENV: HostEnvironment = {
platform: "darwin",
osName: "macOS",
osRelease: "25.2.0",
arch: "arm64",
shell: "/bin/zsh",
nodeVersion: "v22.14.0",
packageManager: "npm",
installedTools: ["git", "node", "npm"],
};

const ROOT = "/Users/bober4ik/agent-bober-workspace/agent-bober-ide";

describe("formatEnvironmentContext", () => {
it("surfaces the absolute project root when provided", () => {
const out = formatEnvironmentContext(ENV, ["read_file", "glob"], ROOT);
expect(out).toContain(`- Project root (absolute): ${ROOT}`);
});

it("omits the project-root line when not provided", () => {
const out = formatEnvironmentContext(ENV, ["read_file", "glob"]);
expect(out).not.toContain("Project root (absolute)");
});

it("adds relative-path guidance when a path-bearing tool is present", () => {
const out = formatEnvironmentContext(ENV, ["glob", "grep"], ROOT);
expect(out).toContain("pass paths RELATIVE to the project root");
expect(out).toContain("Do NOT construct absolute");
});

it("omits relative-path guidance when no path tool is available", () => {
const out = formatEnvironmentContext(ENV, ["report"], ROOT);
expect(out).not.toContain("pass paths RELATIVE to the project root");
});

it("still lists the exact tools and host OS", () => {
const out = formatEnvironmentContext(ENV, ["read_file", "glob"], ROOT);
expect(out).toContain("# Host Environment");
expect(out).toContain("- OS: macOS (darwin 25.2.0, arm64)");
expect(out).toContain("# Your Tools");
expect(out).toContain("You have EXACTLY these tools: read_file, glob.");
});
});
26 changes: 22 additions & 4 deletions src/orchestrator/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,14 +139,18 @@ export function resetEnvironmentCache(): void {
* Render the environment + the role's exact harness tools as a markdown block
* for injection into an agent's system prompt.
*
* @param env Detected host environment.
* @param toolNames The exact harness tool names available to this role (e.g.
* ["read_file","glob","grep"]). Used to stop models inventing
* tools (e.g. a non-existent "bash" for read-only roles).
* @param env Detected host environment.
* @param toolNames The exact harness tool names available to this role (e.g.
* ["read_file","glob","grep"]). Used to stop models inventing
* tools (e.g. a non-existent "bash" for read-only roles).
* @param projectRoot Absolute path to the project root. Surfaced so models stop
* guessing it — non-Claude models otherwise invent an absolute
* path with the wrong home dir, which the path sandbox rejects.
*/
export function formatEnvironmentContext(
env: HostEnvironment,
toolNames: string[],
projectRoot?: string,
): string {
const lines: string[] = [];
lines.push("# Host Environment");
Expand All @@ -157,6 +161,7 @@ export function formatEnvironmentContext(
lines.push(`- Node: ${env.nodeVersion}`);
if (env.packageManager)
lines.push(`- Package manager: ${env.packageManager}`);
if (projectRoot) lines.push(`- Project root (absolute): ${projectRoot}`);
if (env.installedTools.length > 0) {
lines.push(`- Installed CLIs on PATH: ${env.installedTools.join(", ")}`);
}
Expand All @@ -180,5 +185,18 @@ export function formatEnvironmentContext(
`state that in your output rather than inventing a tool.`,
);

const hasPathTool = toolNames.some((t) =>
["read_file", "write_file", "edit_file", "glob", "grep"].includes(t),
);
if (hasPathTool) {
lines.push(
"",
"For file/path tool arguments, pass paths RELATIVE to the project root " +
"(e.g. `src`, `src/index.ts`, `src/**/*.ts`). Do NOT construct absolute " +
"paths — you do not know the real home directory and a wrong guess is " +
"rejected by the path sandbox.",
);
}

return lines.join("\n");
}
76 changes: 76 additions & 0 deletions src/orchestrator/tools/handlers.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/**
* Unit tests for sandboxPath — the path-sandboxing guard shared by the
* read_file / write_file / edit_file / glob / grep tool handlers.
*
* Focus: the re-anchoring recovery for absolute paths invented with the wrong
* home directory (the DeepSeek `/Users/boberik/...` failure mode), without
* widening the sandbox for genuinely-foreign paths.
*/

import { describe, it, expect } from "vitest";
import { resolve } from "node:path";

import { sandboxPath } from "./handlers.js";

const ROOT = "/Users/bober4ik/agent-bober-workspace/agent-bober-ide";

describe("sandboxPath", () => {
it("accepts a relative path and resolves it under the root", () => {
expect(sandboxPath(ROOT, "src")).toBe(resolve(ROOT, "src"));
expect(sandboxPath(ROOT, "src/index.ts")).toBe(
resolve(ROOT, "src/index.ts"),
);
});

it("accepts the root itself (empty relative path)", () => {
expect(sandboxPath(ROOT, ".")).toBe(resolve(ROOT));
expect(sandboxPath(ROOT, ROOT)).toBe(resolve(ROOT));
});

it("accepts a correct absolute path inside the root", () => {
const abs = `${ROOT}/src/app.ts`;
expect(sandboxPath(ROOT, abs)).toBe(resolve(abs));
});

it("re-anchors an absolute path invented with the wrong home dir", () => {
// DeepSeek hallucinated `boberik` instead of the real `bober4ik`.
const bogus =
"/Users/boberik/agent-bober-workspace/agent-bober-ide/src";
expect(sandboxPath(ROOT, bogus)).toBe(resolve(ROOT, "src"));
});

it("re-anchors a deep wrong-home absolute path to the right suffix", () => {
const bogus =
"/home/ci/agent-bober-ide/src/components/Button.tsx";
expect(sandboxPath(ROOT, bogus)).toBe(
resolve(ROOT, "src/components/Button.tsx"),
);
});

it("re-anchors a wrong-home path that points at the root itself", () => {
const bogus = "/Users/boberik/agent-bober-workspace/agent-bober-ide";
expect(sandboxPath(ROOT, bogus)).toBe(resolve(ROOT));
});

it("still blocks a relative traversal escaping the root", () => {
expect(() => sandboxPath(ROOT, "../../etc/passwd")).toThrow(
/outside the project root/,
);
});

it("still blocks a genuinely-foreign absolute path (no root basename)", () => {
expect(() => sandboxPath(ROOT, "/etc/passwd")).toThrow(
/outside the project root/,
);
});

it("re-anchoring never widens the sandbox: suffix stays inside root", () => {
// Even a malicious suffix after the root name cannot escape, because the
// re-anchored path is re-validated against the root.
const sneaky =
"/Users/attacker/agent-bober-ide/../../../../etc/passwd";
expect(() => sandboxPath(ROOT, sneaky)).toThrow(
/outside the project root/,
);
});
});
35 changes: 31 additions & 4 deletions src/orchestrator/tools/handlers.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { readFile, writeFile, mkdir } from "node:fs/promises";
import { resolve, relative, isAbsolute } from "node:path";
import { resolve, relative, isAbsolute, basename, sep } from "node:path";
import { execa } from "execa";

// ── Constants ──────────────────────────────────────────────────────
Expand Down Expand Up @@ -27,14 +27,41 @@ export type ToolHandler = (
/**
* Resolve and validate a file path, ensuring it stays within projectRoot.
* Returns the absolute path or throws if the path escapes the sandbox.
*
* Re-anchoring recovery: non-Claude models (e.g. DeepSeek) sometimes ignore the
* "relative to project root" contract and invent an ABSOLUTE path with the wrong
* home dir — e.g. `/Users/boberik/.../agent-bober-ide/src` when the real root is
* `/Users/bober4ik/.../agent-bober-ide`. When such a path lands outside the root
* but still contains the root's basename, we re-anchor the suffix after it
* (`src`) relative to the real root and retry. This NEVER widens the sandbox: the
* re-anchored path is `resolve(projectRoot, suffix)` and is re-validated to be
* inside the root before use, so a genuinely-foreign path still fails closed.
*/
function sandboxPath(projectRoot: string, inputPath: string): string {
const abs = isAbsolute(inputPath)
export function sandboxPath(projectRoot: string, inputPath: string): string {
let abs = isAbsolute(inputPath)
? resolve(inputPath)
: resolve(projectRoot, inputPath);

let rel = relative(projectRoot, abs);

if ((rel.startsWith("..") || isAbsolute(rel)) && isAbsolute(inputPath)) {
const rootName = basename(projectRoot);
if (rootName) {
const segments = abs.split(sep);
const idx = segments.lastIndexOf(rootName);
if (idx !== -1) {
const suffix = segments.slice(idx + 1).join(sep);
const reAnchored = resolve(projectRoot, suffix);
const reRel = relative(projectRoot, reAnchored);
if (!reRel.startsWith("..") && !isAbsolute(reRel)) {
abs = reAnchored;
rel = reRel;
}
}
}
}

// Ensure the resolved path is within the project root
const rel = relative(projectRoot, abs);
if (rel.startsWith("..") || isAbsolute(rel)) {
throw new Error(
`Path "${inputPath}" resolves outside the project root. Access denied.`,
Expand Down
10 changes: 5 additions & 5 deletions src/orchestrator/tools/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export const readFileTool: ToolDef = {
file_path: {
type: "string",
description:
"Path to the file, relative to the project root or absolute.",
"Path to the file, relative to the project root (e.g. `src/index.ts`). Pass a relative path, not an absolute one.",
},
offset: {
type: "number",
Expand All @@ -62,7 +62,7 @@ export const writeFileTool: ToolDef = {
file_path: {
type: "string",
description:
"Path to the file, relative to the project root or absolute.",
"Path to the file, relative to the project root (e.g. `src/index.ts`). Pass a relative path, not an absolute one.",
},
content: {
type: "string",
Expand All @@ -83,7 +83,7 @@ export const editFileTool: ToolDef = {
file_path: {
type: "string",
description:
"Path to the file, relative to the project root or absolute.",
"Path to the file, relative to the project root (e.g. `src/index.ts`). Pass a relative path, not an absolute one.",
},
old_text: {
type: "string",
Expand Down Expand Up @@ -113,7 +113,7 @@ export const globTool: ToolDef = {
path: {
type: "string",
description:
"Directory to search in, relative to project root. Defaults to project root.",
"Directory to search in, relative to project root (e.g. `src`). Pass a relative path, not an absolute one. Defaults to project root.",
},
},
required: ["pattern"],
Expand All @@ -134,7 +134,7 @@ export const grepTool: ToolDef = {
path: {
type: "string",
description:
"File or directory to search in, relative to project root. Defaults to project root.",
"File or directory to search in, relative to project root (e.g. `src`). Pass a relative path, not an absolute one. Defaults to project root.",
},
glob: {
type: "string",
Expand Down
Loading