chore(clippy): clear warning backlog and harden conversions (#383)
This commit is contained in:
Chummy
GitHub
parent
a91516df7a
commit
3234159c6c
9 changed files with 77 additions and 69 deletions
|
|
@ -1247,18 +1247,16 @@ Done."#;
|
|||
// Recovery Tests - Constants Validation
|
||||
// ═══════════════════════════════════════════════════════════════════════
|
||||
|
||||
#[test]
|
||||
fn max_tool_iterations_is_reasonable() {
|
||||
// Recovery: MAX_TOOL_ITERATIONS should be set to prevent runaway loops
|
||||
const _: () = {
|
||||
assert!(MAX_TOOL_ITERATIONS > 0);
|
||||
assert!(MAX_TOOL_ITERATIONS <= 100);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn max_history_messages_is_reasonable() {
|
||||
// Recovery: MAX_HISTORY_MESSAGES should be set to prevent memory bloat
|
||||
assert!(MAX_HISTORY_MESSAGES > 0);
|
||||
assert!(MAX_HISTORY_MESSAGES <= 1000);
|
||||
};
|
||||
|
||||
#[test]
|
||||
fn constants_bounds_are_compile_time_checked() {
|
||||
// Bounds are enforced by the const assertions above.
|
||||
}
|
||||
|
||||
// ═══════════════════════════════════════════════════════════════════════
|
||||
|
|
|
|||
|
|
@ -1199,7 +1199,7 @@ pub struct LarkConfig {
|
|||
// ── Security Config ─────────────────────────────────────────────────
|
||||
|
||||
/// Security configuration for sandboxing, resource limits, and audit logging
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, Default)]
|
||||
pub struct SecurityConfig {
|
||||
/// Sandbox configuration
|
||||
#[serde(default)]
|
||||
|
|
@ -1214,16 +1214,6 @@ pub struct SecurityConfig {
|
|||
pub audit: AuditConfig,
|
||||
}
|
||||
|
||||
impl Default for SecurityConfig {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
sandbox: SandboxConfig::default(),
|
||||
resources: ResourceLimitsConfig::default(),
|
||||
audit: AuditConfig::default(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Sandbox configuration for OS-level isolation
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
pub struct SandboxConfig {
|
||||
|
|
@ -1251,10 +1241,11 @@ impl Default for SandboxConfig {
|
|||
}
|
||||
|
||||
/// Sandbox backend selection
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, Default)]
|
||||
#[serde(rename_all = "lowercase")]
|
||||
pub enum SandboxBackend {
|
||||
/// Auto-detect best available (default)
|
||||
#[default]
|
||||
Auto,
|
||||
/// Landlock (Linux kernel LSM, native)
|
||||
Landlock,
|
||||
|
|
@ -1268,12 +1259,6 @@ pub enum SandboxBackend {
|
|||
None,
|
||||
}
|
||||
|
||||
impl Default for SandboxBackend {
|
||||
fn default() -> Self {
|
||||
Self::Auto
|
||||
}
|
||||
}
|
||||
|
||||
/// Resource limits for command execution
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
pub struct ResourceLimitsConfig {
|
||||
|
|
|
|||
|
|
@ -20,7 +20,7 @@ use std::path::{Path, PathBuf};
|
|||
// ── Hardware transport enum ──────────────────────────────────────
|
||||
|
||||
/// Transport protocol used to communicate with physical hardware.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)]
|
||||
#[serde(rename_all = "lowercase")]
|
||||
pub enum HardwareTransport {
|
||||
/// Direct GPIO access on a Linux SBC (Raspberry Pi, Orange Pi, etc.)
|
||||
|
|
@ -30,15 +30,10 @@ pub enum HardwareTransport {
|
|||
/// SWD/JTAG debug probe (probe-rs) for bare-metal MCUs
|
||||
Probe,
|
||||
/// No hardware — software-only mode
|
||||
#[default]
|
||||
None,
|
||||
}
|
||||
|
||||
impl Default for HardwareTransport {
|
||||
fn default() -> Self {
|
||||
Self::None
|
||||
}
|
||||
}
|
||||
|
||||
impl std::fmt::Display for HardwareTransport {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
match self {
|
||||
|
|
@ -869,7 +864,9 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn validate_baud_rate_common_values_ok() {
|
||||
for baud in [9600, 19200, 38400, 57600, 115200, 230400, 460800, 921600] {
|
||||
for baud in [
|
||||
9600, 19200, 38400, 57600, 115_200, 230_400, 460_800, 921_600,
|
||||
] {
|
||||
let cfg = HardwareConfig {
|
||||
enabled: true,
|
||||
transport: "serial".into(),
|
||||
|
|
@ -938,7 +935,7 @@ mod tests {
|
|||
enabled: true,
|
||||
transport: "probe".into(),
|
||||
serial_port: None,
|
||||
baud_rate: 115200,
|
||||
baud_rate: 115_200,
|
||||
workspace_datasheets: false,
|
||||
discovered_board: None,
|
||||
probe_target: Some("nRF52840_xxAA".into()),
|
||||
|
|
|
|||
|
|
@ -183,7 +183,9 @@ impl Observer for OtelObserver {
|
|||
],
|
||||
);
|
||||
}
|
||||
ObserverEvent::LlmRequest { .. } => {}
|
||||
ObserverEvent::LlmRequest { .. }
|
||||
| ObserverEvent::ToolCallStart { .. }
|
||||
| ObserverEvent::TurnComplete => {}
|
||||
ObserverEvent::LlmResponse {
|
||||
provider,
|
||||
model,
|
||||
|
|
@ -247,7 +249,6 @@ impl Observer for OtelObserver {
|
|||
// Note: tokens are recorded via record_metric(TokensUsed) to avoid
|
||||
// double-counting. AgentEnd only records duration.
|
||||
}
|
||||
ObserverEvent::ToolCallStart { .. } => {}
|
||||
ObserverEvent::ToolCall {
|
||||
tool,
|
||||
duration,
|
||||
|
|
@ -285,7 +286,6 @@ impl Observer for OtelObserver {
|
|||
self.tool_duration
|
||||
.record(secs, &[KeyValue::new("tool", tool.clone())]);
|
||||
}
|
||||
ObserverEvent::TurnComplete => {}
|
||||
ObserverEvent::ChannelMessage { channel, direction } => {
|
||||
self.channel_messages.add(
|
||||
1,
|
||||
|
|
|
|||
|
|
@ -1999,7 +1999,7 @@ fn setup_hardware() -> Result<HardwareConfig> {
|
|||
hw_config.baud_rate = match baud_idx {
|
||||
1 => 9600,
|
||||
2 => 57600,
|
||||
3 => 230400,
|
||||
3 => 230_400,
|
||||
4 => {
|
||||
let custom: String = Input::new()
|
||||
.with_prompt(" Custom baud rate")
|
||||
|
|
|
|||
|
|
@ -57,7 +57,12 @@ fn parse_retry_after_ms(err: &anyhow::Error) -> Option<u64> {
|
|||
.take_while(|c| c.is_ascii_digit() || *c == '.')
|
||||
.collect();
|
||||
if let Ok(secs) = num_str.parse::<f64>() {
|
||||
return Some((secs * 1000.0) as u64);
|
||||
if secs.is_finite() && secs >= 0.0 {
|
||||
let millis = Duration::from_secs_f64(secs).as_millis();
|
||||
if let Ok(value) = u64::try_from(millis) {
|
||||
return Some(value);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -150,6 +150,18 @@ pub struct AuditLogger {
|
|||
buffer: Mutex<Vec<AuditEvent>>,
|
||||
}
|
||||
|
||||
/// Structured command execution details for audit logging.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct CommandExecutionLog<'a> {
|
||||
pub channel: &'a str,
|
||||
pub command: &'a str,
|
||||
pub risk_level: &'a str,
|
||||
pub approved: bool,
|
||||
pub allowed: bool,
|
||||
pub success: bool,
|
||||
pub duration_ms: u64,
|
||||
}
|
||||
|
||||
impl AuditLogger {
|
||||
/// Create a new audit logger
|
||||
pub fn new(config: AuditConfig, zeroclaw_dir: PathBuf) -> Result<Self> {
|
||||
|
|
@ -183,7 +195,23 @@ impl AuditLogger {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
/// Log a command execution event
|
||||
/// Log a command execution event.
|
||||
pub fn log_command_event(&self, entry: CommandExecutionLog<'_>) -> Result<()> {
|
||||
let event = AuditEvent::new(AuditEventType::CommandExecution)
|
||||
.with_actor(entry.channel.to_string(), None, None)
|
||||
.with_action(
|
||||
entry.command.to_string(),
|
||||
entry.risk_level.to_string(),
|
||||
entry.approved,
|
||||
entry.allowed,
|
||||
)
|
||||
.with_result(entry.success, None, entry.duration_ms, None);
|
||||
|
||||
self.log(&event)
|
||||
}
|
||||
|
||||
/// Backward-compatible helper to log a command execution event.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub fn log_command(
|
||||
&self,
|
||||
channel: &str,
|
||||
|
|
@ -194,24 +222,22 @@ impl AuditLogger {
|
|||
success: bool,
|
||||
duration_ms: u64,
|
||||
) -> Result<()> {
|
||||
let event = AuditEvent::new(AuditEventType::CommandExecution)
|
||||
.with_actor(channel.to_string(), None, None)
|
||||
.with_action(
|
||||
command.to_string(),
|
||||
risk_level.to_string(),
|
||||
approved,
|
||||
allowed,
|
||||
)
|
||||
.with_result(success, None, duration_ms, None);
|
||||
|
||||
self.log(&event)
|
||||
self.log_command_event(CommandExecutionLog {
|
||||
channel,
|
||||
command,
|
||||
risk_level,
|
||||
approved,
|
||||
allowed,
|
||||
success,
|
||||
duration_ms,
|
||||
})
|
||||
}
|
||||
|
||||
/// Rotate log if it exceeds max size
|
||||
fn rotate_if_needed(&self) -> Result<()> {
|
||||
if let Ok(metadata) = std::fs::metadata(&self.log_path) {
|
||||
let current_size_mb = metadata.len() / (1024 * 1024);
|
||||
if current_size_mb >= self.config.max_size_mb as u64 {
|
||||
if current_size_mb >= u64::from(self.config.max_size_mb) {
|
||||
self.rotate()?;
|
||||
}
|
||||
}
|
||||
|
|
@ -283,7 +309,8 @@ mod tests {
|
|||
|
||||
let json = serde_json::to_string(&event);
|
||||
assert!(json.is_ok());
|
||||
let parsed: AuditEvent = serde_json::from_str(&json.unwrap().as_str()).expect("parse");
|
||||
let json = json.expect("serialize");
|
||||
let parsed: AuditEvent = serde_json::from_str(json.as_str()).expect("parse");
|
||||
assert!(parsed.actor.is_some());
|
||||
assert!(parsed.action.is_some());
|
||||
assert!(parsed.result.is_some());
|
||||
|
|
|
|||
|
|
@ -902,8 +902,8 @@ mod tests {
|
|||
let json_str = r#"{"name": "GMAIL_SEND_EMAIL_WITH_ATTACHMENT", "appName": "gmail", "description": "Send email with attachment & special chars: <>'\"\"", "enabled": true}"#;
|
||||
let action: ComposioAction = serde_json::from_str(json_str).unwrap();
|
||||
assert_eq!(action.name, "GMAIL_SEND_EMAIL_WITH_ATTACHMENT");
|
||||
assert!(action.description.as_ref().unwrap().contains("&"));
|
||||
assert!(action.description.as_ref().unwrap().contains("<"));
|
||||
assert!(action.description.as_ref().unwrap().contains('&'));
|
||||
assert!(action.description.as_ref().unwrap().contains('<'));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -31,7 +31,7 @@ impl GitOperationsTool {
|
|||
|| arg_lower.starts_with("--upload-pack=")
|
||||
|| arg_lower.starts_with("--receive-pack=")
|
||||
|| arg_lower.contains("$(")
|
||||
|| arg_lower.contains("`")
|
||||
|| arg_lower.contains('`')
|
||||
|| arg.contains('|')
|
||||
|| arg.contains(';')
|
||||
{
|
||||
|
|
@ -90,10 +90,8 @@ impl GitOperationsTool {
|
|||
branch = line.trim_start_matches("# branch.head ").to_string();
|
||||
} else if let Some(rest) = line.strip_prefix("1 ") {
|
||||
// Ordinary changed entry
|
||||
let parts: Vec<&str> = rest.split(' ').collect();
|
||||
if parts.len() >= 2 {
|
||||
let path = parts.get(1).unwrap_or(&"");
|
||||
let staging = parts.get(0).unwrap_or(&"");
|
||||
let mut parts = rest.splitn(3, ' ');
|
||||
if let (Some(staging), Some(path)) = (parts.next(), parts.next()) {
|
||||
if !staging.is_empty() {
|
||||
let status_char = staging.chars().next().unwrap_or(' ');
|
||||
if status_char != '.' && status_char != ' ' {
|
||||
|
|
@ -203,7 +201,8 @@ impl GitOperationsTool {
|
|||
}
|
||||
|
||||
async fn git_log(&self, args: serde_json::Value) -> anyhow::Result<ToolResult> {
|
||||
let limit = args.get("limit").and_then(|v| v.as_u64()).unwrap_or(10) as usize;
|
||||
let limit_raw = args.get("limit").and_then(|v| v.as_u64()).unwrap_or(10);
|
||||
let limit = usize::try_from(limit_raw).unwrap_or(usize::MAX).min(1000);
|
||||
let limit_str = limit.to_string();
|
||||
|
||||
let output = self
|
||||
|
|
@ -383,7 +382,9 @@ impl GitOperationsTool {
|
|||
"pop" => self.run_git_command(&["stash", "pop"]).await,
|
||||
"list" => self.run_git_command(&["stash", "list"]).await,
|
||||
"drop" => {
|
||||
let index = args.get("index").and_then(|v| v.as_u64()).unwrap_or(0) as i32;
|
||||
let index_raw = args.get("index").and_then(|v| v.as_u64()).unwrap_or(0);
|
||||
let index = i32::try_from(index_raw)
|
||||
.map_err(|_| anyhow::anyhow!("stash index too large: {index_raw}"))?;
|
||||
self.run_git_command(&["stash", "drop", &format!("stash@{{{index}}}")])
|
||||
.await
|
||||
}
|
||||
|
|
@ -516,12 +517,7 @@ impl Tool for GitOperationsTool {
|
|||
error: Some("Action blocked: read-only mode".into()),
|
||||
});
|
||||
}
|
||||
AutonomyLevel::Supervised => {
|
||||
// Allow but require tracking
|
||||
}
|
||||
AutonomyLevel::Full => {
|
||||
// Allow freely
|
||||
}
|
||||
AutonomyLevel::Supervised | AutonomyLevel::Full => {}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue