feat(scheduler-core): implement recording directory provisioning and path validation (task 9.1)

Adds --record-dir CLI flag and sched_validate_record_filename() with
canonical-path containment (realpath()-based, not textual-prefix) plus
separator/".." rejection before any filesystem access.

Adversarial review found and this task fixed two CWE-59 inode-aliasing
bypasses: a direct hard link inside record_dir aliasing an outside file,
and a compound variant where a symlink pointed at an in-directory file
itself hard-linked outside record_dir. Both closed via st_nlink checks
on the direct entry and the fully-resolved symlink target respectively,
each verified with a live RED->GREEN reproduction. TOCTOU residual risk
documented and deferred to task 9.2 (O_NOFOLLOW + fstat() re-check).

18/18 recording subtests, 24/24 scheduler suite, clang-format clean.

.kiro/specs/scheduler-core/tasks.md

@@ -216,12 +216,13 @@
 
 - [ ] 9. Core: Recording Control
 
-- [ ] 9.1 Implement recording directory provisioning and path validation
+- [x] 9.1 Implement recording directory provisioning and path validation
   - Add a scheduler startup CLI flag (parsed in the same argument-parsing pass as the rest of normal-mode's flags) for the recording directory — never settable from any request-path field — defaulting to a fixed system path
   - Validate any caller-supplied record file name before ever opening it for writing: reject any value containing a path separator or a `..` segment, resolve the final path beneath the recording directory, and confirm containment before opening
   - Observable: a record-file name containing a path separator or `..` is rejected before any filesystem access occurs; a bare filename resolves to a path inside the recording directory and is accepted
   - _Requirements: 5.10_
   - _Depends: 8.1_
+  - _Verified: 2026-08-04 — `--record-dir` CLI flag added to main.c/scheduler.h (record_dir field, never a request-path field). sched_validate_record_filename() (new recording.c/recording.h) rejects any `filename` containing `/` or `..` before any filesystem call, then resolves via realpath() and confirms canonical (not textual-prefix) containment beneath record_dir via path_is_contained(); rejects pre-existing non-regular entries (-EEXIST) and a non-directory record_dir (-ENOTDIR). Adversarial security review found and this task fixed two real CWE-59 inode-aliasing bypasses across 3 rounds: round 1 found a direct hard link inside record_dir aliasing an outside file passed validation (S_ISREG doesn't distinguish a hard-linked regular file from a genuine one) — fixed with an entry_st.st_nlink > 1 check (-EACCES) in the direct-entry branch, live RED→GREEN reproduced. Round 2 re-review found a compound variant still open: a symlink inside record_dir pointing at an in-directory file itself hard-linked outside record_dir was still accepted, since the S_ISLNK branch checked containment but never st_nlink on the resolved target — fixed by stat()'ing the already-realpath()-resolved target and applying the same S_ISREG && st_nlink>1 check, generalizing correctly to multi-hop symlink chains, live RED→GREEN reproduced. Round 3 re-review independently reproduced both fixes via a standalone PoC harness and additionally hunted for further variants (2-hop chains, symlink-to-symlink escape, FIFO/socket direct-naming) — all clean, no new bypass found; one non-blocking observation logged (symlink resolving to a contained-but-non-regular target is accepted rather than rejected, pre-existing since round 1, not the CWE-22/CWE-59 escape class, already covered by task 9.2's planned O_NOFOLLOW hardening) — APPROVED. TOCTOU residual risk (validation-to-eventual-fopen() window) explicitly documented in recording.h and deferred to task 9.2 (O_NOFOLLOW + fstat()-based st_nlink==1 re-check on the opened fd), not this task's scope. 18/18 recording subtests (16 original + 2 hard-link/compound regressions), 24/24 scheduler suite, clang-format clean — all independently re-verified by me via a fresh `meson compile`/`meson test --suite scheduler` run on current HEAD before marking complete._
 
 - [ ] 9.2 Implement the record start/stop lifecycle
   - On start: take a full snapshot, write the replay header followed by the initial snapshot entry, begin tracking recording state

src/scheduler/include/recording.h

@@ -0,0 +1,131 @@
+#if !defined(_RECORDING_H_)
+
+#define _RECORDING_H_ 1
+
+#include <stddef.h>
+
+/*
+ * recording.h - scheduler-core spec, task 9.1 (Req 5 AC 10, "Record file
+ * path validation"). First piece of design.md's File Structure Plan
+ * `recording.c` ("NEW (Phase 3): record start/stop/status, ReplayEvent/
+ * ReplaySession writes") -- this task adds ONLY the validation function
+ * every future write must pass through first; the actual record start/
+ * stop/status lifecycle (design.md's NormalDriver service interface --
+ * sched_driver_record_start()/_stop()/_status(), `xce_sched_driver_t`'s
+ * recording-state fields) is task 9.2's job and is NOT implemented here.
+ *
+ * sched_validate_record_filename - validates a caller-supplied, admin-
+ * authenticated but UNTRUSTED record file name (controller.capnp's
+ * `SchedAdminRequest.recordFile` -- "filename (not a path)", per that
+ * field's own comment) and resolves it to a full path beneath `record_dir`
+ * BEFORE the caller (task 9.2) ever opens it for writing. `record_dir`
+ * itself is trusted: per design.md's NormalDriver component, it is
+ * provisioned EXACTLY ONE WAY, the scheduler's own `--record-dir` startup
+ * CLI flag (main.c), and is never settable from any request-path field --
+ * so only `filename` is adversarial input here, never `record_dir`.
+ *
+ * Two-phase check, per requirements.md Req 5 AC 10 "Record file path
+ * validation":
+ *
+ *   1. String-level rejection, BEFORE any filesystem call ever touches
+ *      `filename`: reject if `filename` is NULL/empty, contains a path
+ *      separator ('/'), or contains a ".." substring anywhere (leading,
+ *      trailing, embedded, doubled -- e.g. "..", "../x", "x/..", "..x",
+ *      "x..", "x..y", "...."). Since step 1 also bans '/' outright, a bare
+ *      ".." substring check is both necessary and sufficient: with no
+ *      slash permitted at all, `filename` can never have more than one
+ *      path segment, so there is no "segment" for ".." to hide in other
+ *      than the substring itself. C strings terminate at the first NUL
+ *      byte, so none of these checks -- nor the eventual fopen() -- can
+ *      ever observe bytes past an embedded NUL; there is no bypass there.
+ *
+ *   2. Canonical-path containment, AFTER the string check passes:
+ *      `record_dir` (which must already exist -- provisioning/creating it
+ *      on disk is explicitly out of this task's scope; see tasks.md 9.1's
+ *      boundary) is resolved to its canonical form via realpath(), then
+ *      the validated bare `filename` is joined onto that canonical form
+ *      (never onto the possibly-non-canonical original `record_dir`
+ *      string). Because `filename` has no '/' and no "..", this join
+ *      cannot itself escape the resolved directory by construction -- the
+ *      one remaining escape vector is a pre-existing filesystem entry
+ *      already named `filename` inside `record_dir` that is itself a
+ *      symlink pointing elsewhere. That case is checked explicitly: the
+ *      symlink's target is independently canonicalized via realpath() and
+ *      tested for containment with a true canonical-path comparison
+ *      (resolved target must equal the resolved directory followed by
+ *      '/') -- NEVER a textual prefix match, which would wrongly accept a
+ *      sibling directory like "/var/log/xce-evil/..." against a
+ *      `record_dir` of "/var/log/xce".
+ *
+ *      As a defensive measure beyond the letter of AC 10 (still within its
+ *      "before opening it for writing" intent): if an entry already named
+ *      `filename` exists and is neither a regular file nor a (validated,
+ *      contained) symlink -- e.g. a directory, device node, FIFO, or
+ *      socket -- validation fails rather than handing back a path task
+ *      9.2 would blindly fopen().
+ *
+ *      A second defensive measure, same rationale: an existing REGULAR
+ *      file named `filename` with more than one hard link (st_nlink > 1)
+ *      is also rejected. A hard link shares its target's inode -- unlike
+ *      a symlink, there is no separate "link target" path for realpath()
+ *      to resolve and contain-check, so a hard link planted inside
+ *      record_dir by anything with write access there (pointing at a
+ *      file OUTSIDE record_dir) passes S_ISREG and looks canonically
+ *      contained even though writing through it would alias a file
+ *      outside record_dir. Same threat class as the symlink-escape case
+ *      above (CWE-59 link-following), just via inode aliasing instead of
+ *      path resolution -- rejected with the same -EACCES. This st_nlink
+ *      check applies to the FULLY-RESOLVED target reached after
+ *      realpath() has followed any number of symlink hops -- not just to
+ *      an entry named `filename` directly: a symlink inside record_dir
+ *      may itself point at a hard-linked alias (whether that alias lives
+ *      directly inside record_dir or is reached through further
+ *      intermediate symlinks), and containment alone does not detect
+ *      that the resolved target's inode is also reachable from outside
+ *      record_dir. Both the direct-entry case and every symlink-hop case
+ *      are checked against the same resolved target's st_nlink.
+ *
+ * Returns 0 on success; `out_path` holds the resolved, validated full path
+ * (record_dir's canonical form + '/' + the validated bare filename).
+ * Returns a negative errno on failure:
+ *   -EINVAL        `record_dir`/`filename`/`out_path` is NULL,
+ *                   `out_path_len` is 0, or `filename` is empty, contains
+ *                   '/', or contains "..".
+ *   -ENAMETOOLONG   the resolved path does not fit in `out_path_len`.
+ *   -ENOENT         `record_dir` does not exist / cannot be resolved.
+ *   -ENOTDIR        `record_dir` resolves to something other than a
+ *                   directory.
+ *   -EACCES         an existing entry named `filename` inside `record_dir`
+ *                   either (a) resolves (via realpath()) to a location
+ *                   outside `record_dir` -- the symlink-escape case -- or
+ *                   (b) is, or (via any number of symlink hops) resolves
+ *                   to, a regular file with more than one hard link
+ *                   (st_nlink > 1) -- the hard-link inode-aliasing case,
+ *                   checked against the fully-resolved target either way;
+ *                   this function exists to catch both.
+ *   -EEXIST         an existing entry named `filename` inside `record_dir`
+ *                   is neither a regular file nor a symlink (see the
+ *                   defensive measure above).
+ *
+ * No filesystem call is made on `filename` itself until step 2 passes --
+ * and even then, only `record_dir` and (if present) the pre-existing
+ * `record_dir/filename` entry are ever touched. Any malformed-string
+ * rejection in step 1 returns -EINVAL without a single filesystem call,
+ * even if `record_dir` itself does not exist.
+ *
+ * NOTE for task 9.2 (not implemented here -- this task is validation-only):
+ * this function's containment/aliasing checks are only valid up to the
+ * moment it returns -- a TOCTOU race could still swap a symlink or hard
+ * link into `out_path` between this call returning and task 9.2's eventual
+ * fopen(). When task 9.2 opens `out_path` for writing, it should (a) pass
+ * O_NOFOLLOW so a symlink swapped in after validation is refused at
+ * open() rather than followed, and (b) fstat() the resulting fd and
+ * reject unless st_nlink == 1, since O_NOFOLLOW alone does not defend
+ * against a hard link swapped in after validation (no symlink is involved
+ * in that race, so O_NOFOLLOW never triggers).
+ */
+extern int sched_validate_record_filename(const char* record_dir,
+                                          const char* filename, char* out_path,
+                                          size_t out_path_len);
+
+#endif

src/scheduler/include/scheduler.h

@@ -102,6 +102,21 @@ typedef struct {
     xcecli_ctx_t     apictx;
     xce_cluster_config_t
         cluster_cfg; /* HA candidate list — empty means single-host mode */
+    /* Req 5 AC 10 (scheduler-core spec, task 9.1): recording directory,
+     * provisioned EXACTLY ONE WAY -- the --record-dir startup CLI flag
+     * (main.c), default "/var/log/xce/". Never settable via any
+     * request-path field (SchedAdminRequest included) -- see
+     * recording.h's sched_validate_record_filename() and design.md's
+     * NormalDriver component. Lives here (not xce_sched_core_t) because
+     * recording is a driver-level concern, the same reason apictx/
+     * cluster_cfg above live here rather than on core. Points into argv
+     * or a static string literal (never heap-owned), same convention as
+     * this struct's neighboring driver-level string fields elsewhere in
+     * this codebase (e.g. main.c's `logfile`) -- no free() needed. The
+     * rest of the recording STATE design.md sketches for a future
+     * xce_sched_driver_t (record_file_path/record_file/recording/
+     * record_event_count/...) is task 9.2's job, not this field's. */
+    char* record_dir;
 } xce_scheduler_t;
 
 extern int  sched_init(xce_scheduler_t*, xce_client_config_t*,

src/scheduler/meson.build

@@ -11,6 +11,7 @@ scheduler_srcs = [
   'src/snapshot_wire.c',
   'src/snapshot_load.c',
   'src/decision.c',
+  'src/recording.c',
   'src/schedobj.c',
   'src/plugin.c',
   'src/resource.c',

src/scheduler/src/main.c

@@ -16,7 +16,6 @@
 #include "resreq.h"
 #include "utils.h"
 
-
 extern int scheduler_req_handler(xce_request_t*, void*);
 
 static char doc[]      = "xce_scheduler";
@@ -25,11 +24,17 @@ const char* argp_program_version
     = MK_VERSION("xce v0.0.0.1-", STR(VERSION_COMMIT_ID));
 const char* argp_program_bug_address = BUG_EMAIL;
 
+/* Long-option identifiers (beyond ASCII range to avoid collisions with the
+ * short-option letters above -- same convention as the controller's
+ * main.c, e.g. its OPT_PEERS). */
+#define OPT_RECORD_DIR 256
+
 struct arguments {
     char* logfile;
     int   verbose;
     char* peers;      /* --peers value (optional) */
     char* sim_file;   /* --sim value (optional; Phase 3, not yet implemented) */
+    char* record_dir; /* --record-dir value (optional; Req 5 AC 10, task 9.1) */
 };
 
 static struct argp_option options[] = {
@@ -44,6 +49,13 @@ static struct argp_option options[] = {
     { "sim", 's', "FILE", 0,
      "Replay FILE for simulation mode "
       "(Phase 3 -- not yet implemented)" },
+    /* Req 5 AC 10 "Record file path validation" (scheduler-core spec, task
+     * 9.1): the ONLY way record_dir is ever provisioned -- never settable
+     * via any request-path field (SchedAdminRequest included). Parsed
+     * alongside the rest of normal-mode's flags, same argp pass as above. */
+    { "record-dir", OPT_RECORD_DIR, "DIR", 0,
+     "Directory for scheduler session recordings "
+      "(default: /var/log/xce/)" },
     { 0 }
 };
 
@@ -64,6 +76,9 @@ static error_t parse_opt(int key, char* arg, struct argp_state* state)
     case 's':
         arguments->sim_file = arg;
         break;
+    case OPT_RECORD_DIR:
+        arguments->record_dir = arg;
+        break;
     default:
         return ARGP_ERR_UNKNOWN;
     }
@@ -114,8 +129,12 @@ int main(int argc, char* argv[])
     char*            loglevel = "info";
     int              logsize  = 64 * 1024 * 1024;
     int              logcount = 10;
+    /* Req 5 AC 10 (scheduler-core spec, task 9.1): default recording
+     * directory when --record-dir is not given -- see scheduler.h's
+     * xce_scheduler_t.record_dir field comment. */
+    char*                       record_dir = "/var/log/xce/";
     xce_client_config_t         config;
-    xce_cluster_config_t        cluster_cfg     = {0};
+    xce_cluster_config_t        cluster_cfg     = { 0 };
     const xce_cluster_config_t* cluster_cfg_ptr = NULL;
 
     memset(&arguments, 0x0, sizeof(arguments));
@@ -130,6 +149,13 @@ int main(int argc, char* argv[])
         loglevel = verbose_to_loglevel(arguments.verbose);
     }
 
+    /* Req 5 AC 10 (scheduler-core spec, task 9.1): --record-dir is the
+     * ONLY way record_dir is ever provisioned -- see this flag's
+     * options[] entry above and scheduler.h's field comment. */
+    if (arguments.record_dir != NULL) {
+        record_dir = arguments.record_dir;
+    }
+
     mylog_init(logfile, loglevel, logsize, logcount);
 
     /* Req 5 AC 9 "Mode selection" (scheduler-core spec, task 5.2): --sim
@@ -158,8 +184,7 @@ int main(int argc, char* argv[])
         strncpy(cluster_cfg.identity, "xce_scheduler",
                 sizeof(cluster_cfg.identity) - 1);
         if (xce_cluster_config_validate(&cluster_cfg) != 0) {
-            fprintf(stderr,
+            fprintf(stderr, "xce_scheduler: invalid cluster config (missing "
-                    "xce_scheduler: invalid cluster config (missing "
                             "identity?).\n");
             return 1;
         }
@@ -179,6 +204,12 @@ int main(int argc, char* argv[])
 
     sched_init(&sched, &config, cluster_cfg_ptr, scheduler_req_handler);
 
+    /* Req 5 AC 10 (scheduler-core spec, task 9.1): stash the resolved
+     * --record-dir value on the live xce_scheduler_t -- see scheduler.h's
+     * field comment. Not consumed anywhere yet (task 9.2's job); this
+     * task only wires the value through to a sensible home. */
+    sched.record_dir = record_dir;
+
     /* Req 5 AC 6 (scheduler-core spec, task 5.2): register the driver's
      * decision callback -- replaces main.c's former inline
      * `switch (decision->op)`. Registered once, before the first

src/scheduler/src/recording.c

@@ -0,0 +1,171 @@
+#include <errno.h>
+#include <limits.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+
+#include "recording.h"
+
+/*
+ * recording.c - scheduler-core spec, task 9.1. See recording.h for the
+ * full contract/rationale. This is deliberately the ONLY function in this
+ * file as of task 9.1 -- record start/stop/status (task 9.2, design.md's
+ * NormalDriver) extend this file later; nothing here anticipates that
+ * work's shape beyond what this task's own scope requires.
+ */
+
+/*
+ * path_is_contained - true iff canonical path `child` is `parent` itself
+ * plus a '/'-separated descendant -- i.e. `child` starts with `parent`
+ * AND the byte immediately following that prefix is '/'. The second half
+ * of that check is what makes this safe against the classic prefix trap:
+ * without it, parent="/var/log/xce" would wrongly accept
+ * child="/var/log/xce-evil/target" (textually a prefix match, but a
+ * completely different, sibling directory). Both arguments must already
+ * be canonical (realpath()'d) -- this function does no resolution itself.
+ */
+static int path_is_contained(const char* child, const char* parent)
+{
+    size_t parent_len = strlen(parent);
+
+    if (strncmp(child, parent, parent_len) != 0) {
+        return 0;
+    }
+
+    return child[parent_len] == '/';
+}
+
+int sched_validate_record_filename(const char* record_dir, const char* filename,
+                                   char* out_path, size_t out_path_len)
+{
+    char        resolved_dir[PATH_MAX];
+    struct stat dir_st;
+    struct stat entry_st;
+    int         written;
+
+    if ((record_dir == NULL) || (filename == NULL) || (out_path == NULL)
+        || (out_path_len == 0)) {
+        return -EINVAL;
+    }
+
+    /* Step 1 (Req 5 AC 10): reject a malformed filename before ANY
+     * filesystem call ever touches it -- no realpath(), no stat(), no
+     * fopen(). See recording.h's header comment for why a plain ".."
+     * substring check is sufficient once '/' is already banned outright. */
+    if (filename[0] == '\0') {
+        return -EINVAL;
+    }
+    if (strchr(filename, '/') != NULL) {
+        return -EINVAL;
+    }
+    if (strstr(filename, "..") != NULL) {
+        return -EINVAL;
+    }
+
+    /* Step 2: canonicalize record_dir -- trusted input (only ever comes
+     * from the --record-dir startup CLI flag, main.c), never derived from
+     * `filename`. Must already exist; creating it is out of this task's
+     * scope (tasks.md 9.1's "recording directory provisioning" means
+     * "wire the CLI flag through", not "mkdir it on disk"). */
+    if (realpath(record_dir, resolved_dir) == NULL) {
+        return (errno > 0) ? -errno : -ENOENT;
+    }
+
+    if (stat(resolved_dir, &dir_st) != 0) {
+        return (errno > 0) ? -errno : -ENOENT;
+    }
+    if (!S_ISDIR(dir_st.st_mode)) {
+        return -ENOTDIR;
+    }
+
+    /* The validated target: canonical record_dir + '/' + the bare
+     * filename. filename has no '/' and no ".." (step 1), so this join
+     * cannot itself escape resolved_dir -- the only remaining escape
+     * vector is an EXISTING entry already named `filename` inside
+     * record_dir that happens to be a symlink pointing elsewhere,
+     * checked below. */
+    written = snprintf(out_path, out_path_len, "%s/%s", resolved_dir, filename);
+    if ((written < 0) || ((size_t)written >= out_path_len)) {
+        return -ENAMETOOLONG;
+    }
+
+    if (lstat(out_path, &entry_st) == 0) {
+        if (S_ISLNK(entry_st.st_mode)) {
+            char        resolved_entry[PATH_MAX];
+            struct stat target_st;
+
+            /* Step 3: an existing symlink named `filename` -- resolve ITS
+             * target independently and confirm containment via a true
+             * canonical-path comparison (path_is_contained()), never a
+             * textual prefix match. This is the "record_dir contains a
+             * symlink pointing outside itself" attack this function
+             * exists to catch: a naive record_dir + "/" + filename join
+             * looks contained textually, but fopen()-ing it would follow
+             * the symlink straight out of record_dir. */
+            if (realpath(out_path, resolved_entry) == NULL) {
+                return (errno > 0) ? -errno : -EACCES;
+            }
+            if (!path_is_contained(resolved_entry, resolved_dir)) {
+                return -EACCES;
+            }
+
+            /* Hard-link inode-aliasing escape, reached via a symlink hop:
+             * containment alone only proves resolved_entry's PATH sits
+             * inside record_dir -- it says nothing about how many hard
+             * links point at that same inode. record_dir could contain a
+             * regular file that is itself a hard link to a victim file
+             * OUTSIDE record_dir (caught directly by the st_nlink check in
+             * the branch below when named directly), plus a symlink INSIDE
+             * record_dir pointing at that hard-linked entry. The symlink's
+             * target textually/canonically resolves inside record_dir, so
+             * containment passes, but fopen()-ing out_path would still
+             * follow the symlink into the aliased outside inode -- the
+             * exact CWE-59 impact this function exists to prevent, just
+             * one extra hop away. stat() (not lstat()) here is intentional
+             * and non-racy: realpath() has already fully resolved the
+             * entire symlink chain into resolved_entry, so stat()-ing that
+             * already-canonical path merely reads the final target's link
+             * count -- it does not re-resolve anything a TOCTOU swap could
+             * race against beyond what realpath() itself already assumed. */
+            if (stat(resolved_entry, &target_st) != 0) {
+                return (errno > 0) ? -errno : -EACCES;
+            }
+            if (S_ISREG(target_st.st_mode) && (target_st.st_nlink > 1)) {
+                return -EACCES;
+            }
+        } else if (!S_ISREG(entry_st.st_mode)) {
+            /* Defensive measure beyond AC 10's literal text (see
+             * recording.h): an existing directory/device/FIFO/socket
+             * named `filename` is not a sane recording target -- refuse
+             * rather than hand back a path task 9.2 would blindly
+             * fopen(). Also closes the degenerate filename="." case:
+             * record_dir/. lstat()s as record_dir itself (a directory,
+             * not a symlink), caught here. */
+            return -EEXIST;
+        } else if (entry_st.st_nlink > 1) {
+            /* Hard-link inode-aliasing escape: a symlink has a distinct
+             * target path that realpath()/lstat() can resolve and check
+             * for containment (step 3 above) -- a hard link has none. It
+             * IS the same inode as whatever file it was link()'d from, so
+             * an entry named `filename` here that is a regular file with
+             * more than one link could have been link()'d in from OUTSIDE
+             * record_dir by anything with write access to this directory.
+             * It passes S_ISREG and looks textually/canonically contained
+             * (out_path IS inside record_dir), but task 9.2's
+             * fopen(out_path, "w") would truncate/overwrite whatever else
+             * that inode is linked from -- the exact write-outside-
+             * record_dir impact this function exists to prevent, via
+             * CWE-59 link-following, same threat class and same error
+             * code as the symlink-escape case above. A freshly created
+             * file has st_nlink == 1; nothing this function itself
+             * creates can ever trip this check. */
+            return -EACCES;
+        }
+        /* else: an existing regular file with a single link -- a
+         * legitimate overwrite target, acceptable, will be overwritten
+         * by task 9.2's fopen(). */
+    }
+
+    return 0;
+}

src/scheduler/testsuite/meson.build

@@ -393,6 +393,25 @@ sched_snapshot_save_load_test = executable(
     dependencies: [scheduler_test_deps],
     install: false)
 
+# White-box test for the recording-directory path-validation function
+# (scheduler-core spec, task 9.1, Req 5 AC 10 "Record file path
+# validation"): rejects a recordFile containing a path separator or a ".."
+# segment BEFORE any filesystem access, and confirms containment beneath
+# record_dir via a genuine canonical-path (realpath()) comparison rather
+# than a textual prefix match -- including a symlink-escape case and the
+# "record_dir vs record_dir-evil sibling" prefix-match trap named in this
+# task's own security brief. Same white-box #include pattern as
+# test_job_update.c/test_handlers.c (pulls the real
+# ../src/recording.c into this TU) -- the function under test has no
+# scheduler-struct fixture dependency at all, so this is the simplest
+# white-box test in this directory.
+sched_recording_path_validation_test = executable(
+    'sched_recording_path_validation_test',
+    ['test_recording_path_validation.c'],
+    include_directories:[incdir],
+    dependencies: [scheduler_test_deps],
+    install: false)
+
 # Register as meson tests so `meson test --suite scheduler` runs them all.
 # detect_leaks=0 matches the suite convention (fixtures are not torn down).
 test('scheduler test', scheduler_test, suite: 'scheduler',
@@ -453,3 +472,6 @@ test('scheduler driver session ordering test',
 test('scheduler snapshot save load test',
      sched_snapshot_save_load_test, suite: 'scheduler',
      env: ['ASAN_OPTIONS=detect_leaks=0'])
+test('scheduler recording path validation test',
+     sched_recording_path_validation_test, suite: 'scheduler',
+     env: ['ASAN_OPTIONS=detect_leaks=0'])

src/scheduler/testsuite/test_recording_path_validation.c

@@ -0,0 +1,487 @@
+/*
+ * test_recording_path_validation.c - contract test for
+ * sched_validate_record_filename() (scheduler-core spec, task 9.1, Req 5
+ * AC 10 "Record file path validation").
+ *
+ * Observable under test:
+ *   - A filename containing a path separator ('/') is rejected (-EINVAL)
+ *     before ANY filesystem call is made -- proven by pairing a malicious
+ *     filename with a record_dir that does not exist on disk: if the
+ *     function had proceeded past the string check, resolving that
+ *     nonexistent record_dir would have failed with -ENOENT instead. Only
+ *     -EINVAL coming back proves the string check short-circuited first.
+ *   - A filename containing a ".." substring -- leading, trailing,
+ *     embedded, or doubled -- is rejected the same way, same proof.
+ *   - A record_dir containing a pre-existing symlink whose target escapes
+ *     record_dir is REJECTED even when the filename component looks
+ *     perfectly safe (no '/', no ".."), via a genuine canonical-path
+ *     (realpath()) comparison -- not a textual prefix match. A dedicated
+ *     test also proves the comparison isn't a naive strncmp() prefix
+ *     check by using a sibling directory whose name textually starts with
+ *     record_dir's own canonical path (the "/var/log/xce-evil" vs
+ *     "/var/log/xce" trap called out in this task's own security brief).
+ *   - A legitimate bare filename resolves to record_dir/filename and is
+ *     accepted.
+ *
+ * White-box #include of ../src/recording.c (same convention as every
+ * other white-box test in this directory) -- the function under test has
+ * no internal state or fixture dependencies, so this is purely for the
+ * standalone-executable convention this testsuite already establishes
+ * (e.g. test_job_update.c, test_picker_iface.c), not because anything
+ * static/internal needs exercising beyond the one exported function.
+ */
+#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>
+
+/* Pull in the real sched_validate_record_filename() under test. */
+#include "../src/recording.c"
+
+void setUp(void) { return; }
+void tearDown(void) { return; }
+
+/* ---------------------------------------------------------------------- */
+/* Helpers                                                                 */
+/* ---------------------------------------------------------------------- */
+
+static void make_tmp_dir(char* out, size_t out_len, const char* suffix)
+{
+    char tmpl[PATH_MAX];
+
+    /* mkdtemp() requires the template's LAST SIX characters to be
+     * "XXXXXX" -- suffix must precede it, not follow it. */
+    snprintf(tmpl, sizeof(tmpl), "/tmp/sched_recval%s_XXXXXX", suffix);
+    TEST_ASSERT_NOT_NULL(mkdtemp(tmpl));
+    TEST_ASSERT_TRUE(strlen(tmpl) < out_len);
+    strcpy(out, tmpl);
+}
+
+static void touch_file(const char* path)
+{
+    int fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0644);
+    TEST_ASSERT_TRUE(fd >= 0);
+    close(fd);
+}
+
+/* ---------------------------------------------------------------------- */
+/* NULL / basic argument validation                                        */
+/* ---------------------------------------------------------------------- */
+
+static void test_null_record_dir_returns_einval(void)
+{
+    char out[PATH_MAX];
+    int  rc
+        = sched_validate_record_filename(NULL, "foo.replay", out, sizeof(out));
+    TEST_ASSERT_EQUAL_INT(-EINVAL, rc);
+}
+
+static void test_null_filename_returns_einval(void)
+{
+    char out[PATH_MAX];
+    int  rc = sched_validate_record_filename("/tmp", NULL, out, sizeof(out));
+    TEST_ASSERT_EQUAL_INT(-EINVAL, rc);
+}
+
+static void test_null_out_path_returns_einval(void)
+{
+    int rc = sched_validate_record_filename("/tmp", "foo.replay", NULL, 64);
+    TEST_ASSERT_EQUAL_INT(-EINVAL, rc);
+}
+
+static void test_zero_out_path_len_returns_einval(void)
+{
+    char out[PATH_MAX];
+    int  rc = sched_validate_record_filename("/tmp", "foo.replay", out, 0);
+    TEST_ASSERT_EQUAL_INT(-EINVAL, rc);
+}
+
+static void test_empty_filename_returns_einval(void)
+{
+    char out[PATH_MAX];
+    int  rc = sched_validate_record_filename("/tmp", "", out, sizeof(out));
+    TEST_ASSERT_EQUAL_INT(-EINVAL, rc);
+}
+
+/* ---------------------------------------------------------------------- */
+/* Path-separator rejection -- BEFORE any filesystem access (Req 5 AC 10)  */
+/* ---------------------------------------------------------------------- */
+
+static void test_filename_with_slash_rejected_before_filesystem_access(void)
+{
+    /* record_dir deliberately does NOT exist -- if the implementation had
+     * proceeded past the string check, realpath(record_dir) would fail
+     * with -ENOENT. Getting -EINVAL back instead proves the '/' check
+     * short-circuited first, with zero filesystem calls on the malicious
+     * string (or even on record_dir). */
+    static const char* bad_names[]
+        = { "/etc/passwd", "evil/", "a/b", "../../etc/passwd", "/" };
+    char   out[PATH_MAX];
+    size_t i;
+
+    for (i = 0; i < sizeof(bad_names) / sizeof(bad_names[0]); i++) {
+        int rc = sched_validate_record_filename(
+            "/definitely/does/not/exist/xyz123", bad_names[i], out,
+            sizeof(out));
+        TEST_ASSERT_EQUAL_INT_MESSAGE(-EINVAL, rc, bad_names[i]);
+    }
+}
+
+/* ---------------------------------------------------------------------- */
+/* ".." rejection, all positions -- BEFORE any filesystem access           */
+/* ---------------------------------------------------------------------- */
+
+static void test_dotdot_all_positions_rejected_before_filesystem_access(void)
+{
+    static const char* bad_names[] = {
+        "..",     /* bare */
+        "...",    /* tripled dot, still contains ".." */
+        "....",   /* quadrupled */
+        "..evil", /* leading */
+        "evil..", /* trailing */
+        "ev..il", /* embedded */
+        "a..b..c" /* doubled */
+    };
+    char   out[PATH_MAX];
+    size_t i;
+
+    for (i = 0; i < sizeof(bad_names) / sizeof(bad_names[0]); i++) {
+        int rc = sched_validate_record_filename(
+            "/definitely/does/not/exist/xyz123", bad_names[i], out,
+            sizeof(out));
+        TEST_ASSERT_EQUAL_INT_MESSAGE(-EINVAL, rc, bad_names[i]);
+    }
+}
+
+/* ---------------------------------------------------------------------- */
+/* record_dir configuration errors (distinct from malicious-filename path) */
+/* ---------------------------------------------------------------------- */
+
+static void test_nonexistent_record_dir_with_valid_filename_returns_error(void)
+{
+    /* A perfectly well-formed filename still fails once the (nonexistent)
+     * record_dir itself is resolved -- proves this is a DIFFERENT error
+     * path than the string-rejection tests above (which never reach
+     * realpath() at all). */
+    char out[PATH_MAX];
+    int rc = sched_validate_record_filename("/definitely/does/not/exist/xyz123",
+                                            "session.replay", out, sizeof(out));
+    TEST_ASSERT_TRUE(rc < 0);
+    TEST_ASSERT_NOT_EQUAL(-EINVAL, rc);
+}
+
+static void test_record_dir_that_is_a_file_returns_enotdir(void)
+{
+    char record_dir[PATH_MAX];
+    char file_path[PATH_MAX];
+    char out[PATH_MAX];
+    int  rc;
+
+    make_tmp_dir(record_dir, sizeof(record_dir), "_parent");
+    snprintf(file_path, sizeof(file_path), "%s/not_a_dir", record_dir);
+    touch_file(file_path);
+
+    rc = sched_validate_record_filename(file_path, "session.replay", out,
+                                        sizeof(out));
+    TEST_ASSERT_EQUAL_INT(-ENOTDIR, rc);
+}
+
+/* ---------------------------------------------------------------------- */
+/* Legitimate filename acceptance                                          */
+/* ---------------------------------------------------------------------- */
+
+static void test_legitimate_bare_filename_resolves_inside_record_dir(void)
+{
+    char record_dir[PATH_MAX];
+    char resolved_dir[PATH_MAX];
+    char out[PATH_MAX];
+    char expected[PATH_MAX];
+    int  rc;
+
+    make_tmp_dir(record_dir, sizeof(record_dir), "_ok");
+    TEST_ASSERT_NOT_NULL(realpath(record_dir, resolved_dir));
+
+    rc = sched_validate_record_filename(record_dir, "session.replay", out,
+                                        sizeof(out));
+    TEST_ASSERT_EQUAL_INT(0, rc);
+
+    snprintf(expected, sizeof(expected), "%s/session.replay", resolved_dir);
+    TEST_ASSERT_EQUAL_STRING(expected, out);
+}
+
+static void test_legitimate_filename_overwriting_existing_regular_file_ok(void)
+{
+    /* An existing regular file with the same name is a legitimate
+     * overwrite target (task 9.2's fopen() will truncate it), not an
+     * error. */
+    char record_dir[PATH_MAX];
+    char resolved_dir[PATH_MAX];
+    char existing[PATH_MAX];
+    char out[PATH_MAX];
+    char expected[PATH_MAX];
+    int  rc;
+
+    make_tmp_dir(record_dir, sizeof(record_dir), "_overwrite");
+    TEST_ASSERT_NOT_NULL(realpath(record_dir, resolved_dir));
+    snprintf(existing, sizeof(existing), "%s/session.replay", resolved_dir);
+    touch_file(existing);
+
+    rc = sched_validate_record_filename(record_dir, "session.replay", out,
+                                        sizeof(out));
+    TEST_ASSERT_EQUAL_INT(0, rc);
+
+    snprintf(expected, sizeof(expected), "%s/session.replay", resolved_dir);
+    TEST_ASSERT_EQUAL_STRING(expected, out);
+}
+
+static void test_out_path_buffer_too_small_returns_enametoolong(void)
+{
+    char record_dir[PATH_MAX];
+    char out[4]; /* far too small for any real resolved path */
+    int  rc;
+
+    make_tmp_dir(record_dir, sizeof(record_dir), "_small");
+
+    rc = sched_validate_record_filename(record_dir, "session.replay", out,
+                                        sizeof(out));
+    TEST_ASSERT_EQUAL_INT(-ENAMETOOLONG, rc);
+}
+
+/* ---------------------------------------------------------------------- */
+/* Symlink-escape containment -- canonical-path comparison, not textual    */
+/* prefix matching (Req 5 AC 10's core security property)                  */
+/* ---------------------------------------------------------------------- */
+
+static void test_symlink_inside_record_dir_escaping_outside_is_rejected(void)
+{
+    /* record_dir contains a symlink (named exactly like a legitimate,
+     * innocent-looking filename) whose target lives OUTSIDE record_dir.
+     * The filename component itself violates neither the '/' nor the
+     * ".." rule -- only the canonical-path containment check on the
+     * pre-existing symlink's resolved target can catch this. */
+    char record_dir[PATH_MAX];
+    char outside_dir[PATH_MAX];
+    char outside_target[PATH_MAX];
+    char link_path[PATH_MAX];
+    char out[PATH_MAX];
+    int  rc;
+
+    make_tmp_dir(record_dir, sizeof(record_dir), "_escsrc");
+    make_tmp_dir(outside_dir, sizeof(outside_dir), "_escdst");
+
+    snprintf(outside_target, sizeof(outside_target), "%s/secret", outside_dir);
+    touch_file(outside_target);
+
+    snprintf(link_path, sizeof(link_path), "%s/looks_safe", record_dir);
+    TEST_ASSERT_EQUAL_INT(0, symlink(outside_target, link_path));
+
+    rc = sched_validate_record_filename(record_dir, "looks_safe", out,
+                                        sizeof(out));
+    TEST_ASSERT_EQUAL_INT(-EACCES, rc);
+}
+
+static void test_symlink_escape_not_caught_by_naive_textual_prefix_match(void)
+{
+    /* The exact trap this task's security brief calls out by name:
+     * record_dir="<tmp>/xce" vs a SIBLING directory
+     * "<tmp>/xce-evil" -- the sibling's path textually STARTS WITH
+     * record_dir's canonical path, so a naive strncmp()-only containment
+     * check would wrongly accept it. A pre-existing symlink inside
+     * record_dir points at a file inside that sibling; a correct
+     * canonical-path comparison (requiring the byte after the prefix to
+     * be '/') must still reject it. */
+    char base[PATH_MAX];
+    char record_dir[PATH_MAX];
+    char sibling_dir[PATH_MAX];
+    char sibling_target[PATH_MAX];
+    char link_path[PATH_MAX];
+    char out[PATH_MAX];
+    int  rc;
+
+    make_tmp_dir(base, sizeof(base), "_prefixtrap");
+
+    snprintf(record_dir, sizeof(record_dir), "%s/xce", base);
+    TEST_ASSERT_EQUAL_INT(0, mkdir(record_dir, 0755));
+
+    snprintf(sibling_dir, sizeof(sibling_dir), "%s/xce-evil", base);
+    TEST_ASSERT_EQUAL_INT(0, mkdir(sibling_dir, 0755));
+
+    snprintf(sibling_target, sizeof(sibling_target), "%s/payload", sibling_dir);
+    touch_file(sibling_target);
+
+    snprintf(link_path, sizeof(link_path), "%s/trap", record_dir);
+    TEST_ASSERT_EQUAL_INT(0, symlink(sibling_target, link_path));
+
+    rc = sched_validate_record_filename(record_dir, "trap", out, sizeof(out));
+    TEST_ASSERT_EQUAL_INT(-EACCES, rc);
+}
+
+static void test_symlink_inside_record_dir_staying_contained_is_accepted(void)
+{
+    /* Sanity companion to the two rejection tests above: a symlink whose
+     * target genuinely stays within record_dir must be ACCEPTED -- proves
+     * the containment check isn't simply "reject every symlink". */
+    char record_dir[PATH_MAX];
+    char resolved_dir[PATH_MAX];
+    char real_target[PATH_MAX];
+    char link_path[PATH_MAX];
+    char out[PATH_MAX];
+    char expected[PATH_MAX];
+    int  rc;
+
+    make_tmp_dir(record_dir, sizeof(record_dir), "_inbound");
+    TEST_ASSERT_NOT_NULL(realpath(record_dir, resolved_dir));
+
+    snprintf(real_target, sizeof(real_target), "%s/real.replay", resolved_dir);
+    touch_file(real_target);
+
+    snprintf(link_path, sizeof(link_path), "%s/alias", resolved_dir);
+    TEST_ASSERT_EQUAL_INT(0, symlink(real_target, link_path));
+
+    rc = sched_validate_record_filename(record_dir, "alias", out, sizeof(out));
+    TEST_ASSERT_EQUAL_INT(0, rc);
+
+    snprintf(expected, sizeof(expected), "%s/alias", resolved_dir);
+    TEST_ASSERT_EQUAL_STRING(expected, out);
+}
+
+/* ---------------------------------------------------------------------- */
+/* Hard-link inode-aliasing escape -- CWE-59 link-following via a hard     */
+/* link rather than a symlink (Req 5 AC 10's core security property)       */
+/* ---------------------------------------------------------------------- */
+
+static void test_hardlink_inside_record_dir_aliasing_outside_is_rejected(void)
+{
+    /* record_dir contains a REGULAR file (named exactly like a legitimate,
+     * innocent-looking filename) that is actually a hard link to a victim
+     * file OUTSIDE record_dir -- same inode, no separate "link target"
+     * path for realpath()/lstat() to resolve and contain-check the way
+     * there is for a symlink (mirrors
+     * test_symlink_inside_record_dir_escaping_outside_is_rejected above,
+     * substituting link() for symlink()). The filename component itself
+     * violates neither the '/' nor the ".." rule, and the entry IS
+     * S_ISREG and canonically inside record_dir -- only the st_nlink > 1
+     * check can catch this. This exercises real link(2)/lstat(2) kernel
+     * behavior (no mocking): a genuine hard link is created on disk and
+     * genuinely shares the victim's inode, so if this test passes only
+     * because the fix is in place, the hard link IS a real bypass on this
+     * platform absent that fix (proven by the RED phase below). */
+    char record_dir[PATH_MAX];
+    char outside_dir[PATH_MAX];
+    char victim[PATH_MAX];
+    char aliased_path[PATH_MAX];
+    char out[PATH_MAX];
+    int  rc;
+
+    make_tmp_dir(record_dir, sizeof(record_dir), "_hlsrc");
+    make_tmp_dir(outside_dir, sizeof(outside_dir), "_hldst");
+
+    snprintf(victim, sizeof(victim), "%s/victim", outside_dir);
+    touch_file(victim);
+
+    snprintf(aliased_path, sizeof(aliased_path), "%s/looks_safe", record_dir);
+    TEST_ASSERT_EQUAL_INT(0, link(victim, aliased_path));
+
+    rc = sched_validate_record_filename(record_dir, "looks_safe", out,
+                                        sizeof(out));
+    TEST_ASSERT_EQUAL_INT(-EACCES, rc);
+}
+
+static void test_symlink_to_hardlinked_alias_inside_record_dir_is_rejected(void)
+{
+    /* One extra hop beyond both escape tests above: record_dir contains a
+     * REGULAR file ("internal") that is a hard link to a victim file
+     * OUTSIDE record_dir (mirrors
+     * test_hardlink_inside_record_dir_aliasing_outside_is_rejected), PLUS a
+     * SYMLINK ("alias") inside record_dir pointing at that hard-linked
+     * "internal" entry (mirrors
+     * test_symlink_inside_record_dir_escaping_outside_is_rejected's shape).
+     * Requesting "internal" directly is already caught by the direct-entry
+     * st_nlink > 1 check. Requesting "alias" must ALSO be rejected: the
+     * S_ISLNK branch's realpath()/path_is_contained() call only confirms
+     * the resolved target is textually/canonically inside record_dir --
+     * "internal" IS inside record_dir, so containment alone is not enough.
+     * The resolved target must ALSO be checked for st_nlink > 1, exactly
+     * like the direct-entry branch does, or fopen()-ing the returned
+     * out_path follows the symlink into the aliased outside inode and
+     * corrupts the victim file -- same CWE-59 impact, reached via one
+     * extra symlink hop. */
+    char record_dir[PATH_MAX];
+    char outside_dir[PATH_MAX];
+    char victim[PATH_MAX];
+    char internal_path[PATH_MAX];
+    char alias_path[PATH_MAX];
+    char out[PATH_MAX];
+    int  rc;
+
+    make_tmp_dir(record_dir, sizeof(record_dir), "_hlaliassrc");
+    make_tmp_dir(outside_dir, sizeof(outside_dir), "_hlaliasdst");
+
+    snprintf(victim, sizeof(victim), "%s/victim", outside_dir);
+    touch_file(victim);
+
+    snprintf(internal_path, sizeof(internal_path), "%s/internal", record_dir);
+    TEST_ASSERT_EQUAL_INT(0, link(victim, internal_path));
+
+    snprintf(alias_path, sizeof(alias_path), "%s/alias", record_dir);
+    TEST_ASSERT_EQUAL_INT(0, symlink(internal_path, alias_path));
+
+    rc = sched_validate_record_filename(record_dir, "alias", out, sizeof(out));
+    TEST_ASSERT_EQUAL_INT(-EACCES, rc);
+}
+
+/* ---------------------------------------------------------------------- */
+/* Defensive: existing non-regular, non-symlink entry is rejected          */
+/* ---------------------------------------------------------------------- */
+
+static void test_existing_directory_with_same_name_rejected(void)
+{
+    char record_dir[PATH_MAX];
+    char subdir[PATH_MAX];
+    char out[PATH_MAX];
+    int  rc;
+
+    make_tmp_dir(record_dir, sizeof(record_dir), "_subdir");
+    snprintf(subdir, sizeof(subdir), "%s/oops", record_dir);
+    TEST_ASSERT_EQUAL_INT(0, mkdir(subdir, 0755));
+
+    rc = sched_validate_record_filename(record_dir, "oops", out, sizeof(out));
+    TEST_ASSERT_EQUAL_INT(-EEXIST, rc);
+}
+
+int main(void)
+{
+    UNITY_BEGIN();
+
+    RUN_TEST(test_null_record_dir_returns_einval);
+    RUN_TEST(test_null_filename_returns_einval);
+    RUN_TEST(test_null_out_path_returns_einval);
+    RUN_TEST(test_zero_out_path_len_returns_einval);
+    RUN_TEST(test_empty_filename_returns_einval);
+
+    RUN_TEST(test_filename_with_slash_rejected_before_filesystem_access);
+    RUN_TEST(test_dotdot_all_positions_rejected_before_filesystem_access);
+
+    RUN_TEST(test_nonexistent_record_dir_with_valid_filename_returns_error);
+    RUN_TEST(test_record_dir_that_is_a_file_returns_enotdir);
+
+    RUN_TEST(test_legitimate_bare_filename_resolves_inside_record_dir);
+    RUN_TEST(test_legitimate_filename_overwriting_existing_regular_file_ok);
+    RUN_TEST(test_out_path_buffer_too_small_returns_enametoolong);
+
+    RUN_TEST(test_symlink_inside_record_dir_escaping_outside_is_rejected);
+    RUN_TEST(test_symlink_escape_not_caught_by_naive_textual_prefix_match);
+    RUN_TEST(test_symlink_inside_record_dir_staying_contained_is_accepted);
+
+    RUN_TEST(test_hardlink_inside_record_dir_aliasing_outside_is_rejected);
+    RUN_TEST(test_symlink_to_hardlinked_alias_inside_record_dir_is_rejected);
+
+    RUN_TEST(test_existing_directory_with_same_name_rejected);
+
+    return UNITY_END();
+}