[wshallwshall]

BACKLOG #2596 (steps 1 partly, 2, 5). Batch 437. Source branch b437-2596-snapshot-loader, head 02c1bc98b57bf453694e96db577ff07bdc432750. Own PR because it changes a security control (what an approval binds, and what a sandbox worker runs).

What changed

• messagefoundry/config/snapshot.py: ConfigSnapshot / read_config_snapshot read the config directory once, after the trust check. __pycache__ is never read.
• load_config compiles every module and _ helper from the snapshot bytes (compile+exec, not spec_from_file_location). Code sets and connections.toml parse from the same bytes. The snapshot rides on the graph as Registry.config_snapshot.
• Fingerprint scheme mefor-cfg-fp:v3 folds the snapshot. A code-set table matches by lower-case suffix (codesets/x.CSV is hashed on POSIX; #3535 item 1), and the resolved-path dedup is gone, so a symlinked _ helper changes the digest (#3535 item 2).
• engine.reload_detail reads one strict snapshot, checks the approval against it, loads it, and records its digest. The post-load re-checks and _require_unchanged_code are gone.
• Sandbox: the boot frame carries the snapshot (each code-set table once) instead of config_dir; the worker runs load_snapshot and reads no files. A session with no snapshot reads it before the child starts. The child never refuses on its own (ADR 0087).
• Snapshot names are checked as plain names on the platform (Windows refuses \ and drive names); load diagnostics read the bytes that ran.
• ADR 0041 D1: dated amendment for the scheme change. No ADR number allocated.

Not done (remainder stays on the row)

• Step 1's value file under [environments].base_dir is still read by the serve provider, outside the snapshot.

QA

QA -- korus roles/BUILDER.md step 11
Level: xhigh, from the brief. Tag: xhigh effort → 10 inline angles → dedup (no verify) → sweep → ≤15 findings (forks af5659bbe00686cc8 round-9063c00789, ad0153f88547775c3 round-a271b7cefa); review_tag.py exit 0 both rounds.
Rounds: 2. Findings: 13 confirmed and fixed; 7 rejected; 9 open, filed nowhere yet: 6 in the PR scope, 3 outside it.

The review covers head e33f5fbc1, which holds the round-1 build (Builder 1) plus the review round's 8 commits (Builder 2). Each fix has a regression test in tests/test_config_snapshot.py.

Rejected (quality or bounded): environments bytes held in memory; a file deleted mid-read drops from the snapshot (the one that runs is the one hashed); directory listed twice; validate_config duplicating trust-then-read; fingerprint branches kept for one-argument test fakes.

Round 3: semgrep mf-no-eval-exec cleared (commits 1671d17, d1cc2bc, 02c1bc9)

The one literal exec(code, module.__dict__) the branch added (in _HelperImporter.run) is replaced by _SnapshotLoader(importlib.abc.InspectLoader): config modules run through spec.loader.exec_module, as on origin/main, with the code object compiled from the snapshot bytes. Config modules are code-first by design (CLAUDE.md section 1); only where the bytes come from changed. No nosemgrep, no edit to .semgrep/ or the workflow. The loader serves one code object then drops the bytes (runpy and importlib.reload stay refused), serves no source, has no bytecode methods, and keeps the compile-wall error wording. Semgrep 1.172.0 run locally with CI's command: red reproduced on e33f5fb, 0 findings on 02c1bc9.

QA -- korus roles/BUILDER.md step 11
Level: xhigh, from the brief. Tag: xhigh effort → 10 inline angles → dedup (no verify) → sweep → ≤15 findings (forks afa39985f5aea59b3 round-b5ab9b6663, a9aab16a3f799e4d9 round-bbbd0862df); review_tag.py exit 0 both rounds (korus origin/main 20af3e604237db7293729b7048dae3657b738b56, extracted by git show and run by path from the engine worktree).
Rounds: 2. Findings: 8 confirmed and fixed (compile-wall misclassification, linecache stale source, docstring claim, wrong spec comment, reload test guarding nothing, runpy re-run opened, byte lookup moved outside the try, weak compile-test assertion); 3 rejected (dead guard in run kept because it narrows the type for mypy and an assert would trip bandit; SAST visibility, since an exemption is the owner's call per the brief, and DANGEROUS-FUNCTIONALITY section 2 still names the site; linecache entry retention, resolved by the one-shot loader); 1 open.

This round reviewed the delta e33f5fb..02c1bc9; rounds 1 and 2 above cover everything up to e33f5fb. Round-3 checks: ruff, mypy four legs, 67 related test files (2422 passed, 32 skipped), pre-commit including bandit. Full suite not finished locally.

Open findings (for the Lander to file; two change behaviour)

In scope:

1. Behaviour change (fail-closed): an unreadable _ helper that no module imports now refuses the whole load, where before it only left the fingerprint partial.
2. Behaviour change (audit classification): on an approved release the trust check now runs before the approval digest. A tamper that also loosens permissions is still refused, but it is audited as an invalid config rather than digest_mismatch; the test pinning the old classification was removed in round 1. Manager reading: the refusal is unchanged and the earlier check is the stronger one, so no control is weakened; only the audit label moves. Medium confidence; a ruling on whether the label must be kept is the owner's.
3. Config modules get no loader, so pkgutil.get_data and importlib.resources stop working for config code.
4. No bytecode cache: every load recompiles every module.
5. Base64 overhead for modules in the boot frame.
6. macOS sidecar case matching.

Outside scope:
7. capture_start_provenance still re-reads the directory instead of using the snapshot that ran (#2597 area).
8. The dry-run config_reload_check row in api/app.py re-reads the directory too.
9. The step-1 value file (stays on the row).
10. (Round 3) module.__spec__.cached still resolves to a __pycache__ path while __cached__ is None (already true at e33f5fb). Nothing reads it today, but Python 3.15 reads spec.cached. Finding 3 (pkgutil.get_data) was not restored: a snapshot-only get_data would serve a subset of what disk did.

Checks

• Ran: ruff check + format, mypy all four legs (final tree); targeted pytest of about 1,050 tests (loader, code-set, fingerprint, sandbox, codec, parity, dual control, start alert, passthrough, reference sets, config snapshot), all pass; after round 2 the nine affected suites again, 594 passed.
• Full suite on Windows once, mid-edit: 16 failed, 51,882 passed. 13 fixed or flaky-and-pass-on-rerun; test_gate_installed_parity and test_installed_coord_hooks x2 are installed-hook drift on the Builder's machine, not this change. Not re-run in full after the fixes.
• Not run: Linux, Postgres, SQL Server legs.
• Legs to read: Linux test leg (test_a_posix_module_named_with_a_backslash_loads runs only there; codesets/x.CSV discriminates only there; the symlink digest test may skip on Windows). Postgres and SQL Server legs (test_passthrough, test_reference_sets). windows-service-smoke.

Proposed vault banner (the Lander's to flip)

PARTIAL -- steps 2 and 5 MET, step 1 MET EXCEPT THE VALUE FILE, step 3 MET before (engine PR 2239), step 4 read and holds. Step 1: load_config reads one ConfigSnapshot after the trust check and compiles every module and helper from it (no pycache consulted); code sets and connections.toml parse from it; fingerprint scheme v3 folds it; a reload checks an approval against the snapshot it loads. Remaining: the value file under [environments].base_dir is not in the snapshot. Step 2: the boot frame carries the snapshot and the worker never reads the directory. Step 5: probe tests in tests/test_config_snapshot.py, shown failing on unfixed origin/main. A second code-review round fixed 13 findings. Also closes #3535 items 1 and 2.

Merge conflicts expected

Open PRs 2539 and 2532 touch wiring.py / engine.py; PR 2567 (#3234, same batch) touches wiring.py outside the loader region.

Confidence Score: 5/5

The reviewed changes appear safe to merge; no additional actionable defects were found.

Summary

Config loads now compile and parse one captured set of bytes. Reload approvals and reload fingerprints use that snapshot, and sandbox workers receive its bytes instead of reopening the directory.

• Config loads run from one snapshot of the files they read.
• Approved reloads check and load the same config snapshot.
• Sandbox workers run the config snapshot sent by the engine.

Already acknowledged by wshallwshall, and not reported again:

• Unreadable unused helpers now refuse loads; trust checks can change the audit label for tampering.
• Config modules have no loader; loads recompile modules; boot frames add base64 overhead; macOS sidecar case matching remains open.
• Startup provenance and the API dry-run audit can reread the directory. The external environment value file remains outside the snapshot.
• Keeping environment bytes in memory, dropping files deleted during reading, listing directories twice, duplicating the validator’s trust/read steps, and retaining branches for one-argument test fakes were explicitly rejected or bounded.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Config directory] --> B[Trust check]
  B --> C[Read ConfigSnapshot]
  C --> D[Check approval when required]
  D --> E[Compile modules and parse data]
  E --> F[Registry carries snapshot]
  F --> G[Reload fingerprint from snapshot]
  F --> H[Encode sandbox boot frame]
  H --> I[Worker loads supplied bytes]

Loading

_{Reviews (1) · Last reviewed commit: "A vanished directory stays missing, and ..." · Reviewed by Greptile}

[wshallwshall]

Lander: ready for you. Code review ran at xhigh, 2 rounds (QA line in the body), covering head e33f5fb. Nine open findings are listed for filing; items 1 and 2 change behaviour (item 2 is an audit-label change with a Manager reading). Read the Linux, Postgres and SQL Server legs.

[wshallwshall]

Lander: do not merge yet. semgrep (project SAST rules) is red on e33f5fb: mf-no-eval-exec fires on the new compile+exec in messagefoundry/config/wiring.py. A Builder is replacing it with an in-memory importlib loader (no exemption) and will re-review the delta; I will post the new head and its QA line here.

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

[wshallwshall]

Lander: hold lifted. New head 02c1bc9 replaces the literal exec with an importlib InspectLoader (no exemption); semgrep shows 0 findings locally with CI's command. The delta e33f5fb..02c1bc9 was code-reviewed at xhigh, 2 rounds; QA line in the body under Round 3. Read semgrep, repo-scan, the pytest legs (Linux, Postgres, SQL Server) and CI gate on this head.

[wshallwshall]

Lander (session 8550b02b), 2026-10-27 ~13:55Z. Held at 02c1bc98b5 for an owner ruling on body items 1, 2 and 3, which all change behaviour: 1 is fail-closed on an unreadable unused helper; 2 moves the tamper audit from digest_mismatch to invalid config, and the row loses its digest fields; 3 means config code gets no get_data/importlib.resources. Whatever the ruling, one fix is owed now: docs/SECURITY.md (~2330-2345) still says "After a load that succeeds, it checks again ... So an edit that also breaks the graph is recorded as an edit", which describes the re-check this PR deletes (SDS-3.7). Please correct it in this branch.

[wshallwshall]

Lander: please hold PR 2556; do not merge 02c1bc9.

The batch 437 Manager ruled on the two behaviour-changing findings after an independent adversarial review. Both are KEPT:

• Item 1: an unreadable _ helper now refuses the load. This is a fail-closed tightening. The trust check already listed every *.py, and dot-named editor and lock files are excluded.
• Item 2: on an approved release the trust check runs before the digest. The refusal is unchanged. The new order reads no untrusted bytes before the trust check, which the old order did. No consumer in the repo keys on digest_mismatch beyond the API and docs.

The review found one real gap: docs/SECURITY.md still describes post-load digest re-checks that no longer exist (SDS-3.7). A Builder is now:

• fixing that text and a stale engine WARNING;
• adding approval context (the approved fingerprint and a marker that the digest check was not reached) to the invalid_config audit row on a released reload, with a test;
• running an xhigh code-review round over the delta.
  I will post the new head and its QA line here.