fix(extension): prevent silent 431 wedge from localhost cookie overflow (fixes #2278) - #2282
Open
kagura-agent wants to merge 1 commit into
Open
Conversation
…ow (fixes jackwener#2278) The daemon /ping preflight silently swallows non-OK responses, so a large localhost cookie jar (>16KB) pushes the request past Node's default header limit, the daemon answers 431, and the extension never attempts the WebSocket connection — with no diagnostic anywhere. - omit credentials on the /ping fetch so the browser does not attach the localhost cookie jar in the first place - log the HTTP status (and the error on failure) instead of dropping them - raise the daemon's --max-http-header-size to 128KB so the daemon can still serve endpoints even when a large cookie jar is attached
Contributor
Author
|
Hi @jackwener 👋 — gentle ping. This fix for #2278 (silent 431 wedge from localhost cookie overflow) has been open for 3 days with all CI green. Would appreciate a review when you have a moment. Happy to adjust anything. |
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
1 participant
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.
Problem
Fixes #2278 — the extension silently never connects when the daemon's
/pingreturns HTTP 431.Root cause: the
localhostcookie jar (17 KB) exceeds Node's default 16 KB HTTP header limit. WhenconnectAttempt()pings the daemon, the browser attaches the full cookie jar (defaultsame-origincredentials), the daemon answers431 Request Header Fields Too Large, and the!res.okbranch silentlyscheduleReconnect()s — the WebSocket is never attempted, and nothing is logged, so the wedge is invisible.Fix
extension/src/background.ts) —connectAttempt()pings withcredentials: 'omit'so the browser no longer attaches the cookie jar to the ping, and a non-OK status now logs[opencli] daemon ping failed: HTTP <status>instead of silently swallowing it.src/browser/daemon-lifecycle.ts) —resolveDaemonLaunchSpec()prefixes--max-http-header-size=131072(128 KB) so the daemon tolerates large headers on all HTTP endpoints (including/statusand the WebSocket upgrade, which can't omit cookies).Tests
extension/src/background.test.ts— new test asserting the ping usescredentials: 'omit', logs a non-OKHTTP 431, and does not open a WebSocket after a failed ping.src/browser/daemon-lifecycle.test.ts— new test asserting the header-size flag precedes the entrypoint.npx vitest run extension/src/background.test.ts src/browser/daemon-lifecycle.test.ts→ 73/73 pass.