[PEP 706](https://peps.python.org/pep-0706/), first implemented in Python
3.11.4, mitigates some of the security issues of `TarFile.extract()` and
`TarFile.extractall()` by allowing to specify a `filter` keyword-only
parameter.
Set a safe default (`data_filter`) for the filter if available,
reverting to Python 3.11 behavior ('fully_trusted') otherwise, see
https://docs.python.org/3/library/tarfile.html#supporting-older-python-versions
Also:
- Remove unused `tar` parameter from `upload_tar()`
That's a much better consumer experience. A model generated for
gx_conditional.xml would look like
```typescript
export type ConditionalType =
| WhenTestParameterA
| WhenTestParameterB
| WhenTestParameterC
export type TestParameter = "a"
export type IntegerParameter = number
export type TestParameter1 = "b"
export type BooleanParameter = boolean
export type TestParameter2 = "c"
export type ColorParameter = string
export interface MySchema {
conditional_parameter: ConditionalType
}
export interface WhenTestParameterA {
test_parameter: TestParameter
integer_parameter: IntegerParameter
}
export interface WhenTestParameterB {
test_parameter: TestParameter1
boolean_parameter: BooleanParameter
}
export interface WhenTestParameterC {
test_parameter: TestParameter2
color_parameter: ColorParameter
}
```
instead of
```typescript
export type ConditionalType =
| WhenTestParameterA
| WhenTestParameterB
| WhenTestParameterC
export type TestParameter = string | boolean
export type IntegerParameter = number
export type TestParameter1 = string | boolean
export type BooleanParameter = boolean
export type TestParameter2 = string | boolean
export type ColorParameter = string
export interface MySchema {
conditional_parameter: ConditionalType
}
export interface WhenTestParameterA {
test_parameter: TestParameter
integer_parameter: IntegerParameter
}
export interface WhenTestParameterB {
test_parameter: TestParameter1
boolean_parameter: BooleanParameter
}
export interface WhenTestParameterC {
test_parameter: TestParameter2
color_parameter: ColorParameter
}
```
With the new model I can write an input object without looking at the
tool source, with the previous one I don't know what string I would need
to supply.
(which started failing since ubuntu-latest was moved to 24.04)
by upgrading apptainer to >=1.3.3 to include
https://github.com/apptainer/apptainer/pull/2262 .
Fix:
```
> assert "samtools:1.0--1" in results["passed"], results
E AssertionError:
E {'failed': [{'commands': ['python -c "import pyBigWig; assert(pyBigWig.numpy '
E '== 1); assert(pyBigWig.remote == 1)"'],
E 'container': 'pybigwig:0.3.22--py36h54a71a5_0',
E 'errors': [{'command': 'python -c "import pyBigWig; '
E 'assert(pyBigWig.numpy == 1); '
E 'assert(pyBigWig.remote == 1)"',
E 'output': '\x1b[91mERROR : Could not write info to '
E 'setgroups: Permission denied\n'
E '\x1b[0m\x1b[91mERROR : Error while '
E 'waiting event for user namespace mappings: '
E 'no event received\n'
E '\x1b[0m'},
E {'import': 'pyBigWig',
E 'output': '\x1b[91mERROR : Could not write info to '
E 'setgroups: Permission denied\n'
E '\x1b[0m\x1b[91mERROR : Error while '
E 'waiting event for user namespace mappings: '
E 'no event received\n'
E '\x1b[0m'}],
E 'import_lang': 'python -c',
E 'imports': ['pyBigWig']},
E {'commands': ['samtools view --help 2>&1 | grep Notes > /dev/null'],
E 'container': 'samtools:1.0--1',
E 'errors': [{'command': 'samtools view --help 2>&1 | grep Notes > '
E '/dev/null',
E 'output': '\x1b[91mERROR : Could not write info to '
E 'setgroups: Permission denied\n'
E '\x1b[0m\x1b[91mERROR : Error while '
E 'waiting event for user namespace mappings: '
E 'no event received\n'
E '\x1b[0m'}],
E 'import_lang': 'python -c'}],
E 'notest': ['yasm:1.3.0--0'],
E 'passed': []}
E assert 'samtools:1.0--1' in []
```
Also:
- Improve assertion messages to facilitate debugging
(which started failing since ubuntu-latest was moved to 24.04)
by upgrading apptainer to >=1.3.3 to include
https://github.com/apptainer/apptainer/pull/2262 .
Fix:
```
> assert "samtools:1.0--1" in results["passed"], results
E AssertionError:
E {'failed': [{'commands': ['python -c "import pyBigWig; assert(pyBigWig.numpy '
E '== 1); assert(pyBigWig.remote == 1)"'],
E 'container': 'pybigwig:0.3.22--py36h54a71a5_0',
E 'errors': [{'command': 'python -c "import pyBigWig; '
E 'assert(pyBigWig.numpy == 1); '
E 'assert(pyBigWig.remote == 1)"',
E 'output': '\x1b[91mERROR : Could not write info to '
E 'setgroups: Permission denied\n'
E '\x1b[0m\x1b[91mERROR : Error while '
E 'waiting event for user namespace mappings: '
E 'no event received\n'
E '\x1b[0m'},
E {'import': 'pyBigWig',
E 'output': '\x1b[91mERROR : Could not write info to '
E 'setgroups: Permission denied\n'
E '\x1b[0m\x1b[91mERROR : Error while '
E 'waiting event for user namespace mappings: '
E 'no event received\n'
E '\x1b[0m'}],
E 'import_lang': 'python -c',
E 'imports': ['pyBigWig']},
E {'commands': ['samtools view --help 2>&1 | grep Notes > /dev/null'],
E 'container': 'samtools:1.0--1',
E 'errors': [{'command': 'samtools view --help 2>&1 | grep Notes > '
E '/dev/null',
E 'output': '\x1b[91mERROR : Could not write info to '
E 'setgroups: Permission denied\n'
E '\x1b[0m\x1b[91mERROR : Error while '
E 'waiting event for user namespace mappings: '
E 'no event received\n'
E '\x1b[0m'}],
E 'import_lang': 'python -c'}],
E 'notest': ['yasm:1.3.0--0'],
E 'passed': []}
E assert 'samtools:1.0--1' in []
```
Also:
- Improve assertion messages to facilitate debugging
This required other changes included here as well:
1) Extend `parse_xml()` to accept a `Traversable` (i.e. the output of
`resource_path()`.
Open the file before passing it to `etree.parse()`, since lxml's
implementation probably doesn't support a `Traversable`. This also
simplifies the exception handling, since the workaround for the missing
`errno` in the `OSError` exception is not needed any more.
------
2) Fix Galaxy startup when the ``data_manager_config_file`` config option
is set to an empty value
When does this happen?
----------------------
planemo (which we use in the converters tests) set this option to an
empty value when not testing data managers:
https://github.com/galaxyproject/planemo/blob/master/planemo/galaxy/config.py#L452
This empty config value was resolved to the `path_resolves_to` directory
for this option.
With the change in 1) above, this results in the following traceback at
Galaxy startup:
```
Traceback (most recent call last):
File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/lib/galaxy/webapps/galaxy/buildapp.py", line 68, in app_pair
app = galaxy.app.UniverseApplication(global_conf=global_conf, is_webapp=True, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/lib/galaxy/app.py", line 777, in __init__
self.data_managers = self._register_singleton(DataManagers)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/lib/galaxy/di.py", line 27, in _register_singleton
instance = self[dep_type]
~~~~^^^^^^^^^^
File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/.venv/lib/python3.12/site-packages/lagom/container.py", line 381, in __getitem__
return self.resolve(dep)
^^^^^^^^^^^^^^^^^
File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/.venv/lib/python3.12/site-packages/lagom/container.py", line 265, in resolve
return self._reflection_build_with_err_handling(dep_type, suppress_error)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/.venv/lib/python3.12/site-packages/lagom/container.py", line 392, in _reflection_build_with_err_handling
return self._reflection_build(dep_type)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/.venv/lib/python3.12/site-packages/lagom/container.py", line 408, in _reflection_build
return dep_type(**sub_deps) # type: ignore
^^^^^^^^^^^^^^^^^^^^
File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/lib/galaxy/tools/data_manager/manager.py", line 44, in __init__
self.load_from_xml(filename)
File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/lib/galaxy/tools/data_manager/manager.py", line 58, in load_from_xml
tree = util.parse_xml(xml_filename)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/lib/galaxy/util/__init__.py", line 360, in parse_xml
with source.open("rb") as f:
^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.12/pathlib.py", line 1013, in open
return io.open(self, mode, buffering, encoding, errors, newline)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
IsADirectoryError: [Errno 21] Is a directory: '/tmp/tmps2qq5htq'
```
Why is this a problem now?
--------------------------
Before this commit, `util.parse_xml()` was raising a `XMLSyntaxError`
exception if passed a directory, but now it raises an `IsADirectoryError`,
which is a subclass of `OSError` with errno 21.
As a consequence, when `load_from_xml()` in
lib/galaxy/tools/data_manager/manager.py
calls `util.parse_xml()` passing a directory, the resulting exception
is now re-raised in the `except OSError as e:` branch instead of
just being logged, which is actually good since this is a
misconfiguration.
Solutions
--------
- Do not resolve an empty/None path to the `path_resolves_to` directory.
- Do not check empty/None paths against root
- Adjst unit tests accordingly
- Raise an OSError if the `job_config_file` config option is set to null
(previously this would be resolved to a directory, also raising an
OSError when trying to open that as a file).
N.B.: This will also need to be fixed in Planemo by not setting the
`data_manager_config_file` option when not needed.
`__package__` is deprecated since Python 3.13, see
https://docs.python.org/3/reference/datamodel.html#module.__package__
`importlib.resources.files()` can be passed a non-package module
since Python 3.12 (or importlib-resources 5.10.0, see
https://importlib-resources.readthedocs.io/en/latest/history.html#v5-10-0 ),
so we can use `__name__` instead of `__package__`.
Also, I noticed that in lib/galaxy/config/__init__.py we are passing
a directory to `importlib.resources.as_file()`, which is supported
only since Python 3.12 (or importlib-resources 5.9.0).
For the reasons above:
- Update conditional imports in `lib/galaxy/util/resources.py` to
import from `importlib.resources` only for Python 3.12,
otherwise from the importlib-resources package.
- Update importlib-resources requirement in galaxy-util package.
- Drop importlib-resources requirement from galaxy-web-apps as it's
not imported directly from its code any more.
This required other changes included here as well:
1) Extend `parse_xml()` to accept a `Traversable` (i.e. the output of
`resource_path()`.
Open the file before passing it to `etree.parse()`, since lxml's
implementation probably doesn't support a `Traversable`. This also
simplifies the exception handling, since the workaround for the missing
`errno` in the `OSError` exception is not needed any more.
------
2) Fix Galaxy startup when the ``data_manager_config_file`` config option
is set to an empty value
When does this happen?
----------------------
planemo (which we use in the converters tests) set this option to an
empty value when not testing data managers:
https://github.com/galaxyproject/planemo/blob/master/planemo/galaxy/config.py#L452
This empty config value was resolved to the `path_resolves_to` directory
for this option.
With the change in 1) above, this results in the following traceback at
Galaxy startup:
```
Traceback (most recent call last):
File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/lib/galaxy/webapps/galaxy/buildapp.py", line 68, in app_pair
app = galaxy.app.UniverseApplication(global_conf=global_conf, is_webapp=True, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/lib/galaxy/app.py", line 777, in __init__
self.data_managers = self._register_singleton(DataManagers)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/lib/galaxy/di.py", line 27, in _register_singleton
instance = self[dep_type]
~~~~^^^^^^^^^^
File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/.venv/lib/python3.12/site-packages/lagom/container.py", line 381, in __getitem__
return self.resolve(dep)
^^^^^^^^^^^^^^^^^
File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/.venv/lib/python3.12/site-packages/lagom/container.py", line 265, in resolve
return self._reflection_build_with_err_handling(dep_type, suppress_error)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/.venv/lib/python3.12/site-packages/lagom/container.py", line 392, in _reflection_build_with_err_handling
return self._reflection_build(dep_type)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/.venv/lib/python3.12/site-packages/lagom/container.py", line 408, in _reflection_build
return dep_type(**sub_deps) # type: ignore
^^^^^^^^^^^^^^^^^^^^
File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/lib/galaxy/tools/data_manager/manager.py", line 44, in __init__
self.load_from_xml(filename)
File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/lib/galaxy/tools/data_manager/manager.py", line 58, in load_from_xml
tree = util.parse_xml(xml_filename)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/users/ga002/soranzon/software/nsoranzo_galaxy/lib/galaxy/util/__init__.py", line 360, in parse_xml
with source.open("rb") as f:
^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.12/pathlib.py", line 1013, in open
return io.open(self, mode, buffering, encoding, errors, newline)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
IsADirectoryError: [Errno 21] Is a directory: '/tmp/tmps2qq5htq'
```
Why is this a problem now?
--------------------------
Before this commit, `util.parse_xml()` was raising a `XMLSyntaxError`
exception if passed a directory, but now it raises an `IsADirectoryError`,
which is a subclass of `OSError` with errno 21.
As a consequence, when `load_from_xml()` in
lib/galaxy/tools/data_manager/manager.py
calls `util.parse_xml()` passing a directory, the resulting exception
is now re-raised in the `except OSError as e:` branch instead of
just being logged, which is actually good since this is a
misconfiguration.
Solutions
--------
- Do not resolve an empty/None path to the `path_resolves_to` directory.
- Do not check empty/None paths against root
- Adjst unit tests accordingly
- Raise an OSError if the `job_config_file` config option is set to null
(previously this would be resolved to a directory, also raising an
OSError when trying to open that as a file).
N.B.: This will also need to be fixed in Planemo by not setting the
`data_manager_config_file` option when not needed.
John Davis