Skip to content

rimport destroys the inputdata link when the path given goes through a symlink #30

Description

@samsrabin

(Identified and written by Claude; I haven't yet verified.)

Summary

If you name a file through a symlink that points into the inputdata tree, rimport
stages the file correctly but then replaces the original with a symlink pointing at
itself
. The published file becomes unreadable (ELOOP), and rimport exits 1.

The staged copy is fine, so no data is lost — but the path everyone reads from is broken
until someone repairs it by hand.

Impact

  • The inputdata path is left as a self-referential symlink: cat reports
    Too many levels of symbolic links, and Path.exists() returns False.
  • Anything resolving that path — a CESM run, a download check, a later rimport — sees a
    broken file.
  • It is silent about the real cause. The run reports Error relinking during rimport,
    which reads like a transient failure rather than a corrupted link.

Severity is bounded by two things: the staged copy under the staging root is intact and
correct, and the failure is detected (exit 1) rather than passing as success. It is a
recoverable break, not data loss.

Reproduction

W=$(mktemp -d)
mkdir -p "$W/inputdata/lnd" "$W/staging"
echo data > "$W/inputdata/lnd/a.nc"
ln -s "$W/inputdata" "$W/plink"          # a symlink pointing at the inputdata root

RIMPORT_STAGING="$W/staging" RIMPORT_SKIP_USER_CHECK=1 \
  rimport -inputdata "$W/inputdata" "$W/plink/lnd/a.nc"

Observed:

'<W>/plink/lnd/a.nc':
    [rimport] staged <W>/plink/lnd/a.nc -> <W>/staging/lnd/a.nc
    Deleted original file: <W>/plink/lnd/a.nc
    Created symbolic link: <W>/plink/lnd/a.nc -> <W>/staging/../plink/lnd/a.nc
    rimport: error processing <W>/plink/lnd/a.nc: Error relinking during rimport
EXIT=1

<W>/staging/../plink/lnd/a.nc normalizes to <W>/plink/lnd/a.nc, which is the symlink
that was just created. Afterwards:

$ ls -l "$W/inputdata/lnd/a.nc"
... a.nc -> <W>/staging/../plink/lnd/a.nc
$ cat "$W/inputdata/lnd/a.nc"
cat: ...: Too many levels of symbolic links
$ cat "$W/staging/lnd/a.nc"
data                                   # the staged copy is correct

Expected: either the same result as naming the file directly (staged, and the original
replaced with a working symlink into staging), or a clean pre-flight rejection. Not a
half-completed publish that breaks the path.

Root cause

Two places compute "where does this file live relative to the inputdata root", and they
disagree when the path reaches the tree through a symlink.

Site Computation Result for the repro
rimport:526 (stage_data) src.resolve().relative_to(inputdata_root.resolve())resolved lnd/a.nc → stages to <W>/staging/lnd/a.nc
relink.py:244 (replace_one_file_with_symlink) os.path.relpath(file_path, inputdata_root)lexical ../plink/lnd/a.nc → links to <W>/staging/../plink/lnd/a.nc

os.path.relpath does no filesystem lookup, so it cannot see that plink and inputdata
are the same directory. It escapes the root with .., and joining that onto the staging
root walks straight back out to the original path.

relink's own existence check does not catch it: at that moment link_target still
resolves to the original file, which does exist, so it proceeds to delete the original and
link over it. check_relink (rimport:400-402) notices afterwards and raises — after the
damage is done.

Suggested fix

Make the two agree. The narrower change is in relink.py:244, which also covers relink
used standalone:

relative_path = os.path.relpath(
    os.path.realpath(file_path), os.path.realpath(inputdata_root)
)

This was tried against the reproduction above: the run then exits 0 and produces
a.nc -> <W>/staging/lnd/a.nc, which reads correctly. The existing suite (341 tests) stays
green with it applied, so nothing currently depends on the lexical behaviour.

Worth considering alongside it: have replace_one_file_with_symlink refuse a computed
link_target that is not under target_dir, before deleting anything. That turns any
future instance of this class of bug into a clean refusal rather than a broken link, and
it is a cheaper invariant than keeping two path computations in step by inspection.

A regression test should cover the symlinked-root shape specifically; nothing in the
current suite exercises a path that reaches the inputdata tree through a symlink.

Scope

  • Reproduced identically on main and on the fix-rimport-on-dir branch. Pre-existing;
    not introduced by directory enumeration.
  • On the branch, the directory form reaches it too (rimport <plink>/lnd), because
    enumeration resolves the named directory and then hands each discovered file to the same
    relink path. Same single failure, same cause.
  • Verified on Python 3.13.2. The mechanism is os.path.relpath versus Path.resolve(),
    neither of which is version-sensitive, so the CI matrix (3.9–3.12) will behave the same.

Recovery, if it has already happened

The staged copy is intact. Repoint the link by hand:

ln -sf "$STAGING/<relative path>" "$INPUTDATA/<relative path>"

Verified against the reproduction: the file is readable again afterwards, with the staged
copy as its target. Nothing needs to be re-staged.

Activity

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

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions