Skip to content

Fix #1213: pass JAVA_OPTS as argv array instead of interpolating it into a shell string - #1293

Open
Inf1mum wants to merge 5 commits into
OpenAPITools:masterfrom
Inf1mum:fix/1213-java-opts-shell-split
Open

Inf1mum wants to merge 5 commits into
OpenAPITools:masterfrom
Inf1mum:fix/1213-java-opts-shell-split

Conversation

@Inf1mum

@Inf1mum Inf1mum commented Nov 7, 2026 •

Copy link
Copy Markdown

Summary

Fixes #1213.

pass-through.service.ts built the java command by joining ['java', process.env['JAVA_OPTS'], '-jar "..."', ...] into a single string and spawning it with shell: true. Any character that is meaningful to a shell therefore changed what was executed. The pipe in a legitimate JAVA_OPTS like

JAVA_OPTS=-Dhttp.proxyHost=localhost -Dhttp.proxyPort=9000 -Dhttps.proxyHost=localhost -Dhttps.proxyPort=9000 -Dhttp.nonProxyHosts=127.*|localhost|*.local

was interpreted as a shell pipe, so the CLI failed with

Error: 'localhost' is not recognized as an internal or external command

This also affected the exec()-based command discovery (help/completion are probed at startup), i.e. every CLI invocation crashed before a command even ran. Joining args into one string additionally triggered the Node.js deprecation warning DEP0190 (passing args to a shell: true spawn).

What changed

  • cmd() now returns a proper argv array instead of a shell string: ['java', ...splitArgs(JAVA_OPTS), '-jar', cliPath] (and ['docker', 'run', '--rm', '-v', cwd:/local, image] in docker mode).
  • JAVA_OPTS is split by a lightweight tokenizer that respects single quotes, double quotes and backslash escapes, so -Dhttp.nonProxyHosts=127.*|localhost (quoted or unquoted) arrives at the java process as one verbatim argument.
  • passThrough() and the help/completion discovery now spawn with an args array and shell: false; stdio: 'inherit' and the exit-code handling stay the same. Removing the shell eliminates the entire class of escaping problems (including DEP0190) instead of trying to quote for cmd.exe/POSIX at once.
  • Quoting the jar/classpath arguments is no longer necessary (paths with spaces keep working, as each path is a single argv entry).

Note: version-manager.service.ts still spawns docker pull/docker rmi via a shell string — those commands contain no user-controlled input, so I left them untouched to keep this change focused.

Reproduction / verification

# before: Error: 'localhost' is not recognized as an internal or external command
JAVA_OPTS='-Dhttp.nonProxyHosts=127.*|localhost' openapi-generator-cli help

# after: prints usage, exit code 0
JAVA_OPTS='-Dhttp.nonProxyHosts=127.*|localhost' openapi-generator-cli help

The second case was verified against the built CLI (Node 24, Windows): it downloads the selected generator, runs the help/completion discovery via java and prints the usage text with exit code 0.

Tests

  • pass-through.service.spec.ts now mocks spawn (returning an EventEmitter child) for both the discovery calls and the passthrough spawn, and asserts the exact argv arrays.
  • New cases: JAVA_OPTS with an unquoted pipe is passed as a single argument, and quoted JAVA_OPTS (-Dhttp.nonProxyHosts="127.*|localhost|*.local", -Dsome.opt='a b') is split into separate arguments — both with shell: false.
  • Existing cases updated from the string+options form to the spawn(command, args, options) form, keeping the same scenarios (docker, plain, JAVA_OPTS, custom generator with/without the standard jar).
  • npx nx test generator-cli: 255 tests passing; npx nx lint generator-cli clean.

Summary by cubic

Fixes every CLI command crashing when JAVA_OPTS contains shell-special characters. The java command was built by interpolating JAVA_OPTS into a single string and spawned with shell: true, so a legitimate value like -Dhttp.nonProxyHosts=127.*|localhost was parsed as a pipe and failed startup; commands are now spawned as an argv array with shell: false. JAVA_OPTS is split by a tokenizer that respects quotes; unquoted backslashes are kept verbatim so Windows paths like C:\certs\client.jks survive, while backslash escapes are interpreted only inside double quotes.

Details

  • help/completion discovery now uses spawn with an args array instead of exec, and decodes output as utf8 so multi-byte characters split across chunks aren't rewritten as replacement characters.
  • Quoting jar/classpath paths is no longer needed since each path is a single argv entry.
  • version-manager docker pull/rm still uses shell strings since they contain no user-controlled input.

Written for commit 5091168. Summary will update on new commits.

View guided diff

JAVA_OPTS was interpolated into a single command string which was
spawned with shell: true. Characters that are meaningful to a shell -
most notably the pipe in -Dhttp.nonProxyHosts=127.*|localhost - were
therefore interpreted by the shell instead of reaching the java
process, breaking every command of the CLI (the command discovery via
exec failed the same way). Joining the args into one string also
triggered the Node.js DEP0190 deprecation warning.

The command is now built as a proper argv array: JAVA_OPTS is split
with a lightweight tokenizer that respects single/double quotes and
backslash escapes, and java (or docker) is spawned with shell: false.
This removes the whole class of shell-escaping problems.

Fixes OpenAPITools#1213

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 2 files

Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic

splitArgs treated a backslash outside of quotes as an escape character
the way a POSIX shell would, so a Windows path in JAVA_OPTS such as
C:\certs\client.jks reached java as C:certsclient.jks. Unquoted
backslashes are now kept verbatim; escaping is only interpreted inside
double quotes, where it follows the typical shell rules.
…A_OPTS

Within a double-quoted JAVA_OPTS segment a backslash closed nothing and
a following quote always terminated the segment, so -Dfoo="a\"b"
was passed to java as -Dfoo=a\b" with the trailing quote leaking out
of the quotes. A backslash inside double quotes now escapes exactly the
characters a typical shell lets it escape (quote, backslash, dollar,
backtick), while single-quoted segments keep treating the backslash as
a literal character.
The discovery of getCommands() was only exercised through a mock keyed
on the joined command line; the first two spawn calls were never
verified. The spec now asserts that help and completion are spawned on
the exact argv (java, jar path, sub command) with { shell: false }, and
the discovery beforeEach clears JAVA_OPTS so earlier tests cannot leak
options into the asserted argv.
The discovery run() accumulated stdout/stderr from Buffer chunks, so a
multi-byte utf8 character split across two chunks was decoded twice and
turned into replacement characters, corrupting non-ASCII help or error
text. Both streams now set utf8 encoding before the data handlers are
attached, and the spec asserts the encoding on the discovery streams.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] JAVA_OPTS is incorrectly evaluated

1 participant