diff --git a/changelog.d/2596.security.md b/changelog.d/2596.security.md new file mode 100644 index 000000000..864836ad0 --- /dev/null +++ b/changelog.d/2596.security.md @@ -0,0 +1,19 @@ +- **A config load reads the directory once and runs only the bytes it read.** Each start and + reload reads its config into one snapshot after the config source's trust check, and the loader + compiles every config module, and each `_` helper imported with an `import` statement, from it, + so neither runs from `__pycache__`. A + released reload checks its approval's digest against that same snapshot before any module runs, + so an edit to a module, helper or table made while it loads is neither run nor recorded, and the + second digest check after the load is gone. Where the environment value file sits in the config + directory's `environments/` folder, `serve` parses the values a reload applies from the same + snapshot, so an edit to it during a release never applies under its digest, and a value file + created or removed there since the snapshot is refused. + Where the values are read from disk instead, a release takes a second digest after that read. See + [SECURITY.md](../docs/SECURITY.md). (vault `BACKLOG #2596`) +- **BREAKING: an unreadable `_` helper refuses a start or reload, even when no module imports + it.** Every file the load reads must be readable. (vault `BACKLOG #2596`) +- **A release that also loosens a config module's permissions is audited as `invalid_config`.** + The trust check runs before the digest check, so where it refuses, such a tamper is refused + there, not as `digest_mismatch`. A release refused as a `WiringError` before the digest check now + writes a `config_reload_failed` row carrying `approved_fingerprint` and `approval_digest_check` + set to `not_reached`. (vault `BACKLOG #2596`) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index c5b46547a..1e636b7f8 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -1750,8 +1750,7 @@ started together, can each raise it; the alert list folds them into one instance still compared, on the fingerprint taken after the load. When both cover the same files and still differ, `GET /config/provenance` also has no loaded fingerprint to vouch for until the next reload, and the flag toggle rows that process writes are marked `baseline_unchecked`. The two - reads miss at least an edit that lands and is reverted between them, and a module the loader runs - from a still-valid `__pycache__` bytecode file rather than from its source. + reads miss at least an edit that lands and is reverted between them. - The check is alert-only. A baseline read that fails or takes over five seconds is logged at WARNING, and the start goes on. - A start that never checked its config, because its read failed or it took no fingerprint, diff --git a/docs/DANGEROUS-FUNCTIONALITY.md b/docs/DANGEROUS-FUNCTIONALITY.md index 49dace624..200ab698a 100644 --- a/docs/DANGEROUS-FUNCTIONALITY.md +++ b/docs/DANGEROUS-FUNCTIONALITY.md @@ -96,8 +96,10 @@ break that Handler rather than protect it. ## 2. The config loader imports by file path -`config/wiring.py` builds an import spec from a path and executes the module. Two sites do this: the -loader's own finder, and the direct module load. +`config/wiring.py` reads the config directory once, compiles each module from those bytes, and +executes it. Two sites do this: the loader's helper importer, and the direct module load. Neither +reads a `__pycache__` file, and a sandbox worker runs the bytes the engine read, not the directory +(vault BACKLOG #2596). **What it means.** Every `.py` file in your config directory that does not start with `_` gets imported at startup, and its top-level code runs. A file you dropped there to look at later is not diff --git a/docs/SECURITY.md b/docs/SECURITY.md index fd2f3890b..185166d71 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -2328,16 +2328,22 @@ it and does not compare against it. Only a committed reload or a flag toggle tha fingerprint counts. A later reload that fails and rolls back does not mark the row. The check runs just before the write, so a change after it is not caught. -**A released reload loads only the config its approval named (vault BACKLOG #2596, step 3).** When +**A released reload loads only the config its approval named (vault BACKLOG #2596).** When dual control holds a reload, the hold's `params` carry `fingerprint`: the ADR 0041 D1 content -digest of the target directory, taken at request time. An approver sees it in the queue. The -release checks the directory against it before any config module runs. After a load that -succeeds, it checks again, before the checks that refuse an invalid graph. It first takes the -fail-closed digest. Then it compares the digest the `config_reload` row records, if the engine -took one. After a load that fails as an invalid config, it checks the fail-closed digest once -more. So an edit that also breaks the graph is recorded as an edit. If any check differs from the -approved digest, nothing loads past that point and nothing swaps. The running graph keeps the -environment values it had. A reload applies the values it re-read only with the swap, and a +digest of the target directory, taken at request time. An approver sees it in the queue. At +release the config source's trust check runs before the directory is read. It stats paths and +reads no file's contents. The release then reads the directory once, into a snapshot, and checks +that snapshot's digest against the approved one before any config module runs. The load compiles +and runs that same snapshot, and the `config_reload` row records its digest. So the config +modules, helpers and tables checked are the ones the loader runs, and an edit to them on disk +during the load is neither run nor recorded. The limits below name at least one import that runs +from disk instead. Where the environment value file sits in the config directory's +`environments/` folder, `serve` parses it from the same snapshot, so an edit to it during the load +does not apply, and a value file created or removed there since the snapshot is refused. Otherwise, +at least where the value file sits elsewhere or an embedded engine has another value provider, the +release reads the values from disk and then takes the digest again, which refuses a change made in +between to a file the digest covers. If the digest differs, nothing loads and nothing swaps. The running graph keeps +the environment values it had. A reload applies the values it re-read only with the swap, and a rolled-back swap puts the old ones back. A dry run checks the values and never applies them. The approve answers **422**, the request moves to `failed`, and a `config_reload_failed` row records reason `digest_mismatch` with `approved_fingerprint` and `current_fingerprint`. A hold whose @@ -2345,6 +2351,24 @@ params carry no digest is refused the same way, with reason `no_approved_digest` cannot take the digest now is refused with reason `digest_unavailable`, and the engine logs the cause at WARNING. The requester asks again, and a second approver reviews the new bytes. +**A release refused before the digest check never reaches it.** At least the settings preflight +and the config source's trust check run before it. So where the trust check refuses rather than +warns, a tamper that also loosens the permissions on a file it covers, such as a config module, +is refused as `invalid_config`, not `digest_mismatch`. A `WiringError` refusal made before the +check, by those two or another step, writes a row carrying `approved_fingerprint`, the approval's +digest, and `approval_digest_check` set to `not_reached`, and no `current_fingerprint`, because no +digest was taken. A missing, unlistable or disallowed directory is refused before the check too, +with its own reason and neither field. A refusal after the digest check passed carries neither +field either, whether the approved bytes caused it or an input the digest does not cover, such as +an environment value file outside the config directory or a connection's CA file. The second +digest above is the exception: it refuses as `digest_mismatch` with both fields, or as at least +`digest_unavailable` or `not_found`, as the first check does. + +**Every start and reload refuses a config directory holding a file the load reads that the engine +cannot read, a `_` helper no module imports included.** An unreadable file or folder under +`environments/` is left out, because the load never reads it. A released reload reads strictly, so +it refuses that too, as `digest_unavailable`. + This digest fails closed, unlike the provenance fingerprint, which skips what it cannot read. A file or folder it covers that cannot be read gives no digest at all, never the digest of the rest. A directory that is gone when the digest first lists it is refused as missing. So is a name or @@ -2352,7 +2376,7 @@ path too long for the filesystem, or a name Windows refuses. A directory removed listing is not refused as missing. The digest covers the files it still finds, down to an empty bundle. That digest does not match an approved bundle that held files, so the release is refused as `digest_mismatch`. A file that goes between its stat and its read gives `digest_unavailable` -instead. After the load the release checks this digest first, and then the one the row records. +instead. At least these requests file no hold, because their release could only be refused. Each is answered and audited at once, as an inline reload's refusal is: @@ -2371,15 +2395,21 @@ unchanged. An ungated reload takes no request-time digest. At least these limits integrity seal, so a writer to the `pending_approvals` table who replaces the digest leaves a request the gate refuses. The seal does not stop an older sealed copy of the row being put back, and a keyless store checks nothing (vault BACKLOG #2592); -- the digest covers what `config/fingerprint.py` hashes and no more. A planted `__pycache__` file - runs under an unchanged digest. So does an edit to an environment value file outside the config - directory, or to a `MEFOR_VALUE_*` variable. These gaps are closing step 1 of vault BACKLOG #2596; -- each check samples the files. An edit that lands after the first check runs its module code - before the second check refuses it. An edit reverted while the release loads is not seen at all. - Hashing, compiling and running the same bytes is that same step 1; +- the digest covers what `config/fingerprint.py` hashes and no more. The release reads a value + file outside the config directory's `environments/` folder after the digest check, and a + `MEFOR_VALUE_*` variable too, so an edit to either changes the values a release applies under an + unchanged digest. The second digest samples the files, so where an embedded engine's other value + provider reads a value file under the config directory, an edit to it reverted before that digest + is not seen. The second digest also refuses an edit to any covered file made after the first, + though the load runs the snapshot the first one checked. + The loader compiles each config module, and each `_` helper a config module imports with an + `import` statement, from the snapshot. A module reached through the ordinary import system runs + from disk, a matching `__pycache__` file included, so the bytes that run are not the bytes the + digest covers. At least `importlib.import_module`, and a sibling import while the config directory + is on `sys.path`, reach it that way; - the request-time digest is taken before the route checks its step-up proof; - the digest checks that each folder it reads can be listed, and then lists it again. A folder - that becomes unlistable between the two is left out of the digest; + removed between the two is left out of the digest; - a directory moved away during a release is recorded as `not_found`, not as an edit, unless the move lands while a digest is reading it; - a swap that fails while reconciling outbound connections does not roll those connections back. diff --git a/docs/SERVICE.md b/docs/SERVICE.md index 8c74a1c53..8820e3146 100644 --- a/docs/SERVICE.md +++ b/docs/SERVICE.md @@ -740,9 +740,11 @@ trust boundary: anyone who can write a `.py` file there can run code as the serv run `namei -l` on every file there that is a link. Each directory those commands list must be owned by root, or by the engine's own account for the config directory itself, with no group or other write bit. The shipped image's `/config` sits directly under `/`, which is root-owned. - - **The cache folder is checked because it runs.** Python's source loader runs a matching - `__pycache__\*.pyc` in place of the `.py` beside it and never compares the two. A cache file - the check did not read could run under a source it passed (vault BACKLOG #2596). + - **The cache folder is checked because a cache file there can still run.** The loader compiles + each config module, and each `_` helper imported with an `import` statement, from the bytes it + read, so neither runs from `__pycache__`. A module reached through the ordinary import system + still can, at least through `importlib.import_module` or a sibling import while the config + directory is on `sys.path`, and so can tools such as `python -m` (vault BACKLOG #2596). - **At start, `serve` checks two more files the same way, each with the folder it sits in**: the service settings file it read (`--service-config`, else `.\messagefoundry.toml`) and the value file `environments\.toml`. It refuses to start when another account can write either one diff --git a/docs/adr/0041-load-path-attestation-and-change-attribution.md b/docs/adr/0041-load-path-attestation-and-change-attribution.md index ced30c352..d86d25257 100644 --- a/docs/adr/0041-load-path-attestation-and-change-attribution.md +++ b/docs/adr/0041-load-path-attestation-and-change-attribution.md @@ -105,6 +105,20 @@ advisory provenance — the content digest is the integrity anchor. (`.IB_OLD.py`, an AppleDouble `._X.py`) is no longer hashed, because the loader no longer runs one; the `*.py` set is `config.wiring.config_py_files`, the one rule the loader and ADR 0036's guard share. The `v1` text above is kept as the record of what was decided on its date.* +*Amended 2026-10-27 (vault BACKLOG #2596, step 1): the digest is now of the bytes that ran, and the +scheme is `mefor-cfg-fp:v3`.* A load reads the directory once, into a snapshot +([`config/snapshot.py`](../../messagefoundry/config/snapshot.py)), and runs only that: each module and +`_` helper is compiled by the loader from its snapshot bytes, so a `__pycache__` file is never consulted, +and the code sets and `connections.toml` are parsed from the same snapshot. The fingerprint folds that +snapshot, so a reload's audit digest names the bytes it ran, and a dual-control release checks the +snapshot it then loads (D2). A `[sandbox].mode=subprocess` worker is sent the snapshot and never reads +the directory. What `v3` changes in what is hashed: a code-set table is matched by its suffix in lower +case, as the loader matches it, so `codesets/x.CSV` is hashed on POSIX; and each path is hashed, so a +path that resolves to a file another path names is no longer folded once. **Not changed, and still +open:** `environments/*.toml` is hashed under the config directory, as before. That is not where a +serve start reads its value file, which sits under `[environments].base_dir`; binding the value file +by its resolved path is the remainder of #2596 step 1. The `v2` text above is kept as the record of +what was decided on its date. This binds "reviewed-and-merged in git" to "what actually loaded," makes two reloads with different on-disk code distinguishable, defeats attribution-laundering (the audit now shows *which code* a given diff --git a/messagefoundry/__main__.py b/messagefoundry/__main__.py index d32c42063..4991eee5f 100644 --- a/messagefoundry/__main__.py +++ b/messagefoundry/__main__.py @@ -96,7 +96,9 @@ # `hl7schema` call must not pay for it (see the module docstring on deferred heavy imports). from messagefoundry.auth.service import AuthService from messagefoundry.auth.trust_anchors import TrustAnchorError + from messagefoundry.config.environments import EnvironmentValuesReader from messagefoundry.config.settings import ServiceSettings, StoreSettings + from messagefoundry.config.snapshot import ConfigSnapshot from messagefoundry.config.static_credentials import StaticCredentialGateOutcome from messagefoundry.config.tls_policy import HopPosture from messagefoundry.config.wiring import Registry @@ -2450,14 +2452,45 @@ def _exposure_description(settings: ServiceSettings, *, exposed: bool) -> str: return "admin interface reachable from beyond this host" -def _read_environment_values(*, base_dir: Path, dir_name: str, environment: str) -> dict[str, Any]: +def _environment_values_reader( + *, base_dir: Path, dir_name: str, environment: str +) -> EnvironmentValuesReader: + """``serve``'s ``env_values_provider``: :func:`_read_environment_values` over one value file. + + The Engine calls it with no arguments at start, and with a reload's config snapshot on a + reload that read one, so a value file the snapshot covers is parsed from the bytes the reload's + digest names (vault BACKLOG #2596). ``supervise`` builds the same reader.""" + from messagefoundry.config.environments import EnvironmentValuesReader + + return EnvironmentValuesReader( + _read_environment_values, base_dir=base_dir, dir_name=dir_name, environment=environment + ) + + +def _read_environment_values( + *, + base_dir: Path, + dir_name: str, + environment: str, + snapshot: ConfigSnapshot | None = None, +) -> dict[str, Any]: """This instance's environment values: the value file :func:`~messagefoundry.config.environments.values_file_path` names, plus the ``MEFOR_VALUE_*`` - environment. ``serve`` hands it to the Engine as the ``env_values_provider``, so it runs at - start and again on every reload.""" + environment. ``serve`` hands it to the Engine as the ``env_values_provider`` + (:func:`_environment_values_reader`), so it runs at start and again on every reload. + + With ``snapshot``, the value file is parsed from the bytes the snapshot read and never read + from disk, so a file edited after that read does not apply, and one created or removed since is + refused (vault BACKLOG #2596). A value file outside the snapshot's ``environments/`` folder + raises :class:`~messagefoundry.config.snapshot.NotInSnapshot` instead.""" import os - from messagefoundry.config.environments import load_environment_values, values_file_path + from messagefoundry.config.environments import ( + environment_values_from_bytes, + load_environment_values, + values_file_path, + ) + from messagefoundry.config.snapshot import NotInSnapshot from messagefoundry.config.wiring import WiringError, assert_trusted_start_paths from messagefoundry.redaction import safe_exc @@ -2487,8 +2520,21 @@ def _read_environment_values(*, base_dir: Path, dir_name: str, environment: str) # created after the start is not read unvetted. Outside the try below, because WiringError # is a ValueError and that arm would wrap the refusal as a read failure. env_file = values_file_path(base_dir, dir_name, environment) - assert_trusted_start_paths([env_file], lead="refusing to read environment values from") + if snapshot is not None and not snapshot.covers_environment_file(env_file): + # Before the vetting below, so the plain read the engine then makes vets the file once. + raise NotInSnapshot(f"{env_file} is not in the config snapshot's environments folder") + # With a snapshot, its folder is vetted even when the file has gone: the bytes that apply were + # read earlier, and whoever can write the folder could have planted them (vault BACKLOG #2596). + assert_trusted_start_paths( + [env_file], + lead="refusing to read environment values from", + folders=[env_file.parent] if snapshot is not None else (), + ) try: + if snapshot is not None: + return environment_values_from_bytes( + snapshot.environment_file(env_file), path=env_file, environ=os.environ + ) return load_environment_values( base_dir=base_dir, dir_name=dir_name, @@ -4534,11 +4580,8 @@ def _serve(args: argparse.Namespace) -> int: from messagefoundry.config.wiring import WiringError from messagefoundry.redaction import safe_exc - env_values = functools.partial( - _read_environment_values, - base_dir=env_base, - dir_name=settings.environments.dir, - environment=env_name, + env_values = _environment_values_reader( + base_dir=env_base, dir_name=settings.environments.dir, environment=env_name ) # ADR 0050 anchoring diagnostics. Emitted ONCE here at startup (NOT inside env_values(), which is @@ -5340,8 +5383,7 @@ def refuse_keep_forever_overrides(registry: Registry) -> None: registry_guard=_chain_registry_guards( refuse_keep_forever_overrides, _fleet_connection_key_file_refusal( - functools.partial( - _read_environment_values, + _environment_values_reader( base_dir=env_base, dir_name=settings.environments.dir, environment=env_name, diff --git a/messagefoundry/api/app.py b/messagefoundry/api/app.py index 7526f8ae9..fffad246b 100644 --- a/messagefoundry/api/app.py +++ b/messagefoundry/api/app.py @@ -445,7 +445,11 @@ ) from messagefoundry.pipeline.connscale_shim import maybe_install_executor_shim from messagefoundry.pipeline.dr import DrActivationError -from messagefoundry.pipeline.engine import CONFIG_WRITE_ACTIONS, ConfigWriteAuditFailed +from messagefoundry.pipeline.engine import ( + CONFIG_WRITE_ACTIONS, + ConfigRefusedBeforeApprovalCheck, + ConfigWriteAuditFailed, +) from messagefoundry.pipeline.ingress_guards import IngressGuardError, admit_resubmission from messagefoundry.pipeline.security_notify import security_notifier_from_settings from messagefoundry.pipeline.security_signals import ( @@ -2311,7 +2315,9 @@ async def _audit_refused_reload( ``no_approved_digest`` when the hold took none, with both digests, and answers 422 (vault BACKLOG #2596); only the executor meets it. One whose digest could not be taken now writes reason ``digest_unavailable`` and answers 422; the engine logged why at WARNING. A gated - request whose digest cannot be taken is refused the same way, with no hold filed. A directory + request whose digest cannot be taken is refused the same way, with no hold filed. A release + refused before its digest check (:class:`ConfigRefusedBeforeApprovalCheck`) keeps its reason + and adds ``approved_fingerprint`` and ``approval_digest_check`` ``not_reached``. A directory that cannot be resolved for another reason writes reason ``unreadable`` and answers 422; the engine logged why at WARNING. Any other ``Exception`` is a fault inside the reload, a plain ``FileNotFoundError`` from a runner start or swap among them (not a missing directory): it @@ -2343,6 +2349,14 @@ async def _audit_refused_reload( (BACKLOG #3157).""" warn = _log.warning if reported is None else _log.debug detail: dict[str, object] = {"requested": requested, "dry_run": dry_run, **(extra or {})} + if isinstance(exc, ConfigRefusedBeforeApprovalCheck): + # A release refused before its digest check, such as by the trust check of a source whose + # permissions were loosened: the row names the approval and says the check was not + # reached, so it is not read as a refusal of the approved bytes (vault BACKLOG #2596). A + # digest and a marker only. Before the chain, which still sets the reason: it is a + # WiringError, and its kept cause tells trust_anchor and store_unavailable apart. + detail["approved_fingerprint"] = exc.approved + detail["approval_digest_check"] = "not_reached" if isinstance(exc, ConfigReloadDenied): action = "config_reload_denied" status, answer = 403, "config directory is not an allowed reload root" @@ -2720,9 +2734,7 @@ async def _compare_start_config( digest, as main's did. The caller stops the process vouching for that digest only when both digests were taken over the same files and differ (:func:`_moved_during_load`): a first digest that failed, or two that left out different files, is marked here and still vouched for. Two - reads miss at least two cases: an edit that lands and is reverted between them, and - a module the loader runs from a still-valid ``__pycache__`` bytecode file rather than from the - source the digests hash. + reads miss at least an edit that lands and is reverted between them. A digest that left out files it could not read (:func:`digest_coverage`) is still compared. A start whose own digest is partial is ``partial_start_digest``, logged at WARNING with what it diff --git a/messagefoundry/api/models.py b/messagefoundry/api/models.py index 31ea89c7f..a9cb18295 100644 --- a/messagefoundry/api/models.py +++ b/messagefoundry/api/models.py @@ -679,7 +679,7 @@ class ConfigProvenance(BaseModel): (or if the fingerprint could not be computed); ``drift`` is only meaningful when ``loaded`` is True.""" loaded: bool - fingerprint: str | None = None # content hash of the loaded bundle (scheme mefor-cfg-fp:v2) + fingerprint: str | None = None # content hash of the loaded bundle (scheme mefor-cfg-fp:v3) git_head: str | None = None # commit sha at load, when the config dir is a git work tree files: int | None = None # number of files folded into the fingerprint drift: bool = False # the on-disk config now differs from what was loaded diff --git a/messagefoundry/config/code_sets.py b/messagefoundry/config/code_sets.py index 10af008ea..9856a1aa0 100644 --- a/messagefoundry/config/code_sets.py +++ b/messagefoundry/config/code_sets.py @@ -43,14 +43,16 @@ from __future__ import annotations import csv +import io import logging +import os import tomllib from collections.abc import Callable, Iterator, Mapping from contextlib import contextmanager from contextvars import ContextVar from dataclasses import dataclass from enum import Enum -from pathlib import Path +from pathlib import Path, PurePath from typing import Any from messagefoundry.redaction import codec_safe_str @@ -62,6 +64,10 @@ "CodeSet", "load_code_set", "load_code_sets", + "code_sets_from_files", + "is_code_set_name", + "parse_code_set", + "parse_policy", "code_set", "set_active", "activated", @@ -417,17 +423,42 @@ def load_code_set(path: str | Path) -> CodeSet: Auto-detects the format by extension. Raises :class:`CodeSetError` (naming the file) for an unknown extension, a malformed file, or a duplicate key — never silently drops data.""" path = Path(path) - name = path.stem - suffix = path.suffix.lower() - if suffix == ".csv": - data = _load_csv(path) - elif suffix == ".toml": - data = _load_toml(path) - else: + _require_supported(path.name) + return parse_code_set(path.name, _read_table(path), load_policy(path)) + + +def _require_supported(name: str) -> str: + """``name``'s suffix in lower case, refused unless it is one the loader parses.""" + suffix = Path(name).suffix.lower() + if not is_code_set_name(name): raise CodeSetError( - f"code set {path.name!r}: unsupported extension {suffix!r} (use .csv or .toml)" + f"code set {name!r}: unsupported extension {suffix!r} (use .csv or .toml)" ) - return CodeSet(name, data, load_policy(path)) + return suffix + + +def _read_table(path: Path) -> bytes: + """The bytes of table file ``path``. An unreadable file is a :class:`CodeSetError` naming it, + phrased as its format's parse error is.""" + try: + return path.read_bytes() + except OSError as exc: + if path.suffix.lower() == ".csv": + unreadable = f"code set {path.name!r}: invalid CSV — {exc}" + else: + unreadable = f"code set {path.name!r}: invalid TOML — {codec_safe_str(exc)}" + raise CodeSetError(unreadable) + + +def parse_code_set(name: str, data: bytes, policy: UnmappedPolicy | None = None) -> CodeSet: + """Parse the bytes of table file ``name`` into a :class:`CodeSet` named for its stem. + + The one parser :func:`load_code_set` and a snapshot load share (vault BACKLOG #2596), so a table + read from disk and one read from a :class:`~messagefoundry.config.snapshot.ConfigSnapshot` + give the same set, and the same refusals.""" + suffix = _require_supported(name) + table = _parse_csv(name, data) if suffix == ".csv" else _parse_toml(name, data) + return CodeSet(Path(name).stem, table, policy if policy is not None else UnmappedPolicy()) def _policy_sidecar_path(codeset_path: Path) -> Path: @@ -460,12 +491,25 @@ def load_policy(codeset_path: str | Path) -> UnmappedPolicy: sidecar = _policy_sidecar_path(Path(codeset_path)) if not sidecar.is_file(): return UnmappedPolicy() + return parse_policy(sidecar.name, _read_sidecar(sidecar)) + + +def _read_sidecar(path: Path) -> bytes: try: - with sidecar.open("rb") as fh: - raw = tomllib.load(fh) - except (tomllib.TOMLDecodeError, UnicodeDecodeError, OSError) as exc: + return path.read_bytes() + except OSError as exc: + unreadable = f"policy sidecar {path.name!r}: invalid TOML — {codec_safe_str(exc)}" + raise CodeSetError(unreadable) + + +def parse_policy(sidecar_name: str, data: bytes) -> UnmappedPolicy: + """Parse the bytes of policy sidecar ``sidecar_name``, shared by :func:`load_policy` and a + snapshot load.""" + try: + raw = tomllib.loads(data.decode("utf-8")) + except (tomllib.TOMLDecodeError, UnicodeDecodeError) as exc: invalid: str | None = ( - f"policy sidecar {sidecar.name!r}: invalid TOML — {codec_safe_str(exc)}" + f"policy sidecar {sidecar_name!r}: invalid TOML — {codec_safe_str(exc)}" ) else: invalid = None @@ -476,7 +520,7 @@ def load_policy(codeset_path: str | Path) -> UnmappedPolicy: return UnmappedPolicy.from_mapping(raw) except CodeSetError as exc: # Re-raise with the sidecar filename so a bad policy is self-explanatory at load. - raise CodeSetError(f"policy sidecar {sidecar.name!r}: {exc}") from None + raise CodeSetError(f"policy sidecar {sidecar_name!r}: {exc}") from None def load_code_sets(codesets_dir: str | Path) -> dict[str, CodeSet]: @@ -488,18 +532,51 @@ def load_code_sets(codesets_dir: str | Path) -> dict[str, CodeSet]: codesets_dir = Path(codesets_dir) if not codesets_dir.is_dir(): return {} - out: dict[str, CodeSet] = {} - # Sorted for a deterministic load order, so a clash error names a stable "first" file. + files: dict[str, bytes] = {} for path in sorted(codesets_dir.iterdir()): - if not path.is_file() or path.suffix.lower() not in (".csv", ".toml"): + if path.is_file() and is_code_set_name(path.name): + reader = _read_sidecar if is_policy_sidecar(path) else _read_table + files[path.name] = reader(path) + return code_sets_from_files(files, codesets_dir) + + +def code_sets_from_files( + files: Mapping[str, bytes], codesets_dir: str | Path +) -> dict[str, CodeSet]: + """Parse ``files`` (each table in ``codesets/``, by its file name there) into a registry. + + The rules of :func:`load_code_sets`, over bytes already read: a ``.policy.toml`` beside a + ``.csv`` or ``.toml`` is that table's policy (#162) rather than a table of its own, + and two tables of one name are a :class:`CodeSetError`. A config load parses the code sets of + its snapshot here, so the tables it publishes are the bytes it hashed (vault BACKLOG #2596). + ``codesets_dir`` only names the folder in a refusal. Names are matched as the filesystem would + match them: without case on Windows.""" + by_key = {os.path.normcase(name): name for name in files} + + def sibling(name: str) -> str | None: + return by_key.get(os.path.normcase(name)) + + def is_sidecar(name: str) -> bool: + if not name.endswith(POLICY_SIDECAR_SUFFIX): + return False + base = name[: -len(POLICY_SIDECAR_SUFFIX)] + return sibling(f"{base}.csv") is not None or sibling(f"{base}.toml") is not None + + out: dict[str, CodeSet] = {} + # Sorted for a deterministic load order, so a clash error names a stable "first" file. In path + # order, as the folder listing was sorted: without case on Windows. + for name in sorted(files, key=PurePath): + if not is_code_set_name(name): continue # A ``.policy.toml`` sidecar (#162) is policy METADATA for its code set, not a code set - # of its own — skip it here (it is read by load_policy() alongside the code-set file). Skip - # ONLY a true sidecar (a companion code set exists); a standalone ``x.policy.toml`` with no - # companion is a legacy code set named ``x.policy`` and still loads (no silent drop). - if is_policy_sidecar(path): + # of its own — skip it here (it is read alongside the code-set file). Skip ONLY a true + # sidecar (a companion code set exists); a standalone ``x.policy.toml`` with no companion is + # a legacy code set named ``x.policy`` and still loads (no silent drop). + if is_sidecar(name): continue - cs = load_code_set(path) + sidecar = sibling(f"{Path(name).stem}{POLICY_SIDECAR_SUFFIX}") + policy = parse_policy(sidecar, files[sidecar]) if sidecar is not None else None + cs = parse_code_set(name, files[name], policy) if cs.name in out: raise CodeSetError( f"duplicate code set name {cs.name!r} in {codesets_dir} — two files (different " @@ -509,71 +586,72 @@ def load_code_sets(codesets_dir: str | Path) -> dict[str, CodeSet]: return out -def _load_csv(path: Path) -> dict[str, Any]: +def is_code_set_name(name: str) -> bool: + """Whether a file called ``name`` in ``codesets/`` is one the loader reads: its suffix, in lower + case, is ``.csv`` or ``.toml``. The snapshot, the fingerprint and the trust check use this rule + too, so a table spelled ``x.CSV`` is read, hashed and vetted alike on POSIX (vault BACKLOG + #2596).""" + return Path(name).suffix.lower() in (".csv", ".toml") + + +def _parse_csv(name: str, data: bytes) -> dict[str, Any]: """CSV with a header row: first column = key; one other column → scalar, several → ``{header: cell}``. - A file that is not UTF-8, that the ``csv`` module refuses, or that cannot be opened, is a - :class:`CodeSetError` naming the file, as a malformed TOML one is. Each used to escape raw, past - every caller's + A file that is not UTF-8, or that the ``csv`` module refuses, is a :class:`CodeSetError` naming + the file, as a malformed TOML one is. Each used to escape raw, past every caller's ``CodeSetError`` arm, and the decode error's text names the byte it failed on (vault BACKLOG - #3295). The reader decodes as it iterates, so the whole read is inside the arms. - - No position is given for a decode error: the reader decodes a block at a time, so the error's - offset is into that block, not into the file.""" + #3295). No position is given for a decode error.""" try: - return _read_csv(path) + return _read_csv(name, data.decode("utf-8")) except UnicodeDecodeError: - invalid = f"code set {path.name!r}: invalid CSV — the file is not valid UTF-8" + invalid = f"code set {name!r}: invalid CSV — the file is not valid UTF-8" except csv.Error as exc: # its text is one of the module's fixed phrases - invalid = f"code set {path.name!r}: invalid CSV — {exc}" - except OSError as exc: # as _load_toml maps it: an unreadable file is refused by name - invalid = f"code set {path.name!r}: invalid CSV — {exc}" - # Raised after the handler: the decode error's .object is a block of the file. + invalid = f"code set {name!r}: invalid CSV — {exc}" + # Raised after the handler: the decode error's .object is the file. raise CodeSetError(invalid) -def _read_csv(path: Path) -> dict[str, Any]: +def _read_csv(name: str, text: str) -> dict[str, Any]: data: dict[str, Any] = {} - with path.open(newline="", encoding="utf-8") as fh: - reader = csv.DictReader(fh) - fields = reader.fieldnames - if not fields: - raise CodeSetError(f"code set {path.name!r}: empty CSV (no header row)") - key_field, *value_fields = fields - if not value_fields: + # newline="": the csv module does its own line handling, as over a file opened that way. + reader = csv.DictReader(io.StringIO(text, newline="")) + fields = reader.fieldnames + if not fields: + raise CodeSetError(f"code set {name!r}: empty CSV (no header row)") + key_field, *value_fields = fields + if not value_fields: + raise CodeSetError( + f"code set {name!r}: CSV needs a key column plus at least one value column" + ) + single = len(value_fields) == 1 + for row in reader: + # An over-wide row (more cells than the header) lands in DictReader's ``restkey`` bucket + # under the ``None`` key — silently sinking the extra cells (accept-and-drop, CLAUDE.md §1). + # Reject it loud so the loader is no stricter/looser than the codeset_edit writer/grid (#240). + if None in row: + extra = row[None] raise CodeSetError( - f"code set {path.name!r}: CSV needs a key column plus at least one value column" + f"code set {name!r}: a row has more cells than columns " + f"({len(fields) + len(extra)} > {len(fields)})" ) - single = len(value_fields) == 1 - for row in reader: - # An over-wide row (more cells than the header) lands in DictReader's ``restkey`` bucket - # under the ``None`` key — silently sinking the extra cells (accept-and-drop, CLAUDE.md §1). - # Reject it loud so the loader is no stricter/looser than the codeset_edit writer/grid (#240). - if None in row: - extra = row[None] - raise CodeSetError( - f"code set {path.name!r}: a row has more cells than columns " - f"({len(fields) + len(extra)} > {len(fields)})" - ) - key = row.get(key_field) - if key is None: - continue # short/blank row — DictReader fills missing cells with None - if key in data: - raise CodeSetError(f"code set {path.name!r}: duplicate key {key!r}") - if single: - data[key] = row.get(value_fields[0]) - else: - data[key] = {vf: row.get(vf) for vf in value_fields} + key = row.get(key_field) + if key is None: + continue # short/blank row — DictReader fills missing cells with None + if key in data: + raise CodeSetError(f"code set {name!r}: duplicate key {key!r}") + if single: + data[key] = row.get(value_fields[0]) + else: + data[key] = {vf: row.get(vf) for vf in value_fields} return data -def _load_toml(path: Path) -> dict[str, Any]: +def _parse_toml(name: str, data: bytes) -> dict[str, Any]: """Flat TOML table → ``{key: scalar}``; a nested table value → ``{key: {…}}`` (mirrors environments).""" try: - with path.open("rb") as fh: - raw = tomllib.load(fh) - except (tomllib.TOMLDecodeError, UnicodeDecodeError, OSError) as exc: - invalid = f"code set {path.name!r}: invalid TOML — {codec_safe_str(exc)}" + raw = tomllib.loads(data.decode("utf-8")) + except (tomllib.TOMLDecodeError, UnicodeDecodeError) as exc: + invalid = f"code set {name!r}: invalid TOML — {codec_safe_str(exc)}" else: # tomllib already rejects duplicate keys (TOMLDecodeError), so no extra dup check is needed. return dict(raw) diff --git a/messagefoundry/config/connections_file.py b/messagefoundry/config/connections_file.py index e7dad86f5..d0897c576 100644 --- a/messagefoundry/config/connections_file.py +++ b/messagefoundry/config/connections_file.py @@ -239,9 +239,24 @@ def load_connections_file(path: Path, registry: Registry) -> None: declared in a ``*.py`` module) surfaces as the registry's ``duplicate ... name`` error, so the two authoring surfaces can't silently shadow each other.""" try: - with open(path, "rb") as handle: - data = tomllib.load(handle) - except (OSError, tomllib.TOMLDecodeError, UnicodeDecodeError) as exc: + raw = path.read_bytes() + except OSError as exc: + unreadable = f"cannot read {path.name}: {codec_safe_str(exc)}" + else: + load_connections_data(raw, path, registry) + return + raise WiringError(unreadable) + + +def load_connections_data(raw: bytes, path: Path, registry: Registry) -> None: + """Decode ``raw``, the bytes of ``path``, and add its connections to ``registry`` (in place). + + :func:`load_connections_file` once the file is read. A config load hands it the bytes of its + snapshot, so the connections it declares are the bytes it hashed (vault BACKLOG #2596). + ``path`` names the file in a refusal and as each connection's source; it is not read.""" + try: + data = tomllib.loads(raw.decode("utf-8")) + except (tomllib.TOMLDecodeError, UnicodeDecodeError) as exc: unreadable: str | None = f"cannot read {path.name}: {codec_safe_str(exc)}" else: unreadable = None diff --git a/messagefoundry/config/environments.py b/messagefoundry/config/environments.py index ada6cddf8..ff9fd2880 100644 --- a/messagefoundry/config/environments.py +++ b/messagefoundry/config/environments.py @@ -27,12 +27,20 @@ import logging import tomllib +from abc import ABC, abstractmethod from collections.abc import Mapping from pathlib import Path -from typing import Any +from typing import TYPE_CHECKING, Any, Protocol + +if TYPE_CHECKING: + from messagefoundry.config.snapshot import ConfigSnapshot __all__ = [ "VALUE_ENV_PREFIX", + "EnvironmentValuesReader", + "SnapshotEnvValuesProvider", + "ValuesRead", + "environment_values_from_bytes", "is_drive_relative", "load_environment_values", "resolve_values_base_dir", @@ -45,6 +53,83 @@ _log = logging.getLogger(__name__) +class SnapshotEnvValuesProvider(ABC): + """An engine ``env_values_provider`` that can also read a reload's values from that reload's + config snapshot (vault BACKLOG #2596). A provider opts in by subclassing, never by having a + method of the right name. ``serve``'s provider is one.""" + + __slots__ = () + + @abstractmethod + def __call__(self) -> Mapping[str, Any]: + """The values, read from disk: at start, and on a reload that read no snapshot.""" + + @abstractmethod + def from_snapshot(self, snapshot: ConfigSnapshot) -> Mapping[str, Any] | None: + """The values, with a value file ``snapshot`` covers parsed from its bytes, or ``None`` + when the value file this provider reads is outside the snapshot + (:meth:`~messagefoundry.config.snapshot.ConfigSnapshot.covers_environment_file`). On + ``None`` the engine calls the provider plainly and, on a released reload, checks the digest + again.""" + + +class ValuesRead(Protocol): + """A read of one value file's environment values, from disk, or with ``snapshot`` from the + bytes it read, raising :class:`~messagefoundry.config.snapshot.NotInSnapshot` for a value file + outside it.""" + + def __call__( + self, + *, + base_dir: Path, + dir_name: str, + environment: str, + snapshot: ConfigSnapshot | None = None, + ) -> dict[str, Any]: ... + + +class EnvironmentValuesReader(SnapshotEnvValuesProvider): + """A :class:`SnapshotEnvValuesProvider` over one value file, + ``//.toml``, read by ``read``. ``serve`` passes its own read, + which vets the file's owner and access list and names the file in a refusal.""" + + __slots__ = ("_read", "base_dir", "dir_name", "environment") + + def __init__( + self, read: ValuesRead, *, base_dir: Path, dir_name: str, environment: str + ) -> None: + self._read = read + self.base_dir = base_dir + self.dir_name = dir_name + self.environment = environment + + def __call__(self) -> dict[str, Any]: + return self._read( + base_dir=self.base_dir, dir_name=self.dir_name, environment=self.environment + ) + + def from_snapshot(self, snapshot: ConfigSnapshot) -> dict[str, Any] | None: + from messagefoundry.config.snapshot import NotInSnapshot + + try: + return self._read( + base_dir=self.base_dir, + dir_name=self.dir_name, + environment=self.environment, + snapshot=snapshot, + ) + except NotInSnapshot: + # A path only, never a value. The default layout keeps the value file outside the + # config directory, so this is the usual line there, and it says why the release then + # takes a second digest. + _log.info( + "environment value file %s is outside the config snapshot; the reload reads it " + "from disk", + values_file_path(self.base_dir, self.dir_name, self.environment), + ) + return None + + def is_drive_relative(value: str) -> bool: """Whether ``value`` is rooted-looking (leading ``/`` or ``\\``) but not *truly* absolute here. @@ -111,12 +196,21 @@ def load_environment_values( overlays ``MEFOR_VALUE_`` env vars (env wins). A missing file is **not** an error here — referenced-but-undefined keys fail loud when a connector is built, not at gather time. """ - values: dict[str, Any] = {} - path = values_file_path(base_dir, dir_name, environment) - if path.is_file(): - with path.open("rb") as fh: - data = tomllib.load(fh) + return environment_values_from_bytes( + path.read_bytes() if path.is_file() else None, path=path, environ=environ + ) + + +def environment_values_from_bytes( + file_bytes: bytes | None, *, path: Path, environ: Mapping[str, str] +) -> dict[str, Any]: + """:func:`load_environment_values` over the value file's bytes already read, ``None`` when there + is no file. ``path`` only names the file in a warning. A released reload parses the bytes its + approval digest covered this way, rather than reading the file again (vault BACKLOG #2596).""" + values: dict[str, Any] = {} + if file_bytes is not None: + data = tomllib.loads(file_bytes.decode("utf-8")) for key, value in data.items(): if isinstance(value, dict): # Nested tables are reserved (future grouping); a value must be a scalar/array today. diff --git a/messagefoundry/config/fingerprint.py b/messagefoundry/config/fingerprint.py index 74757e27a..0bf2e9aa1 100644 --- a/messagefoundry/config/fingerprint.py +++ b/messagefoundry/config/fingerprint.py @@ -23,13 +23,16 @@ from __future__ import annotations -import errno import hashlib -import os -import stat from collections.abc import Mapping from pathlib import Path +from messagefoundry.config.snapshot import ( + ConfigDirMissing, + ConfigSnapshot, + is_missing_directory, + read_config_snapshot, +) from messagefoundry.credential import constant_time_equal __all__ = [ @@ -41,13 +44,18 @@ "fingerprint_matches", "digest_coverage", "is_missing_directory", + "snapshot_fingerprint", ] # Version the scheme so a future change to *what* is hashed (or *how*) is itself detectable in the # trail — a fingerprint produced by v1 can never collide with one produced by a later revision. # v2: a dot-named ``*.py`` is no longer hashed, because the loader no longer runs one (vault BACKLOG # #2781). -_SCHEME = b"mefor-cfg-fp:v2\n" +# v3: the digest folds the one snapshot a load runs (:mod:`messagefoundry.config.snapshot`, vault +# BACKLOG #2596). A code-set table is matched by its suffix in lower case, as the loader matches it, +# so ``codesets/x.CSV`` is hashed on POSIX too; and a path that resolves to a file another path +# already names is hashed under both names, so a link added beside a module changes the digest. +_SCHEME = b"mefor-cfg-fp:v3\n" #: The scheme tag :func:`config_fingerprint_detail` records beside each digest. Two digests are #: comparable only under one scheme: a start compares its own digest with the store's last recorded @@ -63,140 +71,24 @@ #: failed reload steps. UNREADABLE_FILES_KEY = "unreadable_files" -# Every file ``load_config`` consumes that defines the running graph's behaviour, transport config, -# or reference data. Globs are relative to the config dir. The config modules themselves are not a -# glob here: they come from ``config.wiring.config_py_files``, the one rule the loader and ADR 0036's -# guard also use. It covers ``_``-prefixed helpers -- the loader skips them as *top-level modules*, -# but a sibling can import them, so they are just as much "what runs" -- and skips a dot-named -# ``.IB_OLD.py`` or AppleDouble ``._X.py``, which the loader never runs. The globs below are -# unchanged: the code-set loader reads a dot-named table file, so hashing one is still right. -_FINGERPRINT_GLOBS: tuple[str, ...] = ( - "connections.toml", - "codesets/*.csv", - "codesets/*.toml", - "environments/*.toml", -) - - -class ConfigDirMissing(FileNotFoundError): - """The directory a strict :func:`config_fingerprint` was asked for is not a directory. Its own - type, so a caller can tell it from a covered file that vanished, which is a plain - ``FileNotFoundError``.""" - - -#: The Windows errors that refuse a path's name itself: ERROR_INVALID_NAME (123), ERROR_BAD_PATHNAME -#: (161) and ERROR_FILENAME_EXCED_RANGE (206). Python maps the first to EINVAL, not ENOENT. -_INVALID_NAME_WINERRORS: frozenset[int] = frozenset({123, 161, 206}) +# What is hashed is what :func:`~messagefoundry.config.snapshot.read_config_snapshot` reads: every +# config module (``config.snapshot.config_py_files``, the one rule the loader and ADR 0036's guard +# also use, ``_``-prefixed helpers included and a dot-named ``.IB_OLD.py`` or AppleDouble ``._X.py`` +# skipped), ``connections.toml``, each ``codesets/`` table, and each ``environments/*.toml`` under +# the config directory. The last is never loaded as config. The value file a serve start reads sits +# under ``[environments].base_dir``; where that is this folder, a reload parses the very bytes +# hashed here (``ConfigSnapshot.environment_file``), so dropping them from the fold would let a +# released reload apply values its digest does not cover. ``__pycache__`` is never read: +# the loader compiles the source itself. -def is_missing_directory(exc: OSError) -> bool: - """Whether ``exc``, raised resolving or listing a directory, says no directory can be opened by - that path: nothing by that name, a file rather than a directory, a name or whole path too long - for the filesystem (ENAMETOOLONG, or Windows error 206), or a name Windows refuses, such as - ``sub?``. ``Path.is_dir`` answers ``False`` for all of them, so a caller that lists the - directory instead reports them as missing too (vault BACKLOG #2596). Any other error, such as - access denied, is not a missing directory.""" - return ( - isinstance(exc, (FileNotFoundError, NotADirectoryError)) - or exc.errno == errno.ENAMETOOLONG - or getattr(exc, "winerror", None) in _INVALID_NAME_WINERRORS - ) - - -def _iter_entries(base: Path, *, strict: bool, unreadable: set[str]) -> list[tuple[str, bytes]]: - """Return ``(posix-relpath, sha256(content))`` for every fingerprinted file, sorted. +def _entries(snapshot: ConfigSnapshot) -> list[tuple[str, bytes]]: + """``(posix-relpath, sha256(content))`` for every file in ``snapshot``, sorted. Sorting by relpath makes the fold order-independent (filesystem enumeration order can vary); keying on the *relative* path (not absolute) makes the digest location-independent, so two - byte-identical bundles at different paths fingerprint identically. A file that cannot be stat'ed - or read, and a folder a glob reads that cannot be listed, raises when ``strict`` (see - :func:`config_fingerprint`); otherwise it is left out and added to ``unreadable``, so a caller - can tell a digest of the whole bundle from one of part of it (vault BACKLOG #2944). - """ - # Lazy: wiring is the loader, and this module stays importable without pulling it in at import time. - from messagefoundry.config.wiring import config_py_files - - try: - # Not strict, every folder that cannot be listed is collected, not just the first, so two - # digests that report the same coverage do cover the same files. - _require_listable(base, unlistable=None if strict else unreadable) - except ConfigDirMissing: - if strict: - raise - # Not strict: a missing directory digests as an empty bundle, as it always has. - seen: set[Path] = set() - entries: list[tuple[str, bytes]] = [] - globbed = (path for pattern in _FINGERPRINT_GLOBS for path in base.glob(pattern)) - for path in [*config_py_files(base), *globbed]: - rel = path.relative_to(base).as_posix() - try: - # A stat that fails for any reason but absence is an unreadable file, not a missing - # one: Path.is_file would answer False and the file would drop out unremarked. - if not _is_file(path): - continue - resolved = path.resolve() - if resolved in seen: - continue - seen.add(resolved) - data = path.read_bytes() - except FileNotFoundError: - if strict: - raise - continue # removed since the listing: gone, as _is_file judges a gone path - except OSError: - if strict: - raise - unreadable.add(rel) - continue - entries.append((rel, hashlib.sha256(data).digest())) - entries.sort() - return entries - - -def _require_listable(base: Path, *, unlistable: set[str] | None = None) -> None: - """Raise unless ``base`` and each folder a glob reads in it can be listed. With ``unlistable``, - a folder that cannot be listed is added to it, by its path relative to ``base`` (``.`` for - ``base``), instead of raising; a missing ``base`` still raises. - - ``glob`` swallows a listing error, so a missing or unlistable directory would digest as an empty - bundle, and an unlistable ``codesets`` folder as a bundle without it. A missing ``base`` raises - :class:`ConfigDirMissing`, and so does one whose name no directory can have - (:func:`is_missing_directory`); any other fault raises its own ``OSError``. A sub-folder that - is missing by the same rule is fine: there is nothing in it to leave out.""" - missing = False - try: - with os.scandir(base): - pass - except OSError as exc: - if not is_missing_directory(exc): - if unlistable is None: - raise - unlistable.add(".") # nothing under it can be listed either - return - missing = True - if missing: - # Raised after the handler, so the error it replaces does not ride __context__. - raise ConfigDirMissing(errno.ENOENT, "config directory not found", str(base)) - for folder in sorted({Path(pattern).parent for pattern in _FINGERPRINT_GLOBS} - {Path(".")}): - try: - with os.scandir(base / folder): - pass - except OSError as exc: - # The same rule as base: a folder no path can open holds nothing the loader reads. - if not is_missing_directory(exc): - if unlistable is None: - raise - unlistable.add(folder.as_posix()) - - -def _is_file(path: Path) -> bool: - """Whether ``path`` is a regular file, following links. ``Path.is_file`` answers ``False`` for a - path it cannot stat, so this stats it instead and raises that error. A path that is gone (a - dangling link, or a file removed since the listing) is not a file.""" - try: - return stat.S_ISREG(path.stat().st_mode) - except FileNotFoundError: - return False + byte-identical bundles at different paths fingerprint identically.""" + return sorted((rel, hashlib.sha256(data).digest()) for rel, data in snapshot.files.items()) def _fold(entries: list[tuple[str, bytes]]) -> str: @@ -212,6 +104,17 @@ def _fold(entries: list[tuple[str, bytes]]) -> str: return h.hexdigest() +def snapshot_fingerprint(snapshot: ConfigSnapshot) -> str: + """The content digest of ``snapshot``: the bytes one load read, and so the bytes it ran. + + A digest that gates a load, such as the one a dual-control release checks, is taken from the + snapshot that load then runs, so nothing can change between the check and the run (vault + BACKLOG #2596). Its files left out as unreadable, if any, are not covered: a caller that gates + on it checks :attr:`~messagefoundry.config.snapshot.ConfigSnapshot.unreadable` first, or reads + the snapshot strictly.""" + return _fold(_entries(snapshot)) + + def config_fingerprint(directory: str | Path, *, strict: bool = False) -> str: """Return a stable SHA-256 hex digest over the content of a config bundle's loaded files. @@ -221,10 +124,12 @@ def config_fingerprint(directory: str | Path, *, strict: bool = False) -> str: the approval never covered, so ``strict`` raises instead: :class:`ConfigDirMissing` when ``directory`` is not a directory, and the ``OSError`` when a folder it reads cannot be listed or a file it covers cannot be stat'ed or read.""" - return _fold(_iter_entries(Path(directory), strict=strict, unreadable=set())) + return snapshot_fingerprint(read_config_snapshot(directory, strict=strict)) -def config_fingerprint_detail(directory: str | Path) -> dict[str, object]: +def config_fingerprint_detail( + directory: str | Path, *, snapshot: ConfigSnapshot | None = None +) -> dict[str, object]: """Return the fingerprint plus the file count and a best-effort git HEAD, for an audit detail. ``git_head`` is included only when ``directory`` resolves inside a git work tree whose commit can @@ -232,6 +137,10 @@ def config_fingerprint_detail(directory: str | Path) -> dict[str, object]: ``fingerprint`` is). Keys: ``fingerprint`` (str), ``scheme`` (str, :data:`FINGERPRINT_SCHEME`), ``files`` (int), and optionally ``git_head``. + With ``snapshot``, the digest is of that snapshot, the bytes a load read and ran, and the + directory is read for the git HEAD alone (vault BACKLOG #2596). Without it, the directory is + read now. + It stays best-effort: a file it cannot read is left out of the digest. But it says so, under :data:`UNREADABLE_FILES_KEY` (the files and folders left out), present only when one was. Two digests describe the same files only when they left out the same ones: a file held by an @@ -240,15 +149,16 @@ def config_fingerprint_detail(directory: str | Path) -> dict[str, object]: dual-control approval binds is :func:`config_fingerprint` with ``strict``, which raises instead. """ base = Path(directory) - unreadable: set[str] = set() - entries = _iter_entries(base, strict=False, unreadable=unreadable) + if snapshot is None: + snapshot = read_config_snapshot(base, strict=False) + entries = _entries(snapshot) detail: dict[str, object] = { "fingerprint": _fold(entries), "scheme": FINGERPRINT_SCHEME, "files": len(entries), } - if unreadable: - detail[UNREADABLE_FILES_KEY] = sorted(unreadable) + if snapshot.unreadable: + detail[UNREADABLE_FILES_KEY] = list(snapshot.unreadable) head = _git_head(base) if head is not None: detail["git_head"] = head diff --git a/messagefoundry/config/snapshot.py b/messagefoundry/config/snapshot.py new file mode 100644 index 000000000..2fabd6ed0 --- /dev/null +++ b/messagefoundry/config/snapshot.py @@ -0,0 +1,377 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Foundation, LLC and contributors +"""One read of a config bundle: the bytes a load hashes, compiles and runs (vault BACKLOG #2596). + +A load used to read the config directory several times: Python's source loader read each module +(and preferred a matching ``__pycache__`` file to it), the code-set and ``connections.toml`` loaders +read their own files, and the content digest read everything again. So the digest an approval named +and an audit row recorded could describe bytes other than the ones that ran, and a planted cache +file ran under an unchanged digest. + +A :class:`ConfigSnapshot` is the one read. :func:`messagefoundry.config.wiring.load_config` compiles +each module from the snapshot's bytes itself, so no cache file is ever consulted, and parses the +code sets and ``connections.toml`` from the same snapshot. The ADR 0041 content fingerprint folds +the same bytes (:mod:`messagefoundry.config.fingerprint`), and a ``[sandbox].mode=subprocess`` +worker is handed the snapshot instead of a directory, so a respawn runs what the engine loaded. + +This module reads and classifies bytes; it hashes nothing, so it adds no entry to the crypto +inventory.""" + +from __future__ import annotations + +import errno +import os +import stat +from collections.abc import Mapping +from dataclasses import dataclass, field +from pathlib import Path, PurePath +from types import MappingProxyType + +from messagefoundry.config.code_sets import CODESETS_DIR_NAME, is_code_set_name + +__all__ = [ + "CONNECTIONS_REL", + "ConfigDirMissing", + "ConfigSnapshot", + "NotInSnapshot", + "config_py_files", + "is_missing_directory", + "read_config_snapshot", +] + +#: The relative path ``connections.toml`` is recorded under, whichever file was read for it (the live +#: one, or the candidate an edit is validating). +CONNECTIONS_REL = "connections.toml" + +#: The folder of per-environment value files the ADR 0041 fingerprint has always covered under the +#: config directory. **Covered, never loaded:** the loader reads none of them. The value file a serve +#: start actually reads sits under ``[environments].base_dir``, which by default is not the config +#: directory. Where it is this folder, a reload's value provider parses the bytes this snapshot read +#: for it (:meth:`ConfigSnapshot.environment_file`); a value file elsewhere is not read here. +_ENVIRONMENTS_DIR = "environments" + +#: The folders a snapshot lists, beside the directory itself. +_FOLDERS: tuple[str, ...] = (CODESETS_DIR_NAME, _ENVIRONMENTS_DIR) + + +class NotInSnapshot(LookupError): + """A value file outside the snapshot's ``environments/`` folder, whose bytes the snapshot did + not read and its digest does not cover (:meth:`ConfigSnapshot.environment_file`).""" + + +class ConfigDirMissing(FileNotFoundError): + """The directory a strict read was asked for is not a directory. Its own type, so a caller can + tell it from a covered file that vanished, which is a plain ``FileNotFoundError``.""" + + +#: The Windows errors that refuse a path's name itself: ERROR_INVALID_NAME (123), ERROR_BAD_PATHNAME +#: (161) and ERROR_FILENAME_EXCED_RANGE (206). Python maps the first to EINVAL, not ENOENT. +_INVALID_NAME_WINERRORS: frozenset[int] = frozenset({123, 161, 206}) + + +def is_missing_directory(exc: OSError) -> bool: + """Whether ``exc``, raised resolving or listing a directory, says no directory can be opened by + that path: nothing by that name, a file rather than a directory, a name or whole path too long + for the filesystem (ENAMETOOLONG, or Windows error 206), or a name Windows refuses, such as + ``sub?``. ``Path.is_dir`` answers ``False`` for all of them, so a caller that lists the + directory instead reports them as missing too (vault BACKLOG #2596). Any other error, such as + access denied, is not a missing directory.""" + return ( + isinstance(exc, (FileNotFoundError, NotADirectoryError)) + or exc.errno == errno.ENAMETOOLONG + or getattr(exc, "winerror", None) in _INVALID_NAME_WINERRORS + ) + + +def config_py_files(directory: Path) -> list[Path]: + """Every ``*.py`` in ``directory`` the loader may run or import, sorted: dot-named files excluded. + + ``Path.glob("*.py")`` matches ``.IB_OLD.py`` (an editor or manual backup) and ``._IB_ACME_ADT.py`` + (a macOS AppleDouble file left by an SMB copy) too, because pathlib has no hidden-file rule. Neither + is config, so the loader, both config-source trust checks and the fingerprint skip a dot-named + module the same way (vault BACKLOG #2781). The ``_``-helper skip is the loader's own rule on top of + this, not part of it: a helper is still run when a sibling imports it, so the trust check and the + fingerprint keep it.""" + return sorted(p for p in directory.glob("*.py") if not p.name.startswith(".")) + + +def _covered_name(folder: str, name: str) -> bool: + if folder == CODESETS_DIR_NAME: + return is_code_set_name(name) + return Path(name).suffix.lower() == ".toml" + + +def _covered_only(rel: str) -> bool: + """Whether ``rel`` (a file, or a folder that could not be listed) is one the fingerprint + covers and no load reads.""" + return rel == _ENVIRONMENTS_DIR or rel.startswith(f"{_ENVIRONMENTS_DIR}/") + + +def _is_module_rel(rel: str) -> bool: + return "/" not in rel and rel.lower().endswith(".py") and not rel.startswith(".") + + +def _plain_name(part: str) -> bool: + """Whether ``part`` names one entry of a folder on this platform, and nothing else: joined to + a directory it stays in that directory, and its ``name`` is itself. On Windows that refuses a + backslash and a drive (``C:x.py``); on POSIX both are ordinary characters, and a file so named + is one the listing can return, so it is accepted there.""" + return part not in ("", ".", "..") and PurePath(part).name == part and not PurePath(part).anchor + + +def _check_rel(rel: object) -> str: + """``rel`` when it is a relative path a snapshot can hold, else ``ValueError``. A snapshot can + arrive from another process (the sandbox boot frame), so its names are checked, not trusted: + a module at the top level, ``connections.toml``, or one file in ``codesets/`` or + ``environments/``, each part a plain name on this platform (:func:`_plain_name`). Nothing here + is ever opened, but a module's name is joined to the snapshot's directory and looked up again + by that path's ``name``, so a part that is not a plain name could select another file's bytes. + Every name :func:`read_config_snapshot` lists on a platform passes there.""" + if not isinstance(rel, str) or not rel or "\0" in rel: + raise ValueError(f"config snapshot path {rel!r} is not a plain relative path") + parts = rel.split("/") + if not all(_plain_name(part) for part in parts): + raise ValueError(f"config snapshot path {rel!r} is not a plain relative path") + if len(parts) == 1 and (rel == CONNECTIONS_REL or _is_module_rel(rel)): + return rel + if len(parts) == 2 and parts[0] in _FOLDERS and _covered_name(parts[0], parts[1]): + return rel + raise ValueError(f"config snapshot path {rel!r} is not a file a config load reads") + + +@dataclass(frozen=True, slots=True, eq=False) +class ConfigSnapshot: + """The bytes of every file one load of a config directory reads, keyed by relative path. + + ``directory`` is the absolute path the bytes were read from: config modules are compiled under + ``directory / name``, so a traceback and a diagnostic name the file an author edits. + ``files`` maps each relative path (``cfg.py``, ``_helpers.py``, ``connections.toml``, + ``codesets/diets.csv``, ``environments/dev.toml``) to its bytes. ``unreadable`` names each file, + or folder, that could not be read and is left out; a strict read raises instead, so it is empty + there. The ``environments/`` files are covered by the fingerprint and never loaded as config; + a reload's value provider may parse one (:meth:`environment_file`).""" + + directory: Path + files: Mapping[str, bytes] + unreadable: tuple[str, ...] = () + _modules: tuple[str, ...] = field(init=False, repr=False) + + def __post_init__(self) -> None: + checked = {_check_rel(rel): bytes(data) for rel, data in self.files.items()} + object.__setattr__(self, "files", MappingProxyType(dict(sorted(checked.items())))) + # In path order, as the loader ran them when it globbed the directory: without case on + # Windows, so a bundle's load order, and which file a duplicate-name refusal names, stay + # what they were. + modules = (rel for rel in self.files if _is_module_rel(rel)) + object.__setattr__(self, "_modules", tuple(sorted(modules, key=PurePath))) + + @property + def modules(self) -> tuple[str, ...]: + """Every config module's file name, in path order (without case on Windows), + ``_``-prefixed helpers included.""" + return self._modules + + @property + def connections(self) -> bytes | None: + """The bytes of ``connections.toml``, or ``None`` when there is none.""" + return self.files.get(CONNECTIONS_REL) + + @property + def code_set_files(self) -> dict[str, bytes]: + """Each table in ``codesets/``, by its file name there.""" + prefix = f"{CODESETS_DIR_NAME}/" + return { + rel[len(prefix) :]: data for rel, data in self.files.items() if rel.startswith(prefix) + } + + @property + def unreadable_loaded(self) -> tuple[str, ...]: + """The unreadable entries a load would have read, as against those only the fingerprint + covers. A load refuses when there is any: it cannot run bytes it does not have.""" + return tuple(rel for rel in self.unreadable if not _covered_only(rel)) + + def covers_environment_file(self, path: str | Path) -> bool: + """Whether ``path`` is in this snapshot's ``environments/`` folder, so the snapshot's digest + covers the file there, or that there is none (:func:`_same_folder`).""" + return _same_folder(Path(path).parent, self.directory / _ENVIRONMENTS_DIR) + + def environment_file(self, path: str | Path) -> bytes | None: + """The bytes this snapshot read for the value file ``path``, which + :meth:`covers_environment_file` must accept, or ``None`` when it read no such file and none + is there now. A caller reading values for a reload parses these instead of reading the file + again, so the values it applies are the bytes the digest covered (vault BACKLOG #2596). + + Raises :class:`NotInSnapshot` for a path the snapshot does not cover. Raises ``OSError`` + where the bytes cannot stand for the file now: the snapshot could not read it or its + folder, read a file that is gone now, or read no file by its name while one is there now, + created since. The caller refuses then; the values are read again on the next reload.""" + value_file = Path(path) + if not self.covers_environment_file(value_file): + raise NotInSnapshot(f"{value_file} is not in the config snapshot's environments folder") + prefix = f"{_ENVIRONMENTS_DIR}/" + want = os.path.normcase(prefix + value_file.name) + if any( + rel == _ENVIRONMENTS_DIR or os.path.normcase(rel) == want for rel in self.unreadable + ): + raise OSError(errno.EACCES, "the config snapshot could not read this value file") + present = os.path.lexists(value_file) + data = self.files.get(prefix + value_file.name) + if data is None and present and os.path.normcase("A") != "A": + # Windows names files without case, so the open of this path reads a file the listing + # spelled otherwise. Only where the path names a file now: in a folder set to tell case + # apart, it does not, and a start would read no file. + data = next( + (data for rel, data in self.files.items() if os.path.normcase(rel) == want), None + ) + if data is None and present: + raise OSError(errno.EEXIST, "the value file was created after the config snapshot") + if data is not None and not present: + # The file's own access list was vetted only if it is still there, so bytes from a + # file gone since are not used. + raise OSError(errno.ENOENT, "the value file the config snapshot read is gone") + return data + + def loaded(self, *, code_sets: bool = True) -> ConfigSnapshot: + """This snapshot without the files only the fingerprint covers: what a sandbox worker + needs to run the same graph, and no more. Without ``code_sets``, the ``codesets/`` tables + are left out too, for a worker handed the engine's parsed tables instead.""" + codesets = f"{CODESETS_DIR_NAME}/" + return ConfigSnapshot( + directory=self.directory, + files={ + rel: data + for rel, data in self.files.items() + if not _covered_only(rel) and (code_sets or not rel.startswith(codesets)) + }, + ) + + +def read_config_snapshot(directory: str | Path, *, strict: bool) -> ConfigSnapshot: + """Read every file a load of ``directory`` reads, once, into a :class:`ConfigSnapshot`. + + The config modules (:func:`config_py_files`), ``connections.toml`` (or the candidate an edit is + validating), each table in ``codesets/`` + (:func:`~messagefoundry.config.code_sets.is_code_set_name`), and each ``environments/*.toml``, + which the fingerprint covers and the loader never reads. Only regular + files, following links, are read: a link is read through, and its target's bytes are what a + load would run. Never a ``__pycache__`` file. + + With ``strict`` a fault raises: :class:`ConfigDirMissing` when ``directory`` is not a directory, + and the ``OSError`` when a folder cannot be listed or a file cannot be stat'ed or read. Without + it, a missing directory reads as an empty snapshot and each fault is recorded in + :attr:`ConfigSnapshot.unreadable` instead. A file removed between the listing and the read is + gone, not unreadable, unless ``strict``.""" + # Lazy: connections_file imports the loader, which imports this module. + from messagefoundry.config.connections_file import connections_file_path + + base = Path(directory) + unreadable: set[str] = set() + try: + listable = _list_folders(base, unlistable=None if strict else unreadable) + except ConfigDirMissing: + if strict: + raise + return ConfigSnapshot(directory=Path(os.path.abspath(base)), files={}) + candidates: list[tuple[str, Path]] = [] + if "." in listable: + candidates += [(path.name, path) for path in config_py_files(base)] + candidates.append((CONNECTIONS_REL, connections_file_path(base))) + for folder in _FOLDERS: + if folder not in listable: + continue + try: + names = sorted(entry.name for entry in os.scandir(base / folder)) + except OSError as exc: + if is_missing_directory(exc): + continue # removed since it was listed: nothing in it to read + if strict: + raise + unreadable.add(folder) + continue + candidates += [ + (f"{folder}/{name}", base / folder / name) + for name in names + if _covered_name(folder, name) + ] + files: dict[str, bytes] = {} + for rel, path in candidates: + try: + # A stat that fails for any reason but absence is an unreadable file, not a missing + # one: Path.is_file would answer False and the file would drop out unremarked. + if not _is_file(path): + continue + data = path.read_bytes() + except FileNotFoundError: + if strict: + raise + continue # removed since the listing: gone, as _is_file judges a gone path + except OSError: + if strict: + raise + unreadable.add(rel) + continue + files[rel] = data + return ConfigSnapshot( + directory=Path(os.path.abspath(base)), files=files, unreadable=tuple(sorted(unreadable)) + ) + + +def _list_folders(base: Path, *, unlistable: set[str] | None) -> set[str]: + """Which of ``base`` (``.``) and each folder a snapshot reads in it exist and can be listed. + + Raise unless each can be listed, or, with ``unlistable``, add each that cannot to it by its + path relative to ``base`` instead. ``glob`` swallows a listing error, so a missing or unlistable + directory would read as an empty bundle. A missing ``base`` raises :class:`ConfigDirMissing`, + and so does one whose name no directory can have (:func:`is_missing_directory`); any other fault + raises its own ``OSError``. A sub-folder that is missing by the same rule is fine: there is + nothing in it to leave out.""" + missing = False + try: + with os.scandir(base): + pass + except OSError as exc: + if not is_missing_directory(exc): + if unlistable is None: + raise + unlistable.add(".") # nothing under it can be listed either + return set() + missing = True + if missing: + # Raised after the handler, so the error it replaces does not ride __context__. + raise ConfigDirMissing(errno.ENOENT, "config directory not found", str(base)) + listable = {"."} + for folder in _FOLDERS: + try: + with os.scandir(base / folder): + pass + except OSError as exc: + # The same rule as base: a folder no path can open holds nothing the loader reads. + if not is_missing_directory(exc): + if unlistable is None: + raise + unlistable.add(folder) + continue + listable.add(folder) + return listable + + +def _same_folder(one: Path, other: Path) -> bool: + """Whether ``one`` and ``other`` name the same folder: the same path after following links + (without case on Windows), which needs neither to exist, or else the same file system object, + which catches a spelling the path comparison misses, such as a second mount point.""" + if os.path.normcase(os.path.realpath(one)) == os.path.normcase(os.path.realpath(other)): + return True + try: + return os.path.samefile(one, other) + except OSError: + return False + + +def _is_file(path: Path) -> bool: + """Whether ``path`` is a regular file, following links. ``Path.is_file`` answers ``False`` for a + path it cannot stat, so this stats it instead and raises that error. A path that is gone (a + dangling link, or a file removed since the listing) is not a file.""" + try: + return stat.S_ISREG(path.stat().st_mode) + except FileNotFoundError: + return False diff --git a/messagefoundry/config/wiring.py b/messagefoundry/config/wiring.py index a7a06b647..dd4737626 100644 --- a/messagefoundry/config/wiring.py +++ b/messagefoundry/config/wiring.py @@ -32,9 +32,11 @@ def handle(msg): import ast import builtins import hashlib +import importlib.abc import importlib.machinery import importlib.util import inspect +import io import ipaddress import logging import math @@ -50,14 +52,15 @@ def handle(msg): from dataclasses import dataclass, field from functools import cache from pathlib import Path -from types import MappingProxyType, ModuleType +from types import CodeType, MappingProxyType, ModuleType from typing import Any, Final, Literal, NamedTuple, TypeIs from messagefoundry.config.code_sets import ( CODESETS_DIR_NAME, CodeSet, CodeSetError, - load_code_sets, + code_sets_from_files, + is_code_set_name, ) from messagefoundry.config.code_sets import ( activated as _code_sets_activated, @@ -89,6 +92,7 @@ def handle(msg): require_hop_reason, ) from messagefoundry.config.send_snapshot import snapshot_on_send_active +from messagefoundry.config.snapshot import ConfigSnapshot, config_py_files, read_config_snapshot from messagefoundry.connection_names import ( CONNECTION_NAME_PATTERN, fhir_lookup_record_name, @@ -5406,9 +5410,13 @@ class Registry: source_dir: Path | None = None # A digest of the config modules' bytes as this load read them # (:func:`config_code_digest`), or - # None for a graph built in code or a module that could not be read. A sandbox worker loads the - # directory again, and the engine refuses one whose digest differs (vault BACKLOG #2724). + # None for a graph built in code or a module that could not be read. The engine refuses a + # sandbox worker whose digest differs (vault BACKLOG #2724). code_digest: str | None = None + # The one read of the directory this graph was loaded from, whose bytes it ran (vault BACKLOG + # #2596), or None for a graph built in code. Its fingerprint is the reload's audit digest, and a + # sandbox worker is sent it, so a worker runs these bytes and never reads the directory. + config_snapshot: ConfigSnapshot | None = field(default=None, repr=False, compare=False) def inbound_names(self) -> frozenset[str]: """Every inbound connection NAME in the deployment — the pinned unfiltered set when this is one @@ -7534,51 +7542,44 @@ def decorate(fn: HandlerFn) -> HandlerFn: _CODE_DIGEST_SCHEME: Final = b"mefor-config-code:v1\n" -def config_code_digest(directory: Path) -> str | None: - """A SHA-256 over every config module a load of ``directory`` may run, ``_`` helpers included: - each file's name and the digest of its bytes, in :func:`config_py_files` order (vault BACKLOG - #2724). +def config_code_digest(source: Path | ConfigSnapshot) -> str | None: + """A SHA-256 over every config module a load may run, ``_`` helpers included: each file's name + and the digest of its bytes, in name order (vault BACKLOG #2724). + ``source`` is the :class:`~messagefoundry.config.snapshot.ConfigSnapshot` a load ran, so the + digest describes the bytes that ran (vault BACKLOG #2596), or a directory, which is read now. The loader records it on the graph, and a sandbox worker's load records its own; the engine refuses a worker whose digest differs. Python modules only: ``connections.toml`` changes the graph's names, which are compared anyway, and the worker serves the code sets the engine hands - it, not its own read. None when a module cannot be read, which is logged. + it. None when a module cannot be read, which is logged. Kept here rather than built on :mod:`messagefoundry.config.fingerprint`: importing that module would give every importer of the loader its comparison and hash operations in the crypto inventory (``scripts/security/crypto_inventory_check.py``).""" - digest = hashlib.sha256(_CODE_DIGEST_SCHEME) - try: - for path in config_py_files(directory): - if not path.is_file(): - continue - digest.update(path.name.encode("utf-8")) - digest.update(b"\0") - digest.update(hashlib.sha256(path.read_bytes()).digest()) - digest.update(b"\0") - except OSError as exc: + # Not strict: only the modules are digested, so a code-set table or value file that cannot be + # read does not take the digest away. + snapshot = ( + source if isinstance(source, ConfigSnapshot) else read_config_snapshot(source, strict=False) + ) + unread = [rel for rel in snapshot.unreadable if rel == "." or rel.lower().endswith(".py")] + if unread: + # A module it could not read is not described by the rest. _logger.warning( "config module digest unavailable for %s, so a sandbox worker's config code is not " - "compared with this load's: %s", - scrub_log_argument(str(directory)), - scrub_log_argument(str(exc)), + "compared with this load's: could not read %s", + scrub_log_argument(str(snapshot.directory)), + scrub_log_argument(", ".join(unread)), ) return None + digest = hashlib.sha256(_CODE_DIGEST_SCHEME) + for name in snapshot.modules: + digest.update(name.encode("utf-8")) + digest.update(b"\0") + digest.update(hashlib.sha256(snapshot.files[name]).digest()) + digest.update(b"\0") return digest.hexdigest() -def config_py_files(directory: Path) -> list[Path]: - """Every ``*.py`` in ``directory`` the loader may run or import, sorted: dot-named files excluded. - - ``Path.glob("*.py")`` matches ``.IB_OLD.py`` (an editor or manual backup) and ``._IB_ACME_ADT.py`` - (a macOS AppleDouble file left by an SMB copy) too, because pathlib has no hidden-file rule. Neither - is config, so the loader, both config-source trust checks and the fingerprint skip a dot-named - module the same way (vault BACKLOG #2781). The ``_``-helper skip is the loader's own rule on top of - this, not part of it: a helper is still run when a sibling imports it, so the trust check and the - fingerprint keep it.""" - return sorted(p for p in directory.glob("*.py") if not p.name.startswith(".")) - - #: How every config-source refusal of a load begins. _CONFIG_SOURCE_LEAD = "refusing to load config from" @@ -7595,10 +7596,12 @@ def config_source_trust_paths(directory: Path) -> list[Path]: with each table the loader reads from it. Both config-source trust checks walk this list (vault BACKLOG #2626 step 3, #2596 step 4). - **The cache folder is the reason for the list.** The loader uses Python's ordinary source loader, - which runs a matching ``.pyc`` in place of the ``.py`` and never compares the two. The checks - used to read the ``.py`` alone, so a cache file nobody vetted could run under a source that - passed (vault BACKLOG #2596). + **The cache folder is on the list although no load reads it.** The loader compiles each module + from the bytes of its snapshot (:func:`load_config`), so a ``.pyc`` there never runs in place of + its ``.py``. It once did: Python's ordinary source loader prefers a matching cache file and never + compares the two, and the checks then read the ``.py`` alone (vault BACKLOG #2596). The folder + stays vetted because a writer of it is a writer inside the config directory, and because tools + other than the loader (``python -m``, an editor's run button) still read it. **Not on the list:** the value file and the settings file, which sit outside the config directory and are checked at ``serve`` start (:func:`assert_trusted_start_paths`), and the @@ -7617,12 +7620,10 @@ def config_source_trust_paths(directory: Path) -> list[Path]: codesets = directory / CODESETS_DIR_NAME if codesets.is_dir(): paths.append(codesets) - # The loader's own rule (code_sets.load_code_sets): a file whose suffix, in lower case, is - # .csv or .toml. Matched the same way here, so a table spelled .CSV is vetted on POSIX too. + # The loader's own rule (code_sets.is_code_set_name), so a table spelled .CSV is vetted on + # POSIX too. paths.extend( - p - for p in sorted(codesets.iterdir()) - if p.is_file() and p.suffix.lower() in (".csv", ".toml") + p for p in sorted(codesets.iterdir()) if p.is_file() and is_code_set_name(p.name) ) return paths @@ -7763,6 +7764,51 @@ def visit(node: ast.AST, in_function: bool, guarded: frozenset[str]) -> None: return found +class _SnapshotLoader(importlib.abc.InspectLoader): + """The import loader for one config module or helper: it serves the bytes the load's + :class:`~messagefoundry.config.snapshot.ConfigSnapshot` holds for ``path``, once. + + It takes the place of the ``SourceFileLoader`` that ``spec_from_file_location`` gave a config + module before (vault BACKLOG #2596). The module still executes through the import system's own + ``exec_module``; only where the bytes come from changed. It has no bytecode methods, so it never + reads or writes a ``__pycache__`` file, and it reads nothing from the directory: a sandbox + worker that was sent the snapshot runs the same bytes. + + **One code object, for the load's own run.** The first :meth:`get_code` drops the snapshot, so + nothing can run the module a second time through its spec (``runpy.run_module`` gets no code + and refuses, as it did when a config module had no loader), and a module kept alive after a + reload holds no snapshot bytes. + + No source and no ``get_data``. :mod:`inspect` and a traceback read the file on disk, which may + hold other bytes than the ones that ran, as before this loader. ``pkgutil.get_data`` and + :mod:`importlib.resources` find no resources beside a config module, since the snapshot holds + only the files a load reads.""" + + def __init__(self, files: Mapping[str, bytes], path: Path) -> None: + self._files: Mapping[str, bytes] | None = files + self._path = path + + def is_package(self, fullname: str) -> bool: + return False + + def get_source(self, fullname: str) -> None: + # No source: linecache would cache what this returned, for a file missing at that moment, + # with no time to check it against, so a later load of the same path would show these lines. + # linecache and inspect read the file, as they did when a config module had no loader. + return None + + def get_code(self, fullname: str) -> CodeType | None: + files, self._files = self._files, None + if files is None: + return None # already run: exec_module and runpy refuse with an ImportError + # compile() here, not InspectLoader.source_to_code, so a compile failure's innermost frame is + # this package's, as _exec_module expects; the lookup is here too, inside its try. Compiled + # from the bytes, so a PEP 263 coding line and a BOM are read from them, as the source loader + # reads them. The file name is the path an author edits, which a traceback and a load + # diagnostic name. + return compile(files[self._path.name], str(self._path), "exec", dont_inherit=True) + + class _HelperImporter: """Resolve a config module's ``import _helpers`` to a sibling ``_helpers.py``, for one load only. @@ -7791,10 +7837,15 @@ class _HelperImporter: builtins are a copy taken when the load starts, so a name added to :mod:`builtins` later is not seen by config code; a live view would cost a Python-level lookup on every builtin name a Handler reads, on the per-message path. :func:`_assert_safe_config_source` vets every ``*.py`` - (including ``_*``), so a helper sits inside the same trust boundary as its importers.""" + (including ``_*``), so a helper sits inside the same trust boundary as its importers. + + Every helper is compiled from the load's :class:`~messagefoundry.config.snapshot.ConfigSnapshot`, + never read from the directory, so a helper runs the bytes the load hashed, and a ``__pycache__`` + file is never consulted (vault BACKLOG #2596).""" - def __init__(self, directory: Path) -> None: - self._dir = directory + def __init__(self, snapshot: ConfigSnapshot) -> None: + self._snapshot = snapshot + self._dir = snapshot.directory self._modules: dict[str, ModuleType] = {} self._open = True # Helper files whose body imports are not yet loaded, drained by preload(). @@ -7807,9 +7858,10 @@ def __init__(self, directory: Path) -> None: self._failed: dict[str, tuple[ImportError, str]] = {} # The helper stems present when the load starts: the only names this importer serves, so an # ordinary ``import json`` in a Handler body costs one set lookup before the real import. - self._stems = frozenset( - p.stem for p in config_py_files(directory) if _is_helper_name(p.stem) - ) + self._files = { + Path(name).stem: name for name in snapshot.modules if _is_helper_name(Path(name).stem) + } + self._stems = frozenset(self._files) self.builtins: dict[str, Any] = {**vars(builtins), "__import__": self._import} def close(self) -> None: @@ -7889,21 +7941,16 @@ def _import( ) from failed return builtins.__import__(name, globals, locals, fromlist, level) - def _load(self, name: str) -> ModuleType | None: - path = self._dir / f"{name}.py" - if not path.is_file(): - return None # gone since the load started: resolved the normal way + def _load(self, name: str) -> ModuleType: + path = self._dir / self._files[name] _refuse_shadowing_helper(name, path) mod_name = _config_module_name(path) - spec = importlib.util.spec_from_file_location(mod_name, path) - if spec is None or spec.loader is None: - raise WiringError(f"cannot load config helper: {path}") - module = self.new_module(spec) + module = self.new_module(mod_name, path) # Cached before it runs, as Python's own import does, so a helper cycle sees the partial module. self._modules[name] = module self.register(mod_name, module) try: - spec.loader.exec_module(module) + self.run(module, path) except BaseException: del self._modules[name] self.unregister(mod_name, module) @@ -7911,12 +7958,44 @@ def _load(self, name: str) -> ModuleType | None: self._pending.append(path) return module - def new_module(self, spec: importlib.machinery.ModuleSpec) -> ModuleType: - """A module for ``spec`` whose own ``import`` statements resolve helpers through this load.""" + def new_module(self, mod_name: str, path: Path) -> ModuleType: + """A module named ``mod_name`` for the config file at ``path``, whose own ``import`` + statements resolve helpers through this load. + + Its spec names ``path`` as its origin, so ``__file__``, a traceback and a diagnostic name + the file an author edits, and its loader is a :class:`_SnapshotLoader` over this load's + snapshot, so it never finds a cache file. ``importlib.reload`` still refuses it: no finder + on ``sys.meta_path`` knows its mangled name, so nothing runs it again from disk.""" + # Built by hand with has_location, as spec_from_file_location builds one, so the module + # gets a __file__; spec_from_loader would leave it without one. + loader = _SnapshotLoader(self._snapshot.files, path) + spec = importlib.machinery.ModuleSpec(mod_name, loader, origin=str(path)) + spec.has_location = True # sets __file__ from the origin module = importlib.util.module_from_spec(spec) + module.__dict__["__cached__"] = None # no cache file is ever read for it module.__dict__["__builtins__"] = self.builtins return module + def run(self, module: ModuleType, path: Path) -> None: + """Run ``module``'s body the way ``import`` runs one, by its spec's loader: the + :class:`_SnapshotLoader` :meth:`new_module` gave it, over the snapshot's bytes for ``path``. + + Config modules are code by design (CLAUDE.md section 1); the directory's trust check (ADR + 0036) is what holds them, and docs/DANGEROUS-FUNCTIONALITY.md section 2 names this site.""" + spec = module.__spec__ + if spec is None or not isinstance(spec.loader, _SnapshotLoader): # pragma: no cover + raise WiringError(f"error loading config file {path.name}: not a snapshot module") + spec.loader.exec_module(module) + + def source_of(self, filename: str) -> bytes | None: + """The snapshot bytes a module compiled under ``filename`` ran, or ``None`` for a file + this load did not compile. A diagnostic reads its line from these rather than from the + directory, which may hold other bytes by now and which a sandbox worker never reads.""" + path = Path(filename) + if path.parent != self._dir: + return None + return self._snapshot.files.get(path.name) + def preload(self, path: Path) -> None: """Load every helper a function body in ``path`` imports -- and, in turn, every helper those helpers' bodies import -- so an in-body import works after the load. Called once ``path`` has @@ -7928,10 +8007,8 @@ def preload(self, path: Path) -> None: def _preload_one(self, current: Path) -> None: if not self._stems: return - try: - source = current.read_bytes() - except OSError: - return # it already ran; the file going away now is not this pass's to report + # The bytes that ran, from the snapshot: the directory is never read again. + source = self._snapshot.files[current.name] for name, caught in _body_helper_imports(source, self._stems): if name in self._modules: continue @@ -7962,7 +8039,9 @@ def _preload_one(self, current: Path) -> None: # load. Names only, never the exception text (CLAUDE.md section 9). self._failed[name] = ( _textless_import_error(exc), - _failure_detail(self._dir.resolve(), f"{name}.py", exc), + _failure_detail( + self._dir.resolve(), self._files[name], exc, source_of=self.source_of + ), ) _logger.warning( "config helper %s failed to load (%s) and is left to the ImportError guard in %s", @@ -7978,16 +8057,16 @@ def _preload_one(self, current: Path) -> None: @contextmanager -def _loading(directory: Path, registry: Registry) -> Iterator[_HelperImporter]: +def _loading(snapshot: ConfigSnapshot, registry: Registry) -> Iterator[_HelperImporter]: """Hold the load lock, publish ``registry`` as the active declaration target **and its code sets as the active set** (so a module-top-level ``code_set(...)`` resolves), and yield the load's - :class:`_HelperImporter`, closed on exit so it serves only what this load imported. A load that - raises inside this block gets back, in ``sys.modules``, whatever each of its names held before, so - a live graph loaded earlier from the same files keeps its modules. (A failure after the block -- - graph validation, the connections.toml merge -- leaves this load's entries, as it leaves its config - modules'.)""" + :class:`_HelperImporter` over ``snapshot``, closed on exit so it serves only what this load + imported. A load that raises inside this block gets back, in ``sys.modules``, whatever each of + its names held before, so a live graph loaded earlier from the same files keeps its modules. (A + failure after the block -- graph validation, the connections.toml merge -- leaves this load's + entries, as it leaves its config modules'.)""" global _active - helpers = _HelperImporter(directory) + helpers = _HelperImporter(snapshot) with _load_lock: _active = registry # Code sets are published BEFORE the modules run so a top-level capture resolves; the registry @@ -8003,7 +8082,50 @@ def _loading(directory: Path, registry: Registry) -> Iterator[_HelperImporter]: helpers.close() -def load_config(directory: str | Path, *, allow_empty: bool = False) -> Registry: +def read_trusted_snapshot(directory: str | Path, *, strict: bool = False) -> ConfigSnapshot: + """Read the config directory a load runs, after its trust check: the one read of it. + + Refuses a missing directory (:class:`~messagefoundry.config.fingerprint.ConfigDirMissing`), a + source :func:`_assert_safe_config_source` refuses, and a file the load needs but cannot read + (``WiringError``). A file only the fingerprint covers (``environments/*.toml``) that cannot be + read is left out and named in :attr:`ConfigSnapshot.unreadable`, as the observational + fingerprint names it. With ``strict``, any file or folder it cannot read raises its + ``OSError`` instead, so the snapshot's digest covers every file the fingerprint names: the + digest a dual-control release checks. Blocking.""" + directory = Path(directory) + # Fail loudly on a missing/typo'd dir: Path.glob() on a nonexistent dir yields nothing, so the + # engine would otherwise start with an empty graph — a silently dead interface (review M-24). + if not directory.is_dir(): + # ConfigDirMissing, a FileNotFoundError: a reload maps only it to not_found, so a directory + # moved away after the reload's own check still answers as missing (BACKLOG #3157). + from messagefoundry.config.snapshot import ConfigDirMissing + + raise ConfigDirMissing(f"config directory not found: {directory}") + _assert_safe_config_source(directory) + snapshot = read_config_snapshot(directory, strict=strict) + if not strict and not snapshot.files and not directory.is_dir(): + # A lax read reads a vanished directory as an empty one; a load must not run that as an + # empty graph, so it still answers as missing (BACKLOG #3157). + from messagefoundry.config.snapshot import ConfigDirMissing + + raise ConfigDirMissing(f"config directory not found: {directory}") + _require_loadable(snapshot) + return snapshot + + +def _require_loadable(snapshot: ConfigSnapshot) -> None: + """Refuse a snapshot missing a file a load would read: it cannot run bytes it does not have.""" + missing = snapshot.unreadable_loaded + if missing: + raise WiringError( + f"refusing to load config from {snapshot.directory}: could not read " + f"{', '.join(missing)}; see the engine log and docs/SERVICE.md for required permissions" + ) + + +def load_config( + directory: str | Path, *, allow_empty: bool = False, snapshot: ConfigSnapshot | None = None +) -> Registry: """Load every ``*.py`` config module in ``directory`` (sorted; ``_*`` skipped) into a Registry. Config modules are **executed** in-process with the engine's full privilege, so the source @@ -8011,72 +8133,83 @@ def load_config(directory: str | Path, *, allow_empty: bool = False) -> Registry group/world-writable directory before any code runs. Blocking: an async caller (engine reload) should run this via ``asyncio.to_thread`` so heavy user-config imports don't stall listeners. + **One read, then the bytes it read** (vault BACKLOG #2596). The directory is read once, into a + :class:`~messagefoundry.config.snapshot.ConfigSnapshot` (:func:`read_trusted_snapshot`), and + everything the load runs or parses comes from it: each module and helper is compiled from its + bytes by the loader itself, so a ``__pycache__`` file is never consulted, and the code sets and + ``connections.toml`` are parsed from it. The graph carries the snapshot + (:attr:`Registry.config_snapshot`), so its fingerprint and a sandbox worker use the same bytes. + ``snapshot`` is one the caller already read with :func:`read_trusted_snapshot`, such as the one a + dual-control release checked against its approval; the directory is then not read again. + ``allow_empty`` is passed straight to :meth:`Registry.validate` and suppresses the empty-graph refusal only. It exists for ``messagefoundry check --allow-empty-config`` (BACKLOG #1648) and is deliberately NOT reachable from ``serve``: a running engine must never start on a graph that would receive and send nothing.""" directory = Path(directory) - # Fail loudly on a missing/typo'd dir: Path.glob() on a nonexistent dir yields nothing, so the - # engine would otherwise start with an empty graph — a silently dead interface (review M-24). - if not directory.is_dir(): - # ConfigDirMissing, a FileNotFoundError: a reload maps only it to not_found, so a directory - # moved away after the reload's own check still answers as missing (BACKLOG #3157). - from messagefoundry.config.fingerprint import ConfigDirMissing + if snapshot is None: + snapshot = read_trusted_snapshot(directory) + # Resolved, so the graph names the directory it came from even if a link in the path moves. + return load_snapshot(snapshot, allow_empty=allow_empty, source_dir=directory.resolve()) - raise ConfigDirMissing(f"config directory not found: {directory}") - _assert_safe_config_source(directory) - # Resolved, so a worker that spawns later loads this target even if a link in the path moves. - # The digest is read just before the modules run (vault BACKLOG #2724). - registry = Registry(source_dir=directory.resolve(), code_digest=config_code_digest(directory)) + +def load_snapshot( + snapshot: ConfigSnapshot, + *, + allow_empty: bool = False, + source_dir: Path | None = None, + code_sets: Mapping[str, CodeSet] | None = None, +) -> Registry: + """Load the graph ``snapshot`` holds, reading nothing from disk. + + :func:`load_config` once the directory is read and checked. A ``[sandbox].mode=subprocess`` + worker calls it with the snapshot its engine sent, so a worker, a respawned one included, runs + the bytes the engine loaded and never reads the directory (vault BACKLOG #2596). The trust + check belongs to whoever read the snapshot; this function runs what it is given. + ``source_dir`` is recorded as :attr:`Registry.source_dir`, the snapshot's directory when + omitted. ``code_sets``, when given, are the tables the load publishes in place of parsing the + snapshot's ``codesets/``: a worker is sent the engine's parsed tables and not their files, so + one boot frame does not carry each table twice.""" + _require_loadable(snapshot) + directory = snapshot.directory + registry = Registry( + source_dir=source_dir if source_dir is not None else directory, + code_digest=config_code_digest(snapshot), + config_snapshot=snapshot, + ) # Load the bundle's reference tables (codesets/ relative to the config dir) BEFORE importing the # config modules, so a module-top-level code_set(...) capture resolves. A bad/duplicate table is a # WiringError here (fail loud), like a bad env value; a missing codesets/ dir is fine (no tables). - try: - registry.code_sets = load_code_sets(directory / CODESETS_DIR_NAME) - except CodeSetError as exc: - raise WiringError(str(exc)) from exc - with _loading(directory, registry) as helpers: - for path in config_py_files(directory): - if not path.name.startswith("_"): - _exec_module(path, helpers) - if registry.code_digest is not None: - # Inside the block, so a refusal discards this load's modules like any failed load. - _require_unchanged_code(directory, registry.code_digest) + if code_sets is not None: + registry.code_sets = dict(code_sets) + else: + try: + registry.code_sets = code_sets_from_files( + snapshot.code_set_files, directory / CODESETS_DIR_NAME + ) + except CodeSetError as exc: + raise WiringError(str(exc)) from exc + with _loading(snapshot, registry) as helpers: + for name in snapshot.modules: + if not name.startswith("_"): + _exec_module(directory / name, helpers) # Connections may also be authored as data (ADR 0007): merge connections.toml into the SAME # registry the code-first inbound()/outbound() calls populated, before validating the whole graph. # Imported lazily to avoid a wiring<->connections_file import cycle. A name in both surfaces is a # duplicate WiringError via add_inbound/add_outbound (no silent precedence). from messagefoundry.config.connections_file import ( connections_file_path, - load_connections_file, + load_connections_data, ) - # The live file, or the candidate a `connections_edit` write is validating before it replaces the - # live one (vault BACKLOG #2782). - conn_file = connections_file_path(directory) - if conn_file.is_file(): - load_connections_file(conn_file, registry) + if snapshot.connections is not None: + # Named as the live file, or the candidate a `connections_edit` write is validating before + # it replaces the live one (vault BACKLOG #2782); the bytes are the snapshot's. + load_connections_data(snapshot.connections, connections_file_path(directory), registry) registry.validate(allow_empty=allow_empty) return registry -def _require_unchanged_code(directory: Path, before: str) -> None: - """Refuse a load whose config modules changed on disk while it ran (vault BACKLOG #2724). - - The digest was read before the modules ran. A module rewritten in between ran bytes the digest - does not describe, and a sandbox worker would then pass the comparison with edited code.""" - after = config_code_digest(directory) - if after is None: - raise WiringError( - f"config modules in {directory} could not be read again after the load, so what ran " - "cannot be shown to match what was read; load again" - ) - if after != before: # both are public digests of public bytes; nothing to time - raise WiringError( - f"config modules in {directory} changed on disk while they were loading; load again" - ) - - # Group-write (0o020) | world-write (0o002): a writable bit for anyone but the owner. _GROUP_WORLD_WRITABLE = 0o022 @@ -8919,7 +9052,7 @@ def _file_and_folder(files: Iterable[Path]) -> list[Path]: def assert_trusted_start_paths( - files: Iterable[Path], *, lead: str = "refusing to start from" + files: Iterable[Path], *, lead: str = "refusing to start from", folders: Iterable[Path] = () ) -> None: """Refuse a start that reads a settings file or value file another account could write. @@ -8935,10 +9068,16 @@ def assert_trusted_start_paths( in the graph. A writer of either would own the service at its next start (vault BACKLOG #2626 step 3). + Each of ``folders`` that exists is checked too, whether or not a file in it does: a reload that + applies value bytes read earlier needs the folder vetted even when the file has gone since + (vault BACKLOG #2596). + **Not checked**, at least: the folders above each file's own folder, and a file named in the settings that the start reads later, such as a key, certificate or CRL file. Those have their own checks where they are read, or none.""" - _check_trusted_paths(_file_and_folder(files), lead=lead, refuse=_refuse_unsafe_config_source) + paths = _file_and_folder(files) + paths += [f.absolute() for f in folders if f.is_dir() and f.absolute() not in paths] + _check_trusted_paths(paths, lead=lead, refuse=_refuse_unsafe_config_source) def install_trust_paths() -> list[Path]: @@ -9003,7 +9142,9 @@ def _raised_by_the_engine(exc: BaseException) -> bool: _WITHHELD: Final = "its message is withheld: it can carry a value the module held" -def _module_failure(path: Path, exc: BaseException) -> str: +def _module_failure( + path: Path, exc: BaseException, source_of: Callable[[str], bytes | None] | None = None +) -> str: """The loader's diagnostic for author code that failed: the exception TYPE, the config-directory file and line it was raised from, and for an import or name error the identifier, but never the exception's text (vault BACKLOG #3249). @@ -9014,11 +9155,18 @@ def _module_failure(path: Path, exc: BaseException) -> str: credential scrub only replaces a LABELLED value, so it is a backstop, not a boundary.""" return ( f"error loading config module {path.name}: " - f"{_failure_detail(path.parent.resolve(), path.name, exc)} ({_WITHHELD})" + f"{_failure_detail(path.parent.resolve(), path.name, exc, source_of=source_of)} " + f"({_WITHHELD})" ) -def _failure_detail(config_dir: Path, own_file: str, exc: BaseException) -> str: +def _failure_detail( + config_dir: Path, + own_file: str, + exc: BaseException, + *, + source_of: Callable[[str], bytes | None] | None = None, +) -> str: """``exc``'s TYPE, the config-directory file and line it was raised from (the file only when it is not ``own_file``), and for an import or name error each identifier the source line spells. @@ -9048,7 +9196,8 @@ def _failure_detail(config_dir: Path, own_file: str, exc: BaseException) -> str: detail += f" at line {line}" if isinstance(exc, _NAMED_EXCEPTIONS): # `from x import y` puts the module in `name` and the symbol in `name_from`. - names_on_line = _dotted_names_on_line(source_file, line) + source = source_of(source_file) if source_of is not None else None + names_on_line = _dotted_names_on_line(source_file, line, source) idents = [ ident for ident in map(_source_name, (exc.name, getattr(exc, "name_from", None))) @@ -9059,34 +9208,47 @@ def _failure_detail(config_dir: Path, own_file: str, exc: BaseException) -> str: return detail -def _dotted_names_on_line(source_file: str, line: int) -> list[str]: +def _dotted_names_on_line(source_file: str, line: int, source: bytes | None = None) -> list[str]: """Every dotted name (``a``, ``a.b.c``) written in code in the statement that starts on, or spans, ``line`` of ``source_file`` -- a parenthesized import over several lines is one statement -- never a string literal's or a comment's content. - The file is read afresh, never through :mod:`linecache`: that cache holds whatever version of the - file was read first, so after an edit and a reload a stale line could vouch for a run-time value. - It is tokenized from the top, so a line inside a multi-line string is read as the string it is. - ``[]`` -- nothing vouched for -- when the file cannot be read or tokenized.""" - runs: list[list[str]] = [[]] + ``source`` is the bytes that ran, from the load's snapshot, so the line read is the line that + raised even when the file has been edited since, and a sandbox worker reads no file (vault + BACKLOG #2596). Without it the file is read afresh, never through :mod:`linecache`: that cache + holds whatever version of the file was read first, so after an edit and a reload a stale line + could vouch for a run-time value. It is tokenized from the top, so a line inside a multi-line + string is read as the string it is. ``[]`` -- nothing vouched for -- when the source cannot be + read or tokenized.""" try: - with tokenize.open(source_file) as f: - for tok in tokenize.generate_tokens(f.readline): - if tok.type in (tokenize.NEWLINE, tokenize.ENDMARKER): - # A logical line ended: keep it only if it spans `line`, else start afresh. - if tok.start[0] >= line: - break - runs = [[]] - elif tok.type == tokenize.NAME or (tok.type == tokenize.OP and tok.string == "."): - runs[-1].append(tok.string) - elif runs[-1]: - runs.append([]) + if source is not None: + # tokenize.tokenize reads a PEP 263 coding line and a BOM from the bytes, as compile did. + runs = _name_runs(tokenize.tokenize(io.BytesIO(source).readline), line) + else: + with tokenize.open(source_file) as f: + runs = _name_runs(tokenize.generate_tokens(f.readline), line) except (OSError, SyntaxError, UnicodeDecodeError, tokenize.TokenError): return [] # "a b" is two runs of names, never one name, so a run is split where two names abut. return [name for run in runs for name in " ".join(run).replace(" . ", ".").split()] +def _name_runs(tokens: Iterable[tokenize.TokenInfo], line: int) -> list[list[str]]: + """The runs of names and dots in the logical line of ``tokens`` that spans ``line``.""" + runs: list[list[str]] = [[]] + for tok in tokens: + if tok.type in (tokenize.NEWLINE, tokenize.ENDMARKER): + # A logical line ended: keep it only if it spans `line`, else start afresh. + if tok.start[0] >= line: + break + runs = [[]] + elif tok.type == tokenize.NAME or (tok.type == tokenize.OP and tok.string == "."): + runs[-1].append(tok.string) + elif runs[-1]: + runs.append([]) + return runs + + def _spelled(ident: str, names: list[str]) -> bool: """Whether dotted ``ident`` is a whole run of parts in one of ``names``: ``b`` and ``a.b`` are spelled by ``a.b.c``, but ``ab`` is not spelled by ``abc`` nor by ``"ab"``.""" @@ -9135,22 +9297,25 @@ def _exec_module(path: Path, helpers: _HelperImporter) -> None: # Register the module in sys.modules under its path-derived name so intra-config imports and # anything relying on sys.modules[__name__] resolve correctly; remove it again on failure. mod_name = _config_module_name(path) - spec = importlib.util.spec_from_file_location(mod_name, path) - if spec is None or spec.loader is None: - raise WiringError(f"cannot load config module: {path}") - module = helpers.new_module(spec) + module = helpers.new_module(mod_name, path) helpers.register(mod_name, module) withheld: WiringError | None = None cause: ImportError | None = None try: - spec.loader.exec_module(module) + helpers.run(module, path) # The helpers its function bodies import, loaded now that it has run (vault BACKLOG #2783). helpers.preload(path) except Exception as exc: helpers.unregister(mod_name, module) # An import or name error is judged by its identifier, not its frame: a failed import ends - # in this package's own helper finder whoever wrote the import. - if _raised_by_the_engine(exc) and not isinstance(exc, _NAMED_EXCEPTIONS): + # in this package's own helper finder whoever wrote the import. A SyntaxError is the + # author's source failing to compile; the loader compiles it, so its innermost frame is + # this package's, and it is judged as the author's failure it is. + if ( + _raised_by_the_engine(exc) + and not isinstance(exc, _NAMED_EXCEPTIONS) + and not isinstance(exc, SyntaxError) + ): # The engine's own wording -- a factory or registry refusal naming a connection or a # setting -- which is the diagnostic an author needs, so it keeps its text, as before. # That relies on engine raise sites not quoting a value. At least a few wrap another @@ -9160,7 +9325,7 @@ def _exec_module(path: Path, helpers: _HelperImporter) -> None: raise raise WiringError(f"error loading config module {path.name}: {exc}") from exc try: - withheld = WiringError(_module_failure(path, exc)) + withheld = WiringError(_module_failure(path, exc, helpers.source_of)) except Exception: # noqa: BLE001 - a fault building the detail must not escape with `exc` # chained as its context; the type alone is still a withheld diagnostic. withheld = WiringError( @@ -9199,23 +9364,33 @@ def validate_config(directory: str | Path, *, allow_empty: bool = False) -> list _assert_safe_config_source(directory) except WiringError as exc: return [Diagnostic(message=str(exc), file=str(directory))] + # One read, as load_config's, so the modules, tables and connections it reports on are one + # set of bytes (vault BACKLOG #2596). A file it needs but cannot read is a diagnostic. + snapshot = read_config_snapshot(directory, strict=False) registry = Registry() - diagnostics: list[Diagnostic] = [] + diagnostics: list[Diagnostic] = [ + Diagnostic(message=f"could not read {rel}", file=str(directory / rel)) + for rel in snapshot.unreadable_loaded + ] # Tracked separately from `diagnostics` on purpose — see the empty-graph note at the end of this # function. Only a source that could have DECLARED a connection counts: a module or # connections.toml. A bad code-set table is a diagnostic that explains nothing about emptiness. - declaring_source_failed = False + declaring_source_failed = any( + rel != CODESETS_DIR_NAME and not rel.startswith(f"{CODESETS_DIR_NAME}/") + for rel in snapshot.unreadable_loaded + ) # Load reference tables first (so a module-top-level code_set(...) resolves during import). A # bad/duplicate table is recorded as a diagnostic, not raised, so the editor sees every problem. codesets_dir = directory / CODESETS_DIR_NAME try: - registry.code_sets = load_code_sets(codesets_dir) + registry.code_sets = code_sets_from_files(snapshot.code_set_files, codesets_dir) except CodeSetError as exc: diagnostics.append(Diagnostic(message=str(exc), file=str(codesets_dir))) - with _loading(directory, registry) as helpers: - for path in config_py_files(directory): - if path.name.startswith("_"): + with _loading(snapshot, registry) as helpers: + for name in snapshot.modules: + if name.startswith("_"): continue + path = snapshot.directory / name try: _exec_module(path, helpers) except WiringError as exc: @@ -9231,15 +9406,15 @@ def validate_config(directory: str | Path, *, allow_empty: bool = False) -> list from messagefoundry.config.connections_file import ( CONNECTIONS_FILE_NAME, connections_file_path, - load_connections_file, + load_connections_data, ) # The candidate a `connections_edit` write is validating, when this runs inside one (vault # BACKLOG #2782), as `load_config` does. conn_file = connections_file_path(directory) - if conn_file.is_file(): + if snapshot.connections is not None: try: - load_connections_file(conn_file, registry) + load_connections_data(snapshot.connections, conn_file, registry) except WiringError as exc: # Scrubbed like the arm below, and for the SAME reason: a WiringError out of the TOML # loader is a project-authored TEMPLATE wrapped around somebody else's exception text. diff --git a/messagefoundry/pipeline/_sandbox_codec.py b/messagefoundry/pipeline/_sandbox_codec.py index 831e0106a..48607c82c 100644 --- a/messagefoundry/pipeline/_sandbox_codec.py +++ b/messagefoundry/pipeline/_sandbox_codec.py @@ -55,12 +55,14 @@ import struct from collections.abc import Mapping, Sequence from dataclasses import dataclass +from pathlib import Path from typing import Any, Final from messagefoundry.config.code_sets import CodeSet, UnmappedKind, UnmappedPolicy from messagefoundry.config.models import ContentType from messagefoundry.config.response import CapturedResponse from messagefoundry.config.run_context import RunContext +from messagefoundry.config.snapshot import ConfigSnapshot from messagefoundry.config.wiring import ( Send, SetMeta, @@ -909,11 +911,9 @@ def enc_code_sets(code_sets: object, blobs: _Blobs) -> Any: Sent **once per spawn**, not per dispatch: re-sending them with every message was ~430 KB and ~4.6 ms of pure marshalling on a realistic crosswalk. Sending them at all — rather than letting the - child use its OWN ``load_config`` of the same directory — is what keeps ``mode=subprocess`` - byte-identical to ``mode=off``: after a transparent respawn (a wall-cap kill, a crash) the child - re-reads ``codesets/`` from disk, so an operator who edits a crosswalk without a ``/config/reload`` - would otherwise have the child serve a value the engine never would. The engine's tables are the - single source of truth on both sides. + child parse its own — is what keeps ``mode=subprocess`` byte-identical to ``mode=off``: the + engine's tables are the single source of truth on both sides, and with them published the boot + frame leaves the snapshot's ``codesets/`` files out (vault BACKLOG #2596). ``None`` means "the caller published no tables" and leaves the child on its own bootstrap load (the pre-existing behaviour for a session constructed without them); an empty mapping means "the engine @@ -985,21 +985,67 @@ def _open(body: bytes, allowed: tuple[str, ...]) -> tuple[dict[str, Any], _Reade return header, _Reader(blobs), kind +def enc_snapshot(snapshot: ConfigSnapshot, blobs: _Blobs, *, code_sets: bool = True) -> Any: + """Describe the config snapshot the engine loaded, for the worker's bootstrap frame. + + The worker runs these bytes and never reads the config directory (vault BACKLOG #2596): a + respawn, after a crash or a wall-cap kill, runs what the engine loaded rather than whatever is + on disk then. Each file's bytes travel as base64 in a segment, so source in any encoding crosses + unchanged. Only what a load reads is sent (:meth:`ConfigSnapshot.loaded`). Without + ``code_sets`` the ``codesets/`` tables are left out: the frame carries the engine's parsed + tables already, and both copies would halve the volume of tables :data:`MAX_FRAME` admits.""" + loaded = snapshot.loaded(code_sets=code_sets) + return { + "dir": blobs.ref(str(loaded.directory)), + "files": [ + [rel, blobs.ref(base64.b64encode(data).decode("ascii"))] + for rel, data in loaded.files.items() + ], + } + + +def dec_snapshot(node: Any, reader: _Reader) -> ConfigSnapshot: + """Rebuild the engine's config snapshot in the worker. Each path is checked against the closed + set of names a snapshot may hold (:class:`ConfigSnapshot`), and nothing is ever opened by it.""" + obj = _req_obj(node, "config snapshot") + directory = reader.text(obj.get("dir")) + files: dict[str, bytes] = {} + for pair in _req_list(obj.get("files"), "config snapshot files"): + if len(_req_list(pair, "config snapshot file")) != 2: + raise SandboxCodecError("a config snapshot file must be [path, base64]") + rel = _req_str(pair[0], "config snapshot path") + if rel in files: + raise SandboxCodecError(f"config snapshot path {rel!r} appears twice") + try: + files[rel] = base64.b64decode(reader.text(pair[1]), validate=True) + except ValueError as exc: # binascii.Error, or a str segment that is not ASCII + raise SandboxCodecError(f"config snapshot file {rel!r} is not base64: {exc}") from exc + try: + return ConfigSnapshot(directory=Path(directory), files=files) + except ValueError as exc: + raise SandboxCodecError(str(exc)) from exc + + def encode_boot( *, - config_dir: str, + snapshot: ConfigSnapshot, forbidden: Sequence[str], mem_mb: int | None, code_sets: object, ) -> bytes: """The one-per-spawn bootstrap frame. - ``env`` is deliberately NOT marshalled: ``load_config()`` takes only a directory and the worker + It carries the config snapshot the engine loaded, not a directory: the worker loads the graph + from those bytes and reads nothing from disk (vault BACKLOG #2596). + + ``env`` is deliberately NOT marshalled: the worker's load takes only the snapshot and the worker never read it — a dead field on the wire is a field nobody validates.""" blobs = _Blobs() header = _envelope( { - "config_dir": config_dir, + # The tables go once: as the engine's parsed ones when it published any, else as + # the snapshot's files, which the worker then parses. + "snapshot": enc_snapshot(snapshot, blobs, code_sets=code_sets is None), "forbidden": list(forbidden), "mem_mb": None if mem_mb is None else int(mem_mb), "code_sets": enc_code_sets(code_sets, blobs), @@ -1013,7 +1059,7 @@ def encode_boot( class Boot: """The decoded bootstrap frame (child side).""" - config_dir: str + snapshot: ConfigSnapshot forbidden: tuple[str, ...] mem_mb: int | None code_sets: dict[str, CodeSet] | None @@ -1021,7 +1067,8 @@ class Boot: def decode_boot(body: bytes) -> Boot: header, reader, _ = _open(body, ("boot",)) - config_dir = _req_str(header.get("config_dir"), "config_dir") + # Walk order matches encode_boot: the snapshot's segments BEFORE the code sets'. + snapshot = dec_snapshot(header.get("snapshot"), reader) forbidden = tuple( _req_str(m, "forbidden module") for m in _req_list(header.get("forbidden"), "forbidden") ) @@ -1029,7 +1076,7 @@ def decode_boot(body: bytes) -> Boot: code_sets = dec_code_sets(header.get("code_sets"), reader) reader.finish() return Boot( - config_dir=config_dir, + snapshot=snapshot, forbidden=forbidden, mem_mb=mem_mb, code_sets=code_sets, diff --git a/messagefoundry/pipeline/_sandbox_worker.py b/messagefoundry/pipeline/_sandbox_worker.py index 9f58dd259..54a16a2e2 100644 --- a/messagefoundry/pipeline/_sandbox_worker.py +++ b/messagefoundry/pipeline/_sandbox_worker.py @@ -9,12 +9,13 @@ decode path cannot name a type, import a module, or reach ``__reduce__``. Nothing is pickled in either direction, in either process. -1. **Bootstrap** — reads one ``boot`` frame ``{config_dir, forbidden, mem_mb, code_sets}``, loads - the message :class:`~messagefoundry.config.wiring.Registry` from ``config_dir`` (the same loader - the engine uses — it executes admin config under the unchanged safe-source gate), - **adopts the engine's code-set tables** from the frame in place of its own re-read of ``codesets/`` - (when the parent published any — a session constructed without them leaves the child on its own - load), applies the POSIX resource caps where available, installs the forbidden-import guard, and +1. **Bootstrap** — reads one ``boot`` frame ``{snapshot, forbidden, mem_mb, code_sets}``, loads + the message :class:`~messagefoundry.config.wiring.Registry` from the engine's config snapshot + (``load_snapshot``, the loader the engine uses, over the bytes the engine loaded and checked; the + worker never reads the config directory, vault BACKLOG #2596), + **adopts the engine's code-set tables** from the frame in place of the snapshot's ``codesets/``, + which the frame then leaves out (when the parent published any — a session constructed without + them sends the files and leaves the child to parse them), applies the POSIX resource caps where available, installs the forbidden-import guard, and replies ``ready`` (or ``bootfail`` on any failure). 2. **Serve** — for each subsequent ``req`` frame ``{id, phase, name, payload, rc}`` it rebuilds the payload with the engine's own constructor, looks the Router/Handler up in *its own* registry, @@ -347,9 +348,13 @@ def main() -> int: return 0 # parent closed the pipe before bootstrap — nothing to do try: boot = codec.decode_boot(frame) - from messagefoundry.config.wiring import load_config + from messagefoundry.config.wiring import load_snapshot - registry = load_config(boot.config_dir) + # The engine's own bytes, never the directory: a respawn runs what the engine loaded, not + # what is on disk now (vault BACKLOG #2596). The engine ran the trust check when it read + # them. The engine's parsed tables, when it sent any, stand in for codesets/, whose files + # the frame then leaves out. + registry = load_snapshot(boot.snapshot, code_sets=boot.code_sets) # Pre-import every module the serve loop touches BEFORE the guard goes up, so a first-time # (transitive) import of an engine helper can't be misread as a forbidden user import. Once # cached in sys.modules, a later `import` short-circuits ahead of the meta_path finder. diff --git a/messagefoundry/pipeline/alerts.py b/messagefoundry/pipeline/alerts.py index 908121b53..ab7c4f0bd 100644 --- a/messagefoundry/pipeline/alerts.py +++ b/messagefoundry/pipeline/alerts.py @@ -183,15 +183,17 @@ def sandbox_worker_refused_detail(*, content_only: bool) -> str: BACKLOG #2724). Fixed words that name the cause and the remedy: never a message body, and not the refusal itself, which the runner logs whole in a WARNING line. The alert instance's reason column keeps only about the first 200 characters, so the cause and the remedy lead.""" + # A worker runs the engine's config snapshot and never reads the directory (vault BACKLOG + # #2596), so an edit on disk is not a cause, and a reload does not cure a graph that differs + # by name. if content_only: cause = ( - "A config file changed on disk since the engine loaded it, with no reload. Reload the " - "config." + "The worker ran other config module bytes than the engine loaded. Reload the config." ) else: cause = ( - "Most often a config file changed on disk with no reload: reload the config. Or config " - "reads a variable the worker lacks: add it to [sandbox].pass_environment and restart." + "Config most likely reads a variable the worker lacks: add it to " + "[sandbox].pass_environment and restart." ) return ( f"{cause} Until then every message on this inbound fails at routing or transform: " diff --git a/messagefoundry/pipeline/engine.py b/messagefoundry/pipeline/engine.py index c4e9afc25..6d45c959e 100644 --- a/messagefoundry/pipeline/engine.py +++ b/messagefoundry/pipeline/engine.py @@ -21,6 +21,7 @@ from typing import Any, Literal, Protocol, get_args, runtime_checkable from messagefoundry.auth.trust_anchors import LaneAnchorCheck +from messagefoundry.config.environments import SnapshotEnvValuesProvider from messagefoundry.config.models import ( AckAfter, BuildupThreshold, @@ -46,6 +47,7 @@ StoreSettings, UpdateCheckSettings, ) +from messagefoundry.config.snapshot import ConfigSnapshot from messagefoundry.config.tls_policy import HopPosture, TrustAnchorPolicy from messagefoundry.config.wiring import ( API_LISTENER_LABEL, @@ -54,6 +56,7 @@ WiringError, connector_secret_env_values, load_config, + read_trusted_snapshot, ) from messagefoundry.controlchars import scrub_log_argument from messagefoundry.pipeline.alerts import ( @@ -128,6 +131,7 @@ "ClusteredConfigWrite", "ConfigDigestMismatch", "ConfigDigestUnavailable", + "ConfigRefusedBeforeApprovalCheck", "ConfigReloadDenied", "Engine", "ReloadOutcome", @@ -235,6 +239,19 @@ def __init__(self, approved: str | None) -> None: self.approved = approved +class ConfigRefusedBeforeApprovalCheck(WiringError): + """A released reload refused as a ``WiringError`` before its snapshot's digest was checked + against its approval (vault BACKLOG #2596): at least by the settings preflight, or by the + config source's trust check, which stats paths and reads no contents. It keeps the refusal's + message, cause and traceback, so it is audited with the reason that refusal gets. ``approved`` + is the digest the hold captured, or ``None``, so the audit row can name the approval the + refusal stopped.""" + + def __init__(self, refusal: WiringError, approved: str | None) -> None: + super().__init__(str(refusal)) + self.approved = approved + + def _log_abandoned_config_write(task: asyncio.Task[dict[str, object] | None]) -> None: """Log how a config write ended after its caller was cancelled, since nobody else reads it. @@ -2976,18 +2993,25 @@ def running_config_dir(self) -> Path | None: A DR activation does not reload it: it re-applies the running graph.""" return self.last_reload_dir or self.config_dir - async def fingerprint_bundle(self, path: Path) -> tuple[dict[str, object] | None, str | None]: + async def fingerprint_bundle( + self, path: Path, snapshot: ConfigSnapshot | None = None + ) -> tuple[dict[str, object] | None, str | None]: """The ADR 0041 D1 content fingerprint of ``path``, or ``(None, reason)`` when it cannot be read. + With ``snapshot``, the digest is of that snapshot, the bytes a load read and ran, and + ``path`` is read only for the advisory VCS head (vault BACKLOG #2596). + The one best-effort rule for every load: the start, an applied reload and a dry run all call it, so a bundle one of them tolerates cannot make another refuse (vault BACKLOG #2597). OSError is an unreadable file; ValueError is a VCS head that is not UTF-8 (UnicodeDecodeError). Anything else, such as an ImportError of the fingerprint module, still raises. The reason is a :func:`safe_exc` rendering, logged here at WARNING.""" + if snapshot is not None: + return await asyncio.to_thread(self.fingerprint_bundle_blocking, path, snapshot) return await asyncio.to_thread(self.fingerprint_bundle_blocking, path) def fingerprint_bundle_blocking( - self, path: Path + self, path: Path, snapshot: ConfigSnapshot | None = None ) -> tuple[dict[str, object] | None, str | None]: """:meth:`fingerprint_bundle` for a caller already on a worker thread, under the same rule. @@ -2998,6 +3022,8 @@ def fingerprint_bundle_blocking( try: # A direct call, so the crypto inventory scanner, which follows a call it can see, # keeps the hash on its record. + if snapshot is not None: + return config_fingerprint_detail(path, snapshot=snapshot), None return config_fingerprint_detail(path), None except (OSError, ValueError) as exc: reason = safe_exc(exc) @@ -3056,6 +3082,35 @@ def _content_digest_blocking(path: Path) -> str | None: ) return None + async def _reload_snapshot(self, path: Path) -> ConfigSnapshot | None: + """The one read of ``path`` a reload runs, after the load's trust check, read strictly so + its digest covers every file the fingerprint names (vault BACKLOG #2596). ``None`` when a + file or folder cannot be read, logged at WARNING as :meth:`_content_digest` logs it. A + missing directory raises :class:`ConfigDirMissing`, and a refused source ``WiringError``.""" + return await asyncio.to_thread(self._reload_snapshot_blocking, path) + + @staticmethod + def _reload_snapshot_blocking(path: Path) -> ConfigSnapshot | None: + from messagefoundry.config.snapshot import ConfigDirMissing + + try: + return read_trusted_snapshot(path, strict=True) + except ConfigDirMissing: + raise + except OSError as exc: + # The path can come from a client's config_dir, so it is scrubbed for CodeQL + # py/log-injection as the reload refusal logs scrub theirs. + log.warning( + "config content digest failed for %s: %s. The reload's snapshot could not be " + "read whole, so a released reload is refused. Any other reload reads the " + "directory again and refuses any file the load reads that it cannot read, every " + "*.py module and helper included; only the environments folder and its files " + "are left out", + scrub_log_argument(str(path)), + safe_exc(exc), + ) + return None + async def _existing_reload_target(self, config_dir: str | Path | None) -> Path: """The directory a reload of ``config_dir`` loads: resolved and confined by :meth:`_resolve_reload_target`, and refused with :class:`ConfigDirMissing` (a @@ -3210,19 +3265,24 @@ async def reload_detail( single-node (``is_clustered()`` False) never bumps. The initiator advances its OWN applied version right after bumping, so its convergence loop sees no change and does not re-reload. + **The directory is read once** (vault BACKLOG #2596, step 1): :meth:`_reload_snapshot` + reads it, after the load's trust check, into a snapshot; ``load_config`` compiles and + parses those bytes and nothing else, so no ``__pycache__`` file runs; and the audit digest + is the digest of the same snapshot. An edit on disk after that read changes nothing this + reload runs or records. + ``approval`` binds the reload to the digest a dual-control approval named (vault BACKLOG - #2596, step 3; :meth:`reload_target_digest` took it at request time). The bundle's digest is - checked against it before ``load_config`` runs any config module. After a load that - succeeds it is checked again, before any check that can refuse the graph as invalid: the - fail-closed digest, then the digest the audit row records when one was taken. After a load - that fails, the fail-closed digest is checked once more, so an edit that breaks the graph - is refused as an edit. A miss raises :class:`ConfigDigestMismatch`, or - :class:`ConfigDigestUnavailable` when no digest can be taken now, with nothing swapped. The - digest fails closed (:meth:`_content_digest`): a file or folder it cannot read gives - :class:`ConfigDigestUnavailable`, and a directory gone since the resolve, when a digest - lists it, gives :class:`ConfigDirMissing`. ``None`` binds nothing: an ungated reload, a - convergence reload and a dry run pass none. The digest covers what :mod:`messagefoundry.config.fingerprint` hashes - and no more. + #2596, step 3; :meth:`reload_target_digest` took it at request time). The snapshot's digest + is checked against it before ``load_config`` runs any config module, and the load then runs + that snapshot, so the bytes checked are the bytes that run. A miss raises + :class:`ConfigDigestMismatch`, or :class:`ConfigDigestUnavailable` when a file or folder + cannot be read, with nothing swapped; a directory gone since the resolve gives + :class:`ConfigDirMissing`. ``None`` binds nothing: an ungated reload, a convergence reload + and a dry run pass none. The digest covers what :mod:`messagefoundry.config.fingerprint` + hashes and no more. A :class:`~messagefoundry.config.environments.SnapshotEnvValuesProvider` + reads a value file in the snapshot's ``environments/`` folder from the snapshot too; after + any other read of the values from disk, a released reload takes the digest again and + refuses a change the same way. Raises ``ConfigReloadDenied`` (path outside the allowed roots), ``ConfigDirMissing`` (missing dir; a ``FileNotFoundError`` subclass, which the callers map to not_found), @@ -3244,24 +3304,46 @@ async def reload_detail( """ failures: list[ReloadStepFailure] = [] restart_required: tuple[str, ...] = () - if not dry_run: - # First, so every reload route refuses a bad settings anchor at the point the reload - # route used to, before anything is read or swapped (BACKLOG #2034). - restart_required = await self.preflight_settings() - path = await self._existing_reload_target(config_dir) + try: + if not dry_run: + # First, so every reload route refuses a bad settings anchor at the point the + # reload route used to, before anything is read or swapped (BACKLOG #2034). + restart_required = await self.preflight_settings() + path = await self._existing_reload_target(config_dir) + # The one read of the directory this reload runs (vault BACKLOG #2596): load_config + # compiles and parses these bytes and nothing else, the audit digest is theirs, and an + # approval is checked against them before any of them runs. None when a file cannot be + # read, which is logged; an ungated reload then lets load_config read and refuse it. + snapshot = await self._reload_snapshot(path) + except WiringError as exc: + if approval is None: + raise + # A released reload refused before its digest check: marked, so its audit row can say + # the check was not reached rather than read as a refusal of the approved bytes. The + # cause is kept, so a trust-anchor or store refusal is audited as it was. + raise ConfigRefusedBeforeApprovalCheck(exc, approval.fingerprint).with_traceback( + exc.__traceback__ + ) from exc.__cause__ if approval is not None: # Before load_config, which runs every config module's top-level code, so bytes - # changed since the request are refused before they run (vault BACKLOG #2596). This - # samples the files: an edit landing between here and the load still runs its module - # code, and the check after the load then refuses it. - _require_approved(await self._content_digest(path), approval) + # changed since the request are refused before they run. An edit after this read + # changes nothing: the load runs this snapshot. + from messagefoundry.config.fingerprint import snapshot_fingerprint + + digest = ( + await asyncio.to_thread(snapshot_fingerprint, snapshot) + if snapshot is not None + else None + ) + _require_approved(digest, approval) # The values this reload resolves env() against. A real reload applies them only at the # swap, so a refusal leaves the running graph's values as they were (vault BACKLOG #2596). env_values = self._env_values # Re-gather this environment's values so a reload/promote picks up edited environments/.toml # (or MEFOR_VALUE_* changes) without a restart — otherwise the WiringError telling the operator # to add a missing value would never clear (review M-23). - if self._env_values_provider is not None: + provider = self._env_values_provider + if provider is not None: # Guard the provider. On the CLI path it is tomllib.load over environments/.toml, and # for an embedder it is arbitrary caller code, so an unreadable value file raised straight # out of the reload: TOMLDecodeError is a ValueError, not a WiringError, so POST @@ -3273,11 +3355,22 @@ async def reload_detail( # dual-control release executor, the cluster convergence loop). A DR activation does not # reach it: it re-applies the running graph (vault BACKLOG #3067). refusal: str | None = None + # Vault BACKLOG #2596: the approval digest covers environments/*.toml under the config + # directory, so a SnapshotEnvValuesProvider is handed the snapshot and parses the bytes + # the digest names. Where it cannot, and for any other provider, the values are read + # from disk after the digest check, and a released reload checks the digest again. + read: Mapping[str, Any] | None = None + reads_snapshot = False try: # Off the event loop: the CLI provider reads the value file and, before that, its # owner and access list (vault BACKLOG #2626), like every other file the engine reads. # Into a local: the running values change only at the swap (vault BACKLOG #2596). - env_values = dict(await asyncio.to_thread(self._env_values_provider)) + if snapshot is not None and isinstance(provider, SnapshotEnvValuesProvider): + read = await asyncio.to_thread(provider.from_snapshot, snapshot) + reads_snapshot = read is not None + if read is None: # no snapshot, or one that cannot vouch for the value file + read = await asyncio.to_thread(provider) + env_values = dict(read) except WiringError: # Already a wiring failure that names its own source. Re-raise unwrapped rather than # nesting the same sentence twice. @@ -3305,31 +3398,18 @@ async def reload_detail( "could not re-read this environment's values for the reload, so the live graph " f"is unchanged: {refusal}" ) - load_error: WiringError | None = None - try: - # Off the event loop: load_config executes user config modules (arbitrary, potentially - # heavy imports), which would otherwise stall every listener mid-reload (review low-3). - registry = await asyncio.to_thread(load_config, path) # WiringError on a bad config - except WiringError as exc: - if approval is None: - raise - load_error = exc - if load_error is not None and approval is not None: - # An edit after the first check can also break the load. Name the edit, so a tamper is - # audited as one and not as a bad config (vault BACKLOG #2596). Checked AFTER the - # handler, so a digest refusal does not carry the load error on __context__, where a - # TOMLDecodeError would keep a whole value file (BACKLOG #2307). - try: + if approval is not None and not reads_snapshot: + # A provider not handed the snapshot may have read a value file the digest covers + # from disk after the check, so the release checks the directory again now, and + # refuses an edit made in between rather than apply values nobody approved. _require_approved(await self._content_digest(path), approval) - except (ConfigDigestMismatch, ConfigDigestUnavailable, FileNotFoundError): - # The refusal names the digest or the missing directory, so the load error would - # otherwise go unlogged. - log.warning( - "a released config reload also failed to load: %s", - scrub_log_argument(safe_exc(load_error)), - ) - raise - raise load_error + # Off the event loop: load_config executes user config modules (arbitrary, potentially + # heavy imports), which would otherwise stall every listener mid-reload (review low-3). + # WiringError on a bad config. With a snapshot, a released reload's load failure is a + # failure of the approved bytes themselves, so it is a bad config and not an edit. + # With no snapshot (one could not be read strictly), load_config reads its own and refuses + # a file it needs but cannot read. + registry = await asyncio.to_thread(load_config, path, snapshot=snapshot) fingerprint: dict[str, object] | None = None if not dry_run: # ADR 0041 D1 (config provenance, item C): fingerprint the bundle right after the load @@ -3341,19 +3421,14 @@ async def reload_detail( # the swap. Still best-effort: an unreadable bundle leaves provenance unknown rather # than refusing an otherwise-applicable config, and is recorded as a step failure so the # caller is not told the reload was clean when GET /config/provenance reports nothing. - fingerprint, fingerprint_failure = await self.fingerprint_bundle(path) + # The digest of the snapshot the load ran (vault BACKLOG #2596), so a released + # reload's row names the approved digest by construction, and the row names the bytes + # that ran rather than a later read of the directory. + fingerprint, fingerprint_failure = await self.fingerprint_bundle( + path, registry.config_snapshot + ) if fingerprint_failure is not None: failures.append(ReloadStepFailure("config_fingerprint", fingerprint_failure)) - if approval is not None: - # Again, on the digest the audit row records, so a released reload's row names the - # approved digest. Before every check that can refuse the graph as invalid, so a tamper - # is audited as one and those checks read only approved bytes. An edit that lands and - # is reverted inside the load is not seen. The fail-closed digest first: the recorded - # one skips a file it cannot read, so on its own it could match with a file unbound. - _require_approved(await self._content_digest(path), approval) - taken = fingerprint.get("fingerprint") if fingerprint is not None else None - if isinstance(taken, str): - _require_approved(taken, approval) # Before the shard filter, so a guard judges the WHOLE graph: an engine-shard process then # reaches the same verdict as its siblings, over one shared config (BACKLOG #1182). self.guard_registry(registry) diff --git a/messagefoundry/pipeline/sandbox.py b/messagefoundry/pipeline/sandbox.py index 56a20bb11..aba52222a 100644 --- a/messagefoundry/pipeline/sandbox.py +++ b/messagefoundry/pipeline/sandbox.py @@ -107,6 +107,7 @@ INSECURE_CONFIG_SOURCE_ESCAPE_ENV, security_enforcement_env_names, ) +from messagefoundry.config.snapshot import ConfigSnapshot from messagefoundry.controlchars import scrub_control_chars from messagefoundry.pipeline import _sandbox_codec as codec from messagefoundry.pipeline._sandbox_codec import GraphShape, SandboxCodecError, SandboxError @@ -229,8 +230,8 @@ def graph_differences(engine: GraphShape, worker: GraphShape) -> list[str]: ``bindings`` is compared over the inbounds the ENGINE holds. An engine shard holds only its own, while its worker loads the whole config, so the worker's other bindings are not a difference. ``code_digest`` is compared whenever the engine has one: a config module whose - bytes changed on disk since the engine loaded it differs there even when every name is the - same (vault BACKLOG #2724). A worker of the same build with no digest could not read its own + bytes differ from the ones the engine loaded differs there even when every name is the same + (vault BACKLOG #2724; since #2596 a worker runs the engine's snapshot, so this is a backstop). A worker of the same build with no digest could not read its own modules, so that is a difference too, never a pass.""" found: list[str] = [] sets = { @@ -560,7 +561,8 @@ class SandboxWorkerRefused(SandboxError): engine graph is refused from the session's cached refusal with ``first`` False, so the runner raises its ``sandbox_worker_refused`` alert once per refusal, not once per message (vault BACKLOG #2724, owner ruling 2026-10-16). ``content_only`` is True when the config modules' - bytes are the only difference, so the cause is an edit on disk with no reload. ``graph`` is + bytes are the only difference: the worker ran other bytes than the engine loaded, which a + session sent the engine's snapshot cannot do (vault BACKLOG #2596). ``graph`` is the engine graph the worker was compared with, so the runner can tell a refusal against a graph it no longer serves. The defaults exist only so that a copy or an unpickle, which passes the message alone, still builds.""" @@ -597,8 +599,16 @@ def __init__( env: str | None, graph: Callable[[], GraphShape] | None, code_sets: Mapping[str, CodeSet] | None = None, + snapshot: ConfigSnapshot | None = None, ) -> None: self.policy = policy + # The config bytes every worker of this session runs: the snapshot the engine's graph was + # loaded from (vault BACKLOG #2596). Sent in the boot frame, so a worker, a respawned one + # included, never reads the config directory and runs what the engine loaded, whatever is + # on disk by then. ``None`` (a caller holding a graph built in code): the session reads + # ``config_dir`` itself, once, with the load's trust check, at its first spawn, and every + # later respawn reuses those bytes. + self._snapshot = snapshot # How to read the graph the engine is serving NOW, which a worker's must match before it is # brought up. A callable, read at each spawn: a session can outlive a registry swap, and a # shape captured when the session was made would then be the wrong one to compare with. @@ -616,9 +626,8 @@ def __init__( self._inbound = inbound self._config_dir = str(Path(config_dir)) # Kept for the caller's signature (``engine.py`` resolves it into the config source and - # ``wiring_runner._sandbox_for`` passes it here), but NOT marshalled: the - # worker never read it — `load_config()` takes only a directory — and a dead field on the wire - # is a field nobody validates. + # ``wiring_runner._sandbox_for`` passes it here), but NOT marshalled: the worker loads only + # the snapshot it is sent, and a dead field on the wire is a field nobody validates. self._env = env # The ENGINE's live code-set tables, sent once per spawn in the boot frame so the child serves # exactly what mode=off would rather than its own re-read of `codesets/` (see @@ -659,6 +668,18 @@ def _spawn(self) -> None: raise SandboxWorkerRefused( self._refused[1], first=False, content_only=self._refused[2], graph=engine_graph ) + # The boot frame is built BEFORE the child exists, so a snapshot that cannot be read (a + # session given none reads its directory here) or a frame that cannot be encoded leaves no + # live child, job handle or drain thread behind. + try: + boot = codec.encode_boot( + snapshot=self._config_snapshot(), + forbidden=self.policy.forbidden_modules, + mem_mb=self.policy.mem_mb, + code_sets=self._code_sets, + ) + except (OSError, ValueError, SandboxError) as exc: # WiringError is a ValueError + raise SandboxError(f"failed to bootstrap sandbox worker: {exc}") from exc # A fresh response queue per spawn so a prior (killed) worker's trailing EOF can't leak into # this generation's reads. self._responses = queue.Queue() @@ -666,10 +687,11 @@ def _spawn(self) -> None: # NAMED, not inherited: an allowlist, so the engine's secrets are not in the child's # environment (vault BACKLOG #2587). Two engine variables cross, and they are one # decision: the config-source escape and the enforcement dial that unlocks it (vault - # BACKLOG #2599). The child runs ``load_config`` itself, so with only the escape it - # would refuse a directory its parent loaded at ``warn``. The dial is a posture - # switch, not a secret, and every spelling of it the parent's check read crosses, so - # the child's check reads the same variables. The operator's + # BACKLOG #2599). They crossed because the child ran ``load_config`` on the directory + # itself, and with only the escape it would refuse a directory its parent loaded at + # ``warn``. Since vault BACKLOG #2596 the child loads the snapshot it is sent and runs no + # trust check of its own, so no code in it reads either; dropping them is a separate + # change. The dial is a posture switch, not a secret. The operator's # ``[sandbox].pass_environment`` names cross with them. # At most one glibc malloc arena besides the main one, where the worker's memory cap is # RLIMIT_DATA. An arena keeps its peak writable size counted in VmData after its threads @@ -743,15 +765,7 @@ def _spawn(self) -> None: # not on what the parent's threads are doing. self._job = _assign_kill_on_close_job(proc) try: - _write_frame( - proc.stdin, - codec.encode_boot( - config_dir=self._config_dir, - forbidden=self.policy.forbidden_modules, - mem_mb=self.policy.mem_mb, - code_sets=self._code_sets, - ), - ) + _write_frame(proc.stdin, boot) except (OSError, SandboxError) as exc: self._kill(proc) raise SandboxError(f"failed to bootstrap sandbox worker: {exc}") from exc @@ -779,20 +793,22 @@ def _spawn(self) -> None: # FAIL CLOSED. A worker that loaded a different graph would answer for this inbound # from that graph, and nothing in its answers would say so. self._kill(proc) - # Only an edit on disk when the worker read its modules: a worker that could not - # read them reports no digest, and that is not an edit. + # Module bytes alone differ only when the worker read its modules: a worker that + # could not read them reports no digest, and that is not the same bytes. content_only = worker_graph.code_digest is not None and all( d.startswith(_CONTENT_DIFFERS) for d in differing ) + # A worker runs the snapshot it is sent and never reads the directory (vault + # BACKLOG #2596), so an edit on disk is not a cause any more, and a reload is no + # remedy for a graph that differs by name. causes = ( - "The config files changed on disk since the engine loaded them: reload the " + "The worker ran other config module bytes than the engine loaded: reload the " "config." if content_only - else "Most often the config files changed on disk since the engine loaded " - "them: reload the config. Or config code reads an environment variable the " - "worker is not given, because the worker loads the config again under an " - "allowlisted environment: name each such variable in " - "[sandbox].pass_environment and restart." + else "Config code most likely reads an environment variable the worker is " + "not given, because the worker runs the config again under an allowlisted " + "environment: name each such variable in [sandbox].pass_environment and " + "restart." ) # Short before the differences: a dead-lettered message keeps about the first 200 # characters of this, behind its stage prefix and the exception's type name. @@ -807,6 +823,16 @@ def _spawn(self) -> None: self._refused = None self._proc = proc + def _config_snapshot(self) -> ConfigSnapshot: + """The snapshot this session's workers run: the engine's, or, when the session was given + none, one read here from ``config_dir`` once, under the load's own trust check, and kept, so + a respawn reuses the same bytes. Called from the worker thread, never the event loop.""" + if self._snapshot is None: + from messagefoundry.config.wiring import read_trusted_snapshot + + self._snapshot = read_trusted_snapshot(self._config_dir) + return self._snapshot + def _reader_loop(self, stdout: Any, sink: queue.Queue[Any]) -> None: """Drain the child's stdout into ``sink`` — **framing only**, never decoding (see :func:`_read_frame_bytes`). Runs on a daemon thread; a fresh queue per spawn means a stale diff --git a/messagefoundry/pipeline/sharding.py b/messagefoundry/pipeline/sharding.py index b25fd4806..848507ac5 100644 --- a/messagefoundry/pipeline/sharding.py +++ b/messagefoundry/pipeline/sharding.py @@ -213,6 +213,8 @@ def filter_registry_for_shard(registry: Registry, shard: str) -> Registry: source_dir=registry.source_dir, # And compares the digest of what it loaded with the engine's (vault BACKLOG #2724). code_digest=registry.code_digest, + # And is sent the bytes the graph was loaded from (vault BACKLOG #2596). + config_snapshot=registry.config_snapshot, shard_id=shard if sharded else None, all_shard_ids=tuple(ids) if sharded else None, # Pin the UNFILTERED loopback inbounds beside the shard identity: `selected` drops the inbounds diff --git a/messagefoundry/pipeline/wiring_runner.py b/messagefoundry/pipeline/wiring_runner.py index 839778e98..2ca80005e 100644 --- a/messagefoundry/pipeline/wiring_runner.py +++ b/messagefoundry/pipeline/wiring_runner.py @@ -5490,9 +5490,11 @@ def _sandbox_for(self, name: str) -> SandboxSession | None: Returns ``None`` — the byte-identical in-process path — unless ``[sandbox].mode=subprocess`` AND a config dir is known. The child re-loads the graph to look a function up by name, from - ``Registry.source_dir`` (the directory the served graph came from), else the constructor's - ``sandbox_config_source``. A reload of a running runner closes the sessions, so a reload - from another root moves the children with it (vault BACKLOG #3094). **A graph built in code with no + ``Registry.config_snapshot``, the bytes the served graph was loaded from, and never reads + the directory (vault BACKLOG #2596). A graph with no snapshot hands the session + ``Registry.source_dir``, else the constructor's ``sandbox_config_source``, which the session + reads once. A reload of a running runner closes the sessions, so a reload from another root + moves the children with it (vault BACKLOG #3094). **A graph built in code with no config dir cannot isolate**, and degrades to in-process. That degradation is **silent**: a library embedder that asked for ``subprocess`` gets the in-process path and nothing says so. Every ``serve`` route carries a config dir. The :class:`SandboxSession` object is created @@ -5519,6 +5521,9 @@ def _sandbox_for(self, name: str) -> SandboxSession | None: env=env, graph=self._engine_graph_shape, code_sets=self.registry.code_sets, + # The bytes the served graph was loaded from: every worker of this session runs + # them and never reads the directory (vault BACKLOG #2596). + snapshot=self.registry.config_snapshot, ) self._sandbox_sessions[name] = session return session diff --git a/tests/test_config_fingerprint.py b/tests/test_config_fingerprint.py index 2443a3a90..7c4980195 100644 --- a/tests/test_config_fingerprint.py +++ b/tests/test_config_fingerprint.py @@ -132,7 +132,7 @@ def test_detail_has_fingerprint_and_file_count(tmp_path: Path) -> None: assert detail["fingerprint"] == config_fingerprint(d) assert detail["files"] == 5 # vault BACKLOG #2597: the scheme tag decides whether two digests are comparable at all. - assert detail["scheme"] == "mefor-cfg-fp:v2" + assert detail["scheme"] == "mefor-cfg-fp:v3" # tmp_path is outside any git work tree, so provenance is omitted (not None, absent). assert "git_head" not in detail diff --git a/tests/test_config_reload_outcome.py b/tests/test_config_reload_outcome.py index 140907956..2f2dcf8b3 100644 --- a/tests/test_config_reload_outcome.py +++ b/tests/test_config_reload_outcome.py @@ -243,7 +243,7 @@ async def test_unreadable_bundle_applies_and_names_the_fingerprint_step( await eng.start() try: - def _unreadable(directory: Any) -> dict[str, object]: + def _unreadable(directory: Any, **_kw: Any) -> dict[str, object]: raise OSError("config dir vanished mid-reload") monkeypatch.setattr( @@ -293,7 +293,7 @@ async def test_fingerprint_is_taken_before_the_swap( assert runner is not None original_reload = runner.reload - def _recording_fingerprint(directory: Any) -> dict[str, object]: + def _recording_fingerprint(directory: Any, **_kw: Any) -> dict[str, object]: order.append("fingerprint") return {"fingerprint": "deadbeef", "files": 1} diff --git a/tests/test_config_snapshot.py b/tests/test_config_snapshot.py new file mode 100644 index 000000000..2108c67b5 --- /dev/null +++ b/tests/test_config_snapshot.py @@ -0,0 +1,579 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Foundation, LLC and contributors +"""The bytes a config load hashes are the bytes it compiles and runs (vault BACKLOG #2596). + +Two probes from the 2026-10-01 architecture review, as tests that fail on the code before the +change: + +* A cache file planted under ``__pycache__``, whose header matches the source's size and time, + ran in place of the source while the fingerprint stayed the same. The loader now compiles each + module from the one snapshot it hashed, so the cache is never consulted. +* A sandbox worker respawn loaded whatever was on disk then, with no reload. The worker is now sent + the engine's snapshot and never reads the directory, so a respawn runs what the engine loaded. + +Plus the two points the row folds in: a code-set table spelled ``x.CSV`` is hashed as the loader +reads it, on POSIX too, and a module linked in beside another changes the digest. + +Synthetic HL7 and marker strings only.""" + +from __future__ import annotations + +import importlib +import importlib.machinery +import importlib.util +import marshal +import os +import runpy +import subprocess +import sys +import tokenize +from pathlib import Path, PurePath +from types import CodeType + +import pytest + +from messagefoundry.config import wiring +from messagefoundry.config.fingerprint import config_fingerprint +from messagefoundry.config.run_context import RunContext +from messagefoundry.config.snapshot import ( + ConfigDirMissing, + ConfigSnapshot, + NotInSnapshot, + config_py_files, + read_config_snapshot, +) +from messagefoundry.config.wiring import WiringError, load_config, load_snapshot, validate_config +from messagefoundry.pipeline import _sandbox_codec as codec +from messagefoundry.pipeline.dryrun import transform_one +from messagefoundry.pipeline.sandbox import ( + SandboxError, + SandboxMode, + SandboxPolicy, + SandboxSession, + SandboxWorkerRefused, + graph_shape, +) + +RAW = "MSH|^~\\&|SEND|F|RECV|F|20240101120000||ADT^A01|MSG00001|P|2.3\rPID|1||900001||DOE^JANE\r" + +_GRAPH = """ +import os +from messagefoundry import inbound, outbound, router, handler, MLLP, Send + +inbound("IB_SNAP", MLLP(port=19471), router="r") +outbound("OB_SNAP", MLLP(host="127.0.0.1", port=19472)) + + +@router("r") +def r(msg): + return "h_ver" + + +@handler("h_ver") +def h_ver(msg): + return Send("OB_SNAP", "__VERSION__") + + +@handler("h_die") +def h_die(msg): + os._exit(3) +""" + + +def _graph(version: str) -> str: + return _GRAPH.replace("__VERSION__", version) + + +def _ran(registry: object, name: str = "h_ver", **kwargs: object) -> str: + deliveries, _, _, _ = transform_one(registry, name, RAW, **kwargs) # type: ignore[arg-type] + return str(deliveries[0].payload) + + +def _strings(code: CodeType | None) -> set[str]: + """Every string constant in ``code`` and the code objects nested in it.""" + found: set[str] = set() + if code is None: + return found + for const in code.co_consts: + if isinstance(const, str): + found.add(const) + elif isinstance(const, CodeType): + found |= _strings(const) + return found + + +def _plant_cache(graph: Path, source: str) -> Path: + """A timestamp-validated cache file for ``graph``, compiled from ``source``, whose header + carries ``graph``'s own size and time: Python's source loader takes it as current.""" + st = graph.stat() + header = ( + importlib.util.MAGIC_NUMBER + + (0).to_bytes(4, "little") + + (int(st.st_mtime) & 0xFFFFFFFF).to_bytes(4, "little") + + (st.st_size & 0xFFFFFFFF).to_bytes(4, "little") + ) + cache = Path(importlib.util.cache_from_source(str(graph))) + cache.parent.mkdir(exist_ok=True) + cache.write_bytes(header + marshal.dumps(compile(source, str(graph), "exec"))) + return cache + + +# --- probe 1: a planted cache file ----------------------------------------------------------------- + + +def test_a_planted_cache_file_does_not_run_and_the_digest_names_what_does(tmp_path: Path) -> None: + graph = tmp_path / "graph.py" + graph.write_text(_graph("V1"), encoding="utf-8") + before = config_fingerprint(tmp_path) + cache = _plant_cache(graph, _graph("V2")) + # Control: the planted file is one Python's ordinary source loader runs in place of graph.py, + # which is the loader this module used before (vault BACKLOG #2596). + spec = importlib.util.spec_from_file_location("mefor_probe_control", graph) + assert spec is not None and isinstance(spec.loader, importlib.machinery.SourceFileLoader) + control = _strings(spec.loader.get_code("mefor_probe_control")) + assert "V2" in control and "V1" not in control + registry = load_config(tmp_path) + assert _ran(registry) == "V1" + assert config_fingerprint(tmp_path) == before # the cache is not part of what loads + assert cache.exists(), "control: the planted file is still there" + + +def test_the_loader_never_reads_the_cache_folder( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + graph = tmp_path / "graph.py" + graph.write_text(_graph("V1"), encoding="utf-8") + _plant_cache(graph, _graph("V2")) + opened: list[str] = [] + real_open = open + + def recording_open(file: object, *args: object, **kwargs: object) -> object: + opened.append(os.fspath(file) if isinstance(file, (str, os.PathLike)) else repr(file)) + return real_open(file, *args, **kwargs) # type: ignore[call-overload] + + monkeypatch.setattr("builtins.open", recording_open) + registry = load_config(tmp_path) + monkeypatch.undo() + assert _ran(registry) == "V1" + assert not [name for name in opened if "__pycache__" in name] + + +def test_a_helper_runs_the_snapshot_bytes_too(tmp_path: Path) -> None: + """A config module and a helper are each executed by an importlib loader over the snapshot, + not by the source loader that finds cache files.""" + (tmp_path / "_shared.py").write_text('VERSION = "V1"\n', encoding="utf-8") + graph = tmp_path / "graph.py" + graph.write_text( + "import _shared\n" + _graph("V0").replace('"V0"', "_shared.VERSION"), encoding="utf-8" + ) + _plant_cache(tmp_path / "_shared.py", 'VERSION = "V2"\n') + registry = load_config(tmp_path) + assert _ran(registry) == "V1" + module_globals = registry.handlers["h_ver"].__globals__ + helper = module_globals["_shared"] + for spec, path in ( + (module_globals["__spec__"], graph), + (helper.__spec__, tmp_path / "_shared.py"), + ): + assert isinstance(spec.loader, wiring._SnapshotLoader) + assert spec.origin == str(path) + assert module_globals["__file__"] == str(graph) + assert module_globals["__cached__"] is None and helper.__cached__ is None + + +def test_a_config_module_cannot_be_reloaded_from_disk(tmp_path: Path) -> None: + """Nothing re-runs a loaded config module: ``importlib.reload`` finds no spec for the mangled + name, so it never reads the directory or its cache, and ``runpy`` gets no second code object.""" + graph = tmp_path / "graph.py" + graph.write_text(_graph("V1"), encoding="utf-8") + registry = load_config(tmp_path) + module = sys.modules[registry.handlers["h_ver"].__module__] + spec = module.__spec__ + graph.write_text(_graph("V2"), encoding="utf-8") + try: + with pytest.raises(ModuleNotFoundError): + importlib.reload(module) + module.__spec__ = spec + # Nor does runpy: the loader served its one code object to the load. + with pytest.raises(ImportError, match="No code object"): + runpy.run_module(module.__name__) + # A reload that ran would have re-bound the module's names to new functions. + assert module.__dict__["h_ver"] is registry.handlers["h_ver"] + finally: + module.__spec__ = spec # reload cleared it before it raised + + +def test_a_compile_failure_is_still_judged_as_raised_in_the_loader(tmp_path: Path) -> None: + """The loader compiles each module in this package, so a parser wall (a MemoryError or + RecursionError from compile, not from author code running) keeps the engine's wording.""" + (tmp_path / "graph.py").write_text("X = " + "-" * 200_000 + "1\n", encoding="utf-8") + with pytest.raises(WiringError) as info: + load_config(tmp_path) + assert wiring._WITHHELD not in str(info.value) + assert isinstance(info.value.__cause__, MemoryError | RecursionError) # compile's own wall + + +# --- probe 2: a sandbox worker respawn ------------------------------------------------------------- + + +def _session(registry: object, config_dir: Path, *, snapshot: bool = True) -> SandboxSession: + return SandboxSession( + SandboxPolicy(mode=SandboxMode.SUBPROCESS, wall_seconds=30.0), + inbound="IB_SNAP", + config_dir=config_dir, + env=None, + graph=lambda: graph_shape(registry), + snapshot=registry.config_snapshot if snapshot else None, # type: ignore[attr-defined] + ) + + +def test_a_respawned_worker_runs_the_engines_bytes_not_the_disk(tmp_path: Path) -> None: + graph = tmp_path / "graph.py" + graph.write_text(_graph("V1"), encoding="utf-8") + registry = load_config(tmp_path) # the engine's graph, never reloaded below + session = _session(registry, tmp_path) + try: + assert _ran(registry, sandbox=session, run_context=RunContext()) == "V1" + first = session._proc.pid if session._proc else None + graph.write_text(_graph("V2"), encoding="utf-8") + with pytest.raises(SandboxError): + _ran(registry, "h_die", sandbox=session, run_context=RunContext()) + # The next dispatch respawns the worker, with no reload asked for. Before, it loaded the + # edited file from disk; then, with the digest comparison, it was refused. + assert _ran(registry, sandbox=session, run_context=RunContext()) == "V1" + assert session._proc is not None and session._proc.pid != first, "control: it respawned" + finally: + session.close() + # Control: a session handed no snapshot reads the directory, and the edit is there to read, so + # its worker is refused for loading other bytes than the engine's. + blind = _session(registry, tmp_path, snapshot=False) + try: + with pytest.raises(SandboxWorkerRefused, match="config module content"): + _ran(registry, sandbox=blind, run_context=RunContext()) + finally: + blind.close() + + +def test_the_boot_frame_carries_the_snapshot_and_no_directory(tmp_path: Path) -> None: + (tmp_path / "graph.py").write_text(_graph("V1"), encoding="utf-8") + (tmp_path / "environments").mkdir() + (tmp_path / "environments" / "dev.toml").write_text('X = "1"\n', encoding="utf-8") + snapshot = load_config(tmp_path).config_snapshot + assert snapshot is not None + frame = codec.encode_boot(snapshot=snapshot, forbidden=(), mem_mb=None, code_sets=None) + boot = codec.decode_boot(frame) + assert dict(boot.snapshot.files) == {"graph.py": snapshot.files["graph.py"]} + assert b"config_dir" not in frame + # The worker's load reads nothing from disk: it runs with the directory gone. + (tmp_path / "graph.py").unlink() + assert _ran(load_snapshot(boot.snapshot)) == "V1" + + +@pytest.mark.parametrize( + "rel", ["../graph.py", "sub/graph.py", "codesets/x.py", "/abs.py", ".hidden.py", "a\0b.py"] +) +def test_a_snapshot_refuses_a_path_no_load_reads(rel: str) -> None: + with pytest.raises(ValueError, match="config snapshot path"): + ConfigSnapshot(directory=Path("cfg"), files={rel: b""}) + + +@pytest.mark.skipif(os.name != "nt", reason="a backslash and a drive are plain names on POSIX") +@pytest.mark.parametrize("rel", ["a\\b.py", "C:x.py", "codesets/C:x.csv"]) +def test_a_snapshot_refuses_a_name_windows_would_not_keep_in_the_folder(rel: str) -> None: + """Joined to the directory, ``C:x.py`` names another drive and ``a\\b.py`` a sub-folder, and + either path's ``name`` is ``x.py`` or ``b.py``: a boot frame naming one could run another + module's bytes under its name.""" + with pytest.raises(ValueError, match="config snapshot path"): + ConfigSnapshot(directory=Path("cfg"), files={rel: b""}) + + +@pytest.mark.skipif(os.name == "nt", reason="Windows refuses a backslash in a file name") +def test_a_posix_module_named_with_a_backslash_loads(tmp_path: Path) -> None: + """A name the listing returns is one a snapshot holds, so it loads as it did, rather than + escaping the load as a bare ValueError.""" + (tmp_path / "a\\graph.py").write_text(_graph("V1"), encoding="utf-8") + registry = load_config(tmp_path) + assert _ran(registry) == "V1" + assert registry.config_snapshot is not None + assert registry.config_snapshot.modules == ("a\\graph.py",) + + +def test_modules_run_in_the_order_the_directory_listing_gave(tmp_path: Path) -> None: + """Path order, as the loader globbed it: without case on Windows, so a bundle's load order and + the file a duplicate-name refusal names stay as they were.""" + names = ["A_feed.py", "a2.py", "b_feed.py", "Router.py"] + for name in names: + (tmp_path / name).write_text("X = 1\n", encoding="utf-8") + snapshot = read_config_snapshot(tmp_path, strict=True) + assert snapshot.modules == tuple(p.name for p in config_py_files(tmp_path)) + assert snapshot.modules == tuple(sorted(names, key=PurePath)) + + +def test_a_worker_sent_the_engines_tables_is_not_sent_their_files(tmp_path: Path) -> None: + """One boot frame carries each table once: the engine's parsed tables when it publishes any, + so the frame's 64 MiB cap admits as many tables under ``subprocess`` as under ``off``. The + worker's load publishes them before any module runs, so a top-level capture resolves.""" + (tmp_path / "graph.py").write_text( + _graph("V1") + '\nfrom messagefoundry import code_set\nDIETS = code_set("diets")\n', + encoding="utf-8", + ) + (tmp_path / "codesets").mkdir() + (tmp_path / "codesets" / "diets.csv").write_text("k,v\na,b\n", encoding="utf-8") + registry = load_config(tmp_path) + snapshot = registry.config_snapshot + assert snapshot is not None and "codesets/diets.csv" in snapshot.files + published = codec.decode_boot( + codec.encode_boot( + snapshot=snapshot, forbidden=(), mem_mb=None, code_sets=registry.code_sets + ) + ) + assert set(published.snapshot.files) == {"graph.py"} + assert published.code_sets is not None + worker = load_snapshot(published.snapshot, code_sets=published.code_sets) + assert worker.code_sets["diets"]["a"] == "b" + assert worker.code_digest == registry.code_digest + # Control: with no tables published, the files go instead, and the worker parses them. + unpublished = codec.decode_boot( + codec.encode_boot(snapshot=snapshot, forbidden=(), mem_mb=None, code_sets=None) + ) + assert "codesets/diets.csv" in unpublished.snapshot.files + assert load_snapshot(unpublished.snapshot).code_sets["diets"]["a"] == "b" + + +def test_a_session_that_cannot_read_its_snapshot_starts_no_worker( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A session given no snapshot reads its directory before the child is started, so a read + that fails leaves no child, job handle or drain thread behind.""" + + def no_child(*_args: object, **_kwargs: object) -> object: + raise AssertionError("a worker was started") + + monkeypatch.setattr(subprocess, "Popen", no_child) + session = SandboxSession( + SandboxPolicy(mode=SandboxMode.SUBPROCESS, wall_seconds=30.0), + inbound="IB_SNAP", + config_dir=tmp_path / "missing", + env=None, + graph=None, + ) + try: + with pytest.raises(SandboxError, match="failed to bootstrap"): + session._spawn() + assert session._proc is None + finally: + session.close() + + +def test_a_load_failure_reads_its_line_from_the_bytes_that_ran( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The diagnostic names an identifier only when the line spells it, and that line comes from + the snapshot: the file may hold other bytes by now, and a worker reads no file.""" + (tmp_path / "graph.py").write_text("X = undefined_marker_name\n", encoding="utf-8") + + def no_file(*_args: object, **_kwargs: object) -> object: + raise OSError("the directory was read") + + monkeypatch.setattr(tokenize, "open", no_file) + with pytest.raises(WiringError, match="naming 'undefined_marker_name'"): + load_config(tmp_path) + + +# --- the points the row folds in ------------------------------------------------------------------- + + +def test_a_code_set_spelled_in_upper_case_is_hashed_as_it_loads(tmp_path: Path) -> None: + """``codesets/*.csv`` was a case-sensitive glob on POSIX, while the loader matches the suffix + in lower case, so ``x.CSV`` loaded unhashed on Linux. Only a POSIX run discriminates: a + Windows glob already matched it.""" + (tmp_path / "graph.py").write_text(_graph("V1"), encoding="utf-8") + (tmp_path / "codesets").mkdir() + table = tmp_path / "codesets" / "diets.CSV" + table.write_text("k,v\na,b\n", encoding="utf-8") + assert "diets" in load_config(tmp_path).code_sets + before = config_fingerprint(tmp_path) + table.write_text("k,v\na,c\n", encoding="utf-8") + assert config_fingerprint(tmp_path) != before + assert "codesets/diets.CSV" in read_config_snapshot(tmp_path, strict=True).files + + +def test_a_module_linked_in_beside_another_changes_the_digest(tmp_path: Path) -> None: + """A path that resolves to a file another path names was hashed once, so a ``_`` helper added + as a link to an existing module released with an unchanged digest (vault BACKLOG #3535 item 2). + Each path is hashed now. The link sorts after its target on every platform, which is the + order in which the old fold skipped it; a link sorting first replaced its target's path and + did change the digest.""" + (tmp_path / "graph.py").write_text(_graph("V1"), encoding="utf-8") + (tmp_path / "_shared.py").write_text("X = 1\n", encoding="utf-8") + before = config_fingerprint(tmp_path) + try: + os.symlink(tmp_path / "_shared.py", tmp_path / "_shared_copy.py") + except (OSError, NotImplementedError) as exc: # Windows without the symlink privilege + pytest.skip(f"cannot create a symlink here: {exc}") + assert config_fingerprint(tmp_path) != before + + +def test_a_load_refuses_a_module_it_cannot_read( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """It cannot run bytes it does not have. The observational digest names the file instead.""" + (tmp_path / "graph.py").write_text(_graph("V1"), encoding="utf-8") + real = Path.read_bytes + + def refuse(self: Path) -> bytes: + if self.name == "graph.py": + raise PermissionError(13, "Permission denied", str(self)) + return real(self) + + monkeypatch.setattr(Path, "read_bytes", refuse) + with pytest.raises(WiringError, match="could not read graph.py"): + load_config(tmp_path) + assert read_config_snapshot(tmp_path, strict=False).unreadable == ("graph.py",) + + +def test_an_unreadable_module_named_like_the_tables_folder_still_declares( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """``codesets_map.py`` is a module, which can declare connections, not a table in + ``codesets/``: its read failure explains an empty graph, so no second, derived diagnostic.""" + (tmp_path / "codesets_map.py").write_text(_graph("V1"), encoding="utf-8") + real = Path.read_bytes + + def refuse(self: Path) -> bytes: + if self.name == "codesets_map.py": + raise PermissionError(13, "Permission denied", str(self)) + return real(self) + + monkeypatch.setattr(Path, "read_bytes", refuse) + messages = [d.message for d in validate_config(tmp_path)] + assert messages == ["could not read codesets_map.py"] + + +def test_a_boot_frame_file_that_is_not_ascii_base64_is_a_codec_refusal() -> None: + """``b64decode`` of a str with a non-ASCII character raises a plain ValueError, not + binascii.Error; a frame rejection is still the codec's own error.""" + node = {"dir": "cfg", "files": [["graph.py", "éAAA"]]} + with pytest.raises(codec.SandboxCodecError, match="not base64"): + codec.dec_snapshot(node, codec._Reader([])) + + +def test_a_directory_gone_after_its_check_still_answers_as_missing( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A lax read reads a vanished directory as an empty one. A load must refuse it as missing, + not run an empty graph, or with ``allow_empty`` return one (BACKLOG #3157).""" + cfg = tmp_path / "cfg" + cfg.mkdir() + (cfg / "graph.py").write_text(_graph("V1"), encoding="utf-8") + + def vanishing(directory: Path, *, strict: bool) -> ConfigSnapshot: + (cfg / "graph.py").unlink() + cfg.rmdir() + return read_config_snapshot(directory, strict=strict) + + monkeypatch.setattr(wiring, "read_config_snapshot", vanishing) + with pytest.raises(ConfigDirMissing): + load_config(cfg, allow_empty=True) + + +# --- vault BACKLOG #2596: a reload's value file comes from the snapshot the digest covered ------- + + +def test_a_snapshot_names_the_value_file_bytes_it_read(tmp_path: Path) -> None: + """``environment_file`` gives the bytes the snapshot read for a file in its own + ``environments/`` folder, never the file's bytes now, and ``None`` for one that is still not + there. A file created or removed since is ``OSError``, and a file in any other folder, a + differently named one under the config directory included, is ``NotInSnapshot``.""" + cfg = tmp_path / "cfg" + (cfg / "environments").mkdir(parents=True) + (cfg / "environments" / "dev.toml").write_bytes(b'host = "approved"\n') + (cfg / "environments" / "gone.toml").write_bytes(b'host = "gone"\n') + snapshot = read_config_snapshot(cfg, strict=True) + (cfg / "environments" / "dev.toml").write_bytes(b'host = "edited"\n') + (cfg / "environments" / "new.toml").write_bytes(b'host = "created"\n') + (cfg / "environments" / "gone.toml").unlink() + + assert snapshot.environment_file(cfg / "environments" / "dev.toml") == b'host = "approved"\n' + assert snapshot.environment_file(cfg / "sub" / ".." / "environments" / "absent.toml") is None + for changed in (cfg / "environments" / "new.toml", cfg / "environments" / "gone.toml"): + with pytest.raises(OSError): + snapshot.environment_file(changed) + for elsewhere in ( + tmp_path / "environments" / "dev.toml", + cfg / "values" / "dev.toml", + cfg / "environments" / "sub" / "dev.toml", + ): + assert not snapshot.covers_environment_file(elsewhere) + with pytest.raises(NotInSnapshot): + snapshot.environment_file(elsewhere) + + +@pytest.mark.skipif(sys.platform != "win32", reason="Windows names folders and files without case") +def test_a_value_file_spelled_in_another_case_is_covered_on_windows(tmp_path: Path) -> None: + cfg = tmp_path / "cfg" + (cfg / "environments").mkdir(parents=True) + (cfg / "environments" / "dev.toml").write_bytes(b'host = "approved"\n') + snapshot = read_config_snapshot(cfg, strict=True) + assert snapshot.environment_file(cfg / "Environments" / "DEV.toml") == b'host = "approved"\n' + + +@pytest.mark.skipif(sys.platform == "win32", reason="a symlink needs a privilege on Windows") +def test_a_value_file_reached_through_a_linked_folder_is_covered(tmp_path: Path) -> None: + cfg = tmp_path / "cfg" + (cfg / "environments").mkdir(parents=True) + (cfg / "environments" / "dev.toml").write_bytes(b'host = "approved"\n') + (tmp_path / "alias").symlink_to(cfg, target_is_directory=True) + snapshot = read_config_snapshot(cfg, strict=True) + alias = tmp_path / "alias" / "environments" / "dev.toml" + assert snapshot.environment_file(alias) == b'host = "approved"\n' + + +def test_a_snapshot_that_could_not_read_a_value_file_names_none_for_it(tmp_path: Path) -> None: + """A lax snapshot that left a value file, or the folder, out cannot say what was there, so it + refuses rather than answer that there was no file. Another environment's unreadable file does + not stop a lookup of one it read.""" + cfg = tmp_path / "cfg" + (cfg / "environments").mkdir(parents=True) + (cfg / "environments" / "dev.toml").write_bytes(b'host = "dev"\n') + for unreadable in (("environments/dev.toml",), ("environments",)): + snapshot = ConfigSnapshot(directory=cfg, files={}, unreadable=unreadable) + with pytest.raises(OSError): + snapshot.environment_file(cfg / "environments" / "dev.toml") + other = ConfigSnapshot( + directory=cfg, + files={"environments/dev.toml": b'host = "dev"\n'}, + unreadable=("environments/prod.toml",), + ) + assert other.environment_file(cfg / "environments" / "dev.toml") == b'host = "dev"\n' + + +def test_serve_s_value_reader_parses_the_snapshot_or_answers_none( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """``serve``'s provider parses a value file the snapshot covers from the snapshot's bytes. For a + value file outside the config directory it answers ``None``, so the engine reads the disk.""" + from messagefoundry.__main__ import _environment_values_reader + from messagefoundry.config.environments import SnapshotEnvValuesProvider + + monkeypatch.setattr( + "messagefoundry.config.wiring.assert_trusted_start_paths", lambda *_a, **_k: None + ) + cfg = tmp_path / "cfg" + (cfg / "environments").mkdir(parents=True) + (cfg / "environments" / "dev.toml").write_bytes(b'HOST = "approved"\n') + snapshot = read_config_snapshot(cfg, strict=True) + (cfg / "environments" / "dev.toml").write_bytes(b'HOST = "edited"\n') + + inside = _environment_values_reader(base_dir=cfg, dir_name="environments", environment="dev") + assert isinstance(inside, SnapshotEnvValuesProvider) + assert inside.from_snapshot(snapshot) == {"host": "approved"} + assert inside() == {"host": "edited"} # the control: a start reads the disk + + (tmp_path / "environments").mkdir() + (tmp_path / "environments" / "dev.toml").write_bytes(b'HOST = "outside"\n') + outside = _environment_values_reader( + base_dir=tmp_path, dir_name="environments", environment="dev" + ) + assert outside.from_snapshot(snapshot) is None + assert outside() == {"host": "outside"} diff --git a/tests/test_corepoint_import_differential.py b/tests/test_corepoint_import_differential.py index 0c44a26d7..e18f95907 100644 --- a/tests/test_corepoint_import_differential.py +++ b/tests/test_corepoint_import_differential.py @@ -80,6 +80,7 @@ import pytest from messagefoundry import corepoint_import as head +from messagefoundry.config.snapshot import read_config_snapshot from messagefoundry.config.wiring import Registry, _loading from messagefoundry.parsing.message import Message @@ -1137,7 +1138,7 @@ def _execute(src: str, where: Path) -> _Run: # Every refusal is read as a human deleting it: the handler runs on past it, so a send after a # refusal is executed and checked too, in both importers alike. runnable = _REFUSAL.sub(r"\1pass # refusal removed by the guard", src) - with _loading(where, registry): + with _loading(read_config_snapshot(where, strict=False), registry): exec(compile(runnable, _GENERATED, "exec"), namespace) inp = Message.parse(_INPUT) try: diff --git a/tests/test_dual_control_reload.py b/tests/test_dual_control_reload.py index 58e0ccadd..cecf98a55 100644 --- a/tests/test_dual_control_reload.py +++ b/tests/test_dual_control_reload.py @@ -288,9 +288,9 @@ async def test_the_reload_audit_takes_no_fingerprint_after_the_swap( calls: list[object] = [] real = fp_mod.config_fingerprint_detail - def _counting(path: Path) -> dict[str, object]: + def _counting(path: Path, **kwargs: Any) -> dict[str, object]: calls.append(path) - return real(path) + return real(path, **kwargs) service = await _service(engine) await _add(service, "deployer", Role.ADMINISTRATOR) @@ -639,6 +639,8 @@ async def test_a_released_reload_refuses_a_swapped_settings_anchor( """Before BACKLOG #2034 only the inline route ran the settings-anchor preflight, so a reload held for a second approver went live on a substituted anchor when it was released. The engine now runs it on every real reload. The control is a release before the swap, which goes live.""" + from messagefoundry.config.fingerprint import config_fingerprint + good = _block(b"good") anchor = tmp_path / "ad-ca.pem" anchor.write_bytes(good) @@ -701,7 +703,17 @@ async def hold_and_release() -> httpx.Response: rows = await store.list_audit(limit=100) assert "approval.failed" in [r["action"] for r in rows] failed = [json.loads(r["detail"]) for r in rows if r["action"] == "config_reload_failed"] - assert failed == [{"requested": None, "dry_run": False, "reason": "trust_anchor"}] + # The settings preflight runs before the digest check, so the row says it was not reached + # and names the approval it stopped (vault BACKLOG #2596). + assert failed == [ + { + "requested": None, + "dry_run": False, + "approved_fingerprint": config_fingerprint(cfg), + "approval_digest_check": "not_reached", + "reason": "trust_anchor", + } + ] events = [json.loads(r["detail"])["event"] for r in rows if r["action"] == ta.AUDIT_ACTION] assert "pin_mismatch" in events finally: @@ -1190,16 +1202,14 @@ def _edit_with_marker(cfg: Path, marker: Path) -> None: ) -def _editing_load(cfg: Path, marker: Path, *, then_raise: bool = False) -> Any: - """A ``load_config`` that loads ``cfg``, then edits it as a writer would mid-release.""" +def _editing_load(cfg: Path, marker: Path) -> Any: + """A ``load_config`` that edits ``cfg`` as a writer would mid-release, after the release read + its snapshot, then loads what the release hands it.""" from messagefoundry.config.wiring import load_config - def _load(path: Path) -> Any: - registry = load_config(path) + def _load(path: Path, **kwargs: Any) -> Any: _edit_with_marker(cfg, marker) - if then_raise: - raise WiringError("the edited module does not load") - return registry + return load_config(path, **kwargs) return _load @@ -1269,26 +1279,32 @@ async def test_a_release_after_the_config_was_edited_is_refused_before_any_of_it assert any("changed after its approval" in rec.getMessage() for rec in caplog.records) -async def test_an_edit_made_while_the_release_loads_is_refused_before_the_swap( +async def test_an_edit_made_while_the_release_loads_neither_runs_nor_is_recorded( engine: Engine, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - """The second check: the bytes matched before load_config and changed during it. The digest - taken before the swap, the one the audit row records, must still be the approved one.""" - editing = _editing_load(tmp_path / "cfg", tmp_path / "unused.txt") + """Vault BACKLOG #2596 step 1. The release reads the directory once, checks that snapshot + against the approval, and loads the snapshot, so an edit after that read never runs and the + audit row names the approved bytes. Before, the edit ran its module code and a second check + refused it after the fact. The control loads the edited directory afresh: its marker appears.""" + from messagefoundry.config.wiring import load_config + + cfg, marker = tmp_path / "cfg", tmp_path / "ran.txt" + editing = _editing_load(cfg, marker) service = await _service(engine) - live = engine.registry_runner - before = live.registry if live is not None else None - _approval_id, r = await _hold_then_release( + approval_id, r = await _hold_then_release( engine, service, {}, between=lambda: monkeypatch.setattr("messagefoundry.pipeline.engine.load_config", editing), ) - assert r.status_code == 422, r.text - rr = engine.registry_runner - assert (rr.registry if rr is not None else None) is before # nothing swapped - assert len(await _digest_mismatch_rows(engine)) == 1 - assert await engine.store.list_audit(action="config_reload") == [] + assert r.status_code == 200, r.text + assert not marker.exists(), "the edit made after the release read the config ran" + approved = _held_params(await engine.store.get_pending_approval(approval_id))["fingerprint"] + rows = await engine.store.list_audit(action="config_reload") + assert [json.loads(row["detail"])["fingerprint"] for row in rows] == [approved] + assert await _digest_mismatch_rows(engine) == [] + load_config(cfg) # the control + assert marker.exists() async def test_a_hold_that_captured_no_digest_is_refused_at_release( @@ -1377,8 +1393,6 @@ async def test_a_release_refused_at_the_second_check_keeps_the_running_env_value ) -> None: """The release re-reads the environment's values before it loads. A refusal after that read must not leave the new values on the running graph, which no approver saw.""" - from messagefoundry.config.wiring import load_config - values = {"HOST": "approved.example.invalid"} monkeypatch.setattr(engine, "_env_values_provider", lambda: dict(values)) await engine.reload_detail(dry_run=False) # a live runner, on the first values @@ -1386,20 +1400,22 @@ async def test_a_release_refused_at_the_second_check_keeps_the_running_env_value assert runner is not None eng_any, runner_any = cast(Any, engine), cast(Any, runner) assert eng_any._env_values == runner_any._env_values == {"HOST": "approved.example.invalid"} - editing = _editing_load(tmp_path / "cfg", tmp_path / "unused.txt") + + async def _refusing_preflight(_registry: object, _values: object) -> None: + raise WiringError("the graph is refused after the values were read") def _between() -> None: values["HOST"] = "unreviewed.example.invalid" - monkeypatch.setattr("messagefoundry.pipeline.engine.load_config", editing) + monkeypatch.setattr(engine, "_registry_preflight", _refusing_preflight) service = await _service(engine) _approval_id, r = await _hold_then_release(engine, service, {}, between=_between) assert r.status_code == 422, r.text - assert len(await _digest_mismatch_rows(engine)) == 1 + assert [d["reason"] for d in await _failed_reload_rows(engine)] == ["invalid_config"] assert engine.registry_runner is runner assert eng_any._env_values == runner_any._env_values == {"HOST": "approved.example.invalid"} # The control: an unrefused reload does apply the new values, so the assert above can fail. - monkeypatch.setattr("messagefoundry.pipeline.engine.load_config", load_config) + monkeypatch.setattr(engine, "_registry_preflight", None) await engine.reload_detail(dry_run=False) assert eng_any._env_values == runner_any._env_values == {"HOST": "unreviewed.example.invalid"} @@ -1460,42 +1476,13 @@ def _recording_check(registry: Any, *env_values: Any) -> None: assert eng_any._env_values == runner_any._env_values == {"HOST": "first.example.invalid"} -@pytest.mark.parametrize("breaks_at", ["load", "preflight"]) -async def test_a_tamper_that_also_breaks_the_graph_is_audited_as_a_digest_mismatch( - engine: Engine, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, breaks_at: str -) -> None: - """An edit during the release that leaves a bad graph was audited as ``invalid_config``, so an - audit filter on ``digest_mismatch`` never saw the tamper. ``load``: the edited module fails to - load. ``preflight``: it loads, and a later check would refuse it; that check must not run.""" - editing = _editing_load( - tmp_path / "cfg", tmp_path / "unused.txt", then_raise=breaks_at == "load" - ) - preflights: list[object] = [] - - async def _refusing_preflight(registry: object, _values: object) -> None: - preflights.append(registry) - raise WiringError("the edited graph is refused") - - def _between() -> None: - monkeypatch.setattr("messagefoundry.pipeline.engine.load_config", editing) - monkeypatch.setattr(engine, "_registry_preflight", _refusing_preflight) - - service = await _service(engine) - _approval_id, r = await _hold_then_release(engine, service, {}, between=_between) - assert r.status_code == 422, r.text - assert r.json()["detail"] == ( - "the config changed after this reload was requested; request the reload again" - ) - assert [d["reason"] for d in await _failed_reload_rows(engine)] == ["digest_mismatch"] - assert preflights == [] - - async def test_a_load_failure_on_the_approved_bytes_is_still_invalid_config( engine: Engine, monkeypatch: pytest.MonkeyPatch ) -> None: - """The control for the test above: with nothing edited, a bad graph is a bad graph.""" + """A load failure of the approved bytes is a bad graph, not an edit: the load runs the + snapshot the release checked, so no edit can have caused it (vault BACKLOG #2596).""" - def _fails(_path: Path) -> Any: + def _fails(_path: Path, **_kwargs: Any) -> Any: raise WiringError("bad graph") service = await _service(engine) @@ -1507,7 +1494,55 @@ def _fails(_path: Path) -> Any: ) assert r.status_code == 422, r.text assert r.json()["detail"] == "invalid configuration" - assert [d["reason"] for d in await _failed_reload_rows(engine)] == ["invalid_config"] + # The digest check ran and passed, so the row carries no not-reached marker. + assert await _failed_reload_rows(engine) == [ + {"requested": None, "dry_run": False, "reason": "invalid_config"} + ] + + +async def test_a_release_refused_by_the_source_trust_check_names_its_approval( + engine: Engine, tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A tamper that also loosens the config directory's permissions is refused by the trust check, + which runs before the digest check, so its row reads ``invalid_config`` and not + ``digest_mismatch``. The row now names the approval's digest and says the digest check was not + reached, so it is not read as a refusal of the approved bytes (vault BACKLOG #2596). The + loosening is the trust check's own refusal, stubbed, as a permission change cannot be made the + same way on every platform; the stub is the name the snapshot read calls.""" + from messagefoundry.config import wiring + from messagefoundry.config.fingerprint import config_fingerprint + + cfg, marker = tmp_path / "cfg", tmp_path / "ran.txt" + approved = config_fingerprint(cfg) + + def _loosened(directory: Path) -> None: + raise WiringError(f"refusing to load config from {directory}: it is writable by others") + + def _tamper() -> None: + _edit_with_marker(cfg, marker) + monkeypatch.setattr(wiring, "_assert_safe_config_source", _loosened) + + service = await _service(engine) + live = engine.registry_runner + before = live.registry if live is not None else None + approval_id, r = await _hold_then_release(engine, service, {}, between=_tamper) + assert r.status_code == 422, r.text + assert r.json()["detail"] == "invalid configuration" + assert not marker.exists(), "the edited module ran before the refusal" + rr = engine.registry_runner + assert (rr.registry if rr is not None else None) is before # nothing swapped + row = await engine.store.get_pending_approval(approval_id) + assert row is not None and str(row["status"]) == "failed" + # Digests and the marker only: the row's exact keys show no path or file content entered it. + assert await _failed_reload_rows(engine) == [ + { + "requested": None, + "dry_run": False, + "approved_fingerprint": approved, + "approval_digest_check": "not_reached", + "reason": "invalid_config", + } + ] async def test_a_digest_that_cannot_be_taken_at_release_says_so( @@ -1645,33 +1680,246 @@ def _scandir(path: Any) -> Any: assert [d["reason"] for d in await _failed_reload_rows(engine)] == ["unreadable"] -async def test_a_file_added_during_the_load_and_left_unreadable_is_not_released( +async def test_a_file_added_during_the_load_is_not_released_with_it( engine: Engine, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - """The digest the audit row records skips a file it cannot read, so on its own it matched the - approval with the added file unbound. The fail-closed digest runs first.""" + """Round 3 refused this release because a later digest could not read the added file. Since + vault BACKLOG #2596 the release loads the snapshot it checked, so the added module never runs + and the row names the approved digest.""" from messagefoundry.config.wiring import load_config - cfg = tmp_path / "cfg" - unreadable = {"on": False} - _unreadable(monkeypatch, "extra.py", when=unreadable) + cfg, marker = tmp_path / "cfg", tmp_path / "extra-ran.txt" - def _load(path: Path) -> Any: - registry = load_config(path) - (cfg / "extra.py").write_text("X = 1\n", encoding="utf-8") - unreadable["on"] = True - return registry + def _load(path: Path, **kwargs: Any) -> Any: + (cfg / "extra.py").write_text( + f"open({str(marker)!r}, 'w').write('ran')\n", encoding="utf-8" + ) + return load_config(path, **kwargs) service = await _service(engine) - _approval_id, r = await _hold_then_release( + approval_id, r = await _hold_then_release( engine, service, {}, between=lambda: monkeypatch.setattr("messagefoundry.pipeline.engine.load_config", _load), ) - assert r.status_code == 422, r.text - assert [d["reason"] for d in await _failed_reload_rows(engine)] == ["digest_unavailable"] - assert await engine.store.list_audit(action="config_reload") == [] + assert r.status_code == 200, r.text + assert not marker.exists(), "a module added after the release read the config ran" + approved = _held_params(await engine.store.get_pending_approval(approval_id))["fingerprint"] + rows = await engine.store.list_audit(action="config_reload") + assert [json.loads(row["detail"])["fingerprint"] for row in rows] == [approved] + + +# --- vault BACKLOG #2596: the environment value file under the config directory ------------------ + +_APPROVED_VALUES = 'HOST = "approved.example.invalid"\n' +_UNREVIEWED_VALUES = 'HOST = "unreviewed.example.invalid"\n' + + +def _serve_values_provider(cfg: Path) -> Any: + """The value provider ``serve`` builds, anchored at the config directory, so the value file it + reads is the ``environments/dev.toml`` the approval digest covers.""" + from messagefoundry.__main__ import _environment_values_reader + + return _environment_values_reader(base_dir=cfg, dir_name="environments", environment="dev") + + +def _opaque_values_provider(cfg: Path) -> Callable[[], dict[str, Any]]: + """An embedder's provider that reads the same file itself, so the engine cannot hand it the + snapshot's bytes.""" + from messagefoundry.config.environments import load_environment_values + + def _read() -> dict[str, Any]: + return load_environment_values( + base_dir=cfg, dir_name="environments", environment="dev", environ={} + ) + + return _read + + +async def _values_engine( + tmp_path: Path, approved_text: str | None, provider: Callable[[Path], Any] +) -> tuple[Engine, Path]: + """The ``engine`` fixture's engine, given ``provider`` over a config directory whose + ``environments/dev.toml`` holds ``approved_text``, or does not exist when it is ``None``.""" + cfg = tmp_path / "cfg" + _write_valid_config(cfg, tmp_path / "in", tmp_path / "out") + values_file = cfg / "environments" / "dev.toml" + if approved_text is not None: + values_file.parent.mkdir() + values_file.write_text(approved_text, encoding="utf-8") + engine = await Engine.create( + tmp_path / "dc.db", + poll_interval=0.02, + config_dir=cfg, + egress_settings=EgressSettings(deny_by_default=False), + env_values_provider=provider(cfg), + ) + return engine, values_file + + +def _edit_values_after_the_snapshot( + monkeypatch: pytest.MonkeyPatch, values_file: Path, text: str +) -> None: + """Write ``text`` to ``values_file`` right after the release reads its snapshot, and so before + the value provider reads the file: the window a writer to the config directory would use.""" + from messagefoundry.config.wiring import read_trusted_snapshot + + def _read_then_edit(directory: Path, **kwargs: Any) -> Any: + snapshot = read_trusted_snapshot(directory, **kwargs) + values_file.parent.mkdir(exist_ok=True) + values_file.write_text(text, encoding="utf-8") + return snapshot + + monkeypatch.setattr("messagefoundry.pipeline.engine.read_trusted_snapshot", _read_then_edit) + + +async def test_a_value_file_edited_after_the_release_read_its_snapshot_never_applies( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Vault BACKLOG #2596. The approval digest covers ``environments/dev.toml`` under the config + directory. ``serve``'s provider reads the snapshot's bytes for it on a reload, so an edit made + between the release's read and the provider's read never applies, and the release goes on with + the approved values. Before, the provider read the file from disk again, and the edited values + applied under the approved digest.""" + engine, values_file = await _values_engine(tmp_path, _APPROVED_VALUES, _serve_values_provider) + approved_values = {"host": "approved.example.invalid"} + try: + assert cast(Any, engine)._env_values == approved_values + service = await _service(engine) + approval_id, r = await _hold_then_release( + engine, + service, + {}, + between=lambda: _edit_values_after_the_snapshot( + monkeypatch, values_file, _UNREVIEWED_VALUES + ), + ) + assert r.status_code == 200, r.text + assert cast(Any, engine)._env_values == approved_values + runner = engine.registry_runner + assert runner is not None and cast(Any, runner)._env_values == approved_values + approved = _held_params(await engine.store.get_pending_approval(approval_id)) + rows = await engine.store.list_audit(action="config_reload") + assert [json.loads(row["detail"])["fingerprint"] for row in rows] == [ + approved["fingerprint"] + ] + # The control: a reload that reads the directory afresh applies the edited file. + monkeypatch.undo() + await engine.reload_detail(dry_run=False) + assert cast(Any, engine)._env_values == {"host": "unreviewed.example.invalid"} + finally: + await engine.stop() + + +@pytest.mark.parametrize("approved_text", [None, _APPROVED_VALUES], ids=["created", "removed"]) +async def test_a_value_file_created_or_removed_after_the_snapshot_is_refused( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, approved_text: str | None +) -> None: + """Vault BACKLOG #2596. The snapshot's bytes cannot stand for a value file created since, nor + for one removed since, whose access list can no longer be vetted. ``serve``'s provider refuses + both, with nothing swapped and the running values kept.""" + engine, values_file = await _values_engine(tmp_path, approved_text, _serve_values_provider) + try: + await engine.reload_detail(dry_run=False) # a live runner, on the approved values + runner = engine.registry_runner + assert runner is not None + approved_values = {"host": "approved.example.invalid"} if approved_text else {} + assert cast(Any, engine)._env_values == approved_values + from messagefoundry.config.wiring import read_trusted_snapshot + + def _read_then_change(directory: Path, **kwargs: Any) -> Any: + snapshot = read_trusted_snapshot(directory, **kwargs) + if approved_text is None: + values_file.parent.mkdir(exist_ok=True) + values_file.write_text(_UNREVIEWED_VALUES, encoding="utf-8") + else: + values_file.unlink() + return snapshot + + service = await _service(engine) + _approval_id, r = await _hold_then_release( + engine, + service, + {}, + between=lambda: monkeypatch.setattr( + "messagefoundry.pipeline.engine.read_trusted_snapshot", _read_then_change + ), + ) + assert r.status_code == 422, r.text + assert engine.registry_runner is runner # nothing swapped + assert cast(Any, engine)._env_values == approved_values + assert cast(Any, runner)._env_values == approved_values + assert [d["reason"] for d in await _failed_reload_rows(engine)] == ["invalid_config"] + finally: + await engine.stop() + + +async def test_an_opaque_provider_s_value_file_edited_during_the_release_is_refused( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Vault BACKLOG #2596. A provider the engine cannot hand the snapshot reads the file from + disk, so the release takes the digest again after that read, and refuses an edit made in the + window as ``digest_mismatch``, with nothing swapped and the running values kept.""" + from messagefoundry.config.fingerprint import config_fingerprint + + engine, values_file = await _values_engine(tmp_path, _APPROVED_VALUES, _opaque_values_provider) + cfg = values_file.parent.parent + try: + await engine.reload_detail(dry_run=False) # a live runner, on the approved values + runner = engine.registry_runner + assert runner is not None + approved_values = {"host": "approved.example.invalid"} + assert cast(Any, engine)._env_values == approved_values + approved = config_fingerprint(cfg) + service = await _service(engine) + _approval_id, r = await _hold_then_release( + engine, + service, + {}, + between=lambda: _edit_values_after_the_snapshot( + monkeypatch, values_file, _UNREVIEWED_VALUES + ), + ) + assert r.status_code == 422, r.text + assert engine.registry_runner is runner # nothing swapped + assert cast(Any, engine)._env_values == approved_values + assert cast(Any, runner)._env_values == approved_values + assert await _digest_mismatch_rows(engine) == [ + { + "requested": None, + "dry_run": False, + "reason": "digest_mismatch", + "approved_fingerprint": approved, + "current_fingerprint": config_fingerprint(cfg), + } + ] + finally: + await engine.stop() + + +async def test_a_provider_with_a_from_snapshot_method_of_its_own_is_not_handed_the_snapshot( + engine: Engine, monkeypatch: pytest.MonkeyPatch +) -> None: + """A provider opts in to the snapshot by subclassing ``SnapshotEnvValuesProvider``. One that + merely has a method of that name, such as an alternate constructor, is called plainly, so the + release still checks the digest again after it reads.""" + calls: list[str] = [] + + class _Provider: + def __call__(self) -> dict[str, Any]: + calls.append("plain") + return {} + + def from_snapshot(self, _snapshot: object) -> dict[str, Any]: + calls.append("from_snapshot") + return {} + + monkeypatch.setattr(engine, "_env_values_provider", _Provider()) + service = await _service(engine) + _approval_id, r = await _hold_then_release(engine, service, {}) + assert r.status_code == 200, r.text + assert calls == ["plain"] async def test_a_directory_moved_away_after_the_resolve_is_refused_as_missing( @@ -1834,27 +2082,6 @@ def _refused(_registry: Any) -> Any: assert eng_any._env_values == new -async def test_a_digest_refusal_after_a_failed_load_does_not_carry_the_load_error( - engine: Engine, tmp_path: Path, monkeypatch: pytest.MonkeyPatch -) -> None: - """Round 2 raises the digest refusal after the load's handler, so the load error rides neither - ``__cause__`` nor ``__context__``, where a TOMLDecodeError would keep a whole value file - (BACKLOG #2307).""" - from messagefoundry.config.fingerprint import config_fingerprint - from messagefoundry.pipeline import ApprovedConfig, ConfigDigestMismatch - - cfg = tmp_path / "cfg" - approval = ApprovedConfig(config_fingerprint(cfg)) - monkeypatch.setattr( - "messagefoundry.pipeline.engine.load_config", - _editing_load(cfg, tmp_path / "unused.txt", then_raise=True), - ) - with pytest.raises(ConfigDigestMismatch) as refused: - await engine.reload_detail(dry_run=False, approval=approval) - assert refused.value.__cause__ is None - assert refused.value.__context__ is None - - # --- vault BACKLOG #2596, PR 2239 review: a name no directory can have is missing ------------- diff --git a/tests/test_file_path_join_census.py b/tests/test_file_path_join_census.py index 520ad83dc..f70f18b7e 100644 --- a/tests/test_file_path_join_census.py +++ b/tests/test_file_path_join_census.py @@ -579,8 +579,17 @@ def derived_script_sites() -> dict[str, int]: OPERATOR, "Reads the git metadata of the operator's own config checkout; read only, to a commit hash.", ), - "messagefoundry/config/fingerprint.py::_require_listable": Site(1, OPERATOR, _CONFIG_FIXED), "messagefoundry/config/impact.py::_validate_new_name": Site(1, OPERATOR, _CODESETS), + "messagefoundry/config/snapshot.py::ConfigSnapshot.covers_environment_file": Site( + 1, OPERATOR, _CONFIG_FIXED + ), + "messagefoundry/config/snapshot.py::_list_folders": Site(1, OPERATOR, _CONFIG_FIXED), + "messagefoundry/config/snapshot.py::read_config_snapshot": Site( + 2, + REVIEWED, + "A fixed folder name under the operator's config directory, and the names its own listing " + "of that folder returned; read, never written.", + ), "messagefoundry/config/tls_policy.py::staged_crl": Site( 1, INTERNAL, "A fixed name in a private temp directory this function made." ), @@ -594,8 +603,20 @@ def derived_script_sites() -> dict[str, int]: "access list of for the trust check; no file is opened through them here.", ), "messagefoundry/config/wiring.py::install_trust_paths": Site(2, INTERNAL, _PKG_LITERAL), - "messagefoundry/config/wiring.py::load_config": Site(1, OPERATOR, _CODESETS), - "messagefoundry/config/wiring.py::validate_config": Site(1, OPERATOR, _CODESETS), + "messagefoundry/config/wiring.py::load_snapshot": Site( + 2, + REVIEWED, + "The snapshot's directory joined to the fixed codesets name, which only names the folder in " + "a refusal, and to each module name the snapshot holds, which ConfigSnapshot checks is a " + "plain name on this platform (vault BACKLOG #2596). Compiled under, never opened.", + ), + "messagefoundry/config/wiring.py::validate_config": Site( + 3, + REVIEWED, + "The operator's config directory joined to the fixed codesets name, to a module name its " + "snapshot holds (checked a plain name, compiled under and never opened), and to an " + "unreadable file's relative path, which only names it in a diagnostic.", + ), "messagefoundry/corepoint_import.py::_replace_modules": Site( 1, INTERNAL, "A pid temp name beside a target the importer already built." ), diff --git a/tests/test_from_none_is_not_redaction.py b/tests/test_from_none_is_not_redaction.py index d2efecf01..6ce185294 100644 --- a/tests/test_from_none_is_not_redaction.py +++ b/tests/test_from_none_is_not_redaction.py @@ -143,7 +143,7 @@ class _Allowed: _SAFE + "KeyError carries the lookup key, which the new KeyError also quotes", ), _Allowed( - "messagefoundry/config/code_sets.py::load_policy::CodeSetError::CodeSetError", + "messagefoundry/config/code_sets.py::parse_policy::CodeSetError::CodeSetError", _SAFE + "the new message interpolates the caught CodeSetError whole", ), _Allowed( diff --git a/tests/test_passthrough.py b/tests/test_passthrough.py index 4e53ea3aa..96349d896 100644 --- a/tests/test_passthrough.py +++ b/tests/test_passthrough.py @@ -767,10 +767,10 @@ def _file_graph(inbox: Any, outdir: Any, *, with_pt: bool) -> Registry: def _patch_load_config(monkeypatch: Any, registry: Registry) -> None: - """Make engine.reload()'s ``load_config(path)`` return ``registry`` (it runs in a thread, so the - target imported into the engine module is what we patch).""" + """Make engine.reload()'s ``load_config(path, snapshot=...)`` return ``registry`` (it runs in a + thread, so the target imported into the engine module is what we patch).""" monkeypatch.setattr( - "messagefoundry.pipeline.engine.load_config", lambda _path: registry, raising=True + "messagefoundry.pipeline.engine.load_config", lambda _path, **_kw: registry, raising=True ) diff --git a/tests/test_reference_sets.py b/tests/test_reference_sets.py index 78b4a4d8e..9f68629bf 100644 --- a/tests/test_reference_sets.py +++ b/tests/test_reference_sets.py @@ -819,7 +819,7 @@ async def test_reload_refuses_adding_a_reference_set_on_unsupporting_backend( monkeypatch.setattr( "messagefoundry.pipeline.engine.load_config", - lambda _path: _ref_graph(csv), + lambda _path, **_kw: _ref_graph(csv), raising=True, ) with pytest.raises(WiringError) as exc: diff --git a/tests/test_sandbox_codec.py b/tests/test_sandbox_codec.py index be0f51f9a..1dad6b259 100644 --- a/tests/test_sandbox_codec.py +++ b/tests/test_sandbox_codec.py @@ -16,6 +16,7 @@ import pickle import struct from collections.abc import Iterator +from pathlib import Path from types import MappingProxyType from typing import Any @@ -25,6 +26,7 @@ from messagefoundry.config.models import ContentType from messagefoundry.config.response import CapturedResponse from messagefoundry.config.run_context import RunContext, run_contexts +from messagefoundry.config.snapshot import ConfigSnapshot from messagefoundry.config.state import state_get from messagefoundry.config.wiring import ( Send, @@ -952,7 +954,7 @@ def _code_sets( def _boot_round_trip(code_sets: object) -> Any: return decode_boot( encode_boot( - config_dir="cfg", + snapshot=ConfigSnapshot(directory=Path("cfg"), files={}), forbidden=("socket",), mem_mb=512, code_sets=code_sets, diff --git a/tests/test_sandbox_graph_parity.py b/tests/test_sandbox_graph_parity.py index f31a7b15d..cc3853257 100644 --- a/tests/test_sandbox_graph_parity.py +++ b/tests/test_sandbox_graph_parity.py @@ -22,7 +22,7 @@ from messagefoundry.config import settings, wiring from messagefoundry.config.run_context import RunContext from messagefoundry.config.settings import EgressSettings -from messagefoundry.config.wiring import Registry, WiringError, config_code_digest, load_config +from messagefoundry.config.wiring import Registry, config_code_digest, load_config from messagefoundry.pipeline import _sandbox_codec as codec from messagefoundry.pipeline.dryrun import transform_one from messagefoundry.pipeline.sandbox import ( @@ -112,7 +112,9 @@ def test_a_worker_whose_graph_differs_from_the_engines_is_refused( assert "IB_PARITY" in text assert "allowlist" in text assert "[sandbox].pass_environment" in text - assert "reload" in text + # A worker runs the snapshot it is sent (vault BACKLOG #2596), so a reload is no remedy for a + # graph that differs by name, and the refusal does not send the operator after one. + assert "reload" not in text # What differs, and on which side: the engine has two handlers the worker never built. assert "handlers (engine only: h_1, h_2)" in text @@ -155,48 +157,56 @@ def test_a_config_module_edited_on_disk_refuses_the_worker_though_every_name_mat assert str(deliveries[0].payload) == "EDITED=1" -def test_a_module_rewritten_while_the_load_runs_refuses_the_load( +def test_a_module_rewritten_while_the_load_runs_does_not_change_what_runs( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - """The digest is read before the modules run. A rewrite in between would let a worker pass the - comparison with code its digest does not describe, so the load is refused. The control is - the same load with nothing rewritten.""" + """Vault BACKLOG #2596 step 1. The load reads the directory once and runs those bytes, so a + rewrite while it runs changes neither the code that runs nor the digest that describes it. + Before, the digest was read first and each module read again to run it, and a rewrite in + between was refused after it had run. The control loads the rewritten file afresh.""" monkeypatch.delenv(_VARIABLE, raising=False) - (tmp_path / "graph.py").write_text(_GRAPH, encoding="utf-8") - assert load_config(tmp_path).code_digest is not None + graph = tmp_path / "graph.py" + graph.write_text(_GRAPH, encoding="utf-8") real = wiring._exec_module def rewrite_then_run(path: Path, helpers: Any) -> None: - path.write_text(_GRAPH + "\n# edited\n", encoding="utf-8") + path.write_text(_GRAPH.replace('"COUNT="', '"EDITED="'), encoding="utf-8") real(path, helpers) monkeypatch.setattr(wiring, "_exec_module", rewrite_then_run) - with pytest.raises(WiringError, match="changed on disk while they were loading"): - load_config(tmp_path) - - -def test_a_module_unreadable_after_the_load_is_not_called_an_edit( + registry = load_config(tmp_path) + monkeypatch.setattr(wiring, "_exec_module", real) + deliveries, _, _, _ = transform_one(registry, "h_0", RAW) + assert str(deliveries[0].payload) == "COUNT=1" + assert registry.config_snapshot is not None + assert registry.code_digest == config_code_digest(registry.config_snapshot) + assert registry.code_digest != config_code_digest(tmp_path) # the disk moved on + edited = load_config(tmp_path) + assert str(transform_one(edited, "h_0", RAW)[0][0].payload) == "EDITED=1" + + +def test_a_load_reads_each_config_file_once( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: + """One read per file, for the module, a helper it imports, a code set and connections.toml: + every later use is of those bytes (vault BACKLOG #2596).""" monkeypatch.delenv(_VARIABLE, raising=False) - (tmp_path / "graph.py").write_text(_GRAPH, encoding="utf-8") - real = wiring._exec_module + (tmp_path / "graph.py").write_text("import _shared\n" + _GRAPH, encoding="utf-8") + (tmp_path / "_shared.py").write_text("X = 1\n", encoding="utf-8") + (tmp_path / "codesets").mkdir() + (tmp_path / "codesets" / "diets.csv").write_text("k,v\na,b\n", encoding="utf-8") + (tmp_path / "connections.toml").write_text("", encoding="utf-8") real_read = Path.read_bytes - ran: list[bool] = [] + reads: list[str] = [] - def refuse_after_the_load(self: Path) -> bytes: - if ran and self.name == "graph.py": - raise PermissionError("held open") + def counting(self: Path) -> bytes: + reads.append(self.name) return real_read(self) - def run(path: Path, helpers: Any) -> None: - real(path, helpers) - ran.append(True) - - monkeypatch.setattr(wiring, "_exec_module", run) - monkeypatch.setattr(Path, "read_bytes", refuse_after_the_load) - with pytest.raises(WiringError, match="could not be read again"): - load_config(tmp_path) + monkeypatch.setattr(Path, "read_bytes", counting) + registry = load_config(tmp_path) + assert "h_0" in registry.handlers + assert sorted(reads) == ["_shared.py", "connections.toml", "diets.csv", "graph.py"] def test_an_unreadable_module_leaves_no_digest_and_says_so( @@ -213,7 +223,7 @@ def refuse(self: Path) -> bytes: monkeypatch.setattr(Path, "read_bytes", refuse) with caplog.at_level("WARNING", logger="messagefoundry.config.wiring"): assert config_code_digest(tmp_path) is None - assert "not compared" in caplog.text and "held open" in caplog.text + assert "not compared" in caplog.text and "graph.py" in caplog.text def test_the_code_digest_covers_each_module_a_load_may_run(tmp_path: Path) -> None: diff --git a/tests/test_sandbox_refusal_alert.py b/tests/test_sandbox_refusal_alert.py index ccb45f65a..31b8f4cf9 100644 --- a/tests/test_sandbox_refusal_alert.py +++ b/tests/test_sandbox_refusal_alert.py @@ -2,8 +2,10 @@ # Copyright (C) 2026 MessageFoundry Foundation, LLC and contributors """A refused sandbox worker raises one ``sandbox_worker_refused`` alert (vault BACKLOG #2724). -Owner ruling 2026-10-16: alert, keep dead-letter. A config file edited on disk with no reload makes -the next ``[sandbox].mode = "subprocess"`` worker refuse. The message in hand still fails as before; +Owner ruling 2026-10-16: alert, keep dead-letter. A worker whose config module bytes differ from the +ones the engine loaded refuses. Since vault BACKLOG #2596 a worker runs the snapshot it is sent, so an +edit on disk no longer reaches it, and these tests send the worker the edited bytes to provoke the +refusal. The message in hand still fails as before; these tests pin that an operator is also told, once per refusal and not once per message, and that the alert carries fixed words and no message content. @@ -26,6 +28,7 @@ _ALERT_EVENT_TYPES, EgressSettings, ) +from messagefoundry.config.snapshot import read_config_snapshot from messagefoundry.config.wiring import Registry, load_config from messagefoundry.pipeline.alert_sinks import NotifierAlertSink from messagefoundry.pipeline.alerts import LoggingAlertSink, sandbox_worker_refused_detail @@ -78,6 +81,15 @@ def _edit(cfg: Path) -> None: (cfg / "graph.py").write_text(_GRAPH.replace('"COUNT="', '"EDITED="'), encoding="utf-8") +def _loaded_then_sent_edited(cfg: Path) -> Registry: + """A graph loaded from ``cfg`` whose snapshot, the bytes its sandbox workers are sent, is then + a read of the edited file: a worker that runs other bytes than the engine loaded.""" + registry = load_config(cfg) + _edit(cfg) + registry.config_snapshot = read_config_snapshot(cfg, strict=True) + return registry + + def _runner(registry: Registry, store: MessageStore, cfg: Path, sink: Any) -> RegistryRunner: return RegistryRunner( registry, @@ -96,8 +108,7 @@ async def test_the_first_refusal_alerts_once_and_later_messages_do_not( fails and dead-letters under the policy), the alert fires on the first, and the cached refusals for the next messages on the same inbound raise no second alert. The whole refusal, digests included, is logged once.""" - registry = load_config(edited_config) - _edit(edited_config) + registry = _loaded_then_sent_edited(edited_config) sink = _Recorder() store = await MessageStore.open(tmp_path / "refusal.db") try: @@ -214,8 +225,7 @@ async def test_the_shipped_notifier_carries_fixed_words_and_no_body( ) -> None: """What an operator's webhook receives and what the alert list shows. The CONTROL is the refusal itself, which the engine log carries: it names what differs.""" - registry = load_config(edited_config) - _edit(edited_config) + registry = _loaded_then_sent_edited(edited_config) transport, state = _RecordingTransport("webhook"), _RecordingStore() sink = NotifierAlertSink([transport], store=cast(Any, state)) store = await MessageStore.open(tmp_path / "notifier.db") @@ -249,9 +259,11 @@ def test_the_detail_names_the_cause_and_the_remedy_first() -> None: names = sandbox_worker_refused_detail(content_only=False) # The alert instance's reason column keeps what this bound leaves (docs/PHI.md section 8). kept_edited, kept_names = safe_text(edited, limit=200), safe_text(names, limit=200) - assert "Reload the config." in kept_edited and "changed on disk" in kept_edited + assert "Reload the config." in kept_edited and "other config module bytes" in kept_edited assert "pass_environment" not in edited - assert "reload the config" in kept_names + # A worker runs the engine's snapshot, so a disk edit is no cause and a reload no remedy. + assert "changed on disk" not in edited + names + assert "reload" not in names.lower() assert "add it to [sandbox].pass_environment and restart" in kept_names @@ -261,7 +273,7 @@ def test_the_logging_sink_warns(caplog: pytest.LogCaptureFixture) -> None: (record,) = caplog.records assert record.levelno == logging.WARNING assert record.getMessage().startswith( - "ALERT sandbox_worker_refused: inbound 'IB_PARITY': A config file changed on disk" + "ALERT sandbox_worker_refused: inbound 'IB_PARITY': The worker ran other config module" ) diff --git a/tests/test_send_pacing.py b/tests/test_send_pacing.py index 8317b9f06..8784fff3c 100644 --- a/tests/test_send_pacing.py +++ b/tests/test_send_pacing.py @@ -189,7 +189,7 @@ def test_validate_config_reports_an_unexpected_toml_loader_failure_as_a_diagnost def _boom(*_args: object, **_kwargs: object) -> None: raise RuntimeError("loader fell over") - monkeypatch.setattr(cf, "load_connections_file", _boom) + monkeypatch.setattr(cf, "load_connections_data", _boom) d = tmp_path / "unexpected" d.mkdir() (d / "connections.toml").write_text("", encoding="utf-8") @@ -228,7 +228,7 @@ def test_an_unexpected_loader_failure_is_scrubbed_before_it_becomes_a_diagnostic def _boom(*_args: object, **_kwargs: object) -> None: raise RuntimeError(f"connect failed password={secret}") - monkeypatch.setattr(cf, "load_connections_file", _boom) + monkeypatch.setattr(cf, "load_connections_data", _boom) d = tmp_path / "leaky" d.mkdir() (d / "connections.toml").write_text("", encoding="utf-8") diff --git a/tests/test_sibling_helper_finder.py b/tests/test_sibling_helper_finder.py index 3762aedf1..4e422b619 100644 --- a/tests/test_sibling_helper_finder.py +++ b/tests/test_sibling_helper_finder.py @@ -20,6 +20,7 @@ import pytest from messagefoundry.config.fingerprint import config_fingerprint +from messagefoundry.config.snapshot import read_config_snapshot from messagefoundry.config.wiring import ( _CONFIG_SOURCE_LEAD, WiringError, @@ -88,7 +89,7 @@ def test_importer_serves_only_underscore_helpers(tmp_path: Path) -> None: (tmp_path / "os.py").write_text("SHADOWED = True\n", encoding="utf-8") (tmp_path / "shared.py").write_text("VALUE = 1\n", encoding="utf-8") (tmp_path / "_h.py").write_text("VALUE = 1\n", encoding="utf-8") - importer = _HelperImporter(tmp_path) + importer = _HelperImporter(read_config_snapshot(tmp_path, strict=True)) do_import = importer.builtins["__import__"] # A real-module name (not '_'-prefixed) goes to the normal import even though os.py exists. assert do_import("os") is os diff --git a/tests/test_start_config_change_alert.py b/tests/test_start_config_change_alert.py index 496a09118..1e7f45a80 100644 --- a/tests/test_start_config_change_alert.py +++ b/tests/test_start_config_change_alert.py @@ -111,6 +111,15 @@ def _edit(cfg: Path) -> None: fh.write("# an edit nobody reloaded\n") +def _covered_file(cfg: Path, name: str) -> None: + """A file the fingerprint covers and no load reads, for a partial digest. Since vault BACKLOG + #2596 a load reads every module from the one snapshot the fingerprint folds, so a module the + digest cannot read is a load that refuses, not a partial digest; only a covered-only file can + be left out of a digest while the graph still loads.""" + (cfg / "environments").mkdir(exist_ok=True) + (cfg / "environments" / name).write_text('X = "1"\n', encoding="utf-8") + + # --- the store read ------------------------------------------------------------------------------ @@ -774,7 +783,7 @@ async def test_a_convergence_row_with_no_digest_is_passed_over( async with app.router.lifespan_context(app): engine: Engine = app.state.engine - async def _no_digest(self: Engine, path: Path) -> tuple[None, str]: + async def _no_digest(self: Engine, path: Path, *_snapshot: object) -> tuple[None, str]: return None, "synthetic: unreadable bundle" with monkeypatch.context() as patch: @@ -1288,12 +1297,13 @@ async def test_a_file_locked_at_one_start_pages_on_the_same_bytes_with_the_marke """Item 4, as the Lander's send-back of engine PR 2370 left it (vault BACKLOG #3765): a file an antivirus lock holds at one start and not at the next gives two digests for the same bytes. main pages on both, and so does this; the marker says why, and nothing is passed over.""" + _covered_file(cfg, "dev.toml") await _start(tmp_path, cfg) with monkeypatch.context() as m: - _unreadable(m, "cfg.py") + _unreadable(m, "dev.toml") partial = await _start(tmp_path, cfg) assert [a["comparison"] for a in alerts] == ["partial_start_digest"] - assert partial[-1][UNREADABLE_FILES_KEY] == ["cfg.py"] + assert partial[-1][UNREADABLE_FILES_KEY] == ["environments/dev.toml"] assert partial[-1]["comparison"] == "partial_start_digest" assert "baseline_unchecked" not in partial[-1] # The next start, with the file readable again, compares against the partial row and pages. @@ -1431,9 +1441,9 @@ async def test_two_digests_that_left_out_the_same_file_are_compared( ) -> None: """Item 4, review round 1: a file that stays unreadable must not turn the check off. Digests that left out the same files cover the same files, so they compare like for like.""" - (cfg / "_locked.py").write_text("LOCKED = 1\n", encoding="utf-8") + _covered_file(cfg, "locked.toml") with monkeypatch.context() as m: - _unreadable(m, "_locked.py") + _unreadable(m, "locked.toml") first = await _start(tmp_path, cfg) assert first[-1]["comparison"] == "no_baseline" rows = await _start(tmp_path, cfg) @@ -1451,16 +1461,19 @@ async def test_a_convergence_row_with_a_partial_digest_is_a_baseline( """A partial convergence digest is a baseline, as any digest is on main: a whole start compares against it and pages (vault BACKLOG #3765, item 15). Passing over it would let a start whose bytes match an older row adopt whatever the convergence reload loaded.""" + _covered_file(cfg, "dev.toml") await _start(tmp_path, cfg) app = _app(tmp_path, cfg) async with app.router.lifespan_context(app): engine: Engine = app.state.engine with monkeypatch.context() as m: - _unreadable(m, "cfg.py") + _unreadable(m, "dev.toml") await engine._converge_reload(1) reload_rows = await engine.store.list_audit(action="config_reload") row = json.loads(reload_rows[0]["detail"]) - assert row[UNREADABLE_FILES_KEY] == ["cfg.py"] and "baseline_unchecked" not in row + assert ( + row[UNREADABLE_FILES_KEY] == ["environments/dev.toml"] and "baseline_unchecked" not in row + ) rows = await _start(tmp_path, cfg) assert rows[-1]["comparison"] == "partial_baseline" assert len(alerts) == 1 and alerts[0]["previous_fingerprint"] == row["fingerprint"] @@ -1473,11 +1486,11 @@ async def test_a_failed_digest_before_the_load_never_blocks_the_start( real = Engine.fingerprint_bundle calls: list[Path] = [] - async def _first_raises(self: Engine, path: Path) -> Any: + async def _first_raises(self: Engine, path: Path, *snapshot: Any) -> Any: calls.append(path) if len(calls) == 1: raise RuntimeError("synthetic") - return await real(self, path) + return await real(self, path, *snapshot) with monkeypatch.context() as m: m.setattr(Engine, "fingerprint_bundle", _first_raises) @@ -1495,10 +1508,10 @@ async def test_a_partial_start_whose_digest_differs_pages_and_is_the_baseline( tmp_path: Path, cfg: Path, monkeypatch: pytest.MonkeyPatch, alerts: list[dict[str, Any]] ) -> None: """Item 15: an edit made while a file is unreadable pages at that start, as main does.""" - (cfg / "_locked.py").write_text("LOCKED = 1\n", encoding="utf-8") + _covered_file(cfg, "locked.toml") await _start(tmp_path, cfg) with monkeypatch.context() as m: - _unreadable(m, "_locked.py") + _unreadable(m, "locked.toml") _edit(cfg) rows = await _start(tmp_path, cfg) assert len(alerts) == 1, "main pages on this digest change" @@ -1516,9 +1529,9 @@ async def test_a_whole_start_after_a_partial_baseline_pages( ) -> None: """Item 16: the first whole start after a partial baseline compares and pages; it does not adopt the edits made since then as a new baseline.""" - (cfg / "_locked.py").write_text("LOCKED = 1\n", encoding="utf-8") + _covered_file(cfg, "locked.toml") with monkeypatch.context() as m: - _unreadable(m, "_locked.py") + _unreadable(m, "locked.toml") await _start(tmp_path, cfg) _edit(cfg) rows = await _start(tmp_path, cfg) @@ -1591,9 +1604,9 @@ async def test_a_partial_digest_whose_fold_equals_the_baseline_still_pages( while the start digested it folds to the baseline's value. Coverage is compared too.""" before = config_fingerprint(cfg) await _start(tmp_path, cfg) - (cfg / "_new.py").write_text("NEW = 1\n", encoding="utf-8") + _covered_file(cfg, "new.toml") with monkeypatch.context() as m: - _unreadable(m, "_new.py") + _unreadable(m, "new.toml") assert config_fingerprint(cfg) == before, "control: the fold matches the baseline's" rows = await _start(tmp_path, cfg) assert len(alerts) == 1 and alerts[0]["comparison"] == "partial_start_digest" diff --git a/tests/test_start_config_loaded_row.py b/tests/test_start_config_loaded_row.py index 33708a465..baa4fad46 100644 --- a/tests/test_start_config_loaded_row.py +++ b/tests/test_start_config_loaded_row.py @@ -133,7 +133,7 @@ async def test_an_unreadable_vcs_head_degrades_the_same_way_at_every_load( app, cfg = _app(tmp_path) - def _undecodable(_path: Path) -> dict[str, object]: + def _undecodable(_path: Path, **_snapshot: object) -> dict[str, object]: raise UnicodeDecodeError("utf-8", b"\xff", 0, 1, "invalid start byte") monkeypatch.setattr(fp_mod, "config_fingerprint_detail", _undecodable) diff --git a/tests/test_unicode_error_log_paths.py b/tests/test_unicode_error_log_paths.py index 77a90c161..136154c6c 100644 --- a/tests/test_unicode_error_log_paths.py +++ b/tests/test_unicode_error_log_paths.py @@ -40,6 +40,7 @@ from messagefoundry.auth import trust_anchors from messagefoundry.config.code_sets import CodeSetError, load_code_set from messagefoundry.config.connections_file import load_connections_file +from messagefoundry.config.snapshot import ConfigSnapshot from messagefoundry.config.wiring import Registry, Send, WiringError from messagefoundry.corepoint_import import ( CorepointImportError, @@ -1165,17 +1166,22 @@ def test_a_sandbox_bootstrap_unicode_failure_is_reported_without_the_character( from messagefoundry.pipeline import _sandbox_worker, sandbox stdin, stdout = io.BytesIO(), io.BytesIO() - boot = codec.encode_boot(config_dir=".", forbidden=(), mem_mb=None, code_sets=None) + boot = codec.encode_boot( + snapshot=ConfigSnapshot(directory=Path("."), files={}), + forbidden=(), + mem_mb=None, + code_sets=None, + ) sandbox._write_frame(stdin, boot) stdin.seek(0) - def unreadable(config_dir: object) -> object: + def unreadable(snapshot: object, **_kw: object) -> object: raise _encode_error() monkeypatch.setattr(sys, "stdin", types.SimpleNamespace(buffer=stdin)) monkeypatch.setattr(sys, "stdout", types.SimpleNamespace(buffer=stdout)) monkeypatch.setattr(_sandbox_worker, "_redirect_stdout_to_stderr", lambda: None) - monkeypatch.setattr(wiring, "load_config", unreadable) + monkeypatch.setattr(wiring, "load_snapshot", unreadable) # Should main() stop reaching the patched load, these must still not touch the test process. monkeypatch.setattr(_sandbox_worker, "_apply_resource_caps", lambda mem_mb: None) monkeypatch.setattr(_sandbox_worker, "_install_import_guard", lambda forbidden: None)