Skip to content

docs(web/guides): fix audited rate-limiting, CORS, and datasource guide errors - #3095

Merged
bpamiri merged 2 commits into
developfrom
peter/docs-audit2-middleware-misc
Jun 12, 2026
Merged

docs(web/guides): fix audited rate-limiting, CORS, and datasource guide errors#3095
bpamiri merged 2 commits into
developfrom
peter/docs-audit2-middleware-misc

Conversation

@bpamiri

@bpamiri bpamiri commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Docs-fix wave for guide-behavioral-audit batch 2, work items p1-11-ratelimit, p1-12-cors, p1-13-datasources. Every correction below was established by the audit verifier with live probes against origin/develop (840274b) on the Lucee 7 + Adobe 2023 docker harness. Scope is limited to the three audited guide pages; pnpm verify:docs passes on all three (18 tagged blocks, 0 failed).

digging-deeper/rate-limiting.mdx

digging-deeper/cors.mdx

  • Per-route Cors example rewritten (claim cors-17, both). Same non-functional .scope(..., callback=function(map){...}) shape — callback routes never registered (/api/ping → 404) and the unclosed scope leaked path prefix + middleware onto every subsequent route (verified live; shipped broken since the original guides rewrite). Replaced with the supported stack form matching middleware-pipeline.mdx, plus an explicit "close the scope with .end()scope() does not take a callback argument" warning. Refs router: scope()/namespace() silently ignore callback= — routes in the callback never register and the unclosed scope swallows every subsequent route (documented in 5+ guides) #3072.
  • Origin matching wording (claim cors-14, docs-wrong): matching uses ListFindNoCase (Cors.cfc:68) — verified Origin: HTTPS://CLIENT.MYAPP.COM matched a lowercase allowlist entry. "Exact string comparison" → "exact match, compared case-insensitively"; "character-for-character" → "exactly (case-insensitively)". No-wildcard/scheme/port/subdomain guidance unchanged (verified accurate).
  • Preflight short-circuit status: observed HTTP 200 + empty body on both engines, never a literal 204 — "204-shaped empty body" → "empty-body response (HTTP 200)". Also noted Access-Control-Max-Age is emitted on every OPTIONS response including disallowed origins (Cors.cfc:147-152).

basics/database-and-multiple-datasources.mdx

  • tableName("X")table("X") in both per-model examples (claim ds-07-overrides-compose, docs-wrong). tableName() is a zero-argument getter (vendor/wheels/model/miscellaneous.cfc:177); the setter is table() (:35). The guide's literal code silently no-ops and dies with Wheels.TableNotFound (verified live on both engines; the corrected form composes with dataSource() + setPrimaryKey() as documented). Added a caution naming the trap. Refs docs+model: guides and CLAUDE.md use non-existent tableName("x") setter — silent no-op, models fall back to the convention table (real setter is table()) #3079.
  • findAll framing fixed (claim ds-21-findall-framing-accuracy, docs-wrong). Default findAll returns a query object (returnAs="query", events/init/functions.cfm:195) — the old text attributed model instances to it. Now points readers at findOne()/findByKey()/findAll(returnAs="objects").
  • invokeWithTransaction contract documented: the invoked method must return a boolean (framework throws otherwise) and returning false rolls back even with transaction="commit" (verifier-confirmed omission).
  • transaction="commit" default chain clarified: defaults to the transactionMode setting, whose framework default is "commit" (events/init/orm.cfm); transactionMode is always set, so "the default when transactionMode is set" was muddled.
  • Cross-datasource transaction engine note: Adobe CF throws Datasource names for all the database tags within the cftransaction tag must be the same.; Lucee 7 runs both statements with no atomicity — silent failure (verified on both engines).
  • Adapter list completed: CockroachDB and Oracle adapters ship (vendor/wheels/databaseAdapters/); H2 noted as Lucee-only in the CI matrix, Oracle as the soft-fail CI leg.
  • Removed the internal "Phase 2b" forward reference (invisible to published-guide readers) and added the dev-mode Wheels.DataSourceNotFound (HTTP 404) note for unresolvable names.

Out of scope (tracked elsewhere in the batch): the same .scope(callback=...) anti-pattern in multi-tenancy.mdx / route-model-binding.mdx, the same req.cgi and tableName() snippets in repo CLAUDE.md, and tableName() misuse in four other guide pages.

🤖 Generated with Claude Code

bpamiri and others added 2 commits June 12, 2026 02:55
…de errors

Guide-behavioral-audit batch 2 (p1-11-ratelimit, p1-12-cors, p1-13-datasources)
corrections, all live-verified against develop on Lucee 7 + Adobe 2023:

rate-limiting.mdx
- keyFunction example read req.cgi, which the middleware context never carries,
  collapsing all clients into one anonymous budget; switched to the verified
  cgi-scope + Len() guard form and documented the context shape (#3074)
- per-route example used .scope(callback=...), which scope() silently ignores
  while the unclosed scope 404s the rest of the app; rewrote to the explicit
  .scope(...)...end() form and called out the footgun (#3072)
- corrected the empty-string-key rationale (budget collapse, not entry growth)
- added an aside: the welcome-page root bypasses the middleware pipeline

cors.mdx
- per-route example had the same non-functional .scope(callback=...) shape;
  rewrote to the stack form matching middleware-pipeline.mdx (#3072)
- origin matching is case-insensitive (ListFindNoCase), not character-for-character
- preflight short-circuit responds HTTP 200, not a 204-shaped body; noted
  Access-Control-Max-Age is emitted even for disallowed origins

database-and-multiple-datasources.mdx
- tableName("X") is a getter no-op (setter is table()); fixed both examples
  and added a caution (#3079)
- findAll default returns a query, not model instances; reworded the framing
- documented the invokeWithTransaction boolean-return contract
- noted the Adobe-vs-Lucee cross-datasource transaction divergence
- clarified the transaction="commit" default chain
- added CockroachDB + Oracle to the adapter list; H2 noted Lucee-only in CI
- removed the internal 'Phase 2b' forward reference

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
…s controller

The aside claimed a fresh app's GET / never counts against a rate-limit
budget. That's only true when root() falls back to the built-in
wheels##wheels welcome page (no to= and no app/views/home/index.cfm) —
the audit-harness shape. A canonical 'wheels new' app routes root to
main##index through a generated user controller, so GET / runs the
middleware pipeline and does count. Recondition the aside on the
internal Wheels controller (/wheels GUI routes or a root left on the
built-in welcome page) per the audit evidence, which scoped this as
harness-relevant rather than a general claim.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer

TL;DR: This PR fixes audited behavioral errors in three guide pages (rate-limiting, CORS, multiple datasources). I independently verified every load-bearing technical claim in the diff against the framework source on this branch and found zero correctness, convention, cross-engine, or security issues — verdict: approve. Note: the head moved during review capture (this session was keyed to 1e0985b9; a second commit 4845e36b reconditioning the rate-limit bypass aside landed after). The review covers the full current diff including that commit, and the second commit's rewording is itself correct per the evidence below.

Verification audit trail

Each doc claim was checked against source rather than taken on faith:

  • tableName()table() — confirmed: table() is the setter (vendor/wheels/model/miscellaneous.cfc:35, writes variables.wheels.class.tableName) and tableName() is a zero-argument getter (:177). The old guide code was a silent no-op; the new caution aside is accurate.
  • keyFunction context shape — confirmed: the middleware request context is exactly {params, route, pathInfo, method} (vendor/wheels/Dispatch.cfc:420-425) and RateLimiter.$resolveKey() passes that struct to the closure (vendor/wheels/middleware/RateLimiter.cfc:197-202). No cgi key, so the old req.cgi.http_x_api_key ?: "anonymous" example could never key per-client. The replacement (engine cgi scope + Len() guard) is correct — missing headers read as empty string in cgi, so an Elvis fallback would indeed never fire.
  • scope() has no callback argument — confirmed: signature is name, path, package, controller, shallow, shallowPath, shallowName, constraints, middleware, binding (vendor/wheels/mapper/scoping.cfc:17-29). A passed callback lands in arguments and is ignored, and without .end() the scope stack leaks onto subsequent routes — both rewritten examples now match the documented stack form (same shape as the CLAUDE.md middleware quick reference).
  • CORS origin matching is case-insensitive — confirmed: $resolveAllowOrigin() uses ListFindNoCase (vendor/wheels/middleware/Cors.cfc:68). "Exact match, compared case-insensitively" is the right wording.
  • Preflight = HTTP 200 empty body; Access-Control-Max-Age on every OPTIONS — confirmed: the OPTIONS branch in Cors.handle() emits Max-Age unconditionally and returns "" with no status-code override, so 200 it is.
  • findAll returns a query by default — confirmed: returnAs = "query" (vendor/wheels/events/init/functions.cfm:195); findOne/findByKey default to "object" (:198-199).
  • invokeWithTransaction boolean contract — confirmed: non-boolean return throws (vendor/wheels/model/transactions.cfc:89-94), and false rolls back even under transaction="commit" (:61).
  • transaction default chain — confirmed: persistence methods default to $get("transactionMode") / application.wheels.transactionMode (e.g. vendor/wheels/model/create.cfc:23, delete.cfc:29), whose framework default is "commit" (vendor/wheels/events/init/orm.cfm:4). The new wording is precise where the old was muddled.
  • Wheels.DataSourceNotFound → 404, names the datasource — confirmed: throw site at vendor/wheels/Model.cfc:407-411 (extendedInfo interpolates the datasource name); 404 mapping pinned by vendor/wheels/tests/specs/events/onerrorSpec.cfc:49-54.
  • Adapter list — confirmed: vendor/wheels/databaseAdapters/ contains CockroachDB and Oracle adapters; H2-is-Lucee-only and Oracle-soft-fail match the CI matrix documented in CLAUDE.md.
  • Internal-controller rate-limit bypass aside (second commit) — confirmed: requests whose controller resolves to wheels are hijacked before the middleware pipeline runs (vendor/wheels/Dispatch.cfc:392 vs the pipeline at :440-450), and root() with no to= falls back to wheels##wheels (vendor/wheels/mapper/matching.cfc:181). Reconditioning the claim from "fresh-app GET /" to "internal Wheels controller" was the right narrowing — a canonical wheels new app routes root through a user controller and does count.

Conventions / Commits / Docs

  • Both commit headers are valid conventional commits (docs(web/guides): …, ≤ 100 chars) with DCO sign-offs; the PR title (the squash-merge lint gate) is likewise valid at 79 chars.
  • Docs-type PR, so no changelog.d/ fragment is required (fragments gate user-facing fix/feat).
  • The PR body explicitly tracks the same req.cgi and tableName() snippets remaining in repo CLAUDE.md and four other guide pages as out-of-scope batch items — good scoping, nothing dropped silently.
  • pnpm verify:docs harness exists (web/sites/guides/package.json:13) and the touched blocks carry {test:compile} tags, consistent with the stated 18-block pass.

One shape I deliberately considered and rejected as a finding: the guide's keyFunction=function(req) {…} inline-closure-as-constructor-arg superficially resembles cross-engine invariant #5 (Adobe ASTcffunction crash), but that crash is specific to CFC compilation contexts — this exact shape is the documented pattern in CLAUDE.md's own RateLimiter reference, pre-exists this PR, and the PR live-verified the closure executing on Adobe 2023.

Clean, evidence-backed docs work. Approving.

@wheels-bot wheels-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wheels Bot — Reviewer

TL;DR: This PR fixes audited behavioral errors across three guide pages (rate-limiting, CORS, multiple datasources), plus a follow-up commit (4845e36) that narrows the rate-limit bypass aside to the internal Wheels controller. I independently re-verified every load-bearing technical claim against the framework source at this head — all of them hold, the rewritten route examples match the repo's documented .scope()…end() prior art, and both commits conform to commitlint with DCO sign-offs. Verdict: approve.

Verification evidence

Each claim the docs now make was checked against the framework source, not just the PR description:

rate-limiting.mdx

  • Middleware request context is {params, route, pathInfo, method} — confirmed at vendor/wheels/Dispatch.cfc:420-425; no cgi key, so the old req.cgi.http_x_api_key example could never work. keyFunction receives exactly this struct (vendor/wheels/middleware/RateLimiter.cfc:199).
  • The new internal-controller aside (the delta commit) is accurate: Dispatch.cfc:392 hijacks any request whose controller resolves to wheels (ListFirst(local.params.controller, '.') EQ "wheels") and returns via $engineAdapter().invokeMethod(...) before the middleware pipeline is built in the else branch — so /wheels GUI routes and a root left on the built-in welcome page never touch a rate-limit budget. The commit message correctly documents why the earlier blanket "fresh-app GET /" framing was wrong for canonical wheels new apps (root → main##index runs the pipeline).
  • The rewritten per-route example is valid mapper API: post() accepts name/pattern/to (vendor/wheels/mapper/matching.cfc:45-47), and the .scope(...) … .end() shape matches the middleware quick reference in CLAUDE.md.

cors.mdx

  • "Exact match, compared case-insensitively" — confirmed: origin matching uses ListFindNoCase at vendor/wheels/middleware/Cors.cfc:68.
  • Access-Control-Max-Age emitted on every OPTIONS including disallowed origins — confirmed: the OPTIONS short-circuit at Cors.cfc:147-152 writes the header unconditionally before returning.
  • "Empty-body response (HTTP 200)" replacing "204-shaped" — consistent with source: no 204 appears anywhere in Cors.cfc; the short-circuit returns "" without setting a status code.
  • scope() takes no callback argument — confirmed against the full signature at vendor/wheels/mapper/scoping.cfc:17-29 (name, path, package, controller, shallow, shallowPath, shallowName, constraints, middleware, binding); a passed callback is silently swallowed, exactly as the new warning states.

database-and-multiple-datasources.mdx

  • table() is the setter (vendor/wheels/model/miscellaneous.cfc:35), tableName() is a zero-argument getter (:177) — both tableName("…")table("…") corrections and the new caution aside are accurate.
  • findAll defaults to returnAs = "query" (vendor/wheels/events/init/functions.cfm:195) while findOne/findByKey default to "object" (:198-199) — the reframed guidance is right.
  • transactionMode framework default is "commit" (vendor/wheels/events/init/orm.cfm:4) — the clarified default chain is correct.
  • invokeWithTransaction boolean contract — confirmed at vendor/wheels/model/transactions.cfc:89-94 ("Methods invoked using invokeWithTransaction must return a boolean value.").
  • CockroachDB and Oracle adapters exist under vendor/wheels/databaseAdapters/ — the completed adapter list is accurate.
  • Wheels.DataSourceNotFound with HTTP 404 in development — the throw is at vendor/wheels/Model.cfc:407-411 (gated on showErrorInformation, names the datasource), and vendor/wheels/events/EventMethods.cfc:83-87 maps any ^Wheels\.[A-Za-z]*NotFound$ type to $header(statusCode = 404).
  • The new <Aside type="caution"> is build-safe: Aside is already imported at line 9 of the file.

Non-blocking observations

  • The PR body already tracks this, but for the record: repo CLAUDE.md's Rate Limiting quick reference still shows the broken req.cgi.http_x_api_key ?: "anonymous" shape this PR corrects in the guide, and the same .scope(callback=...) anti-pattern survives in multi-tenancy.mdx / route-model-binding.mdx. Worth landing those follow-ups soon so the repo doesn't teach agents the pattern these pages now warn against.
  • No changelog fragment, which is correct here — fragments are required for user-facing fix/feat PRs; this is docs type.

Commits

Both commits (1e0985b91, 4845e36b8) use docs(web/guides): … — valid type, sensible scope, subjects under 100 chars, DCO Signed-off-by present, and the follow-up commit body explains the why (audit-harness shape vs. canonical app) rather than restating the diff. Clean.

@bpamiri
bpamiri merged commit 9dc7919 into develop Jun 12, 2026
13 checks passed
@bpamiri
bpamiri deleted the peter/docs-audit2-middleware-misc branch June 12, 2026 11:34
bpamiri pushed a commit that referenced this pull request Jun 12, 2026
…sides now that scope() honors it

#3092 and #3095 landed caution asides on develop (routing.mdx, cors.mdx,
rate-limiting.mdx) stating scope()/namespace() silently ignore callback=,
tracked in #3072. This branch fixes exactly that, so merge develop in and
rewrite the three statements: document the now-supported callback= form
(auto-closing the scope), keep the unclosed-scope warning for the explicit
.end() form, and note that releases <= 4.0.3 still ignore callback=. Adds a
compile-tested namespace(callback=) example to routing.mdx, matching the
already-correct callback examples in route-model-binding.mdx and
multi-tenancy.mdx.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <petera@pai.com>
bpamiri pushed a commit that referenced this pull request Jun 12, 2026
…cope (#3100)

* fix(router): scope()/namespace() honor callback= and auto-close the scope

scope() declared no callback argument, so the documented `.scope(path="/x", callback=function(map){...})` form (and namespace()/package()/controller(), which forward to scope()) silently dropped it: the callback never ran (its routes 404'd) and nothing closed the scope, so every route declared after inherited the scope's path prefix and middleware.

scope() now consumes callback the same way group() does — runs callback(this) then auto-end()s — using the cross-engine-proven IsCustomFunction guard. Adds MapperRobustnessSpec coverage for the scope/namespace/package callback forms plus the no-swallow and no-middleware-leak guarantees for routes declared after the block.

Refs #3072

Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>

* docs: update scope()/namespace() callback= examples in CLAUDE.md (#3072)

Three focused updates:
- Anti-pattern #3: note that scope()/namespace()/package()/controller()
  also accept callback= and auto-close the scope (not just resources())
- Middleware Quick Reference: use callback form for route-scoped .scope()
- Route Model Binding: use callback form for the .scope(binding=true) example

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>

* docs(web/guides): flip the three #3072 'callback= silently ignored' asides now that scope() honors it

#3092 and #3095 landed caution asides on develop (routing.mdx, cors.mdx,
rate-limiting.mdx) stating scope()/namespace() silently ignore callback=,
tracked in #3072. This branch fixes exactly that, so merge develop in and
rewrite the three statements: document the now-supported callback= form
(auto-closing the scope), keep the unclosed-scope warning for the explicit
.end() form, and note that releases <= 4.0.3 still ignore callback=. Adds a
compile-tested namespace(callback=) example to routing.mdx, matching the
already-correct callback examples in route-model-binding.mdx and
multi-tenancy.mdx.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <petera@pai.com>

---------

Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Signed-off-by: Peter Amiri <petera@pai.com>
Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Peter Amiri <petera@pai.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant