Skip to content

Fix error message sanitiziation #3082

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Dec 4, 2017
Merged

Fix error message sanitiziation #3082

merged 2 commits into from
Dec 4, 2017

Conversation

ethantkoenig
Copy link
Member

Fixes two sanitization issues:

  1. Previous usage of the HandleCloneUserCredentials function did not cover the case where a message contained a sensitive URLs multiple times; only the first usage would be sanitized.
  2. In the MigratePost handler, an unsanitized error could reach the handleCreateError(..) function, which itself does not do any sanitization. As a result, unsanitized URL could be printed in logs.

This PR additionally:
@lafriks lafriks added type/refactoring Existing code has been cleaned up. There should be no new functionality. topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! labels Dec 3, 2017
@lafriks lafriks added this to the 1.4.0 milestone Dec 3, 2017
@codecov-io
Copy link

codecov-io commented Dec 3, 2017 •
edited
Loading

Codecov Report

Merging #3082 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@ Coverage Diff @@ ## master #3082 +/- ## ========================================== - Coverage 33.26% 33.24% -0.02%  ========================================== Files 272 273 +1 Lines 39939 39947 +8 ========================================== - Hits 13284 13281 -3  - Misses 24757 24767 +10  - Partials 1898 1899 +1
Impacted Files Coverage Δ
routers/repo/repo.go 21.81% <0%> (-0.16%) ⬇️
models/repo_mirror.go 3.46% <0%> (+0.25%) ⬆️
modules/util/sanitize.go 0% <0%> (ø)
routers/api/v1/repo/repo.go 32.99% <0%> (-0.09%) ⬇️
modules/process/manager.go 76.81% <0%> (-4.35%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5dc37b1...0c10249. Read the comment docs.
@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 3, 2017
@lafriks
Copy link
Member

lafriks commented Dec 3, 2017

LGTM
@tboerger tboerger added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Dec 3, 2017
@sapk
Copy link
Member

sapk commented Dec 4, 2017

LGTM
@tboerger tboerger added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Dec 4, 2017
@lafriks
Copy link
Member

lafriks commented Dec 4, 2017

make LG-TM work
@lafriks lafriks merged commit 3c1b1ca into go-gitea:master Dec 4, 2017
@lafriks
Copy link
Member

lafriks commented Dec 4, 2017

@ethantkoenig please backport this also to #3078
ethantkoenig added a commit to ethantkoenig/gitea that referenced this pull request Dec 4, 2017
@ethantkoenig
Fix error message sanitiziation (go-gitea#3082)
74f75e7
@ethantkoenig ethantkoenig mentioned this pull request Dec 4, 2017
Merged
@lunny lunny added the backport/done All backports for this PR have been created label Dec 7, 2017
lafriks pushed a commit that referenced this pull request Dec 8, 2017
@ethantkoenig @lafriks
Sanitize logs for mirror sync (#3057, #3082) (#3078)
ec6718e 
* Sanitize logs for mirror sync * Fix error message sanitiziation (#3082)
@ethantkoenig ethantkoenig deleted the improve/mirror branch December 16, 2017 04:47
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
project-mirrors-bot-tu bot pushed a commit to project-mirrors/forgejo-as-gitea-fork that referenced this pull request Jan 23, 2025
@Gusted
[BUG] Fix admin layout
7bbb93b 
- Fix the layout of admin pages, it previously was full-width and had the alert at the incorrect place and within an container. - Make the placement of the alert consistent with other pages, inside `flex-container-main` and not wrapped around a container. - We have to revert 145bebc, as this expected that the page contain provided padding, this was provided by the incorrect placement of the alert. As well isn't consistent with how other pages are being shown, non-full width. The solution to the described problem isn't optimal and should rather be fixed with the tables. - Reverts 145bebc - Resolves go-gitea#3082 (cherry picked from commit c2d0f64)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

backport/done All backports for this PR have been created lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! type/refactoring Existing code has been cleaned up. There should be no new functionality.

7 participants

@ethantkoenig @codecov-io @lafriks @sapk @appleboy @lunny @tboerger