From 49ab16ad35ff27b6125331224b8d5adc435bcd2e Mon Sep 17 00:00:00 2001 From: Grant Emsley Date: Fri, 16 Oct 2026 16:11:07 -0400 Subject: [PATCH 1/6] feat(git): read a git.json as plain JSON, byte order mark and all Three places read git.json as plain JSON on purpose, so a stack whose settings fail a check still counts: the cron rebuild, the Credentials tab's in-use scan and the deploy key sharing. Each calls json_decode on the file directly, which returns null for a file that starts with a byte order mark, while GitStackSettings::load() strips one. GitStackSettings::readAsPlainJson() reads the file the way load() does (byte order mark removed) without checking the settings. The next commits move the three readers onto it. Co-Authored-By: Claude Opus 5.5 --- .../include/GitStackSettings.php | 23 ++++++++++++++++++ tests/unit/GitStackSettingsTest.php | 24 +++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/source/compose.manager/include/GitStackSettings.php b/source/compose.manager/include/GitStackSettings.php index cf93fb22..83d3afd3 100644 --- a/source/compose.manager/include/GitStackSettings.php +++ b/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|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. * diff --git a/tests/unit/GitStackSettingsTest.php b/tests/unit/GitStackSettingsTest.php index b055e110..5d8ce823 100644 --- a/tests/unit/GitStackSettingsTest.php +++ b/tests/unit/GitStackSettingsTest.php @@ -411,6 +411,30 @@ public function testFileWithWindowsLineEndingsAndByteOrderMarkLoads(): void $this->assertNotNull(GitStackSettings::load($this->stackDir)); } + public function testReadAsPlainJsonGivesTheFileWithoutCheckingIt(): void + { + $file = $this->stackDir . '/git.json'; + file_put_contents($file, '{"credentialId": "abc", "notASetting": true}'); + $this->assertSame(['credentialId' => 'abc', 'notASetting' => true], GitStackSettings::readAsPlainJson($file)); + } + + public function testReadAsPlainJsonAcceptsAByteOrderMark(): void + { + $file = $this->stackDir . '/git.json'; + file_put_contents($file, "\xEF\xBB\xBF" . '{"autoDeploy": "deploy"}'); + $this->assertSame(['autoDeploy' => 'deploy'], GitStackSettings::readAsPlainJson($file)); + } + + public function testReadAsPlainJsonGivesNullForAMissingOrBrokenFile(): void + { + $file = $this->stackDir . '/git.json'; + $this->assertNull(GitStackSettings::readAsPlainJson($file)); + file_put_contents($file, '{not json'); + $this->assertNull(GitStackSettings::readAsPlainJson($file)); + file_put_contents($file, '"just a string"'); + $this->assertNull(GitStackSettings::readAsPlainJson($file)); + } + /** @return array): mixed}> */ public static function brokenFiles(): array { From ca559ac412fa7892fd7b8cd0efe9cceef3c2bc0e Mon Sep 17 00:00:00 2001 From: Grant Emsley Date: Fri, 16 Oct 2026 16:11:35 -0400 Subject: [PATCH 2/6] fix(cron): keep the git check for a git.json with a byte order mark The cron rebuild decoded each git.json directly, so a file that starts with a byte order mark (saved by a Windows editor) read as no settings. If that stack was the only one asking for automatic checks, the rebuild dropped the scheduled git check, although the stack itself loads fine. Read it with GitStackSettings::readAsPlainJson(), which removes the mark. Found by Copilot on #172. Co-Authored-By: Claude Opus 5.5 --- source/compose.manager/include/CronManager.php | 3 ++- tests/unit/CronManagerTest.php | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/source/compose.manager/include/CronManager.php b/source/compose.manager/include/CronManager.php index a0970461..92b13885 100644 --- a/source/compose.manager/include/CronManager.php +++ b/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 diff --git a/tests/unit/CronManagerTest.php b/tests/unit/CronManagerTest.php index 4341a1d0..26f3ab47 100644 --- a/tests/unit/CronManagerTest.php +++ b/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) { From 565baeb719cb3b4157cf3fa9dac6cdac3f7b0727 Mon Sep 17 00:00:00 2001 From: Grant Emsley Date: Fri, 16 Oct 2026 16:11:51 -0400 Subject: [PATCH 3/6] fix(credentials): count a git stack's credential when its git.json has a byte order mark The Credentials tab's in-use scan decoded each git.json directly, so a file that starts with a byte order mark read as no settings. The stack then did not count as using its credential, and deleteCredential could delete a token or deploy key the stack still needs, although the stack itself loads fine. Read it with GitStackSettings::readAsPlainJson(), which removes the mark. Exec.php already loads GitStackSettings through GitStackWebActions. Found by Copilot on #172. Co-Authored-By: Claude Opus 5.5 --- source/compose.manager/include/Exec.php | 2 +- tests/unit/ExecActionsTest.php | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/source/compose.manager/include/Exec.php b/source/compose.manager/include/Exec.php index b5186107..8fbf48d1 100644 --- a/source/compose.manager/include/Exec.php +++ b/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; diff --git a/tests/unit/ExecActionsTest.php b/tests/unit/ExecActionsTest.php index 5cd71280..22d75a45 100644 --- a/tests/unit/ExecActionsTest.php +++ b/tests/unit/ExecActionsTest.php @@ -858,6 +858,23 @@ 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 testGitCredentialIsNotTestedAsARegistryAndCannotBeARegistryCredential(): void { $stackPath = $this->createTestStack('test-stack'); From df1e043ea9586bedf0026b127546a4f0b5c373e7 Mon Sep 17 00:00:00 2001 From: Grant Emsley Date: Fri, 16 Oct 2026 16:12:09 -0400 Subject: [PATCH 4/6] fix(git): see a stack's deploy key when its git.json has a byte order mark Deploy key sharing reads every git.json as plain JSON, to find which stacks of a repository use which key. It decoded the file directly, so a file that starts with a byte order mark could not be read: that stack was passed over, a new stack of the same repository did not get its key, and its key could be taken for unused and given to a new stack of another repository with the old " deploy key" name. Read it with GitStackSettings::readAsPlainJson(), which removes the mark. The same bug as the cron rebuild and Credentials tab fixes, in a third place. Co-Authored-By: Claude Opus 5.5 --- .../include/GitStackManager.php | 3 +-- tests/unit/GitStackManagerTest.php | 26 +++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/source/compose.manager/include/GitStackManager.php b/source/compose.manager/include/GitStackManager.php index 2bf8f3fc..1ee49b43 100644 --- a/source/compose.manager/include/GitStackManager.php +++ b/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); } /** diff --git a/tests/unit/GitStackManagerTest.php b/tests/unit/GitStackManagerTest.php index f4f56eed..e5308517 100644 --- a/tests/unit/GitStackManagerTest.php +++ b/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(); From 0812e1fb0af3b03060c3926d8a746c1b01d1a2d7 Mon Sep 17 00:00:00 2001 From: Grant Emsley Date: Fri, 16 Oct 2026 16:46:53 -0400 Subject: [PATCH 5/6] fix(credentials): keep a git credential that a broken git.json may name The Credentials tab's in-use scan reads git.json as plain JSON, so a stack whose settings fail a check still counts. A file that is not JSON at all (a trailing comma from a hand edit) counted as using no credential, so its token or deploy key could be deleted. Once the typo was fixed, the stack named a credential that no longer existed. deleteCredential now refuses to delete a git credential while a git.json that cannot be read as JSON may name it: its text contains the credential's id, it cannot be read at all, or it holds NUL bytes (saved as UTF-16). The message names those stacks and where to see the error. Registry credentials are never in a git.json, so they are not held up, and neither is a git credential no broken file names. The Credentials tab now shows the message when a delete is refused or the request fails; it used to reload the list without a word. The message waits for the confirm dialog to close, as other dialogs in the plugin do. Co-Authored-By: Claude Opus 5.5 --- source/compose.manager/include/Exec.php | 55 +++++++++++- .../javascript/credentialManager.js | 17 +++- tests/unit/ExecActionsTest.php | 83 +++++++++++++++++++ 3 files changed, 153 insertions(+), 2 deletions(-) diff --git a/source/compose.manager/include/Exec.php b/source/compose.manager/include/Exec.php index 8fbf48d1..94c8517c 100644 --- a/source/compose.manager/include/Exec.php +++ b/source/compose.manager/include/Exec.php @@ -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,40 @@ 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 { $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'); } diff --git a/source/compose.manager/javascript/credentialManager.js b/source/compose.manager/javascript/credentialManager.js index 4c30d13e..3e51a617 100644 --- a/source/compose.manager/javascript/credentialManager.js +++ b/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($('