Skip to content

Commit 242188b

Browse files
steipetesteipete
committed
refactor: unify boundary-safe reads for bootstrap and includes
1 parent 199ef9f commit 242188b

8 files changed

Lines changed: 373 additions & 99 deletions

File tree

‎src/agents/sandbox/fs-bridge.ts‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
1-
import {
2-
assertNoPathAliasEscape,
3-
PATH_ALIAS_POLICIES,
4-
type PathAliasPolicy,
5-
} from "../../infra/path-alias-guards.js";
1+
import fs from "node:fs";
2+
import { openBoundaryFile } from "../../infra/boundary-file-read.js";
3+
import { PATH_ALIAS_POLICIES, type PathAliasPolicy } from "../../infra/path-alias-guards.js";
64
import { execDockerRaw, type ExecDockerRawResult } from "./docker.js";
75
import {
86
buildSandboxFsMounts,
@@ -24,6 +22,7 @@ type PathSafetyOptions = {
2422
action: string;
2523
aliasPolicy?: PathAliasPolicy;
2624
requireWritable?: boolean;
25+
allowMissingTarget?: boolean;
2726
};
2827

2928
export type SandboxResolvedPath = {
@@ -254,12 +253,23 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
254253
);
255254
}
256255

257-
await assertNoPathAliasEscape({
256+
const guarded = await openBoundaryFile({
258257
absolutePath: target.hostPath,
259258
rootPath: lexicalMount.hostRoot,
260259
boundaryLabel: "sandbox mount root",
261-
policy: options.aliasPolicy,
260+
aliasPolicy: options.aliasPolicy,
262261
});
262+
if (!guarded.ok) {
263+
if (guarded.reason !== "path" || options.allowMissingTarget === false) {
264+
throw guarded.error instanceof Error
265+
? guarded.error
266+
: new Error(
267+
`Sandbox boundary checks failed; cannot ${options.action}: ${target.containerPath}`,
268+
);
269+
}
270+
} else {
271+
fs.closeSync(guarded.fd);
272+
}
263273

264274
const canonicalContainerPath = await this.resolveCanonicalContainerPath({
265275
containerPath: target.containerPath,

‎src/agents/workspace.bootstrap-cache.test.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,34 @@ describe("workspace bootstrap file caching", () => {
7474
expectAgentsContent(agentsFile2, content2);
7575
});
7676

77+
it("invalidates cache when inode changes with same mtime", async () => {
78+
if (process.platform === "win32") {
79+
return;
80+
}
81+
const content1 = "# old-content";
82+
const content2 = "# new-content";
83+
const filePath = path.join(workspaceDir, DEFAULT_AGENTS_FILENAME);
84+
const tempPath = path.join(workspaceDir, ".AGENTS.tmp");
85+
86+
await writeWorkspaceFile({
87+
dir: workspaceDir,
88+
name: DEFAULT_AGENTS_FILENAME,
89+
content: content1,
90+
});
91+
const originalStat = await fs.stat(filePath);
92+
93+
const agentsFile1 = await loadAgentsFile(workspaceDir);
94+
expectAgentsContent(agentsFile1, content1);
95+
96+
await fs.writeFile(tempPath, content2, "utf-8");
97+
await fs.utimes(tempPath, originalStat.atime, originalStat.mtime);
98+
await fs.rename(tempPath, filePath);
99+
await fs.utimes(filePath, originalStat.atime, originalStat.mtime);
100+
101+
const agentsFile2 = await loadAgentsFile(workspaceDir);
102+
expectAgentsContent(agentsFile2, content2);
103+
});
104+
77105
it("handles file deletion gracefully", async () => {
78106
const content = "# Some content";
79107
const filePath = path.join(workspaceDir, DEFAULT_AGENTS_FILENAME);

‎src/agents/workspace.load-extra-bootstrap-files.test.ts‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import fs from "node:fs/promises";
22
import os from "node:os";
33
import path from "node:path";
44
import { afterAll, beforeAll, describe, expect, it } from "vitest";
5-
import { loadExtraBootstrapFiles } from "./workspace.js";
5+
import { loadExtraBootstrapFiles, loadExtraBootstrapFilesWithDiagnostics } from "./workspace.js";
66

77
describe("loadExtraBootstrapFiles", () => {
88
let fixtureRoot = "";
@@ -69,4 +69,43 @@ describe("loadExtraBootstrapFiles", () => {
6969
expect(files[0]?.name).toBe("AGENTS.md");
7070
expect(files[0]?.content).toBe("linked agents");
7171
});
72+
73+
it("rejects hardlinked aliases to files outside workspace", async () => {
74+
if (process.platform === "win32") {
75+
return;
76+
}
77+
78+
const rootDir = await createWorkspaceDir("hardlink");
79+
const workspaceDir = path.join(rootDir, "workspace");
80+
const outsideDir = path.join(rootDir, "outside");
81+
await fs.mkdir(workspaceDir, { recursive: true });
82+
await fs.mkdir(outsideDir, { recursive: true });
83+
const outsideFile = path.join(outsideDir, "AGENTS.md");
84+
const linkedFile = path.join(workspaceDir, "AGENTS.md");
85+
await fs.writeFile(outsideFile, "outside", "utf-8");
86+
try {
87+
await fs.link(outsideFile, linkedFile);
88+
} catch (err) {
89+
if ((err as NodeJS.ErrnoException).code === "EXDEV") {
90+
return;
91+
}
92+
throw err;
93+
}
94+
95+
const files = await loadExtraBootstrapFiles(workspaceDir, ["AGENTS.md"]);
96+
expect(files).toHaveLength(0);
97+
});
98+
99+
it("skips oversized bootstrap files and reports diagnostics", async () => {
100+
const workspaceDir = await createWorkspaceDir("oversized");
101+
const payload = "x".repeat(2 * 1024 * 1024 + 1);
102+
await fs.writeFile(path.join(workspaceDir, "AGENTS.md"), payload, "utf-8");
103+
104+
const { files, diagnostics } = await loadExtraBootstrapFilesWithDiagnostics(workspaceDir, [
105+
"AGENTS.md",
106+
]);
107+
108+
expect(files).toHaveLength(0);
109+
expect(diagnostics.some((d) => d.reason === "security")).toBe(true);
110+
});
72111
});

‎src/agents/workspace.test.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import fs from "node:fs/promises";
2+
import os from "node:os";
23
import path from "node:path";
34
import { describe, expect, it } from "vitest";
45
import { makeTempWorkspace, writeWorkspaceFile } from "../test-helpers/workspace.js";
@@ -142,6 +143,37 @@ describe("loadWorkspaceBootstrapFiles", () => {
142143
const files = await loadWorkspaceBootstrapFiles(tempDir);
143144
expect(getMemoryEntries(files)).toHaveLength(0);
144145
});
146+
147+
it("treats hardlinked bootstrap aliases as missing", async () => {
148+
if (process.platform === "win32") {
149+
return;
150+
}
151+
const rootDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-workspace-hardlink-"));
152+
try {
153+
const workspaceDir = path.join(rootDir, "workspace");
154+
const outsideDir = path.join(rootDir, "outside");
155+
await fs.mkdir(workspaceDir, { recursive: true });
156+
await fs.mkdir(outsideDir, { recursive: true });
157+
const outsideFile = path.join(outsideDir, DEFAULT_AGENTS_FILENAME);
158+
const linkPath = path.join(workspaceDir, DEFAULT_AGENTS_FILENAME);
159+
await fs.writeFile(outsideFile, "outside", "utf-8");
160+
try {
161+
await fs.link(outsideFile, linkPath);
162+
} catch (err) {
163+
if ((err as NodeJS.ErrnoException).code === "EXDEV") {
164+
return;
165+
}
166+
throw err;
167+
}
168+
169+
const files = await loadWorkspaceBootstrapFiles(workspaceDir);
170+
const agents = files.find((file) => file.name === DEFAULT_AGENTS_FILENAME);
171+
expect(agents?.missing).toBe(true);
172+
expect(agents?.content).toBeUndefined();
173+
} finally {
174+
await fs.rm(rootDir, { recursive: true, force: true });
175+
}
176+
});
145177
});
146178

147179
describe("filterBootstrapFilesForSession", () => {

0 commit comments

Comments
 (0)