Conversation
Introduce a feature-gated replay subsystem for time-travel debugging. Adds core replay types and logic (config, entry, meta, diff, truncation, redaction, store) in rustapi-core, CLI commands in cargo-rustapi (list/show/run/diff) behind a `replay` feature, and supporting extras (auth, client, fs/memory stores, routes, retention) in rustapi-extras. Cargo.toml updates make async-trait and related deps optional and wire the new feature; Cargo.lock is updated accordingly. A cookbook recipe (docs/cookbook/src/recipes/replay.md) is included and the replay config defaults are secure (disabled by default, admin token required, sensitive headers redacted, TTL enforced).
Apply whitespace/formatting cleanups and small refactors across replay-related crates for readability. Move the #[cfg(feature = "replay")] pub mod replay declaration up in rustapi-core (removing a duplicate), and tidy several files (commands/replay, replay config/diff/entry/redaction/truncation, extras auth/client/fs_store/layer/memory_store/routes). Changes are non-functional for the most part (reformatting, line-wrapping, import/order tweaks), with the module reordering removing a duplicate declaration that could cause issues.
There was a problem hiding this comment.
Pull request overview
This pull request introduces a comprehensive time-travel debugging feature for HTTP request/response recording and replay. The implementation follows a feature-gated approach, ensuring the functionality is opt-in and doesn't impact existing deployments. The feature includes middleware for recording, multiple storage backends (in-memory and filesystem), admin API routes for management, CLI commands for interaction, and extensive security controls including token authentication and data redaction.
Changes:
- Added
replayfeature flag to all relevant crates with proper dependency management - Implemented core data structures and configuration for replay entries with secure defaults (disabled by default, token-required, sensitive data redacted)
- Created middleware layer for transparent request/response capture with body truncation and sampling support
- Added admin API routes for listing, showing, replaying, and diffing recorded entries
- Implemented CLI commands for replay management with multiple subcommands (list, show, run, diff)
Reviewed changes
Copilot reviewed 27 out of 28 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/cookbook/src/recipes/replay.md | Comprehensive documentation for the replay feature including quick start, configuration, API reference, and security guidance |
| crates/rustapi-core/src/replay/config.rs | Configuration system with builder pattern and secure defaults for replay recording |
| crates/rustapi-core/src/replay/entry.rs | Core data structures for recorded requests/responses with metadata |
| crates/rustapi-core/src/replay/meta.rs | Metadata structure for replay entries including duration, client IP, and tags |
| crates/rustapi-core/src/replay/store.rs | Storage trait and query types for pluggable replay backends |
| crates/rustapi-core/src/replay/redaction.rs | Utilities for redacting sensitive headers and JSON body fields |
| crates/rustapi-core/src/replay/truncation.rs | Body truncation and content type detection utilities |
| crates/rustapi-core/src/replay/diff.rs | JSON-aware diffing logic for comparing original and replayed responses |
| crates/rustapi-core/src/replay/mod.rs | Module exports and documentation for the replay core |
| crates/rustapi-extras/src/replay/layer.rs | Middleware layer for capturing request/response pairs with body buffering |
| crates/rustapi-extras/src/replay/memory_store.rs | In-memory ring buffer implementation of the replay store |
| crates/rustapi-extras/src/replay/fs_store.rs | Filesystem-based JSON Lines replay store with rotation support |
| crates/rustapi-extras/src/replay/client.rs | HTTP client for replaying recorded requests against target servers |
| crates/rustapi-extras/src/replay/auth.rs | Bearer token authentication for admin endpoints |
| crates/rustapi-extras/src/replay/routes.rs | Admin API route handlers for replay management |
| crates/rustapi-extras/src/replay/retention.rs | Background job for TTL-based cleanup of expired entries |
| crates/rustapi-extras/src/replay/mod.rs | Module exports for the replay middleware |
| crates/cargo-rustapi/src/commands/replay.rs | CLI commands for interacting with replay admin API |
| crates/cargo-rustapi/src/commands/mod.rs | Integration of replay commands into CLI |
| crates/cargo-rustapi/src/cli.rs | CLI enum extension for replay subcommand |
| crates/rustapi-core/Cargo.toml | Added async-trait dependency for replay feature |
| crates/rustapi-extras/Cargo.toml | Added dependencies (uuid, reqwest, dashmap) for replay feature |
| crates/rustapi-rs/Cargo.toml | Added replay feature flag to full feature set |
| crates/cargo-rustapi/Cargo.toml | Added replay feature flag with reqwest dependency |
| crates/rustapi-core/src/lib.rs | Feature-gated replay module export |
| crates/rustapi-extras/src/lib.rs | Feature-gated replay module export and re-exports |
| crates/rustapi-rs/src/lib.rs | Feature-gated replay module re-export |
| Cargo.lock | Updated with new dependency versions |
| fn extract_query_param(uri: &http::Uri, key: &str) -> Option<String> { | ||
| uri.query().and_then(|q| { | ||
| q.split('&').find_map(|pair| { | ||
| let (k, v) = pair.split_once('=')?; | ||
| if k == key { | ||
| Some(v.to_string()) | ||
| } else { | ||
| None | ||
| } | ||
| }) | ||
| }) | ||
| } |
There was a problem hiding this comment.
The query parameter extraction does not URL-decode values. If query parameters contain URL-encoded characters (like spaces as %20 or special characters), they won't be decoded. This could cause issues when passing target URLs with encoded characters. Consider using urlencoding::decode or similar to properly handle encoded query parameter values.
| /// Admin bearer token | ||
| #[arg(short, long)] | ||
| pub token: String, | ||
|
|
||
| /// Maximum number of entries to return | ||
| #[arg(short, long)] | ||
| pub limit: Option<usize>, | ||
|
|
||
| /// Filter by HTTP method | ||
| #[arg(short, long)] | ||
| pub method: Option<String>, | ||
|
|
||
| /// Filter by path substring | ||
| #[arg(short, long)] | ||
| pub path: Option<String>, | ||
| } | ||
|
|
||
| /// Arguments for `replay show` | ||
| #[derive(Args, Debug)] | ||
| pub struct ReplayShowArgs { | ||
| /// Replay entry ID | ||
| pub id: String, | ||
|
|
||
| /// Server URL | ||
| #[arg(short, long, default_value = "http://localhost:8080")] | ||
| pub server: String, | ||
|
|
||
| /// Admin bearer token | ||
| #[arg(short, long)] | ||
| pub token: String, |
There was a problem hiding this comment.
The documentation states that the --token parameter can be set via the RUSTAPI_REPLAY_TOKEN environment variable (line 121-125 of docs/cookbook/src/recipes/replay.md), but the CLI implementation does not actually read from this environment variable. The token field is marked as required without any env support in clap. Either add #[arg(short, long, env = "RUSTAPI_REPLAY_TOKEN")] to the token fields in the Args structs, or update the documentation to remove this claim.
| let body = json!({ | ||
| "error": "unauthorized", | ||
| "message": "Missing or invalid admin token. Use Authorization: Bearer <token>" | ||
| }); |
There was a problem hiding this comment.
The token comparison uses simple string equality which is vulnerable to timing attacks. An attacker could potentially use timing differences to guess the token character by character. Consider using a constant-time comparison function like subtle::ConstantTimeEq or a secure comparison from a crypto library to prevent timing side-channel attacks.
| // Ensure consistent ordering | ||
| if !query.newest_first { | ||
| results.reverse(); | ||
| results.reverse(); // already correct order | ||
| } | ||
|
|
There was a problem hiding this comment.
The double reverse operation is redundant. Lines 92-93 reverse the results twice, which effectively leaves them in the original order. This block should be removed entirely since when newest_first is false, the results are already in the correct order (oldest first) from the forward iteration in line 80.
| // Ensure consistent ordering | |
| if !query.newest_first { | |
| results.reverse(); | |
| results.reverse(); // already correct order | |
| } |
| let size = body_bytes.len(); | ||
| if size <= config.max_request_body { | ||
| let body_str = String::from_utf8(body_bytes.to_vec()).ok(); | ||
| // Apply body field redaction | ||
| let redacted = body_str.and_then(|s| { | ||
| if config.redact_body_fields.is_empty() { | ||
| Some(s) | ||
| } else { | ||
| redact_body(&s, &config.redact_body_fields, "[REDACTED]") | ||
| } | ||
| }); | ||
| (size, redacted, false) | ||
| } else { | ||
| // Body too large - truncate | ||
| let body_str = String::from_utf8(body_bytes.to_vec()).ok(); | ||
| let truncated = body_str.map(|s| { | ||
| let (t, _) = truncate_body(&s, config.max_request_body); | ||
| t | ||
| }); | ||
| (size, truncated, true) | ||
| } | ||
| } else { | ||
| (0, None, false) | ||
| } | ||
| } else { | ||
| let size = req | ||
| .headers() | ||
| .get(http::header::CONTENT_LENGTH) | ||
| .and_then(|v| v.to_str().ok()) | ||
| .and_then(|s| s.parse::<usize>().ok()) | ||
| .unwrap_or(0); | ||
| (size, None, false) | ||
| }; | ||
|
|
||
| // Call the next handler | ||
| let response = next(req).await; | ||
|
|
There was a problem hiding this comment.
Critical bug: The request body is consumed with take_body() at line 226 but never restored before calling next(req) at line 262. This means downstream handlers will not be able to access the request body, breaking request handling. The body needs to be restored to the request after capture. Consider storing the captured bytes and then setting req.body = BodyVariant::Buffered(body_bytes) before calling next, or implement a non-destructive way to inspect the body.
| | Method | Path | Description | | ||
| |--------|------|-------------| | ||
| | GET | `/__rustapi/replays` | List recorded entries | | ||
| | GET | `/__rustapi/replays/{id}` | Show a single entry | | ||
| | POST | `/__rustapi/replays/{id}/run?target=URL` | Replay against target | | ||
| | POST | `/__rustapi/replays/{id}/diff?target=URL` | Replay and compute diff | | ||
| | DELETE | `/__rustapi/replays/{id}` | Delete an entry | | ||
|
|
||
| ### Query Parameters for List | ||
|
|
||
| - `limit` - Maximum number of entries to return | ||
| - `method` - Filter by HTTP method (GET, POST, etc.) | ||
| - `path` - Filter by path substring | ||
| - `status_min` - Minimum status code filter | ||
|
|
||
| ### Example: cURL | ||
|
|
||
| ```bash | ||
| # List entries | ||
| curl -H "Authorization: Bearer my-secret-token" \ | ||
| http://localhost:8080/__rustapi/replays?limit=10 | ||
|
|
||
| # Show a specific entry | ||
| curl -H "Authorization: Bearer my-secret-token" \ | ||
| http://localhost:8080/__rustapi/replays/<id> | ||
|
|
||
| # Replay against staging | ||
| curl -X POST -H "Authorization: Bearer my-secret-token" \ | ||
| "http://localhost:8080/__rustapi/replays/<id>/run?target=http://staging:8080" | ||
|
|
||
| # Replay and diff | ||
| curl -X POST -H "Authorization: Bearer my-secret-token" \ | ||
| "http://localhost:8080/__rustapi/replays/<id>/diff?target=http://staging:8080" |
There was a problem hiding this comment.
The admin API and examples here expose a target query parameter that accepts an arbitrary base URL for replaying requests, but there is no documented restriction on which hosts/schemes are allowed. In the current implementation, this value is passed directly into the server-side HTTP client, so anyone with the admin token can use /__rustapi/replays/{id}/run?target=... or /diff?target=... as an SSRF primitive to make the server call internal or cloud-metadata endpoints and return the response. The docs should explicitly warn about this risk and recommend configuring the replay system to only allow a small, predefined set of replay targets (e.g., specific staging hosts) rather than arbitrary user-provided URLs, and the implementation should enforce such an allowlist by default.
1 participant
This pull request introduces a new "replay" feature for time-travel debugging, allowing HTTP request/response pairs to be recorded and replayed for debugging purposes. The implementation is feature-gated (only enabled with the
replayfeature flag) and includes new CLI commands, configuration options, and core data structures for replay entries. Secure and sensible defaults are enforced, such as requiring an admin token and redacting sensitive data.Replay Feature Integration
replayfeature to bothcargo-rustapiandrustapi-corecrates, including necessary dependencies and feature flags inCargo.tomlfiles. [1] [2] [3]replaymodule inrustapi-core(behind the feature flag), enabling the core replay functionality.CLI and Command Enhancements
ReplayCLI subcommand (feature-gated) to the main CLI, and wired it up to the command handler. [1] [2]replaycommand and argument types in the commands module, conditionally compiling them with the feature flag.Replay System Implementation
ReplayConfiginreplay/config.rs, providing a builder-pattern configuration for the replay system with secure defaults (disabled by default, admin token required, redacted headers, TTL, etc.), as well as utility methods and comprehensive tests.replay/entry.rs, defining how HTTP requests and responses are captured, stored, and serialized for replay/debugging. Includes unit tests for serialization and entry creation.