You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
New application:create command to create an application in one of a few standard forms. Also supports --data for passing a JSON application definition to the API for full custom control.
Adds application:create with standard security profiles and custom JSON configuration, alongside CLI safety, automation, documentation, and test improvements.
Changes:
Adds application creation with profile defaults, CORS configuration, and custom JSON support.
Improves kickstart confirmation, unattended installation, and option naming.
Expands unit/integration coverage and supporting documentation.
File
Description
src/utils.ts
Adds reusable confirmation handling.
src/index.ts
Formatting-only change.
src/commands/kickstart-kill.ts
Adds --yes confirmation flow.
src/commands/kickstart-install.ts
Adds unattended installation options and validation.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
Global CORS changes lack required confirmation, while tenant targeting, origin configuration, validation, and macOS readiness contain functional defects.
Apply authorized origins to system CORS configuration
src/commands/application-create.ts:238
--authorized-origin-url is advertised as CORS configuration, but this only writes the application's OAuth authorizedOriginURLs. ensureCorsHeaders() updates global allowedHeaders and enabled, never corsConfiguration.allowedOrigins, so browser requests from a newly supplied origin remain blocked unless it was already configured separately. Merge these origins into system CORS (with deduplication) or change the option's contract.
Honor tenant ID supplied in application data
src/commands/application-create.ts:254
Honor a tenant supplied inside --data. The help says --tenant-id overrides --data, but the client receives only the flag value; without the flag, no tenant header is sent and multi-tenant FusionAuth instances may reject the request or target the API key's default tenant even when application.tenantId is present.
JSON.parse may return null, an array, or a scalar, but the cast accepts all of them as Application. For example, --data null then crashes at application.id, while arrays/scalars can be sent as malformed API payloads. Validate that the parsed value is a non-null, non-array object and return a clear input error before applying overrides.
- 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)
- 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
- 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
- Fix inverted localhost/container-IP fallback order in integration
test setup's auth-readiness check
- Gate CORS system-configuration mutation behind --yes/confirmOrExit
per the Risky Operations Policy
- Make --name optional; required only for --profile, preserved from
--data JSON unless explicitly overridden
- Document that applicationId/clientId are intentionally identical
(FusionAuth never accepts clientId as input)
- Remove NODE_ENV-conditional exit from executeApplicationCreate so
it always returns a result per its documented contract; thread the
raw error through to the CLI wrapper for field-level error detail
Resolve the service container ID via Compose instead of hard-coding its name
__tests__/integration/setup.js:24
This assumes Compose's default project name. If COMPOSE_PROJECT_NAME is set, the container has a different generated name, both docker inspect calls are silently ignored, and the bridge-IP fallback this change adds cannot work. Resolve the service container ID with docker compose --env-file .env.test ps -q fusionauth instead of hard-coding the generated name.
Use fileURLToPath and cwd to support checkout paths containing spaces
__tests__/integration/setup.js:25
URL.pathname leaves characters such as spaces percent-encoded, while the compose commands also interpolate this path into an unquoted cd. A checkout under a path containing spaces therefore fails either at writeFileSync(envFile, ...) or in the shell. Convert with fileURLToPath() and pass cwd: COMPOSE_DIR to execAsync rather than building cd command strings.
JSON.parse can return null, arrays, or primitives, but this unchecked cast lets them through as applications. null then fails at application.id, while other values can produce a non-object API payload, so syntactically valid --data input gets misleading runtime/API errors. Validate that the parsed value is a non-null, non-array object before casting it.
This states the opposite of the documented behavior in src/commands/application-create.ts:260-267: production calls may exit through confirmOrExit; only this test's mocked process.exit turns that path into a returned result. Update the comment so it does not reintroduce the contract that was intentionally removed.
JSON.parse can return null, arrays, or primitives, but parseData() cast
the result straight to Application unchecked. Traced the actual failure
modes: --data 'null' crashed downstream with an opaque
"Cannot read properties of null (reading 'id')" TypeError; arrays and
primitives silently passed through property assignments and produced
nonsensical API payloads sent to FusionAuth, surfacing as confusing
server-side errors instead of a clear client-side validation message.
Added a shape check right after JSON.parse, throwing a clear Error
consistent with parseData()'s other validation errors. Added three
unit tests covering null/array/primitive --data input.
Also fixed two stale comments:
- A test comment claiming confirmOrExit() "never exits the process
itself" — this directly contradicted the JSDoc on
executeApplicationCreate (and the earlier fix in 7bb5071): production
calls CAN still exit via confirmOrExit(); only this specific test's
mocked process.exit turns that into a returned result.
- The resolveResourcesDir() JSDoc (from 52ae42e) describing its src/
layout fallback as "npm start running this file via tsx", which my
very next commit (bd81c93, restoring the build-first start script)
made inaccurate. Reworded to describe direct source execution
generically, independent of npm start.
Re: "Validate parsed --data input is a non-null object" (application-create.ts:251, previously-missed finding, no dedicated inline thread).
Confirmed by tracing the actual failure modes: --data 'null' crashed downstream with an opaque Cannot read properties of null (reading 'id') TypeError; arrays and primitives silently passed through property assignments and produced nonsensical API payloads sent to FusionAuth, surfacing as confusing server-side errors instead of a clear client-side validation message.
Fixed in a648f1c: added a shape check right after JSON.parse in parseData(), throwing a clear error consistent with its other validation errors. Added 3 unit tests (null/array/primitive) and verified the full suite still passes.
Re: "Correct comment about confirmOrExit behavior" (__tests__/commands/application-create.test.js:756, previously-missed finding, no dedicated inline thread).
Agreed — this comment said the opposite of the documented contract. Fixed in a648f1c: reworded to accurately state that production calls can still exit via confirmOrExit() for a non-interactive caller without yes=true; this test only gets a returned result because process.exit is mocked.
This help text promises CORS behavior in every mode, but system CORS is changed only for --profile spa; in --data, native, and webapp modes this option only sets application.oauthConfiguration.authorizedOriginURLs (which the tests identify as a separate hosted-page allowlist). Clarify the scope so users do not assume this flag configured cross-origin API access when it did not.
The old text ('for CORS') implied this flag configures cross-origin
API access in every mode, but system CORS is only touched for
--profile spa. In --data, native, and webapp modes it only sets
application.oauthConfiguration.authorizedOriginURLs, a separate
application-level allowlist. Reworded to make the scope explicit.
Re: "Clarify CORS option scope across profiles" (application-create.ts:470, previously-missed finding, no dedicated inline thread).
Agreed — verified the actual scope across all modes: --profile spa sets application.oauthConfiguration.authorizedOriginURLs and adds the origin to the system CORS allowlist; --profile native, --profile webapp, and --data mode only set the application-level field, never touching system CORS.
Fixed in 2e4ae58: reworded the --help text to 'Authorized origin URLs for the application; also added to the system CORS allowlist for --profile spa'.
Avoid duplicate reporting of direct parseData errors
src/commands/application-create.ts:417
Direct Errors from parseData (malformed JSON, unreadable @file, invalid shape) are stored as both error and rawError. The action then passes both to errorAndExit, and reportError prints the same message once as msg and again as error.message. Preserve rawError only when unwrapping produced distinct structured detail (or the rejection was not already an Error).
Content-Type is only CORS-safelisted for application/x-www-form-urlencoded,
multipart/form-data, or text/plain -- not application/json. A SPA sending
JSON would still fail preflight after this command reported CORS as
"configured", since Content-Type wasn't in the guaranteed header set.
Added it to REQUIRED_CORS_HEADERS and updated the comments that
described these as "DPoP-related" headers (Content-Type is about JSON
bodies not being safelisted, not DPoP specifically). Updated test
fixtures that previously hardcoded the old 3-header "fully compliant"
set, and the integration test's local REQUIRED_CORS_HEADERS constant,
so they continue to validate the correct full set. Verified via a full
local docker-based integration run.
Also fixed unwrapError() printing the same error message twice.
Direct, never-wrapped Errors (e.g. parseData()'s validation errors)
have no distinct .cause, so unwrapError() fell back to returning the
same Error object as rawError -- which errorAndExit()/reportError()
then printed a second time via its generic 'message' in error branch.
Reproduced this empirically before and after the fix. unwrapError()
now returns undefined in that case, while still preserving a
genuinely-wrapped error's distinct cause, or a rejection that was
never an Error at all (e.g. a raw ClientResponse-shaped object).
Added a regression test asserting rawError is undefined for a direct
validation error.
Re: "Avoid duplicate reporting of direct parseData errors" (application-create.ts:417, previously-missed finding, no dedicated inline thread).
Confirmed empirically — reproduced the exact duplicate output described: reportError() printed the same message once directly and once via its generic 'message' in error branch, since unwrapError() fell back to returning the same (never-wrapped) Error object as rawError.
Fixed in ef9c0fa per the suggested approach: unwrapError() now returns undefined for a direct, never-wrapped Error (nothing extra to report beyond the message), while still preserving a genuinely-wrapped error's distinct .cause, or a rejection that was never an Error at all. Added a regression test asserting rawError is undefined for a direct validation error, and re-verified the duplicate output is gone.
This test only exercises the source fallback: it imports src/commands/kickstart-install.js through tsx, so __dirname is always src/commands, even when CI built dist first. The new dist/commands/resources branch is therefore untested despite this comment claiming both layouts are covered. Please test the resolver with controllable base directories (or import the built module in a dedicated build test) so both branches are exercised.
The existing test imported src/commands/kickstart-install.js via tsx,
so __dirname was always .../src/commands for the whole test run --
meaning the dist-layout branch (and the error-throw path) had zero
coverage, despite the test's comment claiming both layouts were
covered.
Added a baseDir parameter to resolveResourcesDir() (defaulting to the
real __dirname, so production behavior is unchanged) so tests can
exercise all three outcomes -- dist found, src fallback found, neither
found -- against controlled, synthetic temp directories instead of
depending on the real repo's build state.
Also added a separate test that imports the actual compiled
dist/commands/kickstart-install.js and verifies resolveResourcesDir()
resolves correctly against the real build output, confirming the
copy-files build step actually produces a working dist/commands/resources
directory. Skips gracefully (not fails) when dist/ hasn't been built
yet, so test:unit still works without requiring a build first --
meaningful in CI, which always builds before testing.
Re: "Test the dist resources path in addition to the source fallback" (kickstart-install.test.js:354, previously-missed finding, no dedicated inline thread).
Confirmed — this test always imports src/commands/kickstart-install.js via tsx, so __dirname was fixed to .../src/commands for the whole run. I verified directly that calling the function from that import only ever exercised the src-fallback branch; the dist-layout branch (and the error-throw path) had zero coverage despite the comment's claim.
Fixed in 1bd1254: added an injectable baseDir parameter to resolveResourcesDir() (defaulting to the real __dirname, so production behavior is unchanged), and replaced the old test with 4 synthetic temp-directory tests covering all three logic branches (dist found / src fallback found / neither found, including the previously-untested throw path). Also added a separate test that imports the real compiled dist/commands/kickstart-install.js directly and verifies it resolves correctly against the actual build output — this one skips gracefully (not fails) if dist/ hasn't been built yet, so test:unit still works without requiring a build first, while still being meaningful in CI (which always builds before testing).
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
The bridge-IP fallback depends on a generated Docker container name that is not stable across Compose project configurations.
Review effort: Balanced Findings: None
Previously missed (1)
In code that hasn't changed since last review
Resolve FusionAuth container ID via Compose instead of generated name
__tests__/integration/setup.js:24
This fallback relies on Docker Compose's generated container name, but the compose file does not declare container_name; the name changes when the project name is overridden (for example with COMPOSE_PROJECT_NAME). In that case both docker inspect calls silently fail and URL resolution falls back to localhost, defeating the new bridge-IP fallback. Resolve the fusionauth service container ID via docker compose ps -q fusionauth and inspect that ID in both lookup paths instead of hard-coding the generated name.
Two previously-missed findings from the same review, neither acted on
across two review cycles -- not a deliberate decision, just missed.
CONTAINER_NAME hard-coded Docker Compose's default generated container
name ('{project}-{service}-{index}'). If COMPOSE_PROJECT_NAME is set,
the real container name differs, both docker inspect calls silently
fail (caught by empty catch blocks), and the bridge-IP fallback this
PR added is defeated without any visible error. Replaced with
resolveContainerId(), which resolves the real ID via
`docker compose ps -q fusionauth`, independent of naming conventions.
Verified end-to-end via a full local docker-based integration run --
the bridge-IP fallback message still appears correctly, confirming
the dynamic resolution works.
COMPOSE_DIR used new URL(...).pathname, which leaves special characters
like spaces percent-encoded (e.g. '%20') rather than decoding them --
not a valid filesystem path component. Verified empirically that
fileURLToPath() correctly decodes it instead. Also replaced the
`cd ${COMPOSE_DIR} && ...` string-concatenation pattern (5 call sites)
with execAsync(cmd, { cwd: COMPOSE_DIR }), avoiding shell-quoting
issues with the path entirely rather than just moving them around.
Re: "Resolve FusionAuth container ID via Compose instead of generated name" (setup.js:24, previously-missed finding, now recurring across two review cycles).
For transparency: this one was not a deliberate decision to skip — it was simply missed. It only ever surfaced as a summary-only "previously missed" bullet with no dedicated inline thread to reply to directly, and it slipped through across both rounds.
Confirmed the issue is real: CONTAINER_NAME hard-coded Compose's default generated name; if COMPOSE_PROJECT_NAME is set, the real name differs and both docker inspect calls silently fail, defeating the bridge-IP fallback with no visible error.
Fixed in da3659d: added resolveContainerId(), which resolves the real container ID via docker compose ps -q fusionauth instead of guessing the name. Verified end-to-end via a full local docker-based integration run — the bridge-IP fallback still engages correctly.
While investigating, I also found a second, closely related finding from the same review that was equally unaddressed ("Use fileURLToPath and cwd to support checkout paths containing spaces", setup.js:25) and fixed that in the same commit: COMPOSE_DIR now uses fileURLToPath() instead of .pathname (verified empirically that .pathname left spaces percent-encoded rather than decoding them), and all 5 cd ${COMPOSE_DIR} && ... call sites now use execAsync(cmd, { cwd: COMPOSE_DIR }) instead, avoiding shell-quoting issues with the path entirely.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🔵 Needs a closer look
Duplicate supplied origins can pollute global CORS configuration, and new resource tests leak temporary directories.
Review effort: Balanced Findings: None
Previously missed (2)
In code that hasn't changed since last review
Deduplicate authorized origins before updating CORS configuration
src/commands/application-create.ts:195
When --authorized-origin-url contains the same origin more than once, each copy passes this filter because it is compared only with the pre-existing allowlist. The patch then writes duplicate entries into the system-wide CORS configuration. Deduplicate the supplied origins before computing the additions.
Each test in this block creates a directory under the system temp directory, but none of those directories are removed. Repeated local and CI runs therefore leave four directory trees behind per run; track them and clean them after each test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
New application:create command to create an application in one of a few standard forms. Also supports --data for passing a JSON application definition to the API for full custom control.