Skip to content

feat: add --siem flag to security-audit report for NDJSON export#1876

Open
jlima8900 wants to merge 4 commits intoKeeper-Security:releasefrom
jlima8900:feat/siem-output-format
Open

feat: add --siem flag to security-audit report for NDJSON export#1876
jlima8900 wants to merge 4 commits intoKeeper-Security:releasefrom
jlima8900:feat/siem-output-format

Conversation

@jlima8900
Copy link

@jlima8900 jlima8900 commented Mar 14, 2026

Summary

Adds SIEM-ready output to security-audit report using NDJSON format (one JSON event per line). The existing audit-log command has SIEM export (Splunk, syslog, Sumo, Azure) for event streaming, but security-audit report (posture snapshots) has no SIEM output.

Usage

security-audit report --siem --output posture.ndjson
security-audit report --siem --breachwatch --output breach.ndjson

Output format (NDJSON — one line per user)

{"event_type":"keeper.security_audit","timestamp":"2026-03-14T22:30:00+00:00","source":"keepersecurity.eu","user":{"email":"user@company.com","securityScore":52,"weak":8,"reused":6,"twoFactorChannel":"none"},"security_score":52,"risk_factors":["weak_passwords","reused_passwords","no_2fa"]}

Design decisions

  • NDJSON not wrapped JSON — supports streaming ingestion. A 10k user enterprise produces 10k lines, not one giant blob. Compatible with jq -c, Filebeat, Splunk HEC, Datadog logs.
  • --siem flag not --format siem — avoids modifying the shared report_output_parser used by all commands. Clean addition, no side effects.
  • Risk factors from data, no hardcoded severity — the output includes risk_factors and security_score. SIEM correlation rules should define what's critical for each organization, not Commander.
  • .ndjson extension — standard for newline-delimited JSON files.

No breaking changes

  • Existing formats (table, csv, json, pdf) unchanged
  • --siem is additive — only affects output when explicitly used
  • audit-log SIEM targets unaffected

Test plan

  • --siem produces valid NDJSON (one JSON object per line)
  • Each line parses independently with jq
  • --breachwatch --siem includes at_risk/passed/ignored
  • --output writes to file with .ndjson extension
  • Without --output, prints to stdout
  • Existing --format json output unchanged

@jlima8900 jlima8900 force-pushed the feat/siem-output-format branch from 0460eea to 465e68a Compare March 15, 2026 03:34
@jlima8900 jlima8900 changed the title feat: add --format siem option to security-audit report for SIEM ingestion feat: add --siem flag to security-audit report for NDJSON export Mar 15, 2026
@jlima8900 jlima8900 force-pushed the feat/siem-output-format branch from 465e68a to d46b168 Compare March 15, 2026 05:21
@craiglurey craiglurey changed the base branch from master to release March 20, 2026 05:51
@craiglurey craiglurey requested a review from sk-keeper March 20, 2026 05:52
report_title = f'Security Audit Report{" (BreachWatch)" if show_breachwatch else ""}'

# SIEM-ready NDJSON output
if kwargs.get('siem'):
Copy link
Contributor

Choose a reason for hiding this comment

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

This gets silently ignored when --record-details is set, because the method returns from the record-detail branch before it ever reaches the SIEM branch. If we want to support that combination, it needs handling there; otherwise we should reject the flag combo explicitly instead of accepting it and returning the normal record-detail report.

report = '\n'.join(ndjson_lines) + '\n'

if filename:
if not os.path.splitext(filename)[1]:
Copy link
Contributor

@aaunario-keeper aaunario-keeper Mar 20, 2026

Choose a reason for hiding this comment

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

This silently bypasses the shared --format / report-output path. Once --siem is set, fmt no longer governs behavior, and --output is honored even though the shared parser help says output is ignored for table mode. The main issue here is the lack of validation or explicit contract for how --siem interacts with --format and the normal report-output path. I would either validate the combination and fail closed, or make the override explicit in the CLI contract.

}
ndjson_lines.append(json.dumps(event, default=str))

report = '\n'.join(ndjson_lines) + '\n'
Copy link
Contributor

Choose a reason for hiding this comment

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

For the empty case this produces a single blank line, not zero NDJSON records, because "\n".join([]) + "\n" is just "\n". That is not actually "one JSON object per line" output, and some ingest pipelines will treat the blank line as a parse error. We should special-case the empty report to emit an empty string/file instead.

filename += '.ndjson'
logging.info('SIEM report path: %s', os.path.abspath(filename))
with open(filename, 'w') as f:
f.write(report)

Check failure

Code scanning / CodeQL

Clear-text storage of sensitive information

This expression stores [sensitive data (password)](1) as clear text.
@jlima8900 jlima8900 force-pushed the feat/siem-output-format branch 2 times, most recently from 1be7358 to 978cb83 Compare March 23, 2026 14:04
Adds NDJSON output for SIEM ingestion (Splunk, Elastic, Datadog).

- --siem flag produces one JSON object per line
- Rejects --siem + --record-details combination
- Warns when --siem overrides explicit --format
- Empty reports produce empty output
- security_score field only emitted in non-BreachWatch mode
- Output file written with 0o600 permissions
@jlima8900 jlima8900 force-pushed the feat/siem-output-format branch from 978cb83 to 5f59cb2 Compare March 23, 2026 15:51
Address PR Keeper-Security#1876 review from @aaunario-keeper:

- --siem + --format now raises CommandError instead of silently ignoring
  the format flag, making the override explicit in the CLI contract
- Add clarifying parentheses on empty-report ternary to make precedence
  obvious (behavior unchanged — empty list already produced '')
@jlima8900
Copy link
Author

Good catches across the board — addressing each:

--siem + --record-details conflict: You're right, --siem is silently ignored when --record-details is set because it returns early. I'll add explicit validation that rejects the combination with a clear error message rather than silently dropping the flag.

--siem bypassing --format/output path: Agreed this needs an explicit contract. I'll add validation that rejects --siem with --format (other than the default) and document in the CLI help that --siem produces NDJSON exclusively, bypassing the normal format/output pipeline.

Empty report blank line: Good edge case — "\n".join([]) + "\n" does produce "\n" instead of empty output. I'll special-case the empty report to emit an empty string/file so ingest pipelines don't choke on a stray blank line.

Clear-text storage (CodeQL): Will review the flagged line and ensure sensitive fields are masked or excluded from the NDJSON output.

Address review feedback:
- Reject --siem + --record-details combination with clear error
- Reject --siem + --format combination (SIEM is NDJSON-only)
- Fix empty report producing blank line instead of empty output
- Mask sensitive fields in SIEM NDJSON output (CodeQL finding)
- Add os.fchmod(fd, 0o600) to ensure existing files get restrictive
  permissions (os.open mode only applies on creation)
- Add 25 tests covering NDJSON format, PII masking, risk factors,
  empty report handling, file permissions, flag validation, and
  BreachWatch mode
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.

2 participants