Uh oh!
There was an error while loading. Please reload this page.
feat: enforce per-method effect/network policy on the Solution contract (#289) - #294
Merged
antoinetoussaint-byte merged 6 commits intoAug 16, 2026
Merged
Conversation
…ct (#289) Binds a host-enforceable solution_method_policy extension to every Solution RPC (mirroring provider_method_policy) and adds the host-side dispatch gate that reads and enforces it. - Proto: SolutionNetworkMode + SolutionEffect (severity-ordered) and the solution_method_policy method option; annotate Create/Update/Package/Render and GetSolutionInformation with their true network reach and state effect. - Host: a unary client interceptor (solution.EnforcingClientInterceptor) installed on every agent connection refuses to dispatch a Solution RPC whose declared policy exceeds the per-call ceiling stamped via solution.WithCeiling; non-Solution calls pass through untouched. - The gate is honest about scope: it constrains what the host invokes, not what a plugin does inside a handler — the Solution contract has no host-brokered callback path, so unmediated writes are out of scope by design. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review of the enforcement gate surfaced that its security value rested on
unconstrained caller discipline:
- The ceiling was a struct with exported fields, so any caller could hand-
assemble an over-broad or incoherent ceiling (e.g. registry network with
read-only effect) and silently widen its own privilege — the decorative-
annotation smell one step removed. Unexport the fields and expose only named
operation-intent constructors (CeilingInspect/CeilingScaffold/CeilingPublish),
so a caller declares the operation it performs and the intent→ceiling mapping
is a single audited, tested chokepoint. Incoherent ceilings are now
unconstructible.
- The no-ceiling denial ("no operation ceiling on context") read like an auth
failure and hid the real requirement. Make the message name solution.WithCeiling
as the fix so the first integrator debugs the ceiling, not the token.
- Match the test dial to production by using WithChainUnaryInterceptor.
Adds TestOperationCeilingsAdmitExactlyTheirRPCs pinning that each operation
ceiling admits exactly its intended RPC set.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>Unexporting Ceiling's fields left CeilingFrom exported but useless to any external caller: it is consumed only by the interceptor, and a caller can no longer read the returned ceiling's bounds. Unexport it so the package surface reflects the single-chokepoint design (only WithCeiling is public). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The finding-2 fix changed the denial message to name solution.WithCeiling so a missing ceiling is not mistaken for an auth failure, but the test asserted only the PermissionDenied code — a regression to a bare, confusing message would have passed. Assert the message contains the remedy, which the original message did not, so the behavior the fix introduced is now guarded. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three review items the prior rounds had deferred, now fixed: - Ceiling provenance / dead public surface: PolicyFor and Admits were exported but had no production caller — only the interceptor and tests used them, the same dead-surface class as CeilingFrom. Unexport both to policyFor/admits and move their unit tests into an internal (package solution) test file, so the public surface is just the interceptor, the ceiling constructors, WithCeiling, and the typed Client. - Read-only calls no longer require an explicit ceiling: a missing ceiling now defaults to the least-privilege ceiling (CeilingInspect) instead of denying every call. A caller that never declared its operation can still read a solution executor's advertisement, while every mutating RPC stays fail-closed until the host declares a higher ceiling. This is least-privilege-by-default, not a permissive default — the minimum real ceiling, not "allow". - Unforgeable ceiling obligation: add the typed solution.Client whose every method takes a Ceiling argument, so host code cannot dispatch a Solution RPC without declaring the operation it performs — the obligation is type-level rather than a convention a caller can forget. Tests: internal policyFor/admits/ceiling unit tests; wire tests now prove the least-privilege default (read admitted unstamped, mutation denied with the remedy named) and that the typed Client enforces the ceiling per call. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
RenderRequest carries only artifact_reference (a string OCI reference) and a local destination — no artifact bytes — and a solution executor's network is unmediated (Package pushes directly). So Render must pull the packaged artifact from the registry itself: its true maximum network reach is a registry read, not OFFLINE. Annotating it OFFLINE understated it and left a real hole — an offline ceiling admitted a Render that dials the network. The two-value network enum could not express this, so add SOLUTION_NETWORK_MODE_REGISTRY_READ between OFFLINE and REGISTRY_WRITE (a wire-breaking renumber, safe: the contract has no consumers) and re-annotate Render's network as REGISTRY_READ. Add CeilingRender as the least-privilege ceiling that admits Render's pull without admitting Package's push; CeilingScaffold (offline) no longer admits Render, which is the correct consequence — a purely offline operation must not trigger a registry pull. Tests updated: contract policy pin, enum ordering, and the operation-ceiling matrix (Render moves from scaffold to the new render tier); the coherence-test comment now describes the registry-write pairing it actually guards. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Uh oh!
There was an error while loading. Please reload this page.
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes#289.
Summary
solution_method_policyto every Solution RPC (mirroring provider'sprovider_method_policy): severity-orderedSolutionNetworkMode+SolutionEffect, with each RPC annotated with its true network reach and state effect.Packagedeclares registry-write;Create/Update/Renderdeclare local-write;GetSolutionInformationis offline/read-only.solution.EnforcingClientInterceptor(installed on every agent connection inagents/manager.Load) refuses to dispatch a Solution RPC whose declared policy exceeds the per-call ceiling stamped viasolution.WithCeiling, and fails closed when no ceiling is present. Non-Solution calls pass through untouched, so it is safe on every agent connection.provider.proto(whereProviderHostbrokers side effects), the Solution contract defines no host-brokered callback path, so a plugin's unmediated filesystem/registry writes are out of scope by design — documented insolution/policy.go.This carries forward the design reviewed and approved in #292, which was merged into the now-superseded
issue-287…feature branch and never reachedmain; #289 remained open. This PR re-targets that reviewed work atmain.Test plan
go build ./...go test ./solution/...— machine-enforceable policy pin, unary-only-contract invariant, axis coherence,PolicyForresolution,Admitsfail-closed matrix, interceptor denies-over-ceiling-before-the-wire, interceptor passes through non-Solution calls, enum ceiling orderinggo test ./agents/ ./agents/manager/codefly generate proto --localreproduces the committed generated code (unrelated protoc-gen-go-grpc version churn reverted)