Merge branch 'feat/vibe-files-prompt' into feat/vibe-generate-publish

Author: Braden-sui

lib/vibe-utils.js

@@ -327,11 +327,16 @@ export const verboseLog = (prefix, message, verbose) => {
  * Fetch with timeout support using AbortController.
  * Prevents fetch() from hanging indefinitely when server doesn't respond.
  *
+ * SECURITY: Properly combines timeout signal with caller-provided signals
+ * using AbortSignal.any(). This ensures external cancellation (e.g., user
+ * aborting an in-flight request) is respected alongside the timeout.
+ *
  * @param {string} url - URL to fetch from
- * @param {RequestInit} [init] - Fetch options (signal will be merged)
+ * @param {RequestInit} [init] - Fetch options (signal will be merged, not overwritten)
  * @param {number} [timeoutMs=30000] - Timeout in milliseconds
  * @returns {Promise<Response>} Fetch Response object
  * @throws {Error} FETCH_TIMEOUT if request exceeds timeout
+ * @throws {Error} AbortError if caller's signal triggered abort (rethrown as-is)
  *
  * @example
  * try {
@@ -341,30 +346,62 @@ export const verboseLog = (prefix, message, verbose) => {
  *     console.log('Request timed out');
  *   }
  * }
+ *
+ * @example
+ * // With external abort signal
+ * const controller = new AbortController();
+ * setTimeout(() => controller.abort(), 1000); // User cancels after 1s
+ * try {
+ *   await fetchWithTimeout("https://api.example.com/slow", { signal: controller.signal }, 30000);
+ * } catch (err) {
+ *   if (err.name === 'AbortError') {
+ *     console.log('Request was cancelled by user');
+ *   }
+ * }
  */
 export const fetchWithTimeout = async (
   url,
   init = {},
   timeoutMs = defaultTimeoutMs,
 ) => {
-  const controller = new AbortController();
-  const timeoutId = setTimeout(() => controller.abort(), timeoutMs);
+  // Create timeout-specific controller to track if timeout triggered the abort
+  const timeoutController = new AbortController();
+  const timeoutId = setTimeout(() => timeoutController.abort(), timeoutMs);
+
+  // Combine timeout signal with any caller-provided signal
+  // This ensures external cancellation is respected alongside timeout
+  const signals = [timeoutController.signal];
+  if (init.signal) {
+    signals.push(init.signal);
+  }
+  const combinedSignal = AbortSignal.any(signals);
 
   try {
+    // Check if external signal is already aborted before starting fetch
+    if (init.signal?.aborted) {
+      throw init.signal.reason ?? new DOMException("Aborted", "AbortError");
+    }
+
     const response = await fetch(url, {
       ...init,
-      signal: controller.signal,
+      signal: combinedSignal,
     });
     return response;
   } catch (err) {
-    // AbortError is thrown when AbortController.abort() is called
+    // AbortError is thrown when any signal aborts
     if (err.name === "AbortError") {
-      throw createError({
-        ...FetchTimeout,
-        message: `Request timed out after ${timeoutMs}ms`,
-        url,
-        timeoutMs,
-      });
+      // Check if OUR timeout triggered the abort
+      if (timeoutController.signal.aborted) {
+        throw createError({
+          ...FetchTimeout,
+          message: `Request timed out after ${timeoutMs}ms`,
+          url,
+          timeoutMs,
+        });
+      }
+      // External signal triggered abort - rethrow as-is for caller to handle
+      // This preserves the original abort reason and allows proper cancellation flow
+      throw err;
     }
     const originalCode = err && typeof err.code === "string" ? err.code : null;
     const retryable =
@@ -859,11 +896,15 @@ export const isPathSafe = (filePath) => {
   const normalizedPath = normalizePathForSecurity(filePath);
 
   // Reject absolute paths (check both original and normalized)
+  // SECURITY: Includes Windows root-relative (\path) and UNC paths (\\server\share)
+  // Both are absolute paths that could escape the working directory
   if (
     filePath.startsWith("/") ||
     normalizedPath.startsWith("/") ||
     /^[A-Za-z]:/.test(filePath) ||
-    /^[A-Za-z]:/.test(normalizedPath)
+    /^[A-Za-z]:/.test(normalizedPath) ||
+    filePath.startsWith("\\") ||
+    normalizedPath.startsWith("\\")
   ) {
     return { safe: false, reason: "Absolute paths are not allowed" };
   }

lib/vibe-utils.test.js

@@ -306,19 +306,24 @@ describe("fetchWithTimeout", () => {
   });
 
   test("throws FETCH_TIMEOUT on abort", async () => {
-    // Simulate a request that gets aborted due to timeout
+    // Simulate a slow request that will be aborted by our timeout
+    // The mock listens to the signal and rejects when aborted
     mockFetch.mockImplementationOnce(
-      () =>
+      (url, options) =>
         new Promise((_, reject) => {
-          // Immediately simulate abort behavior
-          const error = new Error("Aborted");
-          error.name = "AbortError";
-          reject(error);
+          // When the signal aborts (from our timeout), reject with AbortError
+          options.signal.addEventListener("abort", () => {
+            const error = new Error("Aborted");
+            error.name = "AbortError";
+            reject(error);
+          });
+          // This promise would never resolve on its own - simulates slow server
         }),
     );
 
     let error;
     try {
+      // Short timeout so test runs fast
       await fetchWithTimeout("https://api.test.com/slow", {}, 50);
     } catch (e) {
       error = e;
@@ -340,6 +345,91 @@ describe("fetchWithTimeout", () => {
     });
   });
 
+  test("respects external AbortSignal for cancellation", async () => {
+    // Create an external controller that will abort
+    const externalController = new AbortController();
+
+    // Mock fetch to simulate a pending request that gets aborted externally
+    mockFetch.mockImplementationOnce(
+      (url, options) =>
+        new Promise((_, reject) => {
+          // Listen for abort on the combined signal
+          options.signal.addEventListener("abort", () => {
+            const error = new Error("Aborted");
+            error.name = "AbortError";
+            reject(error);
+          });
+        }),
+    );
+
+    // Start the fetch, then abort via external signal
+    const fetchPromise = fetchWithTimeout(
+      "https://api.test.com/slow",
+      { signal: externalController.signal },
+      30000, // Long timeout so it won't timeout first
+    );
+
+    // Abort via external signal
+    externalController.abort();
+
+    let error;
+    try {
+      await fetchPromise;
+    } catch (e) {
+      error = e;
+    }
+
+    // External abort should NOT be wrapped as FETCH_TIMEOUT
+    // It should be rethrown as-is (AbortError)
+    assert({
+      given: "external signal aborts request",
+      should: "throw AbortError (not FETCH_TIMEOUT)",
+      actual: error?.name,
+      expected: "AbortError",
+    });
+
+    // Should NOT have FETCH_TIMEOUT code
+    assert({
+      given: "external signal aborts request",
+      should: "not be classified as timeout",
+      actual: error?.cause?.code !== "FETCH_TIMEOUT",
+      expected: true,
+    });
+  });
+
+  test("rejects immediately if external signal already aborted", async () => {
+    // Create already-aborted signal
+    const abortedController = new AbortController();
+    abortedController.abort();
+
+    let error;
+    try {
+      await fetchWithTimeout(
+        "https://api.test.com/data",
+        { signal: abortedController.signal },
+        5000,
+      );
+    } catch (e) {
+      error = e;
+    }
+
+    // Should throw immediately without calling fetch
+    assert({
+      given: "already-aborted signal",
+      should: "throw AbortError immediately",
+      actual: error?.name,
+      expected: "AbortError",
+    });
+
+    // Fetch should not have been called
+    assert({
+      given: "already-aborted signal",
+      should: "not call fetch",
+      actual: mockFetch.mock.calls.length,
+      expected: 0,
+    });
+  });
+
   test("passes through non-timeout errors with wrapping", async () => {
     const networkError = new Error("Network failure");
     networkError.code = "ECONNREFUSED";
@@ -879,6 +969,75 @@ describe("isPathSafe", () => {
       expected: true,
     });
   });
+
+  test("rejects UNC paths", () => {
+    const result = isPathSafe("\\\\server\\share\\file.txt");
+
+    assert({
+      given: "UNC path (Windows network path)",
+      should: "return safe false",
+      actual: result.safe,
+      expected: false,
+    });
+
+    assert({
+      given: "UNC path rejection",
+      should: "give absolute path reason",
+      actual: result.reason,
+      expected: "Absolute paths are not allowed",
+    });
+  });
+
+  test("rejects Windows root-relative paths (single backslash)", () => {
+    // \Windows\System32 is root-relative on Windows (resolves to current drive root)
+    // This is an absolute path that should be rejected
+    const result = isPathSafe("\\Windows\\System32");
+
+    assert({
+      given: "Windows root-relative path (single backslash)",
+      should: "return safe false",
+      actual: result.safe,
+      expected: false,
+    });
+
+    assert({
+      given: "root-relative path rejection",
+      should: "give absolute path reason",
+      actual: result.reason,
+      expected: "Absolute paths are not allowed",
+    });
+  });
+
+  test("rejects paths starting with single backslash", () => {
+    // Various single-backslash absolute path patterns
+    const testCases = [
+      "\\temp\\file.txt",       // \temp\file.txt
+      "\\Users\\data.json",     // \Users\data.json
+      "\\",                        // Just root
+    ];
+
+    for (const testPath of testCases) {
+      const result = isPathSafe(testPath);
+      assert({
+        given: `path starting with backslash: ${testPath}`,
+        should: "return safe false",
+        actual: result.safe,
+        expected: false,
+      });
+    }
+  });
+
+  test("rejects UNC paths with extended prefix", () => {
+    // \\?\C:\ is Windows extended-length path prefix
+    const result = isPathSafe("\\\\?\\C:\\Windows\\System32");
+
+    assert({
+      given: "UNC extended-length path",
+      should: "return safe false",
+      actual: result.safe,
+      expected: false,
+    });
+  });
 });
 
 // =============================================================================