traitlets icon indicating copy to clipboard operation
traitlets copied to clipboard

Application.load_config_file loads all files with same basename as given file

Open maxnoe opened this issue 2 years ago • 3 comments

Even if load_config_file is given an absolute path to a config file, it will load all files that have the same basename that are in that directory.

This is highly surprising and potentially a security issue.

Example:

from pathlib import Path
from traitlets import Integer
from traitlets.config import Application
from tempfile import TemporaryDirectory


class Foo(Application):
    bar = Integer(0).tag(config=True)

    def start(self):
        print(self.bar)


if __name__ == "__main__":
    with TemporaryDirectory() as tmpdir:
        tmpdir = Path(tmpdir)
        json_path = tmpdir / "foo.json"
        py_path = tmpdir / "foo.py"

        # valid python, invalid
        py_path.write_text("c.Foo.bar = 10")
        json_path.write_text("Invalid json")

        app = Foo()
        app.load_config_file(py_path)
        app.start()

        # other way around
        py_path.write_text("raise Exception('You loaded the python file!')")
        json_path.write_text('{"Foo": {"bar": 11}}')

        app = Foo()
        app.load_config_file(json_path)
        app.start()

maxnoe avatar Jul 11 '23 12:07 maxnoe

Example without the errors, just showing that two files are loaded:

from pathlib import Path
from traitlets import Integer
from traitlets.config import Application
from tempfile import TemporaryDirectory


class Foo(Application):
    bar = Integer(0).tag(config=True)

    def start(self):
        print(self.bar)


if __name__ == "__main__":
    with TemporaryDirectory() as tmpdir:
        tmpdir = Path(tmpdir)
        json_path = tmpdir / "foo.json"
        py_path = tmpdir / "foo.py"

        # valid python, invalid
        py_path.write_text("c.Foo.bar = 10")
        json_path.write_text('{"Foo": {"bar": 11}}')

        app = Foo()
        app.load_config_file(py_path)
        print(app.loaded_config_files)
        app.start()

maxnoe avatar Jul 11 '23 12:07 maxnoe

The file extension returned by splittext is completely unused here https://github.com/ipython/traitlets/blob/c2d04ba8d24f6303b99c9e8e01a1ab16a1551964/traitlets/config/application.py#L946-L956

Maybe add a keyword argument to the functionignore_ext=False and check

if ignore_ext:
    new_config.merge(config)
elif fname == filename:
    new_config.merge(config)

StFroese avatar Jul 11 '23 21:07 StFroese

@StFroese that would still load the config though. Better just not remove the extension in that case.

maxnoe avatar Jul 12 '23 15:07 maxnoe