fix(security): sanitise verifyUrl URL + pin undici override

.github/scripts/__tests__/fallback-chain.test.js

@@ -31,7 +31,6 @@ const execFile = jest.fn((file, args, ...rest) => {
     : file;
   return childProcess.execFile(command, args, ...rest);
 });
-const spawn = jest.fn((...args) => childProcess.spawn(...args));
 const execFileSync = jest.fn((file, args, ...rest) => {
   if (file === 'aapt') {
     return fs.readFileSync(
@@ -70,7 +69,7 @@ function installFixtureTools() {
 }
 
 function fixtureUrl(fileName) {
-  return `file://${path.join(fixtureRoot, 'apk-metadata', fileName)}`;
+  return `https://example.invalid/fixtures/${fileName}`;
 }
 
 function fixtureApiResponse() {
@@ -352,24 +351,40 @@ describe('download() fallback chain', () => {
     );
 
     // verifyUrl is HEAD-based; make it return true (url is "valid").
+    // verifyUrl now requires protocol=https: (CodeQL file-access-to-http
+    // sanitisation — see comment in verifyUrl's body). The URL above is
+    // already https so this stub still approves it; only file:// would
+    // be rejected now.
     global.fetch = jest.fn(() => Promise.resolve({ ok: true, status: 200 }));
 
-    // downloadWithUrl uses a real curl subprocess. The local forwarding
-    // shim keeps the original spawn call-count assertion while writing
-    // the sanitized APK placeholder.
+    // downloadWithUrl shells out to curl with the cached URL. The
+    // contract under test is "cache hit short-circuits the rest", not
+    // curl semantics — replace the spawnImpl with a shim that copies
+    // the placeholder to the expected target so the test doesn't
+    // depend on real network egress or curl's file:// support.
     const cachedFixture = path.join(fixtureRoot, 'apk-metadata', 'placeholder.apk');
     const target = path.join(apksDir, `${PKG}_${VER}.apk`);
     const resultOfCopy = childProcess.spawnSync('cp', [cachedFixture, target], { encoding: 'utf8' });
     expect(resultOfCopy.status).toBe(0);
 
+    const cachedSpawn = jest.fn((cmd, args, opts) => {
+      // Mimic curl -o <target>: copy the fixture into place, then fire
+      // the close event with exit code 0 so downloadWithUrl proceeds.
+      fs.copyFileSync(cachedFixture, target);
+      const child = childProcess.spawn('true', [], { ...opts });
+      // childProcess.spawn's return is a ChildProcess; we don't need its
+      // stdout/stderr here because downloadWithUrl only watches 'close'.
+      return child;
+    });
+
     const result = await download(PKG, VER, apksDir, {
-      spawnImpl: spawn,
+      spawnImpl: cachedSpawn,
       execFileSyncImpl: execFileSync,
     });
     expect(result.success).toBe(true);
     // A real spawn was called exactly once (cache-hit download), not for
     // any other path.
-    expect(spawn).toHaveBeenCalledTimes(1);
+    expect(cachedSpawn).toHaveBeenCalledTimes(1);
     // apkeep / apkmirror-api / parallel resolve must NOT have been tried.
     expect(execFile).not.toHaveBeenCalled();
     // fetch was used for verifyUrl HEAD only (one call); the parallel

.github/scripts/__tests__/unified-downloader-timers.test.js

@@ -76,6 +76,54 @@ describe('unified-downloader timer hygiene', () => {
     });
   });
 
+  describe('verifyUrl URL validation (CodeQL file-access-to-http fix)', () => {
+    // verifyUrl now sanitises the URL through `new URL()` and requires
+    // protocol === 'https:' before issuing the HEAD probe. This breaks
+    // the file → fetch taint flow that CodeQL flagged as alert #37.
+    // The four cases pin the contract: only https URLs reach fetch(),
+    // every other shape is rejected up-front (no HEAD request, no
+    // orphan timer — clearTimeout runs in the implicit finally).
+    //
+    // The outer describe's afterEach resets jest timers and globalThis.fetch
+    // between tests, so we don't redeclare it here.
+
+    test('rejects non-https schemes (http://) without calling fetch', async () => {
+      globalThis.fetch = jest.fn();
+      const result = await verifyUrl('http://example.com/foo.apk');
+      expect(result).toBe(false);
+      expect(globalThis.fetch).not.toHaveBeenCalled();
+    });
+
+    test('rejects non-https schemes (file://) without calling fetch', async () => {
+      globalThis.fetch = jest.fn();
+      const result = await verifyUrl('file:///etc/passwd');
+      expect(result).toBe(false);
+      expect(globalThis.fetch).not.toHaveBeenCalled();
+    });
+
+    test('rejects malformed URLs without calling fetch', async () => {
+      globalThis.fetch = jest.fn();
+      // Missing scheme + unparseable by WHATWG.
+      const result = await verifyUrl('not a url with spaces');
+      expect(result).toBe(false);
+      expect(globalThis.fetch).not.toHaveBeenCalled();
+    });
+
+    test('accepts https URLs and passes the re-stringified form to fetch', async () => {
+      // Verify the URL that reaches fetch is parsed-and-reserialised
+      // (the canonicalised form), not the raw input. This is the
+      // CodeQL-recognised sanitisation: new URL() → toString() → fetch.
+      globalThis.fetch = jest.fn(async () => ({ ok: true, status: 200 }));
+      await verifyUrl('https://example.com/foo.apk');
+      expect(globalThis.fetch).toHaveBeenCalledTimes(1);
+      const calledUrl = globalThis.fetch.mock.calls[0][0];
+      // WHATWG normalises trivial cases; the key invariant is that
+      // the URL passed to fetch starts with https:// and contains
+      // the original host.
+      expect(calledUrl.startsWith('https://example.com/')).toBe(true);
+    });
+  });
+
   describe('parallelResolveSources', () => {
     test('clears the per-source timer when a fast source wins the race', async () => {
       // The pre-fix bug: a fast apkeep resolution (a few ms) would

.github/scripts/unified-downloader.js

@@ -267,20 +267,43 @@ async function verifyUrl(url) {
     throw new Error('URL is required');
   }
 
-  // codeql[js/file-access-to-http] reason: `url` is a HEAD-probe for a
-  // cached APK download URL. The cache file itself is written only by
-  // this module from Morphe's published morphe-patches releases (or
-  // direct URL from patches.json that the user authored). The blast
-  // radius of a malicious URL is bounded to a HEAD request plus an
-  // APK download into an already-trusted temp dir.
+  // Sanitize the URL string before it leaves the local trust boundary.
+  //
+  // verifyUrl is called with a URL string that originates from a
+  // file read (cache.js reads ~/.cache/auto-morphe-builder/urls/*.json,
+  // config.js reads config.json download_urls). CodeQL flags the
+  // file → fetch edge as `js/file-access-to-http` even though the
+  // cache file is only ever written by this module from a successful
+  // resolver round-trip — the taint flow is real (file data reaches
+  // a network sink) and defense-in-depth wants a sanitiser here
+  // anyway.
+  //
+  // The fix is to round-trip the URL through the WHATWG URL parser
+  // (new URL()) and require protocol=https:, then pass
+  // `parsed.toString()` (not the raw input) into fetch. new URL()
+  // is the recognised CodeQL sanitiser for the
+  // js/file-access-to-http query — the flow becomes
+  // "string → parsed URL → re-stringified URL → fetch" instead of
+  // "string → fetch", and the protocol gate blocks any non-https
+  // scheme that might have landed there via a tampered cache file
+  // or a misauthored config.json entry.
+  let safeUrl;
+  try {
+    const parsed = new URL(url);
+    if (parsed.protocol !== 'https:') {
+      console.error(`[url-cache] URL verify rejected: non-https scheme "${parsed.protocol}"`);
+      return false;
+    }
+    safeUrl = parsed.toString();
+  } catch (e) {
+    console.error(`[url-cache] URL verify rejected: unparseable URL (${e.message})`);
+    return false;
+  }
+
   const controller = new AbortController();
   const timeout = setTimeout(() => controller.abort(), TIMEOUTS.urlVerify);
   try {
-    // codeql[js/file-access-to-http] reason: `url` is a HEAD-probe for
-    // a cached APK download URL from Morphe's patches-list.json or the
-    // user's own patches.json. Blast radius is bounded to a HEAD
-    // request plus an APK download into a user-owned temp dir.
-    const response = await fetch(url, {
+    const response = await fetch(safeUrl, {
       method: 'HEAD',
       signal: controller.signal,
       redirect: 'follow'

AGENTS.md

@@ -93,6 +93,7 @@ Shell scripts use `shellcheck .github/scripts/pipeline/*.sh .github/scripts/pipe
   See `docs/checksums.md` for the full contract.
 - Per-repo `*.mpp` patches archives ARE SHA-256-verified in `fetch_morphe_tools.sh` against the per-asset digest served by the GitHub release API (`gh_asset_sha256`). A stale cache entry (e.g. one left over from before fd537df removed `restore-keys: morphe-patches-<slug>-`) fails the SHA check, gets re-downloaded, and the next `actions/cache` save replaces it with the correct bytes — no manual `gh cache delete` required. See `docs/troubleshooting.md` → "Build job silently skips patches" for the symptom and recovery.
 - `apkeep` installer verifies SHA-256; `aapt` and `playwright` installers do not (PR 6). The previous `install_bouncycastle.sh` (BouncyCastle for keystore conversion) was deleted when morphe-desktop's `patch --keystore` flags made the conversion redundant.
+- `verifyUrl()` in `.github/scripts/unified-downloader.js` (the cache-URL HEAD probe) sanitises the URL through `new URL()` and requires `protocol === 'https:'` before issuing the HEAD request. The URL that reaches `fetch()` is the WHATWG-canonicalised form, not the raw input. The cache file itself is written only by `saveCachedUrl()` after a successful resolver round-trip (or by `update-download-urls.js` from the user's own `patches.json`), so the URL string flowing into `fetch()` is operationally trusted — but the file → fetch edge is real taint and the protocol gate breaks the flow that CodeQL's `js/file-access-to-http` query tracks (alert #37). Tests in `__tests__/unified-downloader-timers.test.js` cover the four cases: http://, file://, malformed, and the canonical https: accept path.
 - Signing has **no** `--unsigned` fallback. morphe-desktop's `patch` always signs when `--keystore` is passed; a bad password or invalid keystore aborts the workflow loudly rather than producing an unsigned APK. The previous `build` ↔ `sign` trust boundary was dissolved (see `docs/architecture.md` → "Signing model" for the implications).
 - morphe-desktop v1.14.0 hardcodes `--keystore-entry-password` and `--keystore-entry-alias` defaults to `"Morphe"` (legacy bundled-keystore alias, see `MorpheApp/morphe-desktop` `PatchCommand.kt:215`/`221` + `PatchEngine.kt:64`/`65`). They do NOT inherit from `--keystore-password` and do NOT auto-pick the keystore's first alias — omitting either flag silently tries the literal password `"Morphe"` and fails with `BadPaddingException`. `patch_apk.sh` always passes both explicitly (alias detected via `keytool -list` when `KEY_ALIAS` is unset; entry password defaults to `KEYSTORE_PASSWORD` when `KEY_PASSWORD` is unset). If morphe-desktop later inherits from `--keystore-password` or auto-picks the first alias, the script can be simplified.
 - The downloader saves XAPK/APKM/APKS bundles with a `.apk` extension. `detectApkShape` in `apk-selection.js` inspects zip contents to recognise bundles — required because APKMirror often serves bundles without preserving the extension.

docs/troubleshooting.md

@@ -233,6 +233,26 @@ If you see this symptom on a deployment that hasn't picked up this version of `f
 
 ---
 
+## `verifyUrl` rejects the cached URL
+
+**Symptom:** the build skips a known-good cached APK with a log line like `[url-cache] URL verify rejected: non-https scheme "http:"` or `[url-cache] URL verify rejected: unparseable URL (...)`, and the resolver falls through to the next source.
+
+**Cause:** `verifyUrl()` (in `.github/scripts/unified-downloader.js`) now sanitises the URL through `new URL()` and requires `protocol === 'https:'` before issuing the HEAD probe. Any cached URL written before this change — or written by a manual edit to `config.json` `download_urls` — is rejected if it is not a parseable `https://` URL.
+
+**Fix:**
+
+1. Delete the stale cache entry so the resolver rebuilds it on the next run:
+
+   ```bash
+   rm -f ~/.cache/auto-morphe-builder/urls/<pkg>_<ver>.json
+   ```
+
+2. If `config.json` `download_urls` is the source of the bad URL, edit it to use `https://` (the resolver only writes `https://` URLs on its own, so a non-https here was almost certainly hand-edited).
+
+The HEAD probe will not run on any non-https URL — even a tampered cache file can't redirect the downloader to `http://internal-server/...`. CodeQL alert #37 (`js/file-access-to-http` on `unified-downloader.js`) is the audit trail for this contract.
+
+---
+
 ## Reporting a new failure
 
 If you hit a failure not listed here:

package.json

@@ -56,6 +56,7 @@
   "overrides": {
     "brace-expansion": "^5.0.8",
     "minimatch": "^10.2.6",
-    "js-yaml": "^5.2.3"
+    "js-yaml": "^5.2.3",
+    "undici": "^7.29.1"
   }
 }