Skip to content

update-shape.py apply overwrites a pinned copy, contracts/shape-pin.yaml and the manifest in place (Rollback.write opens the path r+b), so one kept as a HARD link is rewritten through its other name, a file outside the root (or a cp -al snapshot of the whole root) that no check showed and no pin covers, while the README, the one path guarded with st_nlink > 1, is refused #422

Closed

Description

@brettheap

(lane openRepoShape-2, 2026-10-12T17:24Z)

Found by this lane's review of PR #347 (the fix for #286) at e965ab0, its finding 2: the PR keeps apply from writing through a SYMLINK and documents in its "Left out" that a HARD link is still written through. The defect is the same on main, so the PR neither causes it nor makes it worse. Reproduced on origin/main 92e29d9 (git 2.43.0, Python 3.12.3), local repositories only, a stub gh. Not claimed.

What is wrong. update-shape.py apply rewrites a pinned copy, contracts/shape-pin.yaml and the manifest by opening the path r+b and overwriting the bytes in place (Rollback.write, update-shape.py:1966, called for the copies at :2438, for the pin and the manifest at :2465 and :2468; the undo writes back the same way, :2095). A path that has another name on the filesystem, a hard link, is the same inode, so the other name changes too: a file outside the root that no check showed, that no pin covers, and that git does not record. The same file already knows this: for the README the reader refuses when st_nlink > 1 and says why (README_HARDLINK, update-shape.py:1139, the check at :1302): "a rewrite would change that file too, and git would record only this one". Only the README is guarded. The comment there says the other writes behave "the way it does for every copied file".

A hard-linked tree is not exotic: cp -al and the snapshot tools that dedupe with hard links (rsnapshot, rsync --link-dest) make a second name for every file of the root.

# $U: a checkout of openRepoShape at 92e29d9; $R/shape: a clone of it with one further commit that edits templates/assembly-root/scripts/validate-manifest.py;
# $C: an assembly root scaffolded from $R/shape's parent commit (local remotes), cloned. Local; no network.
ln "$C/scripts/validate-manifest.py" "$O/validate-manifest.py"          # a second name for a pinned copy, outside the root
sha256sum "$O/validate-manifest.py"                                     # e15c4745433a...
python3 "$U/update-shape.py" apply --root "$C" --upstream "$R/shape" --yes    # exit 0, `copied scripts/validate-manifest.py`
sha256sum "$O/validate-manifest.py"                                     # 4919e46c6e87...  <- the file outside the root changed
git -C "$C" status --porcelain                                          # M contracts/shape-pin.yaml, M project.yaml, M scripts/validate-manifest.py
cp -al "$C" "$R/yesterday"                                              # a hard-linked snapshot of the whole root, taken BEFORE the apply:
git -C "$R/yesterday" status --porcelain                                #   M contracts/shape-pin.yaml, M project.yaml, M scripts/validate-manifest.py  (the snapshot was rewritten)
the path hard-linked to a file outside the root apply --yes the other name
none (control) exit 0 n/a
scripts/validate-manifest.py (a pinned copy) exit 0, copied bytes changed (e15c4745433a to 4919e46c6e87)
contracts/shape-pin.yaml exit 0, re-pinned bytes changed
project.yaml exit 0, re-pinned bytes changed
README.md (the one path that is guarded) exit 0, kept README.md: another name points at these same bytes, untouched unchanged
the whole root cp -al-ed to a snapshot, then apply exit 0 the snapshot's copy, pin and manifest all moved: its git status is dirty, with nobody having touched it

What it should do. Refuse a rewrite of any path whose st_nlink > 1, as the README's reader does, before the first write, under a code of its own (update-rewrite-hardlink, beside #347's update-rewrite-symlink), naming the path and the remedy (replace the file with a copy of itself, which breaks the link, commit that, and run again). check should say so first, as PR #347 does for a link (copy-is-link), so the refusal is not a surprise. Writing by replace (a temporary file and os.replace) is the other way and the one the README's comment rejects for the ledger's sake: the other name would keep the old bytes and the link would be broken, and the undo could not put a broken link back.

How to test. A new file beside tests/test_update_shape_rewrite_symlink.py (PR #347) or tests/test_update_shape_add_symlink.py, with the harness they use (a scaffolded root, a standard clone with one edit):

  • a hard-linked pinned copy, then the pin, then the manifest, each with an upstream edit: apply exits 2 by name, nothing is written (the tree before and after, by lstat and bytes, is identical), and the other name's bytes are unchanged;
  • the control without a link applies; README.md hard-linked is still kept;
  • on Windows os.link needs no privilege on NTFS, so the test runs on the three CI OSes; skip it where the filesystem cannot link.

Not in this issue. Symlinks (#286, PR #347). A directory above the path that is a link (#347 covers contracts as a link). Rollback.write_added (--add): it creates exclusively, so it never writes through an existing name.

Related: #286 and PR #347 (the symlink case, and its "Left out"); #239 and PR #269 (--add through a link); the README's own README_HARDLINK (PR #152).

Lane: openreposhape-2 (openRepoShape-2)

Activity

  1. added this to the v1.1.0 milestone on Oct 14, 2026
  2. brettheap commented on Oct 14, 2026

    @brettheap
    ContributorAuthor

    v1.1.0 (triage of 2026-10-14): one pull request with #428, the update-shape rollback pair.

  3. brettheap commented on Oct 14, 2026

    @brettheap
    ContributorAuthor

    CLAIMED — lane openRepoShape-4, session 11c6c0ef-495a-4a05-94cf-811b7b0a1012@Eagle, 2026-10-14T16:48:16Z, for #422

    Logged in lanes/log/openRepoShape-4.md at 33a8f898e491e7ec844e525a30faca8698526d75.

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

    1. existing `CLAIMED —` comments on opensoft/openRepoShape#422:
       none
    2. `gh pr list --repo opensoft/openRepoShape --state all --search "#422"`, kept where the row or its body names `#422`:
       none
    3. `git ls-remote --heads git@github.com:opensoft/openRepoShape` branches naming `422` 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