Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
176 changes: 176 additions & 0 deletions app/api/v1/wallets/deprecate/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
import { NextRequest, NextResponse } from "next/server";
import { supabaseAdmin } from "@/app/lib/supabase";
import { withRateLimit } from "@/app/lib/rate-limit";
import { trackApiRequest, trackApiResponse, trackApiError } from "@/app/lib/server-analytics";
import { verifyJWT } from "@/app/lib/jwt";
import { DEFAULT_PRIVY_CONFIG } from "@/app/lib/config";

export const POST = withRateLimit(async (request: NextRequest) => {
const startTime = Date.now();

try {
// Step 1: Verify authentication token
const authHeader = request.headers.get("Authorization");
const token = authHeader?.replace("Bearer ", "");

if (!token) {
trackApiError(request, "/api/v1/wallets/deprecate", "POST", new Error("Unauthorized"), 401);
return NextResponse.json(
{ success: false, error: "Unauthorized" },
{ status: 401 }
);
}

let authenticatedUserId: string;
try {
const jwtResult = await verifyJWT(token, DEFAULT_PRIVY_CONFIG);
authenticatedUserId = jwtResult.payload.sub;

if (!authenticatedUserId) {
trackApiError(request, "/api/v1/wallets/deprecate", "POST", new Error("Invalid token"), 401);
return NextResponse.json(
{ success: false, error: "Invalid token" },
{ status: 401 }
);
}
} catch (jwtError) {
trackApiError(request, "/api/v1/wallets/deprecate", "POST", jwtError as Error, 401);
return NextResponse.json(
{ success: false, error: "Invalid or expired token" },
{ status: 401 }
);
}

const walletAddress = request.headers.get("x-wallet-address")?.toLowerCase();
const body = await request.json();
const { oldAddress, newAddress, txHash, userId } = body;

if (!walletAddress || !oldAddress || !newAddress || !userId) {
trackApiError(request, "/api/v1/wallets/deprecate", "POST", new Error("Missing required fields"), 400);
return NextResponse.json(
{ success: false, error: "Missing required fields" },
{ status: 400 }
);
}
Comment on lines +44 to +54
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

txHash is not validated but used as migration proof.

txHash is destructured from the body but omitted from the required-fields check on Line 48. It's then stored as migration_tx_hash on Line 93. If a caller omits txHash, the migration record will be created with a null transaction hash, undermining auditability.

Proposed fix
-    if (!walletAddress || !oldAddress || !newAddress || !userId) {
+    if (!walletAddress || !oldAddress || !newAddress || !userId || !txHash) {
🤖 Prompt for AI Agents
In `@app/api/v1/wallets/deprecate/route.ts` around lines 44 - 54, The request
validation in this route is missing txHash: ensure txHash is treated as a
required field along with walletAddress, oldAddress, newAddress, and userId by
updating the conditional that checks presence (where walletAddress, oldAddress,
newAddress, userId are validated) to also validate txHash, and update the error
path (trackApiError and NextResponse.json) to reflect the missing txHash; this
ensures migration_tx_hash stored later is never null and points to the same
identifiers (walletAddress, oldAddress, newAddress, txHash, userId) used in the
handler.


// Step 2: Verify userId matches authenticated user (CRITICAL SECURITY FIX)
if (userId !== authenticatedUserId) {
trackApiError(request, "/api/v1/wallets/deprecate", "POST", new Error("Unauthorized: userId mismatch"), 403);
return NextResponse.json(
{ success: false, error: "Unauthorized" },
{ status: 403 }
);
}

// Step 3: Verify wallet addresses match
if (newAddress.toLowerCase() !== walletAddress) {
trackApiError(request, "/api/v1/wallets/deprecate", "POST", new Error("Wallet address mismatch"), 403);
return NextResponse.json(
{ success: false, error: "Wallet address mismatch" },
{ status: 403 }
);
}

trackApiRequest(request, "/api/v1/wallets/deprecate", "POST", {
wallet_address: walletAddress,
old_address: oldAddress,
new_address: newAddress,
});

// Step 4: Atomic database operations with rollback on failure
// Ensure old (SCW) wallet exists and mark as deprecated (upsert so we insert if never saved to DB)
const now = new Date().toISOString();
const { error: deprecateError } = await supabaseAdmin
.from("wallets")
.upsert(
{
address: oldAddress.toLowerCase(),
user_id: userId,
wallet_type: "smart_contract",
status: "deprecated",
deprecated_at: now,
migration_completed: true,
migration_tx_hash: txHash,
updated_at: now,
},
{ onConflict: "address,user_id" }
);

if (deprecateError) {
trackApiError(request, "/api/v1/wallets/deprecate", "POST", deprecateError, 500);
throw deprecateError;
}

// Create or update new EOA wallet record
const { error: upsertError } = await supabaseAdmin
.from("wallets")
.upsert({
address: newAddress.toLowerCase(),
user_id: userId,
wallet_type: "eoa",
status: "active",
created_at: new Date().toISOString(),
});
Comment on lines +104 to +113
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Second upsert is missing onConflict, risking failure on retries.

The first upsert (Line 83-97) correctly specifies { onConflict: "address,user_id" }, but this upsert for the new EOA wallet does not. If the endpoint is called again (e.g., client retry after a timeout), this upsert will either fail on a unique constraint violation or insert a duplicate row depending on table constraints, instead of gracefully updating the existing record.

Proposed fix
     const { error: upsertError } = await supabaseAdmin
       .from("wallets")
       .upsert({
         address: newAddress.toLowerCase(),
         user_id: userId,
         wallet_type: "eoa",
         status: "active",
-        created_at: new Date().toISOString(),
-      });
+        updated_at: new Date().toISOString(),
+      }, { onConflict: "address,user_id" });
🤖 Prompt for AI Agents
In `@app/api/v1/wallets/deprecate/route.ts` around lines 104 - 113, The second
upsert call using supabaseAdmin (the block creating/updating the new EOA wallet
with address/newAddress and user_id/userId) is missing the onConflict option and
may fail or create duplicates on retries; update that upsert to include the same
conflict target used earlier (onConflict: "address,user_id") so retries will
perform an update instead of causing unique constraint errors or duplicate rows.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EOA wallet upsert missing onConflict causes duplicate rows

Medium Severity

The second upsert call (for the new EOA wallet) omits the onConflict: "address,user_id" option that the first upsert (line 96) explicitly includes. Without onConflict, Supabase defaults to conflict detection on the primary key. If the primary key is an auto-generated id not present in the payload, the upsert always inserts a new row instead of updating, creating duplicate wallet records on retries or re-migrations.

Fix in Cursor Fix in Web


if (upsertError) {
// Rollback: Restore old wallet status
await supabaseAdmin
.from("wallets")
.update({
status: "active",
deprecated_at: null,
migration_completed: false,
migration_tx_hash: null,
})
.eq("address", oldAddress.toLowerCase())
.eq("user_id", userId);

trackApiError(request, "/api/v1/wallets/deprecate", "POST", upsertError, 500);
throw upsertError;
}
Comment on lines +115 to +130
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Rollback failure is silently ignored.

If the rollback update on Lines 117-126 fails (e.g., network issue, DB outage), the error is never logged or surfaced. The code proceeds to throw upsertError, but the old wallet remains in a deprecated state with no record of the rollback failure.

Proposed fix
     if (upsertError) {
       // Rollback: Restore old wallet status
-      await supabaseAdmin
+      const { error: rollbackError } = await supabaseAdmin
         .from("wallets")
         .update({
           status: "active",
           deprecated_at: null,
           migration_completed: false,
           migration_tx_hash: null,
         })
         .eq("address", oldAddress.toLowerCase())
         .eq("user_id", userId);
 
+      if (rollbackError) {
+        console.error("Rollback failed after upsert error:", rollbackError);
+      }
+
       trackApiError(request, "/api/v1/wallets/deprecate", "POST", upsertError, 500);
       throw upsertError;
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (upsertError) {
// Rollback: Restore old wallet status
await supabaseAdmin
.from("wallets")
.update({
status: "active",
deprecated_at: null,
migration_completed: false,
migration_tx_hash: null,
})
.eq("address", oldAddress.toLowerCase())
.eq("user_id", userId);
trackApiError(request, "/api/v1/wallets/deprecate", "POST", upsertError, 500);
throw upsertError;
}
if (upsertError) {
// Rollback: Restore old wallet status
const { error: rollbackError } = await supabaseAdmin
.from("wallets")
.update({
status: "active",
deprecated_at: null,
migration_completed: false,
migration_tx_hash: null,
})
.eq("address", oldAddress.toLowerCase())
.eq("user_id", userId);
if (rollbackError) {
console.error("Rollback failed after upsert error:", rollbackError);
}
trackApiError(request, "/api/v1/wallets/deprecate", "POST", upsertError, 500);
throw upsertError;
}
🤖 Prompt for AI Agents
In `@app/api/v1/wallets/deprecate/route.ts` around lines 115 - 130, The rollback
update against the "wallets" table can itself fail and is currently ignored;
wrap the supabaseAdmin.from("wallets").update(...) call in a try/catch, and in
the catch use trackApiError(request, "/api/v1/wallets/deprecate", "POST",
rollbackError, 500) (or similar) to log the rollback failure along with context
(include oldAddress and userId), then either attach the rollback error to the
thrown upsertError (e.g., include both errors in a combined error or add a
property) or throw a new Error that preserves both upsertError and
rollbackError; ensure you still throw after logging so the original failure is
reported to the caller.


// Migrate KYC data
const { error: kycError } = await supabaseAdmin
.from("kyc_data")
.update({ wallet_address: newAddress.toLowerCase() })
.eq("wallet_address", oldAddress.toLowerCase())
.eq("user_id", userId);

if (kycError) {
console.error("KYC migration error:", kycError);
// Return partial success - wallet migrated but KYC migration failed
// This is better than rolling back the entire migration
const responseTime = Date.now() - startTime;
trackApiResponse("/api/v1/wallets/deprecate", "POST", 200, responseTime, {
wallet_address: walletAddress,
migration_successful: true,
kyc_migration_failed: true,
});

return NextResponse.json({
success: true,
message: "Wallet migrated but KYC migration failed",
kycMigrationFailed: true,
});
}

const responseTime = Date.now() - startTime;
trackApiResponse("/api/v1/wallets/deprecate", "POST", 200, responseTime, {
wallet_address: walletAddress,
migration_successful: true,
});

return NextResponse.json({ success: true, message: "Wallet migrated successfully" });
} catch (error) {
console.error("Error deprecating wallet:", error);
const responseTime = Date.now() - startTime;
trackApiError(request, "/api/v1/wallets/deprecate", "POST", error as Error, 500, {
response_time_ms: responseTime,
});

return NextResponse.json(
{ success: false, error: "Internal server error" },
{ status: 500 }
);
}
});
85 changes: 85 additions & 0 deletions app/api/v1/wallets/migration-status/route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
import { NextRequest, NextResponse } from "next/server";
import { supabaseAdmin } from "@/app/lib/supabase";

export async function GET(request: NextRequest) {
try {
const { searchParams } = new URL(request.url);
const userId = searchParams.get("userId");

if (!userId) {
return NextResponse.json({ error: "User ID required" }, { status: 400 });
}
Comment on lines +4 to +11
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

No authentication or authorization — any caller can query any user's migration status.

The userId is taken directly from the query string with no verification that the caller is the corresponding authenticated user. This exposes migration status to unauthenticated probing. Add Privy token validation (or similar auth middleware) and verify the authenticated user matches the requested userId.

🤖 Prompt for AI Agents
In `@app/api/v1/wallets/migration-status/route.ts` around lines 4 - 11, The GET
handler exposes any user's migration status because it trusts the query param
userId directly; update the GET function (export async function GET) to validate
the caller's authentication token (e.g., verify Privy token from the request
headers) and extract the authenticated user id, then compare that id to the
requested searchParams.get("userId"); if the token is missing/invalid return a
401 and if the ids do not match return a 403, otherwise proceed with the
existing logic. Ensure you call the existing token verification utility or
implement a small verifyPrivyToken helper, use NextRequest headers to read the
auth token and NextResponse to return the proper status codes, and fail closed
on any verification error.


// Check if user has completed migration
const { data, error } = await supabaseAdmin
.from("wallets")
.select("migration_completed, status, wallet_type")
.eq("user_id", userId)
.eq("wallet_type", "smart_contract")
.single();
Comment on lines +13 to +19
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

echo "=== Finding SQL migration files ==="
fd -e sql

echo ""
echo "=== Checking for CREATE TABLE statements in migrations ==="
rg -n "CREATE TABLE" --type sql -A 10

echo ""
echo "=== Checking for wallets table definition ==="
rg -n "wallets" --type sql -B 2 -A 5

Repository: paycrest/noblocks

Length of output: 2959


🏁 Script executed:

#!/bin/bash

echo "=== Content of wallet_migrations.sql ==="
cat supabase/migrations/create_wallet_migrations.sql

echo ""
echo "=== Full content of migration-status route ==="
cat app/api/v1/wallets/migration-status/route.ts

echo ""
echo "=== Searching for 'wallets' table reference elsewhere ==="
rg -n "\.from\(['\"]wallets['\"]" --type ts

Repository: paycrest/noblocks

Length of output: 6642


Create the wallets table or update the route to query wallet_migrations with correct column mappings.

The code queries a non-existent .from("wallets") table. SQL migrations only define wallet_migrations, not wallets. Additionally, the queried columns (migration_completed, status, wallet_type) and filter (user_id) do not match the wallet_migrations schema, which uses privy_user_id and has status values of 'pending', 'in_progress', 'completed', or 'failed'. Either:

  1. Create a new SQL migration for the wallets table with the required columns, or
  2. Update the route to query wallet_migrations instead and map column names correctly (e.g., privy_user_id instead of user_id, adjust status field logic)

The PGRST205 error handling suggests this was known, but the route will fail unless the missing table is created or the query is corrected. Multiple other routes (app/api/v1/wallets/deprecate/route.ts) also reference the non-existent wallets table.

🤖 Prompt for AI Agents
In `@app/api/v1/wallets/migration-status/route.ts` around lines 13 - 19, The route
currently queries a non-existent table via supabaseAdmin.from("wallets") and
requests columns that don't match your migrations; update
app/api/v1/wallets/migration-status/route.ts to query "wallet_migrations"
instead of "wallets", select the correct columns (e.g., .select("privy_user_id,
status") or whatever exists in the migration schema), replace .eq("user_id",
userId) with .eq("privy_user_id", userId), and treat migration completion by
checking status === "completed" (or map status values
'pending'|'in_progress'|'completed'|'failed' appropriately) rather than relying
on a non-existent migration_completed field; also apply the same fix to other
routes referencing supabaseAdmin.from("wallets") such as
app/api/v1/wallets/deprecate/route.ts or create a new SQL migration to add a
wallets table with the expected columns if you prefer that approach.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Migration status endpoint lacks authentication check

Medium Severity

The /api/v1/wallets/migration-status GET endpoint accepts a userId query parameter and queries the database without any authentication or authorization checks. Any unauthenticated caller can look up any user's migration status, wallet type, and whether they have a smart wallet. This is inconsistent with the /api/v1/wallets/deprecate endpoint, which properly verifies JWT authentication and enforces user ID matching.

Fix in Cursor Fix in Web


// Handle specific error codes
if (error) {
// PGRST116 = no rows found (user has no smart wallet) - this is OK
if (error.code === "PGRST116") {
return NextResponse.json({
migrationCompleted: false,
status: "unknown",
hasSmartWallet: false
});
}

// PGRST205 = table not found in schema cache (migration not run yet)
if (error.code === "PGRST205") {
console.warn("⚠️ Wallets table not found in schema cache. Migration may not be applied yet.");
return NextResponse.json({
migrationCompleted: false,
status: "unknown",
hasSmartWallet: true, // Assume true to show banner
error: "Database schema not ready"
}, { status: 200 }); // Return 200 so frontend doesn't break
}

// For other errors, log and return safe fallback
console.error("Database query error:", error);
return NextResponse.json({
migrationCompleted: false,
status: "unknown",
hasSmartWallet: true, // Assume true to show banner on error
error: error.message
}, { status: 200 }); // Return 200 so frontend doesn't break
Comment on lines +32 to +50
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Returning HTTP 200 for all database errors masks failures from monitoring.

While returning 200 prevents frontend breakage, it also makes it impossible to set up alerts on error rates. Consider returning appropriate 5xx status codes and handling them gracefully on the client side, or at minimum emit structured error metrics/logs that can be monitored independently.

🤖 Prompt for AI Agents
In `@app/api/v1/wallets/migration-status/route.ts` around lines 32 - 50, The
current handler returns HTTP 200 for DB errors which masks failures; update the
PGRST205 branch and the generic error branch to return appropriate 5xx status
codes (e.g., 503 Service Unavailable for transient DB/schema issues and 500 for
unexpected DB errors) instead of always 200 by changing the NextResponse.json
status values, and add structured logging/metrics calls that include error.code
and error.message (e.g., log with a logger/error-metrics method) so monitoring
can alert on occurrences; locate and modify the error handling around the
error.code === "PGRST205" check and the subsequent generic error path where
NextResponse.json is returned.

}

return NextResponse.json({
migrationCompleted: data?.migration_completed ?? false,
status: data?.status ?? "unknown",
hasSmartWallet: !!data
});
} catch (error: any) {
// Handle connection errors (DNS, network, etc.)
const errorMessage = error?.message || String(error);
const isConnectionError =
errorMessage.includes("ENOTFOUND") ||
errorMessage.includes("fetch failed") ||
errorMessage.includes("ECONNREFUSED") ||
errorMessage.includes("ETIMEDOUT");

if (isConnectionError) {
console.warn("⚠️ Database connection error, returning fallback response:", errorMessage);
return NextResponse.json({
migrationCompleted: false,
status: "unknown",
hasSmartWallet: true, // Assume true to show banner if DB is down
error: "Database temporarily unavailable"
}, { status: 200 }); // Return 200 so frontend doesn't break
}

console.error("Error checking migration status:", error);
return NextResponse.json({
error: error instanceof Error ? error.message : "Internal server error",
migrationCompleted: false,
status: "error",
hasSmartWallet: true // Assume true to show banner on error
}, { status: 200 }); // Return 200 so frontend doesn't break
}
}
11 changes: 7 additions & 4 deletions app/components/AppLayout.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
"use client";
import React from "react";
import Script from "next/script";
import config from "../lib/config";
Expand All @@ -11,8 +12,10 @@ import {
PWAInstall,
NoticeBanner,
} from "./index";
import { MigrationBannerWrapper } from "../context";

export default function AppLayout({ children }: { children: React.ReactNode }) {

return (
<Providers>
<div className="min-h-full min-w-full bg-white transition-colors dark:bg-neutral-900">
Expand All @@ -21,6 +24,7 @@ export default function AppLayout({ children }: { children: React.ReactNode }) {
{config.noticeBannerText && (
<NoticeBanner textLines={config.noticeBannerText.split("|")} />
)}
<MigrationBannerWrapper />
</div>
<LayoutWrapper footer={<Footer />}>
<MainContent>{children}</MainContent>
Expand All @@ -35,11 +39,10 @@ export default function AppLayout({ children }: { children: React.ReactNode }) {
{`window.BrevoConversationsID=${JSON.stringify(config.brevoConversationsId)};
window.BrevoConversations=window.BrevoConversations||function(){
(window.BrevoConversations.q=window.BrevoConversations.q||[]).push(arguments)};
window.BrevoConversationsSetup=${
config.brevoConversationsGroupId
? `{groupId:${JSON.stringify(config.brevoConversationsGroupId)}}`
window.BrevoConversationsSetup=${config.brevoConversationsGroupId
? `{groupId:${JSON.stringify(config.brevoConversationsGroupId)}}`
: '{}'
};
};
`}
</Script>
<Script
Expand Down
Loading