Skip to content

Commit fda71d8

Browse files
mikolalysenkoclaude
andcommitted
Merge origin/main into agent/v5-pypi-pep440-lockonly
Picks up #1351 (Go read-only module cache); no conflicts. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2 parents 09be73f + ba64c7f commit fda71d8

2 files changed

Lines changed: 108 additions & 4 deletions

File tree

‎crates/socket-patch-core/src/patch/copy_tree.rs‎

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,12 @@ fn to_io<E: std::fmt::Display>(e: E) -> std::io::Error {
1818
/// Directories are created fresh (writable, subject to umask) rather than
1919
/// mirroring the cache's read-only modes, so the copy can be patched and later
2020
/// removed without a chmod dance. File *contents* are copied via
21-
/// `std::fs::copy`, which also carries the source's mode bits (often `0o444` in
22-
/// the cache); the downstream apply pipeline replaces files via stage +
23-
/// rename (no write grant needed), and
24-
/// [`remove_tree`] relaxes perms on cleanup. Symlinks / specials are skipped —
21+
/// `std::fs::copy`, which also carries the source's permissions (`0o444` /
22+
/// the Windows read-only attribute in Go's module cache), so each copied file
23+
/// that lands read-only is then granted owner-write: the copy is a private,
24+
/// freshly created inode, and the apply pipeline's stage + rename cannot
25+
/// replace a read-only destination on Windows (`MoveFileEx` refuses it with
26+
/// `ERROR_ACCESS_DENIED`, #346). [`remove_tree`] relaxes perms on cleanup. Symlinks / specials are skipped —
2527
/// crates.io registry and Go module-cache sources contain none, and copying a
2628
/// dangling link would be unsafe.
2729
pub(crate) async fn fresh_copy(
@@ -78,11 +80,35 @@ fn copy_tree_blocking(src: &Path, dst: &Path, skip_file_name: Option<&str>) -> s
7880
}
7981
}
8082
std::fs::copy(entry.path(), &target)?;
83+
make_owner_writable(&target)?;
8184
}
8285
}
8386
Ok(())
8487
}
8588

89+
/// Grant owner-write on a file [`copy_tree_blocking`] just created, when
90+
/// the copy carried a read-only mode/attribute over from its source. Only
91+
/// ever called on the fresh inode `std::fs::copy` wrote, never on a source
92+
/// or a link, so no shared inode's permissions change.
93+
fn make_owner_writable(path: &Path) -> std::io::Result<()> {
94+
let mut perms = std::fs::metadata(path)?.permissions();
95+
if !perms.readonly() {
96+
return Ok(());
97+
}
98+
#[cfg(unix)]
99+
{
100+
use std::os::unix::fs::PermissionsExt;
101+
perms.set_mode(perms.mode() | 0o200);
102+
}
103+
#[cfg(not(unix))]
104+
{
105+
// Windows: clears the read-only attribute, nothing else.
106+
#[allow(clippy::permissions_set_readonly_false)]
107+
perms.set_readonly(false);
108+
}
109+
std::fs::set_permissions(path, perms)
110+
}
111+
86112
/// The previous [`copy_tree_blocking`], which created every file's parent,
87113
/// kept as the equivalence oracle.
88114
#[cfg(test)]
@@ -113,6 +139,7 @@ fn copy_tree_blocking_reference(
113139
std::fs::create_dir_all(p)?;
114140
}
115141
std::fs::copy(entry.path(), &target)?;
142+
make_owner_writable(&target)?;
116143
}
117144
}
118145
Ok(())
@@ -513,6 +540,37 @@ mod tests {
513540
fs::set_permissions(src.path().join("ro"), fs::Permissions::from_mode(0o755)).unwrap();
514541
}
515542

543+
/// #346: a read-only source file (Go's module cache; the Windows
544+
/// read-only attribute) yields an owner-writable copy, so the apply
545+
/// pipeline's stage + rename can replace it on every platform. The
546+
/// source keeps its read-only permission.
547+
#[tokio::test]
548+
async fn fresh_copy_files_are_writable_even_from_readonly_source() {
549+
let src = tempfile::tempdir().unwrap();
550+
let dst = tempfile::tempdir().unwrap();
551+
let d = dst.path().join("copy");
552+
let f = src.path().join("lib.go");
553+
fs::write(&f, b"package lib\n").unwrap();
554+
let mut ro = fs::metadata(&f).unwrap().permissions();
555+
ro.set_readonly(true);
556+
fs::set_permissions(&f, ro).unwrap();
557+
558+
fresh_copy(src.path(), &d, None).await.unwrap();
559+
560+
let copied = fs::metadata(d.join("lib.go")).unwrap().permissions();
561+
assert!(!copied.readonly(), "the copy must be writable: {copied:?}");
562+
#[cfg(unix)]
563+
assert_eq!(copied.mode() & 0o777, 0o644, "only owner-write is added");
564+
assert!(fs::metadata(&f).unwrap().permissions().readonly());
565+
assert_eq!(fs::read(d.join("lib.go")).unwrap(), b"package lib\n");
566+
567+
// Let the temp dir clean up on Windows.
568+
let mut rw = fs::metadata(&f).unwrap().permissions();
569+
#[allow(clippy::permissions_set_readonly_false)]
570+
rw.set_readonly(false);
571+
fs::set_permissions(&f, rw).unwrap();
572+
}
573+
516574
#[cfg(unix)]
517575
#[tokio::test]
518576
async fn remove_tree_does_not_follow_symlink_out_of_tree() {

‎crates/socket-patch-core/src/patch/redirect/golang_local.rs‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -874,6 +874,52 @@ mod tests {
874874
assert_eq!(e.version.as_deref(), Some(VERSION));
875875
}
876876

877+
/// #346: Go extracts module-cache files read-only (on Windows, the
878+
/// read-only attribute). The copy must still be patchable — Windows'
879+
/// rename refuses to replace a read-only destination — and the cache
880+
/// itself must stay read-only and pristine.
881+
#[tokio::test]
882+
async fn test_apply_redirect_over_a_read_only_module_cache() {
883+
let (dir, blobs, pristine, files, _after) = fixture().await;
884+
let root = dir.path();
885+
let set_readonly = |readonly: bool| {
886+
for name in ["bar.go", "go.mod"] {
887+
let path = pristine.join(name);
888+
let mut perms = std::fs::metadata(&path).unwrap().permissions();
889+
#[allow(clippy::permissions_set_readonly_false)]
890+
perms.set_readonly(readonly);
891+
std::fs::set_permissions(&path, perms).unwrap();
892+
}
893+
};
894+
set_readonly(true);
895+
let sources = PatchSources::blobs_only(&blobs);
896+
897+
for base in [GO_PATCHES_DIR, ".socket/vendor/golang/u"] {
898+
let result = apply_go_redirect(
899+
PURL,
900+
MODULE,
901+
VERSION,
902+
&pristine,
903+
root,
904+
base,
905+
&files,
906+
&sources,
907+
false,
908+
MismatchPolicy::Warn,
909+
)
910+
.await;
911+
assert!(result.success, "{base}: apply failed: {:?}", result.error);
912+
let copy = root.join(base).join("github.com/foo/bar@v1.4.2");
913+
assert_eq!(std::fs::read(copy.join("bar.go")).unwrap(), PATCHED);
914+
}
915+
assert_eq!(std::fs::read(pristine.join("bar.go")).unwrap(), PRISTINE);
916+
assert!(std::fs::metadata(pristine.join("bar.go"))
917+
.unwrap()
918+
.permissions()
919+
.readonly());
920+
set_readonly(false);
921+
}
922+
877923
#[tokio::test]
878924
async fn test_apply_is_idempotent_byte_for_byte() {
879925
let (dir, blobs, pristine, files, _after) = fixture().await;

0 commit comments

Comments
 (0)