fix: make IPC CORS headers agree with the request guard - #91
Open
ZK-Andy wants to merge 1 commit into
Open
Conversation
- share one IsAllowedIpcOrigin predicate between IsAuthorized and BuildCorsHeaders, so the response can no longer deny an origin the request guard accepted (a page that moved to a different loopback port mid-session was authorized but got no Access-Control-Allow-Origin, and the browser dropped every window.__ryn.invoke) - send the same CORS headers on /ipc/eval/ responses, which returned none and left a CORS error in DevTools for a request the server had accepted - emit Vary: Origin whenever the request carries an Origin, since the allow-origin decision is origin-dependent either way
9 tasks
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.
Fixes #90.
Problem
The IPC request guard and the IPC CORS headers decided differently:
IsAuthorizedaccepts the configured cross-origin or any loopback origin;BuildCorsHeadersechoedAccess-Control-Allow-Originonly for the singleallowedCorsOriginstring captured fromRynOptions.Urlwhen the window was created.So when the page's loopback origin changes during a session (dev server restarted on another port, crash-restart port fallback), the request is authorized server-side but the browser drops it at the CORS preflight — the host never sees it and nothing is logged.
HandleIpcEvalAsynchad the same mismatch in a simpler form: it returned no CORS headers at all.Change
IsAllowedIpcOrigin(origin)predicate (configured origin ∪ any loopback origin) and use it from bothIsAuthorizedandBuildCorsHeaders, so the two can no longer drift apart./ipc/eval/responses now carry the same CORS headers (503/403/400/200 paths) instead of none.Vary: Originwhenever the request carries anOrigin— the allow-origin decision is origin-dependent either way, so responses must not be cached across origins.Security note: this does not widen the boundary. The per-launch
X-Ryn-Tokenremains the authorization gate (constant-time compared); CORS only lets the browser deliver a response for a request the server already accepted. A cross-site caller still cannot read anything without the token, and non-loopback origins are still rejected byIsAuthorizedand receive noAccess-Control-Allow-Origin.Tests
New
tests/Ryn.Core.Tests/LocalWebServerCorsTests.cs(raw socket, matching the existingLocalWebServerRawSocketTestsstyle) asserts, againstLocalWebServer(contentDirectory: null, allowedCorsOrigin: "http://127.0.0.1:31000"):Access-Control-Allow-Origin+Vary: Origin;Access-Control-Allow-Origin, and the command is still403;/ipc/cmd/and/ipc/eval/response from the second loopback origin carriesAccess-Control-Allow-Origin.Local result:
Ryn.Core.Tests→ 202 passed, 2 skipped, 5 failed. The 5 failures are allWindowStatePersistenceTests, which persist underLocalApplicationData; that path is a read-only filesystem in my sandbox (mkdir: read-only file system), soSavesilently fails andLoadreturns null. They are unrelated to this change and fail the same way on a pristine checkout here; the new CORS tests pass 5/5 (plus the eval case).Notes for review
Access-Control-Allow-Originin static/local-server mode (previously no CORS headers were emitted at all whenallowedCorsOriginwas null). That is the same consistency fix: authorization already accepted such origins there. Same-origin pages send noOrigin, so their responses are unchanged.