remove Qodana, disable XML-RPC#39
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe PR removes the Qodana static analysis CI workflow and configuration while simultaneously adding a security gate to the XML-RPC endpoint that requires an explicit configuration flag to enable it. The endpoint remains disabled by default. ChangesQodana CI Removal
XML-RPC Security Gate
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. Review rate limit: 0/1 reviews remaining, refill in 44 minutes and 26 seconds.Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #39 +/- ##
============================================
- Coverage 19.23% 18.12% -1.11%
- Complexity 7584 7840 +256
============================================
Files 621 665 +44
Lines 40085 42897 +2812
============================================
+ Hits 7709 7774 +65
- Misses 32376 35123 +2747 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Removes Qodana-related configuration from the repository and makes the legacy XML-RPC endpoint opt-in (disabled by default) via a new xoopsconfig.php flag, improving the default security posture.
Changes:
- Removed Qodana configuration (
qodana.yaml) and the GitHub Actions Qodana workflow. - Documented a new
xmlrpc_enabledconfig flag in the defaultxoopsconfig.dist.phptemplate. - Added a guard in
htdocs/xmlrpc.phpto return HTTP 403 unlessxmlrpc_enabledis explicitly enabled.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| qodana.yaml | Removes repository-level Qodana linter configuration. |
| .github/workflows/qodana.yml | Removes the Qodana GitHub Actions workflow job. |
| htdocs/xoops_data/configs/xoopsconfig.dist.php | Documents the new opt-in xmlrpc_enabled setting in the distributed config template. |
| htdocs/xmlrpc.php | Blocks XML-RPC requests unless enabled via config. |
| * | ||
| * To enable, uncomment this line in XOOPS_VAR_PATH . '/configs/xoopsconfig.php': | ||
| */ | ||
| // 'xmlrpc_enabled' => true, |
| header('HTTP/1.1 403 Forbidden'); | ||
| header('Content-type: text/plain'); | ||
| exit('XML-RPC is disabled. To enable it, set \'xmlrpc_enabled\' => true in your xoopsconfig.php'); |
![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 the current code and only fix it if needed.
Inline comments:
In `@htdocs/xmlrpc.php`:
- Around line 20-32: The PHP gate that checks $xoopsConfig['xmlrpc_enabled']
runs after including mainfile.php (causing full bootstrap); to prevent expensive
work for disabled endpoints move the gate to the very top of xmlrpc.php before
the include __DIR__ . '/mainfile.php' and implement a lightweight check (either
read XOOPS_VAR_PATH/configs/xoopsconfig.php with a simple `@include` to inspect
the 'xmlrpc_enabled' value or consult an environment variable/flag file) and
exit with the same 403 headers if disabled; keep the existing $xoopsConfig-based
check as a fallback after mainfile.php for defense-in-depth and preserve the
same header/exit behavior.
- Around line 29-30: Replace the raw status header and plain content-type
header: remove the manual header('HTTP/1.1 403 Forbidden') and instead call
http_response_code(403), and change header('Content-type: text/plain') to
include a charset (e.g., header('Content-Type: text/plain; charset=utf-8')) so
SAPI differences are handled and the response declares its encoding; update the
code around the existing header('HTTP/1.1 403 Forbidden') and
header('Content-type: text/plain') occurrences accordingly.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c18c45a7-d3a4-4534-b913-e9f7724e69cb
📒 Files selected for processing (4)
.github/workflows/qodana.ymlhtdocs/xmlrpc.phphtdocs/xoops_data/configs/xoopsconfig.dist.phpqodana.yaml
💤 Files with no reviewable changes (2)
- qodana.yaml
- .github/workflows/qodana.yml
| include __DIR__ . '/mainfile.php'; | ||
|
|
||
| /** | ||
| * XML-RPC is disabled by default for security. | ||
| * | ||
| * To enable it, add the following to XOOPS_VAR_PATH/configs/xoopsconfig.php: | ||
| * 'xmlrpc_enabled' => true, | ||
| */ | ||
| if (empty($xoopsConfig['xmlrpc_enabled'])) { | ||
| header('HTTP/1.1 403 Forbidden'); | ||
| header('Content-type: text/plain'); | ||
| exit('XML-RPC is disabled. To enable it, set \'xmlrpc_enabled\' => true in your xoopsconfig.php'); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Gate fires after full XOOPS bootstrap — consider web-server-level blocking for hostile traffic.
The security check (line 28) executes only after mainfile.php completes, which includes database queries, session setup, and config loading. A flood of requests to this endpoint — even when disabled — will incur the full bootstrap cost per request.
If xmlrpc.php remains disabled in production, consider blocking it entirely at the web server layer (e.g., an <Files xmlrpc.php> deny rule in Apache or an location = /xmlrpc.php { deny all; } block in nginx). The PHP gate is a sound defence-in-depth fallback, but the web server rule eliminates bootstrap overhead entirely.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@htdocs/xmlrpc.php` around lines 20 - 32, The PHP gate that checks
$xoopsConfig['xmlrpc_enabled'] runs after including mainfile.php (causing full
bootstrap); to prevent expensive work for disabled endpoints move the gate to
the very top of xmlrpc.php before the include __DIR__ . '/mainfile.php' and
implement a lightweight check (either read
XOOPS_VAR_PATH/configs/xoopsconfig.php with a simple `@include` to inspect the
'xmlrpc_enabled' value or consult an environment variable/flag file) and exit
with the same 403 headers if disabled; keep the existing $xoopsConfig-based
check as a fallback after mainfile.php for defense-in-depth and preserve the
same header/exit behavior.
use http_response_code, declare charset, clarify config path" -m "Address PR XOOPS#39 review feedback:`n- Replace raw 'HTTP/1.1 403 Forbidden' header with http_response_code(403) for SAPI portability (module/CGI/FPM)`n- Add charset=UTF-8 to Content-Type`n- Point disabled-message to XOOPS_VAR_PATH/configs/xoopsconfig.php for unambiguous remediation`n- Align xmlrpc_enabled comment style with surrounding entries (no space after //) in xoopsconfig.dist.php
|
Summary by CodeRabbit
Security
Chores