diff --git a/SCOPE.md b/SCOPE.md index c7dc4b7..09e00ba 100644 --- a/SCOPE.md +++ b/SCOPE.md @@ -28,7 +28,7 @@ decision gate, in plain terms; (4) build, only after approval. - Deciding *whether* access should exist — that's architecture/founder, ops-mason builds what's already decided -- Runtime authorization decisions — flex-auth +- Runtime authorization decisions — access-engine - Identity/MFA — key-cape/Keycloak - Routing consumers to lanes once built — ops-warden - SSH certificate issuance — ops-warden @@ -38,9 +38,28 @@ decision gate, in plain terms; (4) build, only after approval. ## Current State -Charter only (`INTENT.md`). `MASON-WP-0001` scopes the four-phase -pipeline and its first real exercise: the `rein-openweights` OpenBao -AppRole that `glas-harness/GLAS-WP-0002-T02` is blocked on. No code yet. +The four-phase construction process, OpenBao AppRole/Kubernetes-auth builders, +metadata inventory, and guarded Kubernetes-plane CLI are implemented and tested. +Construction plans and build evidence live in `plans/` and `docs/evidence/`. + +OpenBao builds require an approved plan whose machine-readable specification +and approved digest match the supplied execution inputs. Existing policies are +content-pinned before reuse. AppRole credentials are delivered through private, +exclusive files; the builders do not read downstream KV values. See +`docs/construction-plan-format.md` for review and execution requirements. + +Kubernetes apply enforces pinned readiness tiers, the explicit whitehat +placement, the dated policy-nexus transition, and recorded emergency activation. +See `docs/kubernetes-plane.md`. + +The layer declaration is `Staff`, with PEP-shaped behavior, in `INTENT.md`. +The founder-approved plan-approval exception and direct-contact gaps remain +explicitly declared; this is not a claim of full layer-model conformance. + +Remaining work is held in existing blocked workplans: credential descriptions +(MASON-WP-0004), the attended Telegram credential lane (MASON-WP-0005), and the +2026-12-21 readiness/approval review (MASON-WP-0006-T06). `WORK-RECORDS.md` is +the generated index; workplan files remain the source of task status. ## Getting Oriented diff --git a/docs/construction-plan-format.md b/docs/construction-plan-format.md index bde70fa..f45844c 100644 --- a/docs/construction-plan-format.md +++ b/docs/construction-plan-format.md @@ -95,3 +95,93 @@ section does not exist until phase 4 has run. See `plans/rein-openweights-openrouter-approle.md` once T01 is implemented against this format — the first real plan, not a synthetic one, per `MASON-WP-0001`'s own scoping. + +## Binding OpenBao approval to execution inputs + +The AppRole and Kubernetes-auth builders additionally require two frontmatter +fields: `build_spec` and `approved_spec_sha256`. `build_spec` is the complete +value-free output of `ops_mason.executor.build_spec_document(spec)`. It includes +the engine, exact policy/role/path and identity bindings, token bounds, policy +reuse and content pin, delivery destination, audit destination, and executable. +`credential_type` must match its `engine`. + +During drafting, render the specification and its digest for review: + +```python +from pathlib import Path +from ops_mason.executor import AppRoleKVSpec, build_spec_document, build_spec_digest + +spec = AppRoleKVSpec( + policy_name="workload-kv-read-example", + kv_path="platform/workloads/example/runtime", + approle_name="example", + token_num_uses=8, + delivery_dir=Path("/home/consumer/.local/example/approle"), + audit_log_path=Path("/home/builder/ops-mason/audit/build-log.jsonl"), +) +document = build_spec_document(spec) # put this mapping under build_spec +candidate_digest = build_spec_digest(document) # present alongside the review +``` + +The digest is SHA-256 of UTF-8 JSON with sorted keys, compact separators and +no NaN values. Paths render as absolute strings; tuples render as lists. All +specification fields, including explicit defaults and null values, participate. +After the founder approves that exact specification, record the digest as +`approved_spec_sha256` alongside `approved_by`, `approved_at` and +`status: approved`. Quote approval dates in YAML. A candidate digest alone +is not approval, and the digest is an integrity marker, not a signature. + +Before any OpenBao command, the builder reloads the plan from disk, checks +approval and engine, validates the specification, and compares both the +frontmatter mapping and approved digest to the actual inputs. Duplicate YAML +keys, missing build specifications, changed scope or changed destinations are +refused. Editing the mapping without renewing its approval digest cannot reuse +the previous approval. The plan file remains the trust boundary: this does not +protect against someone authorized to rewrite the approval record itself. + +Historical plans stay historical. They are not automatically given new +specifications or approvals. Before re-executing one, draft and review the exact +new inputs through the existing four-phase process. No live lane was changed +by this format migration. + +### Input and policy constraints + +The existing AppRole engine creates only exact KV-v2 read lanes: one literal +entry path, `read` on data and metadata, no wildcard, empty/parent path segment, +HCL injection, or write capability. OIDC matrix writers remain a separate, +blocked contract under MASON-WP-0005. Role and policy names must be single +literal names; Kubernetes service-account names/namespaces must be explicit, +nonempty, unique DNS labels. Token use counts must be explicit nonnegative +integers (zero is deliberately unlimited when approved); TTLs are positive +integer durations in `s`, `m` or `h`, with maximum TTL at least initial TTL. +`secret_id_ttl: "0"` remains an explicit reviewable choice. + +For `AppRoleKVSpec(reuse_policy=True)`, also provide +`reuse_policy_sha256`. For `KubernetesKVSpec`, provide `policy_sha256`, since +that builder always binds an existing policy. During the scoped structure +survey, hash the exact UTF-8 output of `bao policy read `, including its +trailing newline, and review that policy's full scope. The builder reads only +policy text, checks its hash before auth-role writes, and refuses drift. A +reused policy may deliberately cover more than the AppRole's nominal KV path; +its reviewed contents, not that path field alone, define the grant. Rechecking +a pin is not an atomic lock against later administrative policy changes. + +### Credential delivery and failed builds + +AppRole delivery requires an absolute directory. Every directory component is +opened without following symlinks; missing directories are created privately. +An existing final directory must belong to the executing account with mode +0700. Both credential files are reserved exclusively at mode 0600 before any +policy/auth write or credential issuance. Existing files, hardlinks and symlink +paths are refused rather than overwritten or permission-repaired. Delivery +uses the held file descriptors and flushes credentials to disk. + +OpenBao error and timeout output is suppressed so a failed credential command +cannot echo sensitive material. Audit output contains object names, the +approved specification digest and approval attribution, never credentials. +A failed build can leave private empty or partially delivered files, and may +have changed policy/auth structure or issued a credential before failing. +Do not blindly retry or delete them: inspect structure and audit metadata, +resolve/revoke any partial issuance through the scoped custody procedure, and +review a fresh delivery destination before retrying. The executor does not +claim transactional rollback or automatically widen its revocation authority. diff --git a/intakes/intakes.md b/intakes/intakes.md index 2e267fe..6fa5ed4 100644 --- a/intakes/intakes.md +++ b/intakes/intakes.md @@ -36,7 +36,8 @@ id: MASON-IN-0002 kind: intake title: 'Declaration requested: state this repository''s layer in INTENT.md (security layer model §11)' -status: open +status: closed +outcome: absorbed origin: cross-repo origin_ref: net-kingdom security-layer-model_v0.4 §11 priority: low @@ -61,5 +62,12 @@ description: 'A conformance sweep on 2026-08-28 found this repository has no lay If the proposed layer is wrong for what this repository actually does, that is more useful to us than a label added to close a checkbox. Standard: net-kingdom/canon/standards/security-layer-model_v0.4.md.' created: '2026-08-28T21:03:29.920563Z' -updated: '2026-08-28T21:03:29.920563Z' +updated: '2026-09-28T09:54:34.065401Z' +notes: >- + Closed against the repository-authored Staff / pep-shaped declaration in + INTENT.md, dated 2026-09-21. That declaration names access-engine as the + authorization decision point and explicitly records direct Tooling contacts + and the founder-accepted plan-approval exception. The declaration request is + fulfilled; the accepted conformance gaps and their 2026-12-21 review remain + declared and are not marked conformant by this intake closure. ``` diff --git a/src/ops_mason/executor.py b/src/ops_mason/executor.py index 6f1208f..3ba2eb8 100644 --- a/src/ops_mason/executor.py +++ b/src/ops_mason/executor.py @@ -18,13 +18,18 @@ printable form beyond confirmation that delivery happened. from __future__ import annotations +import hashlib +import json import os +import re +import stat import subprocess -from dataclasses import dataclass, field +from contextlib import contextmanager +from dataclasses import asdict, dataclass, replace from pathlib import Path from ops_mason.audit import record_build -from ops_mason.plan import ConstructionPlan +from ops_mason.plan import ConstructionPlan, PlanError class BuildRefused(RuntimeError): @@ -56,6 +61,7 @@ class AppRoleKVSpec: # When True the named policy already exists (and may grant more than one # KV path). Do not rewrite it — AppRole bind only. reuse_policy: bool = False + reuse_policy_sha256: str | None = None @dataclass @@ -64,21 +70,21 @@ class KubernetesKVSpec: role_name: str service_account_names: tuple[str, ...] service_account_namespaces: tuple[str, ...] + policy_sha256: str = "" token_ttl: str = "15m" audit_log_path: Path | None = None bao_bin: str = "bao" def _run(bao_bin: str, args: list[str], input_text: str | None = None) -> str: - proc = subprocess.run( - [bao_bin, *args], - input=input_text, - capture_output=True, - text=True, - timeout=30, - ) + try: + proc = subprocess.run( + [bao_bin, *args], input=input_text, capture_output=True, text=True, timeout=30, + ) + except (OSError, UnicodeError, subprocess.TimeoutExpired): + raise BuildError("OpenBao command unavailable or timed out; output suppressed") from None if proc.returncode != 0: - raise BuildError(f"`{bao_bin} {' '.join(args)}` failed: {proc.stderr.strip()[:300]}") + raise BuildError(f"OpenBao command failed (exit {proc.returncode}); output suppressed") return proc.stdout @@ -93,6 +99,12 @@ def _policy_hcl(kv_path: str, capabilities: tuple[str, ...]) -> str: OpenBao evaluates policy against the real `data/`-prefixed path, not the one a caller might naively check capabilities against. """ + if not isinstance(kv_path, str) or len(kv_path.split("/")) < 2: + raise BuildRefused("KV path must contain a mount and a literal entry") + for part in kv_path.split("/"): + _name(part, "KV path segment") + if not isinstance(capabilities, tuple) or capabilities != ("read",): + raise BuildRefused("this KV read-lane engine permits only the read capability") mount, _, rest = kv_path.partition("/") caps = ", ".join(f'"{c}"' for c in capabilities) return ( @@ -101,57 +113,214 @@ def _policy_hcl(kv_path: str, capabilities: tuple[str, ...]) -> str: ) +def _name(value: str, label: str) -> None: + if not isinstance(value, str) or not re.fullmatch(r"[a-zA-Z0-9][a-zA-Z0-9_.-]*", value): + raise BuildRefused(f"{label} must be one literal name, without wildcards or separators") + + +def _duration(value: str, label: str, *, allow_zero: bool = False) -> int: + if allow_zero and value == "0": + return 0 + if not isinstance(value, str) or not re.fullmatch(r"[1-9][0-9]*[smh]", value): + raise BuildRefused(f"{label} must be a positive duration in s, m or h") + return int(value[:-1]) * {"s": 1, "m": 60, "h": 3600}[value[-1]] + + +def _hash(value: str) -> None: + if not isinstance(value, str) or not re.fullmatch(r"[0-9a-f]{64}", value): + raise BuildRefused("reused policy requires an exact SHA-256 content pin") + + +def _validate_spec(spec: AppRoleKVSpec | KubernetesKVSpec) -> None: + _name(spec.policy_name, "policy name") + _duration(spec.token_ttl, "token_ttl") + if not isinstance(spec.bao_bin, str) or not spec.bao_bin.strip(): + raise BuildRefused("bao_bin must name an executable") + if spec.audit_log_path is not None and not isinstance(spec.audit_log_path, Path): + raise BuildRefused("audit_log_path must be a Path") + if isinstance(spec, AppRoleKVSpec): + _name(spec.approle_name, "AppRole name") + _policy_hcl(spec.kv_path, spec.kv_capabilities) + if type(spec.token_num_uses) is not int or spec.token_num_uses < 0: + raise BuildRefused("token_num_uses must be an explicit nonnegative integer") + if _duration(spec.token_max_ttl, "token_max_ttl") < _duration(spec.token_ttl, "token_ttl"): + raise BuildRefused("token_max_ttl must not be shorter than token_ttl") + _duration(spec.secret_id_ttl, "secret_id_ttl", allow_zero=True) + if type(spec.reuse_policy) is not bool: + raise BuildRefused("reuse_policy must be boolean") + if spec.reuse_policy: + _hash(spec.reuse_policy_sha256) + elif spec.reuse_policy_sha256 is not None: + raise BuildRefused("policy pin is only used with reuse_policy") + if not isinstance(spec.delivery_dir, Path) or not spec.delivery_dir.is_absolute(): + raise BuildRefused("credential delivery requires an explicit absolute directory") + if ".." in spec.delivery_dir.parts: + raise BuildRefused("credential delivery path must not contain parent traversal") + if spec.audit_log_path is not None and spec.audit_log_path.absolute() in { + spec.delivery_dir, spec.delivery_dir / "role_id", spec.delivery_dir / "secret_id" + }: + raise BuildRefused("audit path must not overlap credential delivery") + else: + _name(spec.role_name, "Kubernetes role name") + _hash(spec.policy_sha256) + for values in (spec.service_account_names, spec.service_account_namespaces): + if not isinstance(values, tuple) or not values: + raise BuildRefused("Kubernetes role requires explicit service account bindings") + for value in values: + if not isinstance(value, str) or not re.fullmatch(r"[a-z0-9](?:[a-z0-9-]*[a-z0-9])?", value) or len(value) > 63: + raise BuildRefused("service account bindings must be literal DNS labels") + if len(set(values)) != len(values): + raise BuildRefused("duplicate service account binding") + + +def build_spec_document(spec: AppRoleKVSpec | KubernetesKVSpec) -> dict: + """Render the complete, value-free execution input for phase-3 review.""" + _validate_spec(spec) + fields = asdict(spec) + for key, value in fields.items(): + if isinstance(value, Path): + fields[key] = str(value.absolute()) + elif isinstance(value, tuple): + fields[key] = list(value) + return { + "engine": "openbao-approle-kv" if isinstance(spec, AppRoleKVSpec) else "openbao-kubernetes-kv", + **fields, + } + + +def build_spec_digest(document: dict) -> str: + """Hash canonical JSON; this is an integrity marker, not a signature.""" + try: + encoded = json.dumps(document, sort_keys=True, separators=(",", ":"), allow_nan=False).encode() + except (TypeError, ValueError) as exc: + raise BuildRefused("build specification is not canonical JSON") from exc + return hashlib.sha256(encoded).hexdigest() + + +def _approved_build(plan: ConstructionPlan, spec) -> ConstructionPlan: + # The file is authoritative, so revoking approval after load takes effect. + try: + current = ConstructionPlan.load(plan.path) + except (OSError, PlanError) as exc: + raise BuildRefused("cannot reload construction-plan approval") from exc + if current.id != plan.id or not current.is_approved(): + raise BuildRefused("construction plan is not approved; refusing to build") + document = build_spec_document(spec) + if current.credential_type != document["engine"]: + raise BuildRefused("credential_type does not match the build engine") + if not isinstance(current.build_spec, dict) or not current.approved_spec_sha256: + raise BuildRefused("plan requires build_spec and approved_spec_sha256") + expected = build_spec_digest(document) + if build_spec_digest(current.build_spec) != expected or current.approved_spec_sha256 != expected: + raise BuildRefused("build specification does not match the approved specification digest") + return current + + +def _verify_policy(bao_bin: str, name: str, expected: str) -> None: + content = _run(bao_bin, ["policy", "read", name]) + if hashlib.sha256(content.encode()).hexdigest() != expected: + raise BuildRefused("live reused policy does not match the approved content pin") + + +@contextmanager +def _delivery_files(path: Path): + """Reserve private files before Bao writes, without following any symlink. + + Hold directory/file descriptors throughout delivery. Existing files are + never opened or overwritten. Failed issuance leaves private partial files + for explicit recovery; retries must use a newly reviewed destination. + """ + directory = os.open("/", os.O_RDONLY | os.O_DIRECTORY) + files = {} + try: + for component in path.parts[1:]: + try: + os.mkdir(component, 0o700, dir_fd=directory) + except FileExistsError: + pass + child = os.open(component, os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW, dir_fd=directory) + os.close(directory) + directory = child + info = os.fstat(directory) + if info.st_uid != os.geteuid() or stat.S_IMODE(info.st_mode) != 0o700: + raise BuildRefused("delivery directory must be owned by the caller with mode 0700") + # Check both first so a pre-existing destination never causes a write. + for name in ("role_id", "secret_id"): + try: + os.stat(name, dir_fd=directory, follow_symlinks=False) + except FileNotFoundError: + continue + raise BuildRefused("credential destination already exists; refusing overwrite") + for name in ("role_id", "secret_id"): + files[name] = os.open(name, os.O_WRONLY | os.O_CREAT | os.O_EXCL | os.O_NOFOLLOW, + 0o600, dir_fd=directory) + os.fchmod(files[name], 0o600) + except (OSError, BuildRefused) as exc: + for fd in files.values(): + os.close(fd) + os.close(directory) + if isinstance(exc, BuildRefused): + raise + raise BuildRefused("cannot reserve safe credential delivery files") from None + try: + yield files + finally: + for fd in files.values(): + os.close(fd) + os.close(directory) + + +def _write_credential(fd: int, value: str) -> None: + if not value or "\n" in value or "\r" in value or "\x00" in value: + raise BuildError("credential command returned an invalid response") + with os.fdopen(os.dup(fd), "w") as stream: + stream.write(value + "\n") + stream.flush() + os.fsync(stream.fileno()) + + def build_approle_kv_lane(plan: ConstructionPlan, spec: AppRoleKVSpec) -> dict[str, str]: """Create the policy + AppRole for an `openbao-approle-kv` plan, deliver role_id/secret_id. Refuses unless plan.is_approved(). Returns object names only (no secret material) and appends a metadata-only audit record. """ - if not plan.is_approved(): - raise BuildRefused( - f"plan {plan.id!r} is not approved " - f"(status={plan.status!r}, approved_by={plan.approved_by!r}, " - f"approved_at={plan.approved_at!r}) — refusing to build" + spec = replace(spec) + plan = _approved_build(plan, spec) + if spec.reuse_policy: + _verify_policy(spec.bao_bin, spec.policy_name, spec.reuse_policy_sha256) + with _delivery_files(spec.delivery_dir) as files: + if not spec.reuse_policy: + policy_hcl = _policy_hcl(spec.kv_path, spec.kv_capabilities) + _run(spec.bao_bin, ["policy", "write", spec.policy_name, "-"], input_text=policy_hcl) + + _run( + spec.bao_bin, + [ + "write", + f"auth/approle/role/{spec.approle_name}", + f"token_policies={spec.policy_name}", + f"token_ttl={spec.token_ttl}", + f"token_max_ttl={spec.token_max_ttl}", + f"token_num_uses={spec.token_num_uses}", + f"secret_id_ttl={spec.secret_id_ttl}", + ], ) - if not spec.reuse_policy: - policy_hcl = _policy_hcl(spec.kv_path, spec.kv_capabilities) - _run(spec.bao_bin, ["policy", "write", spec.policy_name, "-"], input_text=policy_hcl) + role_id = _run( + spec.bao_bin, ["read", "-field=role_id", f"auth/approle/role/{spec.approle_name}/role-id"] + ).strip() + secret_id = _run( + spec.bao_bin, + ["write", "-field=secret_id", "-f", f"auth/approle/role/{spec.approle_name}/secret-id"], + ).strip() - _run( - spec.bao_bin, - [ - "write", - f"auth/approle/role/{spec.approle_name}", - f"token_policies={spec.policy_name}", - f"token_ttl={spec.token_ttl}", - f"token_max_ttl={spec.token_max_ttl}", - f"token_num_uses={spec.token_num_uses}", - f"secret_id_ttl={spec.secret_id_ttl}", - ], - ) - - role_id = _run( - spec.bao_bin, ["read", "-field=role_id", f"auth/approle/role/{spec.approle_name}/role-id"] - ).strip() - secret_id = _run( - spec.bao_bin, - ["write", "-field=secret_id", "-f", f"auth/approle/role/{spec.approle_name}/secret-id"], - ).strip() - - delivered_to = "" - if spec.delivery_dir is not None: - spec.delivery_dir.mkdir(parents=True, exist_ok=True) - os.chmod(spec.delivery_dir, 0o700) - role_id_path = spec.delivery_dir / "role_id" - secret_id_path = spec.delivery_dir / "secret_id" - role_id_path.write_text(role_id + "\n") - secret_id_path.write_text(secret_id + "\n") - os.chmod(role_id_path, 0o600) - os.chmod(secret_id_path, 0o600) - delivered_to = str(spec.delivery_dir) + _write_credential(files["role_id"], role_id) + _write_credential(files["secret_id"], secret_id) + delivered_to = str(spec.delivery_dir) objects = { + "approved_spec_sha256": plan.approved_spec_sha256, "policy_name": spec.policy_name, "approle_name": spec.approle_name, "kv_path": spec.kv_path, @@ -172,14 +341,9 @@ def build_kubernetes_kv_lane( plan: ConstructionPlan, spec: KubernetesKVSpec ) -> dict[str, str]: """Create a policy-bound Kubernetes auth role without handling secret values.""" - if not plan.is_approved(): - raise BuildRefused( - f"plan {plan.id!r} is not approved " - f"(status={plan.status!r}, approved_by={plan.approved_by!r}, " - f"approved_at={plan.approved_at!r}) — refusing to build" - ) - if not spec.service_account_names or not spec.service_account_namespaces: - raise BuildRefused("Kubernetes role requires explicit service account bindings") + spec = replace(spec) + plan = _approved_build(plan, spec) + _verify_policy(spec.bao_bin, spec.policy_name, spec.policy_sha256) _run( spec.bao_bin, @@ -193,6 +357,7 @@ def build_kubernetes_kv_lane( ], ) objects = { + "approved_spec_sha256": plan.approved_spec_sha256, "policy_name": spec.policy_name, "kubernetes_role_name": spec.role_name, "service_account_names": ",".join(spec.service_account_names), diff --git a/src/ops_mason/plan.py b/src/ops_mason/plan.py index 83ad6b9..72f4229 100644 --- a/src/ops_mason/plan.py +++ b/src/ops_mason/plan.py @@ -19,6 +19,23 @@ class PlanError(RuntimeError): pass +class _PlanLoader(yaml.SafeLoader): + """Reject ambiguous approval documents rather than keeping the last key.""" + + +def _unique_mapping(loader, node, deep=False): + result = {} + for key_node, value_node in node.value: + key = loader.construct_object(key_node, deep=deep) + if not isinstance(key, str) or key in result: + raise PlanError("plan mappings require unique string keys") + result[key] = loader.construct_object(value_node, deep=deep) + return result + + +_PlanLoader.add_constructor(yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG, _unique_mapping) + + @dataclass class ConstructionPlan: id: str @@ -29,15 +46,26 @@ class ConstructionPlan: consumer_repo: str credential_type: str path: Path + build_spec: dict[str, Any] | None = None + approved_spec_sha256: str | None = None @classmethod def load(cls, path: str | Path) -> ConstructionPlan: path = Path(path) text = path.read_text() - parts = text.split("---", 2) - if len(parts) < 3 or not text.startswith("---"): + lines = text.splitlines() + if not lines or lines[0] != "---": raise PlanError(f"{path}: missing YAML frontmatter") - data: dict[str, Any] = yaml.safe_load(parts[1]) or {} + try: + end = lines.index("---", 1) + except ValueError: + raise PlanError(f"{path}: missing YAML frontmatter delimiter") from None + try: + data = yaml.load("\n".join(lines[1:end]), Loader=_PlanLoader) + except yaml.YAMLError as exc: + raise PlanError(f"{path}: invalid YAML frontmatter") from exc + if not isinstance(data, dict): + raise PlanError(f"{path}: frontmatter must be a mapping") missing = {"id", "status"} - set(data) if missing: raise PlanError(f"{path}: frontmatter missing field(s): {', '.join(sorted(missing))}") @@ -50,7 +78,12 @@ class ConstructionPlan: consumer_repo=data.get("consumer_repo", ""), credential_type=data.get("credential_type", ""), path=path, + build_spec=data.get("build_spec"), + approved_spec_sha256=data.get("approved_spec_sha256"), ) def is_approved(self) -> bool: - return self.status == "approved" and bool(self.approved_by) and bool(self.approved_at) + return self.status == "approved" and all( + isinstance(value, str) and bool(value.strip()) + for value in (self.approved_by, self.approved_at) + ) diff --git a/tests/test_executor.py b/tests/test_executor.py index 8cd09d5..92a3a91 100644 --- a/tests/test_executor.py +++ b/tests/test_executor.py @@ -1,4 +1,10 @@ +import hashlib import json +import os +import stat +import subprocess + +import yaml from unittest.mock import MagicMock, patch import pytest @@ -9,6 +15,8 @@ from ops_mason.executor import ( BuildRefused, KubernetesKVSpec, _policy_hcl, + build_spec_document, + build_spec_digest, build_approle_kv_lane, build_kubernetes_kv_lane, ) @@ -23,18 +31,18 @@ def test_policy_hcl_uses_kv_v2_data_and_metadata_paths() -> None: from ops_mason.plan import ConstructionPlan -def _plan(tmp_path, *, status="approved", approved_by="bernd", approved_at="2026-07-27"): - text = ( - "---\n" - "id: test-lane\n" - f"status: {status}\n" - + (f'approved_by: "{approved_by}"\n' if approved_by else "approved_by: null\n") - + (f'approved_at: "{approved_at}"\n' if approved_at else "approved_at: null\n") - + "---\n# Plan\n" - ) +POLICY = 'path "reins/data/test/openrouter" { capabilities = ["read"] }\n' +POLICY_HASH = hashlib.sha256(POLICY.encode()).hexdigest() + + +def _plan(tmp_path, *, status="approved", approved_by="bernd", approved_at="2026-07-27", spec=None): + document = build_spec_document(spec or _spec(tmp_path)) + fields = dict(id="test-lane", status=status, approved_by=approved_by, + approved_at=approved_at, credential_type=document["engine"], + build_spec=document, approved_spec_sha256=build_spec_digest(document)) p = tmp_path / "plan.md" - p.write_text(text) - return ConstructionPlan.load(str(p)) + p.write_text("---\n" + yaml.safe_dump(fields) + "---\n# Plan\n") + return ConstructionPlan.load(p) def _spec(tmp_path) -> AppRoleKVSpec: @@ -81,7 +89,9 @@ def test_build_writes_policy_approle_and_delivers_credentials(tmp_path) -> None: def fake_run(cmd, input=None, capture_output=True, text=True, timeout=30): result = MagicMock(returncode=0, stderr="") - if cmd[1:3] == ["read", "-field=role_id"]: + if cmd[1:3] == ["policy", "read"]: + result.stdout = POLICY + elif cmd[1:3] == ["read", "-field=role_id"]: result.stdout = "role-id-value\n" elif "-field=secret_id" in cmd: result.stdout = "secret-id-value\n" @@ -121,10 +131,14 @@ def test_reuse_policy_does_not_rewrite_existing_policy(tmp_path) -> None: plan = _plan(tmp_path) spec = _spec(tmp_path) spec.reuse_policy = True + spec.reuse_policy_sha256 = POLICY_HASH + plan = _plan(tmp_path, spec=spec) def fake_run(cmd, input=None, capture_output=True, text=True, timeout=30): result = MagicMock(returncode=0, stderr="") - if cmd[1:3] == ["read", "-field=role_id"]: + if cmd[1:3] == ["policy", "read"]: + result.stdout = POLICY + elif cmd[1:3] == ["read", "-field=role_id"]: result.stdout = "role-id-value\n" elif "-field=secret_id" in cmd: result.stdout = "secret-id-value\n" @@ -136,7 +150,7 @@ def test_reuse_policy_does_not_rewrite_existing_policy(tmp_path) -> None: build_approle_kv_lane(plan, spec) bao_cmds = [c.args[0] for c in run.call_args_list] - assert not any(cmd[:2] == ["bao", "policy"] for cmd in bao_cmds) + assert not any(cmd[:3] == ["bao", "policy", "write"] for cmd in bao_cmds) assert any(cmd[1:3] == ["write", "auth/approle/role/test-lane"] for cmd in bao_cmds) @@ -169,7 +183,7 @@ def test_bao_failure_raises_build_error(tmp_path) -> None: fake_result = MagicMock(returncode=1, stderr="permission denied", stdout="") with patch("ops_mason.executor.subprocess.run", return_value=fake_result): - with pytest.raises(BuildError, match="permission denied"): + with pytest.raises(BuildError, match="output suppressed"): build_approle_kv_lane(plan, spec) @@ -177,6 +191,7 @@ def _kubernetes_spec(tmp_path) -> KubernetesKVSpec: return KubernetesKVSpec( policy_name="workload-kv-read-binky-qonto-api", role_name="external-secrets-rapp-qonto", + policy_sha256=POLICY_HASH, service_account_names=("external-secrets",), service_account_namespaces=("external-secrets",), audit_log_path=tmp_path / "audit.jsonl", @@ -194,7 +209,8 @@ def test_kubernetes_lane_refusal_never_calls_bao(tmp_path) -> None: def test_kubernetes_lane_builds_exact_service_account_binding(tmp_path) -> None: plan = _plan(tmp_path) spec = _kubernetes_spec(tmp_path) - result = MagicMock(returncode=0, stderr="", stdout="") + plan = _plan(tmp_path, spec=spec) + result = MagicMock(returncode=0, stderr="", stdout=POLICY) with ( patch("ops_mason.executor.subprocess.run", return_value=result) as run, patch("ops_mason.executor.record_build") as audit, @@ -220,6 +236,7 @@ def test_kubernetes_lane_requires_nonempty_bindings(tmp_path) -> None: spec = KubernetesKVSpec( policy_name="policy", role_name="role", + policy_sha256=POLICY_HASH, service_account_names=(), service_account_namespaces=("external-secrets",), ) @@ -227,3 +244,176 @@ def test_kubernetes_lane_requires_nonempty_bindings(tmp_path) -> None: with pytest.raises(BuildRefused, match="explicit service account"): build_kubernetes_kv_lane(plan, spec) run.assert_not_called() + + +def _edit_plan(plan, edit): + text = plan.path.read_text().split("---", 2) + data = yaml.safe_load(text[1]) + edit(data) + plan.path.write_text("---\n" + yaml.safe_dump(data) + "---" + text[2]) + + +@pytest.mark.parametrize("field,value", [ + ("kv_path", "reins/another/openrouter"), ("policy_name", "other-policy"), + ("approle_name", "other-role"), ("token_num_uses", 0), + ("token_ttl", "10m"), ("token_max_ttl", "1h"), ("secret_id_ttl", "15m"), + ("bao_bin", "another-bao"), +]) +def test_changed_build_scope_refused_before_bao(tmp_path, field, value): + spec = _spec(tmp_path) + plan = _plan(tmp_path, spec=spec) + setattr(spec, field, value) + with patch("ops_mason.executor.subprocess.run") as run: + with pytest.raises(BuildRefused, match="approved specification"): + build_approle_kv_lane(plan, spec) + run.assert_not_called() + assert not spec.delivery_dir.exists() + + +def test_destination_is_part_of_approval(tmp_path): + plan = _plan(tmp_path) + spec = _spec(tmp_path) + spec.delivery_dir = tmp_path / "elsewhere" + with patch("ops_mason.executor.subprocess.run") as run: + with pytest.raises(BuildRefused, match="approved specification"): + build_approle_kv_lane(plan, spec) + run.assert_not_called() + + +@pytest.mark.parametrize("edit", [ + lambda d: d.update(status="reviewed"), + lambda d: d.update(approved_spec_sha256="0" * 64), + lambda d: d.pop("build_spec"), + lambda d: d.update(credential_type="openbao-kubernetes-kv"), + lambda d: d["build_spec"].update(token_num_uses=0), +]) +def test_changed_or_legacy_plan_refused_even_with_previously_loaded_approval(tmp_path, edit): + plan = _plan(tmp_path) + _edit_plan(plan, edit) + with patch("ops_mason.executor.subprocess.run") as run: + with pytest.raises(BuildRefused): + build_approle_kv_lane(plan, _spec(tmp_path)) + run.assert_not_called() + + +@pytest.mark.parametrize("field,value", [ + ("kv_path", "reins/test/*"), ("kv_path", "reins/test/+"), + ("kv_path", "reins/test/"), ("kv_path", "reins/../test"), + ("kv_path", 'reins/test/" { capabilities = ["sudo"] }'), + ("kv_path", "reins//test"), ("kv_path", "reins"), + ("policy_name", "a,b"), ("approle_name", "../other"), + ("kv_capabilities", ("read", "sudo")), ("kv_capabilities", ("create", "read")), + ("token_num_uses", True), ("token_num_uses", -1), + ("token_ttl", "0"), ("token_max_ttl", "1s"), + ("delivery_dir", None), ("reuse_policy", "false"), +]) +def test_unsafe_spec_is_rejected_before_any_command(tmp_path, field, value): + plan = _plan(tmp_path) + spec = _spec(tmp_path) + setattr(spec, field, value) + with patch("ops_mason.executor.subprocess.run") as run: + with pytest.raises(BuildRefused): + build_approle_kv_lane(plan, spec) + run.assert_not_called() + + +@pytest.mark.parametrize("bindings", [("*",), ("a,b",), ("../a",), ("valid", "valid")]) +def test_kubernetes_wildcard_or_ambiguous_bindings_refused(tmp_path, bindings): + spec = _kubernetes_spec(tmp_path) + plan = _plan(tmp_path, spec=spec) + spec.service_account_names = bindings + with patch("ops_mason.executor.subprocess.run") as run: + with pytest.raises(BuildRefused): + build_kubernetes_kv_lane(plan, spec) + run.assert_not_called() + + +def test_kubernetes_valid_but_unapproved_binding_refused(tmp_path): + spec = _kubernetes_spec(tmp_path) + plan = _plan(tmp_path, spec=spec) + spec.service_account_namespaces = ("other",) + with patch("ops_mason.executor.subprocess.run") as run: + with pytest.raises(BuildRefused, match="approved specification"): + build_kubernetes_kv_lane(plan, spec) + run.assert_not_called() + + +@pytest.mark.parametrize("engine", ["approle", "kubernetes"]) +def test_changed_reused_policy_refuses_before_mutation(tmp_path, engine): + if engine == "approle": + spec = _spec(tmp_path) + spec.reuse_policy, spec.reuse_policy_sha256 = True, POLICY_HASH + build = build_approle_kv_lane + else: + spec = _kubernetes_spec(tmp_path) + build = build_kubernetes_kv_lane + plan = _plan(tmp_path, spec=spec) + with patch("ops_mason.executor.subprocess.run", return_value=MagicMock(returncode=0, stdout=POLICY + "# changed")) as run: + with pytest.raises(BuildRefused, match="content pin"): + build(plan, spec) + assert len(run.call_args_list) == 1 + assert run.call_args.args[0][1:3] == ["policy", "read"] + + +def test_credentials_are_private_before_first_bao_call_even_with_open_umask(tmp_path): + plan, spec = _plan(tmp_path), _spec(tmp_path) + def fake_run(*args, **kwargs): + assert stat.S_IMODE(spec.delivery_dir.stat().st_mode) == 0o700 + for name in ("role_id", "secret_id"): + assert stat.S_IMODE((spec.delivery_dir / name).stat().st_mode) == 0o600 + return MagicMock(returncode=0, stdout="synthetic-credential\n", stderr="") + previous = os.umask(0) + try: + with patch("ops_mason.executor.subprocess.run", side_effect=fake_run): + build_approle_kv_lane(plan, spec) + finally: + os.umask(previous) + assert "synthetic-credential" not in spec.audit_log_path.read_text() + + +@pytest.mark.parametrize("case", ["existing", "symlink-file", "symlink-directory", "symlink-parent", "public-directory", "hardlink"]) +def test_unsafe_delivery_refuses_without_mutation_or_overwrite(tmp_path, case): + spec = _spec(tmp_path) + victim = tmp_path / "victim" + victim.write_text("leave untouched") + if case == "symlink-directory": + target = tmp_path / "target" + target.mkdir(mode=0o700) + spec.delivery_dir.symlink_to(target, target_is_directory=True) + elif case == "symlink-parent": + parent = tmp_path / "parent" + parent.symlink_to(tmp_path, target_is_directory=True) + spec.delivery_dir = parent / "delivery" + else: + spec.delivery_dir.mkdir(mode=0o700) + if case == "public-directory": + spec.delivery_dir.chmod(0o755) + elif case == "existing": + (spec.delivery_dir / "secret_id").write_text("existing-credential") + elif case == "hardlink": + os.link(victim, spec.delivery_dir / "secret_id") + else: + (spec.delivery_dir / "secret_id").symlink_to(victim) + plan = _plan(tmp_path, spec=spec) + with patch("ops_mason.executor.subprocess.run") as run: + with pytest.raises(BuildRefused): + build_approle_kv_lane(plan, spec) + run.assert_not_called() + assert victim.read_text() == "leave untouched" + if case == "existing": + assert (spec.delivery_dir / "secret_id").read_text() == "existing-credential" + + +@pytest.mark.parametrize("failure", ["exit", "timeout"]) +def test_command_errors_never_echo_credentials(tmp_path, failure): + plan, spec = _plan(tmp_path), _spec(tmp_path) + marker = "synthetic-sensitive-output" + kwargs = ({"return_value": MagicMock(returncode=1, stdout=marker, stderr=marker)} + if failure == "exit" else + {"side_effect": subprocess.TimeoutExpired(["bao"], 30, output=marker, stderr=marker)}) + with patch("ops_mason.executor.subprocess.run", **kwargs): + with pytest.raises(BuildError) as error: + build_approle_kv_lane(plan, spec) + assert marker not in str(error.value) + assert not spec.audit_log_path.exists() + assert stat.S_IMODE((spec.delivery_dir / "secret_id").stat().st_mode) == 0o600 diff --git a/tests/test_plan.py b/tests/test_plan.py index 35a9e4b..ec7cc11 100644 --- a/tests/test_plan.py +++ b/tests/test_plan.py @@ -49,3 +49,24 @@ def test_missing_required_field_raises(tmp_path) -> None: path = _write(tmp_path, "id: p\n# status omitted") with pytest.raises(PlanError, match="missing field"): ConstructionPlan.load(path) + + +@pytest.mark.parametrize("frontmatter", [ + "id: p\nstatus: reviewed\nstatus: approved", + "id: p\nstatus: approved\nbuild_spec:\n token_num_uses: 8\n token_num_uses: 0", + "- not-a-mapping", +]) +def test_ambiguous_or_malformed_approval_document_refused(tmp_path, frontmatter): + with pytest.raises(PlanError): + ConstructionPlan.load(_write(tmp_path, frontmatter)) + + +@pytest.mark.parametrize("approver", ['" "', "true", "[bernd]"]) +def test_non_string_or_blank_approval_marker_is_not_approval(tmp_path, approver): + path = _write(tmp_path, f'id: p\nstatus: approved\napproved_by: {approver}\napproved_at: "2026-09-28"') + assert not ConstructionPlan.load(path).is_approved() + + +def test_triple_dash_inside_specification_is_not_frontmatter_delimiter(tmp_path): + path = _write(tmp_path, 'id: p\nstatus: draft\nbuild_spec:\n policy_name: "lane---example"') + assert ConstructionPlan.load(path).build_spec["policy_name"] == "lane---example" diff --git a/workplans/MASON-WP-0001-foundation.md b/workplans/MASON-WP-0001-foundation.md index e6919b6..a0ab789 100644 --- a/workplans/MASON-WP-0001-foundation.md +++ b/workplans/MASON-WP-0001-foundation.md @@ -130,6 +130,18 @@ approved (`test_refusal_never_calls_bao`); role_id/secret_id land as `0600` files and never appear in a log line or exception message; the policy HCL goes over stdin, never argv. +**Hardening follow-up (2026-09-28).** The existing build executor now binds +execution to a reviewed `build_spec` plus `approved_spec_sha256`, reloads the +approval before use, validates literal scope/token/binding inputs, and pins +existing policy contents. Credential files are exclusively created as 0600 +inside caller-owned 0700 directories before issuance, with symlinks and +existing destinations refused. Command errors suppress sensitive output. +Historical build approvals were not rewritten and no live lane was changed. +The suite passes 108 tests, including refusal-before-mutation, policy drift, +permissive-umask, symlink/hardlink/overwrite and error-output regressions. +The source contract and recovery limitations are documented in +`docs/construction-plan-format.md`. This is maintenance of T04, not a new task. + ```task id: MASON-WP-0001-T04 status: done