From 49473422ba159be2877d7043c5a51f38570f76f5 Mon Sep 17 00:00:00 2001 From: crisfiorini-oss <301773486+crisfiorini-oss@users.noreply.github.com> Date: Fri, 9 Oct 2026 20:10:18 +0000 Subject: [PATCH 1/6] feat(callertoken): export the one x-codefly-token implementation The caller token existed twice in core, unexported: server interceptors in agents/agents.go and a client credential and generator in agents/manager/loader.go. Service gateways copied both because only the metadata key was exported, and that key sat in the package that links the whole agent runtime. callertoken depends on grpc alone and exports the key, unary and stream server interceptors (health Check and Watch admitted without a token), the per-RPC client credential and DialOption, Generate, and CheckListener for the startup rule that a listener has either a token or an explicit anonymous opt-out, never both. agents.Serve and the agent manager now use it, so core keeps one implementation. An empty presented token is refused as absent, an empty server token fails closed, and health stays an exact Check/Watch match rather than a prefix, so the health List method (which enumerates registered services) needs a token. Co-Authored-By: Claude Sonnet 5.5 --- agents/agents.go | 87 +----------- agents/auth_test.go | 140 +----------------- agents/manager/loader.go | 36 +---- callertoken/callertoken.go | 146 +++++++++++++++++++ callertoken/callertoken_test.go | 244 ++++++++++++++++++++++++++++++++ callertoken/listener.go | 49 +++++++ callertoken/listener_test.go | 65 +++++++++ 7 files changed, 519 insertions(+), 248 deletions(-) create mode 100644 callertoken/callertoken.go create mode 100644 callertoken/callertoken_test.go create mode 100644 callertoken/listener.go create mode 100644 callertoken/listener_test.go diff --git a/agents/agents.go b/agents/agents.go index d8e161eaf..5cdae3862 100644 --- a/agents/agents.go +++ b/agents/agents.go @@ -2,7 +2,6 @@ package agents import ( "context" - "crypto/subtle" "fmt" "net" "os" @@ -15,6 +14,7 @@ import ( "time" "github.com/codefly-dev/core/agents/contract" + "github.com/codefly-dev/core/callertoken" executionv1 "github.com/codefly-dev/core/generated/go/codefly/execution/v1" agentv0 "github.com/codefly-dev/core/generated/go/codefly/services/agent/v0" builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" @@ -33,7 +33,6 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/health" healthpb "google.golang.org/grpc/health/grpc_health_v1" - "google.golang.org/grpc/metadata" "google.golang.org/grpc/status" ) @@ -129,33 +128,10 @@ type PluginRegistration struct { // in-process latency histogram (RPCStats). Callers can read it via // SnapshotRPCStats — useful for daemons that want to expose p50/p99 // per RPC without standing up a separate metrics endpoint. -// AuthMetadataKey is the gRPC metadata key carrying the per-spawn -// auth token. Lowercase per gRPC convention — metadata keys are -// case-insensitive but the wire form is lowercase. -const AuthMetadataKey = "x-codefly-token" - -// authUnaryInterceptor verifies the per-spawn token on every unary gRPC call. -// An empty expected token is a server misconfiguration and fails closed. -// -// The health-check service is exempt: the host's readiness probe -// uses grpc.health.v1.Health/Check which fires BEFORE the host has -// established any client metadata interceptor (it's part of the -// agent-loader's connection setup). Letting Check through unauthed -// is safe — it returns only SERVING/NOT_SERVING, no privileged data. -func authUnaryInterceptor(expectedToken string) grpc.UnaryServerInterceptor { - return func(ctx context.Context, req any, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) { - if isHealthMethod(info.FullMethod) { - return handler(ctx, req) - } - if expectedToken == "" { - return nil, status.Error(codes.Unauthenticated, "agent auth token is not configured") - } - if err := verifyAuthToken(ctx, expectedToken); err != nil { - return nil, err - } - return handler(ctx, req) - } -} +// AuthMetadataKey is the gRPC metadata key carrying the per-spawn auth token. +// It is callertoken.MetadataKey, the one definition of the caller token that +// agents, the agent manager and service gateways share. +const AuthMetadataKey = callertoken.MetadataKey // runtimeLoadTracker records that this agent process has actually entered the // Runtime lifecycle. Builder-only invocations still register a Runtime server, @@ -170,55 +146,6 @@ func runtimeLoadTracker(loaded *atomic.Bool) grpc.UnaryServerInterceptor { } } -// authStreamInterceptor mirrors authUnaryInterceptor for streaming -// RPCs. Same exemption for health checks. -func authStreamInterceptor(expectedToken string) grpc.StreamServerInterceptor { - return func(srv any, ss grpc.ServerStream, info *grpc.StreamServerInfo, handler grpc.StreamHandler) error { - if isHealthMethod(info.FullMethod) { - return handler(srv, ss) - } - if expectedToken == "" { - return status.Error(codes.Unauthenticated, "agent auth token is not configured") - } - if err := verifyAuthToken(ss.Context(), expectedToken); err != nil { - return err - } - return handler(srv, ss) - } -} - -// verifyAuthToken extracts the bearer from metadata and constant-time -// compares against the expected value. Returns codes.Unauthenticated -// on any failure path so the model sees a clear refusal. -func verifyAuthToken(ctx context.Context, expected string) error { - md, ok := metadata.FromIncomingContext(ctx) - if !ok { - return status.Error(codes.Unauthenticated, "no metadata") - } - values := md.Get(AuthMetadataKey) - if len(values) == 0 { - return status.Error(codes.Unauthenticated, "missing auth token") - } - // subtle.ConstantTimeCompare returns 1 on equality. Comparing as - // byte slices avoids the timing side-channel of `==`. - if subtle.ConstantTimeCompare([]byte(values[0]), []byte(expected)) != 1 { - return status.Error(codes.Unauthenticated, "bad auth token") - } - return nil -} - -// isHealthMethod returns true for gRPC health-check method names. -// The set is small enough to enumerate — no need for a string-prefix -// hack that could accidentally match plugin methods. -func isHealthMethod(fullMethod string) bool { - switch fullMethod { - case "/grpc.health.v1.Health/Check", - "/grpc.health.v1.Health/Watch": - return true - } - return false -} - // panicRecoveryInterceptor converts a panic in any handler (or inner // interceptor) into a gRPC error the host can see, instead of letting it crash // the agent process or — worse — be swallowed into a log line while the RPC @@ -441,14 +368,14 @@ func Serve(reg PluginRegistration) { // acting as). The PDP downstream branches on the // stamped Principal — wrong order = no principal + // insecure default. - authUnaryInterceptor(expectedToken), + callertoken.UnaryServerInterceptor(expectedToken), principalUnaryInterceptor(), containerRecoveryInterceptor(), runtimeLoadTracker(&runtimeLoaded), agentRPCInterceptor(), ), grpc.ChainStreamInterceptor( - authStreamInterceptor(expectedToken), + callertoken.StreamServerInterceptor(expectedToken), principalStreamInterceptor(), ), ) diff --git a/agents/auth_test.go b/agents/auth_test.go index 23f025d23..5a831c54a 100644 --- a/agents/auth_test.go +++ b/agents/auth_test.go @@ -7,133 +7,14 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/codes" - "google.golang.org/grpc/metadata" "google.golang.org/grpc/status" ) -// TestAuthUnaryInterceptor_RejectsMissingToken pins the load-bearing -// behavior: when the plugin is configured with an expected token, -// any call without a matching bearer in metadata is rejected with -// Unauthenticated before the handler runs. -func TestAuthUnaryInterceptor_RejectsMissingToken(t *testing.T) { - const token = "deadbeef-correct-token" - - intercept := authUnaryInterceptor(token) - handlerRan := false - handler := func(_ context.Context, _ any) (any, error) { - handlerRan = true - return nil, nil - } - info := &grpc.UnaryServerInfo{FullMethod: "/example.Service/Method"} - - t.Run("no metadata at all", func(t *testing.T) { - handlerRan = false - _, err := intercept(context.Background(), nil, info, handler) - st, _ := status.FromError(err) - if st.Code() != codes.Unauthenticated { - t.Fatalf("expected Unauthenticated, got %v: %v", st.Code(), err) - } - if !strings.Contains(st.Message(), "no metadata") { - t.Fatalf("error must point at no-metadata cause; got %q", st.Message()) - } - if handlerRan { - t.Fatal("handler MUST NOT run when auth fails") - } - }) - - t.Run("metadata without bearer", func(t *testing.T) { - handlerRan = false - ctx := metadata.NewIncomingContext(context.Background(), - metadata.Pairs("x-other-key", "irrelevant")) - _, err := intercept(ctx, nil, info, handler) - st, _ := status.FromError(err) - if st.Code() != codes.Unauthenticated { - t.Fatalf("expected Unauthenticated, got %v", st.Code()) - } - if !strings.Contains(st.Message(), "missing auth token") { - t.Fatalf("error must say bearer is missing; got %q", st.Message()) - } - if handlerRan { - t.Fatal("handler MUST NOT run when auth fails") - } - }) - - t.Run("wrong bearer", func(t *testing.T) { - handlerRan = false - ctx := metadata.NewIncomingContext(context.Background(), - metadata.Pairs(AuthMetadataKey, "wrong-token")) - _, err := intercept(ctx, nil, info, handler) - st, _ := status.FromError(err) - if st.Code() != codes.Unauthenticated { - t.Fatalf("expected Unauthenticated, got %v", st.Code()) - } - if !strings.Contains(st.Message(), "bad auth token") { - t.Fatalf("error must say token mismatch; got %q", st.Message()) - } - if handlerRan { - t.Fatal("handler MUST NOT run when auth fails") - } - }) - - t.Run("correct bearer accepts and runs handler", func(t *testing.T) { - handlerRan = false - ctx := metadata.NewIncomingContext(context.Background(), - metadata.Pairs(AuthMetadataKey, token)) - _, err := intercept(ctx, nil, info, handler) - if err != nil { - t.Fatalf("matching token must accept: %v", err) - } - if !handlerRan { - t.Fatal("handler MUST run when auth passes") - } - }) -} - -// TestAuthUnaryInterceptor_HealthExempt confirms health-check methods -// pass through without bearer. Some gRPC bootstraps fire Check before -// the host has wired client-side metadata; if we required auth here, -// the readiness probe would race-fail. -func TestAuthUnaryInterceptor_HealthExempt(t *testing.T) { - intercept := authUnaryInterceptor("a-required-token") - handlerRan := false - handler := func(_ context.Context, _ any) (any, error) { - handlerRan = true - return nil, nil - } - - for _, method := range []string{ - "/grpc.health.v1.Health/Check", - "/grpc.health.v1.Health/Watch", - } { - t.Run(method, func(t *testing.T) { - handlerRan = false - info := &grpc.UnaryServerInfo{FullMethod: method} - // No bearer in context — would fail for any other method. - if _, err := intercept(context.Background(), nil, info, handler); err != nil { - t.Fatalf("health-exempt method must pass without bearer: %v", err) - } - if !handlerRan { - t.Fatal("handler MUST run for health methods even without auth") - } - }) - } -} - -func TestAuthUnaryInterceptor_NoExpectedTokenFailsClosed(t *testing.T) { - intercept := authUnaryInterceptor("") - handlerRan := false - handler := func(_ context.Context, _ any) (any, error) { - handlerRan = true - return nil, nil - } - info := &grpc.UnaryServerInfo{FullMethod: "/example.Service/Method"} - - _, err := intercept(context.Background(), nil, info, handler) - if status.Code(err) != codes.Unauthenticated { - t.Fatalf("missing server token must fail closed with Unauthenticated: %v", err) - } - if handlerRan { - t.Fatal("handler MUST NOT run when the server has no expected token") +// The caller-token interceptors live in, and are tested through, +// callertoken; AuthMetadataKey must stay the same wire key. +func TestAuthMetadataKeyIsTheCallerTokenKey(t *testing.T) { + if AuthMetadataKey != "x-codefly-token" { + t.Fatalf("AuthMetadataKey = %q", AuthMetadataKey) } } @@ -150,14 +31,3 @@ func TestPanicRecoveryInterceptorRedactsPanicValue(t *testing.T) { t.Fatalf("panic value leaked through RPC error: %v", err) } } - -// TestVerifyAuthToken_ConstantTime confirms a passing token actually -// passes (regression for "someone replaced the constant-time compare -// with =="). -func TestVerifyAuthToken_ConstantTime(t *testing.T) { - ctx := metadata.NewIncomingContext(context.Background(), - metadata.Pairs(AuthMetadataKey, "expected")) - if err := verifyAuthToken(ctx, "expected"); err != nil { - t.Fatalf("matching token should pass: %v", err) - } -} diff --git a/agents/manager/loader.go b/agents/manager/loader.go index d300ff1f9..1ee001658 100644 --- a/agents/manager/loader.go +++ b/agents/manager/loader.go @@ -4,10 +4,8 @@ import ( "bufio" "bytes" "context" - cryptorand "crypto/rand" "crypto/sha256" "encoding/base64" - "encoding/hex" "errors" "fmt" "io" @@ -23,6 +21,7 @@ import ( "time" "github.com/codefly-dev/core/agents" + "github.com/codefly-dev/core/callertoken" "github.com/codefly-dev/core/grpcconfig" "github.com/codefly-dev/core/policy" providerartifact "github.com/codefly-dev/core/provider/artifact" @@ -947,7 +946,7 @@ func spawnAgent(ctx context.Context, bin, identity, telemetryName, artifactDiges // crypto/rand → guaranteed unbiased. Hex encoding (not base64) // avoids any case where shell quoting / env-var escaping could // mangle the bearer in transit. - authToken, err := mintAgentToken() + authToken, err := callertoken.Generate() if err != nil { return nil, w.Wrapf(err, "mint agent token") } @@ -1164,7 +1163,7 @@ func spawnAgent(ctx context.Context, bin, identity, telemetryName, artifactDiges conn, err := grpc.NewClient(addr, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithStatsHandler(otelgrpc.NewClientHandler()), - grpc.WithPerRPCCredentials(bearerCreds{token: authToken}), + callertoken.DialOption(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 by solution.Client. No-ops for every @@ -1400,32 +1399,3 @@ func parseAgentHandshake(line string) (addr string, err error) { } return endpoint, nil } - -// mintAgentToken returns 32 random bytes hex-encoded. 256 bits of -// entropy from crypto/rand — overwhelmingly more than the auth- -// against-local-attacker threat model demands, but cheap. -// -// Hex (not base64) so the token survives env-var quoting through -// every shell + exec layer between host and plugin without any -// special-character footguns. -func mintAgentToken() (string, error) { - var buf [32]byte - if _, err := cryptorand.Read(buf[:]); err != nil { - return "", fmt.Errorf("read random: %w", err) - } - return hex.EncodeToString(buf[:]), nil -} - -// bearerCreds is a grpc.PerRPCCredentials that attaches the per- -// spawn token to every outgoing RPC. RequireTransportSecurity -// returns false because we run over UDS / loopback — the token -// is the auth, the transport doesn't need TLS. -type bearerCreds struct { - token string -} - -func (b bearerCreds) GetRequestMetadata(_ context.Context, _ ...string) (map[string]string, error) { - return map[string]string{agents.AuthMetadataKey: b.token}, nil -} - -func (b bearerCreds) RequireTransportSecurity() bool { return false } diff --git a/callertoken/callertoken.go b/callertoken/callertoken.go new file mode 100644 index 000000000..d3d0882b6 --- /dev/null +++ b/callertoken/callertoken.go @@ -0,0 +1,146 @@ +// Package callertoken is the one implementation of codefly's caller token: a +// shared secret a client presents as the "x-codefly-token" gRPC metadata header +// and a server compares in constant time before it runs a handler. +// +// The codefly host uses it to talk to agent plugins (a fresh token per spawn, +// see agents.Serve and the agent manager), and a service that publishes its own +// gRPC listener — an object-storage or warehouse gateway, say — uses the same +// header to admit only the callers that were handed its token. The package +// depends on grpc alone, so a gateway image can import it without linking the +// agent runtime. +// +// It authenticates and nothing else: whether the caller holds the token, never +// what that caller may do with it. +package callertoken + +import ( + "context" + "crypto/rand" + "crypto/subtle" + "encoding/hex" + "fmt" + + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" +) + +// MetadataKey is the gRPC metadata key carrying the token. Lowercase per gRPC +// convention: metadata keys are case-insensitive but the wire form is lowercase. +const MetadataKey = "x-codefly-token" + +// Health-service methods a readiness probe calls. They return SERVING or +// NOT_SERVING and nothing privileged, and a probe — the codefly host's, a load +// balancer's, a mesh's — presents no token, so guarding them would make the +// server look permanently unready to everything that would route to it. +// +// The set is enumerated rather than matched by prefix: the health service's +// List method returns every registered service name, and a prefix would admit +// it, and any method added to the service later, without a token. +const ( + healthCheck = "/grpc.health.v1.Health/Check" + healthWatch = "/grpc.health.v1.Health/Watch" +) + +// IsHealthMethod reports whether fullMethod is a health-check method the +// interceptors admit without a token. +func IsHealthMethod(fullMethod string) bool { + return fullMethod == healthCheck || fullMethod == healthWatch +} + +// tokenBytes is the entropy of a generated token: 256 bits, far beyond what a +// local-attacker threat model needs, and cheap. +const tokenBytes = 32 + +// Generate returns a new random token: 32 bytes from crypto/rand, hex-encoded +// to 64 characters. Hex rather than base64 so the token survives env-var +// quoting through every shell and exec layer between a host and the process it +// hands the token to. +func Generate() (string, error) { + var buf [tokenBytes]byte + if _, err := rand.Read(buf[:]); err != nil { + return "", fmt.Errorf("read random: %w", err) + } + return hex.EncodeToString(buf[:]), nil +} + +// UnaryServerInterceptor refuses every unary call that does not present token, +// except the health check. An empty token is a server misconfiguration and +// fails closed: no caller can match it, so none is admitted. A server that +// accepts anonymous callers installs no interceptor instead of one built on "". +func UnaryServerInterceptor(token string) grpc.UnaryServerInterceptor { + return func(ctx context.Context, req any, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (any, error) { + if !IsHealthMethod(info.FullMethod) { + if err := verify(ctx, token); err != nil { + return nil, err + } + } + return handler(ctx, req) + } +} + +// StreamServerInterceptor is UnaryServerInterceptor for streaming calls. gRPC +// runs it before the handler observes a single frame, so an unauthorized stream +// is refused at open rather than after its first message is accepted. +func StreamServerInterceptor(token string) grpc.StreamServerInterceptor { + return func(srv any, ss grpc.ServerStream, info *grpc.StreamServerInfo, handler grpc.StreamHandler) error { + if !IsHealthMethod(info.FullMethod) { + if err := verify(ss.Context(), token); err != nil { + return err + } + } + return handler(srv, ss) + } +} + +// verify reads the token from the incoming metadata and compares it in +// constant time. Every failure is codes.Unauthenticated and names the header, +// never the expected value. +func verify(ctx context.Context, expected string) error { + if expected == "" { + return status.Error(codes.Unauthenticated, "no "+MetadataKey+" is configured on this server") + } + md, ok := metadata.FromIncomingContext(ctx) + if !ok { + return status.Error(codes.Unauthenticated, "missing "+MetadataKey) + } + presented := md.Get(MetadataKey) + // An empty presented value is refused as absent rather than reaching the + // comparison. + if len(presented) == 0 || presented[0] == "" { + return status.Error(codes.Unauthenticated, "missing "+MetadataKey) + } + // Comparing as byte slices in constant time avoids the timing side channel + // of ==. + if subtle.ConstantTimeCompare([]byte(presented[0]), []byte(expected)) != 1 { + return status.Error(codes.Unauthenticated, "invalid "+MetadataKey) + } + return nil +} + +// PerRPCCredentials returns the client side: credentials that attach token to +// every call a connection makes. +// +// RequireTransportSecurity is false. The token authenticates the caller; the +// transport is the deployment's decision, and the profiles that use this reach +// the server over a unix socket, loopback, the Docker host bridge, or a mesh +// that terminates transport security itself. Demanding TLS here would make the +// credential unusable on all of them. +func PerRPCCredentials(token string) credentials.PerRPCCredentials { + return bearer(token) +} + +// DialOption is PerRPCCredentials as a grpc.DialOption. +func DialOption(token string) grpc.DialOption { + return grpc.WithPerRPCCredentials(bearer(token)) +} + +type bearer string + +func (b bearer) GetRequestMetadata(context.Context, ...string) (map[string]string, error) { + return map[string]string{MetadataKey: string(b)}, nil +} + +func (b bearer) RequireTransportSecurity() bool { return false } diff --git a/callertoken/callertoken_test.go b/callertoken/callertoken_test.go new file mode 100644 index 000000000..f10b75366 --- /dev/null +++ b/callertoken/callertoken_test.go @@ -0,0 +1,244 @@ +package callertoken_test + +import ( + "context" + "net" + "strings" + "testing" + "time" + + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/health" + healthpb "google.golang.org/grpc/health/grpc_health_v1" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" + "google.golang.org/protobuf/types/known/emptypb" + + "github.com/codefly-dev/core/callertoken" +) + +const token = "89d0d1f6d0e2f5a4c3b2a1908f7e6d5c" + +// echoDesc is a small real service with one unary and one streaming method, so +// the interceptors are driven over a real gRPC server on both paths. The +// health service alone cannot do it: its streaming method is exempt. +var echoDesc = grpc.ServiceDesc{ + ServiceName: "callertoken.test.Echo", + HandlerType: (*any)(nil), + Methods: []grpc.MethodDesc{{ + MethodName: "Ping", + Handler: func(_ any, ctx context.Context, dec func(any) error, ic grpc.UnaryServerInterceptor) (any, error) { + in := new(emptypb.Empty) + if err := dec(in); err != nil { + return nil, err + } + call := func(context.Context, any) (any, error) { return &emptypb.Empty{}, nil } + if ic == nil { + return call(ctx, in) + } + return ic(ctx, in, &grpc.UnaryServerInfo{FullMethod: "/callertoken.test.Echo/Ping"}, call) + }, + }}, + Streams: []grpc.StreamDesc{{ + StreamName: "Chat", + ServerStreams: true, + ClientStreams: true, + Handler: func(_ any, ss grpc.ServerStream) error { + in := new(emptypb.Empty) + if err := ss.RecvMsg(in); err != nil { + return err + } + return ss.SendMsg(&emptypb.Empty{}) + }, + }}, +} + +// serve starts a real server guarded by the interceptors for serverToken and +// returns a connection to it dialled with opts. +func serve(t *testing.T, serverToken string, opts ...grpc.DialOption) *grpc.ClientConn { + t.Helper() + srv := grpc.NewServer( + grpc.ChainUnaryInterceptor(callertoken.UnaryServerInterceptor(serverToken)), + grpc.ChainStreamInterceptor(callertoken.StreamServerInterceptor(serverToken)), + ) + srv.RegisterService(&echoDesc, struct{}{}) + healthpb.RegisterHealthServer(srv, health.NewServer()) + + lis, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + go func() { _ = srv.Serve(lis) }() + t.Cleanup(srv.Stop) + + conn, err := grpc.NewClient(lis.Addr().String(), + append([]grpc.DialOption{grpc.WithTransportCredentials(insecure.NewCredentials())}, opts...)...) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = conn.Close() }) + return conn +} + +func ctx(t *testing.T) context.Context { + t.Helper() + c, cancel := context.WithTimeout(context.Background(), 10*time.Second) + t.Cleanup(cancel) + return c +} + +func ping(c context.Context, conn *grpc.ClientConn) error { + return conn.Invoke(c, "/callertoken.test.Echo/Ping", &emptypb.Empty{}, &emptypb.Empty{}) +} + +// chat opens the stream, sends one message and waits for the reply, so a +// refusal at stream open and a refusal after the first frame both surface. +func chat(c context.Context, conn *grpc.ClientConn) error { + st, err := conn.NewStream(c, &grpc.StreamDesc{ClientStreams: true, ServerStreams: true}, "/callertoken.test.Echo/Chat") + if err != nil { + return err + } + if err := st.SendMsg(&emptypb.Empty{}); err != nil { + return err + } + return st.RecvMsg(&emptypb.Empty{}) +} + +func requireCode(t *testing.T, err error, want codes.Code, contains string) { + t.Helper() + if status.Code(err) != want { + t.Fatalf("code = %v (%v), want %v", status.Code(err), err, want) + } + if contains != "" && !strings.Contains(err.Error(), contains) { + t.Fatalf("error %q does not contain %q", err, contains) + } +} + +func TestPresentedTokenIsAdmittedOnUnaryAndStream(t *testing.T) { + conn := serve(t, token, callertoken.DialOption(token)) + if err := ping(ctx(t), conn); err != nil { + t.Fatalf("unary with the token: %v", err) + } + if err := chat(ctx(t), conn); err != nil { + t.Fatalf("stream with the token: %v", err) + } +} + +func TestEveryWayOfNotPresentingTheTokenIsRefused(t *testing.T) { + cases := []struct { + name string + md metadata.MD + contains string + }{ + {"no metadata", nil, "missing " + callertoken.MetadataKey}, + {"other metadata only", metadata.Pairs("x-other", "irrelevant"), "missing " + callertoken.MetadataKey}, + {"empty token", metadata.Pairs(callertoken.MetadataKey, ""), "missing " + callertoken.MetadataKey}, + {"wrong token", metadata.Pairs(callertoken.MetadataKey, "wrong"), "invalid " + callertoken.MetadataKey}, + {"token with a suffix", metadata.Pairs(callertoken.MetadataKey, token+"0"), "invalid " + callertoken.MetadataKey}, + } + conn := serve(t, token) + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + c := ctx(t) + if tc.md != nil { + c = metadata.NewOutgoingContext(c, tc.md) + } + requireCode(t, ping(c, conn), codes.Unauthenticated, tc.contains) + requireCode(t, chat(c, conn), codes.Unauthenticated, tc.contains) + }) + } +} + +func TestRefusalNeverEchoesTheExpectedToken(t *testing.T) { + conn := serve(t, token) + err := ping(metadata.AppendToOutgoingContext(ctx(t), callertoken.MetadataKey, "wrong"), conn) + if strings.Contains(err.Error(), token) { + t.Fatalf("refusal leaked the expected token: %v", err) + } +} + +// A server with no token configured is a misconfiguration; even a caller that +// presents an empty token must not match it. +func TestServerWithoutATokenFailsClosed(t *testing.T) { + conn := serve(t, "") + c := ctx(t) + requireCode(t, ping(c, conn), codes.Unauthenticated, "") + c = metadata.AppendToOutgoingContext(c, callertoken.MetadataKey, "") + requireCode(t, ping(c, conn), codes.Unauthenticated, "") + requireCode(t, chat(c, conn), codes.Unauthenticated, "") +} + +// The readiness probe presents no token. Check and Watch are admitted; List, +// which enumerates every registered service, is not. +func TestHealthCheckAndWatchAreAdmittedWithoutAToken(t *testing.T) { + conn := serve(t, token) + client := healthpb.NewHealthClient(conn) + + resp, err := client.Check(ctx(t), &healthpb.HealthCheckRequest{}) + if err != nil || resp.GetStatus() != healthpb.HealthCheckResponse_SERVING { + t.Fatalf("Check without a token = %v, %v", resp, err) + } + w, err := client.Watch(ctx(t), &healthpb.HealthCheckRequest{}) + if err != nil { + t.Fatal(err) + } + if got, err := w.Recv(); err != nil || got.GetStatus() != healthpb.HealthCheckResponse_SERVING { + t.Fatalf("Watch without a token = %v, %v", got, err) + } + _, err = client.List(ctx(t), &healthpb.HealthListRequest{}) + requireCode(t, err, codes.Unauthenticated, "") +} + +func TestIsHealthMethodIsExact(t *testing.T) { + for method, want := range map[string]bool{ + healthpb.Health_Check_FullMethodName: true, + healthpb.Health_Watch_FullMethodName: true, + healthpb.Health_List_FullMethodName: false, + "/grpc.health.v1.Health/": false, + "/grpc.health.v1.HealthExtra/Check": false, + "/callertoken.test.Echo/Ping": false, + } { + if got := callertoken.IsHealthMethod(method); got != want { + t.Errorf("IsHealthMethod(%q) = %v, want %v", method, got, want) + } + } +} + +func TestPerRPCCredentialsCarryTheTokenWithoutRequiringTLS(t *testing.T) { + creds := callertoken.PerRPCCredentials(token) + md, err := creds.GetRequestMetadata(context.Background()) + if err != nil { + t.Fatal(err) + } + if len(md) != 1 || md[callertoken.MetadataKey] != token { + t.Fatalf("metadata = %v", md) + } + if creds.RequireTransportSecurity() { + t.Fatal("the credential must be usable over unix sockets, loopback and a mesh") + } +} + +func TestMetadataKeyIsTheWireKeyTheFleetUses(t *testing.T) { + if callertoken.MetadataKey != "x-codefly-token" { + t.Fatalf("MetadataKey = %q: changing it breaks every deployed agent and gateway", callertoken.MetadataKey) + } +} + +func TestGenerateIsHexOf256BitsAndFresh(t *testing.T) { + seen := map[string]bool{} + for range 16 { + tok, err := callertoken.Generate() + if err != nil { + t.Fatal(err) + } + if len(tok) != 64 || strings.Trim(tok, "0123456789abcdef") != "" { + t.Fatalf("token %q is not 64 lowercase hex characters", tok) + } + if seen[tok] { + t.Fatalf("token %q generated twice", tok) + } + seen[tok] = true + } +} diff --git a/callertoken/listener.go b/callertoken/listener.go new file mode 100644 index 000000000..604cad29d --- /dev/null +++ b/callertoken/listener.go @@ -0,0 +1,49 @@ +package callertoken + +import "fmt" + +// Listener describes a server's caller-token configuration for CheckListener: +// the environment variables an operator sets, and what an unauthenticated +// caller could do if the token were absent. +type Listener struct { + // TokenVar names the variable holding the token, e.g. "SOS_AUTH_TOKEN". + TokenVar string + // AnonymousVar names the variable that opts into an unauthenticated + // listener, e.g. "SOS_ALLOW_ANONYMOUS". + AnonymousVar string + // Exposure completes "every caller that can reach the listener ...": what + // an unauthenticated caller could do, e.g. "can run SQL against the bound + // database". + Exposure string +} + +// CheckListener refuses to resolve a configuration that would serve its +// capabilities to anyone who can reach the listen port. A listener inside a +// container binds every interface, because port publishing cannot forward to a +// loopback-bound process, so its address says nothing about who can reach it +// and the token is the only thing the server itself can enforce. +// +// token is the configured token and allowAnonymous is whether the operator +// opted into an unauthenticated listener; the caller reads both from its own +// environment, so this package fixes no variable names and no parsing of +// booleans. +// +// A token with the opt-out set is a contradiction, not a preference: "here is a +// credential" and "accept callers with no credential" cannot both be the +// intent, and silently picking one leaves the operator believing the other. It +// is also the shape a half-finished migration takes, a token added to a secret +// while the manifest still carries the opt-out. +func CheckListener(token string, allowAnonymous bool, l Listener) error { + switch { + case token != "" && allowAnonymous: + return fmt.Errorf("%s and %s=true are mutually exclusive: "+ + "unset %[2]s to enforce the token, or unset %[1]s to accept anonymous callers", + l.TokenVar, l.AnonymousVar) + case token != "", allowAnonymous: + return nil + } + return fmt.Errorf("%[1]s is required: without it every caller that can reach the listener %[2]s. "+ + "Set %[1]s to a shared secret and have clients send it as the %[3]q gRPC metadata header, "+ + "or set %[4]s=true if the listener is confined to a private boundary enforced elsewhere (cluster NetworkPolicy / service-mesh mTLS)", + l.TokenVar, l.Exposure, MetadataKey, l.AnonymousVar) +} diff --git a/callertoken/listener_test.go b/callertoken/listener_test.go new file mode 100644 index 000000000..3d7df97e5 --- /dev/null +++ b/callertoken/listener_test.go @@ -0,0 +1,65 @@ +package callertoken_test + +import ( + "strings" + "testing" + + "github.com/codefly-dev/core/callertoken" +) + +var warehouse = callertoken.Listener{ + TokenVar: "SWH_AUTH_TOKEN", + AnonymousVar: "SWH_ALLOW_ANONYMOUS", + Exposure: "can run SQL against the bound database", +} + +func TestCheckListener(t *testing.T) { + for _, tc := range []struct { + name string + token string + anonymous bool + wantErr []string // substrings; nil means accepted + }{ + {"token alone", "s3cret", false, nil}, + {"anonymous opt-out alone", "", true, nil}, + {"both is a contradiction", "s3cret", true, []string{ + "SWH_AUTH_TOKEN and SWH_ALLOW_ANONYMOUS=true are mutually exclusive", + "unset SWH_ALLOW_ANONYMOUS to enforce the token", + "unset SWH_AUTH_TOKEN to accept anonymous callers", + }}, + {"neither", "", false, []string{ + "SWH_AUTH_TOKEN is required", + "can run SQL against the bound database", + `"x-codefly-token"`, + "SWH_ALLOW_ANONYMOUS=true", + }}, + } { + t.Run(tc.name, func(t *testing.T) { + err := callertoken.CheckListener(tc.token, tc.anonymous, warehouse) + if tc.wantErr == nil { + if err != nil { + t.Fatalf("refused: %v", err) + } + return + } + if err == nil { + t.Fatal("accepted a configuration it must refuse") + } + for _, want := range tc.wantErr { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not contain %q", err, want) + } + } + if strings.Contains(err.Error(), "%!") { + t.Errorf("error has a formatting defect: %q", err) + } + }) + } +} + +func TestCheckListenerNeverEchoesTheToken(t *testing.T) { + err := callertoken.CheckListener("s3cret-value", true, warehouse) + if err == nil || strings.Contains(err.Error(), "s3cret-value") { + t.Fatalf("err = %v", err) + } +} From af6470ea9124ccb3f6760187e3fb7746db029784 Mon Sep 17 00:00:00 2001 From: crisfiorini-oss <301773486+crisfiorini-oss@users.noreply.github.com> Date: Fri, 9 Oct 2026 20:10:19 +0000 Subject: [PATCH 2/6] feat(resources): parse and validate a digest-pinned image lock A service commits -image.json recording where its image is published and the sha256 digest that was built. Two services parsed it separately. ParseImageLock reads it against core's registry and the image's own name, returns a DockerImage addressed by digest alone, and tells a lock that is not yet published (ErrImageUnpublished) from one that is wrong. IsSHA256Digest is exported for locks with more fields than this one. Co-Authored-By: Claude Sonnet 5.5 --- resources/image_lock.go | 65 +++++++++++++++++ resources/image_lock_test.go | 73 +++++++++++++++++++ resources/testdata/image-lock/no-name.json | 1 + resources/testdata/image-lock/not-hex.json | 4 + resources/testdata/image-lock/not-json.json | 1 + .../testdata/image-lock/other-algorithm.json | 4 + .../testdata/image-lock/other-image.json | 4 + .../testdata/image-lock/other-registry.json | 4 + resources/testdata/image-lock/published.json | 4 + .../testdata/image-lock/tag-as-digest.json | 4 + resources/testdata/image-lock/too-long.json | 4 + resources/testdata/image-lock/truncated.json | 4 + .../testdata/image-lock/unpublished.json | 4 + 13 files changed, 176 insertions(+) create mode 100644 resources/image_lock.go create mode 100644 resources/image_lock_test.go create mode 100644 resources/testdata/image-lock/no-name.json create mode 100644 resources/testdata/image-lock/not-hex.json create mode 100644 resources/testdata/image-lock/not-json.json create mode 100644 resources/testdata/image-lock/other-algorithm.json create mode 100644 resources/testdata/image-lock/other-image.json create mode 100644 resources/testdata/image-lock/other-registry.json create mode 100644 resources/testdata/image-lock/published.json create mode 100644 resources/testdata/image-lock/tag-as-digest.json create mode 100644 resources/testdata/image-lock/too-long.json create mode 100644 resources/testdata/image-lock/truncated.json create mode 100644 resources/testdata/image-lock/unpublished.json diff --git a/resources/image_lock.go b/resources/image_lock.go new file mode 100644 index 000000000..1c954d83c --- /dev/null +++ b/resources/image_lock.go @@ -0,0 +1,65 @@ +package resources + +import ( + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "strings" +) + +// ErrImageUnpublished means an image lock names its image but records no +// digest: the image has not been published yet. A tree legitimately sits in +// that state between merging a service and publishing its first image, and the +// only safe reading is a refusal. A reference with no digest and no tag is the +// registry's :latest, which is whatever was pushed last. Callers add what to do +// about it by wrapping the error. +var ErrImageUnpublished = errors.New("image lock records no digest: the image has not been published") + +// ImageLock is the recorded identity of a published image, the content of the +// -image.json a service commits and embeds. It carries no tag: the tag is +// the service version, and a second copy of it is one more thing to disagree +// with the service's own manifest. Name is the full repository the image is +// published to, registry included. +type ImageLock struct { + Name string `json:"name"` + Digest string `json:"digest"` +} + +// ParseImageLock returns the image a lock pins. name is the image's name inside +// ImageRegistry ("service-warehouse"); the lock must name exactly +// ImageRegistry/name. The registry comes from core, never from the lock: a lock +// naming another registry or another image is the lock being wrong, not an +// instruction to run an image from there. +// +// The result is addressed by digest alone (see DockerImage.FullName), so what +// runs, and what evidence describes, is the manifest that was checked and not +// whatever a tag points at by the time it is pulled. A lock with no digest is +// ErrImageUnpublished; one with a digest that is not a complete sha256 digest +// is a different error, so a caller can tell "not yet published" from "wrong". +func ParseImageLock(content []byte, name string) (*DockerImage, error) { + var lock ImageLock + if err := json.Unmarshal(content, &lock); err != nil { + return nil, fmt.Errorf("parse image lock: %w", err) + } + image := PublishedImage(name, "") + if want := image.Repository + "/" + image.Name; lock.Name != want { + return nil, fmt.Errorf("image lock names %q, but the image is %q", lock.Name, want) + } + if lock.Digest == "" { + return nil, ErrImageUnpublished + } + if !IsSHA256Digest(lock.Digest) { + return nil, fmt.Errorf("image digest %q must be a sha256 digest", lock.Digest) + } + image.Digest = lock.Digest + return &image, nil +} + +// IsSHA256Digest reports whether digest is a complete sha256 image digest: +// "sha256:" followed by exactly 64 hexadecimal characters. +func IsSHA256Digest(digest string) bool { + algorithm, encoded, found := strings.Cut(digest, ":") + decoded, err := hex.DecodeString(encoded) + return found && algorithm == "sha256" && err == nil && len(decoded) == 32 +} diff --git a/resources/image_lock_test.go b/resources/image_lock_test.go new file mode 100644 index 000000000..ad1d3df9d --- /dev/null +++ b/resources/image_lock_test.go @@ -0,0 +1,73 @@ +package resources_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/codefly-dev/core/resources" +) + +const lockedImageName = "service-warehouse" + +func lock(t *testing.T, fixture string) []byte { + t.Helper() + content, err := os.ReadFile(filepath.Join("testdata", "image-lock", fixture+".json")) + require.NoError(t, err) + return content +} + +func TestParseImageLockPinsTheDigestInCoresRegistry(t *testing.T) { + image, err := resources.ParseImageLock(lock(t, "published"), lockedImageName) + require.NoError(t, err) + require.Equal(t, resources.ImageRegistry, image.Repository) + require.Equal(t, lockedImageName, image.Name) + require.Empty(t, image.Tag, "the lock carries no tag, so a tag cannot disagree with the digest") + require.Equal(t, + "ghcr.io/codefly-dev/service-warehouse@sha256:1205da0743e56b68c12c754438476e8fdd0f05cdb08ce1c9687ddb8d8f0e1669", + image.FullName()) +} + +// A lock that is accepted but names no immutable image is the failure pinning +// exists to prevent, so every malformed shape is refused at load. +func TestParseImageLockRefusesAnythingButASHA256DigestOfThisImage(t *testing.T) { + for _, fixture := range []string{ + "no-name", "other-image", "other-registry", + "tag-as-digest", "other-algorithm", "truncated", "not-hex", "too-long", + "not-json", + } { + t.Run(fixture, func(t *testing.T) { + image, err := resources.ParseImageLock(lock(t, fixture), lockedImageName) + require.Error(t, err, "accepted a lock that pins no image") + require.Nil(t, image) + require.NotErrorIs(t, err, resources.ErrImageUnpublished, + "a wrong lock must not read as a merely unpublished one") + }) + } +} + +// A lock with no digest is the tree before its first image was published: a +// reference with no digest and no tag is the registry's :latest. +func TestAnUnpublishedLockIsRefusedNeverRunAsLatest(t *testing.T) { + image, err := resources.ParseImageLock(lock(t, "unpublished"), lockedImageName) + require.ErrorIs(t, err, resources.ErrImageUnpublished) + require.Nil(t, image) +} + +func TestIsSHA256Digest(t *testing.T) { + const hex64 = "1205da0743e56b68c12c754438476e8fdd0f05cdb08ce1c9687ddb8d8f0e1669" + for digest, want := range map[string]bool{ + "sha256:" + hex64: true, + "sha256:" + hex64 + "00": false, + "sha256:" + hex64[:62]: false, + "sha512:" + hex64: false, + "SHA256:" + hex64: false, + "sha256:": false, + hex64: false, + "": false, + } { + require.Equal(t, want, resources.IsSHA256Digest(digest), "%q", digest) + } +} diff --git a/resources/testdata/image-lock/no-name.json b/resources/testdata/image-lock/no-name.json new file mode 100644 index 000000000..37391f40d --- /dev/null +++ b/resources/testdata/image-lock/no-name.json @@ -0,0 +1 @@ +{"digest": "sha256:1205da0743e56b68c12c754438476e8fdd0f05cdb08ce1c9687ddb8d8f0e1669"} diff --git a/resources/testdata/image-lock/not-hex.json b/resources/testdata/image-lock/not-hex.json new file mode 100644 index 000000000..b9036e93d --- /dev/null +++ b/resources/testdata/image-lock/not-hex.json @@ -0,0 +1,4 @@ +{ + "name": "ghcr.io/codefly-dev/service-warehouse", + "digest": "sha256:zzzzda0743e56b68c12c754438476e8fdd0f05cdb08ce1c9687ddb8d8f0e1669" +} diff --git a/resources/testdata/image-lock/not-json.json b/resources/testdata/image-lock/not-json.json new file mode 100644 index 000000000..4de6f106a --- /dev/null +++ b/resources/testdata/image-lock/not-json.json @@ -0,0 +1 @@ +sha256:1205da0743e56b68c12c754438476e8fdd0f05cdb08ce1c9687ddb8d8f0e1669 diff --git a/resources/testdata/image-lock/other-algorithm.json b/resources/testdata/image-lock/other-algorithm.json new file mode 100644 index 000000000..28c0c44ef --- /dev/null +++ b/resources/testdata/image-lock/other-algorithm.json @@ -0,0 +1,4 @@ +{ + "name": "ghcr.io/codefly-dev/service-warehouse", + "digest": "sha512:1205da07" +} diff --git a/resources/testdata/image-lock/other-image.json b/resources/testdata/image-lock/other-image.json new file mode 100644 index 000000000..76c966c30 --- /dev/null +++ b/resources/testdata/image-lock/other-image.json @@ -0,0 +1,4 @@ +{ + "name": "ghcr.io/codefly-dev/service-object-storage", + "digest": "sha256:1205da0743e56b68c12c754438476e8fdd0f05cdb08ce1c9687ddb8d8f0e1669" +} diff --git a/resources/testdata/image-lock/other-registry.json b/resources/testdata/image-lock/other-registry.json new file mode 100644 index 000000000..53f76461f --- /dev/null +++ b/resources/testdata/image-lock/other-registry.json @@ -0,0 +1,4 @@ +{ + "name": "registry.example.com/codefly-dev/service-warehouse", + "digest": "sha256:1205da0743e56b68c12c754438476e8fdd0f05cdb08ce1c9687ddb8d8f0e1669" +} diff --git a/resources/testdata/image-lock/published.json b/resources/testdata/image-lock/published.json new file mode 100644 index 000000000..e98a4dfca --- /dev/null +++ b/resources/testdata/image-lock/published.json @@ -0,0 +1,4 @@ +{ + "name": "ghcr.io/codefly-dev/service-warehouse", + "digest": "sha256:1205da0743e56b68c12c754438476e8fdd0f05cdb08ce1c9687ddb8d8f0e1669" +} diff --git a/resources/testdata/image-lock/tag-as-digest.json b/resources/testdata/image-lock/tag-as-digest.json new file mode 100644 index 000000000..88dc00186 --- /dev/null +++ b/resources/testdata/image-lock/tag-as-digest.json @@ -0,0 +1,4 @@ +{ + "name": "ghcr.io/codefly-dev/service-warehouse", + "digest": "0.0.3" +} diff --git a/resources/testdata/image-lock/too-long.json b/resources/testdata/image-lock/too-long.json new file mode 100644 index 000000000..b28969381 --- /dev/null +++ b/resources/testdata/image-lock/too-long.json @@ -0,0 +1,4 @@ +{ + "name": "ghcr.io/codefly-dev/service-warehouse", + "digest": "sha256:1205da0743e56b68c12c754438476e8fdd0f05cdb08ce1c9687ddb8d8f0e166900" +} diff --git a/resources/testdata/image-lock/truncated.json b/resources/testdata/image-lock/truncated.json new file mode 100644 index 000000000..ac01fa35b --- /dev/null +++ b/resources/testdata/image-lock/truncated.json @@ -0,0 +1,4 @@ +{ + "name": "ghcr.io/codefly-dev/service-warehouse", + "digest": "sha256:1205da07" +} diff --git a/resources/testdata/image-lock/unpublished.json b/resources/testdata/image-lock/unpublished.json new file mode 100644 index 000000000..24d8284b8 --- /dev/null +++ b/resources/testdata/image-lock/unpublished.json @@ -0,0 +1,4 @@ +{ + "name": "ghcr.io/codefly-dev/service-warehouse", + "digest": "" +} From d9a56a28a07234fed74ed27ee66960222bf04ee2 Mon Sep 17 00:00:00 2001 From: crisfiorini-oss <301773486+crisfiorini-oss@users.noreply.github.com> Date: Fri, 9 Oct 2026 20:10:19 +0000 Subject: [PATCH 3/6] feat(sbom): write image evidence documents and resolve a local image id Services that publish an image copied the code that inventories each shipped platform, writes one CycloneDX document per platform and an index of them, and asks the Docker daemon for a local image's id. Export it beside sbom.Image: ImageSubjects, CollectImageEvidence, WriteImageEvidenceIndex and LocalImageID. LocalImageID reuses inspectDaemonImage, which sbom.Image already uses for a local source. Documents are named after the scanned repository, platform and digest rather than a name typed by the service. Co-Authored-By: Claude Sonnet 5.5 --- agents/services/sbom/evidence.go | 155 ++++++++++++++++++++++++++ agents/services/sbom/evidence_test.go | 146 ++++++++++++++++++++++++ 2 files changed, 301 insertions(+) create mode 100644 agents/services/sbom/evidence.go create mode 100644 agents/services/sbom/evidence_test.go diff --git a/agents/services/sbom/evidence.go b/agents/services/sbom/evidence.go new file mode 100644 index 000000000..903e354fc --- /dev/null +++ b/agents/services/sbom/evidence.go @@ -0,0 +1,155 @@ +package sbom + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + + builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" +) + +// LocalImageID reports the identity the local Docker daemon holds an image +// under. It is the only immutable identity an image that was built and never +// pushed has, because the daemon resolves whatever reference it is handed to +// that ID: a pin sitting only in the reference is not what such a scan binds +// to. docker's own message is kept in the error, because an absent image, an +// absent daemon and an absent docker are different things to go fix and only +// that message tells them apart. +func LocalImageID(ctx context.Context, reference string) (string, error) { + id, _, err := inspectDaemonImage(ctx, ImageRequest{Reference: reference}) + return id, err +} + +// ImageSubjects describes the images a service ships, as the subjects to +// inventory. A published image is multi-architecture and contributes one +// subject per shipped platform, derived from its digest-pinned reference +// (ExpectedFromImageReference, which refuses a bare tag). A local image was +// built and never pushed, so the daemon holds exactly one platform of it: the +// subject names none, which is read back from the image rather than asserted +// by the caller, and carries the image ID instead of a pin sitting in the +// reference (see LocalImageID). +func ImageSubjects(ctx context.Context, image PublishedImage, local bool) ([]*builderv0.ImageSubject, error) { + if !local { + return ExpectedFromImageReference(image) + } + if image.Service == "" { + return nil, fmt.Errorf("image %s cannot be inventoried for no service", image.Reference) + } + id, err := LocalImageID(ctx, image.Reference) + if err != nil { + return nil, err + } + return []*builderv0.ImageSubject{{ + Reference: image.Reference, + Digest: id, + Role: image.Role, + Service: image.Service, + Source: sourceKind(SourceDockerDaemon), + }}, nil +} + +// ImageEvidenceDocument is one image platform's inventory, written to disk and +// named by the digest it is bound to. +type ImageEvidenceDocument struct { + // Reference is the reference the subject asked for. + Reference string + // Digest is the digest actually scanned. + Digest string + // Platform is "os/arch", read back from the image where it states one. + Platform string + // Path is the CycloneDX file. + Path string + // SHA256 is the inventory's content digest, Result.SHA256: the digest of the + // deterministic encoding of the BOM, which is not the digest of the + // CycloneDX file's bytes. + SHA256 string +} + +// CollectImageEvidence inventories every subject through Image and writes one +// CycloneDX document each into dir, creating it. The document carries the +// scanned digest in its own root component, so an exported artifact still +// names the image it describes rather than relying on the filename to say so. +// +// A single failed scan fails the whole call: publishing evidence for some +// platforms while silently omitting others is the false coverage claim this +// evidence exists to prevent. +func CollectImageEvidence(ctx context.Context, dir string, subjects []*builderv0.ImageSubject) ([]ImageEvidenceDocument, error) { + if len(subjects) == 0 { + return nil, fmt.Errorf("no image subjects to inventory") + } + if err := os.MkdirAll(dir, 0o755); err != nil { + return nil, fmt.Errorf("create evidence directory: %w", err) + } + documents := make([]ImageEvidenceDocument, 0, len(subjects)) + for _, subject := range subjects { + result, err := Image(ctx, ImageRequest{ + Reference: subject.GetReference(), + Platform: subject.GetPlatform(), + Source: SourceOf(subject), + }) + if err != nil { + return nil, fmt.Errorf("inventory %s: %w", describeSubject(subject), err) + } + document, err := writeImageEvidence(dir, result) + if err != nil { + return nil, fmt.Errorf("%s: %w", describeSubject(subject), err) + } + documents = append(documents, document) + } + return documents, nil +} + +// writeImageEvidence encodes one scanned image as CycloneDX and writes it. +func writeImageEvidence(dir string, result *ImageResult) (ImageEvidenceDocument, error) { + encoded, err := MarshalCycloneDXJSON(result.Bom) + if err != nil { + return ImageEvidenceDocument{}, fmt.Errorf("encode: %w", err) + } + path := filepath.Join(dir, evidenceFileName(result.Reference, result.Platform, result.Digest)) + if err := os.WriteFile(path, encoded, 0o644); err != nil { + return ImageEvidenceDocument{}, fmt.Errorf("write %s: %w", path, err) + } + return ImageEvidenceDocument{ + Reference: result.Reference, + Digest: result.Digest, + Platform: result.Platform, + Path: path, + SHA256: result.SHA256, + }, nil +} + +// WriteImageEvidenceIndex records which image and platform each document +// covers in dir/index.txt, so a release asset is readable without opening every +// document. One line per document: platform, repository@digest, file name, and +// the inventory's SHA256. The repository is the reference stripped of any tag, so +// the index names the digest actually scanned rather than the tag asked for. +func WriteImageEvidenceIndex(dir string, documents []ImageEvidenceDocument) error { + var index strings.Builder + for _, document := range documents { + fmt.Fprintf(&index, "%s %s@%s %s %s\n", + document.Platform, referenceName(document.Reference), document.Digest, + filepath.Base(document.Path), document.SHA256) + } + return os.WriteFile(filepath.Join(dir, "index.txt"), []byte(index.String()), 0o644) +} + +// evidenceFileName names a document after the repository it inventories, the +// platform and the digest it is bound to, none of which a caller types: +// "service-warehouse-linux-amd64-.cdx.json". +func evidenceFileName(reference, platform, digest string) string { + repository := referenceName(reference) + name := repository[strings.LastIndex(repository, "/")+1:] + if platform != "" { + name += "-" + strings.ReplaceAll(platform, "/", "-") + } + return name + "-" + strings.TrimPrefix(digest, "sha256:") + ".cdx.json" +} + +func describeSubject(subject *builderv0.ImageSubject) string { + if platform := subject.GetPlatform(); platform != "" { + return subject.GetReference() + " on " + platform + } + return subject.GetReference() +} diff --git a/agents/services/sbom/evidence_test.go b/agents/services/sbom/evidence_test.go new file mode 100644 index 000000000..300ce7040 --- /dev/null +++ b/agents/services/sbom/evidence_test.go @@ -0,0 +1,146 @@ +package sbom + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + agentv0 "github.com/codefly-dev/core/generated/go/codefly/services/agent/v0" + builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" +) + +const ( + digestAMD64 = "sha256:c64c687cbea9300178b30c95835354e34c4e4febc4badfe27102879de0483b5e" + digestARM64 = "sha256:6243dec9286873e0392f0527f96c394684dc6fa12661f0bd1925abcdef012345" + pinned = "ghcr.io/codefly-dev/service-warehouse@sha256:1205da0743e56b68c12c754438476e8fdd0f05cdb08ce1c9687ddb8d8f0e1669" +) + +// scanned is what Image returns for a platform of the image: a real bound +// CycloneDX document, built the way Image builds it from a scanner's output. +func scanned(t *testing.T, reference, platform, digest string) *ImageResult { + t.Helper() + root := &agentv0.Component{Name: "service-warehouse", Type: agentv0.ComponentType_MODULE, BomRef: "root"} + packages := []*agentv0.Component{{Name: "openssl", Version: "3.0", BomRef: "pkg:deb/openssl@3.0"}} + base, err := finish(root, packages, nil, "syft", "DOCKER") + require.NoError(t, err) + bound, err := bindImageDigest(base, digest) + require.NoError(t, err) + return &ImageResult{Result: bound, Reference: reference, Digest: digest, Platform: platform} +} + +// The document on disk is the CycloneDX encoding of the scanned image, carries +// the digest it is bound to in its own root, and is named by repository, +// platform and digest so a directory of them is readable at a glance. +func TestWriteImageEvidenceNamesAndBindsTheDocument(t *testing.T) { + dir := t.TempDir() + result := scanned(t, pinned, "linux/amd64", digestAMD64) + document, err := writeImageEvidence(dir, result) + require.NoError(t, err) + + require.Equal(t, filepath.Join(dir, + "service-warehouse-linux-amd64-c64c687cbea9300178b30c95835354e34c4e4febc4badfe27102879de0483b5e.cdx.json"), document.Path) + require.Equal(t, pinned, document.Reference) + require.Equal(t, digestAMD64, document.Digest) + require.Equal(t, "linux/amd64", document.Platform) + + onDisk, err := os.ReadFile(document.Path) + require.NoError(t, err) + require.Contains(t, string(onDisk), strings.TrimPrefix(digestAMD64, "sha256:"), + "the document must name the image it describes itself, not only through its file name") + require.True(t, json.Valid(onDisk), "document is valid JSON") + require.Contains(t, string(onDisk), `"bomFormat": "CycloneDX"`) + require.Equal(t, result.SHA256, document.SHA256, "the index carries the inventory's own digest") +} + +func TestEvidenceFileName(t *testing.T) { + for _, tc := range []struct{ reference, platform, digest, want string }{ + {pinned, "linux/arm64", "sha256:abc", "service-warehouse-linux-arm64-abc.cdx.json"}, + {"localhost:5000/team/app:dev", "linux/amd64", "sha256:abc", "app-linux-amd64-abc.cdx.json"}, + {"redis:7", "linux/amd64", "sha256:abc", "redis-linux-amd64-abc.cdx.json"}, + {pinned, "", "sha256:abc", "service-warehouse-abc.cdx.json"}, + } { + require.Equal(t, tc.want, evidenceFileName(tc.reference, tc.platform, tc.digest), tc.reference) + } +} + +func TestWriteImageEvidenceIndexListsEveryDocument(t *testing.T) { + dir := t.TempDir() + var documents []ImageEvidenceDocument + for _, platform := range []struct{ name, digest string }{{"linux/amd64", digestAMD64}, {"linux/arm64", digestARM64}} { + document, err := writeImageEvidence(dir, scanned(t, "ghcr.io/codefly-dev/service-warehouse:0.1.0", platform.name, platform.digest)) + require.NoError(t, err) + documents = append(documents, document) + } + require.NoError(t, WriteImageEvidenceIndex(dir, documents)) + + index, err := os.ReadFile(filepath.Join(dir, "index.txt")) + require.NoError(t, err) + lines := strings.Split(strings.TrimSpace(string(index)), "\n") + require.Len(t, lines, 2) + for i, document := range documents { + // The tag that was asked for is replaced by the digest that was scanned. + want := strings.Join([]string{ + document.Platform, + "ghcr.io/codefly-dev/service-warehouse@" + document.Digest, + filepath.Base(document.Path), + document.SHA256, + }, " ") + require.Equal(t, want, lines[i]) + } +} + +func TestCollectImageEvidenceRefusesNoSubjects(t *testing.T) { + _, err := CollectImageEvidence(context.Background(), t.TempDir(), nil) + require.ErrorContains(t, err, "no image subjects") +} + +// Evidence for some platforms with the rest silently omitted is a false +// coverage claim, so a subject that cannot be scanned fails the whole call and +// names the subject. An empty reference cannot be scanned on any machine, with +// or without a scanner installed. +func TestCollectImageEvidenceFailsWholeOnOneUnscannableSubject(t *testing.T) { + dir := t.TempDir() + _, err := CollectImageEvidence(context.Background(), dir, []*builderv0.ImageSubject{ + {Reference: "", Platform: "linux/arm64"}, + }) + require.ErrorContains(t, err, "inventory on linux/arm64") + require.ErrorContains(t, err, "image SBOM requires an image reference") +} + +func TestImageSubjectsForAPublishedImageIsOneSubjectPerPlatform(t *testing.T) { + subjects, err := ImageSubjects(context.Background(), PublishedImage{ + Service: "codefly.dev/warehouse", + Role: "runtime", + Reference: pinned, + Platforms: []string{"linux/amd64", "linux/arm64"}, + }, false) + require.NoError(t, err) + require.Len(t, subjects, 2) + for i, platform := range []string{"linux/amd64", "linux/arm64"} { + require.Equal(t, platform, subjects[i].GetPlatform()) + require.Equal(t, pinned, subjects[i].GetReference()) + require.Equal(t, "runtime", subjects[i].GetRole()) + require.Equal(t, "codefly.dev/warehouse", subjects[i].GetService()) + require.Equal(t, SourceRegistry, SourceOf(subjects[i])) + } +} + +// A tag serves whatever was pushed to it last, so a published image that is +// not pinned is refused rather than inventoried as if it were the shipped one. +func TestImageSubjectsRefuseAPublishedImageThatIsNotPinned(t *testing.T) { + _, err := ImageSubjects(context.Background(), PublishedImage{ + Service: "codefly.dev/warehouse", Role: "runtime", + Reference: "ghcr.io/codefly-dev/service-warehouse:0.1.0", + }, false) + require.ErrorContains(t, err, "not pinned to a sha256 digest") +} + +func TestImageSubjectsRefuseALocalImageForNoService(t *testing.T) { + _, err := ImageSubjects(context.Background(), PublishedImage{Reference: "service-warehouse:local"}, true) + require.ErrorContains(t, err, "no service") +} From f08a1d5826a27366313aef4ecfa097b7a6699bf7 Mon Sep 17 00:00:00 2001 From: crisfiorini-oss <301773486+crisfiorini-oss@users.noreply.github.com> Date: Fri, 9 Oct 2026 20:10:19 +0000 Subject: [PATCH 4/6] ci: add a reusable workflow that publishes a service image by digest Two services carry the same 76-line workflow that builds an image for several platforms, pushes it by digest and records the digest as a lock for the release to pin. publish-service-image.yml is that workflow, parameterised by image name, version, lock file name and platforms. The job receives the caller's registry credential as the secret registry-token, so it is a registered credential job: gated to a dispatch, refusing a commit that is not reachable from the caller's default branch before anything else runs, with its shell executed against real repositories by internal/ciguard. The inputs reach shell only as environment values and are validated before the login step. Co-Authored-By: Claude Sonnet 5.5 --- .github/workflows/publish-service-image.yml | 186 ++++++++++++++++ docs/ci-credentials.md | 1 + docs/runbooks/branch-protection.md | 2 +- internal/ciguard/job_templates_test.go | 16 +- internal/ciguard/service_image_test.go | 222 ++++++++++++++++++++ 5 files changed, 421 insertions(+), 6 deletions(-) create mode 100644 .github/workflows/publish-service-image.yml create mode 100644 internal/ciguard/service_image_test.go diff --git a/.github/workflows/publish-service-image.yml b/.github/workflows/publish-service-image.yml new file mode 100644 index 000000000..4c2a6b5ff --- /dev/null +++ b/.github/workflows/publish-service-image.yml @@ -0,0 +1,186 @@ +name: publish-service-image + +# Builds a service's image for several platforms and pushes it BY DIGEST, +# carrying no tag, then records the digest as for the caller to +# commit. Nothing resolves the image yet: the caller's release creates the tags +# consumers use, from the digest the lock records. +# +# It runs before the release tag rather than from it. A service pins the digest +# it runs, and a digest is only known once the image is built, so a tag cut +# before the build can never carry it. Publishing here, committing the digest, +# and tagging afterwards is what lets the released service name the image that +# was actually built for its version. resources.ParseImageLock reads the lock. +# +# A caller is a workflow_dispatch workflow with a `version` input and one job +# that calls this workflow by its job-level `uses`, pinned to an immutable core +# commit SHA rather than @main. That job grants `contents: read` and +# `packages: write`, passes `image-name` (the repository's own name, say +# service-warehouse) and `version` under `with:`, and passes the registry +# credential under `secrets:` as `registry-token: ${{ secrets.GITHUB_TOKEN }}`. +# +# The registry credential is a secret the caller passes, not this workflow's +# own token. `permissions:` here can only narrow what the caller grants, and a +# reusable workflow declares no write (see internal/ciguard); the caller's job +# is where `packages: write` is decided, and the token travels as a value. +on: + workflow_call: + inputs: + image-name: + description: Name of the image inside core's image registry (resources.ImageRegistry), normally the repository's own name. + required: true + type: string + version: + description: Upcoming stable service version, without a v prefix. Stamped on the image as org.opencontainers.image.version. + required: true + type: string + lock-file: + description: File name recorded for the caller to commit, -image.json. + type: string + default: gateway-image.json + platforms: + description: Comma-separated platforms to build. The evidence the caller publishes has to cover exactly these. + type: string + default: linux/amd64,linux/arm64 + secrets: + registry-token: + description: Credential that can push to the registry, normally the caller's GITHUB_TOKEN from a job that grants packages write. + required: true + outputs: + digest: + description: The sha256 digest of the pushed image index. + value: ${{ jobs.publish.outputs.digest }} + +env: + # core's resources.ImageRegistry. A workflow cannot import a Go constant, so + # internal/ciguard holds the two together. + REGISTRY: ghcr.io/codefly-dev + +jobs: + # This job holds the caller's registry credential, and `permissions:` says + # nothing about a secret. It builds the CALLER's Dockerfile, so what keeps + # that credential from code nobody merged is the same pair of conditions + # go-service-release.yml's release job relies on: the `if:` keeps the job off + # every event but a dispatch, which a pull request cannot produce, and the + # first step refuses a commit that is not reachable from the caller's default + # branch, because a dispatch can name any branch. internal/ciguard holds both. + publish: + if: github.event_name == 'workflow_dispatch' + runs-on: ubuntu-latest + permissions: + # The registry is reached with the secret above, never with this job's + # token, so this job needs no write grant -- and a caller that grants + # packages write does not hand that grant to anything here. + contents: read + outputs: + digest: ${{ steps.build.outputs.digest }} + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 0 + # Credentials stay persisted here, deliberately. The refusal below + # fetches the default branch, and a private caller cannot fetch + # anonymously. What is persisted is this job's own read-only built-in + # token, which expires with the job -- not the registry credential, + # which is only ever passed to the login step. + + - name: require the commit to be on the default branch, or refuse + run: | + set -euo pipefail + + # The authority is the remote's own HEAD, and there is no input that + # can name it. A declaration the caller supplies could be omitted, + # defaulted or disagreed with, and each of those is a way for the + # caller to choose which history counts as reviewed. Whoever may + # approve a publication is this repository's decision, so the only + # source is the repository. + authority=$(git ls-remote --symref origin HEAD \ + | awk '$1 == "ref:" && $3 == "HEAD" { sub("refs/heads/", "", $2); print $2 }') + if [ -z "$authority" ]; then + echo "::error::cannot read this repository's default branch from origin; refusing to publish." >&2 + exit 1 + fi + + git fetch --quiet --no-tags --force origin \ + "+refs/heads/${authority}:refs/remotes/origin/${authority}" + + # Fully qualified: a bare `origin/` is an ambiguous rev that a tag + # of the same name can win. + if ! git merge-base --is-ancestor "$GITHUB_SHA" "refs/remotes/origin/${authority}"; then + echo "::error::${GITHUB_REF_NAME} points at ${GITHUB_SHA}, which is not reachable from refs/remotes/origin/${authority}; refusing to publish it." >&2 + echo "Publishing builds this repository's Dockerfile with a registry credential, so the commit has to be one that was merged." >&2 + exit 1 + fi + + # Through the environment, never interpolated into the script: a `run:` + # built by substituting a caller-supplied string is assembled before any + # shell sees it, so the string becomes part of the program. + - name: Validate the inputs + id: inputs + env: + IMAGE_NAME: ${{ inputs.image-name }} + VERSION: ${{ inputs.version }} + LOCK_FILE: ${{ inputs.lock-file }} + PLATFORMS: ${{ inputs.platforms }} + run: | + set -euo pipefail + if [[ ! "$IMAGE_NAME" =~ ^[a-z0-9]+([._-][a-z0-9]+)*$ ]]; then + echo "image-name must be a lowercase repository name inside the registry" >&2 + exit 1 + fi + if [[ ! "$VERSION" =~ ^(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)$ ]]; then + echo "version must be a stable semantic version without a v prefix" >&2 + exit 1 + fi + if [[ ! "$LOCK_FILE" =~ ^[a-z0-9]+(-[a-z0-9]+)*-image\.json$ ]]; then + echo "lock-file must be a plain file name of the form -image.json" >&2 + exit 1 + fi + if [[ ! "$PLATFORMS" =~ ^[a-z0-9]+/[a-z0-9]+(,[a-z0-9]+/[a-z0-9]+)*$ ]]; then + echo "platforms must be comma-separated os/arch pairs" >&2 + exit 1 + fi + { + echo "image=${REGISTRY}/${IMAGE_NAME}" + echo "version=$VERSION" + } >> "$GITHUB_OUTPUT" + + - uses: docker/setup-buildx-action@f87e5991a6d7451dcb8d9637bfbc97413f497069 # v4.4.1 + - uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3.7.0 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.registry-token }} + + # The version label is how a release tells this image apart from the one + # published for an earlier version: both are digests in the same + # repository, and a lock left unrefreshed would otherwise publish the + # previous release's bytes under the new tag. + - uses: docker/build-push-action@c3c9e263c25d99ce0380d002d59b67737d91b0dc # v7.4.0 + id: build + with: + context: . + platforms: ${{ inputs.platforms }} + labels: org.opencontainers.image.version=${{ steps.inputs.outputs.version }} + outputs: type=image,name=${{ steps.inputs.outputs.image }},push-by-digest=true,name-canonical=true,push=true + + - name: Record the digest + env: + IMAGE: ${{ steps.inputs.outputs.image }} + DIGEST: ${{ steps.build.outputs.digest }} + LOCK_FILE: ${{ inputs.lock-file }} + run: | + set -euo pipefail + mkdir -p release-evidence + jq -n --arg name "$IMAGE" --arg digest "$DIGEST" '{name: $name, digest: $digest}' > "release-evidence/${LOCK_FILE}" + { + echo "Commit this as \`${LOCK_FILE}\`, then tag the release:" + echo + echo '```json' + cat "release-evidence/${LOCK_FILE}" + echo '```' + } >> "$GITHUB_STEP_SUMMARY" + - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: ${{ inputs.lock-file }}-${{ steps.inputs.outputs.version }} + path: release-evidence/${{ inputs.lock-file }} + if-no-files-found: error diff --git a/docs/ci-credentials.md b/docs/ci-credentials.md index 4c51370cd..9a358a36d 100644 --- a/docs/ci-credentials.md +++ b/docs/ci-credentials.md @@ -109,6 +109,7 @@ known shape has no such set. | `go.yml` | `notify` holds `SLACK_WEBHOOK_URL` | Its own job, gated to a push of `main`, with `permissions: {}` and no checkout at all. It reads the outcome from `needs.build.result`. | | `version-tag.yml` | `tag` creates the release tag and publishes the release | Event and head-repository conditions; checks out the default branch; refuses a sha not reachable from it. | | `go-service-release.yml` | `goreleaser` receives the **caller's** release PAT | Gated to a pushed tag, which a pull request cannot produce; then refuses a tag whose commit is not reachable from the caller's default branch. The `test` job that runs the caller's suite names no secret. | +| `publish-service-image.yml` | `publish` receives the **caller's** registry credential, passed as the secret `registry-token` | Gated to a `workflow_dispatch`, which a pull request cannot produce; then refuses a commit that is not reachable from the caller's default branch, as the first step after the checkout and before the login that receives the credential or the build that runs the caller's Dockerfile. A reusable workflow declares no write, so the caller's own job is where `packages: write` is granted and its token travels as a value. | | `combine-deps.yml` | `publish` holds `DEPS_COMBINE_TOKEN` | A job of its own that never checks the combined tree out: the commits arrive as a git bundle and move into a ref by `git fetch`/`git push`. The `plan` job does the replaying, with the read-only built-in token and no secret. Both checkouts pin `ref: main`. | Every other job in every other workflow declares `contents: read` and names no diff --git a/docs/runbooks/branch-protection.md b/docs/runbooks/branch-protection.md index 51c788667..69cdc3aaa 100644 --- a/docs/runbooks/branch-protection.md +++ b/docs/runbooks/branch-protection.md @@ -116,7 +116,7 @@ is a separate job because it is the one write in that workflow and the job that runs the suite must not hold it — see [CI credentials](../ci-credentials.md). -**`agent-ci`, `go-service-ci`, `go-service-release`.** `workflow_call` only — +**`agent-ci`, `go-service-ci`, `go-service-release`, `publish-service-image`.** `workflow_call` only — they run when a service repository dispatches them, never here. **Approving reviews.** `required_pull_request_reviews` stays null deliberately. diff --git a/internal/ciguard/job_templates_test.go b/internal/ciguard/job_templates_test.go index 48d2eda12..46faf0460 100644 --- a/internal/ciguard/job_templates_test.go +++ b/internal/ciguard/job_templates_test.go @@ -34,7 +34,7 @@ import ( // // Changing one of these jobs therefore means updating its template and the test // that executes it, deliberately, in the same change. That friction is the -// point: these five jobs are the only ones in this repository that receive a +// point: these six jobs are the only ones in this repository that receive a // secret or a write token. // jobTemplate is one permitted shape. @@ -61,6 +61,11 @@ var permittedCredentialJobs = []jobTemplate{ executedBy: "TestAReleaseIsAdmittedOnlyFromTheRepositorysOwnDefaultBranch", why: "publishes a release for a tag proven to be on the repository's own default branch", }, + { + workflow: "publish-service-image.yml", job: "publish", + executedBy: "TestAServiceImageIsPublishedOnlyFromTheRepositorysOwnDefaultBranch", + why: "pushes a caller's service image by digest with the caller's registry credential, from a commit proven to be on the repository's own default branch", + }, { workflow: "combine-deps.yml", job: "publish", executedBy: "TestTheCombinedBranchTravelsAsABundleWithoutBeingCheckedOut", @@ -178,10 +183,11 @@ var recordedFileDigests = map[string]string{ } var recordedDigests = map[string]string{ - "combine-deps.yml": "2ddc71ebfde5eff4d8646d53dca254a9b631a4a951589d5ff587962c09e43e50", - "go-service-release.yml": "b96f9dcb767f9a307b4120797007a30bad8b76bb8bd7b461247a555102a54708", - "go.yml": "e484c97790a696fcd2a55e323df7d4b451b7657a917a126d7c6831e02cd2a904", - "version-tag.yml": "25972c82482fb3ae6196371328df126843c3e8a1b4aae7b863693bbad5fac641", + "combine-deps.yml": "2ddc71ebfde5eff4d8646d53dca254a9b631a4a951589d5ff587962c09e43e50", + "go-service-release.yml": "b96f9dcb767f9a307b4120797007a30bad8b76bb8bd7b461247a555102a54708", + "go.yml": "e484c97790a696fcd2a55e323df7d4b451b7657a917a126d7c6831e02cd2a904", + "publish-service-image.yml": "b3424deb195f36102aeb6064f28ff8a4be2e27ae19f05785aaf910081180b282", + "version-tag.yml": "25972c82482fb3ae6196371328df126843c3e8a1b4aae7b863693bbad5fac641", } // TestEveryPermittedJobMatchesItsRecordedShape is the gate: each permitted job diff --git a/internal/ciguard/service_image_test.go b/internal/ciguard/service_image_test.go new file mode 100644 index 000000000..6e0270a97 --- /dev/null +++ b/internal/ciguard/service_image_test.go @@ -0,0 +1,222 @@ +package ciguard + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/codefly-dev/core/resources" +) + +// publish-service-image.yml hands a registry credential to a job that builds +// the CALLER's Dockerfile, so it is a registered credential job and the shell +// that decides whether that is safe is lifted out of the workflow and executed, +// the way go-service-release.yml's release refusal is. Reading shell in a YAML +// string establishes nothing about what it does. + +const ( + publishWorkflow = "publish-service-image.yml" + publishJob = "publish" + publishRefusalStep = "require the commit to be on the default branch, or refuse" + publishInputsStep = "Validate the inputs" +) + +// publishSteps returns the steps of the publish job. +func publishSteps(t *testing.T) []isolatedStep { + t.Helper() + + _, workflows := loadIsolatedWorkflows(t) + wf, ok := workflows[filepath.Join(repoRoot(t), ".github", "workflows", publishWorkflow)] + require.True(t, ok, "%s did not load", publishWorkflow) + job, ok := wf.Jobs[publishJob] + require.True(t, ok, "%s has no job %q", publishWorkflow, publishJob) + return job.Steps +} + +// publishStep lifts one named step out of the publish job. +func publishStep(t *testing.T, name string) isolatedStep { + t.Helper() + + for _, step := range publishSteps(t) { + if step.Name == name { + require.NotEmpty(t, step.Run) + return step + } + } + t.Fatalf("no step named %q, so the shell this test exercises is not the one that runs", name) + return isolatedStep{} +} + +// A commit is published only when it is reachable from the repository's OWN +// default branch: a dispatch can name any branch, and the Dockerfile it builds +// runs with the registry credential. +func TestAServiceImageIsPublishedOnlyFromTheRepositorysOwnDefaultBranch(t *testing.T) { + script := publishStep(t, publishRefusalStep).Run + + for _, tc := range []struct { + name string + commit string + admit bool + }{ + {name: "the default branch's tip", commit: "tip", admit: true}, + {name: "an older commit on the default branch", commit: "older", admit: true}, + // Genuinely reachable from real branches, and still not the default + // branch: nominating one's own history as the authority must not admit + // a commit that was never merged. + {name: "a commit on no default branch", commit: "unmerged", admit: false}, + } { + t.Run(tc.name, func(t *testing.T) { + fixture := newReleaseFixture(t) + sha := map[string]string{ + "tip": fixture.tip, "older": fixture.older, "unmerged": fixture.unmerged, + }[tc.commit] + + admitted, output := fixture.run(t, script, theDefaultBranch, sha) + require.Equal(t, tc.admit, admitted, "commit %s:\n%s", tc.commit, output) + if !tc.admit { + require.Contains(t, output, "refusing to publish", + "a refusal must say so, with the reason") + } + }) + } +} + +// The refusal has to be the thing that decides: neutralising it must be +// visible, which makes the test above one of behaviour rather than of text. +func TestNeutralisingThePublishRefusalIsVisible(t *testing.T) { + script := publishStep(t, publishRefusalStep).Run + fixture := newReleaseFixture(t) + + admitted, output := fixture.run(t, script, theDefaultBranch, fixture.unmerged) + require.False(t, admitted, "%s", output) + + admitted, _ = fixture.run(t, strings.ReplaceAll(script, "exit 1", "exit 0"), theDefaultBranch, fixture.unmerged) + require.True(t, admitted, + "with every refusal turned into a success the unmerged commit is still "+ + "rejected, so something other than the refusal decided it") +} + +// The refusal takes nothing from the caller, so the authority it checks against +// has one source, and it runs before anything the caller's tree can influence: +// the first step after the checkout, ahead of the validation, the login that +// receives the credential and the build that runs the Dockerfile. +func TestThePublishRefusalTakesNoInputAndRunsFirst(t *testing.T) { + steps := publishSteps(t) + require.GreaterOrEqual(t, len(steps), 2) + require.True(t, strings.HasPrefix(steps[0].Uses, "actions/checkout@"), "the first step is the checkout") + require.Equal(t, publishRefusalStep, steps[1].Name, + "the refusal must be the first thing that runs after the checkout") + require.Empty(t, steps[1].Env, "the refusal receives %v; it must take nothing from the caller", steps[1].Env) + require.Empty(t, steps[1].If, "a condition on the refusal is a way to skip it") + + for _, step := range steps[2:] { + for _, text := range []string{step.Run, step.If} { + require.NotContains(t, text, "inputs.", + "step %q reads a caller input outside the environment; inputs reach shell only as values", step.Name) + } + } +} + +// The credential job is reachable by a dispatch and by nothing a pull request +// or a merge queue can produce. +func TestThePublishJobCannotRunOnAPullRequestOrInAMergeQueue(t *testing.T) { + _, workflows := loadIsolatedWorkflows(t) + wf := workflows[filepath.Join(repoRoot(t), ".github", "workflows", publishWorkflow)] + require.True(t, cannotRunOnAPullRequest(t, wf.Jobs[publishJob].If), + "the publish job holds a registry credential and its condition %q is not provably false on a pull request", + wf.Jobs[publishJob].If) +} + +// The workflow cannot import resources.ImageRegistry, so a test holds the two +// together: an image pushed to one registry and a lock read against another is +// a service that never starts. +func TestThePublishWorkflowPushesToCoresImageRegistry(t *testing.T) { + raw, err := os.ReadFile(filepath.Join(repoRoot(t), ".github", "workflows", publishWorkflow)) + require.NoError(t, err) + require.Contains(t, string(raw), "\n REGISTRY: "+resources.ImageRegistry+"\n") +} + +// Only the inputs' values reach shell, and each is validated before the login +// step receives the credential. The validation is shell in a YAML string, so it +// is executed. +func TestThePublishWorkflowValidatesItsInputsBeforeTheCredentialIsUsed(t *testing.T) { + steps := publishSteps(t) + validated, loggedIn := -1, -1 + for i, step := range steps { + if step.Name == publishInputsStep { + validated = i + } + if strings.HasPrefix(step.Uses, "docker/login-action@") { + loggedIn = i + } + } + require.NotEqual(t, -1, validated) + require.NotEqual(t, -1, loggedIn) + require.Less(t, validated, loggedIn, "the inputs are validated before the credential is used") + + step := publishStep(t, publishInputsStep) + valid := map[string]string{ + "IMAGE_NAME": "service-warehouse", + "VERSION": "1.2.3", + "LOCK_FILE": "gateway-image.json", + "PLATFORMS": "linux/amd64,linux/arm64", + "REGISTRY": resources.ImageRegistry, + } + run := func(t *testing.T, overrides map[string]string) (bool, string, string) { + t.Helper() + env := append([]string{}, os.Environ()...) + for key, value := range valid { + if override, ok := overrides[key]; ok { + value = override + } + env = append(env, key+"="+value) + } + output := filepath.Join(t.TempDir(), "output") + env = append(env, "GITHUB_OUTPUT="+output) + command := exec.Command("bash", "-c", step.Run) + command.Env = env + combined, err := command.CombinedOutput() + written, _ := os.ReadFile(output) + return err == nil, string(combined), string(written) + } + + ok, output, written := run(t, nil) + require.True(t, ok, output) + require.Equal(t, "image=ghcr.io/codefly-dev/service-warehouse\nversion=1.2.3\n", written) + + for name, overrides := range map[string]map[string]string{ + "an image name with a registry": {"IMAGE_NAME": "ghcr.io/other/app"}, + "an image name with upper case": {"IMAGE_NAME": "Service"}, + "an empty image name": {"IMAGE_NAME": ""}, + "an image name injecting an output": {"IMAGE_NAME": "app\nversion=9.9.9"}, + "a v prefixed version": {"VERSION": "v1.2.3"}, + "a two part version": {"VERSION": "1.2"}, + "a leading zero": {"VERSION": "01.2.3"}, + "a version injecting an output": {"VERSION": "1.2.3\nimage=evil"}, + "a lock file in another directory": {"LOCK_FILE": "../gateway-image.json"}, + "a lock file not ending in -image": {"LOCK_FILE": "gateway.json"}, + "platforms with a shell metacharacter": {"PLATFORMS": "linux/amd64;rm -rf /"}, + "empty platforms": {"PLATFORMS": ""}, + } { + t.Run(name, func(t *testing.T) { + ok, output, written := run(t, overrides) + require.False(t, ok, "accepted: %s", output) + require.Empty(t, written, "a refused input must write no output") + }) + } + for name, overrides := range map[string]map[string]string{ + "another lock name": {"LOCK_FILE": "minio-image.json"}, + "one platform": {"PLATFORMS": "linux/arm64"}, + "a zero version": {"VERSION": "0.0.0"}, + "a dotted image name": {"IMAGE_NAME": "service.warehouse"}, + } { + t.Run("accepts "+name, func(t *testing.T) { + ok, output, _ := run(t, overrides) + require.True(t, ok, output) + }) + } +} From 3910bf7fe0b883b0d19f160c883e6e1ef365dd9e Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sat, 10 Oct 2026 12:14:59 -0400 Subject: [PATCH 5/6] fix(sbom): bind exported image evidence to the requested identity --- agents/services/base_builder.go | 12 +-- agents/services/sbom/evidence.go | 6 +- .../sbom/evidence_integration_test.go | 76 +++++++++++++++ agents/services/sbom/evidence_test.go | 21 +++- agents/services/sbom/image.go | 26 ++++- .../sbom/testdata/image-evidence/Dockerfile | 2 + .../image-evidence/first/package.json | 1 + .../image-evidence/second/package.json | 1 + docs/ci-credentials.md | 4 +- docs/service-image-plumbing.md | 97 +++++++++++++++++++ 10 files changed, 226 insertions(+), 20 deletions(-) create mode 100644 agents/services/sbom/evidence_integration_test.go create mode 100644 agents/services/sbom/testdata/image-evidence/Dockerfile create mode 100644 agents/services/sbom/testdata/image-evidence/first/package.json create mode 100644 agents/services/sbom/testdata/image-evidence/second/package.json create mode 100644 docs/service-image-plumbing.md diff --git a/agents/services/base_builder.go b/agents/services/base_builder.go index 5ff07a360..5cdb0062a 100644 --- a/agents/services/base_builder.go +++ b/agents/services/base_builder.go @@ -473,20 +473,10 @@ func (s *BuilderWrapper) SBOMImages(ctx context.Context, subjects []*builderv0.I var images []*builderv0.ImageSBOM index := map[string]*builderv0.ImageSBOM{} for _, subject := range subjects { - if err := servicesbom.RequirePinned(subject); err != nil { - return s.SBOMImageError(err) - } - result, err := servicesbom.Image(ctx, servicesbom.ImageRequest{ - Reference: subject.GetReference(), - Platform: subject.GetPlatform(), - Source: servicesbom.SourceOf(subject), - }) + result, err := servicesbom.ImageForSubject(ctx, subject) if err != nil { return s.SBOMImageError(err) } - if want := subject.GetDigest(); want != "" && want != result.Digest { - return s.SBOMImageError(fmt.Errorf("image %s resolved to digest %s, not the requested %s", subject.GetReference(), result.Digest, want)) - } key := result.Digest + "|" + result.Platform if existing, ok := index[key]; ok { existing.Subjects = append(existing.Subjects, subject) diff --git a/agents/services/sbom/evidence.go b/agents/services/sbom/evidence.go index 903e354fc..1f3d39c3f 100644 --- a/agents/services/sbom/evidence.go +++ b/agents/services/sbom/evidence.go @@ -84,11 +84,7 @@ func CollectImageEvidence(ctx context.Context, dir string, subjects []*builderv0 } documents := make([]ImageEvidenceDocument, 0, len(subjects)) for _, subject := range subjects { - result, err := Image(ctx, ImageRequest{ - Reference: subject.GetReference(), - Platform: subject.GetPlatform(), - Source: SourceOf(subject), - }) + result, err := ImageForSubject(ctx, subject) if err != nil { return nil, fmt.Errorf("inventory %s: %w", describeSubject(subject), err) } diff --git a/agents/services/sbom/evidence_integration_test.go b/agents/services/sbom/evidence_integration_test.go new file mode 100644 index 000000000..ddc03bc6e --- /dev/null +++ b/agents/services/sbom/evidence_integration_test.go @@ -0,0 +1,76 @@ +//go:build image_evidence_required + +package sbom + +import ( + "context" + "os" + "os/exec" + "strconv" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// This uses real, locally built scratch images and the installed syft, with no +// registry pulls. Run with -tags=image_evidence_required; missing tools fail. +func TestLocalImageEvidenceRemainsBoundWhenATagMoves(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + _, err := exec.LookPath("syft") + require.NoError(t, err) + name := "codefly-evidence-" + strconv.FormatInt(time.Now().UnixNano(), 10) + ":test" + build := func(fixture string) string { + t.Helper() + out, err := exec.CommandContext(ctx, "docker", "build", "--quiet", "--tag", name+"-"+fixture, + "-f", "testdata/image-evidence/Dockerfile", "testdata/image-evidence/"+fixture).CombinedOutput() + require.NoError(t, err, "%s", out) + out, err = exec.CommandContext(ctx, "docker", "tag", name+"-"+fixture, name).CombinedOutput() + require.NoError(t, err, "%s", out) + id, err := LocalImageID(ctx, name) + require.NoError(t, err) + t.Cleanup(func() { + out, err := exec.Command("docker", "image", "rm", "--force", id).CombinedOutput() + require.NoError(t, err, "%s", out) + }) + return id + } + first := build("first") + subjects, err := ImageSubjects(ctx, PublishedImage{Service: "evidence-fixture", Reference: name}, true) + require.NoError(t, err) + require.Equal(t, first, subjects[0].GetDigest()) + + dir := t.TempDir() + documents, err := CollectImageEvidence(ctx, dir, subjects) + require.NoError(t, err) + require.Len(t, documents, 1) + require.Equal(t, first, documents[0].Digest) + require.NotEmpty(t, documents[0].Platform) + raw, err := os.ReadFile(documents[0].Path) + require.NoError(t, err) + require.Contains(t, string(raw), `"version": "1.0.0"`) + require.NoError(t, WriteImageEvidenceIndex(dir, documents)) + + second := build("second") + require.NotEqual(t, first, second) + refusedDir := t.TempDir() + documents, err = CollectImageEvidence(ctx, refusedDir, subjects) + require.ErrorContains(t, err, "not the requested "+first) + require.Nil(t, documents) + entries, err := os.ReadDir(refusedDir) + require.NoError(t, err) + require.Empty(t, entries, "a changed tag must publish no evidence for the original subject") + + // Simulate the real interval between inspect and scan: the tag now holds + // the second image, but the scanner must read the first image's bytes. + result, err := scanImage(ctx, ImageRequest{Reference: name, Source: SourceDockerDaemon}, first) + require.NoError(t, err) + var versions []string + for _, component := range result.Bom.GetComponents() { + if component.GetName() == "evidence-fixture" { + versions = append(versions, component.GetVersion()) + } + } + require.Equal(t, []string{"1.0.0"}, versions) +} diff --git a/agents/services/sbom/evidence_test.go b/agents/services/sbom/evidence_test.go index 300ce7040..30cd8e4a4 100644 --- a/agents/services/sbom/evidence_test.go +++ b/agents/services/sbom/evidence_test.go @@ -106,7 +106,7 @@ func TestCollectImageEvidenceRefusesNoSubjects(t *testing.T) { func TestCollectImageEvidenceFailsWholeOnOneUnscannableSubject(t *testing.T) { dir := t.TempDir() _, err := CollectImageEvidence(context.Background(), dir, []*builderv0.ImageSubject{ - {Reference: "", Platform: "linux/arm64"}, + {Reference: "", Digest: digestARM64, Platform: "linux/arm64"}, }) require.ErrorContains(t, err, "inventory on linux/arm64") require.ErrorContains(t, err, "image SBOM requires an image reference") @@ -144,3 +144,22 @@ func TestImageSubjectsRefuseALocalImageForNoService(t *testing.T) { _, err := ImageSubjects(context.Background(), PublishedImage{Reference: "service-warehouse:local"}, true) require.ErrorContains(t, err, "no service") } + +// No scanner or registry is consulted for a subject whose identity is absent. +func TestCollectImageEvidenceRequiresAnImmutableSubject(t *testing.T) { + for name, subject := range map[string]*builderv0.ImageSubject{ + "nil": nil, + "floating registry tag": {Reference: "example.com/app:latest"}, + "local reference pin only": {Reference: pinned, Source: builderv0.ImageSourceKind_IMAGE_SOURCE_KIND_DOCKER_DAEMON}, + } { + t.Run(name, func(t *testing.T) { + dir := t.TempDir() + documents, err := CollectImageEvidence(context.Background(), dir, []*builderv0.ImageSubject{subject}) + require.ErrorContains(t, err, "sha256 digest") + require.Nil(t, documents) + entries, err := os.ReadDir(dir) + require.NoError(t, err) + require.Empty(t, entries) + }) + } +} diff --git a/agents/services/sbom/image.go b/agents/services/sbom/image.go index c422adab0..874a6c00c 100644 --- a/agents/services/sbom/image.go +++ b/agents/services/sbom/image.go @@ -68,6 +68,28 @@ type ImageResult struct { Platform string } +// ImageForSubject inventories an immutable subject and checks an explicit +// expected digest against the image actually scanned. A reference pin may name +// a multi-platform index; a Digest field always names the scanned platform (or +// a local daemon image ID), so the two pins are not interchangeable. +func ImageForSubject(ctx context.Context, subject *builderv0.ImageSubject) (*ImageResult, error) { + if err := RequirePinned(subject); err != nil { + return nil, err + } + result, err := Image(ctx, ImageRequest{ + Reference: subject.GetReference(), + Platform: subject.GetPlatform(), + Source: SourceOf(subject), + }) + if err != nil { + return nil, err + } + if want := subject.GetDigest(); want != "" && want != result.Digest { + return nil, fmt.Errorf("image %s resolved to digest %s, not the requested %s", subject.GetReference(), result.Digest, want) + } + return result, nil +} + // Image inventories the OS packages and installed application dependencies of // one image and binds the evidence to the immutable digest it was scanned // from. It never returns a result whose digest is unknown: evidence that is not @@ -268,7 +290,9 @@ func scanImage(ctx context.Context, req ImageRequest, digest string) (*Result, e if _, err := exec.LookPath("syft"); err != nil { return nil, fmt.Errorf("%w: scanning a local-only image requires an installed syft; the managed scanner has no access to the Docker daemon", ErrUnsupported) } - return runSyft(ctx, "syft", []string{"docker:" + req.Reference, "-o", "cyclonedx-json@1.5"}, "syft", req.Reference) + // A tag can move after inspection. Scan the resolved immutable ID so + // the bound digest describes the bytes syft actually inventories. + return runSyft(ctx, "syft", []string{"docker:" + digest, "-o", "cyclonedx-json@1.5"}, "syft", req.Reference) } target := "registry:" + pinnedReference(req.Reference, digest) if _, err := exec.LookPath("syft"); err != nil { diff --git a/agents/services/sbom/testdata/image-evidence/Dockerfile b/agents/services/sbom/testdata/image-evidence/Dockerfile new file mode 100644 index 000000000..520ccdf26 --- /dev/null +++ b/agents/services/sbom/testdata/image-evidence/Dockerfile @@ -0,0 +1,2 @@ +FROM scratch +COPY package.json /app/node_modules/evidence-fixture/package.json diff --git a/agents/services/sbom/testdata/image-evidence/first/package.json b/agents/services/sbom/testdata/image-evidence/first/package.json new file mode 100644 index 000000000..5ed3924d4 --- /dev/null +++ b/agents/services/sbom/testdata/image-evidence/first/package.json @@ -0,0 +1 @@ +{"name":"evidence-fixture","version":"1.0.0","license":"MIT"} diff --git a/agents/services/sbom/testdata/image-evidence/second/package.json b/agents/services/sbom/testdata/image-evidence/second/package.json new file mode 100644 index 000000000..ca935aa2b --- /dev/null +++ b/agents/services/sbom/testdata/image-evidence/second/package.json @@ -0,0 +1 @@ +{"name":"evidence-fixture","version":"2.0.0","license":"MIT"} diff --git a/docs/ci-credentials.md b/docs/ci-credentials.md index 9a358a36d..91f37d671 100644 --- a/docs/ci-credentials.md +++ b/docs/ci-credentials.md @@ -86,7 +86,7 @@ reporting "nothing found". **A job that receives a credential must be one of the shapes this repository runs, recorded exactly, or it is refused.** -There is one list, in `internal/ciguard/job_templates_test.go`: the five jobs +There is one list, in `internal/ciguard/job_templates_test.go`: the six jobs that hold a secret or a write token, each pinned by a digest over the job's canonical content and each named alongside the test that executes it. A job that is not on the list is refused. A job on the list that has changed in any @@ -200,7 +200,7 @@ indirection this rule does not follow — a tainted value can still arrive through them, and that is what the credential model is for. It is separate for a reason worth recording. The credential allowlist pins the -five jobs that hold a secret or a write token, so it covers a reusable workflow +six jobs that hold a secret or a write token, so it covers a reusable workflow holding a credential and says nothing about one holding none. Reverting `go-service-ci.yml` to `run: ${{ inputs.setup-run }}` therefore left `go test ./internal/ciguard/` green, while the identical change to diff --git a/docs/service-image-plumbing.md b/docs/service-image-plumbing.md new file mode 100644 index 000000000..64e42380a --- /dev/null +++ b/docs/service-image-plumbing.md @@ -0,0 +1,97 @@ +# Shared service caller tokens and image evidence + +Core exports the library operations shared by service gateways. Services keep +ownership of their commands, environment names, image build recipes, release +versions, and deployment templates. + +## Caller tokens + +`callertoken` is the gRPC-only implementation used by both `agents.Serve` and +the agent manager. Install `UnaryServerInterceptor(token)` and +`StreamServerInterceptor(token)` on a protected listener; clients use +`DialOption(token)` or `PerRPCCredentials(token)`. The metadata key is +`callertoken.MetadataKey` (`x-codefly-token`); `agents.AuthMetadataKey` aliases it. +The token authenticates possession of a shared secret; Work Context verification +and authorization are separate obligations. + +Only the exact `/grpc.health.v1.Health/Check` and +`/grpc.health.v1.Health/Watch` methods are exempt. `Health/List`, unknown health +methods, and application methods require the token. Empty server tokens fail +closed. `Generate` returns 32 random bytes encoded as 64 hexadecimal characters. +The credentials permit plaintext transports for local agent sockets; a network +service must provide transport protection appropriate to its deployment. + +Services resolve their own token and anonymous-listener setting, then call +`CheckListener(token, allowAnonymous, Listener{...})`. It refuses neither being +set and refuses both being set. An explicitly anonymous listener installs no +token interceptor. The library does not choose environment variable names. + +## Published image locks + +`resources.ParseImageLock(content, name)` reads the existing JSON lock format: +`{"name":"ghcr.io/codefly-dev/","digest":"sha256:<64 hex characters>"}`. +This format is the service release lock, distinct from the protobuf image +subject/evidence contracts. `ImageLock` names that existing two-field document; +it adds no independent service version or tag. + +The registry is `resources.ImageRegistry`, and the name must match the expected +service image exactly. The returned `DockerImage` uses the digest rather than +a floating tag. An empty digest returns `ErrImageUnpublished`; malformed digests +and a different repository are separate errors. Services may embed the lock and +wrap failures with their own publication instructions. + +## Image evidence + +`sbom.ImageSubjects(ctx, PublishedImage{...}, local)` derives one subject per +published platform from the pinned reference. For a locally built image it +reads the daemon's immutable image ID and records it as the subject's expected +digest. Local images need Docker and a host `syft`; registry scans can use the +managed syft image. A requested registry platform needs Docker/buildx to resolve +the platform manifest. + +`sbom.ImageForSubject` is the common scan and validation path used by the builder +and `CollectImageEvidence`. It requires a pin and refuses a result that differs +from an explicit expected digest. A reference may pin a multi-platform index; +its platform child is resolved from that index. The subject's `Digest` field, +when present, instead names the exact scanned manifest or local image ID. +Local scans use the resolved immutable ID, so moving a tag between inspection +and scanning cannot attach the old identity to new bytes. + +`CollectImageEvidence(ctx, dir, subjects)` writes CycloneDX documents named by +repository, platform, and scanned digest. Any failed subject fails the call; +callers publish artifacts only after the complete call succeeds. The function +may have written earlier subjects before a later failure, so use a fresh output +directory per collection. `WriteImageEvidenceIndex` writes `index.txt` with the +platform, repository plus scanned digest, document filename, and inventory +checksum. `ImageEvidenceDocument.SHA256` is the canonical inventory checksum, +not a checksum of the formatted CycloneDX file bytes. + +The live regression uses locally built scratch images, the real Docker daemon, +and installed syft; it never pulls a base image: + +```sh +go test -race -tags=image_evidence_required ./agents/services/sbom -timeout 3m +``` + +The tag requires those tools and fails when either is missing. The ordinary +suite exercises the file format, index, subject derivation, and input refusals. + +## Publishing before the release + +Call `.github/workflows/publish-service-image.yml` pinned to an immutable core +commit from a service's `workflow_dispatch`. Supply `image-name`, stable +`version` without `v`, and optionally `lock-file` and `platforms`; pass the +registry credential as `registry-token`. The workflow builds the caller's +Dockerfile for those platforms and pushes by digest. It returns `digest` and +uploads the lock as an artifact; the service commits that lock before tagging +its release. No tag or lock commit is created by this workflow. + +The publish job refuses a triggering commit not reachable from the caller's +own default branch before registry login or build. The credential boundary and +repository dispatch restrictions are described in +[CI credentials](ci-credentials.md) and +[branch protection](runbooks/branch-protection.md). + +`cmd/image-sbom` stays in the services: it chooses service images, platforms, +output paths, and CLI behavior using these APIs. Adoption follows the core +release by pinning it and removing service-local copies. From a9bd6d61dcf5ef27a04b4907d8652206c1b71f5e Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sat, 10 Oct 2026 12:21:47 -0400 Subject: [PATCH 6/6] fix: address shared service plumbing review findings --- .github/workflows/publish-service-image.yml | 38 ++++++--- AGENTS.md | 2 +- agents/services/sbom/evidence.go | 21 ++++- agents/services/sbom/evidence_test.go | 36 +++++++++ agents/services/sbom/image.go | 20 +++-- agents/services/sbom/image_test.go | 57 +++++++++----- callertoken/listener.go | 9 +-- callertoken/listener_test.go | 3 - docs/ci-credentials.md | 2 +- docs/service-image-plumbing.md | 20 +++-- internal/ciguard/job_templates_test.go | 2 +- internal/ciguard/service_image_test.go | 77 +++++++++++++++---- resources/image_lock.go | 16 ++-- resources/image_lock_test.go | 20 ++--- .../image-lock/trailing-document.json | 2 + .../testdata/image-lock/unknown-tag.json | 5 ++ resources/testdata/image-lock/uppercase.json | 4 + 17 files changed, 247 insertions(+), 87 deletions(-) create mode 100644 resources/testdata/image-lock/trailing-document.json create mode 100644 resources/testdata/image-lock/unknown-tag.json create mode 100644 resources/testdata/image-lock/uppercase.json diff --git a/.github/workflows/publish-service-image.yml b/.github/workflows/publish-service-image.yml index 4c2a6b5ff..aa15aff9c 100644 --- a/.github/workflows/publish-service-image.yml +++ b/.github/workflows/publish-service-image.yml @@ -14,9 +14,9 @@ name: publish-service-image # A caller is a workflow_dispatch workflow with a `version` input and one job # that calls this workflow by its job-level `uses`, pinned to an immutable core # commit SHA rather than @main. That job grants `contents: read` and -# `packages: write`, passes `image-name` (the repository's own name, say -# service-warehouse) and `version` under `with:`, and passes the registry -# credential under `secrets:` as `registry-token: ${{ secrets.GITHUB_TOKEN }}`. +# `packages: write`, passes `version` under `with:` (the image name defaults to +# the caller's repository name), and passes the registry credential under +# `secrets:` as `registry-token: ${{ secrets.GITHUB_TOKEN }}`. # # The registry credential is a secret the caller passes, not this workflow's # own token. `permissions:` here can only narrow what the caller grants, and a @@ -26,8 +26,8 @@ on: workflow_call: inputs: image-name: - description: Name of the image inside core's image registry (resources.ImageRegistry), normally the repository's own name. - required: true + description: Explicit image-name override inside core's image registry; omitted uses the caller's repository name. + default: '' type: string version: description: Upcoming stable service version, without a v prefix. Stamped on the image as org.opencontainers.image.version. @@ -66,6 +66,7 @@ jobs: publish: if: github.event_name == 'workflow_dispatch' runs-on: ubuntu-latest + timeout-minutes: 30 permissions: # The registry is reached with the secret above, never with this job's # token, so this job needs no write grant -- and a caller that grants @@ -77,30 +78,39 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: fetch-depth: 0 - # Credentials stay persisted here, deliberately. The refusal below - # fetches the default branch, and a private caller cannot fetch - # anonymously. What is persisted is this job's own read-only built-in - # token, which expires with the job -- not the registry credential, - # which is only ever passed to the login step. + # The build context may include .git even for COPY . . callers. + # Authentication for the refusal is ephemeral, never stored here. + persist-credentials: false - name: require the commit to be on the default branch, or refuse + env: + GH_TOKEN: ${{ github.token }} run: | set -euo pipefail + # Private repositories need authentication for these two remote + # reads. Command-scoped config disappears with git; neither the token + # nor a credential-bearing config file enters the Docker context. + git_remote() { + local authorization + authorization=$(printf 'x-access-token:%s' "$GH_TOKEN" | base64 | tr -d '\n') + git -c "http.https://github.com/.extraheader=AUTHORIZATION: basic ${authorization}" "$@" + } + # The authority is the remote's own HEAD, and there is no input that # can name it. A declaration the caller supplies could be omitted, # defaulted or disagreed with, and each of those is a way for the # caller to choose which history counts as reviewed. Whoever may # approve a publication is this repository's decision, so the only # source is the repository. - authority=$(git ls-remote --symref origin HEAD \ + authority=$(git_remote ls-remote --symref origin HEAD \ | awk '$1 == "ref:" && $3 == "HEAD" { sub("refs/heads/", "", $2); print $2 }') if [ -z "$authority" ]; then echo "::error::cannot read this repository's default branch from origin; refusing to publish." >&2 exit 1 fi - git fetch --quiet --no-tags --force origin \ + git_remote fetch --quiet --no-tags --force origin \ "+refs/heads/${authority}:refs/remotes/origin/${authority}" # Fully qualified: a bare `origin/` is an ambiguous rev that a tag @@ -118,11 +128,15 @@ jobs: id: inputs env: IMAGE_NAME: ${{ inputs.image-name }} + CALLER_REPOSITORY: ${{ github.repository }} VERSION: ${{ inputs.version }} LOCK_FILE: ${{ inputs.lock-file }} PLATFORMS: ${{ inputs.platforms }} run: | set -euo pipefail + if [ -z "$IMAGE_NAME" ]; then + IMAGE_NAME=$(printf '%s' "${CALLER_REPOSITORY##*/}" | tr '[:upper:]' '[:lower:]') + fi if [[ ! "$IMAGE_NAME" =~ ^[a-z0-9]+([._-][a-z0-9]+)*$ ]]; then echo "image-name must be a lowercase repository name inside the registry" >&2 exit 1 diff --git a/AGENTS.md b/AGENTS.md index d4e6fcf3a..246f2edde 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -96,6 +96,7 @@ modelled, read it there first. | --- | --- | | `resources/` | Workspace/Module/Service/Endpoint/Agent, YAML loading, validation | | `proto/` | schema source of truth (`codefly/*`, `mind/*`); `generated/` is output | +| `callertoken/` | shared gRPC caller-token authentication for agent and service listeners | | `agents/` | agent registration, process manager, the embeddable server types | | `network/` | deterministic port allocation, DNS, native/container/public modes | | `configurations/` | what services provide and consume, injected as env vars | @@ -178,7 +179,6 @@ readiness, dependency graph. Read it rather than re-deriving from the tree. nothing under review. See [`docs/ci-credentials.md`](docs/ci-credentials.md). ## Procedures - Step-by-step procedures live in `.claude/skills/`, loaded on demand rather than carried here: diff --git a/agents/services/sbom/evidence.go b/agents/services/sbom/evidence.go index 1f3d39c3f..4a4f80a37 100644 --- a/agents/services/sbom/evidence.go +++ b/agents/services/sbom/evidence.go @@ -8,6 +8,7 @@ import ( "strings" builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" + "github.com/codefly-dev/core/resources" ) // LocalImageID reports the identity the local Docker daemon holds an image @@ -27,8 +28,9 @@ func LocalImageID(ctx context.Context, reference string) (string, error) { // subject per shipped platform, derived from its digest-pinned reference // (ExpectedFromImageReference, which refuses a bare tag). A local image was // built and never pushed, so the daemon holds exactly one platform of it: the -// subject names none, which is read back from the image rather than asserted -// by the caller, and carries the image ID instead of a pin sitting in the +// subject names the platform read back from the image rather than asserted +// by the caller. A caller-supplied platform list is refused. It carries the +// image ID instead of a pin sitting in the // reference (see LocalImageID). func ImageSubjects(ctx context.Context, image PublishedImage, local bool) ([]*builderv0.ImageSubject, error) { if !local { @@ -37,13 +39,17 @@ func ImageSubjects(ctx context.Context, image PublishedImage, local bool) ([]*bu if image.Service == "" { return nil, fmt.Errorf("image %s cannot be inventoried for no service", image.Reference) } - id, err := LocalImageID(ctx, image.Reference) + if len(image.Platforms) > 0 { + return nil, fmt.Errorf("local image %s cannot declare platforms: the daemon holds one platform, read from the image", image.Reference) + } + id, platform, err := inspectDaemonImage(ctx, ImageRequest{Reference: image.Reference}) if err != nil { return nil, err } return []*builderv0.ImageSubject{{ Reference: image.Reference, Digest: id, + Platform: platform, Role: image.Role, Service: image.Service, Source: sourceKind(SourceDockerDaemon), @@ -99,6 +105,9 @@ func CollectImageEvidence(ctx context.Context, dir string, subjects []*builderv0 // writeImageEvidence encodes one scanned image as CycloneDX and writes it. func writeImageEvidence(dir string, result *ImageResult) (ImageEvidenceDocument, error) { + if !resources.IsSHA256Digest(result.Digest) { + return ImageEvidenceDocument{}, fmt.Errorf("invalid image sha256 digest %q", result.Digest) + } encoded, err := MarshalCycloneDXJSON(result.Bom) if err != nil { return ImageEvidenceDocument{}, fmt.Errorf("encode: %w", err) @@ -124,8 +133,12 @@ func writeImageEvidence(dir string, result *ImageResult) (ImageEvidenceDocument, func WriteImageEvidenceIndex(dir string, documents []ImageEvidenceDocument) error { var index strings.Builder for _, document := range documents { + platform := document.Platform + if platform == "" { + platform = "-" + } fmt.Fprintf(&index, "%s %s@%s %s %s\n", - document.Platform, referenceName(document.Reference), document.Digest, + platform, referenceName(document.Reference), document.Digest, filepath.Base(document.Path), document.SHA256) } return os.WriteFile(filepath.Join(dir, "index.txt"), []byte(index.String()), 0o644) diff --git a/agents/services/sbom/evidence_test.go b/agents/services/sbom/evidence_test.go index 30cd8e4a4..b6b17f37e 100644 --- a/agents/services/sbom/evidence_test.go +++ b/agents/services/sbom/evidence_test.go @@ -163,3 +163,39 @@ func TestCollectImageEvidenceRequiresAnImmutableSubject(t *testing.T) { }) } } + +func TestLocalImageSubjectsRefuseCallerDeclaredPlatforms(t *testing.T) { + for _, platforms := range [][]string{{"linux/amd64"}, {"linux/amd64", "linux/arm64"}} { + subjects, err := ImageSubjects(context.Background(), PublishedImage{ + Service: "fixture", Reference: "fixture:local", Platforms: platforms, + }, true) + require.ErrorContains(t, err, "daemon holds one platform") + require.Nil(t, subjects) + } +} + +func TestEvidenceWriterRefusesDigestPathTraversal(t *testing.T) { + dir := t.TempDir() + result := scanned(t, pinned, "linux/amd64", digestAMD64) + for _, digest := range []string{"sha256:/../../outside", "", "sha256:ABC", "sha256:" + strings.Repeat("A", 64)} { + result.Digest = digest + _, err := writeImageEvidence(dir, result) + require.ErrorContains(t, err, "invalid image sha256 digest") + } + entries, err := os.ReadDir(dir) + require.NoError(t, err) + require.Empty(t, entries) +} + +func TestImageEvidenceIndexKeepsFourColumnsWithoutAPlatform(t *testing.T) { + dir := t.TempDir() + document, err := writeImageEvidence(dir, scanned(t, pinned, "", digestAMD64)) + require.NoError(t, err) + require.NoError(t, WriteImageEvidenceIndex(dir, []ImageEvidenceDocument{document})) + raw, err := os.ReadFile(filepath.Join(dir, "index.txt")) + require.NoError(t, err) + fields := strings.Fields(string(raw)) + require.Len(t, fields, 4) + require.Equal(t, "-", fields[0]) + require.Equal(t, "ghcr.io/codefly-dev/service-warehouse@"+digestAMD64, fields[1]) +} diff --git a/agents/services/sbom/image.go b/agents/services/sbom/image.go index 874a6c00c..cc588c91d 100644 --- a/agents/services/sbom/image.go +++ b/agents/services/sbom/image.go @@ -12,6 +12,7 @@ import ( agentv0 "github.com/codefly-dev/core/generated/go/codefly/services/agent/v0" builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" + "github.com/codefly-dev/core/resources" ) // ImageSource selects how the scanner reaches an image. @@ -125,7 +126,7 @@ func resolveImage(ctx context.Context, req ImageRequest) (string, string, error) if pinned == "" || req.Platform != "" { return "", "", fmt.Errorf("%w: resolving %s needs docker to query the registry; pin the reference to a digest and omit the platform to scan with syft alone", ErrUnsupported, req.Reference) } - return pinned, "", nil + return manifestIdentity(pinned, "") } raw, err := runCommand(ctx, "docker", "buildx", "imagetools", "inspect", req.Reference, "--format", "{{json .Manifest}}") if err != nil { @@ -167,7 +168,7 @@ func inspectDaemonImage(ctx context.Context, req ImageRequest) (string, string, // platform per reference, so a mismatch means the wrong image would be scanned. func daemonIdentity(raw []byte, reference, want string) (string, string, error) { fields := strings.Fields(string(raw)) - if len(fields) != 2 || !strings.HasPrefix(fields[0], "sha256:") { + if len(fields) != 2 || !resources.IsSHA256Digest(fields[0]) { return "", "", fmt.Errorf("local image %s reported no usable image ID", reference) } if want != "" && fields[1] != want { @@ -256,11 +257,11 @@ func selectManifestDigest(data []byte, platform string) (string, string, error) if len(shipped) > 1 { return "", "", fmt.Errorf("image ships %d platforms; name the platform to scan", len(shipped)) } - return shipped[0].Digest, shipped[0].Platform.String(), nil + return manifestIdentity(shipped[0].Digest, shipped[0].Platform.String()) } for _, child := range shipped { if child.Platform.String() == platform { - return child.Digest, platform, nil + return manifestIdentity(child.Digest, platform) } } return "", "", fmt.Errorf("image ships no manifest for platform %s", platform) @@ -279,7 +280,16 @@ func selectManifestDigest(data []byte, platform string) (string, string, error) } // An unstated platform stays unstated. The caller confirms it against the // image config rather than having its own request echoed back as evidence. - return descriptor.Digest, resolved, nil + return manifestIdentity(descriptor.Digest, resolved) +} + +// Registry responses are external data. A digest is also used in a filename, +// so validate its OCI spelling before handing it to scanners or writers. +func manifestIdentity(digest, platform string) (string, string, error) { + if !resources.IsSHA256Digest(digest) { + return "", "", fmt.Errorf("image manifest carries an invalid sha256 digest %q", digest) + } + return digest, platform, nil } func scanImage(ctx context.Context, req ImageRequest, digest string) (*Result, error) { diff --git a/agents/services/sbom/image_test.go b/agents/services/sbom/image_test.go index 7e25eaf09..b0a613ebe 100644 --- a/agents/services/sbom/image_test.go +++ b/agents/services/sbom/image_test.go @@ -1,6 +1,7 @@ package sbom import ( + "encoding/json" "strings" "testing" @@ -46,10 +47,10 @@ func TestSelectManifestDigestParsesRealBuildxIndex(t *testing.T) { // resolver must leave it unstated rather than echo the request back, or // evidence would claim a platform nothing verified. func TestSelectManifestDigestLeavesAnUnstatedPlatformUnstated(t *testing.T) { - descriptor := `{"mediaType":"application/vnd.oci.image.manifest.v1+json","digest":"sha256:c64c","size":1023}` + descriptor := `{"mediaType":"application/vnd.oci.image.manifest.v1+json","digest":"sha256:eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee","size":1023}` digest, platform, err := selectManifestDigest([]byte(descriptor), "linux/amd64") require.NoError(t, err) - require.Equal(t, "sha256:c64c", digest) + require.Equal(t, "sha256:eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee", digest) require.Empty(t, platform) } @@ -62,48 +63,48 @@ func TestSelectManifestDigestRejectsAnAttestationOnlyIndex(t *testing.T) { func TestSelectManifestDigestPicksThePlatformChild(t *testing.T) { index := `{"digest":"sha256:index","manifests":[ - {"digest":"sha256:amd","platform":{"os":"linux","architecture":"amd64"}}, - {"digest":"sha256:arm","platform":{"os":"linux","architecture":"arm64"}}]}` + {"digest":"sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","platform":{"os":"linux","architecture":"amd64"}}, + {"digest":"sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","platform":{"os":"linux","architecture":"arm64"}}]}` digest, platform, err := selectManifestDigest([]byte(index), "linux/arm64") require.NoError(t, err) - require.Equal(t, "sha256:arm", digest) + require.Equal(t, "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", digest) require.Equal(t, "linux/arm64", platform) } func TestSelectManifestDigestRefusesAmbiguousMultiPlatformImage(t *testing.T) { index := `{"digest":"sha256:index","manifests":[ - {"digest":"sha256:amd","platform":{"os":"linux","architecture":"amd64"}}, - {"digest":"sha256:arm","platform":{"os":"linux","architecture":"arm64"}}]}` + {"digest":"sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","platform":{"os":"linux","architecture":"amd64"}}, + {"digest":"sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb","platform":{"os":"linux","architecture":"arm64"}}]}` _, _, err := selectManifestDigest([]byte(index), "") require.ErrorContains(t, err, "name the platform") } func TestSelectManifestDigestIgnoresAttestationManifests(t *testing.T) { index := `{"digest":"sha256:index","manifests":[ - {"digest":"sha256:amd","platform":{"os":"linux","architecture":"amd64"}}, + {"digest":"sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","platform":{"os":"linux","architecture":"amd64"}}, {"digest":"sha256:att","platform":{"os":"unknown","architecture":"unknown"}}]}` digest, platform, err := selectManifestDigest([]byte(index), "") require.NoError(t, err) - require.Equal(t, "sha256:amd", digest) + require.Equal(t, "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", digest) require.Equal(t, "linux/amd64", platform) } func TestSelectManifestDigestRejectsAnUnshippedPlatform(t *testing.T) { index := `{"digest":"sha256:index","manifests":[ - {"digest":"sha256:amd","platform":{"os":"linux","architecture":"amd64"}}]}` + {"digest":"sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","platform":{"os":"linux","architecture":"amd64"}}]}` _, _, err := selectManifestDigest([]byte(index), "linux/arm64") require.ErrorContains(t, err, "no manifest for platform linux/arm64") } func TestSelectManifestDigestRejectsPlatformMismatch(t *testing.T) { - _, _, err := selectManifestDigest([]byte(`{"digest":"sha256:only","platform":{"os":"linux","architecture":"amd64"}}`), "linux/arm64") + _, _, err := selectManifestDigest([]byte(`{"digest":"sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc","platform":{"os":"linux","architecture":"amd64"}}`), "linux/arm64") require.ErrorContains(t, err, "does not match requested platform") } func TestSelectManifestDigestUsesASingleManifest(t *testing.T) { - digest, platform, err := selectManifestDigest([]byte(`{"digest":"sha256:only","platform":{"os":"linux","architecture":"amd64"}}`), "") + digest, platform, err := selectManifestDigest([]byte(`{"digest":"sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc","platform":{"os":"linux","architecture":"amd64"}}`), "") require.NoError(t, err) - require.Equal(t, "sha256:only", digest) + require.Equal(t, "sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc", digest) require.Equal(t, "linux/amd64", platform) } @@ -134,17 +135,17 @@ func TestConfigPlatformDetectsAMismatchedSingleConfig(t *testing.T) { // The local daemon holds one platform per reference, so scanning a reference // whose platform differs from the request would inventory the wrong image. func TestDaemonIdentityRejectsAPlatformMismatch(t *testing.T) { - _, _, err := daemonIdentity([]byte("sha256:abc linux/amd64\n"), "app:dev", "linux/arm64") + _, _, err := daemonIdentity([]byte("sha256:dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd linux/amd64\n"), "app:dev", "linux/arm64") require.ErrorContains(t, err, "not the requested platform linux/arm64") - id, platform, err := daemonIdentity([]byte("sha256:abc linux/amd64\n"), "app:dev", "linux/amd64") + id, platform, err := daemonIdentity([]byte("sha256:dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd linux/amd64\n"), "app:dev", "linux/amd64") require.NoError(t, err) - require.Equal(t, "sha256:abc", id) + require.Equal(t, "sha256:dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd", id) require.Equal(t, "linux/amd64", platform) - id, platform, err = daemonIdentity([]byte("sha256:abc linux/amd64\n"), "app:dev", "") + id, platform, err = daemonIdentity([]byte("sha256:dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd linux/amd64\n"), "app:dev", "") require.NoError(t, err) - require.Equal(t, "sha256:abc", id) + require.Equal(t, "sha256:dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd", id) require.Equal(t, "linux/amd64", platform) } @@ -214,3 +215,23 @@ func TestSourceOfDefaultsToRegistry(t *testing.T) { require.Equal(t, builderv0.ImageSourceKind_IMAGE_SOURCE_KIND_REGISTRY, sourceKind(SourceRegistry)) require.Equal(t, builderv0.ImageSourceKind_IMAGE_SOURCE_KIND_DOCKER_DAEMON, sourceKind(SourceDockerDaemon)) } + +func TestImageIdentitiesRefuseMalformedDigests(t *testing.T) { + for _, digest := range []string{"", "sha256:abc", "sha256:/../../outside", "sha256:" + strings.Repeat("A", 64), "sha256:" + strings.Repeat("a", 63) + " "} { + t.Run(digest, func(t *testing.T) { + for _, descriptor := range []manifestDescriptor{ + {Digest: digest}, + {Manifests: []manifestChildRef{{Digest: digest, Platform: &manifestPlatform{OS: "linux", Architecture: "amd64"}}}}, + } { + raw, err := json.Marshal(descriptor) + require.NoError(t, err) + got, _, err := selectManifestDigest(raw, "linux/amd64") + require.Error(t, err) + require.Empty(t, got) + } + got, _, err := daemonIdentity([]byte(digest+" linux/amd64"), "fixture", "") + require.Error(t, err) + require.Empty(t, got) + }) + } +} diff --git a/callertoken/listener.go b/callertoken/listener.go index 604cad29d..31a1e28eb 100644 --- a/callertoken/listener.go +++ b/callertoken/listener.go @@ -36,14 +36,11 @@ type Listener struct { func CheckListener(token string, allowAnonymous bool, l Listener) error { switch { case token != "" && allowAnonymous: - return fmt.Errorf("%s and %s=true are mutually exclusive: "+ - "unset %[2]s to enforce the token, or unset %[1]s to accept anonymous callers", + return fmt.Errorf("%s and %s=true are mutually exclusive", l.TokenVar, l.AnonymousVar) case token != "", allowAnonymous: return nil } - return fmt.Errorf("%[1]s is required: without it every caller that can reach the listener %[2]s. "+ - "Set %[1]s to a shared secret and have clients send it as the %[3]q gRPC metadata header, "+ - "or set %[4]s=true if the listener is confined to a private boundary enforced elsewhere (cluster NetworkPolicy / service-mesh mTLS)", - l.TokenVar, l.Exposure, MetadataKey, l.AnonymousVar) + return fmt.Errorf("%s is required unless %s=true: without a token every caller that can reach the listener %s", + l.TokenVar, l.AnonymousVar, l.Exposure) } diff --git a/callertoken/listener_test.go b/callertoken/listener_test.go index 3d7df97e5..9b6daae8d 100644 --- a/callertoken/listener_test.go +++ b/callertoken/listener_test.go @@ -24,13 +24,10 @@ func TestCheckListener(t *testing.T) { {"anonymous opt-out alone", "", true, nil}, {"both is a contradiction", "s3cret", true, []string{ "SWH_AUTH_TOKEN and SWH_ALLOW_ANONYMOUS=true are mutually exclusive", - "unset SWH_ALLOW_ANONYMOUS to enforce the token", - "unset SWH_AUTH_TOKEN to accept anonymous callers", }}, {"neither", "", false, []string{ "SWH_AUTH_TOKEN is required", "can run SQL against the bound database", - `"x-codefly-token"`, "SWH_ALLOW_ANONYMOUS=true", }}, } { diff --git a/docs/ci-credentials.md b/docs/ci-credentials.md index 91f37d671..4e4497fba 100644 --- a/docs/ci-credentials.md +++ b/docs/ci-credentials.md @@ -109,7 +109,7 @@ known shape has no such set. | `go.yml` | `notify` holds `SLACK_WEBHOOK_URL` | Its own job, gated to a push of `main`, with `permissions: {}` and no checkout at all. It reads the outcome from `needs.build.result`. | | `version-tag.yml` | `tag` creates the release tag and publishes the release | Event and head-repository conditions; checks out the default branch; refuses a sha not reachable from it. | | `go-service-release.yml` | `goreleaser` receives the **caller's** release PAT | Gated to a pushed tag, which a pull request cannot produce; then refuses a tag whose commit is not reachable from the caller's default branch. The `test` job that runs the caller's suite names no secret. | -| `publish-service-image.yml` | `publish` receives the **caller's** registry credential, passed as the secret `registry-token` | Gated to a `workflow_dispatch`, which a pull request cannot produce; then refuses a commit that is not reachable from the caller's default branch, as the first step after the checkout and before the login that receives the credential or the build that runs the caller's Dockerfile. A reusable workflow declares no write, so the caller's own job is where `packages: write` is granted and its token travels as a value. | +| `publish-service-image.yml` | `publish` receives the **caller's** registry credential, passed as the secret `registry-token` | Gated to a `workflow_dispatch`, which a pull request cannot produce; then refuses a commit that is not reachable from the caller's default branch, as the first step after the checkout and before the login that receives the credential or the build that runs the caller's Dockerfile. Checkout never persists its read token; only the refusal receives it, through command-scoped git authentication for private repositories. The job times out after 30 minutes. A reusable workflow declares no write, so the caller's own job is where `packages: write` is granted and its token travels as a value. | | `combine-deps.yml` | `publish` holds `DEPS_COMBINE_TOKEN` | A job of its own that never checks the combined tree out: the commits arrive as a git bundle and move into a ref by `git fetch`/`git push`. The `plan` job does the replaying, with the read-only built-in token and no secret. Both checkouts pin `ref: main`. | Every other job in every other workflow declares `contents: read` and names no diff --git a/docs/service-image-plumbing.md b/docs/service-image-plumbing.md index 64e42380a..2d4274405 100644 --- a/docs/service-image-plumbing.md +++ b/docs/service-image-plumbing.md @@ -36,7 +36,8 @@ it adds no independent service version or tag. The registry is `resources.ImageRegistry`, and the name must match the expected service image exactly. The returned `DockerImage` uses the digest rather than -a floating tag. An empty digest returns `ErrImageUnpublished`; malformed digests +a floating tag. The digest requires 64 lowercase hex characters, and unknown JSON fields or a +second document are refused. An empty digest returns `ErrImageUnpublished`; malformed digests and a different repository are separate errors. Services may embed the lock and wrap failures with their own publication instructions. @@ -45,7 +46,8 @@ wrap failures with their own publication instructions. `sbom.ImageSubjects(ctx, PublishedImage{...}, local)` derives one subject per published platform from the pinned reference. For a locally built image it reads the daemon's immutable image ID and records it as the subject's expected -digest. Local images need Docker and a host `syft`; registry scans can use the +digest and platform. A local request must omit `Platforms`: the daemon holds +one platform, and a caller list is refused rather than silently dropped. Local images need Docker and a host `syft`; registry scans can use the managed syft image. A requested registry platform needs Docker/buildx to resolve the platform manifest. @@ -63,7 +65,8 @@ callers publish artifacts only after the complete call succeeds. The function may have written earlier subjects before a later failure, so use a fresh output directory per collection. `WriteImageEvidenceIndex` writes `index.txt` with the platform, repository plus scanned digest, document filename, and inventory -checksum. `ImageEvidenceDocument.SHA256` is the canonical inventory checksum, +checksum. An unstated platform occupies its column as `-`. +`ImageEvidenceDocument.SHA256` is the canonical inventory checksum, not a checksum of the formatted CycloneDX file bytes. The live regression uses locally built scratch images, the real Docker daemon, @@ -79,15 +82,20 @@ suite exercises the file format, index, subject derivation, and input refusals. ## Publishing before the release Call `.github/workflows/publish-service-image.yml` pinned to an immutable core -commit from a service's `workflow_dispatch`. Supply `image-name`, stable -`version` without `v`, and optionally `lock-file` and `platforms`; pass the +commit from a service's `workflow_dispatch`. Supply a stable `version` without +`v`, and optionally `lock-file` and `platforms`. The image name defaults to the +caller repository name in lowercase; `image-name` is an explicit override for +a service that intentionally publishes another package. Pass the registry credential as `registry-token`. The workflow builds the caller's Dockerfile for those platforms and pushes by digest. It returns `digest` and uploads the lock as an artifact; the service commits that lock before tagging its release. No tag or lock commit is created by this workflow. The publish job refuses a triggering commit not reachable from the caller's -own default branch before registry login or build. The credential boundary and +own default branch before registry login or build. Checkout does not persist +credentials, and the refusal authenticates its remote reads with command-scoped +git configuration. No read token is written into the caller build context, even +when its Dockerfile copies `.git`. The job has a 30-minute timeout. The credential boundary and repository dispatch restrictions are described in [CI credentials](ci-credentials.md) and [branch protection](runbooks/branch-protection.md). diff --git a/internal/ciguard/job_templates_test.go b/internal/ciguard/job_templates_test.go index 46faf0460..a0a1319fa 100644 --- a/internal/ciguard/job_templates_test.go +++ b/internal/ciguard/job_templates_test.go @@ -186,7 +186,7 @@ var recordedDigests = map[string]string{ "combine-deps.yml": "2ddc71ebfde5eff4d8646d53dca254a9b631a4a951589d5ff587962c09e43e50", "go-service-release.yml": "b96f9dcb767f9a307b4120797007a30bad8b76bb8bd7b461247a555102a54708", "go.yml": "e484c97790a696fcd2a55e323df7d4b451b7657a917a126d7c6831e02cd2a904", - "publish-service-image.yml": "b3424deb195f36102aeb6064f28ff8a4be2e27ae19f05785aaf910081180b282", + "publish-service-image.yml": "10afa917aa051d051756b170dac9d88c37e2bd35bffff03fe4d46c2b10401462", "version-tag.yml": "25972c82482fb3ae6196371328df126843c3e8a1b4aae7b863693bbad5fac641", } diff --git a/internal/ciguard/service_image_test.go b/internal/ciguard/service_image_test.go index 6e0270a97..5f5f38729 100644 --- a/internal/ciguard/service_image_test.go +++ b/internal/ciguard/service_image_test.go @@ -1,6 +1,7 @@ package ciguard import ( + "encoding/base64" "os" "os/exec" "path/filepath" @@ -55,6 +56,7 @@ func publishStep(t *testing.T, name string) isolatedStep { // default branch: a dispatch can name any branch, and the Dockerfile it builds // runs with the registry credential. func TestAServiceImageIsPublishedOnlyFromTheRepositorysOwnDefaultBranch(t *testing.T) { + t.Setenv("GH_TOKEN", "fixture-read-token") script := publishStep(t, publishRefusalStep).Run for _, tc := range []struct { @@ -88,6 +90,7 @@ func TestAServiceImageIsPublishedOnlyFromTheRepositorysOwnDefaultBranch(t *testi // The refusal has to be the thing that decides: neutralising it must be // visible, which makes the test above one of behaviour rather than of text. func TestNeutralisingThePublishRefusalIsVisible(t *testing.T) { + t.Setenv("GH_TOKEN", "fixture-read-token") script := publishStep(t, publishRefusalStep).Run fixture := newReleaseFixture(t) @@ -100,7 +103,7 @@ func TestNeutralisingThePublishRefusalIsVisible(t *testing.T) { "rejected, so something other than the refusal decided it") } -// The refusal takes nothing from the caller, so the authority it checks against +// The refusal takes no caller input or secret, so the authority it checks against // has one source, and it runs before anything the caller's tree can influence: // the first step after the checkout, ahead of the validation, the login that // receives the credential and the build that runs the Dockerfile. @@ -110,7 +113,9 @@ func TestThePublishRefusalTakesNoInputAndRunsFirst(t *testing.T) { require.True(t, strings.HasPrefix(steps[0].Uses, "actions/checkout@"), "the first step is the checkout") require.Equal(t, publishRefusalStep, steps[1].Name, "the refusal must be the first thing that runs after the checkout") - require.Empty(t, steps[1].Env, "the refusal receives %v; it must take nothing from the caller", steps[1].Env) + require.Equal(t, map[string]string{"GH_TOKEN": "${{ github.token }}"}, steps[1].Env, + "only the job read token reaches the refusal, never caller inputs or secrets") + require.Equal(t, false, steps[0].With["persist-credentials"], "the caller build context must contain no persisted checkout credential") require.Empty(t, steps[1].If, "a condition on the refusal is a way to skip it") for _, step := range steps[2:] { @@ -126,9 +131,10 @@ func TestThePublishRefusalTakesNoInputAndRunsFirst(t *testing.T) { func TestThePublishJobCannotRunOnAPullRequestOrInAMergeQueue(t *testing.T) { _, workflows := loadIsolatedWorkflows(t) wf := workflows[filepath.Join(repoRoot(t), ".github", "workflows", publishWorkflow)] - require.True(t, cannotRunOnAPullRequest(t, wf.Jobs[publishJob].If), - "the publish job holds a registry credential and its condition %q is not provably false on a pull request", - wf.Jobs[publishJob].If) + for _, trigger := range []string{"pull_request", "pull_request_target", "merge_group"} { + unreachable, reason := provablyUnreachable(t, wf.Jobs[publishJob].If, scenariosForTrigger(trigger)) + require.True(t, unreachable, "%s can reach the publish job: %s", trigger, reason) + } } // The workflow cannot import resources.ImageRegistry, so a test holds the two @@ -160,11 +166,12 @@ func TestThePublishWorkflowValidatesItsInputsBeforeTheCredentialIsUsed(t *testin step := publishStep(t, publishInputsStep) valid := map[string]string{ - "IMAGE_NAME": "service-warehouse", - "VERSION": "1.2.3", - "LOCK_FILE": "gateway-image.json", - "PLATFORMS": "linux/amd64,linux/arm64", - "REGISTRY": resources.ImageRegistry, + "IMAGE_NAME": "", + "CALLER_REPOSITORY": "codefly-dev/service-warehouse", + "VERSION": "1.2.3", + "LOCK_FILE": "gateway-image.json", + "PLATFORMS": "linux/amd64,linux/arm64", + "REGISTRY": resources.ImageRegistry, } run := func(t *testing.T, overrides map[string]string) (bool, string, string) { t.Helper() @@ -191,7 +198,7 @@ func TestThePublishWorkflowValidatesItsInputsBeforeTheCredentialIsUsed(t *testin for name, overrides := range map[string]map[string]string{ "an image name with a registry": {"IMAGE_NAME": "ghcr.io/other/app"}, "an image name with upper case": {"IMAGE_NAME": "Service"}, - "an empty image name": {"IMAGE_NAME": ""}, + "an empty derived image name": {"CALLER_REPOSITORY": ""}, "an image name injecting an output": {"IMAGE_NAME": "app\nversion=9.9.9"}, "a v prefixed version": {"VERSION": "v1.2.3"}, "a two part version": {"VERSION": "1.2"}, @@ -209,14 +216,52 @@ func TestThePublishWorkflowValidatesItsInputsBeforeTheCredentialIsUsed(t *testin }) } for name, overrides := range map[string]map[string]string{ - "another lock name": {"LOCK_FILE": "minio-image.json"}, - "one platform": {"PLATFORMS": "linux/arm64"}, - "a zero version": {"VERSION": "0.0.0"}, - "a dotted image name": {"IMAGE_NAME": "service.warehouse"}, + "explicit image override": {"IMAGE_NAME": "another-service"}, + "mixed case repository": {"CALLER_REPOSITORY": "codefly-dev/Service-Warehouse"}, + "another lock name": {"LOCK_FILE": "minio-image.json"}, + "one platform": {"PLATFORMS": "linux/arm64"}, + "a zero version": {"VERSION": "0.0.0"}, + "a dotted image name": {"IMAGE_NAME": "service.warehouse"}, } { t.Run("accepts "+name, func(t *testing.T) { - ok, output, _ := run(t, overrides) + ok, output, written := run(t, overrides) require.True(t, ok, output) + name := overrides["IMAGE_NAME"] + if name == "" { + name = "service-warehouse" + } + require.Contains(t, written, "image="+resources.ImageRegistry+"/"+name+"\n") }) } } + +// Execute the authenticated refusal against a real repository. A caller may +// COPY . . without a .dockerignore: no token may remain in its build context. +func TestThePublishRefusalLeavesNoCredentialInTheBuildContext(t *testing.T) { + const token = "fixture-read-token" + t.Setenv("GH_TOKEN", token) + steps := publishSteps(t) + require.Equal(t, false, steps[0].With["persist-credentials"]) + fixture := newReleaseFixture(t) + admitted, output := fixture.run(t, publishStep(t, publishRefusalStep).Run, theDefaultBranch, fixture.tip) + require.True(t, admitted, output) + encoded := base64.StdEncoding.EncodeToString([]byte("x-access-token:" + token)) + require.NotContains(t, output, token) + require.NotContains(t, output, encoded) + err := filepath.WalkDir(fixture.dir, func(path string, entry os.DirEntry, err error) error { + if err != nil || entry.IsDir() { + return err + } + data, err := os.ReadFile(path) + if err != nil { + return err + } + require.NotContains(t, string(data), token, path) + require.NotContains(t, string(data), encoded, path) + return nil + }) + require.NoError(t, err) + for _, step := range steps[2:] { + require.NotContains(t, step.Env, "GH_TOKEN", "only the refusal receives the read token") + } +} diff --git a/resources/image_lock.go b/resources/image_lock.go index 1c954d83c..b448ae25f 100644 --- a/resources/image_lock.go +++ b/resources/image_lock.go @@ -1,10 +1,11 @@ package resources import ( - "encoding/hex" + "bytes" "encoding/json" "errors" "fmt" + "io" "strings" ) @@ -39,9 +40,14 @@ type ImageLock struct { // is a different error, so a caller can tell "not yet published" from "wrong". func ParseImageLock(content []byte, name string) (*DockerImage, error) { var lock ImageLock - if err := json.Unmarshal(content, &lock); err != nil { + decoder := json.NewDecoder(bytes.NewReader(content)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(&lock); err != nil { return nil, fmt.Errorf("parse image lock: %w", err) } + if err := decoder.Decode(new(any)); err != io.EOF { + return nil, fmt.Errorf("parse image lock: expected one JSON document") + } image := PublishedImage(name, "") if want := image.Repository + "/" + image.Name; lock.Name != want { return nil, fmt.Errorf("image lock names %q, but the image is %q", lock.Name, want) @@ -57,9 +63,9 @@ func ParseImageLock(content []byte, name string) (*DockerImage, error) { } // IsSHA256Digest reports whether digest is a complete sha256 image digest: -// "sha256:" followed by exactly 64 hexadecimal characters. +// "sha256:" followed by exactly 64 lowercase hexadecimal characters (OCI). func IsSHA256Digest(digest string) bool { algorithm, encoded, found := strings.Cut(digest, ":") - decoded, err := hex.DecodeString(encoded) - return found && algorithm == "sha256" && err == nil && len(decoded) == 32 + return found && algorithm == "sha256" && len(encoded) == 64 && + strings.Trim(encoded, "0123456789abcdef") == "" } diff --git a/resources/image_lock_test.go b/resources/image_lock_test.go index ad1d3df9d..78942696b 100644 --- a/resources/image_lock_test.go +++ b/resources/image_lock_test.go @@ -3,6 +3,7 @@ package resources_test import ( "os" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/require" @@ -36,7 +37,7 @@ func TestParseImageLockRefusesAnythingButASHA256DigestOfThisImage(t *testing.T) for _, fixture := range []string{ "no-name", "other-image", "other-registry", "tag-as-digest", "other-algorithm", "truncated", "not-hex", "too-long", - "not-json", + "not-json", "uppercase", "unknown-tag", "trailing-document", } { t.Run(fixture, func(t *testing.T) { image, err := resources.ParseImageLock(lock(t, fixture), lockedImageName) @@ -59,14 +60,15 @@ func TestAnUnpublishedLockIsRefusedNeverRunAsLatest(t *testing.T) { func TestIsSHA256Digest(t *testing.T) { const hex64 = "1205da0743e56b68c12c754438476e8fdd0f05cdb08ce1c9687ddb8d8f0e1669" for digest, want := range map[string]bool{ - "sha256:" + hex64: true, - "sha256:" + hex64 + "00": false, - "sha256:" + hex64[:62]: false, - "sha512:" + hex64: false, - "SHA256:" + hex64: false, - "sha256:": false, - hex64: false, - "": false, + "sha256:" + hex64: true, + "sha256:" + strings.ToUpper(hex64): false, + "sha256:" + hex64 + "00": false, + "sha256:" + hex64[:62]: false, + "sha512:" + hex64: false, + "SHA256:" + hex64: false, + "sha256:": false, + hex64: false, + "": false, } { require.Equal(t, want, resources.IsSHA256Digest(digest), "%q", digest) } diff --git a/resources/testdata/image-lock/trailing-document.json b/resources/testdata/image-lock/trailing-document.json new file mode 100644 index 000000000..3f238c4a5 --- /dev/null +++ b/resources/testdata/image-lock/trailing-document.json @@ -0,0 +1,2 @@ +{"name": "ghcr.io/codefly-dev/service-warehouse", "digest": "sha256:1205da0743e56b68c12c754438476e8fdd0f05cdb08ce1c9687ddb8d8f0e1669"} +{} diff --git a/resources/testdata/image-lock/unknown-tag.json b/resources/testdata/image-lock/unknown-tag.json new file mode 100644 index 000000000..9074bde20 --- /dev/null +++ b/resources/testdata/image-lock/unknown-tag.json @@ -0,0 +1,5 @@ +{ + "name": "ghcr.io/codefly-dev/service-warehouse", + "digest": "sha256:1205da0743e56b68c12c754438476e8fdd0f05cdb08ce1c9687ddb8d8f0e1669", + "tag": "9.9.9" +} diff --git a/resources/testdata/image-lock/uppercase.json b/resources/testdata/image-lock/uppercase.json new file mode 100644 index 000000000..bd1cdfe6b --- /dev/null +++ b/resources/testdata/image-lock/uppercase.json @@ -0,0 +1,4 @@ +{ + "name": "ghcr.io/codefly-dev/service-warehouse", + "digest": "sha256:1205DA0743E56B68C12C754438476E8FDD0F05CDB08CE1C9687DDB8D8F0E1669" +}