Skip to content

fix: add try-catch around JSON.parse(userData) in auth services #88

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lawrence3699 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: add try-catch around JSON.parse(userData) in auth services #88

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 7 additions & 1 deletion ...e/express/blueprint/hybrid-auth/mongodb/mongoose/feature/src/modules/auth/auth.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,13 @@ export class AuthService {
throw ApiError.badRequest("Invalid or expired otp");
}

const { name, email: userEmail, role, password } = JSON.parse(userData);
let parsedData;
try {
parsedData = JSON.parse(userData);
} catch {
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

On JSON.parse failure, OTP has already been consumed, but the pending-signup keys remain (user:${email}:${hashCode} and user:pending:${email}) until TTL. That can leave a user stuck in “signup already in progress” with no usable OTP; consider best-effort deleting both keys in this catch before throwing.

Suggested change
} catch {
} catch {
// Best-effort cleanup to avoid leaving the user stuck in "signup already in progress"
await redisClient.del(`user:${email}:${hashCode}`);
await redisClient.del(`user:pending:${email}`);

Copilot uses AI. Check for mistakes.
throw ApiError.badRequest("Invalid user data format");
}
const { name, email: userEmail, role, password } = parsedData;

const user = await User.create({
name,
Expand Down
8 changes: 7 additions & 1 deletion ...ates/node/express/blueprint/hybrid-auth/mongodb/mongoose/mvc/src/services/auth.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,13 @@ export class AuthService {
throw ApiError.badRequest("Invalid or expired otp");
}

const { name, email: userEmail, role, password } = JSON.parse(userData);
let parsedData;
try {
parsedData = JSON.parse(userData);
} catch {
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

On JSON.parse failure, OTP has already been consumed, but the pending-signup keys remain (user:${email}:${hashCode} and user:pending:${email}) until TTL. That can leave a user stuck in “signup already in progress” with no usable OTP; consider best-effort deleting both keys in this catch before throwing.

Suggested change
} catch {
} catch (parseError) {
// Best-effort cleanup to avoid leaving the user stuck with a pending signup
try {
await redisClient.del(`user:${email}:${hashCode}`);
await redisClient.del(`user:pending:${email}`);
} catch (cleanupError) {
logger.error(
cleanupError,
"Failed to clean up pending signup keys after JSON parse error"
);
}

Copilot uses AI. Check for mistakes.
throw ApiError.badRequest("Invalid user data format");
}
const { name, email: userEmail, role, password } = parsedData;

const user = await User.create({
name,
Expand Down
8 changes: 7 additions & 1 deletion ...express/blueprint/hybrid-auth/postgresql/drizzle/feature/src/modules/auth/auth.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,13 @@ export class AuthService {
throw ApiError.badRequest("Invalid or expired otp");
}

const { name, email: userEmail, role, password } = JSON.parse(userData);
let parsedData;
try {
parsedData = JSON.parse(userData);
} catch {
throw ApiError.badRequest("Invalid user data format");
}
Comment on lines +127 to +132
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

On JSON.parse failure, OTP has already been consumed, but the pending-signup keys remain (user:${email}:${hashCode} and user:pending:${email}) until TTL. That can leave a user stuck in “signup already in progress” with no usable OTP; consider best-effort deleting both keys in this catch before throwing.

Copilot uses AI. Check for mistakes.
const { name, email: userEmail, role, password } = parsedData;
Comment on lines +127 to +133
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 2, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "auth.service.ts" | grep -E "auth/auth.service.ts"

Repository: AkkalDhami/servercn

Length of output: 753

🏁 Script executed:

cat -n "packages/templates/node/express/blueprint/hybrid-auth/postgresql/drizzle/feature/src/modules/auth/auth.service.ts" | sed -n '115,145p'

Repository: AkkalDhami/servercn

Length of output: 1128

Validate parsed JSON shape before destructuring.

JSON.parse failure is handled, but line 133 still assumes an object payload. Valid JSON like null bypasses the catch and throws a TypeError during destructuring. This can result in unhandled errors and potentially invalid data being inserted into the database on line 140-144.

Add a type guard to ensure parsedData is an object before destructuring:

Proposed fix 
-    let parsedData;
+    let parsedData: unknown;
     try {
       parsedData = JSON.parse(userData);
     } catch {
       throw ApiError.badRequest("Invalid user data format");
     }
-    const { name, email: userEmail, role, password } = parsedData;
+    if (!parsedData || typeof parsedData !== "object") {
+      throw ApiError.badRequest("Invalid user data format");
+    }
+
+    const {
+      name,
+      email: userEmail,
+      role,
+      password
+    } = parsedData as Partial<{
+      name: string;
+      email: string;
+      role: string;
+      password: string;
+    }>;
+
+    if (
+      typeof name !== "string" ||
+      typeof userEmail !== "string" ||
+      typeof password !== "string"
+    ) {
+      throw ApiError.badRequest("Invalid user data format");
+    }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@packages/templates/node/express/blueprint/hybrid-auth/postgresql/drizzle/feature/src/modules/auth/auth.service.ts`
around lines 127 - 133, The code parses userData into parsedData but then
destructures without validating shape, so values like null or arrays will cause
runtime errors or bad inserts; in auth.service.ts add a type guard after
JSON.parse that checks parsedData is a non-null plain object (typeof parsedData
=== "object" && parsedData !== null && !Array.isArray(parsedData)), then verify
required fields (name, email, password, optional role) exist and have expected
primitive types (e.g., strings) and otherwise throw ApiError.badRequest("Invalid
user data format"); only after these checks perform the destructuring and
proceed to the DB insert (the block that currently destructures name, email:
userEmail, role, password and inserts on the following lines).

const enforcedRole = "user" as const;

if (role && role !== enforcedRole) {
Expand Down
8 changes: 7 additions & 1 deletion ...es/node/express/blueprint/hybrid-auth/postgresql/drizzle/mvc/src/services/auth.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,13 @@ export class AuthService {
throw ApiError.badRequest("Invalid or expired otp");
}

const { name, email: userEmail, role, password } = JSON.parse(userData);
let parsedData;
try {
parsedData = JSON.parse(userData);
} catch {
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

On JSON.parse failure, OTP has already been consumed, but the pending-signup keys remain (user:${email}:${hashCode} and user:pending:${email}) until TTL. That can leave a user stuck in “signup already in progress” with no usable OTP; consider best-effort deleting both keys in this catch before throwing.

Suggested change
} catch {
} catch (error) {
await Promise.allSettled([
redisClient.del(`user:${email}:${hashCode}`),
redisClient.del(`user:pending:${email}`)
]);

Copilot uses AI. Check for mistakes.
throw ApiError.badRequest("Invalid user data format");
}
const { name, email: userEmail, role, password } = parsedData;

const [user] = await db.insert(users).values({
name,
Expand Down
8 changes: 7 additions & 1 deletion ...e/express/blueprint/stateless-auth/mysql/drizzle/feature/src/modules/auth/auth.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,13 @@ export class AuthService {
throw ApiError.badRequest("Invalid or expired otp");
}

const { name, email: userEmail, role, password } = JSON.parse(userData);
let parsedData;
try {
parsedData = JSON.parse(userData);
} catch {
throw ApiError.badRequest("Invalid user data format");
}
Comment on lines +96 to +101
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

If JSON.parse(userData) fails here, OTP has already been consumed by OtpService.verifyOtp, but the Redis user:${email}:${hashCode} key is left behind until TTL. Consider best-effort deleting that key in the catch before throwing so corrupted/sensitive cached signup data doesn’t linger and the user can restart the flow cleanly.

Copilot uses AI. Check for mistakes.
const { name, email: userEmail, role, password } = parsedData;

const [existingUser] = await db
.insert(users)
Expand Down
8 changes: 7 additions & 1 deletion ...ates/node/express/blueprint/stateless-auth/mysql/drizzle/mvc/src/services/auth.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,13 @@ export class AuthService {
throw ApiError.badRequest("Invalid or expired otp");
}

const { name, email: userEmail, role, password } = JSON.parse(userData);
let parsedData;
try {
parsedData = JSON.parse(userData);
} catch {
throw ApiError.badRequest("Invalid user data format");
}
Comment on lines +93 to +98
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

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

If JSON.parse(userData) fails here, OTP has already been consumed by OtpService.verifyOtp, but the Redis user:${email}:${hashCode} key is left behind until TTL. Consider best-effort deleting that key in the catch before throwing so corrupted/sensitive cached signup data doesn’t linger and the user can restart the flow cleanly.

Copilot uses AI. Check for mistakes.
const { name, email: userEmail, role, password } = parsedData;

const [existingUser] = await db
.insert(users)
Expand Down
Loading