From ef535b81fefc358b0bdb655e5aa057fc3949ce5f Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 7 Oct 2026 11:28:28 +0800 Subject: [PATCH] refactor(domain): extract artifact path syntax helpers (#830) Move hasScheme/stripQueryAndFragment/admitsCanonicalPathSyntax plus looksExternal into domain/artifactPathSyntax with the deliberate policy-divergence table (artifact root vs allowedOrigins vs exact-origin fetch). Adopt in JavaScriptArtifactPathResolution and the Electron native specifier check with characterization tests. Policy unification tracked in #829. --- .../JavaScriptArtifactPathResolution.ts | 26 ++++------- src/domain/artifactPathSyntax.test.ts | 45 +++++++++++++++++++ src/domain/artifactPathSyntax.ts | 34 ++++++++++++++ src/domain/electronStaticAnalysisNative.ts | 5 +-- 4 files changed, 89 insertions(+), 21 deletions(-) create mode 100644 src/domain/artifactPathSyntax.test.ts create mode 100644 src/domain/artifactPathSyntax.ts diff --git a/src/application/JavaScriptArtifactPathResolution.ts b/src/application/JavaScriptArtifactPathResolution.ts index fc7eba438..0e3528abb 100644 --- a/src/application/JavaScriptArtifactPathResolution.ts +++ b/src/application/JavaScriptArtifactPathResolution.ts @@ -2,6 +2,12 @@ import { builtinModules } from "node:module"; import { posix } from "node:path"; import type { JavaScriptArtifactFile } from "./JavaScriptArtifactFiles.js"; +import { + admitsCanonicalPathSyntax, + hasScheme, + looksExternal, + stripQueryAndFragment, +} from "../domain/artifactPathSyntax.js"; type ArtifactPathResolutionContext = | "package-entrypoint" @@ -115,12 +121,6 @@ const rejectDeclaration = ( 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 => { @@ -212,7 +212,7 @@ const htmlCandidate = ( input: ResolveArtifactPathInput, ): string | ArtifactPathResolution => { const declared = stripQueryAndFragment(input.declaredPath); - if (hasScheme(declared) || declared.startsWith("//")) + if (looksExternal(declared)) return unresolvedOutcome(input, "external", [ "External HTML references are not mapped to local artifact assets.", ]); @@ -221,11 +221,7 @@ const htmlCandidate = ( rawBase === undefined || rawBase === null ? rawBase : stripQueryAndFragment(rawBase); - if ( - base !== undefined && - base !== null && - (hasScheme(base) || base.startsWith("//")) - ) + if (base !== undefined && base !== null && looksExternal(base)) return unresolvedOutcome(input, "external", [ "The document base href is external, so its script reference is not a local artifact path.", ]); @@ -538,9 +534,6 @@ const notFoundCandidate = (): CandidateResolution => ({ ], }); -const hasScheme = (value: string): boolean => - /^[A-Za-z][A-Za-z+.-]*:/u.test(value); - const fileUrlPath = (value: string): string | undefined => { if (!value.startsWith("file://")) return undefined; try { @@ -554,9 +547,6 @@ const fileUrlPath = (value: string): string | undefined => { } }; -const stripQueryAndFragment = (value: string): string => - value.split("#", 1)[0]?.split("?", 1)[0] ?? ""; - const outcome = ( input: ResolveArtifactPathInput, resolution: CandidateResolution, diff --git a/src/domain/artifactPathSyntax.test.ts b/src/domain/artifactPathSyntax.test.ts new file mode 100644 index 000000000..7a31567ae --- /dev/null +++ b/src/domain/artifactPathSyntax.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from "vitest"; + +import { + admitsCanonicalPathSyntax, + hasScheme, + looksExternal, + stripQueryAndFragment, +} from "./artifactPathSyntax.js"; + +describe("artifact path syntax", () => { + it("detects URI schemes without matching bare paths", () => { + expect(hasScheme("https://example.test/app.js")).toBe(true); + expect(hasScheme("file:///app.js")).toBe(true); + expect(hasScheme("node:fs")).toBe(true); + expect(hasScheme("./relative.js")).toBe(false); + expect(hasScheme("/absolute.js")).toBe(false); + expect(hasScheme("app.js")).toBe(false); + }); + + it("flags scheme-qualified and protocol-relative references as external", () => { + expect(looksExternal("https://example.test/app.js")).toBe(true); + expect(looksExternal("//example.test/app.js")).toBe(true); + expect(looksExternal("./app.js")).toBe(false); + expect(looksExternal("/app.js")).toBe(false); + }); + + it("strips queries and fragments in fragment-first order", () => { + expect(stripQueryAndFragment("./app.js?token=secret#hash")).toBe( + "./app.js", + ); + expect(stripQueryAndFragment("./app.js#hash?token=secret")).toBe( + "./app.js", + ); + expect(stripQueryAndFragment("./app.js")).toBe("./app.js"); + }); + + it("rejects NUL, backslash, and encoded dot/separator bytes", () => { + expect(admitsCanonicalPathSyntax("./app.js")).toBe(true); + expect(admitsCanonicalPathSyntax("dir\\app.js")).toBe(false); + expect(admitsCanonicalPathSyntax("dir\0app.js")).toBe(false); + expect(admitsCanonicalPathSyntax("./%2e%2e/secret.js")).toBe(false); + expect(admitsCanonicalPathSyntax("./%2Fapp.js")).toBe(false); + expect(admitsCanonicalPathSyntax("./%5capp.js")).toBe(false); + }); +}); diff --git a/src/domain/artifactPathSyntax.ts b/src/domain/artifactPathSyntax.ts new file mode 100644 index 000000000..46666dcfb --- /dev/null +++ b/src/domain/artifactPathSyntax.ts @@ -0,0 +1,34 @@ +/** + * Pure string predicates for artifact path syntax. The local artifact + * resolver, the Electron native specifier check, and (by reference) the + * live-DOM and source-map URL handling share these character-level rules. + * + * Policy divergence (deliberate, not yet unified): + * - Local artifact paths resolve against an inventoried container root and + * report rejected | external | not-found. Confinement is lexical + * (posix.normalize must stay inside the root). + * - Live DOM destinations resolve against the document base URL and report + * approved | outside_policy | unsupported against allowedOrigins + * (see CdpCaptureDocuments.domDestination). + * - Source-map fetches require exact-origin http(s) without credentials + * (see WebSourceMapFetcher.approvedUrl). + * Unifying those outcome vocabularies is a separate, observable change. + */ + +/** Whether a reference carries a URI scheme (`https:`, `file:`, …). */ +export const hasScheme = (value: string): boolean => + /^[A-Za-z][A-Za-z+.-]*:/u.test(value); + +/** Whether a reference is scheme-qualified or protocol-relative. */ +export const looksExternal = (value: string): boolean => + hasScheme(value) || value.startsWith("//"); + +/** Drop any query string and fragment from a declared reference. */ +export const stripQueryAndFragment = (value: string): string => + value.split("#", 1)[0]?.split("?", 1)[0] ?? ""; + +/** Path syntax admitted for a canonical artifact path. */ +export const admitsCanonicalPathSyntax = (value: string): boolean => + !value.includes("\0") && + !value.includes("\\") && + !/%(?:2e|2f|5c)/iu.test(value); diff --git a/src/domain/electronStaticAnalysisNative.ts b/src/domain/electronStaticAnalysisNative.ts index 8dd762bae..42f09e43c 100644 --- a/src/domain/electronStaticAnalysisNative.ts +++ b/src/domain/electronStaticAnalysisNative.ts @@ -1,6 +1,7 @@ import * as t from "@babel/types"; import { compareCodePoints } from "./canonicalOrdering.js"; +import { stripQueryAndFragment } from "./artifactPathSyntax.js"; import type { ElectronNativeAddonBindingFinding } from "./electronStaticAnalysisTypes.js"; import { addLocatedFinding } from "./javascriptStaticAnalysisFindings.js"; import { @@ -208,8 +209,6 @@ const isNativeSpecifier = ( moduleKind: ElectronNativeAddonBindingFinding["module_kind"], ): boolean => { const path = - moduleKind === "require" - ? specifier - : (specifier.split("#", 1)[0]?.split("?", 1)[0] ?? ""); + moduleKind === "require" ? specifier : stripQueryAndFragment(specifier); return path.toLowerCase().endsWith(".node"); };