Security¶
Findings¶
F-1: Hardcoded session secret (HIGH)¶
server.js:11:
The literal string 3eB(2:\srlI+qa5 is the cookie-signing key for every deployment — dev, uat, prod. This means:
- 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.
- The secret cannot be rotated without a code change.
- Even though the handlers currently don't use
req.session, the session cookie is still issued to every client; it could be replayed.
Fix:
- Add
SESSION_SECRET(and optionallySESSION_NAME) toconf.js(after line 48) so it is sourced from the environment alongside the other secrets: - 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 old3eB(2:\srlI+qa5is now compromised and must never be reused). - Replace
server.js:11-15with 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" } })) - Setting
saveUninitialized: false(wastrueatserver.js:14) stops a signed cookie being issued to every client when no handler usesreq.session, removing the replay surface noted above. - 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:
- Rotate immediately (highest priority): in the Slack admin / api.slack.com app config, revoke
xoxb-3144030948916-5918653901701-lKq16vALrV1AqVuTjs9lYNkZand reinstall the app to mint a new bot token. The leaked value is unrecoverable once it has been in the repo. - Add
SLACK_BOT_TOKENandSLACK_CHANNEL_IDtoconf.js(after line 48): and put the new (rotated) token +C05TS9AHBH6in each environment's.env. - Replace the literals at
routes/auth.js:20-21and the client init atroutes/auth.js:25:(const token = conf.SLACK_BOT_TOKEN var channel = conf.SLACK_CHANNEL_ID // ... const bot = new Slack({ token })confis already imported atroutes/auth.js:11.) - Purge the literal from git history (see F-10) so the old token is not retrievable from old commits even after rotation.
- Audit the bot's OAuth scopes in the Slack app config and reduce to the minimum needed (
chat:writefor 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:
- Create a persistent revocation table (MySQL):
- Replace the in-memory
Setinhelper/revokeToken.js:1-3with insert/lookup helpers backed by the sharedcon/mysql2pool, 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))); - Update
middlewares/auth.js:12toawait/check the DB result instead ofrevokedTokens.has(token)(make the middlewareasync): - 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. - 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:
helper/tokenGenerator.js:8already passesexpiresIn: expiresIn ?? JWT_EXPIRES_INtojwt.sign, butJWT_EXPIRES_INis not present inconf.jsand is read straight fromprocess.env(helper/tokenGenerator.js:3). When it is unset,expiresInisundefined, sojwt.signmints a non-expiring token. Set it explicitly: addJWT_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:decryptDataathelper/tokenGenerator.js:13usesjwt.verify, which already throwsTokenExpiredErroronceexpis set — so once step 1 lands, expired tokens are rejected automatically.- Confirm
middlewares/auth.jstreats a thrown verify error as a 401: it does (thetry/catchatmiddlewares/auth.js:6,29-33returnsunauthorized access), so no change is needed there once expiry is set. - For belt-and-braces (defence in depth against
expbeing stripped), also embed{ ..., issuedAt, expiresAt }inside the AES-encrypted payload built at theencryptDatacall sites (routes/routes.js:175,routes/routes.js:252) and re-checkDate.now() < expiresAtafter decryption inmiddlewares/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:
- Add an allowlist to
conf.jsso origins are environment-specific: - Replace the bare
app.use(cors())atserver.js:75with an allowlist (the admin FE dev server runs on port 8080 per the repo notes): - The
corspackage is already a dependency (package.json:29) — no install needed. - 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 bothauthRouteandApi.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):
- If vestigial (no webhooks are meant to land on the admin API): delete the
ifbranch atserver.js:20-22and applyexpress.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. - 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_KEYare already exposed inconf.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. - 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:
- Add a
requireRolemiddleware (e.g.middlewares/requireRole.js).middlewares/auth.js:18-19already setsres.rolefrom the decrypted token, so the gate can read it directly:(Use the numericmodule.exports = (allowed) => (req, res, next) => { if (!allowed.includes(Number(res.role))) return res.status(403).send("forbidden"); next(); };role_typeIDs —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.) - Apply it per route after
auth, e.g. at thegetAllOwnerListmount (routes/auth.js:74): - Make the
role=allbranch explicit: only honourrole=allwhen the caller'sres.roleis itself an admin role; otherwise force the restrictiverole_type = 5filter regardless of query string, so a non-admin cannot widen results by passing?role=all. - Sweep both
routes/auth.jsandroutes/routes.jsfor handlers mounted withoutauth/requireRoleand 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`:
The usersFilter value here is hardcoded → safe. But the same handler also does:
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:
- Rewrite request-derived queries with
?placeholders and array-bound params via the already-importedmysql2driver (routes/auth.js:54,routes/routes.js). Forroutes/auth.js:410: - For the list queries at
routes/routes.js:817-821,837, binduserIdand the role IDs as?params instead of+userId+/+process.env.USER_MEMBER+string concatenation. - Run the audit grep to find the rest before declaring done:
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. - 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:
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:
- Lower the global JSON limit at
server.js:23to a sane default ('1mb'); JSON payloads in this API are small. File uploads go throughexpress-fileupload(server.js:17), notexpress.json, so they are unaffected: (Dropextended: true— it is not a valid option forexpress.jsonand is silently ignored.) - 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. - Configure
express-fileupload(server.js:17) with explicit caps so uploads cannot be a separate DoS vector: - Pair this with rate limiting so large-body floods are throttled before parsing. Install
express-rate-limit(not currently inpackage.json) and apply it to the unauthenticated endpoints/authenticate(routes/routes.js:131) and/webauthenticate(routes/routes.js:215):This also addresses the platform-wide "no rate limiting onconst 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) => { /* ... */ });/authenticate//webauthenticate→ credential-stuffing" pattern noted in the cross-repo security overview. - While installing the above, add
app.use(helmet())near the top of the middleware chain inserver.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:
- 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). - Stop tracking the file and ignore it going forward:
- Provide the file at deploy time out-of-band (env-mounted secret / CI secret store), the same way
.envis handled. - Purge it from history so the old secret is unrecoverable from prior commits: 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.jsis 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 SQLrole_typeintegers; drift between the two means an FE that disables a button doesn't prevent the BE from honouring the call. Same finding applies toadmin_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-fileuploaduse temp files; no path traversal observed in spot-checks. - HTTPS / TLS is terminated upstream by nginx — not this repo's concern.