Repository navigation
fix(proxy): refuse the ExApp lifecycle endpoints like HaRP - #1168
Conversation
8841e2c to
d2d4d59
Compare
|
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1 … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title names the main change: the PHP ExApp proxy now refuses the ExApp lifecycle endpoints (/heartbeat, /init, /enabled) to match HaRP. The changeset adds this check in ExAppProxyController::prepa… Description check The description is directly related to the changeset. It explains that the PHP ExApp proxy forwarded /heartbeat, /init and /enabled paths, and that the change now returns 404 for them. It describes th… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title names the main change: the PHP ExApp proxy now refuses the ExApp lifecycle endpoints (/heartbeat, /init, /enabled) to match HaRP. The changeset adds this check in ExAppProxyController::prepareProxy, a CHANGELOG entry, and tests. The title is specific and accurate.
Full details: Description check
-
Explanation
The description is directly related to the changeset. It explains that the PHP ExApp proxy forwarded /heartbeat, /init and /enabled paths, and that the change now returns 404 for them. It describes the path normalization logic (percent-encoded ? and #, single decoding, case and slash handling) that the controller change implements, and it lists the tests added for refused and look-alike paths.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
🔧 PHPStan (2.3.0)
-
PHPStan reported 1 non-file analysis error(s); analysis is incomplete
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 help to get the list of available commands.
![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
-
-
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
59addb55-7215-42fc-9673-1b4f7bd8ceb8
-
CHANGELOG.mdlib/Controller/ExAppProxyController.phptests/php/Controller/ExAppProxyControllerTest.phptests/psalm-baseline.xml
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.
-
Signed-off-by: Oleksandr Piskun <oleksandr2088@icloud.com>
d2d4d59 to
b145bc3
Compare
HaRP refuses a user's request to the endpoints that only AppAPI calls on an ExApp (
/heartbeat,/initand/enabled), but the PHP ExApp proxy forwarded them whenever a route of the ExApp matched, for example a catch-all.*route. The proxy signs every request it forwards, so the ExApp ran its init or enabled handler for a request that did not come from AppAPI, for any user the route allowed.The proxy now answers 404 for these paths (HaRP drops the connection). It compares the path as the ExApp may read it: cut at a percent-encoded
?or#(the proxy forwards the path up to there), decoded once (FastAPI and other Python servers decode the path before routing), in any case and with slashes or newlines around it (Express matches/Init/to an/initroute by default; Starlette's route patterns end with$, which in Python also matches before a final newline). Dot segments are already refused by the check before it (#1163).An ExApp that probes its own
/heartbeatthrough the proxy, for monitoring or as a setup check, is refused now, as through HaRP; a monitor should call another public route of the ExApp instead, such as a/healthroute the ExApp may need to add.Changes
/heartbeat,/initand/enabledbefore it looks for a route.