Skip to content
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,40 @@ public void GnuPassCredentialStore_Remove_NotFound_ReturnsFalse()
Assert.False(result);
}

[PosixFact]
public void GnuPassCredentialStore_ReadWriteDelete_GpgIdInSubdirectory()
{
var fs = new TestFileSystem();
var gpg = new TestGpg(fs);
string storeRoot = InitializePasswordStoreWithGpgIdInSubdirectory(fs, gpg, TestNamespace);

var collection = new GpgPassCredentialStore(fs, gpg, storeRoot, TestNamespace);

// Create a service that is guaranteed to be unique
string uniqueGuid = Guid.NewGuid().ToString("N");
string service = $"https://example.com/{uniqueGuid}";
const string userName = "john.doe";
const string password = "letmein123"; // [SuppressMessage("Microsoft.Security", "CS001:SecretInline", Justification="Fake credential")]

try
{
// Write
collection.AddOrUpdate(service, userName, password);

// Read
ICredential outCredential = collection.Get(service, userName);

Assert.NotNull(outCredential);
Assert.Equal(userName, outCredential.Account);
Assert.Equal(password, outCredential.Password);
}
finally
{
// Ensure we clean up after ourselves even in case of 'get' failures
collection.Remove(service, userName);
}
}

private static string InitializePasswordStore(TestFileSystem fs, TestGpg gpg)
{
string homePath = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile);
Expand All @@ -102,5 +136,27 @@ private static string InitializePasswordStore(TestFileSystem fs, TestGpg gpg)

return storePath;
}

private static string InitializePasswordStoreWithGpgIdInSubdirectory(TestFileSystem fs, TestGpg gpg, string subdirectory)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

{
string homePath = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile);
string storePath = Path.Combine(homePath, ".password-store");
string userId = "gcm-test@example.com";

// Place .gpg-id only in the namespace subdirectory (not the store root),
// simulating a pass store where the root has no .gpg-id but submodules do.
string subDirPath = Path.Combine(storePath, subdirectory);
string gpgIdPath = Path.Combine(subDirPath, ".gpg-id");

// Ensure we have a GPG key for use with testing
gpg.GenerateKeys(userId);

// Init the password store with .gpg-id only in the subdirectory
fs.Directories.Add(storePath);
fs.Directories.Add(subDirPath);
fs.Files[gpgIdPath] = Encoding.UTF8.GetBytes(userId);

return storePath;
}
}
}
12 changes: 0 additions & 12 deletions src/shared/Core/CredentialStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -291,18 +291,6 @@ private void ValidateGpgPass(out string storeRoot, out string execPath)
storeRoot = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.UserProfile), ".password-store");
}

// Check we have a GPG ID to sign credential files with
string gpgIdFile = Path.Combine(storeRoot, ".gpg-id");
if (!_context.FileSystem.FileExists(gpgIdFile))
{
var format =
"Password store has not been initialized at '{0}'; run `pass init <gpg-id>` to initialize the store.";
var message = string.Format(format, storeRoot);
_context.Trace2.WriteError(message);
throw new Exception(message + Environment.NewLine +
$"See {Constants.HelpUrls.GcmCredentialStores} for more information."
);
}
}

private void ValidateCredentialCache(out string options)
Expand Down
18 changes: 10 additions & 8 deletions src/shared/Core/Interop/Posix/GpgPassCredentialStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,19 @@ public GpgPassCredentialStore(IFileSystem fileSystem, IGpg gpg, string storeRoot

private string GetGpgId()
{
string gpgIdPath = Path.Combine(StoreRoot, ".gpg-id");
if (!FileSystem.FileExists(gpgIdPath))
// 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"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

{
throw new Exception($"Cannot find GPG ID in '{gpgIdPath}'; password store has not been initialized");
using (var stream = FileSystem.OpenFileStream(gpgIdPath, FileMode.Open, FileAccess.Read, FileShare.Read))
using (var reader = new StreamReader(stream))
{
return reader.ReadLine();
}
}

using (var stream = FileSystem.OpenFileStream(gpgIdPath, FileMode.Open, FileAccess.Read, FileShare.Read))
using (var reader = new StreamReader(stream))
{
return reader.ReadLine();
}
throw new Exception($"Cannot find GPG ID in password store at '{StoreRoot}'; run `pass init <gpg-id>` to initialize the store.");
}

protected override bool TryDeserializeCredential(string path, out FileCredential credential)
Expand Down
Loading