refactor(media): share local file access guards

This commit is contained in:
Peter Steinberger
2026-03-23 00:58:19 -07:00
parent eac93507c3
commit dc90d3b1d3
13 changed files with 236 additions and 285 deletions

View File

@@ -1,6 +1,5 @@
import fs from "node:fs/promises";
import path from "node:path";
import { fileURLToPath } from "node:url";
import type { AgentToolResult } from "@mariozechner/pi-agent-core";
import { createEditTool, createReadTool, createWriteTool } from "@mariozechner/pi-coding-agent";
import {
@@ -10,6 +9,7 @@ import {
readFileWithinRoot,
writeFileWithinRoot,
} from "../infra/fs-safe.js";
import { trySafeFileURLToPath } from "../infra/local-file-access.js";
import { detectMime } from "../media/mime.js";
import { sniffMimeFromBase64 } from "../media/sniff-mime-from-base64.js";
import type { ImageSanitizationLimits } from "./image-sanitization.js";
@@ -374,22 +374,11 @@ function mapContainerPathToWorkspaceRoot(params: {
let candidate = params.filePath.startsWith("@") ? params.filePath.slice(1) : params.filePath;
if (/^file:\/\//i.test(candidate)) {
try {
candidate = fileURLToPath(candidate);
} catch {
try {
const parsed = new URL(candidate);
if (parsed.protocol !== "file:") {
return params.filePath;
}
candidate = decodeURIComponent(parsed.pathname || "");
if (!candidate.startsWith("/")) {
return params.filePath;
}
} catch {
return params.filePath;
}
const localFilePath = trySafeFileURLToPath(candidate);
if (!localFilePath) {
return params.filePath;
}
candidate = localFilePath;
}
const normalizedCandidate = candidate.replace(/\\/g, "/");

View File

@@ -1,6 +1,5 @@
import path from "node:path";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { wrapToolWorkspaceRootGuardWithOptions } from "./pi-tools.read.js";
import type { AnyAgentTool } from "./pi-tools.types.js";
const mocks = vi.hoisted(() => ({
@@ -24,14 +23,20 @@ function createToolHarness() {
return { execute, tool };
}
async function loadModule() {
return await import("./pi-tools.read.js");
}
describe("wrapToolWorkspaceRootGuardWithOptions", () => {
const root = "/tmp/root";
beforeEach(() => {
mocks.assertSandboxPath.mockClear();
vi.resetModules();
});
it("maps container workspace paths to host workspace root", async () => {
const { wrapToolWorkspaceRootGuardWithOptions } = await loadModule();
const { tool } = createToolHarness();
const wrapped = wrapToolWorkspaceRootGuardWithOptions(tool, root, {
containerWorkdir: "/workspace",
@@ -47,6 +52,7 @@ describe("wrapToolWorkspaceRootGuardWithOptions", () => {
});
it("maps file:// container workspace paths to host workspace root", async () => {
const { wrapToolWorkspaceRootGuardWithOptions } = await loadModule();
const { tool } = createToolHarness();
const wrapped = wrapToolWorkspaceRootGuardWithOptions(tool, root, {
containerWorkdir: "/workspace",
@@ -61,7 +67,24 @@ describe("wrapToolWorkspaceRootGuardWithOptions", () => {
});
});
it("does not remap remote-host file:// paths", async () => {
const { wrapToolWorkspaceRootGuardWithOptions } = await loadModule();
const { tool } = createToolHarness();
const wrapped = wrapToolWorkspaceRootGuardWithOptions(tool, root, {
containerWorkdir: "/workspace",
});
await wrapped.execute("tc-remote-file-url", { path: "file://attacker/share/readme.md" });
expect(mocks.assertSandboxPath).toHaveBeenCalledWith({
filePath: "file://attacker/share/readme.md",
cwd: root,
root,
});
});
it("maps @-prefixed container workspace paths to host workspace root", async () => {
const { wrapToolWorkspaceRootGuardWithOptions } = await loadModule();
const { tool } = createToolHarness();
const wrapped = wrapToolWorkspaceRootGuardWithOptions(tool, root, {
containerWorkdir: "/workspace",
@@ -77,6 +100,7 @@ describe("wrapToolWorkspaceRootGuardWithOptions", () => {
});
it("normalizes @-prefixed absolute paths before guard checks", async () => {
const { wrapToolWorkspaceRootGuardWithOptions } = await loadModule();
const { tool } = createToolHarness();
const wrapped = wrapToolWorkspaceRootGuardWithOptions(tool, root, {
containerWorkdir: "/workspace",
@@ -92,6 +116,7 @@ describe("wrapToolWorkspaceRootGuardWithOptions", () => {
});
it("does not remap absolute paths outside the configured container workdir", async () => {
const { wrapToolWorkspaceRootGuardWithOptions } = await loadModule();
const { tool } = createToolHarness();
const wrapped = wrapToolWorkspaceRootGuardWithOptions(tool, root, {
containerWorkdir: "/workspace",