Skip to content

Refactor/server hardening - #26

Merged
Gouryella merged 4 commits into
mainfrom
refactor/server-hardening
Jun 5, 2026
Merged

Gouryella merged 4 commits into
mainfrom
refactor/server-hardening

Conversation

@Gouryella

Copy link
Copy Markdown
Owner

No description provided.

- 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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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".

Comment on lines +366 to +368
select {
case ch <- streamResult{s, err}:
case <-ctx.Done():

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

Comment thread internal/shared/netutil/ip.go Outdated
Comment on lines +67 to +71
for i := len(parts) - 1; i >= 0; i-- {
ip := strings.TrimSpace(parts[i])
if ip != "" && net.ParseIP(ip) != nil {
if !IsPrivateIP(ip) {
return ip

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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.
@Gouryella
Gouryella merged commit 712f28f into main Jun 5, 2026
1 check passed
@Gouryella
Gouryella deleted the refactor/server-hardening branch June 5, 2026 04:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant