Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Version

- **`scripts/verify.sh --full` runs its ordinary lane in parallel, and the compatibility matrix drops 21 rows and 57 duplicate wheel arms** ([#236](https://github.com/L3DigitalNet/project-standards/issues/236), [#227](https://github.com/L3DigitalNet/project-standards/issues/227)). The `--full` ordinary lane ran single-process for roughly 50 minutes of every release train to claim test isolation, a claim the written record of past trains never once shows catching a red ([`docs/research/2026-09-01-release-train-wall-clock.md`](docs/research/2026-09-01-release-train-wall-clock.md) §2, lever F); it now runs the identical selection at `-n 16 --dist load`, still under the default trace core, so the coverage-core cross-check against the `sysmon` fast gate that the lane also carried is unaffected. `--full` therefore gains a `coverage-combine` lane, which its single data file previously made unnecessary. In the compatibility matrix the 36 pairwise rows now run against the source distribution only — pairwise interaction between two packages' managed blocks is distribution-independent, and source/wheel parity is still proven per package by the single-package, full-set, and partial-migration rows — and the 21 legacy pairwise rows are removed as redundant with the 14 partial-migration rows and the `all-namespace-legacy` full-set row. Collected compatibility rows fall from 150 to 129. Repository tooling only: no package, payload, or consumer-visible byte changes.

- **A Python payload provider no longer inherits the caller's whole environment.** Command-kind providers already ran with an empty environment, but Python-kind children received a copy of `os.environ`, so provider bytes that got past payload integrity verification could read `GITHUB_TOKEN`, `BAO_*`, or any other secret the parent happened to hold ([#230](https://github.com/L3DigitalNet/project-standards/issues/230)). A child now receives an allowlist and nothing else: `PATH`, `PYTHONPATH` (recomputed from the parent's active `sys.path`, as before), `HOME`, `LANG`, `LC_ALL`, `LC_CTYPE`, `TMPDIR`, `PYTHONDONTWRITEBYTECODE`, and every `COVERAGE_*` variable, the last so the coverage lane keeps measuring provider children. This tightens the ADR 0025 execution boundary rather than reinterpreting it, and the ADR now records the allowlist as part of that boundary's contract. A provider that depended on an inherited variable outside the allowlist must have it passed as typed provider input instead.
- **A payload can no longer declare a group- or other-writable artifact mode.** `PosixMode` accepted any four-digit octal mode, and the executor applies a declared mode verbatim through `fchmod`, so `0777` would have shipped a managed file every local account can rewrite ([#230](https://github.com/L3DigitalNet/project-standards/issues/230)). The pattern is now `^0[0-7][0145][0145]$`: `0644`, `0700`, and `0755` remain valid and `0666`, `0775`, and `0777` are refused where the payload is authored. This is a producer-side validation tightening only — every declared artifact mode in the published catalog is `0755`, so no payload, catalog, or consumer byte changes.

## [5.28.0] — 2026-09-01

### Added
Expand Down
6 changes: 5 additions & 1 deletion docs/adr/adr-0025-mcp-service-and-sdk-boundary.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ description: 'Freezes the exact official MCP SDK dependency, the one-way adapter
doc_type: 'adr'
status: 'active'
created: '2026-07-28'
updated: '2026-08-09'
updated: '2026-09-01'
reviewed: '2026-08-09'
owner: 'Chris Purcell / L3DigitalNet'
consumer: 'mix'
Expand Down Expand Up @@ -143,6 +143,10 @@ An import-boundary contract test asserts that no module under `mcp_services` imp

An IPC-cleanup test asserts that after each of the four completion paths — success, timeout, kill, and crash — the parent holds no open worker pipe, queue, temporary file, or socket, and no child process remains unreaped; file-descriptor and child-process counts return to their pre-invocation values. A dispatch-guard test asserts that `invoke_read_provider` refuses any operation outside the approved `findings` set. Dependency resolution, the exact pin, and `uv run pip-audit` are checked in the T1 verification gate.

### Amendments

**Amended 2026-09-01 (#230, child-environment allowlist).** The worker's environment is now part of this boundary's stated contract. A Python provider child receives an allowlist and nothing else: `PATH`, `PYTHONPATH` (always recomputed from the parent's active `sys.path`, never inherited), `HOME`, `LANG`, `LC_ALL`, `LC_CTYPE`, `TMPDIR`, `PYTHONDONTWRITEBYTECODE`, and every `COVERAGE_*` variable — the last so the coverage lane keeps measuring provider children. Any other parent variable, including `GITHUB_TOKEN` and `BAO_*`, is absent in the child, so payload provider bytes cannot read a credential the parent happens to hold. This tightens the boundary and reinterprets nothing: command-kind providers already ran with an empty environment, and the Python kind now matches that posture. The allowlist is `_WORKER_ENVIRONMENT_NAMES` / `_WORKER_ENVIRONMENT_PREFIXES` in `src/project_standards/control_plane/provider_subprocess.py`; adding a name to it is an amendment to this record, because it widens what a provider can read.

## Pros and Cons of the Options

### Depend on the official SDK at an exact stable pin, isolate it, and use a bounded worker
Expand Down
40 changes: 38 additions & 2 deletions src/project_standards/control_plane/provider_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,29 @@
_PYTHON_WORKER_BOOTSTRAP = (
"from project_standards.control_plane.provider_worker import main; raise SystemExit(main())"
)
# The complete allowlist for a Python provider child, each entry load-bearing:
# PATH so the child can resolve its interpreter's neighbours, HOME because a bare
# HOME-less process breaks tooling that resolves a user directory, the LANG/LC_*
# trio so decoding matches the parent, TMPDIR so the child writes into the same
# bounded scratch the parent was given, and PYTHONDONTWRITEBYTECODE so a provider
# leaves no __pycache__ inside a payload tree. PYTHONPATH is deliberately NOT
# inherited here: `python_worker_environment` always recomputes it from the active
# `sys.path`, so an inherited stale value could only shadow the real import path.
_WORKER_ENVIRONMENT_NAMES: frozenset[str] = frozenset(
{
"PATH",
"HOME",
"LANG",
"LC_ALL",
"LC_CTYPE",
"TMPDIR",
"PYTHONDONTWRITEBYTECODE",
}
)
# COVERAGE_* (COVERAGE_PROCESS_START, COVERAGE_FILE, ...) must survive the
# allowlist or the coverage lane silently stops measuring provider children and
# the reported total drops without any test failing.
_WORKER_ENVIRONMENT_PREFIXES: tuple[str, ...] = ("COVERAGE_",)
_ABSOLUTE_PATH_PATTERN = re.compile(r"(?:^|[\s'\"(])/[\w.\-/]{4,}")
REDACTED_FAILURE_DETAIL = "the failure detail was withheld because it named a filesystem path"

Expand Down Expand Up @@ -211,8 +234,21 @@ def close(self) -> None:


def python_worker_environment() -> dict[str, str]:
"""Return the caller environment plus the exact active Python import path."""
environment = dict(os.environ)
"""Return the allowlisted child environment plus the exact active Python import path.

Only the names below reach a provider child. Everything else in the caller's
environment — `GITHUB_TOKEN`, `BAO_*`, and any other secret-bearing variable —
is absent by construction, so payload provider bytes cannot read a credential
they were never handed (issue #230, finding 1). This tightens the ADR 0025
execution boundary rather than reinterpreting it: command-kind providers
already run with `environment={}`, and this brings the Python kind to the same
posture.
"""
environment = {
name: value
for name, value in os.environ.items()
if name in _WORKER_ENVIRONMENT_NAMES or name.startswith(_WORKER_ENVIRONMENT_PREFIXES)
}
entries = [entry for entry in sys.path if entry]
if entries:
environment["PYTHONPATH"] = os.pathsep.join(entries)
Expand Down
26 changes: 24 additions & 2 deletions src/project_standards/package_contract/payload.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,19 @@ def evolve(self, **changes: object) -> _SchemaValidator: ...
str,
StringConstraints(pattern=r"^[a-z0-9][a-z0-9.+-]*/[a-z0-9][a-z0-9.+-]*$"),
]
PosixMode = Annotated[str, StringConstraints(pattern=r"^0[0-7]{3}$")]
# Four octal digits whose group and other digits carry no write bit, so `0777`,
# `0666`, and `0775` are refused while `0755`, `0644`, and `0700` remain expressible.
# `executor.py` applies a declared mode verbatim through `fchmod`, so a payload that
# declared a world-writable mode would hand every local account write access to a
# managed file (issue #230, finding 2). The leading `0` already keeps setuid, setgid,
# and sticky out of reach. Narrowing the grammar rejects no published payload: every
# declared artifact mode across the catalog is `0755`.
#
# The write-free octal digits are exactly {0, 1, 4, 5}, so the last two positions are
# an enumeration, not a range. Issue #230 spells the pattern `^0[0-7][0-57][0-57]$`,
# which is a transcription slip: `[0-57]` is the range 0-5 plus 7, so it admits 2, 3,
# and 7 and would accept the very `0777` the issue exists to reject.
PosixMode = Annotated[str, StringConstraints(pattern=r"^0[0-7][0145][0145]$")]
SharedIdentity = Annotated[
str,
StringConstraints(pattern=r"^[a-z0-9]+(?:[./_-][a-z0-9]+)*$"),
Expand Down Expand Up @@ -245,7 +257,17 @@ class WholeArtifactDeclaration(ConditionalMaterialization):
source: SafeRelativePath
digest: Sha256Digest
policy: ArtifactPolicy
mode: PosixMode | None = None
# The pattern alone reports only itself in a validation error, so the payload
# author's reason for a rejection lives here, where the generated schema carries
# it to every consumer of the contract.
mode: PosixMode | None = Field(
default=None,
description=(
"Octal file mode applied verbatim when the artifact is materialized. A "
"group- or other-write bit is refused, so 0644, 0700, and 0755 are valid "
"and 0666, 0775, and 0777 are not."
),
)


@dataclass(frozen=True, slots=True)
Expand Down
2 changes: 1 addition & 1 deletion src/project_standards/schemas/consumer-lock.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@
"mode": {
"anyOf": [
{
"pattern": "^0[0-7]{3}$",
"pattern": "^0[0-7][0145][0145]$",
"type": "string"
},
{
Expand Down
2 changes: 1 addition & 1 deletion src/project_standards/schemas/mutation-plan.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
"mode": {
"anyOf": [
{
"pattern": "^0[0-7]{3}$",
"pattern": "^0[0-7][0145][0145]$",
"type": "string"
},
{
Expand Down
6 changes: 3 additions & 3 deletions src/project_standards/schemas/reconciliation-plan.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@
"mode": {
"anyOf": [
{
"pattern": "^0[0-7]{3}$",
"pattern": "^0[0-7][0145][0145]$",
"type": "string"
},
{
Expand Down Expand Up @@ -444,7 +444,7 @@
"after_mode": {
"anyOf": [
{
"pattern": "^0[0-7]{3}$",
"pattern": "^0[0-7][0145][0145]$",
"type": "string"
},
{
Expand All @@ -469,7 +469,7 @@
"before_mode": {
"anyOf": [
{
"pattern": "^0[0-7]{3}$",
"pattern": "^0[0-7][0145][0145]$",
"type": "string"
},
{
Expand Down
3 changes: 2 additions & 1 deletion src/project_standards/schemas/standard-payload.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -956,14 +956,15 @@
"mode": {
"anyOf": [
{
"pattern": "^0[0-7]{3}$",
"pattern": "^0[0-7][0145][0145]$",
"type": "string"
},
{
"type": "null"
}
],
"default": null,
"description": "Octal file mode applied verbatim when the artifact is materialized. A group- or other-write bit is refused, so 0644, 0700, and 0755 are valid and 0666, 0775, and 0777 are not.",
"title": "Mode"
},
"policy": {
Expand Down
77 changes: 77 additions & 0 deletions tests/control_plane/test_provider_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -594,3 +594,80 @@ def test_run_provider_subprocess__timeout__terminates_descendant_group(tmp_path:
assert raised.value.code == "provider-timeout"
assert sentinel.is_file(), "the timeout expired before the fixture created its descendant"
_assert_process_gone(int(sentinel.read_text(encoding="utf-8")))


_ENVIRONMENT_SCRIPT = """
import json
import os
import sys

sys.stdin.buffer.read()
with os.fdopen(int(sys.argv[1]), "wb") as stream:
stream.write(json.dumps({"status": "ok", "result": dict(os.environ)}).encode("utf-8"))
"""


def _child_environment() -> dict[str, str]:
"""Return the environment a real provider child observes, read from inside it.

Asserting on `python_worker_environment()` alone would prove nothing about the
boundary: the spawn path is what decides what the child can read, so the fixture
reports `os.environ` back through the result descriptor.
"""
outcome = run_provider_subprocess(
_python_argv(_ENVIRONMENT_SCRIPT),
b"{}",
timeout=5.0,
environment=python_worker_environment(),
)
frame = outcome.frame
assert frame["status"] == "ok"
result = frame["result"]
assert isinstance(result, dict)
return {key: value for key, value in result.items() if isinstance(value, str)}


def test_python_provider_child__secret_bearing_variables__are_absent(
monkeypatch: pytest.MonkeyPatch,
) -> None:
# The canaries are named for the two families the H8 read called out (issue #230):
# a token the parent legitimately holds, and the OpenBao prefix. The third name
# pins the rule rather than the two examples — the allowlist is closed, so an
# arbitrary parent variable is absent too.
monkeypatch.setenv("GITHUB_TOKEN", "canary-github-token")
monkeypatch.setenv("BAO_TOKEN", "canary-bao-token")
monkeypatch.setenv("PROVIDER_CANARY_230", "canary-arbitrary")

environment = _child_environment()

assert "GITHUB_TOKEN" not in environment
assert "BAO_TOKEN" not in environment
assert "PROVIDER_CANARY_230" not in environment
assert "canary-github-token" not in "\n".join(environment.values())
# PATH proves the allowlist passes what it declares rather than emptying the
# environment wholesale, which would be a different (and breaking) contract.
assert environment["PATH"] == os.environ["PATH"]
assert environment["PYTHONPATH"] == os.pathsep.join(entry for entry in sys.path if entry)


def test_python_provider_child__coverage_variables__reach_the_child(
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
) -> None:
# Without this the coverage lane stops measuring provider children and the total
# falls with no test failing, so the allowlist's COVERAGE_ prefix is pinned here.
#
# Both files are real and inside tmp_path on purpose: coverage 7.10+ ships
# `a1_coverage.pth`, which auto-starts coverage in any child that sees
# COVERAGE_PROCESS_START, so a pointer to a missing config would abort the child
# here and a shared COVERAGE_FILE would let the fixture write into the gate's
# own data file.
config = tmp_path / "coveragerc"
config.write_text("[run]\n", encoding="utf-8")
data_file = tmp_path / ".coverage"
monkeypatch.setenv("COVERAGE_PROCESS_START", str(config))
monkeypatch.setenv("COVERAGE_FILE", str(data_file))

environment = _child_environment()

assert environment["COVERAGE_PROCESS_START"] == str(config)
assert environment["COVERAGE_FILE"] == str(data_file)
34 changes: 34 additions & 0 deletions tests/package_contract/test_payload_outputs.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import pytest
from pydantic import ValidationError

from project_standards.package_contract.diagnostics import validation_summary
from project_standards.package_contract.payload import (
AdapterKind,
ArtifactPolicy,
Expand Down Expand Up @@ -376,3 +377,36 @@ def test_payload_rejects_ambiguous_duplicate_required_resource_roles() -> None:

with pytest.raises(ValidationError, match="exactly one"):
PayloadManifest.model_validate(data)


def _artifact(mode: str) -> dict[str, object]:
return {
"id": "python-version",
"target": ".python-version",
"source": "artifacts/python-version",
"digest": f"sha256:{'a' * 64}",
"policy": "create-only",
"mode": mode,
}


@pytest.mark.parametrize("mode", ["0755", "0644", "0700"])
def test_whole_artifact_mode_accepts_modes_without_group_or_other_write(mode: str) -> None:
assert WholeArtifactDeclaration.model_validate(_artifact(mode)).mode == mode


@pytest.mark.parametrize("mode", ["0777", "0666", "0775"])
def test_whole_artifact_mode_refuses_group_or_other_write(mode: str) -> None:
# `executor.py` applies a declared mode verbatim through `fchmod`, so admitting
# one of these would ship a managed file every local account can rewrite
# (issue #230, finding 2).
with pytest.raises(ValidationError) as raised:
WholeArtifactDeclaration.model_validate(_artifact(mode))

summary = validation_summary(raised.value)

assert summary.startswith("mode: ")
assert "^0[0-7][0145][0145]$" in summary
# The diagnostic path never echoes a payload's own bytes back to the operator,
# and the refused mode is payload-controlled input like any other field.
assert mode not in summary
Loading