fix: escape credential.helper path so git-credential-nostr survives spaces - #3435
Open
1nderboi wants to merge 1 commit into
Open
fix: escape credential.helper path so git-credential-nostr survives spaces#34351nderboi wants to merge 1 commit into
1nderboi wants to merge 1 commit into
Conversation
…paces Git builds the credential helper invocation by concatenating the raw credential.helper config value with the operation (get/store/erase) and running the result through `sh -c`. An unescaped absolute path containing a space gets word-split by that shell, so git-credential-nostr never runs when installed under a path like "/Users/x/My Apps/git-credential-nostr". Naively wrapping the whole value in shell quotes doesn't work either: git detects an absolute-path helper by checking whether the *unescaped* string starts with a path separator (or drive letter on Windows), so a leading quote character defeats that check and git falls back to treating the value as a bare helper name. Fix: backslash-escape each shell-special character individually, leaving the leading separator intact. Added a shared git_credential_helper_config_value helper in managed_agents/discovery.rs and used it from both call sites that build this config value (project_git_exec.rs's credential_helper_config_value and runtime.rs's GIT_CONFIG_VALUE_0 write), plus unit tests for plain/space/apostrophe paths. Fixes block#3298 Signed-off-by: 1nderboi <201919958+1nderboi@users.noreply.github.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
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.
Summary
git-credential-nostrfailed to run whenever its resolved absolute path contained a space (e.g./Users/x/My Apps/git-credential-nostr) — git concatenates the rawcredential.helperconfig value with the operation (get/store/erase) and runs the result throughsh -c, so an unescaped path gets word-split by that shell.git credential-'...'), which is worse.sh -cword-split. Added one shared helper (git_credential_helper_config_valueinmanaged_agents/discovery.rs) and used it at both call sites that build this config value:commands/project_git_exec.rs::credential_helper_config_valuemanaged_agents/runtime.rs'sGIT_CONFIG_VALUE_0writeVerified the underlying escaping approach against a real
git credential fillinvocation (plain path, path with a space, path with an apostrophe) before writing the Rust, since the shared build machine hit disk-space limits mid-build (see test plan).Fixes #3298
Test plan
commands/project_git_exec.rs(credential_helper_config_value_uses_forward_slashes,credential_helper_config_value_escapes_spaces,credential_helper_config_value_escapes_apostrophes)rustfmt --checkclean on all three changed filesgit credential fill(outside the Rust build, due to shared-machine disk pressure — see below)cargo test -p desktop --lib— could not run to completion locally; the shared build machine repeatedly hitNo space left on devicewhile three agents ran concurrent full-workspace builds. Please confirm green in CI.