Skip to content

feat: add capture_redirect capability - #108

Merged
chrischall merged 1 commit into
mainfrom
feat/capture-redirect
Jun 4, 2026
Merged

chrischall merged 1 commit into
mainfrom
feat/capture-redirect

Conversation

@chrischall

Copy link
Copy Markdown
Owner

What

Adds a new capture_redirect capability that snapshots the redirect target URL of the next request the browser makes to a declared (host, path?), via chrome.webRequest.onBeforeRedirect. Single-shot.

Mirrors the existing capture_request_header capability, but lighter: there's no per-entry declared scope plumbing. The only gate is that the watched host is one of the MCP's declared domains (equals-or-subdomain). onBeforeRedirect needs no extraInfoSpec.

Why

A Cloudflare-walled endpoint that 302-redirects cross-origin to a presigned URL (e.g. musescore's score/download/index → presigned S3) is opaque to a page-level fetch — the redirect target can't be read cross-origin. But onBeforeRedirect exposes details.redirectUrl at the network layer regardless of CORS, so the consumer can capture the presigned target and fetch it server-side (no CORS off-browser, and the presigned URL carries its own auth so no Cloudflare/TLS binding).

Changes

  • protocol: 'capture_redirect' added to the Capability union + KNOWN_CAPABILITIES; CaptureRedirectInit {host, path?, timeoutMs?} + inner request/response (response value = redirect URL); validateInnerRequest/validateInnerResponse branches.
  • server: FetchproxyServer.captureRedirect({host, path?, timeoutMs?}): Promise<string>, with the same bridge-down / lazy-revive handling as captureRequestHeader.
  • extension-core: dispatcher branch + handleCaptureRedirectRequest (host-in-domains gate, onBeforeRedirect listener filtered on https://${host}${path ?? '/*'}, double-fire guard, 30s default timeout).

Tests

  • protocol validate.test.ts: accepts a valid capture_redirect request/ok-response; rejects bad host, bad path, unexpected fields, bad timeoutMs, missing/non-string value.
  • server convenience.test.ts: captureRedirect() throws when capability undeclared, emits the correct inner-request shape and resolves the URL, threads timeoutMs, omits path when absent, surfaces timeout as a protocol error.

Full suite: 815 tests green. npm run build --workspaces clean.

🤖 Generated with Claude Code

Add a `capture_redirect` capability that snapshots the redirect target
URL of the next request the browser makes to a declared (host, path?),
via `chrome.webRequest.onBeforeRedirect`. Single-shot, mirroring
`capture_request_header` but lighter — the only gate is that the watched
host is one of the MCP's declared `domains` (equals-or-subdomain); no
per-entry declared scope is required, and `onBeforeRedirect` needs no
`extraInfoSpec`.

Use case: a Cloudflare-walled endpoint that 302-redirects cross-origin
to a presigned URL. A page-level fetch sees only an opaque cross-origin
redirect, but `onBeforeRedirect` exposes `details.redirectUrl` at the
network layer — so the consumer can capture the presigned target and
fetch it server-side (no CORS off-browser).

- protocol: add 'capture_redirect' to Capability + KNOWN_CAPABILITIES;
  CaptureRedirectInit + inner request/response (value = redirect URL);
  validateInnerRequest/Response branches.
- server: FetchproxyServer.captureRedirect({host, path?, timeoutMs?}),
  with the same bridge-down/lazy-revive handling as captureRequestHeader.
- extension-core: dispatcher branch + handleCaptureRedirectRequest
  (host-in-domains gate, onBeforeRedirect listener, 30s default timeout,
  double-fire guard).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions github-actions Bot added the auto-review Auto-review pipeline is handling this PR label Jun 4, 2026
@claude

claude Bot commented Jun 4, 2026 •

Copy link
Copy Markdown

Claude finished @chrischall's task in 6m 2s —— View job


Review — PR #108 feat: add capture_redirect capability

  • Read CLAUDE.md conventions
  • Read PR diff
  • Review protocol changes (frames + validators)
  • Review server changes (captureRedirect + dispatch)
  • Review extension-core changes (handler + onBeforeRedirect)
  • Review test coverage
  • Post inline findings + summary

Verdict: ✅ pass (with 🟡 nits)

The implementation faithfully mirrors capture_request_header: same lazy-revive / SW-eviction flow, same validator shape (host / path / timeoutMs / unexpected-field rejection), same listener pattern (closure-captured resolved flag, try/catch on removeListener, timeout fallback). BridgeDownError.op union widened correctly. Reuses the existing webRequest manifest permission — no manifest delta needed. The single-hop semantic (only redirects whose source matches https://${host}${path} fire) matches the stated Cloudflare→presigned-S3 use case.

Test coverage on the surfaces that were touched is solid (12 protocol cases, 5 server-convenience cases). Extension-core handler is uncovered, but that matches the precedent for handleCaptureRequestHeaderRequest.

🟡 Nits (inline)

  1. docs/PRIVACY.md:33 — capabilities table omits capture_redirect. User-facing privacy surface; should grow with new caps.
  2. docs/store-assets/permission-justifications.md:49-56 — webRequest justification claims it's used only for capture_request_header. With this PR it also drives onBeforeRedirect for capture_redirect — worth updating before the next CWS submission.
  3. packages/extension-core/src/background.ts:1814-1822 — timeout fires after success and is a no-op; could clearTimeout on resolve. Matches the un-cleared pattern in the sibling header handler, so out-of-scope-ish.

🟣 Pre-existing (not blocking)

  • packages/extension-chrome/README.md "Manifest highlights" never documented webRequest/cookies/tabs/scripting. The CLAUDE.md rule about documenting new chrome.* API usage in the highlights section would be a good moment to backfill, but every prior PR using these has skipped it too.

Notes / non-findings

  • bridgeReviveDelayMs ?? 0 defensive default in the new method mirrors captureRequestHeader; the constructor always seeds it to 2000, so the fallback is dead code but consistent with prior art.
  • Validator's CAPTURE_PATH_RE allows path: '/', which yields a literal-root filter (only https://host/ matches, no subpaths). Same semantic as captureRequestHeader; intentional and OK.
  • No new console.log/debug/info calls — stdio rule respected.
    • Branch: feat/capture-redirect

Comment on lines +1814 to 1822
ok: false,
op: 'capture_redirect',
error: 'timeout',
});
}, timeoutMs);
}

async function handleReadIndexedDbRequest(
mcpId: string,

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 — consider clearing the timeout on resolve.

The 30 s timeout fires even after the listener successfully resolves; if (resolved) return makes it a no-op, but you'd avoid the dangling timer (and a tiny memory hold on the listener closure) by clearTimeout-ing in the listener and capturing the handle.

This matches the existing handleCaptureRequestHeaderRequest pattern at line 1717 — also un-cleared — so this is consistent with prior art and arguably out-of-scope for this PR. Flagging only because a small refactor would clean both up at once.

@chrischall chrischall added the ready-to-merge Arms auto-merge — added by the pipeline on a pass/warn verdict, never by hand label Jun 4, 2026
@chrischall
chrischall merged commit 3121dc5 into main Jun 4, 2026
9 checks passed
@chrischall
chrischall deleted the feat/capture-redirect branch June 4, 2026 19:25
chrischall added a commit that referenced this pull request Jun 4, 2026
🤖 I have created a release *beep* *boop*
---


##
[1.1.0](v1.0.1...v1.1.0)
(2026-06-04)


### Features

* add capture_redirect capability
([#108](#108))
([3121dc5](3121dc5))

---
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 Auto-review pipeline is handling this PR ready-to-merge Arms auto-merge — added by the pipeline on a pass/warn verdict, never by hand

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant