Skip to content

Commit ba0652f

Browse files
timopollmeiergreenbonebot
timopollmeier
authored and
greenbonebot
committed
Add phase stop status change timeout
Co-authored-by: AI (copilot/full)
1 parent 18ca6d5 commit ba0652f

5 files changed

Lines changed: 199 additions & 0 deletions

File tree

‎src/config/settings.rs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ pub const DEFAULT_SCAN_STOP_GRACE_PERIOD_SECONDS: u64 = 300;
4242
/// Default grace period added to scan-level AJAX spider timeout before forcing a stop request.
4343
pub const DEFAULT_SCAN_AJAX_SPIDER_TIMEOUT_GRACE_PERIOD_SECONDS: u64 = 60;
4444

45+
/// Default time limit for waiting on phase status changes after stop requests.
46+
pub const DEFAULT_SCAN_PHASE_STOP_STATUS_CHANGE_TIMEOUT_SECONDS: u64 = 60;
47+
4548
/// Default maximum number of retry attempts for transient failures.
4649
pub const DEFAULT_SCAN_RETRY_MAX_RETRIES: u32 = 10;
4750

@@ -82,6 +85,8 @@ pub struct Settings {
8285
pub scan_stop_grace_period_seconds: u64,
8386
/// Grace period in seconds added to scan-level AJAX spider timeout before issuing a stop.
8487
pub scan_ajax_spider_timeout_grace_period_seconds: u64,
88+
/// Time limit in seconds for waiting on scan phase status changes after stop requests.
89+
pub scan_phase_stop_status_change_timeout_seconds: u64,
8590
/// Maximum number of retry attempts for transient ZAP or storage failures.
8691
pub scan_retry_max_retries: u32,
8792
/// Maximum backoff delay between retry attempts, in seconds.
@@ -102,6 +107,7 @@ struct RawSettings {
102107
scan_alert_poll_interval_seconds: u64,
103108
scan_stop_grace_period_seconds: u64,
104109
scan_ajax_spider_timeout_grace_period_seconds: u64,
110+
scan_phase_stop_status_change_timeout_seconds: u64,
105111
scan_retry_max_retries: u32,
106112
scan_retry_max_delay_seconds: u64,
107113
}
@@ -145,6 +151,10 @@ impl Settings {
145151
"scan_ajax_spider_timeout_grace_period_seconds",
146152
DEFAULT_SCAN_AJAX_SPIDER_TIMEOUT_GRACE_PERIOD_SECONDS,
147153
)?
154+
.set_default(
155+
"scan_phase_stop_status_change_timeout_seconds",
156+
DEFAULT_SCAN_PHASE_STOP_STATUS_CHANGE_TIMEOUT_SECONDS,
157+
)?
148158
.set_default(
149159
"scan_retry_max_retries",
150160
DEFAULT_SCAN_RETRY_MAX_RETRIES as i64,
@@ -181,6 +191,12 @@ impl Settings {
181191
));
182192
}
183193

194+
if raw.scan_phase_stop_status_change_timeout_seconds == 0 {
195+
return Err(ConfigError::Message(
196+
"scan_phase_stop_status_change_timeout_seconds must be greater than 0".to_string(),
197+
));
198+
}
199+
184200
if raw.scan_retry_max_delay_seconds == 0 {
185201
return Err(ConfigError::Message(
186202
"scan_retry_max_delay_seconds must be greater than 0".to_string(),
@@ -227,6 +243,8 @@ impl Settings {
227243
scan_stop_grace_period_seconds: raw.scan_stop_grace_period_seconds,
228244
scan_ajax_spider_timeout_grace_period_seconds: raw
229245
.scan_ajax_spider_timeout_grace_period_seconds,
246+
scan_phase_stop_status_change_timeout_seconds: raw
247+
.scan_phase_stop_status_change_timeout_seconds,
230248
scan_retry_max_retries: raw.scan_retry_max_retries,
231249
scan_retry_max_delay_seconds: raw.scan_retry_max_delay_seconds,
232250
})

‎src/config/settings_tests.rs‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ fn clear_env() {
2222
env::remove_var("GREENBONE_WAS_SCAN_ALERT_POLL_INTERVAL_SECONDS");
2323
env::remove_var("GREENBONE_WAS_SCAN_STOP_GRACE_PERIOD_SECONDS");
2424
env::remove_var("GREENBONE_WAS_SCAN_AJAX_SPIDER_TIMEOUT_GRACE_PERIOD_SECONDS");
25+
env::remove_var("GREENBONE_WAS_SCAN_PHASE_STOP_STATUS_CHANGE_TIMEOUT_SECONDS");
2526
env::remove_var("GREENBONE_WAS_SCAN_RETRY_MAX_RETRIES");
2627
env::remove_var("GREENBONE_WAS_SCAN_RETRY_MAX_DELAY_SECONDS");
2728
}
@@ -61,6 +62,10 @@ fn test_uses_defaults_when_env_is_unset() {
6162
settings.scan_ajax_spider_timeout_grace_period_seconds,
6263
settings::DEFAULT_SCAN_AJAX_SPIDER_TIMEOUT_GRACE_PERIOD_SECONDS
6364
);
65+
assert_eq!(
66+
settings.scan_phase_stop_status_change_timeout_seconds,
67+
settings::DEFAULT_SCAN_PHASE_STOP_STATUS_CHANGE_TIMEOUT_SECONDS
68+
);
6469
assert_eq!(
6570
settings.scan_retry_max_retries,
6671
settings::DEFAULT_SCAN_RETRY_MAX_RETRIES
@@ -91,6 +96,10 @@ fn test_uses_env_overrides_when_set() {
9196
"GREENBONE_WAS_SCAN_AJAX_SPIDER_TIMEOUT_GRACE_PERIOD_SECONDS",
9297
"45",
9398
);
99+
env::set_var(
100+
"GREENBONE_WAS_SCAN_PHASE_STOP_STATUS_CHANGE_TIMEOUT_SECONDS",
101+
"33",
102+
);
94103
env::set_var("GREENBONE_WAS_SCAN_RETRY_MAX_RETRIES", "7");
95104
env::set_var("GREENBONE_WAS_SCAN_RETRY_MAX_DELAY_SECONDS", "45");
96105
};
@@ -109,6 +118,7 @@ fn test_uses_env_overrides_when_set() {
109118
assert_eq!(settings.scan_alert_poll_interval_seconds, 15);
110119
assert_eq!(settings.scan_stop_grace_period_seconds, 120);
111120
assert_eq!(settings.scan_ajax_spider_timeout_grace_period_seconds, 45);
121+
assert_eq!(settings.scan_phase_stop_status_change_timeout_seconds, 33);
112122
assert_eq!(settings.scan_retry_max_retries, 7);
113123
assert_eq!(settings.scan_retry_max_delay_seconds, 45);
114124
}
@@ -301,3 +311,23 @@ fn test_zero_scan_stop_grace_period_is_error() {
301311
let err = result.err().unwrap();
302312
assert!(err.to_string().contains("scan_stop_grace_period_seconds"));
303313
}
314+
315+
#[test]
316+
#[serial]
317+
fn test_zero_scan_phase_stop_status_change_timeout_is_error() {
318+
clear_env();
319+
unsafe {
320+
env::set_var(
321+
"GREENBONE_WAS_SCAN_PHASE_STOP_STATUS_CHANGE_TIMEOUT_SECONDS",
322+
"0",
323+
);
324+
}
325+
326+
let result = Settings::load();
327+
assert!(result.is_err());
328+
let err = result.err().unwrap();
329+
assert!(
330+
err.to_string()
331+
.contains("scan_phase_stop_status_change_timeout_seconds")
332+
);
333+
}

‎src/lib.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,9 @@ pub async fn run() -> Result<(), AppError> {
6969
ajax_spider_timeout_grace_period: std::time::Duration::from_secs(
7070
settings.scan_ajax_spider_timeout_grace_period_seconds,
7171
),
72+
phase_stop_status_change_timeout: std::time::Duration::from_secs(
73+
settings.scan_phase_stop_status_change_timeout_seconds,
74+
),
7275
retry_max_retries: settings.scan_retry_max_retries,
7376
retry_max_delay: std::time::Duration::from_secs(settings.scan_retry_max_delay_seconds),
7477
..ScanRuntimeConfig::default()

‎src/scan/worker.rs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ const DEFAULT_ALERT_PAGE_SIZE: u32 = 100;
3535
const DEFAULT_PASSIVE_SCAN_PLACEHOLDER_DURATION: Duration = Duration::from_secs(5);
3636
const DEFAULT_AJAX_SPIDER_TIMEOUT_SECONDS: u64 = 60 * 60;
3737
const DEFAULT_AJAX_SPIDER_TIMEOUT_GRACE_PERIOD: Duration = Duration::from_secs(60);
38+
const DEFAULT_PHASE_STOP_STATUS_CHANGE_TIMEOUT: Duration = Duration::from_secs(60);
3839

3940
#[derive(Debug, Clone)]
4041
pub struct ScanRuntimeConfig {
@@ -44,6 +45,7 @@ pub struct ScanRuntimeConfig {
4445
pub alert_page_size: u32,
4546
pub passive_scan_placeholder_duration: Duration,
4647
pub ajax_spider_timeout_grace_period: Duration,
48+
pub phase_stop_status_change_timeout: Duration,
4749
pub stop_grace_period: Duration,
4850
/// Maximum number of retry attempts for transient failures before a scan transitions to `failed`.
4951
pub retry_max_retries: u32,
@@ -60,6 +62,7 @@ impl Default for ScanRuntimeConfig {
6062
alert_page_size: DEFAULT_ALERT_PAGE_SIZE,
6163
passive_scan_placeholder_duration: DEFAULT_PASSIVE_SCAN_PLACEHOLDER_DURATION,
6264
ajax_spider_timeout_grace_period: DEFAULT_AJAX_SPIDER_TIMEOUT_GRACE_PERIOD,
65+
phase_stop_status_change_timeout: DEFAULT_PHASE_STOP_STATUS_CHANGE_TIMEOUT,
6366
stop_grace_period: Duration::from_secs(300),
6467
retry_max_retries: 10,
6568
retry_max_delay: Duration::from_secs(60),
@@ -382,6 +385,7 @@ impl ScanWorker {
382385
)
383386
};
384387
let mut timeout_stop_sent = false;
388+
let mut stop_status_change_deadline: Option<Instant> = None;
385389

386390
self.zap_client
387391
.set_ajax_spider_max_duration(ajax_spider_timeout_seconds)
@@ -401,6 +405,19 @@ impl ScanWorker {
401405
let status = self.zap_client.get_ajax_spider_status().await?;
402406
match status {
403407
AjaxSpiderStatus::Running => {
408+
if stop_status_change_deadline
409+
.is_some_and(|deadline| Instant::now() >= deadline)
410+
{
411+
warn!(
412+
scan_id,
413+
target,
414+
timeout_seconds =
415+
self.config.phase_stop_status_change_timeout.as_secs(),
416+
"ajax spider did not report status change after stop request within deadline; continuing to next phase"
417+
);
418+
break;
419+
}
420+
404421
if !timeout_stop_sent
405422
&& spider_stop_deadline.is_some_and(|deadline| Instant::now() >= deadline)
406423
{
@@ -414,6 +431,8 @@ impl ScanWorker {
414431
);
415432
self.zap_client.stop_ajax_spider_scan().await?;
416433
timeout_stop_sent = true;
434+
stop_status_change_deadline =
435+
Some(Instant::now() + self.config.phase_stop_status_change_timeout);
417436
}
418437
sleep(self.config.scan_poll_interval).await
419438
}

‎src/scan/worker_tests.rs‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -599,6 +599,98 @@ async fn mock_zap_server_for_spider_timeout_grace_stop_request() -> MockServer {
599599
server
600600
}
601601

602+
async fn mock_zap_server_for_spider_stop_status_change_timeout() -> MockServer {
603+
let server = MockServer::start().await;
604+
605+
Mock::given(method("POST"))
606+
.and(path("/JSON/context/action/newContext"))
607+
.respond_with(
608+
ResponseTemplate::new(200).set_body_raw(r#"{"contextId":"ctx-1"}"#, "application/json"),
609+
)
610+
.mount(&server)
611+
.await;
612+
613+
Mock::given(method("POST"))
614+
.and(path("/JSON/context/action/includeInContext"))
615+
.respond_with(
616+
ResponseTemplate::new(200).set_body_raw(r#"{"Result":"OK"}"#, "application/json"),
617+
)
618+
.mount(&server)
619+
.await;
620+
621+
Mock::given(method("POST"))
622+
.and(path("/JSON/ajaxSpider/action/setOptionMaxDuration"))
623+
.and(body_string_contains("Integer=1"))
624+
.respond_with(
625+
ResponseTemplate::new(200).set_body_raw(r#"{"Result":"OK"}"#, "application/json"),
626+
)
627+
.expect(1)
628+
.mount(&server)
629+
.await;
630+
631+
Mock::given(method("POST"))
632+
.and(path("/JSON/ajaxSpider/action/scan"))
633+
.respond_with(
634+
ResponseTemplate::new(200).set_body_raw(r#"{"Result":"OK"}"#, "application/json"),
635+
)
636+
.expect(1)
637+
.mount(&server)
638+
.await;
639+
640+
Mock::given(method("POST"))
641+
.and(path("/JSON/ajaxSpider/view/status"))
642+
.respond_with(
643+
ResponseTemplate::new(200).set_body_raw(r#"{"status":"running"}"#, "application/json"),
644+
)
645+
.mount(&server)
646+
.await;
647+
648+
Mock::given(method("POST"))
649+
.and(path("/JSON/ajaxSpider/action/stop"))
650+
.respond_with(
651+
ResponseTemplate::new(200).set_body_raw(r#"{"Result":"OK"}"#, "application/json"),
652+
)
653+
.expect(1)
654+
.mount(&server)
655+
.await;
656+
657+
Mock::given(method("POST"))
658+
.and(path("/JSON/ascan/action/scan"))
659+
.respond_with(
660+
ResponseTemplate::new(200).set_body_raw(r#"{"scan":"active-1"}"#, "application/json"),
661+
)
662+
.expect(0)
663+
.mount(&server)
664+
.await;
665+
666+
Mock::given(method("POST"))
667+
.and(path("/JSON/ascan/view/status"))
668+
.respond_with(
669+
ResponseTemplate::new(200).set_body_raw(r#"{"status":"100"}"#, "application/json"),
670+
)
671+
.expect(0)
672+
.mount(&server)
673+
.await;
674+
675+
Mock::given(method("POST"))
676+
.and(path("/JSON/alert/view/alerts"))
677+
.respond_with(
678+
ResponseTemplate::new(200).set_body_raw(r#"{"alerts":[]}"#, "application/json"),
679+
)
680+
.mount(&server)
681+
.await;
682+
683+
Mock::given(method("POST"))
684+
.and(path("/JSON/context/action/removeContext"))
685+
.respond_with(
686+
ResponseTemplate::new(200).set_body_raw(r#"{"Result":"OK"}"#, "application/json"),
687+
)
688+
.mount(&server)
689+
.await;
690+
691+
server
692+
}
693+
602694
async fn mock_zap_server_with_active_status_error() -> MockServer {
603695
let server = MockServer::start().await;
604696

@@ -1339,6 +1431,43 @@ async fn runtime_stops_ajax_spider_when_timeout_plus_grace_period_is_exceeded()
13391431
assert_eq!(scan.status, ScanStatus::Stopped);
13401432
}
13411433

1434+
#[tokio::test]
1435+
async fn runtime_continues_when_ajax_spider_status_does_not_change_after_local_stop_request() {
1436+
let (storage, _temp_dir) = temporary_sqlite_storage().await.unwrap();
1437+
let server = mock_zap_server_for_spider_stop_status_change_timeout().await;
1438+
let zap_client = ZapClient::new(server.uri(), "test-api-key".to_string()).unwrap();
1439+
let runtime = start_scan_runtime(
1440+
storage.clone(),
1441+
zap_client,
1442+
ScanRuntimeConfig {
1443+
worker_count: 1,
1444+
alert_poll_interval: Duration::from_millis(1),
1445+
scan_poll_interval: Duration::from_millis(20),
1446+
alert_page_size: 100,
1447+
passive_scan_placeholder_duration: Duration::from_millis(1),
1448+
ajax_spider_timeout_grace_period: Duration::from_millis(50),
1449+
phase_stop_status_change_timeout: Duration::from_millis(80),
1450+
stop_grace_period: Duration::from_secs(5),
1451+
..ScanRuntimeConfig::default()
1452+
},
1453+
);
1454+
let service = DefaultScanService::new(storage.clone(), runtime);
1455+
1456+
let scan_id = service
1457+
.create_scan(make_safe_mode_request_with_ajax_timeout(
1458+
"https://example.test",
1459+
1,
1460+
))
1461+
.await
1462+
.unwrap();
1463+
1464+
service.start_scan(&scan_id).await.unwrap();
1465+
wait_for_status(storage.as_ref(), &scan_id, ScanStatus::Succeeded).await;
1466+
1467+
let scan = storage.get_scan(&scan_id).await.unwrap();
1468+
assert_eq!(scan.status, ScanStatus::Succeeded);
1469+
}
1470+
13421471
#[tokio::test]
13431472
async fn runtime_transitions_running_scan_to_failed_on_worker_error() {
13441473
let (storage, _temp_dir) = temporary_sqlite_storage().await.unwrap();

0 commit comments

Comments
 (0)