lashman/driftwood
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

Fix second audit findings and restore crash detection dialog

Browse Source 
Address 29 issues found in comprehensive API/spec audit:
- Fix .desktop Exec key path escaping per Desktop Entry spec
- Fix update dialog double-dispatch with connect_response
- Fix version comparison total ordering with lexicographic fallback
- Use RETURNING id for reliable upsert in database
- Replace tilde-based path fallbacks with proper XDG helpers
- Fix backup create/restore path asymmetry for non-home paths
- HTML-escape severity class in security reports
- Use AppStream <custom> element instead of <metadata>
- Fix has_appimage_update_tool to check .is_ok() not .success()
- Use ListBoxRow instead of ActionRow::set_child in ExpanderRow
- Add ELF magic validation to architecture detection
- Add timeout to extract_update_info_runtime
- Skip symlinks in dir_size calculation
- Use Condvar instead of busy-wait in analysis thread pool
- Restore crash detection to single blocking call architecture
This commit is contained in:
lashman
2026-02-27 22:48:43 +02:00
parent e9343da249
commit 830c3cad9d
21 changed files with 228 additions and 181 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
src/core/analysis.rs
View File

@@ -1,4 +1,5 @@
use std::path::PathBuf;
use std::sync::{Condvar, Mutex};
use std::sync::atomic::{AtomicUsize, Ordering};
use crate::core::database::Database;
@@ -14,17 +15,21 @@ const MAX_CONCURRENT_ANALYSES: usize = 2;
/// Counter for currently running analyses.
static RUNNING_ANALYSES: AtomicUsize = AtomicUsize::new(0);
/// Condvar to efficiently wait for a slot instead of busy-polling.
static SLOT_AVAILABLE: (Mutex<()>, Condvar) = (Mutex::new(()), Condvar::new());
/// Returns the number of currently running background analyses.
pub fn running_count() -> usize {
RUNNING_ANALYSES.load(Ordering::Relaxed)
}
/// RAII guard that decrements the analysis counter on drop.
/// RAII guard that decrements the analysis counter on drop and notifies waiters.
struct AnalysisGuard;
impl Drop for AnalysisGuard {
fn drop(&mut self) {
RUNNING_ANALYSES.fetch_sub(1, Ordering::Release);
SLOT_AVAILABLE.1.notify_one();
}
}
@@ -36,7 +41,7 @@ impl Drop for AnalysisGuard {
///
/// Blocks until a slot is available if the concurrency limit is reached.
pub fn run_background_analysis(id: i64, path: PathBuf, appimage_type: AppImageType, integrate: bool) {
// Wait for a slot to become available
// Wait for a slot to become available using condvar instead of busy-polling
loop {
let current = RUNNING_ANALYSES.load(Ordering::Acquire);
if current < MAX_CONCURRENT_ANALYSES {
@@ -44,7 +49,8 @@ pub fn run_background_analysis(id: i64, path: PathBuf, appimage_type: AppImageTy
break;
}
} else {
std::thread::sleep(std::time::Duration::from_millis(200));
let lock = SLOT_AVAILABLE.0.lock().unwrap();
let _ = SLOT_AVAILABLE.1.wait_timeout(lock, std::time::Duration::from_secs(1));
}
}
let _guard = AnalysisGuard;

19
src/core/appstream.rs
View File

@@ -446,14 +446,14 @@ pub fn generate_catalog(db: &Database) -> Result<String, AppStreamError> {
xml.push_str(" </categories>\n");
}
// Provide hint about source
xml.push_str(" <metadata>\n");
xml.push_str(" <value key=\"managed-by\">driftwood</value>\n");
// Provide hint about source (AppStream spec uses <custom> for key-value data)
xml.push_str(" <custom>\n");
xml.push_str(" <value key=\"Driftwood::managed-by\">driftwood</value>\n");
xml.push_str(&format!(
" <value key=\"appimage-path\">{}</value>\n",
" <value key=\"Driftwood::appimage-path\">{}</value>\n",
xml_escape(&record.path),
));
xml.push_str(" </metadata>\n");
xml.push_str(" </custom>\n");
xml.push_str(" </component>\n");
}
@@ -467,8 +467,7 @@ pub fn generate_catalog(db: &Database) -> Result<String, AppStreamError> {
pub fn install_catalog(db: &Database) -> Result<PathBuf, AppStreamError> {
let catalog_xml = generate_catalog(db)?;
let catalog_dir = dirs::data_dir()
.unwrap_or_else(|| PathBuf::from("~/.local/share"))
let catalog_dir = crate::config::data_dir_fallback()
.join("swcatalog")
.join("xml");
@@ -484,8 +483,7 @@ pub fn install_catalog(db: &Database) -> Result<PathBuf, AppStreamError> {
/// Remove the AppStream catalog from the local swcatalog directory.
pub fn uninstall_catalog() -> Result<(), AppStreamError> {
let catalog_path = dirs::data_dir()
.unwrap_or_else(|| PathBuf::from("~/.local/share"))
let catalog_path = crate::config::data_dir_fallback()
.join("swcatalog")
.join("xml")
.join("driftwood.xml");
@@ -500,8 +498,7 @@ pub fn uninstall_catalog() -> Result<(), AppStreamError> {
/// Check if the AppStream catalog is currently installed.
pub fn is_catalog_installed() -> bool {
let catalog_path = dirs::data_dir()
.unwrap_or_else(|| PathBuf::from("~/.local/share"))
let catalog_path = crate::config::data_dir_fallback()
.join("swcatalog")
.join("xml")
.join("driftwood.xml");

16
src/core/backup.rs
View File

@@ -25,8 +25,7 @@ pub struct BackupPathEntry {
}
fn backups_dir() -> PathBuf {
let dir = dirs::data_dir()
.unwrap_or_else(|| PathBuf::from("~/.local/share"))
let dir = crate::config::data_dir_fallback()
.join("driftwood")
.join("backups");
fs::create_dir_all(&dir).ok();
@@ -123,17 +122,17 @@ pub fn create_backup(db: &Database, appimage_id: i64) -> Result<PathBuf, BackupE
for entry in &entries {
let source = Path::new(&entry.original_path);
if source.exists() {
// Archive all paths relative to home dir for consistent extract layout.
// For non-home paths, archive with full absolute path (leading / stripped by tar).
if let Ok(rel) = source.strip_prefix(&home_dir) {
tar_args.push("-C".to_string());
tar_args.push(home_dir.to_string_lossy().to_string());
tar_args.push(rel.to_string_lossy().to_string());
} else {
tar_args.push("-C".to_string());
tar_args.push("/".to_string());
tar_args.push(
source.parent().unwrap_or(Path::new("/")).to_string_lossy().to_string(),
);
tar_args.push(
source.file_name().unwrap_or_default().to_string_lossy().to_string(),
source.strip_prefix("/").unwrap_or(source).to_string_lossy().to_string(),
);
}
}
@@ -204,8 +203,9 @@ pub fn restore_backup(archive_path: &Path) -> Result<RestoreResult, BackupError>
let extracted = if let Ok(rel) = source.strip_prefix(&home_dir) {
temp_dir.path().join(rel)
} else {
let source_name = source.file_name().unwrap_or_default();
temp_dir.path().join(source_name)
// Non-home paths are archived with full path (leading / stripped)
let abs_rel = source.strip_prefix("/").unwrap_or(source);
temp_dir.path().join(abs_rel)
};
let target = Path::new(&entry.original_path);

17
src/core/database.rs
View File

@@ -198,8 +198,7 @@ pub struct SandboxProfileRecord {
}
fn db_path() -> PathBuf {
let data_dir = dirs::data_dir()
.unwrap_or_else(|| PathBuf::from("~/.local/share"))
let data_dir = crate::config::data_dir_fallback()
.join("driftwood");
std::fs::create_dir_all(&data_dir).ok();
data_dir.join("driftwood.db")
@@ -753,7 +752,7 @@ impl Database {
is_executable: bool,
file_modified: Option<&str>,
) -> SqlResult<i64> {
self.conn.execute(
let id: i64 = self.conn.query_row(
"INSERT INTO appimages (path, filename, appimage_type, size_bytes, is_executable, file_modified)
VALUES (?1, ?2, ?3, ?4, ?5, ?6)
ON CONFLICT(path) DO UPDATE SET
@@ -762,17 +761,9 @@ impl Database {
size_bytes = excluded.size_bytes,
is_executable = excluded.is_executable,
file_modified = excluded.file_modified,
last_scanned = datetime('now')",
last_scanned = datetime('now')
RETURNING id",
params![path, filename, appimage_type, size_bytes, is_executable, file_modified],
)?;
// last_insert_rowid() returns 0 for ON CONFLICT UPDATE, so query the actual id
let id = self.conn.last_insert_rowid();
if id != 0 {
return Ok(id);
}
let id: i64 = self.conn.query_row(
"SELECT id FROM appimages WHERE path = ?1",
params![path],
|row| row.get(0),
)?;
Ok(id)

7
src/core/duplicates.rs
View File

@@ -329,6 +329,8 @@ fn build_name_group(name: &str, records: &[&AppImageRecord]) -> DuplicateGroup {
}
/// Compare two version strings for ordering.
/// Falls back to lexicographic comparison of cleaned versions to guarantee
/// the total ordering contract (antisymmetry) required by sort_by.
fn compare_versions(a: &str, b: &str) -> std::cmp::Ordering {
use super::updater::{clean_version, version_is_newer};
@@ -339,8 +341,11 @@ fn compare_versions(a: &str, b: &str) -> std::cmp::Ordering {
std::cmp::Ordering::Equal
} else if version_is_newer(a, b) {
std::cmp::Ordering::Greater
} else {
} else if version_is_newer(b, a) {
std::cmp::Ordering::Less
} else {
// Neither is newer (unparseable components) - use lexicographic fallback
ca.cmp(&cb)
}
}

14
src/core/footprint.rs
View File

@@ -396,8 +396,14 @@ pub fn dir_size_pub(path: &Path) -> u64 {
}
fn dir_size(path: &Path) -> u64 {
if path.is_file() {
return path.metadata().map(|m| m.len()).unwrap_or(0);
// Use symlink_metadata to avoid following symlinks outside the tree
if let Ok(meta) = path.symlink_metadata() {
if meta.is_file() {
return meta.len();
}
if meta.is_symlink() {
return 0;
}
}
let mut total = 0u64;
if let Ok(entries) = std::fs::read_dir(path) {
@@ -406,6 +412,10 @@ fn dir_size(path: &Path) -> u64 {
Ok(ft) => ft,
Err(_) => continue,
};
// Skip symlinks to avoid counting external files or recursing out of tree
if ft.is_symlink() {
continue;
}
if ft.is_file() {
total += entry.metadata().map(|m| m.len()).unwrap_or(0);
} else if ft.is_dir() {

3
src/core/inspector.rs
View File

@@ -80,8 +80,7 @@ struct DesktopEntryFields {
}
fn icons_cache_dir() -> PathBuf {
let dir = dirs::data_dir()
.unwrap_or_else(|| PathBuf::from("~/.local/share"))
let dir = crate::config::data_dir_fallback()
.join("driftwood")
.join("icons");
fs::create_dir_all(&dir).ok();

51
src/core/integrator.rs
View File

@@ -25,20 +25,34 @@ impl From<std::io::Error> for IntegrationError {
}
}
/// Escape a string for use inside a double-quoted Exec argument in a .desktop file.
/// Per the Desktop Entry spec, `\`, `"`, `` ` ``, and `$` must be escaped with `\`.
fn escape_exec_arg(s: &str) -> String {
let mut out = String::with_capacity(s.len());
for c in s.chars() {
match c {
'\\' | '"' | '`' | '$' => {
out.push('\\');
out.push(c);
}
_ => out.push(c),
}
}
out
}
pub struct IntegrationResult {
pub desktop_file_path: PathBuf,
pub icon_install_path: Option<PathBuf>,
}
fn applications_dir() -> PathBuf {
dirs::data_dir()
.unwrap_or_else(|| PathBuf::from("~/.local/share"))
crate::config::data_dir_fallback()
.join("applications")
}
fn icons_dir() -> PathBuf {
dirs::data_dir()
.unwrap_or_else(|| PathBuf::from("~/.local/share"))
crate::config::data_dir_fallback()
.join("icons/hicolor")
}
@@ -90,21 +104,22 @@ pub fn integrate(record: &AppImageRecord) -> Result<IntegrationResult, Integrati
let icon_id = format!("driftwood-{}", app_id);
let desktop_content = format!(
"[Desktop Entry]\n\
Type=Application\n\
Name={name}\n\
Exec=\"{exec}\" %U\n\
Icon={icon}\n\
Categories={categories}\n\
Comment={comment}\n\
Terminal=false\n\
X-AppImage-Path={path}\n\
X-AppImage-Version={version}\n\
X-AppImage-Managed-By=Driftwood\n\
X-AppImage-Integrated-Date={date}\n",
let desktop_content = format!("\
[Desktop Entry]
Type=Application
Name={name}
Exec=\"{exec}\" %U
Icon={icon}
Categories={categories}
Comment={comment}
Terminal=false
X-AppImage-Path={path}
X-AppImage-Version={version}
X-AppImage-Managed-By=Driftwood
X-AppImage-Integrated-Date={date}
",
name = app_name,
exec = record.path,
exec = escape_exec_arg(&record.path),
icon = icon_id,
categories = categories,
comment = comment,

78
src/core/launcher.rs
View File

@@ -58,18 +58,18 @@ impl LaunchMethod {
/// Result of a launch attempt.
#[derive(Debug)]
pub enum LaunchResult {
/// Successfully spawned the process and it's still running.
/// Process spawned and survived the startup crash-check window.
Started {
child: Child,
pid: u32,
method: LaunchMethod,
},
/// Process spawned but crashed immediately (within ~1 second).
/// Process spawned but exited during the startup crash-check window.
Crashed {
exit_code: Option<i32>,
stderr: String,
method: LaunchMethod,
},
/// Failed to launch.
/// Failed to launch (binary not found, permission denied, etc.).
Failed(String),
}
@@ -183,34 +183,21 @@ fn execute_appimage(
match cmd.spawn() {
Ok(mut child) => {
// Give the process a brief moment to fail on immediate errors
// (missing libs, exec format errors, Qt plugin failures, etc.)
std::thread::sleep(std::time::Duration::from_millis(150));
match child.try_wait() {
Ok(Some(status)) => {
// Already exited - immediate crash. Read stderr for details.
let stderr_text = child.stderr.take().map(|mut pipe| {
let mut buf = String::new();
use std::io::Read;
// Read with a size cap to avoid huge allocations
let mut limited = (&mut pipe).take(64 * 1024);
let _ = limited.read_to_string(&mut buf);
buf
}).unwrap_or_default();
let pid = child.id();
// Monitor for early crash (2s window). This blocks the current
// thread, so callers should run this inside gio::spawn_blocking.
match check_early_crash(&mut child, std::time::Duration::from_secs(2)) {
Some((exit_code, stderr)) => {
LaunchResult::Crashed {
exit_code: status.code(),
stderr: stderr_text,
exit_code,
stderr,
method: method.clone(),
}
}
_ => {
// Still running after 150ms - drop the stderr pipe so the
// child process won't block if it fills the pipe buffer.
drop(child.stderr.take());
None => {
LaunchResult::Started {
child,
pid,
method: method.clone(),
}
}
@@ -220,7 +207,44 @@ fn execute_appimage(
}
}
/// Parse a launch_args string from the database into a Vec of individual arguments.
/// Check if a recently-launched child process crashed during startup.
/// Waits up to `timeout` for the process to exit. If it exits within that window,
/// reads stderr and returns a Crashed result. If still running, drops the stderr
/// pipe (to prevent pipe buffer deadlock) and returns None.
///
/// Call this from a background thread after spawning the process.
pub fn check_early_crash(
child: &mut Child,
timeout: std::time::Duration,
) -> Option<(Option<i32>, String)> {
let start = std::time::Instant::now();
loop {
match child.try_wait() {
Ok(Some(status)) => {
// Process exited - read stderr for crash details
let stderr_text = child.stderr.take().map(|mut pipe| {
let mut buf = String::new();
use std::io::Read;
let mut limited = (&mut pipe).take(64 * 1024);
let _ = limited.read_to_string(&mut buf);
buf
}).unwrap_or_default();
return Some((status.code(), stderr_text));
}
Ok(None) => {
if start.elapsed() >= timeout {
// Still running - drop stderr pipe to avoid deadlock
drop(child.stderr.take());
return None;
}
std::thread::sleep(std::time::Duration::from_millis(50));
}
Err(_) => return None,
}
}
}
/// Parse launch arguments with basic quote support.
/// Splits on whitespace, respecting double-quoted strings.
/// Returns an empty Vec if the input is None or empty.

6
src/core/orphan.rs
View File

@@ -14,14 +14,12 @@ pub struct CleanupSummary {
}
fn applications_dir() -> PathBuf {
dirs::data_dir()
.unwrap_or_else(|| PathBuf::from("~/.local/share"))
crate::config::data_dir_fallback()
.join("applications")
}
fn icons_dir() -> PathBuf {
dirs::data_dir()
.unwrap_or_else(|| PathBuf::from("~/.local/share"))
crate::config::data_dir_fallback()
.join("icons/hicolor")
}

2
src/core/report.rs
View File

@@ -229,7 +229,7 @@ pub fn render_html(report: &SecurityReport) -> String {
html.push_str("<table>\n<tr><th>CVE</th><th>Severity</th><th>CVSS</th><th>Library</th><th>Fixed In</th><th>Summary</th></tr>\n");
for f in &app.findings {
let sev_class = f.severity.to_lowercase();
let sev_class = html_escape(&f.severity.to_lowercase());
html.push_str(&format!(
"<tr><td>{}</td><td class=\"{}\">{}</td><td>{}</td><td>{} {}</td><td>{}</td><td>{}</td></tr>\n",
html_escape(&f.cve_id),

3
src/core/sandbox.rs
View File

@@ -44,8 +44,7 @@ impl ProfileSource {
/// Directory where local sandbox profiles are stored.
fn profiles_dir() -> PathBuf {
let dir = dirs::config_dir()
.unwrap_or_else(|| PathBuf::from("~/.config"))
let dir = crate::config::config_dir_fallback()
.join("driftwood")
.join("sandbox");
fs::create_dir_all(&dir).ok();

15
src/core/updater.rs
View File

@@ -762,8 +762,12 @@ pub fn version_is_newer(latest: &str, current: &str) -> bool {
}
}
// If all compared parts are equal, longer version wins (1.2.3 > 1.2)
latest_parts.len() > current_parts.len()
// If all compared parts are equal, only consider newer if extra parts are non-zero
// (e.g., 1.2.1 > 1.2, but 1.2.0 == 1.2)
if latest_parts.len() > current_parts.len() {
return latest_parts[current_parts.len()..].iter().any(|&p| p > 0);
}
false
}
/// Parse a version string into numeric parts.
@@ -781,13 +785,13 @@ fn parse_version_parts(version: &str) -> Vec<u64> {
/// Check if AppImageUpdate tool is available on the system.
pub fn has_appimage_update_tool() -> bool {
// Check that the binary exists and can be spawned (--help may return non-zero)
std::process::Command::new("AppImageUpdate")
.arg("--help")
.stdout(std::process::Stdio::null())
.stderr(std::process::Stdio::null())
.status()
.map(|s| s.success())
.unwrap_or(false)
.is_ok()
}
/// Batch check: read update info from an AppImage and check for updates.
@@ -935,7 +939,8 @@ pub fn download_and_apply_update(
// Atomic rename temp -> target
if let Err(e) = fs::rename(&temp_path, appimage_path) {
// Try to restore backup on failure
// Clean up temp file and restore backup on failure
fs::remove_file(&temp_path).ok();
if let Some(ref backup) = backup_path {
fs::rename(backup, appimage_path).ok();
}