fix(security): patch 5 vulnerabilities from security audit

- mask API keys in usage stats/history responses (apiKeyMasked)
- validate proxy URL scheme + reject shell metachars before env write
- escape HTML in OAuth callback page to prevent XSS
- atomic O_EXCL lock file to prevent TOCTOU race in MITM startServer
- set mitmIsRestarting guard synchronously before any await

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
hamsa0x7
2026-06-26 11:01:13 +07:00
committed by decolua
parent 520f5049bf
commit d8c2298d07
5 changed files with 364 additions and 14 deletions

View File

@@ -3,6 +3,12 @@ import { getAdapter } from "../driver.js";
import { parseJson, stringifyJson } from "../helpers/jsonCol.js";
import { getMeta, setMeta } from "../helpers/metaStore.js";
function maskApiKey(key) {
if (!key || typeof key !== "string") return null;
if (key.length <= 8) return key.charAt(0) + "***";
return key.slice(0, 8) + "***";
}
const PENDING_TIMEOUT_MS = 60 * 1000;
const RING_CAP = 50;
const CONN_CACHE_TTL_MS = 30 * 1000;
@@ -342,7 +348,7 @@ export async function getUsageHistory(filter = {}) {
return rows.map((r) => ({
timestamp: r.timestamp, provider: r.provider, model: r.model,
connectionId: r.connectionId, apiKey: r.apiKey, endpoint: r.endpoint,
connectionId: r.connectionId, apiKeyMasked: maskApiKey(r.apiKey), endpoint: r.endpoint,
cost: r.cost, status: r.status, tokens: parseJson(r.tokens, {}),
}));
}
@@ -516,9 +522,10 @@ export async function getUsageStats(period = "all") {
const apiKeyVal = ak.apiKey;
const keyInfo = apiKeyVal ? apiKeyMap[apiKeyVal] : null;
const keyName = keyInfo?.name || (apiKeyVal ? apiKeyVal.slice(0, 8) + "..." : "Local (No API Key)");
const apiKeyKey = apiKeyVal || "local-no-key";
const apiKeyMasked = maskApiKey(apiKeyVal);
const apiKeyKey = apiKeyMasked || "local-no-key";
if (!stats.byApiKey[akKey]) {
stats.byApiKey[akKey] = { requests: 0, promptTokens: 0, completionTokens: 0, cost: 0, rawModel, provider: providerDisplayName, apiKey: apiKeyVal, keyName, apiKeyKey, lastUsed: dateKey };
stats.byApiKey[akKey] = { requests: 0, promptTokens: 0, completionTokens: 0, cost: 0, rawModel, provider: providerDisplayName, apiKeyMasked, keyName, apiKeyKey, lastUsed: dateKey };
}
stats.byApiKey[akKey].requests += ak.requests || 0;
stats.byApiKey[akKey].promptTokens += ak.promptTokens || 0;
@@ -627,16 +634,17 @@ export async function getUsageStats(period = "all") {
if (r.apiKey && typeof r.apiKey === "string") {
const keyInfo = apiKeyMap[r.apiKey];
const keyName = keyInfo?.name || r.apiKey.slice(0, 8) + "...";
const akKey = `${r.apiKey}|${r.model}|${r.provider || "unknown"}`;
const apiKeyMasked = maskApiKey(r.apiKey);
const akKey = `${apiKeyMasked}|${r.model}|${r.provider || "unknown"}`;
if (!stats.byApiKey[akKey]) {
stats.byApiKey[akKey] = { requests: 0, promptTokens: 0, completionTokens: 0, cost: 0, rawModel: r.model, provider: providerDisplayName, apiKey: r.apiKey, keyName, apiKeyKey: r.apiKey, lastUsed: r.timestamp };
stats.byApiKey[akKey] = { requests: 0, promptTokens: 0, completionTokens: 0, cost: 0, rawModel: r.model, provider: providerDisplayName, apiKeyMasked, keyName, apiKeyKey: apiKeyMasked, lastUsed: r.timestamp };
}
const ake = stats.byApiKey[akKey];
ake.requests++; ake.promptTokens += promptTokens; ake.completionTokens += completionTokens; ake.cost += entryCost;
if (new Date(r.timestamp) > new Date(ake.lastUsed)) ake.lastUsed = r.timestamp;
} else {
if (!stats.byApiKey["local-no-key"]) {
stats.byApiKey["local-no-key"] = { requests: 0, promptTokens: 0, completionTokens: 0, cost: 0, rawModel: r.model, provider: providerDisplayName, apiKey: null, keyName: "Local (No API Key)", apiKeyKey: "local-no-key", lastUsed: r.timestamp };
stats.byApiKey["local-no-key"] = { requests: 0, promptTokens: 0, completionTokens: 0, cost: 0, rawModel: r.model, provider: providerDisplayName, apiKeyMasked: null, keyName: "Local (No API Key)", apiKeyKey: "local-no-key", lastUsed: r.timestamp };
}
const ake = stats.byApiKey["local-no-key"];
ake.requests++; ake.promptTokens += promptTokens; ake.completionTokens += completionTokens; ake.cost += entryCost;

View File

@@ -3,6 +3,20 @@ function normalizeString(value) {
return String(value).trim();
}
const ALLOWED_PROXY_SCHEMES = ["http:", "https:", "socks5:", "socks4:", "socks5h:", "socks4a:"];
function validateProxyUrl(url) {
if (!url) return null;
if (/[\n\r`$]/.test(url)) return null;
try {
const parsed = new URL(url);
if (!ALLOWED_PROXY_SCHEMES.includes(parsed.protocol)) return null;
return parsed.href;
} catch {
return null;
}
}
export function applyOutboundProxyEnv(
{ outboundProxyEnabled, outboundProxyUrl, outboundNoProxy } = {}
) {
@@ -46,11 +60,14 @@ export function applyOutboundProxyEnv(
}
if (proxyUrl) {
process.env.HTTP_PROXY = proxyUrl;
process.env.HTTPS_PROXY = proxyUrl;
process.env.ALL_PROXY = proxyUrl;
process.env.NINE_ROUTER_PROXY_URL = proxyUrl;
managed = true;
const validated = validateProxyUrl(proxyUrl);
if (validated) {
process.env.HTTP_PROXY = validated;
process.env.HTTPS_PROXY = validated;
process.env.ALL_PROXY = validated;
process.env.NINE_ROUTER_PROXY_URL = validated;
managed = true;
}
}
if (noProxy) {

View File

@@ -154,14 +154,24 @@ export function clearCodexSession(state) {
pendingExchanges.delete(state);
}
function escapeHtml(str) {
return String(str)
.replace(/&/g, "&amp;")
.replace(/</g, "&lt;")
.replace(/>/g, "&gt;")
.replace(/"/g, "&quot;")
.replace(/'/g, "&#39;");
}
function renderCodexResultPage(success, message) {
const color = success ? "#22c55e" : "#ef4444";
const icon = success ? "&#10003;" : "&#10007;";
const title = success ? "Authentication Successful" : "Authentication Failed";
const safeMessage = escapeHtml(message);
return `<!DOCTYPE html>
<html><head><meta charset="utf-8"><title>${title}</title>
<style>body{font-family:system-ui;display:flex;justify-content:center;align-items:center;height:100vh;margin:0;background:#f5f5f5}.c{text-align:center;padding:2rem;background:#fff;border-radius:8px;box-shadow:0 2px 10px rgba(0,0,0,.1)}.i{color:${color};font-size:3rem}h1{margin:1rem 0}p{color:#666}</style>
</head><body><div class="c"><div class="i">${icon}</div><h1>${title}</h1><p>${message}</p><p>Closing in <span id="cd">3</span>s...</p>
</head><body><div class="c"><div class="i">${icon}</div><h1>${title}</h1><p>${safeMessage}</p><p>Closing in <span id="cd">3</span>s...</p>
<script>let n=3;const c=document.getElementById("cd");const t=setInterval(()=>{n--;c.textContent=n;if(n<=0){clearInterval(t);window.close();}},1000);</script>
</div></body></html>`;
}

View File

@@ -41,6 +41,7 @@ async function resolveMitmRouterBaseUrl() {
const MITM_PORT = 443;
const MITM_WIN_NODE_PORT = 8443;
const PID_FILE = path.join(MITM_DIR, ".mitm.pid");
const LOCK_FILE = path.join(MITM_DIR, ".mitm.lock");
const MITM_MAX_RESTARTS = 5;
const MITM_RESTART_DELAYS_MS = [5000, 10000, 20000, 30000, 60000];
@@ -400,19 +401,22 @@ async function getMitmStatus() {
async function scheduleMitmRestart(apiKey) {
if (mitmIsRestarting) return;
// Set guard synchronously before any await to prevent concurrent calls
// from passing the check above.
mitmIsRestarting = true;
const aliveMs = Date.now() - mitmLastStartTime;
if (aliveMs >= MITM_RESTART_RESET_MS) mitmRestartCount = 0;
if (mitmRestartCount >= MITM_MAX_RESTARTS) {
err("Max restart attempts reached. Giving up.");
mitmIsRestarting = false;
return;
}
const attempt = mitmRestartCount;
const delay = MITM_RESTART_DELAYS_MS[Math.min(attempt, MITM_RESTART_DELAYS_MS.length - 1)];
mitmRestartCount++;
mitmIsRestarting = true;
log(`Restarting in ${delay / 1000}s... (${mitmRestartCount}/${MITM_MAX_RESTARTS})`);
await new Promise((r) => setTimeout(r, delay));
@@ -486,7 +490,19 @@ async function startServer(apiKey, sudoPassword, forceKillPort443 = false) {
throw new Error("MITM server is already running");
}
await killLeftoverMitm(sudoPassword);
// Atomically claim lock to prevent concurrent startServer across processes.
// O_EXCL (flag: "wx") fails with EEXIST if the file already exists.
try {
fs.writeFileSync(LOCK_FILE, String(process.pid), { flag: "wx" });
} catch (e) {
if (e.code === "EEXIST") {
throw new Error("MITM server is already starting (lock contention)");
}
throw e;
}
try {
await killLeftoverMitm(sudoPassword);
if (!IS_WIN) {
const portStatus = await checkPort443Free();
@@ -679,6 +695,7 @@ async function startServer(apiKey, sudoPassword, forceKillPort443 = false) {
serverProcess = null;
serverPid = null;
try { fs.unlinkSync(PID_FILE); } catch { /* ignore */ }
try { fs.unlinkSync(LOCK_FILE); } catch { /* ignore */ }
// Auto-restart on unexpected exit
if (code !== 0 && !mitmIsRestarting) scheduleMitmRestart(apiKey);
});
@@ -706,7 +723,15 @@ async function startServer(apiKey, sudoPassword, forceKillPort443 = false) {
await saveMitmSettings(true, sudoPassword);
if (sudoPassword) setCachedPassword(sudoPassword);
// Server is healthy — remove lock file (PID file persists as the marker)
try { fs.unlinkSync(LOCK_FILE); } catch { /* ignore */ }
return { running: true, pid: serverPid };
} catch (e) {
// Clean up lock on any failure
try { fs.unlinkSync(LOCK_FILE); } catch { /* ignore */ }
throw e;
}
}
/**
@@ -779,6 +804,7 @@ async function stopServer(sudoPassword) {
}
try { fs.unlinkSync(PID_FILE); } catch { /* ignore */ }
try { fs.unlinkSync(LOCK_FILE); } catch { /* ignore */ }
await saveMitmSettings(false, null);
mitmIsRestarting = false;

View File

@@ -0,0 +1,289 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import fs from "fs";
import path from "path";
// ============================================================
// AUDIT-002 (#1962): API key masking in usage stats
// ============================================================
describe("AUDIT-002: API key masking", () => {
it("source should contain maskApiKey function", () => {
const source = fs.readFileSync(
path.resolve("src/lib/db/repos/usageRepo.js"),
"utf-8"
);
expect(source).toContain("function maskApiKey");
});
it("getUsageHistory should use apiKeyMasked instead of apiKey", () => {
const source = fs.readFileSync(
path.resolve("src/lib/db/repos/usageRepo.js"),
"utf-8"
);
// The REST response should use apiKeyMasked
expect(source).toContain("apiKeyMasked: maskApiKey(r.apiKey)");
// The return mapping in getUsageHistory should not have raw apiKey
// (The internal ring buffer still uses apiKey: r.apiKey for internal state - that's fine)
const historyReturn = source.match(/return rows\.map\(\(r\)\s*=>\s*\(\{[\s\S]*?\}\)\);/);
expect(historyReturn).not.toBeNull();
expect(historyReturn[0]).toContain("apiKeyMasked");
expect(historyReturn[0]).not.toContain("apiKey: r.apiKey");
});
it("getUsageStats should use apiKeyMasked in byApiKey entries", () => {
const source = fs.readFileSync(
path.resolve("src/lib/db/repos/usageRepo.js"),
"utf-8"
);
// Both code paths (daily summary + 24h live) should use apiKeyMasked
const maskedCount = (source.match(/apiKeyMasked/g) || []).length;
expect(maskedCount).toBeGreaterThanOrEqual(4); // function def + 3 usage sites
// The byApiKey stats entries should use apiKeyMasked, not raw apiKey
// Check the daily summary path
const dailyPath = source.match(/stats\.byApiKey\[akKey\] = \{[^}]*apiKeyMasked[^}]*\}/);
expect(dailyPath).not.toBeNull();
// Check the 24h live path
const livePath = source.match(/stats\.byApiKey\[akKey\] = \{[^}]*apiKeyMasked[^}]*\}/g);
expect(livePath).not.toBeNull();
expect(livePath.length).toBeGreaterThanOrEqual(1);
});
it("byApiKey object keys should use masked key, not raw key", () => {
const source = fs.readFileSync(
path.resolve("src/lib/db/repos/usageRepo.js"),
"utf-8"
);
// The 24h path should use apiKeyMasked in the akKey template
expect(source).toContain("${apiKeyMasked}|${r.model}|${r.provider");
// Should NOT use raw r.apiKey in the key
expect(source).not.toContain("${r.apiKey}|${r.model}|${r.provider");
});
});
// ============================================================
// AUDIT-003 (#1961): Proxy URL validation
// ============================================================
describe("AUDIT-003: Proxy URL validation", () => {
beforeEach(() => {
delete process.env.HTTP_PROXY;
delete process.env.HTTPS_PROXY;
delete process.env.ALL_PROXY;
delete process.env.NINE_ROUTER_PROXY_MANAGED;
delete process.env.NINE_ROUTER_PROXY_URL;
delete process.env.NINE_ROUTER_NO_PROXY;
delete process.env.NO_PROXY;
});
it("source should contain validateProxyUrl function", () => {
const source = fs.readFileSync(
path.resolve("src/lib/network/outboundProxy.js"),
"utf-8"
);
expect(source).toContain("function validateProxyUrl");
expect(source).toContain("ALLOWED_PROXY_SCHEMES");
});
it("should accept valid http proxy URLs", async () => {
vi.resetModules();
const { applyOutboundProxyEnv } = await import("../../src/lib/network/outboundProxy.js");
applyOutboundProxyEnv({
outboundProxyEnabled: true,
outboundProxyUrl: "http://proxy.example.com:8080",
});
// new URL().href normalizes (adds trailing slash)
expect(process.env.HTTP_PROXY).toContain("http://proxy.example.com:8080");
expect(process.env.HTTPS_PROXY).toContain("http://proxy.example.com:8080");
});
it("should accept valid https proxy URLs", async () => {
vi.resetModules();
const { applyOutboundProxyEnv } = await import("../../src/lib/network/outboundProxy.js");
applyOutboundProxyEnv({
outboundProxyEnabled: true,
outboundProxyUrl: "https://proxy.example.com:443",
});
// new URL().href normalizes (drops default port 443, adds trailing slash)
expect(process.env.HTTP_PROXY).toContain("https://proxy.example.com");
});
it("should accept valid socks5 proxy URLs", async () => {
vi.resetModules();
const { applyOutboundProxyEnv } = await import("../../src/lib/network/outboundProxy.js");
applyOutboundProxyEnv({
outboundProxyEnabled: true,
outboundProxyUrl: "socks5://proxy.example.com:1080",
});
expect(process.env.ALL_PROXY).toBe("socks5://proxy.example.com:1080");
});
it("should reject URLs with shell metacharacters (newline)", async () => {
vi.resetModules();
const { applyOutboundProxyEnv } = await import("../../src/lib/network/outboundProxy.js");
applyOutboundProxyEnv({
outboundProxyEnabled: true,
outboundProxyUrl: "http://proxy.example.com:8080\nmalicious",
});
expect(process.env.HTTP_PROXY).toBeUndefined();
});
it("should reject URLs with shell metacharacters (backtick)", async () => {
vi.resetModules();
const { applyOutboundProxyEnv } = await import("../../src/lib/network/outboundProxy.js");
applyOutboundProxyEnv({
outboundProxyEnabled: true,
outboundProxyUrl: "http://`whoami`.example.com:8080",
});
expect(process.env.HTTP_PROXY).toBeUndefined();
});
it("should reject URLs with shell metacharacters (dollar)", async () => {
vi.resetModules();
const { applyOutboundProxyEnv } = await import("../../src/lib/network/outboundProxy.js");
applyOutboundProxyEnv({
outboundProxyEnabled: true,
outboundProxyUrl: "http://$(whoami).example.com:8080",
});
expect(process.env.HTTP_PROXY).toBeUndefined();
});
it("should reject non-allowed schemes (file://)", async () => {
vi.resetModules();
const { applyOutboundProxyEnv } = await import("../../src/lib/network/outboundProxy.js");
applyOutboundProxyEnv({
outboundProxyEnabled: true,
outboundProxyUrl: "file:///etc/passwd",
});
expect(process.env.HTTP_PROXY).toBeUndefined();
});
it("should reject non-allowed schemes (javascript:)", async () => {
vi.resetModules();
const { applyOutboundProxyEnv } = await import("../../src/lib/network/outboundProxy.js");
applyOutboundProxyEnv({
outboundProxyEnabled: true,
outboundProxyUrl: "javascript:alert(1)",
});
expect(process.env.HTTP_PROXY).toBeUndefined();
});
});
// ============================================================
// AUDIT-018 (#1972): XSS escaping in OAuth callback
// ============================================================
describe("AUDIT-018: XSS escaping in OAuth callback", () => {
it("source should contain escapeHtml function", () => {
const source = fs.readFileSync(
path.resolve("src/lib/oauth/utils/server.js"),
"utf-8"
);
expect(source).toContain("function escapeHtml");
});
it("should escape ampersand, angle brackets, and quotes", () => {
const source = fs.readFileSync(
path.resolve("src/lib/oauth/utils/server.js"),
"utf-8"
);
expect(source).toContain("&amp;");
expect(source).toContain("&lt;");
expect(source).toContain("&gt;");
expect(source).toContain("&quot;");
expect(source).toContain("&#39;");
});
it("should use safeMessage in rendered HTML, not raw message", () => {
const source = fs.readFileSync(
path.resolve("src/lib/oauth/utils/server.js"),
"utf-8"
);
expect(source).toContain("safeMessage");
expect(source).toContain("${safeMessage}");
// Should NOT use raw message in HTML body
expect(source).not.toContain("<p>${message}</p>");
});
});
// ============================================================
// AUDIT-004 (#1963): TOCTOU race - atomic lock file
// ============================================================
describe("AUDIT-004: Atomic lock file for MITM startup", () => {
it("manager.js should define LOCK_FILE constant", () => {
const source = fs.readFileSync(
path.resolve("src/mitm/manager.js"),
"utf-8"
);
expect(source).toContain("LOCK_FILE");
expect(source).toContain(".mitm.lock");
});
it("should use O_EXCL flag (wx) for atomic creation", () => {
const source = fs.readFileSync(
path.resolve("src/mitm/manager.js"),
"utf-8"
);
expect(source).toContain('"wx"');
expect(source).toContain("EEXIST");
});
it("should clean up lock file on all exit paths", () => {
const source = fs.readFileSync(
path.resolve("src/mitm/manager.js"),
"utf-8"
);
const matches = source.match(/unlinkSync\(LOCK_FILE\)/g);
expect(matches).not.toBeNull();
expect(matches.length).toBeGreaterThanOrEqual(4);
});
});
// ============================================================
// AUDIT-001 (#1965): Race condition in retry tracking
// ============================================================
describe("AUDIT-001: Synchronous restart guard", () => {
it("mitmIsRestarting should be set before first await expression", () => {
const source = fs.readFileSync(
path.resolve("src/mitm/manager.js"),
"utf-8"
);
const funcStart = source.indexOf("async function scheduleMitmRestart");
expect(funcStart).toBeGreaterThan(-1);
const funcBody = source.substring(funcStart, funcStart + 2000);
const guardCheckIdx = funcBody.indexOf("if (mitmIsRestarting) return;");
expect(guardCheckIdx).toBeGreaterThan(-1);
const afterGuard = funcBody.substring(guardCheckIdx);
// Strip line comments to avoid matching "await" in comment text
const noComments = afterGuard.replace(/\/\/.*$/gm, "");
// Find the first actual await expression
const firstAwaitIdx = noComments.search(/\bawait\s+/);
expect(firstAwaitIdx).toBeGreaterThan(-1);
// Find mitmIsRestarting = true
const setFlagIdx = noComments.indexOf("mitmIsRestarting = true");
expect(setFlagIdx).toBeGreaterThan(-1);
expect(firstAwaitIdx).toBeGreaterThan(-1);
expect(setFlagIdx).toBeLessThan(firstAwaitIdx);
});
it("mitmIsRestarting should be reset on max-restarts early return", () => {
const source = fs.readFileSync(
path.resolve("src/mitm/manager.js"),
"utf-8"
);
const funcStart = source.indexOf("async function scheduleMitmRestart");
const funcBody = source.substring(funcStart, funcStart + 2000);
const maxRestartsIdx = funcBody.indexOf("Max restart attempts reached");
expect(maxRestartsIdx).toBeGreaterThan(-1);
const afterMax = funcBody.substring(maxRestartsIdx, maxRestartsIdx + 200);
expect(afterMax).toContain("mitmIsRestarting = false");
});
});