Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gitea should offer a configcheck command #2762

Open
stblassitude opened this issue Oct 22, 2017 · 13 comments
Open

Gitea should offer a configcheck command #2762

stblassitude opened this issue Oct 22, 2017 · 13 comments
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@stblassitude
Copy link
Contributor

  • Gitea version (or commit ref): 1.2.0

Using the FreeBSD port for Gitea on FreeBSD 11.1.

Description

After upgrading to Gitea 1.2, Gitea won't start if the config file cannot be written by the user Gitea is running under:

[...s/setting/setting.go:859 NewContext()] [E] Error saving generated JWT Secret to custom config: open /usr/local/etc/gitea/conf/app.ini: permission denied

This is due to the security/INTERNAL_TOKEN option not being set, and Gitea being unable to add it to the config itself.

Start scripts should be able to verify the configuration and print appropriate error messages, before trying to start Gitea as a daemon.

@lunny lunny added the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Oct 22, 2017
@lafriks lafriks added this to the 1.x.x milestone Oct 22, 2017
@bkcsoft
Copy link
Member

bkcsoft commented Nov 1, 2017

In this case, there's nothing wrong with the config file itself, so a verification wouldn't help. The issue is that Gitea can't write to the config-file, hence it fails. With that said, having a gitea admin config command for verification (and maybe other things) would be nice.

@stblassitude
Copy link
Contributor Author

A mandatory config key missing is an error in the config file. The fact that Gitea might be able to fix that automatically doesn't mean its not an error. Otherwise, Gitea would continue to run without the key being set.

@bkcsoft
Copy link
Member

bkcsoft commented Nov 1, 2017

Gitea needs to generate and write this token on each start-up, having the config file in read-only is the error.

@stblassitude
Copy link
Contributor Author

That makes even less sense. Why would it need to be written to the config file, if it is recomputed at every start?

And why do you insist that Gitea must be able to write the config file?

@bkcsoft
Copy link
Member

bkcsoft commented Nov 1, 2017

It needs to be written to a file (the config was available so it ended up there) because gitea serv needs to know about it. I don't remember why it had to be recomputed on every restart, but my guess is #becausesecurity 🙄

As for Gitea needing to write the config, that's because the initial setup in only available in the WebUI (therefore needs to be managed by Gitea itself), and that one needs to write the config to disk afterwards

Edit: It's not perfect, but it's what we have. PRs welcome :trollface:

@lunny
Copy link
Member

lunny commented Nov 2, 2017

Another reason except installation is because Gitea in fact provides serval sub command, gitea web, gitea serve, gitea hook. They are comminuted each other via the same database at past
and should be via internal HTTP in future(partially implementation). This token is the key to keep the communication more secure. Any better idea about it then we can avoid to write data to the config file. Or maybe we could write the token to another standalone file.

@lafriks
Copy link
Member

lafriks commented Nov 2, 2017

@lunny or let's move such data to database setting table

@lunny
Copy link
Member

lunny commented Nov 2, 2017

@lafriks don't want gitea serv / update / hook to visit database directly in future.

@stblassitude
Copy link
Contributor Author

stblassitude commented Nov 2, 2017

Storing this secret in the config file is acceptable, but there should be documentation on what it is used for, and how to create one yourself, if you prefer to keep the config file readonly and managed outside of Gitea.

From reading the source, it appears that the only requirement for a value is a non-empty string; however, a sensibly complex random string is preferreable. In the FreeBSD port, a random number is generated with openssl, producing a similar value to the code in modules/setting/setting.go.

To get back to the original feature request: while it's nice that Gitea can be set up through a web browser, there a re scenarios in which it is preferrable for the config file to be read-only to the Gitea user, and the config file contents being managed by some configuration management tool. It would be great if Gitea would support this scenario with appropriate error messages.

The standard gitea web output is extremely verbose and will likely be logged at debug level or not at all. This is why a seperate command to check the configuration and present appropriate hints on configuration problems is desirable.

Alternatively, the output form gitea web should be reduced to major events only, so that it can be logged as syslog daemon.notice or higher. If this can be configured already, I apologize, but I did not find a log setting that seems to apply to the console output.

@stblassitude
Copy link
Contributor Author

@lunny For gitea serv etc. to communicate with the daemon, you could consider HTTP over unix socket. This way, access is controlled by file system permissions.

A quick google suggest for example: https://gist.github.com/teknoraver/5ffacb8757330715bcbcc90e6d46ac74

@lunny
Copy link
Member

lunny commented Nov 3, 2017

@stblassitude thanks, I will check that.

@lafriks
Copy link
Member

lafriks commented Nov 3, 2017

@lunny @stblassitude gitea does that already if gitea is set to listen on socket using fcgi

@ghost
Copy link

ghost commented Jan 5, 2018

Writing the configuration file automatically, which is meant to be customized by the user, is very bad practise which I'd consider being a bug. In fact, like @stblassitude said, it's very complex to automate configuration deployment in an idempotent manner. If there was a hard requirement to write such information into a file, it might get written into another file, but not in the configuration file, which is meant for the user to customize the runtime of gitea. Furthermore +1 on this feature request. It would be very nice to verify the configuration before restarting a service automatically on a configuration file update.

Zempashi pushed a commit to Zempashi/gitea_aws that referenced this issue May 3, 2018
Zempashi pushed a commit to Zempashi/gitea_aws that referenced this issue May 6, 2018
@lunny lunny removed this from the 1.x.x milestone Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

No branches or pull requests

4 participants