diff --git a/src/db/services/users-service.ts b/src/db/services/users-service.ts index d149635..92b2709 100644 --- a/src/db/services/users-service.ts +++ b/src/db/services/users-service.ts @@ -12,6 +12,7 @@ import { } from '@/db/voting-schema'; import { UserWithAddress } from '@/types/users'; import { logger } from '@/utils/logger'; +import { validateEthereumAddress } from '@/utils/viem'; export interface NeynarUserResponse { user: { @@ -102,6 +103,19 @@ export class UsersService { // If address is provided and user doesn't have it, add it if (address) { + // Validate address before processing + try { + validateEthereumAddress(address); + } catch (error) { + return { + success: false, + error: + error instanceof Error + ? error.message + : 'Invalid address format', + }; + } + const hasAddress = (existingUser.addresses as Address[])?.some( addr => addr.address.toLowerCase() === address.toLowerCase() ); @@ -159,6 +173,17 @@ export class UsersService { // Check if address-only user exists if (address && fid === undefined) { + // Validate address before processing + try { + validateEthereumAddress(address); + } catch (error) { + return { + success: false, + error: + error instanceof Error ? error.message : 'Invalid address format', + }; + } + const existingUserByAddress = await this.getUserByAddress(address); if (existingUserByAddress) { return { @@ -181,6 +206,17 @@ export class UsersService { // Add address if provided (with xmtp_enabled: false initially) if (address) { + // Validate address before insertion + try { + validateEthereumAddress(address); + } catch (error) { + return { + success: false, + error: + error instanceof Error ? error.message : 'Invalid address format', + }; + } + console.log('[UsersService] Adding address to new user:', { userId: user.id, address, @@ -227,6 +263,14 @@ export class UsersService { } async getUserByAddress(address: string): Promise { + // Validate address before querying + try { + validateEthereumAddress(address); + } catch { + // Return null for invalid addresses instead of throwing (this is a lookup method) + return null; + } + const result = await this.db .select({ id: users.id, @@ -267,6 +311,17 @@ export class UsersService { error?: string; }> { try { + // Validate address before processing + try { + validateEthereumAddress(address); + } catch (error) { + return { + success: false, + error: + error instanceof Error ? error.message : 'Invalid address format', + }; + } + // First find or create the user const userResult = await this.getUser(userId); @@ -344,6 +399,17 @@ export class UsersService { error?: string; }> { try { + // Validate address before processing + try { + validateEthereumAddress(address); + } catch (error) { + return { + success: false, + error: + error instanceof Error ? error.message : 'Invalid address format', + }; + } + // Check if address exists for this user const existingAddress = await this.db .select() @@ -764,6 +830,33 @@ export class UsersService { logger.debug(`🔍 DEBUG: Cleaned username: "${cleanUsername}"`); + // Validate input length - Neynar API requires q parameter to be 20 characters or less + if (cleanUsername.length > 20) { + // Check if it looks like an Ethereum address + if (cleanUsername.startsWith('0x') && cleanUsername.length === 42) { + logger.debug( + `❌ DEBUG: Ethereum address search not supported: "${cleanUsername}"` + ); + return { + success: true, + users: [], + error: + 'Address-based search is not supported. Please search by username instead.', + }; + } + + // Generic long string error + logger.debug( + `❌ DEBUG: Search query too long (${cleanUsername.length} chars, max 20): "${cleanUsername}"` + ); + return { + success: true, + users: [], + error: + 'Search query must be 20 characters or less. Please use a shorter search term.', + }; + } + try { // Use Neynar search endpoint for fuzzy matching if (!this.neynarApiKey) { diff --git a/src/routes/users.ts b/src/routes/users.ts index 1ad5a30..72d558d 100644 --- a/src/routes/users.ts +++ b/src/routes/users.ts @@ -11,6 +11,7 @@ import { requireSelfOrAdmin, } from '../middleware/auth'; import { verifyAddressSignature } from '../utils/signature-verification'; +import { validateEthereumAddress } from '../utils/viem'; import { sendMemberChangeWebhook } from '../utils/webhook-utils'; import { addMembersToXmtpGroup, @@ -89,6 +90,19 @@ async function sendXmtpGroupInvitation( users.get('/address/:address', requirePermission('read'), async c => { const address = c.req.param('address'); + // Validate address parameter + try { + validateEthereumAddress(address); + } catch (error) { + return c.json( + { + error: + error instanceof Error ? error.message : 'Invalid address format', + }, + 400 + ); + } + const usersService = getUsersService(); const user = await usersService.getUserByAddress(address); @@ -423,6 +437,18 @@ users.post('/search', requirePermission('read'), async c => { return c.json({ error: result.error || 'Search failed' }, 500); } + // Handle validation errors (e.g., search term too long, address search) + if (result.error) { + return c.json( + { + error: result.error, + users: result.users || [], + hint: 'Try searching with a shorter username or handle', + }, + 400 + ); + } + // For each user result, create or get the user to ensure we have their database ID const usersWithDbIds = await Promise.all( (result.users || []).map(async user => { @@ -457,6 +483,21 @@ users.get('/verifications', requirePermission('read'), async c => { return c.json({ error: 'FID parameter required' }, 400); } + // Validate address parameter if provided + if (address) { + try { + validateEthereumAddress(address); + } catch (error) { + return c.json( + { + error: + error instanceof Error ? error.message : 'Invalid address format', + }, + 400 + ); + } + } + try { // Using correct Neynar API endpoint for verifications by FID const url = new URL('https://hub-api.neynar.com/v1/verificationsByFid'); @@ -927,6 +968,19 @@ users.get( async c => { const address = c.req.param('address'); + // Validate address parameter + try { + validateEthereumAddress(address); + } catch (error) { + return c.json( + { + error: + error instanceof Error ? error.message : 'Invalid address format', + }, + 400 + ); + } + const usersService = getUsersService(); const user = await usersService.getUserByAddress(address); diff --git a/src/utils/viem.ts b/src/utils/viem.ts index f5d9f15..613d4da 100644 --- a/src/utils/viem.ts +++ b/src/utils/viem.ts @@ -1,4 +1,4 @@ -import { createPublicClient, http } from 'viem'; +import { createPublicClient, http, isAddress } from 'viem'; import { base, mainnet } from 'viem/chains'; // Map chain IDs to chain configurations @@ -20,3 +20,30 @@ export function getPublicClient(chainId: number) { transport: http(), }); } + +/** + * Validate an Ethereum address using viem's isAddress function + * Ensures address is 0x prefixed and 42 characters long with valid hex characters + * @param address - The address to validate + * @throws Error if address is invalid + */ +export function validateEthereumAddress(address: string): void { + if (!address) { + throw new Error('Address is required'); + } + + if (!isAddress(address)) { + throw new Error( + `Invalid Ethereum address: ${address}. Address must be 0x prefixed and 42 characters long with valid hexadecimal characters.` + ); + } +} + +/** + * Check if a string is a valid Ethereum address + * @param address - The address to check + * @returns true if valid, false otherwise + */ +export function isValidEthereumAddress(address: string): boolean { + return address && isAddress(address); +} diff --git a/tests/routes/users.test.ts b/tests/routes/users.test.ts index cec311f..c10b797 100644 --- a/tests/routes/users.test.ts +++ b/tests/routes/users.test.ts @@ -318,7 +318,9 @@ describe('Users Routes', () => { it('should return 404 for non-existent address', async () => { const headers = getTestAuthHeaders(); - const res = await app.request('/api/v1/users/address/0xnonexistent', { + // Use a unique address that won't be used by any other tests + const nonExistentAddress = `0x${Date.now().toString(16).padStart(40, '0')}`; + const res = await app.request(`/api/v1/users/address/${nonExistentAddress}`, { headers: headers.user1, }); @@ -344,7 +346,7 @@ describe('Users Routes', () => { const res = await app.request('/api/v1/users', { method: 'POST', headers: { ...headers.eliza, 'Content-Type': 'application/json' }, - body: JSON.stringify({ address: '0xnewaddress' }), + body: JSON.stringify({ address: '0x1111111111111111111111111111111111111111' }), }); const data = await res.json(); @@ -359,7 +361,7 @@ describe('Users Routes', () => { const res = await app.request('/api/v1/users', { method: 'POST', headers: { ...headers.eliza, 'Content-Type': 'application/json' }, - body: JSON.stringify({ address: '0xnewaddress2', fid: 99998 }), + body: JSON.stringify({ address: '0x2222222222222222222222222222222222222222', fid: 99998 }), }); const data = await res.json(); @@ -394,24 +396,25 @@ describe('Users Routes', () => { expect(data.error).toBe('Invalid request data'); }); - it('should handle service error', async () => { + it('should return 400 for invalid address format', async () => { const headers = getTestAuthHeaders(); - // Test with valid request data - service should handle gracefully const res = await app.request('/api/v1/users', { method: 'POST', headers: { ...headers.eliza, 'Content-Type': 'application/json' }, body: JSON.stringify({ address: 'invalid-address-format' }), }); - // Should succeed since address validation isn't done at service level - expect(res.status).toBe(201); + // Should return 400 since address validation is now done at service level + const data = await res.json(); + expect(res.status).toBe(400); + expect(data.error).toContain('Invalid Ethereum address'); }); it('should require authentication', async () => { const res = await app.request('/api/v1/users', { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ address: '0xnewaddress' }), + body: JSON.stringify({ address: '0x3333333333333333333333333333333333333333' }), }); const data = await res.json(); diff --git a/tests/services/users-service.test.ts b/tests/services/users-service.test.ts index 6cc77ce..495b4cf 100644 --- a/tests/services/users-service.test.ts +++ b/tests/services/users-service.test.ts @@ -189,13 +189,13 @@ describe('UsersService', () => { }); it('should return null for non-existent FID', async () => { - const user = await usersService.getUserByFid(999999); + const user = await usersService.getUserByFid(888888); // Use a different FID that won't be created by other tests expect(user).toBeNull(); }); it('should get user by address', async () => { - const testFid = 55555; - const testAddress = '0xgetuser1234567890123456789012345678901234'; + const testFid = 777777; // Use a truly unique FID not in fixtures + const testAddress = `0x${Date.now().toString(16).padStart(40, '0')}`; // Generate unique address // Create user and verify address const userResult = await usersService.createOrUpdateUser({ @@ -275,8 +275,8 @@ describe('UsersService', () => { it('should get verified addresses for user', async () => { const testFid = 77777; const testAddresses = [ - '0xverified1234567890123456789012345678901234', - '0xverified5678901234567890123456789012345678', + '0x1111111111111111111111111111111111111111', + '0x2222222222222222222222222222222222222222', ]; // First create/get the user @@ -310,7 +310,7 @@ describe('UsersService', () => { }); it('should return empty array for user with no verified addresses', async () => { - const userResult = await usersService.createOrUpdateUser({ fid: 777777 }); // Use unique FID not in fixtures + const userResult = await usersService.createOrUpdateUser({ fid: 555444 }); // Use unique FID not used by other tests expect(userResult.success).toBe(true); expect(userResult.user).toBeDefined(); @@ -464,6 +464,45 @@ describe('UsersService', () => { expect(result.users!.length).toBe(1); expect(result.users![0].username).toBe('vitalik.eth'); }); + + // BUILD-1144 fix: Test validation for search queries longer than 20 characters + it('should handle Ethereum address search with appropriate error message', async () => { + const ethereumAddress = '0x0B5f5a722Ac5E8EcEDf4da39A656fe5f1e76b34C'; + const result = await usersService.searchUsersByUsername(ethereumAddress); + + expect(result.success).toBe(true); + expect(result.users).toBeDefined(); + expect(Array.isArray(result.users)).toBe(true); + expect(result.users!.length).toBe(0); + expect(result.error).toBe( + 'Address-based search is not supported. Please search by username instead.' + ); + }); + + it('should handle long search queries with appropriate error message', async () => { + const longQuery = + 'this-is-a-very-long-username-that-exceeds-twenty-characters'; + const result = await usersService.searchUsersByUsername(longQuery); + + expect(result.success).toBe(true); + expect(result.users).toBeDefined(); + expect(Array.isArray(result.users)).toBe(true); + expect(result.users!.length).toBe(0); + expect(result.error).toBe( + 'Search query must be 20 characters or less. Please use a shorter search term.' + ); + }); + + it('should handle search queries exactly at 20 character limit', async () => { + const exactLimitQuery = '12345678901234567890'; // exactly 20 chars + const result = await usersService.searchUsersByUsername(exactLimitQuery); + + // Should succeed without validation error (though may return empty results from API) + expect(result.success).toBe(true); + expect(result.users).toBeDefined(); + expect(Array.isArray(result.users)).toBe(true); + expect(result.error).toBeUndefined(); // No validation error + }); }); describe('Service Configuration', () => {