Skip to content

feat: add distributed transport signing and smoke coverage#85

Merged
wauputr4 merged 8 commits into
mainfrom
codex/issue-71-72-distributed-smoke
Jun 17, 2026
Merged

feat: add distributed transport signing and smoke coverage#85
wauputr4 merged 8 commits into
mainfrom
codex/issue-71-72-distributed-smoke

Conversation

@wauputr4

@wauputr4 wauputr4 commented Jun 17, 2026

Copy link
Copy Markdown
Member

Summary

  • Implement distributed daemon transport security: HMAC-style request signing with timestamp + nonce + freshness window on daemon->control-plane traffic.
  • Add daemon signature verification middleware + nonce replay cache in API layer.
  • Add distributed smoke script (scripts/distributed-smoke.sh) for end-to-end path (mock upstream, control plane, daemon, user model request).
  • Add distributed runbook (docs/DISTRIBUTED_RUNBOOK.md) with execution and troubleshooting steps.
  • Clarify distributed trust boundaries and log behavior in docs/DISTRIBUTED_PROXY.md.

Linked Issues

Notes

  • scripts/distributed-smoke.sh is added but not executed as part of this PR.

Summary by CodeRabbit

  • Security

    • Verified incoming daemon-signed control-plane requests using timestamp TTL and anti-replay nonce caching, rejecting stale or reused requests.
    • Added outbound request signing from the daemon to the control plane for job leasing, job completion, daemon registration, and heartbeats.
  • Documentation

    • Updated distributed proxy trust boundaries and logging/data-handling guidance.
    • Added a distributed runbook, GitHub readiness checklist, project comparison notes, and refreshed the README.
  • Tests

    • Added deterministic signature verification unit tests.
    • Added an end-to-end distributed smoke test script and assertions.

@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 46e66114-e772-4cf0-b725-0011d2370ac7

📥 Commits

Reviewing files that changed from the base of the PR and between c0c42f6 and b2d8292.

📒 Files selected for processing (1)
  • scripts/distributed-smoke.sh
🚧 Files skipped from review as they are similar to previous changes (1)
  • scripts/distributed-smoke.sh

📝 Walkthrough

Walkthrough

Adds daemon-to-control-plane request signing: the daemon computes a SHA-256 signature over the token hash, HTTP method, path, timestamp, and UUID nonce, attaching it via x-mizan-daemon-* headers on every control-plane POST. The API verifies freshness, signature correctness, and nonce uniqueness against a shared cache. A new end-to-end smoke script and runbook document the full distributed flow. README restructured with v0.1.0 release boundary and OSS readiness documentation.

Changes

Daemon Request Signing (P18)

Layer / File(s) Summary
Signature constants, nonce cache state, and DaemonNodeIdentity contract
crates/mizan-api/src/daemon_nodes.rs, crates/mizan-api/src/lib.rs
Defines signature header name constants and TTL constant; extends DaemonNodeIdentity with token_hash field; wires daemon_signature_nonce_cache: Arc<RwLock<HashMap<String, i64>>> into AppState with initialization.
Daemon-side signing: SignedRequest trait and call sites
crates/mizan-daemon/Cargo.toml, crates/mizan-daemon/src/main.rs
Adds sha2 dependency; implements SignedRequest trait on reqwest::RequestBuilder to compute SHA-256 over token hash + method/path/timestamp/nonce and attach x-mizan-daemon-* headers; refactors daemon_http_client() to use a 30 s timeout; inserts .then_sign() at lease, completion, register, and heartbeat call sites; adds determinism unit test.
API-side signature verification and nonce replay protection
crates/mizan-api/src/daemon_nodes.rs
Updates SQL identity query to select token_hash; hooks verify_daemon_signature into daemon_node_auth middleware; implements full verification: header extraction, timestamp TTL check, SHA-256 recomputation, constant-time comparison, nonce deduplication with age-based cache pruning; adds unit tests for accept/reject paths.

Distributed Smoke Test, Runbook, and Release Readiness (P19)

Layer / File(s) Summary
Distributed smoke script
scripts/distributed-smoke.sh
New script orchestrates mock upstream startup, mizan-api provisioning with seeded admin credentials, daemon node creation with TOML config generation, daemon start and model availability polling, user provisioning, chat completion request, and assertions on usage row counts and daemon node identifier in /metrics.
Runbook, proxy docs security clarifications
docs/DISTRIBUTED_RUNBOOK.md, docs/DISTRIBUTED_PROXY.md
Adds runbook with prerequisites, smoke steps, troubleshooting, and security-foundation checks. Updates proxy doc with control-plane data ownership (credit ledger, request metadata), metadata-only log default, and MIZAN_LOG_RAW_REQUEST_BODIES opt-in.
README restructuring, release boundary, and OSS readiness documentation
README.md, docs/GITHUB_READINESS.md, docs/PROJECT_COMPARISON.md
Restructures README with v0.1.0 release boundary, API-first capabilities, architecture diagram, and OSS readiness checklist. Adds GitHub readiness checklist and project comparison documentation.

Sequence Diagram

sequenceDiagram
  participant Daemon
  participant SignedRequest
  participant ControlPlane as mizan-api
  participant NonceCache

  rect rgba(135, 206, 235, 0.5)
    Note over Daemon,SignedRequest: Signing (daemon side)
    Daemon->>SignedRequest: POST /daemon/jobs/lease (token_hash, method, path)
    SignedRequest->>SignedRequest: generate timestamp + UUID nonce
    SignedRequest->>SignedRequest: SHA-256(token_hash|method|path|timestamp|nonce)
    SignedRequest-->>Daemon: RequestBuilder + x-mizan-daemon-{sig,ts,nonce} headers
  end

  Daemon->>ControlPlane: POST with Bearer + x-mizan-daemon-* headers

  rect rgba(144, 238, 144, 0.5)
    Note over ControlPlane,NonceCache: Verification (API side)
    ControlPlane->>ControlPlane: daemon_node_auth extracts bearer → queries token_hash
    ControlPlane->>ControlPlane: verify_daemon_signature: parse ts, enforce TTL
    ControlPlane->>ControlPlane: recompute SHA-256, constant-time compare
    ControlPlane->>NonceCache: check nonce not seen, insert, prune expired
    NonceCache-->>ControlPlane: ok / replay error
    ControlPlane-->>Daemon: 200 OK / 401 Unauthorized
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐇 A bunny hops in signing mode,
Stamping nonces down the road,
Hash the token, path, and time—
Replay attacks? Not on my dime!
The smoke test runs, the daemon sings,
Security found on distributed wings. 🔐

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.63% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary change: adding distributed transport signing and smoke test coverage, which aligns with the main objectives of implementing HMAC-style request signing and providing end-to-end testing infrastructure.
Linked Issues check ✅ Passed The PR comprehensively addresses both linked issues: #71 objectives (daemon authentication with request signing, signature verification, nonce replay protection, trust boundary documentation, and payload privacy) and #72 objectives (distributed smoke test script, runbook documentation, end-to-end flow validation with mock upstream, usage verification, and metrics coverage).
Out of Scope Changes check ✅ Passed All changes are within scope: core signing implementation (daemon_nodes.rs, lib.rs, main.rs), dependencies (sha2 for hashing), smoke test infrastructure (script and runbook), and documentation updates (DISTRIBUTED_PROXY.md clarifications, new comparison doc). README and readiness checklist updates support v0.2.0 release readiness.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/issue-71-72-distributed-smoke

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements request signature verification for daemon nodes to secure communication between mizan-daemon and mizan-api. It introduces signature, nonce, and timestamp headers, along with a nonce cache to prevent replay attacks. The review feedback identifies several critical issues: a timing side-channel vulnerability in secure_eq, a compilation error due to an undefined parse_timestamp function, a mismatch in path validation between the client and server, a potential panic from unchecked timestamp arithmetic, and performance concerns regarding nonce cache pruning under a write lock.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +649 to +651
fn secure_eq(left: &str, right: &str) -> bool {
left.len() == right.len() && left.bytes().zip(right.bytes()).all(|(l, r)| l == r)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The secure_eq function uses all which short-circuits on the first mismatched byte. This introduces a timing side-channel vulnerability, allowing an attacker to guess the signature byte-by-byte. Use a constant-time comparison algorithm instead.

fn secure_eq(left: &str, right: &str) -> bool {
    if left.len() != right.len() {
        return false;
    }
    let mut accum = 0;
    for (l, r) in left.bytes().zip(right.bytes()) {
        accum |= l ^ r;
    }
    accum == 0
}

AppError::Unauthorized,
)?;

let timestamp = parse_timestamp(timestamp_raw).map_err(|error| {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The function parse_timestamp is neither defined in this file nor imported from crate::utils, which will cause a compilation error. Use standard library parsing timestamp_raw.parse::<i64>() instead.

Suggested change
let timestamp = parse_timestamp(timestamp_raw).map_err(|error| {
let timestamp = timestamp_raw.parse::<i64>().map_err(|error| {

Comment on lines +455 to +462
validate_daemon_request_signature(
&state,
request.method().as_str(),
request.uri().path_and_query().map(|value| value.as_str()).unwrap_or("/"),
request.headers(),
&identity,
)
.await?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The server validates the signature against path_and_query(), but the client only signs the path (e.g., /daemon/jobs/lease). Any query parameters added to the request (e.g., by proxies or load balancers) will cause signature verification to fail. Use request.uri().path() instead.

Suggested change
validate_daemon_request_signature(
&state,
request.method().as_str(),
request.uri().path_and_query().map(|value| value.as_str()).unwrap_or("/"),
request.headers(),
&identity,
)
.await?;
validate_daemon_request_signature(
&state,
request.method().as_str(),
request.uri().path(),
request.headers(),
&identity,
)
.await?;

Comment on lines +564 to +567
let now = now_utc_epoch_seconds();
if (now - timestamp).abs() > DAEMON_SIGNATURE_TTL_SECONDS {
return Err(map_error(StatusCode::UNAUTHORIZED, AppError::Unauthorized));
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Direct subtraction now - timestamp and .abs() on untrusted input can overflow/underflow and cause a panic (denial of service) or unexpected behavior. Use checked arithmetic to safely compute the difference.

    let now = now_utc_epoch_seconds();
    let is_stale = now
        .checked_sub(timestamp)
        .and_then(|diff| diff.checked_abs())
        .map_or(true, |abs_diff| abs_diff > DAEMON_SIGNATURE_TTL_SECONDS);
    if is_stale {
        return Err(map_error(StatusCode::UNAUTHORIZED, AppError::Unauthorized));
    }

Comment thread crates/mizan-api/src/daemon_nodes.rs Outdated
Comment on lines +580 to +581
let mut nonces = state.daemon_signature_nonce_cache.write().await;
prune_stale_nonces(&mut nonces, now);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Pruning the entire nonce cache on every single request while holding a write lock is highly inefficient ($O(N)$ complexity) and will cause severe lock contention under high load. Consider pruning periodically (e.g., via a background task) or using a more efficient strategy.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c859051639

ℹ️ 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".

Comment thread crates/mizan-api/src/daemon_nodes.rs Outdated
return Err(map_error(StatusCode::UNAUTHORIZED, AppError::Unauthorized));
}

nonces.insert(nonce_key, now);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Retain future-dated nonces for their full replay window

When a daemon clock is ahead but still within the accepted 300-second skew, storing the nonce with the server receipt time lets the cache prune it too early. For example, a request signed with timestamp = now + 299 is cached at now, pruned after 300 seconds, and can then be replayed while its timestamp is still only a few seconds from the server clock, defeating the replay check for captured daemon requests. Store an expiry derived from the signed timestamp window or reject future timestamps instead of expiring solely by receipt time.

Useful? React with 👍 / 👎.

Comment on lines +620 to +626
digest.update(method.as_bytes());
digest.update("|");
digest.update(path.as_bytes());
digest.update("|");
digest.update(timestamp.to_string().as_bytes());
digest.update("|");
digest.update(nonce);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Bind daemon signatures to the request body

For JSON-bearing daemon endpoints such as /daemon/heartbeat, /daemon/register, and /daemon/jobs/{id}/complete, the verifier hashes only the token hash, method, path, timestamp, and nonce, so a captured signed request can have its JSON payload altered without changing x-mizan-daemon-signature. This leaves capability updates and dispatch completion results unprotected by the new transport signature; include a canonical body hash (or sign the serialized body) on both client and server.

Useful? React with 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🧹 Nitpick comments (2)
scripts/distributed-smoke.sh (2)

102-113: ⚡ Quick win

Validate Redis availability before starting mizan-api.

Line 106 defaults REDIS_URL to redis://127.0.0.1:6379/, but there is no check that Redis is actually reachable. If Redis is unavailable, mizan-api will fail to start with a potentially confusing connection-pool error rather than a clear diagnostic. Add an explicit health check before starting the API.

🔌 Proposed addition before starting mizan-api
# Validate Redis is reachable
echo "Checking Redis connectivity at ${REDIS_URL}"
if ! redis-cli -u "${REDIS_URL}" ping >/dev/null 2>&1; then
  echo "Redis is not reachable at ${REDIS_URL}" >&2
  exit 1
fi

If redis-cli is not available, alternatively use a curl-based check or a small Python script that attempts a Redis connection.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/distributed-smoke.sh` around lines 102 - 113, Add a Redis
connectivity validation check immediately before the cargo run command that
starts mizan-api. The check should verify that the Redis server specified in
REDIS_URL is reachable by attempting a ping operation using redis-cli with the
REDIS_URL, and if the ping fails or redis-cli is unavailable, the script should
output a clear error message and exit with a non-zero status code. This ensures
that any Redis connection failures are caught early with a diagnostic message
rather than allowing mizan-api to start and fail with confusing connection-pool
errors.

111-149: ⚖️ Poor tradeoff

Consider build-time overhead and registration timing.

The script uses cargo run three times (lines 111, 149, 152), each of which rebuilds dependencies. On slow machines or CI environments, these builds may exceed the WAIT_SECONDS timeout (default 600 s). Additionally, the daemon registration at line 149 is synchronous; if the control plane is still initializing, the registration call could race or timeout before the daemon can start.

Consider one or more of the following:

  1. Pre-compile binaries or use pre-built container images rather than building in the script.
  2. Increase the timeout for cargo build separately from curl polling timeouts.
  3. Add explicit logging to the daemon registration call to diagnose timeout causes.
  4. Restructure the sequence to start the daemon registration loop in parallel after the control plane is healthy, rather than serially after all daemon config is written.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/distributed-smoke.sh` around lines 111 - 149, The script executes
multiple cargo run commands (starting mizan-api on line 111 and registering
mizan-daemon on line 149) which rebuild dependencies and can exceed the
WAIT_SECONDS timeout on slower systems or CI environments. Additionally, the
daemon registration is performed synchronously after control plane
initialization, creating a race condition if the control plane is still
initializing. To fix this, add explicit logging statements to the daemon
registration curl call to provide visibility into timeout causes, increase the
wait timeout to account for cargo build time separately from curl polling, and
consider restructuring the sequence to trigger daemon registration in the
background after confirming the control plane is healthy (using & to run in
parallel) rather than blocking on the synchronous registration call.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/mizan-api/src/daemon_nodes.rs`:
- Around line 4-8: The imports in the use block for axum::http in
daemon_nodes.rs are not following rustfmt formatting standards. Reorganize the
imports by flattening or restructuring the nested header::{AUTHORIZATION} import
to match Rust's standard import formatting conventions. The issue is likely that
AUTHORIZATION should be imported directly from axum::http::header rather than
nested, or the entire imports block needs to be reorganized to match the
expected order and grouping that rustfmt enforces.
- Around line 649-651: The `secure_eq` function uses the `.all()` iterator
method which short-circuits on the first mismatched byte, creating a timing
side-channel vulnerability. Replace the current implementation with a
constant-time comparison approach that compares all bytes regardless of
mismatches. You can either implement this using XOR accumulation to combine all
byte comparisons into a single result without short-circuiting, or add the
`subtle` crate to your dependencies and use its `ConstantTimeEq` trait which
provides a well-audited constant-time implementation. Ensure the function still
accepts two string slices and returns a boolean indicating whether they are
equal.

In `@crates/mizan-api/src/lib.rs`:
- Around line 14-18: The imports at the top of the file are not following
rustfmt's expected ordering rules, which is causing the CI pipeline to fail. The
imports (std::collections::HashMap, std::sync::Arc, tokio::net::TcpListener,
tokio::task, and tokio::sync::RwLock) need to be reorganized according to Rust's
standard import convention where standard library imports come first, followed
by external crate imports, with each group sorted alphabetically. Run cargo fmt
--all from the project root to automatically reformat the imports to the correct
order.

In `@crates/mizan-daemon/src/main.rs`:
- Around line 777-785: The test function
computes_deterministic_request_signature has formatting that does not match the
Rust formatter standards, causing the CI pipeline to fail. Run cargo fmt --all
from the project root to automatically reformat the multi-line function calls
and entire codebase according to rustfmt standards. This will ensure all
formatting aligns with the project's style requirements.

In `@docs/DISTRIBUTED_PROXY.md`:
- Around line 100-102: The MIZAN_LOG_RAW_REQUEST_BODIES configuration flag is
parsed in crates/mizan-core/src/config.rs but never used, and the
RequestLogInput struct in crates/mizan-api/src/logging.rs lacks fields to store
request or response bodies. To implement the documented feature, add a field (or
fields) to RequestLogInput to store the request and response bodies, then thread
the log_raw_request_bodies flag from the config through the middleware and
handler layers that construct the RequestLogInput, and conditionally capture and
populate the body fields only when the flag is enabled.

In `@scripts/distributed-smoke.sh`:
- Line 37: The json_has_model() function (and assert_usage_has_rows() at line
78) uses both a pipe and a heredoc to feed stdin to Python, causing the heredoc
to override the pipe. Remove the pipe (`printf '%s' "${payload}" |`) and instead
pass the payload through the heredoc by embedding it directly in the Python code
or using echo within the heredoc to provide the stdin to Python. Ensure the
heredoc content receives the payload variable as input rather than relying on
the conflicting pipe redirection.
- Around line 115-181: Add explicit HTTP status checking and JSON validation to
the critical curl operations that capture responses into variables like
admin_login_json, admin_api_key_response, daemon_node_json, user_login_json, and
user_api_key_json. Before calling json_field() on these responses, either check
the HTTP status code returned by curl (using curl -w with http_code) or validate
that the response is valid JSON (using jq validation). If validation fails, log
a clear error message including the actual response body and exit with an error
code to prevent opaque failures downstream.

---

Nitpick comments:
In `@scripts/distributed-smoke.sh`:
- Around line 102-113: Add a Redis connectivity validation check immediately
before the cargo run command that starts mizan-api. The check should verify that
the Redis server specified in REDIS_URL is reachable by attempting a ping
operation using redis-cli with the REDIS_URL, and if the ping fails or redis-cli
is unavailable, the script should output a clear error message and exit with a
non-zero status code. This ensures that any Redis connection failures are caught
early with a diagnostic message rather than allowing mizan-api to start and fail
with confusing connection-pool errors.
- Around line 111-149: The script executes multiple cargo run commands (starting
mizan-api on line 111 and registering mizan-daemon on line 149) which rebuild
dependencies and can exceed the WAIT_SECONDS timeout on slower systems or CI
environments. Additionally, the daemon registration is performed synchronously
after control plane initialization, creating a race condition if the control
plane is still initializing. To fix this, add explicit logging statements to the
daemon registration curl call to provide visibility into timeout causes,
increase the wait timeout to account for cargo build time separately from curl
polling, and consider restructuring the sequence to trigger daemon registration
in the background after confirming the control plane is healthy (using & to run
in parallel) rather than blocking on the synchronous registration call.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0535f8b3-57cc-4696-b7f3-1f15ae51ab08

📥 Commits

Reviewing files that changed from the base of the PR and between c86c069 and c859051.

📒 Files selected for processing (7)
  • crates/mizan-api/src/daemon_nodes.rs
  • crates/mizan-api/src/lib.rs
  • crates/mizan-daemon/Cargo.toml
  • crates/mizan-daemon/src/main.rs
  • docs/DISTRIBUTED_PROXY.md
  • docs/DISTRIBUTED_RUNBOOK.md
  • scripts/distributed-smoke.sh

Comment thread crates/mizan-api/src/daemon_nodes.rs Outdated
Comment thread crates/mizan-api/src/daemon_nodes.rs
Comment thread crates/mizan-api/src/lib.rs
Comment thread crates/mizan-daemon/src/main.rs
Comment thread docs/DISTRIBUTED_PROXY.md
Comment on lines +100 to +102
- credit ledger writes and final charge status;
- request-log metadata (status, latency, node id, route/provider alias),
but not raw conversation payload by default.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for MIZAN_LOG_RAW_REQUEST_BODIES reference in the API codebase
rg -n 'MIZAN_LOG_RAW_REQUEST_BODIES|log.*request.*body|body.*log' \
  crates/mizan-api/src --type rust -A 3 -B 3

Repository: pendig/mizan

Length of output: 38


🏁 Script executed:

cat -n docs/DISTRIBUTED_PROXY.md | sed -n '100,125p'

Repository: pendig/mizan

Length of output: 1174


🏁 Script executed:

# Search more broadly for request/body logging patterns
rg -n 'request.*body|log.*payload|raw.*request|body.*log' crates/mizan-api/src --type rust -i | head -40

Repository: pendig/mizan

Length of output: 618


🏁 Script executed:

# Check if there are any environment variable references for logging
rg -n 'env.*LOG|LOG.*env|MIZAN.*LOG' crates/mizan-api/src --type rust -i | head -30

Repository: pendig/mizan

Length of output: 38


🏁 Script executed:

# List the directory structure to understand where logging code might be
find crates/mizan-api/src -type d | head -20

Repository: pendig/mizan

Length of output: 77


🏁 Script executed:

# Find the logging module
fd -n 'logging' crates/mizan-api/src

Repository: pendig/mizan

Length of output: 287


🏁 Script executed:

# Search all Rust files for MIZAN_LOG_RAW_REQUEST_BODIES anywhere in the repo
rg -n 'MIZAN_LOG_RAW_REQUEST_BODIES' --type rust

Repository: pendig/mizan

Length of output: 265


🏁 Script executed:

# Look at the logging module implementation
cat crates/mizan-api/src/logging.rs 2>/dev/null | head -100

Repository: pendig/mizan

Length of output: 2899


🏁 Script executed:

# View the config.rs implementation around the MIZAN_LOG_RAW_REQUEST_BODIES definition
cat -n crates/mizan-core/src/config.rs | sed -n '115,140p'

Repository: pendig/mizan

Length of output: 1297


🏁 Script executed:

# Search for where this config value is actually used
rg -n 'log_raw_request_bodies|LOG_RAW_REQUEST_BODIES' --type rust -A 3 -B 3

Repository: pendig/mizan

Length of output: 1351


🏁 Script executed:

# Check what the RequestLogInput struct looks like and if it includes body fields
cat -n crates/mizan-api/src/logging.rs | sed -n '1,80p'

Repository: pendig/mizan

Length of output: 2904


🏁 Script executed:

# Search for where config.log_raw_request_bodies is used
rg -n 'config\.log_raw_request_bodies' --type rust -A 3 -B 3

Repository: pendig/mizan

Length of output: 38


🏁 Script executed:

# Also search for just log_raw_request_bodies variable usage
rg -n 'log_raw_request_bodies' --type rust -A 3 -B 3

Repository: pendig/mizan

Length of output: 1084


🏁 Script executed:

# Check where RequestLogInput is created/instantiated
rg -n 'RequestLogInput\s*\{' --type rust -A 15

Repository: pendig/mizan

Length of output: 4323


🏁 Script executed:

# Check if there are any other raw request/response body logging mechanisms
rg -n 'body|payload|conversation|prompt' crates/mizan-api/src --type rust | grep -i 'log\|debug\|trace' | head -20

Repository: pendig/mizan

Length of output: 1323


🏁 Script executed:

# Double-check: look for any tower middleware or other logging that might handle bodies
find crates/mizan-api/src -name '*.rs' -exec basename {} \; | sort | uniq

Repository: pendig/mizan

Length of output: 184


The MIZAN_LOG_RAW_REQUEST_BODIES environment variable is parsed but never used, and request body logging is not implemented.

The environment variable is defined in crates/mizan-core/src/config.rs (line 23) and parsed with a default of false, but it is never referenced in the codebase (verified via full repository search). Additionally, the RequestLogInput struct in crates/mizan-api/src/logging.rs contains no fields for storing request or response bodies—only metadata (request_id, user_id, status, latency, etc.).

The documentation at lines 117–123 explicitly promises opt-in raw body logging via MIZAN_LOG_RAW_REQUEST_BODIES=true, but this feature is not actually implemented. To fulfill the documented behavior, you must:

  1. Add a body/payload field to RequestLogInput (or create a separate struct variant).
  2. Thread config.log_raw_request_bodies through the middleware/handler layers that construct the log input.
  3. Conditionally capture and persist request/response bodies only when the flag is true.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/DISTRIBUTED_PROXY.md` around lines 100 - 102, The
MIZAN_LOG_RAW_REQUEST_BODIES configuration flag is parsed in
crates/mizan-core/src/config.rs but never used, and the RequestLogInput struct
in crates/mizan-api/src/logging.rs lacks fields to store request or response
bodies. To implement the documented feature, add a field (or fields) to
RequestLogInput to store the request and response bodies, then thread the
log_raw_request_bodies flag from the config through the middleware and handler
layers that construct the RequestLogInput, and conditionally capture and
populate the body fields only when the flag is enabled.

Comment thread scripts/distributed-smoke.sh Outdated
Comment thread scripts/distributed-smoke.sh Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 24e90be7f1

ℹ️ 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".

pub database: AnyPool,
pub redis: RedisClient,
pub metrics: metrics::MetricsRegistry,
pub daemon_signature_nonce_cache: Arc<RwLock<HashMap<String, i64>>>,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Persist daemon nonces outside process memory

In multi-replica control-plane deployments, or immediately after an API process restart, this per-process HashMap starts empty, so a captured signed daemon request can be replayed within the 300-second freshness window against another/fresh process and bypass the new nonce check. Since the API already has a Redis client in AppState, store nonce keys in shared storage with TTL instead of keeping the replay cache only in memory.

Useful? React with 👍 / 👎.

Comment thread scripts/distributed-smoke.sh Outdated
json_has_model() {
local payload="$1"
local model="$2"
printf '%s' "${payload}" | python3 - "$model" <<'PY'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Feed JSON payloads to smoke helper stdin

When the distributed smoke reaches wait_for_model, this pipeline never passes payload to json.load(sys.stdin): python3 - consumes the heredoc as the script on stdin, leaving the program's stdin empty, so the helper fails even when /v1/models contains the expected model. The same pattern is used by assert_usage_has_rows, so the smoke script cannot complete its advertised verification until the JSON is passed via python3 -c, an argument, or an environment variable.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c0c42f6389

ℹ️ 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".

Comment on lines +555 to +559
let body_hash = headers
.get(DAEMON_SIGNATURE_BODY_HASH_HEADER)
.and_then(|value| value.to_str().ok())
.map(str::trim)
.unwrap_or("");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Verify body hashes against the request body

Fresh evidence in the current diff is that x-mizan-daemon-body-hash is read from the headers and then trusted during signature verification, but the middleware never hashes the actual request body and compares it to this value. For JSON daemon endpoints, an active intermediary that captures a signed heartbeat/register/complete request before first delivery can keep the signed hash header unchanged while altering the JSON payload, and the nonce/signature checks still pass because they only authenticate the header value, not the bytes Axum later deserializes.

Useful? React with 👍 / 👎.

@wauputr4 wauputr4 merged commit 64ae917 into main Jun 17, 2026
2 checks passed
@wauputr4 wauputr4 deleted the codex/issue-71-72-distributed-smoke branch June 17, 2026 16:11
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.

P19: Add distributed proxy smoke test and runbook P18: Add distributed transport security and payload privacy foundation

1 participant