fix(isolation): the isolation suite must not itself be ambient

Review round 1: `isolated_construction_is_init_complete` asserted its
paired half — that the *ambient* constructor is unchanged — with an
ambient `EditorState::new()` in an ordinary parent test. That reads the
developer's real `init.lua` and materializes packages into their real
data root: the exposure this suite exists to remove, committed by the
suite itself.

The claim is worth keeping, so it moves rather than dies. It now lives in
the re-exec'd positive control, which runs only as a child under a
hostile-by-construction environment. That is the one place an ambient
constructor is safe, and so it is where every ambient claim this suite
makes belongs.

**The ratchet did not catch this, and that is the more important half.**
`ambient_isolation_acceptance.rs` was on the allowlist for its positive
control, and a bare file-level exemption licenses the named file to grow
new ambient sites forever — which is exactly what happened. So every
exemption now carries its **exact permitted site count**, and a file with
more sites than it was reviewed with fails even while allowlisted. A
count that drops fails too, so the allowlist stays a census rather than
drifting into a ceiling nobody rechecks.

The count immediately earned itself: it rejected the number written from
memory for `journey_acceptance` (47) and reported the real one (26 — 19
`new()` + 7 `open(`, after the scanner drops two assertion-message
mentions and the assembled `concat!` needle).

Verified in both directions: restoring the removed ambient site fails the
ratchet with `2 site(s), allowlist says 1`; and with the ambient half
gone, both init-complete pins still fail under the `if roots.is_ambient()`
mutation, so neither has become a test that passes for the wrong reason.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lv428Fth9LRtffwJSsqH7T
This commit is contained in:
Levi Neuwirth 2026-07-31 19:46:18 -04:00
parent 22925964d9
commit 9ea522f3cc
1 changed files with 67 additions and 18 deletions

View File

@ -91,9 +91,14 @@ fn isolated_construction_is_init_complete() {
"isolated construction must still leave the init phase, or every \ "isolated construction must still leave the init phase, or every \
suite that reopens it (m8_2) breaks" suite that reopens it (m8_2) breaks"
); );
// The paired half: the ambient constructor is unchanged. // The paired half — that the *ambient* constructor is unchanged —
let ambient = EditorState::new(); // deliberately does NOT live here. Asserting it needs an ambient
assert!(ambient.lua_host.is_init_complete()); // `EditorState::new()`, and an ambient construction in an ordinary
// parent test reads the developer's real `init.lua` and materializes
// packages into their real data root: the exact exposure this suite
// exists to remove, committed by the suite itself. It lives in the
// re-exec'd positive control below, where the roots are controlled
// by construction.
} }
/// **N** — the init phase is genuinely closed, not merely reported /// **N** — the init phase is genuinely closed, not merely reported
@ -288,14 +293,30 @@ fn run_child(test_name: &str, env: Vec<(&'static str, PathBuf)>) -> (bool, Strin
/// `init.lua` that never loads under any circumstances would satisfy /// `init.lua` that never loads under any circumstances would satisfy
/// "the isolated editor did not load it" while proving nothing. /// "the isolated editor did not load it" while proving nothing.
/// ///
/// Runs only as a re-exec'd child (marker set), so an ordinary suite run /// **This is the suite's only ambient construction, and it runs only as
/// does not construct an ambient editor. /// a re-exec'd child** (the marker gates it), where the roots are
/// controlled by construction. An ambient `EditorState::new()` in an
/// ordinary parent test would read the developer's real `init.lua` and
/// write their real data root — so the one place that legitimately needs
/// the ambient constructor is also the one place where the environment
/// has already been redirected. Every ambient claim this suite makes
/// belongs here for that reason.
#[test] #[test]
fn ambient_construction_under_a_hostile_environment_is_captured_by_it() { fn ambient_construction_under_a_hostile_environment_is_captured_by_it() {
if std::env::var_os(AMBIENT_CONTROL_CHILD).is_none() { if std::env::var_os(AMBIENT_CONTROL_CHILD).is_none() {
return; return;
} }
let state = EditorState::new(); let state = EditorState::new();
// Relocated from `isolated_construction_is_init_complete`: the
// ambient constructor is unchanged by this lane and still finishes
// initialization. Asserting it needs an ambient construction, which
// is only safe here.
assert!(
state.lua_host.is_init_complete(),
"the ambient constructor must still leave the init phase — the \
roots parameter changes which directory is read, never whether \
the block runs"
);
let ran: bool = state let ran: bool = state
.lua_host .lua_host
.lua() .lua()
@ -409,7 +430,7 @@ fn a_hostile_ambient_environment_is_neither_read_nor_written() {
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
/// Files permitted to construct an editor through the **ambient** entry /// Files permitted to construct an editor through the **ambient** entry
/// points. /// points, **with the exact number of sites each is permitted**.
/// ///
/// `journey_acceptance` is ambient on purpose: it is the golden-journey /// `journey_acceptance` is ambient on purpose: it is the golden-journey
/// ratchet, and its whole claim is that the production entry point /// ratchet, and its whole claim is that the production entry point
@ -417,7 +438,24 @@ fn a_hostile_ambient_environment_is_neither_read_nor_written() {
/// with controlled roots instead (framing §1.10). This file is ambient /// with controlled roots instead (framing §1.10). This file is ambient
/// only inside the positive control above, which never runs except as a /// only inside the positive control above, which never runs except as a
/// deliberately re-exec'd child. /// deliberately re-exec'd child.
const AMBIENT_ALLOWLIST: &[&str] = &["journey_acceptance.rs", "ambient_isolation_acceptance.rs"]; ///
/// **The count is the point, not decoration.** A bare file-level
/// exemption is the weakest form of this ratchet: it licenses the named
/// file to grow *new* ambient sites forever. That is not hypothetical —
/// review round 1 of this PR found an ambient `EditorState::new()` in an
/// ordinary parent test of **this very file**, and the file-level
/// exemption is precisely what let it through a green ratchet. Every
/// exemption is now a census entry, so an added site fails even inside
/// an allowlisted file, and a removed one has to be recorded.
const AMBIENT_ALLOWLIST: &[(&str, usize)] = &[
// 19 `new()` + 7 `open(` — the golden journey, every one of them
// reached only from inside a re-exec'd child. (29 textual
// occurrences; the scanner drops 2 assertion-message mentions and
// the assembled `concat!` needle in the self-source check.)
("journey_acceptance.rs", 26),
// Exactly one: the re-exec'd positive control.
("ambient_isolation_acceptance.rs", 1),
];
/// Strip comments and string-literal *contents* from Rust source, so a /// Strip comments and string-literal *contents* from Rust source, so a
/// scan counts calls rather than mentions. /// scan counts calls rather than mentions.
@ -559,6 +597,7 @@ fn no_test_outside_the_allowlist_constructs_an_ambient_editor() {
concat!("EditorState::", "open("), concat!("EditorState::", "open("),
]; ];
let mut offenders: Vec<String> = Vec::new(); let mut offenders: Vec<String> = Vec::new();
let mut miscounted: Vec<String> = Vec::new();
let mut seen_allowlisted: Vec<&str> = Vec::new(); let mut seen_allowlisted: Vec<&str> = Vec::new();
for (name, src) in &sources { for (name, src) in &sources {
let code = strip_comments_and_strings(src); let code = strip_comments_and_strings(src);
@ -566,18 +605,20 @@ fn no_test_outside_the_allowlist_constructs_an_ambient_editor() {
if hits == 0 { if hits == 0 {
continue; continue;
} }
if AMBIENT_ALLOWLIST.contains(&name.as_str()) { match AMBIENT_ALLOWLIST.iter().find(|(f, _)| *f == name.as_str()) {
seen_allowlisted.push( Some((file, allowed)) => {
AMBIENT_ALLOWLIST seen_allowlisted.push(file);
.iter() // An allowlisted file is exempted for the sites it was
.find(|a| **a == name.as_str()) // reviewed with, not for any it grows later.
.expect("just matched"), if hits != *allowed {
); miscounted.push(format!("{name}: {hits} site(s), allowlist says {allowed}"));
} else { }
offenders.push(format!("{name} ({hits} site(s))")); }
None => offenders.push(format!("{name} ({hits} site(s))")),
} }
} }
offenders.sort(); offenders.sort();
miscounted.sort();
assert!( assert!(
offenders.is_empty(), offenders.is_empty(),
"these suites construct an editor through the ambient entry \ "these suites construct an editor through the ambient entry \
@ -587,11 +628,19 @@ fn no_test_outside_the_allowlist_constructs_an_ambient_editor() {
tests/common/iso.rs), or add the file to AMBIENT_ALLOWLIST with \ tests/common/iso.rs), or add the file to AMBIENT_ALLOWLIST with \
a reason.", a reason.",
); );
assert!(
miscounted.is_empty(),
"allowlisted files whose ambient site count moved: {miscounted:?}\n\
MORE than allowed means a new ambient construction slipped into \
an exempted file the failure mode a bare file-level exemption \
cannot see. FEWER means the census is stale; update the count.",
);
// Dead allowlist entries are how a ratchet rots: an entry that no // Dead allowlist entries are how a ratchet rots: an entry that no
// longer needs to be there silently licenses a future regression. // longer needs to be there silently licenses a future regression.
let mut missing: Vec<&&str> = AMBIENT_ALLOWLIST let mut missing: Vec<&str> = AMBIENT_ALLOWLIST
.iter() .iter()
.filter(|a| !seen_allowlisted.contains(&**a)) .map(|(f, _)| *f)
.filter(|f| !seen_allowlisted.contains(f))
.collect(); .collect();
missing.sort_unstable(); missing.sort_unstable();
assert!( assert!(