[Mikola Lysenko (mikolalysenko)]

LLM Description written by Claude Code:claude-opus-5-5

Fixes #1635

Summary

A hosted or vendored scan run as root (a Docker bind mount, sudo) used to leave the project's poetry.lock owned by root, so the owner's next poetry add / poetry lock failed with Permission denied. A run in a shared group checkout moved the lock to the runner's primary group. Rewritten lockfiles, manifests and .npmrc now keep their owner and group, the same as their mode.

Root cause

Every mode-preserving rewrite of a project file (poetry.lock, package.json, .npmrc, Gemfile.lock, …, plus the rollback / remove / vendor --revert restores and the group-commit replay) runs through stage_and_rename_blocking in crates/socket-patch-core/src/utils/fs.rs. It stages a fresh file, copies only the old file's permission bits onto it, and renames it over the old path. The fresh inode belongs to the process's euid/egid. So the defect isn't Poetry-specific: every ecosystem's lock and manifest rewrite had it.

Fix

keep_owner fchowns the stage to the old file's uid/gid before the fsync and before the chmod (chown(2) clears setuid/setgid), best-effort like npm's write-file-atomic. An unprivileged caller that can't give the file away (EPERM) still keeps the group when it belongs to it; otherwise the file keeps the runner's ownership, as before. CLI_CONTRACT.md now states the ownership rule.

Test evidence

Test	Without fix	With fix
utils::fs::tests::preserving_writes_keep_the_destination_owner (root; every preserving writer, setuid mode kept)	FAIL (0, 0, 2540) vs (1000, 1001, 2540)	pass
utils::fs::tests::preserving_write_keeps_a_supplementary_group (run as a non-root user in a devs group)	FAIL gid 30033 vs 30002	pass
in_process_redirect::root_hosted_scan_keeps_the_lock_owner (end-to-end hosted scan as root)	FAIL (0, 0) vs (1000, 1001)	pass

Per-issue checklist:

• Hosted and vendored rewrites replace poetry.lock with a file owned by the user running socket-patch, so after a root scan (e.g. a Docker bind mount) the lock's owner gets "Permission denied" from poetry add / poetry lock #1635: root run keeps the lock's owner (preserving_writes_keep_the_destination_owner, root_hosted_scan_keeps_the_lock_owner); a group-checkout run keeps the group (preserving_write_keeps_a_supplementary_group).

Commands run locally:

• cargo fmt --all -- --check: clean.
• cargo clippy --workspace --all-features -- -D warnings: clean.
• cargo test -p socket-patch-core --all-features --lib: 6536 passed, 4 failed. cargo test -p socket-patch-cli --all-features --lib --test in_process_redirect --test in_process_redirect_pipenv: 3 failed in in_process_redirect. All 7 failures are read-only-directory tests that root ignores. They fail the same way on main as root, and they pass as a non-root user against this branch. The sandbox runs as root.
• The full cargo test --workspace build ran out of the sandbox's disk allowance, so CI runs the rest.

Review notes (/code-review high)

Fixed: the WriteOpts::preserve_mode doc, a root-only test that assumed a new file's group, and the contract's scope wording. Not fixed, on purpose:

• .socket/manifest.json and the vendored ledgers use the plain (non-preserving) writer, and files a root run creates (a new .npmrc, .socket/vendor/ dirs) are root-owned. The issue is about existing project files, and those writers don't preserve the mode either. Possible follow-up: give new files their parent directory's owner, as restore_file_permissions does for patched files.
• A symlinked destination gets its target's owner, the same source the mode already came from. The content comes from that same target, so nothing is exposed to a new owner.
• Failed fchown calls stay silent. Reporting them like OWNERSHIP_NOT_RESTORED_MARKER would need a warning channel through every writer.
• The mode is still set after the fsync. That ordering is older than this PR.

https://claude.ai/code/session_01XwH2o4SRLPwRpbu8QBzz2T

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

Labeled Ready for review (burn-down agent).

• Head: 1e945c50
• CI: 86/86 green (36 success, 50 skipped); mergeable, no conflicts; no CHANGELOG.md change.
• Bugbot: reviewed 1e945c50, no findings; 0 open review threads.
• Worth a look: keep_owner in crates/socket-patch-core/src/utils/fs.rs chowns the stage before the fsync and before the chmod (chown clears setuid/setgid); the new root-only test root_hosted_scan_keeps_the_lock_owner skips when not run as root.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[agent] Dequeued by e2e (ubuntu-latest, e2e_vex_build, pipenv:: --ignored, 2026.8.0): uv pip install -r pylock.toml fails with "The target environment is not supported by the lockfile". The failure isn't from this PR. The same leg is red on main's nightly run (39376690526), because uv 0.13.2+ refuses the environments = [] that Pipenv 2026.8.0 writes, and it passed in the pr-1605 queue group, which includes this PR's commit. I ported #1639's setup-uv pin to 0.13.1 in e68bb85 (it no-ops once #1639 lands). This push may have dismissed Wenxin Jiang (@Wenxin-Jiang)'s approval, so it needs re-approval before re-queueing.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

✅ 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 e68bb85. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Wenxin Jiang (@Wenxin-Jiang) this PR changed after your approval (on 1e945c50), so it needs another look before it goes into the merge queue.

Besides merges from main, the only new commit is e68bb85: the CI-only uv pin from #1639 (version: "0.13.1" on the setup-uv steps in ci.yml and pipenv-compatibility.yml, 12 added lines). main already carries the same pin, so git diff origin/main e68bb85 -- .github/ is empty, and the PR's diff against main is the same Rust change you approved. CI is green on e68bb85 and there are no open threads. Re-approving sends it to the merge queue.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Ready for review re-confirmed at the new head (burn-down agent).

• Head: e68bb85. It is mergeable with no conflicts, and CHANGELOG.md is not touched.
• CI: 55/55 green (36 success, 19 skipped). No main-wide failures.
• Bugbot: reviewed e68bb85, no findings; no open review threads.
• Reviewer: the head moved after the earlier approval, so it needs a fresh approval before it can go into the merge queue.
• Slack: not announced; this run has no Slack send tool.

---

Generated by Claude Code