Skip to content

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

Merged
huangruiteng merged 2 commits into
loopx-project:mainfrom
hhyykk:codex/public-boundary-scan-perf
Sep 25, 2026
Merged

huangruiteng merged 2 commits into
loopx-project:mainfrom
hhyykk:codex/public-boundary-scan-perf

Conversation

@hhyykk

@hhyykk hhyykk commented Aug 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Speed up the public/private boundary scan used by status, quota, and premerge checks. Each existing authoritative regex has a necessary-literal prefilter at file and line scope; the regex still decides classification, exceptions, policy, and output order. This branch rebases the contributor's optimization onto current main and fixes the Unicode false negatives identified in the previous review.

Python re.IGNORECASE matches İ, ı, ſ, and K with ASCII letters. The prefilter now handles those characters consistently with the regex, including the former larkoffİce and Authorİzation: misses. End-to-end scanner tests cover all four special characters.

Validation and performance

  • 47 focused scanner, lockfile, and credential tests passed; Ruff, diff check, and the real repository validate_public_private_boundary() check passed. Standard loopx canary premerge --from-git-diff --goal-id loopx-meta passed all 9 selected checks with no manual holds; the exact-diff quality receipt is valid.
  • Same-workload synthetic corpus (100 files × 801 lines, warm median): ASCII scan 0.334 s on current main versus 0.0755 s here (~77% faster); Chinese scan 0.1103 s versus 0.0638 s (~42% faster). These figures are for the scanner workload, not an end-to-end status/quota claim.
  • The complete repository-hygiene smoke still fails on both unchanged main and this branch because the release timeline lacks a v1.2.0 version entry; it is unrelated to this scanner change.

Future-facing pass: each regex and its necessary literals stay in the same typed rule; no second classifier or policy is introduced.

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

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_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.

@now-ing

now-ing commented Sep 6, 2026

Copy link
Copy Markdown
Collaborator

Verified the safety argument rule by rule — the required-literals are genuine necessary conditions for each authoritative pattern (token='s lookbehind doesn't affect literal necessity; the folded /users/ literal covers /Users/ under re.I; the private_doc_url developer-console substitution only removes hits, so the pre-filter's candidate set stays a superset), and test_every_authoritative_leak_pattern_has_a_matching_prefilter pinning that invariant structurally is exactly the right guard. The two-level filtering (file-level skip before the line loop, line-level skip inside it) keeps the behavior identical for any text the regexes would have skipped anyway.

Two small observations, neither blocking:

  • "10." as a required literal for the private-IP rule will match version strings and decimals in ordinary code text fairly often, so that particular rule may see limited pre-filter benefit in practice (correctness unaffected — purely a performance-expectation note).
  • The literals are matched against casefold() output while the authoritative patterns run with re.I. For the current ASCII-only pattern set these fold identically, but if a future rule ever carries non-ASCII letters (Turkish dotted-I style casefold expansions), the literal's folded form needs to agree with what re.I considers equal — maybe worth one line in the LeakRule docstring so the invariant survives future edits.

@huangruiteng
huangruiteng force-pushed the codex/public-boundary-scan-perf branch from 4348b72 to fe9fe95 Compare September 25, 2026 14:12
@huangruiteng

huangruiteng commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

Rebased and updated the contributor fork branch to exact head 43d91b5fa79d2735f646d353727e6fc8f968cd06 (base 3e443ad7c285973c40be883293970be6c0c51e06). This addresses the previous Unicode false-negative review: the prefilter is a necessary condition for the authoritative regex even for Python re.I's İ, ı, ſ, and K mappings. End-to-end tests cover the prior private-URL and authorization-header misses. The current-main package-lock boundary check remains intact.

Validation: 47 focused tests, Ruff, diff check, real repository boundary scan, and standard premerge canary (9/9 selected checks, no manual holds) passed; exact-diff quality receipt is valid. On the same 100 × 801-line synthetic corpus, warm median scanner time fell from 0.334 to 0.0755 s for ASCII and 0.1103 to 0.0638 s for Chinese text. The full repository-hygiene smoke still fails identically on unchanged main and this branch because its release timeline lacks v1.2.0; this does not affect the focused boundary checks.

This is ready for a new exact-head maintainer review. No merge performed.

hhyykk and others added 2 commits September 25, 2026 22:30
Signed-off-by: hyk <4408344+hhyykk@users.noreply.github.com>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
@huangruiteng
huangruiteng force-pushed the codex/public-boundary-scan-perf branch from fe9fe95 to 43d91b5 Compare September 25, 2026 14:33
@huangruiteng

Copy link
Copy Markdown
Collaborator

@hhyykk I owe you an apology. I rebased and force-updated your fork branch with a substantial follow-up revision without discussing it with you first. That may have disrupted your local work, and I should have coordinated before changing a contributor-owned branch.

Your necessary-literal prefilter and its performance goal remain the heart of this PR. I added the Unicode parity correction and regression checks raised in review, while preserving the original optimization. Thank you for identifying and implementing this valuable speedup. I'm sorry for the intrusive branch update; if it caused any trouble, I'll help put it right.

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

English verdict: APPROVE — exact head 43d91b5fa79d2735f646d353727e6fc8f968cd06 keeps the regex as the sole classifier, the prefilter is a proven necessary condition over all Unicode code points, and base/head scans are identical on the repository and on a planted-leak corpus while the scan is much faster.

动机

scan_public_boundary 位于 status、quota 和 premerge 的调用链上,每次都要对全部文件的每一行跑五条正则。这个 PR 为每条权威正则配上必要字面量,先在文件级、再在行级做廉价预筛,最终仍由原正则决定命中、例外和策略。上一轮 P1(İ 在 casefold() 下展开,导致含 İ 的私有文档域名和授权头样例漏报)已经修复。

改动思路

LeakRule 把正则和 required_literals 放在同一个不可变结构中,LEAK_PATTERNS 作为兼容映射从中派生,因此只有一份分类知识。_prefilter_fold 对 ASCII 文本走 lower() 快速路径,对非 ASCII 文本先把 İ/ı 映射成 i 再 casefold();ſ 与 K 由 casefold() 本身覆盖。文件级 continue 只跳过行循环,而行循环是每个文件处理的最后一步,所以不会跳过其他逻辑。

具体改动

关键代码讲解

  • LeakRule.is_candidate / _prefilter_fold(loopx/contract.py):必要条件判断,只在折叠后的文本里查找字面量。
  • scan_public_boundary:先按文件筛出候选规则,再按行筛选;private_doc_url 的 developer-console 替换仍然作用于正则输入,而预筛看的是原始行,候选集合只会更大。

我做的独立验证(不依赖 PR 自带测试):

  1. 穷举全部 Unicode 码点:凡是在 re.I 下能匹配某个字面量字符的码点,_prefilter_fold 之后都包含该字符。违反数为 0。另外,把 İ ı ſ K ß fi 混入字面量内部和两侧的随机测试也没有漏报。
  2. 对同一棵仓库树(3203 个文件),分别用 base 3e443ad 和 head 调用 scan_public_boundary:输出哈希完全一致;耗时从 40.6 s 降到 3.2 s(base 先跑,部分差异可能来自冷缓存)。
  3. 植入 17 个泄漏样本(包括 Unicode 大小写变体、凭据引用等):base 与 head 的命中列表逐字节一致,都是 14 个命中和 1 个凭据引用。

PR 自带的 scanner/contract 测试 48 passed。

对主干的风险

没有阻塞问题。剩余风险:今后新增的规则如果漏写或写错 required_literals,会静默漏报。test_every_authoritative_leak_pattern_has_a_matching_prefilter 只检查每条规则都有字面量,不能证明它们是真正的必要条件。一个不阻塞的建议是把上面的码点 oracle 或随机测试沉淀成测试。另外 "10." 这类字面量在普通文本中很常见,对应规则的预筛收益会有限,但不影响正确性。

CI 中的红色检查都与 base 相同(registry_io_census ×2 分布在不同 shard、stage2c (installed 0)、Release Artifacts / build),与本 PR 无关,但仍然构成单独的 merge 阻碍。

我的整体评价

APPROVE。这是一个范围克制、收益明确的性能改动,分类权威保持唯一,上一轮的安全漏报已修复并有端到端回归测试。合并仍需等待 main 上那些基线红色检查被处理。本 review 不代表我已执行合并。

@huangruiteng
huangruiteng merged commit 469574e into loopx-project:main Sep 25, 2026
19 of 26 checks passed
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