fix(daemon): use hash-only Windows named-pipe name - #86
Merged
Conversation
The per-user pipe name embedded %USERNAME% verbatim
(\\.\pipe\bsk-daemon-{user}-{hash}). When the username contains
apostrophes (e.g. z'z'f'l'g'y), NPFS behaviour diverges between create
and open: the daemon's CreateNamedPipeW reports success and logs that it
is listening, yet every client CreateFileW on the same name fails with
ERROR_FILE_NOT_FOUND / ERROR_PATH_NOT_FOUND, so the CLI can never reach
the daemon (issue #75).
The name is now hash-only (\\.\pipe\bsk-daemon-{hash}); the hash
already covers user + home, so uniqueness and per-user/BSK_HOME
isolation are unchanged, and no raw username characters can reach NPFS.
The pipe name is exchanged exclusively through daemon.json (the CLI
never computes it), so old/new CLI/daemon combinations stay compatible.
Unit tests pin the safe character set for apostrophe/space/Unicode
usernames and the per-(user, home) uniqueness.
Fixes #75
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.
Motivation
Fixes #75 — on Windows machines whose username contains an apostrophe (e.g.
z'z'f'l'g'y), the CLI can never reach the daemon over its named-pipe IPC. The daemon logs that it is listening on\\.\pipe\bsk-daemon-z'z'f'l'g'y-<hash>, yet every clientCreateFileWon that exact name fails withERROR_FILE_NOT_FOUND/ERROR_PATH_NOT_FOUND.Root cause
crates/bsk-cli/src/daemon/paths.rsembedded%USERNAME%verbatim in the pipe name (\\.\pipe\bsk-daemon-{user}-{hash}). Although Microsoft documentation says any character except backslash is legal in a pipe name, NPFS behaviour for such names diverges in practice between create and open:CreateNamedPipeWsucceeds while clients opening the same name get FILE_NOT_FOUND. Both sides were verified to use byte-identical names (the CLI reads the name fromdaemon.json, never recomputes it), so the divergence sits at the Win32/NPFS boundary with the raw special characters.Changes
\\.\pipe\bsk-daemon-{hash}. The hash already coversuser + bsk_home, so uniqueness and per-user / per-BSK_HOMEisolation are exactly as before — but no raw username characters (apostrophes, spaces, non-ASCII) ever reach NPFS.render_pipe_name(user, home)helper so its output charset is unit-testable on any host.Backward compatibility
The pipe name is exchanged exclusively through
daemon.json(written by the daemon, read by the CLI — the CLI never computespipe_name()itself; verified there are only two call sites, both inside the daemon process). So any old/new CLI↔daemon combination keeps working: whichever side runs the new code simply writes/reads a different name string.Other username-in-name sites
Grepped the whole CLI/protocol crates for
%USERNAME%,USER,LOGNAME,whoami,user*name: this was the only place embedding the OS username into an IPC/filesystem name. Unix socket paths live under$HOME/.bsk/run/where apostrophes are legal path characters, so no change needed there.Test plan
cargo fmt --all -- --checkcargo clippy --workspace --all-targets --locked -- -D warningscargo test --workspace --locked(newdaemon::pathstests included)