From 93c9b6d96593c2e6a9b078d30b159c828c2c2fc6 Mon Sep 17 00:00:00 2001 From: LIAD Date: Fri, 27 Mar 2026 11:24:53 +0000 Subject: [PATCH 1/6] refactor: simplify approval code, add docs - Cache computePolicyRevision (no per-call JSON marshal) - Unify scanRecord/scanRecordRows via scannable interface - Deduplicate handleApprove/handleDeny into handleDecision - Cache HashArgs (compute once, pass through) - Bound webhook goroutines with semaphore (cap 10) - Use interceptSubdir helper in CLI - Include approval CLI command in error message - Add approval rules to POLICY.md and USAGE.md --- POLICY.md | 72 +++++++++++++++++++++++++++++++++- USAGE.md | 61 +++++++++++++++++++++++++++++ cmd/approvals.go | 4 +- internal/admin/server.go | 75 ++++++++++-------------------------- internal/approvals/sqlite.go | 37 +++--------------- internal/proxy/handler.go | 32 +++++++++++---- internal/proxy/response.go | 2 +- 7 files changed, 184 insertions(+), 99 deletions(-) diff --git a/POLICY.md b/POLICY.md index 991a60e..1b64a41 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,75 @@ 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 + +Global approval settings live under a top-level `approvals` key: + +```yaml +version: "1" + +approvals: + default_timeout: "30m" + webhook_url: "https://example.com/hooks/intercept" + +tools: + # ... +``` + +| Field | Default | Description | +|---|---|---| +| `default_timeout` | `"15m"` | Fallback timeout when a rule omits `approval_timeout` | +| `webhook_url` | | URL to receive `intercept.approval_requested` webhook payloads | + +### Webhook notifications + +When `approvals.webhook_url` is set, Intercept sends a POST request for each new approval request. The payload includes the approval ID, tool name, rule, reason, expiry, fingerprint, args hash, and a safe preview of the top-level arguments (string values truncated to 100 chars, nested objects/arrays redacted). + +### Restrictions + +- `require_approval` needs the approval store enabled (started automatically when the proxy detects approval rules, or when `--enable-admin-api` is set). +- Approval fingerprints include a policy revision hash. Changing the policy file invalidates all pending approvals for affected rules. +- Conditions are optional on `require_approval` rules. Without conditions, every call to the tool requires approval. + ## 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. diff --git a/USAGE.md b/USAGE.md index d8f987d..2e31760 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 appr_abc123 + +# Approve a request +intercept approvals approve appr_abc123 --actor "liad" + +# Deny with a reason +intercept approvals deny appr_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/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/sqlite.go b/internal/approvals/sqlite.go index d1d1871..081e9ae 100644 --- a/internal/approvals/sqlite.go +++ b/internal/approvals/sqlite.go @@ -84,13 +84,13 @@ 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) + rec, err := scanFromRow(row) if err == sql.ErrNoRows { return Record{}, false, nil } @@ -144,7 +144,7 @@ func (s *SQLiteStore) ConsumeApproved(_ context.Context, fingerprint string, now // 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) + rec, err := scanFromRow(row) if err != nil { return Record{}, false, err } @@ -190,7 +190,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,38 +204,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( - &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.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 - } - 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( + 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, diff --git a/internal/proxy/handler.go b/internal/proxy/handler.go index 9552b88..55b3ae5 100644 --- a/internal/proxy/handler.go +++ b/internal/proxy/handler.go @@ -31,16 +31,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 +70,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 +91,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 +105,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 +120,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 +144,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 +203,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 +221,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) @@ -321,7 +337,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 +356,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, diff --git a/internal/proxy/response.go b/internal/proxy/response.go index 03ee647..1948eaf 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", From 4f9035daa600475d7b2f3a1fd500901d2e4cc488 Mon Sep 17 00:00:00 2001 From: LIAD Date: Fri, 27 Mar 2026 11:39:15 +0000 Subject: [PATCH 2/6] fix: guard MarkApproved/MarkDenied to pending-only, honour dedupe_window - MarkApproved/MarkDenied now include WHERE status='pending', preventing re-arming of consumed/denied/expired records from CLI or admin API - FindByFingerprint accepts dedupeWindow param, adds created_at bound so dedupe_window config is actually respected - Fix POLICY.md docs: webhook config uses notify.webhook.url not webhook_url --- POLICY.md | 36 +++++++++++++++++-------------- internal/approvals/sqlite.go | 34 +++++++++++++++++++++-------- internal/approvals/sqlite_test.go | 8 +++---- internal/approvals/store.go | 2 +- internal/proxy/handler.go | 8 ++++++- internal/proxy/handler_test.go | 2 +- 6 files changed, 58 insertions(+), 32 deletions(-) diff --git a/POLICY.md b/POLICY.md index 1b64a41..3be3c6e 100644 --- a/POLICY.md +++ b/POLICY.md @@ -197,33 +197,37 @@ Per-rule timeout controlling how long an approval stays valid. Accepts Go durati ### Top-level approvals block -Global approval settings live under a top-level `approvals` key: - ```yaml -version: "1" - approvals: - default_timeout: "30m" - webhook_url: "https://example.com/hooks/intercept" - -tools: - # ... + default_timeout: 10m + dedupe_window: 10m + notify: + webhook: + url: "https://your-bot.com/intercept" + secret: "your-hmac-secret" ``` -| Field | Default | Description | +| Field | Required | Description | |---|---|---| -| `default_timeout` | `"15m"` | Fallback timeout when a rule omits `approval_timeout` | -| `webhook_url` | | URL to receive `intercept.approval_requested` webhook payloads | +| `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 `approvals.webhook_url` is set, Intercept sends a POST request for each new approval request. The payload includes the approval ID, tool name, rule, reason, expiry, fingerprint, args hash, and a safe preview of the top-level arguments (string values truncated to 100 chars, nested objects/arrays redacted). +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 -- `require_approval` needs the approval store enabled (started automatically when the proxy detects approval rules, or when `--enable-admin-api` is set). -- Approval fingerprints include a policy revision hash. Changing the policy file invalidates all pending approvals for affected rules. -- Conditions are optional on `require_approval` rules. Without conditions, every call to the tool requires approval. +- `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 diff --git a/internal/approvals/sqlite.go b/internal/approvals/sqlite.go index 081e9ae..2ce632b 100644 --- a/internal/approvals/sqlite.go +++ b/internal/approvals/sqlite.go @@ -87,9 +87,11 @@ func (s *SQLiteStore) Get(_ context.Context, id string) (Record, error) { 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) +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) + 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 > ? 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 +104,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 = ?`, + string(StatusApproved), now, actor, note, id, string(StatusPending)) + if err != nil { + return err + } + n, _ := res.RowsAffected() + if n == 0 { + return fmt.Errorf("approval %s is not pending (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 = ?`, + string(StatusDenied), now, actor, note, id, string(StatusPending)) + if err != nil { + return err + } + n, _ := res.RowsAffected() + if n == 0 { + return fmt.Errorf("approval %s is not pending (cannot deny)", id) + } + return nil } func (s *SQLiteStore) MarkExpired(_ context.Context, now time.Time) (int, error) { diff --git a/internal/approvals/sqlite_test.go b/internal/approvals/sqlite_test.go index cc50b5f..1fb9284 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) } @@ -376,14 +376,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 55b3ae5..70fcefa 100644 --- a/internal/proxy/handler.go +++ b/internal/proxy/handler.go @@ -233,7 +233,13 @@ func (h *Handler) handleApproval(req transport.ToolCallRequest, decision engine. } } - existing, found, err := h.approvalStore.FindByFingerprint(ctx, fp) + dedupeWindow := 15 * time.Minute // default + 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" 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) } From 55461d86a2622f09ad595b1a491d52365812ca47 Mon Sep 17 00:00:00 2001 From: LIAD Date: Fri, 27 Mar 2026 11:42:52 +0000 Subject: [PATCH 3/6] fix: dedupe_window applies to pending only, fix doc typos - FindByFingerprint: dedupe_window restricts pending record reuse only; approved records remain valid until their full expires_at lifetime - USAGE.md: fix approval ID prefix (appr_ -> apr_) --- USAGE.md | 6 +++--- internal/approvals/sqlite.go | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/USAGE.md b/USAGE.md index 2e31760..3cbfd6d 100644 --- a/USAGE.md +++ b/USAGE.md @@ -189,13 +189,13 @@ intercept approvals [flags] intercept approvals list # Show details of a specific approval -intercept approvals show appr_abc123 +intercept approvals show apr_abc123 # Approve a request -intercept approvals approve appr_abc123 --actor "liad" +intercept approvals approve apr_abc123 --actor "liad" # Deny with a reason -intercept approvals deny appr_abc123 --reason "amount too high" +intercept approvals deny apr_abc123 --reason "amount too high" # Expire stale approvals intercept approvals expire --all-stale diff --git a/internal/approvals/sqlite.go b/internal/approvals/sqlite.go index 2ce632b..a831993 100644 --- a/internal/approvals/sqlite.go +++ b/internal/approvals/sqlite.go @@ -91,7 +91,9 @@ func (s *SQLiteStore) FindByFingerprint(_ context.Context, fingerprint string, d now := time.Now().UTC() nowStr := now.Format(time.RFC3339) createdAfter := now.Add(-dedupeWindow).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 > ? AND created_at > ? ORDER BY created_at DESC LIMIT 1`, fingerprint, nowStr, createdAfter) + // 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 From f8a754237c608ceff9385b8f615fef12b3b27cce Mon Sep 17 00:00:00 2001 From: LIAD Date: Mon, 30 Mar 2026 12:47:56 +0100 Subject: [PATCH 4/6] fix: address review findings for require_approval - ConsumeApproved: use transaction with SELECT+UPDATE by ID to prevent multi-row consumption on duplicate fingerprints - ConsumeApproved error in handler: return deny instead of falling through to create duplicate pending records - isErrorResponse: detect MCP result.isError in addition to JSON-RPC errors, triggering rollback on upstream tool failures - MarkApproved/MarkDenied: check expires_at to prevent approving naturally expired records ("approved but unusable" state) - List: filter expired pending records so admin API and CLI show only actionable approvals - resolveApprovalTimeout: scope lookup to tool name first, then wildcard, preventing nondeterministic timeout on duplicate rule names - Canonical JSON: recursively sort maps inside arrays for deterministic fingerprinting of array-of-objects arguments - scanFromRow: return errors on malformed timestamps instead of silent zero-value times - Extract defaultApprovalTimeout constant, remove dead sortedMap wrapper, use strings.Join for SQL conditions - Add tests: duplicate fingerprint consumption, approve non-pending, approve expired, array-of-objects fingerprinting, MCP isError - Update POLICY.md validation errors table with 12 approval errors, add require_approval to evaluation order - Update README.md with approval feature and example --- POLICY.md | 14 +++- README.md | 16 +++++ cmd/approvals_test.go | 5 +- internal/approvals/fingerprint.go | 45 +++++++----- internal/approvals/fingerprint_test.go | 28 ++++++++ internal/approvals/sqlite.go | 74 ++++++++++++------- internal/approvals/sqlite_test.go | 98 ++++++++++++++++++++++++++ internal/proxy/handler.go | 34 ++++++--- internal/proxy/response.go | 22 ++++-- internal/proxy/response_test.go | 5 ++ 10 files changed, 282 insertions(+), 59 deletions(-) diff --git a/POLICY.md b/POLICY.md index 3be3c6e..bc7dd00 100644 --- a/POLICY.md +++ b/POLICY.md @@ -395,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. @@ -527,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..` | @@ -545,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..50837dc 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,7 @@ Prompts tell the agent what it should do. Intercept defines what it is allowed t - **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 @@ -83,6 +84,21 @@ create_resource: on_deny: "Resource creation rate limit reached" ``` +**A $4,200 refund was issued without approval** + +```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 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/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 a831993..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" @@ -106,28 +107,28 @@ func (s *SQLiteStore) FindByFingerprint(_ context.Context, fingerprint string, d func (s *SQLiteStore) MarkApproved(_ context.Context, id string, actor string, note string) error { now := time.Now().UTC().Format(time.RFC3339) - res, err := s.db.Exec(`UPDATE approvals SET status = ?, decided_at = ?, decided_by = ?, decision_note = ? WHERE id = ? AND status = ?`, - string(StatusApproved), now, actor, note, id, string(StatusPending)) + 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 (cannot approve)", id) + 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) - res, err := s.db.Exec(`UPDATE approvals SET status = ?, decided_at = ?, decided_by = ?, decision_note = ? WHERE id = ? AND status = ?`, - string(StatusDenied), now, actor, note, id, string(StatusPending)) + 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 (cannot deny)", id) + return fmt.Errorf("approval %s is not pending or has expired (cannot deny)", id) } return nil } @@ -146,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 := scanFromRow(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 } @@ -183,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 = ?") @@ -190,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" @@ -238,10 +250,20 @@ func scanFromRow(s scannable) (Record, error) { } rec.Status = Status(status) - rec.CreatedAt, _ = time.Parse(time.RFC3339, createdAt) - rec.ExpiresAt, _ = time.Parse(time.RFC3339, expiresAt) + 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) + } + rec.ExpiresAt, parseErr = time.Parse(time.RFC3339, expiresAt) + if parseErr != nil { + return Record{}, fmt.Errorf("parse expires_at %q: %w", expiresAt, parseErr) + } 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 1fb9284..7f4cd49 100644 --- a/internal/approvals/sqlite_test.go +++ b/internal/approvals/sqlite_test.go @@ -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() diff --git a/internal/proxy/handler.go b/internal/proxy/handler.go index 70fcefa..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 { @@ -233,7 +235,7 @@ func (h *Handler) handleApproval(req transport.ToolCallRequest, decision engine. } } - dedupeWindow := 15 * time.Minute // default + dedupeWindow := defaultApprovalTimeout if cfg.Approvals != nil && cfg.Approvals.DedupeWindow != "" { if d, err := time.ParseDuration(cfg.Approvals.DedupeWindow); err == nil { dedupeWindow = d @@ -254,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{ @@ -328,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) @@ -389,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 { @@ -402,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 != "" { @@ -410,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/response.go b/internal/proxy/response.go index 1948eaf..5907e0a 100644 --- a/internal/proxy/response.go +++ b/internal/proxy/response.go @@ -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, }, { From dc692b944a4c77c75375c8614185264008f5e2ac Mon Sep 17 00:00:00 2001 From: LIAD Date: Mon, 30 Mar 2026 12:51:31 +0100 Subject: [PATCH 5/6] chore: remove vercel.json, no longer deploying via vercel --- vercel.json | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 vercel.json 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" -} From 71522ad4364481fc40d4c220f8a8ade110eb0b21 Mon Sep 17 00:00:00 2001 From: LIAD Date: Mon, 30 Mar 2026 13:04:42 +0100 Subject: [PATCH 6/6] docs: apply writing style pass to README - remove em dashes from prose and feature list - active voice throughout - parallel construction in feature list (bold label + period + description) - tighten verbose sentences - fix approval example header to match active voice pattern --- README.md | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 50837dc..7ce5667 100644 --- a/README.md +++ b/README.md @@ -20,37 +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 -- **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 +- **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 @@ -84,7 +84,7 @@ create_resource: on_deny: "Resource creation rate limit reached" ``` -**A $4,200 refund was issued without approval** +**It issued a $4,200 refund without asking anyone** ```yaml create_refund: @@ -143,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:** @@ -197,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: @@ -268,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