fix(shared): one-letter link schemes no longer become file links on Windows#24006

open1

main

GitHub fork: fix/shared-file-url-requires-file-scheme

opened9d agobyZZacharon

View on GitHubTwo-way sync enabledPR last imported 2d ago

Problem

On Windows, Chromium's URL parser reads a one-letter scheme as a drive letter. In Chrome, Edge and the Electron desktop app on Windows, new URL("d:alert(1)").href is file:///D:/alert(1). Node and Chromium on other platforms keep d:alert(1).

parseFileUrlHref parses first and checks the protocol after, so on Windows rewriteMarkdownFileUriHref turns a chat link like [unknown](d:alert(1)) into a file chip for D:/alert(1). The existing browser test ChatMarkdown Windows file links > still rejects unsafe schemes fails on a Windows host for this reason. CI runs on Linux, so it never sees the failure.

Change

parseFileUrlHref returns null unless the href starts with file:, before it calls new URL. Two of its three callers (parseMarkdownFileLink and client-runtime's markdownImages) already check this first. The web markdown link rewrite did not.

Scope and approval

Small, focused fix for an obvious bug: one guard in a shared helper, so links render the same on every client OS. This isn't script execution. The chip only opens a path, but it shouldn't exist at all.

Verification

Windows 10 (19045), Node 26.12.1, Vite+ 1.3.0, Playwright Chromium headless shell 148.

Parser difference, from Playwright's Chromium on Windows: d:alert(1), c:foo and x:y become file:///D:/alert(1), file:///C:/foo and file:///X:/y. Node returns them unchanged.
From apps/web: vp test run --project browser src/components/ChatMarkdown.browser.test.tsx. On main, still rejects unsafe schemes fails because the output contains href="D:/alert(1)"; the other 5 pass. With the fix, 6/6 pass.
Related tests pass with the fix: shared fileLinks, markdownLinks and path (63), client-runtime markdownImages (12), web unit markdown-links and terminal-links (48), and the three markdown browser test files (12).
vp run check:changed: format, lint, knip, typecheck (28 packages) and the fileLinks test pass. On Windows this needed a local workaround for check:changed spawning vp.cmd, which isn't part of this PR.

No new test. The existing browser test is the regression test and fails on main on Windows. A Node unit test can't reproduce Chromium's parsing.

Fix written by Claude Opus 5.5 in Claude Code (T3 Code).

🤖 Generated with Claude Code

Checks

7 successful · 2 skipped

All checks passed

Added 1 commit9d ago

fix(shared): one-letter link schemes no longer become file links on Windowseb2e68f

MmacroscopeappBotapproved these changes9d ago

MmacroscopeappBot9d ago

Approvability

Verdict: Approved at eb2e68f

Macroscope's review found this PR approvable — This is a focused four-line bug fix that prevents one-letter schemes from being misclassified as file links on Windows while preserving valid file: URL handling. It affects only existing markdown/file-link parsing paths and introduces no schema, deployment, default, or static-analysis changes.

^{You can add or adjust custom eligibility rules. Learn more.}

CcoderabbitaiBot9d ago

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Source mode: Auto
Repository: pingdotgg/t3code
Branch: main
File: .coderabbit.config.ts
Commit: 807319c
Layers: Repository TS config
Review profile: CHILL
Plan: Advanced
Run ID: 6acb7e71-501b-45ed-8eb2-7bc26f4301b8

📥 Commits

Reviewing files that changed from the base of the PR and between 807319c276c6831b2c219640e9da5b38019a8f1d and eb2e68f35a5dac7fe2c919bc3d4c4f23ea268fa8.

📒 Files selected for processing: 1

packages/shared/src/fileLinks.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

📝 Walkthrough

parseFileUrlHref now checks for a file: prefix before parsing the URL. Inputs that fail the check return null.

Changes: File URL Validation

Layer / File(s)	Summary
Validate the file URL prefix `packages/shared/src/fileLinks.ts`	`parseFileUrlHref` checks the trimmed, lowercased input for a `file:` prefix before URL parsing. Inputs without the prefix return `null`.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested labels: bugfix

Suggested reviewers: hogeheer499-commits, utkarshusername

Merge Risk: ⚪ Minimal · up to eb2e6

The change addresses the Windows file-link issue without an identified regression or merge-blocking risk.

Pre-merge checks: ^Passed 4

Check name	Status	Explanation
Title check	Passed	The title is concise, specific, and accurately describes the Windows file-link parsing fix.
Description check	Passed	The description includes all required sections. It explains the Windows-specific problem, the implementation, the focused scope rationale, detailed verification results, limitations, and the agent att…
Linked Issues check	Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	Passed	Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description includes all required sections. It explains the Windows-specific problem, the implementation, the focused scope rationale, detailed verification results, limitations, and the agent attribution.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

Mmaria2d ago

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified the linux web path at exact head eb2e68f35a5dac7fe2c919bc3d4c4f23ea268fa8, after running github/main (5ef56853ef) in a separate worktree first. used vp i, one background dev server at a time, playwright from the repo dependencies, and /usr/bin/google-chrome. final verification used fresh isolated state at /home/user/t3code/.t3/verify.

created a real git repository, added its project through the web app, and submitted real codex gpt-6.1-sol turns through the composer and local provider proxy. the provider returned the requested markdown on both revisions. no mocks, fixtures, replay provider, or source changes.

d:alert(1), c:foo, and x:y rendered without an href. clicking them did not navigate or open a file.
file:///home/user/link-verification/README.md#L3 rendered as a file chip. clicking it loaded the real file in the panel and highlighted line 3.
https://example.com retained its destination and opened an external tab with example-domain content.
39 focused parser, markdown-link, and markdown-image tests passed through blacksmith-check.
vp run check:changed 807319c276c6831b2c219640e9da5b38019a8f1d passed through blacksmith-check: format, lint, knip, 28 affected package typechecks, and 18 parser tests. the explicit ref is the pr's base; the shallow worker needed its ancestry fetched first.

main baseline with the same prompt and data:

main linux baseline with real codex link response

pr head with matching prompt and data:

pr head linux codex response with blocked schemes and valid file chip

pr head opens the real readme and highlights line 3

pr head link clicks and file panel interaction

short runtime transcript, including selected real provider events

Not verified

the windows-specific regression remains unverified. on this linux vm, chrome returns d:alert(1), c:foo, and x:y unchanged from new URL(...).href; it does not convert them to file: urls. the original bug therefore did not reproduce on main. the linux before/after captures prove the exercised paths still work, but cannot prove the windows fix. no url-parser shim or simulated platform was used.

windows chrome/edge and electron still need the same real-chat scenario on windows. native desktop shells, macos, ios/android, remote relay/tunnel connections, and other providers were not exercised here.

ZZacharon2d ago

Native Windows check at eb2e68f35a5dac7fe2c919bc3d4c4f23ea268fa8

Host: Windows 10 Pro 10.0.19045 x64. Node 26.12.1, Vite+ 1.4.0. Edge 156.0.4314.56, Chrome for Testing 148.0.7778.96 (Playwright's Chromium, since stable Chrome isn't installed), and the desktop dev app (Electron 44.10.0). I ran main (6575ce32508b) first, then the same checkout with this PR's commit applied, using vp run dev / vp run dev:desktop with an isolated T3CODE_HOME. I added a real git repo as a project and sent a real Codex turn. The reply matched the prompt exactly: [one](d:alert(1)) [two](c:foo) [three](x:y), [readme](file:///C:/t3-linktest/README.md#L3), [site](https://example.com).

On Windows the bug shows in the user message, not in Codex's reply. User messages render with parseRawHtml={false}. The assistant's raw-HTML path already drops these hrefs on main, so the reply looks clean on both revisions. The browser test covers both modes.

On main:

In the user message, Edge, Chrome for Testing and the desktop app render d:alert(1), c:foo and x:y as file chips (D:/alert(1), C:/foo, X:/y). Clicking alert(1) opens the file panel with "could not be accessed or read" and the attempted path D:/alert(1).
ChatMarkdown.browser.test.tsx: still rejects unsafe schemes fails and 10 others pass.

With this PR:

In all three, none of the three schemes renders as a file chip. In the desktop app, clicking them does nothing. In the browsers, the user-message ones keep href="" and open the current page in a new tab, the same as javascript: links already do on main.
The README chip opens the file with line 3 highlighted. https://example.com opens externally.
ChatMarkdown.browser.test.tsx: 11/11 pass.

Not covered: Windows 11, ARM64, stable Chrome, and a packaged desktop build.

Written by Claude Opus 5.5 (Claude Code in T3 Code) on behalf of the PR author.