Skip to content

Commit 54def97

Browse files
committed
fix(credentials): strip CR/LF from values to prevent INI injection
1 parent 18f6e6e commit 54def97

2 files changed

Lines changed: 41 additions & 1 deletion

File tree

src/lib/credentials.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,38 @@ describe('serializeCredentials', () => {
8686
expect(text).toContain('api_key = sk');
8787
expect(text).not.toContain('api_url');
8888
});
89+
90+
it('strips newline characters from values to prevent INI injection', () => {
91+
// A malicious apiUrl with embedded newlines could inject new key-value
92+
// pairs or section headers into the credentials file. The serializer
93+
// must strip \n and \r so the written file has exactly one value per
94+
// field and no injected content parsed as separate keys/sections.
95+
const malicious = 'https://evil.com\napi_key = sk-HIJACKED\n[admin]\napi_key = sk-admin';
96+
const text = serializeCredentials({ default: { apiKey: 'sk-real', apiUrl: malicious } });
97+
// The output must NOT contain a standalone [admin] section header
98+
// (it would be on its own line if injection succeeded)
99+
const lines = text.split('\n');
100+
// Only one section header exists: [default]
101+
const sectionHeaders = lines.filter(l => /^\[.+\]$/.test(l.trim()));
102+
expect(sectionHeaders).toEqual(['[default]']);
103+
// Only one api_key line exists (the real one, not an injected duplicate)
104+
const apiKeyLines = lines.filter(l => l.trim().startsWith('api_key'));
105+
expect(apiKeyLines).toHaveLength(1);
106+
expect(apiKeyLines[0]).toContain('sk-real');
107+
// Round-trip: reading back must return only the real key, not the injected one
108+
const parsed = parseCredentials(text);
109+
expect(parsed['default']?.apiKey).toBe('sk-real');
110+
expect(parsed['admin']).toBeUndefined();
111+
});
112+
113+
it('strips \\r\\n (CRLF) injection from values', () => {
114+
const text = serializeCredentials({ default: { apiUrl: 'https://x.com\r\napi_key = pwned' } });
115+
const parsed = parseCredentials(text);
116+
// The injected api_key must NOT be parsed as a real key
117+
expect(parsed['default']?.apiKey).toBeUndefined();
118+
// The api_url value is on one line (newlines stripped)
119+
expect(parsed['default']?.apiUrl).toContain('https://x.com');
120+
});
89121
});
90122

91123
describe('readCredentialsFile / readProfile', () => {

src/lib/credentials.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,15 @@ export function serializeCredentials(file: CredentialsFile): string {
8282
for (const field of fields) {
8383
const value = entry[field];
8484
if (value === undefined || value === '') continue;
85-
lines.push(`${FIELD_TO_FILE_KEY[field]} = ${value}`);
85+
// Guard against INI injection: a value containing newline characters
86+
// would be serialized across multiple lines, allowing an attacker to
87+
// inject arbitrary key-value pairs (or new section headers) into the
88+
// credentials file. A valid API key or URL never contains \n or \r.
89+
// Strip them so a compromised env var or MITM'd backend response
90+
// cannot override the stored api_key on subsequent reads.
91+
const sanitized = value.replace(/[\r\n]/g, '');
92+
if (sanitized === '') continue;
93+
lines.push(`${FIELD_TO_FILE_KEY[field]} = ${sanitized}`);
8694
}
8795
lines.push('');
8896
}

0 commit comments

Comments
 (0)