Repository navigation
feat: send the client's token with the overlay's open-editor request - #2488
alexander-akait wants to merge 3 commits into
Conversation
So a server can tell the overlay's request from one another page made. Only to an endpoint on the page's own server, and only when the client was given a token. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
🦋 Changeset detectedLatest commit: e1e7b49 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe client passes its connection token to the editor overlay. For rooted same-server endpoints, the overlay includes an available token when it builds an editor-opening URL. The new utility encodes file references, preserves URL components, and excludes tokens for other endpoint forms. Unit and end-to-end tests cover URL generation and token forwarding. Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely describes the main change: sending the client token with the overlay's open-editor request. |
| Docstring Coverage | |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 7 files. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
1a2dd696-152b-431e-bff9-d351cf0b5a46
📒 Files selected for processing: 9
.changeset/overlay-open-editor-token.md.cspell.jsonclient-src/index.jsclient-src/overlay.jsclient-src/utils/open-editor-url.jstest/e2e/overlay.test.jstest/open-editor-url.test.jstypes/client/overlay.d.tstypes/client/utils/open-editor-url.d.ts
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
A browser reads `/\host` as protocol-relative, like `//host`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Place fileName before the endpoint fragment. · open-editor-url.js:15-19
client-src/utils/open-editor-url.js:15-19
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPlace
fileNamebefore the endpoint fragment.When a rooted
openEditorEndpointcontains an existing query or fragment, the current template appends?fileName=...after it. The documented/__open-editorroute then does not receivereq.query.fileName, so it can reject the request instead of opening the selected file. InsertfileNameinto the query before the fragment. Keep the existingisOwnServercheck so token delivery remains limited to rooted same-server paths.Suggested fix
export default function openEditorUrl(endpoint, reference, token) { - const url = `${endpoint}?fileName=${encodeURIComponent(reference)}`; + const hashIndex = endpoint.indexOf("#"); + const fragment = hashIndex === -1 ? "" : endpoint.slice(hashIndex); + const withoutFragment = + hashIndex === -1 ? endpoint : endpoint.slice(0, hashIndex); + const separator = withoutFragment.includes("?") ? "&" : "?"; + const url = `${withoutFragment}${separator}fileName=${encodeURIComponent( + reference, + )}${fragment}`; const isOwnServer = /^\/(?![/\\])/.test(endpoint);
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
ea94ced5-d6e9-4235-b6fa-676ff94b42b8
📒 Files selected for processing: 3
client-src/utils/open-editor-url.jstest/open-editor-url.test.jstypes/client/utils/open-editor-url.d.ts
🚧 Files skipped from review as they are similar to previous changes: 3
- client-src/utils/open-editor-url.js
- test/open-editor-url.test.js
- types/client/utils/open-editor-url.d.ts
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.
…s fragment Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjuMAuk9o6UazjHzcAQCTA
|
@coderabbitai Fixed the outside-diff finding ( Generated by Claude Code |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
fd '^open-editor-url(\.test)?\.js$'
ast-grep outline client-src/utils/open-editor-url.js
ast-grep outline test/open-editor-url.test.js
sed -n '1,180p' client-src/utils/open-editor-url.js
sed -n '1,220p' test/open-editor-url.test.jsLength of output: 3333
The regression test covers an existing query and fragment, including ✏️ Learnings added
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! You are interacting with an AI system. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2488 +/- ##
==========================================
+ Coverage 96.63% 96.68% +0.05%
==========================================
Files 24 25 +1
Lines 2644 2655 +11
==========================================
+ Hits 2555 2567 +12
+ Misses 89 88 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
The overlay now sends the client's token along when it asks
openEditorEndpointto open a file it was clicked on:/__open-editor?fileName=…&token=….That lets a server tell the overlay's own request from one another page made.
OriginandSec-Fetch-*cannot do that everywhere: browsers send neither over plainhttpto anything butlocalhost, which is what a dev server on a network address is. webpack-dev-server will use it to protect/webpack-dev-server/open-editorwhenallowedOriginsis set./…, not//…). An absolute url can name any server, and the token is not that server's to have.client-src/utils/open-editor-url.js, which is unit-tested on its own.Tests:
test/open-editor-url.test.js: the token on a rooted path; never onhttps://…,//…or a relative path; nothing without a token.test/e2e/overlay.test.js: clicking a file reference reaches the endpoint carrying the instance's token.writeToDiskEACCES cases that fail onmaintoo). Minor changeset.Generated by Claude Code
Summary by CodeRabbit
Bug Fixes
TypeScript
Tests