ext: pairing-restart test + AMO permission justification
tests/transport/storage.test.ts covers the storage half of "the pairing token survives a browser restart" (round-trip, unpair-clears, corrupted override falls back to auto). websocket.test.ts adds the transport half: a fresh WebSocketTransport instance over the same backing store reuses the persisted token with no re-pairing, plus pairWithCode/unpair coverage. "Wrong token rejected and rate-limited" was already covered (websocket.test.ts's NotPaired/RateLimited cases). docs/amo-permissions.md is the submission-ready permission justification for AMO's Notes to Reviewer field, covering every permission in manifest.json plus what was deliberately not requested and how cookie/ header data is handled. Co-Authored-By: Claude Sonnet 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Ed8KEmAW48v4YHdxLtqsMB
This commit is contained in:
@@ -0,0 +1,49 @@
|
||||
# AMO permission justification
|
||||
|
||||
Submitted with every version bump in the AMO "Notes to Reviewer" field. Kept here so the
|
||||
justification is reviewed and versioned alongside the permission list itself
|
||||
(`manifest.json`), instead of living only in a web form. docs/05 §7 is the design-time
|
||||
version of this; this file is the submission-ready copy.
|
||||
|
||||
## What Velox is
|
||||
|
||||
A download manager. It intercepts a response Firefox is about to download, hands the
|
||||
URL, request headers, and cookies to a companion native application (`veloxd`), and lets
|
||||
that application fetch the file with resumable, multi-connection transfers. The
|
||||
extension itself never stores or transfers file bytes — see `CLAUDE.md` §3 and the
|
||||
`no-download-logic` ESLint gate (`eslint.config.mjs`) that fails CI if it ever does.
|
||||
|
||||
## Permissions requested
|
||||
|
||||
| Permission | Why | Narrowest alternative considered |
|
||||
|---|---|---|
|
||||
| `webRequest` + `webRequestBlocking` | The whole feature: inspect response headers on `onHeadersReceived` to decide whether to intercept a download, and `{cancel: true}` before Firefox starts its own download. This is the one thing MV3 Chrome removed and MV3 Firefox kept — it's why the extension can exist as designed (docs/05 §1). | None. Without blocking `webRequest` there is no way to stop Firefox's own download before it starts; polling `downloads.onCreated` alone (which we also use, see below) only catches what already started. |
|
||||
| `downloads` | Belt-and-braces safety net (`capture/downloads-api.ts`): some downloads (form POSTs, service-worker blobs) never reach `onHeadersReceived` in a way we can act on and only surface via `downloads.onCreated`. Also used to `cancel`/`erase` a download we're taking over so Firefox doesn't keep two copies. | Drop the safety net and accept that those cases silently bypass Velox. Rejected — docs/05 §2 calls this out explicitly as a known gap the safety net exists to close. |
|
||||
| `cookies` | A file behind a login (private CDN links, forum attachments) needs its session cookies handed to `veloxd`, or the daemon's fetch gets a 403 the browser's own request wouldn't have. Read via `cookies.getAll(url)` only for a URL we are about to offer to the daemon — never harvested in bulk or logged. | Skip cookies and only support anonymous URLs. Rejected — it's a top user-facing IDM-parity feature and the reason people leave Chrome download managers behind. |
|
||||
| `contextMenus` | "Download with Velox" on a link/image/video, and "Download all links…" (docs/05 §3). Table-stakes UI for a download manager extension. | None smaller — there's no partial grant for context menus. |
|
||||
| `storage` | `browser.storage.local` holds only extension-local state: the WebSocket pairing token, the manual transport override, the last-good WS port, and the user's default-category preference (`transport/storage.ts`, `options/prefs.ts`). No browsing data. | None — some persistence is required for pairing to survive a restart (M1 DoD), which is the point of the token existing at all. |
|
||||
| `notifications` | Tells the user when the daemon can't be reached for a download that fell back to Firefox, and (native-messaging path) surfaces pairing prompts if the GUI isn't running. | Silent failure. Rejected — capture fails open by design (CLAUDE.md §4) and a silent fallback with no notification would look like a bug. |
|
||||
| `nativeMessaging` | Opportunistic transport to `veloxd` over a Unix socket, for installs where it works (docs/adr/0003). Not the default path — WebSocket is — but shipped because it avoids the WebSocket port-scan on installs where the native host manifest is reachable. | Drop native messaging and use WebSocket exclusively. Considered and rejected in ADR 0003: keeping both means the extension keeps working across deb/snap/flatpak Firefox without per-flavour capture-logic forks. |
|
||||
| `<all_urls>` (host permission) | Downloads happen from every site on the web; `webRequest`'s header inspection and `cookies.getAll` both need to run against whatever site the user is on. This is the item AMO reviewers push back on hardest for extensions of this shape. | A fixed list of "known download sites" — unworkable for a general-purpose download manager, and defeats the point of an IDM-style interceptor. **Mitigation, not a narrower permission:** the exclusion list in Options is front-and-center (`options.html` → "Capture policy") so a user can scope capture down to nothing on sites they don't want Velox touching, and the bypass modifier (default Alt) lets a single click skip capture without changing settings. |
|
||||
|
||||
## What is explicitly *not* requested
|
||||
|
||||
- No `<all_urls>` XHR/fetch use — `webRequest`/`cookies` read metadata about a request
|
||||
Firefox is already making; the extension never issues its own network request for
|
||||
file bytes (enforced by the ESLint gate above).
|
||||
- No `identity`, `history`, `bookmarks`, `tabs` beyond what `contextMenus`/`commands`
|
||||
already imply, `management`, or any permission unrelated to capturing and handing off
|
||||
a download.
|
||||
- No remote code: the manifest ships no CDN scripts and no `eval`; `web-ext lint` fails
|
||||
the build otherwise (CI's `extension-lint` job).
|
||||
|
||||
## Data handling
|
||||
|
||||
- Cookies and headers are held in memory only long enough to answer one
|
||||
`capture.offer` call to the local daemon (`capture/headers.ts`'s ring buffer, 5-minute
|
||||
TTL) — never written to disk by the extension and never sent anywhere but
|
||||
`127.0.0.1`.
|
||||
- The daemon connection is local-only: `WebSocketTransport` connects to
|
||||
`ws://127.0.0.1:<port>`, never a remote host (docs/05 §4, conformance-tested).
|
||||
- Nothing is sent to Anthropic, Mozilla, or any third party beyond the user's own local
|
||||
`veloxd` process.
|
||||
@@ -62,6 +62,11 @@ export interface VeloxTransport {
|
||||
|
||||
/** Fires on every state change. Returns an unsubscribe. */
|
||||
onStateChange(cb: (status: TransportStatus) => void): () => void;
|
||||
|
||||
/** WebSocket transport only (pairing has no meaning over native messaging's uds
|
||||
* socket, which has no token). Options renders these controls only when present. */
|
||||
pairWithCode?(code: string): Promise<void>;
|
||||
unpair?(): Promise<void>;
|
||||
}
|
||||
|
||||
// --- errors ---------------------------------------------------------------------------
|
||||
|
||||
@@ -97,6 +97,9 @@ export class WebSocketTransport implements VeloxTransport {
|
||||
private stopped = false;
|
||||
private connectPromise: Promise<void> | null = null;
|
||||
private reconnectTimer: ReturnType<typeof setTimeout> | null = null;
|
||||
/** Set only for the duration of pairWithCode(); consumed by pair(). docs/05 §4: "the
|
||||
* user clicks Allow (or types the code in the extension options)." */
|
||||
private pendingPairCode: string | null = null;
|
||||
|
||||
private readonly listeners = new Map<string, Set<EventListener>>();
|
||||
private readonly stateListeners = new Set<(status: TransportStatus) => void>();
|
||||
@@ -139,6 +142,29 @@ export class WebSocketTransport implements VeloxTransport {
|
||||
return this.connectPromise;
|
||||
}
|
||||
|
||||
/**
|
||||
* Options → "Pair" with a code typed from the daemon's dialog, for when the GUI isn't
|
||||
* running to click Allow (docs/05 §4). Drops any stored token first so the handshake
|
||||
* takes the pairing branch, then reconnects with the code attached.
|
||||
*/
|
||||
async pairWithCode(code: string): Promise<void> {
|
||||
this.disconnect();
|
||||
await this.deps.setToken(null); // drop any stale token so the handshake takes the pairing branch
|
||||
this.pendingPairCode = code;
|
||||
try {
|
||||
await this.connect();
|
||||
} finally {
|
||||
this.pendingPairCode = null;
|
||||
}
|
||||
}
|
||||
|
||||
/** Options → "Unpair": revoke the local token. The daemon's own record of it is
|
||||
* cleaned up on its side; this only ever forgets our copy. */
|
||||
async unpair(): Promise<void> {
|
||||
await this.deps.setToken(null);
|
||||
this.disconnect();
|
||||
}
|
||||
|
||||
disconnect(): void {
|
||||
this.stopped = true;
|
||||
if (this.reconnectTimer) {
|
||||
@@ -322,6 +348,7 @@ export class WebSocketTransport implements VeloxTransport {
|
||||
const params: SessionPairParams = {
|
||||
clientName: this.clientName,
|
||||
extensionId: this.deps.extensionId,
|
||||
code: this.pendingPairCode,
|
||||
};
|
||||
try {
|
||||
const res = (await rpc.request(
|
||||
|
||||
@@ -0,0 +1,56 @@
|
||||
// browser.storage.local is backed by the real add-on profile on disk, so anything
|
||||
// written through transport/storage.ts is what "survives a browser restart" means in
|
||||
// practice — this covers the storage half of that DoD item; websocket.test.ts's
|
||||
// "survives a browser restart" case covers the transport half (a fresh transport
|
||||
// instance reusing a persisted token with no re-pairing).
|
||||
import { beforeEach, describe, expect, it } from 'vitest';
|
||||
|
||||
import * as storage from '../../src/background/transport/storage.js';
|
||||
|
||||
describe('transport/storage', () => {
|
||||
beforeEach(async () => {
|
||||
await browser.storage.local.clear();
|
||||
});
|
||||
|
||||
it('round-trips the pairing token', async () => {
|
||||
expect(await storage.getToken()).toBeNull();
|
||||
await storage.setToken('tok-abc');
|
||||
expect(await storage.getToken()).toBe('tok-abc');
|
||||
});
|
||||
|
||||
it('clears the token on unpair (null)', async () => {
|
||||
await storage.setToken('tok-abc');
|
||||
await storage.setToken(null);
|
||||
expect(await storage.getToken()).toBeNull();
|
||||
});
|
||||
|
||||
it('a fresh read after a simulated restart still sees the persisted token', async () => {
|
||||
await storage.setToken('tok-survives');
|
||||
// Nothing here recreates browser.storage.local — that's the point: it is the one
|
||||
// thing in the extension that outlives the background page's lifetime, restart
|
||||
// included. A second, independent read call stands in for "the page reloaded".
|
||||
expect(await storage.getToken()).toBe('tok-survives');
|
||||
expect(await storage.getToken()).toBe('tok-survives');
|
||||
});
|
||||
|
||||
it('round-trips the transport override, defaulting to auto', async () => {
|
||||
expect(await storage.getOverride()).toBe('auto');
|
||||
await storage.setOverride('uds');
|
||||
expect(await storage.getOverride()).toBe('uds');
|
||||
await storage.setOverride('ws');
|
||||
expect(await storage.getOverride()).toBe('ws');
|
||||
});
|
||||
|
||||
it('ignores a corrupted override value and falls back to auto', async () => {
|
||||
await browser.storage.local.set({ 'velox.transportOverride': 'not-a-transport' });
|
||||
expect(await storage.getOverride()).toBe('auto');
|
||||
});
|
||||
|
||||
it('round-trips the cached WebSocket port', async () => {
|
||||
expect(await storage.getCachedWsPort()).toBeNull();
|
||||
await storage.setCachedWsPort(52003);
|
||||
expect(await storage.getCachedWsPort()).toBe(52003);
|
||||
await storage.setCachedWsPort(null);
|
||||
expect(await storage.getCachedWsPort()).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -93,6 +93,59 @@ describe('WebSocketTransport', () => {
|
||||
expect(lastHello?.token).toBe('tok-issued-1');
|
||||
});
|
||||
|
||||
it('the pairing token survives a browser restart: a fresh transport instance over the same storage reuses it, no re-pairing', async () => {
|
||||
const port = nextPort();
|
||||
daemon = await FakeDaemon.start({ port, acceptToken: null });
|
||||
const h = memDeps(); // stands in for browser.storage.local, which outlives the page
|
||||
transport = makeTransport(port, h);
|
||||
|
||||
await transport.connect();
|
||||
expect(daemon.pairCount).toBe(1);
|
||||
expect(h.store.token).toBe('tok-issued-1');
|
||||
transport.disconnect();
|
||||
|
||||
// Simulate "the browser restarted": a brand new transport instance, same backing
|
||||
// store (in reality, the same on-disk profile), no in-memory state carried over.
|
||||
const restarted = makeTransport(port, h);
|
||||
await restarted.connect();
|
||||
try {
|
||||
expect(restarted.state).toBe('connected');
|
||||
expect(daemon.pairCount).toBe(1); // still just the one pairing, ever
|
||||
const lastHello = [...daemon.seen].reverse().find((s) => s.method === 'session.hello');
|
||||
expect(lastHello?.token).toBe('tok-issued-1');
|
||||
} finally {
|
||||
restarted.disconnect();
|
||||
}
|
||||
});
|
||||
|
||||
it('pairWithCode drops any stale token and pairs fresh with the typed code', async () => {
|
||||
const port = nextPort();
|
||||
daemon = await FakeDaemon.start({ port, acceptToken: 'stale' });
|
||||
const h = memDeps({ token: 'stale' });
|
||||
transport = makeTransport(port, h);
|
||||
|
||||
await transport.pairWithCode('4821');
|
||||
|
||||
expect(transport.state).toBe('connected');
|
||||
expect(daemon.pairCount).toBe(1);
|
||||
const pairCall = daemon.seen.find((s) => s.method === 'session.pair');
|
||||
expect((pairCall?.params as { code?: string }).code).toBe('4821');
|
||||
expect(h.store.token).toBe('tok-issued-1');
|
||||
});
|
||||
|
||||
it('unpair clears the stored token and disconnects', async () => {
|
||||
const port = nextPort();
|
||||
daemon = await FakeDaemon.start({ port, acceptToken: 'good-token' });
|
||||
const h = memDeps({ token: 'good-token' });
|
||||
transport = makeTransport(port, h);
|
||||
await transport.connect();
|
||||
|
||||
await transport.unpair();
|
||||
|
||||
expect(h.store.token).toBeNull();
|
||||
expect(transport.state).toBe('disconnected');
|
||||
});
|
||||
|
||||
it('with autoPair off, a wrong token surfaces needsPairing and does NOT retry', async () => {
|
||||
const port = nextPort();
|
||||
daemon = await FakeDaemon.start({ port, acceptToken: 'the-real-one' });
|
||||
|
||||
Reference in New Issue
Block a user