fix(web): four "fixes" from this branch that did not actually fix anything
A verification pass re-read every finding from the original sweep against the code on this branch rather than against the commit messages. It found that four of them were still broken, two because the edit I made was inert. Commit messages claim; code decides. - **The Storybook typecheck was never on.** `tsconfig.json` listed `.storybook` as a bare directory name, and tsc silently skips dot-prefixed directories in that form — so the entry typechecked nothing at all. Proved it by planting `export const __probe: string = 1` in `.storybook/preview.tsx` and watching `bun run lint` pass. `.storybook/**/*` is what actually pulls it in; the same probe now fails as it should. - **The Moonlight stale-PIN reset was a no-op.** `submit.reset()` sat at the top of `onSubmit`, immediately before `submit.mutate(...)` — which moves the status to pending in the same update, so it cleared a flag that was already changing. The green "PIN sent" note therefore still greeted the next pairing attempt over an empty PIN box. It now resets on the transition that actually matters: `pin_pending` going false → true. - **The session⇄game controls had the enforcement flag inverted**, and I never touched it. `enforced.length === 0 || …` reads an EMPTY list as "this build enforces everything", when the contract says the opposite in as many words: "Empty on a platform with no launch path (macOS), so the console can say so instead of offering a switch that does nothing". On exactly the platform the flag exists for, every control stayed live and reported success for an axis the host would never act on. Absent still means "assume it acts" — that is the compatible reading for an older host, and a different case from present-empty. - **Logout stopped revoking after a restart.** The epoch was a module-level counter starting at 1, so it revoked within one process run and then reset — and since the seal key derives from the stable mgmt token, a cookie captured before a restart unsealed fine and was accepted again for the rest of its 7-day TTL. One service restart undid the whole fix. It persists next to the host's config now. Verified: log out, restart the console, the captured cookie still 401s, a fresh login still works. Two more the pass rated as partial, both worth closing: - The plugin-UI response filter was a denylist of four header names, so `Clear-Site-Data` sailed through — a plugin error page could wipe `pf_session` and sign the operator out of the console, on our own origin, because the iframe is same-origin by design. It is an allowlist now; a plugin-supplied CSP, `X-Frame-Options` or CORS header no longer speaks for us either. - A half-configured TLS setup now refuses to start instead of logging a warning and serving anyway. Neither shape can work — one path missing puts the login password on the LAN in the clear, and PUNKTFUNK_UI_SECURE without TLS marks the cookie Secure so the browser drops it and login can never stick. Exiting with a reason beats a console that looks fine and is not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -48,21 +48,32 @@ const tls =
|
||||
? { cert: Bun.file(certPath), key: Bun.file(keyPath) }
|
||||
: undefined;
|
||||
|
||||
// Half-configured TLS is the dangerous shape: one path set and the other missing silently drops to
|
||||
// plain HTTP, and if PUNKTFUNK_UI_SECURE is also set the session cookie is marked Secure — which a
|
||||
// browser then refuses to store over http://, so login appears to succeed and every next request is
|
||||
// unauthenticated. Both failure modes are silent, so say something.
|
||||
// Half-configured TLS is not a warning, it is a refusal.
|
||||
//
|
||||
// Two silent failures hide here, and both end with the operator staring at a console that looks
|
||||
// fine. One path set and the other missing drops to plain HTTP — the login password then crosses
|
||||
// the LAN in the clear on a server the operator believes is TLS. And PUNKTFUNK_UI_SECURE without
|
||||
// TLS marks the session cookie Secure, which a browser refuses to store over http://, so login
|
||||
// "succeeds" and every request after it is unauthenticated, forever.
|
||||
//
|
||||
// Neither state can serve a working console, so exiting is strictly better than serving a broken
|
||||
// one: a supervisor logs the reason and the operator sees a stopped service instead of a subtly
|
||||
// wrong one.
|
||||
const secureFlag = /^(1|true)$/i.test(process.env.PUNKTFUNK_UI_SECURE ?? "");
|
||||
if (Boolean(certPath) !== Boolean(keyPath)) {
|
||||
console.error(
|
||||
`punktfunk web console: only ${certPath ? "PUNKTFUNK_UI_TLS_CERT" : "PUNKTFUNK_UI_TLS_KEY"} is set — ` +
|
||||
"TLS needs BOTH. Serving plain HTTP.",
|
||||
"TLS needs BOTH. Refusing to start rather than serve the login password in the clear.",
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
if (!tls && /^(1|true)$/i.test(process.env.PUNKTFUNK_UI_SECURE ?? "")) {
|
||||
if (!tls && secureFlag) {
|
||||
console.error(
|
||||
"punktfunk web console: PUNKTFUNK_UI_SECURE is set but TLS is not configured. The session " +
|
||||
"cookie will be marked Secure and dropped by the browser over http:// — login will not stick.",
|
||||
"cookie would be marked Secure and dropped by the browser over http://, so login could " +
|
||||
"never stick. Refusing to start — set PUNKTFUNK_UI_TLS_CERT/_KEY, or unset PUNKTFUNK_UI_SECURE.",
|
||||
);
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
const server = Bun.serve({
|
||||
|
||||
@@ -84,22 +84,41 @@ export default defineEventHandler(async (event) => {
|
||||
const BODY_METHODS = new Set(["POST", "PUT", "PATCH", "DELETE"]);
|
||||
|
||||
/**
|
||||
* Fix up a plugin's response before it goes out on the console's origin.
|
||||
* Rebuild a plugin's response before it goes out on the console's own origin.
|
||||
*
|
||||
* - `content-encoding` / `content-length` / `transfer-encoding`: `fetch` already decoded the body,
|
||||
* but the plugin's original headers survive on the Response. Re-emitting `content-encoding: gzip`
|
||||
* over plaintext makes the browser fail to decode the page, and a stale `content-length` truncates
|
||||
* it. The framing belongs to OUR response, so drop the plugin's and let it be recomputed.
|
||||
* - `set-cookie`: a plugin runs on the console's own origin, so any cookie it sets is scoped to the
|
||||
* console — it could collide with (or shadow) `pf_session`. A plugin UI has no business setting
|
||||
* cookies on this origin; it authenticates with the injected per-boot bearer.
|
||||
* An ALLOWLIST, not a denylist. A plugin UI is proxied same-origin by design, so any header it
|
||||
* returns is asserted for the console itself — and the first version of this dropped four names it
|
||||
* had thought of. `Clear-Site-Data: "*"` from a plugin's error page was not one of them: the
|
||||
* browser would honour it for this origin and wipe `pf_session`, signing the operator out of the
|
||||
* console because a plugin 500'd. Same shape for a plugin-supplied `Content-Security-Policy`,
|
||||
* `X-Frame-Options` or `Access-Control-Allow-Origin` — all of which would speak for us.
|
||||
*
|
||||
* So: name what a plugin page legitimately needs, and drop the rest. Framing headers
|
||||
* (content-encoding/length, transfer-encoding) are deliberately absent — `fetch` already decoded
|
||||
* the body, so re-emitting the plugin's originals made compressed pages fail to decode; ours are
|
||||
* recomputed.
|
||||
*/
|
||||
const PLUGIN_HEADER_ALLOWLIST = new Set([
|
||||
"content-type",
|
||||
"cache-control",
|
||||
"etag",
|
||||
"last-modified",
|
||||
"expires",
|
||||
"vary",
|
||||
"content-language",
|
||||
"content-disposition",
|
||||
"accept-ranges",
|
||||
"content-range",
|
||||
"location", // its own redirects, within its own prefix
|
||||
"link", // preload hints for its own assets
|
||||
"x-forwarded-prefix",
|
||||
]);
|
||||
|
||||
function sanitize(resp: Response): Response {
|
||||
const headers = new Headers(resp.headers);
|
||||
headers.delete("content-encoding");
|
||||
headers.delete("content-length");
|
||||
headers.delete("transfer-encoding");
|
||||
headers.delete("set-cookie");
|
||||
const headers = new Headers();
|
||||
for (const [k, v] of resp.headers) {
|
||||
if (PLUGIN_HEADER_ALLOWLIST.has(k.toLowerCase())) headers.set(k, v);
|
||||
}
|
||||
// 204/304 must not carry a body — passing one through throws in the Response constructor.
|
||||
const bodyless = resp.status === 204 || resp.status === 304;
|
||||
return new Response(bodyless ? null : resp.body, {
|
||||
|
||||
+55
-24
@@ -1,3 +1,55 @@
|
||||
/**
|
||||
* A revocation marker for issued sessions, PERSISTED across restarts.
|
||||
*
|
||||
* The session is stateless: everything lives inside the sealed cookie, so `session.clear()` only
|
||||
* deletes the BROWSER's copy. A cookie captured beforehand stayed valid for its full 7-day TTL —
|
||||
* "log out" did not log anything out.
|
||||
*
|
||||
* The counter has to survive a restart or it does not do its job: an in-memory `let epoch = 1`
|
||||
* revokes within one process run, then resets to 1 the next time the service starts, and a cookie
|
||||
* captured from that first run is accepted again for the rest of its TTL. (The seal key cannot save
|
||||
* us — it is derived from the stable mgmt token, so pre-restart cookies still unseal fine.) So it
|
||||
* lives in a file next to the host's own config.
|
||||
*
|
||||
* Best-effort by design: if the file cannot be read or written the console still works, it just
|
||||
* falls back to in-memory revocation for this process. Refusing to log anyone out because a state
|
||||
* file is unwritable would be the wrong trade for a LAN console.
|
||||
*/
|
||||
const EPOCH_FILE = (): string =>
|
||||
process.env.PUNKTFUNK_UI_EPOCH_FILE ??
|
||||
join(
|
||||
process.env.PUNKTFUNK_CONFIG_DIR ?? join(homedir(), ".config", "punktfunk"),
|
||||
"web-session-epoch",
|
||||
);
|
||||
|
||||
let epochCache: number | null = null;
|
||||
|
||||
/** The epoch a new session is stamped with, and the one the gate requires. */
|
||||
export function sessionEpoch(): number {
|
||||
if (epochCache !== null) return epochCache;
|
||||
try {
|
||||
const raw = readFileSync(EPOCH_FILE(), "utf8").trim();
|
||||
const n = Number.parseInt(raw, 10);
|
||||
epochCache = Number.isFinite(n) && n > 0 ? n : 1;
|
||||
} catch {
|
||||
epochCache = 1; // no file yet — first run
|
||||
}
|
||||
return epochCache;
|
||||
}
|
||||
|
||||
/** Invalidate every session issued so far (what logging out does). */
|
||||
export function revokeAllSessions(): void {
|
||||
const next = sessionEpoch() + 1;
|
||||
epochCache = next;
|
||||
try {
|
||||
mkdirSync(dirname(EPOCH_FILE()), { recursive: true });
|
||||
writeFileSync(EPOCH_FILE(), String(next), { mode: 0o600 });
|
||||
} catch {
|
||||
// Unwritable state dir: the bump still holds for this process, which is the common case
|
||||
// (log out, walk away). It is weaker than persisted, and better than refusing to log out.
|
||||
}
|
||||
}
|
||||
|
||||
// Shared auth helpers for the Nitro server (the deployed Bun server). Single-user,
|
||||
// shared-password gate: the user logs in with PUNKTFUNK_UI_PASSWORD, which sets a SEALED
|
||||
// (h3 useSession — AES-GCM) cookie; every request is gated by server/middleware/auth.ts.
|
||||
@@ -8,6 +60,9 @@ import {
|
||||
createHash,
|
||||
timingSafeEqual as nodeTimingSafeEqual,
|
||||
} from "node:crypto";
|
||||
import { mkdirSync, readFileSync, writeFileSync } from "node:fs";
|
||||
import { homedir } from "node:os";
|
||||
import { dirname, join } from "node:path";
|
||||
import {
|
||||
getRequestHeader,
|
||||
getRequestIP,
|
||||
@@ -201,27 +256,3 @@ export interface SessionData {
|
||||
/** The epoch this session was sealed under — see `sessionEpoch`. */
|
||||
epoch?: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* A revocation counter for issued sessions.
|
||||
*
|
||||
* The session is stateless: everything lives inside the sealed cookie, so `session.clear()` only
|
||||
* deletes the BROWSER's copy. A cookie captured beforehand (a shared machine, a shell history, a
|
||||
* TLS-inspecting proxy) stayed valid for its full 7-day TTL with nothing the operator could do
|
||||
* about it — "log out" did not log anything out.
|
||||
*
|
||||
* Bumping this invalidates every previously issued cookie, because the gate compares the stamped
|
||||
* epoch against the current one. It lives in memory, so a host restart also revokes — acceptable
|
||||
* for a single-user console, and the safe direction to fail.
|
||||
*/
|
||||
let epoch = 1;
|
||||
|
||||
/** The epoch a new session is stamped with, and the one the gate requires. */
|
||||
export function sessionEpoch(): number {
|
||||
return epoch;
|
||||
}
|
||||
|
||||
/** Invalidate every session issued so far (the "sign out everywhere" lever). */
|
||||
export function revokeAllSessions(): void {
|
||||
epoch += 1;
|
||||
}
|
||||
|
||||
@@ -32,11 +32,18 @@ export const SessionGameCard: FC = () => {
|
||||
const q = useGetSessionSettings();
|
||||
const save = useSetSessionSettings();
|
||||
const server = q.data?.settings;
|
||||
// Which axes this build acts on. Empty on a platform with no launch path (macOS), where the
|
||||
// controls are shown disabled rather than hidden — "does nothing here" is information.
|
||||
const enforced = q.data?.enforced ?? [];
|
||||
const acts = (field: string) =>
|
||||
enforced.length === 0 || enforced.includes(field);
|
||||
// Which axes this build acts on. An EMPTY list means the build enforces nothing — the contract
|
||||
// says so outright ("Empty on a platform with no launch path (macOS), so the console can say so
|
||||
// instead of offering a switch that does nothing"), and this card's own comment promises the
|
||||
// controls are "shown disabled rather than hidden".
|
||||
//
|
||||
// The old `enforced.length === 0 || …` read empty as "enforces EVERYTHING", so on exactly the
|
||||
// platform the flag exists for, every control stayed live: clicking one PUT the setting and
|
||||
// toasted success for an axis the host would never act on. Absent (an older host that never
|
||||
// sent the field) still means "assume it acts" — that is the compatible reading, and it is a
|
||||
// different case from present-and-empty.
|
||||
const enforced = q.data?.enforced;
|
||||
const acts = (field: string) => !enforced || enforced.includes(field);
|
||||
|
||||
// The grace field is free text while being typed, so it gets a local buffer; the other two axes
|
||||
// are discrete and go straight to the host.
|
||||
@@ -162,7 +169,10 @@ export const SessionGameCard: FC = () => {
|
||||
</Field>
|
||||
)}
|
||||
|
||||
{enforced.length === 0 && (
|
||||
{/* Present-and-empty is the "this build acts on none of it" signal; ABSENT
|
||||
is an older host that never sent the field, where claiming inertness
|
||||
would be a guess. Same distinction `acts()` makes above. */}
|
||||
{enforced?.length === 0 && (
|
||||
<Badge variant="outline">{m.session_game_inert()}</Badge>
|
||||
)}
|
||||
{error && <p className="text-sm text-destructive">{error}</p>}
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { useQueryClient } from "@tanstack/react-query";
|
||||
import { Info, KeyRound } from "lucide-react";
|
||||
import { type FC, useState } from "react";
|
||||
import { type FC, useEffect, useRef, useState } from "react";
|
||||
import { getListPairedClientsQueryKey } from "@/api/gen/clients/clients";
|
||||
import type { PairingStatus } from "@/api/gen/model/pairingStatus";
|
||||
import {
|
||||
@@ -23,10 +23,24 @@ export const MoonlightPairingSection: FC = () => {
|
||||
const pairing = useGetPairingStatus({ query: { refetchInterval: 2_000 } });
|
||||
const submit = useSubmitPairingPin();
|
||||
|
||||
// Clear the previous attempt's outcome when a NEW pairing knock arrives.
|
||||
//
|
||||
// The mutation's success flag outlives the form — the section never unmounts, only the inner
|
||||
// <form> is conditional — so the green "PIN sent" note was still on screen above an empty PIN
|
||||
// box the next time Moonlight asked. Resetting inside `onSubmit` (the first attempt at this)
|
||||
// does nothing: `mutate` moves the status to pending in the same update, so `isSuccess` was
|
||||
// already about to go false. The transition that matters is `pin_pending` going false → true.
|
||||
const pending = pairing.data?.pin_pending ?? false;
|
||||
const wasPending = useRef(pending);
|
||||
useEffect(() => {
|
||||
if (pending && !wasPending.current) {
|
||||
submit.reset();
|
||||
setPin("");
|
||||
}
|
||||
wasPending.current = pending;
|
||||
}, [pending, submit.reset]);
|
||||
|
||||
const onSubmit = () => {
|
||||
// The mutation's success/error flags outlive the form: without this, starting a SECOND
|
||||
// pairing attempt showed the previous one's "PIN sent" confirmation before a digit was typed.
|
||||
submit.reset();
|
||||
submit.mutate(
|
||||
{ data: { pin } },
|
||||
{
|
||||
|
||||
+3
-1
@@ -23,7 +23,9 @@
|
||||
"include": [
|
||||
"src",
|
||||
"server",
|
||||
".storybook",
|
||||
// A BARE directory name that starts with a dot is silently skipped by tsc, so the
|
||||
// previous `.storybook` entry typechecked nothing at all. The glob is what pulls it in.
|
||||
".storybook/**/*",
|
||||
"vite.config.ts",
|
||||
"vite.storybook.config.ts",
|
||||
"orval.config.ts"
|
||||
|
||||
Reference in New Issue
Block a user