search

woolfg / dokuwiki-plugin-gitbacked

Plugin for DokuWiki to store/sync files and media with a git repo
GNU General Public License v2.0
74 stars 26 forks source link

Avoid Using $_ENV #91

Closed delphij closed 10 months ago

delphij commented 10 months ago

PHP exposes sensitive information like cookies in $_ENV. Additionally, it may include command line arguments (argv) as an array in $_ENV in certain configurations, causing a warning because proc_open expects env to be an array of string and would issue a warning when it happens:

Warning: Array to string conversion

Because the existing environment information are not generally useful for Git operations anyways, simply remove the merging of existing $_ENV. Instead, the environment options passed from caller are directly passed to proc_open as is, ensuring more controlled and secure handling of environment variables.

mhoffrog commented 10 months ago

@delphij Many thanks for your contribution!

On what runtime environment(s) (OS and PHP) and on what DokuWiki version(s) did you test this update?

delphij commented 10 months ago

Hi, I tested this on FreeBSD, with php-fpm from PHP 8.3. The set up was with nginx with dokuwiki (2023-04-04a) (with "userewrite" enabled) in a PHP debugging enabled.

mhoffrog commented 10 months ago

@delphij Just kindly asking if you could have a look to my recent code review comment above? I really appreciate your solution of your finding and would like to get it merged as soon as possible - if you could spend some comment to my question. Many thanks in advance!

mhoffrog commented 10 months ago

@delphij FYI - this PR is merged now with additional commits to ensure backward compatibility. There was one test scenario on a Windows system that failed, since the PATH variable was not set anymore for the process forked. So I have updated your contribution by inheriting the original process environment by providing $this->envopts via putenv()and passing env=null to proc_open(). THere is appropriate comment in the code as well as on commit https://github.com/woolfg/dokuwiki-plugin-gitbacked/commit/a06ad155c64410ed73716ca9f957b197479bc7af.