feat: v0.1.0 feature wave — SSRF guard, stderr capture, doctor, fixtures#2
Merged
Merged
Conversation
Reverses ADR 0007's deferral of an operator-facing host allowlist:
an attacker-supplied URL could still reach any internal endpoint
resolvable from the test host. Adds exact-match [server].allowed_hosts
plus an always-on private/loopback/link-local/ULA/reserved IP-literal
block (incl. the SSE server-provided endpoint URL), with an allowlist
escape hatch for local testing. A review-found IPv6 bypass (url
host_str() returns bracketed [::1] so parse::<IpAddr>() never matched)
is closed by classifying via the typed url::Host enum.
Breaking, absorbed pre-publish: {Http,Sse,Ws}Transport::connect
take &HostGuard; HostGuard is public. Documented in ADR 0012.
When mcp-loadtest runs under an MCP-aware agent, the target server's stderr blended into the agent's view. Adds run --capture-stderr (to runs/<id>/server-stderr.log) and --tee-stderr (file + live passthrough) via a new SpawnOptions/StderrMode API and Session::spawn_with; the stable 2-arg Session::spawn is unchanged (delegates). The tee pump is a cancellation-aware JoinHandle task that flushes before every exit. Folds F1's HostGuard threading through build_session. stdio.rs kept <300 LoC by splitting out stderr_pump.rs + stdio_line_reader.rs. Breaking, absorbed pre-publish: StdioTransport::spawn is now async; cmd_run::run_from_config takes capture/tee flags. ADR 0013.
Implements the DESIGN §21 AI-friendliness pillar so agents can self-diagnose and plan invocations. doctor runs 4 best-effort checks (Python on PATH, optional --server initialize smoke with captured stderr, stale runs/, Windows MSVC/GNU mismatch) and exits non-zero on any failure. --explain prints static per-subcommand algorithm text; it is serviced by a pre-clap args scan so it works without a subcommand's required args (run --explain needs no --config). ErrorHint maps the lib error enums to a one-line next step, printed at the CLI anyhow boundary so the library surface stays clean. ADR 0014. Also wires F2's --capture-stderr/--tee-stderr flags into Cmd::Run.
DESIGN §16 specced 10 fixtures; v0.1 shipped 6. Adds the remaining 4 to unblock scenario coverage: mock-leak (10 KB/call RSS growth -> soak leak detector), mock-error (cycles -32601/-32602/ -32603 -> error classification), mock-slow-init (5 s initialize -> cold_start v0.2), mock-malformed (newline-terminated broken JSON every 10th -> Malformed not Timeout). new_fixtures.rs pins their contracts; mock-slow-init only pins the fixture (cold_start stays inert in v0.1). DESIGN §16.7-16.10 drop the planned-for-v0.2 tag.
CHANGELOG [Unreleased]: the 4 features, the breaking changes absorbed pre-publish, a Security entry for the SSRF surface (incl. the IPv6 host_str bypass found+fixed in review), and the 0007 deferral note flipped to superseded-by-0012. docs/adr/README index gains rows 0012-0014.
ci-checks' `cargo doc -D warnings` step failed: `rustdoc::private_intra_doc_links` — public docs for `allowed_hosts`, `spawn_with`, and the `doctor` module linked to private items (`validate`, `stderr_pump`, the cmd_doctor check submodules). Converted the intra-doc links to plain code spans; wording and the public API are unchanged. Caught by /release-checks (clippy + tests were green but rustdoc was not run by the feature agents).
macOS/Windows CI failed on two timing-fragile tests this PR added; ubuntu (faster runner) masked both. No production code changed. - cancel_stops_pump_while_child_alive: cancelled on a fixed 150ms sleep, but a cold interpreter on a loaded CI runner hadn't emitted the line yet, so the file was empty at cancel. Now polls the (unbuffered) file until the line is observed, then cancels and asserts the handle resolves well inside the child's now-30s lifetime — deterministic regardless of runner speed. - run_allows_loopback_when_escape_hatch_set: 10s LISTENING wait was too tight for a cold python stdlib http.server on a loaded macOS runner (~10s observed; ~2s locally). Bumped to 30s. Local ci-checks.sh green; targeted tests pass in 0.2s / 1.3s.
host_guard::run_allows_loopback_when_escape_hatch_set timed out ~30s on macOS CI only. Root cause: Python stdlib http.server.HTTPServer.server_bind() calls socket.getfqdn(host), a reverse-DNS lookup that blocks on macOS GitHub runners. It is the sole fresh consumer of mock-http-server.py (http_transport uses httpmock; this fixture isn't otherwise spawned), so only this test hit it; bumping the test timeout could not help (hang >= the DNS timeout). Fix at the root in both HTTP/SSE fixtures: a _Server subclass binds via socketserver.TCPServer.server_bind (no getfqdn) and sets server_name/port directly, so LISTENING is emitted immediately on every platform. mock-sse-server.py shares the identical latent bug (not yet triggered) — fixed preventively for consistency. Local: ci-checks.sh green; 16/16 host_guard/http/sse tests pass; LISTENING smoke immediate. macOS reverse-DNS path is removed entirely, not merely given more time.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
doctor/--explain/error hints, and 4 mock fixtures. Done now (pre-tag, unpublished) so the breaking API churn is absorbed into the initial release.--explain/doctor).url::Url::host_str()returns bracketed IPv6 ([::1]), so the initialparse::<IpAddr>()extraction let every IPv6 literal skip the private-IP block; the guard now classifies via the typedurl::Hostenum (covered by unit tests).Test plan
cargo fmt --checkcleancargo clippy --workspace --all-targets -- -D warningsclean (verified with--all-features)cargo nextest run --workspace --all-features— 368 passed, 2 skipped (#[ignore]: mock-leak slope timing + Vibe-Trading external checkout); doctests greenbash scripts/ci-checks.sh— ran the equivalent individual gate commands (fmt/clippy/nextest/doctests) all green; did not invoke the script wrapper.cargo llvm-cov --fail-under-lines 80not run —cargo-llvm-covis not installed in this environment (numeric coverage unmeasured; every feature ships with unit + integration tests)[Unreleased]updated (Added/Changed/Security/Notes; 0007 note flipped to superseded-by-0012)missing_docs = denypasses on both crates)Notes
Breaking (absorbed into v0.1.0, pre-publish — documented in CHANGELOG):
{Http,Sse,Ws}Transport::connecttake an added&HostGuard;StdioTransport::spawnis nowasync; CLIcmd_run::run_from_configtakescapture_stderr/tee_stderr. The documentedSession::spawn(command, args)2-arg signature is unchanged (delegates tospawn_with).Residual documented gap (v0.2): a hostname that resolves to a private IP (DNS rebinding) is not blocked in v0.1 —
allowed_hostsis the strong control; a resolver-pinning connector is the recorded ADR 0012 follow-up.Public API additions:
SpawnOptions/StderrMode/StderrCapture,Session::spawn_with,StdioTransport::spawn_with,Run::with_stderr_capture, publicprotocol::transport::HostGuard; CLIdoctorsubcommand, global--explain,hints::ErrorHint.Out of scope / follow-up: none blocking. (A 59 s
stderr_pumpcancel test that dominated the suite was fixed in-branch: ~59.5 s → 0.2 s, full suite ~60 s → 9.4 s, assertions strengthened.)Not included (separate maintainer action): no
git tag/cargo publish/ repo-visibility change — this PR only lands the features on a branch for review intomain.By contributing you agree your changes are dual-licensed under MIT and Apache-2.0,
matching the project. See CONTRIBUTING.md.