feat(mcp-proxy): capture action.target resource for non-file actions#855
feat(mcp-proxy): capture action.target resource for non-file actions#855ojongerius wants to merge 3 commits into
Conversation
The dashboard's cross-agent contention graph draws edges from action.target.resource, which only the filesystem hook populated. MCP/network/API/DB tool calls carried no structured resource, so two agents hitting the same API or table drew no contention edge. Add audit.ExtractTarget to derive a stable resource identifier from MCP tool arguments (URI/endpoint, database table, owner/repo, generic resource keys) and thread it through emitToContext into emitter.Event.Target. system is the MCP server name. Extraction is best-effort: arguments matching no shape yield no target, and the pair is always both set or both empty, satisfying the emitter's all-or-nothing Target rule. No schema or daemon change — the daemon already passes frame target fields through to the signed receipt. Closes #852
ExtractTarget capped the resource length but returned serverName as the system unconditionally. The emitter caps Target.System at 256 bytes and rejects the whole frame when exceeded — so a server name over 256 bytes would drop the entire receipt, where before it only populated the uncapped Tool.Server. Guard serverName length too, honouring the best-effort contract that a target must never cost the audit record.
There was a problem hiding this comment.
Pull request overview
Adds best-effort action.target.{system,resource} extraction for MCP tool calls so the dashboard’s cross-agent contention graph can key non-file actions (API endpoints, DB tables, repos) the same way it already keys filesystem actions.
Changes:
- Introduces
audit.ExtractTarget(serverName, toolName, args)with tiered heuristics (URI → table → repo → generic) and length guards. - Wires extracted target through
obsigna-mcp’s pending-call tracking andemitToContext, stamping it onto emitted events for allowed/denied paths. - Adds unit tests for the extractor and extends the daemon integration test to assert
action.targetend-to-end.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| mcp-proxy/internal/audit/target.go | Implements target extraction and URI canonicalisation logic. |
| mcp-proxy/internal/audit/target_test.go | Adds coverage for all tiers, precedence, and guardrails. |
| mcp-proxy/cmd/obsigna-mcp/main.go | Computes/stores target once per tool call and forwards into emitted events. |
| mcp-proxy/cmd/obsigna-mcp/main_test.go | Updates helper test to match new emitToContext signature. |
| mcp-proxy/cmd/obsigna-mcp/emitter_integration_test.go | Asserts action.target materialises in receipts. |
| daemon/CHANGELOG.md | Documents the new behavior in the unified release train changelog. |
| for _, k := range keys { | ||
| if v := stringValue(args[k]); v != "" { | ||
| return v | ||
| } | ||
| } |
There was a problem hiding this comment.
Keeping exact-match here, deliberately. Flattening args into a lowercased map would make the resource nondeterministic: an object carrying both url and URL (distinct JSON keys) would resolve by Go's randomised map-iteration order, and a contention key has to be stable across agents to draw a correct edge. The case-insensitive precedent in classifier.go is for risk scoring (order-independent boolean), where nondeterminism is harmless; this is an identity field where it isn't. In practice MCP servers emit lowercase snake_case argument keys (e.g. github-mcp-server's owner/repo/path), so the miss rate is negligible. If a real server surfaces URL-style keys we can add that specific spelling to the key lists rather than take on the determinism risk.
canonicalURI rebuilt scheme://host/path even when url.Parse left the scheme empty (scheme-relative "//host/path"), yielding a malformed "://host/path". Bail to the trimmed raw value when either scheme or host is absent.
Summary
Closes #852.
The dashboard's cross-agent contention graph draws edges from
action.target.resource, which until now only the filesystem hook populated (native Read/Write/Edit). MCP/network/API/DB tool calls were captured but carried no structured resource, so two agents hitting the same API endpoint, database table, or repo drew no contention edge — exactly the shared-external-state case the attribution story is about (disclosed in #849/#850).This populates
action.target.{system,resource}for MCP tool calls inobsigna-mcp, so shared-API/DB/repo contention surfaces the same way shared-file contention already does.What changed
audit.ExtractTarget(serverName, toolName, args)(mcp-proxy/internal/audit/target.go) — derives a stable resource identifier from a tool call's arguments.systemis the MCP server name;resourceis extracted opportunistically by precedence:url/uri/endpoint) — canonicalised toscheme://host/path, dropping query and fragment so the same endpoint reached with different parameters resolves to one resource.table/collection) — qualified by a neighbouringdatabase/schema/dataset/keyspacekey (e.g.analytics.events).owner+repo→owner/repo) — repo-level granularity, so two agents on the same repo contend even when they target different issues/files.path/key/bucket/object/resource/resource_id). Bareid/nameare deliberately excluded as too generic to be reliable contention keys.emitToContext(mcp-proxy/cmd/obsigna-mcp/main.go) — the target is computed once per tool call, stored on the pending call, and stamped ontoemitter.Event.Targetfor the blocked, denied, and allowed emission paths.Design notes
systemandresourceare always both set or both empty, satisfying the emitter's existing Target rule.validateFrameandbuildAndSignalready pass frame target fields through to the signed receipt (added for filesystem in feat: populate action.target.resource from Claude Code hook tool input #784). Dashboard rendering of the new resource types is downstream of this change.Testing
target_test.gocovers every tier, tier precedence, the guards (nil args, empty server, non-string/whitespace values, oversize resource), and the both-set-or-both-empty invariant.TestEmitToContext_AllowedToolCallto assert the target materialises onaction.targetend-to-end through the daemon.go test ./...andgo vet ./...pass formcp-proxy.