fix(security): Mandate content-type on POST calls (CVE-2025-53095)#656
Conversation
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (1)src/**/*.{cpp,c,h}⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (2)
Summary by CodeRabbit发布说明
Walkthrough在 src/confighttp.cpp 新增 confighttp::check_content_type，并在 saveApp、uploadCover、saveConfig、savePassword、savePin、unpair 等入口强制校验 Content-Type 为 application/json，校验失败返回 400 并中止处理。 ChangesHTTP请求Content-Type验证
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 `@src/confighttp.cpp`:
- Line 780: 几个 POST 处理入口仍然直接解析 JSON body，绕过了本 PR 新增的 Content-Type
强制校验；请在这些处理器的入口处（在实际读取或解析 request body 之前）调用 check_content_type(response,
request, "application/json") 并在返回 false 时立即 return，以阻止继续解析；具体需要修改的函数包括
confighttp::saveAiConfigEndpoint、confighttp::proxyAiChat、confighttp::renameClient、confighttp::testMenuCmd
以及注释中列出的其余入口（对应于 880、945、1212、1272、1295、1313、1384、1602、1621、1718 处），确保每个函数在任何
body 解析调用之前添加该检查。
- Line 880: The Content-Type check is being applied to endpoints that never read
request bodies (confighttp::deleteApp, confighttp::restart,
confighttp::resetDisplayDevicePersistence, confighttp::unpairAll,
confighttp::closeApp), causing valid empty POST/DELETE calls to return 400;
change the logic so you only enforce check_content_type(response, request,
"application/json") when a body is present (e.g., if request has a non-zero
Content-Length or request->content is non-empty) or skip the check entirely in
those handler functions; update the checks in the listed handler functions (and
the other similar locations referenced) to conditionally validate Content-Type
only when content exists.
🪄 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: CHILL
Plan: Pro
Run ID: 93a6afe5-dcac-462c-96be-4a3cdef1d48c
📒 Files selected for processing (1)
src/confighttp.cpp
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{cpp,c,h}
⚙️ CodeRabbit configuration file
src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码，自托管游戏串流服务器。审查要点：内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。
Files:
src/confighttp.cpp
|
Thank you for this PR. |
dkgkdfg65
force-pushed
the
port_family/backport/cve-2025-53095-master
branch
4 times, most recently
from
21c0978 to
4b438ea
Compare
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 `@src/confighttp.cpp`:
- Around line 147-165: The check_content_type function currently writes
plain-text 400 responses; update the two response->write calls inside
check_content_type to return the project JSON error contract and set the
Content-Type header to application/json: construct a JSON object containing
status_code (400), status ("bad_request" or similar consistent value), and error
("Content type not provided" / "Content type mismatch") and send that JSON
string as the body while adding or overriding response->header["content-type"] =
"application/json" before calling response->write so callers always receive the
standardized JSON error format.
🪄 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: CHILL
Plan: Pro
Run ID: 5974703d-37f2-4040-926a-7d72dd1ca09b
📒 Files selected for processing (1)
src/confighttp.cpp
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Windows
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{cpp,c,h}
⚙️ CodeRabbit configuration file
src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码，自托管游戏串流服务器。审查要点：内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。
Files:
src/confighttp.cpp
🔇 Additional comments (2)
src/confighttp.cpp (2)
865-865: 这些无请求体的端点仍会被新的校验直接打成 400。
deleteApp、restart、resetDisplayDevicePersistence、unpairAll和closeApp都没有读取request->content，现在却无条件要求Content-Type: application/json。只要现有调用方继续发空POST/DELETE，这些入口就会稳定失败；这里至少应在“确实有 body”时再校验，或直接跳过这类端点。Also applies to: 1257-1257, 1280-1280, 1587-1587, 1703-1703
765-766: LGTM!Also applies to: 930-931, 1197-1198, 1298-1299, 1369-1370, 1606-1607
Hand-ported subset of upstream LizardByte/Sunshine 738ac93: adds check_content_type() helper and calls it at the start of POST handlers for /api/apps, /api/apps/close, /api/apps/<id> (DELETE), /api/clients/unpair, /api/clients/unpair-all, /api/config, /api/covers/upload, /api/password, /api/pin, /api/restart, and /api/reset-display-device-persistence. AlkaidLab does not have upstreams bad_request() helper, so the bad-request response is inlined inside check_content_type() as a local lambda; the behavior matches upstream (HTTP 400 + JSON body with status_code/status/error). AlkaidLab-specific POST endpoints (AI/LLM proxy at /api/ai/config and /api/ai/chat/completions, QR pairing at /api/qr-pair{,/cancel}, client rename at /api/clients/rename, /api/apps/test-menu-cmd, /api/logout) are intentionally NOT covered by this PR; reviewer may extend the same check to those if desired. HTML-side Content-Type-on-fetch changes from upstream are skipped: AlkaidLab UI is heavily restructured for Chinese localization + AI UI and using a different request framework. (cherry picked from commit 738ac93) Signed-off-by: dkgkdfg65 <219107372+dkgkdfg65@users.noreply.github.com>
dkgkdfg65
force-pushed
the
port_family/backport/cve-2025-53095-master
branch
from
4b438ea to
b2f736b
Compare
|
@Yundi339 thanks — the backward-compat point is fair. just pushed an update that addresses that plus the CodeRabbit notes:
for the AI/LLM endpoints ( re Windows CI: i'm fairly sure that's the |
) Hand-ported subset of upstream LizardByte/Sunshine 738ac93: adds check_content_type() helper and calls it at the start of POST handlers for /api/apps, /api/apps/close, /api/apps/<id> (DELETE), /api/clients/unpair, /api/clients/unpair-all, /api/config, /api/covers/upload, /api/password, /api/pin, /api/restart, and /api/reset-display-device-persistence. AlkaidLab does not have upstreams bad_request() helper, so the bad-request response is inlined inside check_content_type() as a local lambda; the behavior matches upstream (HTTP 400 + JSON body with status_code/status/error). AlkaidLab-specific POST endpoints (AI/LLM proxy at /api/ai/config and /api/ai/chat/completions, QR pairing at /api/qr-pair{,/cancel}, client rename at /api/clients/rename, /api/apps/test-menu-cmd, /api/logout) are intentionally NOT covered by this PR; reviewer may extend the same check to those if desired. HTML-side Content-Type-on-fetch changes from upstream are skipped: AlkaidLab UI is heavily restructured for Chinese localization + AI UI and using a different request framework. (cherry picked from commit 738ac93) Signed-off-by: dkgkdfg65 <219107372+dkgkdfg65@users.noreply.github.com> Co-authored-by: dkgkdfg65 <219107372+dkgkdfg65@users.noreply.github.com>
3 participants
Hand-ported subset of upstream LizardByte/Sunshine 738ac93: adds check_content_type() helper and calls it at the start of POST handlers for /api/apps, /api/apps/close, /api/apps/ (DELETE), /api/clients/unpair, /api/clients/unpair-all, /api/config, /api/covers/upload, /api/password, /api/pin, /api/restart, and /api/reset-display-device-persistence.
AlkaidLab does not have upstreams bad_request() helper, so the bad-request response is inlined inside check_content_type() as a local lambda; the behavior matches upstream (HTTP 400 + JSON body with status_code/status/error).
AlkaidLab-specific POST endpoints (AI/LLM proxy at /api/ai/config and /api/ai/chat/completions, QR pairing at /api/qr-pair{,/cancel}, client rename at /api/clients/rename, /api/apps/test-menu-cmd, /api/logout) are intentionally NOT covered by this PR; reviewer may extend the same check to those if desired.
HTML-side Content-Type-on-fetch changes from upstream are skipped: AlkaidLab UI is heavily restructured for Chinese localization + AI UI and using a different request framework.
(cherry picked from commit 738ac93)