Feature/ansible wp by janw-me · Pull Request #53 · SynioBE/SWORD

janw-me · 2026-03-22T12:25:23Z

Not fully tested

Copilot

Pull request overview

This PR extends the app’s Ansible integration to support WordPress site installation, alongside the existing server provisioning flow. It adds a new Ansible playbook for site creation and wires it into the site installation job.

Changes:

Extend App\Services\Ansible to generate inventory entries for servers and sites, and add runServerPlaybook() / runSitePlaybook() entrypoints.
Update RunAnsible job to call runServerPlaybook() for provisioning.
Add a new Ansible playbook (create-wp.yml) to provision a WordPress site via Docker (PHP-FPM + Nginx + Redis) and report progress via callback.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 12 comments.

File	Description
app/Services/Ansible.php	Adds site inventory data and new playbook runners for servers and sites
app/Jobs/RunAnsible.php	Switches server provisioning execution to `runServerPlaybook()`
app/Jobs/InstallSiteJob.php	Attempts to trigger site install via Ansible (behind feature flag)
ansible-playbooks/create-wp.yml	New playbook to deploy a WordPress stack and report progress callbacks

Comments suppressed due to low confidence (1)

app/Services/Ansible.php:172

$LimitServer and $playbookpath are interpolated into a shell command without escaping. Since server names are only validated as a generic string, a crafted server name (or limit) could break the command or enable command injection. Build the Process command using an argument array (or at minimum escapeshellarg() for both $playbookpath and $LimitServer).

        $command = 'ANSIBLE_HOST_KEY_CHECKING=false ';
        $command .= 'ansible-playbook';
        $command .= ' -i /tmp/ansible-sword-inventory.yml';
        $command .= " $playbookpath";
        $command .= " --limit $LimitServer";
        $command .= ' --extra-vars '.escapeshellarg(\json_encode($extraVars));

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-22T12:29:28Z

+          docker exec sword_mysql mysql -uroot -p"{{ mysql_root_password }}"
+          -e "CREATE DATABASE IF NOT EXISTS `{{ db_name }}` CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;"
+
+    - name: Create MySQL user
+      ansible.builtin.command:
+        cmd: >
+          docker exec sword_mysql mysql -uroot -p"{{ mysql_root_password }}"
+          -e "CREATE USER IF NOT EXISTS '{{ db_user }}'@'%' IDENTIFIED BY '{{ db_password }}';"
+
+    - name: Grant MySQL privileges
+      ansible.builtin.command:
+        cmd: >
+          docker exec sword_mysql mysql -uroot -p"{{ mysql_root_password }}"


The MySQL root password is passed on the command line (mysql -p"{{ mysql_root_password }}"), which can be exposed via process listings on the remote host. Prefer using a safer mechanism (e.g., MYSQL_PWD env for the command, a temporary .my.cnf, or community.mysql modules with a config file) to avoid leaking credentials.

Suggested change

docker exec sword_mysql mysql -uroot -p"{{ mysql_root_password }}"

-e "CREATE DATABASE IF NOT EXISTS `{{ db_name }}` CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;"

- name: Create MySQL user

ansible.builtin.command:

cmd: >

docker exec sword_mysql mysql -uroot -p"{{ mysql_root_password }}"

-e "CREATE USER IF NOT EXISTS '{{ db_user }}'@'%' IDENTIFIED BY '{{ db_password }}';"

- name: Grant MySQL privileges

ansible.builtin.command:

cmd: >

docker exec sword_mysql mysql -uroot -p"{{ mysql_root_password }}"

docker exec -e MYSQL_PWD="{{ mysql_root_password }}" sword_mysql mysql -uroot

-e "CREATE DATABASE IF NOT EXISTS `{{ db_name }}` CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;"

- name: Create MySQL user

ansible.builtin.command:

cmd: >

docker exec -e MYSQL_PWD="{{ mysql_root_password }}" sword_mysql mysql -uroot

-e "CREATE USER IF NOT EXISTS '{{ db_user }}'@'%' IDENTIFIED BY '{{ db_password }}';"

- name: Grant MySQL privileges

ansible.builtin.command:

cmd: >

docker exec -e MYSQL_PWD="{{ mysql_root_password }}" sword_mysql mysql -uroot

Copilot · 2026-03-22T12:29:29Z

+            $formattedData['servers']['hosts']['server-'.$server->id.'-'.$server->name] = [
                'ansible_host' => $server->ip_address,
                'ansible_user' => 'root', // @todo make this a variable.
                'ansible_port' => $server->ssh_port, // @todo make this a variable.
                'ansible_ssh_private_key_file' => $this->createTempPrivateKeyFile($server),
            ];


Inventory hostnames are constructed using $server->name (and later used again for --limit). Because name is only validated as a free-form string, it may contain spaces/shell metacharacters and can produce an invalid Ansible hostname/pattern. Consider using a safe identifier (e.g., just server-{$server->id}) or a slugified/sanitized name consistently in both the inventory and --limit.

Copilot · 2026-03-22T12:29:29Z

+                'php_version' => $site->php_version,
+                'db_name' => $site->db_name,
+                'db_user' => $site->db_user,
+                'db_password' => $site->db_password,


generateInventory() writes each site's db_password into the inventory file. Since db_password uses Laravel's encrypted cast, this will be decrypted and written in plaintext to disk. Prefer passing secrets only via --extra-vars for the specific run (or use Ansible Vault), and avoid persisting per-site credentials in a shared inventory file.

Suggested change

'db_password' => $site->db_password,

Copilot · 2026-03-22T12:29:29Z

+        return $this->runPlaybook( $playbookpath, $extraVars);
+
+    }
+
+    public function runPlaybook( string $playbookpath, array $extraVars = [], string $LimitServer = 'all')
+    {


runSitePlaybook() calls runPlaybook() without a limit, so it will run with the default --limit all. With the current inventory generation this will attempt to connect to every host (including the synthetic site-* hosts that don't have connection variables), which is likely to fail and/or run against unintended hosts. Limit the run to the site's server host (e.g., the corresponding server-* inventory host) or change the playbook to target only the servers group and remove site-* hosts from inventory.

Copilot · 2026-03-22T12:29:29Z

    /**
     * Run a specific playbook.
     *
-     * @param  mixed  $server  The Server model, server ID, or null to run for all servers.
+     * @param  int|Server  $server  The Server model, server ID, or null to run for all servers.
     * @param  string|null  $playbookpath  Path to a playbook, relative to ./ansible-playbooks.
     *                                     Or full paths. Default to main.yml
     * @param  array<string, mixed>  $extraVars  Extra variables to pass to the playbook via --extra-vars.
     */
-    public function runPlaybook($server = null, ?string $playbookpath = null, array $extraVars = [])
+    public function runServerPlaybook( int|Server $server, ?string $playbookpath = null, array $extraVars = [])
    {
-        // Always make sure we have a fresh inventory.
-        $this->generateInventory();

        if (is_int($server)) {
            $server = Server::findOrFail($server);
        }
-        $LimitServer = 'all';
        if ($server) {
            $LimitServer = 'server-'.$server->id.'-'.$server->name;
        }


Docblock says $server can be null to run for all servers, but the signature is int|Server $server (non-nullable) and the implementation assumes a server is always present. Either update the docblock and remove the dead null-handling, or make the parameter nullable and initialize $LimitServer safely (it is currently undefined if null is allowed again).

Copilot · 2026-03-22T12:29:30Z

+            4,
+            Yaml::DUMP_MULTI_LINE_LITERAL_BLOCK
+        );
        file_put_contents('/tmp/ansible-sword-inventory.yml', $yaml);


The generated inventory is written to /tmp/ansible-sword-inventory.yml without restricting permissions, but it contains sensitive data (e.g., database passwords and SSH key file paths). Set the file mode to 0600 (and consider writing to a per-run unique path) to reduce the risk of other local users reading it.

Suggested change

file_put_contents('/tmp/ansible-sword-inventory.yml', $yaml);

$inventoryPath = '/tmp/ansible-sword-inventory.yml';

file_put_contents($inventoryPath, $yaml);

chmod($inventoryPath, 0600);

Copilot · 2026-03-22T12:29:30Z

+                'mysql_root_password' => $server->mysql_root_password,
+                // @todo these should be stored in the Job, and not generated here.
+                'wp_admin_user' => $site->user->name ?? 'admin',
+                'wp_admin_password' => Str::random(16), // @todo Get from the job.


runSitePlaybook() generates a new random wp_admin_password each run, which won't match the credentials provided when dispatching InstallSiteJob (and won't be persisted anywhere for the user to retrieve). Use the job-provided credentials (or persist the generated password to the Site/job and surface it to the user) so the installed site can be logged into predictably.

Suggested change

'wp_admin_password' => Str::random(16), // @todo Get from the job.

'wp_admin_password' => $site->wp_admin_password ?? Str::random(16),

Copilot · 2026-03-22T12:29:30Z

-        $yaml = Yaml::dump($formattedData, 2);
+        $yaml = Yaml::dump(
+            $formattedData,
+            9001, //It's over 9000.


The inline level 9001 (and the joke comment) makes inventory formatting non-obvious and hard to maintain. Unless there's a concrete YAML/Ansible parsing issue being worked around, keep Yaml::dump() parameters to documented defaults or add a clear explanation for why a nonstandard inline level is required.

Suggested change

9001, //It's over 9000.

2,

Copilot · 2026-03-22T12:29:31Z

        $ssh->exec(sprintf('wget -qO create-wp-site.sh "%s" && nohup bash create-wp-site.sh > create-wp-site.log 2>&1 < /dev/null & disown', $installUrl));
+
+        if (! config('services.ansible.enabled')) {
+            logger()->warning('Ansible execution is disabled. Skipping RunAnsible job for server ID: '.$this->serverID);


This log message references $this->serverID, but InstallSiteJob doesn't have a serverID property. When Ansible is disabled this will throw an error and fail the job. Log the site ID / server ID derived from $site instead (e.g., $site->server_id).

Suggested change

logger()->warning('Ansible execution is disabled. Skipping RunAnsible job for server ID: '.$this->serverID);

logger()->warning('Ansible execution is disabled. Skipping RunAnsible job for site ID: '.$site->id.' (server ID: '.$site->server_id.')');

Copilot · 2026-03-22T12:29:31Z

@@ -0,0 +1,476 @@
+---
+- name: SWORD WordPress Site Installation
+  hosts: all


The play targets hosts: all, which makes it easy to accidentally run against every host in the inventory if the caller forgets to set --limit correctly. Since this playbook is intended to install a site on a single server, consider targeting a dedicated group (e.g. hosts: servers) and relying on --limit to pick the specific server.

Suggested change

hosts: all

hosts: servers

janw-me added 2 commits March 22, 2026 13:21

Add basic ansible create site functionality.

5e8be72

Trigger site creation script.

8ef1417

janw-me requested a review from Copilot March 22, 2026 12:25

Copilot started reviewing on behalf of janw-me March 22, 2026 12:25 View session

Copilot AI reviewed Mar 22, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/ansible wp#53

Feature/ansible wp#53
janw-me wants to merge 2 commits intomainfrom
feature/ansible-wp

janw-me commented Mar 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Copilot AI Mar 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-        file_put_contents('/tmp/ansible-sword-inventory.yml', $yaml);
+        $inventoryPath = '/tmp/ansible-sword-inventory.yml';
+        file_put_contents($inventoryPath, $yaml);
+        chmod($inventoryPath, 0600);

	'wp_admin_password' => Str::random(16), // @todo Get from the job.
	'wp_admin_password' => $site->wp_admin_password ?? Str::random(16),

	logger()->warning('Ansible execution is disabled. Skipping RunAnsible job for server ID: '.$this->serverID);
	logger()->warning('Ansible execution is disabled. Skipping RunAnsible job for site ID: '.$site->id.' (server ID: '.$site->server_id.')');

Conversation

janw-me commented Mar 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants