From 5031853b3ee1a6df11ce9feab1d603540997a488 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint Date: Sun, 16 Aug 2026 19:28:16 -0400 Subject: [PATCH 1/3] refactor: collapse the Solution two-API split onto solution.Client (#295) The typed solution.Client makes the operation ceiling a required, type-level argument, but the exported WithCeiling let a caller stamp a ceiling onto a raw solutionv0.SolutionClient and bypass that guarantee. Unexport WithCeiling so Client is the single canonical dispatch path: a caller holding the raw client can no longer reach any ceiling above the least-privilege default, so every mutating RPC fails closed. Co-Authored-By: Claude Opus 4.8 --- agents/manager/loader.go | 2 +- solution/policy.go | 52 +++++++++++++++++++++++----------------- solution/policy_test.go | 31 ++++++++++-------------- 3 files changed, 44 insertions(+), 41 deletions(-) diff --git a/agents/manager/loader.go b/agents/manager/loader.go index 0c2e9326..377c5da2 100644 --- a/agents/manager/loader.go +++ b/agents/manager/loader.go @@ -1031,7 +1031,7 @@ func Load(ctx context.Context, p *resources.Agent, opts ...LoadOption) (*AgentCo grpc.WithPerRPCCredentials(bearerCreds{token: authToken}), // Host-side dispatch gate for the Solution contract: refuses to send a // Solution RPC whose declared effect/network policy exceeds the ceiling - // stamped on the call context (solution.WithCeiling). No-ops for every + // stamped on the call context by solution.Client. No-ops for every // other service, so it is safe on every agent connection. grpc.WithChainUnaryInterceptor(solution.EnforcingClientInterceptor()), grpcconfig.TypedMessageClientDialOption(), diff --git a/solution/policy.go b/solution/policy.go index 85ac0317..89c54a75 100644 --- a/solution/policy.go +++ b/solution/policy.go @@ -82,12 +82,19 @@ func CeilingPublish() Ceiling { } } -// Client is the host-side Solution client that makes the operation ceiling a -// required argument of every call, so host code cannot dispatch a Solution RPC -// without declaring the operation it performs — the obligation is type-level, not -// a convention a caller can forget. Each method stamps the ceiling with -// WithCeiling and delegates to the generated client; EnforcingClientInterceptor, -// installed on the connection, is what actually gates the call. +// Client is the canonical host-side Solution client and the only supported way to +// dispatch a Solution RPC. It makes the operation ceiling a required argument of +// every call, so host code cannot dispatch a Solution RPC without declaring the +// operation it performs — the obligation is type-level, not a convention a caller +// can forget. Each method stamps the ceiling onto the context and delegates to the +// generated client; EnforcingClientInterceptor, installed on the connection, is +// what actually gates the call. +// +// The raw generated solutionv0.SolutionClient is deliberately not a second entry +// point: the context stamp is unexported (withCeiling), so a caller holding the +// raw client can only ever reach the least-privilege default ceiling and every +// mutating RPC fails closed. That leaves this type as the single path that can +// dispatch anything beyond the read-only advertisement. type Client struct { inner solutionv0.SolutionClient } @@ -100,40 +107,41 @@ func NewClient(conn grpc.ClientConnInterface) *Client { // GetSolutionInformation reads a solution executor's advertisement. func (c *Client) GetSolutionInformation(ctx context.Context, ceiling Ceiling, in *solutionv0.GetSolutionInformationRequest, opts ...grpc.CallOption) (*solutionv0.GetSolutionInformationResponse, error) { - return c.inner.GetSolutionInformation(WithCeiling(ctx, ceiling), in, opts...) + return c.inner.GetSolutionInformation(withCeiling(ctx, ceiling), in, opts...) } // Create scaffolds a new solution into a destination directory. func (c *Client) Create(ctx context.Context, ceiling Ceiling, in *solutionv0.CreateRequest, opts ...grpc.CallOption) (*solutionv0.CreateResponse, error) { - return c.inner.Create(WithCeiling(ctx, ceiling), in, opts...) + return c.inner.Create(withCeiling(ctx, ceiling), in, opts...) } // Update reconciles an existing solution source with the executor's template. func (c *Client) Update(ctx context.Context, ceiling Ceiling, in *solutionv0.UpdateRequest, opts ...grpc.CallOption) (*solutionv0.UpdateResponse, error) { - return c.inner.Update(WithCeiling(ctx, ceiling), in, opts...) + return c.inner.Update(withCeiling(ctx, ceiling), in, opts...) } // Package builds an OCI artifact from a solution source directory and pushes it. func (c *Client) Package(ctx context.Context, ceiling Ceiling, in *solutionv0.PackageRequest, opts ...grpc.CallOption) (*solutionv0.PackageResponse, error) { - return c.inner.Package(WithCeiling(ctx, ceiling), in, opts...) + return c.inner.Package(withCeiling(ctx, ceiling), in, opts...) } // Render renders a packaged solution's manifests into a gitops destination. func (c *Client) Render(ctx context.Context, ceiling Ceiling, in *solutionv0.RenderRequest, opts ...grpc.CallOption) (*solutionv0.RenderResponse, error) { - return c.inner.Render(WithCeiling(ctx, ceiling), in, opts...) + return c.inner.Render(withCeiling(ctx, ceiling), in, opts...) } type ceilingContextKey struct{} -// WithCeiling stamps the ceiling admitted for the current operation onto a -// context. Pass a ceiling from one of the operation constructors -// (CeilingInspect/CeilingScaffold/CeilingPublish). The host sets it per call -// because one solution-agent connection is long-lived and reused across -// operations (see agents/manager.loader: AgentConn.GRPCConn), so the ceiling -// belongs to the call, not the dial. A Solution RPC issued without a ceiling is +// withCeiling stamps the ceiling admitted for the current operation onto a +// context. It is unexported so Client is the only way to stamp one: the host sets +// it per call because one solution-agent connection is long-lived and reused +// across operations (see agents/manager.loader: AgentConn.GRPCConn), so the +// ceiling belongs to the call, not the dial — but that per-call stamp is Client's +// job, not something a caller assembles by hand. A Solution RPC issued without a +// ceiling (i.e. through the raw generated client, which cannot reach this) is // gated against the least-privilege ceiling by EnforcingClientInterceptor, so // only the read-only advertisement call succeeds unstamped. -func WithCeiling(ctx context.Context, ceiling Ceiling) context.Context { +func withCeiling(ctx context.Context, ceiling Ceiling) context.Context { return context.WithValue(ctx, ceilingContextKey{}, ceiling) } @@ -151,7 +159,7 @@ func ceilingFrom(ctx context.Context) (Ceiling, bool) { // reports whether the method belongs to the Solution service; a Solution method // with no annotation returns (nil, true) so callers fail closed. It is // unexported: only the interceptor consults policies; a host uses the interceptor -// plus WithCeiling, never the policy lookup directly. +// plus Client, never the policy lookup directly. func policyFor(fullMethod string) (*solutionv0.SolutionMethodPolicy, bool) { method := methodDescriptor(fullMethod) if method == nil { @@ -197,7 +205,7 @@ func admits(policy *solutionv0.SolutionMethodPolicy, ceiling Ceiling) error { // EnforcingClientInterceptor is the host-side dispatch gate: a unary client // interceptor installed on every agent connection (agents/manager.loader). For // each outgoing Solution RPC it reads the declared policy and the ceiling -// stamped on the call context (see WithCeiling) and refuses to dispatch a call +// stamped on the call context (see Client) and refuses to dispatch a call // whose declared network or effect exceeds the admitted ceiling. Calls to // services other than Solution pass through untouched, so installing it // universally does not affect non-solution agents. @@ -205,7 +213,7 @@ func admits(policy *solutionv0.SolutionMethodPolicy, ceiling Ceiling) error { // A call with no ceiling on its context is admitted against the least-privilege // ceiling (CeilingInspect): a caller that never declared its operation may still // read a solution executor's advertisement, but every mutating RPC is refused -// until the host declares a higher ceiling with WithCeiling. Defaulting to the +// until the host declares a higher ceiling through Client. Defaulting to the // minimum — rather than denying even the harmless read — keeps inspection // ergonomic while staying fail-closed for every effectful RPC. // @@ -227,7 +235,7 @@ func EnforcingClientInterceptor() grpc.UnaryClientInterceptor { if !explicit { return status.Errorf(codes.PermissionDenied, "solution method %s denied under the default least-privilege ceiling: %v; "+ - "declare this operation's ceiling with solution.WithCeiling", + "dispatch it through solution.Client (solution.NewClient), which requires an operation ceiling", method, err) } return status.Errorf(codes.PermissionDenied, "solution method %s denied: %v", method, err) diff --git a/solution/policy_test.go b/solution/policy_test.go index f4b1bbd3..8cb6e3f0 100644 --- a/solution/policy_test.go +++ b/solution/policy_test.go @@ -97,7 +97,13 @@ func (s *recordingSolutionServer) Package(context.Context, *solutionv0.PackageRe return &solutionv0.PackageResponse{}, nil } -func TestEnforcingClientInterceptorDeniesOverCeilingBeforeTheWire(t *testing.T) { +// TestEnforcingClientInterceptorDefaultsToLeastPrivilege pins the fail-closed +// behavior seen by a caller holding the raw generated client — the path that can +// no longer stamp a ceiling now that Client is the single canonical entry point. +// Such a call is gated at the least-privilege CeilingInspect, so only the +// read-only advertisement succeeds and every mutating RPC is denied before the +// wire with a message that names the remedy. +func TestEnforcingClientInterceptorDefaultsToLeastPrivilege(t *testing.T) { server := &recordingSolutionServer{handled: map[string]int{}} listener := bufconn.Listen(1 << 20) grpcServer := grpc.NewServer() @@ -116,31 +122,20 @@ func TestEnforcingClientInterceptorDeniesOverCeilingBeforeTheWire(t *testing.T) client := solutionv0.NewSolutionClient(conn) - ctx := solution.WithCeiling(context.Background(), solution.CeilingScaffold()) - - // Create is at the ceiling — admitted and reaches the server. - _, err = client.Create(ctx, &solutionv0.CreateRequest{}) - require.NoError(t, err) - require.Equal(t, 1, server.handled["Create"]) - - // Package exceeds the ceiling — denied before crossing the wire. - _, err = client.Package(ctx, &solutionv0.PackageRequest{}) - require.Equal(t, codes.PermissionDenied, status.Code(err)) - require.Equal(t, 0, server.handled["Package"]) - // No ceiling on the context defaults to least privilege: the read-only // advertisement call is admitted and reaches the server... _, err = client.GetSolutionInformation(context.Background(), &solutionv0.GetSolutionInformationRequest{}) require.NoError(t, err) require.Equal(t, 1, server.handled["GetSolutionInformation"]) - // ...but a mutating RPC without a ceiling is still denied before the wire, - // and the denial names the remedy so it is not mistaken for an auth failure: - // the missing ceiling, not the token, is what the caller must fix. + // ...but a mutating RPC without a ceiling is denied before the wire, and the + // denial names the remedy so it is not mistaken for an auth failure: the + // missing ceiling — declared by routing through solution.Client — not the + // token, is what the caller must fix. _, err = client.Create(context.Background(), &solutionv0.CreateRequest{}) require.Equal(t, codes.PermissionDenied, status.Code(err)) - require.Equal(t, 1, server.handled["Create"]) - require.Contains(t, status.Convert(err).Message(), "solution.WithCeiling") + require.Equal(t, 0, server.handled["Create"]) + require.Contains(t, status.Convert(err).Message(), "solution.Client") } // TestClientRequiresCeilingPerCall proves the typed Client makes the ceiling a From eaa1cf2e90aab6cdf9751b5f7129f07054db2b74 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint Date: Sun, 16 Aug 2026 20:03:10 -0400 Subject: [PATCH 2/3] docs: make Client's canonical-path claim precise about read-only calls (#295) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Client doc asserted it was "the only supported way to dispatch a Solution RPC," which overclaimed: the raw generated client can still dispatch the read-only GetSolutionInformation under the interceptor's least-privilege default ceiling, and the very next paragraph already qualified the claim to "anything beyond the read-only advertisement." Reconcile the first sentence with that reality — Client is the only path for effectful RPCs; the read stays reachable via the raw client by design — so the invariant isn't read as stronger than it is. Co-Authored-By: Claude Opus 4.8 --- solution/policy.go | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/solution/policy.go b/solution/policy.go index 89c54a75..8620f2db 100644 --- a/solution/policy.go +++ b/solution/policy.go @@ -82,19 +82,20 @@ func CeilingPublish() Ceiling { } } -// Client is the canonical host-side Solution client and the only supported way to -// dispatch a Solution RPC. It makes the operation ceiling a required argument of -// every call, so host code cannot dispatch a Solution RPC without declaring the -// operation it performs — the obligation is type-level, not a convention a caller -// can forget. Each method stamps the ceiling onto the context and delegates to the -// generated client; EnforcingClientInterceptor, installed on the connection, is -// what actually gates the call. +// Client is the canonical host-side Solution client and the only path that can +// dispatch an effectful Solution RPC. It makes the operation ceiling a required +// argument of every call, so host code cannot dispatch a Solution RPC without +// declaring the operation it performs — the obligation is type-level, not a +// convention a caller can forget. Each method stamps the ceiling onto the context +// and delegates to the generated client; EnforcingClientInterceptor, installed on +// the connection, is what actually gates the call. // // The raw generated solutionv0.SolutionClient is deliberately not a second entry -// point: the context stamp is unexported (withCeiling), so a caller holding the -// raw client can only ever reach the least-privilege default ceiling and every -// mutating RPC fails closed. That leaves this type as the single path that can -// dispatch anything beyond the read-only advertisement. +// point for effectful calls: the context stamp is unexported (withCeiling), so a +// caller holding the raw client can only ever reach the least-privilege default +// ceiling — enough for the read-only advertisement, which stays reachable that way +// by design — while every mutating RPC fails closed. That leaves this type as the +// single path that can dispatch anything beyond that read. type Client struct { inner solutionv0.SolutionClient } From f618150cfd1662fdfef7abe8e5ec854476be85a2 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint Date: Sun, 16 Aug 2026 20:10:08 -0400 Subject: [PATCH 3/3] fix: make solution.Client enforce the ceiling itself, not via the dial (#295) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Client's fail-closed guarantee hung on an invariant it could not check: that the connection was dialed with EnforcingClientInterceptor. A Client built over a plain connection stamped the ceiling onto a context nobody read and dispatched every RPC — including Package — unchecked. NewClient takes any grpc.ClientConnInterface and cannot introspect a connection's interceptor chain, so the precondition was silent. Enforce the ceiling inside each Client method, before dispatch, reusing the same policyFor/admits check and PermissionDenied shape as the interceptor. The guarantee is now intrinsic to Client regardless of how the connection was dialed; the interceptor stays as defense in depth and to gate callers that reach for the raw generated client. Client still stamps the ceiling so the interceptor, when present, admits the same call instead of defaulting it to least privilege. Co-Authored-By: Claude Opus 4.8 --- solution/policy.go | 48 ++++++++++++++++++++++++++++++++++++----- solution/policy_test.go | 36 +++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 5 deletions(-) diff --git a/solution/policy.go b/solution/policy.go index 8620f2db..a1e219d7 100644 --- a/solution/policy.go +++ b/solution/policy.go @@ -86,9 +86,16 @@ func CeilingPublish() Ceiling { // dispatch an effectful Solution RPC. It makes the operation ceiling a required // argument of every call, so host code cannot dispatch a Solution RPC without // declaring the operation it performs — the obligation is type-level, not a -// convention a caller can forget. Each method stamps the ceiling onto the context -// and delegates to the generated client; EnforcingClientInterceptor, installed on -// the connection, is what actually gates the call. +// convention a caller can forget. +// +// Each method checks the method's declared policy against the ceiling before it +// dispatches, so the guarantee is intrinsic to Client and does not depend on the +// connection having been dialed with EnforcingClientInterceptor: a Client built +// over a plain connection still refuses an over-ceiling call before the wire. It +// also stamps the ceiling onto the context so that interceptor — installed on +// every agent connection (agents/manager.Load) as defense in depth and to gate +// callers that bypass Client — admits the same call rather than defaulting it to +// least privilege. // // The raw generated solutionv0.SolutionClient is deliberately not a second entry // point for effectful calls: the context stamp is unexported (withCeiling), so a @@ -100,37 +107,68 @@ type Client struct { inner solutionv0.SolutionClient } -// NewClient wraps a connection whose dial installed EnforcingClientInterceptor -// (every agent connection from agents/manager.Load does). +// NewClient wraps a connection with the typed, ceiling-enforcing Solution client. +// The connection need not carry EnforcingClientInterceptor — Client enforces the +// ceiling itself — though every agent connection from agents/manager.Load installs +// it anyway to gate any caller that reaches for the raw generated client. func NewClient(conn grpc.ClientConnInterface) *Client { return &Client{inner: solutionv0.NewSolutionClient(conn)} } // GetSolutionInformation reads a solution executor's advertisement. func (c *Client) GetSolutionInformation(ctx context.Context, ceiling Ceiling, in *solutionv0.GetSolutionInformationRequest, opts ...grpc.CallOption) (*solutionv0.GetSolutionInformationResponse, error) { + if err := enforce(solutionv0.Solution_GetSolutionInformation_FullMethodName, ceiling); err != nil { + return nil, err + } return c.inner.GetSolutionInformation(withCeiling(ctx, ceiling), in, opts...) } // Create scaffolds a new solution into a destination directory. func (c *Client) Create(ctx context.Context, ceiling Ceiling, in *solutionv0.CreateRequest, opts ...grpc.CallOption) (*solutionv0.CreateResponse, error) { + if err := enforce(solutionv0.Solution_Create_FullMethodName, ceiling); err != nil { + return nil, err + } return c.inner.Create(withCeiling(ctx, ceiling), in, opts...) } // Update reconciles an existing solution source with the executor's template. func (c *Client) Update(ctx context.Context, ceiling Ceiling, in *solutionv0.UpdateRequest, opts ...grpc.CallOption) (*solutionv0.UpdateResponse, error) { + if err := enforce(solutionv0.Solution_Update_FullMethodName, ceiling); err != nil { + return nil, err + } return c.inner.Update(withCeiling(ctx, ceiling), in, opts...) } // Package builds an OCI artifact from a solution source directory and pushes it. func (c *Client) Package(ctx context.Context, ceiling Ceiling, in *solutionv0.PackageRequest, opts ...grpc.CallOption) (*solutionv0.PackageResponse, error) { + if err := enforce(solutionv0.Solution_Package_FullMethodName, ceiling); err != nil { + return nil, err + } return c.inner.Package(withCeiling(ctx, ceiling), in, opts...) } // Render renders a packaged solution's manifests into a gitops destination. func (c *Client) Render(ctx context.Context, ceiling Ceiling, in *solutionv0.RenderRequest, opts ...grpc.CallOption) (*solutionv0.RenderResponse, error) { + if err := enforce(solutionv0.Solution_Render_FullMethodName, ceiling); err != nil { + return nil, err + } return c.inner.Render(withCeiling(ctx, ceiling), in, opts...) } +// enforce refuses an over-ceiling dispatch from Client itself, so the ceiling +// guarantee holds even on a connection whose dial did not install +// EnforcingClientInterceptor. It mirrors the interceptor's over-ceiling denial +// (same PermissionDenied, same message shape); admits fails closed on a missing or +// unspecified policy, which cannot occur for Client's own annotated Solution +// methods but keeps the check total. +func enforce(fullMethod string, ceiling Ceiling) error { + policy, _ := policyFor(fullMethod) + if err := admits(policy, ceiling); err != nil { + return status.Errorf(codes.PermissionDenied, "solution method %s denied: %v", fullMethod, err) + } + return nil +} + type ceilingContextKey struct{} // withCeiling stamps the ceiling admitted for the current operation onto a diff --git a/solution/policy_test.go b/solution/policy_test.go index 8cb6e3f0..7d531b12 100644 --- a/solution/policy_test.go +++ b/solution/policy_test.go @@ -173,6 +173,42 @@ func TestClientRequiresCeilingPerCall(t *testing.T) { require.Equal(t, 0, server.handled["Package"]) } +// TestClientEnforcesWithoutDialInterceptor proves Client's ceiling guarantee is +// intrinsic, not borrowed from the connection: even on a dial that installed NO +// EnforcingClientInterceptor, an over-ceiling RPC is refused before the wire. +// Without Client's own check, the ceiling stamp would land on a context nobody +// reads and every RPC would dispatch unchecked. +func TestClientEnforcesWithoutDialInterceptor(t *testing.T) { + server := &recordingSolutionServer{handled: map[string]int{}} + listener := bufconn.Listen(1 << 20) + grpcServer := grpc.NewServer() + solutionv0.RegisterSolutionServer(grpcServer, server) + go func() { _ = grpcServer.Serve(listener) }() + t.Cleanup(grpcServer.Stop) + + // Deliberately no EnforcingClientInterceptor on this connection. + conn, err := grpc.NewClient( + "passthrough:///bufconn", + grpc.WithContextDialer(func(context.Context, string) (net.Conn, error) { return listener.Dial() }), + grpc.WithTransportCredentials(insecure.NewCredentials()), + ) + require.NoError(t, err) + t.Cleanup(func() { _ = conn.Close() }) + + client := solution.NewClient(conn) + + // Scaffold ceiling admits Create — it reaches the server. + _, err = client.Create(context.Background(), solution.CeilingScaffold(), &solutionv0.CreateRequest{}) + require.NoError(t, err) + require.Equal(t, 1, server.handled["Create"]) + + // Package exceeds it — refused by Client itself, never reaching the server, + // even though nothing on the connection would have stopped it. + _, err = client.Package(context.Background(), solution.CeilingScaffold(), &solutionv0.PackageRequest{}) + require.Equal(t, codes.PermissionDenied, status.Code(err)) + require.Equal(t, 0, server.handled["Package"]) +} + func TestEnforcingClientInterceptorPassesThroughNonSolutionCalls(t *testing.T) { interceptor := solution.EnforcingClientInterceptor() invoked := false