Skip to content

relink.py raises an unhandled traceback when --inputdata-root contains a symlinked component #29

Description

@samsrabin

(Authored by Claude)

Summary

relink.py mixes lexical and physical path handling. When
--inputdata-root is given as a path with a symlinked component, the
"item is under inputdata root" check fails for items that genuinely
are under the root, and the resulting ArgumentTypeError escapes as a
traceback instead of a clean error message.

Reproduction

# real tree + a symlink to it
mkdir -p /tmp/demo/real_root/sub /tmp/demo/target/sub
echo data > /tmp/demo/real_root/sub/f.txt
echo data > /tmp/demo/target/sub/f.txt
ln -s /tmp/demo/real_root /tmp/demo/link_root

cd /tmp/demo/link_root/sub
relink.py f.txt --target-root /tmp/demo/target --inputdata-root /tmp/demo/link_root

Result:

Traceback (most recent call last):
  ...
argparse.ArgumentTypeError: Item '/tmp/demo/real_root/sub/f.txt' not under
  inputdata root '/tmp/demo/link_root'

Expected: the item is under the inputdata root (via the symlink), so the
relink should succeed — and in any case a user error should never be a
traceback.

Cause

Two path conventions meet and disagree:

  • shared.validate_paths (shared.py:159) returns os.path.abspath(path).
    That is lexical, but it is built on os.getcwd(), which is physical
    the kernel does not retain the logical route you took through a symlink.
    So a relative positional becomes a physical absolute path.
  • process_args (relink.py:380) then does
    Path(item).is_relative_to(args.inputdata_root) — a purely lexical
    comparison against whatever string the user typed for --inputdata-root.

Physical item vs. logical root ⇒ the comparison fails.

The same lexical comparison appears at relink.py:387 for the
"target_root must NOT be under inputdata_root" check, which means that
guard can also be bypassed by passing a symlinked path.

Because the ArgumentTypeError is raised from process_args rather than
from an argparse type= callable, argparse never catches it, so it
surfaces as a traceback rather than a usage error.

Why the test suite doesn't catch this

tests/relink/ passes today only because TMPDIR on the machine where it
runs contains no symlink components. The tests are green for an
environmental reason, not because the code is correct. On GLADE, symlinked
paths are entirely plausible.

Note the asymmetry: rimport handles this case correctly — it resolves
both the inputdata root and the cwd before comparing them — so the two
tools currently disagree about what "under the inputdata root" means.

Suggested fix

Compare resolved paths on both sides at relink.py:380 and
relink.py:387.

Trap to avoid: do not simply call Path(item).resolve(). relink.py
exists to replace files with symlinks into the target root, so an item may
already be a symlink pointing outside the inputdata tree. Fully resolving
it would follow that leaf symlink and make a legitimately in-tree item look
out-of-tree — turning the fix into a new bug.

Resolve the parent, keep the leaf name:

item_path = Path(item)
item_resolved = item_path.parent.resolve() / item_path.name

That canonicalizes every directory component (the actual problem) while
preserving the leaf symlink (which relink must not follow). For
target_root and inputdata_root — directories, not files — a plain
.resolve() is correct.

Test coverage the fix needs

  1. Symlinked inputdata root with an in-tree item — currently raises, should
    be accepted.
  2. An item genuinely outside the root — must still be rejected, so the
    fix doesn't silently disable the guard.
  3. An in-tree item that is already a symlink pointing out of the tree —
    must still be accepted (the trap above).
  4. A target_root under the inputdata root via a symlinked path — must
    still be rejected.
  5. End-to-end via subprocess from inside a symlinked root, asserting rc 0,
    a correct symlink, and no traceback in stderr.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions