lab: take the registry's address only from the forge's own host, built from the parsed URL (hq issue 624, review of #275)

What the registry answers may come over plain HTTP, so it must not choose
where packages come from, nor reach the run's npm configuration as a raw
string.

modules/lab/test/runs.test.ts

@@ -275,21 +275,39 @@ const packument = (tarball: string) => new Response(JSON.stringify({
 
 test("a run names the package registry by the address the registry gives its tarballs", async () => {
   const r = aRegistry(() => packument("https://forge.example.org/api/packages/novox/npm/%40novox%2Fmesh-sdk/-/0.1.14/mesh-sdk-0.1.14.tgz"));
-  const got = await npmRegistry("http://novox.internal:20000/", r.get);
+  const got = await npmRegistry("http://forge.example.org:3000/", r.get);
   assert.deepEqual(got, { registry: "https://forge.example.org/api/packages/novox/npm/" });
-  assert.deepEqual(r.asked, ["http://novox.internal:20000/api/packages/novox/npm/@novox%2fmesh-sdk"]);
+  assert.deepEqual(r.asked, ["http://forge.example.org:3000/api/packages/novox/npm/@novox%2fmesh-sdk"]);
+  // The forge's own scheme is taken too, with its port.
+  const plain = await npmRegistry("http://forge.example.org:3000",
+    aRegistry(() => packument("http://forge.example.org:3000/api/packages/novox/npm/%40novox%2Fmesh-sdk/-/0.1.14/mesh-sdk-0.1.14.tgz")).get);
+  assert.deepEqual(plain, { registry: "http://forge.example.org:3000/api/packages/novox/npm/" });
 });
 
-test("a registry that does not say its address leaves the forge's, and the reason is said", async () => {
-  for (const answer of [
-    () => new Response("not found", { status: 404 }),
-    () => packument("https://forge.example.org/elsewhere/mesh-sdk-0.1.14.tgz"),
-    () => new Response("{}", { status: 200 }),
-    () => { throw new Error("connect ECONNREFUSED"); },
-  ]) {
+// What the registry answers may come over plain HTTP, so it never decides where packages come from: only
+// the forge's own host, at https or the forge's scheme, under the npm path, with nothing after it
+// (review of mesh-catalog #275).
+test("a registry that does not give an address the lab takes leaves the forge's, and the reason is said", async () => {
+  const sdk = "/api/packages/novox/npm/%40novox%2Fmesh-sdk/-/0.1.14/mesh-sdk-0.1.14.tgz";
+  for (const [answer, why] of [
+    [() => new Response("not found", { status: 404 }), /it answered 404/],
+    [() => new Response("{}", { status: 200 }), /no tarball/],
+    [() => { throw new Error("connect ECONNREFUSED"); }, /ECONNREFUSED/],
+    [() => packument(`https://forge.example.org/elsewhere/mesh-sdk-0.1.14.tgz`), /not under \/api\/packages\/novox\/npm\//],
+    [() => packument(`https://attacker.example.net${sdk}`), /attacker\.example\.net, which is not the forge's host novox\.internal/],
+    [() => packument(`https://attacker.example.net/x?u=https://novox.internal${sdk}`), /query or a fragment/],
+    [() => packument(`https://novox.internal${sdk}?x=1`), /query or a fragment/],
+    [() => packument(`https://novox.internal${sdk}#frag`), /query or a fragment/],
+    [() => packument(`https://attacker.example.net/#https://novox.internal${sdk}`), /query or a fragment/],
+    [() => packument(`ftp://novox.internal${sdk}`), /ftp: — neither https: nor the forge's http:/],
+    [() => packument(`file:///api/packages/novox/npm/x.tgz`), /file: — neither/],
+    [() => packument(`https://novox.internal/api/packages/novox/npm/\nregistry=https://attacker.example.net/\nallow-remote=all`), /whitespace or a control character/],
+    [() => packument(`https://user:pw@novox.internal${sdk}`), /user or password/],
+  ] as [() => Response, RegExp][]) {
     const got = await npmRegistry("http://novox.internal:20000", aRegistry(answer).get);
-    assert.equal(got.registry, "http://novox.internal:20000/api/packages/novox/npm/");
-    assert.match(got.problem ?? "", /did not say its own address/);
+    assert.equal(got.registry, "http://novox.internal:20000/api/packages/novox/npm/", String(why));
+    assert.match(got.problem ?? "", /did not give an address the lab takes/);
+    assert.match(got.problem ?? "", why);
   }
 });
 

modules/lab/tools/runs.ts

@@ -64,35 +64,61 @@ fi`;
 export const NPM_PATH = "/api/packages/novox/npm/";
 
 /**
- * The address a run names the @novox registry by: the one the registry names itself by (novox/hq issue 624).
+ * The address a run names the @novox registry by: the one the registry names itself by, when that is the
+ * forge's own host (novox/hq issue 624).
  *
  * **npm 12 installs a locked tarball only from the registry it is configured with.** Its `allow-remote`
  * defaults to `none`, and a lock file's `resolved` URL counts as the registry's only when its origin and
  * path are the configured scope registry's; any other is a "remote" tarball and refused (EALLOWREMOTE).
  * The forge writes every tarball URL — in what it answers, and so in every lock file made against it — at
- * its own public address, whichever address it was asked at; the lab reaches it at the forge setting's,
- * another one. So the lab asks the registry, at the forge, for the SDK's newest tarball and names the
- * registry by the address in it, as the lock files do. Answered with the forge's own address and the
- * reason when the registry does not say, so a run still starts and lab_check names it.
+ * its own public address, whichever address it was asked at. So the lab asks the registry, at the forge,
+ * for the SDK's newest tarball and names the registry by the origin of that URL.
+ *
+ * **What the registry answers is not trusted for where packages come from** (review of mesh-catalog #275):
+ * it may be fetched over plain HTTP. The tarball URL is parsed, and taken only when it is https (or the
+ * forge's own scheme), its path is under the npm path, it carries no query or fragment, and its host is
+ * the forge's own host. The registry is then built from
+ * the parsed origin and the npm path alone, never from the string answered, so nothing the registry says
+ * reaches the run's npm configuration but a scheme, a host and a port. Anything else falls back to the
+ * forge's own address and says why, so a run still starts and lab_check names it.
+ *
+ * So the lab's forge setting is the address the forge names itself by (its public https name), not a
+ * second name for it: a forge set as another name of the same server is refused here, named by lab_check,
+ * and its runs stop at the first locked @novox package.
  */
 export async function npmRegistry(forge: string, get: typeof fetch = fetch): Promise<{ registry: string; problem?: string }> {
   const asked = `${forge.replace(/\/+$/, "")}${NPM_PATH}`;
   const url = `${asked}@novox%2fmesh-sdk`;
   const fallback = (why: string) => ({
     registry: asked,
-    problem: `the package registry at ${url} did not say its own address (${why}), so a run names it ${asked}, and npm 12 refuses a locked @novox tarball at any other`,
+    problem: `the package registry at ${url} did not give an address the lab takes (${why}), so a run names it ${asked}, and npm 12 refuses a locked @novox tarball at any other`,
   });
+  let own: URL;
+  try {
+    own = new URL(forge);
+  } catch {
+    return fallback(`the forge ${JSON.stringify(forge)} is not a URL`);
+  }
   try {
     const res = await get(url, { signal: AbortSignal.timeout(15_000) });
     if (!res.ok) return fallback(`it answered ${res.status}`);
-    const doc = await res.json() as { "dist-tags"?: { latest?: string }; versions?: Record<string, { dist?: { tarball?: string } }> };
+    const doc = await res.json() as { "dist-tags"?: { latest?: string }; versions?: Record<string, { dist?: { tarball?: unknown } }> };
     const latest = doc["dist-tags"]?.latest ?? "";
-    const tarball = doc.versions?.[latest]?.dist?.tarball ?? "";
-    const at = tarball.indexOf(NPM_PATH);
-    if (at < 0) return fallback(tarball ? `its tarball ${tarball} is not under ${NPM_PATH}` : "no tarball for its latest version");
-    const registry = tarball.slice(0, at + NPM_PATH.length);
-    new URL(registry);
-    return { registry };
+    const tarball = doc.versions?.[latest]?.dist?.tarball;
+    if (typeof tarball !== "string" || !tarball) return fallback("no tarball for its latest version");
+    if (/[\s\u0000-\u001f\u007f]/.test(tarball)) return fallback("its tarball URL holds whitespace or a control character");
+    let t: URL;
+    try {
+      t = new URL(tarball);
+    } catch {
+      return fallback("its tarball is not a URL");
+    }
+    if (t.protocol !== "https:" && t.protocol !== own.protocol) return fallback(`its tarball is ${t.protocol} — neither https: nor the forge's ${own.protocol}`);
+    if (t.username || t.password) return fallback("its tarball URL carries a user or password");
+    if (t.search || t.hash) return fallback("its tarball URL carries a query or a fragment");
+    if (!t.pathname.startsWith(NPM_PATH)) return fallback(`its tarball is not under ${NPM_PATH}`);
+    if (t.hostname !== own.hostname) return fallback(`its tarball is at ${t.hostname}, which is not the forge's host ${own.hostname}`);
+    return { registry: `${t.origin}${NPM_PATH}` };
   } catch (e) {
     return fallback(e instanceof Error ? e.message : String(e));
   }