From b3d477bbb52beaa68c732a004e8f4038bd8dafd5 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Fri, 14 Aug 2026 17:02:54 -0700 Subject: [PATCH] fix(extension): deny token/port reads to content-script and foreign senders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit background.js answered getPort — port, connected state, AND the browse server auth token — to any sender that passed the type allowlist, including content scripts running in web-page context and, behind only the sender.id check, anything without extension-page provenance. The getToken sender.tab restriction covered getToken alone, and only after getPort had already handed out the token. Single decision point now: extension/sender-auth.js classifies each message type; the eight privileged types (getPort, setPort, getServerUrl, getToken, fetchRefs, command, sidebar-command, getTabState) require an own-extension-page sender (chrome-extension:/// URL, no sender.tab, own sender.id). Denied senders get { error: 'unauthorized' } and nothing else — never the token, never the port. Content-script flows (elementPicked, pickerCancelled, inspectResult, openSidePanel) are untouched, and the sidepanel/popup keep the getPort token field their connect path reads. The policy mirrors the v1.63 server-side model: AUTH_TOKEN is released only to the pinned extension Origin via POST /extension-token, so the extension must not re-leak it to contexts the server would never have trusted. browse/test/extension-sender-auth.test.ts drives the real background.js onMessage listener under a chrome stub with four sender shapes (own extension page, own content script, foreign extension id, missing sender.url) and pins that denied responses carry no token/port fields, that a denied setPort never persists, that a denied command never reaches the network, and that the inspector + tab-state flows keep working. The helper is loaded via importScripts in the classic service worker and require()-able from bun tests. Contributed by @punksterlabs (PR #1822; reimplemented against the v1.63 POST /extension-token pinned-origin model). Co-Authored-By: Claude Fable 5 --- browse/test/extension-sender-auth.test.ts | 271 ++++++++++++++++++++++ extension/background.js | 31 ++- extension/sender-auth.js | 65 ++++++ 3 files changed, 359 insertions(+), 8 deletions(-) create mode 100644 browse/test/extension-sender-auth.test.ts create mode 100644 extension/sender-auth.js diff --git a/browse/test/extension-sender-auth.test.ts b/browse/test/extension-sender-auth.test.ts new file mode 100644 index 000000000..df9fc1cda --- /dev/null +++ b/browse/test/extension-sender-auth.test.ts @@ -0,0 +1,271 @@ +/** + * Sender authorization for privileged extension messages. + * + * A content script runs in web-page context and can be influenced by page + * content; a foreign extension is not us. Neither may read or spend the + * browse server's auth token or port through background.js's message + * surface. PR #1822 (@punksterlabs) found getPort handing the token to any + * caller that passed the type allowlist; this suite pins the reimplemented + * gate BEHAVIORALLY — it drives the real background.js onMessage listener + * under a chrome stub with four sender shapes (own extension page, own + * content script, foreign extension, url-less) and asserts denied responses + * are { error: 'unauthorized' } with no token/port fields at all. + */ +import { describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; + +const EXT_DIR = path.join(import.meta.dir, '..', '..', 'extension'); +const BG_SRC = fs.readFileSync(path.join(EXT_DIR, 'background.js'), 'utf-8'); + +// eslint-disable-next-line @typescript-eslint/no-require-imports +const senderAuth = require(path.join(EXT_DIR, 'sender-auth.js')); + +// The pinned production id (derivable via browse/scripts/extension-id.ts) — +// the policy only compares it against sender.id, so any stable value works. +const OWN_ID = 'dgbkdbjebeiblbajiilljmhjdpmiglep'; +const FOREIGN_ID = 'ffffffffffffffffffffffffffffffff'; + +// ─── The four sender shapes ───────────────────────────────────── +const PAGE_SENDER = { id: OWN_ID, url: `chrome-extension://${OWN_ID}/sidepanel.html` }; +const CONTENT_SCRIPT_SENDER = { id: OWN_ID, url: 'https://evil.example/page', tab: { id: 42 } }; +const FOREIGN_SENDER = { id: FOREIGN_ID, url: `chrome-extension://${FOREIGN_ID}/background.html` }; +const NO_URL_SENDER = { id: OWN_ID }; + +const PRIVILEGED = [ + 'getPort', 'setPort', 'getServerUrl', 'getToken', 'fetchRefs', + 'command', 'sidebar-command', 'getTabState', +]; +// Content-script-originated flows that must keep working. +const CONTENT_SCRIPT_TYPES = ['openSidePanel', 'elementPicked', 'pickerCancelled', 'inspectResult']; +// Sidepanel-originated, non-privileged (page effects only, no token/port). +const PAGE_EFFECT_TYPES = ['sidebarOpened', 'startInspector', 'stopInspector', 'applyStyle', 'toggleClass', 'injectCSS', 'resetAll']; + +const LEAK_FIELDS = ['token', 'authToken', 'port', 'url', 'connected', 'tabs', 'active', 'ok']; + +// ─── Unit: the policy predicate ───────────────────────────────── + +describe('sender-auth policy (unit)', () => { + test('own extension page is allowed for every privileged type', () => { + for (const type of PRIVILEGED) { + expect(senderAuth.denialFor(type, PAGE_SENDER, OWN_ID)).toBeNull(); + } + expect(senderAuth.isExtensionPageSender(PAGE_SENDER, OWN_ID)).toBe(true); + }); + + test('own popup page is allowed (any own-extension page path)', () => { + const popup = { id: OWN_ID, url: `chrome-extension://${OWN_ID}/popup.html` }; + expect(senderAuth.denialFor('getPort', popup, OWN_ID)).toBeNull(); + }); + + test('own content script (sender.tab + page URL) is denied for every privileged type', () => { + for (const type of PRIVILEGED) { + const denial = senderAuth.denialFor(type, CONTENT_SCRIPT_SENDER, OWN_ID); + expect(denial).toEqual({ error: 'unauthorized' }); + expect(Object.keys(denial)).toEqual(['error']); + } + }); + + test('foreign extension id is denied for every privileged type', () => { + for (const type of PRIVILEGED) { + expect(senderAuth.denialFor(type, FOREIGN_SENDER, OWN_ID)).toEqual({ error: 'unauthorized' }); + } + }); + + test('missing sender.url is denied (no provenance)', () => { + for (const type of PRIVILEGED) { + expect(senderAuth.denialFor(type, NO_URL_SENDER, OWN_ID)).toEqual({ error: 'unauthorized' }); + } + expect(senderAuth.denialFor('getToken', undefined, OWN_ID)).toEqual({ error: 'unauthorized' }); + }); + + test('own extension page opened inside a TAB is denied (conservative: sender.tab wins)', () => { + const pageInTab = { id: OWN_ID, url: `chrome-extension://${OWN_ID}/sidepanel.html`, tab: { id: 7 } }; + expect(senderAuth.denialFor('getToken', pageInTab, OWN_ID)).toEqual({ error: 'unauthorized' }); + }); + + test('non-privileged types are never gated here — content-script flows stay reachable', () => { + for (const type of [...CONTENT_SCRIPT_TYPES, ...PAGE_EFFECT_TYPES]) { + expect(senderAuth.denialFor(type, CONTENT_SCRIPT_SENDER, OWN_ID)).toBeNull(); + expect(senderAuth.denialFor(type, PAGE_SENDER, OWN_ID)).toBeNull(); + } + }); +}); + +// ─── Behavioral: the real background.js listener ──────────────── + +type Listener = (msg: unknown, sender: unknown, sendResponse: (r: unknown) => void) => unknown; + +function loadBackground() { + const captured: { listener?: Listener } = {}; + const calls = { storageSet: [] as unknown[], fetch: [] as unknown[] }; + const never = new Promise(() => {}); // storage.get never settles → startup health polling never starts + const chromeStub = { + runtime: { + id: OWN_ID, + onMessage: { addListener: (fn: Listener) => { captured.listener = fn; } }, + onInstalled: { addListener: () => {} }, + sendMessage: () => Promise.resolve(), + }, + storage: { + local: { + get: () => never, + set: (obj: unknown) => { calls.storageSet.push(obj); return Promise.resolve(); }, + }, + }, + tabs: { + onActivated: { addListener: () => {} }, + onCreated: { addListener: () => {} }, + onRemoved: { addListener: () => {} }, + onUpdated: { addListener: () => {} }, + query: (_opts: unknown, cb?: (tabs: unknown[]) => void) => { + if (cb) { cb([]); return; } + return Promise.resolve([]); + }, + sendMessage: () => Promise.resolve(), + get: () => {}, + }, + action: { setBadgeBackgroundColor: () => {}, setBadgeText: () => {} }, + scripting: { executeScript: () => Promise.resolve(), insertCSS: () => Promise.resolve() }, + // no chrome.sidePanel: autoOpenSidePanel exits immediately (no retry timers) + }; + const fetchSpy = (...args: unknown[]) => { + calls.fetch.push(args); + return Promise.reject(new Error('no network in tests')); + }; + // background.js is a classic (non-module) service worker script — evaluate + // it with its globals injected. importScripts is satisfied by passing the + // already-required sender-auth module under the global name it registers. + const run = new Function('chrome', 'importScripts', 'gstackSenderAuth', 'fetch', BG_SRC); + run(chromeStub, () => {}, senderAuth, fetchSpy); + if (!captured.listener) throw new Error('background.js did not register an onMessage listener'); + return { listener: captured.listener, calls }; +} + +function dispatch(listener: Listener, msg: unknown, sender: unknown) { + const result = { responded: false, response: undefined as Record | undefined }; + listener(msg, sender, (resp: unknown) => { + result.responded = true; + result.response = resp as Record; + }); + return result; +} + +// Denied senders get { error: 'unauthorized' } and nothing else — or no +// response at all (the pre-existing foreign-sender early return). Either +// way: never a token, port, or tab-state field. +function expectDenied(result: ReturnType) { + if (result.responded) { + expect(result.response).toEqual({ error: 'unauthorized' }); + expect(Object.keys(result.response!)).toEqual(['error']); + } + const resp = result.response ?? {}; + for (const leak of LEAK_FIELDS) { + expect(resp[leak]).toBeUndefined(); + } +} + +describe('background.js onMessage listener (behavioral)', () => { + const { listener, calls } = loadBackground(); + + test('own sidepanel page: getPort responds with port/connected/token fields, no error', () => { + const r = dispatch(listener, { type: 'getPort' }, PAGE_SENDER); + expect(r.responded).toBe(true); + expect('port' in r.response!).toBe(true); + expect('connected' in r.response!).toBe(true); + // The sidepanel's tryConnect reads resp.token — the field must exist for + // extension pages (value is null until the token bootstrap completes). + expect('token' in r.response!).toBe(true); + expect(r.response!.error).toBeUndefined(); + }); + + test('own sidepanel page: getToken responds with a token field', () => { + const r = dispatch(listener, { type: 'getToken' }, PAGE_SENDER); + expect(r.responded).toBe(true); + expect('token' in r.response!).toBe(true); + expect(r.response!.error).toBeUndefined(); + }); + + test('own content script: every privileged type is denied with no token/port fields', () => { + for (const type of PRIVILEGED) { + const r = dispatch(listener, { type }, CONTENT_SCRIPT_SENDER); + expect(r.responded).toBe(true); // the gate answers, it does not go silent + expectDenied(r); + } + }); + + test('foreign extension: every privileged type yields no token/port fields', () => { + for (const type of PRIVILEGED) { + expectDenied(dispatch(listener, { type }, FOREIGN_SENDER)); + } + }); + + test('missing sender.url: every privileged type is denied', () => { + for (const type of PRIVILEGED) { + const r = dispatch(listener, { type }, NO_URL_SENDER); + expect(r.responded).toBe(true); + expectDenied(r); + } + }); + + test('denied setPort never persists the attacker port', () => { + const before = calls.storageSet.length; + const r = dispatch(listener, { type: 'setPort', port: 6666 }, CONTENT_SCRIPT_SENDER); + expectDenied(r); + expect(calls.storageSet.length).toBe(before); + }); + + test('denied command never reaches the network and fails at the gate, not the handler', () => { + const before = calls.fetch.length; + const r = dispatch(listener, { type: 'command', command: 'goto', args: ['https://evil.example'] }, CONTENT_SCRIPT_SENDER); + // 'unauthorized' proves the gate fired; the handler's own failure mode is + // 'Not connected to browse server'. + expect(r.response).toEqual({ error: 'unauthorized' }); + expect(calls.fetch.length).toBe(before); + }); + + test('content script can still run the inspector flow (elementPicked → ok)', async () => { + const r = dispatch( + listener, + { type: 'elementPicked', selector: '#hero', tagName: 'div', classes: [], id: null, dimensions: { width: 1, height: 1 } }, + CONTENT_SCRIPT_SENDER, + ); + await new Promise((res) => setTimeout(res, 10)); + expect(r.response).toEqual({ ok: true }); + }); + + test('content script can still request openSidePanel (not rejected as unauthorized)', () => { + const r = dispatch(listener, { type: 'openSidePanel' }, CONTENT_SCRIPT_SENDER); + // chrome.sidePanel is absent in the stub so the handler is a no-op — the + // load-bearing assertion is that the gate did not deny it. + expect(r.response?.error).toBeUndefined(); + }); + + test('sidepanel getTabState still works (terminal pane tab sync)', async () => { + const r = dispatch(listener, { type: 'getTabState' }, PAGE_SENDER); + await new Promise((res) => setTimeout(res, 10)); + expect(r.responded).toBe(true); + expect(r.response).toEqual({ active: null, tabs: [] }); + }); +}); + +// ─── Wiring tripwire ──────────────────────────────────────────── +// The behavioral suite injects senderAuth directly, so pin that the real +// worker actually loads it: importScripts of the helper file plus a +// denialFor call in the listener. A refactor that drops either fails here. + +describe('background.js ↔ sender-auth.js wiring', () => { + test('background.js importScripts sender-auth.js (classic worker load path)', () => { + expect(BG_SRC).toContain("importScripts('sender-auth.js')"); + }); + + test('background.js consults gstackSenderAuth.denialFor in the message listener', () => { + expect(BG_SRC).toContain('gstackSenderAuth.denialFor(msg.type, sender, chrome.runtime.id)'); + }); + + test('manifest keeps a classic (non-module) service worker — importScripts requires it', () => { + const manifest = JSON.parse(fs.readFileSync(path.join(EXT_DIR, 'manifest.json'), 'utf-8')); + expect(manifest.background.service_worker).toBe('background.js'); + expect(manifest.background.type).toBeUndefined(); + }); +}); diff --git a/extension/background.js b/extension/background.js index 249bd9f05..7b9ee7a6f 100644 --- a/extension/background.js +++ b/extension/background.js @@ -5,8 +5,14 @@ * Fetches /refs on snapshot completion, relays to content script. * Proxies commands from sidebar → browse server. * Updates badge: amber (connected), gray (disconnected). + * Denies token/port reads to content-script and foreign senders. */ +// Sender authorization for privileged message types (the token/port surface). +// Classic (non-module) service worker: importScripts puts gstackSenderAuth on +// the worker global. The same file is require()-able from bun tests. +importScripts('sender-auth.js'); + const DEFAULT_PORT = 34567; // Well-known port used by `$B connect` let serverPort = null; let authToken = null; @@ -309,6 +315,19 @@ chrome.runtime.onMessage.addListener((msg, sender, sendResponse) => { return; } + // Privileged types — anything that returns or spends the auth token or + // server port, or dumps the full tab list — are for this extension's own + // pages only (sidepanel/popup). Content scripts run inside web pages and + // can be influenced by page content; foreign extensions are foreign. Both + // get { error: 'unauthorized' } and nothing else — never the token, never + // the port. Policy + type list live in sender-auth.js. + const denial = gstackSenderAuth.denialFor(msg.type, sender, chrome.runtime.id); + if (denial) { + console.warn('[gstack] Rejected privileged message from unauthorized sender:', msg.type, sender.url || '(no sender url)'); + sendResponse(denial); + return true; + } + if (msg.type === 'getPort') { sendResponse({ port: serverPort, connected: isConnected, token: authToken }); return true; @@ -333,15 +352,11 @@ chrome.runtime.onMessage.addListener((msg, sender, sendResponse) => { } // Token delivered via targeted sendResponse, not broadcast — limits exposure. - // Only respond to extension pages (sidepanel/popup) — content scripts have - // sender.tab set, so reject those to prevent token access from injected contexts. + // Only this extension's own pages reach here: the sender-auth gate above + // denies content scripts (sender.tab set) and foreign senders before any + // privileged handler runs. if (msg.type === 'getToken') { - if (sender.tab) { - console.warn('[gstack] Rejected getToken from content script context'); - sendResponse({ token: null }); - } else { - sendResponse({ token: authToken }); - } + sendResponse({ token: authToken }); return true; } diff --git a/extension/sender-auth.js b/extension/sender-auth.js new file mode 100644 index 000000000..b05bb4a7b --- /dev/null +++ b/extension/sender-auth.js @@ -0,0 +1,65 @@ +/** + * gstack browse — sender authorization for privileged extension messages + * + * Single decision point for which chrome.runtime.onMessage senders may read + * or spend the browse server's auth token and port. Loaded into the + * background service worker via importScripts() (classic worker — see + * manifest.json) and require()-able from bun tests + * (browse/test/extension-sender-auth.test.ts). + * + * Policy: privileged types are for this extension's own pages only + * (sidepanel / popup — sender.url is chrome-extension:///...). + * Content scripts run in web-page context (sender.tab is set, sender.url is + * the page URL) and can be influenced by page content; foreign extensions + * have a different sender.id. Both are denied, and a denied sender gets + * { error: 'unauthorized' } with no other fields — never the token, never + * the port. This mirrors the server side of the v1.63 token model: the + * browse server releases AUTH_TOKEN only to the pinned extension Origin via + * POST /extension-token, so the extension must not re-leak it to contexts + * the server would never have trusted. + */ +(function (root) { + 'use strict'; + + // Message types that return or spend the auth token / server port, or leak + // privileged browser state. Every other type in background.js's allowlist + // stays reachable from content scripts — the inspector flow (elementPicked, + // pickerCancelled, inspectResult) and openSidePanel are content-script- + // originated by design. + const PRIVILEGED_TYPES = new Set([ + 'getPort', // response carries port + connected state + token + 'setPort', // repoints the token-bearing client at another port + 'getServerUrl', // response carries the server URL (port) + 'getToken', // response carries the token + 'fetchRefs', // spends the token on an authorized /refs fetch + 'command', // spends the token on an arbitrary browse command + 'sidebar-command', // spends the token on a server POST + 'getTabState', // response carries every open tab's URL + title + ]); + + // Extension-page senders only: popup / sidepanel / options. A content + // script has sender.tab set and a web-page sender.url; a foreign extension + // has a different sender.id; a sender with no URL has no provenance at all. + // All three are denied. + function isExtensionPageSender(sender, ownExtensionId) { + if (!sender || !ownExtensionId) return false; + if (sender.id !== ownExtensionId) return false; + if (sender.tab) return false; + if (typeof sender.url !== 'string') return false; + return sender.url.startsWith('chrome-extension://' + ownExtensionId + '/'); + } + + // Returns null when the message may proceed, or the exact response a + // denied sender receives: { error: 'unauthorized' } and nothing else. + function denialFor(msgType, sender, ownExtensionId) { + if (!PRIVILEGED_TYPES.has(msgType)) return null; + if (isExtensionPageSender(sender, ownExtensionId)) return null; + return { error: 'unauthorized' }; + } + + const api = { PRIVILEGED_TYPES, isExtensionPageSender, denialFor }; + if (typeof module !== 'undefined' && module.exports) { + module.exports = api; // bun test (CommonJS require) + } + root.gstackSenderAuth = api; // importScripts() in the service worker +})(typeof self !== 'undefined' ? self : globalThis);