security(cli): round-2 audit follow-ups on the bridge - #57
Merged
Conversation
A second adversarial pass (two internal red-team reviewers) on the remediated code. Both confirmed the round-1 fixes are correct and complete: finding #1's re-resolve covers EVERY token-bearing path (top-of-forward scan AND the retry-loop reconnect, both via scan_once's dir_is_trusted + identity gate, proven by the regression test); #3 is complete; restart recovery, stdout serialization, poison-recovery, and single-writer backend_up are intact. Two Low items fixed here, plus honest documentation of pre-existing residuals: - Finish #2 (was PARTIAL): the non-SSE relay JSON-VALIDATED the body but relayed it verbatim, so a valid multi-line (pretty-printed) body could be split across stdout lines by writeln! and break a newline-delimited client. Now relays the compact re-serialized form (parsed.to_string()) — one line, matching the SSE path. Low (not attacker-reachable: the backend is trust-gated Victauri, which emits compact JSON). - Close the client_ready lost-edge: if the backend came up during the init-handshake window, the poller consumed the down->up edge without emitting (client not ready yet) and no fresh edge followed. The notifications/initialized handler now emits list_changed once if backend_up is already true (idempotent). Documented residuals (pre-existing, not introduced; deferred with rationale): - Liveness is PID-based; /health carries no token and doesn't authenticate the listener, so PID-reuse + port-reuse can theoretically defeat it. Re-resolving per call shrinks it to a per-call coincidence; a full close needs mutual auth on /health (plugin-side). - Windows dir_is_trusted is an assumption (per-user %TEMP% not world-writable), not a verified ACL check; a redirected shared TEMP is unguarded on Windows. - A tool call racing the sub-second restart gap fails fast by design (the audit #1 rule forbids reusing a cached connection). 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: 3/3 fresh cold-connects, live tools + working call. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KfH8iYXUYm5YruHescthzD
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.
Second adversarial verification pass (two internal red-team reviewers) on the remediated bridge (post-#56). Both confirmed the round-1 fixes are correct and complete — finding #1's re-resolve covers every token-bearing path (top-of-forward scan and the retry-loop reconnect, both through
scan_once'sdir_is_trusted+ identity gate, locked in by the regression test); #3 complete; restart recovery, stdout serialization, poison-recovery, and single-writerbackend_upall intact.Fixed (2 Low)
parsed.to_string()), matching the SSE path. (Not attacker-reachable — the backend is trust-gated Victauri, which emits compact JSON.)client_readylost-edge: if the backend came up during the init-handshake window, the poller consumed the down→up edge without emitting; thenotifications/initializedhandler now emitslist_changedonce if the backend is already up (idempotent).Documented residuals (pre-existing, not introduced; deferred with rationale in code + brief)
/healthcarries no token, so PID-reuse + port-reuse can theoretically defeat it. Per-call re-resolve shrinks it to a per-call coincidence; full close needs mutual auth on/health(plugin-side).dir_is_trustedis an assumption (per-user%TEMP%), not a verified ACL check; a redirected sharedTEMPis unguarded on Windows.Verification
Full gate green:
fmt --all,clippy --workspace --all-targets -D warnings(1.97),test -p victauri-cli(46 unit + 3 e2e). Re-verified live against 4DA: 3/3 fresh cold-connects, live tools + working call.🤖 Generated with Claude Code