Skip to content

Sweep core utils redact, repo_root, uv_workspace and small helpers - #1923

Open
Mikola Lysenko (mikolalysenko) wants to merge 5 commits into
mainfrom
arch-refactor/sweep-core-utils-redact-repo-root
Open

Mikola Lysenko (mikolalysenko) wants to merge 5 commits into
mainfrom
arch-refactor/sweep-core-utils-redact-repo-root

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Nov 1, 2026 •

Copy link
Copy Markdown
Collaborator

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 in coverage/files.tsv on arch-audit/ledger), plus A20 (#1848): one URL-userinfo stripper, in utils::redact. Living document: Part 2 "URL credential stripping", Part 4 "Module docs that claim a monopoly", Part 5 "Misplaced" tests.

Changes

  • redact.rs (A20):
    • deleted the dead pub strip_url_credentials and url_userinfo (no caller outside their own test);
    • moved vex::product::strip_url_userinfo here; vex::product and patch/redirect/upstream/npm.rs call it;
    • the moved helper finds the userinfo with the module's own userinfo_span, so its authority now ends at ?/# as well as / (found by /code-review);
    • folded query_span and query_pairs into their one caller, secret_query_spans;
    • module doc: no longer says the VEX @id uses strip_url_credentials.
  • Tests:
    • credentials_are_stripped_from_an_identifier (tested the deleted helper) is replaced by userinfo_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_redacted gains an empty-value case (?token=&key&v=1 stays as is). Red→green: with the gate mutated to eq < pair.len() the test fails; it passes on the PR.
    • the_service_download_advisory_has_one_builder moved to vendor/service_fetch.rs, the file it lints.
  • repo_root.rs: inlined GitRepo::dot_git (callers: config_path, policy/mod.rs, a test); dropped the bare B22 tag; 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 names vendor::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 --frozen installs the unpatched release and vendored vex attests not_affected #1138 story cut to what is true today.
  • uri.rs: clean; the encode_uri_component asserts that vendor/yarn_berry_lock.rs helper_grammar copied byte for byte are dropped there.
  • Not done, callers held by open PRs: deleting toml_edit_ext (needs utils/mod.rs, held by Sweep core utils http, mod and hatch #1914, and vendor/pypi.rs); folding uv_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 @id and every output byte unchanged, except one pre-existing bug the move fixed: the npm upstream_registry_fallback warning no longer mangles a registry URL whose query or fragment holds an @ (https://h?scope=@acme used to print as https://acme).

/code-review high

7 findings; fixed 3 (the authority boundary above, a doc that contradicted the module's --json rule, a private intra-doc link). Not fixed:

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.
  • No platform-specific code changed.

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 in vex::product and the unused strip_url_credentials / url_userinfo helpers. VEX @id building and npm upstream_registry_fallback warnings now share that single implementation.

The moved helper uses the same userinfo_span authority 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 in repo_root, serde, toml_edit_ext, and uv_workspace; simplify secret-query span parsing in redact.rs; relocate the vendor_prebuilt_downloaded single-builder lint test to vendor/service_fetch.rs; drop redundant encode_uri_component asserts from yarn_berry_lock tests.

Note: strip_url_userinfo no 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

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
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review November 1, 2026 09:28
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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.

Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Nov 1, 2026
Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko Mikola Lysenko (mikolalysenko) added arch-refactor PR opened by the scheduled architecture refactor routine refactor Structural change: duplicated code or logic, missing abstraction, layering, dead code labels Nov 1, 2026
@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Nov 1, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[burn-down agent] Labeled Ready for review at 8f0433c.

  • CI: 82/82 checks on 8f0433c passed or were skipped (Cursor Bugbot neutral: a duplicate run was cancelled). The PR is mergeable.
  • Bugbot: reviewed 8f0433c and found no new issues. No unresolved review threads.
  • Reviewer note: the PR keeps one URL userinfo stripper, utils::redact::strip_url_userinfo, which now ends the authority at ? and #. That fixes npm fallback warnings mangling registry URLs that have @ in a query or fragment. The removed strip_url_credentials had no callers, so the VEX @id output doesn't change. Everything else is core-utils doc trims and moved tests.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does. A tidy-up of six core utils files plus A20 (#1848). There is now one URL-userinfo stripper, in utils::redact: the dead strip_url_credentials and url_userinfo are deleted, and vex::product::strip_url_userinfo moves there. The rest is doc trims, inlining GitRepo::dot_git, and moving one lint-test to the file it checks.

Risk: low. About −100 production lines; the deleted items had no callers. One deliberate behavior change: the moved stripper's authority now ends at ?/# as well as /. So the npm upstream-fallback warning no longer mangles https://h?scope=@acme. VEX @id stays the same for real remotes.

Look here

  • utils/redact.rs:220: strip_url_userinfo now uses the shared userinfo_span/authority_span.
  • utils/redact.rs:154: query_span/query_pairs folded into secret_query_spans. I checked it gives the same result as before, including empty values, valueless keys and # before ?.
  • vex/product.rs:713 and patch/redirect/upstream/npm.rs:158:`` the two callers.
  • utils/redact.rs:387: table test for the issue's acceptance cases (@ in a password, in a query or fragment, scp form, host:repo@v1).

Verified

  • Read the full diff.
  • The encode_uri_component asserts dropped from yarn_berry_lock.rs are still present in utils/uri.rs.
  • No remaining callers of the deleted pub items.
  • CHANGELOG.md untouched.
  • Up to date with main (merge-base is main's head).
  • Locally: cargo clippy -p socket-patch-core -- -D warnings clean, cargo fmt --check clean, 213 tests pass across redact, repo_root, uri, vex::product, service_fetch, helper_grammar and upstream::npm.
  • CI: 31 success, 50 skipped, 1 neutral, ci-ok and clippy green. Bugbot found no new issues on 8f0433c.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch-refactor PR opened by the scheduled architecture refactor routine Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review refactor Structural change: duplicated code or logic, missing abstraction, layering, dead code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Keep one URL credential stripper: utils::redact::strip_url_credentials is dead while VEX and upstream npm use vex::product's copy

2 participants