Skip to content

fix(desktop): the fork's window is a secure context again - #176

Merged
ImZoomBoy merged 1 commit into
mainfrom
fix/t3code-origin-secure-context
Oct 10, 2026
Merged

ImZoomBoy merged 1 commit into
mainfrom
fix/t3code-origin-secure-context

Conversation

@ImZoomBoy

Copy link
Copy Markdown
Owner

In the installed fork (0.0.43-ap.10) T3 Connect links, but every relay call then fails with "Could not load relay DPoP proof key." and "Could not load the environment authorization key."

Cause

The window at t3code://app is not a secure context, so crypto.subtle is undefined there.

Electron keeps the secure, fetch and CORS schemes of the last protocol.registerSchemesAsPrivileged call only. Two calls run before ready:

  1. registerDesktopSchemePrivilegesSync in ElectronProtocol.ts lists t3code, t3code-dev, t3code-fork and t3code-fork-dev.
  2. createClerkBridge in @clerk/electron 0.0.44 (dist/esm/index.js line 293) then lists renderer.scheme alone.

Upstream passes the renderer scheme to the bridge, so its second call keeps t3code secure. Since #172 the fork passes the t3code-fork link scheme, so the second call drops t3code from the secure, fetch and CORS lists. The standard list adds up across calls, which is why the page still loads and still has localStorage.

The comment in ElectronProtocol.ts said Electron allows this call once. It allows more than one. The last one wins for those three lists.

Evidence

Built app. main (8cd0f93) was built with node scripts/build-desktop-artifact.ts --platform win --target nsis --keep-stage and run unpacked on an empty temp home. A DevTools protocol client read the main window:

"origin": "t3code://app",
"isSecureContext": false,
"typeof crypto.subtle": "undefined",
"generateKey": "threw TypeError: Cannot read properties of undefined (reading 'generateKey')",

That TypeError is the one at the end of the exception chain in the user's trace.

Which call wins. The same Electron binary (44.4.2) ran a 40 line script with no app code and no Clerk. Call 1 lists t3code and t3code-fork with all five privileges. Call 2 lists t3code-fork only. It then loads a blank page at each origin in a hidden window:

"switches": {
 "standard-schemes": "t3code,t3code-fork,t3code-fork",
 "secure-schemes": "t3code-fork",
 "fetch-schemes": "t3code-fork",
 "cors-schemes": "t3code-fork",
 "streaming-schemes": "t3code,t3code-fork,t3code-fork"
},
"t3code://app":      { "isSecureContext": false, "cryptoSubtle": "undefined", "localStorage": "object" },
"t3code-fork://app": { "isSecureContext": true,  "cryptoSubtle": "object",    "localStorage": "object" }

The one change that should flip it, a third call listing both schemes again:

"switches": {
 "secure-schemes": "t3code,t3code-fork",
 "fetch-schemes": "t3code,t3code-fork",
 "cors-schemes": "t3code,t3code-fork",
},
"t3code://app":      { "isSecureContext": true, "cryptoSubtle": "object", "localStorage": "object" },
"t3code-fork://app": { "isSecureContext": true, "cryptoSubtle": "object", "localStorage": "object" }

What changed

  • DesktopClerk.ts: createDesktopClerkBridge calls registerDesktopSchemePrivilegesSync() right after createClerkBridge returns, in the same synchronous step. The full list is the last call, so all four schemes keep every privilege.
  • ElectronProtocol.ts: exports registerDesktopSchemePrivilegesSync and replaces the "allows this call once" comment with what Electron does.
  • DesktopClerk.schemePrivileges.test.ts (new): see below.
  • DesktopClerk.test.ts: mocks electron, because DesktopClerk now calls into protocol.
  • fork-features.json: fork-connect-renderer-origin now describes the second call, its keep line names it, and its test list has the new test.

The bridge still gets the t3code-fork link scheme. No origin or authentication check changed. The fork still does not claim t3code:// from the OS.

Before and after, in the built app

Both builds ran unpacked on an empty temp home.

At t3code://app main this branch
window.isSecureContext false true
typeof crypto.subtle "undefined" "object"
crypto.subtle.generateKey (ECDSA P-256) throws TypeError ok: ECDSA private, extractable false
typeof crypto.randomUUID "undefined" "function"
typeof navigator.clipboard "undefined" "object"
typeof navigator.credentials, PublicKeyCredential "undefined" "object", "function"
typeof navigator.serviceWorker, navigator.locks, navigator.storage, caches "undefined" "object"
typeof navigator.mediaDevices "undefined" "object"
fetch("t3code://app/index.html") throws TypeError: Failed to fetch ok 200
fetch("/assets/ghostty-vt-DdA0Zryv.wasm") throws TypeError: Failed to fetch fetch ok 200, 630932 bytes
indexedDB.open, localStorage work work
Clerk.status ready ready

What else was broken since ap.10

All of these come from the same lost privileges, and all are fixed by this change:

  • T3 Connect relay and environment keys (apps/web/src/cloud/dpop.ts): crypto.subtle was undefined. This is the reported bug.
  • The built-in terminal: apps/web/src/terminal/ghostty/runtime.ts loads its WebAssembly with fetch, and fetch to the window's own origin failed.
  • Clipboard: navigator.clipboard was undefined. useCopyToClipboard falls back to execCommand for plain text, but its paste path (readText) and the direct navigator.clipboard calls do not: copy path in ChatView.tsx, ChatMarkdown.tsx, PreviewView.tsx, copy image in MediaActions.tsx, and terminal copy and paste in ghostty/surface.ts.
  • Passkeys: navigator.credentials and PublicKeyCredential were undefined in the page.
  • Browser recording (apps/web/src/browser/browserRecording.ts): navigator.mediaDevices was undefined.

Not broken: IndexedDB and localStorage worked, because the scheme stayed standard. The app does not call crypto.randomUUID, service workers, navigator.locks or caches directly. Its own randomUUID uses crypto.getRandomValues, which works without a secure context.

The test, and how it fails on main

DesktopClerk.schemePrivileges.test.ts runs the real @clerk/electron createClerkBridge behind DesktopClerk.layer, after ElectronProtocol.layerSchemePrivileges, the order main.ts uses. electron is replaced by a stand-in that keeps what the experiment above measured: standard and stream schemes add up, and the last call sets the secure, fetch and CORS lists. It asserts that the renderer scheme and the link scheme are in all three lists, for production and development, and that the bridge still claims only the link scheme from the OS.

A unit test cannot prove a secure context, so the built-app runs above are the proof of that. The test pins the registration order that causes it.

With DesktopClerk.ts and ElectronProtocol.ts put back to main and the test kept:

$ (apps/desktop) vp test run src/app/DesktopClerk.schemePrivileges.test.ts
     × keeps t3code://app a secure context once the bridge exists 9ms
     × keeps t3code-dev://app a secure context once the bridge exists 1ms
 FAIL  src/app/DesktopClerk.schemePrivileges.test.ts > DesktopClerk scheme privileges > keeps t3code://app a secure context once the bridge exists
AssertionError: secure: expected [ 't3code-fork' ] to include 't3code'
 FAIL  src/app/DesktopClerk.schemePrivileges.test.ts > DesktopClerk scheme privileges > keeps t3code-dev://app a secure context once the bridge exists
AssertionError: secure: expected [ 't3code-fork-dev' ] to include 't3code-dev'
 Test Files  1 failed (1)
      Tests  2 failed (2)

With the fix:

 Test Files  1 passed (1)
      Tests  2 passed (2)

Sign-in and the storage copy, in the built app

Sign-in, on the empty home, signed out. The run clicked Settings, Connections, then "Sign in to T3 Connect", and stopped at the email field:

"clerkLoaded": true,
"clerkStatus": "ready",
"connectionRows": ["T3 Connect", "Publish agent activity"],
"signInControl": "Sign in to T3 Connect",
"signInFormOpen": true,
"signInHeading": "Sign in to T3 Code",
"signInFields": ["identifier", "password"],
"signInButtons": ["Continue"]
clerk responses: 200 /v1/environment, 200 /v1/client, ...

Storage copy. A bare Electron script wrote four made-up values at t3code-fork://app into a second temp home. The branch build then started on that home twice:

launch 1: result: { _tag: 'Copied', copied: 4, skipped: 0 }
launch 2: result: { _tag: 'AlreadyMigrated' }

All four values read back at t3code://app on both launches, and the marker file holds copied.

Registry. HKCU\Software\Classes\t3code-fork and HKCU\Software\Classes\t3code were exported before every launch and restored after. During each run t3code-fork pointed at the unpacked build and t3code still pointed at the official install.

Checks

$ vp run --filter @t3tools/desktop typecheck
typecheck exit 0, 0 "error TS" lines

$ vp lint --report-unused-disable-directives
lint exit 0   (warnings only, none in the changed files)

$ vp fmt --check
All matched files use the correct format.
Finished in 1172ms on 4161 files using 24 threads.

$ vp run knip:check
knip exit 0

$ (apps/desktop) vp test run
 Test Files  4 failed | 106 passed | 4 skipped (114)
      Tests  22 failed | 1356 passed | 35 skipped (1413)

$ node scripts/check-fork-features.ts
Fork features: 35 entries, every file and test present.
 ❯ scripts/build-desktop-artifact.test.ts (71 tests | 11 failed)
 Test Files  1 failed | 32 passed (33)
      Tests  11 failed | 571 passed (582)

The failures are the same on main on this Windows machine. With every change put back to main:

$ (apps/desktop) vp test run scripts/electron-launcher.test.mjs src/snapShot/HyprlandSnapShot.test.ts src/snapShot/KdeSnapShot.test.ts src/snapShot/NiriSnapShot.test.ts
 ❯ scripts/electron-launcher.test.mjs (8 tests | 1 failed)
 ❯ src/snapShot/HyprlandSnapShot.test.ts (11 tests | 1 failed)
 ❯ src/snapShot/NiriSnapShot.test.ts (19 tests | 19 failed)
 ❯ src/snapShot/KdeSnapShot.test.ts (12 tests | 1 failed)
 Test Files  4 failed (4)
      Tests  22 failed | 28 passed (50)

$ vp test run scripts/build-desktop-artifact.test.ts
 ❯ scripts/build-desktop-artifact.test.ts (71 tests | 11 failed)
 Test Files  1 failed (1)
      Tests  11 failed | 60 passed (71)

The 22 desktop failures are Linux-only tests: the Niri, KDE and Hyprland screenshot helpers and a Unix file-permission launcher test.

One note on the local build. On this machine build-desktop-artifact.ts exits 1 at its last step on both main and this branch, after the app is packaged: Refusing to report success: C:\Users\Glyn\node_modules is visible from the probe directory. The unpacked app in the kept stage is complete, and that is what ran. The build used T3CODE_DESKTOP_REUSE_RESOURCE_MONITOR=true with a prebuilt resource monitor binary, because the Visual Studio Spectre libraries are missing here.

Since 0.0.43-ap.10 the window at t3code://app was not a secure context.
crypto.subtle was undefined there, so T3 Connect linked but every relay
call failed with "Could not load relay DPoP proof key." The terminal's
WebAssembly, the clipboard API and passkeys were broken the same way.

Electron keeps the secure, fetch and CORS schemes of the last
registerSchemesAsPrivileged call only. The Clerk bridge makes its own
call for the scheme it is given. Upstream gives it the renderer scheme.
The fork gives it the t3code-fork link scheme, so that call dropped
t3code from all three lists.

DesktopClerk now lists every scheme again right after it creates the
bridge. The bridge still gets the link scheme, so the official install
keeps its t3code:// links.
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S labels Oct 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ The exact PR base did not have a successful artifact. Baseline uses the latest successful main measurement shown below.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.5 KiB 13.5 KiB −15 B (−0.1%) 15.1 KiB ✅
Codex Thread snapshot wire 7.1 KiB 7.1 KiB +3 B (+0.0%) 7.3 KiB ✅
Codex Live turn WebSocket wire 6.4 KiB 6.4 KiB −18 B (−0.3%) 7.8 KiB ✅
Codex Live turn WebSocket decoded 56.1 KiB 56.1 KiB 0 B (0.0%) 66.4 KiB ✅
Codex Live turn messages 7 7 0 (0.0%) 21 ✅
Claude Total thread wire 13.5 KiB 13.5 KiB −35 B (−0.3%) 15.1 KiB ✅
Claude Thread snapshot wire 7.1 KiB 7.1 KiB −3 B (−0.0%) 7.3 KiB ✅
Claude Live turn WebSocket wire 6.4 KiB 6.4 KiB −32 B (−0.5%) 7.8 KiB ✅
Claude Live turn WebSocket decoded 57.0 KiB 56.9 KiB −44 B (−0.1%) 66.4 KiB ✅
Claude Live turn messages 8 7 −1 (−12.5%) 21 ✅

Baseline: d482468 · PR result: ab07e91 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 114.0 KiB
  • Claude decoded thread snapshot: 114.7 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@ImZoomBoy

Copy link
Copy Markdown
Owner Author

Code review, two axes. Each finding has a confidence and an estimated severity.

Standards

Standards review: PR 176

Outcome: no blocking findings. One hard violation and one process breach, both in the PR body. Six judgement calls in the code, all low severity.

Hard violations

  1. PR body - AGENTS.md, Pull requests: "End with the model and harness that did the work." The body ends with a local build note. Confidence high, severity low.
  2. PR body, Checks - AGENTS.md, Verifying: "Do not run repo-wide checks." The body reports repo-wide vp lint, vp fmt --check, vp run knip:check and the full desktop suite. This is process, not code. Confidence medium, severity low.

Judgement calls

  1. Mysterious Name - DesktopClerk.ts. createDesktopClerkBridge now also re-registers every scheme privilege (ElectronProtocol.registerDesktopSchemePrivilegesSync(); return bridge;). The name hides a global side effect. Fix: rename it, or make the call beside the call site. Confidence medium, severity low.
  2. Comment names its caller - ElectronProtocol.ts: "DesktopClerk runs this again right after." AGENTS.md, Taste: comments describe how a thing is used and move with the code. This one goes stale if the caller moves. Confidence medium, severity low.
  3. Duplicated explanation - "Electron keeps the secure, fetch and CORS schemes of the last call only" appears in the ElectronProtocol.ts docblock, the DesktopClerk.ts comment, the test header and fork-features.json. The JSON copy is required by the fork contract. The code copies could be one. Confidence medium, severity low.
  4. Duplicated Code - DesktopClerk.schemePrivileges.test.ts. makeStartupLayer repeats the DesktopEnvironment.of({ stateDir: "/tmp/t3-state", ... }) stub and layer wiring of makeDesktopClerkLayer in DesktopClerk.test.ts. Fix: share one helper. Confidence high, severity low.
  5. Test mirrors a model - AGENTS.md, Verifying: no tests that "mirror the implementation." The stand-in registerSchemesAsPrivileged hand-codes Electron's behaviour, so the test proves ordering against the author's own model. The PR body admits this and backs it with built-app runs. Also assert.equal(ElectronProtocol.getDesktopScheme(isDevelopment), rendererScheme) checks an unrelated function. Confidence low, severity low.
  6. Casts - as unknown as DesktopEnvironment.DesktopEnvironment["Service"] and the ElectronApp twin. AGENTS.md, Taste: "any is the enemy." It matches the existing pattern in DesktopClerk.test.ts. Confidence low, severity low.

Compliant

  • The title is a conventional commit in plain language.
  • The PR covers one concern.
  • fork-features.json follows docs/agents/upstream-conflict-resolution.md: the keep line names the new export and call, DesktopClerk.ts is already in files, and the new test is listed.
  • The glossary terms are used correctly.
  • CONTRIBUTING.md: small, focused, and it explains what changed and why.
  • No UI change, so no images are needed.

Note on finding 1: this fork leaves the model and harness line out of pull requests on purpose, so no change is wanted there.

Spec

Spec review: PR 176

Verdict: the fix matches the spec. No blocking gap. Five small findings.

I read the diff, the code around it and the PR body. I could not run the test or read @clerk/electron: this copy has no node_modules.

Checked and correct

  • The re-registration sits in the same synchronous function as createClerkBridge (apps/desktop/src/app/DesktopClerk.ts:93), so it runs before ready whenever the bridge's own call does.
  • main.ts:228-232 builds DesktopPreReadyPlatform.layer first, then the Clerk layer, for packaged and dev alike.
  • No other non-test code calls registerSchemesAsPrivileged.
  • The bridge still gets getDesktopLinkScheme. The test asserts setAsDefaultProtocolClient is called with the link scheme only.
  • No origin or authentication check is touched.
  • The link schemes keep all five privileges, so the migration origin stays standard and secure.
  • The "allows this call once" comment is gone, and the new comments match the PR's experiment.

(a) Missing or partial

  1. "The body pastes the real commands and what they printed at the end." Typecheck, lint and knip show a summary line ("exit 0"), not the printed output. Confidence high, severity low.
  2. "Update the fork-connect-renderer-origin entry." The files list does not name apps/desktop/src/app/DesktopClerk.test.ts, which now carries a fork-only electron mock. The new test and the changed source files are listed. Confidence medium, severity low.
  3. "export both keys before every launch and restore them after, and say so." The body says so but shows no output. Confidence high, severity low.

(b) Scope creep

None found.

(c) Implemented but possibly wrong

  1. "A test that fails on main first and passes with the fix." The test checks a hand-written model of Electron (last call wins), not Electron. It would fail if the call were removed or moved before the bridge. It would still pass if a later Electron changed the rule. The PR says this and backs it with built-app runs, which the spec allows. Confidence high, severity low.
  2. "Fix it without weakening any origin or authentication check." If the new call throws, the bridge is already created and holds the single-instance lock, but acquireRelease never gets it, so cleanup does not run (DesktopClerk.ts:111-113). The bridge's own call would throw first in the same situation, so this is unlikely. Confidence low, severity low.

Worth carrying forward

The PR reports more broken since ap.10 than T3 Connect: the built-in terminal (same-origin fetch failed), clipboard paste and direct copy, passkeys, and browser recording. All from the same lost privileges.

@ImZoomBoy
ImZoomBoy merged commit fddd2c2 into main Oct 10, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant