-
-
Notifications
You must be signed in to change notification settings - Fork 69
Add Laravel Forge hosting integration #318
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Add a new hosting provider integration for Laravel Forge that supports: - Single server and multi-server setups - Load balancer configuration - Auto SSL via Let's Encrypt - Custom deploy commands and symlink support This integration follows the established host provider patterns and stores credentials securely in wp-config.php constants. New files: - inc/integrations/host-providers/class-laravel-forge-host-provider.php - views/wizards/host-integrations/laravel-forge-instructions.php - assets/img/hosts/laravel-forge.svg Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughA complete Laravel Forge host provider integration enabling automatic domain management on Forge servers, including site creation, SSL provisioning, load balancing, and deployment commands. The integration is registered within the domain manager and includes user-facing instructional content. Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin Interface
participant Manager as Domain Manager
participant Provider as Forge Provider
participant API as Forge API
Admin->>Manager: Add new domain to site
activate Manager
Manager->>Provider: on_add_domain(domain, site_id)
activate Provider
Provider->>Provider: get_primary_server_id()
Provider->>API: send_forge_request(create site)
activate API
API-->>Provider: site_id response
deactivate API
Provider->>API: send_forge_request(install SSL)
activate API
API-->>Provider: SSL status
deactivate API
alt Load Balancer Configured
Provider->>Provider: get_load_balancer_server_id()
Provider->>API: send_forge_request(create LB site)
activate API
API-->>Provider: LB site_id
deactivate API
Provider->>API: send_forge_request(configure load balancing)
activate API
API-->>Provider: status
deactivate API
end
alt Additional Servers Configured
loop For each server
Provider->>API: send_forge_request(create site on server)
activate API
API-->>Provider: site_id
deactivate API
end
end
alt Deploy Commands Configured
Provider->>API: send_forge_request(run deploy)
activate API
API-->>Provider: deploy status
deactivate API
end
Provider-->>Manager: Domain provisioning complete
deactivate Provider
Manager-->>Admin: Domain activated
deactivate Manager
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @inc/integrations/host-providers/class-laravel-forge-host-provider.php:
- Around line 578-585: The current logging in the API request block (building
$log_message and calling wu_log_add('integration-forge', ...)) exposes sensitive
data by including the full $url and response body; change it to log only
non-sensitive fields: HTTP method ($method), the endpoint path derived from $url
with query string removed, and the status code (use 'ERROR' for WP_Error), and
do not include the response body unless a debug constant (e.g.,
FORGE_PROVIDER_DEBUG) is true; if body logging is enabled, sanitize it first
(strip Authorization headers, tokens, and any keys) before appending. Ensure you
update the same code that constructs $log_message and calls wu_log_add so it
conditionally includes the sanitized body only when debug is enabled.
🧹 Nitpick comments (5)
inc/integrations/host-providers/class-laravel-forge-host-provider.php (5)
104-107: Consider strengthening the detection heuristic.The current detection checks if the string 'forge' appears anywhere in
ABSPATH, which could produce false positives (e.g., paths like/var/www/forged-site/or/home/forge-user/).Consider a more specific pattern such as checking for
/home/forge/or verifying Forge-specific environment markers.🔍 Proposed enhancement
public function detect(): bool { - return str_contains(ABSPATH, 'forge'); + // Check for the standard Forge path structure + return str_contains(ABSPATH, '/home/forge/') || + (defined('LARAVEL_FORGE') && LARAVEL_FORGE); }
171-171: Prefix unused parameters with underscore to indicate intentional non-use.Several methods have unused parameters that are part of the base class contract. PHP convention is to prefix unused parameters with an underscore to clarify they're intentionally not used.
♻️ Proposed fix
-public function on_add_domain($domain, $site_id): void { +public function on_add_domain($domain, $_site_id): void { -public function on_remove_domain($domain, $site_id): void { +public function on_remove_domain($domain, $_site_id): void { -public function on_add_subdomain($subdomain, $site_id): void { +public function on_add_subdomain($_subdomain, $_site_id): void { -public function on_remove_subdomain($subdomain, $site_id): void { +public function on_remove_subdomain($_subdomain, $_site_id): void {Also applies to: 247-247, 293-293, 307-307
186-186: Fix type inconsistency in server ID comparison.The comparison casts both values to strings, but
$server_idfrom the loop is an integer. For consistency and to avoid potential type coercion issues, compare as integers directly.♻️ Proposed fix
-$is_load_balancer = $load_balancer_server_id && (string) $server_id === (string) $load_balancer_server_id; +$is_load_balancer = $load_balancer_server_id && (int) $server_id === (int) $load_balancer_server_id;
356-373: Consider making project type and directory configurable.The
project_typeanddirectoryare hardcoded to'php'and'/public', which works for standard WordPress setups but may not suit all environments. Users with different server configurations might need customization.💡 Suggested enhancement
Add optional constants to allow customization:
protected $optional_constants = [ 'WU_FORGE_LOAD_BALANCER_SERVER_ID', 'WU_FORGE_LOAD_BALANCER_SITE_ID', 'WU_FORGE_ADDITIONAL_SERVER_IDS', 'WU_FORGE_DEPLOY_COMMAND', 'WU_FORGE_SYMLINK_TARGET', + 'WU_FORGE_PROJECT_TYPE', + 'WU_FORGE_DIRECTORY', ];Then update the method:
protected function create_site_on_server(int $server_id, string $domain) { + $project_type = defined('WU_FORGE_PROJECT_TYPE') ? WU_FORGE_PROJECT_TYPE : 'php'; + $directory = defined('WU_FORGE_DIRECTORY') ? WU_FORGE_DIRECTORY : '/public'; + $response = $this->send_forge_request( sprintf('/servers/%s/sites', $server_id), [ 'domain' => $domain, - 'project_type' => 'php', - 'directory' => '/public', + 'project_type' => $project_type, + 'directory' => $directory, ], 'POST' );
424-453: Consider making load balancing method configurable.The load balancing method is hardcoded to
'least_conn'. While this is a reasonable default, users might prefer'round_robin'or'ip_hash'depending on their use case.💡 Suggested enhancement
protected function configure_load_balancing(int $lb_server_id, int $lb_site_id, array $backend_servers): void { $servers = []; foreach ($backend_servers as $backend) { $servers[] = [ 'id' => (int) $backend['server_id'], 'weight' => 1, ]; } + $method = defined('WU_FORGE_LB_METHOD') ? WU_FORGE_LB_METHOD : 'least_conn'; + $response = $this->send_forge_request( sprintf('/servers/%s/sites/%s/balancing', $lb_server_id, $lb_site_id), [ 'servers' => $servers, - 'method' => 'least_conn', + 'method' => $method, ], 'PUT' );
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
assets/img/hosts/laravel-forge.svgis excluded by!**/*.svg
📒 Files selected for processing (3)
inc/integrations/host-providers/class-laravel-forge-host-provider.phpinc/managers/class-domain-manager.phpviews/wizards/host-integrations/laravel-forge-instructions.php
🧰 Additional context used
🧬 Code graph analysis (2)
inc/managers/class-domain-manager.php (1)
inc/integrations/host-providers/class-laravel-forge-host-provider.php (1)
Laravel_Forge_Host_Provider(27-801)
inc/integrations/host-providers/class-laravel-forge-host-provider.php (3)
inc/integrations/host-providers/class-base-host-provider.php (1)
supports(337-340)inc/functions/helper.php (1)
wu_log_add(208-211)inc/functions/template.php (1)
wu_get_template(20-73)
🪛 PHPMD (2.15.0)
inc/integrations/host-providers/class-laravel-forge-host-provider.php
171-171: Avoid unused parameters such as '$site_id'. (undefined)
(UnusedFormalParameter)
247-247: Avoid unused parameters such as '$site_id'. (undefined)
(UnusedFormalParameter)
293-293: Avoid unused parameters such as '$subdomain'. (undefined)
(UnusedFormalParameter)
293-293: Avoid unused parameters such as '$site_id'. (undefined)
(UnusedFormalParameter)
307-307: Avoid unused parameters such as '$subdomain'. (undefined)
(UnusedFormalParameter)
307-307: Avoid unused parameters such as '$site_id'. (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). (3)
- GitHub Check: Build Plugin for Testing
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: cypress (8.1, chrome)
🔇 Additional comments (7)
inc/integrations/host-providers/class-laravel-forge-host-provider.php (3)
220-224: Verify SSL installation logic when load balancer site creation fails.If
$load_balancer_server_idis configured but the load balancer site creation fails (making$load_balancer_site_idnull), the fallback uses$this->get_primary_site_id()which corresponds to the primary server, not the load balancer server. This mismatch could cause SSL installation to fail or install on the wrong server.Consider explicitly checking if load balancer site creation succeeded before attempting SSL installation on the load balancer:
🔍 Proposed enhancement
// Setup SSL on load balancer or primary server. -$ssl_server_id = $load_balancer_server_id ?: $this->get_primary_server_id(); -$ssl_site_id = $load_balancer_site_id ?: $this->get_primary_site_id(); +if ($load_balancer_server_id && $load_balancer_site_id) { + $ssl_server_id = $load_balancer_server_id; + $ssl_site_id = $load_balancer_site_id; +} else { + // Fall back to primary server if no load balancer or LB site creation failed + $ssl_server_id = $this->get_primary_server_id(); + $ssl_site_id = $this->get_primary_site_id(); +} if ($ssl_server_id && $ssl_site_id) { $this->install_ssl_certificate($ssl_server_id, $ssl_site_id, $domain); }
687-717: LGTM!The server list building logic correctly handles load balancer, primary, and additional servers while avoiding duplicates through strict comparisons.
318-345: LGTM!The connection test implementation properly validates the server configuration and handles errors appropriately.
inc/managers/class-domain-manager.php (1)
1013-1016: LGTM!The Laravel Forge integration is properly registered following the same pattern as other host provider integrations.
views/wizards/host-integrations/laravel-forge-instructions.php (3)
1-13: LGTM!The file header and introduction properly use WordPress i18n functions and include the necessary security check.
15-93: LGTM!The step-by-step instructions are clear, comprehensive, and properly internationalized. The warning about API token visibility is appropriately highlighted.
95-105: LGTM!The SSL certificate section provides important information about automatic certificate provisioning and includes a helpful reminder about DNS propagation requirements.
| $log_message = sprintf( | ||
| "Request: %s %s\nStatus: %s\nResponse: %s", | ||
| $method, | ||
| $url, | ||
| is_wp_error($response) ? 'ERROR' : wp_remote_retrieve_response_code($response), | ||
| is_wp_error($response) ? $response->get_error_message() : wp_remote_retrieve_body($response) | ||
| ); | ||
| wu_log_add('integration-forge', $log_message); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid logging sensitive data in API requests.
The current implementation logs the complete request URL, method, and response body for every API call. This could inadvertently log sensitive information such as API tokens, SSL certificate data, or other credentials in the response bodies.
Consider either:
- Limiting logging to non-sensitive information (method, endpoint, status code only)
- Sanitizing the response body before logging
- Making detailed logging opt-in via a debug constant
🔒 Proposed fix to sanitize logs
// Log the request for debugging.
+$sanitized_response = is_wp_error($response)
+ ? $response->get_error_message()
+ : wp_remote_retrieve_response_code($response) . ' - [Response body hidden for security]';
+
$log_message = sprintf(
- "Request: %s %s\nStatus: %s\nResponse: %s",
+ "Request: %s %s\nStatus: %s",
$method,
- $url,
- is_wp_error($response) ? 'ERROR' : wp_remote_retrieve_response_code($response),
- is_wp_error($response) ? $response->get_error_message() : wp_remote_retrieve_body($response)
+ preg_replace('/Bearer\s+[^\s]+/', 'Bearer [REDACTED]', $url),
+ $sanitized_response
);
wu_log_add('integration-forge', $log_message);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @inc/integrations/host-providers/class-laravel-forge-host-provider.php around
lines 578 - 585, The current logging in the API request block (building
$log_message and calling wu_log_add('integration-forge', ...)) exposes sensitive
data by including the full $url and response body; change it to log only
non-sensitive fields: HTTP method ($method), the endpoint path derived from $url
with query string removed, and the status code (use 'ERROR' for WP_Error), and
do not include the response body unless a debug constant (e.g.,
FORGE_PROVIDER_DEBUG) is true; if body logging is enabled, sanitize it first
(strip Authorization headers, tokens, and any keys) before appending. Ensure you
update the same code that constructs $log_message and calls wu_log_add so it
conditionally includes the sanitized body only when debug is enabled.
| } elseif (defined('WU_FORGE_SYMLINK_TARGET') && WU_FORGE_SYMLINK_TARGET) { | ||
| // Build symlink command if target is specified. | ||
| $target = str_replace('{domain}', $domain, WU_FORGE_SYMLINK_TARGET); | ||
| $command = sprintf( | ||
| 'rm -rf /home/forge/%s/* && ln -s %s /home/forge/%s/public', | ||
| $domain, | ||
| $target, | ||
| $domain | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validate domain input before using in shell commands to prevent command injection.
The generated deploy command uses rm -rf with the domain name directly interpolated. While WordPress likely validates domain names, an attacker-controlled domain value could potentially contain shell metacharacters, leading to command injection.
Ensure the domain is properly validated and escaped before use in shell commands:
🔒 Proposed security fix
} elseif (defined('WU_FORGE_SYMLINK_TARGET') && WU_FORGE_SYMLINK_TARGET) {
// Build symlink command if target is specified.
+ // Validate domain to prevent command injection
+ if (!preg_match('/^[a-z0-9][a-z0-9\-\.]*[a-z0-9]$/i', $domain)) {
+ wu_log_add(
+ 'integration-forge',
+ sprintf('Invalid domain format for shell command: %s', $domain),
+ LogLevel::ERROR
+ );
+ return '';
+ }
+
$target = str_replace('{domain}', $domain, WU_FORGE_SYMLINK_TARGET);
$command = sprintf(
- 'rm -rf /home/forge/%s/* && ln -s %s /home/forge/%s/public',
- $domain,
- $target,
- $domain
+ 'rm -rf %s && ln -s %s %s',
+ escapeshellarg('/home/forge/' . $domain . '/*'),
+ escapeshellarg($target),
+ escapeshellarg('/home/forge/' . $domain . '/public')
);
}Committable suggestion skipped: line range outside the PR's diff.
Summary
Configuration Constants
Required:
WU_FORGE_API_TOKEN- Forge API tokenWU_FORGE_SERVER_ID- Primary server IDWU_FORGE_SITE_ID- Primary site IDOptional (for advanced setups):
WU_FORGE_LOAD_BALANCER_SERVER_IDWU_FORGE_LOAD_BALANCER_SITE_IDWU_FORGE_ADDITIONAL_SERVER_IDSWU_FORGE_DEPLOY_COMMANDWU_FORGE_SYMLINK_TARGETFiles Changed
inc/integrations/host-providers/class-laravel-forge-host-provider.php- Main integration classviews/wizards/host-integrations/laravel-forge-instructions.php- Setup instructionsassets/img/hosts/laravel-forge.svg- Integration logoinc/managers/class-domain-manager.php- Register the integrationTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.