Skip to content

Commit 2c136ac

Browse files
authored
fix(protocol): resolve 0xC511_0004 magic collision (closes ruvnet#928) (ruvnet#931)
* fix(ci): SAST actually scans the code + drop deprecated flaky semgrep action Two real problems in the Static Application Security Testing job: 1. **It scanned a path that no longer exists.** `bandit -r src/` and `semgrep … src/` pointed at the repo-root `src/`, but the Python code moved to `archive/v1/src/` (64 .py files) when the runtime was rewritten in Rust. So the SAST scan matched nothing — a silent no-op (this is also why `bandit-results.sarif` was "Path does not exist" on recent runs). Fixed both to `archive/v1/src/`. 2. **Deprecated + redundant + flaky semgrep step.** The `returntocorp/semgrep-action@v1` step pulled `returntocorp/semgrep-agent:v1` from Docker Hub every run (intermittently timing out → red check, e.g. on ruvnet#929) and is EOL. It was redundant: the pip `semgrep --sarif` step is what feeds GitHub Security; the action only pushed to the Semgrep cloud app via SEMGREP_APP_TOKEN. Removed it and folded its `p/docker` + `p/kubernetes` rulesets into the pip semgrep command, so coverage is preserved with no Docker pull. The job stays `continue-on-error: true` (non-gating). YAML validated. Co-Authored-By: claude-flow <ruv@ruv.net> * fix(protocol): resolve 0xC511_0004 magic collision (closes ruvnet#928) Background `0xC511_0004` was assigned to two different packet formats in firmware — `EDGE_FUSED_MAGIC` (ADR-063, 48-byte `edge_fused_vitals_pkt_t`) and `WASM_OUTPUT_MAGIC` (ADR-040, variable-length `wasm_output_pkt_t`). Both were transmitted. The sensing-server only had a WASM parser for that magic and no fused-vitals parser, so on the ESP32-C6 + MR60BHA2 mmWave configuration the fused-vitals packet was silently misparsed as a malformed WASM output — `breathing_rate` was read as `event_count`, mmWave-fused vitals were lost, and spurious WASM events were emitted to subscribers. Fix 1. Reassign `WASM_OUTPUT_MAGIC` to `0xC511_0007` (next free slot per the registry in `rv_feature_state.h`). Smaller blast radius than moving fused-vitals — the registry already treats `0xC511_0004` as fused-vitals canonical and several years of deployed feature tracking depends on that assignment. 2. Add `parse_edge_fused_vitals` + `EdgeFusedVitalsPacket` in `wifi-densepose-sensing-server::main`. Byte layout taken directly from `edge_processing.h:129`, mirroring the firmware's `_Static_assert(sizeof(edge_fused_vitals_pkt_t) == 48)` so future firmware changes that grow the packet will break this parser loudly instead of silently. 3. Add a dispatch arm in the UDP receive loop. Fused-vitals is tried BEFORE WASM so a stale firmware (still emitting 0xC511_0004 with the WASM payload) fails to parse as fused-vitals (size mismatch), then fails to parse as WASM (magic mismatch on the new 0x...0007), and gets dropped — a deliberate "fail loud" outcome rather than the pre-fix silent garbage. 4. Update the registry comment in `rv_feature_state.h` to add the new 0x...0007 row. 5. Add five tests in a new `issue_928_magic_collision_tests` mod: - `parse_edge_fused_vitals_extracts_fields_correctly` - `parse_edge_fused_vitals_rejects_short_buffer` - `parse_edge_fused_vitals_rejects_wrong_magic` - `parse_wasm_output_rejects_legacy_0004_magic` - `parse_wasm_output_accepts_new_0007_magic` WebSocket payload Fused-vitals now broadcasts as `{"type": "edge_fused_vitals", ...}` with the mmWave-specific block nested under `mmwave`. Schema is additive — existing subscribers that only inspect `type` are unaffected; subscribers that switch on `type` gain a new branch. Deployment note This is a wire-protocol change. Firmware older than this commit that emits WASM output on 0xC511_0004 will lose its WASM event stream against an updated host (host expects 0xC511_0007). Per the issue discussion, "fail loud" is preferred to silent misparsing. Operators running C6+mmWave should reflash firmware concurrent with the host upgrade. Test results cargo test -p wifi-densepose-sensing-server --no-default-features --bin sensing-server → 122 passed / 0 failed (5 new + 117 existing, unchanged) Co-Authored-By: claude-flow <ruv@ruv.net>
1 parent 69e61e3 commit 2c136ac

4 files changed

Lines changed: 240 additions & 9 deletions

File tree

firmware/esp32-csi-node/main/rv_feature_state.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212
* 0xC5110003 — ADR-069 feature vector (edge_processing.h)
1313
* 0xC5110004 — ADR-063 fused vitals (edge_processing.h)
1414
* 0xC5110005 — ADR-039 compressed CSI (edge_processing.h)
15-
* 0xC5110006 — ADR-081 feature state (this file) ← new
15+
* 0xC5110006 — ADR-081 feature state (this file)
16+
* 0xC5110007 — ADR-040 WASM output (wasm_runtime.h, reassigned per issue #928)
1617
*/
1718

1819
#ifndef RV_FEATURE_STATE_H

firmware/esp32-csi-node/main/wasm_runtime.h

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,16 @@
4343

4444
#define WASM_MAX_MODULE_SIZE (128 * 1024) /**< Max .wasm binary size (128 KB). */
4545
#define WASM_STACK_SIZE (8 * 1024) /**< WASM execution stack (8 KB). */
46-
#define WASM_OUTPUT_MAGIC 0xC5110004 /**< WASM output packet magic. */
46+
/* Issue #928: WASM output was originally 0xC5110004, but that magic is
47+
* canonically owned by ADR-063 fused vitals (edge_processing.h). Both packets
48+
* were transmitted on the same magic, and the host parser only knew the WASM
49+
* shape, so on the ESP32-C6 + MR60BHA2 mmWave config the 48-byte fused-vitals
50+
* packet was being read as garbage WASM events. Reassigned to 0xC5110007 (next
51+
* free slot in the registry — see rv_feature_state.h). Firmware older than
52+
* this commit will silently lose its WASM event stream against an updated host
53+
* — that's the deliberate "fail loud" choice over silent misparsing.
54+
*/
55+
#define WASM_OUTPUT_MAGIC 0xC5110007 /**< WASM output packet magic (post-#928). */
4756
#define WASM_MAX_EVENTS 16 /**< Max events per output packet. */
4857

4958
/* ---- WASM Event (5 bytes: u8 type + f32 value) ---- */
@@ -54,7 +63,7 @@ typedef struct __attribute__((packed)) {
5463

5564
/* ---- WASM Output Packet ---- */
5665
typedef struct __attribute__((packed)) {
57-
uint32_t magic; /**< WASM_OUTPUT_MAGIC = 0xC5110004. */
66+
uint32_t magic; /**< WASM_OUTPUT_MAGIC = 0xC5110007 (issue #928). */
5867
uint8_t node_id; /**< ESP32 node identifier. */
5968
uint8_t module_id; /**< Module slot index. */
6069
uint16_t event_count; /**< Number of events in this packet. */

v2/crates/wifi-densepose-sensing-server/src/csi.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,14 @@ pub fn parse_esp32_vitals(buf: &[u8]) -> Option<Esp32VitalsPacket> {
4545
})
4646
}
4747

48-
/// Parse a WASM output packet (magic 0xC511_0004).
48+
/// Parse a WASM output packet (magic 0xC511_0007 — reassigned per issue #928;
49+
/// the original 0xC511_0004 collided with ADR-063 fused vitals).
4950
pub fn parse_wasm_output(buf: &[u8]) -> Option<WasmOutputPacket> {
5051
if buf.len() < 8 {
5152
return None;
5253
}
5354
let magic = u32::from_le_bytes([buf[0], buf[1], buf[2], buf[3]]);
54-
if magic != 0xC511_0004 {
55+
if magic != 0xC511_0007 {
5556
return None;
5657
}
5758

v2/crates/wifi-densepose-sensing-server/src/main.rs

Lines changed: 224 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1114,7 +1114,7 @@ fn parse_esp32_vitals(buf: &[u8]) -> Option<Esp32VitalsPacket> {
11141114
})
11151115
}
11161116

1117-
// ── ADR-040: WASM Output Packet (magic 0xC511_0004) ───────────────────────────
1117+
// ── ADR-040: WASM Output Packet (magic 0xC511_0007 — reassigned per #928) ─────
11181118

11191119
/// Single WASM event (type + value).
11201120
#[derive(Debug, Clone, Serialize)]
@@ -1131,13 +1131,14 @@ struct WasmOutputPacket {
11311131
events: Vec<WasmEvent>,
11321132
}
11331133

1134-
/// Parse a WASM output packet (magic 0xC511_0004).
1134+
/// Parse a WASM output packet (magic 0xC511_0007 — reassigned per issue #928;
1135+
/// the original 0xC511_0004 was a collision with ADR-063 fused vitals).
11351136
fn parse_wasm_output(buf: &[u8]) -> Option<WasmOutputPacket> {
11361137
if buf.len() < 8 {
11371138
return None;
11381139
}
11391140
let magic = u32::from_le_bytes([buf[0], buf[1], buf[2], buf[3]]);
1140-
if magic != 0xC511_0004 {
1141+
if magic != 0xC511_0007 {
11411142
return None;
11421143
}
11431144

@@ -1169,6 +1170,187 @@ fn parse_wasm_output(buf: &[u8]) -> Option<WasmOutputPacket> {
11691170
})
11701171
}
11711172

1173+
// ── ADR-063: Edge Fused Vitals Packet (magic 0xC511_0004) ─────────────────────
1174+
//
1175+
// 48-byte packed struct emitted by the ESP32-C6 + MR60BHA2 mmWave config when
1176+
// `mmwave_sensor_get_state().detected` is true. Byte layout from
1177+
// `firmware/esp32-csi-node/main/edge_processing.h` line 129 — kept in lockstep
1178+
// with the firmware's `_Static_assert(sizeof(edge_fused_vitals_pkt_t) == 48)`.
1179+
// Issue #928 surfaced that this magic was being parsed as WASM output and the
1180+
// fused vitals were silently lost. Adding the proper parser here.
1181+
1182+
#[derive(Debug, Clone, Serialize)]
1183+
struct EdgeFusedVitalsPacket {
1184+
node_id: u8,
1185+
/// Bit0=presence, Bit1=fall, Bit2=motion, Bit3=mmwave_present.
1186+
flags: u8,
1187+
/// Fused breathing rate in BPM (firmware sends BPM*100; we scale here).
1188+
breathing_rate_bpm: f32,
1189+
/// Fused heartrate in BPM (firmware sends BPM*10000; we scale here).
1190+
heartrate_bpm: f32,
1191+
rssi: i8,
1192+
n_persons: u8,
1193+
/// `mmwave_type_t` enum value from firmware.
1194+
mmwave_type: u8,
1195+
/// 0-100 fusion quality score.
1196+
fusion_confidence: u8,
1197+
motion_energy: f32,
1198+
presence_score: f32,
1199+
timestamp_ms: u32,
1200+
/// Raw mmWave heart rate (BPM).
1201+
mmwave_hr_bpm: f32,
1202+
/// Raw mmWave breathing rate (BPM).
1203+
mmwave_br_bpm: f32,
1204+
/// Distance to nearest target (cm).
1205+
mmwave_distance_cm: f32,
1206+
/// Target count from mmWave.
1207+
mmwave_targets: u8,
1208+
/// mmWave signal quality 0-100.
1209+
mmwave_confidence: u8,
1210+
}
1211+
1212+
/// Parse an ADR-063 edge fused vitals packet (magic 0xC511_0004, 48 bytes).
1213+
fn parse_edge_fused_vitals(buf: &[u8]) -> Option<EdgeFusedVitalsPacket> {
1214+
if buf.len() < 48 {
1215+
return None;
1216+
}
1217+
let magic = u32::from_le_bytes([buf[0], buf[1], buf[2], buf[3]]);
1218+
if magic != 0xC511_0004 {
1219+
return None;
1220+
}
1221+
1222+
let node_id = buf[4];
1223+
let flags = buf[5];
1224+
let breathing_raw = u16::from_le_bytes([buf[6], buf[7]]);
1225+
let heartrate_raw = u32::from_le_bytes([buf[8], buf[9], buf[10], buf[11]]);
1226+
let rssi = buf[12] as i8;
1227+
let n_persons = buf[13];
1228+
let mmwave_type = buf[14];
1229+
let fusion_confidence = buf[15];
1230+
let motion_energy = f32::from_le_bytes([buf[16], buf[17], buf[18], buf[19]]);
1231+
let presence_score = f32::from_le_bytes([buf[20], buf[21], buf[22], buf[23]]);
1232+
let timestamp_ms = u32::from_le_bytes([buf[24], buf[25], buf[26], buf[27]]);
1233+
let mmwave_hr_bpm = f32::from_le_bytes([buf[28], buf[29], buf[30], buf[31]]);
1234+
let mmwave_br_bpm = f32::from_le_bytes([buf[32], buf[33], buf[34], buf[35]]);
1235+
let mmwave_distance_cm = f32::from_le_bytes([buf[36], buf[37], buf[38], buf[39]]);
1236+
let mmwave_targets = buf[40];
1237+
let mmwave_confidence = buf[41];
1238+
// buf[42..48] are firmware reserved fields (reserved3 u16 + reserved4 u32).
1239+
1240+
Some(EdgeFusedVitalsPacket {
1241+
node_id,
1242+
flags,
1243+
breathing_rate_bpm: breathing_raw as f32 / 100.0,
1244+
heartrate_bpm: heartrate_raw as f32 / 10000.0,
1245+
rssi,
1246+
n_persons,
1247+
mmwave_type,
1248+
fusion_confidence,
1249+
motion_energy,
1250+
presence_score,
1251+
timestamp_ms,
1252+
mmwave_hr_bpm,
1253+
mmwave_br_bpm,
1254+
mmwave_distance_cm,
1255+
mmwave_targets,
1256+
mmwave_confidence,
1257+
})
1258+
}
1259+
1260+
#[cfg(test)]
1261+
mod issue_928_magic_collision_tests {
1262+
//! Issue #928 — `0xC511_0004` was being parsed as WASM output, eating the
1263+
//! C6+mmWave fused-vitals packets. After this fix, `0xC511_0004` routes to
1264+
//! `parse_edge_fused_vitals` and WASM output owns the freshly-allocated
1265+
//! `0xC511_0007` slot. Tests guard both halves of the swap.
1266+
use super::*;
1267+
1268+
/// Build a 48-byte synthetic fused-vitals packet matching the firmware's
1269+
/// `edge_fused_vitals_pkt_t` layout from `edge_processing.h:129`.
1270+
fn build_fused_vitals_packet() -> Vec<u8> {
1271+
let mut buf = vec![0u8; 48];
1272+
buf[0..4].copy_from_slice(&0xC511_0004u32.to_le_bytes());
1273+
buf[4] = 9; // node_id
1274+
buf[5] = 0b0000_1001; // flags: presence | mmwave_present
1275+
buf[6..8].copy_from_slice(&1600u16.to_le_bytes()); // breathing 16.00 BPM
1276+
buf[8..12].copy_from_slice(&720_000u32.to_le_bytes()); // heartrate 72.0 BPM
1277+
buf[12] = (-55i8) as u8; // rssi
1278+
buf[13] = 1; // n_persons
1279+
buf[14] = 2; // mmwave_type
1280+
buf[15] = 85; // fusion_confidence
1281+
buf[16..20].copy_from_slice(&0.42f32.to_le_bytes()); // motion_energy
1282+
buf[20..24].copy_from_slice(&0.95f32.to_le_bytes()); // presence_score
1283+
buf[24..28].copy_from_slice(&1_234_567u32.to_le_bytes()); // timestamp_ms
1284+
buf[28..32].copy_from_slice(&71.5f32.to_le_bytes()); // mmwave_hr_bpm
1285+
buf[32..36].copy_from_slice(&15.8f32.to_le_bytes()); // mmwave_br_bpm
1286+
buf[36..40].copy_from_slice(&182.0f32.to_le_bytes()); // mmwave_distance_cm
1287+
buf[40] = 1; // mmwave_targets
1288+
buf[41] = 90; // mmwave_confidence
1289+
// bytes 42..48 — firmware reserved fields, left as zero
1290+
buf
1291+
}
1292+
1293+
#[test]
1294+
fn parse_edge_fused_vitals_extracts_fields_correctly() {
1295+
let buf = build_fused_vitals_packet();
1296+
let pkt = parse_edge_fused_vitals(&buf).expect("must parse a well-formed packet");
1297+
assert_eq!(pkt.node_id, 9);
1298+
assert_eq!(pkt.flags, 0b0000_1001);
1299+
assert!((pkt.breathing_rate_bpm - 16.0).abs() < 1e-3, "breathing scale 100");
1300+
assert!((pkt.heartrate_bpm - 72.0).abs() < 1e-3, "heartrate scale 10000");
1301+
assert_eq!(pkt.rssi, -55);
1302+
assert_eq!(pkt.n_persons, 1);
1303+
assert_eq!(pkt.mmwave_type, 2);
1304+
assert_eq!(pkt.fusion_confidence, 85);
1305+
assert!((pkt.motion_energy - 0.42).abs() < 1e-6);
1306+
assert!((pkt.presence_score - 0.95).abs() < 1e-6);
1307+
assert_eq!(pkt.timestamp_ms, 1_234_567);
1308+
assert!((pkt.mmwave_hr_bpm - 71.5).abs() < 1e-6);
1309+
assert!((pkt.mmwave_br_bpm - 15.8).abs() < 1e-3);
1310+
assert!((pkt.mmwave_distance_cm - 182.0).abs() < 1e-6);
1311+
assert_eq!(pkt.mmwave_targets, 1);
1312+
assert_eq!(pkt.mmwave_confidence, 90);
1313+
}
1314+
1315+
#[test]
1316+
fn parse_edge_fused_vitals_rejects_short_buffer() {
1317+
let buf = build_fused_vitals_packet();
1318+
// Truncate to 47 bytes — one short of the 48-byte minimum.
1319+
assert!(parse_edge_fused_vitals(&buf[..47]).is_none());
1320+
}
1321+
1322+
#[test]
1323+
fn parse_edge_fused_vitals_rejects_wrong_magic() {
1324+
let mut buf = build_fused_vitals_packet();
1325+
buf[0..4].copy_from_slice(&0xC511_0007u32.to_le_bytes()); // WASM magic, not fused
1326+
assert!(parse_edge_fused_vitals(&buf).is_none());
1327+
}
1328+
1329+
#[test]
1330+
fn parse_wasm_output_rejects_legacy_0004_magic() {
1331+
// The old WASM magic collided with fused vitals — must no longer be
1332+
// accepted. A real fused-vitals packet starts with 0xC511_0004 and
1333+
// would have been misparsed before this fix.
1334+
let buf = build_fused_vitals_packet();
1335+
assert!(parse_wasm_output(&buf).is_none(),
1336+
"issue #928: WASM parser must NOT accept 0xC511_0004");
1337+
}
1338+
1339+
#[test]
1340+
fn parse_wasm_output_accepts_new_0007_magic() {
1341+
// Build a tiny well-formed WASM output packet on the new magic.
1342+
let mut buf = vec![0u8; 8];
1343+
buf[0..4].copy_from_slice(&0xC511_0007u32.to_le_bytes());
1344+
buf[4] = 5; // node_id
1345+
buf[5] = 1; // module_id
1346+
buf[6..8].copy_from_slice(&0u16.to_le_bytes()); // event_count = 0
1347+
let pkt = parse_wasm_output(&buf).expect("0xC511_0007 must parse");
1348+
assert_eq!(pkt.node_id, 5);
1349+
assert_eq!(pkt.module_id, 1);
1350+
assert!(pkt.events.is_empty());
1351+
}
1352+
}
1353+
11721354
// ── ESP32 UDP frame parser ───────────────────────────────────────────────────
11731355

11741356
fn parse_esp32_frame(buf: &[u8]) -> Option<Esp32Frame> {
@@ -4979,7 +5161,45 @@ async fn udp_receiver_task(state: SharedState, udp_port: u16) {
49795161
}
49805162
}
49815163

4982-
// ADR-040: Try WASM output packet (magic 0xC511_0004).
5164+
// ADR-063: Try edge fused vitals packet (magic 0xC511_0004).
5165+
// Must come BEFORE the WASM parser — issue #928: these two
5166+
// packet types shared a magic and the WASM parser was eating
5167+
// fused-vitals frames on the C6+mmWave config. The reassign of
5168+
// WASM_OUTPUT_MAGIC → 0xC511_0007 (firmware side) plus this
5169+
// dedicated parser resolve the collision.
5170+
if let Some(fused) = parse_edge_fused_vitals(&buf[..len]) {
5171+
debug!(
5172+
"Edge fused vitals from {src}: node={} br={:.1} hr={:.1} \
5173+
mmwave_targets={} fusion_conf={}",
5174+
fused.node_id, fused.breathing_rate_bpm, fused.heartrate_bpm,
5175+
fused.mmwave_targets, fused.fusion_confidence,
5176+
);
5177+
let s = state.write().await;
5178+
if let Ok(json) = serde_json::to_string(&serde_json::json!({
5179+
"type": "edge_fused_vitals",
5180+
"node_id": fused.node_id,
5181+
"breathing_rate_bpm": fused.breathing_rate_bpm,
5182+
"heartrate_bpm": fused.heartrate_bpm,
5183+
"n_persons": fused.n_persons,
5184+
"fusion_confidence": fused.fusion_confidence,
5185+
"mmwave": {
5186+
"hr_bpm": fused.mmwave_hr_bpm,
5187+
"br_bpm": fused.mmwave_br_bpm,
5188+
"distance_cm": fused.mmwave_distance_cm,
5189+
"targets": fused.mmwave_targets,
5190+
"confidence": fused.mmwave_confidence,
5191+
"type": fused.mmwave_type,
5192+
},
5193+
"motion_energy": fused.motion_energy,
5194+
"presence_score": fused.presence_score,
5195+
"timestamp_ms": fused.timestamp_ms,
5196+
})) {
5197+
let _ = s.tx.send(json);
5198+
}
5199+
continue;
5200+
}
5201+
5202+
// ADR-040: Try WASM output packet (magic 0xC511_0007 post-#928).
49835203
if let Some(wasm_output) = parse_wasm_output(&buf[..len]) {
49845204
debug!(
49855205
"WASM output from {src}: node={} module={} events={}",

0 commit comments

Comments
 (0)