Skip to content

Address open issues #147, #162, #163, #216, #226, #233, #234 - #240

Closed
nikhilpatidar wants to merge 7 commits into
NuGuardAI:mainfrom
nikhilpatidar:main
Closed

Address open issues #147, #162, #163, #216, #226, #233, #234#240
nikhilpatidar wants to merge 7 commits into
NuGuardAI:mainfrom
nikhilpatidar:main

Conversation

@nikhilpatidar

Copy link
Copy Markdown

Summary

Addresses the following open issues:

Verification

  • uv run pytest tests/ --ignore=tests/redteam/v2 -q — 1887 passed, 34 skipped.
  • uv run ruff check nuguard/ — clean.
  • uv run mypy nuguard/cli/commands/redteam.py nuguard/cli/commands/behavior.py nuguard/cli/commands/analyze.py nuguard/cli/commands/policy.py nuguard/config.py nuguard/sbom/core/application_summary.py — clean.

Commits

  1. fix(sbom): normalize Windows paths for deployment URL discovery
  2. test(sbom): add regression tests for sub-folder manifest discovery
  3. fix(cli): align analyze/redteam --output parent-dir handling and add behavior --sbom
  4. refactor(redteam): remove unmaintained v2 engine (Remove nuguard/redteam/v2 — unmaintained, unverified engine reachable via --engine v2 #216)
  5. fix(policy): clarify 'not found' message for stub show command (nuguard policy show always reports 'not found' #162)
  6. docs(verbose): mark spec as 'Partially Implemented' and pin shipped contract (Confirm verbose-flag-spec.md is fully adopted or update its status #163)
  7. test(redteam): pin golden-data suppression so OSS golden data is never flagged as a finding (Make sure we do not mark golden data as finding in red teaming #147)

When the CLI ran on Windows, scanned file paths used backslashes while
the deployment-path hint set used POSIX style. Workflow-path hints like
".github/workflows/", "/k8s/", and "infra/" silently failed to match,
so deployment URLs discovered from .github/workflows/*.yml (and Azure
App Service URLs reconstructed from AZURE_WEBAPP_NAME) were dropped
instead of being harvested into deployment_urls.

Normalize path separators to forward slashes before hint matching so
Windows path strings match the same set as POSIX path strings.
Lock in the existing sub-folder discovery behaviour so any future walker
change that breaks it (e.g. introducing a max depth or accidentally
filtering by directory prefix) fails loudly instead of silently dropping
packages. Covers requirements.txt, package.json, and pyproject.toml at
arbitrary nesting depth, plus the venv / node_modules filtering that
must still apply to deeply nested manifest files.
…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.
The v2 engine was a large (~4,800 LOC) scaffold hidden behind
--engine v2 with no verified correctness and no owner. It contradicted
itself in its own documentation (orchestrator.py claimed 8 phases
complete while the CLI wrapper said it was a scaffold), and a direct
import from nuguard.redteam.v2.execution.runner leaked into
tests/redteam/test_codegen_escalation.py. Nothing else in the codebase
consumes it.

- Remove nuguard/redteam/v2 and tests/redteam/v2 entirely.
- Remove the --engine CLI flag and the v2 dispatch in
  nuguard/cli/commands/redteam.py.
- Remove RedteamV2Settings, the redteam.v2.* config keys, the
  resolved_redteam_v2_settings() method, and the redteam.v2.* NuGuardConfig
  fields from nuguard/config.py. Older nuguard.yaml files with these keys
  still load (the parsers drop them silently).
- Refactor tests/redteam/test_codegen_escalation.py to test the v1
  surface only (the module-level detect_codegen_success() helper plus
  the build_codegen_escalation_chains() builder). The v2-internal
  ObjectiveRunner._detect_codegen_success / _merge_outcomes / KillChainState
  tests are dropped because their subject no longer exists.
- Add tests/redteam/test_no_v2_engine.py as a regression net to catch any
  future reintroduction of v2 plumbing.

The v2 engine was a known-suspect area with no test coverage guaranteeing
that --engine v2 produced the same findings as v1, so removing it does
not change observable v1 behavior. Pre-fix: 1867 tests pass. Post-fix:
1887 tests pass.
…rdAI#162)

nuguard policy show --policy-id <id> unconditionally reported 'not found'
because there was no write path anywhere that persisted a compiled
policy for later lookup. Fix per issue NuGuardAI#162's recommended option of
updating the message rather than removing the subcommand outright:

- Hide 'policy show' from --help output (it's a deprecated stub).
- Expand the error message to explain that the policy registry is not
  implemented yet and point users at 'policy compile' instead.
- Add a regression test verifying the command stays hidden from --help
  and still exits non-zero with a helpful message.
…ontract (NuGuardAI#163)

The verbose-flag-spec.md was marked 'Proposed' even though the
basic contract is already shipped across all four scoped commands:

- --verbose / --no-verbose is exposed on behavior, redteam, validate,
  and policy check.
- CLI flag > command config > default precedence is enforced.
- ReportMeta.to_dict() emits verbose in every report payload.
- Redteam findings are invariant between verbose=true and verbose=false
  (covered by tests/redteam/test_report.py).

Update the status to 'Partially Implemented' and add tests/cli/
test_verbose_flag_spec.py to pin the shipped surface so any future
regression that removes --verbose, drops the no-verbose companion, or
breaks ReportMeta.verbose breaks CI loudly. The remaining spec phases
(findings-invariance refactor across commands, bounded diagnostics
envelope, cap enforcement) remain aspirational.
…r flagged as a finding (NuGuardAI#147)

The redteam executor already suppresses findings when a response is just
the authenticated user's own data via nuguard.redteam.executor.
golden_data_filter.classify_response (HitClass.GOLDEN_DATA_ONLY). Pin
this behaviour with regression tests covering the cases that must
always suppress or always fire:

- Response that IS the golden baseline → GOLDEN_DATA_ONLY (no finding).
- Pure canary hit always wins (canary values are not user's own data).
- Cross-account leakage (own data + someone else's) → GOLDEN_PLUS_NOVEL.
- No golden baseline at all → falls through to NEEDS_PROBE / NO_DATA
  (filter cannot block a hit when there's nothing to compare against).
- Placeholder data (Jane Doe / example.com / 555-0199 / CUST-XXXXX)
  does not trip the golden+novel PII gate.
@nikhilpatidar

Copy link
Copy Markdown
Author

Closing this PR and splitting it into 5 focused PRs per the .github/CONTRIBUTING.md rules (one PR per bug/* or feat/* branch, kept small, branched from the correct base). The commits in this PR are preserved verbatim — each new PR is a cherry-pick of the relevant commit(s) from this branch.

PR Branch Base Issues
#241 bug/sbom-extraction-fixes main #234, #226
#242 feat/cli-flag-alignment develop #233
#243 bug/remove-v2-engine main #216
#244 bug/policy-show-stub main #162
#245 bug/test-and-docs-coverage main #147, #163

Each new PR contains only the files relevant to its issue and the targeted regression tests. No review effort is duplicated — please review the new PRs individually.

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.

1 participant