Skip to content

fix(controller): super.config() in chapter 6 + generator templates so CSRF inherits (F10)#2404

Merged
bpamiri merged 2 commits into
developfrom
claude/fresh-vm-batch-l-f10-csrf
May 1, 2026
Merged

fix(controller): super.config() in chapter 6 + generator templates so CSRF inherits (F10)#2404
bpamiri merged 2 commits into
developfrom
claude/fresh-vm-batch-l-f10-csrf

Conversation

@bpamiri
Copy link
Copy Markdown
Collaborator

@bpamiri bpamiri commented May 1, 2026

Summary

F10 from the 2026-05-01 afternoon fresh-VM run. Originally hypothesised as a "CSRF flag never set in production" framework bug needing a new middleware; actually a chapter-6 paste-bug + generator-template defensiveness gap. The framework's CSRF wiring is intact — what was missing was `super.config()` calls in places where users override `config()`.

Investigation that overturned the original diagnosis

Initial grep for `request.$wheelsProtectedFromForgery` only found assignments in test fixtures. Conclusion: "the flag is never set in production." Wrong.

Closer reading: `vendor/wheels/controller/csrf.cfc` defines `$flagRequestAsProtected()` which DOES set the flag, called from `$runCsrfProtection(action)` which DOES run on every action via `vendor/wheels/controller/processing.cfc:12`. The chain only fires when `variables.$class.csrf` is set, which only happens when the controller called `protectsFromForgery()` in `config()`.

The generated `app/controllers/Controller.cfc` (project base, written by `wheels new`) calls `protectsFromForgery()` in its `config()`. So new apps DO get protection — IF child controllers call `super.config()`.

The fresh-VM observation matches this exactly:

Controller `config()` override `super.config()` called Form has token
Chapter 6 `Sessions.cfc` none n/a (inherits)
Chapter 6 `Users.cfc` none n/a (inherits)
Chapter 6 `Posts.cfc` (6a) yes (adds `filters`) no
Chapter 6 `Posts.cfc` (6b) yes (adds `filters`) no

Same code pasted twice in chapter 6 was the root cause. The signup/login forms had tokens because their controllers don't override `config()` at all. The post-create form didn't have a token because chapter 6's `Posts.cfc` paste broke the inheritance chain.

Fixes

Chapter 6 paste-bugs

Both `Posts.cfc` instances now call `super.config();` as the first line of `config()`. Plus a new `

` callout right after the controller walkthrough explaining the failure mode:

The first line of `config()` is `super.config();`. If you skip it, you silently disable everything `app/controllers/Controller.cfc` does in its `config()` — most importantly, `protectsFromForgery()` …

Generator templates

Four templates that were prone to the same trap now call `super.config()` with explanatory comments:

  • `cli/src/templates/ControllerContent.txt` (legacy CommandBox)
  • `cli/lucli/templates/app/app/snippets/ControllerContent.txt` (LuCLI app template)
  • `cli/lucli/templates/snippets/crud-controller.txt` (CRUD snippet)
  • `cli/lucli/templates/snippets/auth-sessions-controller.txt` (auth snippet)

The fifth template, `cli/lucli/templates/snippets/api-controller.txt`, is the deliberate exception. APIs usually authenticate via API keys / bearer tokens, not session cookies, so the session-CSRF check doesn't apply. Updated to call `protectsFromForgery(with="ignore")` explicitly so the opt-out is intentional rather than accidental.

What this PR doesn't do

  • No framework code change. The infrastructure is correct; the bug was upstream of it.
  • No new middleware. My initial proposal involved a `Csrf` middleware that would have duplicated the existing controller-layer logic. Not needed.
  • No global default flip. I considered adding `application.wheels.csrfProtection = true` as a framework-level setting that would force protection even when controllers forget `super.config()`. Decided against it for this PR — would require updating ~10 framework test fixtures (every `vendor/wheels/tests/_assets/controllers/*.cfc` that doesn't already call `protectsFromForgery`) since they'd suddenly start rejecting POSTs with no token. That's a separate larger PR if the team wants belt-and-braces protection against the super.config() footgun.

Tests

Doc/template-only changes, no framework code touched.

  • Full SQLite suite: 3377 passed, 0 failed (same baseline as #2403)
  • The chapter 6 `{test:compile}` doctest annotation will validate that the new `super.config();` line compiles in CI.

Memory note

The `project_csrf_never_enforced.md` memory entry has been corrected. The original misdiagnosis is preserved in the entry as a "lesson learned" anchor — a good example of why grep-based "never assigned" conclusions need a second pass for indirect setters.

🤖 Generated with Claude Code

bpamiri and others added 2 commits May 1, 2026 12:11
… CSRF inherits

F10 from the 2026-05-01 afternoon fresh-VM run. Originally hypothesised
to be a "CSRF flag never set in production" framework bug; turned out to
be a chapter-6 paste-bug plus a generator-template defensiveness gap.

Root cause: Wheels' CSRF wiring is intact — app/controllers/Controller.cfc
(generated by `wheels new`) calls protectsFromForgery() in config(), and
processing.cfc:12 calls $runCsrfProtection per action. The chain breaks
when a child controller overrides config() without calling super.config(),
because the parent's protectsFromForgery() never runs and
variables.$class.csrf is never set.

The fresh-VM report observed exactly this asymmetry:

  - Chapter 6 Sessions.cfc / Users.cfc: no config() override → inherit
    cleanly → forms have hidden authenticityToken.
  - Chapter 6 Posts.cfc (in BOTH 6a hand-rolled and 6b SessionStrategy
    versions): overrides config() to add filters(through="authenticate")
    without super.config() → CSRF chain broken → form has no token.

Fixes:

  - Chapter 6 Posts.cfc (both versions): add `super.config();` as the
    first line of config(). One-line diff each.

  - New "Always call super.config() when overriding" caution callout
    after the Posts.cfc walkthrough, explaining the failure mode so
    future readers don't hit it.

  - Four generator templates updated to call super.config() in their
    config() blocks:
      cli/src/templates/ControllerContent.txt
      cli/lucli/templates/app/app/snippets/ControllerContent.txt
      cli/lucli/templates/snippets/crud-controller.txt
      cli/lucli/templates/snippets/auth-sessions-controller.txt

  - api-controller.txt is the deliberate exception. APIs usually
    authenticate via API keys / bearer tokens, not session cookies, so
    the session-CSRF check doesn't apply. Updated to explicitly call
    protectsFromForgery(with="ignore") so the opt-out is intentional
    rather than accidental.

Tests: doc/template-only changes, no framework code touched. Full
SQLite suite green (3377 passed, same baseline as F15 Phase 2).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…oller comment

Two minor follow-ups from self-review of the F10 fix:

  - Chapter 6 had `</Aside>\n\n\nThe filter is private` (two blank lines
    after the new Aside callout). Other Asides in the file use a single
    blank line as separator; matching that convention.

  - The api-controller template's config() comment said "Either skip the
    super.config() call (as below) or call protectsFromForgery(with=\"ignore\")"
    but the example below actually does the latter, not the former.
    Reworded so "below" matches what's actually below.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bpamiri bpamiri merged commit 7aa4a10 into develop May 1, 2026
10 checks passed
@bpamiri bpamiri deleted the claude/fresh-vm-batch-l-f10-csrf branch May 1, 2026 19:21
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