test(auth): make the revocation fixture a real form-urldecoder

[cliffhall]

Closes #2222

The report's premise turned out to be wrong, and that changed the fix

#2222 says core/auth/revocation.ts breaks a client secret containing +, because encodeURIComponent leaves + bare and a form-urldecoder reads a bare + as a space. It does not:

> encodeURIComponent("ab+cd")
'ab%2Bcd'

encodeURIComponent leaves exactly !'()*-._~ and alphanumerics unescaped, and a form-urldecoder passes every one of those through unchanged — so it can never emit the one character the two algorithms disagree about. Its output decodes identically under both. Checked exhaustively rather than argued: formUrlDecode(encodeURIComponent(s)) === s for every code point up to U+2FFF, zero mismatches.

The proposed switch to URLSearchParams would be a small regression. It is the literal algorithm §2.3.1 names, but it encodes a space as + — which a compliant server reads back as a space and a lenient one (decoding with decodeURIComponent alone) reads as a literal +. %20 is understood by both:

secret	encoder	strict (form) server	lenient (decodeURIComponent) server
ab cd	encodeURIComponent → ab%20cd	✅ ab cd	✅ ab cd
ab cd	URLSearchParams → ab+cd	✅ ab cd	❌ ab+cd
ab+cd	encodeURIComponent → ab%2Bcd	✅ ab+cd	✅ ab+cd

So the encoder is unchanged, and the reasoning is recorded in a comment on it — the next reader will have the same doubt, and the answer is not obvious from the line itself.

What was genuinely broken: the fixture was the encoder's own inverse

The second half of the report stands, and is the better bug. test-servers' /oauth/revoke decoded with decodeURIComponent, the exact inverse of the encoder under test, so the round trip succeeded for every input by construction. No test in either suite could have failed on an encoding mistake — the suite's apparent coverage of RFC 6749 §2.3.1 client authentication was really a statement that the encoder is self-consistent with itself.

Changes

• test-servers/src/test-server-oauth.ts — /oauth/revoke now runs a real Appendix B decode (+ → space before percent-decoding; doing it after would turn a legitimately escaped %2B into a space). The existing try/catch is kept, so a malformed escape is still an invalid_client 401 rather than an Express 500.
• clients/web/src/test/core/auth/revocation.test.ts — cases for a credential containing +, a space, : and %, each asserted twice: the exact wire bytes (which encoder ran) and a round trip through an independent compliant decoder (what a real server gets back). The decoder is written out inline rather than imported, so the assertion does not lean on the fixture it exists to corroborate.
• clients/web/src/test/integration/auth/revocation-e2e.test.ts — a third static client whose secret is base64 with a + in it, revoked end to end against the real fixture; plus a guard that an unencoded credential with a + is now refused, which is the one case that separates the two decoders and the assertion that goes red if anyone reverts the fixture.
• core/auth/revocation.ts — comment only. No behavior change.

The existing surrogate case was also tightened: it asserted only status: "failed", which a stub returning nothing satisfies anyway (reading .ok off undefined throws into the same catch). It now asserts the fetch was never called, so it is about the encoder rather than about the stub.

Mutation-checked

Each new guard was removed in turn to confirm it detects rather than merely passes:

mutation	result
fixture decoder → decodeURIComponent (the old behavior)	1 failed, 8 passed
encoder made to emit a bare + (the bug as reported)	1 failed, 8 passed (e2e) · 2 failed, 54 passed (unit)

npm run local:gate passes. No UI change, so no screenshots.

---

🤖 Generated with Claude Code

https://claude.ai/code/session_01BhVaufrEZAfFAXcg6456hK

[Copilot]

🟢 Approval recommended

The fixture correction is standards-aligned and covered by focused unit and integration tests.

Pull request overview

Improves revocation credential testing by making the OAuth fixture decode form-encoded Basic credentials correctly.

Changes:

• Adds compliant form URL decoding to the revocation fixture.
• Adds unit and integration coverage for reserved characters.
• Documents why encodeURIComponent remains appropriate.

File summaries

File	Description
test-servers/src/test-server-oauth.ts	Corrects credential decoding.
core/auth/revocation.ts	Documents encoding rationale.
clients/web/src/test/core/auth/revocation.test.ts	Adds wire-format and round-trip tests.
clients/web/src/test/integration/auth/revocation-e2e.test.ts	Adds end-to-end + credential coverage.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0
• Review effort level: Balanced

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟢 Approval recommended

The fixture now independently models compliant server decoding, and the expanded tests directly distinguish correct encoding from the previous self-inverse setup.

Review details

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Balanced