From 4b4c20d9a1e410c4ad3fa4ac5ce55a168782ecce Mon Sep 17 00:00:00 2001 From: Mathih13 Date: Wed, 7 Oct 2026 12:43:28 +0200 Subject: [PATCH 1/2] fix(tripwires): stop pinning a Package file by name in the character fence The raw-character-lookup tripwire hard-coded packages/playerbots/src/mod.rs in its budget list, so a Package change could fail a Core test and Core named a Package in its own source. Replace the per-path entry with PACKAGE_FILE_BUDGET, keyed by a file's path relative to its own Package root (src/mod.rs) instead of which Package ships it, so the same budget applies to any installed Package with no Package name in Core. The source-scan test that used packages/playerbots as an example path now uses a made-up package name. Co-Authored-By: Claude Opus 5.5 (1M context) --- module/build.rs | 8 ++-- module/src/test_scan.rs | 8 ++-- module/src/tripwires.rs | 95 ++++++++++++++++++++++++++++++++--------- 3 files changed, 82 insertions(+), 29 deletions(-) diff --git a/module/build.rs b/module/build.rs index 28a5d5fc5..59ce91abf 100644 --- a/module/build.rs +++ b/module/build.rs @@ -1902,14 +1902,14 @@ mod package_api_lint_tests { #[test] fn the_failure_names_the_package_the_file_the_line_and_the_path() { let message = out_of_surface_message( - "playerbots", - Path::new("packages/playerbots/src/mod.rs"), + "sample_package", + Path::new("packages/sample_package/src/mod.rs"), 618, "crate::auth::create_character", ); - assert!(message.contains("playerbots"), "{message}"); + assert!(message.contains("sample_package"), "{message}"); assert!( - message.contains("packages/playerbots/src/mod.rs:618"), + message.contains("packages/sample_package/src/mod.rs:618"), "{message}" ); assert!( diff --git a/module/src/test_scan.rs b/module/src/test_scan.rs index 0d35732ce..f981e892a 100644 --- a/module/src/test_scan.rs +++ b/module/src/test_scan.rs @@ -298,8 +298,8 @@ pub(crate) fn repo_root() -> std::path::PathBuf { /// The optional directory a repo-relative path belongs to, if any — the ONLY directory whose /// absence licenses skipping that path. /// -/// A drop-in package is optional ONE PACKAGE AT A TIME — a missing `packages/playerbots/src/goals.rs` -/// inside an installed `packages/playerbots/` is a typo, not an uninstalled package — unlike the +/// A drop-in package is optional ONE PACKAGE AT A TIME — a missing `packages/sample_package/src/goals.rs` +/// inside an installed `packages/sample_package/` is a typo, not an uninstalled package — unlike the /// private-only tooling trees below, which are optional WHOLESALE. `packages/` itself is always /// there, in every checkout including the public mirror (it ships the reference Package, /// `packages/example/`), so only an individual OTHER package — `packages/` — can be absent; @@ -430,8 +430,8 @@ mod tests { #[test] fn optional_owner_scopes_a_package_to_its_own_directory() { assert_eq!( - optional_owner("packages/playerbots/src/mod.rs").as_deref(), - Some("packages/playerbots"), + optional_owner("packages/sample_package/src/mod.rs").as_deref(), + Some("packages/sample_package"), "a package's absence is per-package: `packages/` itself carries a checked-in README, so \ keying the skip on `packages/` would wave through a typo'd file inside an INSTALLED \ package" diff --git a/module/src/tripwires.rs b/module/src/tripwires.rs index d4613aa04..5df4169b8 100644 --- a/module/src/tripwires.rs +++ b/module/src/tripwires.rs @@ -1156,9 +1156,16 @@ mod partition_discipline_tripwire { /// The fence is `helpers::character_by_guid` / `character_by_name`, which read an in-transit /// character as ABSENT so each caller's existing "no such character" arm fires. This test /// source-scans the same file set as its two sibling tripwires for RAW character lookups and fails -/// on any file over its whitelisted budget. The whitelist FREEZES today's audited exceptions with -/// a verdict each (see the verdict table in `transfer/mod.rs`'s module doc, and `docs/history/transfer-by-guid-verdict-table.md` for each verdict's reasoning); it is a ratchet, so it should only -/// ever shrink. +/// on any file over its whitelisted budget. `WHITELIST` FREEZES today's audited exceptions in +/// Core's own source, each with a verdict (see the verdict table in `transfer/mod.rs`'s module doc, +/// and `docs/history/transfer-by-guid-verdict-table.md` for each verdict's reasoning); it is a +/// ratchet, so it should only ever shrink. +/// +/// An installed Package's source compiles into this same module, so its files need the same fence +/// — but Core must not hold a literal entry for one Package's file, or Core names a Package. +/// `PACKAGE_FILE_BUDGET` covers that case: it keys a budget on a file's path RELATIVE TO ITS +/// PACKAGE ROOT (`src/mod.rs`, not the full `packages//src/mod.rs`), so the same audited +/// shape applies to whichever Package ships it, with no Package name anywhere in Core. /// // Note: same (file, count) granularity, same text-scan mechanism, and the same two ceilings as // `partition_discipline_tripwire` — swapping one whitelisted lookup for a different one inside an @@ -1172,6 +1179,8 @@ mod character_fence_tripwire { /// `(repo-relative path, allowed raw-lookup count, verdict + why)`. One line each; every entry /// is an audited exception from the by-guid verdict table in `module/src/transfer/mod.rs`. + /// Core's own files only — an installed Package's file is never named here; see + /// `PACKAGE_FILE_BUDGET` below. const WHITELIST: &[(&str, usize, &str)] = &[ ("module/src/account_ownership.rs", 2, "Account fencing reads ownership even during Transfer. These two reads only check the Account name before fencing or removing a live entity; Character rows and Transfer records remain intact."), // THE GATE ITSELF. @@ -1197,9 +1206,58 @@ mod character_fence_tripwire { // Split the former single `debug.rs` into a directory; this lookup lives in the one // reducer the collapse put it in. ("module/src/debug/repair.rs", 1, "`debug_repair_after_publish`'s gm-tester backfill (guid 1, formerly the standalone `debug_seed_gm_tester`); every debug WRITER that touches character state is fenced"), - ("packages/playerbots/src/mod.rs", 4, "bot roster bookkeeping over rows this same reducer just created — free-name probe, post-create fetch, post-update re-read; +1 for `ensure_bot_account`'s per-account character COUNT, which reaches no character (it decides which bot account still has room under the 10-character cap) — an in-transit bot counting or not counting toward that cap is harmless either way"), ]; + /// `(path relative to a Package's own root, allowed raw-lookup count, verdict + why)`. Applies + /// identically to every installed Package: the key is a file SHAPE (`src/mod.rs`), never which + /// Package ships it, so Core carries this budget with no Package name anywhere in it. A Package + /// file whose relative path matches no entry gets a budget of zero, same as an unlisted Core + /// file in `WHITELIST`. + /// + /// The one entry here is the audited exception ticket 2 inherited from the former + /// `packages//src/mod.rs` line in `WHITELIST`: roster bookkeeping over rows the same + /// reducer just created — a free-name probe, a post-create fetch, a post-update re-read, and a + /// per-account character COUNT that reaches no character (it only decides whether an account + /// still has room under its character cap, so an in-transit character counting toward it or not + /// is harmless either way). + /// + /// No staleness ratchet checks this list the way `character_whitelist_has_no_stale_entries` + /// checks `WHITELIST`: a generic entry names a file shape, not one Package, so there is no + /// single installed Package to measure it against. Ticket 3 is already expected to revisit this + /// number when the Package that earns it moves its account and character creation behind the + /// Package API. + const PACKAGE_FILE_BUDGET: &[(&str, usize, &str)] = &[( + "src/mod.rs", + 4, + "roster bookkeeping over rows this same reducer just created — free-name probe, post-create fetch, post-update re-read; +1 for a per-account character COUNT, which reaches no character (it decides whether an account still has room under its character cap) — an in-transit character counting or not toward that cap is harmless either way", + )]; + + /// If `rel` sits under `packages//`, its path relative to that Package's own root — + /// `packages/example/src/mod.rs` → `Some("src/mod.rs")`. `None` for a Core file. + fn package_relative_path(rel: &str) -> Option<&str> { + let after = rel.strip_prefix("packages/")?; + let (_pkg, tail) = after.split_once('/')?; + Some(tail) + } + + /// The raw-lookup budget for `rel`: a Package file is checked against `PACKAGE_FILE_BUDGET` by + /// its path relative to the Package root; a Core file is checked against `WHITELIST` by its + /// full repo-relative path. Either way, an unlisted file gets zero. + fn budget_for(rel: &str) -> usize { + if let Some(pkg_rel) = package_relative_path(rel) { + return PACKAGE_FILE_BUDGET + .iter() + .find(|(p, _, _)| *p == pkg_rel) + .map(|(_, n, _)| *n) + .unwrap_or(0); + } + WHITELIST + .iter() + .find(|(p, _, _)| *p == rel) + .map(|(_, n, _)| *n) + .unwrap_or(0) + } + /// The lookup forms that reach a character row raw. `.guid().find(` and `.name().find(` are the /// indexed point reads; `.iter()` is the case-folding name scan (`character_by_name`'s /// convention) and the account-scoped counts. `.guid().update(` is deliberately NOT here: the @@ -1262,11 +1320,7 @@ mod character_fence_tripwire { let content = std::fs::read_to_string(&file).unwrap_or_else(|e| panic!("cannot read {rel}: {e}")); let found = raw_lookups(&content); - let allowed = WHITELIST - .iter() - .find(|(p, _, _)| *p == rel) - .map(|(_, n, _)| *n) - .unwrap_or(0); + let allowed = budget_for(&rel); if found.len() > allowed { violations.push(format!( "{rel}: {} raw game_character lookup(s) (budget {allowed}) at line(s) {found:?}", @@ -1286,29 +1340,28 @@ mod character_fence_tripwire { absent, so your existing not-found arm fires and no new error string reaches the \ gateway. If refusal is the WRONG answer for your path (it would drop a third party's \ value, or the field is connection-derived and regenerated at the destination), pick the \ - DEFER or REGENERATE verdict instead and add the site to `WHITELIST` (module/src/tripwires.rs) \ - WITH its verdict — see the table in `module/src/transfer/mod.rs`'s module doc. \ - Whitelisted today: {} lookup(s) across {} file(s).", + DEFER or REGENERATE verdict instead and add the site WITH its verdict — to `WHITELIST` \ + (module/src/tripwires.rs) for a Core file, or to `PACKAGE_FILE_BUDGET` for a Package \ + file — see the table in `module/src/transfer/mod.rs`'s module doc. Whitelisted today: \ + {} Core lookup(s) across {} Core file(s), plus {} Package lookup(s) across {} Package \ + file shape(s).", violations.join("\n "), WHITELIST.iter().map(|(_, n, _)| n).sum::(), WHITELIST.len(), + PACKAGE_FILE_BUDGET.iter().map(|(_, n, _)| n).sum::(), + PACKAGE_FILE_BUDGET.len(), ); } - /// Ratchet: an entry whose file no longer needs its full budget must be trimmed, or the budget - /// quietly re-opens the door it was closed for. - /// - /// Same optional-drop-in rule as its sibling: an entry under a `packages//` this checkout - /// does not have is skipped; one under an INSTALLED package that has lost the file still counts - /// zero and fails. + /// Ratchet: a `WHITELIST` entry whose file no longer needs its full budget must be trimmed, or + /// the budget quietly re-opens the door it was closed for. `WHITELIST` holds Core files only + /// (never optional), so every entry is checked unconditionally; `PACKAGE_FILE_BUDGET` carries no + /// matching ratchet — see its own doc comment for why. #[test] fn character_whitelist_has_no_stale_entries() { let root = repo_root(); let mut stale = Vec::new(); for (rel, allowed, _) in WHITELIST { - if !crate::test_scan::is_installed(rel) { - continue; - } let actual = std::fs::read_to_string(root.join(rel)) .map(|c| raw_lookups(&c).len()) .unwrap_or(0); From ae025f9718113e11f977e7ba1b4d66c58ecb905d Mon Sep 17 00:00:00 2001 From: Mathih13 Date: Wed, 7 Oct 2026 12:55:31 +0200 Subject: [PATCH 2/2] refactor(tripwires): trim the character-fence doc comments Review feedback on the new PACKAGE_FILE_BUDGET and module-doc comments: drop the ticket cross-references (no issue/ticket refs in code comments) and cut the prose to the terse style the rest of this file uses. Co-Authored-By: Claude Opus 5.5 (1M context) --- module/src/tripwires.rs | 30 +++++------------------------- 1 file changed, 5 insertions(+), 25 deletions(-) diff --git a/module/src/tripwires.rs b/module/src/tripwires.rs index 5df4169b8..efc0a2edf 100644 --- a/module/src/tripwires.rs +++ b/module/src/tripwires.rs @@ -1159,13 +1159,8 @@ mod partition_discipline_tripwire { /// on any file over its whitelisted budget. `WHITELIST` FREEZES today's audited exceptions in /// Core's own source, each with a verdict (see the verdict table in `transfer/mod.rs`'s module doc, /// and `docs/history/transfer-by-guid-verdict-table.md` for each verdict's reasoning); it is a -/// ratchet, so it should only ever shrink. -/// -/// An installed Package's source compiles into this same module, so its files need the same fence -/// — but Core must not hold a literal entry for one Package's file, or Core names a Package. -/// `PACKAGE_FILE_BUDGET` covers that case: it keys a budget on a file's path RELATIVE TO ITS -/// PACKAGE ROOT (`src/mod.rs`, not the full `packages//src/mod.rs`), so the same audited -/// shape applies to whichever Package ships it, with no Package name anywhere in Core. +/// ratchet, so it should only ever shrink. `PACKAGE_FILE_BUDGET` carries the same budget for an +/// installed Package's file, keyed by file shape instead of by Package, so Core names no Package. /// // Note: same (file, count) granularity, same text-scan mechanism, and the same two ceilings as // `partition_discipline_tripwire` — swapping one whitelisted lookup for a different one inside an @@ -1208,24 +1203,9 @@ mod character_fence_tripwire { ("module/src/debug/repair.rs", 1, "`debug_repair_after_publish`'s gm-tester backfill (guid 1, formerly the standalone `debug_seed_gm_tester`); every debug WRITER that touches character state is fenced"), ]; - /// `(path relative to a Package's own root, allowed raw-lookup count, verdict + why)`. Applies - /// identically to every installed Package: the key is a file SHAPE (`src/mod.rs`), never which - /// Package ships it, so Core carries this budget with no Package name anywhere in it. A Package - /// file whose relative path matches no entry gets a budget of zero, same as an unlisted Core - /// file in `WHITELIST`. - /// - /// The one entry here is the audited exception ticket 2 inherited from the former - /// `packages//src/mod.rs` line in `WHITELIST`: roster bookkeeping over rows the same - /// reducer just created — a free-name probe, a post-create fetch, a post-update re-read, and a - /// per-account character COUNT that reaches no character (it only decides whether an account - /// still has room under its character cap, so an in-transit character counting toward it or not - /// is harmless either way). - /// - /// No staleness ratchet checks this list the way `character_whitelist_has_no_stale_entries` - /// checks `WHITELIST`: a generic entry names a file shape, not one Package, so there is no - /// single installed Package to measure it against. Ticket 3 is already expected to revisit this - /// number when the Package that earns it moves its account and character creation behind the - /// Package API. + /// `(path relative to a Package's own root, allowed raw-lookup count, verdict + why)` — keyed + /// this way so Core names no Package. No staleness ratchet: it names a file shape, not one + /// installed Package, so there is no single file to measure it against. const PACKAGE_FILE_BUDGET: &[(&str, usize, &str)] = &[( "src/mod.rs", 4,