search

thclark / pre-commit-sphinx

A pre-commit hook that will fail if documentation (eg for readthedocs.io) can't be built using sphinx
MIT License
3 stars 14 forks source link

Potential for shell injection #2

Open thclark opened 4 years ago

thclark commented 4 years ago

Malicious repositories could configure their pre-commit for shell injection here.

Use either validation of paths or (preferably) avoid os.system altogether and call sphinx directly.

When complete, report back in at https://github.com/pre-commit/pre-commit.com/pull/362

CAM-Gerlach commented 3 years ago

Not sure if this repo is still active, but another, simpler fix would be to replace the legacy os.system call with the modern, recommended alternative, subprocess.run, which (among many other things) performs proper escaping and validation on all input (so long as you don't set shell=True).

So you'd just replace that line with something like

output = subprocess.run(['sphinx-build', '-b', 'html', '-d', cache_dir, src_dir, html_dir])
ret = output.returncode