From afdeb6751208368c592e12690365df3370564bd2 Mon Sep 17 00:00:00 2001
From: stag7824 <atalha54@gmail.com>
Date: Wed, 9 Sep 2026 13:15:28 +0200
Subject: [PATCH] kvm: trim whitespace around backup repository mount options
 on restore

A repository saved with mount options such as "vers=4.1 " backs up but fails to
restore. mountBackupDirectory builds the mount command as an argv list and runs
it without a shell, so ProcessBuilder hands the option list to mount verbatim.
libmount splits that list on commas only, so the blank stays glued to the last
option and the kernel rejects it as part of the option value.

The other backup operations pass the options to nasbackup.sh, which expands
MOUNT_OPTS unquoted, so shell word splitting drops the blank there. Restore is
the only one of the four that builds the mount command in Java.

Trim each comma-separated option and drop the empty ones before the list is
used. Only the whitespace around the delimiters is removed; validating the
option text belongs to the API layer. The normalisation runs before the cifs
branch appends nobrl, otherwise the blank would be moved into the middle of the
list rather than removed.

Fixes #14013
---
 .../LibvirtRestoreBackupCommandWrapper.java   | 26 +++++++
 ...ibvirtRestoreBackupCommandWrapperTest.java | 75 +++++++++++++++++++
 2 files changed, 101 insertions(+)

diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java
index 7f2a3c28cd14..9c9806bdc77c 100644
--- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java
@@ -211,6 +211,7 @@ private String mountBackupDirectory(String backupRepoAddress, String backupRepoT
             mountCmd.add(backupRepoType);
             mountCmd.add(backupRepoAddress);
             mountCmd.add(mountDirectory);
+            mountOptions = normalizeMountOptions(mountOptions);
             if ("cifs".equals(backupRepoType)) {
                 if (StringUtils.isBlank(mountOptions)) {
                     mountOptions = "nobrl";
@@ -230,6 +231,31 @@ private String mountBackupDirectory(String backupRepoAddress, String backupRepoT
         return mountDirectory;
     }
 
+    /**
+     * Removes the whitespace around each option of a comma-separated mount option list.
+     *
+     * mount(8) is handed the list as a single argument and libmount splits it on commas only, so a
+     * blank next to a comma, or at either end of the list, stays glued to the neighbouring option
+     * and is rejected as part of its name or value.
+     *
+     * Only the whitespace around the delimiters is removed. The option text itself is passed
+     * through unchanged, as rejecting unsafe or malformed options belongs to the API layer, not
+     * to the agent.
+     */
+    protected static String normalizeMountOptions(String mountOptions) {
+        if (StringUtils.isBlank(mountOptions)) {
+            return mountOptions;
+        }
+        List<String> options = new ArrayList<>();
+        for (String option : mountOptions.split(",")) {
+            String trimmedOption = option.trim();
+            if (!trimmedOption.isEmpty()) {
+                options.add(trimmedOption);
+            }
+        }
+        return String.join(",", options);
+    }
+
     private void unmountBackupDirectory(String backupDirectory) {
         try {
             String umountPath = Script.getExecutableAbsolutePath("umount");
diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java
index 4bbd040b0d1b..ca7c66f33ffc 100644
--- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java
+++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java
@@ -703,4 +703,79 @@ public void testAttachVolumePassesRbdXmlThroughAFile() throws Exception {
         Assert.assertFalse(args.stream().anyMatch(arg -> arg.contains("EOF")));
         Assert.assertTrue(args.get(args.size() - 1).endsWith(".xml"));
     }
+
+    private String[] captureMountCommand(String backupRepoType, String mountOptions) throws Exception {
+        Method method = LibvirtRestoreBackupCommandWrapper.class.getDeclaredMethod("mountBackupDirectory",
+                String.class, String.class, String.class, Integer.class);
+        method.setAccessible(true);
+
+        final String[][] captured = new String[1][];
+        try (MockedStatic<Files> filesMock = mockStatic(Files.class)) {
+            Path tempPath = Mockito.mock(Path.class);
+            when(tempPath.toString()).thenReturn("/tmp/csbackup.abc123");
+            filesMock.when(() -> Files.createTempDirectory(anyString())).thenReturn(tempPath);
+
+            try (MockedStatic<Script> scriptMock = mockStatic(Script.class)) {
+                scriptMock.when(() -> Script.getExecutableAbsolutePath(anyString()))
+                        .thenAnswer(invocation -> invocation.getArgument(0));
+                scriptMock.when(() -> Script.executeCommand(any(String[].class)))
+                        .thenAnswer(invocation -> {
+                            // Mockito expands varargs, so the command comes back as individual arguments.
+                            captured[0] = Arrays.stream(invocation.getArguments()).map(String::valueOf).toArray(String[]::new);
+                            return "";
+                        });
+                method.invoke(wrapper, "10.0.0.1:/export/backup", backupRepoType, mountOptions, 30);
+            }
+        }
+        return captured[0];
+    }
+
+    private String getMountOptions(String[] mountCmd) {
+        List<String> args = Arrays.asList(mountCmd);
+        int index = args.indexOf("-o");
+        return index < 0 ? null : args.get(index + 1);
+    }
+
+    @Test
+    public void testMountTrimsTrailingWhitespaceFromMountOptions() throws Exception {
+        // A repository saved with "vers=4.1 " reaches mount as a single argument and libmount splits
+        // it on commas only, so the blank would stay glued to the option and be rejected.
+        Assert.assertEquals("vers=4.1", getMountOptions(captureMountCommand("nfs", "vers=4.1 ")));
+    }
+
+    @Test
+    public void testMountTrimsWhitespaceAroundEveryOption() throws Exception {
+        Assert.assertEquals("vers=4.1,soft", getMountOptions(captureMountCommand("nfs", " vers=4.1 , soft ")));
+    }
+
+    @Test
+    public void testMountDropsEmptyMountOptions() throws Exception {
+        Assert.assertEquals("vers=4.1,soft", getMountOptions(captureMountCommand("nfs", "vers=4.1,,soft")));
+    }
+
+    @Test
+    public void testMountDoesNotStripWhitespaceInsideAnOption() throws Exception {
+        // Only the blanks around the delimiters are removed. Whether an option is well formed is
+        // decided when the repository is saved, so the agent passes the text itself through as is
+        // rather than silently rewriting a stored value.
+        Assert.assertEquals("username=some user,nobrl",
+                getMountOptions(captureMountCommand("cifs", "username=some user")));
+    }
+
+    @Test
+    public void testMountAppendsNobrlAfterTrimmingCifsOptions() throws Exception {
+        // nobrl is appended to the list, so the blank has to go first, otherwise it ends up in the
+        // middle of the list where libmount cannot ignore it either.
+        Assert.assertEquals("vers=3.0,nobrl", getMountOptions(captureMountCommand("cifs", "vers=3.0 ")));
+    }
+
+    @Test
+    public void testMountOmitsMountOptionsThatAreOnlyWhitespace() throws Exception {
+        Assert.assertNull(getMountOptions(captureMountCommand("nfs", "   ")));
+    }
+
+    @Test
+    public void testMountFallsBackToNobrlWhenCifsOptionsAreOnlyWhitespace() throws Exception {
+        Assert.assertEquals("nobrl", getMountOptions(captureMountCommand("cifs", "   ")));
+    }
 }