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

The "install" or "ini save" writes garbage options to app.ini #25377

Closed
wxiaoguang opened this issue Jun 20, 2023 · 4 comments · Fixed by #25395
Closed

The "install" or "ini save" writes garbage options to app.ini #25377

wxiaoguang opened this issue Jun 20, 2023 · 4 comments · Fixed by #25395
Labels
Milestone

Comments

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Jun 20, 2023

Description

Users should a have a clear and simple app.ini by default.

APP_NAME=Gitea: Git with a cup of tea
RUN_USER=xiaoguang
RUN_MODE=prod

[database]
DB_TYPE=mysql
HOST=127.0.0.1:3306
NAME=gitea
USER=gitea
PASSWD=gitea
SCHEMA=
SSL_MODE=disable
CHARSET=utf8
PATH=/Users/xiaoguang/work/gitea/data/gitea.db
LOG_SQL=false

[repository]
ROOT=/Users/xiaoguang/work/gitea/data/gitea-repositories

[server]
SSH_DOMAIN=localhost
DOMAIN=localhost
HTTP_PORT=3000
ROOT_URL=http://localhost:3000/
APP_DATA_PATH=/Users/xiaoguang/work/gitea/data
DISABLE_SSH=false
SSH_PORT=22
LFS_START_SERVER=true
LFS_JWT_SECRET=............
OFFLINE_MODE=false
HTTP_ADDR=0.0.0.0
PROTOCOL=
USE_PROXY_PROTOCOL=false
PROXY_PROTOCOL_TLS_BRIDGING=false
PROXY_PROTOCOL_HEADER_TIMEOUT=5s
PROXY_PROTOCOL_ACCEPT_UNKNOWN=false
ALLOW_GRACEFUL_RESTARTS=true
GRACEFUL_HAMMER_TIME=1m0s
STARTUP_TIMEOUT=0s
PER_WRITE_TIMEOUT=30s
PER_WRITE_PER_KB_TIMEOUT=10s
STATIC_URL_PREFIX=
LOCAL_ROOT_URL=http://localhost:3000/
LOCAL_USE_PROXY_PROTOCOL=false
REDIRECT_OTHER_PORT=false
PORT_TO_REDIRECT=80
REDIRECTOR_USE_PROXY_PROTOCOL=false
STATIC_ROOT_PATH=/Users/xiaoguang/work/gitea
STATIC_CACHE_TIME=6h0m0s
ENABLE_GZIP=
ENABLE_PPROF=false
PPROF_DATA_PATH=/Users/xiaoguang/work/gitea/data/tmp/pprof
LANDING_PAGE=home
SSH_SERVER_CIPHERS=
SSH_SERVER_KEY_EXCHANGES=
SSH_SERVER_MACS=
SSH_KEYGEN_PATH=
SSH_LISTEN_PORT=22
SSH_SERVER_USE_PROXY_PROTOCOL=false
SSH_TRUSTED_USER_CA_KEYS_FILENAME=/Users/xiaoguang/.ssh/gitea-trusted-user-ca-keys.pem
SSH_AUTHORIZED_PRINCIPALS_ALLOW=off
MINIMUM_KEY_SIZE_CHECK=true
SSH_AUTHORIZED_KEYS_BACKUP=true
SSH_CREATE_AUTHORIZED_KEYS_FILE=true
SSH_EXPOSE_ANONYMOUS=false
SSH_AUTHORIZED_KEYS_COMMAND_TEMPLATE={{.AppPath}} --config={{.CustomConf}} serv key-{{.Key.ID}}
SSH_PER_WRITE_TIMEOUT=30s
SSH_PER_WRITE_PER_KB_TIMEOUT=10s
BUILTIN_SSH_SERVER_USER=xiaoguang
SSH_USER=xiaoguang

[lfs]
PATH=/Users/xiaoguang/work/gitea/data/lfs

[mailer]
ENABLED=false

[service]
REGISTER_EMAIL_CONFIRM=false
ENABLE_NOTIFY_MAIL=false
DISABLE_REGISTRATION=false
ALLOW_ONLY_EXTERNAL_REGISTRATION=false
ENABLE_CAPTCHA=false
REQUIRE_SIGNIN_VIEW=false
DEFAULT_KEEP_EMAIL_PRIVATE=false
DEFAULT_ALLOW_CREATE_ORGANIZATION=true
DEFAULT_ENABLE_TIMETRACKING=true
NO_REPLY_ADDRESS=noreply.localhost

[openid]
ENABLE_OPENID_SIGNIN=true
ENABLE_OPENID_SIGNUP=true

[cron.update_checker]
ENABLED=false

[session]
PROVIDER=file

[log]
MODE=console
LEVEL=info
ROOT_PATH=/Users/xiaoguang/work/gitea/log
STACKTRACE_LEVEL=none
BUFFER_LEN=10000
ENABLE_SSH_LOG=false
ACCESS_LOG_TEMPLATE={{.Ctx.RemoteHost}} - {{.Identity}} {{.Start.Format "[02/Jan/2006:15:04:05 -0700]" }} "{{.Ctx.Req.Method}} {{.Ctx.Req.URL.RequestURI}} {{.Ctx.Req.Proto}}" {{.ResponseWriter.Status}} {{.ResponseWriter.Size}} "{{.Ctx.Req.Referer}}" "{{.Ctx.Req.UserAgent}}"
REQUEST_ID_HEADERS=

[repository.pull-request]
DEFAULT_MERGE_STYLE=merge

[repository.signing]
DEFAULT_TRUST_MODEL=committer

[security]
INSTALL_LOCK=true
INTERNAL_TOKEN=.........
PASSWORD_HASH_ALGO=pbkdf2

[ssh.minimum_key_sizes]

[oauth2]
JWT_SECRET=........

@wxiaoguang wxiaoguang added this to the 1.20.0 milestone Jun 20, 2023
lunny pushed a commit that referenced this issue Jun 21, 2023
That's a longstanding INI package problem: the "MustXxx" calls change
the option values, and the following "Save" will save a lot of garbage
options into the user's config file.

Ideally we should refactor the INI package to a clear solution, but it's
a huge work.

A clear workaround is what this PR does: when "Save", load a clear INI
instance and save it.

Partially fix #25377, the "install" page needs more fine tunes.
GiteaBot pushed a commit to GiteaBot/gitea that referenced this issue Jun 21, 2023
That's a longstanding INI package problem: the "MustXxx" calls change
the option values, and the following "Save" will save a lot of garbage
options into the user's config file.

Ideally we should refactor the INI package to a clear solution, but it's
a huge work.

A clear workaround is what this PR does: when "Save", load a clear INI
instance and save it.

Partially fix go-gitea#25377, the "install" page needs more fine tunes.
lunny pushed a commit that referenced this issue Jun 21, 2023
Backport #25395 by @wxiaoguang

That's a longstanding INI package problem: the "MustXxx" calls change
the option values, and the following "Save" will save a lot of garbage
options into the user's config file.

Ideally we should refactor the INI package to a clear solution, but it's
a huge work.

A clear workaround is what this PR does: when "Save", load a clear INI
instance and save it.

Partially fix #25377, the "install" page needs more fine tunes.

Co-authored-by: wxiaoguang <[email protected]>
@cboylan
Copy link
Contributor

cboylan commented Jun 26, 2023

Is this issue addressed? I've starting testing gitea v1.20.0-rc2 (which includes 8302b95) locally and have gotten these errors on startup:

2023/06/26 18:44:53 ...es/setting/oauth2.go:140:loadOAuth2From() [F] save oauth2.JWT_SECRET failed: failed to save "/custom/conf/app.ini": open /custom/conf/app.ini: permission denied

In our config we set oauth2.ENABLE = false. I think there are two problems here. The first is that gitea shouldn't attempt to modify the oauth2 configuration if not enabled. Second, ideally gitea would never write config back out to disk. As a user using configuration management and container images I don't want our config to by modified outside of these processes.

@wxiaoguang
Copy link
Contributor Author

I do not think "/custom/conf/app.ini" is a right path for config file.

How do you run Gitea? Can you provide reproducible steps?

@cboylan
Copy link
Contributor

cboylan commented Jun 27, 2023

I do not think "/custom/conf/app.ini" is a right path for config file.

Paths are arbitrary and as far as I know Gitea doesn't actually care what the paths are. In our case we build our docker image with this Dockerfile: https://opendev.org/opendev/system-config/src/branch/master/docker/gitea/Dockerfile (modified to update the git tag checkout to v1.20.0-rc2 in this case) and deploy them with this docker-compose file: https://opendev.org/opendev/system-config/src/branch/master/playbooks/roles/gitea/templates/docker-compose.yaml.j2

How do you run Gitea? Can you provide reproducible steps?

I believe that there are two things necessary to reproduce this problem. The first is the app.ini file must have permissions set such that the gitea user cannot write to the file. Second gitea must see some file difference it wants to write back to that file.

To achieve the first item you can chown root:root app.ini and chmod 0644 app.ini then run gitea as a user other than root. The file will be owned by root with permissions only allowing root to modify the file. Then to trigger the write create an oauth2 section with this content:

[oauth2]
ENABLE = false

This seems to force gitea into thinking it needs to add JWT_SECRET to that config and write it back to the file. As mentioned above I think there are two bugs here. The first is that if oauth2 is disabled gitea shouldn't do anything with secrets for oauth2. The second is that gitea should not write back to its config file. If gitea generates new config like this unexpectedly then configuration can be broken and/or mismatch what configuration management believes is in place.

It is possible that this is separate issue and I'm happy to file one for that. I just saw this issue and it looks very similar (but this is happening outside of the gitea runtime and the issue I have is during runtime?)

Edited to fix file permissions (they were 0600 should be 0644).

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Jun 27, 2023

The first is that if oauth2 is disabled gitea shouldn't do anything with secrets for oauth2.

Yes, it is a bug, it doesn't make sense to generate the secret if the oauth2 is disabled.

The second is that gitea should not write back to its config file.

Gitea requires that the "app.ini" should be writable by itself, it's not well documented but many functions depend on this fact.

silverwind pushed a commit that referenced this issue Jun 28, 2023
…g in some sub-commands (#25567)

Ref:

* #25377 (comment)

And some sub-commands like "generate" / "docs", they do not need to use
the ini config
wxiaoguang added a commit to wxiaoguang/gitea that referenced this issue Jun 29, 2023
…g in some sub-commands (go-gitea#25567)

Ref:

* go-gitea#25377 (comment)

And some sub-commands like "generate" / "docs", they do not need to use
the ini config
# Conflicts:
#	modules/setting/oauth2.go
silverwind pushed a commit that referenced this issue Jun 29, 2023
…g in some sub-commands (#25567) (#25576)

Backport #25567

Ref:

* #25377 (comment)

And some sub-commands like "generate" / "docs", they do not need to use
the ini config
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants