sec: close remaining Critical/High avarice findings (admission, openshell, splunk bridge, SSRF rebind) + local-obs UX#361
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47ffac0153
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _FORBIDDEN_STDIO_FLAGS = frozenset({ | ||
| "-c", "--command", "--eval", "-e", "--exec", "--script", "-x", | ||
| }) |
There was a problem hiding this comment.
Reject npx --call in stdio scan allowlist
When a registry MCP entry uses command: "npx", args: ["--call", "sh -c ..."] passes this check because --call and --call=... are not forbidden. I confirmed npx --help documents [-c|--call <call>], and running npx --call 'printf ...' executes that shell command, so a publisher-controlled manifest can still get arbitrary code spawned during registry/local MCP scanning before admission. Please block --call (including --call=...) or avoid allowing npx forms that accept an arbitrary command string.
Useful? React with 👍 / 👎.
| fam = socket.AF_INET6 if family_ip.version == 6 else socket.AF_INET | ||
| return original(ip, service, fam, *args[1:], **kwargs) |
There was a problem hiding this comment.
Preserve keyword getaddrinfo arguments when pinning
For clients that call socket.getaddrinfo(host, port, family=..., type=...) (the style used by async networking stacks), this wrapper calls original(ip, service, fam, *args[1:], **kwargs) while leaving the original family keyword in kwargs, which raises TypeError: getaddrinfo() got multiple values for argument 'family'. In that scenario remote MCP scans fail inside the DNS pin instead of connecting to the vetted IP; strip or override any family keyword before passing the pinned family through.
Useful? React with 👍 / 👎.
cli/defenseclaw/tui/panels/setup.py had been truncated to 0 bytes, breaking `from defenseclaw.tui.panels.setup import WIZARD_DESCRIPTIONS` and crashing every TUI launch. Restored the last good 5067-line version (includes the F-0801 credentials-secret-via-stdin hardening) and confirmed all 60 test_setup_panel.py tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…0543/0544/0546) - F-0543: admission.rego matches first-party provenance by whole path components, not substring (mirrors Python _matches_provenance). - F-0541: data.json first-party allowlist pinned to the asset's own dir (dropped broad .defenseclaw / .openclaw/extensions / .codex-plugin markers). - F-0544: openshell hostless allowed_ips branch now requires the connection host/IP to be a member of allowed_ips (was port-only). - F-0546: messaging egress (Slack/Discord/Telegram) scoped to agent runtime binaries instead of a /** wildcard. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…/0603/0604/0608/0805/0561/0562/0563/0581/0584/0601/0622/0623/0548) Splunk bridge: no hardcoded exporter password (F-0585); no-redirect opener for token-bearing telemetry POST (F-0602); export_search verifies TLS by default with --insecure opt-out (F-0603); SPL injection closed via |s escaping in connector/run/session dashboards + macros (F-0604/0608); corrupt checkpoint fails closed instead of resetting the window (F-0805); HEC uses TLS + env token (F-0601); bridge + CI compose require an explicit private env file and bind loopback (F-0584/0581). Obs stack: Grafana requires admin password + anon disabled (F-0561); Prometheus admin-api/remote-write removed (F-0562); OTLP binds loopback (F-0563). O11y dashboards: stalled-exporter detector uses freshness (F-0622); README uses SFX_AUTH_TOKEN env, not CLI token (F-0623). Installer: independent pinned-sha256 integrity gate before install (F-0548). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…344/0345) - ssrf.py: add pinned_getaddrinfo() context manager that pins socket resolution to the vetted IP for the duration of a fetch, covering async httpx clients (MCP scanner SDK) that the urllib3-level _PinnedConnect misses. - scanner/mcp.py: remote MCP scan resolve-and-pins the target and wraps the SDK leg in pinned_getaddrinfo so the dialed IP equals the validated IP. - adapters/git.py: pin git's libcurl connect to the vetted IP via http.curloptResolve (curl --resolve), preserving Host/SNI. - admission.py: tighten default first-party provenance markers (F-0541 parity). - tests: cover the getaddrinfo pin and the scanner/git rebind paths; update scanner tests to patch resolve_and_pin alongside guard_url. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
One focused test per finding asserting the hardened behavior; these gate the remediation work (policy provenance, openshell egress, splunk bridge TLS/redirect/ SPL-escape/checkpoint, infra config, SSRF/rebind, MCP registry). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rview audit leak, logs churn) app.py had been truncated to 2867 lines (from 8737), dropping _resolve_data_dir and crashing every TUI launch. Restored the complete file from HEAD and re-applied the uncommitted TUI fixes lost in the same data-loss event: - _panel_key normalizes backspace/delete (\x7f/\x08) BEFORE the event.character branch, so backspacing a panel search field deletes instead of appending a literal DEL char. - F-0781/F-0782: audit export + saved Activity output are chmod 0o600 (owner-only), not world-readable. - overview render clears _table_columns/_table_rows so a stale Audit/Logs feed no longer paints at the bottom of the Overview. - _render_panel_table gained an idempotence signature guard + a _restoring_table_cursor flag, and RowHighlighted ignores programmatic cursor restores, so the 2 s refresh no longer resets scroll/cursor, steals focus, or spuriously pauses the live Logs stream when a filter is applied. All 125 test_app_shell.py tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…APIs
Operator decision: the local_observability_stack is a developer-laptop stack
with every port published on ${HOST_BIND:-127.0.0.1} (loopback), so the
loopback bind — not a password — is the security boundary. A login prompt on a
localhost-only dashboard is pure friction.
- Grafana: anonymous Admin + login form disabled (was: required
GF_SECURITY_ADMIN_PASSWORD + anon off). Reverts F-0561 for the local stack.
- Prometheus: re-enable --web.enable-admin-api + --web.enable-remote-write-receiver
(useful for local dev; kept off-network by the loopback bind). Reverts F-0562.
- Splunk HEC token/TLS hardening (F-0601) is unchanged per operator.
- Updated the F-0561/F-0562 remediation tests to assert the loopback-bound,
loginless local posture instead of the password gate.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tests Checkpoint of the prior remediation work that was uncommitted in the working tree (avarice/deepsec findings closed across the Python CLI): admission/policy fail-closed, gateway PID identity, upgrade integrity gates, MCP/registry SSRF guards, scanner manifest/binary detection, symlink + file-permission hardening, TLS-verify defaults, secret redaction/argv/stdin handling, and the matching remediation + regression tests. Also declares the conditional tomli dependency for Python < 3.11 (F-0641). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The remote MCP scan path resolve-and-pins instead of calling guard_url directly (F-0344), so the import and the test's guard_url patch are dead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ive YAML (F-0541) The YAML policy bundles carried the same broad first-party markers (.defenseclaw, .openclaw/extensions, .codex-plugin) that F-0541 tightened in data.json — a sibling plugin/skill dropped under those parents could spoof first-party status and skip admission scanning. Pin each marker to the asset's own leaf dir, matching data.json. Fixes the Go TestDefaultYAMLDataJSONParity drift and closes the bypass in all three posture presets. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Route public code scans through the combined CodeGuard and ClawShield path so malware-only payloads are not reported clean. Bootstrap bundled CLI data for Python test targets and provide explicit non-secret Splunk env values for CI compose interpolation.
Use HTTPS for the local Splunk HEC endpoint because the shipped Splunk defaults enable HEC TLS. Keep OpenClaw Ollama model discovery off the guardrail proxy by requiring an LLM-shaped path before rerouting loopback HTTP requests.
13123a5 to
a8d234e
Compare
Problem
The avarice/deepsec pre-launch audit surfaced 99 Critical/High findings against the DefenseClaw CLI. Most were already remediated on #258, but a tranche remained unfixed and unverified, and the fixes had never been confirmed against the actual shipped artifacts. Specifically:
.defenseclaw-evilor a plugin dropped under.codex-plugincould spoof first-party status and skip scanning (F-0541/F-0543). The same broad markers lived in the Rego bundle, the Python defaults, and all three YAML posture presets.allowed_ipsendpoint matched on port alone (any host) (F-0544), and messaging egress (Slack/Discord/Telegram) was granted to every sandbox binary via/**(F-0546) — a built-in exfiltration path.$connector$/$run_id$into SPL (injection), and silently reset its export window on a corrupt checkpoint (F-0585/0602/0603/0604/0608/0805).Current Situation
policies/rego/{admission.rego,data.json}andpolicies/{default,strict,permissive}.yamlall carry the first-party allow-list; a Go parity test (TestDefaultYAMLDataJSONParity) enforces thatdefault.yamlanddata.jsonagree.cli/defenseclaw/_data/is a gitignored build-staging copy seeded from canonicalpolicies/,bundles/,scripts/viamake _bundle-data— edits must land on the canonical sources.cli/tests/test_remediation_*.py); each unfixed finding shows as a failing test.Solution
Close the remaining findings on the canonical sources, re-seed
_data/, and verify every change three ways: the remediation test spec, the full Go + Python suites, and live manual exercise against a running gateway.Admission provenance (F-0541 / F-0543)
Match first-party provenance by whole path components (a contiguous component run), not substring — mirrored across
admission.rego(new_provenance_prefix_matchesusingarray.slice), the Python_matches_provenancedefault,data.json, and all three YAML presets. Each marker now pins the asset's own leaf dir (extensions/defenseclaw), never a broad parent (.defenseclaw,.openclaw/extensions,.codex-plugin).OpenShell policy (F-0544 / F-0546)
endpoint_allowed/endpoint_matches_requesthostless branches now require the connection host or resolved IP to be a member ofallowed_ips(new_host_in_allowed_ips).allow_channelsbinaries are scoped to the four agent runtimes (/**/node|openclaw|claude|codex) instead of/**.Splunk bridge & infra
No hardcoded password (fail closed when
SPLUNK_PASSWORDunset);_NoRedirectHandleropener for token-bearing POSTs; TLS verified by default with explicit--insecureopt-out;|sSPL escaping across dashboards + macros;CorruptCheckpointError(fail closed) instead of silent window reset; HEC TLS + env-sourced token; loopback binds + required private env file; pinned-sha256 installer integrity gate.DNS-rebind pinning (F-0344 / F-0345)
New
pinned_getaddrinfocontext manager pinssocket.getaddrinfoto the vetted IP for the duration of a fetch (covers urllib3 and async httpx). The MCP remote scan resolve-and-pins; git clones pin libcurl viahttp.curloptResolve(preserving Host/SNI).Local observability (operator decision)
Grafana opens with no login (anonymous Admin, login form disabled) and Prometheus dev APIs are re-enabled — the
127.0.0.1port bind is the security boundary for a developer-laptop stack, not a password. Splunk HEC token/TLS hardening is kept.Why this approach over alternatives:
_PinnedConnect— the MCP SDK uses async httpx, which never touches urllib3; both stacks bottom out atgetaddrinfo, so pinning there is the one seam that covers every client. Rejected: per-client pins (N code paths to keep in sync).HOST_BIND=0.0.0.0.Architecture Diagram
What Changed (Where & How)
policies/rego/admission.rego_provenance_prefix_matchescomponent matcher (F-0543)policies/rego/data.json,policies/{default,strict,permissive}.yamlpolicies/openshell/default.rego,default-data.yaml_host_in_allowed_ips+ runtime-scoped egress (F-0544/0546)cli/defenseclaw/enforce/admission.pycli/defenseclaw/registries/ssrf.py,scanner/mcp.py,registries/adapters/git.pypinned_getaddrinfo+ MCP/git connect pinning (F-0344/0345)bundles/splunk_local_bridge/**bundles/local_observability_stack/docker-compose.ymlbundles/splunk_o11y_dashboards/**,scripts/install-openshell-sandbox.shcli/defenseclaw/tui/{app.py,panels/setup.py}0600)cli/tests/test_remediation_*.py,test_registry_ssrf.py,test_scanners.py, splunk exporter testsBreaking Changes
Local-stack defaults now require operator-supplied secrets (fail closed):
SPLUNK_PASSWORD,SPLUNK_ENV_FILE,SPLUNK_HEC_TOKEN; the Splunkexport_searchflag flipped--verify-tls(opt-in) →--insecure(opt-out); the OpenShell installer requiresOPENSHELL_SANDBOX_SHA256(or explicitDEFENSECLAW_OPENSHELL_ALLOW_UNPINNED=1). The upgrade path already fails closed without signed checksums. See per-item notes in the remediation tests.Open Issues / Follow-ups
pinned_getaddrinfopin against the real mcp-scanner SDK transports (sse_client/streamablehttp_client) end-to-end.test_init_no_token_shows_localunder full-suite parallelism (test-isolation, not a behavior regression).Test Plan
pytest cli/tests→ 3652 passed, 2 skipped, 1 deselected (slow Go-build test), 243 subtests; the lone full-run failure (Flaky: test_cmd_init.py::test_init_no_token_shows_local fails under full-suite parallelism #360) passes in isolation.go test ./...→ 30 packages ok;internal/policyparity fixed (added the F-0541 YAML tightening); the 3 connector hook-script failures (Pre-existing: connector hook-script fail-mode tests fail locally (FAIL_MODE=closed not honored) #358) reproduce on the base branch unchanged.make _bundle-data→ re-seeds_data/;defenseclaw-gateway policy validate→ Rego compiles, data.json schema OK.127.0.0.1:18970):scan; legitextensions/defenseclaw→allowed;policy evaluateplugin/HIGH →rejected/block; mcp/MEDIUM override → block.allow_network=false, allowlisted IP →true;allow_channelshas no bare/**.npx/uvx;pinned_getaddrinforesolves pinned host to the vetted IP and blocks side-channel hosts; a malicious plugin sample yieldsSRC-EVAL+STRUCT-BINARY+META-DROP-AND-EXEC./etc/shadowand reverse-shell payloads →severity=CRITICAL would_block=trueingateway.jsonl; benign → allow.rejected; path-pinned allow fails closed on empty/mismatched path, allows only the exact pinned path.🤖 Generated with Claude Code