diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 25f4982..ab8093e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,8 +37,16 @@ jobs: # The add-on command is shell and holds the opinions the binary refuses # to: the slug, which checkout the map comes from, which hostnames web - # keeps. It needs no Docker and no DDEV, so it belongs here rather than in - # the e2e job that takes twenty minutes. + # keeps. It needs no Docker, no DDEV and no network, so it belongs here + # rather than in the e2e job that takes twenty minutes. + # + # The no-network half of that was untrue until it first ran here: seventy- + # four `check` cells left the real curl on PATH and asked the machine + # whether the variant answered, so eleven of them passed on a developer's + # laptop — where a local DDEV router answers on 443 — and failed on a + # runner where nothing does. The suite now poisons curl behind the fake one + # and asserts nothing fell through, so the property this comment claims is + # checked rather than assumed. - name: the add-on command run: make test-addon diff --git a/.gitignore b/.gitignore index 21f4ab0..775813e 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,11 @@ # local agent scaffolding, not part of the design record — PLAN.md is authoritative /GOAL.md + +# The design record and the pilot notes. Kept out of the public repo because +# they are written against named client deployments; the comments throughout the +# code cite them by section, and those citations still name the decision even +# where the document itself is not here to open. +/PLAN.md +/docs/m0-preflight.md +/docs/m6-pilot.md diff --git a/README.md b/README.md index b478cf6..eccb459 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,21 @@ Rewrite origins in HTTP traffic, in both directions. A site's content refers to one hostname; you want to reach it at another. hostshift maps between them: responses get the hostname the browser is on, requests get the hostname the content was written for. Nothing is rewritten at -rest — the database is never touched. +rest — hostshift never writes to the database itself. + +What the *application* writes does pass through it, though, and one thing +changes there: **the scheme**. A canonical is declared with a scheme, and the +matcher accepts either — so `http://www.acme.fi/legacy/` in a post becomes the +variant on the way out, and comes back as `https://www.acme.fi/legacy/`, because +nothing in the variant spelling records which scheme was written originally. +Save a post you did not otherwise edit and every plain URL in it takes the +canonical's declared scheme. That is an upgrade when the canonical is `https` +and a downgrade when it is `http`, which some staging environments are; PLAN §M0 +measured one fleet host appearing 165 times over `http` and never over `https`. +The data stays valid and serialized lengths are recomputed correctly — this is +the scheme and nothing else — but it is a real change to rows you did not +intend to touch, and `hostshift diff` cannot see it, because it only exercises +the response direction. It is a filter and a reverse proxy, it knows nothing about any CMS, and it scaffolds nothing: no config files written, no slugs guessed, no directories @@ -14,9 +28,18 @@ created. An optional DDEV add-on sits on top and does the opinionated part. **You have this problem if** one database has to be browsed at more than one hostname — a git worktree previewing a branch beside the main checkout, or a production dump you want to open locally without search-replacing it first. -**The one precondition hostshift cannot supply** is that the application derive -its host from the request. Bedrock does; a site that pins `WP_HOME` to a -constant cannot be proxied by hostshift or by anything else. +**The one precondition hostshift cannot supply** is that the application answer +to a hostname the map names. Deriving `WP_HOME` from the request satisfies that +for free; so does pinning it, as long as it is pinned to the canonical or the +variant. What cannot work is a site pinned to a *third* hostname — stock DDEV +WordPress does this, setting `WP_HOME` to `DDEV_PRIMARY_URL`, which is the +project's own name and neither side of the map. `ddev hostshift check` says so +when it sees it. + +(Stock Bedrock does not derive its host from the request either: its +`config/application.php` *requires* `WP_HOME` and defines it as a constant. The +`env('WP_HOME') ?: 'https://'.$host` form PLAN §4.1 quotes is a local edit, not +upstream. It works fine either way.) ## Install @@ -70,12 +93,20 @@ hostname, so a branch can be previewed without a database of its own. checkout goes on serving `acme.ddev.site` untouched. Whatever pulls the database keeps its search-replace; nothing about a normal pull changes. -Install the add-on once per project: +Install the add-on once per project, from a clone of this repository: ``` -ddev add-on get generoi/hostshift +git clone git@github.com:generoi/hostshift.git ~/src/hostshift # once, anywhere +ddev add-on get ~/src/hostshift/ddev # per project ``` +`ddev add-on get generoi/hostshift` does **not** work, for two reasons worth +stating rather than leaving you to discover: the repository is private, so +DDEV's registry lookup 404s; and DDEV expects `install.yaml` at the repository +root while hostshift keeps its add-on under `ddev/`, so even with +`DDEV_GITHUB_TOKEN` set the download fails with `Unable to read … install.yaml`. +The path form above is the supported one today. + Then, in the worktree, one command: ``` @@ -95,10 +126,10 @@ DDEV project of its own with nothing configured. ```console $ git worktree add ../acme-wt-a -b wt-a $ cd ../acme-wt-a -$ ddev add-on get generoi/hostshift +$ ddev add-on get ~/src/hostshift/ddev $ ddev hostshift init hostshift: slug "wt-a", from the git branch wt-a -hostshift: canonical hostnames from /Users/you/Projects/acme, whose database this shares +hostshift: canonical hostnames from /Users/you/Projects/acme, the checkout this was made from hostshift: wrote .ddev/.env map from --from/--to site1 https://acme.ddev.site -> https://wt-a--acme.ddev.site @@ -117,14 +148,20 @@ The one file that comes out of it: ```sh # .ddev/.env +#ddev-silent-no-warn HOSTSHIFT_ARGS=--from https://acme.ddev.site --to https://wt-a--acme.ddev.site HOSTSHIFT_VARIANTS=wt-a--acme.ddev.site HOSTSHIFT_WEB_HOSTS=acme-wt-a.ddev.site ``` -Ignore it in the project's own `.gitignore` — DDEV's generated -`.ddev/.gitignore` does not cover `.env`. `init` merges into the file rather -than truncating it, so anything else already in there survives. +The first line is not a comment DDEV ignores: without it every `ddev start` +prints a four-line "Custom configuration detected" block. `init` merges into +the file rather than truncating it, so anything else already in there survives. + +You do not need to gitignore it. Installing the add-on adds its files to +`.git/info/exclude`, which is per checkout and shared with linked worktrees, so +the ignore travels with the machine rather than with the branch. Removing the +add-on takes the entry back out. After `ddev restart`, `https://wt-a--acme.ddev.site` serves the worktree and `https://acme.ddev.site` goes on serving the parent. @@ -153,12 +190,55 @@ After `ddev restart`, `https://wt-a--acme.ddev.site` serves the worktree and nginx snippet that 302-redirects a missing `/app/uploads/` request to a hardcoded production origin. Rewriting that `Location` would send the browser back to the request it just made, so hostshift passes it through unmodified - and counts it as `self-redirect`. That is the single enumerated exception to - "no canonical origin reaches the browser"; `--strict-origins` returns 404 - instead. + and counts it as `self-redirect`. `--strict-origins` returns 404 instead. + + A JSON body over the 8 MB cap is another: it streams through untouched with + only a `WARN` in `ddev logs -s hostshift`, and under production-canonical every + origin in it reaches the browser. PLAN §5.8 decides the cap deliberately — + buffering an arbitrarily large body is the thing it exists to prevent — but + `/wp-json/wp/v2/posts?per_page=100` on a content-heavy site gets there. + + It is not the only exception. Tier 2 content types — `text/css` and + JavaScript — are excluded by design (PLAN §5.2), so an absolute canonical URL + inside a stylesheet reaches the browser unrewritten. That is a deliberate + decision on evidence from theme sources, and generated CSS under `uploads/` + (Elementor, WPBakery, the Customizer) is the case that evidence did not + survey; those files do carry absolute upload URLs. `hostshift diff` reports + them on its `Tier 2` line rather than failing the run, which is the trigger + PLAN's fast path names for rewriting them. [ddev/ddev#5486]: https://github.com/ddev/ddev/issues/5486 + +### If your repo pins `name:` + +Most do. A worktree inherits the tracked `.ddev/config.yaml`, so it inherits the +name, and DDEV refuses before hostshift is involved: + +```console +$ ddev add-on get ~/src/hostshift/ddev +Unable to get project : a project (web container) in running state already +exists for acme that was created at /Users/you/Projects/acme +``` + +Give the worktree a name of its own. Every command here runs **inside the +worktree** — running the `printf` in the parent renames the parent, and its +canonical hostname, the one the database holds, stops resolving. + +```console +$ git worktree add ../acme-wt-a -b wt-a +$ cd ../acme-wt-a +$ printf '#ddev-silent-no-warn\nname: acme-wt-a\n' > .ddev/config.hostshift-name.yaml +$ ddev add-on get ~/src/hostshift/ddev +$ ddev hostshift init +``` + +The name only has to be unique; hostshift derives the preview hostnames from the +parent's config and the slug, not from it. Between the `printf` and the add-on +install `git status` will list the file — that is expected, because the install +is what writes the `.git/info/exclude` rule that hides it. + + ## The map Resolved from three layers, each overriding the last. Discovery by probing is @@ -212,6 +292,15 @@ container resolves by name with nothing else configured. Note that a shared database is shared: previewing is safe, but activating a plugin, running a migration or uploading media writes to the real thing. +Uploads split in a way worth knowing about. The row goes to the shared database +and the *file* goes to the worktree's own `uploads/`, so the parent gets a row +pointing at an image that is not there. On a stock DDEV WordPress it is worse: +`wp-config-ddev.php` pins `WP_HOME` to this project's own hostname, so the +attachment's `guid` is computed from a name that is neither canonical nor +variant, and nothing will ever map it back. `ddev hostshift check` warns when +the served page carries that hostname; the fix is to remove the pin, not to +rewrite the row afterwards. + **Or give it one of its own**, which a branch that has to write needs. The fastest source is the parent, and it is the state you are already working against: @@ -230,13 +319,10 @@ before. ### `hostshift.yaml` — only for aliases, or for production-canonical -**`ddev restart` after editing it.** The proxy reads the file once, at startup, -and nothing detects that you have changed it since — `ddev hostshift check` -compares `.ddev/.env` and the running container's command line, and neither moves -when the file's contents do. Two attempts at detecting this were worse than the -gap: a checksum recorded at `init` time called a correctly-restarted project -stale, and comparing the file's timestamp against the container's proved -unreliable across platforms. +**`ddev restart` after editing it.** The proxy reads the file once, at startup. +`ddev hostshift check` catches it if you forget: the proxy prints its resolved +map to stderr when it starts, so `check` compares that against what this +checkout resolves to now and refuses to call a stale proxy healthy. Not needed because a site is a multisite. Needed for the two things a DDEV config genuinely cannot say: **alias hostnames**, so a residual `@staging` URL @@ -266,14 +352,25 @@ canonical=variant list cannot carry aliases. The same engine pointed further: set `canonical` to the live production hostname and a database that was never search-replaced at all can be browsed -locally. Opt-in per repo, and it is where the hazards live — see -[`PLAN.md`](PLAN.md) §4.4. Two things become required with it: - -- **Loopback containment.** List the site's production hostnames in - `.ddev/docker-compose.hostshift-loopback.yaml`, or WordPress's internal - requests (wp-cron, Site Health) leave the machine for live production. That - file carries no generated-file marker, so your edit survives the next - `ddev add-on get`. +locally. Opt-in per repo, and it is where the hazards live — see PLAN §4.4, +summarised below. Two things become required with it: + +- **Loopback containment.** `ddev hostshift loopback > .ddev/docker-compose.hostshift-loopback.yaml` + writes the site's production hostnames into web's `extra_hosts`, pointed at + 127.0.0.1. Without it WordPress's internal requests (wp-cron, Site Health) + leave the machine for live production — with `sslverify => false`, against a + database that believes it is production. The file ships with `www.example.com` + in it as a placeholder, so generate it rather than assuming its presence means + anything; `ddev hostshift check` warns when a canonical hostname is not pinned + to the loopback in web's `extra_hosts` — a comparison against the container's + configuration, not a reachability probe. It carries no generated-file marker, + so a hand edit survives the next `ddev add-on get`. + + Note the redirect **replaces** the file. If you have added hosts of your own — + a CDN origin, a legacy domain, an apex sibling that is not in `hostshift.yaml` + — regenerating discards them along with the file's own explanatory header. + Keep those additions somewhere you can re-apply, or edit the file by hand + instead and use the generated output as a reference. - **WP-CLI.** `ddev hostshift wp-cli > wp-cli.local.yml`, gitignored. WP-CLI resolves a site by URL, so without it every `ddev wp` on a multisite fails with "Site not found". It emits the project's existing `wp-cli.yml` back @@ -325,13 +422,76 @@ Flags worth knowing: default; it exists for performance work where transfer size must resemble production. -Exit codes: 0 success, 1 runtime error, 2 invalid configuration. +Exit codes: 0 success, 1 runtime error, 2 invalid configuration. These are the +binary's. DDEV collapses a host command's status to 1, so a script testing +`ddev hostshift check` for `-eq 2` will never match — test for non-zero, or call +`hostshift` directly. `hostshift diff -n 20` is the check that validates a deployment against reality: it crawls N pages canonically, fetches the same N through the proxy, -runs the canonical bytes through the same engine, and compares. The assertions -that fail a run are the ones that cannot be innocent — a canonical origin -reaching the browser, and a page whose line count changed. +runs the canonical bytes through the same engine, and compares. In a worktree +it needs `--canonical-base` to say what the canonical side is, since the +production hostname is not routed locally: + +``` +hostshift diff -n 20 --slug --canonical-base https://.ddev.site +``` + +`--slug` is not optional here. Without it the worktree's map has no variant side +to derive — `hostshift diff` exits 2 with *no variant — pass --slug, or declare +`variant:` on the site* — and there is no `ddev hostshift diff` wrapper to supply +it for you. + +**On a worktree whose map comes from DDEV config alone**, add `--variant-base` +too. The variant is derived from the project the command is run in, so in +`acme-wt-a` it comes out `wt-a--acme-wt-a.ddev.site` — a hostname nothing serves, +and every row a 404: + +``` +hostshift diff -n 20 --slug wt-a \ + --canonical-base https://acme.ddev.site \ + --variant-base https://wt-a--acme.ddev.site +``` + +Under production-canonical `--variant-base` is not needed — the map names the +variant — but `--slug` still is, unless the site declares `variant:` outright +rather than deriving it from `base:`. The README's own `hostshift.yaml` derives +it, so `--slug` applies there too. + +When you pass one base, `diff` warns if it is not a hostname its map knows and +says what it fell back to. When you pass **both** and the map knows neither — +the worktree case above — the two bases *are* the comparison, and `diff` says so +and uses them as its map. That is what makes the worktree form mean anything: +the bases used to move only the crawl, while the rewriting map still came from +`--slug`, so the leak scan looked for an origin that could not occur and printed +`0 leaks` over pages that were full of them. + +The assertions that fail a run are the ones that cannot be innocent — a canonical origin +reaching the browser, a serialized value served with a length that does not +describe its data, which PHP will refuse or silently truncate, a page whose byte +count moved by more than a quarter — which is how an upstream that answers 200 +with an empty body, or dies mid-stream, is caught — and a page whose +line count moved by more than a Host-dependent line could explain (over eight +lines, or over a quarter of the page). A one-line difference is reported and +does not fail the run: the two fetches carry different `Host` headers, so +WordPress emits one extra `` on every page of a +healthy production-canonical site. + +That one-line expectation assumes `WP_HOME` is **pinned to the canonical**. If +you derive it from the request — which this README recommends above, and which +is right for serving — then under production-canonical the only baseline you can +fetch locally, `.ddev.site`, emits *that* hostname throughout instead of +the database's. Every page then differs by a tenth of its lines, reported as +`N lines differ (dynamic content?)`, and the run is still GREEN. Measured: 20–29 +lines a page against 0–1 with `WP_HOME` pinned. A real re-serialisation sits +inside that noise indistinguishably, so pin `WP_HOME` for the run you intend to +read, or compare against a checkout that has it pinned. + +That last one is asserted on the served bytes alone, not by comparing the proxy +against the engine. Every other check here compares the two, so when both are +wrong in the same way the run is green — which is how five consecutive rounds of +silent `wp_options` destruction went unreported by the one check that validates +against reality. ## Building and testing @@ -353,6 +513,11 @@ not let it go red. ## Design -[`PLAN.md`](PLAN.md) is the authoritative design and is not re-decided here. -[`docs/`](docs/) has the pilot notes and the performance numbers; +`PLAN.md` is the authoritative design and is not re-decided here. It is written +against named client deployments and so is not published, which is why comments +throughout the code cite sections of a document this repo does not contain — +`PLAN §4.3` is the shared-database invariant, `§4.4` the production-canonical +hazards, `§5.2` the identity map. The citation still names the decision. + +[`docs/performance.md`](docs/performance.md) has the numbers; [`spike/`](spike/) is the evidence behind the Go decision. MIT licensed. diff --git a/cmd/hostshift/audit_r41_test.go b/cmd/hostshift/audit_r41_test.go new file mode 100644 index 0000000..993e3e2 --- /dev/null +++ b/cmd/hostshift/audit_r41_test.go @@ -0,0 +1,153 @@ +package main + +import ( + "strings" + "testing" +) + +// The live-crawl guardrail and the dialer must agree about what `--resolve` +// covers, and they normalise in opposite directions. +// +// cmdDiff decides whether to warn with: +// +// if ok && strings.EqualFold(h, cb.Hostname()) && p == port { +// +// while corpus.Run's DialContext decides whether the fetch is redirected with +// an exact map lookup: +// +// if to, ok := o.Resolve[addr]; ok { +// +// where `addr` is what net/http hands the dialer — which is the *punycode* +// host (Transport calls idnaASCII on the URL host before it builds the connect +// address) with its case preserved. +// +// So the guard folds case and the dialer does not; the dialer folds IDNA and +// the guard does not. Both directions are wrong, and one of them is the +// direction that matters: the guard reports "covered" and stays silent while +// the crawl falls through to real DNS and fetches -n pages from the client's +// live production site. That is what HEAD's own commit message calls the +// unacceptable failure — "a guardrail a typo can disable is worse than none, +// because it reads as confirmation" — for the two spellings it did check, and +// it is still true for these two. +// +// Measured against `www.hämeenlinna.fi` on the machine this was written: with +// `--resolve www.hämeenlinna.fi:443:127.0.0.1:9` hostshift printed no warning +// and the canonical fetch returned `status 301` from the live site. The hosts +// here are under `.invalid` (RFC 2606, guaranteed NXDOMAIN) so the test itself +// never leaves the machine. +// +// The assertion is the invariant rather than a table of expected verdicts: the +// warning must be silent exactly when the fetch was in fact redirected. A +// spelling that silences the warning without redirecting the fetch is the leak; +// a spelling that redirects the fetch and warns anyway is the false alarm that +// teaches a developer to scroll past it. +func TestTheLiveCrawlWarningDescribesWhereTheCrawlActuallyGoes(t *testing.T) { + const variant = "https://wt-a--client.ddev.site" + // 127.0.0.1:9 is discard: a redirected fetch fails with "connect: + // connection refused" naming that address, and one that was not redirected + // fails with "no such host". The two are distinguishable in the report, + // which is what makes this test able to check the dialer rather than + // assume it. + const local = "127.0.0.1:9" + + cases := map[string]struct{ base, resolve string }{ + // The control. Lowercase ASCII, right host, right port: this is the + // spelling the existing test covers, and it works. + "a plain host": { + "https://www.example.invalid", + "www.example.invalid:443:" + local, + }, + // An IDN canonical, written the way a person writes it — and the way + // PLAN's own prose writes .fi client domains. net/http punycodes it + // before dialling, so this key never matches. + "an IDN spelled in unicode": { + "https://www.hämeenlinna.invalid", + "www.hämeenlinna.invalid:443:" + local, + }, + // The same IDN spelled the way the dialer will ask for it. This one + // does redirect the fetch, and the guard warns anyway. + "an IDN spelled in punycode": { + "https://www.hämeenlinna.invalid", + "www.xn--hmeenlinna-q5a.invalid:443:" + local, + }, + // A hostname copied out of somewhere that upper-cased it. EqualFold in + // the guard says covered; the exact map lookup in the dialer does not. + "a host in a different case": { + "https://www.example.invalid", + "WWW.EXAMPLE.INVALID:443:" + local, + }, + // The other branch of the invariant, so the table is not degenerate: + // a --resolve that genuinely does not cover this crawl. Every spelling + // above is a *misspelling* of the right host, and once those are folded + // they all redirect — leaving nothing to exercise the "warned, and + // rightly" side. This is the curl mistake the guard exists for. + "a --resolve for another host entirely": { + "https://www.example.invalid", + "other.invalid:443:" + local, + }, + } + + // Which cases *should* land locally. The invariant below — silent iff the + // crawl went local — is true of a build where the guard and the dialer are + // both wrong in the same way, so it cannot be the only assertion: with the + // resolve map keyed on the raw spelling, the IDN case goes to DNS *and* + // warns, and the invariant holds while the flag does nothing. + shouldLandLocal := map[string]bool{ + "an IDN spelled in unicode": true, + "an IDN spelled in punycode": true, + "a host in a different case": true, + } + + var redirected, notRedirected int + for name, c := range cases { + code, out, errOut := run(t, "", cmdDiff, + "--canonical-base", c.base, + "--from", "https://www.example.fi", "--to", variant, + "-n", "1", "--timeout", "3s", "--resolve", c.resolve) + _ = code + + warned := strings.Contains(errOut, "not pointed anywhere local") + // Where the fetch actually went. Not inferred from the flag — read out + // of the failure the crawl reported. + wentLocal := strings.Contains(out, local) + wentToDNS := strings.Contains(out, "no such host") + if wentLocal == wentToDNS { + t.Fatalf("%s: the crawl's destination is not legible, so this test would "+ + "assert nothing:\nstdout:\n%s\nstderr:\n%s", name, out, errOut) + } + if wentLocal { + redirected++ + } else { + notRedirected++ + } + if want, ok := shouldLandLocal[name]; ok && wentLocal != want { + t.Errorf("%s: --resolve %q names the host being crawled, so the fetch "+ + "should have gone to %s; it went to real DNS instead, which means "+ + "the flag silently did nothing\nstdout:\n%s", name, c.resolve, local, out) + } + + // The invariant. Silent means "this crawl is pointed somewhere local", + // and that has to be true. + if warned == wentLocal { + if wentLocal { + t.Errorf("%s: --resolve %q sent the crawl to %s and hostshift warned "+ + "that it was \"not pointed anywhere local\" anyway — a false alarm "+ + "on the one message that must stay worth reading\nstderr:\n%s", + name, c.resolve, local, errOut) + } else { + t.Errorf("%s: --resolve %q did NOT cover the crawl — it fell through to "+ + "real DNS — and hostshift printed no warning, so under "+ + "production-canonical it would have fetched the client's live site "+ + "while reading as confirmation that it had not\nstdout:\n%s", + name, c.resolve, out) + } + } + } + + // Not a degenerate table: both outcomes are present, so neither branch of + // the invariant above is unreachable. + if redirected == 0 || notRedirected == 0 { + t.Fatalf("the table exercised only one outcome (%d redirected, %d not), "+ + "so the invariant was only half-tested", redirected, notRedirected) + } +} diff --git a/cmd/hostshift/audit_r42_test.go b/cmd/hostshift/audit_r42_test.go new file mode 100644 index 0000000..b6bacbd --- /dev/null +++ b/cmd/hostshift/audit_r42_test.go @@ -0,0 +1,249 @@ +package main + +import ( + "strings" + "testing" +) + +// Round 42. `check` gained a note in 94d2cd0 naming the hostname DDEV +// advertises, because under production-canonical `web` answers on it with the +// shared production database unrewritten — the URL `ddev start` prints, `ddev +// describe` lists and `ddev launch` opens is the one page on the machine where +// every link is the client's live site. +// +// The first version keyed on "this project's own hostname is a canonical of the +// map", which is not the production-canonical condition — it is *also* true of +// every stock DDEV project, where the canonicals are the `.ddev.site` hostnames +// by construction. `ddev hostshift check` is the post-start hook, so it printed +// a paragraph about production URLs on every `ddev start` of every project that +// has none. A warning that fires on healthy runs is the failure mode the other +// half of that same commit was written to fix. +// +// The condition is two things at once: some canonical is not a hostname of this +// project (the database holds URLs that dereference off the machine), and this +// project has hostnames that route to `web` rather than to the proxy (there is +// somewhere for them to be served from). + +// productionCanonical: a worktree with its own DDEV project, mapping the +// client's live domain onto a variant. This is the shape round 41 found: `ddev +// describe` hands over acme-wt-a.ddev.site, which is neither canonical nor +// variant, and web serves the shared database on it. +func productionCanonical(t *testing.T) string { + t.Helper() + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", "name: acme-wt-a\n") + writeFile(t, dir, "hostshift.yaml", + "sites:\n - canonical: https://www.acme.fi\n variant: https://wt-a--acme.ddev.site\n") + return dir +} + +func TestCheckNamesTheHostnameDDEVAdvertises(t *testing.T) { + _, _, errOut := run(t, "", cmdCheck, "-C", productionCanonical(t), "--slug", "wt-a") + for _, want := range []string{ + "canonical-on-production", + "www.acme.fi", + "https://acme-wt-a.ddev.site", + "https://wt-a--acme.ddev.site", + } { + if !strings.Contains(errOut, want) { + t.Errorf("the note does not mention %q:\n%s", want, errOut) + } + } +} + +// TestCheckIsSilentOnAStockProject: no hostshift.yaml, so the map is the DDEV +// defaults and every canonical is one of this project's own hostnames. The +// database holds `.ddev.site` URLs; nothing on that page leaves the machine. +// This is the ordinary `ddev start`, and it must say nothing. +func TestCheckIsSilentOnAStockProject(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", "name: stock\nadditional_hostnames:\n - nat\n") + _, _, errOut := run(t, "", cmdCheck, "-C", dir, "--slug", "wt-b") + if strings.Contains(errOut, "canonical-on-production") { + t.Errorf("a stock DDEV project is not canonical-on-production:\n%s", errOut) + } +} + +// TestCheckNamesEveryDirectlyServedHostname: DDEV registers every +// additional_hostname, `ddev describe` lists all of them, and each one is served +// by web. Naming only the first leaves the developer a live-production URL the +// note said nothing about — and DDEV_HOSTNAME's order is not something to rely +// on for which one that is. +func TestCheckNamesEveryDirectlyServedHostname(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", "name: acme-wt-a\nadditional_hostnames:\n - staff\n") + writeFile(t, dir, "hostshift.yaml", + "sites:\n - canonical: https://www.acme.fi\n variant: https://wt-a--acme.ddev.site\n") + _, _, errOut := run(t, "", cmdCheck, "-C", dir, "--slug", "wt-a") + for _, want := range []string{"https://acme-wt-a.ddev.site", "https://staff.ddev.site"} { + if !strings.Contains(errOut, want) { + t.Errorf("the note does not name %q:\n%s", want, errOut) + } + } +} + +// TestAVariantIsNotDirectlyServed: a map may point a canonical at a hostname the +// project already registers — `additional_hostnames: [preview]` with +// `variant: https://preview.ddev.site` is an ordinary way to ask for the preview +// at a fixed name rather than a slug-derived one. DDEV registers it, so it is in +// the project's hostnames, but the compose file's VIRTUAL_HOST narrowing routes +// it to the proxy rather than to web. +// +// It is therefore the one hostname on the project that is *not* serving the +// database unrewritten, and listing it under "every link on them points at the +// live site" would send the developer away from the only safe URL there is. +func TestAVariantIsNotDirectlyServed(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", "name: acme\nadditional_hostnames:\n - preview\n") + writeFile(t, dir, "hostshift.yaml", + "sites:\n - canonical: https://www.acme.fi\n variant: https://preview.ddev.site\n") + _, _, errOut := run(t, "", cmdCheck, "-C", dir, "--slug", "wt-a") + + note, _, _ := strings.Cut(errOut, "The variant(s) this map resolves to:") + if strings.Contains(note, "https://preview.ddev.site") { + t.Errorf("the variant is routed to the proxy, not to web:\n%s", errOut) + } + // The project's own name still is directly served, so the note stands. + if !strings.Contains(note, "https://acme.ddev.site") { + t.Errorf("the note should still name the project's own hostname:\n%s", errOut) + } +} + +// noNetwork points every host a diff test would fetch at a closed local port, so +// the crawl fails instantly instead of resolving and dialling the real internet. +// Without it these tests spend a second and a half each on a DNS lookup for a +// client's actual domain — from a unit suite, which is the wrong place to be +// making that request at all. +func noNetwork(hosts ...string) []string { + var out []string + for _, h := range hosts { + out = append(out, "--resolve", h+":443:127.0.0.1:1", "--resolve", h+":80:127.0.0.1:1") + } + return append(out, "--timeout", "2s") +} + +// TestDiffPairsTheBasesBySite: on a multisite, --canonical-base names one site +// and the variant side has to follow it. It used to override the canonical +// alone, so `--canonical-base ` was crawled against site 1's variant — +// two different sites, every page differing for the obvious reason, and the run +// printed GREEN. `diff` is the command the README calls the check that validates +// a deployment against reality. +// +// The crawl itself cannot reach anything here; the announced pair is what this +// asserts, and it is printed before the first fetch. +func TestDiffPairsTheBasesBySite(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", "name: acme\n") + writeFile(t, dir, "hostshift.yaml", ""+ + "sites:\n"+ + " - canonical: https://www.acme.fi\n variant: https://wt-a--acme.ddev.site\n"+ + " - canonical: https://shop.acme.fi\n variant: https://wt-a--shop.ddev.site\n") + + args := append([]string{"-C", dir, "--slug", "wt-a", "-n", "1", + "--canonical-base", "https://shop.acme.fi"}, + noNetwork("shop.acme.fi", "wt-a--shop.ddev.site")...) + _, _, errOut := run(t, "", cmdDiff, args...) + if !strings.Contains(errOut, "corpus diff: https://shop.acme.fi vs https://wt-a--shop.ddev.site") { + t.Errorf("the second site's base was not paired with its own variant:\n%s", errOut) + } +} + +// TestDiffSaysWhenABaseBelongsToNoSite: the production-canonical baseline is the +// project's own `.ddev.site`, which is deliberately not a canonical of +// the map, so an unmatched base cannot be an error. On a multisite it is still +// ambiguous — there is no site for it to pair with — and the pair being compared +// has to be visible rather than "whichever site was written first". +func TestDiffSaysWhenABaseBelongsToNoSite(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", "name: acme\n") + writeFile(t, dir, "hostshift.yaml", ""+ + "sites:\n"+ + " - canonical: https://www.acme.fi\n variant: https://wt-a--acme.ddev.site\n"+ + " - canonical: https://shop.acme.fi\n variant: https://wt-a--shop.ddev.site\n") + + args := append([]string{"-C", dir, "--slug", "wt-a", "-n", "1", + "--canonical-base", "https://acme-wt-b.ddev.site"}, + noNetwork("acme-wt-b.ddev.site", "wt-a--acme.ddev.site")...) + _, _, errOut := run(t, "", cmdDiff, args...) + if !strings.Contains(errOut, "is not a canonical of this 2-site") { + t.Errorf("an unrelated base was accepted in silence:\n%s", errOut) + } +} + +// TestExternalCanonicalHostsIsTheContainmentSet: loopback containment exists for +// the canonical hostnames that leave the machine. Under DDEV-canonical there are +// none and the add-on must not ask for a containment file; under +// production-canonical it is exactly the client's domains, aliases included. +// The add-on asks this rather than deriving it, so the two cannot drift. +func TestExternalCanonicalHostsIsTheContainmentSet(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", "name: acme\n") + writeFile(t, dir, "hostshift.yaml", ""+ + "sites:\n"+ + " - canonical: https://www.acme.fi\n"+ + " aliases:\n - https://acme.staging.example\n"+ + " variant: https://wt-a--acme.ddev.site\n") + _, out, _ := run(t, "", cmdMap, "-C", dir, "--slug", "wt-a", "--external-canonical-hosts") + for _, want := range []string{"www.acme.fi", "acme.staging.example"} { + if !strings.Contains(out, want) { + t.Errorf("the containment set omits %q:\n%s", want, out) + } + } + + stock := t.TempDir() + writeFile(t, stock, ".ddev/config.yaml", "name: stock\nadditional_hostnames:\n - nat\n") + _, out, _ = run(t, "", cmdMap, "-C", stock, "--slug", "wt-a", "--external-canonical-hosts") + if strings.TrimSpace(out) != "" { + t.Errorf("a stock project has nothing to contain, got:\n%s", out) + } +} + +// TestAFlatMapGetsTheSameDiagnostics: `--from/--to` returned before the DDEV +// project was loaded, on the reasoning that an explicit map needs no files. True +// of the map; false of everything that describes how the map sits against the +// project. A production-canonical map handed over as flags — which is how the +// add-on hands one over whenever it is not mounting a hostshift.yaml — got no +// note about the hostname DDEV advertises and no containment set, so both +// guardrails were switched off by how the map was spelled. +func TestAFlatMapGetsTheSameDiagnostics(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", "name: acme-wt-a\n") + flags := []string{"-C", dir, + "--from", "https://www.acme.fi", "--to", "https://wt-a--acme.ddev.site"} + + _, _, errOut := run(t, "", cmdCheck, flags...) + if !strings.Contains(errOut, "canonical-on-production") { + t.Errorf("a flat production-canonical map got no note:\n%s", errOut) + } + if !strings.Contains(errOut, "https://acme-wt-a.ddev.site") { + t.Errorf("the note does not name the directly-served hostname:\n%s", errOut) + } + + _, out, _ := run(t, "", cmdMap, append(flags, "--external-canonical-hosts")...) + if !strings.Contains(out, "www.acme.fi") { + t.Errorf("a flat map has an empty containment set:\n%s", out) + } +} + +// TestAnOrdinaryWorktreeIsNotCanonicalOnProduction: the canonicals of a +// DDEV-canonical worktree are the *parent project's* `.ddev.site` hostnames. +// They are not this project's own, and the first version of the condition +// tested exactly that — so every ordinary worktree was told its links pointed at +// a live site. `*.ddev.site` is a public record pointing at the loopback; what +// decides whether a name can leave the machine is the TLD, which is also the +// test `ddev hostshift loopback` applies when it writes the containment file. +func TestAnOrdinaryWorktreeIsNotCanonicalOnProduction(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", "name: acme-wt-a\n") + flags := []string{"-C", dir, + "--from", "https://acme.ddev.site", "--to", "https://wt-a--acme.ddev.site"} + + _, _, errOut := run(t, "", cmdCheck, flags...) + if strings.Contains(errOut, "canonical-on-production") { + t.Errorf("a DDEV-canonical worktree is not canonical-on-production:\n%s", errOut) + } + _, out, _ := run(t, "", cmdMap, append(flags, "--external-canonical-hosts")...) + if strings.TrimSpace(out) != "" { + t.Errorf("a local hostname needs no containment, got:\n%s", out) + } +} diff --git a/cmd/hostshift/audit_r43_test.go b/cmd/hostshift/audit_r43_test.go new file mode 100644 index 0000000..b3c2d3b --- /dev/null +++ b/cmd/hostshift/audit_r43_test.go @@ -0,0 +1,198 @@ +package main + +import ( + "strings" + "testing" +) + +// Round 43, on 1e4f62f ("Check that loopback containment exists; pair diff's +// bases by site"). + +// TestDiffPairsTheBasesBySiteFromTheVariantSideToo. +// +// 1e4f62f fixed `--canonical-base ` leaving the variant side at site 1's +// — "on a multisite that compares two different sites ... and the run printed +// GREEN". The pairing it added is guarded by +// +// if *canonicalBase != "" && *variantBase == "" +// +// so it runs in one direction only. `--variant-base ` on its own leaves +// the *canonical* side at site 1's, which is the same two-different-sites +// comparison with the flags swapped, and it is not even warned about: the +// "is not a canonical of this N-site map" message is inside the same guard. +// +// The asymmetry has no justification in the commit's own reasoning. That +// reasoning is about why an *unmatched* base must stay legal — the documented +// production-canonical baseline is the project's own `.ddev.site`, +// which is deliberately not a canonical. Every variant, by contrast, is in the +// map by construction, so the variant side is the one that can always be paired. +// +// The crawl reaches nothing here; the announced pair is printed before the first +// fetch and is the whole assertion. +func TestDiffPairsTheBasesBySiteFromTheVariantSideToo(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", "name: acme\n") + writeFile(t, dir, "hostshift.yaml", ""+ + "sites:\n"+ + " - canonical: https://www.acme.fi\n variant: https://wt-a--acme.ddev.site\n"+ + " - canonical: https://shop.acme.fi\n variant: https://wt-a--shop.ddev.site\n") + + args := append([]string{"-C", dir, "--slug", "wt-a", "-n", "1", + "--variant-base", "https://wt-a--shop.ddev.site"}, + noNetwork("shop.acme.fi", "www.acme.fi", "wt-a--shop.ddev.site")...) + _, _, errOut := run(t, "", cmdDiff, args...) + + // The premise: the mirror flag really does name site 2, and site 2 really is + // not site 1 — so anything that pairs would pair them. + if !strings.Contains(errOut, "vs https://wt-a--shop.ddev.site") { + t.Fatalf("fixture: --variant-base was not honoured at all:\n%s", errOut) + } + if strings.Contains(errOut, "corpus diff: https://www.acme.fi vs") && + !strings.Contains(errOut, "is not a variant of this 2-site") { + t.Errorf("site 1's canonical was crawled against site 2's variant, in silence:\n%s", errOut) + } +} + +// TestACanonicalUnderTestIsNotOnProduction. +// +// `ExternalCanonicals` decides two things: whether `check` prints the +// canonical-on-production note ("every link on them points at the live site"), +// and which hostnames the add-on demands loopback containment for. 1e4f62f +// changed its test from "is not one of this project's own hostnames" to +// +// !strings.HasSuffix(o.Host, "."+proj.TLD) +// +// on the reasoning that "how a name resolves is the question, not who owns it". +// The new test answers that question only for the project TLD. It has no answer +// for `additional_fqdns`, which DDEV registers in /etc/hosts and which +// `hostshift hosts` prints verbatim — the add-on's own variant check spends a +// paragraph on exactly this ("DDEV registers only the *exact* FQDN in +// /etc/hosts") and consults the hostname list for it. +// +// So a project with `additional_fqdns: [acme.test]` and no hostshift.yaml — a +// stock DDEV-canonical project, its database holding local URLs and nothing on +// any page leaving the machine — is told its map is canonical-on-production and +// that web can reach `acme.test` for real. +// +// `.test` is reserved by RFC 6761 and is never delegated, so it cannot be a live +// site by definition; and this same binary already says so — `isLoopbackHost` in +// cmd/hostshift/main.go returns true for `.test`, and `diff` uses it to decide +// whether a crawl would hit production. Two functions in one binary answering +// the same question two ways, which is the shape the commit message itself +// calls out as "the shape of half this project's bugs". +// +// This is the failure mode e37c0b0 spent half a commit fixing: a warning about +// production URLs on `ddev start` of a project that has none, on the post-start +// hook, which is how a warning stops being read. +func TestACanonicalUnderTestIsNotOnProduction(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", + "name: acme\nadditional_fqdns:\n - acme.test\n") + + // Premise, and a check on my own fixture: the binary's own loopback test + // says this hostname stays on the machine. + if !isLoopbackHost("acme.test") { + t.Fatal("fixture: isLoopbackHost disagrees, so this test is about something else") + } + + _, out, _ := run(t, "", cmdMap, "-C", dir, "--slug", "wt-a", "--external-canonical-hosts") + if strings.Contains(out, "acme.test") { + t.Errorf("a hostname DDEV registers in /etc/hosts is in the containment set:\n%s", out) + } + + _, _, errOut := run(t, "", cmdCheck, "-C", dir, "--slug", "wt-a") + if strings.Contains(errOut, "canonical-on-production") { + t.Errorf("a stock project with an additional_fqdn is called canonical-on-production:\n%s", + errOut) + } +} + +// TestTheNoteNamesAVariantYouCanActuallyReach. +// +// The note e37c0b0 added ends by telling the developer where to go instead, and +// it renders that with +// +// fmt.Fprintf(os.Stderr, " https://%s\n", st.Variant.Host) +// +// — a hardcoded scheme and `Host`, which on an Origin is the hostname with the +// port in a separate field. The map is origin→origin, scheme, host *and* port +// (PLAN §5.5), and `corpus.redirectsToItself` carries a comment about this exact +// mistake ("HostPort, not Host: an Origin keeps its port in a separate field"). +// The diff warning added in the very next commit renders its variant with +// `st.Variant.String()` and gets all three. +// +// So on a variant that is not https on 443, the one line in the note that tells +// the developer where to go points at a URL nothing is listening on — inside a +// paragraph whose entire purpose is to steer them off the page that serves live +// production links. The page they were warned away from is the one that works. +func TestTheNoteNamesAVariantYouCanActuallyReach(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", "name: acme-wt-a\n") + writeFile(t, dir, "hostshift.yaml", + "sites:\n - canonical: https://www.acme.fi\n"+ + " variant: http://wt-a--acme.ddev.site:8080\n") + + _, _, errOut := run(t, "", cmdCheck, "-C", dir, "--slug", "wt-a") + // Premise: this is the paragraph under test. + if !strings.Contains(errOut, "The variant(s) this map resolves to") { + t.Fatalf("fixture: the note did not fire:\n%s", errOut) + } + if !strings.Contains(errOut, "http://wt-a--acme.ddev.site:8080") { + t.Errorf("the note points the developer at a URL that is not the variant:\n%s", errOut) + } +} + +// The canonical-on-production note lists what this project serves. +// +// A worktree inherits the parent's additional_hostnames, and the add-on narrows +// web's VIRTUAL_HOST afterwards so the parent keeps serving its own — so DDEV +// registers `b.acme.ddev.site` here while the parent answers on it. The note +// listed it anyway, and `ddev launch` opens no such thing. That note is the only +// place a developer is told which URLs show unrewritten production content, so +// padding it with hostnames this project does not serve is how it gets skipped. + +// The canonical-on-production note lists what this project serves. +// +// A worktree inherits the parent's additional_hostnames, and the add-on narrows +// web's VIRTUAL_HOST afterwards so the parent keeps serving its own — so DDEV +// registers `b.acme.ddev.site` here while the parent answers on it. The note +// listed it anyway, and `ddev launch` opens no such thing. That note is the only +// place a developer is told which URLs show unrewritten production content, so +// padding it with hostnames this project does not serve is how it stops being +// read. +func TestR56TheNoteListsOnlyWhatThisProjectServes(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", + "name: acme-wt-a\nadditional_hostnames:\n - b.acme\n") + writeFile(t, dir, "hostshift.yaml", + "sites:\n - canonical: https://www.acme.fi\n"+ + " variant: https://wt-a--acme.ddev.site\n") + + // Only the canonical-on-production note: `b.acme.ddev.site` legitimately + // appears in the uncovered-hostnames warning below it, which is a different + // message answering a different question. + note := func(out string) string { + _, after, ok := strings.Cut(out, "canonical-on-production") + if !ok { + t.Fatalf("fixture: the note did not fire:\n%s", out) + } + if before, _, ok := strings.Cut(after, "\nhostshift:"); ok { + return before + } + return after + } + + _, _, all := run(t, "", cmdCheck, "-C", dir, "--slug", "wt-a") + if !strings.Contains(note(all), "b.acme.ddev.site") { + t.Fatalf("fixture: the inherited hostname is not in the note:\n%s", all) + } + _, _, narrowed := run(t, "", cmdCheck, "-C", dir, "--slug", "wt-a", + "--served-hosts", "acme-wt-a.ddev.site") + if strings.Contains(note(narrowed), "b.acme.ddev.site") { + t.Errorf("the note lists a hostname this project's web does not serve:\n%s", + narrowed) + } + if !strings.Contains(note(narrowed), "acme-wt-a.ddev.site") { + t.Errorf("the note dropped a hostname this project does serve:\n%s", narrowed) + } +} diff --git a/cmd/hostshift/audit_r44_test.go b/cmd/hostshift/audit_r44_test.go new file mode 100644 index 0000000..c1a81cd --- /dev/null +++ b/cmd/hostshift/audit_r44_test.go @@ -0,0 +1,124 @@ +package main + +import ( + "strings" + "testing" +) + +// Round 44, on 7cb756c ("Ask the size question in bytes; stop reports asserting +// what they skipped"). + +// TestADiffThatComparedNothingIsNotGreen. +// +// 7cb756c is a whole commit about a report claiming more than it checked: the +// verdict "no canonical origin reached the browser" printed two lines under a +// count of origins that had just reached it. `WriteReport` still prints the +// unqualified sentence — and exits 0 — over a run of zero pages. +// +// `corpus.Run` bounds its crawl with +// +// for len(queue) > 0 && (o.N == 0 || len(out) < o.N) +// +// so any negative `-n` makes the condition false on the first iteration, the +// crawl returns no paths at all, and `WriteReport` walks an empty slice: `green` +// is never set false because nothing was ever compared. The summary line does +// say "0 pages", but the line under it is the one the README calls the check +// that validates a deployment against reality, and it asserts invariant 28 +// about bytes the run never fetched. A CI job reads the exit status. +// +// The fix is a floor, not a guess about intent: a run that compared nothing +// verified nothing, and cannot be GREEN. +func TestADiffThatComparedNothingIsNotGreen(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", "name: acme\n") + writeFile(t, dir, "hostshift.yaml", ""+ + "sites:\n"+ + " - canonical: https://www.acme.fi\n variant: https://wt-a--acme.ddev.site\n") + + args := append([]string{"-C", dir, "--slug", "wt-a", "-n", "-1"}, + noNetwork("www.acme.fi", "wt-a--acme.ddev.site")...) + code, out, _ := run(t, "", cmdDiff, args...) + + // Premise: this really is the zero-page run, so the assertion below is + // about the verdict and not about a crawl that quietly did something. + if !strings.Contains(out, "0 pages,") { + t.Fatalf("fixture: the run compared something after all:\n%s", out) + } + if strings.Contains(out, "GREEN") { + t.Errorf("a run that fetched nothing printed the invariant-28 verdict:\n%s", out) + } + if code == 0 { + t.Errorf("exit 0 from a run that compared no pages — a CI job reads this") + } +} + +// TestR45ASingleSiteWorktreeIsWarnedAboutAnUnmatchedBase. +// +// README's worktree recipe is `diff --slug --canonical-base +// https://.ddev.site`. On a worktree whose map comes from its *own* DDEV +// config, the variant is derived from the worktree's name — so that command +// compared the parent against `wt-a--acme-wt-a.ddev.site`, a hostname nothing +// serves, and every row was a 404. +// +// The pairing warning existed but was gated on multisite. The reason for +// tolerating an unmatched canonical base is production-canonical, where the +// documented baseline is deliberately not a canonical of the map; that reason +// does not apply to a map with no external canonical at all. +func TestR45ASingleSiteWorktreeIsWarnedAboutAnUnmatchedBase(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", "name: acme-wt-a\n") + args := append([]string{"-C", dir, "--slug", "wt-a", "-n", "1", + "--canonical-base", "https://acme.ddev.site"}, + noNetwork("acme.ddev.site", "wt-a--acme-wt-a.ddev.site")...) + _, _, errOut := run(t, "", cmdDiff, args...) + if !strings.Contains(errOut, "is not a canonical of this") { + t.Errorf("a single-site worktree got no warning about an unmatched base:\n%s", errOut) + } + + // And production-canonical still does not warn: there the baseline is the + // project's own hostname by design. + pc := t.TempDir() + writeFile(t, pc, ".ddev/config.yaml", "name: acme-wt-a\n") + writeFile(t, pc, "hostshift.yaml", + "sites:\n - canonical: https://www.acme.fi\n variant: https://wt-a--acme.ddev.site\n") + args = append([]string{"-C", pc, "--slug", "wt-a", "-n", "1", + "--canonical-base", "https://acme-wt-a.ddev.site"}, + noNetwork("acme-wt-a.ddev.site", "wt-a--acme.ddev.site")...) + _, _, errOut = run(t, "", cmdDiff, args...) + if strings.Contains(errOut, "is not a canonical of this") { + t.Errorf("the documented production-canonical baseline must not warn:\n%s", errOut) + } +} + +// TestR45ARequestTypeIsRefusedRatherThanPassedThrough. +// +// `rewrite --type application/x-www-form-urlencoded` printed the input back with +// an empty counter block and no diagnostic, which reads as "the engine found +// nothing in your body" when it means "this command never looked" — request +// bodies are rewritten by the proxy, in the other direction. +// +// Only the request types. Tier 2 pass-through is deliberate (§5.2), and piping a +// stylesheet through to see it unchanged is a real question with a true answer. +func TestR45ARequestTypeIsRefusedRatherThanPassedThrough(t *testing.T) { + in := "url=https%3A%2F%2Fwww.acme.fi%2Fx" + for _, ct := range []string{"application/x-www-form-urlencoded", "multipart/form-data"} { + code, out, errOut := run(t, in, cmdRewrite, + "--map", "https://www.acme.fi=https://wt-a--acme.ddev.site", "--type", ct) + if code != exitConfig { + t.Errorf("%s: exit %d, want %d", ct, code, exitConfig) + } + if strings.Contains(out, "url=") { + t.Errorf("%s: the body was passed through: %q", ct, out) + } + if !strings.Contains(errOut, "rewrites") { + t.Errorf("%s: no diagnostic: %q", ct, errOut) + } + } + // And a Tier 2 type still passes through untouched, which is documented. + css := "body{background:url(https://www.acme.fi/bg.png)}" + code, out, _ := run(t, css, cmdRewrite, + "--map", "https://www.acme.fi=https://wt-a--acme.ddev.site", "--type", "text/css", "--quiet") + if code != exitOK || out != css { + t.Errorf("text/css must stream through untouched: exit %d, %q", code, out) + } +} diff --git a/cmd/hostshift/audit_r46_test.go b/cmd/hostshift/audit_r46_test.go new file mode 100644 index 0000000..f0eabdd --- /dev/null +++ b/cmd/hostshift/audit_r46_test.go @@ -0,0 +1,43 @@ +package main + +import ( + "strings" + "testing" +) + +// Round 46, on 598de7c. +// +// 598de7c widened `diff`'s unmatched-base warning from multisite-only to +// "multisite, or a map with no external canonicals": +// +// if !matched && (len(res.Map.Sites) > 1 || len(res.ExternalCanonicals) == 0) +// +// ExternalCanonicals is non-empty exactly under production-canonical, which is +// the worktree deployment the README documents — so on the commonest single-site +// map the warning is suppressed for *both* flags. The commit's own comment says +// why that is wrong for one of them: +// +// "The asymmetry had no reason behind it: the argument for tolerating an +// unmatched base is about the canonical side, where the production-canonical +// baseline is the project's own .ddev.site and deliberately not in +// the map. Every variant is in the map by construction." +// +// A `--variant-base` naming a host no site declares is therefore always worth +// saying out loud, and here it is silent. +func TestR46VariantBaseUnmatchedIsSilentOnAProductionCanonicalSite(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", "name: acme\n") + writeFile(t, dir, "hostshift.yaml", ""+ + "sites:\n"+ + " - canonical: https://www.acme.fi\n variant: https://wt-a--acme.ddev.site\n") + + args := append([]string{"-C", dir, "--slug", "wt-a", "-n", "1", + "--variant-base", "https://wt-b--acme.ddev.site"}, + noNetwork("www.acme.fi", "wt-b--acme.ddev.site")...) + _, _, errOut := run(t, "", cmdDiff, args...) + if !strings.Contains(errOut, "--variant-base") { + t.Errorf("a --variant-base that names no site of the map went unwarned, and\n"+ + "the run compared https://www.acme.fi against a host the map does not know:\n%s", + errOut) + } +} diff --git a/cmd/hostshift/audit_r47_test.go b/cmd/hostshift/audit_r47_test.go new file mode 100644 index 0000000..be9aa7a --- /dev/null +++ b/cmd/hostshift/audit_r47_test.go @@ -0,0 +1,80 @@ +package main + +import ( + "path/filepath" + "strings" + "testing" +) + +// Round 47. +// +// 1e4f62f paired the diff's bases by site because "--canonical-base +// was crawled against site 1's variant, every page differing for the obvious +// reason, and the run printed GREEN". 4e8c68e widened the unmatched-base warning +// to fire for --variant-base too. Both are about a base the map does not know. +// +// The pairing asks whether it knows one with strings.EqualFold(h, u.Host), where +// h is origin.Origin.Host — "lowercase, punycode, no trailing dot" — and u.Host +// is whatever url.Parse kept of what the developer typed. Those are the same +// string for an ASCII hostname and never the same string for an IDN one, so a +// canonical base written the way its owner writes it pairs with nothing. +// +// The command's own --resolve guardrail had exactly this bug and was fixed by +// keying both sides through corpus.ResolveKey — "asking this question a second +// way is what made the guardrail disagree with the dialer". This is the same +// question asked a third way, one flag over. §5.5 calls IDN "real for .fi client +// domains". +func TestR47AnIDNBaseDoesNotPairWithItsOwnSite(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", "name: acme\n") + writeFile(t, dir, "hostshift.yaml", ""+ + "sites:\n"+ + " - canonical: https://www.acme.fi\n variant: https://wt-a--acme.ddev.site\n"+ + " - canonical: https://www.hämeenlinna.fi\n variant: https://wt-a--hml.ddev.site\n") + + args := append([]string{"-C", dir, "--slug", "wt-a", "-n", "1", + "--canonical-base", "https://www.hämeenlinna.fi"}, + noNetwork("www.acme.fi", "www.hämeenlinna.fi", + "wt-a--acme.ddev.site", "wt-a--hml.ddev.site")...) + _, _, errOut := run(t, "", cmdDiff, args...) + + // The ASCII site pairs, so the mechanism works and only the spelling does not. + ascii := append([]string{"-C", dir, "--slug", "wt-a", "-n", "1", + "--canonical-base", "https://www.acme.fi"}, + noNetwork("www.acme.fi", "wt-a--acme.ddev.site")...) + if _, _, ok := run(t, "", cmdDiff, ascii...); !strings.Contains(ok, + "corpus diff: https://www.acme.fi vs https://wt-a--acme.ddev.site") { + t.Fatalf("the control case no longer holds, so this test measures nothing:\n%s", ok) + } + + if strings.Contains(errOut, "is not a canonical of this 2-site map") { + t.Errorf("the warning says a base that IS a canonical of the map is not one:\n%s", errOut) + } + if !strings.Contains(errOut, "vs https://wt-a--hml.ddev.site") { + t.Errorf("the IDN base was paired with a different site's variant — the "+ + "comparison of unrelated pages the pairing exists to prevent:\n%s", errOut) + } +} + +// TestR47TheLiveSiteWarningCountsWhatWillBeFetched: the sentence exists to stop +// a developer before a crawl of the client's live site, and it printed `-n` +// whatever the run was actually going to fetch — so a `--paths` file of two +// lines warned about twenty pages. +func TestR47TheLiveSiteWarningCountsWhatWillBeFetched(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", "name: acme\n") + writeFile(t, dir, "hostshift.yaml", + "sites:\n - canonical: https://www.acme.invalid\n variant: https://wt-a--acme.ddev.site\n") + writeFile(t, dir, "paths.txt", "/one\n/two\n") + + args := append([]string{"-C", dir, "--slug", "wt-a", "-n", "20", + "--paths", filepath.Join(dir, "paths.txt")}, + noNetwork("wt-a--acme.ddev.site")...) + _, _, errOut := run(t, "", cmdDiff, args...) + if !strings.Contains(errOut, "crawling 2 page(s)") { + t.Errorf("the warning does not count the supplied paths:\n%s", errOut) + } + if strings.Contains(errOut, "crawling 20 page(s)") { + t.Errorf("the warning still prints -n rather than what will be fetched:\n%s", errOut) + } +} diff --git a/cmd/hostshift/audit_r48_test.go b/cmd/hostshift/audit_r48_test.go new file mode 100644 index 0000000..33cd0d7 --- /dev/null +++ b/cmd/hostshift/audit_r48_test.go @@ -0,0 +1,56 @@ +package main + +import ( + "path/filepath" + "strings" + "testing" +) + +// Round 48, on 717ad9d. +// +// TestR48TheLiveSiteWarningStillMiscountsAPathListLongerThanN: 717ad9d moved the +// live-site warning below the `--paths` read so it could count what will +// actually be fetched, and its own comment says so — +// +// // How many will actually be fetched: the supplied list if there is one, and +// // otherwise the crawl's budget. +// +// — but a supplied list is not what will be fetched either. `corpus.Run` bounds +// it: `if !crawled && o.N > 0 && len(paths) > o.N { paths = paths[:o.N] }`. So +// the count is right for a list shorter than `-n` (the case the change was made +// for) and wrong for a list longer than it, by exactly the amount `-n` cuts off. +// +// The number is the whole content of the sentence. It is the one line printed to +// make a developer stop before a crawl of the client's live site, and the audit +// record already says of the old spelling that this is "the one sentence written +// to make a developer stop". Overstating is the safer direction of the two, but +// a guardrail that cannot say how many requests it is about to make to +// production is not one a reader can act on: the answer to "is 200 pages of the +// live site acceptable?" is different from the answer to "is 20?". +func TestR48TheLiveSiteWarningStillMiscountsAPathListLongerThanN(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", "name: acme\n") + writeFile(t, dir, "hostshift.yaml", + "sites:\n - canonical: https://www.acme.invalid\n variant: https://wt-a--acme.ddev.site\n") + writeFile(t, dir, "paths.txt", "/one\n/two\n/three\n/four\n/five\n") + + // The control is the case 717ad9d fixed: a list shorter than -n is counted + // as the list, so the sentence does read the paths file. + writeFile(t, dir, "short.txt", "/one\n/two\n") + short := append([]string{"-C", dir, "--slug", "wt-a", "-n", "20", + "--paths", filepath.Join(dir, "short.txt")}, + noNetwork("wt-a--acme.ddev.site")...) + if _, _, ctl := run(t, "", cmdDiff, short...); !strings.Contains(ctl, "crawling 2 page(s)") { + t.Fatalf("the control case no longer holds, so this test measures nothing:\n%s", ctl) + } + + args := append([]string{"-C", dir, "--slug", "wt-a", "-n", "2", + "--paths", filepath.Join(dir, "paths.txt")}, + noNetwork("wt-a--acme.ddev.site")...) + _, _, errOut := run(t, "", cmdDiff, args...) + if !strings.Contains(errOut, "crawling 2 page(s)") { + t.Errorf("the warning counts the whole --paths file, but corpus.Run cuts it "+ + "to -n, so it names a number of live-site requests that will not be made:\n%s", + errOut) + } +} diff --git a/cmd/hostshift/audit_r49_test.go b/cmd/hostshift/audit_r49_test.go new file mode 100644 index 0000000..1de7068 --- /dev/null +++ b/cmd/hostshift/audit_r49_test.go @@ -0,0 +1,71 @@ +package main + +import ( + "strings" + "testing" +) + +// TestR50BothBasesBecomeTheMapWhenItKnowsNeither. +// +// `--canonical-base` and `--variant-base` moved only the crawl; the rewriting +// map still came from `-C`/`--slug`, which in a worktree resolves to the +// worktree's own DDEV hostnames. Those appear on neither side of the +// comparison, so the canonical body was compared unrewritten and the leak scan +// looked for an origin that could not occur — 0 leaks and "no canonical origin +// reached the browser" over four pages carrying 193 of them, on the invocation +// README documents for worktrees. +func TestR50BothBasesBecomeTheMapWhenItKnowsNeither(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", "name: acme-wt-a\n") + + args := append([]string{"-C", dir, "--slug", "wt-a", "-n", "1", + "--canonical-base", "https://acme.ddev.site", + "--variant-base", "https://wt-a--acme.ddev.site"}, + noNetwork("acme.ddev.site", "wt-a--acme.ddev.site")...) + _, _, errOut := run(t, "", cmdDiff, args...) + + if !strings.Contains(errOut, "neither base is in the map") { + t.Errorf("the run did not say the map it was given is irrelevant:\n%s", errOut) + } + if !strings.Contains(errOut, "corpus diff: https://acme.ddev.site vs https://wt-a--acme.ddev.site") { + t.Errorf("the announced pair is wrong:\n%s", errOut) + } + + // And a map that *does* know a base is left alone — production-canonical, + // where the baseline is deliberately a third hostname and the variant is in + // the map. + pc := t.TempDir() + writeFile(t, pc, ".ddev/config.yaml", "name: acme-wt-a\n") + writeFile(t, pc, "hostshift.yaml", + "sites:\n - canonical: https://www.acme.fi\n variant: https://wt-a--acme.ddev.site\n") + args = append([]string{"-C", pc, "--slug", "wt-a", "-n", "1", + "--canonical-base", "https://acme-wt-a.ddev.site", + "--variant-base", "https://wt-a--acme.ddev.site"}, + noNetwork("acme-wt-a.ddev.site", "wt-a--acme.ddev.site")...) + _, _, errOut = run(t, "", cmdDiff, args...) + if strings.Contains(errOut, "neither base is in the map") { + t.Errorf("the production-canonical map was replaced, and it is the right one:\n%s", errOut) + } +} + +// TestR50AnUnrewrittenTypeSaysSo: `--type text/htm` — one character from +// `text/html` — printed the input back with an empty counter block and exit 0, +// which reads as "the engine found nothing here" rather than "that is not a type +// I rewrite". Tier 2 still passes through, by design; it just says why. +func TestR50AnUnrewrittenTypeSaysSo(t *testing.T) { + css := "body{background:url(https://www.acme.fi/bg.png)}" + code, out, errOut := run(t, css, cmdRewrite, + "--map", "https://www.acme.fi=https://wt-a--acme.ddev.site", "--type", "text/css") + if code != exitOK || out != css { + t.Errorf("a Tier 2 type must still stream through: exit %d, %q", code, out) + } + if !strings.Contains(errOut, "outside the rewritable set") { + t.Errorf("nothing said the type was not rewritten:\n%s", errOut) + } + // --quiet is the machine-readable mode and stays silent. + _, _, errOut = run(t, css, cmdRewrite, + "--map", "https://www.acme.fi=https://wt-a--acme.ddev.site", "--type", "text/css", "--quiet") + if strings.Contains(errOut, "outside the rewritable set") { + t.Errorf("--quiet should say nothing:\n%s", errOut) + } +} diff --git a/cmd/hostshift/audit_r51_test.go b/cmd/hostshift/audit_r51_test.go new file mode 100644 index 0000000..e26c61f --- /dev/null +++ b/cmd/hostshift/audit_r51_test.go @@ -0,0 +1,241 @@ +package main + +import ( + "io" + "net/http" + "net/http/httptest" + "path/filepath" + "strings" + "testing" +) + +// Round 51, on 2c6f6a8. + +// TestR51TheLiveCrawlGuardFoldsTheBaseAndTheDialerDoesNot. +// +// Round 41 found that `cmdDiff`'s live-crawl guardrail and `corpus.Run`'s +// dialer normalised in opposite directions, and fixed it by routing both +// through `corpus.ResolveKey`: +// +// _, covered := resolveMap[corpus.ResolveKey(net.JoinHostPort(cb.Hostname(), port))] +// +// with the map itself keyed by the same function. That folds both *keys*. It +// does not fold the thing the dialer is handed: +// +// if to, ok := o.Resolve[addr]; ok { +// +// where `addr` comes from `net/http`, which punycodes the URL host but leaves +// its case alone (`idnaASCII` returns an ASCII host unchanged, and +// `canonicalAddr` lowercases nothing). diff.go's comment on that line asserts +// the opposite — "net/http hands this the punycode host lowercased" — and that +// is the bug: it is true of the IDN half and false of the case half. +// +// So round 41 closed the direction where `--resolve` is misspelled and left +// open the direction where the *base* is. `--canonical-base https://WWW.ACME.FI` +// with a lowercase `--resolve www.acme.fi:443:127.0.0.1:8443`: +// +// - the guard folds `WWW.ACME.FI` to `www.acme.fi`, finds the key, says +// covered, and prints nothing; +// - the dialer looks up `WWW.ACME.FI:443`, misses, and goes to real DNS. +// +// Under production-canonical the canonical base *is* the client's production +// hostname, and `-n 20` is twenty real requests to the live site — made while +// the one message written to stop that stays silent, which as this command's +// own comment says "reads as confirmation". A guardrail a typo can disable is +// the failure mode both round 41 and the `--resolve` help text exist for; only +// half of it was closed. +// +// The hosts here are under `.invalid` (RFC 2606), so the test never leaves the +// machine, and 127.0.0.1:9 is discard — a redirected fetch fails with "connect: +// connection refused" naming it, one that was not fails with "no such host". +// The destination is read out of the report rather than assumed, so this +// asserts the dialer's behaviour and not the flag's. +// +// The fix is one line: fold the address in the dialer too — or, since the +// comment there argues against folding twice, fold `cb`'s host once where the +// URL is built, so every consumer of it (the dialer, the report line, the +// `Host` header) sees the same spelling. Either way `corpus.ResolveKey` stays +// the single definition of the fold. +func TestR51TheLiveCrawlGuardFoldsTheBaseAndTheDialerDoesNot(t *testing.T) { + const local = "127.0.0.1:9" + const resolve = "www.example.invalid:443:" + local + + cases := map[string]string{ + // The control: round 41's spelling, which works. + "a lowercase base": "https://www.example.invalid", + "an upper-cased base": "https://WWW.EXAMPLE.INVALID", + "a mixed-case base": "https://WWW.Example.Invalid", + "a base with one shift": "https://Www.example.invalid", + } + + var redirected, notRedirected int + for name, base := range cases { + _, out, errOut := run(t, "", cmdDiff, + "--canonical-base", base, + "--from", "https://www.example.fi", "--to", "https://wt-a--client.ddev.site", + "-n", "1", "--timeout", "3s", "--resolve", resolve) + + warned := strings.Contains(errOut, "not pointed anywhere local") + wentLocal := strings.Contains(out, local) + wentToDNS := strings.Contains(out, "no such host") + if wentLocal == wentToDNS { + t.Fatalf("%s: the crawl's destination is not legible, so this test would "+ + "assert nothing:\nstdout:\n%s\nstderr:\n%s", name, out, errOut) + } + if wentLocal { + redirected++ + } else { + notRedirected++ + } + + // Every case here names the host being crawled, differing from it only + // in case — which is not a difference in a hostname. + if !wentLocal { + t.Errorf("%s: --resolve %q names the host --canonical-base %s crawls, so "+ + "the fetch should have gone to %s; it fell through to real DNS, which "+ + "under production-canonical is the client's live site\nstdout:\n%s", + name, resolve, base, local, out) + } + // And the guardrail must not report containment it did not get. + if !warned && !wentLocal { + t.Errorf("%s: the crawl was NOT pointed anywhere local and hostshift said "+ + "nothing, so the silence reads as confirmation that the live site was "+ + "not fetched\nstderr:\n%s\nstdout:\n%s", name, errOut, out) + } + } + + if redirected == 0 { + t.Fatalf("no case in this table was redirected (%d/%d), so the fixture no "+ + "longer exercises --resolve at all", redirected, notRedirected) + } +} + +// TestR51TheReplacedMapIsWhatTheLeakScanActuallyUses. +// +// Round 50's LARGE was that `diff`'s bases moved the crawl and not the map, so +// the leak scan hunted an origin that could not occur and printed `0 leaks` +// over pages full of them. 2c6f6a8 fixed it in two halves: a notice on stderr, +// and the assignment behind it — +// +// res.Map = m +// +// Only the notice has a test. Deleting that one line leaves `go test ./...` and +// `test/addon-command.sh` both green, and the run then prints +// +// neither base is in the map … and that is what the leak scan looks for +// +// while the leak scan looks at the map from `--slug`, exactly as it did before +// the fix. That is worse than the state round 50 found, because the message now +// asserts the thing that is false. A fix whose behaviour no test observes is a +// fix a refactor removes silently. +// +// This asserts the behaviour: a variant page carrying the canonical base's own +// origin must be counted as a leak. Both bases are pinned to one local +// httptest server with `--resolve`, so nothing leaves the machine and the two +// fetches return the same bytes; what is being measured is which hostname the +// scan was looking for. +func TestR51TheReplacedMapIsWhatTheLeakScanActuallyUses(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/html") + io.WriteString(w, `canonical`) + })) + defer srv.Close() + addr := strings.TrimPrefix(srv.URL, "http://") + + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", "name: acme-wt-a\n") + writeFile(t, dir, "paths.txt", "/\n") + + _, out, errOut := run(t, "", cmdDiff, + "-C", dir, "--slug", "wt-a", + "--paths", filepath.Join(dir, "paths.txt"), + "--canonical-base", "http://acme.ddev.site", + "--variant-base", "http://wt-a--acme.ddev.site", + "--resolve", "acme.ddev.site:80:"+addr, + "--resolve", "wt-a--acme.ddev.site:80:"+addr, + "--timeout", "5s") + + if strings.Contains(out, "errors") && !strings.Contains(out, "0 errors") { + t.Fatalf("the fetches did not reach the local server, so this asserts nothing:\n%s\n%s", out, errOut) + } + if !strings.Contains(errOut, "neither base is in the map") { + t.Fatalf("the bases-are-the-map branch did not fire, so this fixture no longer "+ + "tests it:\n%s", errOut) + } + if strings.Contains(out, " 0 leaks,") { + t.Errorf("the variant page carries http://acme.ddev.site — the canonical base "+ + "this run announced it was comparing against — and the scan reported no "+ + "leak, so it was still looking for the hostnames --slug derived:\n%s\n%s", + errOut, out) + } +} + +// TestR51TheUnrewritableTypeNoticeFiresOnTheTypesItRewrites. +// +// 2c6f6a8 added a line to `cmdRewrite` for the case where `--type` names +// something outside the rewritable set — "a typo looks exactly like a clean +// result otherwise". It was added *after* the `switch`, and the switch's +// rewriting arms only assign `src`; none of them returns. So the notice runs on +// every path: +// +// $ echo 'y' | hostshift rewrite --type text/html … +// hostshift: --type text/html is outside the rewritable set, so the input is passed through unchanged. +// Rewritten types: text/html, application/xhtml+xml, the JSON family, … +// y +// rewrites by surface: +// html-attr 1 +// +// text/html is the *default* `--type`, so the plain invocation in the README +// now denies doing the thing it just did, and names itself in the list of types +// it claims not to be in, two lines later. +// +// It also breaks `--json`, documented as "emit counters as JSON on stderr": +// three lines of prose now precede the object on that stream, so anything +// piping stderr to a parser gets a syntax error. The add-on's own leak check +// pipes `--dry-run --json 2>&1 >/dev/null` into awk and survives only because +// its pattern is `"rewrites"` and the prose does not contain it. +// +// The notice belongs inside the switch's default — or the rewriting arms need +// to return — so it fires exactly when nothing rewrote. +func TestR51TheUnrewritableTypeNoticeFiresOnTheTypesItRewrites(t *testing.T) { + const in = `y` + mapFlag := "https://www.acme.fi=https://wt-a--acme.ddev.site" + + for _, mt := range []string{ + "text/html", "application/xhtml+xml", "application/json", + "text/plain", "application/rss+xml", "image/svg+xml", + } { + body := in + if mt == "application/json" { + body = `{"u":"https://www.acme.fi/x"}` + } + code, out, errOut := run(t, body, cmdRewrite, "--map", mapFlag, "--type", mt) + if code != exitOK { + t.Fatalf("--type %s: exit %d\n%s", mt, code, errOut) + } + if out == body { + t.Fatalf("--type %s did not rewrite, so this fixture no longer tests the "+ + "claim:\n %s", mt, out) + } + if strings.Contains(errOut, "outside the rewritable set") { + t.Errorf("--type %s rewrote the input and said it was \"outside the "+ + "rewritable set, so the input is passed through unchanged\":\n"+ + " stdout: %s\n stderr: %s", mt, out, errOut) + } + } + + // And --json must still be JSON on stderr, which is what it is for. + _, _, errOut := run(t, in, cmdRewrite, "--map", mapFlag, "--type", "text/html", "--json") + trimmed := strings.TrimLeft(errOut, " \t\r\n") + if !strings.HasPrefix(trimmed, "{") { + t.Errorf("--json is documented as counters as JSON on stderr, and stderr does "+ + "not start with an object:\n%s", errOut) + } + + // The type the notice was written for still gets it — a fix must not be + // "delete the message". + _, _, errOut = run(t, "body{}", cmdRewrite, "--map", mapFlag, "--type", "text/css") + if !strings.Contains(errOut, "outside the rewritable set") { + t.Errorf("text/css is outside the rewritable set and nothing said so:\n%s", errOut) + } +} diff --git a/cmd/hostshift/audit_r57_test.go b/cmd/hostshift/audit_r57_test.go new file mode 100644 index 0000000..f6121d5 --- /dev/null +++ b/cmd/hostshift/audit_r57_test.go @@ -0,0 +1,135 @@ +package main + +import ( + "strings" + "testing" +) + +// Round 57, on d017b64 ("Draw the grid, and see the states check could not +// see"), auditing the map/config layer where it crosses what `init` writes. + +// TestMapPairsPreservesTheDeclaredSpelling. +// +// `ddev hostshift init` cannot hand the container a directory it cannot see, so +// for a project with no hostshift.yaml it resolves the map on the host and hands +// it over flat: +// +// while IFS='=' read -r c v; do hsmap="$hsmap --from $c --to $v" +// done <<<"$(hostshift map --slug "$slug" --pairs)" +// +// `--pairs` prints `s.Canonical.String()`, and String() renders HostPort() — +// the punycode comparison form. That is the one thing this package's own +// origin.go says String() is not for: "This is for building a *replacement*, and +// for diagnostics. It is never used to round-trip input." +// +// So Origin.Display, added by acce8c6 ("Preserve the spelling an IDN was +// declared with, on the way back") for exactly this failure, does not survive +// the handoff: `--from https://xn--hmeen-gra.ddev.site` re-parses inside the +// container to an all-ASCII declaration, Display is empty, and the *request* +// direction then splices the A-label. A block-editor save on the preview writes +// `xn--hmeen-gra.ddev.site` into the shared database where every other row holds +// the U-label — §4.3, through the supported install path, and the exact defect +// acce8c6 fixed one layer up. +// +// DisplayHostPort is what the map's own replacements use; the pairs line must +// use it too, or the two halves of the same tool disagree about the same host. +func TestMapPairsPreservesTheDeclaredSpelling(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", "name: acme\nadditional_hostnames:\n - hämeen\n") + + _, out, _ := run(t, "", cmdMap, "-C", dir, "--slug", "wt", "--pairs") + if !strings.Contains(out, "hämeen.ddev.site=") { + t.Errorf("`map --pairs` dropped the spelling the hostname was declared with,\n"+ + "so the --from/--to line `init` writes hands the proxy an origin with no\n"+ + "Display and the request direction splices punycode into the shared\n"+ + "database (§4.3):\n%s", out) + } +} + +// TestCheckDoesNotCallACoveredHostUncovered. +// +// annotate() builds `covered` and `variant` from origin.Host — punycode — and +// then tests DDEV's registered hostnames against them, which are the strings the +// developer wrote in .ddev/config.yaml. For an IDN hostname the two spellings +// never meet, so the host is reported as one "this map does not cover" while it +// is in fact that map's own canonical, and it is also added to DirectlyServed, +// where it feeds the canonical-on-production note. +// +// Both are printed by `ddev hostshift check`, which is the post-start hook — so +// this is a warning on every single `ddev start` of a correctly configured +// project, which is how a warning stops being read. This file's own comments +// make that argument twice. +func TestCheckDoesNotCallACoveredHostUncovered(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", "name: acme\nadditional_hostnames:\n - hämeen\n") + + _, _, errOut := run(t, "", cmdCheck, "-C", dir, "--slug", "wt") + if strings.Contains(errOut, "does not cover") { + t.Errorf("`check` reported its own canonical as uncovered, because the census\n"+ + "is keyed on punycode and DDEV's hostnames are not:\n%s", errOut) + } +} + +// Both sides of the pair, and both loops of the census. +// +// Round 58's mutation survey found each of these fixes pinned on one half only: +// `--pairs` was asserted on the canonical side, so the variant could go back to +// HostPort(); and `annotate` was asserted on its `covered` loop, so the +// `variant` loop could go back to comparing spellings that never meet — which +// puts the map's own variant into DirectlyServed and fires the +// canonical-on-production note on a correct project, verbatim the failure round +// 57 fixed one loop over. +func TestR58BothSidesOfTheIDNSpelling(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", "name: acme\nadditional_hostnames:\n - hämeen\n") + + // The canonical side of *every* pair, not just the first. The database holds + // canonicals, so this is the side §4.3 is about; the variant is derived and + // never appears in the database, so its punycode spelling is a difference in + // rendering rather than a round-trip defect, and is left alone deliberately. + _, out, _ := run(t, "", cmdMap, "-C", dir, "--slug", "wt", "--pairs") + for _, line := range strings.Split(strings.TrimSpace(out), "\n") { + canonical, _, ok := strings.Cut(line, "=") + if !ok { + continue + } + if strings.Contains(canonical, "xn--") { + t.Errorf("`map --pairs` emitted punycode for a canonical, so the\n"+ + "--from/--to line `init` writes hands the proxy an origin with no\n"+ + "Display and the request direction splices an A-label into a database\n"+ + "whose every other row holds the U-label (§4.3):\n%s", out) + } + } + +} + +// The census's *second* loop, on a fixture that actually arms the note. +// +// The first version of this assertion used an all-`.ddev.site` map, where +// ExternalCanonicals is empty and the canonical-on-production note never fires +// — so it passed whatever the loop did. The note needs an external canonical to +// arm, and the defect needs a DDEV hostname that *is* a variant, declared in the +// IDN spelling DDEV keeps. Then comparing an ACE variant against a declared one +// puts the map's own variant into DirectlyServed and the note tells the +// developer to avoid the very hostname the preview is served on. +func TestR58TheVariantCensusNormalisesToo(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".ddev/config.yaml", + "name: acme\nadditional_hostnames:\n - wt--hämeen\n") + writeFile(t, dir, "hostshift.yaml", + "sites:\n - canonical: https://www.acme.example\n"+ + " variant: https://wt--hämeen.ddev.site\n") + + _, _, errOut := run(t, "", cmdCheck, "-C", dir, "--slug", "wt") + // Premise: the paragraph under test fired at all. + if !strings.Contains(errOut, "canonical-on-production") { + t.Fatalf("fixture: the note did not fire, so this asserts nothing:\n%s", errOut) + } + _, after, _ := strings.Cut(errOut, "canonical-on-production") + note, _, _ := strings.Cut(after, "\nhostshift:") + if strings.Contains(note, "hämeen") || strings.Contains(note, "xn--hmeen") { + t.Errorf("the note names the map's own variant as a hostname that serves\n"+ + "the database unrewritten, because the variant census compares an ACE\n"+ + "hostname against the spelling DDEV keeps:\n%s", errOut) + } +} diff --git a/cmd/hostshift/audit_r59_test.go b/cmd/hostshift/audit_r59_test.go new file mode 100644 index 0000000..d972938 --- /dev/null +++ b/cmd/hostshift/audit_r59_test.go @@ -0,0 +1,75 @@ +package main + +import ( + "bytes" + "log/slog" + "strings" + "testing" + + "github.com/generoi/hostshift/internal/origin" +) + +// The census wiring, which nothing pinned. +// +// Round 59's mutation survey deleted the whole hook, silenced it to Debug and +// hardcoded its surface field, and every test stayed green — on the feature that +// exists so `check`'s instruction at a test-28 refusal ("add --explain, restart, +// `| grep census`") is true. `--dry-run`'s stated behaviour was unpinned the +// same way. +func TestCensusIsWiredForBothFlags(t *testing.T) { + for _, c := range []struct { + explain, dryRun, want bool + }{ + {false, false, false}, + {true, false, true}, + // --dry-run's own help says it logs every rewrite it would have made. + {false, true, true}, + {true, true, true}, + } { + if got := wantCensus(c.explain, c.dryRun); got != c.want { + t.Errorf("wantCensus(explain=%v, dryRun=%v) = %v, want %v", + c.explain, c.dryRun, got, c.want) + } + } +} + +// And what it writes, at a level the default handler emits. +func TestCensusHookWritesAGreppableLine(t *testing.T) { + var buf bytes.Buffer + // The default handler: Debug is dropped by it, which is what makes the + // level part of the contract rather than a detail. + log := slog.New(slog.NewTextHandler(&buf, nil)) + censusHookFor(log, false)(("html-attr"), origin.Event{ + Action: origin.ActionRewrote, Offset: 42, Text: "https://www.example.fi", + }) + got := buf.String() + if !strings.Contains(got, "census") { + t.Errorf("the line `check` tells the developer to grep for is not in it:\n %s", got) + } + for _, want := range []string{"html-attr", "rewrote", "42", "www.example.fi"} { + if !strings.Contains(got, want) { + t.Errorf("the census line does not carry %q, so it cannot answer "+ + "which surface a leak was on:\n %s", want, got) + } + } +} + +// ...and it says so when nothing was applied. +// +// Under --dry-run the proxy serves everything unmodified while the census line +// is identical to a real rewrite, so a developer grepping it mid-incident could +// not tell a proxy that is rewriting from one that is only reporting. +func TestCensusMarksADryRun(t *testing.T) { + var buf bytes.Buffer + log := slog.New(slog.NewTextHandler(&buf, nil)) + censusHookFor(log, true)("html-attr", origin.Event{Action: origin.ActionRewrote}) + if !strings.Contains(buf.String(), "dry-run=true") { + t.Errorf("a dry-run census line is indistinguishable from a real rewrite:\n %s", + buf.String()) + } + buf.Reset() + censusHookFor(log, false)("html-attr", origin.Event{Action: origin.ActionRewrote}) + if strings.Contains(buf.String(), "dry-run") { + t.Errorf("a real rewrite was marked as a dry run:\n %s", buf.String()) + } +} diff --git a/cmd/hostshift/audit_r60_test.go b/cmd/hostshift/audit_r60_test.go new file mode 100644 index 0000000..8ffc597 --- /dev/null +++ b/cmd/hostshift/audit_r60_test.go @@ -0,0 +1,66 @@ +package main + +import ( + "bytes" + "log/slog" + "strings" + "testing" + + "github.com/generoi/hostshift/internal/origin" +) + +// The test written to kill the constant-surface mutation cannot see it. +// +// Round 59 extracted `censusHook` because "the wiring was three mutations deep +// in unpinned code, and nothing failed when the hook was deleted, silenced to +// Debug, or given a constant surface", and wrote +// TestCensusHookWritesAGreppableLine against all three. Two of the three are +// dead now. The third is not: that test calls the hook exactly once, with +// `"html-attr"`, and then asserts the line contains `"html-attr"` — which is +// also true of a hook that ignores its argument. Replacing +// +// log.Info("census", "surface", surface, ...) +// +// with +// +// log.Info("census", "surface", "html-attr", ...) +// +// leaves `go test ./...` green, on the field the census exists to carry. +// +// It is the field, not a detail of it. `ddev hostshift check` tells a developer +// at a test-28 refusal to turn the census on and grep it, and the answer it is +// there to give is *which surface* — a Location on the way out, or a Referer on +// its way into the shared database, which round 59's own first finding is about +// telling apart. A hook that names one surface for every event answers that +// question wrongly for every event but one. +// +// Two surfaces, because one can never distinguish a variable from a constant. +func TestR60CensusHookNamesTheSurfaceItWasGiven(t *testing.T) { + line := func(surface string) string { + var buf bytes.Buffer + log := slog.New(slog.NewTextHandler(&buf, nil)) + censusHook(log)(surface, origin.Event{ + Action: origin.ActionRewrote, Offset: 42, + Text: "https://www.example.fi", + }) + return buf.String() + } + // The two the census is asked to tell apart: round 59's own finding is that + // filing one under the other's name sends the developer at a §4.3 write the + // diagnosis for a test-28 leak, and the reverse. + for _, surface := range []string{"response-header", "header", "request-body"} { + got := line(surface) + if !strings.Contains(got, "surface="+surface) { + t.Errorf("censusHook was given surface %q and did not write it:\n %s", + surface, got) + } + } + // And the pair differs, which is the assertion a single call cannot make: + // a hook that ignores its argument passes every containment check written + // against the one value it happens to emit. + if a, b := line("response-header"), line("header"); a == b { + t.Errorf("two different surfaces produced the same census line, so the field\n"+ + "`check` sends a developer to cannot distinguish a Location on the way\n"+ + "out from a Referer on its way into the shared database:\n %s", a) + } +} diff --git a/cmd/hostshift/audit_r60_wiring_test.go b/cmd/hostshift/audit_r60_wiring_test.go new file mode 100644 index 0000000..70f0c42 --- /dev/null +++ b/cmd/hostshift/audit_r60_wiring_test.go @@ -0,0 +1,128 @@ +package main + +import ( + "net" + "net/http" + "net/http/httptest" + "os" + "strings" + "syscall" + "testing" + "time" +) + +// Round 59 extracted the census helpers and left the wiring exactly as unpinned +// as it found it. +// +// The reason it gives for the extraction is explicit: "the wiring was three +// mutations deep in unpinned code, and nothing failed when the hook was +// deleted, silenced to Debug, or given a constant surface". After the fix, two +// of those three still pass `go test ./...`: +// +// if wantCensus(*explain, *dryRun) { -> if wantCensus(*explain, false) { +// if wantCensus(*explain, *dryRun) { st.OnEvent(censusHook(log)) } -> (deleted) +// +// TestCensusIsWiredForBothFlags tests `wantCensus` and +// TestCensusHookWritesAGreppableLine tests `censusHook`, and nothing tests that +// `cmdProxy` calls either. The two functions are now provably correct and +// provably unreached. +// +// It matters at the moment `ddev hostshift check` refuses a start on a test-28 +// leak, because what it tells the developer to do next is: add the flag, +// restart, `| grep census`. If the wiring is gone, or if `--dry-run` no longer +// reaches it, that instruction returns nothing on a proxy that is leaking — and +// PLAN §5.8 makes `--dry-run` the mode you point at a live canonical checkout +// to decide whether a site needs hostshift at all. +// +// Through the real command, because the wiring is the thing under test: +// `--dry-run` alone, since that is the half the call site can drop while both +// unit tests stay green. +func TestR60TheProxyCommandActuallyInstallsTheCensus(t *testing.T) { + up := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // A Tier 1 response header carrying a canonical origin: one rewrite, + // one census event, and no body to depend on. + w.Header().Set("Content-Location", "https://www.example.fi/x") + w.Header().Set("Content-Type", "text/plain") + w.WriteHeader(http.StatusOK) + })) + defer up.Close() + + // A port of our own, so the census can be read off a real request. + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + addr := ln.Addr().String() + ln.Close() + + dir := t.TempDir() + type result struct { + code int + errOut string + } + done := make(chan result, 1) + go func() { + code, _, errOut := run(t, "", cmdProxy, + "-C", dir, + "--upstream", up.URL, + "--map", "https://www.example.fi=https://wt-a--example.ddev.site", + "--listen", addr, + "--dry-run") + done <- result{code, errOut} + }() + + // Wait for it to be listening. Only then is the signal handler registered, + // which is what makes the SIGTERM below a drain rather than a kill. + deadline := time.Now().Add(10 * time.Second) + for { + c, err := net.DialTimeout("tcp", addr, 200*time.Millisecond) + if err == nil { + c.Close() + break + } + if time.Now().After(deadline) { + t.Fatal("the proxy never started listening") + } + time.Sleep(20 * time.Millisecond) + } + + req, err := http.NewRequest("GET", "http://"+addr+"/p", nil) + if err != nil { + t.Fatal(err) + } + req.Host = "wt-a--example.ddev.site" + res, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + res.Body.Close() + + if err := syscall.Kill(os.Getpid(), syscall.SIGTERM); err != nil { + t.Fatal(err) + } + var got result + select { + case got = <-done: + case <-time.After(30 * time.Second): + t.Fatal("the proxy did not drain") + } + if got.code != exitOK { + t.Fatalf("proxy exited %d:\n%s", got.code, got.errOut) + } + if !strings.Contains(got.errOut, "msg=census") { + t.Errorf("`--dry-run` rewrote a Tier 1 response header and wrote no census\n"+ + "line, so `check`'s instruction at a test-28 refusal — add the flag,\n"+ + "restart, `| grep census` — returns nothing on a proxy that is\n"+ + "leaking. wantCensus and censusHook are both tested; nothing tests\n"+ + "that cmdProxy calls them. stderr was:\n%s", got.errOut) + } + // And the marker that says nothing was applied. Round 60 added it because a + // dry-run census line was indistinguishable from a real rewrite, and this + // test asserted only that *a* census line appeared — so passing `false` + // where `*dryRun` belongs survived the whole suite. + if !strings.Contains(got.errOut, "dry-run=true") { + t.Errorf("this proxy is running with --dry-run and its census does not say\n"+ + "so, so a developer grepping it cannot tell a proxy that is rewriting\n"+ + "from one that is only reporting. stderr was:\n%s", got.errOut) + } +} diff --git a/cmd/hostshift/audit_r61_test.go b/cmd/hostshift/audit_r61_test.go new file mode 100644 index 0000000..6f17ad5 --- /dev/null +++ b/cmd/hostshift/audit_r61_test.go @@ -0,0 +1,111 @@ +package main + +import ( + "strings" + "testing" +) + +// Round 61, on 33cb2a6: `rewrite` is the third engine, and round 60 changed two. +// +// 33cb2a6 made `surfaceDecodesCSS(SurfaceText)` false — correctly, because +// nothing decodes a CSS escape in text/plain — and then split the text arm by +// media type so the XML family keeps the CSS view: "SurfaceXMLText for the XML +// family, SurfaceText for the rest, and the scorer makes the same choice by the +// same question." +// +// It names two call sites. There are three. `cmdRewrite`'s `rewritableText` arm +// is a line-for-line copy of the proxy's, down to the comment, and it still +// passes `rewrite.SurfaceText` on both branches — so the change that switched +// the CSS view off by name switched it off for every feed, sitemap and SVG this +// command is given, in the one direction round 60 went out of its way to keep. +// +// Measured against b9b5c0b: this same body rewrote under all three media types +// before the commit and rewrites under none of them after it. The command +// documents itself as "the same engine" and PLAN §7 leans on that; here it +// disagrees with the proxy about the identical bytes, and answers +// `"rewrites": {}` — the JSON `check`'s leak scan reads as a count of zero. +// +// ada, with the variant origin as base, on what a CSS tokenizer hands the URL +// parser after it unescapes `\3a` and `\2f`: +// +// new URL("https://www.acme.fi/a.png", "https://wt-a--acme.ddev.site/").host +// === "www.acme.fi" +// +// That is this map's canonical, in a `background: url(…)` the browser fetches — +// test 28, with the tool a developer reaches for to diagnose it reporting a +// clean body. +func TestR61RewriteXMLArmKeepsTheCSSView(t *testing.T) { + mapFlag := "https://www.acme.fi=https://wt-a--acme.ddev.site" + const svg = `` + + for _, mt := range []string{"image/svg+xml", "application/rss+xml", "text/xml"} { + t.Run(mt, func(t *testing.T) { + code, out, errOut := run(t, svg, cmdRewrite, + "--map", mapFlag, "--type", mt, "--json") + if code != exitOK { + t.Fatalf("exit %d\n%s", code, errOut) + } + if !strings.Contains(out, "wt-a--acme.ddev.site") { + t.Errorf("an SVG ` + + `

Hello

` + + served, err := io.ReadAll(rewrite.NewResponseBody( + strings.NewReader(page), m.Forward(), nil, rewrite.Options{})) + if err != nil { + t.Fatal(err) + } + if !bytes.Contains(served, []byte("v.ddev.site")) { + t.Fatalf("the engine did not rewrite the fixture at all, so this asserts nothing:\n%s", served) + } + // Nothing serialized anywhere in it, by construction. + if rewrite.BrokenSerialized([]byte(page)) != 0 || rewrite.BrokenSerialized(served) != 0 { + t.Fatalf("the fixture is not the healthy page this test needs") + } + + results, err := Run(context.Background(), Options{ + Canonical: r37serve(t, page), Variant: r37serve(t, string(served)), + Map: m, Paths: []string{"/"}, + }) + if err != nil { + t.Fatal(err) + } + var buf bytes.Buffer + green := WriteReport(&buf, results) + if results[0].UnreadRewrites != 0 { + t.Errorf("%d unread rewrite(s) reported on a page whose only serialized-shaped "+ + "bytes are `border:1px` in a stylesheet:\n page %s\n served %s\n%s", + results[0].UnreadRewrites, page, served, buf.String()) + } + if !green { + t.Errorf("the run was RED over a healthy page that round-trips exactly:\n%s", buf.String()) + } +} + +// The metric asks the byte matcher, but the proxy runs the whole engine — so a +// host the engine rewrites through one of its decoder views is edited without +// being counted. +// +// diff.go feeds `UnreadRewrites` this rewriter: +// +// out, _ := o.Map.Forward().Rewrite(b, rewrite.SurfaceText, false) +// +// `origin.Matcher.Rewrite` is the byte matcher alone. The proxy is not the byte +// matcher alone: `urlobf.go` adds the URL-parser view, the IDNA fold, the CSS +// view and the character-reference views, and those are what rewrite a host +// spelled `https://www.canon.test/terms`. `countLeaks` in this same file +// records having learned exactly this — "every leak class found since — +// obfuscated separators, folded hosts, CSS escapes, character references — was +// invisible by construction" — and was changed to push bytes back through the +// real pipeline. `UnreadRewrites` reintroduces the same blindness one column +// over, and here it hides corruption rather than a leak. +// +// The fixture is a serialized value in a spelling repairAt cannot read, whose +// URL separators are written as numeric references — the form §"obfuscated +// separators" exists for, and which the canonical page decodes to exactly the +// 28 bytes `s:28:` declares. The engine rewrites it (and decodes the +// references while it is there), the value is served as `s:28:` over 25 bytes, +// and PHP 8.4 `unserialize()` returns false. `BrokenSerialized` counts 1 on +// each side and cancels; `UnreadRewrites` reports 0 because its rewriter cannot +// see the host at all. GREEN. +func TestAHostBehindCharacterReferencesIsRewrittenWithoutBeingCounted(t *testing.T) { + m := r37map(t) + + canonURL := "https://www.canon.test/terms" + variantURL := "https://v.ddev.site/terms" + if len(canonURL) == len(variantURL) { + t.Fatalf("the fixture's two URLs are the same length, so this asserts nothing") + } + // Length from the *decoded* data, which is what PHP counts: the references + // below decode back to these same 28 bytes. + blob := fmt.Sprintf(`a:1:{i:0;s:%d:"%s";}`, len(canonURL), canonURL) + q := string([]byte{'\\', '\\', 'u', '0', '0', '2', '2'}) + wire := strings.ReplaceAll(strings.ReplaceAll(blob, `"`, q), "//", "//") + if !strings.Contains(wire, "https://www.canon.test") { + t.Fatalf("fixture does not carry the reference-obfuscated origin: %s", wire) + } + // Single-quoted attribute, and the value carries no `'` or `"` to close it. + if strings.ContainsAny(wire, "'\"") { + t.Fatalf("fixture would close its own attribute: %s", wire) + } + page := `
x
` + + served, err := io.ReadAll(rewrite.NewResponseBody( + strings.NewReader(page), m.Forward(), nil, rewrite.Options{})) + if err != nil { + t.Fatal(err) + } + if !bytes.Contains(served, []byte("v.ddev.site")) { + t.Fatalf("the engine did not rewrite the fixture at all, so this asserts nothing:\n%s", served) + } + if !bytes.Contains(served, []byte(fmt.Sprintf("s:%d:", len(canonURL)))) { + t.Skip("the walk now re-emits this length; nothing broken is served") + } + if !bytes.Contains(served, []byte(variantURL)) { + t.Skip("the engine no longer decodes the references here; the fixture needs rebuilding") + } + + results, err := Run(context.Background(), Options{ + Canonical: r37serve(t, page), Variant: r37serve(t, string(served)), + Map: m, Paths: []string{"/"}, + }) + if err != nil { + t.Fatal(err) + } + var buf bytes.Buffer + if green := WriteReport(&buf, results); green { + t.Errorf("the run was GREEN over a page served with s:%d: in front of %d bytes "+ + "(PHP unserialize() returns false):\n canonical %s\n served %s\n"+ + " unread=%d broken=%d\n%s", + len(canonURL), len(variantURL), page, served, + results[0].UnreadRewrites, results[0].BrokenSerialized, buf.String()) + } +} + +// A Tier 2 body the proxy never touches turns the run RED. +// +// diff.go computes `UnreadRewrites` at line 259, before it has looked at the +// content type at all: +// +// r.UnreadRewrites = rewrite.UnreadRewrites(canon.body, …) +// r.ContentType = variant.contentType +// r.Leaks, r.Tier2 = countLeaks(o.Map.Forward(), variant) +// +// `countLeaks` has the guards — `if r.attachment { return 0, 0 }` and the +// Tier 2 arm — because scoring a body the proxy is documented not to rewrite +// answers the wrong question, and `WriteReport` deliberately does not let a +// Tier 2 count turn the run red: "It does not turn the run RED, because the +// proxy is doing what it says it does". `UnreadRewrites` has no such guard and +// does turn the run red. +// +// So this stylesheet — which the proxy passes through byte for byte, which is +// therefore identical on both sides, and whose only serialized-shaped bytes are +// the `r:1` inside `border:1px` — is reported as an unread rewrite and the run +// is RED. The same is true of an attachment: §5 skips those by design, whatever +// bytes they contain. +func TestATier2BodyTheProxyNeverRewritesIsNotAnUnreadRewrite(t *testing.T) { + m := r37map(t) + + css := `.a{border:1px solid #eee;background:url(https://www.canon.test/x.png)}` + serve := func(body string) *url.URL { + s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/css") + _, _ = io.WriteString(w, body) + })) + t.Cleanup(s.Close) + u, err := url.Parse(s.URL) + if err != nil { + t.Fatal(err) + } + return u + } + + // Both sides identical: the proxy does not rewrite text/css, so this is + // exactly what it serves. + results, err := Run(context.Background(), Options{ + Canonical: serve(css), Variant: serve(css), + Map: m, Paths: []string{"/"}, + }) + if err != nil { + t.Fatal(err) + } + var buf bytes.Buffer + green := WriteReport(&buf, results) + if results[0].UnreadRewrites != 0 { + t.Errorf("%d unread rewrite(s) on a text/css body the proxy passes through "+ + "untouched:\n %s\n%s", results[0].UnreadRewrites, css, buf.String()) + } + if !green { + t.Errorf("the run was RED over a Tier 2 body, which WriteReport's own Tier 2 "+ + "arm says must not happen:\n%s", buf.String()) + } +} diff --git a/internal/corpus/audit_r40_test.go b/internal/corpus/audit_r40_test.go new file mode 100644 index 0000000..2a88036 --- /dev/null +++ b/internal/corpus/audit_r40_test.go @@ -0,0 +1,186 @@ +package corpus + +import ( + "bytes" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/generoi/hostshift/internal/origin" +) + +// The corpus diff's self-redirect carve-out is not the proxy's. +// +// PLAN §4.4 defines the guard exactly: "if rewriting `Location` canonical→variant +// would yield a URL equal to the incoming request URL, emit the `Location` +// unmodified". proxy.modifyResponse implements that — it calls sameURL against +// st.url, the request the browser made — and test 32 spells out the other half: +// "assert that a 3xx whose `Location` differs from the incoming request URL is +// still rewritten normally (a login redirect, test 1, must not be caught by the +// guard)". +// +// compare() in diff.go asks a much weaker question: +// +// unchangedSelfRedirect := !o.StrictOrigins && +// variant.location == canon.location && canon.location != "" +// +// There is no comparison against the path being fetched, so *any* Location the +// variant returned unchanged is exempted — not just the one that would loop. A +// canonical Location that the deployment failed to rewrite is by construction +// identical on both sides, which is the exact condition that switches the check +// off. +// +// The two tests below are the two halves of that: the shape PLAN says must be +// rewritten normally, and the whole-deployment failure the Location comparison +// was added to catch in the first place. +func r40Map(t *testing.T) *origin.Map { + t.Helper() + m, err := origin.NewMap([]origin.Site{{ + Name: "main", + Canonical: origin.MustParse("https://acme.ddev.site"), + Variant: origin.MustParse("https://wt-a--acme.ddev.site"), + }}) + if err != nil { + t.Fatal(err) + } + return m +} + +// TestALoginRedirectLeftUnrewrittenIsNotGreen is test 32's second half, asked of +// the scorer instead of the proxy. +// +// The request is for /wp-admin/ and the Location is /wp-login.php?redirect_to=… +// on the canonical origin — a different URL from the one asked for, so the +// self-redirect guard does not apply to it and the proxy rewrites it. A +// deployment that serves it unchanged sends the developer's browser to the +// production login form, which is a dereferenceable production origin reaching +// the browser: test 28. +// +// The scorer sees the same bytes on both sides and calls it a carve-out. +func TestALoginRedirectLeftUnrewrittenIsNotGreen(t *testing.T) { + const ( + reqPath = "/wp-admin/" + // Deliberately not the request URL: this is the login redirect PLAN + // test 32 names, not the redirect-uploads self-redirect. + leaked = "https://acme.ddev.site/wp-login.php?redirect_to=%2Fwp-admin%2F" + ) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Location", leaked) + w.WriteHeader(http.StatusFound) + })) + defer srv.Close() + + base, err := url.Parse(srv.URL) + if err != nil { + t.Fatal(err) + } + cl := srv.Client() + cl.CheckRedirect = func(*http.Request, []*http.Request) error { + return http.ErrUseLastResponse + } + + r := compare(t.Context(), Options{ + Map: r40Map(t), Canonical: base, Variant: base, Client: cl, + }, reqPath) + + if r.Err == nil { + t.Fatalf("the variant served %q for a request to %q — a canonical origin the "+ + "browser follows, and a URL the self-redirect guard does not cover — "+ + "and the scorer reported no error, so the run is GREEN: %+v", + leaked, reqPath, r) + } + if !strings.Contains(r.Err.Error(), "Location") { + t.Errorf("the reason does not name the header: %v", r.Err) + } +} + +// TestAnAllRedirectCrawlWithHostshiftOutOfThePathIsNotGreen is the failure the +// Location comparison exists for, verbatim from diff.go's own comment: +// +// "Comparing bodies alone scored an all-redirect crawl as '3 pages, 3 +// byte-identical, 0 leaks — GREEN' with hostshift not in the path at all, +// because two empty bodies are equal. The shapes that produce such a crawl +// are the documented ones: a worktree whose database is empty redirects every +// page to install.php…" +// +// So: a worktree with an empty database, and --variant-base pointing at the +// canonical site rather than at the proxy. Every page 302s to install.php on +// the canonical origin, the two sides are byte-identical because they are the +// same server, and the carve-out cancels the one assertion that could have +// noticed. The report says "no canonical origin reached the browser" while +// every page hands the browser a production URL. +func TestAnAllRedirectCrawlWithHostshiftOutOfThePathIsNotGreen(t *testing.T) { + const install = "https://acme.ddev.site/wp-admin/install.php" + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Location", install) + w.WriteHeader(http.StatusFound) + })) + defer srv.Close() + + // Two parses, because this models a *misconfigured* --variant-base: the + // flag was pointed at the canonical site, so hostshift is nowhere in the + // path. fetch() distinguishes the two sides by pointer. + canon, err := url.Parse(srv.URL) + if err != nil { + t.Fatal(err) + } + variant, err := url.Parse(srv.URL) + if err != nil { + t.Fatal(err) + } + + results, err := Run(t.Context(), Options{ + Map: r40Map(t), + Canonical: canon, + Variant: variant, + Paths: []string{"/", "/kotiasiakkaille/", "/yhteystiedot/"}, + }) + if err != nil { + t.Fatal(err) + } + + var out bytes.Buffer + green := WriteReport(&out, results) + if green { + t.Fatalf("every page served %q and the run is GREEN — hostshift is not in "+ + "the path at all:\n%s", install, out.String()) + } +} + +// The self-redirect exemption needs both halves: the same path, and a host the +// map actually names as a variant. +// +// PLAN §4.4 defines the guard as "rewriting this Location would yield the URL +// the browser just requested". Both halves of that matter, and neither is +// implied by the other: a redirect to a *different* path on the right host is +// an ordinary redirect that must be rewritten (test 32's login case), and a +// redirect to the same path on someone else's host is not this deployment's +// redirect at all. +func TestTheSelfRedirectExemptionNeedsPathAndHost(t *testing.T) { + mp, err := origin.NewMap([]origin.Site{{ + Name: "main", Canonical: origin.MustParse("https://acme.ddev.site"), + Variant: origin.MustParse("https://wt-a--acme.ddev.site"), + }}) + if err != nil { + t.Fatal(err) + } + for name, tc := range map[string]struct { + loc string + want bool + }{ + "the same path on a variant host": {"https://wt-a--acme.ddev.site/a", true}, + "a different path": {"https://wt-a--acme.ddev.site/b", false}, + "a different query": {"https://wt-a--acme.ddev.site/a?x=1", false}, + "a third-party host": {"https://cdn.example.net/a", false}, + "the canonical host": {"https://acme.ddev.site/a", false}, + } { + // Options rather than the bare map: the exemption is about *the* variant + // being crawled, so it needs to know which one that is. + o := Options{Map: mp, Variant: &url.URL{Scheme: "https", Host: "wt-a--acme.ddev.site"}} + if got := redirectsToItself(tc.loc, o, "/a"); got != tc.want { + t.Errorf("%s: redirectsToItself(%q) = %v, want %v", name, tc.loc, got, tc.want) + } + } +} diff --git a/internal/corpus/audit_r41_test.go b/internal/corpus/audit_r41_test.go new file mode 100644 index 0000000..55ab481 --- /dev/null +++ b/internal/corpus/audit_r41_test.go @@ -0,0 +1,201 @@ +package corpus + +import ( + "bytes" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/generoi/hostshift/internal/origin" +) + +// redirectsToItself compares an origin against half of one. +// +// PLAN §5.3: "the map is origin→origin (scheme + host + port), never +// host→host". The proxy's guard honours that — normaliseURL runs the Location +// and the request URL through origin.Parse and compares HostPort() — and it +// compares against *the one request the browser made*, st.url. +// +// diff.go:337 asks a different question in both halves: +// +// for _, site := range m.Sites { +// if strings.EqualFold(u.Host, site.Variant.Host) { +// return true +// } +// } +// +// - `site.Variant.Host` is the hostname with the port deliberately stripped +// (origin.Origin keeps Port separately, "" when default), while `u.Host` +// is host:port. They can never be equal for a variant with a port, and are +// equal for two different origins that share a hostname. +// - `m.Sites` is *every* site in the map, not the one being crawled. The +// fleet's 12 multisite repos carry N from 2 to 9 (PLAN §"N→N mapping"), so +// "some variant in the map" and "the variant the browser is on" routinely +// differ. +// +// Each half fails in a different direction, and the tests below are one of +// each. +func r41Map(t *testing.T, sites ...origin.Site) *origin.Map { + t.Helper() + m, err := origin.NewMap(sites) + if err != nil { + t.Fatal(err) + } + return m +} + +// TestACrossSiteCanonicalRedirectIsNotSilentlyGreen is the false GREEN. +// +// A two-site network: www.a.fi and www.b.fi, each with its own variant. The +// secondary domain 301s every path to the same path on the primary — the +// ordinary shape of a consolidated or aliased network domain — so a crawl of +// site A's variant answers, on every page: +// +// HTTP/1.1 301 +// Location: https://www.b.fi/ +// +// That is a dereferenceable production origin handed to the browser, which +// follows it: test 28's failure, and the "hostshift is not in the path at all" +// shape audit_r40_test.go was written for. +// +// The proxy does not exempt it. Its guard is sameURL(rewritten, st.url), and +// st.url is "https://" + the Host the browser used — site A's variant — while +// the rewritten Location names site B's. They differ, so modifyResponse +// rewrites the header, which is why a working deployment never produces these +// bytes. +// +// The scorer exempts it anyway: rewriting the Location yields site B's variant +// host, that host is in m.Sites, and the path matches, so redirectsToItself +// returns true and the Location assertion is skipped. Two empty bodies are +// equal, so nothing else objects and WriteReport prints GREEN. +func TestACrossSiteCanonicalRedirectIsNotSilentlyGreen(t *testing.T) { + mp := r41Map(t, + origin.Site{ + Name: "secondary", + Canonical: origin.MustParse("https://www.a.fi"), + Variant: origin.MustParse("https://wt--a.ddev.site"), + }, + origin.Site{ + Name: "primary", + Canonical: origin.MustParse("https://www.b.fi"), + Variant: origin.MustParse("https://wt--b.ddev.site"), + }, + ) + + // The mechanical statement first: crawling site A's variant, a Location + // naming site B's is *not* a self-redirect. It used to be exempted — the + // exemption accepted any variant in the map rather than the one the browser + // is on — and that is what let the crawl below go green. + crawlingA := Options{Map: mp, Variant: &url.URL{Scheme: "https", Host: "wt--a.ddev.site"}} + if redirectsToItself("https://wt--b.ddev.site/kotiasiakkaille/", crawlingA, "/kotiasiakkaille/") { + t.Fatal("a redirect to another site's variant is still exempted") + } + // And site A's own self-redirect still is, so this is not a blanket refusal. + if !redirectsToItself("https://wt--a.ddev.site/kotiasiakkaille/", crawlingA, "/kotiasiakkaille/") { + t.Fatal("the crawled site's own self-redirect stopped being exempted") + } + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Same path, other host. Not a self-redirect by PLAN §4.4's definition + // — rewriting it does not yield the URL the browser asked for, because + // the browser is on site A's variant and this names site B's. + w.Header().Set("Location", "https://www.b.fi"+r.URL.EscapedPath()) + w.WriteHeader(http.StatusMovedPermanently) + })) + defer srv.Close() + + // Two parses: fetch() tells the sides apart by pointer, and this models + // --variant-base pointed at a deployment hostshift is not in front of. + canon, err := url.Parse(srv.URL) + if err != nil { + t.Fatal(err) + } + variant, err := url.Parse(srv.URL) + if err != nil { + t.Fatal(err) + } + + results, err := Run(t.Context(), Options{ + Map: mp, + Canonical: canon, + Variant: variant, + Paths: []string{"/", "/kotiasiakkaille/", "/yhteystiedot/"}, + }) + if err != nil { + t.Fatal(err) + } + + var out bytes.Buffer + if WriteReport(&out, results) { + t.Fatalf("every page handed the browser a Location on https://www.b.fi — a "+ + "production origin in the map, at a URL the proxy's own guard would "+ + "have rewritten — and the run is GREEN:\n%s", out.String()) + } +} + +// TestTheSelfRedirectExemptionSurvivesAPortedVariant is the false RED, and the +// same line. +// +// `hostshift proxy --listen 127.0.0.1:8080` outside DDEV is a documented mode +// (PLAN §"hostshift proxy --upstream http://web:80 --listen 127.0.0.1:8080"), +// and `variant: http://127.0.0.1:8080` is a config the loader accepts and +// config_test.go asserts. On such a map the browser is on localhost:8080, so +// the proxy's guard exempts redirect-uploads exactly as §4.4 says: +// normaliseURL puts both sides through origin.HostPort(), which keeps the +// port, and "localhost:8080/app/uploads/x.jpg" == "localhost:8080/app/uploads/x.jpg". +// +// The scorer compares u.Host ("localhost:8080") against site.Variant.Host +// ("localhost" — Origin keeps the port in a separate field), so the exemption +// can never fire. 87% of the fleet ships redirect-uploads.conf and 95.2% of +// referenced uploads are absent locally, so every page linking an upload turns +// the run RED on a deployment doing exactly what PLAN §4.4 prescribes — which +// is the outcome diff.go's own comment says the carve-out exists to prevent. +func TestTheSelfRedirectExemptionSurvivesAPortedVariant(t *testing.T) { + const path = "/app/uploads/2025/07/x.jpg" + mp := r41Map(t, origin.Site{ + Name: "main", + Canonical: origin.MustParse("https://www.acme.fi"), + Variant: origin.MustParse("http://localhost:8080"), + }) + + // What the proxy sees on the wire, spelled out: the raw Location, and what + // the forward map turns it into. + loc := "https://www.acme.fi" + path + rewritten, _ := mp.Forward().Rewrite([]byte(loc), "header", false) + if got, want := string(rewritten), "http://localhost:8080"+path; got != want { + t.Fatalf("the premise no longer holds: rewrite(%q) = %q, want %q", loc, got, want) + } + // The proxy's sameURL(rewritten, st.url) with st.url = "https://" + Host + + // RequestURI. origin.HostPort() keeps the port on both sides, so this is + // true and the proxy passes the Location through per §4.4 and test 32. + ported := Options{Map: mp, Variant: &url.URL{Scheme: "http", Host: "localhost:8080"}} + if !redirectsToItself(string(rewritten), ported, path) { + t.Fatalf("the proxy exempts %q for a request to %q (sameURL normalises both "+ + "through origin.HostPort, which keeps :8080), and the scorer does not — "+ + "so every page linking an upload is RED on a healthy deployment", + rewritten, path) + } + + // And end to end, because a verdict is what a developer reads. + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Location", "https://www.acme.fi"+r.URL.EscapedPath()) + w.WriteHeader(http.StatusFound) + })) + defer srv.Close() + base, err := url.Parse(srv.URL) + if err != nil { + t.Fatal(err) + } + cl := srv.Client() + cl.CheckRedirect = func(*http.Request, []*http.Request) error { + return http.ErrUseLastResponse + } + r := compare(t.Context(), Options{ + Map: mp, Canonical: base, Variant: base, Client: cl, + }, path) + if r.Err != nil && strings.Contains(r.Err.Error(), "Location") { + t.Errorf("a documented carve-out reported as an error on a ported variant: %v", r.Err) + } +} diff --git a/internal/corpus/audit_r42_test.go b/internal/corpus/audit_r42_test.go new file mode 100644 index 0000000..24bfa94 --- /dev/null +++ b/internal/corpus/audit_r42_test.go @@ -0,0 +1,185 @@ +package corpus + +import ( + "bytes" + "context" + "io" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" +) + +// Round 42, on 94d2cd0 ("stop failing on one dns-prefetch"). +// +// The line-count assertion was demoted from fatal to a note. The commit's +// justification is that the delta it saw was exactly one line, every time, from +// a Host-dependent ``, and that what it stood in for is +// now covered by `broken` and `unread`. +// +// The demotion is total, not bounded: *any* line delta, of any magnitude, is now +// a note. And nothing else in WriteReport turns byte inequality into RED — +// `green = false` is set only by Err, Leaks, UnreadRewrites and BrokenSerialized. +// So after the demotion the scorer has no assertion at all about the *size* of +// what the proxy served, and the two cases below are GREEN. +// +// The report does not merely stay silent. It prints, in the same breath as a row +// saying the line count went 1800→0: +// +// corpus diff GREEN: no canonical origin reached the browser, no page re-serialised +// +// which is the sentence a developer reads, and it is false: the page it just +// scored lost every line it had. WriteReport's own doc comment still promises +// "no page lost or gained lines", and Result.LinesCanonical's still says "a +// line-count change means something re-serialised". + +// emptyBodyVariant: the proxy answers 200 with nothing in it. +// +// Both sides 200, no Location, so the "empty body and no Location: nothing was +// verified" guard does not fire — it requires *both* bodies to be empty, and the +// canonical one is a full page. The variant carries no canonical origin because +// it carries nothing, so Leaks is 0; there is no serialized value to be broken +// or unread. Before 94d2cd0 the 2→0 line count made this RED; the +// unbounded demotion made it GREEN, and the bound restores it. +// +// This is the "hostshift is not in the path at all" shape that the Location +// comparison above it, audit_r40 and audit_r41 were each written to close, and +// it is back through the one door those did not cover. +func TestAnEmptyVariantBodyFailsTheRun(t *testing.T) { + canonical := map[string]string{ + "/": "a\n

and a second line

\n", + } + blank := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/html") + w.WriteHeader(http.StatusOK) + })) + defer blank.Close() + variant, err := url.Parse(blank.URL) + if err != nil { + t.Fatal(err) + } + + results, err := Run(t.Context(), Options{ + Canonical: site(t, canonical), Variant: variant, Map: testMap(t), Paths: []string{"/"}, + }) + if err != nil { + t.Fatal(err) + } + // The premise, so a fixture mistake cannot be mistaken for the finding: + // the variant really is empty, and the two sides really do disagree. + if len(results) != 1 { + t.Fatalf("crawled %d pages, want 1", len(results)) + } + r := results[0] + if r.Equal { + t.Fatal("fixture: the two sides must differ or this tests nothing") + } + if r.LinesVariant != 0 || r.LinesCanonical == 0 { + t.Fatalf("fixture: want a full canonical page and an empty variant, got %d/%d", + r.LinesCanonical, r.LinesVariant) + } + if r.Err != nil { + t.Fatalf("fixture: no error was expected here, got %v", r.Err) + } + + var buf bytes.Buffer + green := WriteReport(&buf, results) + if green { + t.Errorf("the proxy served an empty body for every page and the run is GREEN:\n%s", + buf.String()) + } + // And the verdict line asserts, in words, the thing it stopped checking. + if green && strings.Contains(buf.String(), "no page re-serialised") { + t.Errorf("GREEN claims \"no page re-serialised\" for a page that went %d lines to %d:\n%s", + r.LinesCanonical, r.LinesVariant, buf.String()) + } +} + +// truncatedVariant: the proxy served the first half of the page. +// +// Less extreme than the empty body and harder to notice: a response cut short +// mid-document — an upstream that dies mid-stream, a Content-Length that +// disagrees with the bytes, a rewriter that stops early. The half that survives +// carries no canonical origin, so every other column is clean, and the only +// evidence the scorer ever had that half the page is missing was the line count. +func TestATruncatedVariantFailsTheRun(t *testing.T) { + head := "\n\nt\n\n\n" + tail := "a\n\n\n" + canonical := map[string]string{"/": head + tail} + + cut := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/html") + io.WriteString(w, head) + })) + defer cut.Close() + variant, err := url.Parse(cut.URL) + if err != nil { + t.Fatal(err) + } + + results, err := Run(t.Context(), Options{ + Canonical: site(t, canonical), Variant: variant, Map: testMap(t), Paths: []string{"/"}, + }) + if err != nil { + t.Fatal(err) + } + r := results[0] + if r.LinesCanonical == r.LinesVariant { + t.Fatalf("fixture: the truncation must change the line count, got %d/%d", + r.LinesCanonical, r.LinesVariant) + } + if r.Leaks != 0 || r.BrokenSerialized != 0 || r.UnreadRewrites != 0 || r.Err != nil { + t.Fatalf("fixture: every other column must be clean, or this does not isolate "+ + "the line count: leaks=%d broken=%d unread=%d err=%v", + r.Leaks, r.BrokenSerialized, r.UnreadRewrites, r.Err) + } + + var buf bytes.Buffer + if WriteReport(&buf, results) { + t.Errorf("the proxy served %d of %d lines and the run is GREEN:\n%s", + r.LinesVariant, r.LinesCanonical, buf.String()) + } +} + +// TestALongPageLosingATailFailsTheRun: the other half of the bound. +// +// The two cases above both lose a quarter or more of a short document, so the +// proportional half of the bound catches them and the absolute half is never +// exercised. This is the shape that needs the absolute half: a page long enough +// that a lost tail is a small fraction of it. 100 lines down to 88 is 12% — well +// inside the ratio — and no Host-dependent markup produces twelve lines. +func TestALongPageLosingATailFailsTheRun(t *testing.T) { + var full strings.Builder + for i := 0; i < 100; i++ { + full.WriteString("

line

\n") + } + whole, tail := full.String(), strings.Repeat("

line

\n", 88) + + canonical := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/html") + io.WriteString(w, whole) + })) + defer canonical.Close() + variant := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/html") + io.WriteString(w, tail) + })) + defer variant.Close() + + cu, _ := url.Parse(canonical.URL) + vu, _ := url.Parse(variant.URL) + results, err := Run(context.Background(), Options{ + Canonical: cu, Variant: vu, Map: testMap(t), N: 1, + }) + if err != nil { + t.Fatal(err) + } + if results[0].Leaks != 0 { + t.Fatalf("this fixture should leak nothing; it tests line counts") + } + var buf bytes.Buffer + if WriteReport(&buf, results) { + t.Errorf("the proxy served 88 of 100 lines and the run is GREEN:\n%s", buf.String()) + } +} diff --git a/internal/corpus/audit_r43_test.go b/internal/corpus/audit_r43_test.go new file mode 100644 index 0000000..dba8946 --- /dev/null +++ b/internal/corpus/audit_r43_test.go @@ -0,0 +1,225 @@ +package corpus + +import ( + "bytes" + "context" + "io" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" +) + +// Round 43, on e37c0b0 ("Bound the line-count check instead of deleting it"). +// +// The bound is expressed entirely in newlines: `strings.Count(body, "\n")` on +// each side, then `d > hostDependentLines || d*4 > r.LinesCanonical`. A document +// that contains no newlines has a line count of 0 whatever is in it — and 0 is +// also the line count of nothing at all. So for that whole class of document the +// two counts are equal by construction, the bound is never consulted, and the +// scorer is back to having no assertion about the size of what the proxy served. +// +// The class is not exotic. It is every minified page — WP Rocket, Autoptimize, +// LiteSpeed Cache and Cloudflare's own minifier all emit one line — and every +// JSON body, which `--paths` reaches routinely. +// +// r42's own TestAnEmptyVariantBodyFailsTheRun is the same fixture with two +// newlines in the canonical page. Remove them and the run is GREEN again, under +// the verdict line the same commit extended to say "every page the length it +// should be". + +// TestAMinifiedPageTruncatedToNothingFailsTheRun: the canonical page is a whole +// document on one line; the variant answers 200 with nothing. Both sides 200 and +// no Location, so the "empty body and no Location" guard does not fire — it +// needs *both* bodies empty. Nothing reached the browser, so Leaks is 0; there +// is no serialized value to be broken or unread. The only thing that could +// notice is the size bound, and newlines are the only unit it can count in. +func TestAMinifiedPageTruncatedToNothingFailsTheRun(t *testing.T) { + // One line, no trailing newline: what an HTML minifier serves. + page := "Acme" + + "a" + + "

the whole document, on one line, as a minifier emits it

" + + "" + if strings.Contains(page, "\n") { + t.Fatal("fixture: the canonical page must have no newline in it") + } + canonical := map[string]string{"/": page} + + blank := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/html") + w.WriteHeader(http.StatusOK) + })) + defer blank.Close() + variant, err := url.Parse(blank.URL) + if err != nil { + t.Fatal(err) + } + + results, err := Run(t.Context(), Options{ + Canonical: site(t, canonical), Variant: variant, Map: testMap(t), Paths: []string{"/"}, + }) + if err != nil { + t.Fatal(err) + } + if len(results) != 1 { + t.Fatalf("crawled %d pages, want 1", len(results)) + } + r := results[0] + // The premise, stated so that a fixture mistake cannot be read as the + // finding: the variant really is empty, the canonical really is a page, and + // the two really do disagree. + if r.Err != nil { + t.Fatalf("fixture: %v", r.Err) + } + if r.Equal { + t.Fatal("fixture: the two sides must differ or this tests nothing") + } + if r.Leaks != 0 || r.BrokenSerialized != 0 || r.UnreadRewrites != 0 { + t.Fatalf("fixture: this must be a page nothing else in the report objects to, got "+ + "leaks=%d broken=%d unread=%d", r.Leaks, r.BrokenSerialized, r.UnreadRewrites) + } + if r.LinesCanonical != 0 || r.LinesVariant != 0 { + t.Fatalf("fixture: a newline-free page and an empty body both count 0 lines, got %d/%d", + r.LinesCanonical, r.LinesVariant) + } + + var buf bytes.Buffer + if WriteReport(&buf, results) { + t.Errorf("the proxy served an empty body for a whole page and the run is GREEN:\n%s", + buf.String()) + } +} + +// TestAMinifiedPageServedInHalfFailsTheRun is the truncation shape rather than +// the empty one: the upstream dies mid-stream and the browser gets the first +// third of the document. On a one-line page that is still 0 lines against 0 +// lines. +func TestAMinifiedPageServedInHalfFailsTheRun(t *testing.T) { + page := "Acme" + + "a" + + strings.Repeat("

body copy that the browser never receives

", 40) + + "" + cut := page[:60] + if strings.Contains(page, "\n") { + t.Fatal("fixture: the canonical page must have no newline in it") + } + + half := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/html") + w.Write([]byte(cut)) + })) + defer half.Close() + variant, err := url.Parse(half.URL) + if err != nil { + t.Fatal(err) + } + + results, err := Run(t.Context(), Options{ + Canonical: site(t, map[string]string{"/": page}), Variant: variant, + Map: testMap(t), Paths: []string{"/"}, + }) + if err != nil { + t.Fatal(err) + } + r := results[0] + if r.Err != nil { + t.Fatalf("fixture: %v", r.Err) + } + if r.Equal { + t.Fatal("fixture: the two sides must differ or this tests nothing") + } + // The premise: the browser received under a tenth of the document. + if len(cut)*10 > len(page) { + t.Fatalf("fixture: %d of %d bytes is not a truncation", len(cut), len(page)) + } + if r.LinesCanonical != r.LinesVariant { + t.Fatalf("fixture: both sides must count 0 lines, got %d/%d", + r.LinesCanonical, r.LinesVariant) + } + + var buf bytes.Buffer + if WriteReport(&buf, results) { + t.Errorf("the proxy served %d bytes of a %d-byte page and the run is GREEN:\n%s", + len(cut), len(page), buf.String()) + } +} + +// TestTheVerdictDoesNotClaimWhatTier2SkipsWasChecked: a production-canonical run +// with live origins inside Elementor CSS printed, two lines apart: +// +// 4 origins in Tier 2 types (text/css, JavaScript), which the proxy excludes… +// corpus diff GREEN: no canonical origin reached the browser, … +// +// and exited 0. The exclusion is designed and stays; the sentence asserting the +// one thing invariant 28 forbids, about bytes the run never looked at, cannot — +// this is the command the README calls the check that validates a deployment +// against reality, and anything gating on its exit status was guarding nothing. +func TestTheVerdictDoesNotClaimWhatTier2SkipsWasChecked(t *testing.T) { + css := `body{background:url(` + canonicalOrigin + `/bg.png)}` + "\n" + page := func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/css") + io.WriteString(w, css) + } + canonical := httptest.NewServer(http.HandlerFunc(page)) + defer canonical.Close() + variant := httptest.NewServer(http.HandlerFunc(page)) + defer variant.Close() + + cu, _ := url.Parse(canonical.URL) + vu, _ := url.Parse(variant.URL) + results, err := Run(context.Background(), Options{ + Canonical: cu, Variant: vu, Map: testMap(t), N: 1, + }) + if err != nil { + t.Fatal(err) + } + var buf bytes.Buffer + green := WriteReport(&buf, results) + if results[0].Tier2 == 0 { + t.Fatalf("fixture: this page should carry a Tier 2 origin:\n%s", buf.String()) + } + // Still green — the exclusion is a decision, not a defect. + if !green { + t.Fatalf("Tier 2 must not fail the run; that is PLAN §5.2:\n%s", buf.String()) + } + // But the sentence must not assert what it skipped. + out := buf.String() + if strings.Contains(out, "GREEN: no canonical origin reached the browser") { + t.Errorf("the verdict claims Tier 2 bytes were checked:\n%s", out) + } + if !strings.Contains(out, "did reach it in Tier 2") { + t.Errorf("the verdict does not say origins reached the browser:\n%s", out) + } +} + +// TestTheCrawlFetchesTheStylesheetsThePageLinks: `links` collected `` +// only, so a default run never fetched a stylesheet — and the Tier 2 line, which +// PLAN's fast path names as its trigger for rewriting CSS, could not fire from +// the command the README points at for exactly that evidence. +// +// Measured before the fix: a page linking its own ``, whose +// file carried a live production origin, scored "3 pages, 0 leaks" and GREEN +// while curl on that stylesheet through the proxy returned the canonical URL. +func TestTheCrawlFetchesTheStylesheetsThePageLinks(t *testing.T) { + pages := map[string]string{ + "/": ``, + "/s.css": `body{background:url(` + canonicalOrigin + `/bg.png)}`, + "/j.js": `var u = "` + canonicalOrigin + `/x";`, + } + results, err := Run(context.Background(), Options{ + Canonical: site(t, pages), Variant: site(t, pages), Map: testMap(t), N: 10, + }) + if err != nil { + t.Fatal(err) + } + got := map[string]bool{} + for _, r := range results { + got[r.Path] = true + } + for _, want := range []string{"/s.css", "/j.js"} { + if !got[want] { + t.Errorf("the crawl never fetched %s; it saw %v", want, got) + } + } +} diff --git a/internal/corpus/audit_r45_test.go b/internal/corpus/audit_r45_test.go new file mode 100644 index 0000000..93aa28c --- /dev/null +++ b/internal/corpus/audit_r45_test.go @@ -0,0 +1,185 @@ +package corpus + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/generoi/hostshift/internal/origin" +) + +// Round 45, on 8fabead ("Fit each condition to its defect"). +// +// 8fabead taught `links` to follow ``, ``, i) + } + b.WriteString("") + for i := 0; i < pages; i++ { + fmt.Fprintf(&b, `p%d`, i, i) + } + b.WriteString("") + return b.String() +} + +// TestR45TheCrawlBudgetIsSpentOnAssets. +// +// `crawl` stops at `o.N` *paths*, and `hostshift diff`'s default is `-n 20`. +// Every subresource now competes for that budget with the pages, and it wins, +// because a WordPress `` is emitted before the `` and the queue is +// FIFO. Measured below on a page with an ordinary number of enqueued assets: +// the default run compares the homepage and nineteen files from its head, and +// **not one other page**. +// +// That is not a wash. Tier 2 is documented as *not* failing a run — WriteReport +// prints it and leaves `green` alone — so what the change buys is a louder note, +// and what it spends is nineteen pages of the only check in the tool that can +// turn RED for invariant 28. The README calls this "the only test that validates +// against reality"; before 8fabead it validated twenty pages and now it +// validates one. +// +// The fix is a budget per kind rather than one shared queue: crawl to `-n` +// *pages* as before, and fetch the subresources those pages reference on top of +// it (or behind their own flag). Nothing about reaching a stylesheet requires +// spending a page slot on it. +func TestR45TheCrawlBudgetIsSpentOnAssets(t *testing.T) { + home := r45Page(15, 10) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/html; charset=utf-8") + if r.URL.Path == "/" { + _, _ = w.Write([]byte(home)) + return + } + _, _ = w.Write([]byte("x")) + })) + defer srv.Close() + + base, err := url.Parse(srv.URL) + if err != nil { + t.Fatal(err) + } + m, err := origin.NewMap([]origin.Site{{ + Name: "r45", + Canonical: origin.MustParse(srv.URL), + Variant: origin.MustParse(srv.URL), + }}) + if err != nil { + t.Fatal(err) + } + + // -n 20, the documented default of `hostshift diff`. + paths, err := crawl(context.Background(), Options{ + Canonical: base, Variant: base, Map: m, N: 20, Client: srv.Client(), + }) + if err != nil { + t.Fatal(err) + } + + var pages, assets int + for _, p := range paths { + switch { + case strings.HasPrefix(p, "/page-"), p == "/": + pages++ + default: + assets++ + } + } + t.Logf("crawled %d paths: %d pages, %d assets\n%s", len(paths), pages, assets, + strings.Join(paths, "\n")) + if pages < 2 { + t.Errorf("the whole -n 20 budget went to subresources: %d page(s), %d asset(s).\n"+ + "Before 8fabead this crawl compared 11 pages; it now compares 1 and 19 files "+ + "from its head, and Tier 2 — the reason the subresources were added — does not "+ + "fail a run.", pages, assets) + } +} + +// TestR45AnAssetOnlyCrawlDoesNotClaimTheInvariant28Verdict. +// +// Every row is a `text/css` file, which the proxy is documented not to rewrite. +// Each is byte-identical, so nothing clears `green` — and the run used to print +// "corpus diff GREEN: no canonical origin reached the browser", a sentence about +// HTML over a table with no HTML in it. `len(results) > 0` is satisfied by +// twenty rows that answered a different question. +// +// Still green: Tier 2 must not fail a run, which is +// TestATier2BodyTheProxyNeverRewritesIsNotAnUnreadRewrite. What it must not do +// is claim an invariant nothing in the run tested. +func TestR45AnAssetOnlyCrawlDoesNotClaimTheInvariant28Verdict(t *testing.T) { + var results []Result + for i := 0; i < 20; i++ { + results = append(results, Result{ + Path: fmt.Sprintf("/wp-content/themes/t/a%d.css", i), + Equal: true, + ContentType: "text/css", + // Byte-identical, so lines and bytes agree and nothing is cleared. + LinesCanonical: 1, LinesVariant: 1, + BytesCanonical: 100, BytesVariant: 100, + }) + } + var b strings.Builder + green := WriteReport(&b, results) + if !green { + t.Fatalf("expected the report to score this GREEN; it did not:\n%s", b.String()) + } + if strings.Contains(b.String(), "no canonical origin reached the browser") { + t.Errorf("the verdict claims an invariant no row in this run tested:\n%s", b.String()) + } + if !strings.Contains(b.String(), "nothing in this run is a type the proxy rewrites") { + t.Errorf("the verdict does not say the run scanned nothing:\n%s", b.String()) + } + t.Logf("report:\n%s", b.String()) +} + +// TestR45APageTwoLinksDeepStillBeatsTheFirstPagesAssets: why the subresources +// need their own queue and not merely their own ordering within a page. +// +// Separating them in `links` puts a page's own links ahead of its own assets. +// It does not put the *next* page's links ahead of the previous page's assets: +// with one FIFO, `/` enqueues [/b, …30 assets], popping /b appends /c behind +// those assets, and a shallow budget never reaches /c. Draining pages first +// makes depth beat breadth-of-assets, which is what a crawl is for. +func TestR45APageTwoLinksDeepStillBeatsTheFirstPagesAssets(t *testing.T) { + pages := map[string]string{ + "/": `b`, + "/b/": `c`, + "/c/": `

the page a shallow budget must still reach

`, + } + // Thirty assets on the first page, all enqueued before /b is popped. + var head strings.Builder + for i := 0; i < 30; i++ { + fmt.Fprintf(&head, ``, i) + pages[fmt.Sprintf("/a%d.css", i)] = "body{}" + } + pages["/"] = head.String() + pages["/"] + + results, err := Run(context.Background(), Options{ + Canonical: site(t, pages), Variant: site(t, pages), Map: testMap(t), N: 5, + }) + if err != nil { + t.Fatal(err) + } + got := map[string]bool{} + for _, r := range results { + got[r.Path] = true + } + for _, want := range []string{"/b/", "/c/"} { + if !got[want] { + t.Errorf("a budget of 5 never reached %s; it spent itself on assets: %v", want, got) + } + } +} diff --git a/internal/corpus/audit_r46_test.go b/internal/corpus/audit_r46_test.go new file mode 100644 index 0000000..d58f8e3 --- /dev/null +++ b/internal/corpus/audit_r46_test.go @@ -0,0 +1,70 @@ +package corpus + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/generoi/hostshift/internal/origin" +) + +// Round 46. 598de7c put pages ahead of subresources in the crawl. Probe: on a +// site with more linked pages than the budget, is a stylesheet ever fetched? +func TestR46SubresourcesStarveOnARealSite(t *testing.T) { + // A homepage with one stylesheet and 40 links, and every linked page links + // 40 more — an ordinary WordPress menu. + page := func(prefix string) string { + var b strings.Builder + b.WriteString(``) + for i := 0; i < 40; i++ { + fmt.Fprintf(&b, `l`, prefix, i) + } + b.WriteString(``) + return b.String() + } + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, ".css") { + w.Header().Set("Content-Type", "text/css") + _, _ = w.Write([]byte("body{}")) + return + } + w.Header().Set("Content-Type", "text/html; charset=utf-8") + _, _ = w.Write([]byte(page(strings.Trim(r.URL.Path, "/") + "x"))) + })) + defer srv.Close() + + base, err := url.Parse(srv.URL) + if err != nil { + t.Fatal(err) + } + m, err := origin.NewMap([]origin.Site{{ + Name: "r46", + Canonical: origin.MustParse(srv.URL), + Variant: origin.MustParse(srv.URL), + }}) + if err != nil { + t.Fatal(err) + } + paths, err := crawl(context.Background(), Options{ + Canonical: base, Variant: base, Map: m, N: 20, Client: srv.Client(), + }) + if err != nil { + t.Fatal(err) + } + css := 0 + for _, p := range paths { + if strings.HasSuffix(p, ".css") { + css++ + } + } + t.Logf("crawled %d paths, %d of them css:\n %s", len(paths), css, strings.Join(paths, "\n ")) + if css == 0 { + t.Errorf("no subresource was fetched at all, so Result.Tier2 — the count "+ + "links() exists to make reachable — is structurally zero on any site with "+ + "more linked pages than -n. paths=%v", paths) + } +} diff --git a/internal/corpus/audit_r47_test.go b/internal/corpus/audit_r47_test.go new file mode 100644 index 0000000..17af082 --- /dev/null +++ b/internal/corpus/audit_r47_test.go @@ -0,0 +1,75 @@ +package corpus + +import ( + "bytes" + "context" + "net/url" + "strings" + "testing" + + "github.com/generoi/hostshift/internal/origin" +) + +// Round 47. +// +// The engine cannot read a JSON `\uXXXX` escape of a non-ASCII host — see +// internal/rewrite's TestR47AnIDNCanonicalEscapedByWPJSONEncodeReachesTheBrowser +// for why that spelling is what wp_json_encode emits on every page of a site +// with an IDN canonical. This is what the corpus diff says about such a page. +// +// originsIn "asks the whole engine, not the byte matcher alone", and its comment +// explains that asking the matcher alone made every leak class the engine could +// not see invisible to the one test §7 calls the only one that validates against +// reality. The engine is a better oracle than the matcher, and it is still the +// engine: a spelling the engine cannot read is a spelling this cannot count. So +// the page below is served to the browser with a live production origin in it, +// the diff reads it byte for byte, and prints GREEN. +func TestR47TheDiffIsGreenOnAPageThatLeaksAnEscapedIDN(t *testing.T) { + const canon = "https://www.hämeenlinna.fi" + // The six bytes wp_json_encode writes for the `ä`. + const escaped = "www.h" + "\\u00e4" + "meenlinna.fi" + + m, err := origin.NewMap([]origin.Site{{ + Name: "hml", + Canonical: origin.MustParse(canon), + Variant: origin.MustParse("https://wt-a--hml.ddev.site"), + }}) + if err != nil { + t.Fatal(err) + } + + page := `` + pages := map[string]string{"/": page} + + // The proxy changed nothing, because it could not see anything — so the + // variant response is the canonical bytes verbatim. + var canonURL, variantURL *url.URL + canonURL = site(t, pages) + variantURL = site(t, pages) + + results, err := Run(context.Background(), Options{ + Canonical: canonURL, Variant: variantURL, Map: m, N: 5, + }) + if err != nil { + t.Fatal(err) + } + if len(results) != 1 { + t.Fatalf("crawled %d pages, want 1", len(results)) + } + + var buf bytes.Buffer + green := WriteReport(&buf, results) + if results[0].Leaks == 0 { + t.Errorf("the production origin in this page was not counted as a leak; "+ + "the browser resolves it to %s/wp-admin/admin-ajax.php:\n%s", canon, page) + } + if green { + t.Errorf("and the run is GREEN, so the check the README calls "+ + "\"the check that validates a deployment against reality\" signs off on it:\n%s", + buf.String()) + } + if strings.Contains(buf.String(), "CANONICAL ORIGIN REACHED THE BROWSER") { + t.Logf("report named the failure:\n%s", buf.String()) + } +} diff --git a/internal/corpus/audit_r58_test.go b/internal/corpus/audit_r58_test.go new file mode 100644 index 0000000..597a760 --- /dev/null +++ b/internal/corpus/audit_r58_test.go @@ -0,0 +1,83 @@ +package corpus + +import ( + "bytes" + "context" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/generoi/hostshift/internal/rewrite" +) + +// `hostshift diff` scores a redirect against a narrower pipeline than the proxy +// runs, and the difference is every view urlobf.go exists for. +// +// diff.go computes its expected Location with `Map.Forward().Rewrite(...)` — the +// byte matcher alone. modifyResponse computes the real one with +// `RepairSerialized(Rewrite → HostLeaksCounted)`. So on every spelling the byte +// matcher cannot see, the scorer's `want` is the canonical origin unchanged: the +// correct variant Location the proxy emitted is reported as a mismatch whose +// wanted value is the production URL, on the run the README calls the check that +// validates a deployment against reality and PLAN §7 calls the only one that +// does. +// +// ada: `new URL("https:\\c.example/x", "https://v.example/p").href` is +// `https://c.example/x`, so the header is a dereferenceable production origin +// and the proxy must rewrite it. A scorer that wants it left alone names a test +// 28 leak as the desired state — and by the same narrowness cannot see the +// inverse, because a Location the proxy failed to rewrite is byte-identical on +// both sides *and* equal to this `want`, which scores GREEN. +// +// The same narrowness drops RepairSerialized, so a `Location: +// /landing.php?state=` whose length the proxy repaired is a mismatch too. +func TestR58DiffScoresTheLocationTheProxyActuallyEmits(t *testing.T) { + m := testMap(t) + + // An obfuscated absolute Location: `\` is a slash to the URL parser, so this + // is the canonical origin to a browser and the byte matcher cannot see it. + const loc = `https:\\c.example/x` + + fwd := m.Forward() + st := rewrite.NewStats(false) + // modifyResponse's Tier 1 header expression, verbatim. + served := string(rewrite.RepairSerialized([]byte(loc), func(b []byte) []byte { + nv, _ := fwd.Rewrite(b, rewrite.SurfaceResponseHeader, false) + return rewrite.HostLeaksCounted(fwd, nv, true, st, rewrite.SurfaceResponseHeader, 0) + })) + if served == loc { + t.Fatalf("fixture is not a rewrite: the proxy leaves %q alone", loc) + } + if bs, _ := fwd.Rewrite([]byte(loc), rewrite.SurfaceResponseHeader, false); string(bs) != loc { + t.Fatalf("fixture is not locator-only: the byte matcher already rewrites %q", loc) + } + + results, err := Run(context.Background(), Options{ + Canonical: r58Redirector(t, loc), + Variant: r58Redirector(t, served), + Map: m, N: 3, + }) + if err != nil { + t.Fatal(err) + } + var buf bytes.Buffer + if !WriteReport(&buf, results) { + t.Errorf("the scorer red-flagged the Location the proxy actually emits, and "+ + "its `want` is the production origin:\n%s", buf.String()) + } +} + +func r58Redirector(t *testing.T, loc string) *url.URL { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Location", loc) + w.WriteHeader(http.StatusFound) + })) + t.Cleanup(srv.Close) + u, err := url.Parse(srv.URL) + if err != nil { + t.Fatal(err) + } + return u +} diff --git a/internal/corpus/audit_r59_test.go b/internal/corpus/audit_r59_test.go new file mode 100644 index 0000000..8e5c20c --- /dev/null +++ b/internal/corpus/audit_r59_test.go @@ -0,0 +1,156 @@ +package corpus + +import ( + "bytes" + "context" + "strings" + "testing" + + "github.com/generoi/hostshift/internal/origin" + "github.com/generoi/hostshift/internal/rewrite" +) + +// Round 59, on 05ae5ea. `hostshift diff`'s Location scorer, the mirror of the +// response-header emission pipeline. + +// Round 58 moved the scorer onto the pipeline the proxy runs and left half of it +// behind. +// +// `compare` now computes: +// +// RepairSerialized(Rewrite(SurfaceResponseHeader) → HostLeaks(…, true)) +// +// and `HostLeaks` takes no surface: it renames the buffer with `bareSurface`, +// which for `value == true` is `SurfaceHTMLAttr`. `surfaceDecodesCSS` is written +// as `surface != SurfaceResponseHeader`, so the rename turns the CSS view back +// on — in the one stage of the expression that does the leak-backstop work. The +// proxy calls `HostLeaksCounted(…, SurfaceResponseHeader, 0)` and gets the view +// off. So the scorer still expects, on exactly the shapes round 58's own finding +// was about, something the proxy does not emit: measured over the corpus, +// 3,622 header-safe Location spellings (3,496 css-encoded, 126 raw). +// +// ada, with the variant origin as base: +// +// new URL("/go.php?u=https://c.exampl\\65/x", "https://v.example/p").href === +// "https://v.example/go.php?u=https://c.exampl\\65/x" +// +// The backslash survives into the query byte for byte: `\65` is a *CSS* hex +// escape for `e`, and neither the URL parser that follows the Location nor the +// PHP that reads `u` runs a CSS tokenizer. So the value names `c.exampl` and +// this map's canonical is nowhere in it. The proxy is right to leave it alone; +// the scorer wants it turned into the variant and prints a RED whose "want" is +// an origin ada never resolves this to, on the run the README calls the check +// that validates a deployment against reality. That is PLAN §566 again: a +// carve-out must be as narrow in the check as it is in the code. +// +// A relative Location and not an absolute one, because `diff`'s own fetcher +// cannot carry the absolute form: `url.Parse` rejects a backslash in a host, so +// `compare` reports "failed to parse Location header" before the scorer is +// reached at all. The reachable half is the ordinary one — a `redirect_to` +// carried in the query, in the spelling a form encoder writes. +func TestR59TheScorerRunsTheBackstopOnTheHeaderSurface(t *testing.T) { + m := testMap(t) + + for _, loc := range []string{ + // The literal CSS escape, and the percent-encoded spelling `post.php` + // sends back — the `hasPercentCSSEsc` cell, gated by the same call. + `/go.php?u=https://c.exampl\65/x`, + `/go.php?u=https%3A%2F%2Fc.exampl%5C65%2Fx`, + } { + t.Run(loc, func(t *testing.T) { + results, err := Run(context.Background(), Options{ + Canonical: r58Redirector(t, loc), + Variant: r58Redirector(t, loc), + Map: m, N: 3, + }) + if err != nil { + t.Fatal(err) + } + var buf bytes.Buffer + if !WriteReport(&buf, results) { + t.Errorf("nothing decodes a CSS escape in a Location, so the proxy correctly\n"+ + "leaves this alone and both sides are byte-identical — the scorer\n"+ + "red-flags it anyway, because HostLeaks renames the surface to\n"+ + "html-attr and turns the CSS view back on:\n%s", buf.String()) + } + if s := buf.String(); strings.Contains(s, "v.example%2Fx") || + strings.Contains(s, variantOrigin+"/x") { + t.Errorf("the scorer's `want` names %s, which nothing resolves this "+ + "Location to:\n%s", variantOrigin, s) + } + }) + } +} + +// And the half round 58 did add, which nothing pins: the length re-emission. +// +// `RepairSerialized` is what makes a `Location` carrying a serialized blob come +// out with a length that still describes it. Removing the wrapper from `compare` +// leaves every test in the tree green, so the scorer could drift back to +// expecting `s:30` against a proxy that emits `s:39` — and a stale length inside +// an `s:N:"…"` is the wp_options corruption serialized.go exists to prevent. +// +// The expectation is arithmetic, not a second reading of the code: +// `https://www.example.fi/landing` is 30 bytes and +// `https://wt-a--example.ddev.site/landing` is 39. +func TestR59TheScorerRepairsTheLengthItReEmits(t *testing.T) { + m, err := origin.NewMap([]origin.Site{{ + Name: "main", + Canonical: origin.MustParse("https://www.example.fi"), + Variant: origin.MustParse("https://wt-a--example.ddev.site"), + }}) + if err != nil { + t.Fatal(err) + } + const canonLoc = `https://www.example.fi/go.php?state=s:30:"https://www.example.fi/landing";` + const variantLoc = `https://wt-a--example.ddev.site/go.php?state=s:39:"https://wt-a--example.ddev.site/landing";` + + results, err := Run(context.Background(), Options{ + Canonical: r58Redirector(t, canonLoc), + Variant: r58Redirector(t, variantLoc), + Map: m, N: 3, + }) + if err != nil { + t.Fatal(err) + } + var buf bytes.Buffer + if !WriteReport(&buf, results) { + t.Errorf("the proxy repairs the length when it splices a longer host into a\n"+ + "serialized Location; a scorer that does not expect the repair red-flags\n"+ + "the correct emission and its `want` carries a stale s:30:\n%s", buf.String()) + } +} + +// The scorer's census names the arm it ran, like the proxy's. +// +// Round 60 split the text arm by media type in both engines and said they "make +// the same choice by the same question". Mutating the scorer's `st.Record` +// surface back to `text` survived the whole suite: the entire census for the +// commonest XML case — a feed `` — moves to the wrong surface unnoticed, +// on the field `check` tells a developer to grep at a test-28 refusal. +func TestR61TheScorerCensusNamesTheArm(t *testing.T) { + m, err := origin.NewMatcher([]origin.Pair{{ + Canonical: origin.MustParse("https://www.canon.test"), + Variant: origin.MustParse("https://v.ddev.site"), + }}) + if err != nil { + t.Fatal(err) + } + for _, c := range []struct{ ctype, body, want string }{ + {"application/rss+xml", + `https://www.canon.test/x`, + rewrite.SurfaceXMLText}, + {"text/plain", `see https://www.canon.test/x here`, rewrite.SurfaceText}, + } { + t.Run(c.ctype, func(t *testing.T) { + st := rewrite.NewStats(false) + if _, err := applyLikeTheProxy(m, []byte(c.body), c.ctype, st); err != nil { + t.Fatal(err) + } + if st.Snapshot().Rewrites[c.want] == 0 { + t.Errorf("a %s body rewrote nothing under %q; the census says %v", + c.ctype, c.want, st.Snapshot().Rewrites) + } + }) + } +} diff --git a/internal/corpus/audit_r68_test.go b/internal/corpus/audit_r68_test.go new file mode 100644 index 0000000..9ca1b80 --- /dev/null +++ b/internal/corpus/audit_r68_test.go @@ -0,0 +1,72 @@ +package corpus + +import ( + "bytes" + "encoding/base64" + "strconv" + "testing" +) + +// The leak column has to see base64 for the same reason the write-back column +// does, and it still cannot. +// +// Round 67 taught `WriteBacks` to look inside base64 +// (internal/corpus/diff.go:377, TestAVariantOriginInsideBase64IsCountedAsAWriteBack) +// because `countLeaks` runs the rewrite pipeline and the pipeline has no base64 +// view. That reasoning is direction-free — it is a property of the pipeline, not +// of which map is pointed at it — but the fix was applied to one direction only. +// `HiddenInBase64` is called in exactly two places in the whole tree +// (internal/proxy/proxy.go:859 and internal/corpus/diff.go:377) and both look +// for a *variant* origin. Nothing anywhere looks for a *canonical* one. +// +// So the mirror image of round 67's fixture — the same widget instance, with +// production's hostname inside it, served through the proxy to the developer's +// browser — is scored: +// +// Leaks 0, WriteBacks 0, byte-identical, GREEN +// +// on the run PLAN §7 calls "the only test that validates against reality". And +// the page is not inert. `WP_REST_Widgets_Controller` returns a legacy widget's +// settings as `instance.encoded` = base64(serialize(...)); the widgets screen +// and the Customizer decode it in JavaScript and render the widget, so an +// `` or an `` inside it becomes a live production URL in the +// developer's authenticated browser. That is test 28. +// +// As with the write-back column, the ask is not that the proxy rewrite it — +// `wp_hash()` covers exactly those bytes, so rewriting makes WordPress discard +// the save, and PLAN §4.3 accepts that. The ask is that something say so. The +// request direction already does, with a WARN naming the decoded blob +// (internal/proxy/proxy.go:859-874). The response direction and this report say +// nothing at all, which is the state PLAN §4.3 describes as the one that "went +// unnoticed for twenty-two rounds". +func TestACanonicalOriginInsideBase64IsCountedAsALeak(t *testing.T) { + inner := `promo` + blob := base64.StdEncoding.EncodeToString([]byte( + `a:1:{s:7:"content";s:` + strconv.Itoa(len(inner)) + `:"` + inner + `";}`)) + body := `
x
` + + // The proxy cannot rewrite it, so both sides carry the same bytes — which is + // exactly why byte comparison cannot see this and a scan has to. + r := compareBodies(t, body, body) + if r.WriteBacks != 0 { + t.Fatalf("this fixture carries no variant origin, got %d write-backs", r.WriteBacks) + } + if r.Leaks == 0 { + t.Error("a production origin inside base64 was served to the browser and the " + + "report counted nothing — the mirror of the hole round 67 closed in the " + + "other direction") + } + var buf bytes.Buffer + if WriteReport(&buf, []Result{r}) { + t.Errorf("the run went GREEN on a page whose base64 decodes to a live "+ + "production origin:\n%s", buf.String()) + } + + // A blob with nothing mapped in it stays at zero, so this is not just + // "any base64 is suspicious". + clean := base64.StdEncoding.EncodeToString([]byte(`a:1:{s:7:"content";s:9:"/promo/ x";}`)) + if q := compareBodies(t, `
x
`, + `
x
`); q.Leaks != 0 { + t.Errorf("an ordinary base64 blob reported %d leaks", q.Leaks) + } +} diff --git a/internal/corpus/audit_r74_test.go b/internal/corpus/audit_r74_test.go new file mode 100644 index 0000000..791709d --- /dev/null +++ b/internal/corpus/audit_r74_test.go @@ -0,0 +1,90 @@ +package corpus + +import ( + "bytes" + "testing" + + "github.com/generoi/hostshift/internal/origin" +) + +// TestR74CountLeaksIsBlindWhereTheEngineIs shows the amplifier directly, on the +// bytes a running proxy actually served. +// +// countLeaks scores a served body by pushing it back through the same pipeline +// the proxy ran on it. That is right for every origin the engine can see, and +// silent for every origin it cannot: an origin the engine declines going out is +// declined again coming in, and the page scores GREEN. Every engine defect is +// therefore also a hole in "the only test that validates against reality". +// +// The body below is stock WordPress 7.1's `wp-admin/site-health.php?tab=debug`, +// reduced to the two lines that matter: the copy-to-clipboard report is one +// attribute value whose lines are separated by raw LF, and a bare origin before +// one of those is not rewritten (see +// proxy.TestR74BareOriginBeforeAControlInAnAttributeIsNotRewritten). Measured +// against the real thing: `hostshift diff` printed +// +// /r74attr.php same 0 17/17 +// corpus diff GREEN: no canonical origin reached the browser +// +// on a page whose served bytes hold `https://www.hostshift-a.example` twice. +// +// A plain `bytes.Contains` of the canonical host is the check that does not +// have that property, because it shares no code with the thing it is checking. +// The cost is false positives on origins the proxy declines on purpose — a bare +// hostname in prose, which the wp-admin sweep below did hit once. Anchoring on +// `//host` rather than `host` removes those: over 65 real pages of a WordPress +// multisite served through the proxy (front end, the whole of wp-admin, network +// admin, REST, feeds, sitemaps) the anchored form fired exactly once, on this +// true positive, and never otherwise. +func TestR74CountLeaksIsBlindWhereTheEngineIs(t *testing.T) { + c, err := origin.Parse("https://www.r74a.example") + if err != nil { + t.Fatal(err) + } + v, err := origin.Parse("https://wt-a--r74w.ddev.site") + if err != nil { + t.Fatal(err) + } + m, err := origin.NewMap([]origin.Site{{Name: "s", Canonical: c, Variant: v}}) + if err != nil { + t.Fatal(err) + } + + // Exactly what the proxy served: the table value rewritten, the attribute + // value not. + served := []byte(`` + + `
WP_HOMEhttps://wt-a--r74w.ddev.site
` + + ``) + + // The independent check, in one line. + independent := bytes.Count(served, []byte("//www.r74a.example")) + if independent == 0 { + t.Fatal("harness: the fixture carries no canonical origin at all") + } + + leaks, tier2 := countLeaks(m.Forward(), response{ + body: served, status: 200, contentType: "text/html; charset=UTF-8", + }) + if tier2 != 0 { + t.Fatalf("harness: text/html is not a Tier 2 type, got tier2=%d", tier2) + } + // countLeaks re-runs the engine, so it cannot see what the engine declines — + // that is the property being recorded, not a bug to assert away. This test + // asserted `leaks != 0` when it was written; the remedy shipped is the one + // this round's report recommended instead, a separate counter, because a + // literal scan also sees origins PLAN says to leave alone and folding it into + // `Leaks` would make every Tier 2 page RED. + if leaks != 0 { + t.Logf("countLeaks now finds %d here; the independent check is still the "+ + "one that cannot inherit an engine mistake", leaks) + } + if got := literalOrigins(m, served); got != independent { + t.Errorf("the independent check found %d canonical origins, the fixture "+ + "has %d — it is the only witness that does not share code with the "+ + "engine, so it has to see them", got, independent) + } +} diff --git a/internal/corpus/audit_r75_test.go b/internal/corpus/audit_r75_test.go new file mode 100644 index 0000000..f55b65d --- /dev/null +++ b/internal/corpus/audit_r75_test.go @@ -0,0 +1,115 @@ +package corpus + +import ( + "bytes" + "strings" + "testing" + + "github.com/generoi/hostshift/internal/origin" +) + +// The literal-scan pointer fires on every Tier 2 body, where nothing was +// declined and the engine was never asked. +// +// Round 74 added `Result.Literal` for one purpose: to catch a canonical origin +// that the *engine* saw and let through, because `countLeaks` re-runs the +// pipeline and so cannot witness its own declines. `WriteReport` reports the +// difference as exactly that — "the difference is something the engine +// declined". +// +// For a Tier 2 body that sentence is false by construction. `countLeaks` +// short-circuits at `isTier2` and returns `leaks = 0` before the engine is run +// at all (diff.go:529), while `literalOrigins` still counts every `//host` in +// the same bytes. So `r.Literal > r.Leaks` holds for *any* Tier 2 body carrying +// a literal canonical origin, and the same origins are then added to two +// separate totals — the "a literal scan found and the engine did not" line and +// the "origins in Tier 2 types" line. +// +// Measured on a real deployment rather than argued: a DDEV WordPress with +// Autoptimize 3.1.15.1 aggregating inline CSS/JS, `hostshift diff` printed +// +// /wp-content/cache/autoptimize/1/autoptimize_0… same 0 25/25 +// a literal scan finds 2 canonical origin(s) here and the engine reported 0 +// …; 2 origins in a Tier 2 type (text/css; charset=utf-8) +// … +// 8 canonical origin(s) a literal scan found and the engine did not +// 10 origins in Tier 2 types (text/css, JavaScript) +// +// Eight of those ten are the same origins counted twice, and all eight of the +// literal-scan hits on that run were Tier 2 bodies — so the pointer added to +// find engine declines found none, and reported eight. PLAN's own rule about +// `mayHoldSerialized` applies: a check that fires on every page carrying the +// thing it is supposed to be quiet about carries no information. +func TestR75LiteralPointerFiresOnEveryTier2Body(t *testing.T) { + c, err := origin.Parse("https://www.r75a.example") + if err != nil { + t.Fatal(err) + } + v, err := origin.Parse("https://wt-a--r75w.ddev.site") + if err != nil { + t.Fatal(err) + } + m, err := origin.NewMap([]origin.Site{{Name: "s", Canonical: c, Variant: v}}) + if err != nil { + t.Fatal(err) + } + + // An Autoptimize-shaped aggregate: a theme's @font-face, absolutised into + // the cache file because the file moved directory. + css := []byte(`@font-face{font-family:Manrope;src:url('https://www.r75a.example` + + `/wp-content/themes/tt5/assets/fonts/manrope/Manrope.woff2') format('woff2')}` + + `.x{background:url("https://www.r75a.example/wp-content/uploads/bg.png")}`) + + r := response{body: css, status: 200, contentType: "text/css; charset=utf-8"} + leaks, tier2 := countLeaks(m.Forward(), r) + lit := literalOrigins(m, css) + + if !isTier2(r.contentType) { + t.Fatal("harness: text/css is supposed to be a Tier 2 type") + } + if lit == 0 { + t.Fatal("harness: the fixture carries no literal canonical origin") + } + if leaks != 0 { + t.Fatalf("harness: countLeaks is documented to return 0 leaks for a "+ + "Tier 2 body, got %d", leaks) + } + if tier2 != lit { + t.Logf("tier2=%d literal=%d — they need not be equal, only both non-zero", + tier2, lit) + } + // This is the whole point: the report's condition is satisfied without the + // engine having declined anything, because the engine was never run. + if !(lit > leaks) { + t.Fatalf("expected the literal pointer's condition to hold on a Tier 2 "+ + "body: literal=%d leaks=%d", lit, leaks) + } +} + +// The fix this round recommends: say what is true, and count each origin once. +// +// Skipped, so the current behaviour is not asserted as correct and a fix does +// not have to delete a passing test. Un-skip it with the change. +func TestR75Tier2OriginsAreNotReportedAsEngineDeclines(t *testing.T) { + + c, _ := origin.Parse("https://www.r75a.example") + v, _ := origin.Parse("https://wt-a--r75w.ddev.site") + m, _ := origin.NewMap([]origin.Site{{Name: "s", Canonical: c, Variant: v}}) + + css := []byte(`.x{background:url("https://www.r75a.example/wp-content/uploads/bg.png")}`) + leaks, tier2 := countLeaks(m.Forward(), response{ + body: css, status: 200, contentType: "text/css; charset=utf-8"}) + + var buf bytes.Buffer + WriteReport(&buf, []Result{{ + Path: "/a.css", ContentType: "text/css; charset=utf-8", + Leaks: leaks, Tier2: tier2, Literal: literalOrigins(m, css), + Equal: true, + }}) + out := buf.String() + if strings.Contains(out, "the engine declined") || + strings.Contains(out, "a literal scan found and the engine did not") { + t.Errorf("a Tier 2 body is not an engine decline — the engine is never "+ + "run on one — yet the report says it is:\n%s", out) + } +} diff --git a/internal/corpus/diff.go b/internal/corpus/diff.go index e8c7f21..c0022dc 100644 --- a/internal/corpus/diff.go +++ b/internal/corpus/diff.go @@ -7,10 +7,12 @@ package corpus import ( + "bytes" "context" "crypto/tls" "fmt" "io" + "mime" "net" "net/http" "net/url" @@ -34,6 +36,11 @@ type Options struct { Paths []string // explicit paths; when empty, crawl from "/" Client *http.Client + // StrictOrigins mirrors the proxy flag of the same name: with the + // self-redirect carve-out turned off there, an unchanged Location is a + // mismatch here too. + StrictOrigins bool + // CanonicalHeaders are added to the canonical fetch only. // // When the canonical base is resolved past the TLS-terminating router @@ -62,13 +69,53 @@ type Result struct { // Equal reports byte equality between the rewritten canonical page and the // page the proxy served. Equal bool + // Tier2 counts canonical origins in a body the proxy is documented not to + // rewrite — `text/css` and the JavaScript types. PLAN's fast path says they + // are "added only if the corpus diff shows a leak", so a non-zero count here + // is this tool's designed trigger for adding them rather than a defect. + Tier2 int + + // ContentType is what the variant response was labelled, because the proxy + // dispatches on it and a verdict that ignores it is scoring a body against a + // pipeline that never ran. + ContentType string + + // BrokenSerialized counts PHP-serialized values in the variant response + // whose declared length does not describe their data. PHP refuses those, or + // worse, truncates them silently and keeps parsing. + BrokenSerialized int + // UnreadRewrites counts spans the rewrite changed inside something + // serialized-shaped that no spelling could read. See rewrite.UnreadRewrites: + // it reports what BrokenSerialized cannot, because it is host-dependent and + // so does not cancel against the canonical baseline. + UnreadRewrites int + + // Literal counts canonical origins found by a plain byte scan of the variant + // response, sharing no code with the rewriter. See compare: the engine cannot + // be the only witness to its own declines. + Literal int + + // WriteBacks counts variant origins in the *canonical* response: production + // serving a worktree hostname, which is §4.3 and means the shared database + // was written through the proxy. Zero on a healthy site. + WriteBacks int + // Leaks counts canonical origins in the variant response. Any non-zero // value is a test 28 failure and is what this whole exercise is for. Leaks int - // LinesCanonical and LinesVariant should match even when bytes do not: - // splicing never rebuilds whitespace, so a line-count change means - // something re-serialised. + // LinesCanonical and LinesVariant are close even when bytes are not: + // splicing never rebuilds whitespace. They are not identical — the two + // fetches carry different Host headers, and WordPress emits Host-dependent + // markup — so a small delta is reported and a large one is fatal, per + // hostDependentLines. LinesCanonical, LinesVariant int + // BytesCanonical and BytesVariant are the same question in a unit every + // document has. Lines are not: minified HTML — WP Rocket, Autoptimize, + // LiteSpeed and Cloudflare's minifier all emit one — and every JSON body + // count zero lines however much or little is in them, so for that whole + // class the line counts were equal by construction and the size bound never + // ran at all. + BytesCanonical, BytesVariant int DiffLines int Err error } @@ -84,7 +131,15 @@ func Run(ctx context.Context, o Options) ([]Result, error) { if len(o.Resolve) > 0 { base := &net.Dialer{Timeout: timeout} tr.DialContext = func(ctx context.Context, network, addr string) (net.Conn, error) { - if to, ok := o.Resolve[addr]; ok { + // The keys are folded where they are built, by ResolveKey, and + // Through the same fold as the guard. net/http punycodes the + // host but does not lowercase it — `idnaASCII` returns an ASCII + // host unchanged and `canonicalAddr` folds nothing — so a + // mis-cased `--canonical-base` was "covered" by the guard and a + // miss for the dialer, which then went to real DNS with nothing + // printed. Round 41 keyed the guard and the map through one + // function; this is the third side of the same question. + if to, ok := o.Resolve[ResolveKey(addr)]; ok { addr = to } return base.DialContext(ctx, network, addr) @@ -106,13 +161,20 @@ func Run(ctx context.Context, o Options) ([]Result, error) { } paths := o.Paths - if len(paths) == 0 { + crawled := len(paths) == 0 + if crawled { var err error if paths, err = crawl(ctx, o); err != nil { return nil, err } } - if o.N > 0 && len(paths) > o.N { + // `-n` bounds a supplied list here, and the crawl there. + // + // Truncating the crawl's own output undid its budgeting: it returns pages + // and subresources, sorted, so `/a0.css` sorts ahead of `/b/` and cutting at + // `-n` kept the assets and dropped the pages — the starvation the two + // budgets exist to prevent, reintroduced one function up. + if !crawled && o.N > 0 && len(paths) > o.N { paths = paths[:o.N] } @@ -137,18 +199,100 @@ func compare(ctx context.Context, o Options, path string) Result { return r } - // The canonical bytes through the same engine the proxy runs. - want, err := io.ReadAll(rewrite.NewResponseBody( - strings.NewReader(string(canon)), o.Map.Forward(), nil, rewrite.Options{})) + // A redirect verifies nothing about the body, and its Location is the header + // this design worries about most. + // + // Comparing bodies alone scored an all-redirect crawl as "3 pages, 3 + // byte-identical, 0 leaks — GREEN" with hostshift not in the path at all, + // because two empty bodies are equal. The shapes that produce such a crawl + // are the documented ones: a worktree whose database is empty redirects every + // page to install.php, and a login-walled preview does the same. The README + // calls this the check that validates a deployment against reality. + if canon.status != variant.status { + r.Err = fmt.Errorf("status %d canonical, %d variant", canon.status, variant.status) + return r + } + if canon.location != "" || variant.location != "" { + // The pipeline the proxy actually runs, not the byte matcher alone. + // + // `modifyResponse` puts a Location through RepairSerialized(Rewrite → + // HostLeaks), which is twelve views and a length re-emission; this + // computed its expectation from `Rewrite` by itself. Measured across + // 236,250 Location shapes, 100,863 of them expect something the proxy + // does not emit — so the check the README calls "validates a deployment + // against reality" red-flagged the *correct* rewrite and printed the + // production URL as the wanted value. Worse in the other direction: a + // Location the proxy failed to rewrite is byte-identical on both sides + // and equal to this expectation too, so it scored GREEN. + // + // PLAN §566 states the rule this broke — a carve-out must be as narrow + // in the check as it is in the code — and redirectsToItself below reads + // this same string, so the self-redirect exemption was being decided on + // it as well. + wantLoc := rewrite.RepairSerialized([]byte(canon.location), func(b []byte) []byte { + nv, _ := o.Map.Forward().Rewrite(b, rewrite.SurfaceResponseHeader, false) + // Named, not renamed. HostLeaks routes through bareSurface, which + // calls a header value an html-attr — and the CSS view keys on the + // name, so the backstop half decoded escapes the proxy's does not. + // 3,622 of 458,200 header-safe Location shapes then expected + // something the proxy never emits, on the run the README calls + // "validates a deployment against reality". + return rewrite.HostLeaksCounted(o.Map.Forward(), nv, true, nil, + rewrite.SurfaceResponseHeader, 0) + }) + // The self-redirect carve-out is not a mismatch. PLAN §4.4 and test 32 + // enumerate it as correct: an asset the worktree does not have is + // redirected to the canonical origin *on purpose*, which is what + // redirect-uploads.conf does in 87% of the fleet with 95.2% of referenced + // uploads absent locally. Flagging it made a RED verdict the ordinary + // outcome on any page linking a PDF or an attachment, which is how a + // verdict stops being read. + // + // Under --strict-origins the guard is off in the proxy too, so the + // exemption goes with it. + // + // And it is the *proxy's* guard, which asks whether rewriting the + // Location would yield the URL the browser just requested — PLAN §4.4's + // wording, and `sameURL(rewritten, st.url)` in modifyResponse. This + // asked only whether the Location came back unchanged, which is a + // strictly wider question and, worse, is the exact signature of the + // failure it is meant to sit beside: an unrewritten canonical Location + // is byte-identical on both sides *by construction*. So the check + // switched itself off precisely when it was needed. An all-redirect + // crawl with hostshift out of the path — the case the comment above + // records this Location comparison as having been added to catch — was + // GREEN again, and so was a login redirect that PLAN test 32 names as + // one the guard must not cover. + unchangedSelfRedirect := !o.StrictOrigins && + variant.location == canon.location && canon.location != "" && + redirectsToItself(string(wantLoc), o, path) + if string(wantLoc) != variant.location && !unchangedSelfRedirect { + r.Err = fmt.Errorf("Location %q, want %q", variant.location, wantLoc) + return r + } + // A redirect with a matching Location and no body is verified; one with a + // body still has its body compared below. + } + if len(canon.body) == 0 && len(variant.body) == 0 && canon.location == "" { + r.Err = fmt.Errorf("empty body and no Location: nothing was verified") + return r + } + + // The canonical bytes through the same arm the proxy would run for this + // content type — not the HTML pipeline regardless, which made `want` + // byte-identical to a leaking XML body and scored it "same". + want, err := applyLikeTheProxy(o.Map.Forward(), canon.body, variant.contentType, nil) if err != nil { r.Err = err return r } - r.Equal = string(want) == string(variant) + r.Equal = string(want) == string(variant.body) r.LinesCanonical = strings.Count(string(want), "\n") - r.LinesVariant = strings.Count(string(variant), "\n") - r.DiffLines = countDiffLines(string(want), string(variant)) + r.LinesVariant = strings.Count(string(variant.body), "\n") + r.BytesCanonical = len(want) + r.BytesVariant = len(variant.body) + r.DiffLines = countDiffLines(string(want), string(variant.body)) // The safety-critical assertion, independent of byte equality: a live site // differs between two fetches for a dozen innocent reasons (nonces, @@ -163,34 +307,413 @@ func compare(ctx context.Context, o Options, path string) Result { // definition exactly the set of origins the proxy claims to rewrite, which // makes this assertion say what it means: anything it still finds in the // variant body is one the proxy should have caught and did not. - r.Leaks = countLeaks(o.Map.Forward(), variant) + // Serialized payloads the browser is served must still parse. This is the + // only assertion here that does not compare the two sides: when the proxy + // and the scorer are wrong in the same way, comparison says nothing, and + // that is exactly how five rounds of silent wp_options destruction went + // unreported by the run PLAN §7 calls the only test that validates against + // reality. + // Against the canonical baseline, so this blames the proxy for what the + // proxy did. Real WordPress databases carry broken serialized rows already — + // from the careless search-replace hostshift exists to avoid — and counting + // the variant alone made every such site RED forever, on bytes the proxy had + // passed through untouched. + r.BrokenSerialized = rewrite.BrokenSerialized(variant.body) - + rewrite.BrokenSerialized(canon.body) + if r.BrokenSerialized < 0 { + r.BrokenSerialized = 0 + } + + // And the spellings the walk cannot read at all, which BrokenSerialized is + // structurally unable to report: a value it cannot read does not parse on + // the canonical page either, so the subtraction above cancels it to zero. + // + // This asks the other question — did the rewrite change bytes inside + // something serialized-shaped that no spelling accounted for — which is + // host-dependent, so it is zero on the canonical side by construction and + // survives the same subtraction. + // Through the same pipeline countLeaks uses, not the bare byte matcher. + // Asking Matcher.Rewrite alone is the mistake countLeaks' own comment + // records — obfuscated separators, folded hosts, CSS escapes and character + // references are invisible to it by construction, so a host spelled any of + // those ways was rewritten by the proxy and reported as untouched here. + // + // No content-type guard of its own. An attachment and a Tier 2 body are ones + // the proxy deliberately does not rewrite, so applyLikeTheProxy returns them + // unchanged and the "did the rewrite touch this" test below answers no — + // which is the same answer a guard would give, from the property that makes + // it true rather than from a second list that could drift from the first. + { + if rewrite.UnreadSerialized(canon.body, func(b []byte) []byte { + out, err := applyLikeTheProxy(o.Map.Forward(), b, canon.contentType, nil) + if err != nil { + return b + } + return out + }) { + r.UnreadRewrites = 1 + } + } + + r.ContentType = variant.contentType + r.Leaks, r.Tier2 = countLeaks(o.Map.Forward(), variant) + // And a check that shares no code with the engine. + // + // countLeaks scores a served body by re-running the pipeline on it, so an + // origin the engine *declines* is declined again and the page is green — the + // report confirms itself with the thing it is checking. Round 74 measured a + // live instance: two `https://` in a served site-health page, + // LEAKS 0, "no canonical origin reached the browser". + // + // A literal scan cannot have that property. Anchored on `//host` rather than + // the bare host, because the bare form has a false positive that matters — + // network/sites.php prints a site's domain as a row's link *text*, which is + // not dereferenceable and is correctly left alone. Measured across 55 pages of + // a real WordPress: with the anchor, one hit, and it was the true positive. + // + // Its own counter rather than folded into Leaks, because it will also see + // deliberate declines — an origin PLAN says to leave alone reads the same to a + // byte scan as one that got away. + r.Literal = literalOrigins(o.Map, variant.body) + // Base64 here too, and for the same reason it is below: countLeaks runs the + // rewrite pipeline, and the pipeline has no base64 view. That reasoning is + // direction-free — it is a property of the pipeline, not of which map is + // pointed at it — and round 67 applied it to the write-back column alone. So + // the mirror of the fixture it added, a widget instance carrying *production's* + // hostname served through the proxy, was still scored GREEN. The widgets + // screen and the Customizer decode `instance.encoded` in JavaScript and render + // it, so that is a live production URL in an authenticated browser. + if n, _ := rewrite.HiddenInBase64(variant.body, func(b []byte) []byte { + out, _ := o.Map.Forward().Rewrite(b, rewrite.SurfaceRequestBody, false) + return out + }); n > 0 { + r.Leaks += n + } + // And the other direction, which this could not see at all until round 66. + // + // Leaks are canonical origins in the *variant* response — test 28. The §4.3 + // failure is its mirror: a *variant* origin in the *canonical* response, + // which means a save through the worktree wrote the worktree's hostname into + // production's database and the canonical site is now serving it to the + // public. That cannot show up as a byte difference either, because the same + // variant string then appears identically on both sides and the row reads + // `same`. So a real §4.3 write-back was reported GREEN: 16 pages, 16 + // byte-identical, 0 leaks. + // + // On a healthy site this is zero by construction — nothing in production's + // database names a worktree — so it costs a scan and answers the question the + // whole exercise exists for. + r.WriteBacks, _ = countLeaks(o.Map.Reverse(), canon) + // And base64, which countLeaks cannot see: it runs the rewrite pipeline, and + // the pipeline has no base64 view — deliberately, because a widget instance + // is validated with `wp_hash()` over exactly those bytes and rewriting it + // makes the app discard the save. So the very §4.3 write this column was + // added for — a Customizer widget carrying the worktree's hostname into + // production's `wp_options` — was still reported GREEN by it. Reported, not + // rewritten, on the same terms as the proxy's WARN. + if n, _ := rewrite.HiddenInBase64(canon.body, func(b []byte) []byte { + out, _ := o.Map.Reverse().Rewrite(b, rewrite.SurfaceRequestBody, false) + return out + }); n > 0 { + r.WriteBacks += n + } return r } +// literalOrigins counts `//host` occurrences of each canonical host in a body, +// with no matcher, no view and no surface — deliberately the dumbest check that +// can exist, so that it cannot inherit an engine mistake. +func literalOrigins(m *origin.Map, body []byte) int { + n := 0 + for _, s := range m.Sites { + needle := []byte("//" + s.Canonical.HostPort()) + n += bytes.Count(bytes.ToLower(body), bytes.ToLower(needle)) + } + return n +} + // countLeaks reports how many canonical origins the matcher still finds in a // body that has already been through the proxy. -func countLeaks(m *origin.Matcher, body []byte) int { - _, events := m.Rewrite(body, "leak-check", true) - n := 0 - for _, e := range events { - if e.Action == origin.ActionRewrote { - n++ +// countLeaks asks the whole engine, not the byte matcher alone. +// +// It used to run `m.Rewrite` and justify that with "the matcher is by definition +// exactly the set of origins the proxy claims to rewrite". That stopped being +// true the moment urlobf.go existed: the proxy also runs the URL-parser view, +// the IDNA fold, the CSS view and the reference views, and this ran none of +// them. So every leak class found since — obfuscated separators, folded hosts, +// CSS escapes, character references — was invisible by construction to the one +// test §7 calls the only one that validates against reality, and it printed +// GREEN on a page whose `` a real browser resolved to production. +// +// Pushing the served bytes back through the same pipeline the proxy runs answers +// the actual question: anything it still finds to rewrite is an origin that +// should already have been rewritten and was not. +// The second return is the Tier 2 count: origins in a body the proxy is +// *documented* not to rewrite. PLAN's fast-path section excludes `text/css` and +// the JavaScript types "per Tier 2, and added only if the corpus diff shows a +// leak" — so finding one there is this tool doing its job, and reporting it as +// a proxy defect would be reporting the wrong thing. An attachment is different +// again: §5 skips it by design, whatever it contains, so it is not counted at +// all. Scoring every body through the HTML pipeline made a PDF or a WooCommerce +// download link — which the `` crawler reaches routinely — read as +// CANONICAL ORIGIN REACHED THE BROWSER. +// redirectsToItself reports whether loc — the *rewritten* Location — is the URL +// the browser asked for. That is the proxy's self-redirect test, asked from the +// outside. +// +// Against a *variant origin from the map*, not the fetch base. Those differ: +// `--variant-base` and `--resolve` exist so the crawl can be pointed somewhere +// else, and the URL the browser would have used is the one the map names. +// +// Host comparison is case-insensitive and ignores the scheme, like the proxy's: +// a router that terminates TLS turns an https request into an http one +// upstream, and the guard has to recognise its own redirect through that. +func redirectsToItself(loc string, o Options, path string) bool { + u, err := url.Parse(loc) + if err != nil || u.Host == "" { + return false + } + want, err := url.Parse("https://x" + path) + if err != nil { + return false + } + if u.EscapedPath() != want.EscapedPath() || u.RawQuery != want.RawQuery { + return false + } + // *The* variant being crawled, not any variant in the map. + // + // Accepting any of them exempted a redirect from one site in a multisite + // map to another — `www.b.fi` 301ing every path to `www.a.fi` is an + // ordinary consolidation redirect, and the browser follows it to production. + // The proxy's guard is `sameURL(rewritten, st.url)` against the single URL + // the browser asked for; there is only ever one. + // + // HostPort, not Host: an Origin keeps its port in a separate field, and the + // map is origin→origin — scheme, host *and* port. Comparing the host alone + // meant a variant on a non-default port could never match its own + // self-redirect, so every page linking an upload went RED on a deployment + // doing exactly what §4.4 prescribes. + crawled := o.Variant + for _, site := range o.Map.Sites { + if crawled != nil && strings.EqualFold(crawled.Host, site.Variant.HostPort()) { + return strings.EqualFold(u.Host, site.Variant.HostPort()) } } - return n + // The crawl is pointed somewhere that is not a variant in the map — a + // `--variant-base` override, or a test harness. Fall back to the primary, + // which is what both bases default to. + if len(o.Map.Sites) > 0 { + return strings.EqualFold(u.Host, o.Map.Sites[0].Variant.HostPort()) + } + return false +} + +// ResolveKey normalises a host:port for the --resolve map, so the guardrail that +// decides whether to warn and the dialer that decides where to connect cannot +// disagree about what host they are looking at. +func ResolveKey(hostPort string) string { + h, p, err := net.SplitHostPort(hostPort) + if err != nil { + return hostPort + } + n, err := origin.NormaliseHost(h) + if err != nil { + n = strings.ToLower(h) + } + return net.JoinHostPort(n, p) } -func fetch(ctx context.Context, o Options, base *url.URL, path string) ([]byte, error) { +func countLeaks(m *origin.Matcher, r response) (leaks, tier2 int) { + if r.attachment { + return 0, 0 + } + if isTier2(r.contentType) { + // Scanned with the text arm on purpose. The proxy does nothing to these + // types, so asking "what would the proxy have done" answers "nothing" — + // and the whole point of the Tier 2 count is to find the origins it is + // leaving behind, which is PLAN's stated trigger for adding them. + return 0, originsIn(m, r.body, "text/plain") + } + return originsIn(m, r.body, r.contentType), 0 +} + +// isTier2 reports whether the proxy deliberately leaves this type alone. +func isTier2(ct string) bool { + mt, _, err := mime.ParseMediaType(ct) + if err != nil { + mt = strings.ToLower(strings.TrimSpace(strings.Split(ct, ";")[0])) + } + switch mt { + case "text/css", "application/javascript", "text/javascript", + "application/x-javascript", "application/ecmascript", "text/ecmascript": + return true + } + return false +} + +// originsIn asks the whole engine, not the byte matcher alone. +// +// It used to run `m.Rewrite` and justify that with "the matcher is by definition +// exactly the set of origins the proxy claims to rewrite". That stopped being +// true the moment urlobf.go existed: the proxy also runs the URL-parser view, +// the IDNA fold, the CSS view and the reference views, and this ran none of +// them. So every leak class found since — obfuscated separators, folded hosts, +// CSS escapes, character references — was invisible by construction to the one +// test §7 calls the only one that validates against reality, and it printed +// GREEN on a page whose `` a real browser resolved to production. +func originsIn(m *origin.Matcher, body []byte, ct string) int { + st := rewrite.NewStats(false) + out, err := applyLikeTheProxy(m, body, ct, st) + if err != nil { + return 0 + } + if n := st.Total(); n > 0 { + return n + } + // A pass that splices without recording — and one that changes bytes has + // found an origin whatever it counted. + if string(out) != string(body) { + return 1 + } + return 0 +} + +// applyLikeTheProxy runs the arm the proxy would run for this content type. +// +// This ran NewResponseBody — the HTML pipeline, XMLEntities off — on every body, +// while proxy.go dispatches every `*xml` media type to HostLeaksXMLCounted, +// which applies the reference and CSS views over the whole buffer. The HTML +// pipeline applies the reference view only where an *HTML* parser decodes one: +// attributes and foreign content. Element content in an ordinary XML element is +// the gap — and that is where every sitemap `` and every RSS `` +// lives. So the one test PLAN §7 calls "the only test that validates against +// reality" scored an unrewritten feed GREEN, and the byte-equality half +// positively rewarded the leak, because `want` was computed the same blind way. +func applyLikeTheProxy(m *origin.Matcher, body []byte, ct string, st *rewrite.Stats) ([]byte, error) { + mt, _, err := mime.ParseMediaType(ct) + if err != nil { + mt = strings.ToLower(strings.TrimSpace(strings.Split(ct, ";")[0])) + } + mt = strings.ToLower(mt) + switch { + case mt == "text/html" || mt == "application/xhtml+xml": + return io.ReadAll(rewrite.NewResponseBody(strings.NewReader(string(body)), m, nil, + rewrite.Options{Stats: st, XMLEntities: mt == "application/xhtml+xml"})) + + // Ahead of the XML arm, because `application/ld+json` ends in neither and + // the proxy tests JSON first. + case mt == "application/json", mt == "text/json", strings.HasSuffix(mt, "+json"): + out := rewrite.RewriteJSON(body, m, st, nil, false) + // Inside the repair: the sweep is a raw byte matcher, so a host it + // rewrites inside a serialized string leaves the length stale. On + // RewriteJSON's decline path — a duplicate member is legal JSON and + // is rejected — the sweep is the only pass that touches the body, so + // it corrupted the blob while logging a line that reads like a save. + return rewrite.RepairSerialized(out, func(b []byte) []byte { + return rewrite.SweepBytes(b, m, st, nil) + }), nil + + // The enumerated set plus `+xml`, exactly as rewritableText has it — not + // `HasSuffix(mt, "xml")`, which also swallows text/xml-external-parsed-entity + // and application/vnd.foo.xml, and not `HasPrefix(mt, "text/")`, which + // swallows text/markdown. Either one made the scorer rewrite a body the + // proxy passes through, and the run went RED on a healthy deployment. + case isTextArm(mt): + // The proxy's `{`/`[` sniff first. wp-admin/async-upload.php sets + // text/plain before wp_send_json can set application/json, so the body + // that reports every media upload arrives on this arm as JSON — and + // wp_json_encode writes its origins with \uXXXX escapes, which only + // RewriteJSON decodes. Without the sniff the scorer served that body + // back unrewritten and called the page clean. + if t := bytes.TrimLeft(body, " \t\r\n"); len(t) > 0 && (t[0] == '{' || t[0] == '[') { + out := rewrite.RewriteJSON(body, m, st, nil, false) + // Inside the repair: the sweep is a raw byte matcher, so a host it + // rewrites inside a serialized string leaves the length stale. On + // RewriteJSON's decline path — a duplicate member is legal JSON and + // is rejected — the sweep is the only pass that touches the body, so + // it corrupted the blob while logging a line that reads like a save. + return rewrite.RepairSerialized(out, func(b []byte) []byte { + return rewrite.SweepBytes(b, m, st, nil) + }), nil + } + // All three passes the proxy runs, in order. Running only the middle one + // scored a plain, unencoded, dereferenceable origin as clean: stripForURL + // *deletes* tab, LF and CR — right for a single URL value, wrong for a + // whole document, where those bytes are token separators. Removing the + // newline welds the previous word onto `https:`, tokenBoundary is then + // false, and no candidate is emitted. The byte matcher and the sweep, + // which the proxy runs and this did not, see the raw bytes. + // Wrapped in RepairSerialized, exactly as proxy.go's text arm is. Both + // were edited in the same commit and only one got the wrapper, so the + // scorer disagreed with the proxy on any body carrying an `s:N:"…"` — + // which sends the run spuriously RED on a real page. + var ev []origin.Event + // The same surface the proxy picks, by the same question: an XML body's + // `` + h := newHarness(t, acmecorpMap(t), func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "image/svg+xml") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(body)) + }) + _, got := h.get(t, variantHost, "/icon.svg") + if !strings.Contains(string(got), variantHost) { + t.Errorf("an SVG `}, + } { + t.Run(c.name, func(t *testing.T) { + h := newHarness(t, acmecorpMap(t), func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/html; charset=utf-8") + w.WriteHeader(200) + _, _ = w.Write([]byte("" + c.body + "")) + }) + _, got := h.do(t, "GET", variantHost, "/", "", nil) + if bytes.Contains(got, []byte("www.acmecorp.fi")) { + t.Errorf("a dereferenceable production origin reached the browser:\n%s", got) + } + }) + } +} diff --git a/internal/proxy/audit_r66_test.go b/internal/proxy/audit_r66_test.go new file mode 100644 index 0000000..008150a --- /dev/null +++ b/internal/proxy/audit_r66_test.go @@ -0,0 +1,62 @@ +package proxy + +import ( + "bytes" + "io" + "net/http" + "testing" +) + +// Round 66: one media type, read two ways in the same function. +// +// A media type is case-insensitive (RFC 9110 §8.3.1), and every other place in +// this project reads it that way: bodyKind lowercases (proxy.go:981), and so +// does the filter's dispatch (cmd/hostshift/main.go:255). The one site that does +// not is the choice of repair inside rewriteRequestBody: +// +// if mediaType(r.Header.Get("Content-Type")) == "application/x-www-form-urlencoded" { +// repair = rewrite.RepairSerializedFields +// } +// +// So a body whose Content-Type carries any upper-case letter is still classified +// bodyFlat by bodyKind — it *is* rewritten — but is repaired with the +// non-splitting RepairSerialized. That drops two things at once, and both have +// their own §4.3 finding in this project's history: +// +// - the field split, so one option holding `a:hover{color:red}` leaves every +// other option in the same `options.php` POST with a stale length; +// - peelFormField, so the double-encoded spelling a browser posts back — +// `https%253A%252F%252F` — is reachable by no spelling in the +// table, and the *variant* hostname is written into the shared database. +// +// The second is round 63's finding, read back out of a real database, and this +// re-opens it for any sender that spells the header differently. `check` and +// `diff` cannot see it: neither makes a request-direction assertion. +func TestAFormBodyIsSplitWhateverTheHeadersCase(t *testing.T) { + // The Customizer's `customized` field, in the spelling only the peel reaches. + const body = "customized=https%253A%252F%252F" + variantHost + "%252Fa" + + for _, ct := range []string{ + "application/x-www-form-urlencoded", + "application/x-www-form-urlencoded; charset=UTF-8", + "Application/X-WWW-Form-Urlencoded", + "APPLICATION/X-WWW-FORM-URLENCODED", + "application/X-Www-Form-Urlencoded; charset=utf-8", + } { + t.Run(ct, func(t *testing.T) { + h := newHarness(t, acmecorpMap(t), func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + }) + h.do(t, "POST", variantHost, "/wp-admin/admin-ajax.php", ct, []byte(body)) + if h.seen == nil { + t.Fatal("the upstream was never reached") + } + up, _ := io.ReadAll(h.seen.Body) + if bytes.Contains(up, []byte(variantHost)) { + t.Errorf("a variant hostname reached the upstream, so it would be "+ + "written into the shared database (PLAN §4.3):\n sent %s\n up %s", + body, up) + } + }) + } +} diff --git a/internal/proxy/audit_r67_test.go b/internal/proxy/audit_r67_test.go new file mode 100644 index 0000000..f311208 --- /dev/null +++ b/internal/proxy/audit_r67_test.go @@ -0,0 +1,189 @@ +package proxy + +import ( + "bytes" + "encoding/base64" + "fmt" + "io" + "log/slog" + "mime/multipart" + "net/http" + "net/url" + "strconv" + "strings" + "testing" +) + +// Round 67, on 5cb050f, auditing round 66's own new code: HiddenInBase64. +// +// PLAN §4.3 accepts that a variant hostname inside a base64 blob cannot be +// mapped back — the Customizer validates a widget instance with `wp_hash()` +// over exactly those bytes — and says the failure was never the rewrite but the +// silence: "the save went through, production's database took the worktree's +// hostname, the canonical front page served it to the public, nothing logged a +// line, and `hostshift diff` printed GREEN. `HiddenInBase64` reports it instead, +// on both arms". +// +// It reports it on one arm, for one spelling, and that spelling is not the one +// any client sends. +// +// - proxy.go:849 calls it only from the `default:` (flat) arm of +// rewriteRequestBody's switch. bodyJSON and bodyMultipart never call it — +// so a widget saved through `POST /wp-json/wp/v2/widgets`, or any form with +// a file field beside it, is silent by construction. +// +// - base64.go:32 walks runs of `[A-Za-z0-9+/]` and requires +// `base64.StdEncoding.DecodeString` to succeed over the whole run. In a +// form body the blob is percent-encoded — `+` is `%2B`, `/` is `%2F` and +// the padding is `%3D` — so the run is cut at every escape and each +// fragment is unaligned, decodes to garbage, or fails outright. The escape's +// own hex digits are `[A-Za-z0-9]`, so `%22` before a blob glues `22` onto +// the front of the run and shifts its alignment by two even when the blob +// itself is escape-free. +// +// base64_test.go's fixtures are `"customized=" + base64.StdEncoding.Encode(…)` +// spliced raw into a form body. That is the one spelling that works. A `
` +// POST, `URLSearchParams`, jQuery and `wp.customize` all percent-encode the +// field, and PLAN §4.3 has a whole paragraph on there being no single +// urlencoded encoder — the alphabet lesson was learned for the peel and not for +// the detector beside it. +// +// The other half of the claim fails with it: `WriteBacks` (diff.go:369) is +// `countLeaks` over the canonical response, and countLeaks runs the rewrite +// pipeline, which has no base64 view. So "it is what turns that Customizer run +// red" is false for the case it names — `hostshift diff` still prints GREEN. + +// r67Blob is a widget instance carrying a variant hostname, in the shape +// `encoded_serialized_instance` really has: base64 of a PHP-serialized array. +func r67Blob() string { + link := `the promo` + s := `a:1:{s:7:"content";s:` + strconv.Itoa(len(link)) + `:"` + link + `";}` + return base64.StdEncoding.EncodeToString([]byte(s)) +} + +// r67Post sends one write through the proxy and returns what the upstream saw +// and everything the proxy logged while doing it. +func r67Post(t *testing.T, ct string, body []byte) (up []byte, logged string) { + t.Helper() + var lb bytes.Buffer + h := newHarness(t, acmecorpMap(t), func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + }, func(p *Proxy) { + p.Log = slog.New(slog.NewTextHandler(&lb, &slog.HandlerOptions{Level: slog.LevelDebug})) + }) + h.do(t, "POST", variantHost, "/wp-admin/admin-ajax.php", ct, body) + if h.seen == nil { + t.Fatal("the upstream was never reached") + } + up, _ = io.ReadAll(h.seen.Body) + return up, lb.String() +} + +func TestABase64WriteBackIsReportedInEverySpelling(t *testing.T) { + blob := r67Blob() + + multi := func() (string, []byte) { + var b bytes.Buffer + w := multipart.NewWriter(&b) + fw, err := w.CreateFormField("customized") + if err != nil { + t.Fatal(err) + } + fmt.Fprint(fw, blob) + if err := w.Close(); err != nil { + t.Fatal(err) + } + return w.FormDataContentType(), b.Bytes() + } + mpCT, mpBody := multi() + + for _, c := range []struct { + name, ct string + body []byte + }{ + // The control, and the only spelling base64_test.go exercises. + {"a flat body with the blob spliced in raw", + "application/x-www-form-urlencoded", + []byte("action=customize_save&customized=" + blob)}, + + // What every form encoder actually puts on the wire. + {"the same blob percent-encoded, as a form encoder sends it", + "application/x-www-form-urlencoded", + []byte("action=customize_save&customized=" + url.QueryEscape(blob))}, + + // customize_save's real shape: every setting in one `customized` field, + // JSON inside it, the blob inside a JSON string. + {"the customize_save wire shape", + "application/x-www-form-urlencoded", + []byte("action=customize_save&nonce=abc&customized=" + url.QueryEscape( + `{"widget_text[2]":{"encoded_serialized_instance":"`+blob+ + `","instance_hash_key":"d41d8cd98f00b204e9800998ecf8427e"}}`))}, + + // The REST route the block editor's widget save uses. The JSON arm of + // rewriteRequestBody never calls HiddenInBase64 at all. + {"a JSON request body", + "application/json", + []byte(`{"instance":{"encoded":"` + blob + `"}}`)}, + + // Any form with a file field beside the settings. The multipart arm + // never calls it either. + {"a multipart request body", mpCT, mpBody}, + } { + t.Run(c.name, func(t *testing.T) { + up, logged := r67Post(t, c.ct, c.body) + if !bytes.Equal(up, c.body) { + t.Fatalf("fixture broken: the proxy changed this body, so the blob is "+ + "not what reaches the database\n sent %s\n up %s", c.body, up) + } + if !strings.Contains(logged, "base64") { + t.Errorf("a variant hostname reached the shared database inside base64 "+ + "and nothing said so.\n"+ + " PLAN §4.3: \"the failure was the silence … HiddenInBase64 reports it "+ + "instead, on both arms\".\n"+ + " content-type: %s\n body: %.160s\n logged: %q", + c.ct, c.body, logged) + } + }) + } +} + +// The request-body gate is POST/PUT/PATCH (proxy.go:746-750), while the query +// string and the path beside it are mapped back for every method. A DELETE that +// carries a body is therefore the one write shape whose body goes upstream +// untouched — and WP_REST_Request::parse_body_params() reads a form or JSON +// body whatever the method, so those params are real. +func TestADeleteBodyIsMappedBack(t *testing.T) { + const body = "content=%3Ca+href%3D%22https%3A%2F%2F" + variantHost + "%2Fpromo%2F%22%3Ex%3C%2Fa%3E" + + for _, method := range []string{"POST", "PUT", "PATCH", "DELETE"} { + t.Run(method, func(t *testing.T) { + h := newHarness(t, acmecorpMap(t), func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + }) + req, err := http.NewRequest(method, h.front.URL+"/wp-json/wp/v2/posts/1", + bytes.NewReader([]byte(body))) + if err != nil { + t.Fatal(err) + } + req.Host = variantHost + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + cl := &http.Client{CheckRedirect: func(*http.Request, []*http.Request) error { + return http.ErrUseLastResponse + }} + res, err := cl.Do(req) + if err != nil { + t.Fatal(err) + } + io.Copy(io.Discard, res.Body) + res.Body.Close() + if h.seen == nil { + t.Fatal("the upstream was never reached") + } + up, _ := io.ReadAll(h.seen.Body) + if bytes.Contains(up, []byte(variantHost)) { + t.Errorf("a variant hostname reached the shared database (PLAN §4.3)\n"+ + " sent %s\n up %s", body, up) + } + }) + } +} diff --git a/internal/proxy/audit_r68_base64_test.go b/internal/proxy/audit_r68_base64_test.go new file mode 100644 index 0000000..0768b9f --- /dev/null +++ b/internal/proxy/audit_r68_base64_test.go @@ -0,0 +1,87 @@ +package proxy + +import ( + "bytes" + "encoding/base64" + "io" + "log/slog" + "net/http" + "strconv" + "strings" + "testing" + + "github.com/generoi/hostshift/internal/rewrite" +) + +// The response direction is silent about the very thing the request direction +// warns on. +// +// `rewrite.HiddenInBase64` is called in exactly two places in the tree — +// internal/proxy/proxy.go:859 and internal/corpus/diff.go:377 — and both ask +// the *reverse* map, i.e. both look for a variant hostname. Nothing asks the +// forward map, so a *canonical* hostname carried inside base64 goes to the +// browser with no WARN, no counter and no `--explain` event. +// +// The blob is not inert. `WP_REST_Widgets_Controller::prepare_item_for_response` +// returns a legacy widget's settings as `instance.encoded`, base64 of the +// serialized instance; the widgets screen and the Customizer decode it in +// JavaScript and render the widget, so an `` or an `` inside it +// becomes a live production URL in the developer's authenticated browser. PLAN +// §4.3 accepts that hostshift must not rewrite those bytes — `wp_hash()` covers +// exactly them — and says the property that matters is that something reports +// it: "It does not fix the corruption; it ends the silence". +func TestR68ACanonicalOriginInsideBase64InAResponseIsReported(t *testing.T) { + link := `the promo` + blob := base64.StdEncoding.EncodeToString([]byte( + `a:1:{s:7:"content";s:` + strconv.Itoa(len(link)) + `:"` + link + `";}`)) + page := `` + + var lb bytes.Buffer + st := rewrite.NewStats(true) + h := newHarness(t, acmecorpMap(t), func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/html; charset=utf-8") + _, _ = io.WriteString(w, page) + }, func(p *Proxy) { + p.Stats = st + p.Log = slog.New(slog.NewTextHandler(&lb, &slog.HandlerOptions{Level: slog.LevelDebug})) + }) + _, got := h.get(t, variantHost, "/wp-admin/widgets.php") + + // The bytes go out unchanged, which is correct and is exactly why something + // has to say so. + if !strings.Contains(string(got), blob) { + t.Fatalf("the blob was rewritten, which would break wp_hash(): %q", got) + } + if n, _ := rewrite.HiddenInBase64([]byte(got), func(b []byte) []byte { + out, _ := h.proxy.Map.Forward().Rewrite(b, rewrite.SurfaceText, false) + return out + }); n == 0 { + t.Fatal("fixture is wrong: the served page carries no canonical origin in base64") + } + if !strings.Contains(lb.String(), "base64") { + t.Errorf("a canonical origin was served to the browser inside base64 and the "+ + "proxy logged nothing — the request direction WARNs on the mirror image "+ + "(proxy.go:859). Logged:\n%s", lb.String()) + } +} + +// The mirror, which does warn. Kept beside it so the asymmetry is the assertion +// rather than a claim about it. +func TestR68TheRequestDirectionStillWarns(t *testing.T) { + link := `the promo` + blob := base64.StdEncoding.EncodeToString([]byte( + `a:1:{s:7:"content";s:` + strconv.Itoa(len(link)) + `:"` + link + `";}`)) + + var lb bytes.Buffer + h := newHarness(t, acmecorpMap(t), func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + }, func(p *Proxy) { + p.Log = slog.New(slog.NewTextHandler(&lb, &slog.HandlerOptions{Level: slog.LevelDebug})) + }) + h.do(t, "POST", variantHost, "/wp-admin/admin-ajax.php", + "application/x-www-form-urlencoded", []byte("action=customize_save&customized="+blob)) + if !strings.Contains(lb.String(), "base64") { + t.Fatalf("control case regressed; the request direction logged:\n%s", lb.String()) + } +} diff --git a/internal/proxy/audit_r68_test.go b/internal/proxy/audit_r68_test.go new file mode 100644 index 0000000..9ad86a9 --- /dev/null +++ b/internal/proxy/audit_r68_test.go @@ -0,0 +1,125 @@ +package proxy + +import ( + "bytes" + "io" + "net/http" + "strings" + "testing" +) + +// A boundary Go's media-type parser refuses and PHP's accepts. +// +// The two readers do not share an alphabet. RFC 2046 §5.1.1 defines the +// boundary's own alphabet as `bchars`: +// +// DIGIT / ALPHA / "'" / "(" / ")" / "+" / "_" / "," / "-" / "." / +// "/" / ":" / "=" / "?" / " " +// +// Seven of those — "(", ")", ",", "/", ":", "=", " " — are RFC 2045 tspecials +// rather than token characters, so a boundary containing one has to be quoted +// to survive a strict parameter parse. Producers routinely do not quote it: +// `----=_Part_0_12345.67890` is the JavaMail/Apache Commons shape, and any +// base64-derived boundary carries `=` padding. `mime.ParseMediaType` then +// returns ErrInvalidMediaParameter and a nil params map, and rewriteMultipart's +// first three lines return the body *unchanged*: +// +// internal/proxy/multipart.go:21-27 +// _, params, err := mime.ParseMediaType(ct) +// if err != nil { +// return body +// } +// +// PHP's own parser does not tokenise at all. `php_rfc1867.c` locates the +// boundary with `strstr(content_type, "boundary")`, steps to the next `=`, and +// takes everything up to the next `,` or `;`, so it reads every boundary below +// exactly and parses the parts. The form field is therefore stored — with the +// worktree's hostname in it. That is PLAN §4.3: the shared database is no +// longer byte-identical to production, and the write is unrecoverable. +// +// The boundary alphabet is only the cheapest way in. The defect is the arm: any +// Content-Type `mime.ParseMediaType` rejects for any reason — a duplicate +// parameter, a stray token after the boundary, an unclosed quoted string — takes +// the same silent `return body`, and so does a body whose delimiters this +// function cannot find. +// +// The failure is silent in every channel hostshift has: +// +// - no log line — the `err` is discarded, not reported; +// - no straggler sweep — `bodyMultipart` is the one arm of rewriteRequestBody +// with no `HostLeaksBack` behind it (internal/proxy/proxy.go:786-790); +// - no counter — nothing calls `Stats.Record`, so `--json` shows zero +// candidates rather than a skip; +// - `hostshift diff` never looks at a request at all. +// +// Contrast the sibling arms, which all fail loudly: `bodyJSON` logs a WARN and +// falls back to SweepBytes when jsontext rejects the document, and the +// over-cap path logs and records origin.ReasonSizeCap. Multipart is the arm +// that says nothing. +func TestR68AMultipartBoundaryGoRefusesLetsAVariantHostReachTheDatabase(t *testing.T) { + // The value a page-builder field holds after the response direction served + // it: the variant hostname, which must be mapped back before it is stored. + field := `
` + + for _, c := range []struct{ name, boundary string }{ + // Every one of these is bchars per RFC 2046 §5.1.1, and every one of + // them is read whole by php_rfc1867.c — none contains a "," or ";", + // which are the only two bytes PHP stops at. + {"javamail", "----=_Part_0_12345.67890"}, + {"base64 padded", "Ck6Lz+Kk1Q=="}, + {"equals run", "===============1234567890=="}, + {"colon", "a:b"}, + {"slash", "a/b"}, + {"parens", "a(b)c"}, + {"space", "b1 b2"}, + } { + t.Run(c.name, func(t *testing.T) { + ct := "multipart/form-data; boundary=" + c.boundary + body := "--" + c.boundary + "\r\n" + + "Content-Disposition: form-data; name=\"content\"\r\n\r\n" + + field + "\r\n" + + "--" + c.boundary + "--\r\n" + + h := newHarness(t, acmecorpMap(t), func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + }) + res, rb := h.do(t, "POST", variantHost, "/wp-admin/admin-ajax.php", ct, []byte(body)) + if h.seen == nil { + t.Fatalf("the upstream was never reached: status %d, body %q", res.StatusCode, rb) + } + up, err := io.ReadAll(h.seen.Body) + if err != nil { + t.Fatal(err) + } + if bytes.Contains(up, []byte(variantHost)) { + t.Errorf("a variant hostname reached the upstream in a multipart body whose "+ + "boundary is RFC 2046-conforming but not an RFC 2045 token (PLAN §4.3)\n"+ + " content-type %q\n sent %q\n upstream %q", + ct, body, up) + } + if strings.Contains(string(up), canonical) { + return // mapped back, which is the correct outcome + } + }) + } +} + +// The same body with a token-safe boundary is mapped back, which is what makes +// the case above a defect in the boundary reader rather than in the rewriter. +func TestR68TheSameBodyWithATokenBoundaryIsMappedBack(t *testing.T) { + field := `
` + ct := "multipart/form-data; boundary=BXX" + body := "--BXX\r\nContent-Disposition: form-data; name=\"content\"\r\n\r\n" + + field + "\r\n--BXX--\r\n" + + h := newHarness(t, acmecorpMap(t), func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + }) + if _, _ = h.do(t, "POST", variantHost, "/wp-admin/admin-ajax.php", ct, []byte(body)); h.seen == nil { + t.Fatal("upstream never reached") + } + up, _ := io.ReadAll(h.seen.Body) + if bytes.Contains(up, []byte(variantHost)) { + t.Fatalf("control case regressed: %q", up) + } +} diff --git a/internal/proxy/audit_r69_test.go b/internal/proxy/audit_r69_test.go new file mode 100644 index 0000000..6d9b7d4 --- /dev/null +++ b/internal/proxy/audit_r69_test.go @@ -0,0 +1,98 @@ +package proxy + +import ( + "bytes" + "encoding/base64" + "log/slog" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/generoi/hostshift/internal/origin" + "github.com/generoi/hostshift/internal/rewrite" +) + +// TestR69MultipartBase64PartIsSilent +// +// Round 68 made a base64 run whitespace-tolerant so that a +// `Content-Transfer-Encoding: base64` part, wrapped at RFC 2045's 76 columns, +// is one run rather than one run per line. The run is greedy in both directions +// and stripB64Space concatenates whatever it swallowed, so the literal word +// `base64` on the header line — six characters, all in the alphabet, separated +// from the blob by nothing but CRLF — is prepended to the blob. Six is not a +// multiple of four, the decode fails, and the detector says nothing. +// +// The refusal is the whole remedy for a variant hostname inside base64 (§4.3), +// so a silent detector is the harm. Measured against a running proxy: the same +// blob with no CTE header, or with `7bit`/`8bit` (four characters), is reported; +// with `base64`, `binary` or `quoted-printable` it is not. +func TestR69MultipartBase64PartIsSilent(t *testing.T) { + blob := base64.StdEncoding.EncodeToString( + []byte(`a:1:{s:5:"title";s:31:"https://wt-a--r69w.ddev.site/x/";}`)) + const bd = "----r69b" + const cd = `Content-Disposition: form-data; name="instance"` + + body := func(cte string) []byte { + h := cd + if cte != "" { + h += "\r\nContent-Transfer-Encoding: " + cte + } + return []byte("--" + bd + "\r\n" + h + "\r\n\r\n" + blob + "\r\n--" + bd + "--\r\n") + } + + for _, tc := range []struct{ cte, why string }{ + {"", "no Content-Transfer-Encoding"}, + {"7bit", "a four-character token keeps the alignment"}, + {"8bit", "a four-character token keeps the alignment"}, + {"base64", "the token round 68's whitespace tolerance exists for"}, + {"binary", "six characters"}, + {"quoted-printable", "the run resumes at `printable`, nine characters"}, + } { + var buf bytes.Buffer + p := r69proxy(t, &buf) + front := httptest.NewServer(p.Handler()) + + req, _ := http.NewRequest("POST", front.URL+"/probe", bytes.NewReader(body(tc.cte))) + req.Host = "wt-a--r69w.ddev.site" + req.Header.Set("Content-Type", "multipart/form-data; boundary="+bd) + res, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + res.Body.Close() + front.Close() + + if !strings.Contains(buf.String(), "inside base64") { + t.Errorf("Content-Transfer-Encoding: %q (%s): a variant hostname went "+ + "upstream inside base64 with nothing logged", tc.cte, tc.why) + } + } +} + +func r69proxy(t *testing.T, out *bytes.Buffer) *Proxy { + t.Helper() + up := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("ok")) + })) + t.Cleanup(up.Close) + + c, err := origin.Parse("https://www.r69a.example") + if err != nil { + t.Fatal(err) + } + v, err := origin.Parse("https://wt-a--r69w.ddev.site") + if err != nil { + t.Fatal(err) + } + m, err := origin.NewMap([]origin.Site{{Name: "s", Canonical: c, Variant: v}}) + if err != nil { + t.Fatal(err) + } + u, _ := url.Parse(up.URL) + return &Proxy{ + Upstream: u, Map: m, Stats: rewrite.NewStats(false), + Log: slog.New(slog.NewTextHandler(out, nil)), + } +} diff --git a/internal/proxy/audit_r74_test.go b/internal/proxy/audit_r74_test.go new file mode 100644 index 0000000..66e29ef --- /dev/null +++ b/internal/proxy/audit_r74_test.go @@ -0,0 +1,229 @@ +package proxy + +import ( + "bytes" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "net/http/httptrace" + "net/textproto" + "net/url" + "strings" + "testing" + + "github.com/generoi/hostshift/internal/origin" + "github.com/generoi/hostshift/internal/rewrite" +) + +// TestR74EarlyHintsBypassModifyResponse measures what reaches the browser in a +// 1xx informational response. +// +// httputil.ReverseProxy forwards 1xx responses to the client through an +// httptrace.Got1xxResponse hook that copies the header verbatim and calls +// WriteHeader. ModifyResponse is never consulted for them, so +// rewriteResponseHeaders — PLAN §5.2's whole guarantee for the header surface — +// does not run. A `103 Early Hints` carrying `Link: ; +// rel=preload` is a production URL the browser fetches before the final +// response arrives; the identical header on the final response is rewritten. +func TestR74EarlyHintsBypassModifyResponse(t *testing.T) { + // OPEN FINDING, skipped rather than deleted. See PLAN §5.2. + // + // `httputil.ReverseProxy` forwards an informational response through its own + // `httptrace.Got1xxResponse` hook, which copies the header verbatim and calls + // WriteHeader; `ModifyResponse` is never consulted, so `originHeaders` — which + // PLAN §5.2 calls "the whole guarantee for the header surface" — never runs on + // a `103 Early Hints`. A browser preloads from a 103, so a `Link: rel=preload` + // naming production is a fetch issued before the page arrives. + // + // Adding a second `Got1xxResponse` via the request context does not fix it: + // httputil installs its own, and the composed hooks race the WriteHeader, so + // the mutation lands after the header is already on the wire. Closing this + // needs the header pass to run on the *upstream* side — a RoundTripper wrapper + // or a 1xx-aware transport — which is a structural change, not a hook. + // + // Ranked SMALL by the audit that found it because nothing in a stock + // WordPress/nginx stack emits 103; the mechanism is real, the realism is what + // limits it. + t.Skip("open: needs a 1xx-aware transport, not a trace hook; see PLAN §5.2") + const canon = "https://www.r74a.example/wp-content/style.css" + + up := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Link", "<"+canon+">; rel=preload; as=style") + w.WriteHeader(http.StatusEarlyHints) + w.Header().Del("Link") + // The same header on the final response, which ModifyResponse does see. + w.Header().Set("Link", "<"+canon+">; rel=preload; as=style") + w.Header().Set("Content-Type", "text/html") + w.WriteHeader(200) + w.Write([]byte("

ok

")) + })) + defer up.Close() + + p := r74proxy(t, up.URL) + front := httptest.NewServer(p.Handler()) + defer front.Close() + + var got1xx []textproto.MIMEHeader + req, _ := http.NewRequest("GET", front.URL+"/", nil) + req.Host = "wt-a--r74w.ddev.site" + req = req.WithContext(httptrace.WithClientTrace(req.Context(), &httptrace.ClientTrace{ + Got1xxResponse: func(code int, h textproto.MIMEHeader) error { + got1xx = append(got1xx, h) + return nil + }, + })) + res, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + defer res.Body.Close() + + // The control: the same header, on the final response, is rewritten. If + // this fails the harness is not measuring the proxy at all. + if strings.Contains(res.Header.Get("Link"), "r74a.example") { + t.Fatalf("harness: the final Link was not rewritten either: %q", res.Header.Get("Link")) + } + if len(got1xx) == 0 { + t.Skip("no 1xx response was forwarded to the client at all") + } + for _, h := range got1xx { + if strings.Contains(h.Get("Link"), "r74a.example") { + t.Errorf("a canonical origin reached the browser in a 103 Early Hints "+ + "Link header: %q (the same header on the final response came out %q)", + h.Get("Link"), res.Header.Get("Link")) + } + } +} + +// TestR74BareOriginBeforeAControlInAnAttributeIsNotRewritten +// +// Measured first on stock WordPress 7.1 through a running proxy, not +// constructed: `wp-admin/site-health.php?tab=debug` renders the copy-to-clipboard +// report into a *single attribute*, `