diff --git a/.ai/lessons.md b/.ai/lessons.md index 7bf7113805..f1c8347295 100644 --- a/.ai/lessons.md +++ b/.ai/lessons.md @@ -144,3 +144,6 @@ ## Keep `:has()` off page-wide ancestors - A `:has()` on `html`, `body`, `.app-shell`, or a sibling of `
` (for example `html:has(.x) .y`) makes Chrome restyle the page on every DOM change. Chart libraries read layout many times per render, so one ApexCharts render took ~400 ms and blocked navigation. Use a class set by the Blade layout instead, and keep `:has()` on small, local subjects. + +## Validate input safely +- In PCRE, `$` also matches before a final `\n`. In validation regexes, use `\z` or reject control characters first, or a trailing newline passes. diff --git a/app/Rules/ValidGitRepositoryUrl.php b/app/Rules/ValidGitRepositoryUrl.php index 8b2d484e54..e30fb8c22b 100644 --- a/app/Rules/ValidGitRepositoryUrl.php +++ b/app/Rules/ValidGitRepositoryUrl.php @@ -34,6 +34,22 @@ class ValidGitRepositoryUrl implements ValidationRule $value, ); + // Git decodes %XX sequences, so check the decoded URL. A '%' that is left after decoding + // is a malformed or double-encoded sequence and is rejected with the other characters. + $decodedValue = rawurldecode($valueWithoutIpv6Host); + + // Control characters (newline, null byte, tab) can inject git credential-helper lines (CVE-2020-5260) + if (preg_match('/[\x00-\x1F\x7F]/', $decodedValue)) { + Log::warning('Git repository URL validation failed - control character', [ + 'url' => $value, + 'ip' => request()->ip(), + 'user_id' => auth()->id(), + ]); + $fail('The :attribute contains invalid characters.'); + + return; + } + // Check for dangerous shell metacharacters that could be used for command injection $dangerousChars = [ ';', '|', '&', '$', '`', '(', ')', '{', '}', @@ -43,7 +59,7 @@ class ValidGitRepositoryUrl implements ValidationRule ]; foreach ($dangerousChars as $char) { - if (str_contains($valueWithoutIpv6Host, $char)) { + if (str_contains($decodedValue, $char)) { Log::warning('Git repository URL validation failed - dangerous character', [ 'url' => $value, 'character' => $char, @@ -70,7 +86,7 @@ class ValidGitRepositoryUrl implements ValidationRule ]; foreach ($dangerousPatterns as $pattern) { - if (preg_match($pattern, $valueWithoutIpv6Host)) { + if (preg_match($pattern, $decodedValue)) { Log::warning('Git repository URL validation failed - dangerous pattern', [ 'url' => $value, 'pattern' => $pattern, @@ -93,7 +109,7 @@ class ValidGitRepositoryUrl implements ValidationRule // Validate scp-style SSH URL format (user@host:user/repo.git) $scp = parseScpStyleGitUrl($value); - if ($scp === null || preg_match('/^(\[[0-9A-Fa-f:.]+\]|[a-zA-Z0-9.-]+)$/', $scp['host']) !== 1 || preg_match('/^[a-zA-Z0-9\-_\/.~]+$/', $scp['path']) !== 1) { + if ($scp === null || preg_match('/^(\[[0-9A-Fa-f:.]+\]|[a-zA-Z0-9.-]+)$/', $scp['host']) !== 1 || preg_match('/^[a-zA-Z0-9\-_\/.~%]+$/', $scp['path']) !== 1) { $fail('The :attribute is not a valid SSH repository URL.'); return; @@ -106,7 +122,7 @@ class ValidGitRepositoryUrl implements ValidationRule } // Validate ssh:// URL format (ssh://user@host[:port]/user/repo.git) - if (! preg_match('/^ssh:\/\/[a-zA-Z0-9._-]+@(\[[0-9A-Fa-f:.]+\]|[a-zA-Z0-9.-]+)(:[0-9]{1,5})?\/[a-zA-Z0-9\-_\/.~]+$/', $value)) { + if (! preg_match('/^ssh:\/\/[a-zA-Z0-9._-]+@(\[[0-9A-Fa-f:.]+\]|[a-zA-Z0-9.-]+)(:[0-9]{1,5})?\/[a-zA-Z0-9\-_\/.~%]+$/', $value)) { $fail('The :attribute is not a valid SSH repository URL.'); return; @@ -151,6 +167,13 @@ class ValidGitRepositoryUrl implements ValidationRule return; } + // Percent-encoding is only allowed in the path, because hosts and credentials can be decoded differently + if (str_contains(($parsed['host'] ?? '').($parsed['user'] ?? '').($parsed['pass'] ?? ''), '%')) { + $fail('The :attribute contains invalid characters.'); + + return; + } + // Ensure no query parameters or fragments if (! empty($parsed['query']) || ! empty($parsed['fragment'])) { $fail('The :attribute should not contain query parameters or fragments.'); @@ -160,14 +183,14 @@ class ValidGitRepositoryUrl implements ValidationRule // Validate path contains only safe characters $path = $parsed['path'] ?? ''; - if (! empty($path) && ! preg_match('/^[a-zA-Z0-9\-_\/\.@~]+$/', $path)) { + if (! empty($path) && ! preg_match('/^[a-zA-Z0-9\-_\/\.@~%]+$/', $path)) { $fail('The :attribute path contains invalid characters.'); return; } } elseif (str_starts_with($value, 'git://')) { // Validate git:// protocol URL (supports both git://host/path and git://host:port/path with tilde) - if (! preg_match('/^git:\/\/(\[[0-9A-Fa-f:.]+\]|[a-zA-Z0-9\.\-]+)(:[0-9]+)?[:\/][a-zA-Z0-9\-_\/\.~]+$/', $value)) { + if (! preg_match('/^git:\/\/(\[[0-9A-Fa-f:.]+\]|[a-zA-Z0-9\.\-]+)(:[0-9]+)?[:\/][a-zA-Z0-9\-_\/\.~%]+$/', $value)) { $fail('The :attribute is not a valid git:// URL.'); return; diff --git a/tests/Unit/ValidGitRepositoryUrlTest.php b/tests/Unit/ValidGitRepositoryUrlTest.php index 6ffa7e4a01..f216b51fe5 100644 --- a/tests/Unit/ValidGitRepositoryUrlTest.php +++ b/tests/Unit/ValidGitRepositoryUrlTest.php @@ -460,3 +460,100 @@ it('provides meaningful error messages', function () { expect($validator->errors()->first('url'))->toContain($testCase['expectedError']); } }); + +it('allows percent-encoded repository paths', function () { + $rule = new ValidGitRepositoryUrl; + + $validUrls = [ + 'https://dev.azure.com/org/My%20Project/_git/repo', + 'https://org@dev.azure.com/org/My%20Project/_git/My%20Repo', + 'https://org.visualstudio.com/My%20Project/_git/repo', + 'git@ssh.dev.azure.com:v3/org/My%20Project/repo', + 'ssh://git@ssh.dev.azure.com/v3/org/My%20Project/repo', + 'https://git.example.com/group/caf%C3%A9.git', + ]; + + foreach ($validUrls as $url) { + $validator = Validator::make(['url' => $url], ['url' => $rule]); + expect($validator->passes())->toBeTrue("Percent-encoded URL should be allowed: {$url}"); + } +}); + +it('validates repository URL formats of common git hosts', function () { + $rule = new ValidGitRepositoryUrl; + + $validUrls = [ + 'https://dev.azure.com/org/project/_git/repo', + 'https://org@dev.azure.com/org/project/_git/repo', + 'git@ssh.dev.azure.com:v3/org/project/repo', + 'https://org.visualstudio.com/DefaultCollection/project/_git/repo', + 'org@vs-ssh.visualstudio.com:v3/org/project/repo', + 'https://git-codecommit.us-east-2.amazonaws.com/v1/repos/MyDemoRepo', + 'ssh://APKAEIBAERJR2EXAMPLE@git-codecommit.us-east-2.amazonaws.com/v1/repos/MyDemoRepo', + 'https://bitbucket.example.com/scm/PROJ/repo.git', + 'ssh://git@bitbucket.example.com:7999/PROJ/repo.git', + 'https://gerrit.example.com/a/project', + 'ssh://user@gerrit.example.com:29418/project', + 'git@git.sr.ht:~user/repo', + 'https://gitlab.com/group/subgroup/subsubgroup/project.git', + 'https://gitea.example.com:3000/user/repo.git', + 'ssh://git@codeberg.org/user/repo.git', + ]; + + foreach ($validUrls as $url) { + $validator = Validator::make(['url' => $url], ['url' => $rule]); + expect($validator->passes())->toBeTrue("Git host URL should be allowed: {$url}"); + } +}); + +it('rejects malformed or dangerous percent-encoding', function () { + $rule = new ValidGitRepositoryUrl; + + $invalidUrls = [ + 'https://github.com/user/repo%', + 'https://github.com/user/repo%2', + 'https://github.com/user/repo%zz', + 'https://github.com/user/repo%0A', + 'https://github.com/user/repo%0d%0aprotocol=https', + 'https://github.com/user/repo%00', + 'https://github.com/user/repo%09', + 'https://github.com/user/repo%7F', + 'https://github.com/user/repo%3Bid', + 'https://github.com/user/%24%28whoami%29', + 'https://github.com/user/%60id%60', + 'https://github.com/user/%2e%2e%2fetc', + 'https://github.com/user/..%2Fetc', + 'https://github.com/user/repo%2527', + 'https://github.com/user/repo%27', + 'https://github.com/user/repo%3Fref=main', + 'https://git%68ub.com/user/repo', + 'https://us%40er@github.com/user/repo', + 'git@github.com:user/repo%0A', + 'git@github.com:user/%3Bid', + 'ssh://git@github.com/user/repo%0A', + 'ssh://git@github.com/user/%2e%2e%2frepo', + 'git://github.com/user/repo%0A', + ]; + + foreach ($invalidUrls as $url) { + $validator = Validator::make(['url' => $url], ['url' => $rule]); + expect($validator->fails())->toBeTrue("URL should be rejected: {$url}"); + } +}); + +it('rejects raw control characters', function () { + $rule = new ValidGitRepositoryUrl; + + $invalidUrls = [ + "ssh://git@github.com/user/repo.git\n", + "git@github.com:user/repo.git\n", + "git://github.com/user/repo.git\n", + "https://github.com/user/repo\t", + ]; + + foreach ($invalidUrls as $url) { + $validator = Validator::make(['url' => $url], ['url' => $rule]); + expect($validator->fails())->toBeTrue('URL with a control character should be rejected: '.json_encode($url)); + expect($validator->errors()->first('url'))->toContain('invalid characters'); + } +});