Skip to content

Commit

Permalink
scuba: Require relative volume host paths to start with "./" or "../"
Browse files Browse the repository at this point in the history
This is to avoid ambiguity.

#227 (comment)
  • Loading branch information
JonathonReinhart committed Sep 13, 2023
1 parent 53c2195 commit 7dc17e1
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 13 deletions.
1 change: 1 addition & 0 deletions docs/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ configuration error.

Volume host paths can be absolute or relative. If a relative path is used, it
is interpreted as relative to the directory in which ``.scuba.yml`` is found.
To avoid ambiguity, relative paths must start with ``./`` or ``../``.

Volume host directories which do not already exist are created as the current
user before creating the container.
Expand Down
15 changes: 12 additions & 3 deletions scuba/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,8 @@ def _expand_path(in_str: str, base_dir: Optional[Path] = None) -> Path:
the host environment.
After environment variable expansion, absolute paths are returned as-is.
Relative paths are joined to base_dir, if provided.
Relative paths must start with ./ or ../ and are joined to base_dir, if
provided.
Args:
in_str: Input path as a string.
Expand All @@ -357,13 +358,14 @@ def _expand_path(in_str: str, base_dir: Optional[Path] = None) -> Path:
ValueError: If base_dir is provided but not absolute.
ConfigError: If a referenced environment variable is not set.
ConfigError: An environment variable reference could not be parsed.
ConfigError: A relative path does not start with "./" or "../".
ConfigError: A relative path is given when base_dir is not provided.
"""
if base_dir is not None and not base_dir.is_absolute():
raise ValueError(f"base_dir is not absolute: {base_dir}")

try:
output = expand_env_vars(in_str)
path_str = expand_env_vars(in_str)
except KeyError as ke:
# pylint: disable=raise-missing-from
raise ConfigError(
Expand All @@ -374,11 +376,18 @@ def _expand_path(in_str: str, base_dir: Optional[Path] = None) -> Path:
f"Unable to expand string '{in_str}' due to parsing errors"
) from ve

path = Path(output)
path = Path(path_str)

if not path.is_absolute():
if base_dir is None:
raise ConfigError(f"Relative path not allowed: {path}")

# Make sure it starts with ./ or ../
# We have to use the original string input since Path() will remove ./
valid_prefixes = ("./", "../")
if not any(path_str.startswith(pfx) for pfx in valid_prefixes):
raise ConfigError(f"Relative path must start with {' or '.join(valid_prefixes)}: {path}")

path = base_dir / path

assert path.is_absolute()
Expand Down
41 changes: 32 additions & 9 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -1012,18 +1012,17 @@ def test_volumes_with_invalid_env_vars(self, monkeypatch) -> None:

def test_volumes_hostpath_rel(self, monkeypatch, in_tmp_path) -> None:
"""volume hostpath can be relative to scuba_root (top dir)"""
monkeypatch.setenv("RELVAR", "spam/eggs")
monkeypatch.setenv("RELVAR", "./spam/eggs")

with open(".scuba.yml", "w") as f:
f.write(
r"""
image: na
volumes:
/one: foo # simple form, naked dir name
/two: ./foo/bar # simple form, dotted path
/three: # complex form
/bar: ./foo/bar # simple form, dotted path
/scp: # complex form
hostpath: ./snap/crackle/pop
/four: $RELVAR # simple form, relative path in variable
/relvar: $RELVAR # simple form, relative path in variable
"""
)

Expand All @@ -1038,10 +1037,34 @@ def test_volumes_hostpath_rel(self, monkeypatch, in_tmp_path) -> None:
assert found_rel == subdir
assert config.volumes is not None

assert_vol(config.volumes, "/one", in_tmp_path / "foo")
assert_vol(config.volumes, "/two", in_tmp_path / "foo" / "bar")
assert_vol(config.volumes, "/three", in_tmp_path / "snap" / "crackle" / "pop")
assert_vol(config.volumes, "/four", in_tmp_path / "spam" / "eggs")
assert_vol(config.volumes, "/bar", in_tmp_path / "foo" / "bar")
assert_vol(config.volumes, "/scp", in_tmp_path / "snap" / "crackle" / "pop")
assert_vol(config.volumes, "/relvar", in_tmp_path / "spam" / "eggs")

def test_volumes_hostpath_rel_requires_dot_simple(self, monkeypatch, in_tmp_path) -> None:
"""relaitve volume hostpath (simple form) must start with ./ or ../"""
with open(".scuba.yml", "w") as f:
f.write(
r"""
image: na
volumes:
/one: foo # Forbidden
"""
)
self._invalid_config("Relative path must start with ./ or ../")

def test_volumes_hostpath_rel_requires_dot_complex(self, monkeypatch, in_tmp_path) -> None:
"""relaitve volume hostpath (complex form) must start with ./ or ../"""
with open(".scuba.yml", "w") as f:
f.write(
r"""
image: na
volumes:
/one:
hostpath: foo # Forbidden
"""
)
self._invalid_config("Relative path must start with ./ or ../")

def test_volumes_contpath_rel(self, monkeypatch, in_tmp_path) -> None:
with open(".scuba.yml", "w") as f:
Expand Down
2 changes: 1 addition & 1 deletion tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -1081,7 +1081,7 @@ def test_volumes_host_path_rel(self) -> None:
f"""
image: {DOCKER_IMAGE}
volumes:
/userdir: {userdir}
/userdir: ./{userdir}
"""
)

Expand Down

0 comments on commit 7dc17e1

Please sign in to comment.