Local connection check was accidentally inverted. Add test to ensure this does not repeat. (#1290)

prvyk · badrishc · TalZaccai · web-flow · commit 9e6e478e96d0 · 2025-07-15T19:09:46.000-07:00
Co-authored-by: Badrish Chandramouli &lt;badrishc@microsoft.com&gt;
Co-authored-by: Tal Zaccai &lt;talzacc@microsoft.com&gt;
diff --git a/libs/server/Resp/RespServerSession.cs b/libs/server/Resp/RespServerSession.cs
@@ -413,7 +413,7 @@ internal bool CanRunDebug()
             return
                 (enableDebugCommand == ConnectionProtectionOption.Yes) ||
                 ((enableDebugCommand == ConnectionProtectionOption.Local) &&
-                    !networkSender.IsLocalConnection());
+                    networkSender.IsLocalConnection());
         }
 
         internal bool CanRunModule()
@@ -423,7 +423,7 @@ internal bool CanRunModule()
             return
                 (enableModuleCommand == ConnectionProtectionOption.Yes) ||
                 ((enableModuleCommand == ConnectionProtectionOption.Local) &&
-                    !networkSender.IsLocalConnection());
+                    networkSender.IsLocalConnection());
         }
 
         public override int TryConsumeMessages(byte* reqBuffer, int bytesReceived)
diff --git a/test/Garnet.test/GarnetJSON/JsonCommandsTest.cs b/test/Garnet.test/GarnetJSON/JsonCommandsTest.cs
@@ -28,7 +28,7 @@ public void Setup()
             TestUtils.DeleteDirectory(TestUtils.MethodTestDir, wait: true);
             binPath = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location);
             server = TestUtils.CreateGarnetServer(TestUtils.MethodTestDir, lowMemory: true,
-                                                    enableModuleCommand: true,
+                                                    enableModuleCommand: Garnet.server.Auth.Settings.ConnectionProtectionOption.Yes,
                                                     extensionAllowUnsignedAssemblies: true,
                                                     extensionBinPaths: [binPath]);
             server.Start();
diff --git a/test/Garnet.test/GarnetServerConfigTests.cs b/test/Garnet.test/GarnetServerConfigTests.cs
@@ -860,6 +860,63 @@ public void UnixSocketPermission_InvalidPermissionFails()
             ClassicAssert.IsFalse(parseSuccessful);
         }
 
+        [Test]
+        [TestCase(ConnectionProtectionOption.No)]
+        [TestCase(ConnectionProtectionOption.Local)]
+        [TestCase(ConnectionProtectionOption.Yes)]
+        public async Task ConnectionProtectionTest(ConnectionProtectionOption connectionProtectionOption)
+        {
+            List<IPAddress> addresses = [IPAddress.IPv6Loopback, IPAddress.Loopback];
+
+            var hostname = TestUtils.GetHostName();
+
+            var address = Dns.GetHostAddresses(hostname).Where(x => !IPAddress.IsLoopback(x)).FirstOrDefault();
+            if (address == default)
+            {
+                if (connectionProtectionOption == ConnectionProtectionOption.Local)
+                    Assert.Ignore("No nonloopback address");
+            }
+            else
+            {
+                addresses.Add(address);
+            }
+
+            var endpoints = addresses.Select(address => new IPEndPoint(address, TestUtils.TestPort)).ToArray();
+            var server = TestUtils.CreateGarnetServer(TestUtils.MethodTestDir, endpoints: endpoints,
+                                                      enableDebugCommand: connectionProtectionOption);
+            server.Start();
+
+            foreach (var endpoint in endpoints)
+            {
+                var shouldfail = connectionProtectionOption == ConnectionProtectionOption.No ||
+                        (!IPAddress.IsLoopback(endpoint.Address) && connectionProtectionOption == ConnectionProtectionOption.Local);
+                var client = TestUtils.GetGarnetClientSession(endPoint: endpoint);
+                client.Connect();
+
+                try
+                {
+                    var result = await client.ExecuteAsync("DEBUG", "LOG", "Loopback test");
+                    if (shouldfail)
+                        Assert.Fail("Connection protection should have not allowed the command to run");
+                    else
+                        ClassicAssert.AreEqual("OK", result);
+                }
+                catch (Exception ex)
+                {
+                    if (shouldfail)
+                        ClassicAssert.AreEqual("ERR", ex.Message[0..3]);
+                    else
+                        Assert.Fail("Connection protection should have allowed command from this address");
+                }
+                finally
+                {
+                    client.Dispose();
+                }
+            }
+
+            server.Dispose();
+        }
+
         [Test]
         public async Task MultiTcpSocketTest()
         {
@@ -868,7 +925,7 @@ public async Task MultiTcpSocketTest()
             var addresses = Dns.GetHostAddresses(hostname);
             addresses = [.. addresses, IPAddress.IPv6Loopback, IPAddress.Loopback];
 
-            var endpoints = addresses.Select(address => new IPEndPoint(address, TestUtils.TestPort)).ToArray();
+            var endpoints = addresses.Distinct().Select(address => new IPEndPoint(address, TestUtils.TestPort)).ToArray();
             var server = TestUtils.CreateGarnetServer(TestUtils.MethodTestDir, endpoints: endpoints);
             server.Start();
 
diff --git a/test/Garnet.test/Resp/ACL/RespCommandTests.cs b/test/Garnet.test/Resp/ACL/RespCommandTests.cs
@@ -33,7 +33,8 @@ public void Setup()
         {
             TestUtils.DeleteDirectory(TestUtils.MethodTestDir, wait: true);
             server = TestUtils.CreateGarnetServer(TestUtils.MethodTestDir, defaultPassword: DefaultPassword,
-                                                  useAcl: true, enableLua: true, enableModuleCommand: true);
+                                                  useAcl: true, enableLua: true,
+                                                  enableModuleCommand: Garnet.server.Auth.Settings.ConnectionProtectionOption.Yes);
 
             // Register custom commands so we can test ACL'ing them
             ClassicAssert.IsTrue(TestUtils.TryGetCustomCommandsInfo(out respCustomCommandsInfo));
diff --git a/test/Garnet.test/RespCommandTests.cs b/test/Garnet.test/RespCommandTests.cs
@@ -77,7 +77,8 @@ .. respCommandsInfo.Values.Where(ci => ci.IsInternal).Select(ci => ci.Command)
             ];
 
             server = TestUtils.CreateGarnetServer(TestUtils.MethodTestDir, disablePubSub: true,
-                enableModuleCommand: true, extensionBinPaths: [extTestDir]);
+                enableModuleCommand: Garnet.server.Auth.Settings.ConnectionProtectionOption.Yes,
+                extensionBinPaths: [extTestDir]);
             server.Start();
         }
 
diff --git a/test/Garnet.test/RespCustomCommandTests.cs b/test/Garnet.test/RespCustomCommandTests.cs
@@ -238,7 +238,7 @@ public void Setup()
             TestUtils.DeleteDirectory(TestUtils.MethodTestDir, wait: true);
             server = TestUtils.CreateGarnetServer(TestUtils.MethodTestDir,
                 disablePubSub: true,
-                enableModuleCommand: true,
+                enableModuleCommand: Garnet.server.Auth.Settings.ConnectionProtectionOption.Yes,
                 extensionBinPaths: [_extTestDir1, _extTestDir2],
                 extensionAllowUnsignedAssemblies: true);
             server.Start();
diff --git a/test/Garnet.test/RespModuleTests.cs b/test/Garnet.test/RespModuleTests.cs
@@ -25,7 +25,7 @@ public void Setup()
             TestUtils.DeleteDirectory(TestUtils.MethodTestDir, wait: true);
             server = TestUtils.CreateGarnetServer(TestUtils.MethodTestDir,
                 disablePubSub: true,
-                enableModuleCommand: true,
+                enableModuleCommand: Garnet.server.Auth.Settings.ConnectionProtectionOption.Yes,
                 extensionBinPaths: [testModuleDir, binPath],
                 extensionAllowUnsignedAssemblies: true);
             server.Start();
@@ -386,7 +386,7 @@ public void TestNoAllowedPathsForModuleLoading()
             using var server = TestUtils.CreateGarnetServer(TestUtils.MethodTestDir,
                 disableObjects: true,
                 disablePubSub: true,
-                enableModuleCommand: true,
+                enableModuleCommand: Garnet.server.Auth.Settings.ConnectionProtectionOption.Yes,
                 extensionBinPaths: null,
                 extensionAllowUnsignedAssemblies: true);
             server.Start();
@@ -415,7 +415,7 @@ public void TestModuleCommandNotEnabled()
             using var server = TestUtils.CreateGarnetServer(TestUtils.MethodTestDir,
                 disableObjects: true,
                 disablePubSub: true,
-                enableModuleCommand: false,
+                enableModuleCommand: Garnet.server.Auth.Settings.ConnectionProtectionOption.No,
                 extensionBinPaths: [testModuleDir, binPath],
                 extensionAllowUnsignedAssemblies: true);
             server.Start();
diff --git a/test/Garnet.test/TestUtils.cs b/test/Garnet.test/TestUtils.cs
@@ -248,7 +248,8 @@ public static GarnetServer CreateGarnetServer(
             bool getSG = false,
             int indexResizeFrequencySecs = 60,
             IAuthenticationSettings authenticationSettings = null,
-            bool enableModuleCommand = false,
+            ConnectionProtectionOption enableDebugCommand = ConnectionProtectionOption.Yes,
+            ConnectionProtectionOption enableModuleCommand = ConnectionProtectionOption.No,
             bool enableLua = false,
             bool enableReadCache = false,
             bool enableObjectStoreReadCache = false,
@@ -345,8 +346,8 @@ public static GarnetServer CreateGarnetServer(
                 ThreadPoolMinThreads = threadPoolMinThreads,
                 LoadModuleCS = loadModulePaths,
                 EnableCluster = enableCluster,
-                EnableDebugCommand = ConnectionProtectionOption.Yes,
-                EnableModuleCommand = enableModuleCommand ? ConnectionProtectionOption.Yes : ConnectionProtectionOption.No,
+                EnableDebugCommand = enableDebugCommand,
+                EnableModuleCommand = enableModuleCommand,
                 EnableReadCache = enableReadCache,
                 EnableObjectStoreReadCache = enableObjectStoreReadCache,
                 ReplicationOffsetMaxLag = asyncReplay ? -1 : 0,

Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,8 @@ public void Setup()`
`33`	`33`	`{`
`34`	`34`	`TestUtils.DeleteDirectory(TestUtils.MethodTestDir, wait: true);`
`35`	`35`	`server = TestUtils.CreateGarnetServer(TestUtils.MethodTestDir, defaultPassword: DefaultPassword,`
`36`		`- useAcl: true, enableLua: true, enableModuleCommand: true);`
	`36`	`+ useAcl: true, enableLua: true,`
	`37`	`+ enableModuleCommand: Garnet.server.Auth.Settings.ConnectionProtectionOption.Yes);`
`37`	`38`
`38`	`39`	`// Register custom commands so we can test ACL'ing them`
`39`	`40`	`ClassicAssert.IsTrue(TestUtils.TryGetCustomCommandsInfo(out respCustomCommandsInfo));`
Original file line number	Diff line number	Diff line change
`@@ -77,7 +77,8 @@ .. respCommandsInfo.Values.Where(ci => ci.IsInternal).Select(ci => ci.Command)`
`77`	`77`	`];`
`78`	`78`
`79`	`79`	`server = TestUtils.CreateGarnetServer(TestUtils.MethodTestDir, disablePubSub: true,`
`80`		`- enableModuleCommand: true, extensionBinPaths: [extTestDir]);`
	`80`	`+ enableModuleCommand: Garnet.server.Auth.Settings.ConnectionProtectionOption.Yes,`
	`81`	`+ extensionBinPaths: [extTestDir]);`
`81`	`82`	`server.Start();`
`82`	`83`	`}`
`83`	`84`