Skip to content

refactor(multi-folder-scan): return ExitCode instead of calling process.exit()#822

Open
luojiyin1987 wants to merge 5 commits into
OWASP:mainfrom
luojiyin1987:refactor/multi-folder-exit-code
Open

refactor(multi-folder-scan): return ExitCode instead of calling process.exit()#822
luojiyin1987 wants to merge 5 commits into
OWASP:mainfrom
luojiyin1987:refactor/multi-folder-exit-code

Conversation

@luojiyin1987

@luojiyin1987 luojiyin1987 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

What

Replace 4 process.exit() calls in handleMultiFolderScan with return ExitCode.

This fixes the multi-folder audit-log close path by returning the exit code to the caller, so src/index.ts can close the audit log before exiting.

Why

  • Fixes audit-log fd leak: main()'s finally block can now run after multi-folder scans
  • Makes handleMultiFolderScan testable without mocking process.exit
  • Moves toward consistent exit code handling across the CLI

Changes

  • src/scan/multi-folder-scan.ts: Return EXIT_OK / EXIT_FINDINGS instead of calling process.exit(), change return type to Promise<ExitCode>
  • src/index.ts: Capture returned exit code and call process.exit() at the call site

Testing

  • TypeScript compiles cleanly
  • No behavior change (same exit codes, same console output)

Closes #821

@luojiyin1987 luojiyin1987 requested a review from sonukapoor as a code owner July 8, 2026 02:20

@sonukapoor sonukapoor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for this - moving process.exit() out of handleMultiFolderScan and into the call site is the right call. It makes the function composable and testable, and the mock change from mockImplementation(() => process.exit(0)) to mockResolvedValue(0) is much cleaner.

Two things before merging: the --fix and --sarif/--cdx unsupported-feature paths return EXIT_FINDINGS (1), but exit code 1 in this codebase means "findings were found above threshold." These paths never evaluate any findings - they bail early because the feature isn't available yet. That's EXIT_ERROR (3) per the contract in src/types.ts. A CI script testing exitCode === 1 would get a false positive here.

The return after process.exit(exitCode) in index.ts is also now unreachable dead code - small cleanup.

Minor: mockResolvedValue(EXIT_OK) would be more intentional than the raw 0, but not a blocker.

@luojiyin1987

Copy link
Copy Markdown
Collaborator Author

Thanks, fixed.

The unsupported multi-folder --fix / --sarif / --cdx paths now return EXIT_ERROR instead of EXIT_FINDINGS, since they bail out before evaluating findings.

I also removed the unreachable return after process.exit(exitCode), and changed the integration test mock to use EXIT_OK instead of a raw 0.

No new test file was added; I only added coverage to the existing multi-folder scan test file.

@sonukapoor sonukapoor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This addresses everything - EXIT_ERROR for the unsupported-feature paths is the right call, the dead return is gone, the test mock uses EXIT_OK, and the new handleMultiFolderScan tests cover all the EXIT_ERROR cases cleanly. The audit log close path is now correct too. Nice work.

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.

refactor(multi-folder-scan): return ExitCode instead of calling process.exit()

2 participants