Failure if invalid `.env` is present

Expected Behavior

Better handling or warning in log of potential problem.

Could validate an .env file has required vars before starting the service, or just checking for a zero sized file should at least avoid this scenario.

Current Behavior

If .env is an empty (0 bytes) file, the current behaviour is to ignore updating the .env:

https://github.com/linuxserver/docker-bookstack/blob/ed91128c6746ec77ef3a6402de745559144b80e5/root/etc/cont-init.d/50-config#L7-L9

This prevents all .env update logic via sed replace operations. Which in turn causes Bookstack to fail to start:

Illuminate\Database\QueryException  : SQLSTATE[HY000] [1045] Access denied for user 'forge'@'bookstack.docker_default' (using password: NO) (SQL: select * from information_schema.tables where table_schema = bookstackapp and table_name = migrations and table_type = 'BASE TABLE')

Exception trace:

1   Doctrine\DBAL\Driver\PDOException::("SQLSTATE[HY000] [1045] Access denied for user 'forge'@'bookstack.docker_default' (using password: NO)")
     /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOConnection.php:31

2   PDOException::("SQLSTATE[HY000] [1045] Access denied for user 'forge'@'bookstack.docker_default' (using password: NO)")
     /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOConnection.php:27

This scenario shouldn't happen, but affected a production setup that had been running fine for about 2 years when the host system ran out of disk space for the volume the container mounted to persist data. The container restarted itself at some point, and some condition to write or write to .env on disk (perhaps due to disk write error) resulted in the file becoming empty/blank.

It was unclear initially why the service was failing to restore after resolving the disk space issue with the given failure message above, especially since the docker-compose.yml ENV (separate from internal container managed .env file) was valid which the error output implied was not being used.

Steps to Reproduce

Use a default setup.
Override the created .env file to be empty.
Start the service again and check logs for failure like shown above.

Environment

OS: Linux CPU architecture: x86_64 How docker service was installed: Official distro repo

Command used to create docker container (run/create/compose/screenshot)

/docker/docker-compose.yml example (_generates default network based on directory name, thus docker_default like above example error snippet_):

bookstack:
    image: linuxserver/bookstack
    container_name: bookstack
    environment:
      - PUID=1000
      - PGID=1000
      - DB_HOST=bookstack_db
      - DB_USER=bookstack
      - DB_PASS=hunter2
      - DB_DATABASE=bookstackapp
      - APP_URL=https://wiki.example.com
    volumes:
      - /path/to/data/app:/config
    restart: unless-stopped
    depends_on:
      - bookstack_db

  bookstack_db:
    image: linuxserver/mariadb
    container_name: bookstack_db
    environment:
      - PUID=1000
      - PGID=1000
      - MYSQL_ROOT_PASSWORD=hunter2
      - TZ=ETC/UTC
      - MYSQL_DATABASE=bookstackapp
      - MYSQL_USER=bookstack
      - MYSQL_PASSWORD=hunter2
    volumes:
      - /path/to/data/db:/config
    restart: unless-stopped

Docker logs

$ docker-compose logs bookstack

Attaching to bookstack
bookstack            | [s6-init] making user provided files available at /var/run/s6/etc...exited 0.
bookstack            | [s6-init] ensuring user provided files have correct perms...exited 0.
bookstack            | [fix-attrs.d] applying ownership & permissions fixes...
bookstack            | [fix-attrs.d] done.
bookstack            | [cont-init.d] executing container initialization scripts...
bookstack            | [cont-init.d] 01-envfile: executing... 
bookstack            | [cont-init.d] 01-envfile: exited 0.
bookstack            | [cont-init.d] 10-adduser: executing... 
bookstack            | 
bookstack            | -------------------------------------
bookstack            |           _         ()
bookstack            |          | |  ___   _    __
bookstack            |          | | / __| | |  /  \ 
bookstack            |          | | \__ \ | | | () |
bookstack            |          |_| |___/ |_|  \__/
bookstack            | 
bookstack            | 
bookstack            | Brought to you by linuxserver.io
bookstack            | -------------------------------------
bookstack            | 
bookstack            | To support LSIO projects visit:
bookstack            | https://www.linuxserver.io/donate/
bookstack            | -------------------------------------
bookstack            | GID/UID
bookstack            | -------------------------------------
bookstack            | 
bookstack            | User uid:    1000
bookstack            | User gid:    1000
bookstack            | -------------------------------------
bookstack            | 
bookstack            | [cont-init.d] 10-adduser: exited 0.
bookstack            | [cont-init.d] 20-config: executing... 
bookstack            | [cont-init.d] 20-config: exited 0.
bookstack            | [cont-init.d] 30-keygen: executing... 
bookstack            | using keys found in /config/keys
bookstack            | [cont-init.d] 30-keygen: exited 0.
bookstack            | [cont-init.d] 50-config: executing... 
bookstack            | App Key found - setting variable for seds
bookstack            | Running config - db_user set
bookstack            | /var/run/s6/etc/cont-init.d/50-config: line 77: warning: command substitution: ignored null byte in input
bookstack            | /var/run/s6/etc/cont-init.d/50-config: line 77: warning: command substitution: ignored null byte in input
bookstack            | 
bookstack            |    Illuminate\Database\QueryException  : SQLSTATE[HY000] [1045] Access denied for user 'forge'@'bookstack.docker_default' (using password: NO) (SQL: select * from information_schema.tables where table_schema = bookstackapp and table_name = migrations and table_type = 'BASE TABLE')
bookstack            | 
bookstack            |   at /var/www/html/vendor/laravel/framework/src/Illuminate/Database/Connection.php:669
bookstack            |     665|         // If an exception occurs when attempting to run a query, we'll format the error
bookstack            |     666|         // message to include the bindings with SQL, which will make this exception a
bookstack            |     667|         // lot more helpful to the developer instead of just the database's errors.
bookstack            |     668|         catch (Exception $e) {
bookstack            |   > 669|             throw new QueryException(
bookstack            |     670|                 $query, $this->prepareBindings($bindings), $e
bookstack            |     671|             );
bookstack            |     672|         }
bookstack            |     673|
bookstack            | 
bookstack            |   Exception trace:
bookstack            | 
bookstack            |   1   Doctrine\DBAL\Driver\PDOException::("SQLSTATE[HY000] [1045] Access denied for user 'forge'@'bookstack.docker_default' (using password: NO)")
bookstack            |       /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOConnection.php:31
bookstack            | 
bookstack            |   2   PDOException::("SQLSTATE[HY000] [1045] Access denied for user 'forge'@'bookstack.docker_default' (using password: NO)")
bookstack            |       /var/www/html/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/PDOConnection.php:27
bookstack            | 
bookstack            |   Please use the argument -v to see more details.
bookstack            | [cont-init.d] 50-config: exited 0.
bookstack            | [cont-init.d] 99-custom-files: executing... 
bookstack            | [custom-init] no custom files found exiting...
bookstack            | [cont-init.d] 99-custom-files: exited 0.
bookstack            | [cont-init.d] done.
bookstack            | [services.d] starting services
bookstack            | [services.d] done.
bookstack            | Signal handled: Terminated.
bookstack            | [cont-finish.d] executing container finish scripts...
bookstack            | [cont-finish.d] done.
bookstack            | [s6-finish] waiting for services.
bookstack            | s6-svwait: fatal: supervisor died
bookstack            | [s6-finish] sending all processes the TERM signal.
bookstack            | Signal handled: Terminated.
bookstack            | Signal handled: Terminated.
bookstack            | [s6-finish] sending all processes the KILL signal and exiting.
bookstack            | [s6-init] making user provided files available at /var/run/s6/etc...exited 0.
bookstack            | [s6-init] ensuring user provided files have correct perms...exited 0.
bookstack            | [fix-attrs.d] applying ownership & permissions fixes...
bookstack            | [fix-attrs.d] done.
bookstack            | [cont-init.d] executing container initialization scripts...
bookstack            | [cont-init.d] 01-envfile: executing... 
bookstack            | [cont-init.d] 01-envfile: exited 0.
bookstack            | [cont-init.d] 10-adduser: executing... 
bookstack            | usermod: no changes

Thanks for opening your first issue here! Be sure to follow the bug or feature issue templates!

Checking that a file is non-zero in size is easy enough to do but it only partially solves the issue because if the file is non-zero in size but still invalid it's going to have the same problem. Unfortunately the error handling when it comes to the contents of the .env itself is part of Bookstack and not our logic.

Unfortunately the error handling when it comes to the contents of the .env itself is part of Bookstack and not our logic.

Ah ok, shall I forward the issue to Bookstack then? I think it belongs here since the container is creating and modifying .env?

TL;DR:

I guess the better solution is to just re-create the .env file if it's 0 bytes?

I'm not sure of any other situation where .env would be missing the expected vars to replace with sed. This is probably a niche issue to run into, but it is confusing when you had a valid config that's unmodified result in a container failing to work.

Rest of response that isn't too important

> but it only partially solves the issue My main reason for the issue is that the given error in logs wasn't as helpful at identifying the cause. It wasn't clear if it was a bookstack specific issue, something to do with this container or if something else was to blame on our end. ~~Can this image not detect the zero size `.env` as being a problem and exiting before starting up bookstack? Or just output some warning to the logs earlier that helps users track down the cause more easily?~~ The error is a fairly generic laravel one AFAIK, I was not sure where the `forge` user was coming from or why no password was in usage when I hadn't modified my docker-compose ENV which had been working fine through updates of the container for several years. I suspected that something may have changed with one of the images. When looking into this repo there are multiple reports that are misleading about ENV var names being incorrect (but misconfiguration related). One did point out the internal `.env` file, which I then looked up and noticed was empty, then looked up the source here and resolved the problem. --- A helpful error/warning would have sped up the process and not required additional time spent investigating the cause elsewhere. Actually.. I guess the better solution is to just re-create the `.env` file if it's 0 bytes. I'm not sure of any other situation where `.env` would be missing the expected vars to replace with `sed`. --- > if the file is non-zero in size but still invalid it's going to have the same problem. I am not as familiar with what would be considered invalid here. If certain ENV vars are expected to be present in `.env`, shouldn't there be a check that the values are set? I assume you meant when a user does provide the values and they are applied to `.env` (or they're bypassing it with bookstack env vars) but have made a typo or similar resulting in invalid config? That's not something you can do much about. This was a working and valid config. Our environment lacked proper alerts for storage space, so I understand we're at fault. I'm just raising awareness of this scenario as it was unexpected (all other container services recovered upon restart, once disk space was available). Hopefully it saves someone else time :smile:

By invalid I mean any other situation where the .env file exists but the data inside is not valid for the purposes of Bookstack being able to parse it; file gets corrupted, user pastes incorrectly-escaped data in or accidentally overwrites it with another file, etc. Bookstack isn't failing to start because our sed fails, it's failing to start because the .env doesn't have valid data in it.

It's not so much that we can't detect and fix your specific issue it's more that generally speaking we don't like to touch any existing user data if we can avoid it, no matter the state, because we can't be sure it's not intentional and we don't want angry users demanding to know why we overwrote their data. If people want to do weird, unsupported stuff with their containers, that's up to them.

That said I don't see any reason we couldn't log a warning in the event that we detect a 0-byte .env on startup, so there's at least an indication of a problem.

I understand and roughly agree with you :+1:

Issue summary:

My setup had a valid config prior, no changes by me.
The container attempted to write to the .env again on restart prior to my knowledge of a lack of disk space, thus broke itself due to I/O error.
It would be ideal if the container detected the issue and could recover from it.
If that is unreasonable a warning will assist anyone else to identify the real issue sooner.

A warning would be good, but I can't think of any valid reason not to treat it the same as a missing .env file. I debated your response in the collapsed section below if that's of any interest.

If you think it would be worthwhile to implement, awesome! If not, well anyone else who runs into the same issue will probably find this issue eventually :sweat_smile:

Original response (roughly, before revising it above)

> Bookstack isn't failing to start because our sed fails, it's failing to start because the .env doesn't have valid data in it. Yes, I understand that. But the `sed` operations won't work because the vars that are expected to be present in `.env` aren't there to update... which shouldn't happen as the image is intended to manipulate this file internally. If a user is mounting it and modifying it themselves and that isn't supported, that's on them? AFAIK, the breakage scenario I experienced was a bug that failed to overwrite the file due to I/O error (not enough space, IIRC file operations writes a new file then replaces the inode reference). It's not something I expect this project to prevent happening, but it is something it could repair when there is no valid reason to have a zero byte `.env` from a previous run that failed. The present behaviour is to copy over a template `.env` if one is missing, then run the `sed` logic to update internal vars on that file from ENV given to the container. I don't see any issue in updating the script to handle that failure scenario. --- > It's not so much that we can't detect and fix your specific issue it's more that generally speaking we don't like to touch any existing user data if we can avoid it, **no matter the state, because we can't be sure it's not intentional** and we don't want angry users demanding to know why we overwrote their data. Sorry? What data are you going to overwrite if the file is 0 bytes? Data loss already occured from a modification from the container on that file due to an I/O error. The container can detect this and ideally recover from it, there is nothing to lose? If someone is intentionally relying on a zero sized `.env` for some reason that is on them, they can just add filler content and continue with their unsupported workaround. Your script modifies `.env` each run anyway doesn't if detected ENV is present? It just has no effect if the var is missing in `.env` because the `sed` operation has little reason to think the var would be missing: https://github.com/linuxserver/docker-bookstack/blob/ed91128c6746ec77ef3a6402de745559144b80e5/root/etc/cont-init.d/50-config#L50-L58 If you're concerned about other cases, you don't have to do any writes. But the image could output a warning like: "${ENV_VAR} set, but unable to update internal `.env` file: ${INTERNAL_VAR} was expected but is missing.". That should only help troubleshoot issues here better when a user follows the issue template correctly and provides a log? A warning for missing vars in `.env` that you attempt to run `sed` on is another opportunity, but if there isn't sufficient user error reports to justify the time supporting that, I understand.

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

linuxserver / docker-bookstack