fix: replace invalid utf-8 sequences in agent logs (#13436)

kylecarbs · spikecurtis · web-flow · commit 7c081dcd6feb · 2024-06-10T15:27:11.000+04:00
* fix: replace invalid utf-8 sequences in agent logs Fixes #13433. * fix: replace invalid UTF-8 with ❌, add regression Signed-off-by: Spike Curtis <spike@coder.com> --------- Signed-off-by: Spike Curtis <spike@coder.com> Co-authored-by: Spike Curtis <spike@coder.com>
diff --git a/codersdk/agentsdk/convert.go b/codersdk/agentsdk/convert.go
@@ -348,7 +348,7 @@ func ProtoFromLog(log Log) (*proto.Log, error) {
 	}
 	return &proto.Log{
 		CreatedAt: timestamppb.New(log.CreatedAt),
-		Output:    log.Output,
+		Output:    strings.ToValidUTF8(log.Output, "❌"),
 		Level:     proto.Log_Level(lvl),
 	}, nil
 }
diff --git a/codersdk/agentsdk/logs_internal_test.go b/codersdk/agentsdk/logs_internal_test.go
@@ -231,6 +231,51 @@ func TestLogSender_SkipHugeLog(t *testing.T) {
 	require.ErrorIs(t, err, context.Canceled)
 }
 
+func TestLogSender_InvalidUTF8(t *testing.T) {
+	t.Parallel()
+	testCtx := testutil.Context(t, testutil.WaitShort)
+	ctx, cancel := context.WithCancel(testCtx)
+	logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
+	fDest := newFakeLogDest()
+	uut := NewLogSender(logger)
+
+	t0 := dbtime.Now()
+	ls1 := uuid.UUID{0x11}
+
+	uut.Enqueue(ls1,
+		Log{
+			CreatedAt: t0,
+			Output:    "test log 0, src 1\xc3\x28",
+			Level:     codersdk.LogLevelInfo,
+		},
+		Log{
+			CreatedAt: t0,
+			Output:    "test log 1, src 1",
+			Level:     codersdk.LogLevelInfo,
+		})
+
+	loopErr := make(chan error, 1)
+	go func() {
+		err := uut.SendLoop(ctx, fDest)
+		loopErr <- err
+	}()
+
+	req := testutil.RequireRecvCtx(ctx, t, fDest.reqs)
+	require.NotNil(t, req)
+	require.Len(t, req.Logs, 2, "it should sanitize invalid UTF-8, but still send")
+	// the 0xc3, 0x28 is an invalid 2-byte sequence in UTF-8.  The sanitizer replaces 0xc3 with ❌, and then
+	// interprets 0x28 as a 1-byte sequence "("
+	require.Equal(t, "test log 0, src 1❌(", req.Logs[0].GetOutput())
+	require.Equal(t, proto.Log_INFO, req.Logs[0].GetLevel())
+	require.Equal(t, "test log 1, src 1", req.Logs[1].GetOutput())
+	require.Equal(t, proto.Log_INFO, req.Logs[1].GetLevel())
+	testutil.RequireSendCtx(ctx, t, fDest.resps, &proto.BatchCreateLogsResponse{})
+
+	cancel()
+	err := testutil.RequireRecvCtx(testCtx, t, loopErr)
+	require.ErrorIs(t, err, context.Canceled)
+}
+
 func TestLogSender_Batch(t *testing.T) {
 	t.Parallel()
 	testCtx := testutil.Context(t, testutil.WaitShort)

Original file line number	Diff line number	Diff line change
`@@ -348,7 +348,7 @@ func ProtoFromLog(log Log) (*proto.Log, error) {`
`348`	`348`	`}`
`349`	`349`	`return &proto.Log{`
`350`	`350`	`CreatedAt: timestamppb.New(log.CreatedAt),`
`351`		`- Output: log.Output,`
	`351`	`+ Output: strings.ToValidUTF8(log.Output, "❌"),`
`352`	`352`	`Level: proto.Log_Level(lvl),`
`353`	`353`	`}, nil`
`354`	`354`	`}`