Security hardening: remove committed session tokens, fix open redirect and debug server, validate input - #5
Open
devin-ai-integration[bot] wants to merge 1 commit into
Conversation
…t and debug server, validate input
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Security scan of the backend, frontend, git history, and dependencies. This PR fixes the code-level issues; two findings need action outside the repo (see below).
Highest-impact fixes
Committed OAuth tokens removed.
flask_session/(13 files) andflask_session_data/(2 files) were tracked and several contained liveaccess_token/refresh_token/spotify_user_idvalues. Untracked here and added to.gitignore(plus.env.*,*.db). The blobs still exist in git history, so app access should be revoked in Spotify.Debugger no longer on by default.
run.pyranapp.run(host='0.0.0.0', debug=True); the Werkzeug debugger is an RCE console. NowFLASK_DEBUG(default off),FLASK_RUN_HOST(default127.0.0.1),FLASK_RUN_PORT.Open redirect in the OAuth flow.
/login?redirect=https://evil.example/xwas stored in the session and used verbatim as the post-/callbackredirect. Now allowlisted:Applied both when storing (
/login) and when consuming (/callback), and the OAuthstateis now compared withsecrets.compare_digestand popped after use.FLASK_SECRET_KEYfallback.SECRET_KEY = os.getenv(...) or secrets.token_hex(32)meant a production deploy without the env var silently got a per-process random key. Now raises whenFLASK_ENV=production; ephemeral in dev only.Expired tokens were used, never refreshed. Playlist routes only checked
'access_token' in session. Added a shared helper used by every authenticated route:Input validation on playlist routes (
routes/playlist.py):playlistLengthclamped to1..MAX_PLAYLIST_LENGTH(wasint(user_value), so10**9drove AI + Spotify request volume),urismust match^spotify:track:[A-Za-z0-9]{22}$and number ≤ 100, cover image must be valid base64 ≤ 256KB,q/name/descriptiontruncated, and a globalMAX_CONTENT_LENGTHof 2MB.Bug fix that was also a 500-generator:
/Search_Trackand/Get_Playlistscalledrequests.get(...)with noimport requests— every hit raisedNameErrorand returned the exception string to the client.Error leakage:
str(e)and raw Spotify response bodies are no longer returned; they go tologger.exception()and clients get a generic message with a 502.CORS origin list moved to the
CORS_ORIGINSenv var (defaults unchanged), and outbound Spotify calls got 10–15s timeouts.Dependencies:
requests2.32.3→2.32.5,urllib32.3.0→2.5.0;npm audit fix(lockfile only, nopackage.jsonchanges) takes the frontend from 23 advisories (17 high) to 5.Needs your action (not fixable in this PR)
.envwith the realCLIENT_ID/CLIENT_SECRETwas committed in40e3f47and deleted in000c2cd— still readable from history. Rotate the Spotify client secret (andGENAI_API_KEY/FLASK_SECRET_KEYif ever committed).flask_session/are dead.Not fixed, deliberately
SameSite=None, which removes the implicit protection. Worth adding before a public deploy.vite5→6,react-router-dom6→7) — separate PR.legacy/still hasdebug=True, a'dev-secret-key'fallback, and debug prints of session dicts / token prefixes. It is dead code (not imported bybackend/); deleting the directory is the cleanest fix.eval/shell) and no exposed debug endpoints inbackend/.Testing
app.test_client()smoke test: unauthenticated/auth/status,/Create_Playlist,/Search_Track,/Playlist_Generatorall 401;/login?redirect=http://evil.com/stealredirects to Spotify with the session redirect coerced to the default; validation helpers reject oversized lengths, malformed URIs, and non-base64/oversized images. Note:npm run buildfails onmaintoo —src/lib/utilswas never committed — so the frontend build is unrelated pre-existing breakage.Link to Devin session: https://app.devin.ai/sessions/95fd45ae26db4cdcae838efb27e2dbef
Requested by: @praneels2005