From a2d3b0a65192bccb89771f54bd556909e7a014e4 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Mon, 10 Aug 2026 19:18:55 +0000
Subject: [PATCH] Fix smb.conf section removal deleting everything after the
 target share
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Reported live: Samba broke on the home box after this ran. Root cause
confirmed by reproducing it directly: the old removal step used
`sed -i "/^\[share\]$/,/^$/d"` — a range delete from the share's header
through the next BLANK line. A home box whose smb.conf has no blank line
separating sections (common — nothing requires one) means that range
never finds a terminator and sed deletes straight through to end of file,
taking every share defined after the target one down with it. Reproduced
against a 4-section smb.conf with no blank lines: the old approach left
only [global] standing, silently destroying two unrelated, pre-existing
shares that had nothing to do with this tool.

Replaced with an awk pass that removes lines from the target share's own
[header] up to the next `[section]` header or EOF — the actual boundary
of an INI-style section, independent of blank-line formatting. Also now
builds the new config in a scratch file and validates it with `testparm`
before it's ever copied over the live smb.conf; on validation failure it
leaves the existing file untouched and exits instead of restarting smbd
against a config that might not even parse. The existing
smb.conf.backup.<timestamp> step (already present before this fix) is
what the user is recovering the home box with in the meantime.

Verified the fix against the exact reproduction: the same 4-section,
no-blank-line smb.conf now retains all three untouched sections after
removing only the target one.
---
 services/vpn-data-mount.sh | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/services/vpn-data-mount.sh b/services/vpn-data-mount.sh
index 1baa3fb..5aefbaf 100644
--- a/services/vpn-data-mount.sh
+++ b/services/vpn-data-mount.sh
@@ -364,12 +364,25 @@ _vdm_setup_remote_samba() {
 sudo smbpasswd -e '${user}'"
     fi
 
+    # Builds the new config in a scratch file and validates it with
+    # testparm BEFORE it ever touches the live smb.conf — confirmed live:
+    # the previous approach here (sed range-deleting from the [share_name]
+    # header through the next BLANK line) silently deleted straight through
+    # to end of file on a home box whose smb.conf had no blank line
+    # separating sections, taking unrelated shares down with it. Section
+    # removal now stops at the next `[section]` header (or EOF) instead of
+    # a blank line, which is the actual boundary of an INI-style section
+    # regardless of how the file happens to be formatted.
     local remote_cmd
     remote_cmd=$(cat << REMOTECMD
 set -e
 sudo mkdir -p '${remote_path}'
-sudo cp /etc/samba/smb.conf /etc/samba/smb.conf.backup.\$(date +%Y%m%d-%H%M%S) 2>/dev/null || true
-sudo sed -i "/^\\[${share_name}\\]\$/,/^\$/d" /etc/samba/smb.conf
+BACKUP="/etc/samba/smb.conf.backup.\$(date +%Y%m%d-%H%M%S)"
+sudo cp /etc/samba/smb.conf "\$BACKUP"
+sudo awk -v target='[${share_name}]' '
+  /^\\[/ { skip = (\$0 == target) }
+  !skip { print }
+' /etc/samba/smb.conf > /tmp/smb.conf.vdm.new
 {
   echo ""
   echo "[${share_name}]"
@@ -379,7 +392,14 @@ sudo sed -i "/^\\[${share_name}\\]\$/,/^\$/d" /etc/samba/smb.conf
   echo "   guest ok = no"
   echo "   valid users = ${user}"
   echo "   force user = ${user}"
-} | sudo tee -a /etc/samba/smb.conf >/dev/null
+} >> /tmp/smb.conf.vdm.new
+if sudo testparm -s /tmp/smb.conf.vdm.new >/dev/null 2>&1; then
+  sudo cp /tmp/smb.conf.vdm.new /etc/samba/smb.conf
+  rm -f /tmp/smb.conf.vdm.new
+else
+  echo "New smb.conf failed testparm validation — leaving the existing config untouched. Backup at \$BACKUP, rejected draft at /tmp/smb.conf.vdm.new for inspection." >&2
+  exit 1
+fi
 ${pw_cmd}
 sudo systemctl restart smbd
 command -v ufw >/dev/null 2>&1 && sudo ufw allow samba >/dev/null 2>&1 || true