fix(events): qualify post-reload $location() call so production/maintenance env switch survives - #3055
Conversation
…enance env switch survives
vendor/wheels/events/onapplicationstart.cfc is a bare `component {` with no
helper scope, yet the redirectAfterReload=true + url.reload present branch
(auto-enabled for production/maintenance) made a bare $location() call. Lucee
threw `No matching function [$LOCATION] found`, aborting the post-switch cold
start, so `?reload=production&password=...` 500'd and silently reverted to the
file-configured environment. Route the call through application.wo.$location()
to match the other 27 helper calls in the file.
Adds security/EventHelperScopeGuardSpec.cfc, a structural guard (in the spirit
of BareCfabortGuardSpec) that fails if any non-mixin event CFC calls an
out-of-scope framework helper without the application.wo. receiver.
Fixes #3054
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Bare event CFCs (component { with no extends) are not processed by
$integrateComponents(), so bare $helper() calls throw on Lucee. Document
the application.wo.$helper(...) pattern in CLAUDE.md invariant #14 and
in .ai/wheels/cross-engine-compatibility.md alongside the guard spec
reference (EventHelperScopeGuardSpec).
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — Docs updatedAdded a doc commit to this PR:
|
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR: This PR fixes #3054 by qualifying the lone bare $location() call in vendor/wheels/events/onapplicationstart.cfc:488 as application.wo.$location(...), and adds a structural guard spec so the bug class cannot reappear. The root-cause analysis holds up under verification: the file is a bare component declaration with no extends (line 1) and therefore no framework-helper surface, $location() is defined only in vendor/wheels/Global.cfc:326, and vendor/wheels/events/init/orm.cfm:52-54 auto-flips redirectAfterReload = true for production/maintenance — so the env-switch cold start deterministically hit the unresolvable call. I traced the new guard spec's scan rules against both event CFCs and against the pre-fix file: pre-fix it flags exactly /events/onapplicationstart.cfc:488 ($location) and nothing else; post-fix it is clean; EventMethods.cfc (which extends wheels.Global) is correctly exempted; comment lines, function definitions, and the locally-defined $resolveAllowEnvironmentSwitchViaUrl call at line 366 are all correctly excluded. No correctness, cross-engine, or security findings — verdict: comment (non-blocking nits below).
Tests
-
Behavioral acceptance criteria #1–#4 of #3054 are covered by inference, not execution. The PR's Test Plan is honest about this: the new automated coverage is the structural scan, and the behavioral criteria (
?reload=productioncompletes, no$LOCATIONlog entries,redirectAfterReload=trueplus?reload=truecompletes) are claimed as direct consequences of the helper now resolving. The inference is strong — the fixed call is now shaped identically to the 26 already-workingapplication.wo.$X(...)sites in the same file — but since the issue ships a concrete curl repro (Lucee 7 docker harness, steps 1–6), running that flow once before un-drafting would close the loop on criteria #1–#4 rather than leaving them inferred. Non-blocking. -
Minor robustness: the bare-component classification can silently exempt a file from the guard.
EventHelperScopeGuardSpec.cfc:72-76classifies by inspecting everything up to the file's first opening brace and checking that the word "extends" is absent. If a future bare event CFC gains a leading docblock that contains the word "extends" (likely — this very spec's own docblock uses it repeatedly while describing the rule) or any opening brace before the component declaration, the file is classified as non-bare and silently dropped from the scan — the guard stops guarding with no failure signal. Both current files classify correctly today (onapplicationstart.cfcopens directly with the component declaration on line 1), so this is future-proofing only. Consider classifying on the first non-comment line containingcomponentinstead, or at least noting the limitation in the docblock's scan rules. Related smaller inconsistency:defPatternis matched with case-sensitiveREMatch(EventHelperScopeGuardSpec.cfc:84) while everything else uses NoCase variants — a definition written asFunction $foowould not be collected as a local method. Non-blocking; the failure mode there is at least a loud false positive, not a silent skip.
Docs
- Off-by-one in the "other 27 helper calls" claim (changelog fragment, PR body, and commit body). Post-fix, the file contains exactly 27
application.wo.$call sites including the newly qualified line 488 (verified by grep — all 27 are real call sites, none in comments), so the count of other qualified calls is 26. The spec docblock's phrasing (EventHelperScopeGuardSpec.cfc:18-19: "Every other one of the 27 helper calls" — i.e. 27 total, all but one qualified) is internally consistent, but the wording "like the other 27 helper calls in the file" inchangelog.d/3054-envswitch-location-scope.fixed.mdis not. Suggest changing "the other 27" to "the other 26" in the fragment (the commit message can stay — not worth a rebase). Cosmetic only.
Everything else checked clean: the commit conforms to commitlint (fix(events): ..., header within 100 chars, DCO sign-off matches the author email exactly); the changelog fragment is correctly named (slug plus .fixed.md) and is a complete bullet line; the spec extends wheels.WheelsTest with BDD syntax, escapes ## correctly inside strings, guards the Left() Lucee 7 edge, and avoids the inline-closure-as-constructor-arg and catch/finally cross-engine traps; the fix itself uses all-named arguments and does not widen the password-gated reload surface.
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR: This PR fixes #3054 by qualifying the lone bare $location() call in vendor/wheels/events/onapplicationstart.cfc as application.wo.$location(...), and adds a structural guard spec so this class of bug cannot reappear. I verified the fix, the guard logic, the helper availability at the call site, and the docs/changelog/commit hygiene — the change is correct and well-scoped. Verdict: comment (one trivial docs nit; nothing blocking).
What I verified
- The fix is correct and sufficient.
onapplicationstart.cfcis a barecomponent {(line 1, noextends), so it never receives thewheels.Globalmixin surface — a bare$location()genuinely cannot resolve there.$location()ispubliconwheels.Global(vendor/wheels/Global.cfc:326) and forwards all arguments (url,addToken) tocflocationvia the plain-struct copy pattern, so it is also safe under cross-engine invariant #10 (Adobe 2023/2025attributeCollectionrestriction). application.wois guaranteed available at the call site. It is assigned inpublic/Application.cfc:135immediately before the event CFC$init(this)invocation at line 138, and lines 17–457 of the event CFC already dereference it unconditionally — line 488 runs strictly after those.- The guard spec exclusion rules hold against the real file. I scanned
onapplicationstart.cfcfor every$name(token: all comment occurrences (lines 4–5, 151, 283, 398) sit on//-prefixed full-line comments and are skipped;new wheels.Plugins().$initializeMixins(variables)(line 421) is dot-preceded and skipped as a member call; the bare local call at line 366 resolves via thelocalMethodsmap (helper defined at line 501); the pre-fix line 488 would be the sole offender, matching the claimed failing-first TDD run. The spec mirrors the acceptedBareCfabortGuardSpecprior art (ExpandPath("/wheels"), recursiveDirectoryList, line-anchored scan,##-escaped hash in strings, no closure-scope or finally-loop pitfalls). - Only two event CFCs exist (
onapplicationstart.cfcbare,EventMethods.cfcextendswheels.Global), so the bare/extends classifier covers the whole scanned surface today. - Tests: the behavioral path (real cold-start redirect) is admittedly not executable from inside a spec — the structural guard is the same compensation pattern this repo established for #3029, and the PR body is honest about that. Acceptable.
- Docs & metadata: changelog fragment present and correctly named (
changelog.d/3054-envswitch-location-scope.fixed.md, no directCHANGELOG.mdedit); CLAUDE.md invariant #14 and the.ai/wheels/cross-engine-compatibility.mdsection are accurate and follow the existing invariant format. - Commits: both conventional (
fix(events): …≈95 chars,docs: …≈73 chars), valid types, DCOSigned-off-bytrailers matching the git author identity. PR title is a valid squash-merge header and targetsdevelop. - Security: no new surface — the redirect URL construction and the
reload,password,lockquery-string stripping are pre-existing and unchanged; only the call receiver changed.
Docs
- Nit (off-by-one):
.ai/wheels/cross-engine-compatibility.md("matches the other 27 helper calls in the file") andchangelog.d/3054-envswitch-location-scope.fixed.md("like the other 27 helper calls in the file") both say other 27. Post-fix there are exactly 27application.wo.$calls in the file including line 488, so the "other" calls number 26. (The spec header phrasing — "every other one of the 27 helper calls already goes throughapplication.wo" — is arithmetically correct; only the two prose spots above double-count.) Suggested wording: "matching the file's 27application.wo.$…helper calls" or "the other 26". Purely cosmetic — fine to fix in a follow-up or ignore.
No correctness, cross-engine, security, or convention findings. Nice work on the failing-first guard spec and the thorough root-cause writeup.
Summary
URL environment switching to
productionormaintenance(?reload=<environment>&password=...) was returning HTTP 500 on the post-switch cold start and silently reverting to the file-configured environment.vendor/wheels/events/onapplicationstart.cfcis a barecomponent {(noextends, and event CFCs are not integrated via the$integrateComponents()mixin pass), so the lone bare$location()call on line 488 was genuinely out of scope. On theredirectAfterReload=true+url.reloadpresent branch — auto-enabled for production/maintenance byvendor/wheels/events/init/orm.cfm, and always hit on the env-switch cold start because the restart redirect preservesreload+password(#3030) — Lucee threwNo matching function [$LOCATION] found, aborting application start. This PR qualifies the call asapplication.wo.$location(...), matching the other 27 helper calls in the file.?reload=testingwas unaffected becausetestingdoes not auto-enableredirectAfterReload, so the bad branch never ran — which is exactly why the existing #3030/#3031 specs and the #3044 smoke probes (which boot directly into production viaenvironment.cfm) never exercised this path.Related Issue
Fixes #3054
Type of Change
Feature Completeness Checklist
Signed-off-by:(git commit -s)vendor/wheels/tests/specs/security/EventHelperScopeGuardSpec.cfc: failing → passingbot-update-docs.ymlbot-update-docs.ymlbot-update-docs.ymlchangelog.d/3054-envswitch-location-scope.fixed.mdwheels.tests.specs.securityon Lucee 7 + SQLite: 289 pass, 0 fail, 0 error (was 1 fail before the fix, pinpointing/events/onapplicationstart.cfc:488 ($location))Test Plan
TDD sequence, run against the Lucee 7 + SQLite core test suite:
EventHelperScopeGuardSpecis a structural guard (in the spirit ofBareCfabortGuardSpecfor dispatch: bare cfabort crashes every Adobe engine in the public-component 404 branch #3029): for every non-mixin (component {with noextends) event CFC, it fails if any bare$helper(...)call references a helper that is not defined locally in the same file. Member-access calls (application.wo.$x(...)), function definitions, and bare calls to locally-defined helpers (e.g.$resolveAllowEnvironmentSwitchViaUrlat line 366) are correctly excluded.wheels.tests.specs.security→ 1 fail, messageFound bare framework-helper call(s) in non-mixin event CFC(s) at: /events/onapplicationstart.cfc:488 ($location), Expected[0]Actual[1].$location(...)→application.wo.$location(...)).wheels.tests.specs.security→ 289 pass, 0 fail, 0 error.This covers acceptance-criterion #5 (the regression guard). The behavioral criteria (production/maintenance switch completes, no
$LOCATIONerrors,redirectAfterReload=true+?reload=truecompletes) are all direct consequences of the helper now resolving — the bare call was the sole cause of the cold-start abort.