fix: honour console URL scheme in VCS commit statuses and authorize link - #7
Conversation
Source PR: appwrite#13476 Source head: 31fbb49
✅ Shipwright · ApproveRecommendation: approve PR #7 · Tier
Findings (6)
Fireworks usage: 6,832 input · 777 output · 7,609 total tokens · $0.0020 · 13s · 0 fix iteration(s) Open the Shipwright check for full evidence and the audit bundle. Use |
![shipwright-agent[bot]](https://avatars.githubusercontent.com/u/140162738?s=60&v=4){kind=small}
| $targetUrl = "{$protocol}://{$hostname}/console/project-{$region}-{$project->getId()}/{$segment}"; | ||
| $collection = $isSite ? 'sites' : 'functions'; | ||
| $type = $isSite ? 'site' : 'function'; | ||
| $targetUrl = System::getEnv('_APP_CONSOLE_URL_SCHEME', 'legacy') !== 'root' |
There was a problem hiding this comment.
Shipwright · HIGH
The URL construction logic is duplicated in three places with subtle differences (GitAction.php and two locations in Deployment.php).
Impact: The URL construction logic is duplicated in three places with subtle differences (GitAction.php and two locations in Deployment.php). The 'root' vs 'legacy' scheme check is repeated inline each time. A future maintainer changing one location will likely miss the others, causing inconsistent commit-status URLs across code paths.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| $hostname = $platform['consoleHostname'] ?? ''; | ||
|
|
||
| $authorizeUrl = $protocol . '://' . $hostname . "/console/git/authorize-contributor?projectId={$projectId}&installationId={$installationId}&repositoryId={$repositoryId}&providerPullRequestId={$providerPullRequestId}"; | ||
| $authorizeUrl = System::getEnv('_APP_CONSOLE_URL_SCHEME', 'legacy') !== 'root' |
There was a problem hiding this comment.
Shipwright · HIGH
The authorize-contributor URL in the root scheme drops the '/console' prefix but still carries sensitive query parameters (projectId, installationId, repositoryId, providerPullRequ
Impact: The authorize-contributor URL in the root scheme drops the '/console' prefix but still carries sensitive query parameters (projectId, installationId, repositoryId, providerPullRequestId) in a GET URL. If this URL is logged by intermediaries, browsers, or the VCS provider, these identifiers are exposed. The legacy path had the same exposure, but the new root path may bypass any console-level access controls or middle…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| $hostname = $platform['consoleHostname'] ?? ''; | ||
|
|
||
| $authorizeUrl = $protocol . '://' . $hostname . "/console/git/authorize-contributor?projectId={$projectId}&installationId={$installationId}&repositoryId={$repositoryId}&providerPullRequestId={$providerPullRequestId}"; | ||
| $authorizeUrl = System::getEnv('_APP_CONSOLE_URL_SCHEME', 'legacy') !== 'root' |
There was a problem hiding this comment.
Shipwright · HIGH
The root-scheme authorize URL '/git/authorize-contributor' is a new top-level route.
Impact: The root-scheme authorize URL '/git/authorize-contributor' is a new top-level route. If the console's routing or middleware previously enforced authentication/CSRF on '/console/' paths, moving this endpoint to '/git/' may expose it without those protections, allowing unauthenticated or cross-site requests to trigger authorization flows.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
| $targetUrl = "{$protocol}://{$hostname}/console/project-{$region}-{$project->getId()}/{$segment}"; | ||
| $collection = $isSite ? 'sites' : 'functions'; | ||
| $type = $isSite ? 'site' : 'function'; | ||
| $targetUrl = System::getEnv('_APP_CONSOLE_URL_SCHEME', 'legacy') !== 'root' |
There was a problem hiding this comment.
Shipwright · LOW
The magic string 'legacy' as the default for _APP_CONSOLE_URL_SCHEME is not documented or defined as a constant.
Impact: The magic string 'legacy' as the default for _APP_CONSOLE_URL_SCHEME is not documented or defined as a constant. A reader cannot tell what other values are valid ('root' is the only one checked) or what happens if an unexpected value is set — it silently falls back to legacy behavior.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
1 participant
What does this PR do?
The VCS PR comment's build-log links already respected
_APP_CONSOLE_URL_SCHEME, but two other links generated for the same pull request did not:GitAction.php, and the "Starting..." status inGitHub/Deployment.php)GitHub/Deployment.php)Both were hardcoded to the legacy
/console/project-{region}-{projectId}/...form. On a root-scheme install that meant a single PR carried contradictory links — the comment's "View Logs" pointed at/projects/{id}/...while the commit status next to it pointed at/console/project-....All three now branch on the same flag.
GitActionalso reads the hostname from the$platformconfig it already receives, instead of re-resolving_APP_CONSOLE_DOMAIN/_APP_DOMAINitself.Resulting URLs under
_APP_CONSOLE_URL_SCHEME=root/projects/{projectId}/sites/{siteId}/projects/{projectId}/functions/{functionId}/projects/{projectId}/sites/{siteId}/deployments/{deploymentId}/projects/{projectId}/functions/{functionId}/deployments/{deploymentId}/git/authorize-contributor?…The
legacybranch is unchanged, so existing installs keep their current links.Deployment note
Cloud does not currently set
_APP_CONSOLE_URL_SCHEME, so production and staging default tolegacyand are unaffected by this PR. Moving them to the new console needs_APP_CONSOLE_URL_SCHEME=rootplus the matching_APP_CONSOLE_DOMAIN(appwrite.io/staging.appwrite.io) in cloud-applications.Not changed here, but worth noting: the QR link (
/v1/avatars/qr) and the comment's status icons (/images/vcs/*) are also built on the console hostname. They are served by the Appwrite container, so that hostname has to route them to the API and not only to the console app.Test Plan
composer linton both changed files — passedcomposer analyze(PHPStan level 4) on both changed files — no errorstests/unit/Vcs/CommentTest.php— 4/4 passingRelated PRs and Issues
n/a
Have you read the Contributing Guidelines on issues?
Yes
🤖 Generated with Claude Code
Source merge-base:
4729c7ba45727a377d32d363e03efe7f7e979324Source head:
31fbb49352709abb066062117bdc35f8854ca8fe