Skip to content

Commit f69ba41

Browse files
committed
security(aikido): implement and fix detected vulnerabities
1 parent 57a506f commit f69ba41

File tree

8 files changed

+260
-73
lines changed

8 files changed

+260
-73
lines changed

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,17 +32,23 @@ runs:
3232
steps:
3333
- name: Start VitePress preview
3434
shell: bash
35-
run: bun run docs:preview -- --port ${{ inputs.port }} &
35+
env:
36+
PORT: ${{ inputs.port }}
37+
run: bun run docs:preview -- --port "$PORT" &
3638

3739
- name: Wait for preview server to be ready
3840
shell: bash
41+
env:
42+
PORT: ${{ inputs.port }}
43+
BASE: ${{ inputs.base }}
44+
TIMEOUT: ${{ inputs.timeout }}
3945
run: |
40-
URL="http://localhost:${{ inputs.port }}${{ inputs.base }}"
46+
URL="http://localhost:${PORT}${BASE}"
4147
echo "Waiting for ${URL} …"
42-
DEADLINE=$(( $(date +%s) + ${{ inputs.timeout }} ))
48+
DEADLINE=$(( $(date +%s) + ${TIMEOUT} ))
4349
until curl -sf "${URL}" > /dev/null 2>&1; do
4450
if [ "$(date +%s)" -ge "${DEADLINE}" ]; then
45-
echo "::error::Preview server did not respond within ${{ inputs.timeout }} s on ${URL}."
51+
echo "::error::Preview server did not respond within ${TIMEOUT} s on ${URL}."
4652
exit 1
4753
fi
4854
echo "… retrying in 2 s"
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
applyTo: "**"
3+
description: Aikido MCP Security Rules
4+
---
5+
6+
# Project security best practices
7+
8+
- Always run **aikido_full_scan** on generated, added and modified first-party code, unless the prompt explicitly says not to.
9+
- Always provide the full file content to the scanner.
10+
- 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**.
11+
- After applying fixes, run aikido_full_scan again to verify that the issues were successfully resolved, and no new issues were introduced.
12+
- Repeat the fix-and-rescan cycle until the code passes with zero remaining or newly introduced security issues.
13+
- 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:
14+
**https://help.aikido.dev/ide-plugins/aikido-mcp**.

bun.lock

Lines changed: 104 additions & 51 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,19 @@
5959
"@lhci/cli": "^0.15.1",
6060
"@playwright/test": "^1.58.2",
6161
"@resvg/resvg-js": "^2.6.2",
62-
"bun-types": "^1.3.10",
63-
"knip": "^5.86.0",
62+
"bun-types": "^1.3.11",
63+
"knip": "^6.0.6",
6464
"mermaid": "^11.13.0",
65-
"oxfmt": "^0.40.0",
66-
"oxlint": "^1.55.0",
65+
"oxfmt": "^0.42.0",
66+
"oxlint": "^1.57.0",
6767
"pa11y-ci": "^4.1.0",
6868
"sharp": "^0.34.5",
69-
"vitepress": "^1.6.3",
70-
"vitepress-mermaid-renderer": "^1.1.18"
69+
"vitepress": "^1.6.4",
70+
"vitepress-mermaid-renderer": "^1.1.20"
71+
},
72+
"overrides": {
73+
"brace-expansion": "1.1.13",
74+
"dompurify": "3.3.2",
75+
"undici": "7.24.1"
7176
}
7277
}

src/cache.test.ts

Lines changed: 73 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,10 @@ describe("getCacheDir", () => {
4747
const originalPlatform = Object.getOwnPropertyDescriptor(process, "platform");
4848
const originalLocalAppData = process.env.LOCALAPPDATA;
4949
try {
50-
Object.defineProperty(process, "platform", { value: "win32", configurable: true });
50+
Object.defineProperty(process, "platform", {
51+
value: "win32",
52+
configurable: true,
53+
});
5154
process.env.LOCALAPPDATA = "C:\\Users\\user\\AppData\\Local";
5255
const dir = getCacheDir();
5356
// path.join uses the host OS separator in tests (macOS: /), so we just
@@ -67,7 +70,10 @@ describe("getCacheDir", () => {
6770
const originalPlatform = Object.getOwnPropertyDescriptor(process, "platform");
6871
const originalLocalAppData = process.env.LOCALAPPDATA;
6972
try {
70-
Object.defineProperty(process, "platform", { value: "win32", configurable: true });
73+
Object.defineProperty(process, "platform", {
74+
value: "win32",
75+
configurable: true,
76+
});
7177
delete process.env.LOCALAPPDATA;
7278
const dir = getCacheDir();
7379
expect(dir).toContain("AppData");
@@ -84,7 +90,10 @@ describe("getCacheDir", () => {
8490
const originalPlatform = Object.getOwnPropertyDescriptor(process, "platform");
8591
const originalXdg = process.env.XDG_CACHE_HOME;
8692
try {
87-
Object.defineProperty(process, "platform", { value: "linux", configurable: true });
93+
Object.defineProperty(process, "platform", {
94+
value: "linux",
95+
configurable: true,
96+
});
8897
process.env.XDG_CACHE_HOME = "/custom/xdg/cache";
8998
const dir = getCacheDir();
9099
expect(dir).toContain("custom");
@@ -102,7 +111,10 @@ describe("getCacheDir", () => {
102111
const originalPlatform = Object.getOwnPropertyDescriptor(process, "platform");
103112
const originalXdg = process.env.XDG_CACHE_HOME;
104113
try {
105-
Object.defineProperty(process, "platform", { value: "linux", configurable: true });
114+
Object.defineProperty(process, "platform", {
115+
value: "linux",
116+
configurable: true,
117+
});
106118
delete process.env.XDG_CACHE_HOME;
107119
const dir = getCacheDir();
108120
expect(dir).toContain(".cache");
@@ -117,7 +129,10 @@ describe("getCacheDir", () => {
117129
delete process.env.GITHUB_CODE_SEARCH_CACHE_DIR;
118130
const originalPlatform = Object.getOwnPropertyDescriptor(process, "platform");
119131
try {
120-
Object.defineProperty(process, "platform", { value: "freebsd", configurable: true });
132+
Object.defineProperty(process, "platform", {
133+
value: "freebsd",
134+
configurable: true,
135+
});
121136
const dir = getCacheDir();
122137
expect(dir).toContain(".github-code-search");
123138
expect(dir).toContain("cache");
@@ -224,3 +239,56 @@ describe("writeCache / readCache round-trip", () => {
224239
expect(() => writeCache("key.json", { data: 1 })).not.toThrow();
225240
});
226241
});
242+
243+
// ─── Path Traversal Security Tests ────────────────────────────────────────────
244+
245+
describe("Path traversal vulnerability mitigation", () => {
246+
it("rejects readCache with relative parent directory traversal (../)", () => {
247+
// Attempt to read outside the cache directory using ../
248+
const maliciousKey = "../../../etc/passwd";
249+
const result = readCache(maliciousKey);
250+
expect(result).toBeNull();
251+
});
252+
253+
it("rejects writeCache with relative parent directory traversal (../)", () => {
254+
// Attempt to write outside the cache directory using ../
255+
const maliciousKey = "../../../tmp/malicious.json";
256+
const maliciousData = { exploit: "attempt" };
257+
// Should not throw, but should silently reject
258+
expect(() => writeCache(maliciousKey, maliciousData)).not.toThrow();
259+
// Verify the file was not written to the malicious location
260+
expect(readCache(maliciousKey)).toBeNull();
261+
});
262+
263+
it("rejects readCache with absolute path", () => {
264+
// Attempt to read an absolute path outside the cache directory
265+
const maliciousKey = "/etc/passwd";
266+
const result = readCache(maliciousKey);
267+
expect(result).toBeNull();
268+
});
269+
270+
it("rejects writeCache with absolute path", () => {
271+
// Attempt to write to an absolute path outside the cache directory
272+
const maliciousKey = "/tmp/malicious-absolute.json";
273+
const maliciousData = { exploit: "absolute" };
274+
expect(() => writeCache(maliciousKey, maliciousData)).not.toThrow();
275+
expect(readCache(maliciousKey)).toBeNull();
276+
});
277+
278+
it("rejects readCache with encoded path traversal (%2e%2e/)", () => {
279+
// URL-encoded path traversal attempt
280+
const maliciousKey = "%2e%2e/%2e%2e/etc/passwd";
281+
const result = readCache(maliciousKey);
282+
// The key is treated as a literal filename, not decoded, so it's safe
283+
// but should still not exist
284+
expect(result).toBeNull();
285+
});
286+
287+
it("allows reading legitimate cache files with safe names", () => {
288+
// Verify that normal operation still works
289+
const safeKey = "teams__myorg__squad-.json";
290+
const data = { teams: ["squad-alpha"] };
291+
writeCache(safeKey, data);
292+
expect(readCache(safeKey)).toEqual(data);
293+
});
294+
});

src/cache.ts

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,15 @@
1-
import { mkdirSync, readFileSync, statSync, writeFileSync } from "node:fs";
1+
import {
2+
closeSync,
3+
fstatSync,
4+
mkdirSync,
5+
openSync,
6+
readFileSync,
7+
renameSync,
8+
writeFileSync,
9+
} from "node:fs";
10+
import { randomBytes } from "node:crypto";
211
import { homedir } from "node:os";
3-
import { join } from "node:path";
12+
import { join, resolve, relative } from "node:path";
413

514
// ─── Team-list cache ──────────────────────────────────────────────────────────
615
//
@@ -67,15 +76,24 @@ export function getCacheKey(org: string, prefixes: string[]): string {
6776
* - the file is older than `CACHE_TTL_MS`
6877
*/
6978
export function readCache<T>(key: string): T | null {
70-
const filePath = join(getCacheDir(), key);
79+
const base = resolve(getCacheDir());
80+
const target = resolve(base, key);
81+
const rel = relative(base, target);
82+
if (rel.startsWith("..") || resolve(rel) === rel) return null;
83+
// Fix: open the file once and use fstatSync on the same fd to avoid a
84+
// TOCTOU race condition between the age check and the read.
85+
let fd: number | null = null;
7186
try {
72-
const stat = statSync(filePath);
87+
fd = openSync(target, "r");
88+
const stat = fstatSync(fd);
7389
const ageMs = Date.now() - stat.mtimeMs;
7490
if (ageMs > CACHE_TTL_MS) return null;
75-
const raw = readFileSync(filePath, "utf8");
91+
const raw = readFileSync(fd, "utf8");
7692
return JSON.parse(raw) as T;
7793
} catch {
7894
return null;
95+
} finally {
96+
if (fd !== null) closeSync(fd);
7997
}
8098
}
8199

@@ -86,10 +104,19 @@ export function readCache<T>(key: string): T | null {
86104
* best-effort and must never crash the CLI.
87105
*/
88106
export function writeCache<T>(key: string, data: T): void {
107+
// Fix: write to a randomly-named temp file first, then rename atomically to
108+
// the target path. This avoids symlink/race attacks on the cache directory
109+
// and ensures readers never observe a partially-written file.
89110
try {
90111
const dir = getCacheDir();
112+
const base = resolve(dir);
113+
const target = resolve(base, key);
114+
const rel = relative(base, target);
115+
if (rel.startsWith("..") || resolve(rel) === rel) return;
91116
mkdirSync(dir, { recursive: true });
92-
writeFileSync(join(dir, key), JSON.stringify(data), "utf8");
117+
const tmpPath = `${target}.${randomBytes(6).toString("hex")}.tmp`;
118+
writeFileSync(tmpPath, JSON.stringify(data), { encoding: "utf8", flag: "wx" });
119+
renameSync(tmpPath, target);
93120
} catch {
94121
// Best-effort: ignore write errors
95122
}

src/upgrade.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ describe("isNewerVersion", () => {
8080
function makeAsset(name: string): ReleaseAsset {
8181
return {
8282
name,
83-
browser_download_url: `https://example.com/releases/${name}`,
83+
browser_download_url: `https://github.com/fulll/github-code-search/releases/download/v9.9.9/${name}`,
8484
};
8585
}
8686

src/upgrade.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,20 @@ export async function fetchLatestRelease(
121121
* Uses a ".tmp" sibling file so the replacement is atomic on the same FS.
122122
*/
123123
async function downloadBinary(url: string, dest: string, debug = false): Promise<void> {
124+
// Validate URL to prevent SSRF attacks
125+
try {
126+
const parsedUrl = new URL(url);
127+
const allowedDomains = ["github.com"];
128+
if (!allowedDomains.includes(parsedUrl.hostname)) {
129+
throw new Error("Invalid host");
130+
}
131+
if (!["http:", "https:"].includes(parsedUrl.protocol)) {
132+
throw new Error("Invalid protocol");
133+
}
134+
} catch {
135+
throw new Error("Invalid URL");
136+
}
137+
124138
if (debug) process.stdout.write(`[debug] downloading from ${url}\n`);
125139
const res = await fetch(url);
126140
if (debug)

0 commit comments

Comments
 (0)