谁家PR机器人

Review notes from v0.8.12 integration

Reviewed the layered permission rulesets as part of merging into v0.8.12. One minor finding:

resolve_prefixes docstring vs implementation

The docstring says:

Higher-priority layers override lower ones; within a layer, longest prefix wins.

But the implementation just concatenates all prefixes from all layers into a flat vec:

for rs in &self.rulesets {
    trusted.extend(rs.trusted_prefixes.iter().cloned());
    denied.extend(rs.denied_prefixes.iter().cloned());
}

There's no priority-based shadowing or longest-prefix-first ordering. The check() method then calls find() which returns the first match in insertion order.

However — this doesn't produce incorrect behavior for the deny-always-wins security model. If BuiltinDefault denies "git" and User trusts "git status", the deny check runs first and catches "git" from the earlier layer. Deny correctly wins. If both layers have trusted rules, the first match passes the command either way.

The behavior is correct. The docstring is just misleading about how it works.

Recommendation

Fix the docstring to describe what actually happens, or implement the priority/shadowing logic the docstring promises. Either is fine — the current behavior is safe.

@Hmbown Thank you for the detailed review — you're exactly right on both points.

Docstring fix (choosing option 1 — update docstring to match implementation):

The current behavior is: all layers are concatenated in priority order (builtin → agent → user), then find() returns the first match. For deny rules this is correct (deny-first-match wins). For trust rules, first match also wins, which means a lower-priority trust rule can shadow a higher-priority one — but this is acceptable because deny always runs before trust in check().

I'll update the docstring to say:

Prefixes are collected in layer order (BuiltinDefault → Agent → User). The first match wins within each pass. Deny rules are checked before trust rules, so deny always wins regardless of layer.

Pushing the fix to the PR branch now.

wangfengcsu@qq.com