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);