fix(pipeline): create the publication staging file exclusively

cbm_pipeline_publish_generation built its staging database name by hand as
"<db>.stage.<pid>.<counter>", unlinked it, then wrote it. Any local process
can compute that name in advance, so a symlink planted between the unlink
and the write redirects the write to a target of the attacker's choosing —
an arbitrary-file clobber when the database sits in a world-writable
directory.

The same file already solves this correctly elsewhere: create_staging_path()
mints the name with mkstemp, so the file is created O_EXCL and we only ever
write one we made ourselves. Publication now shares it. The unlink-first
step goes away with the predictable name — it existed to clear a leftover at
a name we might reuse, and a freshly minted name cannot collide, nor can its
sidecars pre-exist.

SCOPE, stated precisely because the PR description overstates it: the only
caller of cbm_pipeline_publish_generation sits behind
CBM_INCREMENTAL_TEST_API, which is set in CFLAGS_TEST and never in
CFLAGS_PROD. The predictable name was therefore not reachable in a shipped
binary — production publication already went through create_staging_path.
This is removing a bad pattern from a test-only path before it can be
promoted, not patching a live user-facing vulnerability.

The regression test calls the function directly, because no pipeline entry
point reaches it in a production build. It does not try to win the race — a
test that has to win a race is a coin flip, not a gate. It asserts the
property that removes the race: canaries occupy every name the old scheme
could have chosen and all must survive publication. Verified both ways
rather than green-only: against the old code it reports "survived == 31,
expected PREDICTABLE_CANARIES == 32", exactly one canary consumed; with the
fix the suite goes 18 passed/1 failed -> 19 passed.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>

src/pipeline/pipeline.c

@@ -1429,7 +1429,9 @@ int cbm_pipeline_refresh_artifact(cbm_pipeline_t *p, const char *db_path) {
     return 0;
 }
 
-static atomic_ullong g_generation_stage_counter = 0;
+
+/* Defined below, next to the other publication helpers. */
+static char *create_staging_path(const char *final_path);
 
 static void discard_generation_stage(const char *stage_path) {
     if (!stage_path) {
@@ -1598,19 +1600,21 @@ int cbm_pipeline_publish_generation(const cbm_pipeline_generation_t *generation)
         return CBM_PIPELINE_ABORT_PRESERVE_DB;
     }
 
-    size_t stage_size = strlen(generation->final_db_path) + 96;
-    char *stage_path = malloc(stage_size);
+    /* The staging name must be unpredictable and created exclusively. It used
+     * to be "<db>.stage.<pid>.<counter>", which any other process can compute
+     * in advance; this path is then unlinked and written, so in a
+     * world-writable database directory an attacker could land a symlink in
+     * the window between the two and have us clobber the target. Sharing the
+     * mkstemp-based helper the other staging site already uses closes that:
+     * O_EXCL creation means we only ever write a file we made ourselves.
+     *
+     * The old unlink-first step goes with it. It existed to clear a leftover
+     * file at a name we might reuse; a freshly minted name cannot collide,
+     * and its sidecars cannot pre-exist either. */
+    char *stage_path = create_staging_path(generation->final_db_path);
     if (!stage_path) {
         return CBM_PIPELINE_PERSIST_FAILED;
     }
-    unsigned long long serial = atomic_fetch_add(&g_generation_stage_counter, 1) + 1;
-    int path_n = snprintf(stage_path, stage_size, "%s.stage.%ld.%llu", generation->final_db_path,
-                          (long)cbm_pipeline_getpid(), serial);
-    if (path_n < 0 || (size_t)path_n >= stage_size) {
-        free(stage_path);
-        return CBM_PIPELINE_PERSIST_FAILED;
-    }
-    discard_generation_stage(stage_path);
 
     int dump_rc = cbm_gbuf_dump_to_sqlite(generation->gbuf, stage_path);
     if (dump_rc != 0) {

tests/test_pipeline.c

@@ -2580,6 +2580,78 @@ TEST(pipeline_source_mutation_before_publication_preserves_previous_generation)
     PASS();
 }
 
+/* Publication must never write through a name another local process can
+ * predict. The staging database used to be "<db>.stage.<pid>.<counter>",
+ * which anyone can compute in advance; publication then unlinked that name
+ * and wrote it, so a symlink planted in the gap redirects the write to a
+ * target of the attacker's choosing.
+ *
+ * Winning that timing window is not something a test should attempt -- a
+ * test that has to win a race is a coin flip, not a gate. The property that
+ * REMOVES the window is deterministic: publication must not touch any
+ * predictable name at all. Canaries occupy every name the old scheme could
+ * have picked, and all must survive. Against the old code exactly one is
+ * consumed, whichever serial the counter had reached.
+ *
+ * This calls cbm_pipeline_publish_generation directly. The only in-tree
+ * caller sits behind CBM_INCREMENTAL_TEST_API, so going through the pipeline
+ * would never reach the code under test. */
+TEST(pipeline_publication_never_uses_a_predictable_staging_path) {
+    char tmp[256];
+    snprintf(tmp, sizeof(tmp), "/tmp/cbm_publish_predictable_stage_XXXXXX");
+    ASSERT_NOT_NULL(cbm_mkdtemp(tmp));
+    char db_path[512];
+    snprintf(db_path, sizeof(db_path), "%s/generation.db", tmp);
+
+    enum { PREDICTABLE_CANARIES = 32 };
+    static const char canary[] = "canary-must-survive\n";
+    char canary_path[PREDICTABLE_CANARIES][640];
+    for (int i = 0; i < PREDICTABLE_CANARIES; i++) {
+        snprintf(canary_path[i], sizeof(canary_path[i]), "%s.stage.%ld.%d", db_path,
+                 (long)getpid(), i + 1);
+        ASSERT_EQ(th_write_file(canary_path[i], canary), 0);
+    }
+
+    cbm_gbuf_t *gb = cbm_gbuf_new("predictable-stage-proj", tmp);
+    ASSERT_NOT_NULL(gb);
+    cbm_pipeline_generation_t generation = {
+        .gbuf = gb,
+        .final_db_path = db_path,
+        .project = "predictable-stage-proj",
+        .cancelled = NULL,
+        .manifest = NULL,
+        .manifest_count = 0,
+        .adr_content = NULL,
+        .coverage = NULL,
+        .coverage_count = 0,
+    };
+    int publish_rc = cbm_pipeline_publish_generation(&generation);
+    cbm_gbuf_free(gb);
+
+    int survived = 0;
+    int intact = 0;
+    for (int i = 0; i < PREDICTABLE_CANARIES; i++) {
+        FILE *f = cbm_fopen(canary_path[i], "rb");
+        if (!f) {
+            continue;
+        }
+        survived++;
+        char buf[64] = {0};
+        size_t n = fread(buf, 1, sizeof(buf) - 1, f);
+        (void)fclose(f);
+        if (n == strlen(canary) && memcmp(buf, canary, n) == 0) {
+            intact++;
+        }
+    }
+    th_rmtree(tmp);
+
+    ASSERT_EQ(publish_rc, 0);
+    /* Not one may be unlinked, truncated, or written through. */
+    ASSERT_EQ(survived, PREDICTABLE_CANARIES);
+    ASSERT_EQ(intact, PREDICTABLE_CANARIES);
+    PASS();
+}
+
 /* Discovery and extraction must describe the same immutable generation. A
  * source file created after extraction is not present in the original file
  * list, so merely re-hashing that list cannot detect the race. Publication
@@ -11434,6 +11506,7 @@ SUITE(pipeline_semantic_manifest_repro) {
 #if defined(CBM_INCREMENTAL_TEST_API) && CBM_INCREMENTAL_TEST_API
     RUN_TEST(pipeline_git_context_change_forces_full_and_refreshes_branch);
     RUN_TEST(pipeline_global_extension_config_change_forces_full);
+    RUN_TEST(pipeline_publication_never_uses_a_predictable_staging_path);
     RUN_TEST(pipeline_source_mutation_before_publication_preserves_previous_generation);
     RUN_TEST(pipeline_source_addition_before_publication_preserves_previous_generation);
     RUN_TEST(pipeline_tsconfig_mutation_before_publication_preserves_previous_generation);