fix(policy): case-insensitive navigate domain; token-bound sensitive keys - #14
fix(policy): case-insensitive navigate domain; token-bound sensitive keys#14Jimthetaxguy wants to merge 1 commit into
Conversation
…keys target_domain compared tool_name == "navigate", so NAVIGATE/Navigate scheme-checked then applied allow/deny to the current page. Sensitive-key needles used substring contains, so auth hit author/authorization and card hit discard. Co-authored-by: Jimthetaxguy <Jimthetaxguy@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 7a671e7130
ℹ️ 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".
| ] | ||
| .iter() | ||
| .any(|needle| key.contains(needle)) | ||
| .any(|needle| contains_token(&key, needle)) |
There was a problem hiding this comment.
When a tool accepts the conventional authorization argument containing a bearer credential, this boundary rule now deliberately treats it as non-sensitive; it similarly misses common camelCase keys such as accessToken and cardNumber. Compared with the parent implementation, these values neither trigger RequireApproval nor get redacted before decision.redacted_arguments is emitted in agent events, exposing credentials to callers and audit consumers. Preserve recognition of credential-bearing compound names while excluding actual false positives such as author and discard.
Useful? React with 👍 / 👎.
2 participants
Tiny policy honesty pass on
main. Two real bugs, smallest possible diff. Does not redo PR #5 (scaffolding) or PR #13 (ReadOnly blocks navigate).Changes
target_domaintreatsnavigatecase-insensitively. Scheme-block already usedeq_ignore_ascii_case("navigate"), buttarget_domainusedtool_name == "navigate". ANavigate/NAVIGATEcall still scheme-checked, then applied domain allow/deny to the current page URL instead ofarguments["url"]. Same comparison as the scheme arm /denied_tools.is_sensitive_keyis token-bounded. Needles were substring-matched (key.contains), so"auth"hitauthor/authorizationand"card"hitdiscard. Those keys then forced RequireApproval and redacted. Match is now exact or token-boundary on the listed names (credit_cardstill flags).tests/action_policy.rs:navigate_domain_check_is_case_insensitive,sensitive_key_match_is_token_bounded.Out of scope
src-tauri// Swift / docsverify.shrewrite, no crate-wide nits (futures, etc.)Proof
Ran locally on Rust 1.98.1:
cargo test --all-targets --locked— pass (includingnavigate_domain_check_is_case_insensitiveandsensitive_key_match_is_token_bounded)cargo clippy --all-targets --locked -- -D warnings— passsrc/agent/policy.rsandtests/action_policy.rs