forked from EduCraft/curriculum-project-hub
fix: allow arbitrary workspace file delivery
This commit is contained in:
@@ -1,4 +1,4 @@
|
|||||||
import { extname, isAbsolute, join } from "node:path";
|
import { isAbsolute, join, relative, resolve, sep } from "node:path";
|
||||||
import {
|
import {
|
||||||
WorkspaceFileBoundaryError,
|
WorkspaceFileBoundaryError,
|
||||||
readWorkspaceFileNoFollow,
|
readWorkspaceFileNoFollow,
|
||||||
@@ -10,39 +10,21 @@ export interface DeliverableFile {
|
|||||||
readonly data: Buffer;
|
readonly data: Buffer;
|
||||||
}
|
}
|
||||||
|
|
||||||
const DELIVERABLE_EXTENSIONS = new Set([
|
const DELIVERABLE_CPH_DIRECTORY = "inbox";
|
||||||
".csv",
|
|
||||||
".doc",
|
|
||||||
".docx",
|
|
||||||
".gif",
|
|
||||||
".jpeg",
|
|
||||||
".jpg",
|
|
||||||
".md",
|
|
||||||
".pdf",
|
|
||||||
".png",
|
|
||||||
".ppt",
|
|
||||||
".pptx",
|
|
||||||
".svg",
|
|
||||||
".txt",
|
|
||||||
".typ",
|
|
||||||
".xls",
|
|
||||||
".xlsx",
|
|
||||||
".zip",
|
|
||||||
]);
|
|
||||||
|
|
||||||
export async function resolveDeliverableFile(
|
export async function resolveDeliverableFile(
|
||||||
requestedPath: string,
|
requestedPath: string,
|
||||||
workspaceRoot: string,
|
workspaceRoot: string,
|
||||||
workspaceDir: string,
|
workspaceDir: string,
|
||||||
|
maxBytes?: number,
|
||||||
): Promise<DeliverableFile | null> {
|
): Promise<DeliverableFile | null> {
|
||||||
const token = requestedPath.trim();
|
const token = requestedPath.trim();
|
||||||
if (token === "" || !DELIVERABLE_EXTENSIONS.has(extname(token).toLowerCase())) {
|
if (token === "") return null;
|
||||||
return null;
|
|
||||||
}
|
|
||||||
|
|
||||||
for (const candidate of candidatePaths(token)) {
|
for (const candidate of candidatePaths(token)) {
|
||||||
|
assertNotPlatformRuntimePath(candidate, workspaceDir);
|
||||||
try {
|
try {
|
||||||
return await readWorkspaceFileNoFollow(workspaceRoot, workspaceDir, candidate);
|
return await readWorkspaceFileNoFollow(workspaceRoot, workspaceDir, candidate, maxBytes);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
if (!(error instanceof WorkspaceFileBoundaryError) || error.reason !== "not_found") {
|
if (!(error instanceof WorkspaceFileBoundaryError) || error.reason !== "not_found") {
|
||||||
throw error;
|
throw error;
|
||||||
@@ -54,6 +36,20 @@ export async function resolveDeliverableFile(
|
|||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function assertNotPlatformRuntimePath(candidate: string, workspaceDir: string): void {
|
||||||
|
const workspace = resolve(workspaceDir);
|
||||||
|
const target = isAbsolute(candidate) ? resolve(candidate) : resolve(workspace, candidate);
|
||||||
|
const rel = relative(workspace, target);
|
||||||
|
const components = rel.split(sep);
|
||||||
|
if (components[0] === ".cph" && components[1] !== DELIVERABLE_CPH_DIRECTORY) {
|
||||||
|
throw new WorkspaceFileBoundaryError(
|
||||||
|
`platform runtime files cannot be delivered: ${candidate}`,
|
||||||
|
candidate,
|
||||||
|
"boundary",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
function candidatePaths(token: string): string[] {
|
function candidatePaths(token: string): string[] {
|
||||||
if (isAbsolute(token)) return [token];
|
if (isAbsolute(token)) return [token];
|
||||||
const candidates = [token];
|
const candidates = [token];
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ export interface FileDeliveryToolOptions {
|
|||||||
readonly runId: string;
|
readonly runId: string;
|
||||||
readonly workspaceRoot?: string | undefined;
|
readonly workspaceRoot?: string | undefined;
|
||||||
readonly workspaceDir: string;
|
readonly workspaceDir: string;
|
||||||
|
readonly maxFileBytes?: number | undefined;
|
||||||
readonly sendOptions?: SendMessageOptions | undefined;
|
readonly sendOptions?: SendMessageOptions | undefined;
|
||||||
readonly approvalManager: ApprovalManager;
|
readonly approvalManager: ApprovalManager;
|
||||||
readonly onDelivered?: (path: string) => void;
|
readonly onDelivered?: (path: string) => void;
|
||||||
@@ -29,7 +30,7 @@ export function createFileDeliveryMcpServer(options: FileDeliveryToolOptions): M
|
|||||||
tools.push(
|
tools.push(
|
||||||
tool(
|
tool(
|
||||||
"send_file",
|
"send_file",
|
||||||
"Upload an existing file from the current project workspace to the current Feishu chat. The path must point to a concrete no-symlink file, for example build/student.pdf or README.md.",
|
"Upload any existing regular file from the current project workspace to the current Feishu chat. The path must point to a concrete no-symlink file and must not be inside platform runtime directories.",
|
||||||
{
|
{
|
||||||
path: z.string().describe("Workspace-relative path, or an absolute path physically inside the current workspace."),
|
path: z.string().describe("Workspace-relative path, or an absolute path physically inside the current workspace."),
|
||||||
name: z.string().optional().describe("Optional display filename. Defaults to the file's basename."),
|
name: z.string().optional().describe("Optional display filename. Defaults to the file's basename."),
|
||||||
@@ -46,12 +47,18 @@ export function createFileDeliveryMcpServer(options: FileDeliveryToolOptions): M
|
|||||||
content: [{ type: "text", text: "File delivery is unavailable because workspace isolation is not configured." }],
|
content: [{ type: "text", text: "File delivery is unavailable because workspace isolation is not configured." }],
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
if (options.maxFileBytes === undefined) {
|
||||||
|
throw new Error("Agent file delivery requires a configured maximum file size");
|
||||||
|
}
|
||||||
let file;
|
let file;
|
||||||
try {
|
try {
|
||||||
file = await resolveDeliverableFile(args.path, workspaceRoot, options.workspaceDir);
|
file = await resolveDeliverableFile(args.path, workspaceRoot, options.workspaceDir, options.maxFileBytes);
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
const boundaryViolation = error instanceof WorkspaceFileBoundaryError && error.reason === "boundary";
|
const boundaryViolation = error instanceof WorkspaceFileBoundaryError && error.reason === "boundary";
|
||||||
const log = boundaryViolation ? options.rt.logger.warn.bind(options.rt.logger) : options.rt.logger.error.bind(options.rt.logger);
|
const limitViolation = error instanceof WorkspaceFileBoundaryError && error.reason === "limit";
|
||||||
|
const log = boundaryViolation || limitViolation
|
||||||
|
? options.rt.logger.warn.bind(options.rt.logger)
|
||||||
|
: options.rt.logger.error.bind(options.rt.logger);
|
||||||
log(
|
log(
|
||||||
{
|
{
|
||||||
runId: options.runId,
|
runId: options.runId,
|
||||||
@@ -61,12 +68,20 @@ export function createFileDeliveryMcpServer(options: FileDeliveryToolOptions): M
|
|||||||
},
|
},
|
||||||
boundaryViolation
|
boundaryViolation
|
||||||
? "Agent file delivery refused by workspace boundary"
|
? "Agent file delivery refused by workspace boundary"
|
||||||
: "Agent file delivery failed during workspace file access",
|
: limitViolation
|
||||||
|
? "Agent file delivery refused by file size limit"
|
||||||
|
: "Agent file delivery failed during workspace file access",
|
||||||
);
|
);
|
||||||
|
if (limitViolation) {
|
||||||
|
return {
|
||||||
|
isError: true,
|
||||||
|
content: [{ type: "text", text: `File exceeds the configured delivery limit: ${args.path}` }],
|
||||||
|
};
|
||||||
|
}
|
||||||
if (!boundaryViolation) throw error;
|
if (!boundaryViolation) throw error;
|
||||||
return {
|
return {
|
||||||
isError: true,
|
isError: true,
|
||||||
content: [{ type: "text", text: "File path is outside the current workspace or uses a symlink." }],
|
content: [{ type: "text", text: "File path is outside the current workspace, belongs to platform runtime, or uses a symlink." }],
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
if (file === null) {
|
if (file === null) {
|
||||||
|
|||||||
@@ -444,6 +444,7 @@ export function makeTriggerHandler(deps: TriggerDeps): TriggerHandler {
|
|||||||
runId: run.id,
|
runId: run.id,
|
||||||
workspaceRoot: projectWorkspaceRoot,
|
workspaceRoot: projectWorkspaceRoot,
|
||||||
workspaceDir: project.workspaceDir,
|
workspaceDir: project.workspaceDir,
|
||||||
|
maxFileBytes: deps.resourceLimits?.maxBytesPerFile,
|
||||||
sendOptions,
|
sendOptions,
|
||||||
approvalManager,
|
approvalManager,
|
||||||
tools: cphHubMcpToolsForRole(roleTools),
|
tools: cphHubMcpToolsForRole(roleTools),
|
||||||
|
|||||||
@@ -37,6 +37,7 @@ export async function readWorkspaceFileNoFollow(
|
|||||||
workspaceRoot: string,
|
workspaceRoot: string,
|
||||||
workspaceDir: string,
|
workspaceDir: string,
|
||||||
requestedPath: string,
|
requestedPath: string,
|
||||||
|
maxBytes?: number,
|
||||||
): Promise<WorkspaceFileSnapshot> {
|
): Promise<WorkspaceFileSnapshot> {
|
||||||
const workspace = await canonicalWorkspace(workspaceRoot, workspaceDir);
|
const workspace = await canonicalWorkspace(workspaceRoot, workspaceDir);
|
||||||
const components = fileComponents(workspace, requestedPath);
|
const components = fileComponents(workspace, requestedPath);
|
||||||
@@ -53,8 +54,15 @@ export async function readWorkspaceFileNoFollow(
|
|||||||
if (!metadata.isFile()) {
|
if (!metadata.isFile()) {
|
||||||
throw new WorkspaceFileBoundaryError(`deliverable is not a regular file: ${requestedPath}`, requestedPath);
|
throw new WorkspaceFileBoundaryError(`deliverable is not a regular file: ${requestedPath}`, requestedPath);
|
||||||
}
|
}
|
||||||
|
if (maxBytes !== undefined && metadata.size > maxBytes) {
|
||||||
|
throw new WorkspaceFileBoundaryError(
|
||||||
|
`workspace file exceeds ${maxBytes} bytes: ${requestedPath}`,
|
||||||
|
requestedPath,
|
||||||
|
"limit",
|
||||||
|
);
|
||||||
|
}
|
||||||
await assertNameStillReferences(parent, name, metadata.dev, metadata.ino, requestedPath);
|
await assertNameStillReferences(parent, name, metadata.dev, metadata.ino, requestedPath);
|
||||||
const data = await file.readFile();
|
const data = await readFileSnapshot(file, maxBytes, requestedPath);
|
||||||
result = { path: join(workspace, ...components), name, data };
|
result = { path: join(workspace, ...components), name, data };
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
failure = boundaryError(error, requestedPath, "cannot read workspace file without following symlinks");
|
failure = boundaryError(error, requestedPath, "cannot read workspace file without following symlinks");
|
||||||
@@ -67,6 +75,33 @@ export async function readWorkspaceFileNoFollow(
|
|||||||
return result!;
|
return result!;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async function readFileSnapshot(
|
||||||
|
file: FileHandle,
|
||||||
|
maxBytes: number | undefined,
|
||||||
|
requestedPath: string,
|
||||||
|
): Promise<Buffer> {
|
||||||
|
if (maxBytes === undefined) return file.readFile();
|
||||||
|
const chunks: Buffer[] = [];
|
||||||
|
let total = 0;
|
||||||
|
let position = 0;
|
||||||
|
while (true) {
|
||||||
|
const remainingWithSentinel = maxBytes - total + 1;
|
||||||
|
const chunk = Buffer.allocUnsafe(Math.min(64 * 1024, remainingWithSentinel));
|
||||||
|
const { bytesRead } = await file.read(chunk, 0, chunk.length, position);
|
||||||
|
if (bytesRead === 0) return Buffer.concat(chunks, total);
|
||||||
|
total += bytesRead;
|
||||||
|
if (total > maxBytes) {
|
||||||
|
throw new WorkspaceFileBoundaryError(
|
||||||
|
`workspace file exceeds ${maxBytes} bytes: ${requestedPath}`,
|
||||||
|
requestedPath,
|
||||||
|
"limit",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
chunks.push(chunk.subarray(0, bytesRead));
|
||||||
|
position += bytesRead;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Create one inbound file exclusively below the workspace and stream into its
|
* Create one inbound file exclusively below the workspace and stream into its
|
||||||
* already-open descriptor. Linux uses /proc/self/fd-relative traversal so a
|
* already-open descriptor. Linux uses /proc/self/fd-relative traversal so a
|
||||||
|
|||||||
@@ -34,6 +34,65 @@ describe("file delivery path resolution", () => {
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
itOnLinux("accepts arbitrary extensions and extensionless workspace files", async () => {
|
||||||
|
const root = await makeRepo();
|
||||||
|
try {
|
||||||
|
const workspace = join(root, "examples", "TH-141");
|
||||||
|
await writeFile(join(workspace, "lesson.json"), "{\"ok\":true}\n");
|
||||||
|
await writeFile(join(workspace, "Makefile"), "all:\n\t@true\n");
|
||||||
|
|
||||||
|
await expect(resolveDeliverableFile("lesson.json", join(root, "examples"), workspace))
|
||||||
|
.resolves.toMatchObject({ name: "lesson.json" });
|
||||||
|
await expect(resolveDeliverableFile("Makefile", join(root, "examples"), workspace))
|
||||||
|
.resolves.toMatchObject({ name: "Makefile" });
|
||||||
|
} finally {
|
||||||
|
await rm(root, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
itOnLinux("allows the Feishu inbox but refuses other platform runtime files", async () => {
|
||||||
|
const root = await makeRepo();
|
||||||
|
try {
|
||||||
|
const workspace = join(root, "examples", "TH-141");
|
||||||
|
await mkdir(join(workspace, ".cph", "agent-runtime"), { recursive: true });
|
||||||
|
await mkdir(join(workspace, ".cph", "inbox"), { recursive: true });
|
||||||
|
await writeFile(join(workspace, ".cph", "agent-runtime", "session.jsonl"), "internal\n");
|
||||||
|
await writeFile(join(workspace, ".cph", "denials.log"), "internal\n");
|
||||||
|
await writeFile(join(workspace, ".cph", "inbox", "source.bin"), "source\n");
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
resolveDeliverableFile(".cph/inbox/source.bin", join(root, "examples"), workspace),
|
||||||
|
).resolves.toMatchObject({ name: "source.bin" });
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
resolveDeliverableFile(".cph/agent-runtime/session.jsonl", join(root, "examples"), workspace),
|
||||||
|
).rejects.toMatchObject({ reason: "boundary" });
|
||||||
|
await expect(
|
||||||
|
resolveDeliverableFile(".cph/denials.log", join(root, "examples"), workspace),
|
||||||
|
).rejects.toMatchObject({ reason: "boundary" });
|
||||||
|
} finally {
|
||||||
|
await rm(root, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
itOnLinux("refuses a file larger than the configured delivery limit", async () => {
|
||||||
|
const root = await makeRepo();
|
||||||
|
try {
|
||||||
|
const workspace = join(root, "examples", "TH-141");
|
||||||
|
await writeFile(join(workspace, "exact.bin"), Buffer.alloc(10));
|
||||||
|
await writeFile(join(workspace, "artifact.bin"), Buffer.alloc(11));
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
resolveDeliverableFile("exact.bin", join(root, "examples"), workspace, 10),
|
||||||
|
).resolves.toMatchObject({ name: "exact.bin", data: Buffer.alloc(10) });
|
||||||
|
await expect(
|
||||||
|
resolveDeliverableFile("artifact.bin", join(root, "examples"), workspace, 10),
|
||||||
|
).rejects.toMatchObject({ reason: "limit" });
|
||||||
|
} finally {
|
||||||
|
await rm(root, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
itOnLinux("rejects a deliverable symlink even when its target exists", async () => {
|
itOnLinux("rejects a deliverable symlink even when its target exists", async () => {
|
||||||
const root = await makeRepo();
|
const root = await makeRepo();
|
||||||
try {
|
try {
|
||||||
@@ -50,7 +109,7 @@ describe("file delivery path resolution", () => {
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
it("does not infer files from natural-language prompts", async () => {
|
itOnLinux("does not infer files from natural-language prompts", async () => {
|
||||||
const root = await makeRepo();
|
const root = await makeRepo();
|
||||||
try {
|
try {
|
||||||
const workspace = join(root, "examples", "TH-141");
|
const workspace = join(root, "examples", "TH-141");
|
||||||
|
|||||||
Reference in New Issue
Block a user