Hi all, Thank you for taking the time to look at this. I've applied all the changes I had in mind and rebased the branch so it merges cleanly against trunk. The PR is here:
https://github.com/apache/httpd/pull/750/changes I'm very much open to any further changes — whether that's naming, the directive's allowed context, the way the file is parsed, security considerations, tests, or anything else the reviewers feel would make it a better fit for httpd. Happy to iterate as much as needed; I'd rather get it right than get it in. One small thing: I haven't seen any CI checks run on the PR yet. I believe the workflows are sitting in an "awaiting approval" state, which I understand needs a maintainer to approve for a first-time contributor pushing from a fork. If someone with the right access could kick those off when convenient, that would help confirm everything is green. I also wanted to flag that I'll be away for a while starting after next week. I don't want this to end up hanging if there's interest in including it in trunk — so if anyone feels it's worth carrying forward, please feel free to pick it up, adjust it, or commit it as you see fit. No ownership on my side; I just want it to be useful. Thanks again for your time and for maintaining the project. Best regards, Cornel Isbiceanu From: Cornel Isbiceanu via dev <[email protected]> Date: Thursday, 10 September 2026 at 17:34 To: [email protected] <[email protected]> Cc: Cornel Isbiceanu <[email protected]> Subject: Re: [PROPOSAL] mod_env: new SetEnvFromFile directive (PR #750) EXTERNAL: Use caution when clicking on links or opening attachments. Hi all, Thanks everyone for the review and the discussion — it caught a real problem. I've pushed an updated branch; summary of the changes below. PR: https://github.com/apache/httpd/pull/750/changes Joe — done on all three: * Documented that backslash line continuation is respected in the file. * Credited myself in the changes-entry. * Moved the test to a pyhttpd-based one under test/modules/metadata/. Dropped the pytest_suite version. The new tests cover parsing (plain/empty/no-=/whitespace/continuation), the malformed-line warning, the .htaccess rejection below, and failure on a missing file; all pass locally. Eric, Rudiger — on the security concern, you're right, and it's the important point here. To answer Rudiger's question directly: yes, SetEnvFromFile allows strictly more than SetEnv could in .htaccess. SetEnv only sets a value the author already typed, whereas SetEnvFromFile reads the contents of a file into the environment — so as Eric noted, a .htaccess author could point it at anything readable by the server UID, and every =-bearing line would be split into an internal variable, then exfiltrated via SSI, CGI, or logging. That's a disclosure primitive we don't want to hand to untrusted per-directory config. So I've made SetEnvFromFile main-config only: RSRC_CONF | ACCESS_CONF — server config, virtual host, and <Directory>/<Location>, but not .htaccess. I didn't put it under FileInfo, since that's granted too widely, and a new AllowOverride type would still let .htaccess authors read arbitrary files. The docs now include a security note explaining the restriction. Thanks again, Cornel From: Ruediger Pluem <[email protected]> Date: Thursday, 10 September 2026 at 14:39 To: [email protected] <[email protected]> Subject: Re: [PROPOSAL] mod_env: new SetEnvFromFile directive (PR #750) EXTERNAL: Use caution when clicking on links or opening attachments. On 9/10/26 12:01 PM, Eric Covener wrote: >>> Any issue with it allowing unintended files via .htaccess that have >>> some content matching the format ? Seems like the kind of thing we'd >>> get reports on. >>> Maybe a new AllowOverride type? >> >> Honestly I haven't looked deeply into the feature, but does it allow more >> than setenv could do in .htaccess? >> Or are you concerned about possible issues while parsing such a file e.g. >> size, etc? > > With the new directive, a .htaccess user could point anywhere in the > filesystem readable by the server ID, and any line with an = would now > be split and added to the internal environment Fair point. e.g. you could source in something below /etc/sysconfig if readable. This would be something we likely would like to avoid. Hence I guess we should either add it to fileinfo or create a new type. Regards Rüdiger
