Fix: report removed launcher downloads accurately (#741)

* Fix: distinguish removed launcher downloads from checksum failures

Related to #740; keep the antivirus investigation open. Add executable launcher regressions and a native Windows CI lane. AI assistance: prepared with Codex under Paul Bakaus direction.

* Fix: diagnose downloads removed during cache placement

Cover removal and truncation around rename and preserve fail-closed behavior through cache placement. AI-assisted under maintainer direction.

* Test Windows launcher hash and placement failures

Inject failures at command boundaries in a staged test copy while retaining real launcher control flow. Cover both platforms with the same assertions. AI-assisted under maintainer direction.

.github/workflows/ci.yml

@@ -188,6 +188,20 @@ jobs:
       - run: cargo build --workspace --all-targets
       - run: cargo test --workspace --no-fail-fast
 
+  launcher-windows:
+    runs-on: windows-latest
+    needs: changes
+    if: needs.changes.outputs.core == 'true'
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v7
+      - name: Setup Node
+        uses: actions/setup-node@v7
+        with:
+          node-version: 24
+      - name: Exercise Windows launcher downloads and verification
+        run: node --test tests/launcher-download.test.mjs
+
   # Behavior gate: replays the tests/oracle/ goldens against a release build
   # of the engine from THIS checkout (so a PR is judged on its own source,
   # not on the last published binary). Without this job the oracle only ever

scripts/test-suites.mjs

@@ -64,6 +64,7 @@ export const SUITES = {
         files: [
           'tests/ci-test-plan.test.mjs',
           'tests/cli-shim.test.mjs',
+          'tests/launcher-download.test.mjs',
           'tests/publish-platform-packages.test.mjs',
           'tests/github-sheriff.test.mjs',
           'tests/hook-build.test.mjs',

skill/scripts/impeccable

@@ -96,6 +96,19 @@ fetch_url() {
     return 1
   fi
 }
+check_download() {
+  download_file=${1:-$tmp}
+  if [ ! -f "$download_file" ]; then
+    rm -f "$tmp.sha256"
+    echo "impeccable: download completed but the file was removed before execution: $url; check your antivirus quarantine or logs. Refusing to continue; do not disable protection." >&2
+    exit 127
+  fi
+  if [ ! -s "$download_file" ]; then
+    rm -f "$download_file" "$tmp.sha256"
+    echo "impeccable: downloaded file is empty: $url; refusing the unverified download" >&2
+    exit 127
+  fi
+}
 if [ -n "$probing" ]; then
   # Inside another launcher's probe: no download, fail fast and quiet.
   exit 127
@@ -116,6 +129,7 @@ if [ -n "$version" ] && [ "$os" != unknown ] && [ "$arch" != unknown ]; then
     fetch_url "$url" && fetched=1
   fi
   if [ "$fetched" = 1 ]; then
+    check_download
     # Fail closed: a freshly downloaded binary runs only after verifying
     # against its .sha256 sidecar. A sidecar that cannot be fetched, or a
     # machine with no sha256 tool, refuses the download instead of exec'ing
@@ -127,15 +141,20 @@ if [ -n "$version" ] && [ "$os" != unknown ] && [ "$arch" != unknown ]; then
     elif command -v wget >/dev/null 2>&1; then
       wget -q -O "$tmp.sha256" "$url.sha256" 2>/dev/null && sidecar_ok=1
     fi
+    check_download
     expected=""
     [ "$sidecar_ok" = 1 ] && expected=$(cut -d' ' -f1 < "$tmp.sha256")
     actual=""
-    if command -v shasum >/dev/null 2>&1; then actual=$(shasum -a 256 "$tmp" | cut -d' ' -f1)
-    elif command -v sha256sum >/dev/null 2>&1; then actual=$(sha256sum "$tmp" | cut -d' ' -f1); fi
+    if command -v shasum >/dev/null 2>&1; then
+      if digest=$(shasum -a 256 "$tmp" 2>/dev/null); then actual=${digest%% *}; fi
+    elif command -v sha256sum >/dev/null 2>&1; then
+      if digest=$(sha256sum "$tmp" 2>/dev/null); then actual=${digest%% *}; fi
+    fi
+    check_download
     rm -f "$tmp.sha256"
     if [ -z "$expected" ] || [ -z "$actual" ]; then
       rm -f "$tmp"
-      echo "impeccable: cannot verify $url against $url.sha256 (sidecar unavailable or no sha256 tool); refusing the unverified download" >&2
+      echo "impeccable: cannot verify $url against $url.sha256 (sidecar unavailable or hashing failed); refusing the unverified download" >&2
       exit 127
     fi
     if [ "$actual" != "$expected" ]; then
@@ -143,8 +162,22 @@ if [ -n "$version" ] && [ "$os" != unknown ] && [ "$arch" != unknown ]; then
       echo "impeccable: checksum mismatch downloading $url" >&2
       exit 127
     fi
-    chmod +x "$tmp" 2>/dev/null
-    mv -f "$tmp" "$cached" && exec "$cached" "$@"
+    check_download
+    if ! chmod +x "$tmp" 2>/dev/null; then
+      check_download
+      rm -f "$tmp"
+      echo "impeccable: could not make the verified download executable: $url" >&2
+      exit 127
+    fi
+    check_download
+    if ! mv -f "$tmp" "$cached" 2>/dev/null; then
+      check_download
+      rm -f "$tmp"
+      echo "impeccable: could not cache the verified download: $url" >&2
+      exit 127
+    fi
+    check_download "$cached"
+    exec "$cached" "$@"
   fi
   rm -f "$tmp" 2>/dev/null
 fi

skill/scripts/impeccable.cmd

@@ -82,6 +82,8 @@ curl.exe -fsSL -o "%cached%.part" "%url%" >nul 2>nul
 if errorlevel 1 goto fail
 
 :verify
+call :check_download
+if errorlevel 1 exit /b 127
 rem Mirrors the sh launcher and fails closed: a freshly downloaded binary
 rem runs only after verifying against its .sha256 sidecar. A sidecar that
 rem cannot be fetched, or an empty certutil result, refuses the download
@@ -91,8 +93,16 @@ if errorlevel 1 goto verify_refuse
 set "expected="
 set /p expected=<"%cached%.sha256"
 for /f "tokens=1" %%h in ("%expected%") do set "expected=%%h"
+call :check_download
+if errorlevel 1 exit /b 127
 set "actual="
-for /f "skip=1 delims=" %%h in ('certutil -hashfile "%cached%.part" SHA256 2^>nul') do if not defined actual set "actual=%%h"
+rem Reuse the sidecar staging file after reading expected. Check certutil's
+rem status before parsing: its error text on stdout is not a digest.
+certutil -hashfile "%cached%.part" SHA256 >"%cached%.sha256" 2>nul
+if errorlevel 1 goto verify_refuse
+call :check_download
+if errorlevel 1 exit /b 127
+for /f "usebackq skip=1 delims=" %%h in ("%cached%.sha256") do if not defined actual set "actual=%%h"
 del "%cached%.sha256" >nul 2>nul
 if not defined expected goto verify_refuse
 if not defined actual goto verify_refuse
@@ -103,17 +113,48 @@ echo impeccable: checksum mismatch downloading %url% 1>&2
 exit /b 127
 
 :verify_refuse
+call :check_download
+if errorlevel 1 exit /b 127
 del "%cached%.part" >nul 2>nul
 del "%cached%.sha256" >nul 2>nul
 echo impeccable: cannot verify %url% against %url%.sha256; refusing the unverified download 1>&2
 exit /b 127
 
+:check_download
+set "download_file=%~1"
+if not defined download_file set "download_file=%cached%.part"
+if not exist "%download_file%" goto download_missing
+for %%f in ("%download_file%") do if %%~zf==0 goto download_empty
+exit /b 0
+
+:download_missing
+del "%cached%.sha256" >nul 2>nul
+echo impeccable: download completed but the file was removed before execution: %url%; check your antivirus quarantine or logs. Refusing to continue; do not disable protection. 1>&2
+exit /b 127
+
+:download_empty
+del "%download_file%" >nul 2>nul
+del "%cached%.sha256" >nul 2>nul
+echo impeccable: downloaded file is empty: %url%; refusing the unverified download 1>&2
+exit /b 127
+
 :place
+call :check_download
+if errorlevel 1 exit /b 127
 move /y "%cached%.part" "%cached%" >nul 2>nul
-if not exist "%cached%" goto fail
+if errorlevel 1 goto place_failed
+call :check_download "%cached%"
+if errorlevel 1 exit /b 127
 set "run=%cached%"
 goto run
 
+:place_failed
+call :check_download
+if errorlevel 1 exit /b 127
+del "%cached%.part" >nul 2>nul
+echo impeccable: could not cache the verified download: %url% 1>&2
+exit /b 127
+
 :run
 "%run%" %*
 exit /b

tests/launcher-download.test.mjs

@@ -0,0 +1,140 @@
+import assert from 'node:assert/strict';
+import { spawn } from 'node:child_process';
+import { createHash } from 'node:crypto';
+import fs from 'node:fs';
+import http from 'node:http';
+import os from 'node:os';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+import { test } from 'node:test';
+
+const ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
+const WINDOWS = process.platform === 'win32';
+const COMSPEC = process.env.ComSpec || process.env.COMSPEC || 'C:\\Windows\\System32\\cmd.exe';
+// Use the host's command interpreter as a harmless Windows executable. No
+// downloaded release binary is executed, and all network traffic is loopback.
+const PAYLOAD = WINDOWS ? fs.readFileSync(COMSPEC) : Buffer.from('#!/bin/sh\necho verified-engine\n');
+const HASH = createHash('sha256').update(PAYLOAD).digest('hex');
+
+async function exercise(t, scenario) {
+  const root = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-launcher-'));
+  t.after(() => fs.rmSync(root, { recursive: true, force: true }));
+  const scripts = path.join(root, 'skill scripts');
+  const home = path.join(root, 'home');
+  const cache = path.join(root, 'cache');
+  fs.mkdirSync(scripts);
+  fs.mkdirSync(home);
+  fs.writeFileSync(path.join(scripts, 'VERSION'), '0.0.0-test\n');
+  const name = WINDOWS ? 'impeccable.cmd' : 'impeccable';
+  const launcher = path.join(scripts, name);
+  fs.copyFileSync(path.join(ROOT, 'skill/scripts', name), launcher);
+  const cacheDir = path.join(cache, 'bin', '0.0.0-test');
+  const tools = path.join(root, 'tools');
+  fs.mkdirSync(tools);
+  if (!WINDOWS && ['hash-failure', 'removed-during-hash'].includes(scenario)) {
+    fs.writeFileSync(path.join(tools, 'shasum'),
+      `#!/bin/sh\n${scenario === 'removed-during-hash' ? 'rm -f "$3"\n' : ''}printf '%s  %s\\n' '${HASH}' "$3"\nexit ${scenario === 'hash-failure' ? 1 : 0}\n`,
+      { mode: 0o755 });
+  }
+  const placementScenarios = ['removed-before-move', 'removed-after-move', 'emptied-after-move', 'move-failure'];
+  if (!WINDOWS && placementScenarios.includes(scenario)) {
+    const before = scenario === 'removed-before-move' ? 'rm -f "$2"\n' : '';
+    const after = scenario === 'removed-after-move' ? 'rm -f "$3"\n' : scenario === 'emptied-after-move' ? ': > "$3"\n' : '';
+    fs.writeFileSync(path.join(tools, 'mv'), scenario === 'move-failure' ? '#!/bin/sh\nexit 1\n' : `#!/bin/sh\n${before}/bin/mv "$@" || exit $?\n${after}`, { mode: 0o755 });
+  }
+  if (WINDOWS && (placementScenarios.includes(scenario) || ['hash-failure', 'removed-during-hash'].includes(scenario))) {
+    // move is a cmd builtin and certutil is an .exe: PATH shims cannot
+    // intercept them. Instrument ONLY the external-operation boundary in the
+    // staged test copy, preserving the launcher's real labels/checks/status
+    // handling. The separate valid case always runs the unmodified launcher.
+    const fault = path.join(tools, 'fault.cmd');
+    const hashFault = ['hash-failure', 'removed-during-hash'].includes(scenario);
+    const operation = hashFault
+      ? 'certutil -hashfile "%cached%.part" SHA256 >"%cached%.sha256" 2>nul'
+      : 'move /y "%cached%.part" "%cached%" >nul 2>nul';
+    let script;
+    if (hashFault) {
+      script = `@echo off\n${scenario === 'removed-during-hash' ? 'del "%cached%.part" >nul 2>nul\n' : ''}echo hash header\necho ${HASH}\nexit /b 1\n`;
+    } else if (scenario === 'move-failure') {
+      script = '@echo off\nexit /b 1\n';
+    } else {
+      const before = scenario === 'removed-before-move' ? 'del "%cached%.part" >nul 2>nul\n' : '';
+      const after = scenario === 'removed-after-move' ? 'del "%cached%" >nul 2>nul\n' : scenario === 'emptied-after-move' ? 'type nul >"%cached%"\n' : '';
+      script = `@echo off\n${before}${operation}\nif errorlevel 1 exit /b 1\n${after}exit /b 0\n`;
+    }
+    fs.writeFileSync(fault, script.replaceAll('\n', '\r\n'));
+    const source = fs.readFileSync(launcher, 'utf8');
+    assert.equal(source.split(operation).length, 2, 'instrument exactly one operation');
+    const replacement = `call "${fault}"${hashFault ? ' >"%cached%.sha256" 2>nul' : ''}`;
+    fs.writeFileSync(launcher, source.replace(operation, replacement));
+  }
+  const requests = [];
+  const server = http.createServer((req, res) => {
+    requests.push(req.url);
+    if (req.url.endsWith('.sha256')) {
+      const part = fs.readdirSync(cacheDir).find(file => file.includes('.part'));
+      if (scenario === 'removed') fs.unlinkSync(path.join(cacheDir, part));
+      if (scenario === 'emptied') fs.truncateSync(path.join(cacheDir, part));
+      res.writeHead(scenario === 'no-sidecar' ? 404 : 200);
+      res.end(scenario === 'empty-sidecar' ? '' : `${scenario === 'mismatch' ? '0'.repeat(64) : HASH}  engine\n`);
+    } else {
+      res.end(scenario === 'empty-download' ? '' : PAYLOAD);
+    }
+  });
+  await new Promise(resolve => server.listen(0, '127.0.0.1', resolve));
+  t.after(() => new Promise(resolve => server.close(resolve)));
+  // Keep system tools, but exclude user/npm PATH candidates and all launcher
+  // overrides so the test cannot accidentally execute an installed engine.
+  const env = {
+    PATH: WINDOWS ? `${process.env.SystemRoot}\\System32;${process.env.SystemRoot}` : `${tools}:/usr/bin:/bin`,
+    HOME: home, USERPROFILE: home, TEMP: root, TMP: root,
+    IMPECCABLE_HOME: cache,
+    IMPECCABLE_DOWNLOAD_BASE: `http://127.0.0.1:${server.address().port}`,
+    ...(WINDOWS ? { SystemRoot: process.env.SystemRoot, ComSpec: COMSPEC, PROCESSOR_ARCHITECTURE: 'AMD64' } : {}),
+  };
+  const result = await new Promise((resolve, reject) => {
+    const child = WINDOWS
+      ? spawn(COMSPEC, ['/d', '/s', '/c', `""${launcher}" /d /c echo verified-engine"`], { env, cwd: root, windowsVerbatimArguments: true, timeout: 20000 })
+      : spawn('/bin/sh', [launcher], { env, cwd: root, timeout: 20000 });
+    let stdout = '';
+    let stderr = '';
+    child.stdout.on('data', data => { stdout += data; });
+    child.stderr.on('data', data => { stderr += data; });
+    child.on('error', reject);
+    child.on('close', (status, signal) => resolve({ status, signal, stdout, stderr }));
+  });
+  assert.equal(result.signal, null, JSON.stringify(result));
+  assert.equal(requests.filter(url => !url.endsWith('.sha256')).length, 1, 'one binary download, no verification retry loop');
+  return { ...result, files: fs.readdirSync(cacheDir), requests };
+}
+
+test('launcher downloads and runs a verified executable', async t => {
+  const result = await exercise(t, 'valid');
+  assert.equal(result.status, 0, result.stderr);
+  assert.match(result.stdout, /verified-engine/);
+  assert.deepEqual(result.files, [WINDOWS ? 'impeccable.exe' : 'impeccable']);
+  assert.equal(result.requests.length, 2);
+});
+
+for (const scenario of ['removed', 'emptied', 'empty-download', 'no-sidecar', 'empty-sidecar', 'mismatch', 'hash-failure', 'removed-during-hash', 'removed-before-move', 'removed-after-move', 'emptied-after-move', 'move-failure']) {
+  test(`launcher refuses ${scenario} with an accurate diagnostic`, async t => {
+    const result = await exercise(t, scenario);
+    assert.equal(result.status, 127, JSON.stringify(result));
+    assert.doesNotMatch(result.stdout, /verified-engine/);
+    assert.deepEqual(result.files, [], 'no unverified file or sidecar left behind');
+    if (scenario.startsWith('removed')) {
+      assert.match(result.stderr, /download completed but the file was removed before (verification|execution)/);
+      assert.match(result.stderr, /antivirus.*logs/i);
+      assert.doesNotMatch(result.stderr, /checksum mismatch/);
+    } else if (scenario.startsWith('emptied') || scenario === 'empty-download') {
+      assert.match(result.stderr, /downloaded file is empty/);
+      assert.doesNotMatch(result.stderr, /checksum mismatch/);
+    } else if (scenario === 'mismatch') {
+      assert.match(result.stderr, /checksum mismatch/);
+    } else if (scenario === 'move-failure') {
+      assert.match(result.stderr, /could not cache the verified download/);
+    } else {
+      assert.match(result.stderr, /refusing the unverified download/);
+    }
+  });
+}