Skip to content

Commit 383db3a

Browse files
authored
refactor: remove legacy Python compatibility comments and dead code (#331)
**Key Changes:** - Removed comments referencing legacy Python code and behavior throughout the codebase - Deleted unused and dead code paths, including functions, comments, and allow attributes - Cleaned up documentation to focus on current Rust implementation - Clarified and simplified docstrings, removing historical context **Changed:** - Updated docstrings across orchestrator, state, models, telemetry, and tool modules to focus on their Rust implementation and remove historical references to Python - Simplified documentation for configuration, state storage, telemetry, and prompt modules, focusing on current Rust usage and key behaviors - Switched test helpers and doc comments to describe current Rust behavior rather than referencing previous Python code or bugs - Investigation, state, and automation modules now reference only the current Rust approach, clarifying behaviors and removing historical context - Deduplication, credential, and trust logic docstrings updated to describe Rust-native strategies - Telemetry span helpers and prompt documentation now reference only current Rust usage **Removed:** - All legacy Python compatibility comments and docstrings - Dead code, legacy compatibility functions, and unused struct fields - The `certifried.rs` file and all related test cases and automation spawner references - Unused allow(dead_code), allow(unused_imports), and allow attributes across the codebase - All comments referencing Python modules, functions, or compatibility, including "matches Python", "mirrors Python", and "ported from Python" notes in docstrings - Dead code such as allow(dead_code), unused imports, and legacy compatibility functions - Unused functions and struct fields that were only present for legacy compatibility, including test-only or private helpers no longer relevant to the Rust codebase - The entire `certifried.rs` automation module (no longer used or referenced) - Legacy code paths for scalar output processing in result extraction and processing
1 parent e1ee8a6 commit 383db3a

175 files changed

Lines changed: 3792 additions & 2785 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

Cargo.toml

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,25 @@
22
resolver = "2"
33
members = ["ares-core", "ares-cli", "ares-llm", "ares-tools"]
44

5+
[workspace.lints.clippy]
6+
# Functions with many parameters must use a parameter struct (see the
7+
# `*Params` / `*Config` types throughout the workspace). Suppressing this
8+
# with `#[allow(...)]` defeats the whole point — fix the signature instead.
9+
too_many_arguments = "deny"
10+
# Prefer `let ... else { ... }` over `let x = match opt { Some(x) => x, None => ... };`
11+
# — same semantics, fewer lines, no rightward drift.
12+
manual_let_else = "deny"
13+
# `.iter().filter(..).collect::<Vec<_>>().len()` / `.is_empty()` / `.contains(..)` —
14+
# allocate-then-consume when the iterator already answers the question.
15+
needless_collect = "deny"
16+
# `.clone()` on a value whose last use immediately follows — the move would suffice.
17+
redundant_clone = "deny"
18+
# Types that derive `PartialEq` should derive `Eq` too when their fields permit.
19+
# When they don't (e.g. an `f64` or `serde_json::Value` field), suppress with
20+
# `#[expect(clippy::derive_partial_eq_without_eq, reason = "...")]` on the type
21+
# explaining which field blocks it.
22+
derive_partial_eq_without_eq = "deny"
23+
524
[workspace.dependencies]
625
serde = { version = "1", features = ["derive"] }
726
serde_json = "1"

ares-cli/Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,6 @@ tokio = { workspace = true }
4747
rstest = "0.26"
4848
tempfile = "3"
4949
ares-core = { path = "../ares-core", features = ["test-utils", "blue", "telemetry"] }
50+
51+
[lints]
52+
workspace = true

ares-cli/src/blue/mod.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,17 +91,17 @@ pub(crate) async fn run_blue(cmd: BlueCommands, redis_url: Option<String>) -> Re
9191
grafana_url,
9292
grafana_api_key,
9393
} => {
94-
submit::blue_submit(
94+
submit::blue_submit(submit::BlueSubmitParams {
9595
redis_url,
9696
alert_json,
9797
investigation_id,
9898
model,
9999
max_steps,
100100
multi_agent,
101-
!no_auto_route,
101+
auto_route: !no_auto_route,
102102
grafana_url,
103103
grafana_api_key,
104-
)
104+
})
105105
.await
106106
}
107107
BlueCommands::Watch {
@@ -129,15 +129,15 @@ pub(crate) async fn run_blue(cmd: BlueCommands, redis_url: Option<String>) -> Re
129129
grafana_url,
130130
grafana_api_key,
131131
} => {
132-
submit::blue_from_operation(
132+
submit::blue_from_operation(submit::BlueFromOperationParams {
133133
redis_url,
134134
operation_id,
135135
latest,
136136
model,
137137
max_steps,
138138
grafana_url,
139139
grafana_api_key,
140-
)
140+
})
141141
.await
142142
}
143143
}

ares-cli/src/blue/submit.rs

Lines changed: 46 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,31 @@ use ares_core::state::RedisStateReader;
1010
use crate::ops::submit::{collect_env_vars, resolve_model, BLUE_ENV_VAR_NAMES};
1111
use crate::redis_conn::{connect_redis, resolve_operation_id};
1212

13-
#[allow(clippy::too_many_arguments)]
14-
pub(crate) async fn blue_submit(
15-
redis_url: Option<String>,
16-
alert_json: String,
17-
investigation_id: Option<String>,
18-
model: Option<String>,
19-
max_steps: u32,
20-
multi_agent: bool,
21-
auto_route: bool,
22-
grafana_url: Option<String>,
23-
grafana_api_key: Option<String>,
24-
) -> Result<()> {
13+
pub(crate) struct BlueSubmitParams {
14+
pub redis_url: Option<String>,
15+
pub alert_json: String,
16+
pub investigation_id: Option<String>,
17+
pub model: Option<String>,
18+
pub max_steps: u32,
19+
pub multi_agent: bool,
20+
pub auto_route: bool,
21+
pub grafana_url: Option<String>,
22+
pub grafana_api_key: Option<String>,
23+
}
24+
25+
pub(crate) async fn blue_submit(p: BlueSubmitParams) -> Result<()> {
26+
let BlueSubmitParams {
27+
redis_url,
28+
alert_json,
29+
investigation_id,
30+
model,
31+
max_steps,
32+
multi_agent,
33+
auto_route,
34+
grafana_url,
35+
grafana_api_key,
36+
} = p;
37+
2538
let alert: serde_json::Value = if std::path::Path::new(&alert_json).is_file() {
2639
let content = std::fs::read_to_string(&alert_json)
2740
.with_context(|| format!("Failed to read alert file: {alert_json}"))?;
@@ -49,7 +62,6 @@ pub(crate) async fn blue_submit(
4962

5063
let now = Utc::now();
5164

52-
// Format must match Python blue_orchestrator_client.py
5365
let request = serde_json::json!({
5466
"investigation_id": inv_id,
5567
"alert": alert,
@@ -90,16 +102,27 @@ pub(crate) async fn blue_submit(
90102
Ok(())
91103
}
92104

93-
#[allow(clippy::too_many_arguments)]
94-
pub(crate) async fn blue_from_operation(
95-
redis_url: Option<String>,
96-
operation_id: Option<String>,
97-
latest: bool,
98-
model: Option<String>,
99-
max_steps: u32,
100-
grafana_url: Option<String>,
101-
grafana_api_key: Option<String>,
102-
) -> Result<()> {
105+
pub(crate) struct BlueFromOperationParams {
106+
pub redis_url: Option<String>,
107+
pub operation_id: Option<String>,
108+
pub latest: bool,
109+
pub model: Option<String>,
110+
pub max_steps: u32,
111+
pub grafana_url: Option<String>,
112+
pub grafana_api_key: Option<String>,
113+
}
114+
115+
pub(crate) async fn blue_from_operation(p: BlueFromOperationParams) -> Result<()> {
116+
let BlueFromOperationParams {
117+
redis_url,
118+
operation_id,
119+
latest,
120+
model,
121+
max_steps,
122+
grafana_url,
123+
grafana_api_key,
124+
} = p;
125+
103126
let mut conn = connect_redis(redis_url.clone()).await?;
104127
let op_id = resolve_operation_id(&mut conn, operation_id, latest).await?;
105128

ares-cli/src/blue/watch.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,15 @@ pub(crate) async fn blue_watch(
1717
let now = chrono::Utc::now().format("%Y-%m-%d %H:%M:%S");
1818
println!("[{now}] Submitting blue investigation from latest operation...");
1919

20-
match super::submit::blue_from_operation(
21-
redis_url.clone(),
22-
None,
23-
true, // --latest
24-
model.clone(),
20+
match super::submit::blue_from_operation(super::submit::BlueFromOperationParams {
21+
redis_url: redis_url.clone(),
22+
operation_id: None,
23+
latest: true,
24+
model: model.clone(),
2525
max_steps,
26-
grafana_url.clone(),
27-
grafana_api_key.clone(),
28-
)
26+
grafana_url: grafana_url.clone(),
27+
grafana_api_key: grafana_api_key.clone(),
28+
})
2929
.await
3030
{
3131
Ok(()) => info!("Investigation submitted successfully"),

ares-cli/src/cli/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ pub(crate) enum Commands {
8686
#[arg(hide = true)]
8787
_role: Option<String>,
8888

89-
/// Accept and ignore legacy Python-style --worker-args.* flags
89+
/// Accept and ignore legacy `--worker-args.*` flags
9090
#[arg(long = "worker-args.redis-url", hide = true)]
9191
_legacy_redis_url: Option<String>,
9292
},

ares-cli/src/config.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ fn config_set_model(
219219

220220
if all {
221221
// Replace model for all agents
222-
let mut new_contents = contents.clone();
222+
let mut new_contents = contents;
223223
for (role_name, agent) in &cfg.agents {
224224
new_contents = replace_model_in_yaml(&new_contents, role_name, &agent.model, &model);
225225
}

ares-cli/src/detection/playbook.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -226,12 +226,12 @@ mod tests {
226226
.collect();
227227
assert_eq!(ip_targets.len(), 1);
228228
assert_eq!(ip_targets[0].value, "192.168.58.10");
229-
let hostname_targets: Vec<_> = playbook
229+
let hostname_count = playbook
230230
.detection_targets
231231
.iter()
232232
.filter(|t| t.ioc_type == "hostname")
233-
.collect();
234-
assert_eq!(hostname_targets.len(), 1);
233+
.count();
234+
assert_eq!(hostname_count, 1);
235235
}
236236

237237
#[test]

ares-cli/src/detection/techniques/tests.rs

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,28 @@ use ares_core::models::{Credential, Host, Share, SharedRedTeamState};
1919
fn get_technique_name_known() {
2020
assert_eq!(get_technique_name("T1046"), "Network Service Discovery");
2121
assert_eq!(get_technique_name("T1003"), "OS Credential Dumping");
22+
assert_eq!(get_technique_name("T1003.001"), "LSASS Memory");
2223
assert_eq!(get_technique_name("T1003.006"), "DCSync");
24+
assert_eq!(get_technique_name("T1078"), "Valid Accounts");
25+
assert_eq!(get_technique_name("T1078.002"), "Domain Accounts");
26+
assert_eq!(get_technique_name("T1110"), "Brute Force");
27+
assert_eq!(
28+
get_technique_name("T1558"),
29+
"Steal or Forge Kerberos Tickets"
30+
);
31+
assert_eq!(get_technique_name("T1558.001"), "Golden Ticket");
2332
assert_eq!(get_technique_name("T1558.003"), "Kerberoasting");
2433
assert_eq!(get_technique_name("T1558.004"), "AS-REP Roasting");
34+
assert_eq!(get_technique_name("T1021"), "Remote Services");
2535
assert_eq!(get_technique_name("T1021.002"), "SMB/Windows Admin Shares");
2636
assert_eq!(get_technique_name("T1649"), "ADCS Certificate Theft");
37+
assert_eq!(
38+
get_technique_name("T1550"),
39+
"Use Alternate Authentication Material"
40+
);
2741
assert_eq!(get_technique_name("T1550.002"), "Pass the Hash");
42+
assert_eq!(get_technique_name("T1484"), "Domain Policy Modification");
43+
assert_eq!(get_technique_name("T1087"), "Account Discovery");
2844
}
2945

3046
#[test]
@@ -677,3 +693,73 @@ fn detection_query_time_window_is_set() {
677693
assert!(tw.start.as_ref().unwrap().contains('T'));
678694
assert!(tw.end.as_ref().unwrap().contains('T'));
679695
}
696+
697+
#[test]
698+
fn build_technique_detections_sub_technique_parent_t1046() {
699+
// T1046.999 → parent T1046 → build_t1046
700+
let state = SharedRedTeamState::new("test-op".to_string());
701+
let start = Utc::now() - chrono::Duration::hours(1);
702+
let end = Utc::now();
703+
let detections = build_technique_detections(&state, &["T1046.999".to_string()], &start, &end);
704+
let det = &detections["T1046.999"];
705+
assert!(!det.detection_queries.is_empty());
706+
}
707+
708+
#[test]
709+
fn build_technique_detections_sub_technique_parent_t1078() {
710+
// T1078.999 → parent T1078 → build_t1078
711+
let state = SharedRedTeamState::new("test-op".to_string());
712+
let start = Utc::now() - chrono::Duration::hours(1);
713+
let end = Utc::now();
714+
let detections = build_technique_detections(&state, &["T1078.999".to_string()], &start, &end);
715+
let det = &detections["T1078.999"];
716+
assert!(!det.detection_queries.is_empty());
717+
}
718+
719+
#[test]
720+
fn build_technique_detections_sub_technique_parent_t1558() {
721+
// T1558.999 → parent T1558 → build_t1558
722+
let state = SharedRedTeamState::new("test-op".to_string());
723+
let start = Utc::now() - chrono::Duration::hours(1);
724+
let end = Utc::now();
725+
let detections = build_technique_detections(&state, &["T1558.999".to_string()], &start, &end);
726+
let det = &detections["T1558.999"];
727+
assert!(!det.detection_queries.is_empty());
728+
}
729+
730+
#[test]
731+
fn build_technique_detections_sub_technique_parent_t1021() {
732+
// T1021.999 → parent T1021 → build_t1021
733+
let state = SharedRedTeamState::new("test-op".to_string());
734+
let start = Utc::now() - chrono::Duration::hours(1);
735+
let end = Utc::now();
736+
let detections = build_technique_detections(&state, &["T1021.999".to_string()], &start, &end);
737+
let det = &detections["T1021.999"];
738+
assert!(!det.detection_queries.is_empty());
739+
}
740+
741+
#[test]
742+
fn build_technique_detections_sub_technique_parent_t1550() {
743+
// T1550.999 → parent T1550 → build_t1550
744+
let state = SharedRedTeamState::new("test-op".to_string());
745+
let start = Utc::now() - chrono::Duration::hours(1);
746+
let end = Utc::now();
747+
let detections = build_technique_detections(&state, &["T1550.999".to_string()], &start, &end);
748+
let det = &detections["T1550.999"];
749+
assert!(!det.detection_queries.is_empty());
750+
}
751+
752+
#[test]
753+
fn build_technique_detections_unknown_with_known_name() {
754+
// A technique that has a known name (not empty) in get_technique_name
755+
// The T9999 was used in unknown_technique_fallback; use a known one that still
756+
// hits the fallback branch (T1484 is in the names table but not the match arms)
757+
let state = SharedRedTeamState::new("test-op".to_string());
758+
let start = Utc::now() - chrono::Duration::hours(1);
759+
let end = Utc::now();
760+
let detections = build_technique_detections(&state, &["T1484".to_string()], &start, &end);
761+
let det = &detections["T1484"];
762+
// T1484 hits the final fallback; name comes from get_technique_name
763+
assert_eq!(det.technique_id, "T1484");
764+
assert_eq!(det.technique_name, "Domain Policy Modification");
765+
}

0 commit comments

Comments
 (0)