From 195f20935948f1ee7049a70aecea7e6605675c45 Mon Sep 17 00:00:00 2001 From: cliffhall Date: Mon, 14 Sep 2026 00:24:10 -0400 Subject: [PATCH] fix: require an authority for the path-less root form (#2295) `skill:`, `mailto:` and `skill://` also parse to an empty pathname, with an empty host, so an empty path alone accepted authority-less URIs the guard exists to reject. Accept the empty path only when a host is present (Copilot review round 1). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01VQ2eszDCUqTmhtTMaznw3W Signed-off-by: cliffhall --- clients/web/src/test/core/mcp/skills.test.ts | 5 +++++ core/mcp/skills.ts | 10 ++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/clients/web/src/test/core/mcp/skills.test.ts b/clients/web/src/test/core/mcp/skills.test.ts index 2391d389..552b02e0 100644 --- a/clients/web/src/test/core/mcp/skills.test.ts +++ b/clients/web/src/test/core/mcp/skills.test.ts @@ -176,6 +176,11 @@ describe("normalizeSkillUri", () => { expect(normalizeSkillUri("skill:demo/SKILL.md")).toBeUndefined(); // Path-less but still opaque: no authority, so it stays rejected. expect(normalizeSkillUri("skill:demo")).toBeUndefined(); + // Empty path AND no authority — the parser reports `pathname === ""` for + // these too, so an empty path alone must not be read as the root form. + expect(normalizeSkillUri("skill:")).toBeUndefined(); + expect(normalizeSkillUri("mailto:")).toBeUndefined(); + expect(normalizeSkillUri("skill://")).toBeUndefined(); }); it("accepts a path-less authority URI — a skill's root directory (#2295)", () => { diff --git a/core/mcp/skills.ts b/core/mcp/skills.ts index bb55f9fe..1bbb5175 100644 --- a/core/mcp/skills.ts +++ b/core/mcp/skills.ts @@ -173,7 +173,8 @@ export function isSkillsExtensionSupported( * * A **path-less authority** URI (`skill://data-analysis`) is accepted, and is * not the opaque case above: RFC 3986 gives a URI with an authority an empty - * path, and the parser reports that as `pathname === ""`. It has no segments, + * path, and the parser reports that as `pathname === ""`. The authority has to + * be non-empty — `skill:` parses to an empty path too, with no host. It has no segments, * so nothing survives unnormalized. It is what a skill's root directory looks * like once `/SKILL.md` is removed, so rejecting it made a server echoing that * root back in a directory read read as "outside this skill" when it is the @@ -196,9 +197,10 @@ export function normalizeSkillUri(uri: string): string | undefined { } catch { return undefined; } - if (parsed.pathname !== "" && !parsed.pathname.startsWith("/")) { - return undefined; - } + // An empty path is only the root form when an authority carries the identity: + // `skill:` and `mailto:` also parse to `pathname === ""`, with no host. + const pathlessRoot = parsed.pathname === "" && parsed.host !== ""; + if (!pathlessRoot && !parsed.pathname.startsWith("/")) return undefined; return canonicalizePercentEncoding(parsed.href); }