Skip to content

Security

Findings

F-1: Hardcoded session secret (HIGH)

server.js:11:

app.use(expressSession({
    secret: "3eB(2:\\srlI+qa5",
    resave: false,
    saveUninitialized: true
}))

The literal string 3eB(2:\srlI+qa5 is the cookie-signing key for every deployment — dev, uat, prod. This means:

  1. Anyone who reads the repo (and the repo is in the company's GitHub org — many engineers, vendors, contractors have access) can forge session cookies for any environment.
  2. The secret cannot be rotated without a code change.
  3. Even though the handlers currently don't use req.session, the session cookie is still issued to every client; it could be replayed.

Fix:

  1. Add SESSION_SECRET (and optionally SESSION_NAME) to conf.js (after line 48) so it is sourced from the environment alongside the other secrets:
    SESSION_SECRET: process.env.SESSION_SECRET,
    SESSION_NAME: process.env.SESSION_NAME, // optional; default below
    
  2. Generate a fresh random secret (node -e "console.log(require('crypto').randomBytes(48).toString('base64'))") and set a different value per environment in each .env (this rotates the leaked literal — the old 3eB(2:\srlI+qa5 is now compromised and must never be reused).
  3. Replace server.js:11-15 with an env-backed config that fails fast and rotates the cookie name:
    if (!conf.SESSION_SECRET) throw new Error("SESSION_SECRET is not set");
    app.use(expressSession({
        name: conf.SESSION_NAME || "someli.admin.sid", // rotated from default connect.sid → invalidates old cookies
        secret: conf.SESSION_SECRET,
        resave: false,
        saveUninitialized: false, // don't issue a cookie until something is stored in the session
        cookie: { httpOnly: true, secure: true, sameSite: "lax" }
    }))
    
  4. Setting saveUninitialized: false (was true at server.js:14) stops a signed cookie being issued to every client when no handler uses req.session, removing the replay surface noted above.
  5. Purge the literal from git history (see F-10 step on git filter-repo) so the rotated secret is not undermined by the old value remaining in old commits.

F-2: Hardcoded Slack bot token (HIGH)

routes/auth.js:20-21:

const token = 'xoxb-3144030948916-5918653901701-lKq16vALrV1AqVuTjs9lYNkZ'
var channel = 'C05TS9AHBH6'

A live Slack bot token is hardcoded in source. Anyone with repo read access can post to that Slack channel — or worse, depending on the bot's scopes, perform other workspace actions.

Fix:

  1. Rotate immediately (highest priority): in the Slack admin / api.slack.com app config, revoke xoxb-3144030948916-5918653901701-lKq16vALrV1AqVuTjs9lYNkZ and reinstall the app to mint a new bot token. The leaked value is unrecoverable once it has been in the repo.
  2. Add SLACK_BOT_TOKEN and SLACK_CHANNEL_ID to conf.js (after line 48):
    SLACK_BOT_TOKEN: process.env.SLACK_BOT_TOKEN,
    SLACK_CHANNEL_ID: process.env.SLACK_CHANNEL_ID,
    
    and put the new (rotated) token + C05TS9AHBH6 in each environment's .env.
  3. Replace the literals at routes/auth.js:20-21 and the client init at routes/auth.js:25:
    const token = conf.SLACK_BOT_TOKEN
    var channel = conf.SLACK_CHANNEL_ID
    // ...
    const bot = new Slack({ token })
    
    (conf is already imported at routes/auth.js:11.)
  4. Purge the literal from git history (see F-10) so the old token is not retrievable from old commits even after rotation.
  5. Audit the bot's OAuth scopes in the Slack app config and reduce to the minimum needed (chat:write for the notification use case here) so a future leak is lower-impact.

F-3: In-memory token revocation (MEDIUM)

middlewares/auth.js calls revokedTokens.has(token) where revokedTokens is a Set. The set is in-process memory only. Implication:

  • Process restart → all revoked tokens become valid again
  • Multi-replica deploy → revocation on replica A doesn't apply on replica B
  • Since admin tokens have no expiry encoded in them (per authentication.md), a stolen token is effectively long-lived

Fix:

  1. Create a persistent revocation table (MySQL):
    CREATE TABLE tRevokedToken (
      token      VARCHAR(512) NOT NULL,
      revoked_at DATETIME     NOT NULL DEFAULT CURRENT_TIMESTAMP,
      expires_at DATETIME     NULL,
      PRIMARY KEY (token(255)),
      INDEX idx_expires (expires_at)
    );
    
  2. Replace the in-memory Set in helper/revokeToken.js:1-3 with insert/lookup helpers backed by the shared con/mysql2 pool, using parameterised queries:
    const con = require('../routes/routes').con;
    exports.revoke = (token, expiresAt) =>
      con.query('INSERT IGNORE INTO tRevokedToken (token, expires_at) VALUES (?, ?)', [token, expiresAt]);
    exports.isRevoked = (token) =>
      new Promise((resolve) =>
        con.query('SELECT 1 FROM tRevokedToken WHERE token = ? LIMIT 1', [token],
          (e, rows) => resolve(!e && rows.length > 0)));
    
  3. Update middlewares/auth.js:12 to await/check the DB result instead of revokedTokens.has(token) (make the middleware async):
    if (await revokedTokens.isRevoked(token)) return res.status(401).send("unauthorized access");
    
  4. Add a periodic cleanup (cron/job) DELETE FROM tRevokedToken WHERE expires_at < NOW() so the table does not grow unbounded — only tokens still within their (newly-added, see F-4) expiry window need to be retained.
  5. This survives process restarts and is shared across replicas, closing both gaps above.

F-4: Token has no expiry (MEDIUM)

The encrypted-token format produced by helper/tokenGenerator.js (verify) does not include an expiration claim. The middlewares/auth.js decryption succeeds as long as the AES key and ciphertext match, regardless of age.

Fix:

  1. helper/tokenGenerator.js:8 already passes expiresIn: expiresIn ?? JWT_EXPIRES_IN to jwt.sign, but JWT_EXPIRES_IN is not present in conf.js and is read straight from process.env (helper/tokenGenerator.js:3). When it is unset, expiresIn is undefined, so jwt.sign mints a non-expiring token. Set it explicitly: add JWT_EXPIRES_IN=24h (pick an appropriate window) to each .env, and add a safe fallback so an unset env can never produce a perpetual token:
    let { JWT_SECRET_KEY, JWT_EXPIRES_IN } = process.env
    const DEFAULT_EXPIRES_IN = JWT_EXPIRES_IN || '24h'
    // ...
    let token = jwt.sign({ data: ciphertext }, JWT_SECRET_KEY, { expiresIn: expiresIn ?? DEFAULT_EXPIRES_IN })
    
  2. decryptData at helper/tokenGenerator.js:13 uses jwt.verify, which already throws TokenExpiredError once exp is set — so once step 1 lands, expired tokens are rejected automatically.
  3. Confirm middlewares/auth.js treats a thrown verify error as a 401: it does (the try/catch at middlewares/auth.js:6,29-33 returns unauthorized access), so no change is needed there once expiry is set.
  4. For belt-and-braces (defence in depth against exp being stripped), also embed { ..., issuedAt, expiresAt } inside the AES-encrypted payload built at the encryptData call sites (routes/routes.js:175, routes/routes.js:252) and re-check Date.now() < expiresAt after decryption in middlewares/auth.js.

F-5: CORS wildcard (MEDIUM)

server.js does app.use(cors()) with no options — that means Access-Control-Allow-Origin: *. Combined with token-in-header authentication, the wildcard allows any origin's JavaScript to call the API as long as it knows a valid token. This is a defence-in-depth issue rather than an exploit on its own, but the admin API should restrict origins to known FE hosts (e.g., https://admin.someli.ai).

Fix:

  1. Add an allowlist to conf.js so origins are environment-specific:
    CORS_ORIGINS: process.env.CORS_ORIGINS, // comma-separated, e.g. "https://admin.someli.ai,http://localhost:8080"
    
  2. Replace the bare app.use(cors()) at server.js:75 with an allowlist (the admin FE dev server runs on port 8080 per the repo notes):
    const allowed = (conf.CORS_ORIGINS || 'https://admin.someli.ai,http://localhost:8080').split(',');
    app.use(cors({
        origin: (origin, cb) => cb(null, !origin || allowed.includes(origin)),
        credentials: true
    }));
    
  3. The cors package is already a dependency (package.json:29) — no install needed.
  4. Move this app.use(cors()) so it is registered before the route mounts (server.js:77-82); it currently sits at line 75, which is fine, but verify it precedes both authRoute and Api.init().

F-6: Webhook body-parser exemption with no handlers (LOW)

server.js:19-25 exempts /stripe_webhooks, /paddle_sandbox_webhooks, /paddle_production_webhooks from JSON parsing, but no handler exists for those paths. Any request to them will 404. Not exploitable, but a misleading signal — verify whether the exemption is needed.

Fix (pick one):

  1. If vestigial (no webhooks are meant to land on the admin API): delete the if branch at server.js:20-22 and apply express.json({ limit: '150mb', ... }) unconditionally (this also lets you tighten the limit per F-9). Confirm first that no Paddle/Stripe dashboard config points a webhook at this service.
  2. If planned (webhooks should be handled here): keep the raw-body exemption but actually mount handlers that verify signatures before trusting the payload. The raw body is needed precisely for that verification, so register the route with express.raw():
    // mount alongside the other routes near server.js:77
    app.post('/paddle_production_webhooks', express.raw({ type: '*/*' }), (req, res) => {
      const ok = verifyPaddleSignature(req.headers['paddle-signature'], req.body, conf.PADDLE_WEBHOOK_KEY); // HMAC over raw body
      if (!ok) return res.status(401).send('invalid signature');
      const event = JSON.parse(req.body.toString('utf8'));
      // ...handle event...
      res.sendStatus(200);
    });
    
    PADDLE_WEBHOOK_KEY / PADDLE_TEST_WEBHOOK_KEY are already exposed in conf.js:40,46; use the Stripe equivalent (stripe.webhooks.constructEvent) for the Stripe path. Never parse/act on the body before the signature check passes.
  3. Either way, document the decision in the route file so the next engineer does not re-add a dangling exemption.

F-7: Role gating done in SQL only (MEDIUM)

There is no requireRole(...) middleware. Each endpoint enforces role by SQL WHERE filters. A new endpoint added by a future engineer who forgets to filter is a privilege-escalation. Examples to audit: routes/auth.js's getAllOwnerList (routes/auth.js:74) builds usersFilter from the role query string (routes/auth.js:93-102) — it filters by role_type = 11 only when role=test and by role_type = 5 for other values, but when role=all (routes/auth.js:98-99) usersFilter is '', returning everything (which may be correct for admins but should be explicit).

Fix:

  1. Add a requireRole middleware (e.g. middlewares/requireRole.js). middlewares/auth.js:18-19 already sets res.role from the decrypted token, so the gate can read it directly:
    module.exports = (allowed) => (req, res, next) => {
      if (!allowed.includes(Number(res.role))) return res.status(403).send("forbidden");
      next();
    };
    
    (Use the numeric role_type IDs — SUPER_ADMIN, ADMIN, ACCOUNT_MANAGER, USER_MEMBER — that already exist as env vars; see the cross-component note below on keeping FE/BE in sync.)
  2. Apply it per route after auth, e.g. at the getAllOwnerList mount (routes/auth.js:74):
    router.get("/getAllOwnerList", auth, requireRole([SUPER_ADMIN, ADMIN, ACCOUNT_MANAGER]), async (req, res) => { ... })
    
  3. Make the role=all branch explicit: only honour role=all when the caller's res.role is itself an admin role; otherwise force the restrictive role_type = 5 filter regardless of query string, so a non-admin cannot widen results by passing ?role=all.
  4. Sweep both routes/auth.js and routes/routes.js for handlers mounted without auth/requireRole and add the gate; treat "filter by SQL only" as defence-in-depth, not the primary authorization check.

F-8: SQL injection via unparameterised dynamic filters (MEDIUM — needs spot check)

Searching for ${ and con.query(\SELECTpatterns will find places where SQL is built by string concatenation. Example pattern observed inroutes/auth.js:181`:

usersFilter = `AND m.role_type = 11`;
// later: con.query(`SELECT ... WHERE ... ${usersFilter}`)

The usersFilter value here is hardcoded → safe. But the same handler also does:

if (role == 'all') usersFilter = '';
else if (role == 'test') usersFilter = `AND m.role_type = 11`;
which is safe.

However, audit recommends grep -nE "con\.query\(\SELECT.\\$\{" routes/.jsto find places where the interpolated value comes fromreq.query/req.body. Several similar handlers exist insomeli-api` with this anti-pattern.

A concrete instance where the interpolated value originates from the request body is the member-update query at routes/auth.js:410:

let updatedata = { q: `update tMember SET sid='${reqObj.email}',firstName='${reqObj.fName}', lastName='${reqObj.lName}',role_type=${reqObj.role_type},role='${reqObj.role}'  WHERE id=${reqObj.id};` };
reqObj.email, reqObj.fName, reqObj.lName, reqObj.role, reqObj.id are all user-controlled and string-concatenated → injectable. Similar request-derived interpolation appears in the member-list queries around routes/routes.js:817-821,837.

Fix:

  1. Rewrite request-derived queries with ? placeholders and array-bound params via the already-imported mysql2 driver (routes/auth.js:54, routes/routes.js). For routes/auth.js:410:
    const q = 'UPDATE tMember SET sid=?, firstName=?, lastName=?, role_type=?, role=? WHERE id=?';
    const params = [reqObj.email, reqObj.fName, reqObj.lName, reqObj.role_type, reqObj.role, reqObj.id];
    connection.query(q, params, (err, result) => { /* ... */ });
    
  2. For the list queries at routes/routes.js:817-821,837, bind userId and the role IDs as ? params instead of +userId+ / +process.env.USER_MEMBER+ string concatenation.
  3. Run the audit grep to find the rest before declaring done:
    grep -nE "query\(\`(SELECT|UPDATE|INSERT|DELETE).*\\\$\\{" routes/*.js
    
    Manually classify each hit: hardcoded interpolations (e.g. usersFilter =AND m.role_type = 11` atroutes/auth.js:96) are safe and can stay; any value reachable fromreq.query/req.body/req.params` must move to a bound param.
  4. For the few cases where an identifier (column/table) must be dynamic, use an allowlist mapping rather than interpolation, since ? only binds values.

F-9: 150 MB request body limit (LOW)

server.js:

express.json({ limit: '150mb', extended: true, parameterLimit: 50000 })

A 150 MB JSON body is large enough to be a DoS vector if any unauthenticated endpoint accepts JSON. Currently /authenticate and /webauthenticate are unauthenticated; both accept JSON. A flood of large bogus bodies will exhaust memory.

Fix:

  1. Lower the global JSON limit at server.js:23 to a sane default ('1mb'); JSON payloads in this API are small. File uploads go through express-fileupload (server.js:17), not express.json, so they are unaffected:
    express.json({ limit: '1mb', parameterLimit: 1000 })(req, res, next);
    
    (Drop extended: true — it is not a valid option for express.json and is silently ignored.)
  2. If a specific route genuinely needs a large JSON body, mount a per-route parser with a higher limit on that route only (after auth), rather than raising the global limit.
  3. Configure express-fileupload (server.js:17) with explicit caps so uploads cannot be a separate DoS vector:
    app.use(fileUpload({ limits: { fileSize: 25 * 1024 * 1024 }, abortOnLimit: true }));
    
  4. Pair this with rate limiting so large-body floods are throttled before parsing. Install express-rate-limit (not currently in package.json) and apply it to the unauthenticated endpoints /authenticate (routes/routes.js:131) and /webauthenticate (routes/routes.js:215):
    yarn add express-rate-limit helmet
    
    const rateLimit = require('express-rate-limit');
    const authLimiter = rateLimit({ windowMs: 15 * 60 * 1000, max: 20, standardHeaders: true });
    router.post('/authenticate', authLimiter, async (req, res, next) => { /* ... */ });
    router.post('/webauthenticate', authLimiter, async (req, res, next) => { /* ... */ });
    
    This also addresses the platform-wide "no rate limiting on /authenticate / /webauthenticate → credential-stuffing" pattern noted in the cross-repo security overview.
  5. While installing the above, add app.use(helmet()) near the top of the middleware chain in server.js (e.g. after line 17) to set baseline security response headers (HSTS, X-Content-Type-Options, frame options) — these are missing today.

F-10: conf/credentials.json may be committed (HIGH if true)

A file conf/credentials.json is present in the working tree. If this is a Google service-account JSON (which the path suggests), and if it's actually committed (rather than gitignored), it is a leaked credential. Verify with git -C ... log --all -- conf/credentials.json and git -C ... ls-files conf/credentials.json.

Verification (run 2026-05-22): the file is tracked (git ls-files conf/credentials.json returns it; committed in 75b92e9), and .gitignore does not cover it (it lists .env at line 72 but no credentials*/conf/). The file is a Google OAuth web client config containing a client_secret, not a service-account private key — but a committed OAuth client secret is still a leaked credential and must be treated as compromised.

Fix:

  1. Rotate the OAuth client secret now: in Google Cloud Console → APIs & Services → Credentials, for client 305153224854-...apps.googleusercontent.com, reset/rotate the secret (or create a replacement OAuth client and retire this one).
  2. Stop tracking the file and ignore it going forward:
    git rm --cached conf/credentials.json
    printf '\nconf/credentials.json\n' >> .gitignore   # add near the .env entry at line 72
    git commit -m "stop tracking conf/credentials.json"
    
  3. Provide the file at deploy time out-of-band (env-mounted secret / CI secret store), the same way .env is handled.
  4. Purge it from history so the old secret is unrecoverable from prior commits:
    git filter-repo --path conf/credentials.json --invert-paths   # or BFG --delete-files credentials.json
    git push --force-with-lease --all
    
    Coordinate the force-push with the team (this rewrites shared history). Apply the same history-purge to the F-1 session secret and F-2 Slack token.

Findings out of scope (cross-component)

  • Token format shared with someli-api: tokenGenerator.js is byte-identical. A weakness here (e.g., predictable AES key derivation) affects both repos. Audit the underlying crypto in ../someli-api/security.md.

Fix: 1. Extract tokenGenerator.js into one shared internal package (per the CODE-OVERLAP-MATRIX recommendation for byte-identical helpers) so a crypto fix lands once instead of being copy-pasted. 2. Verify JWT_SECRET_KEY (used at helper/tokenGenerator.js:7,8,13) is a high-entropy value set per-environment via .env and is not committed; the AES passphrase and JWT signing key are the same value today — consider splitting them so a JWT-key rotation does not also invalidate AES-encrypted payloads. 3. Apply the F-4 expiry fix in the shared package so both repos inherit token expiry.

  • Role taxonomy mismatch risk: the FE pins role IDs as env vars (SUPER_ADMIN=1, etc.); the BE uses SQL role_type integers; drift between the two means an FE that disables a button doesn't prevent the BE from honouring the call. Same finding applies to admin_console_R.

Fix: 1. Define the role IDs in one authoritative place (a shared constants module or a tRoles-derived lookup) and have both FE and BE import/read from it, rather than each maintaining its own env-var copy. 2. Enforce roles on the BE with the requireRole middleware from F-7 — the FE button state is UX only and must never be the authorization boundary. 3. Add a startup assertion in the BE that the configured role IDs match the rows in tRoles, so a config drift fails loudly instead of silently mis-gating endpoints.

What's not a finding here

  • Most route handlers parameterise their queries via mysql2's ? placeholders. SQL injection is restricted to the dynamic-filter patterns called out in F-8.
  • Password hashing uses bcryptjs — fine (slow but secure).
  • File uploads via express-fileupload use temp files; no path traversal observed in spot-checks.
  • HTTPS / TLS is terminated upstream by nginx — not this repo's concern.