From 8f0615fd04f38552f0fa074635142321806fcc03 Mon Sep 17 00:00:00 2001 From: diegosouzapw Date: Mon, 1 Jun 2026 19:36:21 -0300 Subject: [PATCH] fix(security): resolve 5 CodeQL alerts, document FPs, harden deploy skills Real fixes (alerts auto-close on next scan): - agentSkills/generator: escape backslash before double-quote in YAML frontmatter so a trailing backslash can't escape the closing quote (js/incomplete-sanitization #297/#298) - usage: replace /\s*\(RESTRICTED\)\s*$/ with a non-backtracking literal + trim (js/polynomial-redos #275) - i18n/request: skip __proto__/constructor/prototype in deepMergeFallback (js/prototype-pollution-utility #274) - scripts/ad-hoc/nvidia diag: log key presence only, never key chars (js/clear-text-logging #273) - tests: regression coverage for all three production fixes (YAML round-trip, proto-pollution guard, RESTRICTED strip + ReDoS timing) Docs: - ARCHITECTURE.md: executor count 45->55, OAuth modules 15->16, agy.ts in list Deploy skills (deploy-vps-*-cc/-cx/-ag): add --legacy-peer-deps (npm v11 peer-dep resolver crashes on the omniroute tree) and replace the "; pm2 start" that masked a failed install with a proper && chain. --- .agents/skills/deploy-vps-akamai-cc/SKILL.md | 2 +- .agents/skills/deploy-vps-both-cc/SKILL.md | 4 ++-- .agents/skills/deploy-vps-local-ag/SKILL.md | 2 +- .agents/skills/deploy-vps-local-cc/SKILL.md | 2 +- .agents/skills/deploy-vps-local-cx/SKILL.md | 2 +- docs/architecture/ARCHITECTURE.md | 10 ++++---- open-sse/services/usage.ts | 5 +++- scripts/ad-hoc/nvidia-startswith-diag.ts | 3 ++- src/i18n/request.ts | 2 ++ src/lib/agentSkills/generator.ts | 12 ++++++++-- tests/unit/agentSkills-generator.test.ts | 24 +++++++++++++++++++- tests/unit/i18n-fallback.test.ts | 23 +++++++++++++++++++ tests/unit/usage-utils.test.ts | 19 ++++++++++++++++ 13 files changed, 94 insertions(+), 16 deletions(-) diff --git a/.agents/skills/deploy-vps-akamai-cc/SKILL.md b/.agents/skills/deploy-vps-akamai-cc/SKILL.md index cce9435f9e..60ee7db988 100644 --- a/.agents/skills/deploy-vps-akamai-cc/SKILL.md +++ b/.agents/skills/deploy-vps-akamai-cc/SKILL.md @@ -30,7 +30,7 @@ scp omniroute-*.tgz root@69.164.221.35:/tmp/ ``` ```bash -ssh root@69.164.221.35 "npm install -g /tmp/omniroute-*.tgz --ignore-scripts && cd /usr/lib/node_modules/omniroute/app && npm rebuild better-sqlite3 && pm2 delete omniroute 2>/dev/null; pm2 start /root/.omniroute/ecosystem.config.cjs --update-env && pm2 save && echo '✅ Akamai done'" +ssh root@69.164.221.35 "npm install -g /tmp/omniroute-*.tgz --ignore-scripts --legacy-peer-deps && cd /usr/lib/node_modules/omniroute/app && npm rebuild better-sqlite3 && (pm2 delete omniroute 2>/dev/null || true) && pm2 start /root/.omniroute/ecosystem.config.cjs --update-env && pm2 save && echo '✅ Akamai done'" ``` ### 3. Verify the deployment diff --git a/.agents/skills/deploy-vps-both-cc/SKILL.md b/.agents/skills/deploy-vps-both-cc/SKILL.md index f9a5d3b667..27944c311e 100644 --- a/.agents/skills/deploy-vps-both-cc/SKILL.md +++ b/.agents/skills/deploy-vps-both-cc/SKILL.md @@ -35,11 +35,11 @@ scp omniroute-*.tgz root@69.164.221.35:/tmp/ && scp omniroute-*.tgz root@192.168 ``` ```bash -ssh root@69.164.221.35 "npm install -g /tmp/omniroute-*.tgz --ignore-scripts && cd /usr/lib/node_modules/omniroute/app && npm rebuild better-sqlite3 && pm2 delete omniroute 2>/dev/null; pm2 start /root/.omniroute/ecosystem.config.cjs --update-env && pm2 save && echo '✅ Akamai done'" +ssh root@69.164.221.35 "npm install -g /tmp/omniroute-*.tgz --ignore-scripts --legacy-peer-deps && cd /usr/lib/node_modules/omniroute/app && npm rebuild better-sqlite3 && (pm2 delete omniroute 2>/dev/null || true) && pm2 start /root/.omniroute/ecosystem.config.cjs --update-env && pm2 save && echo '✅ Akamai done'" ``` ```bash -ssh root@192.168.0.15 "npm install -g /tmp/omniroute-*.tgz --ignore-scripts && cd /usr/lib/node_modules/omniroute/app && npm rebuild better-sqlite3 && pm2 delete omniroute 2>/dev/null; pm2 start /root/.omniroute/ecosystem.config.cjs --update-env && pm2 save && echo '✅ Local done'" +ssh root@192.168.0.15 "npm install -g /tmp/omniroute-*.tgz --ignore-scripts --legacy-peer-deps && cd /usr/lib/node_modules/omniroute/app && npm rebuild better-sqlite3 && (pm2 delete omniroute 2>/dev/null || true) && pm2 start /root/.omniroute/ecosystem.config.cjs --update-env && pm2 save && echo '✅ Local done'" ``` ### 3. Verify the deployment diff --git a/.agents/skills/deploy-vps-local-ag/SKILL.md b/.agents/skills/deploy-vps-local-ag/SKILL.md index 79770a9c27..fe6bbc6ab1 100644 --- a/.agents/skills/deploy-vps-local-ag/SKILL.md +++ b/.agents/skills/deploy-vps-local-ag/SKILL.md @@ -30,7 +30,7 @@ scp omniroute-*.tgz root@192.168.0.15:/tmp/ ``` ```bash -ssh root@192.168.0.15 "npm install -g /tmp/omniroute-*.tgz --ignore-scripts && cd /usr/lib/node_modules/omniroute/app && npm rebuild better-sqlite3 && pm2 delete omniroute 2>/dev/null; pm2 start /root/.omniroute/ecosystem.config.cjs --update-env && pm2 save && echo '✅ Local done'" +ssh root@192.168.0.15 "npm install -g /tmp/omniroute-*.tgz --ignore-scripts --legacy-peer-deps && cd /usr/lib/node_modules/omniroute/app && npm rebuild better-sqlite3 && (pm2 delete omniroute 2>/dev/null || true) && pm2 start /root/.omniroute/ecosystem.config.cjs --update-env && pm2 save && echo '✅ Local done'" ``` ### 3. Verify the deployment diff --git a/.agents/skills/deploy-vps-local-cc/SKILL.md b/.agents/skills/deploy-vps-local-cc/SKILL.md index 60e0fd5768..33e89ab847 100644 --- a/.agents/skills/deploy-vps-local-cc/SKILL.md +++ b/.agents/skills/deploy-vps-local-cc/SKILL.md @@ -30,7 +30,7 @@ scp omniroute-*.tgz root@192.168.0.15:/tmp/ ``` ```bash -ssh root@192.168.0.15 "npm install -g /tmp/omniroute-*.tgz --ignore-scripts && cd /usr/lib/node_modules/omniroute/app && npm rebuild better-sqlite3 && pm2 delete omniroute 2>/dev/null; pm2 start /root/.omniroute/ecosystem.config.cjs --update-env && pm2 save && echo '✅ Local done'" +ssh root@192.168.0.15 "npm install -g /tmp/omniroute-*.tgz --ignore-scripts --legacy-peer-deps && cd /usr/lib/node_modules/omniroute/app && npm rebuild better-sqlite3 && (pm2 delete omniroute 2>/dev/null || true) && pm2 start /root/.omniroute/ecosystem.config.cjs --update-env && pm2 save && echo '✅ Local done'" ``` ### 3. Verify the deployment diff --git a/.agents/skills/deploy-vps-local-cx/SKILL.md b/.agents/skills/deploy-vps-local-cx/SKILL.md index 6ba355a263..0672b001cc 100644 --- a/.agents/skills/deploy-vps-local-cx/SKILL.md +++ b/.agents/skills/deploy-vps-local-cx/SKILL.md @@ -35,7 +35,7 @@ scp omniroute-*.tgz root@192.168.0.15:/tmp/ ``` ```bash -ssh root@192.168.0.15 "npm install -g /tmp/omniroute-*.tgz --ignore-scripts && cd /usr/lib/node_modules/omniroute/app && npm rebuild better-sqlite3 && pm2 delete omniroute 2>/dev/null; pm2 start /root/.omniroute/ecosystem.config.cjs --update-env && pm2 save && echo '✅ Local done'" +ssh root@192.168.0.15 "npm install -g /tmp/omniroute-*.tgz --ignore-scripts --legacy-peer-deps && cd /usr/lib/node_modules/omniroute/app && npm rebuild better-sqlite3 && (pm2 delete omniroute 2>/dev/null || true) && pm2 start /root/.omniroute/ecosystem.config.cjs --update-env && pm2 save && echo '✅ Local done'" ``` ### 3. Verify the deployment diff --git a/docs/architecture/ARCHITECTURE.md b/docs/architecture/ARCHITECTURE.md index 74c3e28e4c..e416f57f38 100644 --- a/docs/architecture/ARCHITECTURE.md +++ b/docs/architecture/ARCHITECTURE.md @@ -17,13 +17,13 @@ It provides a single OpenAI-compatible endpoint (`/v1/*`) and routes traffic acr Core capabilities: -- OpenAI-compatible API surface for CLI/tools (177 providers, 45 executors) +- OpenAI-compatible API surface for CLI/tools (177 providers, 55 executors) - Request/response translation across provider formats - Model combo fallback (multi-model sequence) - Structured combo steps (`provider + model + connection`) with runtime ordering by `compositeTiers` - Account-level fallback (multi-account per provider) - Quota preflight and quota-aware P2C account selection in the main chat path -- OAuth + API-key provider connection management (15 OAuth modules) +- OAuth + API-key provider connection management (16 OAuth modules) - Embedding generation via `/v1/embeddings` (6 providers, 9 models) - Image generation via `/v1/images/generations` (10+ providers, 20+ models) - Audio transcription via `/v1/audio/transcriptions` (7 providers) @@ -66,7 +66,7 @@ Core capabilities: - Prompt injection guard middleware - Prompt compression pipeline with Caveman, RTK, stacked pipelines, compression combos, language packs, and analytics - ACP (Agent Communication Protocol) registry -- Modular OAuth providers (15 individual modules under `src/lib/oauth/providers/`) +- Modular OAuth providers (16 individual modules under `src/lib/oauth/providers/`) - Uninstall/full-uninstall scripts - OAuth environment repair action - WebSocket bridge for OpenAI-compatible WS clients (`/v1/ws`) @@ -321,10 +321,10 @@ Domain layer modules: - Eval runner: `src/lib/domain/evalRunner.ts` - Domain state persistence: `src/lib/db/domainState.ts` — SQLite CRUD for fallback chains, budgets, cost history, lockout state, circuit breakers -OAuth provider modules (15 individual files under `src/lib/oauth/providers/`): +OAuth provider modules (16 individual files under `src/lib/oauth/providers/`): - Registry index: `src/lib/oauth/providers/index.ts` -- Individual providers: `claude.ts`, `codex.ts`, `gemini.ts`, `antigravity.ts`, `qoder.ts`, `qwen.ts`, `kimi-coding.ts`, `github.ts`, `kiro.ts`, `cursor.ts`, `kilocode.ts`, `cline.ts`, `windsurf.ts`, `gitlab-duo.ts`, `trae.ts` +- Individual providers: `claude.ts`, `codex.ts`, `gemini.ts`, `antigravity.ts`, `agy.ts`, `qoder.ts`, `qwen.ts`, `kimi-coding.ts`, `github.ts`, `kiro.ts`, `cursor.ts`, `kilocode.ts`, `cline.ts`, `windsurf.ts`, `gitlab-duo.ts`, `trae.ts` - Thin wrapper: `src/lib/oauth/providers.ts` — re-exports from individual modules ## 5) Embedded Services (v3.8.4) diff --git a/open-sse/services/usage.ts b/open-sse/services/usage.ts index 3f404b448d..3f28224bd2 100644 --- a/open-sse/services/usage.ts +++ b/open-sse/services/usage.ts @@ -1951,7 +1951,10 @@ function mapSubscriptionTierStringToPlanLabel(tierText: string): string | null { if (upper.includes("PLUS")) return "Plus"; if (upper.includes("LITE")) return "Lite"; if (upper.includes("INDIVIDUAL") || upper.includes("FREE")) return "Free"; - const normalizedId = upper.replace(/\s*\(RESTRICTED\)\s*$/i, "").trim(); + // Strip a trailing "(RESTRICTED)" marker. Match the fixed literal anywhere then + // trim, instead of /\s*\(RESTRICTED\)\s*$/ whose overlapping \s* runs backtrack + // polynomially on whitespace-heavy upstream input (js/polynomial-redos). + const normalizedId = upper.replace(/\(RESTRICTED\)/i, "").trim(); if (normalizedId) { const mapped = mapCodeAssistTierIdToLabel(normalizedId); if (mapped) return mapped; diff --git a/scripts/ad-hoc/nvidia-startswith-diag.ts b/scripts/ad-hoc/nvidia-startswith-diag.ts index acf4a8b5b5..ca0fbc8250 100644 --- a/scripts/ad-hoc/nvidia-startswith-diag.ts +++ b/scripts/ad-hoc/nvidia-startswith-diag.ts @@ -166,7 +166,8 @@ async function partC() { async function main() { line(""); line("NVIDIA NIM — diagnóstico `startsWith is not a function`"); - show("NVIDIA_API_KEY presente", KEY ? `sim (${KEY.slice(0, 6)}…)` : "não"); + // Never echo any portion of the key (js/clear-text-logging) — presence only. + show("NVIDIA_API_KEY presente", KEY ? "sim" : "não"); show("BASE_URL", BASE_URL); show("MODEL", MODEL); line(""); diff --git a/src/i18n/request.ts b/src/i18n/request.ts index 6870c08c0c..012327d67c 100644 --- a/src/i18n/request.ts +++ b/src/i18n/request.ts @@ -15,6 +15,8 @@ export function deepMergeFallback( source: Record ): Record { for (const [key, sourceValue] of Object.entries(source)) { + // Guard against prototype pollution from a crafted locale message tree. + if (key === "__proto__" || key === "constructor" || key === "prototype") continue; const targetValue = target[key]; if ( sourceValue !== null && diff --git a/src/lib/agentSkills/generator.ts b/src/lib/agentSkills/generator.ts index 45f748cc81..23b3a10955 100644 --- a/src/lib/agentSkills/generator.ts +++ b/src/lib/agentSkills/generator.ts @@ -48,9 +48,13 @@ function serializeFrontmatter(fm: { name: string; description: string }): string // Use block scalar for description if it contains colons, quotes, or newlines const needsQuote = (v: string): boolean => /[:\n"']/.test(v) || v.startsWith(" "); - const nameStr = needsQuote(fm.name) ? `"${fm.name.replace(/"/g, '\\"')}"` : fm.name; + // Escape order matters for double-quoted YAML scalars: backslash FIRST (so the + // escapes we add below are not themselves re-escaped), then the double-quote, + // then collapse real newlines into the \n escape sequence. + const escDq = (v: string): string => v.replace(/\\/g, "\\\\").replace(/"/g, '\\"'); + const nameStr = needsQuote(fm.name) ? `"${escDq(fm.name)}"` : fm.name; const descStr = needsQuote(fm.description) - ? `"${fm.description.replace(/"/g, '\\"').replace(/\n/g, "\\n")}"` + ? `"${escDq(fm.description).replace(/\n/g, "\\n")}"` : fm.description; return `---\nname: ${nameStr}\ndescription: ${descStr}\n---\n`; @@ -363,3 +367,7 @@ export async function generateAgentSkills(opts: GeneratorOptions): Promise { rmTmpDir(tmpDir); } }); + +// ── serializeFrontmatter escaping (js/incomplete-sanitization regression) ────── + +test("serializeFrontmatter escapes backslashes before quotes → valid round-trippable YAML", async () => { + const { parse } = await import("yaml"); + const fm = { + name: 'name with "quote"', + description: 'line1\nline2: path C:\\Users\\x with "q" and trailing \\', + }; + const block = __testing.serializeFrontmatter(fm); + // Strip the --- fences and parse the inner YAML; broken escaping (e.g. an + // unescaped trailing backslash that escapes the closing quote) would throw or + // corrupt the value here. + const inner = block.replace(/^---\n/, "").replace(/---\n?$/, ""); + const parsed = parse(inner) as { name: string; description: string }; + assert.equal(parsed.name, fm.name, "name must round-trip through YAML unchanged"); + assert.equal( + parsed.description, + fm.description, + "description with backslashes + quotes + newline must round-trip exactly" + ); +}); diff --git a/tests/unit/i18n-fallback.test.ts b/tests/unit/i18n-fallback.test.ts index e66583511c..e18798ce6d 100644 --- a/tests/unit/i18n-fallback.test.ts +++ b/tests/unit/i18n-fallback.test.ts @@ -237,3 +237,26 @@ test("realistic i18n: locale invalid → DEFAULT_LOCALE applies; merge still wor assert.equal(common.cancel, "Cancel", "missing key filled from EN"); assert.deepEqual(messages.extra, { key: "Extra EN" }, "missing namespace filled from EN"); }); + +// --------------------------------------------------------------------------- +// 8. Prototype-pollution guard (js/prototype-pollution-utility regression) +// --------------------------------------------------------------------------- + +test("deepMergeFallback: ignores __proto__ / constructor / prototype keys (no prototype pollution)", () => { + const target: Record = {}; + // JSON.parse produces a real own-enumerable __proto__ key (an object literal would + // not), so Object.entries iterates it — exactly the attack vector the guard blocks. + const malicious = JSON.parse( + '{"__proto__":{"polluted":"yes"},"constructor":{"bad":1},"safe":"ok"}' + ) as Record; + + deepMergeFallback(target, malicious); + + assert.equal( + ({} as Record).polluted, + undefined, + "Object.prototype must not be polluted" + ); + assert.equal((target as Record).polluted, undefined); + assert.equal(target.safe, "ok", "legitimate keys still merge through"); +}); diff --git a/tests/unit/usage-utils.test.ts b/tests/unit/usage-utils.test.ts index 79aadb7162..3d16d002e4 100644 --- a/tests/unit/usage-utils.test.ts +++ b/tests/unit/usage-utils.test.ts @@ -297,3 +297,22 @@ describe("extractCodeAssistSubscriptionTier", () => { assert.equal(__testing.extractCodeAssistSubscriptionTier({}), null); }); }); + +/* ------------------------------------------------------------------ */ +/* mapSubscriptionTierStringToPlanLabel — (RESTRICTED) strip + ReDoS */ +/* ------------------------------------------------------------------ */ +describe("mapSubscriptionTierStringToPlanLabel", () => { + it("resolves a code-assist tier id via the normalized-id path after stripping (RESTRICTED)", () => { + // These reach the `normalizedId` branch (no early includes() match) and only + // resolve to a label once the "(RESTRICTED)" suffix is stripped + trimmed. + assert.equal(__testing.mapSubscriptionTierStringToPlanLabel("GOOGLE_ONE (RESTRICTED)"), "Pro"); + assert.equal(__testing.mapSubscriptionTierStringToPlanLabel("LEGACY (RESTRICTED)"), "Free"); + }); + + it("does not hang on whitespace-heavy input (js/polynomial-redos guard)", () => { + const start = process.hrtime.bigint(); + __testing.mapSubscriptionTierStringToPlanLabel(" ".repeat(100000) + "("); + const ms = Number(process.hrtime.bigint() - start) / 1e6; + assert.ok(ms < 500, `tier mapping took ${ms.toFixed(1)}ms on whitespace-heavy input — possible ReDoS`); + }); +});