diff --git a/apps/openant-cli/cmd/analyze.go b/apps/openant-cli/cmd/analyze.go index 986213b5..13e7eb45 100644 --- a/apps/openant-cli/cmd/analyze.go +++ b/apps/openant-cli/cmd/analyze.go @@ -30,6 +30,7 @@ var ( analyzeAppContext string analyzeRepoPath string analyzeExploitOnly bool + analyzeExploitAll bool analyzeLimit int analyzeModel string analyzeWorkers int @@ -43,7 +44,9 @@ func init() { analyzeCmd.Flags().StringVar(&analyzeAnalyzerOutput, "analyzer-output", "", "Path to analyzer_output.json (for Stage 2)") analyzeCmd.Flags().StringVar(&analyzeAppContext, "app-context", "", "Path to application_context.json") analyzeCmd.Flags().StringVar(&analyzeRepoPath, "repo-path", "", "Path to the repository (for context correction)") - analyzeCmd.Flags().BoolVar(&analyzeExploitOnly, "exploitable-only", false, "Only analyze units classified as exploitable by enhancer") + analyzeCmd.Flags().BoolVar(&analyzeExploitAll, "exploitable-all", false, "Analyze units classified as exploitable or vulnerable_internal (safer, compensates for parser gaps)") + analyzeCmd.Flags().BoolVar(&analyzeExploitOnly, "exploitable-only", false, "Analyze only units classified as exploitable (strict, use after parser entry point fixes)") + analyzeCmd.MarkFlagsMutuallyExclusive("exploitable-all", "exploitable-only") analyzeCmd.Flags().IntVar(&analyzeLimit, "limit", 0, "Max units to analyze (0 = no limit)") analyzeCmd.Flags().StringVar(&analyzeModel, "model", "opus", "Model: opus or sonnet") analyzeCmd.Flags().IntVar(&analyzeWorkers, "workers", 8, "Number of parallel workers for LLM steps (default: 8)") @@ -51,6 +54,58 @@ func init() { analyzeCmd.Flags().IntVar(&analyzeBackoff, "backoff", 30, "Seconds to wait when rate-limited (default: 30)") } +// buildAnalyzePyArgs assembles the argv passed to the Python `openant analyze` +// subprocess. Extracted as a pure function (mirrors buildParsePyArgs) so the +// flag-forwarding contract — including the exploitable filter parity with the +// Python backend — is unit-testable without spawning Python. +func buildAnalyzePyArgs( + datasetPath, output string, + verify bool, + analyzerOutput, appContext, repoPath string, + exploitOnly, exploitAll bool, + limit int, + model string, + workers int, + checkpoint string, + backoff int, +) []string { + pyArgs := []string{"analyze", datasetPath, "--output", output} + if verify { + pyArgs = append(pyArgs, "--verify") + } + if analyzerOutput != "" { + pyArgs = append(pyArgs, "--analyzer-output", analyzerOutput) + } + if appContext != "" { + pyArgs = append(pyArgs, "--app-context", appContext) + } + if repoPath != "" { + pyArgs = append(pyArgs, "--repo-path", repoPath) + } + if exploitAll { + pyArgs = append(pyArgs, "--exploitable-all") + } + if exploitOnly { + pyArgs = append(pyArgs, "--exploitable-only") + } + if limit > 0 { + pyArgs = append(pyArgs, "--limit", fmt.Sprintf("%d", limit)) + } + if model != "opus" { + pyArgs = append(pyArgs, "--model", model) + } + if workers != 8 { + pyArgs = append(pyArgs, "--workers", fmt.Sprintf("%d", workers)) + } + if checkpoint != "" { + pyArgs = append(pyArgs, "--checkpoint", checkpoint) + } + if backoff != 30 { + pyArgs = append(pyArgs, "--backoff", fmt.Sprintf("%d", backoff)) + } + return pyArgs +} + func runAnalyze(cmd *cobra.Command, args []string) { datasetPath, ctx, err := resolveFileArg(args, "dataset_enhanced.json") if err != nil { @@ -92,37 +147,12 @@ func runAnalyze(cmd *cobra.Command, args []string) { } } - pyArgs := []string{"analyze", datasetPath, "--output", analyzeOutput} - if analyzeVerify { - pyArgs = append(pyArgs, "--verify") - } - if analyzeAnalyzerOutput != "" { - pyArgs = append(pyArgs, "--analyzer-output", analyzeAnalyzerOutput) - } - if analyzeAppContext != "" { - pyArgs = append(pyArgs, "--app-context", analyzeAppContext) - } - if analyzeRepoPath != "" { - pyArgs = append(pyArgs, "--repo-path", analyzeRepoPath) - } - if analyzeExploitOnly { - pyArgs = append(pyArgs, "--exploitable-only") - } - if analyzeLimit > 0 { - pyArgs = append(pyArgs, "--limit", fmt.Sprintf("%d", analyzeLimit)) - } - if analyzeModel != "opus" { - pyArgs = append(pyArgs, "--model", analyzeModel) - } - if analyzeWorkers != 8 { - pyArgs = append(pyArgs, "--workers", fmt.Sprintf("%d", analyzeWorkers)) - } - if analyzeCheckpoint != "" { - pyArgs = append(pyArgs, "--checkpoint", analyzeCheckpoint) - } - if analyzeBackoff != 30 { - pyArgs = append(pyArgs, "--backoff", fmt.Sprintf("%d", analyzeBackoff)) - } + pyArgs := buildAnalyzePyArgs( + datasetPath, analyzeOutput, analyzeVerify, + analyzeAnalyzerOutput, analyzeAppContext, analyzeRepoPath, + analyzeExploitOnly, analyzeExploitAll, analyzeLimit, + analyzeModel, analyzeWorkers, analyzeCheckpoint, analyzeBackoff, + ) result, err := python.Invoke(rt.Path, pyArgs, "", quiet, requireAPIKey()) if err != nil { diff --git a/apps/openant-cli/cmd/analyze_flags_test.go b/apps/openant-cli/cmd/analyze_flags_test.go new file mode 100644 index 00000000..ded55565 --- /dev/null +++ b/apps/openant-cli/cmd/analyze_flags_test.go @@ -0,0 +1,63 @@ +package cmd + +import ( + "strings" + "testing" +) + +// TestAnalyzeExploitableAllFlagDefined locks the flag-parity fix: +// the Go `analyze` command must expose +// `--exploitable-all`, mirroring the Python backend (cli.py analyze_p +// defines both --exploitable-all and --exploitable-only). Before the fix the +// Go CLI defined only --exploitable-only, so `analyze --exploitable-all` +// failed with "unknown flag". +func TestAnalyzeExploitableAllFlagDefined(t *testing.T) { + flag := analyzeCmd.Flag("exploitable-all") + if flag == nil { + t.Fatal("analyzeCmd has no --exploitable-all flag (parity gap with Python backend)") + } + if got, want := flag.DefValue, "false"; got != want { + t.Errorf("--exploitable-all default = %q, want %q", got, want) + } + // The control flag must still exist. + if analyzeCmd.Flag("exploitable-only") == nil { + t.Fatal("analyzeCmd lost its --exploitable-only flag") + } +} + +// TestAnalyzeExploitableAllForwardedToPython locks that the new flag is +// actually forwarded to the Python subprocess argv (not just defined). It +// exercises buildAnalyzePyArgs, the pure argv-builder helper (mirrors the +// buildParsePyArgs pattern), so a future refactor that drops the forwarding +// fails here. +func TestAnalyzeExploitableAllForwardedToPython(t *testing.T) { + tests := []struct { + name string + exploitAll bool + exploitOnly bool + wantAllInArgv bool + }{ + {"exploitable-all forwarded", true, false, true}, + {"neither flag -> not forwarded", false, false, false}, + {"exploitable-only does not emit --exploitable-all", false, true, false}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + argv := buildAnalyzePyArgs( + "ds.json", "out", false, "", "", "", + tc.exploitOnly, tc.exploitAll, 0, "opus", 8, "", 30, + ) + joined := strings.Join(argv, " ") + got := false + for _, a := range argv { + if a == "--exploitable-all" { + got = true + break + } + } + if got != tc.wantAllInArgv { + t.Errorf("buildAnalyzePyArgs --exploitable-all present=%v, want %v; argv=%q", got, tc.wantAllInArgv, joined) + } + }) + } +} diff --git a/libs/openant-core/core/analyzer.py b/libs/openant-core/core/analyzer.py index f8255f13..4d3534eb 100644 --- a/libs/openant-core/core/analyzer.py +++ b/libs/openant-core/core/analyzer.py @@ -47,6 +47,51 @@ load_context = None +# Truncation priority: higher score = kept first when --limit drops units. +# Units the enhancer flagged as exploitable/vulnerable_internal must outrank +# unclassified/neutral code so a limited run does not silently drop the most +# security-relevant units. +_LIMIT_PRIORITY = {"exploitable": 2, "vulnerable_internal": 1} + + +def _limit_classification(unit): + """Return a unit's security_classification regardless of enhance mode. + + Agentic enhance writes ``unit['agent_context']['security_classification']``; + single-shot enhance writes ``unit['llm_context']``. Reading only one mode + would treat the other's classified units as un-prioritized. + """ + for ctx_key in ("agent_context", "llm_context"): + ctx = unit.get(ctx_key) + if isinstance(ctx, dict) and ctx.get("security_classification") is not None: + return ctx.get("security_classification") + return None + + +def _apply_limit(units, limit): + """Truncate ``units`` to ``limit``, keeping the highest-priority units. + + Units arrive from the parser in alphabetical-by-path order + (repository_scanner.py sorts ``self.files`` by path). A raw head-slice + therefore kept the first N alphabetical units (``Doc/`` before ``Lib/``) + with no relevance weighting, silently dropping high-value code on a + ``--limit`` run. + + Sort by enhancement security_classification (exploitable > + vulnerable_internal > other) before slicing. The sort is stable, so units + in the same classification tier keep their original (alphabetical) order; + a no-limit call returns the list unchanged. + """ + if not limit: + return units + prioritized = sorted( + units, + key=lambda u: _LIMIT_PRIORITY.get(_limit_classification(u), 0), + reverse=True, + ) + return prioritized[:limit] + + def _process_unit(client, unit, index, json_corrector, app_context): """Process a single unit for Stage 1 detection. @@ -356,7 +401,9 @@ def run_analysis( print(f"[Analyze] Exploitable filter ({exploitable_filter}): {original_count} -> {len(units)} units", file=sys.stderr) if limit: - units = units[:limit] + # Priority-sort before truncating so a --limit run keeps the most + # security-relevant units rather than the alphabetically-first ones. + units = _apply_limit(units, limit) total = len(units) print(f"[Analyze] Analyzing {total} units...", file=sys.stderr) diff --git a/libs/openant-core/tests/test_analyzer_limit_priority.py b/libs/openant-core/tests/test_analyzer_limit_priority.py new file mode 100644 index 00000000..e6b508a9 --- /dev/null +++ b/libs/openant-core/tests/test_analyzer_limit_priority.py @@ -0,0 +1,96 @@ +"""Regression tests for priority-sorted ``--limit`` truncation. + +`run_analysis(..., limit=N)` truncates the unit list with a raw head-slice +``units = units[:limit]`` (analyzer.py). The units arrive from the parser in +alphabetical-by-path order (repository_scanner.py: ``self.files.sort(key=lambda +f: f['path'])``), so a ``--limit`` run deterministically kept the first N +alphabetical units (e.g. ``Doc/`` before ``Lib/``) and dropped high-value code +with NO relevance/priority weighting. + +The fix sorts by enhancement security_classification (exploitable > +vulnerable_internal > other) BEFORE the head-slice, stably (alphabetical order +preserved within a classification tier), and reads the classification +mode-agnostically (agentic writes agent_context, single-shot writes +llm_context). These tests drive the extracted ``_apply_limit`` helper directly. +""" +import sys +from pathlib import Path + +_CORE_ROOT = Path(__file__).parent.parent +if str(_CORE_ROOT) not in sys.path: + sys.path.insert(0, str(_CORE_ROOT)) + +from core.analyzer import _apply_limit # noqa: E402 + + +def _unit(uid, classification=None, mode="agentic"): + u = {"id": uid} + if classification is not None: + ctx_key = "agent_context" if mode == "agentic" else "llm_context" + u[ctx_key] = {"security_classification": classification} + return u + + +def test_no_limit_returns_units_unchanged(): + units = [_unit("a"), _unit("b"), _unit("c")] + assert _apply_limit(units, None) is units + assert _apply_limit(units, 0) is units + + +def test_limit_keeps_exploitable_over_alphabetically_early_neutral(): + # Parser order: Doc/ neutral units come first alphabetically, the + # exploitable Lib/ unit comes last. A raw head-slice with limit=2 would + # keep the two Doc/ neutrals and DROP the exploitable unit. + units = [ + _unit("Doc/a", "neutral"), + _unit("Doc/b", "neutral"), + _unit("Lib/danger", "exploitable"), + ] + kept = _apply_limit(units, 2) + kept_ids = [u["id"] for u in kept] + assert "Lib/danger" in kept_ids, ( + "exploitable unit must survive a --limit truncation over neutral units; " + f"got {kept_ids}" + ) + assert len(kept) == 2 + + +def test_priority_order_exploitable_then_vulnerable_internal_then_other(): + units = [ + _unit("a", "neutral"), + _unit("b", "vulnerable_internal"), + _unit("c", "exploitable"), + _unit("d", None), + ] + kept_ids = [u["id"] for u in _apply_limit(units, 4)] + # exploitable first, then vulnerable_internal, then the rest (stable). + assert kept_ids[0] == "c" + assert kept_ids[1] == "b" + assert set(kept_ids[2:]) == {"a", "d"} + + +def test_stable_within_same_classification_tier(): + # Equal-priority units retain their original (alphabetical) order. + units = [ + _unit("Lib/a", "exploitable"), + _unit("Lib/b", "exploitable"), + _unit("Lib/c", "exploitable"), + ] + kept_ids = [u["id"] for u in _apply_limit(units, 2)] + assert kept_ids == ["Lib/a", "Lib/b"] + + +def test_classification_read_mode_agnostically_single_shot(): + # Single-shot enhance writes llm_context, not agent_context. + units = [ + _unit("Doc/a", "neutral", mode="single-shot"), + _unit("Lib/danger", "exploitable", mode="single-shot"), + ] + kept_ids = [u["id"] for u in _apply_limit(units, 1)] + assert kept_ids == ["Lib/danger"] + + +def test_limit_larger_than_list_returns_all_reprioritized(): + units = [_unit("a", "neutral"), _unit("b", "exploitable")] + kept_ids = [u["id"] for u in _apply_limit(units, 10)] + assert kept_ids == ["b", "a"]