T09 · Insecure Skill Coding Practices
Error
- Location
- service/server.js:63
- Finding
- Enabled LLM analysis fails open when the provider or parser fails<![CDATA[ ## Vulnerability Details **File Location**: `service/server.js:63-65`, `service/server.js:128-130`, `service/llm.js:123-141` **Vulnerability Type**: Failure to enforce the configured secondary security control **Risk Level**: High ### Vulnerable Code ```js // service/server.js:63-65 const llmResult = await analyzeWithLLM(sanitized.cleanText, "email"); llmAnalysis = llmResult?.success ? llmResult.analysis : null; if (llmAnalysis?.llmThreatAssessment === "dangerous") { ``` ```js // service/server.js:128-130 const llmResult = await analyzeWithLLM(sanitized.textForAnalysis, "skill"); llmAnalysis = llmResult?.success ? llmResult.analysis : null; if (llmAnalysis?.recommendation === "reject") { ``` ```js // service/llm.js:123-141 } catch (err) { console.error(`[QUARANTINE LLM] Analysis failed: ${err.message}`); return { success: false, error: err.message, // On failure, default to suspicious — fail safe, not fail open analysis: { summary: "LLM analysis failed — treating as suspicious", flags: ["LLM analysis unavailable"], llmThreatAssessment: "suspicious", reasoning: `Analysis failed: ${err.message}`, }, }; } ``` ### Technical Analysis `analyzeWithLLM()` constructs a suspicious fallback assessment when the remote request times out, the provider returns an error, or the response is invalid JSON. However, both request handlers explicitly discard that assessment whenever `success` is false. Consequently, the pattern-engine verdict remains unchanged. If malicious content evades the regular expressions and fuzzy matching, it can be returned as clean even though the administrator enabled LLM analysis as a second security phase. This contradicts both the source comment claiming fail-safe behavior and the Skill's documented fail-closed security model. A related condition occurs if the optional LLM module cannot be imported at startup: `analyzeWithLLM` remains null and requests continue using only the pattern ...[truncated 1198 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Preserve and enforce the fallback analysis regardless of the `success` flag: ```js const llmResult = await analyzeWithLLM(sanitized.cleanText, "email"); llmAnalysis = llmResult?.analysis ?? { llmThreatAssessment: "suspicious", flags: ["LLM analysis unavailable"], summary: "LLM analysis unavailable; human review required.", }; ``` 2. When enabled LLM analysis fails, explicitly force the score and verdict to at least suspicious: ```js if (!llmResult?.success) { patternResults.threatScore = Math.max( patternResults.threatScore, ALERT_THRESHOLD ); patternResults.verdict = "suspicious"; } ``` 3. Apply equivalent behavior to both email and Skill handlers. 4. Decide whether failure should produce `suspicious` or `blocked`; for a quarantine control advertised as fail-closed, blocking is the safer default. 5. If the configured LLM module fails to load at startup, either terminate the service or expose a degraded-state error that prevents clean verdicts. 6. Add tests covering network timeout, HTTP failure, invalid JSON, empty provider responses, and module-loading failure. ]]>