Repository navigation
fix(git): read git.json with a byte order mark everywhere - #182
Open
grantemsley wants to merge 6 commits into
Open
grantemsley wants to merge 6 commits into
grantemsley wants to merge 6 commits into
Conversation
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 <noreply@anthropic.com>
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 mstrhakr#172. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s 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 mstrhakr#172. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… 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 "<stack> 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…r is missing When the projects folder is on the array and the array is stopped, the folder is not there: the in-use scan finds no stacks, every credential looks unused, and deleteCredential deletes one a stack still needs. deleteCredential now refuses while the projects folder is missing and says to start the array. Found by the review of the previous commit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Copilot's second review of #172 found that two places read
git.jsonwith a plainjson_decode, whileGitStackSettings::load()strips a leading byte order mark (a Windows editor adds one). A stack whosegit.jsonstarts with one loads and deploys fine, but these readers see no settings:
deleted from under it.
repository did not get its key.
While checking these, I found two more ways to delete a credential a stack still needs:
git.jsonthat isn't JSON at all (a trailing comma from a hand edit) counted as using no credential. A gitcredential that such a file may name is now kept, and the Credentials tab says which stack to fix. It used to
reload the list without a word when a delete was refused.
unused. The delete is now refused until the array is started.
What it adds:
GitStackSettings::readAsPlainJson(), which reads the file asload()does but without checkingthe settings, and the three readers moved onto it.
Reading it: 6 commits: the helper with its tests, one commit per reader, each with a regression test, then the
two credential cases.
Testing: the full suite (PHPUnit, PHPStan, ShellCheck, Bats as non-root) passes at every commit. On Unraid 7.3.2,
a cron rebuild kept the scheduled check for a
git.jsonwith a byte order mark (it dropped it before), and theCredentials tab showed a marked file's credential as in use, refused the delete for a broken file naming the stack,
and deleted it once the stack was gone.