From e986e58ed28187684975b4ccce8de0976488d644 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0pl=C3=ADchal?= Date: Wed, 13 May 2026 14:00:17 +0200 Subject: [PATCH 1/2] 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 --- fmf/base.py | 16 ++++--------- tests/unit/test_base.py | 53 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 11 deletions(-) diff --git a/fmf/base.py b/fmf/base.py index cfce46a..76ac893 100644 --- a/fmf/base.py +++ b/fmf/base.py @@ -104,12 +104,11 @@ def __init__(self, data, name=None, parent=None): # Special directives self._directives = dict() - # Store symlinks in while walking tree in grow() to detect - # symlink loops + # Track real paths of ancestor directories to detect symlink loops if parent is None: - self._symlinkdirs = [] + self._realpaths = set() else: - self._symlinkdirs = parent._symlinkdirs + self._realpaths = set(parent._realpaths) # Special handling for top parent if self.parent is None: @@ -708,6 +707,7 @@ def grow(self, path): if path in IGNORED_DIRECTORIES: # pragma: no cover log.debug("Ignoring '{0}' (special directory).".format(path)) return + self._realpaths.add(os.path.realpath(path)) log.info("Walking through directory {0}".format( os.path.abspath(path))) try: @@ -754,16 +754,10 @@ def grow(self, path): continue fulldir = os.path.join(dirpath, dirname) if os.path.islink(fulldir): - # According to the documentation, calling os.path.realpath - # with strict = True will raise OSError if a symlink loop - # is encountered. But it does not do that with a loop with - # more than one node fullpath = os.path.realpath(fulldir) - if fullpath in self._symlinkdirs: + if fullpath in self._realpaths: log.debug("Not entering symlink loop {}".format(fulldir)) continue - else: - self._symlinkdirs.append(fullpath) # Ignore metadata subtrees if os.path.isdir(os.path.join(path, dirname, SUFFIX)): diff --git a/tests/unit/test_base.py b/tests/unit/test_base.py index f913b6e..6236ebe 100644 --- a/tests/unit/test_base.py +++ b/tests/unit/test_base.py @@ -575,6 +575,59 @@ def test_validation_invalid_schema(self): with pytest.raises(fmf.utils.JsonSchemaError): self.wget.find('/recursion/deep').validate('invalid') + def test_symlink_shared_target(self): + """ + Multiple symlinks pointing to the same directory should all be followed + """ + + directory = tempfile.mkdtemp() + try: + Tree.init(directory) + common = os.path.join(directory, 'common') + os.mkdir(common) + with open(os.path.join(common, 'main.fmf'), 'w') as main: + main.write('execute:\n how: tmt\n') + for name in ('one', 'two'): + subdir = os.path.join(directory, name) + os.mkdir(subdir) + with open(os.path.join(subdir, 'main.fmf'), 'w') as main: + main.write(f'environment:\n VARIABLE: {name}\n') + os.symlink('../common', os.path.join(subdir, 'plan')) + + tree = Tree(directory) + one_plan = tree.find('/one/plan') + two_plan = tree.find('/two/plan') + assert one_plan is not None + assert two_plan is not None + assert one_plan.get('execute') == {'how': 'tmt'} + assert two_plan.get('execute') == {'how': 'tmt'} + assert one_plan.get('environment') == {'VARIABLE': 'one'} + assert two_plan.get('environment') == {'VARIABLE': 'two'} + finally: + rmtree(directory) + + def test_symlink_loop(self): + """ + Symlink loops should be detected and silently skipped + """ + + directory = tempfile.mkdtemp() + try: + Tree.init(directory) + subdir = os.path.join(directory, 'child') + os.mkdir(subdir) + with open(os.path.join(subdir, 'main.fmf'), 'w') as main: + main.write('key: value\n') + os.symlink('..', os.path.join(subdir, 'loop')) + + tree = Tree(directory) + child = tree.find('/child') + assert child is not None + assert child.get('key') == 'value' + assert tree.find('/child/loop') is None + finally: + rmtree(directory) + class TestRemote: """ From 0aab328e56ffa0c0d2fb884cdbabd6dd2c9bc08f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20=C5=A0pl=C3=ADchal?= Date: Wed, 13 May 2026 14:11:05 +0200 Subject: [PATCH 2/2] Add a short release note --- docs/releases.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/releases.rst b/docs/releases.rst index ab88aab..e70da4a 100644 --- a/docs/releases.rst +++ b/docs/releases.rst @@ -12,6 +12,12 @@ fmf library consumers can define custom :ref:`context` dimension handling by extending :py:class:`fmf.context.ContextDimension` and :py:class:`fmf.context.Context`. +The symlink loop detection has been improved to correctly handle +multiple symlinks pointing to the same target directory. Previously, +only the first symlink was followed while subsequent ones were +incorrectly skipped as loops. The detection now tracks ancestor +directories per branch instead of globally across the whole tree. + fmf-1.7.0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~