fix(api): refuse redirects that turn a write into a GET

Description

When a proxy in front of GitLab answers an http request with a redirect to https, glab release create fails with cannot unmarshal array into Go value of type gitlab.Release, which is the error in #8095 (closed). net/http follows a 301, 302 or 303 answer to a POST, PUT, PATCH or DELETE by sending a GET to the new location without the body, so the create reaches the list releases endpoint instead. An update is worse: the GET reaches the release itself, decodes into the expected type, and glab prints Release updated although nothing was changed.

This MR gives the HTTP client built in internal/api/client.go a CheckRedirect policy, in the new internal/api/redirect.go:

  • A redirect that keeps the method is followed as before: every redirect of a GET or HEAD, and a 307 or 308 of a write, which resends the body.
  • A redirect that would change the method is refused, with an error that names the status and the URL that redirected.
  • When that redirect moves the same host from http to https, on the port the host reaches over https (443 when it names none), the error also says how to set api_protocol to https: glab config set api_protocol https --host <host>, with the port when the host has one, or GLAB_API_PROTOCOL=https. In a CI job like the one in #8095 (closed) that is the GLAB_API_PROTOCOL: https variable. With CI auto-login on (GLAB_ENABLE_CI_AUTOLOGIN=true), glab reads the protocol from CI_SERVER_PROTOCOL alone and neither of those helps, so the error says so and names what does: an external_url with https on the instance, as the triage note says, or CI_SERVER_PROTOCOL set to https in the job.
  • The limit of 10 redirects is kept, because setting CheckRedirect replaces the default policy that enforced it.

Everything that shares this client gets the policy, for example: client-go, the OAuth2 login flows and token refresh, and glab api. GitLab's own redirects are not affected: it answers a write to a moved project with 405 rather than a redirect (lib/api/helpers.rb in gitlab-org/gitlab), and when it forwards a package registry POST it uses 307 (lib/api/helpers/packages/dependency_proxy_helpers.rb), which keeps the method.

Before and after, with the built binary

I built glab from main (8ae0c0ef) and from this branch, and ran glab release create v1.0.0 -R <host>/group/project --notes "new notes" --name v1.0.0 with GLAB_API_PROTOCOL=http against a stand-in GitLab on https, behind a stand-in proxy on the same address that answers every http request with a 301 to the same URL on https.

The release does not exist yet, so glab creates it.

main, exit 1. After the proxy's 301, glab sent GET .../releases in place of the POST:

ERROR
Json: cannot unmarshal array into Go value of type gitlab.Release.

This branch, exit 1, and the POST never reaches GitLab (the error as glab prints it, unwrapped):

Post "https://127.0.0.1:40783/api/v4/projects/group%2Fproject/releases": refusing to follow the 301 redirect from http://127.0.0.1:40783/api/v4/projects/group%2Fproject/releases, which would resend this POST request as a GET without its body; the server redirects http to https, so set api_protocol to https with `glab config set api_protocol https --host 127.0.0.1:40783` or GLAB_API_PROTOCOL=https.

The release exists, so glab updates it.

main, exit 0, and GitLab received GET .../releases/v1%2E0%2E0 in place of the PUT:

✓ Release updated	url=https://example.invalid/r
✓ Release succeeded after 0.00 seconds.

This branch, exit 1:

Put "https://127.0.0.1:40783/api/v4/projects/group%2Fproject/releases/v1%2E0%2E0": refusing to follow the 301 redirect from http://127.0.0.1:40783/api/v4/projects/group%2Fproject/releases/v1%2E0%2E0, which would resend this PUT request as a GET without its body; the server redirects http to https, so set api_protocol to https with `glab config set api_protocol https --host 127.0.0.1:40783` or GLAB_API_PROTOCOL=https.

Under CI auto-login (GLAB_ENABLE_CI_AUTOLOGIN=true, GITLAB_CI=true and CI_SERVER_PROTOCOL=http), this branch, exit 1, for the create:

Post "https://127.0.0.1:40783/api/v4/projects/group%2Fproject/releases": refusing to follow the 301 redirect from http://127.0.0.1:40783/api/v4/projects/group%2Fproject/releases, which would resend this POST request as a GET without its body; the server redirects http to https, and CI auto-login reads api_protocol from CI_SERVER_PROTOCOL, so give the instance an https external_url, or set CI_SERVER_PROTOCOL to https in the job.

In separate runs, these created the release: the glab config set command from the error, with api_protocol set to http for the host before it; GLAB_API_PROTOCOL=https with that same config; and under CI auto-login, CI_SERVER_PROTOCOL=https. Under CI auto-login, GLAB_API_PROTOCOL=https with api_protocol set to https for the host still got the refusal above.

Resolves #8095 (closed)

The same failure was reported for glab mr create in #7728 (closed), closed for inactivity, and in gitlab-org/api/client-go#2094. glab mr create goes through the same client, so it gets the refusal too; client-go itself does not change here.

How has this been tested?

internal/api/redirect_test.go drives the client NewClient builds against a real httptest server:

  • A create answered with 301 and an update answered with 302 are refused, and nothing reaches the redirect target. Both fail on main: the create with the error from #8095 (closed), the update with no error at all.
  • A GET answered with 301 is followed, and a create answered with 308 is followed with its body intact.
  • A redirect loop stops after 10 redirects.
  • The api_protocol hint appears for http to https on the same host and port, and not for another host, another port or a redirect that keeps the scheme; for a host without a port it appears whether the redirect leaves the port out or writes :443, the form Omnibus GitLab sends. It names the glab config set command for the host and GLAB_API_PROTOCOL, and under CI auto-login CI_SERVER_PROTOCOL and the instance's external_url instead; the two hint texts and the other-port case failed before the second commit. This test fails on main too.

Checks I ran with Go 1.27.3 on the second commit:

  • make test: 5520 tests, all passing apart from the 17 the suite skips
  • go test -race on internal/api/... and internal/config/...
  • golangci-lint run with 2.14.0: no findings in the changed files
  • go run ./scripts/comment-overlap -threshold 1.0 -max-findings 0 ./cmd/... ./internal/...: 0 findings
  • make build, make check-embed and make check-args
  • the login line GitLab's release job runs (glab auth login --job-token ... --api-protocol $CI_SERVER_PROTOCOL) with http, against the stand-in above: glab api version then used http, with GLAB_API_PROTOCOL=https it used https, and with GLAB_API_PROTOCOL=https under CI auto-login it still used http
  • commitlint with scripts/commit-lint's rules on both commit messages and on this title
Edited by José M. Requena Plens

Merge request reports

Loading
Loading