.gitignore

 *.log
 *.swp
+.DS_Store
+.GOPATH
 .bundle
 .bundle/
 .gitlab_shell_secret
 .idea
 /*.log*
-/bin/check
-/bin/gitlab-shell
-/bin/gitlab-shell-authorized-keys-check
-/bin/gitlab-shell-authorized-principals-check
-/bin/gitlab-sshd
+/bin/*
+/gl-code-quality-report.json
 /go_build
+/support/bin/golangci-*
+/support/bin/gotestsum-*
 authorized_keys.lock
 config.yml
 cover.out
+cover.xml
 custom_hooks
 hooks/*.d
 tags

.gitlab-ci.yml

@@ -4,8 +4,28 @@ include:
   - template: Security/Dependency-Scanning.gitlab-ci.yml
   - template: Security/Secret-Detection.gitlab-ci.yml
 
+stages:
+  - prepare
+  - lint
+  - test
+  - post-test
+
 variables:
-  DOCKER_VERSION: "20.10.3"
+  FF_USE_FASTZIP: 'true'
+  TRANSFER_METER_FREQUENCY: "1s"
+  DOCKER_VERSION: "20.10.15"
+  BUNDLE_FROZEN: "true"
+  GO_VERSION: "1.23"
+  GOPATH: $CI_PROJECT_DIR/.GOPATH
+  DEBIAN_VERSION: "bookworm"
+  RUBY_VERSION: "3.2.5"
+  BUNDLE_PATH: vendor/ruby
+  POLICY: pull
+  CI_DEBUG_SERVICES: 'true'
+  RUST_VERSION: "1.73"
+  UBI_VERSION: "8.6"
+  IMAGE_TAG: "rubygems-3.5-git-2.45-exiftool-12.60"
+  GITLAB_ADVANCED_SAST_ENABLED: 'true'
 
 workflow:
   rules: &workflow_rules
@@ -16,8 +36,15 @@ workflow:
     # For tags, create a pipeline.
     - if: '$CI_COMMIT_TAG'
 
+.rules:go-changes:
+  rules:
+    - changes:
+        - 'go.mod'
+        - 'go.sum'
+        - '**/*.go'
+
 default:
-  image: golang:1.14
+  image: registry.gitlab.com/gitlab-org/gitlab-build-images/debian-${DEBIAN_VERSION}-ruby-${RUBY_VERSION}-golang-${GO_VERSION}-rust-${RUST_VERSION}:${IMAGE_TAG}
   tags:
     - gitlab-org
 
@@ -29,17 +56,78 @@ default:
     # See https://gitlab.com/gitlab-com/www-gitlab-com/-/issues/7019 for tag descriptions
     - gitlab-org-docker
 
-.test:
+.cached-go: &cached_go
+  - key:
+      prefix: "golang-${GO_VERSION}-cache"
+      files:
+        - go.mod
+        - go.sum
+    policy: $POLICY
+    paths:
+      - .GOPATH/pkg/mod/
+
+.cached-ruby: &cached_ruby
+  - key:
+      prefix: "ruby-${RUBY_VERSION}-cache"
+      files:
+        - Gemfile.lock
+    policy: $POLICY
+    paths:
+      - ${BUNDLE_PATH}
+
+.cached-go-job:
+  variables:
+    CACHE_COMPRESSION_LEVEL: "fastest"
+  cache:
+    - *cached_go
+
+.cached-ruby-job:
+  cache:
+    - *cached_ruby
+
+.cached-job:
+  cache:
+    - *cached_go
+    - *cached_ruby
+
+.go-matrix-job:
+  parallel:
+    matrix:
+      - GO_VERSION: ["1.22", "1.23"]
+
+################################################################################
+# Prepare jobs
+################################################################################
+
+bundle:install:
+  stage: prepare
+  extends: .cached-ruby-job
+  variables:
+    POLICY: pull-push
+  script:
+    - bundle install --jobs $(nproc)
+
+modules:download:
+  stage: prepare
+  extends:
+    - .cached-go-job
+    - .go-matrix-job
+  variables:
+    POLICY: pull-push
+  script:
+    - go mod download
+
+################################################################################
+# Test jobs
+################################################################################
+
+.test-job:
+  needs: ['bundle:install', 'modules:download']
+  rules: !reference [".rules:go-changes", rules]
   variables:
     GITALY_CONNECTION_INFO: '{"address":"tcp://gitaly:8075", "storage":"default"}'
   before_script:
     # Set up the environment to run integration tests (still written in Ruby)
-    - apt-get update -qq && apt-get install -y ruby ruby-dev
-    - ruby -v
-    - export PATH=~/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/go/bin
-    - gem install --force --bindir /usr/local/bin bundler -v 2.3.6
-    - bundle install
-    # Now set up to run the Golang tests
     - make build
     - cp config.yml.example config.yml
     - go version
@@ -47,37 +135,62 @@ default:
   services:
     - name: registry.gitlab.com/gitlab-org/build/cng/gitaly:master
       # Disable the hooks so we don't have to stub the GitLab API
-      command: ["bash", "-c", "mkdir -p /home/git/repositories && rm -rf /srv/gitlab-shell/hooks/* && exec /usr/bin/env GITALY_TESTING_NO_GIT_HOOKS=1 /scripts/process-wrapper"]
+      command: ["bash", "-c", "mkdir -p /home/git/repositories && rm -rf /srv/gitlab-shell/hooks/* && touch /srv/gitlab-shell/.gitlab_shell_secret && exec /usr/bin/env GITALY_TESTING_NO_GIT_HOOKS=1 /scripts/process-wrapper"]
       alias: gitaly
-  script:
-    - make verify test
 
-go:1.16:
-  extends: .test
-  image: golang:1.16
+tests:
+  extends:
+    - .cached-job
+    - .go-matrix-job
+    - .test-job
+  script:
+    - make verify test_fancy
   after_script:
     - make coverage
   coverage: '/\d+.\d+%/'
+  artifacts:
+    when: always
+    paths:
+      - cover.xml
+    reports:
+      junit: cover.xml
 
-go:1.17:
-  extends: .test
-  image: golang:1.17
-  after_script:
-    - make coverage
-  coverage: '/\d+.\d+%/'
+tests_without_cgo:
+  extends:
+    - .cached-job
+    - .go-matrix-job
+    - .test-job
+  variables:
+    CGO_ENABLED: 0
+  script:
+    - make verify test_fancy
+
+tests:fips:
+  image: registry.gitlab.com/gitlab-org/gitlab-build-images/ubi-${UBI_VERSION}-ruby-${RUBY_VERSION}-golang-${GO_VERSION}-rust-${RUST_VERSION}:${IMAGE_TAG}
+  extends:
+    - .cached-job
+    - .test-job
+  variables:
+    FIPS_MODE: 1
+  script:
+    - make test_fancy
 
 race:
-  extends: .test
-  image: golang:1.17
+  extends:
+    - .cached-go-job
+    - .go-matrix-job
+    - .test-job
   script:
     - make test_golang_race
 
 code_quality:
+  stage: lint
   extends: .use-docker-in-docker
   rules: *workflow_rules
 
 code_navigation:
-  image: sourcegraph/lsif-go:v1.3.1
+  stage: post-test
+  image: sourcegraph/lsif-go:v1.9
   allow_failure: true
   script:
     - lsif-go
@@ -86,16 +199,85 @@ code_navigation:
       lsif: dump.lsif
 
 # SAST
-gosec-sast:
+semgrep-sast:
+  stage: lint
   rules: *workflow_rules
 
-# Dependency Scanning
-gemnasium-dependency_scanning:
+gitlab-advanced-sast:
+  stage: lint
   rules: *workflow_rules
 
-bundler-audit-dependency_scanning:
+# Dependency Scanning
+gemnasium-dependency_scanning:
+  stage: lint
   rules: *workflow_rules
 
 # Secret Detection
 secret_detection:
+  stage: lint
   rules: *workflow_rules
+
+build-package-and-qa:
+  stage: post-test
+  trigger:
+    project: 'gitlab-org/build/omnibus-gitlab-mirror'
+    branch: 'master'
+    strategy: depend
+  inherit:
+    variables: false
+  variables:
+    GITLAB_SHELL_VERSION: $CI_MERGE_REQUEST_SOURCE_BRANCH_SHA
+    TOP_UPSTREAM_SOURCE_PROJECT: $CI_PROJECT_PATH
+    TOP_UPSTREAM_SOURCE_REF: $CI_COMMIT_REF_NAME
+    TOP_UPSTREAM_SOURCE_JOB: $CI_JOB_URL
+    ee: "true"
+  rules:
+    # For MRs that change dependencies, we want to automatically ensure builds
+    # aren't broken. In such cases, we don't want the QA tests to be run
+    # automatically, but still available for developers to manually run.
+    - if: '$CI_MERGE_REQUEST_IID'
+      changes:
+        - go.sum
+      variables:
+        BUILD_ON_ALL_OS: "true"
+        MANUAL_QA_TEST: "true"
+      allow_failure: false
+    # For other MRs, we still provide this job as a manual job for developers
+    # to obtain a package for testing and run QA tests.
+    - if: '$CI_MERGE_REQUEST_IID'
+      when: manual
+      allow_failure: true
+  needs: []
+
+modules:tidy:
+  stage: lint
+  needs: ['modules:download']
+  script:
+    - go mod tidy
+    - git diff --exit-code go.mod go.sum
+
+lint:
+  stage: lint
+  script:
+    # Write the code coverage report to gl-code-quality-report.json
+    # and print linting issues to stdout in the format: path/to/file:line description
+    # remove `--issues-exit-code 0` or set to non-zero to fail the job if linting issues are detected
+    - apt update && apt install -y jq
+    - make lint GOLANGCI_LINT_ARGS="--out-format code-climate:gl-code-quality-report-temp.json,line-number"
+    - cat gl-code-quality-report-temp.json | jq '[ .[] | select(.severity == "warning").severity |= "minor" ]' > gl-code-quality-report.json
+    - rm -f gl-code-quality-report-temp.json
+  artifacts:
+    reports:
+      codequality: gl-code-quality-report.json
+    paths:
+      - gl-code-quality-report.json
+
+nilaway:
+  stage: lint
+  rules: !reference [".rules:go-changes", rules]
+  before_script:
+    - go install go.uber.org/nilaway/cmd/nilaway@latest
+  script:
+    - ${GOPATH}/bin/nilaway ./... > /tmp/out.txt 2>&1 || true
+    - cat /tmp/out.txt
+  allow_failure: true

.gitlab/CODEOWNERS

-* @ashmckenzie @igor.drozdov @nick.thomas @patrickbajao
+# https://gitlab.com/groups/gitlab-org/maintainers/gitlab-shell/-/group_members?with_inherited_permissions=exclude
+* @gitlab-org/maintainers/gitlab-shell
 
-[Documentation]
-*.md @aqualls
+[Documentation] @gl-docsteam
+*.md 
+/doc/

.golangci.yml

+# This file contains all available configuration options
+# with their default values.
+
+# options for analysis running
+run:
+  # default concurrency is a available CPU number
+  # concurrency: 4
+
+  # timeout for analysis, e.g. 30s, 5m, default is 1m
+  timeout: 30m
+
+  # exit code when at least one issue was found, default is 1
+  issues-exit-code: 1
+
+  # include test files or not, default is true
+  tests: true
+
+  # list of build tags, all linters use it. Default is empty list.
+  # build-tags:
+  #   - mytag
+
+  # which dirs to skip: issues from them won't be reported;
+  # can use regexp here: generated.*, regexp is applied on full path;
+  # default value is empty list, but default dirs are skipped independently
+  # from this option's value (see skip-dirs-use-default).
+  # skip-dirs:
+  #   - src/external_libs
+  #   - autogenerated_by_my_lib
+
+  # default is true. Enables skipping of directories:
+  #   vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
+  skip-dirs-use-default: true
+
+  # which files to skip: they will be analyzed, but issues from them
+  # won't be reported. Default value is empty list, but there is
+  # no need to include all autogenerated files, we confidently recognize
+  # autogenerated files. If it's not please let us know.
+  # skip-files:
+  #   - ".*\\.my\\.go$"
+  #   - lib/bad.go
+
+  # by default isn't set. If set we pass it to "go list -mod={option}". From "go help modules":
+  # If invoked with -mod=readonly, the go command is disallowed from the implicit
+  # automatic updating of go.mod described above. Instead, it fails when any changes
+  # to go.mod are needed. This setting is most useful to check that go.mod does
+  # not need updates, such as in a continuous integration and testing system.
+  # If invoked with -mod=vendor, the go command assumes that the vendor
+  # directory holds the correct copies of dependencies and ignores
+  # the dependency descriptions in go.mod.
+  # modules-download-mode: readonly|release|vendor
+
+# output configuration options
+output:
+  # colored-line-number|line-number|json|tab|checkstyle|code-climate, default is "colored-line-number"
+  formats:
+    - format: line-number
+
+  # print lines of code with issue, default is true
+  print-issued-lines: true
+
+  # print linter name in the end of issue text, default is true
+  print-linter-name: true
+
+  sort-results: true
+
+# all available settings of specific linters
+linters-settings:
+  errcheck:
+    # report about not checking of errors in type assetions: `a := b.(MyStruct)`;
+    # default is false: such cases aren't reported by default.
+    check-type-assertions: false
+
+    # report about assignment of errors to blank identifier: `num, _ := strconv.Atoi(numStr)`;
+    # default is false: such cases aren't reported by default.
+    check-blank: false
+
+    # [deprecated] comma-separated list of pairs of the form pkg:regex
+    # the regex is used to ignore names within pkg. (default "fmt:.*").
+    # see https://github.com/kisielk/errcheck#the-deprecated-method for details
+    # ignore: fmt:.*,io/ioutil:^Read.*
+    ignore: ''
+
+    # path to a file containing a list of functions to exclude from checking
+    # see https://github.com/kisielk/errcheck#excluding-functions for details
+    # exclude: /path/to/file.txt
+
+  # Disable error checking, as errorcheck detects more errors and is more configurable.
+  gosec:
+    exclude:
+    - "G104"
+
+  funlen:
+    lines: 60
+    statements: 40
+
+  govet:
+    # report about shadowed variables
+    enable:
+      - shadow
+
+    # settings per analyzer
+    settings:
+      printf: # analyzer name, run `go tool vet help` to see all analyzers
+        funcs: # run `go tool vet help printf` to see available settings for `printf` analyzer
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf
+          - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf
+
+    # enable or disable analyzers by name
+    # enable:
+    #   - atomicalign
+    # enable-all: false
+    # disable:
+    #   - shadow
+    # disable-all: false
+  gofmt:
+    # simplify code: gofmt with `-s` option, true by default
+    simplify: true
+  goimports:
+    # put imports beginning with prefix after 3rd-party packages;
+    # it's a comma-separated list of prefixes
+    # local-prefixes: github.com/org/project
+  gocyclo:
+    # minimal code complexity to report, 30 by default (but we recommend 10-20)
+    min-complexity: 30
+  gocognit:
+    # minimal code complexity to report, 30 by default (but we recommend 10-20)
+    min-complexity: 20
+  maligned:
+    # print struct with more effective memory layout or not, false by default
+    suggest-new: true
+  dupl:
+    # tokens count to trigger issue, 150 by default
+    threshold: 100
+  goconst:
+    # minimal length of string constant, 3 by default
+    min-len: 3
+    # minimal occurrences count to trigger, 3 by default
+    min-occurrences: 3
+  depguard:
+    rules:
+      test:
+        files:
+          - $test
+        allow:
+          - $gostd
+          - github.com/stretchr/testify
+          - gitlab.com/gitlab-org/gitlab-shell
+          - gitlab.com/gitlab-org/labkit
+          - gitlab.com/gitlab-org/gitaly
+          - github.com/prometheus/client_golang/prometheus
+          - github.com/pires/go-proxyproto
+          - github.com/otiai10/copy
+          - github.com/hashicorp/go-retryablehttp
+          - github.com/golang-jwt/jwt
+          - github.com/mikesmitty/edkey
+          - github.com/sirupsen/logrus
+          - github.com/grpc-ecosystem/go-grpc-prometheus
+          - github.com/mattn/go-shellwords
+
+  #   list-type: blacklist
+  #   include-go-root: false
+  #   packages:
+  #     - github.com/sirupsen/logrus
+  #   packages-with-error-messages:
+  #     # specify an error message to output when a blacklisted package is used
+  #     github.com/sirupsen/logrus: "logging is allowed only by logutils.Log"
+  misspell:
+    # Correct spellings using locale preferences for US or UK.
+    # Default is to use a neutral variety of English.
+    # Setting locale to US will correct the British spelling of 'colour' to 'color'.
+    locale: US
+    ignore-words:
+      - GitLab
+  lll:
+    # max line length, lines longer will be reported. Default is 120.
+    # '\t' is counted as 1 character by default, and can be changed with the tab-width option
+    line-length: 120
+    # tab width in spaces. Default to 1.
+    tab-width: 1
+  unused:
+    # treat code as a program (not a library) and report unused exported identifiers; default is false.
+    # XXX: if you enable this setting, unused will report a lot of false-positives in text editors:
+    # if it's called for subdir of a project it can't find funcs usages. All text editor integrations
+    # with golangci-lint call it on a directory with the changed file.
+    check-exported: false
+  unparam:
+    # Inspect exported functions, default is false. Set to true if no external program/library imports your code.
+    # XXX: if you enable this setting, unparam will report a lot of false-positives in text editors:
+    # if it's called for subdir of a project it can't find external interfaces. All text editor integrations
+    # with golangci-lint call it on a directory with the changed file.
+    check-exported: false
+  nakedret:
+    # make an issue if func has more lines of code than this setting and it has naked returns; default is 30
+    max-func-lines: 30
+  prealloc:
+    # XXX: we don't recommend using this linter before doing performance profiling.
+    # For most programs usage of prealloc will be a premature optimization.
+
+    # Report preallocation suggestions only on simple loops that have no returns/breaks/continues/gotos in them.
+    # True by default.
+    simple: true
+    range-loops: true # Report preallocation suggestions on range loops, true by default
+    for-loops: false # Report preallocation suggestions on for loops, false by default
+  gocritic:
+    # Which checks should be enabled; can't be combined with 'disabled-checks';
+    # See https://go-critic.github.io/overview#checks-overview
+    # To check which checks are enabled run `GL_DEBUG=gocritic golangci-lint run`
+    # By default list of stable checks is used.
+    # enabled-checks:
+    #   - rangeValCopy
+
+    # Which checks should be disabled; can't be combined with 'enabled-checks'; default is empty
+    # disabled-checks:
+    #   - regexpMust
+
+    # Enable multiple checks by tags, run `GL_DEBUG=gocritic golangci-lint run` to see all tags and checks.
+    # Empty list by default. See https://github.com/go-critic/go-critic#usage -> section "Tags".
+    # enabled-tags:
+    #   - performance
+
+    settings: # settings passed to gocritic
+      captLocal: # must be valid enabled check name
+        paramsOnly: true
+    # rangeValCopy:
+    #   sizeThreshold: 32
+  godox:
+    # report any comments starting with keywords, this is useful for TODO or FIXME comments that
+    # might be left in the code accidentally and should be resolved before merging
+    keywords: # default keywords are TODO, BUG, and FIXME, these can be overwritten by this setting
+      - TODO
+      - BUG
+      - FIXME
+      - NOTE
+      - OPTIMIZE # marks code that should be optimized before merging
+      - HACK # marks hack-arounds that should be removed before merging
+  dogsled:
+    # checks assignments with too many blank identifiers; default is 2
+    max-blank-identifiers: 2
+
+  whitespace:
+    multi-if: false   # Enforces newlines (or comments) after every multi-line if statement
+    multi-func: false # Enforces newlines (or comments) after every multi-line function signature
+  wsl:
+    # If true append is only allowed to be cuddled if appending value is
+    # matching variables, fields or types on line above. Default is true.
+    strict-append: true
+    # Allow calls and assignments to be cuddled as long as the lines have any
+    # matching variables, fields or types. Default is true.
+    allow-assign-and-call: true
+    # Allow multiline assignments to be cuddled. Default is true.
+    allow-multiline-assign: true
+    # Allow declarations (var) to be cuddled.
+    allow-cuddle-declarations: false
+    # Allow trailing comments in ending of blocks
+    allow-trailing-comment: false
+    # Force newlines in end of case at this limit (0 = never).
+    force-case-trailing-whitespace: 0
+
+linters:
+  # please, do not use `enable-all`: it's deprecated and will be removed soon.
+  # inverted configuration with `enable-all` and `disable` is not scalable during updates of golangci-lint
+  disable-all: true
+  enable:
+    - bodyclose
+    - copyloopvar
+    - depguard
+    - dogsled
+    - dupl
+    - errcheck
+    - funlen
+    - gocognit
+    - goconst
+    - gocritic
+    - godox
+    - gofmt
+    - goimports
+    - gosec
+    - gosimple
+    - govet
+    - ineffassign
+    - misspell
+    - nakedret
+    - revive
+    - staticcheck
+    - stylecheck
+    - testifylint
+    - typecheck
+    - unconvert
+    - unparam
+    - unused
+    - whitespace
+  # don't enable:
+  # - deadcode
+  # - gochecknoglobals
+  # - gochecknoinits
+  # - gocyclo
+  # - lll
+  # - maligned
+  # - prealloc
+  # - varcheck
+
+issues:
+  # List of regexps of issue texts to exclude, empty list by default.
+  # But independently from this option we use default exclude patterns,
+  # it can be disabled by `exclude-use-default: false`. To list all
+  # excluded by default patterns execute `golangci-lint run --help`
+  # exclude:
+  #   - abcdef
+
+  # Excluding configuration per-path, per-linter, per-text and per-source
+  exclude-rules:
+    # Exclude some linters from running on tests files.
+    - path: _test\.go
+      linters:
+        - gocyclo
+        - errcheck
+        - dupl
+        - gosec
+        - funlen
+
+    # Exclude known linters from partially hard-vendored code,
+    # which is impossible to exclude via "nolint" comments.
+    # - path: internal/hmac/
+    #   text: "weak cryptographic primitive"
+    #   linters:
+    #     - gosec
+
+    # Exclude some staticcheck messages
+    # - linters:
+    #     - staticcheck
+    #   text: "SA9003:"
+
+    # Exclude lll issues for long lines with go:generate
+    - linters:
+        - lll
+      source: "^//go:generate "
+
+  # Independently from option `exclude` we use default exclude patterns,
+  # it can be disabled by this option. To list all
+  # excluded by default patterns execute `golangci-lint run --help`.
+  # Default value for this option is true.
+  exclude-use-default: false
+
+  # Maximum issues count per one linter. Set to 0 to disable. Default is 50.
+  max-issues-per-linter: 0
+
+  # Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
+  max-same-issues: 0
+
+  # Show only new issues: if there are unstaged changes or untracked files,
+  # only those changes are analyzed, else only changes in HEAD~ are analyzed.
+  # It's a super-useful option for integration of golangci-lint into existing
+  # large codebase. It's not practical to fix all existing issues at the moment
+  # of integration: much better don't allow issues in new code.
+  # Default is false.
+  new: false
+
+  # Show only new issues created after git revision `REV`
+  # This should be passed as flag during individual CI jobs.
+  # new-from-rev: REV
+
+  # Show only new issues created in git patch with set file path.
+  # new-from-patch: path/to/patch/file

.ruby-version

-2.7.5
+3.3.4

.tool-versions

-ruby 2.7.5
-golang 1.17.6
+ruby 3.3.5
+golang 1.23.2

Dangerfile

+# frozen_string_literal: true
+
+require 'gitlab-dangerfiles'
+
+Gitlab::Dangerfiles.for_project(self) do |gitlab_dangerfiles|
+  gitlab_dangerfiles.import_plugins
+
+  gitlab_dangerfiles.import_dangerfiles(except: %w[changelog commit_messages])
+end

Gemfile

 source 'https://rubygems.org'
 
 group :development, :test do
-  gem 'rspec', '~> 3.8.0'
+  gem 'rspec', '~> 3.13.0'
+  gem 'webrick', '~> 1.8', '>= 1.8.2'
+end
+
+group :development, :danger do
+  gem 'gitlab-dangerfiles', '~> 4.8.0'
 end

Gemfile.lock

 GEM
   remote: https://rubygems.org/
   specs:
-    diff-lcs (1.3)
-    rspec (3.8.0)
-      rspec-core (~> 3.8.0)
-      rspec-expectations (~> 3.8.0)
-      rspec-mocks (~> 3.8.0)
-    rspec-core (3.8.0)
-      rspec-support (~> 3.8.0)
-    rspec-expectations (3.8.1)
+    addressable (2.8.7)
+      public_suffix (>= 2.0.2, < 7.0)
+    bigdecimal (3.1.8)
+    claide (1.1.0)
+    claide-plugins (0.9.2)
+      cork
+      nap
+      open4 (~> 1.3)
+    colored2 (3.1.2)
+    cork (0.3.0)
+      colored2 (~> 3.1)
+    csv (3.3.0)
+    danger (9.4.3)
+      claide (~> 1.0)
+      claide-plugins (>= 0.9.2)
+      colored2 (~> 3.1)
+      cork (~> 0.1)
+      faraday (>= 0.9.0, < 3.0)
+      faraday-http-cache (~> 2.0)
+      git (~> 1.13)
+      kramdown (~> 2.3)
+      kramdown-parser-gfm (~> 1.0)
+      no_proxy_fix
+      octokit (>= 4.0)
+      terminal-table (>= 1, < 4)
+    danger-gitlab (8.0.0)
+      danger
+      gitlab (~> 4.2, >= 4.2.0)
+    diff-lcs (1.5.1)
+    faraday (2.9.2)
+      faraday-net_http (>= 2.0, < 3.2)
+    faraday-http-cache (2.5.1)
+      faraday (>= 0.8)
+    faraday-net_http (3.1.0)
+      net-http
+    git (1.19.1)
+      addressable (~> 2.8)
+      rchardet (~> 1.8)
+    gitlab (4.20.1)
+      httparty (~> 0.20)
+      terminal-table (>= 1.5.1)
+    gitlab-dangerfiles (4.8.0)
+      danger (>= 9.3.0)
+      danger-gitlab (>= 8.0.0)
+      rake (~> 13.0)
+    httparty (0.22.0)
+      csv
+      mini_mime (>= 1.0.0)
+      multi_xml (>= 0.5.2)
+    kramdown (2.4.0)
+      rexml
+    kramdown-parser-gfm (1.1.0)
+      kramdown (~> 2.0)
+    mini_mime (1.1.5)
+    multi_xml (0.7.1)
+      bigdecimal (~> 3.1)
+    nap (1.1.0)
+    net-http (0.4.1)
+      uri
+    no_proxy_fix (0.1.2)
+    octokit (6.1.1)
+      faraday (>= 1, < 3)
+      sawyer (~> 0.9)
+    open4 (1.3.4)
+    public_suffix (5.1.1)
+    rake (13.2.1)
+    rchardet (1.8.0)
+    rexml (3.3.1)
+      strscan
+    rspec (3.13.0)
+      rspec-core (~> 3.13.0)
+      rspec-expectations (~> 3.13.0)
+      rspec-mocks (~> 3.13.0)
+    rspec-core (3.13.0)
+      rspec-support (~> 3.13.0)
+    rspec-expectations (3.13.0)
       diff-lcs (>= 1.2.0, < 2.0)
-      rspec-support (~> 3.8.0)
-    rspec-mocks (3.8.0)
+      rspec-support (~> 3.13.0)
+    rspec-mocks (3.13.0)
       diff-lcs (>= 1.2.0, < 2.0)
-      rspec-support (~> 3.8.0)
-    rspec-support (3.8.0)
+      rspec-support (~> 3.13.0)
+    rspec-support (3.13.0)
+    sawyer (0.9.2)
+      addressable (>= 2.3.5)
+      faraday (>= 0.17.3, < 3)
+    strscan (3.1.0)
+    terminal-table (3.0.2)
+      unicode-display_width (>= 1.1.1, < 3)
+    unicode-display_width (2.5.0)
+    uri (0.13.0)
+    webrick (1.8.2)
 
 PLATFORMS
   ruby
 
 DEPENDENCIES
-  rspec (~> 3.8.0)
+  gitlab-dangerfiles (~> 4.8.0)
+  rspec (~> 3.13.0)
+  webrick (~> 1.8, >= 1.8.2)
 
 BUNDLED WITH
-   2.3.6
+   2.5.11

Makefile

-.PHONY: validate verify verify_ruby verify_golang test test_ruby test_golang coverage coverage_golang setup _script_install build compile check clean install
+.PHONY: validate verify verify_ruby verify_golang test test_ruby test_golang test_fancy test_golang_fancy coverage coverage_golang setup _script_install make_necessary_dirs build compile check clean install lint
 
-GO_SOURCES := $(shell find . -name '*.go')
+FIPS_MODE ?= 0
+OS := $(shell uname | tr A-Z a-z)
+GO_SOURCES := $(shell git ls-files \*.go)
 VERSION_STRING := $(shell git describe --match v* 2>/dev/null || awk '$$0="v"$$0' VERSION 2>/dev/null || echo unknown)
 BUILD_TIME := $(shell date -u +%Y%m%d.%H%M%S)
-BUILD_TAGS := tracer_static tracer_static_jaeger continuous_profiler_stackdriver
-GOBUILD_FLAGS := -ldflags "-X main.Version=$(VERSION_STRING) -X main.BuildTime=$(BUILD_TIME)" -tags "$(BUILD_TAGS)" -mod=mod
+GO_TAGS := tracer_static tracer_static_jaeger continuous_profiler_stackdriver
+
+ARCH ?= $(shell uname -m | sed -e 's/x86_64/amd64/' | sed -e 's/aarch64/arm64/')
+
+GOTESTSUM_VERSION := 1.12.0
+GOTESTSUM_FILE := support/bin/gotestsum-${GOTESTSUM_VERSION}
+
+GOLANGCI_LINT_VERSION := 1.60.3
+GOLANGCI_LINT_FILE := support/bin/golangci-lint-${GOLANGCI_LINT_VERSION}
+
+export GOFLAGS := -mod=readonly
+
+ifeq (${FIPS_MODE}, 1)
+    GO_TAGS += fips
+
+    # If the golang-fips compiler is built with CGO_ENABLED=0, this needs to be
+    # explicitly switched on.
+    export CGO_ENABLED=1
+
+    # Go 1.19 now requires GOEXPERIMENT=boringcrypto for FIPS compilation.
+    # See https://github.com/golang/go/issues/51940 for more details.
+    BORINGCRYPTO_SUPPORT := $(shell GOEXPERIMENT=boringcrypto go version > /dev/null 2>&1; echo $$?)
+    ifeq ($(BORINGCRYPTO_SUPPORT), 0)
+        export GOEXPERIMENT=boringcrypto
+    endif
+endif
+
+ifneq (${CGO_ENABLED}, 0)
+	GO_TAGS += gssapi
+endif
+
+ifeq (${OS}, darwin) # Mac OS
+    BREW_PREFIX := $(shell brew --prefix 2>/dev/null || echo "/opt/homebrew")
+
+    # To be able to compile gssapi library
+    export CGO_CFLAGS="-I$(BREW_PREFIX)/opt/heimdal/include"
+endif
+
+GOBUILD_FLAGS := -ldflags "-X main.Version=$(VERSION_STRING) -X main.BuildTime=$(BUILD_TIME)" -tags "$(GO_TAGS)" -mod=mod
 
 PREFIX ?= /usr/local
 
-build: bin/gitlab-shell
+build: compile
 
 validate: verify test
 
@@ -22,41 +61,67 @@ fmt:
 
 test: test_ruby test_golang
 
+test_fancy: test_ruby test_golang_fancy
+
 # The Ruby tests are now all integration specs that test the Go implementation.
 test_ruby:
 	bundle exec rspec --color --format d spec
 
 test_golang:
-	go test -cover -coverprofile=cover.out ./...
+	go test -cover -coverprofile=cover.out -count 1 -tags "$(GO_TAGS)" ./...
+
+test_golang_fancy: ${GOTESTSUM_FILE}
+	@${GOTESTSUM_FILE} --version
+	@${GOTESTSUM_FILE} --junitfile ./cover.xml --format pkgname -- -coverprofile=./cover.out -covermode=atomic -count 1 -tags "$(GO_TAGS)" ./...
+
+${GOTESTSUM_FILE}:
+	mkdir -p $(shell dirname ${GOTESTSUM_FILE})
+	curl -L https://github.com/gotestyourself/gotestsum/releases/download/v${GOTESTSUM_VERSION}/gotestsum_${GOTESTSUM_VERSION}_${OS}_${ARCH}.tar.gz | tar -zOxf - gotestsum > ${GOTESTSUM_FILE} && chmod +x ${GOTESTSUM_FILE}
 
 test_golang_race:
-	go test -race ./...
+	go test -race -count 1 ./...
 
 coverage: coverage_golang
 
 coverage_golang:
 	[ -f cover.out ] && go tool cover -func cover.out
 
-setup: _script_install bin/gitlab-shell
+lint:
+	@support/lint.sh ./...
+
+golangci: ${GOLANGCI_LINT_FILE}
+	@${GOLANGCI_LINT_FILE} run --issues-exit-code 0 --print-issued-lines=false ${GOLANGCI_LINT_ARGS}
+
+${GOLANGCI_LINT_FILE}:
+	@mkdir -p $(shell dirname ${GOLANGCI_LINT_FILE})
+	@curl -L https://github.com/golangci/golangci-lint/releases/download/v${GOLANGCI_LINT_VERSION}/golangci-lint-${GOLANGCI_LINT_VERSION}-${OS}-${ARCH}.tar.gz | tar --strip-components 1 -zOxf - golangci-lint-${GOLANGCI_LINT_VERSION}-${OS}-${ARCH}/golangci-lint > ${GOLANGCI_LINT_FILE} && chmod +x ${GOLANGCI_LINT_FILE}
 
-_script_install:
-	bin/install
+setup: make_necessary_dirs bin/gitlab-shell
 
-compile: bin/gitlab-shell
-bin/gitlab-shell: $(GO_SOURCES)
-	GOBIN="$(CURDIR)/bin" go install $(GOBUILD_FLAGS) ./cmd/...
+make_necessary_dirs:
+	support/make_necessary_dirs
+
+compile: bin/gitlab-shell bin/gitlab-sshd
+
+bin:
+	mkdir -p bin
+
+bin/gitlab-shell: bin $(GO_SOURCES)
+	go build $(GOBUILD_FLAGS) -o $(CURDIR)/bin ./cmd/...
+
+bin/gitlab-sshd: bin $(GO_SOURCES)
+	go build $(GOBUILD_FLAGS) -o $(CURDIR)/bin/gitlab-sshd ./cmd/gitlab-sshd
 
 check:
-	bin/check
+	bin/gitlab-shell-check
 
 clean:
-	rm -f bin/check bin/gitlab-shell bin/gitlab-shell-authorized-keys-check bin/gitlab-shell-authorized-principals-check bin/gitlab-sshd
+	rm -f bin/*
 
 install: compile
 	mkdir -p $(DESTDIR)$(PREFIX)/bin/
-	install -m755 bin/check $(DESTDIR)$(PREFIX)/bin/check
-	install -m755 bin/gitlab-shell $(DESTDIR)$(PREFIX)/bin/gitlab-shell
-	install -m755 bin/gitlab-shell $(DESTDIR)$(PREFIX)/bin/gitlab-shell-authorized-keys-check
-	install -m755 bin/gitlab-shell $(DESTDIR)$(PREFIX)/bin/gitlab-shell-authorized-principals-check
-	install -m755 bin/gitlab-shell $(DESTDIR)$(PREFIX)/bin/gitlab-sshd
-
+	install -m755 bin/gitlab-shell-check $(DESTDIR)$(PREFIX)/bin/
+	install -m755 bin/gitlab-shell $(DESTDIR)$(PREFIX)/bin/
+	install -m755 bin/gitlab-shell-authorized-keys-check $(DESTDIR)$(PREFIX)/bin/
+	install -m755 bin/gitlab-shell-authorized-principals-check $(DESTDIR)$(PREFIX)/bin/
+	install -m755 bin/gitlab-sshd $(DESTDIR)$(PREFIX)/bin/

PROCESS.md

-## Releasing a new version
+# GitLab Shell process
 
-GitLab Shell is versioned by git tags, and the version used by the Rails
-application is stored in
-[`GITLAB_SHELL_VERSION`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/GITLAB_SHELL_VERSION).
-
-For each version, there is a raw version and a tag version:
-
-- The **raw version** is the version number. For instance, `15.2.8`.
-- The **tag version** is the raw version prefixed with `v`. For instance, `v15.2.8`.
-
-To release a new version of GitLab Shell and have that version available to the
-Rails application:
-
-1. Create a merge request to update the [`CHANGELOG`](CHANGELOG) with the
-   **tag version** and the [`VERSION`](VERSION) file with the **raw version**.
-2. Ask a maintainer to review and merge the merge request. If you're already a
-   maintainer, second maintainer review is not required.
-3. Add a new git tag with the **tag version**.
-4. Update `GITLAB_SHELL_VERSION` in the Rails application to the **raw
-   version**. (Note: this can be done as a separate MR to that, or in and MR
-   that will make use of the latest GitLab Shell changes.)
-
-## Security releases
-
-GitLab Shell is included in the packages we create for GitLab, and each version of GitLab specifies the version of GitLab Shell it uses in the `GITLAB_SHELL_VERSION` file, so security fixes in GitLab Shell are tightly coupled to the [GitLab security release](https://about.gitlab.com/handbook/engineering/workflow/#security-issues) workflow.
-
-For a security fix in GitLab Shell, two sets of merge requests are required:
-
-* The fix itself, in the `gitlab-org/security/gitlab-shell` repository and its backports to the previous versions of GitLab Shell
-* Merge requests to change the versions of GitLab Shell included in the GitLab security release, in the `gitlab-org/security/gitlab` repository
-
-The first step could be to create a merge request with a fix targeting `main` in `gitlab-org/security/gitlab-shell`. When the merge request is approved by maintainers, backports targeting previous 3 versions of GitLab Shell must be created. The stable branches for those versions may not exist, feel free to ask a maintainer to create ones. The stable branches must be created out of the GitLab Shell tags/version used by the 3 previous GitLab releases. In order to find out the GitLab Shell version that is used on a particular GitLab stable release, the following steps may be helpful:
-
-```shell
-git fetch security 13-9-stable-ee
-git show refs/remotes/security/13-9-stable-ee:GITLAB_SHELL_VERSION
-```
-
-These steps display the version that is used by `13.9` version of GitLab.
-
-Close to the GitLab security release, a maintainer should merge the fix and backports and cut all the necessary GitLab Shell versions. This allows bumping the `GITLAB_SHELL_VERSION` for `gitlab-org/security/gitlab`. The GitLab merge request will be handled by the general GitLab security release process.
-
-Once the security release is done, a GitLab Shell maintainer is responsible for syncing tags and `main` to the `gitlab-org/gitlab-shell` repository.
+This page [has moved into the `gitlab` repository](https://docs.gitlab.com/ee/development/gitlab_shell/process.html).

README.md

@@ -4,63 +4,48 @@ group: Source Code
 info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments
 ---
 
-# GitLab Shell
-
-## GitLab Shell handles git SSH sessions for GitLab
-
-GitLab Shell handles git SSH sessions for GitLab and modifies the list of authorized keys.
-GitLab Shell is not a Unix shell nor a replacement for Bash or Zsh.
-
-When you access the GitLab server over SSH then GitLab Shell will:
-
-1. Limit you to predefined git commands (git push, git pull).
-1. Call the GitLab Rails API to check if you are authorized, and what Gitaly server your repository is on
-1. Copy data back and forth between the SSH client and the Gitaly server
-
-If you access a GitLab server over HTTP(S) you end up in [gitlab-workhorse](https://gitlab.com/gitlab-org/gitlab/tree/master/workhorse).
-
-An overview of the four cases described above:
-
-1. git pull over SSH -> gitlab-shell -> API call to gitlab-rails (Authorization) -> accept or decline -> establish Gitaly session
-1. git push over SSH -> gitlab-shell (git command is not executed yet) -> establish Gitaly session -> (in Gitaly) gitlab-shell pre-receive hook -> API call to gitlab-rails (authorization) -> accept or decline push
-
-[Full feature list](doc/features.md)
-
-## Code status
-
 [![pipeline status](https://gitlab.com/gitlab-org/gitlab-shell/badges/main/pipeline.svg)](https://gitlab.com/gitlab-org/gitlab-shell/-/pipelines?ref=main)
 [![coverage report](https://gitlab.com/gitlab-org/gitlab-shell/badges/main/coverage.svg)](https://gitlab.com/gitlab-org/gitlab-shell/-/pipelines?ref=main)
 [![Code Climate](https://codeclimate.com/github/gitlabhq/gitlab-shell.svg)](https://codeclimate.com/github/gitlabhq/gitlab-shell)
 
-## Requirements
+# GitLab Shell
 
-GitLab Shell is written in Go, and needs a Go compiler to build. It still requires
-Ruby to build and test, but not to run.
+GitLab Shell handles Git SSH sessions for GitLab and modifies the list of
+authorized keys. GitLab Shell is not a Unix shell nor a replacement for Bash or Zsh.
 
-Download and install the current version of Go from https://golang.org/dl/
+GitLab supports Git LFS authentication through SSH.
 
-We follow the [Golang Release Policy](https://golang.org/doc/devel/release.html#policy)
-of supporting the current stable version and the previous two major versions.
+## Development Documentation
 
-## Rate Limiting
+Development documentation for GitLab Shell [has moved into the `gitlab` repository](https://docs.gitlab.com/ee/development/gitlab_shell/).
 
-GitLab Shell performs rate-limiting by user account and project for git operations. GitLab Shell accepts git operation requests and then makes a call to the Rails rate-limiter (backed by Redis). If the `user + project` exceeds the rate limit then GitLab Shell will then drop further connection requests for that `user + project`.
+## Project structure
 
-The rate-limiter is applied at the git command (plumbing) level. Each command has a rate limit of 600/minute. For example, `git push` has 600/minute and `git pull` has another 600/minute.
+| Directory | Description |
+|-----------|-------------|
+| `cmd/` | 'Commands' that will ultimately be compiled into binaries. |
+| `internal/` | Internal Go source code that is not intended to be used outside of the project/module. |
+| `client/` | HTTP and GitLab client logic that is used internally and by other modules, e.g. Gitaly. |
+| `bin/` | Compiled binaries are created here. |
+| `support/` | Scripts and tools that assist in development and/or testing. |
+| `spec/` | Ruby based integration tests. |
 
-Because they are using the same plumbing command `git-upload-pack`, `git pull` and `git clone` are in effect the same command for the purposes of rate-limiting.
+## Building
 
-There is also a rate-limiter in place in Gitaly, but the calls will never be made to Gitaly if the rate limit is exceeded in Gitlab Shell (Rails).
+Run `make` or `make build`.
 
-## Releasing
+## Testing
 
-See [PROCESS.md](./PROCESS.md)
+Run `make test`.
 
-## Contributing
+## Release Process
 
-- See [CONTRIBUTING.md](./CONTRIBUTING.md).
-- See the [beginner's guide](doc/beginners_guide.md).
+1. Create a `gitlab-org/gitlab-shell` MR to update [`VERSION`](https://gitlab.com/gitlab-org/gitlab-shell/-/blob/main/VERSION) and [`CHANGELOG`](https://gitlab.com/gitlab-org/gitlab-shell/-/blob/main/CHANGELOG) files, e.g. [Release v14.39.0](https://gitlab.com/gitlab-org/gitlab-shell/-/merge_requests/1123).
+2. Once `gitlab-org/gitlab-shell` MR is merged, create the corresponding git tag, e.g. https://gitlab.com/gitlab-org/gitlab-shell/-/tags/v14.39.0.
+3. Create a `gitlab-org/gitlab` MR to update [`GITLAB_SHELL_VERSION`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/GITLAB_SHELL_VERSION) to the proposed tag, e.g. [Bump GitLab Shell to 14.39.0](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/162661).
+4. Announce in `#gitlab-shell` a new version has been created.
 
-## License
+## Licensing
 
-See [LICENSE](./LICENSE).
+See the `LICENSE` file for licensing information as it pertains to files in
+this repository.

VERSION

-13.23.2
+14.39.0

client/client_test.go

@@ -2,54 +2,81 @@ package client
 
 import (
 	"context"
-	"encoding/base64"
 	"encoding/json"
 	"fmt"
 	"io"
 	"net/http"
+	"net/http/httptest"
 	"path"
 	"strings"
 	"testing"
+	"time"
 
+	"github.com/golang-jwt/jwt/v5"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
-	"gitlab.com/gitlab-org/gitlab-shell/client/testserver"
-	"gitlab.com/gitlab-org/gitlab-shell/internal/testhelper"
+	"gitlab.com/gitlab-org/gitlab-shell/v14/client/testserver"
+	"gitlab.com/gitlab-org/gitlab-shell/v14/internal/testhelper"
+)
+
+var (
+	secret          = "sssh, it's a secret"
+	defaultHttpOpts = []HTTPClientOpt{WithHTTPRetryOpts(time.Millisecond, time.Millisecond, 2)}
 )
 
 func TestClients(t *testing.T) {
-	testhelper.PrepareTestRootDir(t)
+	testRoot := testhelper.PrepareTestRootDir(t)
 
 	testCases := []struct {
 		desc            string
 		relativeURLRoot string
 		caFile          string
 		server          func(*testing.T, []testserver.TestRequestHandler) string
+		secret          string
 	}{
 		{
 			desc:   "Socket client",
-			server: testserver.StartSocketHttpServer,
+			server: testserver.StartSocketHTTPServer,
+			secret: secret,
 		},
 		{
 			desc:            "Socket client with a relative URL at /",
 			relativeURLRoot: "/",
-			server:          testserver.StartSocketHttpServer,
+			server:          testserver.StartSocketHTTPServer,
+			secret:          secret,
 		},
 		{
 			desc:            "Socket client with relative URL at /gitlab",
 			relativeURLRoot: "/gitlab",
-			server:          testserver.StartSocketHttpServer,
+			server:          testserver.StartSocketHTTPServer,
+			secret:          secret,
 		},
 		{
 			desc:   "Http client",
-			server: testserver.StartHttpServer,
+			server: testserver.StartHTTPServer,
+			secret: secret,
 		},
 		{
 			desc:   "Https client",
-			caFile: path.Join(testhelper.TestRoot, "certs/valid/server.crt"),
+			caFile: path.Join(testRoot, "certs/valid/server.crt"),
 			server: func(t *testing.T, handlers []testserver.TestRequestHandler) string {
-				return testserver.StartHttpsServer(t, handlers, "")
+				return testserver.StartHTTPSServer(t, handlers, "")
 			},
+			secret: secret,
+		},
+		{
+			desc:   "Secret with newlines",
+			caFile: path.Join(testRoot, "certs/valid/server.crt"),
+			server: func(t *testing.T, handlers []testserver.TestRequestHandler) string {
+				return testserver.StartHTTPSServer(t, handlers, "")
+			},
+			secret: "\n" + secret + "\n",
+		},
+		{
+			desc:   "Retry client",
+			server: testserver.StartRetryHTTPServer,
+			secret: secret,
 		},
 	}
 
@@ -57,12 +84,10 @@ func TestClients(t *testing.T) {
 		t.Run(tc.desc, func(t *testing.T) {
 			url := tc.server(t, buildRequests(t, tc.relativeURLRoot))
 
-			secret := "sssh, it's a secret"
-
-			httpClient, err := NewHTTPClientWithOpts(url, tc.relativeURLRoot, tc.caFile, "", false, 1, nil)
+			httpClient, err := NewHTTPClientWithOpts(url, tc.relativeURLRoot, tc.caFile, "", 1, defaultHttpOpts)
 			require.NoError(t, err)
 
-			client, err := NewGitlabNetClient("", "", secret, httpClient)
+			client, err := NewGitlabNetClient("", "", tc.secret, httpClient)
 			require.NoError(t, err)
 
 			testBrokenRequest(t, client)
@@ -70,7 +95,9 @@ func TestClients(t *testing.T) {
 			testSuccessfulPost(t, client)
 			testMissing(t, client)
 			testErrorMessage(t, client)
-			testAuthenticationHeader(t, client)
+			testJWTAuthenticationHeader(t, client)
+			testXForwardedForHeader(t, client)
+			testHostWithTrailingSlash(t, client)
 		})
 	}
 }
@@ -147,24 +174,47 @@ func testBrokenRequest(t *testing.T, client *GitlabNetClient) {
 	})
 }
 
-func testAuthenticationHeader(t *testing.T, client *GitlabNetClient) {
-	t.Run("Authentication headers for GET", func(t *testing.T) {
-		response, err := client.Get(context.Background(), "/auth")
+func testJWTAuthenticationHeader(t *testing.T, client *GitlabNetClient) {
+	verifyJWTToken := func(t *testing.T, response *http.Response) {
+		responseBody, err := io.ReadAll(response.Body)
+		require.NoError(t, err)
+
+		claims := &jwt.RegisteredClaims{}
+		token, err := jwt.ParseWithClaims(string(responseBody), claims, func(token *jwt.Token) (interface{}, error) {
+			return []byte(secret), nil
+		})
+		require.NoError(t, err)
+		require.True(t, token.Valid)
+		require.Equal(t, "gitlab-shell", claims.Issuer)
+		require.WithinDuration(t, time.Now().Truncate(time.Second), claims.IssuedAt.Time, time.Second)
+		require.WithinDuration(t, time.Now().Truncate(time.Second).Add(time.Minute), claims.ExpiresAt.Time, time.Second)
+	}
+
+	t.Run("JWT authentication headers for GET", func(t *testing.T) {
+		response, err := client.Get(context.Background(), "/jwt_auth")
 		require.NoError(t, err)
 		require.NotNil(t, response)
 
 		defer response.Body.Close()
 
-		responseBody, err := io.ReadAll(response.Body)
-		require.NoError(t, err)
+		verifyJWTToken(t, response)
+	})
 
-		header, err := base64.StdEncoding.DecodeString(string(responseBody))
+	t.Run("JWT authentication headers for POST", func(t *testing.T) {
+		response, err := client.Post(context.Background(), "/jwt_auth", map[string]string{})
 		require.NoError(t, err)
-		require.Equal(t, "sssh, it's a secret", string(header))
+		require.NotNil(t, response)
+
+		defer response.Body.Close()
+
+		verifyJWTToken(t, response)
 	})
+}
 
-	t.Run("Authentication headers for POST", func(t *testing.T) {
-		response, err := client.Post(context.Background(), "/auth", map[string]string{})
+func testXForwardedForHeader(t *testing.T, client *GitlabNetClient) {
+	t.Run("X-Forwarded-For Header inserted if original address in context", func(t *testing.T) {
+		ctx := context.WithValue(context.Background(), OriginalRemoteIPContextKey{}, "196.7.0.238")
+		response, err := client.Get(ctx, "/x_forwarded_for")
 		require.NoError(t, err)
 		require.NotNil(t, response)
 
@@ -172,19 +222,26 @@ func testAuthenticationHeader(t *testing.T, client *GitlabNetClient) {
 
 		responseBody, err := io.ReadAll(response.Body)
 		require.NoError(t, err)
-
-		header, err := base64.StdEncoding.DecodeString(string(responseBody))
-		require.NoError(t, err)
-		require.Equal(t, "sssh, it's a secret", string(header))
+		require.Equal(t, "196.7.0.238", string(responseBody))
 	})
 }
 
+func testHostWithTrailingSlash(t *testing.T, client *GitlabNetClient) {
+	oldHost := client.httpClient.Host
+	client.httpClient.Host = oldHost + "/"
+
+	testSuccessfulGet(t, client)
+	testSuccessfulPost(t, client)
+
+	client.httpClient.Host = oldHost
+}
+
 func buildRequests(t *testing.T, relativeURLRoot string) []testserver.TestRequestHandler {
 	requests := []testserver.TestRequestHandler{
 		{
 			Path: "/api/v4/internal/hello",
 			Handler: func(w http.ResponseWriter, r *http.Request) {
-				require.Equal(t, http.MethodGet, r.Method)
+				assert.Equal(t, http.MethodGet, r.Method)
 
 				fmt.Fprint(w, "Hello")
 			},
@@ -192,20 +249,26 @@ func buildRequests(t *testing.T, relativeURLRoot string) []testserver.TestReques
 		{
 			Path: "/api/v4/internal/post_endpoint",
 			Handler: func(w http.ResponseWriter, r *http.Request) {
-				require.Equal(t, http.MethodPost, r.Method)
+				assert.Equal(t, http.MethodPost, r.Method)
 
 				b, err := io.ReadAll(r.Body)
 				defer r.Body.Close()
 
-				require.NoError(t, err)
+				assert.NoError(t, err)
 
 				fmt.Fprint(w, "Echo: "+string(b))
 			},
 		},
 		{
-			Path: "/api/v4/internal/auth",
+			Path: "/api/v4/internal/jwt_auth",
 			Handler: func(w http.ResponseWriter, r *http.Request) {
-				fmt.Fprint(w, r.Header.Get(secretHeaderName))
+				fmt.Fprint(w, r.Header.Get(apiSecretHeaderName))
+			},
+		},
+		{
+			Path: "/api/v4/internal/x_forwarded_for",
+			Handler: func(w http.ResponseWriter, r *http.Request) {
+				fmt.Fprint(w, r.Header.Get("X-Forwarded-For"))
 			},
 		},
 		{
@@ -236,3 +299,22 @@ func buildRequests(t *testing.T, relativeURLRoot string) []testserver.TestReques
 
 	return requests
 }
+
+func TestRetryOnFailure(t *testing.T) {
+	reqAttempts := 0
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		reqAttempts++
+		w.WriteHeader(500)
+	}))
+	defer srv.Close()
+
+	httpClient, err := NewHTTPClientWithOpts(srv.URL, "/", "", "", 1, defaultHttpOpts)
+	require.NoError(t, err)
+	require.NotNil(t, httpClient.RetryableHTTP)
+	client, err := NewGitlabNetClient("", "", "", httpClient)
+	require.NoError(t, err)
+
+	_, err = client.Get(context.Background(), "/")
+	require.EqualError(t, err, "Internal API unreachable")
+	require.Equal(t, 3, reqAttempts)
+}

client/gitlabnet.go

+// Package client provides a client for interacting with GitLab API
 package client
 
 import (
 	"bytes"
 	"context"
-	"encoding/base64"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -11,36 +11,53 @@ import (
 	"strings"
 	"time"
 
-	"gitlab.com/gitlab-org/labkit/log"
+	"github.com/golang-jwt/jwt/v5"
+	"github.com/hashicorp/go-retryablehttp"
 )
 
 const (
-	internalApiPath  = "/api/v4/internal"
-	secretHeaderName = "Gitlab-Shared-Secret"
-	defaultUserAgent = "GitLab-Shell"
+	internalAPIPath     = "/api/v4/internal"
+	apiSecretHeaderName = "Gitlab-Shell-Api-Request" // #nosec G101
+	defaultUserAgent    = "GitLab-Shell"
+	jwtTTL              = time.Minute
+	jwtIssuer           = "gitlab-shell"
 )
 
+// ErrorResponse represents an error response from the API
 type ErrorResponse struct {
 	Message string `json:"message"`
 }
 
+// GitlabNetClient is a client for interacting with GitLab API
 type GitlabNetClient struct {
-	httpClient *HttpClient
+	httpClient *HTTPClient
 	user       string
 	password   string
 	secret     string
 	userAgent  string
 }
 
+// APIError represents an API error
+type APIError struct {
+	Msg string
+}
+
+// OriginalRemoteIPContextKey is used as the key in a Context to set an X-Forwarded-For header in a request
+type OriginalRemoteIPContextKey struct{}
+
+func (e *APIError) Error() string {
+	return e.Msg
+}
+
+// NewGitlabNetClient creates a new GitlabNetClient instance
 func NewGitlabNetClient(
 	user,
 	password,
 	secret string,
-	httpClient *HttpClient,
+	httpClient *HTTPClient,
 ) (*GitlabNetClient, error) {
-
 	if httpClient == nil {
-		return nil, fmt.Errorf("Unsupported protocol")
+		return nil, fmt.Errorf("unsupported protocol")
 	}
 
 	return &GitlabNetClient{
@@ -63,13 +80,17 @@ func normalizePath(path string) string {
 		path = "/" + path
 	}
 
-	if !strings.HasPrefix(path, internalApiPath) {
-		path = internalApiPath + path
+	if !strings.HasPrefix(path, internalAPIPath) {
+		path = internalAPIPath + path
 	}
 	return path
 }
 
-func newRequest(ctx context.Context, method, host, path string, data interface{}) (*http.Request, error) {
+func appendPath(host string, path string) string {
+	return strings.TrimSuffix(host, "/") + "/" + strings.TrimPrefix(path, "/")
+}
+
+func newRequest(ctx context.Context, method, host, path string, data interface{}) (*retryablehttp.Request, error) {
 	var jsonReader io.Reader
 	if data != nil {
 		jsonData, err := json.Marshal(data)
@@ -80,7 +101,7 @@ func newRequest(ctx context.Context, method, host, path string, data interface{}
 		jsonReader = bytes.NewReader(jsonData)
 	}
 
-	request, err := http.NewRequestWithContext(ctx, method, host+path, jsonReader)
+	request, err := retryablehttp.NewRequestWithContext(ctx, method, appendPath(host, path), jsonReader)
 	if err != nil {
 		return nil, err
 	}
@@ -88,29 +109,44 @@ func newRequest(ctx context.Context, method, host, path string, data interface{}
 	return request, nil
 }
 
-func parseError(resp *http.Response) error {
+func parseError(resp *http.Response, respErr error) error {
+	if resp == nil || respErr != nil {
+		return &APIError{"Internal API unreachable"}
+	}
+
 	if resp.StatusCode >= 200 && resp.StatusCode <= 399 {
 		return nil
 	}
-	defer resp.Body.Close()
+	defer func() { _ = resp.Body.Close() }()
 	parsedResponse := &ErrorResponse{}
 
 	if err := json.NewDecoder(resp.Body).Decode(parsedResponse); err != nil {
-		return fmt.Errorf("Internal API error (%v)", resp.StatusCode)
-	} else {
-		return fmt.Errorf(parsedResponse.Message)
+		return &APIError{fmt.Sprintf("Internal API error (%v)", resp.StatusCode)}
 	}
-
+	return &APIError{parsedResponse.Message}
 }
 
+// Get makes a GET request
 func (c *GitlabNetClient) Get(ctx context.Context, path string) (*http.Response, error) {
 	return c.DoRequest(ctx, http.MethodGet, normalizePath(path), nil)
 }
 
+// Post makes a POST request
 func (c *GitlabNetClient) Post(ctx context.Context, path string, data interface{}) (*http.Response, error) {
 	return c.DoRequest(ctx, http.MethodPost, normalizePath(path), data)
 }
 
+// Do executes a request
+func (c *GitlabNetClient) Do(request *http.Request) (*http.Response, error) {
+	response, respErr := c.httpClient.RetryableHTTP.HTTPClient.Do(request)
+	if err := parseError(response, respErr); err != nil {
+		return nil, err
+	}
+
+	return response, nil
+}
+
+// DoRequest executes a request with the given method, path, and data
 func (c *GitlabNetClient) DoRequest(ctx context.Context, method, path string, data interface{}) (*http.Response, error) {
 	request, err := newRequest(ctx, method, c.httpClient.Host, path, data)
 	if err != nil {
@@ -122,40 +158,25 @@ func (c *GitlabNetClient) DoRequest(ctx context.Context, method, path string, da
 		request.SetBasicAuth(user, password)
 	}
 
-	encodedSecret := base64.StdEncoding.EncodeToString([]byte(c.secret))
-	request.Header.Set(secretHeaderName, encodedSecret)
-
-	request.Header.Add("Content-Type", "application/json")
-	request.Header.Add("User-Agent", c.userAgent)
-	request.Close = true
-
-	start := time.Now()
-	response, err := c.httpClient.Do(request)
-	fields := log.Fields{
-		"method":      method,
-		"url":         request.URL.String(),
-		"duration_ms": time.Since(start) / time.Millisecond,
+	claims := jwt.RegisteredClaims{
+		Issuer:    jwtIssuer,
+		IssuedAt:  jwt.NewNumericDate(time.Now()),
+		ExpiresAt: jwt.NewNumericDate(time.Now().Add(jwtTTL)),
 	}
-	logger := log.WithContextFields(ctx, fields)
-
+	secretBytes := []byte(strings.TrimSpace(c.secret))
+	tokenString, err := jwt.NewWithClaims(jwt.SigningMethodHS256, claims).SignedString(secretBytes)
 	if err != nil {
-		logger.WithError(err).Error("Internal API unreachable")
-		return nil, fmt.Errorf("Internal API unreachable")
-	}
-
-	if response != nil {
-		logger = logger.WithField("status", response.StatusCode)
-	}
-	if err := parseError(response); err != nil {
-		logger.WithError(err).Error("Internal API error")
 		return nil, err
 	}
+	request.Header.Set(apiSecretHeaderName, tokenString)
 
-	if response.ContentLength >= 0 {
-		logger = logger.WithField("content_length_bytes", response.ContentLength)
-	}
+	request.Header.Add("Content-Type", "application/json")
+	request.Header.Add("User-Agent", c.userAgent)
 
-	logger.Info("Finished HTTP request")
+	response, respErr := c.httpClient.RetryableHTTP.Do(request)
+	if err := parseError(response, respErr); err != nil {
+		return nil, err
+	}
 
 	return response, nil
 }

client/httpclient.go

+// Package client provides functionality for interacting with HTTP clients
 package client
 
 import (
@@ -13,31 +14,34 @@ import (
 	"strings"
 	"time"
 
-	"gitlab.com/gitlab-org/labkit/correlation"
-	"gitlab.com/gitlab-org/labkit/log"
-	"gitlab.com/gitlab-org/labkit/tracing"
+	"github.com/hashicorp/go-retryablehttp"
 )
 
 const (
-	socketBaseUrl             = "http://unix"
+	socketBaseURL             = "http://unix"
 	unixSocketProtocol        = "http+unix://"
 	httpProtocol              = "http://"
 	httpsProtocol             = "https://"
 	defaultReadTimeoutSeconds = 300
+	defaultRetryWaitMinimum   = time.Second
+	defaultRetryWaitMaximum   = 15 * time.Second
+	defaultRetryMax           = 2
 )
 
-var (
-	ErrCafileNotFound = errors.New("cafile not found")
-)
+// ErrCafileNotFound indicates that the specified CA file was not found
+var ErrCafileNotFound = errors.New("cafile not found")
 
-type HttpClient struct {
-	*http.Client
-	Host string
+// HTTPClient provides an HTTP client with retry capabilities
+type HTTPClient struct {
+	RetryableHTTP *retryablehttp.Client
+	Host          string
 }
 
 type httpClientCfg struct {
-	keyPath, certPath string
-	caFile, caPath    string
+	keyPath, certPath          string
+	caFile, caPath             string
+	retryWaitMin, retryWaitMax time.Duration
+	retryMax                   int
 }
 
 func (hcc httpClientCfg) HaveCertAndKey() bool { return hcc.keyPath != "" && hcc.certPath != "" }
@@ -54,6 +58,15 @@ func WithClientCert(certPath, keyPath string) HTTPClientOpt {
 	}
 }
 
+// WithHTTPRetryOpts configures HTTP retry options for the HttpClient
+func WithHTTPRetryOpts(waitMin, waitMax time.Duration, maxAttempts int) HTTPClientOpt {
+	return func(hcc *httpClientCfg) {
+		hcc.retryWaitMin = waitMin
+		hcc.retryWaitMax = waitMax
+		hcc.retryMax = maxAttempts
+	}
+}
+
 func validateCaFile(filename string) error {
 	if filename == "" {
 		return nil
@@ -70,53 +83,50 @@ func validateCaFile(filename string) error {
 	return nil
 }
 
-// Deprecated: use NewHTTPClientWithOpts - https://gitlab.com/gitlab-org/gitlab-shell/-/issues/484
-func NewHTTPClient(gitlabURL, gitlabRelativeURLRoot, caFile, caPath string, selfSignedCert bool, readTimeoutSeconds uint64) *HttpClient {
-	c, err := NewHTTPClientWithOpts(gitlabURL, gitlabRelativeURLRoot, caFile, caPath, selfSignedCert, readTimeoutSeconds, nil)
-	if err != nil {
-		log.WithError(err).Error("new http client with opts")
+// NewHTTPClientWithOpts builds an HTTP client using the provided options
+func NewHTTPClientWithOpts(gitlabURL, gitlabRelativeURLRoot, caFile, caPath string, readTimeoutSeconds uint64, opts []HTTPClientOpt) (*HTTPClient, error) {
+	hcc := &httpClientCfg{
+		caFile:       caFile,
+		caPath:       caPath,
+		retryWaitMin: defaultRetryWaitMinimum,
+		retryWaitMax: defaultRetryWaitMaximum,
+		retryMax:     defaultRetryMax,
+	}
+
+	for _, opt := range opts {
+		opt(hcc)
 	}
-	return c
-}
 
-// NewHTTPClientWithOpts builds an HTTP client using the provided options
-func NewHTTPClientWithOpts(gitlabURL, gitlabRelativeURLRoot, caFile, caPath string, selfSignedCert bool, readTimeoutSeconds uint64, opts []HTTPClientOpt) (*HttpClient, error) {
 	var transport *http.Transport
 	var host string
 	var err error
-	if strings.HasPrefix(gitlabURL, unixSocketProtocol) {
+	switch {
+	case strings.HasPrefix(gitlabURL, unixSocketProtocol):
 		transport, host = buildSocketTransport(gitlabURL, gitlabRelativeURLRoot)
-	} else if strings.HasPrefix(gitlabURL, httpProtocol) {
-		transport, host = buildHttpTransport(gitlabURL)
-	} else if strings.HasPrefix(gitlabURL, httpsProtocol) {
+	case strings.HasPrefix(gitlabURL, httpProtocol):
+		transport, host = buildHTTPTransport(gitlabURL)
+	case strings.HasPrefix(gitlabURL, httpsProtocol):
 		err = validateCaFile(caFile)
 		if err != nil {
 			return nil, err
 		}
-
-		hcc := &httpClientCfg{
-			caFile: caFile,
-			caPath: caPath,
-		}
-
-		for _, opt := range opts {
-			opt(hcc)
-		}
-
-		transport, host, err = buildHttpsTransport(*hcc, selfSignedCert, gitlabURL)
+		transport, host, err = buildHTTPSTransport(*hcc, gitlabURL)
 		if err != nil {
 			return nil, err
 		}
-	} else {
+	default:
 		return nil, errors.New("unknown GitLab URL prefix")
 	}
 
-	c := &http.Client{
-		Transport: correlation.NewInstrumentedRoundTripper(tracing.NewRoundTripper(transport)),
-		Timeout:   readTimeout(readTimeoutSeconds),
-	}
+	c := retryablehttp.NewClient()
+	c.RetryMax = hcc.retryMax
+	c.RetryWaitMax = hcc.retryWaitMax
+	c.RetryWaitMin = hcc.retryWaitMin
+	c.Logger = nil
+	c.HTTPClient.Transport = NewTransport(transport)
+	c.HTTPClient.Timeout = readTimeout(readTimeoutSeconds)
 
-	client := &HttpClient{Client: c, Host: host}
+	client := &HTTPClient{RetryableHTTP: c, Host: host}
 
 	return client, nil
 }
@@ -131,7 +141,7 @@ func buildSocketTransport(gitlabURL, gitlabRelativeURLRoot string) (*http.Transp
 		},
 	}
 
-	host := socketBaseUrl
+	host := socketBaseURL
 	gitlabRelativeURLRoot = strings.Trim(gitlabRelativeURLRoot, "/")
 	if gitlabRelativeURLRoot != "" {
 		host = host + "/" + gitlabRelativeURLRoot
@@ -140,9 +150,8 @@ func buildSocketTransport(gitlabURL, gitlabRelativeURLRoot string) (*http.Transp
 	return transport, host
 }
 
-func buildHttpsTransport(hcc httpClientCfg, selfSignedCert bool, gitlabURL string) (*http.Transport, string, error) {
+func buildHTTPSTransport(hcc httpClientCfg, gitlabURL string) (*http.Transport, string, error) {
 	certPool, err := x509.SystemCertPool()
-
 	if err != nil {
 		certPool = x509.NewCertPool()
 	}
@@ -162,18 +171,14 @@ func buildHttpsTransport(hcc httpClientCfg, selfSignedCert bool, gitlabURL strin
 		}
 	}
 	tlsConfig := &tls.Config{
-		RootCAs: certPool,
-		// The self_signed_cert config setting is deprecated
-		// The field and its usage is going to be removed in
-		// https://gitlab.com/gitlab-org/gitlab-shell/-/issues/541
-		InsecureSkipVerify: selfSignedCert,
-		MinVersion:         tls.VersionTLS12,
+		RootCAs:    certPool,
+		MinVersion: tls.VersionTLS12,
 	}
 
 	if hcc.HaveCertAndKey() {
-		cert, err := tls.LoadX509KeyPair(hcc.certPath, hcc.keyPath)
-		if err != nil {
-			return nil, "", err
+		cert, loadErr := tls.LoadX509KeyPair(hcc.certPath, hcc.keyPath)
+		if loadErr != nil {
+			return nil, "", loadErr
 		}
 		tlsConfig.Certificates = []tls.Certificate{cert}
 	}
@@ -186,13 +191,13 @@ func buildHttpsTransport(hcc httpClientCfg, selfSignedCert bool, gitlabURL strin
 }
 
 func addCertToPool(certPool *x509.CertPool, fileName string) {
-	cert, err := os.ReadFile(fileName)
+	cert, err := os.ReadFile(filepath.Clean(fileName))
 	if err == nil {
 		certPool.AppendCertsFromPEM(cert)
 	}
 }
 
-func buildHttpTransport(gitlabURL string) (*http.Transport, string) {
+func buildHTTPTransport(gitlabURL string) (*http.Transport, string) {
 	return &http.Transport{}, gitlabURL
 }
 

client/httpclient_test.go

@@ -10,18 +10,19 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"gitlab.com/gitlab-org/gitlab-shell/client/testserver"
+	"gitlab.com/gitlab-org/gitlab-shell/v14/client/testserver"
 )
 
 func TestReadTimeout(t *testing.T) {
 	expectedSeconds := uint64(300)
 
-	client, err := NewHTTPClientWithOpts("http://localhost:3000", "", "", "", false, expectedSeconds, nil)
+	client, err := NewHTTPClientWithOpts("http://localhost:3000", "", "", "", expectedSeconds, nil)
 	require.NoError(t, err)
 
 	require.NotNil(t, client)
-	require.Equal(t, time.Duration(expectedSeconds)*time.Second, client.Client.Timeout)
+	require.Equal(t, time.Duration(expectedSeconds)*time.Second, client.RetryableHTTP.HTTPClient.Timeout)
 }
 
 const (
@@ -34,7 +35,7 @@ func TestBasicAuthSettings(t *testing.T) {
 		{
 			Path: "/api/v4/internal/get_endpoint",
 			Handler: func(w http.ResponseWriter, r *http.Request) {
-				require.Equal(t, http.MethodGet, r.Method)
+				assert.Equal(t, http.MethodGet, r.Method)
 
 				fmt.Fprint(w, r.Header.Get("Authorization"))
 			},
@@ -42,7 +43,7 @@ func TestBasicAuthSettings(t *testing.T) {
 		{
 			Path: "/api/v4/internal/post_endpoint",
 			Handler: func(w http.ResponseWriter, r *http.Request) {
-				require.Equal(t, http.MethodPost, r.Method)
+				assert.Equal(t, http.MethodPost, r.Method)
 
 				fmt.Fprint(w, r.Header.Get("Authorization"))
 			},
@@ -68,6 +69,7 @@ func testBasicAuthHeaders(t *testing.T, response *http.Response) {
 	require.NoError(t, err)
 
 	headerParts := strings.Split(string(responseBody), " ")
+	require.NotNil(t, headerParts)
 	require.Equal(t, "Basic", headerParts[0])
 
 	credentials, err := base64.StdEncoding.DecodeString(headerParts[1])
@@ -80,16 +82,17 @@ func TestEmptyBasicAuthSettings(t *testing.T) {
 	requests := []testserver.TestRequestHandler{
 		{
 			Path: "/api/v4/internal/empty_basic_auth",
-			Handler: func(w http.ResponseWriter, r *http.Request) {
-				require.Equal(t, "", r.Header.Get("Authorization"))
+			Handler: func(_ http.ResponseWriter, r *http.Request) {
+				assert.Equal(t, "", r.Header.Get("Authorization"))
 			},
 		},
 	}
 
 	client := setup(t, "", "", requests)
 
-	_, err := client.Get(context.Background(), "/empty_basic_auth")
+	resp, err := client.Get(context.Background(), "/empty_basic_auth")
 	require.NoError(t, err)
+	resp.Body.Close()
 }
 
 func TestRequestWithUserAgent(t *testing.T) {
@@ -97,33 +100,35 @@ func TestRequestWithUserAgent(t *testing.T) {
 	requests := []testserver.TestRequestHandler{
 		{
 			Path: "/api/v4/internal/default_user_agent",
-			Handler: func(w http.ResponseWriter, r *http.Request) {
-				require.Equal(t, defaultUserAgent, r.UserAgent())
+			Handler: func(_ http.ResponseWriter, r *http.Request) {
+				assert.Equal(t, defaultUserAgent, r.UserAgent())
 			},
 		},
 		{
 			Path: "/api/v4/internal/override_user_agent",
-			Handler: func(w http.ResponseWriter, r *http.Request) {
-				require.Equal(t, gitalyUserAgent, r.UserAgent())
+			Handler: func(_ http.ResponseWriter, r *http.Request) {
+				assert.Equal(t, gitalyUserAgent, r.UserAgent())
 			},
 		},
 	}
 
 	client := setup(t, "", "", requests)
 
-	_, err := client.Get(context.Background(), "/default_user_agent")
+	defaultUserAgentResp, err := client.Get(context.Background(), "/default_user_agent")
 	require.NoError(t, err)
 
 	client.SetUserAgent(gitalyUserAgent)
-	_, err = client.Get(context.Background(), "/override_user_agent")
+	overriddenUserAgentResp, err := client.Get(context.Background(), "/override_user_agent")
 	require.NoError(t, err)
 
+	defaultUserAgentResp.Body.Close()
+	overriddenUserAgentResp.Body.Close()
 }
 
 func setup(t *testing.T, username, password string, requests []testserver.TestRequestHandler) *GitlabNetClient {
-	url := testserver.StartHttpServer(t, requests)
+	url := testserver.StartHTTPServer(t, requests)
 
-	httpClient, err := NewHTTPClientWithOpts(url, "", "", "", false, 1, nil)
+	httpClient, err := NewHTTPClientWithOpts(url, "", "", "", 1, nil)
 	require.NoError(t, err)
 
 	client, err := NewGitlabNetClient(username, password, "", httpClient)

client/httpsclient_test.go

@@ -8,47 +8,48 @@ import (
 	"path"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"gitlab.com/gitlab-org/gitlab-shell/client/testserver"
-	"gitlab.com/gitlab-org/gitlab-shell/internal/testhelper"
+	"gitlab.com/gitlab-org/gitlab-shell/v14/client/testserver"
+	"gitlab.com/gitlab-org/gitlab-shell/v14/internal/testhelper"
 )
 
 //go:generate openssl req -newkey rsa:4096 -new -nodes -x509 -days 3650 -out ../internal/testhelper/testdata/testroot/certs/client/server.crt -keyout ../internal/testhelper/testdata/testroot/certs/client/key.pem -subj "/C=US/ST=California/L=San Francisco/O=GitLab/OU=GitLab-Shell/CN=localhost"
 func TestSuccessfulRequests(t *testing.T) {
+	testRoot := testhelper.PrepareTestRootDir(t)
+
 	testCases := []struct {
 		desc                                        string
 		caFile, caPath                              string
-		selfSigned                                  bool
 		clientCAPath, clientCertPath, clientKeyPath string // used for TLS client certs
 	}{
 		{
 			desc:   "Valid CaFile",
-			caFile: path.Join(testhelper.TestRoot, "certs/valid/server.crt"),
+			caFile: path.Join(testRoot, "certs/valid/server.crt"),
 		},
 		{
 			desc:   "Valid CaPath",
-			caPath: path.Join(testhelper.TestRoot, "certs/valid"),
-			caFile: path.Join(testhelper.TestRoot, "certs/valid/server.crt"),
+			caPath: path.Join(testRoot, "certs/valid"),
+			caFile: path.Join(testRoot, "certs/valid/server.crt"),
 		},
 		{
-			desc:       "Invalid cert with self signed cert option enabled",
-			caFile:     path.Join(testhelper.TestRoot, "certs/valid/server.crt"),
-			selfSigned: true,
+			desc:   "Invalid cert with self signed cert option enabled",
+			caFile: path.Join(testRoot, "certs/valid/server.crt"),
 		},
 		{
 			desc:   "Client certs with CA",
-			caFile: path.Join(testhelper.TestRoot, "certs/valid/server.crt"),
+			caFile: path.Join(testRoot, "certs/valid/server.crt"),
 			// Run the command "go generate httpsclient_test.go" to
 			// regenerate the following test fixtures:
-			clientCAPath:   path.Join(testhelper.TestRoot, "certs/client/server.crt"),
-			clientCertPath: path.Join(testhelper.TestRoot, "certs/client/server.crt"),
-			clientKeyPath:  path.Join(testhelper.TestRoot, "certs/client/key.pem"),
+			clientCAPath:   path.Join(testRoot, "certs/client/server.crt"),
+			clientCertPath: path.Join(testRoot, "certs/client/server.crt"),
+			clientKeyPath:  path.Join(testRoot, "certs/client/key.pem"),
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			client, err := setupWithRequests(t, tc.caFile, tc.caPath, tc.clientCAPath, tc.clientCertPath, tc.clientKeyPath, tc.selfSigned)
+			client, err := setupWithRequests(t, tc.caFile, tc.caPath, tc.clientCAPath, tc.clientCertPath, tc.clientKeyPath)
 			require.NoError(t, err)
 
 			response, err := client.Get(context.Background(), "/hello")
@@ -65,6 +66,8 @@ func TestSuccessfulRequests(t *testing.T) {
 }
 
 func TestFailedRequests(t *testing.T) {
+	testRoot := testhelper.PrepareTestRootDir(t)
+
 	testCases := []struct {
 		desc                   string
 		caFile                 string
@@ -74,17 +77,17 @@ func TestFailedRequests(t *testing.T) {
 	}{
 		{
 			desc:          "Invalid CaFile",
-			caFile:        path.Join(testhelper.TestRoot, "certs/invalid/server.crt"),
+			caFile:        path.Join(testRoot, "certs/invalid/server.crt"),
 			expectedError: "Internal API unreachable",
 		},
 		{
 			desc:                   "Missing CaFile",
-			caFile:                 path.Join(testhelper.TestRoot, "certs/invalid/missing.crt"),
+			caFile:                 path.Join(testRoot, "certs/invalid/missing.crt"),
 			expectedCaFileNotFound: true,
 		},
 		{
 			desc:          "Invalid CaPath",
-			caPath:        path.Join(testhelper.TestRoot, "certs/invalid"),
+			caPath:        path.Join(testRoot, "certs/invalid"),
 			expectedError: "Internal API unreachable",
 		},
 		{
@@ -95,7 +98,7 @@ func TestFailedRequests(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			client, err := setupWithRequests(t, tc.caFile, tc.caPath, "", "", "", false)
+			client, err := setupWithRequests(t, tc.caFile, tc.caPath, "", "", "")
 			if tc.expectedCaFileNotFound {
 				require.Error(t, err)
 				require.ErrorIs(t, err, ErrCafileNotFound)
@@ -109,28 +112,26 @@ func TestFailedRequests(t *testing.T) {
 	}
 }
 
-func setupWithRequests(t *testing.T, caFile, caPath, clientCAPath, clientCertPath, clientKeyPath string, selfSigned bool) (*GitlabNetClient, error) {
-	testhelper.PrepareTestRootDir(t)
-
+func setupWithRequests(t *testing.T, caFile, caPath, clientCAPath, clientCertPath, clientKeyPath string) (*GitlabNetClient, error) {
 	requests := []testserver.TestRequestHandler{
 		{
 			Path: "/api/v4/internal/hello",
 			Handler: func(w http.ResponseWriter, r *http.Request) {
-				require.Equal(t, http.MethodGet, r.Method)
+				assert.Equal(t, http.MethodGet, r.Method)
 
 				fmt.Fprint(w, "Hello")
 			},
 		},
 	}
 
-	url := testserver.StartHttpsServer(t, requests, clientCAPath)
+	url := testserver.StartHTTPSServer(t, requests, clientCAPath)
 
-	var opts []HTTPClientOpt
+	opts := defaultHttpOpts
 	if clientCertPath != "" && clientKeyPath != "" {
 		opts = append(opts, WithClientCert(clientCertPath, clientKeyPath))
 	}
 
-	httpClient, err := NewHTTPClientWithOpts(url, "", caFile, caPath, selfSigned, 1, opts)
+	httpClient, err := NewHTTPClientWithOpts(url, "", caFile, caPath, 1, opts)
 	if err != nil {
 		return nil, err
 	}

client/testserver/gitalyserver.go

@@ -10,8 +10,8 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
-	"gitlab.com/gitlab-org/gitaly/v14/client"
-	pb "gitlab.com/gitlab-org/gitaly/v14/proto/go/gitalypb"
+	"gitlab.com/gitlab-org/gitaly/v16/client"
+	pb "gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb"
 	"gitlab.com/gitlab-org/labkit/log"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials/insecure"
@@ -32,9 +32,7 @@ func (s *TestGitalyServer) SSHReceivePack(stream pb.SSHService_SSHReceivePackSer
 	s.ReceivedMD, _ = metadata.FromIncomingContext(stream.Context())
 
 	response := []byte("ReceivePack: " + req.GlId + " " + req.Repository.GlRepository)
-	stream.Send(&pb.SSHReceivePackResponse{Stdout: response})
-
-	return nil
+	return stream.Send(&pb.SSHReceivePackResponse{Stdout: response})
 }
 
 func (s *TestGitalyServer) SSHUploadPack(stream pb.SSHService_SSHUploadPackServer) error {
@@ -46,9 +44,7 @@ func (s *TestGitalyServer) SSHUploadPack(stream pb.SSHService_SSHUploadPackServe
 	s.ReceivedMD, _ = metadata.FromIncomingContext(stream.Context())
 
 	response := []byte("UploadPack: " + req.Repository.GlRepository)
-	stream.Send(&pb.SSHUploadPackResponse{Stdout: response})
-
-	return nil
+	return stream.Send(&pb.SSHUploadPackResponse{Stdout: response})
 }
 
 func (s *TestGitalyServer) SSHUploadPackWithSidechannel(ctx context.Context, req *pb.SSHUploadPackWithSidechannelRequest) (*pb.SSHUploadPackWithSidechannelResponse, error) {
@@ -80,35 +76,61 @@ func (s *TestGitalyServer) SSHUploadArchive(stream pb.SSHService_SSHUploadArchiv
 	s.ReceivedMD, _ = metadata.FromIncomingContext(stream.Context())
 
 	response := []byte("UploadArchive: " + req.Repository.GlRepository)
-	stream.Send(&pb.SSHUploadArchiveResponse{Stdout: response})
-
-	return nil
+	return stream.Send(&pb.SSHUploadArchiveResponse{Stdout: response})
 }
 
-func StartGitalyServer(t *testing.T) (string, *TestGitalyServer) {
+func StartGitalyServer(t *testing.T, network string) (string, *TestGitalyServer) {
 	t.Helper()
 
-	tempDir, _ := os.MkdirTemp("", "gitlab-shell-test-api")
-	gitalySocketPath := path.Join(tempDir, "gitaly.sock")
-	t.Cleanup(func() { os.RemoveAll(tempDir) })
-
-	err := os.MkdirAll(filepath.Dir(gitalySocketPath), 0700)
-	require.NoError(t, err)
+	switch network {
+	case "unix":
+		// We can't use t.TempDir() here because it will create a directory that
+		// far exceeds the 108 character limit which results in the socket failing
+		// to be created.
+		//
+		// See https://gitlab.com/gitlab-org/gitlab-shell/-/issues/696#note_1664726924
+		// for more detail.
+		tempDir, err := os.MkdirTemp("", "gitaly")
+		require.NoError(t, err)
+		t.Cleanup(func() { require.NoError(t, os.RemoveAll(tempDir)) })
+
+		gitalySocketPath := path.Join(tempDir, "gitaly.sock")
+		err = os.MkdirAll(filepath.Dir(gitalySocketPath), 0700)
+		require.NoError(t, err)
+
+		addr, testServer := doStartTestServer(t, "unix", gitalySocketPath)
+		return fmt.Sprintf("unix:%s", addr), testServer
+
+	case "tcp":
+		addr, testServer := doStartTestServer(t, "tcp", "127.0.0.1:0")
+		return fmt.Sprintf("tcp://%s", addr), testServer
+
+	case "dns":
+		addr, testServer := doStartTestServer(t, "tcp", "127.0.0.1:0")
+		// gRPC URL with DNS scheme follows this format: https://grpc.github.io/grpc/core/md_doc_naming.html
+		// When the authority is dropped, the URL have 3 splashes.
+		return fmt.Sprintf("dns:///%s", addr), testServer
+
+	default:
+		panic(fmt.Sprintf("Unsupported network %s", network))
+	}
+}
 
+func doStartTestServer(t *testing.T, network string, path string) (string, *TestGitalyServer) {
 	server := grpc.NewServer(
 		client.SidechannelServer(log.ContextLogger(context.Background()), insecure.NewCredentials()),
 	)
 
-	listener, err := net.Listen("unix", gitalySocketPath)
+	listener, err := net.Listen(network, path)
 	require.NoError(t, err)
 
 	testServer := TestGitalyServer{}
 	pb.RegisterSSHServiceServer(server, &testServer)
 
-	go server.Serve(listener)
-	t.Cleanup(func() { server.Stop() })
-
-	gitalySocketUrl := "unix:" + gitalySocketPath
+	go func() {
+		require.NoError(t, server.Serve(listener))
+	}()
+	t.Cleanup(func() { server.GracefulStop() })
 
-	return gitalySocketUrl, &testServer
+	return listener.Addr().String(), &testServer
 }