perf(scheduler): switch FileCleanupTask to find -delete and add minimal path safety guards#967
Merged
jinbai340997 merged 6 commits intoJun 3, 2026
Conversation
StephenRi
reviewed
May 18, 2026
jinbai340997
added a commit
to jinbai340997/ROCK
that referenced
this pull request
May 27, 2026
…onsistency with blacklist terminology (PR alibaba#967 review)
jinbai340997
added a commit
to jinbai340997/ROCK
that referenced
this pull request
May 27, 2026
…onsistency with blacklist terminology (PR alibaba#967 review)
jinbai340997
force-pushed
the
feat/file-cleanup-perf-and-guards
branch
from
May 27, 2026 12:25
33033de to
0c01918
Compare
jinbai340997
added a commit
to jinbai340997/ROCK
that referenced
this pull request
May 27, 2026
…onsistency with blacklist terminology (PR alibaba#967 review)
jinbai340997
added a commit
to jinbai340997/ROCK
that referenced
this pull request
May 28, 2026
…onsistency with blacklist terminology (PR alibaba#967 review)
jinbai340997
force-pushed
the
feat/file-cleanup-perf-and-guards
branch
from
May 28, 2026 03:29
0c01918 to
b0d0689
Compare
StephenRi
approved these changes
Jun 1, 2026
…al path safety guards
Performance:
- Replace `find ... -exec rm -f {} +` with `find ... -delete`
- Replace `find ... -exec rmdir {} +` with `find ... -depth -type d -empty -delete`
- `-delete` calls unlink(2) directly without forking per-batch rm/rmdir;
on dirs with tens of thousands of files this is ~10x faster.
Safety guards:
- Add module-level `_DANGEROUS_PATHS` blacklist with 2 entries: "/" and
"/tmp/miniforge". The list is intentionally minimal — both are observed
incident sources (root wipe, uv runtime deletion). OS dirs like /etc /var
are NOT included: they're hypothetical mistakes, and a longer blacklist
raises the cost of changing the policy later.
- `FileCleanupTask.__init__` rejects target_dirs entries that match a
blacklist entry (exact or subtree) -> ValueError at config-load time.
Note: "/" is exact-match only — a naive subtree check would reject every
absolute path.
- `TargetDirConfig.from_raw` rejects empty / relative / `..` paths. The ".."
check happens BEFORE os.path.normpath, since normalize would collapse
"/data/../etc" to "/etc" and silently accept the traversal.
Compat:
- yaml format unchanged (str + dict still both accepted).
- Behaviour for legitimate paths (/data/logs, /data/sandbox_logs etc.)
unchanged — only the find action verb swapped.
Tests:
- New tests/unit/admin/scheduler/test_file_cleanup_task.py covers
from_raw validation, blacklist guard (root exact-match, miniforge
exact + subtree, /data/logs explicitly allowed), command generation
(-delete present, -exec rm absent), from_config backward-compat,
run_action happy + error paths. 31 tests, all passing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…onsistency with blacklist terminology (PR alibaba#967 review)
…_log_dir_quota is reached
jinbai340997
force-pushed
the
feat/file-cleanup-perf-and-guards
branch
from
June 1, 2026 08:45
5ee1550 to
ad5359f
Compare
…dir-quota cases from bad rebase
StephenRi
approved these changes
Jun 3, 2026
jinbai340997
added a commit
that referenced
this pull request
Jun 3, 2026
…onsistency with blacklist terminology (PR #967 review)
jake11-oho
pushed a commit
to jake11-oho/ROCK
that referenced
this pull request
Jun 10, 2026
…onsistency with blacklist terminology (PR alibaba#967 review)
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
2 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.
Summary
1 个 commit，4 块改动：
-exec rm -f {} +→-delete；-exec rmdir {} +→-depth -type d -empty -deleteTargetDirConfig.from_raw拒绝空 / 相对路径 / 含..的路径_DANGEROUS_PATHS = ("/", "/tmp/miniforge")—— 仅 2 条真实事故源tests/unit/admin/scheduler/test_file_cleanup_task.py覆盖以上 3项 + 既有逻辑回归
Files (3, ≤8)
rock/admin/scheduler/tasks/file_cleanup_task.py(modify)tests/unit/admin/scheduler/__init__.py(NEW, empty)tests/unit/admin/scheduler/test_file_cleanup_task.py(NEW)yaml 兼容
不破坏。同时支持
target_dirs: ["/path"]和target_dirs: [{"path": "...", "exclude_files": [...]}]。Test plan
uv run pytest tests/unit/admin/scheduler/test_file_cleanup_task.py -vuv run ruff check rock/admin/scheduler/tasks/file_cleanup_task.py tests/unit/admin/scheduler/test_file_cleanup_task.pycommand里-delete出现、-exec rm不出现fixes #966