lovasoa / SQLpage

SQL-only webapp builder, empowering data analysts to build websites and applications quickly
https://sql.datapage.app
MIT License
1.29k stars 69 forks source link

web_root directory owned by root in Docker image #94

Closed radarsymphony closed 10 months ago

radarsymphony commented 11 months ago

Hi lovasoa,

This is a cool app! Thanks for making it available.

Bug Description

I get a warning that the current directory is not writeable when I run a stock docker image:

sqlpage-app  | [2023-09-29T20:30:17Z WARN  sqlpage::app_config] No DATABASE_URL provided, and the current directory is not writeable. Using a temporary in-memory SQLite database. All the data created will be lost when this server shuts down.
sqlpage-app  | [2023-09-29T20:30:17Z INFO  sqlpage::webserver::database] Connecting to database: sqlite://:memory:
my docker-compose.yml ``` services: sqlpage-app: image: lovasoa/sqlpage:latest container_name: sqlpage-app labels: - traefik.enable=true - traefik.http.routers.sqlpage-app.tls=true - traefik.http.routers.sqlpage-app.rule=Host(`sqlpage.example.com`) - traefik.http.routers.sqlpage-app.service=sqlpage-app - traefik.http.services.sqlpage-app.loadbalancer.server.port=8080 environment: - TZ=America/Vancouver # doesn't change TZ... #- SQLPAGE_DATABASE_URL=sqlite://sqlpage.db?mode=rwc #- SQLPAGE_WEB_ROOT=/var/html #volumes: #- ./data/src:/var/www networks: - proxy networks: proxy: external: true ```

Expected Behavior

On start up, I think the docker image should create a new SQLite DB at sqlite://sqlpage.db, like this:

sqlpage-app  | [2023-09-29T20:08:06Z INFO  sqlpage::app_config] No DATABASE_URL provided, the current directory is writeable, creating sqlpage.db
sqlpage-app  | [2023-09-29T20:08:06Z INFO  sqlpage::webserver::database] Connecting to database: sqlite://sqlpage.db?mode=rwc

However, in the current latest release, the web_root /var/www is owned by root:root and so the sqlpage user cannot write to that directory.

Potential Solutions

a) Pre-make the sqlite.db and set the ownership, then map the volume to the container b) Modify the Dockerfile to change the ownership on startup (see below) c) Another solution that is more aligned with your design philosophy.

Pull Request / Design options

I can resolve the issue by adding the following line to the Dockerfile and building an image:

RUN chown -R sqlpage:sqlpage /var/www

However, this approach will break when a different web_root is passed since the webroot is hardcoded in the Dockerfile.

Ideally, a startup script would:

What do you think? Is there another way around this? How does this affect the rest of the code base?

I am happy to have a go at a solution/startup script and then we can discuss. Let me know if I can help.

lovasoa commented 11 months ago

Hi Greg, and welcome to SQLPage !

I'm not sure what the best default is, here.

Not giving write access to the web root to the SQLPage process was a deliberate choice, because it should not be required in most cases, and gives a warranty that even an attacker who would gain control of the sqlpage process could not alter the website's files and warrant themselves persistence.

On the other side, I agree that getting a warning about permissions in the default configuration is not ideal.

Maybe the real problem is that the default DATABASE_URL is ./sqlpage.db (which is by default publicly accessible at the server root).

What would you think about the following changes to the defaults ?

radarsymphony commented 11 months ago

Thanks for your quick response.

Yes, of course. The security piece makes sense. (I wish I had considered that. Thanks for catching.)

I agree with changing the default to move the sqlite.db to the sqlpage-owned [web_root]/sqlpage/ directory. If I understand your thought process, a default sqlite.db would be created at /var/www/sqlpage/sqlpage.db if no DATABASE_URL is provided. Is that right?

I don't understand the other two changes yet. Will you provide an example of what root-owned configuration would be placed at /etc/sqlpage to help me understand how this is connected to this issue? Are you thinking of moving /var/www/sqlpage/sqlpage.json to the /etc/ directory?

Thanks for helping me understand this issue.

lovasoa commented 10 months ago

I made the change, the image should be available on docker hub in a few minutes.

In the new official docker image:

And consequently, there is no more warning displayed by default. Just INFO: Connecting to database: sqlite:///etc/sqlpage/sqlpage.db

lovasoa commented 10 months ago

And I gave some love to our docker images: they are now smaller, faster, more secure, and support multiple architectures.

radarsymphony commented 10 months ago

Sweet! I just tested the new image and it is working as expected (no warning on startup, sqlpage.db in /etc/sqlpage, and owned by sqlpage). Thank you so much!