Extract AtRestSecretProtector as the single at-rest secret contract

src/Squid.Core/Services/Deployments/Account/DeploymentAccountDataProvider.cs

@@ -30,7 +30,7 @@ public interface IDeploymentAccountDataProvider : IScopedDependency
 /// upgrade naturally on the next save. Reads use the repository's AsNoTracking query so
 /// decrypting the in-memory entity can never be flushed back to the DB as plaintext.</para>
 /// </summary>
-public class DeploymentAccountDataProvider(IRepository repository, IUnitOfWork unitOfWork, IVariableEncryptionService encryption) : IDeploymentAccountDataProvider
+public class DeploymentAccountDataProvider(IRepository repository, IUnitOfWork unitOfWork, IAtRestSecretProtector protector) : IDeploymentAccountDataProvider
 {
     // The V2 envelope derives its key from a random per-payload salt, so the legacy
     // KDF-scope argument (a V1-only deterministic-salt input) is irrelevant for accounts,
@@ -110,22 +110,20 @@ public async Task<List<DeploymentAccount>> GetAccountsBySpaceIdAsync(int spaceId
         return (count, await DecryptCredentialsAsync(accounts).ConfigureAwait(false));
     }
 
-    // Encrypt only if not already an envelope — idempotent, so a re-save (or a value that
-    // somehow arrives pre-encrypted) never double-wraps.
+    // Protect is idempotent (null/empty/already-encrypted passes through), so a re-save never double-wraps.
     private void EncryptCredentials(DeploymentAccount account)
     {
-        if (account == null || string.IsNullOrEmpty(account.Credentials)) return;
-        if (encryption.IsValidEncryptedValue(account.Credentials)) return;
+        if (account == null) return;
 
-        account.Credentials = encryption.EncryptAsync(account.Credentials, CredentialsKdfScope);
+        account.Credentials = protector.Protect(account.Credentials, CredentialsKdfScope);
     }
 
     private async Task<DeploymentAccount> DecryptCredentialsAsync(DeploymentAccount account)
     {
         if (account == null || string.IsNullOrEmpty(account.Credentials)) return account;
 
-        // Read-both: a plaintext (unprefixed) value is returned verbatim by DecryptAsync.
-        account.Credentials = await encryption.DecryptAsync(account.Credentials, CredentialsKdfScope).ConfigureAwait(false);
+        // Read-both: a plaintext (unprefixed) value is returned verbatim by Unprotect.
+        account.Credentials = await protector.UnprotectAsync(account.Credentials, CredentialsKdfScope).ConfigureAwait(false);
 
         return account;
     }

src/Squid.Core/Services/Deployments/Certificates/CertificateDataProvider.cs

@@ -38,7 +38,7 @@ public interface ICertificateDataProvider : IScopedDependency
 /// create-then-update/delete (as the CRUD tests exercise) does not hit a duplicate-tracking
 /// conflict.</para>
 /// </summary>
-public class CertificateDataProvider(IUnitOfWork unitOfWork, IRepository repository, IVariableEncryptionService encryption) : ICertificateDataProvider
+public class CertificateDataProvider(IUnitOfWork unitOfWork, IRepository repository, IAtRestSecretProtector protector) : ICertificateDataProvider
 {
     // The V2 envelope derives its key from a random per-payload salt, so the legacy KDF-scope
     // argument (a V1-only deterministic-salt input) is irrelevant for certificates (write V2 only).
@@ -102,21 +102,14 @@ public async Task DeleteCertificatesAsync(List<Persistence.Entities.Deployments.
         return await DecryptSecretsAsync(certificate).ConfigureAwait(false);
     }
 
-    // Encrypt the password + the cert blob; each guarded by IsValidEncryptedValue so a re-save
-    // (or an already-encrypted value) never double-wraps.
+    // Encrypt the password + the cert blob; Protect is idempotent (null/empty/already-encrypted
+    // passes through) so a re-save never double-wraps.
     private void EncryptSecrets(Persistence.Entities.Deployments.Certificate certificate)
     {
         if (certificate == null) return;
 
-        certificate.Password = Protect(certificate.Password);
-        certificate.CertificateData = Protect(certificate.CertificateData);
-    }
-
-    private string Protect(string value)
-    {
-        if (string.IsNullOrEmpty(value) || encryption.IsValidEncryptedValue(value)) return value;
-
-        return encryption.EncryptAsync(value, SecretKdfScope);
+        certificate.Password = protector.Protect(certificate.Password, SecretKdfScope);
+        certificate.CertificateData = protector.Protect(certificate.CertificateData, SecretKdfScope);
     }
 
     private async Task<Persistence.Entities.Deployments.Certificate> DecryptSecretsAsync(Persistence.Entities.Deployments.Certificate certificate)
@@ -127,9 +120,9 @@ private string Protect(string value)
         // map is freed (no duplicate-tracking conflict on a same-scope update/delete).
         repository.Detach(certificate);
 
-        // Read-both: a plaintext (unprefixed) value is returned verbatim by DecryptAsync.
-        certificate.Password = await encryption.DecryptAsync(certificate.Password, SecretKdfScope).ConfigureAwait(false);
-        certificate.CertificateData = await encryption.DecryptAsync(certificate.CertificateData, SecretKdfScope).ConfigureAwait(false);
+        // Read-both: a plaintext (unprefixed) value is returned verbatim by Unprotect.
+        certificate.Password = await protector.UnprotectAsync(certificate.Password, SecretKdfScope).ConfigureAwait(false);
+        certificate.CertificateData = await protector.UnprotectAsync(certificate.CertificateData, SecretKdfScope).ConfigureAwait(false);
 
         return certificate;
     }

src/Squid.Core/Services/Deployments/ExternalFeeds/ExternalFeedDataProvider.cs

@@ -33,7 +33,7 @@ public interface IExternalFeedDataProvider : IScopedDependency
 /// in-memory entity can never be flushed back to the DB as plaintext by a later shared-scope
 /// SaveChanges.</para>
 /// </summary>
-public class ExternalFeedDataProvider(IUnitOfWork unitOfWork, IRepository repository, IVariableEncryptionService encryption) : IExternalFeedDataProvider
+public class ExternalFeedDataProvider(IUnitOfWork unitOfWork, IRepository repository, IAtRestSecretProtector protector) : IExternalFeedDataProvider
 {
     // The V2 envelope derives its key from a random per-payload salt, so the legacy KDF-scope
     // argument (a V1-only deterministic-salt input) is irrelevant for feeds, which only write V2.
@@ -100,21 +100,20 @@ public async Task DeleteExternalFeedsAsync(List<Persistence.Entities.Deployments
         return await DecryptPasswordAsync(feed).ConfigureAwait(false);
     }
 
-    // Encrypt only if not already an envelope — idempotent, so a re-save never double-wraps.
+    // Protect is idempotent (null/empty/already-encrypted passes through), so a re-save never double-wraps.
     private void EncryptPassword(Persistence.Entities.Deployments.ExternalFeed feed)
     {
-        if (feed == null || string.IsNullOrEmpty(feed.Password)) return;
-        if (encryption.IsValidEncryptedValue(feed.Password)) return;
+        if (feed == null) return;
 
-        feed.Password = encryption.EncryptAsync(feed.Password, PasswordKdfScope);
+        feed.Password = protector.Protect(feed.Password, PasswordKdfScope);
     }
 
     private async Task<Persistence.Entities.Deployments.ExternalFeed> DecryptPasswordAsync(Persistence.Entities.Deployments.ExternalFeed feed)
     {
         if (feed == null || string.IsNullOrEmpty(feed.Password)) return feed;
 
-        // Read-both: a plaintext (unprefixed) value is returned verbatim by DecryptAsync.
-        feed.Password = await encryption.DecryptAsync(feed.Password, PasswordKdfScope).ConfigureAwait(false);
+        // Read-both: a plaintext (unprefixed) value is returned verbatim by Unprotect.
+        feed.Password = await protector.UnprotectAsync(feed.Password, PasswordKdfScope).ConfigureAwait(false);
 
         return feed;
     }

src/Squid.Core/Services/Security/AtRestSecretProtector.cs

@@ -0,0 +1,57 @@
+namespace Squid.Core.Services.Security;
+
+/// <summary>
+/// Single source of truth for the per-field at-rest secret contract that every DataProvider
+/// seam (account credentials, feed password, certificate password + data, SSH proxy password)
+/// applies before persisting / after loading a sensitive string column or JSON field.
+///
+/// <para>Two halves, both non-breaking by construction:</para>
+/// <list type="bullet">
+///   <item><b>Protect</b> — encrypt idempotently: an already-encrypted envelope (or a null/empty
+///         value) is returned unchanged, so a re-save never double-wraps.</item>
+///   <item><b>Unprotect</b> — decrypt read-both: a legacy plaintext (unprefixed) value is returned
+///         verbatim, so reads work before AND after migration with no DB migration.</item>
+/// </list>
+///
+/// <para>The sync/async split is intentional and mirrors the wrapped service: <c>Protect</c> is
+/// synchronous because <see cref="IVariableEncryptionService.EncryptAsync"/> is synchronous despite
+/// its name (a pre-existing misnomer — it returns <c>string</c>, not a Task), while
+/// <c>UnprotectAsync</c> is genuinely async because <see cref="IVariableEncryptionService.DecryptAsync"/>
+/// is. Do NOT "normalize" the two halves — making <c>Protect</c> async adds a needless Task allocation
+/// and making <c>UnprotectAsync</c> sync would block on a real async decrypt.</para>
+///
+/// <para>This wraps <see cref="IVariableEncryptionService"/> (the AES-256-GCM V2 envelope + KDF) so
+/// the providers depend on this narrow contract rather than re-implementing the
+/// <c>IsValidEncryptedValue</c>-guarded encrypt + passthrough-decrypt in each one (previously four
+/// near-identical copies). The ENTITY-level anti-flush concern (read via <c>QueryNoTracking</c> or
+/// <c>Detach</c> before the in-place decrypt) stays in each provider — it is entity-specific and
+/// orthogonal to this value transform.</para>
+/// </summary>
+public interface IAtRestSecretProtector : IScopedDependency
+{
+    /// <summary>Encrypt <paramref name="value"/> for at-rest storage, idempotently. Null / empty /
+    /// already-encrypted input is returned unchanged. <paramref name="kdfScope"/> is the per-concern
+    /// KDF salt scope (the entity's encryption scope constant).</summary>
+    string Protect(string value, int kdfScope);
+
+    /// <summary>Decrypt an at-rest <paramref name="value"/>, read-both: a legacy unprefixed plaintext
+    /// is returned verbatim. <paramref name="kdfScope"/> must match the scope used to Protect it.</summary>
+    Task<string> UnprotectAsync(string value, int kdfScope);
+}
+
+public sealed class AtRestSecretProtector(IVariableEncryptionService encryption) : IAtRestSecretProtector
+{
+    public string Protect(string value, int kdfScope)
+    {
+        // Guard null/empty AND already-an-envelope so a re-save (or a value a previous save already
+        // encrypted) is never double-wrapped. EncryptAsync also short-circuits empty, but the explicit
+        // guard keeps IsValidEncryptedValue off a null and documents the idempotency contract here.
+        if (string.IsNullOrEmpty(value) || encryption.IsValidEncryptedValue(value)) return value;
+
+        return encryption.EncryptAsync(value, kdfScope);
+    }
+
+    public Task<string> UnprotectAsync(string value, int kdfScope)
+        // DecryptAsync is read-both: an unprefixed plaintext value passes through verbatim.
+        => encryption.DecryptAsync(value, kdfScope);
+}

tests/Squid.UnitTests/Services/Security/AtRestSecretProtectorTests.cs

@@ -0,0 +1,120 @@
+using Microsoft.Extensions.Configuration;
+using Squid.Core.Services.Security;
+using Squid.Core.Settings.Security;
+
+namespace Squid.UnitTests.Services.Security;
+
+/// <summary>
+/// Unit tests for <see cref="AtRestSecretProtector"/> — the SSOT value-transform extracted from the
+/// account / feed / certificate DataProviders (and reused by the SSH proxy slice). Pins the two
+/// non-breaking guarantees: idempotent Protect (null / empty / already-encrypted passes through, so
+/// a re-save never double-wraps) and read-both Unprotect (delegates to the encryption service's
+/// passthrough decrypt). The KDF scope is forwarded verbatim.
+/// </summary>
+public sealed class AtRestSecretProtectorTests
+{
+    private readonly Mock<IVariableEncryptionService> _encryption = new();
+    private readonly AtRestSecretProtector _sut;
+
+    public AtRestSecretProtectorTests()
+    {
+        _sut = new AtRestSecretProtector(_encryption.Object);
+    }
+
+    [Theory]
+    [InlineData(null)]
+    [InlineData("")]
+    public void Protect_NullOrEmpty_ReturnsUnchanged_WithoutEncrypting(string value)
+    {
+        _sut.Protect(value, kdfScope: 0).ShouldBe(value);
+
+        _encryption.Verify(e => e.EncryptAsync(It.IsAny<string>(), It.IsAny<int>()), Times.Never);
+        _encryption.Verify(e => e.IsValidEncryptedValue(It.IsAny<string>()), Times.Never,
+            failMessage: "null/empty must short-circuit before IsValidEncryptedValue is asked.");
+    }
+
+    [Fact]
+    public void Protect_AlreadyEncrypted_ReturnsUnchanged_WithoutReencrypting()
+    {
+        const string envelope = "SQUID_ENCRYPTED_V2:abc";
+        _encryption.Setup(e => e.IsValidEncryptedValue(envelope)).Returns(true);
+
+        _sut.Protect(envelope, kdfScope: 7).ShouldBe(envelope);
+
+        _encryption.Verify(e => e.EncryptAsync(It.IsAny<string>(), It.IsAny<int>()), Times.Never,
+            failMessage: "an already-encrypted envelope must not be double-wrapped on re-save.");
+    }
+
+    [Fact]
+    public void Protect_Plaintext_Encrypts_WithGivenScope()
+    {
+        _encryption.Setup(e => e.IsValidEncryptedValue("secret")).Returns(false);
+        _encryption.Setup(e => e.EncryptAsync("secret", 7)).Returns("SQUID_ENCRYPTED_V2:xyz");
+
+        _sut.Protect("secret", kdfScope: 7).ShouldBe("SQUID_ENCRYPTED_V2:xyz");
+
+        _encryption.Verify(e => e.EncryptAsync("secret", 7), Times.Once,
+            failMessage: "a plaintext value must be encrypted with the supplied KDF scope.");
+    }
+
+    [Fact]
+    public async Task UnprotectAsync_DelegatesToDecrypt_WithGivenScope()
+    {
+        _encryption.Setup(e => e.DecryptAsync("SQUID_ENCRYPTED_V2:xyz", 7)).ReturnsAsync("secret");
+
+        (await _sut.UnprotectAsync("SQUID_ENCRYPTED_V2:xyz", 7)).ShouldBe("secret");
+    }
+
+    [Fact]
+    public async Task UnprotectAsync_LegacyPlaintext_PassesThrough_ViaReadBothDecrypt()
+    {
+        // Read-both is DecryptAsync's contract — the protector just delegates. Pin that a value the
+        // encryption service returns verbatim (legacy unprefixed plaintext) flows through unchanged.
+        _encryption.Setup(e => e.DecryptAsync("legacy-plaintext", 0)).ReturnsAsync("legacy-plaintext");
+
+        (await _sut.UnprotectAsync("legacy-plaintext", 0)).ShouldBe("legacy-plaintext");
+    }
+
+    // ── Real-service round-trip (no mock) — pins the idempotency + envelope contract at the seam ──
+
+    [Fact]
+    public void Protect_RealService_Plaintext_YieldsV2Envelope_AndIsIdempotent()
+    {
+        var sut = new AtRestSecretProtector(RealEncryption());
+
+        var envelope = sut.Protect("plaintext-secret", kdfScope: 0);
+
+        envelope.ShouldStartWith("SQUID_ENCRYPTED_V2:",
+            customMessage: "Protect over the real service must emit the V2 envelope, not the plaintext.");
+        envelope.ShouldNotContain("plaintext-secret");
+        sut.Protect(envelope, kdfScope: 0).ShouldBe(envelope,
+            customMessage: "re-Protecting an already-encrypted envelope must return it unchanged (no double-wrap).");
+    }
+
+    [Fact]
+    public async Task ProtectThenUnprotect_RealService_RoundTrips()
+    {
+        var sut = new AtRestSecretProtector(RealEncryption());
+
+        var envelope = sut.Protect("plaintext-secret", kdfScope: 0);
+
+        (await sut.UnprotectAsync(envelope, kdfScope: 0)).ShouldBe("plaintext-secret");
+    }
+
+    // Real VariableEncryptionService with a real 32-byte key — passes Strict enforcement (a real key
+    // is mode-invariant, so no env control is needed here).
+    private static IVariableEncryptionService RealEncryption()
+    {
+        var key = new byte[32];
+        for (var i = 0; i < key.Length; i++) key[i] = (byte)(i + 1);
+
+        var config = new ConfigurationBuilder()
+            .AddInMemoryCollection(new Dictionary<string, string>
+            {
+                ["Security:VariableEncryption:MasterKey"] = Convert.ToBase64String(key)
+            })
+            .Build();
+
+        return new VariableEncryptionService(new SecuritySetting(config));
+    }
+}