fix(anolisa): register bundle files individually - #2333
Conversation
github-actions
Bot
added
component:anolisa
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c85110f511
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if entry.file_name() == PYCACHE_DIR { | ||
| continue; |
There was a problem hiding this comment.
For Python hook adapters where helpers are imported from hooks/, CPython can execute valid hooks/__pycache__/*.pyc files. Since this branch skips the whole directory, changing or replacing that bytecode after enable no longer affects the recorded/current digest, so ResourceBundleMatches can stay True and the adapter remains healthy while executable hook code has drifted. Please ignore only benign cache writes in a way that still fails closed for executable cache tampering, or prevent bytecode generation instead.
Useful? React with 👍 / 👎.
| - `anolisa adapter status` no longer reports `resource bundle changed since | ||
| enable` after a bundled Python hook has run. The bundle digest now skips | ||
| the `__pycache__` directories the interpreter writes next to hook sources, | ||
| so an adapter that keeps serving stays healthy, while edits to hook | ||
| sources, manifests, and every other managed file are still detected |
There was a problem hiding this comment.
This adds a CHANGELOG entry under Unreleased for a normal fix commit, with no version-bearing files changed. The documentation standard §5 says daily feature/fix PRs should update README/user-guide only and CHANGELOG entries are reserved for release version bump PRs, so keeping this entry here will make the change fail docs review; move it to the next version-bump changelog aggregation.
AGENTS.md reference: AGENTS.md:L319-L321
Useful? React with 👍 / 👎.
kongche-jbw
left a comment
kongche-jbw
left a comment
There was a problem hiding this comment.
Review baseline: bc122a654bf3...c85110f511ff
[P1] 不要让可执行的 Python 缓存绕过 bundle 完整性校验
src/anolisa/crates/anolisa-core/src/adapter/util.rs:46 会跳过所有 __pycache__
子树。Qwen hook 仍以普通 python3 启动，并从同目录导入多个 helper。
CPython 会执行 header 与源码 mtime/size 匹配的 .pyc；因此在源码保持不变时，
替换缓存即可执行不同代码，而 digest_tree 仍返回原摘要，status 继续显示 healthy。
这个共享 helper 同时服务 Codex、Claude Code、Cosh、Qoder、Hermes 和 OpenClaw，
完整性盲区也会扩散到其他 driver。
Possible direction: 将 Python 缓存读写移出受管资源根，或把资源根设为只读，
并继续将可执行缓存纳入摘要。请补充可被 CPython 实际加载的替换 .pyc 回归测试，
确认 status 报告 drift，同时首次运行普通 hook 仍保持 healthy。
[P2] 将 CHANGELOG 条目留给版本发布 PR
src/anolisa/CHANGELOG.md:9 在普通 fix PR 中新增了 Unreleased 条目。
specs/documentation-standard.md 第 5 节要求日常 feature/fix PR 不改 CHANGELOG，
由 release version bump PR 汇总；当前修改会违反仓库的强制文档流程。
Possible direction: 本 PR 删除该条目，在下一次 anolisa 版本 bump 时统一加入。
无需新增代码测试，保留现有文档检查即可。
c85110f to
4287102
Compare
boluochoufeng
changed the title
|
@kongche-jbw 两条都核实过了，都成立，已按此重做。当前 head P1 — 确认成立，原方案的安全论证是错的我原来在注释和 PR 描述里写的是"CPython 只在源码相邻时才加载缓存，而源码仍被哈希，所以篡改仍可检出"。这个推理错了：源码相邻只是加载的前提，pyc 与源码的绑定只靠 header 里的 mtime + size 这种弱校验，两个值都可以伪造。本地按你说的路径复现了一遍： 你指出的扩散风险也成立： 改法：不再排除
参照的是 关于你要求的回归测试， 但"替换 要真正闭合，只能让 bytecode 成为已登记文件，也就是 enable 时预编译。这正是 RPM 的做法（ 如果你认为在预编译落地之前，这个已披露的缺口仍不可接受，我可以把 PR 转为 draft 等那次讨论有结论再推进——请指示。 P2 — 确认成立，已删除
我之前只看了 §2.2 的写作格式，漏了 §5 的时机规定。CHANGELOG 改动已从本 PR 移除，留给下次 anolisa 版本 bump。 顺带
|
kongche-jbw
left a comment
kongche-jbw
left a comment
There was a problem hiding this comment.
Review baseline: bc122a654bf3...42871025a8f2
[P1] 不要把所有未登记文件继续判为 bundle 匹配
src/anolisa/crates/anolisa-core/src/adapter/claim.rs:193 会收集新增文件，
但第 199–203 行只要旧文件未变就仍返回 Matched；util.rs:112–128
也只在 reason 中附注，condition 仍为 True。因此组件升级若只新增 hook 或
helper，stale_enabled_claims 会漏掉它，用户更新不会提示重新 enable。
更严重的是，触发评论已复现的可加载伪造 .pyc 也走同一路径，替换缓存后
status 仍为 Healthy；在 True 条件上显示警告不能恢复完整性门禁。
Possible direction: 让新增的可执行或包管理文件影响 verdict。可将运行时字节码
重定向到资源根之外，或用实际解释器预编译并登记。请补充两条生产链路回归测试：
仅新增文件的组件更新必须产生 stale action；CPython 实际加载伪造 .pyc 后
status 不得保持 Healthy。
Forrest-ly
left a comment
Forrest-ly
left a comment
There was a problem hiding this comment.
总体评价
PR 将整棵资源树的 digest 改为逐文件登记，确实解决了 #2252 中 hook 首次运行生成 __pycache__ 导致 adapter 被误判为 degraded 的问题，并把六个 driver 中重复的 condition 构建逻辑收敛到 util.rs，结构更干净。但当前设计把所有未登记文件都排除在 drift verdict 之外，使得可执行的 .pyc 替换或组件升级新增 hook 都能绕过完整性门禁，这是一个需要阻塞合并的安全/正确性问题。
审查结论
request-changes
详细意见
🔴 必须修改（阻塞合并）
src/anolisa/crates/anolisa-core/src/adapter/claim.rs:199-207—— 未登记文件不影响 verdict，导致完整性门禁被绕过。补充 kongche-jbw review：只要没有已登记文件被修改/删除，inspect_bundle就返回Matched，无论资源根里新增了什么。这意味着攻击者放入一个与相邻源码 mtime/size 匹配的伪造.pyc，或组件更新仅新增了可执行 hook/helper，bundle_match()都不会返回Changed，stale_enabled_claims也不会提示重新 enable。当前实现把风险 disclosure 放在 reason 里但保持 conditionTrue，对只检查 status 的调用方（如qwencode.rs:1178的summarize）等同于放行。建议：要么将运行时字节码重定向到资源根之外（如PYTHONPYCACHEPREFIX或只读资源根），要么对未登记的可执行/包文件返回Changed；在方案确定前不应合并。
🟡 建议修改（不阻塞但推荐）
src/anolisa/crates/anolisa-cli/tests/update_adapter_actions.rs:235—— update 的 stale 检测测试仍走 legacy whole-tree 路径。该测试构造的 claim 使用bundle_files: Vec::new()加bundle_digest，因此没有覆盖新 per-file registry 在claim::stale_enabled_claims/update.rs:472中的行为。建议增加（或修改）一条使用bundle_files的用例，验证：已登记文件被修改时正确识别为 stale；仅新增未登记文件时按当前设计是否识别为 stale（并标注预期行为）。src/anolisa/crates/anolisa-core/src/adapter/qwencode.rs:1568—— end-to-end 测试写入的.pyc不是合法字节码。b"\x00bytecode"不会被 CPython 加载，因此测试无法复现 kongche-jbw 提出的"伪造.pyc被解释器执行但 status 仍 Healthy"的真实攻击链路。建议用py_compile或手工写入符合 PEP-3147 的 header（magic + mtime/size）来构造真正可加载的缓存，再断言 status 行为。
🟢 值得肯定
- 逐文件登记机制正确区分了"已登记文件变更"与"未登记文件出现"，解决了
__pycache__误报。 bundle_files使用#[serde(default, skip_serializing_if = "Vec::is_empty")]且CLAIM_SCHEMA_VERSION未变，legacy receipt 会正确回退到bundle_digest，向后兼容设计合理。- 六个 driver 的
bundle_match_condition统一收敛到util.rs，消除了之前六份略有差异的副本，便于后续统一调整 verdict 策略。 inspect_bundle在资源根不可读时返回Unknown而非Changed，避免基于不完整读取做出错误判决。
4287102 to
9adc032
Compare
|
@kongche-jbw @Forrest-ly 已更新到 1. 未登记文件不影响 verdict —— 确认是本 PR 引入的倒退，已修kongche-jbw 提的第一个后果我完全漏了，而且性质比我原先以为的严重：这不是一个未闭合的遗留问题，而是本 PR 造成的行为倒退。改之前整树 digest 会因新增文件而变化 → 报 stale → 提示 re-enable；我改成"所有未登记文件都不并入 verdict"之后，这条链路直接哑了。组件升级只新增 hook/helper 就不会再提示重新 enable。这个批评完全正确。 已改为：未登记文件默认即 drift，豁免范围收窄到只有
新增测试：
2. 伪造
|
kongche-jbw
left a comment
kongche-jbw
left a comment
There was a problem hiding this comment.
Review baseline: bc122a654bf3...9adc032b0194
[P1] 不要让可加载的字节码缓存绕过完整性判定
src/anolisa/crates/anolisa-core/src/adapter/claim.rs:207 把所有
__pycache__ 成员移出 added，第 210 行随即会在其余文件未变时返回
Matched；qwencode.rs:1582 还把对应摘要固定为 Healthy。实际 hook
会从资源目录导入 Python helper，因此替换一个与源码 mtime/size 匹配的有效
.pyc 后，解释器可执行不同代码，而 condition 和 summary 仍放行。reason 中的
提示不会改变只消费 verdict 的调用方，这一轮仍未闭合此前的完整性问题。
Possible direction: 将缓存重定向到受管树外，或用实际解释器预编译并登记；补充
真实运行 CPython 的回归测试，验证首次正常生成缓存保持 healthy，而替换可加载
.pyc 后 status 不再是 Healthy。
[P2] 修正仅新增文件回归测试的登记摘要
src/anolisa/crates/anolisa-cli/tests/update_adapter_actions.rs:261 使用的
sha() 返回裸十六进制，但生产登记值在 adapter/util.rs:60 带有
sha256: 前缀。因此 fixture 中未改动的 plugin.json 在 update 前就会被判为
modified；即使生产代码完全忽略新增的 extra_hook.py，该测试也照样报告 stale
并通过，未覆盖它声明的“仅新增文件”路径。
Possible direction: 让 fixture 使用与生产一致的摘要格式，并在运行 update 前先断言
registry receipt 为 Matched，再验证新增文件触发 stale action。
A whole-tree digest can only answer "did anything change", so a bundled Python hook writing __pycache__ beside its source on first import was indistinguishable from a tampered hook: running an installed hook degraded the adapter that had just run it. Record each managed file with its own digest at enable time. Status verifies registered files individually, and an unregistered file is drift too — an upgrade that only adds a hook must still mark the receipt stale. Drift now names the files that moved instead of just asserting the bundle changed. The single exemption is a __pycache__ member, which running an installed hook creates. It is reported separately rather than folded into the verdict, and disclosed in the condition reason: a .pyc is loadable by CPython whenever its header matches the adjacent source's mtime and size, both forgeable, so this is an acknowledged hole, not a safety claim. Closing it means registering bytecode at enable time by precompiling it, which needs the component to declare its interpreter and is left to a follow-up. The exemption is deliberately narrow: a .pyc outside __pycache__ is a sourceless module CPython imports directly, so it still counts as drift. Receipts written before the registry existed carry an empty one and fall back to the whole-tree digest, keeping their previous verdict exactly. Six drivers had already grown near-identical copies of the condition builder; they now share one. Fixes: a2a58f8 ("feat(anolisa): add tokenless adapter drivers") Assisted-by: Claude Code:2.1.226 Signed-off-by: blycf <1355990831@qq.com> Assisted-by: Claude Code:2.1.226 Signed-off-by: blycf <1355990831@qq.com>
9adc032 to
5d0444a
Compare
|
@kongche-jbw 已更新到 P2 —— 确认成立，测试确实是假通过的，已修你说得对，而且这条抓得很准。fixture 的 已按你的建议修：
并且做了一次反向验证，确认这次测的是真东西 —— 把生产逻辑临时改回"忽略新增文件"： 修之前，这个反向验证是通不过的（测试照样 PASS）。 P1 —— 我不再争辩，按你们的判断处理这一轮你重申了同一条，我理解为对我上轮 (a)/(b)/(c) 的回答是"不接受带缺口合入"。我认同这个判断： 你要求的"真实运行 CPython 的回归测试"，在缺口闭合之后是写得出来的（预编译登记后，替换 按 (b) 推进：本 PR 转 draft，等 sec-core 侧就"把字节码缓存移出受管树（ 本轮已修的两块（未登记文件默认即 drift、逐文件登记与精确报告）不会丢，会作为闭合方案的基础保留。 |
kongche-jbw
left a comment
kongche-jbw
left a comment
There was a problem hiding this comment.
Review baseline: e28b7ccb95a9...5d0444a12f1a
[P1] 不要让可加载的字节码缓存继续绕过完整性判定
src/anolisa/crates/anolisa-core/src/adapter/claim.rs:203-211 仍把所有新出现的
__pycache__ 文件移出 added，并在其余登记文件未变时返回 Matched。
因此启用后先生成正常缓存，再替换为与源码 mtime/size 匹配、可被 CPython
加载的 .pyc，ResourceBundleMatches 仍为 True，Qwen summary 仍为
Healthy；reason 中的披露不会约束只消费 verdict 的调用方。当前测试也明确
不执行写入的 payload，因而没有覆盖这条攻击链。
Possible direction: 将缓存重定向到受管树外，或用实际解释器预编译并登记；
补充真实运行 CPython 的回归测试，验证正常生成缓存保持 healthy，而替换可加载
.pyc 后 status 报告 drift。
|
补充一些关于 P1 的材料。 先说我认的：kongche-jbw 第二轮提的组件升级只新增文件不再报 stale，是本 PR 引入的正确性倒退，与安全无关，已修并补了生产链路回归测试。下面只谈 P1 的完整性部分。 1.
|
| 模式 | resource root | digest 记录（installed.toml） |
|---|---|---|
| user | ~/.local/share/anolisa/adapters/… |
~/.local/state/anolisa/… |
| system | {prefix}/share/anolisa/…（root） |
{prefix}/var/lib/anolisa/…（root） |
两种模式下，能写 __pycache__ 的主体，同样能写记录 digest 的那个文件。攻击者替换完 .pyc，把 installed.toml 里的 digest 改成新值即可，status 照样 Healthy。
所以即使把 pyc 缺口闭合，也只是把绕过方式从"改一个文件"变成"改两个文件"。要达到防篡改，digest 需要存放在攻击者够不到的地方（签名，或 root-owned 记录配非 root 的 resource root），那是另一个量级的设计。
2. 这个字段的设计意图
代码自身的表述是 for drift/upgrade detection；stale_enabled_claims 的唯一消费者是 anolisa update，用途是提示"该 re-enable 了"。它回答的是"资源还是不是我 enable 时那份"。
3. rpm -V 模型的另一半
rpm -V本身不是防篡改机制，rpmdb 同样 root 可写；- RPM 对包内文件是分角色声明的：
%ghost用于"包知道路径但内容不由包提供"的运行时生成物（不校验内容），%config用于用户可改的配置，%verify(not size filedigest mtime)可逐属性关闭校验，用来避免"安装后被合法修改"的 false positive。
运行时生成的 .pyc 属于 %ghost 那一类。
4. 关于严格化方向
按"未登记文件一律算 drift"走：今天 bundle 里只有源码和 manifest，看起来没问题；将来 adapter 若需要放用户可编辑的配置文件，用户改一次就会 degraded。让文件角色变成声明式的（对标 %ghost / %config）比在 anolisa 里硬编码 __pycache__ 这类判断更可持续。
如果仍认为需要在本 PR 内闭合 pyc 这一环，我可以实现 enable 时预编译并登记：不需要改 sec-core，解释器可以直接从 qwen-extension.json 的 hook 命令读到（raw 打包后是 agent-sec-python，RPM 下是 python3），与运行时天然一致；用 --invalidation-mode checked-hash 编译可避开 mtime 失效（实测 timestamp 模式下组件 update 重铺源码后 pyc 会被重写，checked-hash 不会）。
|
@kongche-jbw 请重新 review。当前 head |
kongche-jbw
left a comment
kongche-jbw
left a comment
There was a problem hiding this comment.
Review baseline: e28b7ccb95a9...5d0444a12f1a
[P1] 即使只作为 drift 信号，也不能把已变化的可执行代码判为匹配
src/anolisa/crates/anolisa-core/src/adapter/claim.rs:203-211 会把所有新出现的
__pycache__ 成员移出 added，随后在其他文件未变时返回 Matched。正常生成
缓存后，只需替换为与源码 mtime/size 匹配且可加载的 .pyc，CPython 就会执行
不同代码，但 ResourceBundleMatches 仍为 True，Qwen summary 仍为
Healthy，update 也不会提示 re-enable。即使把摘要只视为运维信号，这仍是
一次未修改 receipt 即可复现的资源 drift；同权限主体还能改 receipt，并不能让
这个错误分类消失。qwencode.rs:1574-1592 当前只写入不执行的 opaque payload，
并把该路径固定为 Healthy，所以没有覆盖真实加载链路。
Possible direction: 将缓存重定向到受管树外，或用实际解释器预编译并登记；补充
真实运行 CPython 的回归测试，验证正常缓存保持 healthy，而替换可加载 .pyc
后 condition 报告 drift，summary 不再是 Healthy。
3 participants
Why
Enabling an adapter records one SHA-256 digest of the whole resource root, and
statusre-digests that tree. A whole-tree digest can only answer "didanything change", so two very different events collapse into the same verdict:
hooks/__pycache__/*.pycbeside its source onfirst import — the adapter working
The first one is what #2252 reports: running an installed hook degrades the
adapter that just ran it, while Qwen Code still shows the extension enabled and
the scanners keep producing audit events.
What changed
Enable now records each managed file with its own digest
(
AdapterClaim::bundle_files), and status verifies them individually:This mirrors
rpm -V, which verifies package-owned files and leaves unownedpaths alone. Drift reasons now name the files that moved instead of only
asserting that the bundle changed.
Six drivers had grown near-identical copies of the condition builder; they now
share one in
util.rs.Related issue
closes #2252
User / Agent impact
A Qwen Code adapter stays
healthyafter its hooks run instead of flipping todegradedon the first invocation.anolisa updatealso stops listing theadapter as stale, since that path goes through
claim.bundle_match()too.Modifying a hook source, manifest, or any other registered file still reports
resource_bundle_matches: falseand degrades — now with the offending paths inthe reason.
Risk and compatibility
Receipts written before this field existed carry an empty registry and fall
back to the whole-tree
bundle_digest, keeping their previous verdict exactly— pinned by
legacy_receipts_without_a_registry_keep_whole_tree_comparison.bundle_filesis#[serde(default, skip_serializing_if = "Vec::is_empty")], soCLAIM_SCHEMA_VERSIONdoes not move and older readers ignore it.Known limitation, deliberately not closed here. An unregistered file is
unverified, not vetted. A
.pycis loadable by CPython whenever its headermatches the adjacent source's mtime and size — both forgeable — so an attacker
who can write into the resource root can still swap bytecode without touching
any registered file. This PR therefore discloses unregistered files in the
condition reason rather than implying coverage it does not have. Closing the
gap means making bytecode a registered file by precompiling it at enable time,
which requires the component to declare the interpreter its hooks run under
(raw ships a bundled
agent-sec-python, RPM uses the systempython3) — thatneeds an agreement with the sec-core owners and is tracked as follow-up work.
Validation
cargo test --locked --workspace --no-fail-fast,cargo clippy --workspace --all-targets --locked,cargo fmt --all --check. One pre-existing failure isunrelated and reproduces on the base commit
(
raw_update_rollback_hydrates_legacy_required_capability,read central log: NotFound).New coverage:
claim::inspect_bundle_separates_modified_from_unregistered— a.pycappearing keeps the verdict
Matchedand must show up inunregistered;editing a registered hook flips to
Changednaming that exact file.claim::inspect_bundle_treats_a_vanished_registered_file_as_drift— deletionis drift; an unreadable root stays
Unknown.claim::legacy_receipts_without_a_registry_keep_whole_tree_comparison.util::hash_bundle_files_registers_each_file_separately.qwencode::status_stays_healthy_after_a_hook_writes_bytecode— end to end:enable → healthy → write
hooks/__pycache__/pii_text.cpython-311.pyc→ stillhealthy, with the cache disclosed in the reason → tamper with a hook →
condition
Falsenaming it, summaryDegraded.Documentation and rollback
No documentation change: per
specs/documentation-standard.md§5 the CHANGELOGentry belongs to the next anolisa version bump PR, not here. Rollback is
reverting the commit — receipts carrying a registry simply stop consulting it
and fall back to
bundle_digest, which is still written.