Conversation
There was a problem hiding this comment.
Pull request overview
Adds the ability for admins to create new VCS instances (GitHub/GitLab) through the web UI, wiring up backend validation + persistence and exposing the entry point from the repositories screen.
Changes:
- Added admin routes and a new
VcsInstanceControllerfor create/store. - Introduced a VCS instance creation page (Inertia/Vue) and associated TS types.
- Centralized VCS instance validation rules in the
VcsInstancemodel and addedVcsPlatform::getLabels()for UI options.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| routes/web.php | Adds admin resource routes for creating/storing VCS instances. |
| resources/js/types/index.d.ts | Adds a VcsInstance TS interface for the form payload. |
| resources/js/pages/vcsInstances/Create.vue | New UI page to create a VCS instance (platform, API URL, token/installation id). |
| resources/js/pages/repositories/Repositories.vue | Adds a button linking to the VCS instance create page. |
| app/Models/VcsInstance.php | Adds mass-assignable fields and validation rules for VCS instance creation. |
| app/Http/Requests/VcsInstances/VcsInstanceCreateRequest.php | Adds a FormRequest that uses the model’s validation rules. |
| app/Http/Controllers/VcsInstanceController.php | Implements create/store endpoints and persists VCS instances. |
| app/Enums/VcsPlatform.php | Adds getLabels() to provide platform labels for the UI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| return [ | ||
| 'name' => ['required', 'string', 'max:255'], | ||
| 'api_url' => ['required', 'url', 'max:255'], |
There was a problem hiding this comment.
api_url is later used to build outbound HTTP requests (e.g., GitHub/GitLab API calls). Validating with the generic url rule allows non-HTTP(S) schemes and potentially internal/loopback/private-network hosts, which can lead to misconfiguration and SSRF risk. Consider restricting to HTTPS and (if feasible) rejecting localhost/private IP ranges / non-public hosts at validation time.
| 'api_url' => ['required', 'url', 'max:255'], | |
| 'api_url' => [ | |
| 'required', | |
| 'url', | |
| 'max:255', | |
| /** | |
| * Ensure the API URL uses HTTP(S) and does not point to localhost or private/reserved IP ranges. | |
| * | |
| * @param string $attribute | |
| * @param mixed $value | |
| * @param callable $fail | |
| */ | |
| function ($attribute, $value, $fail): void { | |
| if (!is_string($value)) { | |
| $fail('The ' . $attribute . ' must be a valid URL.'); | |
| return; | |
| } | |
| $parts = parse_url($value); | |
| if ($parts === false || !isset($parts['scheme'], $parts['host'])) { | |
| $fail('The ' . $attribute . ' must be a valid URL.'); | |
| return; | |
| } | |
| $scheme = strtolower($parts['scheme']); | |
| if (!in_array($scheme, ['http', 'https'], true)) { | |
| $fail('The ' . $attribute . ' must use the http or https scheme.'); | |
| return; | |
| } | |
| $host = strtolower($parts['host']); | |
| // Explicitly disallow localhost. | |
| if ($host === 'localhost') { | |
| $fail('The ' . $attribute . ' must not point to localhost.'); | |
| return; | |
| } | |
| // If host is a literal IP address, ensure it is not private or reserved. | |
| if (filter_var($host, FILTER_VALIDATE_IP) !== false) { | |
| $publicIp = filter_var( | |
| $host, | |
| FILTER_VALIDATE_IP, | |
| FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE | |
| ); | |
| if ($publicIp === false) { | |
| $fail('The ' . $attribute . ' must not point to a private or reserved IP address.'); | |
| } | |
| } | |
| }, | |
| ], |
| 'name' => ['required', 'string', 'max:255'], | ||
| 'api_url' => ['required', 'url', 'max:255'], | ||
| 'token' => ['required_if:platform,gitlab', 'nullable', 'string'], | ||
| 'installation_id' => ['required_if:platform,github', 'nullable', 'string', 'max:255'], |
There was a problem hiding this comment.
installation_id is accepted as an arbitrary string, but it is used as a path parameter in GitHub API calls and is expected to be numeric. Allowing non-numeric values will pass validation but will fail at runtime when calling the GitHub API. Consider validating it as an integer/numeric value (and optionally enforcing a sensible range) when platform is GitHub.
| 'installation_id' => ['required_if:platform,github', 'nullable', 'string', 'max:255'], | |
| 'installation_id' => ['required_if:platform,github', 'nullable', 'integer', 'min:1', 'max:255'], |
78aadad to
95de5eb
Compare
95de5eb to
4a31bbb
Compare
4a31bbb to
2b77527
Compare
No description provided.