fix(cron): re-validate stale cron-output entries before deletion - #39067
Merged
Conversation
) quick() and dry_run() previously trusted the stored category from tracked.json without re-validating at delete time. Stale entries from before #34840 could carry category="cron-output" for cron control-plane paths (e.g. cron/jobs.json), causing quick() to delete the live scheduler registry. Fix: - Fix guess_category() to only classify cron/output/** as cron-output (was classifying ALL cron/* paths, missing the #34840 fix). - Re-validate cron-output entries via guess_category() at delete time in quick() and dry_run(); stale entries that are no longer classified as cron-output are skipped and removed from tracked.json. - Add _is_protected_cron_path() as a hard defense-in-depth guard that blocks deletion of cron/cronjobs directories and known control-plane files (jobs.json, .tick.lock) regardless of stored category. - Update test_cron_subtree_categorised to match fixed guess_category (only cron/output/* is cron-output, not all of cron/). Tests: add 5 regression tests in TestStaleCronEntryMigration.
Contributor
🔎 Lint report:
|
alt-glitch
added
type/bug
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
disk-cleanup quick()can no longer delete the live cron scheduler registry (cron/jobs.json) via staletracked.jsonentries.Root cause:
guess_category()was fixed in #34840 to only classifycron/output/**ascron-output, buttracked.jsonentries written before that fix still carrycategory: "cron-output"for control-plane paths likecron/jobs.jsonand.tick.lock.quick()/dry_run()trusted the stored category at delete time and would wipe the live registry.Changes
plugins/disk-cleanup/disk_cleanup.py:quick()+dry_run(): re-runguess_category()forcron-outputentries at delete time; stale entries that no longer match are skipped and dropped from tracking._is_protected_cron_path()defense-in-depth guard — never deletescron/,cronjobs/,jobs.json, or.tick.lockregardless of stored category.tests/plugins/test_disk_cleanup_plugin.py: 5 regression tests (TestStaleCronEntryMigration).The contributor's
guess_category()hunk was already onmain(#34840), so this salvage keeps only the genuinely-new delete-time re-validation + protected-path guard.Validation
cron-outputentry for…cron/jobs.json(live registry)cron/.tick.lockcron/output/run.md(real artifact)Verified live: ran
quick()against a real staletracked.jsonmislabelingcron/jobs.jsonascron-output— registry survived with content intact (important-daily-reportjob preserved), while a genuinecron/output/artifact was still cleaned (deleted: 1). 46/46test_disk_cleanup_plugin.pypass.Fixes #37721.
Salvaged from #37868 by @kyssta-exe — authorship preserved via cherry-pick.
Infographic