diff --git a/TODO.md b/TODO.md index 284b711f..04d346a7 100644 --- a/TODO.md +++ b/TODO.md @@ -1,5 +1,5 @@ # Post-Pilot β€” Task List -*Last updated: 2026-08-08 β€” Plan docs rebased to live stack; phases aligned with PLANNING.md* +*Last updated: 2026-08-10 β€” Security audit doc merged; remediations in #12* Priority levels: πŸ”΄ Critical (stop-ship) Β· 🟠 High Β· 🟑 Medium Β· 🟒 Low @@ -7,6 +7,14 @@ Priority levels: πŸ”΄ Critical (stop-ship) Β· 🟠 High Β· 🟑 Medium Β· 🟒 L --- +## πŸ”΄ CRITICAL β€” Security audit findings + +Full write-up: [`docs/SECURITY_AUDIT.md`](docs/SECURITY_AUDIT.md) + +C1–C6 remediations are implemented on `cursor/security-fixes-c1-c6-2720` (PR #12). Remaining high follow-ups from the audit: rate limits (H1), SSRF allowlist (H2), embed.js XSS (H3). + +--- + ## πŸ”΄ CRITICAL β€” Manual go-live blockers (Phase 5) ### STEP 1 Β· Generate secure keys (run locally) diff --git a/blueprints/__pycache__/__init__.cpython-312.pyc b/blueprints/__pycache__/__init__.cpython-312.pyc deleted file mode 100644 index e693bf19..00000000 Binary files a/blueprints/__pycache__/__init__.cpython-312.pyc and /dev/null differ diff --git a/blueprints/__pycache__/api.cpython-312.pyc b/blueprints/__pycache__/api.cpython-312.pyc deleted file mode 100644 index 1ab93f0f..00000000 Binary files a/blueprints/__pycache__/api.cpython-312.pyc and /dev/null differ diff --git a/blueprints/__pycache__/auth.cpython-312.pyc b/blueprints/__pycache__/auth.cpython-312.pyc deleted file mode 100644 index 5aeeda7c..00000000 Binary files a/blueprints/__pycache__/auth.cpython-312.pyc and /dev/null differ diff --git a/blueprints/__pycache__/billing.cpython-312.pyc b/blueprints/__pycache__/billing.cpython-312.pyc deleted file mode 100644 index a22921f8..00000000 Binary files a/blueprints/__pycache__/billing.cpython-312.pyc and /dev/null differ diff --git a/blueprints/__pycache__/pages.cpython-312.pyc b/blueprints/__pycache__/pages.cpython-312.pyc deleted file mode 100644 index d99cab9b..00000000 Binary files a/blueprints/__pycache__/pages.cpython-312.pyc and /dev/null differ diff --git a/blueprints/__pycache__/utils.cpython-312.pyc b/blueprints/__pycache__/utils.cpython-312.pyc deleted file mode 100644 index 09744975..00000000 Binary files a/blueprints/__pycache__/utils.cpython-312.pyc and /dev/null differ diff --git a/blueprints/__pycache__/website.cpython-312.pyc b/blueprints/__pycache__/website.cpython-312.pyc deleted file mode 100644 index 2df29c69..00000000 Binary files a/blueprints/__pycache__/website.cpython-312.pyc and /dev/null differ diff --git a/docs/SECURITY_AUDIT.md b/docs/SECURITY_AUDIT.md new file mode 100644 index 00000000..306cc0f4 --- /dev/null +++ b/docs/SECURITY_AUDIT.md @@ -0,0 +1,276 @@ +# Post-Pilot Security & Project Audit + +**Date:** 2026-08-06 +**Scope:** Full repository review (auth, API, tokens, cron, billing, XSS/CSRF, git hygiene, infra) +**Method:** Static code review of Flask app, blueprints, modules, templates, Vercel config, git tracking +**Status:** Findings only β€” no production secrets rotated in this pass + +--- + +## Executive summary + +Post-Pilot has solid foundations in places (parameterized SQL, Stripe webhook signature verification, OAuth state tokens, Fernet token encryption *when keyed correctly*, cron HMAC when `CRON_SECRET` is set). It is **not production-hardened**. Several issues are stop-ship: CSRF exemption of session-authenticated APIs, `/v1` IDOR via `user_id`, ephemeral encryption keys on serverless, and a large tracked `.venv` tree. + +| Severity | Count | +|----------|------:| +| Critical | 6 | +| High | 8 | +| Medium | 10 | +| Low / hygiene | 8 | + +--- + +## Critical + +### C1 β€” CSRF protection disabled on session-authenticated mutating APIs + +**Where:** `blueprints/__init__.py` exempts `api_bp`, `specials_bp`, `events_bp`, `hours_bp` from CSRF. + +**Impact:** Any site a logged-in user visits can forge `POST /api/publish`, `/api/push_all`, `/api/setup_tokens`, `/api/delete_post`, specials/events/hours mutations using the victim’s session cookie. `/api/setup_tokens` can overwrite OAuth tokens. + +**Fix:** Remove CSRF exemption for browser-session blueprints. Send `X-CSRFToken` (or form token) from dashboard JS. Keep exemptions only for `cron_bp`, `stripe_webhook_bp`, and true Bearer-token `/v1` routes. + +--- + +### C2 β€” `/v1` IDOR: callers can act as any `user_id` + +**Where:** `modules/api_manager.py` β€” `generate_post`, `publish_post`, `generate_and_publish`, `get_history`, `get_site_config`, `set_published`. + +```python +user_id = body.get('user_id') or g.api_user_id +``` + +**Impact:** A valid user API key (or SRN secret) can publish, read history, or toggle website publish for **any** user by supplying their UUID. Cross-tenant takeover of social publishing. + +**Fix:** For user API keys, **always** bind `user_id = g.api_user_id` and ignore client-supplied IDs. For SRN secret, require an allowlist / explicit service ACL, not arbitrary UUID. + +--- + +### C3 β€” `TOKEN_ENCRYPTION_KEY` falls back to ephemeral Fernet key + +**Where:** `modules/auth_manager.py` (lines 33–46). + +**Impact:** On Vercel cold starts without a stable env var, a new key is generated β†’ all stored OAuth tokens fail to decrypt β†’ publish/cron break silently or with crypto errors. Invalid keys are also replaced with another ephemeral key instead of failing closed. + +**Fix:** Refuse to start in production if `TOKEN_ENCRYPTION_KEY` is missing/invalid. Never generate a runtime key in prod. Confirm SEC-1 rotation steps in `TODO.md`. + +--- + +### C4 β€” Entire `.venv` committed to git (~13k files) + +**Where:** `git ls-files '.venv'` β‰ˆ 13,181 paths (Windows-style `Lib/site-packages`). Also thousands of `__pycache__/*.pyc` tracked despite `.gitignore`. + +**Impact:** Repo bloat, accidental secret leakage from local env baked into packages, noisy diffs, supply-chain confusion. `.gitignore` is correct but files were force-added before ignore rules. + +**Fix:** +```bash +git rm -r --cached .venv blueprints/__pycache__ modules/__pycache__ +git commit -m "chore: untrack .venv and __pycache__" +``` + +--- + +### C5 β€” `from app import get_db` but `app.py` does not define `get_db` + +**Where:** `blueprints/api.py`, `specials.py`, `events.py`, `hours.py`, `embed_api.py`. + +**Impact:** Runtime `ImportError` on scheduled posts, history, platform settings, specials/events/hours. Dead or silently failing features; error handlers may mask this. + +**Fix:** `from modules.database import get_db` everywhere (or re-export `get_db` from `app.py`). + +--- + +### C6 β€” Manual token injection endpoint + +**Where:** `POST /api/setup_tokens` (`blueprints/api.py`). + +**Impact:** Authenticated (and CSRF-exempt β€” see C1) clients can write arbitrary platform access tokens. Combined with CSRF, an attacker can plant their tokens or wipe victim tokens. Prefer OAuth-only flows. + +**Fix:** Remove or heavily restrict to admin/dev; never accept raw tokens from the browser in production. + +--- + +## High + +### H1 β€” No rate limits on auth or API + +**Where:** `app.py` β€” `Limiter(..., default_limits=[])`. No `@limiter.limit` on `/login`, `/register`, magic link, or `/api/*`. + +**Impact:** Magic-link email bombing, OTP spam, AI generation cost abuse, publish spam. + +**Fix:** e.g. `5/minute` on login/register; tiered limits on `/api/generate*` and `/api/publish*`. Require `REDIS_URL` in production (memory:// is per-instance and ineffective on serverless). + +--- + +### H2 β€” SSRF via user-controlled media URLs + +**Where:** `modules/publisher.py` `_publish_twitter` does `requests.get(image_url)`. Meta/TikTok paths also pass `image_url`/`video_url`/`file_url` to third parties (second-order SSRF). + +**Impact:** Server-side fetch of internal metadata endpoints (`169.254.169.254`, `localhost`, private RFC1918). Validator only checks `http/https` + netloc β€” no blocklist. + +**Fix:** Allowlist public HTTPS hosts; block private/link-local/metadata IPs after DNS resolve; prefer upload-to-blob then serve known URLs. + +--- + +### H3 β€” Stored XSS in embed widget + +**Where:** `static/embed.js` interpolates API fields into `innerHTML` without escaping (`caption`, hours keys/values, service name/desc/price, `image_url` in `src`). + +**Impact:** A compromised or malicious business profile can XSS any site embedding the widget (session/cookie theft on that origin). + +**Fix:** Use `textContent` / `createElement`; escape HTML; sanitize URLs (`https:` only). + +--- + +### H4 β€” DEV_LOGIN timing-unsafe compare + remember cookie + +**Where:** `blueprints/auth.py` β€” `request.args.get('key') != dev_key`; `login_user(..., remember=True)`. + +**Impact:** If `DEV_LOGIN_KEY` leaks into production, full account impersonation via query string (also lands in logs/Referer). Non-constant-time compare. + +**Fix:** Confirm env absent in Vercel prod (TODO Step 5). Use `hmac.compare_digest`. Prefer short-lived session without `remember=True`. + +--- + +### H5 β€” Session cookie flags not hardened + +**Where:** `app.py` β€” no `SESSION_COOKIE_SECURE`, `HTTPONLY`, `SAMESITE`, `REMEMBER_COOKIE_*`. + +**Impact:** Session leakage over HTTP / some CSRF assistance if SameSite unset. + +**Fix:** In production set `Secure=True`, `HttpOnly=True`, `SameSite='Lax'` (or `Strict` where viable). + +--- + +### H6 β€” Cron routes are POST-only; Vercel Cron invokes GET + +**Where:** `vercel.json` cron paths + `blueprints/cron.py` `methods=['POST']`. + +**Impact:** Scheduled generate/publish may never run (405). Operational outage, not just security. + +**Fix:** Accept `GET` and `POST`, or configure invocation to match. Keep Bearer `CRON_SECRET` check. Note: `/api/cron/health` is unauthenticated and enumerates endpoints (low). + +--- + +### H7 β€” `SRN_SECRET` compared with `==` + +**Where:** `modules/api_manager.py` `require_api_key`. + +**Impact:** Timing side-channel on shared service secret. Prefer `hmac.compare_digest`. Empty `SRN_SECRET` correctly disables that path; ensure it is never accidentally set to a short value. + +--- + +### H8 β€” Analytics accepts client-supplied Meta tokens + +**Where:** `api_analytics` β€” `token = tokens.get('facebook_token') or data.get('access_token')` (same for `page_id` / `ig_id`). + +**Impact:** Authenticated user can point analytics at arbitrary tokens/pages (abuse Meta API via your app, confuse audit trails). Prefer stored tokens only. + +--- + +## Medium + +### M1 β€” `check_post_limit()` never enforced on publish + +**Where:** `modules/plan_guard.py` defines limits; TODO notes wiring missing; `api_publish` / `api_push_all` / cron generate ignore monthly caps. + +**Impact:** Billing bypass / cost overrun. + +### M2 β€” `embed_api` blueprint not registered + +**Where:** `blueprints/embed_api.py` exists; `register_blueprints` never imports `embed_bp`. + +**Impact:** Public embed API dead; docs/`embed.js` call a missing route (or 404). + +### M3 β€” Website hub / disconnect CSRF mismatch + +**Where:** `website_bp` and `auth.disconnect` are **not** CSRF-exempt, but frontend `fetch()` calls omit CSRF tokens (`website_hub.html`, `connect.html`). Disconnect uses GET-style `fetch` against a POST route. + +**Impact:** Features broken in real CSRF-on mode; teams may β€œfix” by exempting more routes (widening C1). + +### M4 β€” DATABASE_URL partially logged + +**Where:** `modules/db.py` logs `DATABASE_URL[:40]` β€” often includes credentials. + +**Impact:** Secret leakage to logs/Sentry/Vercel log drains. + +### M5 β€” `app.run(debug=True)` in entrypoint + +**Where:** `app.py` `__main__`. Low risk on Vercel (not used), high if someone runs the module in a shared host. + +### M6 β€” Public site IDOR-ish enumeration + +**Where:** `GET /site/` β€” predictable UUID/path discloses published site content. Prefer opaque slugs + rate limits. + +### M7 β€” Privacy/legal copy outdated + +**Where:** `templates/legal/privacy.html`, `register.html` still describe passwords/bcrypt while auth is magic-link only. + +**Impact:** Compliance/trust issue, not direct exploit. + +### M8 β€” CORS allowlist includes specific Vercel hosts only + +**Where:** `app.py`. Fine if intentional; custom domains need explicit addition. No credentials mode review documented. + +### M9 β€” Error responses leak exception strings + +**Where:** cron 500 returns `str(e)`; `/v1` returns `str(e)` on several handlers. + +**Impact:** Internal path/DB detail disclosure. + +### M10 β€” No security headers + +Missing CSP, `X-Frame-Options`/`frame-ancestors`, `X-Content-Type-Options`, HSTS (may be at Vercel edge β€” confirm). + +--- + +## Low / hygiene + +| ID | Issue | +|----|--------| +| L1 | `railway.toml` still present; AGENTS.md says Vercel-only (INFRA-5) | +| L2 | AGENTS.md still lists INFRA-6 cron registration as open; cron **is** registered β€” docs drift | +| L3 | Env naming drift: `FACEBOOK_APP_*` vs docs `META_APP_*`; Stripe price env names inconsistent between `.env.example` and `billing_manager.py` | +| L4 | Tests (`test_smoke.py`) still assert password auth β€” suite likely stale vs magic-link | +| L5 | `_uid()` fallback to `'default'` documented as dangerous if `@login_required` missed | +| L6 | Requirements unpinned (`flask>=3.0.0`) β€” reproducibility / supply chain | +| L7 | MCP server can read arbitrary repo files given `GITHUB_TOKEN` β€” expected for ops tool; protect token scope | +| L8 | Demo dashboard routes `/demo` unauthenticated β€” intentional marketing surface; ensure no live tokens | + +--- + +## What looks solid + +- SQL uses bound parameters (`?` / `%s`) β€” no obvious SQLi in reviewed paths +- Stripe webhooks use `Webhook.construct_event` +- OAuth flows use `secrets.token_urlsafe` state and validate on callback +- Cron rejects requests when `CRON_SECRET` unset (`hmac.compare_digest`) +- Sentry `send_default_pii=False` +- Production refuses start without `FLASK_SECRET_KEY` / Supabase URL+anon key when `VERCEL_ENV` / production Flask env set +- Post deletes scoped by `user_id` +- API keys stored as SHA-256 hashes, not plaintext + +--- + +## Recommended remediation order + +1. **Immediate (ops):** Confirm `DEV_LOGIN_KEY` absent in Vercel; set stable `TOKEN_ENCRYPTION_KEY`, `FLASK_SECRET_KEY`, `CRON_SECRET`, `REDIS_URL`, `SENTRY_DSN` (TODO Steps 1–5). +2. **Immediate (code):** Fix C2 IDOR; re-enable CSRF for session APIs (C1); fail closed on encryption key (C3); untrack `.venv` (C4); fix `get_db` imports (C5). +3. **Next:** Rate limits (H1), SSRF controls (H2), embed XSS (H3), session cookie flags (H5), cron GET/POST (H6). +4. **Then:** Wire `check_post_limit`, register or remove embed blueprint, align CSRF on website/disconnect, scrub logs, security headers, doc/test cleanup. + +--- + +## Verification checklist (after fixes) + +- [ ] CSRF: cross-origin POST to `/api/publish` with session cookie β†’ 400 +- [ ] `/v1/publish_post` with user API key + foreign `user_id` β†’ 403 +- [ ] Restart without `TOKEN_ENCRYPTION_KEY` in prod-like env β†’ process exits +- [ ] `git ls-files '.venv' | wc -l` β†’ `0` +- [ ] Specials/events/hours CRUD works (get_db import fixed) +- [ ] Vercel cron hits publish/generate successfully with secret +- [ ] Embed widget with malicious caption does not execute script + +--- + +*Audit performed as a Cloud Agent pass against repo `main` at 2026-08-06. Production Vercel/Supabase live config was not mutated.* diff --git a/modules/__pycache__/__init__.cpython-312.pyc b/modules/__pycache__/__init__.cpython-312.pyc deleted file mode 100644 index ffba5314..00000000 Binary files a/modules/__pycache__/__init__.cpython-312.pyc and /dev/null differ diff --git a/modules/__pycache__/analytics_client.cpython-312.pyc b/modules/__pycache__/analytics_client.cpython-312.pyc deleted file mode 100644 index dc130b13..00000000 Binary files a/modules/__pycache__/analytics_client.cpython-312.pyc and /dev/null differ diff --git a/modules/__pycache__/api_manager.cpython-312.pyc b/modules/__pycache__/api_manager.cpython-312.pyc deleted file mode 100644 index f47c7176..00000000 Binary files a/modules/__pycache__/api_manager.cpython-312.pyc and /dev/null differ diff --git a/modules/__pycache__/auth_manager.cpython-312.pyc b/modules/__pycache__/auth_manager.cpython-312.pyc deleted file mode 100644 index 5a362737..00000000 Binary files a/modules/__pycache__/auth_manager.cpython-312.pyc and /dev/null differ diff --git a/modules/__pycache__/billing_manager.cpython-312.pyc b/modules/__pycache__/billing_manager.cpython-312.pyc deleted file mode 100644 index 966e5b6d..00000000 Binary files a/modules/__pycache__/billing_manager.cpython-312.pyc and /dev/null differ diff --git a/modules/__pycache__/database.cpython-312.pyc b/modules/__pycache__/database.cpython-312.pyc deleted file mode 100644 index 7428bfa1..00000000 Binary files a/modules/__pycache__/database.cpython-312.pyc and /dev/null differ diff --git a/modules/__pycache__/db.cpython-312.pyc b/modules/__pycache__/db.cpython-312.pyc deleted file mode 100644 index 4bd9ba0e..00000000 Binary files a/modules/__pycache__/db.cpython-312.pyc and /dev/null differ diff --git a/modules/__pycache__/google_client.cpython-312.pyc b/modules/__pycache__/google_client.cpython-312.pyc deleted file mode 100644 index 83e221af..00000000 Binary files a/modules/__pycache__/google_client.cpython-312.pyc and /dev/null differ diff --git a/modules/__pycache__/plan_guard.cpython-312.pyc b/modules/__pycache__/plan_guard.cpython-312.pyc deleted file mode 100644 index bac79672..00000000 Binary files a/modules/__pycache__/plan_guard.cpython-312.pyc and /dev/null differ diff --git a/modules/__pycache__/post_generator.cpython-312.pyc b/modules/__pycache__/post_generator.cpython-312.pyc deleted file mode 100644 index 528b1fb7..00000000 Binary files a/modules/__pycache__/post_generator.cpython-312.pyc and /dev/null differ diff --git a/modules/__pycache__/post_scheduler.cpython-312.pyc b/modules/__pycache__/post_scheduler.cpython-312.pyc deleted file mode 100644 index 6ab261ed..00000000 Binary files a/modules/__pycache__/post_scheduler.cpython-312.pyc and /dev/null differ diff --git a/modules/__pycache__/publisher.cpython-312.pyc b/modules/__pycache__/publisher.cpython-312.pyc deleted file mode 100644 index c7385802..00000000 Binary files a/modules/__pycache__/publisher.cpython-312.pyc and /dev/null differ diff --git a/modules/__pycache__/scheduler_worker.cpython-312.pyc b/modules/__pycache__/scheduler_worker.cpython-312.pyc deleted file mode 100644 index d86dc3bb..00000000 Binary files a/modules/__pycache__/scheduler_worker.cpython-312.pyc and /dev/null differ diff --git a/modules/__pycache__/tiktok_client.cpython-312.pyc b/modules/__pycache__/tiktok_client.cpython-312.pyc deleted file mode 100644 index 75592fce..00000000 Binary files a/modules/__pycache__/tiktok_client.cpython-312.pyc and /dev/null differ diff --git a/modules/__pycache__/user_manager.cpython-312.pyc b/modules/__pycache__/user_manager.cpython-312.pyc deleted file mode 100644 index 56a2cf81..00000000 Binary files a/modules/__pycache__/user_manager.cpython-312.pyc and /dev/null differ diff --git a/modules/__pycache__/validator.cpython-312.pyc b/modules/__pycache__/validator.cpython-312.pyc deleted file mode 100644 index 67f8518c..00000000 Binary files a/modules/__pycache__/validator.cpython-312.pyc and /dev/null differ diff --git a/modules/__pycache__/website_manager.cpython-312.pyc b/modules/__pycache__/website_manager.cpython-312.pyc deleted file mode 100644 index a26c965d..00000000 Binary files a/modules/__pycache__/website_manager.cpython-312.pyc and /dev/null differ diff --git a/tests/__pycache__/__init__.cpython-312.pyc b/tests/__pycache__/__init__.cpython-312.pyc deleted file mode 100644 index 9dfb5c21..00000000 Binary files a/tests/__pycache__/__init__.cpython-312.pyc and /dev/null differ diff --git a/tests/__pycache__/conftest.cpython-312-pytest-9.1.1.pyc b/tests/__pycache__/conftest.cpython-312-pytest-9.1.1.pyc deleted file mode 100644 index 15d71dd9..00000000 Binary files a/tests/__pycache__/conftest.cpython-312-pytest-9.1.1.pyc and /dev/null differ diff --git a/tests/__pycache__/test_p0_fixes.cpython-312-pytest-9.1.1.pyc b/tests/__pycache__/test_p0_fixes.cpython-312-pytest-9.1.1.pyc deleted file mode 100644 index 22ffc04f..00000000 Binary files a/tests/__pycache__/test_p0_fixes.cpython-312-pytest-9.1.1.pyc and /dev/null differ diff --git a/tests/__pycache__/test_smoke.cpython-312-pytest-9.1.1.pyc b/tests/__pycache__/test_smoke.cpython-312-pytest-9.1.1.pyc deleted file mode 100644 index 814a1a04..00000000 Binary files a/tests/__pycache__/test_smoke.cpython-312-pytest-9.1.1.pyc and /dev/null differ diff --git a/tests/__pycache__/test_validator.cpython-312-pytest-9.1.1.pyc b/tests/__pycache__/test_validator.cpython-312-pytest-9.1.1.pyc deleted file mode 100644 index ca9e53a6..00000000 Binary files a/tests/__pycache__/test_validator.cpython-312-pytest-9.1.1.pyc and /dev/null differ