diff --git a/AGENTS.md b/AGENTS.md index 4e71a74..75e3e2f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -409,10 +409,20 @@ quota. All Cloudflare API calls are **server-only** (never from client). -**Required token permissions:** -- Account > Worker Scripts > Edit -- Account > Workers KV Storage > Edit -- Zone > Zone > Read +**Required token permissions** — every one maps to an endpoint the code actually calls: + +| Permission | Used by | +|---|---| +| Account > Workers Scripts > Edit | `/accounts/{id}/workers/scripts` (deploy, versions, deployments, delete) and `/accounts/{id}/workers/domains` | +| Account > Account Analytics > Read | `/client/v4/graphql` — request/error counts | +| Zone > Zone > Read | `/zones` — zone list for the domain picker | +| Zone > DNS > Edit | `/zones/{id}/dns_records` — grey-cloud records for raw-IP origins | + +**Optional:** Zone > Workers Routes > Read — `/zones/{id}/workers/routes`, the Worker Routes half of +the hostname conflict check. Without it that check is skipped with a warning; deploys still work. + +**Not needed:** Workers KV Storage. Nothing binds or reads KV — it was only ever probed by its own +validation check, which has been removed. **Hostname conflict detection:** `assertHostnameAvailable` checks two independent bindings — `GET /accounts/{id}/workers/domains` (Custom Domains) and `GET /zones/{id}/workers/routes` (Worker @@ -420,7 +430,9 @@ Routes). A Custom Domain silently takes precedence over a route covering the sam checking only the former would move live traffic off whatever Worker the route points at. Route patterns are matched with `routePatternCoversHostname` (`*` = zero or more chars, path ignored); routes with no `script` attached are bypass rules and are not conflicts. The route check needs a -`zoneId` and is skipped when the caller has none. +`zoneId` and is skipped when the caller has none. It is also skipped — with a warning, never an +error — when the token returns 403/404 for the routes endpoint, since that permission is not part +of the documented minimum and a missing safety net must not block deploys. **Key CF operations:** - `PUT /accounts/{id}/workers/scripts/{name}` — deploy Worker diff --git a/CLAUDE.md b/CLAUDE.md index 05b57a3..a137aa7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -411,10 +411,20 @@ quota. All Cloudflare API calls are **server-only** (never from client). -**Required token permissions:** -- Account > Worker Scripts > Edit -- Account > Workers KV Storage > Edit -- Zone > Zone > Read +**Required token permissions** — every one maps to an endpoint the code actually calls: + +| Permission | Used by | +|---|---| +| Account > Workers Scripts > Edit | `/accounts/{id}/workers/scripts` (deploy, versions, deployments, delete) and `/accounts/{id}/workers/domains` | +| Account > Account Analytics > Read | `/client/v4/graphql` — request/error counts | +| Zone > Zone > Read | `/zones` — zone list for the domain picker | +| Zone > DNS > Edit | `/zones/{id}/dns_records` — grey-cloud records for raw-IP origins | + +**Optional:** Zone > Workers Routes > Read — `/zones/{id}/workers/routes`, the Worker Routes half of +the hostname conflict check. Without it that check is skipped with a warning; deploys still work. + +**Not needed:** Workers KV Storage. Nothing binds or reads KV — it was only ever probed by its own +validation check, which has been removed. **Hostname conflict detection:** `assertHostnameAvailable` checks two independent bindings — `GET /accounts/{id}/workers/domains` (Custom Domains) and `GET /zones/{id}/workers/routes` (Worker @@ -422,7 +432,9 @@ Routes). A Custom Domain silently takes precedence over a route covering the sam checking only the former would move live traffic off whatever Worker the route points at. Route patterns are matched with `routePatternCoversHostname` (`*` = zero or more chars, path ignored); routes with no `script` attached are bypass rules and are not conflicts. The route check needs a -`zoneId` and is skipped when the caller has none. +`zoneId` and is skipped when the caller has none. It is also skipped — with a warning, never an +error — when the token returns 403/404 for the routes endpoint, since that permission is not part +of the documented minimum and a missing safety net must not block deploys. **Key CF operations:** - `PUT /accounts/{id}/workers/scripts/{name}` — deploy Worker diff --git a/client/src/app/onboarding/page.tsx b/client/src/app/onboarding/page.tsx index 0a3e173..af79606 100644 --- a/client/src/app/onboarding/page.tsx +++ b/client/src/app/onboarding/page.tsx @@ -7,14 +7,7 @@ import { api } from '@/lib/api'; import toast from 'react-hot-toast'; import { AuthLayout } from '@/components/auth/AuthLayout'; import { Icons } from '@/components/shared/Icons'; - -const PERMISSIONS = [ - ['Account', 'Worker Scripts', 'Edit'], - ['Account', 'Workers KV Storage', 'Edit'], - ['Account', 'Account Analytics', 'Read'], - ['Zone', 'Zone', 'Read'], - ['Zone', 'DNS', 'Edit'], -]; +import { CLOUDFLARE_PERMISSIONS } from '@/lib/cloudflarePermissions'; export default function OnboardingPage() { const router = useRouter(); @@ -204,9 +197,10 @@ function TokenGuide() { Name it EdgeBalancer - Add all five permissions: + {/* Counted from the list so the copy can never drift out of step with it again. */} + Add all {CLOUDFLARE_PERMISSIONS.length} permissions:
- {PERMISSIONS.map(([scope, resource, level]) => ( + {CLOUDFLARE_PERMISSIONS.map(([scope, resource, level]) => (
- Token must have Workers Scripts: Edit, Account Analytics: Read, and Zone: Read permissions + Token must have {permissionSummary()}
diff --git a/client/src/lib/cloudflarePermissions.ts b/client/src/lib/cloudflarePermissions.ts new file mode 100644 index 0000000..a530af7 --- /dev/null +++ b/client/src/lib/cloudflarePermissions.ts @@ -0,0 +1,19 @@ +/** + * The Cloudflare API token scopes EdgeBalancer asks for. + * + * Shared so onboarding and settings cannot disagree — they already had, with settings naming + * three of the five that onboarding listed. + */ +export const CLOUDFLARE_PERMISSIONS: Array<[scope: string, resource: string, level: string]> = [ + ['Account', 'Workers Scripts', 'Edit'], + ['Account', 'Account Analytics', 'Read'], + ['Zone', 'Zone', 'Read'], + ['Zone', 'DNS', 'Edit'], + // Lets us detect a hostname already served by another Worker through a route. Deploys still + // work without it — the check is skipped with a warning. + ['Zone', 'Workers Routes', 'Read'], +]; + +/** "Workers Scripts: Edit, Account Analytics: Read, …" — for inline hints. */ +export const permissionSummary = (): string => + CLOUDFLARE_PERMISSIONS.map(([, resource, level]) => `${resource}: ${level}`).join(', '); diff --git a/server/package-lock.json b/server/package-lock.json index e2b5d6a..2c1aacf 100644 --- a/server/package-lock.json +++ b/server/package-lock.json @@ -1,12 +1,12 @@ { "name": "server", - "version": "2.3.0", + "version": "2.3.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "server", - "version": "2.3.0", + "version": "2.3.2", "license": "ISC", "dependencies": { "@fastify/rate-limit": "^11.0.0", diff --git a/server/package.json b/server/package.json index d2010d5..9a20311 100644 --- a/server/package.json +++ b/server/package.json @@ -1,6 +1,6 @@ { "name": "server", - "version": "2.3.0", + "version": "2.3.2", "description": "", "main": "index.js", "scripts": { diff --git a/server/src/__tests__/integration/cloudflare.test.ts b/server/src/__tests__/integration/cloudflare.test.ts index a66a790..3fe31ad 100644 --- a/server/src/__tests__/integration/cloudflare.test.ts +++ b/server/src/__tests__/integration/cloudflare.test.ts @@ -37,7 +37,6 @@ beforeEach(() => { MockCloudflareClient.mockImplementation(() => ({ getZones: jest.fn().mockResolvedValue({ result: [{ id: 'zone1', name: 'example.com', status: 'active' }] }), testWorkerScriptsPermission: jest.fn().mockResolvedValue(true), - testWorkersKVPermission: jest.fn().mockResolvedValue(true), testZoneReadPermission: jest.fn().mockResolvedValue(true), workerNameExists: jest.fn().mockResolvedValue(false), getWorkerDomains: jest.fn().mockResolvedValue([]), diff --git a/server/src/__tests__/integration/loadbalancer.test.ts b/server/src/__tests__/integration/loadbalancer.test.ts index d818174..de14003 100644 --- a/server/src/__tests__/integration/loadbalancer.test.ts +++ b/server/src/__tests__/integration/loadbalancer.test.ts @@ -85,7 +85,6 @@ beforeEach(() => { getWorkerDomains: jest.fn().mockResolvedValue([]), getWorkerRoutes: jest.fn().mockResolvedValue([]), testWorkerScriptsPermission: jest.fn().mockResolvedValue(true), - testWorkersKVPermission: jest.fn().mockResolvedValue(true), testZoneReadPermission: jest.fn().mockResolvedValue(true), getZones: jest.fn().mockResolvedValue({ result: [] }), } as any)); diff --git a/server/src/__tests__/unit/hostname.test.ts b/server/src/__tests__/unit/hostname.test.ts index 1eb6f75..70ea71c 100644 --- a/server/src/__tests__/unit/hostname.test.ts +++ b/server/src/__tests__/unit/hostname.test.ts @@ -105,6 +105,31 @@ describe('assertHostnameAvailable — Worker Routes', () => { expect(getWorkerRoutes).not.toHaveBeenCalled(); }); + it('still deploys when the token cannot list routes', async () => { + // Tokens issued before this check exists only carry Zone > Zone > Read. + getWorkerRoutes.mockRejectedValue(Object.assign(new Error('Request failed with status code 403'), { + response: { status: 403 }, + })); + + await expect(assertHostnameAvailable(params)).resolves.toBeUndefined(); + }); + + it('treats a 404 on the routes endpoint the same way', async () => { + getWorkerRoutes.mockRejectedValue(Object.assign(new Error('Not found'), { + response: { status: 404 }, + })); + + await expect(assertHostnameAvailable(params)).resolves.toBeUndefined(); + }); + + it('does not swallow a genuine Cloudflare outage', async () => { + getWorkerRoutes.mockRejectedValue(Object.assign(new Error('Bad gateway'), { + response: { status: 502 }, + })); + + await expect(assertHostnameAvailable(params)).rejects.toThrow('Bad gateway'); + }); + it('still reports a Custom Domain conflict before looking at routes', async () => { getWorkerDomains.mockResolvedValue([{ hostname: 'ankan.in' }]); diff --git a/server/src/modules/loadbalancer/services/hostname.service.ts b/server/src/modules/loadbalancer/services/hostname.service.ts index 05630d2..4a119ab 100644 --- a/server/src/modules/loadbalancer/services/hostname.service.ts +++ b/server/src/modules/loadbalancer/services/hostname.service.ts @@ -83,7 +83,25 @@ export async function assertHostnameAvailable(params: { // A Custom Domain silently takes precedence over any route covering the same hostname, so // without this check we would move live traffic off whatever Worker the route points at. // EdgeBalancer only ever creates Custom Domains, so every route found here is foreign. - const routes = await cloudflareClient.getWorkerRoutes(zoneId); + let routes: any[]; + try { + routes = await cloudflareClient.getWorkerRoutes(zoneId); + } catch (error: any) { + const status = error?.response?.status; + + // Listing routes needs `Zone > Workers Routes > Read`, which older tokens do not carry. + // This check is an extra safety net, not a prerequisite for deploying — refusing to create + // because we cannot read routes would break every account issued a token before it existed. + if (status === 403 || status === 404) { + console.warn( + `Worker Routes check skipped for '${hostname}': the Cloudflare token lacks Zone > Workers Routes > Read.`, + ); + return; + } + + throw error; + } + const conflicting = routes.find((route: any) => ( route?.script && routePatternCoversHostname(route.pattern, hostname) )); diff --git a/server/src/services/cloudflareClient.ts b/server/src/services/cloudflareClient.ts index 577237c..1dc192f 100644 --- a/server/src/services/cloudflareClient.ts +++ b/server/src/services/cloudflareClient.ts @@ -29,18 +29,6 @@ export class CloudflareClient { } } - async testWorkersKVPermission(accountId: string): Promise { - try { - await retryWithBackoff( - () => this.client.get(`/accounts/${accountId}/storage/kv/namespaces`), - { maxRetries: 2 } - ); - return true; - } catch (error) { - return false; - } - } - async testZoneReadPermission(accountId: string): Promise { try { await retryWithBackoff( diff --git a/server/src/services/credentialsService.ts b/server/src/services/credentialsService.ts index 3ad7fc0..c422239 100644 --- a/server/src/services/credentialsService.ts +++ b/server/src/services/credentialsService.ts @@ -21,12 +21,6 @@ export const validateCloudflareCredentials = async ( errors.push('Missing permission: Account > Worker Scripts > Edit'); } - // Test Workers KV Storage permission - const hasWorkersKV = await client.testWorkersKVPermission(accountId); - if (!hasWorkersKV) { - errors.push('Missing permission: Account > Workers KV Storage > Edit'); - } - // Test Zone Read permission const hasZoneRead = await client.testZoneReadPermission(accountId); if (!hasZoneRead) {