Skip to content

Commit 2b62944

Browse files
authored
test(openemr-cmd): bats permission-case coverage — probe matrix + #836 reproducer (#839)
## Summary Hermetic bats coverage for the permission scenarios that have been the most active bug surface in the recent worktree-cmd PRs (#833, #836, #838). Pure test scaffolding — no production code changes. ## What's added Four cases in `tests/bats/openemr-cmd/mountpoint_precreate.bats`: 1. **Empty non-writable + non-writable parent → probe fails.** The probe-loosening rule from #838 ("empty + parent writable = rmdir-able") requires BOTH conditions; guards against accidentally accepting a case where the parent's perms would block the rmdir too. 2. **Unreadable dir (mode 0000) → probe fails conservatively.** When we can't `[[ -r && -x ]]` the dir, we can't verify emptiness via `find`; the probe must treat it as blocking (rm could silently leave content behind otherwise). 3. **Multiple unwritable dirs (one empty-removable, one non-empty) → probe fails on the non-empty one.** Early-termination correctness: the probe must evaluate each unwritable dir against the empty-removable rule rather than failing on the first unwritable it sees regardless of removability. 4. **Reproducer for the multi-3 CI failure in #836:** stage `ccdaservice/packages/oe-cqm-service/node_modules` as empty 0555 — the exact path + state that broke pre-loosening. Confirms BOTH that pre-create populates the path AND that the loosened probe accepts the simulated post-volume-purge state. Pins the bug as a regression guard against future drift. ## Why now The permission-bug class has surfaced repeatedly across recent PRs (apache uid mismatch, docker daemon root-owned mount-points, probe over-strictness). Each fix added defensive logic; this PR turns those scenarios into bats that fail loudly if any future refactor breaks the invariants. The reproducer in particular locks down the specific bug from #836 so it can't silently regress. ## What this PR can't pin The bats simulates the FILESYSTEM STATE (root-owned-empty via chmod 0555/0000) but not the CAUSE (docker daemon creating dirs as root, apache chowning bind mount). The real-docker e2e job in `test-bats-openemr-cmd-real-docker.yml` covers the cause-side end-to-end. The two are complementary. ## Test plan - [ ] BATS openemr-cmd (ubuntu-22.04) and (macos-14) pass — the new 4 tests + existing 12 in mountpoint_precreate.bats - [ ] No other workflow effects (no production code changes; the existing real-docker e2e jobs run unchanged) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved handling of `worktree remove` when mount points or directories are not writable. * Added coverage ensuring the “empty directory” exemption is not used when the parent is also non-writable. * Added conservative behavior checks for effectively unreadable/unenterable directories, preventing unsafe verification. * Strengthened multi-candidate evaluation when multiple unwritable directories exist, ensuring the correct failure is surfaced. * Reproduced and protected a regression so removal succeeds for an empty, non-writable mount point. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
1 parent 82bf49f commit 2b62944

1 file changed

Lines changed: 99 additions & 0 deletions

File tree

tests/bats/openemr-cmd/mountpoint_precreate.bats

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,3 +239,102 @@ oc_remove() {
239239
assert_output --partial "is not writable by you"
240240
assert_output --partial "probe-blocker"
241241
}
242+
243+
@test "remove probe: fails when empty non-writable dir has non-writable parent (rmdir would fail too)" {
244+
oc_add probe-empty-parent -b --env easy >/dev/null
245+
local wt="${TMP_PARENT}/openemr-wt-probe-empty-parent"
246+
# rmdir on an empty child requires write+execute on the PARENT. If
247+
# the parent is also non-writable, rm cannot rmdir the child — so
248+
# the probe loosening's "empty + parent writable" exemption must
249+
# NOT trigger here. Verify by chmod'ing both child and parent 0555.
250+
mkdir -p "${wt}/locked-parent/locked-child"
251+
chmod 0555 "${wt}/locked-parent/locked-child"
252+
chmod 0555 "${wt}/locked-parent"
253+
254+
run oc_remove probe-empty-parent <<< 'y'
255+
# Restore perms before assertion failure (child first, then parent,
256+
# since chmod on parent requires no special perms but we need to
257+
# walk into it to chmod child).
258+
chmod 0755 "${wt}/locked-parent" 2>/dev/null || true
259+
chmod 0755 "${wt}/locked-parent/locked-child" 2>/dev/null || true
260+
assert_failure
261+
assert_output --partial "is not writable by you"
262+
}
263+
264+
@test "remove probe: fails on opaque dir (mode 0000) — conservative since emptiness can't be verified" {
265+
oc_add probe-unreadable -b --env easy >/dev/null
266+
local wt="${TMP_PARENT}/openemr-wt-probe-unreadable"
267+
# 0000: strips all perms including owner. Even the test user
268+
# (the owner) can't read, write, or enter. [[ -w ]] returns
269+
# false; [[ -r ]] returns false. The probe's emptiness-check
270+
# branch is skipped (it requires r+x to walk the dir's contents
271+
# via find), so the dir is conservatively flagged as blocking.
272+
#
273+
# Note: 0700 wouldn't trip the probe here because the test user
274+
# owns the dir and chmod 0700 keeps owner rwx — `[[ -w ]]` would
275+
# return true for owner, never reaching the probe-fail branch.
276+
# 0000 is the minimum mode that strips owner-side r/w/x and
277+
# genuinely makes the dir "opaque" to the walker.
278+
mkdir -p "${wt}/opaque-dir"
279+
chmod 0000 "${wt}/opaque-dir"
280+
281+
run oc_remove probe-unreadable <<< 'y'
282+
chmod 0755 "${wt}/opaque-dir" 2>/dev/null || true
283+
assert_failure
284+
assert_output --partial "is not writable by you"
285+
assert_output --partial "opaque-dir"
286+
}
287+
288+
@test "remove probe: when multiple unwritable dirs exist (one empty-removable, one non-empty), fails on the non-empty one" {
289+
# Early-termination correctness: the probe walks find -type d in
290+
# an order we don't control. With the loosening, an empty-removable
291+
# dir should be skipped silently while the non-empty one still
292+
# triggers the fail. Both dirs present in the same worktree ensures
293+
# the probe doesn't stop at the first unwritable dir it sees —
294+
# it correctly evaluates each one against the empty-removable rule.
295+
oc_add probe-mixed -b --env easy >/dev/null
296+
local wt="${TMP_PARENT}/openemr-wt-probe-mixed"
297+
# Empty 0555 (would pass loosening alone)
298+
mkdir -p "${wt}/empty-blocker"
299+
chmod 0555 "${wt}/empty-blocker"
300+
# Non-empty 0555 (would always fail)
301+
mkdir -p "${wt}/full-blocker"
302+
touch "${wt}/full-blocker/payload"
303+
chmod 0555 "${wt}/full-blocker"
304+
305+
run oc_remove probe-mixed <<< 'y'
306+
chmod 0755 "${wt}/empty-blocker" 2>/dev/null || true
307+
chmod 0755 "${wt}/full-blocker" 2>/dev/null || true
308+
assert_failure
309+
assert_output --partial "is not writable by you"
310+
# The non-empty one is what the probe must surface; the empty
311+
# one would have been silently OK if it were alone.
312+
assert_output --partial "full-blocker"
313+
refute_output --partial "empty-blocker"
314+
}
315+
316+
@test "remove probe: reproducer for the multi-3 CI failure in #836 — empty 0555 at ccdaservice/.../node_modules" {
317+
# Pre-merge of #838, this exact scenario (docker daemon creates
318+
# ccdaservice/packages/oe-cqm-service/node_modules as root:root
319+
# 0755 when first attaching the named volume; after compose down
320+
# --volumes the host dir reappears empty + root-owned) caused
321+
# `worktree remove` to fail with "is not writable by you" even
322+
# though the actual rm could have succeeded (parent is writable,
323+
# dir is empty). The probe loosening + pre-create together fix
324+
# this; regression test pins both paths:
325+
# - The pre-created dir lives at the right path
326+
# - Even if it weren't pre-created and showed up as root-owned-
327+
# empty (chmod 0555 stand-in), the probe still accepts it
328+
oc_add probe-mp-bug -b --env easy >/dev/null
329+
local wt="${TMP_PARENT}/openemr-wt-probe-mp-bug"
330+
# Confirm pre-create did create the dir at the bug's exact path.
331+
[[ -d "${wt}/ccdaservice/packages/oe-cqm-service/node_modules" ]] \
332+
|| fail "pre-create did not create the bug's volume mount-point dir"
333+
# Simulate the post-volume-purge state: empty + non-writable.
334+
chmod 0555 "${wt}/ccdaservice/packages/oe-cqm-service/node_modules"
335+
336+
run oc_remove probe-mp-bug <<< 'y'
337+
chmod 0755 "${wt}/ccdaservice/packages/oe-cqm-service/node_modules" 2>/dev/null || true
338+
assert_success
339+
refute_output --partial "is not writable by you"
340+
}

0 commit comments

Comments
 (0)