chore(deps): consolidate dependency updates#263
Merged
Conversation
- Remove hardcoded SMTP credentials (CWE-798) - Require DEFAULT_ADMIN_PASSWORD from environment (CWE-1392) - Fix missing authorization check in admin panel (CWE-862) - Fix open redirect in login route (CWE-601) Co-Authored-By: Security Researcher <noreply@anthropic.com>
- flask: 3.1.2 → 3.1.3 - ruff: 0.15.0 → 0.15.4 - python-dotenv: 1.2.1 → 1.2.2 - filelock: 3.20.3 → 3.25.0 - markdown2: 2.5.4 → 2.5.5 - requests: 2.32.5 → 2.33.0 Consolidated from 6 dependabot PRs into single update PR.
📝 WalkthroughWalkthroughThe PR pins several runtime dependencies, removes hardcoded SMTP and default-admin credentials, sanitizes login redirect targets, and rejects non-admin access to the admin panel before the route continues. ChangesApplication hardening
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/pyproject.toml (1)
18-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove
ruffandfilelockout of the main dependency set.
ruffalready appears in thedevextra, andfilelockis only used fromtests/e2e/conftest.py; keeping either independenciesmakes production installs carry non-runtime packages.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/pyproject.toml` around lines 18 - 19, Move the non-runtime packages out of the main dependency list in the app project configuration: `ruff` should only remain under the `dev` extra, and `filelock` should be removed from `dependencies` since it is only used by `tests/e2e/conftest.py`. Update the dependency sections in `pyproject.toml` so production installs no longer pull in these packages, while keeping the existing development/test setup intact.app/routes/admin_panel_users.py (1)
28-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGood early authz guard; it leaves a dead branch downstream.
Rejecting non-admins before the
POSThandling at Line 34 is the right fix. As a side effect, the laterif user.role == "admin"at Line 49 is now always true, and itselseblock at Lines 76-81 is unreachable. Consider collapsing theif/elseto remove the dead branch for clarity.♻️ Suggested cleanup (lines 49, 76-81)
- if user.role == "admin": - query = User.query - users_objects, page, total_pages = paginate_query(query) + query = User.query + users_objects, page, total_pages = paginate_query(query) - users = [ + users = [ ( u.user_id, ... ) for u in users_objects - ] + ] - Log.info(f"Rendering admin_panel_users.html: params: users={len(users)}") + Log.info(f"Rendering admin_panel_users.html: params: users={len(users)}") - return render_template( + return render_template( "admin_panel_users.html", users=users, page=page, total_pages=total_pages, - ) - else: - Log.error( - f"{request.remote_addr} tried to reach user admin panel without being admin" - ) - - return redirect("/") + )Note: the
remote_addrlog here is sourced from the connection peer (an IP), so the static-analysis log-injection flag is not actionable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/routes/admin_panel_users.py` around lines 28 - 33, The early admin authz guard in the user admin panel makes the later role check in the POST handling redundant, so remove the now-dead branch from the logic in the user admin view. Update the control flow around the existing user role check in the admin panel handler so the `if user.role == "admin"` path is handled directly and the unreachable `else` block is deleted, keeping the `POST` logic clear and linear.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/routes/login.py`:
- Around line 37-42: The login redirect validation in the direct-target handling
allows backslash-based and scheme-relative targets to slip through because
urlparse() does not flag them as off-site. Update the redirect guard in the
login route’s direct redirect logic to reject any target containing backslashes
or starting with double slashes before reaching redirect(), alongside the
existing scheme/netloc checks, so only safe relative paths are accepted.
---
Nitpick comments:
In `@app/pyproject.toml`:
- Around line 18-19: Move the non-runtime packages out of the main dependency
list in the app project configuration: `ruff` should only remain under the `dev`
extra, and `filelock` should be removed from `dependencies` since it is only
used by `tests/e2e/conftest.py`. Update the dependency sections in
`pyproject.toml` so production installs no longer pull in these packages, while
keeping the existing development/test setup intact.
In `@app/routes/admin_panel_users.py`:
- Around line 28-33: The early admin authz guard in the user admin panel makes
the later role check in the POST handling redundant, so remove the now-dead
branch from the logic in the user admin view. Update the control flow around the
existing user role check in the admin panel handler so the `if user.role ==
"admin"` path is handled directly and the unreachable `else` block is deleted,
keeping the `POST` logic clear and linear.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6e116d55-7a6c-4922-979f-30cbbf9bacde
⛔ Files ignored due to path filters (1)
app/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
app/pyproject.tomlapp/routes/admin_panel_users.pyapp/routes/login.pyapp/settings.py
Comment on lines
+37
to
+42
| if direct: | ||
| parsed = urlparse(direct) | ||
| if parsed.netloc or parsed.scheme or not direct.startswith("/"): | ||
| direct = "/" | ||
| else: | ||
| direct = "/" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python3 - <<'PY'
from urllib.parse import urlparse
for t in ["/\\evil.com", "//evil.com", "/\\/\\evil.com", "javascript:alert(1)"]:
p = urlparse(t)
print(repr(t), "netloc=", repr(p.netloc), "scheme=", repr(p.scheme), "startswith/=", t.startswith("/"))
PYRepository: DogukanUrker/FlaskBlog
Length of output: 405
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
path = Path("app/routes/login.py")
print("FILE_EXISTS", path.exists())
print("LINE_COUNT", sum(1 for _ in path.open()))
with path.open() as f:
lines = f.readlines()
for start, end in [(1, 120)]:
print(f"\n--- {path} lines {start}-{end} ---")
for i in range(start-1, min(end, len(lines))):
print(f"{i+1:4d}: {lines[i].rstrip()}")
PYRepository: DogukanUrker/FlaskBlog
Length of output: 4187
Reject backslash-based redirect targets in app/routes/login.py:37-42. urlparse() leaves /\evil.com with empty netloc/scheme, so it passes this guard even though browsers can interpret the backslash as a slash and treat the redirect as off-site. Block \ (and //) before calling redirect().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/routes/login.py` around lines 37 - 42, The login redirect validation in
the direct-target handling allows backslash-based and scheme-relative targets to
slip through because urlparse() does not flag them as off-site. Update the
redirect guard in the login route’s direct redirect logic to reject any target
containing backslashes or starting with double slashes before reaching
redirect(), alongside the existing scheme/netloc checks, so only safe relative
paths are accepted.
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
1 participant
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.
Consolidated 6 dependabot PRs into a single PR for cleaner update history.
Changes
Both pyproject.toml and uv.lock have been updated to reflect these changes.
Summary by CodeRabbit
Bug Fixes
Chores