Skip to content

fix(test): unflake the apply_lock orphaned-inode race; extend the hosted gem pin for production's fifth 6.0.3 patch #232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Mikola Lysenko (mikolalysenko) merged 2 commits into from
Aug 25, 2026
Merged

fix(test): unflake the apply_lock orphaned-inode race; extend the hosted gem pin for production's fifth 6.0.3 patch #232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7f2f19b
fix(test): retry the apply_lock orphaned-inode choreography to tolera…
mikolalysenko Aug 24, 2026
c7ff7b7
fix(test): extend the hosted gem pin with production's fifth activest…
mikolalysenko Aug 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions crates/socket-patch-cli/tests/e2e_hosted_production.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
//! | npm | `pkg:npm/minimist@1.2.2` | `80630680-4da6-45f9-bba8-b888e0ffd58c` | GHSA-xvch-5gv4-984h (CVE-2021-44906) |
//! | PyPI | `pkg:pypi/urllib3@1.26.18` | *any of three* (see [`PYPI_UUIDS`]) | GHSA-gm62-xv2j-4w53 &co |
//! | Cargo | `pkg:cargo/traitobject@0.1.1` | `cf2e6f58-d9fa-4096-9151-c34afa717f89` | GHSA-pp8r-vv2j-9j5v |
//! | gem | `pkg:gem/activestorage@6.0.3` | *any of* [`GEM_UUIDS`] (four today) | GHSA-m42x-37p3-fv5w (CVE-2020-8162), GHSA-w749-p3v6-hccq (CVE-2022-21831), GHSA-9xrj-h377-fr87 (CVE-2026-33195), GHSA-r4mg-4433-c7g3 (CVE-2025-24293) |
//! | gem | `pkg:gem/activestorage@6.0.3` | *any of* [`GEM_UUIDS`] (five today) | GHSA-m42x-37p3-fv5w (CVE-2020-8162), GHSA-w749-p3v6-hccq (CVE-2022-21831), GHSA-9xrj-h377-fr87 (CVE-2026-33195), GHSA-r4mg-4433-c7g3 (CVE-2025-24293), GHSA-xr9x-r78c-5hrm (CVE-2026-66066) |
//!
//! `docs/testing/hosted-production-e2e.md` explains how these were chosen and
//! how to re-pick one if it is ever withdrawn.
Expand Down Expand Up @@ -162,10 +162,17 @@ const GEM_UUIDS: &[&str] = &[
// Community Patch header.
"eeb6bf9f-96c0-4963-a0f1-2e88f91f8b1a",
// GHSA-r4mg-4433-c7g3 / CVE-2025-24293 (image_processing_transformer.rb),
// published 2026-08-20T20:31Z — the fourth advisory, and the one the
// server-ranked selection now wires. /patch/view blobs live-verified
// 2026-08-20: carries the Socket Community Patch header.
// published 2026-08-20T20:31Z — the fourth advisory. /patch/view blobs
// live-verified 2026-08-20: carries the Socket Community Patch header.
"c1a1cd3c-b670-4e44-b4fa-1a63ecd42db6",
// GHSA-xr9x-r78c-5hrm / CVE-2026-66066 (image_processing_transformer.rb +
// NEW lib/active_storage/vips.rb backporting the libvips
// unfuzzed-operations hardening), published 2026-08-21T19:07Z — the fifth
// advisory, and the one the server-ranked selection now wires.
// /patch/view blobs live-verified 2026-08-24: both files carry the Socket
// Community Patch header and git-blob-sha256-match their manifest
// afterHash entries.
"9c2b4925-b413-4a3a-bb3a-9990440fb446",
];

/// Header the patch service injects into patched npm / PyPI source files.
Expand Down
117 changes: 77 additions & 40 deletions crates/socket-patch-core/src/patch/apply_lock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,52 +359,89 @@ mod tests {
/// i.e. exactly the concurrent manifest/package-file corruption the
/// lock exists to prevent. Re-opening the path on every retry keeps
/// the waiter honest about whatever file `apply.lock` names now.
///
/// The choreography below can lose two *benign* races on a loaded
/// runner (both observed on CI's macos-latest), so it retries: the
/// regressed bug double-holds on essentially every iteration, while
/// the benign losses need an unlucky deschedule and almost never
/// repeat. One clean iteration proves the re-open behavior; a full
/// run of iterations without one is statistically the bug.
#[test]
fn waiter_does_not_lock_orphaned_inode_after_lock_file_deleted() {
use std::sync::mpsc;

let dir = tempfile::tempdir().unwrap();
let lock_path = dir.path().join("apply.lock");

// A `repair` run holds the lock; this is the inode the waiter
// will open below.
let repair_guard = acquire(dir.path(), Duration::ZERO).unwrap();

// The waiter: a concurrent `apply --lock-timeout 1` that parks
// in the retry loop while repair finishes.
let (started_tx, started_rx) = mpsc::channel();
let waiter_dir = dir.path().to_path_buf();
let waiter = std::thread::spawn(move || {
started_tx.send(()).unwrap();
acquire(&waiter_dir, Duration::from_millis(600))
});

// Let the waiter open the lock file and burn its first
// (contended) attempt, so its handle is on the pre-deletion
// inode. Being late here is harmless — it just means the waiter
// burns another attempt on the same handle.
started_rx.recv().unwrap();
std::thread::sleep(Duration::from_millis(50));

// repair's tail: release the guard, then unlink the lock file.
drop(repair_guard);
std::fs::remove_file(&lock_path).unwrap();

// The next mutating command comes along and takes the lock on a
// brand-new inode.
let fresh_guard = acquire(dir.path(), Duration::ZERO).unwrap();

// Mutual exclusion: while `fresh_guard` is alive, nobody else
// may hold the apply lock. Under the bug the waiter locks the
// orphaned inode and hands back a second live guard.
let waiter_result = waiter.join().unwrap();
assert!(
matches!(waiter_result, Err(LockError::Held)),
const ATTEMPTS: usize = 5;
let mut benign = Vec::new();
for _ in 0..ATTEMPTS {
let dir = tempfile::tempdir().unwrap();
let lock_path = dir.path().join("apply.lock");

// A `repair` run holds the lock; this is the inode the
// waiter will open below.
let repair_guard = acquire(dir.path(), Duration::ZERO).unwrap();

// The waiter: a concurrent `apply --lock-timeout 1` that
// parks in the retry loop while repair finishes.
let (started_tx, started_rx) = mpsc::channel();
let waiter_dir = dir.path().to_path_buf();
let waiter = std::thread::spawn(move || {
started_tx.send(()).unwrap();
acquire(&waiter_dir, Duration::from_millis(600))
});

// Let the waiter open the lock file and burn its first
// (contended) attempt, so its handle is on the pre-deletion
// inode. Being late here is harmless — it just means the
// waiter burns another attempt on the same handle.
started_rx.recv().unwrap();
std::thread::sleep(Duration::from_millis(50));

// repair's tail: release the guard, then unlink the lock
// file. The next mutating command comes along and takes the
// lock on a brand-new inode.
drop(repair_guard);
std::fs::remove_file(&lock_path).unwrap();
let fresh = acquire(dir.path(), Duration::ZERO);

let waiter_result = waiter.join().unwrap();
match (fresh, waiter_result) {
// The interleaving under test: the fresh acquire won
// the post-unlink window, and the waiter — re-opening
// the path every retry — saw the new inode held and
// gave up. Under the bug this outcome is unreachable
// (the waiter flocks its orphaned pre-loop handle and
// returns a guard), so one clean iteration is proof.
(Ok(_fresh_guard), Err(LockError::Held)) => return,
// Benign race: the waiter's retry landed between the
// unlink and the fresh acquire, while the lock was
// genuinely free — it recreated the file and is a
// legitimate sole holder, and the fresh try-once
// correctly reported Held. Mutual exclusion held; retry
// for the interleaving under test.
(Err(LockError::Held), Ok(_waiter_guard)) => {
benign.push("waiter won the free-lock window");
}
// Both hold "the" lock at once. For the fixed,
// re-opening waiter this needs the sanctioned
// microsecond window between its open() and flock()
// straddling repair's drop+unlink — vanishingly rare
// twice. The old one-handle waiter lands here on every
// iteration, so repeats fail below.
(Ok(_fresh_guard), Ok(_waiter_guard)) => {
benign.push("double hold via the open->flock window");
}
(fresh, waiter_result) => panic!(
"unexpected lock outcome: fresh={:?} waiter={:?}",
fresh.map(|_| "Ok(guard)"),
waiter_result.map(|_| "Ok(guard)")
),
}
}
panic!(
"waiter must not acquire the apply lock while another holder is live \
(it locked the orphaned pre-deletion inode): got {:?}",
waiter_result.map(|_| "Ok(guard)")
(it locked the orphaned pre-deletion inode): no clean iteration in \
{ATTEMPTS} attempts — {benign:?}"
);
drop(fresh_guard);
}

/// The retry loop must not overshoot the deadline by a full sleep
Expand Down
Loading