From d9d7caca22eedc02beef6b0260c6dfc7e5c7b1c6 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 08:14:16 -0400 Subject: [PATCH 1/2] =?UTF-8?q?Agent=20rules:=20never=20panic=20(AGENTS.md?= =?UTF-8?q?=20=C2=A74,=20CLAUDE.md=20non-negotiables)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Untrusted input (PDFs, scripts, MCP calls, settings) must yield errors, not crashes: Result over unwrap/expect, no input-derived indexing, checked arithmetic, capped allocations, bounded recursion, lock-poison handling, and a regression test for every crash fix. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 13 ++++++++++++- CLAUDE.md | 7 +++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 9d18e662b..ca6831b41 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -91,6 +91,17 @@ If you find an asset that breaks these rules, stop and remove it from the reposi - **Track parity honestly.** Every user-facing feature has an entry in `parity/acrobat-features.toml`. Mark it `shipped` only when it works end to end and the entry cites a passing test (`evidence`); `cargo xtask parity` (part of `xtask ci`) rejects claims whose commands, tools or tests don't exist. - **Use it to test.** Agents should drive the engine through `printcraft-cli run --script` or the MCP tools, and look at `page_render` output, to check their own changes. For UI changes, launch the app with `--control` and use `printcraft-cli ui` (`inspect`, `click`, `screenshot`) to check the real interface. -## 4. Everything else +## 4. Never crash + +PrintCraft opens files from strangers. A malformed PDF, a bad tool argument or a corrupt settings file must produce an error the user (or agent) can act on, never a crash. + +- **Fail with `Result`.** Non-test code returns errors through the crate's error type and `?`. Where readers are lenient by design, fall back gracefully and record the repair (see Fidelity in `CLAUDE.md`). +- **No panicking shortcuts.** No `unwrap()`, `expect()`, `panic!`, `unreachable!`, `todo!` or `unimplemented!` outside tests, unless the call is provably infallible (e.g. writing to a `Vec`), with a comment saying why. `assert!` only for internal invariants that input cannot reach. +- **Treat every input-derived number as hostile.** Use `get()` instead of `[i]` or `[a..b]`. Slice strings only at char boundaries. Use checked/saturating arithmetic for lengths, offsets and counts. Guard against division by zero and NaN/inf casts, and never cast a negative value to `usize`. Cap allocations sized by the file. +- **Bound recursion.** Object graphs can be cyclic or deeply nested. Walk them with seen-sets or depth limits. +- **Don't cascade.** Handle lock poisoning, and keep a panic in a worker thread from taking the app down. Engine entry points that run untrusted-document code keep their last-resort `catch_unwind` guard. It is a safety net, not a substitute for the rules above. +- **Prove it.** Every crash fix comes with a small synthetic regression test that panicked before the fix. Turn fuzzer findings (`cargo xtask fuzz`) into such tests before fixing them. + +## 5. Everything else Follow `CLAUDE.md`. It covers quality gates, layering, commit rules and how to run and look at the app. diff --git a/CLAUDE.md b/CLAUDE.md index 2af43a8f3..6e46da0d2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -19,6 +19,13 @@ PrintCraft is a clean-room, open-source, Rust-native PDF application targeting A - **Privacy.** When observing Acrobat, use synthetic fixtures only. Never capture the Home view, recent files or account info. Capture by window id (`plan/acrobat/tools/`). Never commit Acrobat outputs, corpus files or personal data. - **Layering.** Nothing below L7 depends on egui/winit/eframe/rfd (`plan/architecture.md` §3). `cos`/`filters`/`crypt`/`arlington` stay standalone. - **Fidelity.** The PDF object graph is the model. Preserve unknown data. Saves are incremental unless a full rewrite is required. Never silently drop or repair data without recording it. +- **Never panic.** Every PDF, script, MCP call, settings file and keystroke is untrusted input, and none of it may crash the app. Return `Result` (the crate's error enum) and propagate with `?`, or fall back leniently and record it. In non-test code: + - No `unwrap`/`expect`/`panic!`/`unreachable!`/`todo!` unless it is provably infallible, with a comment saying why. + - No indexing or slicing with input-derived positions (use `get`, and slice strings only at char boundaries). + - Use checked or saturating arithmetic on input-derived numbers, and guard against division by zero and NaN/inf casts. + - Cap allocations sized by input, and bound recursion with depth limits or seen-sets. + - Handle lock poisoning. + - Every crash fix gets a synthetic regression test. See `AGENTS.md` §4. - **Rust only** in the product and build (`xtask`). No handwritten JS/TS. - **Quality gates** before every commit: `cargo fmt --check`, `cargo clippy --workspace -- -D warnings`, `cargo test --workspace`, and the wasm check once `xtask ci` exists (M0). - **Commits:** one task id per commit (e.g. `M1.4: xref stream reader`). Commit only green states. End messages with the attribution line required by the environment. From 554969da7280ee11415f1f0d25cf45f997890216 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 10:59:39 -0400 Subject: [PATCH 2/2] Never crash: forbid unsafe, clippy.toml for tests, guard and craftrules link MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follows the owner's AGENTS.md §4 / CLAUDE.md rules with what the shared standard (craftrules/standards/never-crash.md) adds: why (lost work) and that it outranks feature work, the `unsafe` ban (workspace `unsafe_code = "forbid"`; no unsafe exists outside vendor/), the last-resort guard around commands and file import/export, the clippy ratchet attribute, and a root clippy.toml that lets tests use unwrap/expect/panic/indexing. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 6 ++++-- CLAUDE.md | 5 +++-- Cargo.toml | 2 +- clippy.toml | 4 ++++ 4 files changed, 12 insertions(+), 5 deletions(-) create mode 100644 clippy.toml diff --git a/AGENTS.md b/AGENTS.md index ca6831b41..021ccd248 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -93,14 +93,16 @@ If you find an asset that breaks these rules, stop and remove it from the reposi ## 4. Never crash -PrintCraft opens files from strangers. A malformed PDF, a bad tool argument or a corrupt settings file must produce an error the user (or agent) can act on, never a crash. +PrintCraft opens files from strangers, and people trust it with their work. A malformed PDF, a bad tool argument, a corrupt settings file or a full disk must produce an error the user (or agent) can act on, never a crash and never lost work. **This outranks feature work:** don't ship a feature by adding a panic path, and fix a crash before building on top of it. The shared standard for every Crafting App is [`craftrules/standards/never-crash.md`](https://github.com/storytold/craftrules/blob/main/standards/never-crash.md). - **Fail with `Result`.** Non-test code returns errors through the crate's error type and `?`. Where readers are lenient by design, fall back gracefully and record the repair (see Fidelity in `CLAUDE.md`). - **No panicking shortcuts.** No `unwrap()`, `expect()`, `panic!`, `unreachable!`, `todo!` or `unimplemented!` outside tests, unless the call is provably infallible (e.g. writing to a `Vec`), with a comment saying why. `assert!` only for internal invariants that input cannot reach. +- **No `unsafe`.** The workspace sets `unsafe_code = "forbid"`. If a platform API needs it, use a maintained safe wrapper crate; a real exception needs the owner's sign-off. - **Treat every input-derived number as hostile.** Use `get()` instead of `[i]` or `[a..b]`. Slice strings only at char boundaries. Use checked/saturating arithmetic for lengths, offsets and counts. Guard against division by zero and NaN/inf casts, and never cast a negative value to `usize`. Cap allocations sized by the file. - **Bound recursion.** Object graphs can be cyclic or deeply nested. Walk them with seen-sets or depth limits. -- **Don't cascade.** Handle lock poisoning, and keep a panic in a worker thread from taking the app down. Engine entry points that run untrusted-document code keep their last-resort `catch_unwind` guard. It is a safety net, not a substitute for the rules above. +- **Don't cascade.** Handle lock poisoning, and keep a panic in a worker thread from taking the app down. Engine entry points that run untrusted-document code keep their last-resort `catch_unwind` guard, and command execution and file import/export must sit behind one that turns an escaped panic into an error message and keeps the user's document. Keep `panic = "unwind"` (the default) so the guard works. It is a safety net, not a substitute for the rules above. - **Prove it.** Every crash fix comes with a small synthetic regression test that panicked before the fix. Turn fuzzer findings (`cargo xtask fuzz`) into such tests before fixing them. +- **Enforced by clippy.** Root `clippy.toml` lets tests use `unwrap`/`expect`/`panic!`. A crate that is clean carries `#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]`; new crates start with it. ## 5. Everything else diff --git a/CLAUDE.md b/CLAUDE.md index 6e46da0d2..9e5e674f4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -19,13 +19,14 @@ PrintCraft is a clean-room, open-source, Rust-native PDF application targeting A - **Privacy.** When observing Acrobat, use synthetic fixtures only. Never capture the Home view, recent files or account info. Capture by window id (`plan/acrobat/tools/`). Never commit Acrobat outputs, corpus files or personal data. - **Layering.** Nothing below L7 depends on egui/winit/eframe/rfd (`plan/architecture.md` §3). `cos`/`filters`/`crypt`/`arlington` stay standalone. - **Fidelity.** The PDF object graph is the model. Preserve unknown data. Saves are incremental unless a full rewrite is required. Never silently drop or repair data without recording it. -- **Never panic.** Every PDF, script, MCP call, settings file and keystroke is untrusted input, and none of it may crash the app. Return `Result` (the crate's error enum) and propagate with `?`, or fall back leniently and record it. In non-test code: +- **Never panic.** Every PDF, script, MCP call, settings file and keystroke is untrusted input, and none of it may crash the app or lose the user's work. This outranks feature work. Return `Result` (the crate's error enum) and propagate with `?`, or fall back leniently and record it. In non-test code: - No `unwrap`/`expect`/`panic!`/`unreachable!`/`todo!` unless it is provably infallible, with a comment saying why. - No indexing or slicing with input-derived positions (use `get`, and slice strings only at char boundaries). - Use checked or saturating arithmetic on input-derived numbers, and guard against division by zero and NaN/inf casts. - Cap allocations sized by input, and bound recursion with depth limits or seen-sets. - Handle lock poisoning. - - Every crash fix gets a synthetic regression test. See `AGENTS.md` §4. + - No `unsafe` (`unsafe_code = "forbid"`). + - Every crash fix gets a synthetic regression test. See `AGENTS.md` §4 and `craftrules/standards/never-crash.md`. - **Rust only** in the product and build (`xtask`). No handwritten JS/TS. - **Quality gates** before every commit: `cargo fmt --check`, `cargo clippy --workspace -- -D warnings`, `cargo test --workspace`, and the wasm check once `xtask ci` exists (M0). - **Commits:** one task id per commit (e.g. `M1.4: xref stream reader`). Commit only green states. End messages with the attribution line required by the environment. diff --git a/Cargo.toml b/Cargo.toml index b112b06d5..238095f3c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -66,7 +66,7 @@ egui_kittest = "0.36" rfd = "0.17" [workspace.lints.rust] -unsafe_code = "deny" +unsafe_code = "forbid" [workspace.lints.clippy] dbg_macro = "warn" 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