Archived
refactor(plugin-api): trim plugin API scope to grounded capabilities
Builds on marcus's plugin-api-completeness work. Narrows the new plugin surface to capabilities that expose real, otherwise-unreachable pi-web functionality, and drops invented/duplicative surfaces: Kept: - files.writeFile / deleteFile / moveFile (genuine workspace mutation, federated, path-safe) - prompt.insertText / getText / getSelection (editor state access) Dropped: - attachments.* (insertFileReference/getAttachedFiles/removeFileReference): getAttachedFiles invented a structured-attachment notion pi-web does not have and duplicated prompt.getText() + a regex with a false email-safety claim; insert/removeFileReference were thin sugar over readFile + insertText that plugins can compose themselves. - prompt.onPaste / onKeyDown: an incomplete two-event hook system shaped around a single use case, overlapping the editor's native image-paste handling. Deferred until a real editor event/hook surface is designed. - prompt.focus: redundant and buggier duplicate of the existing focusPrompt() (silently no-ops when not on the chat view). Focus stays as focusPrompt(). Security fix: - deleteWorkspaceFile now resolves the parent via realpath + ensureInside before lstat/unlink, closing a symlinked-parent-directory escape that allowed deleting files outside the workspace (write/move already did this). Final path component is still not resolved, so deleting a symlink removes the link, not its target. Adds a regression test. Docs and the registry test mock updated to match the trimmed surface.
This commit is contained in:
@@ -240,6 +240,22 @@ describe("deleteWorkspaceFile", () => {
|
||||
const realContent = await readFile(join(outsideDir, "real.txt"), "utf8");
|
||||
expect(realContent).toBe("real content");
|
||||
});
|
||||
|
||||
it("prevents deleting through a symlinked parent directory that escapes the workspace", async () => {
|
||||
const root = await tempWorkspace();
|
||||
await mkdir(join(root, "subdir"), { recursive: true });
|
||||
// A real file living outside the workspace that must not be deletable.
|
||||
const outsideDir = await mkdtemp(join(tmpdir(), "pi-web-outside-delete-parent-"));
|
||||
roots.push(outsideDir);
|
||||
await writeFile(join(outsideDir, "victim.txt"), "important");
|
||||
// A symlinked parent directory inside the workspace pointing outside.
|
||||
await symlink(outsideDir, join(root, "subdir", "escape"), "junction");
|
||||
|
||||
await expect(deleteWorkspaceFile(root, "subdir/escape/victim.txt")).rejects.toThrow("Path escapes workspace");
|
||||
// The outside file must survive.
|
||||
const realContent = await readFile(join(outsideDir, "victim.txt"), "utf8");
|
||||
expect(realContent).toBe("important");
|
||||
});
|
||||
});
|
||||
|
||||
describe("moveWorkspaceFile", () => {
|
||||
|
||||
@@ -86,12 +86,19 @@ export async function deleteWorkspaceFile(rootPath: string, path: string | undef
|
||||
// deletes the symlink itself, not the target it points to.
|
||||
// resolveInsideWorkspace would call realpath on the target, following
|
||||
// symlinks and resolving the symlink's destination instead.
|
||||
const { target, relativePath } = await resolveParentInsideWorkspace(rootPath, path);
|
||||
const { root, target, relativePath } = await resolveParentInsideWorkspace(rootPath, path);
|
||||
try {
|
||||
const s = await lstat(target);
|
||||
// Resolve symlinks in the parent path to prevent escape via a symlinked
|
||||
// parent directory. The final path component is intentionally NOT resolved
|
||||
// so that lstat/unlink act on the entry itself (deleting a symlink rather
|
||||
// than the file it points to).
|
||||
const realParent = await realpath(dirname(target));
|
||||
const realTarget = join(realParent, basename(target));
|
||||
ensureInside(root, realTarget);
|
||||
const s = await lstat(realTarget);
|
||||
// Allow deleting regular files and symlinks, but not directories
|
||||
if (s.isDirectory()) throw new Error("Path is a directory, use directory deletion instead");
|
||||
await unlink(target);
|
||||
await unlink(realTarget);
|
||||
return { path: relativePath, existed: true };
|
||||
} catch (error: unknown) {
|
||||
if (isNodeErrorWithCode(error, "ENOENT")) return { path: relativePath, existed: false };
|
||||
|
||||
Reference in New Issue
Block a user