[oleksandr-nc]

The PHP ExApp proxy picked an ExApp's route on the path as Nextcloud passed it, but the ExApp's web server can read the forwarded path differently: Tomcat drops ; parameters, a percent-encoded ? or # ends the forwarded path, most servers decode a %XX that is still in it, Express ignores a trailing slash and the letter case, nginx and Go merge //, and URL parsers read a leading //x as a host. A request could then run the handler of a stricter route than the one the proxy checked.

The proxy now checks the routes on each of these readings, as HaRP 0.4.8 does. The route it picked before still decides, unless another reading of the path, or the path in another letter case, matches a route with a higher access level, which then applies. The /heartbeat, /init and /enabled endpoints are refused in every reading, and, as in HaRP, the dot-segment check also refuses dots followed by an escaped ;, ? or #, which a proxy or a wrapper ExApp that decodes the path turns into a parameter or the end of the path. The forwarded path does not change.

A few requests now need the stricter level, as through HaRP: for example img/ when ^/img/.* is public and .* needs a user, because Express serves it as /img. Paths that every server reads the same way are decided as before.

Changes

• exAppPathReadings() lists the path as checked and the ways ExApp servers read the forwarded one.
• The route of another reading or another letter case can only raise the access level.
• The lifecycle endpoint check covers every reading.
• The dot-segment check also refuses an escaped ;, ? or # after the dots, like HaRP.
• Tests for each reading.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• 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: c6e22c24-72c7-4110-b7ea-8f6c6b13de0d

Commits

Reviewing files that changed from the base of the PR and between 540a293 and b394a02.

Files selected for processing: 3

• CHANGELOG.md
• lib/Controller/ExAppProxyController.php
• tests/php/Controller/ExAppProxyControllerTest.php

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

Walkthrough

The ExApp proxy now checks reserved endpoints and dot segments across multiple readings of the forwarded path. It evaluates route matches across those readings, including case-sensitive and case-insensitive matches, and checks the selected route’s access level. Tests cover path decoding and normalization differences, stricter route selection, and case variants. The changelog records the change.

Severity of issue fixed: Medium

Merge Risk: Minimal · up to b394a

No merge-blocking issue was established; the change is ready for normal merge checks.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: checking proxy routes against each path reading, consistent with HaRP.
Description check		The description directly explains the path-reading differences, stricter route selection, lifecycle endpoint checks, dot-segment checks, and added tests.
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: Docstring Coverage

Explanation

Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 4 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.1)

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.

Share

• X
• Mastodon
• Reddit
• LinkedIn