Skip to content

Vendored requirements.txt can't be reverted once the user touches the # socket-patch vendor: comment: vendor --revert drift-keeps forever (exit 0), and the suggested "re-vendor" remedy is a no-op or adds a duplicate line #977

Open

Description

[agent] Found by the scheduled pip / requirements.txt bug-hunt routine (ledger #309).

Summary

The vendored requirements.txt flavor decides whether a vendor line is still "ours" in two different ways:

So any edit to the trailing # socket-patch vendor: six==1.16.0 comment leaves a project that installs the patched wheel fine but can never be reverted. The edit can be a changed space, a user's own note appended after it, or the comment stripped by a formatter. vendor --revert prints vendor_revert_line_drifted, keeps the wheel and the ledger entry, and exits 0 with status: "success". Its remedy text says to "undo the drift (restore the vendored lock entries or re-vendor) and re-run vendor --revert". But:

  • When the comment was reformatted or extended, vendor reports already_vendored ("artifact and lockfile wiring already in sync") and changes nothing. Re-running vendor --revert drift-keeps again, forever.
  • When the comment was stripped, vendor doesn't recognise the bare wheel path as six's pin. It appends a second wiring line for the same wheel, ./…/six-1.16.0-py2.py3-none-any.whl # socket-patch vendor: six==1.16.0 (transitive). A later vendor --revert removes only that appended line. The user's bare path line stays, the original six==1.16.0 pin is never restored, and the run exits 0 again.

Nothing tells the user which exact bytes would count as "undoing the drift". The warning text also leaks Rust debug formatting and names codes that this flavor never emits (see below).

Impact

A user who annotates or reformats the vendored line can't get back to the registry pin through any socket-patch command. Doing that is normal for a hand-maintained requirements.txt, and so is running a comment-stripping tool. vendor --revert exits 0 and status: "success", so scripts and CI treat the unvendor as done, while requirements.txt still installs the vendored wheel and .socket/vendor/pypi/<uuid>/ plus the ledger entry stay. (remove does exit 1 with vendor_revert_kept, so the two front doors disagree about the same state.) Installs keep working; nothing is unpatched. The harm is a stuck project and a misleading success.

Repro

This uses the in-repo vendoring fixture (tests/prebuilt_common::prepare_command, the same server e2e_vendor_pypi_build.rs::pip_requirements_vendor_fresh_checkout_no_index_and_revert uses), with real pip 24.0 on CPython 3.11 and six==1.16.0 installed in ./.venv. The .socket/manifest.json and blob are staged exactly like that test's stage_patch.

printf 'idna==3.7\nsix==1.16.0\n' > requirements.txt
socket-patch vendor --json --cwd .        # applied: 1
cat requirements.txt
# idna==3.7
# ./.socket/vendor/pypi/<uuid>/six-1.16.0-py2.py3-none-any.whl  # socket-patch vendor: six==1.16.0

# Any one of these edits:
sed -i 's/  # socket-patch vendor/ # socket-patch vendor/' requirements.txt          # (a) one space fewer
sed -i '2s/$/  # CVE fix, do not bump/' requirements.txt                            # (b) user note
sed -i 's/  # socket-patch vendor.*//' requirements.txt                              # (c) comment stripped

socket-patch vendor --check --cwd .; echo $?          # 0: "in sync"
socket-patch vendor --revert --cwd .; echo $?         # 0
# Warning: requirements.txt: the vendor line for Some("requirements.txt:2") changed since vendoring; left untouched
# Warning: requirements.txt still references .socket/vendor/pypi/<uuid> after revert
# Warning: kept .socket/vendor/pypi/<uuid>: some recorded lock entries were left alone (see the vendor_lock_entry_drifted / vendor_lock_entry_removed warnings) … undo the drift (restore the vendored lock entries or re-vendor) and re-run `vendor --revert` to finish cleaning up
# Kept 1 drifted package: lock entries were re-resolved since vendoring, …

socket-patch vendor --json --cwd .     # the suggested remedy
#  (a)/(b): skipped already_vendored "artifact and lockfile wiring already in sync"; file unchanged
#  (c):     applied; appends a second line for the same wheel:
#           ./.socket/vendor/pypi/<uuid>/six-1.16.0-py2.py3-none-any.whl
#           ./.socket/vendor/pypi/<uuid>/six-1.16.0-py2.py3-none-any.whl  # socket-patch vendor: six==1.16.0 (transitive)
socket-patch vendor --revert --cwd .; echo $?
#  (a)/(b): same drift-keep, exit 0, forever
#  (c):     removes only the "(transitive)" line; the bare path line stays, `six==1.16.0` never comes back; exit 0

With (c) after the re-vendor, pip install --no-index -r requirements.txt accepts the duplicated wheel line and installs the patched build on pip 20.3.4, 24.0 and 26.2.1. So install works, and only the unwind is broken.

Expected vs actual

Message defects in the same path:

  • drift_warning formats rec.key (an Option<String>) with {:?}, so users see Some("requirements.txt:2"). The same {:?}-on-rec.key pattern is in pypi_uv.rs:847, pypi_pipenv.rs:468 and common.rs:806.
  • The vendor_artifact_kept hint points at vendor_lock_entry_drifted / vendor_lock_entry_removed, but the requirements flavor emits vendor_revert_line_drifted. The summary also says "lock entries were re-resolved", which isn't what happened here.

Matrix

OS pip / Python Edit to the vendor line vendor --check vendor --revert re-vendor (remedy) Reproduces
Linux 24.0 / CPython 3.11 (a) comment spacing 0 (in sync) drift-keep, exit 0 already_vendored, no-op yes (3/3)
Linux 24.0 / CPython 3.11 (b) user note after the marker 0 drift-keep, exit 0 already_vendored, no-op yes (2/2)
Linux 24.0 / CPython 3.11 (c) marker comment stripped 0 drift-keep, exit 0 appends a duplicate (transitive) line; next revert strands the user's line yes (3/3)
Linux 24.0 / CPython 3.11 none (control) 0 byte-identical revert n/a no

pip only matters for the install check (20.3.4 / 24.0 / 26.2.1 all install the duplicated file). The rewrite and revert logic is pip-independent and OS-independent, so macOS and Windows weren't probed.

First bad version: not bisected (main 9c43dfc; the latest release is v4.0.0).

Suspect code

  • crates/socket-patch-core/src/vendor/pypi_requirements.rs:415: revert finds the line with lines.iter().rposition(|l| l.trim() == new_line.trim()), which compares the whole line including the comment. Compare :900 (requirements_entry_in_use), which checks split_comment(&ll.text).0, and the --check / already_vendored path, which also treats the line as wired.
  • crates/socket-patch-core/src/vendor/pypi_requirements.rs:744 (vendor_line_marker(split_comment(old).0)) and the planner around :594 / :648: a bare wheel path line isn't recognised as the package's pin, so a re-vendor appends a (transitive) line.
  • crates/socket-patch-core/src/vendor/pypi_requirements.rs:482-489 (drift_warning, {:?} on rec.key).

Probe runs: none (OS-independent rewrite logic).


Backlog review — 2026-10-08

Priority: P1 → P2. Editing the vendor marker comment prevents automated revert, but the artifact is retained. Real recovery issue with a manual-edit trigger.

Activity

  1. added
    bugSomething isn't working
    bughuntFound by a scheduled package-manager bug-hunt agent
    pm:pippip / requirements.txt
    on Oct 7, 2026
  2. mikolalysenko commented on Oct 7, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Triaged as priority:p1 (pip / requirements.txt). Confirmed on main 9c43dfc: pypi_requirements.rs:415 matches the recorded vendor line by whole-line equality (comment included), while the --check / already_vendored / in-use paths key on split_comment(..).0, so a comment-only edit is "wired" for one side and "drifted" for revert. Not a duplicate of #867 (that one is about a -r include still referencing the wheel) and no open PR covers it.


    Generated by Claude Code

  3. mikolalysenko commented on Oct 7, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Structural context from the architecture audit (ecosystems and formats): forward and revert recognizing "our" wiring by different predicates is not limited to requirements.txt. On 9c43dfc:

    This issue can still be fixed on its own first.


    Generated by Claude Code

  4. mikolalysenko commented on Oct 9, 2026

    @mikolalysenko
    CollaboratorAuthor

    v5 triage: P3, not a release blocker. The user-edited Socket bookkeeping comment case stays as P3 recovery work; it is not the ordinary unmodified-lockfile path.

    This follows the maintainer's release scope: one normally completing CLI instance, prioritizing valid-lockfile patch/install behavior, compatibility, and actionable CLI UX.

  5. mikolalysenko commented on Oct 29, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Claiming this issue (shared root cause: vendored requirements.txt revert finds its line by whole-line equality, comment included, while vendor/--check/in-use key on the code part). Branch: agent/fix-pypi-requirements-revert-comment. Claim-ID: 2026-10-29T11:29:54Z-adcf8e


    Generated by Claude Code

  6. mikolalysenko commented on Oct 29, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Draft PR: #1825


    Generated by Claude Code

  7. mikolalysenko commented on Oct 29, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] #1825 fixes the stuck revert and the duplicate re-vendor line for all three edits (respaced, note appended, stripped), with unit and real-pip e2e tests. It uses Refs, not Fixes: vendor --revert still exits 0 with status: success on a drift-keep while remove exits 1, and the summary still says "lock entries were re-resolved". Both are shared by every vendored ecosystem, so changing them is a cross-ecosystem contract call, and they are left open here. The {:?} key formatting was already fixed on main (#1608).


    Generated by Claude Code

  8. mikolalysenko commented on Oct 29, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] #1825 merged: comment-only edits to a vendored requirements.txt line no longer block vendor --revert / remove / rollback, and re-vendoring no longer duplicates the line. Two cross-ecosystem parts are left, and they need a maintainer decision on the contract: whether vendor --revert should exit 1 with partialFailure on a drift-keep, as remove does, and the generic "lock entries were re-resolved" summary wording. Releasing the claim and labeling agent:needs-human for that decision.


    Generated by Claude Code

  9. mikolalysenko commented on Oct 29, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] pip bug-hunt re-triage (ledger #309): the pip-side part of this issue is verified fixed on main b291caa (#1825) with real pip. I'm leaving the issue open for the pending agent:needs-human decision on drift-keep exit codes.

    Setup: idna==3.7 + six==1.16.0, scan --mode vendored --yes, then each edit to the six vendor comment, then vendor --check and the revert:

    Edit to the vendor comment pip vendor --check Revert File after revert
    respaced ( # socket-patch vendor:) 26.2.2 0 vendor --revert 0, "Reverted 2 vendored packages" byte-identical
    note appended (… # pinned by sec team) 26.2.2 0 vendor --revert 0 byte-identical
    comment stripped 26.2.2 0 vendor --revert 0 byte-identical
    stripped, then scan --mode vendored again 26.2.2 – – one six line (no duplicate); fresh pip install -r gives patched six and idna
    CRLF, original six==1.16.0 # keep, vendor: respaced plus # edited 26.2.2 – remove pkg:pypi/six@1.16.0 0 six restored to six==1.16.0 # keep\r; idna stays vendored
    same file 26.2.2 – rollback 0 byte-identical
    CRLF hash continuation and a marker line, #socket-patch vendor: plus # sec 20.3.4 / CPython 3.8 0 (and pip install -r gives patched six and idna) vendor --revert 0 byte-identical

    Generated by Claude Code

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent:needs-humanagent:triagedbugSomething isn't workingbughuntFound by a scheduled package-manager bug-hunt agentpm:pippip / requirements.txtpriority:p3uxCLI commands, help, diagnostics, output consistency, or actionable recovery instructions.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions