Fix 29 audit findings across all severity tiers
Critical: fix unsquashfs arg order, quote Exec paths with spaces, fix compare_versions antisymmetry, chunk-based signature detection, bounded ELF header reads. High: handle NULL CVE severity, prevent pipe deadlock in inspector, fix glob_match edge case, fix backup archive path collisions, async crash detection with stderr capture. Medium: gate scan on auto-scan setting, fix window size persistence, fix announce() for Stack containers, claim lightbox gesture, use serde_json for CLI output, remove dead CSS @media blocks, add detail-tab persistence, remove invalid metainfo categories, byte-level fuse signature search. Low: tighten Wayland env var detection, ELF magic validation, timeout for update info extraction, quoted arg parsing, stop watcher timer on window destroy, GSettings choices/range constraints, remove unused CSS classes, define status-ok/status-attention CSS.
This commit is contained in:
107
docs/plans/2026-02-27-audit-fixes-design.md
Normal file
107
docs/plans/2026-02-27-audit-fixes-design.md
Normal file
@@ -0,0 +1,107 @@
|
||||
# Audit Fixes Design
|
||||
|
||||
## Goal
|
||||
|
||||
Fix all 29 findings from the full codebase audit, organized by severity tier with build verification between tiers.
|
||||
|
||||
## Approach
|
||||
|
||||
Fix by severity tier (Critical -> High -> Medium -> Low). Run `cargo build` after each tier to catch regressions early.
|
||||
|
||||
## Tier 1: Critical (5 items)
|
||||
|
||||
### #1 - security.rs: Fix unsquashfs argument order
|
||||
`detect_version_from_binary` passes `appimage_path` after the extract pattern. unsquashfs expects the archive before patterns. Move `appimage_path` before the file pattern, remove the `-e` flag.
|
||||
|
||||
### #2 - integrator.rs: Quote Exec path in .desktop files
|
||||
`Exec={exec} %U` breaks for paths with spaces. Change to `Exec="{exec}" %U`.
|
||||
|
||||
### #3 - duplicates.rs: Fix compare_versions total order
|
||||
`compare_versions("1.0", "v1.0")` returns `Less` both ways (violates antisymmetry). Use `clean_version()` on both inputs for the equality check.
|
||||
|
||||
### #4 - inspector.rs: Chunk-based signature detection
|
||||
`detect_signature` reads entire files (1.5GB+) into memory. Replace with `BufReader` reading 64KB chunks, scanning each for the signature bytes.
|
||||
|
||||
### #5 - updater.rs: Read only first 12 bytes in verify_appimage
|
||||
Replace `fs::read(path)` with `File::open` + `read_exact` for just the ELF/AI magic bytes.
|
||||
|
||||
## Tier 2: High (6 items)
|
||||
|
||||
### #6 - database.rs: Handle NULL severity in CVE summaries
|
||||
`get_cve_summary` and `get_all_cve_summary` fail on NULL severity. Change to `Option<String>`, default `None` to `"MEDIUM"`.
|
||||
|
||||
### #7 - inspector.rs: Fix deadlock in extract_metadata_files
|
||||
Piped stderr + `.status()` can deadlock. Change to `Stdio::null()` since we don't use stderr.
|
||||
|
||||
### #8 - updater.rs: Fix glob_match edge case
|
||||
After matching the last part with `ends_with`, reduce the search text before checking middle parts.
|
||||
|
||||
### #9 - backup.rs: Prevent archive filename collisions
|
||||
Use relative paths from home directory instead of bare filenames, so two dirs with the same leaf name don't collide.
|
||||
|
||||
### #10 - launcher.rs: Async crash detection
|
||||
Remove the 1.5s blocking sleep from `execute_appimage`. Return `Started` immediately with the `Child`. Callers (already async) handle crash detection by polling the child after a delay.
|
||||
|
||||
### #11 - launcher.rs: Drop stderr pipe on success
|
||||
After returning `Started`, either drop `child.stderr` or use `Stdio::null()` for stderr to prevent pipe buffer deadlock on long-running apps.
|
||||
|
||||
## Tier 3: Medium (9 items)
|
||||
|
||||
### #12 - window.rs: Gate scan on auto-scan-on-startup
|
||||
Wrap `self.trigger_scan()` in `if self.settings().boolean("auto-scan-on-startup")`.
|
||||
|
||||
### #13 - window.rs: Fix window size persistence
|
||||
Change `self.default_size()` to `(self.width(), self.height())`.
|
||||
|
||||
### #14 - widgets.rs: Fix announce() for any container
|
||||
Change `announce()` to not require a `gtk::Box` - use a more generic approach or fix callers to pass the correct widget type.
|
||||
|
||||
### #15 - detail_view.rs: Claim gesture in lightbox
|
||||
Add `gesture.set_state(gtk::EventSequenceState::Claimed)` in the picture click handler.
|
||||
|
||||
### #16 - cli.rs: Use serde_json for JSON output
|
||||
Replace hand-crafted `format!` JSON with `serde_json::json!()`.
|
||||
|
||||
### #17 - style.css: Remove dead @media blocks
|
||||
Delete `@media (prefers-color-scheme: dark)` and `@media (prefers-contrast: more)` blocks. libadwaita named colors already adapt.
|
||||
|
||||
### #18 - gschema.xml + detail_view.rs: Wire detail-tab persistence
|
||||
Save active tab on switch, restore on open.
|
||||
|
||||
### #19 - metainfo.xml: Remove invalid categories
|
||||
Delete `<categories>` block (already in .desktop file, invalid in metainfo per AppStream spec).
|
||||
|
||||
### #20 - fuse.rs: Byte-level search
|
||||
Replace `String::from_utf8_lossy().to_lowercase()` with direct byte-level case-insensitive search using `windows()`.
|
||||
|
||||
## Tier 4: Low (9 items)
|
||||
|
||||
### #21 - wayland.rs: Tighten env var detection
|
||||
Remove `WAYLAND_DISPLAY` from fallback heuristic. Keep only `GDK_BACKEND` and `QT_QPA_PLATFORM`.
|
||||
|
||||
### #22 - inspector.rs: Add ELF magic validation
|
||||
Check `\x7fELF` magic and endianness byte before parsing `e_machine`.
|
||||
|
||||
### #23 - updater.rs: Add timeout to extract_update_info_runtime
|
||||
Add 5-second timeout to prevent indefinite blocking.
|
||||
|
||||
### #24 - launcher.rs: Handle quoted args
|
||||
Use a shell-like tokenizer that respects double-quoted strings in `parse_launch_args`.
|
||||
|
||||
### #25 - (merged with #20)
|
||||
|
||||
### #26 - window.rs: Stop watcher timer on window destroy
|
||||
Return `glib::ControlFlow::Break` when `window_weak.upgrade()` returns `None`.
|
||||
|
||||
### #27 - gschema.xml: Add choices/range constraints
|
||||
Add `<choices>` to enumerated string keys, `<range>` to backup-retention-days.
|
||||
|
||||
### #28 - style.css: Remove unused CSS classes
|
||||
Delete `.quick-action-pill`, `.badge-row`, `.detail-view-switcher`, base `.letter-icon`.
|
||||
|
||||
### #29 - style.css/app_card.rs: Fix status-ok/status-attention
|
||||
Define CSS rules for these classes or remove the class additions from code.
|
||||
|
||||
## Verification
|
||||
|
||||
After each tier: `cargo build` with zero errors and zero warnings. After all tiers: manual app launch test.
|
||||
1155
docs/plans/2026-02-27-audit-fixes-implementation.md
Normal file
1155
docs/plans/2026-02-27-audit-fixes-implementation.md
Normal file
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user