Repository navigation
perf(public): serve dev-UI assets from cacheable route instead of inlining ~1MB per page - #3012
Conversation
…ining ~1MB per page Every dev-UI page rendered through _header.cfm inlined ~1.1MB into the response: four JS bundles and two CSS bundles via cfinclude plus a ~53KB base64-encoded Semantic UI icon font data URI, all under no-cache HTML headers, so the full payload re-downloaded on every page view. Serve those assets from a new wheelsAssets route (/wheels/assets/*[file]) on wheels.Public instead: - assets() action mirrors guideImage()'s canonicalize-and-confine shape but allows subdirectory paths (css/woff_files/icons.woff2) while rejecting traversal, absolute/backslash paths, and non-asset extensions via $resolveDevAssetPath(); responses carry Cache-Control: public, max-age=31536000, immutable. - devAssetUrl() builds versioned URLs (?v=<framework version>) through urlFor(), so all three URL-rewriting modes work. It lives on the component (not helpers.cfm) so every _header.cfm include chain can call it and specs can exercise it on an instance. - _header.cfm drops the dead request.wheelsInternalAssetPath line, the application-scoped icon-font data-URI init block, and the megabyte cfinclude blobs in favor of link/script tags; the Icons @font-face override now points at the served woff2 (same cascade mechanism as the data-URI fix from issue 2421, ~53KB lighter per page). - _header_simple.cfm (error pages) intentionally stays fully inline so error rendering never depends on routing. The route lives at /wheels/assets/* (not /wheels/public/assets/*) because the stock urlrewrite.xml excludes the latter prefix from the index.cfm rewrite. Refs #2959 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com>
![wheels-bot[bot]](https://avatars.githubusercontent.com/in/3657588?s=60&v=4){kind=small}
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: This is a solid, well-scoped perf change that moves ~1MB of inlined JS/CSS from every dev-UI page load to a versioned, immutable-cached route. The path-traversal protection is layered correctly (charset allowlist → .. check → absolute-path check → canonical prefix confinement → extension allowlist → FileExists), the route naming is consistent with the wheels namespace convention, and the PR body documents hand-testing on both Lucee and Adobe 2023. One concrete test coverage gap: the existing PublicComponentProductionSpec gated-handler allowlist was not updated to include assets(), so a future PR could remove $blockInProduction() from the new action without CI catching it. Verdict: comment — no correctness, cross-engine, or security issues found; one test maintenance nit.
Tests
assets() missing from PublicComponentProductionSpec.cfc::gatedHandlers
File: vendor/wheels/tests/specs/security/PublicComponentProductionSpec.cfc, lines 140–152.
var gatedHandlers = [
"consoleeval",
"cli",
"packagelist",
...
"guideImage",
"mcp" // <-- "assets" is absent
];Every other production-gated action on Public.cfc is listed here; the spec exists precisely to lock down the list so that removing a $blockInProduction() guard triggers a CI failure. assets() calls $blockInProduction() correctly today, but without an entry in this list a future maintenance PR could silently drop the guard. assets() should be added to gatedHandlers exactly as "assets" (the case-insensitive regex pattern already handles the function assets() declaration correctly).
Suggested fix: add "assets" to the array in PublicComponentProductionSpec.cfc (after "guideImage" fits thematically).
No URL-encoded traversal test in DevAssetServingSpec.cfc
$resolveDevAssetPath() correctly rejects URL-encoded traversal (e.g. %2e%2e/etc/passwd) because % is not in the charset allowlist [^A-Za-z0-9_\-./]. However, the traversal test suite in DevAssetServingSpec.cfc only covers literal .., backslashes, and leading slashes. The sibling PathTraversalSpec.cfc does exercise URL-encoded inputs against guideImage-era helpers; the new resolver has the same input surface and is worth a parallel case. This is a coverage gap, not a vulnerability.
Docs
The changelog fragment at changelog.d/dev-ui-cacheable-assets.changed.md is present and correctly uses the fragment system rather than editing CHANGELOG.md directly. No doc update required beyond that — this is a framework-internal perf change with no user-facing API surface.
Commits
Single commit perf(public): serve dev-UI assets from cacheable route instead of inlining ~1MB per page — type perf, scope public, header 87 chars ≤ 100, not ALL-CAPS. Passes commitlint.
Wheels Bot — Reviewer B (round 1)A's review is accurate. Both findings are verified against the diff. No sycophancy, no false positives, and the one potential naming discrepancy I spot-checked (route defined as SycophancyNone detected. A backed the approve-adjacent verdict with evidence (layered traversal guard, test counts, commit lint check, hand-test log). False positivesNone detected. Every claim A made is accurate against the diff. Missed issuesNone detected. I re-scanned for:
Verdict alignmentA issued ConvergenceAligned. A's two findings are correct and actionable. The |
- Add `assets` to PublicComponentProductionSpec.gatedHandlers so the static-coverage lock-list catches future removal of $blockInProduction() from wheels.Public.assets(). - Add a URL-encoded traversal case (`%2e%2e/etc/passwd` etc.) to DevAssetServingSpec — parallel to the guideImage-era PathTraversalSpec coverage; the charset allowlist [^A-Za-z0-9_\\-./] in $resolveDevAssetPath() rejects `%`, so these inputs resolve to empty. Refs #2959, #3012 Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — Address Review (round 1)Applied consensus findings from Reviewer A and Reviewer B's convergence (round 1):
Test evidenceCore suite re-run via the local Wheels CLI server (Lucee 7 + SQLite, after
Net +2 specs (the URL-encoded The new commit will trigger a fresh Reviewer A run on the updated SHA. Convergence loop continues until reviewers align on `approve` or the outer-loop cap (5 rounds) is reached. |
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: The test(security) commit at 3b2d985a8 directly addresses both findings from my prior round: "assets" is now in PublicComponentProductionSpec::gatedHandlers and a URL-encoded traversal suite (%2e%2e/etc/passwd, %2E%2E%2Fetc%2Fpasswd, css/%2e%2e/Public.cfc, %2fetc/passwd) is now in DevAssetServingSpec. The overall PR remains clean across correctness, security, cross-engine, tests, docs, and commits. Verdict: approve.
Tests
Both previous findings resolved.
assets() now in PublicComponentProductionSpec::gatedHandlers
vendor/wheels/tests/specs/security/PublicComponentProductionSpec.cfc, line 152: "assets" appears after "guideImage" in the gatedHandlers array. A future maintenance PR that silently drops $blockInProduction() from wheels.Public::assets() will now trigger a CI failure on the security suite. ✓
URL-encoded traversal test added
vendor/wheels/tests/specs/security/DevAssetServingSpec.cfc, lines 63–82: four URL-encoded traversal payloads (%2e%2e/..., %2E%2E%2F..., css/%2e%2e/..., %2f...) now have their own it() block. The reason they are blocked is documented inline: % is not in the charset allowlist [^A-Za-z0-9_\-./], so none of these inputs reaches the filesystem. Parallels the existing guideImage-era PathTraversalSpec coverage. ✓
Commits
Both commits pass commitlint:
perf(public): serve dev-UI assets from cacheable route instead of inlining ~1MB per page— typeperf, scopepublic, 87 chars ≤ 100, signed off by Peter Amiri ✓test(security): address Reviewer A/B consensus findings (round 1)— typetest, scopesecurity, 65 chars ≤ 100, signed off by claude[bot] ✓
Wheels Bot — Reviewer B (round 1)A's review checks out. Both round-1 findings were addressed correctly, A's factual claims are accurate against the actual diff, and the approval verdict is well-founded. SycophancyNone detected. A's approval cites specific files, line numbers, and test payloads — not a bare LGTM. False positivesNone detected. I verified both claims against the diff:
A's commit-header char count for the first commit is off by one (88, not 87), but inconsequential — well under the 100-char limit. Missed issuesNone detected. I re-scanned the diff for:
Verdict alignmentApprove is consistent with the findings: both round-1 blockers are resolved and no new issues are present. ConvergenceAligned on approve. The consensus is that this PR is review-clean on SHA |
…vice (#3242) The dev-UI / CLI dispatcher vendor/wheels/public/views/cli.cfm was a 935-line god template with a 44-case cfswitch whose command handlers could not be unit-tested (the template only runs under a full HTTP request context). This was the remaining open item of issue #2959 (review finding P2) — the recursive-dispatch + envelope defects shipped in #2966 and the ~1MB inline-asset defect in #3012. Extract every command into wheels.public.CliBridge — a plain, stateless component with one method per command and an explicit command->method allowlist. cli.cfm (now ~112 lines) builds the response envelope preamble (security gate, lazy migration discovery), constructs a context, checks handles(), and calls dispatch(); the result is merged into the envelope. Behavior-preserving: handlers are the exact former case bodies with request.wheels.params -> arguments.params, preamble reads -> context.*, and framework primitives (model(), get(), $cliFormatMigrationStatus(), $cliResolveDumpPath()) reached through context.host. An unknown command stays a silent no-op, matching the former default-less switch. Security: dispatch() only ever invoke()s a method named in the allowlist, and params is passed as a single named argument (never spread), so a query-string key cannot become an arbitrary function argument. The bridge is stateless, so one instance is cached on application.wheels (via $cliBridge()) and shared across requests; ?reload=true re-creates it. Verified: cli/security/migrator suites green on Lucee 7; live dispatch of read-only + POST write-path commands + the SEC-4 mutation gate + unknown no-op + reload all confirmed against a running server; cross-engine matrix (Lucee 7 + Adobe 2023, SQLite) shows the cli bundle 126/126 pass with only pre-existing unrelated failures. Refs #2959 Signed-off-by: Peter Amiri <petera@pai.com> Co-authored-by: Peter Amiri <petera@pai.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
What
Dev-UI pages (
/wheels/info,/wheels/routes,/wheels/migrator,/wheels/guides, test-runner HTML pages, …) rendered throughvendor/wheels/public/layout/_header.cfminlined ~1.1MB into every response: four JS bundles + two CSS bundles viacfincludeand a ~53KB base64 icon-font data URI — all underno-cacheHTML headers, so the full payload re-downloaded on every page view. This is slice P11 of #2959 (the asset-delivery half; thecli.cfmCliBridge decomposition remains open on the issue).wheelsAssetsroute (/wheels/assets/*[file]) served bywheels.Public.assets()withCache-Control: public, max-age=31536000, immutable. Mirrors the shippedguideImage()canonicalize-and-confine pattern, extended to allow subdirectory paths (css/woff_files/icons.woff2) while rejecting traversal, absolute/backslash paths, and non-asset extensions ($resolveDevAssetPath()allowlist + canonical-prefix confinement +$blockInProduction()gate).devAssetUrl()builds versioned URLs (?v=<framework version>) throughurlFor(), so rewrite-on/partial/off all work. Defined on the component rather thanhelpers.cfm: included-UDFs are not externally callable on Adobe CF (the pre-existingRouteTesterHardeningSpecerror below demonstrates exactly that engine difference), while a component method works in every_header.cfminclude chain AND is unit-testable. This is the one deliberate deviation from the scout plan._header.cfm: deadrequest.wheelsInternalAssetPathline removed; icon-font data-URI init block removed (_header_simple.cfmkeeps its own copy — error pages stay fully inline by design); inline blobs replaced with<link>/<script>tags; theIcons@font-faceoverride now points at the served woff2 (same cascade mechanism as Icons not rendering correctly in Guides section #2421/fix(view): inline Semantic UI icon font so dev page icons render #2563, ~53KB lighter)./wheels/assets/*(not/wheels/public/assets/*) because the stockurlrewrite.xmlexcludes the latter prefix from the index.cfm rewrite.urlrewrite.xmluntouched.Measured:
/wheels/infodocument shrank from ~1.1MB to ~68KB; assets fetch once and cache for a year keyed on framework version.Test evidence
New spec:
vendor/wheels/tests/specs/security/DevAssetServingSpec.cfc(10 specs — TDD: confirmed failing with method-not-found/route-missing before implementation).DevAssetServingSpecitself 10/10Pre-existing failures (present identically on the develop state in the same harness, unrelated to this change):
wheels.tests.specs.internal.testClientSpec— the TestClient makes loopback HTTP requests to the host-mapped port, unreachable from inside the container (status 0 on every request). Green in CI.RouteTesterHardeningSpec("$$findMatchingRoutesnot found in Public.cfc") — the spec calls ahelpers.cfm-included UDF on the instance, which Adobe does not expose externally. Reproduced byte-for-byte with this branch's changes stashed.Hand-testing (live HTTP against the docker containers, both engines)
/wheels/assets/css/semantic.min.css?v=x→ 200,Cache-Control: public, max-age=31536000, immutable,Content-Type: text/css(Lucee and Adobe 2023).css/woff_files/icons.woff2→ 200,font/woff2.--path-as-is /wheels/assets/../Public.cfc,/wheels/assets/helpers.cfm,/wheels/assets/js/nope.js→ all 404 (both engines)./wheels/info,/wheels/routes,/wheels/guides,/wheels/migrator,/wheels/packages,/wheels/core/tests(HTML),/wheels/app/tests(HTML) all render with versioned asset links; zero inline jQuery/semantic blobs; zero data URIs._header_simple.cfm(data URI present — untouched path confirmed working).Verified over curl, not a desktop browser — no interactive icon-glyph/JS-widget eyeballing was done; the @font-face override mechanism is unchanged from the shipped data-URI fix apart from the
srcURL.Refs #2959
🤖 Generated with Claude Code