From 2fe77ac4a2901cf2348d0dab1cac73b8a5fdd691 Mon Sep 17 00:00:00 2001 From: Alexander Kiselev Date: Sun, 25 Jan 2026 02:24:48 -0800 Subject: [PATCH] docs: Rewrite plan for daemon-only architecture Key changes: - Remove HeadlessExecutor as execution path - All query operations require daemon - Wire IPC layer (already implemented) as primary client - 9 milestones for complete implementation Rationale: Binary analysis is slow enough that persistent daemon is always preferable to per-command process spawning. Co-Authored-By: Claude Opus 4.5 --- docs/plan-prod.md | 1096 +++++++-------------------------------------- 1 file changed, 174 insertions(+), 922 deletions(-) diff --git a/docs/plan-prod.md b/docs/plan-prod.md index ed48846..ff5b393 100644 --- a/docs/plan-prod.md +++ b/docs/plan-prod.md @@ -1,10 +1,10 @@ -# Ghidra-CLI Open Source Release Plan +# Ghidra-CLI Open Source Release Plan (Daemon-Only Architecture) ## Overview -Prepare ghidra-cli for professional open source release on GitHub. The project is architecturally complete (~6,800 lines of Rust) with working CLI, daemon mode, and query system, but has release blockers: minimal README (4 lines), invalid repository URL (localhost), 49 compiler warnings, and XRefs query type unimplemented despite backend support existing. +Prepare ghidra-cli for open source release with a **daemon-only architecture**. Binary analysis is slow enough that a persistent Ghidra process (via daemon) is always preferable to spawning new processes per command. -**Chosen approach**: Full Polish - fix all warnings, expand README comprehensively, implement XRefs query (backend already exists in bridge.py and scripts.rs), add E2E test. Estimated effort: 8-10 hours. +**Key architectural change**: Remove HeadlessExecutor as an execution path. All query operations MUST go through the daemon, which maintains a persistent GhidraBridge connection to Ghidra. ## Planning Context @@ -12,134 +12,65 @@ Prepare ghidra-cli for professional open source release on GitHub. The project i | Decision | Reasoning Chain | |----------|-----------------| -| Full Polish over Minimal Change | XRefs backend exists in bridge.py:222-286 and scripts.rs:322-362 -> wiring takes 2-3 hours -> shipping without it wastes existing work -> full polish provides professional release quality | -| Fix ALL 49 warnings | Partial fix leaves project looking incomplete -> enables `#[deny(warnings)]` in CI -> professional open source projects have zero warnings | -| E2E tests over unit tests | Project already uses E2E pattern in tests/e2e.rs -> consistency with existing codebase -> E2E covers real Ghidra integration which is the risky part | -| Repository URL github.com/akiselev | Matches author field in Cargo.toml -> user confirmed this URL -> enables crates.io publishing | -| XRefs uses HeadlessExecutor not Bridge | Bridge requires daemon running -> HeadlessExecutor pattern matches other query types (functions, strings, etc.) -> consistent user experience | -| Remove dead code over #[allow(dead_code)] | Dead code indicates incomplete features or abandoned refactoring -> removal is cleaner than suppression -> forces decision on whether code is needed | -| Keep data.rs structs despite being unused | Structs define data model for future query types -> removing would require re-adding later -> suppress with #[allow(dead_code)] annotation | -| Keep IPC infrastructure despite current non-use | IPC layer (src/ipc/) is newer local-socket infrastructure -> daemon already works via daemon/rpc.rs (TCP) -> IPC provides cross-platform local socket support for future -> suppress with #[allow(dead_code)] | -| README 9-section structure | User confirmed standard open source structure -> covers all stakeholders (users, developers, contributors) -> comprehensive without being excessive | +| Daemon-only for queries | Binary analysis takes 5-30+ seconds cold start -> persistent daemon amortizes this -> always faster for real workflows -> user confirmed this approach | +| Remove HeadlessExecutor fallback | Fallback creates confusion about which path is used -> daemon is always better -> remove fallback, require daemon explicitly | +| Use IPC layer over legacy RPC | IPC uses local sockets (faster, no port conflicts) -> cross-platform (Unix sockets + Windows named pipes) -> already implemented in src/ipc/ | +| Wire CommandQueue to GhidraBridge | Queue already exists with caching -> handler.rs already routes to bridge -> just need to connect the pieces | +| Keep standalone commands local | Config, setup, doctor, version don't need Ghidra -> run locally without daemon | ### Rejected Alternatives | Alternative | Why Rejected | |-------------|--------------| -| Minimal Change approach | Would ship with backend code users can't access -> XRefs is high-value for RE workflows -> extra 2-3 hours is worth it | -| Feature-Forward (Symbols, Sections) | No backend exists for these -> requires new Jython scripts -> 15+ hours vs 8-10 -> diminishing returns for v0.1.0 | -| Unit tests for query parsing | Query parsing already works (5 types operational) -> risk is Ghidra integration not parsing -> E2E catches real issues | -| Connect XRefs via Bridge instead of Headless | Would require daemon to be running for XRefs only -> inconsistent with other query types -> confusing UX | +| Keep HeadlessExecutor as fallback | Creates two code paths to maintain -> daemon is always better -> simpler to require daemon | +| Auto-start daemon | Adds complexity -> explicit start is clearer -> user knows daemon is running | +| Remove daemon RPC entirely | Some users may have scripts using RPC -> deprecate but keep for now | ### Constraints & Assumptions -- **Technical**: Rust 2021 edition, Ghidra 10.x+ compatibility, existing clap CLI structure -- **Pattern preservation**: All query types use HeadlessExecutor pattern (query/mod.rs:104-116) -- **Testing**: E2E tests require working Ghidra installation, 300s timeout for operations -- **Dependencies**: bridge.py xrefs handlers exist and are tested manually (assumed working) -- **User-specified**: GitHub URL is github.com/akiselev/ghidra-cli -- **User-specified**: Testing approach is E2E only -- **User-specified**: Keep IPC infrastructure for daemon mode in v0.2 -- **User-specified**: README uses 9-section structure (Overview, Features, Installation, Quick Start, CLI Reference, AI Agent Integration, Configuration, Development, Contributing, License) +- **User-specified**: Daemon-only architecture for query operations +- **Technical**: GhidraBridge and bridge.py already functional +- **Technical**: IPC protocol and transport already implemented +- **Pattern**: handler.rs already translates IPC Commands to bridge calls -### Known Risks +## Current State Analysis -| Risk | Mitigation | Anchor | -|------|------------|--------| -| XRefs script may have edge cases | E2E test with sample_binary will catch common issues; defer edge cases to bug reports | scripts.rs:322-362 (script exists) | -| README may miss important details | Include comprehensive sections; link to CLAUDE_SKILL.md for advanced usage | CLAUDE_SKILL.md (463 lines of examples) | -| Dead code removal may break compilation | Compile after each file change; warnings guide what's safe to remove | Compiler output lists exact locations | -| Removing IpcServer dead fields may affect future work | Fields store useful data; document in code comment why kept or remove if truly unused | ipc_server.rs:25-27 | - -## Invisible Knowledge - -### Architecture +### What Already Works ``` -User CLI Command - | - v -+-------------+ +------------------+ -| main.rs |---->| HeadlessExecutor | -| (routing) | | (script runner) | -+-------------+ +------------------+ - | | - v v -+-------------+ +------------------+ -| query/mod.rs| | Ghidra Headless | -| (filtering) | | (Jython scripts) | -+-------------+ +------------------+ - | - v -+-------------+ -| format/ | -| (output) | -+-------------+ +✓ GhidraBridge (src/ghidra/bridge.rs) - persistent TCP to Ghidra +✓ bridge.py - Python server inside Ghidra with command handlers +✓ IPC protocol (src/ipc/protocol.rs) - typed Command enum +✓ IPC transport (src/ipc/transport.rs) - cross-platform sockets +✓ IPC client (src/ipc/client.rs) - high-level client API +✓ IPC server (src/daemon/ipc_server.rs) - accepts connections +✓ Handler (src/daemon/handler.rs) - routes commands to bridge +✓ Daemon lifecycle (start/stop/status/ping) ``` -### Data Flow for Query Command +### What Needs Wiring ``` -ghidra query xrefs --to main --program binary - | - v -CLI parses args -> DataType::XRefs + target address - | - v -Query::execute() -> HeadlessExecutor::get_xrefs_to() - | - v -Write Jython script to temp file -> Run analyzeHeadless - | - v -Parse JSON between markers (---GHIDRA_CLI_START/END---) - | - v -Apply filter, sort, pagination -> Format output +✗ CommandQueue.execute_command() is stubbed (returns TODO message) +✗ main.rs uses daemon_rpc (legacy) not ipc (new) +✗ Fallback to HeadlessExecutor still exists +✗ Query operations bypass daemon entirely ``` -### Why This Structure - -The query system uses a universal pattern where all data types flow through the same execute() method. This enables: -- Consistent filtering/sorting/pagination across all types -- Single point to add new output formats -- Reusable CLI argument parsing - -XRefs differs from other types by requiring a target address parameter, which is passed via script args to Ghidra. - -### Invariants - -- All Jython scripts MUST wrap output in `---GHIDRA_CLI_START---` / `---GHIDRA_CLI_END---` markers -- Query data types in enum must match case in execute() or return "not implemented" error -- HeadlessExecutor methods must return `Result` where JsonValue is an array - -### Tradeoffs - -- **XRefs via Headless vs Bridge**: Chose Headless for consistency even though Bridge is faster. Cost: ~5-10s per query instead of <1s. Benefit: Works without daemon, matches other commands. -- **Remove vs suppress dead code**: Chose remove for cleanup, suppress for data.rs. Cost: More investigation time. Benefit: Cleaner codebase, clear intent. - ## Milestones -> All file paths are relative to repository root (`/home/kiselev/git/ghidra-cli/`) +> All file paths are relative to repository root -### Milestone 1: Fix Cargo.toml and Create docs Directory +### Milestone 1: Fix Cargo.toml Repository URL -**Files**: -- `Cargo.toml` +**Files**: `Cargo.toml` **Requirements**: - Update repository URL from localhost to GitHub -- Ensure docs/ directory exists for plan file **Acceptance Criteria**: -- `cargo metadata` shows valid repository URL - URL matches `https://github.com/akiselev/ghidra-cli` -**Tests**: Skip - configuration change, no runtime behavior - -**Code Intent**: -- Modify `Cargo.toml` line 8: change repository URL from `http://127.0.0.1:62915/git/akiselev/ghidra-cli` to `https://github.com/akiselev/ghidra-cli` - **Code Changes**: ```diff --- a/Cargo.toml @@ -150,892 +81,213 @@ XRefs differs from other types by requiring a target address parameter, which is license = "GPL-3.0" -repository = "http://127.0.0.1:62915/git/akiselev/ghidra-cli" +repository = "https://github.com/akiselev/ghidra-cli" - - [dependencies] - # CLI framework ``` --- -### Milestone 2: Fix Compiler Warnings - Unused Imports +### Milestone 2: Wire IPC Client in main.rs -**Files**: -- `src/daemon/handler.rs` -- `src/daemon/queue.rs` -- `src/daemon/ipc_server.rs` -- `src/ghidra/bridge.rs` -- `src/ghidra/setup.rs` -- `src/query/mod.rs` -- `src/main.rs` - -**Flags**: `conformance` +**Files**: `src/main.rs` **Requirements**: -- Remove all unused import warnings -- Preserve imports that are actually used +- Replace `daemon_rpc::DaemonClient` with `ipc::client::DaemonClient` +- Route query commands through IPC when daemon is running +- Return error (not fallback) when daemon required but not running **Acceptance Criteria**: -- `cargo build 2>&1 | grep "unused import"` returns no results -- Project compiles successfully - -**Tests**: Skip - removing unused code, compilation is the test +- `ghidra query functions` uses IPC when daemon running +- `ghidra query functions` errors with "start daemon" message when daemon not running +- No fallback to HeadlessExecutor **Code Intent**: -- `handler.rs:9`: Remove `info`, `warn` from tracing import -- `queue.rs:11`: Remove `error` from tracing import -- `ipc_server.rs:16`: Remove `platform::Listener` from transport import -- `bridge.rs:9`: Remove `Path` from std::path import (keep PathBuf) -- `setup.rs:2`: Remove `Read`, `Seek` from std::io import (keep Write) -- `query/mod.rs:3`: Remove `FilterExpr` from filter import -- `main.rs:22`: Remove `error` from tracing import - -**Code Changes**: -```diff ---- a/src/daemon/handler.rs -+++ b/src/daemon/handler.rs -@@ -6,7 +6,7 @@ use std::sync::Arc; - - use serde_json::json; - use tokio::sync::Mutex; --use tracing::{debug, info, warn}; -+use tracing::debug; - - use crate::ghidra::bridge::GhidraBridge; - use crate::ipc::protocol::{Command, Response}; -``` - -```diff ---- a/src/daemon/queue.rs -+++ b/src/daemon/queue.rs -@@ -8,7 +8,7 @@ use std::sync::Arc; - - use anyhow::{Context, Result}; - use tokio::sync::{Mutex, Semaphore, oneshot}; --use tracing::{info, warn, error}; -+use tracing::{info, warn}; - - use crate::cli::Commands; - use crate::daemon::cache::Cache; -``` - -```diff ---- a/src/daemon/ipc_server.rs -+++ b/src/daemon/ipc_server.rs -@@ -13,7 +13,6 @@ use tracing::{debug, error, info}; - - use crate::ghidra::bridge::GhidraBridge; - use crate::ipc::protocol::{Command, Request, Response}; --use crate::ipc::transport::{self, platform::Listener}; -+use crate::ipc::transport; - - use super::handler; -``` - -```diff ---- a/src/ghidra/bridge.rs -+++ b/src/ghidra/bridge.rs -@@ -6,7 +6,7 @@ - - use std::io::{BufRead, BufReader, Write}; - use std::net::TcpStream; --use std::path::{Path, PathBuf}; -+use std::path::PathBuf; - use std::process::{Child, Command, Stdio}; - use std::sync::atomic::{AtomicBool, Ordering}; - use std::sync::Arc; -``` - -```diff ---- a/src/ghidra/setup.rs -+++ b/src/ghidra/setup.rs -@@ -1,5 +1,5 @@ - use std::fs::File; --use std::io::{Read, Write, Seek}; -+use std::io::Write; - use std::path::{Path, PathBuf}; - use anyhow::{Context, Result, anyhow}; - use futures_util::StreamExt; -``` - -```diff ---- a/src/query/mod.rs -+++ b/src/query/mod.rs -@@ -1,6 +1,6 @@ - use serde_json::Value as JsonValue; - use crate::error::{GhidraError, Result}; --use crate::filter::{Filter, FilterExpr}; -+use crate::filter::Filter; - use crate::format::{OutputFormat, Formatter, DefaultFormatter}; - use crate::ghidra::GhidraClient; - use crate::ghidra::headless::HeadlessExecutor; -``` - -```diff ---- a/src/main.rs -+++ b/src/main.rs -@@ -19,7 +19,7 @@ use ghidra::GhidraClient; - use query::{Query, DataType, FieldSelector, SortKey}; - use std::path::PathBuf; --use tracing::{info, error}; -+use tracing::info; - - #[cfg(unix)] - use daemonize::Daemonize; -``` +- In `run_with_daemon_check()`: Replace `daemon_rpc::DaemonClient::connect(port)` with `ipc::client::DaemonClient::connect(socket_path)` +- Add match on command type: query-based commands REQUIRE daemon, others can run standalone +- Remove the `else { run(cli) }` fallback for query commands +- Add helpful error message: "This command requires the daemon. Start with: ghidra daemon start --project " --- -### Milestone 3: Fix Compiler Warnings - Dead Code +### Milestone 3: Implement CommandQueue Execute -**Files**: -- `src/daemon/ipc_server.rs` -- `src/daemon/queue.rs` -- `src/config.rs` -- `src/daemon/cache.rs` -- `src/ghidra/data.rs` -- `src/daemon/state.rs` -- `src/ipc/client.rs` -- `src/ipc/transport.rs` -- `src/ipc/protocol.rs` -- `src/format/mod.rs` - -**Flags**: `conformance`, `needs-rationale` +**Files**: `src/daemon/queue.rs` **Requirements**: -- Address dead code warnings for methods/fields that won't be used -- For data.rs and state.rs: add #[allow(dead_code)] with comment explaining future use -- For truly dead code: remove it +- Implement `execute_command()` to actually execute commands via GhidraBridge +- Replace the TODO stub with real execution **Acceptance Criteria**: -- `cargo build 2>&1 | grep "never used\|never read\|never called"` returns no results OR only intentionally suppressed items -- Project compiles without warnings (or with only documented allowances) - -**Tests**: Skip - removing/suppressing unused code, compilation is the test +- Commands submitted to queue are executed via bridge +- Results are cached and returned **Code Intent**: -- `ipc_server.rs:25-27`: Remove `shutdown_tx` and `started_at` fields OR add #[allow(dead_code)] with comment if needed for future shutdown handling -- `queue.rs:115-138`: Remove `queue_depth()`, `queue_depth_async()`, `completed_count()`, `completed_count_async()` methods - they return hardcoded 0 or are never called -- `config.rs:153`: Remove `get_timeout()` if unused, or wire up to actual usage -- `cache.rs:82`: Remove `clear()`, `cleanup()` methods if unused -- `ghidra/data.rs`: Add `#[allow(dead_code)]` to module with comment "Data structures for future query type implementations" -- `daemon/state.rs`: Add `#[allow(dead_code)]` to DaemonState with comment "State tracking for daemon lifecycle management" -- `ipc/client.rs`, `ipc/transport.rs`, `ipc/protocol.rs`: Add `#[allow(dead_code)]` with comment "IPC infrastructure for daemon communication - preserved for v0.2 daemon mode" (Decision: "Keep IPC infrastructure despite current non-use") -- `format/mod.rs:47`: Remove `is_human_friendly()`, `is_machine_friendly()` if unused - -**Code Changes**: -```diff ---- a/src/daemon/ipc_server.rs -+++ b/src/daemon/ipc_server.rs -@@ -21,10 +21,6 @@ use super::handler; - pub struct IpcServer { - /// The Ghidra bridge instance - bridge: Arc>>, -- // Shutdown signal and timing are unused: current IPC implementation -- // delegates to the TCP-based daemon/rpc.rs for shutdown coordination -- /// Shutdown signal sender -- shutdown_tx: broadcast::Sender<()>, -- /// Server start time -- started_at: Instant, - } - - impl IpcServer { -@@ -35,8 +31,6 @@ impl IpcServer { - ) -> Self { - Self { - bridge, -- shutdown_tx, -- started_at: Instant::now(), - } - } -``` - -```diff ---- a/src/daemon/queue.rs -+++ b/src/daemon/queue.rs -@@ -111,34 +111,6 @@ impl CommandQueue { - }); - } - -- // Removed sync/async queue depth and completed count methods: -- // Sync versions returned hardcoded 0 (unreliable), async versions unused. -- // Decision: remove incomplete/unused methods to reduce dead code. -- /// Get the current queue depth. -- pub fn queue_depth(&self) -> usize { -- // This is a synchronous method, so we can't await the lock -- // Return 0 as an estimate (actual depth available via async method) -- 0 -- } -- -- /// Get the current queue depth (async version). -- pub async fn queue_depth_async(&self) -> usize { -- let queue = self.queue.lock().await; -- queue.len() -- } -- -- /// Get the number of completed commands. -- pub fn completed_count(&self) -> usize { -- // This is a synchronous method, so we can't await the lock -- // Return 0 as an estimate (actual count available via async method) -- 0 -- } -- -- /// Get the number of completed commands (async version). -- pub async fn completed_count_async(&self) -> usize { -- let count = self.completed_count.lock().await; -- *count -- } -- - /// Get the project path. - pub fn project_path(&self) -> &Path { - &self.project_path -``` - -```diff ---- a/src/config.rs -+++ b/src/config.rs -@@ -150,13 +150,6 @@ impl Config { - None - } - -- pub fn get_timeout(&self) -> u64 { -- std::env::var("GHIDRA_TIMEOUT") -- .ok() -- .and_then(|s| s.parse().ok()) -- .or(self.timeout) -- .unwrap_or(300) -- } -- - pub fn get_default_program(&self) -> Option { - std::env::var("GHIDRA_DEFAULT_PROGRAM") - .ok() -``` - -```diff ---- a/src/daemon/cache.rs -+++ b/src/daemon/cache.rs -@@ -78,20 +78,6 @@ impl Cache { - } - } - -- /// Clear all cached entries. -- pub async fn clear(&self) { -- let mut entries = self.entries.write().await; -- entries.clear(); -- debug!("Cache cleared"); -- } -- -- /// Remove expired entries. -- pub async fn cleanup(&self) { -- let mut entries = self.entries.write().await; -- let ttl = self.ttl; -- entries.retain(|_, entry| !entry.is_expired(ttl)); -- debug!("Cache cleanup completed"); -- } -- - /// Generate a cache key for a command. - /// Only cacheable commands return Some. - fn cache_key(&self, command: &Commands) -> Option { -``` - -```diff ---- a/src/ghidra/data.rs -+++ b/src/ghidra/data.rs -@@ -1,3 +1,5 @@ -+// Data structures for query type implementations -+#![allow(dead_code)] - use serde::{Deserialize, Serialize}; - - #[derive(Debug, Clone, Serialize, Deserialize)] -``` - -```diff ---- a/src/daemon/state.rs -+++ b/src/daemon/state.rs -@@ -1,6 +1,8 @@ - //! Daemon state management. - //! - //! Manages the state of loaded Ghidra projects and maintains metadata. -+// State tracking for daemon lifecycle management -+#![allow(dead_code)] - - use std::path::{Path, PathBuf}; - use std::sync::Arc; -``` - -```diff ---- a/src/ipc/client.rs -+++ b/src/ipc/client.rs -@@ -1,3 +1,5 @@ -+// IPC infrastructure: provides cross-platform local socket support for daemon communication -+#![allow(dead_code)] - // IPC client implementation will go here -``` - -```diff ---- a/src/ipc/transport.rs -+++ b/src/ipc/transport.rs -@@ -1,3 +1,5 @@ -+// IPC infrastructure: provides cross-platform local socket support for daemon communication -+#![allow(dead_code)] - // IPC transport layer implementation will go here -``` - -```diff ---- a/src/ipc/protocol.rs -+++ b/src/ipc/protocol.rs -@@ -1,3 +1,5 @@ -+// IPC infrastructure: provides cross-platform local socket support for daemon communication -+#![allow(dead_code)] - use serde::{Deserialize, Serialize}; -``` - -```diff ---- a/src/format/mod.rs -+++ b/src/format/mod.rs -@@ -44,14 +44,6 @@ impl OutputFormat { - } - } - -- pub fn is_human_friendly(&self) -> bool { -- matches!(self, Self::Full | Self::Compact | Self::Table | Self::Tree) -- } -- -- pub fn is_machine_friendly(&self) -> bool { -- matches!(self, Self::Json | Self::JsonCompact | Self::JsonStream | Self::Csv | Self::Tsv) -- } --} -- - pub trait Formatter { - fn format(&self, data: &[T], format: OutputFormat) -> Result; - } -``` +- Change `execute_command()` to take a reference to `GhidraBridge` +- Translate `Commands` enum to bridge operations +- Call `bridge.send_command()` for each operation type +- Parse JSON response and return formatted result --- -### Milestone 4: Implement XRefs Query Type +### Milestone 4: Add XRefs Support to Handler -**Files**: -- `src/query/mod.rs` -- `src/ghidra/headless.rs` -- `src/cli.rs` - -**Flags**: `conformance`, `needs-rationale` +**Files**: `src/daemon/handler.rs`, `src/ipc/protocol.rs` **Requirements**: -- Add XRefs case to Query::execute() in query/mod.rs -- Add get_xrefs_to() method to HeadlessExecutor -- Modify CLI to accept --to parameter for xrefs query -- Use existing get_xrefs_to_script() from scripts.rs +- Ensure XRefs commands are handled in the daemon +- Handler routes XRefsTo/XRefsFrom to bridge **Acceptance Criteria**: -- `ghidra query xrefs --to 0x401000 --program binary` returns JSON array of xrefs -- `ghidra query xrefs --to main --program binary` works with function name -- Output format matches other query types (filterable, sortable) - -**Tests**: -- **Test files**: `tests/e2e.rs` -- **Test type**: E2E -- **Backing**: user-specified (E2E only approach) -- **Scenarios**: - - Normal (name): Query xrefs using `--to main` (function name) returns results - - Normal (address): Query xrefs using `--to 0x` (numeric address) returns results - - Edge: Query xrefs to non-existent address returns empty array +- `ghidra query xrefs --to main` works via daemon +- Returns JSON array of cross-references **Code Intent**: -- `query/mod.rs:104-116`: Add `DataType::XRefs => executor.get_xrefs_to(project, program, target)?` case in match statement -- `query/mod.rs`: Add `target: Option` field to Query struct for XRefs target address -- `headless.rs`: Add `pub fn get_xrefs_to(&self, project: &str, program: &str, target: &str) -> Result` method using existing `get_xrefs_to_script()` pattern -- `cli.rs`: Add `--to
` parameter to query subcommand, required when data_type is xrefs (Decision: "XRefs requires target address") - -**Code Changes**: -```diff ---- a/src/query/mod.rs -+++ b/src/query/mod.rs -@@ -49,6 +49,7 @@ impl DataType { - - pub struct Query { - pub data_type: DataType, -+ // Target address for XRefs queries; unused by other query types. -+ // XRefs requires specifying where references point (a function or address). -+ pub target: Option, - pub filter: Option, - pub fields: Option, - pub format: OutputFormat, -@@ -62,6 +63,7 @@ impl Query { - pub fn new(data_type: DataType) -> Self { - Self { - data_type, -+ target: None, - filter: None, - fields: None, - format: OutputFormat::Json, -@@ -97,6 +99,11 @@ impl Query { - self - } - -+ pub fn with_target(mut self, target: String) -> Self { -+ self.target = Some(target); -+ self -+ } -+ - pub fn execute(&self, client: &GhidraClient, project: &str, program: &str) -> Result { - let executor = HeadlessExecutor::new(client); - -@@ -107,6 +114,11 @@ impl Query { - DataType::Imports => executor.list_imports(project, program)?, - DataType::Exports => executor.list_exports(project, program)?, - DataType::Memory => executor.get_memory_map(project, program)?, -+ DataType::XRefs => { -+ let target = self.target.as_ref() -+ .ok_or_else(|| GhidraError::Other("XRefs query requires --to parameter".to_string()))?; -+ executor.get_xrefs_to(project, program, target)? -+ } - _ => { - return Err(GhidraError::Other(format!( - "Data type {:?} not yet implemented", -``` - -```diff ---- a/src/cli.rs -+++ b/src/cli.rs -@@ -145,6 +145,10 @@ pub struct QueryArgs { - /// Filter expression - #[arg(short, long)] - pub filter: Option, -+ -+ /// Target address or function name (required for xrefs) -+ #[arg(long)] -+ pub to: Option, - - /// Field selection (comma-separated) - #[arg(long)] -``` - -```diff ---- a/src/main.rs -+++ b/src/main.rs -@@ -483,6 +483,11 @@ fn handle_query(args: QueryArgs) -> anyhow::Result<()> { - // Build query - let mut query = Query::new(data_type); - -+ // Target address for XRefs query type -+ if let Some(target) = args.to { -+ query = query.with_target(target); -+ } -+ - // Add filter if provided - if let Some(filter_str) = args.filter { - let filter = filter::Filter::parse(&filter_str)?; -``` - -Note: `get_xrefs_to()` method already exists in headless.rs at lines 182-190, no changes needed to headless.rs. +- Verify `Command::XRefsTo` and `Command::XRefsFrom` exist in protocol.rs +- Add handling in `handler.rs` for these commands +- Call `bridge.xrefs_to()` / `bridge.xrefs_from()` --- -### Milestone 5: Add XRefs E2E Test +### Milestone 5: Add --to Parameter for XRefs Query -**Files**: -- `tests/e2e.rs` +**Files**: `src/cli.rs`, `src/main.rs` **Requirements**: -- Add test for xrefs query command -- Follow existing test patterns (serial, timeout, ensure_project_setup) +- Add `--to` parameter to query subcommand +- Pass target to daemon when querying xrefs **Acceptance Criteria**: -- `cargo test test_xrefs -- --nocapture` passes -- Test verifies command returns success and valid output - -**Tests**: -- **Test files**: `tests/e2e.rs` -- **Test type**: E2E -- **Backing**: user-specified -- **Scenarios**: - - Normal: xrefs to main function succeeds +- `ghidra query xrefs --to main --project x --program y` parses correctly +- Target is sent to daemon in IPC request **Code Intent**: -- Add `test_xrefs_by_name()` function following pattern of `test_function_list()` - - Use `ensure_project_setup()` for fixture - - Query xrefs --to main with PROJECT_NAME and PROGRAM_NAME - - Assert success and stdout contains expected fields ("from", "to", "ref_type") -- Add `test_xrefs_by_address()` function - - Use `ensure_project_setup()` for fixture - - Query xrefs using --to with a known address (e.g., entry point from summary) - - Assert success -- Add `test_xrefs_nonexistent()` function for edge case - - Query xrefs --to 0xdeadbeef (invalid address) - - Assert success (returns empty array, not error) - -**Code Changes**: -```diff ---- a/tests/e2e.rs -+++ b/tests/e2e.rs -@@ -232,4 +232,67 @@ mod e2e_tests { - .assert() - .success() - .stdout(predicate::str::contains("Program Summary")); - } -+ -+ /// Test xrefs query by function name -+ #[test] -+ #[serial] -+ fn test_xrefs_by_name() { -+ ensure_project_setup(); -+ -+ let mut cmd = Command::cargo_bin("ghidra").unwrap(); -+ cmd.arg("query") -+ .arg("xrefs") -+ .arg("--to") -+ .arg("main") -+ .arg("--project") -+ .arg(PROJECT_NAME) -+ .arg("--program") -+ .arg(PROGRAM_NAME) -+ .timeout(std::time::Duration::from_secs(300)) -+ .assert() -+ .success() -+ .stdout(predicate::str::contains("from")) -+ .stdout(predicate::str::contains("to")) -+ .stdout(predicate::str::contains("ref_type")); -+ } -+ -+ /// Test xrefs query by address -+ #[test] -+ #[serial] -+ fn test_xrefs_by_address() { -+ ensure_project_setup(); -+ -+ // First get the entry point address from summary -+ let mut summary_cmd = Command::cargo_bin("ghidra").unwrap(); -+ let summary_output = summary_cmd -+ .arg("summary") -+ .arg("--project") -+ .arg(PROJECT_NAME) -+ .arg("--program") -+ .arg(PROGRAM_NAME) -+ .timeout(std::time::Duration::from_secs(300)) -+ .output() -+ .expect("Failed to get summary"); -+ -+ // Query xrefs using a hardcoded entry point (typical for x86_64 ELF) -+ let mut cmd = Command::cargo_bin("ghidra").unwrap(); -+ cmd.arg("query") -+ .arg("xrefs") -+ .arg("--to") -+ .arg("0x00100000") // Common entry point for test binary -+ .arg("--project") -+ .arg(PROJECT_NAME) -+ .arg("--program") -+ .arg(PROGRAM_NAME) -+ .timeout(std::time::Duration::from_secs(300)) -+ .assert() -+ .success(); -+ } -+ -+ /// Test xrefs query to nonexistent address (should return empty array) -+ #[test] -+ #[serial] -+ fn test_xrefs_nonexistent() { -+ ensure_project_setup(); -+ -+ let mut cmd = Command::cargo_bin("ghidra").unwrap(); -+ cmd.arg("query") -+ .arg("xrefs") -+ .arg("--to") -+ .arg("0xdeadbeef") -+ .arg("--project") -+ .arg(PROJECT_NAME) -+ .arg("--program") -+ .arg(PROGRAM_NAME) -+ .timeout(std::time::Duration::from_secs(300)) -+ .assert() -+ .success() -+ .stdout(predicate::str::contains("[]")); -+ } - } -``` +- Add `to: Option` to QueryArgs in cli.rs +- In main.rs query handling, include target in IPC command --- -### Milestone 6: Expand README.md +### Milestone 6: Deprecate HeadlessExecutor -**Files**: -- `README.md` +**Files**: `src/ghidra/headless.rs`, `src/ghidra/mod.rs` **Requirements**: -- Comprehensive README for open source release -- Installation instructions (cargo install, from source) -- Quick start guide with examples -- Feature overview -- Link to CLAUDE_SKILL.md for AI agent integration -- License and contributing sections +- Mark HeadlessExecutor as deprecated +- Remove direct calls from main.rs +- Keep module for potential future use (import/analyze operations) **Acceptance Criteria**: -- Contains all 9 sections: Overview, Features, Installation, Quick Start, CLI Reference, AI Agent Integration, Configuration, Development, Contributing, License -- Links to CLAUDE_SKILL.md in AI Agent Integration section -- Each section contains at least one code example or substantive content - -**Tests**: Skip - documentation only +- No query operations use HeadlessExecutor +- Compile succeeds with deprecation warnings only **Code Intent**: -- Replace 4-line README with comprehensive documentation -- Sections: Overview, Features, Installation, Quick Start, CLI Reference (brief), AI Agent Integration (link to CLAUDE_SKILL.md), Configuration, Development, Contributing, License -- Include code examples for: ghidra doctor, ghidra import, ghidra query functions, ghidra decompile - -**Code Changes**: -```diff ---- a/README.md -+++ b/README.md -@@ -1,4 +1,215 @@ - # Ghidra CLI - - A high-performance Rust CLI for automating Ghidra reverse engineering tasks, designed for both direct usage and AI agent integration (like Claude Code). - -+## Overview -+ -+`ghidra-cli` is a Rust-based command-line interface for Ghidra, the NSA's reverse engineering platform. It enables: -+ -+- **Headless automation**: Run Ghidra analysis without the GUI -+- **Query-based data extraction**: Functions, strings, imports, exports, memory maps, cross-references -+- **Decompilation**: Extract C pseudocode from binaries -+- **Project management**: Create, import, and manage Ghidra projects from the command line -+- **AI agent integration**: Structured output formats (JSON, CSV) for use with Claude Code and other AI tools -+ -+## Features -+ -+- **Universal query system**: Query any Ghidra data type (functions, strings, xrefs, etc.) with filtering, sorting, and pagination -+- **Multiple output formats**: JSON, CSV, TSV, table, minimal -+- **Headless execution**: Runs Ghidra scripts without opening the GUI -+- **Built-in setup**: Automatic Ghidra download and installation -+- **Fast**: Rust implementation with optimized Jython scripts -+- **Type-safe**: Rust's type system ensures reliable operations -+ -+## Installation -+ -+### Prerequisites -+ -+- **JDK 17+** (required by Ghidra) -+- **Ghidra 10.x+** (can be auto-installed via `ghidra setup`) -+ -+### From source -+ -+```bash -+git clone https://github.com/akiselev/ghidra-cli -+cd ghidra-cli -+cargo build --release -+cargo install --path . -+``` -+ -+### Install Ghidra -+ -+If you don't have Ghidra installed, use the built-in setup command: -+ -+```bash -+ghidra setup -+``` -+ -+This downloads and installs the latest Ghidra release automatically. -+ -+## Quick Start -+ -+### 1. Verify installation -+ -+```bash -+ghidra doctor -+``` -+ -+### 2. Import a binary -+ -+```bash -+ghidra import /path/to/binary --project my-project --program my-binary -+``` -+ -+### 3. Query functions -+ -+```bash -+# List all functions -+ghidra query functions --project my-project --program my-binary -+ -+# List functions with filtering -+ghidra query functions --project my-project --program my-binary --filter "size > 100" -+ -+# Output as CSV -+ghidra query functions --project my-project --program my-binary --format csv -+``` -+ -+### 4. Decompile a function -+ -+```bash -+ghidra decompile main --project my-project --program my-binary -+``` -+ -+### 5. Query cross-references -+ -+```bash -+# XRefs to main function -+ghidra query xrefs --to main --project my-project --program my-binary -+ -+# XRefs to a specific address -+ghidra query xrefs --to 0x401000 --project my-project --program my-binary -+``` -+ -+### Quick analysis -+ -+For a one-shot analysis without setting up a project: -+ -+```bash -+ghidra quick /path/to/binary -+``` -+ -+This imports, analyzes, and displays a summary in one command. -+ -+## CLI Reference -+ -+### Core Commands -+ -+- `ghidra query ` - Query Ghidra data (functions, strings, imports, exports, xrefs, memory) -+- `ghidra decompile ` - Decompile a function by name or address -+- `ghidra import ` - Import a binary into a project -+- `ghidra analyze` - Run Ghidra analysis on a program -+- `ghidra summary` - Display program summary -+- `ghidra quick ` - Quick analysis (import + analyze + summary) -+ -+### Project Management -+ -+- `ghidra project create ` - Create a new project -+- `ghidra project list` - List all projects -+- `ghidra project info ` - Show project details -+- `ghidra project delete ` - Delete a project -+ -+### Configuration -+ -+- `ghidra config list` - Show current configuration -+- `ghidra config get ` - Get a specific config value -+- `ghidra config set ` - Set a config value -+- `ghidra set-default program ` - Set default program -+- `ghidra set-default project ` - Set default project -+ -+### Utilities -+ -+- `ghidra doctor` - Verify installation and configuration -+- `ghidra init` - Initialize configuration -+- `ghidra setup` - Download and install Ghidra -+- `ghidra version` - Show version information -+ -+### Query Options -+ -+All query commands support: -+ -+- `--filter ` - Filter results (e.g., `"name LIKE main"`, `"size > 100"`) -+- `--fields ` - Select specific fields (comma-separated) -+- `--sort ` - Sort by field (prefix with `-` for descending) -+- `--limit ` - Limit number of results -+- `--offset ` - Skip first N results -+- `--format ` - Output format (json, csv, tsv, table, minimal) -+- `--count` - Only return count of results -+ -+## AI Agent Integration -+ -+ghidra-cli is designed for AI agent integration. See [CLAUDE_SKILL.md](CLAUDE_SKILL.md) for detailed usage with Claude Code, including: -+ -+- Skill configuration for Claude Code -+- Example workflows and commands -+- Advanced usage patterns -+- Tool integration examples -+ -+### Example: Using with Claude Code -+ -+```bash -+# Claude can use this to analyze a binary -+ghidra query functions --project malware-analysis --program sample.exe --format json -+``` -+ -+The JSON output is structured for parsing by AI agents, enabling automated reverse engineering workflows. -+ -+## Configuration -+ -+Configuration is stored in `~/.config/ghidra-cli/config.yaml` (Linux/macOS) or `%APPDATA%\ghidra-cli\config.yaml` (Windows). -+ -+### Environment Variables -+ -+- `GHIDRA_INSTALL_DIR` - Path to Ghidra installation -+- `GHIDRA_PROJECT_DIR` - Default project directory -+- `GHIDRA_DEFAULT_PROJECT` - Default project name -+- `GHIDRA_DEFAULT_PROGRAM` - Default program name -+- `GHIDRA_TIMEOUT` - Timeout for Ghidra operations (seconds) -+ -+### Configuration File -+ -+```yaml -+ghidra_install_dir: /path/to/ghidra_10.4_PUBLIC -+ghidra_project_dir: ~/git -+default_project: my-project -+default_program: my-binary -+default_output_format: json -+timeout: 300 -+``` -+ -+## Development -+ -+### Building from source -+ -+```bash -+cargo build --release -+``` -+ -+### Running tests -+ -+```bash -+# Unit tests -+cargo test -+ -+# E2E tests (requires Ghidra installation) -+cargo test --test e2e -+``` -+ -+## Contributing -+ -+Contributions are welcome! Please open an issue or pull request on [GitHub](https://github.com/akiselev/ghidra-cli). -+ -+## License -+ -+GPL-3.0 - See [LICENSE](LICENSE) for details. -+ -+Ghidra is developed by the National Security Agency and is licensed separately under the Apache License 2.0. -``` +- Add `#[deprecated(note = "Use daemon for query operations")]` to HeadlessExecutor +- Remove/comment out direct HeadlessExecutor usage in main.rs handle_* functions +- Route through daemon instead --- -### Milestone 7: Documentation +### Milestone 7: Fix Compiler Warnings -**Delegated to**: @agent-technical-writer (mode: post-implementation) - -**Source**: `## Invisible Knowledge` section of this plan - -**Files**: -- `src/query/README.md` (query system architecture) -- `src/ghidra/README.md` (Ghidra integration details) +**Files**: Multiple (same as original plan) **Requirements**: -- Document query system data flow -- Document XRefs implementation rationale -- Reference Decision Log for architectural choices +- Remove unused imports +- Handle dead code appropriately **Acceptance Criteria**: -- README.md files explain non-obvious design decisions -- Architecture diagrams match Invisible Knowledge section -- Self-contained (no external documentation references) +- `cargo build` produces no warnings (or only deprecation warnings for HeadlessExecutor) + +**Code Intent**: +- Remove unused imports from handler.rs, queue.rs, etc. +- Keep IPC layer code (now actually used!) +- Remove truly dead methods + +--- + +### Milestone 8: Add E2E Tests for Daemon Queries + +**Files**: `tests/e2e.rs` + +**Requirements**: +- Test query operations through daemon +- Test XRefs query + +**Acceptance Criteria**: +- Tests start daemon, run queries, stop daemon +- All tests pass + +**Code Intent**: +- Add test helper to start/stop daemon +- Add `test_daemon_query_functions()` +- Add `test_daemon_query_xrefs()` +- Add `test_daemon_required_error()` - verify error when daemon not running + +--- + +### Milestone 9: Expand README.md + +**Files**: `README.md` + +**Requirements**: +- Document daemon-first architecture +- Explain why daemon is required for queries +- Quick start with daemon workflow + +**Acceptance Criteria**: +- README explains daemon requirement +- Includes daemon workflow example + +**Code Intent**: +- Expand README with: + - Overview explaining persistent Ghidra benefit + - Quick start: `ghidra daemon start`, then queries + - Architecture section explaining daemon design + - All 9 sections from original plan + +--- ## Milestone Dependencies ``` M1 (Cargo.toml) ----+ | -M2 (Imports) ----+----> M4 (XRefs) ----> M5 (E2E Test) +M2 (IPC Client) ----+----> M4 (XRefs Handler) ----> M5 (--to param) | -M3 (Dead Code) ----+ +M3 (Queue Execute) -+----> M6 (Deprecate Headless) | - +----> M6 (README) ----> M7 (Docs) + +----> M7 (Warnings) ----> M8 (Tests) ----> M9 (README) ``` -**Parallel execution**: M1, M2, M3 can run in parallel (no dependencies) -**Sequential**: M4 requires M2/M3 (clean compilation), M5 requires M4 (feature exists), M7 requires M6 (README first) +## Architecture After Changes + +``` +CLI Command + ↓ +[Command Type?] + ├─ Daemon Control (start/stop) → Handle locally + ├─ Config/Setup/Doctor → Handle locally + └─ Query/Decompile/etc → REQUIRES DAEMON + ↓ + [Daemon Running?] + ├─ NO → Error: "Start daemon first" + └─ YES → IPC Client + ↓ + Send Command (local socket) + ↓ + IPC Server receives + ↓ + handler.rs routes + ↓ + GhidraBridge.send_command() + ↓ + bridge.py in Ghidra + ↓ + Response back + ↓ + Format & display +``` + +## Key Benefits + +1. **Faster queries**: Ghidra stays loaded, no 5-30s startup per command +2. **Simpler code**: One execution path, not two +3. **Better UX**: Consistent behavior, clear daemon requirement +4. **Easier debugging**: All queries go through same path