fix(cliproxy): harden nickname validation and race condition handling

- Trim nickname in API route for consistency with CLI
- Block URL-unsafe chars (%, /, &, ?, #) in nickname
- Block reserved patterns (kiro-N, ghcp-N) used by auto-discovery
- Add reload-merge pattern in discovery to reduce race condition
This commit is contained in:
kaitranntt
2026-01-06 13:00:50 -05:00
parent d96c67ba81
commit 5970e70e26
2 changed files with 36 additions and 3 deletions
+33 -2
View File
@@ -95,7 +95,7 @@ export function generateNickname(email?: string): string {
/**
* Validate nickname
* Rules: 1-50 chars, any non-whitespace allowed (permissive)
* Rules: 1-50 chars, no whitespace, URL-safe, no reserved patterns
* @returns null if valid, error message if invalid
*/
export function validateNickname(nickname: string): string | null {
@@ -108,6 +108,14 @@ export function validateNickname(nickname: string): string | null {
if (/\s/.test(nickname)) {
return 'Nickname cannot contain whitespace';
}
// Block URL-unsafe chars that break routing
if (/[%\/&?#]/.test(nickname)) {
return 'Nickname cannot contain special URL characters (%, /, &, ?, #)';
}
// Block reserved patterns used by auto-discovery (kiro-1, ghcp-2, etc.)
if (/^(kiro|ghcp)-\d+$/i.test(nickname)) {
return 'Nickname cannot match reserved pattern (kiro-N, ghcp-N)';
}
return null;
}
@@ -597,7 +605,30 @@ export function discoverExistingAccounts(): void {
}
}
saveAccountsRegistry(registry);
// Reload-merge pattern: reduce race condition with concurrent OAuth registration
// Reload fresh registry and merge discovered accounts (fresh registry wins on conflicts)
const freshRegistry = loadAccountsRegistry();
for (const [providerName, discovered] of Object.entries(registry.providers)) {
if (!discovered) continue;
const prov = providerName as CLIProxyProvider;
if (!freshRegistry.providers[prov]) {
freshRegistry.providers[prov] = discovered;
} else {
// Merge accounts, preferring fresh registry's existing entries
const freshProviderAccounts = freshRegistry.providers[prov];
if (!freshProviderAccounts) continue;
for (const [id, meta] of Object.entries(discovered.accounts)) {
if (!freshProviderAccounts.accounts[id]) {
freshProviderAccounts.accounts[id] = meta;
// Set default if none exists
if (!freshProviderAccounts.default || freshProviderAccounts.default === 'default') {
freshProviderAccounts.default = id;
}
}
}
}
}
saveAccountsRegistry(freshRegistry);
}
/**
@@ -277,7 +277,9 @@ router.delete('/accounts/:provider/:accountId', (req: Request, res: Response): v
*/
router.post('/:provider/start', async (req: Request, res: Response): Promise<void> => {
const { provider } = req.params;
const { nickname, noIncognito: noIncognitoBody } = req.body;
const { nickname: nicknameRaw, noIncognito: noIncognitoBody } = req.body;
// Trim nickname for consistency with CLI (oauth-handler.ts trims input)
const nickname = typeof nicknameRaw === 'string' ? nicknameRaw.trim() : nicknameRaw;
// Validate provider
if (!validProviders.includes(provider as CLIProxyProvider)) {