From 916a7cac770f10bf40dd82e3788e9c8a23097019 Mon Sep 17 00:00:00 2001
From: Sword <rrylee1994@gmail.com>
Date: Fri, 22 Dec 2023 15:49:57 +0000
Subject: [PATCH] Add unit-test for SendGitAuditEvent
- /workhorse/internal/api/api.go
- /workhorse/internal/api/api_test.go
- /workhorse/internal/git/git-http.go
- /workhorse/internal/git/receive-pack.go
- /workhorse/internal/git/upload-pack.go
- /workhorse/internal/gitaly/smarthttp.go
---
workhorse/internal/api/api.go | 40 +++++++++++-
workhorse/internal/api/api_test.go | 76 ++++++++++++++++++++++
workhorse/internal/git/git-http.go | 38 +++++++++--
workhorse/internal/git/receive-pack.go | 11 ++--
workhorse/internal/git/upload-pack.go | 15 +++--
workhorse/internal/git/upload-pack_test.go | 2 +-
workhorse/internal/gitaly/smarthttp.go | 13 ++--
7 files changed, 173 insertions(+), 22 deletions(-)
diff --git a/workhorse/internal/api/api.go b/workhorse/internal/api/api.go
index 91bf37fec555..bacd3c27cc1f 100644
--- a/workhorse/internal/api/api.go
+++ b/workhorse/internal/api/api.go
@@ -2,6 +2,7 @@ package api
import (
"bytes"
+ "context"
"encoding/json"
"fmt"
"io"
@@ -13,7 +14,6 @@ import (
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
-
"gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/config"
@@ -174,6 +174,8 @@ type Response struct {
MaximumSize int64
// A list of permitted hash functions. If empty, then all available are permitted.
UploadHashFunctions []string
+ // NeedAudit indicates whether git events should be audited to rails.
+ NeedAudit bool `json:"NeedAudit"`
}
type GitalyServer struct {
@@ -275,6 +277,42 @@ func (api *API) newRequest(r *http.Request, suffix string) (*http.Request, error
return authReq, nil
}
+type GitAuditEventRequest struct {
+ Action string `json:"action"`
+ Protocol string `json:"protocol"`
+ Repo string `json:"gl_repository"`
+ Username string `json:"username"`
+ PackfileStats *gitalypb.PackfileNegotiationStatistics `json:"packfile_stats,omitempty"`
+}
+
+func (api *API) SendGitAuditEvent(ctx context.Context, body GitAuditEventRequest) error {
+ b, err := json.Marshal(body)
+ if err != nil {
+ return fmt.Errorf("failed to marshal GitAuditEventRequest: %w", err)
+ }
+
+ auditURL := *api.URL
+ auditURL.Path = "/api/v4/internal/shellhorse/git_audit_event"
+ auditReq, err := http.NewRequest(http.MethodPost, auditURL.String(), bytes.NewReader(b))
+ if err != nil {
+ return fmt.Errorf("failed to create request: %w", err)
+ }
+ auditReq.Header.Set("User-Agent", "GitLab-Workhorse")
+ auditReq.Header.Set("Content-Type", "application/json")
+ auditReq = auditReq.WithContext(ctx)
+ httpResponse, err := api.doRequestWithoutRedirects(auditReq)
+ if err != nil {
+ return fmt.Errorf("SendGitAuditEvent: do request: %v", err)
+ }
+ defer httpResponse.Body.Close()
+
+ if httpResponse.StatusCode != http.StatusOK {
+ return fmt.Errorf("SendGitAuditEvent: response status: %s", httpResponse.Status)
+ }
+
+ return nil
+}
+
// PreAuthorize performs a pre-authorization check against the API for the given HTTP request
//
// If the returned *http.Response is not nil, the caller is responsible for closing its body
diff --git a/workhorse/internal/api/api_test.go b/workhorse/internal/api/api_test.go
index 71a0b703cea6..8f15e3965168 100644
--- a/workhorse/internal/api/api_test.go
+++ b/workhorse/internal/api/api_test.go
@@ -1,6 +1,8 @@
package api
import (
+ "context"
+ "encoding/json"
"fmt"
"io"
"net/http"
@@ -10,6 +12,7 @@ import (
"testing"
"github.com/stretchr/testify/require"
+ "gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/helper"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/secret"
@@ -130,3 +133,76 @@ func testRailsServer(url *regexp.Regexp, code int, body string) *httptest.Server
fmt.Fprint(w, body)
})
}
+
+func TestPreAuthorizeFixedPath(t *testing.T) {
+ var (
+ upstreamHeaders http.Header
+ upstreamQuery url.Values
+ )
+
+ ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ if r.URL.Path != "/my/api/path" {
+ return
+ }
+
+ upstreamHeaders = r.Header
+ upstreamQuery = r.URL.Query()
+ w.Header().Set("Content-Type", ResponseContentType)
+ io.WriteString(w, `{"TempPath":"HELLO!!"}`)
+ }))
+ defer ts.Close()
+
+ req, err := http.NewRequest("GET", "/original/request/path?q1=Q1&q2=Q2", nil)
+ require.NoError(t, err)
+ req.Header.Set("key1", "value1")
+
+ api := NewAPI(helper.URLMustParse(ts.URL), "123", http.DefaultTransport)
+ resp, err := api.PreAuthorizeFixedPath(req, "POST", "/my/api/path")
+ require.NoError(t, err)
+
+ require.Equal(t, "value1", upstreamHeaders.Get("key1"), "original headers must propagate")
+ require.Equal(t, url.Values{"q1": []string{"Q1"}, "q2": []string{"Q2"}}, upstreamQuery,
+ "original query must propagate")
+ require.Equal(t, "HELLO!!", resp.TempPath, "sanity check: successful API call")
+}
+
+func TestSendGitAuditEvent(t *testing.T) {
+ testhelper.ConfigureSecret()
+
+ var (
+ requestHeaders http.Header
+ requestBody GitAuditEventRequest
+ )
+
+ ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ if r.URL.Path != "/api/v4/internal/shellhorse/git_audit_event" {
+ return
+ }
+
+ requestHeaders = r.Header
+ defer r.Body.Close()
+ b, err := io.ReadAll(r.Body)
+ require.NoError(t, err)
+ err = json.Unmarshal(b, &requestBody)
+ require.NoError(t, err)
+ }))
+ defer ts.Close()
+
+ api := NewAPI(helper.URLMustParse(ts.URL), "123", http.DefaultTransport)
+ auditRequest := GitAuditEventRequest{
+ Action: "git-receive-request",
+ Protocol: "http",
+ Repo: "project-1",
+ Username: "GitLab-Shell",
+ PackfileStats: &gitalypb.PackfileNegotiationStatistics{
+ Wants: 3,
+ Haves: 23,
+ },
+ }
+ err := api.SendGitAuditEvent(context.Background(), auditRequest)
+ require.NoError(t, err)
+
+ require.NotEmpty(t, requestHeaders)
+ require.NotEmpty(t, requestHeaders["Gitlab-Workhorse-Api-Request"])
+ require.Equal(t, auditRequest, requestBody)
+}
diff --git a/workhorse/internal/git/git-http.go b/workhorse/internal/git/git-http.go
index 86007e160640..db318b2ce5cb 100644
--- a/workhorse/internal/git/git-http.go
+++ b/workhorse/internal/git/git-http.go
@@ -11,6 +11,8 @@ import (
"path/filepath"
"sync"
+ "gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
+
"gitlab.com/gitlab-org/gitlab/workhorse/internal/api"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/log"
)
@@ -22,11 +24,11 @@ const (
)
func ReceivePack(a *api.API) http.Handler {
- return postRPCHandler(a, "handleReceivePack", handleReceivePack, writeReceivePackError)
+ return postRPCHandler(a, "handleReceivePack", handleReceivePack, sendGitAuditEvent("git-receive-pack"), writeReceivePackError)
}
func UploadPack(a *api.API) http.Handler {
- return postRPCHandler(a, "handleUploadPack", handleUploadPack, writeUploadPackError)
+ return postRPCHandler(a, "handleUploadPack", handleUploadPack, sendGitAuditEvent("git-upload-pack"), writeUploadPackError)
}
func gitConfigOptions(a *api.Response) []string {
@@ -42,7 +44,8 @@ func gitConfigOptions(a *api.Response) []string {
func postRPCHandler(
a *api.API,
name string,
- handler func(*HttpResponseWriter, *http.Request, *api.Response) error,
+ handler func(*HttpResponseWriter, *http.Request, *api.Response) (*gitalypb.PackfileNegotiationStatistics, error),
+ postFunc func(*api.API, *http.Request, *api.Response, *gitalypb.PackfileNegotiationStatistics),
errWriter func(io.Writer) error,
) http.Handler {
return repoPreAuthorizeHandler(a, func(rw http.ResponseWriter, r *http.Request, ar *api.Response) {
@@ -54,7 +57,8 @@ func postRPCHandler(
w.Log(r, cr.Count())
}()
- if err := handler(w, r, ar); err != nil {
+ stats, err := handler(w, r, ar)
+ if err != nil {
handleLimitErr(err, w, errWriter)
// If the handler, or handleLimitErr already wrote a response this WriteHeader call is a
// no-op. It never reaches net/http because GitHttpResponseWriter calls
@@ -62,6 +66,8 @@ func postRPCHandler(
w.WriteHeader(500)
log.WithRequest(r).WithError(fmt.Errorf("%s: %v", name, err)).Error()
}
+
+ postFunc(a, r, ar, stats)
})
}
@@ -71,6 +77,30 @@ func repoPreAuthorizeHandler(myAPI *api.API, handleFunc api.HandleFunc) http.Han
}, "")
}
+func sendGitAuditEvent(action string) func(*api.API, *http.Request, *api.Response, *gitalypb.PackfileNegotiationStatistics) {
+ return func(a *api.API, r *http.Request, response *api.Response, stats *gitalypb.PackfileNegotiationStatistics) {
+ if !response.NeedAudit {
+ return
+ }
+
+ ctx := r.Context()
+ err := a.SendGitAuditEvent(ctx, api.GitAuditEventRequest{
+ Action: action,
+ Protocol: "http",
+ Repo: response.GL_REPOSITORY,
+ Username: response.GL_USERNAME,
+ PackfileStats: stats,
+ })
+ if err != nil {
+ log.WithContextFields(ctx, log.Fields{
+ "repo": response.GL_REPOSITORY,
+ "action": action,
+ "username": response.GL_USERNAME,
+ }).WithError(err).Error("failed to send git audit event")
+ }
+ }
+}
+
func writePostRPCHeader(w http.ResponseWriter, action string) {
w.Header().Set("Content-Type", fmt.Sprintf("application/x-%s-result", action))
w.Header().Set("Cache-Control", "no-cache")
diff --git a/workhorse/internal/git/receive-pack.go b/workhorse/internal/git/receive-pack.go
index 5e93c0f36d11..3a87f6d1f240 100644
--- a/workhorse/internal/git/receive-pack.go
+++ b/workhorse/internal/git/receive-pack.go
@@ -4,13 +4,16 @@ import (
"fmt"
"net/http"
+ "gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
+
"gitlab.com/gitlab-org/gitlab/workhorse/internal/api"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/gitaly"
)
// Will not return a non-nil error after the response body has been
// written to.
-func handleReceivePack(w *HttpResponseWriter, r *http.Request, a *api.Response) error {
+// and `git push` doesn't provide `gitalypb.PackfileNegotiationStatistics`.
+func handleReceivePack(w *HttpResponseWriter, r *http.Request, a *api.Response) (*gitalypb.PackfileNegotiationStatistics, error) {
action := getService(r)
writePostRPCHeader(w, action)
@@ -21,12 +24,12 @@ func handleReceivePack(w *HttpResponseWriter, r *http.Request, a *api.Response)
ctx, smarthttp, err := gitaly.NewSmartHTTPClient(r.Context(), a.GitalyServer)
if err != nil {
- return fmt.Errorf("smarthttp.ReceivePack: %v", err)
+ return nil, fmt.Errorf("smarthttp.ReceivePack: %v", err)
}
if err := smarthttp.ReceivePack(ctx, &a.Repository, a.GL_ID, a.GL_USERNAME, a.GL_REPOSITORY, a.GitConfigOptions, cr, cw, gitProtocol); err != nil {
- return fmt.Errorf("smarthttp.ReceivePack: %w", err)
+ return nil, fmt.Errorf("smarthttp.ReceivePack: %w", err)
}
- return nil
+ return nil, nil
}
diff --git a/workhorse/internal/git/upload-pack.go b/workhorse/internal/git/upload-pack.go
index ef2a00bf3ac5..d67613b6f9b7 100644
--- a/workhorse/internal/git/upload-pack.go
+++ b/workhorse/internal/git/upload-pack.go
@@ -7,6 +7,8 @@ import (
"net/http"
"time"
+ "gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
+
"gitlab.com/gitlab-org/gitlab/workhorse/internal/api"
"gitlab.com/gitlab-org/gitlab/workhorse/internal/gitaly"
)
@@ -17,7 +19,7 @@ var (
// Will not return a non-nil error after the response body has been
// written to.
-func handleUploadPack(w *HttpResponseWriter, r *http.Request, a *api.Response) error {
+func handleUploadPack(w *HttpResponseWriter, r *http.Request, a *api.Response) (*gitalypb.PackfileNegotiationStatistics, error) {
ctx := r.Context()
// Prevent the client from holding the connection open indefinitely. A
@@ -42,15 +44,16 @@ func handleUploadPack(w *HttpResponseWriter, r *http.Request, a *api.Response) e
return handleUploadPackWithGitaly(ctx, a, cr, cw, gitProtocol)
}
-func handleUploadPackWithGitaly(ctx context.Context, a *api.Response, clientRequest io.Reader, clientResponse io.Writer, gitProtocol string) error {
+func handleUploadPackWithGitaly(ctx context.Context, a *api.Response, clientRequest io.Reader, clientResponse io.Writer, gitProtocol string) (*gitalypb.PackfileNegotiationStatistics, error) {
ctx, smarthttp, err := gitaly.NewSmartHTTPClient(ctx, a.GitalyServer)
if err != nil {
- return fmt.Errorf("get gitaly client: %w", err)
+ return nil, fmt.Errorf("get gitaly client: %w", err)
}
- if err := smarthttp.UploadPack(ctx, &a.Repository, clientRequest, clientResponse, gitConfigOptions(a), gitProtocol); err != nil {
- return fmt.Errorf("do gitaly call: %w", err)
+ resp, err := smarthttp.UploadPack(ctx, &a.Repository, clientRequest, clientResponse, gitConfigOptions(a), gitProtocol)
+ if err != nil {
+ return nil, fmt.Errorf("do gitaly call: %w", err)
}
- return nil
+ return resp.PackfileNegotiationStatistics, nil
}
diff --git a/workhorse/internal/git/upload-pack_test.go b/workhorse/internal/git/upload-pack_test.go
index 359262542f18..e816e6a34c95 100644
--- a/workhorse/internal/git/upload-pack_test.go
+++ b/workhorse/internal/git/upload-pack_test.go
@@ -66,7 +66,7 @@ func TestUploadPackTimesOut(t *testing.T) {
r := httptest.NewRequest("GET", "/", body)
a := &api.Response{GitalyServer: api.GitalyServer{Address: addr}}
- err := handleUploadPack(NewHttpResponseWriter(w), r, a)
+ _, err := handleUploadPack(NewHttpResponseWriter(w), r, a)
require.True(t, errors.Is(err, context.DeadlineExceeded))
}
diff --git a/workhorse/internal/gitaly/smarthttp.go b/workhorse/internal/gitaly/smarthttp.go
index e0ddf5ae4ebb..420c549895c9 100644
--- a/workhorse/internal/gitaly/smarthttp.go
+++ b/workhorse/internal/gitaly/smarthttp.go
@@ -94,7 +94,7 @@ func (client *SmartHTTPClient) ReceivePack(ctx context.Context, repo *gitalypb.R
return nil
}
-func (client *SmartHTTPClient) UploadPack(ctx context.Context, repo *gitalypb.Repository, clientRequest io.Reader, clientResponse io.Writer, gitConfigOptions []string, gitProtocol string) error {
+func (client *SmartHTTPClient) UploadPack(ctx context.Context, repo *gitalypb.Repository, clientRequest io.Reader, clientResponse io.Writer, gitConfigOptions []string, gitProtocol string) (*gitalypb.PostUploadPackWithSidechannelResponse, error) {
ctx, waiter := client.sidechannelRegistry.Register(ctx, func(conn gitalyclient.SidechannelConn) error {
if _, err := io.Copy(conn, clientRequest); err != nil {
return fmt.Errorf("copy request body: %w", err)
@@ -118,13 +118,14 @@ func (client *SmartHTTPClient) UploadPack(ctx context.Context, repo *gitalypb.Re
GitProtocol: gitProtocol,
}
- if _, err := client.PostUploadPackWithSidechannel(ctx, rpcRequest); err != nil {
- return fmt.Errorf("PostUploadPackWithSidechannel: %w", err)
+ resp, err := client.PostUploadPackWithSidechannel(ctx, rpcRequest)
+ if err != nil {
+ return nil, fmt.Errorf("PostUploadPackWithSidechannel: %w", err)
}
- if err := waiter.Close(); err != nil {
- return fmt.Errorf("close sidechannel waiter: %w", err)
+ if err = waiter.Close(); err != nil {
+ return nil, fmt.Errorf("close sidechannel waiter: %w", err)
}
- return nil
+ return resp, nil
}
--
GitLab