fix(tools): require permission before web_search requests

[GautamBytes]

Fixes #380

Summary

• Gate the built-in web_search tool behind the normal permission flow before it sends queries to a configured backend.
• Keep web_search visible in auto mode with AdvertiseInAuto: true, matching the existing web_fetch pattern.
• Add coverage for registry, agent-loop, and sandbox behavior so denied permission prevents execution while approved calls still work.

Why

web_search is marked as a network side-effecting tool, but it previously used PermissionAllow. That let hosted search run without a permission prompt even though queries may include prompt or workspace context.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2ef96e14-8860-4d66-a5b8-856d229abaaf

📥 Commits

Reviewing files that changed from the base of the PR and between 570fa21 and cc12b55.

📒 Files selected for processing (2)

• internal/agent/loop_test.go
• internal/tools/web_search.go

👮 Files not reviewed due to content moderation or server errors (2)

• internal/agent/loop_test.go
• internal/tools/web_search.go

---

Warning

Walkthrough skipped

File diffs could not be summarized.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Copilot]

Pull request overview

This PR tightens web_search’s safety/permission behavior so that networked hosted search cannot execute until the normal permission flow approves it, aligning it with web_fetch while still keeping it visible to the model in auto mode.

Changes:

• Switch web_search safety metadata from PermissionAllow to PermissionPrompt and set AdvertiseInAuto: true.
• Add/adjust tests to ensure permission is required before any backend call, and that approved runs still execute correctly.
• Update sandbox/agent-loop expectations to reflect prompting behavior for web_search under network-deny shell egress policies.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
internal/tools/web_search.go	Updates web_search safety metadata to prompt for permission while remaining advertised in auto mode.
internal/tools/web_search_test.go	Adds coverage ensuring permission is enforced before backend execution, and validated after grant.
internal/tools/registry_test.go	Updates core tool safety expectations for web_search to require prompt + auto advertisement.
internal/sandbox/engine_test.go	Adjusts sandbox evaluation expectations to prompt for web_search based on tool permission metadata.
internal/agent/loop_test.go	Adds agent-loop coverage to ensure permission is requested and denied calls do not execute web_search.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Vasanthdev2004]

Verdict: Approve ✅ — correct, and it makes web_search consistent with web_fetch

Is it useful? Yes. web_search was the one first-party network tool that ran without a permission prompt (PermissionAllow), even though it ships model-generated query text — which can embed prompt/workspace context — to a hosted backend. Its sibling web_fetch already prompts for exactly this reason. I confirmed web_fetch's metadata is SideEffectNetwork + PermissionPrompt + AdvertiseInAuto: true (web_fetch.go:128-131) — this PR makes web_search byte-for-byte identical in posture. Closing that asymmetry is the right call.

Implementation is correct:

• AdvertiseInAuto: true is required and right — advertisement in auto mode is gated on Permission == PermissionAllow || AdvertiseInAuto (loop.go:2620/2628), so without it a now-PermissionPrompt tool would vanish from the model's toolset in auto mode. With it, web_search stays visible in auto and prompts only where the mode prompts — same as web_fetch. ✔
• The sandbox exemption is preserved correctly: engine_test.go now asserts web_search yields ActionPrompt (from tool permission) rather than ReasonNetworkBlocked (shell egress), and ActionAllow once granted. So first-party network tools are still exempt from the shell egress policy and governed by their permission metadata — the intended design. ✔
• No stragglers: the web_search-specific PermissionAllow assertions (registry/web_search/engine/loop tests) are all updated; I grepped the tree and found no missed reference.

Behavioral impact (worth the maintainer noting): this is a deliberate reversal of the old "read-only discovery action, allow without prompt" choice. Net effect — Ask mode now prompts before a search (previously frictionless); Auto mode is unchanged (still runs). Session/always grants remain the compatibility path, so it's one approval per session, not per query. Given it simply brings web_search in line with web_fetch, the trade-off is reasonable — flagging only so it's a conscious product decision, not a surprise.

Verification (branch checked out): go build ./... clean, gofmt clean on all five changed files, and the full tools + agent packages plus every new test pass. (The one sandbox failure — TestNewScopeNormalizesAndValidatesExtraRoots, "want default /tmp write root" — is a pre-existing Windows-only failure that reproduces identically on main; unrelated to this change.)

Approving. Thanks @GautamBytes — clean, well-tested, and the red-green permission test is exactly the right guard.

[kevincodex1]

nice catch!