Skip to content

🔒 feat: add LLM application security review rules - #1453

Open
cyberspace-cs wants to merge 1 commit into
alibaba:mainfrom
cyberspace-cs:feat/add-llm-security-rules
Open

cyberspace-cs wants to merge 1 commit into
alibaba:mainfrom
cyberspace-cs:feat/add-llm-security-rules

Conversation

@cyberspace-cs

Copy link
Copy Markdown

背景

随着 LLM 应用的爆发式增长,AI 应用特有的安全风险(prompt injection、不安全的 LLM 输出处理、agent 工具滥用等)越来越突出。目前的安全规则主要覆盖传统 Web 安全(XSS、SQL 注入等),缺少针对 LLM/Agent 应用的专项检查。

新增规则集

新增 llm_security.md 规则文件,覆盖四大类 LLM 应用安全风险:

类别 检查点
Prompt Injection 用户输入直接拼接进 prompt、RAG 上下文未隔离、工具输出注入
不安全 LLM 输出处理 LLM 输出传给 eval/exec、LLM 生成 SQL 未参数化、LLM 生成路径未验证
Agent 工具安全 任意命令执行、敏感操作无审批、Agent 循环无界
敏感数据泄露 硬编码 API Key、PII 进入 prompt、明文日志

注册方式

system_rules.jsonpath_rule_map 中注册,匹配以下文件模式:

  • **/*llm*.{py,ts,js,go,java}
  • **/*agent*.{py,ts,js,go,java}
  • **/*prompt*.{py,ts,js,go,java}

设计考虑

  • 精准优先:遵循项目一贯的「precision over recall」原则,只在确认风险真实可利用时才标记
  • 不重复现有规则:现有的 eval/exec/subprocess 检查已覆盖传统代码安全,这里补充 LLM 特有的场景
  • 可扩展:后续可以加更多 LLM 安全检查点(如输出过滤、工具权限最小化等)

价值

这个规则集填补了 AI 应用安全审查的空白,让 OCR 工具能更好地服务于 AI 开发者。

Add llm_security.md rule set for AI/LLM application code:
- Prompt injection risks (user input, RAG context, tool outputs)
- Unsafe LLM output handling (eval/exec, SQL injection, path traversal)
- Agent tool safety (arbitrary command execution, sensitive ops, unbounded loops)
- Sensitive data leakage (hardcoded keys, PII in prompts, plaintext logs)

Registered for files matching *llm*, *agent*, or *prompt* patterns.
@github-actions

Copy link
Copy Markdown
Contributor

OpenCodeReview: Review complete: 0 finding(s) across 1 selected item(s).

@Qiyuanqiii Qiyuanqiii left a comment

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.

Thanks for putting this together. Dedicated checks for LLM applications would be useful.

The main thing I'd like to see changed is the rule selection. The new patterns replace the existing language-specific rules for matching files, so we lose checks we already have. I've left an inline comment with examples from the PR build.

A couple of wording suggestions as well:

  • For prompt injection, could we focus on what untrusted input can actually cause? Missing delimiters alone doesn't establish a vulnerability, and adding them doesn't make the input safe.
  • For tool access, it would help to account for existing user authorization and enforced permission scopes. A missing approval prompt doesn't necessarily mean an action is unauthorized.
  • For sensitive data, could we distinguish unauthorized disclosure from an approved use of a provider? Encryption and redaction address different risks.

I'd request changes for the rule-selection issue. The other points are suggestions to make the checks more precise; they don't each need to be treated as separate blockers.

Comment on lines +4 to +6
"**/*llm*.{py,ts,js,go,java}": "llm_security.md",
"**/*agent*.{py,ts,js,go,java}": "llm_security.md",
"**/*prompt*.{py,ts,js,go,java}": "llm_security.md",

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.

Could we keep the language-specific rules when adding these checks? The resolver stops at the first match, so these entries replace the rules below rather than supplementing them. I checked the PR build: agent.go gets only llm_security.md, and user_agent.go matches too.

An opt-in rule with merge_system_rule: true might be one way to preserve both, without these built-in overrides. It would also be helpful to add a few tests for the intended matches.

@wu21-web

Copy link
Copy Markdown
Contributor

I don't think this is a good idea. You should probably update the every rule doc to contain these content, avoid putting them together int a shared doc.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants