Review/optimize checklist, executive-summary format, build executor (T02-T04)
docs/review-optimize-checklist.md: six checks (naming, TTL/scoping,
redundancy, compaction, ease of use, posture), applied for real to the
rein-openweights plan's section 4 -- including a genuinely useful
finding (credentials.py already expects this exact path/delivery shape,
zero code changes needed to consume it).
docs/executive-summary-format.md: six fixed fields, no bao syntax, no
restating earlier sections, explicit approve/reject/revise decision.
Rendered for real into the plan's section 5 -- ready for an actual
decision.
src/ops_mason/{plan,executor,audit}.py: the phase-4 build executor for
credential_type openbao-approle-kv. Refuses to run against anything but
an approved plan -- verified the refusal never even calls subprocess.run.
role_id/secret_id (the AppRole's own access credential, not the
downstream secret) land as 0600 files, never logged; the HCL policy
goes over stdin, never argv; the audit trail is metadata-only. 12 tests,
all mocked at the bao boundary (no live OpenBao access from this
session).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
123ccfe20c
commit
0d62ac501d
12 changed files with 645 additions and 12 deletions
3
.gitignore
vendored
3
.gitignore
vendored
|
|
@ -9,3 +9,6 @@ __pycache__/
|
|||
.claude/*
|
||||
!.claude/rules/
|
||||
!.claude/rules/*.md
|
||||
|
||||
# Local runtime audit trail -- not repo content
|
||||
audit/
|
||||
|
|
|
|||
55
docs/executive-summary-format.md
Normal file
55
docs/executive-summary-format.md
Normal file
|
|
@ -0,0 +1,55 @@
|
|||
# Executive summary format (phase 3)
|
||||
|
||||
The one mandatory human checkpoint (`INTENT.md` design principle 4). Lives
|
||||
as section 5 of the plan file itself (`docs/construction-plan-format.md`)
|
||||
— generated from sections 1-4, not a separate document that can drift
|
||||
from the plan it's summarizing.
|
||||
|
||||
## Why this shape
|
||||
|
||||
Written so the founder never has to read a `bao` command, a policy JSON
|
||||
blob, or an AppRole config to make the call. Six fixed fields, always in
|
||||
this order, always answerable in one or two sentences each:
|
||||
|
||||
1. **One-line ask.** What is being requested, in a sentence a non-expert
|
||||
would understand.
|
||||
2. **Who/what gets access.** The exact new principal (AppRole, role,
|
||||
service identity) — and an explicit statement that nothing *else*
|
||||
gains anything, if that's true (it almost always should be, per
|
||||
`INTENT.md`'s reuse-before-creation principle).
|
||||
3. **To what.** The exact path/policy/scope — narrow enough that a reader
|
||||
can tell at a glance whether it's proportionate to the ask.
|
||||
4. **For how long.** Token TTL and, separately, whether the underlying
|
||||
AppRole credential itself expires or rotates — these are two different
|
||||
questions and both need an answer, not just the token TTL.
|
||||
5. **Blast radius if the credential leaks.** What could actually be done
|
||||
with it — bounded to what section 3 grants, plus a note on whether the
|
||||
downstream secret itself (e.g. a provider API key) carries its own
|
||||
independent risk that this plan doesn't change either way.
|
||||
6. **Cost to reverse.** How many commands, touching how many other
|
||||
consumers, to undo this if it turns out wrong. If the answer is
|
||||
"everything is isolated, reversing this touches nothing else," say so
|
||||
explicitly — that's exactly the property `INTENT.md`'s
|
||||
reuse-vs-new reasoning is supposed to produce.
|
||||
|
||||
Ends with an explicit three-way decision: **approve**, **reject**, or
|
||||
**send back to phase 1** with what to change. No plan should read as a
|
||||
foregone conclusion — "send back" needs to be as easy to pick as
|
||||
"approve."
|
||||
|
||||
## What this format deliberately excludes
|
||||
|
||||
- No `bao` command syntax, no policy HCL, no JSON.
|
||||
- No restating sections 1-4 in full — the executive summary is a
|
||||
compression of what's already there, findable by scrolling up in the
|
||||
same file if more detail is wanted.
|
||||
- No default recommendation baked into the rendering itself beyond what
|
||||
the plan's own reuse-vs-new reasoning already produced — the summary
|
||||
reports the plan's conclusion, it doesn't editorialize a second opinion
|
||||
on top of it.
|
||||
|
||||
## Worked example
|
||||
|
||||
`plans/rein-openweights-openrouter-approle.md` section 5 — the real
|
||||
executive summary for the first real plan, ready for an actual approve /
|
||||
reject / revise decision.
|
||||
63
docs/review-optimize-checklist.md
Normal file
63
docs/review-optimize-checklist.md
Normal file
|
|
@ -0,0 +1,63 @@
|
|||
# Review / optimize checklist (phase 2)
|
||||
|
||||
Runs against a draft plan (`plans/<id>.md`, section 2-3 filled in) before
|
||||
it's shown to anyone. Fill in section 4 ("Review notes") of the plan file
|
||||
directly — this checklist is not a separate artifact, it's what section 4
|
||||
*is*. A plan with an empty section 4 is not ready for phase 3.
|
||||
|
||||
This can be run by a human or an agent going through the plan by hand —
|
||||
it does not need to be automated tooling before it's useful. Automating
|
||||
individual checks (e.g. a script that diffs proposed TTLs against
|
||||
existing lanes) is a reasonable future step once there's more than one
|
||||
worked example to generalize a checker from — not before (same
|
||||
generalize-from-a-second-example discipline the `glas-harness` reins use
|
||||
for their own deferred decisions).
|
||||
|
||||
## The checks
|
||||
|
||||
1. **Naming convention.** Does every new object's name follow the
|
||||
`<consumer>-<credential-purpose>` shape already visible in existing
|
||||
lanes (`agent-harness-binky-mail`, `workload-kv-read-<path-slug>`)? A
|
||||
plan proposing an unrelated naming scheme should be flagged, not
|
||||
waved through for "this one's different."
|
||||
|
||||
2. **TTL/scoping match.** Do proposed `token_ttl`/`token_max_ttl`/
|
||||
`token_num_uses`/`secret_id_ttl` values match an existing comparable
|
||||
lane, or is there a stated reason to diverge? Silent divergence (a
|
||||
shorter or longer TTL with no explanation) is a review finding, not a
|
||||
detail to skip past.
|
||||
|
||||
3. **Redundancy check.** Does a policy, AppRole, or KV path already exist
|
||||
that overlaps what's being proposed? This should already have been
|
||||
asked in section 2 (existing-structure survey) — phase 2 is the
|
||||
second pass confirming that answer still holds after the proposed
|
||||
changes are fully drafted, not skipping it because section 2 already
|
||||
looked.
|
||||
|
||||
4. **Compaction opportunity.** Does satisfying this demand make any
|
||||
*existing* lane redundant or mergeable? Not every plan will find one —
|
||||
most won't — but the check should be asked every time, not only when
|
||||
it's obviously true.
|
||||
|
||||
5. **Ease of use for the consumer.** Is the credential-acquisition path
|
||||
this plan builds toward straightforward for the actual consuming code
|
||||
to use (e.g. does it match an env-var/file-path convention the
|
||||
consumer's code already expects, per its own `credentials.py` or
|
||||
equivalent)? A technically-correct plan that's awkward for the
|
||||
consumer to actually integrate is a real review finding.
|
||||
|
||||
6. **Posture check.** Do the choices here (TTL length, rotation policy,
|
||||
`secret_id_ttl` expiry-or-not) match current organizational posture
|
||||
(`ops-warden/wiki/WorkloadSecurityPosture.md` — build phase today) —
|
||||
not assuming production-tier rigor that isn't the current posture, and
|
||||
not assuming dev-tier looseness forever. State the posture assumption
|
||||
explicitly rather than leaving it implicit.
|
||||
|
||||
## Worked example
|
||||
|
||||
`plans/rein-openweights-openrouter-approle.md` section 4 applies all six
|
||||
checks against a real plan — including one genuine finding (the TTL
|
||||
values were carried over from `agent-harness-binky-mail` by direct
|
||||
analogy rather than re-derived from scratch, which is exactly check 2
|
||||
working as intended: matching an existing lane rather than inventing a
|
||||
new number).
|
||||
|
|
@ -3,7 +3,7 @@ id: rein-openweights-openrouter-approle
|
|||
demand_source: glas-harness/workplans/GLAS-WP-0002-T02
|
||||
consumer_repo: rein-openweights
|
||||
credential_type: openbao-approle-kv
|
||||
status: draft
|
||||
status: reviewed
|
||||
approved_by: null
|
||||
approved_at: null
|
||||
created: "2026-07-27"
|
||||
|
|
@ -97,12 +97,59 @@ as-is for its current interactive-caller consumers.
|
|||
- **Compaction opportunity:** none identified — nothing existing becomes
|
||||
redundant once this lands; the shared `activity-core` lane keeps its
|
||||
own consumers.
|
||||
- **Open item carried into phase 3/4:** confirm current build-phase
|
||||
posture (`ops-warden/wiki/WorkloadSecurityPosture.md`) still supports
|
||||
`secret_id_ttl=0` (no expiry, matching `agent-harness-binky-mail`'s
|
||||
choice) rather than a rotation schedule — note this explicitly in the
|
||||
executive summary rather than assuming it silently.
|
||||
- **Ease of use for the consumer:** matches directly — `credentials.py`
|
||||
already reads `REIN_OPENWEIGHTS_APPROLE_DIR` (defaults to a
|
||||
`role_id`/`secret_id` file pair) and a KV path override via
|
||||
`REIN_OPENWEIGHTS_OPENROUTER_KV_PATH` (defaulting to exactly
|
||||
`reins/rein-openweights/openrouter`, field `api_key`) — this plan's
|
||||
path #1 and delivery #4 need **zero code changes** in
|
||||
`rein-openweights` to consume; the code was already written expecting
|
||||
this exact shape.
|
||||
- **Posture check:** build phase (one founder-operator, pre-revenue) per
|
||||
`ops-warden/wiki/WorkloadSecurityPosture.md` — `agent-harness-binky-mail`
|
||||
used `secret_id_ttl=0` (no expiry) at this same posture; carrying that
|
||||
forward here rather than adding a rotation schedule neither
|
||||
`agent-harness-binky-mail` nor current posture requires. **Explicit
|
||||
assumption, not a silent default** — flagged again in the executive
|
||||
summary below for the approval decision to confirm or override.
|
||||
|
||||
<!-- Phase 3 (executive summary) and phase 4 (build result) sections are
|
||||
appended once MASON-WP-0001-T03/T04 land — this file stops at phase 2
|
||||
until then. -->
|
||||
## 5. Executive summary (phase 3)
|
||||
|
||||
**One-line ask:** create a narrowly-scoped, non-interactive credential
|
||||
lane so `rein-openweights` can fetch its own OpenRouter API key at run
|
||||
time without an operator present.
|
||||
|
||||
**Who/what gets access:** a new OpenBao AppRole named `rein-openweights`.
|
||||
Nothing else — no existing role, human, or service gains anything new.
|
||||
|
||||
**To what:** read-only access to exactly one KV path,
|
||||
`reins/rein-openweights/openrouter` (field `api_key`) — a path that does
|
||||
not exist today and holds nothing else. The AppRole cannot read the
|
||||
existing shared `activity-core`/`llm-connect` secret or any other path.
|
||||
|
||||
**For how long:** each login is a 15-minute token, renewable up to a
|
||||
30-minute cap, bounded number of uses per token — matches the existing
|
||||
`agent-harness-binky-mail` lane exactly. The AppRole credential itself
|
||||
(`role_id`/`secret_id`) does not expire on a schedule at current
|
||||
(build-phase) posture, same choice already made for
|
||||
`agent-harness-binky-mail`.
|
||||
|
||||
**Blast radius if the credential leaks:** whoever holds a valid
|
||||
`role_id`/`secret_id` pair can mint a short-lived token that reads one
|
||||
OpenRouter API key — nothing else in OpenBao. Actual damage from the
|
||||
OpenRouter key itself is real but bounded (API spend on that key, no
|
||||
data-store or infra access) and independent of this plan (same exposure
|
||||
`openrouter-llm-connect`'s existing consumers already carry).
|
||||
|
||||
**Cost to reverse:** delete one AppRole, one policy, one KV path — three
|
||||
`bao` commands, no other lane or consumer affected. Fully isolated
|
||||
blast/reversal radius by construction (that was the point of declining
|
||||
reuse in §2).
|
||||
|
||||
**Decision needed:** approve as proposed, reject, or send back to phase 1
|
||||
with feedback (e.g. different TTL, different path name, or "actually
|
||||
reuse the shared lane instead").
|
||||
|
||||
## 6. Build result (phase 4)
|
||||
|
||||
<!-- Appended once MASON-WP-0001-T04/T05 execute this plan after approval. -->
|
||||
|
|
|
|||
22
pyproject.toml
Normal file
22
pyproject.toml
Normal file
|
|
@ -0,0 +1,22 @@
|
|||
[build-system]
|
||||
requires = ["hatchling"]
|
||||
build-backend = "hatchling.build"
|
||||
|
||||
[project]
|
||||
name = "ops-mason"
|
||||
version = "0.1.0"
|
||||
description = "Builder of NetKingdom security infrastructure (AppRoles, policies, KV paths) for ops-warden to route to"
|
||||
requires-python = ">=3.11"
|
||||
dependencies = [
|
||||
"PyYAML>=6.0",
|
||||
]
|
||||
|
||||
[project.optional-dependencies]
|
||||
dev = ["pytest>=8"]
|
||||
|
||||
[tool.hatch.build.targets.wheel]
|
||||
packages = ["src/ops_mason"]
|
||||
|
||||
[tool.pytest.ini_options]
|
||||
testpaths = ["tests"]
|
||||
pythonpath = ["src"]
|
||||
0
src/ops_mason/__init__.py
Normal file
0
src/ops_mason/__init__.py
Normal file
48
src/ops_mason/audit.py
Normal file
48
src/ops_mason/audit.py
Normal file
|
|
@ -0,0 +1,48 @@
|
|||
"""Metadata-only audit trail for what ops-mason built.
|
||||
|
||||
Same invariant as ops-warden's own audit trail (`warden activity`):
|
||||
object names and plan references only, never secret material.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from dataclasses import asdict, dataclass
|
||||
from datetime import UTC, datetime
|
||||
from pathlib import Path
|
||||
|
||||
_DEFAULT_LOG = Path(__file__).resolve().parents[2] / "audit" / "build-log.jsonl"
|
||||
|
||||
|
||||
@dataclass
|
||||
class BuildRecord:
|
||||
plan_id: str
|
||||
plan_path: str
|
||||
approved_by: str
|
||||
approved_at: str
|
||||
objects: dict[str, str]
|
||||
built_at: str
|
||||
|
||||
|
||||
def record_build(
|
||||
*,
|
||||
plan_id: str,
|
||||
plan_path: str,
|
||||
approved_by: str,
|
||||
approved_at: str,
|
||||
objects: dict[str, str],
|
||||
log_path: Path | None = None,
|
||||
) -> BuildRecord:
|
||||
record = BuildRecord(
|
||||
plan_id=plan_id,
|
||||
plan_path=plan_path,
|
||||
approved_by=approved_by,
|
||||
approved_at=approved_at,
|
||||
objects=objects,
|
||||
built_at=datetime.now(UTC).isoformat(),
|
||||
)
|
||||
path = log_path or _DEFAULT_LOG
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
with path.open("a") as fh:
|
||||
fh.write(json.dumps(asdict(record)) + "\n")
|
||||
return record
|
||||
134
src/ops_mason/executor.py
Normal file
134
src/ops_mason/executor.py
Normal file
|
|
@ -0,0 +1,134 @@
|
|||
"""Phase 4: execute an approved construction plan against OpenBao.
|
||||
|
||||
Narrow on purpose (MASON-WP-0001-T04) — implements only the operations
|
||||
the first real plan (rein-openweights-openrouter-approle,
|
||||
credential_type: openbao-approle-kv) needs: a read-only policy scoped to
|
||||
one KV path, an AppRole bound to it, and role_id/secret_id delivery.
|
||||
|
||||
Never writes to the KV path itself and never touches the downstream
|
||||
secret value (the OpenRouter API key). The KV path a policy references
|
||||
comes into existence later, when the founder does a paste-once-provision
|
||||
through ops-warden's desk (`bao kv put`) — not through this executor.
|
||||
role_id/secret_id are the AppRole's own access credential, not the
|
||||
downstream secret; generating and delivering them is squarely
|
||||
ops-mason's job (INTENT.md: "create ... credentials, tokens"), but they
|
||||
are still handled write-only — never logged, never returned in a
|
||||
printable form beyond confirmation that delivery happened.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import subprocess
|
||||
from dataclasses import dataclass, field
|
||||
from pathlib import Path
|
||||
|
||||
from ops_mason.audit import record_build
|
||||
from ops_mason.plan import ConstructionPlan
|
||||
|
||||
|
||||
class BuildRefused(RuntimeError):
|
||||
"""The plan is not approved. This is the one place a bug is a security incident."""
|
||||
|
||||
|
||||
class BuildError(RuntimeError):
|
||||
"""The plan was approved but a bao operation failed."""
|
||||
|
||||
|
||||
@dataclass
|
||||
class AppRoleKVSpec:
|
||||
policy_name: str
|
||||
kv_path: str
|
||||
approle_name: str
|
||||
kv_capabilities: tuple[str, ...] = ("read",)
|
||||
token_ttl: str = "15m"
|
||||
token_max_ttl: str = "30m"
|
||||
token_num_uses: int = 0
|
||||
secret_id_ttl: str = "0"
|
||||
delivery_dir: Path | None = None
|
||||
bao_bin: str = "bao"
|
||||
audit_log_path: Path | None = None
|
||||
|
||||
|
||||
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,
|
||||
)
|
||||
if proc.returncode != 0:
|
||||
raise BuildError(f"`{bao_bin} {' '.join(args)}` failed: {proc.stderr.strip()[:300]}")
|
||||
return proc.stdout
|
||||
|
||||
|
||||
def _policy_hcl(kv_path: str, capabilities: tuple[str, ...]) -> str:
|
||||
caps = ", ".join(f'"{c}"' for c in capabilities)
|
||||
return f'path "{kv_path}" {{\n capabilities = [{caps}]\n}}\n'
|
||||
|
||||
|
||||
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"
|
||||
)
|
||||
|
||||
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}",
|
||||
],
|
||||
)
|
||||
|
||||
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)
|
||||
|
||||
objects = {
|
||||
"policy_name": spec.policy_name,
|
||||
"approle_name": spec.approle_name,
|
||||
"kv_path": spec.kv_path,
|
||||
"delivered_to": delivered_to,
|
||||
}
|
||||
record_build(
|
||||
plan_id=plan.id,
|
||||
plan_path=str(plan.path),
|
||||
approved_by=plan.approved_by or "",
|
||||
approved_at=plan.approved_at or "",
|
||||
objects=objects,
|
||||
log_path=spec.audit_log_path,
|
||||
)
|
||||
return objects
|
||||
56
src/ops_mason/plan.py
Normal file
56
src/ops_mason/plan.py
Normal file
|
|
@ -0,0 +1,56 @@
|
|||
"""Parse a construction plan file's YAML frontmatter.
|
||||
|
||||
See docs/construction-plan-format.md. The frontmatter's status field is
|
||||
the phase-4 hard gate: the build executor (executor.py) refuses to run
|
||||
against anything but status: approved with both approved_by/approved_at
|
||||
set.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
import yaml
|
||||
|
||||
|
||||
class PlanError(RuntimeError):
|
||||
pass
|
||||
|
||||
|
||||
@dataclass
|
||||
class ConstructionPlan:
|
||||
id: str
|
||||
status: str
|
||||
approved_by: str | None
|
||||
approved_at: str | None
|
||||
demand_source: str
|
||||
consumer_repo: str
|
||||
credential_type: str
|
||||
path: Path
|
||||
|
||||
@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("---"):
|
||||
raise PlanError(f"{path}: missing YAML frontmatter")
|
||||
data: dict[str, Any] = yaml.safe_load(parts[1]) or {}
|
||||
missing = {"id", "status"} - set(data)
|
||||
if missing:
|
||||
raise PlanError(f"{path}: frontmatter missing field(s): {', '.join(sorted(missing))}")
|
||||
return cls(
|
||||
id=data["id"],
|
||||
status=data["status"],
|
||||
approved_by=data.get("approved_by"),
|
||||
approved_at=data.get("approved_at"),
|
||||
demand_source=data.get("demand_source", ""),
|
||||
consumer_repo=data.get("consumer_repo", ""),
|
||||
credential_type=data.get("credential_type", ""),
|
||||
path=path,
|
||||
)
|
||||
|
||||
def is_approved(self) -> bool:
|
||||
return self.status == "approved" and bool(self.approved_by) and bool(self.approved_at)
|
||||
123
tests/test_executor.py
Normal file
123
tests/test_executor.py
Normal file
|
|
@ -0,0 +1,123 @@
|
|||
import json
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from ops_mason.executor import AppRoleKVSpec, BuildError, BuildRefused, build_approle_kv_lane
|
||||
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"
|
||||
)
|
||||
p = tmp_path / "plan.md"
|
||||
p.write_text(text)
|
||||
return ConstructionPlan.load(str(p))
|
||||
|
||||
|
||||
def _spec(tmp_path) -> AppRoleKVSpec:
|
||||
return AppRoleKVSpec(
|
||||
policy_name="workload-kv-read-test-lane",
|
||||
kv_path="reins/test/openrouter",
|
||||
approle_name="test-lane",
|
||||
delivery_dir=tmp_path / "delivery",
|
||||
audit_log_path=tmp_path / "audit.jsonl",
|
||||
)
|
||||
|
||||
|
||||
def test_refuses_when_not_approved(tmp_path) -> None:
|
||||
plan = _plan(tmp_path, status="draft", approved_by=None, approved_at=None)
|
||||
with pytest.raises(BuildRefused, match="not approved"):
|
||||
build_approle_kv_lane(plan, _spec(tmp_path))
|
||||
|
||||
|
||||
def test_refuses_when_approved_status_but_missing_approver(tmp_path) -> None:
|
||||
plan = _plan(tmp_path, status="approved", approved_by=None, approved_at="2026-07-27")
|
||||
with pytest.raises(BuildRefused, match="not approved"):
|
||||
build_approle_kv_lane(plan, _spec(tmp_path))
|
||||
|
||||
|
||||
def test_refusal_never_calls_bao(tmp_path) -> None:
|
||||
plan = _plan(tmp_path, status="reviewed", approved_by=None, approved_at=None)
|
||||
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()
|
||||
|
||||
|
||||
def test_build_writes_policy_approle_and_delivers_credentials(tmp_path) -> None:
|
||||
plan = _plan(tmp_path)
|
||||
spec = _spec(tmp_path)
|
||||
|
||||
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"]:
|
||||
result.stdout = "role-id-value\n"
|
||||
elif "-field=secret_id" in cmd:
|
||||
result.stdout = "secret-id-value\n"
|
||||
else:
|
||||
result.stdout = ""
|
||||
return result
|
||||
|
||||
with patch("ops_mason.executor.subprocess.run", side_effect=fake_run) as run:
|
||||
objects = build_approle_kv_lane(plan, spec)
|
||||
|
||||
assert objects["policy_name"] == "workload-kv-read-test-lane"
|
||||
assert objects["approle_name"] == "test-lane"
|
||||
assert objects["kv_path"] == "reins/test/openrouter"
|
||||
|
||||
role_id_file = spec.delivery_dir / "role_id"
|
||||
secret_id_file = spec.delivery_dir / "secret_id"
|
||||
assert role_id_file.read_text() == "role-id-value\n"
|
||||
assert secret_id_file.read_text() == "secret-id-value\n"
|
||||
assert oct(role_id_file.stat().st_mode)[-3:] == "600"
|
||||
assert oct(secret_id_file.stat().st_mode)[-3:] == "600"
|
||||
|
||||
# policy write call carried the HCL on stdin, not argv -- never in a log line
|
||||
policy_call = next(c for c in run.call_args_list if c.args[0][:2] == ["bao", "policy"])
|
||||
assert "reins/test/openrouter" in policy_call.kwargs["input"]
|
||||
|
||||
# audit record landed at the explicit path, metadata only, no role_id/secret_id values
|
||||
audit_text = spec.audit_log_path.read_text()
|
||||
assert "policy_name" in audit_text
|
||||
assert "role-id-value" not in audit_text
|
||||
assert "secret-id-value" not in audit_text
|
||||
|
||||
|
||||
def test_build_appends_audit_record(tmp_path) -> None:
|
||||
plan = _plan(tmp_path)
|
||||
spec = _spec(tmp_path)
|
||||
|
||||
def fake_run(cmd, input=None, capture_output=True, text=True, timeout=30):
|
||||
result = MagicMock(returncode=0, stderr="")
|
||||
result.stdout = "value\n"
|
||||
return result
|
||||
|
||||
with (
|
||||
patch("ops_mason.executor.subprocess.run", side_effect=fake_run),
|
||||
patch("ops_mason.executor.record_build") as record_build_mock,
|
||||
):
|
||||
build_approle_kv_lane(plan, spec)
|
||||
|
||||
record_build_mock.assert_called_once()
|
||||
kwargs = record_build_mock.call_args.kwargs
|
||||
assert kwargs["plan_id"] == "test-lane"
|
||||
assert kwargs["approved_by"] == "bernd"
|
||||
assert "policy_name" in kwargs["objects"]
|
||||
assert kwargs["log_path"] == spec.audit_log_path
|
||||
|
||||
|
||||
def test_bao_failure_raises_build_error(tmp_path) -> None:
|
||||
plan = _plan(tmp_path)
|
||||
spec = _spec(tmp_path)
|
||||
|
||||
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"):
|
||||
build_approle_kv_lane(plan, spec)
|
||||
51
tests/test_plan.py
Normal file
51
tests/test_plan.py
Normal file
|
|
@ -0,0 +1,51 @@
|
|||
import pytest
|
||||
|
||||
from ops_mason.plan import ConstructionPlan, PlanError
|
||||
|
||||
|
||||
def _write(tmp_path, frontmatter: str, body: str = "\n# Plan\n") -> str:
|
||||
p = tmp_path / "plan.md"
|
||||
p.write_text(f"---\n{frontmatter}\n---\n{body}")
|
||||
return str(p)
|
||||
|
||||
|
||||
def test_load_parses_frontmatter(tmp_path) -> None:
|
||||
path = _write(
|
||||
tmp_path,
|
||||
"id: test-plan\nstatus: draft\ndemand_source: x\nconsumer_repo: y\ncredential_type: openbao-approle-kv",
|
||||
)
|
||||
plan = ConstructionPlan.load(path)
|
||||
assert plan.id == "test-plan"
|
||||
assert plan.status == "draft"
|
||||
assert plan.consumer_repo == "y"
|
||||
|
||||
|
||||
def test_is_approved_false_when_draft(tmp_path) -> None:
|
||||
path = _write(tmp_path, "id: p\nstatus: draft")
|
||||
assert ConstructionPlan.load(path).is_approved() is False
|
||||
|
||||
|
||||
def test_is_approved_false_when_approved_but_missing_approver(tmp_path) -> None:
|
||||
path = _write(tmp_path, "id: p\nstatus: approved")
|
||||
assert ConstructionPlan.load(path).is_approved() is False
|
||||
|
||||
|
||||
def test_is_approved_true_when_fully_approved(tmp_path) -> None:
|
||||
path = _write(
|
||||
tmp_path,
|
||||
'id: p\nstatus: approved\napproved_by: "bernd"\napproved_at: "2026-07-27"',
|
||||
)
|
||||
assert ConstructionPlan.load(path).is_approved() is True
|
||||
|
||||
|
||||
def test_missing_frontmatter_raises(tmp_path) -> None:
|
||||
p = tmp_path / "plan.md"
|
||||
p.write_text("# no frontmatter here\n")
|
||||
with pytest.raises(PlanError, match="missing YAML frontmatter"):
|
||||
ConstructionPlan.load(str(p))
|
||||
|
||||
|
||||
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)
|
||||
|
|
@ -57,9 +57,20 @@ note any compaction opportunity (two existing lanes that could merge).
|
|||
This can start as a checklist a human/agent runs manually against a
|
||||
draft plan — doesn't need to be automated tooling on day one.
|
||||
|
||||
**Done (2026-07-27).** `docs/review-optimize-checklist.md`: six checks
|
||||
(naming, TTL/scoping, redundancy, compaction, ease of use for the
|
||||
consumer, posture) — manual for now, automating a checker deferred until
|
||||
there's a second worked example to generalize from. Applied for real to
|
||||
`plans/rein-openweights-openrouter-approle.md` §4 — including a
|
||||
genuinely useful finding (`credentials.py` already expects exactly this
|
||||
plan's path/delivery shape, zero code changes needed to consume it) and
|
||||
one explicit posture assumption flagged rather than left silent
|
||||
(`secret_id_ttl=0`, carried from `agent-harness-binky-mail`, surfaced
|
||||
again in the executive summary for the approval decision to confirm).
|
||||
|
||||
```task
|
||||
id: MASON-WP-0001-T02
|
||||
status: todo
|
||||
status: done
|
||||
priority: high
|
||||
state_hub_task_id: "58a446fb-78d0-42b4-b10a-1119266b1016"
|
||||
```
|
||||
|
|
@ -74,9 +85,17 @@ be readable without knowing `bao` syntax. Document in
|
|||
`docs/executive-summary-format.md`, with the rein-openweights AppRole
|
||||
plan (task T05) as the worked example.
|
||||
|
||||
**Done (2026-07-27).** `docs/executive-summary-format.md`: six fixed
|
||||
fields (one-line ask, who/what gets access, to what, for how long,
|
||||
blast radius, cost to reverse) plus an explicit approve/reject/revise
|
||||
decision — no `bao` syntax, no restating sections 1-4, no editorializing
|
||||
beyond the plan's own reuse-vs-new conclusion. Rendered for real into
|
||||
`plans/rein-openweights-openrouter-approle.md` §5 — ready for an actual
|
||||
decision, not a mockup.
|
||||
|
||||
```task
|
||||
id: MASON-WP-0001-T03
|
||||
status: todo
|
||||
status: done
|
||||
priority: medium
|
||||
state_hub_task_id: "e37debfd-8a00-4780-b036-9376c6a10557"
|
||||
```
|
||||
|
|
@ -98,9 +117,21 @@ the one place a bug is a real security incident, not a bad UX. Start
|
|||
narrow: implement only the operations the first real plan (T05) needs,
|
||||
not a general OpenBao automation framework.
|
||||
|
||||
**Done (2026-07-27).** `src/ops_mason/plan.py` (frontmatter parser +
|
||||
`is_approved()`), `src/ops_mason/executor.py`
|
||||
(`build_approle_kv_lane` — policy write, AppRole create,
|
||||
role_id/secret_id delivery, never the KV path's value), `src/ops_mason/audit.py`
|
||||
(metadata-only JSONL build log). 12 tests, all mocked at the `bao`
|
||||
subprocess boundary (no live OpenBao access from this session — see
|
||||
`GLAS-WP-0002-T02`'s original blocker). Explicitly verified: the refusal
|
||||
gate never even calls `subprocess.run` when the plan isn't fully
|
||||
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.
|
||||
|
||||
```task
|
||||
id: MASON-WP-0001-T04
|
||||
status: todo
|
||||
status: done
|
||||
priority: high
|
||||
state_hub_task_id: "e949f4b7-5ef4-4e42-8a07-61b6c8040298"
|
||||
```
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue