mirror of
https://github.com/instructkr/claw-code.git
synced 2026-06-05 14:07:11 +08:00
fix: redact MCP server sensitive fields in JSON (#90)
MCP server details JSON now redacts args (shows count only), strips URL query params which may contain tokens, and shows headers_helper as configured/not-configured boolean instead of the raw command string. env_keys and header_keys still exposed (key names only, not values). Generated with https://github.com/Yeachan-Heo/gajae-code Co-authored-by: Gajae Code <dev@gajae-code.com>
This commit is contained in:
@@ -1240,7 +1240,7 @@ Model name prefix now wins unconditionally over env-var presence. Regression tes
|
||||
43. **DONE — Hook ingress opacity: typed hook-health/delivery report missing** — **verified likely external tracking on 2026-04-12:** repo-local searches for `/hooks/health`, `/hooks/status`, and hook-ingress route code found no implementation surface outside `ROADMAP.md`, and the prior state-surface note below already records that the HTTP server is not owned by claw-code. Treat this as likely upstream/server-surface tracking rather than an immediate claw-code task. **Original filing below.**
|
||||
43. **DONE — Hook ingress opacity: typed hook-health/delivery report missing** — dogfooded 2026-04-09 while wiring the agentika timer→hook→session bridge. Debugging hook delivery required manual HTTP probing and inferring state from raw status codes (404 = no route, 405 = route exists, 400 = body missing required field). No typed endpoint exists to report: route present/absent, accepted methods, mapping matched/not matched, target session resolved/not resolved, last delivery failure class. Fix shape: add `GET /hooks/health` (or `/hooks/status`) returning a structured JSON diagnostic — no auth exposure, just routing/matching/session state. Source: gaebal-gajae dogfood 2026-04-09.
|
||||
|
||||
44. **Broad-CWD guardrail is warning-only; needs policy-level enforcement** — dogfooded 2026-04-09. `5f6f453` added a stderr warning when claw starts from `$HOME` or filesystem root (live user kapcomunica scanned their whole machine). Warning is a mitigation, not a guardrail: the agent still proceeds with unbounded scope. Follow-up fix shape: (a) add `--allow-broad-cwd` flag to suppress the warning explicitly (for legitimate home-dir use cases); (b) in default interactive mode, prompt "You are running from your home directory — continue? [y/N]" and exit unless confirmed; (c) in `--output-format json` or piped mode, treat broad-CWD as a hard error (exit 1) with `{"type":"error","error":"broad CWD: running from home directory requires --allow-broad-cwd"}`. Source: kapcomunica in #claw-code 2026-04-09; gaebal-gajae ROADMAP note same cycle.
|
||||
44. **DONE — Broad-CWD guardrail is warning-only; needs policy-level enforcement** — dogfooded 2026-04-09. `5f6f453` added a stderr warning when claw starts from `$HOME` or filesystem root (live user kapcomunica scanned their whole machine). Warning is a mitigation, not a guardrail: the agent still proceeds with unbounded scope. Follow-up fix shape: (a) add `--allow-broad-cwd` flag to suppress the warning explicitly (for legitimate home-dir use cases); (b) in default interactive mode, prompt "You are running from your home directory — continue? [y/N]" and exit unless confirmed; (c) in `--output-format json` or piped mode, treat broad-CWD as a hard error (exit 1) with `{"type":"error","error":"broad CWD: running from home directory requires --allow-broad-cwd"}`. Source: kapcomunica in #claw-code 2026-04-09; gaebal-gajae ROADMAP note same cycle.
|
||||
|
||||
45. **DONE — `claw dump-manifests` fails with opaque "No such file or directory"** — dogfooded 2026-04-09. `claw dump-manifests` emits `error: failed to extract manifests: No such file or directory (os error 2)` with no indication of which file or directory is missing. **Partial fix at `47aa1a5`+1**: error message now includes `looked in: <path>` so the build-tree path is visible, what manifests are, or how to fix it. Fix shape: (a) surface the missing path in the error message; (b) add a pre-check that explains what manifests are and where they should be (e.g. `.claw/manifests/` or the plugins directory); (c) if the command is only valid after `claw init` or after installing plugins, say so explicitly. Source: Jobdori dogfood 2026-04-09.
|
||||
|
||||
@@ -1800,7 +1800,7 @@ Original filing (2026-04-13): user requested a `-acp` parameter to support ACP p
|
||||
|
||||
**Source.** Jobdori dogfood 2026-04-17 against `/tmp/git-state-probe` on main HEAD `9882f07` in response to Clawhip pinpoint nudge at `1494698980091756678`. Eighth member of the truth-audit / diagnostic-integrity cluster after #80, #81, #82, #83, #84, #86, #87 — and the one most directly in scope for the "branch freshness before blame" principle the ROADMAP's preflight section is built around. Distinct from the discovery-overreach cluster (#85, #88): here the workspace surface is not reaching into state it shouldn't — it is failing to report state that lives in plain view inside `.git/`.
|
||||
|
||||
90. **`claw mcp` JSON/text surface redacts MCP server `env` values but dumps `args`, `url`, and `headersHelper` verbatim — standard secret-carrying fields leak to every consumer of the machine-readable MCP surface** — dogfooded 2026-04-17 on main HEAD `64b29f1` from `/tmp/cdB`. The MCP details surface deliberately redacts `env` to `env_keys` (only key names, not values) and `headers` to `header_keys` — a correct design choice. The same surface then dumps `args`, the `url`, and `headersHelper` unredacted, even though all three routinely carry inline credentials.
|
||||
90. **DONE — `claw mcp` JSON/text surface redacts MCP server `env` values but dumps `args`, `url`, and `headersHelper` verbatim — standard secret-carrying fields leak to every consumer of the machine-readable MCP surface** — dogfooded 2026-04-17 on main HEAD `64b29f1` from `/tmp/cdB`. The MCP details surface deliberately redacts `env` to `env_keys` (only key names, not values) and `headers` to `header_keys` — a correct design choice. The same surface then dumps `args`, the `url`, and `headersHelper` unredacted, even though all three routinely carry inline credentials.
|
||||
|
||||
**Three concrete repros, all on one `.claw.json`.**
|
||||
|
||||
|
||||
@@ -5190,34 +5190,51 @@ fn mcp_oauth_json(oauth: Option<&McpOAuthConfig>) -> Value {
|
||||
}
|
||||
|
||||
fn mcp_server_details_json(config: &McpServerConfig) -> Value {
|
||||
// #90: redact sensitive fields — args/url/headers_helper can contain
|
||||
// credentials. Show structure without leaking secrets.
|
||||
match config {
|
||||
McpServerConfig::Stdio(config) => json!({
|
||||
"command": &config.command,
|
||||
"args": &config.args,
|
||||
"args_count": config.args.len(),
|
||||
"env_keys": config.env.keys().cloned().collect::<Vec<_>>(),
|
||||
"tool_call_timeout_ms": config.tool_call_timeout_ms,
|
||||
}),
|
||||
McpServerConfig::Sse(config) | McpServerConfig::Http(config) => json!({
|
||||
"url": &config.url,
|
||||
"header_keys": config.headers.keys().cloned().collect::<Vec<_>>(),
|
||||
"headers_helper": &config.headers_helper,
|
||||
"oauth": mcp_oauth_json(config.oauth.as_ref()),
|
||||
}),
|
||||
McpServerConfig::Ws(config) => json!({
|
||||
"url": &config.url,
|
||||
"header_keys": config.headers.keys().cloned().collect::<Vec<_>>(),
|
||||
"headers_helper": &config.headers_helper,
|
||||
}),
|
||||
McpServerConfig::Sse(config) | McpServerConfig::Http(config) => {
|
||||
let redacted_url = redact_url(&config.url);
|
||||
json!({
|
||||
"url": redacted_url,
|
||||
"header_keys": config.headers.keys().cloned().collect::<Vec<_>>(),
|
||||
"headers_helper_configured": config.headers_helper.is_some(),
|
||||
"oauth": mcp_oauth_json(config.oauth.as_ref()),
|
||||
})
|
||||
}
|
||||
McpServerConfig::Ws(config) => {
|
||||
let redacted_url = redact_url(&config.url);
|
||||
json!({
|
||||
"url": redacted_url,
|
||||
"header_keys": config.headers.keys().cloned().collect::<Vec<_>>(),
|
||||
"headers_helper_configured": config.headers_helper.is_some(),
|
||||
})
|
||||
}
|
||||
McpServerConfig::Sdk(config) => json!({
|
||||
"name": &config.name,
|
||||
}),
|
||||
McpServerConfig::ManagedProxy(config) => json!({
|
||||
"url": &config.url,
|
||||
"url": redact_url(&config.url),
|
||||
"id": &config.id,
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
fn redact_url(url: &str) -> String {
|
||||
// #90: strip query params which may contain tokens, keep scheme+host+path
|
||||
if let Some(query_start) = url.find('?') {
|
||||
format!("{}?...", &url[..query_start])
|
||||
} else {
|
||||
url.to_string()
|
||||
}
|
||||
}
|
||||
|
||||
fn mcp_server_json(name: &str, server: &ScopedMcpServerConfig) -> Value {
|
||||
json!({
|
||||
"name": name,
|
||||
|
||||
Reference in New Issue
Block a user