diff --git a/.cursor/skills/ship-a-change/SKILL.md b/.cursor/skills/ship-a-change/SKILL.md index 2319274..72b795c 100644 --- a/.cursor/skills/ship-a-change/SKILL.md +++ b/.cursor/skills/ship-a-change/SKILL.md @@ -44,7 +44,10 @@ Three details keep the app working from a repository sub-path. Do not undo them: straight from the browser on Pages, so the hosted site gets the same tool list as the dev server. Keep it that way: a tool that _only_ works behind the optional proxy cannot ship here. The proxy in `tools/agent-api.ts` is an extra path for DuckDuckGo search and page reads, mounted by - `pnpm dev` and `pnpm proxy`, never by the Pages build. + `pnpm dev` and `pnpm proxy`, never by the Pages build. The repository variable `AGENT_API_BASE` + can point the build at a hosted one, and both tools fall back to the browser path when it fails — + which is the only reason aiming every visitor at one process is defensible. Do not remove that + fallback to surface a proxy error. The Pages concurrency group deliberately does not cancel in-progress runs: cancelling mid-deploy can leave the site half-published. diff --git a/.env.example b/.env.example index fcbea16..073c320 100644 --- a/.env.example +++ b/.env.example @@ -12,7 +12,9 @@ # providers from the browser. To send DuckDuckGo search and page reads through # the optional tool proxy instead, set this to the proxy origin or to # `same-origin` (which is what `.env.development` does for `pnpm dev`). -# Leave it unset on GitHub Pages so the hosted site stays fully static. +# Empty or unset stays browser-direct. A proxy that fails falls back to the +# browser path, so this is an optimisation rather than a dependency. +# For the Pages deploy this comes from the repository variable AGENT_API_BASE. # VITE_AGENT_API_BASE=same-origin # VITE_AGENT_API_BASE=http://localhost:8787 diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 86e316b..fb51a5a 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -51,6 +51,13 @@ jobs: - name: Build run: pnpm build + env: + # Optional. Set the repository variable AGENT_API_BASE to a tool proxy + # origin and every visitor's DuckDuckGo search and page reads go there + # without touching the Tools panel. Unset, the site stays + # browser-direct, which is what a fork with no proxy needs. Either way + # a proxy that fails falls back to calling the provider from the page. + VITE_AGENT_API_BASE: ${{ vars.AGENT_API_BASE }} - name: Add SPA fallback # Pages serves 404.html for unknown paths; making it the app shell keeps diff --git a/README.md b/README.md index 82c53c3..9d010de 100644 --- a/README.md +++ b/README.md @@ -304,10 +304,16 @@ GitHub Pages cannot host a process, so the published site stays browser-direct. - **`pnpm proxy`** — the same handlers on http://localhost:8787, for a static build or the hosted site. Paste that origin into **Tools → Tool proxy URL**, or build with `VITE_AGENT_API_BASE=http://localhost:8787`. - **Railway (or any host)** — the `Dockerfile` in the repo root runs only this process. Create a project, connect `devbadya/Jarvis`, set `PROXY_ORIGINS` to `https://devbadya.github.io`, wait until the deploy is live, then **Settings → Networking → Generate domain**. That `https://….up.railway.app` origin is the URL: paste it into **Tools → Tool proxy URL** on the hosted site. There is no URL until a domain exists; the Hobby plan alone does not create one. +To give every visitor that proxy without asking them to paste anything, set the repository variable **`AGENT_API_BASE`** to its origin: `deploy.yml` passes it to the build as `VITE_AGENT_API_BASE`. Leave it unset and the hosted site stays browser-direct, which is what a fork with no proxy of its own needs — an empty value is not a proxy, so a workflow forwarding a variable nobody set cannot aim the build at an `/api` the host does not serve. + +**A proxy failure is not a failed turn.** Both tools try the proxy first and fall back to calling the provider from the page, so an outage, a spent budget or an allowlist that has not caught up costs a slower search rather than the answer. That fallback is what makes it safe to point every visitor at one process. + The proxy scrapes DuckDuckGo HTML itself and fetches pages itself. It does not spend the Jina reader budget, and it is not limited to CORS-friendly endpoints. Wikipedia, LangSearch and Jina still leave the tab directly — they already send the headers, and their keys must not travel through this process. A fetch-on-behalf proxy is still a confused deputy. Every target is resolved and refused if it lands on loopback, link-local or RFC1918, and redirects are re-checked. Do not bind `pnpm proxy` to the public internet without setting `PROXY_ORIGINS` to the pages that may call it (for example `https://devbadya.github.io`). Inference never goes through it. +The allowlist says who may call, not how often, and an allowed page is exactly what a scraper would forge. `pnpm proxy` therefore allows **30 requests a minute per caller** and answers `429` beyond that, counted from the forwarded address rather than the socket, since every edge terminates the connection itself. `PROXY_RATE_LIMIT` changes the number; `0` switches it off. Health checks are exempt. + ### What leaves the browser Inference does not: prompts, reasoning, and replies never leave the GPU, and neither do [memories](#memory), which are written to IndexedDB in this browser and read back into a prompt that goes no further than the GPU either. Tools are the exception, and always were. A `web_search` call sends the query to the chosen provider, a `read_page` call sends the URL to the reader, a `weather` call sends the place name to Open-Meteo's geocoder and its coordinates to the two forecast services, and a `current_time` call with a place sends the name to the same geocoder. diff --git a/src/tools/web.test.ts b/src/tools/web.test.ts index 79e866b..18c0441 100644 --- a/src/tools/web.test.ts +++ b/src/tools/web.test.ts @@ -755,6 +755,20 @@ describe('configuredProxyBase', () => { ) expect(configuredProxyBase({ provider: 'duckduckgo', proxyUrl: 'localhost:8787' })).toBeUndefined() }) + + // A workflow forwarding a repository variable nobody set hands the build an + // empty string. Reading that as same-origin would aim every hosted visitor at + // an `/api` the static host does not serve. + it('treats an empty build-time value as no proxy', () => { + vi.stubEnv('VITE_AGENT_API_BASE', '') + expect(configuredProxyBase({ provider: 'duckduckgo' })).toBeUndefined() + + vi.stubEnv('VITE_AGENT_API_BASE', 'same-origin') + expect(configuredProxyBase({ provider: 'duckduckgo' })).toBe('') + + vi.stubEnv('VITE_AGENT_API_BASE', 'https://proxy.example/') + expect(configuredProxyBase({ provider: 'duckduckgo' })).toBe('https://proxy.example') + }) }) describe('searchWeb with a tool proxy', () => { @@ -779,9 +793,30 @@ describe('searchWeb with a tool proxy', () => { expect(lastRequest(fetchMock).body).toMatchObject({ region: 'de-de' }) }) - it('relays a proxy error rather than wrapping it', async () => { - stubFetch(jsonResponse({ error: 'Refusing to fetch a private or loopback address' }, 502)) - await expect(searchWeb('x', 5, proxied)).rejects.toThrow(/private or loopback/) + // A hosted build points every visitor at one proxy, so its bad day must not + // become theirs. Browser-direct is the same path this build took before. + it('falls back to the reader when the proxy fails', async () => { + const fetchMock = stubFetch( + jsonResponse({ error: 'Origin not allowed' }, 403), + jsonResponse({ + data: { + content: [ + '1.[WebGPU](https://duckduckgo.com/l/?uddg=https%3A%2F%2Fwebgpu.org%2F&rut=1)', + 'A GPU API for the web.', + ].join('\n'), + }, + }), + ) + + const results = await searchWeb('webgpu', 3, proxied) + + expect(lastRequest(fetchMock).url.href).toContain('r.jina.ai') + expect(results.at(0)).toMatchObject({ title: 'WebGPU', url: 'https://webgpu.org/' }) + }) + + it('reports the reader failure when neither path works', async () => { + stubFetch(jsonResponse({ error: 'Too many requests' }, 429), jsonResponse({}, 500), jsonResponse({}, 500)) + await expect(searchWeb('x', 5, proxied)).rejects.toThrow() }) it('does not send Wikipedia through the proxy', async () => { @@ -803,6 +838,18 @@ describe('readPage with a tool proxy', () => { expect(lastRequest(fetchMock).body).toEqual({ url: 'https://example.com/a' }) expect(page).toEqual({ url: 'https://example.com/a', title: 'A', text: 'Body.' }) }) + + it('falls back to the reader when the proxy fails', async () => { + const fetchMock = stubFetch( + jsonResponse({ error: 'Too many requests' }, 429), + jsonResponse({ data: { url: 'https://example.com/a', title: 'A', content: 'Body.' } }), + ) + + const page = await readPage('https://example.com/a', proxied) + + expect(lastRequest(fetchMock).url.href).toContain('r.jina.ai') + expect(page).toEqual({ url: 'https://example.com/a', title: 'A', text: 'Body.' }) + }) }) describe('missingSearchKey', () => { diff --git a/src/tools/web.ts b/src/tools/web.ts index 5e38037..2bbdfdd 100644 --- a/src/tools/web.ts +++ b/src/tools/web.ts @@ -146,6 +146,10 @@ export function normalizeWebAccess( * Where DuckDuckGo search and page reads should go, or `undefined` to stay * browser-direct. An empty string means same-origin `/api`, which is what * `pnpm dev` serves. + * + * An empty variable is not a proxy. A workflow that forwards an unset + * repository variable produces `''`, and reading that as same-origin would + * point every hosted visitor at a `/api` their host does not serve. */ export function configuredProxyBase(config: WebAccessConfig): string | undefined { const runtime = config.proxyUrl?.trim() @@ -161,7 +165,8 @@ export function configuredProxyBase(config: WebAccessConfig): string | undefined const env = import.meta.env.VITE_AGENT_API_BASE if (typeof env !== 'string') return undefined const trimmed = env.trim() - if (trimmed === '' || trimmed === 'same-origin') return '' + if (trimmed === '') return undefined + if (trimmed === 'same-origin') return '' return trimmed.replace(/\/$/, '') } @@ -712,7 +717,16 @@ export async function searchWeb( return searchWikipedia(query, limit) } const proxy = configuredProxyBase(config) - if (proxy !== undefined) return searchViaProxy(proxy, query, limit) + if (proxy !== undefined) { + try { + return await searchViaProxy(proxy, query, limit) + } catch { + // The proxy is an optimisation, not a dependency. A hosted build points + // every visitor at one process, so an outage, a spent budget or an + // allowlist that has not caught up must cost a slower search rather than + // the answer. Browser-direct is what this build did before the proxy. + } + } return searchDuckDuckGo(query, limit, config) } @@ -833,7 +847,16 @@ export async function readPage(rawUrl: string, config: WebAccessConfig): Promise } const proxy = configuredProxyBase(config) - if (proxy !== undefined) return fetchViaProxy(proxy, url.toString()) + if (proxy !== undefined) { + try { + return await fetchViaProxy(proxy, url.toString()) + } catch { + // Same bargain as search: the reader behind this still works, and a page + // the proxy could not fetch is worth one more attempt rather than an + // apology. `assertPublicHttpUrl` already refused the private targets, so + // nothing the proxy blocks on principle reaches this line. + } + } const data = await readWithReader(url.toString(), 'The page reader', config) if (!data?.content) throw new Error(`No readable content found at ${url.toString()}`) diff --git a/src/vite-env.d.ts b/src/vite-env.d.ts index 5329a56..9a13ea4 100644 --- a/src/vite-env.d.ts +++ b/src/vite-env.d.ts @@ -6,8 +6,9 @@ interface ImportMetaEnv { /** Path layout under that host, with {model} and {revision} placeholders. */ readonly VITE_MODEL_PATH_TEMPLATE?: string /** - * Origin of the optional tool proxy. `same-origin` (or an empty value) means - * this page's `/api`. Unset on the Pages build, so tools stay browser-direct. + * Origin of the optional tool proxy. `same-origin` means this page's `/api`. + * Empty or unset stays browser-direct, which is what a Pages build with no + * proxy of its own gets. */ readonly VITE_AGENT_API_BASE?: string } diff --git a/tools/agent-api-listen.ts b/tools/agent-api-listen.ts index c9b3f88..1bb6425 100644 --- a/tools/agent-api-listen.ts +++ b/tools/agent-api-listen.ts @@ -10,7 +10,7 @@ */ import { createServer, type IncomingMessage, type ServerResponse } from 'node:http' -import { handleAgentApiRequest } from './agent-api.ts' +import { createRateLimiter, handleAgentApiRequest } from './agent-api.ts' const PORT = Number(process.env.PORT) || 8787 const HOST = process.env.HOST ?? '0.0.0.0' @@ -19,6 +19,21 @@ const ALLOWLIST = (process.env.PROXY_ORIGINS ?? '') .map((origin) => origin.trim()) .filter(Boolean) +const RATE_LIMIT = Number(process.env.PROXY_RATE_LIMIT ?? 30) +const RATE_WINDOW_MS = 60_000 +const limiter = createRateLimiter(RATE_LIMIT, RATE_WINDOW_MS) + +/** + * Railway and every other edge terminate the connection themselves, so the + * socket address is the proxy in front of us and would put every visitor in one + * bucket. The left-most forwarded address is the caller. + */ +function callerKey(req: IncomingMessage): string { + const forwarded = req.headers['x-forwarded-for'] + const first = (Array.isArray(forwarded) ? forwarded[0] : forwarded)?.split(',')[0]?.trim() + return first || req.socket.remoteAddress || 'unknown' +} + function allowedOrigin(req: IncomingMessage): string | undefined { const origin = req.headers.origin if (ALLOWLIST.length === 0) return origin ?? '*' @@ -50,6 +65,16 @@ const server = createServer((req, res) => { res.end() return } + // Health is exempt: the platform polls it far more often than a person + // searches, and a restart loop triggered by our own limit would be absurd. + const url = new URL(req.url ?? '/', 'http://localhost') + if (url.pathname !== '/api/health' && !limiter.take(callerKey(req))) { + res.statusCode = 429 + res.setHeader('content-type', 'application/json; charset=utf-8') + res.setHeader('retry-after', String(Math.ceil(RATE_WINDOW_MS / 1000))) + res.end(JSON.stringify({ error: 'Too many requests' })) + return + } const handled = await handleAgentApiRequest(req, res) if (!handled) { res.statusCode = 404 @@ -70,4 +95,5 @@ server.listen(PORT, HOST, () => { console.log('POST /api/search { query, limit?, region? }') console.log('POST /api/fetch { url }') if (ALLOWLIST.length > 0) console.log(`Origins: ${ALLOWLIST.join(', ')}`) + console.log(RATE_LIMIT > 0 ? `Rate limit: ${RATE_LIMIT} per minute per caller` : 'Rate limit: off') }) diff --git a/tools/agent-api.test.ts b/tools/agent-api.test.ts index 1269c40..520d854 100644 --- a/tools/agent-api.test.ts +++ b/tools/agent-api.test.ts @@ -1,6 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { assertPublicUrl, + createRateLimiter, decodeEntities, extractPage, isBlockedHostname, @@ -227,3 +228,42 @@ describe('routeAgentApi', () => { expect(result.payload).toEqual({ error: 'Refusing to fetch a private or loopback address' }) }) }) + +describe('createRateLimiter', () => { + it('allows the quota and refuses the next call', () => { + const limiter = createRateLimiter(3, 60_000) + expect([1, 2, 3].map(() => limiter.take('1.2.3.4', 1000))).toEqual([true, true, true]) + expect(limiter.take('1.2.3.4', 1000)).toBe(false) + }) + + it('counts each caller on its own', () => { + const limiter = createRateLimiter(1, 60_000) + expect(limiter.take('1.2.3.4', 1000)).toBe(true) + expect(limiter.take('1.2.3.4', 1000)).toBe(false) + expect(limiter.take('5.6.7.8', 1000)).toBe(true) + }) + + it('lets the window slide rather than resetting on a tick', () => { + const limiter = createRateLimiter(2, 60_000) + limiter.take('1.2.3.4', 1000) + limiter.take('1.2.3.4', 30_000) + expect(limiter.take('1.2.3.4', 50_000)).toBe(false) + // The first call has aged out by now; the second has not. + expect(limiter.take('1.2.3.4', 62_000)).toBe(true) + expect(limiter.take('1.2.3.4', 62_000)).toBe(false) + }) + + // The map is the one thing here that grows with the number of strangers who + // find the URL, so callers that stopped must not stay in it. + it('forgets a caller that fell silent', () => { + const limiter = createRateLimiter(1, 60_000) + limiter.take('1.2.3.4', 1000) + limiter.take('5.6.7.8', 200_000) + expect(limiter.take('1.2.3.4', 200_000)).toBe(true) + }) + + it('is off when the limit is zero', () => { + const limiter = createRateLimiter(0, 60_000) + expect([1, 2, 3, 4].every(() => limiter.take('1.2.3.4', 1000))).toBe(true) + }) +}) diff --git a/tools/agent-api.ts b/tools/agent-api.ts index 98cf13e..9855bec 100644 --- a/tools/agent-api.ts +++ b/tools/agent-api.ts @@ -346,6 +346,47 @@ function clampLimit(value: unknown): number { return Math.min(Math.max(Math.trunc(n) || 5, 1), MAX_SEARCH_LIMIT) } +export interface RateLimiter { + /** True when this caller may proceed. Counts the call when it does. */ + take(key: string, now?: number): boolean +} + +/** + * A sliding window per caller, in memory. + * + * The origin allowlist says which page may call; it says nothing about how + * often, and an allowed page is exactly what a scraper would forge. The window + * is the difference between one visitor searching and a stranger spending a + * month of budget in an afternoon. One process holds one window, so a service + * on several replicas allows that multiple — still a ceiling, which is the + * point. + */ +export function createRateLimiter(limit: number, windowMs: number): RateLimiter { + const seen = new Map() + + return { + take(key, now = Date.now()) { + if (limit <= 0) return true + const cutoff = now - windowMs + const hits = (seen.get(key) ?? []).filter((at) => at > cutoff) + + // Callers that stopped must not stay in memory: this map is the only + // thing here that grows with the number of strangers who found the URL. + for (const [other, times] of seen) { + if (other !== key && times.every((at) => at <= cutoff)) seen.delete(other) + } + + if (hits.length >= limit) { + seen.set(key, hits) + return false + } + hits.push(now) + seen.set(key, hits) + return true + }, + } +} + export async function routeAgentApi( method: string, pathname: string,