refactor(public): extract cli.cfm command handlers into CliBridge service (#2959) - #3242
Conversation
Sorry, something went wrong.
…vice 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 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Peter Amiri <petera@pai.com>
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR — This PR completes the last open item of #2959: it decomposes the 935-line cli.cfm god template (a 44-arm cfswitch that could only run under a live HTTP request) into a plain, stateless, allowlist-gated CliBridge service with one method per command, leaving cli.cfm as a ~112-line dispatcher. I traced every handler against its former case body, confirmed the SEC-4 mutation gate and lazy-discovery preamble are untouched, verified all 31 commands survive the move, and checked that the security posture (allowlist + single named params arg, never spread) is preserved. The refactor is behavior-preserving and ships the unit-test net the template never had. Verdict: comment — no blocking findings; a couple of minor, non-blocking observations below.
Correctness
No blocking issues. One change I scrutinized and cleared:
- vendor/wheels/public/CliBridge.cfc:677-703 (dbSetup) drops the old data.command = "dbSetup" re-assignment that followed StructAppend(data, seedResult, true) in the former switch (cli.cfm old lines ~1536-1537). I confirmed this is not a regression: neither runSeeds (vendor/wheels/Seeder.cfc:67-129) nor generateSeeds (vendor/wheels/Seeder.cfc:250-319) ever returns a command key, and data.command is set in the preamble (cli.cfm:69) before dispatch — so the merged result can never clobber it. The old reset was defensive/redundant.
Cross-engine
No issues found. dispatch() uses invoke(this, local.method, {context=…, params=…}) (CliBridge.cfc:139) — a named-arg struct, portable across engines — and the PR reports the cli bundle (which includes CliBridgeSpec) at 126/126 on both lucee7 and adobe2023. CliBridge is a standalone CreateObject'd component, not a $integrateComponents mixin, so the private $runDbSeed helper (CliBridge.cfc:880) is fine — invariant #7 (public/$-prefixed mixins) does not apply here.
Tests
vendor/wheels/tests/specs/cli/CliBridgeSpec.cfc adds the dispatch-contract net: every declared command resolves via handles(), component methods (init/dispatch/handles) are not reachable as commands, the off-allowlist guard throws Wheels.UnknownCliCommand, and four pure handler branches (dbVersion, introspect missing-param, forgetVersion missing-arg, migrateToLatest delegation) are exercised. This is the right contract to pin.
- Non-blocking: the DB- and worker-backed handlers (dbSchema, dbReset, dbDump, the jobs* family, etc.) remain untested at the unit level, relying on the cross-engine matrix + live-endpoint verification the PR describes. Reasonable for a mechanical move, but now that these branches are extracted into an injectable plain component, they're cheap to cover with a fake context.host — worth a follow-up to lock the seam.
Conventions
- Singleton lifecycle is implemented correctly: $cliBridge() (Public.cfc:18-30) uses double-checked locking keyed on application.applicationName and caches one stateless instance on application.wheels, matching the documented "safe to share across concurrent requests" middleware/cache contract. ?reload=true rebuilds it.
- Minor doc nit: the CliBridge.cfc header and cli.cfm:71 both say "44-case switch", while commandMap declares 31 commands (the 44 presumably counts the nested per-database-type switch arms). Harmless, but the two numbers invite a double-take.
Commits
refactor(public): extract cli.cfm command handlers into CliBridge service (#2959) conforms to commitlint.config.js — valid type, unrestricted scope, subject well under 100 chars, not ALL-CAPS. No changelog fragment needed: this is an internal, behavior-preserving refactor, not a user-facing fix/feat.
Security
Posture is preserved and slightly improved. dispatch() only ever invoke()s a method named in variables.commandMap (CliBridge.cfc:131-140), and params is passed as a single named argument — never spread — so a query-string key cannot become an arbitrary function argument. The SEC-4 POST + loopback + reload-password mutation gate (cli.cfm:8-38) and the SEC-5 $cliResolveDumpPath app-root confinement (CliBridge.cfc:728, resolving against Public.cfc:150) are both intact.
Nice work — this is a well-scoped, well-tested extraction.
Sorry, something went wrong.
Summary
Completes the last open item of #2959 (review finding P2): decompose the vendor/wheels/public/views/cli.cfm god template into a CliBridge service. The other two acceptance items already shipped — recursive-dispatch + envelope normalization (#2966) and cacheable dev-UI assets (#3012).
cli.cfm was a 935-line template with a 44-case cfswitch whose handlers couldn't be unit-tested (the template only runs under a full HTTP request context).
What changed
Behavior-preserving
Each handler is the exact former case body with mechanical substitutions: request.wheels.params → arguments.params, preamble reads (data.migrations, data.currentVersion, …) → arguments.context.*, framework primitives (model(), get(), $cliFormatMigrationStatus(), $cliResolveDumpPath()) → arguments.context.host.*, and data.X = … writes → an accumulating local.rv the dispatcher merges via StructAppend. 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 remote arg-injection risk flagged in the #2959 cross-framework research). The mutation gate (POST + reload password for state-changing commands) is unchanged in the dispatcher preamble.
Testability win
CliBridge is a plain component, so the dispatch contract and pure handler branches are now unit-testable — the regression net the template never had. New vendor/wheels/tests/specs/cli/CliBridgeSpec.cfc covers the allowlist, unknown-command guard, and representative handlers (dbVersion, introspect missing-param, forgetVersion missing-arg, migrateToLatest delegation).
Verification
Refs #2959
🤖 Generated with Claude Code