From 79a5734112d58523f3632403515f4d1fa940f127 Mon Sep 17 00:00:00 2001 From: Tanishq Date: Tue, 3 Nov 2026 07:32:40 +0530 Subject: [PATCH] fix(pipeline): skip a file named nul in the control-file walk instead of aborting the index (#2646) semantic_manifest_control_entry() reads the metadata of every entry the control-file walk lists, and one failed read fails the whole manifest. On Win32 the plain path \nul names the NUL device, so a real file named nul (Git Bash leaves one behind after `cmd > nul` or `2>nul`) cannot be read: GetFileAttributesExW fails with ERROR_INVALID_PARAMETER. The manifest returns CBM_NOT_FOUND and every index run on the repository ends in CBM_PIPELINE_ABORT_PRESERVE_DB (aborted_previous_preserved). Listing nul in .gitignore does not help, because the walk visits every entry. Only directories and control files can contribute to the manifest. Decide the control name first and skip an entry that the listing already shows to be neither, without reading it. A Windows listing always carries the attributes; on POSIX an entry whose d_type is DT_UNKNOWN is still read. Directories and control-named entries are read exactly as before. Signed-off-by: Tanishq --- src/pipeline/pipeline_incremental.c | 51 +++++++++---- tests/test_pipeline.c | 106 ++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 13 deletions(-) diff --git a/src/pipeline/pipeline_incremental.c b/src/pipeline/pipeline_incremental.c index ac2c253af..3bf321107 100644 --- a/src/pipeline/pipeline_incremental.c +++ b/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 .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 .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; diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index 1958fdb5e..58c6d796d 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -9657,6 +9657,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 /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. */ @@ -31994,6 +32096,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);