fix: address code review issues (mutex safety, schema, CSP, error handling)

- Replace .unwrap() on Mutex lock with proper error propagation
- Add worktree_path and branch_name columns to processed_tickets
- Add style-src to CSP for Tailwind compatibility
- Implement std::error::Error for AppError
- Check affected row count in update/delete operations
- Handle non-UTF-8 paths in git clone

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
thibaud-leclere 2026-04-13 10:16:44 +02:00
parent a626666275
commit 81d3b72e34
5 changed files with 22 additions and 9 deletions

View file

@ -34,6 +34,8 @@ CREATE TABLE IF NOT EXISTS processed_tickets (
status TEXT NOT NULL DEFAULT 'Pending', status TEXT NOT NULL DEFAULT 'Pending',
analyst_report TEXT, analyst_report TEXT,
developer_report TEXT, developer_report TEXT,
worktree_path TEXT,
branch_name TEXT,
detected_at TEXT NOT NULL DEFAULT (datetime('now')), detected_at TEXT NOT NULL DEFAULT (datetime('now')),
processed_at TEXT processed_at TEXT
); );

View file

@ -20,8 +20,11 @@ pub fn create_project(
let clone_dir = home.join("orchai-repos").join(&name); let clone_dir = home.join("orchai-repos").join(&name);
std::fs::create_dir_all(&clone_dir)?; std::fs::create_dir_all(&clone_dir)?;
let clone_dir_str = clone_dir.to_str()
.ok_or_else(|| AppError::from("Clone path contains invalid characters".to_string()))?;
let output = Command::new("git") let output = Command::new("git")
.args(["clone", &path_or_url, clone_dir.to_str().unwrap()]) .args(["clone", &path_or_url, clone_dir_str])
.output()?; .output()?;
if !output.status.success() { if !output.status.success() {
@ -41,21 +44,21 @@ pub fn create_project(
(path_or_url, None) (path_or_url, None)
}; };
let db = state.db.lock().unwrap(); let db = state.db.lock().map_err(|e| AppError::from(format!("Database lock failed: {}", e)))?;
let project = Project::insert(&db, &name, &local_path, cloned_from.as_deref(), &base_branch)?; let project = Project::insert(&db, &name, &local_path, cloned_from.as_deref(), &base_branch)?;
Ok(project) Ok(project)
} }
#[tauri::command] #[tauri::command]
pub fn list_projects(state: State<'_, AppState>) -> Result<Vec<Project>, AppError> { pub fn list_projects(state: State<'_, AppState>) -> Result<Vec<Project>, AppError> {
let db = state.db.lock().unwrap(); let db = state.db.lock().map_err(|e| AppError::from(format!("Database lock failed: {}", e)))?;
let projects = Project::list(&db)?; let projects = Project::list(&db)?;
Ok(projects) Ok(projects)
} }
#[tauri::command] #[tauri::command]
pub fn get_project(state: State<'_, AppState>, id: String) -> Result<Project, AppError> { pub fn get_project(state: State<'_, AppState>, id: String) -> Result<Project, AppError> {
let db = state.db.lock().unwrap(); let db = state.db.lock().map_err(|e| AppError::from(format!("Database lock failed: {}", e)))?;
let project = Project::get_by_id(&db, &id)?; let project = Project::get_by_id(&db, &id)?;
Ok(project) Ok(project)
} }
@ -67,14 +70,14 @@ pub fn update_project(
name: String, name: String,
base_branch: String, base_branch: String,
) -> Result<(), AppError> { ) -> Result<(), AppError> {
let db = state.db.lock().unwrap(); let db = state.db.lock().map_err(|e| AppError::from(format!("Database lock failed: {}", e)))?;
Project::update(&db, &id, &name, &base_branch)?; Project::update(&db, &id, &name, &base_branch)?;
Ok(()) Ok(())
} }
#[tauri::command] #[tauri::command]
pub fn delete_project(state: State<'_, AppState>, id: String) -> Result<(), AppError> { pub fn delete_project(state: State<'_, AppState>, id: String) -> Result<(), AppError> {
let db = state.db.lock().unwrap(); let db = state.db.lock().map_err(|e| AppError::from(format!("Database lock failed: {}", e)))?;
Project::delete(&db, &id)?; Project::delete(&db, &id)?;
Ok(()) Ok(())
} }

View file

@ -32,3 +32,5 @@ impl std::fmt::Display for AppError {
write!(f, "{}", self.message) write!(f, "{}", self.message)
} }
} }
impl std::error::Error for AppError {}

View file

@ -73,15 +73,21 @@ impl Project {
} }
pub fn update(conn: &Connection, id: &str, name: &str, base_branch: &str) -> Result<()> { pub fn update(conn: &Connection, id: &str, name: &str, base_branch: &str) -> Result<()> {
conn.execute( let affected = conn.execute(
"UPDATE projects SET name = ?1, base_branch = ?2 WHERE id = ?3", "UPDATE projects SET name = ?1, base_branch = ?2 WHERE id = ?3",
params![name, base_branch, id], params![name, base_branch, id],
)?; )?;
if affected == 0 {
return Err(rusqlite::Error::QueryReturnedNoRows);
}
Ok(()) Ok(())
} }
pub fn delete(conn: &Connection, id: &str) -> Result<()> { pub fn delete(conn: &Connection, id: &str) -> Result<()> {
conn.execute("DELETE FROM projects WHERE id = ?1", params![id])?; let affected = conn.execute("DELETE FROM projects WHERE id = ?1", params![id])?;
if affected == 0 {
return Err(rusqlite::Error::QueryReturnedNoRows);
}
Ok(()) Ok(())
} }
} }

View file

@ -18,7 +18,7 @@
} }
], ],
"security": { "security": {
"csp": "default-src 'self'; script-src 'self'; connect-src ipc: http://ipc.localhost" "csp": "default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; connect-src ipc: http://ipc.localhost"
} }
}, },
"bundle": { "bundle": {