Fix permissions when restore run as root user.

When restore was run as the root user the pg_control file would end up with root permissions. This bug was introduced in e634fd8. Fix this by directly overwriting the pg_control temp file rather than doing an atomic write that updates permissions. Also update other parameters to more closely match similar calls.

There was also an adjacent error where restore as the root user would fail if the base path did not exist. Fix this by ignoring the missing path since it will be created later and this logic is just trying to find an alternate user for permissions if the user in the manifest does not exist.

doc/xml/release/2024/2.54.xml

@@ -1,5 +1,20 @@
 <release date="XXXX-XX-XX" version="2.54dev" title="UNDER DEVELOPMENT">
     <release-core-list>
+        <release-bug-list>
+            <release-item>
+                <github-issue id="2407"/>
+                <github-pull-request id="2416"/>
+
+                <release-item-contributor-list>
+                    <release-item-ideator id="will.m"/>
+                    <release-item-contributor id="david.steele"/>
+                    <release-item-reviewer id="stefan.fercot"/>
+                </release-item-contributor-list>
+
+                <p>Fix permissions when <cmd>restore</cmd> run as root user.</p>
+            </release-item>
+        </release-bug-list>
+
         <release-improvement-list>
             <release-item>
                 <github-pull-request id="2399"/>

doc/xml/release/contributor.xml

@@ -1098,6 +1098,10 @@
     <contributor-id type="github">vthriller</contributor-id>
 </contributor>
 
+<contributor id="will.m">
+    <contributor-name-display>Will M</contributor-name-display>
+</contributor>
+
 <contributor id="william.cox">
     <contributor-name-display>William Cox</contributor-name-display>
     <contributor-id type="github">mydimension</contributor-id>

src/command/restore/restore.c

@@ -778,7 +778,7 @@ restoreManifestOwner(const Manifest *const manifest, const String **const rootRe
         if (userRoot())
         {
             // Get user/group info from data directory to use for invalid user/groups
-            StorageInfo pathInfo = storageInfoP(storagePg(), manifestTargetBase(manifest)->path);
+            StorageInfo pathInfo = storageInfoP(storagePg(), manifestTargetBase(manifest)->path, .ignoreMissing = true);
 
             // If user/group is null then set it to root
             if (pathInfo.user == NULL)                                                                              // {vm_covered}
@@ -2618,7 +2618,7 @@ cmdRestore(void)
                 storagePutP(
                     storageNewWriteP(
                         storagePgWrite(), STRDEF(PG_PATH_GLOBAL "/" PG_FILE_PGCONTROL "." STORAGE_FILE_TEMP_EXT),
-                        .modeFile = pgControlFile.mode, .timeModified = pgControlFile.timestamp),
+                        .timeModified = pgControlFile.timestamp, .noAtomic = true, .noCreatePath = true, .noSyncPath = true),
                     pgControlBuffer);
             }
 

test/src/common/harnessHost.c

@@ -266,6 +266,7 @@ hrnHostExecBr(HrnHost *const this, const char *const command, const HrnHostExecB
     FUNCTION_HARNESS_BEGIN();
         FUNCTION_HARNESS_PARAM(HRN_HOST, this);
         FUNCTION_HARNESS_PARAM(STRINGZ, command);
+        FUNCTION_HARNESS_PARAM(STRINGZ, param.user);
         FUNCTION_HARNESS_PARAM(STRINGZ, param.option);
         FUNCTION_HARNESS_PARAM(STRINGZ, param.param);
         FUNCTION_HARNESS_PARAM(INT, param.resultExpect);
@@ -278,6 +279,7 @@ hrnHostExecBr(HrnHost *const this, const char *const command, const HrnHostExecB
 
     MEM_CONTEXT_TEMP_BEGIN()
     {
+        const String *const user = param.user == NULL ? NULL : STR(param.user);
         String *const commandStr = strCatFmt(strNew(), "pgbackrest --stanza=" HRN_STANZA);
 
         if (param.option != NULL)
@@ -288,7 +290,7 @@ hrnHostExecBr(HrnHost *const this, const char *const command, const HrnHostExecB
         if (param.param != NULL)
             strCatFmt(commandStr, " %s", param.param);
 
-        strCat(result, hrnHostExecP(this, commandStr, .resultExpect = param.resultExpect));
+        strCat(result, hrnHostExecP(this, commandStr, .user = user, .resultExpect = param.resultExpect));
     }
     MEM_CONTEXT_TEMP_END();
 

test/src/common/harnessHost.h

@@ -315,6 +315,7 @@ String *hrnHostExec(HrnHost *this, const String *command, HrnHostExecParam param
 typedef struct HrnHostExecBrParam
 {
     VAR_PARAM_HEADER;
+    const char *user;                                               // User to execute command
     const char *option;                                             // Options
     const char *param;                                              // Parameters
     int resultExpect;                                               // Expected result, if not 0

test/src/module/integration/allTest.c

@@ -117,8 +117,48 @@ testRun(void)
         // -------------------------------------------------------------------------------------------------------------------------
         TEST_TITLE("standby restore");
         {
-            TEST_HOST_BR(
-                pg2, CFGCMD_RESTORE, .option = zNewFmt("--type=standby --tablespace-map=ts1='%s'", strZ(hrnHostPgTsPath(pg2))));
+            // If the repo is running on pg2 then switch the user to root to test restoring as root. Use this configuration so the
+            // repo is local and root does not need to be configured for TLS or SSH.
+            const char *user = NULL;
+
+            if (pg2 == repo)
+            {
+                user = "root";
+
+                // Create the pg/ts directory so it will not be created with root permissions
+                HRN_STORAGE_PATH_CREATE(hrnHostDataStorage(pg2), strZ(hrnHostPgTsPath(pg2)), .mode = 0700);
+
+                // Create the restore log file so it will not be created with root permissions
+                HRN_STORAGE_PUT_EMPTY(hrnHostDataStorage(pg2), zNewFmt("%s/test-restore.log", strZ(hrnHostLogPath(pg2))));
+            }
+
+            // Run restore
+            const char *option = zNewFmt("--type=standby --tablespace-map=ts1='%s'", strZ(hrnHostPgTsPath(pg2)));
+            TEST_HOST_BR(pg2, CFGCMD_RESTORE, .option = option, .user = user);
+
+            // If the repo is running on pg2 then perform additional ownership tests
+            if (pg2 == repo)
+            {
+                // Update some ownership to root
+                hrnHostExecP(
+                    pg2,
+                    strNewFmt(
+                        "chown %s:%s %s &&"
+                        "chown %s:%s %s/" PG_PATH_GLOBAL " &&"
+                        "chown %s:%s %s/" PG_PATH_GLOBAL "/" PG_FILE_PGCONTROL,
+                        user, user, strZ(hrnHostPgDataPath(pg2)), user, user, strZ(hrnHostPgDataPath(pg2)), user, user,
+                        strZ(hrnHostPgDataPath(pg2))),
+                    .user = STRDEF(user));
+
+                // Expect an error when running without root since permissions cannot be updated
+                TEST_HOST_BR(
+                    pg2, CFGCMD_RESTORE, .option = zNewFmt("--delta %s", option), .user = NULL,
+                    .resultExpect = errorTypeCode(&FileOpenError));
+
+                // Running as root fixes the ownership
+                TEST_HOST_BR(pg2, CFGCMD_RESTORE, .option = zNewFmt("--delta %s", option), .user = user);
+            }
+
             HRN_HOST_PG_START(pg2);
 
             // Check standby