From ffbde92f9ab65a868b660abf17ce9d3c4577d125 Mon Sep 17 00:00:00 2001
From: Rudy Mizrahi Celekli <47457359+rudycelekli@users.noreply.github.com>
Date: Mon, 5 Oct 2026 13:02:56 -0400
Subject: [PATCH] fix(javascript): respect HTML base URL components (#589)
* fix(javascript): respect HTML base URL components
Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
* fix(javascript): resolve HTML base href dot segments and gate base syntax
A relative HTML reference resolves against the base URL's directory, and that
directory drops a trailing "." or ".." segment rather than stepping through
it. Only a trailing "/" was treated as a directory, so
resolved against the artifact root instead of the document directory, and
resolved against the parent of "sub".
The base href is also a second untrusted path input, but rejectDeclaration
only applied its admission rules to the declared path, so an encoded-traversal
or backslash base href relied on files-map confinement to stay safe. Both
inputs now share one admitsCanonicalPathSyntax predicate, and a local base
href is rejected with a reason naming the base href rather than a generic
declared-path message. External base hrefs are still reported as external,
so an encoded path on an unreachable origin keeps the more accurate reason.
7 of the new cases fail without this change.
* style: apply Prettier to the HTML base resolution changes
---------
Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
Co-authored-by: morluto <76467478+morluto@users.noreply.github.com>
---
.../JavaScriptArtifactPathResolution.ts | 53 +++++++--
.../JavaScriptHtmlBaseResolution.test.ts | 111 ++++++++++++++++++
2 files changed, 155 insertions(+), 9 deletions(-)
create mode 100644 src/application/JavaScriptHtmlBaseResolution.test.ts
diff --git a/src/application/JavaScriptArtifactPathResolution.ts b/src/application/JavaScriptArtifactPathResolution.ts
index c30c04fe78..8a165eb7b8 100644
--- a/src/application/JavaScriptArtifactPathResolution.ts
+++ b/src/application/JavaScriptArtifactPathResolution.ts
@@ -108,13 +108,19 @@ const rejectDeclaration = (
return unresolvedOutcome(input, "rejected", [
"NUL and backslash path syntax are not admitted for canonical artifact paths.",
]);
- if (/%(?:2e|2f|5c)/iu.test(declared))
+ if (!admitsCanonicalPathSyntax(declared))
return unresolvedOutcome(input, "rejected", [
"Encoded dot or separator bytes are rejected before artifact path resolution.",
]);
return null;
};
+/** Path syntax admitted for a canonical artifact path. */
+const admitsCanonicalPathSyntax = (value: string): boolean =>
+ !value.includes("\0") &&
+ !value.includes("\\") &&
+ !/%(?:2e|2f|5c)/iu.test(value);
+
const contextualCandidate = (
input: ResolveArtifactPathInput,
): string | ArtifactPathResolution => {
@@ -206,25 +212,54 @@ const htmlCandidate = (
input: ResolveArtifactPathInput,
): string | ArtifactPathResolution => {
const declared = stripQueryAndFragment(input.declaredPath);
- if (hasScheme(declared))
+ if (hasScheme(declared) || declared.startsWith("//"))
return unresolvedOutcome(input, "external", [
"External HTML references are not mapped to local artifact assets.",
]);
- if (declared.startsWith("/")) return declared.slice(1);
- const base = input.htmlBaseHref;
- if (base === undefined || base === null || base === "")
- return posix.join(posix.dirname(input.sourcePath), declared);
- if (hasScheme(base))
+ const rawBase = input.htmlBaseHref;
+ const base =
+ rawBase === undefined || rawBase === null
+ ? rawBase
+ : stripQueryAndFragment(rawBase);
+ if (
+ base !== undefined &&
+ base !== null &&
+ (hasScheme(base) || base.startsWith("//"))
+ )
return unresolvedOutcome(input, "external", [
"The document base href is external, so its script reference is not a local artifact path.",
]);
+ if (declared.startsWith("/")) return declared.slice(1);
+ if (base === undefined || base === null || base === "")
+ return posix.join(posix.dirname(input.sourcePath), declared);
+ // A local base href is a second untrusted path input; apply the same
+ // admission rules a declared path gets so it cannot smuggle traversal or
+ // separator syntax past canonicalization.
+ if (!admitsCanonicalPathSyntax(base))
+ return unresolvedOutcome(input, "rejected", [
+ "The document base href uses NUL, backslash, or encoded dot and separator bytes that are not admitted for canonical artifact paths.",
+ ]);
const basePath = base.startsWith("/")
? base.slice(1)
: posix.join(posix.dirname(input.sourcePath), base);
- const baseDirectory = base.endsWith("/") ? basePath : posix.dirname(basePath);
- return posix.join(baseDirectory, declared);
+ return posix.join(htmlBaseDirectory(base, basePath), declared);
};
+/**
+ * The directory a relative HTML reference resolves against. A base path
+ * ending in "/" or in a "." or ".." segment is already a directory, because a
+ * relative reference resolves against the base URL's directory and those
+ * segments are dropped rather than stepped through.
+ */
+const htmlBaseDirectory = (base: string, basePath: string): string =>
+ base.endsWith("/") ||
+ base.endsWith("/.") ||
+ base.endsWith("/..") ||
+ base === "." ||
+ base === ".."
+ ? basePath
+ : posix.dirname(basePath);
+
const confineCandidate = (
input: ResolveArtifactPathInput,
candidate: string,
diff --git a/src/application/JavaScriptHtmlBaseResolution.test.ts b/src/application/JavaScriptHtmlBaseResolution.test.ts
new file mode 100644
index 0000000000..a5a9fe93cb
--- /dev/null
+++ b/src/application/JavaScriptHtmlBaseResolution.test.ts
@@ -0,0 +1,111 @@
+import { describe, expect, it } from "vitest";
+import { resolveArtifactPathByContext } from "./JavaScriptArtifactPathResolution.js";
+import type { JavaScriptArtifactFile } from "./JavaScriptArtifactFiles.js";
+const paths = [
+ "renderer/index.html",
+ "assets/app.js",
+ "renderer/app.js",
+ "renderer/a/app.js",
+ "app.js",
+];
+const files = new Map(
+ paths.map((path) => [
+ path,
+ {
+ path,
+ container_sha256: "a".repeat(64),
+ sha256: "b".repeat(64),
+ bytes: 0,
+ inventory_artifact_id: path,
+ kind: "javascript",
+ unpacked: false,
+ text: { included: true, value: "" },
+ },
+ ]),
+);
+const resolve = (declaredPath: string, htmlBaseHref: string) =>
+ resolveArtifactPathByContext({
+ declaredPath,
+ htmlBaseHref,
+ sourcePath: "renderer/index.html",
+ context: "html-reference",
+ files,
+ });
+const documentUrl = "https://artifact.test/renderer/index.html";
+describe("HTML base href URL components", () => {
+ it.each([
+ "/assets/?cache=/wrong/",
+ "/assets/#/wrong/",
+ "/assets/index.html?cache=/wrong/",
+ ])("ignores query and fragment path-looking characters in %s", (base) => {
+ expect(
+ new URL(
+ "app.js",
+ new URL(base, "https://artifact.test/renderer/index.html"),
+ ).pathname,
+ ).toBe("/assets/app.js");
+ expect(resolve("app.js", base)).toMatchObject({
+ resolution_status: "resolved",
+ resolved_path: "assets/app.js",
+ });
+ });
+ it.each(["https://external.test/", "//external.test/"])(
+ "keeps root-relative references external when their base is %s",
+ (base) => {
+ expect(resolve("/assets/app.js", base)).toMatchObject({
+ resolution_status: "external",
+ resolved_path: null,
+ });
+ },
+ );
+});
+describe("HTML base href dot segments", () => {
+ it.each([
+ [".", "renderer/app.js"],
+ ["./", "renderer/app.js"],
+ ["a/.", "renderer/a/app.js"],
+ ["a/..", "renderer/app.js"],
+ ["..", "app.js"],
+ ])(
+ "resolves %s against the document directory like the URL parser",
+ (base, expected) => {
+ // A relative reference resolves against the base URL's directory, which
+ // drops a trailing "." or ".." segment instead of stepping through it.
+ expect(
+ decodeURIComponent(
+ new URL("app.js", new URL(base, documentUrl)).pathname,
+ ).replace(/^\/+/, ""),
+ ).toBe(expected);
+ expect(resolve("app.js", base)).toMatchObject({
+ resolution_status: "resolved",
+ resolved_path: expected,
+ });
+ },
+ );
+});
+describe("HTML base href path syntax", () => {
+ it.each(["%2e%2e/secret/", "/a/%2e%2e/", "/a\\b/", "a\0b"])(
+ "rejects base href %j with the same admission rules a declared path gets",
+ (base) => {
+ const outcome = resolve("app.js", base);
+ expect(outcome.resolution_status).toBe("rejected");
+ expect(outcome.limitations[0]).toContain(
+ "not admitted for canonical artifact paths",
+ );
+ },
+ );
+ it("still confines a base href that escapes the artifact root", () => {
+ expect(resolve("app.js", "/a/../../secret/")).toMatchObject({
+ resolution_status: "rejected",
+ limitations: [
+ "The resolved candidate escapes the canonical artifact root.",
+ ],
+ });
+ });
+ it("reports an external base href as external rather than malformed", () => {
+ expect(resolve("app.js", "https://external.test/%2e%2e/")).toMatchObject({
+ resolution_status: "external",
+ resolved_path: null,
+ });
+ });
+});