Skip to content

Commit 080a929

Browse files
committed
fix(project): guard credential/auto-auth secret file reads (#282)
The v0.4.0 `project credential` and `project auto-auth` commands read their `--*-file` flags with a raw `readFileSync(path).trim()`. A missing file, a directory, or an oversized file escaped as an unwrapped Node error (exit 1) that also broke the `--output json` envelope (bare `{"error":"ENOENT…"}` instead of the structured `{code,message,nextAction}`) and leaked fs internals. Generalize the guard into `readSecretFileGuarded(path, flagName)` — mirroring the `--code-file` guard in test.ts and PR #248's `readPasswordFileGuarded` — and apply it to all six project.ts file-read sites: - project create/update --password-file - project credential --credential-file - project auto-auth --password-file / --client-secret-file / --refresh-token-file Missing/non-regular files now return VALIDATION_ERROR (exit 5) and oversized files return PAYLOAD_TOO_LARGE (exit 5), each carrying the flag name — the same contract every other file flag already honors. Adds 11 tests covering missing/directory/oversized inputs across all four new flags plus the two password-file sites, and the trimmed happy path. Closes #282
1 parent fe07bc9 commit 080a929

2 files changed

Lines changed: 253 additions & 7 deletions

File tree

src/commands/project.test.ts

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
runGet,
1717
runList,
1818
runUpdate,
19+
MAX_SECRET_FILE_BYTES,
1920
} from './project.js';
2021

2122
const PROJECT_FIXTURE: CliProject = {
@@ -1423,3 +1424,185 @@ describe('dogfood 2026-06-30 — whitespace-only --name is rejected (parity with
14231424
).rejects.toMatchObject({ code: 'VALIDATION_ERROR', exitCode: 5 });
14241425
});
14251426
});
1427+
1428+
describe('#282 — secret --*-file flags are guarded (structured error, exit 5, no raw ENOENT)', () => {
1429+
const noNetwork = () => {
1430+
throw new Error('network should not be hit');
1431+
};
1432+
const deps = (credentialsPath: string) => ({
1433+
credentialsPath,
1434+
fetchImpl: makeFetch(noNetwork),
1435+
stdout: () => {},
1436+
stderr: () => {},
1437+
});
1438+
const missingPath = () => join(mkdtempSync(join(tmpdir(), 'cli-missing-')), 'no-such-secret.txt');
1439+
1440+
it('runCredential --credential-file missing → VALIDATION_ERROR (exit 5), no network', async () => {
1441+
const { credentialsPath } = makeCreds();
1442+
await expect(
1443+
runCredential(
1444+
{
1445+
profile: 'default',
1446+
output: 'json',
1447+
debug: false,
1448+
projectId: 'p1',
1449+
authType: 'API key',
1450+
credentialFile: missingPath(),
1451+
},
1452+
deps(credentialsPath),
1453+
),
1454+
).rejects.toMatchObject({ code: 'VALIDATION_ERROR', exitCode: 5 });
1455+
});
1456+
1457+
it('runCredential --credential-file pointing at a directory → VALIDATION_ERROR (exit 5)', async () => {
1458+
const { credentialsPath } = makeCreds();
1459+
const dir = mkdtempSync(join(tmpdir(), 'cli-cred-dir-'));
1460+
await expect(
1461+
runCredential(
1462+
{
1463+
profile: 'default',
1464+
output: 'json',
1465+
debug: false,
1466+
projectId: 'p1',
1467+
authType: 'API key',
1468+
credentialFile: dir,
1469+
},
1470+
deps(credentialsPath),
1471+
),
1472+
).rejects.toMatchObject({ code: 'VALIDATION_ERROR', exitCode: 5 });
1473+
});
1474+
1475+
it('runCredential --credential-file over the size cap → PAYLOAD_TOO_LARGE (exit 5)', async () => {
1476+
const { credentialsPath } = makeCreds();
1477+
const dir = mkdtempSync(join(tmpdir(), 'cli-cred-big-'));
1478+
const bigFile = join(dir, 'big.txt');
1479+
writeFileSync(bigFile, 'a'.repeat(MAX_SECRET_FILE_BYTES + 1));
1480+
await expect(
1481+
runCredential(
1482+
{
1483+
profile: 'default',
1484+
output: 'json',
1485+
debug: false,
1486+
projectId: 'p1',
1487+
authType: 'API key',
1488+
credentialFile: bigFile,
1489+
},
1490+
deps(credentialsPath),
1491+
),
1492+
).rejects.toMatchObject({ code: 'PAYLOAD_TOO_LARGE', exitCode: 5 });
1493+
});
1494+
1495+
it('runCredential reads a valid --credential-file (trimmed) and sends it', async () => {
1496+
const { credentialsPath } = makeCreds();
1497+
const dir = mkdtempSync(join(tmpdir(), 'cli-cred-ok-'));
1498+
const credFile = join(dir, 'cred.txt');
1499+
writeFileSync(credFile, ' tok-from-file\n');
1500+
let sentBody: { credential?: string } | undefined;
1501+
const fetchImpl = makeFetch((_url, init) => {
1502+
sentBody = init.body ? JSON.parse(init.body as string) : undefined;
1503+
return { status: 200, body: { projectId: 'p1', authType: 'API key', rewroteCount: 1 } };
1504+
});
1505+
await runCredential(
1506+
{
1507+
profile: 'default',
1508+
output: 'json',
1509+
debug: false,
1510+
projectId: 'p1',
1511+
authType: 'API key',
1512+
credentialFile: credFile,
1513+
},
1514+
{ credentialsPath, fetchImpl, stdout: () => {}, stderr: () => {} },
1515+
);
1516+
// blindfold: manual — the on-disk fixture written above is " tok-from-file\n"; the guard trims it
1517+
expect(sentBody?.credential).toBe('tok-from-file');
1518+
});
1519+
1520+
it('runAutoAuth --password-file missing → VALIDATION_ERROR (exit 5), no network', async () => {
1521+
const { credentialsPath } = makeCreds();
1522+
await expect(
1523+
runAutoAuth(
1524+
{
1525+
profile: 'default',
1526+
output: 'json',
1527+
debug: false,
1528+
projectId: 'p1',
1529+
method: 'password',
1530+
inject: 'bearer',
1531+
passwordFile: missingPath(),
1532+
},
1533+
deps(credentialsPath),
1534+
),
1535+
).rejects.toMatchObject({ code: 'VALIDATION_ERROR', exitCode: 5 });
1536+
});
1537+
1538+
it('runAutoAuth --client-secret-file missing → VALIDATION_ERROR (exit 5), no network', async () => {
1539+
const { credentialsPath } = makeCreds();
1540+
await expect(
1541+
runAutoAuth(
1542+
{
1543+
profile: 'default',
1544+
output: 'json',
1545+
debug: false,
1546+
projectId: 'p1',
1547+
method: 'refresh_token',
1548+
inject: 'bearer',
1549+
clientSecretFile: missingPath(),
1550+
},
1551+
deps(credentialsPath),
1552+
),
1553+
).rejects.toMatchObject({ code: 'VALIDATION_ERROR', exitCode: 5 });
1554+
});
1555+
1556+
it('runAutoAuth --refresh-token-file missing → VALIDATION_ERROR (exit 5), no network', async () => {
1557+
const { credentialsPath } = makeCreds();
1558+
await expect(
1559+
runAutoAuth(
1560+
{
1561+
profile: 'default',
1562+
output: 'json',
1563+
debug: false,
1564+
projectId: 'p1',
1565+
method: 'refresh_token',
1566+
inject: 'bearer',
1567+
refreshTokenFile: missingPath(),
1568+
},
1569+
deps(credentialsPath),
1570+
),
1571+
).rejects.toMatchObject({ code: 'VALIDATION_ERROR', exitCode: 5 });
1572+
});
1573+
1574+
it('runCreate --password-file missing → VALIDATION_ERROR (exit 5), no network', async () => {
1575+
const { credentialsPath } = makeCreds();
1576+
await expect(
1577+
runCreate(
1578+
{
1579+
profile: 'default',
1580+
output: 'json',
1581+
debug: false,
1582+
type: 'frontend',
1583+
name: 'FE',
1584+
targetUrl: 'https://example.com',
1585+
username: 'u',
1586+
passwordFile: missingPath(),
1587+
},
1588+
deps(credentialsPath),
1589+
),
1590+
).rejects.toMatchObject({ code: 'VALIDATION_ERROR', exitCode: 5 });
1591+
});
1592+
1593+
it('runUpdate --password-file missing → VALIDATION_ERROR (exit 5), no network', async () => {
1594+
const { credentialsPath } = makeCreds();
1595+
await expect(
1596+
runUpdate(
1597+
{
1598+
profile: 'default',
1599+
output: 'json',
1600+
debug: false,
1601+
projectId: 'p1',
1602+
passwordFile: missingPath(),
1603+
},
1604+
deps(credentialsPath),
1605+
),
1606+
).rejects.toMatchObject({ code: 'VALIDATION_ERROR', exitCode: 5 });
1607+
});
1608+
});

src/commands/project.ts

Lines changed: 70 additions & 7 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
emitDryRunBanner,
@@ -210,7 +211,7 @@ export async function runCreate(
210211
// Resolve password: flag > file > none
211212
let password = opts.password;
212213
if (password === undefined && opts.passwordFile !== undefined) {
213-
password = readFileSync(opts.passwordFile, 'utf8').trim();
214+
password = readSecretFileGuarded(opts.passwordFile, 'password-file');
214215
}
215216

216217
const idempotencyKey = opts.idempotencyKey ?? `cli-proj-create-${randomUUID()}`;
@@ -326,7 +327,7 @@ export async function runUpdate(
326327
// filesystem, even when --password-file is present.
327328
let password = opts.password;
328329
if (password === undefined && opts.passwordFile !== undefined) {
329-
password = readFileSync(opts.passwordFile, 'utf8').trim();
330+
password = readSecretFileGuarded(opts.passwordFile, 'password-file');
330331
}
331332

332333
const idempotencyKey = opts.idempotencyKey ?? `cli-proj-update-${randomUUID()}`;
@@ -467,7 +468,7 @@ export async function runCredential(
467468
// except `public` (which clears it).
468469
let credential = opts.credential;
469470
if (credential === undefined && opts.credentialFile !== undefined) {
470-
credential = readFileSync(opts.credentialFile, 'utf8').trim();
471+
credential = readSecretFileGuarded(opts.credentialFile, 'credential-file');
471472
}
472473
if (opts.authType !== 'public' && (credential === undefined || credential === '')) {
473474
throw localValidationError(
@@ -576,16 +577,18 @@ export async function runAutoAuth(
576577
// Resolve secrets from --*-file variants so they stay out of shell history.
577578
const password =
578579
opts.password ??
579-
(opts.passwordFile !== undefined ? readFileSync(opts.passwordFile, 'utf8').trim() : undefined);
580+
(opts.passwordFile !== undefined
581+
? readSecretFileGuarded(opts.passwordFile, 'password-file')
582+
: undefined);
580583
const clientSecret =
581584
opts.clientSecret ??
582585
(opts.clientSecretFile !== undefined
583-
? readFileSync(opts.clientSecretFile, 'utf8').trim()
586+
? readSecretFileGuarded(opts.clientSecretFile, 'client-secret-file')
584587
: undefined);
585588
const refreshToken =
586589
opts.refreshToken ??
587590
(opts.refreshTokenFile !== undefined
588-
? readFileSync(opts.refreshTokenFile, 'utf8').trim()
591+
? readSecretFileGuarded(opts.refreshTokenFile, 'refresh-token-file')
589592
: undefined);
590593

591594
const enabled = opts.disable !== true;
@@ -1082,3 +1085,63 @@ function localValidationError(message: string): ApiError {
10821085
},
10831086
});
10841087
}
1088+
1089+
/** Upper bound for any secret read from a `--*-file` flag. */
1090+
export const MAX_SECRET_FILE_BYTES = 64 * 1024;
1091+
1092+
/**
1093+
* Read a secret (password / credential / client-secret / refresh-token) from a
1094+
* `--*-file` flag, failing with a structured VALIDATION_ERROR (exit 5) instead
1095+
* of an unwrapped `readFileSync` error. A missing path, a non-regular file, or
1096+
* an oversized file all surface a typed envelope carrying the flag name — the
1097+
* same contract the `--code-file` guard in `test.ts` already provides. Without
1098+
* this, a missing file escaped as a raw `ENOENT` (exit 1) and broke the
1099+
* `--output json` envelope. `flagName` is the bare flag (e.g. `credential-file`).
1100+
*/
1101+
function readSecretFileGuarded(path: string, flagName: string): string {
1102+
const absolute = resolve(path);
1103+
let stat;
1104+
try {
1105+
stat = statSync(absolute);
1106+
} catch (err) {
1107+
const message = err instanceof Error ? err.message : String(err);
1108+
throw secretFileError(flagName, 'must point to a readable file', {
1109+
path: absolute,
1110+
error: message,
1111+
});
1112+
}
1113+
1114+
if (!stat.isFile()) {
1115+
throw secretFileError(flagName, 'must point to a regular file', { path: absolute });
1116+
}
1117+
1118+
if (stat.size > MAX_SECRET_FILE_BYTES) {
1119+
throw ApiError.fromEnvelope({
1120+
error: {
1121+
code: 'PAYLOAD_TOO_LARGE',
1122+
message: 'Secret file is too large.',
1123+
nextAction: `Flag \`--${flagName}\` is invalid: file must be at most ${MAX_SECRET_FILE_BYTES} bytes.`,
1124+
requestId: 'local',
1125+
details: { field: flagName, sizeBytes: stat.size, maxBytes: MAX_SECRET_FILE_BYTES },
1126+
},
1127+
});
1128+
}
1129+
1130+
return readFileSync(absolute, 'utf8').trim();
1131+
}
1132+
1133+
function secretFileError(
1134+
flagName: string,
1135+
reason: string,
1136+
details: Record<string, unknown>,
1137+
): ApiError {
1138+
return ApiError.fromEnvelope({
1139+
error: {
1140+
code: 'VALIDATION_ERROR',
1141+
message: 'Invalid request.',
1142+
nextAction: `Flag \`--${flagName}\` is invalid: ${reason}.`,
1143+
requestId: 'local',
1144+
details: { field: flagName, reason, ...details },
1145+
},
1146+
});
1147+
}

0 commit comments

Comments
 (0)