perf(contract): prefilter public boundary scans - #3718
Conversation
Signed-off-by: hyk <4408344+hhyykk@users.noreply.github.com>
huangruiteng
left a comment
There was a problem hiding this comment.
详细中文评审
评审 exact head:4348b721357e95ed3b13b1aab1a6d5eecc8c41b8
动机
这个 PR 希望降低 scan_public_boundary 在 status、quota 和 premerge canary 等调用链上的扫描成本:先用廉价字面量筛出可能命中的文件与行,再运行现有正则。方向合理,而且保留 LEAK_PATTERNS 兼容映射、Lark developer-console 例外、credential-reference 降级和既有输出结构,范围集中在公共/私有边界扫描器及其测试。
改动思路
LeakRule 把权威正则与 required_literals 放在同一个不可变结构中;scan_public_boundary 先对全文 casefold() 建立候选规则,再对候选行重复筛选,最后才调用原正则。正向路径是:文件包含必要字面量 → 行包含必要字面量 → 权威正则命中 → 进入既有例外、策略和命中分类逻辑。负向路径本应只跳过“权威正则不可能命中”的文件或行。
具体改动
loopx/contract.py新增LeakRule.is_candidate、LEAK_RULES和由其派生的兼容LEAK_PATTERNS;_credential_hits_are_all_references改为读取结构化规则中的正则。scan_public_boundary在文件级和行级增加候选过滤,仍由rule.pattern.search(scan_line)执行最终分类。- 新测试覆盖了五类规则的常见 ASCII 分支、候选行隔离及命中顺序;本地复核中相关 contract 测试 51 个通过,Ruff 与 compileall 通过,远端 9 个必需 checks 也均为成功。
但这里有一个 P1 阻塞问题:LeakRule.is_candidate 使用 text.casefold() 后的普通子串判断,并不是 re.I 正则的严格必要条件。Python 的 Unicode re.IGNORECASE 会把 İ(U+0130)视为 ASCII i 的大小写匹配,但 "İ".casefold() 是 "i\u0307"。因此例如 larkoffİce 和 Authorİzation: 会被现有权威正则命中,却会在第 52-53 行以及第 839-850 行的新预筛阶段被跳过。实际端到端复核得到:两条权威正则均为 match、两个 prefilter 均为 false,而 scan_public_boundary 最终错误返回 ok: true 且没有 hits。这使安全边界产生新的 false negative,与 PR 声称的“正则仍是唯一分类权威、行为不变”相冲突。
最小修复是让预筛与权威正则共享可证明等价/蕴含的匹配语义;例如只对明确使用 re.ASCII | re.I 的规则做 ASCII 归一化预筛,或为 re.I 规则采用不会排除其 Unicode 命中集合的候选策略。请加入覆盖 Unicode re.I 特殊折叠字符(至少 İ)的回归测试,并证明所有 pattern.search(text) 的代表性分支都满足 is_candidate(...)。
对主干的风险
最高风险是公共/私有边界守卫漏报本来会被既有正则拦截的私有文档 URL 或 credential header,影响 loopx check、premerge canary 以及所有复用该扫描器的发布路径。由于优化在文件级就可能跳过全部行,漏报不会留下降级告警或可观测证据;当前 ASCII 测试和 corpus 哈希无法覆盖这类 Unicode 语义差异。回滚很简单:在修复等价性前移除该候选过滤即可恢复原有正则行为。
未来演进方面,把规则与 prefilter 放在同一 owner 中是有价值的 bounded refactor;但应先把“必要条件”变成可机器验证的契约,而不是维护一个看似同源、实际拥有不同 Unicode 语义的第二套分类知识。
我的整体评价
REQUEST_CHANGES。性能收益和结构化规则方向都很好,改动范围也克制;不过当前实现改变了安全边界的既有默认行为,并引入可复现漏报,因此不能按 exact head 4348b721357e95ed3b13b1aab1a6d5eecc8c41b8 合入。补齐 Unicode 等价性修复与负向回归后,我愿意快速复审新 head。
English verdict: REQUEST_CHANGES for exact head 4348b721357e95ed3b13b1aab1a6d5eecc8c41b8. The casefold() substring prefilter is not a necessary condition for Python re.I: inputs containing U+0130 can match the authoritative private-URL or authorization regex while being skipped by both candidate stages, producing a public-boundary false negative. I verified the failure end to end; 51 related tests, Ruff, compileall, and all 9 required remote checks otherwise passed. Make the prefilter semantics provably cover every regex match and add a Unicode regression test before merge.
Summary
Performance
Seven matched runs on the package scan root used by
statusandquota:status --limit 5quota should-run --dry-runNearest-rank observed tails also improved: scanner 2.938 to 0.712 seconds, status 3.123 to 1.442 seconds, and quota 3.440 to 1.669 seconds.
Baseline and candidate produced byte-identical canonical boundary payloads on the same package corpus (
sha256: 4406507c1f2580ad8a52b379a9a98fd3282146c394ca6d6c7dbb1ef6e27e7e93).Validation
The canary's only failure is
quota-plan-smoke.pywithValueError: runtime_root must be absolute. The same command fails with the same stack and message on the exactorigin/mainbaseline (0233a2f3), so this PR leaves that unrelated quota behavior unchanged.Scope and safety
This changes no rule, exception, policy, scan-root, file-discovery, or result contract. The regular expressions remain the classification authority; prefilters only prove that a regex cannot match. The future-facing pass was applied by colocating each regex with its necessary literals in
LeakRule, preventing a second free-floating rule map while retaining the derived legacy pattern mapping for import compatibility.