From b826803e1506f7bff01e0c4e1f4689d974fd0373 Mon Sep 17 00:00:00 2001 From: Leszek Date: Mon, 29 Jun 2026 21:50:29 +0200 Subject: [PATCH] fix: resolve correctness, security, and resource bugs across subsystems Fixes surfaced by a multi-agent audit + bug-hunt pass, each verified against the real code. Build clean, clippy zero-warning, all tests pass. Archives: - tar.zst creation finalizes the zstd frame (auto_finish); previously every created archive was silently truncated/corrupt - tar/zip/7z extraction re-verifies each entry's parent (drop the parent-dir cache that let a symlink swap escape the destination) - zip extraction fails loudly instead of silently truncating at 100k - zip creation enforces the same entry-count limit as tar - tar.xz decompression is size-capped (decompression-bomb guard) and no longer leaks its temp file on a reopen error File ops: - cross-device overwrite move no longer destroys the destination on a post-copy cancel (a completed copy is the point of no return) - files/symlinks get the critical-directory guard in the cross-device move fallback and the top-level symlink delete path - rename refuses to clobber a dangling symlink (symlink_metadata, not try_exists) - batch delete keeps hardlinked siblings (dedup on path, not inode) - single-owner temp cleanup removes the double-unlink + misleading log Jobs / event loop: - RunningJob::shutdown drains the bounded channel while joining, so a worker blocked on send() can no longer deadlock teardown - watcher debounced events flush every loop tick; event::poll errors shut the job down symmetrically with event::read - drop the always-overwritten final-progress dialog fs / watcher: - UID/GID name cache releases the global lock across NSS lookups - inotify rename pairing no longer steals an unrelated From on an unmatched cookie - deleted-symlink watcher state is reclaimed via the path cache Input / UI / viewer: - mouse double-click works again (Up no longer clears last_click) - page_down uses saturating_add; visual-offset cache guards overflow - hex-search highlight spans the 8-byte group gap correctly - input dialogs clear stale status on success; dialog toggle dedup'd Misc: - panel history uses VecDeque (O(1) eviction) - show_hidden config default consistent for missing table vs field - MTIME_TOLERANCE widened for FAT32 boundary rounding - PIPE_JOIN_TIMEOUT matched to CHAFA_TIMEOUT - CompareMode::ALL completeness test; tar.zst roundtrip + rename guard regression tests --- .github/ISSUE_TEMPLATE/bug_report.md | 41 ++ .github/ISSUE_TEMPLATE/config.yml | 8 + .github/ISSUE_TEMPLATE/feature_request.md | 23 + .github/PULL_REQUEST_TEMPLATE.md | 34 ++ .gitignore | 7 + CONTRIBUTING.md | 208 +++++++ Cargo.toml | 2 +- README.md | 693 +++++++++++++--------- assets/README.md | 38 ++ src/app/config.rs | 19 +- src/app/job_runner.rs | 68 ++- src/app/types/modes.rs | 23 + src/app/types/panel.rs | 23 +- src/app/user_menu.rs | 20 +- src/fs/reader.rs | 104 ++-- src/fs/watcher.rs | 24 +- src/input/dialogs.rs | 39 +- src/input/mouse.rs | 17 +- src/input/normal.rs | 9 +- src/main.rs | 31 +- src/ops/archive/mod.rs | 50 ++ src/ops/archive/sevenz.rs | 11 +- src/ops/archive/tar.rs | 64 +- src/ops/archive/zip.rs | 48 +- src/ops/batch.rs | 82 +-- src/ops/chunk_copy.rs | 23 +- src/ops/compare.rs | 8 +- src/ops/file_ops/delete.rs | 83 +++ src/ops/file_ops/entry_ops.rs | 58 +- src/ops/file_ops/move_ops.rs | 34 +- src/ui/viewer/loader.rs | 8 +- src/ui/viewer/search.rs | 7 +- src/ui/viewer/toggle.rs | 17 +- 33 files changed, 1430 insertions(+), 494 deletions(-) create mode 100644 .github/ISSUE_TEMPLATE/bug_report.md create mode 100644 .github/ISSUE_TEMPLATE/config.yml create mode 100644 .github/ISSUE_TEMPLATE/feature_request.md create mode 100644 .github/PULL_REQUEST_TEMPLATE.md create mode 100644 CONTRIBUTING.md create mode 100644 assets/README.md diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 0000000..484f91f --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,41 @@ +--- +name: Bug report +about: Report something that's broken or behaves incorrectly +title: "bug: " +labels: bug +--- + +## Summary + + + +## Steps to reproduce + +1. +2. +3. + +## Expected behavior + + + +## Actual behavior + + + +## Environment + +- `lc` version: +- OS: +- Terminal: +- Rust version (if built from source): + +## Config & context + + + +``` +COLORTERM=? +``` diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 0000000..f4544a7 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,8 @@ +blank_issues_enabled: false +contact_links: + - name: πŸ“– Documentation + url: https://github.com/leszek3737/LibreCommander#readme + about: Install, keybindings, config, and FAQ β€” check the README first. + - name: πŸ” Search existing issues + url: https://github.com/leszek3737/LibreCommander/issues?q=is%3Aissue + about: Your question may already be answered in an open or closed issue. diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md new file mode 100644 index 0000000..5220e46 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -0,0 +1,23 @@ +--- +name: Feature request +about: Suggest a new feature or enhancement +title: "feat: " +labels: enhancement +--- + +## Problem + + + +## Proposed solution + + + +## Alternatives considered + + + +## Additional context + + diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 0000000..7385fc5 --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,34 @@ + + +## Summary + + + +Closes # + +## Type of change + +- [ ] Bug fix (non-breaking) +- [ ] New feature (non-breaking) +- [ ] Refactor / cleanup +- [ ] Docs +- [ ] Breaking change + +## Checklist + +- [ ] Linked the issue this closes (if applicable) +- [ ] `cargo fmt` +- [ ] `cargo clippy --locked --all-targets -- -D warnings` β€” zero warnings +- [ ] `cargo test --locked` β€” green +- [ ] `cargo build --release --locked` β€” succeeds +- [ ] Added/updated tests for the change +- [ ] No `unsafe`, no `println!`/`eprintln!`/`dbg!` in committed code +- [ ] Commit messages follow Conventional Commits, no attribution trailers + +## Notes for reviewers + + diff --git a/.gitignore b/.gitignore index 2defbd3..f2aab71 100755 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,10 @@ yazi .claude/worktrees/ docs/pr-plan/ +.claude-flow/ +.claude/ +.mcp.json +.swarm/ +ruvector.db + +.wayland/ diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..c4d50d2 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,208 @@ +# Contributing to Libre Commander + +Thanks for your interest in improving `lc`! This guide covers everything you need +to build, test, and submit changes. + +> **Working with an AI assistant?** [`AGENTS.md`](AGENTS.md) contains the +> machine-oriented instructions (Serena/GitNexus usage, hard rules, gotchas). +> Read that first β€” humans and agents follow the same rules. + +--- + +## Quick Start + +```bash +git clone https://github.com/leszek3737/LibreCommander.git +cd LibreCommander +cargo run # dev build, optimized for fast incremental rebuilds +cargo test --locked # full test suite +``` + +Requirements: **Rust 1.95+** (edition 2024), `cargo`. + +Before proposing a feature, please [open an issue](https://github.com/leszek3737/LibreCommander/issues) +to discuss scope β€” especially for anything beyond a bug fix. + +--- + +## The Quality Gate + +CI runs these on every push and PR (`.github/workflows/rust.yml`, Linux + macOS +matrix). They must be green before merge β€” run them locally first: + +```bash +cargo fmt +cargo clippy --locked --all-targets -- -D warnings +cargo test --locked +cargo build --release --locked +``` + +| Command | Purpose | +|---------|---------| +| `cargo fmt` | Format the code | +| `cargo clippy --locked --all-targets -- -D warnings` | Zero lint warnings | +| `cargo test --locked` | All tests pass | +| `cargo build --release --locked` | Release build succeeds | + +> `--locked` is intentional: contributions must build against the committed +> `Cargo.lock`, not a freshly resolved dependency set. + +--- + +## Hard Rules + +These are non-negotiable β€” they protect the TUI and your data: + +- **`unsafe_code = "forbid"`** β€” no `unsafe`, anywhere. No exceptions. +- **No `println!` / `eprintln!` / `dbg!`** in committed code β€” they corrupt the + TUI display and are denied by clippy. Use the `app::debug_log!` macro instead. +- **Never mutate state from `ui::*` draw code** β€” rendering must be a pure + function of `AppState`. Only `input::*` handlers mutate state. +- **Never block the event thread** β€” any work over ~50 ms goes to `rayon` or the + `app::job_runner`, never inline. +- **Never introduce `tokio`/async** β€” the project is intentionally synchronous. + Use `rayon` for parallelism, `mpsc` channels for progress. +- **Destructive ops need explicit confirmation** β€” delete/move/overwrite must + confirm unless already confirmed in the current flow. +- **Symlinks are data** β€” don't follow them during chmod/copy/delete unless the + operation explicitly requires it. +- **Cross-device moves** use copy+delete fallback with cancellation and no-clobber. +- **Archive extraction** validates paths (zip-slip), sets size limits, handles + symlinks safely. +- **No network calls** β€” `lc` is offline by design. + +--- + +## Architecture (at a glance) + +``` + β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” + crossterm ──► β”‚ event loop β”‚ (main.rs) blocking read + poll(33ms) + β””β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”˜ + β”‚ events + β–Ό + β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” + β”‚ input/* β”‚ ──mut──► β”‚ AppState β”‚ single source of truth (~36 fields) + β”‚ handlers β”‚ β””β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”˜ + β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ β”‚ pure read + β–² β–Ό + β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” + β”‚ ops/* β”‚ ◄─jobs─ β”‚ ui/* β”‚ pure render, never mutates + β”‚ (rayon) β”‚ β”‚ ratatui β”‚ + β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ +``` + +- **Sync event loop + rayon offloading** β€” no async runtime. +- **One `AppState` struct** holds all UI-relevant data β†’ enables pure rendering. +- **Ratatui** immediate-mode + crossterm backend + double-buffered diff. + +### Repository map + +| Directory | Responsibility | Notes | +|-----------|---------------|-------| +| `src/main.rs` | Event loop, dispatch, `TerminalGuard` | RAII terminal cleanup on panic | +| `src/render.rs` | Render orchestration | | +| `src/render_dialog_map.rs` | Dialog render dispatch | by `DialogKind` | +| `src/input/` | Key/mouse handling β€” **mutates state** | `normal.rs`, `dialogs.rs`, `mode_dispatch.rs` | +| `src/app/` | State types, config, keymaps, job runner, watcher sync | `types/app_state.rs` | +| `src/ops/` | File operations β€” copy, move, delete, search, archive, sort | MUST be cancellable | +| `src/ui/` | Pure rendering β€” **never mutates state** | `panels/`, `dialogs/`, `viewer/` | +| `src/fs/` | Directory reads (rayon), `notify` watcher, path helpers | | +| `src/tests/` | Integration tests | `AppState` harness | +| `src/menu.rs` | `F9` menu bar definitions | | + +For deeper detail (event-loop internals, ADRs), read `src/main.rs` and +`AGENTS.md`. + +--- + +## Code Conventions + +We follow standard Rust style; only the **non-default** rules are listed: + +- **Functions over ~100 lines** trip `too_many_lines`. Split along natural seams; + propose the split in your PR first. +- Prefer `?` and `let ... else` over `.unwrap()`/`.expect()`. + `#[allow(...)]` only on `mod tests` blocks. +- Use `unicode-width` for column math β€” `len() != display width` for CJK/emoji + filenames. +- Prefer `std::io::Result`; avoid `anyhow`. +- `#[allow(...)]` to suppress lints is allowed **only** for: + `unwrap_used`/`expect_used`/`panic` on tests, `print_stdout` when the TUI is + suspended, `non_snake_case` for external tokens. +- ANSI escape sequences in strings are **not** rendered β€” use the `ansi-to-tui` crate. +- The `notify` watcher backend differs on macOS (`cfg(target_os)`) β€” test + path/permission logic for both platforms. + +### File-size policy + +800 lines is a **checkpoint**, not a hard limit. Evaluate whether a split along +natural seams exists; propose it before splitting. Cohesive files (single state +machine, one impl block) stay even if large. Never split by line count alone. + +--- + +## Testing + +- **Unit tests:** inline `#[cfg(test)] mod tests` in the same file; directory + modules have `tests.rs` siblings. +- **Integration tests:** `src/tests/` β€” use the `AppState` harness. +- **File ops:** always use `tempfile::TempDir`; cover symlinks, cross-device, + and Unicode filenames. +- **UI rendering:** `Terminal::new(TestBackend::new(w, h))` + assert on the + buffer. See `ui/viewer/tests.rs`. +- **Async/watcher patterns:** `EventHarness` from `fs/watcher/tests.rs`. + +```bash +cargo test --locked # everything +cargo test -- --nocapture # single test with output +``` + +--- + +## Gotchas + +- The event loop uses blocking `crossterm::event::read()` + `event::poll(33ms)` + (`EVENT_POLL_TIMEOUT_MS` in `main.rs`). Don't change the timeout without + understanding the spinner tick (200 ms). +- `TerminalGuard` provides RAII cleanup on panic β€” don't bypass it in error paths. +- When spawning an external process: you **must** `LeaveAlternateScreen` + + `disable_raw_mode` first, and reclaim after. (Vim queries terminal + capabilities via ANSI that crossterm reads as keyboard events.) +- Adding a new dialog touches **4 places**: `DialogKind` variant (`modes.rs`), + detail struct (`types/dialogs.rs`), input handler (`input/dialogs.rs`), and + render (`render_dialog_map.rs` + `ui/dialogs/`). +- The main loop calls `sync_watcher_job_state` before `sync_watcher_paths`, and + `pre_draw()` before `terminal.draw()` β€” check this before modifying the loop. +- Config migration requires user approval β€” users hand-edit `config.toml`. + +--- + +## Commits + +We use [Conventional Commits](https://www.conventionalcommits.org/): + +``` +feat(archive): add zstd write support +fix(viewer): correct horizontal scroll on wide lines +refactor(input): split mode dispatch +docs(readme): add screenshot +``` + +- Don't bump the `Cargo.toml` version unless asked. +- Keep commit messages clean and self-contained β€” no co-author trailers, + attribution lines, or "generated with …" notes. +- Each logical change is its own commit; never amend a commit that has been + pushed. + +--- + +## Filing issues & pull requests + +- **Bugs:** use the bug-report template; include OS, terminal, Rust version, and + minimal reproduction steps. +- **Features:** open an issue to discuss before implementing large changes. +- **Pull requests:** use the PR template, ensure the quality gate is green, and + link the issue you're closing (`Closes #123`). + +Happy hacking! πŸ¦€ diff --git a/Cargo.toml b/Cargo.toml index 374b35f..46c3e06 100755 --- a/Cargo.toml +++ b/Cargo.toml @@ -6,7 +6,7 @@ rust-version = "1.95" description = "A fast Rust terminal file manager inspired by Midnight Commander" license = "MIT" readme = "README.md" -repository = "https://github.com/leszek3737/lc---Libre-Commander" +repository = "https://github.com/leszek3737/LibreCommander" [dependencies] bitflags = "2" diff --git a/README.md b/README.md index 4892c50..94caab0 100755 --- a/README.md +++ b/README.md @@ -1,91 +1,195 @@ -# Libre Commander (lc) +
-A fast, Rust-based file manager inspired by Midnight Commander. +# πŸ¦€ Libre Commander + +**A fast, keyboard-driven, dual-panel terminal file manager β€” inspired by Midnight Commander.** + +[![CI](https://github.com/leszek3737/LibreCommander/actions/workflows/rust.yml/badge.svg)](https://github.com/leszek3737/LibreCommander/actions/workflows/rust.yml) +[![License: MIT](https://img.shields.io/badge/license-MIT-blue.svg?style=flat-square)](LICENSE) +[![Rust](https://img.shields.io/badge/Rust-1.95%2B-orange?logo=rust&style=flat-square)](https://www.rust-lang.org/) +[![Edition](https://img.shields.io/badge/edition-2024-yellow?style=flat-square)](https://doc.rust-lang.org/edition-guide/) +[![Platform](https://img.shields.io/badge/platform-Linux%20%7C%20macOS-lightgrey?style=flat-square)](#supported-platforms) +[![Built with Ratatui](https://img.shields.io/badge/built%20with-ratatui-red?style=flat-square)](https://ratatui.rs/) + +
+ +Libre Commander (`lc`) brings the classic two-pane file-manager workflow to a +modern, async-free Rust core. Copy, move, browse archives, preview images, and +search files β€” all without leaving the keyboard. One static binary, fully +offline, no runtime dependencies. + +> πŸ’¬ `lc` is short for **L**ibre **C**ommander. + + + +``` + Left ~/lc Right ~/projects/LibreCommander +β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β” +β”‚Permissions Size Modified Name β”‚Permissions Size Modified Name β”‚ +│────────────────────────────────── │────────────────────────────────── β”‚ +β”‚drwxr-xr-x 06-26 09:10 src β–Ί β”‚-rw-r--r-- 1.8K 06-14 23:18 Cargo β”‚ +β”‚drwxr-xr-x 06-26 09:10 target β”‚-rw-r--r-- 73K 06-15 00:52 Cargo β”‚ +β”‚πŸ“ app/ 12 files β”‚πŸ“„ README.md 17 KiB β”‚ +β”‚πŸ“ ops/ 8 files β”‚πŸ“œ LICENSE 1.0 KiB β”‚ +β”‚πŸ“ ui/ 9 filesβ”‚βš™ .gitignore β”‚ +β”‚πŸ¦€ main.rs 14 KiB β”‚πŸ“¦ lc 3.2 MiB release β”‚ +β”‚πŸ¦€ render.rs 4 KiB β”‚πŸ–Ό logo.png 128 KiB β”‚ +β”‚πŸ“œ AGENTS.md 9.4 KiB β”‚πŸ“œ CHANGELOG.md β€” β”‚ +β”‚... β”‚... β”‚ +β”œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”΄β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€ +β”‚1Help 2Menu 3View 4Edit 5Copy 6Move 7Mkdir 8Delete 9Menu 10Quit β”‚ +β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜ +``` + +--- + +## Table of Contents + +- [Features](#features) +- [Quick Start](#quick-start) + - [Install](#install) + - [First Run](#first-run) + - [30-Second Cheatsheet](#30-second-cheatsheet) +- [Keyboard Reference](#keyboard-reference) +- [Configuration](#configuration) +- [Archives](#archives) +- [File Viewer & Image Preview](#file-viewer--image-preview) +- [Search & Filter](#search--filter) +- [Sorting](#sorting) +- [Directory Compare](#directory-compare) +- [User Menu](#user-menu) +- [FAQ & Troubleshooting](#faq--troubleshooting) +- [Supported Platforms](#supported-platforms) +- [Contributing](#contributing) +- [Acknowledgments](#acknowledgments) +- [License](#license) + +--- ## Features -- **Dual-panel interface** - Navigate and manage files in two panels side-by-side -- **Async file operations** - Copy, move, delete, rename, chmod files and directories with background progress and cancellation -- **Safe recursive copy/move/delete** - Handles directories with symlink preservation, no-clobber copy, cross-device fallback, partial-copy cleanup, and cancellation safeguards -- **Archive support** - Browse, extract, and create archives (ZIP, TAR/GZ/BZ2/XZ/ZST, 7Z) -- **Advanced search** - Incremental panel filter, recursive file search (glob patterns), content search (grep-like) -- **File viewer** - Built-in text viewer with search, hex dump, line numbers, and word wrap -- **Directory tree** - Interactive expandable directory tree view -- **Directory compare** - Compare panels by name, size, or modification time (3 modes) -- **Directory hotlist** - Bookmark directories for quick access via Alt+1 through Alt+9 -- **Directory history** - Navigate back with Alt+Backspace -- **User menu** - Extensible menu system via `.mc.menu` or `~/.config/lc/menu` (MC-compatible) -- **Sorting** - 12 sort modes: by name (standard & natural), size, modification time, creation time, or extension (ascending/descending) -- **File watcher** - Automatic panel refresh on external filesystem changes while preserving filters, sorting, and selection -- **Panel views** - Long (detailed) and Brief (compact) listing modes -- **File type icons** - Emoji icons and color coding for archives, images, source code, audio, video, config files -- **Mouse support** - Single click to select, double click to open/view, click to switch panels -- **Keyboard-driven** - Rich keyboard shortcut set for power users -- **Configurable** - Customizable settings stored in `~/.config/lc/config.toml` -- **System protection** - Refuses to delete critical system directories (`/`, `/etc`, `/usr`, etc.) - -## Build Instructions - -### Prerequisites - -- Rust 1.95+ (edition 2024 support required) -- Cargo - -### Building from Source +**Workflow** +- **Dual-panel interface** β€” navigate and manage files in two panels side-by-side +- **Directory tree** β€” interactive expandable tree view +- **Directory compare** β€” diff panels by name, size, or modification time (3 modes) +- **Directory hotlist** β€” bookmark directories via `Alt+1` … `Alt+9` +- **Directory history** β€” jump back with `Alt+Backspace` +- **Quick cd** β€” jump to any path with `Alt+C` +- **Panel views** β€” Long (detailed) and Brief (compact) listing modes +- **Mouse support** β€” click to select, double-click to open, drag to select ranges + +**File operations** +- **Async, cancellable jobs** β€” copy, move, delete, rename, `chmod` with live byte/item progress +- **Safe recursive ops** β€” symlink preservation, no-clobber copy, cross-device fallback, partial-copy cleanup +- **System protection** β€” refuses to delete critical directories (`/`, `/etc`, `/usr`, …) +- **External editor** β€” `F4` opens files in `$EDITOR` + +**Archives** β€” browse, extract, and create (see [Archives](#archives)) +- Read **and** write: `zip`, `tar`, `tar.gz`, `tar.bz2`, `tar.xz`, `tar.zst` +- Read-only: `7z` +- Zip-slip protection, size limits, symlink-safe extraction + +**Search & view** +- **Incremental filter** β€” type to filter the panel in real time (glob patterns) +- **Recursive file search** β€” glob-pattern find with auto-navigation to first match +- **Content search** β€” grep-like, line-by-line +- **Built-in viewer** β€” text + hex dump + image preview, in-file search, word wrap, line numbers + +**Polish** +- **File-type icons & colors** β€” emoji/ASCII/Nerd-Font themes for archives, images, code, audio, video, config +- **File watcher** β€” auto-refresh on external changes, preserving filters, sort, and selection +- **12 sort modes** β€” name, natural, size, mtime, btime, extension (asc/desc) +- **MC-compatible user menu** β€” `.mc.menu` / `~/.config/lc/menu` with conditionals and substitution tokens + +
+Why lc over ranger / yazi / mc? + +| | Libre Commander | Midnight Commander | ranger / yazi | +|---|---|---|---| +| Panel model | Dual-panel (MC-style) | Dual-panel | Single-pane + preview | +| Runtime | Sync, no async runtime | C | ranger: Python Β· yazi: async Tokio | +| Binary | One static binary | system pkg | ranger: script Β· yazi: binary | +| Network | Offline by design | β€” | yazi: fetches previews | +| `unsafe` | `forbid` crate-wide | β€” | β€” | +| Config | TOML | INI | Python / TOML | + +`lc` is for people who want the **MC dual-panel muscle memory**, in a **single +deterministic Rust binary** that runs offline and refuses to do anything unsafe. +
+ +--- + +## Quick Start + +### Install + +**Option A β€” one command (any OS, from git):** ```bash -cd ~/git/LibreCommander -cargo build --release +cargo install --git https://github.com/leszek3737/LibreCommander ``` -The binary will be located at `target/release/lc`. +Puts the `lc` binary on your `PATH` (`~/.cargo/bin`). Requires Rust 1.95+. -### Running +**Option B β€” build from source:** ```bash -./target/release/lc +git clone https://github.com/leszek3737/LibreCommander.git +cd LibreCommander +cargo build --release # binary: target/release/lc +# or install it on your PATH: +cargo install --path . ``` -Or install system-wide: +> Prebuilt binaries and Homebrew/apt packages are not published yet β€” track +> [issues](https://github.com/leszek3737/LibreCommander/issues) if you'd like to +> help package `lc` for your distro. + +### First Run ```bash -cargo install --path . +lc ``` -### Dependencies - -| Crate | Purpose | -|-------|---------| -| `ratatui` 0.30 | Terminal UI framework | -| `crossterm` 0.29 | Cross-platform terminal I/O | -| `serde` 1.0 | Config serialization | -| `toml` 1 | Config file parsing | -| `chrono` 0.4 | Date/time formatting | -| `regex` 1.0 | User menu condition matching | -| `unicode-width` 0.2 | Unicode character width for alignment | -| `users` 0.11 | File owner/group lookup | -| `notify` 8 | Filesystem watcher for auto-refresh (platform-conditional: macOS uses `macos_fsevent`) | -| `bitflags` 2 | Bitflag types | -| `dirs` 6 | XDG/user directories | -| `rayon` 1 | Parallel iteration / background jobs | -| `infer` 0.19 | MIME type detection | -| `filetime` 0.2 | File modification time handling | -| `ansi-to-tui` 8 | Parse ANSI sequences for image viewing | -| `zip` 8.6 | ZIP archive support (bzip2, zstd, lzma, deflate64) | -| `tar` 0.4 | TAR archive support | -| `flate2` 1.1 | Gzip compression | -| `zstd` 0.13 | Zstandard compression | -| `ruzstd` 0.8 | Zstandard decompression | -| `sevenz-rust` 0.6 | 7z archive support | -| `bzip2` 0.6 | Bzip2 compression | -| `lzma-rs` 0.3 | LZMA/XZ compression | -| `memchr` 2 | Fast byte scanning for content detection/search | -| `shlex` 2 | Shell-style splitting/quoting | -| `unicode-segmentation` 1 | Unicode grapheme-aware text editing | - -Dev dependency: `tempfile` 3 (for tests). - -## Keyboard Shortcuts +That's it β€” `lc` opens with both panels and a default config. There are no CLI +flags to learn; everything is driven from the keyboard and `~/.config/lc/config.toml`. + +**Optional β€” enable image preview:** `lc` renders images as character art via +[`chafa`](https://hpjansson.org/chafa/), which is **not** bundled: + +```bash +# macOS +brew install chafa +# Debian / Ubuntu +sudo apt install chafa +# Fedora +sudo dnf install chafa +# Arch +sudo pacman -S chafa +``` + +If `chafa` is missing, opening an image in the viewer shows +`Failed to execute chafa (is it installed?)` β€” install it to enable preview. + +### 30-Second Cheatsheet + +| Key | Action | | Key | Action | +|-----|--------|-|-----|--------| +| `Tab` | Switch panel | | `F3` | View / preview | +| `h j k l` / arrows | Move | | `F4` | Edit (`$EDITOR`) | +| `Enter` | Open dir / archive | | `F5` | Copy | +| `Alt+Backspace` | History back | | `F6` | Move | +| `Ctrl+H` | Toggle hidden | | `F7` | Mkdir / extract | +| `Alt+C` | Quick cd | | `F8` | Delete | +| `Alt+1`…`Alt+9` | Hotlist | | `F10` / `q` | Quit | + +Press **`F1`** any time for the in-app help dialog. + +--- + +## Keyboard Reference ### General @@ -104,7 +208,7 @@ Dev dependency: `tempfile` 3 (for tests). | `Tab` | Switch between panels | | `↑` / `k` | Move up | | `↓` / `j` | Move down | -| `Enter` | Open directory / Preview archive | +| `Enter` | Open directory / preview archive | | `Alt+Backspace` | Go to previous directory (history) | | `Home` | Go to first entry | | `End` | Go to last entry | @@ -116,11 +220,11 @@ Dev dependency: `tempfile` 3 (for tests). | Key | Action | |-----|--------| -| `F3` | View file / Preview archive contents | +| `F3` | View file / preview archive contents | | `F4` | Edit file (opens in `$EDITOR`) | | `F5` | Copy file(s) | | `F6` | Move file(s) | -| `F7` | Create directory / Extract archive | +| `F7` | Create directory / extract archive | | `F8` | Delete file(s) | | `F11` | Rename file or directory | | `F12` | Archive operations menu | @@ -131,7 +235,7 @@ Dev dependency: `tempfile` 3 (for tests). | `Ctrl+R` | Refresh current panel | | `Ctrl+O` | External viewer (temporarily exit to shell) | -Additional file actions are available from the F9 menu: File > Rename and File > Chmod. +Additional actions are available from the `F9` menu: **File β†’ Rename** and **File β†’ Chmod**. ### Search & Filter @@ -153,9 +257,9 @@ Additional file actions are available from the F9 menu: File > Rename and File > | Key | Action | |-----|--------| -| `Alt+1` through `Alt+9` | Jump to directory hotlist slot 1-9 | -| `Mouse Click` | Select file / Switch panel | -| `Mouse Double-Click` | Open directory / View file | +| `Alt+1` … `Alt+9` | Jump to directory hotlist slot 1–9 | +| `Mouse click` | Select file / switch panel | +| `Mouse double-click` | Open directory / view file | ### File Viewer Mode @@ -180,11 +284,11 @@ Additional file actions are available from the F9 menu: File > Rename and File > | `Esc` | Exit tree | | `↑` / `↓` / `Home` / `End` / `PageUp` / `PageDown` | Navigate | | `Enter` | Expand/collapse directory or view file | -| `c` | cd to selected directory | +| `c` | `cd` to selected directory | ### Command Line Mode -Enter command line mode with `Alt+X` or Menu: Command > Command line. +Enter with `Alt+X` or **Menu β†’ Command β†’ Command line**. | Key | Action | |-----|--------| @@ -198,7 +302,7 @@ Enter command line mode with `Alt+X` or Menu: Command > Command line. | `Ctrl+U` | Delete to line start | | `Ctrl+C` | Cancel command line | -### Menu Bar (F9) +### Menu Bar (`F9`) | Key | Action | |-----|--------| @@ -217,8 +321,6 @@ Enter command line mode with `Alt+X` or Menu: Command > Command line. | `a` | Add to hotlist (hotlist picker only) | | `d` | Delete from hotlist (hotlist picker only) | -Note: `a` (add) and `d` (delete) work only in the Hotlist picker. - ### Mouse | Action | Effect | @@ -227,307 +329,334 @@ Note: `a` (add) and `d` (delete) work only in the Hotlist picker. | Left double-click | Open directory or view file | | Left click on panel | Switch active panel | | Left drag in panel | Select range of entries | -| Middle click | Copy (F5 equivalent) | -| Right click | Cancel / close (Esc equivalent) | +| Middle click | Copy (`F5` equivalent) | +| Right click | Cancel / close (`Esc` equivalent) | | Scroll | Scroll panel cursor | -| Click function bar (bottom) | F1-F10 actions | +| Click function bar (bottom) | `F1`–`F10` actions | -## Configuration +--- -Configuration file location: `~/.config/lc/config.toml` +## Configuration -### Config Schema +Config location: **`~/.config/lc/config.toml`** ```toml -active_panel = "left" # "left" or "right" -dir_first = true # directories before files in sort -sort_sensitive = false # case-sensitive name sorting +active_panel = "left" # "left" or "right" +dir_first = true # directories before files in sort +sort_sensitive = false # case-sensitive name sorting [left] -path = "/home/user" -show_hidden = true +path = "/home/user" +show_hidden = true show_permissions = false -listing_mode = "long" # "long" or "brief" -sort_mode = "name_asc" # see sort modes below -filter = "" # glob pattern, empty = no filter +listing_mode = "long" # "long" or "brief" +sort_mode = "name_asc" # see Sort Modes below +filter = "" # glob pattern, empty = no filter [right] -path = "/home/user/projects" -show_hidden = true +path = "/home/user/projects" +show_hidden = true show_permissions = false -listing_mode = "long" -sort_mode = "name_asc" -filter = "" +listing_mode = "long" +sort_mode = "name_asc" +filter = "" hotlist = ["/home/user", "/home/user/projects"] ``` ### Sort Modes -`name_asc`, `name_desc`, `natural_name_asc`, `natural_name_desc`, `size_asc`, `size_desc`, `mod_time_asc`, `mod_time_desc`, `btime_asc`, `btime_desc`, `extension_asc`, `extension_desc` +`name_asc`, `name_desc`, `natural_name_asc`, `natural_name_desc`, `size_asc`, +`size_desc`, `mod_time_asc`, `mod_time_desc`, `btime_asc`, `btime_desc`, +`extension_asc`, `extension_desc` + +### Theming -An optional `[theme]` section is supported for color customization; all fields have defaults. +An optional `[theme]` section customizes colors. All fields have defaults. ```toml [theme] -icon_theme = "emoji" # "emoji", "ascii", or "nerd_font" -panel_bg = "navy" -panel_fg = "white" +icon_theme = "emoji" # "emoji", "ascii", or "nerd_font" +panel_bg = "navy" +panel_fg = "white" highlight_bg = "cyan" highlight_fg = "black" -directory = "white" -executable = "green" -symlink = "cyan" -archive = "red" -image = "magenta" -video = "light_magenta" -audio = "light_green" -source_code = "yellow" -config = "light_blue" +directory = "white" +executable = "green" +symlink = "cyan" +archive = "red" +image = "magenta" +video = "light_magenta" +audio = "light_green" +source_code = "yellow" +config = "light_blue" regular_file = "white" ``` -Color values accept named colors (`red`, `light_blue`, `navy`), hex colors (`#RRGGBB` or `#RGB`), or 0-255 ANSI color indexes. +Color values accept **named colors** (`red`, `light_blue`, `navy`), **hex** +(`#RRGGBB` or `#RGB`), or **0–255 ANSI** indexes. ### Environment Variables | Variable | Purpose | Default | |----------|---------|---------| -| `EDITOR` | External editor for F4 | `vi` | +| `EDITOR` | External editor for `F4` | `vi` | | `HOME` | Config/menu file location | (required) | | `XDG_CONFIG_HOME` | Config/menu file base directory | `$HOME/.config` | -## User Menu +--- -Create custom menu entries in: -- Local: `.mc.menu` in the active panel's directory -- Global: `~/.config/lc/menu` +## Archives -### Menu Format +`lc` browses, extracts, and creates archives. -``` -# Comment line +| Format | Extension | Read | Write | +|--------|-----------|:----:|:-----:| +| ZIP | `.zip` | βœ… | βœ… | +| TAR | `.tar` | βœ… | βœ… | +| TAR+Gzip | `.tar.gz`, `.tgz` | βœ… | βœ… | +| TAR+Bzip2 | `.tar.bz2`, `.tbz`, `.tbz2` | βœ… | βœ… | +| TAR+XZ | `.tar.xz`, `.txz` | βœ… | βœ… | +| TAR+Zstd | `.tar.zst`, `.tzst` | βœ… | βœ… | +| 7z | `.7z` | βœ… | ❌ | -+ f \.rs$ -T Run Rust tests - cargo test %f +| Key | Action | +|-----|--------| +| `Enter` on archive | Preview archive contents | +| `F3` on archive | Preview archive contents | +| `F7` on archive | Extract archive | +| `F12` | Archive menu (extract / create) | +| `F12` with selected files | Create archive from selection | -+ f \.py$ -R Run Python script - python3 %f +The extract dialog lists archive contents and asks for the destination; the +create dialog picks the archive name and format (`zip`, `tar.gz`, `tar.xz`, +`tar`). All archive operations run in the background with progress and +cancellation. -A Archive selected files - tar czf archive.tgz %t +**Safety:** extraction validates paths against [zip-slip](https://snyk.io/research/zip-slip-vulnerability), +enforces size limits, and handles symlinks safely. -D Diff panels - diff -rq %d %D -``` +--- -### Entry Structure +## File Viewer & Image Preview -- **Hotkey**: First character of the line (single char) -- **Title**: Rest of the hotkey line (display label) -- **Body**: Indented lines (tab or space) as shell commands -- **Condition**: `+ f ` β€” only show entry when filename matches regex; multiple condition lines are OR'd together. Conditions can appear before or after the hotkey line. +The built-in viewer (`F3`) supports: -### Substitution Tokens +- **Text mode** with word wrap (`w`) +- **Line numbers** (`l`) +- **Hex dump** (`h`) β€” standard hex+offset, 16 bytes per line +- **Image preview** β€” auto-detected via MIME; rendered as character art through `chafa` +- **In-file search** (`/` to search, `n` / `N` to navigate) +- **Horizontal scrolling** for wide lines +- **Unicode** β€” lossy UTF-8 display for binary files +- **Auto content detection** β€” MIME-based with null-byte fallback -| Token | Expands to | -|-------|------------| -| `%f` | Current filename (shell-quoted) | -| `%d` | Active panel directory (shell-quoted) | -| `%D` | Other panel directory (shell-quoted) | -| `%t` / `%s` | Tagged/selected files (space-separated, shell-quoted); `%s` is an alias for `%t` | -| `%%` | Literal `%` | +Limits: files up to 100 MiB (larger are truncated). -Commands are executed via `sh -c` with the active panel's directory as working directory. +### Image preview in detail -Menu files are limited to 1 MiB. +On first view (and on terminal resize), `lc` spawns `chafa --size WxH ` +and parses its ANSI output into terminal cells via `ansi-to-tui`. The result is +cached β€” subsequent frames only clone the cached buffer, keeping rendering at +full speed. Preview size adapts to the terminal area. -## File Operations +--- -Long-running copy, move, and delete operations run as background jobs with live item and byte progress. Operations can be canceled between safe boundaries; move operations finish cleanup after a successful cross-device copy so source and destination do not diverge unexpectedly. +## Search & Filter -Safety guarantees: +**Incremental filter** β€” type any character in normal mode to filter the panel +in real time. Supports glob patterns (`*`, `?`), case-insensitive. -- Existing destinations are not overwritten by chunked copies. -- Copy/move conflicts show an overwrite confirmation before replacing existing entries. -- Recursive directory copies publish through a temporary sibling and clean up partial output on failure or cancellation. -- Symlinks are copied or deleted as symlinks rather than following their targets. -- Cross-device moves fall back to copy-then-delete only after the copy succeeds. -- Critical system directories are protected from deletion. +**Find file** β€” **Menu β†’ Command β†’ Find file**. Recursive glob search from the +active panel's directory; the first match is navigated to automatically. -## File Viewer +**Content search** β€” grep-like, line-by-line, case-insensitive. Limits: files +over 10 MiB skipped, lines over 64 KiB skipped, max 1000 results, max depth 20, +max 10 000 items scanned. *(Not yet wired to a UI action β€” see +[issue tracker](https://github.com/leszek3737/LibreCommander/issues) for progress.)* -The built-in viewer (F3) supports: +--- -- **Text mode** with word wrap (toggle with `w`) -- **Line numbers** (toggle with `l`) -- **Hex dump** (toggle with `h`) β€” standard hex+offset format, 16 bytes per line -- **Image preview** β€” automatic image preview rendering in character art using `chafa` (toggle with `h` to hex mode) -- **In-file search** (`/` to search, `n`/`N` to navigate matches) -- **Horizontal scrolling** for wide lines -- **Unicode support** β€” lossy UTF-8 display for binary files -- **Size limit** β€” files up to 100 MiB (larger files are truncated) -- **Content detection** β€” auto-detection of text vs binary content (MIME-based with null-byte fallback) +## Sorting -## Image Preview +Twelve sort modes, cycled via **Left/Right β†’ Sort order**: + +| Mode | Key | Order | +|------|-----|-------| +| Name ↑ | `name_asc` | A–Z | +| Name ↓ | `name_desc` | Z–A | +| Nat ↑ | `natural_name_asc` | A–Z (digit-aware) | +| Nat ↓ | `natural_name_desc` | Z–A (digit-aware) | +| Size ↑ | `size_asc` | Smallest first | +| Size ↓ | `size_desc` | Largest first | +| Time ↑ | `mod_time_asc` | Oldest first | +| Time ↓ | `mod_time_desc` | Newest first | +| Created ↑ | `btime_asc` | Oldest first | +| Created ↓ | `btime_desc` | Newest first | +| Ext ↑ | `extension_asc` | A–Z | +| Ext ↓ | `extension_desc` | Z–A | + +Rules: `..` always first, directories before files, case-insensitive. These +defaults are configurable via `dir_first` and `sort_sensitive` in `config.toml`. +Natural sort compares multi-digit runs numerically (`file9` < `file10`). + +--- -lc renders images as character art (ANSI TrueColor) using **chafa**. Open any -image file with `F3` β€” the viewer auto-detects image MIME types and switches to -Image mode. +## Directory Compare -### Requirements +**Command menu β†’ Compare dirs.** Three modes: -```bash -# macOS -brew install chafa +| Mode | Matching criteria | +|------|-------------------| +| Quick | Filename + entry type | +| Size | Filename + size (dirs: name + type only) | +| Thorough | Filename + size + modification time (dirs: name + type only) | -# Debian / Ubuntu -sudo apt install chafa +Differing and unique entries are auto-selected in both panels. -# Fedora -sudo dnf install chafa +--- -# Arch -sudo pacman -S chafa -``` +## User Menu -chafa is not bundled with lc. If missing, the viewer shows -"Failed to execute chafa (is it installed?)". +Create custom menu entries in: +- **Local:** `.mc.menu` in the active panel's directory +- **Global:** `~/.config/lc/menu` -### Controls in Image mode +### Format -| Key | Action | -|-----|--------| -| `h` | Toggle between image preview and hex dump | -| `Up` / `Down` / `k` / `j` | No-op (image fills available area) | -| `Esc` / `F3` / `F10` / `q` | Close viewer | +``` +# Comment line -### How it works ++ f \.rs$ +T Run Rust tests + cargo test %f -- On first view or terminal resize, lc spawns `chafa --size WxH ` and - parses its ANSI output into terminal characters via `ansi-to-tui`. -- The result is cached β€” subsequent frames only clone the cached `Text`, - keeping **60 FPS** rendering. -- Preview size adapts to the terminal area, leaving one line for the status bar. ++ f \.py$ +R Run Python script + python3 %f -## Archive Support +A Archive selected files + tar czf archive.tgz %t -lc supports browsing, extracting, and creating archives. Supported formats: +D Diff panels + diff -rq %d %D +``` -| Format | Extension | Read | Write | -|--------|-----------|------|-------| -| ZIP | `.zip` | Yes | Yes | -| TAR | `.tar` | Yes | Yes | -| TAR+Gzip | `.tar.gz`, `.tgz` | Yes | Yes | -| TAR+Bzip2 | `.tar.bz2`, `.tbz`, `.tbz2` | Yes | Yes | -| TAR+XZ | `.tar.xz`, `.txz` | Yes | Yes | -| TAR+Zstd | `.tar.zst`, `.tzst` | Yes | Yes | -| 7z | `.7z` | Yes | No | +- **Hotkey:** first character of the line (single char) +- **Title:** rest of the hotkey line (display label) +- **Body:** indented lines (tab or space) as shell commands +- **Condition:** `+ f ` β€” only show the entry when the filename matches; + multiple condition lines are OR'd; may appear before or after the hotkey line -### Archive Operations +### Substitution Tokens -| Key | Action | -|-----|--------| -| `Enter` on archive | Preview archive contents in viewer | -| `F3` on archive | Preview archive contents in viewer | -| `F7` on archive | Extract archive | -| `F12` | Archive menu (extract/create) | -| `F12` with selected files | Create archive from selection | +| Token | Expands to | +|-------|------------| +| `%f` | Current filename (shell-quoted) | +| `%d` | Active panel directory (shell-quoted) | +| `%D` | Other panel directory (shell-quoted) | +| `%t` / `%s` | Tagged/selected files (space-separated, shell-quoted); `%s` is an alias for `%t` | +| `%%` | Literal `%` | -Extract dialog shows archive contents and lets you specify the destination directory. Create dialog lets you choose the archive name and format (zip, tar.gz, tar.xz, tar). +Commands run via `sh -c` with the active panel's directory as the working +directory. Menu files are limited to 1 MiB. -All archive operations run in the background with progress reporting and cancellation support. +--- -## Search +## File Operation Safety -### Incremental Search (Panel Filter) +Long-running copy, move, and delete operations run as background jobs with live +item and byte progress, and can be canceled between safe boundaries. -Type any character in normal mode to start filtering. The panel updates in real-time. Supports glob patterns (`*`, `?`). Case-insensitive. +- Existing destinations are **not** overwritten by chunked copies. +- Copy/move conflicts show an overwrite confirmation before replacing. +- Recursive directory copies publish through a temporary sibling and clean up + partial output on failure or cancellation. +- Symlinks are copied or deleted **as symlinks**, never by following their target. +- Cross-device moves fall back to copy-then-delete **only after** the copy succeeds. +- Critical system directories are protected from deletion. +- Terminal state is always restored β€” even on panic. -### File Search (Find File) +--- -Menu: Command > Find file. Recursive glob-pattern search from the active panel's directory. First match is navigated to automatically. +## FAQ & Troubleshooting -### Content Search +**Image preview shows "Failed to execute chafa".** +Install [`chafa`](https://hpjansson.org/chafa/) (see [First Run](#first-run)). +It is not bundled with `lc`. -Available programmatically via `FileSearch::search_content()`. Searches file contents line-by-line. Case-insensitive. Content search limits: files over 10 MiB skipped, lines over 64 KiB skipped, max 1000 results, max depth 20, max 10000 items scanned. Not yet wired to a UI action. +**Colors look wrong / no truecolor.** +Set `COLORTERM=truecolor` in your shell. Most modern terminals enable it by +default; legacy 8-color terminals will look flat. -## Sorting +**Icons render as boxes / question marks.** +Your font lacks the glyphs. Use the `ascii` icon theme (no font dependency) or +install a [Nerd Font](https://www.nerdfonts.com/) and set `icon_theme = "nerd_font"`. -Twelve sort modes, cycled via menu (Left/Right > Sort order): +**Where is my config?** +`~/.config/lc/config.toml` (or `$XDG_CONFIG_HOME/lc/config.toml`). Edit it +directly β€” `lc` does not migrate config without your approval. -| Mode | Key | Order | -|------|-----|-------| -| Name ↑ | name_asc | A-Z | -| Name ↓ | name_desc | Z-A | -| Nat ↑ | natural_name_asc | A-Z (digit-aware) | -| Nat ↓ | natural_name_desc | Z-A (digit-aware) | -| Size ↑ | size_asc | Smallest first | -| Size ↓ | size_desc | Largest first | -| Time ↑ | mod_time_asc | Oldest first | -| Time ↓ | mod_time_desc | Newest first | -| Created ↑ | btime_asc | Oldest first | -| Created ↓ | btime_desc | Newest first | -| Ext ↑ | extension_asc | A-Z | -| Ext ↓ | extension_desc | Z-A | - -Rules: `..` always first, directories before files, case-insensitive. These defaults are configurable via `dir_first` and `sort_sensitive` in `config.toml`. Natural sort compares multi-digit runs numerically (e.g. `file9` < `file10`). +**Does `lc` support Windows?** +Not yet. The CI matrix covers **Linux and macOS**. The file watcher uses a +platform-specific `notify` backend (macOS: `macos_fsevent`). -## Directory Compare +**Can I make `lc` my default file manager?** +`lc` is a terminal application, not a desktop/GUI file manager. It is meant to +be launched from a terminal, not wired into `xdg-open`. -Command menu > Compare dirs. Three modes: +**It crashed / did something wrong.** +Please open an issue with the steps to reproduce, your OS, terminal, and Rust +version β€” see [Contributing](#contributing). -| Mode | Matching criteria | -|------|-------------------| -| Quick | Filename + entry type | -| Size | Filename + size (dirs: name + type only) | -| Thorough | Filename + size + modification time (dirs: name + type only) | +--- -Differing and unique entries are auto-selected in both panels. +## Supported Platforms -## Testing +| OS | Status | Notes | +|----|:------:|-------| +| Linux | βœ… CI-tested | Primary target | +| macOS | βœ… CI-tested | Watcher uses `macos_fsevent` backend | +| Windows | ❌ Not yet | Help wanted β€” see issues | +| BSDs | ⚠️ Untested | May work; please report | -Run the test suite: +Requires **Rust 1.95+** (edition 2024). -```bash -cargo test --locked -``` +--- -The test suite covers: -- File operations (copy, move, delete, rename, chmod) -- Search (incremental, glob, content, symlink safety) -- Sorting (all 12 modes, edge cases) -- UI rendering (colors, icons, formatting, truncation) -- Config persistence (roundtrip serialization) -- User menu parsing and substitution -- Directory tree building and toggling -- Viewer (scroll, search, hex mode, Unicode) -- Batch operations (copy/move/delete with progress and cancellation) -- File watcher events and debouncing +## Contributing -## Quality Gates +Contributions are welcome! Please read **[CONTRIBUTING.md](CONTRIBUTING.md)** +for build steps, testing, code style, and the quality gate that must pass before +merge. -Run these checks before submitting changes: +The quick gate (run before declaring done): ```bash -cargo fmt --check +cargo fmt cargo clippy --locked --all-targets -- -D warnings cargo test --locked cargo build --release --locked ``` -File operations include safety guards: system directories are protected from deletion, symlinks are handled correctly during copy/move/delete, and terminal state is always restored (even on panic). - -## License +See the [open issues](https://github.com/leszek3737/LibreCommander/issues) for +ideas, and please open an issue before starting large changes. -MIT License +--- ## Acknowledgments -Libre Commander is inspired by: -- [Midnight Commander](https://midnight-commander.org/) - The original dual-panel file manager -- [Yazi](https://github.com/sxyazi/yazi) - Some code components were adapted from this project by [Sxyazi](https://github.com/sxyazi) (MIT License) -- [Rust](https://www.rust-lang.org/) - The programming language -- [Ratatui](https://ratatui.rs/) - Terminal UI library +Libre Commander stands on the shoulders of giants: + +- **[Midnight Commander](https://midnight-commander.org/)** β€” the original dual-panel file manager that defined the workflow +- **[Yazi](https://github.com/sxyazi/yazi)** β€” some code components adapted by [Sxyazi](https://github.com/sxyazi) (MIT) +- **[Rust](https://www.rust-lang.org/)** β€” the language +- **[Ratatui](https://ratatui.rs/)** β€” the terminal UI library +- **[Crossterm](https://github.com/crossterm-rs/crossterm)** β€” cross-platform terminal I/O +- **[chafa](https://hpjansson.org/chafa/)** β€” image-to-character-art rendering + +--- + +## License + +[MIT](LICENSE) Β© 2026 Leszek3737 diff --git a/assets/README.md b/assets/README.md new file mode 100644 index 0000000..43bb0d1 --- /dev/null +++ b/assets/README.md @@ -0,0 +1,38 @@ +# Assets + +This directory holds images, GIFs, and other media referenced by the top-level +[`README.md`](../README.md). + +## Adding a real screenshot / demo + +The README currently ships an ASCII mockup of the dual-panel UI so it renders +everywhere with no binary assets. To upgrade it to a real capture: + +1. **Record a demo.** A short GIF or asciinema cast works best for a TUI: + + ```bash + # Option A: animated GIF via asciinema + agg + asciinema rec lc-demo.cast --command "./target/release/lc" + agg lc-demo.cast assets/lc-demo.gif # https://github.com/asciinema/agg + + # Option B: static screenshot of a representative panel + # (use your terminal's built-in screenshot, or `teiler`, `grim`, etc.) + ``` + +2. **Save the file here**, e.g. `assets/lc-demo.gif` or `assets/lc-demo.png`. + +3. **Reference it in the README.** Replace the ASCII mockup block with: + + ```html +

+ Libre Commander in action +

+ ``` + +4. Keep the file **small** (≀ ~2 MB) so the README stays fast. Prefer a cropped, + representative shot over a long recording. + +## Logo + +`assets/logo.png` (optional) β€” a project logo for the README hero and social +preview. If added, reference it in the `
` hero block. diff --git a/src/app/config.rs b/src/app/config.rs index 12c1807..5d2b465 100755 --- a/src/app/config.rs +++ b/src/app/config.rs @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize}; use crate::app::paths; use crate::app::types::{ActivePanel, AppState, ListingMode, PanelState, SortMode, SortOptions}; -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct PersistedPanel { #[serde(default)] pub path: Option, @@ -24,6 +24,23 @@ pub struct PersistedPanel { pub show_permissions: bool, } +// Hand-written so the `Default` (used when a whole `[left]`/`[right]` table is +// absent) agrees with the per-field `#[serde(default = "default_true")]` and the +// runtime `PanelState` default. A derived `Default` would make `show_hidden` +// false for a missing table but true for a present table with the field omitted. +impl Default for PersistedPanel { + fn default() -> Self { + Self { + path: None, + listing_mode: ListingMode::default(), + sort_mode: SortMode::default(), + filter: String::new(), + show_hidden: true, + show_permissions: false, + } + } +} + fn default_true() -> bool { true } diff --git a/src/app/job_runner.rs b/src/app/job_runner.rs index fc85e21..97d15a9 100644 --- a/src/app/job_runner.rs +++ b/src/app/job_runner.rs @@ -63,10 +63,28 @@ pub struct RunningJob { impl RunningJob { pub fn shutdown(&mut self) { self.cancel.store(true, Ordering::Relaxed); - if let Some(handle) = self.handle.take() - && let Err(e) = handle.join() - { - debug_log!("worker thread panicked during shutdown: {:?}", e); + if let Some(handle) = self.handle.take() { + // Drain the channel we still own while waiting, bounded by the same + // deadline as the Drop reaper. The progress channel is a bounded + // `sync_channel`, so a worker blocked in `send()` on a full channel + // would never reach its next `cancel` check; an unbounded `join()` + // here would then deadlock teardown (we wait for the worker, the + // worker waits for a free slot). Draining lets it proceed and observe + // `cancel`. + let start = std::time::Instant::now(); + while !handle.is_finished() && start.elapsed() < REAPER_JOIN_DEADLINE { + while self.receiver.try_recv().is_ok() {} + std::thread::sleep(REAPER_POLL_INTERVAL); + } + if handle.is_finished() { + if let Err(e) = handle.join() { + debug_log!("worker thread panicked during shutdown: {:?}", e); + } + } else { + debug_log!( + "worker thread did not finish within deadline during shutdown β€” detaching" + ); + } } } } @@ -223,19 +241,13 @@ pub fn poll_running_job( latest_progress = Some(progress); } JobMessage::Finished { report } => { - // Show the final progress snapshot and prevent the - // second pass (lines below) from formatting the same - // progress message again. - if let Some(progress) = latest_progress.take() { - let msg = - format_progress_message(&progress, job.cancel.load(Ordering::Relaxed)); - state.mode = AppMode::Dialog(DialogKind::progress( - msg, - progress.byte_percent() / 100.0, - true, - )); - dirty = true; - } + // `finish_running_job` (run unconditionally below when `finished` + // is set) switches the mode to `AppMode::Normal`, and the caller + // only renders after this function returns β€” so any progress + // dialog built here would be overwritten before it could be + // displayed. Just drop the pending progress so the second pass + // below also skips the now-pointless formatting. + latest_progress = None; finished = Some(report); } JobMessage::SearchFinished { outcome, pattern } => { @@ -244,10 +256,10 @@ pub fn poll_running_job( } } - // When `Finished` was the last message, `latest_progress` was already - // formatted inside the `Finished` arm above (via `take()`). This block - // handles the remaining case where Progress arrived but Finished did not - // (or arrived in a later poll cycle). + // Handles the case where Progress arrived but Finished did not (or arrives in + // a later poll cycle). When `Finished` was seen this cycle, the arm above + // cleared `latest_progress`, because `finish_running_job` owns the final mode + // and would overwrite any dialog built here before the next render. if let Some(progress) = latest_progress { let msg = format_progress_message(&progress, job.cancel.load(Ordering::Relaxed)); state.mode = AppMode::Dialog(DialogKind::progress( @@ -258,6 +270,12 @@ pub fn poll_running_job( dirty = true; } + // Handle `Finished` and `SearchFinished` as independent `if`s rather than an + // `if/else if` chain: a worker sends exactly one of them today, but should the + // protocol ever emit both in a single poll cycle, the second would otherwise + // be silently dropped. The `handled_final` flag keeps the no-report fallback + // below mutually exclusive with both terminal paths. + let mut handled_final = false; if let Some(report) = finished { if let Some(mut job) = running_job.take() && let Some(handle) = job.handle.take() @@ -266,7 +284,9 @@ pub fn poll_running_job( } finish_running_job(state, &report, refresh_both); dirty = true; - } else if let Some((outcome, pattern)) = finished_search { + handled_final = true; + } + if let Some((outcome, pattern)) = finished_search { // Capture `search_origin` in a local before `take()` consumes the job. let search_origin = running_job.as_ref().and_then(|j| j.search_origin); if let Some(mut job) = running_job.take() @@ -276,7 +296,9 @@ pub fn poll_running_job( } finish_search_job(state, &outcome, &pattern, search_origin, refresh_both); dirty = true; - } else if let Some(job) = running_job.as_mut() { + handled_final = true; + } + if !handled_final && let Some(job) = running_job.as_mut() { let worker_finished = job.handle.as_ref().is_some_and(JoinHandle::is_finished); if worker_finished && let Some(handle) = job.handle.take() { let exited_cleanly = diff --git a/src/app/types/modes.rs b/src/app/types/modes.rs index 31d2029..dc2f4fc 100644 --- a/src/app/types/modes.rs +++ b/src/app/types/modes.rs @@ -119,3 +119,26 @@ impl PendingAction { } } } + +#[cfg(test)] +mod tests { + use super::*; + + // Guards the hand-maintained `CompareMode::ALL` against drift. The inner + // `match` is exhaustive, so adding a new `CompareMode` variant breaks + // compilation here until the author updates both the match and `ALL`; the + // length assertion then catches a variant that was added to the enum but + // forgotten in the array (or vice versa). + #[test] + fn compare_mode_all_is_exhaustive_and_in_order() { + for (i, variant) in CompareMode::ALL.iter().enumerate() { + let expected = match variant { + CompareMode::Quick => 0, + CompareMode::Size => 1, + CompareMode::Thorough => 2, + }; + assert_eq!(i, expected, "{variant:?} is out of position in ALL"); + } + assert_eq!(CompareMode::ALL.len(), 3); + } +} diff --git a/src/app/types/panel.rs b/src/app/types/panel.rs index 79758bc..91672f8 100644 --- a/src/app/types/panel.rs +++ b/src/app/types/panel.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{HashMap, VecDeque}; use std::path::{Path, PathBuf}; use super::file_entry::FileEntry; @@ -203,8 +203,10 @@ impl PanelListing { /// Remove the entry at `path`, returning whether it existed. Uses /// `swap_remove` and repairs `path_index` for the moved tail entry. /// - /// `swap_remove` can leave indices in the filtered view stale; the caller - /// must mark the panel dirty so the view is rebuilt before the next read. + /// `swap_remove` can leave indices in the filtered view stale, so on a real + /// removal this self-marks the panel dirty (via [`mark_dirty`]) to enforce + /// the rebuild-before-next-read invariant instead of trusting every caller to + /// remember it. pub fn remove(&mut self, path: &Path) -> bool { if self.unfiltered_entries.is_empty() { return false; @@ -221,6 +223,7 @@ impl PanelListing { } else { self.unfiltered_entries.pop(); } + self.mark_dirty(); true } @@ -292,7 +295,7 @@ pub struct PanelState { pub listing: PanelListing, pub cursor: usize, pub scroll_offset: usize, - pub(crate) history: Vec, + pub(crate) history: VecDeque, pub(crate) sort_mode: SortMode, pub(crate) sort_options: SortOptions, pub(crate) listing_mode: ListingMode, @@ -314,7 +317,7 @@ impl PanelState { listing: PanelListing::new(), cursor: 0, scroll_offset: 0, - history: Vec::new(), + history: VecDeque::new(), sort_mode: SortMode::default(), sort_options: SortOptions::default(), listing_mode: ListingMode::default(), @@ -345,21 +348,23 @@ impl PanelState { self.canonical_path = canonical; } - pub fn history(&self) -> &[PathBuf] { + pub fn history(&self) -> &VecDeque { &self.history } const MAX_HISTORY: usize = 256; pub fn push_history(&mut self, path: PathBuf) { + // `VecDeque` so the capacity cap evicts the oldest entry in O(1) + // (`pop_front`) instead of `Vec::remove(0)`'s O(n) element shift. if self.history.len() >= Self::MAX_HISTORY { - self.history.remove(0); + self.history.pop_front(); } - self.history.push(path); + self.history.push_back(path); } pub fn pop_history(&mut self) -> Option { - self.history.pop() + self.history.pop_back() } pub fn sort_mode(&self) -> SortMode { diff --git a/src/app/user_menu.rs b/src/app/user_menu.rs index 395cbd0..f5071e1 100755 --- a/src/app/user_menu.rs +++ b/src/app/user_menu.rs @@ -58,10 +58,19 @@ impl PartialEq for MenuEntry { /// against option injection (filenames starting with `-`). /// Use `safe_file_arg` which prepends `./` to `-`-prefixed names. pub fn shell_quote(s: &str) -> String { - // Exact for the common quote-free case (content + two surrounding quotes); - // only the rare embedded-quote path triggers a reallocation. - let mut out = String::with_capacity(s.len() + 2); + shell_quote_prefixed("", s) +} + +/// Single-quote-escape `s`, emitting `prefix` (already trusted, quote-free) +/// inside the quotes before the escaped body. Fuses the `./` option-injection +/// guard into the same allocation as the quoting, avoiding the intermediate +/// `format!("./{s}")` string that `safe_file_arg` used to build. +fn shell_quote_prefixed(prefix: &str, s: &str) -> String { + // Exact for the common quote-free case (prefix + content + two surrounding + // quotes); only the rare embedded-quote path triggers a reallocation. + let mut out = String::with_capacity(prefix.len() + s.len() + 2); out.push('\''); + out.push_str(prefix); for ch in s.chars() { if ch == '\'' { out.push_str("'\\''"); @@ -75,8 +84,9 @@ pub fn shell_quote(s: &str) -> String { fn safe_file_arg(s: &str) -> String { if s.starts_with('-') { - // format! + shell_quote = two String allocations; worth fusing later. - shell_quote(&format!("./{s}")) + // Prepend `./` so the shell cannot mistake a `-`-prefixed filename for an + // option, fused into a single allocation via `shell_quote_prefixed`. + shell_quote_prefixed("./", s) } else { shell_quote(s) } diff --git a/src/fs/reader.rs b/src/fs/reader.rs index 959b71f..e07a7cd 100755 --- a/src/fs/reader.rs +++ b/src/fs/reader.rs @@ -67,23 +67,56 @@ static UID_CACHE: LazyLock> = LazyLock::new(|| { }) }); +/// Locks the global id->name cache, recovering from poisoning by clearing the +/// (possibly inconsistent) cache and un-poisoning so normal caching resumes. +#[cfg(unix)] +fn lock_cache() -> std::sync::MutexGuard<'static, UidCache> { + match UID_CACHE.lock() { + Ok(guard) => guard, + Err(poisoned) => { + // A previous holder panicked mid-mutation, so map/order may be out of + // sync. Take ownership, dump the suspect data, and rebuild from an + // empty, consistent state rather than trusting it. + let mut guard = poisoned.into_inner(); + guard.clear(); + UID_CACHE.clear_poison(); + guard + } + } +} + #[cfg(unix)] fn get_or_insert_name( - cache: &mut NameMapCache, + select: impl Fn(&mut UidCache) -> &mut NameMapCache, id: u32, lookup: impl FnOnce(u32) -> Option>, ) -> Arc { - if let Some(name) = cache.map.get(&id) { - return name.clone(); - } - if cache.map.len() >= CACHE_MAX_SIZE - && let Some(old) = cache.order.pop_front() + // Phase 1: fast cache hit, then DROP the lock before any lookup. The lookup + // (getpwuid/getgrgid via NSS) can block for hundreds of ms on networked + // backends (LDAP/AD/SSSD); holding the single global mutex across it would + // stall every other thread resolving metadata. { - cache.map.remove(&old); + let mut cache = lock_cache(); + if let Some(name) = select(&mut cache).map.get(&id) { + return name.clone(); + } } - cache.order.push_back(id); + // Phase 2: possibly-slow lookup with NO lock held. let name = lookup(id).unwrap_or_else(|| Arc::from(id.to_string())); - cache.map.insert(id, name.clone()); + // Phase 3: re-acquire to insert; another thread may have resolved the same id + // meanwhile β€” prefer the existing entry so the cache stays single-valued. + let mut cache = lock_cache(); + let map = select(&mut cache); + if let Some(existing) = map.map.get(&id) { + return existing.clone(); + } + if map.map.len() >= CACHE_MAX_SIZE + && let Some(old) = map.order.pop_front() + { + map.map.remove(&old); + } + map.order.push_back(id); + map.map.insert(id, name.clone()); name } @@ -101,27 +134,16 @@ fn os_str_to_arc(s: &std::ffi::OsStr) -> Arc { #[cfg(unix)] fn lookup_owner_group(uid: u32, gid: u32) -> (Arc, Arc) { - let mut cache = match UID_CACHE.lock() { - Ok(guard) => guard, - Err(poisoned) => { - // A previous holder panicked mid-mutation, so map/order may be out - // of sync. Take ownership of the guard, dump the suspect data, and - // rebuild from an empty, consistent state rather than trusting it. - let mut guard = poisoned.into_inner(); - guard.clear(); - // Un-poison the lock so subsequent callers take the fast Ok path and - // normal caching resumes; otherwise every later lock would re-clear - // the (now-consistent) cache, degrading to no-cache mode forever. - UID_CACHE.clear_poison(); - guard - } - }; - let owner = get_or_insert_name(&mut cache.uid, uid, |id| { - users::get_user_by_uid(id).map(|u| os_str_to_arc(u.name())) - }); - let group = get_or_insert_name(&mut cache.gid, gid, |id| { - users::get_group_by_gid(id).map(|g| os_str_to_arc(g.name())) - }); + let owner = get_or_insert_name( + |c| &mut c.uid, + uid, + |id| users::get_user_by_uid(id).map(|u| os_str_to_arc(u.name())), + ); + let group = get_or_insert_name( + |c| &mut c.gid, + gid, + |id| users::get_group_by_gid(id).map(|g| os_str_to_arc(g.name())), + ); (owner, group) } @@ -142,7 +164,13 @@ fn os_str_to_string(s: &std::ffi::OsStr) -> String { } fn file_name_from_path(path: &Path) -> String { - os_str_to_string(path.file_name().unwrap_or_default()) + // `Path::file_name()` returns `None` for the filesystem root (`/`) and for + // paths ending in `..`; fall back to the whole path's display form so the + // root renders as `/` rather than an empty name. + match path.file_name() { + Some(name) => os_str_to_string(name), + None => path.as_os_str().to_string_lossy().into_owned(), + } } fn build_file_entry(entry: &std::fs::DirEntry) -> io::Result { @@ -308,12 +336,16 @@ pub fn get_file_info(path: &Path) -> io::Result { /// the entry must not be a symlink β€” symlinks need their target metadata, which /// this fast path does not resolve. pub fn file_info_from_metadata(path: PathBuf, metadata: &fs::Metadata) -> FileEntry { - debug_assert!( - !metadata.is_symlink(), - "file_info_from_metadata requires non-symlink metadata (symlinks need target metadata): {}", - path.display() - ); + // The fast path (no target lookup) is only correct for non-symlink metadata. + // If a caller violates the precondition, resolve the target metadata here + // rather than silently rendering the symlink as a broken/orphan entry β€” the + // old `debug_assert` only caught this in debug builds and was a no-op in + // release, where the precondition matters most. let file_name = file_name_from_path(&path); + if metadata.is_symlink() { + let target_meta = fs::metadata(&path).ok(); + return build_file_entry_from_metadata(path, file_name, metadata, target_meta.as_ref()); + } build_file_entry_from_metadata(path, file_name, metadata, None) } diff --git a/src/fs/watcher.rs b/src/fs/watcher.rs index ae57dfd..cf4b281 100644 --- a/src/fs/watcher.rs +++ b/src/fs/watcher.rs @@ -253,8 +253,22 @@ impl Watcher { fn remove_watched_dir_state(&mut self, path: &Path) { let clean = crate::fs::path::clean_path(path); + // A deleted symlink can no longer be resolved via `read_link`, so + // `path_points_to_missing_watch` cannot map it to its canonical watch key + // and the watcher would leak. Recover the key from `path_cache`, which + // recorded the symlink -> canonical mapping when the watch was added: any + // cached spelling that cleans to `clean` names the canonical entry to + // evict. + let cached_canonical: Vec = self + .path_cache + .iter() + .filter(|(k, _)| crate::fs::path::clean_path(k) == clean) + .map(|(_, v)| v.clone()) + .collect(); self.watchers.retain(|watched, _| { - watched.as_path() != clean && !path_points_to_missing_watch(&clean, watched) + watched.as_path() != clean + && !cached_canonical.iter().any(|c| c == watched) + && !path_points_to_missing_watch(&clean, watched) }); self.path_cache .retain(|_, v| self.watchers.contains_key(v.as_path())); @@ -745,9 +759,15 @@ fn take_paired_from( ) -> Option { let entries = pending.get_mut(parent_key)?; let taken = match to_cookie { + // A cookie that matches no buffered From means there is genuinely no pair + // for this To (e.g. a move-in from outside the watched dir): return None + // so it surfaces as `Created` and the orphaned From times out to + // `Deleted`, rather than stealing an unrelated From via FIFO and emitting + // a bogus `Renamed`. FIFO is only the right heuristic when the backend + // gives no cookie at all (macOS FSEvents). Some(cookie) => match entries.iter().position(|e| e.cookie == Some(cookie)) { Some(idx) => entries.remove(idx), - None => entries.pop_front(), + None => None, }, None => entries.pop_front(), }; diff --git a/src/input/dialogs.rs b/src/input/dialogs.rs index c7b8528..517797a 100644 --- a/src/input/dialogs.rs +++ b/src/input/dialogs.rs @@ -121,6 +121,18 @@ fn is_same_file(src: &std::path::Path, dest: &std::path::Path) -> bool { } } +/// Reports which destination names already exist, so the caller can prompt +/// before overwriting. +/// +/// This is a pre-flight check only: the actual copy/move runs asynchronously in +/// `job_runner`, so a file can appear (or vanish) in the window between this +/// `symlink_metadata` probe and the operation. The check therefore drives the +/// confirmation UX, not safety β€” atomicity is the operation layer's job: +/// non-overwrite copies open the target with `create_new`/`O_NOFOLLOW` and moves +/// call `ensure_destination_absent` immediately before `fs::rename`. A fully +/// race-free guarantee would need OS-specific atomics (e.g. `renameat2` with +/// `RENAME_NOREPLACE`), which the codebase's non-adversarial-filesystem contract +/// deliberately does not pursue. pub(crate) fn check_overwrite_conflict(state: &AppState) -> Option> { let action = state.ui.pending_action.as_ref()?; match action { @@ -193,6 +205,16 @@ pub(crate) fn check_overwrite_conflict(state: &AppState) -> Option> } } +/// Flip the two-button dialog selection (0 <-> 1). Shared by the confirmation +/// and archive dialogs so the Left/Right toggle stays identical in both. +fn toggle_dialog_selection(state: &mut AppState) { + state.input.dialog_selection = if state.input.dialog_selection == 0 { + 1 + } else { + 0 + }; +} + fn confirm_dialog_key(state: &mut AppState, key: KeyCode) -> Option { match key { KeyCode::Char('y' | 'Y') => Some(true), @@ -203,11 +225,7 @@ fn confirm_dialog_key(state: &mut AppState, key: KeyCode) -> Option { None } KeyCode::Left | KeyCode::Right => { - state.input.dialog_selection = if state.input.dialog_selection == 0 { - 1 - } else { - 0 - }; + toggle_dialog_selection(state); None } _ => None, @@ -328,6 +346,9 @@ fn input_action_viewer_search( } fn input_action_create_directory(state: &mut AppState, input: &str) -> InputOutcome { + // Clear any stale message up front so a success leaves no leftover error from + // a prior attempt; the validation/failure paths below set a fresh one. + state.ui.status_message = None; if let Err(msg) = validate_create_or_rename(input, "Directory name") { state.ui.status_message = Some(msg); return InputOutcome::Finalized; @@ -340,6 +361,7 @@ fn input_action_create_directory(state: &mut AppState, input: &str) -> InputOutc } fn input_action_rename(state: &mut AppState, input: &str) -> InputOutcome { + state.ui.status_message = None; if let Err(msg) = validate_create_or_rename(input, "New name") { state.ui.status_message = Some(msg); return InputOutcome::Finalized; @@ -354,6 +376,7 @@ fn input_action_rename(state: &mut AppState, input: &str) -> InputOutcome { } fn input_action_chmod(state: &mut AppState, input: &str) -> InputOutcome { + state.ui.status_message = None; let Some(mode) = parse_octal_mode(input) else { if input.trim().is_empty() { state.ui.status_message = Some("Octal mode cannot be empty".to_string()); @@ -565,11 +588,7 @@ fn archive_dialog_nav(state: &mut AppState, key: KeyCode) -> ArchiveNav { ArchiveNav::Dismissed } KeyCode::Left | KeyCode::Right => { - state.input.dialog_selection = if state.input.dialog_selection == 0 { - 1 - } else { - 0 - }; + toggle_dialog_selection(state); ArchiveNav::Handled } KeyCode::Enter if state.input.dialog_selection == 1 => { diff --git a/src/input/mouse.rs b/src/input/mouse.rs index 21faeb5..aec01bf 100644 --- a/src/input/mouse.rs +++ b/src/input/mouse.rs @@ -27,9 +27,13 @@ const DEFAULT_DROPDOWN_WIDTH: usize = 10; /// (top border, header row, bottom border, function bar). Used by /// [`panel_bounds`] to derive the file-row range from the terminal height. const PANEL_CHROME_ROWS: u16 = 4; -/// Minimum terminal height that can host at least one panel file row. Below -/// this, [`panel_bounds`] yields an empty range and clicks/scrolls in the panel -/// body are no-ops (see [`in_panel_file_rows`]). +/// Smallest terminal height for which `height - PANEL_CHROME_ROWS` stays `>= 1`, +/// i.e. the threshold below which [`panel_bounds`] would underflow into a +/// malformed `(1, 0)` range. It is NOT the height needed to *display* a file +/// row: the body has `height - LAYOUT_OVERHEAD_ROWS` (= 6) rows, so the first +/// real file row only appears at `height >= 7`. For heights 5–6 the bounds are +/// well-formed but the body is empty, which correctly matches the renderer +/// (`panel_visible_height`), so clicks/scrolls in the body are no-ops. const MIN_PANEL_HEIGHT: u16 = PANEL_CHROME_ROWS + 1; /// The function bar is split into 10 equal-width F-key buttons (F1..=F10). const FUNCTION_BAR_BUTTONS: u32 = 10; @@ -743,8 +747,13 @@ fn set_selection_range( } fn handle_mouse_up(state: &mut AppState) { + // Do NOT clear `last_click` here. crossterm reports a physical double-click as + // Down, Up, Down, Up (no native double-click event), so clearing it on the + // first Up would erase the timestamp the second Down needs, making + // double-click detection impossible. Stale entries are already invalidated by + // the `DOUBLE_CLICK_THRESHOLD_MS` timestamp check and reset on a successful + // double-click. state.interaction.drag_anchor_index = None; - state.interaction.last_click = None; } #[cfg(test)] diff --git a/src/input/normal.rs b/src/input/normal.rs index 0ed0d02..299b674 100644 --- a/src/input/normal.rs +++ b/src/input/normal.rs @@ -363,8 +363,11 @@ fn page_up(state: &mut AppState, visible: usize) { fn page_down(state: &mut AppState, visible: usize) { let len = state.active_panel().listing.filtered_len(); let p = state.active_panel_mut(); - p.cursor = (p.cursor + visible).min(len.saturating_sub(1)); - p.scroll_offset = (p.scroll_offset + visible).min(len.saturating_sub(visible)); + p.cursor = p.cursor.saturating_add(visible).min(len.saturating_sub(1)); + p.scroll_offset = p + .scroll_offset + .saturating_add(visible) + .min(len.saturating_sub(visible)); } fn switch_active_panel(state: &mut AppState, visible: usize) { @@ -425,7 +428,7 @@ pub(crate) fn handle_enter_key( None }; let p = state.active_panel_mut(); - if p.history().last().map(|p| p.as_path()) != Some(p.path()) { + if p.history().back().map(|p| p.as_path()) != Some(p.path()) { p.push_history(p.path().to_path_buf()); } p.set_path(path); diff --git a/src/main.rs b/src/main.rs index 99aafc5..73b3526 100644 --- a/src/main.rs +++ b/src/main.rs @@ -215,16 +215,15 @@ fn pre_draw( image_preview_loader: &mut Option, term_size: Size, ) { - let size = term_size; start_image_preview_if_needed( viewer_state, image_preview_loader, - (size.width, size.height), + (term_size.width, term_size.height), ); if let Some(vs) = viewer_state && state.mode == AppMode::Viewing { - vs.update_wrap_layout(size.width as usize); + vs.update_wrap_layout(term_size.width as usize); } } @@ -303,10 +302,18 @@ fn poll_async( state, &mut loop_state.watcher_sync_state, ); + // Flush BEFORE polling, every iteration: debounced Created/Modified events + // sit in the watcher's debounce map and are only pushed onto the channel by + // `flush_pending`. If we only flushed after a non-empty poll, an event whose + // debounce window expired during a quiet period (no further filesystem + // activity) would never be delivered to the UI. Flushing unconditionally each + // ~33ms loop tick guarantees expired events surface within one debounce + // interval, and also drains entries left pending after `sync_watcher_paths` + // removed their watch. + if let Some(ref w) = loop_state.watcher { + w.flush_pending(); + } if watcher_sync::poll_watcher_events(state, watch_rx) { - if let Some(ref w) = loop_state.watcher { - w.flush_pending(); - } dirty = true; } if poll_running_job(state, &mut loop_state.running_job, panel_ops::refresh_both) { @@ -393,7 +400,17 @@ fn run_app(terminal: &mut Terminal>) -> io::Result< } dirty = false; } - if event::poll(Duration::from_millis(EVENT_POLL_TIMEOUT_MS))? { + let has_event = match event::poll(Duration::from_millis(EVENT_POLL_TIMEOUT_MS)) { + Ok(ready) => ready, + Err(e) => { + // Mirror the `event::read()` error path below: tear down any + // running job before propagating, instead of relying solely on + // the destructor's best-effort reaper. + shutdown_job(&mut loop_state.running_job); + return Err(e); + } + }; + if has_event { let key = match event::read() { Ok(k) => k, Err(e) => { diff --git a/src/ops/archive/mod.rs b/src/ops/archive/mod.rs index 818a4be..3ed00c9 100644 --- a/src/ops/archive/mod.rs +++ b/src/ops/archive/mod.rs @@ -649,4 +649,54 @@ mod tests { assert_eq!(fs::read(dest.join("hello.txt")).unwrap(), f1_content); assert_eq!(fs::read(dest.join("data.bin")).unwrap(), f2_content); } + + // Regression: `.tar.zst` creation must finalize the zstd frame. Before the + // fix the encoder was dropped without `finish()`/`auto_finish()`, so the + // frame epilogue and the tar end-of-archive blocks were never written and + // every created archive was silently truncated/corrupt. This drives the full + // create -> extract round trip; extraction would fail (or lose contents) on a + // truncated frame. + #[test] + fn tar_zst_roundtrip_preserves_contents() { + use std::sync::atomic::AtomicBool; + use std::sync::mpsc; + + let work = tempfile::tempdir().unwrap(); + let src_dir = work.path().join("src"); + fs::create_dir(&src_dir).unwrap(); + let f1 = src_dir.join("hello.txt"); + let f2 = src_dir.join("data.bin"); + let f1_content = b"hello tar zst world"; + let f2_content: Vec = (0..8192u32).map(|i| (i % 251) as u8).collect(); + fs::write(&f1, f1_content).unwrap(); + fs::write(&f2, &f2_content).unwrap(); + + let archive_path = work.path().join("out.tar.zst"); + let (tx, _rx) = mpsc::channel(); + crate::ops::archive::create::create_archive( + &[f1, f2], + &archive_path, + ArchiveFormat::TarZst, + &tx, + &AtomicBool::new(false), + ) + .unwrap(); + + let (fmt, _file) = detect_format(&archive_path).unwrap(); + assert_eq!(fmt, ArchiveFormat::TarZst); + + let dest = work.path().join("extract"); + fs::create_dir(&dest).unwrap(); + let (tx2, _rx2) = mpsc::channel(); + crate::ops::archive::extract::extract_archive( + &archive_path, + &dest, + &tx2, + &AtomicBool::new(false), + ) + .unwrap(); + + assert_eq!(fs::read(dest.join("hello.txt")).unwrap(), f1_content); + assert_eq!(fs::read(dest.join("data.bin")).unwrap(), f2_content); + } } diff --git a/src/ops/archive/sevenz.rs b/src/ops/archive/sevenz.rs index d4658f6..a7bc50c 100644 --- a/src/ops/archive/sevenz.rs +++ b/src/ops/archive/sevenz.rs @@ -58,7 +58,6 @@ struct SevenzEntryExtractor<'a> { progress: &'a Sender, cancel: &'a AtomicBool, error_slot: &'a Cell>, - last_parent: Option, total_size: super::TotalSizeGuard, extracted_paths: &'a mut Vec, } @@ -73,15 +72,16 @@ impl<'a> SevenzEntryExtractor<'a> { } fn create_parent_if_needed(&mut self, outpath: &Path) -> Result<(), sevenz_rust::Error> { - if let Some(parent) = outpath.parent() - && self.last_parent.as_deref() != Some(parent) - { + // Re-verify the parent on EVERY entry rather than caching the last one: a + // cached parent could be swapped for a symlink between entries (TOCTOU), + // and skipping `verify_inside` on a cache hit would let a later entry be + // written outside `canonical_dest`. + if let Some(parent) = outpath.parent() { if let Err(e) = fs::create_dir_all(parent) { self.error_slot.set(Some(SevenzExtractError::Io(e))); return Err(sevenz_rust::Error::Other("create_dir_all failed".into())); } self.verify_inside(parent)?; - self.last_parent = Some(parent.to_path_buf()); } Ok(()) } @@ -215,7 +215,6 @@ pub fn extract_7z( progress, cancel, error_slot: &error_slot, - last_parent: None, total_size: super::TotalSizeGuard::default(), extracted_paths: &mut extracted_paths, }; diff --git a/src/ops/archive/tar.rs b/src/ops/archive/tar.rs index 277e40f..170903e 100644 --- a/src/ops/archive/tar.rs +++ b/src/ops/archive/tar.rs @@ -15,6 +15,33 @@ use crate::debug_log; const MAX_CREATE_ENTRIES: usize = 100_000; +/// Write adapter that aborts once cumulative output exceeds +/// `MAX_TOTAL_ARCHIVE_SIZE`. Used to bound the xz -> temp-file materialization so +/// the decompression-bomb guard applies before the whole payload is on disk +/// (the streaming gz/bz2/zst decoders are already bounded lazily by the extract +/// loop's `TotalSizeGuard`). +struct CappedWriter { + inner: W, + written: u64, +} + +impl Write for CappedWriter { + fn write(&mut self, buf: &[u8]) -> io::Result { + self.written = self.written.saturating_add(buf.len() as u64); + if self.written > super::MAX_TOTAL_ARCHIVE_SIZE { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "decompressed xz exceeds maximum archive size", + )); + } + self.inner.write(buf) + } + + fn flush(&mut self) -> io::Result<()> { + self.inner.flush() + } +} + /// Appends the contents of `src` into the tar builder under `dest_name`, /// counting entries against `count`. Returns an error if `MAX_CREATE_ENTRIES` /// would be exceeded, making the limit check and the append a single pass. @@ -128,7 +155,6 @@ pub fn extract_tar( let result = (|| -> Result<(), ArchiveError> { let canonical_dest = dest.canonicalize().map_err(ArchiveError::Io)?; - let mut last_parent: Option = None; let mut total_size = super::TotalSizeGuard::default(); for entry in archive.entries()? { super::check_cancel(cancel)?; @@ -174,12 +200,13 @@ pub fn extract_tar( extracted_paths.push(outpath); let _ = progress.send(size); } else { - if let Some(parent) = outpath.parent() - && last_parent.as_deref() != Some(parent) - { + // Re-verify the parent on EVERY entry rather than caching the last + // one: a cached parent could be swapped for a symlink between + // entries (TOCTOU), and skipping `verify_within_dest` on a cache hit + // would let a later entry be written outside `canonical_dest`. + if let Some(parent) = outpath.parent() { fs::create_dir_all(parent)?; super::verify_within_dest(&canonical_dest, parent)?; - last_parent = Some(parent.to_path_buf()); } // MITIGATION: TOCTOU symlink race β€” an attacker could replace a // regular file with a symlink between the symlink_metadata check below @@ -399,12 +426,23 @@ fn wrap_decompress(file: File, format: ArchiveFormat) -> Result, A super::create_archive_temp_file("lc-xz-decompress", ".tar") .map_err(ArchiveError::Io)?; let mut reader = io::BufReader::with_capacity(super::IO_BUFFER_SIZE, file); - if let Err(e) = lzma_rs::xz_decompress(&mut reader, &mut tmp_file) { - cleanup_temp_file(&tmp_path); - return Err(ArchiveError::Io(io::Error::other(e))); + { + // Inner scope so the `CappedWriter`'s `&mut tmp_file` borrow ends + // before we drop the file and reopen it for reading. + let mut capped = CappedWriter { + inner: &mut tmp_file, + written: 0, + }; + if let Err(e) = lzma_rs::xz_decompress(&mut reader, &mut capped) { + cleanup_temp_file(&tmp_path); + return Err(ArchiveError::Io(io::Error::other(e))); + } } drop(tmp_file); - let tmp_file = File::open(&tmp_path)?; + // Clean up the temp file if the reopen fails, matching the + // xz_decompress error path above; otherwise it would be orphaned + // (the cleanup-on-drop `TempFileReader` is only built after this). + let tmp_file = File::open(&tmp_path).inspect_err(|_| cleanup_temp_file(&tmp_path))?; struct TempFileReader { inner: File, path: PathBuf, @@ -449,8 +487,14 @@ fn wrap_compress(file: File, format: ArchiveFormat) -> Result, Ar Ok(Box::new(encoder)) } ArchiveFormat::TarZst => { + // `auto_finish()` so the zstd frame epilogue (and any buffered tail, + // including the tar end-of-archive blocks) is written when the boxed + // writer is dropped. A plain `Encoder` requires an explicit + // `finish()` that `build_tar_into` never calls β€” it only drops the + // builder β€” which silently produced truncated, corrupt `.tar.zst` + // archives. Mirrors the drop-finalizing GzEncoder/BzEncoder branches. let encoder = zstd::stream::write::Encoder::new(file, 0)?; - Ok(Box::new(encoder)) + Ok(Box::new(encoder.auto_finish())) } ArchiveFormat::TarXz | ArchiveFormat::Zip | ArchiveFormat::SevenZ => { Err(ArchiveError::UnsupportedFormat) diff --git a/src/ops/archive/zip.rs b/src/ops/archive/zip.rs index f47befb..c45e626 100644 --- a/src/ops/archive/zip.rs +++ b/src/ops/archive/zip.rs @@ -16,6 +16,22 @@ use crate::debug_log; const DEFAULT_COMPRESSION_LEVEL: i64 = 6; +/// Upper bound on the number of entries written into a created archive. Mirrors +/// the tar create-side limit (`tar::MAX_CREATE_ENTRIES`) so both formats reject +/// pathologically large directory trees instead of exhausting memory/CPU. +const MAX_CREATE_ENTRIES: usize = 100_000; + +/// Increments `count` for one archive entry and errors if the limit is crossed. +fn count_entry(count: &mut usize) -> Result<(), ArchiveError> { + *count = count.saturating_add(1); + if *count > MAX_CREATE_ENTRIES { + return Err(ArchiveError::InvalidArchive(format!( + "too many entries (limit {MAX_CREATE_ENTRIES})" + ))); + } + Ok(()) +} + /// Removes a partially written temporary archive, logging any failure instead of /// silently dropping the error (mirrors the tar create path). fn cleanup_temp_file(path: &Path) { @@ -75,13 +91,21 @@ fn extract_zip_entries( extracted_paths: &mut Vec, ) -> Result<(), ArchiveError> { let entry_count = archive.len(); - extracted_paths.reserve(entry_count.min(MAX_LIST_ENTRIES)); + // Fail loudly instead of silently truncating: `.min(MAX_LIST_ENTRIES)` on the + // loop would extract only the first 100k entries yet still return `Ok`, + // reporting success on an incomplete extraction. The cap still guards against + // pathological entry counts β€” it just can no longer be mistaken for success. + if entry_count > MAX_LIST_ENTRIES { + return Err(ArchiveError::InvalidArchive(format!( + "archive has {entry_count} entries, exceeding the maximum {MAX_LIST_ENTRIES}" + ))); + } + extracted_paths.reserve(entry_count); fs::create_dir_all(dest)?; let canonical_dest = dest.canonicalize().map_err(ArchiveError::Io)?; - let mut last_parent: Option = None; let mut total_size = super::TotalSizeGuard::default(); - for i in 0..entry_count.min(MAX_LIST_ENTRIES) { + for i in 0..entry_count { check_cancel(cancel)?; let mut entry = archive.by_index(i).map_err(map_zip_err)?; @@ -115,12 +139,13 @@ fn extract_zip_entries( entry.compressed_size() ))); } - if let Some(parent) = outpath.parent() - && last_parent.as_deref() != Some(parent) - { + // Re-verify the parent on EVERY entry rather than caching the last + // one: a cached parent could be swapped for a symlink between entries + // (TOCTOU), and skipping `verify_within_dest` on a cache hit would let + // a later entry be written outside `canonical_dest`. + if let Some(parent) = outpath.parent() { fs::create_dir_all(parent)?; super::verify_within_dest(&canonical_dest, parent)?; - last_parent = Some(parent.to_path_buf()); } super::check_symlink_at_dest(&outpath)?; let mut outfile = super::open_outfile(&outpath)?; @@ -168,6 +193,7 @@ fn add_sources_to_zip( progress: &Sender, cancel: &AtomicBool, ) -> Result<(), ArchiveError> { + let mut count: usize = 0; for source in sources { check_cancel(cancel)?; @@ -176,8 +202,9 @@ fn add_sources_to_zip( // (add_dir_to_zip) and the final open is hardened with O_NOFOLLOW. let meta = fs::symlink_metadata(source)?; if meta.is_dir() { - add_dir_to_zip(zip, source, source, options, progress, cancel)?; + add_dir_to_zip(zip, source, source, options, progress, cancel, &mut count)?; } else { + count_entry(&mut count)?; let name = source .file_name() .ok_or_else(|| { @@ -251,6 +278,7 @@ fn add_dir_to_zip( options: &SimpleFileOptions, progress: &Sender, cancel: &AtomicBool, + count: &mut usize, ) -> Result<(), ArchiveError> { for entry in fs::read_dir(dir)? { check_cancel(cancel)?; @@ -276,9 +304,11 @@ fn add_dir_to_zip( continue; } if meta.is_dir() { + count_entry(count)?; zip.add_directory(&name, *options).map_err(map_zip_err)?; - add_dir_to_zip(zip, base, &path, options, progress, cancel)?; + add_dir_to_zip(zip, base, &path, options, progress, cancel, count)?; } else { + count_entry(count)?; add_file_to_zip(zip, &name, &path, options, progress)?; } } diff --git a/src/ops/batch.rs b/src/ops/batch.rs index 0dee8f4..8016f8c 100644 --- a/src/ops/batch.rs +++ b/src/ops/batch.rs @@ -625,74 +625,36 @@ fn process_batch_entry( report_transition(progress, ctx, idx + 1, state.bytes_done, state.bytes_total); } -#[cfg(unix)] -mod identity { - use std::io; - use std::os::unix::fs::MetadataExt; - use std::path::Path; - - #[derive(Hash, Eq, PartialEq)] - pub struct Identity { - dev: u64, - ino: u64, - } - - pub fn get_identity(path: &Path) -> io::Result { - let meta = path.symlink_metadata()?; - Ok(Identity { - dev: meta.dev(), - ino: meta.ino(), - }) - } -} - -#[cfg(not(unix))] -mod identity { - use std::fs; - use std::io; - use std::path::{Path, PathBuf}; - - #[derive(Hash, Eq, PartialEq)] - pub struct Identity { - path: PathBuf, - } - - pub fn get_identity(path: &Path) -> io::Result { - let _meta = path.symlink_metadata()?; - Ok(Identity { - path: path.to_path_buf(), - }) - } -} - -use identity::Identity; - fn dedup_paths(paths: &[PathBuf]) -> Vec { - let mut seen_identities: HashSet = HashSet::new(); - let mut identity_ok: Vec = Vec::new(); - let mut identity_fail: Vec = Vec::new(); + // Dedup on the literal selected path (exact-duplicate selections only), NOT on + // (dev, ino): distinct directory entries that are hardlinks to the same inode + // β€” or a symlink and its target β€” are genuinely separate selections and must + // all be acted on. Inode-based dedup silently dropped a hardlinked sibling, + // reporting success on an incomplete delete. A path appears at most once in a + // panel listing, so a literal-path set is sufficient to collapse true + // duplicate selections. + let mut seen: HashSet = HashSet::new(); + let mut existing: Vec = Vec::new(); + let mut nonexistent: Vec = Vec::new(); for p in paths { - match identity::get_identity(p) { - Ok(id) => { - if !seen_identities.insert(id) { - continue; - } - identity_ok.push(p.clone()); - } - Err(_) => { - identity_fail.push(p.clone()); - } + if !seen.insert(p.clone()) { + continue; + } + if p.symlink_metadata().is_ok() { + existing.push(p.clone()); + } else { + nonexistent.push(p.clone()); } } // PERF: sort + O(nΒ·depth) ancestors check. For very large batches (100k+) // this may become a bottleneck; profile before considering a trie/hash // prefix tree to reduce worst-case from O(nΒ·depth) to amortized O(n). - identity_ok.sort_by_key(|p| p.components().count()); + existing.sort_by_key(|p| p.components().count()); let mut filtered: Vec = Vec::new(); - let mut accepted: HashSet<&Path> = HashSet::with_capacity(identity_ok.len()); - for p in &identity_ok { + let mut accepted: HashSet<&Path> = HashSet::with_capacity(existing.len()); + for p in &existing { let dominated = p.ancestors().skip(1).any(|a| accepted.contains(a)); if !dominated { accepted.insert(p); @@ -700,9 +662,7 @@ fn dedup_paths(paths: &[PathBuf]) -> Vec { } } - identity_fail.sort(); - identity_fail.dedup(); - filtered.extend(identity_fail); + filtered.extend(nonexistent); filtered } diff --git a/src/ops/chunk_copy.rs b/src/ops/chunk_copy.rs index 94f8a03..baa78bc 100644 --- a/src/ops/chunk_copy.rs +++ b/src/ops/chunk_copy.rs @@ -117,6 +117,9 @@ fn copy_to_temp( } if cancel.load(Ordering::Relaxed) { + // Best-effort final flush: the copy is being aborted anyway, so a + // disconnected receiver is not an error here (unlike the mid-loop + // send above, where it signals the UI is gone and we bail out). if pending_delta > 0 { let _ = progress_tx.send(pending_delta); } @@ -124,6 +127,8 @@ fn copy_to_temp( } } + // Best-effort: the copy has already completed, so a dropped receiver only + // means the last progress tick is not displayed β€” nothing left to interrupt. if pending_delta > 0 { let _ = progress_tx.send(pending_delta); } @@ -142,8 +147,14 @@ fn publish_temp( cancel: &AtomicBool, overwrite: bool, ) -> io::Result<()> { + // Error-path temp cleanup is owned solely by the caller + // (`copy_with_progress`), which calls `cleanup_file(&temp_dest)` on any `Err` + // returned here. Cleaning up again in each error arm caused a double + // `remove_file` (the second failing `NotFound`) plus a misleading "failed to + // clean up" debug log. Only the hard_link success path below cleans up + // internally, because there the temp must be unlinked after it has been + // linked into place. if cancel.load(Ordering::Relaxed) { - cleanup_file(temp_dest); return Err(io::Error::new(io::ErrorKind::Interrupted, "copy canceled")); } if overwrite { @@ -159,12 +170,10 @@ fn publish_temp( return Ok(()); } Err(err) if err.kind() == io::ErrorKind::AlreadyExists => { - cleanup_file(temp_dest); return Err(err); } Err(_) => { if fs::symlink_metadata(dest).is_ok() { - cleanup_file(temp_dest); return Err(io::Error::new( io::ErrorKind::AlreadyExists, format!("destination appeared during copy: {}", dest.display()), @@ -173,13 +182,7 @@ fn publish_temp( } } - match fs::rename(temp_dest, dest) { - Ok(()) => Ok(()), - Err(err) => { - cleanup_file(temp_dest); - Err(err) - } - } + fs::rename(temp_dest, dest) } fn temp_path_for(dest: &Path) -> std::path::PathBuf { diff --git a/src/ops/compare.rs b/src/ops/compare.rs index a7caf16..f82d311 100644 --- a/src/ops/compare.rs +++ b/src/ops/compare.rs @@ -4,8 +4,12 @@ use std::time::Duration; use crate::app::types::{CompareMode, FileEntry, PanelState}; /// Cross-filesystem mtime resolution tolerance (e.g. FAT32 has 2s granularity, -/// network filesystems may lose sub-second precision during sync). -const MTIME_TOLERANCE: Duration = Duration::from_secs(2); +/// network filesystems may lose sub-second precision during sync). Set slightly +/// above FAT32's 2s granularity: when two filesystems round a timestamp in +/// opposite directions the recorded values can differ by *just over* 2s for what +/// is really the same modification, so an exact 2s bound would report a spurious +/// difference. The extra 500ms absorbs that boundary rounding. +const MTIME_TOLERANCE: Duration = Duration::from_millis(2500); /// The PARENT_DIR (parent directory) pseudo-entry β€” ignored during comparison. const PARENT_DIR: &str = ".."; diff --git a/src/ops/file_ops/delete.rs b/src/ops/file_ops/delete.rs index 321dde3..fd58bec 100644 --- a/src/ops/file_ops/delete.rs +++ b/src/ops/file_ops/delete.rs @@ -129,6 +129,38 @@ fn validate_not_critical(canonical: &Path) -> io::Result<()> { Ok(()) } +/// Validates that a file or symlink at `path` does not sit at or inside a +/// critical system directory, mirroring the protection `delete_dir_recursive_*` +/// already applies to directories. Used by the cross-device move fallback before +/// unlinking the source, so a regular file or symlink in `/usr/bin`, `/etc`, … +/// gets the same guard a directory there would. +/// +/// A final-component symlink is NOT followed: its own location (parent chain +/// resolved, link name re-attached) is validated, so the check guards where the +/// link lives rather than where it points. +pub fn ensure_entry_not_critical(path: &Path) -> io::Result<()> { + let meta = fs::symlink_metadata(path)?; + let canonical = if meta.file_type().is_symlink() { + let parent = path.parent().filter(|p| !p.as_os_str().is_empty()); + let parent_canonical = match parent { + Some(parent) => parent + .canonicalize() + .map_err(|e| io::Error::new(e.kind(), format!("Cannot verify path safety: {e}")))?, + None => Path::new(".") + .canonicalize() + .map_err(|e| io::Error::new(e.kind(), format!("Cannot verify path safety: {e}")))?, + }; + match path.file_name() { + Some(name) => parent_canonical.join(name), + None => parent_canonical, + } + } else { + path.canonicalize() + .map_err(|e| io::Error::new(e.kind(), format!("Cannot verify path safety: {e}")))? + }; + validate_not_critical(&canonical) +} + /// Recursive delete operates under a non-adversarial filesystem guarantee. /// It assumes no concurrent process is actively replacing directories with /// symlinks during the deletion. The critical-directory blocklist provides @@ -137,6 +169,11 @@ fn delete_dir_recursive_with_cancel(path: &Path, cancel: Option<&AtomicBool>) -> check_optional_canceled(cancel)?; let root_metadata = fs::symlink_metadata(path)?; if root_metadata.file_type().is_symlink() { + // Validate the symlink's own location before unlinking: on macOS `/etc`, + // `/var`, `/tmp` are themselves symlinks, so this early-return path would + // otherwise bypass the critical-directory blocklist that the rest of the + // function applies to real directories. + ensure_entry_not_critical(path)?; return remove_symlink(path); } let canonical = path @@ -189,3 +226,49 @@ fn delete_dir_contents_impl( } Ok(()) } + +#[cfg(test)] +#[allow(clippy::unwrap_used, clippy::expect_used)] +mod tests { + use super::*; + + #[test] + fn ensure_entry_not_critical_allows_ordinary_file() { + let dir = tempfile::tempdir().unwrap(); + let file = dir.path().join("ordinary.txt"); + fs::write(&file, b"data").unwrap(); + assert!(ensure_entry_not_critical(&file).is_ok()); + } + + #[cfg(unix)] + #[test] + fn ensure_entry_not_critical_rejects_file_in_critical_dir() { + // `/etc/hosts` resolves under a critical prefix (`/etc`, or `/private/etc` + // on macOS) on essentially every Unix; guard on existence so the test is + // a no-op on the rare system without it rather than a false failure. + let hosts = Path::new("/etc/hosts"); + if hosts.exists() { + assert!( + ensure_entry_not_critical(hosts).is_err(), + "expected /etc/hosts to be rejected as critical" + ); + } + } + + #[cfg(unix)] + #[test] + fn ensure_entry_not_critical_rejects_symlink_in_critical_dir_without_following() { + // A symlink whose *location* is critical must be rejected even if it + // points somewhere harmless: the guard validates where the link lives, + // not its target. We can't create files in `/etc`, so assert the inverse + // property instead β€” a symlink in a temp dir pointing INTO `/etc` is + // allowed, proving the target is not what gets validated. + let dir = tempfile::tempdir().unwrap(); + let link = dir.path().join("link-to-etc"); + std::os::unix::fs::symlink("/etc/hosts", &link).unwrap(); + assert!( + ensure_entry_not_critical(&link).is_ok(), + "a symlink in a temp dir must be judged by its own location, not its target" + ); + } +} diff --git a/src/ops/file_ops/entry_ops.rs b/src/ops/file_ops/entry_ops.rs index 24edf91..f4e1de4 100644 --- a/src/ops/file_ops/entry_ops.rs +++ b/src/ops/file_ops/entry_ops.rs @@ -73,14 +73,23 @@ pub fn rename_entry(old: &Path, new_name: &str) -> io::Result<()> { if new_path == old { return Ok(()); } - let same_file = match (fs::symlink_metadata(old), fs::symlink_metadata(&new_path)) { - (Ok(old_meta), Ok(new_meta)) => super::common::same_inode(&old_meta, &new_meta), + // Use `symlink_metadata` (not `try_exists`) for the existence check: a + // dangling symlink at `new_path` must count as "present" so we don't silently + // clobber it, but `try_exists` follows the broken link and reports `false`. + // This mirrors `common::ensure_destination_absent`. + let new_meta = match fs::symlink_metadata(&new_path) { + Ok(meta) => Some(meta), + Err(err) if err.kind() == io::ErrorKind::NotFound => None, + Err(err) => return Err(err), + }; + let same_file = match (fs::symlink_metadata(old), new_meta.as_ref()) { + (Ok(old_meta), Some(new_meta)) => super::common::same_inode(&old_meta, new_meta), _ => false, }; - // TOCTOU: `try_exists` + `fs::rename` is non-atomic. On POSIX, rename + // TOCTOU: this check + `fs::rename` is non-atomic. On POSIX, rename // atomically replaces the destination regardless; on Windows it errors. // This check is best-effort β€” atomic no-replace requires OS-specific APIs. - if !same_file && new_path.try_exists()? { + if !same_file && new_meta.is_some() { return Err(io::Error::new( io::ErrorKind::AlreadyExists, MSG_DEST_EXISTS, @@ -120,3 +129,44 @@ pub fn chmod(path: &Path, mode: u32) -> io::Result<()> { let result = fs::set_permissions(path, permissions); result } + +#[cfg(all(test, unix))] +#[allow(clippy::unwrap_used, clippy::expect_used)] +mod tests { + use super::*; + + // Regression: a dangling symlink at the destination must count as "present" + // so `rename_entry` refuses rather than silently clobbering it. The old + // `try_exists()` guard followed the broken link and reported `false`. + #[test] + fn rename_refuses_to_clobber_dangling_symlink() { + let dir = tempfile::tempdir().unwrap(); + let src = dir.path().join("src.txt"); + fs::write(&src, b"data").unwrap(); + + let dangling = dir.path().join("dest.txt"); + std::os::unix::fs::symlink(dir.path().join("missing-target"), &dangling).unwrap(); + + let err = rename_entry(&src, "dest.txt").expect_err("should refuse to clobber"); + assert_eq!(err.kind(), io::ErrorKind::AlreadyExists); + // Source untouched and the dangling symlink still there. + assert!(src.exists()); + assert!( + fs::symlink_metadata(&dangling) + .unwrap() + .file_type() + .is_symlink() + ); + } + + #[test] + fn rename_to_fresh_name_succeeds() { + let dir = tempfile::tempdir().unwrap(); + let src = dir.path().join("src.txt"); + fs::write(&src, b"data").unwrap(); + + rename_entry(&src, "renamed.txt").unwrap(); + assert!(!src.exists()); + assert_eq!(fs::read(dir.path().join("renamed.txt")).unwrap(), b"data"); + } +} diff --git a/src/ops/file_ops/move_ops.rs b/src/ops/file_ops/move_ops.rs index 8990796..49d76a1 100644 --- a/src/ops/file_ops/move_ops.rs +++ b/src/ops/file_ops/move_ops.rs @@ -8,7 +8,7 @@ use super::common::{ check_canceled, ensure_destination_absent, path_contains, remove_any, same_inode, }; use super::copy::{copy_dir_recursive_with_progress, copy_file_with_progress, copy_symlink}; -use super::delete::delete_dir_recursive_cancelable; +use super::delete::{delete_dir_recursive_cancelable, ensure_entry_not_critical}; #[derive(Clone, Copy)] enum MoveKind { @@ -63,8 +63,18 @@ impl MoveKind { fn remove_src(self, src: &Path, cancel: &AtomicBool) -> io::Result<()> { match self { - MoveKind::Symlink => remove_any(src), - MoveKind::File => fs::remove_file(src), + // Guard files and symlinks against critical-directory deletion to + // match the protection `delete_dir_recursive_cancelable` already gives + // directories; otherwise a cross-device move fallback could unlink a + // file/symlink in `/usr/bin`, `/etc`, … without any check. + MoveKind::Symlink => { + ensure_entry_not_critical(src)?; + remove_any(src) + } + MoveKind::File => { + ensure_entry_not_critical(src)?; + fs::remove_file(src) + } MoveKind::Directory => delete_dir_recursive_cancelable(src, cancel), } } @@ -151,6 +161,7 @@ fn move_entry_impl( src, dest, cancel, + overwrite, || kind.copy(src, dest, progress_tx, cancel, overwrite), || kind.remove_src(src, cancel), || remove_any(dest), @@ -161,17 +172,28 @@ fn move_entry_impl( } } +// The 8 parameters are intrinsic: three injected closures (so tests can drive +// the copy/remove/rollback steps independently) plus the data they report on. +#[allow(clippy::too_many_arguments)] fn copy_then_remove_src( src: &Path, dest: &Path, cancel: &AtomicBool, + overwrite: bool, copy_fn: impl FnOnce() -> io::Result<()>, remove_src_fn: impl FnOnce() -> io::Result<()>, rollback_fn: impl FnOnce() -> io::Result<()>, entry_kind: &str, ) -> io::Result<()> { copy_fn()?; - if let Err(err) = check_canceled(cancel) { + // The dest-removing rollback is only safe when the destination did NOT + // previously exist (non-overwrite): there, removing the freshly-copied dest + // restores the pre-move state. For an overwrite move, `publish_temp_dir`/ + // `swap_temp_to_dest` have already replaced and deleted the original dest by + // the time `copy_fn` returns `Ok`, so a completed copy is the point of no + // return β€” rolling back here would delete the only remaining copy and leave + // nothing. In that case skip the rollback and proceed to remove the source. + if !overwrite && let Err(err) = check_canceled(cancel) { if let Err(rollback_err) = rollback_fn() { return Err(io::Error::other(format!( "cross-device move canceled after copy, rollback also failed: \ @@ -185,7 +207,7 @@ fn copy_then_remove_src( return Err(err); } if let Err(del_err) = remove_src_fn() { - if let Err(rollback_err) = rollback_fn() { + if !overwrite && let Err(rollback_err) = rollback_fn() { return Err(io::Error::other(format!( "cross-device move: copied '{}' to '{}' but failed to remove source {}: {}. rollback also failed: {}", src.display(), @@ -227,6 +249,7 @@ mod tests { &src, &dest, &cancel, + false, || { fs::copy(&src, &dest)?; cancel.store(true, Ordering::Relaxed); @@ -262,6 +285,7 @@ mod tests { &src, &dest, &cancel, + false, || fs::copy(&src, &dest).map(|_| ()), || fs::remove_file(&src), || { diff --git a/src/ui/viewer/loader.rs b/src/ui/viewer/loader.rs index d008c90..25752f5 100644 --- a/src/ui/viewer/loader.rs +++ b/src/ui/viewer/loader.rs @@ -14,7 +14,13 @@ use crate::debug_log; const CHAFA_TIMEOUT: Duration = Duration::from_secs(10); const PIPE_READ_LIMIT: u64 = 50 * 1024 * 1024; -const PIPE_JOIN_TIMEOUT: Duration = Duration::from_secs(2); +/// Join budget for the background pipe readers, collected only after the chafa +/// child has already exited (so the writers are closed and `read_to_end` +/// returns as fast as the OS drains the buffers). Matched to `CHAFA_TIMEOUT` +/// rather than a tight 2s so a large pipe or scheduling delay on the reader +/// thread cannot truncate chafa's output into an empty preview. Runs on the +/// background image-preview loader thread, never the UI event loop. +const PIPE_JOIN_TIMEOUT: Duration = CHAFA_TIMEOUT; const CHILD_POLL_INTERVAL: Duration = Duration::from_millis(20); /// Owns a background worker plus its cancellation flag and result channel. diff --git a/src/ui/viewer/search.rs b/src/ui/viewer/search.rs index 9e3c085..e571876 100644 --- a/src/ui/viewer/search.rs +++ b/src/ui/viewer/search.rs @@ -213,7 +213,12 @@ impl ViewerState { let hex_col = HEX_OFFSET_PREFIX_WIDTH + byte_in_line * 3 + if byte_in_line >= 8 { 1 } else { 0 }; let segment_len = remaining.min(bpl - byte_in_line); - let match_hex_len = segment_len * 3 - 1; + // Add one column when the highlighted run spans the 8-byte group gap: + // `format_hex_line_to_buffer` inserts an extra space after byte 8, so a + // match starting before it and ending at/after it must cover that gap + // column too (the start column at line 214 is already offset by it). + let crosses_gap = byte_in_line < 8 && byte_in_line + segment_len > 8; + let match_hex_len = segment_len * 3 - 1 + crosses_gap as usize; if first_segment { self.search_matches.push(SearchMatch { diff --git a/src/ui/viewer/toggle.rs b/src/ui/viewer/toggle.rs index 50feb25..e05f696 100644 --- a/src/ui/viewer/toggle.rs +++ b/src/ui/viewer/toggle.rs @@ -109,8 +109,21 @@ impl ViewerState { let mut new_offsets = Vec::with_capacity(new_heights.len()); let mut acc = 0usize; for &h in &new_heights { - acc += h; - new_offsets.push(acc); + // Guard the cumulative offset against `usize` overflow (only reachable + // for absurd inputs). Storing a wrapped/partial offset would corrupt + // the binary search in `visual_row_to_logical`; instead drop the cache + // and fall back to logical-line scrolling, mirroring the `checked_add` + // guard on the linear-search path in `scroll.rs`. + match acc.checked_add(h) { + Some(v) => { + acc = v; + new_offsets.push(acc); + } + None => { + self.invalidate_visual_cache(); + return; + } + } } *self.render_cache.visual_heights.borrow_mut() = new_heights; *self.render_cache.visual_offsets.borrow_mut() = new_offsets;