Skip to content

Refactor auth controller into modular structure #1376

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

Merged
alexanmtz merged 4 commits into from
Feb 4, 2026
Merged

Refactor auth controller into modular structure #1376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
01f8639
Initial plan
Copilot Feb 4, 2026
cc4461b
Refactor auth controller: move methods to new structure
Copilot Feb 4, 2026
8eb9fb0
Replace require() with import statements in refactored controllers
Copilot Feb 4, 2026
cc059d2
Fix TypeScript build errors for userDisconnectGithub and models.User
Copilot Feb 4, 2026
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
441 changes: 0 additions & 441 deletions src/app/controllers/auth.ts
View file
Open in desktop

This file was deleted.

48 changes: 48 additions & 0 deletions src/app/controllers/auth/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { userChangeEmail } from '../../../modules/users/userChangeEmail'
import { userConfirmChangeEmail } from '../../../modules/users/userConfirmChangeEmail'
import userDisconnectGithub from '../../../modules/users/userDisconectGithub'
import secrets from '../../../config/secrets'
import * as user from '../../../modules/users'
import passport from 'passport'

export const changeEmail = async (req: any, res: any) => {
const userId = req.user.id
Expand Down Expand Up @@ -36,3 +40,47 @@ export const confirmChangeEmail = async (req: any, res: any) => {
res.redirect(`${process.env.FRONTEND_HOST}/#/signin/email-change-failed`)
}
}

export const authorizeGithubPrivateIssue = (req: any, res: any) => {
const params = req.query
const uri = encodeURIComponent(
`${process.env.API_HOST}/callback/github/private?userId=${params.userId}&url=${params.url}`
)
res.redirect(
`https://github.com/login/oauth/authorize?response_type=code&redirect_uri=${uri}&scope=repo&client_id=${secrets.github.id}`
)
}

export const disconnectGithub = async (req: any, res: any) => {
try {
const data = await userDisconnectGithub({ userId: req.user.id })
if (data) {
res.redirect(`${process.env.FRONTEND_HOST}/#/profile/user-account/?disconnectAction=success`)
} else {
res.redirect(`${process.env.FRONTEND_HOST}/#/profile/user-account/?disconnectAction=error`)
}
} catch (error: any) {
// eslint-disable-next-line no-console
console.log(error)
res.redirect(`${process.env.FRONTEND_HOST}/#/profile/user-account/?disconnectAction=error`)
}
}

export const connectGithub = (req: any, res: any, next: any) => {
const user = req.user
if (user) {
passport.authenticate('github', {
scope: ['user:email'],
state: req.user.email
})(req, res, next)
} else {
res.redirect(`${process.env.FRONTEND_HOST}/#/signin/invalid`)
}
}

export const authorizeLocal = (req: any, res: any, next: any) => {
if (req.user && req.user.token) {
res.set('Authorization', 'Bearer ' + req.user.token)
res.redirect(`${process.env.FRONTEND_HOST}/#/token/${req.user.token}`)
}
}
18 changes: 18 additions & 0 deletions src/app/controllers/auth/callbacks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
export const callbackGithub = (req: any, res: any) => {
const user = req.user
if (user && user.token) {
if (user.login_strategy === 'local' || user.login_strategy === null) {
res.redirect(
`${process.env.FRONTEND_HOST}/#/profile/user-account/?connectGithubAction=success`
)
} else {
res.redirect(`${process.env.FRONTEND_HOST}/#/token/${user.token}`)
}
}
}

export const callbackBitbucket = (req: any, res: any) => {
if (req.user && req.user.token) {
res.redirect(`${process.env.FRONTEND_HOST}/#/token/` + req.user.token)
}
}
68 changes: 68 additions & 0 deletions src/app/controllers/user/account.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import * as user from '../../../modules/users'

export const account = async (req: any, res: any) => {
try {
const data = await user.userAccount({ id: req.user.id })
res.send(data)

Check warning

Code scanning / CodeQL

Information exposure through a stack trace Medium

This information exposed to the user depends on
stack trace information
Loading
.

Copilot Autofix

AI 6 months ago

General approach: do not propagate raw exceptions or stack traces to user-facing responses. Instead, log detailed error information on the server and return a generic, safe payload to the caller (controller), which in turn should send only that generic payload to the client. That means userAccount should never return e directly; it should return either an empty object or a normalized error structure that does not include stack traces or internal implementation details. The controller logic can remain the same if the returned data is guaranteed safe.

Best concrete fix for this code:

  1. In src/modules/users/userAccount.ts, change the catch block inside userAccount so that:
    • It still logs the full error to the server (console.log('could not find customer', e)).
    • It returns a safe, generic object instead of the raw error: for example, { error: 'ACCOUNT_RETRIEVAL_FAILED' } or {}. This prevents potential stack trace or internal Stripe information from being sent out.
  2. In src/app/controllers/user/account.ts, the account handler currently returns whatever userAccount yields via res.send(data). Once userAccount is returning a sanitized object, this is safe and we do not need to modify the controller logic for this issue. The other handlers (accountCreate, accountCountries, accountBalance, accountUpdate, accountDelete) are not part of the taint path identified for this specific alert, and we leave them as is per the instructions not to change functionality unnecessarily.

No new imports or helper methods are needed; we only adjust the return value in the existing catch block.

Suggested changeset 1
src/modules/users/userAccount.ts
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/modules/users/userAccount.ts b/src/modules/users/userAccount.ts
--- a/src/modules/users/userAccount.ts
+++ b/src/modules/users/userAccount.ts
@@ -20,7 +20,7 @@
     } catch (e) {
       // eslint-disable-next-line no-console
       console.log('could not find customer', e)
-      return e
+      return { error: 'ACCOUNT_RETRIEVAL_FAILED' }
     }
   }
   return {}
EOF
@@ -20,7 +20,7 @@
} catch (e) {
// eslint-disable-next-line no-console
console.log('could not find customer', e)
return e
return { error: 'ACCOUNT_RETRIEVAL_FAILED' }
}
}
return {}
Copilot is powered by AI and may make mistakes. Always verify output.
} catch (error: any) {
// eslint-disable-next-line no-console
console.log(error)
res.send(false)
}
}

export const accountCreate = async (req: any, res: any) => {
req.body.id = req.user.id
try {
const data = await user.userAccountCreate(req.body)
res.send(data)
} catch (error: any) {
// eslint-disable-next-line no-console
console.log(error)
res.send(false)
}
}

export const accountCountries = async (req: any, res: any) => {
try {
const data = await user.userAccountCountries({ id: req.user.id })
res.send(data)
} catch (error: any) {
// eslint-disable-next-line no-console
console.log(error)
res.send(false)
}
}

export const accountBalance = async (req: any, res: any) => {
try {
const data = await user.userAccountBalance({ account_id: req.user.account_id })
res.send(data)
} catch (error: any) {
// eslint-disable-next-line no-console
console.log(error)
res.send(false)
}
}

export const accountUpdate = async (req: any, res: any) => {
try {
const data = await user.userAccountUpdate({ userParams: req.user, accountParams: req.body })
res.send(data)
} catch (error: any) {
// eslint-disable-next-line no-console
console.log('error on account update', error)
res.status(401).send(error)

Check warning

Code scanning / CodeQL

Information exposure through a stack trace Medium

This information exposed to the user depends on
stack trace information
Loading
.

Copilot Autofix

AI 6 months ago

In general, to fix information exposure through stack traces you should avoid sending raw exception objects or stack traces to clients. Instead, log the details on the server and return a generic, non-sensitive error response to the client (optionally with a simple error code/message that doesn’t reveal internal structure).

For this file, the minimal, non-breaking fix is to change accountUpdate (and, for consistency and security, accountDelete) so that they no longer send the raw error object. They should instead log the error (as they already do) and respond with a generic message or a simple boolean as done in the other handlers. To preserve existing semantics as much as possible, we can keep the same HTTP status code (401) but replace send(error) with a safe payload such as send(false) or a generic string like "Unauthorized" or "An error occurred while updating the account". Since the other endpoints respond with false on error, the most consistent choice is res.status(401).send(false).

Concretely:

  • In src/app/controllers/user/account.ts, in accountUpdate, replace res.status(401).send(error) with res.status(401).send(false).
  • In accountDelete, replace res.status(401).send(error) with res.status(401).send(false) to avoid the same pattern of exposing the raw error and to keep behavior consistent.
    No new imports or helper methods are required; we only change what is sent in the response.
Suggested changeset 1
src/app/controllers/user/account.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/app/controllers/user/account.ts b/src/app/controllers/user/account.ts
--- a/src/app/controllers/user/account.ts
+++ b/src/app/controllers/user/account.ts
@@ -52,7 +52,7 @@
   } catch (error: any) {
     // eslint-disable-next-line no-console
     console.log('error on account update', error)
-    res.status(401).send(error)
+    res.status(401).send(false)
   }
 }
 
@@ -63,6 +63,6 @@
   } catch (error: any) {
     // eslint-disable-next-line no-console
     console.log('error on account delete', error)
-    res.status(401).send(error)
+    res.status(401).send(false)
   }
 }
EOF
@@ -52,7 +52,7 @@
} catch (error: any) {
// eslint-disable-next-line no-console
console.log('error on account update', error)
res.status(401).send(error)
res.status(401).send(false)
}
}

@@ -63,6 +63,6 @@
} catch (error: any) {
// eslint-disable-next-line no-console
console.log('error on account delete', error)
res.status(401).send(error)
res.status(401).send(false)
}
}
Copilot is powered by AI and may make mistakes. Always verify output.

Check warning

Code scanning / CodeQL

Exception text reinterpreted as HTML Medium

Exception text
Loading
is reinterpreted as HTML without escaping meta-characters.

Copilot Autofix

AI 6 months ago

In general, the problem should be fixed by ensuring that exception text or error objects that may contain user input are not sent back to clients in a form that can be interpreted as HTML/JS. Instead, return a controlled, neutral error structure (e.g., a generic message and optional error code), and avoid echoing raw exception messages. If you must include details, send them only in safe contexts (logging, monitoring) or after proper encoding/sanitization.

For this codebase, the best minimal fix is to change the accountUpdate and accountDelete controllers so they no longer send the raw error object. Instead, log the full error server-side (as already done), and return a generic, structured JSON error payload such as { success: false, error: '...' } with an appropriate content type and HTTP status code. This keeps existing semantics (401 status, indication of failure) but removes user-controlled content from the HTTP body. We will modify:

  • src/app/controllers/user/account.ts, line 55: replace res.status(401).send(error) with res.status(401).json({ error: 'Could not update account' }) (or similar generic message).
  • src/app/controllers/user/account.ts, line 66: replace res.status(401).send(error) with res.status(401).json({ error: 'Could not delete account' }).

No changes are needed in src/modules/users/userAccountUpdate.ts for this specific sink; the core issue is the unsafe exposure of error in the controller. No new imports are required, as res.json is part of Express’s response API and already available.

Suggested changeset 1
src/app/controllers/user/account.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/app/controllers/user/account.ts b/src/app/controllers/user/account.ts
--- a/src/app/controllers/user/account.ts
+++ b/src/app/controllers/user/account.ts
@@ -52,7 +52,7 @@
   } catch (error: any) {
     // eslint-disable-next-line no-console
     console.log('error on account update', error)
-    res.status(401).send(error)
+    res.status(401).json({ error: 'Could not update account' })
   }
 }
 
@@ -63,6 +63,6 @@
   } catch (error: any) {
     // eslint-disable-next-line no-console
     console.log('error on account delete', error)
-    res.status(401).send(error)
+    res.status(401).json({ error: 'Could not delete account' })
   }
 }
EOF
@@ -52,7 +52,7 @@
} catch (error: any) {
// eslint-disable-next-line no-console
console.log('error on account update', error)
res.status(401).send(error)
res.status(401).json({ error: 'Could not update account' })
}
}

@@ -63,6 +63,6 @@
} catch (error: any) {
// eslint-disable-next-line no-console
console.log('error on account delete', error)
res.status(401).send(error)
res.status(401).json({ error: 'Could not delete account' })
}
}
Copilot is powered by AI and may make mistakes. Always verify output.
}
}

export const accountDelete = async (req: any, res: any) => {
try {
const data = await user.userAccountDelete({ userId: req.user.id })
res.send(data)
} catch (error: any) {
// eslint-disable-next-line no-console
console.log('error on account delete', error)
res.status(401).send(error)

Check warning

Code scanning / CodeQL

Information exposure through a stack trace Medium

This information exposed to the user depends on
stack trace information
Loading
.

Copilot Autofix

AI 6 months ago

In general, to fix this issue you should never send raw Error objects or stack traces to the client. Instead, log the full error on the server side and return a generic, non-sensitive error response to the user (for example, a boolean flag, a simple message, or a structured error code that does not include stack trace or internal implementation details).

For this specific file, the minimal and consistent fix is:

  • Keep logging the full error object to the server console for debugging (as the rest of the file already does).
  • Replace res.status(401).send(error) in accountDelete with a generic response, such as res.status(401).send(false) or a simple message like "Unable to delete account". To preserve existing behavior patterns in this file, it is best to respond with false, as done in account, accountCreate, accountCountries, and accountBalance.
  • No new imports or helper methods are required; we only adjust what is returned to the client.

Concretely:

  • In src/app/controllers/user/account.ts, lines 55–56 and 65–66 are currently sending the raw error. Update both accountUpdate and accountDelete to send a generic failure (false) instead:
    • Change res.status(401).send(error) to res.status(401).send(false) in both catch blocks.
Suggested changeset 1
src/app/controllers/user/account.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/app/controllers/user/account.ts b/src/app/controllers/user/account.ts
--- a/src/app/controllers/user/account.ts
+++ b/src/app/controllers/user/account.ts
@@ -52,7 +52,7 @@
   } catch (error: any) {
     // eslint-disable-next-line no-console
     console.log('error on account update', error)
-    res.status(401).send(error)
+    res.status(401).send(false)
   }
 }
 
@@ -63,6 +63,6 @@
   } catch (error: any) {
     // eslint-disable-next-line no-console
     console.log('error on account delete', error)
-    res.status(401).send(error)
+    res.status(401).send(false)
   }
 }
EOF
@@ -52,7 +52,7 @@
} catch (error: any) {
// eslint-disable-next-line no-console
console.log('error on account update', error)
res.status(401).send(error)
res.status(401).send(false)
}
}

@@ -63,6 +63,6 @@
} catch (error: any) {
// eslint-disable-next-line no-console
console.log('error on account delete', error)
res.status(401).send(error)
res.status(401).send(false)
}
}
Copilot is powered by AI and may make mistakes. Always verify output.
}
}
37 changes: 37 additions & 0 deletions src/app/controllers/user/bank-account.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import * as user from '../../../modules/users'

export const createBankAccount = async (req: any, res: any) => {
try {
const data = await user.userBankAccountCreate({
userParams: req.user,
bankAccountParams: req.body
})
res.send(data)
} catch (error: any) {
// eslint-disable-next-line no-console
console.log(error)
res.send(error)

Check warning

Code scanning / CodeQL

Information exposure through a stack trace Medium

This information exposed to the user depends on
stack trace information
Loading
.

Copilot Autofix

AI 6 months ago

In general, the fix is to avoid sending raw error/exception objects (including stack traces) to the client. Instead, log the detailed error on the server and return a generic, non-sensitive error message and appropriate HTTP status code (e.g., 500). This preserves debuggability while preventing information exposure.

For this file, the best low-impact fix is:

  • Keep console.log(error) so developers still see the full error on the server.
  • Change res.send(error) in all three catch blocks to a generic error response, e.g. res.status(500).send({ message: 'Internal server error' }) or similar.
  • Optionally, you can use res.status(500).json({ ... }) if JSON is the standard, but since the existing code uses res.send, we can keep that and only adjust the content and status code.
  • No new imports are required; we only modify the catch blocks.

Concretely:

  • In createBankAccount (around line 10–14), replace res.send(error) with res.status(500).send({ message: 'Internal server error' }).
  • In updateBankAccount (around line 21–25), perform the same replacement.
  • In userBankAccount (around line 32–36), perform the same replacement.

This does not change the functional success path, only the error payload and status code.

Suggested changeset 1
src/app/controllers/user/bank-account.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/app/controllers/user/bank-account.ts b/src/app/controllers/user/bank-account.ts
--- a/src/app/controllers/user/bank-account.ts
+++ b/src/app/controllers/user/bank-account.ts
@@ -10,7 +10,7 @@
   } catch (error: any) {
     // eslint-disable-next-line no-console
     console.log(error)
-    res.send(error)
+    res.status(500).send({ message: 'Internal server error' })
   }
 }
 
@@ -21,7 +21,7 @@
   } catch (error: any) {
     // eslint-disable-next-line no-console
     console.log(error)
-    res.send(error)
+    res.status(500).send({ message: 'Internal server error' })
   }
 }
 
@@ -32,6 +32,6 @@
   } catch (error: any) {
     // eslint-disable-next-line no-console
     console.log(error)
-    res.send(error)
+    res.status(500).send({ message: 'Internal server error' })
   }
 }
EOF
@@ -10,7 +10,7 @@
} catch (error: any) {
// eslint-disable-next-line no-console
console.log(error)
res.send(error)
res.status(500).send({ message: 'Internal server error' })
}
}

@@ -21,7 +21,7 @@
} catch (error: any) {
// eslint-disable-next-line no-console
console.log(error)
res.send(error)
res.status(500).send({ message: 'Internal server error' })
}
}

@@ -32,6 +32,6 @@
} catch (error: any) {
// eslint-disable-next-line no-console
console.log(error)
res.send(error)
res.status(500).send({ message: 'Internal server error' })
}
}
Copilot is powered by AI and may make mistakes. Always verify output.
}
}

export const updateBankAccount = async (req: any, res: any) => {
try {
const data = await user.userBankAccountUpdate({ userParams: req.user, bank_account: req.body })
res.send(data)
} catch (error: any) {
// eslint-disable-next-line no-console
console.log(error)
res.send(error)

Check warning

Code scanning / CodeQL

Information exposure through a stack trace Medium

This information exposed to the user depends on
stack trace information
Loading
.

Copilot Autofix

AI 6 months ago

In general, the fix is to avoid sending raw exception objects (and thus stack traces and internal details) in HTTP responses. Instead, log the detailed error on the server (for developers) and return a generic, user-safe error response with an appropriate HTTP status code and a non-sensitive message.

For this file, the best targeted fix without changing existing functional behavior too much is:

  • Keep logging the caught error with console.log(error) so developers retain visibility.
  • Replace res.send(error) with:
    • Set a reasonable HTTP status (e.g. 500) to indicate an internal server error.
    • Send a generic JSON payload or message, such as { message: 'Internal server error' } or similar.
  • Apply this pattern consistently in all three exported controller functions: createBankAccount, updateBankAccount, and userBankAccount.

Concretely, in src/app/controllers/user/bank-account.ts:

  • In createBankAccount, change lines 13–14 from res.send(error) to setting res.status(500) (or res.statusCode = 500) and sending a generic object, e.g. res.status(500).send({ message: 'Failed to create bank account' }).
  • In updateBankAccount, change line 24 similarly to send a generic error message (e.g. 'Failed to update bank account').
  • In userBankAccount, change line 35 similarly (e.g. 'Failed to fetch bank account').

No new imports are strictly necessary; we just use existing res methods that are standard in Express-style handlers.

Suggested changeset 1
src/app/controllers/user/bank-account.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/app/controllers/user/bank-account.ts b/src/app/controllers/user/bank-account.ts
--- a/src/app/controllers/user/bank-account.ts
+++ b/src/app/controllers/user/bank-account.ts
@@ -10,7 +10,7 @@
   } catch (error: any) {
     // eslint-disable-next-line no-console
     console.log(error)
-    res.send(error)
+    res.status(500).send({ message: 'Failed to create bank account' })
   }
 }
 
@@ -21,7 +21,7 @@
   } catch (error: any) {
     // eslint-disable-next-line no-console
     console.log(error)
-    res.send(error)
+    res.status(500).send({ message: 'Failed to update bank account' })
   }
 }
 
@@ -32,6 +32,6 @@
   } catch (error: any) {
     // eslint-disable-next-line no-console
     console.log(error)
-    res.send(error)
+    res.status(500).send({ message: 'Failed to fetch bank account' })
   }
 }
EOF
@@ -10,7 +10,7 @@
} catch (error: any) {
// eslint-disable-next-line no-console
console.log(error)
res.send(error)
res.status(500).send({ message: 'Failed to create bank account' })
}
}

@@ -21,7 +21,7 @@
} catch (error: any) {
// eslint-disable-next-line no-console
console.log(error)
res.send(error)
res.status(500).send({ message: 'Failed to update bank account' })
}
}

@@ -32,6 +32,6 @@
} catch (error: any) {
// eslint-disable-next-line no-console
console.log(error)
res.send(error)
res.status(500).send({ message: 'Failed to fetch bank account' })
}
}
Copilot is powered by AI and may make mistakes. Always verify output.
}
}

export const userBankAccount = async (req: any, res: any) => {
try {
const data = await user.userBankAccount({ id: req.user.id })
res.send(data)
} catch (error: any) {
// eslint-disable-next-line no-console
console.log(error)
res.send(error)

Check warning

Code scanning / CodeQL

Information exposure through a stack trace Medium

This information exposed to the user depends on
stack trace information
Loading
.

Copilot Autofix

AI 6 months ago

In general terms, the fix is to stop sending the raw error object (or stack trace) to the client and instead return a generic error message or structured error response that does not reveal internal details. Detailed error information should be logged on the server side only.

Concretely for src/app/controllers/user/bank-account.ts, we should change each catch block to:

  • Log the full error on the server (keeping console.log(error) is fine for now, though a logging framework would be better).
  • Set an appropriate HTTP status code (e.g., 500).
  • Send a generic JSON response (or at least a generic string) that does not include the raw error object or its stack trace.

To avoid changing existing functionality more than necessary, we will:

  • Keep the try logic and success res.send(data) calls intact.
  • Only modify the catch blocks to send a sanitized error response such as:
    • Set res.status(500) (or res.statusCode = 500 depending on style).
    • res.send({ message: 'Internal server error' }) (or similar stable message).

No new methods or imports are strictly required to implement this change; we will only adjust the lines inside the shown file.

Suggested changeset 1
src/app/controllers/user/bank-account.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/app/controllers/user/bank-account.ts b/src/app/controllers/user/bank-account.ts
--- a/src/app/controllers/user/bank-account.ts
+++ b/src/app/controllers/user/bank-account.ts
@@ -10,7 +10,7 @@
   } catch (error: any) {
     // eslint-disable-next-line no-console
     console.log(error)
-    res.send(error)
+    res.status(500).send({ message: 'Internal server error' })
   }
 }
 
@@ -21,7 +21,7 @@
   } catch (error: any) {
     // eslint-disable-next-line no-console
     console.log(error)
-    res.send(error)
+    res.status(500).send({ message: 'Internal server error' })
   }
 }
 
@@ -32,6 +32,6 @@
   } catch (error: any) {
     // eslint-disable-next-line no-console
     console.log(error)
-    res.send(error)
+    res.status(500).send({ message: 'Internal server error' })
   }
 }
EOF
@@ -10,7 +10,7 @@
} catch (error: any) {
// eslint-disable-next-line no-console
console.log(error)
res.send(error)
res.status(500).send({ message: 'Internal server error' })
}
}

@@ -21,7 +21,7 @@
} catch (error: any) {
// eslint-disable-next-line no-console
console.log(error)
res.send(error)
res.status(500).send({ message: 'Internal server error' })
}
}

@@ -32,6 +32,6 @@
} catch (error: any) {
// eslint-disable-next-line no-console
console.log(error)
res.send(error)
res.status(500).send({ message: 'Internal server error' })
}
}
Copilot is powered by AI and may make mistakes. Always verify output.
}
}
37 changes: 37 additions & 0 deletions src/app/controllers/user/customers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import * as user from '../../../modules/users'

export const customer = async (req: any, res: any) => {
if (!req.user) {
return res.status(401).json({ error: 'Unauthorized' })
}
try {
const data = await user.userCustomer({ id: req.user.id })
res.send(data)

Check warning

Code scanning / CodeQL

Information exposure through a stack trace Medium

This information exposed to the user depends on
stack trace information
Loading
.

Copilot Autofix

AI 6 months ago

In general, to fix this kind of issue we must avoid propagating raw exception objects from lower layers (like userCustomer) up to HTTP controllers that send them directly to clients. Instead, we should log detailed errors on the server and return either sanitized, generic error objects or simple flags that do not include stack traces or internal details.

For this specific code, the minimal, behavior‑preserving change is in src/modules/users/userCustomer.ts: within the inner catch (e) that wraps stripe.customers.retrieve, stop returning e and instead return a generic value such as false. The outer catch (error) already returns false on failure, and callers (like customer in customers.ts) already handle false as an error case in some paths, so this aligns with the existing pattern and keeps functionality consistent while eliminating the direct exposure of the exception object. No changes are necessary in customers.ts for the information‑exposure issue itself, since after the change the tainted error object no longer flows into res.send.

Concretely:

  • In src/modules/users/userCustomer.ts, lines 24–28:
    • Keep logging the detailed error on the server (console.log('could not find customer', e)).
    • Change return e to return false.
  • No new imports, methods, or definitions are needed.
Suggested changeset 1
src/modules/users/userCustomer.ts
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/modules/users/userCustomer.ts b/src/modules/users/userCustomer.ts
--- a/src/modules/users/userCustomer.ts
+++ b/src/modules/users/userCustomer.ts
@@ -24,7 +24,7 @@
       } catch (e) {
         // eslint-disable-next-line no-console
         console.log('could not find customer', e)
-        return e
+        return false
       }
     }
     return false
EOF
@@ -24,7 +24,7 @@
} catch (e) {
// eslint-disable-next-line no-console
console.log('could not find customer', e)
return e
return false
}
}
return false
Copilot is powered by AI and may make mistakes. Always verify output.
} catch (error: any) {
// eslint-disable-next-line no-console
console.log(error)
res.send(false)
}
}

export const customerCreate = async (req: any, res: any) => {
try {
const data = await user.userCustomerCreate(req.user.id, req.body)
res.send(data)

Check warning

Code scanning / CodeQL

Information exposure through a stack trace Medium

This information exposed to the user depends on
stack trace information
Loading
.

Copilot Autofix

AI 6 months ago

In general, to fix this type of issue you should never return a raw error object (which may include a stack trace and internal details) from a lower-level module to a controller that directly responds to the client. Instead, the lower-level module should either return a safe, structured error representation (for example, { success: false, code: 'something' }) or throw and let the controller map any errors to a generic, user-safe response. The controller itself should not send stack traces or raw Error instances to the client; it should send a generic error message or safe error code and log details on the server.

For this specific case, the minimal change that preserves existing behavior while removing the vulnerability is:

  • In userCustomerCreate:

    • Stop returning the raw error e from the inner catch. Instead, log the error and return a safe, generic error object (e.g., { error: 'could_not_find_customer' }) or a known sentinel value, consistent with existing returns.
    • This ensures the value that reaches the controller is no longer an Error object containing a stack trace.

  • In the customerCreate controller:

    • Instead of sending data blindly, respond with a more appropriate HTTP status and body for error cases. Since userCustomerCreate already returns false on outer errors and we will return a simple object in the inner case, the controller should normalize responses: on success send the Stripe customer or success result, on known error/error-like results send a generic failure response. At minimum, make sure that if data is an Error instance, it is not sent directly.

Given the constraint to avoid changing behavior more than needed and the fact we only see limited snippets, the safest, smallest fix is:

  • Change the return e in userCustomerCreate to return false (or another non-error sentinel) so the controller never sends a raw error object.
  • Optionally, for clarity and forward-safety, adjust customerCreate to distinguish success vs failure and send a clear, generic error response (e.g., res.status(500).json({ success: false })) instead of just false.

Concretely:

  • In src/modules/users/userCustomerCreate.ts, replace return e with return false.
  • In src/app/controllers/user/customers.ts, adjust customerCreate to:
    • Check data and, if it's falsy (or otherwise indicates failure), send a generic error response (res.status(500).json({ error: 'Internal server error' })) instead of echoing any error-like object.
    • Keep success behavior (res.send(data)) unchanged for non-error results.

No new methods or external libraries are strictly required; we only modify return values and HTTP response handling.

Suggested changeset 2
src/app/controllers/user/customers.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/app/controllers/user/customers.ts b/src/app/controllers/user/customers.ts
--- a/src/app/controllers/user/customers.ts
+++ b/src/app/controllers/user/customers.ts
@@ -17,11 +17,14 @@
 export const customerCreate = async (req: any, res: any) => {
   try {
     const data = await user.userCustomerCreate(req.user.id, req.body)
+    if (!data) {
+      return res.status(500).json({ success: false })
+    }
     res.send(data)
   } catch (error: any) {
     // eslint-disable-next-line no-console
     console.log(error)
-    res.send(false)
+    res.status(500).json({ success: false })
   }
 }
 
EOF
@@ -17,11 +17,14 @@
export const customerCreate = async (req: any, res: any) => {
try {
const data = await user.userCustomerCreate(req.user.id, req.body)
if (!data) {
return res.status(500).json({ success: false })
}
res.send(data)
} catch (error: any) {
// eslint-disable-next-line no-console
console.log(error)
res.send(false)
res.status(500).json({ success: false })
}
}

src/modules/users/userCustomerCreate.ts
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/modules/users/userCustomerCreate.ts b/src/modules/users/userCustomerCreate.ts
--- a/src/modules/users/userCustomerCreate.ts
+++ b/src/modules/users/userCustomerCreate.ts
@@ -21,7 +21,7 @@
       } catch (e) {
         // eslint-disable-next-line no-console
         console.log('could not find customer', e)
-        return e
+        return false
       }
     } else {
       const customer = await stripe.customers.create({
EOF
@@ -21,7 +21,7 @@
} catch (e) {
// eslint-disable-next-line no-console
console.log('could not find customer', e)
return e
return false
}
} else {
const customer = await stripe.customers.create({
Copilot is powered by AI and may make mistakes. Always verify output.
} catch (error: any) {
// eslint-disable-next-line no-console
console.log(error)
res.send(false)
}
}

export const customerUpdate = async (req: any, res: any) => {
try {
const data = await user.userCustomerUpdate(req.user.id, req.body)
res.send(data)

Check warning

Code scanning / CodeQL

Information exposure through a stack trace Medium

This information exposed to the user depends on
stack trace information
Loading
.

Copilot Autofix

AI 6 months ago

In general, the fix is to avoid returning raw exception objects from lower-level modules to the HTTP layer, and to avoid sending those exception objects directly in responses. Instead, log the detailed error on the server and return either a boolean, null, or a controlled error payload that does not expose stack trace or internal details.

The minimal change that preserves existing functionality is to change userCustomerUpdate so that it never returns the caught exception e. Instead, keep logging for debugging, but return false (which the controller already handles by sending false), or some other non-sensitive value. That way, customerUpdate can keep calling user.userCustomerUpdate and sending its result, but the result will no longer sometimes be an error object with a stack trace.

Concretely:

  • In src/modules/users/userCustomerUpdate.ts, update the inner catch (e) block (around line 24) so that it logs the error but returns false instead of e.
  • No change is strictly required in src/app/controllers/user/customers.ts, since once userCustomerUpdate stops returning the exception, res.send(data) will no longer expose stack trace information. The controller already has an outer try/catch that logs errors and sends false.

No new methods or imports are required; we only adjust the return value in the catch block.

Suggested changeset 1
src/modules/users/userCustomerUpdate.ts
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/modules/users/userCustomerUpdate.ts b/src/modules/users/userCustomerUpdate.ts
--- a/src/modules/users/userCustomerUpdate.ts
+++ b/src/modules/users/userCustomerUpdate.ts
@@ -24,7 +24,7 @@
       } catch (e) {
         // eslint-disable-next-line no-console
         console.log('could not find customer', e)
-        return e
+        return false
       }
     }
     return false
EOF
@@ -24,7 +24,7 @@
} catch (e) {
// eslint-disable-next-line no-console
console.log('could not find customer', e)
return e
return false
}
}
return false
Copilot is powered by AI and may make mistakes. Always verify output.

Check warning

Code scanning / CodeQL

Exception text reinterpreted as HTML Medium

Exception text
Loading
is reinterpreted as HTML without escaping meta-characters.

Copilot Autofix

AI 6 months ago

In general, to fix this kind of problem you should avoid returning raw exception objects to callers, especially when upstream parameters contain user input. Instead, log full errors on the server and return a controlled, non-reflective error structure (for example, { success: false } or a generic message/code) that does not echo any user-controlled content.

For this specific case, the minimal, non‑breaking change is:

  1. In userCustomerUpdate, stop returning the raw exception e. Return a simple, predictable error response instead (e.g., { error: 'stripe_update_failed' } or just false), which cannot contain user-controlled HTML.
  2. In the controller customerUpdate, continue to send whatever userCustomerUpdate returns, but now that will never be a raw exception object, avoiding the tainted error text from reaching res.send as a string/HTML.

This keeps existing calling patterns (controller still awaits user.userCustomerUpdate and res.send(data)), but removes the problematic data flow from user input → Stripe error → response body.

Concretely:

  • Modify src/modules/users/userCustomerUpdate.ts at the catch (e) in the inner try to return a safe value instead of return e.
  • Optionally (but not required for the taint issue), you could standardize error responses from this module; however, to keep changes minimal, we will only change that return.

No imports or new helper methods are required.

Suggested changeset 1
src/modules/users/userCustomerUpdate.ts
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/modules/users/userCustomerUpdate.ts b/src/modules/users/userCustomerUpdate.ts
--- a/src/modules/users/userCustomerUpdate.ts
+++ b/src/modules/users/userCustomerUpdate.ts
@@ -24,7 +24,7 @@
       } catch (e) {
         // eslint-disable-next-line no-console
         console.log('could not find customer', e)
-        return e
+        return false
       }
     }
     return false
EOF
@@ -24,7 +24,7 @@
} catch (e) {
// eslint-disable-next-line no-console
console.log('could not find customer', e)
return e
return false
}
}
return false
Copilot is powered by AI and may make mistakes. Always verify output.
} catch (error: any) {
// eslint-disable-next-line no-console
console.log(error)
res.send(false)
}
}
Loading
Loading