fix(client/windows): settings persist when the app isn't installed on C: #62

Merged

enricobuehler merged 1 commits from worktree-client-msix-persist into main 2026-08-05 20:53:40 +00:00

Conversation 0 Commits 1 Files Changed 3

+256 -15

enricobuehler commented 2026-08-05 20:39:54 +00:00

Owner

Copy Link

Copy Source

Field report (2026-08-05, bonorenof): fresh Win11 with a data partition, "New apps will save to: D:", client installed there. It launches, finds hosts and streams — but no setting and no profile survives a restart. Reinstalling to C: fixes it completely. Reporter's read was "it's in read-only mode".

What localises it

The client generates its mTLS identity with a plain fs::write on first run and hard-exits if that fails. Their app started, so ordinary file creation in the config directory works. Only the three stores that go through write_atomic (settings, profiles, known-hosts) were being lost — and that helper writes a sibling temp, then renames it over the target.

So: creation works, the rename doesn't.

Why the rename fails

The client ships as a full-trust MSIX package, so %APPDATA% writes are redirected into the package container. With the package on a secondary drive, Windows keeps that redirected state on the package's volume: C:\Users\<u>\AppData\Local\Packages\<pfn>\ stays a real directory on C:, but its children (LocalCache, RoamingState, …) are junctions to D:\WpSystem\<SID>\….

Both sides of our rename still spell C:\Users\…, so nothing looks unusual — but they can resolve across that junction boundary, and std::fs::rename is MoveFileExW with MOVEFILE_REPLACE_EXISTING and not MOVEFILE_COPY_ALLOWED, so a cross-volume move fails outright rather than degrading to a copy.

The change

Not "make the rename work" — stop treating it as the only way to persist.

• write_atomic falls back to an in-place write when the atomic route fails, then reads the bytes back to verify. That fallback is the same operation the identity files already use, and those demonstrably round-trip on the affected install. Temp+rename stays the normal route everywhere it works. Microsoft documents a single private-location-first resolution order for both reads and writes, so a write cannot land in a layer the next read misses — the read-back is belt-and-braces on a path that only runs when the install has already proven it does something unusual.
• Pid-scoped temp path. It was a single shared <name>.json.tmp, but these stores have five whole-file writers (WinUI shell, session, console UI, CLI, Decky). Two saving at once collide — on Windows the second write hits a sharing violation, and worse, one process can rename the other's half-written bytes over the target.
• trust::store_health + an error bar on Settings. Every save here is fire-and-forget by design (a failed settings write must never take a stream down), so ~15 call sites discard the error and the UI cheerfully shows the toggle you just moved. The cell records the last failure centrally, so a client that cannot save now says so and names the path, without unpicking any call site.
• update.rs had hand-rolled the same temp+rename inline (no temp cleanup, no fallback) → routed through the one writer. A floor that silently never rises is how a declined update comes back forever.

Deliberately not done

Disabling MSIX AppData virtualization in the manifest (desktop6:FileSystemWriteVirtualization). It stops the redirection at source, but every existing packaged install's settings, profiles and pairings live inside the container today — flipping it points the client at an empty real %APPDATA% and silently resets all of them. That needs a migration, not a manifest flag.

Resolving the destination dir via GetFinalPathNameByHandleW and creating the temp inside it (to keep atomicity). It does not reliably close the hole — when the target exists only in the unvirtualized layer while its directory resolves to the private one, the rename still straddles — and it would rest on canonicalisation behaving through the redirection, which we have never verified on a packaged run.

Verification

Run on the RTX box (.173, Win11 26200) — the platform that actually has these rename semantics; Linux cannot exercise this bug's platform. .133 was down.

gate	result
cargo fmt --all --check	0
cargo test -p pf-client-core --lib (native Windows)	109 passed, 0 failed
cargo clippy --all-targets -p pf-client-core -- -D warnings	0
cargo clippy --all-targets -p punktfunk-client-windows -- -D warnings	0

Also green under pf-lxcheck2 linux/amd64 (116 passed). Three new tests: the pid-scoped scratch path; the fallback persisting and reading back when the atomic route is blocked; and a genuinely unwritable store surfacing its error instead of swallowing it.

Caveats

• Not reproduced on hardware — no second-drive install exists here. The mechanism is established from Microsoft docs plus third-party reports. The fix does not depend on the diagnosis being exactly right: it repairs any install where the rename fails but a direct write succeeds.
• The reporter could send no logs because "Open log folder" opens Documents — a separate bug, already fixed on main in f3c0ee47, but not in the v0.24.0 tag they were running. Both fixes ship together in 0.25.0.
• Interim workaround for the reporter: install on C:.

Field report (2026-08-05, bonorenof): fresh Win11 with a data partition, "New apps will save to: **D:**", client installed there. It launches, finds hosts and streams — but no setting and no profile survives a restart. Reinstalling to C: fixes it completely. Reporter's read was "it's in read-only mode". ## What localises it The client generates its mTLS identity with a plain `fs::write` on first run and hard-exits if that fails. **Their app started**, so ordinary file creation in the config directory works. Only the three stores that go through `write_atomic` (settings, profiles, known-hosts) were being lost — and that helper writes a sibling temp, then renames it over the target. **So: creation works, the rename doesn't.** ## Why the rename fails The client ships as a full-trust MSIX package, so `%APPDATA%` writes are redirected into the package container. With the package on a secondary drive, Windows keeps that redirected state on the *package's* volume: `C:\Users\<u>\AppData\Local\Packages\<pfn>\` stays a real directory on C:, but its children (`LocalCache`, `RoamingState`, …) are junctions to `D:\WpSystem\<SID>\…`. Both sides of our rename still spell `C:\Users\…`, so nothing looks unusual — but they can resolve across that junction boundary, and `std::fs::rename` is `MoveFileExW` with `MOVEFILE_REPLACE_EXISTING` and **not** `MOVEFILE_COPY_ALLOWED`, so a cross-volume move fails outright rather than degrading to a copy. ## The change Not "make the rename work" — stop treating it as the only way to persist. - **`write_atomic` falls back to an in-place write** when the atomic route fails, then **reads the bytes back** to verify. That fallback is the same operation the identity files already use, and those demonstrably round-trip on the affected install. Temp+rename stays the normal route everywhere it works. Microsoft documents a single private-location-first resolution order for *both* reads and writes, so a write cannot land in a layer the next read misses — the read-back is belt-and-braces on a path that only runs when the install has already proven it does something unusual. - **Pid-scoped temp path.** It was a single shared `<name>.json.tmp`, but these stores have five whole-file writers (WinUI shell, session, console UI, CLI, Decky). Two saving at once collide — on Windows the second write hits a sharing violation, and worse, one process can rename the other's half-written bytes over the target. - **`trust::store_health` + an error bar on Settings.** Every save here is fire-and-forget by design (a failed settings write must never take a stream down), so ~15 call sites discard the error and the UI cheerfully shows the toggle you just moved. The cell records the last failure centrally, so a client that cannot save now says so and names the path, without unpicking any call site. - **`update.rs`** had hand-rolled the same temp+rename inline (no temp cleanup, no fallback) → routed through the one writer. A floor that silently never rises is how a declined update comes back forever. ## Deliberately not done **Disabling MSIX AppData virtualization in the manifest** (`desktop6:FileSystemWriteVirtualization`). It stops the redirection at source, but every existing packaged install's settings, profiles and **pairings** live inside the container today — flipping it points the client at an empty real `%APPDATA%` and silently resets all of them. That needs a migration, not a manifest flag. **Resolving the destination dir via `GetFinalPathNameByHandleW`** and creating the temp inside it (to keep atomicity). It does not reliably close the hole — when the target exists only in the unvirtualized layer while its directory resolves to the private one, the rename still straddles — and it would rest on canonicalisation behaving through the redirection, which we have never verified on a packaged run. ## Verification Run on the RTX box (.173, Win11 26200) — the platform that actually has these rename semantics; Linux cannot exercise this bug's platform. `.133` was down. | gate | result | |---|---| | `cargo fmt --all --check` | 0 | | `cargo test -p pf-client-core --lib` (native Windows) | **109 passed**, 0 failed | | `cargo clippy --all-targets -p pf-client-core -- -D warnings` | 0 | | `cargo clippy --all-targets -p punktfunk-client-windows -- -D warnings` | 0 | Also green under `pf-lxcheck2` linux/amd64 (116 passed). Three new tests: the pid-scoped scratch path; the fallback persisting **and reading back** when the atomic route is blocked; and a genuinely unwritable store surfacing its error instead of swallowing it. ## Caveats - **Not reproduced on hardware** — no second-drive install exists here. The mechanism is established from Microsoft docs plus third-party reports. **The fix does not depend on the diagnosis being exactly right**: it repairs any install where the rename fails but a direct write succeeds. - The reporter could send no logs because "Open log folder" opens Documents — a **separate** bug, already fixed on main in `f3c0ee47`, but **not in the v0.24.0 tag** they were running. Both fixes ship together in 0.25.0. - Interim workaround for the reporter: install on C:.

enricobuehler added 1 commit 2026-08-05 20:39:55 +00:00

fix(client/windows): settings persist when the app isn't installed on C: ... 5ebe840320

Reported from the field (2026-08-05): a fresh Windows 11 box with a data
partition, "New apps will save to: D:", and the client installed there. It
launches, finds hosts and streams — but no setting and no profile survives a
restart. Reinstalling to C: fixes it completely. The reporter's read was "it's
in read-only mode", and that is almost exactly right.

The one clue that localises it: the client creates its mTLS identity with a
plain `fs::write` on first run and hard-exits if that fails. Their app started,
so ordinary file creation in the config directory works. Only the config stores
were being lost — and those are the three files that go through `write_atomic`,
which writes a sibling temp and renames it over the target.

The rename is what breaks. The client ships as a full-trust MSIX package, so
its `%APPDATA%` writes are redirected into the package container. When the
package lives on a secondary drive, Windows keeps that redirected state on the
package's own volume: `C:\Users\<u>\AppData\Local\Packages\<pfn>\` stays a real
directory on C:, but its children (LocalCache, RoamingState, …) are junctions to
`D:\WpSystem\<SID>\…`. Both sides of our rename still spell `C:\Users\…`, so
nothing looks unusual, but they can resolve across that junction boundary — and
`std::fs::rename` is `MoveFileExW` with `MOVEFILE_REPLACE_EXISTING` and *not*
`MOVEFILE_COPY_ALLOWED`, so a cross-volume move fails outright rather than
degrading to a copy. Creating files still works, which is why everything else
about the install looks healthy.

So the fix is not to make the rename work — it is to stop treating it as the
only way to persist. `write_atomic` now falls back to writing the target in
place when the atomic route fails. That is the same operation the identity files
already use, and those demonstrably round-trip on the affected installs, so the
fallback lands on a path we know resolves. It trades crash-atomicity for exactly
the writes that would otherwise be lost, and nowhere else: temp+rename stays the
normal route everywhere it works.

Writing into a redirected location cannot desync from reading it — Microsoft
documents one private-location-first resolution order for both, so whichever
layer a write lands in is the layer the next read finds. The fallback verifies
anyway, by reading the bytes straight back: a write that reports success and
disappears is precisely the bug being fixed, so this path does not get to claim
success on an `Ok(())` alone. It costs nothing normally — it only runs on an
install that has already shown it does something unusual.

Two things this uncovered on the way:

The temp file was a single shared `<name>.json.tmp`, but these stores have five
whole-file writers (WinUI shell, session, console UI, CLI, Decky). Two saving at
once collide on it — on Windows the second write hits a sharing violation, and
worse, one process can rename the other's half-written bytes over the target.
The scratch path now carries the pid.

And none of this was visible to anyone. Every save on this page is
fire-and-forget by design (a failed settings write must never take a stream
down), so ~15 call sites discard the error and the UI cheerfully shows the
toggle you just moved. The reporter had no log file to send either, because
"Open log folder" was handing out a phantom path — a separate bug, already fixed
in f3c0ee47 but not in the 0.24.0 they were running. `store_health` records the
last persistence failure centrally, and Settings shows an error bar naming the
path when the store is refusing writes, so a client that cannot save says so
instead of pretending.

`update.rs` had hand-rolled the same temp+rename inline, so it neither cleaned
up its temp on a failed rename nor picks up the fallback; it now goes through
the one writer. The update floor silently never rising is how a declined update
comes back forever.

Deliberately NOT done: disabling MSIX AppData virtualization in the manifest
(`desktop6:FileSystemWriteVirtualization`). It would stop the redirection at the
source, but every existing packaged install's settings, profiles and pairings
live inside the container today — turning it off points the client at an empty
real `%APPDATA%` and silently resets all of them. That needs a migration, not a
manifest flag.

Also considered and not taken: resolving the destination directory with
`GetFinalPathNameByHandleW` and creating the temp inside the resolved path, to
keep atomicity. It does not reliably close this hole — when the target file
exists only in the unvirtualized layer while its directory resolves to the
private one, the rename still straddles the boundary — and it would rest on
canonicalisation behaving through the redirection, which we have never verified
on a packaged run.

Verified on the RTX box (.173, Windows 11 26200), which is the platform that
actually has these rename semantics: `cargo fmt --all --check`, the full
`pf-client-core` lib suite (109 passed), and clippy `-D warnings --all-targets`
on both `pf-client-core` and `punktfunk-client-windows` — all clean. Also green
under linux/amd64 (116 passed). Three new tests: the pid-scoped scratch path,
the fallback actually persisting and reading back when the atomic route is
blocked, and a genuinely unwritable store surfacing its error instead of
swallowing it.

The mechanism above is established from documentation and third-party reports,
not from a reproduction on a second-drive install — that box does not exist
here. The fix does not depend on the diagnosis being exactly right: it repairs
any install where the rename fails but a direct write succeeds.

enricobuehler merged commit de6b9e94ec into main 2026-08-05 20:53:40 +00:00

enricobuehler deleted branch worktree-client-msix-persist 2026-08-05 20:53:42 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

FernandoMX enricobuehler (Enrico Bühler) kszlim nannku neoney sassycrown

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unom/punktfunk#62