Skip to content

Commit 4fb0c38

Browse files
geroplgeropl
authored
Cap length of GitStatus updates (#19810)
* [server] ValidateGitStatusLength (max: 4096) Behind feature flag api_validate_git_status_length * [supervisor] Cap length of GitStatus message * [supervisor] cap gitstatus test
1 parent 1628e75 commit 4fb0c38

File tree

3 files changed

+224
-2
lines changed

3 files changed

+224
-2
lines changed

‎components/server/src/workspace/workspace-service.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -717,6 +717,19 @@ export class WorkspaceService {
717717
) {
718718
await this.auth.checkPermissionOnWorkspace(userId, "access", workspaceId);
719719

720+
if (!!gitStatus) {
721+
const validateGitStatusLength = await getExperimentsClientForBackend().getValueAsync(
722+
"api_validate_git_status_length",
723+
false,
724+
{
725+
user: { id: userId || "" },
726+
},
727+
);
728+
if (validateGitStatusLength) {
729+
this.validateGitStatusLength(gitStatus);
730+
}
731+
}
732+
720733
let instance = await this.getCurrentInstance(userId, workspaceId);
721734
if (WorkspaceInstanceRepoStatus.equals(instance.gitStatus, gitStatus)) {
722735
return;
@@ -731,6 +744,23 @@ export class WorkspaceService {
731744
});
732745
}
733746

747+
protected validateGitStatusLength(gitStatus: Required<WorkspaceInstanceRepoStatus>) {
748+
/** [bytes] */
749+
const MAX_GIT_STATUS_LENGTH = 4096;
750+
751+
try {
752+
const s = JSON.stringify(gitStatus);
753+
if (Buffer.byteLength(s, "utf8") > MAX_GIT_STATUS_LENGTH) {
754+
throw new ApplicationError(
755+
ErrorCodes.BAD_REQUEST,
756+
`gitStatus too long, maximum is ${MAX_GIT_STATUS_LENGTH} bytes`,
757+
);
758+
}
759+
} catch (err) {
760+
throw new ApplicationError(ErrorCodes.BAD_REQUEST, "Invalid gitStatus: " + err.message);
761+
}
762+
}
763+
734764
public async getSupportedWorkspaceClasses(user: { id: string }): Promise<SupportedWorkspaceClass[]> {
735765
return this.installationService.getInstallationWorkspaceClasses(user.id);
736766
}

‎components/supervisor/pkg/serverapi/publicapi.go

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package serverapi
77
import (
88
"context"
99
"crypto/tls"
10+
"encoding/json"
1011
"errors"
1112
"fmt"
1213
"io"
@@ -246,7 +247,7 @@ func (s *Service) UpdateGitStatus(ctx context.Context, status *gitpod.WorkspaceI
246247
WorkspaceId: workspaceID,
247248
}
248249
if status != nil {
249-
payload.Status = &v1.GitStatus{
250+
payload.Status = capGitStatusLength(&v1.GitStatus{
250251
Branch: status.Branch,
251252
LatestCommit: status.LatestCommit,
252253
TotalUncommitedFiles: int32(status.TotalUncommitedFiles),
@@ -255,7 +256,7 @@ func (s *Service) UpdateGitStatus(ctx context.Context, status *gitpod.WorkspaceI
255256
UncommitedFiles: status.UncommitedFiles,
256257
UnpushedCommits: status.UnpushedCommits,
257258
UntrackedFiles: status.UntrackedFiles,
258-
}
259+
})
259260
}
260261
_, err = service.UpdateGitStatus(ctx, payload)
261262
return
@@ -515,3 +516,47 @@ func workspaceStatusToWorkspaceInstance(status *v1.WorkspaceStatus) *gitpod.Work
515516
}
516517
return instance
517518
}
519+
520+
const GIT_STATUS_API_LIMIT_BYTES = 4096
521+
522+
func capGitStatusLength(s *v1.GitStatus) *v1.GitStatus {
523+
const MARGIN = 200 // bytes (we account for differences in JSON formatting, as well JSON escape characters in the static part of the status)
524+
const API_BUDGET = GIT_STATUS_API_LIMIT_BYTES - MARGIN // bytes
525+
526+
// calculate JSON length in bytes
527+
bytes, err := json.Marshal(s)
528+
if err != nil {
529+
log.WithError(err).Warn("cannot marshal GitStatus to calculate byte length")
530+
s.UncommitedFiles = nil
531+
s.UnpushedCommits = nil
532+
s.UntrackedFiles = nil
533+
return s
534+
}
535+
if len(bytes) < API_BUDGET {
536+
return s
537+
}
538+
539+
// roughly estimate how many bytes we have left for the path arrays (containing long strings)
540+
budget := API_BUDGET - len(s.Branch) - len(s.LatestCommit)
541+
bytesUsed := 0
542+
const PLACEHOLDER = "..."
543+
capArrayAtByteLimit := func(arr []string) []string {
544+
result := make([]string, 0, len(arr))
545+
for _, s := range arr {
546+
bytesRequired := len(s) + 4 // 4 bytes for the JSON encoding
547+
if bytesUsed+bytesRequired+len(PLACEHOLDER) > budget {
548+
result = append(result, PLACEHOLDER)
549+
bytesUsed += len(PLACEHOLDER) + 4
550+
break
551+
}
552+
result = append(result, s)
553+
bytesUsed += bytesRequired
554+
}
555+
return result
556+
}
557+
s.UncommitedFiles = capArrayAtByteLimit(s.UncommitedFiles)
558+
s.UnpushedCommits = capArrayAtByteLimit(s.UnpushedCommits)
559+
s.UntrackedFiles = capArrayAtByteLimit(s.UntrackedFiles)
560+
561+
return s
562+
}

‎components/supervisor/pkg/serverapi/publicapi_test.go

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
// Copyright (c) 2024 Gitpod GmbH. All rights reserved.
2+
// Licensed under the GNU Affero General Public License (AGPL).
3+
// See License.AGPL.txt in the project root for license information.
4+
5+
package serverapi
6+
7+
import (
8+
"encoding/json"
9+
"fmt"
10+
"strings"
11+
"testing"
12+
13+
v1 "github.com/gitpod-io/gitpod/components/public-api/go/experimental/v1"
14+
"github.com/google/go-cmp/cmp"
15+
"github.com/google/go-cmp/cmp/cmpopts"
16+
)
17+
18+
func generateArray(prefix string, number int, length int, withDot bool) []string {
19+
var arr []string
20+
for i := 0; i < number; i++ {
21+
lengthyPart := strings.Repeat("a", length)
22+
arr = append(arr, fmt.Sprintf("%s_%d_%s", prefix, i, lengthyPart))
23+
}
24+
if withDot {
25+
arr = append(arr, "...")
26+
}
27+
return arr
28+
}
29+
30+
func TestCapGitStatusLength(t *testing.T) {
31+
32+
tests := []struct {
33+
name string
34+
input *v1.GitStatus
35+
expected *v1.GitStatus
36+
}{
37+
{
38+
name: "Short GitStatus",
39+
input: &v1.GitStatus{
40+
Branch: "main",
41+
LatestCommit: "abc123",
42+
TotalUncommitedFiles: 2,
43+
TotalUnpushedCommits: 3,
44+
TotalUntrackedFiles: 4,
45+
UncommitedFiles: []string{"file1.txt", "file2.txt"},
46+
UnpushedCommits: []string{"commit1", "commit2", "commit3"},
47+
UntrackedFiles: []string{"file3.txt", "file4.txt", "file5.txt", "file6.txt"},
48+
},
49+
expected: &v1.GitStatus{
50+
Branch: "main",
51+
LatestCommit: "abc123",
52+
TotalUncommitedFiles: 2,
53+
TotalUnpushedCommits: 3,
54+
TotalUntrackedFiles: 4,
55+
UncommitedFiles: []string{"file1.txt", "file2.txt"},
56+
UnpushedCommits: []string{"commit1", "commit2", "commit3"},
57+
UntrackedFiles: []string{"file3.txt", "file4.txt", "file5.txt", "file6.txt"},
58+
},
59+
},
60+
{
61+
name: "Long GitStatus",
62+
input: &v1.GitStatus{
63+
Branch: "main",
64+
LatestCommit: "abc123",
65+
TotalUncommitedFiles: 2,
66+
TotalUnpushedCommits: 3,
67+
TotalUntrackedFiles: 4,
68+
UncommitedFiles: []string{"file1.txt", "file2.txt", "file3.txt", "file4.txt", "file5.txt", "file6.txt"},
69+
UnpushedCommits: []string{"commit1", "commit2", "commit3", "commit4", "commit5", "commit6", "commit7"},
70+
UntrackedFiles: generateArray("file", 800, 10, false),
71+
},
72+
expected: &v1.GitStatus{
73+
Branch: "main",
74+
LatestCommit: "abc123",
75+
TotalUncommitedFiles: 2,
76+
TotalUnpushedCommits: 3,
77+
TotalUntrackedFiles: 4,
78+
UncommitedFiles: []string{"file1.txt", "file2.txt", "file3.txt", "file4.txt", "file5.txt", "file6.txt"},
79+
UnpushedCommits: []string{"commit1", "commit2", "commit3", "commit4", "commit5", "commit6", "commit7"},
80+
UntrackedFiles: generateArray("file", 166, 10, true),
81+
},
82+
},
83+
{
84+
name: "Long paths in GitStatus",
85+
input: &v1.GitStatus{
86+
Branch: "main",
87+
LatestCommit: "abc123",
88+
TotalUncommitedFiles: 2,
89+
TotalUnpushedCommits: 3,
90+
TotalUntrackedFiles: 4,
91+
UncommitedFiles: []string{"file1.txt", "file2.txt", "file3.txt", "file4.txt", "file5.txt", "file6.txt"},
92+
UnpushedCommits: []string{"commit1", "commit2", "commit3", "commit4", "commit5", "commit6", "commit7"},
93+
UntrackedFiles: generateArray("file", 50, 200, false),
94+
},
95+
expected: &v1.GitStatus{
96+
Branch: "main",
97+
LatestCommit: "abc123",
98+
TotalUncommitedFiles: 2,
99+
TotalUnpushedCommits: 3,
100+
TotalUntrackedFiles: 4,
101+
UncommitedFiles: []string{"file1.txt", "file2.txt", "file3.txt", "file4.txt", "file5.txt", "file6.txt"},
102+
UnpushedCommits: []string{"commit1", "commit2", "commit3", "commit4", "commit5", "commit6", "commit7"},
103+
UntrackedFiles: generateArray("file", 17, 200, true),
104+
},
105+
},
106+
{
107+
name: "Empty GitStatus",
108+
input: &v1.GitStatus{
109+
Branch: "",
110+
LatestCommit: "",
111+
TotalUncommitedFiles: 0,
112+
TotalUnpushedCommits: 0,
113+
TotalUntrackedFiles: 0,
114+
UncommitedFiles: nil,
115+
UnpushedCommits: nil,
116+
UntrackedFiles: nil,
117+
},
118+
expected: &v1.GitStatus{
119+
Branch: "",
120+
LatestCommit: "",
121+
TotalUncommitedFiles: 0,
122+
TotalUnpushedCommits: 0,
123+
TotalUntrackedFiles: 0,
124+
UncommitedFiles: nil,
125+
UnpushedCommits: nil,
126+
UntrackedFiles: nil,
127+
},
128+
},
129+
}
130+
131+
for _, tt := range tests {
132+
t.Run(tt.name, func(t *testing.T) {
133+
got := capGitStatusLength(tt.input)
134+
if diff := cmp.Diff(tt.expected, got, cmpopts.IgnoreUnexported(v1.GitStatus{})); diff != "" {
135+
t.Errorf("CapGitStatusLength (-want +got):\n%s", diff)
136+
}
137+
138+
bytes, err := json.Marshal(tt.input)
139+
if err != nil {
140+
t.Error(err)
141+
}
142+
if len(bytes) > GIT_STATUS_API_LIMIT_BYTES {
143+
t.Errorf("JSON size exceeds GIT_STATUS_API_LIMIT_BYTES: %d (%d)", len(bytes), GIT_STATUS_API_LIMIT_BYTES)
144+
}
145+
})
146+
}
147+
}

0 commit comments

Comments
 (0)