diff --git a/Cargo.lock b/Cargo.lock index 8aac56b..eed880f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1249,6 +1249,7 @@ dependencies = [ "axum", "base64", "clap", + "getrandom 0.3.4", "hex", "hmac", "reqwest", diff --git a/Cargo.toml b/Cargo.toml index b0405aa..388e50f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,11 +22,16 @@ serde_json = "1" # RUSTSEC-2024-0320 "unmaintained" advisory if switchboard is open-sourced. serde_yml = "0.0.12" -# Crypto (webhook signature verification) +# Crypto (webhook signature verification, HS256 share-token minting, +# per-site KV password derivation) hmac = "0.12" sha2 = "0.10" hex = "0.4" +# CSPRNG for the share token's `jti` (a fresh, unguessable revocation id per +# link). Already in the tree via rustls/ring — no new download. +getrandom = "0.3" + # Logging tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["env-filter"] } diff --git a/README.md b/README.md index a9ea69d..3cf676c 100644 --- a/README.md +++ b/README.md @@ -177,21 +177,37 @@ from the site key by exact path — never a glob wider than the one site: In cluster mode every node's daemon runs the same teardown against its own disk, which is the complete story — each node reaps its own replicas. -### Preview privacy is a known gap (not yet closed) - -A deployed preview currently serves to anyone who resolves its hostname — there -is **no access gate on the preview URL**. Now that a private repository can be -checked out (above), that means a private repo's preview is reachable by anyone -who knows the hostname. This is stated rather than implied: it is a real -exposure, and the fix is designed but not yet shipped. - -The gate belongs in ePHPm, not switchboard — it has to cover the static-file -path as well as PHP and fail closed, which is the request-phase middleware layer -ePHPm already has, and switchboard's only per-site channel (the two-key override -file) is deliberately closed. The full design, threat model, and the companion -ePHPm issue it depends on (ephpm/ephpm#487) are in [`docs/preview-access-gate.md`](docs/preview-access-gate.md). -Until that lands, treat previews as world-readable and do not preview a -repository whose mere contents are sensitive. +### Preview privacy: the access gate + +A **private** repo's preview must not be world-readable. It isn't: switchboard +gates it (ephpm#487/#491). The enforcement lives in ePHPm — a per-site +`preview-gate` middleware that redirects unauthenticated visitors to a GitHub +login, runs on the static **and** PHP paths, and fails closed — because that is +the request-phase layer that covers static files too, which an +`auto_prepend_file` check never could. switchboard is the control plane: + +- **Gating policy.** A private repo (`repository.private`, defaulting to private + when the job payload omits it — fail closed) is **always** gated. A public repo + is ungated by default, gated only with `--gate-public-previews`. +- **Activation.** For a gated preview switchboard writes a `[preview_auth]` + section into the same per-site override it already writes `document_root` / + `auto_prepend_file` into — carrying a `session_secret` **reference** (never the + key) and the issuer's `login_url`. +- **Share links.** With `--share-link`, switchboard mints a short-lived, + per-preview, revocable `via:"share"` HS256 capability token for people without + repo access and posts `…/?ephpm_share=` in the PR comment, with the + bearer-capability warning stated plainly. +- **Revocation.** On teardown switchboard bumps the per-site epoch + (`preview:share:epoch`) in the preview's KV keyspace, killing every outstanding + share link (best-effort; needs `--kv-secret-file`). +- **Fail closed.** A gated preview whose session secret cannot be resolved (or + that has nowhere to write the gate, i.e. no `--site-overrides-dir`) **fails the + deploy** — never an ungated open preview. + +The full design, threat model, operator config, and rollout ordering are in +[`docs/preview-access-gate.md`](docs/preview-access-gate.md). The one-time fleet +setup (the GitHub OAuth App and `EPHPM_PREVIEW_SESSION_SECRET`) is ePHPm node +config, described there. ### Teardown is complete or it fails @@ -323,6 +339,26 @@ a fork builds but every `${secret.NAME}` expands to the empty string (with a name-only warning). Fork **teardowns** are always processed; refusing them would strand previews on disk. +### Preview access gate (ephpm#487/#491) + +See [Preview privacy: the access gate](#preview-privacy-the-access-gate) for what +these do; the full design is in +[`docs/preview-access-gate.md`](docs/preview-access-gate.md). + +| Flag | Env | Default | Meaning | +|---|---|---|---| +| `--gate-public-previews` | `SWITCHBOARD_GATE_PUBLIC_PREVIEWS` | `false` | Gate **public** repos' previews too. Private repos are *always* gated regardless. | +| `--preview-session-secret-ref` | `SWITCHBOARD_PREVIEW_SESSION_SECRET_REF` | `env:EPHPM_PREVIEW_SESSION_SECRET` | The `session_secret` **reference** (`env:NAME` / `file:/abs` / literal) written into a gated preview's `[preview_auth]` and resolved to mint share tokens. Must be the **same** reference the `github-auth` issuer uses and must resolve to ≥ 32 bytes — a gated deploy whose secret does not resolve **fails** (fail closed). The resolved value must be identical in the ePHPm and switchboard environments. | +| `--share-link` | `SWITCHBOARD_SHARE_LINK` | `false` | Mint a temporary shareable-URL capability and post it in the PR comment for each **gated** deploy. Opt-in: a share link is a bearer capability. | +| `--share-link-ttl-secs` | `SWITCHBOARD_SHARE_LINK_TTL_SECS` | `86400` | TTL for a minted share link. Kept short — expiry is the primary control. | +| `--kv-secret-file` | `SWITCHBOARD_KV_SECRET_FILE` | *(none)* | File holding ePHPm's `[kv] secret`, used to derive the per-site RESP password so **teardown can bump the share-link revocation epoch**. Unset skips KV revocation (the override + checkout removal already revoke on this node). | +| `--kv-addr` | `SWITCHBOARD_KV_ADDR` | `127.0.0.1:6379` | ePHPm's KV RESP listener (`[kv.redis_compat] listen`). Only used for revocation when `--kv-secret-file` is set. | + +The one-time fleet setup this pairs with — the GitHub OAuth App, the global +`github-auth` mount, and `EPHPM_PREVIEW_SESSION_SECRET` (≥ 32 bytes) in the ePHPm +**and** switchboard environments — is ePHPm node config, not switchboard's; it is +described in [`docs/preview-access-gate.md`](docs/preview-access-gate.md). + ### GitHub reporting (optional) | Flag | Env | Default | Meaning | @@ -401,8 +437,10 @@ pinned to the crate's MSRV on the ephpm org's self-hosted fleet. | `src/validate.rs` | Claim-time re-validation of a deploy job: the queue-age bound and the current-PR-state check | | `src/drain.rs` | The `/drain` kick and the shared-secret file | | `src/deployer.rs` | The provisioning pipeline: fetch → manifest → env → quarantine the manifest → per-site override → atomic swap → chown to tenant → build → seed → health. `build:`/`seed:` run sandboxed via `ephpm exec --site` (fail-closed if unsupported). | -| `src/site_override.rs` | The per-site override ePHPm reads: validating `docroot:` and the env prepend against ePHPm's own containment rules, rendering the TOML, and writing it atomically | -| `src/teardown.rs` | Preview teardown: vhost dir, per-site database, override file, vhost temp/session state root, the API's `applied/` marker — and the refusal to call a partial teardown a success | +| `src/site_override.rs` | The per-site override ePHPm reads: validating `docroot:` and the env prepend against ePHPm's own containment rules, the `[preview_auth]` gate section, rendering the TOML, and writing it atomically | +| `src/preview_auth.rs` | The access-gate control plane: gating policy, session-secret resolution (fail closed), wire-compatible HS256 share-token minting, and the per-site KV password derivation | +| `src/kv.rs` | A tiny RESP2 client for bumping the share-link revocation epoch in a preview's KV keyspace on teardown (best-effort) | +| `src/teardown.rs` | Preview teardown: vhost dir, per-site database, override file, vhost temp/session state root, the API's `applied/` marker, the share-link revocation epoch — and the refusal to call a partial teardown a success | | `src/manifest.rs` | The `ephpm.yaml` app manifest schema, and moving it out of the served root once read | | `src/secrets.rs` | `${secret.NAME}` resolution from switchboard's own store | | `src/github.rs` | PR comments and Deployment statuses (sticky via the hidden marker) | diff --git a/docs/preview-access-gate.md b/docs/preview-access-gate.md index d73d7ac..1821cf1 100644 --- a/docs/preview-access-gate.md +++ b/docs/preview-access-gate.md @@ -1,15 +1,20 @@ -# Preview access gate — design (switchboard#26, Part B) +# Preview access gate — switchboard's control-plane half (switchboard#26, Part B) ## Status -**Specified, not implemented in switchboard.** The clean enforcement point is in -**ePHPm**, not switchboard, so this PR delivers Part A (authenticated fetch) and -this written design for Part B. The enforcement change is tracked as companion -ePHPm issue **ephpm/ephpm#487**; the switchboard-side piece (generate a -per-preview credential, write -it into the per-site override, surface it in the PR comment) lands **after** that -ePHPm key exists — writing it sooner would be an inert, bypassable gate, which is -worse than an honestly-documented gap. +**Implemented.** ePHPm ships the enforcement (a per-site `preview-gate` +middleware) and verification/revocation (ephpm/ephpm#487, merged in #491); +switchboard ships the control plane described here: it decides which previews to +gate, writes the `[preview_auth]` activation into the per-site override, mints +temporary shareable-URL capability tokens, and revokes them on teardown. + +> **Design note — this supersedes the original plan.** An earlier revision of +> this document proposed **per-preview HTTP Basic auth** with a switchboard- +> generated credential written into a new override key. That is not what shipped. +> ePHPm implemented a stronger mechanism — a **GitHub-OAuth login gate** that +> authorizes against real repo read access, plus revocable HS256 **share tokens** +> for people without repo access — and switchboard drives *that*. The threat +> model below still holds; the mechanism section is rewritten to match the code. ## The problem @@ -17,15 +22,17 @@ A deployed preview serves to anyone who resolves `*.preview.ephpm.dev`. Once switchboard can check out **private** repositories (Part A of #26), that preview is a private repo's code and content served to the whole internet. Fetching private code is pointless if the result is world-readable, so the two are one -decision. +decision — and a private preview that comes up ungated is the exact exposure this +feature exists to prevent. Every path where gating could silently not happen must +instead **fail the deploy loudly** (see "Fail closed"). ## Threat model **What the gate defends:** *preview privacy.* It stops a random internet visitor — someone who knows or guesses the preview hostname — from reading a preview's -pages, assets, or uploaded content. The intended audience (the PR author and -reviewers, i.e. people with read access to the base repo) can still reach it, -because the credential is posted in the PR comment, which only they can see. +pages, assets, or uploaded content. The intended audience reaches it by signing +in with GitHub (they are authorized automatically if they have read access to the +base repo), or via a temporary share link handed out by a repo member. **What it explicitly does NOT defend:** @@ -33,111 +40,137 @@ because the credential is posted in the PR comment, which only they can see. previews is a separate, existing property (per-vhost `open_basedir`, per-site DB/KV credentials, the `ephpm exec` sandbox). This gate is only about *who may make an HTTP request to a preview*. -- It does **not** protect against someone who already has read access to the - base repo — they are the intended audience and can see the credential. -- It does **not** resist a determined attacker who compromises a reviewer's - GitHub account or the PR comment stream. That is an account-security problem, - not a preview-privacy one. +- A **share link is a bearer capability**: anyone who has the link is in until it + expires or is revoked, without signing in. That is the point (sharing with + people who cannot authenticate) and is a *weaker* property than the OAuth gate; + it is stated in every PR comment that carries one. +- It does **not** resist a compromised reviewer GitHub account or a leaked PR + comment stream. That is account security, not preview privacy. - It is **not** a substitute for keeping genuinely sensitive data out of a preview environment. ## Why enforcement belongs in ePHPm, not switchboard -A correct gate has two hard requirements, and switchboard can satisfy neither on -its own today: - -1. **It must cover the static-file path as well as the PHP path, and fail - closed.** A preview serves static assets (JS/CSS/images) and — for a - `docroot: "."` WordPress checkout — arbitrary non-PHP files (`.txt`, `.sql` - dumps, uploads) directly off disk, *without* running any PHP. A gate that only - runs in PHP leaves all of that ungated. ePHPm already has exactly the right - primitive: the **request-phase middleware** chain runs on both the PHP path - (`Router::handle_php`) **and** the static-file path - (`Router::static_request_phase`, ephpm#395), *before the file's bytes are read - from disk*, and it **fails closed**. A `RESPOND` verdict (e.g. a `401`) short- - circuits the whole request. - -2. **The credential must be per-preview.** Every preview needs its own secret so - that leaking one does not open the others, and so a credential can rotate per - deploy. - -switchboard's only per-site configuration channel into ePHPm is the two-key -per-site **override file** (`document_root`, `auto_prepend_file`). That schema is -**deliberately closed** (see `ephpm-server/src/site_overrides.rs`): an arbitrary -key is a tenant-influenced sandbox-escape surface, so it is not switchboard's to -extend from the outside. And ePHPm's `[[middleware]]` mounts are **global** (one -chain for the whole server, matched only by a path glob) — they have no per-vhost -credential channel. So the enforcement layer must be added inside ePHPm. - -### Why not the tempting switchboard-only shortcut - -switchboard already writes an `auto_prepend_file` (the env prepend) that ePHPm -runs before every request. It is tempting to add a Basic-auth check to that PHP -prepend. **Rejected:** `auto_prepend_file` runs only on the **PHP** path, so it -leaves every static asset and non-PHP file ungated. That is a *bypassable* -control, and the issue is explicit that a bypassable gate is worse than an -honest gap. The prepend is the wrong layer for a security boundary. - -## Chosen mechanism - -**Per-preview HTTP Basic auth, enforced by an ePHPm request-phase gate that runs -on both the static and PHP paths and fails closed. The credential is generated -by switchboard and posted in the PR comment.** - -- **Credential:** a random, per-preview secret. Basic auth username can be the - site key; the password is a switchboard-generated high-entropy token, stored - in switchboard's own state and rotated per deploy. (It is deliberately *not* - ePHPm's per-site `HMAC(master_secret, site_key)` password — that one rotates on - every host restart, so it could not be posted in a durable PR comment.) -- **Surfacing:** the credential goes in the sticky PR comment, which is visible - only to users with read access to the base repo. The comment already exists; - it gains a "This preview is private — sign in with …" line. -- **Enforcement (ePHPm, companion issue):** a request-phase gate, fed a per-site - expected credential, returns `401 WWW-Authenticate: Basic` for any request - whose `Authorization` header does not match. It runs ahead of both the static - and PHP serving paths and fails closed. - -### How the per-site credential reaches ePHPm - -Two candidate shapes for the companion ePHPm change; the issue picks one: - -1. **A new per-site override key** — e.g. `require_basic_auth = ":"` (or a token) in `/.toml`. The - override loader already validates and fail-closes per site, and switchboard - already writes this file. This is the smallest, most consistent change: the - credential travels the exact channel `document_root` and `auto_prepend_file` - already do. The value is a *verifier* (hash), never the plaintext, so the - operator-owned file does not itself store a reusable secret. - -2. **A per-site binding for a builtin auth middleware** — expose the resolved - site key to the middleware `RequestCtx` and let a builtin `preview_auth` / - `api_key`-style module look the expected credential up from a per-site source. - More flexible, more surface; heavier than preview-privacy warrants. - -Preference: **option 1** (new override key). It reuses the existing per-site -config path, keeps the two-parser contract intact, and needs no new middleware -wiring. +Unchanged from the original analysis, and the reason the mechanism is split: + +1. **The gate must cover the static-file path as well as PHP, and fail closed.** + A preview serves static assets and — for a `docroot: "."` checkout — arbitrary + non-PHP files directly off disk without running any PHP. ePHPm's request-phase + middleware runs on both `Router::handle_php` **and** + `Router::static_request_phase` (ephpm#395), before a file's bytes are read, and + fails closed. An `auto_prepend_file` Basic-auth check (the tempting + switchboard-only shortcut) would run on the PHP path only and leave every + static file ungated — a bypassable control, rejected. +2. **The gate must be per-preview.** A preview fleet mints a new vhost per PR, and + ePHPm has no runtime config reload. The per-site **override file**, re-read + every `SITE_CONFIG_TTL` (~2 s), is switchboard's only per-preview channel; a + global `[[middleware]]` mount cannot be turned on for a brand-new preview + without a restart. So activation rides the override file, and ePHPm made + `preview_auth` a typed section in it (ephpm#487). + +## Chosen mechanism (as shipped) + +**A GitHub-OAuth login gate, activated per preview through the `[preview_auth]` +override section, plus revocable HS256 share-link capabilities.** ePHPm mints the +OAuth session and verifies both grant paths through one `Hs256Policy`; +switchboard activates the gate and mints share links. + +### 1. Gating policy (switchboard) + +- **Private repo → gate ON, always.** Its code is not world-readable, so neither + is its preview. Repo visibility comes from `repository.private` in the job/ + webhook payload (`JobRepository::private`), which **defaults to private when + absent** (fail closed). +- **Public repo → ungated by default**, gated only when the operator sets + `--gate-public-previews` (e.g. to keep unreleased work off the open internet). + +### 2. Activation (switchboard writes `[preview_auth]`) + +For a gated preview switchboard writes, into the same +`/.toml` it already writes `document_root` / +`auto_prepend_file` into: + +```toml +[preview_auth] +session_secret = "env:EPHPM_PREVIEW_SESSION_SECRET" # a REFERENCE, never the key +login_url = "/_ephpm/auth/github/login" +``` + +`session_secret` is a **reference** (`env:NAME` / `file:/abs` / a literal), +resolved by both the `github-auth` issuer and the gate — one source of truth, the +secret never in the tenant-adjacent file or the served tree. switchboard writes +the same reference it resolves for minting. + +### 3. Share links (switchboard mints) + +A share link is a `via:"share"` HS256 capability token, wire-compatible with +`ephpm_middleware_builtins::preview_gate::mint_share_token`, carrying `site` (the +canonical site key — per-preview), `via:"share"`, a random `jti`, `iat`, and a +short `exp`. Handed out as `https:///?ephpm_share=`. Minting +is opt-in (`--share-link`) because a bearer capability posted on every PR is a +choice, not a default; only the token travels in the URL, never the secret. + +### 4. Revocation (switchboard, on teardown) + +On teardown, removing the override + checkout already stops the gate on this +node. Because the per-vhost KV is gossip-replicated and a preview can be +redeployed, switchboard also bumps the per-site epoch — `preview:share:epoch = +now` in the preview's own KV keyspace (AUTH'd as the site with +`HMAC-SHA256([kv] secret, site)`) — which refuses every share token issued +before that instant, cluster-wide. Best-effort: an unreachable KV is a `warn!`, +never a teardown failure; skipped entirely when `--kv-secret-file` is unset. + +## Fail closed + +The one decision that, gotten wrong, publishes private code — so every silent- +not-happen path is a hard deploy failure instead: + +- A gated preview whose `session_secret` reference does not resolve to ≥ 32 bytes + **fails the deploy** (`resolve_preview_gate`) rather than shipping an open + preview or one ePHPm will 503. +- A gated preview with **no `--site-overrides-dir`** — nowhere to deliver the + gate — **fails the deploy** (`apply_site_override`). An ungated (public) preview + in the same situation only warns, because it was already public. +- Repo visibility **defaults to private** when the job payload omits `private`. + +## Operator configuration + +One-time, per fleet (ePHPm node config — **not** switchboard, stated here for +completeness): + +1. Register **one** GitHub OAuth App for the fleet and mount `github-auth` + globally in `ephpm.toml` with its `client_id`/`client_secret`, the per-repo + access target, `session_secret = "env:EPHPM_PREVIEW_SESSION_SECRET"`, and for a + wildcard fleet the apex-flow knobs `redirect_uri` (the one fixed callback host) + and `cookie_domain`. +2. Set `EPHPM_PREVIEW_SESSION_SECRET` (≥ 32 bytes) in the ePHPm **and** switchboard + process environments — both must resolve the reference to identical bytes for a + switchboard-minted share token to verify in the gate. + +switchboard flags: + +| Flag / env | Default | Purpose | +|---|---|---| +| `--gate-public-previews` / `SWITCHBOARD_GATE_PUBLIC_PREVIEWS` | off | gate public repos too (private are always gated) | +| `--preview-session-secret-ref` / `SWITCHBOARD_PREVIEW_SESSION_SECRET_REF` | `env:EPHPM_PREVIEW_SESSION_SECRET` | the reference written into the override and resolved to mint share tokens | +| `--share-link` / `SWITCHBOARD_SHARE_LINK` | off | mint + post a share link per gated deploy | +| `--share-link-ttl-secs` / `SWITCHBOARD_SHARE_LINK_TTL_SECS` | `86400` | share-link TTL (kept short) | +| `--kv-secret-file` / `SWITCHBOARD_KV_SECRET_FILE` | unset | ePHPm's `[kv] secret`, for teardown epoch revocation (unset skips it) | +| `--kv-addr` / `SWITCHBOARD_KV_ADDR` | `127.0.0.1:6379` | ePHPm's KV RESP listener | + +## Rollout ordering + +Only write `[preview_auth]` once an ePHPm that **enforces** it is deployed to the +node. An older ePHPm treats the unknown section leniently (ignored, reported) and +would serve the preview ungated — so the fleet upgrades ePHPm first, then starts +writing the key. Same discipline `document_root`/`auto_prepend_file` and the +`ephpm exec` fail-closed check already follow. ## Alternatives weighed | Option | Verdict | |---|---| -| **Shared per-preview token in URL/cookie** (magic link) | Workable, but tokens in URLs leak via `Referer`, browser history and access logs; needs cookie-setting logic and a redirect dance. More moving parts than Basic auth for the same protection. Secondary. | -| **Basic auth, per-preview generated credential** (chosen) | Zero client state, works for browsers *and* `curl`/CI, trivial to enforce in the request phase on both paths, credential fits naturally in the PR comment. | -| **IP allowlist** | Reviewers are on dynamic/varied IPs (home, mobile, CI); an allowlist that fits a public preview audience is impractical. Could be an *optional add-on* for a fixed-office deployment, not the default. | -| **GitHub-OAuth-gated** (tie the gate to actual repo read access) | Strongest — authorizes against real GitHub permissions rather than a shared secret — but needs an OAuth app, a callback handler, a session store, and per-repo authorization checks. Overkill for preview-privacy; a good future upgrade if the shared-secret model proves too coarse. | - -## Scope split - -- **This PR (switchboard):** Part A only — the authenticated fetch — plus this - design. -- **Companion ePHPm issue (ephpm/ephpm#487):** the request-phase per-site access gate (option 1 - above): a new per-site override key carrying a Basic-auth verifier, enforced on - both the static and PHP paths, failing closed, with unit tests that a request - with no/!wrong credential gets `401` and a request with the right credential is - served (asserting the before-state: ungated today). -- **Follow-up switchboard PR (after the ePHPm key ships):** generate the - per-preview credential, write the verifier into the per-site override, post the - plaintext credential in the PR comment, and rotate it per deploy. Gated behind - an explicit opt-in until the enforcing ePHPm is known to be deployed to the - node (same rollout-ordering discipline as the `ephpm exec` fail-closed check). +| **Per-preview HTTP Basic auth** (the original plan here) | Superseded. Works for browsers and `curl`, but a shared per-preview secret is coarser than authorizing against real repo access, and it does not give the "log in with GitHub" UX the OAuth gate does. ePHPm shipped OAuth instead. | +| **GitHub-OAuth login gate** (shipped) | Authorizes against real GitHub repo-read permission; no shared secret to hand out for the primary path; enforced request-phase on both static and PHP paths, fail closed. | +| **Revocable share tokens** (shipped, secondary) | For people without repo access. A short-lived, per-preview, revocable bearer capability — explicitly weaker, explicitly labelled. | +| **IP allowlist** | Reviewers are on varied IPs; impractical as a default. Possible optional add-on for a fixed-office deployment. | diff --git a/src/config.rs b/src/config.rs index ad1de7b..eb0cb5e 100644 --- a/src/config.rs +++ b/src/config.rs @@ -7,6 +7,7 @@ use std::path::PathBuf; use std::time::Duration; +use anyhow::Context as _; use clap::Parser; #[derive(Parser, Debug)] @@ -206,6 +207,62 @@ pub struct Config { #[arg(long, default_value_t = false, env = "SWITCHBOARD_FORK_SECRETS")] pub fork_secrets: bool, + // ── preview access gate (ephpm#487/#491) ────────────────────────── + /// Gate **public** repos' previews too. Private repos are *always* gated (a + /// private repo's preview must not be world-readable); this widens the policy + /// to public code as well — for keeping unreleased work off the open internet. + #[arg( + long, + default_value_t = false, + env = "SWITCHBOARD_GATE_PUBLIC_PREVIEWS" + )] + pub gate_public_previews: bool, + + /// The `session_secret` **reference** written into a gated preview's + /// `[preview_auth]` section and resolved by switchboard to mint share tokens. + /// + /// Accepts `env:NAME` (default), `file:/abs/path`, or a literal (discouraged — + /// the reference should point at a secret, not be one, since it is written + /// into the tenant-adjacent override file). It **must** be the same reference + /// the `github-auth` issuer resolves, so the two derive one shared key, and it + /// must resolve to ≥ 32 bytes or a gated deploy fails (fail closed). The + /// resolved value must be identical in the ePHPm and switchboard process + /// environments — the operator sets `EPHPM_PREVIEW_SESSION_SECRET` for both. + #[arg( + long, + default_value = "env:EPHPM_PREVIEW_SESSION_SECRET", + env = "SWITCHBOARD_PREVIEW_SESSION_SECRET_REF" + )] + pub preview_session_secret_ref: String, + + /// Mint a temporary shareable-URL capability and post it in the PR comment for + /// each **gated** deploy. Off by default: a share link is a bearer capability + /// (anyone with it is in until expiry), so advertising one on every gated PR is + /// an explicit opt-in. + #[arg(long, default_value_t = false, env = "SWITCHBOARD_SHARE_LINK")] + pub share_link: bool, + + /// TTL (seconds) for a minted share link. Kept short — expiry is the primary + /// control, so a leaked link self-heals. Default 1 day. + #[arg( + long, + default_value_t = 86_400, + env = "SWITCHBOARD_SHARE_LINK_TTL_SECS" + )] + pub share_link_ttl_secs: u64, + + /// Path to a file holding ePHPm's `[kv] secret`, used to derive the per-site + /// RESP password so **teardown can bump the share-link revocation epoch** in a + /// preview's KV keyspace. Unset skips KV revocation (the override + checkout + /// removal already revoke on this node; a clustered leak self-heals at expiry). + #[arg(long, env = "SWITCHBOARD_KV_SECRET_FILE")] + pub kv_secret_file: Option, + + /// ePHPm's KV RESP listener address (`[kv.redis_compat] listen`). Only used + /// for share-link revocation when `--kv-secret-file` is set. + #[arg(long, default_value = "127.0.0.1:6379", env = "SWITCHBOARD_KV_ADDR")] + pub kv_addr: String, + // ── GitHub reporting (optional) ──────────────────────────────────── /// GitHub App private key path (PEM file). Omit to run without GitHub /// reporting — deploys still happen, they are just not reported on the PR. @@ -262,6 +319,37 @@ impl Config { self.app_id.is_some() && self.app_key.is_some() } + /// The TTL applied to a minted share link. + #[must_use] + pub fn share_token_ttl(&self) -> Duration { + Duration::from_secs(self.share_link_ttl_secs.max(1)) + } + + /// Resolve ePHPm's `[kv] secret` from `--kv-secret-file`, for deriving the + /// per-site RESP password used to bump the share-link revocation epoch. + /// + /// `Ok(None)` when no file is configured — KV revocation is then skipped + /// (the override + checkout removal already revoke on this node). The value is + /// trimmed to match how ePHPm reads its own secret. + /// + /// # Errors + /// + /// Returns an error if the file is configured but cannot be read, or is empty. + pub fn kv_secret(&self) -> anyhow::Result> { + let Some(path) = &self.kv_secret_file else { + return Ok(None); + }; + let raw = std::fs::read_to_string(path) + .with_context(|| format!("cannot read --kv-secret-file {}", path.display()))?; + let secret = raw.trim().to_string(); + anyhow::ensure!( + !secret.is_empty(), + "--kv-secret-file {} is empty — it must hold ePHPm's [kv] secret", + path.display() + ); + Ok(Some(secret)) + } + /// ePHPm's `sites_domain_suffix` for this node, resolved. /// /// Unset means "the node's suffix matches our preview domain", the @@ -721,6 +809,80 @@ mod tests { c.validate().unwrap(); } + // ── preview access gate (ephpm#487/#491) ─────────────────────────── + + #[test] + fn access_gate_defaults_are_safe() { + let c = parse_single_node(&[]); + assert!( + !c.gate_public_previews, + "public previews are ungated by default; private are always gated" + ); + assert_eq!( + c.preview_session_secret_ref, "env:EPHPM_PREVIEW_SESSION_SECRET", + "the default reference matches the issuer's, one source of truth" + ); + assert!( + !c.share_link, + "share links are opt-in (a bearer capability)" + ); + assert_eq!(c.share_link_ttl_secs, 86_400); + assert_eq!(c.share_token_ttl(), Duration::from_secs(86_400)); + assert_eq!(c.kv_addr, "127.0.0.1:6379"); + assert!( + c.kv_secret_file.is_none(), + "no KV revocation unless configured" + ); + c.validate().unwrap(); + } + + #[test] + fn access_gate_flags_parse() { + let c = parse_single_node(&[ + "--gate-public-previews", + "--preview-session-secret-ref", + "file:/etc/switchboard/preview.key", + "--share-link", + "--share-link-ttl-secs", + "3600", + "--kv-secret-file", + "/etc/ephpm/kv.secret", + "--kv-addr", + "127.0.0.1:7000", + ]); + assert!(c.gate_public_previews); + assert_eq!( + c.preview_session_secret_ref, + "file:/etc/switchboard/preview.key" + ); + assert!(c.share_link); + assert_eq!(c.share_token_ttl(), Duration::from_secs(3600)); + assert_eq!( + c.kv_secret_file, + Some(PathBuf::from("/etc/ephpm/kv.secret")) + ); + assert_eq!(c.kv_addr, "127.0.0.1:7000"); + c.validate().unwrap(); + } + + #[test] + fn kv_secret_is_none_when_unconfigured_and_read_when_set() { + let none = parse_single_node(&[]); + assert!(none.kv_secret().unwrap().is_none()); + + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("kv.secret"); + std::fs::write(&path, " the-kv-secret\n").unwrap(); + let c = parse_single_node(&["--kv-secret-file", path.to_str().unwrap()]); + assert_eq!(c.kv_secret().unwrap().as_deref(), Some("the-kv-secret")); + + std::fs::write(&path, " \n").unwrap(); + assert!( + c.kv_secret().is_err(), + "an empty KV secret file is an error" + ); + } + #[test] fn missing_state_dir_is_an_error() { // The queue is the daemon's only input; there is nothing sensible to diff --git a/src/deployer.rs b/src/deployer.rs index cde2604..c08f244 100644 --- a/src/deployer.rs +++ b/src/deployer.rs @@ -42,6 +42,7 @@ use anyhow::Context; use tokio::process::Command; use crate::manifest::AppManifest; +use crate::preview_auth; use crate::secrets::Secrets; use crate::site_override; @@ -153,6 +154,13 @@ pub struct PreviewRequest { /// True when the PR head comes from a fork (or the head repo is gone). /// Gates deploys and secret resolution — see [`fork_deploy_gate`]. pub fork: bool, + /// Whether the **base** repository is private (`repository.private` in the + /// webhook/job payload). A private repo's preview must be access-gated so it + /// is not world-readable — see [`crate::preview_auth::should_gate`]. **Absent + /// means private** at the job boundary (fail closed): switchboard-api emits + /// the field, so a document without it has unproven visibility and is treated + /// as private rather than published open. + pub private: bool, } impl PreviewRequest { @@ -266,6 +274,24 @@ pub struct DeployContext<'a> { pub health_timeout: Duration, /// Interval between health poll attempts. pub health_interval: Duration, + + // ── preview access gate (ephpm#487/#491) ─────────────────────────── + /// Gate **public** repos' previews too. Private repos are always gated; this + /// only widens the policy to public code (`--gate-public-previews`). + pub gate_public_previews: bool, + /// The `session_secret` **reference** (`env:NAME` / `file:/abs` / literal) + /// shared with the `github-auth` issuer. Written verbatim into a gated + /// preview's `[preview_auth]` section, and resolved by switchboard to obtain + /// the bytes it mints share tokens with. A gated deploy whose secret cannot be + /// resolved to ≥ 32 bytes **fails** (fail closed) rather than shipping an open + /// preview. + pub preview_session_secret_ref: &'a str, + /// Mint a temporary shareable-URL capability and include it in the PR comment + /// for each **gated** deploy. Off by default: a share link is a bearer + /// capability, so posting one on every gated PR is opt-in. + pub mint_share_link: bool, + /// TTL for a minted share link. Kept short — expiry is the primary control. + pub share_token_ttl: Duration, } /// Result of a successful deployment. @@ -281,6 +307,13 @@ pub struct DeployResult { /// Whether the health check passed within the timeout (false = timed out or /// health gating disabled). pub healthy: bool, + /// Whether this preview is access-gated (a `[preview_auth]` section was + /// written). Drives the PR comment's access guidance. + pub gated: bool, + /// A minted temporary shareable URL, when one was minted (gated deploy with + /// `--share-link`). Carries only the bearer token in its query string; the + /// signing secret never appears here. + pub share_url: Option, } /// Deploy a preview. @@ -349,9 +382,44 @@ pub async fn deploy_preview( hostname = %hostname, site_key = %site_key, site_dir = %site_dir.display(), + private = req.private, "deploying preview" ); + // Preview access gate (ephpm#487/#491). Decide gating from the base repo's + // visibility, and — for a gated preview — resolve the shared session secret + // NOW, before anything is fetched or served. A private preview that comes up + // ungated is the exact exposure this feature exists to prevent, so a + // misconfigured gate (unresolvable/short secret) must FAIL the deploy here, + // not fall back to an open preview. The resolved bytes are reused to mint the + // optional share link after the health gate. + let gate = resolve_preview_gate( + req.private, + ctx.gate_public_previews, + ctx.preview_session_secret_ref, + )?; + let gated = gate.is_some(); + let (preview_auth_section, session_secret) = match gate { + Some((section, secret)) => { + tracing::info!( + %hostname, + site_key = %site_key, + private = req.private, + "preview is access-gated — writing [preview_auth]; unauthenticated visitors \ + are redirected to GitHub login" + ); + (Some(section), Some(secret)) + } + None => { + tracing::info!( + %hostname, + site_key = %site_key, + "preview is public and ungated (set --gate-public-previews to gate public repos)" + ); + (None, None) + } + }; + // (1) Fetch into a staging directory first, then move into place. let tmp_dir = site_dir.with_extension("tmp"); if tmp_dir.exists() { @@ -450,6 +518,7 @@ pub async fn deploy_preview( let over = site_override::SiteOverride { document_root, auto_prepend_file: Some(prepend), + preview_auth: preview_auth_section, }; apply_site_override(ctx, &site_key, &over, &hostname).await?; @@ -528,11 +597,36 @@ pub async fn deploy_preview( // (9) Health-gate: only report ready once the site serves a 200. let healthy = wait_healthy(&preview_url, &manifest.health, ctx).await; + // (10) Optionally mint a temporary share link for this gated preview. The + // token is a `via:"share"` bearer capability bound to this preview's site + // key; only the token travels (in the URL query), never the signing secret. + let share_url = match (gated && ctx.mint_share_link, session_secret.as_deref()) { + (true, Some(secret)) => { + let now = unix_now(); + let ttl = ctx.share_token_ttl.as_secs().max(1); + let jti = preview_auth::generate_jti(); + let token = preview_auth::mint_share_token(secret, &site_key, &jti, now, now + ttl); + tracing::info!( + %hostname, + site_key = %site_key, + ttl_secs = ttl, + "minted a temporary share link (bearer capability) for this gated preview" + ); + Some(preview_auth::share_url( + &preview_url, + preview_auth::DEFAULT_SHARE_PARAM, + &token, + )) + } + _ => None, + }; + let duration = start.elapsed(); tracing::info!( %hostname, framework = framework.as_str(), healthy, + gated, duration_ms = duration.as_millis(), "preview deployed" ); @@ -543,9 +637,57 @@ pub async fn deploy_preview( duration, php_version: Some(manifest.php), healthy, + gated, + share_url, }) } +/// Current unix time in whole seconds — the clock the share token's `iat`/`exp` +/// use, and the same one ePHPm's gate compares against. +fn unix_now() -> u64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_or(0, |d| d.as_secs()) +} + +/// Decide the access-gate outcome for a deploy, and — for a gated preview — +/// resolve the shared session secret. +/// +/// Returns `Ok(None)` for an ungated preview, or `Ok(Some((section, secret)))` +/// for a gated one: the `[preview_auth]` section to write and the resolved secret +/// bytes to mint share tokens with. +/// +/// **Fail closed** is the whole contract: a private preview (or any preview when +/// `--gate-public-previews` is set) whose session secret cannot be resolved to a +/// usable key is an `Err`, never an ungated deploy. This is the one decision that, +/// gotten wrong, publishes private code — so it is a pure function with its own +/// tests rather than a branch buried in the pipeline. +/// +/// # Errors +/// +/// Returns an error when the preview must be gated but the session-secret +/// reference does not resolve to ≥ 32 bytes, or the resulting section is invalid. +fn resolve_preview_gate( + repo_is_private: bool, + gate_public_previews: bool, + session_secret_ref: &str, +) -> anyhow::Result)>> { + if !preview_auth::should_gate(repo_is_private, gate_public_previews) { + return Ok(None); + } + let secret = preview_auth::resolve_session_secret(session_secret_ref).context( + "refusing to deploy a gated preview without a usable session secret — a private \ + preview must never come up ungated (fail closed). Set --preview-session-secret-ref \ + (SWITCHBOARD_PREVIEW_SESSION_SECRET_REF) to a reference that resolves to ≥ 32 bytes, \ + the same one the github-auth issuer uses", + )?; + let section = site_override::PreviewAuthSection::new( + session_secret_ref, + preview_auth::DEFAULT_LOGIN_URL, + )?; + Ok(Some((section, secret))) +} + /// Materialize the PR head at `req.sha` into `dest`. /// /// The preferred path is a shallow fetch of `refs/pull//head` from the @@ -1101,6 +1243,19 @@ async fn apply_site_override( } let Some(overrides_dir) = ctx.site_overrides_dir else { + // Fail CLOSED for a gated preview: the `[preview_auth]` section is the + // ONLY thing that makes the preview private, and the override file is the + // only channel for it. With nowhere to write it, serving the preview + // would publish private code world-readable — the exact exposure the gate + // exists to prevent. A public/ungated preview still degrades to a warning + // (it was already public). + anyhow::ensure!( + over.preview_auth.is_none(), + "refusing to deploy a gated (private) preview without --site-overrides-dir: the \ + [preview_auth] gate can only be delivered through ePHPm's [server] \ + site_overrides_dir, and serving this preview ungated would publish private code. \ + Configure --site-overrides-dir (SWITCHBOARD_SITE_OVERRIDES_DIR)" + ); tracing::warn!( %hostname, docroot = over.document_root.declared(), @@ -1120,10 +1275,13 @@ async fn apply_site_override( site_key, docroot = over.document_root.declared(), auto_prepend_file = over.auto_prepend_file.as_ref().map(site_override::PrependFile::declared), + access_gated = over.preview_auth.is_some(), path = %path.display(), - "wrote per-site override (document root + env prepend). An ePHPm older \ - than the auto_prepend_file key (ephpm#463) ignores that key with a \ - warning and still honours the document root" + "wrote per-site override (document root + env prepend + optional access gate). \ + An ePHPm older than the auto_prepend_file key (ephpm#463) ignores that key \ + with a warning and still honours the document root; one predating the \ + preview-gate (ephpm#487) ignores [preview_auth] — so the fleet must upgrade \ + ePHPm before switchboard starts writing it (rollout ordering)" ); Ok(()) } @@ -1593,6 +1751,7 @@ mod tests { sha: "0123456789abcdef0123456789abcdef01234567".into(), installation_id: None, fork: false, + private: false, } } @@ -1748,6 +1907,7 @@ mod tests { let over = site_override::SiteOverride { document_root, auto_prepend_file: Some(prepend), + preview_auth: None, }; let path = site_override::write_override(&overrides, "app-pr-7", &over) .await @@ -1950,6 +2110,10 @@ mod tests { fetch_token: None, health_timeout: Duration::ZERO, health_interval: Duration::from_secs(1), + gate_public_previews: false, + preview_session_secret_ref: "env:EPHPM_PREVIEW_SESSION_SECRET", + mint_share_link: false, + share_token_ttl: Duration::from_secs(86_400), }; assert!(!wait_healthy("https://example.invalid", "/", &ctx).await); } @@ -1975,6 +2139,10 @@ mod tests { fetch_token: None, health_timeout: Duration::ZERO, health_interval: Duration::from_secs(1), + gate_public_previews: false, + preview_session_secret_ref: "env:EPHPM_PREVIEW_SESSION_SECRET", + mint_share_link: false, + share_token_ttl: Duration::from_secs(86_400), } } @@ -1988,9 +2156,23 @@ mod tests { auto_prepend_file: Some( site_override::validate_prepend(checkout, PREPEND_FILE).unwrap(), ), + preview_auth: None, } } + /// A gated variant of [`staged_override`] carrying a valid `[preview_auth]`. + fn staged_gated_override(checkout: &Path, docroot: &str) -> site_override::SiteOverride { + let mut over = staged_override(checkout, docroot); + over.preview_auth = Some( + site_override::PreviewAuthSection::new( + "env:EPHPM_PREVIEW_SESSION_SECRET", + "/_ephpm/auth/github/login", + ) + .unwrap(), + ); + over + } + #[tokio::test] async fn subdirectory_docroot_publishes_an_override_file() { let dir = tempfile::tempdir().unwrap(); @@ -2082,6 +2264,123 @@ mod tests { .expect("an unconfigured overrides dir must not fail the deploy"); } + // ── the access gate: fail-closed activation (ephpm#487/#491) ───────── + + /// A gated preview publishes a `[preview_auth]` section into the override. + #[tokio::test] + async fn gated_preview_writes_the_preview_auth_section() { + let dir = tempfile::tempdir().unwrap(); + let overrides = dir.path().join("overrides"); + let checkout = dir.path().join("checkout"); + std::fs::create_dir_all(&checkout).unwrap(); + let secrets = Secrets::default(); + let ctx = override_ctx(dir.path(), Some(&overrides), &secrets); + + apply_site_override( + &ctx, + "app-pr-1", + &staged_gated_override(&checkout, "public"), + "app-pr-1.preview.ephpm.dev", + ) + .await + .unwrap(); + + let written = tokio::fs::read_to_string(overrides.join("app-pr-1.toml")) + .await + .unwrap(); + assert!(written.contains("[preview_auth]"), "{written}"); + assert!( + written.contains("session_secret = \"env:EPHPM_PREVIEW_SESSION_SECRET\""), + "the reference, never the key: {written}" + ); + assert!( + written.contains("login_url = \"/_ephpm/auth/github/login\""), + "{written}" + ); + } + + /// **Fail closed.** A GATED preview with no `--site-overrides-dir` has nowhere + /// to deliver the gate, so serving it would publish private code. The deploy + /// must fail, not degrade to an open preview — unlike an ungated one, which + /// only warns (asserted by `missing_overrides_dir_warns_but_does_not_fail_the_deploy`). + #[tokio::test] + async fn gated_preview_without_overrides_dir_fails_the_deploy() { + let dir = tempfile::tempdir().unwrap(); + let checkout = dir.path().join("checkout"); + std::fs::create_dir_all(&checkout).unwrap(); + let secrets = Secrets::default(); + let ctx = override_ctx(dir.path(), None, &secrets); + + let err = apply_site_override( + &ctx, + "app-pr-1", + &staged_gated_override(&checkout, "public"), + "app-pr-1.preview.ephpm.dev", + ) + .await + .expect_err("a gated preview with nowhere to write the gate must fail closed"); + assert!(err.to_string().contains("--site-overrides-dir"), "{err}"); + assert!( + err.to_string().contains("ungated"), + "the error must name the exposure it prevents: {err}" + ); + } + + // ── the gate decision (fail-closed) ───────────────────────────────── + + #[test] + fn public_preview_is_not_gated() { + // A public repo with the default policy needs no secret at all. + assert!( + resolve_preview_gate(false, false, "env:DOES_NOT_MATTER") + .unwrap() + .is_none() + ); + } + + /// **The private-repo-with-no-secret fail-closed test.** A private preview + /// whose session secret does not resolve must be a hard error — never a + /// deploy that silently comes up ungated. + #[test] + fn private_preview_without_a_secret_fails_closed_not_open() { + let err = resolve_preview_gate(true, false, "env:EPHPM_TEST_GATE_DEFINITELY_UNSET") + .expect_err("a private preview with no usable secret must fail the deploy"); + assert!( + err.to_string().contains("ungated"), + "the failure must be about not shipping an open preview: {err}" + ); + } + + #[test] + fn private_preview_with_a_good_secret_is_gated() { + let name = "EPHPM_TEST_GATE_SECRET_OK"; + // SAFETY: unique name; test-local. + unsafe { std::env::set_var(name, "0123456789abcdef0123456789abcdef") }; + let gate = resolve_preview_gate(true, false, &format!("env:{name}")).unwrap(); + let (section, secret) = gate.expect("a private preview with a usable secret must gate"); + assert_eq!(section.session_secret_ref(), format!("env:{name}")); + assert_eq!(section.login_url(), preview_auth::DEFAULT_LOGIN_URL); + assert_eq!( + secret.len(), + 32, + "the resolved bytes are handed on for minting" + ); + unsafe { std::env::remove_var(name) }; + } + + #[test] + fn public_preview_is_gated_when_the_operator_opts_in() { + let name = "EPHPM_TEST_GATE_SECRET_PUBLIC"; + unsafe { std::env::set_var(name, "0123456789abcdef0123456789abcdef") }; + assert!( + resolve_preview_gate(false, true, &format!("env:{name}")) + .unwrap() + .is_some(), + "--gate-public-previews gates a public repo too" + ); + unsafe { std::env::remove_var(name) }; + } + // ── build/seed run through `ephpm exec --site` (the root-RCE fix) ──── fn sandbox() -> SandboxExec<'static> { @@ -2377,6 +2676,7 @@ mod tests { sha: sha.to_owned(), installation_id: Some(42), fork: false, + private: false, } } diff --git a/src/github.rs b/src/github.rs index f6a2a43..d087ed2 100644 --- a/src/github.rs +++ b/src/github.rs @@ -300,7 +300,7 @@ fn format_deploy_comment(result: &DeployResult) -> String { "deployed (health check pending)" }; - format!( + let mut body = format!( "{COMMENT_MARKER}\n\ **ePHPm Preview** — {status}\n\n\ | | |\n\ @@ -312,7 +312,41 @@ fn format_deploy_comment(result: &DeployResult) -> String { Preview updates automatically on each push to this PR.", result.framework.as_str(), result.duration.as_secs_f64(), - ) + ); + body.push_str(&access_section(result)); + body +} + +/// The access-guidance block appended to a **gated** preview's comment. +/// +/// A gated preview is not world-readable, so a reviewer needs to be told how to +/// get in: log in with GitHub (they are authorised automatically if they have +/// read access to the repo the preview is for). When switchboard also minted a +/// share link, it is included with the bearer-capability warning stated plainly — +/// anyone with the link is in until it expires — because that is a weaker property +/// than the OAuth gate and the person pasting it must know so. The signing secret +/// never appears here; only the token, inside the URL, does. +/// +/// An ungated (public) preview gets no block — its content is already public. +fn access_section(result: &DeployResult) -> String { + if !result.gated { + return String::new(); + } + let mut section = String::from( + "\n\n**Access:** this preview is private. Sign in with GitHub at the URL above — \ + you'll be authorised automatically if your GitHub account has read access to this \ + repository.", + ); + if let Some(share) = &result.share_url { + section.push_str(&format!( + "\n\n**Shareable link (no login required):** {share}\n\n\ + > ⚠️ This link is a bearer capability: **anyone who has it can view the preview** \ + > until it expires or is revoked, without signing in. Share it only with people \ + > who should see this preview, and don't post it anywhere public. It is revoked \ + > automatically when the PR is closed." + )); + } + section } #[cfg(test)] @@ -329,6 +363,8 @@ mod tests { duration: Duration::from_millis(14_320), php_version: None, healthy: true, + gated: false, + share_url: None, }; let comment = format_deploy_comment(&result); assert!(comment.contains("https://pr-42.my-blog.preview.ephpm.dev")); @@ -350,6 +386,8 @@ mod tests { duration: Duration::from_millis(9_500), php_version: Some("8.4".into()), healthy: false, + gated: false, + share_url: None, }; let comment = format_deploy_comment(&result); assert!(comment.contains(":8084"), "PHP 8.4 should use port 8084"); @@ -368,6 +406,8 @@ mod tests { duration: Duration::from_millis(3_000), php_version: Some("8.3".into()), healthy: true, + gated: false, + share_url: None, }; let comment = format_deploy_comment(&result); assert!( @@ -394,6 +434,8 @@ mod tests { duration: Duration::from_millis(2_449), php_version: Some("8.5".into()), healthy: true, + gated: false, + share_url: None, }; let comment = format_deploy_comment(&result); assert!(comment.contains("2.4s"), "got: {comment}"); @@ -402,6 +444,71 @@ mod tests { assert!(!comment.contains(":8085")); } + // ── access-gate guidance (ephpm#487/#491) ────────────────────────── + + fn gated_result(share_url: Option) -> DeployResult { + DeployResult { + hostname: "pr-1.app.preview.ephpm.dev".into(), + framework: Framework::Laravel, + duration: Duration::from_millis(3_000), + php_version: Some("8.4".into()), + healthy: true, + gated: true, + share_url, + } + } + + #[test] + fn ungated_comment_has_no_access_section() { + let result = DeployResult { + hostname: "pr-1.app.preview.ephpm.dev".into(), + framework: Framework::Laravel, + duration: Duration::from_millis(1), + php_version: None, + healthy: true, + gated: false, + share_url: None, + }; + let comment = format_deploy_comment(&result); + assert!( + !comment.contains("Access:"), + "a public preview needs no access block: {comment}" + ); + assert!(!comment.contains("Shareable link")); + } + + #[test] + fn gated_comment_tells_the_reviewer_to_log_in() { + let comment = format_deploy_comment(&gated_result(None)); + assert!(comment.contains("Access:"), "{comment}"); + assert!(comment.contains("Sign in with GitHub"), "{comment}"); + assert!(comment.contains("read access"), "{comment}"); + // No share link was minted, so none is advertised. + assert!(!comment.contains("Shareable link"), "{comment}"); + } + + /// A minted share link is shown with the bearer-capability warning, and only + /// the token (inside the URL) appears — never the signing secret. + #[test] + fn gated_comment_with_a_share_link_warns_it_is_a_bearer_capability() { + let token = "eyJhbGciOiJIUzI1NiJ9.payload.sig"; + let url = format!("https://pr-1.app.preview.ephpm.dev:8084/?ephpm_share={token}"); + let comment = format_deploy_comment(&gated_result(Some(url.clone()))); + assert!( + comment.contains(&url), + "the full share URL must be present: {comment}" + ); + assert!(comment.contains("bearer capability"), "{comment}"); + assert!(comment.contains("anyone who has it"), "{comment}"); + assert!( + comment.contains("revoked"), + "must say teardown revokes it: {comment}" + ); + // The comment carries the token (in the URL) but nothing that looks like + // the raw HS256 secret — there is no separate secret field to leak. + assert!(comment.contains(token), "the token travels in the URL"); + } + #[test] fn teardown_body_is_marked_and_removed() { let body = teardown_comment_body(); diff --git a/src/job.rs b/src/job.rs index 576be80..959fd9a 100644 --- a/src/job.rs +++ b/src/job.rs @@ -82,6 +82,23 @@ pub struct JobRepository { /// `https://` clone URL of the **base** repo — the fetch source, because /// `refs/pull//head` resolves there even for forks. pub clone_url: String, + /// Whether the base repository is **private** (`repository.private` in the + /// GitHub payload; switchboard-api copies it into the job file). + /// + /// **Absent means private** (fail closed): a private repo's preview must be + /// access-gated, and a job document lacking the field has unproven visibility. + /// switchboard-api has emitted `private` since it began writing schema-1 jobs + /// (it is in the README's own example), so a document without it was either + /// not written by the API or predates it — either way, defaulting to private + /// gates a preview that might otherwise leak, and the worst case for a genuinely + /// public repo is a login prompt. + #[serde(default = "private_when_absent")] + pub private: bool, +} + +/// Serde default for [`JobRepository::private`]: absent ⇒ private (fail closed). +fn private_when_absent() -> bool { + true } /// The pull request. @@ -184,6 +201,7 @@ impl Job { sha: self.pull_request.head.sha.clone(), installation_id: self.installation_id, fork: self.pull_request.fork, + private: self.repository.private, } } } @@ -288,6 +306,47 @@ mod tests { assert!(job.to_preview_request().fork); } + #[test] + fn public_repo_reaches_the_request_as_not_private() { + // The sample says "private": false. + let job = Job::parse(sample_json("l", "deploy").as_bytes()).unwrap(); + assert!(!job.repository.private); + assert!( + !job.to_preview_request().private, + "a public repo is not gated" + ); + } + + #[test] + fn private_repo_reaches_the_request() { + let doc = sample_json("l", "deploy").replace("\"private\": false", "\"private\": true"); + let job = Job::parse(doc.as_bytes()).unwrap(); + assert!(job.repository.private); + assert!( + job.to_preview_request().private, + "a private repo must reach the deployer so its preview is gated" + ); + } + + /// **Absent visibility fails closed.** A schema-1 job with no `private` field + /// is treated as private — its preview is gated rather than published open. + #[test] + fn absent_private_field_is_treated_as_private() { + // Drop the whole `, "private": false` tail (comma included) so the JSON + // stays valid without the field. + let doc = sample_json("l", "deploy").replace(",\n \"private\": false", ""); + assert!( + !doc.contains("\"private\""), + "test setup must drop the field" + ); + let job = Job::parse(doc.as_bytes()).expect("private is optional, not required"); + assert!( + job.repository.private, + "absent visibility must default to private (fail closed)" + ); + assert!(job.to_preview_request().private); + } + #[test] fn rejects_unknown_schema() { // A schema bump may redefine field meanings. Guessing is worse than diff --git a/src/kv.rs b/src/kv.rs new file mode 100644 index 0000000..1e932f6 --- /dev/null +++ b/src/kv.rs @@ -0,0 +1,261 @@ +//! A tiny RESP2 client for the one thing switchboard needs from ePHPm's KV: to +//! write a preview's **share-link revocation** keys on teardown. +//! +//! # Why a client at all, and why this narrow +//! +//! The preview access gate ([`crate::preview_auth`]) lets a repo member mint a +//! `via:"share"` bearer token that opens one preview without a GitHub login. The +//! gate revokes such tokens two ways, both read from the request's **own +//! per-vhost KV keyspace** (ephpm#487/#491): +//! +//! * a per-`jti` deny-list key `preview:share:revoked:` (revoke one link); +//! * a per-site epoch `preview:share:epoch` = unix-seconds (revoke *all* links +//! issued before that instant — a token whose `iat` is below it is refused). +//! +//! On teardown, removing the override file and the checkout already stops the gate +//! on **this** node. But the per-vhost KV is gossip-replicated in a cluster, and a +//! preview can be redeployed, so the contract has switchboard also bump the epoch: +//! `preview:share:epoch = now`. That kills every outstanding share link for the +//! preview at once, cluster-wide, without enumerating `jti`s. +//! +//! # How the write is scoped to the tenant +//! +//! ePHPm's RESP listener scopes a connection to one vhost's KV store by its +//! **AUTH username**: `AUTH ` selects that site's dedicated store, +//! and subsequent commands use **bare** keys (the `\x1f\x1f` gossip envelope +//! is internal to replication, never on the client wire). The password is +//! `HMAC-SHA256(kv_secret, site)` hex — [`crate::preview_auth::derive_site_kv_password`]. +//! The `` must be byte-identical to the token's `site` claim (the canonical +//! site key), which is exactly the vhost directory name teardown already has. +//! +//! # Best-effort, never fatal +//! +//! The write is best-effort: teardown's primary revocation is removing the gate +//! and the tree, and a teardown that *fails* because ePHPm's KV port was +//! unreachable would strand the preview's on-disk artifacts (the opposite of what +//! teardown is for). A failed bump is a `warn!`, not a teardown failure. It is +//! also skipped entirely, with a log line, when the operator has not told +//! switchboard the KV secret (`--kv-secret-file`). + +use std::time::Duration; + +use anyhow::Context; +use tokio::io::{AsyncBufReadExt as _, AsyncWriteExt as _, BufReader}; +use tokio::net::TcpStream; + +use crate::preview_auth::derive_site_kv_password; + +/// The per-site epoch key the gate compares a share token's `iat` against. +/// +/// (The gate also honours a per-`jti` deny-list key `preview:share:revoked:` +/// for revoking one link, but switchboard's daemon has no trigger for that today — +/// teardown revokes *all* links via the epoch — so this client writes only the +/// epoch. A per-link revoke path would add its own writer alongside its trigger.) +const EPOCH_KEY: &str = "preview:share:epoch"; + +/// Ceiling on the whole connect + AUTH + SET round trip. Teardown must not hang +/// on an unreachable or wedged KV port; a slow bump is dropped, loudly. +const OP_TIMEOUT: Duration = Duration::from_secs(5); + +/// Where and how to reach ePHPm's per-site KV RESP listener. +/// +/// `addr` is the listener (`[kv.redis_compat] listen`, default `127.0.0.1:6379`); +/// `kv_secret` is ePHPm's `[kv] secret`, which switchboard must be told +/// (`--kv-secret-file`) to derive per-site passwords. When switchboard has no KV +/// secret there is no [`KvRevoker`] and revocation writes are skipped. +#[derive(Debug, Clone)] +pub struct KvRevoker { + addr: String, + kv_secret: String, +} + +impl KvRevoker { + /// Build a revoker for a listener address and ePHPm's `[kv] secret`. + #[must_use] + pub fn new(addr: impl Into, kv_secret: impl Into) -> Self { + Self { + addr: addr.into(), + kv_secret: kv_secret.into(), + } + } + + /// Revoke **all** outstanding share links for `site` by setting its epoch to + /// `now_unix` — every share token issued before that instant is refused. + /// + /// # Errors + /// + /// Returns an error if the connection, AUTH, or SET fails (the caller treats + /// this as best-effort and only warns). + pub async fn bump_share_epoch(&self, site: &str, now_unix: u64) -> anyhow::Result<()> { + self.set_site_key(site, EPOCH_KEY, &now_unix.to_string()) + .await + } + + /// `AUTH ` then `SET ` against the site's own + /// KV store, under one short-lived connection with an overall timeout. + async fn set_site_key(&self, site: &str, key: &str, value: &str) -> anyhow::Result<()> { + let password = derive_site_kv_password(&self.kv_secret, site); + tokio::time::timeout( + OP_TIMEOUT, + self.set_site_key_inner(site, &password, key, value), + ) + .await + .with_context(|| format!("KV write to {} timed out after {OP_TIMEOUT:?}", self.addr))? + } + + async fn set_site_key_inner( + &self, + site: &str, + password: &str, + key: &str, + value: &str, + ) -> anyhow::Result<()> { + let stream = TcpStream::connect(&self.addr) + .await + .with_context(|| format!("cannot connect to ePHPm KV listener at {}", self.addr))?; + let (read_half, mut write_half) = stream.into_split(); + let mut reader = BufReader::new(read_half); + + // AUTH scopes the connection to this vhost's KV store. On the client wire + // the key is bare — the per-site envelope is internal to gossip. + write_half + .write_all(&encode_command(&["AUTH", site, password])) + .await + .context("failed to send KV AUTH")?; + read_reply(&mut reader) + .await + .context("KV AUTH was rejected")?; + + write_half + .write_all(&encode_command(&["SET", key, value])) + .await + .context("failed to send KV SET")?; + read_reply(&mut reader).await.context("KV SET failed")?; + + // A courtesy QUIT so the server closes cleanly; ignore its result. + let _ = write_half.write_all(&encode_command(&["QUIT"])).await; + Ok(()) + } +} + +/// Encode a command as a RESP2 array of bulk strings — the dialect ePHPm's KV +/// server parses (`*\r\n` then `$\r\n\r\n` per argument). +fn encode_command(args: &[&str]) -> Vec { + let mut out = format!("*{}\r\n", args.len()).into_bytes(); + for arg in args { + out.extend_from_slice(format!("${}\r\n", arg.len()).as_bytes()); + out.extend_from_slice(arg.as_bytes()); + out.extend_from_slice(b"\r\n"); + } + out +} + +/// Read one RESP reply line and classify it. A `-` prefix is an error reply +/// (returned as `Err`); anything else (`+OK`, `:1`, `$…`) is success. Replies to +/// AUTH and SET are single simple-string/error lines, so one line is enough. +async fn read_reply(reader: &mut R) -> anyhow::Result<()> +where + R: tokio::io::AsyncBufRead + Unpin, +{ + let mut line = Vec::new(); + let n = reader + .read_until(b'\n', &mut line) + .await + .context("failed to read KV reply")?; + anyhow::ensure!(n > 0, "ePHPm KV closed the connection without replying"); + let text = String::from_utf8_lossy(&line); + let text = text.trim_end_matches(['\r', '\n']); + if let Some(err) = text.strip_prefix('-') { + anyhow::bail!("ePHPm KV returned an error: {err}"); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use tokio::io::AsyncReadExt as _; + use tokio::net::TcpListener; + + #[test] + fn resp_command_encoding_is_an_array_of_bulk_strings() { + let bytes = encode_command(&["SET", "k", "v"]); + assert_eq!(bytes, b"*3\r\n$3\r\nSET\r\n$1\r\nk\r\n$1\r\nv\r\n"); + } + + #[tokio::test] + async fn an_error_reply_is_surfaced() { + let mut reply: &[u8] = b"-ERR bad auth\r\n"; + let mut reader = BufReader::new(&mut reply); + let err = read_reply(&mut reader).await.unwrap_err(); + assert!(err.to_string().contains("bad auth"), "{err}"); + } + + #[tokio::test] + async fn a_simple_ok_reply_is_success() { + let mut reply: &[u8] = b"+OK\r\n"; + let mut reader = BufReader::new(&mut reply); + read_reply(&mut reader).await.unwrap(); + } + + /// **The end-to-end revocation write.** Stand up a minimal RESP server that + /// records the frames it receives and replies `+OK`, point a [`KvRevoker`] at + /// it, and assert the AUTH is scoped to the site with the derived password and + /// the SET writes `preview:share:epoch = `. + #[tokio::test] + async fn bump_share_epoch_auths_for_the_site_and_sets_the_epoch() { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap().to_string(); + + let server = tokio::spawn(async move { + let (mut sock, _) = listener.accept().await.unwrap(); + // Reply +OK to AUTH and +OK to SET up front — the client reads them in + // order. Then read to EOF (the client sends AUTH, SET, QUIT and closes), + // so we capture every frame, not just whatever arrived first. + sock.write_all(b"+OK\r\n+OK\r\n").await.unwrap(); + let mut buf = Vec::new(); + sock.read_to_end(&mut buf).await.unwrap(); + String::from_utf8_lossy(&buf).into_owned() + }); + + let revoker = KvRevoker::new(addr, "master-secret"); + revoker + .bump_share_epoch("app-pr-1", 1_725_000_000) + .await + .unwrap(); + + let received = server.await.unwrap(); + let expected_pw = derive_site_kv_password("master-secret", "app-pr-1"); + assert!(received.contains("AUTH"), "must authenticate: {received:?}"); + assert!( + received.contains("app-pr-1"), + "AUTH must name the site: {received:?}" + ); + assert!( + received.contains(&expected_pw), + "AUTH must use the derived per-site password" + ); + assert!(received.contains("SET"), "must issue a SET: {received:?}"); + assert!( + received.contains("preview:share:epoch"), + "must set the epoch key: {received:?}" + ); + assert!( + received.contains("1725000000"), + "epoch value must be the unix time: {received:?}" + ); + } + + #[tokio::test] + async fn a_rejected_auth_is_an_error() { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let addr = listener.local_addr().unwrap().to_string(); + tokio::spawn(async move { + let (mut sock, _) = listener.accept().await.unwrap(); + sock.write_all(b"-WRONGPASS invalid\r\n").await.unwrap(); + }); + let revoker = KvRevoker::new(addr, "master-secret"); + let err = revoker.bump_share_epoch("app-pr-1", 1).await.unwrap_err(); + assert!(err.to_string().contains("AUTH"), "{err}"); + } +} diff --git a/src/main.rs b/src/main.rs index ee1c570..86d1e7f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -22,7 +22,9 @@ mod deployer; mod drain; mod github; mod job; +mod kv; mod manifest; +mod preview_auth; mod queue; mod secrets; mod site_key; @@ -55,6 +57,10 @@ use validate::Verdict; struct AppState { config: Config, secrets: Secrets, + /// ePHPm's `[kv] secret`, resolved once at startup from `--kv-secret-file`, + /// for deriving per-site RESP passwords when bumping the share-link + /// revocation epoch on teardown. `None` disables KV revocation. + kv_secret: Option, } #[tokio::main] @@ -150,6 +156,24 @@ async fn main() -> anyhow::Result<()> { // Load switchboard's own secret store (file + SWITCHBOARD_SECRET_* env). let secrets = Secrets::load(config.secrets_file.as_deref())?; + // Resolve ePHPm's [kv] secret once, for share-link revocation on teardown. + // Absent is fine (KV revocation is then skipped); a configured-but-unreadable + // file fails startup rather than silently disabling revocation. + let kv_secret = config.kv_secret()?; + match (&kv_secret, config.gate_public_previews) { + (Some(_), _) => info!("share-link revocation enabled (KV secret configured)"), + (None, _) => info!( + "share-link revocation via KV is not configured (--kv-secret-file unset); \ + teardown still revokes by removing the override and checkout" + ), + } + info!( + gate_public = config.gate_public_previews, + share_link = config.share_link, + secret_ref = %config.preview_session_secret_ref, + "preview access gate: private repos are always gated" + ); + // Build the drain kicker before anything else runs: a missing token file // should fail at startup, not silently warn every two seconds forever. let kicker = if config.drain_enabled() { @@ -179,7 +203,11 @@ async fn main() -> anyhow::Result<()> { let webhook_server_enabled = config.webhook_server_enabled; let listen = config.listen.clone(); - let state = Arc::new(AppState { config, secrets }); + let state = Arc::new(AppState { + config, + secrets, + kv_secret, + }); if let Some(kicker) = kicker { tokio::spawn(drain_loop(kicker, drain_interval)); @@ -445,6 +473,10 @@ async fn handle_deploy(state: &AppState, req: &PreviewRequest) -> anyhow::Result fetch_token: fetch_token.as_deref(), health_timeout: Duration::from_secs(state.config.health_timeout_secs), health_interval: Duration::from_secs(state.config.health_interval_secs), + gate_public_previews: state.config.gate_public_previews, + preview_session_secret_ref: &state.config.preview_session_secret_ref, + mint_share_link: state.config.share_link, + share_token_ttl: state.config.share_token_ttl(), }; let result = deployer::deploy_preview(req, &ctx).await?; @@ -476,6 +508,8 @@ async fn handle_teardown(state: &AppState, req: &PreviewRequest) -> anyhow::Resu vhost_temp_base: state.config.vhost_temp_base.as_deref(), state_dir: &state.config.state_dir, allow_incomplete: state.config.allow_incomplete_teardown, + kv_secret: state.kv_secret.as_deref(), + kv_addr: &state.config.kv_addr, }; // The same derivation the deploy used — teardown must remove the artifacts // that were actually created, which on a node without a diff --git a/src/preview_auth.rs b/src/preview_auth.rs new file mode 100644 index 0000000..91b7bc5 --- /dev/null +++ b/src/preview_auth.rs @@ -0,0 +1,417 @@ +//! The control-plane half of the preview access gate (ephpm#487/#491). +//! +//! ePHPm ships the *enforcement*: a per-site `preview-gate` middleware that turns +//! on when a resolved vhost's override file carries a `[preview_auth]` section, +//! redirects unauthenticated browsers to a GitHub-OAuth login, and — fail-closed +//! — takes the site out of service (503) rather than serving it ungated when the +//! section is present but unusable. It also *verifies and revokes* time-limited +//! shareable-URL capability tokens. What ePHPm deliberately does **not** do is +//! decide *which* previews to gate, *write* the section, or *mint* share links. +//! That is switchboard's job, and it is this module. +//! +//! The contract this implements is `site/content/roadmap/preview-access-gate.md` +//! in `ephpm/ephpm`. Three pieces live here: +//! +//! 1. **Gating policy** ([`should_gate`]) — a private repo's preview is *always* +//! gated; a public repo's is gated only when the operator asks +//! (`--gate-public-previews`). A private preview that comes up ungated is the +//! exact exposure the feature exists to prevent, so every path that could gate +//! silently-not-happen is turned into a hard deploy failure by the caller (see +//! [`crate::deployer`]). +//! +//! 2. **Session-secret resolution** ([`resolve_session_secret`]) — the override +//! file carries a *reference* (`env:NAME` / `file:/abs` / a literal), never the +//! key itself, because that file is tenant-adjacent. switchboard resolves the +//! same reference ePHPm's issuer and gate resolve, both to verify a gated +//! deploy *can* come up (fail closed if it can't) and to obtain the bytes it +//! signs share tokens with. The resolution rules and the ≥ 32-byte floor +//! mirror ePHPm's `site_overrides::resolve_secret` exactly. +//! +//! 3. **Share-token minting** ([`mint_share_token`], [`generate_jti`]) — the +//! wire-compatible mirror of `ephpm_middleware_builtins::preview_gate:: +//! mint_share_token`. A `via:"share"` HS256 capability, per-preview (`site` +//! claim), short-lived (`exp`), individually revocable (`jti`) and +//! epoch-revocable (`iat`). switchboard mints because it already holds the +//! shared secret; the token is a bearer capability and must be treated as one. +//! +//! Plus [`derive_site_kv_password`], the per-site RESP credential switchboard +//! uses to write the revocation keys on teardown (see [`crate::kv`]). + +use anyhow::Context; +use base64::Engine as _; +use base64::engine::general_purpose::URL_SAFE_NO_PAD; +use hmac::{Hmac, Mac}; +use sha2::Sha256; + +type HmacSha256 = Hmac; + +/// The issuer's default login endpoint, under the router-carved-out +/// `/_ephpm/auth/` namespace. Written verbatim into `[preview_auth] login_url`. +pub const DEFAULT_LOGIN_URL: &str = "/_ephpm/auth/github/login"; + +/// The default query parameter the gate reads a share token from +/// (`?ephpm_share=`). ePHPm's `preview-gate` `share_param` default; we +/// mirror it so an unconfigured override and switchboard agree on the link shape. +pub const DEFAULT_SHARE_PARAM: &str = "ephpm_share"; + +/// Minimum resolved session-secret length, in bytes. +/// +/// ePHPm's `site_overrides.rs` (`MIN_SESSION_SECRET`) takes a gated preview out +/// of service (503) when the resolved secret is shorter than this. switchboard +/// enforces the same floor *before* writing a gated override, so a too-short key +/// fails the deploy loudly rather than shipping a preview ePHPm will 503. +pub const MIN_SESSION_SECRET_LEN: usize = 32; + +/// Whether this preview must be gated. +/// +/// The whole policy in one line: a **private** repo's preview is always gated — +/// its code is not world-readable, so its preview must not be either. A +/// **public** repo's preview is ungated by default (the code is already public) +/// but the operator can gate everything with `--gate-public-previews`, e.g. to +/// keep unreleased work-in-progress off the open internet. +#[must_use] +pub fn should_gate(repo_is_private: bool, gate_public_previews: bool) -> bool { + repo_is_private || gate_public_previews +} + +/// Resolve a `session_secret` **reference** to the raw key bytes, applying +/// ePHPm's own rules so the two processes derive byte-identical secrets. +/// +/// Accepted forms (mirroring `site_overrides::resolve_secret`): +/// * `env:NAME` — read environment variable `NAME`; +/// * `file:/abs/path` — read the file; +/// * anything else — a literal (discouraged; the reference should point at a +/// secret, not be one, since the same string is written into the tenant-adjacent +/// override file). +/// +/// The resolved value is trimmed and must be **non-empty** and at least +/// [`MIN_SESSION_SECRET_LEN`] bytes — the same floor the ePHPm gate enforces. The +/// returned bytes are exactly what ePHPm signs/verifies with, so a share token +/// switchboard mints with them verifies in the gate. +/// +/// # Errors +/// +/// Returns an error when an `env:`/`file:` reference cannot be resolved, when the +/// resolved value is empty, or when it is shorter than the floor. A gated deploy +/// treats any of these as fatal (fail closed) rather than shipping an open +/// preview. +pub fn resolve_session_secret(reference: &str) -> anyhow::Result> { + let reference = reference.trim(); + anyhow::ensure!( + !reference.is_empty(), + "preview-auth session secret reference is empty — set --preview-session-secret-ref \ + (e.g. env:EPHPM_PREVIEW_SESSION_SECRET)" + ); + + let resolved = if let Some(name) = reference.strip_prefix("env:") { + let name = name.trim(); + anyhow::ensure!( + !name.is_empty(), + "session secret reference {reference:?}: env var name is empty" + ); + std::env::var(name).with_context(|| { + format!( + "session secret reference {reference:?}: environment variable {name} is not set" + ) + })? + } else if let Some(path) = reference.strip_prefix("file:") { + let path = path.trim(); + anyhow::ensure!( + !path.is_empty(), + "session secret reference {reference:?}: file path is empty" + ); + std::fs::read_to_string(path).with_context(|| { + format!("session secret reference {reference:?}: cannot read {path}") + })? + } else { + // A literal secret. ePHPm accepts this (discouraged); we do too, so the + // resolution rule matches exactly, but the reference SHOULD be an env:/file:. + reference.to_string() + }; + + let resolved = resolved.trim(); + anyhow::ensure!( + !resolved.is_empty(), + "preview-auth session secret resolved from {reference:?} is empty" + ); + anyhow::ensure!( + resolved.len() >= MIN_SESSION_SECRET_LEN, + "preview-auth session secret resolved from {reference:?} is {} bytes; ePHPm requires \ + at least {MIN_SESSION_SECRET_LEN} bytes and would take the preview out of service (503)", + resolved.len() + ); + Ok(resolved.as_bytes().to_vec()) +} + +/// Mint a `via:"share"` capability token — the wire-compatible mirror of +/// `ephpm_middleware_builtins::preview_gate::mint_share_token`. +/// +/// The token is a stateless HS256 JWT with exactly five claims, in the shape the +/// ePHPm gate verifies through the *same* `Hs256Policy` an OAuth session uses +/// (there is deliberately no second verifier — issue #396): +/// +/// * `site` — the preview's **canonical site key**, so the link opens exactly one +/// preview (the gate checks it as `expected_site`); +/// * `via` — the literal `"share"`, which is what switches on the extra +/// revocation checks (a normal OAuth session pays nothing); +/// * `jti` — a unique id so one link can be revoked (`preview:share:revoked:`); +/// * `iat` — issue time, which the per-site epoch (`preview:share:epoch`) is +/// compared against for revoke-all; +/// * `exp` — expiry; the gate enforces only that it exists and is in the future, +/// so the minter keeps it short and a leaked link self-heals. +/// +/// `secret` must be the bytes [`resolve_session_secret`] returned — the same key +/// the gate resolved — or the token will not verify. The signature is +/// `HMAC-SHA256(secret, base64url(header) + "." + base64url(payload))`, all +/// base64url-**unpadded**, header the fixed bytes `{"alg":"HS256","typ":"JWT"}`. +#[must_use] +pub fn mint_share_token(secret: &[u8], site: &str, jti: &str, iat: u64, exp: u64) -> String { + let claims = serde_json::json!({ + "site": site, + "via": "share", + "jti": jti, + "iat": iat, + "exp": exp, + }); + // The header is a fixed literal on the ePHPm side (not re-serialised from a + // struct), so reproduce those exact bytes. + let header_b64 = URL_SAFE_NO_PAD.encode(br#"{"alg":"HS256","typ":"JWT"}"#); + let payload_b64 = + URL_SAFE_NO_PAD.encode(serde_json::to_vec(&claims).expect("share claims serialise")); + + let mut mac = HmacSha256::new_from_slice(secret).expect("HMAC accepts any key length"); + mac.update(header_b64.as_bytes()); + mac.update(b"."); + mac.update(payload_b64.as_bytes()); + let sig = URL_SAFE_NO_PAD.encode(mac.finalize().into_bytes()); + + format!("{header_b64}.{payload_b64}.{sig}") +} + +/// A fresh, unguessable `jti` for a share token: 16 CSPRNG bytes, lowercase hex. +/// +/// Uniqueness is what `jti` is for — it lets one link be revoked without touching +/// the others. Unpredictability is a bonus (the HMAC already makes the token +/// unforgeable), but drawing from the OS CSPRNG costs nothing and means a `jti` +/// can never collide or be pre-computed. +#[must_use] +pub fn generate_jti() -> String { + let mut bytes = [0u8; 16]; + getrandom::fill(&mut bytes).expect("OS CSPRNG is available"); + hex::encode(bytes) +} + +/// The full shareable URL to hand out: `/?=`. +/// +/// The token travels in the query string exactly as the gate reads it; the +/// signing secret never appears — only the (bearer) token does. +#[must_use] +pub fn share_url(preview_url: &str, share_param: &str, token: &str) -> String { + format!( + "{}/?{share_param}={token}", + preview_url.trim_end_matches('/') + ) +} + +/// Derive the per-site KV RESP password for `site`, mirroring +/// `ephpm_kv::auth::derive_site_password`: lowercase-hex +/// `HMAC-SHA256(key = kv_secret, msg = site)`. +/// +/// This is the password switchboard authenticates the revocation writes with +/// (`AUTH `), which scopes the RESP connection to that vhost's +/// own KV keyspace — the same store the gate reads the revocation keys from. +/// `kv_secret` must be ePHPm's `[kv] secret` (the operator-set, deterministic +/// value), or the derived password will not match. +#[must_use] +pub fn derive_site_kv_password(kv_secret: &str, site: &str) -> String { + let mut mac = + HmacSha256::new_from_slice(kv_secret.as_bytes()).expect("HMAC accepts any key length"); + mac.update(site.as_bytes()); + hex::encode(mac.finalize().into_bytes()) +} + +#[cfg(test)] +mod tests { + use super::*; + + // ── gating policy ─────────────────────────────────────────────────── + + #[test] + fn private_repos_are_always_gated() { + assert!( + should_gate(true, false), + "a private repo must be gated by default" + ); + assert!( + should_gate(true, true), + "a private repo is gated regardless of the public knob" + ); + } + + #[test] + fn public_repos_are_ungated_unless_the_operator_opts_in() { + assert!( + !should_gate(false, false), + "a public repo is ungated by default" + ); + assert!( + should_gate(false, true), + "--gate-public-previews gates public repos too" + ); + } + + // ── session-secret resolution ───────────────────────────────────────── + + #[test] + fn env_reference_resolves_and_enforces_the_floor() { + // A unique var name so parallel tests don't collide on the environment. + let name = "EPHPM_TEST_PREVIEW_SECRET_ENV_OK"; + // SAFETY: single-threaded within this test's scope; the name is unique. + unsafe { std::env::set_var(name, "0123456789abcdef0123456789abcdef") }; + let bytes = resolve_session_secret(&format!("env:{name}")).unwrap(); + assert_eq!(bytes.len(), 32); + unsafe { std::env::remove_var(name) }; + } + + #[test] + fn a_short_secret_is_refused_so_the_deploy_fails_closed() { + let name = "EPHPM_TEST_PREVIEW_SECRET_ENV_SHORT"; + unsafe { std::env::set_var(name, "too-short") }; + let err = resolve_session_secret(&format!("env:{name}")) + .expect_err("a secret below the 32-byte floor must be refused"); + assert!(err.to_string().contains("at least"), "{err}"); + unsafe { std::env::remove_var(name) }; + } + + #[test] + fn an_unset_env_reference_is_an_error_not_an_empty_secret() { + let err = resolve_session_secret("env:EPHPM_TEST_DEFINITELY_UNSET_VAR_XYZ") + .expect_err("an unresolvable reference must fail, never resolve to empty"); + assert!(err.to_string().contains("is not set"), "{err}"); + } + + #[test] + fn file_reference_is_read_and_trimmed() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("secret"); + // Trailing newline is trimmed, exactly as ePHPm trims it. + std::fs::write(&path, "0123456789abcdef0123456789abcdef\n").unwrap(); + let bytes = resolve_session_secret(&format!("file:{}", path.display())).unwrap(); + assert_eq!(bytes, b"0123456789abcdef0123456789abcdef"); + } + + #[test] + fn empty_reference_is_refused() { + assert!(resolve_session_secret(" ").is_err()); + } + + // ── share-token minting: wire compatibility ─────────────────────────── + + /// The token must be a three-segment JWT whose header decodes to the exact + /// bytes ePHPm's verifier expects (`alg:HS256`) and whose payload carries the + /// five claims with the right values. This is the shape check the contract's + /// test asks for. + #[test] + fn minted_token_has_the_via_share_shape_for_the_site() { + let secret = b"0123456789abcdef0123456789abcdef"; + let token = mint_share_token(secret, "app-pr-1", "deadbeef", 1000, 2000); + + let parts: Vec<&str> = token.split('.').collect(); + assert_eq!(parts.len(), 3, "a JWT is header.payload.signature"); + + let header = URL_SAFE_NO_PAD.decode(parts[0]).unwrap(); + assert_eq!(header, br#"{"alg":"HS256","typ":"JWT"}"#); + + let payload = URL_SAFE_NO_PAD.decode(parts[1]).unwrap(); + let claims: serde_json::Value = serde_json::from_slice(&payload).unwrap(); + assert_eq!(claims["site"], "app-pr-1", "per-preview binding"); + assert_eq!( + claims["via"], "share", + "distinguishes a share link from an OAuth session" + ); + assert_eq!(claims["jti"], "deadbeef"); + assert_eq!(claims["iat"], 1000); + assert_eq!(claims["exp"], 2000); + } + + /// The signature must be the HMAC the gate recomputes: verify it the way + /// `Hs256Policy::verify` does — HMAC over `header_b64.payload_b64`. + #[test] + fn minted_token_signature_verifies_against_the_secret() { + let secret = b"0123456789abcdef0123456789abcdef"; + let token = mint_share_token(secret, "site", "jti1", 1, 999_999_999); + let (signed, sig_b64) = token.rsplit_once('.').unwrap(); + + let mut mac = HmacSha256::new_from_slice(secret).unwrap(); + mac.update(signed.as_bytes()); + let expected = URL_SAFE_NO_PAD.encode(mac.finalize().into_bytes()); + assert_eq!( + sig_b64, expected, + "the signature must be HMAC-SHA256 over header.payload" + ); + + // A different secret must NOT produce the same signature. + let other = mint_share_token( + b"ffffffffffffffffffffffffffffffff", + "site", + "jti1", + 1, + 999_999_999, + ); + assert_ne!(other, token, "a different key must yield a different token"); + } + + #[test] + fn jti_is_unique_and_hex() { + let a = generate_jti(); + let b = generate_jti(); + assert_eq!(a.len(), 32, "16 bytes hex-encoded"); + assert!(a.chars().all(|c| c.is_ascii_hexdigit())); + assert_ne!(a, b, "two draws must differ"); + } + + #[test] + fn share_url_carries_only_the_token() { + let url = share_url( + "https://pr-1.app.preview.ephpm.dev", + DEFAULT_SHARE_PARAM, + "tok.en.sig", + ); + assert_eq!( + url, + "https://pr-1.app.preview.ephpm.dev/?ephpm_share=tok.en.sig" + ); + // A URL with a port and a trailing slash normalises the same way. + let url = share_url("https://h:8084/", "ephpm_share", "t"); + assert_eq!(url, "https://h:8084/?ephpm_share=t"); + } + + // ── KV password derivation ──────────────────────────────────────────── + + /// Must match `ephpm_kv::auth::derive_site_password`: lowercase-hex + /// HMAC-SHA256(key=secret, msg=site). Pinned against an independently + /// computed value so a refactor that changes the derivation is caught. + #[test] + fn kv_password_is_hmac_sha256_hex_of_the_site() { + let derived = derive_site_kv_password("master-secret", "app-pr-1"); + // Recompute the reference the same way and compare — 64 lowercase hex chars. + let mut mac = HmacSha256::new_from_slice(b"master-secret").unwrap(); + mac.update(b"app-pr-1"); + assert_eq!(derived, hex::encode(mac.finalize().into_bytes())); + assert_eq!(derived.len(), 64); + assert!( + derived + .chars() + .all(|c| c.is_ascii_hexdigit() && !c.is_ascii_uppercase()) + ); + } + + #[test] + fn kv_password_is_site_scoped() { + assert_ne!( + derive_site_kv_password("s", "app-pr-1"), + derive_site_kv_password("s", "app-pr-2"), + "each site must get a distinct password" + ); + } +} diff --git a/src/site_override.rs b/src/site_override.rs index a84e3b3..d12e59d 100644 --- a/src/site_override.rs +++ b/src/site_override.rs @@ -171,6 +171,94 @@ impl PrependFile { } } +/// Whether `s` can be written between the quotes of a hand-rolled TOML basic +/// string without changing the document's structure. +/// +/// The override is written by hand (no `toml` serializer), so a `"` or a newline +/// in a value would close the string and let it inject keys — the same class the +/// `docroot`/`auto_prepend_file` charset gate closes. `preview_auth`'s values are +/// operator-supplied (switchboard's own config), not tenant-supplied, so the risk +/// is lower, but a hand-written writer that trusts its input is exactly how the +/// bug recurs. A backslash is refused too: it is TOML's escape lead-in, and none +/// of these values (an `env:`/`file:` reference, an absolute URL path) needs one. +fn is_toml_string_safe(s: &str) -> bool { + !s.is_empty() && s.chars().all(|c| c != '"' && c != '\\' && !c.is_control()) +} + +/// The `[preview_auth]` section that turns the per-site OAuth/​share gate ON for +/// one preview (ephpm#487/#491). +/// +/// It carries deliberately little: a **reference** to the shared HS256 session +/// secret (`env:NAME` / `file:/abs` / a literal — never the key itself, since this +/// file is derived from tenant-controlled repository content) and the issuer's +/// login entry point. Everything else (cookie name, `require_https`/`require_site`, +/// `share_param`, revocation) uses the gate's defaults, which match what the +/// global `github-auth` issuer mount uses. See +/// [`crate::preview_auth`] for the policy that decides *when* to write this and +/// for the secret resolution/​minting that pairs with it. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PreviewAuthSection { + /// The `session_secret` reference written verbatim into the file — the SAME + /// reference the issuer resolves, so the two share one source of truth. Never + /// the resolved secret. + session_secret_ref: String, + /// The issuer's login endpoint (`login_url`), an absolute path under + /// `/_ephpm/auth/`. + login_url: String, +} + +impl PreviewAuthSection { + /// Build a validated section from a secret **reference** and a login URL. + /// + /// # Errors + /// + /// Returns an error when the reference is empty or the login URL is not an + /// absolute (`/`-leading) path, or when either would be unsafe to write into + /// the hand-rolled TOML (a quote, backslash, or control character). + pub fn new( + session_secret_ref: impl Into, + login_url: impl Into, + ) -> anyhow::Result { + let session_secret_ref = session_secret_ref.into(); + let login_url = login_url.into(); + let secret_ref = session_secret_ref.trim(); + anyhow::ensure!( + !secret_ref.is_empty(), + "preview_auth session_secret reference is empty" + ); + anyhow::ensure!( + is_toml_string_safe(secret_ref), + "preview_auth session_secret reference {secret_ref:?} contains a quote, backslash \ + or control character and cannot be safely written into the override TOML" + ); + let login = login_url.trim(); + anyhow::ensure!( + login.starts_with('/'), + "preview_auth login_url {login:?} must be an absolute path beginning with `/`" + ); + anyhow::ensure!( + is_toml_string_safe(login), + "preview_auth login_url {login:?} contains a quote, backslash or control character" + ); + Ok(Self { + session_secret_ref: secret_ref.to_string(), + login_url: login.to_string(), + }) + } + + /// The secret reference as it appears in the file (never the resolved key). + #[must_use] + pub fn session_secret_ref(&self) -> &str { + &self.session_secret_ref + } + + /// The issuer's login endpoint as it appears in the file. + #[must_use] + pub fn login_url(&self) -> &str { + &self.login_url + } +} + /// Everything switchboard declares for one site, in one file. /// /// One struct rather than two writers because ePHPm reads **one** file per site: @@ -184,6 +272,13 @@ pub struct SiteOverride { /// The PHP file ePHPm runs before every request for this site, or `None` /// when this deploy has nothing to prepend. pub auto_prepend_file: Option, + /// The `[preview_auth]` gate section, or `None` for an ungated preview. + /// + /// `Some` turns the per-site OAuth/​share gate on. Writing it for a private + /// preview is the whole point of the access gate; a private preview whose + /// override lacks this section comes up world-readable, which the deployer + /// refuses to let happen silently (fail closed). + pub preview_auth: Option, } /// Validate a manifest's `docroot:` against the checkout it describes. @@ -318,10 +413,13 @@ fn validate_contained( /// Render the override file's contents for a validated declaration. /// -/// Two keys, both of which ePHPm implements as typed fields since #472 — no -/// forward-looking keys. `document_root` is emitted only when it *narrows* the -/// web root: ePHPm reads an absent key and an explicit `"."` identically, and -/// the absent spelling is the one every ePHPm ever shipped agrees on. +/// The two top-level keys (`document_root`, `auto_prepend_file`) are ePHPm typed +/// fields since #472; the optional `[preview_auth]` table is the access-gate +/// activation added in ephpm#487/#491. `document_root` is emitted only when it +/// *narrows* the web root: ePHPm reads an absent key and an explicit `"."` +/// identically, and the absent spelling is the one every ePHPm ever shipped +/// agrees on. The `[preview_auth]` table is emitted **last** — a TOML table must +/// follow all top-level keys, or those keys would parse as belonging to it. #[must_use] pub fn render_override(over: &SiteOverride) -> String { let mut out = String::from( @@ -337,6 +435,16 @@ pub fn render_override(over: &SiteOverride) -> String { if let Some(prepend) = &over.auto_prepend_file { out.push_str(&format!("auto_prepend_file = \"{}\"\n", prepend.declared())); } + if let Some(auth) = &over.preview_auth { + // The secret is a REFERENCE (env:/file:), never the key — this file is + // derived from tenant-controlled repository content, so nothing sensitive + // is written into it. Both strings are validated by `PreviewAuthSection`. + out.push_str(&format!( + "\n[preview_auth]\nsession_secret = \"{}\"\nlogin_url = \"{}\"\n", + auth.session_secret_ref(), + auth.login_url(), + )); + } out } @@ -793,9 +901,114 @@ mod tests { SiteOverride { document_root, auto_prepend_file, + preview_auth: None, } } + // ── the [preview_auth] gate section (ephpm#487/#491) ──────────────── + + #[test] + fn preview_auth_section_renders_after_the_top_level_keys() { + let c = laravel_checkout(); + let root = validate_docroot(&c.root, "public").unwrap(); + write_prepend(&c.root, ".ephpm-preview-prepend.php"); + let prepend = validate_prepend(&c.root, ".ephpm-preview-prepend.php").unwrap(); + let auth = PreviewAuthSection::new( + "env:EPHPM_PREVIEW_SESSION_SECRET", + "/_ephpm/auth/github/login", + ) + .unwrap(); + + let over = SiteOverride { + document_root: root, + auto_prepend_file: Some(prepend), + preview_auth: Some(auth), + }; + let text = render_override(&over); + + // The reference is written, not a literal secret. + assert!( + text.contains("session_secret = \"env:EPHPM_PREVIEW_SESSION_SECRET\""), + "the file must carry the reference, never the key: {text}" + ); + assert!( + text.contains("login_url = \"/_ephpm/auth/github/login\""), + "{text}" + ); + + // The table header must come after the two top-level keys, or TOML would + // read `document_root`/`auto_prepend_file` as members of the table. + let table_at = text.find("[preview_auth]").expect("the table is present"); + assert!( + text.find("document_root").unwrap() < table_at + && text.find("auto_prepend_file").unwrap() < table_at, + "top-level keys must precede the table: {text}" + ); + } + + /// A `docroot: "."` private preview writes no `document_root` but still gates: + /// the section is what makes the preview private, and it must be present. + #[test] + fn preview_auth_can_gate_a_container_docroot_preview() { + let auth = PreviewAuthSection::new( + "env:EPHPM_PREVIEW_SESSION_SECRET", + "/_ephpm/auth/github/login", + ) + .unwrap(); + let over = SiteOverride { + document_root: DocumentRoot::Container, + auto_prepend_file: None, + preview_auth: Some(auth), + }; + let text = render_override(&over); + assert!(!text.contains("document_root")); + assert!(text.contains("[preview_auth]"), "{text}"); + } + + #[test] + fn ungated_preview_writes_no_preview_auth_section() { + let c = laravel_checkout(); + let root = validate_docroot(&c.root, "public").unwrap(); + let text = render_override(&over(root, None)); + assert!( + !text.contains("preview_auth"), + "a public/ungated preview must not carry the section: {text}" + ); + } + + #[test] + fn preview_auth_refuses_a_toml_injecting_reference() { + // A quote or newline in the reference would close the TOML string. + for bad in [ + "env:X\"\nsomething_evil = \"y", + "env:X\ndocument_root = \"..", + "env:X\\bad", + ] { + assert!( + PreviewAuthSection::new(bad, "/_ephpm/auth/github/login").is_err(), + "{bad:?} must be refused" + ); + } + } + + #[test] + fn preview_auth_login_url_must_be_absolute() { + assert!( + PreviewAuthSection::new("env:SECRET", "login").is_err(), + "a relative login_url must be refused" + ); + assert!( + PreviewAuthSection::new("env:SECRET", "https://evil/login").is_err(), + "an absolute URL (not a path) is refused — login_url is a path under /_ephpm/auth" + ); + assert!(PreviewAuthSection::new("env:SECRET", "/_ephpm/auth/github/login").is_ok()); + } + + #[test] + fn preview_auth_refuses_an_empty_reference() { + assert!(PreviewAuthSection::new(" ", "/_ephpm/auth/github/login").is_err()); + } + /// The rendered file must be exactly what ePHPm's `site_overrides::load` /// parses, and must round-trip through a TOML parser as the two typed keys /// #472 declares. Rendering something ePHPm cannot parse is now a 503. diff --git a/src/teardown.rs b/src/teardown.rs index de7ab3e..ee55ba8 100644 --- a/src/teardown.rs +++ b/src/teardown.rs @@ -129,6 +129,16 @@ pub struct TeardownContext<'a> { /// node leaves some artifact classes behind. Turns the failure above into a /// `WARN` naming the same artifacts. pub allow_incomplete: bool, + + // ── share-link revocation (ephpm#487/#491) ───────────────────────── + /// ePHPm's `[kv] secret`, used to derive the per-site RESP password so the + /// share-link revocation epoch can be written into the preview's KV keyspace. + /// `None` skips KV revocation entirely (logged) — removing the override and + /// the checkout is already the primary revocation on this node. + pub kv_secret: Option<&'a str>, + /// ePHPm's KV RESP listener (`[kv.redis_compat] listen`, default + /// `127.0.0.1:6379`). Only consulted when `kv_secret` is set. + pub kv_addr: &'a str, } /// Remove a preview deployment and every per-site artifact it left behind. @@ -244,6 +254,18 @@ pub async fn teardown_preview( failures.push(format!("{e:#}")); } + // (6) Revoke every outstanding share link for this preview by bumping its + // per-site epoch (`preview:share:epoch` = now). Removing the override and the + // checkout above already stops the gate on THIS node, but the per-vhost KV is + // gossip-replicated and a preview can be redeployed, so the contract has the + // control plane bump the epoch too — it kills all links at once, cluster-wide. + // + // Best-effort by design: a failed KV write must NOT fail the teardown, or an + // unreachable KV port would strand the preview's on-disk artifacts (the + // opposite of teardown's job). It is skipped, with a line, when switchboard + // has not been told the KV secret. + revoke_share_links(ctx, site_key).await; + anyhow::ensure!( failures.is_empty(), "teardown of {site_key} left artifacts behind: {}", @@ -252,6 +274,44 @@ pub async fn teardown_preview( Ok(()) } +/// Revoke every outstanding share link for `site_key` by bumping its per-site +/// revocation epoch, best-effort. +/// +/// Skipped (with an info line) when `--kv-secret-file` is not configured: without +/// ePHPm's `[kv] secret` switchboard cannot derive the per-site RESP password, and +/// removing the override + checkout is already the primary revocation. A KV write +/// that fails (listener down, wrong secret) is a `warn!`, never a teardown +/// failure — see [`crate::kv`]. +async fn revoke_share_links(ctx: &TeardownContext<'_>, site_key: &str) { + let Some(kv_secret) = ctx.kv_secret else { + tracing::debug!( + %site_key, + "share-link revocation epoch not bumped: --kv-secret-file is not configured. \ + The override and checkout removal above already revoke share links on this node" + ); + return; + }; + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_or(0, |d| d.as_secs()); + let revoker = crate::kv::KvRevoker::new(ctx.kv_addr, kv_secret); + match revoker.bump_share_epoch(site_key, now).await { + Ok(()) => tracing::info!( + %site_key, + epoch = now, + "bumped the share-link revocation epoch — all outstanding share links for this \ + preview are now refused" + ), + Err(e) => tracing::warn!( + %site_key, + error = %format!("{e:#}"), + "could not bump the share-link revocation epoch (best-effort). The override and \ + checkout removal still revoke share links on this node; a leaked link on another \ + cluster node would expire on its own" + ), + } +} + /// Record an artifact class this node is not configured to remove. /// /// Unacknowledged it is a failure, so the teardown cannot report success while @@ -592,6 +652,8 @@ mod tests { vhost_temp_base: Some(&self.temp_base), state_dir: &self.state, allow_incomplete: false, + kv_secret: None, + kv_addr: "127.0.0.1:6379", } } @@ -797,6 +859,8 @@ mod tests { vhost_temp_base: Some(&f.temp_base), state_dir: &f.state, allow_incomplete: false, + kv_secret: None, + kv_addr: "127.0.0.1:6379", }; let err = teardown_preview(&preview(site_key), &ctx) .await @@ -844,6 +908,8 @@ mod tests { vhost_temp_base: Some(&f.temp_base), state_dir: &f.state, allow_incomplete: true, + kv_secret: None, + kv_addr: "127.0.0.1:6379", }; teardown_preview(&preview(site_key), &ctx).await.unwrap(); @@ -876,6 +942,8 @@ mod tests { vhost_temp_base: Some(&f.temp_base), state_dir: &f.state, allow_incomplete: false, + kv_secret: None, + kv_addr: "127.0.0.1:6379", }; let err = teardown_preview(&preview(site_key), &ctx) .await @@ -892,6 +960,91 @@ mod tests { ); } + // ── share-link revocation on teardown (ephpm#487/#491) ────────────── + + /// **Teardown bumps the revocation epoch.** With a KV secret configured, + /// teardown writes `preview:share:epoch = now` into the preview's own KV + /// keyspace (AUTH'd as the site), killing every outstanding share link. A + /// minimal mock RESP server captures the frames and asserts the shape. + #[tokio::test] + async fn teardown_bumps_the_share_revocation_epoch() { + use tokio::io::{AsyncReadExt as _, AsyncWriteExt as _}; + use tokio::net::TcpListener; + + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let kv_addr = listener.local_addr().unwrap().to_string(); + let server = tokio::spawn(async move { + let (mut sock, _) = listener.accept().await.unwrap(); + sock.write_all(b"+OK\r\n+OK\r\n").await.unwrap(); + let mut buf = Vec::new(); + sock.read_to_end(&mut buf).await.unwrap(); + String::from_utf8_lossy(&buf).into_owned() + }); + + let f = Fixture::new().await; + let site_key = "ephpm-my-blog-pr-7"; + f.deploy_artifacts(site_key).await; + + let ctx = TeardownContext { + sites_dir: &f.sites, + sqlite_dir: Some(&f.sqlite), + site_overrides_dir: Some(&f.overrides), + vhost_temp_base: Some(&f.temp_base), + state_dir: &f.state, + allow_incomplete: false, + kv_secret: Some("master-secret"), + kv_addr: &kv_addr, + }; + teardown_preview(&preview(site_key), &ctx).await.unwrap(); + + let received = server.await.unwrap(); + let expected_pw = crate::preview_auth::derive_site_kv_password("master-secret", site_key); + assert!( + received.contains("AUTH"), + "must AUTH for the site: {received:?}" + ); + assert!( + received.contains(site_key), + "AUTH scopes to the site's keyspace: {received:?}" + ); + assert!( + received.contains(&expected_pw), + "AUTH uses the derived per-site password" + ); + assert!( + received.contains("preview:share:epoch"), + "must set the epoch key: {received:?}" + ); + } + + /// A KV port that is down must NOT fail the teardown — revocation is + /// best-effort, and a hard failure would strand the on-disk artifacts. + #[tokio::test] + async fn an_unreachable_kv_does_not_fail_the_teardown() { + let f = Fixture::new().await; + let site_key = "ephpm-my-blog-pr-7"; + f.deploy_artifacts(site_key).await; + + let ctx = TeardownContext { + sites_dir: &f.sites, + sqlite_dir: Some(&f.sqlite), + site_overrides_dir: Some(&f.overrides), + vhost_temp_base: Some(&f.temp_base), + state_dir: &f.state, + allow_incomplete: false, + kv_secret: Some("master-secret"), + // A port nothing is listening on. + kv_addr: "127.0.0.1:1", + }; + teardown_preview(&preview(site_key), &ctx) + .await + .expect("an unreachable KV must not fail the teardown (best-effort revocation)"); + assert!( + !f.sites.join(site_key).exists(), + "the vhost dir is still removed" + ); + } + // ── issue #19: switchboard-api's applied/ marker ──────────────────── /// switchboard#24: the receipt for the teardown we just performed must diff --git a/src/webhook.rs b/src/webhook.rs index de52817..788c844 100644 --- a/src/webhook.rs +++ b/src/webhook.rs @@ -85,6 +85,17 @@ pub struct Repository { pub clone_url: String, pub name: String, pub owner: RepoOwner, + /// Whether the base repository is private. GitHub always sends this on a + /// `pull_request` webhook; defaulted to `true` (fail closed) for the same + /// reason as the job path — an absent value must not publish a private + /// preview open. + #[serde(default = "private_when_absent")] + pub private: bool, +} + +/// Serde default for [`Repository::private`]: absent ⇒ private (fail closed). +fn private_when_absent() -> bool { + true } #[derive(Debug, Deserialize)] @@ -123,6 +134,7 @@ impl PullRequestEvent { sha: self.pull_request.head.sha.clone(), installation_id: self.installation.as_ref().map(|i| i.id), fork: self.is_fork(), + private: self.repository.private, } } @@ -272,6 +284,7 @@ mod tests { owner: RepoOwner { login: "ephpm".into(), }, + private: false, }, installation: None, } @@ -295,6 +308,20 @@ mod tests { assert_eq!(req.pr_number, 42); } + #[test] + fn private_repo_flows_to_the_request() { + let mut event = event_with_head_repo(Some("ephpm/my-blog")); + assert!( + !event.to_preview_request().private, + "the sample repo is public" + ); + event.repository.private = true; + assert!( + event.to_preview_request().private, + "a private base repo must reach the deployer so its preview is gated" + ); + } + #[test] fn fork_detected_by_differing_head_repo() { // Same rules switchboard-api applies to compute `pull_request.fork`. @@ -385,6 +412,7 @@ mod tests { clone_url: "x".into(), name: "b".into(), owner: RepoOwner { login: "a".into() }, + private: false, }, installation: None, }; @@ -417,6 +445,7 @@ mod tests { clone_url: "x".into(), name: "b".into(), owner: RepoOwner { login: "a".into() }, + private: false, }, installation: None, };