fix(git): read git.json with a byte order mark everywhere

source/compose.manager/include/CronManager.php

@@ -9,6 +9,7 @@
 
 require_once("/usr/local/emhttp/plugins/compose.manager/include/Defines.php");
 require_once("/usr/local/emhttp/plugins/compose.manager/include/Util.php");
+require_once("/usr/local/emhttp/plugins/compose.manager/include/GitStackSettings.php");
 
 /**
  * Factory function to create a CronManager instance.
@@ -240,7 +241,7 @@ private function addGitCheckLineIfWanted(array &$lines): void
         }
         foreach (glob(rtrim($this->composeRoot, '/') . '/*/git.json') ?: [] as $gitSettingsFile) {
             // Read as plain JSON: the scheduled check itself validates each stack's settings.
-            $settings = json_decode((string) @file_get_contents($gitSettingsFile), true);
+            $settings = GitStackSettings::readAsPlainJson($gitSettingsFile);
             if (is_array($settings) && is_string($settings['autoDeploy'] ?? null) && $settings['autoDeploy'] !== 'off') {
                 $command = $this->pluginRoot . 'scripts/compose-git';
                 // Every 15 minutes, 7 minutes off the image auto-update's quarter hours, so a git

source/compose.manager/include/Exec.php

@@ -201,7 +201,7 @@ function composeBuildCredentialStackMap(string $composeRoot): array
         }
         foreach (glob(rtrim($composeRoot, '/') . '/*/git.json') ?: [] as $gitSettingsFile) {
             // Read as plain JSON: a git.json that fails validation still keeps its credential in use.
-            $gitSettings = json_decode((string) @file_get_contents($gitSettingsFile), true);
+            $gitSettings = GitStackSettings::readAsPlainJson($gitSettingsFile);
             $credentialId = is_array($gitSettings) ? ($gitSettings['credentialId'] ?? null) : null;
             if (!is_string($credentialId) || $credentialId === '') {
                 continue;
@@ -215,6 +215,32 @@ function composeBuildCredentialStackMap(string $composeRoot): array
     }
 }
 
+if (!function_exists('composeListUnreadableGitStacksThatMayUse')) {
+    /**
+     * The git stacks whose git.json cannot be read as JSON (a typo from a hand edit, say) but may
+     * still name this credential. composeBuildCredentialStackMap() cannot see them.
+     *
+     * Such a file may name it when its text contains the credential id, when the file cannot be
+     * read at all, or when it holds NUL bytes (saved as UTF-16, so the id cannot be searched for).
+     *
+     * @return string[] Stack folder names
+     */
+    function composeListUnreadableGitStacksThatMayUse(string $composeRoot, string $credentialId): array
+    {
+        $stacks = [];
+        foreach (glob(rtrim($composeRoot, '/') . '/*/git.json') ?: [] as $gitSettingsFile) {
+            if (GitStackSettings::readAsPlainJson($gitSettingsFile) !== null) {
+                continue;
+            }
+            $content = @file_get_contents($gitSettingsFile);
+            if ($content === false || str_contains($content, "\0") || str_contains($content, $credentialId)) {
+                $stacks[] = basename(dirname($gitSettingsFile));
+            }
+        }
+        return $stacks;
+    }
+}
+
 if (!function_exists('composeResolveContainerIcon')) {
     /**
      * Resolve container icon using stack cache first, then docker inspect fallback.
@@ -1074,13 +1100,47 @@ function composeResolveContainerIcon(string $containerName, string $service, arr
     case 'deleteCredential':
         $credentialId = trim((string) ($_POST['id'] ?? ''));
         try {
-            $deleted = CredentialVault::withCredentialAssignmentLock(function () use ($compose_root, $credentialId): bool {
+            $refusal = null;
+            $deleted = CredentialVault::withCredentialAssignmentLock(function () use ($compose_root, $credentialId, &$refusal): bool {
+                // A projects folder on the array is gone while the array is stopped: no stack would be
+                // found, and every credential would look unused.
+                if (!is_dir($compose_root)) {
+                    $refusal = "The projects folder ($compose_root) is not there, so the stacks that use this credential "
+                        . 'cannot be checked. If it is on the array, start the array first.';
+                    return false;
+                }
                 $stacks = composeBuildCredentialStackMap($compose_root)[$credentialId] ?? [];
                 if (!empty($stacks)) {
                     throw new RuntimeException('Credential is assigned to: ' . implode(', ', $stacks));
                 }
+                // A git stack whose git.json cannot be read may use this credential: do not delete
+                // it from under that stack. Registry credentials are never in a git.json.
+                // Looked up in the list, not with getCredentialSummary(), so a missing id is still "not found".
+                $provider = '';
+                foreach ((new CredentialVault())->listCredentials() as $credential) {
+                    if (($credential['id'] ?? '') === $credentialId) {
+                        $provider = $credential['provider'] ?? '';
+                    }
+                }
+                $unreadable = in_array($provider, ['git', 'git-ssh'], true)
+                    ? composeListUnreadableGitStacksThatMayUse($compose_root, $credentialId)
+                    : [];
+                if (count($unreadable) === 1) {
+                    $refusal = "The git settings (git.json) of {$unreadable[0]} could not be read, so it may use this credential. "
+                        . "Fix the file first: the stack's Sources tab, or compose-git status, shows the error.";
+                    return false;
+                }
+                if (count($unreadable) > 1) {
+                    $refusal = 'The git settings (git.json) of ' . implode(', ', $unreadable) . ' could not be read, so they may use '
+                        . "this credential. Fix the files first: each stack's Sources tab, or compose-git status, shows the error.";
+                    return false;
+                }
                 return (new CredentialVault())->deleteCredential($credentialId);
             });
+            if ($refusal !== null) {
+                echo json_encode(['result' => 'error', 'message' => $refusal]);
+                break;
+            }
             if ($deleted) {
                 composeLogger('Deleted credential', ['id' => $credentialId], 'user', 'info', 'credential');
             }

source/compose.manager/include/GitStackManager.php

@@ -927,8 +927,7 @@ private function gitStackUrl(string $folder): ?string
      */
     private function readGitJson(string $folder): ?array
     {
-        $data = json_decode((string) @file_get_contents($this->composeRoot . '/' . $folder . '/' . GitStackSettings::FILE_NAME), true);
-        return is_array($data) ? $data : null;
+        return GitStackSettings::readAsPlainJson($this->composeRoot . '/' . $folder . '/' . GitStackSettings::FILE_NAME);
     }
 
     /**

source/compose.manager/include/GitStackSettings.php

@@ -181,6 +181,29 @@ public function isSsh(): bool
         return self::isSshUrl($this->url);
     }
 
+    /**
+     * Read a git.json as plain JSON, without checking its settings.
+     *
+     * For code that only needs one value and must not depend on the file being
+     * valid, such as finding which credentials are in use. Like load(), it
+     * accepts a file that starts with a byte order mark.
+     *
+     * @return array<mixed>|null null when the file cannot be read or is not a JSON object or list
+     */
+    public static function readAsPlainJson(string $file): ?array
+    {
+        $content = @file_get_contents($file);
+        if ($content === false) {
+            return null;
+        }
+        // A file saved by a Windows editor may start with a byte order mark.
+        if (str_starts_with($content, "\xEF\xBB\xBF")) {
+            $content = substr($content, 3);
+        }
+        $data = json_decode($content, true);
+        return is_array($data) ? $data : null;
+    }
+
     /**
      * Load a stack's git settings.
      *

source/compose.manager/javascript/credentialManager.js

@@ -548,12 +548,27 @@
         }
         swal({ title: 'Delete credential?', text: credential.name, type: 'warning', showCancelButton: true }, function(confirmed) {
             if (!confirmed) return;
-            $.post(window.caURL || '/plugins/compose.manager/include/Exec.php', { action: 'deleteCredential', id: credential.id }).done(function() {
+            $.post(window.caURL || '/plugins/compose.manager/include/Exec.php', { action: 'deleteCredential', id: credential.id }).done(function(data) {
+                var response;
+                try { response = typeof data === 'string' ? JSON.parse(data) : data; } catch (error) { response = {}; }
+                if (response.result !== 'success') {
+                    showDeleteError(response.message || 'Unknown error.');
+                }
+                loadCredentials();
+            }).fail(function() {
+                showDeleteError('Unable to reach credential service.');
                 loadCredentials();
             });
         });
     }
 
+    function showDeleteError(message) {
+        // Wait for the confirm dialog to finish closing, or SweetAlert can hide this one with it.
+        setTimeout(function() {
+            swal({ title: 'Unable to delete credential', text: message, type: 'error' });
+        }, 300);
+    }
+
     function populateSelect($select, selectedId) {
         $select.empty().append($('<option value="">').text('Anonymous / no credential'));
         credentials.forEach(function(credential) {

tests/unit/CronManagerTest.php

@@ -314,6 +314,21 @@ public function testGitCheckLineIsAddedWhileAGitStackAsksForIt(): void
         }
     }
 
+    public function testGitCheckLineIsAddedForAGitJsonWithAByteOrderMark(): void
+    {
+        // As saved by a Windows editor. GitStackSettings::load() accepts it, so the cron must too.
+        $root = $this->projectsWithGitStack('deploy');
+        $file = $root . '/whoami/git.json';
+        file_put_contents($file, "\xEF\xBB\xBF" . file_get_contents($file));
+        try {
+            $cron = new CronManager($this->cronFile, '/usr/local/emhttp/plugins/compose.manager/', false, $root);
+            $cron->rebuild();
+            $this->assertTrue($cron->isGitCheckInstalled());
+        } finally {
+            $this->removeProjects($root);
+        }
+    }
+
     public function testNoGitCheckLineWhenNoGitStackAsksForIt(): void
     {
         foreach (['off', null] as $autoDeploy) {

tests/unit/ExecActionsTest.php

@@ -858,6 +858,123 @@ public function testCredentialUsedByAGitStackCannotBeDeleted(): void
         $this->assertSame(['git-stack'], $deleteResult['stacks']);
     }
 
+    public function testCredentialUsedByAGitStackWhoseGitJsonHasAByteOrderMarkCannotBeDeleted(): void
+    {
+        // As saved by a Windows editor. GitStackSettings::load() accepts it, so the stack keeps working
+        // and its credential must not be deleted from under it.
+        $stackPath = $this->createTestStack('git-stack');
+        $saved = json_decode($this->executeAction('saveCredential', [
+            'name' => 'Git stacks', 'provider' => 'git', 'registry' => 'github.com',
+            'username' => 'bot', 'secret' => 'repository-token',
+        ]), true);
+        $credentialId = $saved['credential']['id'];
+        file_put_contents($stackPath . '/git.json', "\xEF\xBB\xBF" . json_encode(['credentialId' => $credentialId]));
+
+        $deleteResult = json_decode($this->executeAction('deleteCredential', ['id' => $credentialId]), true);
+        $this->assertSame('error', $deleteResult['result']);
+        $this->assertSame(['git-stack'], $deleteResult['stacks']);
+    }
+
+    public function testGitCredentialIsNotDeletedWhileAGitStacksSettingsCannotBeRead(): void
+    {
+        // A hand edit left a trailing comma: the stack may use this credential, but which one cannot be read.
+        $stackPath = $this->createTestStack('git-stack');
+        $saved = json_decode($this->executeAction('saveCredential', [
+            'name' => 'Git stacks', 'provider' => 'git', 'registry' => 'github.com',
+            'username' => 'bot', 'secret' => 'repository-token',
+        ]), true);
+        $credentialId = $saved['credential']['id'];
+        file_put_contents($stackPath . '/git.json', '{"credentialId": "' . $credentialId . '",}');
+
+        $deleteResult = json_decode($this->executeAction('deleteCredential', ['id' => $credentialId]), true);
+        $this->assertSame('error', $deleteResult['result']);
+        $this->assertStringContainsString('git-stack', $deleteResult['message']);
+        $this->assertStringContainsString('could not be read', $deleteResult['message']);
+        $this->assertCount(1, (new \CredentialVault())->listCredentials());
+
+        // Once the file is fixed, the stack counts as using it in the usual way.
+        file_put_contents($stackPath . '/git.json', '{"credentialId": "' . $credentialId . '"}');
+        $deleteResult = json_decode($this->executeAction('deleteCredential', ['id' => $credentialId]), true);
+        $this->assertSame(['git-stack'], $deleteResult['stacks']);
+    }
+
+    public function testRegistryCredentialCanBeDeletedWhileAGitStacksSettingsCannotBeRead(): void
+    {
+        // Only git credentials are named in a git.json, so a broken one does not hold up a registry credential,
+        // even one whose id it happens to contain.
+        $stackPath = $this->createTestStack('git-stack');
+        $saved = json_decode($this->executeAction('saveCredential', [
+            'name' => 'Work GitHub', 'provider' => 'github', 'registry' => 'ghcr.io',
+            'username' => 'octocat', 'secret' => 'read-only-token',
+        ]), true);
+        file_put_contents($stackPath . '/git.json', '{"credentialId": "' . $saved['credential']['id'] . '",}');
+
+        $deleteResult = json_decode($this->executeAction('deleteCredential', ['id' => $saved['credential']['id']]), true);
+        $this->assertSame('success', $deleteResult['result']);
+    }
+
+    public function testGitCredentialCanBeDeletedWhenNoGitStackNamesIt(): void
+    {
+        // A broken git.json that names another credential, and a readable one that names none, do not hold it up.
+        $brokenPath = $this->createTestStack('broken-stack');
+        $readablePath = $this->createTestStack('readable-stack');
+        $saved = json_decode($this->executeAction('saveCredential', [
+            'name' => 'Git stacks', 'provider' => 'git', 'registry' => 'github.com',
+            'username' => 'bot', 'secret' => 'repository-token',
+        ]), true);
+        file_put_contents($brokenPath . '/git.json', '{"credentialId": "' . str_repeat('cd', 16) . '",}');
+        file_put_contents($readablePath . '/git.json', '{"url": "https://github.com/owner/repo.git"}');
+
+        $deleteResult = json_decode($this->executeAction('deleteCredential', ['id' => $saved['credential']['id']]), true);
+        $this->assertSame('success', $deleteResult['result']);
+    }
+
+    public function testDeployKeyIsNotDeletedWhileTwoGitStacksSettingsCannotBeRead(): void
+    {
+        // One names the key; the other was saved as UTF-16, so its text cannot be searched and it may name it too.
+        $firstPath = $this->createTestStack('first-stack');
+        $secondPath = $this->createTestStack('second-stack');
+        $key = (new \CredentialVault())->saveCredential([
+            'name' => 'first-stack deploy key', 'provider' => 'git-ssh', 'registry' => 'github.com',
+            'username' => 'git', 'secret' => 'private key',
+        ]);
+        file_put_contents($firstPath . '/git.json', '{"credentialId": "' . $key['id'] . '",}');
+        $utf16 = '';
+        foreach (str_split('{"credentialId": "' . $key['id'] . '"}') as $character) {
+            $utf16 .= $character . "\0";
+        }
+        file_put_contents($secondPath . '/git.json', $utf16);
+
+        $deleteResult = json_decode($this->executeAction('deleteCredential', ['id' => $key['id']]), true);
+        $this->assertSame('error', $deleteResult['result']);
+        $this->assertStringContainsString('first-stack, second-stack could not be read, so they may use', $deleteResult['message']);
+        $this->assertCount(1, (new \CredentialVault())->listCredentials());
+    }
+
+    public function testCredentialIsNotDeletedWhileTheProjectsFolderIsMissing(): void
+    {
+        // As with the projects folder on the array while the array is stopped: the stack using it cannot be seen.
+        $stackPath = $this->createTestStack('test-stack');
+        $saved = json_decode($this->executeAction('saveCredential', [
+            'name' => 'Work GitHub', 'provider' => 'github', 'registry' => 'ghcr.io',
+            'username' => 'octocat', 'secret' => 'read-only-token',
+        ]), true);
+        file_put_contents($stackPath . '/credential_id', $saved['credential']['id']);
+        $this->recursiveDelete($this->testComposeRoot);
+
+        $deleteResult = json_decode($this->executeAction('deleteCredential', ['id' => $saved['credential']['id']]), true);
+        $this->assertSame('error', $deleteResult['result']);
+        $this->assertStringContainsString('start the array first', $deleteResult['message']);
+        $this->assertCount(1, (new \CredentialVault())->listCredentials());
+    }
+
+    public function testDeletingAMissingCredentialSaysItIsNotFound(): void
+    {
+        $deleteResult = json_decode($this->executeAction('deleteCredential', ['id' => str_repeat('ab', 16)]), true);
+        $this->assertSame('error', $deleteResult['result']);
+        $this->assertSame('Credential not found.', $deleteResult['message']);
+    }
+
     public function testGitCredentialIsNotTestedAsARegistryAndCannotBeARegistryCredential(): void
     {
         $stackPath = $this->createTestStack('test-stack');

tests/unit/GitStackManagerTest.php

@@ -808,6 +808,32 @@ public function testAStackWhoseSettingsFailACheckStillSharesItsKey(): void
         $this->assertStringContainsString("Using the deploy key of 'first'", $existing['message'] ?? '');
     }
 
+    public function testAStackWhoseGitJsonHasAByteOrderMarkStillSharesItsKey(): void
+    {
+        // As saved by a Windows editor. GitStackSettings::load() accepts it, so the stack is in use.
+        // The key has the old-style name, so only the stack's git.json can lead to it.
+        $key = $this->emptyVault()->saveCredential(['name' => 'first deploy key', 'provider' => 'git-ssh', 'registry' => 'github.com', 'username' => 'git', 'secret' => 'k']);
+        $this->makeSshStack('first', 'git@github.com:owner/repo.git', $key['id']);
+        $file = $this->composeRoot . '/first/git.json';
+        file_put_contents($file, "\xEF\xBB\xBF" . file_get_contents($file));
+
+        $existing = $this->manager->existingDeployKey('second', GitStackSettings::sshAddress('git@github.com:owner/repo.git'));
+
+        $this->assertSame($key['id'], $existing['id'] ?? null);
+        $this->assertStringContainsString("Using the deploy key of 'first'", $existing['message'] ?? '');
+    }
+
+    public function testAKeyUsedByAStackWhoseGitJsonHasAByteOrderMarkIsNotGivenToAnotherRepository(): void
+    {
+        // As in testAKeyNamedAfterTheStackIsNotUsedWhileAnotherStackUsesIt, with the mark on the user's git.json.
+        $key = $this->emptyVault()->saveCredential(['name' => 'myapp deploy key', 'provider' => 'git-ssh', 'registry' => 'github.com', 'username' => 'git', 'secret' => 'k']);
+        $this->makeSshStack('other', 'git@github.com:owner/repo.git', $key['id']);
+        $file = $this->composeRoot . '/other/git.json';
+        file_put_contents($file, "\xEF\xBB\xBF" . file_get_contents($file));
+
+        $this->assertNull($this->manager->existingDeployKey('myapp', GitStackSettings::sshAddress('git@github.com:owner/new.git')));
+    }
+
     public function testAStackWhoseSettingsCannotBeReadIsNamed(): void
     {
         $this->emptyVault();