diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 27c838ca..65f5ddf3 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -23,7 +23,12 @@ Added set of standard library paths that give code execution, e.g. ``os``, ``subprocess`` and ``pickle``, is denied by default, see :ref:`untrusted-configs` (`#959 - `__). + `__, `#960 + `__). +- Class types nested in a ``tuple``, ``set``, ``frozenset`` or mapping, e.g. + ``dict[str, SomeBaseClass]``, now have a ``--*.help`` option, and subclasses + in any container are now included in the known subclasses shown in the help + (`#960 `__). Fixed ^^^^^ @@ -35,6 +40,16 @@ Fixed - Instance factory protocols with a ``__call__`` that takes no parameters instantiated the class instead of giving a factory (`#959 `__). +- ``Literal`` with arguments that are attributes of the class in whose body the + method is defined failed to resolve when annotations are postponed (`#960 + `__). +- ``set`` and ``frozenset`` of a class type failed to parse because subclass + specs are not hashable (`#960 + `__). +- Shell completion of a ``--*.help`` option gave wrong choices when the class + type is nested in a container or optional, e.g. ``builtins.NoneType`` for + ``Optional[list[SomeBaseClass]]`` (`#960 + `__). Deprecated ^^^^^^^^^^ diff --git a/DOCUMENTATION.rst b/DOCUMENTATION.rst index 39d5b3b1..0ff974d2 100644 --- a/DOCUMENTATION.rst +++ b/DOCUMENTATION.rst @@ -561,7 +561,9 @@ Some notes about this support are: element position can have its own type and will be validated as such. ``tuple`` with ellipsis (``tuple[type, ...]``) is also supported. In command line arguments, config files and environment variables, tuples and sets are - represented as an array. + represented as an array. A ``set`` or ``frozenset`` of a class type is kept as + a list when parsing, since subclass specs are not hashable, and becomes a set + on :meth:`instantiate <.ArgumentParser.instantiate>`. - To set a value to ``None`` it is required to use ``null`` since this is how JSON/YAML defines it. To avoid confusion in the help, ``NoneType`` is @@ -2399,14 +2401,20 @@ An entry denies or allows a dot import path and everything under it, so ``os`` also denies ``os.system``. The most specific entry decides, which is why ``functools.partial`` above is allowed even though ``functools`` is denied by default. An entry given in both lists is allowed, so naming a default entry in -``import_path_allowlist`` is how to stop denying it. +``import_path_allowlist`` is how to stop denying it. The one exception is +``jsonargparse`` itself, which is denied by default and not accepted in +``import_path_allowlist``, since a value that names it would be able to call +:func:`.set_parsing_settings` and thus change the policy that is checking it. An object is denied by where it is defined, not only by the path used to reach it. Modules commonly import others, e.g. ``import os``, so without this ``some.module.os.system`` would give the same object as the denied ``os.system``. This second check can only happen once the object is resolved, so it prevents the object from being used, unlike the check on the given path, -which prevents the import from happening at all. +which prevents the import from happening at all. An object that has no defining +path of its own is denied by the callable it reaches, i.e. the bound function +for a ``functools.partial`` and the defining class for an instance, e.g. +``builtins.help`` is an instance of the ``_sitebuiltins._Helper`` class. Entries given are added to the ones denied by default, they don't replace them. For configs that are entirely untrusted, prefer denying everything and allowing @@ -2425,6 +2433,16 @@ only what the application expects. The ``*`` entry is only accepted in _common.parsing_settings.clear() _common.parsing_settings.update(saved_import_path_settings) +The denylist is not the only thing that limits what a config can reach. Type +hints do as well, since a ``class_path`` is only accepted where the annotation +allows one, and must name a subclass of the annotated type. The exceptions are +``Any`` and ``object``, which accept a subclass spec of any class, see +:ref:`sub-classes`. Setting ``instantiate_subclass_spec_in_any=False``, which is +the default from v5.0.0, keeps these values as plain dicts, so nothing is +imported or instantiated and the code that receives the dict decides what to do +with it. The denylist still applies when ``validate_subclass_spec_in_any=True``, +since validating a spec requires importing the class it names. + .. note:: A denylist is a mitigation, not a sandbox. A large enough set of installed @@ -2433,6 +2451,15 @@ only what the application expects. The ``*`` entry is only accepted in Only ``*`` plus a narrow allowlist gives a bound on what a config can import. +.. note:: + + The ``omegaconf`` parser modes, see :ref:`omegaconf-interpolation`, give a + config access to OmegaConf's resolvers, which the import path denylist does + not check. The built-in ``oc.env`` resolver reads environment variables, so + a value of ``${oc.env:AWS_SECRET_ACCESS_KEY}`` puts that variable's value + into the config, and the resolvers that the application registers are + equally reachable. Avoid these parser modes for untrusted configs. + .. note:: Until v5.0.0 a denied import path only gives a deprecation warning and the @@ -3050,6 +3077,8 @@ instantiates ``Data`` first, then use the ``num_classes`` attribute to instantiate ``Model``. +.. _omegaconf-interpolation: + OmegaConf variable interpolation ================================ diff --git a/jsonargparse/_actions.py b/jsonargparse/_actions.py index 8e38cd26..b2c5e657 100644 --- a/jsonargparse/_actions.py +++ b/jsonargparse/_actions.py @@ -570,7 +570,7 @@ def __init__( ValueError: If the parser parameter is invalid. """ self._parser = parser - if not isinstance(self._parser, import_object("jsonargparse.ArgumentParser", check_path=False)): + if not isinstance(self._parser, __import__("jsonargparse").ArgumentParser): raise ValueError("Expected parser keyword argument to be an ArgumentParser.") @staticmethod diff --git a/jsonargparse/_common.py b/jsonargparse/_common.py index b71d075e..8247562d 100644 --- a/jsonargparse/_common.py +++ b/jsonargparse/_common.py @@ -135,6 +135,10 @@ class ImportDenied(ImportError, ValueError): # that exposes them and both are needed, since a pure Python implementation is # used when the C one is unavailable. default_import_path_denylist = ( + # Policy self-modification. Naming jsonargparse itself would give a value a + # way to change the settings that decide what values are allowed, which is + # why it is also not accepted in import_path_allowlist. + "jsonargparse", # Command and code execution. Instantiation is the execution, so a class # path plus init_args suffices to run arbitrary code or shell commands. "builtins.eval", @@ -158,7 +162,11 @@ class ImportDenied(ImportError, ValueError): "timeit", "pdb", "bdb", + "builtins.breakpoint", "trace", + "cProfile", + "profile", + "doctest", # Untrusted deserialization. Loading attacker influenced bytes is execution, # and only a loader plus a file path is needed, not the payload itself. "pickle", @@ -169,16 +177,22 @@ class ImportDenied(ImportError, ValueError): # Import machinery and package installation. Resolve or install code by name # at runtime, which would reopen everything the other groups deny. "importlib", + "_frozen_importlib", + "_frozen_importlib_external", "_imp", "pkgutil", "zipimport", "pydoc", + "builtins.help", + "unittest", "sys", "site", "pip", + "pkg_resources", "setuptools", "distutils", "venv", + "ensurepip", "sysconfig", # Callable adapters and reflection. Wrap or synthesize a callable so that the # call happens later in code that receives it, where no type check applies. @@ -191,6 +205,11 @@ class ImportDenied(ImportError, ValueError): "ast", "py_compile", "compileall", + "builtins.getattr", + "builtins.setattr", + "builtins.delattr", + "builtins.vars", + "builtins.globals", # Filesystem and process lifetime. Destructive or disruptive instead of # executing, e.g. deleting trees, signals or deferring a call to exit. "shutil", @@ -200,11 +219,17 @@ class ImportDenied(ImportError, ValueError): "_signal", "atexit", "gc", + "resource", + "faulthandler", + "builtins.exit", + "builtins.quit", + "_sitebuiltins", # File access. Opening a path for writing truncates or creates it, and the # archive and database openers do the same, so only a path is needed to # destroy or plant a file, not any payload. builtins.open and the io classes # are the primitives, the rest wrap them. "builtins.open", + "codecs.open", "io", "_io", "fileinput", @@ -220,6 +245,7 @@ class ImportDenied(ImportError, ValueError): "logging.config", "logging.handlers", "logging.FileHandler", + "winreg", # Network and external launch. Exfiltration primitives and launching an # external program with an argument that the config decides. The client and # server modules connect or listen with a destination the config decides. @@ -227,6 +253,7 @@ class ImportDenied(ImportError, ValueError): "_socket", "ssl", "webbrowser", + "antigravity", "asyncio", "urllib", "http", @@ -237,6 +264,7 @@ class ImportDenied(ImportError, ValueError): "nntplib", "telnetlib", "socketserver", + "xml", "xmlrpc", "wsgiref", ) @@ -272,6 +300,11 @@ def set_import_path_verdicts(denylist: list[str] | None, allowlist: list[str] | raise ValueError("'*' is only accepted in import_path_denylist, to deny all not allowed paths.") if not isinstance(entry, str) or (entry != "*" and not all(p.isidentifier() for p in entry.split("."))): raise ValueError(f"Expected import path entries to be dot import paths or '*', but got {entry!r}.") + if allowed and (entry == "jsonargparse" or entry.startswith("jsonargparse.")): + raise ValueError( + "Import paths under 'jsonargparse' can't be allowed, since a value that names them " + "would be able to change the import path policy itself." + ) previous = verdicts.get(entry) verdicts[entry] = allowed if previous is None: @@ -409,7 +442,9 @@ class when a value for a type that accepts any value, i.e. ``Any``, import_path_allowlist: Import paths that a value is allowed to name, taking precedence over the denylist for the same entry. The most specific entry decides, so ``functools.partial`` here allows only - that path out of a denied ``functools``. + that path out of a denied ``functools``. Paths under ``jsonargparse`` + are not accepted, since a value that names them would be able to + change these settings. """ # validate_defaults if isinstance(validate_defaults, bool): diff --git a/jsonargparse/_completions.py b/jsonargparse/_completions.py index 2ba4e0d1..21b17d26 100644 --- a/jsonargparse/_completions.py +++ b/jsonargparse/_completions.py @@ -1,5 +1,4 @@ import argparse -import inspect import locale import os import re @@ -27,6 +26,7 @@ callable_origin_types, get_all_subclass_paths, get_callable_return_type, + get_help_types, get_typed_dict_key_type, get_typehint_origin, is_single_subclass_or_closed_type, @@ -439,14 +439,10 @@ def add_subactions_and_get_subclass_choices( def get_help_class_choices(typehint) -> list[str]: - choices = [] - if get_typehint_origin(typehint) == Union: - for subtype in typehint.__args__: - # a subscripted generic typed dict is a generic alias instead of a class - if inspect.isclass(subtype) or is_typed_dict(subtype): - choices.extend(get_help_class_choices(subtype)) - elif is_typed_dict(typehint): - choices = [typehint.__name__] # typed dicts don't accept a class path, only their name - else: - choices = get_all_subclass_paths(typehint) + choices: list[str] = [] + for help_type in get_help_types(typehint) or []: + if is_typed_dict(help_type): + choices.append(help_type.__name__) # typed dicts don't accept a class path, only their name + else: + choices += [p for p in get_all_subclass_paths(help_type) if p not in choices] return choices diff --git a/jsonargparse/_postponed_annotations.py b/jsonargparse/_postponed_annotations.py index 689c4f50..9015a38a 100644 --- a/jsonargparse/_postponed_annotations.py +++ b/jsonargparse/_postponed_annotations.py @@ -4,11 +4,12 @@ import sys import textwrap from dataclasses import is_dataclass +from enum import Enum from importlib import import_module from types import UnionType from typing import Any, ForwardRef, TypeAlias, TypeVar, Union, get_type_hints -from ._typehints import mapping_origin_types, sequence_origin_types, tuple_set_origin_types +from ._typehints import literal_types, mapping_origin_types, sequence_origin_types, tuple_set_origin_types from ._util import get_typehint_origin _TRIGGER_MODULE_CACHE_MAXSIZE = 1024 @@ -304,11 +305,49 @@ class that defines it, so names defined there, e.g. a nested class, must be return {key: value for key, value in vars(owner).items() if is_type_like(value)} +literal_value_types = (bool, bytes, int, str, Enum) + + +def get_local_literal_values(owner: Any) -> dict: + """Returns the names defined in the body of a class whose value is valid as a Literal argument.""" + if not inspect.isclass(owner): # None when the method has no owner, i.e. a plain function + return {} + return {key: value for key, value in vars(owner).items() if isinstance(value, literal_value_types)} + + +def is_literal_ast(node: ast.AST, aliases: dict) -> bool: + """Whether an ast node in the position of the value of a subscript is a Literal.""" + value = None + if isinstance(node, ast.Name): + value = aliases.get(node.id) + elif isinstance(node, ast.Attribute) and isinstance(node.value, ast.Name): + value = getattr(aliases.get(node.value.id), node.attr, None) + return any(value is literal_type for literal_type in literal_types) + + +def add_literal_value_aliases(arg_ast: ast.AST, aliases: dict, literal_values: dict) -> dict: + """Adds to the aliases the class attributes used as arguments of a Literal. + + Strictly a Literal is invalid when its arguments are variables. Still, it is written, e.g. + ``Literal[TRAIN_SET, VALIDATION_SET]`` referencing attributes of the class in whose body the + method is defined. Since these are values and not types, they are only added for the names in + the subscript of a Literal, so that a class attribute never shadows a global in a type position. + """ + names: set[str] = set() + if literal_values: + for node in ast.walk(arg_ast): + if isinstance(node, ast.Subscript) and is_literal_ast(node.value, aliases): + names.update(NamesVisitor().find(node.slice)) + values = {name: literal_values[name] for name in names if name in literal_values} + return {**aliases, **values} if values else aliases + + def get_types(obj: Any, logger: logging.Logger | None = None, parent: Any = None) -> dict: global_vars = get_global_vars(obj, logger) # Locals are only needed for methods. For a class get_type_hints already uses as locals # the namespace of each of the bases that the annotations come from. - local_vars = None if inspect.isclass(obj) else get_local_vars(get_owner_class(obj) or parent) + owner = None if inspect.isclass(obj) else get_owner_class(obj) or parent + local_vars = None if inspect.isclass(obj) else get_local_vars(owner) try: types = get_type_hints(obj, global_vars, local_vars) except Exception as ex1: @@ -337,11 +376,14 @@ def get_types(obj: Any, logger: logging.Logger | None = None, parent: Any = None arg_asts = [(a.arg, a.annotation) for a in node.args.args + node.args.kwonlyargs] # type: ignore[union-attr] + literal_values = get_local_literal_values(owner) + for name, annotation in arg_asts: if annotation and (name not in types or type_requires_eval(types[name])): + arg_aliases = add_literal_value_aliases(annotation, aliases, literal_values) try: - arg_type = get_arg_type(annotation, aliases) - types[name] = resolve_forward_refs(arg_type, aliases, logger) + arg_type = get_arg_type(annotation, arg_aliases) + types[name] = resolve_forward_refs(arg_type, arg_aliases, logger) except Exception as ex3: types[name] = ex3 diff --git a/jsonargparse/_stubs_resolver.py b/jsonargparse/_stubs_resolver.py index 0398c762..31ca3424 100644 --- a/jsonargparse/_stubs_resolver.py +++ b/jsonargparse/_stubs_resolver.py @@ -272,5 +272,5 @@ def get_stub_types(params, component, parent, logger) -> dict[str, Any] | None: f"Failed to parse type stub for {component.__qualname__!r} parameter {name!r}", exc_info=ex ) if name not in known_params: - types[name] = inspect._empty # pragma: no cover + types[name] = inspect._empty return types diff --git a/jsonargparse/_typehints.py b/jsonargparse/_typehints.py index 8dba9342..15465a51 100644 --- a/jsonargparse/_typehints.py +++ b/jsonargparse/_typehints.py @@ -790,14 +790,9 @@ def get_class_parser(val_class, sub_add_kwargs=None, skip_args=None): def extra_help(self): extra = "" - typehint = get_optional_arg(self._typehint) - typehint = get_callable_return_type(typehint) or typehint - if get_typehint_origin(typehint) is type: - typehint = typehint.__args__[0] - if self.is_subclass_typehint(typehint, all_subtypes=False): - class_paths = get_all_subclass_paths(typehint) - if class_paths: - extra = ", known subclasses: " + ", ".join(class_paths) + class_paths = get_all_subclass_paths(self._typehint, closed_types=False) + if class_paths: + extra = ", known subclasses: " + ", ".join(class_paths) return extra def completer(self, prefix, **kwargs): @@ -1421,10 +1416,10 @@ def adapt_typehints( if not serialize: if typehint_origin in {Tuple, tuple}: val = tuple(val) - elif typehint_origin is frozenset: - val = frozenset(val) - else: - val = set(val) + elif all(isinstance(v, abc.Hashable) for v in val): + val = frozenset(val) if typehint_origin is frozenset else set(val) + # values that are not hashable, e.g. subclass specs, are kept as a + # list and become a set once the classes are instantiated # List, Iterable or Sequence elif typehint_origin in sequence_origin_types: @@ -2035,20 +2030,34 @@ def is_single_class_type(typehint, typehint_origin, closed_class): is_single_subclass_or_closed_type = partial(is_single_class_type, closed_class=True) -def yield_class_types(typehint, is_single, also_lists=False, callable_return=False): +def yield_class_types(typehint, is_single, also_lists=False, also_containers=False, callable_return=False): typehint = typehint_from_action(typehint) if typehint is None: return typehint = get_unaliased_type(get_optional_arg(get_unaliased_type(typehint))) typehint_origin = get_typehint_origin(typehint) - kwargs = {"is_single": is_single, "also_lists": also_lists, "callable_return": callable_return} + kwargs = { + "is_single": is_single, + "also_lists": also_lists, + "also_containers": also_containers, + "callable_return": callable_return, + } if callable_return and (typehint_origin in callable_origin_types or is_instance_factory_protocol(typehint)): return_type = get_callable_return_type(typehint) if return_type: yield from yield_class_types(return_type, **kwargs) - elif typehint_origin == Union or (also_lists and typehint_origin in sequence_origin_types): - for subtype in typehint.__args__: + elif also_containers and typehint_origin in mapping_origin_types and not is_typed_dict(typehint): + # only the value type of mappings can be a class, since keys are always simple types + for subtype in getattr(typehint, "__args__", [])[1:]: yield from yield_class_types(subtype, **kwargs) + elif ( + typehint_origin == Union + or ((also_lists or also_containers) and typehint_origin in sequence_origin_types) + or (also_containers and typehint_origin in tuple_set_origin_types) + ): + for subtype in typehint.__args__: + if subtype is not Ellipsis: + yield from yield_class_types(subtype, **kwargs) if is_single(typehint, typehint_origin): if is_typed_dict(typehint): # a subscripted generic TypedDict is yielded as is, since its keys are @@ -2060,10 +2069,14 @@ def yield_class_types(typehint, is_single, also_lists=False, callable_return=Fal yield get_generic_origin(typehint) -def get_subclass_types(typehint, also_lists=False, callable_return=False): +def get_subclass_types(typehint, also_lists=False, also_containers=False, callable_return=False): types = tuple( yield_class_types( - typehint, is_single=is_single_subclass_type, also_lists=also_lists, callable_return=callable_return + typehint, + is_single=is_single_subclass_type, + also_lists=also_lists, + also_containers=also_containers, + callable_return=callable_return, ) ) return types or None @@ -2087,7 +2100,9 @@ def is_single_help_type(typehint, typehint_origin): def get_help_types(typehint): """Types in a type hint for which a --*.help option shows the accepted arguments.""" - types = tuple(yield_class_types(typehint, is_single=is_single_help_type, also_lists=True, callable_return=True)) + types = tuple( + yield_class_types(typehint, is_single=is_single_help_type, also_containers=True, callable_return=True) + ) return types or None @@ -2132,7 +2147,7 @@ def adapt_partial_callable_class(callable_type, subclass_spec): return subclass_spec, partial_skip_args -def get_all_subclass_paths(cls: type, include_abstract: bool = False) -> list[str]: +def get_all_subclass_paths(cls: type, include_abstract: bool = False, closed_types: bool = True) -> list[str]: subclass_list = [] def is_local(cl): @@ -2142,10 +2157,6 @@ def is_private(class_path): return "._" in class_path def add_subclasses(cl): - if hasattr(cl, "__args__") and get_typehint_origin(cl) in sequence_origin_types.union({Union}): - for arg in cl.__args__: - add_subclasses(arg) - return try: class_path = get_import_path(cl) except (ImportError, AttributeError) as err: # Attribute is added in case of dot notation imports @@ -2160,19 +2171,29 @@ def add_subclasses(cl): for subclass in cl.__subclasses__() if hasattr(cl, "__subclasses__") else []: add_subclasses(subclass) - if get_typehint_origin(cls) in callable_origin_types: - cls = cls.__args__[-1] # type: ignore[attr-defined] - - if get_typehint_origin(cls) in {Union, Type, type}: - for arg in cls.__args__: # type: ignore[union-attr] - if ActionTypeHint.is_subclass_typehint(arg, also_lists=True) and arg not in {object, type}: - add_subclasses(arg) - else: - add_subclasses(cls) + for class_type in get_class_types(cls, closed_types=closed_types): + add_subclasses(class_type) return subclass_list +def get_class_types(typehint, closed_types: bool = True) -> tuple: + """Classes in a type hint for which a class path is accepted as value. + + Types that have subclasses disabled only accept their own class path, so they + are excluded when closed_types is False, e.g. to show the known subclasses. + """ + typehint = get_unaliased_type(get_optional_arg(get_unaliased_type(typehint))) + if get_typehint_origin(typehint) in {Type, type}: + args = getattr(typehint, "__args__", ()) + return tuple(a for a in args if ActionTypeHint.is_subclass_typehint(a)) + if inspect.isclass(typehint) or is_generic_class(typehint): + cls = get_generic_origin(typehint) + is_single = is_single_subclass_or_closed_type if closed_types else is_single_subclass_type + return (cls,) if is_single(cls, get_typehint_origin(cls)) else () + return get_subclass_types(typehint, also_containers=True, callable_return=True) or () + + def resolve_class_path_by_name(cls: type | tuple[type], name: str) -> str: class_path = name if "." not in class_path: diff --git a/jsonargparse_tests/test_import_paths.py b/jsonargparse_tests/test_import_paths.py index 5a9417a1..6dd0dae5 100644 --- a/jsonargparse_tests/test_import_paths.py +++ b/jsonargparse_tests/test_import_paths.py @@ -114,14 +114,74 @@ def test_star_denies_everything_not_allowed(): check_import_path("json.JSONEncoder") -def test_only_builtins_code_execution_names_denied(): +def test_jsonargparse_denied_by_default(): set_parsing_settings(import_path_denylist=[]) - check_import_path("builtins.print") - for name in ["eval", "exec", "compile", "__import__"]: + with pytest.raises(ImportDenied, match="'jsonargparse'"): + check_import_path("jsonargparse.set_parsing_settings") + + +@pytest.mark.parametrize("entry", ["jsonargparse", "jsonargparse._common.parsing_settings"]) +def test_jsonargparse_not_accepted_in_allowlist(entry): + set_parsing_settings(import_path_denylist=[]) + with pytest.raises(ValueError, match="Import paths under 'jsonargparse' can't be allowed"): + set_parsing_settings(import_path_allowlist=[entry]) + with pytest.raises(ImportDenied, match="'jsonargparse'"): + check_import_path("jsonargparse.set_parsing_settings") + + +def test_allowlist_accepts_package_named_after_jsonargparse(): + set_parsing_settings(import_path_denylist=["*"], import_path_allowlist=["jsonargparse_tests"]) + check_import_path("jsonargparse_tests.test_import_paths.Data") + + +denied_builtins = [ + "eval", + "exec", + "compile", + "__import__", + "breakpoint", # enters pdb + "help", # instance that runs pydoc + "exit", # instance that raises SystemExit + "quit", + "getattr", # reflection, the builtins counterparts of the denied inspect and operator + "setattr", + "delattr", + "vars", + "globals", +] + + +def test_only_dangerous_builtins_denied(): + set_parsing_settings(import_path_denylist=[]) + for name in ["print", "int", "sorted"]: + check_import_path(f"builtins.{name}") + for name in denied_builtins: with pytest.raises(ImportDenied): check_import_path(f"builtins.{name}") +@pytest.mark.parametrize( + "path", + [ + "cProfile.run", + "profile.Profile.runctx", + "doctest.testfile", + "_frozen_importlib.__import__", + "_frozen_importlib_external.SourceFileLoader", + "pkg_resources.load_entry_point", + "ensurepip.bootstrap", + "unittest.mock.patch", + "_sitebuiltins.Quitter", + "resource.setrlimit", + "faulthandler.dump_traceback_later", + ], +) +def test_execution_and_disruption_paths_denied_by_default(path): + set_parsing_settings(import_path_denylist=[]) + with pytest.raises(ImportDenied): + check_import_path(path) + + @pytest.mark.parametrize( "path", [ @@ -139,6 +199,10 @@ def test_only_builtins_code_execution_names_denied(): "socketserver.TCPServer", "xmlrpc.client.ServerProxy", "asyncio.create_subprocess_shell", + "codecs.open", + "winreg.SetValueEx", + "xml.sax.parse", + "antigravity", ], ) def test_file_and_network_paths_denied_by_default(path): @@ -179,9 +243,9 @@ def test_denied_before_the_module_is_imported(): def test_denied_module_reexported_by_another_module(): set_parsing_settings(import_path_denylist=[]) with pytest.raises(ImportDenied, match="'os'"): - import_object("jsonargparse._util.os") + import_object(f"{__name__}.os") with pytest.raises(ImportDenied, match=f"'{os.system.__module__}'"): - import_object("jsonargparse._util.os.system") # os.system is defined in posix, nt on Windows + import_object(f"{__name__}.os.system") # os.system is defined in posix, nt on Windows def test_denied_object_reexported_under_another_name(): @@ -202,6 +266,12 @@ def test_denied_callable_exposed_by_an_instance(): import_object(f"{__name__}.attr_getter") # instance of the denied operator.attrgetter +def test_denied_instance_by_its_defining_class(): + set_parsing_settings(import_path_allowlist=["builtins.help"]) + with pytest.raises(ImportDenied, match="'_sitebuiltins'"): + import_object("builtins.help") # instance of _sitebuiltins._Helper, which reaches pydoc + + def test_object_without_canonical_path_is_not_rechecked(): set_parsing_settings(import_path_denylist=[]) assert import_object("calendar.day_name") is calendar.day_name @@ -270,6 +340,16 @@ def test_star_callable_bound_to_denied_callable_denied(parser): parser.parse_args([f"--fn={__name__}.system_partial"]) +def test_parsing_settings_class_path_denied(parser): + set_parsing_settings(import_path_denylist=[]) + parser.add_argument("--fn", type=Callable) + spec = json.dumps( + {"class_path": "jsonargparse.set_parsing_settings", "init_args": {"import_path_allowlist": ["os"]}} + ) + with pytest.raises(ArgumentError, match="not allowed"): + parser.parse_args([f"--fn={spec}"]) + + def test_any_type_class_path_denied(parser): set_parsing_settings(import_path_denylist=[], validate_subclass_spec_in_any=True) parser.add_argument("--any", type=Any) diff --git a/jsonargparse_tests/test_postponed_annotations.py b/jsonargparse_tests/test_postponed_annotations.py index 69b9c13a..0a8e9af5 100644 --- a/jsonargparse_tests/test_postponed_annotations.py +++ b/jsonargparse_tests/test_postponed_annotations.py @@ -9,7 +9,19 @@ from collections.abc import Callable from textwrap import dedent from types import GenericAlias, SimpleNamespace, UnionType -from typing import TYPE_CHECKING, Dict, ForwardRef, List, Optional, Protocol, Tuple, Type, TypedDict, Union +from typing import ( + TYPE_CHECKING, + Dict, + ForwardRef, + List, + Literal, + Optional, + Protocol, + Tuple, + Type, + TypedDict, + Union, +) from unittest.mock import patch import pytest @@ -690,6 +702,42 @@ def test_get_types_class_scope_non_type_attribute_does_not_shadow(): assert types == {"path": Optional[Path_drw]} +LITERAL_MODULE_VALUE = "module" +LITERAL_SHADOWED_VALUE = "global" + + +class ClassScopeLiteralValues: + TRAIN_SET = "train" + VALIDATION_SET = "validation" + LITERAL_SHADOWED_VALUE = "class" + + def __init__( + self, + example_set: Literal[TRAIN_SET, VALIDATION_SET] = VALIDATION_SET, # type: ignore[valid-type] + qualified: typing.Literal[TRAIN_SET] = TRAIN_SET, # type: ignore[valid-type] + mixed: Optional[Literal[TRAIN_SET, LITERAL_MODULE_VALUE]] = None, # type: ignore[valid-type] + shadowed: Literal[LITERAL_SHADOWED_VALUE] = LITERAL_SHADOWED_VALUE, # type: ignore[valid-type] + ): + self.example_set = example_set # pragma: no cover + + +def test_get_types_class_scope_literal_values(): + types = get_types(ClassScopeLiteralValues.__init__) + assert types == { + "example_set": Literal["train", "validation"], + "qualified": Literal["train"], + "mixed": Optional[Literal["train", "module"]], + "shadowed": Literal["class"], + } + + +def test_parse_class_scope_literal_values(parser): + parser.add_class_arguments(ClassScopeLiteralValues, "s") + assert parser.parse_args(["--s.example_set=train"]).s.example_set == "train" + with pytest.raises(ArgumentError, match=r"Expected a typing.Literal\['train', 'validation']"): + parser.parse_args(["--s.example_set=test"]) + + class ClassScopeTypeVarAttribute: ScopedTypeVar = typing.TypeVar("ScopedTypeVar", bound=int) diff --git a/jsonargparse_tests/test_shtab.py b/jsonargparse_tests/test_shtab.py index b494a968..4c9d2e6d 100644 --- a/jsonargparse_tests/test_shtab.py +++ b/jsonargparse_tests/test_shtab.py @@ -13,7 +13,7 @@ from importlib.util import find_spec from os import PathLike from pathlib import Path -from typing import Any, Callable, Generic, Literal, Optional, TypedDict, TypeVar, Union +from typing import Any, Callable, Dict, Generic, List, Literal, Optional, TypedDict, TypeVar, Union from unittest.mock import patch import pytest @@ -560,6 +560,14 @@ def test_bash_union_subclasses(parser, subtests): ) +@pytest.mark.parametrize("container_type", [Optional[List[Base]], Dict[str, Base]]) +def test_bash_subclasses_in_container_help_choices(parser, container_type): + parser.add_argument("--cls", type=container_type) + shtab_script = get_shtab_script(parser, "bash") + classes = [f"{__name__}.Base", f"{__name__}.SubA", f"{__name__}.SubB"] + assert get_bash_array(shtab_script, "_shtab_tool___cls_help_choices") == classes + + class SupBase: def __init__(self, s1: Base): pass # pragma: no cover diff --git a/jsonargparse_tests/test_stubs_resolver.py b/jsonargparse_tests/test_stubs_resolver.py index e236a5d8..affc180c 100644 --- a/jsonargparse_tests/test_stubs_resolver.py +++ b/jsonargparse_tests/test_stubs_resolver.py @@ -3,6 +3,7 @@ import ast import inspect import sys +from asyncio.subprocess import create_subprocess_exec from calendar import Calendar, TextCalendar from contextlib import contextmanager from email.headerregistry import DateHeader @@ -241,6 +242,25 @@ def alias_is_unique(aliases, name, source, value): assert "non-unique alias 'UUID': problem (module)" in logs.getvalue() +def test_get_params_stub_only_param_with_unresolvable_type(parser, logger): + # create_subprocess_exec accepts **kwds, so the stub has keyword-only params not in its + # signature. The type of env fails to resolve because subprocess._ENV only exists in stubs. + with capture_logs(logger) as logs: + params = get_params(create_subprocess_exec, logger=logger) + assert "Failed to parse type stub for 'create_subprocess_exec' parameter 'env'" in logs.getvalue() + param_types = dict(get_param_types(params)) + assert param_types["creationflags"] is int + assert param_types["env"] is inspect._empty + env = next(p for p in params if p.name == "env") + assert env.kind is inspect.Parameter.KEYWORD_ONLY + assert str(env.default) == "Unknown" + + parser.add_function_arguments(create_subprocess_exec, fail_untyped=False) + help_str = get_parser_help(parser) + assert "--creationflags CREATIONFLAGS" in help_str + assert "--env ENV" in help_str + + @skip_if_requests_unavailable def test_get_params_complex_function_requests_get(parser): from requests import get diff --git a/jsonargparse_tests/test_subclasses.py b/jsonargparse_tests/test_subclasses.py index 04da0842..e39899e4 100644 --- a/jsonargparse_tests/test_subclasses.py +++ b/jsonargparse_tests/test_subclasses.py @@ -15,12 +15,14 @@ Any, Callable, Dict, + FrozenSet, Generic, Iterable, List, Mapping, Optional, Protocol, + Set, Tuple, Type, TypeVar, @@ -249,6 +251,83 @@ def test_subclass_known_subclasses_multiple_bases(parser, allow_gzip_import_path assert class_path in help_str +# subclasses in containers tests + + +container_types = [ + List[BaseC], + Optional[List[BaseC]], + Iterable[BaseC], + Set[BaseC], + FrozenSet[BaseC], + Optional[Set[BaseC]], + Tuple[BaseC, int], + Tuple[BaseC, ...], + Dict[str, BaseC], + Optional[Dict[str, BaseC]], + Mapping[str, BaseC], + Optional[Union[Dict[str, BaseC], str]], +] + + +@pytest.mark.parametrize("container_type", container_types) +def test_subclass_in_container_help(parser, container_type): + parser.add_argument("--op", type=container_type) + help_str = " ".join(get_parser_help(parser).split()) + assert "Show the help for the given subclass of BaseC" in help_str + assert f"known subclasses: {__name__}.BaseC, {__name__}.SubA, {__name__}.SubB" in help_str + class_help_str = get_parse_args_stdout(parser, [f"--op.help={__name__}.SubA"]) + assert "--op.p P" in class_help_str + + +def test_subclass_in_dict_parse_and_instantiate(parser): + parser.add_argument("--op", type=Optional[Dict[str, BaseC]]) + value = {"a": f"{__name__}.SubA", "b": {"class_path": "SubB", "init_args": {"p": 2}}} + cfg = parser.parse_args([f"--op={json.dumps(value)}"]) + assert cfg.op["a"] == Namespace(class_path=f"{__name__}.SubA", init_args=Namespace(p=0)) + init = parser.instantiate(cfg) + assert isinstance(init.op["a"], SubA) + assert isinstance(init.op["b"], SubB) + assert init.op["b"].p == 2 + + +def test_subclass_in_set_parse_and_instantiate(parser): + parser.add_argument("--op", type=Set[BaseC]) + cfg = parser.parse_args([f'--op=["{__name__}.SubA", "{__name__}.SubB"]']) + # subclass specs are not hashable, so they are a list until the classes are instantiated + assert cfg.op == [ + Namespace(class_path=f"{__name__}.SubA", init_args=Namespace(p=0)), + Namespace(class_path=f"{__name__}.SubB", init_args=Namespace(p=0)), + ] + assert json_or_yaml_load(parser.dump(cfg))["op"] == [ + {"class_path": f"{__name__}.SubA", "init_args": {"p": 0}}, + {"class_path": f"{__name__}.SubB", "init_args": {"p": 0}}, + ] + init = parser.instantiate(cfg) + assert {type(x) for x in init.op} == {SubA, SubB} + + +@final +class ClosedC: + def __init__(self, p: int = 0): + pass # pragma: no cover + + +@pytest.mark.parametrize("closed_type", [Optional[ClosedC], List[ClosedC], Dict[str, ClosedC]]) +def test_closed_type_no_known_subclasses(parser, closed_type): + parser.add_argument("--op", type=closed_type) + help_str = get_parser_help(parser) + assert "--op.help" in help_str + assert "known subclasses" not in help_str + + +def test_subclass_as_dict_key_no_help(parser): + parser.add_argument("--op", type=Dict[BaseC, int]) + help_str = get_parser_help(parser) + assert "--op.help" not in help_str + assert "known subclasses" not in help_str + + # abstract class tests @@ -2384,15 +2463,15 @@ def test_subclass_multifile_save(parser, tmp_cwd): def test_subclass_error_not_subclass(parser): parser.add_argument("--op", type=BaseC) with pytest.raises(ArgumentError) as ctx: - parser.parse_args(['--op={"class_path": "jsonargparse.ArgumentParser"}']) + parser.parse_args(['--op={"class_path": "calendar.Calendar"}']) ctx.match("does not correspond to a subclass") def test_subclass_error_undefined_attribute(parser): parser.add_argument("--op", type=BaseC) with pytest.raises(ArgumentError) as ctx: - parser.parse_args(['--op={"class_path": "jsonargparse.DoesNotExist"}']) - ctx.match("module 'jsonargparse' has no attribute 'DoesNotExist'") + parser.parse_args(['--op={"class_path": "calendar.DoesNotExist"}']) + ctx.match("module 'calendar' has no attribute 'DoesNotExist'") def test_subclass_error_undefined_module(parser): diff --git a/jsonargparse_tests/test_typehints.py b/jsonargparse_tests/test_typehints.py index 82d6833e..7bb9f66d 100644 --- a/jsonargparse_tests/test_typehints.py +++ b/jsonargparse_tests/test_typehints.py @@ -2319,7 +2319,7 @@ def test_callable_function_path(parser): assert "(type: Callable, default: time.time)" in help_str with pytest.raises(ArgumentError) as ctx: - parser.parse_args(["--callable=jsonargparse.not_exist"]) + parser.parse_args(["--callable=calendar.not_exist"]) ctx.match("Callable expects a function or a callable class") @@ -2330,7 +2330,7 @@ def test_callable_list_of_function_paths(parser): assert [random.randint, time.time] == cfg.callables with pytest.raises(ArgumentError) as ctx: - parser.parse_args(['--callables=["jsonargparse.not_exist"]']) + parser.parse_args(['--callables=["calendar.not_exist"]']) ctx.match("Callable expects a function or a callable class") @@ -2377,7 +2377,7 @@ def test_callable_class_path_simple(parser): assert 2 == init.callable() pytest.raises(ArgumentError, lambda: parser.parse_args(["--callable={}"])) - pytest.raises(ArgumentError, lambda: parser.parse_args(["--callable=jsonargparse.SUPPRESS"])) + pytest.raises(ArgumentError, lambda: parser.parse_args(["--callable=time.timezone"])) pytest.raises(ArgumentError, lambda: parser.parse_args([f"--callable={__name__}.BaseC"])) value = {"class_path": f"{__name__}.CallableClassPath", "key": "val"} pytest.raises(ArgumentError, lambda: parser.parse_args([f"--callable={json.dumps(value)}"])) diff --git a/pyproject.toml b/pyproject.toml index fab01b9e..cf3e01a1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -269,14 +269,15 @@ deps = -e ./jsonargparse_tests allowlist_externals = sh commands = sh -c "\ - rm -rf /tmp/_without_future_annotations; \ - mkdir /tmp/_without_future_annotations; \ - cp *.py /tmp/_without_future_annotations; \ - sed -i -e '/^from __future__ import annotations$/d' /tmp/_without_future_annotations/*.py; \ + rm -rf {envtmpdir}/without_future_annotations; \ + mkdir -p {envtmpdir}/without_future_annotations; \ + cp *.py {envtmpdir}/without_future_annotations; \ + sed -i.bak -e '/^from __future__ import annotations$/d' {envtmpdir}/without_future_annotations/*.py; \ + rm -f {envtmpdir}/without_future_annotations/*.bak; \ " - python -m pytest /tmp/_without_future_annotations {posargs} + python -m pytest {envtmpdir}/without_future_annotations {posargs} commands_post = - sh -c "rm -rf /tmp/_without_future_annotations" + sh -c "rm -rf {envtmpdir}/without_future_annotations" [testenv:coverage-report] skip_install = true