feat(proxy,mcp): tool argument and network response DLP hardening (#33)

* feat(mcp): add ExecInspector for trampoline and dangerous pattern detection

* feat(mcp): wire ExecInspector into MCP gateway tool-call path

* feat(proxy): add MITM response DLP scanning

* feat(proxy): load MITM DLP redact rules from store

* docs: describe ExecInspector and MITM response DLP

* feat(cli,telegram): support adding redact rules

* fix(proxy): silence errcheck on deferred decoder Close()

* fix(cli): add explicit return after t.Fatal to satisfy staticcheck SA5011

* fix(test): add return after t.Fatal in more nil-checks for staticcheck SA5011

* ci(lint): exclude SA5011 in test files to stop false positives after t.Fatal

Author: nnemirovsky

.golangci.yml

@@ -23,6 +23,10 @@ linters:
         linters:
           - revive
         text: "avoid package names that conflict"
+      - path: _test\.go
+        linters:
+          - staticcheck
+        text: "SA5011"
 
 formatters:
   enable:

CLAUDE.md

@@ -83,6 +83,7 @@ Two governance layers work together:
 ```
 sluice policy list [--verdict allow|deny|ask|redact] [--db sluice.db]
 sluice policy add allow|deny|ask <destination> [--ports 443,80] [--name "reason"]
+sluice policy add redact <pattern> --replacement "[REDACTED_X]" [--name "reason"]
 sluice policy remove <id>
 sluice policy import <path.toml>    # seed DB from TOML (merge semantics)
 sluice policy export                # dump current rules as TOML
@@ -190,7 +191,38 @@ Extends phantom swap to handle OAuth credentials bidirectionally. Static credent
 
 **Per-request policy evaluation** applies to HTTP/HTTPS, gRPC-over-HTTP/2, and QUIC/HTTP3. Policy is re-evaluated for every HTTP request (or HTTP/2 stream, or HTTP/3 request), so "Allow Once" permits a single request and subsequent requests on the same connection re-trigger the approval flow. When a per-request approval resolves to "Always Allow" or "Always Deny", the `RequestPolicyChecker` persists the new rule to the policy store via its `PersistRuleFunc` callback and swaps in a freshly compiled engine, so subsequent requests match via the fast path instead of re-entering the approval flow. A fast path skips per-request checks when the SOCKS5 CONNECT matched an explicit allow rule (`RuleMatch`, not default verdict) so normally allowed destinations incur no extra overhead. WebSocket, SSH, and IMAP/SMTP remain connection-level on purpose: per-message or per-command policy on those would blow past the broker's 5/min per-destination rate limit and break normal usage.
 
-**MITM library:** HTTPS interception uses go-mitmproxy (`github.com/lqqyt2423/go-mitmproxy`). The `SluiceAddon` struct in `internal/proxy/addon.go` implements go-mitmproxy's `Addon` interface. `Requestheaders` fires per HTTP/2 stream, giving true per-request policy for gRPC and other HTTP/2 traffic. `Request` handles credential injection (three-pass phantom swap). `Response` handles OAuth token interception.
+**MITM library:** HTTPS interception uses go-mitmproxy (`github.com/lqqyt2423/go-mitmproxy`). The `SluiceAddon` struct in `internal/proxy/addon.go` implements go-mitmproxy's `Addon` interface. `Requestheaders` fires per HTTP/2 stream, giving true per-request policy for gRPC and other HTTP/2 traffic. `Request` handles credential injection (three-pass phantom swap). `Response` handles OAuth token interception and response DLP scanning.
+
+**Response DLP** (`internal/proxy/response_dlp.go`, wired via `SluiceAddon.Response` in `internal/proxy/addon.go`) scans HTTPS response bodies and header values for credential patterns using `InspectRedactRule` regexes from the policy store. Redact rules can be managed via CLI (`sluice policy add redact <pattern> --replacement "..."`), Telegram (`/policy redact <pattern> [replacement]`), or HTTP API (`POST /api/rules` with `verdict="redact"`).
+
+* Complements phantom token stripping. Phantom stripping protects outbound requests so real credentials never leak to upstreams. Response DLP protects inbound responses so real credentials from upstream bodies (echoed auth headers in API errors, debug endpoints leaking env vars, misconfigured services returning secrets) never reach the agent.
+* Header scan runs unconditionally. Headers are scanned regardless of content type and regardless of whether the body scan later succeeds. A decompression failure or a binary Content-Type cannot suppress redaction of a header-borne leak.
+* Body scan skips binary content. `image/*`, `video/*`, `audio/*`, `application/octet-stream`, `application/pdf`, `application/zip`, and `font/*` responses skip the body pass.
+* Hop-by-hop headers are never mutated. `Connection`, `Transfer-Encoding`, `Keep-Alive`, etc. are left alone. When the body is rewritten, `Transfer-Encoding` is stripped and `Content-Length` rewritten so the agent receives a well-framed response.
+* Compressed bodies are decoded. A safe wrapper around go-mitmproxy's `ReplaceToDecodedBody` handles single-value `Content-Encoding: gzip | br | deflate | zstd` (all four have unit tests), multi-value `Content-Encoding: gzip, identity` (identity tokens are stripped then the remaining single encoding is decoded), and stacked encodings like `gzip, br` (rejected as unsupported, body scan skipped with a warning log so a still-compressed body is never scanned as plaintext). The wrapper restores the original `Content-Encoding` header values on decode failure so callers see a consistent pre-state on error.
+* Oversized bodies fail-open. Bodies over `maxProxyBody` (16 MiB) skip the body scan because the data already left the upstream.
+* Streamed responses are not scanned. `f.Stream=true` skips the `Response` addon callback, which go-mitmproxy sets automatically for `text/event-stream` (SSE, LLM streaming) and for bodies above `StreamLargeBodies` (default 5 MiB). `StreamResponseModifier` emits a one-shot WARNING per client connection when DLP rules are configured and the stream path fires (deduped by `dlpStreamWarned` sync.Map, keyed by client connection id). When the connection state is unavailable (`f.ConnContext` or `f.ConnContext.ClientConn` nil, rare defensive case), the warning falls back to a non-dedup log so the bypass notification is never silently suppressed. See "Known limitation: streaming bypass" below.
+* Audit event. Redactions emit a `response_dlp_redact` audit action whose `Reason` field is formatted as `rule1=count1,rule2=count2` so ops can distinguish one Bearer token from fifty AWS keys. No-match scans emit a rate-limited debug log (one line per 500 scans).
+* Rule loading. Rules are loaded at startup via `SluiceAddon.SetRedactRules` (all-or-nothing compile: if any rule pattern fails, the old rule set stays in place) and hot-reloaded on SIGHUP through `Server.UpdateInspectRules`, with lock-free swap via `atomic.Pointer`.
+
+**Known limitation: streaming bypass.** Two response classes bypass Response DLP entirely:
+
+1. **Server-Sent Events** (any response with `Content-Type: text/event-stream`). Used by SSE endpoints, LLM streaming completions, etc.
+2. **Bodies larger than `StreamLargeBodies`** (default 5 MiB). Anything between 5 MiB and `maxProxyBody` (16 MiB) lands here. Bodies over 16 MiB also bypass via the oversized-body path described above.
+
+go-mitmproxy sets `f.Stream=true` for these classes and skips the `Response` addon callback that runs DLP scanning. Sluice substitutes a `StreamResponseModifier` that handles OAuth token swapping (small token bodies are buffered) and emits one of two log lines per client connection when DLP rules are configured:
+
+```
+[ADDON-DLP] WARNING: streaming response bypasses DLP for <host> (<N> rules configured)
+```
+
+or, when `f.ConnContext` is nil and dedup cannot be applied:
+
+```
+[ADDON-DLP] WARNING: streaming response bypasses DLP for <host> (<N> rules configured; connection state unavailable, dedup disabled)
+```
+
+**Operator guidance.** Treat these warning lines as a credential-leak monitoring signal. Pipe sluice's stderr/stdout to a log aggregator (Loki, Datadog, CloudWatch, etc.) and alert on the substring `[ADDON-DLP] WARNING`. The host field tells you which upstream is hot-pathed past DLP so you can decide whether to deny the destination, route around it, or accept the risk. The rule count tells you what would have been redacted had the body been buffered. Implementing chunked stream-aware scanning is on the future-work list (see `docs/plans/completed/20260405-tool-network-dlp-hardening.md`); until then, log-based alerting is the operator's only signal that a credential pattern may have flowed to the agent through a streaming response.
 
 **QUIC per-request:** `EvaluateQUICDetailed` returns Ask when an ask rule matches and falls back to the engine's configured default verdict (not hardcoded Deny). The UDP dispatch loop creates a `RequestPolicyChecker` and passes it to `buildHandler`, which calls `CheckAndConsume` per HTTP/3 request. When the default verdict is "allow", a per-request checker is still attached (with seed credits of 1) so long-lived QUIC sessions re-evaluate policy on subsequent requests.
 
@@ -224,6 +256,8 @@ Two-phase detection: port-based guess first, then byte-level for non-standard po
 
 Optional. JSON lines with blake3 hash chain (`prev_hash` field). Genesis hash: blake3(""). Recovers chain across restarts by reading last line. `sluice audit verify` walks log and reports broken links.
 
+Action names operators commonly grep for: `tool_call` (MCP tool call policy verdict), `inspect_block` (ContentInspector argument block), `exec_block` (ExecInspector trampoline/dangerous-command/env-override block), `response_dlp_redact` (MITM HTTPS response body or header redacted by InspectRedactRule), `inject` (phantom token injected into outbound request), and `deny` (network connection denied at SOCKS5 or SNI layer).
+
 ### MCP gateway
 
 Three upstream transports: stdio (child processes), Streamable HTTP, WebSocket. All satisfy `MCPUpstream` interface. Tools namespaced as `<upstream>__<tool>`. Policy evaluation: deny/allow/ask priority. `ContentInspector` blocks arguments and redacts responses using regex (JSON parsed before matching to prevent unicode escape bypass). Per-upstream timeouts (default 120s).
@@ -232,6 +266,10 @@ Three upstream transports: stdio (child processes), Streamable HTTP, WebSocket.
 
 Agent connection: OpenClaw is configured once (via `openclaw mcp set`) to connect to `http://sluice:3000/mcp`. Sluice's `SelfBypass` auto-allows connections to its own MCP listener so the traffic is not policy-checked.
 
+**ExecInspector** (`internal/mcp/exec_inspect.go`) adds structural exec-argument inspection for tools whose names match configurable globs (defaults: `*exec*`, `*shell*`, `*run_command*`, `*terminal*`). It runs in `HandleToolCall` after the ContentInspector argument check and before the Ask/approval flow (exec-block is a hard deny: a dangerous command should not be presented to a human for approval). It detects trampoline patterns (`bash -c`, `sh -c`, `zsh -c`, `python[23]? -c`, `ruby -e`, `perl -e`, `node -e`, and combined-short-flag variants like `bash -ce` / `bash -ec` / `sh -xc`), shell metacharacters (`|`, `;`, `&`, `$`, `<`, `>`, backticks) in non-shell tools, dangerous commands (`rm -rf /`, `chmod 777` including `chmod 0777` octal and the full setuid/setgid/sticky combined-bit range `[0-7]?777` which covers 1777, 2777, 3777, 4777, 5777, 6777, 7777, `curl | sh/bash/python/ruby/perl/node/php/fish`, `wget | sh`, `dd if=/dev/`, `mkfs`), and blacklisted env overrides (`GIT_SSH_COMMAND`, `LD_PRELOAD`, `LD_LIBRARY_PATH`, `DYLD_*`) matched case-insensitively (via `strings.EqualFold`, also whitespace-trimmed before comparison so padded keys like ` GIT_SSH_COMMAND ` cannot bypass) and recursively scanned through the full arg tree under any env-style slot (`env`, `envs`, `env_vars`, `envvars`, `environment`, `environments`, `environment_variables`, `environmentvariables`, `vars`). Command-string scanning is field-scoped: preferred command slots (`command`, `cmd`, `script`, `code`, `args`, `arguments`, `argv`) are always scanned, plus known smuggle slots (`input`, `stdin`, `body`, `data`, `payload`) when any preferred slot is present. Prose fields (`description`, `notes`, `comment`, `documentation`, `summary`, `title`, `name`) are never scanned because legitimate tool metadata can mention `bash -c` or `rm -rf /` as example text and would false-positive. Top-level non-object payloads (arrays, strings) are scanned as a whole because there is no field structure to lean on. Returned command strings are sorted before inspection so the first-match category is deterministic across runs. Dedicated shell tools (matched by the anchored globs `*__shell`, `*__bash`, or literal `shell`/`bash`) skip the metacharacter check because legitimate shell invocations contain `$`, `|`, etc. (e.g. `echo $HOME`). Trampoline and dangerous-command checks still apply. Because the shell-tool globs are anchored on `__`, tools like `github__shellcheck` and `vim__bashsyntax` will still receive the metacharacter check despite the substring match on the broader ExecTool globs (`*shell*`). That is by design: shellcheck is a linter, not a shell, so it must not get the shell-tool metachar bypass.
+
+Wired in both production entry points via `mcp.NewExecInspector(nil)` which compiles the default patterns. The two entry points are `cmd/sluice/main.go` (the `sluice` command, which runs the full proxy plus MCP gateway) and `cmd/sluice/mcp.go` (the `sluice mcp` subcommand, which runs only the MCP gateway standalone). Both need wiring so the standalone mode is not silently missing exec inspection. A block emits an `exec_block` audit event with `Reason` set to `category:match` (e.g. `trampoline:bash -c` or `env_override:GIT_SSH_COMMAND`) for forensics, then returns an error ToolResult. This is separate from ContentInspector because exec inspection needs structural understanding of command arguments rather than pattern matching on arbitrary text.
+
 ### Vault providers
 
 Seven providers via `Provider` interface. `NewProviderFromConfig` reads from SQLite config singleton:

README.md

@@ -304,9 +304,17 @@ curl -X POST http://localhost:3000/api/credentials \
   -d '{"name":"openai_oauth","type":"oauth","token_url":"https://auth.example.com/token","access_token":"at-xxx","refresh_token":"rt-xxx","destination":"api.openai.com","env_var":"OPENAI_API_KEY"}'
 ```
 
+## Data Loss Prevention
+
+Two complementary inspection layers protect against credential leakage and dangerous tool use:
+
+**Exec argument inspection** (MCP layer): Tools whose names match `*exec*`, `*shell*`, `*run_command*`, or `*terminal*` patterns are scanned for trampoline interpreters (`bash -c`, `python -c`, `node -e`, ...), dangerous commands (`rm -rf /`, `chmod 777`/`chmod 0777`, `curl | sh` piped to any shell or scripting language, `dd if=/dev/`, `mkfs`), and blacklisted env overrides (`GIT_SSH_COMMAND`, `LD_PRELOAD`, `DYLD_*`). Blocks emit an `exec_block` audit event. Dedicated shell tools still accept legitimate `$VAR` expansion.
+
+**Response DLP** (MITM layer): HTTPS response bodies and headers are scanned for credential patterns defined via `[[redact]]` rules in policy. Matches are redacted before the response reaches the agent. Catches credentials echoed in API errors, leaked by debug endpoints, or returned by misconfigured services. Supports `gzip`, `br`, `deflate`, and `zstd` compressed bodies (decompressed before scanning, recompressed headers stripped). Binary content types (images, fonts, archives) skip scanning. Redactions emit a `response_dlp_redact` audit event.
+
 ## Audit Log
 
-Tamper-evident JSON Lines log with blake3 hash chaining. Every connection, tool call, approval, and denial is recorded.
+Tamper-evident JSON Lines log with blake3 hash chaining. Every connection, tool call, approval, and denial is recorded. Common action names include `tool_call`, `inspect_block`, `exec_block`, `response_dlp_redact`, and `inject`.
 
 ```bash
 sluice audit verify   # check hash chain integrity

cmd/sluice/cert_test.go

@@ -50,6 +50,7 @@ func TestCertGenerate(t *testing.T) {
 	certBlock, _ := pem.Decode(certData)
 	if certBlock == nil {
 		t.Fatal("ca-cert.pem is not valid PEM")
+		return
 	}
 	if certBlock.Type != "CERTIFICATE" {
 		t.Errorf("unexpected PEM type: %s", certBlock.Type)
@@ -58,6 +59,7 @@ func TestCertGenerate(t *testing.T) {
 	keyBlock, _ := pem.Decode(keyData)
 	if keyBlock == nil {
 		t.Fatal("ca-key.pem is not valid PEM")
+		return
 	}
 	if keyBlock.Type != "EC PRIVATE KEY" {
 		t.Errorf("unexpected key PEM type: %s", keyBlock.Type)

cmd/sluice/cred_test.go

@@ -913,6 +913,7 @@ func TestHandleCredAddOAuth(t *testing.T) {
 	}
 	if meta == nil {
 		t.Fatal("expected credential_meta row")
+		return
 	}
 	if meta.CredType != "oauth" {
 		t.Errorf("meta cred_type = %q, want %q", meta.CredType, "oauth")
@@ -1032,6 +1033,7 @@ func TestHandleCredAddOAuthWithoutDestination(t *testing.T) {
 	}
 	if meta == nil {
 		t.Fatal("expected credential_meta even without --destination")
+		return
 	}
 	if meta.CredType != "oauth" {
 		t.Errorf("cred_type = %q, want oauth", meta.CredType)
@@ -1185,6 +1187,7 @@ func TestHandleCredAddOAuthCreationFlow(t *testing.T) {
 	}
 	if meta == nil {
 		t.Fatal("expected credential_meta row")
+		return
 	}
 	if meta.CredType != "oauth" {
 		t.Errorf("meta cred_type = %q, want oauth", meta.CredType)

cmd/sluice/main.go

@@ -487,6 +487,15 @@ func main() {
 			}
 		}
 
+		// Wire the exec argument inspector with default tool name patterns
+		// (*exec*, *shell*, *run_command*, *terminal*). Blocks trampoline
+		// patterns, dangerous commands, and GIT_SSH_COMMAND-style env
+		// overrides before the tool call reaches the upstream.
+		execInspector, execErr := mcp.NewExecInspector(nil)
+		if execErr != nil {
+			log.Fatalf("create MCP exec inspector: %v", execErr)
+		}
+
 		var credResolver mcp.CredentialResolver
 		if provider != nil {
 			credResolver = func(name string) (string, error) {
@@ -504,6 +513,7 @@ func main() {
 			Upstreams:          mcpUpstreams,
 			ToolPolicy:         toolPolicy,
 			Inspector:          mcpInspector,
+			ExecInspector:      execInspector,
 			Audit:              logger,
 			Broker:             broker,
 			TimeoutSec:         eng.TimeoutSec,

cmd/sluice/mcp.go

@@ -179,6 +179,15 @@ func handleMCPGateway(args []string) error {
 			len(eng.InspectBlockRules), len(eng.InspectRedactRules))
 	}
 
+	// Wire the exec argument inspector with default tool name patterns
+	// (*exec*, *shell*, *run_command*, *terminal*). Blocks trampoline
+	// patterns, dangerous commands, and GIT_SSH_COMMAND-style env
+	// overrides before the tool call reaches the upstream.
+	execInspector, err := mcp.NewExecInspector(nil)
+	if err != nil {
+		return fmt.Errorf("create exec inspector: %w", err)
+	}
+
 	// Build credential resolver so vault: prefixed env values in upstream
 	// configs are resolved to real credentials.
 	var credResolver mcp.CredentialResolver
@@ -206,6 +215,7 @@ func handleMCPGateway(args []string) error {
 		Upstreams:          upstreams,
 		ToolPolicy:         toolPolicy,
 		Inspector:          inspector,
+		ExecInspector:      execInspector,
 		Audit:              logger,
 		Broker:             broker,
 		TimeoutSec:         eng.TimeoutSec,

cmd/sluice/mcp_test.go

@@ -991,3 +991,40 @@ func TestMCPGatewayStoreBackedUpstreams(t *testing.T) {
 		t.Errorf("expected default timeout 120, got %d", fsUpstream.TimeoutSec)
 	}
 }
+
+// TestDefaultExecInspectorConstructs verifies that the production code
+// paths (cmd/sluice/main.go and cmd/sluice/mcp.go) can construct a default
+// ExecInspector without error. If NewExecInspector(nil) ever starts
+// erroring on default patterns, this smoke test fails and prevents the
+// CRITICAL regression where ExecInspector silently gets nil-ed out in
+// production.
+//
+// This test checks ONLY that NewExecInspector(nil) succeeds and the
+// returned inspector can ShouldInspect + Inspect a simple case. It does
+// NOT exercise wiring into NewGateway. The wiring through Gateway is
+// covered end-to-end by TestGatewayExecInspector* in the mcp package,
+// which constructs a real Gateway and asserts the block path executes.
+// The historical name was misleading (it implied wiring verification);
+// this rename makes the scope explicit.
+func TestDefaultExecInspectorConstructs(t *testing.T) {
+	ei, err := mcp.NewExecInspector(nil)
+	if err != nil {
+		t.Fatalf("default ExecInspector construction failed: %v", err)
+	}
+	if ei == nil {
+		t.Fatal("NewExecInspector(nil) returned nil inspector with no error")
+	}
+
+	// Basic sanity: a trampoline pattern must be blocked for an
+	// exec-matching tool name. If defaults ever drift, this catches it.
+	if !ei.ShouldInspect("sandbox__exec") {
+		t.Error("default ExecInspector does not match *exec* tools")
+	}
+	res := ei.Inspect("sandbox__exec", []byte(`{"command":"bash -c 'evil'"}`))
+	if !res.Blocked {
+		t.Error("default ExecInspector should block trampoline patterns")
+	}
+	if res.Category != "trampoline" {
+		t.Errorf("expected category trampoline, got %q", res.Category)
+	}
+}