reviewing-security

Compare original and translation side by side

🇺🇸

Original

English

🇨🇳

Translation

Chinese

Reviewing Security

安全审查

Overview

概述

Most security review output is worthless because it names categories instead of paths. "Validate all user input" is true of every program ever written and changes nothing. A useful finding is a path: an input an attacker controls, the code that carries it, the operation it reaches, and what the attacker gets.

Core principle: Follow the data, not a checklist. Vulnerabilities live in the gap between where a value enters and where it acts.

Companion skills:

requesting-code-review

for the general review loop,

systematic-debugging

when confirming an exploit path.

大多数安全审查输出之所以毫无价值，是因为它们只罗列类别，而不是攻击路径。“验证所有用户输入”适用于有史以来写过的每个程序，却无法改变任何问题。真正有用的发现是一条路径：攻击者能够控制的输入、承载该输入的代码、它最终触达的操作，以及攻击者能够获得的结果。

核心原则： 追踪数据，而不是清单。漏洞存在于值进入系统与值发挥作用之间的间隙。

配套 skill：

requesting-code-review

用于常规审查流程，

systematic-debugging

用于确认可利用路径。

The Iron Law

铁律

NO FINDING WITHOUT A PATH FROM ATTACKER-CONTROLLED INPUT TO IMPACT

If you cannot name the input, the route it takes, and what it costs the operator, you have a code smell — file it under Observations, not Findings. Padding a report with categorical advice buries the one path that is real.

The corollary: trace the path in the actual code before reporting it. A grep hit is a lead, not a finding.

NO FINDING WITHOUT A PATH FROM ATTACKER-CONTROLLED INPUT TO IMPACT

如果你无法指出输入是什么、它经过的路径，以及它会让运维人员付出什么代价，那么这只是一个代码坏味道（code smell）——应归入“观察项（Observations）”，而不是“发现项（Findings）”。用泛化的分类建议填充报告，会掩埋唯一真实存在的路径。

推论： 报告前要在实际代码中追踪这条路径。grep 命中只是线索，不是发现项。

Checklist

检查清单

Map the attack surface — entry points, who reaches them, what privilege they run with
Trace inputs to sinks — injection in all its forms
Authorization — the highest-yield pass
Secrets and crypto
Process and plugin boundaries
Dependencies
Data exposure — logs, errors, responses
Run the scanners — then verify reachability before reporting
Report — severity by exploitability, each with a path

绘制攻击面——入口点、哪些人能触达、以何种权限运行
将输入追踪至危险汇聚点（sink）——各种形式的注入
授权——最有价值的一轮检查
机密信息与加密
进程与插件边界
依赖项
数据暴露——日志、错误、响应
运行扫描器——报告前先验证可达性
编写报告——按可利用性划分严重程度，每项都附路径

Step 1: Map the Attack Surface

第 1 步：绘制攻击面

Do this before reading implementation. You are building the list of places a stranger can reach.

Entry point	Who can reach it	Privilege it runs with
HTTP route, gRPC method	anon / authed / admin / internal-only	process, DB role, filesystem
Queue or event consumer	whoever can publish	usually full process
File upload, import	any authed user	parser runs in-process
Webhook receiver	the internet, until signature is verified
CLI flags, env, config	local operator
LLM / tool output	anyone who can influence the model's context	whatever the tool can do

bash

undefined

在阅读实现之前先做这一步。你要列出陌生人可以触达的所有位置。

入口点	谁能触达	运行权限
HTTP 路由、gRPC 方法	匿名 / 已认证 / 管理员 / 仅限内部	进程、数据库角色、文件系统
队列或事件消费者	任何能够发布消息的人	通常为完整进程权限
文件上传、导入	任何已认证用户	解析器在进程内运行
Webhook 接收器	互联网用户（在验证签名之前）
CLI 参数、环境变量、配置	本地运维人员
LLM / 工具输出	任何能够影响模型上下文的人	工具所拥有的全部权限

bash

undefined

Enumerate handlers and consumers — adapt the pattern to the framework


That last row is not a novelty. **Model output is untrusted input.** If a document, a web page, or a prior tool result can steer the model into calling a tool, then every sink that tool reaches is attacker-reachable. Review tool definitions with the same suspicion as HTTP handlers.


最后一行并不是什么新概念。**模型输出属于不可信输入。** 如果文档、网页或之前的工具结果能够引导模型调用某个工具，那么该工具能触达的所有危险汇聚点都可被攻击者触达。审查工具定义时，应抱着与审查 HTTP 处理器同样的警惕。

Step 2: Trace Inputs to Sinks

第 2 步：将输入追踪至危险汇聚点

For each entry point, follow the value. A sink is any operation whose behavior changes if the value is hostile.

Sink	What makes it dangerous	The safe form
SQL / query builder	string concatenation or interpolation	parameterized query; allowlist for identifiers, which cannot be parameterized
Shell / process spawn	a shell interpreter, or user data in argv[0]	exec with an argv array, no shell, fixed binary path
Filesystem path	`..` , absolute paths, symlinks, NUL	join, then `filepath.EvalSymlinks` , then verify the result is still under the root
Outbound HTTP	user-supplied URL → SSRF	allowlist hosts; block private, link-local, and metadata ranges; re-check after redirects
Template render	autoescape off, or `dangerouslySetInnerHTML`	escaped by default; sanitize HTML with a real sanitizer
Deserialization	polymorphic or type-carrying formats	fixed schema, decode into a concrete type, cap size and depth
Archive extraction	zip-slip, symlinks, decompression bombs	validate each entry path, cap total bytes and entry count
Redirect target	open redirect → phishing, token theft	allowlist paths, or same-origin only
Regex over user input	catastrophic backtracking	bound input length, avoid nested quantifiers
`eval` / reflection / dynamic import	arbitrary code	there is no safe form — remove it

Follow the value across boundaries. The common miss is a value that is validated at the edge, stored, and then read back and trusted. Storage is not sanitization. If it is rendered or executed later, it is still tainted.

Concrete vulnerable/fixed pairs for each row: see references/vulnerability-patterns.md.

对于每个入口点，跟踪该值。危险汇聚点（sink）是任何其行为会因该值为恶意输入而改变的操作。

危险汇聚点	危险来源	安全形式
SQL / 查询构建器	字符串拼接或插值	参数化查询；标识符使用允许列表，因为标识符无法参数化
Shell / 进程创建	shell 解释器，或用户数据进入 argv[0]	使用 argv 数组执行 exec，不经过 shell，并固定二进制路径
文件系统路径	`..` 、绝对路径、符号链接、NUL	先 join，再 `filepath.EvalSymlinks` ，然后验证结果仍位于根目录之下
出站 HTTP	用户提供的 URL → SSRF	主机允许列表；阻止私有、链路本地和元数据地址范围；重定向后重新检查
模板渲染	关闭自动转义，或使用 `dangerouslySetInnerHTML`	默认转义；使用真正的 sanitizer 清理 HTML
反序列化	多态或携带类型信息的格式	固定 schema，解码为具体类型，限制大小和深度
归档解压	zip-slip、符号链接、解压炸弹	校验每个条目路径，限制总字节数和条目数量
重定向目标	开放重定向 → 钓鱼、令牌窃取	路径允许列表，或仅允许同源
对用户输入执行正则	灾难性回溯	限制输入长度，避免嵌套量词
`eval` / 反射 / 动态导入	任意代码执行	没有安全形式——应移除

跨边界跟踪这个值。 最常见的遗漏，是某个值在边缘处经过验证、被存储，随后又被读出并直接信任。存储不等于净化。如果它之后会被渲染或执行，它仍应被视为受污染数据。

每一行对应的具体漏洞与修复示例：参见 references/vulnerability-patterns.md。

Step 3: Authorization

第 3 步：授权

This is where the real bugs are. Injection is well-defended by modern libraries; broken object-level authorization is not, because it cannot be fixed by a framework — it requires knowing which rows belong to whom.

Every handler taking an ID verifies the caller may access that object — not merely that they are logged in
The check is on the path the data actually takes; a check in the route layer that the service layer can bypass is not a check
Tenant/org scoping is in the query (
```
WHERE tenant_id = ?
```
), not applied to results after fetching
No role, tenant, price, or user ID is trusted from the request body — mass assignment is authorization bypass with extra steps
Default deny: a new route without an explicit policy is unreachable, not public
State-changing operations are not reachable by
```
GET
```
Sequential IDs are not the only thing standing between a user and someone else's record
Admin surfaces are gated by more than an unlinked URL
Authorization is re-checked on each step of multi-step flows, not only at the start

bash

undefined

真正的问题往往出现在这里。 现代库已经能够较好地防御注入；但对象级授权失效并非如此，因为它无法依靠框架修复——必须知道哪些数据行属于谁。

每个接收 ID 的处理器都要验证调用者是否有权访问该对象，而不能只验证其已登录
检查必须位于数据实际经过的路径上；如果服务层可以绕过路由层中的检查，那么这就不算检查
租户或组织范围限制必须放在查询中（
```
WHERE tenant_id = ?
```
），不能在取回结果后再过滤
不得信任请求体中的角色、租户、价格或用户 ID——批量赋值（mass assignment）就是多绕几步的授权绕过
默认拒绝：没有明确策略的新路由应当不可触达，而不是公开访问
会改变状态的操作不得通过
```
GET
```
触达
顺序递增的 ID 不能成为用户与他人记录之间的唯一屏障
管理界面不能仅靠一个未与权限系统关联的 URL 来防护
多步骤流程中的每一步都要重新检查授权，而不只是开始时检查

bash

undefined

Handlers that take an identifier — each one needs an ownership check

grep -rnE 'Param("id")|params.id|req.body.(userId|tenantId|role)' --include='.go' --include='.ts'

undefined

grep -rnE 'Param("id")|params.id|req.body.(userId|tenantId|role)' --include='.go' --include='.ts'

undefined

Step 4: Secrets and Crypto

第 4 步：机密信息与加密

No secret in source, config committed to git, logs, error messages, URLs, or client-visible bundles
No secret in git history — rotate anything ever committed; deleting it in a later commit fixes nothing
Passwords hashed with argon2id/scrypt/bcrypt — never a bare SHA, never unsalted
Tokens and signatures compared with a constant-time function
Randomness for tokens, IDs, and salts comes from a CSPRNG (
```
crypto/rand
```
,
```
crypto.randomUUID
```
) — never
```
math/rand
```
JWTs: signature verified, algorithm pinned (reject
```
none
```
and algorithm confusion),
```
exp
```
/
```
aud
```
/
```
iss
```
checked

TLS verification never disabled — no

InsecureSkipVerify: true

, no

NODE_TLS_REJECT_UNAUTHORIZED=0

Session tokens rotate on privilege change; logout invalidates server-side
Cookies:
```
HttpOnly
```
,
```
Secure
```
,
```
SameSite
```
set deliberately
No hand-rolled crypto primitives or custom "encryption"

bash

gitleaks detect --no-banner          # or: trufflehog git file://.
git log -p -S'BEGIN PRIVATE KEY' --oneline | head

源代码、提交到 git 的配置、日志、错误消息、URL 或客户端可见的 bundle 中不得包含机密信息
git 历史记录 中也不得包含机密信息——对曾经提交过的任何机密信息都必须进行轮换；事后提交删除无法解决问题
密码使用 argon2id/scrypt/bcrypt 哈希——绝不能使用裸 SHA，也不能无盐
令牌和签名使用恒定时间函数进行比较
令牌、ID 和盐值所需的随机性来自 CSPRNG（
```
crypto/rand
```
、
```
crypto.randomUUID
```
）——绝不能使用
```
math/rand
```
JWT：验证签名，固定算法（拒绝
```
none
```
和算法混淆），检查
```
exp
```
/
```
aud
```
/
```
iss
```

绝不禁用 TLS 验证——不得出现

InsecureSkipVerify: true

或

NODE_TLS_REJECT_UNAUTHORIZED=0

权限变化时轮换会话令牌；注销时在服务端使其失效
Cookie：明确设置
```
HttpOnly
```
、
```
Secure
```
、
```
SameSite
```
不使用自行实现的加密原语或自定义“加密”

bash

gitleaks detect --no-banner          # or: trufflehog git file://.
git log -p -S'BEGIN PRIVATE KEY' --oneline | head

Step 5: Process and Plugin Boundaries

第 5 步：进程与插件边界

Step 6: Dependencies

第 6 步：依赖项

Lockfile committed and honored in the build (
```
npm ci
```
,
```
go mod verify
```
)
Advisory scan clean, or each exception justified in writing with a reachability argument
New direct dependencies: check age, maintenance, download counts, and name against typosquats
Install/postinstall scripts reviewed for any new dependency
CI actions pinned to a SHA, not a moving tag

bash

govulncheck ./...          # Go — reports only vulnerabilities you actually reach
npm audit --omit=dev       # Node
pip-audit                  # Python

govulncheck

is worth more than a generic scanner precisely because it does reachability analysis. Prefer tools that tell you whether the vulnerable function is called.

提交 lockfile，并在构建时严格遵守（
```
npm ci
```
、
```
go mod verify
```
）
安全公告扫描无问题；若有例外，必须以书面形式说明理由并给出可达性论证
新增直接依赖：检查其存续时间、维护状态、下载量，并将名称与仿冒包（typosquatting）名单进行比对
审查任何新依赖的安装或 postinstall 脚本
CI actions 固定到 SHA，而不是可移动的 tag

bash

govulncheck ./...          # Go — reports only vulnerabilities you actually reach
npm audit --omit=dev       # Node
pip-audit                  # Python

govulncheck

比通用扫描器更有价值，正是因为它会执行可达性分析。应优先选择能够告诉你漏洞函数是否被调用的工具。

Step 7: Data Exposure

第 7 步：数据暴露

Errors returned to clients carry no stack traces, SQL, file paths, or internal hostnames
Logs contain no passwords, tokens, session IDs, full card numbers, or PII beyond what is needed
Debug endpoints (
```
/debug/pprof
```
, GraphQL introspection, admin consoles) are disabled or gated in production
CORS: no
```
Access-Control-Allow-Origin: *
```
combined with credentials; the origin allowlist is explicit
Authenticated responses are not cacheable by shared caches
Object storage buckets and their listings are private by default
Timing and error-message differences do not reveal whether an account exists

返回客户端的错误不得包含堆栈跟踪、SQL、文件路径或内部主机名
日志不得包含密码、令牌、会话 ID、完整卡号或超出必要范围的 PII
调试端点（
```
/debug/pprof
```
、GraphQL introspection、管理控制台）在生产环境中应禁用或设防
CORS：不得将
```
Access-Control-Allow-Origin: *
```
与凭据结合使用；来源允许列表必须明确
经认证的响应不得被共享缓存缓存
对象存储 bucket 及其列表默认私有
响应时间及错误消息的差异不得泄露账户是否存在

Step 8: Run the Scanners, Then Verify

第 8 步：运行扫描器，然后验证

bash

semgrep --config=auto .

Scanner output is a lead list, not a report. Before reporting any scanner hit, confirm it is reachable in this codebase — check whether the sink is actually called with a tainted value. Forwarding raw tool output is how a review becomes noise that gets ignored, taking the real findings with it.

bash

semgrep --config=auto .

扫描器输出只是线索清单，不是报告。报告任何扫描器命中之前，先确认它在本代码库中可达——检查危险汇聚点是否确实以受污染的值被调用。直接转发原始工具输出，会让审查变成噪声并被忽视，真实发现也会随之被掩埋。

Severity

严重程度

Grade by exploitability, not by category name.

Severity	Definition
Critical	Unauthenticated attacker gains code execution, credentials, or bulk access to other users' data. No unusual preconditions.
High	Authenticated attacker escalates privilege, reads or writes another tenant's data, or extracts secrets.
Medium	Requires an unusual precondition (a specific race, a chained bug, a user action) or yields limited data.
Low	Hardening. Real weakness, no demonstrated path today — a missing defense-in-depth layer that would contain a future bug.

Do not inflate. A report where everything is Critical is a report nobody acts on.

按可利用性分级，而不是按类别名称。

严重程度	定义
严重（Critical）	未认证攻击者获得代码执行能力、凭据或批量访问其他用户数据的能力，且不需要特殊前置条件。
高危（High）	已认证攻击者提升权限、读写其他租户的数据或提取机密信息。
中危（Medium）	需要异常的前置条件（特定竞态、链式缺陷、用户操作），或仅造成有限的数据泄露。
低危（Low）	加固项。确实存在弱点，但目前没有已证实的利用路径——缺少一层本可遏制未来缺陷的纵深防御。

不要夸大严重程度。如果报告中所有问题都是“严重”，最终谁也不会采取行动。

Report Format

报告格式

markdown

undefined

markdown

undefined

Scope

[What was reviewed — diff range, paths, or "full repo". What was NOT reviewed.] [Scanners run and their versions.]

Critical

Tenant isolation missing on document fetch

internal/api/documents.go:88

— the handler loads by primary key only:

db.First(&doc, c.Param("id"))

. There is no

tenant_id

predicate and no post-fetch ownership check. Path: any authenticated user →

GET /api/documents/{id}

→ sequential integer IDs → full document body of any tenant. Impact: cross-tenant read of all documents; IDs are enumerable. Fix: scope the query —

db.Where("tenant_id = ?", claims.TenantID).First(&doc, id)

— and add the predicate at the repository layer so new callers inherit it.

Tenant isolation missing on document fetch

internal/api/documents.go:88

— the handler loads by primary key only:

db.First(&doc, c.Param("id"))

. There is no

tenant_id

predicate and no post-fetch ownership check. Path: any authenticated user →

GET /api/documents/{id}

→ sequential integer IDs → full document body of any tenant. Impact: cross-tenant read of all documents; IDs are enumerable. Fix: scope the query —

db.Where("tenant_id = ?", claims.TenantID).First(&doc, id)

— and add the predicate at the repository layer so new callers inherit it.

High

[same shape]

Medium / Low

[same shape]

Observations (not findings)

[Weaknesses with no demonstrated path. Say so plainly rather than promoting them.]

Verified Clean

[What you checked and found sound. Silence reads as "not checked".]

Assessment

Ship? Yes / No / After the Critical and High items Reasoning: [1-2 sentences]

undefined

Ship? Yes / No / After the Critical and High items Reasoning: [1-2 sentences]

undefined

What NOT to Report

不应报告的内容

Do not report	Why
"Add input validation" with no sink	Categorical advice, not a finding
Missing rate limiting with no abusable endpoint named	Name the endpoint and the cost, or drop it
Missing browser security headers on a service with no browser client	Threat does not apply
Findings in tests, fixtures, or example code	Not attacker-reachable
Scanner output you did not confirm is reachable	Noise that buries real findings
Cryptographic preference nits with no threat	"Use AES-256 instead of AES-128" is not a finding
Anything you would describe as "theoretically"	If it were exploitable you would have the path

不应报告	原因
没有危险汇聚点却只建议“添加输入验证”	泛化的分类建议，不是发现项
未指出可滥用端点却称缺少速率限制	指出端点及其造成的代价，否则删除此项
服务没有浏览器客户端却称缺少浏览器安全响应头	威胁不适用
测试、fixture 或示例代码中的发现	攻击者无法触达
未确认可达性的扫描器输出	噪声，会掩埋真实发现
没有对应威胁的加密偏好建议	“改用 AES-256 而不是 AES-128”不构成发现项
任何你会称为“理论上”的问题	如果能够利用，就应能给出路径

Common Mistakes

常见错误

Checklist review instead of path review. Walking OWASP Top 10 top to bottom finds the vulnerabilities that are already handled by the framework and misses the authorization bug that is unique to this domain.

Stopping at the entry point. Validation at the edge tells you nothing if the value is stored and later concatenated into a query.

Trusting the framework blindly. ORMs have raw-query escape hatches, templates have raw-output escape hatches, and both get used. Grep for them specifically.

Treating authenticated as authorized. The single most common real-world finding, and the easiest to miss by reading the route table instead of the query.

Reporting the model as trusted. If a tool call can be influenced by content the model read, its arguments are attacker-controlled.

Severity inflation to seem thorough. It costs you the reader's attention on the finding that matters.

用清单审查代替路径审查。 从头到尾走一遍 OWASP Top 10，只会找到框架已经处理好的漏洞，却漏掉该领域特有的授权缺陷。

止步于入口点。 边缘处的验证无法说明任何问题；如果值被存储，之后又拼接到查询中，它依然危险。

盲目信任框架。 ORM 存在原始查询的逃生口，模板存在原始输出的逃生口，而这两者都有人使用。应专门搜索它们。

把已认证等同于已授权。 这是现实中最常见、也最容易因只看路由表而不看查询而漏掉的问题。

把模型视为可信。 如果工具调用可能受模型所读取内容的影响，那么它的参数就是攻击者可控的。

为了显得全面而夸大严重程度。 这会消耗读者对真正重要发现的注意力。