From 9bcb5819de3bc58c6b60418f486153be5bd32621 Mon Sep 17 00:00:00 2001 From: tegwick Date: Mon, 27 Jul 2026 01:50:18 +0200 Subject: [PATCH] Fix KV v2 policy path shape; live end-to-end verification succeeded Two more real bugs found completing this lane for real: 1. platform-admin's own policy had no entry for the new reins/ mount -- the founder's paste-once-provision write 403'd because the admin identity that created the mount was never granted access to operate on it. Fixed live (added path "reins/*" matching every other mount already in that policy). 2. _policy_hcl wrote the bare KV-v1-shaped path (reins/rein-openweights/openrouter) instead of KV v2's data/+metadata/ sub-paths -- bao token capabilities on the bare path even reported full access, but the actual kv get still 403'd, because OpenBao evaluates the real request against the data/-prefixed path. Caught when the AppRole's own read failed during live verification. Fixed in code (now emits both data/ and metadata/ paths), locked in with a dedicated unit test, and re-applied to the live policy. Live end-to-end verification succeeded after both fixes: real AppRole login, real KV v2 read via the corrected policy, real OpenRouter call, real commit -- with OPENROUTER_API_KEY unset the whole time. Plan status: catalogued. glas-harness/GLAS-WP-0002-T02 is closed by this. Co-Authored-By: Claude Sonnet 5 --- plans/rein-openweights-openrouter-approle.md | 46 +++++++++++++++++++- src/ops_mason/executor.py | 16 ++++++- tests/test_executor.py | 21 ++++++++- 3 files changed, 79 insertions(+), 4 deletions(-) diff --git a/plans/rein-openweights-openrouter-approle.md b/plans/rein-openweights-openrouter-approle.md index 56cc442..141bc64 100644 --- a/plans/rein-openweights-openrouter-approle.md +++ b/plans/rein-openweights-openrouter-approle.md @@ -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: built +status: catalogued approved_by: "Bernd Worsch" approved_at: "2026-07-27" created: "2026-07-27" @@ -199,3 +199,47 @@ below — no secret material. the real OpenRouter API key into `reins/rein-openweights/openrouter` (field `api_key`) through ops-warden's `paste_once_provision` desk — ops-mason built structure only, per `INTENT.md`. + +## 7. Post-build corrections (found completing phases 4-onward for real) + +Two more real bugs surfaced only by actually exercising the built lane, +both fixed at the root cause, not just patched live: + +- **`platform-admin`'s own policy had no entry for the new `reins/` + mount.** The founder's `paste_once_provision` desk write failed + (`403`, "preflight capability check") because the admin identity that + created the mount was never granted access to operate on it — every + other KV mount (`platform/`, `secret/`, `tenants/`, ...) is explicitly + listed in `platform-admin`'s policy; `reins/*` wasn't, since the policy + predates the mount. Fixed by adding a `path "reins/*" {...}` block + matching the existing entries exactly. **Process gap, not just a code + bug:** `build_approle_kv_lane` creates the *consumer's* policy/AppRole + but never checks whether the *founder's own* admin policy can reach a + brand-new mount — worth a follow-up task to check this automatically + for any future plan that creates a new mount, not just this one. +- **The generated policy used the wrong path shape for a KV v2 mount.** + `_policy_hcl` wrote `path "reins/rein-openweights/openrouter"` — the + bare, KV-v1-shaped path. KV v2 routes actual reads/writes through + `/data/` (with a separate `/metadata/` for + version operations); a policy against the bare path silently denies + everything on a v2 mount. `bao token capabilities` on the bare path + had even reported full access earlier in this build — misleading, + since OpenBao evaluates the real request against the `data/`-prefixed + path, not whatever a caller checks capabilities against. Caught when + the AppRole's own `kv get` 403'd during the live `GLAS-WP-0002-T02` + verification. Fixed in `ops_mason/executor.py::_policy_hcl` (now + emits both `data/` and `metadata/` sub-paths) and re-applied to the + live policy; a dedicated unit test + (`test_policy_hcl_uses_kv_v2_data_and_metadata_paths`) locks the + correct shape in for every future plan. + +**Live end-to-end verification succeeded (2026-07-27)** after both +fixes: real AppRole login → real KV v2 read via the corrected policy → +real OpenRouter API call → real 2-turn tool-calling loop → real commit +(`e30d520c79a3493aeea5e8ea4d5ec331512b781d`), with `OPENROUTER_API_KEY` +unset the whole time — the vault round trip, not the env-var +short-circuit, is what ran. `glas-harness/GLAS-WP-0002-T02` is closed by +this. + +**Plan status: catalogued** — ops-warden entry promoted `draft` -> +`active` (see that repo's commit history). diff --git a/src/ops_mason/executor.py b/src/ops_mason/executor.py index 7ac9da6..bab06d6 100644 --- a/src/ops_mason/executor.py +++ b/src/ops_mason/executor.py @@ -69,8 +69,22 @@ def _run(bao_bin: str, args: list[str], input_text: str | None = None) -> str: def _policy_hcl(kv_path: str, capabilities: tuple[str, ...]) -> str: + """Policy HCL for a KV v2 path — grants on both data/ and metadata/ sub-paths. + + KV v2 routes actual secret reads/writes through `/data/`; + a policy written against the bare `/` (the KV v1 shape) + silently denies everything on a v2 mount. Caught live during + MASON-WP-0001-T05's build — `bao token capabilities` on the bare path + reported full access, but the actual `kv get` still 403'd, because + OpenBao evaluates policy against the real `data/`-prefixed path, not + the one a caller might naively check capabilities against. + """ + mount, _, rest = kv_path.partition("/") caps = ", ".join(f'"{c}"' for c in capabilities) - return f'path "{kv_path}" {{\n capabilities = [{caps}]\n}}\n' + return ( + f'path "{mount}/data/{rest}" {{\n capabilities = [{caps}]\n}}\n\n' + f'path "{mount}/metadata/{rest}" {{\n capabilities = [{caps}]\n}}\n' + ) def build_approle_kv_lane(plan: ConstructionPlan, spec: AppRoleKVSpec) -> dict[str, str]: diff --git a/tests/test_executor.py b/tests/test_executor.py index a6fe5d5..cb3d384 100644 --- a/tests/test_executor.py +++ b/tests/test_executor.py @@ -3,7 +3,21 @@ from unittest.mock import MagicMock, patch import pytest -from ops_mason.executor import AppRoleKVSpec, BuildError, BuildRefused, build_approle_kv_lane +from ops_mason.executor import ( + AppRoleKVSpec, + BuildError, + BuildRefused, + _policy_hcl, + build_approle_kv_lane, +) + + +def test_policy_hcl_uses_kv_v2_data_and_metadata_paths() -> None: + hcl = _policy_hcl("reins/rein-openweights/openrouter", ("read",)) + assert 'path "reins/data/rein-openweights/openrouter" {' in hcl + assert 'path "reins/metadata/rein-openweights/openrouter" {' in hcl + assert 'path "reins/rein-openweights/openrouter" {' not in hcl + assert 'capabilities = ["read"]' in hcl from ops_mason.plan import ConstructionPlan @@ -88,8 +102,11 @@ def test_build_writes_policy_approle_and_delivers_credentials(tmp_path) -> None: 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 + # -- KV v2 shape: data/ and metadata/ sub-paths, not the bare path 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"] + assert 'path "reins/data/test/openrouter"' in policy_call.kwargs["input"] + assert 'path "reins/metadata/test/openrouter"' in policy_call.kwargs["input"] + assert 'path "reins/test/openrouter"' not 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()