fix(permissions): match rule patterns against url and fail closed - #563
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
![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: e32ead3a00
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for key in PATTERN_INPUT_FIELDS { | ||
| if let Some(value) = input.get(*key).and_then(|v| v.as_str()) { | ||
| saw_comparable = true; | ||
| if glob_match(pattern, value) { | ||
| return true; |
There was a problem hiding this comment.
With a default-deny configuration and an allow rule such as WebFetch: https://docs.example.com/*, a valid call can set url to an untrusted destination and the optional prompt to https://docs.example.com/guide; because any listed field may satisfy the pattern, the prompt match returns Allow for the unrelated URL. This bypasses the destination restriction the new URL rule is meant to enforce, so comparable fields need to be selected per tool (in particular, WebFetch rules must match url, not prompt).
AGENTS.md reference: AGENTS.md:L194-L194
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e32ead3a00
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for key in PATTERN_INPUT_FIELDS { | ||
| if let Some(value) = input.get(*key).and_then(|v| v.as_str()) { | ||
| saw_comparable = true; | ||
| if glob_match(pattern, value) { | ||
| return true; |
There was a problem hiding this comment.
With a patterned WebFetch allow rule and a restrictive default, this any-field match lets the optional prompt satisfy the rule even when url does not; for example, {"url":"https://evil.example/","prompt":"https://docs.example.com/guide"} is allowed by the new https://docs.example.com/* rule. WebFetchTool::call ignores prompt and fetches the disallowed URL, so auxiliary fields must not authorize the URL subject.
AGENTS.md reference: AGENTS.md:L194-L194
Useful? React with 👍 / 👎.
| /// branch below rather than a silent empty-string match. | ||
| const PATTERN_INPUT_FIELDS: &[&str] = &[ | ||
| "file_path", // FileRead / FileWrite / FileEdit / MultiEdit / NotebookEdit | ||
| "url", // WebFetch |
There was a problem hiding this comment.
When a permitted public URL redirects, only the original input URL is matched here, while WebFetchTool::call configures reqwest::redirect::Policy::limited(10) and follows the new target without another permission check. Consequently, an allowed host with an open redirect can bypass a restrictive default, and a public URL can redirect into a host covered by the documented deny rule; redirects must be disabled or each destination must be checked before following it.
AGENTS.md reference: AGENTS.md:L194-L194
Useful? React with 👍 / 👎.
matches_input_pattern only inspected command/file_path/pattern and fell back to glob_match against "". WebFetch (and any other tool whose subject is not those three keys) never matched a literal pattern, so a deny rule on a URL loaded and evaluated but silently did nothing. Match any present subject field (file_path, url, path, pattern, query, prompt). When none are present, fail closed: allow does not grant, deny and ask still apply.
WebFetch allow rules must compare url only so a crafted prompt cannot satisfy a docs-site pattern. Scope other tools similarly and fail closed when no comparable field exists.
f66d6b8 to
1d1b05e
Compare
1 participant
Summary
matches_input_patternonly inspectedcommand/file_path/patternand fell back toglob_match(pattern, ""). WebFetch (subject fieldurl) never matched a literal pattern, so a configured deny rule loaded, evaluated, and silently failed open.file_path,url,path,pattern,query,prompt.allowdoes not grant;deny/askstill apply. Pure*patterns still mean any input.Fixes #554.
Test plan
cargo check --all-targetscargo test --all-targets(only known hostbwrap_*failures)cargo clippy --all-targets -- -D warningscargo fmt --all -- --check*a_deny_rule_on_url_blocks_matching_webfetchfails with "must block a matching fetch"; drop fail-closed →no_comparable_field_fails_closedfails as expected