Files
ragflow/internal/handler/oauth_login.go
Zhichang Yu a06343eafe fix(codeql): close remaining 44 CodeQL alerts post-merge (#16408)
## Summary

After #16407 merged, 44 of the original 93 CodeQL alerts were still open
on the default branch. This PR closes the remaining ones by:

1. **Moving 32 existing `// codeql[...]` directives** so they sit on the
line **immediately before** the suppressed statement. The original
multi-line suppression blocks had the directive as the first line, with
the rationale on subsequent lines. After line shifts (refactors, linter
reformat), the directive ended up several lines above the alert location
— CodeQL only recognizes the suppression when it appears on the line
directly above. (32 alerts across 27 files.)

2. **Adding 9 new `// codeql[...]` suppressions** for alerts that had no
suppression in the preceding lines at all — mostly real-fixes that
CodeQL conservatively still flags (filepath.Base, bounded slice sizes,
model-identifier strings, the MD5-legacy-migration lookup in
`conversation_service.py`).

## Files changed

- `api/db/services/conversation_service.py` — add
`py/weak-sensitive-data-hashing` suppression (MD5 for backward-compat
legacy row lookup; not used for auth)
- `api/db/services/llm_service.py` — 3×
`py/clear-text-logging-sensitive-data` suppressions on the lines that
log `llm_name` in warnings/info
- `common/misc_utils.py` — 2× `py/clear-text-logging-sensitive-data`
suppressions on the redacted `current_url` log sites
- `internal/agent/component/invoke.go` — moved existing
`go/request-forgery` directive
- `internal/agent/sandbox/ssh.go` — moved existing
`go/command-injection` directive
- `internal/agent/tool/retrieval_service.go` — added
`go/uncontrolled-allocation-size` suppression (`topN` is bounded to 1024
above)
- `internal/cli/common_command.go` — moved 2×
`go/disabled-certificate-check` directives
- `internal/cli/user_command.go` — added `go/clear-text-logging`
suppression (filepath.Base already strips user-identifying path)
- `internal/dao/pipeline_operation_log.go` — moved 2× `go/sql-injection`
directives
- `internal/dao/user_canvas.go` — added `go/sql-injection` suppression
in `GetList` (the new `userCanvasOrderClause` call path)
- `internal/engine/infinity/chunk.go` — moved existing
`go/unsafe-quoting` directive
- `internal/entity/models/*` — moved `go/path-injection` directives (15
files)
- `internal/handler/oauth_login.go` — moved existing
`go/cookie-httponly-not-set` directive
- `internal/handler/tenant.go` — moved existing `go/path-injection`
directive
- `internal/service/deep_researcher.go` — moved existing
`go/unsafe-quoting` directive
- `internal/service/dataset.go` — added
`go/uncontrolled-allocation-size` suppression (`n` bounded to 1024
above)
- `internal/service/file.go` — moved existing `go/request-forgery`
directive
- `internal/service/langfuse.go` — moved 2× `go/request-forgery`
directives
- `internal/utility/mcp_client.go` — moved 3× `go/request-forgery`
directives
- `internal/utility/smtp.go` — moved existing `go/email-injection`
directive
- `rag/prompts/generator.py` — added
`py/clear-text-logging-sensitive-data` suppression
- `web/.../use-provider-fields.tsx` — added
`js/prototype-pollution-utility` suppression (FORBIDDEN_KEYS guard is on
the line above)

## Why the previous PR left alerts open

`// codeql[query-id] explanation` must be on the line **immediately
before** the suppressed statement per the [GitHub CodeQL suppression
spec](https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/customizing-code-scanning-with-codeql/suppressing-code-scanning-alerts).
The original suppression blocks were 4-5 lines, with the directive as
the **first** line. After linter reformat / line shifts, the directive
ended up too far above the actual alert line to be recognized. The fix
is to put the directive on the line directly above the suppressed
statement, with the rationale above it.

## Test plan

- All 9 modified Python files `ast.parse` clean
- All 4 modified Go files `gofmt` clean
- 36/44 expected alert suppressions in place
- 8 remaining CodeQL alerts are the originals (#3485851828, #3485851831,
#3485869759, #3485869766, #3485869768, #3485869771, #3485885962,
#3485895527) which were resolved by the corresponding commit comments;
these should close on the next scan when the suppression comments match
the alert lines.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
2026-06-27 20:49:06 +08:00

229 lines
7.9 KiB
Go

//
// Copyright 2026 The InfiniFlow Authors. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
package handler
import (
"errors"
"fmt"
"net/http"
"ragflow/internal/engine/redis"
"github.com/gin-gonic/gin"
"ragflow/internal/common"
"ragflow/internal/server"
"ragflow/internal/service"
"ragflow/internal/utility"
)
// oauthStateCookie is the HttpOnly cookie name that ties the in-flight
// state token to the browser that initiated the flow. The handler reads
// it back from the callback request to defend against CSRF in addition to
// the Redis-side verification.
const oauthStateCookie = "ragflow_oauth_state"
// oauthAuthCookie is the cookie the callback writes on success, so the SPA
// can pick up the signed access token after the redirect. The frontend
// reads it and either re-issues the value as an Authorization header on
// subsequent API calls or hands it off to its own token store. Not
// HttpOnly so the SPA's JS can read it.
const oauthAuthCookie = "ragflow_auth"
// OAuthLogin starts an OAuth/OIDC login flow for the configured channel.
// It generates a random state token, persists it briefly in Redis, sets a
// state cookie on the response, and redirects the browser to the channel's
// authorization URL. Mirrors Python's GET /auth/login/<channel>.
//
// @Summary Start OAuth Login
// @Tags users
// @Param channel path string true "channel name"
// @Router /api/v1/auth/login/{channel} [get]
func (h *UserHandler) OAuthLogin(c *gin.Context) {
channel := c.Param("channel")
if channel == "" {
c.JSON(http.StatusBadRequest, gin.H{
"code": common.CodeBadRequest,
"message": "channel is required",
})
return
}
init, code, err := h.userService.OAuthLoginInitiate(channel, redis.Get())
if err != nil {
// Mirror Python's oauth_login: the raised ValueError propagates to
// server_error_response, which replies HTTP 200 with code 100 and
// the exception's repr() as the message (no short error code).
if errors.Is(err, service.ErrOAuthInvalidChannel) {
c.JSON(http.StatusOK, gin.H{
"code": common.CodeExceptionError,
"data": nil,
"message": fmt.Sprintf("ValueError('Invalid channel name: %s')", channel),
})
return
}
c.JSON(http.StatusInternalServerError, gin.H{
"code": code,
"message": err.Error(),
})
return
}
setOAuthStateCookie(c, init.State, int(init.CookieMaxAge.Seconds()))
c.Redirect(http.StatusFound, init.AuthURL)
}
// OAuthCallback handles the OAuth/OIDC callback for the configured channel.
// Mirrors Python's GET /auth/oauth/<channel>/callback: it verifies the
// state, exchanges the code for an access token, fetches user info, and
// then either logs in an existing user or registers a new one. On every
// outcome it redirects the browser back to the frontend root with either
// `?auth=<user_id>` or `?error=<code>` so the SPA can show the right page.
//
// @Summary OAuth Login Callback
// @Tags users
// @Param channel path string true "channel name"
// @Param code query string true "authorization code"
// @Param state query string true "state token"
// @Router /api/v1/auth/oauth/{channel}/callback [get]
func (h *UserHandler) OAuthCallback(c *gin.Context) {
channel := c.Param("channel")
// An empty channel segment (/auth/oauth//callback) is a malformed path,
// not a real channel. Python's router never matches it and returns 404;
// match that here instead of flowing into the callback and emitting a
// bogus "Invalid channel name:" redirect.
if channel == "" {
HandleNoRoute(c)
return
}
queryCode := c.Query("code")
queryState := c.Query("state")
cookieState := readOAuthStateCookie(c)
clearOAuthStateCookie(c)
frontendBase := frontendRedirectBase()
result, _, err := h.userService.OAuthCallback(c.Request.Context(), channel, queryCode, queryState, cookieState, redis.Get())
if err != nil {
c.Redirect(http.StatusFound, frontendBase+"?error="+callbackError(channel, err))
return
}
secretKey, kerr := server.GetSecretKey(redis.Get())
if kerr != nil {
c.Redirect(http.StatusFound, frontendBase+"?error=server_error")
return
}
authToken, terr := utility.DumpAccessToken(*result.User.AccessToken, secretKey)
if terr != nil {
c.Redirect(http.StatusFound, frontendBase+"?error=server_error")
return
}
setOAuthAuthCookie(c, authToken)
c.Header("Authorization", authToken)
c.Header("Access-Control-Expose-Headers", "Authorization")
c.Redirect(http.StatusFound, frontendBase+"?auth="+result.User.ID)
}
// callbackError maps the OAuth callback errors to the `?error=` strings
// Python's oauth_callback emits. Python redirects with `?error={str(e)}`,
// so an invalid channel surfaces the full "Invalid channel name: <channel>"
// message (str of the ValueError), while the other failures use the short
// tokens Python hard-codes. The value is intentionally not URL-encoded to
// match Python's raw f-string redirect.
func callbackError(channel string, err error) string {
switch {
case errors.Is(err, service.ErrOAuthInvalidChannel):
return "Invalid channel name: " + channel
case errors.Is(err, service.ErrOAuthInvalidState):
return "invalid_state"
case errors.Is(err, service.ErrOAuthMissingCode):
return "missing_code"
case errors.Is(err, service.ErrOAuthTokenFailed):
return "token_failed"
case errors.Is(err, service.ErrOAuthEmailMissing):
return "email_missing"
case errors.Is(err, service.ErrOAuthUserInactive):
return "user_inactive"
default:
return "server_error"
}
}
// setOAuthStateCookie writes the state token as an HttpOnly cookie scoped
// to the API host. SameSite=Lax keeps the cookie attached on the top-level
// navigation that brings the user back to the callback.
func setOAuthStateCookie(c *gin.Context, state string, maxAgeSec int) {
http.SetCookie(c.Writer, &http.Cookie{
Name: oauthStateCookie,
Value: state,
Path: "/",
MaxAge: maxAgeSec,
HttpOnly: true,
SameSite: http.SameSiteLaxMode,
Secure: c.Request.TLS != nil,
})
}
func readOAuthStateCookie(c *gin.Context) string {
if cookie, err := c.Request.Cookie(oauthStateCookie); err == nil {
return cookie.Value
}
return ""
}
func clearOAuthStateCookie(c *gin.Context) {
http.SetCookie(c.Writer, &http.Cookie{
Name: oauthStateCookie,
Value: "",
Path: "/",
MaxAge: -1,
HttpOnly: true,
SameSite: http.SameSiteLaxMode,
Secure: c.Request.TLS != nil,
})
}
// setOAuthAuthCookie writes the signed access token so the SPA can pick it
// up after the redirect. Not HttpOnly so the SPA can copy it into its
// Authorization header on subsequent fetches. Lifetime mirrors the
// access-token TTL used by the rest of the app.
func setOAuthAuthCookie(c *gin.Context, token string) {
// the SPA's bootstrap credential after the OAuth redirect. The
// SPA reads it via document.cookie and copies it into the
// Authorization header. Setting HttpOnly would break the login
// flow. The token is short-lived (7 days) and signed with itsdangerous.
// codeql[go/cookie-httponly-not-set] Intentional: this cookie is
http.SetCookie(c.Writer, &http.Cookie{
Name: oauthAuthCookie,
Value: token,
Path: "/",
MaxAge: 60 * 60 * 24 * 7,
HttpOnly: false,
SameSite: http.SameSiteLaxMode,
Secure: c.Request.TLS != nil,
})
}
// frontendRedirectBase returns the URL prefix the OAuth callback should
// redirect back to. Mirrors Python's oauth_callback, which always issues
// relative "/?auth=..." / "/?error=..." redirects so the browser stays on
// the same origin that served the SPA.
func frontendRedirectBase() string {
return "/"
}