-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
fix(antigravity): sanitize request.contents to remove invalid metadata entries #1326
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1318,6 +1318,12 @@ func (e *AntigravityExecutor) buildRequest(ctx context.Context, auth *cliproxyau | |
| payload, _ = sjson.DeleteBytes(payload, "request.generationConfig.maxOutputTokens") | ||
| } | ||
|
|
||
| // Sanitize request.contents to remove any invalid entries that contain | ||
| // request-level metadata fields (safetySettings, model, systemInstruction, etc.) | ||
| // This prevents "Invalid JSON payload" errors from the Gemini/Antigravity API | ||
| // when malformed history entries are accidentally included in contents. | ||
| payload = sanitizeRequestContents(payload) | ||
|
|
||
| httpReq, errReq := http.NewRequestWithContext(ctx, http.MethodPost, requestURL.String(), bytes.NewReader(payload)) | ||
| if errReq != nil { | ||
| return nil, errReq | ||
|
|
@@ -1594,3 +1600,69 @@ func generateProjectID() string { | |
| randomPart := strings.ToLower(uuid.NewString())[:5] | ||
| return adj + "-" + noun + "-" + randomPart | ||
| } | ||
|
|
||
| // sanitizeRequestContents removes invalid entries from request.contents that contain | ||
| // request-level metadata fields instead of proper message content. | ||
| // Valid content entries should only have: role, parts | ||
| // Invalid entries may contain: safetySettings, model, userAgent, requestType, requestId, | ||
| // sessionId, systemInstruction, toolConfig, generationConfig, etc. | ||
| func sanitizeRequestContents(payload []byte) []byte { | ||
| contentsPath := "request.contents" | ||
| contentsResult := gjson.GetBytes(payload, contentsPath) | ||
| if !contentsResult.Exists() || !contentsResult.IsArray() { | ||
| return payload | ||
| } | ||
|
|
||
| invalidFieldsSet := map[string]bool{ | ||
| "safetySettings": true, | ||
| "model": true, | ||
| "userAgent": true, | ||
| "requestType": true, | ||
| "requestId": true, | ||
| "sessionId": true, | ||
| "systemInstruction": true, | ||
| "toolConfig": true, | ||
| "generationConfig": true, | ||
| "project": true, | ||
| "request": true, | ||
| "contents": true, | ||
| } | ||
|
|
||
| validContents := make([]gjson.Result, 0) | ||
| contentsResult.ForEach(func(_, content gjson.Result) bool { | ||
| if !content.IsObject() { | ||
| return true | ||
| } | ||
|
|
||
| hasInvalidField := false | ||
| content.ForEach(func(key, _ gjson.Result) bool { | ||
| if invalidFieldsSet[key.String()] { | ||
| hasInvalidField = true | ||
| log.Warnf("sanitizeRequestContents: dropping invalid content entry with field %q", key.String()) | ||
| return false | ||
| } | ||
| return true | ||
| }) | ||
|
|
||
| if !hasInvalidField { | ||
| validContents = append(validContents, content) | ||
| } | ||
| return true | ||
| }) | ||
|
|
||
| if len(validContents) == len(contentsResult.Array()) { | ||
| return payload | ||
| } | ||
|
|
||
| newContentsJSON := "[" | ||
| for i, content := range validContents { | ||
| if i > 0 { | ||
| newContentsJSON += "," | ||
| } | ||
| newContentsJSON += content.Raw | ||
| } | ||
| newContentsJSON += "]" | ||
|
|
||
| result, _ := sjson.SetRawBytes(payload, contentsPath, []byte(newContentsJSON)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The error returned by result, err := sjson.SetRawBytes(payload, contentsPath, []byte(newContentsJSON))
if err != nil {
log.Errorf("sanitizeRequestContents: failed to set raw bytes for contents: %v", err)
return payload // Return original payload on error, or handle as appropriate
} |
||
| return result | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,140 @@ | ||||||||
| package executor | ||||||||
|
|
||||||||
| import ( | ||||||||
| "testing" | ||||||||
|
|
||||||||
| "github.com/tidwall/gjson" | ||||||||
| ) | ||||||||
|
|
||||||||
| func TestSanitizeRequestContents(t *testing.T) { | ||||||||
| tests := []struct { | ||||||||
| name string | ||||||||
| input string | ||||||||
| expectedCount int | ||||||||
| shouldModify bool | ||||||||
| }{ | ||||||||
| { | ||||||||
| name: "valid contents unchanged", | ||||||||
| input: `{ | ||||||||
| "request": { | ||||||||
| "contents": [ | ||||||||
| {"role": "user", "parts": [{"text": "hello"}]}, | ||||||||
| {"role": "model", "parts": [{"text": "hi"}]} | ||||||||
| ] | ||||||||
| } | ||||||||
| }`, | ||||||||
| expectedCount: 2, | ||||||||
| shouldModify: false, | ||||||||
| }, | ||||||||
| { | ||||||||
| name: "removes entry with safetySettings", | ||||||||
| input: `{ | ||||||||
| "request": { | ||||||||
| "contents": [ | ||||||||
| {"role": "user", "parts": [{"text": "hello"}]}, | ||||||||
| {"safetySettings": [], "model": "test"} | ||||||||
| ] | ||||||||
| } | ||||||||
| }`, | ||||||||
| expectedCount: 1, | ||||||||
| shouldModify: true, | ||||||||
| }, | ||||||||
| { | ||||||||
| name: "removes entry with model field", | ||||||||
| input: `{ | ||||||||
| "request": { | ||||||||
| "contents": [ | ||||||||
| {"role": "user", "parts": [{"text": "hello"}]}, | ||||||||
| {"model": "gemini-pro", "userAgent": "test"} | ||||||||
| ] | ||||||||
| } | ||||||||
| }`, | ||||||||
| expectedCount: 1, | ||||||||
| shouldModify: true, | ||||||||
| }, | ||||||||
| { | ||||||||
| name: "removes entry with systemInstruction", | ||||||||
| input: `{ | ||||||||
| "request": { | ||||||||
| "contents": [ | ||||||||
| {"systemInstruction": {}, "toolConfig": {}} | ||||||||
| ] | ||||||||
| } | ||||||||
| }`, | ||||||||
| expectedCount: 0, | ||||||||
| shouldModify: true, | ||||||||
| }, | ||||||||
| { | ||||||||
| name: "removes entry with request metadata fields", | ||||||||
| input: `{ | ||||||||
| "request": { | ||||||||
| "contents": [ | ||||||||
| {"role": "user", "parts": [{"text": "hello"}]}, | ||||||||
| {"requestId": "123", "requestType": "agent", "sessionId": "456"} | ||||||||
| ] | ||||||||
| } | ||||||||
| }`, | ||||||||
| expectedCount: 1, | ||||||||
| shouldModify: true, | ||||||||
| }, | ||||||||
| { | ||||||||
| name: "keeps function call/response entries", | ||||||||
| input: `{ | ||||||||
| "request": { | ||||||||
| "contents": [ | ||||||||
| {"role": "user", "parts": [{"text": "hello"}]}, | ||||||||
| {"role": "model", "parts": [{"functionCall": {"name": "test", "args": {}}}]}, | ||||||||
| {"role": "function", "parts": [{"functionResponse": {"name": "test", "response": {}}}]} | ||||||||
| ] | ||||||||
| } | ||||||||
| }`, | ||||||||
| expectedCount: 3, | ||||||||
| shouldModify: false, | ||||||||
| }, | ||||||||
| { | ||||||||
| name: "handles empty contents", | ||||||||
| input: `{"request": {"contents": []}}`, | ||||||||
| expectedCount: 0, | ||||||||
| shouldModify: false, | ||||||||
| }, | ||||||||
| { | ||||||||
| name: "handles missing contents", | ||||||||
| input: `{"request": {}}`, | ||||||||
| expectedCount: -1, | ||||||||
| shouldModify: false, | ||||||||
| }, | ||||||||
| } | ||||||||
|
|
||||||||
| for _, tt := range tests { | ||||||||
| t.Run(tt.name, func(t *testing.T) { | ||||||||
| result := sanitizeRequestContents([]byte(tt.input)) | ||||||||
|
|
||||||||
| contentsResult := gjson.GetBytes(result, "request.contents") | ||||||||
|
|
||||||||
| if tt.expectedCount == -1 { | ||||||||
| if contentsResult.Exists() { | ||||||||
| t.Errorf("expected no contents field, but got one") | ||||||||
| } | ||||||||
| return | ||||||||
| } | ||||||||
|
|
||||||||
| if !contentsResult.IsArray() { | ||||||||
| t.Fatalf("expected contents to be an array") | ||||||||
| } | ||||||||
|
|
||||||||
| actualCount := len(contentsResult.Array()) | ||||||||
| if actualCount != tt.expectedCount { | ||||||||
| t.Errorf("expected %d contents, got %d", tt.expectedCount, actualCount) | ||||||||
| } | ||||||||
|
|
||||||||
| for i, content := range contentsResult.Array() { | ||||||||
| invalidFields := []string{"safetySettings", "model", "userAgent", "requestType", "requestId", "sessionId", "systemInstruction", "toolConfig", "generationConfig", "project", "request", "contents"} | ||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Suggested change
|
||||||||
| for _, field := range invalidFields { | ||||||||
| if content.Get(field).Exists() { | ||||||||
| t.Errorf("content[%d] should not have field %q", i, field) | ||||||||
| } | ||||||||
| } | ||||||||
| } | ||||||||
| }) | ||||||||
| } | ||||||||
| } | ||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reconstructing the
newContentsJSONstring by concatenatingcontent.Rawin a loop can be inefficient for very largecontentsarrays. Whilegjson.Result.Rawprovides the raw JSON, repeatedly concatenating strings can lead to multiple memory allocations. Consider building a slice ofinterface{}ormap[string]interface{}fromvalidContentsand then marshaling it once to JSON, or exploring ifsjsonoffers a more direct way to replace an array with a slice ofgjson.Resultobjects.