Skip to content

improvement(copilot): refuse approval-gated tools on the in-band lane - #8044

Merged
waleedlatif1 merged 2 commits into
stagingfrom
sec/copilot-inband-approval-guard
Sep 20, 2026
Merged

waleedlatif1 merged 2 commits into
stagingfrom
sec/copilot-inband-approval-guard

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Copilot's tool-approval gate is scaffolding today — COPILOT_TOOL_PERMISSIONS_ENABLED is off by default, so nothing is gated on any lane. This changes no behavior while it stays off.
  • The gate is built only on the dispatch lane, which holds a call against a streaming context and a decision row, then deliberately declines to dispatch anything the mothership marks in-band. Those calls run via POST /api/copilot/tools/execute, which has neither a context nor a waiter — so enabling the flag would gate the foreground and leave background lanes ungated. A gate that looks enforced but is not is worse than no gate.
  • Adds toolRequiresApprovalLane next to toolCallNeedsApproval so the covered tool set is defined once, and refuses a gated tool at the in-band route before it runs.
  • Refuses rather than blocks: a background lane must never hang on a prompt with no row behind it. The mothership takes the checkpoint lane, where the gate lives.
  • The check deliberately ignores the stored auto-allow list. An auto-allowed tool sent to the checkpoint lane is admitted there without prompting anyone, so reading it here would add a database read to every in-band call to reach the same place by a longer route.
  • Records on the flag itself that the gate is a property of the lane, not only of the tool, since that doc comment is what gets read before enabling it.

Type of Change

  • Bug fix

Testing

  • New tests: the in-band route refuses a gated tool without executing it when permissions are enabled; an ungated tool still runs; a gated tool runs unchanged while permissions are disabled (pinning that this ships inert). Unit tests for toolRequiresApprovalLane across flag/catalog combinations.
  • Verified the refusal test fails when the guard is removed.
  • vitest run app/api/copilot lib/copilot — 2642 passed, 3 skipped
  • tsc --noEmit clean, bun run lint clean, check:audits 47/47, docs-manifest:check in sync, and the workflow's inline env-flag audit passes (env-flags.ts changed)

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

Copilot's approval gate is scaffolding today: COPILOT_TOOL_PERMISSIONS_ENABLED is off
by default, so nothing is gated on any lane. It is built only on the dispatch lane,
which holds a call against a streaming context and a decision row and then declines
to dispatch anything the mothership marks in-band. Those calls run via
POST /api/copilot/tools/execute, which has no context and no waiter, so turning the
flag on would gate the foreground and leave background lanes ungated — a gate that
looks enforced but is not.

Add toolRequiresApprovalLane next to toolCallNeedsApproval so the covered tool set is
defined once, and refuse a gated tool at the in-band route before it runs. Refuse
rather than block: a background lane must never hang on a prompt with no row behind
it. The check deliberately ignores the stored auto-allow list — an auto-allowed tool
sent to the checkpoint lane is admitted there without prompting anyone, so reading it
here would only add a database read to reach the same place.

Inert while the flag is off, which is the state this ships in; a test pins that.
Also record on the flag itself that the gate is a property of the lane, since that is
what the next person reads before enabling it.
@vercel

vercel Bot commented Sep 20, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Sep 20, 2026 12:20am UTC

Request Review

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 5 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread apps/sim/app/api/copilot/tools/execute/route.ts
Comment thread apps/sim/lib/copilot/request/tools/permission.ts Outdated
@greptile-apps

greptile-apps Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the prior import-side-effect issue is fixed and no new actionable failures remain.

Summary

This PR prevents approval-gated Copilot tools from executing through the in-band route when tool permissions are enabled.

  • Adds a dependency-light lane predicate beside the tool router.
  • Refuses gated in-band calls before tool execution while preserving current behavior when permissions are disabled.
  • Adds route and router coverage for gated, ungated, disabled-flag, and unknown-tool cases.
  • Moves the predicate away from the permission persistence module, resolving the previously reported unnecessary Pub/Sub initialization.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  Call[Copilot tool call] --> Lane{Execution lane}
  Lane -->|Dispatch/checkpoint| Gate[Approval gate with streaming context]
  Gate --> Decision{Approval required?}
  Decision -->|No or already allowed| Execute[Execute tool]
  Decision -->|Yes| Wait[Wait for user decision]
  Lane -->|In-band| Predicate{Permissions enabled and tool gated?}
  Predicate -->|No| Execute
  Predicate -->|Yes| Refuse[Refuse without execution]
Loading

Reviews (2) · Last reviewed commit: "improvement(copilot): move the approval-..."

Comment thread apps/sim/app/api/copilot/tools/execute/route.ts Outdated
…l router

Importing the dispatch gate module for a one-line predicate pulled the permission
persistence layer in with it, whose module body opens a pub/sub channel — two Redis
clients and a channel subscription — in every process that loads the in-band route.

Move toolRequiresApprovalLane to tool-executor/router.ts, which imports only the
catalog. The route already imported @/lib/copilot/tool-executor for
ensureHandlersRegistered, so the guard now costs no new import edge at all. The
dispatch gate keeps a pointer to it.

Its flag-and-catalog behavior is covered in the router tests against the real flag and
the real catalog; the route tests keep to what the route does with the answer.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 7 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit 3868bf9 into staging Sep 20, 2026
34 checks passed
@waleedlatif1
waleedlatif1 deleted the sec/copilot-inband-approval-guard branch September 20, 2026 00:41

This branch was previously deployed

1 inactive deployment
Preview 2986e563 Deployed Sep 20, 2026 by vercel[bot]
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.

1 participant