security(aikido): implement and fix detected vulnerabities

Author: shouze

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

@@ -32,17 +32,43 @@ 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 strictly positive integer to prevent arithmetic injection
+        if ! printf '%s' "${TIMEOUT}" | grep -qE '^[1-9][0-9]*$'; then
+          echo "::error::timeout input must be a strictly positive integer, got: ${TIMEOUT}"
+          exit 1
+        fi
+        # Validate PORT is a strictly positive integer to prevent userinfo injection (e.g. 80@attacker.tld)
+        if ! printf '%s' "${PORT}" | grep -qE '^[1-9][0-9]*$'; then
+          echo "::error::port input must be a strictly positive integer, got: ${PORT}"
+          exit 1
+        fi
+        # Validate PORT is within the valid TCP port range to avoid unreachable URLs
+        if [ "${PORT}" -lt 1 ] || [ "${PORT}" -gt 65535 ]; then
+          echo "::error::port input must be between 1 and 65535, got: ${PORT}"
+          exit 1
+        fi
+        # Validate BASE starts with / and contains no spaces to prevent URL injection
+        if ! printf '%s' "${BASE}" | grep -qE '^/[^ ]*$'; then
+          echo "::error::base input must start with / and contain no spaces, got: ${BASE}"
+          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/dependabot.yml

@@ -0,0 +1,11 @@
+# To get started with Dependabot version updates, you'll need to specify which
+# package ecosystems to update and where the package manifests are located.
+# Please see the documentation for all configuration options:
+# https://docs.github.com/code-security/dependabot/dependabot-version-updates/configuration-options-for-the-dependabot.yml-file
+
+version: 2
+updates:
+  - package-ecosystem: "bun" # Covers package.json / bun.lockb managed via Bun
+    directory: "/" # Location of package manifests
+    schedule:
+      interval: "weekly"

.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,15 +1,17 @@
 import { afterEach, beforeEach, describe, expect, it } from "bun:test";
-import { mkdirSync, rmSync, utimesSync, writeFileSync } from "node:fs";
+import { existsSync, mkdirSync, mkdtempSync, 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";
 
-// Use env-var override to redirect the cache to an isolated temp directory
-const TEST_CACHE_DIR = join(tmpdir(), `gcs-cache-test-${process.pid}`);
+// Use env-var override to redirect the cache to a process-private temp directory.
+// mkdtempSync creates the directory with 0o700 permissions (owner-only access),
+// which is the safe temp-dir pattern recognised by static analysers (CWE-377).
+let TEST_CACHE_DIR: string;
 
 beforeEach(() => {
+  TEST_CACHE_DIR = mkdtempSync(join(tmpdir(), "gcs-cache-test-"));
   process.env.GITHUB_CODE_SEARCH_CACHE_DIR = TEST_CACHE_DIR;
-  mkdirSync(TEST_CACHE_DIR, { recursive: true });
 });
 
 afterEach(() => {
@@ -47,7 +49,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 +72,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 +92,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 +113,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 +131,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 +241,61 @@ 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);
+    // URL-encoded ".." segments that also contain a literal "/" path separator.
+    // readCache rejects it because the key contains a literal "/" (separator check),
+    // not because it decodes the percent-encoded characters.
+    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,17 @@
-import { mkdirSync, readFileSync, statSync, writeFileSync } from "node:fs";
+import {
+  closeSync,
+  constants,
+  fstatSync,
+  mkdirSync,
+  openSync,
+  readFileSync,
+  renameSync,
+  unlinkSync,
+  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 +78,33 @@ 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);
+  // Reject keys containing path separators or absolute-path markers
+  // before any filesystem operation (defence-in-depth for static analysers).
+  if (key.includes("/") || key.includes("\\") || isAbsolute(key)) return null;
+  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);
+    // Verify the descriptor points to a regular file — prevents reading from
+    // FIFOs or device files that could block or expose unintended data (CWE-377).
+    if (!stat.isFile()) return null;
     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 +115,34 @@ 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 {
+    // Reject keys containing path separators or absolute-path markers
+    // before any filesystem operation (defence-in-depth for static analysers).
+    if (key.includes("/") || key.includes("\\") || isAbsolute(key)) return;
     const dir = getCacheDir();
-    mkdirSync(dir, { recursive: true });
-    writeFileSync(join(dir, key), JSON.stringify(data), "utf8");
+    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, mode: 0o700 });
+    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 });
+    // Fix: on Windows, renameSync fails if the destination already exists
+    // (unlike POSIX where it is atomic). Unlink the target first on win32
+    // to restore replace semantics without sacrificing atomicity on POSIX.
+    if (process.platform === "win32") {
+      try {
+        unlinkSync(target);
+      } catch {
+        // Target may not exist yet — ignore
+      }
+    }
+    renameSync(tmpPath, target);
   } catch {
     // Best-effort: ignore write errors
   }