refactor: collapse the Solution two-API split onto solution.Client (#295)

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(),

solution/policy.go

@@ -82,58 +82,105 @@ 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 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 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
+// 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
 }
 
-// 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) {
-	return c.inner.GetSolutionInformation(WithCeiling(ctx, ceiling), in, opts...)
+	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) {
-	return c.inner.Create(WithCeiling(ctx, ceiling), in, opts...)
+	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) {
-	return c.inner.Update(WithCeiling(ctx, ceiling), in, opts...)
+	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) {
-	return c.inner.Package(WithCeiling(ctx, ceiling), in, opts...)
+	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) {
-	return c.inner.Render(WithCeiling(ctx, ceiling), in, opts...)
+	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
-// 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 +198,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,15 +244,15 @@ 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.
 //
 // 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 +274,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)

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
@@ -178,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