Skip to content

hook: Fix allowed errors not propagating correctly anymore

Patrick Steinhardt requested to merge pks-hook-prereceive-errors into master

In 773668f5 (hook: Fix prereceive returning AllowedError for generic errors, 2021-08-13), we have refactored our allowed checks to return a generic error in case the call to Allowed() itself failed. The assumption here was that because the call returns an allowed boolean, it would not raise an error if the call wasn't allowed.

As it turns out, this assumption is wrong: the GitlabNetClient in gitlab-shell will always return an error if the HTTP status code is not between 200 and 399, and access checks do return an error code in case access was denied. As a result, the error we returned to the Gitaly client wouldn't have its "GitLab: " prefix anymore and thus GitLab wouldn't recognize this error as an error that shall be returned to the user.

Ideally, we'd properly fix this by inspecting errors returned by the GitLabNetClient. But this isn't possible without parsing the error messages given that the client will only return generic errors. So this commit just reverts the state to what we had before, where we simply treat all errors returned by the client as user-facing errors. This logic is broken and will reveal messages to the user which aren't intended for him in the first place, but we cannot help this for now.

Changelog: fixed

Fixes gitlab#339727 (closed)

Fixes gitlab#339728 (closed)

Edited by Patrick Steinhardt

Merge request reports