Skip to content

Commit 32c3cf6

Browse files
axelmarcianoclaude
andcommitted
fix: move UpsertBranch after auth and input validation in handlers
Prevents unauthenticated requests from creating branches as a side-effect. Also adds missing return after http.Error for auth failures and invalid platform, removes dead expoAccount nil check, fixes wrong error variable in RollbackHandler log, and handles UploadFileIntoUpdate error. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c3af057 commit 32c3cf6

4 files changed

Lines changed: 40 additions & 35 deletions

File tree

internal/handlers/republish_handler.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,6 @@ func RepublishHandler(w http.ResponseWriter, r *http.Request) {
4040
http.Error(w, "No expo account found", http.StatusUnauthorized)
4141
return
4242
}
43-
err = branch.UpsertBranch(branchName)
44-
if err != nil {
45-
log.Printf("[RequestID: %s] Error upserting branch: %v", requestID, err)
46-
http.Error(w, "Error upserting branch", http.StatusInternalServerError)
47-
return
48-
}
4943
runtimeVersion := r.URL.Query().Get("runtimeVersion")
5044
if runtimeVersion == "" {
5145
log.Printf("[RequestID: %s] No runtime version provided", requestID)
@@ -59,6 +53,12 @@ func RepublishHandler(w http.ResponseWriter, r *http.Request) {
5953
http.Error(w, "No updateId provided", http.StatusBadRequest)
6054
return
6155
}
56+
err = branch.UpsertBranch(branchName)
57+
if err != nil {
58+
log.Printf("[RequestID: %s] Error upserting branch: %v", requestID, err)
59+
http.Error(w, "Error upserting branch", http.StatusInternalServerError)
60+
return
61+
}
6262
update, err := update2.GetUpdate(branchName, runtimeVersion, updateId)
6363
if err != nil {
6464
log.Printf("[RequestID: %s] Error getting update: %v", requestID, err)

internal/handlers/rollback_handler.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,18 @@ func RollbackHandler(w http.ResponseWriter, r *http.Request) {
3939
http.Error(w, "No expo account found", http.StatusUnauthorized)
4040
return
4141
}
42-
errUpsert := branch.UpsertBranch(branchName)
43-
if errUpsert != nil {
44-
log.Printf("[RequestID: %s] Error upserting branch: %v", requestID, err)
45-
http.Error(w, "Error upserting branch", http.StatusInternalServerError)
46-
return
47-
}
4842
runtimeVersion := r.URL.Query().Get("runtimeVersion")
4943
if runtimeVersion == "" {
5044
log.Printf("[RequestID: %s] No runtime version provided", requestID)
5145
http.Error(w, "No runtime version provided", http.StatusBadRequest)
5246
return
5347
}
48+
errUpsert := branch.UpsertBranch(branchName)
49+
if errUpsert != nil {
50+
log.Printf("[RequestID: %s] Error upserting branch: %v", requestID, errUpsert)
51+
http.Error(w, "Error upserting branch", http.StatusInternalServerError)
52+
return
53+
}
5454
commitHash := r.URL.Query().Get("commitHash")
5555
rollback, err := update.CreateRollback(platform, commitHash, runtimeVersion, branchName)
5656
if err != nil {

internal/handlers/upload_handler.go

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,13 @@ import (
1111
"expo-open-ota/internal/types"
1212
"expo-open-ota/internal/update"
1313
"fmt"
14-
"github.com/google/uuid"
15-
"github.com/gorilla/mux"
1614
"log"
1715
"net/http"
1816
"path/filepath"
1917
"time"
18+
19+
"github.com/google/uuid"
20+
"github.com/gorilla/mux"
2021
)
2122

2223
type FileNamesRequest struct {
@@ -38,17 +39,12 @@ func MarkUpdateAsUploadedHandler(w http.ResponseWriter, r *http.Request) {
3839
http.Error(w, "No branch provided", http.StatusBadRequest)
3940
return
4041
}
41-
err := branch.UpsertBranch(branchName)
42-
if err != nil {
43-
log.Printf("[RequestID: %s] Error upserting branch: %v", requestID, err)
44-
http.Error(w, "Error upserting branch", http.StatusInternalServerError)
45-
return
46-
}
4742
expoAuth := helpers.GetExpoAuth(r)
4843
expoAccount, err := services.ValidateExpoAuth(expoAuth)
4944
if err != nil {
5045
log.Printf("[RequestID: %s] Error validating expo auth: %v", requestID, err)
5146
http.Error(w, "Error validating expo auth", http.StatusUnauthorized)
47+
return
5248
}
5349
if expoAccount == nil {
5450
log.Printf("[RequestID: %s] No expo account found", requestID)
@@ -67,6 +63,12 @@ func MarkUpdateAsUploadedHandler(w http.ResponseWriter, r *http.Request) {
6763
http.Error(w, "No update id provided", http.StatusBadRequest)
6864
return
6965
}
66+
err = branch.UpsertBranch(branchName)
67+
if err != nil {
68+
log.Printf("[RequestID: %s] Error upserting branch: %v", requestID, err)
69+
http.Error(w, "Error upserting branch", http.StatusInternalServerError)
70+
return
71+
}
7072
currentUpdate, err := update.GetUpdate(branchName, runtimeVersion, updateId)
7173
if err != nil {
7274
log.Printf("[RequestID: %s] Error getting update: %v", requestID, err)
@@ -149,6 +151,7 @@ func RequestUploadLocalFileHandler(w http.ResponseWriter, r *http.Request) {
149151
if err != nil || expoAccount == nil {
150152
log.Printf("[RequestID: %s] Error validating expo auth: %v", requestID, err)
151153
http.Error(w, "Error validating expo auth", http.StatusUnauthorized)
154+
return
152155
}
153156
token := r.URL.Query().Get("token")
154157
if token == "" {
@@ -207,25 +210,14 @@ func RequestUploadUrlHandler(w http.ResponseWriter, r *http.Request) {
207210
if err != nil || expoAccount == nil {
208211
log.Printf("[RequestID: %s] Error validating expo auth: %v", requestID, err)
209212
http.Error(w, "Error validating expo auth", http.StatusUnauthorized)
210-
}
211-
212-
err = branch.UpsertBranch(branchName)
213-
if err != nil {
214-
log.Printf("[RequestID: %s] Error upserting branch: %v", requestID, err)
215-
http.Error(w, "Error upserting branch", http.StatusInternalServerError)
216-
return
217-
}
218-
219-
if expoAccount == nil {
220-
log.Printf("[RequestID: %s] No expo account found", requestID)
221-
http.Error(w, "No expo account found", http.StatusUnauthorized)
222213
return
223214
}
224215

225216
platform := r.URL.Query().Get("platform")
226217
if platform != "" && (platform != "ios" && platform != "android") {
227218
log.Printf("[RequestID: %s] Invalid platform: %s", requestID, platform)
228219
http.Error(w, "Invalid platform", http.StatusBadRequest)
220+
return
229221
}
230222
commitHash := r.URL.Query().Get("commitHash")
231223
runtimeVersion := r.URL.Query().Get("runtimeVersion")
@@ -248,6 +240,13 @@ func RequestUploadUrlHandler(w http.ResponseWriter, r *http.Request) {
248240
return
249241
}
250242

243+
err = branch.UpsertBranch(branchName)
244+
if err != nil {
245+
log.Printf("[RequestID: %s] Error upserting branch: %v", requestID, err)
246+
http.Error(w, "Error upserting branch", http.StatusInternalServerError)
247+
return
248+
}
249+
251250
updateId := update.GenerateUpdateTimestamp()
252251
updateRequests, err := bucket.RequestUploadUrlsForFileUpdates(branchName, runtimeVersion, update.ConvertUpdateTimestampToString(updateId), request.FileNames)
253252
if err != nil {
@@ -274,6 +273,12 @@ func RequestUploadUrlHandler(w http.ResponseWriter, r *http.Request) {
274273
CreatedAt: time.Duration(updateId) * time.Millisecond,
275274
}, "update-metadata.json", metadataReader)
276275

276+
if err != nil {
277+
log.Printf("[RequestID: %s] Error uploading file update metadata: %v", requestID, err)
278+
http.Error(w, "Error uploading file update metadata", http.StatusInternalServerError)
279+
return
280+
}
281+
277282
cache := cache2.GetCache()
278283
cacheKey := update.ComputeLastUpdateCacheKey(branchName, runtimeVersion, platform)
279284
cache.Delete(cacheKey)

test/requestUpload_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ func TestRequestUploadUrlWithoutBearer(t *testing.T) {
163163
w, _, _, r := createUploadRequest(t, projectRoot, "DO_NOT_USE", "1", sampleUpdatePath, "Authorization", "Bearer expo_alternative_token", "ios")
164164
handlers.RequestUploadUrlHandler(w, r)
165165
assert.Equal(t, 401, w.Code, "Expected status code 401")
166-
assert.Equal(t, "Error validating expo auth\nNo expo account found\n", w.Body.String(), "Expected error message")
166+
assert.Equal(t, "Error validating expo auth\n", w.Body.String(), "Expected error message")
167167
}
168168

169169
func TestRequestUploadUrlWithBadBearer(t *testing.T) {
@@ -178,7 +178,7 @@ func TestRequestUploadUrlWithBadBearer(t *testing.T) {
178178
w, _, _, r := createUploadRequest(t, projectRoot, "DO_NOT_USE", "1", sampleUpdatePath, "Authorization", "Bearer expo_bad_token", "ios")
179179
handlers.RequestUploadUrlHandler(w, r)
180180
assert.Equal(t, 401, w.Code, "Expected status code 401")
181-
assert.Equal(t, "Error validating expo auth\nNo expo account found\n", w.Body.String(), "Expected error message")
181+
assert.Equal(t, "Error validating expo auth\n", w.Body.String(), "Expected error message")
182182
}
183183

184184
func TestRequestUploadUrlWithoutRuntimeVersion(t *testing.T) {
@@ -480,7 +480,7 @@ func TestRequestUploadUrlWithInvalidExpoSession(t *testing.T) {
480480
r.Body = io.NopCloser(bytes.NewReader(uploadRequestsInputJSON))
481481
handlers.RequestUploadUrlHandler(w, r)
482482
assert.Equal(t, 401, w.Code, "Expected status code 401")
483-
assert.Equal(t, "Error validating expo auth\nNo expo account found\n", w.Body.String(), "Expected error message")
483+
assert.Equal(t, "Error validating expo auth\n", w.Body.String(), "Expected error message")
484484
}
485485

486486
func TestIdenticalUpload(t *testing.T) {

0 commit comments

Comments
 (0)