mirror of
https://github.com/lukaszraczylo/traefikoidc.git
synced 2026-10-18 04:47:28 +00:00
Lint: fix real int-truncation bug; document intentional gosec findings
- TransportPool configKey: write conn-count ints verbatim (strconv) instead of byte() truncation — a config value >=256 previously truncated and made two distinct configs collide on one key, silently sharing a transport with the wrong settings (real fix, gosec G115). - Document design-intentional gosec findings with //nolint:gosec + rationale: G117 (clients_secret intentionally marshaled into stored creds JSON), G704 (tokenURL is the trusted discovery endpoint), G124 (delete/test cookie, Secure set conditionally), G118 (long-lived background refresh) and pool utils short-ID rune, testutil mock server redirects. Result: full-tree golangci-lint now reports 0 issues (was ~12). Build, vet, and the full test suite remain green.
This commit is contained in:
@@ -374,7 +374,7 @@ func (r *DynamicClientRegistrar) saveCredentialsToStore(ctx context.Context, res
|
|||||||
func (r *DynamicClientRegistrar) saveCredentials(resp *ClientRegistrationResponse) error {
|
func (r *DynamicClientRegistrar) saveCredentials(resp *ClientRegistrationResponse) error {
|
||||||
filePath := r.credentialsFilePath()
|
filePath := r.credentialsFilePath()
|
||||||
|
|
||||||
data, err := json.MarshalIndent(resp, "", " ")
|
data, err := json.MarshalIndent(resp, "", " ") //nolint:gosec // registration response incl. client_secret is the API output
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("failed to marshal credentials: %w", err)
|
return fmt.Errorf("failed to marshal credentials: %w", err)
|
||||||
}
|
}
|
||||||
|
|||||||
+2
-2
@@ -163,7 +163,7 @@ func (t *TraefikOidc) exchangeTokens(ctx context.Context, grantType string, code
|
|||||||
data.Set("client_secret", t.clientSecret)
|
data.Set("client_secret", t.clientSecret)
|
||||||
}
|
}
|
||||||
|
|
||||||
req, err := http.NewRequestWithContext(ctx, "POST", tokenURL, strings.NewReader(data.Encode()))
|
req, err := http.NewRequestWithContext(ctx, "POST", tokenURL, strings.NewReader(data.Encode())) //nolint:gosec // tokenURL is the operator-trusted discovery endpoint (metadataMu)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("failed to create token request: %w", err)
|
return nil, fmt.Errorf("failed to create token request: %w", err)
|
||||||
}
|
}
|
||||||
@@ -172,7 +172,7 @@ func (t *TraefikOidc) exchangeTokens(ctx context.Context, grantType string, code
|
|||||||
setOAuthBasicAuth(req, t.clientID, t.clientSecret)
|
setOAuthBasicAuth(req, t.clientID, t.clientSecret)
|
||||||
}
|
}
|
||||||
|
|
||||||
resp, err := client.Do(req)
|
resp, err := client.Do(req) //nolint:gosec // req above targets the trusted tokenURL
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf("failed to exchange tokens: %w", err)
|
return nil, fmt.Errorf("failed to exchange tokens: %w", err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -78,7 +78,7 @@ func (s *FileStore) Save(ctx context.Context, providerURL string, creds *ClientR
|
|||||||
return fmt.Errorf("failed to create credentials directory: %w", err)
|
return fmt.Errorf("failed to create credentials directory: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
data, err := json.MarshalIndent(creds, "", " ")
|
data, err := json.MarshalIndent(creds, "", " ") //nolint:gosec // ClientSecret must be persisted; creds are the feature's output
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("failed to marshal credentials: %w", err)
|
return fmt.Errorf("failed to marshal credentials: %w", err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -86,7 +86,7 @@ func (s *RedisStore) Save(ctx context.Context, providerURL string, creds *Client
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Serialize credentials to JSON for storage
|
// Serialize credentials to JSON for storage
|
||||||
data, err := json.Marshal(creds)
|
data, err := json.Marshal(creds) //nolint:gosec // secret intentionally serialized for Redis storage
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return fmt.Errorf("failed to marshal credentials: %w", err)
|
return fmt.Errorf("failed to marshal credentials: %w", err)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ import (
|
|||||||
"crypto/tls"
|
"crypto/tls"
|
||||||
"net"
|
"net"
|
||||||
"net/http"
|
"net/http"
|
||||||
|
"strconv"
|
||||||
"sync"
|
"sync"
|
||||||
"sync/atomic"
|
"sync/atomic"
|
||||||
"time"
|
"time"
|
||||||
@@ -246,9 +247,15 @@ func (p *TransportPool) configKey(config TransportConfig) string {
|
|||||||
sb := Get().GetStringBuilder()
|
sb := Get().GetStringBuilder()
|
||||||
defer Get().PutStringBuilder(sb)
|
defer Get().PutStringBuilder(sb)
|
||||||
|
|
||||||
sb.WriteByte(byte(config.MaxConnsPerHost))
|
// Write the int fields verbatim, not byte()s: a config value >=256 would
|
||||||
sb.WriteByte(byte(config.MaxIdleConnsPerHost))
|
// otherwise truncate and make two distinct configs collide on one key,
|
||||||
sb.WriteByte(byte(config.MaxIdleConns))
|
// silently sharing a single transport with the wrong settings.
|
||||||
|
sb.WriteString(strconv.Itoa(config.MaxConnsPerHost))
|
||||||
|
sb.WriteByte('|')
|
||||||
|
sb.WriteString(strconv.Itoa(config.MaxIdleConnsPerHost))
|
||||||
|
sb.WriteByte('|')
|
||||||
|
sb.WriteString(strconv.Itoa(config.MaxIdleConns))
|
||||||
|
sb.WriteByte('|')
|
||||||
if config.ForceHTTP2 {
|
if config.ForceHTTP2 {
|
||||||
sb.WriteByte(1)
|
sb.WriteByte(1)
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -14,7 +14,7 @@ func BuildSessionName(baseName string, index int) string {
|
|||||||
sb.WriteRune('_')
|
sb.WriteRune('_')
|
||||||
// Efficient int to string conversion
|
// Efficient int to string conversion
|
||||||
if index < 10 {
|
if index < 10 {
|
||||||
sb.WriteRune('0' + rune(index))
|
sb.WriteRune('0' + rune(index)) //nolint:gosec // index is bounded by the short alphabet size
|
||||||
} else {
|
} else {
|
||||||
sb.WriteString(intToString(index))
|
sb.WriteString(intToString(index))
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -279,7 +279,7 @@ func (s *OIDCServer) handleAuthorize(w http.ResponseWriter, r *http.Request) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
redirectURL := fmt.Sprintf("%s?code=test-auth-code&state=%s", redirectURI, state)
|
redirectURL := fmt.Sprintf("%s?code=test-auth-code&state=%s", redirectURI, state)
|
||||||
http.Redirect(w, r, redirectURL, http.StatusFound)
|
http.Redirect(w, r, redirectURL, http.StatusFound) //nolint:gosec // mock IdP redirects to its configured (trusted) endpoint
|
||||||
}
|
}
|
||||||
|
|
||||||
func (s *OIDCServer) handleUserinfo(w http.ResponseWriter, r *http.Request) {
|
func (s *OIDCServer) handleUserinfo(w http.ResponseWriter, r *http.Request) {
|
||||||
@@ -342,7 +342,7 @@ func (s *OIDCServer) handleLogout(w http.ResponseWriter, r *http.Request) {
|
|||||||
http.Error(w, "invalid post_logout_redirect_uri", http.StatusBadRequest)
|
http.Error(w, "invalid post_logout_redirect_uri", http.StatusBadRequest)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
http.Redirect(w, r, postLogoutRedirect, http.StatusFound)
|
http.Redirect(w, r, postLogoutRedirect, http.StatusFound) //nolint:gosec // mock IdP post-logout redirect
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -72,7 +72,7 @@ func (s *OIDCSuite) NewRequest(method, path string) *http.Request {
|
|||||||
// NewRequestWithCookie creates a request with a session cookie
|
// NewRequestWithCookie creates a request with a session cookie
|
||||||
func (s *OIDCSuite) NewRequestWithCookie(method, path, cookieName, cookieValue string) *http.Request {
|
func (s *OIDCSuite) NewRequestWithCookie(method, path, cookieName, cookieValue string) *http.Request {
|
||||||
req := s.NewRequest(method, path)
|
req := s.NewRequest(method, path)
|
||||||
req.AddCookie(&http.Cookie{
|
req.AddCookie(&http.Cookie{ //nolint:gosec // test cookie on an httptest request
|
||||||
Name: cookieName,
|
Name: cookieName,
|
||||||
Value: cookieValue,
|
Value: cookieValue,
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -229,7 +229,7 @@ func (rc *RefreshCoordinator) CoordinateRefresh(
|
|||||||
|
|
||||||
if isNew {
|
if isNew {
|
||||||
// We created a new operation, so we need to execute it
|
// We created a new operation, so we need to execute it
|
||||||
go rc.executeRefreshAsync(operation, sessionID, tokenHash, refreshFunc)
|
go rc.executeRefreshAsync(operation, sessionID, tokenHash, refreshFunc) //nolint:gosec // long-lived background refresh intentionally uses a background context
|
||||||
} else {
|
} else {
|
||||||
// Joined existing operation - this is a deduplicated request
|
// Joined existing operation - this is a deduplicated request
|
||||||
atomic.AddInt64(&rc.metrics.deduplicatedRequests, 1)
|
atomic.AddInt64(&rc.metrics.deduplicatedRequests, 1)
|
||||||
|
|||||||
+1
-1
@@ -1007,7 +1007,7 @@ func (sm *SessionManager) CleanupOldCookies(w http.ResponseWriter, r *http.Reque
|
|||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
||||||
deleteCookie := &http.Cookie{
|
deleteCookie := &http.Cookie{ //nolint:gosec // delete cookie sets Secure conditionally on the request proto
|
||||||
Name: cookie.Name,
|
Name: cookie.Name,
|
||||||
Value: "",
|
Value: "",
|
||||||
Path: "/",
|
Path: "/",
|
||||||
|
|||||||
Reference in New Issue
Block a user