fix(worker): don't follow links planted at a job's output file name (bug-1868)

uv_proc_exec() created /tmp/job_<jobid>_task<N>.out as root with
O_WRONLY|O_CREAT|O_TRUNC. The name is predictable and /tmp is world-
writable, so a local user could plant a symlink (or hardlink) there before
the job started and make the root worker truncate and write job output
into the target. Only the host's fs.protected_* sysctls stood in the way.

xcecd_create_job_output() (job_output_path.h) now unlinks whatever is at
the name and creates a fresh file with O_EXCL|O_NOFOLLOW|O_CLOEXEC, mode
0600 as before; O_EXCL never follows a symlink and fails if anything
reappears, so a race can only fail the create, never redirect it (retried
3 times). A re-run job still starts with an empty file, as O_TRUNC gave.

New test_job_output_path.c (4 tests, 2 RED before the fix); worker suite
11/11.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

src/worker/include/job_output_path.h

@@ -6,12 +6,18 @@
  * name; handle_fetchfile() (controller_connector.c, xcecd) serves reads of it
  * and must accept nothing else (bug-1508). The two live in different
  * executables, so the format is shared here rather than in either .c file.
+ * xcecd_create_job_output() is the only way the file should be created
+ * (bug-1868).
  */
+#include <errno.h>
+#include <fcntl.h>
 #include <stdbool.h>
 #include <stddef.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <string.h>
+#include <sys/stat.h>
+#include <unistd.h>
 
 #include "utils.h"
 
@@ -69,3 +75,29 @@ static inline bool xcecd_is_job_output_path(uint64_t jobid, const char* path)
 
     return strcmp(p, ".out") == 0;
 }
+
+/* bug-1868: create (or re-create, for a re-run job) a task's output file.
+ * The name is predictable and lives in world-writable /tmp while the worker
+ * runs as root, so a plain O_CREAT|O_TRUNC open would truncate and write
+ * through a symlink or hardlink a local user planted there beforehand.
+ * Instead, remove whatever is at the name (root may unlink in a sticky dir)
+ * and create a new file with O_EXCL|O_NOFOLLOW: O_EXCL never follows a
+ * symlink and fails if anything reappears in between, so a racing attacker
+ * can at worst make this fail, never redirect the write. Retries a few
+ * times on that race. Returns an fd or -errno. */
+static inline int xcecd_create_job_output(const char* path)
+{
+    int attempt;
+    int fd = -1;
+
+    for (attempt = 0; attempt < 3; attempt++) {
+        if (unlink(path) < 0 && errno != ENOENT)
+            return -errno;
+        fd = open(path,
+                  O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW | O_CLOEXEC,
+                  S_IRUSR | S_IWUSR);
+        if (fd >= 0 || errno != EEXIST)
+            break;
+    }
+    return (fd < 0) ? -errno : fd;
+}

src/worker/src/uv_proc.c

@@ -248,13 +248,10 @@ int uv_proc_exec(uv_proc_manager_t* manager, const char* command,
                               manager->job_id, i);
 
         uv_stdio_container_t stdio[3];
-        uv_fs_t              file_req;
-        int                  fd
-            = uv_fs_open(manager->loop, &file_req, job_output,
-                         O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR, NULL);
+        int                  fd = xcecd_create_job_output(job_output);
         if (fd < 0) {
-            XCECD_WORKER_LOG(XCE_LOG_ERROR, "uv_fs_open failed: %s",
-                             uv_strerror(fd));
+            XCECD_WORKER_LOG(XCE_LOG_ERROR, "create %s failed: %s", job_output,
+                             strerror(-fd));
             free(process);
 
             for (int j = 0; j < manager->proc_count; j++) {
@@ -290,8 +287,7 @@ int uv_proc_exec(uv_proc_manager_t* manager, const char* command,
 
         int result = uv_spawn(manager->loop, process, &options);
 
-        uv_fs_close(manager->loop, &file_req, fd, NULL);
-        uv_fs_req_cleanup(&file_req);
+        close(fd);
 
         if (result != 0) {
             XCECD_WORKER_LOG(XCE_LOG_ERROR, "uv_spawn failed: %s",

src/worker/testsuite/meson.build

@@ -180,3 +180,16 @@ test('worker xcecd_cgroups rusage test', xcecd_cgroups_rusage_test,
     suite: 'worker',
     timeout: 60,
     env: ['ASAN_OPTIONS=detect_leaks=0'])
+
+# bug-1868: the root worker's per-task output file creation must not follow
+# a symlink/hardlink planted at its predictable /tmp name (job_output_path.h).
+job_output_path_test = executable('job_output_path_test',
+                                  'test_job_output_path.c',
+                                  include_directories: [worker_test_incdir],
+                                  dependencies: worker_test_deps,
+                                  install: false)
+
+test('worker job_output_path test', job_output_path_test,
+    suite: 'worker',
+    timeout: 60,
+    env: ['ASAN_OPTIONS=detect_leaks=0'])

src/worker/testsuite/test_job_output_path.c

@@ -0,0 +1,130 @@
+/*
+ * test_job_output_path.c — bug-1868: the root worker creates each task's
+ * output file at a predictable name in world-writable /tmp
+ * (job_output_path.h). xcecd_create_job_output() must never write through a
+ * symlink or hardlink planted at that name, while still giving a re-run job
+ * a fresh, empty file.
+ */
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#include <unity/unity.h>
+
+#include "job_output_path.h"
+
+#define JOP_JOBID ((uint64_t)918680001ULL)
+
+static char g_path[256];
+static char g_victim[64];
+
+void setUp(void)
+{
+    int fd;
+
+    xcecd_job_output_path(g_path, sizeof(g_path), JOP_JOBID, 0);
+    unlink(g_path);
+
+    strcpy(g_victim, "/tmp/xce_jop_victim_XXXXXX");
+    fd = mkstemp(g_victim);
+    TEST_ASSERT_TRUE(fd >= 0);
+    TEST_ASSERT_EQUAL_INT(8, (int)write(fd, "PRECIOUS", 8));
+    close(fd);
+}
+
+void tearDown(void)
+{
+    unlink(g_path);
+    unlink(g_victim);
+}
+
+static void assert_victim_untouched(void)
+{
+    char buf[16] = { 0 };
+    int  fd      = open(g_victim, O_RDONLY);
+
+    TEST_ASSERT_TRUE(fd >= 0);
+    TEST_ASSERT_EQUAL_INT(8, (int)read(fd, buf, sizeof(buf) - 1));
+    close(fd);
+    TEST_ASSERT_EQUAL_STRING_MESSAGE("PRECIOUS", buf,
+        "the file a planted link pointed at must not be truncated/written");
+}
+
+static void assert_fresh_private_regular_file(int fd)
+{
+    struct stat st;
+
+    TEST_ASSERT_EQUAL_INT(0, lstat(g_path, &st));
+    TEST_ASSERT_TRUE_MESSAGE(S_ISREG(st.st_mode),
+        "the output name must now be a regular file, not a link");
+    TEST_ASSERT_EQUAL_INT(1, (int)st.st_nlink);
+    TEST_ASSERT_EQUAL_INT(0, (int)st.st_size);
+    TEST_ASSERT_EQUAL_INT(S_IRUSR | S_IWUSR, (int)(st.st_mode & 0777));
+
+    TEST_ASSERT_EQUAL_INT(2, (int)write(fd, "ok", 2));
+}
+
+static void test_creates_new_output_file(void)
+{
+    int fd = xcecd_create_job_output(g_path);
+
+    TEST_ASSERT_TRUE(fd >= 0);
+    assert_fresh_private_regular_file(fd);
+    close(fd);
+}
+
+static void test_rerun_gets_empty_file(void)
+{
+    int fd = open(g_path, O_WRONLY | O_CREAT, 0600);
+
+    TEST_ASSERT_TRUE(fd >= 0);
+    TEST_ASSERT_EQUAL_INT(3, (int)write(fd, "old", 3));
+    close(fd);
+
+    fd = xcecd_create_job_output(g_path);
+    TEST_ASSERT_TRUE(fd >= 0);
+    assert_fresh_private_regular_file(fd);
+    close(fd);
+}
+
+static void test_planted_symlink_is_not_followed(void)
+{
+    int fd;
+
+    TEST_ASSERT_EQUAL_INT(0, symlink(g_victim, g_path));
+
+    fd = xcecd_create_job_output(g_path);
+    if (fd >= 0) {
+        assert_fresh_private_regular_file(fd);
+        close(fd);
+    }
+    assert_victim_untouched();
+}
+
+static void test_planted_hardlink_is_not_written_through(void)
+{
+    int fd;
+
+    TEST_ASSERT_EQUAL_INT(0, link(g_victim, g_path));
+
+    fd = xcecd_create_job_output(g_path);
+    if (fd >= 0) {
+        assert_fresh_private_regular_file(fd);
+        close(fd);
+    }
+    assert_victim_untouched();
+}
+
+int main(void)
+{
+    UNITY_BEGIN();
+    RUN_TEST(test_creates_new_output_file);
+    RUN_TEST(test_rerun_gets_empty_file);
+    RUN_TEST(test_planted_symlink_is_not_followed);
+    RUN_TEST(test_planted_hardlink_is_not_written_through);
+    return UNITY_END();
+}