Properly sanitize gitea admin output #590

2023-12-20T22:34:19Z

justusbunsi commented

2023-12-20 22:34:19 +00:00

(Migrated from gitea.com)

Description of the change

With https://github.com/go-gitea/gitea/pull/28390, Gitea 1.21.2 introduced warning log output within the result of gitea admin <subcommand> and therefore affects the current provisioning script.
That script previously assumed a clean result set and was therefore doomed to fail at some point.

This introduces output sanitizing to trim such logs above the actual result table.

Applicable issues

fixes #589

Additional information

The non-sanitized output were only an issue for admin account provisioning, and only when the username matched one of these words (in case of #589 it was gitea):

.../setting/security.go:168:loadSecurityFrom() [W] Enabling Query API Auth tokens is not recommended. DISABLE_QUERY_AUTH_TOKEN will default to true in gitea 1.23 and will be removed in gitea 1.24.

LDAP and OAuth sources were not affected by this particular log line, but also processed non-sanitized result sets. Changing their code is a precaution.

### Description of the change With https://github.com/go-gitea/gitea/pull/28390, Gitea 1.21.2 introduced warning log output within the result of `gitea admin <subcommand>` and therefore affects the current provisioning script. That script previously assumed a clean result set and was therefore doomed to fail at _some_ point. This introduces output sanitizing to trim such logs above the actual result table. ### Applicable issues - fixes #589 ### Additional information The non-sanitized output were only an issue for admin account provisioning, and only when the username matched one of these words (in case of #589 it was `gitea`): ```text .../setting/security.go:168:loadSecurityFrom() [W] Enabling Query API Auth tokens is not recommended. DISABLE_QUERY_AUTH_TOKEN will default to true in gitea 1.23 and will be removed in gitea 1.24. ``` LDAP and OAuth sources were not affected by this particular log line, but also processed non-sanitized result sets. Changing their code is a precaution.

pat-s (Migrated from gitea.com) reviewed 2023-12-20 22:35:25 +00:00

justusbunsi commented

2023-12-20 22:37:53 +00:00

(Migrated from gitea.com)

I am aware that I introduce nearly duplicated code here. But the current configure_gitea.sh doesn't allow for deduplication. And I didn't want to open Pandora's box in this PR.

I am aware that I introduce nearly duplicated code here. But the current `configure_gitea.sh` doesn't allow for deduplication. And I didn't want to open Pandora's box in this PR.

pat-s commented

2023-12-21 07:59:02 +00:00

(Migrated from gitea.com)

Thanks for the quick reaction and fix! This is indeed unfortunate. I wonder how we can prevent similar issues from happening in the future. Also when the warning will go away after 1.24 again - there might be others in the future.

Maybe we could also temporarily change the log level, though I haven't found if this would even apply to this internal call.

Let's get the fix out first!

Thanks for the quick reaction and fix! This is indeed unfortunate. I wonder how we can prevent similar issues from happening in the future. Also when the warning will go away after 1.24 again - there might be others in the future. Maybe we could also temporarily change the log level, though I haven't found if this would even apply to this internal call. Let's get the fix out first!

pat-s (Migrated from gitea.com) approved these changes 2023-12-21 07:59:08 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: lunny/helm-chart#590

Properly sanitize gitea admin output #590

Description of the change

Applicable issues

Additional information

Properly sanitize `gitea admin` output #590