From 2d245c2e022ca92856cc8b57f796b603a0635724 Mon Sep 17 00:00:00 2001 From: Michael Hanke Date: Tue, 10 Oct 2023 18:29:46 +0200 Subject: [PATCH 1/2] Add protection against invalid image path declarations for `add` This is a bit painful, because datalad-next parameter validation is not available, but worth adding given the issue. Closes #150 --- datalad_container/containers_add.py | 10 ++++++++++ datalad_container/tests/test_add.py | 25 ++++++++++++++++++++++--- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/datalad_container/containers_add.py b/datalad_container/containers_add.py index 3840e05a..44c100be 100644 --- a/datalad_container/containers_add.py +++ b/datalad_container/containers_add.py @@ -199,6 +199,16 @@ def __call__(name, url=None, dataset=None, call_fmt=None, image=None, purpose='add container') runner = WitlessRunner() + if image is not None: + if op.isabs(image): + raise ValueError( + f'image must be a relative path, got {image!r}') + if ds.pathobj.resolve() \ + not in (ds.pathobj / image).resolve().parents: + raise ValueError( + 'image must be a relative path pointing inside' + f'the dataset, got {image!r}') + # prevent madness in the config file if not re.match(r'^[0-9a-zA-Z-]+$', name): raise ValueError( diff --git a/datalad_container/tests/test_add.py b/datalad_container/tests/test_add.py index 9adb9f92..bbc05f32 100644 --- a/datalad_container/tests/test_add.py +++ b/datalad_container/tests/test_add.py @@ -15,8 +15,27 @@ from datalad_container.containers_add import _ensure_datalad_remote -# NOTE: At the moment, testing of the containers-add itself happens implicitly -# via use in other tests. +# NOTE: At the moment, most testing of the containers-add itself happens implicitly +# this via use in other tests. + +common_kwargs = {'result_renderer': 'disabled'} + + +def test_add_invalid_imgpath(tmp_path): + ds = Dataset(tmp_path).create(**common_kwargs) + # path spec must be relative + with pytest.raises(ValueError): + ds.containers_add( + 'dummy', + image=tmp_path, + **common_kwargs + ) + with pytest.raises(ValueError): + ds.containers_add( + 'dummy', + image=Path('..', 'sneaky'), + **common_kwargs + ) @with_tempfile @@ -41,4 +60,4 @@ def test_ensure_datalad_remote_maybe_enable(path=None, *, autoenable): if not autoenable: assert_not_in("datalad", repo.get_remotes()) _ensure_datalad_remote(repo) - assert_in("datalad", repo.get_remotes()) \ No newline at end of file + assert_in("datalad", repo.get_remotes()) From 7938498eb29894c002c1b9cfdb3614f701c48505 Mon Sep 17 00:00:00 2001 From: Yaroslav Halchenko Date: Wed, 11 Oct 2023 15:29:12 -0400 Subject: [PATCH 2/2] Minor: add missing space --- datalad_container/containers_add.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datalad_container/containers_add.py b/datalad_container/containers_add.py index 44c100be..03163bc4 100644 --- a/datalad_container/containers_add.py +++ b/datalad_container/containers_add.py @@ -206,7 +206,7 @@ def __call__(name, url=None, dataset=None, call_fmt=None, image=None, if ds.pathobj.resolve() \ not in (ds.pathobj / image).resolve().parents: raise ValueError( - 'image must be a relative path pointing inside' + 'image must be a relative path pointing inside ' f'the dataset, got {image!r}') # prevent madness in the config file