fix(public): encode path in route-tester verb-mismatch message, scan alternatives only on no-match

[bpamiri]

Summary

Section D of #2961 (held behind PR #2909, now unblocked):

• SEC-8 (reflected XSS) — the verb-mismatch error message interpolated raw arguments.path into output rendered by routetester.cfm / routetesterprocess.cfm. The sibling 404 branch already wrapped the path in EncodeForHTML; the verb-mismatch branch now does too.
• P14 (double scan) — the alternative-verbs scan (a second full pass over the route table, including lazy .regex writes onto application-scope route structs from the request thread) ran unconditionally on every invocation but is only consumed when nothing matched. It now lives inside the no-match branch, matching Dispatch.cfc's structure.

Refs #2961 — sections A (rendering.cfc) and B/C (db-adapters + EventMethods) follow as separate PRs.

Type of Change

• Security fix

Test Plan

• New security/RouteTesterHardeningSpec: a <img onerror=...> payload path against a hermetic single-POST-route table produces a verb-mismatch error whose message contains <img and never the raw <img (red before: raw payload reached the buffer); plus a structure pin that the alternatives scan starts after the no-match guard. The XSS spec swaps in an isolated route table (restored in finally) because the app's catch-all routes genuinely match the payload path — discovered when the first version passed in isolation but failed in the full suite.
• Full core suite (Lucee 7 + SQLite): 4291 pass / 0 fail / 0 error
• Changelog fragment under changelog.d/

🤖 Generated with Claude Code

[wheels-bot]

Wheels Bot — Reviewer A

TL;DR: This PR closes a reflected-XSS sink in the route-tester's verb-mismatch error path (SEC-8) and moves the alternative-verbs scan to the no-match branch where it is actually needed (P14). Both changes are correct, the fix is consistent with how the sibling 404 branch already worked, and the test coverage is solid. Approving.

Correctness

The fix is accurate and complete:

• helpers.cfm:236 — all user-controlled values in extendedInfo are now encoded. EncodeForHTML(arguments.path) closes the verb-mismatch XSS sink; EncodeForHTML(arguments.requestMethod) was already present; UCase(local.alternativeMatchingMethodsForURL) is built from developer-defined route.methods values (not user input), so no encoding is needed there.
• The 404 branch at helpers.cfm:244-246 already had EncodeForHTML(arguments.path) — the two branches are now symmetric.
• Path normalization (query-param stripping, leading-slash removal at lines 193-197) happens before the error branch, so EncodeForHTML is applied to the already-normalized value. No interaction issue.
• Moving the alternative-verbs scan inside the !ArrayLen(local.matches) guard is correct: the scan result is only consumed in that branch, and it eliminates the eager application-scope mutation on every matched request.

One observation for the record: both rendering templates still emit extendedInfo unencoded (routetesterprocess.cfm:19, routetester.cfm:43). This works correctly here because encoding is applied at source in helpers.cfm, but it creates an implicit trust contract — any future expansion of extendedInfo with user-supplied content must also encode at source. Given the route tester is a dev-only surface this is low risk, but worth noting if the error-struct API is ever extended.

The lazy .regex write onto application.wheels.routes struct members still occurs in the no-match branch (lines 219-220). This was the original behaviour; the PR reduces its frequency from every call to no-match-only, which is a meaningful improvement. A full fix (pre-compute all regexes at route-load time under a lock) is a separate concern from this security PR.

Tests

vendor/wheels/tests/specs/security/RouteTesterHardeningSpec.cfc — two specs:

1. XSS encoding test: swaps in a hermetic single-POST-route table (restored in finally), fires a <img onerror=... GET request, and asserts the verb-mismatch branch is hit and extendedInfo contains <img not <img. Correctly tests the failure mode.
2. Structure pin: reads helpers.cfm via FileRead, extracts 4000 chars from the function start, and asserts alternativeMatchingMethodsForURL appears after !ArrayLen(local.matches). Prevents regression to the unconditional-scan pattern. Note: the 4000-char window is tight against current function size; if the function grows significantly the test will fail loudly (both Find() calls return 0, toBeGT(0) fails), which is an acceptable failure mode rather than a silent false pass.

The application.wheels.routes swap + finally restore pattern is correct.

Commits

fix(public): encode path in route-tester verb-mismatch message, scan alternatives only on no-match — 98 chars, valid type, sentence-case subject, DCO sign-off present. Clean.

Docs

Changelog fragment changelog.d/route-tester-xss-and-scan.fixed.md is present and complete. No user-facing guide changes needed (route tester is a dev-mode tool with no docs page).

[wheels-bot]

Wheels Bot — Reviewer B (round 1)

A's review is thorough and evidence-backed. The claims check out against the diff, the verdict is consistent with the findings, and I have nothing material to add.

Sycophancy

None detected. A cites specific lines for every claim and notes two pre-existing issues it deliberately scopes out (lazy regex writes, unencoded extendedInfo in the rendering templates) rather than waving them away.

False positives

None detected.

• A says EncodeForHTML(arguments.requestMethod) was already present before this PR — confirmed by the diff context (the - line shows it in the old extendedInfo string).
• A says UCase(local.alternativeMatchingMethodsForURL) needs no encoding because it comes from developer-defined route config — correct, route.methods is not user input.
• A's characterisation of the structure-pin test (4000-char window is tight but produces a loud failure rather than a silent false-pass) is accurate.

Missed issues

None detected. One minor point A could have flagged but does not materially affect correctness: the structure pin uses Find("$$findMatchingRoutes", ...) which returns the first occurrence in the file. If the function name appeared in a comment before the definition, the 4000-char window would start at the wrong offset. In practice this is not a problem — the function name is unlikely to appear before its definition in this file — and the failure mode is a false-fail rather than a false-pass, so the test remains reliable.

Verdict alignment

A's APPROVED verdict is consistent with the findings: the XSS fix is correct and symmetric with the 404 branch, the scan relocation is correct, test coverage is solid, commits and changelog are clean.

Convergence

Aligned. The PR closes a reflected-XSS sink in a dev-only surface with correct encoding, paired with a performance tidy-up. No changes needed.