dd0c/products/SECURITY-REVIEW-POST-HARDENING.md

Adversarial Security & Architecture Review

Commit: eb953cd Focus: Security Hardening Changes

Overview

The security hardening pass successfully implemented a few surface-level mitigations, but the implementation is fundamentally flawed. In several cases, the "fixes" are incomplete, introduced new critical vulnerabilities, or suffer from severe architectural oversights. The system is still highly vulnerable.

PASS

• JWT Algorithms: Hardcoding { algorithms: ['HS256'] } correctly mitigates algorithm confusion attacks.
• Plugin Encapsulation: Registering the authHook locally inside protectedApp.addHook correctly prevents bypasses compared to global URL prefix matching.
• CORS: Locked down correctly to http://localhost:5173.
• Async Webhooks: Redis LPUSH/BRPOP architecture and dead-letter queue provide a reasonable foundation for async processing.

FAIL

1. [CRITICAL] SQL Injection in RLS Wrapper (db.ts)

File: 03-alert-intelligence/src/data/db.ts:13 Issue: await client.query(\SET LOCAL app.tenant_id = '${tenantId}'`);ThetenantIdis string-interpolated directly into the query instead of being parameterized. WhiletenantIdcomes from a signed JWT, if the JWT secret is compromised (see issue #10) or user input ever leaks into this function, it allows trivial SQL injection. **Fix:** Useawait client.query('SELECT set_config($1, $2, true)', ['app.tenant_id', tenantId]);`

2. [CRITICAL] Rate Limiting is a Mirage (index.ts & middleware.ts)

File: 03-alert-intelligence/src/index.ts Issue: The route definitions proudly declare { config: { rateLimit: { max: 5, timeWindow: '1 minute' } } }, but the @fastify/rate-limit plugin is never actually registered on the Fastify app. Furthermore, the planned "100 req/min global" limit is entirely missing. Rate limiting is 100% bypassable right now.

3. [CRITICAL] Invite Token Hijacking (middleware.ts)

File: 03-alert-intelligence/src/auth/middleware.ts:168 Issue: During signup via an invite_token, the server pulls the invite record but never verifies that body.email matches the invited email. Anyone who obtains the invite link can register an account using their own email address, completely bypassing intended access controls.

4. [HIGH] Fragmented Identity Architecture (Microservice Anti-Pattern)

File: 03-alert, 05-cost, 04-portal (all products) Issue: The authentication routes, invite logic, and users/tenants DB tables have been copy-pasted into every microservice. Since each service uses its own separate database (dd0c_alert, dd0c_cost, etc.), identity is completely siloed. A user who signs up in the Portal doesn't exist in the Alert database. This breaks the unified console experience and means foreign keys will fail if a valid JWT from one service is used against another.

5. [HIGH] Webhook HMAC Verification Broken on Valid Payloads

File: 03-alert-intelligence/src/api/webhooks.ts Issue: Reconstructing the raw body via const rawBody = JSON.stringify(body); for HMAC validation is disastrous. Fastify parses the JSON, and stringifying it back will reorder keys or alter whitespace, meaning perfectly valid Datadog/PagerDuty webhooks will fail signature verification randomly. You must use a raw body parser plugin.

6. [HIGH] Token TOCTOU Race Condition

File: 03-alert-intelligence/src/auth/middleware.ts:175 Issue: When checking and updating an invite (SELECT ... FROM tenant_invites followed by UPDATE ... SET accepted_at = NOW()), there is no row-level locking (e.g., FOR UPDATE). An attacker can send concurrent requests to use the same invite token multiple times before the first request marks it accepted.

7. [HIGH] Webhook Incident Creation Race Condition

File: 03-alert-intelligence/src/workers/webhook-processor.ts:51 Issue: The worker queries SELECT id FROM incidents WHERE fingerprint = $1. If no incident exists, it inserts one. If two webhooks for the same fingerprint are popped by parallel workers, both will see no incident and insert duplicate incidents.

WARN

8. [MEDIUM] getPoolForAuth() Defeats Pool Encapsulation

File: 03-alert-intelligence/src/data/db.ts Issue: The PR claimed to restrict access to the raw DB pool, but merely added an export named getPoolForAuth() which returns the raw pg.Pool. This allows any developer to easily bypass the withTenant RLS controls by importing the raw pool.

9. [MEDIUM] Timing Attack Vector in Grafana Webhooks

File: 03-alert-intelligence/src/api/webhooks.ts:80 Issue: token !== secret.secret uses standard string equality instead of crypto.timingSafeEqual. While low risk for standard APIs, webhook secrets should be validated securely to prevent timing attacks.

10. [MEDIUM] Insecure Default Secrets in Infrastructure

File: docker-compose.yml:51 Issue: The compose file falls back to ${JWT_SECRET:-dev-secret-change-me-in-production!!}. If operations forgets to configure JWT_SECRET in a production environment, the app fails open with a globally known secret, leading to total environment compromise.

11. [LOW] "Least Privilege" Misnomer

File: docker-init-db.sh:15 Issue: The script claims to create "least-privilege" users, but immediately runs GRANT ALL PRIVILEGES ON DATABASE and GRANT ALL ON SCHEMA public. The service users have full DDL rights and can DROP or TRUNCATE tables.

12. [LOW] Plaintext Invite Tokens in Database

File: shared/003_invites.sql Issue: tenant_invites.token is stored as plaintext. If the database is dumped or compromised, the attacker has a list of active invite links. They should be stored as SHA-256 hashes, similar to API keys.

13. [LOW] Weak Password Hashing Defaults

File: 03-alert-intelligence/src/auth/middleware.ts:84 Issue: crypto.scrypt is called without explicit options, defaulting to Node.js's native N=16384. This is considered too low for modern offline cracking resistance. Explicit parameters (e.g., N=65536) should be provided.