Repository navigation
Refactor/server hardening - #26
Conversation
- Enforce connSem limit on WebSocket connections (bypassed maxConns) - Fix data race in authRateLimiter.isRateLimited (copy fields under lock) - Use RLock fast path in sessionStore.validate - Clean up per-tunnel Prometheus labels on unregister (cardinality leak) - Fix potential deadlock in CleanupStale (lock ordering vs RegisterWithIP) - Replace time.After with time.NewTimer in tunnel Send (timer leak) - Cache tunnelTypeStr to avoid lock contention on hot path - Use atomic connID sequence to prevent sync.Map key collision - Add http.Flusher to httpResponseWriter - Unify TCP socket buffer sizes (256KB → 512KB) - Extract utils.ContainsIgnoreCase, deduplicate IsTransportAllowed - Hoist reserved subdomain map to package-level var - Rename ambiguous bufioWriterPool → httpRespWriterPool - Remove unused: scalerLoop, ensureSessions, handleProxyLogin, cleanupShutdownOnce, initCleanupShutdown, sessionIdx, errorAfterConn - Fix staticcheck: redundant type assertion, single-case select, redundant CanonicalHeaderKey
- Enforce connSem limit on WebSocket connections (bypassed maxConns) - Fix data race in authRateLimiter.isRateLimited (copy fields under lock) - Use RLock fast path in sessionStore.validate - Clean up per-tunnel Prometheus labels on unregister (cardinality leak) - Fix potential deadlock in CleanupStale (lock ordering vs RegisterWithIP) - Replace time.After with time.NewTimer in tunnel Send (timer leak) - Cache tunnelTypeStr to avoid lock contention on hot path - Use atomic connID sequence to prevent sync.Map key collision - Add http.Flusher to httpResponseWriter - Unify TCP socket buffer sizes (256KB → 512KB) - Extract utils.ContainsIgnoreCase, deduplicate IsTransportAllowed - Hoist reserved subdomain map to package-level var - Rename ambiguous bufioWriterPool → httpRespWriterPool - Remove unused: scalerLoop, ensureSessions, handleProxyLogin, cleanupShutdownOnce, initCleanupShutdown, sessionIdx, errorAfterConn - Fix staticcheck: redundant type assertion, single-case select, redundant CanonicalHeaderKey
- Added --skip-local-tls-verify option to HTTP, HTTPS and TCP tunnel commands - Automatically skip TLS verification for loopback addresses to improve user experience - Update Go version from 1.25 to 1.26.4 and upgrade related dependency packages - Added CI workflow configuration files including testing, static checking and security scanning - Fixed the case error of main.Version variable name in Dockerfile - Improved WebSocket connection handling and HTTP request forwarding logic - Remove unnecessary path traversal protection checks (handled by filepath.Clean) - Add code comments to mark security-related configurations to avoid false positives
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 16d2cfe631
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| select { | ||
| case ch <- streamResult{s, err}: | ||
| case <-ctx.Done(): |
There was a problem hiding this comment.
Close streams after open-stream timeout
When the 3s timeout fires and OpenStream returns just afterward with a newly opened stream, both this buffered-channel send and ctx.Done() are ready, so select may choose the send case. At that point the caller has already returned and nobody drains ch, leaving the yamux stream open but unowned; repeated slow opens can leak streams/resources on the tunnel. The old timeout path drained the result and closed any late stream.
Useful? React with 👍 / 👎.
| for i := len(parts) - 1; i >= 0; i-- { | ||
| ip := strings.TrimSpace(parts[i]) | ||
| if ip != "" && net.ParseIP(ip) != nil { | ||
| if !IsPrivateIP(ip) { | ||
| return ip |
There was a problem hiding this comment.
Preserve the original client IP from X-Forwarded-For
For deployments where the server is behind a private load balancer and traffic passes through a public proxy/CDN, X-Forwarded-For commonly contains client, public-proxy; walking from the right returns the proxy IP instead of the original client. Since ExtractClientIP feeds IP allow/deny checks and auth rate limiting, all users behind that proxy can be treated as the same IP and client-specific ACLs stop matching.
Useful? React with 👍 / 👎.
…rameter passing Split openStreamWithTimeout into an openStream function with configurable timeout, Unify timeout parameter management and remove fixed-size channel buffers to simplify logic. fix(netutil): Fix X-Forwarded-For header parsing logic Adjust the X-Forwarded-For header parsing strategy from right to left traversal to left to right. Keep the original public client IP address and trust the edge proxy to sanitize the headers.
No description provided.