Repository navigation
Never read or rewrite project files outside the git repository root - #2025
Draft
Mikola Lysenko (mikolalysenko) wants to merge 2 commits into
Draft
Mikola Lysenko (mikolalysenko) wants to merge 2 commits into
Mikola Lysenko (mikolalysenko) wants to merge 2 commits into
Conversation
`parse_vendor_path` found the `.socket/vendor/` anchor anywhere in a string, so a lock entry naming a sibling checkout's store (`file:../.socket/vendor/npm/<uuid>/x.tgz`) read as ours to every vendored writer, reverter and GC, while discovery's `vendor_ref` already required a root-anchored path. With the same patch vendored in both checkouts, `vendor --revert` restored our recorded original over the other project's wiring. The ownership parse is now root-anchored and shared with discovery (`vendor_rest_from`): only `.`/`..` may precede the anchor and they must land exactly on the project root (`parse_vendor_path_from` for a file below the root). Yarn berry locators cut their `name@` first (`parse_vendor_locator`); pnpm <= 8 keeps its machine-absolute specifier (`parse_vendor_path_or_machine_absolute`), which pnpm itself writes. Readers that only ask "is this a vendored spec" (the lock inventory, composer/pnpm registry listings, the hosted-wiring probe, the reference scan behind repair and the orphan sweep) use `recognize_vendor_path_anywhere`, which nothing rewrites by. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
One shared bound (`utils::repo_root::ReadBoundary`): the git checkout enclosing the project, else the project itself; `..` and symlinked levels are resolved before the check. `bounded_project_path` refuses a climbing or absolute project-relative path as `NotFound` (a reader sees a missing file), and `is_refused_read` lets a writer refuse instead. - go.work: an ancestor's or an absolute `GOWORK` outside the root is only stat'ed; the workspace is Invalid, so writers refuse and discovery withholds. `use` members and `replace` directories outside the root read as missing (the selection stays Unknown). - ProjectView reads (discovery, lock inventory, requirements walks, yarn file:/portal:/link: copies) and the hosted restore's staged reads are bounded; the requirements planner and the lock flavors' out-of-root pin check refuse `pypi_requirements_outside_root`, the restore refuses with the reason. - NuGet parent configs above the root are not read; vendoring refuses and hosted mode skips the dep with `redirect_nuget_inherited_config_outside_root` (CLI_CONTRACT.md). Fixtures that model files beside the project inside a checkout now create the `.git` they assumed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Owner decision: socket-patch must not read project files outside the git repository root, or outside the project root when there is no repository. This PR applies that rule to VEX discovery, the lock inventory, hosted mode and vendored mode. It also fixes one concrete bug, N5: the vendored writers could rewrite or delete wiring that belongs to another project.
Two commits:
1. N5: vendored writers claimed a sibling checkout's vendor paths
vendor::parse_vendor_pathmatched the.socket/vendor/anchor anywhere in a string. Discovery'svendor_refalready required the path to start with that anchor. So a lock entry such aspoints at a sibling checkout's vendor store, but every vendored writer, reverter and GC treated it as ours. If the same patch uuid was vendored in both checkouts,
vendor --revertwrote this project's recorded pre-vendor original over the other project's wiring.Fix (
vendor/path.rs):parse_vendor_pathandparse_vendor_reference(the ownership parse) now accept only a root-anchored path:.and..segments may come before the anchor, and they must land exactly on the project root;parse_vendor_path_from(base, s)handles a file below the root, for example../../.socket/…frompackages/a.One function,
vendor_rest_from, now does this anchoring for both the writers and discovery'svendor_ref, so the two cannot drift apart again.parse_vendor_locator: yarn berryresolution: "left-pad@file:./.socket/…"has itsname@cut before the root-anchored parse.parse_vendor_path_or_machine_absolutecovers pnpm <= 8 only. pnpm itself writesfile:override specifiers as machine-absolute paths, so a committed lock holds another checkout's absolute root. Such a path is still claimed when it names the entry's own uuid and leaf, but never when it contains...DepRefgains aparsefield so the shared pnpm revert uses this rule only for legacy locks.recognize_vendor_path_anywherekeeps the old match-anywhere behavior, for recognition only. Its callers never rewrite anything:scan_vendor_references, which repair,vendor --check, rollback's ledger-missing guard and the orphan sweep use.A sibling reference there can only keep one of our own dirs from being swept, or flag it as still wired. It never leads to a rewrite.
Every
parse_vendor_pathcaller was reviewed:vendored_wiring_in_files): strict.recognize_vendor_path_anywhere."". pip resolves vendor-line paths against the working directory, not the including file.2. Bounds added
The shared check lives in
utils/repo_root.rs:ReadBoundary::of(project_root)returns the root offind_git_repo, else the project root.contains(path)makes the path absolute, folds.., and canonicalizes its deepest existing level, so neither..nor a symlink can escape.bounded_project_path(root, rel)returnsNotFound(a refused read looks like a missing file) and checks only whenrelclimbs or is absolute.is_refused_read(&e)lets a writer tell "refused" apart from "missing" and refuse.go_workspace::resolve_with/load_with, the shared loader that #2011 makes discovery use too)go.workup to/, or any absoluteGOWORKGoWork::Invalidwith the reason, so vendored and hosted writers refuse through the existing workspace refusal and discovery withholds.usemembers outside the rootUnknownreplace => ../dirgo.mod in the module graph (read_dep_go_mod) andgo_native_vendor::expected_textProjectView::read_text/read_bytes(Disk + Snapshot), used by discovery, the lock inventory and the requirements walks:-rincludes, yarnfile:/portal:/link:copies, any..//absolute relcollect_requirements_files) and the lock flavors'pin_outside_rootrefuse withpypi_requirements_outside_root.redirect/staged.rs::read_rel), including the absolute ancestorpnpm-workspace.yaml/.npmrcprobesread <rel>: … is outside the repository root …nuget.config(nuget_config::inherited_source_keys_traced)/Inherited::outsidenames it; vendoring refuses, and hosted mode skips each NuGet dep with the new coderedirect_nuget_inherited_config_outside_root, documented in CLI_CONTRACT.mdCLI_CONTRACT.md has a new section, "Reads bounded to the repository root", and a sentence in the Go paragraph.
Machine-level reads (not changed)
crates/socket-patch-core/src/utils/go_env.rs:26(go_env),:148-177: theGOENVfile ($GOENV, else the OS config dir'sgo/env), used for GOWORK / GOFLAGS / GOPROXY / GOPRIVATE.crates/socket-patch-core/src/utils/go_env.rs:62gomodcache: the Go module cache. go.mod files are read from it invendor/go_workspace.rs(read_dep_go_mod),vendor/go_build_list.rs:313andvendor/go_native_vendor.rs:55.crates/socket-patch-core/src/vendor/nuget_config.rs:56user_config_path(read at:145) andsettings_chain: the userNuGet.Config(%APPDATA%/$DOTNET_CLI_HOME/~/.nuget/NuGet/NuGet.Config).crates/socket-patch-core/src/patch/redirect/npmrc.rs:564outer_config_layers: npm builtinnpmrc, user~/.npmrc/npm_config_userconfig, global<prefix>/etc/npmrc.crates/socket-patch-core/src/patch/redirect/yarnrc.rs:317RcLayout::files:~/.yarnrc,~/.npmrc,<prefix>/etc/yarnrc|npmrc.crates/socket-patch-core/src/crawlers/pkg_managers.rs:208,247: the home.yarnrc.yml.crates/socket-patch-core/src/patch/redirect/upstream/npm.rs(~639-648 home.yarnrc.yml; ~2057-2075 / ~2466-2474$XDG_CONFIG_HOME/.npmrcor~/.npmrc, and.bunfig.toml).crates/socket-patch-core/src/patch/redirect/upstream/uv.rs:999global_uv_settings:$XDG_CONFIG_HOME/uv/uv.toml,$XDG_CONFIG_DIRS,/etc/uv/uv.toml,%APPDATA%,%PROGRAMDATA%.crates/socket-patch-core/src/vendor/cargo_config.rs:150-178andcrates/socket-patch-core/src/formats/cargo/source_replacement.rs:112-139:$CARGO_HOME/config*.crates/socket-patch-core/src/crawlers/ruby_crawler.rs:51,173(and ~2012-2035, ~2221-2240): Bundler app config ($BUNDLE_APP_CONFIG) and global config ($BUNDLE_CONFIG,$BUNDLE_USER_CONFIG,$BUNDLE_USER_HOME,~/.bundle/config).crates/socket-patch-core/src/crawlers/maven_crawler.rs:619m2_repo_path_with:~/.m2/settings.xml,$MAVEN_HOME/conf/settings.xml. Alsojvm_cache.rslocal repository and Gradle cache roots.crates/socket-patch-core/src/gradle/home.rs,crawlers/gradle_cache.rs: the Gradle user homeinit.dand wrapper init scripts.crates/socket-patch-core/src/utils/repo_root.rs:33GitRepo::config_path: git's own config, which agitdir:/commondirmay place outside the worktree.Left unbounded (owner to judge)
These walks search upward for a workspace root that the package manager really uses. If they are bounded and there is no
.git(a CI tarball checkout, a Docker build context), the boundary becomes the project itself, so a workspace member would be treated as standalone. Today, hosted and vendored use these walks to refuse ("settings live above the project, nothing is written"); a silent bound would turn those refusals into wrong writes. Each one needs its own writer refusal, as go.work and NuGet now have. With a.gitpresent they already stop at the checkout in practice: none of them reads above the repo root unless a workspace file actually sits there.hosted/governing_root.rs:pnpm_lock_elsewhere(ancestorpnpm-workspace.yaml, workspace-root.npmrc,lockfileDirstat),package_json_workspace_root,vlt_workspace_root,vlt_project_root,member_stray_lock.utils/pnpm_workspace.rs:governing_workspace_file,git_branch_locks.utils/uv_workspace.rsgoverning_uv_workspace;utils/python_script.rsfind_ancestor_uv_config/ancestor_uv_config_files;utils/hatch_workspace.rs(bounded by the checkout, but walks to$HOMEwithout one).vendor/cargo.rsworkspace_root_refusal,vendor/cargo_config.rsread_config_chain_with,formats/cargo/source_replacement.rsconfig_layer_paths,utils/cargo_build_cache.rs(agent mode).crawlers/pkg_managers.rs.yarnrc.ymlancestor walk;patch/redirect/yarnrc.rsRcLayout::filesancestor.yarnrc/.npmrc;patch/redirect/upstream/npm.rsBerryRegistrySettings::read. The restore's absolute-path probes now refuse throughread_rel, but these reads use their own I/O.vendor/maven_repo.rsnot_build_root: bounded byancestor_search_dirs, except with no repo.crawlers/gradle_cache.rsbuild_roots,vex/verify.rsnode_resolve: scan/verify only, stat-level.vendor/cargo.rspath_crate_version: a user[patch]path = "../x"; a writer-side refusal needs its own code.formats/yarn/rc.rsinjectEnvironmentFiles(dotenv paths may be../or absolute, and may come from the home rc).patch/redirect/yarnrc.rsyarn_classic_workspace_members: follows directory symlinks to members.vendor/go_consumer_sync.rsmodules_txt_location: when the ancestor go.work is out of bounds,resolvenow says Invalid and this falls back to the project's ownvendor/modules.txt. The audit there may then miss staleness in the out-of-root workspace, but the write path refuses first.scan_vendor_referencestext scan (repair /vendor --check/ rollback guard / orphan sweep) still recognizes a../.socket/vendor/…mention as a reference. It never rewrites it. The only effect is that one of our own dirs with the same uuid is kept rather than swept, or reported as still wired.-rincludes (Vendored pip: a pin in a-rinclude outside the project root is ignored when a pylock.toml exists, sovendor --checkpasses andvexattests not_affected whilepip install -r requirements.txtinstalls the unpatched release #1851) now read as missing, following "readers treat a refused read as missing". Their registry pins no longer contest a lock's wiring. Includes outside the project but inside the repo are still read.Tests
Each new test fails with the bound disabled (
ReadBoundary::containsforced totrue) or with the old anywhere-anchored parse, and passes with this change:vendor::npm_lock::tests::revert_never_claims_a_sibling_checkouts_vendor_path(N5: the entry is left byte-for-byte,vendor_lock_entry_drifted, artifact kept)vendor::path::tests::ownership_parse_is_root_anchoredutils::repo_root::tests::read_boundary_is_the_checkout_else_the_project,read_boundary_sees_through_linksvendor::go_workspace::tests::go_work_outside_the_repository_is_never_readvex::discover::golang::tests::gowork_naming_another_workspace_attests_nothing(expectation updated: an out-of-repoGOWORKnow says "outside the repository root")vendor::pypi_requirements::tests::includes_outside_the_repository_are_never_readvendor::nuget_config::tests::a_parent_config_above_the_repository_is_never_read,vendor::nuget_feed::tests::vendor_refuses_under_a_parent_config_above_the_repositorypatch::redirect::tests::nuget_inherited_config_outside_the_repository_skips_the_deppatch::redirect::staged::tests::reads_outside_the_repository_refuseFixtures that model "beside the project, inside the checkout" now create a
.gitat their tempdir root: the go workspace / consumer-sync / go_mod_edit tests, the nuget parent-config tests, the pypi out-of-root include tests, discover'sProject::nested, and one hosted engine test and one upstream npm test. Before this change these fixtures were not repositories at all.Local runs (macOS):
cargo test -p socket-patch-core --lib -- vendor:: vex::discover:: patch::redirect:: hosted:: utils::repo_rootvex::discover::testing::golden::tests::committed_fixture_corpus_matches_goldencargo test -p socket-patch-core --lib--lib--test vendor--test in_process_rollback_vendored--test in_process_remove_repair_lifecycle--test in_process_vendor_bun_takeover--test in_process_redirect--test in_process_rollback_hosted--test hosted_memory_engine--test in_process_scancargo clippy --locked --workspace --all-features -- -D warningscargo fmt --all -- --check🤖 Generated with Claude Code