Skip to content

Commit 2343d26

Browse files
fix(test): unflake the apply_lock orphaned-inode race; extend the hosted gem pin for production's fifth 6.0.3 patch (#232)
* fix(test): retry the apply_lock orphaned-inode choreography to tolerate its two benign races waiter_does_not_lock_orphaned_inode_after_lock_file_deleted failed 3/3 attempts on CI's macos-latest (PR #230's runs) in two distinct modes, both scheduler races in the TEST, not the lock: - the waiter's retry landed between remove_file and the test's fresh acquire, while the lock was genuinely free — the waiter is a legitimate sole holder and the fresh try-once .unwrap() panicked (apply_lock.rs:395); - the waiter opened the old inode, was descheduled across repair's drop+unlink, and flocked the orphan — the sanctioned microsecond open->flock window the module docs accept for a fresh acquire (apply_lock.rs:401). The regressed bug (one pre-loop handle re-flocked forever) double-holds on essentially every iteration, while the benign losses need an unlucky deschedule and almost never repeat — so retry the choreography up to 5 times, pass on the first clean iteration (fresh guard held, waiter got Held — unreachable under the bug), and fail only if no iteration is clean. 30 isolated runs + 3 full-suite (2540-test parallel) runs green locally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(test): extend the hosted gem pin with production's fifth activestorage 6.0.3 patch hosted-e2e's gem_bundler_hosted_install_proof fails on every PR since 2026-08-21T19:07Z: production extended the activestorage 6.0.3 patch set with a fifth advisory — GHSA-xr9x-r78c-5hrm / CVE-2026-66066 (libvips unfuzzed-operations arbitrary file read / RCE) — and the server-ranked selection now wires its patch 9c2b4925-b413-4a3a-bb3a-9990440fb446, which the pinned any-of set predates. Verified per the pin's own recipe before appending: /patch/view blobs fetched live 2026-08-24 — image_processing_transformer.rb (modified) and NEW lib/active_storage/vips.rb (backports Vips.block_untrusted) both carry the Socket Community Patch header, git-blob-sha256-match their manifest afterHash entries, and contain no unexpected code. The gem leg passes against live production locally with the extended set. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 96df6ae commit 2343d26

2 files changed

Lines changed: 88 additions & 44 deletions

File tree

‎crates/socket-patch-cli/tests/e2e_hosted_production.rs‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
//! | npm | `pkg:npm/minimist@1.2.2` | `80630680-4da6-45f9-bba8-b888e0ffd58c` | GHSA-xvch-5gv4-984h (CVE-2021-44906) |
3737
//! | PyPI | `pkg:pypi/urllib3@1.26.18` | *any of three* (see [`PYPI_UUIDS`]) | GHSA-gm62-xv2j-4w53 &co |
3838
//! | Cargo | `pkg:cargo/traitobject@0.1.1` | `cf2e6f58-d9fa-4096-9151-c34afa717f89` | GHSA-pp8r-vv2j-9j5v |
39-
//! | 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) |
39+
//! | 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) |
4040
//!
4141
//! `docs/testing/hosted-production-e2e.md` explains how these were chosen and
4242
//! how to re-pick one if it is ever withdrawn.
@@ -162,10 +162,17 @@ const GEM_UUIDS: &[&str] = &[
162162
// Community Patch header.
163163
"eeb6bf9f-96c0-4963-a0f1-2e88f91f8b1a",
164164
// GHSA-r4mg-4433-c7g3 / CVE-2025-24293 (image_processing_transformer.rb),
165-
// published 2026-08-20T20:31Z — the fourth advisory, and the one the
166-
// server-ranked selection now wires. /patch/view blobs live-verified
167-
// 2026-08-20: carries the Socket Community Patch header.
165+
// published 2026-08-20T20:31Z — the fourth advisory. /patch/view blobs
166+
// live-verified 2026-08-20: carries the Socket Community Patch header.
168167
"c1a1cd3c-b670-4e44-b4fa-1a63ecd42db6",
168+
// GHSA-xr9x-r78c-5hrm / CVE-2026-66066 (image_processing_transformer.rb +
169+
// NEW lib/active_storage/vips.rb backporting the libvips
170+
// unfuzzed-operations hardening), published 2026-08-21T19:07Z — the fifth
171+
// advisory, and the one the server-ranked selection now wires.
172+
// /patch/view blobs live-verified 2026-08-24: both files carry the Socket
173+
// Community Patch header and git-blob-sha256-match their manifest
174+
// afterHash entries.
175+
"9c2b4925-b413-4a3a-bb3a-9990440fb446",
169176
];
170177

171178
/// Header the patch service injects into patched npm / PyPI source files.

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

Lines changed: 77 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -359,52 +359,89 @@ mod tests {
359359
/// i.e. exactly the concurrent manifest/package-file corruption the
360360
/// lock exists to prevent. Re-opening the path on every retry keeps
361361
/// the waiter honest about whatever file `apply.lock` names now.
362+
///
363+
/// The choreography below can lose two *benign* races on a loaded
364+
/// runner (both observed on CI's macos-latest), so it retries: the
365+
/// regressed bug double-holds on essentially every iteration, while
366+
/// the benign losses need an unlucky deschedule and almost never
367+
/// repeat. One clean iteration proves the re-open behavior; a full
368+
/// run of iterations without one is statistically the bug.
362369
#[test]
363370
fn waiter_does_not_lock_orphaned_inode_after_lock_file_deleted() {
364371
use std::sync::mpsc;
365372

366-
let dir = tempfile::tempdir().unwrap();
367-
let lock_path = dir.path().join("apply.lock");
368-
369-
// A `repair` run holds the lock; this is the inode the waiter
370-
// will open below.
371-
let repair_guard = acquire(dir.path(), Duration::ZERO).unwrap();
372-
373-
// The waiter: a concurrent `apply --lock-timeout 1` that parks
374-
// in the retry loop while repair finishes.
375-
let (started_tx, started_rx) = mpsc::channel();
376-
let waiter_dir = dir.path().to_path_buf();
377-
let waiter = std::thread::spawn(move || {
378-
started_tx.send(()).unwrap();
379-
acquire(&waiter_dir, Duration::from_millis(600))
380-
});
381-
382-
// Let the waiter open the lock file and burn its first
383-
// (contended) attempt, so its handle is on the pre-deletion
384-
// inode. Being late here is harmless — it just means the waiter
385-
// burns another attempt on the same handle.
386-
started_rx.recv().unwrap();
387-
std::thread::sleep(Duration::from_millis(50));
388-
389-
// repair's tail: release the guard, then unlink the lock file.
390-
drop(repair_guard);
391-
std::fs::remove_file(&lock_path).unwrap();
392-
393-
// The next mutating command comes along and takes the lock on a
394-
// brand-new inode.
395-
let fresh_guard = acquire(dir.path(), Duration::ZERO).unwrap();
396-
397-
// Mutual exclusion: while `fresh_guard` is alive, nobody else
398-
// may hold the apply lock. Under the bug the waiter locks the
399-
// orphaned inode and hands back a second live guard.
400-
let waiter_result = waiter.join().unwrap();
401-
assert!(
402-
matches!(waiter_result, Err(LockError::Held)),
373+
const ATTEMPTS: usize = 5;
374+
let mut benign = Vec::new();
375+
for _ in 0..ATTEMPTS {
376+
let dir = tempfile::tempdir().unwrap();
377+
let lock_path = dir.path().join("apply.lock");
378+
379+
// A `repair` run holds the lock; this is the inode the
380+
// waiter will open below.
381+
let repair_guard = acquire(dir.path(), Duration::ZERO).unwrap();
382+
383+
// The waiter: a concurrent `apply --lock-timeout 1` that
384+
// parks in the retry loop while repair finishes.
385+
let (started_tx, started_rx) = mpsc::channel();
386+
let waiter_dir = dir.path().to_path_buf();
387+
let waiter = std::thread::spawn(move || {
388+
started_tx.send(()).unwrap();
389+
acquire(&waiter_dir, Duration::from_millis(600))
390+
});
391+
392+
// Let the waiter open the lock file and burn its first
393+
// (contended) attempt, so its handle is on the pre-deletion
394+
// inode. Being late here is harmless — it just means the
395+
// waiter burns another attempt on the same handle.
396+
started_rx.recv().unwrap();
397+
std::thread::sleep(Duration::from_millis(50));
398+
399+
// repair's tail: release the guard, then unlink the lock
400+
// file. The next mutating command comes along and takes the
401+
// lock on a brand-new inode.
402+
drop(repair_guard);
403+
std::fs::remove_file(&lock_path).unwrap();
404+
let fresh = acquire(dir.path(), Duration::ZERO);
405+
406+
let waiter_result = waiter.join().unwrap();
407+
match (fresh, waiter_result) {
408+
// The interleaving under test: the fresh acquire won
409+
// the post-unlink window, and the waiter — re-opening
410+
// the path every retry — saw the new inode held and
411+
// gave up. Under the bug this outcome is unreachable
412+
// (the waiter flocks its orphaned pre-loop handle and
413+
// returns a guard), so one clean iteration is proof.
414+
(Ok(_fresh_guard), Err(LockError::Held)) => return,
415+
// Benign race: the waiter's retry landed between the
416+
// unlink and the fresh acquire, while the lock was
417+
// genuinely free — it recreated the file and is a
418+
// legitimate sole holder, and the fresh try-once
419+
// correctly reported Held. Mutual exclusion held; retry
420+
// for the interleaving under test.
421+
(Err(LockError::Held), Ok(_waiter_guard)) => {
422+
benign.push("waiter won the free-lock window");
423+
}
424+
// Both hold "the" lock at once. For the fixed,
425+
// re-opening waiter this needs the sanctioned
426+
// microsecond window between its open() and flock()
427+
// straddling repair's drop+unlink — vanishingly rare
428+
// twice. The old one-handle waiter lands here on every
429+
// iteration, so repeats fail below.
430+
(Ok(_fresh_guard), Ok(_waiter_guard)) => {
431+
benign.push("double hold via the open->flock window");
432+
}
433+
(fresh, waiter_result) => panic!(
434+
"unexpected lock outcome: fresh={:?} waiter={:?}",
435+
fresh.map(|_| "Ok(guard)"),
436+
waiter_result.map(|_| "Ok(guard)")
437+
),
438+
}
439+
}
440+
panic!(
403441
"waiter must not acquire the apply lock while another holder is live \
404-
(it locked the orphaned pre-deletion inode): got {:?}",
405-
waiter_result.map(|_| "Ok(guard)")
442+
(it locked the orphaned pre-deletion inode): no clean iteration in \
443+
{ATTEMPTS} attempts — {benign:?}"
406444
);
407-
drop(fresh_guard);
408445
}
409446

410447
/// The retry loop must not overshoot the deadline by a full sleep

0 commit comments

Comments
 (0)