refactor: extract addDnsRules helper to eliminate duplicate DNS iptables rule pairs (#3273)

* Initial plan

* refactor: extract addDnsRules helper for DNS iptables rules

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Landon Cox <landon.cox@microsoft.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Author: 3 people

src/host-iptables-rules.ts

@@ -7,6 +7,7 @@ import {
   CHAIN_NAME,
   CHAIN_NAME_V6,
   NETWORK_NAME,
+  addDnsRules,
   disableIpv6ViaSysctl,
   getDockerBridgeGateway,
   getNetworkBridgeName,
@@ -224,28 +225,10 @@ export async function setupHostIptables(squidIp: string, squidPort: number, dnsS
     const isV6 = dnsServer.includes(':');
     if (isV6) {
       if (ip6tablesAvailable) {
-        await execa('ip6tables', [
-          '-t', 'filter', '-A', CHAIN_NAME_V6,
-          '-p', 'udp', '-d', dnsServer, '--dport', '53',
-          '-j', 'ACCEPT',
-        ]);
-        await execa('ip6tables', [
-          '-t', 'filter', '-A', CHAIN_NAME_V6,
-          '-p', 'tcp', '-d', dnsServer, '--dport', '53',
-          '-j', 'ACCEPT',
-        ]);
+        await addDnsRules('ip6tables', CHAIN_NAME_V6, dnsServer);
       }
     } else {
-      await execa('iptables', [
-        '-t', 'filter', '-A', CHAIN_NAME,
-        '-p', 'udp', '-d', dnsServer, '--dport', '53',
-        '-j', 'ACCEPT',
-      ]);
-      await execa('iptables', [
-        '-t', 'filter', '-A', CHAIN_NAME,
-        '-p', 'tcp', '-d', dnsServer, '--dport', '53',
-        '-j', 'ACCEPT',
-      ]);
+      await addDnsRules('iptables', CHAIN_NAME, dnsServer);
     }
   }
 
@@ -259,16 +242,7 @@ export async function setupHostIptables(squidIp: string, squidPort: number, dnsS
   // 5a. Allow DNS traffic to DoH proxy sidecar (when enabled)
   if (dohProxyIp) {
     logger.debug(`Allowing DNS traffic to DoH proxy sidecar at ${dohProxyIp}:53`);
-    await execa('iptables', [
-      '-t', 'filter', '-A', CHAIN_NAME,
-      '-p', 'udp', '-d', dohProxyIp, '--dport', '53',
-      '-j', 'ACCEPT',
-    ]);
-    await execa('iptables', [
-      '-t', 'filter', '-A', CHAIN_NAME,
-      '-p', 'tcp', '-d', dohProxyIp, '--dport', '53',
-      '-j', 'ACCEPT',
-    ]);
+    await addDnsRules('iptables', CHAIN_NAME, dohProxyIp);
   }
 
   // 5b. Allow traffic to API proxy sidecar (when enabled)

src/host-iptables-shared.ts

@@ -107,6 +107,44 @@ export async function disableIpv6ViaSysctl(): Promise<void> {
   }
 }
 
+/**
+ * Adds both UDP and TCP ACCEPT rules on port 53 for the given destination to a chain.
+ * This helper keeps DNS allowlist rules as a consistent pair by rolling back any
+ * successfully-added rule if a later add fails.
+ */
+export async function addDnsRules(
+  cmd: 'iptables' | 'ip6tables',
+  chain: string,
+  destination: string,
+): Promise<void> {
+  const addedProtos: Array<'udp' | 'tcp'> = [];
+
+  try {
+    for (const proto of ['udp', 'tcp'] as const) {
+      await execa(cmd, [
+        '-t', 'filter', '-A', chain,
+        '-p', proto, '-d', destination, '--dport', '53',
+        '-j', 'ACCEPT',
+      ]);
+      addedProtos.push(proto);
+    }
+  } catch (error) {
+    for (const proto of addedProtos.reverse()) {
+      try {
+        await execa(cmd, [
+          '-t', 'filter', '-D', chain,
+          '-p', proto, '-d', destination, '--dport', '53',
+          '-j', 'ACCEPT',
+        ]);
+      } catch (rollbackError) {
+        logger.warn(`Failed to roll back ${cmd} DNS ${proto} rule for ${destination}:`, rollbackError);
+      }
+    }
+
+    throw error;
+  }
+}
+
 /**
  * Re-enables IPv6 via sysctl if it was previously disabled.
  */