From 48e522ec00f667c235a751018092125528511f53 Mon Sep 17 00:00:00 2001 From: Dennis Date: Fri, 7 Aug 2026 15:50:35 +0800 Subject: [PATCH] fix(aitable): harden pagination and upload inputs --- .../shortcut/aitable/attachment_composite.go | 9 +++++- .../aitable/attachment_composite_e2e_test.go | 29 +++++++++++++++++++ .../aitable/record_coverage_gap_test.go | 21 +++++++++++++- internal/shortcut/aitable/record_query_all.go | 2 +- .../shortcut/aitable/record_upsert_by_key.go | 5 ++++ .../aitable/table_copy_bulk_patch_e2e_test.go | 19 ++++++++++++ 6 files changed, 82 insertions(+), 3 deletions(-) diff --git a/internal/shortcut/aitable/attachment_composite.go b/internal/shortcut/aitable/attachment_composite.go index 60fc8b9a..449bd4a6 100644 --- a/internal/shortcut/aitable/attachment_composite.go +++ b/internal/shortcut/aitable/attachment_composite.go @@ -21,7 +21,14 @@ import ( const maxAttachmentFileSize = int64(100 * 1024 * 1024) -var attachmentHTTPDo = http.DefaultClient.Do +func rejectAttachmentUploadRedirect(*http.Request, []*http.Request) error { + // Upload credentials are issued for one exact endpoint. Do not let a 3xx + // response redirect local file bytes or the credential to another host or + // to a URL outside validateAttachmentUploadURL's transport policy. + return http.ErrUseLastResponse +} + +var attachmentHTTPDo = (&http.Client{CheckRedirect: rejectAttachmentUploadRedirect}).Do var AttachmentPut = shortcut.Shortcut{ Service: "aitable", diff --git a/internal/shortcut/aitable/attachment_composite_e2e_test.go b/internal/shortcut/aitable/attachment_composite_e2e_test.go index 37b257d5..6c25621b 100644 --- a/internal/shortcut/aitable/attachment_composite_e2e_test.go +++ b/internal/shortcut/aitable/attachment_composite_e2e_test.go @@ -112,6 +112,35 @@ func TestCrossPlatformCoverageAttachmentPutFailuresDoNotBecomeSuccessE2E(t *test }) } +func TestCrossPlatformCoverageAttachmentPutRejectsHTTPSRedirectToPlaintextNonLoopbackE2E(t *testing.T) { + filePath := writeAttachmentFixture(t, "redirect-sensitive bytes") + sourceHits := 0 + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + sourceHits++ + w.Header().Set("Location", "http://example.com/upload") + w.WriteHeader(http.StatusFound) + })) + defer server.Close() + client := server.Client() + client.CheckRedirect = rejectAttachmentUploadRedirect + testseam.Swap(t, &attachmentHTTPDo, client.Do) + + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ + {text: attachmentRecordJSON(t, "field", []any{})}, + {text: mustJSONText(t, map[string]any{"uploadUrl": server.URL + "/put", "fileToken": "ft-redirect"})}, + }} + out, err := runAITableCompositeCLI(t, caller, "+attachment-put", + "--base-id", "base", "--table-id", "table", "--record-id", "record", "--field-id", "field", "--file", filePath, "--mode", "replace", "--yes") + var typed *apperrors.Error + if err == nil || out != "" || !errors.As(err, &typed) || typed.Reason != "aitable_composite_partial_success" || + typed.Cause == nil || !strings.Contains(typed.Cause.Error(), "status 302") { + t.Fatalf("redirect upload = output:%q err:%v", out, err) + } + if sourceHits != 1 || len(caller.calls) != 2 { + t.Fatalf("redirect upload reached an unexpected target: sourceHits=%d calls=%#v", sourceHits, caller.calls) + } +} + func TestCrossPlatformCoverageAttachmentRemoveClearAndSelectiveE2E(t *testing.T) { t.Run("clear empty write response recovered by empty read-back", func(t *testing.T) { caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ diff --git a/internal/shortcut/aitable/record_coverage_gap_test.go b/internal/shortcut/aitable/record_coverage_gap_test.go index 948b29cd..00d89975 100644 --- a/internal/shortcut/aitable/record_coverage_gap_test.go +++ b/internal/shortcut/aitable/record_coverage_gap_test.go @@ -324,6 +324,24 @@ func TestCrossPlatformCoverageRecordUpsertJSONScalarSuccessE2E(t *testing.T) { } } +func TestCrossPlatformCoverageRecordUpsertRejectsTrailingJSONBeforeMCPE2E(t *testing.T) { + for name, value := range map[string]string{ + "trailing garbage": `1 garbage`, + "second JSON value": `1 2`, + "second string value": `true "extra"`, + } { + t.Run(name, func(t *testing.T) { + caller := &upsertByKeyCaller{} + out, err := runAITableCompositeCLI(t, caller, "+record-upsert-by-key", + "--base-id", "base", "--table-id", "table", "--key-field-id", "key", "--key-value-json", value, + "--cells", `{"value":1}`, "--yes") + if err == nil || out != "" || len(caller.calls) != 0 { + t.Fatalf("trailing JSON = output:%q err:%v calls:%#v", out, err, caller.calls) + } + }) + } +} + func TestCrossPlatformCoverageRecordShapeHelpers(t *testing.T) { if records, ok := findRecords(nil); ok || records != nil { t.Fatalf("nil records = %#v, %v", records, ok) @@ -343,7 +361,8 @@ func TestCrossPlatformCoverageRecordShapeHelpers(t *testing.T) { if err := verifyRecordCells(map[string]any{"recordId": "r"}, map[string]any{"f": 1}); err == nil { t.Fatal("missing cells must fail") } - if responseCursor(nil) != "" || responseCursor(map[string]any{"data": map[string]any{"next_cursor": " next "}}) != "next" { + if responseCursor(nil) != "" || responseCursor(map[string]any{"data": map[string]any{"next_cursor": " next "}}) != "next" || + responseCursor(map[string]any{"result": map[string]any{"cursor": " legacy "}}) != "legacy" { t.Fatal("responseCursor shape mismatch") } } diff --git a/internal/shortcut/aitable/record_query_all.go b/internal/shortcut/aitable/record_query_all.go index 9e2970e5..0606abba 100644 --- a/internal/shortcut/aitable/record_query_all.go +++ b/internal/shortcut/aitable/record_query_all.go @@ -54,7 +54,7 @@ func responseCursor(data map[string]any) string { if data == nil { return "" } - for _, key := range []string{"nextCursor", "next_cursor"} { + for _, key := range []string{"nextCursor", "next_cursor", "cursor"} { if value, ok := data[key].(string); ok && strings.TrimSpace(value) != "" { return strings.TrimSpace(value) } diff --git a/internal/shortcut/aitable/record_upsert_by_key.go b/internal/shortcut/aitable/record_upsert_by_key.go index b6c3efbc..ac4a1638 100644 --- a/internal/shortcut/aitable/record_upsert_by_key.go +++ b/internal/shortcut/aitable/record_upsert_by_key.go @@ -6,6 +6,7 @@ package aitable import ( "encoding/json" "fmt" + "io" "reflect" "sort" "strings" @@ -191,6 +192,10 @@ func recordKeyValue(rt *shortcut.RuntimeContext) (any, error) { if err := decoder.Decode(&value); err != nil { return nil, apperrors.NewValidation("--key-value-json 不是合法 JSON: " + err.Error()) } + var trailing any + if err := decoder.Decode(&trailing); err != io.EOF { + return nil, apperrors.NewValidation("--key-value-json 必须只包含一个 JSON 标量") + } if value == nil || reflect.ValueOf(value).Kind() == reflect.Map || reflect.ValueOf(value).Kind() == reflect.Slice { return nil, apperrors.NewValidation("--key-value-json 只接受 string/number/bool 标量") } diff --git a/internal/shortcut/aitable/table_copy_bulk_patch_e2e_test.go b/internal/shortcut/aitable/table_copy_bulk_patch_e2e_test.go index 5bb1ca9f..8c121947 100644 --- a/internal/shortcut/aitable/table_copy_bulk_patch_e2e_test.go +++ b/internal/shortcut/aitable/table_copy_bulk_patch_e2e_test.go @@ -41,6 +41,25 @@ func TestCrossPlatformCoverageRecordBulkPatchPaginatesChunksAndVerifiesE2E(t *te } } +func TestCrossPlatformCoverageRecordBulkPatchPaginatesNestedCursorShapeE2E(t *testing.T) { + records := updateFixtureRecords(0, 2, "旧") + patched := updateFixtureRecords(0, 2, "新") + caller := &upsertByKeyCaller{steps: []upsertByKeyStep{ + {text: mustJSONText(t, map[string]any{"data": map[string]any{"records": records[:1], "hasMore": true, "cursor": "legacy-next"}})}, + {text: mustJSONText(t, map[string]any{"data": map[string]any{"records": records[1:], "hasMore": false}})}, + {text: `{"updatedCount":2}`}, + {text: recordListJSON(t, patched)}, + }} + out, err := runAITableCompositeCLI(t, caller, "+record-bulk-patch", + "--base-id", "base", "--table-id", "table", "--all", "--patch", `{"fldStatus":"新"}`, "--yes") + if err != nil || !strings.Contains(out, `"verifiedCount": 2`) { + t.Fatalf("nested cursor bulk patch = output:%q err:%v", out, err) + } + if len(caller.calls) != 4 || caller.calls[1].args["cursor"] != "legacy-next" || caller.calls[2].tool != "update_records" { + t.Fatalf("nested cursor calls = %#v", caller.calls) + } +} + func TestCrossPlatformCoverageRecordBulkPatchSelectionBoundsAndEmptyE2E(t *testing.T) { t.Run("explicit no matches is success", func(t *testing.T) { caller := &upsertByKeyCaller{steps: []upsertByKeyStep{{text: `{"records":[]}`}}}