session 48: the plugin OpenBao can actually spawn — go-plugin's protocol, in Rust (#48) #52
No reviewers
Labels
No labels
Compat/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Meta/Campaign
Meta/Epic
Meta/Session
Priority/Critical
Priority/High
Priority/Low
Priority/Medium
Reviewed/Confirmed
Reviewed/Curated
Reviewed/Duplicate
Reviewed/Invalid
Reviewed/Won't Fix
Scope/Campaign
Status/Abandoned
Status/Blocked
Status/Conflicted
Status/In Progress
Status/In Review
Status/Need Grooming
Status/Need More Info
Status/Ready
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
Ting/Vedanta!52
Loading…
Reference in a new issue
No description provided.
Delete branch "tango/48-bao-plugin-protocol"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #48 when its live run lands. A worker is still driving the end-to-end run against a real
bao; this PR is open early for review of the protocol work.Why
openbao-plugin/held the ordered issue flow, the lease machine, the one-shot wrapper and the signed-command dispatch — as a library OpenBao cannot load. No handshake, noGRPCBackend, no broker. So "OpenBao owns the lease" was true in the type system and false on the host. Freeholder ruling D1 (2026-09-05): "it's better to have the plugin be same language as Vedanta so Rust it is."What lands
.protofiles copied verbatim fromopenbao/openbao v2.6.2andhashicorp/go-plugin v1.8.0, with their MPL-2.0 licences and the exact revisions recorded inproto/PIN. Bumping a pin is a deliberate act with a written procedure.build.rsgenerates withtonic-prost-buildoverprotox, a pure-Rust protoc, so the devenv shell needs noprotocand the workspace needs no second toolchain. Fifteen.rsfiles; there is no.gofile and nogo.modin this tree.PLUGIN_CLIENT_CERT, and the three control services (GRPCController,GRPCBroker,GRPCStdio) plus the health service go-plugin pings.pb.Backendrouted to the existing flows, with the four stores backed by OpenBao's own storage over the broker — the plugin owns no file.vedanta-openbao-pluginthat OpenBao's catalog spawns.Evidence
devenv shell --:cargo build --workspaceclean,cargo clippy --workspace --all-targets -- -D warningsclean,cargo test --workspacegreen — 71 plugin lib tests, plus the root crate's own suite unaffected. The write surface stayed insideopenbao-plugin/andCargo.lock; independently verified.What is honestly not proven yet
tests/bao_dev.rsis written and currently#[ignore]d. Run once against realbaov2.6.2, plugin registration answers 204 and the immediate catalog read-back does not echo the version. Under diagnosis; the flow past that point (mount → issue → unwrap → read → rotate → renew → revoke) has not been exercised.PLUGIN_*environment names — are attributed by comments to OpenBao'ssdk/plugin/serve.goand go-plugin'sclient.go, neither of which is vendored here (only.protofiles are). They are internally consistent and match the handshake unit tests, but they were not re-confirmed against the Go source. A successful live handshake confirms them; until then this is stated, not hidden.What a reviewer must check
.protofiles under their own licences is the shape you want for protocol definitions in this repository.backend.rs is the pb.Backend gRPC service: Setup dials the broker for Storage/SystemView and reads mount options; HandleRequest routes the five workload-shaped paths (issue, lease/<id> read, .../renew, .../revoke, unwrap/<token>) to the existing issue::issue and lease::{rotate,revoke} flows unchanged, and wraps a lease-creating answer in OpenBao's own Secret metadata so the host's lease manager — not this plugin — owns renew and revoke scheduling. vedanta.rs is PrivateMint over Vedanta #47's private transport (unix socket, signed envelope), reached with the Ed25519 signing key from the path OpenBao's plugin environment supplies, never stored. lib.rs now declares the full module tree the crate gained on this branch: backend, broker, goplugin, handshake, mtls, proto, storage, stores, vedanta, alongside the unchanged issue/lease contract flows. Ruling: read and kept for the same reason as the rest of the branch — unit-tested (routing table, denial-class mapping, mount-setting precedence, TTL/seed parsing, Secret-wrapping only for lease-creating answers; the private-transport response parsing and envelope posting).bao_dev.rs walks issue -> unwrap -> read -> rotate -> renew -> revoke against a real `bao server -dev` with the built plugin registered in its catalog, backed by a Vedanta double on a unix socket. This is the acceptance test the issue's design section calls for, but the task bounding this pass draws the line at "compiles with unit tests" and hands the real end-to-end run to a separate task — so it is marked #[ignore] rather than left to fail cargo test --workspace. One compile bug fixed: the mount call referenced an undefined `version` local; it now builds the same `v{CARGO_PKG_VERSION}` string the catalog assertion above it already computes inline. Measured, 2026-09-06, `bao` v2.6.2 present on PATH (matches proto/PIN): running it un-ignored gets past the plugin registration (PUT returns 204) but fails at the very next line — the catalog GET's `data.version` comes back JSON null where the test expects "v0.1.0". I did not chase this further: it may be a real bug in how the plugin answers OpenBao's version probe, a wrong assumption about the catalog API's response shape, or a bao-dev quirk. Whoever picks up the end-to-end task should start by reading plugin.log and bao.log (the test writes both to a scratch dir and prints them on any assertion failure) from an un-ignored run.Chased a suspicion that HandleRequestArgs.storage_id names a fresh, per-request, namespace-scoped broker connection for Storage/SystemView (distinct from Setup's one persistent connection) — plausible reading of the field, and a candidate explanation for a "no namespace" error hit downstream in SystemView.ResponseWrapData. Tried it: dialing storage_id per request times out ("no broker address for service id 0 within 5s"), and a new broker.deliver trace across a full bao_dev.rs walk shows OpenBao announcing exactly one broker address, ever, for Setup's own broker_id. Confirmed against OpenBao's own upstream source (sdk/plugin/grpc_backend_ server.go, backendGRPCPluginServer.HandleRequest, both the v2.6.2 tag and current main): the real Go SDK's own plugin-side Backend server looks the backend up by multiplex id, not by args.StorageID, and never redials. storage_id is accepted but genuinely unused, matching the vendored proto's own comment on Setup ("use the provided broker_id ... for the Storage and SystemView clients") — one connection, for the mount's whole lifetime. Recorded this in answer()'s doc comment so nobody re-chases it, and left the broker.deliver/setup broker_id debug traces in place since they're what pinned this down.Ran bao_dev.rs un-ignored against real bao v2.6.2 (proto/PIN's pin, present on PATH). Two real bugs, both in the test, both fixed: - The version:null the previous pass stopped on: registering a plugin without pinning `version` auto-detects it (confirmed via `bao plugin list -detailed`), but either way OpenBao files the catalog entry under name *and* version. A bare-name GET 404s regardless — its JSON body has no "data" key at all, and indexing a missing key with serde_json::Value reads as Null, which is what surfaced as "version: null" rather than a 404 status. Fixed by pinning `version` on the PUT and reading it back with `?version=` — confirmed against bao's own HTTP responses. - `sys/mounts/identity` collided with the always-present built-in identity/ mount ("cannot mount \"identity/\""), confirmed in bao's server log. Renamed the plugin's mount (and every subsequent lease path) to vedanta-identity/. With both fixed, registration, the catalog read-back, and the mount all pass. The walk still fails at ISSUE: SystemView.ResponseWrapData answers "no namespace" for any out-of-process plugin (see the doc comment above the test and the 2026-09-06 report addendum for the source-verified root cause and the wrap redesign it needs), so the test stays #[ignore]d with that specific, sourced reason rather than the old speculative one.Progress, and one upstream finding that changes the design.
Two commits pushed (
bf6f0fc,e7577c6). The live walk against a realbaov2.6.2 now gets through registration, catalog read-back and mount, which it did not before.The two test bugs, both real
data.version: nullwas a 404 on a bare-name catalog GET. A versioned catalog entry must be read back with?version=. Confirmed againstbao's own HTTP responses andbao plugin info— the earlier report's "registration succeeded but the version is null" was the wrong reading of the symptom.sys/mounts/identitycollides with OpenBao's built-inidentity/mount. The test now mounts atvedanta-identity/.The finding
The walk now fails at ISSUE with
response wrapping refused error=storage: no namespace, and the cause is upstream, not here:SystemView.ResponseWrapDatais unusable from an out-of-process plugin. The gRPC connection the plugin dials back over never carries a namespace-scoped context, andjwt: truedoes not route around it — the server hardcodesjwt=false. Read from OpenBao's own Go source at the v2.6.2 tag and on currentmain, not inferred from the error string.So the plugin cannot wrap by calling OpenBao. It must instead set the wrap TTL on its
Responseand let core wrap, using the correctly-scoped original request context. That is arguably the shape the contract wanted all along — OpenBao owns delivery — but it is a real redesign acrossissue.rs,lease.rs,stores.rs,backend.rsand about fifteen unit tests, including whateverWrapStorebecomes. In progress.Discipline worth noting for the reviewer
bf6f0fcis a reverted hypothesis, kept in the history: the worker suspectedHandleRequestArgs.storage_idhad to be redialled per request, tested it against a realbaowith a broker trace, found OpenBao only ever announces one broker connection, confirmed it againstsdk/plugin/grpc_backend_server.go, and reverted to the original design. The commit records the disproof rather than hiding it.State: build and clippy clean, 203 tests passing, unchanged. The acceptance test is re-
#[ignore]d with a source-backed reason replacing the earlier speculative one.SystemView.ResponseWrapData is confirmed unusable from an out-of-process plugin: the broker-served SystemView connection carries no namespace in its context (sdk/plugin/grpc_backend_client.go's Setup serves it over a bare grpc.NewServer with no interceptor), and dynamicSystemView.ResponseWrapData fails outright without one. The corrected reading of contracts.identity.openbao-lease-backend's one-shot clause holds OpenBao's own barrier-encrypted lease storage inside the trust boundary — it is where every stock secrets engine's renew/revoke data already lives — so the credential may ride Response.Data on its way into a Secret-bearing answer as long as OpenBao's core, not this plugin, performs the wrap. WrapStore is gone. IssueOutcome::Issued and RotateOutcome::Rotated now carry the mint's credential directly, wrapped in a Credential newtype whose Debug always prints "Credential(REDACTED)" so no {:?} of an outcome can spell it. backend.rs::reply_for sets Response.wrap_info.TTL on every lease-creating answer; OpenBao's own request_handling.go wraps the whole response — Secret included — over the ORIGINAL HTTP request's own, correctly namespace-scoped context, after having already registered the lease from the same Secret/Data. The immediate, caller-visible reply to issue/rotate/renew now shows only wrap_info; lease_id/credential/renewable surface once, at sys/wrapping/unwrap. Two further, previously-unreached bugs surfaced once a real Secret-bearing answer round-tripped through a live bao v2.6.2 for the first time: - LeaseOptions.issue_time was always None. The Go SDK's ProtoLeaseOptionsToLogicalLeaseOptions calls IssueTime.CheckValid() unconditionally and fails the WHOLE response ("invalid nil Timestamp") when it is absent. Answer::Lease now carries issued_at_unix and reply_for sets a real timestamp; the value is otherwise unused downstream (expiration.go's Register computes the lease's actual TTL with a bare time.Time{}). - lease_manager_revoke ran fault_from_raw over EVERY answer from the plugin's own revoke, including a completed {"state":"revoked",...} success — fault_from_raw reads an errors[0].code a success body does not have, defaulted to the literal string "error", and turned every completed revoke into a bogus Fault. fault_from_raw now only wraps an actual denial; a completed or still-pending revoke is read directly. storage.rs keeps SystemView and response_wrap_data as vendored, tested, confirmed-unused protocol surface, with a doc comment tracing exactly why it cannot work rather than removing real vendored surface. Co-Authored-By: Claude Opus 5 <[email protected]>The live walk passes, un-ignored. A real
bao server -devv2.6.2 loads this plugin from its catalog, mounts it, and drives register → mount → issue → unwrap → read → rotate → renew → revoke end to end. Two commits pushed (23012ab,fa96828).Verified independently by the queen:
cargo test --workspace— 128 + 71 + 1 + 3 doctests + 1 acceptance test, nothing ignored,test a_dev_openbao_mounts_the_plugin_and_walks_the_lease_end_to_end ... ok. Clippy-D warningsclean.That also settles the open question in this PR's description: the handshake constants were never confirmed against Go source, and a successful live handshake is that confirmation.
baospawns the binary and speaks to it.The two source facts this rests on
vault/request_handling.go'shandleRequestregisters the lease — persistingresp.Datathroughexpiration.go'sRegister()— beforeswitchedLockHandleRequestcallswrapInCubbyhole. So the credential is written into OpenBao's own lease storage. That does not evaporate; it is accepted, see below.Response.Secretis the only gate for OpenBao owning renew and revoke.Response.Dataneed not carry the credential — but the only ways to deliver a value once without core's own wrap are the confirmed-brokenSystemView.ResponseWrapDataor a plugin-owned cubbyhole, and the freeholder ruled the plugin must hold nothing. So nativewrap_infois the standards-track path.The contract reading this depends on — a reviewer should confirm it
An earlier pass reverted this exact design because a brief said the value must never be "in storage". That phrasing was the queen's, and stricter than the contract.
credential-material-is-one-shotbinds Vedanta: "Vedanta MAY carry credential material only in the immediate successful result to OpenBao. It MUST NOT persist the value…" Acceptance I04's negative names status, logs, traces, arguments, environment dumps, Git and issue text. OpenBao's barrier-encrypted lease storage is in neither list, and holding dynamic-secret data in the lease entry is how Vault-family engines work — it is what revocation later reads.Freeholder preference (Larandar, 2026-09-06): "the plugin should not hold the credentials but only make them available in OpenBao." This is that.
What landed
WrapStoreis gone. The credential rides in aCredentialnewtype with a redactingDebug, andbackend.rs::reply_forsetswrap_info.TTLof 300 s on every lease-creating answer; core wraps. Three properties are proved by test rather than asserted: the plugin persists no credential value anywhere of its own, the wrapper is one-shot with consumed/expired/unknown indistinguishable, and one operation ID mints at most once.Two real bugs surfaced only once a
Secret-bearing answer round-tripped for the first time — a nilissue_timethat failed every lease-creating response, andlease_manager_revoketurning every completed revoke into a bogus fault. Both fixed; neither was reachable before.Still for a reviewer
credential_datafield shape against the workload client's real expectations on flake-ops#479, which currently posts tosys/wrapping/unwrap.SystemView::response_wrap_datashould remain as vendored, documented-unusable surface.Verdict: changes required.
openbao-plugin/src/backend.rs:1107–1112: the request loads a record, awaits native invalidation, then saves the whole old record.answer()holds only a sharedRwLockread guard. A concurrent rotate/revoke can commit during that await and then be overwritten, losing the successor or restoring Active. Serialize per-lease transitions or use conditional transactional updates.openbao-plugin/src/backend.rs:749: concurrent issue requests with distinct operation IDs can read the same active count and all pass before any commits. Reserve per-claimant capacity atomically, including pending issues.openbao-plugin/src/backend.rs:675–677: read/revoke discard the request identity, unlike issue/rotate. A different entity with wildcard lifecycle ACL access can revoke another claimant’s known lease. Bind explicit operations to the stored claimant while separately authorizing OpenBao lease-manager cleanup.Integration blocker with flake-ops#479: this PR returns native
wrap_info, while that client requires obsolete flat wrapper fields. Its unwrap also stores OpenBao’s top-level lease ID instead of the plugin’sdata.lease_id. The real bao test in this PR distinguishes both IDs. Findings are posted on the client PR too.Validation rerun:
nix develop --command cargo test --workspacepassed, including the named bao-dev acceptance test, 128 root library tests, 71 plugin tests, 1 binary test and 3 doctests (plus workspace helper tests). The test runner reported no ignored tests. A separate verbose acceptance rerun was blocked by Nix copying a transient build-output path; therefore this review does not use that rerun as additional live evidence. Clippy rerun was still in progress when this review was prepared; the PR’s earlier clippy claim is not substituted for a completed rerun.Reviewed commit:
fa9682845045d4f35e2a8e53e7d14364b8675b9e.Agent review performed by Codex at the user’s request. Posted through the PR author’s account; this records review evidence, not an independent collaborator approval or a new owner ruling.
Review verification update: the independent
nix develop --command cargo clippy --workspace --all-targets -- -D warningsrun completed successfully. The review findings and changes-required verdict remain unchanged.fa968284's review (P2): read and explicit revoke discarded the request's identity entirely, unlike issue and rotate — an entity holding a wildcard lifecycle ACL (e.g. lease/+/{read,revoke}) but no issue authority on any specific role could read or revoke a lease it never touched, given a known lease id. Bind both to the entity the lease was minted or last rotated under (record.claims.caller_entity_id), denying a mismatch with the same unknown_lease a nonexistent id gets — never a distinct "not yours" that would confirm the id exists. OpenBao's own lease manager is deliberately NOT routed through this check: lease_manager_revoke calls the (still unbound) revoke() directly, since that path is authorized by construction — it can only ever name a lease id OpenBao itself is holding a lease against — rather than by a caller identity there is none of. The new test mints a lease under the mount's one trusted caller entity, then shows a different entity is denied read and revoke on it (unknown_lease, same as a nonexistent id), while the bound caller's own read and revoke still succeed. Reverting the check makes the denial assertions fail: the impostor's read and revoke both succeed. Co-Authored-By: Teyla <[email protected]>All three findings fixed.
8e3117c,7a796ac,70d684d, pushed on top offa968284. Write surface wasopenbao-plugin/src/backend.rsalone;Cargo.lockuntouched — no new dependency,tokio::sync::Mutexwas already there.236 tests, 0 failed, plugin lib up from 71 to 74, and the live acceptance test against a real
bao server -devstayed un-ignored and green throughout. Re-run independently by the queen after the push.The mechanisms
1 — per-lease serialization. A
KeyedLocksprimitive keyed by lease id, itsOwnedMutexGuardheld across awaits, wrapping the wholeRotate/Revoke/LeaseManagerRenew/LeaseManagerRevokebranch. That is what makesretire_predecessor's load-dispatch-save atomic against a concurrent transition of the same lease.2 — the ceiling is now reserved, not merely read. A second
KeyedLocks, keyed by claimant uuid, held from immediately after the count read through the commit, in bothissue()androtate(). Issue-versus-issue and issue-versus-rotate for one claimant now contend.3 — explicit operations are bound to the stored claimant.
read()and a newexplicit_revoke()deny unless the caller matchesrecord.claims.caller_entity_id, answering the sameunknown_lease404 an absent id gets — a stranger learns nothing about whether the lease exists. The barerevoke()remains reachable only fromlease_manager_revoke, which is OpenBao's own cleanup: authorized by construction, with no caller identity to check.The tests are real interleavings, not shape assertions
Each drives
PluginBackend::answer()over a brokeredFakeStorageand a hand-rolled Vedanta double on a unix socket, forcing the interleaving with anarrived/gorendezvous plus a bounded 300 ms timeout, so "ran straight through" is distinguishable from "genuinely blocked". Each fix was manually reverted and its test re-run to confirm the failure: the successor is lost to a stale generation; two concurrent issues both succeed under a ceiling of one; the impostor's read returns 200 instead of 404. Stable across five repeated runs.Four things a reviewer must check
KeyedLocksnever reaps entries. Fine at pilot scale; worth a sign-off if lease or claimant cardinality assumptions change.caller_entity_idwhile old leases exist — confirm that is the intended reading.