test(cli): invoke the CLI without a shell

[clay-good]

Status: Test-only. Clears the last 25 code scanning alerts.

What was wrong

Twenty-five test call sites built a command string and handed it to execSync, which runs it through a shell:

execSync(`node ${openspecBin} spec show auth --json`, { encoding: 'utf-8' })

The interpolated value is a constant path in every case, so nothing was exploitable. But it is the pattern CodeQL reports as js/shell-command-injection-from-environment, and it accounts for every remaining alert on the repository.

How it was fixed

Each call passes an argument array to execFileSync, which never involves a shell:

execFileSync('node', [openspecBin, 'spec', 'show', 'auth', '--json'], { encoding: 'utf-8' })

File	Calls
test/commands/spec.test.ts	15
test/commands/show.test.ts	5
test/commands/validate.enriched-output.test.ts	1
test/commands/spec.interactive-show.test.ts	1
test/commands/spec.interactive-validate.test.ts	1
test/commands/change.interactive-show.test.ts	1
test/commands/change.interactive-validate.test.ts	1

Why this is safe

• Nothing ships. test/ is not in package.json files. Packing the tarball gives 362 files, zero from test/.
• No shell features to lose. Every command was node ${bin} followed by plain literal arguments or one simple variable — no pipes, redirects, chaining, or globs.
• Error handling is unchanged. Both APIs reject with the same spawnSync error carrying status and stderr, which these tests assert on. The error-path tests pass, which is direct evidence the subprocess still runs and still fails as expected.
• A path containing spaces is now passed as one argument instead of being word-split — more correct, and no test depends on the old behavior.

Proof

Check	Result
Full suite	2196 pass, 111 files — identical count, nothing skipped or lost
The 7 converted files	25 tests pass, real subprocess timings (~330-400ms each)
Build / lint	clean (one pre-existing warning, untouched)
execSync remaining in these files	none

No changeset: test-only, nothing to release.

Note

Unlike the guard change in #1425, this one is a categorical fix rather than a bet on the analyzer's dataflow. js/shell-command-injection-from-environment targets shell-invoking APIs; execFileSync without shell: true is not one, so the rule should no longer apply at all. Still only confirmable once CodeQL re-analyzes main.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Improved CLI test execution reliability by using direct process invocation instead of shell command strings.
  • Preserved coverage for successful output, validation failures, missing arguments, JSON responses, and error messages across show, list, and validate commands.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d780c3f5-9bbf-46b8-a9de-e85e38d6db3d

📥 Commits

Reviewing files that changed from the base of the PR and between 040a869 and 2f4c3b1.

📒 Files selected for processing (7)

• test/commands/change.interactive-show.test.ts
• test/commands/change.interactive-validate.test.ts
• test/commands/show.test.ts
• test/commands/spec.interactive-show.test.ts
• test/commands/spec.interactive-validate.test.ts
• test/commands/spec.test.ts
• test/commands/validate.enriched-output.test.ts

---

📝 Walkthrough

Walkthrough

The CLI test suite replaces shell-based execSync calls with execFileSync invocations using explicit Node arguments. Existing command outputs, error statuses, stderr checks, and JSON assertions remain unchanged.

Changes

CLI test invocation

Layer / File(s)	Summary
Interactive command test invocations test/commands/change.interactive-*.test.ts, test/commands/spec.interactive-*.test.ts, test/commands/validate.enriched-output.test.ts	Interactive and enriched-output tests run CLI commands through execFileSync with explicit argument arrays while preserving failure assertions.
CLI command suite invocations test/commands/show.test.ts, test/commands/spec.test.ts	Show, list, validate, JSON, and error-handling tests use structured arguments while preserving existing output and status assertions.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: tabishb

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: test CLI invocations now avoid shell execution by using execFileSync.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch chore/execfile-in-tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[alfred-openspec]

Approved at exact head 2f4c3b1. All 25 conversions preserve argument order and subprocess error behavior without relying on shell features; the seven affected files pass 25 real subprocess tests locally, the full exact-head cross-platform/security/CodeQL matrix is green, and the test-only no-changeset scope is correct.