Repository navigation
fix: reject cross-origin websocket upgrades before session lookup - #24
Merged
Merged
Conversation
Что: - Добавлена общая проверка Origin для всех WebSocket-серверов панели (apps/dokploy/server/wss/origin.ts), подключена через verifyClient в terminal, docker-container-terminal, docker-container-logs, docker-stats, listen-deployment и drawer-logs. - Upgrade принимается, если Origin отсутствует (не браузерные клиенты с x-api-key), совпадает с Host запроса (host:port без учета схемы) или входит в trustedOrigins better-auth (resolveTrustedOriginsForAuthRequest: https://<домен панели>, http://<IP сервера>:3000, dev-origins). Иначе ответ 403 до события connection. - Путь Next.js HMR не изменен. Зачем: - Cookie сессии SameSite=Lax, поэтому браузер прикладывает ее к WebSocket-рукопожатию со страниц на соседних поддоменах домена панели. Приложение, развернутое на таком поддомене, могло открыть терминал или поток логов от имени вошедшего пользователя. Риски: - Если прокси перед панелью переписывает Host и домен панели не указан в настройках веб-сервера, браузерные WebSocket получат 403; нужно указать домен панели в настройках. - Wildcard и tenant trustedOrigins (только для /sso/register) не учитываются. Проверки: - Команды и результаты: CI=true bunx vitest run --config __test__/vitest.config.ts __test__/wss/origin.test.ts - 29 passed; то же для __test__/wss, cluster-router-security и domain-validation - 12 файлов, 127 passed; bun run typecheck - exit 0; bunx biome check по измененным файлам - 0 ошибок, 5 старых предупреждений useOptionalChain не в измененных строках. Новые handler-тесты на старых обработчиках из origin/AgentHits-Dev - 6 failed (upgrade принимался и шел до проверки сессии). - Ограничения: не проверялось вручную в браузере и за реальным Traefik; полный набор тестов не запускался. What: - Added a shared Origin check for every panel WebSocket server (apps/dokploy/server/wss/origin.ts), wired through verifyClient in terminal, docker-container-terminal, docker-container-logs, docker-stats, listen-deployment and drawer-logs. - The upgrade is accepted when Origin is absent (non-browser x-api-key clients), matches the request Host (scheme-insensitive host:port) or is one of better-auth's trustedOrigins (resolveTrustedOriginsForAuthRequest: https://<panel domain>, http://<server IP>:3000, dev origins). Otherwise it gets a 403 before the connection event. - The Next.js HMR path is unchanged. Why: - The session cookie is SameSite=Lax, so browsers attach it to WebSocket handshakes from pages on sibling subdomains of the panel domain. An app deployed on such a subdomain could open a terminal or log stream as the logged-in user. Risks: - If a proxy in front of the panel rewrites Host and the panel domain is not set in the web server settings, browser WebSockets get 403; the panel domain must be configured there. - Wildcard and tenant trustedOrigins (used only for /sso/register) are not honored. Checks: - Commands and results: CI=true bunx vitest run --config __test__/vitest.config.ts __test__/wss/origin.test.ts - 29 passed; same for __test__/wss, cluster-router-security and domain-validation - 12 files, 127 passed; bun run typecheck - exit 0; bunx biome check on changed files - 0 errors, 5 pre-existing useOptionalChain warnings outside changed lines. New handler tests against the old handlers from origin/AgentHits-Dev - 6 failed (upgrade accepted and reached the session check). - Limitations: not checked manually in a browser or behind a real Traefik; full test suite not run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is this PR about?
WebSocket upgrades (
/terminal,/docker-container-terminal,/docker-container-logs,/listen-docker-stats-monitoring,/listen-deployment,/drawer-logs) were authenticated only by the session cookie and never looked atOrigin. The cookie isSameSite=Lax, so a browser attaches it to a WebSocket handshake started by a page on a sibling subdomain of the panel domain. An app deployed on such a subdomain could open a host terminal, a container shell or a log stream with the visiting user's session.This PR adds one shared check,
apps/dokploy/server/wss/origin.ts, and wires it into every panelWebSocketServerviaverifyClient. It runs insidehandleUpgrade, before theconnectionevent, so a rejected handshake gets an HTTP 403 with no session lookup and no side effects.The upgrade is accepted when:
Originis absent. Browsers always send it on WebSocket handshakes. The only clients that leave it out are non-browser clients (x-api-key scripts, Nodews, wscat), and those cannot ride a browser cookie anyway. RequiringOriginwould break those API-key clients.Originmatches the requestHost, comparing host:port without the scheme (the panel often sits behind TLS-terminating Traefik withpassHostHeader: true).Originis exactly one of better-auth'strustedOriginsfor regular auth requests (resolveTrustedOriginsForAuthRequest():https://<panel host>,http://<server IP>:3000, dev origins). This list is only read when the Host check fails.null, empty and non-http(s) origins are rejected, and so is the same IP on another port. The Next.js HMR early return is unchanged.Checks
CI=true bunx vitest run --config __test__/vitest.config.ts __test__/wss/origin.test.ts: 29 passed. The file covers same host, configured trusted origin, sibling subdomain, foreign origin, another port, opaque origins, absent origin and fail-closed behavior. It also has handler-level tests for all six servers: a sibling origin gets a 403 andvalidateRequestis never called, and the panel origin still reaches the session check.__test__/wss+cluster/cluster-router-security+domains/domain-validation: 12 files, 127 passed.bun run typecheck: exit 0.bunx biome checkon changed files: 0 errors. The 5useOptionalChainwarnings were already there and are not on changed lines.Rollout notes
Host(for example a tunnel forwarding tolocalhost:3000), browser WebSockets will now get 403 unless the panel domain is set as the web server host in settings, which puts it in trusted origins.🤖 Generated with Claude Code