Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
2 changes: 2 additions & 0 deletions config.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@
CRON_SECRET = os.getenv("CRON_SECRET", "")
ADMIN_SECRET = os.getenv("ADMIN_SECRET", "")
ADMIN_USER = os.getenv("ADMIN_USER", "admin")
# ADMIN_PASSWORD legacy plaintext removed from active auth flow; keep only for backward compatibility in env migration scripts
ADMIN_PASSWORD = os.getenv("ADMIN_PASSWORD", "")
ADMIN_PASSWORD_HASH = os.getenv("ADMIN_PASSWORD_HASH", "") # bcrypt hash
ALERT_EMAIL = os.getenv("ALERT_EMAIL", "") # where to send error alerts (set to your email)
BASE_URL = os.getenv("BASE_URL", "http://localhost:8000")

Expand Down
30 changes: 22 additions & 8 deletions main.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import hmac
import time
import collections
import bcrypt
from datetime import datetime, timedelta
from typing import List, Optional

Expand All @@ -28,7 +29,7 @@
from scheduler_jobs import send_due_emails, generate_upcoming, sync_to_sheets, advance_processing
from transcript import resolve_playlist
from mailer import send_email as do_send_email, send_welcome_email
from config import CRON_SECRET, ADMIN_SECRET, ADMIN_USER, ADMIN_PASSWORD, SENTRY_DSN, CORS_ORIGINS
from config import CRON_SECRET, ADMIN_SECRET, ADMIN_USER, ADMIN_PASSWORD, ADMIN_PASSWORD_HASH, SENTRY_DSN, CORS_ORIGINS

import sentry_sdk
from sentry_sdk.integrations.fastapi import FastApiIntegration
Expand Down Expand Up @@ -447,13 +448,26 @@ def _get_admin_token(request: Request) -> str:

def _check_admin(request: Request):
token = _get_admin_token(request)
if ADMIN_SECRET and token == ADMIN_SECRET:
if ADMIN_SECRET and hmac.compare_digest(token, ADMIN_SECRET):
return
Comment on lines +451 to 452
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

ADMIN_SECRET now has full-admin blast radius in the bookmarklet flow.

_check_admin() accepts ADMIN_SECRET as a universal admin bearer, and this PR now sends that same secret through a client-side bookmarklet flow. If the bookmarklet token leaks, every /api/admin/* endpoint is exposed, not just transcript ingest. Please split this into a dedicated ingest-only secret or stop treating ADMIN_SECRET as a global admin bypass.

Also applies to: 858-866

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@main.py` around lines 451 - 452, The current _check_admin() treats
ADMIN_SECRET as a universal admin bearer and the bookmarklet now sends that
secret; instead create a separate ingest-only secret (e.g., INGEST_SECRET) and
change the bookmarklet flow to send INGEST_SECRET instead of ADMIN_SECRET, then
modify _check_admin() (or wrap it with a new function used by ingest endpoints)
to only accept INGEST_SECRET for ingest routes and reserve ADMIN_SECRET for full
admin actions; update any places that call _check_admin() from ingest handlers
to perform the ingest-only check, and ensure /api/admin/* handlers that require
full admin still validate against ADMIN_SECRET.

if db.validate_admin_session(token):
return
raise HTTPException(401, "Unauthorized")


def _verify_admin_password(candidate: str) -> bool:
if not candidate:
return False

if not ADMIN_PASSWORD_HASH:
return False

try:
return bcrypt.checkpw(candidate.encode("utf-8"), ADMIN_PASSWORD_HASH.encode("utf-8"))
except (ValueError, TypeError):
return False


# ── Auth ──────────────────────────────────────────────────────────────────

class AdminLoginRequest(BaseModel):
Expand All @@ -479,9 +493,7 @@ def admin_login(req: AdminLoginRequest, request: Request):
raise HTTPException(429, "Too many login attempts. Try again in a minute.")
attempts.append(now)

if not ADMIN_PASSWORD:
raise HTTPException(503, "Admin password not configured")
if req.username != ADMIN_USER or not hmac.compare_digest(req.password, ADMIN_PASSWORD):
if req.username != ADMIN_USER or not _verify_admin_password(req.password):
raise HTTPException(401, "Invalid credentials")
token = db.create_admin_session()
db.prune_old_sessions()
Expand Down Expand Up @@ -839,12 +851,14 @@ def admin_edit_transcript(video_id: str, body: TranscriptBody, request: Request)
class IngestByYoutubeIdBody(BaseModel):
youtube_id: str
text: str
password: str
token: Optional[str] = None

@app.post("/api/admin/ingest-by-youtube-id")
def admin_ingest_by_youtube_id(body: IngestByYoutubeIdBody):
"""Bookmarklet endpoint: accepts youtube_id + transcript text + admin password."""
if not ADMIN_PASSWORD or not hmac.compare_digest(body.password, ADMIN_PASSWORD):
"""Bookmarklet endpoint: accepts youtube_id + transcript text + admin token."""
if not ADMIN_SECRET:
raise HTTPException(503, "Admin token is not configured")
if not body.token or not hmac.compare_digest(body.token, ADMIN_SECRET):
raise HTTPException(401, "Unauthorized")
text = body.text.strip()
if len(text) < 100:
Expand Down
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ google-auth
apscheduler
pytz
sentry-sdk[fastapi]
bcrypt