Skip to content

Security hardening, case-path performance, and reliability fixes - #26

Merged
alxxjohn merged 10 commits into
mainfrom
hardening/security-perf-reliability
Jul 2, 2026
Merged

Security hardening, case-path performance, and reliability fixes#26
alxxjohn merged 10 commits into
mainfrom
hardening/security-perf-reliability

Conversation

@alxxjohn

@alxxjohn alxxjohn commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

A codebase-wide audit turned up security, performance, and reliability issues; this PR fixes them. Work was split across three cohesive areas (security, hot-path performance/correctness, CLI/tooling/hygiene) with disjoint file ownership and integrated here.

Static checks on the merged tree are green: go build ./..., go vet ./..., full go test ./... all pass, and -race is clean on the newly-concurrent engine/adapter paths.

Security

  • MCP remote-code-execution / secret exfiltration closed. cleanr_run / cleanr_generate_dataset accepted inline configs with no sandbox, so a prompt-injected agent could run type: cli targets or plugins/state_adapters/probes (all inheriting the full host env, including provider API keys). New toolkit.GuardMCPConfig rejects these on the MCP surface unless CLEANR_MCP_ALLOW_EXEC is set. The CLI path is unchanged.
  • MCP path traversal closed. config_path/dataset_path/etc. are now confined to the working directory (no absolute paths, no ..), and errors no longer echo file contents.
  • Credential SSRF closed. A config could name any env var in api_key_env and have its value sent as a Bearer token to any URL. Provider-secret env vars (OPENAI/ANTHROPIC/AWS/…) are now only sent to an egress allowlist or loopback; every send/refusal is logged.
  • Plugin/WASM env leakage closed. Plugin subprocesses and the wazero sandbox no longer inherit os.Environ() — only declared vars are passed.
  • gRPC transport secured. The adapter dialed plaintext unconditionally. It now defaults to TLS for remote hosts; insecure is limited to loopback or an explicit grpc.plaintext: true.

Performance & correctness

  • SDK zero-timeout bug fixed. Programmatically-built Configs never ran applyDefaults, so Timeout() returned 0 and every request failed instantly with context deadline exceeded. Timeout() now falls back to a sane default.
  • Panic in LoadEngine fixed. A load suite with zero scenarios hit a %len(scenarios) divide-by-zero — now guarded (a panic in the public SDK is an API bug).
  • Bounded worker pool on the case path. Read-heavy engines (prompt_injection, security, token_optimization, release_policy, provenance) now run scenarios concurrently via runBoundedByIndex, limit from Config.Concurrency (default 4), results written by index so ordering stays deterministic. Roughly an order-of-magnitude CI wall-clock win on large suites.
  • Response cache across read-only engines. Multiple engines were each making their own live target call for the same unmodified request (~5× API spend). Read-only engines now share one call per scenario; mutating engines still invoke fresh.
  • Retry/backoff for 429/503 and transport errors (exponential backoff + jitter, honors Retry-After, never exceeds the request deadline).
  • Context cancellation checks in the run loop and per-scenario loops, so a timed-out run stops promptly instead of emitting a wall of spurious "context deadline exceeded" findings.
  • Smaller wins: regex compilation hoisted out of hot loops, API keys resolved once instead of re-reading the profile file per call, judge pool built once per run, descriptive extract errors instead of a bare io.EOF sentinel.

Deferred (documented): llm_judge scenario-loop concurrency is left serial due to stateful sampling; drift/claim_trace/shadow_state stay serial by design.

CLI, tooling & hygiene

  • Report is written before persistence side-effects. Previously a failed trend/replay/attestation write discarded the entire (paid) run's report. Those are now best-effort warnings; the report is written first, and a persistence failure surfaces as a non-zero exit only when the run itself passed.
  • Signal handling + atomic writes. SIGINT/SIGTERM handled gracefully with a partial report on interrupt; trend/snapshot files written via temp-file + rename so an interrupt can't corrupt cleanr.trends.yaml.
  • html report format now validates (it was implemented and CLI-documented but rejected by config validation).
  • Provider-type list deduped to a single source of truth.
  • Structured logging (log/slog) with a -v/--debug flag at the CLI boundary.
  • CI coverage gate now measures ./cleanr/... in addition to ./internal/... (the attestation/signing path was previously unmeasured); threshold kept at a conservative floor with a note to ratchet up.
  • golangci upgraded from default: none to standard + errcheck/staticcheck/bodyclose.
  • Repo hygiene: untracked the checked-in 2.6MB cleanr-dev binary and 936KB coverage.out (both are build artifacts; make rebuilds cleanr-dev from source).

Reviewer notes / behavior changes

  • gRPC now defaults to TLS for non-loopback targets — a remote gRPC test target needs a TLS endpoint or grpc.plaintext: true. Loopback is unaffected.
  • New additive Config.Concurrency field (default 4 via Config.CaseConcurrency()).
  • The stricter golangci config surfaces ~60 pre-existing findings left untouched here (lint is not yet a CI gate) — a follow-up cleanup pass is warranted before promoting lint to gating.
  • Two tests were updated to match intentional behavior changes (descriptive extract error; content-addressed stub for the now-concurrent security engine).
  • Non-obvious behavior is captured in .claude/knowledge/architecture-boundaries.md.

Test plan

  • go build ./...
  • go vet ./...
  • go test ./... (all packages pass)
  • go test -race ./cleanr/engines/... ./cleanr/adapters/... ./tests/engines/... (clean)

🤖 Generated with Claude Code

alxxjohn and others added 10 commits July 2, 2026 11:23
…ion creds & plugin env, gRPC TLS

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ging, CI coverage & lint, hygiene

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…anic fixes

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…security engine

The io.EOF sentinel for a missing response field is now a descriptive error,
and the security engine runs scenarios concurrently, so the ordering-based
sequenceTarget is replaced with a scenario-name-keyed target in the coverage test.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…behavior in repo knowledge

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The repo enforces that all _test.go files live under tests/ (validateGoFileLayout
in internal/devtools). The security agent's in-package white-box tests broke
'make fmt' in CI. Relocated them as black-box tests under tests/{mcp,integrations,plugins}:

- MCP guard + path traversal tested via exported GuardMCPConfig/LoadConfigSource.
- Credential egress: extracted the pure policy into exported CredentialEgressAllowed
  (used by applyAuth) so it is testable without a live HTTP server.
- Plugin env: exported BuildEntryEnv/BuildWASMEnv (pure functions over Entry).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Clears the two blocking semgrep findings introduced by the hardening work:
- gRPC TLS credentials now pin MinVersion to TLS 1.2 (missing-ssl-minversion).
- Retry backoff jitter uses crypto/rand instead of math/rand (math-random-used);
  randomness quality is immaterial for jitter, this just satisfies the scanner.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@alxxjohn
alxxjohn merged commit f83b717 into main Jul 2, 2026
16 checks passed
@alxxjohn
alxxjohn deleted the hardening/security-perf-reliability branch July 13, 2026 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant