Skip to content

feat(download): browser-native download capability (clears Cloudflare via chrome.downloads) - #113

Merged
chrischall merged 1 commit into
mainfrom
feat/download-capability
Jun 5, 2026
Merged

feat(download): browser-native download capability (clears Cloudflare via chrome.downloads)#113
chrischall merged 1 commit into
mainfrom
feat/download-capability

Conversation

@chrischall

Copy link
Copy Markdown
Owner

Why

Some authenticated downloads can't be done with a content-script fetch(): the endpoint is Cloudflare-walled and a cors-mode fetch() gets the bot-challenge even from the signed-in tab (only a top-level browser request carries the right cf_clearance + TLS/JA3 fingerprint). The concrete case is musescore-mcp's official PDF: /score/download/index 302s to a presigned s3w.musescore.com URL, but capture_redirect ends up capturing the challenge page, not the S3 object, so the server-side fetch 403s. chrome.downloads.download is the only browser primitive that both clears the wall and yields the bytes.

What

A new download capability. The extension hands the URL to chrome.downloads.download — the browser fetches it (real cookies + fingerprint), clears Cloudflare, follows the cross-origin redirect, and saves the file. The extension returns the saved local file path + size; the bridge is loopback-only / single-host, so the consuming MCP reads the bytes from the same disk and moves them. Only the download record is erased; the file is left for the MCP.

Wire-additive and opt-in — unknown capabilities are still rejected by the validator, and the verb only does anything when an MCP declares download (which forces a re-pair, surfaced in the popup as "Download files to your computer ⚠️").

Changes by package

  • protocol: download in Capability + KNOWN_CAPABILITIES; DownloadInit, InnerRequestDownload, DownloadResult, InnerResponseDownloadOk; request validator (https url, relative filename with no ..) + structured-value response validator.
  • server: FetchproxyServer.download() (capability guard + lazy-revive mirror of captureRedirect), pendingDownload map, response routing, rejectAllPending cleanup; FetchproxyBridgeDownError.op union extended.
  • extension-core: handleDownloadRequest (download → onChanged completesearch → respond → erase record; interrupted/timeout/off-domain errors; race-guard for fast completions); chrome.downloads type stub; popup label; exported downloadValueFromItem helper.
  • extension-chrome: "downloads" manifest permission + README manifest highlight.

Tests

TDD; +21 unit tests (protocol validators, server convenience, extension-core helper). Following repo convention, the async handler orchestration is live-tested out of band (handlers no-op without a live session); the pure value-mapping is unit-tested.

npm test is green except 3 pre-existing flaky timeouts in all-bootstrap-verbs.test — verified identical on clean main (serial run: main = 3 fail / branch = 3 fail, same file), and the mass parallel-run failures were purely localhost-port contention on a loaded machine, not this change. npm run typecheck and the full workspace build pass.

Follow-up

A companion musescore-mcp PR will consume this (declare download, replace the capture_redirect+curl official-PDF path) once this releases via Tag & Bump. Live test requires reloading the unpacked extension + re-pair (new capability).

🤖 Generated with Claude Code

… via chrome.downloads)

Adds a new `download` capability so an MCP can fetch a file through the
BROWSER's own network stack (chrome.downloads.download) rather than a
content-script fetch(). The browser request carries the user's real cookies +
TLS/JA3 fingerprint, so it clears a Cloudflare bot-challenge that a cors-mode
fetch() cannot, and follows the cross-origin redirect to the final file. The
extension returns the saved local file path + size; the bridge is loopback-only
/ single-host, so the consuming MCP reads the bytes from the same disk. The
download RECORD is erased after responding; the file is left for the MCP to move.

Motivation: musescore.com's official PDF download endpoint 302s to a presigned
S3 URL, but the endpoint is Cloudflare-walled and a page-level fetch() gets the
bot-challenge (capture_redirect then captures the challenge page, not the S3
object). chrome.downloads is the only browser primitive that both clears the
wall and yields the bytes.

- protocol: `download` in Capability + KNOWN_CAPABILITIES; DownloadInit,
  InnerRequestDownload, DownloadResult, InnerResponseDownloadOk; request +
  response validators (https url, relative no-`..` filename, structured value).
- server: FetchproxyServer.download() with capability guard + lazy-revive, a
  pendingDownload map, response routing, and rejectAllPending cleanup;
  BridgeDownError op union extended.
- extension-core: handleDownloadRequest (chrome.downloads.download → onChanged
  'complete' → search → respond → erase record; interrupted/timeout/off-domain
  errors; race guard for fast completions); chrome.downloads type stub; popup
  capability label; exported downloadValueFromItem helper (unit-tested — the
  async orchestration is live-tested like other handlers).
- extension-chrome: "downloads" manifest permission + README manifest highlight.

Tests: +21 (protocol validators, server convenience, extension-core helper).
Full suite green except 3 pre-existing flaky WS-port timeouts in
all-bootstrap-verbs.test (identical on clean main; unrelated to this change).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@chrischall chrischall added the enhancement New feature or request label Jun 5, 2026
@github-actions github-actions Bot added the auto-review Trigger Claude + Copilot review on this PR label Jun 5, 2026
@claude

claude Bot commented Jun 5, 2026

Copy link
Copy Markdown

Claude finished @chrischall's task in 4m 51s —— View job


Review — PR #113 feat(download): browser-native download capability

  • Read PR diff
  • Review protocol changes (frames + validators)
  • Review server changes (download + pending map + cleanup)
  • Review extension-core changes (handler + helpers)
  • Review extension-chrome (manifest + README)
  • Check test coverage of new code paths
  • Check CLAUDE.md conventions (no console.log, ESM .js imports, etc.)
  • Post inline findings + summary

Verdict: warn — no blocking issues; 2 🟡 nits worth a look.

Summary

Clean, wire-additive PR. Validators are tight (https-only URL, no absolute filename, no .. traversal, unknown-field rejection on both request and response, integer non-negative bytes). Server download() mirrors captureRedirect's lazy-revive shape and rejectAllPending is updated. Extension handler is mostly defensive: listener registered before download(), race-guard for fast completion, conflictAction: 'uniquify' so the MCP can't overwrite existing files via a chosen filename, only the download record is erased. Capability disclosed in popup with ⚠️, manifest + README updated.

No CLAUDE.md violations spotted: no console.log/debug/info added on the MCP side, ESM .js imports preserved, no cross-workspace literal-version deps touched, no version bumps. New code paths look unit-covered (21 new tests across protocol + server convenience + extension-core helper).

Findings

🟡 succeed() race on fast downloadsbackground.ts:2034–2064done = true is set after the await downloads.search(...), so the onChanged 'complete' path and the post-download() race-guard can both pass the guard, both reach sendInner, and emit a duplicate response frame for the same req.id. Server silently drops the second frame (pendingDownload is already deleted), so impact is just extra WS traffic + a wire-protocol "1 response per request" violation. Inline comment includes a suggestion to claim ownership before the await. Fix this →

🟡 download() skips server-side assertUrlInDomainsws-server.ts:2206–2215request() pre-validates URL host at the call site for a clean error; download() waits for the extension to reject and surfaces it as a FetchproxyProtocolError after a full bridge round-trip. Consistent with captureRedirect's lazy style, but a one-line assertUrlInDomains('download url', opts.url, this.opts.domains) after the capability check would fail fast. Fix this →

No 🔴 Important findings.
feat/download-capability

Comment on lines +2034 to +2064
done = true;
cleanup();
void sendInner(mcpId, { type: 'response', id: req.id, ok: false, op: 'download', error });
};
const succeed = async (): Promise<void> => {
if (done || downloadId === undefined) return;
let items;
try {
items = await downloads.search({ id: downloadId });
} catch (e) {
fail(`download search failed: ${String(e)}`);
return;
}
const item = items[0];
if (!item) {
fail('download completed but its record was not found');
return;
}
done = true;
cleanup();
// Erase only the download RECORD — the file stays for the MCP to move.
void downloads.erase({ id: downloadId }).catch(() => {
// best-effort record cleanup
});
void sendInner(mcpId, {
type: 'response',
id: req.id,
ok: true,
op: 'download',
value: downloadValueFromItem(item),
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Nit — succeed() can run twice and emit a duplicate response frame.

done is set after await downloads.search(...). For a fast download, both wake-up paths can pass the early-return check before either gets to done = true:

  1. await downloads.download(...) resolves and assigns downloadId.
  2. Chrome dispatches the onChanged 'complete' delta to the listener → void succeed() (call chore(deps-dev): bump @types/chrome from 0.0.270 to 0.1.42 in the dev-dependencies group #1) → awaits downloads.search.
  3. The race-guard await downloads.search runs in parallel, sees state === 'complete'void succeed() (call fix(extension): chrome.alarms keepalive so MV3 SW doesn't sleep between MCP calls #2) → awaits downloads.search.
  4. Both calls clear the done guard, both reach sendInner(... ok: true ...) for the same req.id.

Bounded harm — the server's pendingDownload.get(id) is undefined on the second response (already deleted), so the extra frame is silently dropped — but it does mean two encrypted WS frames per fast download and a wire-protocol "1 response per request" violation.

Trivial fix: claim ownership before the await.

Suggested change
done = true;
cleanup();
void sendInner(mcpId, { type: 'response', id: req.id, ok: false, op: 'download', error });
};
const succeed = async (): Promise<void> => {
if (done || downloadId === undefined) return;
let items;
try {
items = await downloads.search({ id: downloadId });
} catch (e) {
fail(`download search failed: ${String(e)}`);
return;
}
const item = items[0];
if (!item) {
fail('download completed but its record was not found');
return;
}
done = true;
cleanup();
// Erase only the download RECORD — the file stays for the MCP to move.
void downloads.erase({ id: downloadId }).catch(() => {
// best-effort record cleanup
});
void sendInner(mcpId, {
type: 'response',
id: req.id,
ok: true,
op: 'download',
value: downloadValueFromItem(item),
});
const succeed = async (): Promise<void> => {
if (done || downloadId === undefined) return;
done = true;
cleanup();
let items;
try {
items = await downloads.search({ id: downloadId });
} catch (e) {
void sendInner(mcpId, {
type: 'response',
id: req.id,
ok: false,
op: 'download',
error: `download search failed: ${String(e)}`,
});
return;
}
const item = items[0];
if (!item) {
void sendInner(mcpId, {
type: 'response',
id: req.id,
ok: false,
op: 'download',
error: 'download completed but its record was not found',
});
return;
}
// Erase only the download RECORD — the file stays for the MCP to move.
void downloads.erase({ id: downloadId }).catch(() => {
// best-effort record cleanup
});
void sendInner(mcpId, {
type: 'response',
id: req.id,
ok: true,
op: 'download',
value: downloadValueFromItem(item),
});
};

(Inlined the failure paths instead of calling fail(), since fail would now early-return on done.)

Comment on lines +2206 to +2215
throw new Error(
'FetchproxyServer.download(): MCP did not declare "download" in capabilities',
);
}
await this.ensureConnected();
this.throwIfPendingPair();
try {
const result = await this._downloadOnce(opts);
this.recordSuccess();
return result;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Nit — download() doesn't pre-validate the URL host against declared domains server-side.

request() calls assertUrlInDomains('request url', url, this.opts.domains) before sending (ws-server.ts:1491) so MCP authors get a clean call-site error. The new download() skips this and relies solely on the extension's isUrlAllowedForAnyDomain check, which surfaces as a FetchproxyProtocolError with the message "download url host not in domains [...]" after a full WS round-trip.

Functionally equivalent — captureRedirect is also lazy like this — but a pre-flight assertUrlInDomains('download url', opts.url, this.opts.domains) right after the capability check would fail fast with a stack pointing at the MCP author's call site and avoid spending a bridge round-trip on a URL that was never going to work. Up to you whether to bring download() in line with request() or to consider this consistent with the lazy verbs.

@chrischall chrischall added the ready-to-merge Owner has reviewed; arm auto-merge to land when CI is green label Jun 5, 2026
@chrischall
chrischall merged commit 2f465ca into main Jun 5, 2026
14 checks passed
@chrischall
chrischall deleted the feat/download-capability branch June 5, 2026 20:17
chrischall added a commit that referenced this pull request Jun 6, 2026
…url (PR #113 nits) (#115)

Addresses the two 🟡 nits from the
[#113](#113) auto-review
(verdict was *warn*, no blockers).

**1. Duplicate response frame on fast downloads** —
`handleDownloadRequest`'s `succeed()` set `done = true` *after* `await
downloads.search(...)`, so the `onChanged` 'complete' path and the
post-`download()` race-guard could both pass the `if (done)` guard while
a prior call was still awaiting, and emit two response frames for one
`req.id`. Now `succeed()` claims ownership (`done = true; cleanup()`)
**before** the first await; the error paths send directly (since
`fail()` would no-op once `done` is set). Exactly one frame per request.

**2. `download()` skipped server-side host pre-validation** —
`request()` calls `assertUrlInDomains()` at the call site for a clean
local error; `download()` waited for the extension to reject after a
full bridge round-trip. Added the same one-liner after the capability
check. New convenience test asserts an out-of-domain URL rejects locally
and sends **nothing** over the bridge.

## Test
`npm test` (serial) green except the same 3 pre-existing flaky
`all-bootstrap-verbs` WS-port timeouts (identical on `main`); `+1` new
test (3236 passed). Typecheck + build clean.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
chrischall added a commit that referenced this pull request Jun 6, 2026
🤖 I have created a release *beep* *boop*
---


##
[1.3.0](v1.2.0...v1.3.0)
(2026-06-06)


### Features

* **download:** browser-native download capability (clears Cloudflare
via chrome.downloads)
([#113](#113))
([2f465ca](2f465ca))


### Bug Fixes

* **download:** one response frame per request + fast-fail off-domain
url (PR [#113](#113)
nits) ([#115](#115))
([0d22a53](0d22a53))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-review Trigger Claude + Copilot review on this PR enhancement New feature or request ready-to-merge Owner has reviewed; arm auto-merge to land when CI is green

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant