fix: Resolving symlinks when updating relative paths in template (#3764)

* fix: Resolving symlinks when updating relative paths in template

* adding unit tests

Co-authored-by: Daniel Mil <84205762+mildaniel@users.noreply.github.com>
Co-authored-by: Wing Fung Lau <4760060+hawflau@users.noreply.github.com>

samcli/commands/_utils/template.py

@@ -247,7 +247,9 @@ def _resolve_relative_to(path, original_root, new_root):
 
     # Value is definitely a relative path. Change it relative to the destination directory
     return os.path.relpath(
-        os.path.normpath(os.path.join(original_root, path)), new_root  # Absolute original path w.r.t ``original_root``
+        # Resolve the paths to take care of symlinks
+        os.path.normpath(os.path.join(pathlib.Path(original_root).resolve(), path)),
+        pathlib.Path(new_root).resolve(),  # Absolute original path w.r.t ``original_root``
     )  # Resolve the original path with respect to ``new_root``
 
 

tests/unit/commands/_utils/test_template.py

@@ -1,7 +1,9 @@
 import copy
 import os
+import tempfile
 from unittest import TestCase
 from unittest.mock import patch, mock_open, MagicMock
+import shutil
 
 import yaml
 from botocore.utils import set_value_from_jmespath
@@ -13,6 +15,7 @@
     METADATA_WITH_LOCAL_PATHS,
     RESOURCES_WITH_LOCAL_PATHS,
     _update_relative_paths,
+    _resolve_relative_to,
     move_template,
     get_template_parameters,
     TemplateNotFoundException,
@@ -421,6 +424,78 @@ def test_must_update_aws_include_also(self):
         self.assertEqual(result, expected_template_dict)
 
 
+class Test_resolve_relative_to(TestCase):
+    def setUp(self):
+        self.scratchdir = os.path.split(tempfile.mkdtemp(dir=os.curdir))[-1]
+        self.curpath = os.path.join("foo", "bar")
+
+    def tearDown(self):
+        shutil.rmtree(self.scratchdir)
+
+    def test_must_resolve_relative_to_with_simple_paths(self):
+        original_root = os.path.abspath("src")
+        new_root = os.path.abspath("src/destination")
+
+        result = _resolve_relative_to(self.curpath, original_root, new_root)
+        expected_result = os.path.join("..", self.curpath)
+
+        self.assertEqual(result, expected_result)
+
+    def test_must_resolve_relative_to_with_symlinked_original_root(self):
+        original_root = os.path.abspath(os.path.join(self.scratchdir, "some", "src"))
+        original_root_link = os.path.abspath(os.path.join(self.scratchdir, "originallink"))
+        self.create_symlink(original_root, original_root_link)
+
+        new_root = os.path.abspath("destination")
+
+        result = _resolve_relative_to(self.curpath, original_root_link, new_root)
+        # path = foo/bar
+        # original_path = /path/from/root/scratchdir/originallink -> /path/from/root/scratchdir/some/src
+        # new_path = /path/from/root/destination
+        # relative path must be ../scratchdir/some/src/foo/bar
+        expected_result = os.path.join("..", self.scratchdir, "some", "src", self.curpath)
+
+        self.assertEqual(result, expected_result)
+
+    def test_must_resolve_relative_to_with_symlinked_new_root(self):
+        original_root = os.path.abspath("src")
+
+        new_root = os.path.abspath(os.path.join(self.scratchdir, "some", "destination"))
+        new_root_link = os.path.abspath(os.path.join(self.scratchdir, "newlink"))
+        self.create_symlink(new_root, new_root_link)
+
+        result = _resolve_relative_to(self.curpath, original_root, new_root_link)
+        # path = foo/bar
+        # original_path = /path/from/root/src
+        # new_path = /path/from/root/scratchdir/newlink -> /path/from/root/scratchdir/some/destination
+        # relative path must be ../../../src/foo/bar
+        expected_result = os.path.join("..", "..", "..", "src", self.curpath)
+
+        self.assertEqual(result, expected_result)
+
+    def test_must_resolve_relative_to_symlinked_original_root_and_new_root(self):
+        original_root = os.path.abspath(os.path.join(self.scratchdir, "some", "src"))
+        original_root_link = os.path.abspath(os.path.join(self.scratchdir, "originallink"))
+        self.create_symlink(original_root, original_root_link)
+
+        new_root = os.path.abspath(os.path.join(self.scratchdir, "another", "destination"))
+        new_root_link = os.path.abspath(os.path.join(self.scratchdir, "newlink"))
+        self.create_symlink(new_root, new_root_link)
+
+        result = _resolve_relative_to(self.curpath, original_root, new_root_link)
+        # path = foo/bar
+        # original_path = /path/from/root/scratchdir/originallink -> /path/from/root/scratchdir/some/src
+        # new_path = /path/from/root/scratchdir/newlink -> /path/from/root/scratchdir/another/destination
+        # relative path must be ../../some/srcfoo/bar
+        expected_result = os.path.join("..", "..", "some", "src", self.curpath)
+
+        self.assertEqual(result, expected_result)
+
+    def create_symlink(self, src, dest):
+        os.makedirs(src)
+        os.symlink(src, dest)
+
+
 class Test_move_template(TestCase):
     @patch("samcli.commands._utils.template._update_relative_paths")
     @patch("samcli.commands._utils.template.yaml_dump")