diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a4ecaa..9534331 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ All notable changes to Collie are recorded here. The format follows `version` in `herdr-plugin.toml`, `package.json`, and `web/package.json` (enforced by `scripts/check-version.sh`). See [`CLAUDE.md`](./CLAUDE.md) → *Versioning* for the bump policy. +## [0.18.0] - 2026-07-28 + +### Added +- **Approvals are bound server-side to the prompt they were decided against.** `/keys` and `/reply` accept an optional `expected_prompt`; the bridge re-reads the pane immediately before writing and refuses with `409 prompt_changed` if the dialog moved. Shrinks the guard window from human latency to two local RPCs — a mitigation, not a guarantee, until herdr gains a conditional-input primitive (#29) — thanks @Optic00 (6afaf5b) +- **`/auth/` is reserved for a fronting proxy's sign-in page**, and the service worker always passes it to the network. An installed PWA could not reach a proxy page at all before — the precache answered every navigation, reload included — so operators had to squat a page inside `/api/`. The refusal banner now links there (#31) — thanks @Optic00 (1a5972b) + ## [0.17.0] - 2026-07-27 ### Fixed diff --git a/README.md b/README.md index 4669f35..f7ebfd9 100644 --- a/README.md +++ b/README.md @@ -547,6 +547,48 @@ indefinitely — clients keep running old code with no way to notice. If your pr honor origin headers (Caddy and stock Nginx `proxy_cache` do by default; CDNs often need it enabled explicitly). +**Serve your sign-in page under `/auth/`.** Collie reserves that path for you and routes nothing +there. It matters because of how an installed PWA behaves: the service worker answers every +navigation it owns from the precached app shell without touching the network, and there is no +address bar to work around it. So a proxy page served anywhere Collie owns — including `/` — is +invisible to the installed app, and a reload just re-renders the refused UI. `/auth/` (and anything +beneath it) is the one path the service worker always passes through, so it is the only address that +reaches you. When the bridge answers there itself, nothing claimed the path — that placeholder is +your signal that the proxy rule is missing. + +```caddyfile +collie.example.com { + handle /auth/* { + # your sign-in / device-enrolment flow, exempt from the auth check that guards the rest + reverse_proxy 127.0.0.1:9091 + } + handle { + forward_auth 127.0.0.1:9091 { ... } + reverse_proxy 127.0.0.1:8787 { ... } + } +} +``` + +Collie's refusal banner links to `/auth/` when the bridge or your proxy answers 401/403, so a +signed-out phone has a tappable way back in. A `?rd=`/`?next=` return-to parameter on the redirect is +fine — the passthrough matches the query string too. If your flow lives at a path you can't move, +redirect `/auth/` to it; the redirect is followed on the network side, where the service worker isn't +looking. Cloudflare Access is the exception that can't be redirected, since it owns `/cdn-cgi/access/` +outright — that prefix is reserved as well, so its flow works untouched. + +> ⚠️ **Let the static bundle through even when the session has lapsed** — everything except `/api/` +> and page navigations. It is public client code with no secrets in it, and it is the only way an +> installed app can receive an update. If your proxy refuses `/sw.js` to a signed-out client, that +> client's service worker can never be replaced: `registration.update()` fails outright, so a device +> that lapsed while running an old build stays on that build forever. Measured, not theorised — with +> the bundle refused, `update()` throws; with it allowed, the worker updates cleanly while `/api/` is +> still answering 401. +> +> This bites hardest on the very fix described above: a device that was already locked out **before** +> upgrading to 0.18.0 cannot pick up the new service worker, and its `/auth/` link won't exist. Those +> devices need their site data cleared once (browser settings → the site → clear data), then a fresh +> load. New installs and any device that updated while signed in are unaffected. + ### Variant D — off-host identity proxy over the tailnet Choose this when you already run a **central ingress node** for your tailnet — one forward-auth/SSO diff --git a/bridge/auth-path-contract.test.ts b/bridge/auth-path-contract.test.ts new file mode 100644 index 0000000..b104835 --- /dev/null +++ b/bridge/auth-path-contract.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, test } from "bun:test"; + +import { isReservedAuthPath } from "./server.ts"; +import { isNetworkOnlyNavigation } from "../web/src/lib/sw-routes.ts"; + +// The `/auth/` reservation is implemented twice — once in the service worker's navigation denylist +// (web/src/lib/sw-routes.ts) and once in the bridge's request dispatch (isReservedAuthPath). Each +// side has its own tests, so either could be changed alone and stay green while the two drift, and +// drift is silent in the way that matters: the SW hands the navigation to the network, the bridge +// doesn't recognise it, the SPA fallback answers with the app shell, and the operator is back to +// staring at the UI they were trying to escape. That is issue #31 all over again. +// +// So run one shared list through BOTH and require them to agree. This is the only test that fails +// when someone edits one side and not the other. It imports across the bridge/web boundary on +// purpose: sw-routes.ts is pure (no DOM, no workbox), which is why it was split out at all. +// +// The inputs carry query strings because workbox matches the denylist against `pathname + search`; +// the bridge matches a parsed `pathname`, so it takes the same cases with the query stripped. +const CASES: { url: string; reserved: boolean }[] = [ + { url: "/auth", reserved: true }, + { url: "/auth/", reserved: true }, + { url: "/auth/sign-in", reserved: true }, + { url: "/auth/oidc/callback", reserved: true }, + { url: "/auth?rd=%2F", reserved: true }, + { url: "/auth/?next=/settings", reserved: true }, + { url: "/", reserved: false }, + { url: "/settings", reserved: false }, + { url: "/pane/w1:p1", reserved: false }, + { url: "/pane/w1:p1?s=demo", reserved: false }, + { url: "/space/w1", reserved: false }, + { url: "/authors", reserved: false }, + { url: "/authors?x=1", reserved: false }, +]; + +describe("the /auth reservation agrees across the service worker and the bridge", () => { + for (const { url, reserved } of CASES) { + test(`${url} → ${reserved ? "reserved" : "Collie's"}`, () => { + const pathname = url.split("?")[0]!; + expect(isReservedAuthPath(pathname)).toBe(reserved); + // The API is network-only too, but it is not part of the reservation, so only /auth cases + // are expected to agree with the bridge — every case here is a navigation, not an API call. + expect(isNetworkOnlyNavigation(url)).toBe(reserved); + }); + } +}); diff --git a/bridge/server.test.ts b/bridge/server.test.ts index fbf1e06..824a7ce 100644 --- a/bridge/server.test.ts +++ b/bridge/server.test.ts @@ -8,6 +8,7 @@ import { guard, historyParams, isHostAllowed, + isReservedAuthPath, keysPane, normalizeTabLabel, paneReadResponse, @@ -845,3 +846,21 @@ describe("cacheControlFor", () => { } }); }); + +// The other end of web/src/lib/sw-routes.ts. The service worker hands `/auth/` to the network; if +// the bridge doesn't recognise the same set, the SPA fallback answers with the app shell and the +// operator gets the very UI they were trying to escape. These two must agree exactly. +describe("isReservedAuthPath — the namespace a fronting proxy owns", () => { + test("claims /auth with or without a trailing slash, and everything beneath it", () => { + expect(isReservedAuthPath("/auth")).toBe(true); + expect(isReservedAuthPath("/auth/")).toBe(true); + expect(isReservedAuthPath("/auth/sign-in")).toBe(true); + expect(isReservedAuthPath("/auth/oidc/callback")).toBe(true); + }); + + test("leaves Collie's own routes alone, including a mere prefix match", () => { + for (const path of ["/", "/settings", "/pane/w1:p1", "/authors", "/api/snapshot"]) { + expect(isReservedAuthPath(path)).toBe(false); + } + }); +}); diff --git a/bridge/server.ts b/bridge/server.ts index 50ec01e..11b8a7d 100644 --- a/bridge/server.ts +++ b/bridge/server.ts @@ -310,6 +310,14 @@ export function startServer(opts: { return json(updateMonitor.status(), req.headers.get("accept-encoding")); } + // ── Reserved for a fronting proxy's sign-in page ───────────────────── + // `/auth/` is the one path the service worker always passes to the network (web/src/lib/ + // sw-routes.ts), so it is the only address an installed PWA can reach when a proxy in front of + // the bridge refuses a stale session. Collie never routes it. If a request gets this far, no + // proxy claimed it — say so, instead of letting the SPA fallback answer with the app shell and + // leave the operator staring at the UI they were trying to escape. + if (isReservedAuthPath(pathname)) return reservedAuthPlaceholder(); + // ── Static PWA (with SPA fallback) ─────────────────────────────────── return serveStatic(pathname); }, @@ -1260,6 +1268,55 @@ export function resolveStaticPath( return { rel, full }; } +/** + * The namespace reserved for the operator's front door. Matches `/auth` with or without a trailing + * slash and anything beneath it — a proxy may serve one page or a whole flow. Kept in lockstep with + * the service worker's navigation denylist (`web/src/lib/sw-routes.ts`); if these two disagree, an + * installed PWA either can't reach the proxy or can't reach Collie. Pure + exported for tests. + */ +export function isReservedAuthPath(pathname: string): boolean { + return pathname === "/auth" || pathname.startsWith("/auth/"); +} + +/** + * What `/auth/` says when nothing is in front of the bridge. Deliberately a 404: the path is + * reserved, not implemented — Collie has no sign-in of its own and must not imply otherwise. Plain + * HTML with no inline style or script (the strict CSP forbids both) and a link home, because in an + * installed PWA this page may be the only thing on screen and there is no address bar to leave it. + * Unauthenticated by design: it sits outside every gate, since the reason to be here is that a gate + * refused you. + */ +function reservedAuthPlaceholder(): Response { + const body = ` + + + + +Nothing configured here — Collie + + +

Nothing is configured at this address

+

Collie reserves /auth/ for a reverse proxy sitting in front of it, so that an +installed app has somewhere to reach a sign-in or device-enrolment page. Collie itself serves +nothing here and has no sign-in of its own.

+

If you are the operator: point this path at your proxy's sign-in flow. See Serving Collie +behind your own reverse proxy in the README.

+

Back to Collie

+ + +`; + return secure( + new Response(body, { + status: 404, + headers: { + "content-type": "text/html; charset=utf-8", + "content-security-policy": CSP, + "cache-control": "no-store", + }, + }), + ); +} + async function serveStatic(pathname: string): Promise { const resolved = resolveStaticPath(pathname); if (!resolved) return text("forbidden", 403); diff --git a/herdr-plugin.toml b/herdr-plugin.toml index f3fccdc..f145bd4 100644 --- a/herdr-plugin.toml +++ b/herdr-plugin.toml @@ -1,6 +1,6 @@ id = "herdr.collie" name = "Collie" -version = "0.17.0" +version = "0.18.0" min_herdr_version = "0.7.0" description = "Mobile web UI to monitor and reply to your agent herd, served over Tailscale" platforms = ["linux", "macos"] diff --git a/package.json b/package.json index 7b5ef94..46645f6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "collie", - "version": "0.17.0", + "version": "0.18.0", "private": true, "license": "MIT", "description": "Collie — a mobile web UI to monitor and reply to your Herdr agent herd over Tailscale", diff --git a/web/package.json b/web/package.json index 86104a9..70276e1 100644 --- a/web/package.json +++ b/web/package.json @@ -1,6 +1,6 @@ { "name": "collie-web", - "version": "0.17.0", + "version": "0.18.0", "private": true, "license": "MIT", "type": "module", diff --git a/web/src/components/connection-banner.test.tsx b/web/src/components/connection-banner.test.tsx index 13d9c35..0505620 100644 --- a/web/src/components/connection-banner.test.tsx +++ b/web/src/components/connection-banner.test.tsx @@ -80,6 +80,18 @@ describe("ConnectionBanner — the single connection surface", () => { expect(cfg.calls).toBe(0); }); + // The escape hatch for an installed PWA, which has no address bar: a real link to the one path the + // service worker always passes to the network. It must stay an with a real href — a button + // with an onClick would be a same-document action the SW never sees as a navigation, which is the + // whole bug (#31). If this assertion is ever "fixed" by swapping in a Button, the PWA is bricked + // again behind a refused session and nothing else will fail. + it("offers a real link to the reserved proxy path, not a click handler", () => { + renderBanner({ authError: true }); + + const signIn = screen.getByRole("link", { name: "Sign in" }); + expect(signIn).toHaveAttribute("href", "/auth/"); + }); + it("renders nothing while healthy — no bar at all", () => { renderBanner({ bridge: "connected" }); expect(screen.queryByRole("status")).toBeNull(); diff --git a/web/src/components/connection-banner.tsx b/web/src/components/connection-banner.tsx index ff17924..cc8b4b3 100644 --- a/web/src/components/connection-banner.tsx +++ b/web/src/components/connection-banner.tsx @@ -1,9 +1,19 @@ import { useCallback, useEffect, useRef, useState } from "react"; import { useRevalidator } from "react-router"; -import { CheckCircle2, Loader2, Plug, RefreshCw, RotateCw, TriangleAlert, WifiOff } from "lucide-react"; +import { + CheckCircle2, + Loader2, + LogIn, + Plug, + RefreshCw, + RotateCw, + TriangleAlert, + WifiOff, +} from "lucide-react"; -import { Button } from "@/components/ui/button"; +import { Button, buttonVariants } from "@/components/ui/button"; import { cn } from "@/lib/utils"; +import { PROXY_AUTH_PATH } from "@/lib/sw-routes"; import { useConnectionLost, useConnectionTrouble } from "@/hooks/use-connection-lost"; import { useLoadingStalled } from "@/hooks/use-loading-stalled"; import { useOnline } from "@/hooks/use-online"; @@ -52,8 +62,15 @@ export function ConnectionBanner({ bridge, error, authError }: ConnectionBannerP // the cause. The flag covers 401 and 403 alike, and a 403 can equally mean "this device is not // allowlisted", "host not allowed" or "cross-origin rejected", so naming any one of them would be // wrong more often than right. What the operator needs here is the one fact the old behaviour hid: -// this is not the network. Reload is the useful action either way, because it is what lets a -// fronting proxy serve its sign-in page. +// this is not the network. +// +// Reload alone is NOT enough to reach a fronting proxy, which is what this banner used to claim. In +// an installed PWA the service worker answers every navigation it owns — a reload included — from +// the precached app shell, so a reload re-renders the same refused UI and never touches the proxy. +// "Sign in" is the escape: a real navigation to the one path the SW always passes to the network +// (lib/sw-routes). An , not a button, so it is an ordinary navigation the SW sees as such — and +// so it still works if React is wedged. Reload stays alongside it, since a merely stale session on +// an already-signed-in device recovers without leaving the app. function AuthErrorBanner() { return (
@@ -70,13 +87,24 @@ function AuthErrorBanner() { Access refused. This is not a connection problem. + + + Sign in +
diff --git a/web/src/components/ui/button.tsx b/web/src/components/ui/button.tsx index bc226f9..c1d891c 100644 --- a/web/src/components/ui/button.tsx +++ b/web/src/components/ui/button.tsx @@ -42,4 +42,6 @@ function Button({ ); } -export { Button }; +// `buttonVariants` is exported so a real can wear the button's clothes where a navigation, not a +// click handler, is the point — see the access-refused banner's "Sign in". +export { Button, buttonVariants }; diff --git a/web/src/lib/sw-routes.test.ts b/web/src/lib/sw-routes.test.ts new file mode 100644 index 0000000..480dff1 --- /dev/null +++ b/web/src/lib/sw-routes.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, it } from "vitest"; + +import { + NAVIGATION_NETWORK_ONLY, + PROXY_AUTH_PATH, + isNetworkOnlyNavigation, +} from "./sw-routes"; + +// These rules are the difference between an installed PWA that can reach its front door and one that +// is bricked behind a refused session, and the failure is silent in both directions: too narrow and +// the sign-in page is invisible, too wide and Collie's own deep links stop resolving offline. The +// regexes are what the service worker actually installs, so pin the contract here. +describe("service-worker navigation passthrough", () => { + it("never answers the API from the precache", () => { + expect(isNetworkOnlyNavigation("/api/snapshot")).toBe(true); + expect(isNetworkOnlyNavigation("/api/pane/w1:p1/keys")).toBe(true); + }); + + it("passes the reserved proxy namespace to the network, with or without the slash", () => { + expect(isNetworkOnlyNavigation("/auth")).toBe(true); + expect(isNetworkOnlyNavigation("/auth/")).toBe(true); + expect(isNetworkOnlyNavigation("/auth/sign-in")).toBe(true); + expect(isNetworkOnlyNavigation("/auth/oidc/callback")).toBe(true); + }); + + // Workbox tests the denylist against `url.pathname + url.search` (verified in the vendored + // workbox-routing/NavigationRoute.js `_match`), so these inputs carry their query string — a rule + // anchored on a trailing slash passes every pathname-only test above and still bricks the app the + // moment a proxy bounces you to `/auth?rd=…`, which is what Authelia and oauth2-proxy do. + it("still passes through when the proxy appends a return-to query string", () => { + expect(isNetworkOnlyNavigation("/auth?rd=%2F")).toBe(true); + expect(isNetworkOnlyNavigation("/auth?rd=%2Fpane%2Fw1%3Ap1")).toBe(true); + expect(isNetworkOnlyNavigation("/auth/?next=/settings")).toBe(true); + expect(isNetworkOnlyNavigation("/api/snapshot?s=demo")).toBe(true); + }); + + // Cloudflare Access can't move off /cdn-cgi/access/, so the reservation has to come to it. + it("passes Cloudflare Access's non-relocatable prefix to the network", () => { + expect(isNetworkOnlyNavigation("/cdn-cgi/access/login")).toBe(true); + expect(isNetworkOnlyNavigation("/cdn-cgi/access/callback?code=abc")).toBe(true); + }); + + it("does not leak a Collie route that merely carries a query string", () => { + expect(isNetworkOnlyNavigation("/authors?x=1")).toBe(false); + expect(isNetworkOnlyNavigation("/?s=collie-demo")).toBe(false); + expect(isNetworkOnlyNavigation("/pane/w1:p1?s=demo")).toBe(false); + }); + + it("still owns every Collie route, so deep links keep resolving offline", () => { + for (const path of [ + "/", + "/settings", + "/pane/w1:p1", + "/pane/w1:p1/history", + "/space/w1", + ]) { + expect(isNetworkOnlyNavigation(path)).toBe(false); + } + }); + + // A route merely STARTING with the reserved word is Collie's, not the proxy's: `/authors` must not + // be handed to the network just because it shares five letters with `/auth`. + it("does not leak a route that only shares the prefix", () => { + expect(isNetworkOnlyNavigation("/authors")).toBe(false); + expect(isNetworkOnlyNavigation("/apidocs")).toBe(false); + }); + + it("exports the reserved path the UI links to, ending in a slash", () => { + expect(PROXY_AUTH_PATH).toBe("/auth/"); + expect(isNetworkOnlyNavigation(PROXY_AUTH_PATH)).toBe(true); + }); + + it("keeps the denylist the SW installs to exactly these two rules", () => { + expect(NAVIGATION_NETWORK_ONLY.map(String)).toEqual([ + String(/^\/api\//), + String(/^\/auth(?:[/?]|$)/), + String(/^\/cdn-cgi\//), + ]); + }); +}); diff --git a/web/src/lib/sw-routes.ts b/web/src/lib/sw-routes.ts new file mode 100644 index 0000000..cd4982c --- /dev/null +++ b/web/src/lib/sw-routes.ts @@ -0,0 +1,53 @@ +/** + * Which navigations the service worker must NEVER answer from the precache. + * + * The SW's NavigationRoute serves the precached app shell for every navigation it handles, without + * touching the network. That is what makes deep links work offline — and it is also why an installed + * PWA, which has no address bar to fall back on, cannot reach anything a fronting reverse proxy + * serves at a path Collie doesn't own. A proxy that authenticates devices ahead of the bridge + * (README Variant C/E) has a sign-in or enrolment page, and before this list existed there was no + * legitimate place to put it: the `/api/` denylist was the only crack in the precache, so operators + * squatted a page inside the namespace the API owns. + * + * `/auth/` is therefore RESERVED. Collie routes nothing there, precaches nothing there, and will + * never claim it for a UI route — it exists so the operator's front door has an address. The bridge + * answers it with a placeholder explaining that nothing is configured, so an operator without a + * proxy finds out immediately instead of silently getting the app shell. + * + * Kept in its own module, free of workbox imports, so the contract is unit-testable and so the app + * and the service worker can't drift on what the reserved path is. + */ + +/** The reserved prefix a fronting proxy owns. Trailing slash: it's a namespace, not one page. */ +export const PROXY_AUTH_PATH = "/auth/"; + +/** + * Navigation paths the SW passes straight to the network. `/api/` was always here (the API must + * never be answered from a cache); `/auth` joins it, with or without the trailing slash, so a proxy + * can serve its page at either. + * + * These are matched against `pathname + search`, NOT pathname alone — verified in the vendored + * workbox-routing/NavigationRoute.js (`_match` builds `url.pathname + url.search` and tests the + * denylist against that). Hence `[/?]` rather than a bare `/`: a proxy that bounces you to + * `/auth?rd=%2Fpane%2Fw1` — the shape Authelia and oauth2-proxy both use — produces the string + * "/auth?rd=%2Fpane%2Fw1", and a rule anchored on a trailing slash would miss it and hand the + * operator the precached app shell. That is this whole bug, in its most likely real-world form. + */ +export const NAVIGATION_NETWORK_ONLY = [ + /^\/api\//, + /^\/auth(?:[/?]|$)/, + // Cloudflare Access serves its login and callback under `/cdn-cgi/access/` and the path is NOT + // relocatable, so pointing the operator at `/auth/` cannot help them — the flow would break on a + // callback the precache swallowed. `/cdn-cgi/` is Cloudflare-reserved; Collie will never route it. + // Proxies whose prefix IS movable (oauth2-proxy's `--proxy-prefix`, Authelia) are documented in + // the README instead of listed here — this list stays for paths nobody can move. + /^\/cdn-cgi\//, +] as const; + +/** + * True when the SW must not answer this navigation from the precache. Takes `pathname + search`, + * matching what workbox feeds the denylist — pass the query string if there is one. + */ +export function isNetworkOnlyNavigation(pathnameAndSearch: string): boolean { + return NAVIGATION_NETWORK_ONLY.some((re) => re.test(pathnameAndSearch)); +} diff --git a/web/src/sw.ts b/web/src/sw.ts index 77085ef..498caeb 100644 --- a/web/src/sw.ts +++ b/web/src/sw.ts @@ -4,6 +4,7 @@ import { NavigationRoute, registerRoute } from "workbox-routing"; import { clientsClaim } from "workbox-core"; import { decidePush, type PushPayload } from "./lib/push-decision"; +import { NAVIGATION_NETWORK_ONLY } from "./lib/sw-routes"; // Custom service worker (vite-plugin-pwa `injectManifest`). It does everything the old generated // Workbox SW did — precache the app shell + SPA-fallback navigations — PLUS the two handlers a @@ -21,8 +22,16 @@ declare const self: ServiceWorkerGlobalScope & { // ── App-shell caching (parity with the previous generateSW config) ────────────────────────────── precacheAndRoute(self.__WB_MANIFEST); -// SPA fallback so deep links (/pane/:id) resolve offline too; never intercept the API. -registerRoute(new NavigationRoute(createHandlerBoundToURL("/index.html"), { denylist: [/^\/api\//] })); +// SPA fallback so deep links (/pane/:id) resolve offline too. The denylist is the set of paths this +// SW must never answer from the precache — the API, and the `/auth/` namespace reserved for a +// fronting proxy's sign-in page. Without that second entry an installed PWA, which has no address +// bar, has no reachable path to the proxy at all: every navigation, including a reload, is answered +// by the cached app shell. See lib/sw-routes for the contract. +registerRoute( + new NavigationRoute(createHandlerBoundToURL("/index.html"), { + denylist: [...NAVIGATION_NETWORK_ONLY], + }), +); // `registerType: "autoUpdate"` means a fresh build should take over without a user gesture. With // injectManifest we own that lifecycle: skip the waiting phase on install, claim open clients on