The comments were the spec, and the code had drifted — 2026-08-25 security review #396

Merged

enricobuehler merged 7 commits from worktree-security-review-fixes into main 2026-08-26 18:56:35 +00:00

Conversation 2 Commits 7 Files Changed 117

+4134 -883

enricobuehler commented 2026-08-26 07:23:29 +00:00

Owner

Copy Link

Copy Source

Fixes the confirmed findings from the 2026-08-25 full-surface security review (58 confirmed of 89 candidates).

Critical

A console session cookie alone reached code execution on the host. confirm.ts gated three routes; pairing was not among them, and every pairing route rode the generic catch-all with the operator's admin bearer attached — so a cookie without the console password could approve its own device into full, permanent access with keyboard and mouse on the host desktop.

Arming, approving and PIN submission now sit behind the console password. The GET status route could not be gated as the report proposed (a password cannot ride a 1 s poll), so instead the PIN is stripped from it entirely and returned exactly once in the password-gated arm response — the disclosure is removed rather than guarded. UI wired through; the one cost is that reloading mid-window loses the displayed PIN and the card falls back to its arm form.

High

Plugin lane no longer reads the unredacted log ring (it carried the very webhook credentials the /hooks carve-out withholds); hook lines log an origin + short id, never a URL or command line (23 sites, not the 8 the report counted). Plugin-reported pids are held to procscan's start-time floor before the SYSTEM host will signal them. ClipOffer gated on the live grant mask — including the host-opened fetch stream the report missed, which kept pulling bytes from a revoked client on every host paste. ENet refuses connects with no live launch instead of letting LAN peers squat all four slots. Windows secrets are created empty, DACL'd, then written, with DACL failure fatal. The sysext feed binds FEED + a monotonic SERIAL inside the signed bytes. privileged_field inverted to an allowlist. Five parser panics reachable from one malformed NALU range-checked (plus two more found in the same path). Release-signing jobs pin bun/sccache/actions by checksum/SHA. h2 → 0.4.19 (RUSTSEC-2026-0258).

Where the report was wrong

Three of its suggested fixes would have caused regressions if applied literally, and are noted in the commit:

• the gamelease slack comparison is inverted in the report — implementing it would reject legitimate game launches;
• the proposed launch-kind allowlist would have broken four shipped plugin flows (plugin, playnite, launcher_ui, lutris_id/heroic);
• routing deep links through the existing trust sheet would have done a no-PIN request-access connect and dropped the host's pin.

One finding does not reproduce: the reassembler stopped allocating max_frame_bytes up front in 97b2c01a, three weeks before the audited commit. A stale comment describing the old design is what misled the audit — corrected here.

Deliberately not fixed

• Plugin token → command execution. The plugin launch kind exists so a plugin names a command the host runs; per-plugin identity does not change that, and the runner is one process hosting plugins as fibers, so there is nowhere to hang a per-plugin credential. The real gap is narrower: on Windows the runner is shared LocalService, so anything else running as it can read plugin-token. Closing that means process-per-plugin (breaking for plugin authors) or running the plugin's answer as the runner's principal. The docs that claimed the guarantee are corrected.
• Shared plugin-UI origin — needs per-plugin listeners plus a trusted local CA; a per-origin self-signed cert exception per plugin is not shippable.
• Rollback Authenticode publisher pin — leaf pinning provably cannot work (Azure Trusted Signing mints a fresh leaf per request, ~3 day validity) and the real signer subject is not in the tree.
• Hole-punch selection — needs an authenticated punch payload; deployed clients would keep sending bare magic. Regression test pins the current guarantee.

Verification

• cargo clippy --workspace --all-targets --locked -- -D warnings — clean in the CI image (pinned 1.96.0)
• cargo fmt --all -- --check — clean
• web: bun run build + tsc --noEmit clean, 22/22 server tests, i18n parity
• Apple: Swift debug and release builds, 26/26 tests · Android: :kit 7/7, :app compiles
• ⚠ windows-host.yml dispatched separately — it never runs on pull_request, and this branch changes a lot of Windows-gated code

Operator actions required before merge

1. New CI secret DEPLOY_KNOWN_HOSTS (ssh-keyscan -p "$DEPLOY_PORT" "$DEPLOY_HOST") — until set, the Nix cache publish skips with a warning instead of TOFU-connecting.
2. Re-seal each live sysext feed once: TOKEN=… bash packaging/bazzite/publish-sysext-feed.sh --seal <feed> for f43, f43-canary, f44, f44-canary. Idempotent, no image rebuild. The consumer hard-requires the new headers — a soft-warn transition would have been the replay hole wearing a compatibility hat.

Known follow-ups

• clients/windows deep links currently refuse name-resolved links with an explanatory message (marked ⚠ in code) — that shell has no prompt surface; it needs a real dialog from its owner.
• Same accept-new SSH and pipe-to-bash patterns remain in flatpak.yml, deb.yml, arch.yml (deb also holds a signing key).
• Pre-existing arm64 c_char bug in pf-zerocopy/src/imp/egl/gl.rs:203 blocks native Apple-silicon checks downstream of pf-capture; invisible to CI, which is x86_64 only.

Fixes the confirmed findings from the 2026-08-25 full-surface security review (58 confirmed of 89 candidates). ## Critical A console **session cookie alone reached code execution on the host**. `confirm.ts` gated three routes; pairing was not among them, and every pairing route rode the generic catch-all with the operator's admin bearer attached — so a cookie without the console password could approve its own device into full, permanent access with keyboard and mouse on the host desktop. Arming, approving and PIN submission now sit behind the console password. The GET status route could not be gated as the report proposed (a password cannot ride a 1 s poll), so instead the **PIN is stripped from it entirely** and returned exactly once in the password-gated arm response — the disclosure is removed rather than guarded. UI wired through; the one cost is that reloading mid-window loses the displayed PIN and the card falls back to its arm form. ## High Plugin lane no longer reads the unredacted log ring (it carried the very webhook credentials the `/hooks` carve-out withholds); hook lines log an origin + short id, never a URL or command line (23 sites, not the 8 the report counted). Plugin-reported pids are held to procscan's start-time floor before the SYSTEM host will signal them. `ClipOffer` gated on the live grant mask — including the host-opened fetch stream the report missed, which kept pulling bytes from a revoked client on every host paste. ENet refuses connects with no live launch instead of letting LAN peers squat all four slots. Windows secrets are created empty, DACL'd, *then* written, with DACL failure fatal. The sysext feed binds `FEED` + a monotonic `SERIAL` inside the signed bytes. `privileged_field` inverted to an allowlist. Five parser panics reachable from one malformed NALU range-checked (plus two more found in the same path). Release-signing jobs pin bun/sccache/actions by checksum/SHA. `h2` → 0.4.19 (RUSTSEC-2026-0258). ## Where the report was wrong Three of its suggested fixes would have caused regressions if applied literally, and are noted in the commit: - the gamelease slack comparison is **inverted** in the report — implementing it would reject legitimate game launches; - the proposed launch-kind allowlist would have broken four shipped plugin flows (`plugin`, `playnite`, `launcher_ui`, `lutris_id`/`heroic`); - routing deep links through the existing trust sheet would have done a no-PIN request-access connect and **dropped the host's pin**. One finding does not reproduce: the reassembler stopped allocating `max_frame_bytes` up front in `97b2c01a`, three weeks before the audited commit. A stale comment describing the old design is what misled the audit — corrected here. ## Deliberately not fixed - **Plugin token → command execution.** The `plugin` launch kind exists so a plugin names a command the host runs; per-plugin identity does not change that, and the runner is one process hosting plugins as fibers, so there is nowhere to hang a per-plugin credential. The real gap is narrower: on Windows the runner is shared `LocalService`, so anything else running as it can read `plugin-token`. Closing that means process-per-plugin (breaking for plugin authors) or running the plugin's answer as the runner's principal. The docs that claimed the guarantee are corrected. - **Shared plugin-UI origin** — needs per-plugin listeners plus a trusted local CA; a per-origin self-signed cert exception per plugin is not shippable. - **Rollback Authenticode publisher pin** — leaf pinning provably cannot work (Azure Trusted Signing mints a fresh leaf per request, ~3 day validity) and the real signer subject is not in the tree. - **Hole-punch selection** — needs an authenticated punch payload; deployed clients would keep sending bare magic. Regression test pins the current guarantee. ## Verification - `cargo clippy --workspace --all-targets --locked -- -D warnings` — **clean** in the CI image (pinned 1.96.0) - `cargo fmt --all -- --check` — **clean** - web: `bun run build` + `tsc --noEmit` clean, 22/22 server tests, i18n parity - Apple: Swift debug **and** release builds, 26/26 tests · Android: `:kit` 7/7, `:app` compiles - ⚠ `windows-host.yml` dispatched separately — it never runs on `pull_request`, and this branch changes a lot of Windows-gated code ## Operator actions required before merge 1. **New CI secret `DEPLOY_KNOWN_HOSTS`** (`ssh-keyscan -p "$DEPLOY_PORT" "$DEPLOY_HOST"`) — until set, the Nix cache publish skips with a warning instead of TOFU-connecting. 2. **Re-seal each live sysext feed once**: `TOKEN=… bash packaging/bazzite/publish-sysext-feed.sh --seal <feed>` for `f43`, `f43-canary`, `f44`, `f44-canary`. Idempotent, no image rebuild. The consumer hard-requires the new headers — a soft-warn transition would have been the replay hole wearing a compatibility hat. ## Known follow-ups - `clients/windows` deep links currently **refuse** name-resolved links with an explanatory message (marked ⚠ in code) — that shell has no prompt surface; it needs a real dialog from its owner. - Same `accept-new` SSH and pipe-to-bash patterns remain in `flatpak.yml`, `deb.yml`, `arch.yml` (deb also holds a signing key). - Pre-existing arm64 `c_char` bug in `pf-zerocopy/src/imp/egl/gl.rs:203` blocks native Apple-silicon checks downstream of pf-capture; invisible to CI, which is x86_64 only.

enricobuehler added 1 commit 2026-08-26 07:23:30 +00:00

fix(host,web,clients,ci): the comments were the spec, and the code had drifted ... 98e68a49a7

Security review 2026-08-25, 58 confirmed findings across host, console,
clients and supply chain. Nearly every serious one is a documented boundary
whose code stopped enforcing what its comment promised — so where the two
disagreed, the comment won and the code was made to match, and where it
could not be, the comment was corrected instead.

Critical — a console session cookie alone reached code execution: every
pairing route rode the generic catch-all with the operator's admin bearer
attached. Arming, approving and PIN submission now sit behind the console
password like the other trust-root routes, and the armed PIN is returned
once in that gated response instead of riding a 1 s status poll.

High — the plugin lane no longer reads the unredacted log ring (which
carried the webhook credentials the /hooks carve-out exists to withhold);
hook lines log an origin and a short id, never a URL or a command line; a
plugin-reported pid is held to procscan's start-time floor before the
SYSTEM host will signal it; ClipOffer is gated on the live grant mask, so a
revoked guest loses the host clipboard in both directions; ENet refuses
connects with no live launch instead of letting LAN peers squat all four
slots; Windows secrets are born with their DACL applied rather than
world-readable; the sysext feed binds FEED and a monotonic SERIAL inside
the signed bytes; privileged_field allowlists the host-resolved launch
kinds so a new kind is privileged by default; five parser panics reachable
from one malformed NALU are range-checked; release-signing jobs pin bun,
sccache and actions by checksum/SHA; h2 -> 0.4.19 (RUSTSEC-2026-0258).

Deep links only auto-dial by stable record id now — a display name or an
address gets a confirmation on every client. The Apple identity key moves
to ThisDeviceOnly so it stops riding encrypted backups.

pf-vdisplay stops routing session identity through the process environment:
the injector backend threads through a typed slot, so per-batch getenv no
longer races a per-session setenv. The four remaining writes have no
in-repo readers and are documented as such; the SAFETY proof that claimed
ENV_LOCK made them sound is gone.

Verified: cargo clippy --workspace --all-targets --locked -D warnings and
cargo fmt --all --check clean in the CI image; web builds, tsc --noEmit
clean, 22/22 server tests; Swift debug+release + 26/26, Kotlin :kit 7/7.

Not fixed, deliberately: the plugin token can still mint command execution
(the plugin launch kind exists so a plugin names a command the host runs —
per-plugin identity does not change that, and the runner is one process
hosting fibers, so there is nowhere to hang a credential); the shared
plugin-UI origin; the rollback Authenticode publisher pin (Azure mints a
fresh leaf per request, and the signer subject is not in the tree). Each is
now described accurately where it lives instead of being claimed closed.

enricobuehler added 1 commit 2026-08-26 07:32:48 +00:00

fix(inject): the doc naming the write we removed trips the grep gate ... 1b8d4799ef

Gate C in check-unsafe-hygiene.sh matches the qualified `env::set_var`
anywhere in a file, comments included — deliberately, so the contract stays
dumb and stable. The new SESSION_BACKEND doc explained what it replaced by
naming it, which reads as a fresh call site to the gate. Reword rather than
raise the baseline: the baseline is there to catch a real write later, and
this change deleted one.

enricobuehler commented 2026-08-26 14:19:43 +00:00

Author

Owner

Copy Link

Copy Source

Verification results

CI on 1b8d4799 — all jobs green: rust, rust-arm64, web, docs-site, docs-drift, bun-nix.

windows-host.yml dispatched on the branch (run 20335) — success, and it genuinely compiled rather than skipping: Build (release, nvenc + amf-qsv + qsv), Build (status tray), Clippy (host + tray, Windows), Test (pf-capture), Test (pf-vdisplay), then packed and signed the installer. That is the only coverage the Windows-gated changes get (pf-paths' new DACL ordering, windows/service.rs, client_logs.rs, gog_spawn, hooks' Windows sites, sendinput.rs), since this workflow never runs on pull_request.

Locally, in the CI image (punktfunk-rust-ci, pinned 1.96.0):

• cargo clippy --workspace --all-targets --locked -- -D warnings — clean
• cargo fmt --all -- --check — clean
• cargo test --workspace --locked — 2280 tests pass across 43 suites, plus the C ABI harness round-trip

The only test failures anywhere are 3–4 in pf-zerocopy's imp/client.rs / imp/ipc.rs. Proven pre-existing: the same tests fail identically on the pristine base commit d2c6e1c9 in the same container, the failing set varies between runs, and neither file is touched by this branch. Both symptoms are QEMU artifacts — EINTR on a timed recv, and a re-exec'd worker subprocess rejecting --fd.

One CI fix during review

The first push failed rust on the unsafe-hygiene gate: crates/pf-inject/src/lib.rs: 1 process-global-API mentions (baseline 0). Gate C greps for env::set_var anywhere in a file, comments included — deliberately ("the grep is the contract; keep it dumb and stable"). The new SESSION_BACKEND doc explained what it replaced by naming it, which reads as a fresh call site. Reworded rather than raising the baseline: the baseline exists to catch a real write later, and this change deleted one.

A downstream break the review pass caught

Growing the HEVC tile arrays to 20/22 (needed — upstream's were one entry short of the spec maximum, so bounding at the array size would have refused a conformant 20-column stream) broke pf-vaadec and pf-vkdecode, which still declared 19/21 and were in nobody's scope. pf-vaadec's narrowing helpers now take slices, so the libva ABI side stays frozen at 19/21 — it has offset_of! assertions, those sizes are hardware contract — while the parser side can grow again without breaking the pair.

## Verification results **CI on `1b8d4799` — all jobs green**: `rust`, `rust-arm64`, `web`, `docs-site`, `docs-drift`, `bun-nix`. **`windows-host.yml` dispatched on the branch** (run [20335](https://git.unom.io/unom/punktfunk/actions/runs/20335)) — **success**, and it genuinely compiled rather than skipping: Build (release, nvenc + amf-qsv + qsv), Build (status tray), **Clippy (host + tray, Windows)**, Test (pf-capture), Test (pf-vdisplay), then packed and signed the installer. That is the only coverage the Windows-gated changes get (`pf-paths`' new DACL ordering, `windows/service.rs`, `client_logs.rs`, `gog_spawn`, hooks' Windows sites, `sendinput.rs`), since this workflow never runs on `pull_request`. **Locally, in the CI image** (`punktfunk-rust-ci`, pinned 1.96.0): - `cargo clippy --workspace --all-targets --locked -- -D warnings` — clean - `cargo fmt --all -- --check` — clean - `cargo test --workspace --locked` — **2280 tests pass across 43 suites**, plus the C ABI harness round-trip The only test failures anywhere are 3–4 in `pf-zerocopy`'s `imp/client.rs` / `imp/ipc.rs`. **Proven pre-existing**: the same tests fail identically on the pristine base commit `d2c6e1c9` in the same container, the failing set varies between runs, and neither file is touched by this branch. Both symptoms are QEMU artifacts — `EINTR` on a timed `recv`, and a re-exec'd worker subprocess rejecting `--fd`. ### One CI fix during review The first push failed `rust` on the unsafe-hygiene gate: `crates/pf-inject/src/lib.rs: 1 process-global-API mentions (baseline 0)`. Gate C greps for `env::set_var` **anywhere in a file, comments included** — deliberately ("the grep is the contract; keep it dumb and stable"). The new `SESSION_BACKEND` doc explained what it replaced by naming it, which reads as a fresh call site. Reworded rather than raising the baseline: the baseline exists to catch a real write later, and this change deleted one. ### A downstream break the review pass caught Growing the HEVC tile arrays to 20/22 (needed — upstream's were one entry short of the spec maximum, so bounding at the array size would have refused a conformant 20-column stream) broke `pf-vaadec` and `pf-vkdecode`, which still declared 19/21 and were in nobody's scope. `pf-vaadec`'s narrowing helpers now take slices, so the libva ABI side stays frozen at 19/21 — it has `offset_of!` assertions, those sizes are hardware contract — while the parser side can grow again without breaking the pair.

enricobuehler added 1 commit 2026-08-26 15:41:00 +00:00

fix(windows,ci,zerocopy): finish the deferred half — a real prompt, the rest of the pipes, and an arch bug CI cannot see ... f632ee68d2

Follow-ups left open by the security-review pass.

The Windows client had the deep-link fix's enforcement but not its UX: a
name- or address-resolved link was REFUSED with a note pointing at the host
list, marked in-code as degraded, because that shell had no prompt surface
wired up. It has one — `ContentDialog`, already used for "Remove saved
host?" and "Delete profile?" — so the link now asks, naming the host and
anything it wants to launch, and on confirm runs the identical dial the
id-referenced path runs. Same wake, trust and error surfaces; confirming is
one click in front of the same code.

The supply-chain pins only covered the jobs the report cited. deb.yml
(a signing-key job) and arch.yml still piped bun's installer into bash;
flatpak.yml still took the deploy host key on first contact while holding
FLATPAK_GPG_PRIVATE_KEY. deb now takes the same pinned, SHA-256-checked bun
asset as rpm and windows-host; arch takes bun from pacman, which verifies
package signatures; flatpak verifies the host key from DEPLOY_KNOWN_HOSTS
through the fail-open-through-setup preflight it already had.

Found while sweeping, worse than the cited sites: flatpak.yml fetched
flatpak-cargo-generator.py from a mutable `master` and ran it in the job
holding the GPG key — third-party Python deciding which crate sources the
signed build vendors. Pinned to a commit and checksummed.

pf-zerocopy's EGL bindings declared glShaderSource/glGetUniformLocation as
`*const i8`, but `CStr::as_ptr()` yields `*const c_char` — i8 on x86_64,
u8 on aarch64. So the crate did not compile for aarch64 at all, and CI is
x86_64-only and could never see it. Verified both directions: with c_char
`cargo check -p pf-zerocopy --target aarch64-unknown-linux-gnu` passes,
and the old spelling still fails E0308 there.

Docs: the note saying Windows has no prompt is gone, and the first-connect
sentence put Windows in the wrong group — it pre-fills its trust prompt
from the link like Linux and Android; only the Apple apps show a notice.
Corrected an Apple comment claiming it seeds the add sheet with the address
and fingerprint when it only sets a notice string.

enricobuehler added 1 commit 2026-08-26 16:17:34 +00:00

fix(ci): pin the tools the builder images bake in, not just the ones the jobs fetch ... eed7b5e589

The workflow-level pins closed the bootstrap path only. On the normal path
the bun that gets vendored into the published .deb/.rpm comes from the
builder image, and every image still installed it with the upstream
installer piped into bash — so the script still chose bytes that ship to
users. rust-ci and fedora-rpm now take the same pinned, SHA-256-checked
bun-v1.3.14 asset the workflows do; arch-ci takes bun from pacman, which
verifies package signatures.

Same class, found while sweeping and worse than the cited sites: five
images fetched sccache over a version-pinned URL with no integrity check at
all. sccache is RUSTC_WRAPPER for every binary we ship — it can serve
poisoned object files straight into a signed package, which is the position
the review called the highest-leverage in a build. Download, verify against
upstream's published sum, then extract.

packaging/flatpak/build-flatpak.sh took flatpak-cargo-generator.py from a
mutable master, the same fetch flatpak.yml just pinned; both now name the
same commit and sum, so the local build and CI agree.

Arch note: bun rides the existing -Syu transaction rather than a later
layer. A separate layer resolves against the DB baked into the CACHED -Syu
layer, and Arch mirrors carry only current versions, so a cache-hit rebuild
months on would fail to fetch a package the stale snapshot names.

None of this takes effect until the images are rebuilt: docker.yml's
`builders` job keys on the git tree hash of ci/, so this re-keys the whole
family. Until then the workflow bun pins sit behind `command -v bun ||` and
short-circuit against the image's baked bun.

rustup's own installer is left piped, as apple.yml already does — pinning
rustup-init is a separate decision, and the same argument reaches every
image at once.

enricobuehler commented 2026-08-26 16:18:00 +00:00

Author

Owner

Copy Link

Copy Source

Follow-ups closed (f632ee68, eed7b5e5)

The three items this PR originally listed as known follow-ups are now done, plus two things found while doing them.

Windows deep links now ask instead of refusing. The ⚠ degraded arm is gone. It reuses ContentDialog — the same modal the shell already uses for Remove saved host? and Delete profile? — names the host and any launch id, and on confirm runs the identical dial the id-referenced path runs, so wake/trust/error surfaces cannot diverge. Verified by CI: windows-client.yml built, clippy'd (-D warnings) and tested the client on x64 and arm64, all green.

The rest of the pipe-to-bash sites. deb.yml (a signing-key job) and arch.yml no longer pipe bun's installer into bash; flatpak.yml verifies the deploy host key from DEPLOY_KNOWN_HOSTS instead of trusting first contact while holding FLATPAK_GPG_PRIVATE_KEY.

Worse than the cited sites, found by sweeping:

• flatpak.yml fetched flatpak-cargo-generator.py from a mutable master and ran it in the job holding the GPG key — third-party Python deciding which crate sources the signed build vendors. Pinned to a commit + SHA-256. I re-verified that hash independently.
• Five builder images fetched sccache over a version-pinned URL with no integrity check at all. sccache is RUSTC_WRAPPER for every binary we ship, i.e. exactly the position the review called "the highest-leverage in a build". Now verified against upstream's published sum (independently re-checked).
• The builder images were the real bun hole: the workflow pins only covered the bootstrap path, while the bun actually vendored into published packages comes from the image.

pf-zerocopy did not compile for aarch64 at all. The EGL externs declared *const i8 where CStr::as_ptr() yields *const c_char — i8 on x86_64, u8 on aarch64. Invisible to CI, which is x86_64-only, and it blocked native Apple-silicon verification of anything downstream of pf-capture. Proved both directions: cargo check -p pf-zerocopy --target aarch64-unknown-linux-gnu now passes, and the old spelling still fails E0308 there.

Rebuild required for the image pins to take effect

docker.yml's builders job keys on the git tree hash of ci/, so this re-keys the whole family (rust-ci, rust-ci-noble, gamescope-trixie, fedora-rpm, fedora44-rpm, android-ci, arch-ci, flatpak-ci, rust-ci-arm64cross). Until they are rebuilt, the workflow-level bun pins sit behind command -v bun || and short-circuit against the image's baked bun, so published packages still vendor installer-sourced bytes. The flatpak script change is not under ci/ and takes effect immediately.

Deliberately left, with reasons

• rustup's installer stays piped (4 images) — apple.yml already made that call for the identical pattern; overriding it here would be inconsistent, and pinning rustup-init is one decision that should cover every site at once.
• scripts/steamdeck/install.sh — an end-user build-from-source script holding no signing keys. It installs rustup the same way one line above, so pinning only bun there buys nothing.
• 82 tag-pinned actions/checkout@v4 / cache@v4 across 12 workflows — pinning the 19 in these files would leave a half-pinned tree where a reader cannot tell a deliberate omission from a miss, and @v4 is several majors stale (checkout v7, cache v6), so a real sweep needs a version decision rather than a SHA lookup. Worth its own mechanical PR.
• FFmpeg / nv-codec-headers git clone --branch <tag> in rust-ci-noble — tags are mutable and those .sos ship in the Ubuntu .deb, but the remedy is a pinned-commit assertion, not the checksum idiom, and it invalidates a ~20 min from-source layer. Flagging for a follow-up.

Docs

profiles-and-links.md said Windows has no prompt (now false) and put Windows in the wrong group for first connects — verified in code that Linux, Android and Windows pre-fill the trust prompt, while only the Apple apps show a notice. Also corrected an Apple comment claiming it seeds the add sheet with the address and fingerprint when it only sets a notice string, and documented DEPLOY_KNOWN_HOSTS in packaging/flatpak/README.md.

## Follow-ups closed (`f632ee68`, `eed7b5e5`) The three items this PR originally listed as known follow-ups are now done, plus two things found while doing them. **Windows deep links now ask instead of refusing.** The ⚠ degraded arm is gone. It reuses `ContentDialog` — the same modal the shell already uses for *Remove saved host?* and *Delete profile?* — names the host and any launch id, and on confirm runs the identical dial the id-referenced path runs, so wake/trust/error surfaces cannot diverge. Verified by CI: `windows-client.yml` built, clippy'd (`-D warnings`) and tested the client on **x64 and arm64**, all green. **The rest of the pipe-to-bash sites.** `deb.yml` (a signing-key job) and `arch.yml` no longer pipe bun's installer into bash; `flatpak.yml` verifies the deploy host key from `DEPLOY_KNOWN_HOSTS` instead of trusting first contact while holding `FLATPAK_GPG_PRIVATE_KEY`. **Worse than the cited sites, found by sweeping:** - `flatpak.yml` fetched `flatpak-cargo-generator.py` from a mutable `master` and ran it *in the job holding the GPG key* — third-party Python deciding which crate sources the signed build vendors. Pinned to a commit + SHA-256. I re-verified that hash independently. - Five builder images fetched **sccache** over a version-pinned URL with **no integrity check at all**. sccache is `RUSTC_WRAPPER` for every binary we ship, i.e. exactly the position the review called "the highest-leverage in a build". Now verified against upstream's published sum (independently re-checked). - The builder images were the real bun hole: the workflow pins only covered the bootstrap path, while the bun actually vendored into published packages comes from the image. **`pf-zerocopy` did not compile for aarch64 at all.** The EGL externs declared `*const i8` where `CStr::as_ptr()` yields `*const c_char` — `i8` on x86_64, `u8` on aarch64. Invisible to CI, which is x86_64-only, and it blocked native Apple-silicon verification of anything downstream of `pf-capture`. Proved both directions: `cargo check -p pf-zerocopy --target aarch64-unknown-linux-gnu` now passes, and the old spelling still fails `E0308` there. ### Rebuild required for the image pins to take effect `docker.yml`'s `builders` job keys on the git tree hash of `ci/`, so this re-keys the whole family (`rust-ci`, `rust-ci-noble`, `gamescope-trixie`, `fedora-rpm`, `fedora44-rpm`, `android-ci`, `arch-ci`, `flatpak-ci`, `rust-ci-arm64cross`). Until they are rebuilt, the workflow-level bun pins sit behind `command -v bun ||` and short-circuit against the image's baked bun, so published packages still vendor installer-sourced bytes. The flatpak script change is not under `ci/` and takes effect immediately. ### Deliberately left, with reasons - **rustup's installer stays piped** (4 images) — `apple.yml` already made that call for the identical pattern; overriding it here would be inconsistent, and pinning `rustup-init` is one decision that should cover every site at once. - **`scripts/steamdeck/install.sh`** — an end-user build-from-source script holding no signing keys. It installs rustup the same way one line above, so pinning only bun there buys nothing. - **82 tag-pinned `actions/checkout@v4` / `cache@v4` across 12 workflows** — pinning the 19 in these files would leave a half-pinned tree where a reader cannot tell a deliberate omission from a miss, and `@v4` is several majors stale (checkout v7, cache v6), so a real sweep needs a version decision rather than a SHA lookup. Worth its own mechanical PR. - **FFmpeg / nv-codec-headers `git clone --branch <tag>`** in `rust-ci-noble` — tags are mutable and those `.so`s ship in the Ubuntu `.deb`, but the remedy is a pinned-commit assertion, not the checksum idiom, and it invalidates a ~20 min from-source layer. Flagging for a follow-up. ### Docs `profiles-and-links.md` said Windows has no prompt (now false) and put Windows in the wrong group for first connects — verified in code that Linux, Android **and** Windows pre-fill the trust prompt, while only the Apple apps show a notice. Also corrected an Apple comment claiming it seeds the add sheet with the address and fingerprint when it only sets a notice string, and documented `DEPLOY_KNOWN_HOSTS` in `packaging/flatpak/README.md`.

enricobuehler added 1 commit 2026-08-26 17:35:44 +00:00

docs(nix): key the deploy host key port-agnostically, and say which address to scan ... 987ecabfde

Two things that cost real time setting DEPLOY_KNOWN_HOSTS for the first
time. ssh looks a host key up by the exact string it dialled, so a plain
entry stops matching the moment DEPLOY_PORT is not 22 — and it fails
looking like a host-key error, not a formatting one. The comma form covers
both. And DEPLOY_HOST is unom-1's public Hetzner IP since the 2026-07-12
cutover, so a stale private address in an SSH config is the wrong thing to
scan.

Also say ed25519 only: pinning every type ssh-keyscan prints means the
weakest pinned key decides, since a host offering just RSA would satisfy
the check on the RSA line.

enricobuehler added 1 commit 2026-08-26 18:25:03 +00:00

fix(ci): a git tag is not a pin — assert the FFmpeg commits we actually build ... c62cfd58de

rust-ci-noble builds FFmpeg and nv-codec-headers from `git clone --branch
<tag>`, and the .so files land in the host .deb every Ubuntu user
installs. Tags are mutable: upstream can repoint one and, unlike a branch,
nothing would look different. The comment above it claimed "immutable-tag
clone, so no separate checksum needed", which is the same
comment-asserts-what-the-code-doesn't shape this whole branch has been
unpicking.

Both are now asserted against the commit the tag points at TODAY, so the
built tree is byte-identical and only the mutability is closed. A moved tag
now fails the build loudly, matching the sha256sum -c pins next door.

Both tags are ANNOTATED, so the bare ref is a tag object, not a commit —
the pin is the peeled `refs/tags/X^{}`, which is what `rev-parse HEAD`
returns after a --depth 1 tag clone. Pinning the tag object would have
failed every build.

rustup's installer and `cargo install cargo-ndk` are left: both float, but
crates.io is append-only with a checksummed index and rustup verifies its
own downloads — neither is a fixed name whose bytes can move.

enricobuehler added 1 commit 2026-08-26 18:26:40 +00:00

fix(ci): pin cargo-ndk too — it builds the shipped Android .so files ... 7ebdd7e5d9

Last floating tool in the builder images. Unpinned, two rebuilds months
apart bake different cargo-ndk versions with nothing recording it, and this
is the tool that drives the .so files in the published APK/AAB. crates.io
is append-only with a checksummed index, so the version string is the whole
pin — no checksum needed, unlike the bun/sccache/FFmpeg cases.

4.1.2 is current, so this is a zero-change pin like the others.

enricobuehler marked the pull request as ready for review 2026-08-26 18:56:10 +00:00

enricobuehler merged commit 809f1faa26 into main 2026-08-26 18:56:34 +00:00

enricobuehler referenced this pull request 2026-08-26 19:08:45 +00:00

A tag is a pointer, not a version — pin every checkout/cache to a commit #405

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 FlipperDan HitFrostbite bluelightspecial enricobuehler (Enrico Bühler) kszlim luxus nannku neoney sonar

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: unom/punktfunk#396