Issue 127 Open Basedir Warning On The System Page - edmozley/freeitsm GitHub Wiki

A PHP warning across the top of the System page (issue #127)

Reported by mbsouth ยท Fixed in #1447

Opening System printed a block of red PHP text above the page, every time. Nothing was broken and nothing was at risk. The interesting part of this one is not the fix, which is a single character, but the proposed fix โ€” which would have quietly switched off the warning that stops Docker users losing their files.


1. What you saw

Warning: file_exists(): open_basedir restriction in effect.
File(/.dockerenv) is not within the allowed path(s): (
/var/www/clients/client1/web6/web:/var/www/clients/client1/web6/private:
... /etc/phpmyadmin:/var/lib/phpmyadmin:/dev/random:/dev/urandom
) in .../includes/storage_persistence.php on line 73

Debian 13, Apache, PHP 8.4 under FPM. The paths are the shape a hosting control panel produces: the site's own directories, and nothing above them.


2. Why the check was there at all

FreeITSM needs to know whether it is running inside a container, because of issue #109: under Docker, docker compose up -d --build does not update the running container, it replaces it and discards everything written inside that is not on a volume. Uploads, attachments, recordings, branding and the encryption key all go, and none of it is recoverable afterwards by any route.

So the System page raises a warning before the operator runs that command. It is the only moment anything can be done.

The way it asked was simple, and correct: Docker writes a marker file, /.dockerenv, into every container it starts. Present means container.


3. Why the question was rude

open_basedir restricts PHP to a list of permitted directories. /.dockerenv sits at the root of the filesystem, which is not on anybody's list.

PHP does not answer "no" to a question about a forbidden path. It raises a warning and then answers no. Two of FreeITSM's own choices turned that from a log line into a wall of text:

  • config.php ships ini_set('display_errors', 1), so PHP diagnostics are painted into the page instead of going to the server's log. (docker/config.php sets it to 0, so the container has always been quiet and every hand-installed site has not.)
  • The check runs at system/index.php line 28, and <!DOCTYPE html> is not until line 46. So the warning was emitted before the doctype โ€” the literal first bytes of the response, sitting above the entire page, outside <html>.

โš ๏ธ The guard that exists for this cannot see it

try { $storageReport = storagePersistenceReport(); } catch (Throwable $e) { /* never break the page over a diagnostic */ }

A PHP warning is not a Throwable. The one line written to stop this diagnostic breaking the page is blind to the only way this diagnostic breaks the page. Worth remembering anywhere a try/catch is doing duty as "this can't hurt us": it covers exceptions and errors, not warnings and notices.


4. ๐Ÿ”ด The proposed fix, and why it was not taken

The reporter supplied a patch, tested it, and said it appeared to work:

// proposed
return file_exists($_SERVER['DOCUMENT_ROOT'] . '/.dockerenv');

It does silence the warning. It also permanently disables container detection, and with it the issue #109 warning, for every Docker user.

Docker writes /.dockerenv at the root of the filesystem and nowhere else. Our image serves from /var/www/html, so this tests for /var/www/html/.dockerenv, which nothing creates:

Checked Result
A .dockerenv anywhere in the repository none
The Dockerfile creating one in the web root no mention

So the function returns false for ever, storagePersistenceReport() returns applicable: false and exits before examining a single directory, and the red panel never appears again โ€” for exactly the people it was built for, on the one screen where it could still have saved their files.

Why it looked right

The reporter is not running Docker. On his server the correct answer is false, and a function that always returns false is indistinguishable from a working one. He tested honestly and the test could not have failed.

This is the general shape worth naming: a fix that removes a symptom by making a question unanswerable will always pass on a machine where the answer was going to be "no" anyway.

There is a second flaw too. $_SERVER['DOCUMENT_ROOT'] is not set under CLI โ€” it comes back as '' โ€” so under cron the patched line tests '' . '/.dockerenv', which is the original path, and would raise the identical warning. Latent today, since nothing calls this from the command line.


5. The fix

One character, for the warning

return @file_exists('/.dockerenv');

The path does not move. Where PHP is permitted to look it behaves exactly as before; where it is not, it answers false without the complaint.

A second signal, so the answer cannot be silenced

@ alone leaves a gap. Somebody running Docker and open_basedir would get a quiet false โ€” the storage warning gone, with nothing to indicate it. That is the dangerous direction, so the image now identifies itself:

ENV FREEITSM_CONTAINER=1
$marker = getenv(STORAGE_PERSISTENCE_ENV_MARKER, true);
if ($marker !== false && filter_var($marker, FILTER_VALIDATE_BOOLEAN)) {
    return true;
}

An environment variable needs no filesystem access, so no restriction can hide it. It is read with local_only = true, from the real process environment rather than anything the server folded into $_SERVER.

The marker can only ever say yes. Set to 0 it falls through to the file check rather than forcing a false โ€” the file's existing doctrine is that uncertainty resolves to silence, never to a false alarm, and nothing here should be able to invent a warning either.

โญ A bonus worth having: Podman rebuilds with the same consequences as Docker and leaves no /.dockerenv, so this file had nothing to say to those operators. Setting the variable opts them in.

The diagnostic tool tells the truth

D013 used to print /.dockerenv present: no, which on a restricted host meant "we were not allowed to look". It now separates the two:

Container detected : no
  via FREEITSM_CONTAINER: not set
  via /.dockerenv    : COULD NOT LOOK โ€” open_basedir does not permit reading /.
                       This is not the same as "no"; the marker above answers here.

6. ๐Ÿ“ Files

File What changed
includes/storage_persistence.php the marker, the @, and storagePersistenceRootReadable() for diagnostics
Dockerfile ENV FREEITSM_CONTAINER=1, with a do-not-remove note saying what depends on it
api/system/debug-tools/D013_storage_persistence.php reports which signal answered, and when one could not be asked
tests/container-detection.php new โ€” 12 assertions

7. How it was verified

The failure only exists when open_basedir is set, which no developer here has, and its symptom is silence rather than an error. So the test drives PHP as a subprocess with the restriction switched on; the condition cannot be created any other way.

1. The reported symptom: no warning is printed any more          PASS
2. Positive control: the warning IS raised without suppression   PASS
3. The environment marker is believed, with no disk access       PASS
4. The marker only ever says YES, never NO                       PASS
5. ๐Ÿ”ด The check still asks about the ROOT of the filesystem       PASS
6. The Dockerfile still sets the marker                          PASS
7. open_basedir is reported honestly, not as "not a container"   PASS

Case 2 is there because case 1 would otherwise pass on a platform where the restriction is not really in force โ€” the test would be agreeing with the bug. Case 5 is there because nothing else in the suite would notice the wrong fix.

Proved load-bearing by breaking it twice

A test that has never failed has not been shown to test anything, so both faults were reintroduced and the suite re-run:

Reintroduced Result
The original bug โ€” @ removed 3 failures, the warning back in the output
The proposed fix โ€” re-pointed at DOCUMENT_ROOT 2 failures, case 5: "RE-POINTED โ€” Docker users would lose the #109 warning"

Then restored, and back to 12 of 12.

Alongside that: the System page, the setup page and D013 driven with a signed-in session and their bodies read rather than their status codes.


8. What this means for you

  • If you have never seen this warning, nothing changes for you.
  • If you did, it is gone, and nothing was ever wrong underneath it. The check was working and its answer was correct.
  • If you run Docker, container detection no longer depends on being allowed to read the root of the filesystem, so the storage warning is harder to lose than it was.
  • If you run Podman, set FREEITSM_CONTAINER=1 in your environment and the storage checks will apply to you as well. They never have before.

9. What is not fixed

FreeITSM still ships with display_errors on. That is why a warning that would have been a log line on most production stacks was painted across an administrator's screen instead โ€” and why the message published the server's directory layout, hosting account and all, into a web page. This fix removes one warning; it does nothing about the next one, anywhere in the product.

Turning it off is a one-line change with a real trade-off โ€” it makes life harder for somebody diagnosing a problem who does not know where their log file lives โ€” so it is a deliberate decision rather than part of a bug fix, and it has not been taken.


10. Related

  • Installation โ€” including the Docker compose setup and its volumes
  • issue #109 โ€” the storage loss this check exists to warn about. There is no wiki page for it yet, and there should be: the warning shipped in #1256, the compose fix is still parked, and the only written explanation of the problem lives in the source comments of includes/storage_persistence.php.
โš ๏ธ **GitHub.com Fallback** โš ๏ธ