From ca8a76a01674e2d98f8f2a33d42a1e016bf654ee Mon Sep 17 00:00:00 2001 From: Mike Lin Date: Sat, 10 Apr 2021 13:14:56 -1000 Subject: [PATCH] CLI: --no-outside-imports to deny imports from outside of top-level WDL file's directory --- WDL/CLI.py | 109 +++++++++++++++++++++++++++++++++++-------------- tests/runner.t | 24 ++++++++++- 2 files changed, 101 insertions(+), 32 deletions(-) diff --git a/WDL/CLI.py b/WDL/CLI.py index a7b0603a..b0fddc3c 100644 --- a/WDL/CLI.py +++ b/WDL/CLI.py @@ -140,7 +140,7 @@ def __call__(self, parser, namespace, values, option_string=None): sys.exit(0) -def fill_common(subparser, path=True): +def fill_common(subparser): group = subparser.add_argument_group("language") group.add_argument( "--no-quant-check", @@ -151,15 +151,19 @@ def fill_common(subparser, path=True): "backwards compatibility with older WDL)" ), ) - if path: - group.add_argument( - "-p", - "--path", - metavar="DIR", - type=str, - action="append", - help="local directory to search for imports", - ) + group.add_argument( + "-p", + "--path", + metavar="DIR", + type=str, + action="append", + help="local directory to search for imports", + ) + group.add_argument( + "--no-outside-imports", + action="store_true", + help="deny local imports from outside top-level WDL file's directory", + ) group = subparser.add_argument_group("debugging") group.add_argument( "--debug", action="store_true", help="maximally verbose logging & exception tracebacks" @@ -216,6 +220,7 @@ def check( show_all=False, suppress=None, shellcheck=True, + no_outside_imports=False, **kwargs, ): from . import Lint @@ -231,7 +236,12 @@ def check( shown = [0] for uri1 in uri or []: try: - doc = load(uri1, path or [], check_quant=check_quant, read_source=read_source) + doc = load( + uri1, + path or [], + check_quant=check_quant, + read_source=make_read_source(no_outside_imports), + ) except (Error.SyntaxError, Error.ValidationError, Error.MultipleValidationErrors) as exn: if not getattr(exn, "declared_wdl_version", None): atexit.register( @@ -390,23 +400,44 @@ def print_error(exn): quant_warning = True -async def read_source(uri, path, importer): - from urllib import parse, request +def make_read_source(no_outside_imports): + top_dir = None - if uri.startswith("http:") or uri.startswith("https:"): - fn = os.path.join( - tempfile.mkdtemp(prefix="miniwdl_import_uri_"), - os.path.basename(parse.urlsplit(uri).path), - ) - request.urlretrieve(uri, filename=fn) - with open(fn, "r") as infile: - return ReadSourceResult(infile.read(), uri) - elif importer and ( - importer.pos.abspath.startswith("http:") or importer.pos.abspath.startswith("https:") - ): - assert not os.path.isabs(uri), "absolute import from downloaded WDL" - return await read_source(parse.urljoin(importer.pos.abspath, uri), [], importer) - return await read_source_default(uri, path, importer) + async def read_source(uri, path, importer): + from urllib import parse, request + + if uri.startswith("http:") or uri.startswith("https:"): + with tempfile.TemporaryDirectory(prefix="miniwdl_import_uri_") as tmpdir: + assert isinstance(tmpdir, str) and os.path.isdir(tmpdir) + fn = os.path.join( + tmpdir, + os.path.basename(parse.urlsplit(uri).path), + ) + request.urlretrieve(uri, filename=fn) + with open(fn, "r") as infile: + return ReadSourceResult(infile.read(), uri) + elif importer and ( + importer.pos.abspath.startswith("http:") or importer.pos.abspath.startswith("https:") + ): + assert not os.path.isabs(uri), "absolute import from downloaded WDL" + return await read_source(parse.urljoin(importer.pos.abspath, uri), [], importer) + ans = await read_source_default(uri, path, importer) + if no_outside_imports: + # Require all imported local WDL files to be in/under the directory of the top-level + # WDL file (the first imported), or one of the --path directoires. + nonlocal top_dir + if not top_dir: + top_dir = os.path.dirname(ans.abspath) + if not next( + (p for p in ([top_dir] + path) if path_really_within(ans.abspath, p)), False + ): + raise PermissionError( + "denied import from outside top-level WDL file's directory; " + "strike --no-outside-imports or add to --path: " + os.path.dirname(ans.abspath) + ) + return ans + + return read_source def fill_run_subparser(subparsers): @@ -558,6 +589,7 @@ def runner( error_json=False, log_json=False, stdout_file=None, + no_outside_imports=False, **kwargs, ): # set up logging @@ -658,7 +690,12 @@ def runner( try: # load WDL document - doc = load(uri, path or [], check_quant=check_quant, read_source=read_source) + doc = load( + uri, + path or [], + check_quant=check_quant, + read_source=make_read_source(no_outside_imports), + ) # parse and validate the provided inputs eff_root = ( @@ -784,7 +821,11 @@ def runner_input_completer(prefix, parsed_args, **kwargs): uri, path=(parsed_args.path if hasattr(parsed_args, "path") else []), check_quant=parsed_args.check_quant, - read_source=read_source, + read_source=make_read_source( + parsed_args.no_outside_imports + if hasattr(parsed_args, "no_outside_imports") + else False + ), ) except Exception as exn: argcomplete.warn( @@ -987,7 +1028,7 @@ def runner_input_json_file(available_inputs, namespace, input_file, downloadable else: input_json = ( asyncio.get_event_loop() - .run_until_complete(read_source(input_file, [], None)) + .run_until_complete(make_read_source(False)(input_file, [], None)) .source_text ) input_json = YAML(typ="safe", pure=True).load(input_json) @@ -1387,6 +1428,7 @@ def localize( cfg=None, path=None, check_quant=True, + no_outside_imports=False, **kwargs, ): # set up logging @@ -1433,7 +1475,12 @@ def localize( if infile: # load WDL document - doc = load(wdlfile, path or [], check_quant=check_quant, read_source=read_source) + doc = load( + wdlfile, + path or [], + check_quant=check_quant, + read_source=make_read_source(no_outside_imports), + ) try: target, input_env, input_json = runner_input( diff --git a/tests/runner.t b/tests/runner.t index fe078a5c..4bdd178c 100644 --- a/tests/runner.t +++ b/tests/runner.t @@ -11,7 +11,7 @@ source tests/bash-tap/bash-tap-bootstrap export PYTHONPATH="$SOURCE_DIR:$PYTHONPATH" miniwdl="python3 -m WDL" -plan tests 76 +plan tests 78 $miniwdl run_self_test is "$?" "0" "run_self_test" @@ -429,3 +429,25 @@ MINIWDL__SCHEDULER__FAIL_FAST=false $miniwdl run fail_slow.wdl is "$?" "1" "fail-slow" test -f _LAST/call-succeeder/outputs.json is "$?" "0" "fail-slow -- in-progress task allowed to succeed" + +# test --no-outside-imports +cat << 'EOF' > outside.wdl +version 1.1 +task hello { + command { + echo "Hello from outside!" + } +} +EOF +mkdir inside +cat << 'EOF' > inside/inside.wdl +version 1.1 +import "../outside.wdl" +workflow w { + call outside.hello +} +EOF +$miniwdl run inside/inside.wdl +is "$?" "0" "outside import allowed" +$miniwdl run inside/inside.wdl --no-outside-imports +is "$?" "2" "outside import denied"