diff --git a/POLICY.md b/POLICY.md index 991a60e..bc7dd00 100644 --- a/POLICY.md +++ b/POLICY.md @@ -114,12 +114,13 @@ The `action` field controls how the rule behaves: |---|---|---| | `"evaluate"` (default) | Checks conditions; denies the call if any condition fails | Required (at least one) | | `"deny"` | Unconditionally blocks the tool call | Not allowed | +| `"require_approval"` | Blocks the call until a human approves it via CLI or admin API | Optional | If `action` is omitted, it defaults to `"evaluate"`. ### on_deny -The `on_deny` field is an optional message returned to the AI agent when a rule denies a tool call. The agent sees it prefixed with `[INTERCEPT POLICY DENIED]`: +The `on_deny` field is an optional message returned to the AI agent when a rule denies a tool call. For `deny` and `evaluate` rules the agent sees it prefixed with `[INTERCEPT POLICY DENIED]`. For `require_approval` rules the prefix is `[INTERCEPT APPROVAL REQUIRED]`: ```yaml on_deny: "Hourly limit of 5 new issues reached. Wait before creating more." @@ -155,6 +156,79 @@ Restrictions: For wildcard (`"*"`) tools, the counter is scoped to `_global` as usual. +## Approval rules + +Rules with `action: "require_approval"` pause tool calls until a human explicitly approves or denies them. The agent receives a JSON-RPC error (`-32003`) with the approval ID and instructions. + +```yaml +tools: + create_charge: + rules: + - name: "large charge approval" + action: "require_approval" + conditions: + - path: "args.amount" + op: "gt" + value: 50000 + on_deny: "Charges over $500.00 require human approval" + approval_timeout: "30m" +``` + +When a matching call arrives, Intercept creates a pending approval record. The agent is told to wait. A human can then approve or deny via: + +```sh +intercept approvals approve +intercept approvals deny --reason "too expensive" +``` + +If the same tool call (identical arguments and rule) is retried while a pending approval exists, Intercept reuses the existing record rather than creating a duplicate. + +Once approved, the next identical call is allowed through and the approval is consumed (single use). If the approval expires before being consumed, the next call creates a fresh pending record. + +### approval_timeout + +Per-rule timeout controlling how long an approval stays valid. Accepts Go duration strings (`5m`, `1h`, `24h`). Defaults to the global `approvals.default_timeout`, which itself defaults to `15m`. + +```yaml +- name: "destructive action" + action: "require_approval" + approval_timeout: "1h" +``` + +### Top-level approvals block + +```yaml +approvals: + default_timeout: 10m + dedupe_window: 10m + notify: + webhook: + url: "https://your-bot.com/intercept" + secret: "your-hmac-secret" +``` + +| Field | Required | Description | +|---|---|---| +| `default_timeout` | no | Default approval lifetime (Go duration, default 15m) | +| `dedupe_window` | no | Window for deduplicating identical retry attempts | +| `notify.webhook.url` | no | URL to POST when an approval is requested | +| `notify.webhook.secret` | no | HMAC-SHA256 secret for signing webhook payloads | + +### Webhook notifications + +When configured, Intercept POSTs a JSON payload to the webhook URL each time a new approval is requested. The payload includes the approval ID, tool name, rule, reason, expiry, and a safe preview of the arguments. + +Webhook deliveries are signed with HMAC-SHA256. Headers: `X-Intercept-Signature` (hex-encoded) and `X-Intercept-Timestamp`. + +Webhooks are fire-and-forget: delivery failure is logged but never blocks the tool call or causes an accidental allow. + +### Restrictions + +- `rate_limit` cannot be combined with `action: require_approval` on the same rule (use separate rules) +- `state` blocks should not be placed on `require_approval` rules (use a separate rate limit rule) +- Hidden tools (`hide` list) cannot be rescued by approval rules +- Approval fingerprints include a policy revision hash -- changing the policy file invalidates pending approvals + ## Conditions Conditions compare a value at a given `path` against an expected `value` using an `op` (operator). All conditions within a rule are ANDed: every condition must pass for the rule to allow the call. @@ -321,6 +395,7 @@ When a `tools/call` request arrives, Intercept processes it as follows: 2. Look up wildcard (`"*"`) rules. 3. Evaluate all matching rules in order (tool-specific first, then wildcard): - If a rule has `action: "deny"`, the call is denied immediately. + - If a rule has `action: "require_approval"`, and conditions match (or no conditions), the call is held for approval. - If a rule has `action: "evaluate"`, all its conditions must pass. 4. If any rule denies the call, the `on_deny` message from that rule is returned to the agent. 5. If all rules pass, reserve any stateful counters atomically. @@ -453,7 +528,7 @@ Run `intercept validate -c policy.yaml` to check a policy file for errors. Every | `version must be "1", got ""` | The `version` field is missing or not `"1"` | Set `version: "1"` | | `default must be "allow" or "deny", got ""` | The `default` field has an unrecognised value | Use `"allow"` or `"deny"`, or omit the field | | `rule must have a name` | A rule is missing the `name` field | Add a `name` to the rule | -| `action must be "evaluate" or "deny", got ""` | Unrecognised action value | Use `"evaluate"` or `"deny"` | +| `action must be "evaluate", "deny", or "require_approval", got ""` | Unrecognised action value | Use `"evaluate"`, `"deny"`, or `"require_approval"` | | `deny rules must not have conditions` | A rule with `action: "deny"` has a `conditions` list | Remove `conditions` from deny rules | | `evaluate rules must have at least one condition` | A rule with `action: "evaluate"` has no conditions | Add at least one condition | | `path must start with "args." or "state.", got ""` | Condition path does not reference tool arguments or state | Use `args.` or `state..` | @@ -471,4 +546,15 @@ Run `intercept validate -c policy.yaml` to check a policy file for errors. Every | `rate_limit window must be "minute", "hour", or "day", got ""` | Unrecognised window in `rate_limit` | Use `minute`, `hour`, or `day` | | `rate_limit cannot be combined with conditions or state` | A rule uses `rate_limit` alongside `conditions` or `state` | Use separate rules, or switch to the full syntax | | `rate_limit cannot be used with action "deny"` | A deny rule also has `rate_limit` | Remove `rate_limit` or change the action | +| `rate_limit cannot be used with action "require_approval"` | An approval rule also has `rate_limit` | Use separate rules for rate limiting and approval | +| `require_approval rules must not have a state block` | An approval rule has a `state` block | Move the state block to a separate evaluate rule | +| `invalid approval_timeout ""` | The `approval_timeout` value is not a valid Go duration | Use a duration like `5m`, `1h`, `24h` | +| `approval_timeout must be positive` | The `approval_timeout` resolves to zero or negative | Use a positive duration | +| `approvals.default_timeout: invalid duration ""` | The global default timeout is not a valid Go duration | Use a duration like `15m` | +| `approvals.default_timeout must be positive` | The global default timeout is zero or negative | Use a positive duration | +| `approvals.dedupe_window: invalid duration ""` | The dedupe window is not a valid Go duration | Use a duration like `10m` | +| `approvals.dedupe_window must be positive` | The dedupe window is zero or negative | Use a positive duration | +| `approvals.notify.webhook.url must not be empty` | Webhook URL is blank | Provide a URL or remove the webhook block | +| `approvals.notify.webhook.url must use http or https` | Webhook URL uses an unsupported scheme | Use an `http://` or `https://` URL | +| `approvals.notify.webhook.secret must not be empty` | Webhook secret is blank | Provide a secret for HMAC-SHA256 signing | | `duplicate state counter "" (also used by rules[N])` | Two rules on the same tool define the same counter name | Use different counter names or different windows | diff --git a/README.md b/README.md index 88cd692..7ce5667 100644 --- a/README.md +++ b/README.md @@ -20,36 +20,37 @@ Intercept is the open-source control layer for AI agents in production. It sits ## Try it in 30 seconds -See every tool your AI agent has access to — before adding any rules: +See every tool your AI agent has access to, before adding any rules: ```sh npx -y @policylayer/intercept scan -- npx -y @modelcontextprotocol/server-github ``` -This connects to the server, discovers all available tools, and shows you the full attack surface. Then add a policy to lock it down. +Connects to the server, discovers all available tools, and shows the full attack surface. Add a policy to lock it down. ## Why Intercept, not system prompts? | | System prompt | Intercept | |---|---|---| -| **Enforcement** | Probabilistic — model can ignore | Deterministic — blocked at transport layer | -| **Bypassable** | Yes — injection, reasoning, context overflow | No — agent never sees the rules | -| **Stateful** | No — no memory of previous calls | Yes — counters, spend tracking, sliding windows | -| **Auditable** | No — no structured log of decisions | Yes — every allow/deny logged with reason | +| **Enforcement** | Probabilistic, model can ignore | Deterministic, blocked at transport layer | +| **Bypassable** | Yes, via injection, reasoning, context overflow | No, agent never sees the rules | +| **Stateful** | No, no memory of previous calls | Yes, counters, spend tracking, sliding windows | +| **Auditable** | No, no structured log of decisions | Yes, every allow/deny logged with reason | | **Latency** | N/A | <1ms per evaluation | Prompts tell the agent what it should do. Intercept defines what it is allowed to do. ## What it does -- **Block tool calls** — deny dangerous tools unconditionally (e.g. `delete_repository`) -- **Validate arguments** — enforce constraints on tool arguments (`amount <= 500`, `currency in [usd, eur]`) -- **Rate limit** — cap calls per minute, hour, or day with `rate_limit: 5/hour` shorthand -- **Track spend** — stateful counters with dynamic increments (e.g. sum `args.amount` across calls) -- **Hide tools** — strip tools from `tools/list` so the agent never sees them, saving context window tokens -- **Default deny** — allowlist mode where only explicitly listed tools are permitted -- **Hot reload** — edit the policy file while running; changes apply immediately without restart -- **Validate policies** — `intercept validate -c policy.yaml` catches errors before deployment +- **Block tool calls.** Deny dangerous tools unconditionally (e.g. `delete_repository`) +- **Validate arguments.** Enforce constraints on tool arguments (`amount <= 500`, `currency in [usd, eur]`) +- **Rate limit.** Cap calls per minute, hour, or day with `rate_limit: 5/hour` shorthand +- **Track spend.** Stateful counters with dynamic increments (e.g. sum `args.amount` across calls) +- **Hide tools.** Strip tools from `tools/list` so the agent never sees them, saving context window tokens +- **Require approval.** Hold tool calls for human approval via CLI or local HTTP API before execution +- **Default deny.** Allowlist mode where only explicitly listed tools are permitted +- **Hot reload.** Edit the policy file while running; changes apply immediately without restart +- **Validate policies.** `intercept validate -c policy.yaml` catches errors before deployment ## Real-world examples @@ -83,6 +84,21 @@ create_resource: on_deny: "Resource creation rate limit reached" ``` +**It issued a $4,200 refund without asking anyone** + +```yaml +create_refund: + rules: + - name: "large-refund-approval" + action: require_approval + conditions: + - path: "args.amount" + op: "gt" + value: 10000 + on_deny: "Refunds over $100.00 require human approval" + approval_timeout: "30m" +``` + **It emailed 400,000 customers a test template** ```yaml @@ -127,7 +143,7 @@ Download from [GitHub Releases](https://github.com/policylayer/intercept/release intercept scan -o policy.yaml -- npx -y @modelcontextprotocol/server-stripe ``` -This connects to the server, discovers all available tools, and writes a commented YAML file listing each tool with its parameters. +Connects to the server, discovers all tools, and writes a commented YAML file listing each tool with its parameters. **2. Edit the policy to add rules:** @@ -181,11 +197,11 @@ tools: intercept -c policy.yaml --upstream https://mcp.stripe.com --header "Authorization: Bearer sk_live_..." ``` -Intercept proxies all MCP traffic and enforces your policy on every tool call. Hidden tools are stripped from the agent's view entirely. +Intercept proxies all MCP traffic and enforces your policy on every tool call. Hidden tools are stripped from the agent's view. ## Example policies -The `policies/` directory contains ready-made policy scaffolds for 130+ popular MCP servers including GitHub, Stripe, AWS, Notion, Slack, and more. Each file lists every tool with its description, grouped by category (Read, Write, Execute, Financial, Destructive). +The `policies/` directory contains ready-made scaffolds for 130+ MCP servers including GitHub, Stripe, AWS, Notion, and Slack. Each file lists every tool with its description, grouped by risk category. Copy one as a starting point: @@ -252,7 +268,7 @@ intercept -c policy.yaml --state-dsn redis://localhost:6379 --upstream https://m ## Contributing -Contributions welcome — open an issue to discuss what you'd like to change. +Contributions welcome. Open an issue to discuss what you'd like to change. ## License diff --git a/USAGE.md b/USAGE.md index d8f987d..3cbfd6d 100644 --- a/USAGE.md +++ b/USAGE.md @@ -26,6 +26,8 @@ intercept -c [flags] --upstream | `--header` | | string[] | | Custom headers for upstream requests (e.g. `"Authorization: Bearer tok"`) | | `--bind` | | string | `127.0.0.1` | Bind address for HTTP/SSE listener | | `--port` | | int | `0` (auto) | Port for HTTP/SSE listener | +| `--enable-admin-api` | | bool | `false` | Start the local admin API for managing approvals | +| `--admin-addr` | | string | `127.0.0.1:9111` | Bind address for the admin API server | `--state-dir` and `--state-dsn` are mutually exclusive. @@ -153,6 +155,65 @@ No flags. The command reads event files from `~/.intercept/events/` and prints: Sections with no data are omitted. +## Approvals command + +Manages pending approval requests created by `require_approval` rules. See [POLICY.md, Approval rules](POLICY.md#approval-rules) for the policy format. + +``` +intercept approvals [flags] +``` + +### Subcommands + +| Subcommand | Description | +|---|---| +| `list` | List all pending approval requests | +| `show [id]` | Show full details of a single approval | +| `approve [id]` | Approve a pending request (allows the next matching call) | +| `deny [id]` | Deny a pending request | +| `expire --all-stale` | Expire all approvals past their timeout | + +### Flags + +| Flag | Subcommand | Type | Default | Description | +|---|---|---|---|---| +| `--state-dir` | all | string | `~/.intercept/state` | Directory for persistent state | +| `--actor` | `approve` | string | | Identity of the approver | +| `--reason` | `deny` | string | | Reason for denial | +| `--all-stale` | `expire` | bool | `false` | Required flag to confirm expiry | + +### Examples + +```sh +# List pending approvals +intercept approvals list + +# Show details of a specific approval +intercept approvals show apr_abc123 + +# Approve a request +intercept approvals approve apr_abc123 --actor "liad" + +# Deny with a reason +intercept approvals deny apr_abc123 --reason "amount too high" + +# Expire stale approvals +intercept approvals expire --all-stale +``` + +### Admin API endpoints + +When `--enable-admin-api` is set, the proxy exposes a local HTTP API for approval management: + +| Method | Path | Description | +|---|---|---| +| `GET` | `/api/pending` | List all pending approvals | +| `GET` | `/api/pending/{id}` | Get a single approval by ID | +| `POST` | `/api/approve/{id}` | Approve (body: `{"actor": "..."}`) | +| `POST` | `/api/deny/{id}` | Deny (body: `{"reason": "..."}`) | + +Expired approvals return `409 Conflict`. Already-decided approvals return the current state (idempotent). + ## MCP client integration To use Intercept with Claude Code or any MCP client that reads `.mcp.json`, point the server's `command` at Intercept. diff --git a/cmd/approvals.go b/cmd/approvals.go index cd138bb..10b0f4a 100644 --- a/cmd/approvals.go +++ b/cmd/approvals.go @@ -3,7 +3,6 @@ package cmd import ( "context" "fmt" - "os" "path/filepath" "time" @@ -29,8 +28,7 @@ func newApprovalsCmd(opener storeOpener) *cobra.Command { Short: "Manage pending approval requests", } - home, _ := os.UserHomeDir() - defaultStateDir := filepath.Join(home, ".intercept", "state") + defaultStateDir, _ := interceptSubdir("state") approvalsCmd.PersistentFlags().StringVar(&approvalStateDir, "state-dir", defaultStateDir, "directory for persistent state") if opener == nil { diff --git a/cmd/approvals_test.go b/cmd/approvals_test.go index 2296d74..98d7bc3 100644 --- a/cmd/approvals_test.go +++ b/cmd/approvals_test.go @@ -59,6 +59,7 @@ func execApprovals(t *testing.T, store approvals.Store, args ...string) (string, } func seedPendingRecord() approvals.Record { + now := time.Now().UTC() return approvals.Record{ ID: "apr_test001", Fingerprint: "fp_abc123", @@ -69,8 +70,8 @@ func seedPendingRecord() approvals.Record { Status: approvals.StatusPending, Reason: "PR merges require human approval", ArgsHash: "argshash", - CreatedAt: time.Date(2026, 3, 26, 12, 15, 0, 0, time.UTC), - ExpiresAt: time.Date(2026, 3, 26, 12, 30, 0, 0, time.UTC), + CreatedAt: now, + ExpiresAt: now.Add(15 * time.Minute), } } diff --git a/internal/admin/server.go b/internal/admin/server.go index 1355894..c8c460c 100644 --- a/internal/admin/server.go +++ b/internal/admin/server.go @@ -113,58 +113,14 @@ func (s *Server) handleGetByID(w http.ResponseWriter, r *http.Request) { } func (s *Server) handleApprove(w http.ResponseWriter, r *http.Request) { - id := r.PathValue("id") - - // Check current state for idempotency and expiry handling. - rec, err := s.store.Get(r.Context(), id) - if err != nil { - writeError(w, http.StatusNotFound, "approval not found") - return - } - - if rec.Status == approvals.StatusExpired { - writeError(w, http.StatusConflict, "approval has expired") - return - } - - // Already approved or consumed -- idempotent return. - if rec.Status == approvals.StatusApproved || rec.Status == approvals.StatusConsumed { - writeJSON(w, http.StatusOK, toResponse(rec)) - return - } - - // Already denied -- idempotent (different target, but still return as-is). - if rec.Status == approvals.StatusDenied { - writeJSON(w, http.StatusOK, toResponse(rec)) - return - } - - var body struct { - Actor string `json:"actor"` - } - if r.Body != nil { - json.NewDecoder(r.Body).Decode(&body) // ignore errors -- optional - } - - if err := s.store.MarkApproved(r.Context(), id, body.Actor, ""); err != nil { - writeError(w, http.StatusInternalServerError, "approving: "+err.Error()) - return - } - - rec, _ = s.store.Get(r.Context(), id) - - if s.emitter != nil { - s.emitter.Emit(events.Event{ - Type: "approval_approved", - ApprovalID: id, - ApprovalStatus: "approved", - }) - } - - writeJSON(w, http.StatusOK, toResponse(rec)) + s.handleDecision(w, r, "approve") } func (s *Server) handleDeny(w http.ResponseWriter, r *http.Request) { + s.handleDecision(w, r, "deny") +} + +func (s *Server) handleDecision(w http.ResponseWriter, r *http.Request, action string) { id := r.PathValue("id") rec, err := s.store.Get(r.Context(), id) @@ -178,31 +134,40 @@ func (s *Server) handleDeny(w http.ResponseWriter, r *http.Request) { return } - // Already denied, approved, or consumed -- idempotent. + // Already decided -- idempotent return. if rec.Status != approvals.StatusPending { writeJSON(w, http.StatusOK, toResponse(rec)) return } var body struct { + Actor string `json:"actor"` Reason string `json:"reason"` } if r.Body != nil { json.NewDecoder(r.Body).Decode(&body) } - if err := s.store.MarkDenied(r.Context(), id, "", body.Reason); err != nil { - writeError(w, http.StatusInternalServerError, "denying: "+err.Error()) - return + switch action { + case "approve": + if err := s.store.MarkApproved(r.Context(), id, body.Actor, ""); err != nil { + writeError(w, http.StatusInternalServerError, "approving: "+err.Error()) + return + } + case "deny": + if err := s.store.MarkDenied(r.Context(), id, "", body.Reason); err != nil { + writeError(w, http.StatusInternalServerError, "denying: "+err.Error()) + return + } } rec, _ = s.store.Get(r.Context(), id) if s.emitter != nil { s.emitter.Emit(events.Event{ - Type: "approval_denied", + Type: "approval_" + action + "d", ApprovalID: id, - ApprovalStatus: "denied", + ApprovalStatus: action + "d", }) } diff --git a/internal/approvals/fingerprint.go b/internal/approvals/fingerprint.go index 20aa4e3..c2d0bb0 100644 --- a/internal/approvals/fingerprint.go +++ b/internal/approvals/fingerprint.go @@ -12,16 +12,10 @@ import ( // CanonicalJSON produces a deterministic JSON encoding of args by recursively // sorting map keys at every level. func CanonicalJSON(args map[string]any) ([]byte, error) { - sorted := sortedMap(args) + sorted := orderedMap(args) return json.Marshal(sorted) } -// sortedMap recursively converts a map[string]any into an ordered structure -// suitable for deterministic JSON marshaling. -func sortedMap(m map[string]any) json.Marshaler { - return orderedMap(m) -} - // orderedMap marshals a map[string]any with sorted keys. type orderedMap map[string]any @@ -46,23 +40,38 @@ func (o orderedMap) MarshalJSON() ([]byte, error) { buf.WriteByte(':') v := o[k] - // Recursively handle nested maps. - if nested, ok := v.(map[string]any); ok { - valJSON, err := orderedMap(nested).MarshalJSON() - if err != nil { - return nil, err + valJSON, err := marshalSorted(v) + if err != nil { + return nil, err + } + buf.Write(valJSON) + } + buf.WriteByte('}') + return buf.Bytes(), nil +} + +func marshalSorted(v any) ([]byte, error) { + switch val := v.(type) { + case map[string]any: + return orderedMap(val).MarshalJSON() + case []any: + var buf bytes.Buffer + buf.WriteByte('[') + for i, elem := range val { + if i > 0 { + buf.WriteByte(',') } - buf.Write(valJSON) - } else { - valJSON, err := json.Marshal(v) + elemJSON, err := marshalSorted(elem) if err != nil { return nil, err } - buf.Write(valJSON) + buf.Write(elemJSON) } + buf.WriteByte(']') + return buf.Bytes(), nil + default: + return json.Marshal(v) } - buf.WriteByte('}') - return buf.Bytes(), nil } // Fingerprint computes a deterministic SHA-256 hash of a tool call's identity. diff --git a/internal/approvals/fingerprint_test.go b/internal/approvals/fingerprint_test.go index 2e85792..784af92 100644 --- a/internal/approvals/fingerprint_test.go +++ b/internal/approvals/fingerprint_test.go @@ -96,3 +96,31 @@ func TestCanonicalJSONNilMap(t *testing.T) { t.Errorf("expected {}, got %s", string(data)) } } + +func TestFingerprintArrayOfObjects(t *testing.T) { + // Maps inside arrays should be sorted too. + args1 := map[string]any{ + "recipients": []any{ + map[string]any{"name": "alice", "email": "a@b.com"}, + map[string]any{"name": "bob", "email": "b@b.com"}, + }, + } + args2 := map[string]any{ + "recipients": []any{ + map[string]any{"email": "a@b.com", "name": "alice"}, + map[string]any{"email": "b@b.com", "name": "bob"}, + }, + } + + fp1, err := Fingerprint("tool", args1, "rh", "rev") + if err != nil { + t.Fatal(err) + } + fp2, err := Fingerprint("tool", args2, "rh", "rev") + if err != nil { + t.Fatal(err) + } + if fp1 != fp2 { + t.Errorf("fingerprints differ for array-of-objects with reordered keys: %s vs %s", fp1, fp2) + } +} diff --git a/internal/approvals/sqlite.go b/internal/approvals/sqlite.go index d1d1871..fb8c95a 100644 --- a/internal/approvals/sqlite.go +++ b/internal/approvals/sqlite.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "fmt" + "strings" "time" _ "modernc.org/sqlite" @@ -84,13 +85,17 @@ func (s *SQLiteStore) CreatePending(_ context.Context, rec Record) error { func (s *SQLiteStore) Get(_ context.Context, id string) (Record, error) { row := s.db.QueryRow(`SELECT id, fingerprint, tool_name, rule_name, rule_hash, policy_revision, status, reason, args_hash, args_preview_json, created_at, expires_at, decided_at, decided_by, decision_note FROM approvals WHERE id = ?`, id) - return scanRecord(row) + return scanFromRow(row) } -func (s *SQLiteStore) FindByFingerprint(_ context.Context, fingerprint string) (Record, bool, error) { - now := time.Now().UTC().Format(time.RFC3339) - row := s.db.QueryRow(`SELECT id, fingerprint, tool_name, rule_name, rule_hash, policy_revision, status, reason, args_hash, args_preview_json, created_at, expires_at, decided_at, decided_by, decision_note FROM approvals WHERE fingerprint = ? AND status IN ('pending', 'approved') AND expires_at > ? ORDER BY created_at DESC LIMIT 1`, fingerprint, now) - rec, err := scanRecord(row) +func (s *SQLiteStore) FindByFingerprint(_ context.Context, fingerprint string, dedupeWindow time.Duration) (Record, bool, error) { + now := time.Now().UTC() + nowStr := now.Format(time.RFC3339) + createdAfter := now.Add(-dedupeWindow).Format(time.RFC3339) + // Approved records: valid until expires_at (full approval lifetime). + // Pending records: valid until expires_at AND within dedupe window (prevents stale reuse). + row := s.db.QueryRow(`SELECT id, fingerprint, tool_name, rule_name, rule_hash, policy_revision, status, reason, args_hash, args_preview_json, created_at, expires_at, decided_at, decided_by, decision_note FROM approvals WHERE fingerprint = ? AND expires_at > ? AND (status = 'approved' OR (status = 'pending' AND created_at > ?)) ORDER BY created_at DESC LIMIT 1`, fingerprint, nowStr, createdAfter) + rec, err := scanFromRow(row) if err == sql.ErrNoRows { return Record{}, false, nil } @@ -102,16 +107,30 @@ func (s *SQLiteStore) FindByFingerprint(_ context.Context, fingerprint string) ( func (s *SQLiteStore) MarkApproved(_ context.Context, id string, actor string, note string) error { now := time.Now().UTC().Format(time.RFC3339) - _, err := s.db.Exec(`UPDATE approvals SET status = ?, decided_at = ?, decided_by = ?, decision_note = ? WHERE id = ?`, - string(StatusApproved), now, actor, note, id) - return err + res, err := s.db.Exec(`UPDATE approvals SET status = ?, decided_at = ?, decided_by = ?, decision_note = ? WHERE id = ? AND status = ? AND expires_at > ?`, + string(StatusApproved), now, actor, note, id, string(StatusPending), now) + if err != nil { + return err + } + n, _ := res.RowsAffected() + if n == 0 { + return fmt.Errorf("approval %s is not pending or has expired (cannot approve)", id) + } + return nil } func (s *SQLiteStore) MarkDenied(_ context.Context, id string, actor string, note string) error { now := time.Now().UTC().Format(time.RFC3339) - _, err := s.db.Exec(`UPDATE approvals SET status = ?, decided_at = ?, decided_by = ?, decision_note = ? WHERE id = ?`, - string(StatusDenied), now, actor, note, id) - return err + res, err := s.db.Exec(`UPDATE approvals SET status = ?, decided_at = ?, decided_by = ?, decision_note = ? WHERE id = ? AND status = ? AND expires_at > ?`, + string(StatusDenied), now, actor, note, id, string(StatusPending), now) + if err != nil { + return err + } + n, _ := res.RowsAffected() + if n == 0 { + return fmt.Errorf("approval %s is not pending or has expired (cannot deny)", id) + } + return nil } func (s *SQLiteStore) MarkExpired(_ context.Context, now time.Time) (int, error) { @@ -128,26 +147,38 @@ func (s *SQLiteStore) MarkExpired(_ context.Context, now time.Time) (int, error) func (s *SQLiteStore) ConsumeApproved(_ context.Context, fingerprint string, now time.Time) (Record, bool, error) { nowStr := now.UTC().Format(time.RFC3339) - // Atomic update: only updates if status is 'approved' and not expired. - res, err := s.db.Exec(`UPDATE approvals SET status = ? WHERE fingerprint = ? AND status = ? AND expires_at > ?`, - string(StatusConsumed), fingerprint, string(StatusApproved), nowStr) + tx, err := s.db.Begin() + if err != nil { + return Record{}, false, fmt.Errorf("begin tx: %w", err) + } + defer tx.Rollback() + + // Select the single best approved record (most recent, not expired). + row := tx.QueryRow(`SELECT id, fingerprint, tool_name, rule_name, rule_hash, policy_revision, status, reason, args_hash, args_preview_json, created_at, expires_at, decided_at, decided_by, decision_note FROM approvals WHERE fingerprint = ? AND status = ? AND expires_at > ? ORDER BY created_at DESC LIMIT 1`, fingerprint, string(StatusApproved), nowStr) + rec, err := scanFromRow(row) + if err == sql.ErrNoRows { + return Record{}, false, nil + } if err != nil { return Record{}, false, err } - n, err := res.RowsAffected() + + // Update only that specific record by ID. + res, err := tx.Exec(`UPDATE approvals SET status = ? WHERE id = ? AND status = ?`, + string(StatusConsumed), rec.ID, string(StatusApproved)) if err != nil { return Record{}, false, err } + n, _ := res.RowsAffected() if n == 0 { - return Record{}, false, nil + return Record{}, false, nil // raced with another consumer } - // Read back the consumed record. - row := s.db.QueryRow(`SELECT id, fingerprint, tool_name, rule_name, rule_hash, policy_revision, status, reason, args_hash, args_preview_json, created_at, expires_at, decided_at, decided_by, decision_note FROM approvals WHERE fingerprint = ? AND status = ?`, fingerprint, string(StatusConsumed)) - rec, err := scanRecord(row) - if err != nil { - return Record{}, false, err + if err := tx.Commit(); err != nil { + return Record{}, false, fmt.Errorf("commit tx: %w", err) } + + rec.Status = StatusConsumed return rec, true, nil } @@ -165,6 +196,11 @@ func (s *SQLiteStore) List(_ context.Context, filter ListFilter) ([]Record, erro if filter.Status != "" { conditions = append(conditions, "status = ?") args = append(args, string(filter.Status)) + // For pending records, exclude those past their expiry. + if filter.Status == StatusPending { + conditions = append(conditions, "expires_at > ?") + args = append(args, time.Now().UTC().Format(time.RFC3339)) + } } if filter.ToolName != "" { conditions = append(conditions, "tool_name = ?") @@ -172,13 +208,7 @@ func (s *SQLiteStore) List(_ context.Context, filter ListFilter) ([]Record, erro } if len(conditions) > 0 { - query += " WHERE " - for i, c := range conditions { - if i > 0 { - query += " AND " - } - query += c - } + query += " WHERE " + strings.Join(conditions, " AND ") } query += " ORDER BY created_at DESC" @@ -190,7 +220,7 @@ func (s *SQLiteStore) List(_ context.Context, filter ListFilter) ([]Record, erro var records []Record for rows.Next() { - rec, err := scanRecordRows(rows) + rec, err := scanFromRow(rows) if err != nil { return nil, err } @@ -204,13 +234,13 @@ type scannable interface { Scan(dest ...any) error } -func scanRecord(row *sql.Row) (Record, error) { +func scanFromRow(s scannable) (Record, error) { var rec Record var status string var createdAt, expiresAt string var decidedAt sql.NullString - err := row.Scan( + err := s.Scan( &rec.ID, &rec.Fingerprint, &rec.ToolName, &rec.RuleName, &rec.RuleHash, &rec.PolicyRevision, &status, &rec.Reason, &rec.ArgsHash, &rec.ArgsPreviewJSON, &createdAt, &expiresAt, &decidedAt, &rec.DecidedBy, &rec.DecisionNote, @@ -220,35 +250,20 @@ func scanRecord(row *sql.Row) (Record, error) { } rec.Status = Status(status) - rec.CreatedAt, _ = time.Parse(time.RFC3339, createdAt) - rec.ExpiresAt, _ = time.Parse(time.RFC3339, expiresAt) - if decidedAt.Valid { - t, _ := time.Parse(time.RFC3339, decidedAt.String) - rec.DecidedAt = &t + var parseErr error + rec.CreatedAt, parseErr = time.Parse(time.RFC3339, createdAt) + if parseErr != nil { + return Record{}, fmt.Errorf("parse created_at %q: %w", createdAt, parseErr) } - return rec, nil -} - -func scanRecordRows(rows *sql.Rows) (Record, error) { - var rec Record - var status string - var createdAt, expiresAt string - var decidedAt sql.NullString - - err := rows.Scan( - &rec.ID, &rec.Fingerprint, &rec.ToolName, &rec.RuleName, &rec.RuleHash, - &rec.PolicyRevision, &status, &rec.Reason, &rec.ArgsHash, &rec.ArgsPreviewJSON, - &createdAt, &expiresAt, &decidedAt, &rec.DecidedBy, &rec.DecisionNote, - ) - if err != nil { - return Record{}, err + rec.ExpiresAt, parseErr = time.Parse(time.RFC3339, expiresAt) + if parseErr != nil { + return Record{}, fmt.Errorf("parse expires_at %q: %w", expiresAt, parseErr) } - - rec.Status = Status(status) - rec.CreatedAt, _ = time.Parse(time.RFC3339, createdAt) - rec.ExpiresAt, _ = time.Parse(time.RFC3339, expiresAt) if decidedAt.Valid { - t, _ := time.Parse(time.RFC3339, decidedAt.String) + t, parseErr := time.Parse(time.RFC3339, decidedAt.String) + if parseErr != nil { + return Record{}, fmt.Errorf("parse decided_at %q: %w", decidedAt.String, parseErr) + } rec.DecidedAt = &t } return rec, nil diff --git a/internal/approvals/sqlite_test.go b/internal/approvals/sqlite_test.go index cc50b5f..7f4cd49 100644 --- a/internal/approvals/sqlite_test.go +++ b/internal/approvals/sqlite_test.go @@ -72,7 +72,7 @@ func TestFindByFingerprintDedup(t *testing.T) { } s.CreatePending(ctx, rec) - found, ok, err := s.FindByFingerprint(ctx, "fp_dedup") + found, ok, err := s.FindByFingerprint(ctx, "fp_dedup", 15*time.Minute) if err != nil { t.Fatal(err) } @@ -102,7 +102,7 @@ func TestFindByFingerprintExpired(t *testing.T) { } s.CreatePending(ctx, rec) - _, ok, err := s.FindByFingerprint(ctx, "fp_expired") + _, ok, err := s.FindByFingerprint(ctx, "fp_expired", 15*time.Minute) if err != nil { t.Fatal(err) } @@ -363,6 +363,104 @@ func TestListWithStatusFilter(t *testing.T) { } } +func TestConsumeApprovedDuplicateFingerprint(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + now := time.Now().UTC().Truncate(time.Second) + + // Two records with same fingerprint, both approved. + rec1 := Record{ + ID: "apr_dup1", Fingerprint: "fp_dup", ToolName: "tool", + RuleName: "rule", RuleHash: "rh", PolicyRevision: "rev", + Status: StatusPending, CreatedAt: now, ExpiresAt: now.Add(15 * time.Minute), + } + rec2 := Record{ + ID: "apr_dup2", Fingerprint: "fp_dup", ToolName: "tool", + RuleName: "rule", RuleHash: "rh", PolicyRevision: "rev", + Status: StatusPending, CreatedAt: now.Add(time.Second), ExpiresAt: now.Add(15 * time.Minute), + } + s.CreatePending(ctx, rec1) + s.CreatePending(ctx, rec2) + s.MarkApproved(ctx, "apr_dup1", "admin", "") + s.MarkApproved(ctx, "apr_dup2", "admin", "") + + // First consumption should consume exactly one record. + consumed, ok, err := s.ConsumeApproved(ctx, "fp_dup", time.Now()) + if err != nil { + t.Fatal(err) + } + if !ok { + t.Fatal("expected consumption to succeed") + } + + // Verify only the consumed record changed status. + r1, _ := s.Get(ctx, "apr_dup1") + r2, _ := s.Get(ctx, "apr_dup2") + + consumedCount := 0 + if r1.Status == StatusConsumed { + consumedCount++ + } + if r2.Status == StatusConsumed { + consumedCount++ + } + if consumedCount != 1 { + t.Errorf("expected exactly 1 consumed record, got %d (r1=%s, r2=%s)", consumedCount, r1.Status, r2.Status) + } + + // The returned record should be the one that was consumed. + got, _ := s.Get(ctx, consumed.ID) + if got.Status != StatusConsumed { + t.Errorf("returned record %s has status %s, want consumed", consumed.ID, got.Status) + } +} + +func TestMarkApprovedNonPending(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + now := time.Now().UTC().Truncate(time.Second) + + rec := Record{ + ID: "apr_nonpend", Fingerprint: "fp_nonpend", ToolName: "tool", + RuleName: "rule", RuleHash: "rh", PolicyRevision: "rev", + Status: StatusPending, CreatedAt: now, ExpiresAt: now.Add(15 * time.Minute), + } + s.CreatePending(ctx, rec) + s.MarkDenied(ctx, "apr_nonpend", "admin", "nope") + + // Trying to approve a denied record should fail. + err := s.MarkApproved(ctx, "apr_nonpend", "admin", "oops") + if err == nil { + t.Error("expected error when approving a denied record") + } +} + +func TestMarkApprovedExpiredRecord(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + now := time.Now().UTC().Truncate(time.Second) + + rec := Record{ + ID: "apr_stale", Fingerprint: "fp_stale", ToolName: "tool", + RuleName: "rule", RuleHash: "rh", PolicyRevision: "rev", + Status: StatusPending, CreatedAt: now.Add(-20 * time.Minute), + ExpiresAt: now.Add(-5 * time.Minute), // already expired + } + s.CreatePending(ctx, rec) + + // Approving an expired-but-still-pending record should fail. + err := s.MarkApproved(ctx, "apr_stale", "admin", "too late") + if err == nil { + t.Error("expected error when approving an expired record") + } + + // Status should remain pending (not approved). + got, _ := s.Get(ctx, "apr_stale") + if got.Status != StatusPending { + t.Errorf("Status = %q, want pending", got.Status) + } +} + func TestFullLifecycle(t *testing.T) { s := newTestStore(t) ctx := context.Background() @@ -376,14 +474,14 @@ func TestFullLifecycle(t *testing.T) { } s.CreatePending(ctx, rec) - found, ok, _ := s.FindByFingerprint(ctx, "fp_lifecycle") + found, ok, _ := s.FindByFingerprint(ctx, "fp_lifecycle", 15*time.Minute) if !ok || found.Status != StatusPending { t.Fatal("should find pending record") } s.MarkApproved(ctx, "apr_lifecycle", "liad", "approved") - found, ok, _ = s.FindByFingerprint(ctx, "fp_lifecycle") + found, ok, _ = s.FindByFingerprint(ctx, "fp_lifecycle", 15*time.Minute) if !ok || found.Status != StatusApproved { t.Fatal("should find approved record") } diff --git a/internal/approvals/store.go b/internal/approvals/store.go index 906ca70..e0e3b3b 100644 --- a/internal/approvals/store.go +++ b/internal/approvals/store.go @@ -48,7 +48,7 @@ type ListFilter struct { // Store manages approval records. Implementations must be safe for concurrent use. type Store interface { - FindByFingerprint(ctx context.Context, fingerprint string) (Record, bool, error) + FindByFingerprint(ctx context.Context, fingerprint string, dedupeWindow time.Duration) (Record, bool, error) CreatePending(ctx context.Context, rec Record) error MarkApproved(ctx context.Context, id string, actor string, note string) error MarkDenied(ctx context.Context, id string, actor string, note string) error diff --git a/internal/proxy/handler.go b/internal/proxy/handler.go index 9552b88..dd40b9c 100644 --- a/internal/proxy/handler.go +++ b/internal/proxy/handler.go @@ -24,6 +24,8 @@ import ( "github.com/policylayer/intercept/internal/webhook" ) +const defaultApprovalTimeout = 15 * time.Minute + // Handler evaluates tool calls against policy rules, manages stateful // reservations, and produces denied responses for blocked calls. type Handler struct { @@ -31,16 +33,19 @@ type Handler struct { store state.StateStore approvalStore approvals.Store // nil when no require_approval rules cfg atomic.Pointer[config.Config] + policyRevision atomic.Value emitter events.EventEmitter webhookNotifier *webhook.Notifier // nil when not configured + webhookSem chan struct{} } // New creates a Handler. store may be nil if no stateful rules are configured. // approvalStore may be nil to disable approval handling (require_approval falls // back to deny). emitter may be nil to disable event emission. func New(eng *engine.Engine, store state.StateStore, cfg *config.Config, emitter events.EventEmitter, opts ...Option) *Handler { - h := &Handler{engine: eng, store: store, emitter: emitter} + h := &Handler{engine: eng, store: store, emitter: emitter, webhookSem: make(chan struct{}, 10)} h.cfg.Store(cfg) + h.policyRevision.Store(computePolicyRevision(cfg)) for _, o := range opts { o(h) } @@ -67,6 +72,7 @@ func WithWebhookNotifier(n *webhook.Notifier) Option { // SetConfig replaces the active configuration for hot reload. func (h *Handler) SetConfig(cfg *config.Config) { h.cfg.Store(cfg) + h.policyRevision.Store(computePolicyRevision(cfg)) } // ToolListFilter returns a ToolListFilter that removes hidden tools from @@ -87,6 +93,8 @@ func (h *Handler) ToolListFilter() transport.ToolListFilter { func (h *Handler) Handle(req transport.ToolCallRequest) transport.ToolCallResult { cfg := h.cfg.Load() + argsHash := events.HashArgs(req.Arguments) + var result string var ruleName, denyMsg string var approvalEvent *events.Event // additional approval event to emit @@ -99,7 +107,7 @@ func (h *Handler) Handle(req transport.ToolCallRequest) transport.ToolCallResult Type: "tool_call", Tool: req.Name, Result: result, - ArgsHash: events.HashArgs(req.Arguments), + ArgsHash: argsHash, } if result == "denied" { ev.Rule = ruleName @@ -114,7 +122,17 @@ func (h *Handler) Handle(req transport.ToolCallRequest) transport.ToolCallResult if approvalEvent != nil { h.emitter.Emit(*approvalEvent) if h.webhookNotifier != nil && approvalEvent.Type == "approval_requested" { - go h.sendWebhook(req, *approvalEvent, denyMsg) + evCopy := *approvalEvent + msgCopy := denyMsg + select { + case h.webhookSem <- struct{}{}: + go func() { + defer func() { <-h.webhookSem }() + h.sendWebhook(req, evCopy, msgCopy) + }() + default: + slog.Warn("webhook semaphore full, dropping notification", "approval_id", approvalEvent.ApprovalID) + } } } }() @@ -128,7 +146,7 @@ func (h *Handler) Handle(req transport.ToolCallRequest) transport.ToolCallResult // Handle require_approval before the generic deny path. if decision.Type == engine.DecisionRequireApproval { - return h.handleApproval(req, decision, cfg, &result, &ruleName, &denyMsg, &approvalEvent) + return h.handleApproval(req, decision, cfg, argsHash, &result, &ruleName, &denyMsg, &approvalEvent) } if !decision.Allowed { @@ -187,7 +205,7 @@ func (h *Handler) Handle(req transport.ToolCallRequest) transport.ToolCallResult } // handleApproval manages the approval lifecycle for require_approval decisions. -func (h *Handler) handleApproval(req transport.ToolCallRequest, decision engine.Decision, cfg *config.Config, result, ruleName, denyMsg *string, approvalEvent **events.Event) transport.ToolCallResult { +func (h *Handler) handleApproval(req transport.ToolCallRequest, decision engine.Decision, cfg *config.Config, argsHash string, result, ruleName, denyMsg *string, approvalEvent **events.Event) transport.ToolCallResult { *ruleName = decision.Rule ctx := context.Background() @@ -205,7 +223,7 @@ func (h *Handler) handleApproval(req transport.ToolCallRequest, decision engine. } } - policyRevision := computePolicyRevision(cfg) + policyRevision := h.policyRevision.Load().(string) fp, err := approvals.Fingerprint(req.Name, req.Arguments, decision.RuleHash, policyRevision) if err != nil { slog.Error("fingerprint computation failed", "error", err) @@ -217,7 +235,13 @@ func (h *Handler) handleApproval(req transport.ToolCallRequest, decision engine. } } - existing, found, err := h.approvalStore.FindByFingerprint(ctx, fp) + dedupeWindow := defaultApprovalTimeout + if cfg.Approvals != nil && cfg.Approvals.DedupeWindow != "" { + if d, err := time.ParseDuration(cfg.Approvals.DedupeWindow); err == nil { + dedupeWindow = d + } + } + existing, found, err := h.approvalStore.FindByFingerprint(ctx, fp, dedupeWindow) if err != nil { slog.Error("approval store lookup failed", "error", err) *result = "denied" @@ -232,6 +256,12 @@ func (h *Handler) handleApproval(req transport.ToolCallRequest, decision engine. rec, consumed, err := h.approvalStore.ConsumeApproved(ctx, fp, time.Now()) if err != nil { slog.Error("approval consumption failed", "error", err) + *result = "denied" + *denyMsg = "Internal policy error" + return transport.ToolCallResult{ + Handled: true, + Response: buildDeniedResponse(req.ID, "Internal policy error"), + } } if consumed { *approvalEvent = &events.Event{ @@ -306,7 +336,7 @@ func (h *Handler) handleApproval(req transport.ToolCallRequest, decision engine. } // Create new pending record. - timeout := resolveApprovalTimeout(decision.Rule, cfg) + timeout := resolveApprovalTimeout(req.Name, decision.Rule, cfg) reason := decision.Message if reason == "" { reason = fmt.Sprintf("Requires approval: rule %q", decision.Rule) @@ -321,7 +351,7 @@ func (h *Handler) handleApproval(req transport.ToolCallRequest, decision engine. PolicyRevision: policyRevision, Status: approvals.StatusPending, Reason: reason, - ArgsHash: events.HashArgs(req.Arguments), + ArgsHash: argsHash, CreatedAt: now, ExpiresAt: now.Add(timeout), } @@ -340,7 +370,7 @@ func (h *Handler) handleApproval(req transport.ToolCallRequest, decision engine. Type: "approval_requested", Tool: req.Name, Rule: decision.Rule, - ArgsHash: events.HashArgs(req.Arguments), + ArgsHash: argsHash, ApprovalID: rec.ID, ApprovalStatus: string(approvals.StatusPending), Fingerprint: fp, @@ -367,11 +397,11 @@ func computePolicyRevision(cfg *config.Config) string { } // resolveApprovalTimeout determines the approval timeout for a rule. It checks -// the rule's ApprovalTimeout, then the global Approvals.DefaultTimeout, -// then falls back to 15 minutes. -func resolveApprovalTimeout(ruleName string, cfg *config.Config) time.Duration { - // Check per-rule timeout by finding the rule in config. - for _, tool := range cfg.Tools { +// the matching tool's rule ApprovalTimeout first, then the global +// Approvals.DefaultTimeout, then falls back to 15 minutes. +func resolveApprovalTimeout(toolName, ruleName string, cfg *config.Config) time.Duration { + // Check per-rule timeout on the specific tool first. + if tool, ok := cfg.Tools[toolName]; ok { for _, r := range tool.Rules { if r.Name == ruleName && r.ApprovalTimeout != "" { if d, err := time.ParseDuration(r.ApprovalTimeout); err == nil { @@ -380,6 +410,16 @@ func resolveApprovalTimeout(ruleName string, cfg *config.Config) time.Duration { } } } + // Then check wildcard rules. + if wildcard, ok := cfg.Tools["*"]; ok { + for _, r := range wildcard.Rules { + if r.Name == ruleName && r.ApprovalTimeout != "" { + if d, err := time.ParseDuration(r.ApprovalTimeout); err == nil { + return d + } + } + } + } // Check global default. if cfg.Approvals != nil && cfg.Approvals.DefaultTimeout != "" { @@ -388,7 +428,7 @@ func resolveApprovalTimeout(ruleName string, cfg *config.Config) time.Duration { } } - return 15 * time.Minute + return defaultApprovalTimeout } // sendWebhook builds and delivers a webhook payload for an approval event. diff --git a/internal/proxy/handler_test.go b/internal/proxy/handler_test.go index 6627ecf..6f7ad8a 100644 --- a/internal/proxy/handler_test.go +++ b/internal/proxy/handler_test.go @@ -856,7 +856,7 @@ type mockApprovalStore struct { rollbackCalls2 []string } -func (m *mockApprovalStore) FindByFingerprint(ctx context.Context, fp string) (approvals.Record, bool, error) { +func (m *mockApprovalStore) FindByFingerprint(ctx context.Context, fp string, dedupeWindow time.Duration) (approvals.Record, bool, error) { if m.findFn != nil { return m.findFn(ctx, fp) } diff --git a/internal/proxy/response.go b/internal/proxy/response.go index 03ee647..5907e0a 100644 --- a/internal/proxy/response.go +++ b/internal/proxy/response.go @@ -71,7 +71,7 @@ func buildApprovalRequiredResponse(id json.RawMessage, approvalID, toolName, exp ID: id, Error: rpcError{ Code: -32003, - Message: "[INTERCEPT APPROVAL REQUIRED] " + reason, + Message: "[INTERCEPT APPROVAL REQUIRED] " + reason + ". Run 'intercept approvals approve " + approvalID + "' to approve.", Data: &rpcErrorData{ Intercept: &interceptApprovalData{ Type: "approval_required", @@ -88,14 +88,28 @@ func buildApprovalRequiredResponse(id json.RawMessage, approvalID, toolName, exp return data } -// isErrorResponse returns true when data contains a JSON-RPC error (non-null "error" field). -// MCP tool results with isError:true inside "result" are NOT JSON-RPC errors. +// isErrorResponse returns true when data contains a JSON-RPC error (non-null "error" field) +// OR an MCP tool result with isError:true inside the "result" field. func isErrorResponse(data json.RawMessage) bool { var msg struct { - Error json.RawMessage `json:"error"` + Error json.RawMessage `json:"error"` + Result json.RawMessage `json:"result"` } if err := json.Unmarshal(data, &msg); err != nil { return false } - return len(msg.Error) > 0 && string(msg.Error) != "null" + // JSON-RPC level error. + if len(msg.Error) > 0 && string(msg.Error) != "null" { + return true + } + // MCP tool result with isError:true. + if len(msg.Result) > 0 { + var result struct { + IsError bool `json:"isError"` + } + if json.Unmarshal(msg.Result, &result) == nil && result.IsError { + return true + } + } + return false } diff --git a/internal/proxy/response_test.go b/internal/proxy/response_test.go index 03c2ed7..eeec787 100644 --- a/internal/proxy/response_test.go +++ b/internal/proxy/response_test.go @@ -77,6 +77,11 @@ func TestIsErrorResponse(t *testing.T) { { name: "tool result with isError", data: `{"jsonrpc":"2.0","id":1,"result":{"content":[{"type":"text","text":"fail"}],"isError":true}}`, + want: true, + }, + { + name: "tool result with isError false", + data: `{"jsonrpc":"2.0","id":1,"result":{"content":[{"type":"text","text":"ok"}],"isError":false}}`, want: false, }, { diff --git a/vercel.json b/vercel.json deleted file mode 100644 index d59d649..0000000 --- a/vercel.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "outputDirectory": "site" -}