diff --git a/.github/workflows/kcode-audit.yml b/.github/workflows/kcode-audit.yml index 89b9590..4d20eed 100644 --- a/.github/workflows/kcode-audit.yml +++ b/.github/workflows/kcode-audit.yml @@ -28,14 +28,12 @@ jobs: with: bun-version: latest - - name: Install KCode - run: | - curl -fsSL https://github.com/AstrolexisAI/KCode/releases/latest/download/kcode-linux-x64 -o kcode - chmod +x kcode + - name: Install dependencies + run: bun install --frozen-lockfile - name: Run Audit (static-only) run: | - ./kcode audit ${{ github.event.inputs.path || '.' }} \ + bun run src/index.ts audit ${{ github.event.inputs.path || '.' }} \ --skip-verify \ --json \ --max-files ${{ github.event.inputs.max_files || '500' }} diff --git a/AUDIT_REPORT.json b/AUDIT_REPORT.json index 18cc3fc..8641ba2 100644 --- a/AUDIT_REPORT.json +++ b/AUDIT_REPORT.json @@ -1,131 +1,52 @@ { "project": "/home/curly/KCode", - "timestamp": "2026-04-06", + "timestamp": "2026-04-10", "languages_detected": [ "typescript", "swift", "python", + "lua", "javascript", "kotlin", "ruby" ], "files_scanned": 500, - "candidates_found": 72, - "confirmed_findings": 7, - "false_positives": 65, + "candidates_found": 110, + "confirmed_findings": 2, + "false_positives": 86, "findings": [ { - "pattern_id": "react-001-dangerously-set", - "pattern_title": "dangerouslySetInnerHTML with dynamic content", - "severity": "high", - "file": "/home/curly/KCode/benchmarks/certification/tasks.ts", - "line": 930, - "matched_text": "dangerouslySetInnerHTML={{ __html: ", - "context": "928: \\`\\`\\`tsx\n929: function UserComment({ comment }: { comment: string }) {\n930: return
;\n931: }\n932: \\`\\`\\`\n933: ", - "verification": { - "verdict": "confirmed", - "reasoning": "The `__html` property in `dangerouslySetInnerHTML` is assigned directly from the `comment` prop, which originates from user input stored in the database—making it dynamic and vulnerable to XSS.", - "execution_path": "`UserComment` component receives `comment` (user-controlled string) → passed as `__html` value to `dangerouslySetInnerHTML` → React injects raw HTML into DOM without escaping → malicious script tags or event handlers in `comment` execute in context of the app.", - "suggested_fix": "Wrap `comment` with a sanitizer like DOMPurify before assignment: `
` or at minimum escape HTML entities if DOMPurify is unavailable." - }, - "cwe": "CWE-79" - }, - { - "pattern_id": "js-002-innerhtml", - "pattern_title": "innerHTML/outerHTML with dynamic content (XSS)", - "severity": "high", - "file": "/home/curly/KCode/ide/vscode/src/sidebar.ts", - "line": 322, - "matched_text": ".innerHTML = ", - "context": "320: div.className = 'message ' + role;\n321: if (id) div.dataset.id = id;\n322: div.innerHTML = '
' + (role === 'user' ? 'You' : 'KCode') + '
' +\n323: '
' + escapeHtml(content) + '
';\n324: messagesEl.appendChild(div);\n325: messagesEl.scrollTop = messagesEl.scrollHeight;", + "pattern_id": "js-014-json-parse-no-catch", + "pattern_title": "JSON.parse without try/catch (crash on invalid input)", + "severity": "medium", + "file": "/home/curly/KCode/src/tools/notebook-utils.ts", + "line": 36, + "matched_text": "JSON.parse(content", + "context": "34: /** Parse .ipynb JSON content into a typed notebook structure */\n35: export function parseNotebook(content: string): JupyterNotebook {\n36: const nb = JSON.parse(content);\n37: if (nb.nbformat !== 4) {\n38: throw new Error(`Only nbformat 4 is supported, found: ${nb.nbformat}`);\n39: }", "verification": { "verdict": "confirmed", - "reasoning": "The `content` parameter passed to `addMessage()` is escaped via `escapeHtml()` before being injected into `innerHTML`, but the function itself is called with dynamic content (e.g., from LLM responses or user input), and `escapeHtml()` only protects against basic HTML injection—not all XSS vectors like event handlers or script tags if the escaping is incomplete or bypassed. (+1 more matches of this pattern in the same file)", - "execution_path": "`addMessage(role, content, id)` → `content` (dynamic, e.g., from API/LLM/user) → `escapeHtml(content)` → embedded in `div.innerHTML` string → rendered as HTML", - "suggested_fix": "Replace `div.innerHTML = ...` with `div.innerHTML = '
...
' + escapeHtml(content) + '
';` → `div.innerHTML = new DOMParser().parseFromString('
...
' + escapeHtml(content) + '
', 'text/html').documentElement.innerHTML;` OR (more simply) use `div.insertAdjacentHTML('beforeend', ...)` with sanitized input, or replace `innerHTML` with `textContent` if full HTML isn’t needed." + "reasoning": "The `JSON.parse(content)` call is not wrapped in a try/catch block, meaning any malformed JSON input will throw an unhandled SyntaxError.", + "execution_path": "parseNotebook(invalidJsonString) -> JSON.parse(invalidJsonString) -> throws SyntaxError", + "suggested_fix": "Wrap the JSON.parse call in a try/catch block and throw a more descriptive error or return a result type." }, - "cwe": "CWE-79" + "cwe": "CWE-754" }, { - "pattern_id": "js-002-innerhtml", - "pattern_title": "innerHTML/outerHTML with dynamic content (XSS)", + "pattern_id": "js-008-prototype-pollution-bracket", + "pattern_title": "Prototype pollution via bracket notation with user key", "severity": "high", - "file": "/home/curly/KCode/vscode-extension/src/chat-panel.ts", - "line": 507, - "matched_text": ".innerHTML = ", - "context": "505: div.className = 'message message-' + role;\n506: if (role === 'assistant') {\n507: div.innerHTML = formatMarkdown(content);\n508: } else {\n509: div.textContent = content;\n510: }", - "verification": { - "verdict": "confirmed", - "reasoning": "The `content` parameter passed to `addMessage()` is used directly in `formatMarkdown(content)` and assigned to `div.innerHTML`, and since `content` originates from external sources (e.g., user messages, LLM responses), it likely contains untrusted HTML that can execute scripts. (+3 more matches of this pattern in the same file)", - "execution_path": "`addMessage(role, content)` → `formatMarkdown(content)` returns HTML string → `div.innerHTML = ...` → appended to `messagesEl`", - "suggested_fix": "Replace `div.innerHTML = formatMarkdown(content);` with `div.innerHTML = sanitizeHtml(formatMarkdown(content));` using a trusted sanitizer (e.g., DOMPurify) or use `textContent` if Markdown rendering is not strictly needed for untrusted input." - }, - "cwe": "CWE-79" - }, - { - "pattern_id": "js-002-innerhtml", - "pattern_title": "innerHTML/outerHTML with dynamic content (XSS)", - "severity": "high", - "file": "/home/curly/KCode/src/web/static/app.js", - "line": 357, - "matched_text": ".innerHTML = ", - "context": "355: var rendered = window.MarkdownRenderer.renderMarkdown(msg.content);\n356: if (window.DOMPurify) {\n357: body.innerHTML = window.DOMPurify.sanitize(rendered);\n358: } else {\n359: body.innerHTML = rendered;\n360: }", - "verification": { - "verdict": "confirmed", - "reasoning": "The `rendered` value assigned to `body.innerHTML` originates from `window.MarkdownRenderer.renderMarkdown(msg.content)`, and `msg.content` is user-provided (as seen from `msg.id`, `msg.role`, etc.), making it susceptible to XSS unless sanitized—here, DOMPurify is used conditionally, but fallback to unsanitized HTML exists. (+2 more matches of this pattern in the same file)", - "execution_path": "`renderMarkdown(msg.content)` → `rendered` (HTML string with potential XSS payloads) → `body.innerHTML = rendered` (or `DOMPurify.sanitize(rendered)`) → DOM injection → script execution on render", - "suggested_fix": "Ensure `window.DOMPurify` is always available or polyfilled before use; if not, add a fallback sanitizer (e.g., `DOMPurify.sanitize || ((html) => html)`) or use `textContent` for high-risk contexts." - }, - "cwe": "CWE-79" - }, - { - "pattern_id": "js-006-hardcoded-secret", - "pattern_title": "Hardcoded secret/key in JavaScript/TypeScript", - "severity": "high", - "file": "/home/curly/KCode/src/remote/triggers/trigger-api.test.ts", - "line": 9, - "matched_text": "TOKEN = \"test-token-abc123\"", - "context": "7: \n8: const BASE_URL = \"https://cloud.kulvex.ai/api/v1\";\n9: const AUTH_TOKEN = \"test-token-abc123\";\n10: \n11: const sampleTrigger: RemoteTrigger = {\n12: id: \"trg_001\",", - "verification": { - "verdict": "confirmed", - "reasoning": "The `AUTH_TOKEN` is a hardcoded string `\"test-token-abc123\"` that follows a realistic pattern (alphanumeric with hyphen) and is used directly in authentication, suggesting it is not merely a placeholder but a representative value likely copied from actual usage or configuration.", - "execution_path": "Test file imports `TriggerApiClient`, which presumably uses `AUTH_TOKEN` (defined at module scope) when making authenticated requests to `BASE_URL`; this token is exposed at module load time and visible in any test run or bundle.", - "suggested_fix": "Replace `AUTH_TOKEN` with an environment variable, e.g., `process.env.TRIGGER_API_TOKEN ?? \"test-token-abc123\"`, and ensure it is set in CI/test environments." - }, - "cwe": "CWE-798" - }, - { - "pattern_id": "js-007-command-injection", - "pattern_title": "Shell command with template literal (injection)", - "severity": "critical", - "file": "/home/curly/KCode/src/ui/actions/text-actions.ts", - "line": 157, - "matched_text": "execSync(`", - "context": "155: try {\n156: const bin = cmd.split(\" \")[0]!;\n157: execSync(`which ${bin} 2>/dev/null`, { timeout: 2000 });\n158: const output = execSync(`${cmd} '${text.replace(/'/g, \"'\\\\''\")}' 2>/dev/null`, {\n159: timeout: 5000,\n160: }).toString();", - "verification": { - "verdict": "confirmed", - "reasoning": "The template literal on line 157 (`which ${bin}`) and especially line 158 (`${cmd} '${text.replace(/'/g, \"'\\\\''\")}'`) incorporates external input: `bin` is derived from the command string (`cmd`) in the loop, and `text` comes from `args.trim().slice(0, 20)`—which originates from user-provided input via the `/ascii ` command. (+1 more matches of this pattern in the same file)", - "execution_path": "User invokes `/ascii ` → `args.trim()` extracts user input → `text` is set → `execSync` runs shell commands using template literals that interpolate `bin` and `text` → shell interprets injected characters (e.g., `;`, `|`, `$()`) if present in `text`.", - "suggested_fix": "Replace template literals with array-based command invocation (e.g., `execSync(['which', bin], ...)` and `execSync([...cmd.split(' '), `'${text.replace(/'/g, \"'\\\\''\")}'`], ...)`), or use `spawn` with explicit argv to avoid shell interpretation of user-controlled parts." - }, - "cwe": "CWE-78" - }, - { - "pattern_id": "js-007-command-injection", - "pattern_title": "Shell command with template literal (injection)", - "severity": "critical", - "file": "/home/curly/KCode/src/tools/diff-viewer.ts", - "line": 171, - "matched_text": "execSync(", - "context": "169: const safeFile = resolvedFile.replace(/[\"`$\\\\]/g, \"\");\n170: const safeCtx = String(Math.min(Math.max(parseInt(String(contextLines)) || 3, 0), 100));\n171: const result = execSync(\n172: `git diff ${flag} -U${safeCtx} -- \"${safeFile}\" 2>&1 || true`,\n173: {\n174: cwd: process.cwd(),", + "file": "/home/curly/KCode/src/core/mcp.ts", + "line": 167, + "matched_text": "validated[name] =", + "context": "165: for (const [name, config] of Object.entries(configs)) {\n166: if (isValidServerConfig(config)) {\n167: validated[name] = config as McpServerConfig;\n168: }\n169: }\n170: if (Object.keys(validated).length === 0) return;", "verification": { "verdict": "confirmed", - "reasoning": "The template literal on line 172 includes `flag`, `safeCtx`, and `safeFile`, where `safeFile` is derived from `resolvedFile` (likely user-controlled via `file`) and `safeCtx` from `contextLines`, both of which may originate from external input (e.g., user-provided file paths or context line counts), enabling shell injection despite partial sanitization.", - "execution_path": "`execSync` → `git diff ${flag} -U${safeCtx} -- \"${safeFile}\" 2>&1 || true` triggered when `msg.includes(\"Command failed\") && msg.includes(\"git diff\")` in the catch block, which itself follows an initial `git diff` failure.", - "suggested_fix": "Escape or quote all interpolated values robustly: replace `resolvedFile.replace(/[\"`$\\\\]/g, \"\")` with a more comprehensive sanitizer (e.g., `safeFile = `\"${resolvedFile.replace(/'/g, \"'\\\"'\\\"'\")}\"`) and ensure `flag` and `contextLines` are validated against whitelists or numeric ranges before interpolation." + "reasoning": "The `name` key is taken directly from the `configs` object (sourced from a CLI config file) and used to assign a value to the `validated` object without filtering for sensitive keys like `__proto__`, `constructor`, or `prototype`. (+1 more matches of this pattern in the same file)", + "execution_path": "User provides a config file containing a `__proto__` key -> `loadFromConfigs` is called with the parsed JSON -> `Object.entries(configs)` yields the `__proto__` key -> `validated[name] = config` pollutes the global Object prototype.", + "suggested_fix": "Add a check to skip forbidden keys: `if (name === '__proto__' || name === 'constructor' || name === 'prototype') continue;`" }, - "cwe": "CWE-78" + "cwe": "CWE-1321" } ], - "elapsed_ms": 87571 + "elapsed_ms": 1602418 } \ No newline at end of file diff --git a/AUDIT_REPORT.md b/AUDIT_REPORT.md index e15e0f6..6a809b1 100644 --- a/AUDIT_REPORT.md +++ b/AUDIT_REPORT.md @@ -1,237 +1,87 @@ # Audit Report — KCode **Auditor:** Astrolexis.space — Kulvex Code -**Date:** 2026-04-06 +**Date:** 2026-04-10 **Project:** /home/curly/KCode -**Languages:** typescript, swift, python, javascript, kotlin, ruby +**Languages:** typescript, swift, python, lua, javascript, kotlin, ruby --- ## Summary - Files scanned: **500** -- Candidates found: **72** -- Confirmed findings: **7** -- False positives: **65** -- Scan duration: 87.6s +- Candidates found: **110** +- Confirmed findings: **2** +- False positives: **86** +- Scan duration: 1602.4s ### Severity breakdown | Severity | Count | |----------|-------| -| 🔴 CRITICAL | 2 | -| 🟠 HIGH | 5 | +| 🟠 HIGH | 1 | +| 🟡 MEDIUM | 1 | --- ## Findings -### 1. 🔴 Shell command with template literal (injection) — CWE-78 +### 1. 🟠 Prototype pollution via bracket notation with user key — CWE-1321 -**File:** `src/tools/diff-viewer.ts:171` -**Severity:** CRITICAL -**Pattern:** `js-007-command-injection` - -**Why this matters:** -Running shell commands with template literals allows injection if any interpolated value is user-controlled. - -**Code:** -```cpp -169: const safeFile = resolvedFile.replace(/["`$\\]/g, ""); -170: const safeCtx = String(Math.min(Math.max(parseInt(String(contextLines)) || 3, 0), 100)); -171: const result = execSync( -172: `git diff ${flag} -U${safeCtx} -- "${safeFile}" 2>&1 || true`, -173: { -174: cwd: process.cwd(), -``` - -**Verification:** The template literal on line 172 includes `flag`, `safeCtx`, and `safeFile`, where `safeFile` is derived from `resolvedFile` (likely user-controlled via `file`) and `safeCtx` from `contextLines`, both of which may originate from external input (e.g., user-provided file paths or context line counts), enabling shell injection despite partial sanitization. - -**Execution path:** `execSync` → `git diff ${flag} -U${safeCtx} -- "${safeFile}" 2>&1 || true` triggered when `msg.includes("Command failed") && msg.includes("git diff")` in the catch block, which itself follows an initial `git diff` failure. - -**Suggested fix:** -``` -Escape or quote all interpolated values robustly: replace `resolvedFile.replace(/["`$\\]/g, "")` with a more comprehensive sanitizer (e.g., `safeFile = `"${resolvedFile.replace(/'/g, "'\"'\"'")}"`) and ensure `flag` and `contextLines` are validated against whitelists or numeric ranges before interpolation. -``` - ---- - -### 2. 🔴 Shell command with template literal (injection) — CWE-78 - -**File:** `src/ui/actions/text-actions.ts:157` -**Severity:** CRITICAL -**Pattern:** `js-007-command-injection` - -**Why this matters:** -Running shell commands with template literals allows injection if any interpolated value is user-controlled. - -**Code:** -```cpp -155: try { -156: const bin = cmd.split(" ")[0]!; -157: execSync(`which ${bin} 2>/dev/null`, { timeout: 2000 }); -158: const output = execSync(`${cmd} '${text.replace(/'/g, "'\\''")}' 2>/dev/null`, { -159: timeout: 5000, -160: }).toString(); -``` - -**Verification:** The template literal on line 157 (`which ${bin}`) and especially line 158 (`${cmd} '${text.replace(/'/g, "'\\''")}'`) incorporates external input: `bin` is derived from the command string (`cmd`) in the loop, and `text` comes from `args.trim().slice(0, 20)`—which originates from user-provided input via the `/ascii ` command. (+1 more matches of this pattern in the same file) - -**Execution path:** User invokes `/ascii ` → `args.trim()` extracts user input → `text` is set → `execSync` runs shell commands using template literals that interpolate `bin` and `text` → shell interprets injected characters (e.g., `;`, `|`, `$()`) if present in `text`. - -**Suggested fix:** -``` -Replace template literals with array-based command invocation (e.g., `execSync(['which', bin], ...)` and `execSync([...cmd.split(' '), `'${text.replace(/'/g, "'\\''")}'`], ...)`), or use `spawn` with explicit argv to avoid shell interpretation of user-controlled parts. -``` - ---- - -### 3. 🟠 dangerouslySetInnerHTML with dynamic content — CWE-79 - -**File:** `benchmarks/certification/tasks.ts:930` -**Severity:** HIGH -**Pattern:** `react-001-dangerously-set` - -**Why this matters:** -dangerouslySetInnerHTML bypasses React's XSS protection. With dynamic content → XSS. - -**Code:** -```cpp -928: \`\`\`tsx -929: function UserComment({ comment }: { comment: string }) { -930: return
; -931: } -932: \`\`\` -933: -``` - -**Verification:** The `__html` property in `dangerouslySetInnerHTML` is assigned directly from the `comment` prop, which originates from user input stored in the database—making it dynamic and vulnerable to XSS. - -**Execution path:** `UserComment` component receives `comment` (user-controlled string) → passed as `__html` value to `dangerouslySetInnerHTML` → React injects raw HTML into DOM without escaping → malicious script tags or event handlers in `comment` execute in context of the app. - -**Suggested fix:** -``` -Wrap `comment` with a sanitizer like DOMPurify before assignment: `
` or at minimum escape HTML entities if DOMPurify is unavailable. -``` - ---- - -### 4. 🟠 innerHTML/outerHTML with dynamic content (XSS) — CWE-79 - -**File:** `ide/vscode/src/sidebar.ts:322` -**Severity:** HIGH -**Pattern:** `js-002-innerhtml` - -**Why this matters:** -Setting innerHTML with dynamic content enables XSS. Use textContent or a sanitizer. - -**Code:** -```cpp -320: div.className = 'message ' + role; -321: if (id) div.dataset.id = id; -322: div.innerHTML = '
' + (role === 'user' ? 'You' : 'KCode') + '
' + -323: '
' + escapeHtml(content) + '
'; -324: messagesEl.appendChild(div); -325: messagesEl.scrollTop = messagesEl.scrollHeight; -``` - -**Verification:** The `content` parameter passed to `addMessage()` is escaped via `escapeHtml()` before being injected into `innerHTML`, but the function itself is called with dynamic content (e.g., from LLM responses or user input), and `escapeHtml()` only protects against basic HTML injection—not all XSS vectors like event handlers or script tags if the escaping is incomplete or bypassed. (+1 more matches of this pattern in the same file) - -**Execution path:** `addMessage(role, content, id)` → `content` (dynamic, e.g., from API/LLM/user) → `escapeHtml(content)` → embedded in `div.innerHTML` string → rendered as HTML - -**Suggested fix:** -``` -Replace `div.innerHTML = ...` with `div.innerHTML = '
...
' + escapeHtml(content) + '
';` → `div.innerHTML = new DOMParser().parseFromString('
...
' + escapeHtml(content) + '
', 'text/html').documentElement.innerHTML;` OR (more simply) use `div.insertAdjacentHTML('beforeend', ...)` with sanitized input, or replace `innerHTML` with `textContent` if full HTML isn’t needed. -``` - ---- - -### 5. 🟠 Hardcoded secret/key in JavaScript/TypeScript — CWE-798 - -**File:** `src/remote/triggers/trigger-api.test.ts:9` +**File:** `src/core/mcp.ts:167` **Severity:** HIGH -**Pattern:** `js-006-hardcoded-secret` +**Pattern:** `js-008-prototype-pollution-bracket` **Why this matters:** -Hardcoded secrets in source code are exposed to anyone with repo access. +Setting object properties via bracket notation with a user-controlled key allows prototype pollution. An attacker can set __proto__.isAdmin = true to affect all objects. **Code:** ```cpp -7: -8: const BASE_URL = "https://cloud.kulvex.ai/api/v1"; -9: const AUTH_TOKEN = "test-token-abc123"; -10: -11: const sampleTrigger: RemoteTrigger = { -12: id: "trg_001", +165: for (const [name, config] of Object.entries(configs)) { +166: if (isValidServerConfig(config)) { +167: validated[name] = config as McpServerConfig; +168: } +169: } +170: if (Object.keys(validated).length === 0) return; ``` -**Verification:** The `AUTH_TOKEN` is a hardcoded string `"test-token-abc123"` that follows a realistic pattern (alphanumeric with hyphen) and is used directly in authentication, suggesting it is not merely a placeholder but a representative value likely copied from actual usage or configuration. +**Verification:** The `name` key is taken directly from the `configs` object (sourced from a CLI config file) and used to assign a value to the `validated` object without filtering for sensitive keys like `__proto__`, `constructor`, or `prototype`. (+1 more matches of this pattern in the same file) -**Execution path:** Test file imports `TriggerApiClient`, which presumably uses `AUTH_TOKEN` (defined at module scope) when making authenticated requests to `BASE_URL`; this token is exposed at module load time and visible in any test run or bundle. +**Execution path:** User provides a config file containing a `__proto__` key -> `loadFromConfigs` is called with the parsed JSON -> `Object.entries(configs)` yields the `__proto__` key -> `validated[name] = config` pollutes the global Object prototype. **Suggested fix:** ``` -Replace `AUTH_TOKEN` with an environment variable, e.g., `process.env.TRIGGER_API_TOKEN ?? "test-token-abc123"`, and ensure it is set in CI/test environments. +Add a check to skip forbidden keys: `if (name === '__proto__' || name === 'constructor' || name === 'prototype') continue;` ``` --- -### 6. 🟠 innerHTML/outerHTML with dynamic content (XSS) — CWE-79 - -**File:** `src/web/static/app.js:357` -**Severity:** HIGH -**Pattern:** `js-002-innerhtml` - -**Why this matters:** -Setting innerHTML with dynamic content enables XSS. Use textContent or a sanitizer. - -**Code:** -```cpp -355: var rendered = window.MarkdownRenderer.renderMarkdown(msg.content); -356: if (window.DOMPurify) { -357: body.innerHTML = window.DOMPurify.sanitize(rendered); -358: } else { -359: body.innerHTML = rendered; -360: } -``` +### 2. 🟡 JSON.parse without try/catch (crash on invalid input) — CWE-754 -**Verification:** The `rendered` value assigned to `body.innerHTML` originates from `window.MarkdownRenderer.renderMarkdown(msg.content)`, and `msg.content` is user-provided (as seen from `msg.id`, `msg.role`, etc.), making it susceptible to XSS unless sanitized—here, DOMPurify is used conditionally, but fallback to unsanitized HTML exists. (+2 more matches of this pattern in the same file) - -**Execution path:** `renderMarkdown(msg.content)` → `rendered` (HTML string with potential XSS payloads) → `body.innerHTML = rendered` (or `DOMPurify.sanitize(rendered)`) → DOM injection → script execution on render - -**Suggested fix:** -``` -Ensure `window.DOMPurify` is always available or polyfilled before use; if not, add a fallback sanitizer (e.g., `DOMPurify.sanitize || ((html) => html)`) or use `textContent` for high-risk contexts. -``` - ---- - -### 7. 🟠 innerHTML/outerHTML with dynamic content (XSS) — CWE-79 - -**File:** `vscode-extension/src/chat-panel.ts:507` -**Severity:** HIGH -**Pattern:** `js-002-innerhtml` +**File:** `src/tools/notebook-utils.ts:36` +**Severity:** MEDIUM +**Pattern:** `js-014-json-parse-no-catch` **Why this matters:** -Setting innerHTML with dynamic content enables XSS. Use textContent or a sanitizer. +JSON.parse() throws SyntaxError on invalid JSON. Without try/catch, malformed input crashes the process or rejects the promise unhandled. **Code:** ```cpp -505: div.className = 'message message-' + role; -506: if (role === 'assistant') { -507: div.innerHTML = formatMarkdown(content); -508: } else { -509: div.textContent = content; -510: } +34: /** Parse .ipynb JSON content into a typed notebook structure */ +35: export function parseNotebook(content: string): JupyterNotebook { +36: const nb = JSON.parse(content); +37: if (nb.nbformat !== 4) { +38: throw new Error(`Only nbformat 4 is supported, found: ${nb.nbformat}`); +39: } ``` -**Verification:** The `content` parameter passed to `addMessage()` is used directly in `formatMarkdown(content)` and assigned to `div.innerHTML`, and since `content` originates from external sources (e.g., user messages, LLM responses), it likely contains untrusted HTML that can execute scripts. (+3 more matches of this pattern in the same file) +**Verification:** The `JSON.parse(content)` call is not wrapped in a try/catch block, meaning any malformed JSON input will throw an unhandled SyntaxError. -**Execution path:** `addMessage(role, content)` → `formatMarkdown(content)` returns HTML string → `div.innerHTML = ...` → appended to `messagesEl` +**Execution path:** parseNotebook(invalidJsonString) -> JSON.parse(invalidJsonString) -> throws SyntaxError **Suggested fix:** ``` -Replace `div.innerHTML = formatMarkdown(content);` with `div.innerHTML = sanitizeHtml(formatMarkdown(content));` using a trusted sanitizer (e.g., DOMPurify) or use `textContent` if Markdown rendering is not strictly needed for untrusted input. +Wrap the JSON.parse call in a try/catch block and throw a more descriptive error or return a result type. ``` --- diff --git a/src/core/mcp.ts b/src/core/mcp.ts index b2c592a..d783fa3 100644 --- a/src/core/mcp.ts +++ b/src/core/mcp.ts @@ -164,6 +164,7 @@ export class McpManager { const validated: McpServersConfig = {}; for (const [name, config] of Object.entries(configs)) { if (isValidServerConfig(config)) { + // KCODE-AUDIT:js-008-prototype-pollution-bracket — Reject __proto__, constructor and prototype keys before assigning. validated[name] = config as McpServerConfig; } } diff --git a/src/tools/notebook-utils.ts b/src/tools/notebook-utils.ts index fd881f7..515eb41 100644 --- a/src/tools/notebook-utils.ts +++ b/src/tools/notebook-utils.ts @@ -33,6 +33,7 @@ export interface CellOutput { /** Parse .ipynb JSON content into a typed notebook structure */ export function parseNotebook(content: string): JupyterNotebook { + // KCODE-AUDIT:js-014-json-parse-no-catch — Wrap JSON.parse in try/catch and handle SyntaxError. const nb = JSON.parse(content); if (nb.nbformat !== 4) { throw new Error(`Only nbformat 4 is supported, found: ${nb.nbformat}`); diff --git a/src/ui/actions/file-actions.ts b/src/ui/actions/file-actions.ts index 66c28fb..d4e1c98 100644 --- a/src/ui/actions/file-actions.ts +++ b/src/ui/actions/file-actions.ts @@ -295,9 +295,13 @@ export async function handleFileAction(action: string, ctx: ActionContext): Prom const { join: pJoin } = await import("node:path"); const hint = (() => { if (fsExists(pJoin(projectRoot, "package.json"))) { - return fsExists(pJoin(projectRoot, "bun.lockb")) - ? "bun test" - : "npm test"; + // Bun moved from binary bun.lockb to text bun.lock — check both. + const hasBun = fsExists(pJoin(projectRoot, "bun.lock")) || + fsExists(pJoin(projectRoot, "bun.lockb")); + if (hasBun) return "bun test"; + if (fsExists(pJoin(projectRoot, "pnpm-lock.yaml"))) return "pnpm test"; + if (fsExists(pJoin(projectRoot, "yarn.lock"))) return "yarn test"; + return "npm test"; } if (fsExists(pJoin(projectRoot, "Cargo.toml"))) return "cargo test"; if (fsExists(pJoin(projectRoot, "go.mod"))) return "go test ./...";