Skip to content

Commit 8da07fb

Browse files
authored
fix(project): avoid password file reads in dry-run update (#54)
1 parent 76fff29 commit 8da07fb

3 files changed

Lines changed: 74 additions & 17 deletions

File tree

src/commands/project.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -729,6 +729,33 @@ describe('runUpdate', () => {
729729
expect(err).toContain(DRY_RUN_BANNER);
730730
});
731731

732+
it('P7 — dry-run with --password-file does not read the filesystem', async () => {
733+
const { credentialsPath } = makeCreds();
734+
const fetchImpl = vi.fn(async () => {
735+
throw new Error('should not hit network');
736+
});
737+
const result = await runUpdate(
738+
{
739+
profile: 'default',
740+
output: 'json',
741+
debug: false,
742+
dryRun: true,
743+
projectId: 'proj_dry',
744+
passwordFile: '/tmp/definitely-not-here-testsprite',
745+
},
746+
{
747+
credentialsPath,
748+
fetchImpl: fetchImpl as unknown as typeof fetch,
749+
stdout: () => {},
750+
stderr: () => {},
751+
},
752+
);
753+
754+
expect(fetchImpl).not.toHaveBeenCalled();
755+
expect(result.id).toBe('proj_dry');
756+
expect(result.updatedFields).toContain('password');
757+
});
758+
732759
it('P7 — renders text mode with updatedFields and updatedAt', async () => {
733760
const { credentialsPath } = makeCreds();
734761
const updateResponse: CliUpdateProjectResponse = {

src/commands/project.ts

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -270,27 +270,24 @@ export async function runUpdate(
270270
throw localValidationError('--description must be at most 2000 characters');
271271
}
272272

273-
// Resolve password
274-
let password = opts.password;
275-
if (password === undefined && opts.passwordFile !== undefined) {
276-
password = readFileSync(opts.passwordFile, 'utf8').trim();
277-
}
278-
279273
// P2-7: guard --url against localhost/RFC1918/non-http(s).
280274
if (opts.targetUrl !== undefined) {
281275
assertNotLocal(opts.targetUrl);
282276
}
283277

284-
const mutableFields: Record<string, string | undefined> = {
285-
name: opts.name,
286-
targetUrl: opts.targetUrl,
287-
username: opts.username,
288-
password,
289-
description: opts.description,
290-
instruction: opts.instruction,
278+
const passwordSupplied = opts.password !== undefined || opts.passwordFile !== undefined;
279+
const mutableFields: Record<string, boolean> = {
280+
name: opts.name !== undefined,
281+
targetUrl: opts.targetUrl !== undefined,
282+
username: opts.username !== undefined,
283+
password: passwordSupplied,
284+
description: opts.description !== undefined,
285+
instruction: opts.instruction !== undefined,
291286
};
292-
const presentFields = Object.entries(mutableFields).filter(([, v]) => v !== undefined);
293-
if (presentFields.length === 0) {
287+
const presentFieldNames = Object.entries(mutableFields)
288+
.filter(([, present]) => present)
289+
.map(([field]) => field);
290+
if (presentFieldNames.length === 0) {
294291
throw localValidationError(
295292
'At least one mutable flag is required: --name, --url, --username, --password/--password-file, --description, or --instruction.',
296293
);
@@ -308,19 +305,36 @@ export async function runUpdate(
308305
}
309306
const sample: CliUpdateProjectResponse = {
310307
id: opts.projectId,
311-
updatedFields: presentFields.map(([k]) => k),
308+
updatedFields: presentFieldNames,
312309
updatedAt: '2026-05-16T00:00:00.000Z',
313310
};
314311
out.print(sample, data => renderUpdateText(data as CliUpdateProjectResponse));
315312
return sample;
316313
}
317314

315+
// Resolve password only on the real path. Dry-run must not touch the
316+
// filesystem, even when --password-file is present.
317+
let password = opts.password;
318+
if (password === undefined && opts.passwordFile !== undefined) {
319+
password = readFileSync(opts.passwordFile, 'utf8').trim();
320+
}
321+
318322
const idempotencyKey = opts.idempotencyKey ?? `cli-proj-update-${randomUUID()}`;
319323
if (opts.idempotencyKey === undefined && (opts.output === 'json' || opts.verbose || opts.debug)) {
320324
stderr(`idempotency-key: ${idempotencyKey}`);
321325
}
322326

323-
const body = Object.fromEntries(presentFields) as Record<string, string>;
327+
const bodyFields: Record<string, string | undefined> = {
328+
name: opts.name,
329+
targetUrl: opts.targetUrl,
330+
username: opts.username,
331+
password,
332+
description: opts.description,
333+
instruction: opts.instruction,
334+
};
335+
const body = Object.fromEntries(
336+
Object.entries(bodyFields).filter(([, v]) => v !== undefined),
337+
) as Record<string, string>;
324338
const client = makeClient(opts, deps);
325339
const updated = await client.patch<CliUpdateProjectResponse>(
326340
`/projects/${encodeURIComponent(opts.projectId)}`,

test/cli.subprocess.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -923,6 +923,22 @@ describe('--dry-run subprocess smoke', () => {
923923
expect(parsed.id).toBeTruthy();
924924
}, 30_000);
925925

926+
it('project update --dry-run does not read a missing --password-file', async () => {
927+
const result = await runCli([
928+
'project',
929+
'update',
930+
'proj_anything',
931+
'--password-file',
932+
'/tmp/definitely-not-here-testsprite',
933+
'--dry-run',
934+
'--output',
935+
'json',
936+
]);
937+
expect(result.exitCode).toBe(0);
938+
const parsed = JSON.parse(result.stdout) as { updatedFields: string[] };
939+
expect(parsed.updatedFields).toContain('password');
940+
}, 30_000);
941+
926942
it('test list --dry-run returns canned TestList', async () => {
927943
const result = await runCli([
928944
'test',

0 commit comments

Comments
 (0)