fix(platform): verify who is serving the named pipe before sending - #41
Merged
Conversation
Review 2026-08-27, R1. The Windows client dialled the pipe and checked nothing about the serving side, while the name is derivable and the pipe namespace is flat: another account could hold it before the daemon came up and be handed every request, elevation included. The client now reads the owner of the pipe object and hangs up before the first byte on one this account does not own. The owner, not the serving pid the review suggested: a pid may be reused between being handed over and being looked up, which is the objection peer_of already states for the other direction. So the pipe's descriptor states O:<sid> instead of letting the creating token's default owner — a machine policy — supply one. bind's probe answers the same question while it is there, so a squat is reported as "held by <sid>" rather than as a daemon of the user's to go and stop. Both error mappers classified the new variant as internal, which would ask the user to file a bug about it; both now say conflict. Nine tests, none of which crosses the account boundary: Windows will not let a standard process claim another account's SID as an owner, so the refusal tests vary the expectation over a real pipe and a real read. The cross-account case needs the system job (R3). Verified on Windows (fmt, clippy, rustdoc, cargo test --workspace) and on Linux via WSL (clippy -p mixengine-platform); macOS is left to CI.
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
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.
Closes R1 of the 2026-08-27 review.
What was wrong
On Windows,
connect()dialled the pipe and checked nothing about the serving side, while\.\pipe\mixengine.<sid>.<fnv(run)>is derivable — the SID is public and the fingerprint isFNV-1a written out in the file — the pipe namespace is flat and machine-wide, and
CreateNamedPipeWneeds no privilege. Another account could hold the name before the daemon cameup and be handed every request,
elevation.*included. The daemon's own half was right; only thereverse direction was missing.
What changed
account does not own — not the serving pid the review suggested. A pid may be reused between
being handed over and being looked up, which is the objection
peer_ofalready states for theother direction; an object's owner is stamped on at creation and cannot be set to an account the
creator does not hold.
O:<sid>. Without it the owner comes from the creating token'sdefault, which is a machine policy — where "Default owner for objects created by members of the
Administrators group" names the group, an administrator's pipe would be owned by
S-1-5-32-544and every client would refuse its own daemon.
bind's probe answers the same question. It already dialled the name to tell "taken" from"refused", so it now says who: "another process is already listening" would send the user
hunting for a daemon of their own to stop while a stranger holds the name.
internal, which asks the user to file a bugabout the one message telling them another account is on their endpoint. Both now say
conflict,and neither hint offers
mix daemon stop. Autostart needed nothing:is_absentalready excludedeverything but
NotFound/ConnectionRefused.security-model.mdnow states the thing the review found unwritten — "single-user" describes themachine, not a licence to hand another account the API.
How it was verified
cargo fmt --all --check,cargo clippy --workspace --all-targets -- -D warnings,RUSTDOCFLAGS=-D warnings cargo doc,cargo test --workspacelocally, plus thetest,systemand
benchlegs in CI.cargo clippy -p mixengine-platform --all-targets -- -D warningsunder WSL beforepushing, plus all three CI legs.
bench (ubuntu-latest)failed on the first run at 10859 ms against a 10000 ms budget and passedon a rerun of the same commit at 9076 ms. That test is bimodal on this runner pool and the
budget sits between the two clusters; nothing here touches the service start path, and none of it
compiles on Linux at all.
What this does not cover
The account boundary itself is not crossed by any test. Windows will not let a standard process
claim another account's SID as an owner, so the two refusal tests vary the expectation while the
pipe, the read and the refusal path stay real. A genuine cross-account test needs a second account
on the machine and belongs to the
systemjob — which R3 of the same review shows does not run itsgated suites yet.