Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions backend/internal/provider/openai_image.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,13 @@ func (p *OpenAIImageProvider) ValidateParams(params map[string]interface{}) erro
return fmt.Errorf("size 仅支持 auto、1024x1024、1024x1536、1536x1024")
}

quality, _ := params["quality"].(string)
switch strings.TrimSpace(strings.ToLower(quality)) {
case "", "auto", "low", "medium", "high":
default:
return fmt.Errorf("quality 仅支持 auto、low、medium、high")
Comment on lines +64 to +68

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The new quality validation silently accepts non-string values because params["quality"].(string) ignores failed type assertions and falls back to "", which passes the whitelist. This means inputs like numbers/objects bypass validation and are then dropped from the request body, causing incorrect behavior instead of a clear 400 error. Validate the type explicitly when the key is present, and return an error if quality is not a string. [type error]

Severity Level: Major ⚠️
- ❌ /api/v1/tasks/generate ignores non-string quality silently.
- ⚠️ Image quality may differ from caller's configuration.
Steps of Reproduction ✅
1. Start the backend server (`backend/cmd/server/main.go:260-33`) so that the HTTP route
`POST /api/v1/tasks/generate` is registered with `GenerateHandler` in
`backend/internal/api/handlers.go:399-420`.

2. Send an HTTP request to `POST /api/v1/tasks/generate` with JSON body bound to
`GenerateRequest` (`handlers.go:16-21`), for example:

   `{"provider":"openai-image","model_id":"gpt-image","params":{"prompt":"test
   image","quality":1}}`; because `Params` is `map[string]interface{}`, the `quality`
   value `1` is decoded as a non-string type (`float64`).

3. Inside `GenerateHandler`, `p := provider.GetProvider(req.Provider)` resolves to an
`OpenAIImageProvider` instance for provider `"openai-image"` via
`backend/internal/provider/provider.go:14-21`, and `p.ValidateParams(req.Params)` is
called at `handlers.go:60-63`.

4. In `OpenAIImageProvider.ValidateParams` (`openai_image.go:40-71`), `quality, _ :=
params["quality"].(string)` at line 64 fails the type assertion (value is `float64`), so
`quality` becomes the zero value `""`; the `switch` at lines 65-68 treats `""` as allowed
and returns `nil` instead of an error, and later `buildImagesGenerationRequestBody` at
`openai_image.go:134-155` again asserts `params["quality"].(string)`, sees a non-string,
and omits `body.Quality`, causing the OpenAI Images request to ignore the caller-supplied
`quality` value without returning the expected 400-type validation error.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** backend/internal/provider/openai_image.go
**Line:** 64:68
**Comment:**
	*Type Error: The new quality validation silently accepts non-string values because `params["quality"].(string)` ignores failed type assertions and falls back to `""`, which passes the whitelist. This means inputs like numbers/objects bypass validation and are then dropped from the request body, causing incorrect behavior instead of a clear 400 error. Validate the type explicitly when the key is present, and return an error if `quality` is not a string.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

Comment on lines +64 to +68

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Do not accept quality as a valid input for openai-image; treat it as unsupported for this provider (or explicitly reject when provided) to align with the project rule that gpt-image-2 does not support quality settings. [custom_rule]

Severity Level: Minor ⚠️

Why it matters? 🤔

The final file state explicitly accepts and validates quality as a supported input, but the project rule for openai-image says this provider does not support quality. That makes the suggestion a real rule violation.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** backend/internal/provider/openai_image.go
**Line:** 64:68
**Comment:**
	*Custom Rule: Do not accept `quality` as a valid input for `openai-image`; treat it as unsupported for this provider (or explicitly reject when provided) to align with the project rule that gpt-image-2 does not support quality settings.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

}
Comment on lines +64 to +69

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

建议在 quality 的校验白名单中增加 OpenAI 官方支持的 standardhd。虽然目前项目可能主要配合特定的代理(如 yunwu.ai)使用,但保留对官方标准值的支持可以增强代码的兼容性,防止在切换回官方 API 时发生验证错误。

Suggested change
quality, _ := params["quality"].(string)
switch strings.TrimSpace(strings.ToLower(quality)) {
case "", "auto", "low", "medium", "high":
default:
return fmt.Errorf("quality 仅支持 auto、low、medium、high")
}
quality, _ := params["quality"].(string)
switch strings.TrimSpace(strings.ToLower(quality)) {
case "", "auto", "low", "medium", "high", "standard", "hd":
default:
return fmt.Errorf("quality 仅支持 auto、low、medium、high、standard、hd")
}
References
  1. 所有用户输入必须进行验证和清理。 (link)


return nil
}

Expand Down Expand Up @@ -140,6 +147,9 @@ func (p *OpenAIImageProvider) buildImagesGenerationRequestBody(modelID string, p
if size, _ := params["size"].(string); strings.TrimSpace(size) != "" {
body.Size = strings.TrimSpace(strings.ToLower(size))
}
if quality, _ := params["quality"].(string); strings.TrimSpace(quality) != "" {
body.Quality = strings.TrimSpace(strings.ToLower(quality))
Comment on lines +150 to +151

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Stop mapping quality into the OpenAI Images request payload for openai-image, so requests do not include unsupported endpoint parameters. [custom_rule]

Severity Level: Minor ⚠️

Why it matters? 🤔

The code serializes quality into the OpenAI image request payload even though the provider's rule set indicates quality is unsupported for openai-image. This is a real violation of the custom rule.

Fix in Cursor | Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖
This is a comment left during a code review.

**Path:** backend/internal/provider/openai_image.go
**Line:** 150:151
**Comment:**
	*Custom Rule: Stop mapping `quality` into the OpenAI Images request payload for `openai-image`, so requests do not include unsupported endpoint parameters.

Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
👍 | 👎

}
Comment on lines +150 to +152

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

此处对 quality 进行了重复的 strings.TrimSpace 调用。建议优化逻辑以减少不必要的计算,这符合项目性能优化规范中关于避免冗余计算的要求。

Suggested change
if quality, _ := params["quality"].(string); strings.TrimSpace(quality) != "" {
body.Quality = strings.TrimSpace(strings.ToLower(quality))
}
if quality, _ := params["quality"].(string); quality != "" {
body.Quality = strings.TrimSpace(strings.ToLower(quality))
}
References
  1. 性能优化规范建议避免不必要的重复计算。 (link)

if count, ok := toInt(params["count"]); ok && count >= 1 && count <= 10 {
body.N = count
}
Expand Down
Loading