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 <noreply@anthropic.com>
This commit is contained in:
parent
846ef0561c
commit
9bcb5819de
3 changed files with 79 additions and 4 deletions
|
|
@ -3,7 +3,7 @@ id: rein-openweights-openrouter-approle
|
||||||
demand_source: glas-harness/workplans/GLAS-WP-0002-T02
|
demand_source: glas-harness/workplans/GLAS-WP-0002-T02
|
||||||
consumer_repo: rein-openweights
|
consumer_repo: rein-openweights
|
||||||
credential_type: openbao-approle-kv
|
credential_type: openbao-approle-kv
|
||||||
status: built
|
status: catalogued
|
||||||
approved_by: "Bernd Worsch"
|
approved_by: "Bernd Worsch"
|
||||||
approved_at: "2026-07-27"
|
approved_at: "2026-07-27"
|
||||||
created: "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`
|
the real OpenRouter API key into `reins/rein-openweights/openrouter`
|
||||||
(field `api_key`) through ops-warden's `paste_once_provision` desk —
|
(field `api_key`) through ops-warden's `paste_once_provision` desk —
|
||||||
ops-mason built structure only, per `INTENT.md`.
|
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
|
||||||
|
`<mount>/data/<path>` (with a separate `<mount>/metadata/<path>` 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).
|
||||||
|
|
|
||||||
|
|
@ -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:
|
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 `<mount>/data/<path>`;
|
||||||
|
a policy written against the bare `<mount>/<path>` (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)
|
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]:
|
def build_approle_kv_lane(plan: ConstructionPlan, spec: AppRoleKVSpec) -> dict[str, str]:
|
||||||
|
|
|
||||||
|
|
@ -3,7 +3,21 @@ from unittest.mock import MagicMock, patch
|
||||||
|
|
||||||
import pytest
|
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
|
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"
|
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 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"])
|
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 record landed at the explicit path, metadata only, no role_id/secret_id values
|
||||||
audit_text = spec.audit_log_path.read_text()
|
audit_text = spec.audit_log_path.read_text()
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue