[Nachiket-Roy]

What changed and why

This pull request introduces a validation to prevent writing the advisory database to stdout, which is not supported. The main change is to reject the use of - (dash) as the output path for the advisory database, ensuring users provide a valid file path. The tests have been updated to reflect and verify this new behavior.

Validation and error handling improvements:

• Added a check in getDefaultAdvisoryDbPath (in src/advisory/osv-sync.ts) to throw an error if the output path is -, preventing attempts to write the advisory database to stdout. [
• Updated CLI and internal tests to verify that using --output - is rejected with a clear error message, and removed previous test logic that incorrectly allowed this scenario.

Closes #1655

[coderabbitai]

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

• Bug Fixes
  • The advisory sync command now rejects - as a database output path and directs users to choose a database file path.

--output - now fails instead of creating a database file named -. The error directs callers to choose a database file path.

Changes: Advisory database output

Layer / File(s)	Summary
Reject stdout database output src/advisory/osv-sync.ts, tests/osv-sync.test.ts, tests/cli/advisories-sync.test.ts	getDefaultAdvisoryDbPath rejects - before resolving a file path. Tests cover the helper, sync function, and CLI error.

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

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: Minimal · up to 5706a

Passing "--output -" to advisories sync now fails with a clear error instead of silently creating a file named "-". The change is small and easily reversible, and no merge-blocking risk was found.

Pre-merge checks: 7

Check name	Status	Explanation
Title check		The title follows the required "type: short description" form with a valid type (fix), lowercase after the colon, and no trailing period. It clearly names the main change: rejecting stdout (-) as th…
Description check		The description follows the repository template's "What changed and why" section and includes a "Closes #1655" line. It explains the new validation in getDefaultAdvisoryDbPath, the reason for it (st…
Linked Issues check		I checked the whole-PR diff from merge base bd4b051 to head 5706a4e. Issue #1655 asks that advisories sync --output - reject stdout instead of creating a literal - file. The change in `src/advisor…
Out of Scope Changes check		All changes connect to #1655. The osv-sync.ts change adds the stdout rejection. The import of isOutputStdout from src/utils/string.js supports that guard. The helper is already present at the ba…
Linked Issue		The pull request description ends with "Closes #1655". This uses the Closes keyword followed by an issue number, which meets the check. The linked issue is #1655, and the objectives section confirms t…
Closes Keyword Matches Scope		I checked the PR body, which uses "Closes #1655". I read issue #1655 from the supplied linked-issue context. Its only ask is that advisories sync --output - stop creating a literal file named -. I…
No New Runtime Dependency		I checked the reviewed range for package.json changes. The changed-file list has three files: src/advisory/osv-sync.ts, tests/cli/advisories-sync.test.ts, and tests/osv-sync.test.ts. package.json does…

Full details: Title check

Explanation

The title follows the required "type: short description" form with a valid type (fix), lowercase after the colon, and no trailing period. It clearly names the main change: rejecting stdout (-) as the advisory database output. For users, this means --output - now fails with a clear error instead of creating a literal ./- file.

Full details: Description check

Explanation

The description follows the repository template's "What changed and why" section and includes a "Closes #1655" line. It explains the new validation in getDefaultAdvisoryDbPath, the reason for it (stdout is unsupported for the SQLite database), and the updated tests. The template's placeholder comment is still present and the closing reference is filled in, so the description is mostly complete. One bullet also ends with a stray "[" character, which should be cleaned up.

Full details: Linked Issues check

Explanation

I checked the whole-PR diff from merge base bd4b051 to head 5706a4e. Issue #1655 asks that advisories sync --output - reject stdout instead of creating a literal - file. The change in src/advisory/osv-sync.ts adds a guard to getDefaultAdvisoryDbPath that calls isOutputStdout(outputPath) and throws Advisory database output cannot be stdout ('-'). Choose a database file path. The guard runs before path resolution, so no file is created for -. The CLI test in tests/cli/advisories-sync.test.ts now asserts that runAdvisoriesSync rejects with that message. The removed test had expected a literal - file, so it no longer conflicts with the requirement. The tests/osv-sync.test.ts case was reviewed from the summary only. This is coding evidence for the requirement, and it is consistent with the code. No unmet coding requirement remains for #1655.

Full details: Out of Scope Changes check

Explanation

All changes connect to #1655. The osv-sync.ts change adds the stdout rejection. The import of isOutputStdout from src/utils/string.js supports that guard. The helper is already present at the base, so the import is the only new dependency. The CLI test replaces the old literal-file test for --output -. The tests/osv-sync.test.ts addition checks the same guard at the function level. I found no unrelated refactoring or unrelated file changes in the reviewed diff.

Full details: Linked Issue

Explanation

The pull request description ends with "Closes #1655". This uses the Closes keyword followed by an issue number, which meets the check. The linked issue is #1655, and the objectives section confirms that the PR body names it as a direct link. No code evidence is needed for this check, so I did not inspect the diff. I read this as a maintainer would. The PR links its work to the bug report, so reviewers can trace why the change exists. The author should keep that line in the body when the PR merges, so GitHub closes the issue automatically.

Full details: Closes Keyword Matches Scope

Explanation

I checked the PR body, which uses "Closes #1655". I read issue #1655 from the supplied linked-issue context. Its only ask is that advisories sync --output - stop creating a literal file named -. It should reject stdout, because a SQLite advisory database cannot be piped to stdout. The linked-issue objectives state the requested outcome is to reject this output path. The PR delivers that item: - src/advisory/osv-sync.ts: getDefaultAdvisoryDbPath now throws "Advisory database output cannot be stdout ('-'). Choose a database file path." when isOutputStdout(outputPath) is true. isOutputStdout trims input and tests for -. - src/cli/advisories-sync.ts calls getDefaultAdvisoryDbPath(options.output) on its first line, before the spinner starts and before any sync work. syncOsvAdvisories also calls it before rejectDirectoryOutput and before any directory or file creation. The CLI therefore rejects --output - before a - file can be written. - Tests: tests/cli/advisories-sync.test.ts expects runAdvisoriesSync to reject --output - with the new message. tests/osv-sync.test.ts checks both getDefaultAdvisoryDbPath("-") and syncOsvAdvisories({ outputPath: "-" }). The old test that allowed the literal - file was removed. The issue has no other requested items. Every item it asks for is delivered, so the Closes keyword matches the scope. The check passes.

Full details: No New Runtime Dependency

Explanation

I checked the reviewed range for package.json changes. The changed-file list has three files: src/advisory/osv-sync.ts, tests/cli/advisories-sync.test.ts, and tests/osv-sync.test.ts. package.json does not appear in the list. The patch for package.json is empty, and no lockfile or other package manifest changed either. The PR adds no entry to "dependencies" or "optionalDependencies", and it does not move a package from devDependencies to dependencies. The check therefore passes. For users, this change is small. The CLI now rejects --output - with a clear error. It no longer creates a literal file named -. No new runtime package comes with the fix.

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

Recent review info

Automatic approval policy

Commit: 5706a4e760f7.

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: bd4b051
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 5bd458ff-94d8-4443-b586-b3a91813a59a

Commits

Reviewing files that changed from the base of the PR and between bd4b051 and 5706a4e.

Files selected for processing: 3

• src/advisory/osv-sync.ts
• tests/cli/advisories-sync.test.ts
• tests/osv-sync.test.ts

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