dolthub / dolt

Dolt – Git for Data
Apache License 2.0
17.88k stars 507 forks source link

`secure_file_priv` warning is confusing #7210

Open max-hoffman opened 10 months ago

max-hoffman commented 10 months ago

A default dolt sql-server includes the following warning statement on startup:

WARN[0000] secure_file_priv is set to "", which is insecure. 
WARN[0000] Any user with GRANT FILE privileges will be able to read any file which the sql-server process can read. 
WARN[0000] Please consider restarting the server with secure_file_priv set to a safe (or non-existent) directory. 

This means that LOAD DATA INFILE commands can access arbitrary files on disk. For example, any user of the database could read SSH key or other sensitive files on the server from the MySQL client.

The two ways to limit this risk are to 1) set the secure_file_priv to a folder limiting the file-system visibility, or 2) limit LOAD DATA privileges with GRANT statements.

Adding this line to the server config.yaml limits the LOAD DATA visibility to the working directory on startup, silencing the warning message:

system_variables:
  secure_file_priv: .

There are also file-related GRANTS that have a similar effect at the user-level.

The current default for the LOAD DATA behavior simplifies importing data for first time users. The opposite behavior, requiring users to set the variable to LOAD DATA, would disable LOAD DATA by default and warn users how to enable. We may change the default in the future.

There are a lot of MySQL configuration options. Setting the system variables that MySQL uses is usually how we are going to support this sort of thing.

TheGrum commented 1 month ago

I was caught by this as well - the message gives no hint as to how the 'secure_file_priv' should be set.

The comment above suggests adding to config.yaml, a simple search for MySQL and secure_file_priv indicates a my.cnf needing to be changed.

https://docs.dolthub.com/cli-reference/cli#dolt-sql-server claims that it shows a sample config file with "all supported items and their default values", and secure_file_priv is not listed, nor is secure_file_priv mentioned in the entire dolt sql-server -help. Oddly, searching the Dolt documentation website for secure_file_priv returns two results, and yet neither result has that text in the page anywhere. Possibly it was present previously when the search index was built, and subsequently removed.

Additionally, the documentation at https://docs.dolthub.com/introduction/getting-started/database says to start the server use dolt sql-server, and makes no mention that doing this is going to give you an error message.

Recommendation:

You already have a sample config.yaml in the help. Add a sub-command to output this by itself (something like https://github.com/vharitonsky/iniflags's -dumpflags), and change the directions in the https://docs.dolthub.com/introduction/getting-started/database page to add one line to create a config.yaml, and modify the call to use it: dolt sql-server -dumpconfig > config.yaml dolt sql-server -config=config.yaml

and include the lines from @max-hoffman above in the output (you should include this regardless of whether you add a dumpconfig or not - if the docs say it shows all supported items, it ought to do so.)

You might say that someone could just copy/paste from the help - while this is true, dolt actually chokes on the resulting file

Alternate Recommendation:

If you add support for setting such values via environment variables, you could use a line like: SECURE_FILE_PRIV=. dolt sql-server