security(aikido): implement and fix detected vulnerabities

Author: shouze

.github/actions/start-preview-server/action.yml

@@ -32,17 +32,28 @@ runs:
   steps:
     - name: Start VitePress preview
       shell: bash
-      run: bun run docs:preview -- --port ${{ inputs.port }} &
+      env:
+        PORT: ${{ inputs.port }}
+      run: bun run docs:preview -- --port "$PORT" &
 
     - name: Wait for preview server to be ready
       shell: bash
+      env:
+        PORT: ${{ inputs.port }}
+        BASE: ${{ inputs.base }}
+        TIMEOUT: ${{ inputs.timeout }}
       run: |
-        URL="http://localhost:${{ inputs.port }}${{ inputs.base }}"
+        # Validate TIMEOUT is a positive integer to prevent arithmetic injection
+        if ! printf '%s' "${TIMEOUT}" | grep -qE '^[0-9]+$'; then
+          echo "::error::timeout input must be a positive integer, got: ${TIMEOUT}"
+          exit 1
+        fi
+        URL="http://localhost:${PORT}${BASE}"
         echo "Waiting for ${URL} …"
-        DEADLINE=$(( $(date +%s) + ${{ inputs.timeout }} ))
+        DEADLINE=$(( $(date +%s) + TIMEOUT ))
         until curl -sf "${URL}" > /dev/null 2>&1; do
           if [ "$(date +%s)" -ge "${DEADLINE}" ]; then
-            echo "::error::Preview server did not respond within ${{ inputs.timeout }} s on ${URL}."
+            echo "::error::Preview server did not respond within ${TIMEOUT} s on ${URL}."
             exit 1
           fi
           echo "… retrying in 2 s"

.github/instructions/aikido_rules.instructions.md

@@ -0,0 +1,14 @@
+---
+applyTo: "**"
+description: Aikido MCP Security Rules
+---
+
+# Project security best practices
+
+- Always run **aikido_full_scan** on generated, added and modified first-party code, unless the prompt explicitly says not to.
+- Always provide the full file content to the scanner.
+- If the scan identifies any security issues in newly generated or modified code, fix the issues using the scan result and remediation provided by **Aikido**.
+- After applying fixes, run aikido_full_scan again to verify that the issues were successfully resolved, and no new issues were introduced.
+- Repeat the fix-and-rescan cycle until the code passes with zero remaining or newly introduced security issues.
+- If the **Aikido MCP server** required to run scans is not installed or not started successfully, inform the user with a message directing them to install it using the official Aikido MCP setup guide at:  
+  **https://help.aikido.dev/ide-plugins/aikido-mcp**.

bun.lock

@@ -59,14 +59,19 @@
     "@lhci/cli": "^0.15.1",
     "@playwright/test": "^1.58.2",
     "@resvg/resvg-js": "^2.6.2",
-    "bun-types": "^1.3.10",
-    "knip": "^5.86.0",
+    "bun-types": "^1.3.11",
+    "knip": "^6.0.6",
     "mermaid": "^11.13.0",
-    "oxfmt": "^0.40.0",
-    "oxlint": "^1.55.0",
+    "oxfmt": "^0.42.0",
+    "oxlint": "^1.57.0",
     "pa11y-ci": "^4.1.0",
     "sharp": "^0.34.5",
-    "vitepress": "^1.6.3",
-    "vitepress-mermaid-renderer": "^1.1.18"
+    "vitepress": "^1.6.4",
+    "vitepress-mermaid-renderer": "^1.1.20"
+  },
+  "overrides": {
+    "brace-expansion": "1.1.13",
+    "dompurify": "3.3.2",
+    "undici": "7.24.1"
   }
 }

package.json

@@ -1,5 +1,5 @@
 import { afterEach, beforeEach, describe, expect, it } from "bun:test";
-import { mkdirSync, rmSync, utimesSync, writeFileSync } from "node:fs";
+import { existsSync, mkdirSync, rmSync, utimesSync, writeFileSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import { CACHE_TTL_MS, getCacheDir, getCacheKey, readCache, writeCache } from "./cache.ts";
@@ -47,7 +47,10 @@ describe("getCacheDir", () => {
     const originalPlatform = Object.getOwnPropertyDescriptor(process, "platform");
     const originalLocalAppData = process.env.LOCALAPPDATA;
     try {
-      Object.defineProperty(process, "platform", { value: "win32", configurable: true });
+      Object.defineProperty(process, "platform", {
+        value: "win32",
+        configurable: true,
+      });
       process.env.LOCALAPPDATA = "C:\\Users\\user\\AppData\\Local";
       const dir = getCacheDir();
       // path.join uses the host OS separator in tests (macOS: /), so we just
@@ -67,7 +70,10 @@ describe("getCacheDir", () => {
     const originalPlatform = Object.getOwnPropertyDescriptor(process, "platform");
     const originalLocalAppData = process.env.LOCALAPPDATA;
     try {
-      Object.defineProperty(process, "platform", { value: "win32", configurable: true });
+      Object.defineProperty(process, "platform", {
+        value: "win32",
+        configurable: true,
+      });
       delete process.env.LOCALAPPDATA;
       const dir = getCacheDir();
       expect(dir).toContain("AppData");
@@ -84,7 +90,10 @@ describe("getCacheDir", () => {
     const originalPlatform = Object.getOwnPropertyDescriptor(process, "platform");
     const originalXdg = process.env.XDG_CACHE_HOME;
     try {
-      Object.defineProperty(process, "platform", { value: "linux", configurable: true });
+      Object.defineProperty(process, "platform", {
+        value: "linux",
+        configurable: true,
+      });
       process.env.XDG_CACHE_HOME = "/custom/xdg/cache";
       const dir = getCacheDir();
       expect(dir).toContain("custom");
@@ -102,7 +111,10 @@ describe("getCacheDir", () => {
     const originalPlatform = Object.getOwnPropertyDescriptor(process, "platform");
     const originalXdg = process.env.XDG_CACHE_HOME;
     try {
-      Object.defineProperty(process, "platform", { value: "linux", configurable: true });
+      Object.defineProperty(process, "platform", {
+        value: "linux",
+        configurable: true,
+      });
       delete process.env.XDG_CACHE_HOME;
       const dir = getCacheDir();
       expect(dir).toContain(".cache");
@@ -117,7 +129,10 @@ describe("getCacheDir", () => {
     delete process.env.GITHUB_CODE_SEARCH_CACHE_DIR;
     const originalPlatform = Object.getOwnPropertyDescriptor(process, "platform");
     try {
-      Object.defineProperty(process, "platform", { value: "freebsd", configurable: true });
+      Object.defineProperty(process, "platform", {
+        value: "freebsd",
+        configurable: true,
+      });
       const dir = getCacheDir();
       expect(dir).toContain(".github-code-search");
       expect(dir).toContain("cache");
@@ -224,3 +239,60 @@ describe("writeCache / readCache round-trip", () => {
     expect(() => writeCache("key.json", { data: 1 })).not.toThrow();
   });
 });
+
+// ─── Path Traversal Security Tests ────────────────────────────────────────────
+
+describe("Path traversal vulnerability mitigation", () => {
+  it("rejects readCache with relative parent directory traversal (../)", () => {
+    // Attempt to read outside the cache directory using ../
+    const maliciousKey = "../../../etc/passwd";
+    const result = readCache(maliciousKey);
+    expect(result).toBeNull();
+  });
+
+  it("rejects writeCache with relative parent directory traversal (../)", () => {
+    // Use a subdirectory as cache dir so we have a writable location just above it
+    const cacheSubDir = join(TEST_CACHE_DIR, "sub");
+    mkdirSync(cacheSubDir, { recursive: true });
+    process.env.GITHUB_CODE_SEARCH_CACHE_DIR = cacheSubDir;
+    // This key resolves to TEST_CACHE_DIR/escaped.json — one level above cacheSubDir
+    const maliciousKey = "../escaped.json";
+    const outsidePath = join(TEST_CACHE_DIR, "escaped.json");
+    expect(() => writeCache(maliciousKey, { exploit: true })).not.toThrow();
+    // Prove the file was NOT created at the resolved outside location
+    expect(existsSync(outsidePath)).toBe(false);
+  });
+
+  it("rejects readCache with absolute path", () => {
+    // Attempt to read an absolute path outside the cache directory
+    // Use a portable OS temp path that exists on all platforms
+    const maliciousKey = join(tmpdir(), "gcs-read-absolute-test.json");
+    const result = readCache(maliciousKey);
+    expect(result).toBeNull();
+  });
+
+  it("rejects writeCache with absolute path", () => {
+    // Attempt to write to an absolute path outside the cache directory
+    const outsidePath = join(tmpdir(), `gcs-write-absolute-test-${process.pid}.json`);
+    expect(() => writeCache(outsidePath, { exploit: "absolute" })).not.toThrow();
+    // Prove the file was NOT created at the absolute outside location
+    expect(existsSync(outsidePath)).toBe(false);
+  });
+
+  it("rejects readCache with encoded path traversal (%2e%2e/)", () => {
+    // URL-encoded path traversal attempt
+    const maliciousKey = "%2e%2e/%2e%2e/etc/passwd";
+    const result = readCache(maliciousKey);
+    // The key is treated as a literal filename, not decoded, so it's safe
+    // but should still not exist
+    expect(result).toBeNull();
+  });
+
+  it("allows reading legitimate cache files with safe names", () => {
+    // Verify that normal operation still works
+    const safeKey = "teams__myorg__squad-.json";
+    const data = { teams: ["squad-alpha"] };
+    writeCache(safeKey, data);
+    expect(readCache(safeKey)).toEqual(data);
+  });
+});

src/cache.test.ts

@@ -1,6 +1,16 @@
-import { mkdirSync, readFileSync, statSync, writeFileSync } from "node:fs";
+import {
+  closeSync,
+  constants,
+  fstatSync,
+  mkdirSync,
+  openSync,
+  readFileSync,
+  renameSync,
+  writeFileSync,
+} from "node:fs";
+import { randomBytes } from "node:crypto";
 import { homedir } from "node:os";
-import { join } from "node:path";
+import { isAbsolute, join, relative, resolve, sep } from "node:path";
 
 // ─── Team-list cache ──────────────────────────────────────────────────────────
 //
@@ -67,15 +77,27 @@ export function getCacheKey(org: string, prefixes: string[]): string {
  * - the file is older than `CACHE_TTL_MS`
  */
 export function readCache<T>(key: string): T | null {
-  const filePath = join(getCacheDir(), key);
+  const base = resolve(getCacheDir());
+  const target = resolve(base, key);
+  const rel = relative(base, target);
+  if (rel === ".." || rel.startsWith(".." + sep) || isAbsolute(rel)) return null;
+  // Fix: open the file once and use fstatSync on the same fd to avoid a
+  // TOCTOU race condition between the age check and the read.
+  // O_NOFOLLOW prevents symlink hijacking in world-writable directories
+  // (CWE-377). On Windows the constant is undefined so we fall back to 0.
+  const O_NOFOLLOW = constants.O_NOFOLLOW ?? 0;
+  let fd: number | null = null;
   try {
-    const stat = statSync(filePath);
+    fd = openSync(target, constants.O_RDONLY | O_NOFOLLOW);
+    const stat = fstatSync(fd);
     const ageMs = Date.now() - stat.mtimeMs;
     if (ageMs > CACHE_TTL_MS) return null;
-    const raw = readFileSync(filePath, "utf8");
+    const raw = readFileSync(fd, "utf8");
     return JSON.parse(raw) as T;
   } catch {
     return null;
+  } finally {
+    if (fd !== null) closeSync(fd);
   }
 }
 
@@ -86,10 +108,21 @@ export function readCache<T>(key: string): T | null {
  * best-effort and must never crash the CLI.
  */
 export function writeCache<T>(key: string, data: T): void {
+  // Fix: write to a randomly-named temp file first, then rename atomically to
+  // the target path. This avoids symlink/race attacks on the cache directory
+  // and ensures readers never observe a partially-written file.
   try {
     const dir = getCacheDir();
+    const base = resolve(dir);
+    const target = resolve(base, key);
+    const rel = relative(base, target);
+    if (rel === ".." || rel.startsWith(".." + sep) || isAbsolute(rel)) return;
     mkdirSync(dir, { recursive: true });
-    writeFileSync(join(dir, key), JSON.stringify(data), "utf8");
+    const tmpPath = `${target}.${randomBytes(6).toString("hex")}.tmp`;
+    // mode 0o600: owner-only read/write, so the temp file is private even
+    // when the parent directory is world-writable (CWE-377).
+    writeFileSync(tmpPath, JSON.stringify(data), { encoding: "utf8", flag: "wx", mode: 0o600 });
+    renameSync(tmpPath, target);
   } catch {
     // Best-effort: ignore write errors
   }

src/cache.ts

@@ -80,7 +80,7 @@ describe("isNewerVersion", () => {
 function makeAsset(name: string): ReleaseAsset {
   return {
     name,
-    browser_download_url: `https://example.com/releases/${name}`,
+    browser_download_url: `https://github.com/fulll/github-code-search/releases/download/v9.9.9/${name}`,
   };
 }
 

src/upgrade.test.ts

@@ -121,12 +121,38 @@ export async function fetchLatestRelease(
  * Uses a ".tmp" sibling file so the replacement is atomic on the same FS.
  */
 async function downloadBinary(url: string, dest: string, debug = false): Promise<void> {
+  // Validate URL to prevent SSRF attacks
+  const allowedDomains = ["github.com"];
+  try {
+    const parsedUrl = new URL(url);
+    if (!allowedDomains.includes(parsedUrl.hostname)) {
+      throw new Error("Invalid host");
+    }
+    if (parsedUrl.protocol !== "https:") {
+      throw new Error("Invalid protocol");
+    }
+  } catch {
+    throw new Error("Invalid URL");
+  }
+
   if (debug) process.stdout.write(`[debug] downloading from ${url}\n`);
   const res = await fetch(url);
   if (debug)
     process.stdout.write(
       `[debug] fetch response: status=${res.status} ok=${res.ok} url=${res.url}\n`,
     );
+  // Validate the final URL after potential redirects to guard against redirect-based SSRF.
+  // res.url is empty for mocked responses (tests), so we only validate when it is set.
+  if (res.url) {
+    try {
+      const finalUrl = new URL(res.url);
+      if (!allowedDomains.includes(finalUrl.hostname) || finalUrl.protocol !== "https:") {
+        throw new Error("Redirect blocked");
+      }
+    } catch {
+      throw new Error("Redirect blocked: final URL does not match allowed domains");
+    }
+  }
   if (!res.ok) {
     throw new Error(`Download failed (${res.status}): ${url}`);
   }