diff --git a/api/wallet_changes.go b/api/wallet_changes.go index 2349176..3044e0f 100644 --- a/api/wallet_changes.go +++ b/api/wallet_changes.go @@ -278,7 +278,7 @@ func (s *Server) restWalletChanges(w http.ResponseWriter, r *http.Request) { } var resumeHeight uint64 var resumeHash crypto.Hash32 - var resumeEventIndex uint64 + var resumeEventIndex int if resumeProvided { if id != "" || !witnessProvided || q.Get("cursor") != "" { writeJSONError(w, http.StatusBadRequest, "resume coordinates require a new snapshot and a witness") @@ -294,8 +294,8 @@ func (s *Server) restWalletChanges(w http.ResponseWriter, r *http.Request) { writeJSONError(w, http.StatusBadRequest, "resume_hash must be 64 hexadecimal characters") return } - resumeEventIndex, err = strconv.ParseUint(q.Get("resume_event_index"), 10, 64) - if err != nil || resumeEventIndex > walletChangesMaxResumeEvents { + resumeEventIndex, err = strconv.Atoi(q.Get("resume_event_index")) + if err != nil || resumeEventIndex < 0 || resumeEventIndex > walletChangesMaxResumeEvents { writeJSONError(w, http.StatusBadRequest, "resume_event_index exceeds the supported event index") return } @@ -414,13 +414,13 @@ func (s *Server) restWalletChanges(w http.ResponseWriter, r *http.Request) { reconciliationError(w, resumeHeight, resumeHeight, "resume block event list exceeds decode bounds") return } - if resumeEventIndex > uint64(len(resumeEvents)) { + if resumeEventIndex > len(resumeEvents) { writeWalletSnapshotError(w, http.StatusConflict, "SNAPSHOT_CURSOR_MISMATCH", "resume event index is outside the canonical block") return } } cursor.Height = resumeHeight - cursor.EventIndex = int(resumeEventIndex) + cursor.EventIndex = resumeEventIndex } if token := q.Get("cursor"); token != "" { cursor, err = parseWalletChangesCursor(entry.cursorKey[:], id, token) @@ -452,7 +452,9 @@ func (s *Server) restWalletChanges(w http.ResponseWriter, r *http.Request) { return } - events := make([]map[string]interface{}, 0, limit) + // Use a fixed, protocol-bounded capacity; never size an allocation directly + // from a request parameter. + events := make([]map[string]interface{}, 0, walletChangesMaxEvents) next := cursor expectedParent := fromHash if cursor.Height > fromHeight+1 { diff --git a/api/wallet_changes_test.go b/api/wallet_changes_test.go index 90b0181..5e4a90f 100644 --- a/api/wallet_changes_test.go +++ b/api/wallet_changes_test.go @@ -152,6 +152,63 @@ func TestWalletChangesCursorIsAuthenticatedAndSnapshotBound(t *testing.T) { } } +func TestWalletChangesRejectsOutOfRangePaginationInputs(t *testing.T) { + keys, err := crypto.GenerateWalletKeys() + if err != nil { + t.Fatal(err) + } + zeroHash := hex.EncodeToString(make([]byte, 32)) + base := url.Values{ + "address": {string(crypto.AddressFromKeys(crypto.MainnetByte, keys))}, + "from_height": {"0"}, + "from_hash": {zeroHash}, + } + + tests := []struct { + name string + field string + value string + withResume bool + wantStatus int + }{ + {name: "negative limit", field: "limit", value: "-1", wantStatus: http.StatusBadRequest}, + {name: "zero limit", field: "limit", value: "0", wantStatus: http.StatusBadRequest}, + {name: "limit above page bound", field: "limit", value: "129", wantStatus: http.StatusBadRequest}, + {name: "oversized limit integer", field: "limit", value: "18446744073709551615", wantStatus: http.StatusBadRequest}, + {name: "negative resume index", field: "resume_event_index", value: "-1", withResume: true, wantStatus: http.StatusBadRequest}, + {name: "resume index above bound", field: "resume_event_index", value: "16385", withResume: true, wantStatus: http.StatusBadRequest}, + {name: "oversized resume index integer", field: "resume_event_index", value: "18446744073709551615", withResume: true, wantStatus: http.StatusBadRequest}, + {name: "valid small limit reaches snapshot lookup", field: "limit", value: "1", wantStatus: http.StatusServiceUnavailable}, + {name: "valid zero resume index reaches snapshot lookup", field: "resume_event_index", value: "0", withResume: true, wantStatus: http.StatusServiceUnavailable}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + query := make(url.Values, len(base)) + for key, values := range base { + query[key] = append([]string(nil), values...) + } + query.Set(tt.field, tt.value) + if tt.withResume { + query.Set("witness_height", "0") + query.Set("witness_hash", zeroHash) + query.Set("resume_height", "0") + query.Set("resume_hash", zeroHash) + } + req := httptest.NewRequest( + http.MethodGet, + "/api/v1/wallet/changes?"+query.Encode(), + nil, + ) + recorder := httptest.NewRecorder() + (&Server{}).restWalletChanges(recorder, req) + if recorder.Code != tt.wantStatus { + t.Fatalf("status=%d, want %d; body=%s", recorder.Code, tt.wantStatus, recorder.Body.String()) + } + }) + } +} + func TestWalletChangesPagesAreIdempotentAndIncludeNativeStakeSpend(t *testing.T) { db, migration, address, path, _, _ := syntheticWalletSnapshotLPoDV3Funding(t) parent, height, err := db.GetTip() diff --git a/deploy/publish_public_repo.py b/deploy/publish_public_repo.py index a76cf8b..40fb2a7 100644 --- a/deploy/publish_public_repo.py +++ b/deploy/publish_public_repo.py @@ -46,6 +46,10 @@ "install-node.sh e2e smoke test (validator)", "install-node.sh e2e smoke test (update)", "install-node.sh e2e smoke test (upgrade)", + "Analyze (actions)", + "Analyze (go)", + "Analyze (javascript-typescript)", + "Analyze (python)", } diff --git a/deploy/test_publish_public_repo.py b/deploy/test_publish_public_repo.py index 69853cd..c283124 100644 --- a/deploy/test_publish_public_repo.py +++ b/deploy/test_publish_public_repo.py @@ -116,6 +116,25 @@ def request(method, url, data=None): publisher.await_merge({"number": 1, "html_url": "test"}, request) self.assertNotIn("PUT", methods) + def test_failed_codeql_check_never_merges(self): + methods = [] + failed = "Analyze (go)" + + def request(method, url, data=None): + methods.append(method) + if url.endswith("/pulls/1"): + return {"state": "open", "head": {"sha": "reviewed"}} + return {"check_runs": [ + {"name": name, "status": "completed", + "conclusion": "failure" if name == failed else "success", + "app": {"id": 15368}} + for name in publisher.REQUIRED_CHECKS + ]} + + with self.assertRaisesRegex(RuntimeError, "check failed"): + publisher.await_merge({"number": 1, "html_url": "test"}, request) + self.assertNotIn("PUT", methods) + def test_untrusted_status_provider_cannot_authorize_merge(self): methods = [] def request(method, url, data=None):