Skip to content

Commit 50387a4

Browse files
committed
fix(cli): validate profile names to prevent credentials-file corruption
A profile name (`--profile` / `TESTSPRITE_PROFILE`) is written verbatim as an INI section header (`[name]`) in `~/.testsprite/credentials`, but was never validated. A name containing the characters that break that grammar silently corrupted the file: --profile "prod]" -> serialises to `[prod]]`, which the section regex cannot match, so the api_key/api_url lines that follow are DROPPED on read. `setup` reports success while the credential never persists. --profile $'a\nb' -> the newline splits the header across two lines. --profile " x " -> does not round-trip (the parser trims section names, so it reads back as `x`). Add `assertValidProfileName` in credentials.ts (a conservative allowlist: letters, digits, dot, underscore, hyphen — covering `default`, `prod`, `ci-staging`, `team.qa`) and call it from every profile-keyed entry point (`readProfile`, `writeProfile`, `deleteProfile`). A malformed name now throws a typed VALIDATION_ERROR (exit 5) before any file write, instead of silently corrupting or failing to persist credentials. Adds unit coverage for the guard and the three entry points, plus a subprocess regression.
1 parent 18f6e6e commit 50387a4

3 files changed

Lines changed: 95 additions & 0 deletions

File tree

src/lib/credentials.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { join } from 'node:path';
44
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
55
import {
66
DEFAULT_PROFILE,
7+
assertValidProfileName,
78
defaultCredentialsPath,
89
deleteProfile,
910
ensureRestrictiveMode,
@@ -13,6 +14,7 @@ import {
1314
serializeCredentials,
1415
writeProfile,
1516
} from './credentials.js';
17+
import { ApiError } from './errors.js';
1618

1719
let tmpRoot: string;
1820
let credentialsPath: string;
@@ -168,3 +170,42 @@ describe('defaultCredentialsPath', () => {
168170
expect(defaultCredentialsPath().endsWith('/.testsprite/credentials')).toBe(true);
169171
});
170172
});
173+
174+
describe('assertValidProfileName / profile-name guard', () => {
175+
it('accepts conventional profile names', () => {
176+
for (const name of ['default', 'dev', 'prod', 'ci-staging', 'team.qa', 'a_b', 'P1']) {
177+
expect(() => assertValidProfileName(name)).not.toThrow();
178+
}
179+
});
180+
181+
it('rejects names that would corrupt the INI file, with a VALIDATION_ERROR (exit 5)', () => {
182+
// `prod]` -> `[prod]]` (unreadable); newline splits the header; padded
183+
// names do not round-trip (the parser trims section names); empty is not a
184+
// valid section.
185+
for (const name of ['prod]', '[weird', 'a\nb', ' spaced ', 'has space', '', 'a/b']) {
186+
let caught: unknown;
187+
try {
188+
assertValidProfileName(name);
189+
} catch (err) {
190+
caught = err;
191+
}
192+
expect(caught).toBeInstanceOf(ApiError);
193+
const apiErr = caught as ApiError;
194+
expect(apiErr.code).toBe('VALIDATION_ERROR');
195+
expect(apiErr.exitCode).toBe(5);
196+
expect(apiErr.nextAction).toContain('profile');
197+
}
198+
});
199+
200+
it('writeProfile rejects a malformed name and does NOT create the file', () => {
201+
expect(() => writeProfile('prod]', { apiKey: 'sk-1' }, { path: credentialsPath })).toThrow(
202+
ApiError,
203+
);
204+
expect(existsSync(credentialsPath)).toBe(false);
205+
});
206+
207+
it('readProfile and deleteProfile reject a malformed name', () => {
208+
expect(() => readProfile('a\nb', { path: credentialsPath })).toThrow(ApiError);
209+
expect(() => deleteProfile('a\nb', { path: credentialsPath })).toThrow(ApiError);
210+
});
211+
});

src/lib/credentials.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,43 @@ import {
99
} from 'node:fs';
1010
import { homedir } from 'node:os';
1111
import { dirname, join } from 'node:path';
12+
import { localValidationError } from './errors.js';
1213

1314
export const DEFAULT_PROFILE = 'default';
1415

16+
/**
17+
* Allowed profile-name characters. A profile name is written verbatim as an
18+
* INI section header (`[name]`) in the credentials file, so any character that
19+
* breaks that grammar must be rejected:
20+
* - `]` closes the header early — `prod]` serialises to `[prod]]`, which the
21+
* section regex cannot match, so the api_key/api_url lines that follow are
22+
* silently dropped on read (the credential never persists).
23+
* - CR/LF splits the header across lines, corrupting the file.
24+
* - leading/trailing whitespace does not round-trip — the parser trims
25+
* section names, so `[ prod ]` reads back as `prod`.
26+
* A conservative allowlist (letters, digits, dot, underscore, hyphen) matches
27+
* conventional profile names (`default`, `prod`, `ci-staging`, `team.qa`) and
28+
* cannot corrupt the file.
29+
*/
30+
const PROFILE_NAME_RE = /^[A-Za-z0-9._-]+$/;
31+
32+
/**
33+
* Throw a typed VALIDATION_ERROR (exit 5) when `profile` is not a safe INI
34+
* section name. Guards every credential read/write so a malformed `--profile`
35+
* (or `TESTSPRITE_PROFILE`) value fails loudly instead of silently corrupting
36+
* `~/.testsprite/credentials` or failing to persist a key written by `setup`.
37+
*/
38+
export function assertValidProfileName(profile: string): void {
39+
if (!PROFILE_NAME_RE.test(profile)) {
40+
throw localValidationError(
41+
'profile',
42+
'must contain only letters, digits, dot, underscore, or hyphen (no spaces, brackets, or newlines)',
43+
undefined,
44+
'flag',
45+
);
46+
}
47+
}
48+
1549
export function defaultCredentialsPath(): string {
1650
return join(homedir(), '.testsprite', 'credentials');
1751
}
@@ -99,6 +133,7 @@ export function readProfile(
99133
profile: string,
100134
options: CredentialsOptions = {},
101135
): ProfileEntry | undefined {
136+
assertValidProfileName(profile);
102137
const file = readCredentialsFile(options);
103138
return file[profile];
104139
}
@@ -108,13 +143,15 @@ export function writeProfile(
108143
entry: ProfileEntry,
109144
options: CredentialsOptions = {},
110145
): void {
146+
assertValidProfileName(profile);
111147
const path = resolvePath(options);
112148
const file = readCredentialsFile(options);
113149
file[profile] = { ...file[profile], ...entry };
114150
writeCredentialsAtomic(path, file);
115151
}
116152

117153
export function deleteProfile(profile: string, options: CredentialsOptions = {}): boolean {
154+
assertValidProfileName(profile);
118155
const path = resolvePath(options);
119156
const file = readCredentialsFile(options);
120157
if (!(profile in file)) return false;

test/cli.subprocess.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,23 @@ describe('project list subprocess', () => {
499499
}, 30_000);
500500
});
501501

502+
describe('a malformed --profile is rejected (exit 5), not silently corrupting credentials', () => {
503+
// A profile name becomes an INI section header (`[name]`). `prod]` would
504+
// serialise to `[prod]]`, which the parser cannot read back — `setup` would
505+
// report success while the key silently fails to persist. The guard fires on
506+
// any credential read/write path.
507+
it('exits 5 with a VALIDATION_ERROR naming the profile flag', async () => {
508+
const result = await runCli(['--output', 'json', '--profile', 'prod]', 'project', 'list'], {
509+
TESTSPRITE_API_KEY: 'sk-subproc',
510+
TESTSPRITE_API_URL: baseUrl,
511+
});
512+
expect(result.exitCode).toBe(5);
513+
const parsed = JSON.parse(result.stderr) as { error: { code: string; nextAction: string } };
514+
expect(parsed.error.code).toBe('VALIDATION_ERROR');
515+
expect(parsed.error.nextAction).toContain('profile');
516+
}, 30_000);
517+
});
518+
502519
describe('project get subprocess', () => {
503520
it('--output json returns the §6.1 Project shape', async () => {
504521
const result = await runCli(['--output', 'json', 'project', 'get', 'project_subproc'], {

0 commit comments

Comments
 (0)