Skip to content

fix(auth): recognise the common proxy scheme headers for Secure cookies - #492

Open
IBazsecI wants to merge 1 commit into
vxcontrol:mainfrom
IBazsecI:fix/cookie-secure-proxy-headers
Open

IBazsecI wants to merge 1 commit into
vxcontrol:mainfrom
IBazsecI:fix/cookie-secure-proxy-headers

Conversation

@IBazsecI

@IBazsecI IBazsecI commented Nov 14, 2026 •

Copy link
Copy Markdown

Description of the Change

Problem

When a reverse proxy terminates TLS and forwards to PentAGI over plain HTTP (SERVER_USE_SSL=false), isHTTPS() in backend/pkg/server/services/cookies.go marks the session and OAuth cookies Secure only if the request carries exactly X-Forwarded-Proto: https. Other common forms produce a cookie without Secure:

  • Forwarded: proto=https (RFC 7239)
  • X-Forwarded-Ssl: on
  • X-Forwarded-Proto: HTTPS (upper case)
  • X-Forwarded-Proto: https, http (appended by chained proxies)

In those setups the session cookie can be sent over plain HTTP. Google login also fails, because its state/nonce cookies are SameSite=None (auth.go:371-376), and browsers reject a SameSite=None cookie that is not Secure.

The stock docker-compose.yml is not affected, because there the server terminates TLS itself (SERVER_USE_SSL=true plus the certificate generated by entrypoint.sh).

Solution

  • isHTTPS() also accepts X-Forwarded-Ssl: on and proto=https from Forwarded, compares case-insensitively, and reads only the client-facing (first) element of a chained X-Forwarded-Proto / Forwarded.
  • The change can only turn Secure on, so no existing deployment changes behaviour. The headers stay honoured regardless of TRUSTED_PROXIES, as X-Forwarded-Proto is today: a forged header can only mark the sender's own cookie Secure.
  • backend/docs/config.md: a short note next to TRUSTED_PROXIES on which header the proxy must send (with the nginx line) and that HSTS belongs at the proxy.

I deliberately did not derive Secure from PUBLIC_URL or force it on. That would break login for plain-HTTP deployments that kept the .env.example default PUBLIC_URL=https://localhost:8443. If you'd like an opt-in COOKIE_SECURE switch or HSTS emission, I'm happy to send that as a follow-up.

Type of Change

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • 📚 Documentation update
  • 🛡️ Security update

Areas Affected

  • Core Services (Frontend UI/Backend API)
  • Documentation

Testing and Verification

Test Configuration

PentAGI Version: main @ cffed94
Docker Version: n/a (unit tests only)
Host OS: Ubuntu 24.04, Go 1.27.4
LLM Provider: n/a
Enabled Features: n/a

Test Steps

  1. go test -count=1 -run TestCookies_ -v ./pkg/server/services/
  2. Mutation check: run the same tests with the original cookies.go restored in a throwaway copy of the tree.
  3. go vet ./pkg/server/services/, go test -count=1 -race ./pkg/server/... and golangci-lint run ./pkg/server/services/

Test Results

The new TestCookies_SessionOptions_ReadsTheSchemeFromEveryCommonProxyHeader drives the real AuthLogin handler via httptest. It has 13 rows, 6 of them negative (http, http, https, X-Forwarded-Ssl: off, proto=https only on an inner hop, Forwarded without proto, no headers).

  • With the patch, all rows pass and the existing TestCookies_* tests still pass.
  • With the original cookies.go, exactly the 5 newly recognised forms fail and the negatives pass:
    --- FAIL: .../x-forwarded-proto_upper_case
    --- FAIL: .../x-forwarded-proto_chain_keeps_the_client-facing_hop
    --- FAIL: .../x-forwarded-ssl_on
    --- FAIL: .../forwarded_proto_https
    --- FAIL: .../forwarded_quoted_proto
    
  • go vet is clean. go test -race ./pkg/server/... passes for all packages.

golangci-lint run ./pkg/server/services/ (v2.12.2 on Go 1.26.5, as pinned in CI) reports 0 issues.

Security Considerations

This is hardening. The cookie Secure flag is now set in more proxy configurations and never unset in any configuration. There are no new dependencies, permissions or config fields.

Performance Impact

Negligible: a few string operations per cookie write.

Documentation Updates

  • Configuration documentation updates

Deployment Notes

None. Operators whose proxy sends none of these headers still need to add one; the new doc note says how.

Checklist

Code Quality

  • My code follows the project's coding standards
  • I have added/updated necessary documentation
  • I have added tests to cover my changes
  • All new and existing tests pass
  • I have run go fmt and go vet (for Go code)

Security

  • I have considered security implications
  • Changes maintain or improve the security model

Compatibility

  • Changes are backward compatible

Documentation

  • Documentation is clear and complete

Behind a TLS-terminating reverse proxy the session and OAuth cookies were
marked Secure only when the proxy sent exactly "X-Forwarded-Proto: https".
"Forwarded: proto=https" (RFC 7239), "X-Forwarded-Ssl: on", an upper-case
value or a chained "https, http" all produced a cookie without Secure, and
Google login then failed because browsers reject SameSite=None without
Secure.

isHTTPS now also accepts these forms, reading only the client-facing hop
of a chained header. It can only turn Secure on, so no existing
deployment changes behaviour. The reverse-proxy note in config.md says
which header the proxy must send and that HSTS belongs at the proxy.
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