mirror of
https://github.com/appwrite/appwrite.git
synced 2026-05-26 13:51:13 +00:00
fix: sanitize branch names for valid domain generation
Branch names containing invalid domain characters (like '/') were being used directly when creating VCS preview domains, resulting in invalid domains like 'branch-abc/test.appwrite.network'. This adds a Domain helper class that sanitizes branch names by replacing invalid characters with hyphens before generating domains. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Hemachandar
co-authored by
Claude Opus 4.5
Claude Opus 4.5
parent
5e495aa2da
commit
3b96abf02a
@@ -13,6 +13,7 @@ use Appwrite\Utopia\Database\Validator\Queries\Installations;
|
||||
use Appwrite\Utopia\Request;
|
||||
use Appwrite\Utopia\Response;
|
||||
use Appwrite\Vcs\Comment;
|
||||
use Appwrite\Vcs\Domain;
|
||||
use Swoole\Coroutine\WaitGroup;
|
||||
use Utopia\App;
|
||||
use Utopia\CLI\Console;
|
||||
@@ -369,13 +370,7 @@ $createGitDeployments = function (GitHub $github, string $providerInstallationId
|
||||
|
||||
// VCS branch preview
|
||||
if (!empty($providerBranch)) {
|
||||
$branchPrefix = substr($providerBranch, 0, 16);
|
||||
if (strlen($providerBranch) > 16) {
|
||||
$remainingChars = substr($providerBranch, 16);
|
||||
$branchPrefix .= '-' . substr(hash('sha256', $remainingChars), 0, 7);
|
||||
}
|
||||
$resourceProjectHash = substr(hash('sha256', $resource->getId() . $project->getId()), 0, 7);
|
||||
$domain = "branch-{$branchPrefix}-{$resourceProjectHash}.{$sitesDomain}";
|
||||
$domain = Domain::generateBranchDomain($providerBranch, $resource->getId(), $project->getId(), $sitesDomain);
|
||||
$ruleId = md5($domain);
|
||||
try {
|
||||
$authorization->skip(
|
||||
|
||||
@@ -6,6 +6,7 @@ use Appwrite\Event\Build;
|
||||
use Appwrite\Extend\Exception;
|
||||
use Appwrite\Platform\Action;
|
||||
use Appwrite\Platform\Modules\Compute\Validator\Specification as SpecificationValidator;
|
||||
use Appwrite\Vcs\Domain;
|
||||
use Utopia\Config\Config;
|
||||
use Utopia\Database\Database;
|
||||
use Utopia\Database\Document;
|
||||
@@ -296,13 +297,7 @@ class Base extends Action
|
||||
|
||||
// VCS branch preview
|
||||
if (!empty($providerBranch)) {
|
||||
$branchPrefix = substr($providerBranch, 0, 16);
|
||||
if (strlen($providerBranch) > 16) {
|
||||
$remainingChars = substr($providerBranch, 16);
|
||||
$branchPrefix .= '-' . substr(hash('sha256', $remainingChars), 0, 7);
|
||||
}
|
||||
$resourceProjectHash = substr(hash('sha256', $site->getId() . $project->getId()), 0, 7);
|
||||
$domain = "branch-{$branchPrefix}-{$resourceProjectHash}.{$sitesDomain}";
|
||||
$domain = Domain::generateBranchDomain($providerBranch, $site->getId(), $project->getId(), $sitesDomain);
|
||||
$ruleId = md5($domain);
|
||||
try {
|
||||
$authorization->skip(
|
||||
|
||||
@@ -11,6 +11,7 @@ use Appwrite\Event\StatsUsage;
|
||||
use Appwrite\Event\Webhook;
|
||||
use Appwrite\Utopia\Response\Model\Deployment;
|
||||
use Appwrite\Vcs\Comment;
|
||||
use Appwrite\Vcs\Domain;
|
||||
use Exception;
|
||||
use Executor\Executor;
|
||||
use Swoole\Coroutine as Co;
|
||||
@@ -1038,13 +1039,7 @@ class Builds extends Action
|
||||
$branchName = $deployment->getAttribute('providerBranch');
|
||||
if (!empty($branchName)) {
|
||||
$sitesDomain = $platform['sitesDomain'];
|
||||
$branchPrefix = substr($branchName, 0, 16);
|
||||
if (strlen($branchName) > 16) {
|
||||
$remainingChars = substr($branchName, 16);
|
||||
$branchPrefix .= '-' . substr(hash('sha256', $remainingChars), 0, 7);
|
||||
}
|
||||
$resourceProjectHash = substr(hash('sha256', $resource->getId() . $project->getId()), 0, 7);
|
||||
$domain = "branch-{$branchPrefix}-{$resourceProjectHash}.{$sitesDomain}";
|
||||
$domain = Domain::generateBranchDomain($branchName, $resource->getId(), $project->getId(), $sitesDomain);
|
||||
$ruleId = md5($domain);
|
||||
|
||||
try {
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
<?php
|
||||
|
||||
namespace Appwrite\Vcs;
|
||||
|
||||
class Domain
|
||||
{
|
||||
/**
|
||||
* Maximum length for branch prefix in domain name
|
||||
*/
|
||||
public const BRANCH_PREFIX_MAX_LENGTH = 16;
|
||||
|
||||
/**
|
||||
* Length of hash suffix when branch name exceeds max length
|
||||
*/
|
||||
public const HASH_SUFFIX_LENGTH = 7;
|
||||
|
||||
/**
|
||||
* Sanitize a branch name for use in a domain name.
|
||||
* Replaces any characters that are not alphanumeric or hyphens with hyphens,
|
||||
* and removes leading/trailing hyphens.
|
||||
*
|
||||
* @param string $branch The branch name to sanitize
|
||||
* @return string The sanitized branch name
|
||||
*/
|
||||
public static function sanitizeBranchName(string $branch): string
|
||||
{
|
||||
// Replace any sequence of invalid characters with a single hyphen
|
||||
$sanitized = preg_replace('/[^a-zA-Z0-9-]+/', '-', $branch);
|
||||
// Remove leading and trailing hyphens
|
||||
return trim($sanitized, '-');
|
||||
}
|
||||
|
||||
/**
|
||||
* Generate a branch prefix for domain name from a branch name.
|
||||
* Takes up to 16 characters, sanitizes them for domain use,
|
||||
* and appends a hash suffix if the branch name is longer than 16 characters.
|
||||
*
|
||||
* @param string $branch The branch name
|
||||
* @return string The branch prefix for domain name
|
||||
*/
|
||||
public static function generateBranchPrefix(string $branch): string
|
||||
{
|
||||
$branchPrefix = substr($branch, 0, self::BRANCH_PREFIX_MAX_LENGTH);
|
||||
$branchPrefix = self::sanitizeBranchName($branchPrefix);
|
||||
|
||||
if (strlen($branch) > self::BRANCH_PREFIX_MAX_LENGTH) {
|
||||
$remainingChars = substr($branch, self::BRANCH_PREFIX_MAX_LENGTH);
|
||||
$branchPrefix .= '-' . substr(hash('sha256', $remainingChars), 0, self::HASH_SUFFIX_LENGTH);
|
||||
}
|
||||
|
||||
return $branchPrefix;
|
||||
}
|
||||
|
||||
/**
|
||||
* Generate a full branch preview domain name.
|
||||
*
|
||||
* @param string $branch The branch name
|
||||
* @param string $resourceId The resource ID (site or function)
|
||||
* @param string $projectId The project ID
|
||||
* @param string $sitesDomain The base sites domain
|
||||
* @return string The full domain name
|
||||
*/
|
||||
public static function generateBranchDomain(string $branch, string $resourceId, string $projectId, string $sitesDomain): string
|
||||
{
|
||||
$branchPrefix = self::generateBranchPrefix($branch);
|
||||
$resourceProjectHash = substr(hash('sha256', $resourceId . $projectId), 0, self::HASH_SUFFIX_LENGTH);
|
||||
|
||||
return "branch-{$branchPrefix}-{$resourceProjectHash}.{$sitesDomain}";
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,207 @@
|
||||
<?php
|
||||
|
||||
namespace Tests\Unit\Vcs;
|
||||
|
||||
use Appwrite\Vcs\Domain;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
|
||||
class DomainTest extends TestCase
|
||||
{
|
||||
/**
|
||||
* Test sanitizing branch names with various invalid characters
|
||||
*/
|
||||
public function testSanitizeBranchNameWithSlash(): void
|
||||
{
|
||||
$this->assertEquals('feature-test', Domain::sanitizeBranchName('feature/test'));
|
||||
$this->assertEquals('user-john-fix', Domain::sanitizeBranchName('user/john/fix'));
|
||||
$this->assertEquals('abc-test-235', Domain::sanitizeBranchName('abc/test-235'));
|
||||
}
|
||||
|
||||
public function testSanitizeBranchNameWithUnderscore(): void
|
||||
{
|
||||
$this->assertEquals('feature-test', Domain::sanitizeBranchName('feature_test'));
|
||||
$this->assertEquals('my-branch-name', Domain::sanitizeBranchName('my_branch_name'));
|
||||
}
|
||||
|
||||
public function testSanitizeBranchNameWithMultipleInvalidChars(): void
|
||||
{
|
||||
// Multiple consecutive invalid characters should become a single hyphen
|
||||
$this->assertEquals('feature-test', Domain::sanitizeBranchName('feature//test'));
|
||||
$this->assertEquals('feature-test', Domain::sanitizeBranchName('feature__test'));
|
||||
$this->assertEquals('feature-test', Domain::sanitizeBranchName('feature/_test'));
|
||||
}
|
||||
|
||||
public function testSanitizeBranchNameWithSpecialChars(): void
|
||||
{
|
||||
$this->assertEquals('feature-test', Domain::sanitizeBranchName('feature@test'));
|
||||
$this->assertEquals('feature-test', Domain::sanitizeBranchName('feature#test'));
|
||||
$this->assertEquals('feature-test', Domain::sanitizeBranchName('feature$test'));
|
||||
$this->assertEquals('feature-test', Domain::sanitizeBranchName('feature%test'));
|
||||
$this->assertEquals('feature-test', Domain::sanitizeBranchName('feature&test'));
|
||||
$this->assertEquals('feature-test', Domain::sanitizeBranchName('feature*test'));
|
||||
$this->assertEquals('feature-test', Domain::sanitizeBranchName('feature+test'));
|
||||
$this->assertEquals('feature-test', Domain::sanitizeBranchName('feature=test'));
|
||||
$this->assertEquals('feature-test', Domain::sanitizeBranchName('feature!test'));
|
||||
$this->assertEquals('feature-test', Domain::sanitizeBranchName('feature~test'));
|
||||
$this->assertEquals('feature-test', Domain::sanitizeBranchName('feature`test'));
|
||||
$this->assertEquals('feature-test', Domain::sanitizeBranchName('feature^test'));
|
||||
$this->assertEquals('feature-test', Domain::sanitizeBranchName('feature:test'));
|
||||
$this->assertEquals('feature-test', Domain::sanitizeBranchName('feature;test'));
|
||||
$this->assertEquals('feature-test', Domain::sanitizeBranchName('feature,test'));
|
||||
$this->assertEquals('feature-test', Domain::sanitizeBranchName('feature.test'));
|
||||
}
|
||||
|
||||
public function testSanitizeBranchNameWithSpaces(): void
|
||||
{
|
||||
$this->assertEquals('feature-test', Domain::sanitizeBranchName('feature test'));
|
||||
$this->assertEquals('my-branch-name', Domain::sanitizeBranchName('my branch name'));
|
||||
}
|
||||
|
||||
public function testSanitizeBranchNameTrimsHyphens(): void
|
||||
{
|
||||
// Leading and trailing invalid chars should be removed
|
||||
$this->assertEquals('feature', Domain::sanitizeBranchName('/feature'));
|
||||
$this->assertEquals('feature', Domain::sanitizeBranchName('feature/'));
|
||||
$this->assertEquals('feature', Domain::sanitizeBranchName('/feature/'));
|
||||
$this->assertEquals('feature', Domain::sanitizeBranchName('//feature//'));
|
||||
}
|
||||
|
||||
public function testSanitizeBranchNamePreservesValidChars(): void
|
||||
{
|
||||
// Valid branch names should remain unchanged
|
||||
$this->assertEquals('main', Domain::sanitizeBranchName('main'));
|
||||
$this->assertEquals('develop', Domain::sanitizeBranchName('develop'));
|
||||
$this->assertEquals('feature-123', Domain::sanitizeBranchName('feature-123'));
|
||||
$this->assertEquals('v1-2-3', Domain::sanitizeBranchName('v1-2-3'));
|
||||
$this->assertEquals('UPPERCASE', Domain::sanitizeBranchName('UPPERCASE'));
|
||||
$this->assertEquals('MixedCase123', Domain::sanitizeBranchName('MixedCase123'));
|
||||
}
|
||||
|
||||
public function testSanitizeBranchNameWithEmptyString(): void
|
||||
{
|
||||
$this->assertEquals('', Domain::sanitizeBranchName(''));
|
||||
}
|
||||
|
||||
public function testSanitizeBranchNameWithOnlyInvalidChars(): void
|
||||
{
|
||||
$this->assertEquals('', Domain::sanitizeBranchName('///'));
|
||||
$this->assertEquals('', Domain::sanitizeBranchName('___'));
|
||||
$this->assertEquals('', Domain::sanitizeBranchName('@#$'));
|
||||
}
|
||||
|
||||
/**
|
||||
* Test generating branch prefix for domain names
|
||||
*/
|
||||
public function testGenerateBranchPrefixShortBranch(): void
|
||||
{
|
||||
// Branch names <= 16 characters should not have hash suffix
|
||||
$prefix = Domain::generateBranchPrefix('main');
|
||||
$this->assertEquals('main', $prefix);
|
||||
|
||||
$prefix = Domain::generateBranchPrefix('feature-test');
|
||||
$this->assertEquals('feature-test', $prefix);
|
||||
|
||||
$prefix = Domain::generateBranchPrefix('exactly16chars12');
|
||||
$this->assertEquals('exactly16chars12', $prefix);
|
||||
}
|
||||
|
||||
public function testGenerateBranchPrefixLongBranch(): void
|
||||
{
|
||||
// Branch names > 16 characters should have hash suffix
|
||||
$prefix = Domain::generateBranchPrefix('this-is-a-very-long-branch-name');
|
||||
// First 16 chars: "this-is-a-very-l" + hash of remaining chars
|
||||
$this->assertStringStartsWith('this-is-a-very-l-', $prefix);
|
||||
$this->assertEquals(24, strlen($prefix)); // 16 + 1 (hyphen) + 7 (hash)
|
||||
}
|
||||
|
||||
public function testGenerateBranchPrefixWithInvalidChars(): void
|
||||
{
|
||||
// Branch with slash should be sanitized
|
||||
$prefix = Domain::generateBranchPrefix('feature/test');
|
||||
$this->assertEquals('feature-test', $prefix);
|
||||
|
||||
// Long branch with slash
|
||||
$prefix = Domain::generateBranchPrefix('feature/very/long/branch/name');
|
||||
$this->assertStringStartsWith('feature-very-lon-', $prefix);
|
||||
$this->assertEquals(24, strlen($prefix));
|
||||
}
|
||||
|
||||
public function testGenerateBranchPrefixConsistency(): void
|
||||
{
|
||||
// Same input should produce same output
|
||||
$prefix1 = Domain::generateBranchPrefix('feature/my-long-branch-name');
|
||||
$prefix2 = Domain::generateBranchPrefix('feature/my-long-branch-name');
|
||||
$this->assertEquals($prefix1, $prefix2);
|
||||
}
|
||||
|
||||
public function testGenerateBranchPrefixDifferentHashes(): void
|
||||
{
|
||||
// Different branch names with same first 16 chars should have different hashes
|
||||
$prefix1 = Domain::generateBranchPrefix('feature-branch-01234567890');
|
||||
$prefix2 = Domain::generateBranchPrefix('feature-branch-0abcdefghij');
|
||||
|
||||
// Both start with sanitized first 16 chars
|
||||
$this->assertStringStartsWith('feature-branch-0-', $prefix1);
|
||||
$this->assertStringStartsWith('feature-branch-0-', $prefix2);
|
||||
|
||||
// But have different hash suffixes
|
||||
$this->assertNotEquals($prefix1, $prefix2);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test generating full branch domain names
|
||||
*/
|
||||
public function testGenerateBranchDomain(): void
|
||||
{
|
||||
$domain = Domain::generateBranchDomain('main', 'site123', 'proj456', 'appwrite.network');
|
||||
$this->assertStringStartsWith('branch-main-', $domain);
|
||||
$this->assertStringEndsWith('.appwrite.network', $domain);
|
||||
}
|
||||
|
||||
public function testGenerateBranchDomainWithSlash(): void
|
||||
{
|
||||
$domain = Domain::generateBranchDomain('feature/test', 'site123', 'proj456', 'appwrite.network');
|
||||
// Should NOT contain slash
|
||||
$this->assertStringNotContainsString('/', $domain);
|
||||
$this->assertStringStartsWith('branch-feature-test-', $domain);
|
||||
$this->assertStringEndsWith('.appwrite.network', $domain);
|
||||
}
|
||||
|
||||
public function testGenerateBranchDomainConsistency(): void
|
||||
{
|
||||
// Same inputs should produce same domain
|
||||
$domain1 = Domain::generateBranchDomain('feature/test', 'site123', 'proj456', 'appwrite.network');
|
||||
$domain2 = Domain::generateBranchDomain('feature/test', 'site123', 'proj456', 'appwrite.network');
|
||||
$this->assertEquals($domain1, $domain2);
|
||||
}
|
||||
|
||||
public function testGenerateBranchDomainDifferentResources(): void
|
||||
{
|
||||
// Different resources should produce different domains
|
||||
$domain1 = Domain::generateBranchDomain('main', 'site123', 'proj456', 'appwrite.network');
|
||||
$domain2 = Domain::generateBranchDomain('main', 'site789', 'proj456', 'appwrite.network');
|
||||
$this->assertNotEquals($domain1, $domain2);
|
||||
}
|
||||
|
||||
public function testGenerateBranchDomainDifferentProjects(): void
|
||||
{
|
||||
// Different projects should produce different domains
|
||||
$domain1 = Domain::generateBranchDomain('main', 'site123', 'proj456', 'appwrite.network');
|
||||
$domain2 = Domain::generateBranchDomain('main', 'site123', 'proj789', 'appwrite.network');
|
||||
$this->assertNotEquals($domain1, $domain2);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test real-world branch name scenarios
|
||||
*/
|
||||
public function testRealWorldBranchNames(): void
|
||||
{
|
||||
// Common Git branch naming conventions
|
||||
$this->assertEquals('feature-SER-1234', Domain::sanitizeBranchName('feature/SER-1234'));
|
||||
$this->assertEquals('bugfix-fix-login', Domain::sanitizeBranchName('bugfix/fix-login'));
|
||||
$this->assertEquals('hotfix-v1-2-3', Domain::sanitizeBranchName('hotfix/v1.2.3'));
|
||||
$this->assertEquals('release-2024-01', Domain::sanitizeBranchName('release/2024.01'));
|
||||
$this->assertEquals('user-john-experiment', Domain::sanitizeBranchName('user/john/experiment'));
|
||||
$this->assertEquals('dependabot-npm-and-yarn-lodash-4-17-21', Domain::sanitizeBranchName('dependabot/npm_and_yarn/lodash-4.17.21'));
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user