Merge pull request DeusData#2903 from tanishqbaweja/fix/manifest-walk-nul-file

fix(pipeline): skip a file named nul in the control-file walk instead of aborting the index (DeusData#2646)

Author: DeusData

src/pipeline/pipeline_incremental.c

@@ -379,10 +379,37 @@ static int semantic_manifest_walk_controls(semantic_manifest_builder_t *builder,
                                            const char *rel_dir, int depth, char **excluded_dirs,
                                            int excluded_count);
 
+/* Control files are the only regular files the walk records. */
+static bool semantic_manifest_control_name(const char *rel_dir, const char *name) {
+    bool root_control = (!rel_dir || !rel_dir[0]) && (strcmp(name, ".cbmignore") == 0 ||
+                                                      strcmp(name, ".codebase-memory.json") == 0);
+    /* A build context's ignore file (or BuildKit's <Dockerfile>.dockerignore)
+     * shapes which files an image holds (pass_docker.c image_ignore), and
+     * discovery indexes neither. */
+    static const char DOCKERIGNORE[] = ".dockerignore";
+    size_t nl = strlen(name);
+    bool dockerignore = nl >= sizeof(DOCKERIGNORE) - SKIP_ONE &&
+                        strcmp(name + nl - (sizeof(DOCKERIGNORE) - SKIP_ONE), DOCKERIGNORE) == 0;
+    return strcmp(name, ".gitignore") == 0 || root_control ||
+           semantic_manifest_package_control(name) || dockerignore;
+}
+
+/* True when the directory listing alone shows that the entry is not a
+ * directory. A Windows listing always carries the attributes; on POSIX a
+ * d_type of DT_UNKNOWN (0), which readdir could not resolve, proves nothing. */
+static bool semantic_manifest_listed_non_directory(const cbm_dirent_t *entry) {
+#ifdef _WIN32
+    return !entry->is_dir;
+#else
+    return entry->d_type != 0 && !entry->is_dir;
+#endif
+}
+
 static int semantic_manifest_control_entry(semantic_manifest_builder_t *builder,
                                            const char *project, const char *abs_dir,
                                            const char *rel_dir, int depth, char **excluded_dirs,
-                                           int excluded_count, const char *name) {
+                                           int excluded_count, const cbm_dirent_t *entry) {
+    const char *name = entry->name;
     if (strcmp(name, ".") == 0 || strcmp(name, "..") == 0)
         return 0;
     char abs_path[CBM_SZ_4K], rel_path[CBM_SZ_4K];
@@ -392,6 +419,15 @@ static int semantic_manifest_control_entry(semantic_manifest_builder_t *builder,
     if (abs_n < 0 || (size_t)abs_n >= sizeof(abs_path) || rel_n < 0 ||
         (size_t)rel_n >= sizeof(rel_path))
         return CBM_NOT_FOUND;
+    /* Only directories and control files contribute, so an entry the listing
+     * already shows to be neither is skipped without reading its metadata.
+     * That read can fail on such an entry: on Windows the plain path of a file
+     * named nul names the NUL device (#2646), and the plain path of a name
+     * ending in a dot or a space does not reach the file. */
+    bool control = semantic_manifest_control_name(rel_dir, name);
+    if (!control && semantic_manifest_listed_non_directory(entry)) {
+        return 0;
+    }
     cbm_path_info_t info;
     if (cbm_path_info_utf8(abs_path, &info) != 0)
         return CBM_NOT_FOUND;
@@ -404,17 +440,6 @@ static int semantic_manifest_control_entry(semantic_manifest_builder_t *builder,
         return semantic_manifest_walk_controls(builder, project, abs_path, rel_path, depth + 1,
                                                excluded_dirs, excluded_count);
     }
-    bool root_control = (!rel_dir || !rel_dir[0]) && (strcmp(name, ".cbmignore") == 0 ||
-                                                      strcmp(name, ".codebase-memory.json") == 0);
-    /* A build context's ignore file (or BuildKit's <Dockerfile>.dockerignore)
-     * shapes which files an image holds (pass_docker.c image_ignore), and
-     * discovery indexes neither. */
-    static const char DOCKERIGNORE[] = ".dockerignore";
-    size_t nl = strlen(name);
-    bool dockerignore = nl >= sizeof(DOCKERIGNORE) - SKIP_ONE &&
-                        strcmp(name + nl - (sizeof(DOCKERIGNORE) - SKIP_ONE), DOCKERIGNORE) == 0;
-    bool control = strcmp(name, ".gitignore") == 0 || root_control ||
-                   semantic_manifest_package_control(name) || dockerignore;
     return info.is_regular && control ? semantic_manifest_add(builder, project, rel_path, abs_path)
                                       : 0;
 }
@@ -439,7 +464,7 @@ static int semantic_manifest_walk_controls(semantic_manifest_builder_t *builder,
         rc = cbm_pipeline_frozen_checkpoint(builder->frozen);
         if (rc == 0)
             rc = semantic_manifest_control_entry(builder, project, abs_dir, rel_dir, depth,
-                                                 excluded_dirs, excluded_count, entry->name);
+                                                 excluded_dirs, excluded_count, entry);
     }
     cbm_closedir(dir);
     return rc;

tests/test_pipeline.c

@@ -9708,6 +9708,108 @@ TEST(pipeline_semantic_manifest_skips_unreadable_subdirectory) {
 }
 #endif /* !_WIN32 */
 
+#ifdef _WIN32
+/* Create or delete a real file named nul in dir, as a Git Bash redirect such
+ * as `cmd > nul` leaves behind (#2646). The plain Win32 path <dir>/nul names
+ * the NUL device, so the file is reached only through the extended-length
+ * namespace, which needs an absolute path with backslashes (CI's TEMP uses
+ * forward slashes). */
+static bool pipeline_test_nul_file(const char *dir, bool create) {
+    enum { NUL_PATH_CAP = 1024, PREFIX_LEN = 4, LEAF_LEN = 4 };
+    static const wchar_t prefix[] = L"\\\\?\\";
+    static const wchar_t leaf[] = L"\\nul";
+    wchar_t path[NUL_PATH_CAP];
+    const DWORD room = NUL_PATH_CAP - PREFIX_LEN - LEAF_LEN;
+    wchar_t *wide_dir = cbm_utf8_to_wide(dir);
+    DWORD length = wide_dir ? GetFullPathNameW(wide_dir, room, path + PREFIX_LEN, NULL) : 0;
+    free(wide_dir);
+    if (length == 0 || length >= room) {
+        return false;
+    }
+    wmemcpy(path, prefix, PREFIX_LEN);
+    wmemcpy(path + PREFIX_LEN + length, leaf, LEAF_LEN + 1);
+    if (!create) {
+        return DeleteFileW(path) != 0;
+    }
+    HANDLE file =
+        CreateFileW(path, GENERIC_WRITE, 0, NULL, CREATE_NEW, FILE_ATTRIBUTE_NORMAL, NULL);
+    if (file == INVALID_HANDLE_VALUE) {
+        return false;
+    }
+    CloseHandle(file);
+    return true;
+}
+
+/* #2646: the control-file walk cannot inspect a file named nul through the
+ * plain path, and it never needs to: only directories and control files can
+ * contribute to the manifest. Listing nul in .gitignore does not help, since
+ * the walk visits every entry. */
+TEST(pipeline_semantic_manifest_skips_nul_device_file_issue2646) {
+    char tmp[256];
+    snprintf(tmp, sizeof(tmp), "/tmp/cbm_manifest_nul_XXXXXX");
+    ASSERT_NOT_NULL(cbm_mkdtemp(tmp));
+    write_temp_file(tmp, "app.py", "def NulNeighbor():\n    return 1\n");
+    write_temp_file(tmp, ".gitignore", "nul\n");
+    bool created = pipeline_test_nul_file(tmp, true);
+
+    cbm_file_hash_t *manifest = NULL;
+    int manifest_count = -1;
+    int rc = cbm_pipeline_build_semantic_manifest(NULL, "manifest-skip-nul", tmp, NULL, 0, NULL, 0,
+                                                  NULL, NULL, &manifest, &manifest_count);
+    bool found_gitignore = false;
+    for (int i = 0; i < manifest_count; i++) {
+        if (manifest[i].rel_path && strcmp(manifest[i].rel_path, ".gitignore") == 0) {
+            found_gitignore = true;
+        }
+    }
+    cbm_pipeline_free_semantic_manifest(manifest, manifest_count > 0 ? manifest_count : 0);
+    bool deleted = created && pipeline_test_nul_file(tmp, false);
+    th_rmtree(tmp);
+
+    ASSERT_TRUE(created);
+    ASSERT_EQ(rc, 0);
+    ASSERT_TRUE(found_gitignore);
+    ASSERT_TRUE(deleted);
+    PASS();
+}
+
+/* The user-visible half of #2646: one such file made every index run return
+ * CBM_PIPELINE_ABORT_PRESERVE_DB (aborted_previous_preserved), so a new
+ * project never got an index. */
+TEST(pipeline_full_index_survives_nul_device_file_issue2646) {
+    char tmp[256];
+    snprintf(tmp, sizeof(tmp), "/tmp/cbm_index_nul_XXXXXX");
+    ASSERT_NOT_NULL(cbm_mkdtemp(tmp));
+    write_temp_file(tmp, "app.py", "def NulNeighbor():\n    return 1\n");
+    write_temp_file(tmp, ".gitignore", "nul\n");
+    bool created = pipeline_test_nul_file(tmp, true);
+
+    char db_path[512];
+    snprintf(db_path, sizeof(db_path), "%s/generation.db", tmp);
+    cbm_pipeline_t *pipeline = cbm_pipeline_new(tmp, db_path, CBM_MODE_FULL);
+    int run_rc = pipeline ? cbm_pipeline_run(pipeline) : -1;
+    char project[256] = "";
+    if (pipeline) {
+        snprintf(project, sizeof(project), "%s", cbm_pipeline_project_name(pipeline));
+        cbm_pipeline_free(pipeline);
+    }
+    int indexed = -1;
+    cbm_store_t *store = cbm_store_open_path_query(db_path);
+    if (store) {
+        indexed = named_node_count(store, project, "NulNeighbor");
+        cbm_store_close(store);
+    }
+    bool deleted = created && pipeline_test_nul_file(tmp, false);
+    th_rmtree(tmp);
+
+    ASSERT_TRUE(created);
+    ASSERT_EQ(run_rc, 0);
+    ASSERT_EQ(indexed, 1);
+    ASSERT_TRUE(deleted);
+    PASS();
+}
+#endif /* _WIN32 */
+
 /* A fully validated staged graph must be able to recover from a definitely
  * non-SQLite destination without deleting evidence or overwriting an earlier
  * quarantine. The replacement happens only after the corrupt bytes are moved. */
@@ -32362,6 +32464,10 @@ SUITE(pipeline_semantic_manifest_repro) {
     RUN_TEST(pipeline_semantic_manifest_rejects_non_directory_root);
 #ifndef _WIN32
     RUN_TEST(pipeline_semantic_manifest_skips_unreadable_subdirectory);
+#endif
+#ifdef _WIN32
+    RUN_TEST(pipeline_semantic_manifest_skips_nul_device_file_issue2646);
+    RUN_TEST(pipeline_full_index_survives_nul_device_file_issue2646);
 #endif
     RUN_TEST(pipeline_full_reindex_quarantines_corrupt_destination_without_overwrite);
     RUN_TEST(pipeline_full_reindex_replaces_legacy_schema_without_quarantine);