From 671bd9471dd11f140ef201071005820d56a0a841 Mon Sep 17 00:00:00 2001 From: sebi Date: Tue, 18 Aug 2026 20:48:25 -0500 Subject: [PATCH 1/2] fix(ui,scraping): a crafted icon executed at import, and a session cookie went to the wrong host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two proven security defects from a five-agent sweep, plus the accessibility set that shipped alongside them. RCE, proven exploitable. `build-icons.ts` interpolated attribute values from a NETWORK-FETCHED `icon-nodes.json` into generated TypeScript as `'${value}'`, unescaped. The validator checked tag names, attribute NAMES and `fill` — never the value of `d`, `points` or `cx`. A value that closes the string, the object and the array element and reopens all three yields a glyph module that runs arbitrary code in every app importing that icon. The regression test transpiles and EVALUATES the emitted module; the payload set a global before the fix and does not after. `icon-glyph.ts` already stated the principle — "data reaching an attribute sink unchecked is how `onload=` gets into the DOM" — and this was a code sink, which is worse. The 1767 committed glyphs are deliberately NOT regenerated: that would bury the fix in noise. A session cookie was sent to any host whose name merely ENDS WITH the cookie's domain. A host-only cookie for `bank.test` reached `evilbank.test` and `sub.bank.test` — proven both ways. The CDP jar is every domain the session ever touched, so an SSO hop's cookies rode along. Scoping is now one function implementing RFC 6265 §5.1.3 and §5.1.4 — domain and path, both boundaries, failing closed on an unparseable URL, and a `secure` cookie no longer reaches an `http:` URL. Path scoping was a third leak the audit had not named: the type already carried `path` and nothing read it. Two tenants shared one authenticated session. A declared `auth.key` replaced the whole session key instead of discriminating within the tenant, so `orgOf(ctx)` was never consulted and the value was never sanitised. `sessionKeyFor`'s third parameter is named `discriminator` and had no caller anywhere; it does now. Keyboard groups were unusable in three ways. A disabled item made everything past it unreachable — a disabled control cannot take focus, so the reducer returned its index forever — and if the disabled item was first, nothing in the group was tabbable at all. A Toolbar stole arrow keys from a text field inside it, which is its own documented use. `ToastRegion` was not a live region, which is exactly the failure its own header says the region/child split prevents. Deliberately NOT done: Toolbar's docs promised one Tab stop, and implementing it would be strictly worse than the bug — once a text field keeps its own arrows, a single stop TRAPS, and every button past the field becomes unreachable. The claim was deleted instead. `SessionSnapshot.headers` is dead on the only production driver and stays dead: reading headers off CDP would persist a cookie or an authorization into the session record, which `http.ts` then spreads onto every allowed host — a wider leak than the dead field. The divergence is pinned instead. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 50 +++ packages/scraping/CLAUDE.md | 11 +- packages/scraping/README.md | 9 +- packages/scraping/src/auth.test.ts | 10 + packages/scraping/src/auth.ts | 13 +- packages/scraping/src/cdp-port.ts | 6 + packages/scraping/src/cdp-target.test.ts | 184 +++++++++++ packages/scraping/src/cdp-target.ts | 64 +++- packages/scraping/src/cookie-scope.test.ts | 103 ++++++ packages/scraping/src/cookie-scope.ts | 84 +++++ packages/scraping/src/driver-cdp.test.ts | 128 ++++++++ packages/scraping/src/driver-cdp.ts | 36 ++- packages/scraping/src/driver-parity.test.ts | 35 +++ packages/scraping/src/error-throws.ts | 19 +- packages/scraping/src/expect.ts | 5 + packages/scraping/src/http-recorded.test.ts | 71 +++++ packages/scraping/src/http-recorded.ts | 8 + packages/scraping/src/http.test.ts | 45 +++ packages/scraping/src/http.ts | 16 +- packages/scraping/src/index.ts | 6 + packages/scraping/src/intercept.ts | 7 +- packages/scraping/src/offline-session.ts | 2 + packages/scraping/src/page-over-target.ts | 1 + packages/scraping/src/page.ts | 6 + packages/scraping/src/scrape-run.test.ts | 163 ++++++++++ packages/scraping/src/scrape-run.ts | 20 +- packages/scraping/src/scrape.test.ts | 9 +- packages/scraping/src/scrape.ts | 10 +- packages/scraping/src/session-state.ts | 9 +- packages/ui/CATALOG.md | 15 +- packages/ui/CLAUDE.md | 9 + packages/ui/README.md | 23 +- packages/ui/src/a11y.test.ts | 145 ++++++++- packages/ui/src/a11y.ts | 33 +- packages/ui/src/components/Checkbox.tsx | 9 +- packages/ui/src/components/Dialog.tsx | 7 +- packages/ui/src/components/Dropzone.tsx | 12 +- packages/ui/src/components/Field.tsx | 8 +- packages/ui/src/components/Form.tsx | 25 +- packages/ui/src/components/Menu.tsx | 16 +- packages/ui/src/components/Pagination.tsx | 9 +- packages/ui/src/components/Popover.tsx | 12 +- packages/ui/src/components/Select.tsx | 11 +- packages/ui/src/components/Switch.tsx | 15 +- packages/ui/src/components/Table.tsx | 8 +- packages/ui/src/components/Tabs.tsx | 18 +- packages/ui/src/components/Toast.tsx | 32 +- packages/ui/src/components/Toolbar.tsx | 8 +- packages/ui/src/components/file-input-view.ts | 25 ++ .../ui/src/components/interaction.test.ts | 293 ++++++++++++++++++ .../ui/src/components/style-classes.test.ts | 39 +++ packages/ui/src/errors.ts | 15 + packages/ui/src/fake-dom.ts | 179 +++++++++++ packages/ui/src/icons/build-icons.test.ts | 84 ++++- packages/ui/src/icons/build-icons.ts | 47 ++- packages/ui/src/index.ts | 10 + packages/ui/src/jsx-probe.ts | 98 ++++++ packages/ui/src/roving.test.ts | 61 ++++ packages/ui/src/roving.ts | 65 ++++ packages/ui/src/tokens/reset.scss | 7 + packages/ui/src/tokens/reset.test.ts | 24 ++ wiki/Error-Codes.md | 2 +- 62 files changed, 2368 insertions(+), 126 deletions(-) create mode 100644 packages/scraping/src/cdp-target.test.ts create mode 100644 packages/scraping/src/cookie-scope.test.ts create mode 100644 packages/scraping/src/cookie-scope.ts create mode 100644 packages/scraping/src/driver-cdp.test.ts create mode 100644 packages/scraping/src/http-recorded.test.ts create mode 100644 packages/scraping/src/scrape-run.test.ts create mode 100644 packages/ui/src/components/interaction.test.ts create mode 100644 packages/ui/src/components/style-classes.test.ts create mode 100644 packages/ui/src/fake-dom.ts create mode 100644 packages/ui/src/jsx-probe.ts create mode 100644 packages/ui/src/roving.test.ts create mode 100644 packages/ui/src/roving.ts create mode 100644 packages/ui/src/tokens/reset.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 43b74645..aeb6b97b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -252,6 +252,56 @@ Semver applies from 1.0.0. A breaking change to a documented API needs a major ### Fixed +- **SECURITY — the icon generator wrote network-fetched data into TypeScript source unescaped, so a + crafted glyph executed at import.** `build-icons.ts` emitted `${key}: '${value}'` where `value` + came from `icon-nodes.json` fetched over the network. `iconElements` validated tag names, + attribute *names* and the `fill` value — never the value of `d`, `points` or `cx`. A value that + closes the string, the object and the array element and reopens all three produces a glyph module + that runs arbitrary code **in every app that imports that icon**. **Proven exploitable**: the + regression test transpiles and evaluates the generated module, and the payload set a global before + the fix. `JSON.stringify` is the fix; a `SAFE_ATTR_VALUE` allowlist is defence in depth, added only + because it demonstrably refuses none of the 1767 committed glyphs. The sibling `icon-glyph.ts` + already stated the principle — *"data reaching an attribute sink unchecked is how `onload=` gets + into the DOM"* — and this was a **code** sink, which is worse. + +- **SECURITY — a scraped session's cookies were sent to hosts they do not belong to.** The match was + `host.endsWith(cookie.domain)`, with no dot boundary in either direction: a host-only cookie for + `bank.test` was sent to **`evilbank.test`** *and* to `sub.bank.test`. The CDP jar is every domain + the session ever touched, so an SSO hop's cookies rode along. Cookie scoping is now one function + implementing RFC 6265 §5.1.3 and §5.1.4 — domain **and** path, both boundaries — failing closed on + an unparseable URL, and a `secure` cookie no longer reaches an `http:` URL. Path scoping was a + third leak the audit had not named. + +- **SECURITY — two tenants shared one authenticated scrape session.** A declared `auth.key` replaced + the whole session key instead of discriminating within the tenant, so `orgOf(ctx)` was never + consulted and the value was never sanitised. `sessionKeyFor`'s third parameter is named + `discriminator` and had no caller anywhere — it does now. + +- **A disabled item made the rest of a `Menu` or `Tabs` unreachable by keyboard.** A disabled control + cannot take focus, so the roving reducer returned its index forever and arrow keys stopped dead. If + the disabled item was first, nothing in the group was tabbable at all. A `Toolbar` separately stole + arrow keys from a text field inside it — its own documented use — swallowing the caret move. + +- **`ToastRegion` was not a live region**, which is precisely the failure its own header says the + region/child split exists to prevent: each toast created a fresh live region with its content + already in it, which most screen readers do not announce. + +- **A file dropped on `Dropzone` never reached the form.** `onSelect` fired, `input.files` stayed + empty, and `required` blocked the submit — while `name`/`required` are props and native form + participation is the advertised contract. + +- **Seven more instances of the caught-value totality class**, in `@ultimat3/ai` and `@ultimat3/mcp`: + a tool result that could not be serialised took down the tool loop, a hostile provider rejection + escaped the gateway's retry classifier, and the MCP server's error renderer read four fields off a + value the framework did not build. A tool whose *output* is unserialisable now reports that the + tool **ran**, never that it failed — so the model does not re-buy the side effects. + +- Also: a refused scrape credential was walked back to the login form when `reuse: false`; + `robots: 'obey'` was unenforced on the offline HTTP leg every test exercises, so a `Disallow:`ed + endpoint replayed green and failed in production; a browser process leaked per failed attempt, and + the throws that caused it reached the job retry classifier as bare `Error`s with no code; and + restored `localStorage` was written on `about:blank` where it can never reach the site's origin. + - **The outbox relay's claim locked nothing, so two relays could publish one batch and a job could run twice.** `SQL_OUTBOX_CLAIM` ends in `for update skip locked`, but the relay issues it on a **pooled** connection with no transaction — a bare statement runs in an implicit transaction that diff --git a/packages/scraping/CLAUDE.md b/packages/scraping/CLAUDE.md index 106b664d..b26f3764 100644 --- a/packages/scraping/CLAUDE.md +++ b/packages/scraping/CLAUDE.md @@ -51,6 +51,10 @@ and a value cannot leak. `driver-parity.test.ts` runs the real driver's code pat | the fake never drifts from the real driver | `driver-parity.test.ts` runs one suite against `fake`, `fixture` and the puppeteer path, and pins the one honest divergence (no layout engine offline, so no box and no hit-target) | | an unrecorded request throws | `html-target.ts` and `http-recorded.ts` — an offline driver that fell through to the network would make a green suite secretly live | | `allowHosts` is enforced, never advisory | `intercept.ts` is the single decision, asked by every driver AND by the HTTP leg, before a byte leaves | +| robots is enforced on BOTH legs | `http-recorded.ts` takes the gate too (`http-recorded.test.ts`) — the offline leg is the one every test runs, so a `Disallow:`ed endpoint that only the live leg refuses is a rule no suite can see | +| a session cookie reaches one host | `cookie-scope.ts`, RFC 6265 §5.1.3/§5.1.4, pinned in `cookie-scope.test.ts`. The jar is `browser.cookies()` — every domain the session touched — so the boundary is a dot in both directions: `evilbank.test` is not `bank.test`, and a host-only cookie is not a subdomain's | +| a launched browser is never orphaned | `driver-cdp.ts`'s `opened()` rolls back with `browser.close()` on any throw between the launch and the `WedgeGuard` (`driver-cdp.test.ts`) — `runScrape`'s `finally` cannot close a session `open()` never returned | +| restored `localStorage` lands on its ORIGIN | `cdp-target.ts` defers the storage half to the first navigation that reaches `session.origin` (`cdp-target.test.ts`); `restore()` runs on `about:blank`, which has no storage to write to and is not the site | | Chrome is never needed for `bun test` | `fakeBrowser`/`fakePage` run on Bun's own `HTMLRewriter` | ## Logging @@ -66,7 +70,12 @@ never a value. - `secrets:` on the definition holds **names**; values are resolved in the worker, per attempt. - A `Secret` typed into a page **taints** it: `screenshot()` and `pdf()` then refuse (`X_SCRAPE_SECRET_EXPOSED`). Pixels cannot be redacted after the fact; `page.html()` can, and is. -- A session is credential material: tenant-scoped key, never logged, never an artifact. +- A session is credential material: tenant-scoped key, never logged, never an artifact. The key is + ALWAYS `sessionKeyFor({ scrape, tenant, discriminator })` — `auth.key` supplies the + discriminator and never the whole key, or two tenants naming one account share one authenticated + session, and a key that is also a storage path goes unsanitised (`scrape-run.test.ts`). +- The refusal tombstone is read BEFORE `reuse` is honoured: `reuse: false` means "do not restore + this session", never "present the rejected credential again" (`auth.ts`). - `X_SCRAPE_AUTH_FAILED` is registered `terminal`, so `executeJob` dead-letters it on the attempt that threw it (`packages/jobs/src/retry-classification.ts` — `nextRetryForError`). It ALSO writes a refusal into the session record, which is a different distance: `restorableSession()` reads it diff --git a/packages/scraping/README.md b/packages/scraping/README.md index fa329a49..2de5ef5b 100644 --- a/packages/scraping/README.md +++ b/packages/scraping/README.md @@ -74,8 +74,15 @@ browser's cookies, headers and proxy, the same `allowHosts`, the same robots gat limit, the same cancellation. `page.session()` exposes the handoff so an author can see what carried over. +The jar is scoped per request, RFC 6265: a cookie stored for `bank.test` is sent to `bank.test` +and to nothing else — not `evilbank.test`, not `sub.bank.test` — and only a domain-scoped +`.bank.test` reaches subdomains. `SessionSnapshot.headers` is the one field the real driver cannot +fill (CDP exposes no read for it, and `driver-parity.test.ts` pins that): a token the HTTP leg must +carry goes on the request, `http.request(url, { headers })`. + Both legs replay from **one** fixture directory (`fixtureBrowser(dir)`), so a hybrid run — browser -login, session handoff, HTTP bulk fetch — is tested end to end. +login, session handoff, HTTP bulk fetch — is tested end to end. Both legs apply the same robots +gate, offline included. ## What it owns diff --git a/packages/scraping/src/auth.test.ts b/packages/scraping/src/auth.test.ts index 7d6ad664..76c626af 100644 --- a/packages/scraping/src/auth.test.ts +++ b/packages/scraping/src/auth.test.ts @@ -121,6 +121,16 @@ describe('unit · validate decides, and an invalid session is burned before the }); describe('unit · a refused credential is never presented twice', () => { + test('reuse: false still refuses — the tombstone is read BEFORE the reuse decision', async () => { + const store = memorySessionStore(); + // `reuse: false` means "do not restore this session", never "present the rejected credential + // again": a second wrong password is what locks the account, and the record is the only thing + // that knows it was wrong. + const plan = planFor({ login: () => Promise.resolve(), store, reuse: false }); + await markRefused(plan); + expect(await codeOf(restorableSession(plan))).toBe('X_SCRAPE_AUTH_FAILED'); + }); + test('the refusal is written down, and the NEXT attempt fails before reaching a login form', async () => { const store = memorySessionStore(); const plan = planFor({ login: () => Promise.resolve(), store }); diff --git a/packages/scraping/src/auth.ts b/packages/scraping/src/auth.ts index 8be7ccc8..ef1b50f1 100644 --- a/packages/scraping/src/auth.ts +++ b/packages/scraping/src/auth.ts @@ -64,7 +64,12 @@ export interface ScrapeAuth { validate?(context: AuthContext): Promise; /** Where sessions live. Omitted means no reuse at all — every run logs in. */ readonly store?: ScrapeSessionStore | undefined; - /** Distinguishes two sessions for one scrape and one tenant — a second account, say. */ + /** + * The DISCRIMINATOR inside this tenant's key space — a second account, say. It is one segment of + * `sessionKeyFor({ scrape, tenant, discriminator })` and never the whole key: a value that + * replaced the key would put two tenants declaring the same account name on one authenticated + * session. Sanitised like every other segment, so it cannot escape the key space either. + */ key?(input: I): string; /** Reuse a stored session. `false` forces a fresh login every run. Defaults to `true`. */ readonly reuse?: boolean | undefined; @@ -106,10 +111,14 @@ export async function restorableSession( plan: AuthPlanInput, ): Promise { const store = plan.auth?.store; - if (store === undefined || plan.auth?.reuse === false) return undefined; + if (store === undefined) return undefined; const found = await store.load(plan.key); if (found === undefined) return undefined; + // The tombstone is read BEFORE `reuse` is honoured. `reuse: false` says "do not restore this + // session"; it does not say "present the rejected credential again", and reading it first meant + // a `reuse: false` scrape walked a refused password back to the login form on every requeue. if (found.refusedAt !== undefined) throw authFailed(plan.scrape, `refused at ${found.refusedAt}`); + if (plan.auth?.reuse === false) return undefined; const maxAge = plan.auth?.maxAge; if (maxAge !== undefined) { const age = plan.clock.now().getTime() - new Date(found.savedAt).getTime(); diff --git a/packages/scraping/src/cdp-port.ts b/packages/scraping/src/cdp-port.ts index bfe251ea..6ccb91d6 100644 --- a/packages/scraping/src/cdp-port.ts +++ b/packages/scraping/src/cdp-port.ts @@ -15,6 +15,12 @@ export interface CdpRequestLike { url(): string; resourceType(): string; + /** + * OPTIONAL, and read defensively: this is the shape of somebody else's event payload, so a + * launcher that predates the method (or a provider SDK that never had it) still satisfies the + * port and its requests are recorded as `GET` rather than crashing the interception handler. + */ + method?(): string; abort(): Promise; continue(): Promise; } diff --git a/packages/scraping/src/cdp-target.test.ts b/packages/scraping/src/cdp-target.test.ts new file mode 100644 index 00000000..01660d08 --- /dev/null +++ b/packages/scraping/src/cdp-target.test.ts @@ -0,0 +1,184 @@ +// The real driver's target, over a hand-built CDP page. Three things only this file can see: +// WHEN a restored session's `localStorage` is written, what a network entry says the method was, +// and what a console line says its level was — all three are read straight off the library's own +// event payloads, so the offline drivers cannot pin any of them. + +import { describe, expect, test } from 'bun:test'; +import type { CdpBrowserLike, CdpPageLike } from './cdp-port'; +import { cdpTarget } from './cdp-target'; +import { testClock } from './clock'; +import type { SessionSnapshot } from './session-state'; + +interface Recorder { + readonly page: CdpPageLike; + readonly browser: CdpBrowserLike; + /** Every `evaluate` expression and `goto`, in order, so ORDER is what the test asserts on. */ + readonly calls: readonly string[]; + emit(event: string, payload: unknown): void; +} + +const recorder = (start = 'about:blank'): Recorder => { + const calls: string[] = []; + const handlers = new Map void)[]>(); + let url = start; + const page: CdpPageLike = { + url: () => url, + goto: (next: string) => { + calls.push(`goto ${next}`); + url = next; + return Promise.resolve(undefined); + }, + content: () => Promise.resolve(''), + evaluate: (expression: string) => { + calls.push(`evaluate ${expression}`); + return Promise.resolve(undefined); + }, + click: () => Promise.resolve(), + type: () => Promise.resolve(), + select: () => Promise.resolve([]), + screenshot: () => Promise.resolve(new Uint8Array()), + pdf: () => Promise.resolve(new Uint8Array()), + setRequestInterception: () => Promise.resolve(), + on: (event: string, handler: (payload: unknown) => void) => { + const listeners = handlers.get(event) ?? []; + listeners.push(handler); + handlers.set(event, listeners); + return undefined; + }, + frames: () => [], + close: () => Promise.resolve(), + }; + return { + page, + browser: { + newPage: () => Promise.resolve(page), + setCookie: () => Promise.resolve(), + close: () => Promise.resolve(), + process: () => null, + }, + calls, + emit: (event, payload) => { + for (const handler of handlers.get(event) ?? []) handler(payload); + }, + }; +}; + +const SESSION: SessionSnapshot = { + cookies: [ + { name: 'sid', value: 'x', domain: 'shop.test', path: '/', httpOnly: true, secure: true }, + ], + headers: {}, + storage: { token: 'bearer-abc' }, + userAgent: 'agent', + origin: 'https://shop.test', +}; + +const open = (start?: string) => { + const rec = recorder(start); + return { + rec, + target: cdpTarget({ + page: rec.page, + browser: rec.browser, + rules: { allowHosts: ['*'] }, + clock: testClock(), + }), + }; +}; + +const storageWrites = (calls: readonly string[]): readonly string[] => + calls.filter((call) => call.includes('setItem')); + +describe('unit · restored localStorage lands on the session ORIGIN, never on about:blank', () => { + test('restore() before the first navigation writes no storage — an opaque origin has none', async () => { + const { rec, target } = open(); + await (await target).restore(SESSION); + expect(storageWrites(rec.calls)).toEqual([]); + }); + + test('it lands on the first navigation to the origin the session belongs to', async () => { + const { rec, target } = open(); + const page = await target; + await page.restore(SESSION); + await page.goto('https://shop.test/orders', { timeoutMs: 1_000 }); + expect(storageWrites(rec.calls)).toHaveLength(1); + expect(rec.calls.indexOf('goto https://shop.test/orders')).toBeLessThan( + rec.calls.findIndex((call) => call.includes('setItem')), + ); + }); + + test('and never on another origin — a bearer token is not handed to a site it is not for', async () => { + const { rec, target } = open(); + const page = await target; + await page.restore(SESSION); + await page.goto('https://other.test/', { timeoutMs: 1_000 }); + expect(storageWrites(rec.calls)).toEqual([]); + // Still pending: the run that finally reaches the site gets its session. + await page.goto('https://shop.test/orders', { timeoutMs: 1_000 }); + expect(storageWrites(rec.calls)).toHaveLength(1); + }); + + test('it is written once, not on every navigation', async () => { + const { rec, target } = open(); + const page = await target; + await page.restore(SESSION); + await page.goto('https://shop.test/a', { timeoutMs: 1_000 }); + await page.goto('https://shop.test/b', { timeoutMs: 1_000 }); + expect(storageWrites(rec.calls)).toHaveLength(1); + }); +}); + +describe('unit · a network entry says what the request actually was', () => { + const request = (url: string, method: string | undefined) => ({ + url: () => url, + resourceType: () => 'fetch', + ...(method === undefined ? {} : { method: () => method }), + abort: () => Promise.resolve(), + continue: () => Promise.resolve(), + }); + + test('a POST is recorded as a POST — page.network() is what X_SCRAPE_HTTP_FAILED points at', async () => { + const { rec, target } = open(); + const page = await target; + rec.emit('request', request('https://shop.test/api', 'POST')); + expect(page.network.entries().map((entry) => entry.method)).toEqual(['POST']); + }); + + test('a refused request keeps its method too — a blocked POST is not a blocked GET', async () => { + const rec = recorder(); + const page = await cdpTarget({ + page: rec.page, + browser: rec.browser, + rules: { allowHosts: ['shop.test'] }, + clock: testClock(), + }); + rec.emit('request', request('https://evil.test/api', 'PUT')); + expect(page.network.entries().map((entry) => [entry.method, entry.refused])).toEqual([ + ['PUT', 'host'], + ]); + }); + + test('a launcher whose request has no method() still records one', async () => { + const { rec, target } = open(); + const page = await target; + rec.emit('request', request('https://shop.test/api', undefined)); + expect(page.network.entries().map((entry) => entry.method)).toEqual(['GET']); + }); +}); + +describe('unit · a console line keeps its level', () => { + test('the four levels below log are reachable on the real driver', async () => { + const { rec, target } = open(); + const page = await target; + for (const type of ['error', 'warning', 'info', 'debug', 'table']) { + rec.emit('console', { type: () => type, text: () => `a ${type}` }); + } + expect(page.console.entries().map((line) => line.level)).toEqual([ + 'error', + 'warn', + 'info', + 'debug', + 'log', + ]); + }); +}); diff --git a/packages/scraping/src/cdp-target.ts b/packages/scraping/src/cdp-target.ts index 728737a8..c9d7eb5f 100644 --- a/packages/scraping/src/cdp-target.ts +++ b/packages/scraping/src/cdp-target.ts @@ -55,6 +55,21 @@ const asRequest = (payload: unknown): CdpRequestLike | undefined => { : undefined; }; +/** + * CDP's console levels, mapped onto this package's five. `warning` is the library's spelling of + * `warn`, `verbose` of `debug`, and everything structural (`table`, `startGroup`, `dir`) is a log + * line with a shape — never its own level, because `ConsoleLine.level` is what an author filters on. + */ +const CONSOLE_LEVELS: Readonly> = { + error: 'error', + assert: 'error', + warning: 'warn', + warn: 'warn', + info: 'info', + debug: 'debug', + verbose: 'debug', +}; + const readString = (value: unknown): string | undefined => typeof value === 'function' ? readString((value as () => unknown)()) @@ -87,20 +102,24 @@ async function arm( if (request === undefined) return; const url = request.url(); const type = asResourceType(request.resourceType()); + // The METHOD the browser is actually sending. Recording every request as a GET made + // `page.network()` — which `X_SCRAPE_HTTP_FAILED`'s own fix line tells the reader to open — + // misreport every POST and PUT the page made. + const method = readString(request.method) ?? 'GET'; const verdict = interceptVerdict(url, type, init.rules); const at = init.clock.now().getTime(); if (verdict === 'allow') { - network.push({ method: 'GET', url, resourceType: type, at }); + network.push({ method, url, resourceType: type, at }); void request.continue(); return; } - network.push(refusalEntry(url, type, verdict, at)); + network.push(refusalEntry(url, type, verdict, at, method)); void request.abort(); }); init.page.on('console', (payload) => { const record = payload as { type?: unknown; text?: unknown }; console_.push({ - level: 'log', + level: CONSOLE_LEVELS[(readString(record.type) ?? '').toLowerCase()] ?? 'log', text: readString(record.text) ?? '', at: init.clock.now().getTime(), }); @@ -117,6 +136,35 @@ export async function cdpTarget(init: CdpTargetInit): Promise { const network = createRing(init.ringCapacity); const crashed: { value: string | undefined } = { value: undefined }; await arm(init, network, console_, crashed); + let pendingStorage: SessionSnapshot | undefined; + + const originOf = (url: string): string => { + try { + return new URL(url).origin; + } catch { + return ''; + } + }; + + /** + * `localStorage` is PER ORIGIN, and `restore()` runs before the first navigation — on + * `about:blank`, an opaque origin with no storage to write to. So the storage half waits for the + * navigation that reaches the origin the session belongs to, and lands there. + * + * The origin has to MATCH: applying it to whatever page loaded first would write the site's + * bearer token — `session-state.ts` says this is where most sites keep it — into a different + * site's storage. A session whose origin is never visited simply keeps its storage, which is + * the same answer a browser gives. + */ + const applyPendingStorage = async (): Promise => { + const pending = pendingStorage; + if (pending === undefined) return; + if (originOf(init.page.url()) !== pending.origin || pending.origin === '') return; + pendingStorage = undefined; + await init.page.evaluate( + `(() => { const entries = ${JSON.stringify(pending.storage)}; for (const key of Object.keys(entries)) localStorage.setItem(key, entries[key]); })()`, + ); + }; const live = (): void => { if (crashed.value !== undefined) throw pageCrashed(init.page.url()); @@ -158,6 +206,7 @@ export async function cdpTarget(init: CdpTargetInit): Promise { goto: (url: string, options: GotoOptions) => guard('goto', async () => { await init.page.goto(url, { timeout: options.timeoutMs }); + await applyPendingStorage(); }), content: () => guard('content', () => init.page.content()), query: (selector) => @@ -243,17 +292,16 @@ export async function cdpTarget(init: CdpTargetInit): Promise { }), restore: (session: SessionSnapshot) => guard('restore', async () => { + // Two halves, because they belong to two different moments: cookies are the browser's and + // can be put back now, storage is an ORIGIN's and cannot exist until one is loaded. const source = init.browser as { setCookie?: (...cookies: readonly unknown[]) => Promise; }; if (typeof source.setCookie === 'function' && session.cookies.length > 0) { await source.setCookie(...session.cookies); } - if (Object.keys(session.storage).length > 0) { - await init.page.evaluate( - `(() => { const entries = ${JSON.stringify(session.storage)}; for (const key of Object.keys(entries)) localStorage.setItem(key, entries[key]); })()`, - ); - } + pendingStorage = Object.keys(session.storage).length > 0 ? session : undefined; + await applyPendingStorage(); }), close: async (): Promise => { await init.page.close(); diff --git a/packages/scraping/src/cookie-scope.test.ts b/packages/scraping/src/cookie-scope.test.ts new file mode 100644 index 00000000..c1fc2a93 --- /dev/null +++ b/packages/scraping/src/cookie-scope.test.ts @@ -0,0 +1,103 @@ +// The jar a session carries is every domain the browser touched, so "which cookie may this URL +// see" is an authorization decision. Both directions of the boundary are pinned here: a suffix +// that is not a domain (`evilbank.test` for `bank.test`) and a host-only cookie reaching down into +// a subdomain (`sub.bank.test`). + +import { describe, expect, test } from 'bun:test'; +import { + cookieDomainMatches, + cookieHeaderFor, + cookiePathMatches, + cookiesForUrl, +} from './cookie-scope'; +import type { ScrapeCookie } from './target'; + +const cookie = (over: Partial = {}): ScrapeCookie => ({ + name: 'sid', + value: 'SECRET', + domain: 'bank.test', + path: '/', + httpOnly: true, + secure: true, + ...over, +}); + +describe('unit · domain-match, RFC 6265 §5.1.3', () => { + test('a suffix is not a domain — evilbank.test does not match bank.test', () => { + expect(cookieDomainMatches('evilbank.test', 'bank.test')).toBe(false); + expect(cookieDomainMatches('evilbank.test', '.bank.test')).toBe(false); + }); + + test('a host-only cookie stays on its host — sub.bank.test does not match bank.test', () => { + expect(cookieDomainMatches('sub.bank.test', 'bank.test')).toBe(false); + }); + + test('a domain-scoped cookie reaches the apex and its subdomains', () => { + expect(cookieDomainMatches('bank.test', '.bank.test')).toBe(true); + expect(cookieDomainMatches('sub.bank.test', '.bank.test')).toBe(true); + expect(cookieDomainMatches('a.b.bank.test', '.bank.test')).toBe(true); + }); + + test('the exact host always matches, whatever the case', () => { + expect(cookieDomainMatches('BANK.test', 'bank.TEST')).toBe(true); + }); + + test('an empty domain matches nothing — a jar entry with no scope is not a wildcard', () => { + expect(cookieDomainMatches('bank.test', '')).toBe(false); + expect(cookieDomainMatches('bank.test', '.')).toBe(false); + }); +}); + +describe('unit · path-match, RFC 6265 §5.1.4', () => { + test('the boundary is a slash, so /admin never covers /administrators', () => { + expect(cookiePathMatches('/admin', '/admin')).toBe(true); + expect(cookiePathMatches('/admin/users', '/admin')).toBe(true); + expect(cookiePathMatches('/administrators', '/admin')).toBe(false); + }); + + test('a trailing slash on the cookie path is its own boundary', () => { + expect(cookiePathMatches('/admin/users', '/admin/')).toBe(true); + expect(cookiePathMatches('/admin', '/admin/')).toBe(false); + }); + + test('an absent path is /', () => { + expect(cookiePathMatches('/anything', '')).toBe(true); + }); +}); + +describe('unit · the jar a request may see', () => { + const jar = [ + cookie({ name: 'host-only' }), + cookie({ name: 'scoped', domain: '.bank.test' }), + cookie({ name: 'admin', path: '/admin' }), + cookie({ name: 'plain', secure: false, domain: 'shop.test' }), + ]; + + test('the two leaks this file exists for are refused', () => { + expect(cookiesForUrl(jar, 'https://evilbank.test/a')).toEqual([]); + expect(cookiesForUrl(jar, 'https://sub.bank.test/a').map((c) => c.name)).toEqual(['scoped']); + }); + + test('the legitimate cases still carry — the fix cannot pass by refusing everything', () => { + expect(cookieHeaderFor(jar, 'https://bank.test/')).toBe('host-only=SECRET; scoped=SECRET'); + expect(cookiesForUrl(jar, 'https://bank.test/admin/users').map((c) => c.name)).toEqual([ + 'host-only', + 'scoped', + 'admin', + ]); + }); + + test('a secure cookie is not handed to plaintext, and a plain one is', () => { + expect(cookiesForUrl(jar, 'http://bank.test/')).toEqual([]); + expect(cookiesForUrl(jar, 'http://shop.test/').map((c) => c.name)).toEqual(['plain']); + // Every browser trusts loopback; a fixture on http://localhost keeps working. + expect(cookiesForUrl([cookie({ domain: 'localhost' })], 'http://localhost:3000/')).toHaveLength( + 1, + ); + }); + + test('a URL that will not parse gets nothing — the same fail-closed rule hostDecision uses', () => { + expect(cookiesForUrl(jar, 'not a url')).toEqual([]); + expect(cookieHeaderFor(jar, 'not a url')).toBeUndefined(); + }); +}); diff --git a/packages/scraping/src/cookie-scope.ts b/packages/scraping/src/cookie-scope.ts new file mode 100644 index 00000000..c84c9925 --- /dev/null +++ b/packages/scraping/src/cookie-scope.ts @@ -0,0 +1,84 @@ +// Which cookies a URL may see — RFC 6265 §5.1.3 (domain-match) and §5.1.4 (path-match), as one +// decision every transport asks. +// +// It is a SECURITY rule and not a formatting one: a session snapshot's jar is `browser.cookies()`, +// i.e. every domain the session ever touched — an SSO hop's included — and the HTTP leg picks from +// it by hand rather than by a browser's own jar. A suffix test with no dot boundary sends a +// `bank.test` session cookie to `evilbank.test`; one with no host-only rule sends it to +// `sub.bank.test`. Both are the same one-line mistake, in opposite directions. + +import type { ScrapeCookie } from './target'; + +/** + * The dot IS the rule, in both directions. + * + * A stored `.bank.test` is DOMAIN-scoped — a browser records the leading dot for a cookie set with + * a `Domain=` attribute, and that is what CDP's `Network.getAllCookies` hands back — so it reaches + * `bank.test` and any subdomain of it. A stored `bank.test` is HOST-ONLY and reaches exactly that + * host. `ScrapeCookie` carries no `hostOnly` flag because the CDP cookie shape has none to carry + * (`cdp-port.ts`), so the leading dot is the only signal there is, and it is enough. + */ +export function cookieDomainMatches(host: string, domain: string): boolean { + const requested = host.trim().toLowerCase(); + const stored = domain.trim().toLowerCase(); + const scoped = stored.startsWith('.'); + const bare = scoped ? stored.slice(1) : stored; + if (bare === '' || requested === '') return false; + if (requested === bare) return true; + return scoped && requested.endsWith(`.${bare}`); +} + +/** + * RFC 6265 §5.1.4. `/admin` covers `/admin` and `/admin/users`, and never `/administrators` — + * the boundary is a `/`, exactly as it is for a domain. An empty stored path is `/`, which is what + * a jar entry written by hand usually means. + */ +export function cookiePathMatches(requestPath: string, cookiePath: string): boolean { + const wanted = requestPath === '' ? '/' : requestPath; + const stored = cookiePath === '' ? '/' : cookiePath; + if (wanted === stored) return true; + if (!wanted.startsWith(stored)) return false; + return stored.endsWith('/') || wanted.charAt(stored.length) === '/'; +} + +/** + * A `secure` cookie over plaintext is the same leak one hop further down: `http:` on a hostile + * network is readable. `localhost` is the one exception every browser makes, and a fixture host + * that is not is simply refused the cookie rather than silently downgraded. + */ +const trustworthy = (url: URL): boolean => + url.protocol === 'https:' || + url.hostname === 'localhost' || + url.hostname === '127.0.0.1' || + url.hostname === '[::1]'; + +/** + * Fails CLOSED, like `hostDecision()`: a URL that will not parse gets no cookies at all. Expiry is + * deliberately NOT filtered here — `ScrapeCookie.expires` carries no unit (CDP answers seconds, + * a hand-written jar tends to hold milliseconds) and dropping a live session cookie over a guess + * is worse than sending one the site will refuse itself. + */ +export function cookiesForUrl( + cookies: readonly ScrapeCookie[], + url: string, +): readonly ScrapeCookie[] { + let parsed: URL; + try { + parsed = new URL(url); + } catch { + return []; + } + const secureOk = trustworthy(parsed); + return cookies.filter( + (cookie) => + cookieDomainMatches(parsed.hostname, cookie.domain) && + cookiePathMatches(parsed.pathname, cookie.path) && + (!cookie.secure || secureOk), + ); +} + +/** The `cookie:` header this URL earns, or `undefined` when the jar has nothing for it. */ +export function cookieHeaderFor(cookies: readonly ScrapeCookie[], url: string): string | undefined { + const jar = cookiesForUrl(cookies, url); + return jar.length === 0 ? undefined : jar.map((c) => `${c.name}=${c.value}`).join('; '); +} diff --git a/packages/scraping/src/driver-cdp.test.ts b/packages/scraping/src/driver-cdp.test.ts new file mode 100644 index 00000000..61105ead --- /dev/null +++ b/packages/scraping/src/driver-cdp.test.ts @@ -0,0 +1,128 @@ +// What happens to a real Chrome process when `open()` does not finish. Between the launch and the +// `WedgeGuard` that owns `quit`/`kill` there are three awaits that can throw, and until they were +// wrapped every one of them leaked a browser: `runScrape`'s `finally { session.close() }` cannot +// run for a session that was never returned, so the process — or a paid remote session — outlived +// the attempt with nobody holding a handle to it. + +import { describe, expect, test } from 'bun:test'; +import type { CdpBrowserLike, CdpLauncherLike, CdpPageLike } from './cdp-port'; +import { testClock } from './clock'; +import type { SessionInit } from './driver'; +import { localBrowser, remoteBrowser } from './driver-cdp'; + +interface Broken { + readonly newPage?: boolean; + readonly intercept?: boolean; + readonly setCookie?: boolean; +} + +const brokenLauncher = (broken: Broken): CdpLauncherLike & { readonly closes: () => number } => { + let closes = 0; + const page: CdpPageLike = { + url: () => 'about:blank', + goto: () => Promise.resolve(undefined), + content: () => Promise.resolve(''), + evaluate: () => Promise.resolve(undefined), + click: () => Promise.resolve(), + type: () => Promise.resolve(), + select: () => Promise.resolve([]), + screenshot: () => Promise.resolve(new Uint8Array()), + pdf: () => Promise.resolve(new Uint8Array()), + setRequestInterception: () => + broken.intercept === true ? Promise.reject(new Error('too many targets')) : Promise.resolve(), + on: () => undefined, + frames: () => [], + close: () => Promise.resolve(), + }; + const browser: CdpBrowserLike = { + newPage: () => + broken.newPage === true ? Promise.reject(new Error('tab limit')) : Promise.resolve(page), + setCookie: () => + broken.setCookie === true ? Promise.reject(new Error('bad cookie')) : Promise.resolve(), + close: () => { + closes += 1; + return Promise.resolve(); + }, + process: () => null, + }; + return { + closes: () => closes, + launch: () => Promise.resolve(browser), + connect: () => Promise.resolve(browser), + }; +}; + +const init = (over: Partial = {}): SessionInit => ({ + name: 'orders', + rules: { allowHosts: ['shop.test'] }, + clock: testClock(), + timeoutMs: 1_000, + ...over, +}); + +const failedOpen = async ( + broken: Broken, + over: Partial = {}, +): Promise<{ readonly code: string | undefined; readonly closes: number }> => { + const launcher = brokenLauncher(broken); + let code: string | undefined; + try { + await localBrowser({ launcher }).open(init(over)); + } catch (thrown) { + code = (thrown as { code?: string }).code; + } + return { code, closes: launcher.closes() }; +}; + +const RESTORE = { + cookies: [ + { name: 'sid', value: 'x', domain: 'shop.test', path: '/', httpOnly: true, secure: true }, + ], + headers: {}, + storage: {}, + userAgent: 'agent', + origin: 'https://shop.test', +}; + +describe('unit · a browser that was launched is closed when open() cannot finish', () => { + test('newPage() rejecting closes the browser rather than leaking it', async () => { + expect(await failedOpen({ newPage: true })).toEqual({ + code: 'X_SCRAPE_BROWSER_UNREACHABLE', + closes: 1, + }); + }); + + test('setRequestInterception() rejecting closes the browser', async () => { + expect(await failedOpen({ intercept: true })).toEqual({ + code: 'X_SCRAPE_BROWSER_UNREACHABLE', + closes: 1, + }); + }); + + test('a restore that throws closes the browser — the common case, and the one that retries', async () => { + expect(await failedOpen({ setCookie: true }, { restore: RESTORE })).toEqual({ + code: 'X_SCRAPE_BROWSER_UNREACHABLE', + closes: 1, + }); + }); + + test('the attached browser gets the same rollback — a remote session is somebody billing', async () => { + const launcher = brokenLauncher({ newPage: true }); + let code: string | undefined; + try { + await remoteBrowser({ launcher, cdpUrl: 'ws://browser.test/1' }).open(init()); + } catch (thrown) { + code = (thrown as { code?: string }).code; + } + expect(code).toBe('X_SCRAPE_BROWSER_UNREACHABLE'); + expect(launcher.closes()).toBe(1); + }); + + test('an open that SUCCEEDS closes nothing — the rollback is not a teardown', async () => { + const launcher = brokenLauncher({}); + const session = await localBrowser({ launcher }).open(init()); + expect(launcher.closes()).toBe(0); + await session.close(); + expect(launcher.closes()).toBe(1); + }); +}); diff --git a/packages/scraping/src/driver-cdp.ts b/packages/scraping/src/driver-cdp.ts index a88740eb..76d52fea 100644 --- a/packages/scraping/src/driver-cdp.ts +++ b/packages/scraping/src/driver-cdp.ts @@ -14,8 +14,10 @@ import type { CdpBrowserLike, CdpLauncherLike } from './cdp-port'; import { CDP_DRIVER, cdpTarget } from './cdp-target'; import type { ScrapeDriver, ScrapeSession, SessionInit } from './driver'; import { browserUnreachable, cdpAttachFailed, remoteRequired } from './error-throws'; +import { isScrapeError } from './errors'; import { httpOverFetch } from './http'; import { pageOverTarget } from './page-over-target'; +import type { ScrapeTarget } from './target'; import { createWedgeGuard } from './watchdog'; export { CDP_DRIVER } from './cdp-target'; @@ -45,19 +47,37 @@ export interface RemoteBrowserOptions extends BrowserOptions { readonly cdpUrl: string; } +/** + * The page, its interception and its restored session — or a closed browser and the failure. + * A throw from here is classified before it leaves: `newPage()` and `setRequestInterception()` are + * outside `cdpTarget`'s own `guard()`, so a bare library `Error` would otherwise reach the job's + * retry classifier with no code at all. + */ +async function opened(browser: CdpBrowserLike, init: SessionInit): Promise { + try { + const page = await browser.newPage(); + const target = await cdpTarget({ page, browser, rules: init.rules, clock: init.clock }); + if (init.restore !== undefined) await target.restore(init.restore); + return target; + } catch (thrown) { + // Best effort, and it may never replace the failure that caused it: a close that also throws + // would hide the tab limit or the refused interception the reader actually needs. + await browser.close().catch(() => undefined); + throw isScrapeError(thrown) ? thrown : browserUnreachable(CDP_DRIVER, thrown); + } +} + async function sessionOver( browser: CdpBrowserLike, init: SessionInit, options: BrowserOptions, ): Promise { - const page = await browser.newPage(); - const target = await cdpTarget({ - page, - browser, - rules: init.rules, - clock: init.clock, - }); - if (init.restore !== undefined) await target.restore(init.restore); + // Acquire, then roll back on ANY throw — the shape `releaseBoot` uses in `packages/cli/src/ + // serve.ts`. Between the launch and the `WedgeGuard` below, nothing else holds this browser: + // `runScrape`'s `finally { session.close() }` never runs for a session `open()` did not return, + // so a tab limit, a refused interception or a restore that threw left a real Chrome process — + // or a remote session somebody is billing for — running per attempt, unattributed. + const target = await opened(browser, init); const guard = createWedgeGuard({ clock: init.clock, what: `scrape "${init.name}"`, diff --git a/packages/scraping/src/driver-parity.test.ts b/packages/scraping/src/driver-parity.test.ts index 5720f76a..3381003c 100644 --- a/packages/scraping/src/driver-parity.test.ts +++ b/packages/scraping/src/driver-parity.test.ts @@ -187,6 +187,41 @@ describe('unit · where the drivers genuinely cannot agree, pinned in one place' expect(boxes).toEqual({ fake: false, fixture: false, puppeteer: true }); }); + test('SessionSnapshot.headers is answerable OFFLINE and empty on the real driver', async () => { + const restore = { + cookies: [], + headers: { 'x-csrf': 'tok' }, + storage: {}, + userAgent: 'agent', + origin: 'https://shop.test', + }; + const withRestore = (driver: ScrapeDriver): Promise => + driver.open({ + name: 'orders', + rules: { allowHosts: ['shop.test'], block: ['image'] }, + clock: testClock(), + timeoutMs: 5_000, + restore, + }); + const offline = await withRestore(fakeBrowser(PAGES, { http: HTTP })); + const real = await withRestore( + localBrowser({ launcher: fakeCdpLauncher({ url: URL_A, html: HTML_A }) }), + ); + try { + expect((await offline.page.session()).headers).toEqual({ 'x-csrf': 'tok' }); + // PINNED, and it is a divergence rather than a bug: CDP exposes no "headers this site now + // expects" to read back. The alternative considered was capturing observed request headers — + // rejected, because that persists a `cookie`/`authorization` into the session record and + // `httpOverFetch` spreads `session.headers` onto EVERY allowed host, which is a wider leak + // than the empty field. So a fixture may assert on headers the real driver will not send: + // put a token the HTTP leg must carry in the request's own `headers`, never in the session. + expect((await real.page.session()).headers).toEqual({}); + } finally { + await offline.close(); + await real.close(); + } + }); + test('only the offline drivers refuse an unrecorded request', async () => { const fixture = await open(fixtureBrowser(dir)); try { diff --git a/packages/scraping/src/error-throws.ts b/packages/scraping/src/error-throws.ts index 3ddf3e03..f4a383ac 100644 --- a/packages/scraping/src/error-throws.ts +++ b/packages/scraping/src/error-throws.ts @@ -6,12 +6,25 @@ import { renderThrowable, UltimateError } from '@ultimat3/core'; import { ScrapeError } from './errors'; -export const driverUnknown = (name: string, installed: readonly string[]): ScrapeError => +/** + * Two shapes of the same failure, one code. `name` is the driver a definition ASKED for; `scrape` + * is passed when there is no driver at all to name — and that is the reachable case, so the cause + * has to read as one. It said `no scrape driver named "orders.daily" is installed`, naming the + * scrape as a driver, which sends its reader hunting for a driver nobody ever declared. + */ +export const driverUnknown = ( + name: string | undefined, + installed: readonly string[], + scrape?: string, +): ScrapeError => new ScrapeError({ code: 'X_SCRAPE_DRIVER_UNKNOWN', - cause: `no scrape driver named "${name}" is installed; installed: ${installed.join(', ') || 'none'}`, + cause: + scrape === undefined + ? `no scrape driver named "${name ?? 'none'}" is installed; installed: ${installed.join(', ') || 'none'}` + : `scrape "${scrape}" has no browser driver: nothing called setScrapeDriver() and the definition declares no driver:; installed: ${installed.join(', ') || 'none'}`, fix: 'call setScrapeDriver(localBrowser()) at boot, or pass driver: fakeBrowser() on the scrape() definition', - meta: { driver: name }, + meta: { driver: name ?? 'none', ...(scrape === undefined ? {} : { scrape }) }, }); export const cdpAttachFailed = (cdpUrl: string, thrown: unknown): ScrapeError => diff --git a/packages/scraping/src/expect.ts b/packages/scraping/src/expect.ts index 56c342cb..7203ffd7 100644 --- a/packages/scraping/src/expect.ts +++ b/packages/scraping/src/expect.ts @@ -95,6 +95,11 @@ export interface YieldGuardInput { * exists to prevent, one level up. */ export async function guardYield(input: YieldGuardInput): Promise { + // No `expect` is no baseline either, deliberately: with no floor and no drop rule there is + // nothing deciding whether a run was good, so recording it would let a stretch of silent + // zero-row runs become the median an `expect` added later is measured against. The cost is that + // `maxDrop` needs `MIN_BASELINE_RUNS` runs after it is declared before it can fire — a delay, + // not a hole. `expect.test.ts` pins both halves. if (input.expect === undefined) return; const window = input.expect.window ?? DEFAULT_YIELD_WINDOW; const history = diff --git a/packages/scraping/src/http-recorded.test.ts b/packages/scraping/src/http-recorded.test.ts new file mode 100644 index 00000000..33588895 --- /dev/null +++ b/packages/scraping/src/http-recorded.test.ts @@ -0,0 +1,71 @@ +// The offline HTTP leg applies the same gates as the live one. Both of them: a rule only the live +// transport enforces is a rule a green suite cannot see, and the first attempt that meets it is a +// production one. + +import { describe, expect, test } from 'bun:test'; +import { testClock } from './clock'; +import { recordedHttp } from './http-recorded'; +import type { HttpRecording } from './recording'; +import type { NetworkEntry } from './rings'; +import { createRing } from './rings'; +import { createRobotsGate } from './robots'; + +const ROBOTS = 'User-agent: *\nDisallow: /api/private\n'; + +const codeOf = async (promise: Promise): Promise => { + try { + await promise; + return undefined; + } catch (thrown) { + return (thrown as { code?: string }).code; + } +}; + +const recordings: readonly HttpRecording[] = [ + { url: 'https://shop.test/api/private/orders', method: 'GET', status: 200, body: '{}' }, + { url: 'https://shop.test/api/public/orders', method: 'GET', status: 200, body: '{}' }, +]; + +const offline = (robots = true) => + recordedHttp({ + lookup: (method, url) => + Promise.resolve(recordings.find((r) => r.method === method && r.url === url)), + rules: { allowHosts: ['shop.test'] }, + network: createRing(), + clock: testClock(), + source: 'test', + ...(robots + ? { + robots: createRobotsGate({ + policy: 'obey', + // Never the network, ever: an offline transport that fetched robots.txt for real would + // make a green suite secretly live, which is the rule this file exists for. + fetchText: () => Promise.resolve(ROBOTS), + }), + } + : {}), + }); + +describe('unit · robots.txt gates the replayed HTTP leg, not just the live one', () => { + test('a Disallow:ed endpoint is refused offline, where the test can see it', async () => { + expect(await codeOf(offline().request('https://shop.test/api/private/orders'))).toBe( + 'X_SCRAPE_ROBOTS_DISALLOWED', + ); + }); + + test('an allowed endpoint still replays', async () => { + const response = await offline().request('https://shop.test/api/public/orders'); + expect(response.status).toBe(200); + }); + + test('no gate declared is no gate applied — the offline drivers stay usable bare', async () => { + const response = await offline(false).request('https://shop.test/api/private/orders'); + expect(response.status).toBe(200); + }); + + test('the host rule is still checked FIRST — the cheaper refusal keeps its meaning', async () => { + expect(await codeOf(offline().request('https://evil.test/api/private/orders'))).toBe( + 'X_SCRAPE_HOST_BLOCKED', + ); + }); +}); diff --git a/packages/scraping/src/http-recorded.ts b/packages/scraping/src/http-recorded.ts index 854f6a9a..ff895239 100644 --- a/packages/scraping/src/http-recorded.ts +++ b/packages/scraping/src/http-recorded.ts @@ -13,6 +13,7 @@ import type { InterceptRules } from './intercept'; import { interceptVerdict } from './intercept'; import type { HttpRecording } from './recording'; import type { NetworkRing } from './rings'; +import type { RobotsGate } from './robots'; export type HttpRecordingLookup = ( method: string, @@ -25,6 +26,12 @@ export interface RecordedHttpInit { readonly network: NetworkRing; readonly clock: ScrapeClock; readonly source: string; + /** + * The SAME gate the page leg holds. Without it a hybrid scrape whose JSON endpoint is + * `Disallow:`ed replayed green offline and threw terminal `X_SCRAPE_ROBOTS_DISALLOWED` on its + * first real attempt — the identical argument the host rule below already makes for itself. + */ + readonly robots?: RobotsGate | undefined; readonly maxAgeMs?: number | undefined; } @@ -46,6 +53,7 @@ export function recordedHttp(init: RecordedHttpInit): ScrapeHttp { if (interceptVerdict(url, 'fetch', init.rules) !== 'allow') { throw hostBlocked(url, init.rules.allowHosts); } + await init.robots?.assertAllowed(url); const found = await init.lookup(method, url); if (found === undefined) throw fixtureMissing(`${method} ${url}`, init.source); if (init.maxAgeMs !== undefined && found.recordedAt !== undefined) { diff --git a/packages/scraping/src/http.test.ts b/packages/scraping/src/http.test.ts index 8362c93c..34a13380 100644 --- a/packages/scraping/src/http.test.ts +++ b/packages/scraping/src/http.test.ts @@ -142,3 +142,48 @@ describe('unit · reading a response', () => { expect(await response.json()).toEqual({ id: 7 }); }); }); + +describe('unit · the session jar is EVERY domain the browser touched, so scoping is a security rule', () => { + const hostOnly = (domain: string, path = '/') => ({ + ...EMPTY_SESSION, + cookies: [{ name: 'sid', value: 'SECRET', domain, path, httpOnly: true, secure: true }], + }); + + test('a host-only bank.test cookie never reaches evilbank.test — a suffix is not a domain', async () => { + const { http, calls } = transport({ status: 200, body: '{}' }, hostOnly('bank.test'), ['*']); + await http.request('https://evilbank.test/a'); + expect(calls[0]?.headers.cookie).toBeUndefined(); + }); + + test('a host-only bank.test cookie never reaches sub.bank.test — host-only means the host', async () => { + const { http, calls } = transport({ status: 200, body: '{}' }, hostOnly('bank.test'), ['*']); + await http.request('https://sub.bank.test/a'); + expect(calls[0]?.headers.cookie).toBeUndefined(); + }); + + test('the cookie DOES reach the host it belongs to', async () => { + const { http, calls } = transport({ status: 200, body: '{}' }, hostOnly('bank.test'), ['*']); + await http.request('https://bank.test/a'); + expect(calls[0]?.headers.cookie).toBe('sid=SECRET'); + }); + + test('a domain-scoped .bank.test cookie DOES reach a subdomain, and still not evilbank.test', async () => { + const { http, calls } = transport({ status: 200, body: '{}' }, hostOnly('.bank.test'), ['*']); + await http.request('https://sub.bank.test/a'); + await http.request('https://evilbank.test/a'); + expect(calls[0]?.headers.cookie).toBe('sid=SECRET'); + expect(calls[1]?.headers.cookie).toBeUndefined(); + }); + + test('a cookie scoped to /admin is not sent to /public', async () => { + const { http, calls } = transport( + { status: 200, body: '{}' }, + hostOnly('bank.test', '/admin'), + ['*'], + ); + await http.request('https://bank.test/public'); + await http.request('https://bank.test/admin/users'); + expect(calls[0]?.headers.cookie).toBeUndefined(); + expect(calls[1]?.headers.cookie).toBe('sid=SECRET'); + }); +}); diff --git a/packages/scraping/src/http.ts b/packages/scraping/src/http.ts index 46e45e5b..80178282 100644 --- a/packages/scraping/src/http.ts +++ b/packages/scraping/src/http.ts @@ -13,6 +13,7 @@ import type { StandardSchemaV1 } from '@ultimat3/schema'; import { parse } from '@ultimat3/schema'; import type { ScrapeClock } from './clock'; +import { cookieHeaderFor } from './cookie-scope'; import { hostBlocked, httpFailed, scrapeTimeout } from './error-throws'; import type { InterceptRules } from './intercept'; import { interceptVerdict } from './intercept'; @@ -68,19 +69,6 @@ export interface HttpTransportInit { readonly fetch?: typeof fetch | undefined; } -const cookieHeader = (session: SessionSnapshot, url: string): string | undefined => { - let host: string; - try { - host = new URL(url).hostname; - } catch { - return undefined; - } - const jar = session.cookies.filter( - (cookie) => host === cookie.domain.replace(/^\./, '') || host.endsWith(cookie.domain), - ); - return jar.length === 0 ? undefined : jar.map((c) => `${c.name}=${c.value}`).join('; '); -}; - const headerRecord = (headers: Headers): Record => { const out: Record = {}; headers.forEach((value, key) => { @@ -127,7 +115,7 @@ export function httpOverFetch(init: HttpTransportInit): ScrapeHttp { await init.robots?.assertAllowed(url); await init.pace?.(init.signal); const session = await init.session(); - const cookies = cookieHeader(session, url); + const cookies = cookieHeaderFor(session.cookies, url); const timeoutMs = request.timeout ?? init.timeoutMs; // `AbortSignal.timeout` and NOT `clock.sleep`: this is a deadline handed to the platform's // own fetch, not a wait this package performs — and under a test clock a slept deadline diff --git a/packages/scraping/src/index.ts b/packages/scraping/src/index.ts index 19030134..9b7191d4 100644 --- a/packages/scraping/src/index.ts +++ b/packages/scraping/src/index.ts @@ -25,6 +25,12 @@ export type { CdpTargetInit } from './cdp-target'; export { CDP_DRIVER, cdpTarget } from './cdp-target'; export type { Deadline, ScrapeClock, TestScrapeClock } from './clock'; export { deadline, systemScrapeClock, testClock, throwIfAborted } from './clock'; +export { + cookieDomainMatches, + cookieHeaderFor, + cookiePathMatches, + cookiesForUrl, +} from './cookie-scope'; export type { ScrapeDriver, ScrapeSession, SessionInit } from './driver'; export { resetScrapeDriver, scrapeDriver, setScrapeDriver } from './driver'; export type { BrowserOptions, LocalBrowserOptions, RemoteBrowserOptions } from './driver-cdp'; diff --git a/packages/scraping/src/intercept.ts b/packages/scraping/src/intercept.ts index 350fee61..236dcca6 100644 --- a/packages/scraping/src/intercept.ts +++ b/packages/scraping/src/intercept.ts @@ -29,10 +29,13 @@ export function interceptVerdict( } /** The ring entry a refusal earns. Refusals are RECORDED, never silent — a scrape that came back - * empty is diagnosed from this list. */ + * empty is diagnosed from this list, and a blocked POST reported as a GET sends its reader hunting + * for a request the page never made. `method` is last and optional because a driver that cannot + * read one (a parsed document's `` is a GET by construction) says so by omitting it. */ export const refusalEntry = ( url: string, resourceType: ResourceType, verdict: Exclude, at: number, -): NetworkEntry => ({ method: 'GET', url, resourceType, at, refused: verdict }); + method = 'GET', +): NetworkEntry => ({ method, url, resourceType, at, refused: verdict }); diff --git a/packages/scraping/src/offline-session.ts b/packages/scraping/src/offline-session.ts index 5485453c..bd74eec5 100644 --- a/packages/scraping/src/offline-session.ts +++ b/packages/scraping/src/offline-session.ts @@ -58,6 +58,8 @@ export async function openOfflineSession(init: OfflineSessionInit): Promise target.close(), diff --git a/packages/scraping/src/page-over-target.ts b/packages/scraping/src/page-over-target.ts index 083f98ba..46e0d38b 100644 --- a/packages/scraping/src/page-over-target.ts +++ b/packages/scraping/src/page-over-target.ts @@ -210,5 +210,6 @@ export function pageOverTarget(target: ScrapeTarget, ctx: PageContext): ScrapePa session: () => target.session(), console: () => target.console.entries(), network: () => target.network.entries(), + networkDropped: () => target.network.dropped, }; } diff --git a/packages/scraping/src/page.ts b/packages/scraping/src/page.ts index 64de402e..c496ea51 100644 --- a/packages/scraping/src/page.ts +++ b/packages/scraping/src/page.ts @@ -94,4 +94,10 @@ export interface ScrapePage extends ScrapeFrame { /** The bounded tail. Bounded because a long run's full history is an OOM, not a log. */ console(): readonly ConsoleLine[]; network(): readonly NetworkEntry[]; + /** + * How many entries the bound above threw away. It is the same honesty `Ring.dropped` carries: + * any count taken from `network()` — the run's `refused` total included — is a floor once this + * is non-zero, and a scrape that blocked 5,000 images otherwise reports 200 with no hint. + */ + networkDropped(): number; } diff --git a/packages/scraping/src/scrape-run.test.ts b/packages/scraping/src/scrape-run.test.ts new file mode 100644 index 00000000..e2f54494 --- /dev/null +++ b/packages/scraping/src/scrape-run.test.ts @@ -0,0 +1,163 @@ +// One attempt's assembly, at the two places it decides something a scrape body cannot see: which +// session key this run owns, and what the report says about requests interception refused. +// +// The session key is a TENANCY boundary — a session is credential material, and two tenants +// sharing one key share an authenticated browser session — so it is pinned here rather than left +// to `sessionKeyFor`'s unit test, which cannot see who calls it with what. + +import { describe, expect, test } from 'bun:test'; +import { createContext, createLogger, userActor } from '@ultimat3/core'; +import type { JobRunArgs, StepApi } from '@ultimat3/jobs'; +import { t } from '@ultimat3/schema'; +import { testClock } from './clock'; +import { resetScrapeDriver } from './driver'; +import { fakeBrowser } from './driver-fake'; +import type { ScrapeDefinition, ScrapeReport } from './scrape'; +import { runScrape } from './scrape-run'; +import type { ScrapeSessionStore, SessionState } from './session-state'; + +const URL_A = 'https://shop.test/orders'; +const HTML = '
  • One
'; + +const passThroughStep = (): StepApi => + ({ + run: (_name: string, fn: () => Promise | T) => Promise.resolve(fn()), + }) as unknown as StepApi; + +const runArgs = (orgId: string | undefined): JobRunArgs<{ page: number }> => ({ + input: { page: 1 }, + step: passThroughStep(), + ctx: createContext({ + logger: createLogger({ writer: () => undefined }), + ...(orgId === undefined ? {} : { actor: userActor({ id: 'u-1', orgId }) }), + }), + attempt: 1, + jobId: 'job-1', + runId: 'run-1', +}); + +/** Records every key written, which is the only place the plan's key is observable from outside. */ +const recordingStore = (): ScrapeSessionStore & { readonly keys: readonly string[] } => { + const keys: string[] = []; + const states = new Map(); + return { + keys, + load: (key) => Promise.resolve(states.get(key)), + save: (state) => { + keys.push(state.key); + states.set(state.key, state); + return Promise.resolve(); + }, + burn: (key) => { + states.delete(key); + return Promise.resolve(); + }, + }; +}; + +const define = ( + over: Partial> = {}, +): ScrapeDefinition<{ page: number }, { id: string }> => ({ + name: 'orders', + input: t.object({ page: t.number }), + extract: t.object({ id: t.string }), + idempotencyKey: (input) => `orders:${String(input.page)}`, + tenant: 'none', + allowHosts: ['shop.test'], + clock: testClock(), + driver: fakeBrowser([{ url: URL_A, html: HTML }]), + async run({ page }) { + await page.goto(URL_A); + return (await page.values('.row')).map((element) => ({ id: element.attrs['data-id'] })); + }, + ...over, +}); + +const keyAfterLogin = async ( + store: ScrapeSessionStore & { readonly keys: readonly string[] }, + orgId: string | undefined, + key?: (input: { page: number }) => string, +): Promise => { + await runScrape( + define({ + auth: { store, login: () => Promise.resolve(), ...(key === undefined ? {} : { key }) }, + }), + runArgs(orgId), + ); + return store.keys.at(-1); +}; + +describe('unit · the session key is a tenancy boundary, and auth.key discriminates INSIDE it', () => { + test('two tenants with the SAME auth.key get different session keys', async () => { + const store = recordingStore(); + const first = await keyAfterLogin(store, 'org-1', () => 'shared'); + const second = await keyAfterLogin(store, 'org-2', () => 'shared'); + expect(first).toBeDefined(); + expect(first).not.toBe(second); + expect(first?.startsWith('org-1/')).toBe(true); + expect(second?.startsWith('org-2/')).toBe(true); + }); + + test('two accounts inside ONE tenant get different session keys — what auth.key is for', async () => { + const store = recordingStore(); + const first = await keyAfterLogin(store, 'org-1', () => 'account-a'); + const second = await keyAfterLogin(store, 'org-1', () => 'account-b'); + expect(first).toBe('org-1/orders/account-a'); + expect(second).toBe('org-1/orders/account-b'); + }); + + test('no auth.key is the tenant default, unchanged', async () => { + const store = recordingStore(); + expect(await keyAfterLogin(store, 'org-1')).toBe('org-1/orders/default'); + }); + + test('a discriminator cannot escape the key space — separators and NUL are stripped', async () => { + const store = recordingStore(); + // The key is also a storage path. Before this ran through `sessionKeyFor`, an `auth.key` + // returning a path was written to the store verbatim, unsanitised. + const escaped = await keyAfterLogin(store, 'org-1', () => '../../etc/passwd\0'); + expect(escaped?.split('/')).toHaveLength(3); + expect(escaped).toBe('org-1/orders/..-..-etc-passwd-'); + }); +}); + +describe('unit · the refusal count says when it is not the whole count', () => { + test('a run whose network ring overflowed reports the drop, so `refused` reads as a floor', async () => { + // 250 refusals into a 200-entry ring: the ring is bounded on purpose (a scrape of ten thousand + // pages must not hold its whole browsing history), so the count taken from it is a FLOOR and + // the report has to say so — otherwise "5,000 images blocked" prints as 200 with no hint. + const many = Array.from( + { length: 250 }, + (_, index) => ``, + ).join(''); + const report = (await runScrape( + define({ + driver: fakeBrowser([{ url: URL_A, html: `${many}` }]), + run: async ({ page }) => { + await page.goto(URL_A); + return []; + }, + }), + runArgs('org-1'), + )) as ScrapeReport<{ id: string }>; + expect(report.refused).toBe(200); + expect(report.networkDropped).toBe(51); + }); +}); + +describe('unit · a run with no driver at all says so', () => { + test('the cause names the SCRAPE as a scrape, never as a driver nobody named', async () => { + resetScrapeDriver(); + const { driver: _dropped, ...bare } = define(); + let cause: string | undefined; + try { + await runScrape(bare, runArgs('org-1')); + } catch (thrown) { + cause = (thrown as { cause?: string }).cause; + } + // The old cause read `no scrape driver named "orders" is installed`, which sends its reader + // hunting for a driver called "orders" — the scrape's own name. + expect(cause).toContain('scrape "orders"'); + expect(cause).not.toContain('named "orders"'); + }); +}); diff --git a/packages/scraping/src/scrape-run.ts b/packages/scraping/src/scrape-run.ts index c1f2d88b..44c34bd5 100644 --- a/packages/scraping/src/scrape-run.ts +++ b/packages/scraping/src/scrape-run.ts @@ -58,7 +58,9 @@ export async function runScrape( ): Promise> { const clock = definition.clock ?? systemScrapeClock; const driver = definition.driver ?? scrapeDriver(); - if (driver === undefined) throw driverUnknown(definition.name, []); + // The scrape's name goes in the SCRAPE slot: there is no driver here to name, which is the + // whole failure. + if (driver === undefined) throw driverUnknown(undefined, [], definition.name); const logger = scrapeLogger(args.ctx.logger, { scrape: definition.name, runId: args.runId, @@ -77,9 +79,14 @@ export async function runScrape( const plan: AuthPlanInput = { scrape: definition.name, auth: definition.auth, - key: - definition.auth?.key?.(args.input) ?? - sessionKeyFor({ scrape: definition.name, tenant: orgOf(args.ctx) }), + // `auth.key` DISCRIMINATES inside the tenant's key space; it never replaces it. Letting it + // replace the key gave two tenants declaring the same account name one authenticated session, + // and skipped the sanitising `sessionKeyFor` does to a value that is also a storage path. + key: sessionKeyFor({ + scrape: definition.name, + tenant: orgOf(args.ctx), + discriminator: definition.auth?.key?.(args.input), + }), clock, logger, }; @@ -134,12 +141,17 @@ export async function runScrape( history: definition.history, }); const refused = session.page.network().filter((entry) => entry.refused !== undefined).length; + // The ring is bounded, so `refused` is a FLOOR and this is what says so. Reporting the count + // alone made a run that blocked 5,000 images print 200 and discarded the one number + // (`Ring.dropped`) that exists to say "you are not seeing it all". + const networkDropped = session.page.networkDropped(); logger.info('scrape.ok', { rows: rows.length, refused }); return { scrape: definition.name, rows, artifacts: artifact.saved.map((ref) => ref.key), refused, + networkDropped, }; } catch (thrown) { logger.error('scrape.failed', { code: errorCode(thrown) }); diff --git a/packages/scraping/src/scrape.test.ts b/packages/scraping/src/scrape.test.ts index 30329a09..2ce3b4aa 100644 --- a/packages/scraping/src/scrape.test.ts +++ b/packages/scraping/src/scrape.test.ts @@ -156,7 +156,10 @@ describe('unit · the login path', () => { secrets: ['SHOP_PASSWORD'], auth: { store, - key: () => 'org-1/orders', + // A DISCRIMINATOR, not the key: the tenant and the scrape name are the framework's to + // put in front of it (`sessionKeyFor`), so two tenants naming one account never share + // an authenticated session. + key: () => 'account-a', login: async ({ page }) => { logins += 1; await page.goto(URL_A); @@ -173,7 +176,7 @@ describe('unit · the login path', () => { const report = (await handle.run(runArgs({ page: 1 }))) as ScrapeReport<{ id: string }>; expect(logins).toBe(1); expect(report.rows).toHaveLength(2); - expect(await store.load('org-1/orders')).toBeDefined(); + expect(await store.load('no-tenant/orders/account-a')).toBeDefined(); delete process.env.SHOP_PASSWORD; }); @@ -183,7 +186,7 @@ describe('unit · the login path', () => { const definition = define({ auth: { store, - key: () => 'org-1/orders', + key: () => 'account-a', login: () => { logins += 1; // What a body throws when the site says "wrong password". diff --git a/packages/scraping/src/scrape.ts b/packages/scraping/src/scrape.ts index 7cdb6545..b2c1dcf4 100644 --- a/packages/scraping/src/scrape.ts +++ b/packages/scraping/src/scrape.ts @@ -112,8 +112,16 @@ export interface ScrapeReport { readonly scrape: string; readonly rows: readonly Row[]; readonly artifacts: readonly string[]; - /** Requests interception refused, by reason. A zero-row run usually explains itself here. */ + /** + * Requests interception refused, by reason. A zero-row run usually explains itself here — and it + * is a FLOOR, not a total, whenever `networkDropped` is non-zero. + */ readonly refused: number; + /** + * Entries the bounded network ring dropped to stay bounded (`rings.ts`). Non-zero is the honest + * "you are not seeing it all": `refused` was counted from what survived the bound. + */ + readonly networkDropped: number; } export function scrape(definition: ScrapeDefinition): JobHandle { diff --git a/packages/scraping/src/session-state.ts b/packages/scraping/src/session-state.ts index f8cf1f4e..9ad16eca 100644 --- a/packages/scraping/src/session-state.ts +++ b/packages/scraping/src/session-state.ts @@ -16,7 +16,14 @@ import type { ScrapeCookie } from './target'; export interface SessionSnapshot { readonly cookies: readonly ScrapeCookie[]; - /** Headers the HTTP leg must send to stay the same client — user-agent, language, site tokens. */ + /** + * Headers the HTTP leg must send to stay the same client — user-agent, language, site tokens. + * + * Only an OFFLINE driver can fill it: CDP exposes no read for "the headers this site now + * expects", so the puppeteer driver answers `{}` and `driver-parity.test.ts` pins the + * divergence. A token the HTTP leg must carry belongs on the request (`http.request(url, { + * headers })`), not here — a fixture that proves otherwise proves it only offline. + */ readonly headers: Readonly>; /** `localStorage`, flattened. Many sites keep the bearer token here and not in a cookie. */ readonly storage: Readonly>; diff --git a/packages/ui/CATALOG.md b/packages/ui/CATALOG.md index f56dc536..78bc8946 100644 --- a/packages/ui/CATALOG.md +++ b/packages/ui/CATALOG.md @@ -146,7 +146,7 @@ Native checkbox with a token-drawn indicator. The label element wraps the input, | `name` | `string` | — | | | `value` | `string` | — | | | `checked` | `boolean` | — | | -| `indeterminate` | `boolean` | — | Tri-state for "some children selected". Mirrored to `aria-checked`. | +| `indeterminate` | `boolean` | — | Tri-state for "some children selected". The ONLY thing mirrored to `aria-checked`. | | `disabled` | `boolean` | — | | | `required` | `boolean` | — | | | `description` | `string` | — | | @@ -230,7 +230,7 @@ Renders