Conversation
* fix(security): confine and anchor every cleanup delete The dirs were authorized once at enqueue, but each destructive op still resolved its own path, so a symlink swap between scan and mutation could send shutil.move/os.remove/shutil.rmtree outside the roots. Every sink now re-confines its target immediately before acting, and the two delete sinks go further: they open the confined PARENT with O_NOFOLLOW, pin it by (st_dev, st_ino) the way prune_old_backups does, and operate by name through that descriptor — so the name cannot be swapped for a link after the check. rmtree takes dir_fd since CPython 3.11, so recursive delete is anchored too, no reimplementation. Moves stay at re-confinement only: shutil.move has no dir_fd and hardening it would mean reimplementing the cross-filesystem fallback. Deletes can no longer escape the authorized root; the leaf name is still resolved at the syscall, which is the honest remaining gap. * fix(security): open the delete parent one component at a time O_NOFOLLOW guards only the last component and the inode pin re-resolves the same string, so an intermediate symlink swapped in after confinement landed the descriptor outside the roots. Walk down from the containing root instead. Empty-dir sweep gains a base_dir floor for the bloat pass, which passes no config. Refusals aggregate to one line per pass. * fix(security): anchor moves and rmdir, reload config per item os.rename takes src_dir_fd/dst_dir_fd, so same-filesystem moves are now anchored at both ends; shutil.move survives only as the cross-device fallback, the one path that still re-resolves strings. rmdir gets the same descriptor treatment when a config is available. Both executors re-read config per destructive item instead of trusting a pre-loop snapshot. Makefile now gates at 3.11, which is what rmtree(dir_fd=) needs. * test: bind de-authorization asserts to the refusing pass Share the tightening-config stub as a conftest fixture. assert errors also passed on an empty-dir-sweep refusal; key on the pass label and count.
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
1 participant
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.
@coderabbitai ignore
develophas drifted behindmain(releases, fixes, dependency bumps). Do not merge this PR — it reports the drift, it does not fix it.developrequires branches be up to date, andhead:maincan never satisfy that without pulling develop's extension files into main, which the branch invariant forbids. Squash or rebase would also leavemainunreachable fromdevelop, so this workflow would just open another PR next push.Sync locally instead:
Then verify
git diff main developis added extension files plusdeploy/docker/Dockerfileonly. GitHub marks this PR merged on its own once develop contains main's tip. Opened by the sync-develop workflow.