[ZiadEwis]

Summary

Fixes an issue where cve-lite config set ca-cert --json incorrectly accepts a CLI flag as a certificate path.

• Reuse the existing requireFlagValue helper to reject missing, empty, and flag-like path arguments.
• Add unit tests covering valid paths, missing paths, empty paths, and flag-like values.
• Add E2E regression tests verifying the expected exit code and error message.

Testing

• npm run lint:tests
• npm run build
• node dist/index.js advisories sync
• npm test — 201 test suites passed, 3047 tests passed.
• node scripts/typecheck-changed-tests.mjs upstream/main

Closes #1479

[coderabbitai]

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

• Bug Fixes
  • Improved validation for config set ca-cert: missing, empty, or flag-like paths now produce a clear required-path error instead of being saved.
  • Dash-prefixed relative paths are accepted when written with a ./ prefix, and invalid input no longer writes to the configuration file.

config set ca-cert now rejects missing, empty, or flag-prefixed paths with a required-path error. A dash-prefixed relative filename such as ./-cert.pem remains accepted.

Changes: CA certificate path validation

Layer / File(s)	Summary
Validate config set CA certificate paths src/cli/args.ts, tests/cli/config-command.test.ts, tests/e2e/commands-and-exit-codes.test.ts	The parser uses requireFlagValue to validate the CA certificate path. Parser tests cover valid paths and missing, empty, or flag-prefixed values. End-to-end tests verify error output, exit status, config-file behavior, and saving a valid dash-prefixed relative path.

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

Change: Bug fix · Severity of issue fixed: Low

Possibly related PRs

• OWASP/cve-lite-cli#1478: Both changes use required-value validation for CA certificate paths in separate command paths.
• OWASP/cve-lite-cli#1466: This change uses the shared requireFlagValue helper.
• OWASP/cve-lite-cli#1417: Both changes validate CA certificate paths with requireFlagValue in separate command paths.

Merge Risk: Minimal · up to a4a17

The change prevents flags from being saved as certificate paths while preserving valid paths such as ./-cert.pem. No actionable merge-blocking risk remains; test execution was not independently verified.

Pre-merge checks: 7

Check name	Status	Explanation
Title check		I think this title is a pass. It follows the type: description form with the allowed fix type, and it is lowercase with no trailing period. It names the change: the CLI now rejects flags as the ca-cer…
Description check		The description covers the summary of the change, the reason for it, and the testing done. It links the issue, which closes #1479, and the template only needs a brief description with a Closes line. T…
Linked Issues check		For users, cve-lite config set ca-cert --json now fails. It exits with status 1 and prints cve-lite config set ca-cert requires a path argument. Before this change, the parser accepted --json as…
Out of Scope Changes check		The whole-PR diff against the merge base changes three files: src/cli/args.ts, tests/cli/config-command.test.ts, and tests/e2e/commands-and-exit-codes.test.ts. In src/cli/args.ts, the diff at…
Linked Issue		The pull request body references GitHub issue #1479 with the supported keyword “Closes #1479”.
Closes Keyword Matches Scope		The body uses Closes #1479. That issue asks config set ca-cert to reject missing, empty, and flag-like certificate paths, including --json. The changed parser now uses requireFlagValue, which …
No New Runtime Dependency		The pull request changes only src/cli/args.ts and two test files. It does not modify package.json. The base and head dependency sections are identical, so no new runtime dependency or dev-to-runti…

Full details: Title check

Explanation

I think this title is a pass. It follows the type: description form with the allowed fix type, and it is lowercase with no trailing period. It names the change: the CLI now rejects flags as the ca-cert path argument. This matches the summary, which shows requireFlagValue now guards the ca-cert value in config set.

Full details: Description check

Explanation

The description covers the summary of the change, the reason for it, and the testing done. It links the issue, which closes #1479, and the template only needs a brief description with a Closes line. The description meets the template. It explains the change in plain terms: the config set ca-cert command no longer takes a flag as a certificate path. It also lists the unit and E2E tests added and the checks run.

Full details: Linked Issues check

Explanation

For users, cve-lite config set ca-cert --json now fails. It exits with status 1 and prints cve-lite config set ca-cert requires a path argument. Before this change, the parser accepted --json as the certificate path. [#1479] asks the parser to reject a path argument that is empty or starts with -.

Implementation at the reviewed head: In src/cli/args.ts, the config set branch checks the key first. It then calls requireFlagValue("cve-lite config set ca-cert", argv[3], "path"). The helper throws when the value is undefined, empty, or starts with -. The old if (!value) check rejected undefined and empty values, but it accepted flag-like values. That gap was the reported defect. The issue text says empty values are not rejected. The original code already rejected the empty string, so the flag-like case is the real gap, and the change covers both.

Tests: The change summary for tests/cli/config-command.test.ts says it covers valid paths, missing paths, empty paths, and flag-like values. The change summary for tests/e2e/commands-and-exit-codes.test.ts says it covers a flag used as the path, a missing path, and a valid dash-prefixed relative path (./-cert.pem). I could not read these test blobs at the reviewed head in this review, because the changed blobs are missing from the checkout. The test coverage therefore rests on the change summary and the previous assessment. The author reports that npm test passed with 201 suites and 3047 tests. I did not run the tests.

No unmet coding requirement was found.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff against the merge base changes three files: src/cli/args.ts, tests/cli/config-command.test.ts, and tests/e2e/commands-and-exit-codes.test.ts.

In src/cli/args.ts, the diff at the reviewed head is one hunk in the config set branch. It moves the const value = argv[3]; read to the line that calls requireFlagValue, removes the old falsy check and its throw, and adds the requireFlagValue call. The key check still runs before the value check, so the behavior for valid keys does not change. The line count (+1/-4) matches the change summary. This edit directly implements [#1479].

The test changes cover only config set ca-cert path parsing and its E2E output. The summaries show no unrelated code. I could not read the test diffs in this review, so that conclusion rests on the change summaries and the previous assessment.

No unrelated change was demonstrated.

Full details: Closes Keyword Matches Scope

Explanation

The body uses Closes #1479. That issue asks config set ca-cert to reject missing, empty, and flag-like certificate paths, including --json. The changed parser now uses requireFlagValue, which rejects all three cases. The added unit and E2E tests cover these cases and confirm the required error and exit status. The PR also preserves valid paths such as ./-cert.pem.

Full details: No New Runtime Dependency

Explanation

The pull request changes only src/cli/args.ts and two test files. It does not modify package.json. The base and head dependency sections are identical, so no new runtime dependency or dev-to-runtime move was introduced.

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

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

CodeRabbit Verify

• Run CodeRabbit Verify to test the changes in this pull request.

Recent review info

Automatic approval policy

Commit: a4a17898a5ea.

Setting	Effective value	Source
Automatic approvals	Enabled	Global configuration
Merge risk	Moderate	Global configuration
Security risk	Not set	Global configuration

No overrides matched. The global limits apply.

Not set means no limit was configured after inheritance. Use @coderabbitai configuration to inspect the resolved settings and their sources.

Run configuration

• Source mode: Auto
• Repository: OWASP/cve-lite-cli
• Branch: main
• File: .coderabbit.yaml
• Commit: 280f098
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: b1928426-b9b1-49a0-9446-e952759ebb33

Commits

Reviewing files that changed from the base of the PR and between af6a2a0 and a4a1789.

Files selected for processing: 2

• tests/cli/config-command.test.ts
• tests/e2e/commands-and-exit-codes.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

[sonukapoor]

Thanks @ZiadEwis, and welcome. Reusing the shared helper was the right call: --json, a missing value, an empty value and -cert.pem all stop with the existing requires a path argument message and nothing is saved, while a path with spaces, a relative path and ./-cert.pem still save fine. Putting the old check back fails exactly your two new tests. Approved.

One optional thing, no need to hold it for this: could the two new e2e tests pass { env: { HOME: scratch() } } like the round-trip test above them, and assert that .cve-lite-cli/config.json was not created? That pins the "nothing saved on rejection" half, and keeps a future regression from writing a real user's config during the test run.

[ZiadEwis]

Thanks for the suggestions!

I've pushed the additional test improvements:

• Updated both rejection E2E tests to use an isolated temporary HOME.
• Added assertions to verify that config.json is not created when validation fails.
• Added unit and E2E coverage to ensure that valid relative paths such as ./-cert.pem are still accepted and saved correctly.

No production code changes were needed, as the existing validation already handles ./-cert.pem correctly.

All checks are passing, including the full test suite (3,049 tests).

[sonukapoor]

Thanks for isolating HOME and checking that nothing gets saved, that was exactly the gap. The relative-path case is a nice addition too. Approved. I merge these one at a time rather than in a batch, so it will land shortly.

[sonukapoor]

Merged - thank you @ZiadEwis!