deployed_allowlist_parses cannot observe the deployment it is named after #11

New issue

Closed

opened 2026-08-26 04:58:33 +00:00 by jlxq0 · 0 comments

jlxq0 commented

2026-08-26 04:58:33 +00:00

Owner

src/oauth_redirect.rs:347 is named deployed_allowlist_parses and holds
nothing about the deployment. It hardcodes a nine-entry literal, parses it, and
asserts the count is nine. Nothing in the suite reads an environment variable;
grep -n 'env::var' src/oauth_redirect.rs is empty.

AGENTS.md already carries the prose warning ("Adding a URI to
deployed_allowlist_parses does not enable it at runtime"). This issue exists
because the test's name is the thing making the false promise, and a name is
read far more often than a pitfall list.

Measured, not inferred. Three mutations against 12395a1:

Add https://attacker.example/callback to the literal and bump the count to
ten. All 10 tests in the module pass. A URI that is in no deployment and
was reviewed by nobody sails through a test whose name says "deployed".
Neuter the loopback-host guard in validate_redirect_uri, so cleartext
callbacks are accepted on any host. http_is_loopback_only goes red;
deployed_allowlist_parses stays green. It pins nothing about safety.
For contrast, matches_loopback_entry → return false reddens
loopback_entry_matches_any_port and
loopback_port_relaxation_does_not_relax_host_or_path. Those two do hold
their behaviour.

I also compared the literal against the live value
(kubectl get deploy carddav-mcp -n carddav-mcp -o jsonpath=…) and they are
byte-identical right now. That is agreement, not a measurement: mutation 1 shows
the suite would not notice if they stopped agreeing, in either direction — a
dropped entry silently breaking a client, or an added one silently enabling a
redirect nobody reviewed.

Fix is a rename plus a comment saying what it does and does not cover, e.g.
configured_allowlist_shape_parses. Deliberately kept out of PR #8, which Alan
scoped to three prose lines.

A real deployment check would have to read the platform repo's manifest, which
is a different repo and a different pipeline, and is probably not worth it. The
name is.

`src/oauth_redirect.rs:347` is named `deployed_allowlist_parses` and holds nothing about the deployment. It hardcodes a nine-entry literal, parses it, and asserts the count is nine. Nothing in the suite reads an environment variable; `grep -n 'env::var' src/oauth_redirect.rs` is empty. `AGENTS.md` already carries the prose warning ("Adding a URI to `deployed_allowlist_parses` does not enable it at runtime"). This issue exists because the test's *name* is the thing making the false promise, and a name is read far more often than a pitfall list. **Measured, not inferred.** Three mutations against `12395a1`: 1. Add `https://attacker.example/callback` to the literal and bump the count to ten. **All 10 tests in the module pass.** A URI that is in no deployment and was reviewed by nobody sails through a test whose name says "deployed". 2. Neuter the loopback-host guard in `validate_redirect_uri`, so cleartext callbacks are accepted on any host. `http_is_loopback_only` goes red; `deployed_allowlist_parses` **stays green**. It pins nothing about safety. 3. For contrast, `matches_loopback_entry` → `return false` reddens `loopback_entry_matches_any_port` and `loopback_port_relaxation_does_not_relax_host_or_path`. Those two do hold their behaviour. I also compared the literal against the live value (`kubectl get deploy carddav-mcp -n carddav-mcp -o jsonpath=…`) and they are byte-identical right now. That is agreement, not a measurement: mutation 1 shows the suite would not notice if they stopped agreeing, in either direction — a dropped entry silently breaking a client, or an added one silently enabling a redirect nobody reviewed. Fix is a rename plus a comment saying what it does and does not cover, e.g. `configured_allowlist_shape_parses`. Deliberately kept out of PR #8, which Alan scoped to three prose lines. A real deployment check would have to read the platform repo's manifest, which is a different repo and a different pipeline, and is probably not worth it. The name is.

jlxq0 referenced this issue from a commit

2026-08-27 04:37:38 +00:00

fix(test): name the allowlist test for what it checks

jlxq0 referenced this issue from a pull request that will close it,

2026-08-27 04:38:05 +00:00

fix(test): name the allowlist test for what it checks #18

jlxq0 closed this issue

2026-08-27 04:39:48 +00:00