Skip to content

fix: production deployment blockers and security gaps #23

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Copilot wants to merge 1 commit into from
Closed

fix: production deployment blockers and security gaps #23

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions backend/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ USER appuser
EXPOSE 8000

# Production Command: Run Alembic migrations, then start Gunicorn with Uvicorn workers
# -w 4: Use 4 worker processes (adjust based on CPU cores)
# -w ${GUNICORN_WORKERS:-2}: Worker count (default 2; set GUNICORN_WORKERS env var to tune)
# -k uvicorn.workers.UvicornWorker: Use async workers
# --bind 0.0.0.0:8000: Listen on all interfaces
CMD ["sh", "-c", "cd /app/backend && alembic upgrade head && cd /app && gunicorn -w 4 -k uvicorn.workers.UvicornWorker backend.app:app --bind 0.0.0.0:8000"]
CMD ["sh", "-c", "cd /app/backend && alembic upgrade head && cd /app && gunicorn -w ${GUNICORN_WORKERS:-2} -k uvicorn.workers.UvicornWorker backend.app:app --bind 0.0.0.0:8000"]
4 changes: 3 additions & 1 deletion backend/alembic.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ path_separator = os
# database URL. This is consumed by the user-maintained env.py script only.
# other means of configuring database URLs may be customized within the env.py
# file.
sqlalchemy.url = driver://user:pass@localhost/dbname
# database URL is set at runtime by migrations/env.py from the DATABASE_URL env var.
# Do NOT set it here; the value below is intentionally blank.
sqlalchemy.url =


[post_write_hooks]
Expand Down
17 changes: 8 additions & 9 deletions backend/routes/posts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,15 @@
publish_limiter = None

try:
from middleware.clerk_auth import get_current_user
from middleware.clerk_auth import get_current_user, require_auth
except ImportError:
logger.error("clerk_auth_import_failed", detail="Authentication middleware unavailable - all authenticated endpoints will reject requests")
async def get_current_user():
"""Fallback that rejects all requests when auth middleware is unavailable."""
raise HTTPException(status_code=503, detail="Authentication service unavailable")
async def require_auth():
"""Fallback that rejects all requests when auth middleware is unavailable."""
raise HTTPException(status_code=503, detail="Authentication service unavailable")

from services.db import get_database
from repositories.posts import PostRepository
Expand Down Expand Up @@ -157,11 +160,11 @@ class BatchGenerateRequest(BaseModel):
@router.post("/generate-preview")
async def generate_preview(
req: GenerateRequest,
current_user: dict = Depends(get_current_user)
current_user: dict = Depends(require_auth)
):
"""Generate an AI post preview from context.

Rate limited to 10 requests per hour per user to prevent abuse.
Requires authentication. Rate limited to 10 requests per hour per user to prevent abuse.

Supports multiple AI providers with tier enforcement:
- Free tier: Always routes to Groq (fast, free)
Expand All @@ -172,12 +175,8 @@ async def generate_preview(
if not generate_post_with_ai:
raise HTTPException(status_code=503, detail="AI service not available")

Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This endpoint now ignores req.user_id to prevent impersonation, but the request schema (GenerateRequest) still accepts user_id and tests/clients may continue sending it. Silent ignoring can mask client bugs and makes the API contract ambiguous. Consider removing user_id from GenerateRequest for this endpoint, or explicitly rejecting requests that include user_id (or that include a value different from the authenticated user) with a 400/403.

Suggested change
# Validate that any client-supplied user_id matches the authenticated user
req_user_id = getattr(req, "user_id", None)
if req_user_id is not None and req_user_id != current_user["user_id"]:
logger.warning(
"user_id_mismatch_in_request",
requested_user_id=str(req_user_id),
auth_user_id=str(current_user["user_id"]),
)
raise HTTPException(status_code=403, detail="user_id in request does not match authenticated user")

Copilot uses AI. Check for mistakes.
# Use authenticated user_id if available, otherwise fall back to request body
user_id = None
if current_user and current_user.get("user_id"):
user_id = current_user["user_id"]
elif req.user_id:
user_id = req.user_id
# Use the authenticated user's ID only — req.user_id is ignored to prevent impersonation
user_id = current_user["user_id"]
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Authentication/impersonation behavior changed here (authenticated user ID is now enforced and request-body user_id is ignored), but the backend tests for /api/post/generate-preview still send user_id in the JSON and don’t assert that mismatched user_id is rejected/ignored. Add/adjust tests to cover: (1) providing a different user_id than the authenticated user results in 400/403 (or is rejected), and (2) request succeeds without requiring a body user_id.

Suggested change
user_id = current_user["user_id"]
user_id = current_user["user_id"]
# If the request body includes a user_id, ensure it matches the authenticated user
request_user_id = getattr(req, "user_id", None)
if request_user_id is not None and request_user_id != user_id:
logger.warning(
"user_id_mismatch_in_generate_preview_request",
authenticated_user_id=(user_id[:8] if isinstance(user_id, str) else user_id),
request_user_id=(request_user_id[:8] if isinstance(request_user_id, str) else request_user_id),
)
raise HTTPException(
status_code=403,
detail="user_id in request body does not match authenticated user",
)

Copilot uses AI. Check for mistakes.

# Rate limiting check (10 requests/hour for AI generation)
if RATE_LIMITING_ENABLED and post_generation_limiter and user_id:
Expand Down
4 changes: 3 additions & 1 deletion render.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,11 @@ services:
- key: NEXT_PUBLIC_API_URL
sync: false

# Clerk Authentication (public key)
# Clerk Authentication
- key: NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY
sync: false
- key: CLERK_SECRET_KEY
sync: false

# OAuth Redirect URI
- key: NEXT_PUBLIC_REDIRECT_URI
Expand Down
2 changes: 1 addition & 1 deletion services/ai_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
OPENAI_API_KEY = os.getenv('OPENAI_API_KEY', '')
ANTHROPIC_API_KEY = os.getenv('ANTHROPIC_API_KEY', '')
MISTRAL_API_KEY = os.getenv('MISTRAL_API_KEY', '')
GITHUB_USERNAME = os.getenv('GITHUB_USERNAME', 'cliff-de-tech')
GITHUB_USERNAME = os.getenv('GITHUB_USERNAME', '')
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GITHUB_USERNAME is now defaulted to an empty string, but this constant is not referenced anywhere else in ai_service.py (only defined). Keeping an unused config value here is confusing and can drift from the real source of truth (e.g., settings/env usage elsewhere). Consider removing it from this module entirely, or wiring it into the code path that actually needs it.

Suggested change
GITHUB_USERNAME = os.getenv('GITHUB_USERNAME', '')

Copilot uses AI. Check for mistakes.

# Model configurations
GROQ_MODEL = "llama-3.3-70b-versatile"
Expand Down
3 changes: 2 additions & 1 deletion services/encryption.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@
# SECURITY: Determine environment mode
# - "production" = strict mode, encryption required
# - "development" or unset = permissive mode for local dev
ENV = os.getenv('ENV', 'development').lower()
# Check both ENV and ENVIRONMENT (render.yaml/docker-compose set ENVIRONMENT=production)
ENV = os.getenv('ENV', os.getenv('ENVIRONMENT', 'development')).lower()
IS_PRODUCTION = ENV == 'production'

# Load encryption key from environment
Expand Down
4 changes: 4 additions & 0 deletions web/next.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
const nextConfig = {
reactStrictMode: true,

// Required for Docker standalone deployment (copies server.js + minimal deps)
output: 'standalone',

// Enable gzip compression
compress: true,

Expand Down Expand Up @@ -42,6 +45,7 @@ const nextConfig = {
{ key: 'X-Content-Type-Options', value: 'nosniff' },
{ key: 'Referrer-Policy', value: 'origin-when-cross-origin' },
{ key: 'Permissions-Policy', value: 'camera=(), microphone=(), geolocation=()' },
{ key: 'Strict-Transport-Security', value: 'max-age=63072000; includeSubDomains; preload' },
{
key: 'Content-Security-Policy',
value: "default-src 'self'; script-src 'self' 'unsafe-eval' 'unsafe-inline' https://cdn.clerk.io https://*.clerk.accounts.dev; style-src 'self' 'unsafe-inline'; img-src 'self' data: https://images.unsplash.com https://media.licdn.com https://img.clerk.com; connect-src 'self' https://*.clerk.accounts.dev https://api.clerk.io " + (process.env.NEXT_PUBLIC_API_URL || 'http://localhost:8000') + "; font-src 'self'; frame-src 'self' https://*.clerk.accounts.dev;"
Expand Down
Loading