From 38d422f0303a3e91875a0ffb7396783f0b5caaea Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 20 Sep 2026 17:49:27 +0200 Subject: [PATCH] fix(routes): a document or log path is not an HTTP route literal Distilled from #1245 by Andrew Hundt (a slice of 0370d52f). detect_url_in_args in the parallel resolver runs for every resolved call with no callee gating. A first argument like "/new/file.txt" clears normalize_url_arg (it has a second slash and no whitespace) and then reaches cbm_service_pattern_is_http_route_literal, whose filesystem extension list knew about config, key and socket files but not about documents or logs. So open_file("/new/file.txt") read_file("/docs/guide.md") write_file("/data/app.log") each minted a Route node and an HTTP_CALLS edge -- the graph reported a web endpoint where the program opens a file. Five extensions join the list: .log .md .pdf .rst .txt, alphabetical order preserved. Two things the tests are careful about, because the obvious fixture would not have been RED. A bare Python builtin open(...) never resolves to an indexed target, so it never reaches the URL detector at all; the bug shows only for calls that resolve to a project symbol, which is what the fixture defines. And /var/... is already rejected by the filesystem ROOT list, so the fixture paths start with segments that list does not know, leaving the extension guard as the only thing that can reject them. RED before the fix: infrascan_http_route_literal_guard_rejects_document_extensions FAIL tests/test_infrascan.c:44: ASSERT(!(cbm_service_pattern_is_http_route_literal("/new/file.txt", "open"))) pipeline_arg_url_rejects_document_file_paths FAIL tests/test_pipeline.c:6652: count_nodes_named(s, project, "/new/file.txt") == 1, expected 0 == 0 The pipeline test keeps a positive control above that line -- a genuine requests.get("/api/items") still yields its Route node and HTTP_CALLS edge -- so the guard cannot pass by rejecting everything. GREEN after: infrascan 4 passed; pipeline 282 passed; extraction edge_types_probe route_canon 420 passed. Co-authored-by: Andrew Hundt Signed-off-by: Martin Vogel --- internal/cbm/service_patterns.c | 6 +-- tests/test_infrascan.c | 20 +++++++ tests/test_pipeline.c | 95 +++++++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+), 3 deletions(-) diff --git a/internal/cbm/service_patterns.c b/internal/cbm/service_patterns.c index c2425e09e1..2963b44c0c 100644 --- a/internal/cbm/service_patterns.c +++ b/internal/cbm/service_patterns.c @@ -752,9 +752,9 @@ static bool has_filesystem_extension(const char *path) { ext[ext_len] = '\0'; static const char *const hard_file_exts[] = { - ".cfg", ".conf", ".credentials", ".crt", ".db", ".env", - ".ini", ".key", ".pem", ".pid", ".properties", ".service", - ".sock", ".socket", ".sqlite", ".toml", NULL}; + ".cfg", ".conf", ".credentials", ".crt", ".db", ".env", ".ini", ".key", + ".log", ".md", ".pdf", ".pem", ".pid", ".properties", ".rst", ".service", + ".sock", ".socket", ".sqlite", ".toml", ".txt", NULL}; for (int i = 0; hard_file_exts[i]; i++) { if (path_ext_matches(ext, hard_file_exts[i])) { return true; diff --git a/tests/test_infrascan.c b/tests/test_infrascan.c index a9bf1aef94..4ae8a4e426 100644 --- a/tests/test_infrascan.c +++ b/tests/test_infrascan.c @@ -225,6 +225,25 @@ TEST(infrascan_service_pattern_match_keeps_real_library_qns) { PASS(); } +/* Document/log extensions are never HTTP routes (distilled from PR #1245): + * the arg-URL detector runs for every resolved call, so without the + * extension guard `open("/new/file.txt")` minted a Route + HTTP_CALLS. + * First segments deliberately avoid the filesystem-root list so ONLY the + * extension guard can reject them; `/api/...` marker paths stay routes. */ +TEST(infrascan_http_route_literal_guard_rejects_document_extensions) { + ASSERT_FALSE(cbm_service_pattern_is_http_route_literal("/new/file.txt", "open")); + ASSERT_FALSE(cbm_service_pattern_is_http_route_literal("/docs/guide.md", "read")); + ASSERT_FALSE(cbm_service_pattern_is_http_route_literal("/data/app.log", "write")); + ASSERT_FALSE(cbm_service_pattern_is_http_route_literal("/fake/path.pdf", "open")); + ASSERT_FALSE(cbm_service_pattern_is_http_route_literal("/docs/index.rst", "open")); + /* Same extensions under a query string are still files. */ + ASSERT_FALSE(cbm_service_pattern_is_http_route_literal("/docs/guide.md?raw=1", "open")); + /* Positive controls: extension-less routes and the same callees. */ + ASSERT_TRUE(cbm_service_pattern_is_http_route_literal("/api/items", "requests.get")); + ASSERT_TRUE(cbm_service_pattern_is_http_route_literal("/docs/guide", "open")); + PASS(); +} + TEST(infrascan_route_nodes_skip_bad_http_url_paths) { cbm_gbuf_t *gb = cbm_gbuf_new("test", "/tmp/cbm_infrascan_route_guard"); ASSERT_NOT_NULL(gb); @@ -305,6 +324,7 @@ TEST(infrascan_http_calls_join_matching_handler_route) { SUITE(infrascan) { RUN_TEST(infrascan_http_route_literal_guard_rejects_filesystem_paths); + RUN_TEST(infrascan_http_route_literal_guard_rejects_document_extensions); RUN_TEST(infrascan_service_pattern_match_rejects_ids_inside_words); RUN_TEST(infrascan_service_pattern_match_keeps_real_library_qns); RUN_TEST(infrascan_route_nodes_skip_bad_http_url_paths); diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index 56be6b88e3..3cd4ae641f 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -7517,6 +7517,100 @@ TEST(pipeline_ts_config_object_url_http_calls_issue2235) { PASS(); } +/* Count HTTP_CALLS-style edges whose url_path property contains `keyword`. */ +static int count_edges_with_url_path(cbm_store_t *s, const char *project, const char *keyword) { + cbm_edge_t *es = NULL; + int n = 0; + cbm_store_find_edges_by_url_path(s, project, keyword, &es, &n); + if (es) { + cbm_store_free_edges(es, n); + } + return n; +} + +/* Distilled from PR #1245: detect_url_in_args (parallel resolver, >= 50 files) + * runs for EVERY resolved call with no callee gating, so a filesystem path + * with a document/log extension passed to a project-defined open/read/write + * helper passed normalize_url_arg (second slash, no space) and the route + * literal guard, minting a Route node + an HTTP_CALLS edge for a file. The + * callees are project functions on purpose: a bare builtin `open(...)` never + * resolves to a target and so never reaches emit_service_edge at all. The + * first path segments avoid the filesystem-root list (etc/var/tmp/...) so + * only the extension guard can reject them. requests.get('/api/items') is + * the positive control that must keep its Route + HTTP_CALLS. */ +TEST(pipeline_arg_url_rejects_document_file_paths) { + char tmp[256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_arg_url_docs_XXXXXX"); + if (!cbm_mkdtemp(tmp)) { + FAIL("tmpdir"); + } + + write_temp_file(tmp, "src/files.py", + "import requests\n" + "def open_file(path):\n" + " return path\n" + "def read_file(path):\n" + " return path\n" + "def write_file(path):\n" + " return path\n" + "def load_docs():\n" + " open_file('/new/file.txt')\n" + " read_file('/docs/guide.md')\n" + " write_file('/data/app.log')\n" + " read_file('/reports/summary.pdf')\n" + " open_file('/docs/index.rst')\n" + "def fetch_items():\n" + " return requests.get('/api/items')\n"); + for (int i = 0; i < 52; i++) { + char name[64]; + char body[128]; + snprintf(name, sizeof(name), "src/filler%d.ts", i); + snprintf(body, sizeof(body), "export function filler%d(): number { return %d; }\n", i, i); + write_temp_file(tmp, name, body); + } + + char *old_workers = getenv("CBM_WORKERS"); + char *saved = old_workers ? strdup(old_workers) : NULL; + cbm_setenv("CBM_WORKERS", "4", 1); + + char db_path[512]; + snprintf(db_path, sizeof(db_path), "%s/arg_url_docs.db", tmp); + cbm_pipeline_t *p = cbm_pipeline_new(tmp, db_path, CBM_MODE_FULL); + ASSERT_NOT_NULL(p); + ASSERT_EQ(cbm_pipeline_run(p), 0); + const char *project = cbm_pipeline_project_name(p); + + cbm_store_t *s = cbm_store_open_path(db_path); + ASSERT_NOT_NULL(s); + + /* Positive control first: the genuine HTTP call still mints its route. */ + ASSERT_GTE(count_nodes_named(s, project, "/api/items"), 1); + ASSERT_GTE(count_edges_with_url_path(s, project, "/api/items"), 1); + /* No Route node for any filesystem document path... */ + ASSERT_EQ(count_nodes_named(s, project, "/new/file.txt"), 0); + ASSERT_EQ(count_nodes_named(s, project, "/docs/guide.md"), 0); + ASSERT_EQ(count_nodes_named(s, project, "/data/app.log"), 0); + ASSERT_EQ(count_nodes_named(s, project, "/reports/summary.pdf"), 0); + ASSERT_EQ(count_nodes_named(s, project, "/docs/index.rst"), 0); + /* ...and no HTTP_CALLS edge carrying one as its url_path either. */ + ASSERT_EQ(count_edges_with_url_path(s, project, "/new/file.txt"), 0); + ASSERT_EQ(count_edges_with_url_path(s, project, "/docs/guide.md"), 0); + ASSERT_EQ(count_edges_with_url_path(s, project, "/data/app.log"), 0); + ASSERT_EQ(count_edges_with_url_path(s, project, "/reports/summary.pdf"), 0); + ASSERT_EQ(count_edges_with_url_path(s, project, "/docs/index.rst"), 0); + + cbm_store_close(s); + cbm_pipeline_free(p); + if (saved) { + cbm_setenv("CBM_WORKERS", saved, 1); + free(saved); + } else { + cbm_unsetenv("CBM_WORKERS"); + } + th_rmtree(tmp); + PASS(); +} + /* Native `fetch()` (#856), sequential path (< 50 files → pass_calls.c). A bare * unqualified call to the global fetch API has no import and no local * definition anywhere in this project, so registry resolution comes back @@ -16360,6 +16454,7 @@ SUITE(pipeline) { RUN_TEST(pipeline_parallel_rust_cross_only_macro_hidden_gets_synthetic_carrier); RUN_TEST(pipeline_arg_url_rejects_non_http_slash_arguments); RUN_TEST(pipeline_ts_config_object_url_http_calls_issue2235); + RUN_TEST(pipeline_arg_url_rejects_document_file_paths); RUN_TEST(pipeline_native_fetch_classified_as_http_calls); RUN_TEST(pipeline_swift_nested_url_makes_route_issue1892); RUN_TEST(pipeline_swift_http_call_makes_route_issue1892);