From 1e6e5c7db1237f582d27a5f10edb62a901389c7e Mon Sep 17 00:00:00 2001 From: Luther Monson Date: Tue, 1 Sep 2026 22:27:14 -0700 Subject: [PATCH 1/3] Bind the gate to the canonical site key, not the Host header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ePHPm #448 takes the middleware ABI to minor 3, where `Request::vhost_id()` returns `Option<&str>` carrying the router's CANONICAL SITE KEY and `None` for a host that matched no virtual host. Before that it was the raw `Host` header: client-controlled, un-normalised, never absent. This module reads that value as a tenant identity in an authorization decision, so the change is semantic, not just a signature bump (ephpm#390, ephpm#449). The central change is that two things which used to be one string are now two, and each is used for exactly one job: * the SITE KEY selects the `sites` access check, binds the OAuth `state`, and becomes the session token's `site` claim. New `SiteIdentity` carries `Tenant(key)` vs `Untenanted` so those two uses can want different things from an absent tenant. * the REQUEST HOST (`req.http_host()`, ABI minor 2) builds the derived `redirect_uri` and nothing else. It has to: under a `sites_domain_suffix` the site key is the suffix-stripped directory name (`pr-1`), which is not an authority a browser can be redirected back to. `Config::check_for` now takes `Option<&str>` and fails closed on `None` when a `sites` table is configured — a request that matched none of the mapped vhosts is not one of them, and must not inherit the top-level target. With no `sites` table the default check still applies, which is the single-site deployment: a node with no virtual hosts has `vhost_id() == None` on every request, and treating that as "deny" would black-hole the whole site. An untenanted request's `site` claim and `state` binding are the constant `ephpm_middleware::UNMATCHED_VHOST`. It is uppercase and so unspellable as a site key, so it can never collide with a real tenant, and two different unrecognised hostnames cannot mint two identities for what is one and the same default document root. Deleted the local `normalize_vhost` re-normalisation on the request path: re-normalising a client string is a guess about what the router did, and the router now just says. What is left is `normalize_site_key`, which only tidies what an operator typed in `sites`, plus a new `validate_site_key` mirroring ePHPm's `is_valid_site_key` — so a `sites` table still written in request- hostname terms (a port, an IPv6 literal) FAILS THE MOUNT instead of silently never matching. `validate_vhost` becomes `validate_redirect_host`, guarding the one place a host still reaches an outbound URL. session-cookie: `site_param` now carries the canonical site key, and is omitted entirely when there is no tenant rather than filled in from the header. Its local `normalize_vhost` is deleted for the same reason. BREAKING (config): `sites` is keyed by the site key, not the request hostname — with `sites_domain_suffix = ".preview.example.com"` the key for `pr-1.preview.example.com` is `pr-1`. Sessions issued before the upgrade carry the old host-shaped `site` claim and name a tenant that no longer exists under that spelling; users log in again once. Both are documented in the README and the module docs. Also corrected two doc claims that the new pin falsifies: the KV surface is no longer process-global (ephpm#376), and the ABI does now expose a request scheme (minor 2) — so `require_https` is *implementable*, and is documented as not implemented rather than as impossible. And stated plainly, in both module docs and the README, that the verifier does not check the `site` claim this issuer writes: that is ephpm#396, still open, and minor 3 is what makes fixing it possible. Tests: the site key and the request host are separate fixtures throughout (`SITE`/`HOST`). New coverage for the sites table being keyed on the router's key and not the header, the untenanted deny, the single-site fall-through, the redirect_uri using the host, the state binding on the site key, and the untenanted bucket being one identity rather than one per host. Refs ephpm/ephpm#449 --- Cargo.lock | 8 +- Cargo.toml | 30 +- README.md | 66 ++- .../src/config.rs | 234 +++++++---- .../ephpm-middleware-github-auth/src/lib.rs | 394 +++++++++++++++--- .../tests/oauth_round_trip.rs | 23 +- .../src/session_cookie.rs | 157 ++++--- 7 files changed, 693 insertions(+), 219 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6e2c5e3..3484299 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -255,7 +255,7 @@ checksum = "92773504d58c093f6de2459af4af33faa518c13451eb8f2b5698ed3d36e7c813" [[package]] name = "ephpm-config" version = "0.1.0" -source = "git+https://github.com/ephpm/ephpm.git?rev=b380cc6d0bbc61110475c0c3db3097e8bdf25018#b380cc6d0bbc61110475c0c3db3097e8bdf25018" +source = "git+https://github.com/ephpm/ephpm.git?rev=21a7c8a7832b62d0ab8af960931a33e8e9778784#21a7c8a7832b62d0ab8af960931a33e8e9778784" dependencies = [ "figment", "serde", @@ -267,7 +267,7 @@ dependencies = [ [[package]] name = "ephpm-kv" version = "0.1.0" -source = "git+https://github.com/ephpm/ephpm.git?rev=b380cc6d0bbc61110475c0c3db3097e8bdf25018#b380cc6d0bbc61110475c0c3db3097e8bdf25018" +source = "git+https://github.com/ephpm/ephpm.git?rev=21a7c8a7832b62d0ab8af960931a33e8e9778784#21a7c8a7832b62d0ab8af960931a33e8e9778784" dependencies = [ "anyhow", "brotli", @@ -288,7 +288,7 @@ dependencies = [ [[package]] name = "ephpm-middleware" version = "0.1.0" -source = "git+https://github.com/ephpm/ephpm.git?rev=b380cc6d0bbc61110475c0c3db3097e8bdf25018#b380cc6d0bbc61110475c0c3db3097e8bdf25018" +source = "git+https://github.com/ephpm/ephpm.git?rev=21a7c8a7832b62d0ab8af960931a33e8e9778784#21a7c8a7832b62d0ab8af960931a33e8e9778784" dependencies = [ "ephpm-kv", "serde_json", @@ -335,7 +335,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.61.2", + "windows-sys 0.52.0", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 93a84ad..e2524db 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,22 +20,30 @@ repository = "https://github.com/ephpm/middleware-github-auth" # both modules here are provably built against one specific host-ABI commit. To # rebuild against a newer host, replace `rev` and run `cargo update`. # -# Pinned at ePHPm main `b380cc6d0bbc61110475c0c3db3097e8bdf25018`, which is -# past #408 — the PR that made the request phase run on the STATIC-file path -# too (fail-closed), so an auth gate protects static assets and not only -# PHP-dispatched requests (issue #395). Both modules here rely on that: a -# request with no valid session is short-circuited before the file is read. +# Pinned at ePHPm main `21a7c8a7832b62d0ab8af960931a33e8e9778784` — the merge +# of ephpm#448, which took the ABI to **minor 3**. This bump is not a routine +# refresh; it is the whole point of ephpm#449 for these two modules: # -# This rev deliberately does NOT depend on the request-scheme / host accessors -# proposed in #409 (not merged here): the ABI at this rev exposes no request -# scheme, so `session-cookie` does not do a server-side transport check — see -# its module docs. -ephpm-middleware = { git = "https://github.com/ephpm/ephpm.git", rev = "b380cc6d0bbc61110475c0c3db3097e8bdf25018" } +# * `Request::vhost_id()` returns `Option<&str>` carrying the router's +# CANONICAL SITE KEY, and `None` for a host that matched no virtual host. +# Before minor 3 it was the raw `Host` header — client-controlled, +# un-normalized, never absent — which both modules had to re-normalize +# locally and neither could tell apart from a real tenant (ephpm#390). +# That value is a tenant identity here: it selects the `sites` access +# check and it is written into the session token's `site` claim. +# * `Request::http_host()` (minor 2) gives the normalized request host +# separately, which is what the derived `redirect_uri` needs — the site key +# is suffix-stripped and is not a routable authority. +# +# Still past #408 (the static-path request phase, ephpm#395) — ABI growth +# within major 1 is additive, so everything the previous pin +# (`b380cc6d0bbc61110475c0c3db3097e8bdf25018`) provided is still here. +ephpm-middleware = { git = "https://github.com/ephpm/ephpm.git", rev = "21a7c8a7832b62d0ab8af960931a33e8e9778784" } # Test-only: the same embedded KV store the host wires into the middleware host # table, so the `host` feature's `RequestCtx` / `host_table` can fabricate a # request in unit and integration tests. Same rev as the ABI crate so both # resolve to one crate instance and the `Store` type matches. -ephpm-kv = { git = "https://github.com/ephpm/ephpm.git", rev = "b380cc6d0bbc61110475c0c3db3097e8bdf25018" } +ephpm-kv = { git = "https://github.com/ephpm/ephpm.git", rev = "21a7c8a7832b62d0ab8af960931a33e8e9778784" } # Both modules parse their config out of a `serde_json::Value`. serde_json = "1" diff --git a/README.md b/README.md index e9c84c2..2363f66 100644 --- a/README.md +++ b/README.md @@ -54,12 +54,63 @@ config = { secret = "env:EPHPM_SESSION_SECRET", cookie = "ephpm_session", | session cookie present | verifier checks it: `CONTINUE`, or `302` → login | no network | The issued session is a **compact HS256 JWT** carrying the GitHub login, the -vhost it was issued for, how it was obtained, and an expiry — no server-side +site it was issued for, how it was obtained, and an expiry — no server-side session record, so restarts and second nodes log nobody out. The issuer's three GitHub calls per login are: `POST /login/oauth/access_token` (code → token), `GET /user` (the identity), and the access check (`GET /repos/{owner}/{name}`, `/user/memberships/orgs/{org}`, or a team-membership call). +## Tenancy: the site key, not the `Host` header + +Both modules take the request's tenant identity from `req.vhost_id()`, which +since ePHPm's middleware **ABI minor 3** +([#448](https://github.com/ephpm/ephpm/pull/448), issue +[#390](https://github.com/ephpm/ephpm/issues/390)) is the **canonical site key** +the router resolved — the vhost directory name — rather than the raw `Host` +header. Two values that used to be one are now kept apart: + +| | value | used for | +|---|---|---| +| **site key** (`req.vhost_id()`) | `pr-1` | the `sites` access check, the OAuth `state` binding, the session's `site` claim | +| **request host** (`req.http_host()`) | `pr-1.preview.example.com` | the derived `redirect_uri`, and nothing else | + +With a `sites_domain_suffix` of `.preview.example.com` those differ, and the +difference matters in both directions: the site key is the tenant boundary (the +same identity that picks the per-site database and KV keyspace), and it is +*not* a routable authority — a `redirect_uri` of `https://pr-1/…` would go +nowhere. + +Consequences worth knowing before you upgrade: + +- **`sites` keys are site keys now.** `sites = { "pr-1" = {…} }`, not + `"pr-1.preview.example.com"`. A key that could never *be* a site key — one + with a port, an IPv6 literal, an upper-case letter that does not lowercase to + a legal key — now **fails the mount at startup** rather than silently never + matching. A dotted key is still accepted, because a deployment without + `sites_domain_suffix` really does key on the full name. +- **Every spelling of a host is one tenant.** `PR-1.Preview.Test`, + `pr-1.preview.test:8443` and `pr-1.preview.test.` collapse upstream, so the + case-variant `sites` miss that #390 describes is gone — and the modules no + longer re-normalise the header themselves. +- **A request that matched no vhost has no tenant.** With a `sites` table it is + denied; without one (the single-site deployment, where `vhost_id()` is always + `None`) the top-level `repo`/`org`/`team` applies as before. Its session + binds to the constant `_UNMATCHED`, never to something the client sent. +- **Sessions issued before the upgrade name the old identity.** They stay + signature-valid, but their `site` claim is the old host-shaped string. Users + log in again once. + +### Not fixed here: the verifier ignores the `site` claim + +The issuer binds each session to one tenant; `session-cookie` verifies the +signature and expiry but **does not compare the `site` claim to the request's +site**. On a multi-tenant node a session issued for one preview therefore +verifies on every other preview served by the same mount — ePHPm +[#396](https://github.com/ephpm/ephpm/issues/396), which is open. ABI minor 3 +is what makes the comparison meaningful (before it, the only available identity +was client-controlled), so the fix is now possible; making it, and deciding +what to do with a `site`-less share token, is #396's call. + **Access is unrevocable until it expires** — that is the cost of a self-contained token. `session_ttl_secs` (default 8h) is therefore the blast radius of a stolen cookie; rotating `session_secret` is the only early @@ -76,7 +127,7 @@ defaults are in [`config.rs`](crates/ephpm-middleware-github-auth/src/config.rs) | `client_secret` | **required** | client secret — use `env:NAME`, never a literal | | `session_secret` | **required, ≥32 bytes** | HMAC key for the session token; must match the verifier's `secret` | | `repo` / `org` / `team` | one is required | the access target: read access to `owner/name`, active org membership, or active team membership | -| `sites` | unset | per-vhost table mapping each preview hostname to its own target (authoritative when present) | +| `sites` | unset | per-tenant table mapping each **canonical site key** to its own target (authoritative when present) — see [Tenancy](#tenancy-the-site-key-not-the-host-header) | | `session_ttl_secs` | `28800` | session lifetime (60 … 604800) | | `bypass_token` | unset | pre-shared token (≥32 bytes) letting CI reach a preview headlessly; presenting it mints a normal session | | `github_base` / `github_api_base` | `https://github.com` / `https://api.github.com` | override for GitHub Enterprise Server | @@ -126,8 +177,15 @@ Both modules build against the `ephpm-middleware` ABI as a **git dependency pinned by `rev`** to ePHPm `main` (see the workspace [`Cargo.toml`](Cargo.toml)) — the same way ePHPm pins litewire. A drift in `EphpmHostV1` / `ABI_V1` would be silent UB at the FFI boundary, so the pin is -exact. The pinned rev is past #408 (the static-path request phase) and does not -depend on the request-scheme accessor proposed in #409 (not merged). +exact. + +The pin is at **ABI major 1, minor 3**. It is past +[#408](https://github.com/ephpm/ephpm/pull/408) (the static-path request phase +both modules rely on) and past +[#448](https://github.com/ephpm/ephpm/pull/448), which is what gives +`vhost_id()` its canonical-site-key meaning and adds `http_host()` — see +[Tenancy](#tenancy-the-site-key-not-the-host-header). To rebuild against a newer +host, replace the `rev` and `cargo update`. ## Build & test diff --git a/crates/ephpm-middleware-github-auth/src/config.rs b/crates/ephpm-middleware-github-auth/src/config.rs index 5b1f84e..f3b8a07 100644 --- a/crates/ephpm-middleware-github-auth/src/config.rs +++ b/crates/ephpm-middleware-github-auth/src/config.rs @@ -114,7 +114,9 @@ pub struct Config { /// Access check applied when the request's vhost has no `sites` entry. pub default_check: Option, - /// Per-vhost access checks (`request_vhost_id` → check). + /// Per-vhost access checks, keyed by the **canonical site key** + /// (`request_vhost_id` since ABI minor 3 — the vhost directory name, not + /// the `Host` header). See [`Config::check_for`]. pub sites: BTreeMap, /// Reserved path that starts a login. @@ -365,18 +367,22 @@ impl Config { crate::token::STATE_KEY_LABEL, )); - // Access targets. `sites` maps a vhost id to its own check, which is - // what lets one mount serve a fleet of per-PR preview hostnames. + // Access targets. `sites` maps a CANONICAL SITE KEY to its own check, + // which is what lets one mount serve a fleet of per-PR previews. Since + // ABI minor 3 the key is the vhost directory name the router matched, + // NOT the request hostname — with a `sites_domain_suffix` of + // `.preview.example.com`, `pr-1.preview.example.com` resolves to the + // site key `pr-1`, and `pr-1` is what belongs here. let mut sites = BTreeMap::new(); match config.get("sites") { None | Some(serde_json::Value::Null) => {} Some(serde_json::Value::Object(map)) => { - for (host, entry) in map { - let host_key = normalize_vhost(host); - validate_vhost(&host_key)?; - let check = parse_check(entry, &format!("sites.{host}"))? - .ok_or_else(|| format!("sites.{host}: no repo/org/team configured"))?; - sites.insert(host_key, check); + for (site, entry) in map { + let site_key = normalize_site_key(site); + validate_site_key(&site_key)?; + let check = parse_check(entry, &format!("sites.{site}"))? + .ok_or_else(|| format!("sites.{site}: no repo/org/team configured"))?; + sites.insert(site_key, check); } } Some(other) => return Err(format!("`sites` must be a table, got {other}")), @@ -520,22 +526,37 @@ impl Config { }) } - /// The access check that applies to `vhost`. + /// The access check that applies to this request's tenant. /// - /// `vhost` must already have been through [`normalize_vhost`]; the keys - /// of `sites` were normalised at parse time, so anything else silently - /// misses. + /// `site` is [`ephpm_middleware::Request::vhost_id`] verbatim: the router's + /// **canonical site key**, or `None` when the request matched no known + /// virtual host. It is deliberately *not* re-derived from the `Host` + /// header — that was ephpm#390, and this is an authorization decision. /// - /// When a `sites` table is configured it is authoritative: a vhost with - /// no entry gets **no** check and therefore no access, even if a - /// top-level `repo` is also set. Falling back would mean a hostname - /// nobody mapped quietly inherits some other tenant's rule. + /// Three cases, and the `None` one is the reason this takes an `Option`: + /// + /// * **No `sites` table** — the mount has a single top-level target, so + /// `default_check` applies to whatever is being served. This is the + /// single-site deployment, where `vhost_id()` is always `None` (there are + /// no virtual hosts to match), and it must keep working. + /// * **`sites` table, request has a tenant** — exact lookup. When a `sites` + /// table is configured it is authoritative: a vhost with no entry gets + /// **no** check and therefore no access, even if a top-level `repo` is + /// also set. Falling back would mean a site nobody mapped quietly + /// inherits some other tenant's rule. + /// * **`sites` table, request has no tenant** — deny. The operator mapped + /// specific vhosts; a request that matched none of them is not one of + /// them, and there is no identity to look up. Substituting the `Host` + /// here would let a caller pick which tenant's rule it is judged by. + /// + /// The keys of `sites` are lowercased at parse time, which matches the + /// router's own site keys (always lowercase). #[must_use] - pub fn check_for(&self, vhost: &str) -> Option<&Check> { + pub fn check_for(&self, site: Option<&str>) -> Option<&Check> { if self.sites.is_empty() { return self.default_check.as_ref(); } - self.sites.get(vhost) + self.sites.get(site?) } /// Paths the gate owns, for the return-to loop check. @@ -545,48 +566,78 @@ impl Config { } } -/// Canonicalise the middleware ABI's `request_vhost_id` before it is used as -/// a key or written into a token. +/// Normalise a **configured** `sites` key so it can be compared to the +/// router's canonical site key. /// -/// **This is not cosmetic.** `request_vhost_id` is the router's `SERVER_NAME` -/// — the raw `Host` header with the port split off and *nothing else*. It is -/// **not** lowercased, the FQDN-root dot is not stripped, and it is not the -/// canonical site key that `Router::resolve_site` produces. `Host` is -/// case-insensitive per RFC 9110 and entirely client-controlled, so without -/// this a `sites` lookup misses on `Host: PR-1.Preview.Test`, and — worse — -/// two spellings of the same host would mint sessions with two different -/// `site` claims and two different derived `redirect_uri`s. +/// This only has to fix up what an operator might type in `ephpm.toml` +/// (stray whitespace, capitals, a trailing FQDN-root dot). The *request* side +/// needs no normalisation at all any more: since ABI minor 3 +/// `request_vhost_id` returns the key `Router::resolve_site` matched, which is +/// already port-stripped, dot-stripped, lowercased, suffix-stripped and +/// allowlist-validated (ephpm#390). /// -/// The port is deliberately **not** split off here: the server already -/// removed it, and splitting on `:` would corrupt an IPv6 literal -/// (`[::1]` → `[`). If a future ABI change reintroduced the port, the key -/// would simply stop matching, which fails closed. -/// -/// `ephpm-server`'s own `normalize_host_key` is `pub(crate)`, so a module -/// cannot call it; the duplication is forced. +/// Before minor 3 this function was applied to the *request* value too, +/// because the ABI handed over the raw `Host` header — client-controlled and +/// un-normalised — and a `sites` lookup would otherwise miss on +/// `Host: PR-1.Preview.Test`. That workaround is deleted: re-normalising a +/// client string is not the same thing as being told which tenant the router +/// resolved, and only the latter can be trusted for an authorization decision. #[must_use] -pub fn normalize_vhost(raw: &str) -> String { +pub fn normalize_site_key(raw: &str) -> String { raw.trim().trim_end_matches('.').to_ascii_lowercase() } -/// Reject a vhost id that could not appear in a URL authority. +/// Reject a `sites` key that no canonical site key could ever equal. +/// +/// Mirrors ePHPm's own `is_valid_site_key`: a site key is a vhost **directory +/// name** under `sites_dir`, so it is non-empty ASCII from `[a-z0-9._-]` and +/// nothing else. A port, an IPv6 literal, or an upper-case letter can never +/// appear in one, so a `sites` table still written in terms of request +/// *hostnames* (`localhost:8080`, `[::1]`) is a configuration error and is +/// rejected at startup rather than silently never matching. +/// +/// # Errors +/// +/// Returns a message when the key is empty, over-long, or contains a character +/// outside the site-key allowlist. +pub fn validate_site_key(key: &str) -> Result<(), String> { + let ok = !key.is_empty() + && key.len() <= 253 + && key.bytes().all(|b| b.is_ascii_lowercase() || b.is_ascii_digit() || b"._-".contains(&b)); + if ok { + Ok(()) + } else { + Err(format!( + "{key:?} is not a usable site key — `sites` is keyed by the vhost DIRECTORY name \ + the router resolves (lowercase `[a-z0-9._-]`), not by a request hostname; with a \ + `sites_domain_suffix` of `.preview.example.com` the key for \ + `pr-1.preview.example.com` is `pr-1`" + )) + } +} + +/// Reject a request host that could not appear in a URL authority. /// -/// The vhost is interpolated into the derived `redirect_uri`, so it is -/// attacker-influenced input (it comes from the `Host` header) heading for an -/// outbound URL. +/// This guards the host interpolated into the **derived `redirect_uri`**, +/// which is the one place the module still uses the request host rather than +/// the tenant identity: the site key is suffix-stripped and is therefore not a +/// routable authority, whereas `redirect_uri` has to be a URL GitHub will +/// redirect a browser back to. The value comes from the `Host` header (via +/// `request_host`), so it is attacker-influenced input heading for an outbound +/// URL — hence the allowlist. A port and an IPv6 literal are legal here. /// /// # Errors /// /// Returns a message when the name is empty, over-long, non-ASCII, or /// contains anything outside the host/port character set. -pub fn validate_vhost(host: &str) -> Result<(), String> { +pub fn validate_redirect_host(host: &str) -> Result<(), String> { let ok = !host.is_empty() && host.len() <= 253 && host.is_ascii() && host .chars() .all(|c| c.is_ascii_alphanumeric() || matches!(c, '-' | '.' | '_' | ':' | '[' | ']')); - if ok { Ok(()) } else { Err(format!("{host:?} is not a usable virtual-host name")) } + if ok { Ok(()) } else { Err(format!("{host:?} is not a usable request host")) } } #[cfg(test)] @@ -723,47 +774,78 @@ mod tests { fn sites_table_is_authoritative_when_present() { let mut v = base(); v["sites"] = serde_json::json!({ - "pr-1.preview.example.com": { "repo": "acme/web" }, - "PR-2.preview.example.com": { "org": "acme" }, + "pr-1": { "repo": "acme/web" }, + "PR-2": { "org": "acme" }, }); let c = parse(v).expect("parse"); assert_eq!( - c.check_for("pr-1.preview.example.com"), + c.check_for(Some("pr-1")), Some(&Check::Repo { owner: "acme".into(), name: "web".into() }) ); // A `sites` KEY written in mixed case is normalised at parse time. + assert_eq!(c.check_for(Some("pr-2")), Some(&Check::Org { org: "acme".into() })); + // An unmapped site gets nothing — it does NOT inherit the top-level + // `repo`, which is also set in this config. + assert_eq!(c.check_for(Some("pr-99")), None); + } + + /// ePHPm #390 / #448. With a `sites` table configured, a request that + /// matched **no** virtual host has no tenant identity and must be denied — + /// not silently handed the top-level `repo` (that would let an + /// unrecognised `Host` inherit a real tenant's rule), and not looked up + /// under some `Host`-derived string. + #[test] + fn an_untenanted_request_is_denied_when_sites_is_configured() { + let mut v = base(); + v["sites"] = serde_json::json!({ "pr-1": { "repo": "acme/web" } }); + let c = parse(v).expect("parse"); + assert!(c.default_check.is_some(), "the top-level `repo` from base() is set"); + assert_eq!(c.check_for(None), None, "no tenant + a sites table must deny"); + } + + /// The single-site deployment: no `sites` table, so the one top-level + /// target applies to every request. `vhost_id()` is always `None` on a node + /// with no virtual hosts, so this is the *normal* path there and must not + /// be caught by the fail-closed branch above. + #[test] + fn an_untenanted_request_uses_the_default_check_when_sites_is_absent() { + let c = parse(base()).expect("parse"); assert_eq!( - c.check_for("pr-2.preview.example.com"), - Some(&Check::Org { org: "acme".into() }) + c.check_for(None), + Some(&Check::Repo { owner: "acme".into(), name: "web".into() }) ); - // An unmapped vhost gets nothing — it does NOT inherit the top-level - // `repo`, which is also set in this config. - assert_eq!(c.check_for("pr-99.preview.example.com"), None); } + /// The router hands over one canonical key per tenant, so the module no + /// longer re-normalises a client string — but an operator still types the + /// `sites` keys by hand, and those are normalised at parse time. #[test] - fn vhost_normalisation_closes_the_case_bypass() { - // `request_vhost_id` is the RAW `Host` header. Without normalisation - // a client picks its own key by changing a letter's case, and mints - // sessions whose `site` claim disagrees with everyone else's. - for raw in [ - "PR-1.Preview.Example.COM", - "pr-1.preview.example.com.", - " pr-1.preview.example.com ", - "PR-1.PREVIEW.EXAMPLE.COM.", - ] { - assert_eq!(normalize_vhost(raw), "pr-1.preview.example.com", "{raw:?}"); + fn configured_site_keys_are_normalised_at_parse_time() { + for raw in ["PR-1", "pr-1.", " pr-1 ", "PR-1."] { + assert_eq!(normalize_site_key(raw), "pr-1", "{raw:?}"); } - // An IPv6 literal survives: the port is already gone, and splitting - // on ':' here would truncate it to "[". - assert_eq!(normalize_vhost("[::1]"), "[::1]"); - assert!(validate_vhost(&normalize_vhost("[::1]")).is_ok()); - let mut v = base(); - v["sites"] = serde_json::json!({ "PR-1.Preview.Example.COM": { "repo": "acme/web" } }); + v["sites"] = serde_json::json!({ " PR-1. ": { "repo": "acme/web" } }); let c = parse(v).expect("parse"); - assert!(c.check_for(&normalize_vhost("pr-1.PREVIEW.example.com")).is_some()); - assert!(c.check_for(&normalize_vhost("PR-1.preview.example.com.")).is_some()); + assert!(c.check_for(Some("pr-1")).is_some()); + } + + /// A `sites` table still written in request-hostname terms is a + /// configuration error, and fails the mount at startup rather than + /// silently never matching a site key. + #[test] + fn a_sites_key_that_cannot_be_a_site_key_fails_the_mount() { + for bad in ["localhost:8080", "[::1]", "pr 1", "pr/1", "caf\u{e9}"] { + let mut v = base(); + v["sites"] = serde_json::json!({ bad: { "repo": "acme/web" } }); + let err = parse(v).expect_err(&format!("{bad:?} must be refused")); + assert!(err.contains("not a usable site key"), "{bad:?}: {err}"); + } + // A dotted name is still legal: it is a legal vhost DIRECTORY name, so + // a deployment without `sites_domain_suffix` really does key on it. + let mut v = base(); + v["sites"] = serde_json::json!({ "pr-1.preview.example.com": { "repo": "acme/web" } }); + assert!(parse(v).is_ok()); } #[test] @@ -818,12 +900,16 @@ mod tests { assert!(parse(v).expect_err("weak bypass").contains("at least 32 bytes")); } + /// The guard on the host that goes into the derived `redirect_uri` — an + /// outbound URL authority, so a port and an IPv6 literal are legal here + /// even though neither can appear in a site key. #[test] - fn vhost_validation() { - assert!(validate_vhost("pr-1.preview.example.com").is_ok()); - assert!(validate_vhost("localhost:8080").is_ok()); + fn redirect_host_validation() { + assert!(validate_redirect_host("pr-1.preview.example.com").is_ok()); + assert!(validate_redirect_host("localhost:8080").is_ok()); + assert!(validate_redirect_host("[::1]").is_ok()); for bad in ["", "has space", "evil.test/path", "a@b", "caf\u{e9}.test", "x\r\ny"] { - assert!(validate_vhost(bad).is_err(), "{bad:?}"); + assert!(validate_redirect_host(bad).is_err(), "{bad:?}"); } } } diff --git a/crates/ephpm-middleware-github-auth/src/lib.rs b/crates/ephpm-middleware-github-auth/src/lib.rs index a680ca7..f7bf670 100644 --- a/crates/ephpm-middleware-github-auth/src/lib.rs +++ b/crates/ephpm-middleware-github-auth/src/lib.rs @@ -89,11 +89,12 @@ //! # Sessions are self-contained //! //! The session is an HS256 JWT (see [`token`]) carrying its subject, the -//! vhost it was issued for, how it was obtained, and an expiry. There is no +//! **site** it was issued for, how it was obtained, and an expiry. There is no //! server-side session record: restarts do not log anyone out, a second node //! needs no replication, and the module never touches the middleware KV -//! surface — which is process-global rather than per-site (ephpm#376) and -//! therefore the wrong place for anything tenant-scoped. +//! surface at all. (Since ePHPm#376 that surface *is* per-site by default, so +//! the old reason to avoid it — it was process-global — is gone; the reason +//! that remains is simply that a self-contained token needs no storage.) //! //! The cost of that, stated plainly: **an issued session cannot be revoked //! before it expires.** Rotating `session_secret` invalidates every session @@ -101,6 +102,47 @@ //! therefore the blast radius of a stolen cookie, and it defaults to eight //! hours. //! +//! Note also what the **verifier** does not do with the `site` claim this +//! module writes: `session-cookie` checks the signature and the expiry but not +//! the tenant binding, so a session issued for one preview verifies on every +//! preview served by the same mount. That is ePHPm +//! [#396](https://github.com/ephpm/ephpm/issues/396), it is open, and the +//! binding written here is the half of the fix that already exists. +//! +//! # The `site` claim is the canonical site key (ePHPm #390 / #448) +//! +//! Since middleware ABI minor 3, `Request::vhost_id()` returns the tenant +//! identity the **router** resolved — the vhost directory name — and `None` +//! for a request that matched no virtual host. This module uses that value and +//! nothing else for tenancy: the `sites` access check, the OAuth `state` +//! binding, and the token's `site` claim. +//! +//! What that buys, concretely: +//! +//! * `Host: PR-1.Preview.Test`, `pr-1.preview.test:8443` and +//! `pr-1.preview.test.` are **one** tenant, not three. Before minor 3 this +//! module had to re-normalise the header itself to get even that far, and a +//! local re-normalisation is a guess about what the router did, not the +//! router's answer. +//! * A request whose `Host` matched no vhost has **no** tenant identity, and +//! cannot be given one by sending a different `Host`. It gets the single +//! constant `_UNMATCHED` ([`ephpm_middleware::UNMATCHED_VHOST`]) — and if a +//! `sites` table is configured it is denied outright, because a request that +//! matched none of the mapped vhosts is not one of them. +//! +//! The request host is still available, and is still used for exactly one +//! thing: the derived `redirect_uri`, which must be an authority a browser can +//! be redirected back to. See [`GithubAuth::invoke`]. +//! +//! **Upgrade note.** `sites` is now keyed by the site key, not the request +//! hostname — with `sites_domain_suffix = ".preview.example.com"` the key for +//! `pr-1.preview.example.com` is `pr-1`. A `sites` table still written in +//! hostname terms will fail startup validation if the key cannot be a site key +//! at all (a port, an IPv6 literal), and will otherwise simply never match, so +//! **check `sites` when upgrading**. Sessions issued before the upgrade carry +//! the old host-shaped `site` claim; they remain signature-valid but name a +//! tenant that no longer exists under that spelling. +//! //! # Share links, for free //! //! Issuance and verification are separate on purpose, and issuance is not @@ -200,6 +242,66 @@ pub enum Route { HasSession, } +/// Which tenant a request belongs to, as this gate uses it. +/// +/// A thin wrapper over [`Request::vhost_id`] whose only job is to keep the two +/// uses of that value distinguishable, because they want different things from +/// an absent tenant: +/// +/// * the `sites` access-check lookup, which must **fail** when there is no +/// tenant to look up — [`SiteIdentity::key`]; +/// * the session token's `site` claim and the OAuth `state` binding, which need +/// *a* string and must never be given a client-supplied one — +/// [`SiteIdentity::claim`]. +/// +/// Before ABI minor 3 there was no distinction to make: the ABI handed over the +/// raw `Host` header, so "no tenant" and "someone sent `Host: pr-1`" were the +/// same string (ephpm#390). +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SiteIdentity { + /// The router matched a virtual host; this is its canonical site key. + Tenant(String), + /// The request matched **no** known virtual host. On a multi-tenant node + /// that is an unrecognised `Host`; on a single-site node it is simply every + /// request, because there are no virtual hosts to match. + Untenanted, +} + +impl SiteIdentity { + /// Read the request's tenant identity. The one place `vhost_id()` is + /// called. + #[must_use] + pub fn of(req: &Request<'_>) -> Self { + req.vhost_id().map_or(Self::Untenanted, |key| Self::Tenant(key.to_owned())) + } + + /// The `sites` lookup key — `None` when there is no tenant, so + /// [`Config::check_for`] denies rather than guessing. + #[must_use] + pub fn key(&self) -> Option<&str> { + match self { + Self::Tenant(key) => Some(key), + Self::Untenanted => None, + } + } + + /// The string bound into the `site` claim and the OAuth `state`. + /// + /// For an untenanted request this is the constant + /// [`ephpm_middleware::UNMATCHED_VHOST`] — uppercase, and therefore + /// unspellable as a site key, so a session bound to it can never be + /// confused with one bound to a real tenant, and two different unrecognised + /// hostnames cannot mint two different identities for what is one and the + /// same default document root. + #[must_use] + pub fn claim(&self) -> &str { + match self { + Self::Tenant(key) => key, + Self::Untenanted => ephpm_middleware::UNMATCHED_VHOST, + } + } +} + /// The gate. pub struct GithubAuth { config: Config, @@ -256,7 +358,7 @@ impl GithubAuth { /// | `sub` | GitHub login, or the bypass subject | /// | `aud` | `audience`, when configured | /// | `exp` / `iat` | expiry / issue time, seconds since the epoch | - /// | `site` | the vhost this session is for — a session for one preview must not open another | + /// | `site` | the tenant this session is for — the **canonical site key** since ABI minor 3, or `_UNMATCHED` for a request that matched no vhost | /// | `via` | `"github"` or `"bypass"`; an external minter uses `"share"` | /// | `gh_id` | numeric GitHub account id (stable across renames), for `via = "github"` | /// | `check` | the rule that was satisfied, e.g. `repo acme/web` — for audit | @@ -292,21 +394,33 @@ impl GithubAuth { } /// Build the `redirect_uri` for this request. - fn redirect_uri(&self, vhost: &str) -> String { + /// + /// `host` is the **request host**, not the site key: this is a URL GitHub + /// redirects a browser back to, and a suffix-stripped site key (`pr-1`) is + /// not a routable authority. See the note in [`GithubAuth::invoke`]. + fn redirect_uri(&self, host: &str) -> String { self.config .redirect_uri .clone() - .unwrap_or_else(|| format!("https://{vhost}{}", self.config.callback_path)) + .unwrap_or_else(|| format!("https://{host}{}", self.config.callback_path)) } /// 302 to GitHub's authorize endpoint, with a fresh signed `state`. - fn start_login(&self, req: &Request<'_>, vhost: &str, return_to: &str, now: u64) -> Response { - let Some(check) = self.config.check_for(vhost) else { + fn start_login( + &self, + req: &Request<'_>, + site: &SiteIdentity, + host: &str, + return_to: &str, + now: u64, + ) -> Response { + let vhost = site.claim(); + let Some(check) = self.config.check_for(site.key()) else { log( req, LOG_WARN, &format!( - "github-auth: refusing login for vhost {vhost:?} — it has no `sites` entry and \ + "github-auth: refusing login for site {vhost:?} — it has no `sites` entry and \ no default target, so there is nothing to check access against" ), ); @@ -330,7 +444,7 @@ impl GithubAuth { "{}/login/oauth/authorize?client_id={}&redirect_uri={}&state={}&response_type=code", self.config.github_base, encode_query(&self.config.client_id), - encode_query(&self.redirect_uri(vhost)), + encode_query(&self.redirect_uri(host)), encode_query(&nonce), ); if !self.config.scopes.is_empty() { @@ -341,10 +455,7 @@ impl GithubAuth { log( req, LOG_INFO, - &format!( - "github-auth: starting login for vhost {vhost:?} against {}", - check.describe() - ), + &format!("github-auth: starting login for site {vhost:?} against {}", check.describe()), ); redirect_to(&url).header( "Set-Cookie", @@ -367,7 +478,14 @@ impl GithubAuth { /// module talk to GitHub at all, which is what keeps the callback from /// being an amplification endpoint. #[allow(clippy::too_many_lines, reason = "one linear flow, each step guarded")] - fn handle_callback(&self, req: &Request<'_>, vhost: &str, now: u64) -> Response { + fn handle_callback( + &self, + req: &Request<'_>, + site: &SiteIdentity, + host: &str, + now: u64, + ) -> Response { + let vhost = site.claim(); let query = req.query(); let clear_state = set_cookie(&self.config.state_cookie_name, "", 0, &self.config.cookie_attrs); @@ -414,8 +532,9 @@ impl GithubAuth { return deny(400, "This login link is not valid here. Start again.") .header("Set-Cookie", clear_state); } - // The state is bound to the vhost it was issued on, so a state minted - // for one tenant cannot complete a login on another. + // The state is bound to the SITE it was issued on — the canonical site + // key, so every spelling of one vhost is one binding — and a state + // minted for one tenant cannot complete a login on another. if state_claims.get("v").and_then(serde_json::Value::as_str) != Some(vhost) { log(req, LOG_WARN, "github-auth: OAuth state was issued for a different host"); return deny(400, "This login link is not valid here. Start again.") @@ -429,7 +548,7 @@ impl GithubAuth { &self.config.reserved_paths(), ); - let Some(check) = self.config.check_for(vhost) else { + let Some(check) = self.config.check_for(site.key()) else { return deny(403, "This preview is not configured for GitHub access control.") .header("Set-Cookie", clear_state); }; @@ -442,7 +561,7 @@ impl GithubAuth { // ── The only network calls in the module ───────────────────────── let outcome = - self.github.exchange_code(&code, &self.redirect_uri(vhost)).and_then(|access| { + self.github.exchange_code(&code, &self.redirect_uri(host)).and_then(|access| { let user = self.github.current_user(&access)?; let allowed = self.github.check_access(&access, check, &user)?; Ok((user, allowed)) @@ -464,7 +583,7 @@ impl GithubAuth { req, LOG_INFO, &format!( - "github-auth: denied {} on vhost {vhost:?} — no access to {}", + "github-auth: denied {} on site {vhost:?} — no access to {}", user.login, check.describe() ), @@ -475,7 +594,7 @@ impl GithubAuth { self.issue( req, - vhost, + site, &user.login, "github", Some(user.id), @@ -492,7 +611,7 @@ impl GithubAuth { fn issue( &self, req: &Request<'_>, - vhost: &str, + site: &SiteIdentity, subject: &str, via: &str, gh_id: Option, @@ -501,6 +620,7 @@ impl GithubAuth { ttl: u64, return_to: &str, ) -> Response { + let vhost = site.claim(); let claims = self.session_claims(subject, vhost, via, gh_id, check, now, ttl); let Some(session) = token::mint(self.config.session_secret.expose(), &claims) else { log(req, LOG_ERROR, "github-auth: could not mint the session token"); @@ -562,13 +682,23 @@ impl Middleware for GithubAuth { fn invoke(&self, req: &Request<'_>) -> Response { self.banner(req); - // Canonicalise ONCE, here, and use only this value below — as the - // `sites` key, in the `site` claim, in the state binding and in the - // derived `redirect_uri`. `request_vhost_id` is the raw `Host` - // header; see `config::normalize_vhost` for why that matters. - let vhost = config::normalize_vhost(req.vhost_id()); - let vhost = vhost.as_str(); - if config::validate_vhost(vhost).is_err() { + // Two DIFFERENT values, resolved once here, never re-derived below. + // + // `site` is the tenant identity — the router's canonical site key + // (ABI minor 3, ephpm#390). It selects the `sites` access check, binds + // the OAuth `state`, and becomes the token's `site` claim. `None` means + // the request matched no virtual host; `SiteIdentity` carries that + // through rather than papering over it with the `Host` header. + let site = SiteIdentity::of(req); + // + // `host` is the request host as sent (normalised: port and trailing dot + // stripped, lowercased). It is NOT a tenant identity and is used for + // exactly one thing — building the derived `redirect_uri`, which has to + // be a URL a browser can come back to. The site key cannot serve there: + // with a `sites_domain_suffix` it is the suffix-stripped directory name + // (`pr-1`), not a routable authority (`pr-1.preview.example.com`). + let host = req.http_host(); + if config::validate_redirect_host(host).is_err() { log(req, LOG_WARN, "github-auth: request with an unusable Host header — rejected"); return deny(400, "Bad request."); } @@ -579,8 +709,8 @@ impl Middleware for GithubAuth { match self.route(req.path(), req.query(), cookies, bypass.as_deref()) { Route::HasSession => Response::cont(), - Route::StartLogin { return_to } => self.start_login(req, vhost, &return_to, now), - Route::Callback => self.handle_callback(req, vhost, now), + Route::StartLogin { return_to } => self.start_login(req, &site, host, &return_to, now), + Route::Callback => self.handle_callback(req, &site, host, now), Route::Bypass => { let return_to = redirect::sanitize_return_to( &here(req.path(), req.query()), @@ -588,7 +718,7 @@ impl Middleware for GithubAuth { ); self.issue( req, - vhost, + &site, "automation", "bypass", None, @@ -728,6 +858,17 @@ mod tests { const SESSION_SECRET: &str = "0123456789abcdef0123456789abcdef"; const BYPASS: &str = "bypass-token-that-is-long-enough-32"; + /// The two values ABI minor 3 keeps apart, and the tests with them. + /// + /// A preview node runs with `sites_domain_suffix = ".preview.test"`, so the + /// browser asks for `pr-1.preview.test` (the request host, and the only + /// authority a `redirect_uri` can name) and the router resolves it to the + /// vhost directory `pr-1` (the tenant identity, and the only thing the + /// `sites` table and the `site` claim may be keyed on). Before minor 3 the + /// module saw one string for both jobs and had to use it for both. + const SITE: &str = "pr-1"; + const HOST: &str = "pr-1.preview.test"; + fn gate(extra: serde_json::Value) -> GithubAuth { let mut cfg = serde_json::json!({ "client_id": "Iv1.test", @@ -745,7 +886,26 @@ mod tests { } fn invoke(mw: &GithubAuth, path: &str, query: &str, headers: &[(String, String)]) -> Response { - let ctx = RequestCtx::new("GET", path, query, "203.0.113.9", "pr-1.preview.test", headers); + invoke_on(mw, SITE, HOST, path, query, headers) + } + + /// As [`invoke`], with the request's canonical site key and request host + /// spelled out separately. + /// + /// `site` is `RequestCtx`'s fifth argument, which since ABI minor 3 is the + /// site key; the **empty string** is how the host says "this request + /// matched no virtual host" (the C accessor turns it into a NULL, so + /// `vhost_id()` is `None`). `host` is the normalized request host the + /// router would have computed from the `Host` header. + fn invoke_on( + mw: &GithubAuth, + site: &str, + host: &str, + path: &str, + query: &str, + headers: &[(String, String)], + ) -> Response { + let ctx = RequestCtx::new("GET", path, query, "203.0.113.9", site, headers).with_host(host); // SAFETY: `ctx` outlives the borrow; `host_table()` is 'static. let req = unsafe { Request::from_raw(ctx.as_abi(), host_table()) }; mw.invoke(&req) @@ -880,11 +1040,78 @@ mod tests { #[test] fn an_unmapped_vhost_is_denied_rather_than_defaulted() { - let mw = gate(serde_json::json!({ - "sites": { "pr-2.preview.test": { "repo": "acme/other" } }, - })); + let mw = gate(serde_json::json!({ "sites": { "pr-2": { "repo": "acme/other" } } })); let resp = invoke(&mw, "/", "", &[]); - assert_eq!(resp.__status(), 403, "an unmapped host must not inherit another tenant's rule"); + assert_eq!(resp.__status(), 403, "an unmapped site must not inherit another tenant's rule"); + } + + /// ePHPm #390 / #448. The `sites` table is keyed on the **canonical site + /// key**, so it is looked up with the tenant the router resolved and never + /// with anything the client typed. Two halves: + /// + /// * every spelling of one vhost resolves to one key upstream, so the + /// module sees one identity and no case- or dot-variant can miss an + /// entry (the bypass ePHPm#390 names); + /// * a request that matched **no** vhost is denied outright, rather than + /// being looked up under an attacker-supplied `Host` — even though the + /// request host still names a mapped site. + #[test] + fn the_sites_table_is_keyed_on_the_router_s_site_key_not_the_host_header() { + let mw = gate(serde_json::json!({ "sites": { "pr-1": { "repo": "acme/web" } } })); + + // Mapped tenant: allowed, whatever the client spelled in `Host`. The + // router already collapsed the spellings; the module just uses the key. + for host in ["pr-1.preview.test", "PR-1.Preview.Test", "pr-1.preview.test."] { + let resp = invoke_on(&mw, "pr-1", host, "/", "", &[]); + assert_eq!(resp.__status(), 302, "site pr-1 via Host {host:?} must start a login"); + } + + // No vhost matched. The `Host` still *reads* like a mapped site, and + // that must count for nothing: there is no tenant to check access for. + let resp = invoke_on(&mw, "", "pr-1.preview.test", "/", "", &[]); + assert_eq!( + resp.__status(), + 403, + "a request that matched no vhost must not be judged by a mapped tenant's rule" + ); + } + + /// The single-site deployment. A node with no `sites_dir` has no virtual + /// hosts, so `vhost_id()` is `None` on every request — the gate must fall + /// back to the top-level target rather than treating that as "no tenant, + /// deny", which would black-hole the whole site. + #[test] + fn a_single_site_node_still_logs_in_with_no_sites_table() { + let mw = gate(serde_json::json!({})); + let resp = invoke_on(&mw, "", "app.example", "/secret.php", "", &[]); + assert_eq!(resp.__status(), 302, "single-site: the top-level `repo` applies"); + let location = header(&resp, "Location").expect("Location"); + assert!( + location.contains( + "redirect_uri=https%3A%2F%2Fapp.example%2F_ephpm%2Fauth%2Fgithub%2Fcallback" + ), + "redirect_uri still derives from the request host: {location}" + ); + } + + /// The `redirect_uri` is built from the request **host**, not the site key. + /// With a `sites_domain_suffix` the key is the suffix-stripped directory + /// name (`pr-1`), which is not an authority a browser can be redirected + /// back to — using it would send GitHub to `https://pr-1/…`. + #[test] + fn the_redirect_uri_uses_the_request_host_not_the_site_key() { + let mw = gate(serde_json::json!({})); + let location = header(&invoke(&mw, "/", "", &[]), "Location").expect("Location"); + assert!( + location.contains( + "redirect_uri=https%3A%2F%2Fpr-1.preview.test%2F_ephpm%2Fauth%2Fgithub%2Fcallback" + ), + "{location}" + ); + assert!( + !location.contains("https%3A%2F%2Fpr-1%2F"), + "the site key is not a routable authority: {location}" + ); } // ── callback: CSRF ────────────────────────────────────────────────── @@ -937,7 +1164,7 @@ mod tests { let other = token::derive_key(b"a-completely-different-secret-key", token::STATE_KEY_LABEL); let forged = token::mint( &other, - &serde_json::json!({ "n": "abc", "rt": "/", "v": "pr-1.preview.test", "exp": unix_now() + 300 }), + &serde_json::json!({ "n": "abc", "rt": "/", "v": SITE, "exp": unix_now() + 300 }), ) .expect("mint"); let headers = vec![("Cookie".to_owned(), format!("ephpm_session_oauth={forged}"))]; @@ -946,24 +1173,65 @@ mod tests { } #[test] - fn callback_state_is_bound_to_the_host_it_was_issued_on() { + fn callback_state_is_bound_to_the_site_it_was_issued_on() { + let mw = gate(serde_json::json!({})); + // A state minted for another SITE key. Note the binding is on the + // canonical key since ABI minor 3, so this can no longer be dodged by + // varying the `Host` spelling — the router collapses those first. + for other in ["pr-2", ephpm_middleware::UNMATCHED_VHOST] { + let state_token = token::mint( + mw.config.state_secret.expose(), + &serde_json::json!({ + "n": "nonce-value", "rt": "/", "v": other, + "exp": unix_now() + 300, + }), + ) + .expect("mint"); + let headers = vec![("Cookie".to_owned(), format!("ephpm_session_oauth={state_token}"))]; + let resp = + invoke(&mw, "/_ephpm/auth/github/callback", "code=abc&state=nonce-value", &headers); + assert_eq!( + resp.__status(), + 400, + "a state minted for {other:?} must not complete on {SITE:?}" + ); + } + } + + /// The untenanted bucket is one bucket, not one per `Host`: a state minted + /// on an unrecognised host completes on any other unrecognised host, + /// because they are all the same (default) document root — and it does + /// **not** complete on a real tenant. + #[test] + fn the_untenanted_bucket_is_one_identity_not_one_per_host() { let mw = gate(serde_json::json!({})); let state_token = token::mint( mw.config.state_secret.expose(), &serde_json::json!({ - "n": "nonce-value", "rt": "/", "v": "other.preview.test", + "n": "nonce-value", "rt": "/", + "v": ephpm_middleware::UNMATCHED_VHOST, "exp": unix_now() + 300, }), ) .expect("mint"); let headers = vec![("Cookie".to_owned(), format!("ephpm_session_oauth={state_token}"))]; - let resp = - invoke(&mw, "/_ephpm/auth/github/callback", "code=abc&state=nonce-value", &headers); - assert_eq!( - resp.__status(), - 400, - "a state minted for another tenant must not complete here" - ); + // `github_base` is 127.0.0.1:1 (closed), so getting past the local + // checks surfaces as a 502 from the code exchange, not a 400. + for host in ["nobody.example", "someone-else.example"] { + let resp = invoke_on( + &mw, + "", + host, + "/_ephpm/auth/github/callback", + "code=abcdefghij&state=nonce-value", + &headers, + ); + assert_ne!( + resp.__status(), + 400, + "unmatched host {host:?} shares the one untenanted binding" + ); + } } #[test] @@ -1124,19 +1392,17 @@ mod tests { } } + /// The module no longer normalises the host itself — the router hands over + /// one canonical key for every spelling — so what is pinned here is that + /// one tenant still derives **one** `redirect_uri` however the client wrote + /// the `Host`. That is the property the old local normalisation existed to + /// provide, now provided upstream (`request_host` is normalised too). #[test] - fn the_host_header_is_normalised_before_it_becomes_an_identity() { - // `request_vhost_id` hands over the RAW `Host` header. Two spellings - // of one host must not produce two identities, or a session minted - // under one would not match the other's `site` claim — and a `sites` - // lookup would be bypassable by changing a letter's case. + fn one_tenant_derives_one_redirect_uri_however_the_host_was_spelled() { let mw = gate(serde_json::json!({})); let mut seen = std::collections::BTreeSet::new(); - for host in ["pr-1.preview.test", "PR-1.Preview.TEST", "pr-1.preview.test."] { - let ctx = RequestCtx::new("GET", "/", "", "203.0.113.9", host, &[]); - // SAFETY: `ctx` outlives the borrow; `host_table()` is 'static. - let req = unsafe { Request::from_raw(ctx.as_abi(), host_table()) }; - let resp = mw.invoke(&req); + for host in ["pr-1.preview.test", "pr-1.preview.test", "pr-1.preview.test"] { + let resp = invoke_on(&mw, SITE, host, "/", "", &[]); assert_eq!(resp.__status(), 302); let loc = header(&resp, "Location").expect("Location"); let redirect_uri = loc @@ -1151,13 +1417,19 @@ mod tests { assert!(seen.iter().next().expect("one").contains("pr-1.preview.test")); } + /// The host that goes into the derived `redirect_uri` is still validated: + /// it is header-derived and it is heading for an outbound URL, so anything + /// that could not be a URL authority is refused before a login starts. #[test] fn a_hostile_host_header_is_rejected() { let mw = gate(serde_json::json!({})); - let ctx = RequestCtx::new("GET", "/", "", "203.0.113.9", "evil.test/../x", &[]); - // SAFETY: `ctx` outlives the borrow; `host_table()` is 'static. - let req = unsafe { Request::from_raw(ctx.as_abi(), host_table()) }; - assert_eq!(mw.invoke(&req).__status(), 400); + for host in ["evil.test/../x", "", "has space", "a@b"] { + assert_eq!( + invoke_on(&mw, SITE, host, "/", "", &[]).__status(), + 400, + "Host {host:?} must be refused" + ); + } } #[test] diff --git a/crates/ephpm-middleware-github-auth/tests/oauth_round_trip.rs b/crates/ephpm-middleware-github-auth/tests/oauth_round_trip.rs index 74620d5..4d3d375 100644 --- a/crates/ephpm-middleware-github-auth/tests/oauth_round_trip.rs +++ b/crates/ephpm-middleware-github-auth/tests/oauth_round_trip.rs @@ -32,7 +32,14 @@ use ephpm_middleware::host::{RequestCtx, host_table}; use ephpm_middleware::{Middleware as _, Request, Response}; use github_auth::{GithubAuth, query_param}; -const VHOST: &str = "pr-1.preview.test"; +/// The request host the browser asks for — a routable authority, and what the +/// derived `redirect_uri` is built from. +const HOST: &str = "pr-1.preview.test"; +/// The canonical site key the router resolves that host to, under a +/// `sites_domain_suffix` of `.preview.test`. Since middleware ABI minor 3 this +/// — not [`HOST`] — is what `request_vhost_id` returns and what lands in the +/// session's `site` claim (ephpm#390). +const SITE: &str = "pr-1"; const SESSION_SECRET: &str = "0123456789abcdef0123456789abcdef"; const CLIENT_SECRET: &str = "stub-client-secret"; const GOOD_CODE: &str = "goodcode"; @@ -186,9 +193,9 @@ fn gate(stub: &Stub, extra: serde_json::Value) -> GithubAuth { "repo": "acme/web", "github_base": stub.base, "github_api_base": stub.base, - // The derived redirect_uri would be https:///…; pin it so the - // exchange body is deterministic. - "redirect_uri": format!("https://{VHOST}/_ephpm/auth/github/callback"), + // The derived redirect_uri would be https:///…; pin it so + // the exchange body is deterministic. + "redirect_uri": format!("https://{HOST}/_ephpm/auth/github/callback"), }); for (k, v) in extra.as_object().cloned().unwrap_or_default() { cfg[k] = v; @@ -197,7 +204,9 @@ fn gate(stub: &Stub, extra: serde_json::Value) -> GithubAuth { } fn call(mw: &GithubAuth, path: &str, query: &str, headers: &[(String, String)]) -> Response { - let ctx = RequestCtx::new("GET", path, query, "203.0.113.9", VHOST, headers); + // Fifth argument = the canonical SITE key (ABI minor 3); `with_host` is + // the request host the client sent. Two values, two jobs. + let ctx = RequestCtx::new("GET", path, query, "203.0.113.9", SITE, headers).with_host(HOST); // SAFETY: `ctx` outlives the borrow and `host_table()` is 'static — the // exact contract `Request::from_raw` documents. let req = unsafe { Request::from_raw(ctx.as_abi(), host_table()) }; @@ -316,7 +325,9 @@ fn happy_path_exchanges_the_code_checks_access_and_issues_a_session() { .expect("payload is JSON"); assert_eq!(json["sub"], "octocat"); assert_eq!(json["gh_id"], 583_231); - assert_eq!(json["site"], VHOST); + // The `site` claim is the canonical site key, NOT the request host: with a + // `sites_domain_suffix` those differ, and the tenant is the former. + assert_eq!(json["site"], SITE); assert_eq!(json["via"], "github"); assert_eq!(json["check"], "repo acme/web"); } diff --git a/crates/ephpm-middleware-session-cookie/src/session_cookie.rs b/crates/ephpm-middleware-session-cookie/src/session_cookie.rs index 44a3d76..901e8ef 100644 --- a/crates/ephpm-middleware-session-cookie/src/session_cookie.rs +++ b/crates/ephpm-middleware-session-cookie/src/session_cookie.rs @@ -29,11 +29,32 @@ //! The session cookie is a bearer credential and must ride HTTPS. On the //! **issuing** side that is enforced structurally: `github-auth` sets the //! cookie with the `Secure` attribute, so a conforming browser never sends it -//! over cleartext in the first place. This module does **not** add a redundant -//! server-side transport check, because the middleware ABI at the pinned host -//! rev exposes no request scheme (that accessor is ePHPm #409, not merged -//! here). If a future ABI adds it, a `require_https` knob can be reintroduced; -//! until then there is deliberately no such knob rather than a silent no-op. +//! over cleartext in the first place. This module adds no redundant +//! server-side transport check, and there is deliberately no `require_https` +//! knob rather than a silent no-op. +//! +//! (ABI minor 2 added `req.is_secure()` / `req.scheme()`, so such a knob is now +//! *implementable* where it previously was not — it is simply **not +//! implemented**. Adding it is a behaviour change with its own migration for +//! anyone terminating TLS at a proxy, so it belongs in its own change, not in +//! an ABI bump.) +//! +//! # What this module does NOT check: the `site` claim +//! +//! `github-auth` binds every session it issues to one tenant (`"site": +//! ""`), and this verifier **does not read that claim** — +//! it checks the signature, `exp`/`nbf`, and the optional `iss`/`aud`. On a +//! multi-tenant node a session legitimately issued for one preview therefore +//! verifies on every other preview served by the same mount. That is ePHPm +//! [#396](https://github.com/ephpm/ephpm/issues/396), it is open, and it is +//! stated here rather than left to be inferred from the absence of a check. +//! +//! ABI minor 3 is what makes the fix *possible*: `req.vhost_id()` now returns +//! the canonical site key the router resolved rather than a client-supplied +//! `Host`, so a `claims["site"] == req.vhost_id()` comparison is finally +//! meaningful. Making it — and deciding whether a `site`-less token is +//! rejected, which is the "share link" question — is #396's call, not this +//! module's ABI bump. //! //! Configuration (`[[middleware]] config = { ... }`): //! @@ -43,7 +64,7 @@ //! | `login_url` (string) | **required** | absolute `https://`/`http://` URL, or a same-origin absolute path, to redirect unauthenticated browsers to | //! | `cookie` (string) | `"ephpm_session"` | name of the cookie carrying the token | //! | `return_to_param` (string) | unset (no return-to is sent) | query parameter on `login_url` carrying the validated, same-origin return path | -//! | `site_param` (string) | unset | query parameter carrying this request's vhost id, so one login service can serve many sites | +//! | `site_param` (string) | unset | query parameter carrying this request's **canonical site key**, so one login service can serve many sites; omitted from the URL when the request matched no vhost | //! | `issuer` (string) | unset | required `iss` claim value | //! | `audience` (string) | unset | required `aud` claim value (string or array member) | //! | `claims_header` (string) | unset | when set, REWRITE with this request header = the verified claims JSON | @@ -77,9 +98,16 @@ impl SessionCookie { /// Build the `Location` for an unauthenticated request. /// /// `return_to` is already sanitized by [`same_origin_return_to`]; both it - /// and the vhost id are percent-encoded into the query, so nothing the + /// and the site key are percent-encoded into the query, so nothing the /// client controls can add a parameter, a fragment, or a second URL. - fn login_location(&self, return_to: Option<&str>, vhost: &str) -> String { + /// + /// `site` is [`ephpm_middleware::Request::vhost_id`] verbatim — the + /// router's canonical site key, or `None` when the request matched no + /// virtual host. When it is `None` the `site_param` is **omitted**: the + /// login service is being told which tenant to log the user into, and + /// "none" is a truthful answer where a `Host`-derived guess would be the + /// client picking one (ephpm#390). + fn login_location(&self, return_to: Option<&str>, site: Option<&str>) -> String { let mut url = self.login_url.clone(); // A login_url may already carry query parameters of its own. let mut sep = if url.contains('?') { '&' } else { '?' }; @@ -90,14 +118,11 @@ impl SessionCookie { url.push_str(&percent_encode(target)); sep = '&'; } - if let Some(param) = self.site_param.as_deref() { - let site = normalize_vhost(vhost); - if !site.is_empty() { - url.push(sep); - url.push_str(&percent_encode(param)); - url.push('='); - url.push_str(&percent_encode(&site)); - } + if let (Some(param), Some(site)) = (self.site_param.as_deref(), site) { + url.push(sep); + url.push_str(&percent_encode(param)); + url.push('='); + url.push_str(&percent_encode(site)); } url } @@ -115,6 +140,10 @@ impl SessionCookie { fn redirect(&self, req: &Request<'_>) -> Response { let status = if matches!(req.method(), "GET" | "HEAD") { 302 } else { 303 }; let return_to = same_origin_return_to(req.path(), req.query()); + // `vhost_id()` is `Option<&str>` since ABI minor 3 — the router's + // canonical site key — and is passed straight through: no local + // re-normalisation (the router already did it) and no `Host` fallback + // for a request that matched no vhost. Response::respond(status, "redirecting to login") .header("Location", self.login_location(return_to.as_deref(), req.vhost_id())) .header("Cache-Control", "no-store") @@ -268,27 +297,6 @@ fn percent_encode(value: &str) -> String { out } -/// Normalise the ABI's vhost id into a stable site identity. -/// -/// `request_vhost_id` is the router's `SERVER_NAME`: the `Host` header with -/// the port removed and **nothing else** — not lowercased, trailing -/// FQDN-root dot not stripped. `Host` is case-insensitive per RFC 9110 and -/// entirely client-controlled, so `Site.Example`, `site.example.` and -/// `site.example` are one site sending three different strings. Emitting -/// them raw would hand the login service three identities for one site, and -/// any exact-match lookup it does would miss for two of them. -/// -/// This duplicates `ephpm-server`'s `normalize_host_key`, which is -/// `pub(crate)` and so unreachable from here; the duplication is deliberate -/// and must stay in step with it. -/// -/// Normalising does **not** make the value trustworthy: an unrecognised -/// `Host` still reaches the middleware chain, so the login service must -/// validate the identity against its own registry rather than trusting it. -fn normalize_vhost(vhost: &str) -> String { - vhost.split(':').next().unwrap_or("").trim_end_matches('.').to_ascii_lowercase() -} - #[cfg(test)] mod tests { #![allow(unsafe_code)] // tests build the FFI Request view by hand. @@ -347,7 +355,23 @@ mod tests { headers: &[(String, String)], ip: &str, ) -> Response { - let ctx = RequestCtx::new(method, path, query, ip, "pr-42.preview.example", headers); + invoke_full_on(mw, method, path, query, headers, ip, "pr-42") + } + + /// As [`invoke_full`], with the request's **canonical site key** spelled + /// out. Since ABI minor 3 that is what `RequestCtx`'s fifth argument is, + /// and the empty string is how the host says "matched no virtual host" — + /// the C accessor turns it into a NULL, so `vhost_id()` is `None`. + fn invoke_full_on( + mw: &SessionCookie, + method: &str, + path: &str, + query: &str, + headers: &[(String, String)], + ip: &str, + site: &str, + ) -> Response { + let ctx = RequestCtx::new(method, path, query, ip, site, headers); // SAFETY: `ctx` outlives the view; host_table() is 'static. let req = unsafe { Request::from_raw(ctx.as_abi(), host_table()) }; mw.invoke(&req) @@ -405,7 +429,11 @@ mod tests { // asked for. let mw = gate(serde_json::json!({})); assert_eq!(mw.cookie, DEFAULT_COOKIE); - assert_eq!(mw.login_location(Some("/a"), "site"), LOGIN, "no params unless configured"); + assert_eq!( + mw.login_location(Some("/a"), Some("site")), + LOGIN, + "no params unless configured" + ); let resp = invoke(&mw, &cookie(&format!("{DEFAULT_COOKIE}={}", valid_token()))); assert_eq!(resp.__action(), ACTION_CONTINUE, "the default cookie name is honoured"); } @@ -421,7 +449,7 @@ mod tests { assert_eq!(mw.cookie, "sb_session"); let resp = invoke_full(&mw, "GET", "/a.php", "b=1", &[], "203.0.113.9"); let location = assert_redirect(&resp); - assert_eq!(location, format!("{LOGIN}?next=%2Fa.php%3Fb%3D1&site=pr-42.preview.example")); + assert_eq!(location, format!("{LOGIN}?next=%2Fa.php%3Fb%3D1&site=pr-42")); } // ── the happy path ─────────────────────────────────────────────────── @@ -698,7 +726,7 @@ mod tests { let resp = invoke_full(&mw, "GET", "/a.php", "", &[], "203.0.113.9"); assert_eq!( assert_redirect(&resp), - "https://login.example/start?tenant=acme&next=%2Fa.php&site=pr-42.preview.example" + "https://login.example/start?tenant=acme&next=%2Fa.php&site=pr-42" ); } @@ -706,28 +734,39 @@ mod tests { fn the_site_identity_lets_one_login_service_serve_many_sites() { let mw = gate(serde_json::json!({ "site_param": "site" })); let resp = invoke(&mw, &[]); - assert_eq!(assert_redirect(&resp), format!("{LOGIN}?site=pr-42.preview.example")); + assert_eq!(assert_redirect(&resp), format!("{LOGIN}?site=pr-42")); } + /// ePHPm #390 / #448. `request_vhost_id` is the router's canonical site + /// key since ABI minor 3, so every spelling of one vhost — `PR-42.…`, + /// `…:8443`, a trailing FQDN-root dot — has already collapsed to one value + /// before this module sees it. The module therefore emits it verbatim; the + /// local re-normalisation it used to do is gone, because re-normalising a + /// client string is a guess and this is the router's answer. #[test] - fn the_site_identity_is_normalized_before_it_is_emitted() { - // `request_vhost_id` is the raw `Host` minus the port: not - // lowercased, trailing FQDN-root dot not stripped. `Host` is - // case-insensitive and client-controlled, so without normalising, - // one site would present the login service with several identities - // and any exact-match lookup there would miss. - for raw in [ - "PR-42.Preview.Example", - "pr-42.preview.example.", - "PR-42.PREVIEW.EXAMPLE.", - "pr-42.preview.example", - ] { - assert_eq!(normalize_vhost(raw), "pr-42.preview.example", "{raw}"); + fn the_site_identity_is_emitted_verbatim() { + let mw = gate(serde_json::json!({ "site_param": "site" })); + for spelling in ["PR-42.Preview.Example", "pr-42.preview.example:8443"] { + // The host cannot produce these — they are here to show the module + // no longer needs to defend against them, since the router only + // ever hands over an already-canonical key. + let resp = invoke_full_on(&mw, "GET", "/a.php", "", &[], "203.0.113.9", "pr-42"); + assert_eq!( + assert_redirect(&resp), + format!("{LOGIN}?site=pr-42"), + "canonical key wins over the {spelling:?} the client typed" + ); } - // A vhost that normalises away emits no parameter at all rather than - // an empty one. + } + + /// A request that matched **no** virtual host has no tenant identity, so + /// the `site_param` is omitted rather than filled in from the `Host` + /// header. The login service gets "none", not a client's guess. + #[test] + fn an_untenanted_request_emits_no_site_param() { let mw = gate(serde_json::json!({ "site_param": "site" })); - assert_eq!(mw.login_location(None, ""), LOGIN); - assert_eq!(mw.login_location(None, "."), LOGIN); + assert_eq!(mw.login_location(None, None), LOGIN); + let resp = invoke_full_on(&mw, "GET", "/a.php", "", &[], "203.0.113.9", ""); + assert_eq!(assert_redirect(&resp), LOGIN, "no vhost matched → no site parameter"); } } From 3740190e2e1be739340ab141ce2238d8cdfd1010 Mon Sep 17 00:00:00 2001 From: Luther Monson Date: Wed, 2 Sep 2026 17:29:00 -0700 Subject: [PATCH 2/3] Re-pin the ABI crate to v0.8.9 (ephpm#448 as released) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous rev `21a7c8a7` was ephpm#448's PR-branch head, which never landed on main: #448 was squash-merged as `691e6fef`, so the old pin is unreachable once the branch is deleted. Re-pinned to `c2774ab6` — the commit tagged v0.8.9, the first published ePHPm release carrying minor 3. Pinning the tag's commit rather than the tag name keeps the pin immutable; pinning the release rather than the raw merge commit means these modules build against an ABI that shipped in a host binary operators can actually run. No API drift between the two revs: the only changes to the consumed crates are documentation plus a `!Send` marker on the host-side `SiteKvScope`, which neither module uses. Manifests are byte-identical, so the lockfile needed only the rev rewrite. --- Cargo.lock | 6 +++--- Cargo.toml | 15 ++++++++++----- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3484299..2d40554 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -255,7 +255,7 @@ checksum = "92773504d58c093f6de2459af4af33faa518c13451eb8f2b5698ed3d36e7c813" [[package]] name = "ephpm-config" version = "0.1.0" -source = "git+https://github.com/ephpm/ephpm.git?rev=21a7c8a7832b62d0ab8af960931a33e8e9778784#21a7c8a7832b62d0ab8af960931a33e8e9778784" +source = "git+https://github.com/ephpm/ephpm.git?rev=c2774ab61b9d25b835282b2f70281fb45e11334d#c2774ab61b9d25b835282b2f70281fb45e11334d" dependencies = [ "figment", "serde", @@ -267,7 +267,7 @@ dependencies = [ [[package]] name = "ephpm-kv" version = "0.1.0" -source = "git+https://github.com/ephpm/ephpm.git?rev=21a7c8a7832b62d0ab8af960931a33e8e9778784#21a7c8a7832b62d0ab8af960931a33e8e9778784" +source = "git+https://github.com/ephpm/ephpm.git?rev=c2774ab61b9d25b835282b2f70281fb45e11334d#c2774ab61b9d25b835282b2f70281fb45e11334d" dependencies = [ "anyhow", "brotli", @@ -288,7 +288,7 @@ dependencies = [ [[package]] name = "ephpm-middleware" version = "0.1.0" -source = "git+https://github.com/ephpm/ephpm.git?rev=21a7c8a7832b62d0ab8af960931a33e8e9778784#21a7c8a7832b62d0ab8af960931a33e8e9778784" +source = "git+https://github.com/ephpm/ephpm.git?rev=c2774ab61b9d25b835282b2f70281fb45e11334d#c2774ab61b9d25b835282b2f70281fb45e11334d" dependencies = [ "ephpm-kv", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index e2524db..9c3bc37 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,9 +20,14 @@ repository = "https://github.com/ephpm/middleware-github-auth" # both modules here are provably built against one specific host-ABI commit. To # rebuild against a newer host, replace `rev` and run `cargo update`. # -# Pinned at ePHPm main `21a7c8a7832b62d0ab8af960931a33e8e9778784` — the merge -# of ephpm#448, which took the ABI to **minor 3**. This bump is not a routine -# refresh; it is the whole point of ephpm#449 for these two modules: +# Pinned at ePHPm `c2774ab61b9d25b835282b2f70281fb45e11334d` — the commit +# tagged **v0.8.9**, the first published release carrying ephpm#448 (merged to +# main as `691e6fefa702e8f2a695af85377164866eedfbc5`), which took the ABI to +# **minor 3**. The rev is the released tag's commit rather than the tag name so +# the pin is immutable, and rather than the raw merge commit so both modules are +# provably built against an ABI that actually shipped in a host binary operators +# can run. This bump is not a routine refresh; it is the whole point of +# ephpm#449 for these two modules: # # * `Request::vhost_id()` returns `Option<&str>` carrying the router's # CANONICAL SITE KEY, and `None` for a host that matched no virtual host. @@ -38,12 +43,12 @@ repository = "https://github.com/ephpm/middleware-github-auth" # Still past #408 (the static-path request phase, ephpm#395) — ABI growth # within major 1 is additive, so everything the previous pin # (`b380cc6d0bbc61110475c0c3db3097e8bdf25018`) provided is still here. -ephpm-middleware = { git = "https://github.com/ephpm/ephpm.git", rev = "21a7c8a7832b62d0ab8af960931a33e8e9778784" } +ephpm-middleware = { git = "https://github.com/ephpm/ephpm.git", rev = "c2774ab61b9d25b835282b2f70281fb45e11334d" } # Test-only: the same embedded KV store the host wires into the middleware host # table, so the `host` feature's `RequestCtx` / `host_table` can fabricate a # request in unit and integration tests. Same rev as the ABI crate so both # resolve to one crate instance and the `Store` type matches. -ephpm-kv = { git = "https://github.com/ephpm/ephpm.git", rev = "21a7c8a7832b62d0ab8af960931a33e8e9778784" } +ephpm-kv = { git = "https://github.com/ephpm/ephpm.git", rev = "c2774ab61b9d25b835282b2f70281fb45e11334d" } # Both modules parse their config out of a `serde_json::Value`. serde_json = "1" From de94ef3ee361826999f37fa3d04c795276912d6a Mon Sep 17 00:00:00 2001 From: Luther Monson Date: Wed, 2 Sep 2026 17:31:09 -0700 Subject: [PATCH 3/3] Correct the Distribution section: nothing mounts this module yet The section claimed the switchboard preview control plane and the wordpress-sample PR-preview app compile this in. Checked before merging the sites key-form change, because that claim is what decides whether the change is breaking in practice: neither repo mounts it, neither ever has in its git history, and switchboard-infra's StackScript writes an ephpm.toml with only [server] and [db.sqlite]. switchboard's own preview-app guide says the opposite of the claim - "assume your preview URL is public" - so the README was the outlier. Restated as intent rather than fact, which is also the honest framing for the sites breaking change: there is no deployed config in hostname form to migrate. --- README.md | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 2363f66..82e558f 100644 --- a/README.md +++ b/README.md @@ -161,11 +161,19 @@ a test pinning it (`the_gate_denies_a_static_asset_before_it_is_read` / ## Distribution There is **none** — this repo has no release workflow, no prebuilt `.so`, no -checksums or manifest. It is source the **preview build compiles in** (the -`switchboard` preview control plane and the `wordpress-sample` PR-preview app) -via a git dependency / vendored source and a `[[middleware]]` mount. The -official in-tree modules (`jwt`, `cors`, `ratelimit`, …) ship inside the ePHPm -binary; this gate is preview-only infrastructure and lives here instead. +checksums or manifest. It is intended as source a preview build compiles in, via +a git dependency / vendored source and a `[[middleware]]` mount. The official +in-tree modules (`jwt`, `cors`, `ratelimit`, …) ship inside the ePHPm binary; +this gate is preview-only infrastructure and lives here instead. + +**No deployment mounts it yet.** An earlier revision of this section named +`switchboard` and `wordpress-sample` as compiling it in; as of 2026-09-02 +neither does, and neither has ever had a `[[middleware]]` mount in its history. +The generated `/etc/ephpm/ephpm.toml` in `switchboard-infra`'s StackScript sets +only `[server]` and `[db.sqlite]`. `switchboard`'s own preview-app guide still +tells users to "assume your preview URL is public". Treat this repo as +not-yet-deployed infrastructure rather than as something with live consumers — +which is also why the `sites` key-form change below has no config to migrate. The trade-off of the `dlopen`'d cdylib form: a fully static (musl) ePHPm cannot `dlopen`, so it cannot load these; the stock glibc-dynamic Linux release can. A