feat(rules): add AI600-AI900 security rules for agents, RAG, API keys, and output handling - #93
mmustafasenoglu wants to merge 3 commits into
Conversation
…API keys, and output handling Adds 18 new rules covering: - AI600: Unsafe agent behavior & tool poisoning (web browsing, subprocess, file write, indirect injection) - AI700: RAG security (embedding poisoning, context overflow, untrusted sources) - AI800: API key management (OpenAI, Anthropic, Cohere hardcoded keys) - AI900: Output handling & DoS (YAML unsafe load, JSON DoS, exec/eval of LLM output, XSS) Also adds 4 new taint sources/sinks for RAG and agent web tool flows. Closes ParzivalHack#91
- Remove AI601/AI603/AI604: line-based regex cannot establish agent context; SSRF and file access already covered by AI501/AISK08 and AI502/AISK09 taint analysis - Remove AITS12/AISK11: AITS12 tainted every requests.get response, AISK11 duplicated the existing AISK08 sink - AI602: require shell=True instead of flagging all subprocess usage - AI702: pattern now checks score_threshold (was checking top-k count) - AI902/AI903/AI904: narrow patterns to LLM-output variables; drop broad .*response|.*output|.*completion alternatives that matched any line containing those words - AI904: exclude sanitized output (sanitize|bleach) - AITS11: fix malformed function path, match instance calls via .similarity_search - Add negative tests: plain HTTP clients, subprocess without shell, plain json.loads, other embedding models, sanitized HTML, top-k without threshold, plain eval, multiline taint flow - Taint tests: trusted constants stay untainted, no duplicate AI501/AI601 findings
|
Hello mmustafasenoglu, thanks for the review of the PR and sorry for the late response.
But maybe its my fault.
What you think about it? Thanks for your helpful contribution |
Tighten regex to match only thresholds 0.0-0.5 (e.g., 0.3, 0.4, 0.5, 0.50) instead of all decimals. Thresholds >= 0.6 (0.7, 0.9) are considered reasonable and no longer flagged.
|
Hey @desimetallica, can you check this one when you can? :) |
|
Hey Davide, thanks for checking it out and no worries about the delay. On the failing tests — I just re-ran locally and both pass for me. Could be a local env thing? Can you share the traceback if you get a chance? About your points: AITS11 — yeah you're right that AI702 — yes, 0.9 gets flagged. The idea is that any float threshold is technically user-configurable and worth a second look. Low confidence + the warning text says "check if this is production-appropriate" so it reads more like a nudge than a hard finding. I'm fine with it. AI602 — fair point on multiline. The line-by-line scan won't catch something spread across lines. But since we already mark it low confidence, and the regex is pretty specific ( AI904 — the AI902/AI903 — same reasoning, regex will produce some noise but low confidence keeps it from being a blocker. It's more of a signal to the reviewer to pay attention to those patterns. Let me know if you want me to tweak any of these. |
Summary
Adds AI600-AI900 security rules for detecting vulnerabilities in AI/LLM-related code:
Changes
pyspector/rules/ai_security.pyReview Status
Previously reviewed by @desimetallica — addressed all feedback:
confidence = "Low"where appropriateFixes #92 (reopened per maintainer request — volunteers need time, thank you for patience!)