feat(cli): align --output parent-dir handling and add behavior --sbom (#233) - #242
feat(cli): align --output parent-dir handling and add behavior --sbom (#233)#242nikhilpatidar wants to merge 2 commits into
Conversation
…behavior --sbom Analyze and redteam previously crashed with FileNotFoundError when the parent directory of --output did not exist, while behavior silently auto-created it. Normalize all three commands to behavior's contract. Also add a --sbom flag to behavior so the SBOM can be supplied on the CLI, matching the analyze / redteam subcommands. CLI --sbom overrides sbom: in nuguard.yaml.
15b038c to
6b56d7e
Compare
KanishkThamman
left a comment
KanishkThamman
left a comment
There was a problem hiding this comment.
The --output parent-dir fix (analyze/redteam) is correct and safe. Holding off approval because the headline feature, behavior --sbom, doesn't actually work yet — see inline comments. The two new tests for it pass anyway because their assertions are vacuous (or-chained / exit-code-only), so this shipped silently broken.
| config: Optional[Path] = typer.Option( | ||
| None, "--config", "-c", help="Path to nuguard.yaml" | ||
| ), | ||
| sbom: Optional[str] = typer.Option( |
There was a problem hiding this comment.
This flag is never forwarded to _run_behavior() — the call site's kwargs don't include it, so the CLI value is discarded.
There was a problem hiding this comment.
Fixed in d96b39a. Threaded into _run_behavior() via a dedicated sbom_override: Optional[str] parameter and passed from the call site; the orchestrator now uses sbom_override or cfg.sbom_path so the CLI value wins over the config fallback.
| sbom = None | ||
| sbom_path_obj: Path | None = None | ||
| raw_sbom_path = cfg.sbom_path | ||
| raw_sbom_path = sbom or cfg.sbom_path |
There was a problem hiding this comment.
sbom here refers to the local sbom = None two lines above (the parsed SBOM doc), not the new CLI flag — this always evaluates to cfg.sbom_path. Needs a dedicated param (e.g. sbom_override) threaded through _run_behavior()'s signature and the call site.
There was a problem hiding this comment.
Fixed in d96b39a. Renamed the local parsed-SBOM variable's shadowing behavior away from the path: the new sbom_override parameter on _run_behavior() is used in raw_sbom_path = sbom_override or cfg.sbom_path. The two --sbom tests now assert the literal 'Loaded SBOM: ' log line and the resolved path, so they fail (not vacuously pass) if the wiring breaks again.
The --sbom flag on 'nuguard behavior' was declared on the CLI but its value was never forwarded to _run_behavior(): the call site kwargs omitted it, and the orchestrator's local `sbom = None` (the parsed SBOM doc) shadowed the intended value, so `raw_sbom_path = sbom or cfg.sbom_path` always fell back to the config path. The two new tests that were meant to cover the flag passed anyway because their assertions were vacuous (or-chained / exit-code-only). Fix: - Add a dedicated `sbom_override: Optional[str]` parameter to _run_behavior() and pass it from the CLI entrypoint. - Use `sbom_override or cfg.sbom_path` so the CLI override wins over the config-file fallback. - Strengthen the two --sbom tests to assert the literal 'Loaded SBOM:' log line and the resolved path, so they fail — not vacuously pass — if the flag is dropped again. Whitespace is normalized because Rich soft-wraps long paths and may insert a space inside the path. - Drop a pre-existing unused `import json` left over from the previous commit on this branch so ruff stays clean in CI.
2 participants
PR Type
Fixes #233.
Three CLI commands (
nuguard analyze,nuguard redteam,nuguard behavior) needed--outputto create its parent directory automatically when it didn't exist, andbehaviorwas missing an--sbomflag consistent with the other commands.Changes
nuguard/cli/commands/analyze.py— create parent dir before writing outputnuguard/cli/commands/redteam.py— create parent dir before writing outputnuguard/cli/commands/behavior.py— add--sbomflag, create parent dir before writing outputTests
tests/cli/test_output_parent_dir.py— 5 tests covering each command's parent-dir handling and the behavior --sbom flagCloses #233