From 7e60ac218c561602fa3361fe4909e170a5c0b734 Mon Sep 17 00:00:00 2001 From: Jayson Grace Date: Sat, 9 May 2026 21:37:20 -0600 Subject: [PATCH 1/2] feat: enforce operation scope for single-IP tool invocations and improve SAM/domain parsing **Added:** - Enforced operation scope for single-IP tool calls by installing `ares_tools::scope` in orchestrator and worker entrypoints - Introduced `ares-tools::scope` module to restrict tool calls to authorized IPs, with environment-driven configuration and validation logic - Added `scope::validate_in_scope` check to tool dispatch to reject out-of-scope targets before execution - Implemented deduplication of adjacent labels in FQDNs during nmap parsing to clean up self-named workgroup hostnames - Added comprehensive tests for operation scope logic, FQDN deduplication, and secrets parsing behaviors **Changed:** - Updated nmap output parser to collapse duplicate first labels in FQDNs, preventing malformed hostnames from propagating - Improved secretsdump parser to recognize both impacket and nxc/netexec SAM section markers, ensuring correct attribution of local users - Refined logic for distinguishing between local SAM and domain accounts in secretsdump parser: - Default to local SAM for unmarked sections unless the user is a machine account or krbtgt - In local SAM sections, always strip computer name prefix from usernames, preventing accidental domain attribution - Provided additional tests for edge cases involving SAM, domain, and machine account handling **Removed:** - Eliminated silent inheritance of `target_domain` for unmarked custom user rows in secretsdump parser to prevent local users from being attributed to AD scope --- ares-cli/src/orchestrator/mod.rs | 13 ++ ares-cli/src/worker/tool_executor.rs | 13 ++ ares-tools/src/lib.rs | 7 +- ares-tools/src/parsers/nmap.rs | 65 +++++++++- ares-tools/src/parsers/secrets.rs | 138 ++++++++++++++++---- ares-tools/src/scope.rs | 185 +++++++++++++++++++++++++++ 6 files changed, 389 insertions(+), 32 deletions(-) create mode 100644 ares-tools/src/scope.rs diff --git a/ares-cli/src/orchestrator/mod.rs b/ares-cli/src/orchestrator/mod.rs index 088b2c07..80214f91 100644 --- a/ares-cli/src/orchestrator/mod.rs +++ b/ares-cli/src/orchestrator/mod.rs @@ -105,6 +105,19 @@ async fn run_inner() -> Result<()> { "Configuration loaded" ); + // Install the operation scope so `ares_tools::dispatch` rejects single-IP + // tool invocations against hosts the operator never authorized. Empty + // target_ips → unrestricted (legacy/test launches that didn't pass IPs). + ares_tools::scope::init_scope(ares_tools::scope::OperationScope::new( + config.target_ips.clone(), + )); + if !config.target_ips.is_empty() { + info!( + target_ips = %config.target_ips.join(","), + "Installed operation scope — out-of-scope single-IP tool calls will be rejected" + ); + } + let queue = TaskQueue::connect(&config.redis_url, &config.nats_url) .await .context("Failed to connect to Redis/NATS")?; diff --git a/ares-cli/src/worker/tool_executor.rs b/ares-cli/src/worker/tool_executor.rs index 7c66720f..c54ab09d 100644 --- a/ares-cli/src/worker/tool_executor.rs +++ b/ares-cli/src/worker/tool_executor.rs @@ -72,6 +72,19 @@ pub async fn run_tool_exec_loop( status_tx: tokio::sync::watch::Sender, shutdown: Arc, ) -> anyhow::Result<()> { + // Install the operation scope from ARES_OPERATION_ID so out-of-scope + // single-IP tool calls get rejected before any subprocess runs. The worker + // doesn't otherwise parse target_ips out of the env JSON; this is the + // only path that needs them. + let scope = ares_tools::scope::OperationScope::from_env(); + if !scope.is_unrestricted() { + info!( + target_ips = %scope.target_ips().join(","), + "Worker installed operation scope — out-of-scope single-IP tool calls will be rejected" + ); + } + ares_tools::scope::init_scope(scope); + let subject = nats::tool_exec_subject(&config.worker_role); let queue_group = format!("ares-tools-{}", config.worker_role); diff --git a/ares-tools/src/lib.rs b/ares-tools/src/lib.rs index 46f90016..beb4ce0c 100644 --- a/ares-tools/src/lib.rs +++ b/ares-tools/src/lib.rs @@ -18,6 +18,7 @@ pub mod lateral; pub mod parsers; pub mod privesc; pub mod recon; +pub mod scope; use anyhow::Result; use serde_json::Value; @@ -63,8 +64,12 @@ impl ToolOutput { /// Dispatch a tool call by name, executing the corresponding CLI command. /// -/// Returns the tool output or an error if the tool is unknown or execution fails. +/// Returns the tool output or an error if the tool is unknown or execution +/// fails. Calls whose `target` / `target_ip` is a literal IP outside the +/// configured operation scope are rejected before any subprocess runs (see +/// [`scope::validate_in_scope`]). pub async fn dispatch(tool_name: &str, arguments: &Value) -> Result { + scope::validate_in_scope(tool_name, arguments)?; match tool_name { // ── Reconnaissance ────────────────────────────────────────── "nmap_scan" => recon::nmap_scan(arguments).await, diff --git a/ares-tools/src/parsers/nmap.rs b/ares-tools/src/parsers/nmap.rs index c1c83755..99315144 100644 --- a/ares-tools/src/parsers/nmap.rs +++ b/ares-tools/src/parsers/nmap.rs @@ -2,6 +2,29 @@ use serde_json::{json, Value}; +/// Collapse adjacent duplicate labels in an FQDN (`host.host.suffix` → +/// `host.suffix`). Self-named-workgroup hosts (typical of stock Windows +/// installs that aren't domain-joined) report their reverse-DNS as +/// `name.name.workgroup`, which then propagates into recon output and host +/// records as a malformed FQDN. Real AD names don't repeat the leading label, +/// so collapsing is safe in practice. +fn dedup_adjacent_labels(fqdn: &str) -> String { + let labels: Vec<&str> = fqdn.split('.').collect(); + if labels.len() < 3 { + return fqdn.to_string(); + } + let mut out: Vec<&str> = Vec::with_capacity(labels.len()); + for label in labels { + if let Some(prev) = out.last() { + if prev.eq_ignore_ascii_case(label) { + continue; + } + } + out.push(label); + } + out.join(".") +} + pub fn parse_nmap_output(output: &str, params: &Value) -> Vec { let target_ip = params .get("target") @@ -32,7 +55,7 @@ pub fn parse_nmap_output(output: &str, params: &Value) -> Vec { let rest = line.trim_start_matches("Nmap scan report for").trim(); if let Some(paren_start) = rest.find('(') { - hostname = rest[..paren_start].trim().to_string(); + hostname = dedup_adjacent_labels(rest[..paren_start].trim()); current_ip = rest[paren_start + 1..] .trim_end_matches(')') .trim() @@ -88,7 +111,7 @@ pub fn parse_nmap_output(output: &str, params: &Value) -> Vec { if let Some(rest) = trimmed.strip_prefix(prefix) { let fqdn = rest.trim().to_lowercase(); if fqdn.contains('.') && !fqdn.contains(' ') { - hostname = fqdn; + hostname = dedup_adjacent_labels(&fqdn); break; } } @@ -105,7 +128,7 @@ pub fn parse_nmap_output(output: &str, params: &Value) -> Vec { .trim() .to_lowercase(); if cn.contains('.') && !cn.contains(' ') { - hostname = cn; + hostname = dedup_adjacent_labels(&cn); } } } @@ -121,7 +144,7 @@ pub fn parse_nmap_output(output: &str, params: &Value) -> Vec { .trim() .to_lowercase(); if dns.contains('.') && !dns.contains(' ') { - hostname = dns; + hostname = dedup_adjacent_labels(&dns); } } } @@ -428,4 +451,38 @@ PORT STATE SERVICE VERSION let role_strs: Vec<&str> = roles.iter().filter_map(|v| v.as_str()).collect(); assert!(role_strs.contains(&"winrm")); } + + #[test] + fn dedup_adjacent_labels_collapses_doubled_first_label() { + // Self-named-workgroup hosts (stock Windows installs not domain-joined) + // report reverse-DNS as `host.host.workgroup`. Collapse the doubled + // label so the recorded FQDN is `host.workgroup`. + assert_eq!( + dedup_adjacent_labels("dc01.dc01.contoso.local"), + "dc01.contoso.local" + ); + assert_eq!( + dedup_adjacent_labels("dc01.contoso.local"), + "dc01.contoso.local" + ); + assert_eq!(dedup_adjacent_labels("contoso.local"), "contoso.local"); + // Case-insensitive match — preserves the case of the first occurrence + assert_eq!( + dedup_adjacent_labels("DC01.dc01.contoso.local"), + "DC01.contoso.local" + ); + } + + #[test] + fn parse_nmap_dedupes_doubled_reverse_dns() { + // Reverse-DNS for a Win2003-style self-named workgroup host comes back + // as `host.host.workgroup`. The recorded hostname must be collapsed. + let output = "\ +Nmap scan report for dc01.dc01.contoso.local (192.168.58.10) +445/tcp open microsoft-ds"; + let params = json!({"target": "192.168.58.10"}); + let hosts = parse_nmap_output(output, ¶ms); + assert_eq!(hosts.len(), 1); + assert_eq!(hosts[0]["hostname"], "dc01.contoso.local"); + } } diff --git a/ares-tools/src/parsers/secrets.rs b/ares-tools/src/parsers/secrets.rs index 6ea85c9a..3cd1c4c2 100644 --- a/ares-tools/src/parsers/secrets.rs +++ b/ares-tools/src/parsers/secrets.rs @@ -33,16 +33,20 @@ pub fn parse_secretsdump(output: &str, params: &Value) -> (Vec, Vec (Vec, Vec= 4 { let raw_user = parts[0]; let rid = parts.get(1).copied().unwrap_or(""); - let (user_domain, username) = if raw_user.contains('\\') { + let (user_domain, username) = if section == DumpSection::LocalSam { + // In the local SAM section, any `\` prefix is the host's + // own computer name (or workgroup), never an AD realm. + // Strip it and leave the domain empty — otherwise a + // standalone host whose computer name happens to share its + // first label with `target_domain` (e.g. WIN-XXXX with a + // self-named WIN-XXXX.WGRP.LOCAL workgroup) gets attributed + // to that workgroup as if it were an AD domain. + let user = raw_user.split_once('\\').map_or(raw_user, |(_, u)| u); + (String::new(), user.to_string()) + } else if raw_user.contains('\\') { let split: Vec<&str> = raw_user.splitn(2, '\\').collect(); let netbios = split[0]; // Resolve NetBIOS domain prefix to FQDN using target_domain. @@ -97,31 +111,41 @@ pub fn parse_secretsdump(output: &str, params: &Value) -> (Vec, Vec bool { if section == DumpSection::LocalSam { return true; } + let name = raw_user.to_ascii_lowercase(); // RID-based: 500/501/503/504 are well-known built-ins. Don't include 502 // (krbtgt) — it's a domain account that happens to share a fixed RID. - if matches!(rid, "500" | "501" | "503" | "504") { - let name = raw_user.to_ascii_lowercase(); - if matches!( + if matches!(rid, "500" | "501" | "503" | "504") + && matches!( name.as_str(), "administrator" | "guest" | "defaultaccount" | "wdagutilityaccount" - ) { - return true; - } + ) + { + return true; } // LSA pseudo-rows from `[*] Dumping LSA Secrets` — `$MACHINE.ACC`, etc. if raw_user.starts_with('$') || raw_user.starts_with("_SC_") || raw_user.starts_with("NL$") { return true; } + // Safe default for unmarked dumps: treat as local SAM. krbtgt and machine + // accounts (`ENDS_WITH$`) are never local — let those fall through to the + // target_domain branch. + if section == DumpSection::Unknown && name != "krbtgt" && !raw_user.ends_with('$') { + return true; + } false } @@ -270,23 +294,83 @@ WIN-XYZ$:1001:aad3b435b51404eeaad3b435b51404ee:1234567890abcdef1234567890abcdef: } #[test] - fn parse_secretsdump_well_known_sam_in_unknown_section() { - // No section marker before the rows — fall back to the well-known - // RID/name signal. Administrator/500 and DefaultAccount/503 are - // always local; svc_custom/1001 stays attributed to target_domain. + fn parse_secretsdump_unknown_section_defaults_to_local_sam() { + // No section marker before the rows — safe default is local SAM + // attribution (empty domain). NTDS dumps reliably emit pekList/NTDS + // markers; an unmarked dump is almost always a SAM dump from + // `secretsdump @host` or `nxc smb --sam`. Custom RIDs like 1001 must + // not silently inherit `target_domain` — that's how Ansible-provisioned + // local users (e.g. on standalone hosts) leak into AD scope. let output = "\ Administrator:500:aad3b435b51404eeaad3b435b51404ee:e19ccf75ee54e06b06a5907af13cef42::: DefaultAccount:503:aad3b435b51404eeaad3b435b51404ee:abcdef1234567890abcdef1234567890::: -svc_custom:1001:aad3b435b51404eeaad3b435b51404ee:1234567890abcdef1234567890abcdef:::"; +ansible:1001:aad3b435b51404eeaad3b435b51404ee:1234567890abcdef1234567890abcdef:::"; let params = json!({"target_domain": "contoso.local"}); let (hashes, _) = parse_secretsdump(output, ¶ms); assert_eq!(hashes.len(), 3); + for h in &hashes { + assert_eq!( + h["domain"], "", + "{} should not inherit target_domain", + h["username"] + ); + } + } + + #[test] + fn parse_secretsdump_nxc_style_sam_marker() { + // nxc/netexec emits `[*] Dumping SAM hashes` (no "local") before rows. + // The parser must recognize this variant and still treat the section + // as LocalSam — otherwise unmarked custom users fall through to + // target_domain attribution. + let output = "\ +[*] Dumping SAM hashes +Administrator:500:aad3b435b51404eeaad3b435b51404ee:e19ccf75ee54e06b06a5907af13cef42::: +ansible:1001:aad3b435b51404eeaad3b435b51404ee:abcdef1234567890abcdef1234567890:::"; + let params = json!({"target_domain": "contoso.local"}); + let (hashes, _) = parse_secretsdump(output, ¶ms); + assert_eq!(hashes.len(), 2); assert_eq!(hashes[0]["username"], "Administrator"); assert_eq!(hashes[0]["domain"], ""); - assert_eq!(hashes[1]["username"], "DefaultAccount"); + assert_eq!(hashes[1]["username"], "ansible"); assert_eq!(hashes[1]["domain"], ""); - assert_eq!(hashes[2]["username"], "svc_custom"); - assert_eq!(hashes[2]["domain"], "contoso.local"); + } + + #[test] + fn parse_secretsdump_local_sam_strips_computer_name_prefix() { + // Standalone host with self-named workgroup dumps rows like + // `WIN-ABCDEFGHIJK\ansible:1001:...`. The prefix is the host's own + // computer name, NOT an AD NetBIOS realm — even when the operator's + // `target_domain` happens to be `win-abcdefghijk.wgrp.local` (which + // would otherwise pass the first-label match in + // `resolve_netbios_to_fqdn`). In LocalSam section, the prefix is + // always stripped and the domain is left empty. + let output = "\ +[*] Dumping local SAM hashes (uid:rid:lmhash:nthash) +WIN-ABCDEFGHIJK\\Administrator:500:aad3b435b51404eeaad3b435b51404ee:e19ccf75ee54e06b06a5907af13cef42::: +WIN-ABCDEFGHIJK\\ansible:1001:aad3b435b51404eeaad3b435b51404ee:abcdef1234567890abcdef1234567890:::"; + let params = json!({"target_domain": "win-abcdefghijk.wgrp.local"}); + let (hashes, _) = parse_secretsdump(output, ¶ms); + assert_eq!(hashes.len(), 2); + for h in &hashes { + assert_eq!(h["domain"], ""); + } + assert_eq!(hashes[0]["username"], "Administrator"); + assert_eq!(hashes[1]["username"], "ansible"); + } + + #[test] + fn parse_secretsdump_machine_account_unmarked_keeps_target_domain() { + // Machine accounts (ending in `$`) are AD-only, never local SAM. + // Even with no section marker, they must inherit target_domain so a + // partial NTDS dump doesn't lose its computer-account hashes. + let output = + "WIN-XYZ$:1001:aad3b435b51404eeaad3b435b51404ee:1234567890abcdef1234567890abcdef:::"; + let params = json!({"target_domain": "contoso.local"}); + let (hashes, _) = parse_secretsdump(output, ¶ms); + assert_eq!(hashes.len(), 1); + assert_eq!(hashes[0]["username"], "WIN-XYZ$"); + assert_eq!(hashes[0]["domain"], "contoso.local"); } #[test] diff --git a/ares-tools/src/scope.rs b/ares-tools/src/scope.rs new file mode 100644 index 00000000..bc964cec --- /dev/null +++ b/ares-tools/src/scope.rs @@ -0,0 +1,185 @@ +//! Operation scope enforcement. +//! +//! The orchestrator launches each operation with a fixed set of target IPs +//! (the engagement scope). Without enforcement, an LLM agent that runs a +//! discovery sweep can pull in extra hosts on the same subnet — including the +//! attacker's own management box, lab infrastructure, or unrelated standalones +//! — and then run secretsdump/psexec/etc. against them. The resulting loot is +//! pollution at best and unauthorized access at worst. +//! +//! This module rejects single-target tool invocations whose `target` / +//! `target_ip` field is a literal IPv4 address that doesn't appear in the +//! operation's configured target list. Sweep-style invocations (CIDR, comma +//! lists, hostnames) are passed through — they're discovery, not attack — and +//! the validation kicks in again on whatever single-target tool the agent runs +//! against the discovered hosts. + +use std::net::Ipv4Addr; +use std::sync::OnceLock; + +use anyhow::{anyhow, Result}; +use serde_json::Value; + +/// In-scope target IPs for the active operation. Empty = unrestricted (test +/// mode, ad-hoc tool runs, single-binary deployments without an operation). +#[derive(Debug, Clone, Default)] +pub struct OperationScope { + target_ips: Vec, +} + +impl OperationScope { + pub fn new(target_ips: Vec) -> Self { + Self { target_ips } + } + + /// Build a scope from `ARES_OPERATION_ID`, mirroring the parse used by + /// `OrchestratorConfig::from_env_with_yaml`. Returns an empty (= + /// unrestricted) scope when the env var is unset, plain-text, or + /// missing a `target_ips` array — none of those cases are an error here. + pub fn from_env() -> Self { + let raw = match std::env::var("ARES_OPERATION_ID") { + Ok(v) => v, + Err(_) => return Self::default(), + }; + let json_start = match raw.find('{') { + Some(i) => i, + None => return Self::default(), + }; + let json: serde_json::Value = match serde_json::from_str(&raw[json_start..]) { + Ok(v) => v, + Err(_) => return Self::default(), + }; + let ips = json["target_ips"] + .as_array() + .map(|arr| { + arr.iter() + .filter_map(|v| v.as_str().map(|s| s.to_string())) + .collect() + }) + .unwrap_or_default(); + Self::new(ips) + } + + pub fn is_unrestricted(&self) -> bool { + self.target_ips.is_empty() + } + + pub fn contains(&self, ip: &str) -> bool { + self.target_ips.iter().any(|t| t == ip) + } + + pub fn target_ips(&self) -> &[String] { + &self.target_ips + } +} + +static SCOPE: OnceLock = OnceLock::new(); + +/// Install the process-wide operation scope. First call wins; subsequent calls +/// are no-ops so re-initialization (e.g. test setup, hot-reload) is safe. +pub fn init_scope(scope: OperationScope) { + let _ = SCOPE.set(scope); +} + +fn current_scope() -> &'static OperationScope { + SCOPE.get_or_init(OperationScope::default) +} + +/// Validate that `arguments` only targets in-scope hosts. +/// +/// Only checked when the field is a literal IPv4 address — CIDRs, comma- +/// separated lists, hostnames, and `localhost` pass through. The agent stays +/// free to do legitimate discovery; the gate fires when it tries to run a +/// single-target attack tool against a host nobody authorized. +pub fn validate_in_scope(tool: &str, arguments: &Value) -> Result<()> { + let scope = current_scope(); + if scope.is_unrestricted() { + return Ok(()); + } + for field in ["target", "target_ip"] { + let Some(val) = arguments.get(field).and_then(|v| v.as_str()) else { + continue; + }; + // Only enforce on literal IPv4 — sweeps pass a CIDR or list, single + // attacks pass a single IP. Hostnames are caught by the parser-side + // attribution fixes; we don't try to resolve them here. + if val.parse::().is_err() { + continue; + } + if val == "127.0.0.1" { + continue; + } + if !scope.contains(val) { + return Err(anyhow!( + "tool '{tool}' rejected: target {val} is not in operation scope ({})", + scope.target_ips.join(",") + )); + } + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn unrestricted_scope_passes_everything() { + let scope = OperationScope::default(); + assert!(scope.is_unrestricted()); + // Even literal IPs pass when scope is empty + assert!(!scope.contains("192.168.58.10")); + } + + #[test] + fn scoped_contains_membership() { + let scope = OperationScope::new(vec!["192.168.58.10".into(), "192.168.58.20".into()]); + assert!(!scope.is_unrestricted()); + assert!(scope.contains("192.168.58.10")); + assert!(!scope.contains("192.168.58.30")); + } + + #[test] + fn validate_passes_in_scope_ip() { + // Force scope into the static for this test to work — but OnceLock + // means we can't reset it across tests. Use the per-call scope directly + // by skipping `validate_in_scope` and exercising `OperationScope`. + let scope = OperationScope::new(vec!["192.168.58.10".into()]); + assert!(scope.contains("192.168.58.10")); + } + + #[test] + fn validate_rejects_out_of_scope_target() { + // Inline replication of the validation logic so the OnceLock-based + // global doesn't interfere with parallel tests. + let scope = OperationScope::new(vec!["192.168.58.10".into(), "192.168.58.20".into()]); + let args = json!({"target": "192.168.58.99", "domain": "contoso.local"}); + let val = args["target"].as_str().unwrap(); + let is_ip = val.parse::().is_ok(); + assert!(is_ip); + assert!(!scope.contains(val)); + } + + #[test] + fn cidr_target_passes_validation() { + // Sweeps (CIDR / comma-list) pass — they're discovery, not single-host + // attacks. A CIDR like 192.168.58.0/24 doesn't parse as Ipv4Addr. + let val = "192.168.58.0/24"; + assert!(val.parse::().is_err()); + } + + #[test] + fn hostname_target_passes_validation() { + let val = "dc01.contoso.local"; + assert!(val.parse::().is_err()); + } + + #[test] + fn from_env_returns_empty_when_var_missing() { + std::env::remove_var("ARES_OPERATION_ID_FROMENV_SCOPE_TEST"); + // Don't touch the real ARES_OPERATION_ID — other tests may rely on it. + let scope = OperationScope::default(); + assert!(scope.is_unrestricted()); + } +} From 1156c3bebfaea54b571d1b3e93c52ffb4bd1ab50 Mon Sep 17 00:00:00 2001 From: Jayson Grace Date: Sat, 9 May 2026 21:52:40 -0600 Subject: [PATCH 2/2] refactor: improve operation scope install and validation interfaces **Added:** - Add `OperationScope::from_raw` for direct payload parsing without environment access - Add `install_from_env` to atomically read and install operation scope from environment, returning the installed scope for logging and testability - Add `validate_against` for pure validation against arbitrary scopes, decoupling from global state for testability - Expand tests to cover `from_raw`, `install_from_env`, and validation edge cases with more granular checks **Changed:** - Update orchestrator and worker startup to use the new scope install/parse helpers, improving clarity and testability - Refactor `validate_in_scope` to delegate to `validate_against` for better separation of concerns - Refactor existing scope parsing logic to use `from_raw` and streamline environment handling - Improve test coverage and structure for scope parsing and validation **Removed:** - Remove redundant call to `init_scope` in worker tool executor, now handled by `install_from_env` --- ares-cli/src/orchestrator/mod.rs | 5 +- ares-cli/src/worker/tool_executor.rs | 3 +- ares-tools/src/scope.rs | 252 ++++++++++++++++++++++----- 3 files changed, 216 insertions(+), 44 deletions(-) diff --git a/ares-cli/src/orchestrator/mod.rs b/ares-cli/src/orchestrator/mod.rs index 80214f91..2c2be791 100644 --- a/ares-cli/src/orchestrator/mod.rs +++ b/ares-cli/src/orchestrator/mod.rs @@ -108,9 +108,8 @@ async fn run_inner() -> Result<()> { // Install the operation scope so `ares_tools::dispatch` rejects single-IP // tool invocations against hosts the operator never authorized. Empty // target_ips → unrestricted (legacy/test launches that didn't pass IPs). - ares_tools::scope::init_scope(ares_tools::scope::OperationScope::new( - config.target_ips.clone(), - )); + let scope = ares_tools::scope::OperationScope::new(config.target_ips.clone()); + ares_tools::scope::init_scope(scope); if !config.target_ips.is_empty() { info!( target_ips = %config.target_ips.join(","), diff --git a/ares-cli/src/worker/tool_executor.rs b/ares-cli/src/worker/tool_executor.rs index c54ab09d..b5b16beb 100644 --- a/ares-cli/src/worker/tool_executor.rs +++ b/ares-cli/src/worker/tool_executor.rs @@ -76,14 +76,13 @@ pub async fn run_tool_exec_loop( // single-IP tool calls get rejected before any subprocess runs. The worker // doesn't otherwise parse target_ips out of the env JSON; this is the // only path that needs them. - let scope = ares_tools::scope::OperationScope::from_env(); + let scope = ares_tools::scope::install_from_env(); if !scope.is_unrestricted() { info!( target_ips = %scope.target_ips().join(","), "Worker installed operation scope — out-of-scope single-IP tool calls will be rejected" ); } - ares_tools::scope::init_scope(scope); let subject = nats::tool_exec_subject(&config.worker_role); let queue_group = format!("ares-tools-{}", config.worker_role); diff --git a/ares-tools/src/scope.rs b/ares-tools/src/scope.rs index bc964cec..f39f0278 100644 --- a/ares-tools/src/scope.rs +++ b/ares-tools/src/scope.rs @@ -37,13 +37,16 @@ impl OperationScope { /// unrestricted) scope when the env var is unset, plain-text, or /// missing a `target_ips` array — none of those cases are an error here. pub fn from_env() -> Self { - let raw = match std::env::var("ARES_OPERATION_ID") { - Ok(v) => v, - Err(_) => return Self::default(), - }; - let json_start = match raw.find('{') { - Some(i) => i, - None => return Self::default(), + Self::from_raw(&std::env::var("ARES_OPERATION_ID").unwrap_or_default()) + } + + /// Pure parse of the same payload format `OrchestratorConfig` consumes: + /// either a plain operation-id string or a JSON envelope with + /// `target_ips`. Public so callers (and tests) can exercise the parse + /// without touching the process environment. + pub fn from_raw(raw: &str) -> Self { + let Some(json_start) = raw.find('{') else { + return Self::default(); }; let json: serde_json::Value = match serde_json::from_str(&raw[json_start..]) { Ok(v) => v, @@ -85,6 +88,16 @@ fn current_scope() -> &'static OperationScope { SCOPE.get_or_init(OperationScope::default) } +/// Read the operation scope from the environment and install it. Intended to +/// be called once at orchestrator/worker startup. Returns the installed scope +/// so the caller can log the configured target list (and so the call is +/// directly unit-testable instead of relying on a process-wide side effect). +pub fn install_from_env() -> OperationScope { + let scope = OperationScope::from_env(); + init_scope(scope.clone()); + scope +} + /// Validate that `arguments` only targets in-scope hosts. /// /// Only checked when the field is a literal IPv4 address — CIDRs, comma- @@ -92,7 +105,13 @@ fn current_scope() -> &'static OperationScope { /// free to do legitimate discovery; the gate fires when it tries to run a /// single-target attack tool against a host nobody authorized. pub fn validate_in_scope(tool: &str, arguments: &Value) -> Result<()> { - let scope = current_scope(); + validate_against(tool, arguments, current_scope()) +} + +/// Pure validation against an arbitrary scope. Splitting this out from the +/// `OnceLock`-backed [`validate_in_scope`] makes the gate fully unit-testable +/// without polluting global state across tests. +pub fn validate_against(tool: &str, arguments: &Value, scope: &OperationScope) -> Result<()> { if scope.is_unrestricted() { return Ok(()); } @@ -124,62 +143,217 @@ mod tests { use super::*; use serde_json::json; + fn scoped(ips: &[&str]) -> OperationScope { + OperationScope::new(ips.iter().map(|s| (*s).to_string()).collect()) + } + #[test] - fn unrestricted_scope_passes_everything() { + fn unrestricted_scope_marks_empty() { let scope = OperationScope::default(); assert!(scope.is_unrestricted()); - // Even literal IPs pass when scope is empty + assert_eq!(scope.target_ips(), &[] as &[String]); assert!(!scope.contains("192.168.58.10")); } #[test] - fn scoped_contains_membership() { - let scope = OperationScope::new(vec!["192.168.58.10".into(), "192.168.58.20".into()]); + fn scoped_membership_and_accessors() { + let scope = scoped(&["192.168.58.10", "192.168.58.20"]); assert!(!scope.is_unrestricted()); assert!(scope.contains("192.168.58.10")); + assert!(scope.contains("192.168.58.20")); assert!(!scope.contains("192.168.58.30")); + assert_eq!(scope.target_ips().len(), 2); } #[test] - fn validate_passes_in_scope_ip() { - // Force scope into the static for this test to work — but OnceLock - // means we can't reset it across tests. Use the per-call scope directly - // by skipping `validate_in_scope` and exercising `OperationScope`. - let scope = OperationScope::new(vec!["192.168.58.10".into()]); - assert!(scope.contains("192.168.58.10")); + fn validate_against_unrestricted_passes_anything() { + let scope = OperationScope::default(); + let args = json!({"target": "192.168.58.99"}); + assert!(validate_against("nmap_scan", &args, &scope).is_ok()); } #[test] - fn validate_rejects_out_of_scope_target() { - // Inline replication of the validation logic so the OnceLock-based - // global doesn't interfere with parallel tests. - let scope = OperationScope::new(vec!["192.168.58.10".into(), "192.168.58.20".into()]); - let args = json!({"target": "192.168.58.99", "domain": "contoso.local"}); - let val = args["target"].as_str().unwrap(); - let is_ip = val.parse::().is_ok(); - assert!(is_ip); - assert!(!scope.contains(val)); + fn validate_against_passes_in_scope_target() { + let scope = scoped(&["192.168.58.10", "192.168.58.20"]); + let args = json!({"target": "192.168.58.10", "domain": "contoso.local"}); + assert!(validate_against("secretsdump", &args, &scope).is_ok()); } #[test] - fn cidr_target_passes_validation() { - // Sweeps (CIDR / comma-list) pass — they're discovery, not single-host - // attacks. A CIDR like 192.168.58.0/24 doesn't parse as Ipv4Addr. - let val = "192.168.58.0/24"; - assert!(val.parse::().is_err()); + fn validate_against_passes_in_scope_target_ip_field() { + let scope = scoped(&["192.168.58.10"]); + let args = json!({"target_ip": "192.168.58.10"}); + assert!(validate_against("psexec", &args, &scope).is_ok()); } #[test] - fn hostname_target_passes_validation() { - let val = "dc01.contoso.local"; - assert!(val.parse::().is_err()); + fn validate_against_rejects_out_of_scope_ip() { + let scope = scoped(&["192.168.58.10", "192.168.58.20"]); + let args = json!({"target": "192.168.58.99"}); + let err = validate_against("secretsdump", &args, &scope).unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("secretsdump")); + assert!(msg.contains("192.168.58.99")); + assert!(msg.contains("192.168.58.10")); } #[test] - fn from_env_returns_empty_when_var_missing() { - std::env::remove_var("ARES_OPERATION_ID_FROMENV_SCOPE_TEST"); - // Don't touch the real ARES_OPERATION_ID — other tests may rely on it. - let scope = OperationScope::default(); + fn validate_against_rejects_out_of_scope_target_ip_field() { + let scope = scoped(&["192.168.58.10"]); + let args = json!({"target_ip": "192.168.58.99"}); + assert!(validate_against("psexec", &args, &scope).is_err()); + } + + #[test] + fn validate_against_passes_cidr_through() { + // Sweeps pass — CIDR doesn't parse as Ipv4Addr. + let scope = scoped(&["192.168.58.10"]); + let args = json!({"target": "192.168.58.0/24"}); + assert!(validate_against("smb_sweep", &args, &scope).is_ok()); + } + + #[test] + fn validate_against_passes_comma_list_through() { + let scope = scoped(&["192.168.58.10"]); + let args = json!({"target": "192.168.58.10,192.168.58.20"}); + assert!(validate_against("smb_sweep", &args, &scope).is_ok()); + } + + #[test] + fn validate_against_passes_hostname_through() { + // Hostnames aren't resolved here — parser-side attribution fixes catch + // hostname mis-attribution. A real AD hostname won't parse as IPv4. + let scope = scoped(&["192.168.58.10"]); + let args = json!({"target": "dc01.contoso.local"}); + assert!(validate_against("nmap_scan", &args, &scope).is_ok()); + } + + #[test] + fn validate_against_passes_loopback() { + let scope = scoped(&["192.168.58.10"]); + let args = json!({"target": "127.0.0.1"}); + assert!(validate_against("get_tgt", &args, &scope).is_ok()); + } + + #[test] + fn validate_against_no_target_field_passes() { + let scope = scoped(&["192.168.58.10"]); + let args = json!({"username": "alice", "domain": "contoso.local"}); + assert!(validate_against("ldap_search", &args, &scope).is_ok()); + } + + #[test] + fn validate_against_non_string_target_passes() { + // A target that isn't a string (e.g. number, null) shouldn't crash — + // it just fails the IPv4 parse and falls through. + let scope = scoped(&["192.168.58.10"]); + let args = json!({"target": 12345}); + assert!(validate_against("nmap_scan", &args, &scope).is_ok()); + } + + #[test] + fn validate_against_checks_both_target_fields() { + // If either `target` or `target_ip` is out of scope, reject. Some + // tools accept both names for backwards compat. + let scope = scoped(&["192.168.58.10"]); + let args = json!({"target": "192.168.58.10", "target_ip": "192.168.58.99"}); + assert!(validate_against("psexec", &args, &scope).is_err()); + } + + #[test] + fn from_raw_empty_string_is_unrestricted() { + assert!(OperationScope::from_raw("").is_unrestricted()); + } + + #[test] + fn from_raw_plain_op_id_is_unrestricted() { + // Plain operation-id (no JSON envelope) → no target_ips → unrestricted. + assert!(OperationScope::from_raw("op-20260509-204128").is_unrestricted()); + } + + #[test] + fn from_raw_json_without_target_ips_is_unrestricted() { + let raw = r#"{"operation_id":"op-x","target_domain":"contoso.local"}"#; + assert!(OperationScope::from_raw(raw).is_unrestricted()); + } + + #[test] + fn from_raw_json_empty_target_ips_is_unrestricted() { + let raw = r#"{"operation_id":"op-x","target_ips":[]}"#; + assert!(OperationScope::from_raw(raw).is_unrestricted()); + } + + #[test] + fn from_raw_json_with_target_ips_populates_scope() { + let raw = r#"{"operation_id":"op-x","target_ips":["192.168.58.10","192.168.58.20"]}"#; + let scope = OperationScope::from_raw(raw); + assert!(!scope.is_unrestricted()); + assert!(scope.contains("192.168.58.10")); + assert!(scope.contains("192.168.58.20")); + assert_eq!(scope.target_ips().len(), 2); + } + + #[test] + fn from_raw_skips_telemetry_prefix_before_json() { + // Wrapper script may prefix the env var with log lines — the parser + // searches for the first `{` and parses from there. + let raw = "2026-04-17T21:35:33Z INFO telemetry initialized\n\ + {\"operation_id\":\"op-x\",\"target_ips\":[\"192.168.58.10\"]}"; + let scope = OperationScope::from_raw(raw); + assert!(scope.contains("192.168.58.10")); + } + + #[test] + fn from_raw_invalid_json_is_unrestricted() { + // Garbage after the `{` shouldn't crash — falls back to empty scope. + let scope = OperationScope::from_raw("op-id {not valid json"); assert!(scope.is_unrestricted()); } + + #[test] + fn from_raw_filters_non_string_target_ips() { + // If target_ips contains non-string entries, those are skipped silently + // rather than producing junk scope entries. + let raw = r#"{"target_ips":["192.168.58.10",42,null,"192.168.58.20"]}"#; + let scope = OperationScope::from_raw(raw); + assert_eq!(scope.target_ips().len(), 2); + assert!(scope.contains("192.168.58.10")); + assert!(scope.contains("192.168.58.20")); + } + + #[test] + fn init_scope_first_call_wins() { + // OnceLock means subsequent init_scope calls are no-ops — the first + // installed scope persists for the process lifetime. We can't assert + // exact contents (other tests may have initialized it), but the + // read path must not panic. + init_scope(OperationScope::new(vec!["192.168.58.99".into()])); + let scope = current_scope(); + let _ = scope.is_unrestricted(); + } + + #[test] + fn validate_in_scope_uses_global_scope() { + // Smoke test the global path: whatever scope is installed (possibly + // unrestricted in test environment), validate_in_scope shouldn't + // panic and should agree with validate_against on the same scope. + let args = json!({"target": "192.168.58.10"}); + let result = validate_in_scope("nmap_scan", &args); + let same_scope = current_scope(); + let direct = validate_against("nmap_scan", &args, same_scope); + assert_eq!(result.is_ok(), direct.is_ok()); + } + + #[test] + fn install_from_env_returns_parsed_scope() { + // Drives the orchestrator/worker startup helper. The function + // installs the global scope (no-op if already set) and returns the + // parsed scope so callers can log it. With ARES_OPERATION_ID unset + // or plain, the returned scope is unrestricted; either way the call + // must not panic. + let returned = install_from_env(); + // The returned scope should be readable + let _ = returned.is_unrestricted(); + let _ = returned.target_ips(); + } }