(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
- Symlinked inputdata root with an in-tree item — currently raises, should
be accepted.
- An item genuinely outside the root — must still be rejected, so the
fix doesn't silently disable the guard.
- An in-tree item that is already a symlink pointing out of the tree —
must still be accepted (the trap above).
- A
target_root under the inputdata root via a symlinked path — must
still be rejected.
- End-to-end via subprocess from inside a symlinked root, asserting rc 0,
a correct symlink, and no traceback in stderr.
(Authored by Claude)
Summary
relink.pymixes lexical and physical path handling. When--inputdata-rootis 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
ArgumentTypeErrorescapes as atraceback instead of a clean error message.
Reproduction
Result:
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) returnsos.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 doesPath(item).is_relative_to(args.inputdata_root)— a purely lexicalcomparison 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:387for the"
target_rootmust NOT be underinputdata_root" check, which means thatguard can also be bypassed by passing a symlinked path.
Because the
ArgumentTypeErroris raised fromprocess_argsrather thanfrom an argparse
type=callable, argparse never catches it, so itsurfaces as a traceback rather than a usage error.
Why the test suite doesn't catch this
tests/relink/passes today only becauseTMPDIRon the machine where itruns 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:
rimporthandles this case correctly — it resolvesboth 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:380andrelink.py:387.Trap to avoid: do not simply call
Path(item).resolve().relink.pyexists 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:
That canonicalizes every directory component (the actual problem) while
preserving the leaf symlink (which relink must not follow). For
target_rootandinputdata_root— directories, not files — a plain.resolve()is correct.Test coverage the fix needs
be accepted.
fix doesn't silently disable the guard.
must still be accepted (the trap above).
target_rootunder the inputdata root via a symlinked path — muststill be rejected.
a correct symlink, and no traceback in stderr.