# PLAN-sidecar.md
## Where We Are
`select_precompressed_sidecar` opens a precompressed variant with a bare
`std::fs::File::open` and serves it. It performs **no containment check on the opened
descriptor**, unlike every other file this crate serves. Its only guard is a
`debug_assert_eq!` comparing the sidecar's parent directory to the original's, which is
compiled out of release builds and which checks the *constructed path*, not what the
descriptor actually points at.
The consequence is demonstrated, not theorised. With `styles.css.br` a symlink to
`/tmp/outside-secret.txt`:
- `GET /styles.css.br` → `404`. The symlink is refused, because that path goes through
`resolve::open_verified`.
- `GET /styles.css` with `Accept-Encoding: br` → `200`, 24 bytes, the contents of
`/tmp/outside-secret.txt`, labelled `content-encoding: br`.
Same file, two doors, one open. This falsifies the guarantee `THREAT_MODEL.md` states as
`containment-verified-on-fd` — "a symlink cannot escape the root and there is no
check-to-open window". That guarantee has a passing mutation test
(`resolve_rejects_symlink_escaping_root`), which passes only because it exercises
`resolve` and never the sidecar probe. The same shape of gap as the content cache's
`/.env` leak: a second path to bytes that skipped the first path's refusals.
Introduced in 0.9.0 (2026-08-02, commit `739e2de`). Present in **every published version**.
The content-cache path inherits it: a cached hit with no cached variant deliberately falls
through to this disk probe.
Reachability requires a symlink inside the served root. That is not equivalent to "the
attacker already owns the box": extracting an untrusted archive (`tar`/`unzip` preserve
symlinks), a user-upload directory, and a CI artifact drop all produce it without shell
access. The crate defends this case for ordinary files, so an operator has every reason to
assume it is defended here.
Separately, and the reason this plan was opened: the probe costs 31% on a sidecar-served
request because `handle_request` opens and `fstat`s the **original** first, then opens the
sidecar, then discards the original's descriptor. Two `open()`+`fstat` pairs, one used.
nginx's `gzip_static` pays one, which is why its precompressed throughput is
indistinguishable from its plain throughput while ours drops a third (40,330 vs 58,573
req/s on `index.html`).
## Where We Need To Be
1. A precompressed sidecar is served only if containment is proven on **its own** opened
descriptor, by the same function that proves it for every other file. A symlinked
sidecar pointing outside the root causes the original file to be served instead —
identical to the sidecar being absent — never the outside file, in release builds, with
and without the content cache.
2. `containment-verified-on-fd` holds on the sidecar path, with a mutation proving it: the
check removed must make a test fail.
3. A sidecar-served request performs **one** verified open, not two. `index.html` with
sidecars present returns to within noise of the non-probing path (~58,000 req/s from
40,330), measured interleaved.
4. Response bytes, status and the full header set for every non-symlink case are unchanged
— the ETag and `Last-Modified` continue to come from the *sidecar's* metadata.
## Commits
### 1. Verify the sidecar on its own descriptor
- **Does:** route the sidecar open through `resolve`'s fd-based containment check, so a
symlinked sidecar is declined and the original is served instead.
- **Why it is first and alone:** it is a security fix to published code and must be
publishable without waiting on any performance work. It makes the probe marginally
*slower* (one `real_path_of` per successful probe) and that is accepted here; commit 2
more than repays it.
- **Verifies:** a new test builds a root where `styles.css.br` symlinks outside it, and
asserts `GET /styles.css` with `Accept-Encoding: br` returns the original CSS bytes with
no `content-encoding` header — run against a **release** build too, since the existing
guard is a `debug_assert`. A second case covers a symlinked sidecar whose target is
*inside* the root, which must be **served**: measured against the running crate,
`resolve` returns `200` for an in-root symlink, because the rule is "the descriptor
resolves inside the root", not "no symlinks". A fix that refused all symlinked sidecars
would match neither `resolve` nor a build system that links compressed assets from a
shared directory under the root, so both cases are tested to pin the boundary in the
right place. Mutation: delete the containment check and the escaping-sidecar test must
fail.
- **Also refuses anything that is not a regular file**, which is not scope creep but a
consequence of the fix: `open_verified` retries a *directory* with `index.html` appended,
so routing the sidecar through it unmodified would make a directory named
`styles.css.br` resolve to `styles.css.br/index.html` and serve it as a brotli body. A
FIFO sidecar blocks the request for as long as no writer appears — the same hazard
`cache::is_cacheable` already refuses for the same reason. The sidecar open therefore
asserts `metadata.is_file()` and declines otherwise, which is one condition covering both.
- **Both `open_verified` implementations are in scope.** `resolve.rs` carries two
`cfg`-gated variants — fd-based (`real_path_of`) for macOS/iOS/Linux, canonicalize-then-open
elsewhere. The sidecar must be verified on whichever the platform compiles, or the fix
holds only where it was tested.
- **Removes the `debug_assert_eq!` on parent equality.** Once containment is proven on the
descriptor, a compiled-out check on the constructed path is a relic asserting a weaker
property than the one now enforced. Leaving it would suggest the guard it was standing in
for is still needed.
- **Verifies in release, specifically.** The existing guard is a `debug_assert`, so a
debug-only test cannot distinguish "fixed" from "the assertion fired". The new escaping
case runs under `cargo test --release` in `verify-guarantees.sh`, and the guarantees table
records that this is why.
- **Touches:** `src/resolve.rs` (make the verified open reachable for a caller-supplied
path — likely `pub(crate) fn open_verified_path`), `src/server.rs`
(`select_precompressed_sidecar`), `tests/` (new cases), `THREAT_MODEL.md` (two rows —
`sidecar-containment-verified-on-fd` and `sidecar-refuses-irregular-files` — plus a note
that `containment-verified-on-fd` was false on this path from 0.9.0),
`verify-guarantees.sh` (checks 22 and 23).
- **Reverts cleanly:** yes. Self-contained; restores the prior behaviour including the bug.
- **Bounded diff:** ~4 files, ~120 LOC including tests and docs.
### 2. Open the original only when the probe declines
- **Does:** move the sidecar probe ahead of the original open, so a request served a
sidecar performs one verified open instead of two.
- **Three preconditions, each of which the grill found by trying to break this commit
alone. An executor who ignores any one of them ships a behaviour change, not an
optimisation:**
1. **The sidecar name is derivable without an open, including for directories.** A request
path ending in `/` resolves to `<path>` + `INDEX_FILE_NAME` by definition, so the
candidate is `<path>index.html` and the sidecar is that plus `.br`/`.gz`. A slashless
path that is a directory redirects before reaching this point (see the unconditional
redirect restored in `ae8e0e1`) — **confirm that during coding rather than assuming
it**, because if such a path can reach here it would probe `<dir>.br`, miss, and cost
directory requests two extra failed opens. This is what makes probing-first
deterministic rather than a guess about which paths are files.
2. **The original's existence is still established, with a `stat` rather than an open.**
A sidecar present while the original is deleted must stay a `404`: today the original's
open fails and the probe never runs. Dropping that check would turn a `404` into a
`200`. A `stat` is one syscall against the `open` + `fstat` + `F_GETPATH` it replaces,
so the win survives — a claimed win that instead came from skipping a `404` would not
be a win.
3. **Probing-first is valid only when HTML injection is impossible.** `wants_sidecar`
depends on `html_injection`, which depends on the **original's** length and content
type — neither known before the original is opened. So the reordering applies when
`self.broadcaster.is_none() && !self.spa_mode`; with live-reload or SPA mode the
existing order stands. That is the default configuration and the benchmarked one, so
the measured win is the one users get, but the fallback path must remain and be tested.
- **Verifies:** `bench/faceoff.sh` — `static-probe/index.html` rises from ~40,300 toward the
~58,600 non-probing figure, interleaved, three reps. Correctness is held by the existing
differential suite (`tests/cached_serving.rs`) plus commit 1's new cases: status, body and
the **full header set** must be byte-identical for every non-symlink shape.
- **Touches:** `src/server.rs` (`handle_request` ordering only), `bench/README.md`,
`README.md`.
- **Reverts cleanly:** yes — reverting restores commit 1's ordering and its verification.
- **Bounded diff:** 1 source file, ~60 LOC net, plus docs.
- **Depends on commit 1** and says so: without the sidecar's own fd verification, skipping
the original's open would leave containment resting on a path-based argument only —
strictly worse than today. Commit 1 is what makes this safe, which is the ordering's whole
reason.
## Open Questions
- **How does commit 2 obtain a path to derive the sidecar name from, without opening the
original?** From the joined request path, with `INDEX_FILE_NAME` appended for a trailing
slash. This is safe because commit 1 verifies the sidecar's own descriptor, so a derived
name that escapes is caught at the point of use rather than trusted — the derived name is
an *input* to a verified open, not evidence of anything. That is precisely why commit 1
must land first.
- ~~**Directory requests may need commit 2 split.**~~ **Answered by the grill: no split.**
A trailing slash makes the target `<path>index.html` by definition, so the sidecar name is
derivable with no open and no extension heuristic. Recorded as precondition 1 on commit 2,
including the one thing still to confirm in code (that a slashless directory path cannot
reach this point).
- **Neither commit addresses the `Vary: Accept-Encoding` header, deliberately.** It is
emitted unconditionally today, on every response including declines and `304`s, so no
case in this plan can change it. Noted because "the sidecar was declined" is exactly the
situation where a conditional `Vary` would be a cache-poisoning bug, and the reason there
is nothing to do is that the header was never conditional.
- ~~**Does `without_precompressed()` fully mitigate in the interim?**~~ **Answered:
yes, measured.** With it enabled the same request returns the original 16 bytes and no
`content-encoding`; `wants_sidecar` is false so the probe never runs. That is the advice
for anyone who cannot upgrade immediately, and it is the one mitigation that needs no new
release. Note it is only available from 0.34, which is unpublished — so for users on a
published version the mitigation is "do not allow symlinks into the served root."
- **Disclosure.** The bug is in ~29 published versions. Whether it warrants a yank, a
`RUSTSEC` advisory, or a note in the changelog is the maintainer's call, not this plan's.
## Grill Verdict
Round: 1
Status: PASS
Findings resolved:
- `C1-G1`: only one of `open_verified`'s two `cfg`-gated implementations was in scope — both
named explicitly, since a fix that holds only on the platform it was tested on is not a fix.
- `C1-G1`: irregular-file sidecars unaddressed, and routing through `open_verified` made one
case actively worse — a directory sidecar would have resolved to
`styles.css.br/index.html` via the index retry and served it, and a FIFO sidecar blocks the
request. Commit 1 now refuses anything that is not a regular file, one condition covering
both.
- `C1-G3`: the `debug_assert_eq!` on parent equality becomes a relic once the descriptor is
verified — removal made explicit rather than left to the coder's judgement.
- `C1-G4`: no mutation ids and no release-test mechanism; both named, with the reason release
matters (the existing guard is a `debug_assert`, so a debug-only test cannot tell "fixed"
from "asserted").
- `C1-G4`: the plan asserted an in-root symlinked sidecar must be *declined*. Measured against
the running crate, `resolve` serves in-root symlinks with `200`. Corrected before grill —
the rule is "the descriptor resolves inside the root", and a fix refusing all symlinked
sidecars would have broken a build system linking assets from a shared in-root directory.
- `C2-G1`: the directory case was left as an open question for grill to decide. Decided: no
split. A trailing slash makes the target `<path>index.html` by definition.
- `C2-G1`: a sidecar present with the original deleted would have turned a `404` into a `200`.
Commit 2 now keeps an existence check, as a `stat` rather than an open, so the syscall win
survives without the semantic change.
- `C2-G1`: `wants_sidecar` depends on `html_injection`, which depends on the original's length
and content type — unknown before the original is opened. Probing-first is now scoped to
configurations where injection is impossible, with the existing order retained and tested
for live-reload and SPA mode.
- `C2-G4`: all three of the above promoted to numbered preconditions inside commit 2, so it is
executable from its own entry.
Findings accepted as tradeoffs:
- **Commit 1 makes a successful probe marginally slower** (one `real_path_of` per hit) and
ships before the commit that repays it. Accepted: it is a security fix to published code and
must not wait on a performance change to the same function.
- **Disclosure handling is out of scope.** Whether ~29 published versions warrant a yank, a
RUSTSEC advisory, or a changelog note is the maintainer's decision, not this plan's. The
interim mitigation is recorded (no symlinks into the served root; `without_precompressed()`
from 0.34, which is unpublished).
- **`G2` found nothing on either commit.** Both are narrower after grilling than before; the
additions are consequences of the stated fix, not new goals.