From e95e9259ea3544d166802a93b1cd48ae30287088 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 11:39:13 -0400 Subject: [PATCH] Never crash: steering rule, clippy.toml for the no-panic lints AGENTS.md/CLAUDE.md gain a "Never crash" section ahead of the non-negotiables (no unwrap/expect/panic/unreachable/todo/unimplemented or unsafe in non-test code, Result + ?, checked indexing and arithmetic on input-derived values, bounded recursion, lock poisoning, the last-resort guard, a regression test per crash fix), linking craftrules/standards/never-crash.md. README's Contributing list points to it. clippy.toml allows unwrap/expect/panic/indexing in tests only, so crates can carry #![deny(clippy::unwrap_used, ...)] without touching their tests. unsafe_code stays "deny" for now: apps/lightcraft and apps/lightcraft-cli call malloc_zone_pressure_relief (libSystem) through a two-line FFI block to return freed memory on macOS, and no safe wrapper crate exists. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 24 ++++++++++++++++++++++++ CLAUDE.md | 24 ++++++++++++++++++++++++ README.md | 3 +++ clippy.toml | 4 ++++ 4 files changed, 55 insertions(+) create mode 100644 clippy.toml diff --git a/AGENTS.md b/AGENTS.md index 8252ed1be..93aa1b192 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -17,6 +17,30 @@ branches can still break each other (e.g. a new struct field vs. a new construct `git log main..HEAD` + `git status` for unmerged commits or uncommitted work before starting anything new. Commit small and often so a crash loses minutes, not hours. +## Never crash (outranks feature work) +People trust LightCraft with their photo libraries and edits; a crash loses their work. A malformed raw/JPEG/XMP, a bad +command, control or MCP argument, a corrupt catalog or settings file, or a full disk must produce an error the user (or +agent) can act on, never a panic. Don't ship a feature by adding a panic path, and fix a crash before building on top +of it. Full standard: `../craftrules/standards/never-crash.md` +([storytold/craftrules](https://github.com/storytold/craftrules/blob/main/standards/never-crash.md)). +- **Non-test code never panics:** no `unwrap()`, `expect()`, `panic!`, `unreachable!`, `todo!`, `unimplemented!`, and + no `unsafe`. Return the crate's error type through `Result` and `?`; use `ok_or(..)?`, `let … else`, `if let`, or a + fallback (`unwrap_or…`) only where it can't silently corrupt a document. An unfinished feature returns an + "unsupported" error or is disabled. Sole exception: a provably infallible literal, as `#[allow(clippy::expect_used)]` + + `.expect("why it can't fail")`. +- **Input-derived numbers are hostile:** `get()` instead of `[i]`/`[a..b]` for offsets from files, users, agents or + arithmetic on them; checked/saturating math for lengths, offsets and counts; no division by zero, NaN/inf or negative + casts to `usize`; cap allocations sized by input; slice strings only at char boundaries. +- **Bound recursion** (depth limits or seen-sets: IFD chains, nested metadata, collections). **Don't cascade:** + `lock().unwrap_or_else(PoisonError::into_inner)` or an error; worker-thread joins are `Result`s. +- **Last-resort guard:** a panic hook plus `catch_unwind` around command dispatch and import/export turns an escaped + panic into an error dialog and keeps the document. It's a safety net, not a licence; keep `panic = "unwind"` on native. +- **Prove it:** every crash fix lands with a small synthetic regression test that panicked before the fix. +- Enforced by clippy: root `clippy.toml` allows unwrap/expect/panic/indexing in tests only; crates that are clean carry + `#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, + clippy::unreachable)]`, and the lints move to `[workspace.lints.clippy]` once every crate is clean. New crates start + with the attribute. + ## Non-negotiables - **Clean-room.** Never read/disassemble anything inside Adobe app bundles (names/listings only). Never copy Adobe icons, presets, profiles (DCP), lens profiles (LCP), camera matrices, fonts. Observation of the installed Lightroom is read-only (it syncs the user's personal library: never import/edit/rate/delete there). Never copy GPL/LGPL/AGPL code (darktable, RawTherapee, ART, LibRaw, rawspeed, rawloader, rawler, lensfun, dcraw-derived GPL code…). - **Pure Rust** in the product. No C/C++ dependencies. diff --git a/CLAUDE.md b/CLAUDE.md index 8252ed1be..93aa1b192 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -17,6 +17,30 @@ branches can still break each other (e.g. a new struct field vs. a new construct `git log main..HEAD` + `git status` for unmerged commits or uncommitted work before starting anything new. Commit small and often so a crash loses minutes, not hours. +## Never crash (outranks feature work) +People trust LightCraft with their photo libraries and edits; a crash loses their work. A malformed raw/JPEG/XMP, a bad +command, control or MCP argument, a corrupt catalog or settings file, or a full disk must produce an error the user (or +agent) can act on, never a panic. Don't ship a feature by adding a panic path, and fix a crash before building on top +of it. Full standard: `../craftrules/standards/never-crash.md` +([storytold/craftrules](https://github.com/storytold/craftrules/blob/main/standards/never-crash.md)). +- **Non-test code never panics:** no `unwrap()`, `expect()`, `panic!`, `unreachable!`, `todo!`, `unimplemented!`, and + no `unsafe`. Return the crate's error type through `Result` and `?`; use `ok_or(..)?`, `let … else`, `if let`, or a + fallback (`unwrap_or…`) only where it can't silently corrupt a document. An unfinished feature returns an + "unsupported" error or is disabled. Sole exception: a provably infallible literal, as `#[allow(clippy::expect_used)]` + + `.expect("why it can't fail")`. +- **Input-derived numbers are hostile:** `get()` instead of `[i]`/`[a..b]` for offsets from files, users, agents or + arithmetic on them; checked/saturating math for lengths, offsets and counts; no division by zero, NaN/inf or negative + casts to `usize`; cap allocations sized by input; slice strings only at char boundaries. +- **Bound recursion** (depth limits or seen-sets: IFD chains, nested metadata, collections). **Don't cascade:** + `lock().unwrap_or_else(PoisonError::into_inner)` or an error; worker-thread joins are `Result`s. +- **Last-resort guard:** a panic hook plus `catch_unwind` around command dispatch and import/export turns an escaped + panic into an error dialog and keeps the document. It's a safety net, not a licence; keep `panic = "unwind"` on native. +- **Prove it:** every crash fix lands with a small synthetic regression test that panicked before the fix. +- Enforced by clippy: root `clippy.toml` allows unwrap/expect/panic/indexing in tests only; crates that are clean carry + `#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, + clippy::unreachable)]`, and the lints move to `[workspace.lints.clippy]` once every crate is clean. New crates start + with the attribute. + ## Non-negotiables - **Clean-room.** Never read/disassemble anything inside Adobe app bundles (names/listings only). Never copy Adobe icons, presets, profiles (DCP), lens profiles (LCP), camera matrices, fonts. Observation of the installed Lightroom is read-only (it syncs the user's personal library: never import/edit/rate/delete there). Never copy GPL/LGPL/AGPL code (darktable, RawTherapee, ART, LibRaw, rawspeed, rawloader, rawler, lensfun, dcraw-derived GPL code…). - **Pure Rust** in the product. No C/C++ dependencies. diff --git a/README.md b/README.md index 0a8d94a11..7e6c31736 100644 --- a/README.md +++ b/README.md @@ -347,6 +347,9 @@ Humans and agents follow the same rules, so read [AGENTS.md](AGENTS.md) first. T an entry in [assets/ATTRIBUTION.md](assets/ATTRIBUTION.md) added in the same commit. - **Pure Rust**, enforced crate layering, everything is a command, and `cargo xtask ci` green before every commit (one task id per commit). +- **Never crash.** Non-test code returns errors instead of panicking: no `unwrap()`, `expect()`, `panic!` or + `unsafe`, checked indexing on anything derived from input, and a regression test with every crash fix. Details in + [AGENTS.md](AGENTS.md#never-crash-outranks-feature-work). Questions, ideas or a bug you'd like to talk through first? Bring them to [Discord](https://discord.gg/artcraft). diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 000000000..4b6c79f55 --- /dev/null +++ b/clippy.toml @@ -0,0 +1,4 @@ +allow-unwrap-in-tests = true +allow-expect-in-tests = true +allow-panic-in-tests = true +allow-indexing-slicing-in-tests = true