Repository navigation
[Bugs] Editor proxy: harden middleware matcher - #1156
Conversation
The matcher skipped any path ending in a file extension. It now runs on every path except Next's own assets, app/favicon.ico and the files served from public/, listed by folder. Tests check the matcher with Next's unstable_doesMiddlewareMatch for API, project and locale-prefixed paths and for real static assets.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
Maintainer triage: the matcher hardening looks right to me. Static files are now excluded by location instead of by extension, and the new matcher tests cover both the gated and the excluded sides.
One thing to watch: the folder list now has to match public/ by hand, so adding a new public folder without updating it will gate those assets behind login on password-protected hosts. That fails closed, which is the safe direction.
Since this changes what the self-host auth gate covers, it needs Security's approval before merge, per the fleet rule. I'm leaving this as a comment rather than an approval; it can merge once CI is green and Security approves.
What does this PR do?
Hardens the matcher for the self-host proxy in
apps/editor/proxy.ts._next/plus any path ending in a file extension. The new one runs on every path except Next's own assets (_next/),app/favicon.icoand the files served frompublic/. Those files are listed by location: the asset folders (audios,demos,fonts,hdri,icons,items,material) and the top-level.svgfiles. A code comment notes that the list has to trackpublic/./api/*and/project/*path, including locale-prefixed ones, now goes throughgateRequest. The excluded set is limited to known static asset locations.apps/editor/proxy.test.tscheckconfig.matcherwith Next'sunstable_doesMiddlewareMatch(fromnext/experimental/testing/server):/project/a.json,/api/x.json,/fr/adminand/en/api/x, among others. The test also checks thatgateRequestanswers each of these on a password-protected host._next/static,_next/image,favicon.ico, a top-level SVG and one real file from eachpublic/folder.How to test
cd apps/editor && bun test proxy.test.ts: all 9 tests pass. Withmain'sproxy.ts, the new "runs on every page and API path" test fails at/project/a.json.PASCAL_ADMIN_PASSWORD=pw bunx next dev -p 3111inapps/editorand request pages with a non-loopbackHostheader and no session./project/<id>and/project/a.jsonboth redirect to/login./api/plugins/mint/x.jsonanswers 401./pascal-logo-shape.svg,/items/round-carpet/model.glb,/favicon.icoand/loginanswer 200.turbo run check-types --filter=editor....Screenshots / screen recording
N/A — non-visual change
Checklist
bun devbun checkto verify)mainbranchNote
Medium Risk
Changes which requests hit auth middleware; misaligned
public/exclusions could block static assets or leave routes ungated, but behavior is covered by new matcher tests.Reviewed by Cursor Bugbot for commit e5e060b. Bugbot is set up for automated code reviews on this repo. Configure here.