Skip to content

fix(extension): reject tab URLs without a scheme - #580

Open
columbia11 wants to merge 10 commits into
Tencent:mainfrom
columbia11:fix/reject-relative-urls
Open

columbia11 wants to merge 10 commits into
Tencent:mainfrom
columbia11:fix/reject-relative-urls

Conversation

@columbia11

@columbia11 columbia11 commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Problem

navigate, tab_create and record_start pass the requested URL straight to Chrome. Chrome resolves a relative tab URL against the extension. Checked in Chrome 156.0.8078.80 with a throwaway extension:

chrome.tabs.create({url: "example.com"})       -> chrome-extension://<id>/example.com
chrome.tabs.update(tabId, {url: "example.com"}) -> chrome-extension://<id>/example.com

So:

  • bsk tab new --url example.com opens the extension's own (missing) page.
  • bsk navigate example.com from a page that cannot be driven over CDP, such as chrome://newtab, takes the chrome.tabs.update fallback and lands on the same page.
  • Over CDP the same request fails late as cdp_failed: Invalid URL, from new URL(params.url), after the tab was already prepared.

localhost:3000 and example.com:8080/path parse as URLs with localhost: / example.com: as the scheme, so they also pass through and cannot open as websites.

Fix

relativeUrlError() in tools/shared.ts returns invalid_params ("url must be absolute, including its scheme (for example https://example.com)") when the URL does not parse, or when it is an opaque URL whose path is a port (host:port typed without a scheme; tel: and sms: are allowed). navigate, tab_create and record_start call it before touching the tab.

  • CHANGELOG: one line under Unreleased → Fixed.
  • No skill or README example uses a URL without a scheme.

Tests

  • shared.test.ts: rejects example.com, /path, localhost:3000, example.com:8080/path; accepts https://, about:blank, data:, tel: and an omitted URL. The host:port cases fail without the port check.
  • navigation.test.ts, tabs.test.ts, record-steps.test.ts: each entry point returns invalid_params without sending CDP commands, creating a tab or messaging the page. These fail without the fix.
  • Full extension vitest run: 3626 passed, 418 skipped. tsc --noEmit clean. biome check clean on the changed files.

This and #579 both add a line at the top of the CHANGELOG's Fixed list, so whichever merges second needs a trivial rebase.

🤖 Generated with Claude Code

`navigate`, `tab_create` and `record_start` passed the requested URL straight
to Chrome. Chrome resolves a relative tab URL against the extension: in
Chrome 156 both chrome.tabs.create and chrome.tabs.update turned
`example.com` into chrome-extension://<id>/example.com. That is what
`bsk tab new --url example.com` opened, and what `bsk navigate example.com`
opened from a page that cannot be driven over CDP (for example
chrome://newtab). Over CDP, navigate failed as `cdp_failed: Invalid URL`
after the tab was prepared.

Reject a URL that does not parse as absolute with `invalid_params` before
touching the tab. `localhost:3000` and `example.com:8080` parse with the host
as their scheme, so an opaque URL whose path is a port is rejected too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@columbia11
columbia11 force-pushed the fix/reject-relative-urls branch from 2fb18c0 to 69cda79 Compare November 4, 2026 06:26
Keep both Unreleased "Fixed" entries in CHANGELOG.md and add the Chinese
entries for this PR and for Tencent#583 (merged before the Chinese changelog
landed), so check-changelog-sync passes again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
columbia11 and others added 7 commits November 5, 2026 12:14
Keep both Unreleased "Fixed" entries (this PR's and Tencent#579's) in both
changelogs; Tencent#583's Chinese entry is now on main.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@columbia11

Copy link
Copy Markdown
Contributor Author

「Windows daemon, installer and cancellation regression tests」在 #580 上失败过两次,都出在 #601 新增的 cli::update::sources::tests,每次失败的用例不同:

  • e80fcde:both_manifest_failures_include_sources_urls_and_http_causes 等 3 个(测试服务器记录到的请求路径为空)
  • 9bc2995:manifest_timeout_falls_back_without_waiting_for_the_slow_source(请求本地 127.0.0.1 测试服务器失败)

同一份代码在 main(4c87b51f、ac6aea9a)和 #578/#577/#573/#572 上通过,#567 也遇到了同一个用例失败;#580 没有改动 Rust 代码。看起来是这组测试在 Windows runner 上不稳定。刚合入最新 main(e64b882a)已重新触发 CI,如果这个 job 再失败,麻烦帮忙重跑一下(fork 这边没有重跑权限)。

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant