Skip to content

Commit e986e58

Browse files
psssclaude
andcommitted
Fix symlink loop detection to allow shared symlink targets
The previous implementation used a single shared list of visited symlink targets across the entire tree. This caused sibling symlinks pointing to the same directory to be incorrectly skipped as loops, because the second symlink found its target already recorded by the first one. Replace the shared list with a per-branch set of ancestor real paths. Each child node gets its own copy of the set, so sibling branches cannot interfere with each other. A symlink is now only skipped when its target resolves to an ancestor directory of the current walk path, which is the actual condition for a cycle. Add tests for both the shared symlink target scenario and for the symlink loop detection to prevent future regressions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent e90b1af commit e986e58

2 files changed

Lines changed: 58 additions & 11 deletions

File tree

fmf/base.py

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -104,12 +104,11 @@ def __init__(self, data, name=None, parent=None):
104104
# Special directives
105105
self._directives = dict()
106106

107-
# Store symlinks in while walking tree in grow() to detect
108-
# symlink loops
107+
# Track real paths of ancestor directories to detect symlink loops
109108
if parent is None:
110-
self._symlinkdirs = []
109+
self._realpaths = set()
111110
else:
112-
self._symlinkdirs = parent._symlinkdirs
111+
self._realpaths = set(parent._realpaths)
113112

114113
# Special handling for top parent
115114
if self.parent is None:
@@ -708,6 +707,7 @@ def grow(self, path):
708707
if path in IGNORED_DIRECTORIES: # pragma: no cover
709708
log.debug("Ignoring '{0}' (special directory).".format(path))
710709
return
710+
self._realpaths.add(os.path.realpath(path))
711711
log.info("Walking through directory {0}".format(
712712
os.path.abspath(path)))
713713
try:
@@ -754,16 +754,10 @@ def grow(self, path):
754754
continue
755755
fulldir = os.path.join(dirpath, dirname)
756756
if os.path.islink(fulldir):
757-
# According to the documentation, calling os.path.realpath
758-
# with strict = True will raise OSError if a symlink loop
759-
# is encountered. But it does not do that with a loop with
760-
# more than one node
761757
fullpath = os.path.realpath(fulldir)
762-
if fullpath in self._symlinkdirs:
758+
if fullpath in self._realpaths:
763759
log.debug("Not entering symlink loop {}".format(fulldir))
764760
continue
765-
else:
766-
self._symlinkdirs.append(fullpath)
767761

768762
# Ignore metadata subtrees
769763
if os.path.isdir(os.path.join(path, dirname, SUFFIX)):

tests/unit/test_base.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -575,6 +575,59 @@ def test_validation_invalid_schema(self):
575575
with pytest.raises(fmf.utils.JsonSchemaError):
576576
self.wget.find('/recursion/deep').validate('invalid')
577577

578+
def test_symlink_shared_target(self):
579+
"""
580+
Multiple symlinks pointing to the same directory should all be followed
581+
"""
582+
583+
directory = tempfile.mkdtemp()
584+
try:
585+
Tree.init(directory)
586+
common = os.path.join(directory, 'common')
587+
os.mkdir(common)
588+
with open(os.path.join(common, 'main.fmf'), 'w') as main:
589+
main.write('execute:\n how: tmt\n')
590+
for name in ('one', 'two'):
591+
subdir = os.path.join(directory, name)
592+
os.mkdir(subdir)
593+
with open(os.path.join(subdir, 'main.fmf'), 'w') as main:
594+
main.write(f'environment:\n VARIABLE: {name}\n')
595+
os.symlink('../common', os.path.join(subdir, 'plan'))
596+
597+
tree = Tree(directory)
598+
one_plan = tree.find('/one/plan')
599+
two_plan = tree.find('/two/plan')
600+
assert one_plan is not None
601+
assert two_plan is not None
602+
assert one_plan.get('execute') == {'how': 'tmt'}
603+
assert two_plan.get('execute') == {'how': 'tmt'}
604+
assert one_plan.get('environment') == {'VARIABLE': 'one'}
605+
assert two_plan.get('environment') == {'VARIABLE': 'two'}
606+
finally:
607+
rmtree(directory)
608+
609+
def test_symlink_loop(self):
610+
"""
611+
Symlink loops should be detected and silently skipped
612+
"""
613+
614+
directory = tempfile.mkdtemp()
615+
try:
616+
Tree.init(directory)
617+
subdir = os.path.join(directory, 'child')
618+
os.mkdir(subdir)
619+
with open(os.path.join(subdir, 'main.fmf'), 'w') as main:
620+
main.write('key: value\n')
621+
os.symlink('..', os.path.join(subdir, 'loop'))
622+
623+
tree = Tree(directory)
624+
child = tree.find('/child')
625+
assert child is not None
626+
assert child.get('key') == 'value'
627+
assert tree.find('/child/loop') is None
628+
finally:
629+
rmtree(directory)
630+
578631

579632
class TestRemote:
580633
"""

0 commit comments

Comments
 (0)