Skip to content

Commit 2efcbee

Browse files
Jing-yilinclaude
andcommitted
Test what an attachment's name resolves to, beside folderOf
The resolution moves to sp.ts as canvasFile, with a test for a board, a video, a shape record, an app canvas, and names that try to leave the folder. Drop the em dashes REVIEW.md rules out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 56d3a8d commit 2efcbee

5 files changed

Lines changed: 65 additions & 29 deletions

File tree

‎canvas/server/agent.ts‎

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import path from "node:path";
1717
import { CANVASES } from "./boards.ts";
1818
import { command, stop } from "./command.ts";
1919
import { AGENT_SKILLS, installSkills } from "./skills.ts";
20-
import { folderOf, sameOrigin } from "./sp.ts";
20+
import { canvasFile, folderOf, sameOrigin } from "./sp.ts";
2121
import { SAFE_NAME } from "../src/layoutEdit.ts";
2222
import {
2323
attach,
@@ -511,22 +511,21 @@ export function createAgentServer(options: {
511511
data: string;
512512
reference?: { project?: string; community?: string };
513513
}) => {
514-
// Anything but a picture — a board, a video, a note — keeps its picture for the
515-
// panel, and the agent is pointed at its file, `<slug>/<path>` in the canvases of
516-
// the project it was attached from, which need not be the one it is sent from. A
517-
// community project's is not on this machine, so it keeps the name `sp fetch`
518-
// finds it by.
519-
const [slug, ...rest] = i.name.split("/");
514+
// Anything but a picture (a board, a video, a note) keeps its picture for the
515+
// panel, and the agent is pointed at its file, in the canvases of the project it
516+
// was attached from, which need not be the one it is sent from. A community
517+
// project's is not on this machine, so it keeps the name `sp fetch` finds it by.
520518
const ref = i.reference;
521519
const from = ref?.project && projects().get(ref.project);
522-
const local =
520+
const file =
523521
ref?.community === undefined &&
524-
(ref?.project === undefined || from) &&
525-
SAFE_NAME.test(slug) &&
526-
rest.length > 0 &&
527-
rest.every(
528-
(s) => s && !s.startsWith(".") && !s.includes("\\"),
529-
);
522+
(ref?.project === undefined || from)
523+
? canvasFile(
524+
from ? path.join(from, CANVASES) : examplesDir,
525+
examplesDir,
526+
i.name,
527+
)
528+
: undefined;
530529
return {
531530
...i,
532531
path: picture(
@@ -537,18 +536,10 @@ export function createAgentServer(options: {
537536
),
538537
reference:
539538
ref &&
540-
(local
541-
? path.join(
542-
folderOf(
543-
from ? path.join(from, CANVASES) : examplesDir,
544-
examplesDir,
545-
slug,
546-
),
547-
...rest,
548-
)
549-
: ref.community
539+
(file ??
540+
(ref.community
550541
? `${i.name} of the community project ${ref.community}`
551-
: i.name),
542+
: i.name)),
552543
};
553544
},
554545
);

‎canvas/server/sp.test.ts‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import os from "node:os";
55
import path from "node:path";
66
import { pathToFileURL } from "node:url";
77
import { expect, it, vi } from "vitest";
8-
import { createSpServer } from "./sp.ts";
8+
import { canvasFile, createSpServer } from "./sp.ts";
99

1010
// The examples directory is listed beside the project's canvases, shadowed by a folder of the
1111
// project's own, refused every write, and cloned into the project.
@@ -522,3 +522,34 @@ async function serve(options: Parameters<typeof createSpServer>[0]) {
522522
};
523523
return { ask, listen, close };
524524
}
525+
526+
// What a chat attachment names is a file in a canvas folder, and nothing a browser sends reaches
527+
// outside one.
528+
it("resolves an attachment's name to a file in its canvas folder", () => {
529+
const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "sp-attach-"));
530+
const own = path.join(tmp, "project");
531+
const examples = path.join(tmp, "examples");
532+
fs.mkdirSync(path.join(examples, "demo"), { recursive: true });
533+
expect(canvasFile(own, examples, "shop/01-home.html")).toBe(
534+
path.join(own, "shop", "01-home.html"),
535+
);
536+
expect(canvasFile(own, examples, "shop/files/clip.mp4")).toBe(
537+
path.join(own, "shop", "files", "clip.mp4"),
538+
);
539+
expect(canvasFile(own, examples, "shop/canvas.json#shape:a1")).toBe(
540+
path.join(own, "shop", "canvas.json#shape:a1"),
541+
);
542+
expect(canvasFile(own, examples, "demo/01-a.html")).toBe(
543+
path.join(examples, "demo", "01-a.html"),
544+
);
545+
for (const name of [
546+
"shop",
547+
"../etc/passwd",
548+
"shop/../../etc",
549+
"shop/files/..",
550+
"shop//x",
551+
"shop/.env",
552+
"shop/a\\..\\..\\x",
553+
])
554+
expect(canvasFile(own, examples, name)).toBeUndefined();
555+
});

‎canvas/server/sp.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,20 @@ export function folderOf(
8080
return !isCanvas && fs.existsSync(example) ? example : own;
8181
}
8282

83+
/**
84+
* The file a chat attachment names as `<slug>/<path>`: a board's HTML, a file under `files/`, or
85+
* `canvas.json#<shape id>`. Undefined for a name that is not one, so nothing a browser sends can
86+
* point outside a canvas folder: no part may be empty, start with a dot, or hold a backslash.
87+
*/
88+
export function canvasFile(canvasesDir: string, examplesDir: string, name: string) {
89+
const [slug, ...rest] = name.split("/");
90+
return SAFE_NAME.test(slug) &&
91+
rest.length > 0 &&
92+
rest.every((s) => s && !s.startsWith(".") && !s.includes("\\"))
93+
? path.join(folderOf(canvasesDir, examplesDir, slug), ...rest)
94+
: undefined;
95+
}
96+
8397
/**
8498
* The project's own settings, beside its canvases: which cover it chose, and the name it is shown
8599
* by when that is not its folder's. A project made without a name is an "Untitled" folder whose

‎canvas/src/agents.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ export interface AgentImage {
8080
/** Where the server wrote it, for an agent that takes files rather than bytes; gone once
8181
* that agent has exited. */
8282
path: string;
83-
/** The file behind it — a board's HTML, a video, a canvas.json record — when the picture is
83+
/** The file behind it (a board's HTML, a video, a canvas.json record) when the picture is
8484
* only the panel's drawing of it: the agent is pointed at the file, which is what it can read
8585
* and change, and not handed the picture. */
8686
reference?: string;

‎canvas/src/canvasAttach.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ async function attach(editor: Editor, target: TLShape) {
5757
return dispatchAttach({ kind: "board", name, src, reference: true });
5858
}
5959
// One of the person's own, drawn for the tile and named by where the agent reads the thing
60-
// itself (canvasContent.ts). A picture goes over as one; anything else — a video, a note, a
61-
// drawing — is a file the agent is pointed at, as a board is.
60+
// itself (canvasContent.ts). A picture goes over as one. Anything else, such as a video, a
61+
// note or a drawing, is a file the agent is pointed at, as a board is.
6262
const slug = personsShape(editor, target);
6363
if (slug) {
6464
const { blob } = await editor.toImage([target.id], { format: "png" });

0 commit comments

Comments
 (0)