3 Commits

Include renames

Author	SHA1	Message	Date
enricobuehler	5ebe840320	fix(client/windows): settings persist when the app isn't installed on C: ... 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.	2026-08-05 22:33:36 +02:00
enricobuehlerandClaude Opus 5	0985726415	fix(host,web): an empty update channel stops looking like a broken host ... A channel nobody has published to answers `manifest.json` with a 404, and the check reported that the same way it reports a dead registry or a bad signature: "Last check failed: feed returned HTTP 404". Every host on the stable channel shows it today, because the stable manifest only publishes when someone dispatches `announce` for a release tag — so the first thing an operator sees from the new Updates card is a red failure caused by nothing being wrong. The shared checker now distinguishes the two. `feed::fetch_manifest_blocking` returns a typed `FeedError` instead of a string, and only a 404 on the manifest ITSELF becomes `NotPublished` — a 404 on the detached signature still fails loudly, because that is the half-published pair the manifest-then-signature upload order can produce, and it must stay fail-closed. The host carries that through as `UpdateStatus.not_published`, mutually exclusive with `last_error`. It is benign only while no manifest has ever been seen for the channel: once a check has succeeded, the same 404 means the feed LOST a document it used to serve, which stays an error. The console then shows a plain sentence naming the channel instead of the failure banner, and "None published yet" rather than "Not checked yet". The Linux client makes the same distinction but deliberately NOT the same choice: `--check-update` keeps exiting 1 and keeps `error` set, because its consumer is a shell script and an empty channel is the absence of evidence that this build is current — not a confirmation that it is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-01 00:37:56 +02:00
3 people	a87fbebc0a	feat(client/linux): the client says whether it is current, and can update itself ... `punktfunk-client --check-update` fetches the same Ed25519-signed per-channel manifest the host trusts, works out how this client was installed, and reports what — if anything — can be done about a newer build. `--apply-update` does the part it owns. `--version` prints the CI-stamped build string, which is both what the comparison needs and what a packaging gate can run the binary for. The engine lives here rather than in the Decky plugin for two reasons: verifying a signature needs crypto that Decky's embedded Python does not have, and the plugin is not the only surface that wants the answer. Every UI becomes a caller, exactly as it already is for --pair, --library and --list-hosts. What is possible depends on the install, and the module refuses to pretend otherwise. A flatpak is a per-user install its launcher can update. A .deb/.rpm/ pacman client goes through the packaged root helper — a fixed, parameterless systemd oneshot polkit authorises for members of the punktfunk-update group, so the request carries nothing an attacker could influence and the payload comes from the distro's own signed repositories. A sysext, a nix profile or a source build gets the command and nothing else: the signed sysext feed carries the HOST image, so a client sysext has no feed to update from, and a button that can only fail is worse than one honest line. A failed check reports the failure. "Could not tell" rendering as "up to date" is the one answer that must never happen. The box's capabilities are probed once into a `Caps` struct so the routing rules are a pure function of (install kind, caps) — all seven of them are tested without a box, including that the kill switch beats every kind and that pacman needs its own root-owned full-sysupgrade opt-in on top of the group. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-07-31 09:41:26 +02:00