feat: Implement white-label branding system with SASS compilation #207
Conversation
- DynamicAssetController with SASS compilation support - BrandingController for managing white-label configuration - WhiteLabelService with comprehensive branding logic - BrandingCacheService for caching rendered styles - CssValidationService for secure CSS customization - SassCompilationService for SCSS to CSS conversion - DomainValidationService for custom domain management - EmailTemplateService for branding email templates - DynamicBrandingMiddleware for injecting custom styles - Complete SCSS templates for branding - Database migration and factory for white-label configs
WalkthroughThis PR introduces a comprehensive white-label branding system enabling multi-tenant domain-based and organization-based asset serving with theme compilation, caching, email templating, and domain management. It includes controllers, models, services, middleware, configuration, and SCSS templates for enterprise branding customization. Changes
Sequence DiagramssequenceDiagram
autonumber
participant Client
participant Middleware as DynamicBrandingMiddleware
participant Config as WhiteLabelConfig
participant Container as App Container
participant View
Client->>Middleware: HTTP Request (domain: example.com)
Middleware->>Config: findByDomain('example.com')
alt Branding Found
Config-->>Middleware: WhiteLabelConfig
Middleware->>Container: Store branding & organization
Middleware->>View: Share branding data
Middleware-->>Client: Next middleware/controller
else No Branding
Middleware->>Container: Store default branding
Middleware->>View: Share default data
Middleware-->>Client: Next middleware/controller
end
sequenceDiagram
autonumber
participant Admin
participant BrandingController as BrandingController
participant WhiteLabelService
participant SassCompiler as SassCompilationService
participant CacheService as BrandingCacheService
participant Storage
Admin->>BrandingController: updateTheme(theme_config)
BrandingController->>WhiteLabelService: compileTheme(config)
WhiteLabelService->>SassCompiler: compile(config)
SassCompiler->>Storage: Load theme.scss
SassCompiler-->>WhiteLabelService: Compiled CSS
WhiteLabelService->>WhiteLabelService: Append custom CSS
WhiteLabelService->>CacheService: cacheCompiledTheme(css)
CacheService-->>WhiteLabelService: Cached
WhiteLabelService-->>BrandingController: Success
BrandingController-->>Admin: Redirect with success
sequenceDiagram
autonumber
participant Client
participant EnterpriseController as DynamicAssetController<br/>(Enterprise)
participant Cache as BrandingCacheService
participant Compiler as SassCompilationService
participant Redis
participant LaravelCache
Client->>EnterpriseController: GET /styles/:organization
EnterpriseController->>EnterpriseController: Build cache key & ETag
alt If-None-Match ETag matches
EnterpriseController-->>Client: 304 Not Modified
else Cache Hit (Redis/Laravel)
EnterpriseController->>Cache: getCachedTheme(orgId)
Cache->>Redis: Check cached CSS
Redis-->>Cache: CSS found
Cache-->>EnterpriseController: CSS
EnterpriseController-->>Client: 200 + CSS + ETag
else Cache Miss
EnterpriseController->>Compiler: compile(config)
Compiler-->>EnterpriseController: Compiled CSS
EnterpriseController->>Cache: cacheCompiledTheme(css)
Cache->>Redis: Store with TTL
Cache->>LaravelCache: Fallback store
Cache-->>EnterpriseController: Cached
EnterpriseController-->>Client: 200 + CSS + ETag
end
sequenceDiagram
autonumber
participant Admin
participant BrandingController
participant WhiteLabelService
participant DomainValidator as DomainValidationService
participant DnsLookup as External DNS
participant SslCheck as External SSL
Admin->>BrandingController: addDomain('example.com')
BrandingController->>DomainValidator: performComprehensiveValidation()
DomainValidator->>DnsLookup: validateDns(domain)
DnsLookup-->>DomainValidator: A/AAAA/CNAME records
DomainValidator->>SslCheck: validateSsl(domain)
SslCheck-->>DomainValidator: Certificate details & validity
DomainValidator->>DomainValidator: isDomainAvailable(domain)
DomainValidator->>DomainValidator: generateVerificationToken()
DomainValidator-->>BrandingController: Validation result + token
alt Valid
BrandingController->>WhiteLabelService: setCustomDomain(domain)
WhiteLabelService-->>BrandingController: Success
BrandingController-->>Admin: Domain added + verification instructions
else Invalid
BrandingController-->>Admin: Error + validation details
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Possibly related issues
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| $wildcardDomain = '*.'.$parentDomain; | ||
| $ip = gethostbyname('test-'.uniqid().'.'.$parentDomain); | ||
|
|
||
| if ($ip !== 'test-'.uniqid().'.'.$parentDomain) { |
There was a problem hiding this comment.
Bug: Wildcard DNS check uses different uniqid values
The wildcard DNS check generates a unique ID for the hostname but then compares the resolved IP against a different unique ID. Line 135 calls uniqid() to create the test hostname, but line 137 calls uniqid() again for the comparison, generating a different value. This means the condition will always be false since gethostbyname returns the hostname unchanged when DNS resolution fails, and the two uniqid() values will never match.
| return response($css, 200) | ||
| ->header('Content-Type', 'text/css; charset=UTF-8') | ||
| ->header('ETag', $etag) | ||
| ->header('X-Cache-Hit', Cache::has($cacheKey) ? 'true' : 'false'); |
There was a problem hiding this comment.
Bug: Cache hit check occurs after cache retrieval
The X-Cache-Hit header checks Cache::has($cacheKey) after Cache::remember() has already been called. Since Cache::remember() stores the value in cache when it's not present, Cache::has() will always return true at line 85, making the header always report 'true' even when the cache was initially empty and the CSS was just generated.
| } | ||
|
|
||
| try { | ||
| return $this->compiler->compileString(File::get($darkModePath))->getCss(); |
There was a problem hiding this comment.
Bug: Dark mode compilation ignores theme configuration
The compileDarkMode method compiles the dark mode SCSS template without passing any theme variables from the configuration. Unlike the compile method which calls generateSassVariables to inject theme colors, compileDarkMode just compiles the raw template. This means dark mode won't use the organization's custom theme colors, defeating the purpose of white-label branding for dark mode.
| $validForDomain = true; | ||
| } elseif (in_array('*.'.substr($domain, strpos($domain, '.') + 1), $certInfo['san'])) { | ||
| $validForDomain = true; | ||
| } |
There was a problem hiding this comment.
Bug: Wildcard certificate validation fails for top-level domains
The wildcard certificate validation uses strpos($domain, '.') which returns false for top-level domains without dots. When passed to substr with false + 1, this creates an invalid substring operation. While top-level domains are unlikely in practice, the code doesn't handle this edge case and could produce unexpected results or errors.
| // Check if domain is already configured for another organization | ||
| $existing = \App\Models\WhiteLabelConfig::whereJsonContains('custom_domains', $domain)->first(); | ||
|
|
||
| return $existing === null; |
There was a problem hiding this comment.
Bug: Domain availability check doesn't exclude current organization
The isDomainAvailable method checks if a domain is already in use but doesn't accept an organization ID parameter to exclude the current organization. When performComprehensiveValidation is called with an organization ID, it passes this to isDomainAvailable, but the method ignores it. This prevents organizations from re-validating or updating their own existing domains, as the check will always find the domain already in use by themselves.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (5)
app/Http/Controllers/Enterprise/DynamicAssetController.php-82-85 (1)
82-85: X-Cache-Hit header logic is incorrect.After
Cache::remember()executes, the value is always in the cache, soCache::has($cacheKey)will always returntrue. This header becomes meaningless for indicating actual cache hits.+ $cacheHit = Cache::has($cacheKey); $css = Cache::remember($cacheKey, $ttl, fn () => $this->buildCssResponse($config)); return response($css, 200) ->header('Content-Type', 'text/css; charset=UTF-8') ->header('ETag', $etag) - ->header('X-Cache-Hit', Cache::has($cacheKey) ? 'true' : 'false'); + ->header('X-Cache-Hit', $cacheHit ? 'true' : 'false');Committable suggestion skipped: line range outside the PR's diff.
app/Http/Controllers/Enterprise/BrandingController.php-469-480 (1)
469-480: Fallback organization resolution could lead to authorization confusion.Using session or
user()->organizations()->first()as fallbacks when route parameter is missing is risky. If authorization checks are performed against a different organization than intended, this could lead to unintended access.Consider failing explicitly rather than falling back:
protected function getCurrentOrganization(Request $request): Organization { - // This would typically come from session or auth context - $organizationId = $request->route('organization') ?? - $request->session()->get('current_organization_id') ?? - $request->user()->organizations()->first()?->id; + $organizationId = $request->route('organization'); + + if (! $organizationId) { + abort(400, 'Organization ID is required'); + } return Organization::findOrFail($organizationId); }app/Http/Controllers/Enterprise/BrandingController.php-485-507 (1)
485-507: Hardcoded 'example.com' generates unusable verification token.The token is generated for
'example.com'instead of the actual domain, making the displayed TXT record value incorrect for the user's real domain.Either use a placeholder or document that this is a template:
protected function getVerificationInstructions(Organization $organization): array { - $token = $this->domainService->generateVerificationToken('example.com', $organization->id); + // Token shown is a template - actual token is generated per-domain during validation + $exampleToken = $this->domainService->generateVerificationToken('yourdomain.com', $organization->id); return [ 'dns_txt' => [ 'type' => 'TXT', 'name' => '@', - 'value' => "coolify-verify={$token}", + 'value' => "coolify-verify=<your-verification-token>", + 'note' => 'The actual token will be provided when you add a domain', 'ttl' => 3600, ],Committable suggestion skipped: line range outside the PR's diff.
app/Services/Enterprise/BrandingCacheService.php-287-305 (1)
287-305: Same Redis KEYS concern and unused cache tags.This method has the same
Redis::keys()performance issue. Additionally,Cache::tags(['branding'])is used for flushing, but nothing in this service stores cache entries with tags, so this flush will have no effect.Either remove the unused tags flush or add tags when caching:
- // Also clear from Laravel cache - // Note: This requires cache tags support - if (method_exists(Cache::getStore(), 'tags')) { - Cache::tags(['branding'])->flush(); - } + // Note: Cache tags flush removed as entries aren't stored with tagsOr if you want tags support, add them when storing:
Cache::tags(['branding'])->put($key, $css, self::CACHE_TTL);app/Services/Enterprise/BrandingCacheService.php-174-182 (1)
174-182: Hardcoded config keys will miss dynamically added keys.
cacheBrandingConfig()stores all keys from the config array dynamically, butclearOrganizationCache()only clears a hardcoded subset (platform_name,primary_color,secondary_color,accent_color). Any other config keys stored will remain in cache after clearing.Consider pattern-based clearing for config keys or track stored keys:
// Clear individual config element caches - $configKeys = [ - self::CACHE_PREFIX."config:{$organizationId}:platform_name", - self::CACHE_PREFIX."config:{$organizationId}:primary_color", - self::CACHE_PREFIX."config:{$organizationId}:secondary_color", - self::CACHE_PREFIX."config:{$organizationId}:accent_color", - ]; - foreach ($configKeys as $key) { - Cache::forget($key); + // Clear config keys using pattern if Redis available, otherwise clear known keys + if ($this->isRedisAvailable()) { + $configPattern = self::CACHE_PREFIX."config:{$organizationId}:*"; + $configKeys = Redis::keys($configPattern); + if (!empty($configKeys)) { + Redis::del($configKeys); + } }Committable suggestion skipped: line range outside the PR's diff.
🧹 Nitpick comments (21)
config/enterprise.php (1)
7-20: Consider centralizing default theme values to avoid duplication.The default theme values here duplicate those in
WhiteLabelConfig::getDefaultThemeVariables(). Additionally,font_familyis defined here but missing from the model's defaults. Consider having the model read from this config to maintain a single source of truth:public function getDefaultThemeVariables(): array { return config('enterprise.white_label.default_theme', [ // fallback defaults... ]); }app/Services/Enterprise/DomainValidationService.php (2)
211-222: Consider using$errnoand$errstrfor better error diagnostics.These variables capture error details from
stream_socket_clientbut are currently unused. Including them in error handling would improve debugging.$stream = @stream_socket_client( "ssl://{$domain}:".self::SSL_PORT, $errno, $errstr, self::SSL_TIMEOUT, STREAM_CLIENT_CONNECT, $context ); if (! $stream) { + Log::debug('SSL connection failed', [ + 'domain' => $domain, + 'errno' => $errno, + 'errstr' => $errstr, + ]); return null; }
367-383: Unused loop variable$value.The
$valuevariable is declared but never used. Usearray_keys()or the underscore convention.- foreach ($headers as $key => $value) { + foreach (array_keys($headers) as $key) { if (strtolower($key) === $headerLower) { $results['security_headers'][$name] = true; $found = true; break; } }resources/sass/branding/dark.scss (1)
1-4: Hardcoded color value breaks the variable substitution pattern.This file hardcodes
#0056b3instead of using a SCSS variable liketheme.scssdoes. For consistent theming throughSassCompilationService, consider using a dark mode variable:// Dark mode specific styles +$primary-color-dark: #0056b3 !default; + body.dark { - --primary-color: #0056b3; + --primary-color: #{$primary-color-dark}; }This allows organizations to customize dark mode colors through
WhiteLabelConfig.theme_config.resources/sass/branding/variables.md (1)
5-11: Documentation is incomplete - missing most theme variables.The
config/enterprise.phpandWhiteLabelConfigmodel define additional variables (accent_color,background_color,text_color,sidebar_color,border_color,success_color,warning_color,error_color,info_color) that are not documented here. Consider documenting the full set for completeness.resources/sass/enterprise/dark-mode-template.scss (1)
14-118: Consider extracting dark mode variables into a reusable mixin to reduce duplication.The
@media (prefers-color-scheme: dark)block (lines 14-66) and the.darkclass (lines 68-118) contain nearly identical variable declarations. This duplication increases maintenance burden when colors or tokens need updating.+// Mixin for dark mode variables +@mixin dark-mode-variables { + --color-background: #1a1a1a; + --color-background-rgb: 26, 26, 26; + --color-sidebar: #2a2a2a; + // ... remaining variables + --color-primary: #{adjust-color($primary_color, $lightness: 10%)}; + // ... rest of accent and status colors + --shadow-sm: 0 1px 2px 0 rgba(0, 0, 0, 0.3); + // ... remaining shadows +} + @media (prefers-color-scheme: dark) { :root { - // All current declarations... + @include dark-mode-variables; } } .dark { - // All current declarations... + @include dark-mode-variables; }resources/sass/enterprise/white-label-template.scss (2)
112-131: Hardcoded white text may cause accessibility issues with light-colored themes.Button text is hardcoded to
whitewhich could fail WCAG contrast requirements if an organization configures a light primary color (e.g., yellow or light blue).Consider using a contrast-aware approach or exposing a button text color variable:
$primary_color: #3b82f6 !default; +$button_text_color: #ffffff !default; .btn-primary { background-color: var(--color-primary); border-color: var(--color-primary); - color: white; + color: var(--color-button-text, #{$button_text_color});
170-180: Input fields missing background-color declaration.Without an explicit background-color, inputs will inherit or default to browser styles, potentially causing visual inconsistencies across themes.
.input, .form-input { + background-color: var(--color-background); border-color: var(--color-border); color: var(--color-text);app/Http/Middleware/DynamicBrandingMiddleware.php (1)
27-28: Potential N+1 query when accessing the organization relationship.
$branding->organizationon line 28 triggers a lazy load sincefindByDomain()doesn't eager load the relationship.Update
findByDomain()inWhiteLabelConfigmodel to eager load:public static function findByDomain(string $domain): ?self { return self::with('organization') ->whereJsonContains('custom_domains', $domain) ->first(); }app/Http/Controllers/Enterprise/DynamicAssetController.php (1)
235-246: Consider using a dedicated CSS minifier library.The manual regex-based minification may produce incorrect results in edge cases (e.g., content in strings, data URIs). Since SASS is already compiled with
OutputStyle::COMPRESSED, this step may be redundant.Verify if this additional minification is necessary given that
SassCompilationServicealready uses compressed output.app/Services/Enterprise/SassCompilationService.php (1)
114-118: Named colors list is incomplete.CSS defines 140+ named colors, but only 6 are listed here. Values like
gray,green,orange, etc., will be rejected or improperly quoted.Consider using a more comprehensive approach:
- $namedColors = ['transparent', 'currentColor', 'white', 'black', 'red', 'blue']; // Add more if needed - if (in_array(strtolower($value), $namedColors)) { + // Check if it matches a simple word that could be a CSS color keyword + // This allows common named colors without maintaining an exhaustive list + if (preg_match('/^[a-z]+$/i', $value) && strlen($value) <= 20) { return $value; }Alternatively, maintain a comprehensive list of CSS named colors or validate against the SCSS compiler's color functions.
app/Services/Enterprise/CssValidationService.php (1)
9-17: Consider expanding the blocklist for more comprehensive CSS security.The current patterns cover major XSS vectors but miss some edge cases.
Consider adding these patterns:
private const DANGEROUS_PATTERNS = [ '@import', 'expression(', 'javascript:', 'vbscript:', 'behavior:', 'data:text/html', '-moz-binding', + 'url(', // Validate URLs separately to prevent data: and javascript: in url() + '@charset', // Can cause encoding attacks + '@font-face', // Can load external resources + 'src:', // External resource loading ];Note: Adding
url(requires additional logic to allow safeurl()values while blocking dangerous schemes.app/Models/WhiteLabelConfig.php (2)
224-228: Consider caching domain lookups for performance.
whereJsonContainson JSON arrays can be slow at scale. TheDynamicAssetControlleralready caches CSS, but the domain lookup itself happens before caching kicks in.Consider caching the domain-to-config mapping:
public static function findByDomain(string $domain): ?self { return Cache::remember("whitelabel:domain:{$domain}", 3600, function () use ($domain) { return self::whereJsonContains('custom_domains', $domain)->first(); }); }Ensure cache invalidation when domains are modified.
43-48: Document that setter methods require explicitsave()call.Methods like
setThemeVariable,addCustomDomain,removeCustomDomain, andsetEmailTemplatemodify attributes but don't persist. This is fine but should be documented for API clarity.Add a brief docblock noting the need to call
save():/** * Set a theme variable. Call save() to persist. */ public function setThemeVariable(string $variable, $value): voidAlso applies to: 94-101, 103-107, 125-130
app/Services/Enterprise/WhiteLabelService.php (3)
163-172: Placeholder method has unused parameters and dead code.As flagged by static analysis,
$imageand$pathare unused. Either implement or remove to avoid confusion.Consider removing until implemented, or add suppression with TODO:
/** * Generate SVG version of logo for theming + * + * @todo Implement with image tracing library */ + /** @phpstan-ignore-next-line */ protected function generateSvgVersion($image, Organization $organization): void { - // This would require image tracing library - // Placeholder for SVG generation logic - $path = "branding/logos/{$organization->id}/logo.svg"; - // Storage::disk('public')->put($path, $svgContent); + // Intentionally empty - requires image tracing library }
236-262: Unused$variablesparameter can be removed.The method uses CSS
var()references rather than the passed variables array. Remove the parameter for cleaner API./** * Generate component-specific styles */ - protected function generateComponentStyles(array $variables): string + protected function generateComponentStyles(): string { // ... method body unchanged }Update the caller in
compileTheme():- $css .= $this->generateComponentStyles($variables); + $css .= $this->generateComponentStyles();
500-506: Float values may cause issues withdechex()in PHP 8+.The arithmetic produces floats, but
dechex()expects integers. PHP 8+ may emit deprecation warnings.- $r = max(0, min(255, $r + ($r * $percent / 100))); - $g = max(0, min(255, $g + ($g * $percent / 100))); - $b = max(0, min(255, $b + ($b * $percent / 100))); + $r = (int) max(0, min(255, $r + ($r * $percent / 100))); + $g = (int) max(0, min(255, $g + ($g * $percent / 100))); + $b = (int) max(0, min(255, $b + ($b * $percent / 100))); - return '#'.str_pad(dechex($r), 2, '0', STR_PAD_LEFT) - .str_pad(dechex($g), 2, '0', STR_PAD_LEFT) - .str_pad(dechex($b), 2, '0', STR_PAD_LEFT); + return sprintf('#%02x%02x%02x', $r, $g, $b);app/Services/Enterprise/EmailTemplateService.php (2)
136-169: Conditional processing is simplistic but may have edge cases.The
@ifprocessing only checks if a variable exists and is truthy - it doesn't evaluate expressions. This works for simple cases like@if(hide_branding)but could be confusing if someone expects expression support.Additionally, nested conditionals aren't supported due to the greedy regex.
Consider adding a docblock clarifying the limitations:
/** * Process conditional statements in template * * Note: Only supports simple variable checks (e.g., @if(variable_name)). * Does not support expressions, comparisons, or nested conditionals. */
868-878: Text version generation is basic - consider preserving structure.
strip_tags()removes all HTML without preserving link URLs or structure. Email clients that render plain text may lose important information like action URLs.Consider extracting href values before stripping tags:
protected function generateTextVersion(string $html): string { // Convert links to text format $text = preg_replace('/<a[^>]+href=["\']([^"\']+)["\'][^>]*>([^<]+)<\/a>/i', '$2 ($1)', $html); // Remove HTML tags $text = strip_tags($text); // Clean up whitespace $text = preg_replace('/\s+/', ' ', $text); $text = preg_replace('/\s*\n\s*/', "\n", $text); return trim($text); }app/Services/Enterprise/BrandingCacheService.php (2)
327-339: Method name is misleading - returns CSS content, not version.
getCurrentCssVersion()suggests it returns a version identifier, but it actually returns the CSS content string for the current version./** - * Get current CSS version + * Get CSS content for the current version */ - public function getCurrentCssVersion(string $organizationId): ?string + public function getCurrentVersionedCss(string $organizationId): ?string { $version = Cache::get(self::THEME_CACHE_PREFIX."{$organizationId}:current"); if ($version) { return Cache::get(self::THEME_CACHE_PREFIX."{$organizationId}:v{$version}"); } return null; }
202-206: Duplicate asset clearing logic.Lines 202-206 duplicate the logic in
clearAssetCache()(lines 232-239). This creates maintenance burden and inconsistency risk.Remove the duplication and reuse the protected method:
- // Clear asset keys - $assetTypes = ['logo', 'favicon', 'favicon-16', 'favicon-32', 'favicon-64', 'favicon-128', 'favicon-192']; - foreach ($assetTypes as $type) { - $assetKey = $this->getAssetCacheKey($organizationId, $type); - Redis::del($assetKey); - } + // Clear asset keys from Redis + foreach (['logo', 'favicon', 'favicon-16', 'favicon-32', 'favicon-64', 'favicon-128', 'favicon-192'] as $type) { + Redis::del($this->getAssetCacheKey($organizationId, $type)); + }Or better, extract the asset types to a class constant to ensure consistency.
Also applies to: 232-239
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
app/Http/Controllers/DynamicAssetController.php(1 hunks)app/Http/Controllers/Enterprise/BrandingController.php(1 hunks)app/Http/Controllers/Enterprise/DynamicAssetController.php(1 hunks)app/Http/Middleware/DynamicBrandingMiddleware.php(1 hunks)app/Models/WhiteLabelConfig.php(1 hunks)app/Services/Enterprise/BrandingCacheService.php(1 hunks)app/Services/Enterprise/CssValidationService.php(1 hunks)app/Services/Enterprise/DomainValidationService.php(1 hunks)app/Services/Enterprise/EmailTemplateService.php(1 hunks)app/Services/Enterprise/SassCompilationService.php(1 hunks)app/Services/Enterprise/WhiteLabelService.php(1 hunks)config/enterprise.php(1 hunks)database/factories/WhiteLabelConfigFactory.php(1 hunks)database/migrations/2025_08_26_225748_create_white_label_configs_table.php(1 hunks)resources/sass/branding/dark.scss(1 hunks)resources/sass/branding/theme.scss(1 hunks)resources/sass/branding/variables.md(1 hunks)resources/sass/enterprise/dark-mode-template.scss(1 hunks)resources/sass/enterprise/white-label-template.scss(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
app/Http/Controllers/Enterprise/DynamicAssetController.php (3)
app/Models/WhiteLabelConfig.php (2)
WhiteLabelConfig(9-234)organization(32-35)app/Services/Enterprise/CssValidationService.php (2)
CssValidationService(7-113)sanitize(19-42)app/Services/Enterprise/SassCompilationService.php (4)
SassCompilationService(15-132)__construct(19-25)compile(35-51)compileDarkMode(60-73)
app/Services/Enterprise/DomainValidationService.php (1)
app/Models/WhiteLabelConfig.php (1)
WhiteLabelConfig(9-234)
app/Http/Controllers/DynamicAssetController.php (1)
app/Models/WhiteLabelConfig.php (10)
WhiteLabelConfig(9-234)findByDomain(225-228)generateCssVariables(74-91)getLogoUrl(158-161)getPlatformName(153-156)getThemeVariables(50-55)createDefault(199-209)getDefaultThemeVariables(57-72)getCustomDomains(114-117)shouldHideCoolifyBranding(168-171)
app/Http/Middleware/DynamicBrandingMiddleware.php (1)
app/Models/WhiteLabelConfig.php (9)
WhiteLabelConfig(9-234)findByDomain(225-228)organization(32-35)getPlatformName(153-156)getLogoUrl(158-161)shouldHideCoolifyBranding(168-171)getThemeVariables(50-55)createDefault(199-209)getDefaultThemeVariables(57-72)
app/Services/Enterprise/WhiteLabelService.php (3)
app/Models/WhiteLabelConfig.php (7)
WhiteLabelConfig(9-234)organization(32-35)getThemeVariables(50-55)generateCssVariables(74-91)getThemeVariable(38-41)isValidDomain(180-183)addCustomDomain(94-101)app/Services/Enterprise/BrandingCacheService.php (4)
BrandingCacheService(8-386)clearOrganizationCache(162-213)cacheCompiledTheme(23-36)clearDomainCache(218-227)app/Services/Enterprise/DomainValidationService.php (3)
DomainValidationService(8-495)validateDns(21-68)validateSsl(160-195)
app/Services/Enterprise/BrandingCacheService.php (2)
app/Models/WhiteLabelConfig.php (1)
WhiteLabelConfig(9-234)app/Services/Enterprise/WhiteLabelService.php (2)
WhiteLabelService(12-518)compileTheme(177-207)
app/Models/WhiteLabelConfig.php (2)
app/Services/Enterprise/WhiteLabelService.php (1)
generateCssVariables(212-233)app/Http/Controllers/Enterprise/BrandingController.php (2)
domains(189-202)update(71-90)
app/Services/Enterprise/EmailTemplateService.php (1)
app/Models/WhiteLabelConfig.php (7)
WhiteLabelConfig(9-234)getPlatformName(153-156)getLogoUrl(158-161)getThemeVariable(38-41)shouldHideCoolifyBranding(168-171)hasCustomEmailTemplate(132-135)getEmailTemplate(120-123)
app/Http/Controllers/Enterprise/BrandingController.php (4)
app/Models/WhiteLabelConfig.php (7)
WhiteLabelConfig(9-234)organization(32-35)getThemeVariables(50-55)getAvailableEmailTemplates(137-150)removeCustomDomain(103-107)setEmailTemplate(125-130)resetToDefaults(211-222)app/Services/Enterprise/BrandingCacheService.php (4)
BrandingCacheService(8-386)getCacheStats(260-282)clearOrganizationCache(162-213)clearDomainCache(218-227)app/Services/Enterprise/EmailTemplateService.php (3)
EmailTemplateService(8-972)__construct(14-18)previewTemplate(845-863)app/Services/Enterprise/WhiteLabelService.php (8)
WhiteLabelService(12-518)__construct(20-28)getOrCreateConfig(33-45)processLogo(72-102)compileTheme(177-207)setCustomDomain(358-398)exportConfiguration(411-422)importConfiguration(427-443)
🪛 PHPMD (2.15.0)
app/Services/Enterprise/DomainValidationService.php
134-134: Avoid unused local variables such as '$wildcardDomain'. (undefined)
(UnusedLocalVariable)
213-213: Avoid unused local variables such as '$errno'. (undefined)
(UnusedLocalVariable)
214-214: Avoid unused local variables such as '$errstr'. (undefined)
(UnusedLocalVariable)
371-371: Avoid unused local variables such as '$value'. (undefined)
(UnusedLocalVariable)
database/factories/WhiteLabelConfigFactory.php
58-58: Avoid unused parameters such as '$attributes'. (undefined)
(UnusedFormalParameter)
68-68: Avoid unused parameters such as '$attributes'. (undefined)
(UnusedFormalParameter)
app/Services/Enterprise/WhiteLabelService.php
166-166: Avoid unused parameters such as '$image'. (undefined)
(UnusedFormalParameter)
170-170: Avoid unused local variables such as '$path'. (undefined)
(UnusedLocalVariable)
238-238: Avoid unused parameters such as '$variables'. (undefined)
(UnusedFormalParameter)
app/Services/Enterprise/EmailTemplateService.php
345-345: Avoid unused parameters such as '$config'. (undefined)
(UnusedFormalParameter)
app/Http/Controllers/Enterprise/BrandingController.php
189-189: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
262-262: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
284-284: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
346-346: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
369-369: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
420-420: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
441-441: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
455-455: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (19)
app/Services/Enterprise/DomainValidationService.php (2)
405-408: Verification tokens will invalidate ifAPP_KEYrotates.The token depends on
config('app.key'). If the application key is rotated, all pending domain verifications will fail. Consider storing the token in the database or using a dedicated secret for domain verification.
21-68: DNS validation implementation looks solid.The method properly handles various DNS record types, checks IP resolution, and includes appropriate error handling with logging. The structure with separate helper methods for different checks promotes maintainability.
resources/sass/branding/theme.scss (1)
1-6: I'll verify the original review comment by investigating whetherSassCompilationServiceauto-generates theme variables and understanding the full scope of theme configuration. Let me search the codebase for relevant information.
<function_calls>
#!/bin/bashSearch for SassCompilationService class
fd -type f -name ".php" -o -name ".ts" -o -name "*.js" | xargs rg -l "SassCompilationService" 2>/dev/null | head -20
</function_calls><function_calls>
#!/bin/bashSearch for WhiteLabelConfig to understand theme variables scope
rg -l "WhiteLabelConfig" --type php --type ts --type js 2>/dev/null | head -20
</function_calls><function_calls>
#!/bin/bashLet's search more broadly for any SCSS service or compilation-related files
fd -type f ( -name "Sass" -o -name "sass" -o -name "theme" -o -name "Theme" ) | grep -v node_modules | head -30
</function_calls>database/migrations/2025_08_26_225748_create_white_label_configs_table.php (1)
14-28: LGTM! Migration structure is well-designed.The schema correctly uses UUIDs, enforces referential integrity with cascade delete, and maintains a unique constraint on
organization_id. The JSON column defaults are properly specified.Note: The
findByDomain()method useswhereJsonContains('custom_domains', $domain)which may have performance implications at scale. If domain lookups become a bottleneck, consider a separatewhite_label_domainstable with proper indexing.database/factories/WhiteLabelConfigFactory.php (1)
12-72: LGTM! Well-structured factory with useful state modifiers.The factory follows Laravel conventions correctly. The static analysis warnings about unused
$attributesparameters inwithDomains()andwithCustomCss()are false positives—Laravel'sstate()callback requires this signature even when the parameter isn't used.app/Services/Enterprise/SassCompilationService.php (1)
60-73: Dark mode compilation doesn't receive theme variables.
compileDarkMode()compilesdark.scsswithout the organization's theme variables. Ifdark.scssreferences variables like$primary-color, they'll use the hardcoded defaults rather than the organization's custom colors.Verify if this is intentional. If dark mode should respect organization theme colors:
- public function compileDarkMode(): string + public function compileDarkMode(?WhiteLabelConfig $config = null): string { $darkModePath = resource_path('sass/branding/dark.scss'); if (! File::exists($darkModePath)) { throw new \Exception("Dark mode SASS file not found at {$darkModePath}"); } + $sassVariables = $config ? $this->generateSassVariables($config->getThemeVariables()) : ''; + $sassInput = $sassVariables . "\n" . File::get($darkModePath); + try { - return $this->compiler->compileString(File::get($darkModePath))->getCss(); + return $this->compiler->compileString($sassInput)->getCss();app/Http/Controllers/DynamicAssetController.php (1)
18-33: LGTM for caching and response structure.The caching strategy with domain-specific keys is appropriate. The debug header
X-Generated-For-Domainis useful for troubleshooting but consider making it conditional on environment.Consider restricting the debug header to non-production environments:
return response($css, 200, [ 'Content-Type' => 'text/css', 'Cache-Control' => 'public, max-age=3600', - 'X-Generated-For-Domain' => $domain, // Debug header - ]); + ] + (app()->environment('local', 'staging') ? ['X-Generated-For-Domain' => $domain] : []));app/Services/Enterprise/CssValidationService.php (2)
19-42: Good defensive error handling with graceful degradation.The service appropriately falls back when the parser is unavailable and returns safe placeholder CSS on errors.
76-85: Let me search more carefully for test files and investigate the Sabberworm library structure:
<function_calls>
#!/bin/bashSearch for test files related to CssValidationService
find . -name "CssValidation" -type f
Check if there's a composer.lock file to see Sabberworm version
if [ -f "composer.lock" ]; then
grep -A 10 '"name": "sabberworm/css-parser"' composer.lock | head -20
fiSearch for any test methods that test removeImports
rg -n "removeImports|testRemoveImports" --type php
</function_calls>#!/bin/bash # Let's check the vendor directory structure for Sabberworm ls -la vendor/sabberworm/ 2>/dev/null || echo "Vendor directory not available in sandbox" # Search for any actual @import CSS in test files or examples rg -n "@import" --type php -C 2 | head -40 </parameter> </invoke> </function_calls> </blockquote></details> <details> <summary>app/Models/WhiteLabelConfig.php (2)</summary><blockquote> `13-29`: **LGTM for fillable attributes and casts.** The fillable attributes are appropriately scoped and the array casts for JSON columns are correct. --- `180-183`: **LGTM for domain validation.** Using `FILTER_VALIDATE_DOMAIN` with `FILTER_FLAG_HOSTNAME` is the correct approach for validating domain names. </blockquote></details> <details> <summary>app/Services/Enterprise/WhiteLabelService.php (1)</summary><blockquote> `136-150`: **LGTM for favicon generation approach.** Cloning the image for each size and using `cover()` for aspect-ratio-aware cropping is appropriate. </blockquote></details> <details> <summary>app/Http/Controllers/Enterprise/BrandingController.php (2)</summary><blockquote> `189-202`: **Static analysis: Unused `$request` parameters are conventional for Laravel controller signatures.** These methods follow Laravel's controller method signature conventions where `$request` is commonly included even if unused, for consistency and future extensibility. This is acceptable. Also applies to: 262-279, 284-297, 346-364, 369-380, 420-436, 441-450, 455-467 --- `385-415`: **Import handling is reasonable but consider additional validation.** The JSON parsing with error checking is correct. The `mimes:json` rule is extension-based but the subsequent `json_decode` validation catches invalid content. Consider logging import attempts for audit purposes: ```php Log::info('Branding config import', [ 'organization_id' => $organization->id, 'user_id' => $request->user()->id, ]);app/Services/Enterprise/EmailTemplateService.php (2)
1-55: LGTM - Well-structured template generation pipeline.The constructor initialization and main
generateTemplate()method implement a clear pipeline: prepare variables → get template → process template → apply styles → inline CSS. The flow is logical and easy to follow.
362-840: Default email templates are comprehensive and well-structured.The templates follow email best practices with proper DOCTYPE, meta tags, and semantic structure. The branding placeholders (
{{platform_name}},{{logo_url}}) and conditional branding footer (@unless(hide_branding)) provide good customization support.app/Services/Enterprise/BrandingCacheService.php (3)
23-36: Good dual-storage pattern for theme caching.Storing in both Laravel Cache and Redis provides redundancy, and the version hash tracking enables efficient cache validation.
360-369: Good Redis availability check with exception handling.The try-catch around
Redis::ping()properly handles connection failures without crashing the application.
241-255: Based on my comprehensive verification, I can now provide the rewritten review comment:
Cache warming on every clear is intentional—verify if the design trade-off meets your needs.
warmCache()is explicitly called at the end ofclearOrganizationCache()(line 211–213), with a comment confirming this is intentional: "Trigger cache warming in background (skip in testing to avoid interference)". The design accepts two trade-offs:
- Source data corruption risk: If
WhiteLabelConfigcorruption triggers a cache clear, the same data will be used to regenerate. This assumes the source config is reliable; the cache is treated as disposable.- High load job accumulation: Rapid successive clears will queue multiple background warming jobs (no deduplication mechanism visible). These run async via
afterResponse(), so queue growth is the primary risk, not system impact.The testing environment is explicitly excluded to prevent interference. If these trade-offs don't align with your system's reliability model (e.g., corruption is possible in source data, or queue congestion is a concern), consider adding deduplication logic or making warming conditional.
| // Add custom CSS variables | ||
| $css .= "\n\n/* Custom theme for {$domain} */\n"; | ||
| $css .= $branding->generateCssVariables(); | ||
|
|
||
| // Add any custom CSS | ||
| if ($branding->custom_css) { | ||
| $css .= "\n\n/* Custom CSS for {$domain} */\n"; | ||
| $css .= $branding->custom_css; | ||
| } |
There was a problem hiding this comment.
Custom CSS is appended twice causing duplication.
The generateCssVariables() method in WhiteLabelConfig (lines 73-90 of the model) already appends custom_css if present. Lines 55-58 here add it again, resulting in duplicate custom CSS in the output.
Remove the duplicate custom CSS block:
// Add custom CSS variables
$css .= "\n\n/* Custom theme for {$domain} */\n";
$css .= $branding->generateCssVariables();
- // Add any custom CSS
- if ($branding->custom_css) {
- $css .= "\n\n/* Custom CSS for {$domain} */\n";
- $css .= $branding->custom_css;
- }
-
return $css;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Add custom CSS variables | |
| $css .= "\n\n/* Custom theme for {$domain} */\n"; | |
| $css .= $branding->generateCssVariables(); | |
| // Add any custom CSS | |
| if ($branding->custom_css) { | |
| $css .= "\n\n/* Custom CSS for {$domain} */\n"; | |
| $css .= $branding->custom_css; | |
| } | |
| // Add custom CSS variables | |
| $css .= "\n\n/* Custom theme for {$domain} */\n"; | |
| $css .= $branding->generateCssVariables(); | |
| return $css; |
🤖 Prompt for AI Agents
In app/Http/Controllers/DynamicAssetController.php around lines 50 to 58, the
controller appends branding->generateCssVariables() which already includes the
custom_css, and then conditionally appends branding->custom_css again (lines
~55-58), causing duplicated CSS; remove the conditional block that appends the
custom_css (the second addition) so only generateCssVariables() supplies the CSS
variables and any custom CSS once.
| public function dynamicFavicon(Request $request): Response | ||
| { | ||
| $domain = $request->getHost(); | ||
| $branding = WhiteLabelConfig::findByDomain($domain); | ||
|
|
||
| if ($branding && $branding->getLogoUrl()) { | ||
| // Redirect to custom logo | ||
| return redirect($branding->getLogoUrl()); | ||
| } | ||
|
|
||
| // Serve default favicon | ||
| $defaultFavicon = public_path('favicon.ico'); | ||
| if (file_exists($defaultFavicon)) { | ||
| return response(file_get_contents($defaultFavicon), 200, [ | ||
| 'Content-Type' => 'image/x-icon', | ||
| 'Cache-Control' => 'public, max-age=86400', | ||
| ]); | ||
| } | ||
|
|
||
| return response('', 404); | ||
| } |
There was a problem hiding this comment.
Potential open redirect vulnerability via favicon endpoint.
The getLogoUrl() is user-controlled and redirecting to arbitrary URLs can be exploited for phishing. Validate the URL or serve the asset directly instead of redirecting.
Consider validating the URL scheme and domain, or proxy the asset:
if ($branding && $branding->getLogoUrl()) {
- // Redirect to custom logo
- return redirect($branding->getLogoUrl());
+ $logoUrl = $branding->getLogoUrl();
+ // Only redirect to HTTPS URLs on trusted domains or relative paths
+ if (str_starts_with($logoUrl, '/') || $this->isAllowedAssetUrl($logoUrl)) {
+ return redirect($logoUrl);
+ }
+ // Fall through to default favicon if URL is not allowed
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/Http/Controllers/DynamicAssetController.php around lines 178 to 198, the
controller currently redirects to a user-controlled getLogoUrl(), creating an
open-redirect risk; instead validate the URL or proxy the asset: check that the
returned URL is a well-formed absolute URL with an allowed scheme (http or
https) and either matches an allowlist of trusted hostnames or is same-origin,
and only perform a redirect if it passes validation; otherwise fetch the remote
asset server-side (with a timeout and size limit), validate the Content-Type
(image/*) and return the binary data with proper headers, falling back to
serving the local favicon or returning 404 when validation/fetch fails.
| public function debugBranding(Request $request): array | ||
| { | ||
| $domain = $request->getHost(); | ||
| $branding = WhiteLabelConfig::findByDomain($domain); | ||
|
|
||
| return [ | ||
| 'domain' => $domain, | ||
| 'has_custom_branding' => $branding !== null, | ||
| 'platform_name' => $branding?->getPlatformName() ?? 'Coolify (Default)', | ||
| 'custom_logo' => $branding?->getLogoUrl(), | ||
| 'theme_variables' => $branding?->getThemeVariables() ?? WhiteLabelConfig::createDefault('')->getDefaultThemeVariables(), | ||
| 'custom_domains' => $branding?->getCustomDomains() ?? [], | ||
| 'hide_coolify_branding' => $branding?->shouldHideCoolifyBranding() ?? false, | ||
| 'organization_id' => $branding?->organization_id, | ||
| 'request_headers' => [ | ||
| 'host' => $request->header('host'), | ||
| 'user_agent' => $request->header('user-agent'), | ||
| 'x_forwarded_host' => $request->header('x-forwarded-host'), | ||
| ], | ||
| ]; | ||
| } |
There was a problem hiding this comment.
Debug endpoint exposes sensitive information without authorization.
This endpoint returns organization_id, request headers, and internal branding configuration without any authentication. This aids reconnaissance and should be restricted.
Restrict to non-production environments or add authorization:
public function debugBranding(Request $request): array
{
+ if (app()->environment('production')) {
+ abort(404);
+ }
+
$domain = $request->getHost();
$branding = WhiteLabelConfig::findByDomain($domain);
return [
'domain' => $domain,
'has_custom_branding' => $branding !== null,
'platform_name' => $branding?->getPlatformName() ?? 'Coolify (Default)',
'custom_logo' => $branding?->getLogoUrl(),
'theme_variables' => $branding?->getThemeVariables() ?? WhiteLabelConfig::createDefault('')->getDefaultThemeVariables(),
'custom_domains' => $branding?->getCustomDomains() ?? [],
'hide_coolify_branding' => $branding?->shouldHideCoolifyBranding() ?? false,
- 'organization_id' => $branding?->organization_id,
- 'request_headers' => [
- 'host' => $request->header('host'),
- 'user_agent' => $request->header('user-agent'),
- 'x_forwarded_host' => $request->header('x-forwarded-host'),
- ],
];
}🤖 Prompt for AI Agents
In app/Http/Controllers/DynamicAssetController.php around lines 203 to 223, the
debugBranding endpoint returns sensitive data (organization_id, request headers
and internal branding) with no authorization; restrict access by gating this
endpoint to non-production environments or adding an authorization check (e.g.,
require a configured env flag like APP_DEBUG_BRANDING or an authenticated admin
role/API key) before building the response, and remove or redact sensitive
fields (organization_id, request_headers) unless the caller is authorized;
ensure any gating logic uses existing auth middleware or environment config and
return a safe minimal response (domain, has_custom_branding, non-sensitive
branding flags) for unauthorized/production requests.
| 'has_custom_branding' => $branding !== null, | ||
| 'platform_name' => $branding?->getPlatformName() ?? 'Coolify (Default)', | ||
| 'custom_logo' => $branding?->getLogoUrl(), | ||
| 'theme_variables' => $branding?->getThemeVariables() ?? WhiteLabelConfig::createDefault('')->getDefaultThemeVariables(), |
There was a problem hiding this comment.
Calling createDefault('') may create invalid database records.
createDefault('') calls self::create() which persists to the database with an empty organization_id. Use getDefaultThemeVariables() on a new (unsaved) instance instead.
- 'theme_variables' => $branding?->getThemeVariables() ?? WhiteLabelConfig::createDefault('')->getDefaultThemeVariables(),
+ 'theme_variables' => $branding?->getThemeVariables() ?? (new WhiteLabelConfig())->getDefaultThemeVariables(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 'theme_variables' => $branding?->getThemeVariables() ?? WhiteLabelConfig::createDefault('')->getDefaultThemeVariables(), | |
| 'theme_variables' => $branding?->getThemeVariables() ?? (new WhiteLabelConfig())->getDefaultThemeVariables(), |
🤖 Prompt for AI Agents
In app/Http/Controllers/DynamicAssetController.php around line 213, the code
calls WhiteLabelConfig::createDefault('') which invokes self::create() and
persists a record with an empty organization_id; replace that with a new,
unsaved WhiteLabelConfig instance and call getDefaultThemeVariables() on it
(i.e., instantiate a WhiteLabelConfig without saving and use its
getDefaultThemeVariables method) so no invalid DB record is created.
| /** | ||
| * Preview theme changes | ||
| */ | ||
| public function previewTheme(Request $request, string $organizationId) | ||
| { | ||
| $organization = Organization::findOrFail($organizationId); | ||
|
|
||
| Gate::authorize('manage-branding', $organization); | ||
|
|
||
| $config = WhiteLabelConfig::where('organization_id', $organization->id)->firstOrFail(); | ||
|
|
||
| // Create temporary config with preview changes | ||
| $tempConfig = clone $config; | ||
| $tempConfig->theme_config = $request->input('theme_config', $config->theme_config); | ||
| $tempConfig->custom_css = $request->input('custom_css', $config->custom_css); | ||
|
|
||
| $compiledCss = $this->whiteLabelService->compileTheme($tempConfig); | ||
|
|
||
| return response()->json([ | ||
| 'success' => true, | ||
| 'compiled_css' => $compiledCss, | ||
| ]); | ||
| } |
There was a problem hiding this comment.
Preview theme incorrectly caches the temporary configuration.
The cloned config is passed to compileTheme() which calls cacheCompiledTheme(), persisting the preview to cache and overwriting the actual theme.
Either skip caching for previews or use a dedicated preview method:
public function previewTheme(Request $request, string $organizationId)
{
$organization = Organization::findOrFail($organizationId);
Gate::authorize('manage-branding', $organization);
$config = WhiteLabelConfig::where('organization_id', $organization->id)->firstOrFail();
// Create temporary config with preview changes
$tempConfig = clone $config;
$tempConfig->theme_config = $request->input('theme_config', $config->theme_config);
$tempConfig->custom_css = $request->input('custom_css', $config->custom_css);
- $compiledCss = $this->whiteLabelService->compileTheme($tempConfig);
+ // Compile without caching for preview
+ $compiledCss = $this->whiteLabelService->compileThemePreview($tempConfig);
return response()->json([
'success' => true,
'compiled_css' => $compiledCss,
]);
}Add a non-caching compile method to WhiteLabelService:
public function compileThemePreview(WhiteLabelConfig $config): string
{
// Same as compileTheme but without cacheCompiledTheme call
}🤖 Prompt for AI Agents
In app/Http/Controllers/Enterprise/BrandingController.php around lines 162 to
184, the previewTheme method passes a cloned WhiteLabelConfig into
compileTheme(), but compileTheme() writes the compiled result into the cache
which persists the preview and overwrites the real theme; change the controller
to call a non-caching compile method (e.g. add
compileThemePreview(WhiteLabelConfig $config): string to WhiteLabelService) or
extend compileTheme with a boolean $cache = true flag and call it with $cache =
false for previews, ensuring the preview path does not invoke
cacheCompiledTheme() or write any cache keys so previews remain transient.
| // Add custom CSS if provided | ||
| if ($config->custom_css) { | ||
| $styles .= "\n/* Custom CSS */\n".$config->custom_css; | ||
| } | ||
|
|
||
| return $styles; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for CssValidationService to understand its implementation and usage
rg -n "CssValidationService" --type=php -C 3Repository: johnproblems/topgun
Length of output: 1952
🏁 Script executed:
#!/bin/bash
# Search for WhiteLabelConfig class and custom_css property
rg -n "custom_css" --type=php -C 5Repository: johnproblems/topgun
Length of output: 15727
🏁 Script executed:
#!/bin/bash
# Find where custom_css is assigned or stored
rg -n "custom_css\s*=" --type=php -B 2 -A 2Repository: johnproblems/topgun
Length of output: 649
🏁 Script executed:
#!/bin/bash
# Read the CssValidationService to understand what it does
cat -n app/Services/Enterprise/CssValidationService.phpRepository: johnproblems/topgun
Length of output: 4048
🏁 Script executed:
#!/bin/bash
# Check if there's any validation in BrandingController before saving custom_css
rg -n "CssValidationService\|custom_css" --type=php app/Http/Controllers/Enterprise/BrandingController.php -C 5Repository: johnproblems/topgun
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Find all calls to sanitize() to see where validation is applied
rg -n "\.sanitize\(" --type=php -C 2Repository: johnproblems/topgun
Length of output: 400
🏁 Script executed:
#!/bin/bash
# Check BrandingController update method more carefully
sed -n '74,90p' app/Http/Controllers/Enterprise/BrandingController.phpRepository: johnproblems/topgun
Length of output: 666
🏁 Script executed:
#!/bin/bash
# Check WhiteLabelConfig model for any mutators or sanitization
rg -n "setAttribute\|custom_css" app/Models/WhiteLabelConfig.php -A 3Repository: johnproblems/topgun
Length of output: 45
🏁 Script executed:
#!/bin/bash
# Check if there are any observers or events that might sanitize custom_css
rg -n "WhiteLabelConfig|custom_css" --type=php app/Models/ -B 2 -A 2 | grep -E "(Observer|Listener|Event|saved|saving|create|update)"Repository: johnproblems/topgun
Length of output: 45
Add validation to EmailTemplateService to prevent CSS injection, and consider sanitizing at storage time for defense-in-depth.
The review comment is valid. Investigation confirms that custom_css is stored without validation in BrandingController (line 84 only validates type and length, not content safety). While CssValidationService exists with robust sanitization logic, it's only applied in one runtime path (Enterprise/DynamicAssetController line 117), not before storage.
This means multiple code paths retrieve and use custom_css directly without sanitization:
EmailTemplateService.php:335-336(the flagged lines)WhiteLabelService.php:194-195DynamicAssetController.php:55-57(non-Enterprise version)WhiteLabelConfig.php:86-87
The recommended approach:
- Short-term: Apply
CssValidationService::sanitize()inEmailTemplateServiceand other services that outputcustom_css. - Stronger: Sanitize
custom_cssinBrandingControllerbefore storage, ensuring all code paths benefit from validation.
The original suggested refactor should include actually calling the validator rather than just a comment:
// Add custom CSS if provided
if ($config->custom_css) {
- $styles .= "\n/* Custom CSS */\n".$config->custom_css;
+ $sanitized = app(CssValidationService::class)->sanitize($config->custom_css);
+ $styles .= "\n/* Custom CSS */\n".$sanitized;
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/Services/Enterprise/EmailTemplateService.php around lines 334 to 340, the
service appends raw $config->custom_css into email styles which allows CSS
injection; call CssValidationService::sanitize($config->custom_css) and use the
sanitized result when building $styles (replace direct use of
$config->custom_css), and additionally add a short note to sanitize on write in
BrandingController so $config->custom_css is sanitized before storage to provide
defense-in-depth.
| protected function inlineCss(string $html, WhiteLabelConfig $config): string | ||
| { | ||
| // Extract styles from HTML | ||
| preg_match_all('/<style[^>]*>(.*?)<\/style>/si', $html, $matches); | ||
| $css = implode("\n", $matches[1]); | ||
|
|
||
| // Remove style tags | ||
| $html = preg_replace('/<style[^>]*>.*?<\/style>/si', '', $html); | ||
|
|
||
| // Inline the CSS | ||
| if (! empty($css)) { | ||
| $html = $this->cssInliner->convert($html, $css); | ||
| } | ||
|
|
||
| return $html; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove unused $config parameter.
The static analysis correctly identifies that the $config parameter is never used in this method.
- protected function inlineCss(string $html, WhiteLabelConfig $config): string
+ protected function inlineCss(string $html): string
{
// Extract styles from HTML
preg_match_all('/<style[^>]*>(.*?)<\/style>/si', $html, $matches);Also update the call site on line 52:
- $html = $this->inlineCss($html, $config);
+ $html = $this->inlineCss($html);Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 PHPMD (2.15.0)
345-345: Avoid unused parameters such as '$config'. (undefined)
(UnusedFormalParameter)
🤖 Prompt for AI Agents
In app/Services/Enterprise/EmailTemplateService.php around lines 345 to 360, the
inlineCss method accepts a WhiteLabelConfig $config parameter that is never
used; remove the unused $config parameter from the method signature and update
every call site (including the call on line 52) to stop passing the config
argument, ensuring argument counts match; search the class for other references
to inlineCss and adjust them accordingly, then run/static-analyze/tests to
confirm no remaining signature mismatches.
| $sassVariables = $this->generateSassVariables($config->theme_config); | ||
| $sassInput = $sassVariables."\n".File::get($templatePath); |
There was a problem hiding this comment.
Use getThemeVariables() instead of accessing theme_config directly.
Accessing $config->theme_config bypasses the default values merged in getThemeVariables(). If theme_config is empty or missing keys, the compiled CSS won't include the default theme colors.
- $sassVariables = $this->generateSassVariables($config->theme_config);
+ $sassVariables = $this->generateSassVariables($config->getThemeVariables());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $sassVariables = $this->generateSassVariables($config->theme_config); | |
| $sassInput = $sassVariables."\n".File::get($templatePath); | |
| $sassVariables = $this->generateSassVariables($config->getThemeVariables()); | |
| $sassInput = $sassVariables."\n".File::get($templatePath); |
🤖 Prompt for AI Agents
In app/Services/Enterprise/SassCompilationService.php around lines 42-43, the
code reads the raw $config->theme_config which bypasses merged default values;
replace that access with the merged defaults by calling
$config->getThemeVariables() and pass its result into generateSassVariables so
the compiled SASS uses defaults when keys are missing.
| // Add custom CSS if provided | ||
| if ($config->custom_css) { | ||
| $css .= "\n/* Custom CSS */\n".$config->custom_css; | ||
| } |
There was a problem hiding this comment.
Custom CSS should be sanitized before inclusion.
The CssValidationService exists but custom_css is appended directly without sanitization. This could include malicious CSS.
Inject and use the CSS validation service:
+ protected CssValidationService $cssValidator;
+
public function __construct(
BrandingCacheService $cacheService,
DomainValidationService $domainService,
- EmailTemplateService $emailService
+ EmailTemplateService $emailService,
+ CssValidationService $cssValidator
) {
// ... existing assignments
+ $this->cssValidator = $cssValidator;
}Then in compileTheme:
// Add custom CSS if provided
if ($config->custom_css) {
- $css .= "\n/* Custom CSS */\n".$config->custom_css;
+ $css .= "\n/* Custom CSS */\n".$this->cssValidator->sanitize($config->custom_css);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/Services/Enterprise/WhiteLabelService.php around lines 193 to 196, the
code appends $config->custom_css directly which risks injecting malicious CSS;
inject the CssValidationService into WhiteLabelService (constructor or property
via dependency injection) and in compileTheme call the service to
validate/sanitize $config->custom_css before appending; replace the direct
append with appending the sanitized result only if validation succeeds (or
log/ignore invalid CSS), and ensure any exceptions from the validator are caught
and handled so theme compilation cannot be exploited.
| public function importConfiguration(WhiteLabelConfig $config, array $data): void | ||
| { | ||
| // Validate import data | ||
| $this->validateImportData($data); | ||
|
|
||
| // Import configuration | ||
| $config->update([ | ||
| 'platform_name' => $data['platform_name'] ?? $config->platform_name, | ||
| 'theme_config' => $data['theme_config'] ?? $config->theme_config, | ||
| 'custom_css' => $data['custom_css'] ?? $config->custom_css, | ||
| 'custom_email_templates' => $data['email_templates'] ?? $config->custom_email_templates, | ||
| 'hide_coolify_branding' => $data['hide_coolify_branding'] ?? $config->hide_coolify_branding, | ||
| ]); | ||
|
|
||
| // Clear cache | ||
| $this->cacheService->clearOrganizationCache($config->organization_id); | ||
| } |
There was a problem hiding this comment.
Imported configuration bypasses CSS and theme validation.
importConfiguration accepts custom_css and theme_config without sanitization. This bypasses the CssValidationService and color validation that the normal update flow would apply.
Validate imported data:
public function importConfiguration(WhiteLabelConfig $config, array $data): void
{
// Validate import data
$this->validateImportData($data);
+ // Sanitize custom CSS if present
+ if (isset($data['custom_css']) && $data['custom_css']) {
+ $cssValidator = app(CssValidationService::class);
+ $data['custom_css'] = $cssValidator->sanitize($data['custom_css']);
+ }
+
+ // Validate theme colors if present
+ if (isset($data['theme_config']) && is_array($data['theme_config'])) {
+ foreach ($data['theme_config'] as $key => $value) {
+ if (str_ends_with($key, '_color') && !$config->isValidThemeColor($value)) {
+ throw new \InvalidArgumentException("Invalid color value for {$key}");
+ }
+ }
+ }
+
// Import configuration
$config->update([Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/Services/Enterprise/WhiteLabelService.php around lines 427 to 443, the
importConfiguration method currently writes custom_css and theme_config directly
to the model and bypasses the normal CSS and theme color validation; update this
method to run incoming custom_css through the CssValidationService (or existing
validation helper) and run theme_config through the same color/structure
validation used by the regular update flow, reject or sanitize invalid values
(throw a ValidationException or omit invalid fields) and only persist
validated/sanitized values (fall back to the current config values when
validation fails), then continue to update and clear cache as before.
Summary
Comprehensive white-label branding system allowing custom domain configuration, CSS/SCSS customization, email template branding, and dynamic asset compilation.
Features
Database
create_white_label_configs_tableWhiteLabelConfigFactoryTesting
All white-label features tested and verified.
🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com
Note
Introduces a white‑label branding system with org/domain‑specific theming, SASS‑compiled assets, email template branding, domain validation, and caching, plus management endpoints and DB schema.
app/Http/Controllers/Enterprise/BrandingController: CRUD for branding, theme preview/update, logo upload, custom domains (validate/add/remove), email templates (edit/preview/reset), cache stats/clear, import/export, reset.app/Http/Controllers/Enterprise/DynamicAssetController: Serve org‑scoped CSS with SASS compilation, custom CSS sanitization, ETag/Cache, error handling.app/Http/Controllers/DynamicAssetController: Domain‑based CSS/favicons and branding debug endpoint.DynamicBrandingMiddleware: Injects domain‑resolved branding into app context/views.WhiteLabelService: Core theme compile, domain setup, email generation, import/export, asset processing (logos/favicons).SassCompilationService: Compile SCSS (theme.scss,dark.scss).CssValidationService: Sanitize/validate custom CSS.DomainValidationService: DNS/SSL checks, ownership token, comprehensive validation.BrandingCacheService: Theme/asset/config caching, versioning, warming, invalidation.EmailTemplateService: Branded templates + CSS inlining and previews.WhiteLabelConfigmodel with theme vars, custom domains, email templates, helpers; factory and migrationcreate_white_label_configs_table.config/enterprise.phpdefaults for theme/cache.resources/sass/**.Written by Cursor Bugbot for commit 4f100f4. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.