harden(cli): pre-audit hardening of the bridge cold-start change - #56
Merged
Conversation
Pre-emptive adversarial review (3 red-team passes: security, concurrency, semver/robustness) before an external GPT audit + 0.8.7. No new Critical/High/ Medium security defect was found (the dir_is_trusted token/hijack guard survives the refactor); these are the reconciled real findings, fixed pre-emptively. Correctness / robustness: - H1: the availability poller is now the SOLE owner of `backend_up`. Previously a tool call that reconnected inside the 1.5s poll window set the flag and made the poller's down->up edge detection miss, silently leaving the client on the baked fallback tool list. list_changed now always fires on the real transition. - H2: read stdin on a dedicated OS thread + async channel, so the blocking read never parks a tokio worker. On a single-vCPU host the old blocking loop starved the timer and the poller never fired (auto-go-live dead). Now works on any host. - M1: the poller no longer emits notifications before the client's `initialize`, and backs off to a 5s cadence once up (was a tasklist/ps + /health probe every 1.5s for the whole session). - M2: replay `notifications/initialized` to the backend after its handshake, so a stateful rmcp backend sees a complete MCP lifecycle (harmless for the stateless default). - L1: a 202 response to a REQUEST no longer returns empty payloads (client hang) — a JSON-RPC error is synthesized for that id. - Mutex locks recover from poisoning (`locked()` helper) instead of cascading a single panic into the death of the bridge or, silently, the reconnect poller. Compat / security: - M3: the local `initialize` no longer advertises `resources.subscribe` — the plugin deliberately omits it (no server-initiated push), so advertising it here misled clients into subscribing and waiting forever. - LOW-1: `ps`/`tasklist`/`kill` are invoked by absolute path (System32 / /bin / /usr/bin), not via PATH — closing a PATH-hijack surface the poller exercises continuously. - Swept vestigial `--wait` from README / MIGRATION / getting-started examples. Tests: M3 (no subscribe) unit-asserted; M2 proven e2e (mock counts the backend `notifications/initialized`). Full gate green (fmt --all, clippy --workspace --all-targets -D warnings on clippy 1.97, test -p victauri-cli 46 unit + 2 e2e). Re-verified LIVE against 4DA: 5/5 fresh cold-connects return the live tool list + a working get_plugin_info on first contact, subscribe absent. Deferred (documented in the audit brief with rationale): session-adoption from a -32601 error handshake (risks the verified stateless path; trust-gated), the pre-existing PID+port-reuse hijack window (narrow; needs an /info identity cross-check), multi-line SSE relay (backend-controlled, not reachable), and a cross-crate fallback-manifest drift guard. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KfH8iYXUYm5YruHescthzD
External adversarial audit (Cursor 5.5-xhigh) against the exact SHA. 3 findings, all verified against live code and fixed. My own pre-audit pass had DEFERRED finding #1 as "narrow/pre-existing" — the auditor reproduced a real working token leak, so it is escalated to a fix here (I was wrong to defer it). #1 (Medium — token leak via cached connection): forward_with_retries reused a cached (port, token) without re-verifying the port still belonged to the trusted app. After the app shut down (discovery entry gone), a tool call would POST the cached Bearer token to whatever process had bound the freed port — the auditor's harness had an attacker server receive the token and relay forged tools/call results. FIX: re-resolve the trusted backend via scan_once on EVERY forward (re-applies dir_is_trusted + liveness + --app identity, yielding port+token together); the poller also drops the cached connection/session when the backend goes away. New e2e regression: after the trusted entry is removed, the next tool call errors and the (now-untrusted) backend receives NO request. #2 (Low — malformed/empty 2xx corrupts stream or hangs client): post_message relayed non-SSE 2xx bodies without JSON validation and let an empty body produce zero payloads. FIX: relay a non-SSE body only if valid JSON; a request that yields no payload (empty/non-JSON 2xx, or 202) gets a synthesized JSON-RPC error for its id (shared error_for_request helper). #3 (Low — list_changed races the initialize response): the ready flag was set on `initialize` receipt, before the response was written. FIX: set it on the client's `notifications/initialized` ack (renamed initialized -> client_ready). Auditor confirmed SOLID: the dir_is_trusted token/hijack gate before every token read, atomic token+port under one lock, serialized stdout, --wait compat, and fallback listing. Gate green: fmt --all, clippy --workspace --all-targets -D warnings (1.97), test -p victauri-cli (46 unit + 3 e2e). Re-verified LIVE vs 4DA: 5/5 fresh cold-connects, live tools + working get_plugin_info. Deferred (documented): a full pre-Bearer HMAC identity handshake (needs a plugin-side proof endpoint — larger, cross-crate); strict response-id matching against a malicious backend (already trust-gated). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KfH8iYXUYm5YruHescthzD
This was referenced Jul 13, 2026
runyourempire
added a commit
that referenced
this pull request
Jul 13, 2026
…58) victauri-cli-only, semver-clean within ^0.8 (plugin + public Rust API unchanged). Bundles: the bridge cold-start fix (#55 — connects in a fresh terminal even with the app down), pre-emptive internal red-team hardening (#56), external GPT audit remediation incl. a real token-leak fix + regression test, and round-2 verification follow-ups (#57). - scripts/bump-version.ps1 0.8.7 (workspace version, dep pins, action pin, CLAUDE.md refs, Cargo.lock) - CHANGELOG [Unreleased] -> [0.8.7] - 2026-07-13 - MIGRATION.md v0.8.6 -> v0.8.7 (bridge connects in a fresh terminal; --wait no-op; per-forward trusted re-resolve) - CLAUDE.md Current State -> v0.8.7 Gate green locally: fmt --all, clippy --workspace --all-targets -D warnings (1.97), build --workspace, test -p victauri-cli (46 unit + 3 e2e). NOT published (operator's crates.io gate). Claude-Session: https://claude.ai/code/session_01KfH8iYXUYm5YruHescthzD Co-authored-by: audit <audit@example.test> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Pre-emptive adversarial hardening of the bridge cold-start change (#55) before an external GPT audit and the 0.8.7 cut. Three internal red-team passes (security, concurrency/correctness, semver/robustness) + a self-review. No new Critical/High/Medium security defect — the
dir_is_trustedtoken-theft/hijack guard survives the refactor (enforced before any token read; token↔port binding atomic;/healthcarries no token). These are the reconciled real findings, fixed pre-emptively.Correctness / robustness
backend_up. A tool call that reconnected inside the 1.5s poll window used to set the flag and make the poller's down→up edge detection miss, silently leaving the client on the baked fallback tool list.tools/list_changednow always fires on the real transition.initialize, and backs off to a 5s cadence when up (was atasklist/ps+/healthprobe every 1.5s for the whole session — ~19k needless spawns / 8h).notifications/initializedto the backend after its handshake, completing the MCP lifecycle for a stateful rmcp backend (harmless for the stateless default). Proven e2e (mock counts it).202to a request no longer returns empty payloads (client hang); a JSON-RPC error is synthesized for that id.locked()helper) instead of cascading a single panic into the death of the bridge or, silently, the reconnect poller.Compat / security
initializeno longer advertisesresources.subscribe— the plugin deliberately omits it (server.rs:98; no server-initiated push), so advertising it here misled clients into subscribing and waiting forever. Unit-asserted.ps/tasklist/killinvoked by absolute path (System32 //bin//usr/bin), notPATH— closes a PATH-hijack surface the poller exercises continuously.--waitfrom README / MIGRATION / getting-started examples.Verification
Full gate green:
cargo fmt --all --check,cargo clippy --workspace --all-targets -D warnings(clippy 1.97),cargo test -p victauri-cli(46 unit + 2 e2e). Re-verified LIVE against 4DA: 5/5 fresh cold-connects return the live tool list (real schemas) + a workingget_plugin_infoon first contact,subscribeabsent.Deferred (documented with rationale in the audit brief)
-32601error handshake — risks the verified stateless path; the threat is trust-gated./infoidentity cross-check.🤖 Generated with Claude Code