Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
272 changes: 272 additions & 0 deletions src/core/tools/error-interception/ErrorClassifier.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,272 @@
import { ERROR_PATTERNS } from "./errorPatterns"
import type { ClassifyOptions, ErrorClassification, ErrorPattern, InterceptionSignal } from "./types"

// ---------------------------------------------------------------------------
// Safe-identifier validation (prompt-injection prevention)
// ---------------------------------------------------------------------------

const SAFE_IDENTIFIER_RE = /^[a-zA-Z_][\w.]*$/
const MAX_PARAM_NAME_LENGTH = 128

/**
* Returns `true` only when `name` is a safe identifier suitable for
* interpolation into model-facing guidance text.
*
* Accepts plain identifiers (`path`, `file_pattern`) and dotted member
* access chains (`options.timeout`). Rejects anything that could carry
* prompt-injection payloads: newlines, quotes, angle brackets, brackets,
* shell metacharacters, backslashes, and overlength strings.
*/
export function isValidIdentifier(name: string | undefined): boolean {
if (typeof name !== "string") return false
if (name.length === 0 || name.length > MAX_PARAM_NAME_LENGTH) return false
if (!SAFE_IDENTIFIER_RE.test(name)) return false
// Reject instruction-like patterns.
if (/[\n\r"'><\[\]{}()|;`\\]/.test(name)) return false
return true
}

const SAFE_FACT_KEYS = new Set<string>([
"category",
"code",
"commandSubmitted",
"contextLengthExceeded",
"contextOverflow",
"contextWindowExceeded",
"errorCode",
"errorName",
"errorSource",
"errorStage",
"errorType",
"emptyArguments",
"fileNotFound",
"fileRestriction",
"invalidProtocol",
"missingNativeArgs",
"missingParameter",
"missingRequiredParameters",
"modeRestriction",
"parameterName",
"parseFailureKind",
"pathEmpty",
"repetitionCount",
"retryDisposition",
"server",
"shellIntegrationError",
"status",
"tool",
"toolName",
"type",
"typeMismatch",
"unknownTool",
"validSiblingPresent",
"xmlToolCall",
])

const SENSITIVE_KEYS = new Set<string>([
"command",
"commandText",
"cwd",
"env",
"environmentVariable",
"path",
"absolutePath",
"homePath",
"apiKey",
"api_key",
"token",
"secret",
"password",
"prompt",
"response",
"resultText",
"mcpArguments",
"arguments",
"args",
])

function isSafeFactKey(key: string): boolean {
if (!SAFE_FACT_KEYS.has(key)) return false
return !SENSITIVE_KEYS.has(key)
}

function hasToolContext(signal: InterceptionSignal): boolean {
return signal.toolName !== undefined || signal.toolCallId !== undefined
}

/**
* Extract a parameter name from an error message or result text.
*
* Common patterns from tool execution errors:
* - "Required parameter 'path' is missing"
* - "The 'path' parameter must be a string"
* - "Missing required parameter: command"
* - "parameter 'path' is required"
*/
function extractParameterName(signal: InterceptionSignal): string | undefined {
// Check metadata first (explicitly provided by the caller).
const metaName = signal.metadata["parameterName"]
if (typeof metaName === "string" && metaName.length > 0) return metaName

// Try to extract from error.message.
if (signal.error !== null && typeof signal.error === "object") {
const message = (signal.error as { message?: unknown }).message
if (typeof message === "string") {
const name = tryExtractParamNameFromText(message)
if (name) return name
}
}

// Try to extract from result.text.
if (typeof signal.result === "object" && signal.result !== null) {
const text = (signal.result as { text?: unknown }).text
if (typeof text === "string") {
const name = tryExtractParamNameFromText(text)
if (name) return name
}
}

return undefined
}

function tryExtractParamNameFromText(text: string): string | undefined {
// Pattern: "parameter 'name'" or "parameter \"name\"" or "parameter: name"
const paramQuoteMatch = text.match(/parameter\s*['"']([^'"']+)['"']/i)
if (paramQuoteMatch) return paramQuoteMatch[1]

// Pattern: "Required parameter 'name'" — already covered above, but also
// try "Missing required parameter: name" (colon-separated, no quotes).
const colonMatch = text.match(/(?:missing|required)\s+parameter\s*[:\s]+(\w+)/i)
if (colonMatch) return colonMatch[1]

// Pattern: "The 'name' parameter must be..." — extract the quoted name
// before the word "parameter".
const theParamMatch = text.match(/the\s+['"']([^'"']+)['"']\s+parameter/i)
if (theParamMatch) return theParamMatch[1]

return undefined
}

function isEligible(pattern: ErrorPattern, signal: InterceptionSignal): boolean {
if (pattern.category === "UNCLASSIFIED") return false
return !pattern.requiresToolContext || hasToolContext(signal)
}

function sanitizeFacts(signal: InterceptionSignal, pattern: ErrorPattern): Readonly<Record<string, unknown>> {
const facts: Record<string, unknown> = {}

for (const key of Object.keys(signal.metadata)) {
if (!isSafeFactKey(key)) continue

const value = signal.metadata[key]
if (value === undefined || value === null) continue

if (typeof value === "boolean" || typeof value === "number" || typeof value === "string") {
facts[key] = value
continue
}

// Arrays of primitive tool/server identifiers only.
if (Array.isArray(value) && value.every((item) => typeof item === "string")) {
facts[key] = value
}
}

// Validate metadata-provided parameterName through the same
// safe-identifier check. The loop above copies metadata values
// verbatim, so an unsafe parameterName from metadata would bypass
// the extraction-path validation below.
if (typeof facts.parameterName === "string" && !isValidIdentifier(facts.parameterName)) {
delete facts.parameterName
}
Comment on lines +158 to +181

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Trace how classification facts reach model-facing text, to confirm the injection sink.
set -euo pipefail

rg -n -C4 '\bfacts\b' --type=ts -g '!**/__tests__/**'
rg -n -C4 'classifyError|classifyToolResult|GuidancePayload|error_details' --type=ts -g '!**/__tests__/**'

Repository: Zoo-Code-Org/Zoo-Code

Length of output: 159


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== files =="
git ls-files | sed -n '/ErrorClassifier\|errorPatterns\|types\.ts\|error-interception/p' | head -200

echo "== ErrorClassifier outline =="
ast-grep outline src/core/tools/error-interception/ErrorClassifier.ts || true

echo "== ErrorClassifier relevant lines =="
sed -n '1,320p' src/core/tools/error-interception/ErrorClassifier.ts | nl -ba

echo "== inter-file facts references =="
rg -n '\bfacts\b|classifyError|classifyToolResult|GuidancePayload|error_details' src/core --glob 'ts' --glob '!**/__tests__/**' || true

Repository: Zoo-Code-Org/Zoo-Code

Length of output: 2166


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== ErrorClassifier relevant lines =="
sed -n '1,320p' src/core/tools/error-interception/ErrorClassifier.ts | awk '{printf "%5d\t%s\n", NR, $0}'

echo "== cross-repository facts/usages excluding tests =="
rg -n '\bfacts\b|classifyError|classifyToolResult|GuidancePayload|error_details' src --glob 'ts' --glob '!**/__tests__/**' || true

Repository: Zoo-Code-Org/Zoo-Code

Length of output: 10206


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== files mentioning classifyError classification ErrorClassification guidance model =="
rg -n 'classifyError|classifyToolResult|ErrorClassification|error classification|classification facts|facts\.' src packages apps --glob 'ts' --glob '!**/__tests__/**' || true

echo "== ErrorClassifier tests around metadata sanitize/classifyToolResult =="
sed -n '100,1140p' src/core/tools/error-interception/__tests__/ErrorClassifier.spec.ts | awk '{printf "%5d\t%s\n", NR, $0}'

echo "== exact symbols in tests =="
rg -n 'sanitizeFacts|metadata=|facts|status|type|missingRequiredParameters|server|toolName' src/core/tools/error-interception/__tests__/ErrorClassifier.spec.ts || true

Repository: Zoo-Code-Org/Zoo-Code

Length of output: 43486


LLM Security (CWE-1427)

Reachability path
● Entry
  src/core/tools/error-interception/__tests__/ErrorClassifier.spec.ts
│
▼
● Sink
  src/core/tools/error-interception/ErrorClassifier.ts

Sanitize every metadata string copied into facts.

sanitizeFacts copies all SAFE_FACT_KEYS string and string-array values verbatim except parameterName, so classifyToolResult({ status: "missing-parameter" }) stores unbounded result.status and result.type as facts. Values like status, type, code, errorName, parseFailureKind, toolName, server, and entries in missingRequiredParameters should be bounded and screened for control characters/quotes/newlines.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/tools/error-interception/ErrorClassifier.ts` around lines 158 - 181,
The metadata-copying loop in sanitizeFacts must sanitize every string and
string-array value before assigning it to facts, not only parameterName. Apply
the existing bounded identifier/string sanitizer to status, type, code,
errorName, parseFailureKind, toolName, server, and missingRequiredParameters
entries; reject or omit values containing control characters, quotes, or
newlines while preserving valid primitive metadata.


facts.pattern = pattern.id
facts.category = pattern.category
facts.errorSource = signal.source

// Inject extracted parameter name for PARAM_MISSING and generic
// PARAM_TYPE_MISMATCH patterns so the transformer can include it in
// guidance messages. Skip the CWD_OBJECT_MISUSE and NESTED_PARAM_OVERFLOW
// variants — they have their own specific guidance.
if (
pattern.category === "PARAM_MISSING" ||
(pattern.category === "PARAM_TYPE_MISMATCH" && pattern.id === "EI/PARAM_TYPE_MISMATCH/001")
) {
if (facts.parameterName === undefined) {
const paramName = extractParameterName(signal)
// Only store the parameter name if it passes the safe-identifier
// check. Untrusted content (file contents, shell/MCP output) can
// flow through error messages and result text, so we must reject
// anything that looks like a prompt-injection payload.
if (paramName !== undefined && isValidIdentifier(paramName)) {
facts.parameterName = paramName
}
}
}

return Object.freeze(facts)
}

export function classifyError(signal: InterceptionSignal, _options?: ClassifyOptions): ErrorClassification {
// First pass: exact/structural matchers only.
for (const pattern of ERROR_PATTERNS) {
if (!isEligible(pattern, signal)) continue
if (pattern.matches(signal)) {
return {
category: pattern.category,
patternId: pattern.id,
confidence: "exact",
retryPolicy: pattern.retryPolicy,
facts: sanitizeFacts(signal, pattern),
}
}
}

// Second pass: heuristic fallback matchers, excluding the UNCLASSIFIED
// catch-all at the end of the list.
for (const pattern of ERROR_PATTERNS) {
if (!isEligible(pattern, signal)) continue
if (pattern.fallback?.(signal)) {
return {
category: pattern.category,
patternId: pattern.id,
confidence: "heuristic",
retryPolicy: pattern.retryPolicy,
facts: sanitizeFacts(signal, pattern),
}
}
}

// UNCLASSIFIED catch-all.
const fallback = ERROR_PATTERNS[ERROR_PATTERNS.length - 1]
return {
category: fallback.category,
patternId: fallback.id,
confidence: "heuristic",
retryPolicy: fallback.retryPolicy,
facts: sanitizeFacts(signal, fallback),
}
Comment on lines +240 to +248

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check whether noUncheckedIndexedAccess is enabled for the reviewed source tree.
set -euo pipefail

fd -H -t f 'tsconfig*.json' --exec sh -c 'echo "== $1"; cat "$1"' _ {}
rg -n 'noUncheckedIndexedAccess' -g '*.json' || echo "noUncheckedIndexedAccess not found"

Repository: Zoo-Code-Org/Zoo-Code

Length of output: 4164


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate files =="
fd -t f 'ErrorClassifier.ts|errorPatterns.ts|base.json|package.json|tsconfig.json' . | sort

echo
echo "== ErrorClassifier relevant lines =="
sed -n '220,260p' packages/core/src/core/tools/error-interception/ErrorClassifier.ts
echo
echo "== errorPatterns snippet =="
sed -n '1,160p' packages/core/src/core/tools/error-interception/errorPatterns.ts
echo
echo "== fallback/UNCLASSIFIED usage =="
rg -n 'UNCLASSIFIED|length - 1|ERROR_PATTERNS|fallback|classifyError|noUncheckedIndexedAccess' packages/core/src/core/tools/error-interception -S

echo
echo "== base config =="
fd -t f 'base.json' . -x sh -c 'echo "-- $1"; sed -n "1,220p" "$1"' sh {}

Repository: Zoo-Code-Org/Zoo-Code

Length of output: 1264


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="src/core/tools/error-interception/ErrorClassifier.ts"

echo "== ErrorClassifier relevant lines =="
sed -n '220,260p' "$FILE"
echo
echo "== ErrorClassifier outline around classifyError =="
ast-grep outline "$FILE" --match classifyError --view expanded || true
echo
echo "== errorPatterns relevant lines =="
sed -n '1,160p' src/core/tools/error-interception/errorPatterns.ts
echo
echo "== fallback/UNCLASSIFIED usage =="
rg -n 'UNCLASSIFIED|length - 1|ERROR_PATTERNS|fallback|classifyError|noUncheckedIndexedAccess' src/core/tools/error-interception -S

echo
echo "== base config =="
fd -t f 'base.json' . -x sh -c 'echo "-- $1"; sed -n "1,220p" "$1"' sh {}

Repository: Zoo-Code-Org/Zoo-Code

Length of output: 21445


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== package files extending base =="
for f in $(fd -t f 'package.json' packages/core src apps/webview-ui | sort | tr '\n' ' '); do
  echo "-- $f"
  sed -n '1,160p' "$f" | awk '/eslintConfig|noUncheckedIndexedAccess|files:|references:|include:|extends:"`@roo-code`\/config-typescript\/base"|extends:[[:space:]]*"/{c=1} c; /^}/ && c{exit}'
done

echo
echo "== eslint config candidates =="
fd -t f 'eslint.config.*|.*eslint.*' . | sed -n '1,120p'

echo
echo "== TypeScript noUncheckedIndexedAccess project files =="
python3 - <<'PY'
from pathlib import Path
import json
base = json.loads(Path("packages/config-typescript/base.json").read_text())
flag = base["compilerOptions"].get("noUncheckedIndexedAccess")
print("base noUncheckedIndexedAccess =", flag)
for p in sorted(Path(".").rglob("tsconfig.json")):
    text=p.read_text()
    try:
        cfg=json.loads(text)
    except Exception:
        continue
    path=str(p)
    ext=cfg.get("extends")
    inc=cfg.get("include", [])
    refs=cfg.get("references", [])
    if any(path.endswith(r) for r in ("src/core","src","packages/config-typescript")) and ext == "`@roo-code/config-typescript/base.json`":
        print("extended base", path, "includes", inc, "extensions", refs)
PY

echo
echo "== local array length - 1 patterns =="
rg -n '\[.*\.[^.]*length\s*-\s*1\]|length\s*-\s*1' src packages/core -S || true

Repository: Zoo-Code-Org/Zoo-Code

Length of output: 27002


Select the UNCLASSIFIED fallback by category, not by array index.

classifyError currently maps every unmatched signal to ERROR_PATTERNS[ERROR_PATTERNS.length - 1]. Reordering or inserting a low-priority catch-all changes these classifications to the wrong category. Also, the TypeScript base config enables noUncheckedIndexedAccess, so this indexed access is not covered by the existing ordering test and needs a guarded resolve.

♻️ Proposed fix: resolve the catch-all by category
+const UNCLASSIFIED_PATTERN = ERROR_PATTERNS.find((pattern) => pattern.category === "UNCLASSIFIED")
+
+if (UNCLASSIFIED_PATTERN === undefined) {
+	throw new Error("ERROR_PATTERNS must contain an UNCLASSIFIED catch-all pattern")
+}
+
 export function classifyError(signal: InterceptionSignal, _options?: ClassifyOptions): ErrorClassification {
 	// UNCLASSIFIED catch-all.
-	const fallback = ERROR_PATTERNS[ERROR_PATTERNS.length - 1]
+	const fallback = UNCLASSIFIED_PATTERN
 	return {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// UNCLASSIFIED catch-all.
const fallback = ERROR_PATTERNS[ERROR_PATTERNS.length - 1]
return {
category: fallback.category,
patternId: fallback.id,
confidence: "heuristic",
retryPolicy: fallback.retryPolicy,
facts: sanitizeFacts(signal, fallback),
}
const UNCLASSIFIED_PATTERN = ERROR_PATTERNS.find((pattern) => pattern.category === "UNCLASSIFIED")
if (UNCLASSIFIED_PATTERN === undefined) {
throw new Error("ERROR_PATTERNS must contain an UNCLASSIFIED catch-all pattern")
}
// UNCLASSIFIED catch-all.
const fallback = UNCLASSIFIED_PATTERN
return {
category: fallback.category,
patternId: fallback.id,
confidence: "heuristic",
retryPolicy: fallback.retryPolicy,
facts: sanitizeFacts(signal, fallback),
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/tools/error-interception/ErrorClassifier.ts` around lines 240 - 248,
Update the UNCLASSIFIED fallback resolution in classifyError to locate the
ERROR_PATTERNS entry by its category rather than relying on the final array
index. Guard the lookup for noUncheckedIndexedAccess and preserve the existing
fallback result shape, including sanitizeFacts(signal, fallback); handle a
missing matching pattern explicitly.

}

/** Convenience helper to classify a structured tool result directly. */
export function classifyToolResult(
result: InterceptionSignal["result"],
taskId: string,
toolCallId?: string,
): ErrorClassification {
const metadata: Record<string, unknown> = {}
if (result && typeof result === "object") {
if (result.status) metadata.status = result.status
if (result.type) metadata.type = result.type
}

const signal: InterceptionSignal = {
source: "tool_result",
stage: "result",
taskId,
toolCallId,
result: result ?? undefined,
metadata,
}
return classifyError(signal)
}
Loading
Loading