test(server): cover desktop-origin CORS on environment descriptor GET - #7248
test(server): cover desktop-origin CORS on environment descriptor GET#7248matsvarn wants to merge 1 commit into
Conversation
Packaged nightly pingdotgg#7102 reported missing ACAO on GET /.well-known/t3/environment while OPTIONS still sent it. Current main already returns ACAO on that GET, including from t3code://app with Accept-Encoding. Lock GET and OPTIONS on the real descriptor route so a middleware regression is red without re-implementing pingdotgg#2594. Co-authored-by: Mats Varnskühler <imMxts@users.noreply.github.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Closing this. It only added tests for CORS behavior that already works on current |
ApprovabilityVerdict: Approved 0db7dff This PR adds two test cases for desktop-origin CORS behavior on the environment descriptor endpoint. It modifies only test code with no production changes, following existing test patterns in the file. You can customize Macroscope's approvability policy. Learn more. |
What Changed
Adds two tests on the real
makeRoutesLayerdescriptor route:GET /.well-known/t3/environmentfromt3code://appwith ChromiumAccept-EncodingOPTIONSpreflight from that originNo production code change. Packaged CORS stays wildcard ACAO without credentials.
Why
#7102 reported that a packaged nightly returned 200 JSON for the environment descriptor without
Access-Control-Allow-Originon the GET, while OPTIONS 204 included it. Desktop pairing to remote environments then failed in Chromium.On current
mainthe packaged server already returns ACAO on that GET. The existing test used a generic origin and noAccept-Encoding, so it did not lock the desktop Chromium request shape. These tests do. Re-implementing #2594 would duplicate a fix that is already present.Fixes #7102
Validation
vp test run apps/server/src/server.test.ts -t "includes CORS headers on desktop-origin"vp test run apps/server/src/server.test.ts -t "includes CORS headers on public environment descriptor"vp fmt/vp lintonapps/server/src/server.test.tsUI Changes
None.
Checklist
Note
Low Risk
Test-only changes in
server.test.tswith no runtime behavior modifications.Overview
Adds test-only coverage so desktop Chromium cannot regress on CORS for
/.well-known/t3/environment.Two integration tests hit the real server routes: a GET with origin
t3code://appandAccept-Encoding: gzip, deflate, br(matching packaged desktop), asserting 200, JSON body, and browser API CORS response headers; and an OPTIONS preflight from the same origin withaccess-control-request-method/access-control-request-headers, asserting 204 and preflight CORS headers.No production or CORS policy changes—the prior public-descriptor CORS test used a generic origin and omitted
Accept-Encoding, so it did not pin the desktop request shape reported in #7102.Reviewed by Cursor Bugbot for commit 0db7dff. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add CORS tests for desktop-origin requests to the environment descriptor endpoint
Adds two tests in server.test.ts to verify CORS behavior for requests from
t3code://appto/.well-known/t3/environment: one for a GET request (asserts 200, correct body, and CORS response headers) and one for an OPTIONS preflight (asserts 204 and preflight headers).Macroscope summarized 0db7dff.