fix: v1 API audit - stealth support, enriched responses, new endpoints, doc corrections#25
Conversation
|
@Shreyassp002 is attempting to deploy a commit to the Shreyas' projects Team on Vercel. A member of the Team first needs to authorize it. |
Shreyassp002
changed the title
⚡ Flash Review
🚨 Critical (must fix before merge)
💡 Suggestions (nice to have)
✅ What's good: The ⚡ Powered by Flash Review · Report Issue
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| try { | ||
| const { searchParams } = new URL(req.url) | ||
| const type = searchParams.get('type') || 'all' | ||
| const hasUSDC = searchParams.get('hasUSDC') === 'true' |
There was a problem hiding this comment.
⚡ Flash Review
🚨 Security: The type query parameter is taken directly from user input without validation against allowed values. If getChains uses this parameter directly in a SQL query, it could be vulnerable to SQL injection.
Fix: Implement Zod validation for type to ensure it's one of the expected enum values ('all', 'evm', 'solana').
import { z } from 'zod'
const querySchema = z.object({
type: z.enum(['all', 'evm', 'solana']).default('all'),
hasUSDC: z.string().optional().transform(s => s === 'true')
})
const { type, hasUSDC } = querySchema.parse(Object.fromEntries(searchParams))
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const supabase = createServerClient() as any | ||
|
|
There was a problem hiding this comment.
⚡ Flash Review
🧹 Code Quality: Using as any for the Supabase client bypasses TypeScript's type checking. This can lead to runtime errors that could have been caught at compile time.
Fix: Ensure createServerClient() returns a properly typed Supabase client. You might need to define a type for your Supabase database schema and pass it to createServerClient<Database>().
| total: result.total, | ||
| }) | ||
| } catch (err) { | ||
| console.error('V1 Chains Error:', err) |
There was a problem hiding this comment.
⚡ Flash Review
🧹 Code Quality: console.error should generally be avoided in production API routes. While it's an error, it doesn't integrate with structured logging systems.
Fix: Replace console.error with a dedicated logging utility (e.g., a custom logger, Pino, Winston) that can provide structured logs, context, and integrate with monitoring tools.
| import { NextRequest, NextResponse } from 'next/server' | ||
| import { createServerClient } from '@/lib/supabase' | ||
| import { verifyApiKey } from '@/lib/api/verify-api-key' | ||
| import { z } from 'zod' |
There was a problem hiding this comment.
⚡ Flash Review
🐛 Bug: Chain IDs are typically numbers. Allowing string | number for default_receive_chain can lead to type mismatches or unexpected behavior if the Supabase column is strictly typed (e.g., integer or bigint).
Fix: Standardize on z.number() for chain IDs. If string inputs are expected, add a transform to parse them to numbers, ensuring robust validation.
default_receive_chain: z
.union([z.number(), z.string().transform(s => parseInt(s, 10))])
.nullable()
.optional()
.refine(val => val === null || !isNaN(val), {
message: 'Invalid chain ID format',
}),
|
|
||
| if (error) { | ||
| return NextResponse.json({ error }, { status: 401 }) | ||
| } |
There was a problem hiding this comment.
⚡ Flash Review
🧹 Code Quality: Using any bypasses TypeScript's type checking and can hide potential issues. While sometimes necessary for external libraries or specific Supabase client setups, it's best to type the Supabase client correctly if possible.
Fix: Explore options to correctly type createServerClient() or define a more specific interface for the client methods used.
|
|
||
| if (data.default_receive_chain !== undefined) updateObj.default_receive_chain = data.default_receive_chain | ||
| if (data.default_receive_token !== undefined) updateObj.default_receive_token = data.default_receive_token | ||
| if (data.business_name !== undefined) updateObj.business_name = data.business_name |
There was a problem hiding this comment.
⚡ Flash Review
🐛 Bug: Token addresses must be consistently lowercased to avoid issues with different casing representations on-chain (e.g., checksummed vs. non-checksummed addresses). Storing them as-is can lead to inconsistencies when querying or comparing.
Fix: Convert data.default_receive_token to lowercase before storing it in the database.
if (data.default_receive_token !== undefined) {
updateObj.default_receive_token = data.default_receive_token?.toLowerCase()
}
| .from('merchants') | ||
| .select( | ||
| 'wallet_address, business_name, email, default_receive_chain, default_receive_token, stealth_enabled, created_at', | ||
| ) |
There was a problem hiding this comment.
⚡ Flash Review
⚡ Performance: Supabase update operations can return the updated row directly by chaining .select() to the update query. This avoids a second database call to fetch the updated data.
Fix: Modify the update query to include .select() and capture the updated data directly.
const { data: updatedMerchant, error: updateError } = await supabase
.from('merchants')
.update(updateObj)
.eq('id', merchant.id)
.select(
'wallet_address, business_name, email, default_receive_chain, default_receive_token, stealth_enabled, created_at',
)
.single()
// Then use updatedMerchant instead of calling supabase.from('merchants').select(...) again
| const { searchParams } = new URL(req.url) | ||
| const rawType = searchParams.get('type') || 'all' | ||
| const type = ['all', 'evm', 'solana', 'bitcoin', 'near', 'tron', 'sui', 'ton', 'starknet', 'aptos'].includes(rawType) ? rawType : 'all' | ||
| const hasUSDC = searchParams.get('hasUSDC') === 'true' |
There was a problem hiding this comment.
⚡ Flash Review
🧹 Code Quality: Using as any here bypasses TypeScript's type checking. While createServerClient might return a complex type, it's better to explicitly type it or infer it correctly to maintain type safety and catch potential errors early.
Fix: Consider importing the correct Supabase client type (e.g., SupabaseClient<Database>) or ensuring createServerClient is typed correctly to avoid any.
|
|
||
| return NextResponse.json({ | ||
| data: result.chains, | ||
| total: result.total, |
There was a problem hiding this comment.
⚡ Flash Review
🧹 Code Quality: console.error should be replaced with a structured logging solution in production environments. This helps with centralized error monitoring, analysis, and avoids potential performance overhead or missing critical alerts.
Fix: Integrate a dedicated logger (e.g., Pino, Winston, or a custom wrapper around a service like Sentry) to handle errors consistently across the application.
| if (error) { | ||
| return NextResponse.json({ error }, { status: 401 }) | ||
| } | ||
|
|
There was a problem hiding this comment.
⚡ Flash Review
🧹 Code Quality: Using as any bypasses TypeScript's type safety. The createServerClient() function should return a properly typed Supabase client. Consider defining a Database type for Supabase and using createServerClient<Database>() to ensure type safety for database operations.
| const data = validation.data | ||
|
|
||
| // Build update object dynamically — only include provided fields | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any |
There was a problem hiding this comment.
⚡ Flash Review
🧹 Code Quality: Using Record<string, any> for updateObj bypasses type safety. Since updateObj is built from validation.data, its type can be more accurately inferred or explicitly defined based on updateMerchantSchema's partial type. For example, Partial<z.infer<typeof updateMerchantSchema>> would provide better type guarantees.
| return NextResponse.json({ error: 'No fields to update' }, { status: 400 }) | ||
| } | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any |
There was a problem hiding this comment.
⚡ Flash Review
🧹 Code Quality: Similar to line 21, using as any here bypasses type safety. Ensure the Supabase client is correctly typed to leverage TypeScript's benefits and prevent potential runtime errors due to type mismatches.
| 'wallet_address, business_name, email, default_receive_chain, default_receive_token, stealth_enabled, created_at', | ||
| ) | ||
| .single() | ||
|
|
There was a problem hiding this comment.
⚡ Flash Review
🧹 Code Quality: console.error should be replaced with a structured logging solution (e.g., Pino, Winston) for production environments. This ensures errors are properly captured, monitored, and don't leak sensitive information in raw console logs, which is crucial for a platform handling REAL money.
| transactions: transactions || [] | ||
| updated_at: link.updated_at, | ||
| transactions: transactions || [], | ||
| }) |
There was a problem hiding this comment.
⚡ Flash Review
🐛 Bug: Failing to parse the request body (e.g., malformed JSON) would previously throw an unhandled error, resulting in a generic 500 Internal Server Error. This try/catch block correctly handles such cases by returning a specific 400 Bad Request, improving API robustness and developer experience.
Fix: This change is a good fix.
| req: NextRequest, | ||
| { params }: { params: Promise<{ id: string }> } | ||
| { params }: { params: Promise<{ id: string }> }, | ||
| ) { |
There was a problem hiding this comment.
⚡ Flash Review
🚨 Security: This is a critical security improvement. Without proper input validation, an attacker could send malformed or unexpected data, potentially leading to database errors, unexpected application behavior, or even injection attacks if the data were used in a less secure context (though Supabase's ORM mitigates direct SQL injection, it's still best practice to validate at the API boundary).
Fix: The use of updatePaymentLinkSchema.safeParse(body) correctly validates the incoming request body against a predefined schema, ensuring only valid data is processed.
| let body | ||
| try { | ||
| body = await req.json() | ||
| } catch { |
There was a problem hiding this comment.
⚡ Flash Review
🧹 Code Quality: The updateObj is explicitly typed as Record<string, any>. While functional, using any reduces type safety. It would be more robust to derive a type from the Zod schema, such as Partial<z.infer<typeof updatePaymentLinkSchema>> & { updated_at: string }.
Fix: Consider defining a more specific type for updateObj to leverage TypeScript's type checking capabilities, e.g., const updateObj: Partial<PaymentLinkTable> & { updated_at: string } = { ... } (assuming PaymentLinkTable is your DB type).
| }, { status: 400 }) | ||
| } | ||
|
|
||
| const data = validation.data |
There was a problem hiding this comment.
⚡ Flash Review
🎨 UX: This check is a good addition for API consumer experience. If a client sends a PATCH request with no actual fields to update (only updated_at would be set internally), returning a 400 Bad Request with a clear message is more informative than a 200 OK, which might imply a change occurred when none did.
Fix: This change is a good improvement.
|
|
||
| try { | ||
| const { searchParams } = new URL(req.url) | ||
| const chainKey = searchParams.get('chainKey') |
There was a problem hiding this comment.
⚡ Flash Review
🚨 Security: The chainKey query parameter is used to fetch tokens but lacks proper validation. If getTokens uses this value directly in a Supabase query, it could be vulnerable to SQL injection. Even if getTokens uses parameterized queries, validating inputs at the API boundary is a critical security practice.
Fix: Add Zod validation to ensure chainKey is a valid chain identifier (e.g., a numeric string or a specific format) before passing it to getTokens:
import { z } from 'zod'
// ...
const querySchema = z.object({
chainKey: z.string().regex(/^\d+$/).transform(Number), // Assuming numeric chain IDs
})
const parsed = querySchema.safeParse({ chainKey })
if (!parsed.success) {
return NextResponse.json({ error: 'Invalid chainKey parameter' }, { status: 400 })
}
const validatedChainKey = parsed.data.chainKey
// ... use validatedChainKey
| } | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const supabase = createServerClient() as any |
There was a problem hiding this comment.
⚡ Flash Review
🧹 Code Quality: Using as any bypasses TypeScript's type safety. This can hide potential type mismatches or incorrect usage of the Supabase client, which is critical for a system handling real money.
Fix: Infer the correct type for the Supabase client or import the specific type definition if available (e.g., SupabaseClient<Database>) to maintain strong typing throughout the application.
| chainKey: result.chainKey, | ||
| total: result.total, | ||
| }) | ||
| } catch (err) { |
There was a problem hiding this comment.
⚡ Flash Review
🧹 Code Quality: console.error should not be used in production code. It lacks structured logging capabilities, making it difficult to monitor, filter, and analyze errors in a production environment.
Fix: Replace console.error with a proper structured logging solution (e.g., a dedicated logger service or a library like Pino/Winston) that can integrate with your observability stack.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
1 participant
Summary
src/lib/api/get-chains.tsandsrc/lib/api/get-tokens.ts, refactored internal routes to use themdescription,use_stealthto schema, stealth enforcement logic, writesreceive_token_symbolto DB, enriched response with all fields