Skip to content

Avoid recursive container stub cleanup#1113

Merged
tw93 merged 1 commit into
tw93:mainfrom
M-Hassan-Raza:audit/cleanup-opportunities
Jun 13, 2026
Merged

Avoid recursive container stub cleanup#1113
tw93 merged 1 commit into
tw93:mainfrom
M-Hassan-Raza:audit/cleanup-opportunities

Conversation

@M-Hassan-Raza

Copy link
Copy Markdown
Contributor

Summary:
Change orphaned container-stub cleanup to remove only the verified metadata plist, then rmdir the container instead of recursively deleting the directory.

Why:
The old path verified a metadata-only container, then ran rm -rf on the directory. If anything appeared after verification, that recursive delete could remove content that was no longer part of the verified stub.

Validation:
./scripts/check.sh --format
./scripts/check.sh
env -u NO_COLOR MOLE_TEST_NO_AUTH=1 bats tests/clean_apps.bats
MOLE_TEST_NO_AUTH=1 bats --filter "clean_orphaned_container_stubs preserves content that appears during removal" tests/clean_apps.bats
env -u NO_COLOR MOLE_TEST_NO_AUTH=1 ./scripts/test.sh

@M-Hassan-Raza M-Hassan-Raza marked this pull request as ready for review June 13, 2026 08:53
@M-Hassan-Raza M-Hassan-Raza requested a review from tw93 as a code owner June 13, 2026 08:53
@tw93 tw93 merged commit 9f13abf into tw93:main Jun 13, 2026
9 checks passed
@tw93

tw93 commented Jun 13, 2026

Copy link
Copy Markdown
Owner

@M-Hassan-Raza Merged, thanks. Replacing the rm -rf with a targeted rm -f on the metadata plist plus a rmdir is the right call: if anything repopulates the container during the mdfind/whitelist window, rmdir refuses the non-empty directory and the new content survives instead of getting recursively deleted.

The regression test that injects raced content and asserts it's preserved is a nice touch, it fails on the old path and passes on the new one. Verified locally with the format check and the full clean_apps.bats suite (28/28 green).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants