Fix GPG ID lookup and remove hardcoded test credentials#2274
Fix GPG ID lookup and remove hardcoded test credentials#2274
Conversation
…2263) Co-authored-by: marekzmyslowski <1062877+marekzmyslowski@users.noreply.github.com>
…teFiles Co-authored-by: marekzmyslowski <1062877+marekzmyslowski@users.noreply.github.com>
Co-authored-by: marekzmyslowski <1062877+marekzmyslowski@users.noreply.github.com>
… in GetGpgId Co-authored-by: marekzmyslowski <1062877+marekzmyslowski@users.noreply.github.com>
| // Search for a .gpg-id file anywhere under the store root. | ||
| // This handles configurations where .gpg-id is in a subdirectory | ||
| // (e.g., a git submodule) rather than the store root itself. | ||
| foreach (string gpgIdPath in FileSystem.EnumerateFiles(StoreRoot, ".gpg-id")) |
There was a problem hiding this comment.
This is incorrect. This is going to do a top-down walk from the StoreRoot looking for any .gpg-id file.
We should instead be searching UP from the full credential path looking for the closest .gpg-id. Otherwise doing the top-down walk could result in us picking an incorrect GPG identity for the given credential.
We must align the behaviour with GNU Pass.
…behaviour Co-authored-by: marekzmyslowski <1062877+marekzmyslowski@users.noreply.github.com>
…asswords Co-authored-by: marekzmyslowski <1062877+marekzmyslowski@users.noreply.github.com>
mjcheetham
left a comment
mjcheetham
left a comment
There was a problem hiding this comment.
Last request to simplify one of the tests, but this is a good direction now!
| return storePath; | ||
| } | ||
|
|
||
| private static string InitializePasswordStoreWithGpgIdInSubdirectory(TestFileSystem fs, TestGpg gpg, string subdirectory) |
There was a problem hiding this comment.
This method is now only used in one test. Can we inline this in the GnuPassCredentialStore_ReadWriteDelete_GpgIdInSubdirectory test, so that it matches the other test (GnuPassCredentialStore_WriteCredential_MultipleGpgIds_UsesNearestGpgId) that does all the store/.gpg-id setup in the test method?
There was a problem hiding this comment.
Done in b641577. The InitializePasswordStoreWithGpgIdInSubdirectory helper has been removed and its logic inlined directly into GnuPassCredentialStore_ReadWriteDelete_GpgIdInSubdirectory, matching the style of the MultipleGpgIds test.
Co-authored-by: mjcheetham <5658207+mjcheetham@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
GetGpgIdwalk-up logic (previous session)"letmein123"hardcoded fake password withGuid.NewGuid().ToString("N")in new testsInitializePasswordStoreWithGpgIdInSubdirectoryhelper directly intoGnuPassCredentialStore_ReadWriteDelete_GpgIdInSubdirectorytest (per reviewer request)✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.