From 738574b9f09d7b01d06d6ff19290b13455c82f10 Mon Sep 17 00:00:00 2001 From: Hibryda Date: Tue, 17 Mar 2026 03:56:44 +0100 Subject: [PATCH] fix(security): resolve all HIGH/MEDIUM/LOW audit findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rust fixes (HIGH): - symbols.rs: path validation (reject near-root, 50K file limit, symlink filter) - memory.rs: FTS5 query quoting (prevent operator injection), 1000 fragment cap, content length limit, transaction wrapping - budget.rs: atomic check-and-reserve via transaction, input validation, index on budget_log - export.rs: safe UTF-8 truncation via chars().take() - git_context.rs: reject paths starting with '-' (flag injection) - branch_policy.rs: action validation (block|warn only), path validation Rust fixes (MEDIUM): - export.rs: named column access (positional→named) - budget.rs: named column access, negative value guards Svelte fixes: - AccountSwitcher: 2-step confirmation before account switch - ProjectMemory: expand/collapse content, 2-step delete confirm, tags split fix - CodeIntelligence: min 2-char symbol query, CodeSymbol rename, aria-labels - BudgetManager: 10M upper bound, aria-label on input, named constants - SessionExporter: timeout cleanup on destroy, aria-live feedback - AnalyticsDashboard: SVG aria-label, removed unused import, named constant --- agor-pro/src/analytics.rs | 49 ++++++------ agor-pro/src/branch_policy.rs | 24 +++--- agor-pro/src/budget.rs | 80 ++++++++++++++++---- agor-pro/src/export.rs | 15 ++-- agor-pro/src/git_context.rs | 3 + agor-pro/src/memory.rs | 49 ++++++++---- agor-pro/src/symbols.rs | 28 ++++++- src/lib/commercial/AccountSwitcher.svelte | 37 ++++++++- src/lib/commercial/AnalyticsDashboard.svelte | 7 +- src/lib/commercial/BudgetManager.svelte | 13 +++- src/lib/commercial/CodeIntelligence.svelte | 13 ++-- src/lib/commercial/ProjectMemory.svelte | 42 ++++++++-- src/lib/commercial/SessionExporter.svelte | 11 ++- 13 files changed, 280 insertions(+), 91 deletions(-) diff --git a/agor-pro/src/analytics.rs b/agor-pro/src/analytics.rs index 8580b9b..d6a7416 100644 --- a/agor-pro/src/analytics.rs +++ b/agor-pro/src/analytics.rs @@ -44,17 +44,19 @@ pub fn pro_analytics_summary(project_id: String, days: Option) -> Result= ?2" ).map_err(|e| format!("Query failed: {e}"))?; let result = stmt.query_row(rusqlite::params![project_id, cutoff], |row| { - let count: i64 = row.get(0)?; - let cost: f64 = row.get(1)?; - let tokens: i64 = row.get(2)?; - let turns: i64 = row.get(3)?; - let tools: i64 = row.get(4)?; + let count: i64 = row.get("cnt")?; + let cost: f64 = row.get("total_cost")?; + let tokens: i64 = row.get("total_tokens")?; + let turns: i64 = row.get("total_turns")?; + let tools: i64 = row.get("total_tools")?; Ok(AnalyticsSummary { total_sessions: count, total_cost_usd: cost, @@ -77,9 +79,11 @@ pub fn pro_analytics_daily(project_id: String, days: Option) -> Result= ?2 GROUP BY day ORDER BY day ASC" @@ -87,12 +91,12 @@ pub fn pro_analytics_daily(project_id: String, days: Option) -> Result, _>>() @@ -108,21 +112,22 @@ pub fn pro_analytics_model_breakdown(project_id: String, days: Option) -> R let cutoff = now_epoch() - (period * 86400); let mut stmt = conn.prepare( - "SELECT COALESCE(model, 'unknown'), COUNT(*), COALESCE(SUM(cost_usd), 0), - COALESCE(SUM(peak_tokens), 0) + "SELECT COALESCE(model, 'unknown') AS model_name, COUNT(*) AS cnt, + COALESCE(SUM(cost_usd), 0) AS total_cost, + COALESCE(SUM(peak_tokens), 0) AS total_tokens FROM session_metrics WHERE project_id = ?1 AND end_time >= ?2 GROUP BY model ORDER BY SUM(cost_usd) DESC" ).map_err(|e| format!("Query failed: {e}"))?; let rows = stmt.query_map(rusqlite::params![project_id, cutoff], |row| { - let count: i64 = row.get(1)?; - let cost: f64 = row.get(2)?; + let count: i64 = row.get("cnt")?; + let cost: f64 = row.get("total_cost")?; Ok(ModelBreakdown { - model: row.get(0)?, + model: row.get("model_name")?, session_count: count, total_cost_usd: cost, - total_tokens: row.get(3)?, + total_tokens: row.get("total_tokens")?, avg_cost_per_session: if count > 0 { cost / count as f64 } else { 0.0 }, }) }).map_err(|e| format!("Query failed: {e}"))? diff --git a/agor-pro/src/branch_policy.rs b/agor-pro/src/branch_policy.rs index 5e46cdd..fc57ad2 100644 --- a/agor-pro/src/branch_policy.rs +++ b/agor-pro/src/branch_policy.rs @@ -35,7 +35,7 @@ fn ensure_tables(conn: &rusqlite::Connection) -> Result<(), String> { // Seed default policies if table is empty let count: i64 = conn.query_row( - "SELECT COUNT(*) FROM pro_branch_policies", [], |row| row.get(0) + "SELECT COUNT(*) AS cnt FROM pro_branch_policies", [], |row| row.get("cnt") ).unwrap_or(0); if count == 0 { @@ -62,6 +62,9 @@ fn glob_match(pattern: &str, value: &str) -> bool { } fn get_current_branch(project_path: &str) -> Result { + if project_path.starts_with('-') { + return Err("Invalid project path: cannot start with '-'".into()); + } let output = Command::new("git") .args(["-C", project_path, "branch", "--show-current"]) .output() @@ -87,10 +90,10 @@ pub fn pro_branch_check(project_path: String) -> Result let policies: Vec = stmt.query_map([], |row| { Ok(BranchPolicy { - id: row.get(0)?, - pattern: row.get(1)?, - action: row.get(2)?, - reason: row.get(3)?, + id: row.get("id")?, + pattern: row.get("pattern")?, + action: row.get("action")?, + reason: row.get("reason")?, }) }).map_err(|e| format!("Query failed: {e}"))? .collect::, _>>() @@ -127,10 +130,10 @@ pub fn pro_branch_policy_list() -> Result, String> { let rows = stmt.query_map([], |row| { Ok(BranchPolicy { - id: row.get(0)?, - pattern: row.get(1)?, - action: row.get(2)?, - reason: row.get(3)?, + id: row.get("id")?, + pattern: row.get("pattern")?, + action: row.get("action")?, + reason: row.get("reason")?, }) }).map_err(|e| format!("Query failed: {e}"))? .collect::, _>>() @@ -144,6 +147,9 @@ pub fn pro_branch_policy_add(pattern: String, action: Option, reason: Op let conn = super::open_sessions_db()?; ensure_tables(&conn)?; let act = action.unwrap_or_else(|| "block".into()); + if !["block", "warn"].contains(&act.as_str()) { + return Err(format!("Invalid action '{}': must be 'block' or 'warn'", act)); + } let rsn = reason.unwrap_or_default(); conn.execute( "INSERT INTO pro_branch_policies (pattern, action, reason) VALUES (?1, ?2, ?3)", diff --git a/agor-pro/src/budget.rs b/agor-pro/src/budget.rs index 2466f3a..a737c5d 100644 --- a/agor-pro/src/budget.rs +++ b/agor-pro/src/budget.rs @@ -46,7 +46,8 @@ fn ensure_tables(conn: &rusqlite::Connection) -> Result<(), String> { session_id TEXT NOT NULL, tokens_used INTEGER NOT NULL, timestamp INTEGER NOT NULL - );" + ); + CREATE INDEX IF NOT EXISTS idx_budget_log_project ON pro_budget_log(project_id, timestamp);" ).map_err(|e| format!("Failed to create budget tables: {e}")) } @@ -54,15 +55,59 @@ fn now_epoch() -> i64 { super::analytics::now_epoch() } -/// Calculate reset date: first day of next month as epoch. +/// Calculate reset date: first day of next calendar month as epoch. fn next_month_epoch() -> i64 { let now = now_epoch(); - // Approximate: 30 days from now - now + 30 * 86400 + let days_since_epoch = now / 86400; + let mut year = 1970i64; + let mut remaining_days = days_since_epoch; + loop { + let days_in_year = if is_leap_year(year) { 366 } else { 365 }; + if remaining_days < days_in_year { break; } + remaining_days -= days_in_year; + year += 1; + } + let month_days: [i64; 12] = if is_leap_year(year) { + [31,29,31,30,31,30,31,31,30,31,30,31] + } else { + [31,28,31,30,31,30,31,31,30,31,30,31] + }; + let mut month = 0usize; + for (i, &d) in month_days.iter().enumerate() { + if remaining_days < d { month = i; break; } + remaining_days -= d; + } + // Advance to first of next month + let (next_year, next_month) = if month >= 11 { + (year + 1, 0usize) + } else { + (year, month + 1) + }; + // Calculate epoch for first of next_month in next_year + let mut epoch: i64 = 0; + for y in 1970..next_year { + epoch += if is_leap_year(y) { 366 } else { 365 }; + } + let nm_days: [i64; 12] = if is_leap_year(next_year) { + [31,29,31,30,31,30,31,31,30,31,30,31] + } else { + [31,28,31,30,31,30,31,31,30,31,30,31] + }; + for i in 0..next_month { + epoch += nm_days[i]; + } + epoch * 86400 +} + +fn is_leap_year(y: i64) -> bool { + (y % 4 == 0 && y % 100 != 0) || y % 400 == 0 } #[tauri::command] pub fn pro_budget_set(project_id: String, monthly_limit_tokens: i64) -> Result<(), String> { + if monthly_limit_tokens <= 0 { + return Err("monthly_limit_tokens must be positive".into()); + } let conn = super::open_sessions_db()?; ensure_tables(&conn)?; let reset = next_month_epoch(); @@ -86,9 +131,9 @@ pub fn pro_budget_get(project_id: String) -> Result { ).map_err(|e| format!("Query failed: {e}"))?; stmt.query_row(params![project_id], |row| { - let limit: i64 = row.get(0)?; - let used: i64 = row.get(1)?; - let reset_date: i64 = row.get(2)?; + let limit: i64 = row.get("monthly_limit_tokens")?; + let used: i64 = row.get("used_tokens")?; + let reset_date: i64 = row.get("reset_date")?; let remaining = (limit - used).max(0); let percent = if limit > 0 { (used as f64 / limit as f64) * 100.0 } else { 0.0 }; Ok(BudgetStatus { project_id: project_id.clone(), limit, used, remaining, percent, reset_date }) @@ -97,6 +142,9 @@ pub fn pro_budget_get(project_id: String) -> Result { #[tauri::command] pub fn pro_budget_check(project_id: String, estimated_tokens: i64) -> Result { + if estimated_tokens < 0 { + return Err("estimated_tokens must be non-negative".into()); + } let conn = super::open_sessions_db()?; ensure_tables(&conn)?; auto_reset_if_expired(&conn, &project_id)?; @@ -105,7 +153,7 @@ pub fn pro_budget_check(project_id: String, estimated_tokens: i64) -> Result(0)?, row.get::<_, i64>(1)?)) + Ok((row.get::<_, i64>("monthly_limit_tokens")?, row.get::<_, i64>("used_tokens")?)) }); match result { @@ -133,14 +181,18 @@ pub fn pro_budget_log_usage(project_id: String, session_id: String, tokens_used: let conn = super::open_sessions_db()?; ensure_tables(&conn)?; let ts = now_epoch(); - conn.execute( + + let tx = conn.unchecked_transaction() + .map_err(|e| format!("Transaction failed: {e}"))?; + tx.execute( "INSERT INTO pro_budget_log (project_id, session_id, tokens_used, timestamp) VALUES (?1, ?2, ?3, ?4)", params![project_id, session_id, tokens_used, ts], ).map_err(|e| format!("Failed to log usage: {e}"))?; - conn.execute( + tx.execute( "UPDATE pro_budgets SET used_tokens = used_tokens + ?2 WHERE project_id = ?1", params![project_id, tokens_used], ).map_err(|e| format!("Failed to update used tokens: {e}"))?; + tx.commit().map_err(|e| format!("Commit failed: {e}"))?; Ok(()) } @@ -166,10 +218,10 @@ pub fn pro_budget_list() -> Result, String> { let rows = stmt.query_map([], |row| { Ok(BudgetEntry { - project_id: row.get(0)?, - monthly_limit_tokens: row.get(1)?, - used_tokens: row.get(2)?, - reset_date: row.get(3)?, + project_id: row.get("project_id")?, + monthly_limit_tokens: row.get("monthly_limit_tokens")?, + used_tokens: row.get("used_tokens")?, + reset_date: row.get("reset_date")?, }) }).map_err(|e| format!("Query failed: {e}"))? .collect::, _>>() diff --git a/agor-pro/src/export.rs b/agor-pro/src/export.rs index b2804eb..32ca8c9 100644 --- a/agor-pro/src/export.rs +++ b/agor-pro/src/export.rs @@ -38,8 +38,8 @@ pub fn pro_export_session(project_id: String, session_id: String) -> Result, String, Option) = stmt.query_row(rusqlite::params![project_id, session_id], |row| { - Ok((row.get(0)?, row.get(1)?, row.get(2)?, row.get(3)?, - row.get(4)?, row.get(5)?, row.get(6)?, row.get(7)?, row.get(8)?)) + Ok((row.get("start_time")?, row.get("end_time")?, row.get("peak_tokens")?, row.get("turn_count")?, + row.get("tool_call_count")?, row.get("cost_usd")?, row.get("model")?, row.get("status")?, row.get("error_message")?)) }).map_err(|e| format!("Session not found: {e}"))?; let model_name = model.clone().unwrap_or_else(|| "unknown".into()); @@ -55,7 +55,7 @@ pub fn pro_export_session(project_id: String, session_id: String) -> Result = msg_stmt .query_map(rusqlite::params![session_id], |row| { - Ok((row.get(0)?, row.get(1)?, row.get(2)?)) + Ok((row.get("message_type")?, row.get("content")?, row.get("created_at")?)) }) .map_err(|e| format!("Messages query failed: {e}"))? .collect::, _>>() @@ -91,9 +91,10 @@ pub fn pro_export_session(project_id: String, session_id: String) -> Result "**Tool Result**", _ => msg_type.as_str(), }; - // Truncate long content + // Truncate long content (safe UTF-8 boundary) let display = if content.len() > 500 { - format!("{}... *(truncated, {} chars)*", &content[..500], content.len()) + let truncated: String = content.chars().take(500).collect(); + format!("{}... *(truncated, {} chars)*", truncated, content.len()) } else { content.clone() }; @@ -126,8 +127,8 @@ pub fn pro_export_project_summary(project_id: String, days: Option) -> Resu let sessions: Vec<(String, i64, i64, f64, i64, i64, Option, String)> = stmt .query_map(rusqlite::params![project_id, cutoff], |row| { - Ok((row.get(0)?, row.get(1)?, row.get(2)?, row.get(3)?, - row.get(4)?, row.get(5)?, row.get(6)?, row.get(7)?)) + Ok((row.get("session_id")?, row.get("start_time")?, row.get("end_time")?, row.get("cost_usd")?, + row.get("turn_count")?, row.get("tool_call_count")?, row.get("model")?, row.get("status")?)) }) .map_err(|e| format!("Query failed: {e}"))? .collect::, _>>() diff --git a/agor-pro/src/git_context.rs b/agor-pro/src/git_context.rs index e89de86..d03ce5a 100644 --- a/agor-pro/src/git_context.rs +++ b/agor-pro/src/git_context.rs @@ -34,6 +34,9 @@ pub struct BranchInfo { } fn git_cmd(project_path: &str, args: &[&str]) -> Result { + if project_path.starts_with('-') { + return Err("Invalid project path: cannot start with '-'".into()); + } let output = Command::new("git") .args(["-C", project_path]) .args(args) diff --git a/agor-pro/src/memory.rs b/agor-pro/src/memory.rs index cda83a5..5719734 100644 --- a/agor-pro/src/memory.rs +++ b/agor-pro/src/memory.rs @@ -64,15 +64,15 @@ fn prune_expired(conn: &rusqlite::Connection) -> Result<(), String> { fn row_to_fragment(row: &rusqlite::Row) -> rusqlite::Result { Ok(MemoryFragment { - id: row.get(0)?, - project_id: row.get(1)?, - content: row.get(2)?, - source: row.get(3)?, - trust: row.get(4)?, - confidence: row.get(5)?, - created_at: row.get(6)?, - ttl_days: row.get(7)?, - tags: row.get(8)?, + id: row.get("id")?, + project_id: row.get("project_id")?, + content: row.get("content")?, + source: row.get("source")?, + trust: row.get("trust")?, + confidence: row.get("confidence")?, + created_at: row.get("created_at")?, + ttl_days: row.get("ttl_days")?, + tags: row.get("tags")?, }) } @@ -83,8 +83,21 @@ pub fn pro_memory_add( source: Option, tags: Option, ) -> Result { + if content.len() > 10000 { + return Err("Memory content too long (max 10000 chars)".into()); + } let conn = super::open_sessions_db()?; ensure_tables(&conn)?; + + // Per-project memory cap + let count: i64 = conn.query_row( + "SELECT COUNT(*) FROM pro_memories WHERE project_id = ?1", + params![project_id], |row| row.get(0) + ).unwrap_or(0); + if count >= 1000 { + return Err("Memory limit reached for this project (max 1000 fragments)".into()); + } + let ts = now_epoch(); let src = source.unwrap_or_default(); let tgs = tags.unwrap_or_default(); @@ -121,6 +134,9 @@ pub fn pro_memory_search(project_id: String, query: String) -> Result Result, _>>() .map_err(|e| format!("Row read failed: {e}"))?; @@ -147,18 +163,23 @@ pub fn pro_memory_update( let conn = super::open_sessions_db()?; ensure_tables(&conn)?; + let tx = conn.unchecked_transaction() + .map_err(|e| format!("Transaction failed: {e}"))?; + if let Some(c) = content { - conn.execute("UPDATE pro_memories SET content = ?2 WHERE id = ?1", params![id, c]) + tx.execute("UPDATE pro_memories SET content = ?2 WHERE id = ?1", params![id, c]) .map_err(|e| format!("Update content failed: {e}"))?; } if let Some(t) = trust { - conn.execute("UPDATE pro_memories SET trust = ?2 WHERE id = ?1", params![id, t]) + tx.execute("UPDATE pro_memories SET trust = ?2 WHERE id = ?1", params![id, t]) .map_err(|e| format!("Update trust failed: {e}"))?; } if let Some(c) = confidence { - conn.execute("UPDATE pro_memories SET confidence = ?2 WHERE id = ?1", params![id, c]) + tx.execute("UPDATE pro_memories SET confidence = ?2 WHERE id = ?1", params![id, c]) .map_err(|e| format!("Update confidence failed: {e}"))?; } + + tx.commit().map_err(|e| format!("Commit failed: {e}"))?; Ok(()) } @@ -185,7 +206,7 @@ pub fn pro_memory_inject(project_id: String, max_tokens: Option) -> Result< ).map_err(|e| format!("Query failed: {e}"))?; let entries: Vec<(String, String, f64)> = stmt - .query_map(params![project_id], |row| Ok((row.get(0)?, row.get(1)?, row.get(2)?))) + .query_map(params![project_id], |row| Ok((row.get("content")?, row.get("trust")?, row.get("confidence")?))) .map_err(|e| format!("Query failed: {e}"))? .collect::, _>>() .map_err(|e| format!("Row read failed: {e}"))?; diff --git a/agor-pro/src/symbols.rs b/agor-pro/src/symbols.rs index 3b9b639..a9b9bce 100644 --- a/agor-pro/src/symbols.rs +++ b/agor-pro/src/symbols.rs @@ -56,14 +56,32 @@ fn should_skip(name: &str) -> bool { SKIP_DIRS.contains(&name) } +const MAX_FILES: usize = 50_000; +const MAX_DEPTH: usize = 20; + fn walk_files(dir: &Path, files: &mut Vec) { + walk_files_bounded(dir, files, 0); +} + +fn walk_files_bounded(dir: &Path, files: &mut Vec, depth: usize) { + if depth >= MAX_DEPTH || files.len() >= MAX_FILES { + return; + } let Ok(entries) = std::fs::read_dir(dir) else { return }; for entry in entries.flatten() { + if files.len() >= MAX_FILES { + return; + } + let ft = entry.file_type(); + // Skip symlinks + if ft.as_ref().map_or(false, |ft| ft.is_symlink()) { + continue; + } let path = entry.path(); - if path.is_dir() { + if ft.as_ref().map_or(false, |ft| ft.is_dir()) { if let Some(name) = path.file_name().and_then(|n| n.to_str()) { if !should_skip(name) { - walk_files(&path, files); + walk_files_bounded(&path, files, depth + 1); } } } else if let Some(ext) = path.extension().and_then(|e| e.to_str()) { @@ -164,6 +182,9 @@ fn extract_ts_const_fn(line: &str) -> Option { pub fn pro_symbols_scan(project_path: String) -> Result { let start = std::time::Instant::now(); let root = PathBuf::from(&project_path); + if !root.is_absolute() || root.components().count() < 3 { + return Err("Invalid project path: must be an absolute path at least 3 levels deep".into()); + } if !root.is_dir() { return Err(format!("Not a directory: {project_path}")); } @@ -205,6 +226,9 @@ pub fn pro_symbols_search(project_path: String, query: String) -> Result Result, String> { let root = PathBuf::from(&project_path); + if !root.is_absolute() || root.components().count() < 3 { + return Err("Invalid project path: must be an absolute path at least 3 levels deep".into()); + } if !root.is_dir() { return Err(format!("Not a directory: {project_path}")); } diff --git a/src/lib/commercial/AccountSwitcher.svelte b/src/lib/commercial/AccountSwitcher.svelte index f1fc39a..55a5dab 100644 --- a/src/lib/commercial/AccountSwitcher.svelte +++ b/src/lib/commercial/AccountSwitcher.svelte @@ -10,6 +10,7 @@ let loading = $state(true); let error = $state(null); let switching = $state(null); + let confirmSwitch = $state(null); async function load() { loading = true; @@ -75,14 +76,31 @@ @@ -199,6 +217,9 @@ .account-action { flex-shrink: 0; + display: flex; + gap: 0.25rem; + align-items: center; } .active-label { @@ -226,4 +247,14 @@ opacity: 0.4; cursor: not-allowed; } + + .switch-btn.confirm { + color: var(--ctp-peach); + border-color: var(--ctp-peach); + } + + .switch-btn.confirm:hover:not(:disabled) { + background: color-mix(in srgb, var(--ctp-peach) 12%, transparent); + color: var(--ctp-peach); + } diff --git a/src/lib/commercial/AnalyticsDashboard.svelte b/src/lib/commercial/AnalyticsDashboard.svelte index 6923ff3..c9be4cb 100644 --- a/src/lib/commercial/AnalyticsDashboard.svelte +++ b/src/lib/commercial/AnalyticsDashboard.svelte @@ -1,6 +1,5 @@ // SPDX-License-Identifier: LicenseRef-Commercial