Redact path-embedded Newznab credentials - #468
Conversation
|
Warning Review limit reached
Next review available in: 54 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?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 reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. 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, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughCredential redaction now handles Newznab-style ChangesGETNzb path credential redaction
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. 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 |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![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
🤖 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 `@repo/plugin.video.nzbdav/resources/lib/http_util.py`:
- Around line 160-161: Update the GETNzb handling in redact_url() so
_GETNZB_PATH_CRED_RE is applied only to matched URL path components, not the
entire free-form redacted string. Preserve unrelated URLs, ordinary text, and
query parameters, and add a regression test covering a message containing both a
GETNzb URL and unrelated content.
🪄 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: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 50fd7b21-c0b4-4b26-a4e3-e4adb44f9cf4
📒 Files selected for processing (2)
repo/plugin.video.nzbdav/resources/lib/http_util.pytests/test_http_util.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
repo/plugin.video.nzbdav/resources/lib/http_util.py (2)
44-53: LGTM!
104-107: LGTM!tests/test_http_util.py (2)
232-249: LGTM!
306-319: LGTM!
| if "getnzb" in redacted.lower() or ".nzb&" in redacted.lower(): | ||
| redacted = _GETNZB_PATH_CRED_RE.sub(r"\1=REDACTED", redacted) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Scope GETNzb redaction to URL paths.
Unlike redact_url(), this applies _GETNZB_PATH_CRED_RE to the entire free-form string once any getnzb marker is present. It therefore redacts unrelated &i=/&r= parameters in another URL, ordinary text, or even the query portion of the same URL. Apply the substitution only to matched URL paths and add a mixed-message regression test.
🤖 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 `@repo/plugin.video.nzbdav/resources/lib/http_util.py` around lines 160 - 161,
Update the GETNzb handling in redact_url() so _GETNZB_PATH_CRED_RE is applied
only to matched URL path components, not the entire free-form redacted string.
Preserve unrelated URLs, ordinary text, and query parameters, and add a
regression test covering a message containing both a GETNzb URL and unrelated
content.
1 participant
What changed
/getnzb/*.nzb&i=...&r=...URL pathsiandrparameters untouched outside getnzb-shaped pathsWhy
Some Newznab indexers return NZB download links without a
?, which makes account and API credentials part of the parsed URL path. Existing query-parameter redaction therefore leaves those values visible in logs and error messages.Validation
just lint: passed (Ruff, Black, Pylint 10.00/10, Python 3.8 Vermin gate)just test: 2,587 passed, 2 skipped; 3 unrelated existing Windows-environment/timing failures (test_install_nzbdav_addon_scriptand two completed-hint latency tests)