[Notes] [Git][BuildStream/buildstream][master] 7 commits: local.py: Do not follow symlinks in local directories

[Jürg Billeter <gitlab mg gitlab com>]

Title: GitLab

Jürg Billeter pushed to branch master at BuildStream / buildstream

Commits:

• 99e1be45
  by Jürg Billeter at 2019-02-11T05:12:25Z

  local.py: Do not follow symlinks in local directories
  
  isdir() follows symlinks on the host, resulting in potential host
  contamination. This change reorders the file checks to avoid this issue.
  

• f95e222e
  by Jürg Billeter at 2019-02-11T05:12:25Z

  sandbox/sandbox.py: Do not follow symlinks in _has_command()
  
  This is required to ensure symlinks are not resolved on the host.
  

• 89973fb3
  by Jürg Billeter at 2019-02-11T05:12:25Z

  utils.py: Remove list_dirs parameter from list_relative_paths()
  
  list_dirs was always True in the BuildStream code base. There was also a
  bug in the list_dirs=False code path as it did not return symlinks in
  `dirnames`.
  
  This is an API break, however, there are no known external callers.
  

• d1da3fb0
  by Jürg Billeter at 2019-02-11T05:12:25Z

  utils.py: Fix sorting of symlinks to directories
  
  os.walk() resolves symlinks to check whether they point to a directory
  even when followlinks is set to False. We already work around that
  broken behavior by extracting symlinks from `dirnames`. However, the
  sort order was still incorrect as we returned symlinks in dirnames
  before files and other symlinks. This change fixes this, sorting all
  files and symlinks in a single list.
  

• 7cf67ed3
  by Jürg Billeter at 2019-02-11T05:12:25Z

  _casbaseddirectory.py: Do not mimic os.walk() in list_relative_paths()
  
  This matches the change in utils.list_relative_paths() that now sorts
  all symlinks as files, instead of following the broken behavior of
  os.walk().
  

• 7ec0bb5e
  by Jürg Billeter at 2019-02-11T05:44:20Z

  tests/sources/local.py: Add directory symlink test
  

• c07cc967
  by Jürg Billeter at 2019-02-11T07:13:28Z

  Merge branch 'juerg/symlinks' into 'master'
  
  Symlink fixes
  
  See merge request BuildStream/buildstream!1138

5 changed files:

• buildstream/plugins/sources/local.py
• buildstream/sandbox/sandbox.py
• buildstream/storage/_casbaseddirectory.py
• buildstream/utils.py
• tests/sources/local.py

Changes:

buildstream/plugins/sources/local.py

---

...	...	@@ -97,7 +97,7 @@ class LocalSource(Source):
97	97	with self.timed_activity("Staging local files at {}".format(self.path)):
98	98
99	99	if os.path.isdir(self.fullpath):
100		- files = list(utils.list_relative_paths(self.fullpath, list_dirs=True))
	100	+ files = list(utils.list_relative_paths(self.fullpath))
101	101	utils.copy_files(self.fullpath, directory, files=files)
102	102	else:
103	103	destfile = os.path.join(directory, os.path.basename(self.path))
...	...	@@ -133,11 +133,11 @@ def unique_key(filename):
133	133
134	134	# Return some hard coded things for files which
135	135	# have no content to calculate a key for
136		- if os.path.isdir(filename):
137		- return "0"
138		- elif os.path.islink(filename):
	136	+ if os.path.islink(filename):
139	137	# For a symbolic link, use the link target as its unique identifier
140	138	return os.readlink(filename)
	139	+ elif os.path.isdir(filename):
	140	+ return "0"
141	141
142	142	return utils.sha256sum(filename)
143	143

buildstream/sandbox/sandbox.py

---

...	...	@@ -525,11 +525,11 @@ class Sandbox():
525	525	# (bool): Whether a command exists inside the sandbox.
526	526	def _has_command(self, command, env=None):
527	527	if os.path.isabs(command):
528		- return os.path.exists(os.path.join(
	528	+ return os.path.lexists(os.path.join(
529	529	self._root, command.lstrip(os.sep)))
530	530
531	531	for path in env.get('PATH').split(':'):
532		- if os.path.exists(os.path.join(
	532	+ if os.path.lexists(os.path.join(
533	533	self._root, path.lstrip(os.sep), command)):
534	534	return True
535	535

buildstream/storage/_casbaseddirectory.py

---

...	...	@@ -795,24 +795,11 @@ class CasBasedDirectory(Directory):
795	795	Return value: List(str) - list of all paths
796	796	"""
797	797
798		- symlink_list = filter(lambda i: isinstance(i[1].pb_object, remote_execution_pb2.SymlinkNode),
799		- self.index.items())
800		- file_list = list(filter(lambda i: isinstance(i[1].pb_object, remote_execution_pb2.FileNode),
	798	+ file_list = list(filter(lambda i: not isinstance(i[1].buildstream_object, CasBasedDirectory),
801	799	self.index.items()))
802	800	directory_list = filter(lambda i: isinstance(i[1].buildstream_object, CasBasedDirectory),
803	801	self.index.items())
804	802
805		- # We need to mimic the behaviour of os.walk, in which symlinks
806		- # to directories count as directories and symlinks to file or
807		- # broken symlinks count as files. os.walk doesn't follow
808		- # symlinks, so we don't recurse.
809		- for (k, v) in sorted(symlink_list):
810		- target = self._resolve(k, absolute_symlinks_resolve=True)
811		- if isinstance(target, CasBasedDirectory):
812		- yield os.path.join(relpath, k)
813		- else:
814		- file_list.append((k, v))
815		-
816	803	if file_list == [] and relpath != "":
817	804	yield relpath
818	805	else:

buildstream/utils.py

---

...	...	@@ -111,7 +111,7 @@ class FileListResult():
111	111	return ret
112	112
113	113
114		-def list_relative_paths(directory, *, list_dirs=True):
	114	+def list_relative_paths(directory):
115	115	"""A generator for walking directory relative paths
116	116
117	117	This generator is useful for checking the full manifest of
...	...	@@ -125,13 +125,26 @@ def list_relative_paths(directory, *, list_dirs=True):
125	125
126	126	Args:
127	127	directory (str): The directory to list files in
128		- list_dirs (bool): Whether to list directories
129	128
130	129	Yields:
131	130	Relative filenames in `directory`
132	131	"""
133	132	for (dirpath, dirnames, filenames) in os.walk(directory):
134	133
	134	+ # os.walk does not decend into symlink directories, which
	135	+ # makes sense because otherwise we might have redundant
	136	+ # directories, or end up descending into directories outside
	137	+ # of the walk() directory.
	138	+ #
	139	+ # But symlinks to directories are still identified as
	140	+ # subdirectories in the walked `dirpath`, so we extract
	141	+ # these symlinks from `dirnames` and add them to `filenames`.
	142	+ #
	143	+ for d in dirnames:
	144	+ fullpath = os.path.join(dirpath, d)
	145	+ if os.path.islink(fullpath):
	146	+ filenames.append(d)
	147	+
135	148	# Modifying the dirnames directly ensures that the os.walk() generator
136	149	# allows us to specify the order in which they will be iterated.
137	150	dirnames.sort()
...	...	@@ -143,25 +156,10 @@ def list_relative_paths(directory, *, list_dirs=True):
143	156	# `directory`, prefer to have no prefix in that case.
144	157	basepath = relpath if relpath != '.' and dirpath != directory else ''
145	158
146		- # os.walk does not decend into symlink directories, which
147		- # makes sense because otherwise we might have redundant
148		- # directories, or end up descending into directories outside
149		- # of the walk() directory.
150		- #
151		- # But symlinks to directories are still identified as
152		- # subdirectories in the walked `dirpath`, so we extract
153		- # these symlinks from `dirnames`
154		- #
155		- if list_dirs:
156		- for d in dirnames:
157		- fullpath = os.path.join(dirpath, d)
158		- if os.path.islink(fullpath):
159		- yield os.path.join(basepath, d)
160		-
161	159	# We've decended into an empty directory, in this case we
162	160	# want to include the directory itself, but not in any other
163	161	# case.
164		- if list_dirs and not filenames:
	162	+ if not filenames:
165	163	yield relpath
166	164
167	165	# List the filenames in the walked directory

tests/sources/local.py

---

...	...	@@ -136,3 +136,23 @@ def test_stage_file_exists(cli, tmpdir, datafiles):
136	136	result = cli.run(project=project, args=['build', 'target.bst'])
137	137	result.assert_main_error(ErrorDomain.STREAM, None)
138	138	result.assert_task_error(ErrorDomain.SOURCE, 'ensure-stage-dir-fail')
	139	+
	140	+
	141	+@pytest.mark.datafiles(os.path.join(DATA_DIR, 'directory'))
	142	+def test_stage_directory_symlink(cli, tmpdir, datafiles):
	143	+ project = os.path.join(datafiles.dirname, datafiles.basename)
	144	+ checkoutdir = os.path.join(str(tmpdir), "checkout")
	145	+
	146	+ symlink = os.path.join(project, 'files', 'symlink-to-subdir')
	147	+ os.symlink('subdir', symlink)
	148	+
	149	+ # Build, checkout
	150	+ result = cli.run(project=project, args=['build', 'target.bst'])
	151	+ result.assert_success()
	152	+ result = cli.run(project=project, args=['artifact', 'checkout', 'target.bst', '--directory', checkoutdir])
	153	+ result.assert_success()
	154	+
	155	+ # Check that the checkout contains the expected directory and directory symlink
	156	+ assert(os.path.exists(os.path.join(checkoutdir, 'subdir', 'anotherfile.txt')))
	157	+ assert(os.path.exists(os.path.join(checkoutdir, 'symlink-to-subdir', 'anotherfile.txt')))
	158	+ assert(os.path.islink(os.path.join(checkoutdir, 'symlink-to-subdir')))

—
View it on GitLab.
You're receiving this email because of your account on gitlab.com. If you'd like to receive fewer emails, you can adjust your notification settings.