From 36eab83c1bdd303bc8459d24f64942b456c9257c Mon Sep 17 00:00:00 2001 From: Nick Bradley Date: Mon, 3 Aug 2026 17:00:28 +0100 Subject: [PATCH] Let the server derive the delivery allow-list by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The API appends the requester's resolved address to delivery_ips on every create, so the CLI no longer sends a default list — omitting the field entirely and keeping explicit --ip values verbatim. delivery_ips is now optional in the library API. The public-IP lookup becomes diagnostics-only: after create it checks whether this machine's externally observed IP is in the returned allow-list and, if not, warns with the exact fix — the API path and the delivery path can exit from different addresses behind VPNs and NAT pools, which otherwise surfaces as an unexplained delivery 401. The lookup rejects private and reserved addresses, never shapes the request, and never blocks a create. Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 8 ++++---- README.md | 13 +++++++------ src/commands/create.ts | 37 ++++++++++++------------------------- src/lib/args.ts | 6 +++--- src/lib/ip-check.ts | 17 ++++++++++++++++- src/lib/provision.ts | 19 ++++++++++++------- test/create.test.mjs | 22 +++++----------------- test/ip-check.test.mjs | 11 ++++++++++- 8 files changed, 69 insertions(+), 64 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ea51f1e..556de9f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -27,10 +27,10 @@ opens later to keep it. Bin: `cloudinary-cloud`; `create` is the only command an failures surface as a `failed` result (never an exception) so a provisioned cloud's credentials always reach the output; `--no-env` skips the write. `isEnvExposedToGit` backs the gitignore warning. -- `src/lib/ip-check.ts` — best-effort public-IP echo (3s timeout, never throws). Default - `create` sends `[observed-ip, "requester_ip"]` because the API path and delivery path - can exit from different addresses (VPN split tunneling; proxies in front of the API); - explicit `--ip` skips the lookup, and a post-create mismatch warning covers the rest. +- `src/lib/ip-check.ts` — best-effort public-IP echo (3s timeout, never throws, + private/reserved addresses rejected). Diagnostics only: it never shapes the request. + After create it powers the mismatch warning when this machine's IP is outside the + returned allow-list (VPN split egress, NAT pools → silent delivery 401 otherwise). - `src/commands/create.ts` — `runCreate()` (programmatic core) + `createCommand()` (output + exit codes). Refuses to provision when `./.env` already has `CLOUDINARY_URL` — the check runs *before* the API call to avoid burning rate-limited clouds; `--force` overrides. diff --git a/README.md b/README.md index 5fab04a..f3ae9aa 100644 --- a/README.md +++ b/README.md @@ -41,11 +41,11 @@ Running with no arguments defaults to `create`. a provisioned cloud is never swallowed by a filesystem error. - If `.env` isn't covered by `.gitignore` in a git repo, `create` warns after writing. - Cloud media delivery is locked at the CDN edge to `delivery_ips`. By default the CLI - auto-detects this machine's public IP and sends it together with the `requester_ip` - sentinel — covering both the caller-is-viewer case and setups where the API path and - the delivery path exit from different addresses (VPNs, proxies). Explicit `--ip` values - are sent verbatim and skip the detection; if the detected IP ends up outside the final - allow-list, `create` warns with the exact fix. + sends none and the server locks delivery to the address the request came from — the + right default when the caller is also the viewer. Explicit `--ip` values are sent + verbatim. After creation the CLI checks (best-effort) whether this machine's public IP + is in the returned allow-list, and warns with the exact fix if not — the API path and + the delivery path can exit from different addresses behind VPNs and NAT pools. - The claim URL and expiry are persisted to `.env` too (`CLOUDINARY_CLOUD_CLAIM_URL`, `CLOUDINARY_CLOUD_EXPIRES_AT`), so the claim path survives lost terminal output. - Cloud lifetime is server-controlled (no TTL parameter in the API). @@ -80,7 +80,8 @@ CLOUDINARY_API_HOST=https://staging.example node dist/index.js create Speaks `POST /v1_1/provisioning/clouds` (public, unauthenticated, rate limited per IP): -- Request: `delivery_ips` (required; array of 1–3 public IPs and/or `"requester_ip"`), +- Request: `delivery_ips` (optional; array of 1–3 public IPs and/or `"requester_ip"` — + the server always appends the requester's resolved address), `email` (optional, unverified pre-fill). No TTL parameter — lifetime is server-set. - Response: `id`, `email`, `expires_at`, `delivery_ips`, `claim_url`, `guidance`, and credentials in `product_environments[0].api_access_keys[]` (`key`/`secret`). diff --git a/src/commands/create.ts b/src/commands/create.ts index 1f422ce..204508f 100644 --- a/src/commands/create.ts +++ b/src/commands/create.ts @@ -4,7 +4,6 @@ import { provisionCloud, getCloudinaryUrl, ProvisionError, - REQUESTER_IP_SENTINEL, type ProvisionRequest, type CloudAccount, } from '../lib/provision.js'; @@ -34,8 +33,6 @@ export interface CreateResult { cloudinaryUrl: string; envPath: string; envResult: EnvWriteResult; - /** This machine's public IP as seen from outside, when it could be determined. */ - observedIp: string | null; } /** @@ -43,21 +40,11 @@ export interface CreateResult { * Programmatic core shared by the `create` command and @cloudinary/dev-cli. */ export async function runCreate(options: CreateOptions): Promise { - // Default allow-list: this machine's externally observed IP plus the server- - // resolved requester IP. The two can differ (VPN split paths, proxies in - // front of the API), and delivery only works from addresses in the list — - // including the observed IP keeps the create→view loop working either way. - // Explicit --ip values are used verbatim and skip the lookup. - let observedIp: string | null = null; - let deliveryIps: string[]; - if (options.ip && options.ip.length > 0) { - deliveryIps = options.ip; - } else { - observedIp = await getObservedPublicIp(options.ipEchoFetch ?? fetch); - deliveryIps = observedIp ? [observedIp, REQUESTER_IP_SENTINEL] : [REQUESTER_IP_SENTINEL]; - } - - const request: ProvisionRequest = { deliveryIps }; + // Default: omit delivery_ips — the server derives the allow-list from the + // requester's resolved address. Explicit --ip values are sent verbatim. + // (The public-IP lookup is diagnostics-only; see the post-create warning.) + const request: ProvisionRequest = {}; + if (options.ip && options.ip.length > 0) request.deliveryIps = options.ip; if (options.email !== undefined) request.email = options.email; @@ -97,7 +84,7 @@ export async function runCreate(options: CreateOptions): Promise { } } - return { account, cloudinaryUrl, envPath, envResult, observedIp }; + return { account, cloudinaryUrl, envPath, envResult }; } /** CLI action wrapper: output + exit codes around runCreate. */ @@ -120,12 +107,12 @@ export async function createCommand(options: CreateOptions): Promise { console.error(pc.yellow('Warning: .env is not covered by .gitignore here — add it before committing.')); } - // Delivery is IP-locked; if this machine isn't in the final allow-list (an - // explicit --ip list that excludes it, or the server dropped the observed - // IP), say so now rather than letting the first media request 401 - // mysteriously. Explicit --ip runs need the lookup here since runCreate - // skipped it. - const observed = result.observedIp ?? (options.ip?.length ? await getObservedPublicIp(options.ipEchoFetch ?? fetch) : null); + // Diagnostics only: delivery is IP-locked to the allow-list the server + // returned. If this machine's externally observed IP isn't in it (VPN + // split egress, NAT pools), the first media request would 401 with no + // explanation — warn now with the exact fix instead. Never shapes the + // request; failure to determine the IP just means no warning. + const observed = await getObservedPublicIp(options.ipEchoFetch ?? fetch); const mismatch = deliveryIpMismatchWarning(result.account.delivery_ips ?? [], observed); if (mismatch) console.error(pc.yellow(`Warning: ${mismatch}`)); } catch (err) { diff --git a/src/lib/args.ts b/src/lib/args.ts index 1004e38..7e21a47 100644 --- a/src/lib/args.ts +++ b/src/lib/args.ts @@ -99,9 +99,9 @@ Commands: Options: --ip
public IP allowed to view delivered media (repeatable, - max 3, "requester_ip" for the address the API sees). - Default: this machine's public IP (auto-detected) plus - requester_ip; explicit values skip the detection + max 3). Default: the server locks delivery to the + address the request came from; a warning is printed if + this machine's IP ends up outside the allow-list --email
pre-fill the claim page with this email (never verified at creation) --goal what you are building — sent as attribution alongside the diff --git a/src/lib/ip-check.ts b/src/lib/ip-check.ts index a552b87..a4c96ad 100644 --- a/src/lib/ip-check.ts +++ b/src/lib/ip-check.ts @@ -10,13 +10,28 @@ const IP_ECHO_URL = 'https://checkip.amazonaws.com'; const CHECK_TIMEOUT_MS = 3_000; +/** + * True for globally routable addresses only. Corporate proxies and split-DNS + * setups can make the echo service return a private address; sending one as a + * delivery IP is a guaranteed 400, so those count as "couldn't determine". + */ +export function isPublicIp(ip: string): boolean { + if (/^(10\.|127\.|0\.|192\.168\.|169\.254\.)/.test(ip)) return false; + if (/^172\.(1[6-9]|2\d|3[01])\./.test(ip)) return false; + if (/^100\.(6[4-9]|[7-9]\d|1[01]\d|12[0-7])\./.test(ip)) return false; // CGNAT + const lower = ip.toLowerCase(); + if (lower === '::1' || lower.startsWith('fe80:') || lower.startsWith('fc') || lower.startsWith('fd')) return false; + return true; +} + /** This machine's public IP, or null if it can't be determined quickly. Never throws. */ export async function getObservedPublicIp(fetchImpl: typeof fetch = fetch): Promise { try { const res = await fetchImpl(IP_ECHO_URL, { signal: AbortSignal.timeout(CHECK_TIMEOUT_MS) }); if (!res.ok) return null; const ip = (await res.text()).trim(); - return /^[0-9a-fA-F.:]+$/.test(ip) ? ip : null; + if (!/^[0-9a-fA-F.:]+$/.test(ip)) return null; + return isPublicIp(ip) ? ip : null; } catch { return null; } diff --git a/src/lib/provision.ts b/src/lib/provision.ts index b16c982..4bad568 100644 --- a/src/lib/provision.ts +++ b/src/lib/provision.ts @@ -16,8 +16,12 @@ export const REQUESTER_IP_SENTINEL = 'requester_ip'; export const MAX_DELIVERY_IPS = 3; export interface ProvisionRequest { - /** 1-3 entries: public IPs and/or the "requester_ip" sentinel. */ - deliveryIps: string[]; + /** + * 1-3 entries: public IPs and/or the "requester_ip" sentinel. Omit to let + * the server derive the allow-list (it always appends the requester's + * resolved address). + */ + deliveryIps?: string[]; /** Optional pre-fill hint for the claim page; never verified at creation. */ email?: string; /** @@ -127,15 +131,16 @@ export async function provisionCloud( request: ProvisionRequest, options: ProvisionOptions = {}, ): Promise { - const invalid = validateDeliveryIps(request.deliveryIps); - if (invalid) throw new ProvisionError(invalid, 400, 'delivery_ips_invalid', 'user_error'); + if (request.deliveryIps !== undefined) { + const invalid = validateDeliveryIps(request.deliveryIps); + if (invalid) throw new ProvisionError(invalid, 400, 'delivery_ips_invalid', 'user_error'); + } const host = resolveApiHost(options.apiHost); const doFetch = options.fetchImpl ?? fetch; - const body: Record = { - delivery_ips: request.deliveryIps, - }; + const body: Record = {}; + if (request.deliveryIps !== undefined) body.delivery_ips = request.deliveryIps; if (request.email !== undefined) body.email = request.email; for (const [key, value] of Object.entries(request.agentMetadata ?? {})) { if (value) body[key] = value; diff --git a/test/create.test.mjs b/test/create.test.mjs index f930bfd..0c95627 100644 --- a/test/create.test.mjs +++ b/test/create.test.mjs @@ -150,34 +150,22 @@ function withBodyCapture(run) { }); } -const echoOf = ip => async () => new Response(`${ip}\n`, { status: 200 }); - -test('default sends the observed public IP plus the requester_ip sentinel', () => +test('default omits delivery_ips entirely — the server derives the allow-list', () => inTempCwd(() => withBodyCapture(async (host, bodies) => { - const result = await runCreate({ apiHost: host, ipEchoFetch: echoOf('94.7.253.136') }); - assert.deepEqual(bodies[0].delivery_ips, ['94.7.253.136', 'requester_ip']); - assert.equal(result.observedIp, '94.7.253.136'); - }), - )); - -test('default falls back to requester_ip alone when the IP lookup fails', () => - inTempCwd(() => - withBodyCapture(async (host, bodies) => { - const result = await runCreate({ apiHost: host, ipEchoFetch: noEcho }); - assert.deepEqual(bodies[0].delivery_ips, ['requester_ip']); - assert.equal(result.observedIp, null); + await runCreate({ apiHost: host, ipEchoFetch: noEcho }); + assert.equal('delivery_ips' in bodies[0], false); }), )); -test('explicit --ip values are sent verbatim and skip the lookup', () => +test('explicit --ip values are sent verbatim, no lookup involved', () => inTempCwd(() => withBodyCapture(async (host, bodies) => { let lookups = 0; const countingEcho = async () => { lookups++; return new Response('9.9.9.9', { status: 200 }); }; await runCreate({ apiHost: host, ip: ['203.0.113.7'], ipEchoFetch: countingEcho }); assert.deepEqual(bodies[0].delivery_ips, ['203.0.113.7']); - assert.equal(lookups, 0); + assert.equal(lookups, 0, 'runCreate must not perform the lookup'); }), )); diff --git a/test/ip-check.test.mjs b/test/ip-check.test.mjs index 22f1835..43772d1 100644 --- a/test/ip-check.test.mjs +++ b/test/ip-check.test.mjs @@ -1,6 +1,6 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; -import { getObservedPublicIp, deliveryIpMismatchWarning } from '../dist/lib/ip-check.js'; +import { getObservedPublicIp, deliveryIpMismatchWarning, isPublicIp } from '../dist/lib/ip-check.js'; const okFetch = body => async () => new Response(body, { status: 200 }); @@ -15,6 +15,15 @@ test('getObservedPublicIp never throws: bad status, garbage body, network error' assert.equal(await getObservedPublicIp(async () => { throw new Error('offline'); }), null); }); +test('private and reserved addresses count as undetermined', async () => { + for (const ip of ['10.16.236.105', '192.168.1.10', '172.20.0.1', '127.0.0.1', '169.254.1.1', '100.64.0.1', 'fe80::1', 'fd00::1']) { + assert.equal(await getObservedPublicIp(okFetch(ip)), null, ip); + assert.equal(isPublicIp(ip), false, ip); + } + assert.equal(isPublicIp('203.0.113.9'), true); + assert.equal(isPublicIp('2001:db8::1'), true); +}); + test('mismatch warning fires only when observed IP is outside the allow-list', () => { assert.equal(deliveryIpMismatchWarning(['203.0.113.9'], '203.0.113.9'), null); assert.equal(deliveryIpMismatchWarning(['203.0.113.9'], null), null);