From 35a1d8d19255c9f78a251ac63ce5ea27e0c43584 Mon Sep 17 00:00:00 2001 From: Nic Boet Date: Sun, 25 Oct 2026 19:47:28 -0500 Subject: [PATCH 1/2] fix(monitor): forced alarm cause and text as API v2 records them Filter the cause and text of POST /monitors/{id}/alarm to letters, digits, spaces and plain punctuation, as ZoneMinder's API v2 does (MonitorsController::alarmCause): both end up in Events.Cause and Events.Notes. An empty or unusable cause is recorded as "API", as a missing one is. Write score, cause and texts to shared memory before the trigger state, as Monitor::ForceAlarmOn does. fixes #254 Co-Authored-By: Claude Opus 5.5 --- src/service/monitor.rs | 94 ++++++++++++++++++++++++++++++++++++++++-- src/zm_shm.rs | 10 ++++- 2 files changed, 98 insertions(+), 6 deletions(-) diff --git a/src/service/monitor.rs b/src/service/monitor.rs index 0aba0187..7a36e950 100644 --- a/src/service/monitor.rs +++ b/src/service/monitor.rs @@ -10,6 +10,8 @@ use crate::repo; use crate::server::state::AppState; use crate::service::monitor_acl::MonitorScope; use crate::util::authz::Level; +use once_cell::sync::Lazy; +use regex::Regex; use rust_decimal::prelude::*; use sea_orm::ActiveValue::{NotSet, Set}; use sea_orm::{ActiveModelTrait, ColumnTrait, EntityTrait, QueryFilter, TransactionTrait}; @@ -973,6 +975,43 @@ pub async fn audio_level( }) } +/// What an alarm's cause and text may not contain: anything but letters, +/// combining marks, digits, spaces and plain punctuation. Both are stored on +/// the event (`Events.Cause`, `Events.Notes`), so markup, quotes, backslashes +/// and control characters are dropped. The same class as ZoneMinder's API v2 +/// (`MonitorsController::alarmCause`); marks keep decomposed accents. +static ALARM_TEXT_DROP: Lazy = + Lazy::new(|| Regex::new(r"[^\p{L}\p{M}\p{N} _.,:;!?()\[\]/+#@%=-]+").unwrap()); + +fn plain_alarm_text(value: &str) -> String { + let kept = ALARM_TEXT_DROP.replace_all(value, ""); + kept.split(' ') + .filter(|w| !w.is_empty()) + .collect::>() + .join(" ") +} + +/// The cause to record for a forced alarm: the request's, filtered, or "API" +/// when none is given or nothing usable is left. +fn alarm_cause(cause: Option<&str>) -> String { + let cause = plain_alarm_text(cause.unwrap_or("")); + if cause.is_empty() { + "API".to_string() + } else { + cause + } +} + +/// The note text for a forced alarm, filtered as the cause is. +fn alarm_text(text: Option<&str>) -> String { + let text = plain_alarm_text(text.unwrap_or("")); + if text.is_empty() { + "Triggered via REST API".to_string() + } else { + text + } +} + pub async fn control_alarm( state: &AppState, id: u32, @@ -999,13 +1038,13 @@ pub async fn control_alarm( "on" => { // Trigger alarm via shared memory let score = req.score.unwrap_or(100); - let cause = req.cause.as_deref().unwrap_or("API"); - let text = req.text.as_deref().unwrap_or("Triggered via REST API"); + let cause = alarm_cause(req.cause.as_deref()); + let text = alarm_text(req.text.as_deref()); // The shared-memory mmap/poke is blocking; run it off the async // executor so it can't stall the runtime. REVIEW_FIXES_PLAN §4.3. - let cause_owned = cause.to_string(); - let text_owned = text.to_string(); + let cause_owned = cause.clone(); + let text_owned = text; tokio::task::spawn_blocking(move || { zm_shm::trigger_alarm(id, score, &cause_owned, &text_owned) }) @@ -1180,3 +1219,50 @@ mod linked_monitor_tests { } } } + +#[cfg(test)] +mod alarm_text_tests { + use super::{alarm_cause, alarm_text}; + + #[test] + fn plain_causes_are_kept() { + assert_eq!(alarm_cause(Some("HA-test")), "HA-test"); + assert_eq!( + alarm_cause(Some("Doorbell (front): pressed!")), + "Doorbell (front): pressed!" + ); + assert_eq!(alarm_cause(Some("Türklingel")), "Türklingel"); + // A decomposed accent (e + U+0301) keeps its combining mark + assert_eq!(alarm_cause(Some("Cafe\u{301}")), "Cafe\u{301}"); + // "0" is a cause like any other, not a missing one + assert_eq!(alarm_cause(Some("0")), "0"); + } + + #[test] + fn markup_quotes_and_control_characters_are_dropped() { + assert_eq!( + alarm_cause(Some("")), + "svg onload=alert(1)" + ); + assert_eq!(alarm_cause(Some("a'b\"c\\d&e`f$g|h")), "abcdefgh"); + assert_eq!(alarm_cause(Some("l1\nl2\tx\0")), "l1l2x"); + assert_eq!(alarm_cause(Some(" a b ")), "a b"); + } + + #[test] + fn missing_empty_or_unusable_cause_is_the_default() { + assert_eq!(alarm_cause(None), "API"); + assert_eq!(alarm_cause(Some("")), "API"); + assert_eq!(alarm_cause(Some("<>\"'&")), "API"); + } + + #[test] + fn text_is_filtered_the_same_way() { + assert_eq!(alarm_text(None), "Triggered via REST API"); + assert_eq!( + alarm_text(Some("Front door opened")), + "bFront door/b opened" + ); + assert_eq!(alarm_text(Some("\u{7}")), "Triggered via REST API"); + } +} diff --git a/src/zm_shm.rs b/src/zm_shm.rs index b52a3010..6d889097 100644 --- a/src/zm_shm.rs +++ b/src/zm_shm.rs @@ -623,8 +623,9 @@ impl MonitorShm { let td = self.trigger_data_mut(); - // Set trigger state to ON - td.trigger_state = TriggerState::On as u32; + // Score, cause and texts before the state: the analysis thread reads + // them once it sees TRIGGER_ON (ZoneMinder's Monitor::ForceAlarmOn + // writes in this order for the same reason). td.trigger_score = score; // Copy cause string @@ -639,6 +640,11 @@ impl MonitorShm { td.trigger_showtext.fill(0); td.trigger_showtext[..showtext.len()].copy_from_slice(showtext.as_bytes()); + // Set trigger state to ON, after the writes above (a release fence: + // neither the compiler nor the CPU moves them past it) + std::sync::atomic::fence(std::sync::atomic::Ordering::Release); + td.trigger_state = TriggerState::On as u32; + // Ensure changes are flushed to shared memory self.mmap.flush()?; From 86b2c35dbb437fcde60289fcd82e062a764ae459 Mon Sep 17 00:00:00 2001 From: Steve Gilvarry Date: Sun, 15 Nov 2026 14:04:59 +1100 Subject: [PATCH 2/2] fix(monitor): cut a forced alarm's cause to 31 bytes after filtering, as v2 does The 31- and 255-byte limits were checked on the request as sent, so a cause that fits once its quotes and control characters are dropped got a 400. API v2's alarmCause filters first and then cuts to 31 bytes on a character boundary. Do the same, and cut the text to the 255-byte shm field the same way. The request still refuses a cause over 1024 bytes or a text over 4096, so an oversized body is a 400 rather than work for the filter. Co-Authored-By: Claude Opus 5.5 --- openapi.json | 4 +-- src/dto/request/monitor.rs | 37 +++++++++++++++++++++--- src/service/monitor.rs | 58 ++++++++++++++++++++++++++++++++------ 3 files changed, 85 insertions(+), 14 deletions(-) diff --git a/openapi.json b/openapi.json index 2c9b726a..d2e071b2 100644 --- a/openapi.json +++ b/openapi.json @@ -21192,7 +21192,7 @@ "string", "null" ], - "description": "Short cause string (max 31 chars). Only used when action is \"on\". Default: \"API\"" + "description": "Short cause. Only used when action is \"on\". Default: \"API\".\nFiltered as API v2 does, then cut to 31 bytes; up to 1024 bytes\nmay be sent." }, "score": { "type": [ @@ -21208,7 +21208,7 @@ "string", "null" ], - "description": "Description text (max 255 chars). Only used when action is \"on\"." + "description": "Description text. Only used when action is \"on\". Filtered as the\ncause is, then cut to 255 bytes; up to 4096 bytes may be sent." } } }, diff --git a/src/dto/request/monitor.rs b/src/dto/request/monitor.rs index 66cdd601..039c330f 100644 --- a/src/dto/request/monitor.rs +++ b/src/dto/request/monitor.rs @@ -1297,13 +1297,16 @@ pub struct AlarmControlRequest { #[serde(default)] pub score: Option, - /// Short cause string (max 31 chars). Only used when action is "on". Default: "API" - #[garde(length(max = 31))] + /// Short cause. Only used when action is "on". Default: "API". + /// Filtered as API v2 does, then cut to 31 bytes; up to 1024 bytes + /// may be sent. + #[garde(length(max = 1024))] #[serde(default)] pub cause: Option, - /// Description text (max 255 chars). Only used when action is "on". - #[garde(length(max = 255))] + /// Description text. Only used when action is "on". Filtered as the + /// cause is, then cut to 255 bytes; up to 4096 bytes may be sent. + #[garde(length(max = 4096))] #[serde(default)] pub text: Option, } @@ -1574,3 +1577,29 @@ impl MonitorListParams { } } } + +#[cfg(test)] +mod alarm_request_tests { + use super::AlarmControlRequest; + use garde::Validate; + + fn on(cause: &str, text: &str) -> AlarmControlRequest { + AlarmControlRequest { + action: "on".to_string(), + score: None, + cause: Some(cause.to_string()), + text: Some(text.to_string()), + } + } + + #[test] + fn cause_and_text_longer_than_the_shm_fields_are_accepted_and_cut_later() { + assert!(on(&"a".repeat(40), &"b".repeat(300)).validate().is_ok()); + } + + #[test] + fn oversized_cause_or_text_is_still_refused() { + assert!(on(&"a".repeat(1025), "x").validate().is_err()); + assert!(on("x", &"b".repeat(4097)).validate().is_err()); + } +} diff --git a/src/service/monitor.rs b/src/service/monitor.rs index 7a36e950..ae2dfd01 100644 --- a/src/service/monitor.rs +++ b/src/service/monitor.rs @@ -983,18 +983,32 @@ pub async fn audio_level( static ALARM_TEXT_DROP: Lazy = Lazy::new(|| Regex::new(r"[^\p{L}\p{M}\p{N} _.,:;!?()\[\]/+#@%=-]+").unwrap()); -fn plain_alarm_text(value: &str) -> String { +/// Bytes of shared memory each field holds, less the terminating NUL +/// (`TriggerData::trigger_cause` / `trigger_text`). +const ALARM_CAUSE_MAX: usize = 31; +const ALARM_TEXT_MAX: usize = 255; + +/// `value` filtered, then cut to `max` bytes on a character boundary with +/// any trailing space dropped: v2's order, so the limit applies to what is +/// stored rather than to what was sent. +fn plain_alarm_text(value: &str, max: usize) -> String { let kept = ALARM_TEXT_DROP.replace_all(value, ""); - kept.split(' ') + let joined = kept + .split(' ') .filter(|w| !w.is_empty()) .collect::>() - .join(" ") + .join(" "); + let mut end = joined.len().min(max); + while !joined.is_char_boundary(end) { + end -= 1; + } + joined[..end].trim_end().to_string() } -/// The cause to record for a forced alarm: the request's, filtered, or "API" -/// when none is given or nothing usable is left. +/// The cause to record for a forced alarm: the request's, filtered and cut +/// to 31 bytes, or "API" when none is given or nothing usable is left. fn alarm_cause(cause: Option<&str>) -> String { - let cause = plain_alarm_text(cause.unwrap_or("")); + let cause = plain_alarm_text(cause.unwrap_or(""), ALARM_CAUSE_MAX); if cause.is_empty() { "API".to_string() } else { @@ -1002,9 +1016,10 @@ fn alarm_cause(cause: Option<&str>) -> String { } } -/// The note text for a forced alarm, filtered as the cause is. +/// The note text for a forced alarm, filtered as the cause is and cut to +/// 255 bytes. fn alarm_text(text: Option<&str>) -> String { - let text = plain_alarm_text(text.unwrap_or("")); + let text = plain_alarm_text(text.unwrap_or(""), ALARM_TEXT_MAX); if text.is_empty() { "Triggered via REST API".to_string() } else { @@ -1265,4 +1280,31 @@ mod alarm_text_tests { ); assert_eq!(alarm_text(Some("\u{7}")), "Triggered via REST API"); } + + #[test] + fn a_cause_that_fits_once_filtered_is_kept_whole() { + // 40 bytes as sent, 31 once the quotes go + let sent = "\"\"\"\"\"\"\"\"\"Front door camera motion at 3am"; + assert_eq!(sent.len(), 40); + assert_eq!(alarm_cause(Some(sent)), "Front door camera motion at 3am"); + } + + #[test] + fn a_long_cause_is_cut_to_31_bytes_on_a_character_boundary() { + assert_eq!(alarm_cause(Some(&"a".repeat(50))), "a".repeat(31)); + // 15 two-byte letters fill 30 bytes; the 16th would make 32 + let cut = alarm_cause(Some(&"ü".repeat(20))); + assert_eq!(cut, "ü".repeat(15)); + // A cut that ends on a space doesn't keep it, as v2 rtrims + assert_eq!( + alarm_cause(Some(&format!("{} tail", "a".repeat(30)))), + "a".repeat(30) + ); + } + + #[test] + fn a_long_text_is_cut_to_the_255_byte_field() { + assert_eq!(alarm_text(Some(&"b".repeat(300))), "b".repeat(255)); + assert_eq!(alarm_text(Some(&"é".repeat(200))), "é".repeat(127)); + } }