Repository navigation
Sweep core utils redact, repo_root, uv_workspace and small helpers - #1923
Mikola Lysenko (mikolalysenko) wants to merge 5 commits into
Conversation
Assisted-by: Claude Code:claude-opus-5-5
Move vex::product::strip_url_userinfo into utils::redact, where upstream npm and the VEX product id now both reach it, and delete the dead strip_url_credentials and url_userinfo. The moved code is byte-identical, so VEX @id and fallback warnings are unchanged. Fold query_span and query_pairs into their one caller, secret_query_spans. Move the vendor_prebuilt_downloaded builder lint to service_fetch, the file it guards. Fixes #1848 Assisted-by: Claude Code:claude-opus-5-5
Inline GitRepo::dot_git into its two callers, trim the module docs of repo_root, serde, toml_edit_ext and uv_workspace to what is true today, and drop the encode_uri_component asserts that yarn_berry_lock copied byte for byte from uri.rs. Assisted-by: Claude Code:claude-opus-5-5
Assisted-by: Claude Code:claude-opus-5-5
strip_url_userinfo now finds the userinfo with the module's own userinfo_span, so an @ in a query or fragment (https://h?scope=@acme) is no longer taken for one. The VEX @id is unchanged: it drops the query and fragment before stripping. Unlink the pub(crate) helper from the public module doc. Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 8f0433c. Configure here.
Assisted-by: Claude Code:claude-opus-5-5
|
[burn-down agent] Labeled Ready for review at
Generated by Claude Code |
Final review briefWhat it does. A tidy-up of six core Risk: low. About −100 production lines; the deleted items had no callers. One deliberate behavior change: the moved stripper's authority now ends at Look here
Verified
Changes I made: none. Open questions: none. The PR description lists three /code-review findings left unfixed, with reasons: the pub-item semver point (consistent with #1907/#1914/#1920), and two pre-existing edge cases noted for follow-up issues. Auto-merge is armed, so approving sends this straight to the merge queue. Generated by Claude Code |
Wenxin Jiang (Wenxin-Jiang)
left a comment
There was a problem hiding this comment.
[agent] Reviewed 8f0433c756022984d69a9acf159a3c4fbdd60d8a. Checked URL userinfo consolidation, query-value redaction, repository-root access and helper visibility. No major correctness issues found. Validation: source and regression-test review plus the available exact-head CI checks; no local test pass is claimed.
LLM Description written by Claude Code:claude-opus-5-5
Fixes #1848
Summary
Sweep of the audited core rows
utils/{redact,repo_root,serde,toml_edit_ext,uri,uv_workspace}.rs(coverage notes incoverage/files.tsvonarch-audit/ledger), plus A20 (#1848): one URL-userinfo stripper, inutils::redact. Living document: Part 2 "URL credential stripping", Part 4 "Module docs that claim a monopoly", Part 5 "Misplaced" tests.Changes
redact.rs(A20):strip_url_credentialsandurl_userinfo(no caller outside their own test);vex::product::strip_url_userinfohere;vex::productandpatch/redirect/upstream/npm.rscall it;userinfo_span, so its authority now ends at?/#as well as/(found by /code-review);query_spanandquery_pairsinto their one caller,secret_query_spans;@idusesstrip_url_credentials.credentials_are_stripped_from_an_identifier(tested the deleted helper) is replaced byuserinfo_is_stripped_from_an_identifier, a table over the surviving helper:https://u:p@host/x?y#z,https://u@host?x(the issue's acceptance cases),@in a password,@in a query/fragment, scp form,host:repo@v1, plain.secret_query_values_are_redactedgains an empty-value case (?token=&key&v=1stays as is). Red→green: with the gate mutated toeq < pair.len()the test fails; it passes on the PR.the_service_download_advisory_has_one_buildermoved tovendor/service_fetch.rs, the file it lints.repo_root.rs: inlinedGitRepo::dot_git(callers:config_path,policy/mod.rs, a test); dropped the bareB22tag; module doc no longer claims every caller goes through it (A21).serde.rs: doc trimmed from 9 lines to 2.toml_edit_ext.rs: module doc no longer namesvendor::cargo_config(it has its own variant).uv_workspace.rs: module doc's Scan from a hatchling member of a uv workspace rewrites the member's pyproject.toml as a lockless Hatch project, so the root uv.lock goes stale,uv sync --frozeninstalls the unpatched release and vendoredvexattests not_affected #1138 story cut to what is true today.uri.rs: clean; theencode_uri_componentasserts thatvendor/yarn_berry_lock.rshelper_grammarcopied byte for byte are dropped there.toml_edit_ext(needsutils/mod.rs, held by Sweep core utils http, mod and hatch #1914, andvendor/pypi.rs); foldinguv_workspace's two calls (governing_root.rs,vendor/pypi.rs).Size
git diff --stat: 10 files, +114 / −219. Production about −100 lines, tests about −5.Behavior change
VEX
@idand every output byte unchanged, except one pre-existing bug the move fixed: the npmupstream_registry_fallbackwarning no longer mangles a registry URL whose query or fragment holds an@(https://h?scope=@acmeused to print ashttps://acme)./code-review high
7 findings; fixed 3 (the authority boundary above, a doc that contradicted the module's
--jsonrule, a private intra-doc link). Not fixed:socket-patch-coreis a semver break. The crate is the CLI's engine, and earlier sweeps (Sweep core constants.rs and utils (concurrent, durability, …) #1907, Sweep core utils http, mod and hatch #1914, Sweep core utils notice, npm_range, pep440, pipenv, purl #1920) narrowed and deleted pub items the same way; nothing outside the workspace is a supported consumer.redact_urls_in) changes the warning text. Noted in the refactor register for an issue.user:pw@host:team@2024/r.git, a://inside an scp remote's query) return the credential unchanged. Pre-existing in the moved code, and they change VEX@idbytes: noted for an issue.Verification
cargo clippy --workspace --all-features -- -D warnings: clean.cargo test -p socket-patch-core --lib: 6995 pass; 4 fail only because the sandbox runs as root (copy_tree::relax_loop_must_not_traverse_symlinked_root,vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry,pypi_poetry::wire_write_failure_maps_error_and_leaves_lock_untouched,pypi_requirements::wire_failure_rolls_back_already_written_files), none touching changed code.Risk
Low: core helpers with unit coverage, one moved function.
🤖 Generated with Claude Code
https://claude.ai/code/session_011f9aeT6eZVa8qE4TzityNu
Note
Low Risk
Refactor of well-tested URL helpers with a small, intentional fix to warning text for registry URLs containing
@in query/fragment; no auth or payment paths touched.Overview
Consolidates URL userinfo handling into
utils::redact::strip_url_userinfo, removing the duplicate invex::productand the unusedstrip_url_credentials/url_userinfohelpers. VEX@idbuilding and npmupstream_registry_fallbackwarnings now share that single implementation.The moved helper uses the same
userinfo_spanauthority rules as display redaction (authority stops at/,?, and#), which fixes fallback warnings that used to corrupt registry URLs when@appeared in a query or fragment (e.g.https://host?scope=@acme).Supporting cleanups: inline
GitRepo::dot_git()at call sites; trim module docs inrepo_root,serde,toml_edit_ext, anduv_workspace; simplify secret-query span parsing inredact.rs; relocate thevendor_prebuilt_downloadedsingle-builder lint test tovendor/service_fetch.rs; drop redundantencode_uri_componentasserts fromyarn_berry_locktests.Note:
strip_url_userinfono longer strips secret query params (only userinfo)—aligned with the renamed, narrower helper. npm fallback warnings may still echo non-userinfo secrets in query strings (pre-existing).Reviewed by Cursor Bugbot for commit 8f0433c. Configure here.
Generated by Claude Code