mirror of
https://gerrit.googlesource.com/git-repo
synced 2024-12-21 07:16:21 +00:00
manifest: relax include name rules for user-specified path
Allow the user to specify relative or absolute or any other funky path that they want when using `repo init` or `repo sync`. Our goal is to restrict the paths in the remote manifest git repo we cloned from the network, not protect the user from themselves. Bug: https://crbug.com/gerrit/14156 Change-Id: I1ccfb2a6bd1dce2bd765e261bef0bbf0f8a9beb6 Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/298823 Reviewed-by: Jonathan Nieder <jrn@google.com> Tested-by: Mike Frysinger <vapier@google.com>
This commit is contained in:
parent
13cb7f799d
commit
5413397204
4
error.py
4
error.py
@ -22,12 +22,12 @@ class ManifestParseError(Exception):
|
|||||||
"""
|
"""
|
||||||
|
|
||||||
|
|
||||||
class ManifestInvalidRevisionError(Exception):
|
class ManifestInvalidRevisionError(ManifestParseError):
|
||||||
"""The revision value in a project is incorrect.
|
"""The revision value in a project is incorrect.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
|
||||||
class ManifestInvalidPathError(Exception):
|
class ManifestInvalidPathError(ManifestParseError):
|
||||||
"""A path used in <copyfile> or <linkfile> is incorrect.
|
"""A path used in <copyfile> or <linkfile> is incorrect.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
|
@ -624,16 +624,22 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
|
|||||||
b = b[len(R_HEADS):]
|
b = b[len(R_HEADS):]
|
||||||
self.branch = b
|
self.branch = b
|
||||||
|
|
||||||
|
# The manifestFile was specified by the user which is why we allow include
|
||||||
|
# paths to point anywhere.
|
||||||
nodes = []
|
nodes = []
|
||||||
nodes.append(self._ParseManifestXml(self.manifestFile,
|
nodes.append(self._ParseManifestXml(
|
||||||
self.manifestProject.worktree))
|
self.manifestFile, self.manifestProject.worktree,
|
||||||
|
restrict_includes=False))
|
||||||
|
|
||||||
if self._load_local_manifests and self.local_manifests:
|
if self._load_local_manifests and self.local_manifests:
|
||||||
try:
|
try:
|
||||||
for local_file in sorted(platform_utils.listdir(self.local_manifests)):
|
for local_file in sorted(platform_utils.listdir(self.local_manifests)):
|
||||||
if local_file.endswith('.xml'):
|
if local_file.endswith('.xml'):
|
||||||
local = os.path.join(self.local_manifests, local_file)
|
local = os.path.join(self.local_manifests, local_file)
|
||||||
nodes.append(self._ParseManifestXml(local, self.repodir))
|
# Since local manifests are entirely managed by the user, allow
|
||||||
|
# them to point anywhere the user wants.
|
||||||
|
nodes.append(self._ParseManifestXml(
|
||||||
|
local, self.repodir, restrict_includes=False))
|
||||||
except OSError:
|
except OSError:
|
||||||
pass
|
pass
|
||||||
|
|
||||||
@ -651,7 +657,19 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
|
|||||||
|
|
||||||
self._loaded = True
|
self._loaded = True
|
||||||
|
|
||||||
def _ParseManifestXml(self, path, include_root, parent_groups=''):
|
def _ParseManifestXml(self, path, include_root, parent_groups='',
|
||||||
|
restrict_includes=True):
|
||||||
|
"""Parse a manifest XML and return the computed nodes.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
path: The XML file to read & parse.
|
||||||
|
include_root: The path to interpret include "name"s relative to.
|
||||||
|
parent_groups: The groups to apply to this projects.
|
||||||
|
restrict_includes: Whether to constrain the "name" attribute of includes.
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
List of XML nodes.
|
||||||
|
"""
|
||||||
try:
|
try:
|
||||||
root = xml.dom.minidom.parse(path)
|
root = xml.dom.minidom.parse(path)
|
||||||
except (OSError, xml.parsers.expat.ExpatError) as e:
|
except (OSError, xml.parsers.expat.ExpatError) as e:
|
||||||
@ -670,6 +688,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
|
|||||||
for node in manifest.childNodes:
|
for node in manifest.childNodes:
|
||||||
if node.nodeName == 'include':
|
if node.nodeName == 'include':
|
||||||
name = self._reqatt(node, 'name')
|
name = self._reqatt(node, 'name')
|
||||||
|
if restrict_includes:
|
||||||
msg = self._CheckLocalPath(name)
|
msg = self._CheckLocalPath(name)
|
||||||
if msg:
|
if msg:
|
||||||
raise ManifestInvalidPathError(
|
raise ManifestInvalidPathError(
|
||||||
@ -681,13 +700,13 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
|
|||||||
include_groups = node.getAttribute('groups') + ',' + include_groups
|
include_groups = node.getAttribute('groups') + ',' + include_groups
|
||||||
fp = os.path.join(include_root, name)
|
fp = os.path.join(include_root, name)
|
||||||
if not os.path.isfile(fp):
|
if not os.path.isfile(fp):
|
||||||
raise ManifestParseError("include %s doesn't exist or isn't a file"
|
raise ManifestParseError("include [%s/]%s doesn't exist or isn't a file"
|
||||||
% (name,))
|
% (include_root, name))
|
||||||
try:
|
try:
|
||||||
nodes.extend(self._ParseManifestXml(fp, include_root, include_groups))
|
nodes.extend(self._ParseManifestXml(fp, include_root, include_groups))
|
||||||
# should isolate this to the exact exception, but that's
|
# should isolate this to the exact exception, but that's
|
||||||
# tricky. actual parsing implementation may vary.
|
# tricky. actual parsing implementation may vary.
|
||||||
except (KeyboardInterrupt, RuntimeError, SystemExit):
|
except (KeyboardInterrupt, RuntimeError, SystemExit, ManifestParseError):
|
||||||
raise
|
raise
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
raise ManifestParseError(
|
raise ManifestParseError(
|
||||||
|
@ -298,8 +298,8 @@ class IncludeElementTests(ManifestParseTestCase):
|
|||||||
# Check level2 proj group not removed.
|
# Check level2 proj group not removed.
|
||||||
self.assertIn('l2g1', proj.groups)
|
self.assertIn('l2g1', proj.groups)
|
||||||
|
|
||||||
def test_bad_name_checks(self):
|
def test_allow_bad_name_from_user(self):
|
||||||
"""Check handling of bad name attribute."""
|
"""Check handling of bad name attribute from the user's input."""
|
||||||
def parse(name):
|
def parse(name):
|
||||||
manifest = self.getXmlManifest(f"""
|
manifest = self.getXmlManifest(f"""
|
||||||
<manifest>
|
<manifest>
|
||||||
@ -307,6 +307,34 @@ class IncludeElementTests(ManifestParseTestCase):
|
|||||||
<default remote="default-remote" revision="refs/heads/main" />
|
<default remote="default-remote" revision="refs/heads/main" />
|
||||||
<include name="{name}" />
|
<include name="{name}" />
|
||||||
</manifest>
|
</manifest>
|
||||||
|
""")
|
||||||
|
# Force the manifest to be parsed.
|
||||||
|
manifest.ToXml()
|
||||||
|
|
||||||
|
# Setup target of the include.
|
||||||
|
target = os.path.join(self.tempdir, 'target.xml')
|
||||||
|
with open(target, 'w') as fp:
|
||||||
|
fp.write('<manifest></manifest>')
|
||||||
|
|
||||||
|
# Include with absolute path.
|
||||||
|
parse(os.path.abspath(target))
|
||||||
|
|
||||||
|
# Include with relative path.
|
||||||
|
parse(os.path.relpath(target, self.manifest_dir))
|
||||||
|
|
||||||
|
def test_bad_name_checks(self):
|
||||||
|
"""Check handling of bad name attribute."""
|
||||||
|
def parse(name):
|
||||||
|
# Setup target of the include.
|
||||||
|
with open(os.path.join(self.manifest_dir, 'target.xml'), 'w') as fp:
|
||||||
|
fp.write(f'<manifest><include name="{name}"/></manifest>')
|
||||||
|
|
||||||
|
manifest = self.getXmlManifest("""
|
||||||
|
<manifest>
|
||||||
|
<remote name="default-remote" fetch="http://localhost" />
|
||||||
|
<default remote="default-remote" revision="refs/heads/main" />
|
||||||
|
<include name="target.xml" />
|
||||||
|
</manifest>
|
||||||
""")
|
""")
|
||||||
# Force the manifest to be parsed.
|
# Force the manifest to be parsed.
|
||||||
manifest.ToXml()
|
manifest.ToXml()
|
||||||
|
Loading…
Reference in New Issue
Block a user