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 committed 2026-01-06 13:00:50 -05:00
1 parent d96c67ba81
commit 5970e70e26
2 files changed
+36 -3

No files matched your search

+33 -2
View File
@@ -95,7 +95,7 @@ export function generateNickname(email?: string): string {
/** /**
* Validate nickname * 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 * @returns null if valid, error message if invalid
*/ */
export function validateNickname(nickname: string): string | null { export function validateNickname(nickname: string): string | null {
@@ -108,6 +108,14 @@ export function validateNickname(nickname: string): string | null {
if (/\s/.test(nickname)) { if (/\s/.test(nickname)) {
return 'Nickname cannot contain whitespace'; 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; 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> => { router.post('/:provider/start', async (req: Request, res: Response): Promise<void> => {
const { provider } = req.params; 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 // Validate provider
if (!validProviders.includes(provider as CLIProxyProvider)) { if (!validProviders.includes(provider as CLIProxyProvider)) {