Skip to content

adopt-project.py: the clone plan or check makes and removes is claimed and checked by path, so a shared, writable --work-dir lets another local user turn the removal onto a directory of theirs, two runs on one --work-dir remove each other's clone, a skipped removal says nothing, and a source-readonly that is a file ends in a traceback #803

Closed

Description

@brettheap

Goal: the clone that plan and check make, and later remove, is claimed by this run and removed only as what this run made. A --work-dir that another user can write in is refused before anything is made there. A removal that is skipped says so. A source-readonly the run cannot use is refused by name.

Filed by lane openRepoShape-1 on Brett Heap's word of 2026-10-17T10:21:32Z, under lane 3's v1.1.1 rule (#473, issuecomment-6216697223). It comes from #457's lines L1-97, L1-98, L1-99 and L1-102 (#457, comment), which #652's review found, and L1-65 (#457, comment). Measured on main bf1f099b with a stand-in gh.

Milestone: v1.1.1. This hardens the removal that #749 (#652) added. Ordinary use in a --work-dir the person owns does not reach L1-99's data loss. A --work-dir in a shared, writable place such as /tmp does, and #749 introduced the removal, so lane 1 hardens it.

Lines

  • L1-99 (b): the real-path check and the removal are two steps. I swapped W/source-readonly for a symlink to victim/ after the check, injecting the swap at the start of _remove_a_tree. check exited 0 and removed victim/<repo>, with the file in it.
    • Only another local user who can rename entries inside the work directory can open that window. The default work directory (mkdtemp, mode 0700) and a --work-dir this user owns are safe.
    • A shared, writable --work-dir where the other user made source-readonly/ first is not. That user can already swap the clone plan reads.
  • L1-99 (a): a skipped removal says nothing. With W/source-readonly a symlink to bigdisk/ before the run, check --source testorg/Thing --work-dir W exits 0 and keeps the clone at bigdisk/Thing. stderr is empty: the skip is a bare continue, while a failed removal prints a NOTE.
  • L1-98: two runs on one --work-dir.
    • Run A was check --plan P --source testorg/Thing --work-dir W, with a gh that fails once B's plan exists. Run B was plan --source testorg/Thing --work-dir W, started 1.5 s later.
    • B exited 0, and its plan recorded W/source-readonly/Thing. A exited 2 (source-unclonable) and removed B's clone and source-readonly/.
  • L1-97: a --source clone at the plan's own record.
    • Plan P recorded W/source-readonly/Thing with cloned: true, and that clone was then moved away. check --plan P --source testorg/Thing --work-dir W exits 0, clones there, then removes it.
    • The same run without --source keeps it.
    • The cost is one repeat clone.
  • L1-102 (Windows, read from the code): case. made_at is built from the constant source-readonly, and compared with os.path.realpath, which on Windows returns the disk's case. With a Source-Readonly already there, every clone silently skips removal.
  • L1-65: a --work-dir whose source-readonly is a file. plan, check and execute with --source acme/Atlas each exit 1 with FileExistsError: [Errno 17] File exists: '<work-dir>/source-readonly', at Source.clone's path.parent.mkdir(...), before any gh call.

Direction

  1. On POSIX, refuse a --work-dir, or a source-readonly/ in it, that this user does not own, or that others can write without the sticky bit. Refuse it by name, before anything is made there. This is the root cause of L1-99 (b).
  2. Claim the clone target with an exclusive os.mkdir before gh, and list only what this run's mkdir made (L1-98).
  3. Print a NOTE for every skipped removal, as for a failed one (L1-99 (a)). Compare with os.path.normcase on both sides (L1-102).
  4. Never list a --source clone whose target is the plan's own recorded path (L1-97).
  5. Catch OSError from the source-readonly mkdir and refuse it as adopt-work-dir-occupied or adopt-work-dir-unwritable, naming source-readonly (L1-65). Keep adopt-project.py: --source is read by its spelling, an org/repo in any spelling is cloned by its name, the plan records source.cloned, and a clone no plan records is removed, so a recorded clone that is gone or emptied is refused or cloned again for what it is (#652) #749's order: writable, occupied, NOTE, clone.
  6. Optionally, use fd-relative removal on Python 3.11 and later (shutil.rmtree(dir_fd=)). The repository supports Python 3.9.

Tests

  • The forced race (L1-98).
  • The symlinked source-readonly/ NOTE (L1-99 (a)).
  • A source-readonly/ owned by another user, or a --work-dir others can write without the sticky bit, refused before anything is made. Skip it where the test cannot create such a directory.
  • The in-process swap that removes victim/<repo> today removes nothing (L1-99 (b)).
  • The plan's own record kept (L1-97).
  • A file at source-readonly refused with no traceback, with the stub gh never called (L1-65).
  • A normcase comparison test with os.path patched (L1-102).

Overlaps: lane 1's #796 changes _source_spec, Source.open and Source.in_place. This issue changes Source.clone, _leave_the_clones, _remove_a_tree and the work-directory checks.

Lane: openreposhape-1 (openRepoShape-1)

🤖 Generated with Claude Code

Activity

  1. added this to the v1.1.1 milestone on Oct 17, 2026
  2. brettheap commented on Oct 17, 2026

    @brettheap
    ContributorAuthor

    CLAIMED — lane openRepoShape-1, session 300f8f46-004a-4e72-8587-4f192be2e108@Eagle, 2026-10-17T10:51:14Z, for #803

    Logged in lanes/log/openRepoShape-1.md at baceb00872d0013c71569ebefdf4c768da730f85.

    The three reads (lane-collision-protocol Rule 1):

    1. existing `CLAIMED —` comments on opensoft/openRepoShape#803:
       none
    2. `gh pr list --repo opensoft/openRepoShape --state all --search "#803"`, kept where the row or its body names `#803`:
       none
    3. `git ls-remote --heads git@github.com:opensoft/openRepoShape` branches naming `803` as a whole token:
       none
    
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions