Skip to content

fix(cli): source reloadPassword from .env in scaffold; reconcile on WHEELS_RELOAD_PASSWORD - #2857

Merged
bpamiri merged 4 commits into
developfrom
peter/scaffold-reload-password-env
Jun 4, 2026
Merged

bpamiri merged 4 commits into
developfrom
peter/scaffold-reload-password-env

Conversation

@bpamiri

@bpamiri bpamiri commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

Why

Discussion #2854 reported that wheels new hard-codes the generated reload password as a literal in config/settings.cfm — a committed file — and echoes it again in a comment, so the secret lands in version control. The docs also disagreed on the variable name and accessor, so copy-pasting a documented snippet into a fresh app could resolve to "" and trip the "reloadPassword is empty" warning.

Tracing the code confirmed:

  • The scaffold wrote the random value to three places: config/settings.cfm set(), a settings.cfm comment, and .env.
  • .env used RELOAD_PASSWORD while the deploy templates + upgrade guide used WHEELS_RELOAD_PASSWORD.
  • The "Environments and Configuration" guide used application.wo.env(...); the migration guide called bare env() the "canonical pattern"; production-config.mdx called application.wo.env(...) "the canonical accessor." Genuinely split.

What changed

Canonical form going forward: set(reloadPassword = env("WHEELS_RELOAD_PASSWORD", "")), with the secret living only in the git-ignored .env.

Scaffold (cli/lucli/templates/app/)

  • config/settings.cfm + both app/snippets/*.txt now read the password via env("WHEELS_RELOAD_PASSWORD", ""). The generator's random value is written only to .env; the literal is gone from the committed file and its comment.
  • _env now emits WHEELS_RELOAD_PASSWORD — consistent with WHEELS_ENV / WHEELS_DATASOURCE and the wheels deploy secrets contract, so the same env() lookup works unchanged from a laptop .env to a production deploy.

CLI / console

  • detectReloadPassword() (and its e2e mirror) now match (?:WHEELS_)?RELOAD_PASSWORD, so apps generated before the rename keep working. Added a spec asserting WHEELS_RELOAD_PASSWORD detection.
  • Console + wheels reload hint strings now name WHEELS_RELOAD_PASSWORD.

Docs — standardized on bare env() + WHEELS_RELOAD_PASSWORD

  • core-concepts/environments-and-configuration and all 5 deployment guides (production-config, security-hardening, docker-deployment, vm-deployment, dev-server).
  • Replaced an insecure docker example — Server.System.getEnv("RELOAD_PASSWORD") ?: "changeme" (wrong accessor that bypasses .env, unprefixed name, guessable fallback) — with env("WHEELS_RELOAD_PASSWORD", "").
  • Refreshed the stale security-hardening note that claimed the template hard-codes {{reloadPassword}}, plus a stale Global.cfc line reference.

Scope note

The audit found the drift was wider than the two docs cited in the discussion — hence the 5 deploy-guide edits. Deferred to a follow-up: 13 application.wo.env usages remain in 4 non-deploy guides (database-and-multiple-datasources, sending-email, authentication-patterns, testing) for a separate accessor sweep.

  • Follow-up: convert remaining application.wo.env → env() in the 4 non-deploy guides

Verification

  • ✅ Regex logic verified: (?:WHEELS_)?RELOAD_PASSWORD\s*=\s*(.+) matches both WHEELS_RELOAD_PASSWORD= and legacy RELOAD_PASSWORD= and captures the value.
  • ✅ Scaffold output grep'd clean — no reloadPassword="..." literal remains.
  • ✅ All {test:compile} doc snippets use the bare <cfscript> set(…=env(…)) form already proven to compile in the migration guide.
  • ⏳ Full CFML/e2e/docs-harness/compat-matrix validation runs in CI against this branch (the e2e suite that holds the new spec runs in deploy-ci.yml; the docs harness needs npm ci + a CFML backend; the onboarding harness can't reliably test worktree templates locally due to the brew-wrapper LUCLI_HOME gotcha). Not claimed as locally green.

🤖 Generated with Claude Code

…HEELS_RELOAD_PASSWORD

`wheels new` hard-coded the generated reload password as a literal into
config/settings.cfm (a committed file) and echoed it again in a comment,
leaking a secret into version control. It also wrote the value to .env
under RELOAD_PASSWORD while docs/deploy templates used WHEELS_RELOAD_PASSWORD,
so copy-pasting the documented env() snippet into a fresh app resolved to ""
and tripped the "reloadPassword is empty" warning.

Scaffold:
- config/settings.cfm and both app/snippets/*.txt now read the password via
  `set(reloadPassword=env("WHEELS_RELOAD_PASSWORD", ""))`; the random value
  the generator creates lives only in .env (git-ignored). Comment no longer
  contains the literal.
- _env now emits WHEELS_RELOAD_PASSWORD, consistent with WHEELS_ENV /
  WHEELS_DATASOURCE and the `wheels deploy` secrets contract (so the same
  env() lookup works unchanged from laptop .env to production deploy).

CLI / console:
- detectReloadPassword() and the e2e mirror accept `(?:WHEELS_)?RELOAD_PASSWORD`
  so apps generated before the rename keep working. Added a CLI spec asserting
  WHEELS_RELOAD_PASSWORD detection.
- Console + reload hint strings now name WHEELS_RELOAD_PASSWORD.

Docs (standardize on bare env() + WHEELS_RELOAD_PASSWORD):
- core-concepts/environments-and-configuration and the 5 deployment guides
  (production-config, security-hardening, docker-deployment, vm-deployment,
  dev-server) reconciled to the bare env() accessor and the prefixed name.
- Replaced an insecure docker example that used
  `Server.System.getEnv("RELOAD_PASSWORD") ?: "changeme"` (wrong accessor,
  unprefixed, guessable fallback) with `env("WHEELS_RELOAD_PASSWORD", "")`.
- Refreshed the stale security-hardening note that claimed the template
  hard-codes {{reloadPassword}}, and the Global.cfc line reference.

Follow-up: 13 `application.wo.env` usages remain in 4 non-deploy guides
(database-and-multiple-datasources, sending-email, authentication-patterns,
testing) — deferred to a separate docs-accessor sweep.

Reported in #2854

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 A

TL;DR: The PR correctly identifies and fixes the primary security issue — the scaffold template was committing a random literal reload password to version control via config/settings.cfm and its snippets. The CLI rename (RELOAD_PASSWORD → WHEELS_RELOAD_PASSWORD), docs sweep, and backwards-compatible regex are all well-executed. However, two files that directly exhibit the exact same vulnerability are left unchanged: examples/starter-app/config/settings.cfm still has set(reloadPassword="changeme"), and cli/src/commands/wheels/config/check.cfc still generates a sample .env with the old variable name. Requesting changes on those two, plus a minor test message nit and a missing CHANGELOG entry.


Correctness

examples/starter-app/config/settings.cfm:26 — hardcoded literal survives

// Reload your application with ?reload=true&password=changeme
set(reloadPassword="changeme");

This is the same committed-literal pattern the PR exists to eliminate. The starter-app is a reference implementation that developers copy from. Leaving "changeme" here undermines the PR's goal: a user who scaffolds from the starter-app or follows it as a template will commit a guessable password. Fix:

// Reload your application with ?reload=true&password=<your WHEELS_RELOAD_PASSWORD>
// The secret is read from .env (git-ignored) so it never lands in version control.
set(reloadPassword=env("WHEELS_RELOAD_PASSWORD", ""));

The starter-app's .env.example (examples/starter-app/.env.example) should also gain a WHEELS_RELOAD_PASSWORD= entry so users know what variable to populate.

cli/src/commands/wheels/config/check.cfc:374 — auto-repair generates mismatched variable name

"RELOAD_PASSWORD=your_reload_password",

wheels config check --fix auto-creates a sample .env using the old variable name. Any new app's config/settings.cfm now calls env("WHEELS_RELOAD_PASSWORD", ""), so a user who runs wheels config check --fix gets a .env with RELOAD_PASSWORD — which the framework will not find, silently producing an empty reload password and the "reloadPassword is empty" boot warning. (The CLI's backwards-compatible detectReloadPassword() regex helps password detection in the CLI itself, but env("WHEELS_RELOAD_PASSWORD", "") in config/settings.cfm uses an exact key lookup via application.env["WHEELS_RELOAD_PASSWORD"] — there is no fallback alias at the CFML runtime layer.)

Fix: change line 374 to "WHEELS_RELOAD_PASSWORD=your_reload_password".


Tests

cli/tests/specs/e2e/ServerCommandsTest.cfc:171 — stale assertion message

expect(envPos).toBeGT(0, "Should check .env for RELOAD_PASSWORD");

The test checks that Module.cfc source contains RELOAD_PASSWORD — which it still does (in the backwards-compat regex comment and pattern). The test passes, but the message now reads as an assertion about the old name being canonical rather than a legacy-path check. Suggest updating to:

expect(envPos).toBeGT(0, "Should check .env for WHEELS_RELOAD_PASSWORD (or legacy RELOAD_PASSWORD)");

Docs

No CHANGELOG.md [Unreleased] entry

The PR renames the scaffolded .env variable from RELOAD_PASSWORD to WHEELS_RELOAD_PASSWORD. Existing apps that already have a .env with RELOAD_PASSWORD=... are unaffected (their config/settings.cfm still has the old literal form), but anyone who regenerates the scaffold or copies a snippet from the new guides will need to know to update their .env key. A short note under [Unreleased] ### Fixed or ### Changed documents the migration path and the backwards-compatible detection in detectReloadPassword().


Minor nit (no change required)

The two snippet files ConfigDataSourceH2Content.txt and ConfigReloadPasswordContent.txt remain byte-for-byte identical after the change (they were identical before too — pre-existing duplication, out of scope here).

The Dockerfile example in docker-deployment.mdx:61 still bakes WHEELS_RELOAD_PASSWORD=change-me into the image via ENV. The rename is correct; the guessable placeholder is a pre-existing documentation choice and the compose file's ${WHEELS_RELOAD_PASSWORD:?required} form is the authoritative pattern. Worth a doc note somewhere that the ENV default should be overridden before shipping, but not blocking.

@wheels-bot

wheels-bot Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Wheels Bot — Reviewer B (round 1)

A's review is well-grounded. Every finding traces to actual file content, the verdict is consistent, and there is no sycophancy. I verified all three claims independently and find no false positives. Aligned on request-changes.

Sycophancy

None detected. A requested changes and cited specific file paths and line numbers for every finding.

False positives

None detected.

  • examples/starter-app/config/settings.cfm — verified: the file exists and line 28 reads set(reloadPassword="changeme"). A's claim is accurate.
  • cli/src/commands/wheels/config/check.cfc:374 — verified: the file exists and line 374 still contains "RELOAD_PASSWORD=your_reload_password". A's claim is accurate. A's reasoning that env("WHEELS_RELOAD_PASSWORD", "") in the generated config/settings.cfm does an exact key lookup and has no CFML-layer alias fallback is also correct — detectReloadPassword() in the CLI handles both names but the CFML runtime env() call does not.
  • Test message nit (line 171) — verified: the stale message "Should check .env for RELOAD_PASSWORD" is present. A correctly marks it non-blocking.
  • CHANGELOG — confirmed: the [Unreleased] section has no entry for the RELOAD_PASSWORD to WHEELS_RELOAD_PASSWORD scaffold rename or the security fix.

Missed issues

None that rise to blocking. One minor downstream note A does not call out explicitly: cli/tests/specs/commands/ConfigCommandsTest.cfc:190 has a createTestEnvironment() fixture that writes RELOAD_PASSWORD=... into a test .env. This is a companion update that will be needed when check.cfc:374 is fixed — the test fixture should reflect the new output. It is implied by the check.cfc fix rather than an independent miss, so not a reason to widen the request-changes scope.

Verdict alignment

A's request-changes verdict is consistent with two substantive findings: the starter-app still exhibits the exact vulnerability the PR was written to eliminate, and wheels config check --fix would silently create a mismatched .env key that would leave the app with an empty reload password and a boot warning.

Convergence

Aligned. A's findings are all confirmed, no false positives to retract, and the request-changes verdict is correct. The two blocking items — examples/starter-app/config/settings.cfm and cli/src/commands/wheels/config/check.cfc:374 — are concrete and actionable.

… .env name

Reviewer A (wheels-bot) findings:

- examples/starter-app/config/settings.cfm committed a guessable
  `set(reloadPassword="changeme")` — the same vuln class this PR removes
  from the scaffold. Now reads `env("WHEELS_RELOAD_PASSWORD", "")` (the
  starter-app's Application.cfc already loads .env), and its .env.example
  gains a WHEELS_RELOAD_PASSWORD entry.

- cli/src/commands/wheels/config/check.cfc (`wheels config check --fix`,
  still shipped as the legacy CommandBox wheels-cli module) generated a
  sample .env with the old RELOAD_PASSWORD key. Since env() does an exact
  key lookup at the CFML layer (no WHEELS_ alias there — only the CLI's
  detection regex has it), a new app's config/settings.cfm would not find
  it. Renamed to WHEELS_RELOAD_PASSWORD (now consistent with the WHEELS_ENV
  line already in the same sample).

- Updated the stale ServerCommandsTest assertion message to name
  WHEELS_RELOAD_PASSWORD (or legacy RELOAD_PASSWORD).

- Added a CHANGELOG [Unreleased] Fixed entry documenting the rename and the
  RELOAD_PASSWORD -> WHEELS_RELOAD_PASSWORD .env migration path.

Not changed (intentional): the docker ENV `change-me` default is a
documented placeholder and the compose `${WHEELS_RELOAD_PASSWORD:?required}`
form is authoritative (non-blocking nit); the tweet example already uses
`set(reloadPassword="")` (no secret); the root demo app + tools/build/base
keep their known dev passwords (test/build infrastructure).

Signed-off-by: Peter Amiri <peter@alurium.com>
@github-actions github-actions Bot added the docs label Jun 4, 2026
@bpamiri

bpamiri commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks @wheels-bot — all four addressed in a7b5cd7:

  • examples/starter-app/config/settings.cfm committed literal — ✅ fixed. Now set(reloadPassword=env("WHEELS_RELOAD_PASSWORD", "")) (the starter-app's Application.cfc already loads .env), and examples/starter-app/.env.example gains a WHEELS_RELOAD_PASSWORD= entry. Good catch — a committed "changeme" is actually worse than the random literal the scaffold used.
  • cli/src/.../config/check.cfc:374 --fix sample .env — ✅ fixed (RELOAD_PASSWORD= → WHEELS_RELOAD_PASSWORD=). You're right that env() is an exact-key lookup at the CFML layer — the (?:WHEELS_)? alias only lives in the CLI's own detectReloadPassword() regex — so the legacy CommandBox wheels config check --fix would otherwise hand users a .env the framework can't read. The fix also makes that sample internally consistent with the WHEELS_ENV line two rows above it.
  • Stale test assertion message — ✅ updated to "Should check .env for WHEELS_RELOAD_PASSWORD (or legacy RELOAD_PASSWORD)".
  • CHANGELOG — ✅ added an [Unreleased] ### Fixed entry covering the rename and the RELOAD_PASSWORD → WHEELS_RELOAD_PASSWORD .env migration path for existing apps.

On the non-blocking nits: left the docker ENV …=change-me default as-is (the compose ${WHEELS_RELOAD_PASSWORD:?required} form is the authoritative pattern, as you noted) and the pre-existing snippet duplication. For completeness on the other in-repo apps I checked: examples/tweet already uses set(reloadPassword="") (no secret), and the root demo app + tools/build/base keep their known dev passwords as test/build infrastructure (the CLI test harness reloads against the root app), so I left those untouched.

@wheels-bot

wheels-bot Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Wheels Bot — Address Review (round 1)

Applied consensus findings from Reviewer A and Reviewer B's convergence (round 1):

  • examples/starter-app/config/settings.cfm:24-26 — replaced the committed set(reloadPassword="changeme") literal (and the password=changeme comment) with set(reloadPassword=env("WHEELS_RELOAD_PASSWORD", "")). The reference scaffold no longer exhibits the same leak the parent PR eliminates from the generator.
  • examples/starter-app/.env.example — added a WHEELS_RELOAD_PASSWORD= entry so the example mirrors what config/settings.cfm now reads.
  • cli/src/commands/wheels/config/check.cfc:374 — renamed the auto-generated sample-.env entry from RELOAD_PASSWORD=your_reload_password to WHEELS_RELOAD_PASSWORD=your_reload_password, so wheels config check --fix produces a key the scaffolded env("WHEELS_RELOAD_PASSWORD", "") lookup actually resolves.
  • cli/tests/specs/e2e/ServerCommandsTest.cfc:171 — updated the stale assertion message to "Should check .env for WHEELS_RELOAD_PASSWORD (or legacy RELOAD_PASSWORD)". The findNoCase("RELOAD_PASSWORD", …) search still matches because Module.cfc retains the (?:WHEELS_)?RELOAD_PASSWORD regex.
  • CHANGELOG.md — added an [Unreleased] ### Fixed entry documenting the scaffold rename, the env() canonical accessor, and the backwards-compatible detection regex, referencing fix(cli): source reloadPassword from .env in scaffold; reconcile on WHEELS_RELOAD_PASSWORD #2857.

Skipped (not part of the consensus):

  • The docker-deployment.mdx ENV WHEELS_RELOAD_PASSWORD=change-me placeholder — A explicitly marked it "not blocking" and noted the ${WHEELS_RELOAD_PASSWORD:?required} compose form is already the authoritative pattern.
  • The duplicate-snippet observation (ConfigDataSourceH2Content.txt ≡ ConfigReloadPasswordContent.txt) — A marked it "no change required" and explicitly out of scope.
  • cli/tests/specs/commands/ConfigCommandsTest.cfc:190 createTestEnvironment() writing RELOAD_PASSWORD=… — B mentioned it as a "companion update implied by the check.cfc fix" but explicitly chose not to widen the request-changes scope; the fixture's .env key is independent of check.cfc's --fix output assertion. Left for the next loop if a reviewer pulls it in.

Tests: not re-run for this round. The branch is not fix/bot-* so the skill's mandatory test step doesn't apply, and the consensus changes are string-only (sample-.env template content, an assertion message, a .env.example, a CFML config literal swapped for the same env() call already used elsewhere in the PR, and a CHANGELOG markdown entry). No vendor/wheels/ code path was touched.

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.

@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 A

TL;DR: The author has addressed every finding from the previous Reviewer A pass - starter-app literal, check.cfc sample .env, test message nit, and CHANGELOG entry are all fixed. The core security change (removing committed reload secrets from scaffold templates) is correct, the backwards-compat regex is sound, and the docs sweep is thorough. One correctness regression was introduced: tools/test-onboarding.sh uses grep -E '^RELOAD_PASSWORD=' (anchored, no WHEELS_ prefix) in six places to read the reload password from the scaffold-generated .env, but the scaffold _env template now emits WHEELS_RELOAD_PASSWORD= - so every reload the onboarding harness attempts falls back to "wheels" and fails. Requesting changes on that one file.


Correctness

tools/test-onboarding.sh - six anchored greps will silently miss the renamed key

The scaffold _env template now writes:

WHEELS_RELOAD_PASSWORD={{reloadPassword}}

But tools/test-onboarding.sh reads the password at all six reload points with:

local_password=$(grep -E '^RELOAD_PASSWORD=' "$APP_DIR/.env" 2>/dev/null | cut -d= -f2 || echo "wheels")

(lines 407, 487, 563, 665, 805, 1049)

The ^ anchor means ^RELOAD_PASSWORD= does not match a line that starts with WHEELS_RELOAD_PASSWORD=. The grep returns empty, the shell takes the || echo "wheels" branch, and every subsequent curl "/?reload=true&password=wheels" call is sent against an app whose actual reload password is a random 32-hex-char string. All harness phases that touch the reload endpoint fail.

Note: tools/test-cli-e2e.sh:505 uses the unanchored grep RELOAD_PASSWORD (no -E, no ^) which correctly matches WHEELS_RELOAD_PASSWORD=… as a substring - that file is unaffected.

Fix for all six lines in tools/test-onboarding.sh:

local_password=$(grep -E '^(WHEELS_)?RELOAD_PASSWORD=' "$APP_DIR/.env" 2>/dev/null | cut -d= -f2 || echo "wheels")

Tests

cli/tests/specs/e2e/ServerCommandsTest.cfc:919 - test setup fixture uses legacy key (non-blocking)

The createTestServer() helper at line 919 writes RELOAD_PASSWORD=testapp to its fixture .env. The backwards-compat regex (?:WHEELS_)?RELOAD_PASSWORD\s*=\s*(.+) in detectReloadPasswordFromEnv still matches this, so no test breaks today. Worth updating to WHEELS_RELOAD_PASSWORD=testapp in a follow-up for consistency, but not blocking.

cli/tests/specs/commands/ConfigCommandsTest.cfc:190 - multi-env fixture uses old key (non-blocking)

The createTestEnvironment() helper writes RELOAD_PASSWORD=... to .env.$environment (not .env), so it is isolated from the main reload-password flow. Still worth a pass once the test-onboarding.sh fix lands.


Minor nit (no change required)

The six tools/test-onboarding.sh lines are the only blocking item. Everything else in the PR - scaffold templates, starter-app, check.cfc, docs, consoleeval.cfm, backwards-compat CLI regex, and CHANGELOG - is correct and complete.

@teleotic

teleotic commented Jun 4, 2026

Copy link
Copy Markdown

wheels new also hard-codes the same password into lucee.json as the Lucee admin password.

@wheels-bot

wheels-bot Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Wheels Bot — Reviewer B (round 1)

A's review is accurate and well-grounded on the new SHA. The single blocking finding — six anchored greps in tools/test-onboarding.sh that miss the renamed key — is confirmed. No false positives, no sycophancy, no significant missed issues. Aligned on request-changes.

Sycophancy

None detected. A requested changes with a specific file, six concrete line numbers, and a ready-to-apply fix.

False positives

None detected.

  • Six ^RELOAD_PASSWORD= greps in tools/test-onboarding.sh — verified. Lines 407, 487, 563, 665, 805, and 1049 all use grep -E '^RELOAD_PASSWORD='. The ^ anchor means the pattern does not match a line that begins with WHEELS_RELOAD_PASSWORD=, so every reload attempt in the harness silently falls back to the literal "wheels" default. The scaffold _env template change (RELOAD_PASSWORD={{reloadPassword}} → WHEELS_RELOAD_PASSWORD={{reloadPassword}}) is in the diff; tools/test-onboarding.sh is not. A's claim and proposed fix are both accurate.
  • tools/test-cli-e2e.sh not affected — verified. Lines 429 and 505 use unanchored grep RELOAD_PASSWORD (no -E, no ^), which matches WHEELS_RELOAD_PASSWORD=… as a substring. A correctly exempted this file.
  • tools/installer/macos/install-wheels RELOAD_PASSWORD local variable — not a miss. The installer uses RELOAD_PASSWORD as a bash local variable whose value is forwarded to wheels generate app … reloadPassword=$RELOAD_PASSWORD. That is the CLI argument name, not the .env key name; the scaffold template writes WHEELS_RELOAD_PASSWORD= regardless of what the caller named its local variable.

Missed issues

None that rise to blocking. A's two non-blocking notes (ServerCommandsTest.cfc:919 and ConfigCommandsTest.cfc:190 fixture keys) are reasonable deferral calls — the back-compat regex keeps those tests green.

Verdict alignment

A's request-changes verdict is consistent: one concrete harness regression that will silently break every onboarding-harness reload, with a one-line fix already supplied.

Convergence

Aligned. A's single blocking finding is confirmed accurate, no false positives to retract, and request-changes is the correct verdict. The fix is one-liner: replace the six grep -E '^RELOAD_PASSWORD=' invocations in tools/test-onboarding.sh with grep -E '^(WHEELS_)?RELOAD_PASSWORD='.

…d grep missed renamed key)

The reload-password rename (RELOAD_PASSWORD -> WHEELS_RELOAD_PASSWORD in the
scaffold _env template) broke tools/test-onboarding.sh, which read the
password at six reload points with an anchored `grep -E '^RELOAD_PASSWORD='`.
The `^` anchor does not match a line starting with `WHEELS_RELOAD_PASSWORD=`,
so the grep returned empty, the shell fell back to `|| echo "wheels"`, and
every `?reload=true&password=wheels` call ran against an app whose real
password is the random 32-hex value from .env — failing every harness phase
that touches the reload endpoint.

Anchored pattern is now `^(WHEELS_)?RELOAD_PASSWORD=`, matching both the new
prefixed key and the legacy unprefixed one. tools/test-cli-e2e.sh is
unaffected (it uses an unanchored `grep RELOAD_PASSWORD` that matches the
prefixed key as a substring). Caught by Reviewer A round 2 on #2857.

Signed-off-by: Peter Amiri <peter@alurium.com>
@wheels-bot

wheels-bot Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Wheels Bot — Address Review (round 2)

Applied consensus findings from Reviewer A and Reviewer B's convergence on a7b5cd7224571aab89ed09054f371bb591fa18e5:

  • tools/test-onboarding.sh:407,487,563,665,805,1049 — replaced all six grep -E '^RELOAD_PASSWORD=' invocations with grep -E '^(WHEELS_)?RELOAD_PASSWORD='. The ^-anchored pattern was silently missing the renamed WHEELS_RELOAD_PASSWORD= line emitted by the updated _env scaffold template, so every onboarding-harness reload was falling back to the literal "wheels" default and hitting a random-secret app with the wrong password. The optional-prefix pattern matches both forms and keeps legacy .env files working.

Skipped (not part of the consensus):

  • cli/tests/specs/e2e/ServerCommandsTest.cfc:919 createTestServer() fixture writing RELOAD_PASSWORD=testapp — Reviewer A explicitly marked this non-blocking (the (?:WHEELS_)? regex in detectReloadPasswordFromEnv matches it). Left for a follow-up.
  • cli/tests/specs/commands/ConfigCommandsTest.cfc:190 createTestEnvironment() writing RELOAD_PASSWORD=… to .env.$environment — A marked it non-blocking as well (isolated from the main reload flow). Carried over from the round 1 skip list.
  • @teleotic's note that wheels new hard-codes the same password into lucee.json as the Lucee admin password — this is from a human contributor, not part of the Reviewer A/B converged set, and it touches a different surface (Lucee server admin credentials vs. Wheels app reload password). Not actionable in this convergence loop; worth a separate issue/PR if confirmed.

Tests: not re-run for this round. The branch is peter/scaffold-reload-password-env (not fix/bot-*), so the skill's mandatory test step doesn't apply, and the change is a six-line regex update in the onboarding test harness. No vendor/wheels/ code path was touched.

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.

@bpamiri

bpamiri commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator Author

wheels new also hard-codes the same password into lucee.json as the Lucee admin password

This one is probably fine since we are seeding the admin password for the Lucee server but it should probably then be .gitignored. Let me analyze this one and the ramifications.

@bpamiri

bpamiri commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator Author

Fixed the blocking item in 78d4a94, @wheels-bot — genuinely good catch, this was a regression I introduced by renaming the key without sweeping all consumers.

  • tools/test-onboarding.sh (6 reload points) — ✅ the anchored grep -E '^RELOAD_PASSWORD=' can't match a line starting with WHEELS_RELOAD_PASSWORD=, so the harness fell back to "wheels" and every reload hit the app with the wrong password. Now ^(WHEELS_)?RELOAD_PASSWORD= (matches both the new and legacy keys); verified the grep | cut extracts the value for each form.

I also did the full-repo sweep I should have run when renaming:

  • tools/test-cli-e2e.sh — unaffected (unanchored grep RELOAD_PASSWORD matches the prefixed key as a substring), as you noted.
  • v3-0-0/* guides, docs/superpowers/* plans, and examples/tweet/.ai/* reference docs — left as-is (prior-version docs / historical planning records / example-internal AI docs on a different accessor — out of this PR's scope).

On the two non-blocking test fixtures (ServerCommandsTest.cfc:919, ConfigCommandsTest.cfc:190): leaving both. Neither models real scaffold output — they use arbitrary keys (APP_NAME/PORT, DATABASE_NAME/ENCRYPTION_SALT), the back-compat regex reads their legacy RELOAD_PASSWORD fine, and they give incidental coverage of the legacy detection path. That matches your "non-blocking / follow-up" call.

wheels-bot[bot]
wheels-bot Bot previously approved these changes Jun 4, 2026

@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 A

TL;DR: All blocking findings from both previous review rounds are resolved. The core security fix is complete and correct — scaffold templates no longer commit a reload-password literal to version control, the backwards-compat detection regex is sound, the starter-app guessable literal is gone, check.cfc generates the renamed key, the CHANGELOG is updated, and all six tools/test-onboarding.sh anchored-grep lines now handle the renamed key. Two minor test-fixture nits remain (flagged as non-blocking in round 2) but are genuine follow-up candidates. Approving.


Tests

cli/tests/specs/e2e/ServerCommandsTest.cfc:919 — createTestServer() fixture still writes legacy key (non-blocking)

"APP_NAME=testapp" & nl & "PORT=8080" & nl & "RELOAD_PASSWORD=testapp" & nl

The backwards-compat regex (?:WHEELS_)?RELOAD_PASSWORD\s*=\s*(.+) continues to match this, so no test breaks. Worth updating to WHEELS_RELOAD_PASSWORD=testapp in a follow-up for consistency, but not blocking.

cli/tests/specs/commands/ConfigCommandsTest.cfc:190 — createTestEnvironment() writes to .env.$environment, not .env (non-blocking)

The fixture writes RELOAD_PASSWORD= to .env.production / .env.development, which is isolated from the main reload-password detection path. Same follow-up opportunity as above.


Minor observation (no change required)

tools/test-cli-e2e.sh:429 and :505 use unanchored grep RELOAD_PASSWORD — these already correctly match WHEELS_RELOAD_PASSWORD= as a substring and were not touched by this PR. No action needed.

@wheels-bot

wheels-bot Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Wheels Bot — Reviewer B (round 1)

A's review is accurate and well-grounded on the current SHA. Every approval claim traces to actual diff content I independently verified. No sycophancy, no false positives, no missed issues. Aligned on approve.

Sycophancy

None detected. A's approval cites specific file paths, line numbers, and diff evidence for every resolved blocking item. The two non-blocking nits are correctly categorised rather than glossed over.

False positives

None detected.

  • Scaffold template no longer commits a literal — verified: cli/lucli/templates/app/config/settings.cfm in the diff now emits set(reloadPassword=env("WHEELS_RELOAD_PASSWORD", "")). A's claim is accurate.
  • All six tools/test-onboarding.sh anchored greps updated — verified: the diff shows all six grep -E '^RELOAD_PASSWORD=' lines replaced with grep -E '^(WHEELS_)?RELOAD_PASSWORD='. A's claim is accurate.
  • check.cfc generates the renamed key — verified: the diff shows -"RELOAD_PASSWORD=your_reload_password" / +"WHEELS_RELOAD_PASSWORD=your_reload_password" at the relevant site. A's claim is accurate.
  • tools/test-cli-e2e.sh not affected — verified: A correctly exempted this file; the unanchored grep RELOAD_PASSWORD at lines 429/505 matches the prefixed key as a substring and the diff does not touch it.
  • Non-blocking test fixture nits (ServerCommandsTest.cfc:919, ConfigCommandsTest.cfc:190) — A correctly marks these non-blocking. The back-compat regex (?:WHEELS_)?RELOAD_PASSWORD\s*=\s*(.+) matches the legacy fixture keys, so no tests break.

Missed issues

None detected. The diff covers all surfaces called out in prior rounds (scaffold template, _env, snippets, starter-app committed literal and .env.example, check.cfc --fix output, CHANGELOG, onboarding harness, CLI error messages, and deployment guides). No cross-engine CFML concerns are introduced — the only CFML change is the env() helper call already used elsewhere in the PR, and it is engine-agnostic.

Verdict alignment

A's approve is consistent with the findings: every blocking item from prior rounds is resolved, both non-blocking nits are explicitly acknowledged with a rationale for deferral, and no new issues are introduced.

Convergence

Aligned. All blocking findings from rounds 1 and 2 are confirmed resolved, no false positives to dispute, and the approve verdict is correct. The PR is review-clean for this SHA.

teleotic noted on #2857 that `wheels new` also embedded the same generated
random value into `lucee.json` as the Lucee Server Admin password — a second
committed-secret path independent of config/settings.cfm, and arguably higher
impact (the admin gates the Lucee Server Admin UI).

The scaffold `lucee.json` admin password now reads `#env:WHEELS_RELOAD_PASSWORD#`
instead of the literal. LuCLI resolves `#env:VAR#` against the project `.env`
(combined with system environment) when it loads lucee.json at server start
(LuceeServerConfig.substituteField applies to admin.password, which is not a
protected zone; envFile defaults to `.env`). So the random value lives only in
the git-ignored `.env`, the committed `lucee.json` carries no secret, and its
shareable config (ports, mappings, datasources) stays version-controlled — no
fresh-clone breakage that git-ignoring the whole file would cause.

Reuses the existing WHEELS_RELOAD_PASSWORD value (the scaffold already used one
value for both the reload password and the Lucee admin password); this just
stops committing it. Decoupling into a distinct WHEELS_LUCEE_ADMIN_PASSWORD
would be a separate enhancement.

Reported by @teleotic on #2857.

Signed-off-by: Peter Amiri <peter@alurium.com>
@bpamiri

bpamiri commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator Author

Analyzed — good catch, @teleotic, and you're right that it's the same class of leak (and arguably higher-impact, since the Lucee Server Admin password gates the admin UI). Confirmed lucee.json was committing the generated value, independent of the config/settings.cfm path.

Rather than git-ignore the whole lucee.json — which would drop its shareable config (ports, mappings, datasources) from version control and break wheels start on a fresh clone — LuCLI's native env interpolation gives a cleaner fix. LuceeServerConfig runs admin.password through #env:VAR# substitution at server start, resolved against the project .env (which LuCLI auto-loads). So as of 289f6d5 the scaffold's lucee.json reads:

"admin": { "enabled": true, "password": "#env:WHEELS_RELOAD_PASSWORD#" }

The random value now lives only in the git-ignored .env, the committed lucee.json carries no secret, and its config stays version-controlled. (It reuses the existing WHEELS_RELOAD_PASSWORD since the scaffold already used one value for both; decoupling into a distinct WHEELS_LUCEE_ADMIN_PASSWORD would be an easy follow-up.) Thanks for flagging it.

@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 A

TL;DR: The previous round-3 approval was dismissed to pick up this 4th commit, which extends the secret-elimination work to lucee.json -- the Lucee Server Admin password was being committed as a literal alongside the CFML reload password. The fix is directionally correct, and the commit message is unusually detailed about the LuCLI mechanism. One open question about fallback behavior when WHEELS_RELOAD_PASSWORD is absent, and no dedicated test for the new interpolation path. Posting a comment (not requesting changes) because neither concern is blocking, but both are worth the author confirming before merge.


Correctness

cli/lucli/templates/app/lucee.json -- unverified fallback when WHEELS_RELOAD_PASSWORD is unset

The commit message asserts that LuCLI's LuceeServerConfig.substituteField resolves #env:VAR# in admin.password at server start from the project .env. That claim is plausible and the author cites a specific internal class -- but the diff alone does not confirm what happens when WHEELS_RELOAD_PASSWORD is absent from both .env and the process environment (e.g., a bare clone before wheels new, or a CI pipeline that does not inject the var).

Two possible outcomes:

  • LuCLI substitutes an empty string: Lucee starts with no admin password (undesirable, but not an open credential)
  • LuCLI leaves the literal #env:WHEELS_RELOAD_PASSWORD# as the password: admin UI is gated behind the interpolation string itself, which is surprising but at least not guessable

For the wheels new happy path this is a non-issue: the generated .env always contains WHEELS_RELOAD_PASSWORD=<random>. The risk is narrow (bare-clone or template-copy without scaffolding), but it is worth a sentence in the PR body or a comment in lucee.json clarifying the expected fallback behavior.


Tests

No test covers the #env:WHEELS_RELOAD_PASSWORD# substitution in lucee.json

The existing e2e spec at cli/tests/specs/e2e/ServerCommandsTest.cfc verifies detectReloadPassword() and password detection from .env / settings.cfm. There is no test that scaffolds an app (or uses a fixture lucee.json containing the interpolation syntax), starts the server, and confirms the Lucee admin panel is accessible with the value from .env.

This is a higher-effort integration gap and acceptable to defer. The onboarding harness (tools/test-onboarding.sh) would naturally exercise this end-to-end, so it may already be covered implicitly -- worth noting in the PR body if so.


Minor observations (no change required)

cli/tests/specs/e2e/ServerCommandsTest.cfc:919 and cli/tests/specs/commands/ConfigCommandsTest.cfc:190 still write the legacy RELOAD_PASSWORD key in test fixtures. These were flagged as non-blocking in round 2 -- the backwards-compat regex covers them. Worth a follow-up pass once this lands.

The docker-deployment.mdx Dockerfile example still has WHEELS_RELOAD_PASSWORD=change-me baked into the ENV directive. The compose file's required-substitution form is the authoritative pattern; the Dockerfile default is a documented placeholder, not introduced by this PR.


Everything from the previous three review rounds is addressed. The core security fix -- no committed reload-password literal in scaffold templates, starter-app, or (now) lucee.json -- is complete. The CHANGELOG entry, backwards-compat detection regex, onboarding harness fix, and guide sweep are all correct. The two open items above are non-blocking.

@bpamiri

bpamiri commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks @wheels-bot — both non-blocking points confirmed, no code change needed.

Fallback when WHEELS_RELOAD_PASSWORD is unset: LuCLI leaves the literal #env:WHEELS_RELOAD_PASSWORD# as the admin password — it does not substitute empty. In LuceeServerConfig.replaceLucliVars(), the no-default branch is:

replacement = getEnvVar(varExpression);
if (replacement == null) {
    replacement = matcher.group(0); // keep the literal "#env:WHEELS_RELOAD_PASSWORD#"
}

getEnvVar() checks the loaded .env first, then the system environment; if absent from both, the placeholder token is kept verbatim. So the admin password is never left blank/open — it falls back to a non-empty literal. On the wheels new happy path .env always carries WHEELS_RELOAD_PASSWORD=<random>, so it resolves to the random value. (We could switch to the #env:VAR:-default# form, but an empty default would be worse — passwordless admin — so the literal fallback is the safer default. Your second hypothesized outcome is the actual one.)

Test coverage: yes, exercised implicitly by tools/test-onboarding.sh — that harness runs wheels start, which is exactly where LuCLI loads lucee.json and resolves admin.password through this #env: path against the generated .env. A dedicated unit test for the interpolation itself belongs in the LuCLI repo (where the resolver lives), not here.

The two test-fixture nits (ServerCommandsTest.cfc:919, ConfigCommandsTest.cfc:190) and the docker ENV placeholder are unchanged from prior rounds — non-blocking, follow-up.

@wheels-bot

wheels-bot Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Wheels Bot — Reviewer B (round 1)

A's review is accurate and cites real concerns, but both are explicitly characterised as non-blocking. The template processing mechanism is safe (verified from source), and the author's prior PR comment already supplies the mechanism detail A marks as unverified from the diff. The COMMENT verdict is defensible but too conservative; B would approve. Not aligned on verdict — flagging for A to reconsider.

Sycophancy

None detected. A deliberately chose COMMENTED rather than approve, and every observation is backed by a specific file reference or diff assertion.

False positives

None detected.

  • Unverified fallback when WHEELS_RELOAD_PASSWORD is absent — A correctly says the diff alone does not confirm LuCLI's LuceeServerConfig.substituteField behavior. That class is external to this repo, so the uncertainty is real. However, the scenario is even narrower than A frames it: cli/lucli/templates/app/lucee.json is a scaffold template, never committed by a user directly. It is only materialised by wheels new, which always generates .env with WHEELS_RELOAD_PASSWORD=<random> in the same run. A bare-clone-without-scaffolding cannot happen with this file; the only way a generated app's lucee.json ever contains the interpolation string is after wheels new has already written the key to .env. A is not wrong to raise it, but "risk is narrow" understates it further.
  • Could the #env:WHEELS_RELOAD_PASSWORD# string be CFML-evaluated during template processing? A does not raise this, but I checked it proactively. copyTemplateDir() reads each template file with fileRead(sourcePath), passes the raw string to processPlaceholders(), which calls replace() on it and then fileWrite(). CFML does not evaluate # expressions in strings held in variables or passed as function arguments — only in directly-interpolated string literals or cfoutput blocks. The interpolation string is written verbatim to the generated lucee.json. Not an issue. (Flagging here to close the loop, not a fault of A's review.)

Missed issues

None that rise to blocking.

  • CHANGELOG update — the latest commit amends the existing [Unreleased] CHANGELOG entry to include the lucee.json fix (CHANGELOG.md | 2 +- in the commit stat). A does not call this out explicitly, but it was handled correctly and is consistent with the prior-round approach. No action needed.

Verdict alignment

A's COMMENTED verdict is internally consistent — choosing COMMENTED over approve when you have open questions you want the author to confirm before merge is a reasonable stance. The inconsistency is that A labels both concerns "non-blocking" in the same comment. If neither concern blocks merge, COMMENTED (which holds the loop in a non-converged state) is harder to justify than approve.

Convergence

Not aligned. A said COMMENTED; B would approve. A's own characterisation that "neither concern is blocking" is the crux: non-blocking observations are documentation improvements, not merge gates. The author (who maintains LuCLI) explained the mechanism in their PR comment before this review ran, and the template-processing path is verified safe. A should reconsider whether COMMENTED is warranted when the two open items are (a) a PR-body note about a corner case the normal flow cannot hit, and (b) an acknowledged higher-effort test gap that A itself calls "acceptable to defer." If A agrees both are below the approve threshold, this PR is review-clean.

@bpamiri
bpamiri merged commit d4ce24e into develop Jun 4, 2026
14 checks passed
@bpamiri
bpamiri deleted the peter/scaffold-reload-password-env branch June 4, 2026 18:21

@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 A response (round 1)

B is correct on the verdict-alignment point. Both items I raised in my round-4 review were explicitly labelled "non-blocking" and "acceptable to defer" in my own text — choosing COMMENT rather than approve was internally inconsistent. B's verification and the author's inline PR comment have also resolved the factual uncertainty I cited as my reason for not approving outright. Conceding the verdict and updating to approve.

Conceded points

  • COMMENT verdict was inconsistent with my own finding characterisation. I marked both the fallback-behavior question and the test-coverage gap as "non-blocking" in the same review body, then posted COMMENT. B correctly identifies the contradiction: if neither concern gates merge, COMMENT holds the loop in a non-converged state without justification.
  • Fallback behavior was already documented, not an open question. bpamiri's PR comment (posted before my review ran) explained the exact LuceeServerConfig.replaceLucliVars() no-default branch: the literal #env:WHEELS_RELOAD_PASSWORD# is kept as-is when the env var is absent — not empty string. That was the information I said the diff did not confirm, and it was already present in the thread.
  • Template-processing risk is resolved. B's proactive verification of copyTemplateDir() → processPlaceholders() → replace() / fileWrite() confirms CFML does not evaluate #-delimited expressions in strings held in variables or written via fileWrite. The interpolation string is written verbatim to the generated lucee.json.

Defended points

None. Both concerns in my COMMENT-state review were accurately characterised as non-blocking by me at the time. B's evidence closes the remaining factual uncertainty and demonstrates the verdict should have been approve.

Updated verdict

Approve. Every blocking finding across four rounds is resolved, both non-blocking nits are explicitly acknowledged as follow-up candidates, and the two concerns behind my COMMENT verdict are resolved — one by B's independent verification and one by the author's already-present PR-body explanation. This PR is review-clean.

bpamiri added a commit that referenced this pull request Jun 4, 2026
Replace the remaining `application.wo.env(...)` calls in four v4-0-0
guides with the bare `env(...)` form, in both `{test:compile}` code
fences and inline-code prose. Bare `env()` is the same public function
(vendor/wheels/Global.cfc:597) and is already the canonical, shipping
pattern in the migration guide (upgrading/3x-to-4x.mdx) under
{test:compile}.

Accessor-style only — no env-var names changed (DB_USER, SMTP_PASSWORD,
JWT_SECRET, POSTMARK_TOKEN, WHEELS_DATASOURCE, etc. all preserved).

Deferred follow-up from #2857, which converted the scaffold plus the
deployment/ and core-concepts env() guides; these four files were the
remainder it did not touch.

Files:
- basics/database-and-multiple-datasources.mdx
- digging-deeper/sending-email.mdx
- digging-deeper/authentication-patterns.mdx
- command-line-tools/wheels-commands/testing.mdx

verify:docs (4 touched files): 35 passed, 0 failed.

Signed-off-by: Peter Amiri <peter@alurium.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
bpamiri added a commit that referenced this pull request Jun 5, 2026
…-argument

Resolve CHANGELOG.md [Unreleased]/Fixed conflict by keeping both the --no-* flag fix entry (#2855) and develop's #2835/#2840/#2857 entries. cli/lucli/Module.cfc auto-merged cleanly — the PR's argsFromCollection one-liner sits in a different region than develop's CLI changes.

Signed-off-by: Peter Amiri <peter@alurium.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants