🔍 Senior Dev Audit — Security, Architecture & Tech Debt

[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 something like <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. It works but defeats module-level import caching and is a code smell. More importantly, if requests ever isn't installed, you get a silent 500 at callback time instead of a startup error. Move to top-level imports.

---

🟡 Medium Issues

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

In api_post_history:

limit = min(int(request.args.get('limit', 20)), 100)

?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 = {}. These make production debugging a nightmare. 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 and one flow will fail validation. Fix:

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, no sanitization, no URL validation on image_url. Instagram caps captions at 2,200 characters; sending beyond that silently fails at the API level. Add validation before hitting the publisher.

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'. Any route that calls _get_tokens() without @login_required would leak tokens across users. Audit every caller — 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 and lets a client inject arbitrary tokens. If intentional, document it and 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 a column is added to post_history or users, existing databases won't get the change. Add Alembic or Flask-Migrate before any real users are onboarded.

14. Synchronous HTTP in request handlers

Every third-party API call (Facebook, Google, TikTok token exchanges) is a blocking requests.get() inside a Flask route. On Railway with a single-threaded gunicorn config, one slow OAuth callback ties up the entire server. Switch to multi-worker gunicorn (-w 4) or move token exchanges to a task queue.

15. Flask-Limiter defaults to memory storage

In production with multiple gunicorn workers, storage_uri='memory://' means each worker has its own counter. A user effectively gets 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: platform_tokens TEXT column + auth_manager token store

The users table has a platform_tokens TEXT column, but auth_manager.py also has its own token storage. If both get written, you'll get stale-token bugs that are nearly impossible to reproduce. Pick one source of truth.

---

🔵 Minor / Polish

• README Quick Start references social-media-post-generator.git as the clone URL but the actual repo is Post-Pilot.git — will confuse first-time contributors
• WTF_CSRF_TIME_LIMIT = 3600 — forms left open for 1 hr 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 JSON column type on Postgres
• No FLASK_ENV / APP_ENV in .env.example — new devs won't know how to toggle dev mode
• tests/ directory exists but CI workflow should be audited to confirm tests actually run on push

---

Priority Fix Order

Priority	Issue	Est. 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 on bad input	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/image 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.