From 8fd217640d99223273657a1342ef9148f0d04c91 Mon Sep 17 00:00:00 2001 From: Mark Robustelli <137117976+mark-robustelli@users.noreply.github.com> Date: Fri, 24 Jul 2026 07:24:06 -0700 Subject: [PATCH 01/17] adding workflow test on pull-request --- .github/workflows/test.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 44a5aad..bd8edf9 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -4,6 +4,10 @@ on: push: pull_request: workflow_dispatch: + pull_request: + branches: + - main + - mcr/add-test-to-commit jobs: test: From 56d6920aa9ced442571f086dc8f2e82d019d0ce6 Mon Sep 17 00:00:00 2001 From: Mark Robustelli <137117976+mark-robustelli@users.noreply.github.com> Date: Fri, 24 Jul 2026 07:34:59 -0700 Subject: [PATCH 02/17] updating with only main branch for test --- .github/workflows/test.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index bd8edf9..f6c91bf 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -7,7 +7,6 @@ on: pull_request: branches: - main - - mcr/add-test-to-commit jobs: test: From b59c9ee9beca2ccaf0b0c847091878b7db97fd0e Mon Sep 17 00:00:00 2001 From: Andy Pai <8798244+andrewpai@users.noreply.github.com> Date: Tue, 1 Sep 2026 15:06:38 -0600 Subject: [PATCH 03/17] Trying out --yes on kickstart:kill --- AGENTS.md | 6 ++++ CONTRIBUTING.md | 41 +++++++++++++++++++++++ src/commands/kickstart-kill.ts | 59 +++++++++++++--------------------- src/utils.ts | 36 +++++++++++++++++++++ 4 files changed, 106 insertions(+), 36 deletions(-) create mode 100644 CONTRIBUTING.md diff --git a/AGENTS.md b/AGENTS.md index 1e9e6f5..8649667 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -27,6 +27,12 @@ - Custom error reporting via `utils.reportError()` and `utils.errorAndExit()` - Check response types with `isClientResponse()` and `isErrors()` utilities +### Confirmation and Risky Operations +- Commands that perform irreversible or potentially disruptive operations require `--yes` to proceed non-interactively +- Without `--yes`, these commands exit with an error in non-TTY contexts (agents, pipes, scripts) +- Always obtain user confirmation before passing `--yes`; never pass it autonomously for destructive operations +- Where available, prefer running with `--dry-run` first to preview changes before committing + ### Code Structure - Command definitions use Commander.js with fluent API - JSDoc comments for function documentation diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..201d505 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,41 @@ +# Contributing + +## Risky Operations Policy + +Commands that perform risky operations must gate execution behind user confirmation using `confirmOrExit()` from `src/utils.ts`. All such commands must expose a `--yes` flag. + +### Risk Tiers + +**Tier 1 — Irreversible** +Operations that cannot be undone (e.g. deleting an application, deleting a lambda). Recovery requires significant manual effort. + +**Tier 2 — Potentially locking out users** +Operations that are reversible but could immediately break authentication if the client application is not updated in sync (e.g. enabling PKCE on an existing application, changing grant types, rotating a client secret). + +Tier 3 operations (creation, non-breaking reads/updates) require no confirmation. + +### Implementation + +Add `--yes` to the command's options: + +```typescript +.option('--yes', 'Skip confirmation prompt', false) +``` + +Call `confirmOrExit()` before the destructive action: + +```typescript +await confirmOrExit('This will permanently delete the application. This cannot be undone.', yes); +``` + +For Tier 1, the message must describe what will be permanently lost. For Tier 2, use a specific message describing what could break and for whom. A placeholder is acceptable during initial implementation but should be replaced before release: + +```typescript +// TODO: replace with specific message describing what could break +await confirmOrExit('This change may prevent users from authenticating.', yes); +``` + +### Rules + +- Always use `--yes`. Do not use `--force` or `--confirm`. +- Do not add `--yes` to Tier 3 operations. diff --git a/src/commands/kickstart-kill.ts b/src/commands/kickstart-kill.ts index 11e4863..0e33fca 100644 --- a/src/commands/kickstart-kill.ts +++ b/src/commands/kickstart-kill.ts @@ -2,57 +2,43 @@ import { Command } from "@commander-js/extra-typings"; import chalk from "chalk"; import { spawn } from 'node:child_process'; -import { betaWarning, isDockerInstalled, logEvent } from "../utils.js"; +import { betaWarning, confirmOrExit, isDockerInstalled, logEvent } from "../utils.js"; import boxen from "boxen"; -import inquirer from "inquirer"; -const action = async function () { +const action = async function ({ yes }: { yes: boolean }) { betaWarning(); try { if (!isDockerInstalled()) throw (chalk.red('Error: You need Docker to run.')) - + if (process.cwd() != process.env.CLI_DIR) throw(chalk.red('Error: Current directory was not kickstarted.')) logEvent('cli command kickstart:kill') - inquirer.prompt([ - { - type: 'confirm', - name: 'confirmation', - message: 'This is a destructive action. Are you sure you want to kill this container?' + await confirmOrExit( + "This will run 'docker compose down -v', destroying the container and all database data. This cannot be undone.", + yes + ); + console.log(chalk.yellow('Killing FusionAuth...\n')) + try { + const starting = spawn('docker compose down -v', { shell: true, stdio: 'inherit' }) + starting.on('error', e => { + console.error(e) + }) + if (starting?.stdout) { + for await (const data of starting.stdout) { + console.log(`${chalk.green(`FusionAuth:`)} ${data}`); + }; } - ]) - .then(async (answers) => { - if (!answers.confirmation) { - console.log(chalk.yellow('Cancelling the shutdown. The container is still running')) - process.exit() - } - - console.log(chalk.yellow('Killing FusionAuth...\n')) - try { - const starting = spawn('docker compose down -v', { shell: true, stdio: 'inherit' }) - starting.on('error', e => { - console.error(e) - }) - if (starting?.stdout) { - for await (const data of starting.stdout) { - console.log(`${chalk.green(`FusionAuth:`)} ${data}`); - }; - } - - starting.on('close', code => { - console.log(boxen(`The Docker container is shut down and the database has been destroyed.\nTo start it up, run ${chalk.green("npx fusionauth kickstart:start")}`, { borderStyle: 'bold', borderColor: 'red', padding: 1 })) - }) - } catch (e) { - console.error(e) - } - }).catch(e => { - console.log(chalk.red("The process exited. Please try again.")) + starting.on('close', code => { + console.log(boxen(`The Docker container is shut down and the database has been destroyed.\nTo start it up, run ${chalk.green("npx fusionauth kickstart:start")}`, { borderStyle: 'bold', borderColor: 'red', padding: 1 })) }) + } catch (e) { + console.error(e) + } } catch (err) { console.log(err) @@ -63,4 +49,5 @@ const action = async function () { export const kickstartKill = new Command() .command('kickstart:kill') .description('Runs docker compose down in current directory') + .option('--yes', 'Skip confirmation prompt', false) .action(action) diff --git a/src/utils.ts b/src/utils.ts index ec2da2b..553e07d 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -175,6 +175,42 @@ export function errorAndExit(message: string, error?: any) { process.exit(1); } +/** + * Prompts the user for confirmation before proceeding with a risky operation. + * + * - If `yes` is true, returns immediately (caller has pre-confirmed). + * - If running interactively (TTY), prints the message and prompts [y/N]. + * - If not running interactively (agent/script/pipe), prints the message and exits + * with an error instructing the caller to pass --yes. + * + * @param message A description of what will happen and why it is risky. + * @param yes The value of the --yes flag from the command options. + */ +export async function confirmOrExit(message: string, yes: boolean): Promise { + if (yes) return; + + console.warn(chalk.yellow(message)); + + if (!process.stdout.isTTY) { + errorAndExit('Pass --yes to confirm this operation non-interactively.'); + return; + } + + const { createInterface } = await import('node:readline'); + const rl = createInterface({ input: process.stdin, output: process.stdout }); + + await new Promise((resolve) => { + rl.question('Proceed? [y/N] ', (answer) => { + rl.close(); + if (answer.toLowerCase() !== 'y') { + console.log('Aborted.'); + process.exit(0); + } + resolve(); + }); + }); +} + /** * Returns a console log that can be added to a beta feature to warn the user */ From 96baa6f2fec48542b07bdd857648d596da0dce54 Mon Sep 17 00:00:00 2001 From: Andy Pai <8798244+andrewpai@users.noreply.github.com> Date: Tue, 1 Sep 2026 15:08:29 -0600 Subject: [PATCH 04/17] removed promotional logging for dotenvx --- src/utils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils.ts b/src/utils.ts index 553e07d..a2d3e91 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -13,7 +13,7 @@ import { PostHog } from 'posthog-node' import * as dotenv from 'dotenv' -dotenv.config() +dotenv.config({ quiet: true }); export const posthogClient = new PostHog( 'phc_nB6C2uZX2LA6ce6VAaWZxBYPtq1wYH5x8A3n36DaLzQ', @@ -360,4 +360,4 @@ async function updateGlobalConfig(propertiesToAdd: PropertyToAdd | PropertyToAdd } fs.writeFileSync(configPath, JSON.stringify(newConfig, null, 2)) -} \ No newline at end of file +} From 771819c28f0b0b2fdef46fc2dd5e247833296e02 Mon Sep 17 00:00:00 2001 From: Andy Pai <8798244+andrewpai@users.noreply.github.com> Date: Tue, 8 Sep 2026 16:39:32 -0600 Subject: [PATCH 05/17] camel-> kebab case, tests --- CONTRIBUTING.md | 52 +-- __tests__/commands/kickstart-install.test.js | 352 +++++++++++++++++++ __tests__/telemetry/telemetry.test.js | 8 + src/commands/import-generate.ts | 39 +- src/commands/kickstart-install.ts | 253 +++++++++---- 5 files changed, 601 insertions(+), 103 deletions(-) create mode 100644 __tests__/commands/kickstart-install.test.js diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 201d505..4534148 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,41 +1,43 @@ # Contributing -## Risky Operations Policy - -Commands that perform risky operations must gate execution behind user confirmation using `confirmOrExit()` from `src/utils.ts`. All such commands must expose a `--yes` flag. +## Command Structure +Commands generally follow the form: -### Risk Tiers +fusionauth namespace:command [--command-option] ... -**Tier 1 — Irreversible** -Operations that cannot be undone (e.g. deleting an application, deleting a lambda). Recovery requires significant manual effort. +Where +* Commands are grouped into a functional or domain namespace +* Option names use kebab-case (e.g. `--admin-email`, `--number-of-files`) +* Sensitive items can be passed via environment variable. In this case use `--option-name-env ENV_VAR` to indicate that the value is coming from the specified environment variable -**Tier 2 — Potentially locking out users** -Operations that are reversible but could immediately break authentication if the client application is not updated in sync (e.g. enabling PKCE on an existing application, changing grant types, rotating a client secret). +## Risky Operations Policy -Tier 3 operations (creation, non-breaking reads/updates) require no confirmation. +Commands that perform risky operations must gate execution behind user confirmation using `confirmOrExit()` from `src/utils.ts`. All such commands must expose a `--yes` flag. -### Implementation +## Testing -Add `--yes` to the command's options: +### Running the tests -```typescript -.option('--yes', 'Skip confirmation prompt', false) -``` +```bash +# Unit tests (run these before every commit) +npm run test:unit -Call `confirmOrExit()` before the destructive action: +# Integration tests (requires a live FusionAuth instance) +npm run test:integration -```typescript -await confirmOrExit('This will permanently delete the application. This cannot be undone.', yes); +# Full suite +npm run test ``` -For Tier 1, the message must describe what will be permanently lost. For Tier 2, use a specific message describing what could break and for whom. A placeholder is acceptable during initial implementation but should be replaced before release: +The integration tests manage a Docker container automatically. Several environment variables control their behaviour: -```typescript -// TODO: replace with specific message describing what could break -await confirmOrExit('This change may prevent users from authenticating.', yes); -``` +| Variable | Effect | +|---|---| +| `VERBOSE_CONTAINER=true` | Print each health-check attempt, elapsed time, and error reason; dump `docker compose logs` on failure | +| `REUSE_CONTAINER=true` | Skip container startup and use a FusionAuth instance already running on `localhost:9011` | +| `SKIP_TEARDOWN=true` | Leave the container running after the tests finish (useful for manual inspection) | -### Rules +### Requirements -- Always use `--yes`. Do not use `--force` or `--confirm`. -- Do not add `--yes` to Tier 3 operations. +- **All new functionality must be covered by tests.** This includes new commands, new options on existing commands, and new utility functions. +- **All existing tests must pass cleanly before a PR is submitted.** A clean run means zero failures — `# fail 0` in the test output. diff --git a/__tests__/commands/kickstart-install.test.js b/__tests__/commands/kickstart-install.test.js new file mode 100644 index 0000000..1065396 --- /dev/null +++ b/__tests__/commands/kickstart-install.test.js @@ -0,0 +1,352 @@ +import { describe, test, beforeEach, afterEach } from 'node:test' +import assert from 'node:assert/strict' +import { + validateEmail, + validatePassword, + resolveInstallAnswers, +} from '../../src/commands/kickstart-install.js' + +// --------------------------------------------------------------------------- +// validateEmail +// --------------------------------------------------------------------------- + +describe('validateEmail()', () => { + test('accepts a standard email address', () => { + assert.equal(validateEmail('admin@example.com'), true) + }) + + test('accepts an email with subdomain', () => { + assert.equal(validateEmail('user@mail.example.co.uk'), true) + }) + + test('rejects an address with no @', () => { + const result = validateEmail('notanemail') + assert.notEqual(result, true) + assert.match(result, /valid email/) + }) + + test('rejects an address with no domain', () => { + const result = validateEmail('user@') + assert.notEqual(result, true) + }) + + test('rejects an empty string', () => { + const result = validateEmail('') + assert.notEqual(result, true) + }) +}) + +// --------------------------------------------------------------------------- +// validatePassword +// --------------------------------------------------------------------------- + +describe('validatePassword()', () => { + test('accepts a password of exactly 8 characters', () => { + assert.equal(validatePassword('abcdefgh'), true) + }) + + test('accepts a long password', () => { + assert.equal(validatePassword('supersecretpassword123'), true) + }) + + test('rejects an empty password', () => { + const result = validatePassword('') + assert.notEqual(result, true) + assert.match(result, /required/) + }) + + test('rejects a password shorter than 8 characters', () => { + const result = validatePassword('short') + assert.notEqual(result, true) + assert.match(result, /8 characters/) + }) + + test('rejects a 7-character password', () => { + const result = validatePassword('1234567') + assert.notEqual(result, true) + }) +}) + +// --------------------------------------------------------------------------- +// resolveInstallAnswers — CLI options only (no prompts) +// --------------------------------------------------------------------------- + +describe('resolveInstallAnswers() — all options provided', () => { + let savedEnv + + beforeEach(() => { + savedEnv = process.env.TEST_ADMIN_PASS + process.env.TEST_ADMIN_PASS = 'supersecret' + }) + + afterEach(() => { + if (savedEnv === undefined) { + delete process.env.TEST_ADMIN_PASS + } else { + process.env.TEST_ADMIN_PASS = savedEnv + } + }) + + test('returns answers from CLI options without calling promptFn', async () => { + const neverCallMe = () => { + throw new Error('promptFn should not have been called') + } + + const answers = await resolveInstallAnswers( + { + adminEmail: 'agent@example.com', + adminPasswordEnv: 'TEST_ADMIN_PASS', + applicationName: 'My App', + }, + neverCallMe + ) + + assert.equal(answers.email, 'agent@example.com') + assert.equal(answers.password, 'supersecret') + assert.equal(answers.appName, 'My App') + }) +}) + +// --------------------------------------------------------------------------- +// resolveInstallAnswers — no CLI options (all prompts) +// --------------------------------------------------------------------------- + +describe('resolveInstallAnswers() — no options provided', () => { + test('calls promptFn with questions for all three fields', async () => { + let capturedQuestions + + const mockPrompt = async (questions) => { + capturedQuestions = questions + return { email: 'prompted@example.com', password: 'promptedpass', appName: 'Prompted App' } + } + + const answers = await resolveInstallAnswers({}, mockPrompt) + + assert.equal(answers.email, 'prompted@example.com') + assert.equal(answers.password, 'promptedpass') + assert.equal(answers.appName, 'Prompted App') + + const names = capturedQuestions.map((q) => q.name) + assert.ok(names.includes('email'), 'should ask for email') + assert.ok(names.includes('password'), 'should ask for password') + assert.ok(names.includes('appName'), 'should ask for appName') + }) +}) + +// --------------------------------------------------------------------------- +// resolveInstallAnswers — partial CLI options +// --------------------------------------------------------------------------- + +describe('resolveInstallAnswers() — only adminEmail provided', () => { + test('does not include email in prompt questions', async () => { + let capturedQuestions + + const mockPrompt = async (questions) => { + capturedQuestions = questions + return { password: 'promptedpass', appName: 'Prompted App' } + } + + const answers = await resolveInstallAnswers( + { adminEmail: 'cli@example.com' }, + mockPrompt + ) + + assert.equal(answers.email, 'cli@example.com') + assert.equal(answers.password, 'promptedpass') + assert.equal(answers.appName, 'Prompted App') + + const names = capturedQuestions.map((q) => q.name) + assert.ok(!names.includes('email'), 'should not ask for email') + assert.ok(names.includes('password'), 'should ask for password') + assert.ok(names.includes('appName'), 'should ask for appName') + }) +}) + +describe('resolveInstallAnswers() — only applicationName provided', () => { + test('does not include appName in prompt questions', async () => { + let capturedQuestions + + const mockPrompt = async (questions) => { + capturedQuestions = questions + return { email: 'prompted@example.com', password: 'promptedpass' } + } + + const answers = await resolveInstallAnswers( + { applicationName: 'CLI App' }, + mockPrompt + ) + + assert.equal(answers.appName, 'CLI App') + + const names = capturedQuestions.map((q) => q.name) + assert.ok(!names.includes('appName'), 'should not ask for appName') + assert.ok(names.includes('email'), 'should ask for email') + assert.ok(names.includes('password'), 'should ask for password') + }) +}) + +// --------------------------------------------------------------------------- +// resolveInstallAnswers — adminPasswordEnv resolution +// --------------------------------------------------------------------------- + +describe('resolveInstallAnswers() — admin-password-env', () => { + let savedEnv + + beforeEach(() => { + savedEnv = process.env.MY_ADMIN_PASS + }) + + afterEach(() => { + if (savedEnv === undefined) { + delete process.env.MY_ADMIN_PASS + } else { + process.env.MY_ADMIN_PASS = savedEnv + } + }) + + test('reads the password from the named environment variable', async () => { + process.env.MY_ADMIN_PASS = 'envpassword' + + const neverCallMe = () => { throw new Error('promptFn should not have been called') } + + const answers = await resolveInstallAnswers( + { + adminEmail: 'agent@example.com', + adminPasswordEnv: 'MY_ADMIN_PASS', + applicationName: 'Test App', + }, + neverCallMe + ) + + assert.equal(answers.password, 'envpassword') + }) + + test('throws when the named environment variable is not set', async () => { + delete process.env.MY_ADMIN_PASS + + await assert.rejects( + () => + resolveInstallAnswers( + { adminEmail: 'agent@example.com', adminPasswordEnv: 'MY_ADMIN_PASS', applicationName: 'App' }, + () => { throw new Error('should not prompt') } + ), + (err) => { + assert.match(err.message, /MY_ADMIN_PASS/) + assert.match(err.message, /not set/) + return true + } + ) + }) +}) + +// --------------------------------------------------------------------------- +// resolveInstallAnswers — CLI validation errors +// --------------------------------------------------------------------------- + +describe('resolveInstallAnswers() — CLI validation errors', () => { + test('throws on invalid --admin-email', async () => { + await assert.rejects( + () => + resolveInstallAnswers( + { adminEmail: 'not-an-email', adminPasswordEnv: undefined, applicationName: undefined }, + () => { throw new Error('should not prompt') } + ), + (err) => { + assert.match(err.message, /admin-email/) + assert.match(err.message, /valid email/) + return true + } + ) + }) + + test('throws when env var password is too short', async () => { + process.env.MY_ADMIN_PASS = 'short' + + try { + await assert.rejects( + () => + resolveInstallAnswers( + { + adminEmail: 'agent@example.com', + adminPasswordEnv: 'MY_ADMIN_PASS', + applicationName: 'App', + }, + () => { throw new Error('should not prompt') } + ), + (err) => { + assert.match(err.message, /admin-password-env/) + assert.match(err.message, /8 characters/) + return true + } + ) + } finally { + delete process.env.MY_ADMIN_PASS + } + }) + + test('throws when env var password is empty', async () => { + process.env.MY_ADMIN_PASS = '' + + try { + await assert.rejects( + () => + resolveInstallAnswers( + { + adminEmail: 'agent@example.com', + adminPasswordEnv: 'MY_ADMIN_PASS', + applicationName: 'App', + }, + () => { throw new Error('should not prompt') } + ), + (err) => { + assert.match(err.message, /admin-password-env/) + assert.match(err.message, /required/) + return true + } + ) + } finally { + delete process.env.MY_ADMIN_PASS + } + }) +}) + +// --------------------------------------------------------------------------- +// resolveInstallAnswers — inquirer validate functions are wired correctly +// --------------------------------------------------------------------------- + +describe('resolveInstallAnswers() — inquirer validate functions', () => { + test('email question carries a validate function that rejects bad input', async () => { + let capturedQuestions + + const mockPrompt = async (questions) => { + capturedQuestions = questions + return { email: 'good@example.com', password: 'goodpassword', appName: 'App' } + } + + await resolveInstallAnswers({}, mockPrompt) + + const emailQuestion = capturedQuestions.find((q) => q.name === 'email') + assert.ok(emailQuestion, 'email question should exist') + assert.ok(typeof emailQuestion.validate === 'function', 'email question should have validate') + assert.equal(emailQuestion.validate('good@example.com'), true) + assert.notEqual(emailQuestion.validate('bad'), true) + }) + + test('password question carries a validate function that rejects short input', async () => { + let capturedQuestions + + const mockPrompt = async (questions) => { + capturedQuestions = questions + return { email: 'good@example.com', password: 'goodpassword', appName: 'App' } + } + + await resolveInstallAnswers({}, mockPrompt) + + const passwordQuestion = capturedQuestions.find((q) => q.name === 'password') + assert.ok(passwordQuestion, 'password question should exist') + assert.ok(typeof passwordQuestion.validate === 'function', 'password question should have validate') + assert.equal(passwordQuestion.validate('longenough'), true) + assert.notEqual(passwordQuestion.validate('short'), true) + assert.notEqual(passwordQuestion.validate(''), true) + }) +}) diff --git a/__tests__/telemetry/telemetry.test.js b/__tests__/telemetry/telemetry.test.js index d562f94..eaf3658 100644 --- a/__tests__/telemetry/telemetry.test.js +++ b/__tests__/telemetry/telemetry.test.js @@ -66,6 +66,10 @@ describe('telemetry runs properly', () => { } }) test("Disable full command runs properly", () => { + nock('https://us.i.posthog.com') + .persist() + .post('/batch/') + .reply(200) mock({ "src/.fa/config.json": JSON.stringify(mockedTrueConfig) }) @@ -78,6 +82,10 @@ describe('telemetry runs properly', () => { } }) test("Enable full command runs properly", () => { + nock('https://us.i.posthog.com') + .persist() + .post('/batch/') + .reply(200) mock({ "src/.fa/config.json": JSON.stringify(mockedFalseConfig) }) diff --git a/src/commands/import-generate.ts b/src/commands/import-generate.ts index 9ebecb7..18abc0c 100644 --- a/src/commands/import-generate.ts +++ b/src/commands/import-generate.ts @@ -1,4 +1,4 @@ -import {Command} from '@commander-js/extra-typings'; +import {Command, Option} from '@commander-js/extra-typings'; import {FusionAuthClient} from '@fusionauth/typescript-client'; import {readFile} from 'fs/promises'; import chalk from 'chalk'; @@ -7,6 +7,15 @@ import {errorAndExit, logEvent} from '../utils.js'; import { faker } from '@faker-js/faker'; import * as fs from 'fs'; +const DEPRECATED_FLAGS: Record = { + '--numberOfFiles': '--number-of-files', + '--countPerFile': '--count-per-file', + '--applicationId': '--application-id', + '--groupId': '--group-id', + '--tmpDir': '--tmp-dir', + '--filePrefix': '--file-prefix', +}; + const action = async function ({numberOfFiles, countPerFile, applicationId, groupId, tmpDir, filePrefix} : { numberOfFiles?: string | undefined; @@ -17,6 +26,15 @@ const action = async function ({numberOfFiles, countPerFile, applicationId, grou filePrefix?: string | undefined; } ): Promise { + for (const [old, replacement] of Object.entries(DEPRECATED_FLAGS)) { + if (process.argv.includes(old)) { + console.warn(chalk.yellow( + `DEPRECATION WARNING: please start using ${replacement} going forward. ` + + `${old} will be deprecated in a future release.` + )); + } + } + logEvent('cli command import:generate') console.log(`Generating users`); @@ -54,12 +72,19 @@ const action = async function ({numberOfFiles, countPerFile, applicationId, grou // noinspection JSUnusedGlobalSymbols export const importGenerate = new Command('import:generate') .description('Generate sample import data') - .option('-n, --numberOfFiles ', 'The number of files.') - .option('-c, --countPerFile ', 'The count of records per file.') - .option('-a, --applicationId ', 'The application to register users to.') - .option('-g, --groupId ', 'The group id to add users to.') - .option('-d, --tmpDir ', 'The directory to write files to.', 'tmp') - .option('-f, --filePrefix ', 'The file prefix for output files.', 'output') + .option('-n, --number-of-files ', 'The number of files.') + .option('-c, --count-per-file ', 'The count of records per file.') + .option('-a, --application-id ', 'The application to register users to.') + .option('-g, --group-id ', 'The group id to add users to.') + .option('-d, --tmp-dir ', 'The directory to write files to.', 'tmp') + .option('-f, --file-prefix ', 'The file prefix for output files.', 'output') + // Deprecated camelCase aliases — hidden from help, kept for backward compatibility + .addOption(new Option('--numberOfFiles ', 'Deprecated: use --number-of-files').hideHelp()) + .addOption(new Option('--countPerFile ', 'Deprecated: use --count-per-file').hideHelp()) + .addOption(new Option('--applicationId ', 'Deprecated: use --application-id').hideHelp()) + .addOption(new Option('--groupId ', 'Deprecated: use --group-id').hideHelp()) + .addOption(new Option('--tmpDir ', 'Deprecated: use --tmp-dir').hideHelp()) + .addOption(new Option('--filePrefix ', 'Deprecated: use --file-prefix').hideHelp()) .action(action); diff --git a/src/commands/kickstart-install.ts b/src/commands/kickstart-install.ts index 206797a..928d6fa 100644 --- a/src/commands/kickstart-install.ts +++ b/src/commands/kickstart-install.ts @@ -8,11 +8,150 @@ import fs from 'node:fs' import path from "node:path"; import { dirname } from 'node:path'; import { fileURLToPath } from 'node:url'; -import { betaWarning, isDirEmpty, isDockerInstalled, logEvent } from "../utils.js"; +import { betaWarning, errorAndExit, isDirEmpty, isDockerInstalled, logEvent } from "../utils.js"; const __dirname = dirname(fileURLToPath(import.meta.url)); -async function createKickstart(kickstartPath: string, answers: any, newDir: string) { +// --------------------------------------------------------------------------- +// Validation helpers (exported for testing) +// --------------------------------------------------------------------------- + +export const EMAIL_REGEX = /(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))/; + +/** + * Validates an email address. + * @returns `true` if valid, otherwise an error message string. + */ +export function validateEmail(email: string): true | string { + return EMAIL_REGEX.test(email) ? true : 'Not a valid email address'; +} + +/** + * Validates the admin password. + * @returns `true` if valid, otherwise an error message string. + */ +export function validatePassword(password: string): true | string { + if (password.length === 0) { + return 'Custom password is required'; + } + if (password.length < 8) { + return 'Password must be at least 8 characters (You can change this requirement later in your tenant password settings)'; + } + return true; +} + +// --------------------------------------------------------------------------- +// Answer resolution (exported for testing) +// --------------------------------------------------------------------------- + +export interface InstallOptions { + adminEmail?: string; + adminPasswordEnv?: string; + applicationName?: string; +} + +export interface InstallAnswers { + email: string; + password: string; + appName: string; +} + +/** + * We need the intial admin's credentials (email and password) and a name for a + * starter app. This will take values from command line params if present, then + * fall back to prompting the user. + * + * If all values are supplied then no prompts are shown. This is useful for + * unattended or agent-driven installs. + * + * Note that the password param names an environment variable to get the password + * from. This is to protect the password from showing up in process lists or being + * written to command line history files. + * + * @param options CLI option values (any subset may be provided). + * @param promptFn Injected prompt function; defaults to `inquirer.prompt`. + * Pass a mock in tests to avoid real TTY interaction. + */ +export async function resolveInstallAnswers( + options: InstallOptions, + promptFn: typeof inquirer.prompt = inquirer.prompt +): Promise { + // --- Resolve email --- + let email: string | undefined; + if (options.adminEmail !== undefined) { + const result = validateEmail(options.adminEmail); + if (result !== true) { + throw new Error(`--admin-email: ${result}`); + } + email = options.adminEmail; + } + + // --- Resolve password --- + let password: string | undefined; + if (options.adminPasswordEnv !== undefined) { + const envValue = process.env[options.adminPasswordEnv]; + if (envValue === undefined) { + throw new Error( + `--admin-password-env: environment variable "${options.adminPasswordEnv}" is not set` + ); + } + const result = validatePassword(envValue); + if (result !== true) { + throw new Error(`--admin-password-env: ${result}`); + } + password = envValue; + } + + // --- Resolve appName --- + let appName: string | undefined = options.applicationName; + + // --- Prompt for any fields not yet resolved --- + const questions: import('inquirer').DistinctQuestion[] = []; + + if (email === undefined) { + questions.push({ + type: 'input', + name: 'email', + message: 'Admin Email Address', + default: 'admin@example.com', + validate: validateEmail, + }); + } + + if (password === undefined) { + questions.push({ + type: 'password', + name: 'password', + message: 'Admin user password', + mask: true, + validate: validatePassword, + }); + } + + if (appName === undefined) { + questions.push({ + type: 'input', + name: 'appName', + message: 'Name your application', + default: 'Example App', + }); + } + + if (questions.length > 0) { + const prompted = await promptFn(questions); + if (email === undefined) email = prompted.email as string; + if (password === undefined) password = prompted.password as string; + if (appName === undefined) appName = prompted.appName as string; + } + + return { email: email!, password: password!, appName: appName! }; +} + +// --------------------------------------------------------------------------- +// Kickstart file generation +// --------------------------------------------------------------------------- + +async function createKickstart(kickstartPath: string, answers: InstallAnswers, newDir: string) { const salt = bcrypt.genSaltSync(10) const saltBase = salt.split('$10$')[1]; const fullHash = bcrypt.hashSync(answers.password, salt) @@ -29,11 +168,15 @@ async function createKickstart(kickstartPath: string, answers: any, newDir: stri fs.writeFileSync(`${newDir}/kickstart/kickstart.json`, JSON.stringify(kickstartObject, null, 2)) } -const action = async function (dir: string) { +// --------------------------------------------------------------------------- +// Command action +// --------------------------------------------------------------------------- + +const action = async function (dir: string, options: InstallOptions) { const dockerInstalled = isDockerInstalled(); const directory = path.resolve(dir) logEvent('cli command kickstart:install') - + betaWarning() try { @@ -53,81 +196,49 @@ const action = async function (dir: string) { console.error(chalk.red(`Can't write to ${parentDir}. Please check permissions on the directory`)) } - inquirer.prompt([ - { - type: 'input', - name: 'email', - message: "Admin Email Address", - default: 'admin@example.com', - validate: function (email) { - return /(([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)|(\".+\"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))/.test(email) ? true : 'Not a valid email address'; - } - }, - { - type: 'password', - name: 'password', - message: "Admin user password", - mask: true, - validate: (text) => { - if (text.length == 0) { - return 'Custom password is required' - } else if (text.length < 8) { - return 'Password must be at least 8 characters (You can change this requirement later in your tenant password settings)' - } else { - return true - } - } - }, - { - type: 'input', - name: 'appName', - message: 'Name your application', - default: "Example App" - } - ]) - .then((answers) => { - const spinner = yoctoSpinner({ text: "Building..." }).start() - setTimeout(() => { - // move fusionauth folder to user's project - console.log(chalk.green(`\nTransferring files to ${dir}`)) - fs.cpSync(`${__dirname}/resources/kickstart/fusionauth`, directory, { recursive: true }) - }, 500) - setTimeout(() => { - console.log(chalk.green(`Creating Kickstart file`)) - if (!fs.existsSync(directory)) throw (chalk.red(`Something went wrong. ${directory} does not exists.`)) - createKickstart(__dirname + '/resources/kickstart/kickstart.json', answers, directory) - }, 1500) - - setTimeout(() => { - const postgresPass = crypto.randomUUID() - const dbPass = crypto.randomUUID() - - console.log(chalk.green(`Transferring environment variables`)) - fs.renameSync(`${directory}/.env.defaults`, `${directory}/.env`) - fs.appendFileSync(`${directory}/.env`, `\nPOSTGRES_PASSWORD=${postgresPass}\nDATABASE_PASSWORD=${dbPass}\nCLI_DIR=${directory}`) - }, 2500) - - setTimeout(() => { - spinner.success("Done building!\n") - - console.log(boxen(`You're ready to start your Docker container\n${chalk.magenta(`Step 1:`)} cd ${dir}\n${chalk.magenta("Step 2: ")}npx fusionauth kickstart:start`, { padding: 1, title: "Next Steps", borderColor: "green", borderStyle: 'bold' })) - - }, 3500) - - }).catch((err) => { - console.error(chalk.yellow('Cancelling kickstart installation...')) - }) - + let answers: InstallAnswers; + try { + answers = await resolveInstallAnswers(options); + } catch (e: any) { + errorAndExit(e.message ?? String(e)); + return; + } + const spinner = yoctoSpinner({ text: "Building..." }).start() + setTimeout(() => { + console.log(chalk.green(`\nTransferring files to ${dir}`)) + fs.cpSync(`${__dirname}/resources/kickstart/fusionauth`, directory, { recursive: true }) + }, 500) + setTimeout(() => { + console.log(chalk.green(`Creating Kickstart file`)) + if (!fs.existsSync(directory)) throw (chalk.red(`Something went wrong. ${directory} does not exists.`)) + createKickstart(__dirname + '/resources/kickstart/kickstart.json', answers, directory) + }, 1500) + + setTimeout(() => { + const postgresPass = crypto.randomUUID() + const dbPass = crypto.randomUUID() + + console.log(chalk.green(`Transferring environment variables`)) + fs.renameSync(`${directory}/.env.defaults`, `${directory}/.env`) + fs.appendFileSync(`${directory}/.env`, `\nPOSTGRES_PASSWORD=${postgresPass}\nDATABASE_PASSWORD=${dbPass}\nCLI_DIR=${directory}`) + }, 2500) + + setTimeout(() => { + spinner.success("Done building!\n") + console.log(boxen(`You're ready to start your Docker container\n${chalk.magenta(`Step 1:`)} cd ${dir}\n${chalk.magenta("Step 2: ")}npx fusionauth kickstart:start`, { padding: 1, title: "Next Steps", borderColor: "green", borderStyle: 'bold' })) + }, 3500) } catch (e) { console.error(e) } - } export const kickstartInstall = new Command() .command('kickstart:install') .description('Adds a directory with a FusionAuth Docker + Kickstart') .argument('[dir]', 'Optional directory to install FusionAuth', 'fusionauth') - .action((dir) => action(dir)) \ No newline at end of file + .option('--admin-email ', 'Admin user email address (skips prompt)') + .option('--admin-password-env ', 'Name of environment variable containing the admin password (skips prompt)') + .option('--application-name ', 'Application name (skips prompt)') + .action((dir, options) => action(dir, options)) From ede789d0bd106eaa3e98218e76f47651af269845 Mon Sep 17 00:00:00 2001 From: Andy Pai <8798244+andrewpai@users.noreply.github.com> Date: Tue, 8 Sep 2026 16:46:26 -0600 Subject: [PATCH 06/17] package-lock version update --- src/commands/import-generate.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commands/import-generate.ts b/src/commands/import-generate.ts index 18abc0c..b03ecdc 100644 --- a/src/commands/import-generate.ts +++ b/src/commands/import-generate.ts @@ -29,7 +29,7 @@ const action = async function ({numberOfFiles, countPerFile, applicationId, grou for (const [old, replacement] of Object.entries(DEPRECATED_FLAGS)) { if (process.argv.includes(old)) { console.warn(chalk.yellow( - `DEPRECATION WARNING: please start using ${replacement} going forward. ` + + `DEPRECATION WARNING: please use ${replacement} going forward. ` + `${old} will be deprecated in a future release.` )); } From 7970a98a47753c6e26d1ebf58638cb7838ca1d12 Mon Sep 17 00:00:00 2001 From: Andy Pai <8798244+andrewpai@users.noreply.github.com> Date: Wed, 9 Sep 2026 15:18:28 -0600 Subject: [PATCH 07/17] Address PR review feedback - import-generate: detect deprecated flags in --flag=value form, not just bare --flag - kickstart-install: replace setTimeout-chained install steps with sequential awaited steps so errors propagate through try/catch and ordering is deterministic; also await createKickstart (was previously fire-and-forget) - utils: confirmOrExit now requires both stdin and stdout to be TTYs before treating the session as interactive, and normalizes confirmation input (trims whitespace, accepts y/yes case-insensitively) --- src/commands/import-generate.ts | 3 ++- src/commands/kickstart-install.ts | 43 ++++++++++++++----------------- src/utils.ts | 14 ++++++---- 3 files changed, 31 insertions(+), 29 deletions(-) diff --git a/src/commands/import-generate.ts b/src/commands/import-generate.ts index b03ecdc..0f50ff9 100644 --- a/src/commands/import-generate.ts +++ b/src/commands/import-generate.ts @@ -27,7 +27,8 @@ const action = async function ({numberOfFiles, countPerFile, applicationId, grou } ): Promise { for (const [old, replacement] of Object.entries(DEPRECATED_FLAGS)) { - if (process.argv.includes(old)) { + const wasUsed = process.argv.some((arg) => arg === old || arg.startsWith(`${old}=`)); + if (wasUsed) { console.warn(chalk.yellow( `DEPRECATION WARNING: please use ${replacement} going forward. ` + `${old} will be deprecated in a future release.` diff --git a/src/commands/kickstart-install.ts b/src/commands/kickstart-install.ts index 928d6fa..c88ca24 100644 --- a/src/commands/kickstart-install.ts +++ b/src/commands/kickstart-install.ts @@ -205,29 +205,26 @@ const action = async function (dir: string, options: InstallOptions) { } const spinner = yoctoSpinner({ text: "Building..." }).start() - setTimeout(() => { - console.log(chalk.green(`\nTransferring files to ${dir}`)) - fs.cpSync(`${__dirname}/resources/kickstart/fusionauth`, directory, { recursive: true }) - }, 500) - setTimeout(() => { - console.log(chalk.green(`Creating Kickstart file`)) - if (!fs.existsSync(directory)) throw (chalk.red(`Something went wrong. ${directory} does not exists.`)) - createKickstart(__dirname + '/resources/kickstart/kickstart.json', answers, directory) - }, 1500) - - setTimeout(() => { - const postgresPass = crypto.randomUUID() - const dbPass = crypto.randomUUID() - - console.log(chalk.green(`Transferring environment variables`)) - fs.renameSync(`${directory}/.env.defaults`, `${directory}/.env`) - fs.appendFileSync(`${directory}/.env`, `\nPOSTGRES_PASSWORD=${postgresPass}\nDATABASE_PASSWORD=${dbPass}\nCLI_DIR=${directory}`) - }, 2500) - - setTimeout(() => { - spinner.success("Done building!\n") - console.log(boxen(`You're ready to start your Docker container\n${chalk.magenta(`Step 1:`)} cd ${dir}\n${chalk.magenta("Step 2: ")}npx fusionauth kickstart:start`, { padding: 1, title: "Next Steps", borderColor: "green", borderStyle: 'bold' })) - }, 3500) + + // Sequential, awaited steps (rather than setTimeout-chained callbacks) so that: + // - exceptions propagate through the surrounding try/catch + // - step ordering is deterministic regardless of machine speed + console.log(chalk.green(`\nTransferring files to ${dir}`)) + fs.cpSync(`${__dirname}/resources/kickstart/fusionauth`, directory, { recursive: true }) + + console.log(chalk.green(`Creating Kickstart file`)) + if (!fs.existsSync(directory)) throw (chalk.red(`Something went wrong. ${directory} does not exists.`)) + await createKickstart(__dirname + '/resources/kickstart/kickstart.json', answers, directory) + + const postgresPass = crypto.randomUUID() + const dbPass = crypto.randomUUID() + + console.log(chalk.green(`Transferring environment variables`)) + fs.renameSync(`${directory}/.env.defaults`, `${directory}/.env`) + fs.appendFileSync(`${directory}/.env`, `\nPOSTGRES_PASSWORD=${postgresPass}\nDATABASE_PASSWORD=${dbPass}\nCLI_DIR=${directory}`) + + spinner.success("Done building!\n") + console.log(boxen(`You're ready to start your Docker container\n${chalk.magenta(`Step 1:`)} cd ${dir}\n${chalk.magenta("Step 2: ")}npx fusionauth kickstart:start`, { padding: 1, title: "Next Steps", borderColor: "green", borderStyle: 'bold' })) } catch (e) { console.error(e) diff --git a/src/utils.ts b/src/utils.ts index a2d3e91..517ed5d 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -179,9 +179,12 @@ export function errorAndExit(message: string, error?: any) { * Prompts the user for confirmation before proceeding with a risky operation. * * - If `yes` is true, returns immediately (caller has pre-confirmed). - * - If running interactively (TTY), prints the message and prompts [y/N]. - * - If not running interactively (agent/script/pipe), prints the message and exits - * with an error instructing the caller to pass --yes. + * - If running interactively (both stdin and stdout are TTYs), prints the message + * and prompts [y/N]. Accepts "y" or "yes" (case-insensitive, whitespace trimmed) + * as confirmation; anything else aborts. + * - If not running interactively (agent/script/pipe — e.g. stdin is piped even if + * stdout is a TTY), prints the message and exits with an error instructing the + * caller to pass --yes. * * @param message A description of what will happen and why it is risky. * @param yes The value of the --yes flag from the command options. @@ -191,7 +194,7 @@ export async function confirmOrExit(message: string, yes: boolean): Promise((resolve) => { rl.question('Proceed? [y/N] ', (answer) => { rl.close(); - if (answer.toLowerCase() !== 'y') { + const normalized = answer.trim().toLowerCase(); + if (normalized !== 'y' && normalized !== 'yes') { console.log('Aborted.'); process.exit(0); } From 32a1c5e4f8f8cde18f8632cd12ef0b764dd897c9 Mon Sep 17 00:00:00 2001 From: Andy Pai <8798244+andrewpai@users.noreply.github.com> Date: Wed, 9 Sep 2026 15:46:18 -0600 Subject: [PATCH 08/17] Add test coverage for confirmOrExit and kickstart:kill - utils.ts: extract isConfirmationAccepted() as a pure, exported function so the accept/reject decision logic can be unit tested directly without simulating a real TTY - kickstart-kill.ts: export action() and add an injectable deps parameter (isDockerInstalled, confirmOrExit, spawn) so tests can exercise the confirmation gating without touching real docker or exiting the process - add __tests__/utils.test.js covering isConfirmationAccepted and the yes-bypass / non-interactive TTY-detection paths of confirmOrExit - add __tests__/commands/kickstart-kill.test.js covering docker-not-installed, CLI_DIR mismatch, --yes bypass, and confirm-rejected gating paths - wire both new test files into the test and test:unit npm scripts --- __tests__/commands/kickstart-kill.test.js | 118 ++++++++++++++++++++++ __tests__/utils.test.js | 105 +++++++++++++++++++ src/commands/kickstart-kill.ts | 20 +++- src/utils.ts | 9 +- 4 files changed, 245 insertions(+), 7 deletions(-) create mode 100644 __tests__/commands/kickstart-kill.test.js create mode 100644 __tests__/utils.test.js diff --git a/__tests__/commands/kickstart-kill.test.js b/__tests__/commands/kickstart-kill.test.js new file mode 100644 index 0000000..1eff41f --- /dev/null +++ b/__tests__/commands/kickstart-kill.test.js @@ -0,0 +1,118 @@ +import { describe, test } from "node:test" +import assert from "node:assert/strict" +import { action } from "../../src/commands/kickstart-kill.js" + +/** + * Fake child-process-like object returned by mocked spawn — supports the + * minimal surface kickstart-kill's action() touches (.on, .stdout) without + * running any real process. + */ +function fakeChildProcess() { + return { + on: () => {}, + stdout: undefined, + } +} + +describe('kickstart:kill action()', () => { + test('does not call confirmOrExit or spawn when Docker is not installed', async () => { + const confirmCalls = [] + const spawnCalls = [] + + await action( + { yes: false }, + { + isDockerInstalled: () => false, + confirmOrExit: async (...args) => { confirmCalls.push(args) }, + spawn: (...args) => { spawnCalls.push(args); return fakeChildProcess() }, + } + ) + + assert.equal(confirmCalls.length, 0, 'confirmOrExit should not be called') + assert.equal(spawnCalls.length, 0, 'spawn should not be called') + }) + + test('does not call confirmOrExit or spawn when CLI_DIR does not match cwd', async () => { + const originalCliDir = process.env.CLI_DIR + process.env.CLI_DIR = '/not/the/current/directory' + + const confirmCalls = [] + const spawnCalls = [] + + try { + await action( + { yes: false }, + { + isDockerInstalled: () => true, + confirmOrExit: async (...args) => { confirmCalls.push(args) }, + spawn: (...args) => { spawnCalls.push(args); return fakeChildProcess() }, + } + ) + } finally { + if (originalCliDir === undefined) { + delete process.env.CLI_DIR + } else { + process.env.CLI_DIR = originalCliDir + } + } + + assert.equal(confirmCalls.length, 0, 'confirmOrExit should not be called') + assert.equal(spawnCalls.length, 0, 'spawn should not be called') + }) + + test('yes=true calls confirmOrExit (which resolves immediately) then spawn', async () => { + const originalCliDir = process.env.CLI_DIR + process.env.CLI_DIR = process.cwd() + + const confirmCalls = [] + const spawnCalls = [] + + try { + await action( + { yes: true }, + { + isDockerInstalled: () => true, + confirmOrExit: async (...args) => { confirmCalls.push(args) }, + spawn: (...args) => { spawnCalls.push(args); return fakeChildProcess() }, + } + ) + } finally { + if (originalCliDir === undefined) { + delete process.env.CLI_DIR + } else { + process.env.CLI_DIR = originalCliDir + } + } + + assert.equal(confirmCalls.length, 1, 'confirmOrExit should be called once') + assert.equal(confirmCalls[0][1], true, 'confirmOrExit should receive yes=true') + assert.equal(spawnCalls.length, 1, 'spawn should be called once') + assert.equal(spawnCalls[0][0], 'docker compose down -v') + }) + + test('when confirmOrExit rejects (declined/non-interactive), spawn is never called', async () => { + const originalCliDir = process.env.CLI_DIR + process.env.CLI_DIR = process.cwd() + + const spawnCalls = [] + + try { + await action( + { yes: false }, + { + isDockerInstalled: () => true, + confirmOrExit: async () => { throw new Error('declined') }, + spawn: (...args) => { spawnCalls.push(args); return fakeChildProcess() }, + } + ) + } finally { + if (originalCliDir === undefined) { + delete process.env.CLI_DIR + } else { + process.env.CLI_DIR = originalCliDir + } + } + + assert.equal(spawnCalls.length, 0, 'spawn should not be called') + }) +}) diff --git a/__tests__/utils.test.js b/__tests__/utils.test.js new file mode 100644 index 0000000..2dd34fe --- /dev/null +++ b/__tests__/utils.test.js @@ -0,0 +1,105 @@ +import { describe, test } from "node:test" +import assert from "node:assert/strict" +import { isConfirmationAccepted, confirmOrExit } from "../src/utils.js" + +describe('isConfirmationAccepted()', () => { + test('accepts "y"', () => { + assert.equal(isConfirmationAccepted('y'), true) + }) + + test('accepts "yes"', () => { + assert.equal(isConfirmationAccepted('yes'), true) + }) + + test('accepts case-insensitive variants', () => { + assert.equal(isConfirmationAccepted('Y'), true) + assert.equal(isConfirmationAccepted('YES'), true) + assert.equal(isConfirmationAccepted('Yes'), true) + }) + + test('accepts whitespace-padded variants', () => { + assert.equal(isConfirmationAccepted(' y '), true) + assert.equal(isConfirmationAccepted(' yes '), true) + }) + + test('rejects "n"', () => { + assert.equal(isConfirmationAccepted('n'), false) + }) + + test('rejects empty string', () => { + assert.equal(isConfirmationAccepted(''), false) + }) + + test('rejects unrelated text', () => { + assert.equal(isConfirmationAccepted('nope'), false) + assert.equal(isConfirmationAccepted('ye'), false) + assert.equal(isConfirmationAccepted('sure'), false) + }) +}) + +describe('confirmOrExit()', () => { + test('yes=true resolves immediately without touching stdin/stdout', async (t) => { + const exitMock = t.mock.method(process, 'exit', () => {}) + + await confirmOrExit('This is risky', true) + + assert.equal(exitMock.mock.calls.length, 0, 'process.exit should not be called') + }) + + test('non-interactive (stdin not a TTY) exits with code 1', async (t) => { + const exitMock = t.mock.method(process, 'exit', () => {}) + const originalStdinTTY = process.stdin.isTTY + const originalStdoutTTY = process.stdout.isTTY + + process.stdin.isTTY = false + process.stdout.isTTY = true + + try { + await confirmOrExit('This is risky', false) + } finally { + process.stdin.isTTY = originalStdinTTY + process.stdout.isTTY = originalStdoutTTY + } + + assert.equal(exitMock.mock.calls.length, 1, 'process.exit should be called once') + assert.equal(exitMock.mock.calls[0].arguments[0], 1) + }) + + test('non-interactive (stdout not a TTY) exits with code 1', async (t) => { + const exitMock = t.mock.method(process, 'exit', () => {}) + const originalStdinTTY = process.stdin.isTTY + const originalStdoutTTY = process.stdout.isTTY + + process.stdin.isTTY = true + process.stdout.isTTY = false + + try { + await confirmOrExit('This is risky', false) + } finally { + process.stdin.isTTY = originalStdinTTY + process.stdout.isTTY = originalStdoutTTY + } + + assert.equal(exitMock.mock.calls.length, 1, 'process.exit should be called once') + assert.equal(exitMock.mock.calls[0].arguments[0], 1) + }) + + test('non-interactive (both not TTYs) exits with code 1', async (t) => { + const exitMock = t.mock.method(process, 'exit', () => {}) + const originalStdinTTY = process.stdin.isTTY + const originalStdoutTTY = process.stdout.isTTY + + process.stdin.isTTY = false + process.stdout.isTTY = false + + try { + await confirmOrExit('This is risky', false) + } finally { + process.stdin.isTTY = originalStdinTTY + process.stdout.isTTY = originalStdoutTTY + } + + assert.equal(exitMock.mock.calls.length, 1, 'process.exit should be called once') + assert.equal(exitMock.mock.calls[0].arguments[0], 1) + }) +}) diff --git a/src/commands/kickstart-kill.ts b/src/commands/kickstart-kill.ts index 0e33fca..4474ba6 100644 --- a/src/commands/kickstart-kill.ts +++ b/src/commands/kickstart-kill.ts @@ -5,24 +5,34 @@ import { spawn } from 'node:child_process'; import { betaWarning, confirmOrExit, isDockerInstalled, logEvent } from "../utils.js"; import boxen from "boxen"; +// Dependencies below are injectable for testing — avoids real docker/confirm/exit calls +export interface KillDeps { + isDockerInstalled?: typeof isDockerInstalled; + confirmOrExit?: typeof confirmOrExit; + spawn?: typeof spawn; +} + +export const action = async function ({ yes }: { yes: boolean }, deps: KillDeps = {}) { + const checkDocker = deps.isDockerInstalled ?? isDockerInstalled; + const confirm = deps.confirmOrExit ?? confirmOrExit; + const spawnFn = deps.spawn ?? spawn; -const action = async function ({ yes }: { yes: boolean }) { betaWarning(); try { - if (!isDockerInstalled()) throw (chalk.red('Error: You need Docker to run.')) + if (!checkDocker()) throw (chalk.red('Error: You need Docker to run.')) if (process.cwd() != process.env.CLI_DIR) throw(chalk.red('Error: Current directory was not kickstarted.')) logEvent('cli command kickstart:kill') - await confirmOrExit( + await confirm( "This will run 'docker compose down -v', destroying the container and all database data. This cannot be undone.", yes ); console.log(chalk.yellow('Killing FusionAuth...\n')) try { - const starting = spawn('docker compose down -v', { shell: true, stdio: 'inherit' }) + const starting = spawnFn('docker compose down -v', { shell: true, stdio: 'inherit' }) starting.on('error', e => { console.error(e) }) @@ -50,4 +60,4 @@ export const kickstartKill = new Command() .command('kickstart:kill') .description('Runs docker compose down in current directory') .option('--yes', 'Skip confirmation prompt', false) - .action(action) + .action((options) => action(options)) diff --git a/src/utils.ts b/src/utils.ts index 517ed5d..ff8dd18 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -175,6 +175,12 @@ export function errorAndExit(message: string, error?: any) { process.exit(1); } +// Exported for testability — pure logic, no I/O, easy to unit test directly. +export function isConfirmationAccepted(answer: string): boolean { + const normalized = answer.trim().toLowerCase(); + return normalized === 'y' || normalized === 'yes'; +} + /** * Prompts the user for confirmation before proceeding with a risky operation. * @@ -205,8 +211,7 @@ export async function confirmOrExit(message: string, yes: boolean): Promise((resolve) => { rl.question('Proceed? [y/N] ', (answer) => { rl.close(); - const normalized = answer.trim().toLowerCase(); - if (normalized !== 'y' && normalized !== 'yes') { + if (!isConfirmationAccepted(answer)) { console.log('Aborted.'); process.exit(0); } From ec3866e978425945474d48b7ea6992f0043b8d7b Mon Sep 17 00:00:00 2001 From: Andy Pai <8798244+andrewpai@users.noreply.github.com> Date: Wed, 9 Sep 2026 15:58:18 -0600 Subject: [PATCH 09/17] Fix confirmOrExit silently proceeding when process.exit is mocked/deferred Previously, the rl.question callback called process.exit(0) on decline but had no return statement, so resolve() ran unconditionally afterward. In production this was masked because process.exit halts execution synchronously, but in any environment where exit is mocked or deferred (e.g. tests), a declined confirmation would be silently treated as accepted, letting the caller proceed with the risky operation. - extract handleConfirmationAnswer(answer, resolve, reject): resolves on accept, exits + rejects on decline, so the promise can never silently resolve when exit doesn't actually happen - confirmOrExit now passes both resolve and reject into handleConfirmationAnswer - add 3 tests in __tests__/utils.test.js covering accept, decline, and the decline-with-mocked-exit case that reproduces the original bug --- __tests__/utils.test.js | 36 +++++++++++++++++++++++++++++++++++- src/utils.ts | 26 ++++++++++++++++++++------ 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/__tests__/utils.test.js b/__tests__/utils.test.js index 2dd34fe..53df9fa 100644 --- a/__tests__/utils.test.js +++ b/__tests__/utils.test.js @@ -1,6 +1,6 @@ import { describe, test } from "node:test" import assert from "node:assert/strict" -import { isConfirmationAccepted, confirmOrExit } from "../src/utils.js" +import { isConfirmationAccepted, handleConfirmationAnswer, confirmOrExit } from "../src/utils.js" describe('isConfirmationAccepted()', () => { test('accepts "y"', () => { @@ -37,6 +37,40 @@ describe('isConfirmationAccepted()', () => { }) }) +describe('handleConfirmationAnswer()', () => { + test('accepted answer calls resolve, not reject or process.exit', (t) => { + const exitMock = t.mock.method(process, 'exit', () => {}) + let resolved = false + let rejected = false + + handleConfirmationAnswer('y', () => { resolved = true }, () => { rejected = true }) + + assert.equal(resolved, true) + assert.equal(rejected, false) + assert.equal(exitMock.mock.calls.length, 0) + }) + + test('declined answer calls process.exit(0)', (t) => { + const exitMock = t.mock.method(process, 'exit', () => {}) + + handleConfirmationAnswer('n', () => {}, () => {}) + + assert.equal(exitMock.mock.calls.length, 1) + assert.equal(exitMock.mock.calls[0].arguments[0], 0) + }) + + test('declined answer rejects rather than resolving when process.exit is mocked (does not actually exit)', (t) => { + t.mock.method(process, 'exit', () => {}) + let resolved = false + let rejectedWith + + handleConfirmationAnswer('n', () => { resolved = true }, (err) => { rejectedWith = err }) + + assert.equal(resolved, false, 'resolve should never be called on decline') + assert.ok(rejectedWith instanceof Error, 'reject should be called with an Error') + }) +}) + describe('confirmOrExit()', () => { test('yes=true resolves immediately without touching stdin/stdout', async (t) => { const exitMock = t.mock.method(process, 'exit', () => {}) diff --git a/src/utils.ts b/src/utils.ts index ff8dd18..c03deff 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -181,6 +181,24 @@ export function isConfirmationAccepted(answer: string): boolean { return normalized === 'y' || normalized === 'yes'; } +// Exported for testability — settles the prompt's Promise without needing a real TTY/readline round-trip. +export function handleConfirmationAnswer( + answer: string, + resolve: () => void, + reject: (reason?: any) => void +): void { + if (isConfirmationAccepted(answer)) { + resolve(); + return; + } + console.log('Aborted.'); + process.exit(0); + // Only reached if process.exit was mocked/deferred (e.g. in tests) — reject rather + // than falling through to resolve(), which would incorrectly treat a decline as + // confirmation. + reject(new Error('Aborted by user.')); +} + /** * Prompts the user for confirmation before proceeding with a risky operation. * @@ -208,14 +226,10 @@ export async function confirmOrExit(message: string, yes: boolean): Promise((resolve) => { + await new Promise((resolve, reject) => { rl.question('Proceed? [y/N] ', (answer) => { rl.close(); - if (!isConfirmationAccepted(answer)) { - console.log('Aborted.'); - process.exit(0); - } - resolve(); + handleConfirmationAnswer(answer, resolve, reject); }); }); } From edf29b6e94d5c04ec084c569d03fcb62841215e9 Mon Sep 17 00:00:00 2001 From: Andy Pai <8798244+andrewpai@users.noreply.github.com> Date: Wed, 9 Sep 2026 16:08:50 -0600 Subject: [PATCH 10/17] Add test coverage for import:generate deprecated-flag detection - extract getDeprecatedFlagUsage(argv) as a pure, exported function so the deprecation-detection logic is testable without mocking process.argv or console.warn - export DEPRECATED_FLAGS for use in tests - add __tests__/commands/import-generate.test.js covering: no deprecated flags used, bare --flag and --flag=value forms detected, multiple deprecated flags detected together, new kebab-case form not flagged, and that both the deprecated and current flag spellings populate the same underlying Commander option property - wire the new test file into the test and test:unit npm scripts --- __tests__/commands/import-generate.test.js | 64 ++++++++++++++++++++++ src/commands/import-generate.ts | 28 ++++++---- 2 files changed, 82 insertions(+), 10 deletions(-) create mode 100644 __tests__/commands/import-generate.test.js diff --git a/__tests__/commands/import-generate.test.js b/__tests__/commands/import-generate.test.js new file mode 100644 index 0000000..c64fc1f --- /dev/null +++ b/__tests__/commands/import-generate.test.js @@ -0,0 +1,64 @@ +import { describe, test } from "node:test" +import assert from "node:assert/strict" +import { getDeprecatedFlagUsage, importGenerate } from "../../src/commands/import-generate.js" + +describe('getDeprecatedFlagUsage()', () => { + test('returns empty array when no deprecated flags are used', () => { + const usage = getDeprecatedFlagUsage(['node', 'script', '--number-of-files', '5']) + assert.deepEqual(usage, []) + }) + + test('detects a bare deprecated flag (--flag value form)', () => { + const usage = getDeprecatedFlagUsage(['node', 'script', '--numberOfFiles', '5']) + assert.equal(usage.length, 1) + assert.deepEqual(usage[0], ['--numberOfFiles', '--number-of-files']) + }) + + test('detects a deprecated flag in --flag=value form', () => { + const usage = getDeprecatedFlagUsage(['node', 'script', '--numberOfFiles=5']) + assert.equal(usage.length, 1) + assert.deepEqual(usage[0], ['--numberOfFiles', '--number-of-files']) + }) + + test('detects multiple deprecated flags used together', () => { + const usage = getDeprecatedFlagUsage(['node', 'script', '--numberOfFiles', '5', '--groupId=abc']) + const oldFlags = usage.map(([old]) => old) + assert.ok(oldFlags.includes('--numberOfFiles')) + assert.ok(oldFlags.includes('--groupId')) + assert.equal(usage.length, 2) + }) + + test('does not flag the new kebab-case form as deprecated', () => { + const usage = getDeprecatedFlagUsage(['node', 'script', '--group-id', 'abc']) + assert.deepEqual(usage, []) + }) +}) + +describe('import:generate option parsing', () => { + test('deprecated --numberOfFiles populates the same option as --number-of-files', async () => { + let capturedOptions + importGenerate.action((options) => { capturedOptions = options }) + + await importGenerate.parseAsync(['--numberOfFiles', '5'], { from: 'user' }) + + assert.equal(capturedOptions.numberOfFiles, '5') + }) + + test('--number-of-files populates the same numberOfFiles property', async () => { + let capturedOptions + importGenerate.action((options) => { capturedOptions = options }) + + await importGenerate.parseAsync(['--number-of-files', '7'], { from: 'user' }) + + assert.equal(capturedOptions.numberOfFiles, '7') + }) + + test('deprecated --groupId populates the same option as --group-id', async () => { + let capturedOptions + importGenerate.action((options) => { capturedOptions = options }) + + await importGenerate.parseAsync(['--groupId', 'abc-123'], { from: 'user' }) + + assert.equal(capturedOptions.groupId, 'abc-123') + }) +}) diff --git a/src/commands/import-generate.ts b/src/commands/import-generate.ts index 0f50ff9..75de421 100644 --- a/src/commands/import-generate.ts +++ b/src/commands/import-generate.ts @@ -7,7 +7,7 @@ import {errorAndExit, logEvent} from '../utils.js'; import { faker } from '@faker-js/faker'; import * as fs from 'fs'; -const DEPRECATED_FLAGS: Record = { +export const DEPRECATED_FLAGS: Record = { '--numberOfFiles': '--number-of-files', '--countPerFile': '--count-per-file', '--applicationId': '--application-id', @@ -16,6 +16,22 @@ const DEPRECATED_FLAGS: Record = { '--filePrefix': '--file-prefix', }; +// Exported for testability — pure function, no I/O; returns [oldFlag, replacement] pairs found in argv. +export function getDeprecatedFlagUsage(argv: string[]): Array<[string, string]> { + return Object.entries(DEPRECATED_FLAGS).filter(([old]) => + argv.some((arg) => arg === old || arg.startsWith(`${old}=`)) + ); +} + +function warnDeprecatedFlags(argv: string[] = process.argv): void { + for (const [old, replacement] of getDeprecatedFlagUsage(argv)) { + console.warn(chalk.yellow( + `DEPRECATION WARNING: please use ${replacement} going forward. ` + + `${old} will be deprecated in a future release.` + )); + } +} + const action = async function ({numberOfFiles, countPerFile, applicationId, groupId, tmpDir, filePrefix} : { numberOfFiles?: string | undefined; @@ -26,15 +42,7 @@ const action = async function ({numberOfFiles, countPerFile, applicationId, grou filePrefix?: string | undefined; } ): Promise { - for (const [old, replacement] of Object.entries(DEPRECATED_FLAGS)) { - const wasUsed = process.argv.some((arg) => arg === old || arg.startsWith(`${old}=`)); - if (wasUsed) { - console.warn(chalk.yellow( - `DEPRECATION WARNING: please use ${replacement} going forward. ` + - `${old} will be deprecated in a future release.` - )); - } - } + warnDeprecatedFlags(); logEvent('cli command import:generate') From 3b36c2af05cbd9c95a02c5bad1e0dce008335ae9 Mon Sep 17 00:00:00 2001 From: Andy Pai <8798244+andrewpai@users.noreply.github.com> Date: Wed, 9 Sep 2026 16:22:54 -0600 Subject: [PATCH 11/17] Fix crypto.randomUUID global usage and confirmOrExit non-interactive gap - kickstart-install.ts: import randomUUID from node:crypto explicitly instead of relying on the global WebCrypto object, matching the convention already used elsewhere in the codebase - utils.ts: confirmOrExit() now throws after errorAndExit() in the non-interactive path, mirroring the fix already applied to handleConfirmationAnswer in the interactive path. In production this is a no-op since process.exit(1) halts synchronously first, but in any environment where exit is mocked/deferred, the promise now rejects instead of silently resolving and letting the caller proceed with the risky operation - update the three non-interactive tests in __tests__/utils.test.js to assert.rejects, which now actually exercises the fixed behavior --- __tests__/utils.test.js | 12 ++++++------ src/commands/kickstart-install.ts | 5 +++-- src/utils.ts | 4 +++- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/__tests__/utils.test.js b/__tests__/utils.test.js index 53df9fa..2f2b097 100644 --- a/__tests__/utils.test.js +++ b/__tests__/utils.test.js @@ -80,7 +80,7 @@ describe('confirmOrExit()', () => { assert.equal(exitMock.mock.calls.length, 0, 'process.exit should not be called') }) - test('non-interactive (stdin not a TTY) exits with code 1', async (t) => { + test('non-interactive (stdin not a TTY) exits with code 1 and rejects (does not let caller proceed) when exit is mocked', async (t) => { const exitMock = t.mock.method(process, 'exit', () => {}) const originalStdinTTY = process.stdin.isTTY const originalStdoutTTY = process.stdout.isTTY @@ -89,7 +89,7 @@ describe('confirmOrExit()', () => { process.stdout.isTTY = true try { - await confirmOrExit('This is risky', false) + await assert.rejects(() => confirmOrExit('This is risky', false)) } finally { process.stdin.isTTY = originalStdinTTY process.stdout.isTTY = originalStdoutTTY @@ -99,7 +99,7 @@ describe('confirmOrExit()', () => { assert.equal(exitMock.mock.calls[0].arguments[0], 1) }) - test('non-interactive (stdout not a TTY) exits with code 1', async (t) => { + test('non-interactive (stdout not a TTY) exits with code 1 and rejects (does not let caller proceed) when exit is mocked', async (t) => { const exitMock = t.mock.method(process, 'exit', () => {}) const originalStdinTTY = process.stdin.isTTY const originalStdoutTTY = process.stdout.isTTY @@ -108,7 +108,7 @@ describe('confirmOrExit()', () => { process.stdout.isTTY = false try { - await confirmOrExit('This is risky', false) + await assert.rejects(() => confirmOrExit('This is risky', false)) } finally { process.stdin.isTTY = originalStdinTTY process.stdout.isTTY = originalStdoutTTY @@ -118,7 +118,7 @@ describe('confirmOrExit()', () => { assert.equal(exitMock.mock.calls[0].arguments[0], 1) }) - test('non-interactive (both not TTYs) exits with code 1', async (t) => { + test('non-interactive (both not TTYs) exits with code 1 and rejects (does not let caller proceed) when exit is mocked', async (t) => { const exitMock = t.mock.method(process, 'exit', () => {}) const originalStdinTTY = process.stdin.isTTY const originalStdoutTTY = process.stdout.isTTY @@ -127,7 +127,7 @@ describe('confirmOrExit()', () => { process.stdout.isTTY = false try { - await confirmOrExit('This is risky', false) + await assert.rejects(() => confirmOrExit('This is risky', false)) } finally { process.stdin.isTTY = originalStdinTTY process.stdout.isTTY = originalStdoutTTY diff --git a/src/commands/kickstart-install.ts b/src/commands/kickstart-install.ts index c88ca24..4181cc5 100644 --- a/src/commands/kickstart-install.ts +++ b/src/commands/kickstart-install.ts @@ -8,6 +8,7 @@ import fs from 'node:fs' import path from "node:path"; import { dirname } from 'node:path'; import { fileURLToPath } from 'node:url'; +import { randomUUID } from 'node:crypto'; import { betaWarning, errorAndExit, isDirEmpty, isDockerInstalled, logEvent } from "../utils.js"; const __dirname = dirname(fileURLToPath(import.meta.url)); @@ -216,8 +217,8 @@ const action = async function (dir: string, options: InstallOptions) { if (!fs.existsSync(directory)) throw (chalk.red(`Something went wrong. ${directory} does not exists.`)) await createKickstart(__dirname + '/resources/kickstart/kickstart.json', answers, directory) - const postgresPass = crypto.randomUUID() - const dbPass = crypto.randomUUID() + const postgresPass = randomUUID() + const dbPass = randomUUID() console.log(chalk.green(`Transferring environment variables`)) fs.renameSync(`${directory}/.env.defaults`, `${directory}/.env`) diff --git a/src/utils.ts b/src/utils.ts index c03deff..c2fa85f 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -220,7 +220,9 @@ export async function confirmOrExit(message: string, yes: boolean): Promise Date: Tue, 15 Sep 2026 15:02:42 -0600 Subject: [PATCH 12/17] fix(ci): remove stale duplicate pull_request trigger from test workflow The rebase onto next carried forward an old commit that added a second pull_request trigger (scoped to branches: main) to what was then integration-tests.yml. That file has since been renamed to test.yaml on next, which already has its own unscoped pull_request trigger. The duplicate key is invalid YAML (most parsers, including GitHub Actions', silently keep only the last occurrence), which risked the main-scoped trigger silently overriding the intended unscoped one and breaking CI for PRs targeting next. Removed the stale block. next takes priority over main going forward, so a main-specific trigger no longer serves any purpose here. File is now byte-identical to next's original. --- .github/workflows/test.yaml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index f6c91bf..44a5aad 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -4,9 +4,6 @@ on: push: pull_request: workflow_dispatch: - pull_request: - branches: - - main jobs: test: From b65851b51e050db2a3df4bc693df3ab9ffc76ded Mon Sep 17 00:00:00 2001 From: Andy Pai <8798244+andrewpai@users.noreply.github.com> Date: Wed, 16 Sep 2026 09:46:43 -0600 Subject: [PATCH 13/17] fix(test): pin integration test FusionAuth image to 1.69.2 The integration test fixture pinned fusionauth/fusionauth-app:latest, a floating tag. This made the integration test's pass/fail status depend on whatever FusionAuth happened to publish as latest at run time, independent of anything in this repo's history. Pin to 1.69.2 (current release) for reproducible test runs. Confirmed passing against a clean container/volume state. The kickstart:install command's own docker-compose.yml template (src/resources/kickstart/fusionauth/docker-compose.yml), which gets copied into end users' projects, intentionally remains on :latest so new installs always get the current FusionAuth release. --- .../fusionauth-integration-test-base/docker-compose.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/__tests__/integration/fixtures/kickstarts/fusionauth-integration-test-base/docker-compose.yml b/__tests__/integration/fixtures/kickstarts/fusionauth-integration-test-base/docker-compose.yml index 6ac70fb..2314490 100644 --- a/__tests__/integration/fixtures/kickstarts/fusionauth-integration-test-base/docker-compose.yml +++ b/__tests__/integration/fixtures/kickstarts/fusionauth-integration-test-base/docker-compose.yml @@ -46,7 +46,7 @@ services: - search_net fusionauth: - image: fusionauth/fusionauth-app:latest + image: fusionauth/fusionauth-app:1.69.2 depends_on: db: condition: service_healthy From 7ef41682e5532ca5c713718bd38dc94795c701f7 Mon Sep 17 00:00:00 2001 From: Andy Pai <8798244+andrewpai@users.noreply.github.com> Date: Wed, 16 Sep 2026 10:42:15 -0600 Subject: [PATCH 14/17] test: cleaned up tests and brought in validator lib for email validation --- __tests__/commands/kickstart-install.test.js | 35 ++++---------------- package-lock.json | 18 ++++++++++ package.json | 2 ++ src/commands/kickstart-install.ts | 5 ++- 4 files changed, 28 insertions(+), 32 deletions(-) diff --git a/__tests__/commands/kickstart-install.test.js b/__tests__/commands/kickstart-install.test.js index 1065396..cf7ba13 100644 --- a/__tests__/commands/kickstart-install.test.js +++ b/__tests__/commands/kickstart-install.test.js @@ -15,21 +15,12 @@ describe('validateEmail()', () => { assert.equal(validateEmail('admin@example.com'), true) }) - test('accepts an email with subdomain', () => { - assert.equal(validateEmail('user@mail.example.co.uk'), true) - }) - test('rejects an address with no @', () => { const result = validateEmail('notanemail') assert.notEqual(result, true) assert.match(result, /valid email/) }) - test('rejects an address with no domain', () => { - const result = validateEmail('user@') - assert.notEqual(result, true) - }) - test('rejects an empty string', () => { const result = validateEmail('') assert.notEqual(result, true) @@ -45,25 +36,16 @@ describe('validatePassword()', () => { assert.equal(validatePassword('abcdefgh'), true) }) - test('accepts a long password', () => { - assert.equal(validatePassword('supersecretpassword123'), true) - }) - test('rejects an empty password', () => { const result = validatePassword('') assert.notEqual(result, true) assert.match(result, /required/) }) - test('rejects a password shorter than 8 characters', () => { - const result = validatePassword('short') - assert.notEqual(result, true) - assert.match(result, /8 characters/) - }) - - test('rejects a 7-character password', () => { + test('rejects a password one character short of the minimum', () => { const result = validatePassword('1234567') assert.notEqual(result, true) + assert.match(result, /8 characters/) }) }) @@ -315,7 +297,7 @@ describe('resolveInstallAnswers() — CLI validation errors', () => { // --------------------------------------------------------------------------- describe('resolveInstallAnswers() — inquirer validate functions', () => { - test('email question carries a validate function that rejects bad input', async () => { + test('email question uses validateEmail directly', async () => { let capturedQuestions const mockPrompt = async (questions) => { @@ -327,12 +309,10 @@ describe('resolveInstallAnswers() — inquirer validate functions', () => { const emailQuestion = capturedQuestions.find((q) => q.name === 'email') assert.ok(emailQuestion, 'email question should exist') - assert.ok(typeof emailQuestion.validate === 'function', 'email question should have validate') - assert.equal(emailQuestion.validate('good@example.com'), true) - assert.notEqual(emailQuestion.validate('bad'), true) + assert.equal(emailQuestion.validate, validateEmail) }) - test('password question carries a validate function that rejects short input', async () => { + test('password question uses validatePassword directly', async () => { let capturedQuestions const mockPrompt = async (questions) => { @@ -344,9 +324,6 @@ describe('resolveInstallAnswers() — inquirer validate functions', () => { const passwordQuestion = capturedQuestions.find((q) => q.name === 'password') assert.ok(passwordQuestion, 'password question should exist') - assert.ok(typeof passwordQuestion.validate === 'function', 'password question should have validate') - assert.equal(passwordQuestion.validate('longenough'), true) - assert.notEqual(passwordQuestion.validate('short'), true) - assert.notEqual(passwordQuestion.validate(''), true) + assert.equal(passwordQuestion.validate, validatePassword) }) }) diff --git a/package-lock.json b/package-lock.json index 252d350..442e916 100644 --- a/package-lock.json +++ b/package-lock.json @@ -30,6 +30,7 @@ "queue": "7.0.0", "remove-undefined-objects": "9.0.0", "uuid": "14.0.1", + "validator": "^13.15.35", "yocto-spinner": "^1.2.1" }, "bin": { @@ -43,6 +44,7 @@ "@types/html-to-text": "9.0.4", "@types/mock-fs": "^4.13.4", "@types/node": "26.1.1", + "@types/validator": "^13.15.10", "husky": "^9.1.7", "mock-fs": "^5.5.0", "nock": "^14.0.16", @@ -1329,6 +1331,13 @@ "undici-types": "~8.3.0" } }, + "node_modules/@types/validator": { + "version": "13.15.10", + "resolved": "https://registry.npmjs.org/@types/validator/-/validator-13.15.10.tgz", + "integrity": "sha512-T8L6i7wCuyoK8A/ZeLYt1+q0ty3Zb9+qbSSvrIVitzT3YjZqkTZ40IbRsPanlB4h1QB3JVL1SYCdR6ngtFYcuA==", + "dev": true, + "license": "MIT" + }, "node_modules/@typescript/typescript-aix-ppc64": { "version": "7.0.2", "resolved": "https://registry.npmjs.org/@typescript/typescript-aix-ppc64/-/typescript-aix-ppc64-7.0.2.tgz", @@ -3224,6 +3233,15 @@ "uuid": "dist-node/bin/uuid" } }, + "node_modules/validator": { + "version": "13.15.35", + "resolved": "https://registry.npmjs.org/validator/-/validator-13.15.35.tgz", + "integrity": "sha512-TQ5pAGhd5whStmqWvYF4OjQROlmv9SMFVt37qoCBdqRffuuklWYQlCNnEs2ZaIBD1kZRNnikiZOS1eqgkar0iw==", + "license": "MIT", + "engines": { + "node": ">= 0.10" + } + }, "node_modules/webidl-conversions": { "version": "3.0.1", "resolved": "https://registry.npmjs.org/webidl-conversions/-/webidl-conversions-3.0.1.tgz", diff --git a/package.json b/package.json index bd605d5..8ae24bb 100644 --- a/package.json +++ b/package.json @@ -56,6 +56,7 @@ "queue": "7.0.0", "remove-undefined-objects": "9.0.0", "uuid": "14.0.1", + "validator": "^13.15.35", "yocto-spinner": "^1.2.1" }, "devDependencies": { @@ -66,6 +67,7 @@ "@types/html-to-text": "9.0.4", "@types/mock-fs": "^4.13.4", "@types/node": "26.1.1", + "@types/validator": "^13.15.10", "husky": "^9.1.7", "mock-fs": "^5.5.0", "nock": "^14.0.16", diff --git a/src/commands/kickstart-install.ts b/src/commands/kickstart-install.ts index 4181cc5..a826f3f 100644 --- a/src/commands/kickstart-install.ts +++ b/src/commands/kickstart-install.ts @@ -9,6 +9,7 @@ import path from "node:path"; import { dirname } from 'node:path'; import { fileURLToPath } from 'node:url'; import { randomUUID } from 'node:crypto'; +import validator from 'validator'; import { betaWarning, errorAndExit, isDirEmpty, isDockerInstalled, logEvent } from "../utils.js"; const __dirname = dirname(fileURLToPath(import.meta.url)); @@ -17,14 +18,12 @@ const __dirname = dirname(fileURLToPath(import.meta.url)); // Validation helpers (exported for testing) // --------------------------------------------------------------------------- -export const EMAIL_REGEX = /(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))/; - /** * Validates an email address. * @returns `true` if valid, otherwise an error message string. */ export function validateEmail(email: string): true | string { - return EMAIL_REGEX.test(email) ? true : 'Not a valid email address'; + return validator.isEmail(email) ? true : 'Not a valid email address'; } /** From be6e13c01ec1488cbf53453cd2a3342dc2febc0e Mon Sep 17 00:00:00 2001 From: Andy Pai <8798244+andrewpai@users.noreply.github.com> Date: Wed, 16 Sep 2026 11:11:35 -0600 Subject: [PATCH 15/17] chore: remove CONTRIBUTING.md Content will be migrated into README.md separately. --- CONTRIBUTING.md | 43 ------------------------------------------- 1 file changed, 43 deletions(-) delete mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md deleted file mode 100644 index 4534148..0000000 --- a/CONTRIBUTING.md +++ /dev/null @@ -1,43 +0,0 @@ -# Contributing - -## Command Structure -Commands generally follow the form: - -fusionauth namespace:command [--command-option] ... - -Where -* Commands are grouped into a functional or domain namespace -* Option names use kebab-case (e.g. `--admin-email`, `--number-of-files`) -* Sensitive items can be passed via environment variable. In this case use `--option-name-env ENV_VAR` to indicate that the value is coming from the specified environment variable - -## Risky Operations Policy - -Commands that perform risky operations must gate execution behind user confirmation using `confirmOrExit()` from `src/utils.ts`. All such commands must expose a `--yes` flag. - -## Testing - -### Running the tests - -```bash -# Unit tests (run these before every commit) -npm run test:unit - -# Integration tests (requires a live FusionAuth instance) -npm run test:integration - -# Full suite -npm run test -``` - -The integration tests manage a Docker container automatically. Several environment variables control their behaviour: - -| Variable | Effect | -|---|---| -| `VERBOSE_CONTAINER=true` | Print each health-check attempt, elapsed time, and error reason; dump `docker compose logs` on failure | -| `REUSE_CONTAINER=true` | Skip container startup and use a FusionAuth instance already running on `localhost:9011` | -| `SKIP_TEARDOWN=true` | Leave the container running after the tests finish (useful for manual inspection) | - -### Requirements - -- **All new functionality must be covered by tests.** This includes new commands, new options on existing commands, and new utility functions. -- **All existing tests must pass cleanly before a PR is submitted.** A clean run means zero failures — `# fail 0` in the test output. From a0bd2b98dce3e3ffe8e0b9e1c17bf81f34eab1a4 Mon Sep 17 00:00:00 2001 From: Andy Pai <8798244+andrewpai@users.noreply.github.com> Date: Wed, 16 Sep 2026 11:11:41 -0600 Subject: [PATCH 16/17] fix: exit with non-zero status on kickstart:install failure, fix typo The outer catch block only logged the error and let the command return successfully. A failed file copy, kickstart-file write, rename, or environment update would produce an error message while the CLI still exited with status 0, masking failures from scripts/CI that check the exit code. Set process.exitCode = 1 in that path. Also fix a JSDoc typo: intial -> initial. --- src/commands/kickstart-install.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/commands/kickstart-install.ts b/src/commands/kickstart-install.ts index a826f3f..52aaf8c 100644 --- a/src/commands/kickstart-install.ts +++ b/src/commands/kickstart-install.ts @@ -57,7 +57,7 @@ export interface InstallAnswers { } /** - * We need the intial admin's credentials (email and password) and a name for a + * We need the initial admin's credentials (email and password) and a name for a * starter app. This will take values from command line params if present, then * fall back to prompting the user. * @@ -228,6 +228,9 @@ const action = async function (dir: string, options: InstallOptions) { } catch (e) { console.error(e) + // Ensure a failed install (copy, kickstart write, rename, env update, etc.) + // is reflected in the process exit code rather than silently exiting 0. + process.exitCode = 1 } } From 43d08cae4bec838692202b8f1eb29cc15efae86a Mon Sep 17 00:00:00 2001 From: Andy Pai <8798244+andrewpai@users.noreply.github.com> Date: Wed, 16 Sep 2026 13:10:59 -0600 Subject: [PATCH 17/17] test: replaced mock-fs with real temp dir-based file testing --- __tests__/commands/kickstart-kill.test.js | 12 +- __tests__/helpers/temp-dir.js | 26 +++ __tests__/postInstall/postinstall.test.js | 129 +++++++-------- __tests__/telemetry/index.js | 148 ------------------ __tests__/telemetry/telemetry.test.js | 141 ++++++++++------- .../utilities/kickstart/validator.test.js | 123 ++++++++------- .../kickstart/variable-substitution.test.js | 1 - package-lock.json | 22 --- package.json | 2 - src/commands/telemetry/telemetry-utils.ts | 6 +- src/utils.ts | 17 +- 11 files changed, 248 insertions(+), 379 deletions(-) create mode 100644 __tests__/helpers/temp-dir.js delete mode 100644 __tests__/telemetry/index.js diff --git a/__tests__/commands/kickstart-kill.test.js b/__tests__/commands/kickstart-kill.test.js index 1eff41f..37bcf08 100644 --- a/__tests__/commands/kickstart-kill.test.js +++ b/__tests__/commands/kickstart-kill.test.js @@ -1,4 +1,4 @@ -import { describe, test } from "node:test" +import { describe, test, beforeEach, afterEach } from "node:test" import assert from "node:assert/strict" import { action } from "../../src/commands/kickstart-kill.js" @@ -15,6 +15,16 @@ function fakeChildProcess() { } describe('kickstart:kill action()', () => { + // action() calls logEvent() internally (unmocked). Disable telemetry so it + // short-circuits before touching the real filesystem or network. + beforeEach(() => { + process.env.FUSIONAUTH_TELEMETRY = 'false' + }) + + afterEach(() => { + delete process.env.FUSIONAUTH_TELEMETRY + }) + test('does not call confirmOrExit or spawn when Docker is not installed', async () => { const confirmCalls = [] const spawnCalls = [] diff --git a/__tests__/helpers/temp-dir.js b/__tests__/helpers/temp-dir.js new file mode 100644 index 0000000..6eb841c --- /dev/null +++ b/__tests__/helpers/temp-dir.js @@ -0,0 +1,26 @@ +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' + +/** + * Creates a real, unique temporary directory outside the repo (under the OS + * temp dir) for tests that need to exercise real filesystem behavior instead + * of mocking it. Prefer this over mock-fs, which has had shaky support for + * newer Node versions. + * + * @param prefix Prefix for the generated directory name. + * @returns The absolute path to the newly created temp directory. + */ +export function createTempDir(prefix = 'fa-cli-') { + return fs.mkdtempSync(path.join(os.tmpdir(), prefix)) +} + +/** + * Recursively removes a temp directory created by createTempDir(). Safe to + * call even if the directory doesn't exist. + * + * @param dir The directory to remove. + */ +export function removeTempDir(dir) { + fs.rmSync(dir, { recursive: true, force: true }) +} diff --git a/__tests__/postInstall/postinstall.test.js b/__tests__/postInstall/postinstall.test.js index cfcd096..3315af0 100644 --- a/__tests__/postInstall/postinstall.test.js +++ b/__tests__/postInstall/postinstall.test.js @@ -1,96 +1,73 @@ -import { describe, test } from "node:test" +import { describe, test, beforeEach, afterEach } from "node:test" import assert from "node:assert/strict" +import path from "node:path" import { createConfig } from '../../src/utils.js' +import { createTempDir, removeTempDir } from '../helpers/temp-dir.js' -import mock from 'mock-fs' -import fs, { readdirSync, readFileSync } from 'node:fs' +import fs from 'node:fs' describe('postInstall runs properly', () => { + let tempDir + let configDir + + beforeEach(() => { + tempDir = createTempDir() + configDir = path.join(tempDir, 'dist', '.fa') + }) + + afterEach(() => { + removeTempDir(tempDir) + }) + test('No config creates dir', () => { - mock({ - 'dist': {}, - }) - try { - const configFileExists = createConfig('dist/.fa') - assert.equal(configFileExists, true, 'Config not created at dist/.fa/config.json') - } finally { - mock.restore() - } + const configFileExists = createConfig(configDir) + assert.equal(configFileExists, true, 'Config not created at dist/.fa/config.json') }) + test('No dist directory, still create the directory and file', () => { - mock({ - "./": {} - }) - try { - const configFileExists = createConfig('dist/.fa') - assert.equal(configFileExists, true, 'Config not created at dist/.fa/config.json') - } finally { - mock.restore() - } + // tempDir exists but the nested dist/.fa path does not yet. + const configFileExists = createConfig(configDir) + assert.equal(configFileExists, true, 'Config not created at dist/.fa/config.json') }) + test('No config creates full config file with expected types', () => { - mock({ - 'dist': {}, - }) - try { - const configFileExists = createConfig('dist/.fa') - const configObject = JSON.parse(readFileSync('dist/.fa/config.json')) - assert(configObject.telemetry, true, 'Default telemetry not set to true') - assert(typeof configObject.id, 'string', "ID doesn't exist or isn't a string") - } finally { - mock.restore() - } + createConfig(configDir) + const configObject = JSON.parse(fs.readFileSync(path.join(configDir, 'config.json'))) + assert(configObject.telemetry, true, 'Default telemetry not set to true') + assert(typeof configObject.id, 'string', "ID doesn't exist or isn't a string") }) test('Complete config returns false', () => { - mock({ - dist: { - '.fa': { - 'config.json': JSON.stringify({id: '8c0a77f2-27e4-4284-b5d3-5618ec2a56eb', telemetry: true}) - } - } - }) - try { - assert.equal(createConfig('dist/.fa'), false, 'Postinstall did not return false properly') - } finally { - mock.restore() - } + fs.mkdirSync(configDir, { recursive: true }) + fs.writeFileSync( + path.join(configDir, 'config.json'), + JSON.stringify({ id: '8c0a77f2-27e4-4284-b5d3-5618ec2a56eb', telemetry: true }) + ) + + assert.equal(createConfig(configDir), false, 'Postinstall did not return false properly') }) + test('No ID in config, but telemetry false', () => { - mock({ - dist: { - '.fa': { - 'config.json': JSON.stringify({telemetry: false}) - } - } - }) - try { - createConfig('dist/.fa') - const configObject = JSON.parse(fs.readFileSync('dist/.fa/config.json')) - assert.equal(typeof configObject.id, 'string', 'No ID after run') - assert.equal(configObject.telemetry, false, 'Telemetry got reset') - } finally { - mock.restore() - } - }) - test('No telemetry in config, but ID', () => { - mock({ - dist: { - '.fa': { - 'config.json': JSON.stringify({id: '8c0a77f2-27e4-4284-b5d3-5618ec2a56eb'}) - } - } - }) - try { - createConfig('dist/.fa') - const configObject = JSON.parse(fs.readFileSync('dist/.fa/config.json')) - assert.equal(configObject.id, '8c0a77f2-27e4-4284-b5d3-5618ec2a56eb', 'ID got reset') - assert.equal(configObject.telemetry, true, 'Telemetry did not get set') - } finally { - mock.restore() - } + fs.mkdirSync(configDir, { recursive: true }) + fs.writeFileSync(path.join(configDir, 'config.json'), JSON.stringify({ telemetry: false })) + + createConfig(configDir) + const configObject = JSON.parse(fs.readFileSync(path.join(configDir, 'config.json'))) + assert.equal(typeof configObject.id, 'string', 'No ID after run') + assert.equal(configObject.telemetry, false, 'Telemetry got reset') }) + test('No telemetry in config, but ID', () => { + fs.mkdirSync(configDir, { recursive: true }) + fs.writeFileSync( + path.join(configDir, 'config.json'), + JSON.stringify({ id: '8c0a77f2-27e4-4284-b5d3-5618ec2a56eb' }) + ) + createConfig(configDir) + const configObject = JSON.parse(fs.readFileSync(path.join(configDir, 'config.json'))) + assert.equal(configObject.id, '8c0a77f2-27e4-4284-b5d3-5618ec2a56eb', 'ID got reset') + assert.equal(configObject.telemetry, true, 'Telemetry did not get set') + }) }) diff --git a/__tests__/telemetry/index.js b/__tests__/telemetry/index.js deleted file mode 100644 index a56d1f2..0000000 --- a/__tests__/telemetry/index.js +++ /dev/null @@ -1,148 +0,0 @@ -import test, { describe, after, before, beforeEach, afterEach } from "node:test" -import assert from "node:assert" -import fs, { readFileSync } from "node:fs" -import mock from "mock-fs" -import { telemetryUpdate } from "../../dist/commands/telemetry/telemetry-utils.js" -import { telemetryDisable } from "../../dist/commands/telemetry/telemetry-disable.js" -import { telemetryEnable } from "../../dist/commands/telemetry/telemetry-enable.js" -import path from "node:path" -import { loadConfig, logEvent } from "../../dist/utils.js" -import nock from 'nock' - -export function telemetry() { - const mockedTrueConfig = { - id: '8c0a77f2-27e4-4284-b5d3-5618ec2a56eb', - telemetry: true, - version: '1.0' - } - const mockedFalseConfig = { - id: '8c0a77f2-27e4-4284-b5d3-5618ec2a56eb', - telemetry: false, - version: '1.0' - } - describe('telemetry runs properly', () => { - test("Creates config if no config exists", (t) => { - before(() => { - mock({ - "dist": {} - }) - }) - - const updatedConfig = telemetryUpdate(true) - assert(fs.existsSync('dist/.fa/config.json'), "File wasn't created") - }) - test("Only changes telemetry value", () => { - before(() => { - mock({ - "dist/.fa/config.json": JSON.stringify(mockedFalseConfig) - }) - }) - - const updatedConfig = telemetryUpdate(true) - assert.deepEqual(updatedConfig.globalConfig, mockedTrueConfig) - }) - - test("Enable works", (t) => { - before(() => { - mock({ - "dist/.fa/config.json": JSON.stringify(mockedFalseConfig) - }) - }) - const actualConfig = telemetryUpdate(true) - assert.equal(actualConfig.globalConfig.telemetry, true, "Telemetry not set to true") - }) - test("Disable works", (t) => { - before(() => { - mock({ - "dist/.fa/config.json": JSON.stringify(mockedTrueConfig) - }) - }) - const actualConfig = telemetryUpdate(true) - assert.equal(actualConfig.globalConfig.telemetry, true, "Telemetry not set to true") - }) - test("Disable full command runs properly", (t) => { - before(() => { - mock({ - "dist/.fa/config.json": JSON.stringify(mockedTrueConfig) - }) - }) - - // TODO: Add quiet flag to remove outputs - telemetryDisable.parse() - const actualConfig = JSON.parse(fs.readFileSync('dist/.fa/config.json').toString()) - assert.equal(actualConfig.telemetry, false) - }) - test("Enable full command runs properly", (t) => { - before(() => { - mock({ - "dist/.fa/config.json": JSON.stringify(mockedFalseConfig) - }) - }) - - // TODO: Add quiet flag to remove outputs - telemetryEnable.parse() - const actualConfig = JSON.parse(fs.readFileSync('dist/.fa/config.json').toString()) - assert.equal(actualConfig.telemetry, true) - }) - }) - describe('tests for logEvent', () => { - test("If FUSIONAUTH_TELEMETRY === false don't run", async (t) => { - before(() => { - process.env.FUSIONAUTH_TELEMETRY = false - }) - after(() => { - delete process.env.FUSIONAUTH_TELEMETRY - }) - - const response = await logEvent('test event') - assert.equal(response, false, "logEvent still fired") - }) - - test("If FUSIONAUTH_TELEMETRY === true DO run", async (t) => { - before(() => { - process.env.FUSIONAUTH_TELEMETRY = true - nock('https://us.i.posthog.com') - .post('/batch/') - .reply(200) - }) - after(() => { - nock.cleanAll(); - delete process.env.FUSIONAUTH_TELEMETRY - }) - - const response = await logEvent('test event') - assert.equal(response, true, "logEvent didn't fire") - }) - test("If no .env, event submits", async (t) => { - before(() => { - nock('https://us.i.posthog.com') - .post('/batch/') - .reply(200) - }) - after(() => { - nock.cleanAll(); - }) - - assert.equal(process.env.FUSIONAUTH_TELEMETRY, undefined, 'Env variable FUSIONAUTH_TELEMETRY is defined') - const response = await logEvent('test event') - assert.equal(response, true, "logEvent didn't fire") - }) - - test("Disables warning after first log", async (t) => { - before(() => { - process.env.FUSIONAUTH_TELEMETRY = true - mock({ - "dist/.fa/config.json": JSON.stringify(mockedTrueConfig) - }) - }) - after(() => { - delete process.env.FUSIONAUTH_TELEMETRY - mock.restore() - }) - await logEvent('cli test') - const newConfig = await loadConfig() - assert.equal(newConfig.globalConfig.telemetryNoWarn, true) - }) - - }) -} diff --git a/__tests__/telemetry/telemetry.test.js b/__tests__/telemetry/telemetry.test.js index eaf3658..8de09df 100644 --- a/__tests__/telemetry/telemetry.test.js +++ b/__tests__/telemetry/telemetry.test.js @@ -1,13 +1,13 @@ -import { describe, test } from "node:test" +import { describe, test, beforeEach, afterEach } from "node:test" import assert from "node:assert/strict" -import fs, { readFileSync } from "node:fs" -import mock from "mock-fs" +import fs from "node:fs" +import path from "node:path" import { telemetryUpdate } from "../../src/commands/telemetry/telemetry-utils.js" import { telemetryDisable } from "../../src/commands/telemetry/telemetry-disable.js" import { telemetryEnable } from "../../src/commands/telemetry/telemetry-enable.js" -import path from "node:path" -import { logEvent } from "../../src/utils.js" +import { logEvent, loadConfig } from "../../src/utils.js" import nock from 'nock' +import { createTempDir, removeTempDir } from '../helpers/temp-dir.js' const mockedTrueConfig = { id: '8c0a77f2-27e4-4284-b5d3-5618ec2a56eb', @@ -20,65 +20,61 @@ const mockedFalseConfig = { version: '1.0' } +// All telemetry helpers read/write a global config file at +// `${FUSIONAUTH_CONFIG_DIR}/.fa/config.json`. Point that at a fresh real +// temp directory per test rather than mocking the filesystem, and rather +// than letting these tests write to the real repo's src/.fa/ directory. describe('telemetry runs properly', () => { + let tempDir + let configPath + + beforeEach(() => { + tempDir = createTempDir() + process.env.FUSIONAUTH_CONFIG_DIR = tempDir + configPath = path.join(tempDir, '.fa', 'config.json') + }) + + afterEach(() => { + delete process.env.FUSIONAUTH_CONFIG_DIR + removeTempDir(tempDir) + }) + + function writeConfig(config) { + fs.mkdirSync(path.dirname(configPath), { recursive: true }) + fs.writeFileSync(configPath, JSON.stringify(config)) + } + test("Creates config if no config exists", () => { - mock({ - "src": {} - }) - try { - const updatedConfig = telemetryUpdate(true) - assert(fs.existsSync('src/.fa/config.json'), "File wasn't created") - } finally { - mock.restore() - } + const updatedConfig = telemetryUpdate(true) + assert(fs.existsSync(configPath), "File wasn't created") }) test("Only changes telemetry value", () => { - mock({ - "src/.fa/config.json": JSON.stringify(mockedFalseConfig) - }) - try { - const updatedConfig = telemetryUpdate(true) - assert.deepEqual(updatedConfig.globalConfig, mockedTrueConfig) - } finally { - mock.restore() - } + writeConfig(mockedFalseConfig) + const updatedConfig = telemetryUpdate(true) + assert.deepEqual(updatedConfig.globalConfig, mockedTrueConfig) }) test("Enable works", () => { - mock({ - "src/.fa/config.json": JSON.stringify(mockedFalseConfig) - }) - try { - const actualConfig = telemetryUpdate(true) - assert.equal(actualConfig.globalConfig.telemetry, true, "Telemetry not set to true") - } finally { - mock.restore() - } + writeConfig(mockedFalseConfig) + const actualConfig = telemetryUpdate(true) + assert.equal(actualConfig.globalConfig.telemetry, true, "Telemetry not set to true") }) test("Disable works", () => { - mock({ - "src/.fa/config.json": JSON.stringify(mockedTrueConfig) - }) - try { - const actualConfig = telemetryUpdate(false) - assert.equal(actualConfig.globalConfig.telemetry, false, "Telemetry not set to false") - } finally { - mock.restore() - } + writeConfig(mockedTrueConfig) + const actualConfig = telemetryUpdate(false) + assert.equal(actualConfig.globalConfig.telemetry, false, "Telemetry not set to false") }) test("Disable full command runs properly", () => { nock('https://us.i.posthog.com') .persist() .post('/batch/') .reply(200) - mock({ - "src/.fa/config.json": JSON.stringify(mockedTrueConfig) - }) + writeConfig(mockedTrueConfig) try { telemetryDisable.parse() - const actualConfig = JSON.parse(fs.readFileSync('src/.fa/config.json').toString()) + const actualConfig = JSON.parse(fs.readFileSync(configPath).toString()) assert.equal(actualConfig.telemetry, false) } finally { - mock.restore() + nock.cleanAll() } }) test("Enable full command runs properly", () => { @@ -86,39 +82,64 @@ describe('telemetry runs properly', () => { .persist() .post('/batch/') .reply(200) - mock({ - "src/.fa/config.json": JSON.stringify(mockedFalseConfig) - }) + writeConfig(mockedFalseConfig) try { telemetryEnable.parse() - const actualConfig = JSON.parse(fs.readFileSync('src/.fa/config.json').toString()) + const actualConfig = JSON.parse(fs.readFileSync(configPath).toString()) assert.equal(actualConfig.telemetry, true) } finally { - mock.restore() + nock.cleanAll() } }) - }) - describe('tests for logEvent', () => { +}) + +describe('tests for logEvent', () => { + let tempDir + + beforeEach(() => { + tempDir = createTempDir() + process.env.FUSIONAUTH_CONFIG_DIR = tempDir + }) + + afterEach(() => { + delete process.env.FUSIONAUTH_CONFIG_DIR + delete process.env.FUSIONAUTH_TELEMETRY + removeTempDir(tempDir) + }) + test("If FUSIONAUTH_TELEMETRY === false don't run", async () => { process.env.FUSIONAUTH_TELEMETRY = 'false' + const response = await logEvent('test event') + assert.equal(response, false, "logEvent still fired") + }) + test("If no .env, event submits", async () => { + nock('https://us.i.posthog.com') + .persist() + .post('/batch/') + .reply(200) try { + assert.equal(process.env.FUSIONAUTH_TELEMETRY, undefined, 'Env variable FUSIONAUTH_TELEMETRY is defined') const response = await logEvent('test event') - assert.equal(response, false, "logEvent still fired") + assert.equal(response, true, "logEvent didn't fire") } finally { - delete process.env.FUSIONAUTH_TELEMETRY + nock.cleanAll() } }) - test("If no .env, event submits", async () => { + // Ported forward from the now-removed __tests__/telemetry/index.js — this + // is the one test there that covered behavior not exercised anywhere + // else: that the "we collect anonymous data" warning only prints once. + test("Disables warning after first log", async () => { nock('https://us.i.posthog.com') + .persist() .post('/batch/') .reply(200) try { - assert.equal(process.env.FUSIONAUTH_TELEMETRY, undefined, 'Env variable FUSIONAUTH_TELEMETRY is defined') - const response = await logEvent('test event') + const response = await logEvent('cli test') assert.equal(response, true, "logEvent didn't fire") + const newConfig = loadConfig() + assert.equal(newConfig.globalConfig.telemetryNoWarn, true) } finally { nock.cleanAll() } }) - - }) +}) diff --git a/__tests__/utilities/kickstart/validator.test.js b/__tests__/utilities/kickstart/validator.test.js index 2e9196a..b71b0cf 100644 --- a/__tests__/utilities/kickstart/validator.test.js +++ b/__tests__/utilities/kickstart/validator.test.js @@ -1,7 +1,9 @@ -import { describe, test } from "node:test" +import { describe, test, beforeEach, afterEach } from "node:test" import assert from "node:assert/strict" -import mock from "mock-fs" +import fs from "node:fs" +import path from "node:path" import { KickstartValidator } from "../../../src/utilities/kickstart/validator.js" +import { createTempDir, removeTempDir } from "../../helpers/temp-dir.js" describe('KickstartValidator', () => { @@ -304,6 +306,16 @@ describe('KickstartValidator', () => { }) describe('validateFileExists()', () => { + let tempDir + + beforeEach(() => { + tempDir = createTempDir() + }) + + afterEach(() => { + removeTempDir(tempDir) + }) + test('should report error for missing file', () => { const validator = new KickstartValidator() const result = validator.validateFileExists('/nonexistent/file.json') @@ -313,37 +325,36 @@ describe('KickstartValidator', () => { }) test('should accept existing file', () => { - mock({ - '/test/kickstart.json': '{"requests": []}' - }) - try { - const validator = new KickstartValidator() - const result = validator.validateFileExists('/test/kickstart.json') - - assert.equal(result.valid, true) - assert.equal(result.errors.length, 0) - } finally { - mock.restore() - } + const filePath = path.join(tempDir, 'kickstart.json') + fs.writeFileSync(filePath, '{"requests": []}') + + const validator = new KickstartValidator() + const result = validator.validateFileExists(filePath) + + assert.equal(result.valid, true) + assert.equal(result.errors.length, 0) }) test('should report error if path is directory', () => { - mock({ - '/test/': {} - }) - try { - const validator = new KickstartValidator() - const result = validator.validateFileExists('/test') - - assert.equal(result.valid, false) - assert(result.errors.some(e => e.message.includes('not a file'))) - } finally { - mock.restore() - } + const validator = new KickstartValidator() + const result = validator.validateFileExists(tempDir) + + assert.equal(result.valid, false) + assert(result.errors.some(e => e.message.includes('not a file'))) }) }) describe('loadAndValidateJSON()', () => { + let tempDir + + beforeEach(() => { + tempDir = createTempDir() + }) + + afterEach(() => { + removeTempDir(tempDir) + }) + test('should return error if file not found', () => { const validator = new KickstartValidator() const result = validator.loadAndValidateJSON('/nonexistent.json') @@ -353,18 +364,14 @@ describe('KickstartValidator', () => { }) test('should return error if JSON is invalid', () => { - mock({ - '/test/bad.json': '{ invalid json }' - }) - try { - const validator = new KickstartValidator() - const result = validator.loadAndValidateJSON('/test/bad.json') - - assert.equal(result.valid, false) - assert(result.errors.some(e => e.category === 'schema_invalid')) - } finally { - mock.restore() - } + const filePath = path.join(tempDir, 'bad.json') + fs.writeFileSync(filePath, '{ invalid json }') + + const validator = new KickstartValidator() + const result = validator.loadAndValidateJSON(filePath) + + assert.equal(result.valid, false) + assert(result.errors.some(e => e.category === 'schema_invalid')) }) test('should load and parse valid JSON', () => { @@ -373,19 +380,15 @@ describe('KickstartValidator', () => { { method: 'POST', url: '/api/app' } ] } - mock({ - '/test/valid.json': JSON.stringify(config) - }) - try { - const validator = new KickstartValidator() - const result = validator.loadAndValidateJSON('/test/valid.json') - - assert('config' in result) - assert.equal(result.config.requests.length, 1) - assert('lineNumbers' in result) - } finally { - mock.restore() - } + const filePath = path.join(tempDir, 'valid.json') + fs.writeFileSync(filePath, JSON.stringify(config)) + + const validator = new KickstartValidator() + const result = validator.loadAndValidateJSON(filePath) + + assert('config' in result) + assert.equal(result.config.requests.length, 1) + assert('lineNumbers' in result) }) test('should include line numbers in result', () => { @@ -395,17 +398,13 @@ describe('KickstartValidator', () => { { method: 'POST', url: '/api/app2' } ] } - mock({ - '/test/valid.json': JSON.stringify(config) - }) - try { - const validator = new KickstartValidator() - const result = validator.loadAndValidateJSON('/test/valid.json') - - assert('lineNumbers' in result) - } finally { - mock.restore() - } + const filePath = path.join(tempDir, 'valid.json') + fs.writeFileSync(filePath, JSON.stringify(config)) + + const validator = new KickstartValidator() + const result = validator.loadAndValidateJSON(filePath) + + assert('lineNumbers' in result) }) }) }) diff --git a/__tests__/utilities/kickstart/variable-substitution.test.js b/__tests__/utilities/kickstart/variable-substitution.test.js index 62826d3..c593664 100644 --- a/__tests__/utilities/kickstart/variable-substitution.test.js +++ b/__tests__/utilities/kickstart/variable-substitution.test.js @@ -1,7 +1,6 @@ import { describe, test, afterEach } from "node:test" import assert from "node:assert/strict" import nock from "nock" -import mock from 'mock-fs' import { VariableSubstitutor } from "../../../src/utilities/kickstart/variable-substitution.js" describe('VariableSubstitutor', () => { diff --git a/package-lock.json b/package-lock.json index 442e916..fd8ea8c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -42,11 +42,9 @@ "@types/figlet": "1.7.0", "@types/fs-extra": "11.0.4", "@types/html-to-text": "9.0.4", - "@types/mock-fs": "^4.13.4", "@types/node": "26.1.1", "@types/validator": "^13.15.10", "husky": "^9.1.7", - "mock-fs": "^5.5.0", "nock": "^14.0.16", "tsx": "^4.23.0", "type-fest": "5.8.0", @@ -1311,16 +1309,6 @@ "@types/node": "*" } }, - "node_modules/@types/mock-fs": { - "version": "4.13.4", - "resolved": "https://registry.npmjs.org/@types/mock-fs/-/mock-fs-4.13.4.tgz", - "integrity": "sha512-mXmM0o6lULPI8z3XNnQCpL0BGxPwx1Ul1wXYEPBGl4efShyxW2Rln0JOPEWGyZaYZMM6OVXM/15zUuFMY52ljg==", - "dev": true, - "license": "MIT", - "dependencies": { - "@types/node": "*" - } - }, "node_modules/@types/node": { "version": "26.1.1", "resolved": "https://registry.npmjs.org/@types/node/-/node-26.1.1.tgz", @@ -2767,16 +2755,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/mock-fs": { - "version": "5.5.0", - "resolved": "https://registry.npmjs.org/mock-fs/-/mock-fs-5.5.0.tgz", - "integrity": "sha512-d/P1M/RacgM3dB0sJ8rjeRNXxtapkPCUnMGmIN0ixJ16F/E4GUZCvWcSGfWGz8eaXYvn1s9baUwNjI4LOPEjiA==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=12.0.0" - } - }, "node_modules/mute-stream": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/mute-stream/-/mute-stream-3.0.0.tgz", diff --git a/package.json b/package.json index 8ae24bb..77ba0b0 100644 --- a/package.json +++ b/package.json @@ -65,11 +65,9 @@ "@types/figlet": "1.7.0", "@types/fs-extra": "11.0.4", "@types/html-to-text": "9.0.4", - "@types/mock-fs": "^4.13.4", "@types/node": "26.1.1", "@types/validator": "^13.15.10", "husky": "^9.1.7", - "mock-fs": "^5.5.0", "nock": "^14.0.16", "tsx": "^4.23.0", "type-fest": "5.8.0", diff --git a/src/commands/telemetry/telemetry-utils.ts b/src/commands/telemetry/telemetry-utils.ts index d6515ae..76c4291 100644 --- a/src/commands/telemetry/telemetry-utils.ts +++ b/src/commands/telemetry/telemetry-utils.ts @@ -1,12 +1,10 @@ import fs from "node:fs" -import { __dirname } from '../../utils.js' - -import { loadConfig } from "../../utils.js" +import { getConfigDir, loadConfig } from "../../utils.js" export function telemetryUpdate(value: boolean) { let config = loadConfig() config.globalConfig.telemetry = value - fs.writeFileSync(__dirname + '/.fa/config.json', JSON.stringify(config.globalConfig, null, 2)) + fs.writeFileSync(getConfigDir() + '/.fa/config.json', JSON.stringify(config.globalConfig, null, 2)) return config } \ No newline at end of file diff --git a/src/utils.ts b/src/utils.ts index c2fa85f..c8358f0 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -270,6 +270,16 @@ export function isDirEmpty(path: string) { } } +/** + * Returns the base directory used for the global `.fa/config.json` file. + * Defaults to the directory containing this module, but can be overridden + * via FUSIONAUTH_CONFIG_DIR — primarily so tests can point at a real + * temporary directory instead of mocking the filesystem. + */ +export function getConfigDir(): string { + return process.env.FUSIONAUTH_CONFIG_DIR ?? __dirname +} + export function loadConfig() { const defaultConfig = { telemetry: true, @@ -277,10 +287,11 @@ export function loadConfig() { version: "1.0" } - const configPath = __dirname + '/.fa/config.json' + const configDir = getConfigDir() + const configPath = configDir + '/.fa/config.json' try { if (!fs.existsSync(configPath)) { - createConfig(__dirname + '/.fa', defaultConfig) + createConfig(configDir + '/.fa', defaultConfig) } const globalConfig = JSON.parse(fs.readFileSync(configPath).toString()) // TODO: Combine this with a local-project config @@ -365,7 +376,7 @@ export function createConfig(dir: string, configObject: ConfigObject = { id: ran type PropertyToAdd = {[key:string]: any} async function updateGlobalConfig(propertiesToAdd: PropertyToAdd | PropertyToAdd[]) { const config = loadConfig() - const configPath = __dirname + '/.fa/config.json' + const configPath = getConfigDir() + '/.fa/config.json' let newConfig: any; if (Array.isArray(propertiesToAdd)) {