Skip to content

Commit 523264d

Browse files
iPLAYCAFE-devBrooooooklynclaude
authored
fix(cli): do not require DNS Edit when a plan only adopts a DNS record (#668)
--- ### Problem If a zone already has a compatible, proxied wildcard record that no installation has recorded, `addDnsPlan` plans an `adopt` action for it. The description is "Use the existing compatible wildcard DNS record without taking ownership": https://github.com/voidzero-dev/void/blob/afeeff6a4d72682f6cfb6c947e9a2c29393c00a6/packages/void/src/cli/platform-plan.ts#L929-L938 The resource phase never calls the DNS API for an `adopt` action. It only records the adoption in the installation manifest and checkpoint: https://github.com/voidzero-dev/void/blob/afeeff6a4d72682f6cfb6c947e9a2c29393c00a6/packages/void/src/cli/platform-installer-resources.ts#L62-L83 `planWritesDns`, however, counts every `dns-record` action other than `reuse` as a DNS write, so `adopt` is included: https://github.com/voidzero-dev/void/blob/afeeff6a4d72682f6cfb6c947e9a2c29393c00a6/packages/void/src/cli/platform-auth.ts#L145-L161 As a result, browser (OAuth) login is treated as unable to apply a plan that makes no DNS change: - **Non-interactive:** the install stops with `Cloudflare browser login cannot authorize the planned zone or DNS changes. Set CLOUDFLARE_API_TOKEN with Zone → DNS → Edit …`. - **Interactive:** the installer opens the management token page and prompts for a management API token: https://github.com/voidzero-dev/void/blob/afeeff6a4d72682f6cfb6c947e9a2c29393c00a6/packages/void/src/cli/platform-lifecycle.ts#L799-L836 The same check also runs for `void platform domain set`. ### Repro (synthetic) 1. In a Cloudflare zone `example.app`, create a proxied `AAAA` record `*.example.app` → `100::` by hand. No Void installation has recorded it. 2. Sign in with browser login only, so that neither `CLOUDFLARE_API_TOKEN` nor `CF_API_TOKEN` is set. 3. Run `void platform install --application-domain example.app --zone example.app`. 4. The plan shows `adopt` for `application-dns`. Before this change, the installer then asks for a management token with **Zone → DNS → Edit** (interactive), or exits with the error above (non-interactive). ### Change - `planWritesDns` in `packages/void/src/cli/platform-auth.ts` now treats only `create` as a DNS write. `reuse` and `adopt` no longer count. - The "Required Credentials" summary in `formatPlatformSetupPlan` has the same predicate inline, so it now uses the same rule. Without this, `--plan` would still list "DNS changes" for an adopt-only plan: https://github.com/voidzero-dev/void/blob/afeeff6a4d72682f6cfb6c947e9a2c29393c00a6/packages/void/src/cli/platform-setup.ts#L247-L251 Zone creation, Access setup, and `create` of a DNS record still require a management token, exactly as before. ### Tests This extends `requires an API token before browser OAuth applies zone or DNS writes` in `packages/void/test/unit/platform-auth.test.ts`: https://github.com/voidzero-dev/void/blob/afeeff6a4d72682f6cfb6c947e9a2c29393c00a6/packages/void/test/unit/platform-auth.test.ts#L168-L196 For an OAuth credential: | Plan contains | `platformOperatorNeedsManagementToken` | | --- | --- | | `dns-record` `adopt` | `false` | | `dns-record` `reuse` | `false` | | `dns-record` `create` | `true` | `assertPlatformOperatorCredentialCanApplyPlan` does not throw for the adopt-only plan. ### Verification ``` $ vp test run packages/void/test/unit/platform-auth.test.ts packages/void/test/unit/platform-setup-flow.test.ts packages/void/test/unit/platform-setup.test.ts Test Files 3 passed (3) Tests 131 passed (131) $ vp run --filter ./packages/void typecheck # exit 0 $ vp fmt --check / vp lint on the changed files # clean ``` `vp run test` on this machine has 60 pre-existing failures that also fail on `main` without this change (Playwright Chromium not installed, playground output not generated); none are in the touched files. With the source fix reverted, the new `platform-auth` test fails (`expected true to be false`). The `platform-setup-flow` fixture used a DNS `adopt` action as its example of "needs a DNS write"; it now uses `create`. _Prepared with Claude Code on behalf of @iPLAYCAFE-dev; reviewed by the maintainer of this account._ ### Maintainer follow-up ``` plan action DNS API call needs Zone → DNS → Edit dns-record create yes yes dns-record adopt no no ← this PR dns-record reuse no no zone create — Account → Zone → Edit ``` - `planCreatesZone` / `planWritesDns` now live in `platform-plan-writes.ts`, which has only type imports. `formatPlatformSetupPlan` calls them instead of keeping its own copy. This keeps the setup prompts from loading the auth module's runtime imports. - `platform-auth.test.ts` shares one plan-action helper. - `docs/guide/platform/installation/{setup,credentials}.md`: a management token is needed when the install creates the zone or its wildcard record, or sets up Access. An existing proxied `*.<application domain>` record works with browser login. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: LongYinan <github@lyn.one> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent e1d6c5c commit 523264d

8 files changed

Lines changed: 79 additions & 29 deletions

File tree

‎docs/guide/platform/installation/credentials.md‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ Keep one password-manager entry for this platform. Paste the saved values into t
88

99
| Credential | Created by | Why the platform needs it |
1010
| -------------------------------------- | -------------------------------- | ---------------------------------------------------------------------------------------- |
11-
| Management API token | You, in Cloudflare | Lets the CLI change DNS and Access for a domain installation or Access setup. |
11+
| Management API token | You, in Cloudflare | Lets the CLI create a zone, change DNS, and set up Access. |
1212
| Runtime API token | You, in Cloudflare | Stored as a Worker secret so the platform can deploy apps after you close your terminal. |
1313
| PostgreSQL URL | You, with your database provider | Stores platform users, projects, and deployments. |
1414
| Login provider client ID and secret | You, or Void for Access login | Signs people in to the platform. |
@@ -20,11 +20,13 @@ Keep one password-manager entry for this platform. Paste the saved values into t
2020

2121
## Cloudflare API Tokens {#runtime-token-permissions}
2222

23-
For a domain installation or Cloudflare Access setup, the installer guides you through creating separate management and runtime tokens using Cloudflare's [API token setup](https://developers.cloudflare.com/fundamentals/api/get-started/create-token/). Its links preselect the permissions and suggest names based on your platform's display name, but Cloudflare's form cannot preselect a zone: for a domain installation, change the zone policy from **All Domains** to **Specified Domains** and select your zone. If Void will create the zone, keep **All Domains** on both tokens: the zone does not exist yet, and the runtime token needs it to purge cache. Save the tokens in your password manager and paste them into the masked CLI prompts.
23+
For a domain installation or Cloudflare Access setup, the installer guides you through creating tokens using Cloudflare's [API token setup](https://developers.cloudflare.com/fundamentals/api/get-started/create-token/): a runtime token, and a separate management token when the installation creates the zone or its wildcard DNS record, or sets up Access. Its links preselect the permissions and suggest names based on your platform's display name, but Cloudflare's form cannot preselect a zone: for a domain installation, change the zone policy from **All Domains** to **Specified Domains** and select your zone. If Void will create the zone, keep **All Domains** on both tokens: the zone does not exist yet, and the runtime token needs it to purge cache. Save the tokens in your password manager and paste them into the masked CLI prompts.
2424

2525
For workers.dev testing with the default API hostname, browser login can authorize
26-
infrastructure setup. Void also checks account-wide Access requirements; if the
27-
current credential cannot read them, the installer asks for a scoped Access token.
26+
infrastructure setup. So can a domain installation on an existing zone whose
27+
`*.<application domain>` record is already proxied. Void also checks account-wide
28+
Access requirements; if the current credential cannot read them, the installer
29+
asks for a scoped Access token.
2830
Protected apps and public apps under Default-Deny need an ongoing app-management
2931
token, stored encrypted. Access setup requires a management token even on
3032
workers.dev. Skip zone permissions until you add a domain.

‎docs/guide/platform/installation/setup.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ For workers.dev testing, start the interactive install and choose your login met
1212
void platform install --workers-dev
1313
```
1414

15-
For a domain installation or Access setup, Void guides you to create a management API token and paste it into a masked prompt. Browser login can preview DNS, but cannot change it or configure Access. The token-creation link preselects the permissions for your choices. When using an existing domain, change the zone policy from **All Domains** to **Specified Domains** and select your zone, as the CLI prompt describes. No shell environment variable is needed for interactive installation.
15+
When a domain installation creates the zone or its wildcard DNS record, or you set up Access, Void guides you to create a management API token and paste it into a masked prompt. Browser login can preview DNS and use an existing proxied `*.<application domain>` record, but cannot change DNS or configure Access. The token-creation link preselects the permissions for your choices. When using an existing domain, change the zone policy from **All Domains** to **Specified Domains** and select your zone, as the CLI prompt describes. No shell environment variable is needed for interactive installation.
1616

1717
::: details Use an existing management token from the shell
1818

‎packages/void/src/cli/platform-auth.ts‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
type CloudflareAccount,
55
type PlatformCloudflareClient,
66
} from './platform-cloudflare.ts';
7+
import { planCreatesZone, planWritesDns } from './platform-plan-writes.ts';
78
import type { PlatformInstallPlan } from './platform-plan.ts';
89

910
export type PlatformCloudflareCredential = Readonly<{
@@ -152,14 +153,6 @@ export function platformOperatorNeedsManagementToken(
152153
);
153154
}
154155

155-
function planCreatesZone(plan: Pick<PlatformInstallPlan, 'actions'>): boolean {
156-
return plan.actions.some((action) => action.kind === 'zone' && action.action === 'create');
157-
}
158-
159-
function planWritesDns(plan: Pick<PlatformInstallPlan, 'actions'>): boolean {
160-
return plan.actions.some((action) => action.kind === 'dns-record' && action.action !== 'reuse');
161-
}
162-
163156
export async function resolvePlatformCloudflareCredential(
164157
dependencies: PlatformCloudflareAuthDependencies = {},
165158
): Promise<PlatformCloudflareCredential> {
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import type { PlatformInstallPlan } from './platform-plan.ts';
2+
3+
// Type-only imports keep this module cheap to load from the setup prompts.
4+
5+
export function planCreatesZone(plan: Pick<PlatformInstallPlan, 'actions'>): boolean {
6+
return plan.actions.some((action) => action.kind === 'zone' && action.action === 'create');
7+
}
8+
9+
/**
10+
* Whether applying the plan writes a DNS record, which needs Zone → DNS → Edit.
11+
* 'reuse' changes nothing and 'adopt' only records an existing record in the
12+
* manifest; only 'create' calls the DNS API.
13+
*/
14+
export function planWritesDns(plan: Pick<PlatformInstallPlan, 'actions'>): boolean {
15+
return plan.actions.some(
16+
(action) =>
17+
action.kind === 'dns-record' && action.action !== 'reuse' && action.action !== 'adopt',
18+
);
19+
}

‎packages/void/src/cli/platform-setup.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { type CANCEL_SYMBOL, isCancel, log } from '@voidzero-dev/vite-plus-promp
33
import { openBrowser } from './auth.ts';
44
import { formatLink, pc } from './output.ts';
55
import type { PlatformManagementAccess } from './platform-auth.ts';
6+
import { planCreatesZone, planWritesDns } from './platform-plan-writes.ts';
67
import type { PlatformInstallPlan } from './platform-plan.ts';
78

89
type PlatformSetupPage = Readonly<{
@@ -244,11 +245,7 @@ export function formatPlatformSetupPlan(
244245
plan: PlatformInstallPlan,
245246
options: { managementToken?: boolean; managementAccess?: boolean } = {},
246247
): string {
247-
const writesDns = plan.actions.some(
248-
(action) =>
249-
(action.kind === 'zone' && action.action === 'create') ||
250-
(action.kind === 'dns-record' && action.action !== 'reuse'),
251-
);
248+
const writesDns = planCreatesZone(plan) || planWritesDns(plan);
252249
const credentials = [
253250
...(options.managementToken
254251
? [

‎packages/void/test/unit/platform-auth.test.ts‎

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -165,16 +165,16 @@ describe('platform Cloudflare authentication', () => {
165165
expect(request.mock.calls.every((call) => call.length === 1)).toBe(true);
166166
},
167167
);
168-
it('requires an API token before browser OAuth applies zone or DNS writes', () => {
169-
const action = (kind: 'dns-record' | 'zone', action: 'create' | 'reuse') => ({
170-
action,
171-
id: action === 'reuse' ? `${kind}-id` : null,
172-
key: kind,
173-
kind,
174-
name: 'example.com',
175-
reason: 'test',
176-
});
168+
const action = (kind: 'dns-record' | 'zone', action: 'adopt' | 'create' | 'reuse') => ({
169+
action,
170+
id: action === 'create' ? null : `${kind}-id`,
171+
key: kind,
172+
kind,
173+
name: 'example.com',
174+
reason: 'test',
175+
});
177176

177+
it('requires an API token before browser OAuth applies zone or DNS writes', () => {
178178
expect(() =>
179179
assertPlatformOperatorCredentialCanApplyPlan(
180180
{ source: 'oauth', token: 'browser-token' },
@@ -195,6 +195,26 @@ describe('platform Cloudflare authentication', () => {
195195
).not.toThrow();
196196
});
197197

198+
it('does not require an API token for a DNS record the install only adopts', () => {
199+
const oauth = { source: 'oauth' as const, token: 'browser-token' };
200+
201+
const adopt = { actions: [action('zone', 'reuse'), action('dns-record', 'adopt')] };
202+
expect(platformOperatorNeedsManagementToken(oauth, adopt)).toBe(false);
203+
expect(() => assertPlatformOperatorCredentialCanApplyPlan(oauth, adopt)).not.toThrow();
204+
const create = { actions: [action('zone', 'reuse'), action('dns-record', 'create')] };
205+
expect(platformOperatorNeedsManagementToken(oauth, create)).toBe(true);
206+
expect(() => assertPlatformOperatorCredentialCanApplyPlan(oauth, create)).toThrow(
207+
'Zone → DNS → Edit',
208+
);
209+
const reuse = { actions: [action('zone', 'reuse'), action('dns-record', 'reuse')] };
210+
expect(platformOperatorNeedsManagementToken(oauth, reuse)).toBe(false);
211+
const zone = { actions: [action('zone', 'create'), action('dns-record', 'adopt')] };
212+
expect(platformOperatorNeedsManagementToken(oauth, zone)).toBe(true);
213+
expect(() => assertPlatformOperatorCredentialCanApplyPlan(oauth, zone)).toThrow(
214+
'Account → Zone → Edit',
215+
);
216+
});
217+
198218
it.each([false, true])(
199219
'uses saved management authority before browser login (interactive: %s)',
200220
async (interactive) => {

‎packages/void/test/unit/platform-setup-flow.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -338,10 +338,10 @@ function requireManagementToken() {
338338
}),
339339
actions: [
340340
{
341-
action: 'adopt',
341+
action: 'create',
342342
kind: 'dns-record',
343343
key: 'wildcard-dns',
344-
id: 'dns-id',
344+
id: null,
345345
name: '*.example.com',
346346
reason: 'test',
347347
},

‎packages/void/test/unit/platform-setup.test.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,25 @@ describe('platform credential setup links', () => {
450450
expect(formatPlatformSetupPlan(plan())).not.toContain('management API token');
451451
expect(formatPlatformSetupPlan(plan())).not.toContain('Access application');
452452
});
453+
it('does not list DNS changes when the plan only adopts or reuses DNS records', () => {
454+
const original = plan('example.com');
455+
const record = (action: 'adopt' | 'reuse' | 'create') =>
456+
({
457+
kind: 'dns-record',
458+
action,
459+
name: '*.example.com',
460+
}) as unknown as PlatformInstallPlan['actions'][number];
461+
const preview = (action: 'adopt' | 'reuse' | 'create') =>
462+
formatPlatformSetupPlan(
463+
{ ...original, actions: [record(action)] },
464+
{ managementToken: true, managementAccess: true },
465+
);
466+
expect(preview('adopt')).toContain('Cloudflare management API token for Access setup');
467+
expect(preview('reuse')).toContain('Cloudflare management API token for Access setup');
468+
expect(preview('create')).toContain(
469+
'Cloudflare management API token for DNS changes and Access setup',
470+
);
471+
});
453472
it('includes zone permissions only for an application domain', () => {
454473
const page = platformSetupPages(plan('example.com')).runtime;
455474
expect(permissionGroupKeys(page.url)).toContainEqual({ key: 'cache', type: 'purge' });

0 commit comments

Comments
 (0)