diff --git a/.changeset/harden-remote-plugin-assets.md b/.changeset/harden-remote-plugin-assets.md new file mode 100644 index 0000000..62797c2 --- /dev/null +++ b/.changeset/harden-remote-plugin-assets.md @@ -0,0 +1,5 @@ +--- +"@jmfederico/pi-web": patch +--- + +Harden remote machine plugin asset proxying so plugin asset URLs cannot escape the remote plugin directory. diff --git a/src/server/app.test.ts b/src/server/app.test.ts index 9113b5a..aebed93 100644 --- a/src/server/app.test.ts +++ b/src/server/app.test.ts @@ -343,6 +343,45 @@ describe("buildApp", () => { expect(request).toHaveBeenCalledWith("GET", "/pi-web-plugins/remote-tools/pi-web-plugin.js?v=123"); }); + it("drops unsafe remote machine plugin manifest modules", async () => { + const addResponse = await app.inject({ method: "POST", url: "/api/machines", payload: { name: "Remote", baseUrl: "https://remote.example.test/" } }); + const remote = addResponse.json<{ id: string }>(); + remoteClient = fakeRemoteClient({ + requestJson: vi.fn(() => Promise.resolve({ + statusCode: 200, + headers: { "content-type": "application/json" }, + body: { + plugins: [ + { id: "safe-tools", module: "nested/pi-web-plugin.js?v=1", source: "local", scope: "local" }, + { id: "traversal-tools", module: "..%2F..%2Fapi%2Fconfig", source: "local", scope: "local" }, + { id: "wrong-root", module: "/pi-web-plugins/other/pi-web-plugin.js", source: "local", scope: "local" }, + ], + }, + })), + }); + + const manifestResponse = await app.inject({ method: "GET", url: `/api/machines/${remote.id}/pi-web-plugins/manifest.json` }); + + expect(manifestResponse.statusCode).toBe(200); + expect(manifestResponse.json()).toEqual({ + plugins: [{ id: "safe-tools", module: `/pi-web-plugins/${machineScopedPluginId(remote.id, "safe-tools")}/nested/pi-web-plugin.js?v=1`, source: "local", scope: "local" }], + }); + }); + + it("rejects remote machine plugin asset traversal before proxying", async () => { + const addResponse = await app.inject({ method: "POST", url: "/api/machines", payload: { name: "Remote", baseUrl: "https://remote.example.test/" } }); + const remote = addResponse.json<{ id: string }>(); + const request = vi.fn(() => Promise.resolve({ statusCode: 200, headers: {}, body: Readable.from([]) })); + remoteClient = fakeRemoteClient({ request }); + const scopedPluginId = machineScopedPluginId(remote.id, "remote-tools"); + + const response = await app.inject({ method: "GET", url: `/pi-web-plugins/${scopedPluginId}/..%2F..%2Fapi%2Fconfig` }); + + expect(response.statusCode).toBe(400); + expect(response.json()).toEqual({ error: "Invalid remote PI WEB plugin asset path" }); + expect(request).not.toHaveBeenCalled(); + }); + it("returns stable errors for invalid project requests", async () => { const addResponse = await app.inject({ method: "POST", diff --git a/src/server/machines/machinePluginProxyRoutes.ts b/src/server/machines/machinePluginProxyRoutes.ts index 20b37ee..65856b2 100644 --- a/src/server/machines/machinePluginProxyRoutes.ts +++ b/src/server/machines/machinePluginProxyRoutes.ts @@ -59,8 +59,14 @@ export async function proxyMachinePluginAsset(machines: MachinePluginProxyMachin return true; } + const requestPath = remotePluginAssetRequestPath(remotePlugin, assetPath, requestUrl); + if (requestPath === undefined) { + await reply.code(400).send({ error: "Invalid remote PI WEB plugin asset path" }); + return true; + } + try { - const upstream = await client.request("GET", remotePluginAssetRequestPath(remotePlugin, assetPath, requestUrl)); + const upstream = await client.request("GET", requestPath); reply.code(upstream.statusCode); applySafeHeaders(reply, upstream.headers); if (upstream.body === undefined) await reply.send(); @@ -87,29 +93,57 @@ function rewriteRemotePluginManifest(machineId: string, manifest: RemotePluginMa function remotePluginModulePath(pluginId: string, module: string): { path: string; query: string } | undefined { if (!isPiWebPluginId(pluginId)) return undefined; + const prefix = `/pi-web-plugins/${encodeURIComponent(pluginId)}/`; + const base = new URL(prefix, "http://pi-web.local"); try { - const url = new URL(module, "http://pi-web.local"); - const prefix = `/pi-web-plugins/${encodeURIComponent(pluginId)}/`; - if (url.pathname.startsWith(prefix)) { - return { path: url.pathname.slice(prefix.length), query: url.search }; - } - if (!module.startsWith("/") && !/^https?:\/\//iu.test(module)) { - const [path, query = ""] = module.split("?", 2); - if (path !== undefined && path !== "") return { path, query: query === "" ? "" : `?${query}` }; - } + const url = new URL(module, base); + if (url.origin !== base.origin || !url.pathname.startsWith(prefix)) return undefined; + const path = safeRemotePluginAssetPath(url.pathname.slice(prefix.length)); + return path === undefined ? undefined : { path, query: url.search }; } catch { return undefined; } - return undefined; } -function remotePluginAssetRequestPath(remotePlugin: MachineScopedPluginIdParts, assetPath: string, requestUrl: string): string { +function remotePluginAssetRequestPath(remotePlugin: MachineScopedPluginIdParts, assetPath: string, requestUrl: string): string | undefined { + const path = safeRemotePluginAssetPath(assetPath); + if (path === undefined) return undefined; const query = requestUrl.includes("?") ? requestUrl.slice(requestUrl.indexOf("?")) : ""; - return `/pi-web-plugins/${encodeURIComponent(remotePlugin.pluginId)}/${encodePathSegments(assetPath)}${query}`; + return `/pi-web-plugins/${encodeURIComponent(remotePlugin.pluginId)}/${path}${query}`; } -function encodePathSegments(path: string): string { - return path.split("/").map((segment) => encodeURIComponent(segment)).join("/"); +function safeRemotePluginAssetPath(path: string): string | undefined { + const segments: string[] = []; + for (const rawSegment of path.split("/")) { + const segment = safeRemotePluginAssetPathSegment(rawSegment); + if (segment === undefined) return undefined; + if (segment === "") continue; + segments.push(segment); + } + if (segments.length === 0) return undefined; + return segments.map((segment) => encodeURIComponent(segment)).join("/"); +} + +function safeRemotePluginAssetPathSegment(rawSegment: string): string | undefined { + if (rawSegment === "" || rawSegment === ".") return ""; + if (/%(?:2f|5c)/iu.test(rawSegment)) return undefined; + let segment: string; + try { + segment = decodeURIComponent(rawSegment); + } catch { + return undefined; + } + if (segment === "" || segment === ".") return ""; + if (segment === ".." || segment.includes("/") || segment.includes("\\") || hasControlCharacter(segment)) return undefined; + return segment; +} + +function hasControlCharacter(value: string): boolean { + for (let index = 0; index < value.length; index += 1) { + const code = value.charCodeAt(index); + if (code <= 0x1f || code === 0x7f) return true; + } + return false; } function parseRemoteManifest(value: unknown): RemotePluginManifest {