Skip to content
Merged
Show file tree
Hide file tree
Changes from 10 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
5 changes: 5 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ NEXT_PUBLIC_DISCORD_CLIENT_ID=your_discord_client_id
# then right-click your name → Copy User ID.
BOT_OWNER_IDS=your_discord_user_id

# ── Session Storage ──────────────────────────

# Redis URL for session storage (optional — falls back to in-memory)
# REDIS_URL=redis://localhost:6379

# ── Optional Integrations ────────────────────

# mem0 API key for user long-term memory (optional — memory features disabled without it)
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"discord.js": "^14.25.1",
"dotenv": "^17.3.1",
"express": "^5.2.1",
"ioredis": "^5.9.3",
"jsonwebtoken": "^9.0.3",
"mem0ai": "^2.2.2",
"node-pg-migrate": "^8.0.4",
Expand Down
61 changes: 61 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions src/api/middleware/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export function isValidSecret(secret) {
* @returns {import('express').RequestHandler} Express middleware function
*/
export function requireAuth() {
return (req, res, next) => {
return async (req, res, next) => {
// Try API secret first
const apiSecret = req.headers['x-api-secret'];
if (apiSecret) {
Expand All @@ -59,7 +59,7 @@ export function requireAuth() {
}

// Try OAuth2 JWT
if (handleOAuthJwt(req, res, next)) {
if (await handleOAuthJwt(req, res, next)) {
return;
}

Expand Down
4 changes: 2 additions & 2 deletions src/api/middleware/oauth.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { handleOAuthJwt } from './oauthJwt.js';
* @returns {import('express').RequestHandler} Express middleware function
*/
export function requireOAuth() {
return (req, res, next) => {
return handleOAuthJwt(req, res, next, { missingTokenError: 'No token provided' });
return async (req, res, next) => {
await handleOAuthJwt(req, res, next, { missingTokenError: 'No token provided' });
};
}
7 changes: 4 additions & 3 deletions src/api/middleware/oauthJwt.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ export function getBearerToken(authHeader) {
* @param {import('express').Response} res - Express response
* @param {import('express').NextFunction} next - Express next callback
* @param {{ missingTokenError?: string }} [options] - Behavior options
* @returns {boolean} True if middleware chain has been handled, false if no Bearer token was provided and no missing-token error was requested
* @returns {Promise<boolean>} True if middleware chain has been handled, false if no Bearer token
* was provided and no missing-token error was requested
*/
export function handleOAuthJwt(req, res, next, options = {}) {
export async function handleOAuthJwt(req, res, next, options = {}) {
const token = getBearerToken(req.headers.authorization);
if (!token) {
if (options.missingTokenError) {
Expand All @@ -37,7 +38,7 @@ export function handleOAuthJwt(req, res, next, options = {}) {
return false;
}

const result = verifyJwtToken(token);
const result = await verifyJwtToken(token);
if (result.error) {
if (result.status === 500) {
error('SESSION_SECRET not configured — cannot verify OAuth token', {
Expand Down
23 changes: 16 additions & 7 deletions src/api/middleware/verifyJwt.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,25 +34,34 @@ function getSecret() {
* Verify a JWT token and validate the associated server-side session.
*
* @param {string} token - The JWT Bearer token to verify
* @returns {{ user: Object } | { error: string, status: number }}
* @returns {Promise<{ user: Object } | { error: string, status: number }>}
* On success: `{ user }` with the decoded JWT payload.
* On failure: `{ error, status }` with an error message and HTTP status code.
*/
export function verifyJwtToken(token) {
export async function verifyJwtToken(token) {
let secret;
try {
secret = getSecret();
} catch {
return { error: 'Session not configured', status: 500 };
}

let decoded;
try {
const decoded = jwt.verify(token, secret, { algorithms: ['HS256'] });
if (!getSessionToken(decoded.userId)) {
return { error: 'Session expired or revoked', status: 401 };
}
return { user: decoded };
decoded = jwt.verify(token, secret, { algorithms: ['HS256'] });
} catch {
return { error: 'Invalid or expired token', status: 401 };
}

let sessionToken;
try {
sessionToken = await getSessionToken(decoded.userId);
} catch {
return { error: 'Session lookup failed', status: 503 };
}

if (!sessionToken) {
return { error: 'Session expired or revoked', status: 401 };
}
return { user: decoded };
}
Comment on lines +42 to 69
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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for jwt.sign calls to see what payloads are being signed
rg -n "jwt\.sign\(" --type js -A 3 -B 1

Repository: VolvoxLLC/volvox-bot

Length of output: 1739


🏁 Script executed:

#!/bin/bash
# Search for jwt.verify calls to understand usage patterns
rg -n "jwt\.verify\(" --type js -B 2 -A 5

Repository: VolvoxLLC/volvox-bot

Length of output: 1739


🏁 Script executed:

#!/bin/bash
# Check the file structure and identify jwt import statements
rg -n "import.*jwt|require.*jwt" --type js

Repository: VolvoxLLC/volvox-bot

Length of output: 1739


🌐 Web query:

jsonwebtoken library jwt.verify() return type - when does it return a string vs object

💡 Result:

In jsonwebtoken (auth0/node-jsonwebtoken), jwt.verify() returns different shapes based on (a) what was signed as the payload and (b) whether you pass complete: true:

  • Returns a string when the token was signed with a string (or Buffer) payload. jwt.sign() explicitly allows payload to be an “object literal, buffer or string”, and non-object payloads aren’t treated as claim objects. (github.com)
  • Returns an object (i.e., the decoded JSON payload) when the token was signed with an object literal payload. (github.com)
  • Returns an object { payload, header, signature } if you call jwt.verify(..., { complete: true })—and in that case, payload inside that object can still be string or object depending on what was signed. (github.com)

Citations:


Guard decoded JWT payload shape before accessing decoded.userId to prevent TypeError on non-object returns.

According to jsonwebtoken's specification, jwt.verify() returns a string (or Buffer) if the token was signed with a string/Buffer payload, and an object only if signed with an object literal. The current code assumes decoded is always an object and accesses decoded.userId without validation. If a string is returned, this causes a TypeError that gets caught nowhere and produces a 500 error instead of the intended 401.

Suggested patch
 export async function verifyJwtToken(token) {
@@
   let decoded;
   try {
     decoded = jwt.verify(token, secret, { algorithms: ['HS256'] });
   } catch {
     return { error: 'Invalid or expired token', status: 401 };
   }
 
+  if (!decoded || typeof decoded !== 'object' || typeof decoded.userId !== 'string') {
+    return { error: 'Invalid or expired token', status: 401 };
+  }
+  const userId = decoded.userId;
+
   let sessionToken;
   try {
-    sessionToken = await getSessionToken(decoded.userId);
+    sessionToken = await getSessionToken(userId);
   } catch (err) {
-    logError('Session lookup failed', { error: err.message, userId: decoded.userId });
+    const errMsg = err instanceof Error ? err.message : String(err);
+    logError('Session lookup failed', { error: errMsg, userId });
     return { error: 'Session lookup failed', status: 503 };
   }
@@
   if (!sessionToken) {
     return { error: 'Session expired or revoked', status: 401 };
   }
   return { user: decoded };
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/middleware/verifyJwt.js` around lines 42 - 69, The verifyJwtToken
function assumes jwt.verify(token, secret, ...) returns an object with userId;
first validate that decoded is an object and has a userId (e.g., typeof decoded
=== 'object' && decoded !== null && 'userId' in decoded) before using
decoded.userId, and if that check fails return { error: 'Invalid or expired
token', status: 401 }; update the block around jwt.verify and the subsequent
getSessionToken usage to use this guard so you never call getSessionToken or
access decoded.userId on a string/Buffer or null value, and keep the existing
error paths (500 for missing secret, 401 for invalid token, 503 for session
lookup failure).

23 changes: 19 additions & 4 deletions src/api/routes/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ router.get('/discord/callback', async (req, res) => {
}

// Store access token server-side (never in the JWT)
sessionStore.set(user.id, accessToken);
await sessionStore.set(user.id, accessToken);

// Create JWT with user info only (no access token — stored server-side)
const token = jwt.sign(
Expand Down Expand Up @@ -269,7 +269,14 @@ router.get('/discord/callback', async (req, res) => {
*/
router.get('/me', requireOAuth(), async (req, res) => {
const { userId, username, avatar } = req.user;
const accessToken = sessionStore.get(userId);

let accessToken;
try {
accessToken = await sessionStore.get(userId);
} catch (err) {
error('Redis error fetching session in /me', { error: err.message, userId });
return res.status(503).json({ error: 'Session store unavailable' });
}

let guilds = [];
if (accessToken) {
Expand All @@ -291,8 +298,16 @@ router.get('/me', requireOAuth(), async (req, res) => {
/**
* POST /logout — Invalidate the user's server-side session
*/
router.post('/logout', requireOAuth(), (req, res) => {
sessionStore.delete(req.user.userId);
router.post('/logout', requireOAuth(), async (req, res) => {
try {
await sessionStore.delete(req.user.userId);
} catch (err) {
error('Redis error deleting session on logout', {
error: err.message,
userId: req.user.userId,
});
// User's intent is to log out — succeed anyway
}
res.json({ message: 'Logged out successfully' });
});

Expand Down
36 changes: 27 additions & 9 deletions src/api/routes/guilds.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,14 +172,23 @@ function formatBucketLabel(bucket, interval) {
* @returns {boolean} `true` if the user has any of the specified permission flags on the guild, `false` otherwise.
*/
async function hasOAuthGuildPermission(user, guildId, anyOfFlags) {
const accessToken = getSessionToken(user?.userId);
if (!accessToken) return false;
const guilds = await fetchUserGuilds(user.userId, accessToken);
const guild = guilds.find((g) => g.id === guildId);
if (!guild) return false;
const permissions = Number(guild.permissions);
if (Number.isNaN(permissions)) return false;
return (permissions & anyOfFlags) !== 0;
try {
const accessToken = await getSessionToken(user?.userId);
if (!accessToken) return false;
const guilds = await fetchUserGuilds(user.userId, accessToken);
const guild = guilds.find((g) => g.id === guildId);
if (!guild) return false;
const permissions = Number(guild.permissions);
if (Number.isNaN(permissions)) return false;
return (permissions & anyOfFlags) !== 0;
} catch (err) {
error('Redis error in hasOAuthGuildPermission', {
error: err.message,
userId: user?.userId,
guildId,
});
throw err;
}
Comment on lines 174 to +191
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

Harden error logging: don’t assume err is an Error (and consider “session store” wording).

Both catch blocks log err.message. If a non-Error is ever thrown/rejected, the logger call can throw and you lose the original context.

Suggested patch
 async function hasOAuthGuildPermission(user, guildId, anyOfFlags) {
   try {
     const accessToken = await getSessionToken(user?.userId);
     if (!accessToken) return false;
@@
     return (permissions & anyOfFlags) !== 0;
   } catch (err) {
+    const errMsg = err instanceof Error ? err.message : String(err);
     error('Error in hasOAuthGuildPermission (session lookup or guild fetch)', {
-      error: err.message,
+      error: errMsg,
       userId: user?.userId,
       guildId,
     });
     throw err;
   }
 }
@@
     try {
       accessToken = await getSessionToken(req.user?.userId);
     } catch (err) {
+      const errMsg = err instanceof Error ? err.message : String(err);
-      error('Redis error fetching session token in GET /guilds', {
-        error: err.message,
+      error('Session store error fetching session token in GET /guilds', {
+        error: errMsg,
         userId: req.user?.userId,
       });
       return res.status(503).json({ error: 'Session store unavailable' });
     }

Also applies to: 319-328

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/api/routes/guilds.js` around lines 174 - 191, The catch block in
hasOAuthGuildPermission (and the similar catch at lines 319-328) assumes err is
an Error and accesses err.message, which can throw if a non-Error is thrown;
update both catch handlers to safely stringify the error (e.g. use String(err)
or a safe serializer) and log that instead of err.message, and adjust the log
text to refer to "session store" when describing session lookup failures so the
message is accurate (targets: function hasOAuthGuildPermission and the
corresponding function around lines 319-328).

}

/**
Expand Down Expand Up @@ -307,7 +316,16 @@ router.get('/', async (req, res) => {
return res.json(ownerGuilds);
}

const accessToken = getSessionToken(req.user?.userId);
let accessToken;
try {
accessToken = await getSessionToken(req.user?.userId);
} catch (err) {
error('Redis error fetching session token in GET /guilds', {
error: err.message,
userId: req.user?.userId,
});
return res.status(503).json({ error: 'Session store unavailable' });
}
if (!accessToken) {
return res.status(401).json({ error: 'Missing access token' });
}
Expand Down
5 changes: 3 additions & 2 deletions src/api/routes/health.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,9 @@ router.get('/', async (req, res) => {
const pool = getRestartPool();
if (pool) {
const rows = await getRestarts(pool, 20);
body.restarts = rows.map(r => ({
timestamp: r.timestamp instanceof Date ? r.timestamp.toISOString() : String(r.timestamp),
body.restarts = rows.map((r) => ({
timestamp:
r.timestamp instanceof Date ? r.timestamp.toISOString() : String(r.timestamp),
reason: r.reason || 'unknown',
version: r.version ?? null,
uptimeBefore: r.uptime_seconds ?? null,
Expand Down
Loading
Loading