diff --git a/changelog.d/3080-migrate-info-doctor-read-side-fallback.fixed.md b/changelog.d/3080-migrate-info-doctor-read-side-fallback.fixed.md new file mode 100644 index 0000000000..b0b6a15e66 --- /dev/null +++ b/changelog.d/3080-migrate-info-doctor-read-side-fallback.fixed.md @@ -0,0 +1 @@ +- `wheels migrate info` and `wheels migrate doctor` honor the read-side common-port fallback again. #2879 documented that read-only commands keep the legacy port probe (8080, 60000, 3000, 8500) when no `lucee.json` / `.env` port is configured, but `cli.lucli.Module::runMigration()` gated every migrate subcommand behind `requireProjectConfig = true`, so the two read-only inspectors refused a server on 8080 — the first entry of their own documented fallback list — with `Wheels.ServerNotRunning`. The schema-mutating actions (`latest`, `up`, `down`) keep the strict project-bound gate, and because a fallback attach can reach a *sibling* project's server (and report the wrong app's migration state), `info`/`doctor` now print a yellow notice naming the port whenever the fallback was used, with the `lucee.json` / `PORT` hint to pin the project. Covered by new call-site gating specs in `cli/lucli/tests/specs/services/ServerDetectionSpec.cfc` (#3080) diff --git a/cli/lucli/Module.cfc b/cli/lucli/Module.cfc index c2979530c3..db65a3bd18 100644 --- a/cli/lucli/Module.cfc +++ b/cli/lucli/Module.cfc @@ -3976,13 +3976,41 @@ component extends="modules.BaseModule" { } private string function runMigration(required string action) { - var serverPort = $requireRunningServer( - hints = [ - "Migrations require a running server bound to this project.", - "Set 'port' in lucee.json (or PORT in .env), then start with: wheels start" - ], - requireProjectConfig = true - ); + // latest/up/down change the schema — they must only ever target the + // server bound to this project's own lucee.json/.env port (#2878). + // info/doctor are read-only and keep the legacy common-port fallback, + // matching the other read-side commands (info, routes, console, + // dbStatus, dbVersion) — the contract #2879 documented but the gate + // here didn't honor (#3080). + var mutatingAction = listFindNoCase("latest,up,down", arguments.action) > 0; + + var serverPort = 0; + if (mutatingAction) { + serverPort = $requireRunningServer( + hints = [ + "Migrations require a running server bound to this project.", + "Set 'port' in lucee.json (or PORT in .env), then start with: wheels start" + ], + requireProjectConfig = true + ); + } else { + serverPort = $requireRunningServer( + hints = ["Start one with: wheels start"], + requireProjectConfig = false + ); + // Transparency for the fallback attach: with no project-bound port + // we cannot prove the server on a common port belongs to this + // project — a sibling app's server would report the WRONG + // project's migration state. Say which port we attached to and + // how to pin it. + if (!detectServerPort(requireProjectConfig = true)) { + out( + "Attached to localhost:#serverPort# via the common-port fallback (no project-bound port in lucee.json / .env).", + "yellow" + ); + out("If this is not this project's server, set 'port' in lucee.json (or PORT in .env) and re-run.", "yellow"); + } + } out("Running migration: #action#...", "cyan"); @@ -4000,8 +4028,8 @@ component extends="modules.BaseModule" { // latest/up/down change the schema — the framework's /wheels/cli // bridge requires POST + the reload password for state-changing // commands. info/doctor are read-only and stay on GET. - var mutatingAction = listFindNoCase("latest,up,down", arguments.action) > 0; - + // (`mutatingAction` is resolved at the top of this function — the + // same read/write split also decides the server-identity gate.) var httpResult = ""; try { httpResult = mutatingAction ? makeBridgePost(migrateUrl) : makeHttpRequest(migrateUrl); diff --git a/cli/lucli/tests/specs/services/ServerDetectionSpec.cfc b/cli/lucli/tests/specs/services/ServerDetectionSpec.cfc index 42dc45d3a7..38956a4af2 100644 --- a/cli/lucli/tests/specs/services/ServerDetectionSpec.cfc +++ b/cli/lucli/tests/specs/services/ServerDetectionSpec.cfc @@ -53,6 +53,38 @@ component extends="wheels.wheelstest.system.BaseSpec" { testHelper.cleanupTempProject(variables.tempRoot); } + /** + * Capture the requireProjectConfig flag runMigration() hands to + * $requireRunningServer() for a given migrate action (#3080). The mocked + * guard throws so the command aborts before any HTTP probing — the call + * log then exposes the exact named arguments the call site passed. + */ + private boolean function capturedRequireProjectConfig(required string action) { + // MockBox writes its generated method stubs to /testbox/system/stubs + // (webroot-relative) and removes them after mixing in — make sure the + // directory exists. java.io.File.mkdirs() recurses parents on every + // engine and is a no-op when the directory already exists (same + // workaround as vendor/wheels/tests/specs/controller/channelSpec.cfc). + createObject("java", "java.io.File").init(expandPath("/testbox/system/stubs")).mkdirs(); + + var m = new cli.lucli.Module(cwd = variables.tempRoot); + prepareMock(m); + m.$( + method = "$requireRunningServer", + throwException = true, + throwType = "TestAbort.ServerGuard", + throwMessage = "spec capture — abort before HTTP" + ); + try { + m.migrate(arg1 = arguments.action); + } catch (any e) { + // expected: the mocked guard throws TestAbort.ServerGuard + } + var log = m.$callLog()["$requireRunningServer"]; + expect(arrayLen(log)).toBeGTE(1, "migrate #arguments.action# never reached $requireRunningServer()"); + return log[1].requireProjectConfig; + } + function run() { describe("detectServerPort — server-identity guard (##2878)", () => { @@ -109,6 +141,58 @@ component extends="wheels.wheelstest.system.BaseSpec" { }); + describe("read-side migrate gating — info + doctor (##3080)", () => { + + // #2879 documented that read-side commands keep the legacy + // common-port fallback, but runMigration() gated EVERY migrate + // subcommand behind requireProjectConfig=true — so `migrate info` + // and `migrate doctor` refused a server on 8080 (the first + // documented fallback port). These specs pin the call-site wiring: + // info/doctor pass requireProjectConfig=false, the schema-mutating + // actions keep requireProjectConfig=true. + + it("migrate info keeps the read-side common-port fallback (requireProjectConfig=false)", () => { + expect(capturedRequireProjectConfig("info")).toBeFalse(); + }); + + it("migrate doctor keeps the read-side common-port fallback (requireProjectConfig=false)", () => { + expect(capturedRequireProjectConfig("doctor")).toBeFalse(); + }); + + it("migrate latest still refuses the common-port fallback (requireProjectConfig=true)", () => { + expect(capturedRequireProjectConfig("latest")).toBeTrue(); + }); + + it("migrate up still refuses the common-port fallback (requireProjectConfig=true)", () => { + expect(capturedRequireProjectConfig("up")).toBeTrue(); + }); + + it("migrate down still refuses the common-port fallback (requireProjectConfig=true)", () => { + expect(capturedRequireProjectConfig("down")).toBeTrue(); + }); + + it("migrate info in a no-config project never throws the project-bound refusal", () => { + // End-to-end through the real (unmocked) guard. Environment + // tolerant: when something IS listening on a common port the + // command proceeds past the guard (and fails later on HTTP / + // response parsing — fine); when nothing is listening it must + // throw the READ-SIDE ServerNotRunning message (which names + // the probed ports), never the project-bound refusal. + if (fileExists(tempRoot & "/lucee.json")) fileDelete(tempRoot & "/lucee.json"); + if (fileExists(tempRoot & "/.env")) fileDelete(tempRoot & "/.env"); + var state = {sawProjectBoundRefusal = false}; + try { + mod.migrate(arg1 = "info"); + } catch (any e) { + if (e.type == "Wheels.ServerNotRunning" && !findNoCase("8080", e.message)) { + state.sawProjectBoundRefusal = true; + } + } + expect(state.sawProjectBoundRefusal).toBeFalse(); + }); + + }); + describe("write-side command gating — reload + generate admin", () => { // Drive the real callers (not detectServerPort) to prove the call diff --git a/web/sites/guides/src/content/docs/v4-0-0/basics/migrations.mdx b/web/sites/guides/src/content/docs/v4-0-0/basics/migrations.mdx index 00e41d16df..f4b1718f3f 100644 --- a/web/sites/guides/src/content/docs/v4-0-0/basics/migrations.mdx +++ b/web/sites/guides/src/content/docs/v4-0-0/basics/migrations.mdx @@ -39,7 +39,7 @@ wheels --version The runner wraps each migration in a transaction so a failing `up()` or `down()` rolls back cleanly on databases that support transactional DDL — but that support varies, so read the caution below before relying on it. One more caveat for scripts and CI: a failed migration is loud in the command output but the CLI currently still exits `0`, so don't gate a pipeline on the exit code alone ([#3081](https://github.com/wheels-dev/wheels/issues/3081)).