From 272558acc56db7ab8f4396c47aeaf47aea1d25f9 Mon Sep 17 00:00:00 2001 From: Paymahn Moghadasian Date: Fri, 11 Sep 2026 08:33:19 -0500 Subject: [PATCH] fix: send Slack tokens as Authorization bearer headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit slack-go's postForm puts the token in an x-www-form-urlencoded `token` field and sends no Authorization header. Slack accepts either, but a body-carried credential is invisible to anything that inspects headers: Sinatra's egress proxy swaps an opaque `sin_` sentinel for the real bot token on the way out, saw no header to rewrite, passed the sentinel through verbatim, and every sandbox call came back `invalid_auth`. Promote the `token` form field to `Authorization: Bearer` in the shared HTTP client, so every Web API call authenticates the way brokers, MITM proxies, and audit tooling expect. Content-Length and GetBody are kept honest so slack-go's retries replay the rewritten body. Browser-session tokens (`xoxc-`, paired with the `d` cookie) stay in the body — they are not bearer credentials and the edge API wants them there. --- pkg/transport/auth_header.go | 82 ++++++++++++++++++++ pkg/transport/auth_header_test.go | 125 ++++++++++++++++++++++++++++++ pkg/transport/transport.go | 1 + 3 files changed, 208 insertions(+) create mode 100644 pkg/transport/auth_header.go create mode 100644 pkg/transport/auth_header_test.go diff --git a/pkg/transport/auth_header.go b/pkg/transport/auth_header.go new file mode 100644 index 0000000..359c257 --- /dev/null +++ b/pkg/transport/auth_header.go @@ -0,0 +1,82 @@ +package transport + +import ( + "bytes" + "io" + "net/http" + "net/url" + "strings" +) + +// AuthHeaderTransport moves the Slack `token` form field into an +// `Authorization: Bearer` header. +// +// slack-go's postForm sends every Web API call as x-www-form-urlencoded with +// the token in a `token` field and no Authorization header. Slack accepts +// either, but body-carried credentials are invisible to the credential +// brokers, MITM proxies, and audit tooling that sit in front of this CLI — +// they only inspect headers, so a body token is passed through verbatim and +// Slack answers `invalid_auth`. +// +// Browser-session credentials (`xoxc-`, paired with the `d` cookie) stay in +// the body: they are not bearer tokens and the edge API expects them there. +type AuthHeaderTransport struct { + roundTripper http.RoundTripper +} + +// NewAuthHeaderTransport wraps rt so form-carried tokens become bearer headers. +func NewAuthHeaderTransport(rt http.RoundTripper) *AuthHeaderTransport { + return &AuthHeaderTransport{roundTripper: rt} +} + +// RoundTrip implements the http.RoundTripper interface. +func (t *AuthHeaderTransport) RoundTrip(req *http.Request) (*http.Response, error) { + rewritten, err := bearerizeFormToken(req) + if err != nil { + return nil, err + } + return t.roundTripper.RoundTrip(rewritten) +} + +// bearerizeFormToken returns req with its `token` form field promoted to an +// Authorization header, or req untouched when there is nothing to promote. +func bearerizeFormToken(req *http.Request) (*http.Request, error) { + if req.Body == nil || req.Header.Get("Authorization") != "" { + return req, nil + } + if !strings.HasPrefix(req.Header.Get("Content-Type"), "application/x-www-form-urlencoded") { + return req, nil + } + + body, err := io.ReadAll(req.Body) + req.Body.Close() + if err != nil { + return nil, err + } + // Any bail-out below must hand the consumed body back to the caller. + restore := func() *http.Request { + req.Body = io.NopCloser(bytes.NewReader(body)) + return req + } + + values, err := url.ParseQuery(string(body)) + if err != nil { + return restore(), nil + } + token := values.Get("token") + if token == "" || strings.HasPrefix(token, "xoxc-") || strings.HasPrefix(token, "xoxd-") { + return restore(), nil + } + + values.Del("token") + encoded := values.Encode() + + out := req.Clone(req.Context()) + out.Header.Set("Authorization", "Bearer "+token) + out.Body = io.NopCloser(strings.NewReader(encoded)) + out.ContentLength = int64(len(encoded)) + out.GetBody = func() (io.ReadCloser, error) { + return io.NopCloser(strings.NewReader(encoded)), nil + } + return out, nil +} diff --git a/pkg/transport/auth_header_test.go b/pkg/transport/auth_header_test.go new file mode 100644 index 0000000..49090c9 --- /dev/null +++ b/pkg/transport/auth_header_test.go @@ -0,0 +1,125 @@ +package transport + +import ( + "io" + "net/http" + "net/url" + "strings" + "testing" +) + +type captureRoundTripper struct{ req *http.Request } + +func (c *captureRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) { + c.req = req + return &http.Response{StatusCode: 200, Body: http.NoBody, Request: req}, nil +} + +func formRequest(t *testing.T, body string) *http.Request { + t.Helper() + req, err := http.NewRequest(http.MethodPost, "https://slack.com/api/auth.test", strings.NewReader(body)) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + return req +} + +func roundTrip(t *testing.T, req *http.Request) *http.Request { + t.Helper() + capture := &captureRoundTripper{} + if _, err := NewAuthHeaderTransport(capture).RoundTrip(req); err != nil { + t.Fatal(err) + } + return capture.req +} + +func sentBody(t *testing.T, req *http.Request) url.Values { + t.Helper() + raw, err := io.ReadAll(req.Body) + if err != nil { + t.Fatal(err) + } + values, err := url.ParseQuery(string(raw)) + if err != nil { + t.Fatal(err) + } + return values +} + +func TestUnitFormTokenBecomesBearerHeader(t *testing.T) { + sent := roundTrip(t, formRequest(t, "token=xoxb-real&channel=C1")) + + if got := sent.Header.Get("Authorization"); got != "Bearer xoxb-real" { + t.Fatalf("Authorization = %q", got) + } + body := sentBody(t, sent) + if body.Has("token") { + t.Fatalf("token left in body: %v", body) + } + if body.Get("channel") != "C1" { + t.Fatalf("other fields lost: %v", body) + } + // A wrong Content-Length makes Slack hang up or truncate the form. + if want := int64(len(body.Encode())); sent.ContentLength != want { + t.Fatalf("ContentLength = %d, want %d", sent.ContentLength, want) + } + // slack-go's retries replay the body. + replay, err := sent.GetBody() + if err != nil { + t.Fatal(err) + } + raw, _ := io.ReadAll(replay) + if strings.Contains(string(raw), "token=") { + t.Fatalf("replayed body still carries the token: %q", raw) + } +} + +func TestUnitProxySentinelIsPromoted(t *testing.T) { + // The whole point: an opaque broker sentinel must reach the header path. + sent := roundTrip(t, formRequest(t, "token=sin_abc123&channel=C1")) + if got := sent.Header.Get("Authorization"); got != "Bearer sin_abc123" { + t.Fatalf("Authorization = %q", got) + } +} + +func TestUnitBrowserTokenStaysInBody(t *testing.T) { + sent := roundTrip(t, formRequest(t, "token=xoxc-browser&channel=C1")) + + if got := sent.Header.Get("Authorization"); got != "" { + t.Fatalf("Authorization = %q, want empty", got) + } + if got := sentBody(t, sent).Get("token"); got != "xoxc-browser" { + t.Fatalf("token = %q, want xoxc-browser", got) + } +} + +func TestUnitNonFormRequestBodyIsPreserved(t *testing.T) { + req, err := http.NewRequest(http.MethodPost, "https://slack.com/api/files.upload", strings.NewReader("token=xoxb-real")) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Content-Type", "multipart/form-data; boundary=x") + + sent := roundTrip(t, req) + if got := sent.Header.Get("Authorization"); got != "" { + t.Fatalf("Authorization = %q, want empty", got) + } + raw, _ := io.ReadAll(sent.Body) + if string(raw) != "token=xoxb-real" { + t.Fatalf("body = %q, want untouched", raw) + } +} + +func TestUnitExistingAuthorizationHeaderWins(t *testing.T) { + req := formRequest(t, "token=xoxb-real") + req.Header.Set("Authorization", "Bearer preset") + + sent := roundTrip(t, req) + if got := sent.Header.Get("Authorization"); got != "Bearer preset" { + t.Fatalf("Authorization = %q", got) + } + if got := sentBody(t, sent).Get("token"); got != "xoxb-real" { + t.Fatalf("body token = %q, want untouched", got) + } +} diff --git a/pkg/transport/transport.go b/pkg/transport/transport.go index a30e2d2..6ef5c8e 100644 --- a/pkg/transport/transport.go +++ b/pkg/transport/transport.go @@ -441,6 +441,7 @@ func ProvideHTTPClient(cookies []*http.Cookie, logger *zap.Logger) *http.Client } transport = NewUserAgentTransport(transport, userAgent, cookies, logger) + transport = NewAuthHeaderTransport(transport) client := &http.Client{ Transport: transport,