From 6eeb8d9dbae9dd04b1dde5842871f18ab4c7ff22 Mon Sep 17 00:00:00 2001 From: weigang Date: Thu, 28 May 2026 19:47:58 +0800 Subject: [PATCH] builder, util: fix symlinked vmodules import and temp dir v.mod pollution builder: also check candidate paths without resolving symlinks, so modules installed as symlinks inside `.vmodules` are recognized as belonging to the project. util: reject v.mod files found in or above the system temp directory when the relative path contains uppercase letters (e.g. ULID-based test session dirs), as they are likely unrelated to the project. Also respect `.v.mod.stop` and `.git` as project boundaries when walking up to find the project root. Add regression test for symlinked vmodules import. --- vlib/v/builder/builder.v | 29 +++++++++++- ...vmodules_package_compile_regression_test.v | 47 +++++++++++++++++++ vlib/v/util/module.v | 44 +++++++++++++++++ 3 files changed, 119 insertions(+), 1 deletion(-) diff --git a/vlib/v/builder/builder.v b/vlib/v/builder/builder.v index 724334026a0709..7c9e6e5990a0a9 100644 --- a/vlib/v/builder/builder.v +++ b/vlib/v/builder/builder.v @@ -846,15 +846,25 @@ fn (b &Builder) candidate_belongs_to_foreign_project(candidate_path string, impo if candidate_vmod_matches_import(abs_candidate, mod) { return false } + candidate_lookup_path := comparable_path(candidate_path) for lookup in b.pref.lookup_path { abs_lookup := comparable_real_path(lookup) if path_is_at_or_inside(abs_candidate, abs_lookup) { return false } + // Also check without resolving symlinks, so that a module installed + // as a symlink inside a lookup path (e.g. `.vmodules/pkg -> /real/pkg`) + // is still recognized as belonging to the project. + lookup_path := comparable_path(lookup) + if path_is_at_or_inside(candidate_lookup_path, lookup_path) { + return false + } } return true } +// path_belongs_to_lookup_path returns true when the given path is at or +// inside any of the configured lookup paths. fn (b &Builder) path_belongs_to_lookup_path(path string) bool { abs_path := comparable_real_path(path) for lookup in b.pref.lookup_path { @@ -866,8 +876,25 @@ fn (b &Builder) path_belongs_to_lookup_path(path string) bool { return false } +// comparable_real_path normalizes a path for comparison, resolving symlinks +// via os.real_path. Use when both sides of a comparison should refer to the +// same physical location on disk. fn comparable_real_path(path string) string { - mut normalized := os.real_path(path).replace('\\', '/') + return comparable_path_from(os.real_path(path)) +} + +// comparable_path normalizes a path for comparison without resolving symlinks. +// Use when the original (logical) path matters, e.g. for symlinked modules +// inside `.vmodules` that should match their lookup path as-is. +fn comparable_path(path string) string { + return comparable_path_from(os.abs_path(path)) +} + +// comparable_path_from normalizes a path string for consistent comparison: +// converts backslashes to forward slashes, collapses duplicate separators, +// and strips trailing slashes. +fn comparable_path_from(path string) string { + mut normalized := path.replace('\\', '/') for normalized.contains('//') { normalized = normalized.replace('//', '/') } diff --git a/vlib/v/tests/vmodules_package_compile_regression_test.v b/vlib/v/tests/vmodules_package_compile_regression_test.v index e8dcbd1d8d25ac..6917b108b01b09 100644 --- a/vlib/v/tests/vmodules_package_compile_regression_test.v +++ b/vlib/v/tests/vmodules_package_compile_regression_test.v @@ -30,6 +30,32 @@ fn issue_20147_write_file(path string, contents string) { os.write_file(path, contents) or { panic(err) } } +fn vmodules_symlink_workspace() string { + return os.join_path(os.vtmp_dir(), 'vmodules_symlink_import_compile') +} + +fn vmodules_symlink_write_project() ! { + workspace := vmodules_symlink_workspace() + vmodules_dir := os.join_path(workspace, '.vmodules') + real_module_root := os.join_path(workspace, 'real_vmarkdown') + link_module_root := os.join_path(vmodules_dir, 'vmarkdown') + app_root := os.join_path(workspace, 'app') + os.rmdir_all(workspace) or {} + os.mkdir_all(vmodules_dir)! + os.mkdir_all(real_module_root)! + os.mkdir_all(app_root)! + os.symlink(real_module_root, link_module_root)! + module_contents := + ['module vmarkdown', '', "pub fn ok() string { return 'ok' }"].join_lines() + '\n' + app_vmod_contents := ['Module {', "\tname: 'app'", '}'].join_lines() + '\n' + app_contents := + ['module main', '', 'import vmarkdown', '', 'fn main() {', '\tprintln(vmarkdown.ok())', '}'].join_lines() + + '\n' + issue_20147_write_file(os.join_path(real_module_root, 'vmarkdown.v'), module_contents) + issue_20147_write_file(os.join_path(app_root, 'v.mod'), app_vmod_contents) + issue_20147_write_file(os.join_path(app_root, 'main.v'), app_contents) +} + fn issue_20147_write_project() { basepath := issue_20147_module_root() vmod_contents := ['Module {', "\tname: 'msgpack'", '}'].join_lines() + '\n' @@ -75,3 +101,24 @@ fn test_issue_20147_vmodules_package_tests_compile() { res := os.execute('${os.quoted_path(issue_20147_vexe)} test .') assert res.exit_code == 0, res.output } + +fn test_vmodules_symlinked_package_import_compiles() { + vmodules_symlink_write_project() or { + $if windows { + eprintln('skipping symlink import regression test: ${err}') + return + } $else { + panic(err) + } + } + old_vmodules, had_vmodules := issue_20147_env_snapshot('VMODULES') + os.setenv('VMODULES', os.join_path(vmodules_symlink_workspace(), '.vmodules'), true) + defer { + issue_20147_restore_env('VMODULES', old_vmodules, had_vmodules) + os.rmdir_all(vmodules_symlink_workspace()) or {} + } + main_file := os.join_path(vmodules_symlink_workspace(), 'app', 'main.v') + res := os.execute('${os.quoted_path(issue_20147_vexe)} run ${os.quoted_path(main_file)}') + assert res.exit_code == 0, res.output + assert res.output.trim_space() == 'ok' +} diff --git a/vlib/v/util/module.v b/vlib/v/util/module.v index 8ab31055ed70d4..5de7c4c98d08d1 100644 --- a/vlib/v/util/module.v +++ b/vlib/v/util/module.v @@ -187,6 +187,13 @@ fn mod_path_to_full_name(pref_ &pref.Preferences, mod string, path string) !stri // after `'v.mod' in ls` can be removed once a proper solution is added if 'v.mod' in ls && (try_path_parts.len > i && try_path_parts[i] != 'v' && 'vlib' !in ls) { + // Reject v.mod files in or above the system temp + // directory when the path contains uppercase + // letters (e.g. ULID-based test session dirs), + // as they are likely unrelated to the project. + if j < i && is_unrelated_vmod_in_temp_dir(parent, try_path_parts[j..i]) { + continue + } last_v_mod = j break } @@ -238,6 +245,8 @@ fn module_name_has_empty_part(name string) bool { // enclosing v.mod for the current compilation (`pref_.path`). Module-name // qualification uses this as the boundary so a nested v.mod inside the // project does not silently rename its sub-modules. +// It also respects `.v.mod.stop` and `.git` as project boundaries to +// prevent walking past the current project's root. fn project_root_vmod_folder(pref_ &pref.Preferences) string { if pref_.path == '' { return '' @@ -252,10 +261,30 @@ fn project_root_vmod_folder(pref_ &pref.Preferences) string { return '' } mut cfolder := os.real_path(start) + start_folder := cfolder for { if os.is_file(os.join_path(cfolder, 'v.mod')) { + // Reject v.mod files in or above the system temp directory + // when the path contains uppercase letters (e.g. ULID-based + // test session dirs), as they are likely unrelated. + if cfolder != start_folder { + rel := start_folder.all_after(cfolder + os.path_separator) + if is_unrelated_vmod_in_temp_dir(cfolder, rel.split(os.path_separator)) { + return '' + } + } return cfolder } + // `.v.mod.stop` and `.git` mark project boundaries; stop walking + // up to avoid picking up a v.mod from an unrelated parent project + // (e.g. the V compiler repo when compiling tests in a temp dir). + // These markers are NOT v.mod roots — they only stop the search. + // `.git` can be a directory (normal repos) or a file (worktrees, + // submodules), so use os.exists instead of os.is_dir. + if os.is_file(os.join_path(cfolder, '.v.mod.stop')) + || os.exists(os.join_path(cfolder, '.git')) { + return '' + } parent := os.dir(cfolder) if parent == cfolder || parent == '' { return '' @@ -265,6 +294,21 @@ fn project_root_vmod_folder(pref_ &pref.Preferences) string { return '' } +// is_unrelated_vmod_in_temp_dir returns true when `vmod_folder` is in or +// above the system temp directory and `rel_parts` (the path segments between +// the v.mod and the source file) contain uppercase letters. This pattern +// indicates the v.mod belongs to an unrelated project that happens to live +// in a shared temp location, not to the current compilation. +fn is_unrelated_vmod_in_temp_dir(vmod_folder string, rel_parts []string) bool { + temp_dir := os.real_path(os.temp_dir()) + is_in_temp := vmod_folder == temp_dir || vmod_folder.starts_with(temp_dir + os.path_separator) + || temp_dir.starts_with(vmod_folder + os.path_separator) + if !is_in_temp { + return false + } + return rel_parts.filter(it.len > 0).any(contains_capital(it)) +} + // normalize_base_url_mod_name strips the `base_url` prefix from `mod_full_name` // when the module lives in a folder configured via v.mod's `base_url`. Without // this, a module rooted at `/source/feature` would be named `pkg.source.feature`