Skip to content

Commit 75448b3

Browse files
harden pagination and password file inputs
1 parent 15e95de commit 75448b3

4 files changed

Lines changed: 215 additions & 5 deletions

File tree

src/commands/project.test.ts

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
type CliProject,
88
type CliUpdateProjectResponse,
99
createProjectCommand,
10+
MAX_PASSWORD_FILE_BYTES,
1011
runCreate,
1112
runGet,
1213
runList,
@@ -484,6 +485,66 @@ describe('runCreate', () => {
484485
expect(result.type).toBe('backend');
485486
});
486487

488+
it('P6 — reads --password-file from a bounded regular file', async () => {
489+
const { credentialsPath } = makeCreds();
490+
const dir = mkdtempSync(join(tmpdir(), 'project-password-'));
491+
const passwordPath = join(dir, 'password.txt');
492+
writeFileSync(passwordPath, ' secret-from-file \n', 'utf8');
493+
const sentBodies: unknown[] = [];
494+
const fetchImpl = (async (_input: Parameters<typeof fetch>[0], init: RequestInit = {}) => {
495+
if (init.body) sentBodies.push(JSON.parse(init.body as string) as unknown);
496+
return new Response(JSON.stringify({ ...PROJECT_FIXTURE, id: 'proj_pw' }), {
497+
status: 200,
498+
headers: { 'content-type': 'application/json' },
499+
});
500+
}) as typeof fetch;
501+
502+
await runCreate(
503+
{
504+
profile: 'default',
505+
output: 'json',
506+
debug: false,
507+
type: 'backend',
508+
name: 'Password File',
509+
passwordFile: passwordPath,
510+
},
511+
{ credentialsPath, fetchImpl, stdout: () => {}, stderr: () => {} },
512+
);
513+
514+
expect(sentBodies[0]).toMatchObject({ password: 'secret-from-file' });
515+
});
516+
517+
it('P6 — rejects --password-file directories before network', async () => {
518+
const { credentialsPath } = makeCreds();
519+
const dir = mkdtempSync(join(tmpdir(), 'project-password-dir-'));
520+
const fetchImpl = vi.fn(async () => {
521+
throw new Error('should not hit network');
522+
});
523+
524+
await expect(
525+
runCreate(
526+
{
527+
profile: 'default',
528+
output: 'json',
529+
debug: false,
530+
type: 'backend',
531+
name: 'Bad Password File',
532+
passwordFile: dir,
533+
},
534+
{
535+
credentialsPath,
536+
fetchImpl: fetchImpl as unknown as typeof fetch,
537+
stdout: () => {},
538+
stderr: () => {},
539+
},
540+
),
541+
).rejects.toMatchObject({
542+
code: 'VALIDATION_ERROR',
543+
details: expect.objectContaining({ field: 'password-file' }),
544+
});
545+
expect(fetchImpl).not.toHaveBeenCalled();
546+
});
547+
487548
it('P6 — dry-run returns canned shape without hitting the network', async () => {
488549
const { credentialsPath } = makeCreds();
489550
const fetchImpl = vi.fn(async () => {
@@ -641,6 +702,41 @@ describe('runUpdate', () => {
641702
expect(fetchImpl).not.toHaveBeenCalled();
642703
});
643704

705+
it('P7 — rejects oversized --password-file before network', async () => {
706+
const { credentialsPath } = makeCreds();
707+
const dir = mkdtempSync(join(tmpdir(), 'project-password-big-'));
708+
const passwordPath = join(dir, 'password.txt');
709+
writeFileSync(passwordPath, 'x'.repeat(MAX_PASSWORD_FILE_BYTES + 1), 'utf8');
710+
const fetchImpl = vi.fn(async () => {
711+
throw new Error('should not hit network');
712+
});
713+
714+
await expect(
715+
runUpdate(
716+
{
717+
profile: 'default',
718+
output: 'json',
719+
debug: false,
720+
projectId: 'proj_abc',
721+
passwordFile: passwordPath,
722+
},
723+
{
724+
credentialsPath,
725+
fetchImpl: fetchImpl as unknown as typeof fetch,
726+
stdout: () => {},
727+
stderr: () => {},
728+
},
729+
),
730+
).rejects.toMatchObject({
731+
code: 'PAYLOAD_TOO_LARGE',
732+
details: expect.objectContaining({
733+
field: 'password-file',
734+
maxBytes: MAX_PASSWORD_FILE_BYTES,
735+
}),
736+
});
737+
expect(fetchImpl).not.toHaveBeenCalled();
738+
});
739+
644740
it('P7 — dry-run returns canned shape without network call', async () => {
645741
const { credentialsPath } = makeCreds();
646742
const fetchImpl = vi.fn(async () => {

src/commands/project.ts

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { randomUUID } from 'node:crypto';
2-
import { readFileSync } from 'node:fs';
2+
import { readFileSync, statSync } from 'node:fs';
3+
import { resolve } from 'node:path';
34
import { Command } from 'commander';
45
import {
56
makeHttpClient,
@@ -19,6 +20,8 @@ import {
1920
type PaginationFlags,
2021
} from '../lib/pagination.js';
2122

23+
export const MAX_PASSWORD_FILE_BYTES = 64 * 1024;
24+
2225
export interface CliProject {
2326
id: string;
2427
name: string;
@@ -185,7 +188,7 @@ export async function runCreate(
185188
// Resolve password: flag > file > none
186189
let password = opts.password;
187190
if (password === undefined && opts.passwordFile !== undefined) {
188-
password = readFileSync(opts.passwordFile, 'utf8').trim();
191+
password = readPasswordFileGuarded(opts.passwordFile);
189192
}
190193

191194
const idempotencyKey = opts.idempotencyKey ?? `cli-proj-create-${randomUUID()}`;
@@ -257,7 +260,7 @@ export async function runUpdate(
257260
// Resolve password
258261
let password = opts.password;
259262
if (password === undefined && opts.passwordFile !== undefined) {
260-
password = readFileSync(opts.passwordFile, 'utf8').trim();
263+
password = readPasswordFileGuarded(opts.passwordFile);
261264
}
262265

263266
// P2-7: guard --url against localhost/RFC1918/non-http(s).
@@ -595,6 +598,51 @@ function renderUpdateText(r: CliUpdateProjectResponse): string {
595598
].join('\n');
596599
}
597600

601+
function readPasswordFileGuarded(path: string): string {
602+
const absolute = resolve(path);
603+
let stat;
604+
try {
605+
stat = statSync(absolute);
606+
} catch (err) {
607+
const message = err instanceof Error ? err.message : String(err);
608+
throw passwordFileError('must point to a readable file', { path: absolute, error: message });
609+
}
610+
611+
if (!stat.isFile()) {
612+
throw passwordFileError('must point to a regular file', { path: absolute });
613+
}
614+
615+
if (stat.size > MAX_PASSWORD_FILE_BYTES) {
616+
throw ApiError.fromEnvelope({
617+
error: {
618+
code: 'PAYLOAD_TOO_LARGE',
619+
message: 'Password file is too large.',
620+
nextAction: `Flag \`--password-file\` is invalid: file must be at most ${MAX_PASSWORD_FILE_BYTES} bytes.`,
621+
requestId: 'local',
622+
details: {
623+
field: 'password-file',
624+
sizeBytes: stat.size,
625+
maxBytes: MAX_PASSWORD_FILE_BYTES,
626+
},
627+
},
628+
});
629+
}
630+
631+
return readFileSync(absolute, 'utf8').trim();
632+
}
633+
634+
function passwordFileError(reason: string, details: Record<string, unknown>): ApiError {
635+
return ApiError.fromEnvelope({
636+
error: {
637+
code: 'VALIDATION_ERROR',
638+
message: 'Invalid request.',
639+
nextAction: `Flag \`--password-file\` is invalid: ${reason}.`,
640+
requestId: 'local',
641+
details: { field: 'password-file', reason, ...details },
642+
},
643+
});
644+
}
645+
598646
function localValidationError(message: string): ApiError {
599647
return ApiError.fromEnvelope({
600648
error: {

src/lib/pagination.test.ts

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
import { describe, expect, it } from 'vitest';
22
import { ApiError } from './errors.js';
3-
import { paginate, validatePaginationFlags, type FetchPage, type Page } from './pagination.js';
3+
import {
4+
MAX_AUTO_PAGES,
5+
paginate,
6+
validatePaginationFlags,
7+
type FetchPage,
8+
type Page,
9+
} from './pagination.js';
410

511
function makePages<T>(pages: Page<T>[]): {
612
fetchPage: FetchPage<T>;
@@ -88,6 +94,35 @@ describe('paginate', () => {
8894
expect(calls[0]!.cursor).toBe('resume');
8995
});
9096

97+
it('rejects a repeated nextToken instead of following a cursor cycle forever', async () => {
98+
const { fetchPage, calls } = makePages([
99+
{ items: [1], nextToken: 'cursor-a' },
100+
{ items: [2], nextToken: 'cursor-b' },
101+
{ items: [3], nextToken: 'cursor-a' },
102+
]);
103+
104+
await expect(paginate(fetchPage)).rejects.toMatchObject({
105+
code: 'UNAVAILABLE',
106+
details: expect.objectContaining({ reason: 'repeated_next_token' }),
107+
});
108+
expect(calls).toHaveLength(3);
109+
});
110+
111+
it('rejects when auto-pagination exceeds the hard page budget', async () => {
112+
const { fetchPage, calls } = makePages(
113+
Array.from({ length: MAX_AUTO_PAGES + 1 }, (_, i) => ({
114+
items: [i],
115+
nextToken: `cursor-${i}`,
116+
})),
117+
);
118+
119+
await expect(paginate(fetchPage)).rejects.toMatchObject({
120+
code: 'UNAVAILABLE',
121+
details: expect.objectContaining({ reason: 'max_pages_exceeded' }),
122+
});
123+
expect(calls).toHaveLength(MAX_AUTO_PAGES);
124+
});
125+
91126
it('shrinks the per-call pageSize when remaining < pageSize', async () => {
92127
const { fetchPage, calls } = makePages([
93128
{ items: [1], nextToken: 'cursor-x' },

src/lib/pagination.ts

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { HttpClient } from './http.js';
2-
import { localValidationError } from './errors.js';
2+
import { ApiError, localValidationError } from './errors.js';
33

44
/**
55
* Page shape returned by every list endpoint per
@@ -22,6 +22,7 @@ export interface PaginationFlags {
2222

2323
const HARD_PAGE_SIZE_CAP = 100;
2424
const DEFAULT_PAGE_SIZE = 25;
25+
export const MAX_AUTO_PAGES = 1000;
2526

2627
/**
2728
* Validates and normalizes pagination flags. Per the CLI OpenAPI spec
@@ -91,14 +92,24 @@ export async function paginate<T>(
9192
const items: T[] = [];
9293
let cursor: string | undefined = flags.startingToken;
9394
let lastNextToken: string | null = null;
95+
let pagesFetched = 0;
96+
const seenNextTokens = new Set<string>();
97+
if (cursor !== undefined) seenNextTokens.add(cursor);
9498

9599
while (true) {
96100
const remaining = maxItems !== undefined ? maxItems - items.length : Infinity;
97101
if (remaining <= 0) break;
102+
if (pagesFetched >= MAX_AUTO_PAGES) {
103+
throw paginationSafetyError('max_pages_exceeded', {
104+
maxPages: MAX_AUTO_PAGES,
105+
cursor,
106+
});
107+
}
98108

99109
const callPageSize = Number.isFinite(remaining) ? Math.min(pageSize, remaining) : pageSize;
100110

101111
const page = await fetchPage({ pageSize: callPageSize, cursor });
112+
pagesFetched += 1;
102113
lastNextToken = page.nextToken;
103114

104115
for (const item of page.items) {
@@ -107,6 +118,13 @@ export async function paginate<T>(
107118
}
108119

109120
if (page.nextToken === null) break;
121+
if (seenNextTokens.has(page.nextToken)) {
122+
throw paginationSafetyError('repeated_next_token', {
123+
nextToken: page.nextToken,
124+
pagesFetched,
125+
});
126+
}
127+
seenNextTokens.add(page.nextToken);
110128
cursor = page.nextToken;
111129
}
112130

@@ -129,3 +147,16 @@ export async function fetchSinglePage<T>(
129147
query: { ...extraQuery, pageSize, cursor },
130148
});
131149
}
150+
151+
function paginationSafetyError(reason: string, details: Record<string, unknown>): ApiError {
152+
return ApiError.fromEnvelope({
153+
error: {
154+
code: 'UNAVAILABLE',
155+
message: 'Pagination did not make safe progress.',
156+
nextAction:
157+
'Retry with --page-size and --starting-token, or use --max-items to bound auto-pagination. Report this if the server keeps returning cursors indefinitely.',
158+
requestId: 'local',
159+
details: { reason, ...details },
160+
},
161+
});
162+
}

0 commit comments

Comments
 (0)