Skip to content

Commit ee2d641

Browse files
branoverclaude
andauthored
fix: enforce the analysis invariant — only re_analyze runs a full analysis (#270)
* fix: enforce the analysis invariant — only re_analyze runs a full analysis The analyze/decompile split (#256–261) gated the whole-program MCP tools on a saved analysis at the run_tool seam, but several paths could still trigger a cold whole-binary analysis on their own — the exact ~2547s-timeout class of incident the split was meant to end: * the r2 `decompile_probe` ran a cold `aaa` on a plain focus/list miss (so the LLM `static_analysis` decompile and enrich_recon cold-analyzed directly, bypassing run_tool's gate); * `xrefs_probe` never loaded the warm r2 project, so a radare2-backend xref / search re-ran `aaa` per call; * `ghidra_probe` cold-imported on any per-call mode without a warm slot; * `re_recover_constant` (emulate) and the `static_analysis` taint pass reach the Ghidra ops outside run_tool's gate and cold-analyzed. Make re_analyze the SINGLE chokepoint. Every other analysis-needing probe / tool / task is now warm-only: on a cold miss it returns a structured `needs_analysis` payload pointing at re_analyze instead of analyzing. Only two EXPLICIT entry points cold-analyze — `re_analyze` (`--analyze`, detached, generous budget) and `re_reanalyze` (`--reanalyze`, the deliberate deeper re-analysis). Targeted `re_disassemble`/`--range` and store-reading `re_search_decompiled` need no analysis and stay ungated; a raw byte/immediate SEARCH also needs none and still scans the loaded image. Probes: * decompile_probe: refuse a cold whole-binary decompile/list unless --analyze/--reanalyze; return the re_analyze lead (rc=0, structured). * xrefs_probe: reload the warm r2 project (dir.projects + -p) for the index modes; never `aaa`; cold miss → re_analyze lead. SEARCH proceeds cold. * ghidra_probe: install-check first (missing toolchain → "rebuild WITH_GHIDRA=1"), then warm-only for every non-analyze mode. Warm-miss is rc=0 + a structured payload (not a non-zero exit run_probe would raise on), matching xrefs_probe/decompile_probe. Host: * new engine.re.analysis.analysis_lead() — the single host gate for the paths that DON'T go through run_tool (recover_constant, the taint task); mirrors _analysis_gate, best-effort. * emulation.emulate_constant / taint.analyze_taint gate on it (the arg-dependent pre-check runs first — a pure graph read, more actionable than "needs_analysis"). * agent_tools._r2_project_mount wires the warm r2 slot into the xrefs/search probe calls. Docs: docs/mcp.md updated (r2 xref verbs are warm-only too; recover_constant + taint gate the same way). Tests: real-Ghidra/r2 tests now run the re_analyze step (new conftest warm_ghidra_slot / warm_r2_slot helpers) before a warm-only op — proving the enforcement end to end; new offline coverage for analysis_lead, the emulate/taint gates, and xrefs_probe._warm_r2_flags. `just test` green (1648 passed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: address PR #270 review — remote-executor guard, gate dedup, lead surfacing, CI gate Follow-up to the analysis-invariant enforcement, per the pr-reviewer findings on #270: - MEDIUM-LOW (remote executor): `_r2_project_mount` returned a slot path regardless of executor, but `RemoteDockerExecutor.run_probe` refuses any project_mount — so the r2 xref family + search_code byte-scan would hard-error on a remote executor where they previously worked. Add an explicit `supports_project_mount` capability (True on the local executor, False on the remote one) and skip the mount when it's unsupported, so those calls degrade (index modes → re_analyze lead; search → raw scan) instead of erroring. - LOW (lead swallowed): a cold-miss xrefs/callgraph payload carries the re_analyze lead in `out["error"]` with empty results; `_run_xrefs_probe` now surfaces a `needs_analysis` payload as the error so every caller points at re_analyze instead of formatting an empty graph. - NIT (duplication): `_analysis_gate` now DELEGATES to `analysis.analysis_lead` (the same gate the recover_constant / taint paths use), so the two can't drift. - CI: `scripts/ci_ghidra_decompile_check.py` (the WITH_GHIDRA decompile gate) did a COLD `ghidra_probe` decompile + xrefs, which the invariant now refuses — so it builds the warm project once via `--analyze` (bind-mounted slot) and drives the decompile + xrefs warm, exactly the production shape. Verified locally against the WITH_GHIDRA image (real C recovered, cgi_handler among strcpy's callers). Offline tests added: `_r2_project_mount` degrades to None on a remote executor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: accept the runner kwarg in list_functions gate monkeypatch _analysis_gate now delegates to analysis.analysis_lead (PR #270 dedup), which calls analysis_state(project, target, runner=None). This test stubbed analysis_state with a strict 2-arg lambda, so the runner kwarg raised TypeError → caught → gate bypassed. Accept **kw (matching the real signature), as _fake_state already does. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1645b3b commit ee2d641

22 files changed

Lines changed: 486 additions & 82 deletions

docs/mcp.md

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,10 @@ already state, and `meta_get_schemas` spells out in its `substrate_vs_graph` and
165165
reference index, the same analyze-once project the decompile verbs build, instead of re-analyzing the
166166
whole binary on every call. On a large target that is the difference between an instant answer and a query
167167
that never finishes, and an unknown symbol comes back as "not found" right away rather than after a long
168-
wait. With the radare2 backend, or before a first analysis has warmed the project, the verbs fall back to
169-
a fresh cross-reference pass.
168+
wait. The radare2 backend works the same way: the verbs reload the warm r2 project's own index rather than
169+
re-running a whole-binary pass. Either way, until a first analysis has warmed the project they point you at
170+
`re_analyze` instead of quietly analyzing (see the next point) — no per-call verb ever launches a cold pass
171+
of its own.
170172
- **Analysis is explicit — `re_analyze` first, then the per-call verbs.** The whole-program tools
171173
(`re_decompile_function`/`re_decompile_at`, `re_list_functions`, and the xref family
172174
`re_xrefs`/`re_function_xrefs`/`re_data_xrefs`/`re_call_graph`) **require a saved analysis**: on a warm
@@ -176,8 +178,10 @@ already state, and `meta_get_schemas` spells out in its `substrate_vs_graph` and
176178
generous, size-scaled budget, so it finishes and commits the warm project. It's **single-flight** (a second
177179
`re_analyze` of the same target attaches to the running one, never a duplicate) and idempotent: re-call it to
178180
poll (`state` walks `none → running → analyzed`), and once `analyzed` every gated verb is instant.
179-
`re_disassemble` (targeted) and `re_search_decompiled` (reads the store) need no whole-program analysis and
180-
are never gated. This is **backend-aware**: it covers both headless Ghidra (a warm Ghidra project) and
181+
`re_recover_constant` (P-Code emulation) and the `static_analysis` taint pass gate the same way — they run
182+
the deeper analysis over the warm project, so on a cold target they return the `re_analyze` lead rather than
183+
analyzing themselves. `re_disassemble` (targeted) and `re_search_decompiled` (reads the store) need no
184+
whole-program analysis and are never gated. This is **backend-aware**: it covers both headless Ghidra (a warm Ghidra project) and
181185
radare2 (a warm r2 project, reclaimable via `hexgraph prune --r2-cache-mb`), each with its own persistent
182186
slot. Only the Ghidra *bridge* backend — which attaches to a Ghidra you already have open — has nothing to
183187
build here, so it reports `unavailable` and isn't gated.

scripts/ci_ghidra_decompile_check.py

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
from __future__ import annotations
3232

3333
import sys
34+
import tempfile
3435
from pathlib import Path
3536

3637
REPO_ROOT = Path(__file__).resolve().parent.parent
@@ -65,13 +66,30 @@ def main() -> int:
6566
_fail(f"could not import hexgraph (install the package in this job): {exc}")
6667

6768
runner = SandboxRunner(image=image)
69+
70+
# Analysis is WARM-ONLY now (the invariant: only re_analyze runs a full analysis). Build the warm
71+
# Ghidra project ONCE via `--analyze` into a bind-mounted slot, then drive decompile + xrefs
72+
# against THAT warm project — exactly the production shape (re_analyze → warm per-call verbs). A
73+
# per-run temp slot; CI is ephemeral, so no cleanup needed. This ALSO exercises the `--analyze`
74+
# detached-analysis path end to end under production hardening.
75+
proj = tempfile.mkdtemp(prefix="ci-ghidra-slot-")
76+
try:
77+
warm = runner.run_json_probe("ghidra_probe.py", str(fixture),
78+
extra_args=["--analyze"], project_mount=proj)
79+
except SandboxError as exc:
80+
_fail(f"ghidra --analyze (warm-project build) failed under production hardening: {exc}")
81+
if isinstance(warm, dict) and warm.get("error"):
82+
_fail(f"ghidra --analyze reported an error: {warm['error']}")
83+
6884
try:
6985
# run_json_probe == run_probe + json.loads of stdout, the same call GhidraDecompiler
7086
# makes. On a non-zero probe exit it raises SandboxError; ghidra_probe.py emits its
7187
# diagnostic (the analyzeHeadless log tail) on STDOUT as {"error": ...}, so surface
7288
# that too — the previous standalone gate printed only the empty stderr, hiding the
73-
# cause (the launcher's "Failed to create directory" under --read-only).
74-
result = runner.run_json_probe("ghidra_probe.py", str(fixture), extra_args=[FOCUS])
89+
# cause (the launcher's "Failed to create directory" under --read-only). WARM decompile
90+
# against the project built above (project_mount=proj) — a cold decompile now refuses.
91+
result = runner.run_json_probe("ghidra_probe.py", str(fixture),
92+
extra_args=[FOCUS], project_mount=proj)
7593
except SandboxError as exc:
7694
# SandboxRunner raises on a non-zero probe exit and the SandboxError message carries
7795
# the probe's STDERR tail (runner._run: proc.stderr.strip()[:500]); it does NOT carry
@@ -115,11 +133,12 @@ def main() -> int:
115133
# REAL Ghidra: this is the reference-index query the re_xrefs / re_function_xrefs / re_call_graph
116134
# verbs serve from the warm project, and the offline tier has no Ghidra to run it. cgi_handler
117135
# does `strcpy(buf, token)`, so "who calls strcpy" (ReferenceManager.getReferencesTo, filtered
118-
# to the containing function) MUST include cgi_handler. Drives the SAME production run_probe path
119-
# (a cold throwaway project here, which still imports+analyzes then runs XREFS_SCRIPT).
136+
# to the containing function) MUST include cgi_handler. Drives the SAME production run_probe path,
137+
# served WARM from the project built above (project_mount=proj) — the reference index the re_xrefs
138+
# family reuses, never a per-call re-analysis.
120139
try:
121140
xr = runner.run_json_probe("ghidra_probe.py", str(fixture),
122-
extra_args=["--xrefs", "callers", "strcpy"])
141+
extra_args=["--xrefs", "callers", "strcpy"], project_mount=proj)
123142
except SandboxError as exc:
124143
_fail(f"ghidra xrefs probe failed under production hardening: {exc}")
125144
if "error" in xr:

src/hexgraph/agent/agent_tools.py

Lines changed: 50 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -784,22 +784,13 @@ def _analysis_gate(ctx: ToolContext) -> str | None:
784784
entirely off `analysis_state` — backend-aware since C1b, so it gates BOTH headless Ghidra and
785785
radare2 (each with its own warm slot). `unavailable` (no persistent-slot backend — Ghidra bridge
786786
— / Docker down / no byte artifact) means "not gated, behave as before", so those paths are
787-
unaffected. Best-effort: any error ⇒ don't gate."""
788-
try:
789-
from hexgraph.engine.re.analysis import analysis_state
787+
unaffected. Best-effort: any error ⇒ don't gate.
790788
791-
st = analysis_state(ctx.project, ctx.target)
792-
except Exception: # noqa: BLE001 — a gate hiccup must never block a tool that could run
793-
return None
794-
state = st.get("state")
795-
if state in ("analyzed", "unavailable"):
796-
return None
797-
lead = {"none": "No saved analysis for this target yet.",
798-
"running": "A whole-binary analysis is already in progress.",
799-
"failed": "The last analysis did not finish."}.get(state, "No saved analysis.")
800-
return (f"{lead} Run re_analyze(target) first — it builds the warm analysis (a Ghidra or radare2 "
801-
"project) ONCE with a generous budget (detached; re-call re_analyze to poll until "
802-
f"state='analyzed'), then retry this tool and it'll be instant. [{st.get('detail', '')}]")
789+
Delegates to `analysis.analysis_lead` — the SAME gate the tools that bypass run_tool
790+
(recover_constant, the taint task) consult — so the two can never drift."""
791+
from hexgraph.engine.re.analysis import analysis_lead
792+
793+
return analysis_lead(ctx.project, ctx.target)
803794

804795

805796
def run_tool(ctx: ToolContext, name: str, args: dict) -> str:
@@ -2056,12 +2047,40 @@ def fmt_group(group: dict) -> list[str]:
20562047
return ctx.cache[key]
20572048

20582049

2050+
def _r2_project_mount(ctx: ToolContext) -> str | None:
2051+
"""The WARM r2-project slot root for this target, bind-mounted into xrefs_probe so it RELOADS the
2052+
analysis instead of re-running `aaa` (the invariant: only re_analyze analyzes). None when there's
2053+
no data dir / slot, or the active executor can't bind-mount the slot (the remote executor — the
2054+
slot lives on the LOCAL data dir; xrefs_probe then returns the re_analyze lead for the index modes,
2055+
and still scans raw for search). Best-effort; mirrors R2Decompiler._resolve_slot. Read-only reload
2056+
→ no slot lock needed (never saves)."""
2057+
project = getattr(ctx, "project", None)
2058+
if project is None or not getattr(project, "data_dir", None):
2059+
return None
2060+
try:
2061+
from hexgraph.engine.re import r2_project as rp
2062+
from hexgraph.sandbox.executor import get_executor
2063+
from hexgraph.sandbox.runner import sandbox_image
2064+
2065+
ex = get_executor()
2066+
if not getattr(ex, "supports_project_mount", True):
2067+
return None # remote executor: run_probe refuses a project_mount — degrade, don't error
2068+
sha = rp.content_hash(ctx.target.path)
2069+
version = rp.r2_version_for_image(sandbox_image(), runner=ex)
2070+
slot = rp.resolve(project.data_dir, sha, version)
2071+
slot.prepare()
2072+
return str(slot.root)
2073+
except Exception: # noqa: BLE001 — best-effort; a resolve failure reads as "no warm slot"
2074+
return None
2075+
2076+
20592077
def _run_xrefs_probe(ctx: ToolContext, subject: str | None, mode: str):
20602078
"""Run a breadth xrefs query in `mode`, returning (out, error_text). Prefers the warm persistent
20612079
Ghidra project's reference index when headless Ghidra is active (analyze-once, fast on a large
20622080
target where the cold r2 sweep times out); falls back to the r2 xrefs_probe (`--mode
2063-
function|data|callgraph`) only when Ghidra can't run. `mode="callers"` passes no --mode flag on
2064-
the r2 path (legacy compat)."""
2081+
function|data|callgraph`) only when Ghidra can't run. The r2 probe is WARM-ONLY too — it RELOADS
2082+
the persistent r2 project (bind-mounted) and NEVER runs `aaa`; a cold slot returns the re_analyze
2083+
lead. `mode="callers"` passes no --mode flag on the r2 path (legacy compat)."""
20652084
if _ghidra_xrefs_active():
20662085
out = _ghidra_xrefs(ctx, mode, subject)
20672086
if out is not None:
@@ -2073,10 +2092,17 @@ def _run_xrefs_probe(ctx: ToolContext, subject: str | None, mode: str):
20732092
return None, f"{mode} unavailable (Docker/sandbox not running)"
20742093
extra = ([subject] if subject else []) + (["--mode", mode] if mode != "callers" else [])
20752094
try:
2076-
return get_executor().run_json_probe("xrefs_probe.py", ctx.target.path,
2077-
extra_args=extra or None), None
2095+
out = get_executor().run_json_probe("xrefs_probe.py", ctx.target.path,
2096+
extra_args=extra or None,
2097+
project_mount=_r2_project_mount(ctx))
20782098
except Exception as exc: # noqa: BLE001
20792099
return None, f"{mode} xrefs failed: {exc}"
2100+
# A warm-MISS payload carries the re_analyze lead (a cold index mode, e.g. reached with the
2101+
# run_tool gate reporting `unavailable`). Surface it as the error so EVERY caller (function/data
2102+
# xrefs, call graph) points at re_analyze instead of formatting an empty result.
2103+
if isinstance(out, dict) and out.get("needs_analysis") and out.get("error"):
2104+
return None, out["error"]
2105+
return out, None
20802106

20812107

20822108
def _recon_function_xrefs(ctx: ToolContext, function: str) -> tuple[list[str], list[str]]:
@@ -2406,14 +2432,16 @@ def _ghidra_search(ctx: ToolContext, *, bytes_pat, immediate) -> dict | None:
24062432

24072433
def _r2_search(ctx: ToolContext, *, bytes_pat, immediate):
24082434
"""The radare2 raw-scan fallback (`xrefs_probe --mode search`, `/xj`//`/vj`) for when Ghidra
2409-
can't answer. The probe no longer runs a whole-binary `aaa` for a search, so it's fast (function
2410-
mapping best-effort from the symbol table). Returns the probe dict, or a string error message."""
2435+
can't answer. The probe never runs a whole-binary `aaa` for a search, so it's fast; passing the
2436+
warm r2 project (when one exists) lets it map hits to the containing function. Returns the probe
2437+
dict, or a string error message."""
24112438
from hexgraph.sandbox.executor import get_executor
24122439

24132440
extra = ["--mode", "search"] + (["--bytes", str(bytes_pat)] if bytes_pat
24142441
else ["--imm", str(immediate)])
24152442
try:
2416-
return get_executor().run_json_probe("xrefs_probe.py", ctx.target.path, extra_args=extra)
2443+
return get_executor().run_json_probe("xrefs_probe.py", ctx.target.path, extra_args=extra,
2444+
project_mount=_r2_project_mount(ctx))
24172445
except Exception as exc: # noqa: BLE001
24182446
return f"search_code scan failed: {exc}"
24192447

src/hexgraph/engine/re/analysis.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,28 @@ def analysis_state(project, target, *, runner=None) -> dict:
193193
"build it", "container": name}
194194

195195

196+
def analysis_lead(project, target, *, runner=None) -> str | None:
197+
"""A re_analyze lead when `target` has NO saved analysis for the active backend, else None
198+
(proceed). `analyzed` → None; `unavailable` (Ghidra-bridge / Docker down / no byte artifact) →
199+
None too, since those paths serve warm anyway or can't be gated. This is the single host-side
200+
gate for the analysis-needing tools/tasks that DON'T go through agent_tools.run_tool's gate
201+
(recover_constant, the taint task) — so, like every per-call tool, they point at re_analyze on a
202+
cold target instead of triggering a full analysis themselves. Best-effort: any error ⇒ None."""
203+
try:
204+
st = analysis_state(project, target, runner=runner)
205+
except Exception: # noqa: BLE001 — a gate hiccup must never block a tool that could run
206+
return None
207+
state = st.get("state")
208+
if state in ("analyzed", "unavailable"):
209+
return None
210+
lead = {"none": "No saved analysis for this target yet.",
211+
"running": "A whole-binary analysis is already in progress.",
212+
"failed": "The last analysis did not finish."}.get(state, "No saved analysis.")
213+
return (f"{lead} Run re_analyze(target) first — it builds the warm analysis ONCE with a generous "
214+
"budget (detached; re-call re_analyze to poll until state='analyzed'), then retry this — "
215+
f"it's warm-only and never runs a cold analysis itself. [{st.get('detail', '')}]")
216+
217+
196218
def start_analysis(project, target, *, runner=None) -> dict:
197219
"""Start OR attach to a detached whole-binary analysis for the ACTIVE backend. Idempotent and
198220
single-flight: already-warm ⇒ no-op ``analyzed``; already-running ⇒ ``running`` (attach);

src/hexgraph/engine/re/emulation.py

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,18 +92,32 @@ def emulate_constant(session: Session, project: Any, target: Any, *, function: s
9292
"reached_ret": False, "steps": None, "observation_id": None,
9393
"error": "emulation requires the Ghidra headless decompiler (features.ghidra)"}
9494

95-
# PRE-CHECK: skip the doomed run for an argument-dependent routine. If a prior decompile
96-
# recorded the recovered signature on the function node and it takes arguments, emulating
97-
# it over uninitialized inputs won't reach a clean ret — return an informative result
98-
# instead of burning a sandbox emulation. (No recorded signature ⇒ fall through and let
99-
# the emulation's own in-probe arg guard decide; we never fabricate.)
95+
# PRE-CHECK (first — a pure graph read, triggers NO analysis): skip the doomed run for an
96+
# argument-dependent routine. If a prior decompile recorded the recovered signature on the
97+
# function node and it takes arguments, emulating it over uninitialized inputs won't reach a clean
98+
# ret — return an informative result (pointing at the solver) instead of burning a sandbox
99+
# emulation. This is the MORE actionable answer than "needs_analysis" (re_analyze won't make an
100+
# arg-dependent routine emulable), AND a recorded signature implies analysis already ran. (No
101+
# recorded signature ⇒ fall through to the analysis gate; we never fabricate.)
100102
arg_count = _recovered_arg_count(session, project, target, function)
101103
if arg_count and arg_count > 0:
102104
return {"available": True, "function": function, "value": None, "value_hex": None,
103105
"reached_ret": False, "steps": 0, "observation_id": None,
104106
"skipped": "arg_dependent", "param_count": arg_count,
105107
"error": _ARG_DEPENDENT_HINT}
106108

109+
# ANALYSIS GATE: recover_constant is invoked directly (it bypasses run_tool's gate), so gate it
110+
# HERE — on a cold target with no warm project it must point at re_analyze, NEVER cold-analyze
111+
# (the invariant: only re_analyze runs a full analysis). A live bridge / warm slot reads as
112+
# 'analyzed'|'unavailable' → proceeds and serves it warm.
113+
from hexgraph.engine.re.analysis import analysis_lead
114+
115+
_lead = analysis_lead(project, target)
116+
if _lead:
117+
return {"available": True, "function": function, "value": None, "value_hex": None,
118+
"reached_ret": False, "steps": None, "observation_id": None,
119+
"skipped": "needs_analysis", "error": _lead}
120+
107121
from hexgraph.engine import observations as obs
108122
from hexgraph.engine.graph.nodes import get_or_create_node
109123
from hexgraph.sandbox.decompiler import ghidra_op_backend

src/hexgraph/engine/re/taint.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,17 @@ def analyze_taint(session: Session, project: Any, target: Any, *,
137137
return {"available": False, "flows": [], "analyzed": 0,
138138
"promoted": promoted, "observation_id": None, "cached": False, "error": None}
139139

140+
# ANALYSIS GATE (the invariant): the Ghidra taint pass needs the warm project — on a cold target
141+
# point at re_analyze instead of cold-analyzing (the static_analysis task reaches here outside
142+
# run_tool's gate). A live bridge / warm slot reads as analyzed → proceeds.
143+
if isinstance(analyzer, GhidraTaintAnalyzer):
144+
from hexgraph.engine.re.analysis import analysis_lead
145+
146+
_lead = analysis_lead(project, target)
147+
if _lead:
148+
return {"available": False, "flows": [], "analyzed": 0, "promoted": promoted,
149+
"observation_id": None, "cached": False, "error": _lead}
150+
140151
result = analyzer.analyze(path, project=project)
141152
flows = result.get("flows") or []
142153
status = "error" if result.get("error") else "ok"

0 commit comments

Comments
 (0)