Skip to content

fix(permissions): scope smart approval by request - #10457

Merged
alexhancock merged 4 commits into
mainfrom
jbg/security-smart-approve-request-scope
Jul 17, 2026
Merged

fix(permissions): scope smart approval by request#10457
alexhancock merged 4 commits into
mainfrom
jbg/security-smart-approve-request-scope

Conversation

@jbg

@jbg jbg commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • give the SmartApprove judge each request ID, tool name, and argument object
  • classify results by request ID so same-name calls with different operations remain distinct
  • stop persisting positive LLM judgments as tool-name-wide permissions and ignore legacy positive cache entries
  • retain conservative name-wide caching only for decisions that require approval

Validation

  • cargo fmt --all -- --check
  • cargo test -p goose permission::
  • cargo build -p goose
  • cargo clippy -p goose --all-targets -- -D warnings
  • git diff --check

This finding was discovered by Project Loupe.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 05ee410a1b

ℹ️ 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".

Comment thread crates/goose/src/permission/permission_judge.rs
@jbg
jbg requested review from DOsinga and jamadeo July 14, 2026 13:41

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

} else if goose_mode == GooseMode::SmartApprove
&& permission_manager
.get_smart_approve_permission(tool_name)
.is_none()

P2 Badge Treat legacy smart-approve allows as uncached

When a user already has a smart_approve.always_allow entry from an older release, this branch does not send the request to the new per-request judge because get_smart_approve_permission(tool_name) is Some(...), but the entry is also no longer trusted as an allow and the code falls through to RequireApproval. That means SmartApprove becomes permanently equivalent to Approve for those tools until the user manually edits permission.yaml or creates a user-level rule, even for clearly read-only calls; treating legacy AlwaysAllow as absent or removing it would let the request be reclassified safely.

ℹ️ 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".

@alexhancock
alexhancock merged commit efc7ccc into main Jul 17, 2026
26 checks passed
@alexhancock
alexhancock deleted the jbg/security-smart-approve-request-scope branch July 17, 2026 20:27
michaelneale added a commit that referenced this pull request Jul 20, 2026
* origin/main: (24 commits)
  fix(session): create inventory tables atomically with schema version (#10586)
  fix(providers): rewrite oneOf to anyOf in tool schemas for OpenAI-compatible backends (#10571)
  fix(evals): report cache-aware Harbor costs (#10430)
  fix(acp): allow custom model as default for non-local providers (#10438)
  fix(config): require absolute goose path roots (#10454)
  chore(deps): bump astral-sh/setup-uv from 8.2.0 to 8.3.2 (#10541)
  fix(permissions): scope smart approval by request (#10457)
  fix(summon): preserve fixed subrecipe values (#10452)
  chore(deps): bump websocket-driver from 0.7.4 to 0.7.5 in /documentation (#10506)
  fix(flatpak): bundle git so hermit can clone its package registry (#10511)
  feat(hooks): pass working_dir to the Stop hook context (#10296)
  chore(deps): bump actions/setup-java from 5.5.0 to 5.6.0 (#10540)
  chore(deps): bump actions/setup-node from 6 to 7 (#10539)
  chore(deps): bump EmbarkStudios/cargo-deny-action from 2.0.20 to 2.1.1 (#10542)
  chore(deps): bump gradle/actions/setup-gradle from 4.4.3 to 6.2.0 (#10543)
  Add declarative Sakana AI provider for the OpenAI-compatible Fugu API (#10357)
  fix(developer): expose AGENT_SESSION_ID to shell commands (#10428)
  Clean up stale documentation audit findings (#10114)
  Restore model interactions viewer (#10205)
  fix(acp): forward image content chunks to client during live session (#10485)
  ...

# Conflicts:
#	crates/goose/src/session/session_manager.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants