git.lucas.co / cce-designer
graphic design tool
git clone https://git.lucas.co/cce-designer.git

commit4646007e842602be72d30efb1dfd7ff6a32f0b20
parent113bac64f5
authorLucas Galante <[email protected]>
date2026-09-23 12:54
test: refuse source that builds a path the user owns

`cargo test` had been rewriting the user's own state.kdl on every run, and
leaving scratch files at fixed /tmp names that every other user of the machine
shares. Both are fixed; neither had anything stopping them coming back.

`tests/user_paths.rs` scans the crate's source for the two shapes. No line may
assemble a config path out of `.config` by hand — `cce_ui::config::
cce_config_dir()` is the only way in, because that is what the `cfg(test)`
redirect keys off — and every `temp_dir()` must be scoped with
`std::process::id()` within a line or two of the call, which may wrap. It
lives in `tests/` for the reason `doc_claims.rs` gives about its own scans: a
scanner under `src/` is the first thing it finds, and from out here the
needles can be written plainly. Verified by reintroducing each bug in
`app.rs`: both are caught, each naming the file, the line and what to do
instead.

The `temp_dir()` rule deliberately covers shipped code too, not just tests: a
fixed /tmp name is no better there. A site that genuinely needs an unscoped
name should earn a line in the rule saying why.

A scan cannot see behaviour, so `the_recent_files_list_is_not_the_users`
covers the other half of the recent-files fix, which had no test at all: it
asserts the load comes back empty — a suite that read the real list would
assert against whatever projects happen to be on the machine running it — and
that exercising `add_recent_file` leaves the real file untouched. The two
halves are separate `cfg!(test)` gates in separate functions, so one can be
removed without the other.

Co-Authored-By: Claude Opus 5 <[email protected]>

 CLAUDE.md           |  30 +++++++++++++--
 src/main.rs         |  36 ++++++++++++++++++
 tests/user_paths.rs | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 170 insertions(+), 3 deletions(-)

diff --git a/CLAUDE.md b/CLAUDE.md
index b4a2589..3f2bba8 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -1324,6 +1324,30 @@ name and every wire to them ambiguous.
 ## Repo hygiene
 
 `scratch/` holds ad-hoc debug scripts/logs and `screenshot*.png` at the root are
-debugging artifacts — not source, don't extend them. All tests live in
-`src/main.rs`'s `#[cfg(test)]` module; add new ones there. Commit messages follow
-`feat:` / `fix:` / `refactor:` style (see `git log`).
+debugging artifacts — not source, don't extend them. Tests live in
+`src/main.rs`'s `#[cfg(test)]` module; add new ones there. The exception is
+`tests/`, which holds the two tests that SCAN the crate's own source —
+`doc_claims.rs` (CLAUDE.md's `(~Nk lines)` figures) and `user_paths.rs` (below)
+— and they are out there because a scanner under `src/` is the first thing it
+finds. Both are deliberately mirrored per crate rather than shared, since every
+crate here is its own git repository that must build standalone; they need
+nothing but `std`, so copying one into a sibling is the whole job. Commit
+messages follow `feat:` / `fix:` / `refactor:` style (see `git log`).
+
+**`tests/user_paths.rs` refuses source that builds a path the user owns**, the
+class of bug that had this suite rewriting `~/.config/cce/cce-designer/state.kdl`
+on every run (see "App-written settings" above). Two rules, one per shape that
+actually shipped: no line assembles a config path out of `.config` by hand —
+`cce_ui::config::cce_config_dir()` is the only way in, because that is what the
+`cfg(test)` redirect keys off — and every `temp_dir()` is scoped with
+`std::process::id()` within a line or two, since /tmp is one namespace shared
+with every other user and every concurrent run. Verified by reintroducing each
+bug: both are caught, naming the file and line. The `temp_dir()` rule is not
+limited to tests, because a fixed /tmp name is no better in shipped code.
+
+Two runtime guards sit alongside it in `src/main.rs`, since a scan cannot see
+behaviour: `the_suite_does_not_write_the_users_own_settings` and
+`the_recent_files_list_is_not_the_users` each snapshot the real file, exercise
+the write path, and assert it did not move — the second checking the load side
+too, because reading the user's recent list would make the suite's behaviour
+depend on the machine.
diff --git a/src/main.rs b/src/main.rs
index 29148fd..043c985 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -5142,6 +5142,42 @@ mod tests {
         );
     }
 
+    /// The recent-files list is not the user's, under test.
+    ///
+    /// The toolkit derives that path from the EXE's basename, so each test
+    /// binary wrote a real `~/.config/cce/cce_designer-<hash>/` of its own —
+    /// seven had accumulated by 2026-09-23. Reading was no safer than
+    /// writing: a test that loaded the real list would assert against
+    /// whatever projects happen to be on the machine running it.
+    ///
+    /// Both halves are asserted because they fail differently — a load that
+    /// reached the real file makes this suite's behaviour depend on the
+    /// machine, a save leaves a directory behind on it — and because the
+    /// gate is one `cfg!(test)` in each of two functions, so one can be
+    /// removed without the other.
+    #[test]
+    fn the_recent_files_list_is_not_the_users() {
+        assert!(
+            State::load_recent_files().is_empty(),
+            "the suite loaded the real recent-files list"
+        );
+
+        let real = cce_ui::config::get_app_recent_files_path();
+        let before = std::fs::read(&real).ok();
+
+        let mut state = State::new(false);
+        state.add_recent_file(
+            std::env::temp_dir().join(format!("cce-designer-recent-{}", std::process::id())),
+        );
+
+        assert_eq!(
+            std::fs::read(&real).ok(),
+            before,
+            "{} changed — a test wrote a real recent-files list",
+            real.display()
+        );
+    }
+
     /// The suite runs on a lattice of its own, not the machine's.
     ///
     /// `configured_grid_geometry` read `style.surface.graph.spacing_x` and
diff --git a/tests/user_paths.rs b/tests/user_paths.rs
new file mode 100644
index 0000000..b0d50f8
--- /dev/null
+++ b/tests/user_paths.rs
@@ -0,0 +1,107 @@
+//! The crate's own source, checked for paths that belong to the user.
+//!
+//! An integration test on purpose, for the reason `doc_claims.rs` gives about
+//! its own scans: anything under `src/` is counted by them, and a scanner
+//! living there is the first thing it finds. From out here the needles can be
+//! written plainly, because this file is not in what it reads.
+//!
+//! Deliberately MIRRORED per crate rather than shared from a helper: each is
+//! its own git repository that must build standalone, and this needs nothing
+//! but std. Same call `doc_claims.rs` makes.
+
+pub mod user_paths {
+    use std::path::{Path, PathBuf};
+
+    /// Every `.rs` file under `src/`, as (path, contents).
+    fn sources(root: &Path) -> Vec<(PathBuf, String)> {
+        fn walk(dir: &Path, out: &mut Vec<(PathBuf, String)>) {
+            let Ok(entries) = std::fs::read_dir(dir) else { return };
+            for e in entries.flatten() {
+                let p = e.path();
+                if p.is_dir() {
+                    walk(&p, out);
+                } else if p.extension().and_then(|x| x.to_str()) == Some("rs") {
+                    if let Ok(s) = std::fs::read_to_string(&p) {
+                        out.push((p, s));
+                    }
+                }
+            }
+        }
+        let mut v = Vec::new();
+        walk(&root.join("src"), &mut v);
+        v
+    }
+
+    /// Prose says what the code used to do, so only code counts.
+    fn is_comment(line: &str) -> bool {
+        let t = line.trim_start();
+        t.starts_with("//") || t.starts_with("*")
+    }
+
+    /// `cargo test` must not touch anything the user owns. Two shapes of that
+    /// shipped from this crate, and this is the scan that would have caught
+    /// either the day it was written.
+    ///
+    /// **A config path built by hand.** `DesignSettings::file_path` spelled
+    /// out `$HOME/.config/cce/cce-designer/state.kdl`. `State::new` loads the
+    /// bundled project, whose meta subnets overwrite the live viewport flags,
+    /// so every test that reached `save_settings` wrote the bundled project's
+    /// show_grid / show_cube / show_origin over the user's own. The suite
+    /// reset three of their toggles on every run and stayed green. The path
+    /// goes through `cce_ui::config::cce_config_dir()` now, which is what the
+    /// `cfg(test)` redirect keys off — so hand-assembling one out of
+    /// `.config` is precisely the regression to refuse, and going through the
+    /// toolkit is the only way in.
+    ///
+    /// **A scratch path shared with the rest of the machine.** Two tests kept
+    /// their files at fixed `/tmp` names. /tmp is one namespace shared by
+    /// every user — the point `../cce-compositor/WORKSPACE.md` makes about
+    /// `cce_runtime_dir` — so a fixed name belongs to whoever ran first and
+    /// the sticky bit denies it to everyone else; nearer to hand, two
+    /// checkouts running their suites at once shared one directory. Scoping
+    /// by pid is the crate's own convention, followed at every other site.
+    ///
+    /// The rule is on `temp_dir()` rather than on tests alone because a fixed
+    /// /tmp path is no better in shipped code; if one ever needs an unscoped
+    /// name it should earn a line here saying why.
+    #[test]
+    fn no_source_builds_a_path_the_user_owns() {
+        let root = Path::new(env!("CARGO_MANIFEST_DIR"));
+        let files = sources(root);
+        assert!(!files.is_empty(), "found no sources under src/ — the walk is broken");
+
+        let mut bad: Vec<String> = Vec::new();
+        for (path, src) in &files {
+            let name = path.strip_prefix(root).unwrap_or(path).display();
+            let lines: Vec<&str> = src.lines().collect();
+            for (i, line) in lines.iter().enumerate() {
+                if is_comment(line) {
+                    continue;
+                }
+                if line.contains("\".config\"") || line.contains("/.config") {
+                    bad.push(format!(
+                        "{name}:{}: builds a config path by hand — go through \
+                         cce_ui::config::cce_config_dir(), which is what the \
+                         cfg(test) redirect keys off:\n      {}",
+                        i + 1,
+                        line.trim()
+                    ));
+                }
+                // The call may wrap, so the scoping can be a line or two down.
+                if line.contains("temp_dir()") {
+                    let window = lines[i..lines.len().min(i + 3)].join(" ");
+                    if !window.contains("process::id()") {
+                        bad.push(format!(
+                            "{name}:{}: a scratch path shared with every other \
+                             user and every concurrent run — scope it with \
+                             std::process::id():\n      {}",
+                            i + 1,
+                            line.trim()
+                        ));
+                    }
+                }
+            }
+        }
+        assert!(bad.is_empty(), "source builds paths the user owns:\n  {}", bad.join("\n  "));
+    }
+}