diff --git a/src/commands/project.test.ts b/src/commands/project.test.ts index 0037b8c..df676ed 100644 --- a/src/commands/project.test.ts +++ b/src/commands/project.test.ts @@ -7,6 +7,7 @@ import { type CliProject, type CliUpdateProjectResponse, createProjectCommand, + MAX_PASSWORD_FILE_BYTES, runCreate, runGet, runList, @@ -484,6 +485,66 @@ describe('runCreate', () => { expect(result.type).toBe('backend'); }); + it('P6 — reads --password-file from a bounded regular file', async () => { + const { credentialsPath } = makeCreds(); + const dir = mkdtempSync(join(tmpdir(), 'project-password-')); + const passwordPath = join(dir, 'password.txt'); + writeFileSync(passwordPath, ' secret-from-file \n', 'utf8'); + const sentBodies: unknown[] = []; + const fetchImpl = (async (_input: Parameters[0], init: RequestInit = {}) => { + if (init.body) sentBodies.push(JSON.parse(init.body as string) as unknown); + return new Response(JSON.stringify({ ...PROJECT_FIXTURE, id: 'proj_pw' }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + }) as typeof fetch; + + await runCreate( + { + profile: 'default', + output: 'json', + debug: false, + type: 'backend', + name: 'Password File', + passwordFile: passwordPath, + }, + { credentialsPath, fetchImpl, stdout: () => {}, stderr: () => {} }, + ); + + expect(sentBodies[0]).toMatchObject({ password: 'secret-from-file' }); + }); + + it('P6 — rejects --password-file directories before network', async () => { + const { credentialsPath } = makeCreds(); + const dir = mkdtempSync(join(tmpdir(), 'project-password-dir-')); + const fetchImpl = vi.fn(async () => { + throw new Error('should not hit network'); + }); + + await expect( + runCreate( + { + profile: 'default', + output: 'json', + debug: false, + type: 'backend', + name: 'Bad Password File', + passwordFile: dir, + }, + { + credentialsPath, + fetchImpl: fetchImpl as unknown as typeof fetch, + stdout: () => {}, + stderr: () => {}, + }, + ), + ).rejects.toMatchObject({ + code: 'VALIDATION_ERROR', + details: expect.objectContaining({ field: 'password-file' }), + }); + expect(fetchImpl).not.toHaveBeenCalled(); + }); + it('P6 — dry-run returns canned shape without hitting the network', async () => { const { credentialsPath } = makeCreds(); const fetchImpl = vi.fn(async () => { @@ -641,6 +702,41 @@ describe('runUpdate', () => { expect(fetchImpl).not.toHaveBeenCalled(); }); + it('P7 — rejects oversized --password-file before network', async () => { + const { credentialsPath } = makeCreds(); + const dir = mkdtempSync(join(tmpdir(), 'project-password-big-')); + const passwordPath = join(dir, 'password.txt'); + writeFileSync(passwordPath, 'x'.repeat(MAX_PASSWORD_FILE_BYTES + 1), 'utf8'); + const fetchImpl = vi.fn(async () => { + throw new Error('should not hit network'); + }); + + await expect( + runUpdate( + { + profile: 'default', + output: 'json', + debug: false, + projectId: 'proj_abc', + passwordFile: passwordPath, + }, + { + credentialsPath, + fetchImpl: fetchImpl as unknown as typeof fetch, + stdout: () => {}, + stderr: () => {}, + }, + ), + ).rejects.toMatchObject({ + code: 'PAYLOAD_TOO_LARGE', + details: expect.objectContaining({ + field: 'password-file', + maxBytes: MAX_PASSWORD_FILE_BYTES, + }), + }); + expect(fetchImpl).not.toHaveBeenCalled(); + }); + it('P7 — dry-run returns canned shape without network call', async () => { const { credentialsPath } = makeCreds(); const fetchImpl = vi.fn(async () => { diff --git a/src/commands/project.ts b/src/commands/project.ts index 277f3ca..ee5ab40 100644 --- a/src/commands/project.ts +++ b/src/commands/project.ts @@ -1,5 +1,6 @@ import { randomUUID } from 'node:crypto'; -import { readFileSync } from 'node:fs'; +import { readFileSync, statSync } from 'node:fs'; +import { resolve } from 'node:path'; import { Command } from 'commander'; import { makeHttpClient, @@ -19,6 +20,8 @@ import { type PaginationFlags, } from '../lib/pagination.js'; +export const MAX_PASSWORD_FILE_BYTES = 64 * 1024; + export interface CliProject { id: string; name: string; @@ -185,7 +188,7 @@ export async function runCreate( // Resolve password: flag > file > none let password = opts.password; if (password === undefined && opts.passwordFile !== undefined) { - password = readFileSync(opts.passwordFile, 'utf8').trim(); + password = readPasswordFileGuarded(opts.passwordFile); } const idempotencyKey = opts.idempotencyKey ?? `cli-proj-create-${randomUUID()}`; @@ -257,7 +260,7 @@ export async function runUpdate( // Resolve password let password = opts.password; if (password === undefined && opts.passwordFile !== undefined) { - password = readFileSync(opts.passwordFile, 'utf8').trim(); + password = readPasswordFileGuarded(opts.passwordFile); } // P2-7: guard --url against localhost/RFC1918/non-http(s). @@ -595,6 +598,51 @@ function renderUpdateText(r: CliUpdateProjectResponse): string { ].join('\n'); } +function readPasswordFileGuarded(path: string): string { + const absolute = resolve(path); + let stat; + try { + stat = statSync(absolute); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + throw passwordFileError('must point to a readable file', { path: absolute, error: message }); + } + + if (!stat.isFile()) { + throw passwordFileError('must point to a regular file', { path: absolute }); + } + + if (stat.size > MAX_PASSWORD_FILE_BYTES) { + throw ApiError.fromEnvelope({ + error: { + code: 'PAYLOAD_TOO_LARGE', + message: 'Password file is too large.', + nextAction: `Flag \`--password-file\` is invalid: file must be at most ${MAX_PASSWORD_FILE_BYTES} bytes.`, + requestId: 'local', + details: { + field: 'password-file', + sizeBytes: stat.size, + maxBytes: MAX_PASSWORD_FILE_BYTES, + }, + }, + }); + } + + return readFileSync(absolute, 'utf8').trim(); +} + +function passwordFileError(reason: string, details: Record): ApiError { + return ApiError.fromEnvelope({ + error: { + code: 'VALIDATION_ERROR', + message: 'Invalid request.', + nextAction: `Flag \`--password-file\` is invalid: ${reason}.`, + requestId: 'local', + details: { field: 'password-file', reason, ...details }, + }, + }); +} + function localValidationError(message: string): ApiError { return ApiError.fromEnvelope({ error: { diff --git a/src/lib/pagination.test.ts b/src/lib/pagination.test.ts index 33fd735..815a0f7 100644 --- a/src/lib/pagination.test.ts +++ b/src/lib/pagination.test.ts @@ -1,6 +1,12 @@ import { describe, expect, it } from 'vitest'; import { ApiError } from './errors.js'; -import { paginate, validatePaginationFlags, type FetchPage, type Page } from './pagination.js'; +import { + MAX_AUTO_PAGES, + paginate, + validatePaginationFlags, + type FetchPage, + type Page, +} from './pagination.js'; function makePages(pages: Page[]): { fetchPage: FetchPage; @@ -88,6 +94,35 @@ describe('paginate', () => { expect(calls[0]!.cursor).toBe('resume'); }); + it('rejects a repeated nextToken instead of following a cursor cycle forever', async () => { + const { fetchPage, calls } = makePages([ + { items: [1], nextToken: 'cursor-a' }, + { items: [2], nextToken: 'cursor-b' }, + { items: [3], nextToken: 'cursor-a' }, + ]); + + await expect(paginate(fetchPage)).rejects.toMatchObject({ + code: 'UNAVAILABLE', + details: expect.objectContaining({ reason: 'repeated_next_token' }), + }); + expect(calls).toHaveLength(3); + }); + + it('rejects when auto-pagination exceeds the hard page budget', async () => { + const { fetchPage, calls } = makePages( + Array.from({ length: MAX_AUTO_PAGES + 1 }, (_, i) => ({ + items: [i], + nextToken: `cursor-${i}`, + })), + ); + + await expect(paginate(fetchPage)).rejects.toMatchObject({ + code: 'UNAVAILABLE', + details: expect.objectContaining({ reason: 'max_pages_exceeded' }), + }); + expect(calls).toHaveLength(MAX_AUTO_PAGES); + }); + it('shrinks the per-call pageSize when remaining < pageSize', async () => { const { fetchPage, calls } = makePages([ { items: [1], nextToken: 'cursor-x' }, diff --git a/src/lib/pagination.ts b/src/lib/pagination.ts index 59dbc12..2e3c6e4 100644 --- a/src/lib/pagination.ts +++ b/src/lib/pagination.ts @@ -1,5 +1,5 @@ import type { HttpClient } from './http.js'; -import { localValidationError } from './errors.js'; +import { ApiError, localValidationError } from './errors.js'; /** * Page shape returned by every list endpoint per @@ -22,6 +22,7 @@ export interface PaginationFlags { const HARD_PAGE_SIZE_CAP = 100; const DEFAULT_PAGE_SIZE = 25; +export const MAX_AUTO_PAGES = 1000; /** * Validates and normalizes pagination flags. Per the CLI OpenAPI spec @@ -91,14 +92,24 @@ export async function paginate( const items: T[] = []; let cursor: string | undefined = flags.startingToken; let lastNextToken: string | null = null; + let pagesFetched = 0; + const seenNextTokens = new Set(); + if (cursor !== undefined) seenNextTokens.add(cursor); while (true) { const remaining = maxItems !== undefined ? maxItems - items.length : Infinity; if (remaining <= 0) break; + if (pagesFetched >= MAX_AUTO_PAGES) { + throw paginationSafetyError('max_pages_exceeded', { + maxPages: MAX_AUTO_PAGES, + cursor, + }); + } const callPageSize = Number.isFinite(remaining) ? Math.min(pageSize, remaining) : pageSize; const page = await fetchPage({ pageSize: callPageSize, cursor }); + pagesFetched += 1; lastNextToken = page.nextToken; for (const item of page.items) { @@ -107,6 +118,13 @@ export async function paginate( } if (page.nextToken === null) break; + if (seenNextTokens.has(page.nextToken)) { + throw paginationSafetyError('repeated_next_token', { + nextToken: page.nextToken, + pagesFetched, + }); + } + seenNextTokens.add(page.nextToken); cursor = page.nextToken; } @@ -129,3 +147,16 @@ export async function fetchSinglePage( query: { ...extraQuery, pageSize, cursor }, }); } + +function paginationSafetyError(reason: string, details: Record): ApiError { + return ApiError.fromEnvelope({ + error: { + code: 'UNAVAILABLE', + message: 'Pagination did not make safe progress.', + nextAction: + 'Retry with --page-size and --starting-token, or use --max-items to bound auto-pagination. Report this if the server keeps returning cursors indefinitely.', + requestId: 'local', + details: { reason, ...details }, + }, + }); +}