From dac4ea43f366561df1931ef4a4b4cd7fd27f80c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Thu, 16 Jul 2026 17:37:41 +0200 Subject: [PATCH] core/runtime/v2: Preserve protobuf shim response bytes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Shim start output was trimmed before protobuf decoding. Because arbitrary protobuf fields may legitimately end with whitespace bytes, including `\n`, trimming could corrupt metadata or capabilities and cause decoding to fail. Pass the raw command output to `parseStartResponse` and attempt protobuf decoding before modifying the response. Whitespace trimming now applies only to legacy JSON and plain-address responses. Signed-off-by: PaweÅ‚ Gronowski --- core/runtime/v2/binary.go | 4 +--- core/runtime/v2/shim.go | 2 ++ core/runtime/v2/shim_test.go | 37 ++++++++++++++++++++++++++++-------- 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/core/runtime/v2/binary.go b/core/runtime/v2/binary.go index a0c8848577c7d..253b7deaedad5 100644 --- a/core/runtime/v2/binary.go +++ b/core/runtime/v2/binary.go @@ -118,8 +118,6 @@ func (b *binary) Start(ctx context.Context, opts *types.Any, onClose func()) (_ if err != nil { return nil, fmt.Errorf("%s: %w", out, err) } - response := bytes.TrimSpace(out) - onCloseWithShimLog := func() { onClose() cancelShimLog() @@ -130,7 +128,7 @@ func (b *binary) Start(ctx context.Context, opts *types.Any, onClose func()) (_ return nil, err } - params, err := parseStartResponse(response) + params, err := parseStartResponse(out) if err != nil { return nil, err } diff --git a/core/runtime/v2/shim.go b/core/runtime/v2/shim.go index 8d617130d258e..ba823b65b256e 100644 --- a/core/runtime/v2/shim.go +++ b/core/runtime/v2/shim.go @@ -17,6 +17,7 @@ package v2 import ( + "bytes" "context" "encoding/json" "errors" @@ -229,6 +230,7 @@ func parseStartResponse(response []byte) (*bootapi.BootstrapResult, error) { } // Fallback to legacy parsing for backward compatibility with legacy shims that return the address as a plain string or JSON. + response = bytes.TrimSpace(response) var params client.BootstrapParams //nolint:staticcheck // Used for backward compatibility with legacy shims if err := json.Unmarshal(response, ¶ms); err != nil || params.Version < 2 { diff --git a/core/runtime/v2/shim_test.go b/core/runtime/v2/shim_test.go index 3ada09157463f..7fbe324bb5e1e 100644 --- a/core/runtime/v2/shim_test.go +++ b/core/runtime/v2/shim_test.go @@ -23,29 +23,49 @@ import ( "testing" bootapi "github.com/containerd/containerd/api/runtime/bootstrap/v1" + "github.com/containerd/containerd/v2/pkg/protobuf/proto" "github.com/containerd/errdefs" "github.com/stretchr/testify/require" ) func TestParseStartResponse(t *testing.T) { + protobufResponse, err := proto.Marshal(&bootapi.BootstrapResult{ + Version: 3, + Address: "unix:///run/containerd/shim.sock", + Protocol: "ttrpc", + Metadata: map[string]string{"note": "line\n"}, + }) + require.NoError(t, err) + require.Equal(t, byte('\n'), protobufResponse[len(protobufResponse)-1]) + testCases := []struct { Name string - Response string + Response []byte Expected bootapi.BootstrapResult Err error }{ { - Name: "v2 shim", - Response: "/somedirectory/somesocket", + Name: "v2 shim with trailing newline", + Response: []byte("/somedirectory/somesocket\n"), Expected: bootapi.BootstrapResult{ Version: 2, Address: "/somedirectory/somesocket", Protocol: "ttrpc", }, }, + { + Name: "v3 shim protobuf", + Response: protobufResponse, + Expected: bootapi.BootstrapResult{ + Version: 3, + Address: "unix:///run/containerd/shim.sock", + Protocol: "ttrpc", + Metadata: map[string]string{"note": "line\n"}, + }, + }, { Name: "v2 shim using grpc", - Response: `{"version":2,"address":"/somedirectory/somesocket","protocol":"grpc"}`, + Response: []byte(`{"version":2,"address":"/somedirectory/somesocket","protocol":"grpc"}`), Expected: bootapi.BootstrapResult{ Version: 2, Address: "/somedirectory/somesocket", @@ -54,7 +74,7 @@ func TestParseStartResponse(t *testing.T) { }, { Name: "v2 shim using ttrpc", - Response: `{"version":2,"address":"/somedirectory/somesocket","protocol":"ttrpc"}`, + Response: []byte(`{"version":2,"address":"/somedirectory/somesocket","protocol":"ttrpc"}`), Expected: bootapi.BootstrapResult{ Version: 2, Address: "/somedirectory/somesocket", @@ -63,7 +83,7 @@ func TestParseStartResponse(t *testing.T) { }, { Name: "invalid shim v2 response", - Response: `{"address":"/somedirectory/somesocket","protocol":"ttrpc"}`, + Response: []byte(`{"address":"/somedirectory/somesocket","protocol":"ttrpc"}`), Expected: bootapi.BootstrapResult{ Version: 2, Address: `{"address":"/somedirectory/somesocket","protocol":"ttrpc"}`, @@ -72,7 +92,7 @@ func TestParseStartResponse(t *testing.T) { }, { Name: "later unsupported shim", - Response: `{"Version": 4,"Address":"/somedirectory/somesocket","Protocol":"ttrpc"}`, + Response: []byte(`{"Version": 4,"Address":"/somedirectory/somesocket","Protocol":"ttrpc"}`), Expected: bootapi.BootstrapResult{}, Err: errdefs.ErrNotImplemented, }, @@ -81,7 +101,7 @@ func TestParseStartResponse(t *testing.T) { for i := range testCases { tc := &testCases[i] t.Run(tc.Name, func(t *testing.T) { - params, err := parseStartResponse([]byte(tc.Response)) + params, err := parseStartResponse(tc.Response) if err != nil { if !errors.Is(err, tc.Err) { t.Errorf("unexpected error: %v", err) @@ -99,6 +119,7 @@ func TestParseStartResponse(t *testing.T) { if params.Address != tc.Expected.Address { t.Errorf("unexpected address %q, expected %q", params.Address, tc.Expected.Address) } + require.Equal(t, tc.Expected.Metadata, params.Metadata) }) } }