fix: verify validity of next and prev commit IDs (#1475)

If next and prev commits were invalid, they could end up crashing cd-service (i.e id = 1) --------- Co-authored-by: Sven Urbanski <[email protected]>
freiheit-com · Mar 26, 2024 · c4d33c9 · c4d33c9
1 parent d14053c
commit c4d33c9
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 4 deletions.
diff --git a/services/cd-service/pkg/repository/transformer.go b/services/cd-service/pkg/repository/transformer.go
@@ -562,12 +562,12 @@ func writeCommitData(ctx context.Context, sourceCommitId string, sourceMessage s
 		return GetCreateReleaseGeneralFailure(err)
 	}
 
-	if previousCommitId != "" {
+	if previousCommitId != "" && valid.SHA1CommitID(previousCommitId) {
 		if err := writeNextPrevInfo(ctx, sourceCommitId, strings.ToLower(previousCommitId), fieldPreviousCommitId, app, fs); err != nil {
 			return GetCreateReleaseGeneralFailure(err)
 		}
 	}
-	if nextCommitId != "" {
+	if nextCommitId != "" && valid.SHA1CommitID(nextCommitId) {
 		if err := writeNextPrevInfo(ctx, sourceCommitId, strings.ToLower(nextCommitId), fieldNextCommidId, app, fs); err != nil {
 			return GetCreateReleaseGeneralFailure(err)
 		}

diff --git a/services/cd-service/pkg/repository/transformer_test.go b/services/cd-service/pkg/repository/transformer_test.go
@@ -1675,6 +1675,7 @@ func TestNextAndPreviousCommitCreation(t *testing.T) {
 		Name            string
 		Transformers    []Transformer
 		expectedContent []FileWithContent
+		expectedError   error
 	}
 
 	tcs := []TestCase{
@@ -1836,6 +1837,29 @@ func TestNextAndPreviousCommitCreation(t *testing.T) {
 				},
 			},
 		},
+		{
+			Name: "Invalid commit IDS do not create files",
+			// no need to bother with environments here
+			Transformers: []Transformer{
+				&CreateApplicationVersion{
+					Application:    "app",
+					SourceCommitId: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+					Manifests: map[string]string{
+						"staging": "doesn't matter",
+					},
+					WriteCommitData: true,
+					NextCommit:      "1",
+					PreviousCommit:  "1234",
+				},
+			},
+			expectedContent: []FileWithContent{
+				{
+					Path:    "commits/12/34/nextCommit",
+					Content: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+				},
+			},
+			expectedError: errMatcher{"Error while opening file commits/12/34/nextCommit, error: file does not exist"},
+		},
 	}
 
 	for _, tc := range tcs {
@@ -1856,8 +1880,8 @@ func TestNextAndPreviousCommitCreation(t *testing.T) {
 
 			verErr := verifyContent(fs, tc.expectedContent)
 
-			if verErr != nil {
-				t.Fatalf("Error while verifying content of : %v. Filesystem content:\n%s", verErr, strings.Join(listFiles(fs), "\n"))
+			if diff := cmp.Diff(tc.expectedError, verErr, cmpopts.EquateErrors()); diff != "" {
+				t.Fatalf("error mismatch (-want, +got):\n%s", diff)
 			}
 		})
 	}