fix: redirect anonymous commenters to login instead of crashing#256
Conversation
The comment POST handler in post() read session["username"] directly without checking it was set. Any unauthenticated POST to /post/<id> with a comment field (and no delete button fields) raised an unhandled KeyError / 500 instead of failing gracefully. Add the same not-logged-in guard already used for the post-delete and comment-delete paths in this codebase, redirecting to the login page with a return path, consistent with the &-delimited redirect convention used elsewhere (see login.py, edit_post.py).
|
Warning Review limit reached
More reviews will be available in 14 minutes and 31 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe comment POST handler in ChangesUnauthenticated Comment Guard
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 2
🤖 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/post.py`:
- Line 53: The session authentication check in the conditional statement is
using the incorrect key name "username" when it should use "userName" to align
with the established coding guidelines. Update the condition that checks `if
"username" not in session` to use the correct session key `"userName"` instead,
ensuring the authentication guard properly validates the required session state
key as per project standards.
- Around line 54-57: The Log.error call in the post route is logging
request.remote_addr and url_id directly without sanitizing carriage return and
line feed characters, which can allow log forging attacks. Sanitize both
request.remote_addr and url_id by removing or replacing CR/LF characters before
interpolating them into the log message string, then use the sanitized values in
the Log.error call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
The not-logged-in-comment log line interpolated request.remote_addr and url_id directly into the message. Both are attacker-influenced (client IP / URL path segment), so an attacker could inject CR/LF characters and forge fake log lines. Add a small sanitize_for_log() helper that strips CR/LF before the values are written to the log, and use it for both fields. Addresses a CodeRabbit review comment on this PR.
New helper used by post.py to strip CR/LF from request-derived values before they're written to the log (CWE-117). See the previous commit on this branch for the call sites.
2 participants
The comment POST handler in post() read session['username'] directly without checking it was set. Any unauthenticated POST to /post/ with a comment field (and no delete button fields) raised an unhandled KeyError / 500 instead of failing gracefully.
Add the same not-logged-in guard already used for the post-delete and comment-delete paths in this codebase, redirecting to the login page with a return path, consistent with the &-delimited redirect convention used elsewhere (see login.py, edit_post.py).
Summary by CodeRabbit