From 63d10cb2d0d20d6512fe623cf7d13c3ddc669e61 Mon Sep 17 00:00:00 2001
From: Tam Nhu Tran <kaitran.ntt@gmail.com>
Date: Sun, 7 Jun 2026 16:38:08 -0400
Subject: [PATCH] fix(cli): harden ccs bar install + align launch path

Resolve the app path via os.homedir() in launch to match install/uninstall;
validate the host on every redirect hop (manual follow, not blind
maxRedirections); reject zip-slip entries before extracting into ~/Applications.
---
 src/commands/bar/install-subcommand.ts  | 125 +++++++++++++++++++-----
 src/commands/bar/launch-subcommand.ts   |  10 +-
 tests/unit/commands/bar-command.test.ts | 118 ++++++++++++++++++++++
 3 files changed, 224 insertions(+), 29 deletions(-)

diff --git a/src/commands/bar/install-subcommand.ts b/src/commands/bar/install-subcommand.ts
index 4cd881fb..8f286932 100644
--- a/src/commands/bar/install-subcommand.ts
+++ b/src/commands/bar/install-subcommand.ts
@@ -145,13 +145,17 @@ async function defaultFetchReleaseAsset(tag: string, asset: string): Promise<Rel
 }
 
 /**
- * Download a zip from `url` (following up to 5 GitHub 302 redirects) and
- * extract its contents into `dest`.
+ * Download a zip from `url` (following up to 5 GitHub 302 redirects, re-validating
+ * each hop's Location hostname) and extract its contents into `dest`.
  *
  * Fixes applied:
- * - Finding #8: pass `maxRedirections: 5` so GitHub's 302 → objects.githubusercontent.com is followed.
+ * - Fix #6: maxRedirections:0 + manual redirect following so every hop's Location
+ *   header is passed through validateDownloadUrl before following it.
+ *   The previous maxRedirections:5 let undici follow redirects to ANY host unchecked.
+ * - Fix #14: list zip entries with `unzip -l` before extracting; reject the archive
+ *   if any entry path contains ".." or starts with "/" (zip-slip guard).
  * - Finding #11: check `statusCode` and throw a descriptive error before streaming.
- * - Finding #9: validate host+HTTPS before making the request.
+ * - Finding #9: validate host+HTTPS before making the first request.
  */
 async function defaultDownloadAndExtract(url: string, dest: string): Promise<void> {
   const { request } = await import('undici');
@@ -162,34 +166,107 @@ async function defaultDownloadAndExtract(url: string, dest: string): Promise<voi
   const { execFile } = await import('child_process');
   const execFileAsync = promisify(execFile);
 
-  // Finding #9: validate host + HTTPS before any network request.
+  // Validate initial URL (Finding #9)
   validateDownloadUrl(url);
 
   mkdirSync(dest, { recursive: true });
-  const tmpZip = path.join(os.tmpdir(), `ccs-bar-${Date.now()}.zip`);
 
-  // Finding #8: maxRedirections:5 ensures GitHub's 302 to objects.githubusercontent.com is followed.
-  // Finding #11: destructure statusCode and reject non-200 before streaming.
-  const { statusCode, body } = await request(url, {
-    maxRedirections: 5,
-  });
+  // Fix #6: follow redirects manually so each hop is re-validated.
+  const MAX_REDIRECTS = 5;
+  let currentUrl = url;
+  let redirectsFollowed = 0;
 
-  if (statusCode !== 200) {
-    throw new Error(`Download failed: HTTP ${statusCode} for ${url}`);
-  }
+  while (true) {
+    const { statusCode, headers, body } = await request(currentUrl, {
+      maxRedirections: 0, // disable undici's auto-follow; we follow manually
+    });
 
-  // Stream body to tmpZip — undici body is a Node ReadableStream
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  await streamPipeline(body as any, createWriteStream(tmpZip));
+    if (statusCode >= 300 && statusCode < 400) {
+      const location = Array.isArray(headers['location'])
+        ? headers['location'][0]
+        : headers['location'];
 
-  // Extract the zip into dest
-  await execFileAsync('unzip', ['-o', tmpZip, '-d', dest]);
+      if (!location) {
+        throw new Error(`Redirect (HTTP ${statusCode}) from ${currentUrl} has no Location header`);
+      }
 
-  // Clean up the temp archive
-  try {
-    fs.unlinkSync(tmpZip);
-  } catch {
-    /* ignore */
+      // Resolve relative redirects against the current URL
+      const resolved = new URL(location, currentUrl).toString();
+
+      // Re-validate the redirect target — this is the key fix for #6
+      validateDownloadUrl(resolved);
+
+      if (redirectsFollowed >= MAX_REDIRECTS) {
+        throw new Error(`Too many redirects (>${MAX_REDIRECTS}) while downloading ${url}`);
+      }
+
+      // Drain the body to free the socket before following the redirect
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      await (body as any).dump?.();
+      currentUrl = resolved;
+      redirectsFollowed++;
+      continue;
+    }
+
+    if (statusCode !== 200) {
+      throw new Error(`Download failed: HTTP ${statusCode} for ${currentUrl}`);
+    }
+
+    const tmpZip = path.join(os.tmpdir(), `ccs-bar-${Date.now()}.zip`);
+
+    // Stream body to tmpZip
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    await streamPipeline(body as any, createWriteStream(tmpZip));
+
+    // Fix #14: zip-slip guard — inspect entries before extraction.
+    // `unzip -l` lists entries in a machine-readable format; we scan for ".." or
+    // absolute paths that would escape the destination directory.
+    try {
+      const { stdout: listing } = await execFileAsync('unzip', ['-l', tmpZip]);
+      const lines = listing.split('\n');
+      for (const line of lines) {
+        // Entry lines look like: "  <size>  <date> <time>  <path>"
+        // We extract the path from the last whitespace-delimited field.
+        const match = /^\s+\d+\s+[\d-]+\s+[\d:]+\s+(.+)$/.exec(line);
+        if (!match || !match[1]) continue;
+        const entryPath = match[1].trim();
+        if (!entryPath || entryPath.endsWith('/')) continue; // skip directory entries
+
+        // Reject absolute paths and paths with traversal components
+        if (path.isAbsolute(entryPath) || entryPath.includes('..')) {
+          try {
+            fs.unlinkSync(tmpZip);
+          } catch {
+            /* ignore */
+          }
+          throw new Error(
+            `Zip-slip detected: archive entry "${entryPath}" contains a path traversal ` +
+              `component. Refusing to extract.`
+          );
+        }
+      }
+    } catch (err) {
+      // If the guard itself throws (e.g. zip-slip detected above), propagate it.
+      // If it's a system error (unzip not available), let extraction proceed and
+      // surface the issue then.
+      if ((err as Error).message?.includes('Zip-slip')) throw err;
+      // Warn but continue — extraction will likely also fail if unzip is missing
+      console.error(
+        `[!] Zip entry scan failed (will attempt extraction): ${(err as Error).message}`
+      );
+    }
+
+    // Extract the zip into dest
+    await execFileAsync('unzip', ['-o', tmpZip, '-d', dest]);
+
+    // Clean up the temp archive
+    try {
+      fs.unlinkSync(tmpZip);
+    } catch {
+      /* ignore */
+    }
+
+    break;
   }
 }
 
diff --git a/src/commands/bar/launch-subcommand.ts b/src/commands/bar/launch-subcommand.ts
index a73b909c..89cc33ef 100644
--- a/src/commands/bar/launch-subcommand.ts
+++ b/src/commands/bar/launch-subcommand.ts
@@ -8,6 +8,7 @@
  */
 
 import * as fs from 'fs';
+import * as os from 'os';
 import * as path from 'path';
 import { getCcsDir } from '../../config/config-loader-facade';
 
@@ -90,11 +91,10 @@ function defaultGetCcsDir(): string {
   return getCcsDir();
 }
 
-const DEFAULT_APP_INSTALL_PATH = path.join(
-  process.env.HOME ?? process.env.CCS_HOME ?? '~',
-  'Applications',
-  'CCS Bar.app'
-);
+// Fix #5: use os.homedir() to match install-subcommand.ts and uninstall-subcommand.ts.
+// process.env.HOME may be unset in restricted environments, and CCS_HOME is the CCS
+// data directory (~/.ccs), not the user's home — neither is a safe fallback here.
+const DEFAULT_APP_INSTALL_PATH = path.join(os.homedir(), 'Applications', 'CCS Bar.app');
 
 // ---------------------------------------------------------------------------
 // Implementation
diff --git a/tests/unit/commands/bar-command.test.ts b/tests/unit/commands/bar-command.test.ts
index 4f249fe5..5b97b6d6 100644
--- a/tests/unit/commands/bar-command.test.ts
+++ b/tests/unit/commands/bar-command.test.ts
@@ -1074,3 +1074,121 @@ describe('version subcommand', () => {
     expect(exitCode).toBe(0);
   });
 });
+
+// ---------------------------------------------------------------------------
+// 4f. Fix #6 — redirect host bypass: every hop is re-validated
+// ---------------------------------------------------------------------------
+
+describe('bar install: redirect host re-validation (fix #6)', () => {
+  const INITIAL_URL =
+    'https://github.com/kaitranntt/ccs/releases/download/ccs-bar-latest/CCS-Bar.app.zip';
+  const EVIL_REDIRECT_URL = 'https://evil.example.com/CCS-Bar.app.zip';
+  const FAKE_VERSION = '1.0.0';
+
+  it('rejects a download when a redirect leads to an untrusted host', async () => {
+    // Simulate a downloadAndExtract that follows a redirect to an untrusted host
+    // and validates each hop (the production fix). The mock replicates the fix logic.
+    const { handleBarInstall, validateDownloadUrl } = await loadInstallSubcommand();
+
+    const redirectFollowingExtract = async (url: string, _dest: string) => {
+      // Validate initial URL
+      validateDownloadUrl(url);
+      // Simulate a 302 to an evil host — production code re-validates Location
+      validateDownloadUrl(EVIL_REDIRECT_URL); // should throw
+    };
+
+    await handleBarInstall([], {
+      fetchReleaseAsset: async () => ({
+        downloadUrl: INITIAL_URL,
+        version: FAKE_VERSION,
+      }),
+      downloadAndExtract: redirectFollowingExtract,
+      verifyCompat: async () => ({ version: FAKE_VERSION, compatible: true }),
+      getCcsDir: () => path.join(tempHome, '.ccs'),
+      getAppsDir: () => path.join(tempHome, 'Applications'),
+    });
+
+    const allOutput = consoleOutput.join('\n');
+    // The validation error should surface as a download/extraction failure
+    expect(allOutput).toMatch(/\[X\]/);
+    expect(allOutput.toLowerCase()).toMatch(/download|extraction|failed/i);
+  });
+
+  it('validateDownloadUrl blocks redirect targets with untrusted hostnames', async () => {
+    const { validateDownloadUrl } = await loadInstallSubcommand();
+
+    // The redirect target must be rejected just like any initial URL
+    expect(() => validateDownloadUrl(EVIL_REDIRECT_URL)).toThrow(/allowlist|trusted/i);
+  });
+
+  it('validateDownloadUrl allows the expected redirect target (githubusercontent.com)', async () => {
+    const { validateDownloadUrl } = await loadInstallSubcommand();
+    const legitimateRedirect = 'https://objects.githubusercontent.com/file/CCS-Bar.app.zip';
+    expect(() => validateDownloadUrl(legitimateRedirect)).not.toThrow();
+  });
+});
+
+// ---------------------------------------------------------------------------
+// 4g. Fix #14 — zip-slip guard: reject archives with traversal entries
+// ---------------------------------------------------------------------------
+
+describe('bar install: zip-slip guard (fix #14)', () => {
+  const FAKE_DOWNLOAD_URL =
+    'https://github.com/kaitranntt/ccs/releases/download/ccs-bar-latest/CCS-Bar.app.zip';
+  const FAKE_VERSION = '1.0.0';
+
+  it('rejects download when downloadAndExtract detects a zip-slip entry', async () => {
+    const { handleBarInstall } = await loadInstallSubcommand();
+
+    // Simulate an extractor that detects a zip-slip entry and throws
+    const zipSlipExtract = async (_url: string, _dest: string) => {
+      throw new Error(
+        'Zip-slip detected: archive entry "../../../evil" contains a path traversal component. Refusing to extract.'
+      );
+    };
+
+    await handleBarInstall([], {
+      fetchReleaseAsset: async () => ({
+        downloadUrl: FAKE_DOWNLOAD_URL,
+        version: FAKE_VERSION,
+      }),
+      downloadAndExtract: zipSlipExtract,
+      verifyCompat: async () => ({ version: FAKE_VERSION, compatible: true }),
+      getCcsDir: () => path.join(tempHome, '.ccs'),
+      getAppsDir: () => path.join(tempHome, 'Applications'),
+    });
+
+    const allOutput = consoleOutput.join('\n');
+    // Should report failure and not proceed to install
+    expect(allOutput).toMatch(/\[X\]/);
+    expect(allOutput.toLowerCase()).toMatch(/download|extraction|failed/i);
+    // App should not be installed
+    expect(fs.existsSync(path.join(tempHome, 'Applications', 'CCS Bar.app'))).toBe(false);
+  });
+
+  it('allows a clean archive with safe paths to proceed normally', async () => {
+    const appsDir = path.join(tempHome, 'Applications');
+    const { handleBarInstall } = await loadInstallSubcommand();
+
+    // Simulate an extractor that validates entries and finds no traversal
+    const safeExtract = async (_url: string, dest: string) => {
+      // All entry paths are safe relative paths — no ".." or absolute
+      fs.mkdirSync(path.join(dest, 'CCS Bar.app'), { recursive: true });
+    };
+
+    await handleBarInstall([], {
+      fetchReleaseAsset: async () => ({
+        downloadUrl: FAKE_DOWNLOAD_URL,
+        version: FAKE_VERSION,
+      }),
+      downloadAndExtract: safeExtract,
+      verifyCompat: async () => ({ version: FAKE_VERSION, compatible: true }),
+      getCcsDir: () => path.join(tempHome, '.ccs'),
+      getAppsDir: () => appsDir,
+    });
+
+    const allOutput = consoleOutput.join('\n');
+    expect(allOutput).toMatch(/\[OK\]/);
+    expect(allOutput).not.toMatch(/\[X\]/);
+  });
+});