fix: Audit-Befunde in Protokoll-Parsing, Error-Handling und Eingabe-Validierung

BragiResponse-Felder korrekt zugeordnet (endpoint=raw[1], command=raw[2],
status=raw[3]) gemäß Protokoll-Doku. PropertyNotSupported durch DeviceError
ersetzt, parse_response_validated in device.rs aktiviert, flush() mit
Iterationslimit gegen Endlosschleifen, Sidetone-Range per clap validiert
statt clamp, JSON-Escaping im hidpp-battery-waybar.sh, udev auf uaccess
umgestellt. 52 Tests grün.
This commit is contained in:
nevaforget 2026-03-28 00:37:36 +01:00
parent 05d138e922
commit 25eacfc02d
15 changed files with 208 additions and 48 deletions

View File

@ -35,6 +35,8 @@ sudo ./target/debug/corsairctl info
## udev-Regel für rootless Zugriff
Nutzt `TAG+="uaccess"` — gibt dem am Seat eingeloggten User automatisch Zugriff, ohne Gruppen-Setup.
```bash
corsairctl udev | sudo tee /etc/udev/rules.d/99-corsair.rules
sudo udevadm control --reload-rules && sudo udevadm trigger

View File

@ -0,0 +1,35 @@
#!/bin/bash
# ABOUTME: Waybar-Modul für Logitech HID++ Geräte-Batterien.
# ABOUTME: Findet den richtigen hidpp_battery_* Eintrag dynamisch per MODEL_NAME.
MODEL="G515 LS TKL"
ICON="󰌌"
# hidpp_battery_* mit passendem MODEL_NAME finden
for dev in /sys/class/power_supply/hidpp_battery_*; do
[[ -d "$dev" ]] || continue
name=$(cat "$dev/model_name" 2>/dev/null)
if [[ "$name" == "$MODEL" ]]; then
capacity=$(cat "$dev/capacity" 2>/dev/null || echo 0)
status=$(cat "$dev/status" 2>/dev/null || echo "Unknown")
# Eingabe-Validierung gegen JSON-Injection
[[ "$capacity" =~ ^[0-9]+$ ]] || capacity=0
[[ "$status" =~ ^[A-Za-z\ ]+$ ]] || status="Unknown"
class="normal"
if [[ "$status" == "Charging" ]]; then
class="charging"
elif (( capacity <= 15 )); then
class="critical"
elif (( capacity <= 30 )); then
class="warning"
fi
echo "{\"text\":\"${capacity}% ${ICON}\",\"tooltip\":\"${MODEL}: ${capacity}% — ${status}\",\"class\":\"${class}\",\"percentage\":${capacity}}"
exit 0
fi
done
# Gerät nicht gefunden — nichts ausgeben, Waybar versteckt das Modul
exit 0

View File

@ -4,7 +4,10 @@
use hidapi::{HidApi, HidDevice};
use crate::bragi::properties::{self, BatteryStatus, Property};
use crate::bragi::protocol::{self, BragiResponse, ENDPOINT_HEADSET, ENDPOINT_RECEIVER};
use crate::bragi::protocol::{
self, BragiResponse, ENDPOINT_HEADSET, ENDPOINT_RECEIVER,
RESPONSE_ENDPOINT_HEADSET, CMD_GET, CMD_SET,
};
use crate::error::{CorsairError, Result};
use crate::hid;
@ -128,7 +131,7 @@ impl BragiDevice {
let hex: Vec<String> = raw.iter().take(10).map(|b| format!("{b:02X}")).collect();
eprintln!("[query] GET 0x{:02X}{}", property.id(), hex.join(" "));
}
protocol::parse_response(&raw)
protocol::parse_response_validated(&raw, Some(RESPONSE_ENDPOINT_HEADSET), Some(CMD_GET))
}
/// Setzt eine Property auf dem Headset.
@ -138,7 +141,7 @@ impl BragiDevice {
hid::flush(&self.device)?;
let packet = protocol::build_set_packet(ENDPOINT_HEADSET, property.id(), data);
let raw = hid::send_recv(&self.device, &packet)?;
protocol::parse_response(&raw)
protocol::parse_response_validated(&raw, Some(RESPONSE_ENDPOINT_HEADSET), Some(CMD_SET))
}
/// Batterie-Level in Prozent (0.0 - 100.0).

View File

@ -9,4 +9,5 @@ pub use device::BragiDevice;
pub use properties::{BatteryStatus, Property};
pub use protocol::{
BragiResponse, CORSAIR_VID, ENDPOINT_HEADSET, ENDPOINT_RECEIVER, HID_INTERFACE,
RESPONSE_ENDPOINT_HEADSET, RESPONSE_ENDPOINT_RECEIVER,
};

View File

@ -33,6 +33,10 @@ pub const STATUS_OK: u8 = 0x00;
pub const STATUS_ERROR_F0: u8 = 0xF0;
pub const STATUS_ERROR_F1: u8 = 0xF1;
// Response-Endpoints (weichen von Request-Endpoints ab!)
pub const RESPONSE_ENDPOINT_RECEIVER: u8 = 0x00;
pub const RESPONSE_ENDPOINT_HEADSET: u8 = 0x01;
/// Baut ein Bragi-Paket (65 Bytes) für den HID-Versand.
///
/// Format: [0x00, 0x02, endpoint, command, property, ...data, 0x00-padding]
@ -65,11 +69,14 @@ pub fn build_set_packet(endpoint: u8, property: u8, data: &[u8]) -> [u8; PACKET_
}
/// Geparste Bragi-Antwort.
///
/// Byte-Zuordnung laut Protokoll-Doku:
/// raw[0] = Report-Typ (0x01), raw[1] = Endpoint, raw[2] = Command, raw[3] = Status
#[derive(Debug, Clone)]
pub struct BragiResponse {
pub status: u8,
pub endpoint: u8,
pub command: u8,
pub status: u8,
pub data: Vec<u8>,
}
@ -97,11 +104,11 @@ impl BragiResponse {
}
}
/// Parst eine rohe HID-Response (64 Bytes ohne Report-ID) in eine BragiResponse.
/// Parst eine rohe HID-Response (64 Bytes) in eine BragiResponse.
///
/// Response-Format: [marker, status, endpoint, command, ...data]
/// Response-Format: [report_type, endpoint, command, status, ...data]
///
/// Validiert den Protokoll-Marker und optional den erwarteten Endpoint/Command,
/// Validiert optional den erwarteten Endpoint/Command,
/// um veraltete oder fremde Antworten aus dem HID-Puffer zu erkennen.
pub fn parse_response(raw: &[u8]) -> Result<BragiResponse> {
parse_response_validated(raw, None, None)
@ -120,13 +127,14 @@ pub fn parse_response_validated(
});
}
let status = raw[1];
let endpoint = raw[2];
let command = raw[3];
// raw[0] = Report-Typ (0x01)
let endpoint = raw[1];
let command = raw[2];
let status = raw[3];
// Fehler-Status prüfen
if status == STATUS_ERROR_F0 || status == STATUS_ERROR_F1 {
return Err(CorsairError::PropertyNotSupported { property: raw[3] });
// Fehler-Status prüfen: Endpoint-Byte 0xF0/0xF1 signalisiert Fehler
if endpoint == STATUS_ERROR_F0 || endpoint == STATUS_ERROR_F1 {
return Err(CorsairError::DeviceError { status: endpoint });
}
// Response-Korrelation: Endpoint und Command gegen die Anfrage prüfen
@ -148,9 +156,9 @@ pub fn parse_response_validated(
};
Ok(BragiResponse {
status,
endpoint,
command,
status,
data,
})
}

View File

@ -22,7 +22,8 @@ pub enum Command {
/// Sidetone-Level lesen oder setzen (0-23, ALSA-Mixer)
Sidetone {
/// Sidetone-Level (0-23). Ohne Angabe wird der aktuelle Wert gelesen.
level: Option<i64>,
#[arg(value_parser = clap::value_parser!(u8).range(0..=23))]
level: Option<u8>,
},
/// LED-Helligkeit lesen oder setzen (0-1000)

View File

@ -14,8 +14,8 @@ pub enum CorsairError {
#[error("Headset antwortet nicht — möglicherweise ausgeschaltet")]
HeadsetOffline,
#[error("Gerät-Fehler: Property 0x{property:02X} nicht unterstützt")]
PropertyNotSupported { property: u8 },
#[error("Gerät-Fehler: Status 0x{status:02X} (Property nicht unterstützt)")]
DeviceError { status: u8 },
#[error("Ungültige Antwort: erwartet mindestens {expected} Bytes, bekommen {got}")]
ResponseTooShort { expected: usize, got: usize },

View File

@ -89,10 +89,13 @@ pub fn send_recv_optional(device: &HidDevice, packet: &[u8; PACKET_SIZE]) -> Res
}
}
/// Maximale Iterationen beim Flushen, um Endlosschleifen zu verhindern.
const MAX_FLUSH_ITERATIONS: usize = 64;
/// Leert den HID-Empfangspuffer (nonblocking reads bis leer).
pub fn flush(device: &HidDevice) -> Result<()> {
let mut buf = [0u8; REPORT_SIZE];
loop {
for _ in 0..MAX_FLUSH_ITERATIONS {
// read_timeout mit 0ms = nonblocking
let bytes_read = device.read_timeout(&mut buf, 0)?;
if bytes_read == 0 {

View File

@ -2,6 +2,7 @@
// ABOUTME: Re-exportiert alle Module für externe Nutzung.
pub mod bragi;
pub mod cli;
pub mod error;
pub mod hid;
pub mod output;

View File

@ -25,9 +25,8 @@ fn run() -> error::Result<()> {
Command::Sidetone { level } => {
if let Some(value) = level {
let clamped = value.clamp(0, 23);
sidetone::set_level(clamped)?;
println!("{}", output::format_sidetone(clamped));
sidetone::set_level(value as i64)?;
println!("{}", output::format_sidetone(value as i64));
} else {
let current = sidetone::get_level()?;
println!("{}", output::format_sidetone(current));

46
tests/cli_test.rs Normal file
View File

@ -0,0 +1,46 @@
// ABOUTME: Unit-Tests für CLI-Argument-Parsing und Validierung.
// ABOUTME: Testet clap-Constraints wie Sidetone Range-Validierung.
use clap::Parser;
use corsairctl::cli::Cli;
/// Hilfsfunktion: Parst CLI-Argumente aus einem String-Slice.
fn parse_args(args: &[&str]) -> Result<Cli, clap::Error> {
Cli::try_parse_from(args)
}
#[test]
fn sidetone_rejects_negative() {
let result = parse_args(&["corsairctl", "sidetone", "--", "-1"]);
assert!(result.is_err());
}
#[test]
fn sidetone_rejects_above_23() {
let result = parse_args(&["corsairctl", "sidetone", "24"]);
assert!(result.is_err());
}
#[test]
fn sidetone_accepts_valid() {
let result = parse_args(&["corsairctl", "sidetone", "15"]);
assert!(result.is_ok());
}
#[test]
fn sidetone_accepts_zero() {
let result = parse_args(&["corsairctl", "sidetone", "0"]);
assert!(result.is_ok());
}
#[test]
fn sidetone_accepts_max() {
let result = parse_args(&["corsairctl", "sidetone", "23"]);
assert!(result.is_ok());
}
#[test]
fn sidetone_without_value_is_read_mode() {
let result = parse_args(&["corsairctl", "sidetone"]);
assert!(result.is_ok());
}

View File

@ -37,9 +37,8 @@ fn format_sidetone_value() {
#[test]
fn format_info_values() {
let result = output::format_info(0x1B1C, 0x0A69, 1797, 0);
assert!(result.contains("0x1B1C"));
assert!(result.contains("0x0A69"));
assert!(result.contains("1797.0"));
let expected = "Vendor ID: 0x1B1C\nProduct ID: 0x0A69\nFirmware: 1797.0";
assert_eq!(result, expected);
}
// Waybar-JSON Tests

View File

@ -68,6 +68,23 @@ fn battery_promille_to_percent_fractional() {
assert!((battery_promille_to_percent(735) - 73.5).abs() < f32::EPSILON);
}
#[test]
fn battery_status_icons() {
// Jede Variante muss ein nicht-leeres Icon liefern
assert!(!BatteryStatus::Offline.icon().is_empty());
assert!(!BatteryStatus::Discharging.icon().is_empty());
assert!(!BatteryStatus::Low.icon().is_empty());
assert!(!BatteryStatus::Charging.icon().is_empty());
assert!(!BatteryStatus::FullyCharged.icon().is_empty());
assert!(!BatteryStatus::Unknown(0xFF).icon().is_empty());
// Discharging und Low teilen das gleiche Icon
assert_eq!(BatteryStatus::Discharging.icon(), BatteryStatus::Low.icon());
// Charging und FullyCharged haben unterschiedliche Icons
assert_ne!(BatteryStatus::Charging.icon(), BatteryStatus::FullyCharged.icon());
}
#[test]
fn property_ids_match_protocol() {
assert_eq!(Property::BatteryLevel.id(), 0x0F);

View File

@ -46,22 +46,23 @@ fn build_packet_total_size() {
#[test]
fn parse_response_extracts_fields() {
// Simulierte Response: [marker, status, endpoint, command, data0, data1, ...]
let mut raw = vec![0x02, 0x00, 0x09, 0x02, 0xE8, 0x03];
// Simulierte Response: [report_type, endpoint, command, status, data0, data1, ...]
// Laut Protokoll-Doku: raw[0]=0x01 Report-Typ, raw[1]=Endpoint, raw[2]=Command, raw[3]=Status
let mut raw = vec![0x01, RESPONSE_ENDPOINT_HEADSET, CMD_GET, STATUS_OK, 0xE8, 0x03];
raw.resize(REPORT_SIZE, 0x00);
let resp = parse_response(&raw).unwrap();
assert_eq!(resp.status, STATUS_OK);
assert_eq!(resp.endpoint, ENDPOINT_HEADSET);
assert_eq!(resp.endpoint, RESPONSE_ENDPOINT_HEADSET);
assert_eq!(resp.command, CMD_GET);
assert_eq!(resp.status, STATUS_OK);
assert_eq!(resp.data[0], 0xE8);
assert_eq!(resp.data[1], 0x03);
}
#[test]
fn parse_response_value_u16_little_endian() {
let mut raw = vec![0x02, 0x00, 0x09, 0x02, 0xE8, 0x03];
let mut raw = vec![0x01, RESPONSE_ENDPOINT_HEADSET, CMD_GET, STATUS_OK, 0xE8, 0x03];
raw.resize(REPORT_SIZE, 0x00);
let resp = parse_response(&raw).unwrap();
@ -73,7 +74,7 @@ fn parse_response_value_u16_little_endian() {
#[test]
fn parse_response_value_u8() {
let mut raw = vec![0x02, 0x00, 0x09, 0x02, 0x03];
let mut raw = vec![0x01, RESPONSE_ENDPOINT_HEADSET, CMD_GET, STATUS_OK, 0x03];
raw.resize(REPORT_SIZE, 0x00);
let resp = parse_response(&raw).unwrap();
@ -82,31 +83,31 @@ fn parse_response_value_u8() {
#[test]
fn parse_response_error_f0() {
let raw = vec![0x02, 0xF0, 0x09, 0x02];
let raw = vec![0x01, 0xF0, CMD_GET, STATUS_OK];
let result = parse_response(&raw);
assert!(result.is_err());
assert!(matches!(
result.unwrap_err(),
CorsairError::PropertyNotSupported { .. }
));
match result.unwrap_err() {
CorsairError::DeviceError { status } => assert_eq!(status, 0xF0),
other => panic!("Erwarteter DeviceError, bekommen: {other:?}"),
}
}
#[test]
fn parse_response_error_f1() {
let raw = vec![0x02, 0xF1, 0x09, 0x02];
let raw = vec![0x01, 0xF1, CMD_GET, STATUS_OK];
let result = parse_response(&raw);
assert!(result.is_err());
assert!(matches!(
result.unwrap_err(),
CorsairError::PropertyNotSupported { .. }
));
match result.unwrap_err() {
CorsairError::DeviceError { status } => assert_eq!(status, 0xF1),
other => panic!("Erwarteter DeviceError, bekommen: {other:?}"),
}
}
#[test]
fn parse_response_too_short() {
let raw = vec![0x02, 0x00];
let raw = vec![0x01, 0x00];
let result = parse_response(&raw);
assert!(result.is_err());
@ -118,7 +119,7 @@ fn parse_response_too_short() {
#[test]
fn value_u16_on_empty_data_returns_error() {
let raw = vec![0x02, 0x00, 0x09, 0x02]; // Keine Daten-Bytes
let raw = vec![0x01, RESPONSE_ENDPOINT_HEADSET, CMD_GET, STATUS_OK]; // Keine Daten-Bytes
let resp = parse_response(&raw).unwrap();
let result = resp.value_u16();
@ -132,7 +133,7 @@ fn value_u16_on_empty_data_returns_error() {
#[test]
fn battery_level_full_is_1000_promille() {
// Battery Level = 1000 (voll) → 0xE8, 0x03 LE
let mut raw = vec![0x02, 0x00, 0x09, 0x02, 0xE8, 0x03];
let mut raw = vec![0x01, RESPONSE_ENDPOINT_HEADSET, CMD_GET, STATUS_OK, 0xE8, 0x03];
raw.resize(REPORT_SIZE, 0x00);
let resp = parse_response(&raw).unwrap();
@ -143,10 +144,55 @@ fn battery_level_full_is_1000_promille() {
#[test]
fn battery_level_half_is_500_promille() {
// Battery Level = 500 (50%) → 0xF4, 0x01 LE
let mut raw = vec![0x02, 0x00, 0x09, 0x02, 0xF4, 0x01];
let mut raw = vec![0x01, RESPONSE_ENDPOINT_HEADSET, CMD_GET, STATUS_OK, 0xF4, 0x01];
raw.resize(REPORT_SIZE, 0x00);
let resp = parse_response(&raw).unwrap();
let promille = resp.value_u16().unwrap();
assert_eq!(promille, 500);
}
// --- Schritt 4: parse_response_validated Tests ---
#[test]
fn parse_response_validated_rejects_wrong_endpoint() {
let mut raw = vec![0x01, RESPONSE_ENDPOINT_RECEIVER, CMD_GET, STATUS_OK, 0x00];
raw.resize(REPORT_SIZE, 0x00);
let result = parse_response_validated(&raw, Some(RESPONSE_ENDPOINT_HEADSET), None);
assert!(result.is_err());
assert!(matches!(
result.unwrap_err(),
CorsairError::UnexpectedResponse { .. }
));
}
#[test]
fn parse_response_validated_rejects_wrong_command() {
let mut raw = vec![0x01, RESPONSE_ENDPOINT_HEADSET, CMD_SET, STATUS_OK, 0x00];
raw.resize(REPORT_SIZE, 0x00);
let result = parse_response_validated(&raw, None, Some(CMD_GET));
assert!(result.is_err());
assert!(matches!(
result.unwrap_err(),
CorsairError::UnexpectedResponse { .. }
));
}
#[test]
fn parse_response_validated_accepts_matching() {
let mut raw = vec![0x01, RESPONSE_ENDPOINT_HEADSET, CMD_GET, STATUS_OK, 0xE8, 0x03];
raw.resize(REPORT_SIZE, 0x00);
let resp = parse_response_validated(
&raw,
Some(RESPONSE_ENDPOINT_HEADSET),
Some(CMD_GET),
)
.unwrap();
assert_eq!(resp.endpoint, RESPONSE_ENDPOINT_HEADSET);
assert_eq!(resp.command, CMD_GET);
assert_eq!(resp.status, STATUS_OK);
}

View File

@ -1,8 +1,7 @@
# Corsair Bragi-Geräte — rootless HID-Zugriff
# Installation: corsairctl udev | sudo tee /etc/udev/rules.d/99-corsair.rules
# Danach: sudo udevadm control --reload-rules && sudo udevadm trigger
# Benutzer muss in der Gruppe "plugdev" sein: sudo usermod -aG plugdev $USER
# HS80 RGB Wireless
SUBSYSTEM=="hidraw", ATTRS{idVendor}=="1b1c", ATTRS{idProduct}=="0a6b", MODE="0660", GROUP="plugdev"
SUBSYSTEM=="usb", ATTRS{idVendor}=="1b1c", ATTRS{idProduct}=="0a6b", MODE="0660", GROUP="plugdev"
# HS80 RGB Wireless — TAG+="uaccess" gibt dem eingeloggten Seat-User Zugriff
SUBSYSTEM=="hidraw", ATTRS{idVendor}=="1b1c", ATTRS{idProduct}=="0a6b", TAG+="uaccess"
SUBSYSTEM=="usb", ATTRS{idVendor}=="1b1c", ATTRS{idProduct}=="0a6b", TAG+="uaccess"