Skip to content

perf(contract): prefilter public boundary scans - #3718

Open
hhyykk wants to merge 1 commit into
huangruiteng:mainfrom
hhyykk:codex/public-boundary-scan-perf
Open

perf(contract): prefilter public boundary scans#3718
hhyykk wants to merge 1 commit into
huangruiteng:mainfrom
hhyykk:codex/public-boundary-scan-perf

Conversation

@hhyykk

@hhyykk hhyykk commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • couple each public-boundary regex to a cheap necessary-literal prefilter
  • skip line splitting for files that cannot match any rule, then skip impossible regexes per line
  • keep the existing regexes, Lark developer-console exception, credential-reference downgrade, policy handling, and output ordering authoritative
  • add focused coverage for every regex alternative, candidate-line isolation, and all five hit categories

Performance

Seven matched runs on the package scan root used by status and quota:

Path Baseline p50 Candidate p50 Change
Boundary scanner 2.274 s 0.468 s -79.4%
status --limit 5 2.854 s 1.215 s -57.4%
quota should-run --dry-run 3.113 s 1.442 s -53.7%

Nearest-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

  • Ruff check: passed
  • focused and contract regression suite: 51 passed
  • full pytest run: 4,665 passed, 12 skipped; 26 child-launcher tests selected unsupported system Python 3.9
  • failed subset with the supported repository Python propagated to child launchers: 26 passed
  • repository hygiene smoke: passed
  • full repository contract scan: 2,942 files, zero blocking hits, seven credential references downgraded
  • LoopX standard premerge canary: direct diff/compile checks and 7 of 8 catalog smokes passed; no manual holds

The canary's only failure is quota-plan-smoke.py with ValueError: runtime_root must be absolute. The same command fails with the same stack and message on the exact origin/main baseline (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.

Signed-off-by: hyk <4408344+hhyykk@users.noreply.github.com>

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

详细中文评审

评审 exact head:4348b721357e95ed3b13b1aab1a6d5eecc8c41b8

动机

这个 PR 希望降低 scan_public_boundarystatusquota 和 premerge canary 等调用链上的扫描成本:先用廉价字面量筛出可能命中的文件与行,再运行现有正则。方向合理,而且保留 LEAK_PATTERNS 兼容映射、Lark developer-console 例外、credential-reference 降级和既有输出结构,范围集中在公共/私有边界扫描器及其测试。

改动思路

LeakRule 把权威正则与 required_literals 放在同一个不可变结构中;scan_public_boundary 先对全文 casefold() 建立候选规则,再对候选行重复筛选,最后才调用原正则。正向路径是:文件包含必要字面量 → 行包含必要字面量 → 权威正则命中 → 进入既有例外、策略和命中分类逻辑。负向路径本应只跳过“权威正则不可能命中”的文件或行。

具体改动

  • loopx/contract.py 新增 LeakRule.is_candidateLEAK_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İceAuthorİ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.

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.

2 participants