Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -409,18 +409,30 @@ 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
Routes). A Custom Domain silently takes precedence over a route covering the same hostname, so
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
Expand Down
22 changes: 17 additions & 5 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -411,18 +411,30 @@ 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
Routes). A Custom Domain silently takes precedence over a route covering the same hostname, so
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
Expand Down
14 changes: 4 additions & 10 deletions client/src/app/onboarding/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -204,9 +197,10 @@ function TokenGuide() {
Name it <span className="mono" style={{ color: 'var(--text-2)' }}>EdgeBalancer</span>
</TokenStep>
<TokenStep n="04">
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:
<div style={{ display: 'flex', flexDirection: 'column', gap: 6, marginTop: 10 }}>
{PERMISSIONS.map(([scope, resource, level]) => (
{CLOUDFLARE_PERMISSIONS.map(([scope, resource, level]) => (
<div key={`${scope}-${resource}`} style={{
display: 'flex', alignItems: 'center', justifyContent: 'space-between', gap: 8,
padding: '7px 10px', borderRadius: 'var(--radius)',
Expand Down
3 changes: 2 additions & 1 deletion client/src/app/settings/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { api } from '@/lib/api';
import { Sidebar, Topbar } from '@/components/dashboard/Sidebar';
import { Icons } from '@/components/shared/Icons';
import toast from 'react-hot-toast';
import { permissionSummary } from '@/lib/cloudflarePermissions';

export default function SettingsPage() {
const router = useRouter();
Expand Down Expand Up @@ -227,7 +228,7 @@ function CloudflareTab({ user, refreshUser }: any) {
disabled={loading}
/>
<div className="hint">
Token must have <span className="mono" style={{ color: 'var(--accent)' }}>Workers Scripts: Edit</span>, <span className="mono" style={{ color: 'var(--accent)' }}>Account Analytics: Read</span>, and <span className="mono" style={{ color: 'var(--accent)' }}>Zone: Read</span> permissions
Token must have <span className="mono" style={{ color: 'var(--accent)' }}>{permissionSummary()}</span>
</div>
</div>

Expand Down
19 changes: 19 additions & 0 deletions client/src/lib/cloudflarePermissions.ts
Original file line number Diff line number Diff line change
@@ -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(', ');
4 changes: 2 additions & 2 deletions server/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion server/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "server",
"version": "2.3.0",
"version": "2.3.2",
"description": "",
"main": "index.js",
"scripts": {
Expand Down
1 change: 0 additions & 1 deletion server/src/__tests__/integration/cloudflare.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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([]),
Expand Down
1 change: 0 additions & 1 deletion server/src/__tests__/integration/loadbalancer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
25 changes: 25 additions & 0 deletions server/src/__tests__/unit/hostname.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' }]);

Expand Down
20 changes: 19 additions & 1 deletion server/src/modules/loadbalancer/services/hostname.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
));
Expand Down
12 changes: 0 additions & 12 deletions server/src/services/cloudflareClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,6 @@ export class CloudflareClient {
}
}

async testWorkersKVPermission(accountId: string): Promise<boolean> {
try {
await retryWithBackoff(
() => this.client.get(`/accounts/${accountId}/storage/kv/namespaces`),
{ maxRetries: 2 }
);
return true;
} catch (error) {
return false;
}
}

async testZoneReadPermission(accountId: string): Promise<boolean> {
try {
await retryWithBackoff(
Expand Down
6 changes: 0 additions & 6 deletions server/src/services/credentialsService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading