🔍 Senior Dev Audit — Full Security, Architecture & Tech Debt Review

[ShadowWalkerNC]

Post-Pilot — Senior Dev Audit

Stack: Flask 3 · SQLite/Postgres · APScheduler · Flask-Login · Flask-WTF/CSRF · Flask-Limiter · Stripe · Meta Graph API · OpenAI · MCP server
Repo size: ~1,000-line app.py, 10+ modules, 6 templates, tests directory, Railway + Render deploy configs

---

✅ What's Working Well

Security fundamentals are solid for a v0 SaaS:

• OAuth state nonce (secrets.token_urlsafe(32)) on all three OAuth flows (FB, Google, TikTok) — proper CSRF prevention
• flask-wtf CSRF enabled globally; Stripe webhook correctly exempted via csrf.exempt(v1_blueprint)
• Hard fail on missing FLASK_SECRET_KEY in production — that's the right call
• flask-limiter on auth routes (5/min login, 10/min register) — good bot prevention
• Per-user token isolation with user_id threading through save_token()/load_token()
• @require_plan decorator for plan-gating premium routes — clean pattern
• check_platform_limit enforced in api_push_all before publishing
• init_db() wrapped in try/except — won't hard-fail on cold start

Architecture positives:

• Clean separation of concerns: publisher, scheduler, analytics, billing, auth all in their own modules
• Blueprint registration for v1 API is the right foundation
• .env.example is well-documented and commit-safe
• Sentry + gunicorn + Railway/Render configs show production readiness thinking
• Tests directory + pytest in requirements — commitment to testing exists

---

🔴 Critical Issues

1. init_scheduler() runs at module import level

# bottom of app.py
init_scheduler()

This fires on every gunicorn worker, every pytest import, and every hot reload. With multiple gunicorn workers, APScheduler will try to fire scheduled posts multiple times. In tests, background jobs will run during your test suite. Fix:

if __name__ == '__main__':
    init_scheduler()
    app.run()

# And in your Procfile/startup, trigger via Flask CLI or use APScheduler's
# jobstores with Postgres to handle distributed deduplication

2. api_publish bypasses the platform limit check

/api/push_all calls check_platform_limit — /api/publish and /api/publish_post do not. A free-tier user can bypass plan enforcement by posting to /api/publish instead of /api/push_all. This is a billing bypass on a SaaS product.

3. Dual write path inconsistency

The module-level comment says routes were "updated to use modules.db for Postgres compatibility," but api_post_history, api_scheduled_posts, and api_delete_post all use get_db() (raw SQLite via g._database), while UserManager.log_post() almost certainly writes through a different connection/abstraction. On Postgres/Railway, these routes will read from SQLite and write to Postgres, or vice versa. This will produce empty history or double-write depending on the runtime environment.

4. XSS in public site fallback renderer

# _render_public_site — the fallback path
f'<h1>{title}</h1>'  # title is user-controlled SEO data

If the Jinja2 template fails to render, the raw HTML fallback interpolates user-provided data directly into HTML without escaping. Any user who sets their SEO title to <script>...</script> gets stored XSS on their public site page. Fix:

from markupsafe import escape
f'<h1>{escape(title)}</h1>'

5. requests imported inside route functions

def auth_facebook_callback():
    import requests as req

This is repeated across all three OAuth callbacks. Move to top-level imports — if requests isn't installed, you want a startup error, not a silent 500 at callback time.

---

🟡 Medium Issues

6. int(request.args.get('limit', 20)) — no validation

?limit=abc throws an unhandled ValueError → 500. Fix:

try:
    limit = min(int(request.args.get('limit', 20)), 100)
except (ValueError, TypeError):
    limit = 20

7. Silent exception swallowing throughout

Multiple routes do except Exception: posts = [] or except Exception: profile = {}. At minimum, log before swallowing:

except Exception as e:
    app.logger.exception("Failed to load post history for user %s", uid)
    return jsonify({'success': False, 'error': 'Server error', 'posts': []})

8. OAuth state stored under one shared key

All three OAuth flows write to session['oauth_state']. If a user opens two OAuth tabs simultaneously, the second overwrites the first's nonce. Namespace it:

session[f'oauth_state_{platform}']

9. No input validation on captions or image URLs

data.get('caption', '') passes directly to the publisher with no length check or URL validation. Instagram caps captions at 2,200 characters; sending beyond silently fails at the API level.

10. _uid() falls back to 'default'

def _uid() -> str:
    return current_user.id if current_user.is_authenticated else 'default'

All unauthenticated state gets bucketed under 'default'. Audit every caller of _get_tokens() — or remove the fallback and let it raise so unauthenticated access is explicit.

11. api_setup_tokens accepts raw token strings from the request body

This bypasses the OAuth flow. If intentional, add an admin-only guard. If it's leftover scaffolding, remove it.

---

🟠 Architecture / Tech Debt

12. app.py is ~1,000 lines and owns ~50 routes

Needs to be split into Flask blueprints:

blueprints/
  auth.py        # login, register, logout, OAuth flows
  billing.py     # Stripe routes
  api.py         # /api/* endpoints
  website.py     # website hub + public renderer
  webhooks.py    # stripe_webhook

13. No migration system

Schema bootstrapped via raw CREATE TABLE IF NOT EXISTS in init_db(). When you add a column, existing databases won't get the change. Add Alembic or Flask-Migrate before you have real users.

14. Synchronous HTTP in request handlers

Every third-party API call is a blocking requests.get() inside a Flask route. On Railway with single-threaded gunicorn, one slow OAuth callback ties up the entire server. Use -w 4 gunicorn workers or move token exchanges to a task queue (Celery/RQ).

15. Flask-Limiter defaults to memory storage

In production with multiple gunicorn workers, storage_uri='memory://' means each worker has its own rate limit counter — effectively giving users 5 × N login attempts where N = worker count. Add REDIS_URL to .env.example and note it's required in production.

16. Dual token storage paths

The users table has a platform_tokens TEXT column, but auth_manager.py also has its own token store (separate tokens table). Which is the source of truth? This will produce stale-token bugs that are nearly impossible to reproduce.

---

🔵 Minor / Polish

• README Quick Start references social-media-post-generator.git as the clone URL — should be Post-Pilot.git
• WTF_CSRF_TIME_LIMIT = 3600 — forms left open for 1 hour will fail silently; consider 7200 or session-scoped
• post_history stores platforms as a JSON string in a TEXT column — should be a junction table or native JSON column on Postgres
• No FLASK_ENV or APP_ENV in .env.example — new devs won't know how to toggle dev mode
• GitHub Actions workflow should be audited to confirm the tests/ directory is actually exercised in CI

---

Priority Fix Order

Priority	Issue	Effort
P0	api_publish bypasses plan limit check	5 min
P0	init_scheduler() at module level → duplicate fires	30 min
P0	XSS in fallback site renderer	10 min
P1	Dual DB write path (SQLite vs Postgres)	1–2 days
P1	Silent exception swallowing → add logging	2 hrs
P1	limit param not validated → 500	15 min
P1	OAuth state key collision across flows	30 min
P2	Split app.py into blueprints	1 day
P2	Add Alembic migrations	1 day
P2	Add Redis URL to .env.example	10 min
P3	Move requests imports to top level	15 min
P3	Input validation on captions/URLs	1 hr

---

Bottom line: The security skeleton is better than most early-stage SaaS — OAuth nonces, CSRF, rate limiting, and plan gating are all in place. The biggest risks are the billing bypass on /api/publish, the scheduler firing on every worker, and the dual DB path that will silently break post history on Postgres. Knock out the P0s this week and the app is safe to pilot with real users.

[ShadowWalkerNC]

Post-Pilot is now pointed at Postgres via DATABASE_URL, but auth is still partially blocked by schema drift in modules/user_manager.py and related code. app.py already expects PostgreSQL and modules/db.py now sets search_path=pp, but user_manager.py still bootstraps SQLite-era tables/columns like display_name, subscription_tier, business_profiles, and post_history, while the new fresh Postgres schema uses pp.users with columns such as full_name, business_name, and plan. Next recommended work: refactor modules/user_manager.py to match the pp schema, remove SQLite-specific exception handling/placeholders assumptions, and verify blueprints/auth.py register/login routes still align with the updated User model and create/load flows.