fix(security): urlguard for go/request-forgery (closes SEC-1/2/3, bot #848, #808)#955
Conversation
…848, #808) Add an explicit outbound-URL allowlist (`pkg/llmproxy/util/urlguard`) and gate every `http.NewRequestWithContext` call site in `pkg/llmproxy/auth/kiro/sso_oidc.go` and `pkg/llmproxy/executor/antigravity_executor.go` through `urlguard.ValidateOutboundURL` before the URL reaches `net/http`. The allowlist is built from the call-site contexts: * `.amazonaws.com` — AWS SSO OIDC + CodeWhisperer (Kiro auth flows) * `.googleapis.com` — Antigravity / Cloud Code base URLs * `oauth2.googleapis.com` — Antigravity refresh-token endpoint Even though the upstream values come from a region validator (`isValidAWSRegion`) or a base-URL validator (`validateAntigravityBaseURL`), CodeQL's go/request-forgery dataflow does not see those guards. The explicit `ValidateOutboundURL` at each call site: 1. Closes the CodeQL alerts for SEC-1 (#373), SEC-2 (#374), SEC-3 (#375), and the bot-mirrored alerts #848 and #808. 2. Adds a defence-in-depth allowlist so that any future regression that widens the upstream validators still cannot reach an unintended host. 3. Rejects userinfo-bearing URLs and non-http(s) schemes. No third-party dependencies added (stdlib `net/url` only). Pure Go per Phenotype scripting policy. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 7 minutes and 42 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
Note
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request implements a centralized security mechanism to validate outbound URLs before they are used in HTTP requests. By introducing an allowlist-based guard, the changes address specific CodeQL request-forgery alerts and provide a robust, reusable pattern for ensuring that only permitted hostnames are contacted by the application. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
codeant-ai
Bot
added
the
size:L
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| guardedURL, gerr := urlguard.ValidateOutboundURL(requestURL.String()) | ||
| if gerr != nil { | ||
| return cliproxyexecutor.Response{}, gerr | ||
| } | ||
| httpReq, errReq := http.NewRequestWithContext(ctx, http.MethodPost, guardedURL, bytes.NewReader(payload)) |
There was a problem hiding this comment.
Suggestion: Extract the outbound URL guarding and request construction into a small helper used by CountTokens so this modified function can be reduced below the 40-line limit. [custom_rule]
Severity Level: Minor
Why it matters? 🤔
This function is far longer than the 40-line limit; the added URL-guarding/request creation is part of that oversized body.
The suggestion accurately identifies a real custom-rule violation and proposes a valid refactor.
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:** pkg/llmproxy/executor/antigravity_executor.go
**Line:** 1021:1025
**Comment:**
*Custom Rule: Extract the outbound URL guarding and request construction into a small helper used by `CountTokens` so this modified function can be reduced below the 40-line limit.
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| guardedModelsURL, gerr := urlguard.ValidateOutboundURL(modelsURL) | ||
| if gerr != nil { | ||
| log.Warnf("antigravity executor: fetch models rejected by urlguard: %v", gerr) | ||
| lastErr = gerr | ||
| continue | ||
| } | ||
| httpReq, errReq := http.NewRequestWithContext(ctx, http.MethodPost, guardedModelsURL, bytes.NewReader([]byte(`{}`))) |
There was a problem hiding this comment.
Suggestion: Move URL-guard validation plus guarded request creation into a dedicated helper and call it from FetchAntigravityModels to keep this modified function under 40 lines. [custom_rule]
Severity Level: Minor
Why it matters? 🤔
FetchAntigravityModels is also well over the 40-line limit, and the newly added guarded request creation contributes to that violation.
This suggestion matches the custom rule and reflects a real issue in the current code.
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:** pkg/llmproxy/executor/antigravity_executor.go
**Line:** 1142:1148
**Comment:**
*Custom Rule: Move URL-guard validation plus guarded request creation into a dedicated helper and call it from `FetchAntigravityModels` to keep this modified function under 40 lines.
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| guardedTokenURL, gerr := urlguard.ValidateOutboundURL("https://oauth2.googleapis.com/token") | ||
| if gerr != nil { | ||
| return auth, gerr | ||
| } | ||
| httpReq, errReq := http.NewRequestWithContext(ctx, http.MethodPost, guardedTokenURL, strings.NewReader(form.Encode())) |
There was a problem hiding this comment.
Suggestion: Extract token endpoint guarding and HTTP request creation into a reusable helper so the modified refresh flow is split into smaller units and no longer exceeds 40 lines. [custom_rule]
Severity Level: Minor
Why it matters? 🤔
refreshToken is well above the 40-line threshold in the final file state.
The added guarded token request setup is part of that oversized function, so the suggestion is correctly pointing at 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:** pkg/llmproxy/executor/antigravity_executor.go
**Line:** 1364:1368
**Comment:**
*Custom Rule: Extract token endpoint guarding and HTTP request creation into a reusable helper so the modified refresh flow is split into smaller units and no longer exceeds 40 lines.
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| guardedReqURL, gerr := urlguard.ValidateOutboundURL(requestURL.String()) | ||
| if gerr != nil { | ||
| return nil, gerr | ||
| } | ||
| httpReq, errReq := http.NewRequestWithContext(ctx, http.MethodPost, guardedReqURL, strings.NewReader(payloadStr)) |
There was a problem hiding this comment.
Suggestion: Factor out URL validation and guarded request creation from buildRequest into a helper so this modified function is decomposed to stay within the 40-line body limit. [custom_rule]
Severity Level: Minor
Why it matters? 🤔
buildRequest is also substantially longer than 40 lines in the final file state.
The suggestion addresses a real max-line custom-rule violation present in the existing code.
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:** pkg/llmproxy/executor/antigravity_executor.go
**Line:** 1537:1541
**Comment:**
*Custom Rule: Factor out URL validation and guarded request creation from `buildRequest` into a helper so this modified function is decomposed to stay within the 40-line body limit.
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
|
CodeAnt AI finished reviewing your PR. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new urlguard package designed to mitigate Server-Side Request Forgery (SSRF) by validating outbound URLs against a hostname allowlist. The validation logic is integrated into the sso_oidc authentication client and the antigravity_executor to ensure all external requests are authorized. Review feedback suggests unexporting the default allowlist to prevent runtime modification and optimizing the pattern normalization process within the validation function for better performance.
| var Allowlist = []string{ | ||
| // AWS SSO OIDC + CodeWhisperer (Kiro auth flows) | ||
| ".amazonaws.com", | ||
| // Google Antigravity / Cloud Code (executor base URLs) | ||
| ".googleapis.com", | ||
| // Google OAuth token endpoint (antigravity refresh) | ||
| "oauth2.googleapis.com", | ||
| } |
There was a problem hiding this comment.
The Allowlist variable is exported, which allows any package in the application to modify the security policy at runtime. For better security and to prevent accidental or malicious bypasses of the SSRF protection, this should be unexported (e.g., defaultAllowlist).
| var Allowlist = []string{ | |
| // AWS SSO OIDC + CodeWhisperer (Kiro auth flows) | |
| ".amazonaws.com", | |
| // Google Antigravity / Cloud Code (executor base URLs) | |
| ".googleapis.com", | |
| // Google OAuth token endpoint (antigravity refresh) | |
| "oauth2.googleapis.com", | |
| } | |
| var defaultAllowlist = []string{ | |
| // AWS SSO OIDC + CodeWhisperer (Kiro auth flows) | |
| ".amazonaws.com", | |
| // Google Antigravity / Cloud Code (executor base URLs) | |
| ".googleapis.com", | |
| // Google OAuth token endpoint (antigravity refresh) | |
| "oauth2.googleapis.com", | |
| } |
| // This function is the security boundary for go/request-forgery alerts; do | ||
| // not bypass it with a comment-only suppression. | ||
| func ValidateOutboundURL(rawURL string) (string, error) { | ||
| return ValidateOutboundURLAgainst(rawURL, Allowlist) |
There was a problem hiding this comment.
Update the reference to the unexported allowlist variable.
| return ValidateOutboundURLAgainst(rawURL, Allowlist) | |
| return ValidateOutboundURLAgainst(rawURL, defaultAllowlist) |
| return "", fmt.Errorf("urlguard: userinfo not allowed in outbound URL") | ||
| } | ||
| for _, pattern := range allowlist { | ||
| p := strings.ToLower(strings.TrimSpace(pattern)) |
There was a problem hiding this comment.
Normalizing each pattern in the allowlist (trimming and lowercasing) on every call to ValidateOutboundURLAgainst is inefficient, especially since the default allowlist is static and already clean. Consider normalizing the allowlist slice once at the beginning of the function or ensuring the input is pre-normalized.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Security-critical allowlist is exported and mutable
- Changed Allowlist to unexported allowlist to prevent runtime mutation by external packages, hardening the SSRF defense boundary.
- ✅ Fixed: Redundant allowlist entry already covered by suffix pattern
- Removed redundant oauth2.googleapis.com entry as it is already covered by the .googleapis.com suffix pattern.
Or push these changes by commenting:
@cursor push 91d063aeee
Preview (91d063aeee)
diff --git a/pkg/llmproxy/util/urlguard/urlguard.go b/pkg/llmproxy/util/urlguard/urlguard.go
--- a/pkg/llmproxy/util/urlguard/urlguard.go
+++ b/pkg/llmproxy/util/urlguard/urlguard.go
@@ -19,30 +19,28 @@
"strings"
)
-// Allowlist is the canonical set of hostname suffix patterns the proxy is
+// allowlist is the canonical set of hostname suffix patterns the proxy is
// permitted to dial. Keep ordered by subsystem and add a comment for each
// entry explaining the call site.
//
// Patterns:
// - ".example.com" → any subdomain of example.com
// - "host.example" → exact hostname only
-var Allowlist = []string{
+var allowlist = []string{
// AWS SSO OIDC + CodeWhisperer (Kiro auth flows)
".amazonaws.com",
- // Google Antigravity / Cloud Code (executor base URLs)
+ // Google Antigravity / Cloud Code (executor base URLs + OAuth token endpoint)
".googleapis.com",
- // Google OAuth token endpoint (antigravity refresh)
- "oauth2.googleapis.com",
}
// ValidateOutboundURL parses rawURL and returns it unchanged if the host
-// matches any entry in Allowlist. Otherwise it returns an error describing
+// matches any entry in allowlist. Otherwise it returns an error describing
// the rejected host. Schemes other than http/https are always rejected.
//
// This function is the security boundary for go/request-forgery alerts; do
// not bypass it with a comment-only suppression.
func ValidateOutboundURL(rawURL string) (string, error) {
- return ValidateOutboundURLAgainst(rawURL, Allowlist)
+ return ValidateOutboundURLAgainst(rawURL, allowlist)
}
// ValidateOutboundURLAgainst is the testable form of ValidateOutboundURLYou can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 9211ba5. Configure here.
| ".googleapis.com", | ||
| // Google OAuth token endpoint (antigravity refresh) | ||
| "oauth2.googleapis.com", | ||
| } |
There was a problem hiding this comment.
Security-critical allowlist is exported and mutable
Medium Severity
Allowlist is declared as an exported var slice, making it mutable at runtime by any importing package. This is the security boundary for SSRF prevention, yet any code in the binary can do urlguard.Allowlist = append(urlguard.Allowlist, ".evil.com") or urlguard.Allowlist = nil to bypass or break it. A grep confirms urlguard.Allowlist is never referenced outside the package, and ValidateOutboundURLAgainst already exists for callers needing a custom list, so there's no reason for the export.
Reviewed by Cursor Bugbot for commit 9211ba5. Configure here.
| // Google Antigravity / Cloud Code (executor base URLs) | ||
| ".googleapis.com", | ||
| // Google OAuth token endpoint (antigravity refresh) | ||
| "oauth2.googleapis.com", |
There was a problem hiding this comment.
Redundant allowlist entry already covered by suffix pattern
Low Severity
The exact entry "oauth2.googleapis.com" is fully redundant because the suffix pattern ".googleapis.com" on the preceding line already matches it (and any other subdomain of googleapis.com). Having both creates confusion about allowlist semantics — a future maintainer might assume the exact entry is needed for some reason, or might remove the suffix pattern thinking individual entries cover all cases.
Reviewed by Cursor Bugbot for commit 9211ba5. Configure here.
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:L
Sequence DiagramThis PR inserts a shared URL allowlist guard between the Kiro SSO OIDC client and Antigravity executor and their external providers, so that every constructed outbound URL is validated against approved hostnames before any HTTP request is made. sequenceDiagram
participant KiroSSO as Kiro SSO OIDC client
participant Antigravity as Antigravity executor
participant URLGuard as URL allowlist guard
participant AWSOIDC as AWS SSO OIDC and CodeWhisperer
participant GoogleAPIs as Google APIs
KiroSSO->>URLGuard: Validate AWS OIDC URL
URLGuard-->>KiroSSO: Allowed URL or error
alt AWS URL allowed
KiroSSO->>AWSOIDC: Send OIDC and CodeWhisperer requests
else AWS URL rejected
KiroSSO-->>KiroSSO: Abort and surface validation error
end
Antigravity->>URLGuard: Validate Google API URL
URLGuard-->>Antigravity: Allowed URL or error
alt Google URL allowed
Antigravity->>GoogleAPIs: Send model or token requests
else Google URL rejected
Antigravity-->>Antigravity: Abort and surface validation error
end
Generated by CodeAnt AI |
| guardedURL, gerr := guardURL(ssoOIDCEndpoint + "/token") | ||
| if gerr != nil { | ||
| return nil, gerr | ||
| } | ||
| req, err := http.NewRequestWithContext(ctx, http.MethodPost, guardedURL, strings.NewReader(string(body))) |
There was a problem hiding this comment.
Suggestion: RefreshToken is a modified function whose body is over 40 lines; extract shared refresh request execution and result validation into helper(s) to reduce its length. [custom_rule]
Severity Level: Minor
Why it matters? 🤔
RefreshToken is a modified function in the final file and its body is longer than 40 lines.
The added guarded request code is part of that oversized body, so the suggestion correctly points to a real 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:** pkg/llmproxy/auth/kiro/sso_oidc.go
**Line:** 850:854
**Comment:**
*Custom Rule: `RefreshToken` is a modified function whose body is over 40 lines; extract shared refresh request execution and result validation into helper(s) to reduce its length.
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
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:L
Sequence DiagramThis PR adds a shared URL guard that validates outbound HTTP targets for Kiro SSO OIDC and the Antigravity executor against a strict allowlist before any request is sent. It ensures only approved cloud auth and model endpoints are reachable, satisfying CodeQL request-forgery rules and hardening against SSRF. sequenceDiagram
participant Client
participant LLMProxy
participant URLGuard
participant CloudService
Client->>LLMProxy: Send auth or model request
LLMProxy->>URLGuard: Validate constructed outbound URL against allowlist
URLGuard-->>LLMProxy: Approved outbound URL
LLMProxy->>CloudService: HTTP request using approved URL
CloudService-->>LLMProxy: Return tokens, models, or user info
LLMProxy-->>Client: Return successful response
Generated by CodeAnt AI |
| guardedURL, gerr := guardURL(endpoint + "/device_authorization") | ||
| if gerr != nil { | ||
| return nil, gerr | ||
| } | ||
| req, err := http.NewRequestWithContext(ctx, http.MethodPost, guardedURL, strings.NewReader(string(body))) |
There was a problem hiding this comment.
Suggestion: Extract URL-guarding and request creation into a reusable helper to reduce this modified function's body below 40 lines. [custom_rule]
Severity Level: Minor
Why it matters? 🤔
The modified function remains longer than the 40-line limit, so extracting the repeated URL/request setup is a valid response to 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:** pkg/llmproxy/auth/kiro/sso_oidc.go
**Line:** 343:347
**Comment:**
*Custom Rule: Extract URL-guarding and request creation into a reusable helper to reduce this modified function's body below 40 lines.
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| guardedURL, gerr := guardURL(ssoOIDCEndpoint + "/client/register") | ||
| if gerr != nil { | ||
| return nil, gerr | ||
| } | ||
| req, err := http.NewRequestWithContext(ctx, http.MethodPost, guardedURL, strings.NewReader(string(body))) |
There was a problem hiding this comment.
Suggestion: Introduce a small helper for guarded POST request creation so this modified auth-code registration function remains within 40 lines. [custom_rule]
Severity Level: Minor
Why it matters? 🤔
The function body is over the 40-line limit after the added guard/request setup, so this is a real rule violation rather than a speculative refactor suggestion.
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:** pkg/llmproxy/auth/kiro/sso_oidc.go
**Line:** 691:695
**Comment:**
*Custom Rule: Introduce a small helper for guarded POST request creation so this modified auth-code registration function remains within 40 lines.
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
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:L
Sequence DiagramThis PR introduces a shared URL guard that validates outbound HTTP destinations for Kiro SSO OIDC and Antigravity executor calls against a strict hostname allowlist before any request reaches the network layer. sequenceDiagram
participant Client
participant LLMProxy
participant URLGuard
participant ExternalService
Client->>LLMProxy: Trigger auth or model operation
LLMProxy->>LLMProxy: Build outbound URL from region or base URL
LLMProxy->>URLGuard: Validate outbound URL against allowlist
alt URL allowed
URLGuard-->>LLMProxy: Return approved URL
LLMProxy->>ExternalService: Send HTTP request with approved URL
ExternalService-->>LLMProxy: Return response
LLMProxy-->>Client: Deliver auth or model result
else URL rejected
URLGuard-->>LLMProxy: Return error for disallowed host or scheme
LLMProxy-->>Client: Fail operation with URL validation error
end
Generated by CodeAnt AI |
| guardedURL, gerr := guardURL("https://codewhisperer.us-east-1.amazonaws.com") | ||
| if gerr != nil { | ||
| return "" | ||
| } | ||
| req, err := http.NewRequestWithContext(ctx, http.MethodPost, guardedURL, strings.NewReader(string(body))) |
There was a problem hiding this comment.
Suggestion: tryListProfiles is a modified function exceeding 40 lines; extract outbound request creation and response decoding into helpers to satisfy the function-length rule. [custom_rule]
Severity Level: Minor
Why it matters? 🤔
The final code shows tryListProfiles is modified and spans more than 40 lines, so the function-length complaint is valid.
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:** pkg/llmproxy/auth/kiro/sso_oidc.go
**Line:** 1126:1130
**Comment:**
*Custom Rule: `tryListProfiles` is a modified function exceeding 40 lines; extract outbound request creation and response decoding into helpers to satisfy the function-length rule.
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| guardedURL, gerr := guardURL(ssoOIDCEndpoint + "/client/register") | ||
| if gerr != nil { | ||
| return nil, gerr | ||
| } | ||
| req, err := http.NewRequestWithContext(ctx, http.MethodPost, guardedURL, strings.NewReader(string(body))) |
There was a problem hiding this comment.
Suggestion: RegisterClientForAuthCode exceeds the 40-line cap after modification; factor out the shared client-registration HTTP call path into a helper to keep this function under the limit. [custom_rule]
Severity Level: Minor
Why it matters? 🤔
RegisterClientForAuthCode is also modified and remains above the 40-line limit in the final file state, so this suggestion points to a real 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:** pkg/llmproxy/auth/kiro/sso_oidc.go
**Line:** 1254:1258
**Comment:**
*Custom Rule: `RegisterClientForAuthCode` exceeds the 40-line cap after modification; factor out the shared client-registration HTTP call path into a helper to keep this function under the limit.
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
| var Allowlist = []string{ | ||
| // AWS SSO OIDC + CodeWhisperer (Kiro auth flows) | ||
| ".amazonaws.com", | ||
| // Google Antigravity / Cloud Code (executor base URLs) | ||
| ".googleapis.com", | ||
| // Google OAuth token endpoint (antigravity refresh) | ||
| "oauth2.googleapis.com", | ||
| } |
There was a problem hiding this comment.
Suggestion: The allowlist is exported as a mutable package variable, so any other package can modify it at runtime and silently bypass or alter the SSRF boundary. Keep the canonical allowlist immutable/private and validate against a copy to avoid accidental or malicious mutation. [security]
Severity Level: Critical 🚨
- URL guard SSRF boundary can be weakened at runtime.
- Antigravity executor may dial unintended hosts with tokens.
- Kiro AWS SSO flows could target malicious endpoints.Steps of Reproduction ✅
1. Inspect the global allowlist definition at
`pkg/llmproxy/util/urlguard/urlguard.go:22-36`, where `Allowlist` is declared as an
exported `var Allowlist = []string{...}`, making the canonical SSRF allowlist both
package-global and mutable from any importing package.
2. Note that `ValidateOutboundURL()` at `urlguard.go:44-45` reads directly from this
mutable `Allowlist` slice on every call without copying or freezing it, so any runtime
mutation (for example appending a new pattern or replacing elements) will immediately
affect all outbound URL validation.
3. Trace the primary security-sensitive callers: AWS SSO/IDC flows in
`pkg/llmproxy/auth/kiro/sso_oidc.go` call `guardURL()` at `lines 25-31`, which delegates
to `urlguard.ValidateOutboundURL` and is used in `RegisterClientWithRegion` (`lines
27-80`), `StartDeviceAuthorizationWithIDC` (`82-137`), `CreateTokenWithRegion`
(`139-209`), and `RefreshTokenWithRegion` (`222-259`) to protect endpoints carrying client
credentials, device codes, and tokens.
4. Similarly, Antigravity flows in `pkg/llmproxy/executor/antigravity_executor.go` call
`urlguard.ValidateOutboundURL` for token-counting (`989-1010` with guarded URL at `42-46`
in the shown snippet), model listing (`161-169` in `FetchAntigravityModels`), OAuth token
refresh (`385-390` in `refreshToken`), and main request building (`558-562` in
`buildRequest`), all sending bearer tokens or OAuth secrets over these guarded URLs.
5. Because `Allowlist` is exported and mutable, any code running in the same process
(including future packages or plugin-style extensions) can execute statements like
`urlguard.Allowlist = append(urlguard.Allowlist, ".attacker.com")` or mutate existing
entries; after such a change, the above Kiro and Antigravity flows will happily send their
authenticated HTTP requests (with Authorization headers and tokens) to attacker-controlled
hosts without any change to `urlguard.go` itself, undermining the intended SSRF boundary.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:** pkg/llmproxy/util/urlguard/urlguard.go
**Line:** 29:36
**Comment:**
*Security: The allowlist is exported as a mutable package variable, so any other package can modify it at runtime and silently bypass or alter the SSRF boundary. Keep the canonical allowlist immutable/private and validate against a copy to avoid accidental or malicious mutation.
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
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
1 participant
User description
Summary
pkg/llmproxy/util/urlguard— outbound-URL allowlist +ValidateOutboundURL(stdlib only, no new deps)http.NewRequestWithContextsite inpkg/llmproxy/auth/kiro/sso_oidc.goandpkg/llmproxy/executor/antigravity_executor.gothroughurlguard.ValidateOutboundURLbefore the URL reachesnet/http.amazonaws.com(AWS SSO OIDC + CodeWhisperer / Kiro).googleapis.com(Antigravity / Cloud Code)oauth2.googleapis.com(Antigravity refresh)Why
CodeQL's go/request-forgery dataflow does not see the upstream
isValidAWSRegion/validateAntigravityBaseURLguards. Adding an explicit allowlist check at every NewRequest call site:Per
repos/docs/governance/cliproxyapi-security-triage-2026-04.mdpriority 1.Test plan
go test ./pkg/llmproxy/util/urlguard/...passes (allow + reject cases)gofmt -eclean on changed filesNotes
net/urlonlypkg/llmproxy/auth/qwen,pkg/llmproxy/auth/gemini,pkg/llmproxy/auth/kiro/aws_auth.go,sdk/cliproxy/auth/manager_ops.goetc. exist onmainand are out of scope for this security PRNote
Medium Risk
Adds a new outbound URL allowlist check that can cause authentication/executor HTTP calls to fail if legitimate endpoints fall outside the configured patterns, impacting login/refresh and model/execution flows. The change is security-motivated and localized but touches multiple network request paths.
Overview
Introduces
pkg/llmproxy/util/urlguard, a stdlib-only outbound URL validator that enforces an HTTP(S)-only, no-userinfo policy and a hostname allowlist (AWS.amazonaws.com, Google.googleapis.com, andoauth2.googleapis.com) to mitigate SSRF / CodeQLgo/request-forgery.Routes all constructed outbound
http.NewRequestWithContextURLs in Kiro AWS SSO OIDC flows (sso_oidc.go) and Antigravity executor flows (antigravity_executor.go, including token refresh, model fetch, count-tokens, and request execution) throughurlguard.ValidateOutboundURLbefore issuing requests, returning/continuing on validation failure. Adds unit tests covering allowed and rejected URLs.Reviewed by Cursor Bugbot for commit 9211ba5. Bugbot is set up for automated code reviews on this repo. Configure here.
CodeAnt-AI Description
Block outbound requests to unapproved hosts
What Changed
Impact
✅ Lower risk of unintended outbound requests✅ Safer sign-in and token refresh flows✅ Clearer rejection of invalid request URLs🔄 Retrigger CodeAnt AI Review
Details
💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.