Bind OpenBao builds to approved inputs and secure credential delivery
Assistant: codex Assistant-Model: gpt-6-astra Assistant-Session: 01a0e75a-fc5c-7913-9dba-9846210c766d
This commit is contained in:
parent
ee1dc2b651
commit
2b83324c01
8 changed files with 622 additions and 84 deletions
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue