Fix desktop Cloudflare Access remote connections#86
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0c013e35f4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| <div className="space-y-2"> | ||
| <span className="block text-xs font-medium text-foreground"> | ||
| Cloudflare Access service token | ||
| </span> |
There was a problem hiding this comment.
In the web/desktop settings panel, any user who fills these new Cloudflare Access service-token fields (or pastes a pairing URL that populates them) will always be rejected before pairing completes: preparePairingRegistration calls validateCloudflareAccessRuntime, which rejects service-token auth unless canPassWebSocketHeaderOptions() is true, while apps/web/src/lib/runtime.ts provides the unmarked browser/Electron Socket.layerWebSocketConstructorGlobal (only mobile wraps the constructor as header-capable). This makes the newly exposed desktop UI path unusable unless the desktop runtime is made header-capable or the fields are hidden/disabled there.
Useful? React with 👍 / 👎.
| ...(target.cloudflareAccessToken | ||
| ? { cloudflareAccessToken: target.cloudflareAccessToken } | ||
| : {}), |
There was a problem hiding this comment.
When a pasted pairing URL carries cf_access_token, these lines now save that JWT and pairing can complete over HTTP, but the saved connection still opens its WebSocket from the web/desktop runtime where Socket.layerWebSocketConstructorGlobal is not header-capable. Unlike the desktop cookie sign-in path, no CF_Authorization cookie is installed for this JWT, so a Cloudflare Access-protected backend will reject the later WebSocket handshake even though the environment was saved successfully.
Useful? React with 👍 / 👎.
wizzoapp
Bot
force-pushed
the
codex/desktop-cfaccess-fix
branch
from
0c013e3 to
ecd5110
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ecd511059d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ); | ||
| const handleSavedBackendHostChange = useCallback((value: string) => { | ||
| setSavedBackendCloudflareAccess(null); | ||
| setSavedBackendCloudflareAccessToken(""); |
There was a problem hiding this comment.
When a user has populated the Cloudflare Access service-token fields (for example by pasting a pairing URL) and then edits the host to a different bare/manual host, this handler clears the saved cookie/JWT state but leaves savedBackendCloudflareAccessClientId and savedBackendCloudflareAccessClientSecret intact in the non-URL branch. The next Add Environment attempt will therefore send the old cf-access-client-secret to the newly typed host. Clear the service-token state on host changes unless the newly parsed URL repopulates it.
Useful? React with 👍 / 👎.
wizzoapp
Bot
force-pushed
the
codex/desktop-cfaccess-fix
branch
from
ecd5110 to
1ce9f43
Compare
|
Superseded by a fresh PR from the same branch because Codex review stopped attaching to fresh pushes on this PR after multiple review cycles. Replacement PR follows. |
Summary
Proof
apps/desktop/src/ipc/methods/cloudflareAccess.test.ts,packages/shared/src/remote.test.ts,packages/client-runtime/src/connection/onboarding.test.ts,apps/web/src/components/settings/ConnectionsSettings.logic.test.tsvp fmt --check,vp check,vp run typechecknode scripts/build-desktop-artifact.ts --platform linux --target dir --arch x64 --skip-build --keep-stageagainst a built desktop artifactxvfb-run -a node scripts/desktop-launch-smoke.mjs --command /home/adam/tmp/t3code-desktop-build/t3code-desktop-linux-stage-PcCDvh/app/dist/linux-unpacked/t3code --timeout-ms 120000 --stability-ms 2000window.desktopBridge.authenticateCloudflareAccess({ host: "https://oc.agriffiths.dev/" }), observed renderer sourcehttps://agriffiths.cloudflareaccess.com/cdn-cgi/access/login/..., and asserted IPC state stayedpendinginstead of rejectingNotes
vp checkstill reports 12 pre-existing mobile unused-symbol warnings outside this diff.vp run typecheckstill reports two pre-existing Effect suggestions outside this diff.