Skip to content

Security

Findings (this standalone repo)

F-1: Hardcoded fallback session secret (low)

In server.js:

secret: process.env.SESSION_SECRET || "change-me",

If SESSION_SECRET is not set, the literal string "change-me" becomes the cookie-signing secret. The session middleware is currently unused by the handlers (nothing reads or writes req.session), so the practical impact is low. But: if a future handler starts using sessions, this fallback will silently allow trivially-forgeable cookies in deployments that forgot to set the secret.

Fix:

Preferred path — remove the unused middleware entirely. Nothing in routes/index.js reads or writes req.session, so express-session is dead weight.

  1. Delete the import and the app.use(expressSession(...)) block in server.js:4 and server.js:8-12:
    // server.js — delete these lines
    const expressSession = require('express-session')   // line 4
    app.use(expressSession({                             // lines 8-12
        secret: process.env.SESSION_SECRET || "change-me",
        resave: false,
        saveUninitialized: true
    }))
    
  2. Remove express-session from package.json:11 (npm uninstall express-session).

Fallback path — if a future handler will use sessions, keep the middleware but fail fast on a missing secret. Replace server.js:8-12 with:

if (!process.env.SESSION_SECRET) {
    throw new Error("SESSION_SECRET is required");
}
app.use(expressSession({
    secret: process.env.SESSION_SECRET,
    resave: false,
    saveUninitialized: false,   // do not create sessions for anonymous requests
    cookie: { httpOnly: true, sameSite: "lax", secure: process.env.NODE_ENV !== "development" }
}))
3. Add SESSION_SECRET to the deployment env (a random 32+ byte value, e.g. openssl rand -hex 32). Never commit it.

Note: the in-process production copy in someli-api/dashboard/ does not boot its own Express app (it re-enters the parent via appData), so this server.js-level fix applies to this standalone repo only.

F-2: Auth-by-convention (medium)

In production mode, handlers read res.userId and res.accountId as if they're always set — but the auth that sets them lives in the parent someli-api and is mounted before the dashboard router. If the dashboard router is ever mounted in a context where that upstream auth is absent (for example, if someone extracts this repo into a standalone service per Path B in relationship-to-someli-api.md), the handlers will run with userId === undefined and accountId === undefined.

In SQL terms, this would result in queries like:

WHERE accountId = ?  -- bound to undefined → likely an error or empty result
which would fail safely (no data returned) — but the behaviour isn't intentional.

Fix:

  1. Add a single in-router guard at the top of routes/index.js, immediately after the router is created (routes/index.js:24) and before the first route (routes/index.js:31). This runs in both modes (production and the local mock, which already sets res.userId/res.accountId in mock/middlewares/auth.js:16-19):
    // routes/index.js — after `const router = require("express").Router();` (line 24)
    router.use((req, res, next) => {
      if (!res.userId || !res.accountId) {
        return res.status(401).json({ status: false, code: 401, message: 'unauthenticated' });
      }
      next();
    });
    
  2. Keep it above the existing if (isLocal) { router.use(auth); } line (routes/index.js:26-28) is fine either way — the mock auth sets the values before this guard runs because middleware executes in registration order, so register the mock auth first, then this guard. Re-order to:
    if (isLocal) {
      router.use(auth);
    }
    router.use((req, res, next) => { /* guard above */ });
    
  3. This is defensive but cheap; it does not replace the upstream auth, it only fails closed if that auth is ever absent.

Note: the canonical in-process copy in someli-api/dashboard/ relies on the parent's auth middleware being mounted first. If you want the same belt-and-braces guard there, add the identical router.use block to that copy too — the two have drifted and a fix here does not propagate.

F-3: Mock auth would be deployable (low-likelihood, high-impact if it happens)

In local-dev mode, mock/middlewares/auth.js sets fake userId / accountId and never validates anything. If this repo is ever accidentally deployed with NODE_ENV=development, every request would succeed as a fake user with hardcoded privileges.

Fix:

  1. Gate the mock branch on an explicit opt-in flag. In routes/index.js, the isLocal switch is decided at line 1 (const isLocal = process.env.NODE_ENV === "development";) and the mock modules are wired in at lines 9-14. Add a guard before the mock requires so a stray NODE_ENV=development in prod fails loudly instead of silently loading fake auth:
    // routes/index.js — replace line 1
    const isLocal = process.env.NODE_ENV === "development";
    if (isLocal && process.env.ALLOW_MOCK_AUTH !== 'yes') {
      throw new Error('Mock auth disabled. Set ALLOW_MOCK_AUTH=yes only in local dev.');
    }
    
  2. Document ALLOW_MOCK_AUTH=yes in the local-dev .env example / README and ensure it is never set in any deployed environment.
  3. As a second layer, set NODE_ENV=production explicitly in the production process manager (PM2/systemd env) so the default cannot drift to development.
  4. Optionally, in mock/middlewares/auth.js:4 (top of the exported middleware) add a runtime assertion if (process.env.NODE_ENV !== 'development') throw new Error('mock auth loaded outside development'); so the fake auth refuses to run even if reached.

Note: this concerns the standalone repo only — the in-process copy in someli-api/dashboard/ has no mock branch and uses the real auth, so no parallel change is needed there.

F-4: Inherited security posture of sibling repos

Because the production copy lives inside someli-api, it inherits everything in ../someli-api/security.md — JWT in encrypted token header, apptype base64 header, sync MySQL with parameterised queries (mostly — see the SQL injection risks documented in someli-api/security.md), CORS wildcard, etc. Re-reading that doc is recommended.

Separately, the standalone server.js in this repo ships with no baseline HTTP hardening at all: no helmet(), no CORS policy, and no rate limiting on its /dashboard/* routes. When this repo is run on its own (local dev, or if extracted to a service per Path B) those gaps are real.

Fix: add baseline middleware to server.js for the standalone process. After const app = express(); (server.js:6) and before the router is mounted (server.js:14):

  1. Security headers and CORS allowlist:
    npm install helmet cors express-rate-limit
    
    // server.js — after line 6
    const helmet = require("helmet");
    const cors = require("cors");
    const rateLimit = require("express-rate-limit");
    
    app.use(helmet());
    
    const allowedOrigins = (process.env.CORS_ORIGINS || "").split(",").filter(Boolean);
    app.use(cors({
      origin: (origin, cb) => {
        if (!origin || allowedOrigins.includes(origin)) return cb(null, true);
        return cb(new Error("origin not allowed"));
      },
      credentials: true,
    }));
    
  2. Rate-limit the dashboard routes (they are all read-only analytics, so a modest per-IP cap is safe):
    // server.js — before app.use("/dashboard", router) on line 14
    app.use("/dashboard", rateLimit({ windowMs: 60_000, max: 60, standardHeaders: true, legacyHeaders: false }));
    
  3. Provide CORS_ORIGINS (comma-separated, e.g. the platform/admin app origins) via process.env; never use a * wildcard with credentials: true.
  4. The canonical in-process copy in someli-api/dashboard/ does not own its own Express app — these headers/CORS/rate-limit settings are configured at the someli-api server.js level. Apply the equivalent hardening there per ../someli-api/security.md; the two do not share this file.

F-5: Spurious dependencies

The package.json declares jsonwebtoken, crypto-js, request, node-cron, path — none of which are require'd by any file in this repo (mock/middlewares/auth.js:1 does require("jsonwebtoken") but never calls it, and conf.js:1/mock/routes.js/routes/index.js only use the built-in path/moment/sync-mysql, not the npm path clone). Each declared-but-unused dependency is unnecessary attack surface, especially request (officially deprecated since 2020) and path (the npm path package is a third-party clone of a built-in module, last published in 2015).

Fix:

  1. Confirm nothing imports them, then prune:
    # from the repo root — verify zero real usages first
    grep -rn "require(['\"]\(jsonwebtoken\|crypto-js\|request\|node-cron\|path\)['\"]" \
      --include='*.js' --exclude-dir=node_modules .
    
    The npm path package shadows the Node built-in; since conf.js:1 relies on the built-in path, removing the npm package is safe and actually restores the intended built-in resolution.
  2. Remove the dead jsonwebtoken import line in mock/middlewares/auth.js:1 (it is never used), then uninstall:
    npm uninstall jsonwebtoken crypto-js request node-cron path
    
  3. Run npm install / commit the updated package.json (lines 8-16) and lockfile, then npm audit to confirm the removed transitive vulnerabilities (notably request's) are gone.
  4. This pruning applies to the standalone repo's package.json only; the in-process copy under someli-api/dashboard/ shares someli-api's own package.json, so audit that dependency set separately.

What's not a finding here

  • SQL injection — the con.query(...) calls in routes/index.js use parameterised arguments (the second arg is an array). They are mostly safe. A spot-check at line ~38 confirms con.query(\… WHERE accountId = ? AND ...`, [selaccountId])`. Spot-check the additions for the same property.

Fix (defensive — keep the queries parameterised as new ones are added): the con.query(... , [selaccountId]) lookups against tAccountMembers (e.g. routes/index.js:42, :124, :268, :507, :581, :777, :902) are the parameterised, safe ones. The larger analytics queries built with template literals interpolate values directly (e.g. ma.accountId = ${selaccountId}, ma.member_Id = ${userId}, ma.provider_Id IN (${providerIds.toString()}), ORDER BY SUM(i.${sortBy}), WHERE postId IN (${cpIds.join(',')}) at routes/index.js:79, :82, :78, :341, :352). Today the interpolated values come from auth-set numerics (userId, accountId) and from JSON.parse'd arrays cast to numbers, so the practical exposure is limited — but keep it that way: 1. For every new analytics handler, pass scalar inputs as bound parameters rather than interpolating, matching the existing con.query(\… = ?`, [value])pattern. 2. ForIN (...)lists, coerce to integers before joining, e.g.const ids = providerIds.map(Number).filter(Number.isInteger);then buildIN (${ids.join(',')})— never interpolate rawreq.params. 3. WhitelistORDER BYcolumns (thesortByswitch atroutes/index.js:248-261already does this — preserve that allow-list approach and never let a column/direction come straight from input). 4. Apply the same discipline to the canonical in-process copy insomeli-api/dashboard/`; the two files have drifted, so a hardening fix here does not propagate. - XSS / SSRF — no user input is rendered or forwarded to external systems by this repo. - CSRF — endpoints are all GET; no state-changing operations. - CSP / X-Frame-Options — not this repo's concern (responses are JSON, not HTML).