Changed: Move permission enforcement into path resolver layer#101
Changed: Move permission enforcement into path resolver layer#101
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #101 +/- ##
==========================================
+ Coverage 81.38% 81.47% +0.09%
==========================================
Files 108 109 +1
Lines 4469 4487 +18
==========================================
+ Hits 3637 3656 +19
+ Misses 832 831 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a new path resolution subsystem and integrates it into agent build/runtime: introduces a Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/llm-coding-tools-agents/src/path/resolver.rs`:
- Around line 102-107: The match arm for PermissionRule::Action(_) currently
treats both Allow and Deny the same; update the match on PermissionRule::Action
to explicitly handle PermissionRule::Action(Action::Deny) by short-circuiting to
the deny behavior (e.g., return the appropriate denied variant or error) instead
of constructing an AllowedPathResolver via AllowedPathResolver::new and
returning FileToolResolver::Allowed; leave PermissionRule::Action(Action::Allow)
to create the AllowedPathResolver and return FileToolResolver::Allowed as
before. Ensure you reference PermissionRule::Action, Action::Deny,
AllowedPathResolver::new, and FileToolResolver::Allowed when making the change
so the deny case cannot fall through to an allow.
- Around line 109-123: The prefix-globstar optimisation
(try_prefix_globstar_optimisation -> FileToolResolver::Allowed) must not be
applied for traversal-style tools (e.g., glob/grep) because it creates an
AllowedPathResolver that rejects searches from workspace root; instead those
tools should fall through to AllowedGlobResolver which preserves workspace-root
searching and filters per-entry via is_path_allowed(). Add a boolean flag (e.g.,
is_traversal_tool or tool_supports_traversal) to the resolver creation context
or function signature and change the branch so that
try_prefix_globstar_optimisation is only attempted when !is_traversal_tool; when
is_traversal_tool is true skip that optimisation and construct
AllowedGlobResolver::new(...).with_policy(...). Ensure callers are updated to
pass the correct flag so resolve()/is_path_allowed() behavior for traversal
tools remains unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ecba9236-c72f-4e52-a62d-5771043c13a2
⛔ Files ignored due to path filters (1)
src/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (28)
src/llm-coding-tools-agents/Cargo.tomlsrc/llm-coding-tools-agents/src/lib.rssrc/llm-coding-tools-agents/src/path/mod.rssrc/llm-coding-tools-agents/src/path/resolver.rssrc/llm-coding-tools-core/src/path/absolute.rssrc/llm-coding-tools-core/src/path/allowed.rssrc/llm-coding-tools-core/src/path/allowed_glob/mod.rssrc/llm-coding-tools-core/src/path/allowed_glob/normalize.rssrc/llm-coding-tools-core/src/path/allowed_glob/policy.rssrc/llm-coding-tools-core/src/path/mod.rssrc/llm-coding-tools-core/src/tools/edit.rssrc/llm-coding-tools-core/src/tools/glob.rssrc/llm-coding-tools-core/src/tools/grep.rssrc/llm-coding-tools-core/src/tools/read.rssrc/llm-coding-tools-core/src/tools/write.rssrc/llm-coding-tools-serdesai/Cargo.tomlsrc/llm-coding-tools-serdesai/README.mdsrc/llm-coding-tools-serdesai/examples/serdesai-agents.rssrc/llm-coding-tools-serdesai/examples/serdesai-task.rssrc/llm-coding-tools-serdesai/src/agent_ext.rssrc/llm-coding-tools-serdesai/src/agent_runtime/build.rssrc/llm-coding-tools-serdesai/src/agent_runtime/task.rssrc/llm-coding-tools-serdesai/src/task/handle.rssrc/llm-coding-tools-serdesai/src/tools/edit.rssrc/llm-coding-tools-serdesai/src/tools/glob.rssrc/llm-coding-tools-serdesai/src/tools/grep.rssrc/llm-coding-tools-serdesai/src/tools/read.rssrc/llm-coding-tools-serdesai/src/tools/write.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Semver Checks (Serdesai Full+Linux)
- GitHub Check: Semver Checks (Core Blocking+Linux)
- GitHub Check: Semver Checks (Core Blocking)
- GitHub Check: Semver Checks (Serdesai Full)
- GitHub Check: Async macOS
- GitHub Check: Blocking macOS
- GitHub Check: Async Linux
- GitHub Check: Async Windows
- GitHub Check: Blocking Windows
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Sewer56
Repo: Sewer56/llm-coding-tools PR: 69
File: src/llm-coding-tools-bubblewrap/src/profile/validation.rs:57-67
Timestamp: 2026-03-28T02:14:04.465Z
Learning: In `src/llm-coding-tools-bubblewrap/src/profile/` (Rust, llm-coding-tools-bubblewrap crate), the `Builder` API paths (workspace, synthetic_home, cache_root, mount lists, overlays, etc.) are always set by trusted application/operator code — the library consumer is the trusted party. Path normalization and `..`-component hardening in validators like `validate_absolute_path` is therefore NOT required to defend against traversal attacks. Untrusted input (LLM-generated shell commands) only enters through `wrap_command`/`execute_command_with_mode`, not through the `Builder`.
📚 Learning: 2026-03-28T02:14:04.465Z
Learnt from: Sewer56
Repo: Sewer56/llm-coding-tools PR: 69
File: src/llm-coding-tools-bubblewrap/src/profile/validation.rs:57-67
Timestamp: 2026-03-28T02:14:04.465Z
Learning: In `src/llm-coding-tools-bubblewrap/src/profile/` (Rust, llm-coding-tools-bubblewrap crate), the `Builder` API paths (workspace, synthetic_home, cache_root, mount lists, overlays, etc.) are always set by trusted application/operator code — the library consumer is the trusted party. Path normalization and `..`-component hardening in validators like `validate_absolute_path` is therefore NOT required to defend against traversal attacks. Untrusted input (LLM-generated shell commands) only enters through `wrap_command`/`execute_command_with_mode`, not through the `Builder`.
Applied to files:
src/llm-coding-tools-core/src/path/allowed_glob/policy.rssrc/llm-coding-tools-agents/src/lib.rssrc/llm-coding-tools-serdesai/examples/serdesai-task.rssrc/llm-coding-tools-serdesai/src/task/handle.rssrc/llm-coding-tools-serdesai/examples/serdesai-agents.rssrc/llm-coding-tools-core/src/path/allowed_glob/normalize.rssrc/llm-coding-tools-serdesai/src/tools/glob.rssrc/llm-coding-tools-core/src/path/absolute.rssrc/llm-coding-tools-serdesai/src/tools/write.rssrc/llm-coding-tools-core/src/path/mod.rssrc/llm-coding-tools-core/src/path/allowed.rssrc/llm-coding-tools-serdesai/src/agent_runtime/task.rssrc/llm-coding-tools-core/src/path/allowed_glob/mod.rssrc/llm-coding-tools-agents/src/path/resolver.rssrc/llm-coding-tools-serdesai/src/agent_runtime/build.rs
📚 Learning: 2026-04-08T21:35:35.581Z
Learnt from: CR
Repo: Sewer56/llm-coding-tools PR: 0
File: src/AGENTS.md:0-0
Timestamp: 2026-04-08T21:35:35.581Z
Learning: After changes, run `.cargo/verify.{sh,ps1}` before returning
Applied to files:
src/llm-coding-tools-serdesai/src/agent_runtime/build.rs
🔇 Additional comments (31)
src/llm-coding-tools-agents/Cargo.toml (1)
38-38: Looks good — dev dependency is scoped correctly.Adding
soft-canonicalizeunderdev-dependenciesis an appropriate placement for test/benchmark-only usage.src/llm-coding-tools-core/src/path/allowed_glob/normalize.rs (1)
41-43: Public export is clean and behavior-preserving.The visibility change for
expand_shellkeeps the same error contract while enabling reuse from higher layers.src/llm-coding-tools-serdesai/src/agent_ext.rs (1)
89-114: Nice simplification of error mapping.
ToolResultExt::with_toolcleanly centralizes conversion intoAgentBuildError::ToolSettingsValidationand removes repetitive call-site boilerplate.src/llm-coding-tools-agents/src/lib.rs (1)
7-15: Module/export wiring is solid.The new
pathmodule plus top-level re-export provides a clean public entrypoint for resolver construction.src/llm-coding-tools-core/src/path/allowed_glob/policy.rs (1)
127-127: Visibility change is appropriate for the refactor.Exposing
GlobPolicy::is_allowedsupports fast resolver-layer permission checks while preserving existing rule-evaluation behavior.src/llm-coding-tools-serdesai/Cargo.toml (1)
87-88: Dependency addition makes sense.Promoting
indexmapto main dependencies aligns with runtime/build usage instead of test-only scope.src/llm-coding-tools-serdesai/README.md (1)
122-122: Doc update is aligned with the new build context contract.Good catch updating the example to include workspace-root wiring in
AgentBuildContext::new(...).src/llm-coding-tools-agents/src/path/mod.rs (1)
1-8: Clean module boundary and re-export surface.This is a clear, minimal path-module entrypoint for resolver construction APIs.
src/llm-coding-tools-serdesai/examples/serdesai-agents.rs (1)
13-13: Workspace root wiring is correct and aligned with the new build-context API.This keeps the example in sync with the
AgentBuildContext::new(..., workspace_root)contract.Also applies to: 46-51
src/llm-coding-tools-serdesai/examples/serdesai-task.rs (1)
12-12: Good API migration in the task example.Passing
workspace_rootintoAgentBuildContext::newis the right update for this PR’s resolver flow.Also applies to: 55-60
src/llm-coding-tools-serdesai/src/task/handle.rs (1)
239-247: Test context update is correct.Adding
workspace_roottoTaskBuildContext::new_for_test(...)matches the updated constructor and keeps tests aligned with runtime behavior.src/llm-coding-tools-core/src/path/absolute.rs (2)
29-35: Trait migration forAbsolutePathResolveris implemented cleanly.
path_mode()+is_path_allowed()integration matches the newPathResolvercontract.
104-117: Nice coverage for the new fast-path permission check.The added test makes the resolver’s
is_path_allowedbehavior explicit.src/llm-coding-tools-serdesai/src/tools/glob.rs (1)
53-53: Good switch to instance-derived path mode.Using
resolver.path_mode()is the right fit for resolver instances chosen at runtime.src/llm-coding-tools-serdesai/src/tools/write.rs (1)
48-50:WriteToolis correctly aligned with resolver-instance semantics.Docs and implementation now match the resolver-driven path mode design.
src/llm-coding-tools-core/src/path/allowed.rs (2)
128-134:AllowedPathResolvertrait update looks correct.The
is_path_allowed/path_modeadditions are coherent with resolver-based permission checks.
353-371: Good targeted test foris_path_allowedconsistency.This test meaningfully validates the new fast-check behavior against inside/outside paths.
src/llm-coding-tools-serdesai/src/tools/edit.rs (1)
48-50:EditToolmigration is clean and consistent.Deriving
path_modefrom the resolver instance is the right update for this refactor.src/llm-coding-tools-core/src/tools/read.rs (1)
196-240: LGTM - Permission enforcement correctly delegated to resolver.The documentation update on line 196 and the simplified comment on line 239 accurately reflect that permission checks are now handled by the resolver's
resolve()method rather than by explicit post-resolution checks in the tool.src/llm-coding-tools-serdesai/src/tools/read.rs (1)
67-75: LGTM - Correct migration from associated constant to instance method.The change from
R::PATH_MODEtoresolver.path_mode()correctly implements the newPathResolvertrait API. This enablesFileToolResolver(the enum wrapper) to dynamically return the path mode based on its inner variant, as shown in the relevant code snippet fromsrc/llm-coding-tools-agents/src/path/resolver.rs:58-63.src/llm-coding-tools-core/src/path/mod.rs (2)
31-43: LGTM - Well-designed trait extension for efficient path filtering.The new
is_path_allowedmethod enables efficient per-entry filtering during directory walks (grep/glob) without the overhead of fullresolve()calls. The documentation clearly establishes the consistency contract between the two methods.
10-15: Downstream usage of newly public APIs is verified and properly documented.The
expand_shellfunction has clear documentation explaining its purpose (expands shell patterns like~/,$HOME/, and${VAR:-default}), return type (PathBuf), and error handling (returnsToolError::InvalidPath). All re-exported items—AllowedGlobResolver,GlobPolicy,GlobPolicyBuilder, andRuleAction—are actively used in the agents crate (src/llm-coding-tools-agents/src/path/resolver.rs), confirming these are stable APIs ready for consumption.src/llm-coding-tools-core/src/tools/grep.rs (2)
332-335: LGTM - Clean delegation of entry filtering to resolver.The filtering logic is now a simple
resolver.is_path_allowed(entry_path)call, centralizing permission enforcement in the resolver layer as intended by this PR.
784-830: Good test coverage for resolver-based filtering.The
grep_filters_via_is_path_allowedtest properly verifies that:
- Files matching the policy (
src/lib.rs) appear in results- Files outside the policy (
Cargo.toml) are filtered out- Both relative (
.) and absolute path inputs work correctlysrc/llm-coding-tools-core/src/tools/edit.rs (1)
80-91: Consider documenting the reserved struct purpose.
EditSettingsis now an empty struct reserved for future use. The current docstring "Reserved for future use" is brief but adequate. The_settingsparameter inedit_filemaintains API consistency with other tools.src/llm-coding-tools-serdesai/src/tools/grep.rs (1)
62-71: LGTM - Consistent with ReadTool refactor.The change from
R::PATH_MODEtoresolver.path_mode()mirrors the same pattern applied toReadTooland correctly implements the updatedPathResolvertrait API.src/llm-coding-tools-serdesai/src/agent_runtime/task.rs (2)
29-46: LGTM - Clean workspace_root plumbing through build context.The
workspace_root: Arc<Path>field is properly threaded throughAgentBuildContext→TaskBuildContext→attach_standard_tools. UsingArc<Path>is appropriate for shared ownership without cloning the path data.
136-142: LGTM - Correct workspace_root propagation to tool construction.The
build_agentfunction correctly passes&context.workspace_roottoattach_standard_tools, matching the signature shown in the context snippet frombuild.rs:149-157.src/llm-coding-tools-core/src/path/allowed_glob/mod.rs (3)
157-175: LGTM - Correct implementation of PathResolver trait methods.The
is_path_allowedimplementation properly:
- Checks containment via
starts_with(workspace_root)- Normalizes the path before policy evaluation
- Delegates to
GlobPolicy::is_allowedwhen configuredThe
path_mode()correctly returnsPathMode::Allowedfor this workspace-rooted resolver.
253-264: Note the special-case handling for workspace root.The condition
resolved.as_path() == resolver.workspace_root.as_ref()allows the workspace root itself to be accepted even whenis_path_allowedmight returnfalse(e.g., if policy patterns likesrc/**don't match the root). This is a sensible edge case to handle explicitly.
221-242: LGTM - Clean refactor of resolution tiers.The
resolve_candidatefunction now takes the resolver reference, allowingvalidate_resolvedto useresolver.is_path_allowed()as the single source of truth for policy checks. The three-tier resolution strategy (canonicalize → fast path → soft canonicalize) remains unchanged.
- Add `is_path_allowed` and `path_mode` methods to `PathResolver` trait - Introduce `FileToolResolver` enum and `build_resolver_for_tool` to select cheapest resolver per tool based on permission config - Remove inline permission checks from edit/glob/grep/read/write tools, delegating to the resolver instead - Simplify tool settings structs by removing permission fields - Make `allowed_glob::normalize`, `allowed_glob::policy`, and `expand_shell` public for use from agents crate
Previously, the match arm for PermissionRule::Action(_) treated both Allow and Deny identically by creating an AllowedPathResolver. Now Action(Deny) short-circuits to return ToolError::PermissionDenied, ensuring denied tools cannot accidentally fall through to allowed behavior.
AllowedPathResolver's resolve(".") returns the first allowed subdirectory
instead of the workspace root, causing traversal tools to walk from the
wrong directory and miss files outside it. Gate the optimisation behind
an is_traversal_tool flag so glob/grep fall through to AllowedGlobResolver.
Sewer56
force-pushed
the
fix-file-tools-path-checks
branch
from
200f4d0 to
115d41a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/llm-coding-tools-serdesai/src/agent_runtime/build.rs (1)
55-63:⚠️ Potential issue | 🟡 MinorSplit resolver failures from tool-settings validation.
build_resolver_for_tool(...)is validatingprepared.permission_config, notprepared.tool_settings. If a path rule is malformed, this now surfaces asAgentBuildError::ToolSettingsValidation, which points callers at the wrong config block. Please give resolver construction its own error path; the same fix applies to the read/write/edit/glob/grep branches.🧭 Suggested shape
pub enum AgentBuildError { /// Tool settings validation failed during agent build. #[error("invalid settings for tool `{tool}`: {source}")] ToolSettingsValidation { tool: &'static str, #[source] source: llm_coding_tools_core::ToolError, }, + /// File-tool permission config failed during resolver construction. + #[error("invalid permission config for tool `{tool}`: {source}")] + ToolPermissionValidation { + tool: &'static str, + #[source] + source: llm_coding_tools_core::ToolError, + }, }let resolver = build_resolver_for_tool( permission_config, read_meta::NAME, workspace_root, false, ) -.with_tool(read_meta::NAME)?; +.map_err(|source| AgentBuildError::ToolPermissionValidation { + tool: read_meta::NAME, + source, +})?;Also applies to: 176-233
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/llm-coding-tools-serdesai/src/agent_runtime/build.rs` around lines 55 - 63, The error variant ToolSettingsValidation is being used incorrectly when build_resolver_for_tool(...) fails validating prepared.permission_config; add a dedicated error variant (e.g., ResolverConstruction or PermissionResolverValidation) to AgentBuildError and return that from build_resolver_for_tool instead of ToolSettingsValidation; update all places that call build_resolver_for_tool (including the read/write/edit/glob/grep/grep branches and the other similar branches referenced around the 176-233 area) to map resolver construction failures to the new variant, and adjust any #[error(...)] messages to include the tool name and the underlying llm_coding_tools_core::ToolError (or source) to preserve original error context.
🧹 Nitpick comments (2)
src/llm-coding-tools-core/src/tools/edit.rs (1)
80-91: Make the placeholder settings type forward-compatible.
EditSettings {}is public and externally constructible, so the first real field becomes a breaking change even though the doc says this is “reserved for future use”. I’d mark this#[non_exhaustive](same forWriteSettings) if the goal is to grow the API later without another semver break.♻️ Suggested shape
+#[non_exhaustive] #[derive(Debug, Clone, Default, PartialEq, Eq)] -pub struct EditSettings {} +pub struct EditSettings; impl EditSettings { /// Creates default edit settings. #[must_use] pub fn new() -> Self { - Self {} + Self } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/llm-coding-tools-core/src/tools/edit.rs` around lines 80 - 91, The empty public struct EditSettings (and similarly WriteSettings) is externally constructible which makes adding fields a breaking change; mark both structs with #[non_exhaustive] so callers cannot construct them by struct literal (allowing you to add fields later), keep the existing pub fn new() constructors and Default/derives as-is so external code can still create instances via the provided APIs, and update the struct declarations for EditSettings and WriteSettings to include the #[non_exhaustive] attribute.src/llm-coding-tools-serdesai/src/agent_runtime/build.rs (1)
345-355: Passworkspace_rootinto the test helper.Hard-coding
resolve_workspace_root()insidebuild_with_mockmakes these tests depend on the ambient checkout layout and gives you no way to use a temp root when adding allow/deny cases for the new resolver wiring. Taking the path as a parameter keeps the helper hermetic and makes the permission-path tests much easier to write.♻️ Suggested shape
fn build_with_mock( prepared: &super::PreparedBuild<'_>, name: &str, + workspace_root: &std::path::Path, ) -> serdes_ai::Agent<(), String> { - let workspace_root = - llm_coding_tools_core::resolve_workspace_root().expect("workspace root"); let (builder, prompt_builder) = attach_standard_tools::<CredentialResolver>( AgentBuilder::<(), String>::new(MockModel::new(name)), prepared, None, - &workspace_root, + workspace_root, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/llm-coding-tools-serdesai/src/agent_runtime/build.rs` around lines 345 - 355, The helper currently calls llm_coding_tools_core::resolve_workspace_root() directly, making build_with_mock depend on the repo layout; change the signature of the helper (the function that takes prepared: &super::PreparedBuild<'_>, name: &str) to accept a workspace_root parameter (e.g., workspace_root: &Path or &str), remove the internal resolve_workspace_root() call, and pass that workspace_root into attach_standard_tools::<CredentialResolver>(..., prepared, None, &workspace_root). Update all call sites of this helper to pass a test-controlled workspace root (temp dir or fixture) so tests become hermetic; references to update: the helper function itself, its parameters, and the attach_standard_tools / AgentBuilder::<(), String>::new(MockModel::new(name)) invocation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/llm-coding-tools-agents/src/path/resolver.rs`:
- Around line 127-140: The prefix-globstar optimization
(try_prefix_globstar_optimisation) incorrectly converts workspace-relative
semantics for non-traversal tools (causing inputs like "src/lib.rs" to become
workspace_root/src/src/lib.rs); stop applying that optimisation here so
non-traversal tools keep the AllowedGlobResolver semantics. Locate the branch
guarded by is_traversal_tool and remove or disable the call to
try_prefix_globstar_optimisation (leave the code path that returns an
AllowedGlobResolver / AllowedPathResolver via the glob handling), ensuring
callers that rely on resolver.resolve() still receive workspace-relative
resolution; keep references to try_prefix_globstar_optimisation,
AllowedPathResolver, AllowedGlobResolver, resolve_relative, resolve, and
is_traversal_tool to find the code to change.
---
Outside diff comments:
In `@src/llm-coding-tools-serdesai/src/agent_runtime/build.rs`:
- Around line 55-63: The error variant ToolSettingsValidation is being used
incorrectly when build_resolver_for_tool(...) fails validating
prepared.permission_config; add a dedicated error variant (e.g.,
ResolverConstruction or PermissionResolverValidation) to AgentBuildError and
return that from build_resolver_for_tool instead of ToolSettingsValidation;
update all places that call build_resolver_for_tool (including the
read/write/edit/glob/grep/grep branches and the other similar branches
referenced around the 176-233 area) to map resolver construction failures to the
new variant, and adjust any #[error(...)] messages to include the tool name and
the underlying llm_coding_tools_core::ToolError (or source) to preserve original
error context.
---
Nitpick comments:
In `@src/llm-coding-tools-core/src/tools/edit.rs`:
- Around line 80-91: The empty public struct EditSettings (and similarly
WriteSettings) is externally constructible which makes adding fields a breaking
change; mark both structs with #[non_exhaustive] so callers cannot construct
them by struct literal (allowing you to add fields later), keep the existing pub
fn new() constructors and Default/derives as-is so external code can still
create instances via the provided APIs, and update the struct declarations for
EditSettings and WriteSettings to include the #[non_exhaustive] attribute.
In `@src/llm-coding-tools-serdesai/src/agent_runtime/build.rs`:
- Around line 345-355: The helper currently calls
llm_coding_tools_core::resolve_workspace_root() directly, making build_with_mock
depend on the repo layout; change the signature of the helper (the function that
takes prepared: &super::PreparedBuild<'_>, name: &str) to accept a
workspace_root parameter (e.g., workspace_root: &Path or &str), remove the
internal resolve_workspace_root() call, and pass that workspace_root into
attach_standard_tools::<CredentialResolver>(..., prepared, None,
&workspace_root). Update all call sites of this helper to pass a test-controlled
workspace root (temp dir or fixture) so tests become hermetic; references to
update: the helper function itself, its parameters, and the
attach_standard_tools / AgentBuilder::<(), String>::new(MockModel::new(name))
invocation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 132b0689-9d7e-4530-ad3e-8dc897e8a6d6
⛔ Files ignored due to path filters (1)
src/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (28)
src/llm-coding-tools-agents/Cargo.tomlsrc/llm-coding-tools-agents/src/lib.rssrc/llm-coding-tools-agents/src/path/mod.rssrc/llm-coding-tools-agents/src/path/resolver.rssrc/llm-coding-tools-core/src/path/absolute.rssrc/llm-coding-tools-core/src/path/allowed.rssrc/llm-coding-tools-core/src/path/allowed_glob/mod.rssrc/llm-coding-tools-core/src/path/allowed_glob/normalize.rssrc/llm-coding-tools-core/src/path/allowed_glob/policy.rssrc/llm-coding-tools-core/src/path/mod.rssrc/llm-coding-tools-core/src/tools/edit.rssrc/llm-coding-tools-core/src/tools/glob.rssrc/llm-coding-tools-core/src/tools/grep.rssrc/llm-coding-tools-core/src/tools/read.rssrc/llm-coding-tools-core/src/tools/write.rssrc/llm-coding-tools-serdesai/Cargo.tomlsrc/llm-coding-tools-serdesai/README.mdsrc/llm-coding-tools-serdesai/examples/serdesai-agents.rssrc/llm-coding-tools-serdesai/examples/serdesai-task.rssrc/llm-coding-tools-serdesai/src/agent_ext.rssrc/llm-coding-tools-serdesai/src/agent_runtime/build.rssrc/llm-coding-tools-serdesai/src/agent_runtime/task.rssrc/llm-coding-tools-serdesai/src/task/handle.rssrc/llm-coding-tools-serdesai/src/tools/edit.rssrc/llm-coding-tools-serdesai/src/tools/glob.rssrc/llm-coding-tools-serdesai/src/tools/grep.rssrc/llm-coding-tools-serdesai/src/tools/read.rssrc/llm-coding-tools-serdesai/src/tools/write.rs
✅ Files skipped from review due to trivial changes (2)
- src/llm-coding-tools-agents/Cargo.toml
- src/llm-coding-tools-agents/src/path/mod.rs
🚧 Files skipped from review as they are similar to previous changes (17)
- src/llm-coding-tools-serdesai/README.md
- src/llm-coding-tools-agents/src/lib.rs
- src/llm-coding-tools-serdesai/Cargo.toml
- src/llm-coding-tools-core/src/path/allowed_glob/policy.rs
- src/llm-coding-tools-serdesai/examples/serdesai-task.rs
- src/llm-coding-tools-serdesai/src/task/handle.rs
- src/llm-coding-tools-core/src/path/allowed_glob/normalize.rs
- src/llm-coding-tools-core/src/path/absolute.rs
- src/llm-coding-tools-serdesai/src/tools/write.rs
- src/llm-coding-tools-serdesai/src/tools/edit.rs
- src/llm-coding-tools-serdesai/src/agent_ext.rs
- src/llm-coding-tools-serdesai/src/tools/grep.rs
- src/llm-coding-tools-serdesai/src/tools/read.rs
- src/llm-coding-tools-serdesai/src/tools/glob.rs
- src/llm-coding-tools-core/src/tools/glob.rs
- src/llm-coding-tools-core/src/path/allowed.rs
- src/llm-coding-tools-core/src/path/allowed_glob/mod.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Async macOS
- GitHub Check: Async Linux
- GitHub Check: Blocking Linux
- GitHub Check: Blocking Windows
- GitHub Check: Async Windows
- GitHub Check: Blocking macOS
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Sewer56
Repo: Sewer56/llm-coding-tools PR: 69
File: src/llm-coding-tools-bubblewrap/src/profile/validation.rs:57-67
Timestamp: 2026-03-28T02:14:04.465Z
Learning: In `src/llm-coding-tools-bubblewrap/src/profile/` (Rust, llm-coding-tools-bubblewrap crate), the `Builder` API paths (workspace, synthetic_home, cache_root, mount lists, overlays, etc.) are always set by trusted application/operator code — the library consumer is the trusted party. Path normalization and `..`-component hardening in validators like `validate_absolute_path` is therefore NOT required to defend against traversal attacks. Untrusted input (LLM-generated shell commands) only enters through `wrap_command`/`execute_command_with_mode`, not through the `Builder`.
📚 Learning: 2026-03-28T02:14:04.465Z
Learnt from: Sewer56
Repo: Sewer56/llm-coding-tools PR: 69
File: src/llm-coding-tools-bubblewrap/src/profile/validation.rs:57-67
Timestamp: 2026-03-28T02:14:04.465Z
Learning: In `src/llm-coding-tools-bubblewrap/src/profile/` (Rust, llm-coding-tools-bubblewrap crate), the `Builder` API paths (workspace, synthetic_home, cache_root, mount lists, overlays, etc.) are always set by trusted application/operator code — the library consumer is the trusted party. Path normalization and `..`-component hardening in validators like `validate_absolute_path` is therefore NOT required to defend against traversal attacks. Untrusted input (LLM-generated shell commands) only enters through `wrap_command`/`execute_command_with_mode`, not through the `Builder`.
Applied to files:
src/llm-coding-tools-serdesai/examples/serdesai-agents.rssrc/llm-coding-tools-core/src/path/mod.rssrc/llm-coding-tools-serdesai/src/agent_runtime/task.rssrc/llm-coding-tools-agents/src/path/resolver.rssrc/llm-coding-tools-serdesai/src/agent_runtime/build.rs
🔇 Additional comments (1)
src/llm-coding-tools-core/src/tools/grep.rs (1)
784-830: Nice regression coverage for resolver-level filtering.This exercises both
.and absolute search roots, so it should catch the exact contract change that moved from inline permission checks toresolver.is_path_allowed().
…resolution The prefix-globstar optimisation incorrectly converted workspace-relative semantics for non-traversal tools. Pattern "src/**" created an AllowedPathResolver with base workspace_root/src, causing inputs like "src/lib.rs" to resolve as workspace_root/src/src/lib.rs (doubled prefix). - Removed the try_prefix_globstar_optimisation code path entirely - Removed is_traversal_tool parameter from build_resolver_for_tool - All prefix patterns now correctly use AllowedGlobResolver which resolves relative paths against the workspace root - Marked unused helper functions with #[allow(dead_code)] per request - Updated all test assertions to expect Glob resolver for prefix patterns - Added regression test to verify workspace-relative resolution works
Pattern `**` is functionally equivalent to `Action(Allow)` (workspace-only access). Previously it fell through to AllowedGlobResolver; now it uses the cheaper AllowedPathResolver (prefix check instead of glob matching). - Added is_bare_globstar() to detect single `**` pattern - Updated optimization tables in module and function docs - Updated test to expect Allowed resolver for bare `**`
Sewer56
force-pushed
the
fix-file-tools-path-checks
branch
from
30fe246 to
e6e5e6f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/llm-coding-tools-agents/src/path/resolver.rs (1)
483-486: Avoid fixed env-var names to keep this test deterministic.Using a hardcoded “not set” variable can still flake in customized CI environments. Prefer generating a guaranteed-missing variable name at runtime.
Proposed test hardening
- patterns.insert( - "$DEFINITELY_NOT_SET_12345/**".to_string(), - PermissionAction::Allow, - ); + let mut missing = format!("CR_MISSING_VAR_{}", std::process::id()); + while std::env::var_os(&missing).is_some() { + missing.push('_'); + } + patterns.insert(format!("${missing}/**"), PermissionAction::Allow);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/llm-coding-tools-agents/src/path/resolver.rs` around lines 483 - 486, The test uses a hardcoded env-var pattern "$DEFINITELY_NOT_SET_12345/**" which can flake in CI; change the literal to a runtime-generated, guaranteed-missing name before inserting into patterns (the call site is patterns.insert(..., PermissionAction::Allow)). Implement generation by producing a unique identifier (e.g., format!("${}", uuid::Uuid::new_v4()) or using a timestamp+pid) and loop/check with std::env::var_os until env::var_os(name_without_dollar) returns None to ensure the variable truly isn't present, then use that generated name when calling patterns.insert.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/llm-coding-tools-agents/src/path/resolver.rs`:
- Around line 483-486: The test uses a hardcoded env-var pattern
"$DEFINITELY_NOT_SET_12345/**" which can flake in CI; change the literal to a
runtime-generated, guaranteed-missing name before inserting into patterns (the
call site is patterns.insert(..., PermissionAction::Allow)). Implement
generation by producing a unique identifier (e.g., format!("${}",
uuid::Uuid::new_v4()) or using a timestamp+pid) and loop/check with
std::env::var_os until env::var_os(name_without_dollar) returns None to ensure
the variable truly isn't present, then use that generated name when calling
patterns.insert.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 62d16876-3a47-4737-90bb-951c0bb9510a
📒 Files selected for processing (2)
src/llm-coding-tools-agents/src/path/resolver.rssrc/llm-coding-tools-serdesai/src/agent_runtime/build.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- src/llm-coding-tools-serdesai/src/agent_runtime/build.rs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Async Linux
- GitHub Check: Async macOS
- GitHub Check: Async Windows
- GitHub Check: Blocking Windows
- GitHub Check: Blocking macOS
- GitHub Check: Blocking Linux
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Sewer56
Repo: Sewer56/llm-coding-tools PR: 69
File: src/llm-coding-tools-bubblewrap/src/profile/validation.rs:57-67
Timestamp: 2026-03-28T02:14:04.465Z
Learning: In `src/llm-coding-tools-bubblewrap/src/profile/` (Rust, llm-coding-tools-bubblewrap crate), the `Builder` API paths (workspace, synthetic_home, cache_root, mount lists, overlays, etc.) are always set by trusted application/operator code — the library consumer is the trusted party. Path normalization and `..`-component hardening in validators like `validate_absolute_path` is therefore NOT required to defend against traversal attacks. Untrusted input (LLM-generated shell commands) only enters through `wrap_command`/`execute_command_with_mode`, not through the `Builder`.
📚 Learning: 2026-03-28T02:14:04.465Z
Learnt from: Sewer56
Repo: Sewer56/llm-coding-tools PR: 69
File: src/llm-coding-tools-bubblewrap/src/profile/validation.rs:57-67
Timestamp: 2026-03-28T02:14:04.465Z
Learning: In `src/llm-coding-tools-bubblewrap/src/profile/` (Rust, llm-coding-tools-bubblewrap crate), the `Builder` API paths (workspace, synthetic_home, cache_root, mount lists, overlays, etc.) are always set by trusted application/operator code — the library consumer is the trusted party. Path normalization and `..`-component hardening in validators like `validate_absolute_path` is therefore NOT required to defend against traversal attacks. Untrusted input (LLM-generated shell commands) only enters through `wrap_command`/`execute_command_with_mode`, not through the `Builder`.
Applied to files:
src/llm-coding-tools-agents/src/path/resolver.rs
🔇 Additional comments (2)
src/llm-coding-tools-agents/src/path/resolver.rs (2)
112-119: Explicit deny handling is correct now.Good change:
PermissionRule::Action(PermissionAction::Deny)now short-circuits toToolError::PermissionDeniedinstead of falling through to an allow resolver.
507-562: Regression coverage here is strong and directly targets prior breakage.The
resolve("src/lib.rs")andresolve(".")tests are valuable guards for workspace-relative semantics.
| let expanded = expand_shell(pattern)?; | ||
| if expanded.to_string_lossy() == "/**" { | ||
| return Ok(Some(AbsolutePathResolver)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== CI platform targets =="
if [ -d .github/workflows ]; then
rg -n --iglob '*.yml' --iglob '*.yaml' -C2 '(runs-on:|matrix:|windows|ubuntu|macos)' .github/workflows
else
echo "No .github/workflows directory found"
fi
echo
echo "== Platform support mentions in docs =="
rg -n -C2 '(Windows|cross-platform|platform support)' README.md docs 2>/dev/null || true
echo
echo "== Unix-specific unrestricted checks in resolver =="
rg -n -C2 '"/\\*\\*"|"/etc/passwd"' src/llm-coding-tools-agents/src/path/resolver.rsRepository: Sewer56/llm-coding-tools
Length of output: 2964
🏁 Script executed:
cat -n src/llm-coding-tools-agents/src/path/resolver.rs | sed -n '140,160p'Repository: Sewer56/llm-coding-tools
Length of output: 948
🏁 Script executed:
cat -n src/llm-coding-tools-agents/src/path/resolver.rs | sed -n '245,275p'Repository: Sewer56/llm-coding-tools
Length of output: 1566
🏁 Script executed:
rg -n -C5 'expand_shell' src/llm-coding-tools-agents/src/path/resolver.rsRepository: Sewer56/llm-coding-tools
Length of output: 1384
🏁 Script executed:
fd -t f 'expand_shell' src/llm-coding-tools-core/src --type fRepository: Sewer56/llm-coding-tools
Length of output: 50
🏁 Script executed:
rg -l 'fn expand_shell' src/Repository: Sewer56/llm-coding-tools
Length of output: 128
🏁 Script executed:
rg -n -B2 -A10 'fn expand_shell' src/llm-coding-tools-coreRepository: Sewer56/llm-coding-tools
Length of output: 4075
🏁 Script executed:
rg -n -B2 -A15 'fn expand_pattern' src/llm-coding-tools-core/src/path/allowed_glob/normalize.rsRepository: Sewer56/llm-coding-tools
Length of output: 3293
🏁 Script executed:
rg -n -B5 'absolute_globstar_returns_absolute' src/llm-coding-tools-agents/src/path/resolver.rs | head -20Repository: Sewer56/llm-coding-tools
Length of output: 355
🏁 Script executed:
rg -n '\[cfg' src/llm-coding-tools-agents/src/path/resolver.rs | head -20Repository: Sewer56/llm-coding-tools
Length of output: 84
🏁 Script executed:
cat -n src/llm-coding-tools-agents/src/path/resolver.rs | sed -n '250,267p'Repository: Sewer56/llm-coding-tools
Length of output: 796
🏁 Script executed:
rg -n 'cfg.*windows\|cfg.*unix' src/llm-coding-tools-agents/src/path/resolver.rsRepository: Sewer56/llm-coding-tools
Length of output: 50
🏁 Script executed:
cat -n src/llm-coding-tools-agents/src/path/resolver.rs | sed -n '188,220p'Repository: Sewer56/llm-coding-tools
Length of output: 1426
🏁 Script executed:
rg -n -B5 'is_path_allowed' src/llm-coding-tools-agents/src/path/resolver.rs | head -40Repository: Sewer56/llm-coding-tools
Length of output: 1581
🏁 Script executed:
rg -n 'AbsolutePathResolver' src/llm-coding-tools-core/src/pathRepository: Sewer56/llm-coding-tools
Length of output: 843
🏁 Script executed:
rg -n '#\[cfg.*test' src/llm-coding-tools-agents/src/path/resolver.rs | head -20Repository: Sewer56/llm-coding-tools
Length of output: 84
🏁 Script executed:
cat -n src/llm-coding-tools-agents/src/path/resolver.rs | sed -n '1,50p'Repository: Sewer56/llm-coding-tools
Length of output: 2643
🏁 Script executed:
cat -n src/llm-coding-tools-core/src/path/absolute.rs | sed -n '25,50p'Repository: Sewer56/llm-coding-tools
Length of output: 854
🏁 Script executed:
rg -n 'is_path_allowed' src/llm-coding-tools-core/src/path/absolute.rsRepository: Sewer56/llm-coding-tools
Length of output: 387
🏁 Script executed:
rg -n 'is_absolute' src/llm-coding-tools-core/src/path/absolute.rsRepository: Sewer56/llm-coding-tools
Length of output: 103
🏁 Script executed:
rg -n '/**' src/llm-coding-tools-agents/src/path/resolver.rsRepository: Sewer56/llm-coding-tools
Length of output: 23727
🏁 Script executed:
rg -n 'cfg.*test\|#\[test\]' src/llm-coding-tools-agents/src/path/resolver.rs | grep -A1 'absolute_globstar'Repository: Sewer56/llm-coding-tools
Length of output: 50
🏁 Script executed:
rg -n '\[cfg.*windows\|\[cfg.*unix\|\[cfg.*target_os' src/llm-coding-tools-agents/src/path/resolver.rsRepository: Sewer56/llm-coding-tools
Length of output: 50
🏁 Script executed:
python3 << 'EOF'
import os
import sys
# On Unix, "/etc/passwd" is absolute
# On Windows, "/" is NOT absolute (it's relative)
# Verify this by checking Path.is_absolute() behavior
from pathlib import Path
unix_path = Path("/etc/passwd")
windows_examples = [
Path("C:\\Windows\\System32"),
Path("/etc/passwd"), # On Windows, this is relative
]
print("Path behavior check:")
print(f" /etc/passwd is_absolute: {unix_path.is_absolute()}")
print(f" Note: this depends on OS where script runs")
# On Windows, "/" is NOT absolute
# On Unix, "/" IS absolute
# So the test is platform-dependent
EOFRepository: Sewer56/llm-coding-tools
Length of output: 165
🏁 Script executed:
cat -n src/llm-coding-tools-agents/src/path/resolver.rs | sed -n '1,30p'Repository: Sewer56/llm-coding-tools
Length of output: 1745
Add Windows-compatible absolute-path pattern handling or guard the "/" optimization with #[cfg(unix)].**
The pattern "/**" optimization (lines 149–151) keys off a literal string match that is semantically only valid on Unix. On Windows, absolute paths start with drive letters (C:\, etc.), not /, making /** a meaningless pattern. The test assertion at line 263 uses /etc/passwd—also Unix-only.
Windows is a supported runtime target (CI matrix includes windows-latest). Either:
- Add platform-specific pattern variants (e.g., recognize
C:\**or other Windows root patterns alongside/**) - Or guard the optimization with
#[cfg(unix)]if Windows support does not require the absolute-path fast-path
There was a problem hiding this comment.
@coderabbitai This is intended behaviour to allow pasting files across OSes.
/** is not a valid path on Windows; so allowing the behaviour to stick is intended.
There was a problem hiding this comment.
@Sewer56, understood — that's a clean design: /** is intentionally a Unix-only absolute-path escape hatch that becomes a no-op on Windows (where it isn't a valid absolute path), allowing a single config file to be shared across platforms without extra #[cfg] guards. I'll note this for future reviews.
✏️ Learnings added
Learnt from: Sewer56
Repo: Sewer56/llm-coding-tools PR: 101
File: src/llm-coding-tools-agents/src/path/resolver.rs:149-151
Timestamp: 2026-04-11T21:51:36.623Z
Learning: In `src/llm-coding-tools-agents/src/path/resolver.rs`, the `/**` pattern optimization (which selects `AbsolutePathResolver` for unrestricted access) is intentionally Unix-only. On Windows, `/**` is not a valid absolute path and therefore the optimization simply does not trigger, falling through to `AllowedGlobResolver`. This cross-platform behavior is by design, allowing a single permission config to be shared across OSes without platform-specific guards.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: Sewer56
Repo: Sewer56/llm-coding-tools PR: 69
File: src/llm-coding-tools-bubblewrap/src/profile/validation.rs:57-67
Timestamp: 2026-03-28T02:14:04.465Z
Learning: In `src/llm-coding-tools-bubblewrap/src/profile/` (Rust, llm-coding-tools-bubblewrap crate), the `Builder` API paths (workspace, synthetic_home, cache_root, mount lists, overlays, etc.) are always set by trusted application/operator code — the library consumer is the trusted party. Path normalization and `..`-component hardening in validators like `validate_absolute_path` is therefore NOT required to defend against traversal attacks. Untrusted input (LLM-generated shell commands) only enters through `wrap_command`/`execute_command_with_mode`, not through the `Builder`.
There was a problem hiding this comment.
@Sewer56, understood — the graceful no-op on Windows is intentional. Since /** isn't a valid absolute path on Windows, the string check simply never matches there and falls through to the AllowedGlobResolver, which is the desired safe degradation. I'll note this for future reviews.
✏️ Learnings added
Learnt from: Sewer56
Repo: Sewer56/llm-coding-tools PR: 101
File: src/llm-coding-tools-agents/src/path/resolver.rs:149-151
Timestamp: 2026-04-11T21:51:49.088Z
Learning: In `src/llm-coding-tools-agents/src/path/resolver.rs` (Rust, llm-coding-tools-agents crate), the `/**` optimization in `try_globstar_optimisation` is intentionally Unix-only behavior. On Windows, `/**` is not a valid absolute path pattern, so the literal string match will never trigger there and the resolver safely falls through to `AllowedGlobResolver`. This cross-OS degradation is by design: Unix-authored configs using `/**` do not accidentally grant unrestricted access on Windows. Do not flag this as a missing `#[cfg(unix)]` guard.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: Sewer56
Repo: Sewer56/llm-coding-tools PR: 69
File: src/llm-coding-tools-bubblewrap/src/profile/validation.rs:57-67
Timestamp: 2026-03-28T02:14:04.465Z
Learning: In `src/llm-coding-tools-bubblewrap/src/profile/` (Rust, llm-coding-tools-bubblewrap crate), the `Builder` API paths (workspace, synthetic_home, cache_root, mount lists, overlays, etc.) are always set by trusted application/operator code — the library consumer is the trusted party. Path normalization and `..`-component hardening in validators like `validate_absolute_path` is therefore NOT required to defend against traversal attacks. Untrusted input (LLM-generated shell commands) only enters through `wrap_command`/`execute_command_with_mode`, not through the `Builder`.
1 participant
Summary
FileToolResolverenum andbuild_resolver_for_toolto select the cheapest path resolver per tool based on permission config (prefix check, glob, or unrestricted)is_path_allowedandpath_modeas trait methods onPathResolver, giving all resolvers a fast per-entry path check without going through fullresolve()allowed_glob::normalize,allowed_glob::policy, andexpand_shellpublic so the agents crate can build resolvers