Skip to content

fix: emit audit-log lifecycle for multi-folder scans#887

Open
rksharma-owg wants to merge 1 commit into
OWASP:mainfrom
rksharma-owg:codex/fix-multifolder-audit-log
Open

fix: emit audit-log lifecycle for multi-folder scans#887
rksharma-owg wants to merge 1 commit into
OWASP:mainfrom
rksharma-owg:codex/fix-multifolder-audit-log

Conversation

@rksharma-owg

Copy link
Copy Markdown

Fixes #879

Summary

  • Emit scan.started and scan.finished for multi-folder scans when audit logging is enabled.
  • Route multi-folder override audits to the same audit log handle as the scan run.
  • Add regression coverage for audit-log forwarding and lifecycle events.
  • Includes one e2e coverage test for multi-folder --check-overrides --audit-log.

Validation

  • Unit tests: tests/multi-folder-scan.test.ts (pass)
  • e2e: tests/e2e/fix-and-auditlog.test.ts (blocked by existing build typing issue: src/utils/network.ts preconnect typing in npm run build, pre-existing on main)

@rksharma-owg
rksharma-owg requested a review from sonukapoor as a code owner July 25, 2026 01:17

@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 @rksharma-owg - this is the right fix for #879, and much cleaner than the earlier attempt: routing the per-folder override audit to the real audit-log handle instead of NULL_AUDIT_LOG is exactly it, and it is well tested.

Two things before it goes in:

  1. The scan.finished lifecycle only fires on 2 of the 4 exit paths in handleMultiFolderScan (src/scan/multi-folder-scan.ts). The --fix-unsupported and --sarif/--cdx-unsupported early returns emit scan.started but return without a matching scan.finished, so a consumer cannot pair start/finish for exactly those flag combos (which are common in CI). The single-folder path in index.ts pairs finish on every early return - could you add the two scan.finished emits on those paths, with a test? (No fd leak here, to be clear - close() is called unconditionally after the function returns, so this is an audit-completeness gap, not a resource one.)

  2. Same as your other PRs: this bundles the unrelated overrides: brace-expansion + jest commit. We already resolved that advisory via the baseline in #897, so please drop that commit and rebase.

The core routing fix is solid and should survive as-is once those are addressed.

@rksharma-owg
rksharma-owg force-pushed the codex/fix-multifolder-audit-log branch from 145691a to 2c63666 Compare July 26, 2026 19:31
@rksharma-owg

Copy link
Copy Markdown
Author

Thanks for the review. I rebased onto current main, removed the unrelated dependency commit, and added matching scan.finished events for the unsupported --fix and --sarif/--cdx exits. The new parameterized coverage verifies all three flags return EXIT_ERROR with paired lifecycle events.

Validation:

  • tests/multi-folder-scan.test.ts — 26/26 passed
  • git diff --check — passed

The local full build still encounters the unrelated typeof fetch/preconnect error tracked in #885 and #886. CI is running on commit 2c63666.

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: multi-folder --audit-log is a silent no-op

2 participants