Merge branch '181-migrate-gitlab-shell-checks-fallback' into 'master'

Support falling back to ruby version of checkers

See merge request gitlab-org/gitlab-shell!318

Makefile

@@ -16,8 +16,14 @@ test_ruby:
 	# bin/gitlab-shell must exist and needs to be the Ruby version for
 	# rspec to be able to test.
 	cp bin/gitlab-shell-ruby bin/gitlab-shell
+	# bin/gitlab-shell-authorized-keys-check and bin/gitlab-shell-authorized-principals-check
+	# should link to ruby scripts for rspec to be able to test.
+	ln -sf ./gitlab-shell-authorized-keys-check-ruby bin/gitlab-shell-authorized-keys-check
+	ln -sf ./gitlab-shell-authorized-principals-check-ruby bin/gitlab-shell-authorized-principals-check
 	bundle exec rspec --color --tag '~go' --format d spec
 	rm -f bin/gitlab-shell
+	ln -sf ./gitlab-shell bin/gitlab-shell-authorized-keys-check
+	ln -sf ./gitlab-shell bin/gitlab-shell-authorized-principals-check
 
 test_golang:
 	support/go-test

bin/gitlab-shell-authorized-keys-check

-#!/usr/bin/env ruby
-
-#
-# GitLab shell authorized_keys helper. Query GitLab API to get the authorized
-# command for a given ssh key fingerprint
-#
-# Ex.
-#   bin/gitlab-shell-authorized-keys-check <username> <public-key>
-#
-# Returns
-#   command="/bin/gitlab-shell key-#",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-rsa AAAA...
-#
-# Expects to be called by the SSH daemon, via configuration like:
-#     AuthorizedKeysCommandUser git
-#     AuthorizedKeysCommand /bin/gitlab-shell-authorized-keys-check git %u %k
-
-abort "# Wrong number of arguments. #{ARGV.size}. Usage:
-#     gitlab-shell-authorized-keys-check <expected-username> <actual-username> <key>" unless ARGV.size == 3
-
-expected_username = ARGV[0]
-abort '# No username provided' if expected_username.nil? || expected_username == ''
-
-actual_username = ARGV[1]
-abort '# No username provided' if actual_username.nil? || actual_username == ''
-
-# Only check access if the requested username matches the configured username.
-# Normally, these would both be 'git', but it can be configured by the user
-exit 0 unless expected_username == actual_username
-
-key = ARGV[2]
-abort "# No key provided" if key.nil? || key == ''
-
-require_relative '../lib/gitlab_init'
-require_relative '../lib/gitlab_net'
-require_relative '../lib/gitlab_keys'
-
-authorized_key = GitlabNet.new.authorized_key(key)
-if authorized_key.nil?
-  puts "# No key was found for #{key}"
-else
-  puts GitlabKeys.key_line("key-#{authorized_key['id']}", authorized_key['key'])
-end

bin/gitlab-shell-authorized-keys-check

+./gitlab-shell
\ No newline at end of file

bin/gitlab-shell-authorized-keys-check-ruby

+#!/usr/bin/env ruby
+
+#
+# GitLab shell authorized_keys helper. Query GitLab API to get the authorized
+# command for a given ssh key fingerprint
+#
+# Ex.
+#   bin/gitlab-shell-authorized-keys-check <username> <public-key>
+#
+# Returns
+#   command="/bin/gitlab-shell key-#",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty ssh-rsa AAAA...
+#
+# Expects to be called by the SSH daemon, via configuration like:
+#     AuthorizedKeysCommandUser git
+#     AuthorizedKeysCommand /bin/gitlab-shell-authorized-keys-check git %u %k
+
+abort "# Wrong number of arguments. #{ARGV.size}. Usage:
+#     gitlab-shell-authorized-keys-check <expected-username> <actual-username> <key>" unless ARGV.size == 3
+
+expected_username = ARGV[0]
+abort '# No username provided' if expected_username.nil? || expected_username == ''
+
+actual_username = ARGV[1]
+abort '# No username provided' if actual_username.nil? || actual_username == ''
+
+# Only check access if the requested username matches the configured username.
+# Normally, these would both be 'git', but it can be configured by the user
+exit 0 unless expected_username == actual_username
+
+key = ARGV[2]
+abort "# No key provided" if key.nil? || key == ''
+
+require_relative '../lib/gitlab_init'
+require_relative '../lib/gitlab_net'
+require_relative '../lib/gitlab_keys'
+
+authorized_key = GitlabNet.new.authorized_key(key)
+if authorized_key.nil?
+  puts "# No key was found for #{key}"
+else
+  puts GitlabKeys.key_line("key-#{authorized_key['id']}", authorized_key['key'])
+end

bin/gitlab-shell-authorized-principals-check

-#!/usr/bin/env ruby
-
-#
-# GitLab shell authorized principals helper. Emits the same sort of
-# command="..." line as gitlab-shell-authorized-principals-check, with
-# the right options.
-#
-# Ex.
-#   bin/gitlab-shell-authorized-keys-check <key-id> <principal1> [<principal2>...]
-#
-# Returns one line per principal passed in, e.g.:
-#   command="/bin/gitlab-shell username-{KEY_ID}",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty {PRINCIPAL}
-#   [command="/bin/gitlab-shell username-{KEY_ID}",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty {PRINCIPAL2}]
-#
-# Expects to be called by the SSH daemon, via configuration like:
-#     AuthorizedPrincipalsCommandUser root
-#     AuthorizedPrincipalsCommand /bin/gitlab-shell-authorized-principals-check git %i sshUsers
-
-abort "# Wrong number of arguments. #{ARGV.size}. Usage:
-#     gitlab-shell-authorized-principals-check <key-id> <principal1> [<principal2>...]" unless ARGV.size >= 2
-
-key_id = ARGV[0]
-abort '# No key_id provided' if key_id.nil? || key_id == ''
-
-principals = ARGV[1..-1]
-principals.each { |principal|
-  abort '# An invalid principal was provided' if principal.nil? || principal == ''  
-}
-
-require_relative '../lib/gitlab_init'
-require_relative '../lib/gitlab_net'
-require_relative '../lib/gitlab_keys'
-
-principals.each { |principal|
-  puts GitlabKeys.principal_line("username-#{key_id}", principal.dup)
-}

bin/gitlab-shell-authorized-principals-check

+./gitlab-shell
\ No newline at end of file

bin/gitlab-shell-authorized-principals-check-ruby

+#!/usr/bin/env ruby
+
+#
+# GitLab shell authorized principals helper. Emits the same sort of
+# command="..." line as gitlab-shell-authorized-principals-check, with
+# the right options.
+#
+# Ex.
+#   bin/gitlab-shell-authorized-keys-check <key-id> <principal1> [<principal2>...]
+#
+# Returns one line per principal passed in, e.g.:
+#   command="/bin/gitlab-shell username-{KEY_ID}",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty {PRINCIPAL}
+#   [command="/bin/gitlab-shell username-{KEY_ID}",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty {PRINCIPAL2}]
+#
+# Expects to be called by the SSH daemon, via configuration like:
+#     AuthorizedPrincipalsCommandUser root
+#     AuthorizedPrincipalsCommand /bin/gitlab-shell-authorized-principals-check git %i sshUsers
+
+abort "# Wrong number of arguments. #{ARGV.size}. Usage:
+#     gitlab-shell-authorized-principals-check <key-id> <principal1> [<principal2>...]" unless ARGV.size >= 2
+
+key_id = ARGV[0]
+abort '# No key_id provided' if key_id.nil? || key_id == ''
+
+principals = ARGV[1..-1]
+principals.each { |principal|
+  abort '# An invalid principal was provided' if principal.nil? || principal == ''
+}
+
+require_relative '../lib/gitlab_init'
+require_relative '../lib/gitlab_net'
+require_relative '../lib/gitlab_keys'
+
+principals.each { |principal|
+  puts GitlabKeys.principal_line("username-#{key_id}", principal.dup)
+}

go/cmd/gitlab-shell/main.go

@@ -3,42 +3,13 @@ package main
 import (
 	"fmt"
 	"os"
-	"path/filepath"
 
 	"gitlab.com/gitlab-org/gitlab-shell/go/internal/command"
-	"gitlab.com/gitlab-org/gitlab-shell/go/internal/command/fallback"
 	"gitlab.com/gitlab-org/gitlab-shell/go/internal/command/readwriter"
 	"gitlab.com/gitlab-org/gitlab-shell/go/internal/config"
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/executable"
 )
 
-// findRootDir determines the root directory (and so, the location of the config
-// file) from os.Executable()
-func findRootDir() (string, error) {
-	if path := os.Getenv("GITLAB_SHELL_DIR"); path != "" {
-		return path, nil
-	}
-
-	path, err := os.Executable()
-	if err != nil {
-		return "", err
-	}
-
-	// Start: /opt/.../gitlab-shell/bin/gitlab-shell
-	// Ends:  /opt/.../gitlab-shell
-	return filepath.Dir(filepath.Dir(path)), nil
-}
-
-// rubyExec will never return. It either replaces the current process with a
-// Ruby interpreter, or outputs an error and kills the process.
-func execRuby(rootDir string, readWriter *readwriter.ReadWriter) {
-	cmd := &fallback.Command{RootDir: rootDir, Args: os.Args}
-
-	if err := cmd.Execute(); err != nil {
-		fmt.Fprintf(readWriter.ErrOut, "Failed to exec: %v\n", err)
-		os.Exit(1)
-	}
-}
-
 func main() {
 	readWriter := &readwriter.ReadWriter{
 		Out:    os.Stdout,
@@ -46,21 +17,19 @@ func main() {
 		ErrOut: os.Stderr,
 	}
 
-	rootDir, err := findRootDir()
+	executable, err := executable.New()
 	if err != nil {
-		fmt.Fprintln(readWriter.ErrOut, "Failed to determine root directory, exiting")
+		fmt.Fprintln(readWriter.ErrOut, "Failed to determine executable, exiting")
 		os.Exit(1)
 	}
 
-	// Fall back to Ruby in case of problems reading the config, but issue a
-	// warning as this isn't something we can sustain indefinitely
-	config, err := config.NewFromDir(rootDir)
+	config, err := config.NewFromDir(executable.RootDir)
 	if err != nil {
-		fmt.Fprintln(readWriter.ErrOut, "Failed to read config, falling back to gitlab-shell-ruby")
-		execRuby(rootDir, readWriter)
+		fmt.Fprintln(readWriter.ErrOut, "Failed to read config, exiting")
+		os.Exit(1)
 	}
 
-	cmd, err := command.New(os.Args, config, readWriter)
+	cmd, err := command.New(executable, os.Args[1:], config, readWriter)
 	if err != nil {
 		// For now this could happen if `SSH_CONNECTION` is not set on
 		// the environment

go/internal/command/command.go

@@ -11,29 +11,40 @@ import (
 	"gitlab.com/gitlab-org/gitlab-shell/go/internal/command/uploadarchive"
 	"gitlab.com/gitlab-org/gitlab-shell/go/internal/command/uploadpack"
 	"gitlab.com/gitlab-org/gitlab-shell/go/internal/config"
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/executable"
 )
 
 type Command interface {
 	Execute() error
 }
 
-func New(arguments []string, config *config.Config, readWriter *readwriter.ReadWriter) (Command, error) {
-	args, err := commandargs.Parse(arguments)
-
+func New(e *executable.Executable, arguments []string, config *config.Config, readWriter *readwriter.ReadWriter) (Command, error) {
+	args, err := commandargs.Parse(e, arguments)
 	if err != nil {
 		return nil, err
 	}
 
-	if config.FeatureEnabled(string(args.CommandType)) {
-		if cmd := buildCommand(args, config, readWriter); cmd != nil {
+	if cmd := buildCommand(e, args, config, readWriter); cmd != nil {
 		return cmd, nil
 	}
+
+	return &fallback.Command{Executable: e, RootDir: config.RootDir, Args: args}, nil
+}
+
+func buildCommand(e *executable.Executable, args commandargs.CommandArgs, config *config.Config, readWriter *readwriter.ReadWriter) Command {
+	switch e.Name {
+	case executable.GitlabShell:
+		return buildShellCommand(args.(*commandargs.Shell), config, readWriter)
 	}
 
-	return &fallback.Command{RootDir: config.RootDir, Args: arguments}, nil
+	return nil
 }
 
-func buildCommand(args *commandargs.CommandArgs, config *config.Config, readWriter *readwriter.ReadWriter) Command {
+func buildShellCommand(args *commandargs.Shell, config *config.Config, readWriter *readwriter.ReadWriter) Command {
+	if !config.FeatureEnabled(string(args.CommandType)) {
+		return nil
+	}
+
 	switch args.CommandType {
 	case commandargs.Discover:
 		return &discover.Command{Config: config, Args: args, ReadWriter: readWriter}

go/internal/command/command_test.go

@@ -13,18 +13,22 @@ import (
 	"gitlab.com/gitlab-org/gitlab-shell/go/internal/command/uploadarchive"
 	"gitlab.com/gitlab-org/gitlab-shell/go/internal/command/uploadpack"
 	"gitlab.com/gitlab-org/gitlab-shell/go/internal/config"
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/executable"
 	"gitlab.com/gitlab-org/gitlab-shell/go/internal/testhelper"
 )
 
 func TestNew(t *testing.T) {
 	testCases := []struct {
 		desc         string
+		executable   *executable.Executable
 		config       *config.Config
 		environment  map[string]string
+		arguments    []string
 		expectedType interface{}
 	}{
 		{
 			desc:       "it returns a Discover command if the feature is enabled",
+			executable: &executable.Executable{Name: executable.GitlabShell},
 			config: &config.Config{
 				GitlabUrl: "http+unix://gitlab.socket",
 				Migration: config.MigrationConfig{Enabled: true, Features: []string{"discover"}},
@@ -33,10 +37,12 @@ func TestNew(t *testing.T) {
 				"SSH_CONNECTION":       "1",
 				"SSH_ORIGINAL_COMMAND": "",
 			},
+			arguments:    []string{},
 			expectedType: &discover.Command{},
 		},
 		{
 			desc:       "it returns a Fallback command no feature is enabled",
+			executable: &executable.Executable{Name: executable.GitlabShell},
 			config: &config.Config{
 				GitlabUrl: "http+unix://gitlab.socket",
 				Migration: config.MigrationConfig{Enabled: false},
@@ -45,10 +51,12 @@ func TestNew(t *testing.T) {
 				"SSH_CONNECTION":       "1",
 				"SSH_ORIGINAL_COMMAND": "",
 			},
+			arguments:    []string{},
 			expectedType: &fallback.Command{},
 		},
 		{
 			desc:       "it returns a TwoFactorRecover command if the feature is enabled",
+			executable: &executable.Executable{Name: executable.GitlabShell},
 			config: &config.Config{
 				GitlabUrl: "http+unix://gitlab.socket",
 				Migration: config.MigrationConfig{Enabled: true, Features: []string{"2fa_recovery_codes"}},
@@ -57,10 +65,12 @@ func TestNew(t *testing.T) {
 				"SSH_CONNECTION":       "1",
 				"SSH_ORIGINAL_COMMAND": "2fa_recovery_codes",
 			},
+			arguments:    []string{},
 			expectedType: &twofactorrecover.Command{},
 		},
 		{
 			desc:       "it returns an LfsAuthenticate command if the feature is enabled",
+			executable: &executable.Executable{Name: executable.GitlabShell},
 			config: &config.Config{
 				GitlabUrl: "http+unix://gitlab.socket",
 				Migration: config.MigrationConfig{Enabled: true, Features: []string{"git-lfs-authenticate"}},
@@ -69,10 +79,12 @@ func TestNew(t *testing.T) {
 				"SSH_CONNECTION":       "1",
 				"SSH_ORIGINAL_COMMAND": "git-lfs-authenticate",
 			},
+			arguments:    []string{},
 			expectedType: &lfsauthenticate.Command{},
 		},
 		{
 			desc:       "it returns a ReceivePack command if the feature is enabled",
+			executable: &executable.Executable{Name: executable.GitlabShell},
 			config: &config.Config{
 				GitlabUrl: "http+unix://gitlab.socket",
 				Migration: config.MigrationConfig{Enabled: true, Features: []string{"git-receive-pack"}},
@@ -81,10 +93,12 @@ func TestNew(t *testing.T) {
 				"SSH_CONNECTION":       "1",
 				"SSH_ORIGINAL_COMMAND": "git-receive-pack",
 			},
+			arguments:    []string{},
 			expectedType: &receivepack.Command{},
 		},
 		{
-			desc: "it returns a UploadPack command if the feature is enabled",
+			desc:       "it returns an UploadPack command if the feature is enabled",
+			executable: &executable.Executable{Name: executable.GitlabShell},
 			config: &config.Config{
 				GitlabUrl: "http+unix://gitlab.socket",
 				Migration: config.MigrationConfig{Enabled: true, Features: []string{"git-upload-pack"}},
@@ -93,10 +107,12 @@ func TestNew(t *testing.T) {
 				"SSH_CONNECTION":       "1",
 				"SSH_ORIGINAL_COMMAND": "git-upload-pack",
 			},
+			arguments:    []string{},
 			expectedType: &uploadpack.Command{},
 		},
 		{
-			desc: "it returns a UploadArchive command if the feature is enabled",
+			desc:       "it returns an UploadArchive command if the feature is enabled",
+			executable: &executable.Executable{Name: executable.GitlabShell},
 			config: &config.Config{
 				GitlabUrl: "http+unix://gitlab.socket",
 				Migration: config.MigrationConfig{Enabled: true, Features: []string{"git-upload-archive"}},
@@ -105,10 +121,12 @@ func TestNew(t *testing.T) {
 				"SSH_CONNECTION":       "1",
 				"SSH_ORIGINAL_COMMAND": "git-upload-archive",
 			},
+			arguments:    []string{},
 			expectedType: &uploadarchive.Command{},
 		},
 		{
 			desc:       "it returns a Fallback command if the feature is unimplemented",
+			executable: &executable.Executable{Name: executable.GitlabShell},
 			config: &config.Config{
 				GitlabUrl: "http+unix://gitlab.socket",
 				Migration: config.MigrationConfig{Enabled: true, Features: []string{"git-unimplemented-feature"}},
@@ -117,6 +135,14 @@ func TestNew(t *testing.T) {
 				"SSH_CONNECTION":       "1",
 				"SSH_ORIGINAL_COMMAND": "git-unimplemented-feature",
 			},
+			arguments:    []string{},
+			expectedType: &fallback.Command{},
+		},
+		{
+			desc:         "it returns a Fallback command if executable is unknown",
+			executable:   &executable.Executable{Name: "unknown"},
+			config:       &config.Config{},
+			arguments:    []string{},
 			expectedType: &fallback.Command{},
 		},
 	}
@@ -126,7 +152,7 @@ func TestNew(t *testing.T) {
 			restoreEnv := testhelper.TempEnv(tc.environment)
 			defer restoreEnv()
 
-			command, err := New([]string{}, tc.config, nil)
+			command, err := New(tc.executable, tc.arguments, tc.config, nil)
 
 			require.NoError(t, err)
 			require.IsType(t, tc.expectedType, command)
@@ -135,12 +161,9 @@ func TestNew(t *testing.T) {
 }
 
 func TestFailingNew(t *testing.T) {
-	t.Run("It returns an error when SSH_CONNECTION is not set", func(t *testing.T) {
-		restoreEnv := testhelper.TempEnv(map[string]string{})
-		defer restoreEnv()
-
-		_, err := New([]string{}, &config.Config{}, nil)
+	t.Run("It returns an error parsing arguments failed", func(t *testing.T) {
+		_, err := New(&executable.Executable{Name: executable.GitlabShell}, []string{}, &config.Config{}, nil)
 
-		require.Error(t, err, "Only ssh allowed")
+		require.Error(t, err)
 	})
 }

go/internal/command/commandargs/command_args.go

 package commandargs
 
 import (
-	"errors"
-	"os"
-	"regexp"
-
-	"github.com/mattn/go-shellwords"
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/executable"
 )
 
 type CommandType string
 
-const (
-	Discover         CommandType = "discover"
-	TwoFactorRecover CommandType = "2fa_recovery_codes"
-	LfsAuthenticate  CommandType = "git-lfs-authenticate"
-	ReceivePack      CommandType = "git-receive-pack"
-	UploadPack       CommandType = "git-upload-pack"
-	UploadArchive    CommandType = "git-upload-archive"
-)
-
-var (
-	whoKeyRegex      = regexp.MustCompile(`\bkey-(?P<keyid>\d+)\b`)
-	whoUsernameRegex = regexp.MustCompile(`\busername-(?P<username>\S+)\b`)
-)
-
-type CommandArgs struct {
-	GitlabUsername string
-	GitlabKeyId    string
-	SshArgs        []string
-	CommandType    CommandType
-}
-
-func Parse(arguments []string) (*CommandArgs, error) {
-	if sshConnection := os.Getenv("SSH_CONNECTION"); sshConnection == "" {
-		return nil, errors.New("Only ssh allowed")
-	}
-
-	args := &CommandArgs{}
-	args.parseWho(arguments)
-
-	if err := args.parseCommand(os.Getenv("SSH_ORIGINAL_COMMAND")); err != nil {
-		return nil, errors.New("Invalid ssh command")
-	}
-	args.defineCommandType()
-
-	return args, nil
-}
-
-func (c *CommandArgs) parseWho(arguments []string) {
-	for _, argument := range arguments {
-		if keyId := tryParseKeyId(argument); keyId != "" {
-			c.GitlabKeyId = keyId
-			break
-		}
-
-		if username := tryParseUsername(argument); username != "" {
-			c.GitlabUsername = username
-			break
-		}
-	}
-}
-
-func tryParseKeyId(argument string) string {
-	matchInfo := whoKeyRegex.FindStringSubmatch(argument)
-	if len(matchInfo) == 2 {
-		// The first element is the full matched string
-		// The second element is the named `keyid`
-		return matchInfo[1]
-	}
-
-	return ""
+type CommandArgs interface {
+	Parse() error
+	GetArguments() []string
 }
 
-func tryParseUsername(argument string) string {
-	matchInfo := whoUsernameRegex.FindStringSubmatch(argument)
-	if len(matchInfo) == 2 {
-		// The first element is the full matched string
-		// The second element is the named `username`
-		return matchInfo[1]
-	}
-
-	return ""
-}
+func Parse(e *executable.Executable, arguments []string) (CommandArgs, error) {
+	var args CommandArgs = &GenericArgs{Arguments: arguments}
 
-func (c *CommandArgs) parseCommand(commandString string) error {
-	args, err := shellwords.Parse(commandString)
-	if err != nil {
-		return err
+	switch e.Name {
+	case executable.GitlabShell:
+		args = &Shell{Arguments: arguments}
 	}
 
-	// Handle Git for Windows 2.14 using "git upload-pack" instead of git-upload-pack
-	if len(args) > 1 && args[0] == "git" {
-		command := args[0] + "-" + args[1]
-		commandArgs := args[2:]
-
-		args = append([]string{command}, commandArgs...)
+	if err := args.Parse(); err != nil {
+		return nil, err
 	}
 
-	c.SshArgs = args
-
-	return nil
-}
-
-func (c *CommandArgs) defineCommandType() {
-	if len(c.SshArgs) == 0 {
-		c.CommandType = Discover
-	} else {
-		c.CommandType = CommandType(c.SshArgs[0])
-	}
+	return args, nil
 }

go/internal/command/commandargs/command_args_test.go

@@ -3,100 +3,127 @@ package commandargs
 import (
 	"testing"
 
-	"github.com/stretchr/testify/require"
-
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/executable"
 	"gitlab.com/gitlab-org/gitlab-shell/go/internal/testhelper"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestParseSuccess(t *testing.T) {
 	testCases := []struct {
 		desc         string
-		arguments    []string
+		executable   *executable.Executable
 		environment  map[string]string
-		expectedArgs *CommandArgs
+		arguments    []string
+		expectedArgs CommandArgs
 	}{
 		// Setting the used env variables for every case to ensure we're
 		// not using anything set in the original env.
 		{
 			desc:       "It sets discover as the command when the command string was empty",
+			executable: &executable.Executable{Name: executable.GitlabShell},
 			environment: map[string]string{
 				"SSH_CONNECTION":       "1",
 				"SSH_ORIGINAL_COMMAND": "",
 			},
-			expectedArgs: &CommandArgs{SshArgs: []string{}, CommandType: Discover},
+			arguments:    []string{},
+			expectedArgs: &Shell{Arguments: []string{}, SshArgs: []string{}, CommandType: Discover},
 		},
 		{
 			desc:       "It finds the key id in any passed arguments",
+			executable: &executable.Executable{Name: executable.GitlabShell},
 			environment: map[string]string{
 				"SSH_CONNECTION":       "1",
 				"SSH_ORIGINAL_COMMAND": "",
 			},
 			arguments:    []string{"hello", "key-123"},
-			expectedArgs: &CommandArgs{SshArgs: []string{}, CommandType: Discover, GitlabKeyId: "123"},
+			expectedArgs: &Shell{Arguments: []string{"hello", "key-123"}, SshArgs: []string{}, CommandType: Discover, GitlabKeyId: "123"},
 		}, {
 			desc:       "It finds the username in any passed arguments",
+			executable: &executable.Executable{Name: executable.GitlabShell},
 			environment: map[string]string{
 				"SSH_CONNECTION":       "1",
 				"SSH_ORIGINAL_COMMAND": "",
 			},
 			arguments:    []string{"hello", "username-jane-doe"},
-			expectedArgs: &CommandArgs{SshArgs: []string{}, CommandType: Discover, GitlabUsername: "jane-doe"},
+			expectedArgs: &Shell{Arguments: []string{"hello", "username-jane-doe"}, SshArgs: []string{}, CommandType: Discover, GitlabUsername: "jane-doe"},
 		}, {
 			desc:       "It parses 2fa_recovery_codes command",
+			executable: &executable.Executable{Name: executable.GitlabShell},
 			environment: map[string]string{
 				"SSH_CONNECTION":       "1",
 				"SSH_ORIGINAL_COMMAND": "2fa_recovery_codes",
 			},
-			expectedArgs: &CommandArgs{SshArgs: []string{"2fa_recovery_codes"}, CommandType: TwoFactorRecover},
+			arguments:    []string{},
+			expectedArgs: &Shell{Arguments: []string{}, SshArgs: []string{"2fa_recovery_codes"}, CommandType: TwoFactorRecover},
 		}, {
 			desc:       "It parses git-receive-pack command",
+			executable: &executable.Executable{Name: executable.GitlabShell},
 			environment: map[string]string{
 				"SSH_CONNECTION":       "1",
 				"SSH_ORIGINAL_COMMAND": "git-receive-pack group/repo",
 			},
-			expectedArgs: &CommandArgs{SshArgs: []string{"git-receive-pack", "group/repo"}, CommandType: ReceivePack},
+			arguments:    []string{},
+			expectedArgs: &Shell{Arguments: []string{}, SshArgs: []string{"git-receive-pack", "group/repo"}, CommandType: ReceivePack},
 		}, {
 			desc:       "It parses git-receive-pack command and a project with single quotes",
+			executable: &executable.Executable{Name: executable.GitlabShell},
 			environment: map[string]string{
 				"SSH_CONNECTION":       "1",
 				"SSH_ORIGINAL_COMMAND": "git receive-pack 'group/repo'",
 			},
-			expectedArgs: &CommandArgs{SshArgs: []string{"git-receive-pack", "group/repo"}, CommandType: ReceivePack},
+			arguments:    []string{},
+			expectedArgs: &Shell{Arguments: []string{}, SshArgs: []string{"git-receive-pack", "group/repo"}, CommandType: ReceivePack},
 		}, {
 			desc:       `It parses "git receive-pack" command`,
+			executable: &executable.Executable{Name: executable.GitlabShell},
 			environment: map[string]string{
 				"SSH_CONNECTION":       "1",
 				"SSH_ORIGINAL_COMMAND": `git receive-pack "group/repo"`,
 			},
-			expectedArgs: &CommandArgs{SshArgs: []string{"git-receive-pack", "group/repo"}, CommandType: ReceivePack},
+			arguments:    []string{},
+			expectedArgs: &Shell{Arguments: []string{}, SshArgs: []string{"git-receive-pack", "group/repo"}, CommandType: ReceivePack},
 		}, {
 			desc:       `It parses a command followed by control characters`,
+			executable: &executable.Executable{Name: executable.GitlabShell},
 			environment: map[string]string{
 				"SSH_CONNECTION":       "1",
 				"SSH_ORIGINAL_COMMAND": `git-receive-pack group/repo; any command`,
 			},
-			expectedArgs: &CommandArgs{SshArgs: []string{"git-receive-pack", "group/repo"}, CommandType: ReceivePack},
+			arguments:    []string{},
+			expectedArgs: &Shell{Arguments: []string{}, SshArgs: []string{"git-receive-pack", "group/repo"}, CommandType: ReceivePack},
 		}, {
 			desc:       "It parses git-upload-pack command",
+			executable: &executable.Executable{Name: executable.GitlabShell},
 			environment: map[string]string{
 				"SSH_CONNECTION":       "1",
 				"SSH_ORIGINAL_COMMAND": `git upload-pack "group/repo"`,
 			},
-			expectedArgs: &CommandArgs{SshArgs: []string{"git-upload-pack", "group/repo"}, CommandType: UploadPack},
+			arguments:    []string{},
+			expectedArgs: &Shell{Arguments: []string{}, SshArgs: []string{"git-upload-pack", "group/repo"}, CommandType: UploadPack},
 		}, {
 			desc:       "It parses git-upload-archive command",
+			executable: &executable.Executable{Name: executable.GitlabShell},
 			environment: map[string]string{
 				"SSH_CONNECTION":       "1",
 				"SSH_ORIGINAL_COMMAND": "git-upload-archive 'group/repo'",
 			},
-			expectedArgs: &CommandArgs{SshArgs: []string{"git-upload-archive", "group/repo"}, CommandType: UploadArchive},
+			arguments:    []string{},
+			expectedArgs: &Shell{Arguments: []string{}, SshArgs: []string{"git-upload-archive", "group/repo"}, CommandType: UploadArchive},
 		}, {
 			desc:       "It parses git-lfs-authenticate command",
+			executable: &executable.Executable{Name: executable.GitlabShell},
 			environment: map[string]string{
 				"SSH_CONNECTION":       "1",
 				"SSH_ORIGINAL_COMMAND": "git-lfs-authenticate 'group/repo' download",
 			},
-			expectedArgs: &CommandArgs{SshArgs: []string{"git-lfs-authenticate", "group/repo", "download"}, CommandType: LfsAuthenticate},
+			arguments:    []string{},
+			expectedArgs: &Shell{Arguments: []string{}, SshArgs: []string{"git-lfs-authenticate", "group/repo", "download"}, CommandType: LfsAuthenticate},
+		}, {
+			desc:         "Unknown executable",
+			executable:   &executable.Executable{Name: "unknown"},
+			arguments:    []string{},
+			expectedArgs: &GenericArgs{Arguments: []string{}},
 		},
 	}
 
@@ -105,7 +132,7 @@ func TestParseSuccess(t *testing.T) {
 			restoreEnv := testhelper.TempEnv(tc.environment)
 			defer restoreEnv()
 
-			result, err := Parse(tc.arguments)
+			result, err := Parse(tc.executable, tc.arguments)
 
 			require.NoError(t, err)
 			require.Equal(t, tc.expectedArgs, result)
@@ -114,22 +141,39 @@ func TestParseSuccess(t *testing.T) {
 }
 
 func TestParseFailure(t *testing.T) {
-	t.Run("It fails if SSH connection is not set", func(t *testing.T) {
-		_, err := Parse([]string{})
-
-		require.Error(t, err, "Only ssh allowed")
-	})
-
-	t.Run("It fails if SSH command is invalid", func(t *testing.T) {
-		environment := map[string]string{
+	testCases := []struct {
+		desc          string
+		executable    *executable.Executable
+		environment   map[string]string
+		arguments     []string
+		expectedError string
+	}{
+		{
+			desc:          "It fails if SSH connection is not set",
+			executable:    &executable.Executable{Name: executable.GitlabShell},
+			arguments:     []string{},
+			expectedError: "Only SSH allowed",
+		},
+		{
+			desc:       "It fails if SSH command is invalid",
+			executable: &executable.Executable{Name: executable.GitlabShell},
+			environment: map[string]string{
 				"SSH_CONNECTION":       "1",
 				"SSH_ORIGINAL_COMMAND": `git receive-pack "`,
+			},
+			arguments:     []string{},
+			expectedError: "Invalid SSH allowed",
+		},
 	}
-		restoreEnv := testhelper.TempEnv(environment)
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			restoreEnv := testhelper.TempEnv(tc.environment)
 			defer restoreEnv()
 
-		_, err := Parse([]string{})
+			_, err := Parse(tc.executable, tc.arguments)
 
-		require.Error(t, err, "Invalid ssh command")
+			require.Error(t, err, tc.expectedError)
 		})
+	}
 }

go/internal/command/commandargs/generic_args.go

+package commandargs
+
+type GenericArgs struct {
+	Arguments []string
+}
+
+func (b *GenericArgs) Parse() error {
+	// Do nothing
+	return nil
+}
+
+func (b *GenericArgs) GetArguments() []string {
+	return b.Arguments
+}

go/internal/command/commandargs/shell.go

+package commandargs
+
+import (
+	"errors"
+	"os"
+	"regexp"
+
+	"github.com/mattn/go-shellwords"
+)
+
+const (
+	Discover         CommandType = "discover"
+	TwoFactorRecover CommandType = "2fa_recovery_codes"
+	LfsAuthenticate  CommandType = "git-lfs-authenticate"
+	ReceivePack      CommandType = "git-receive-pack"
+	UploadPack       CommandType = "git-upload-pack"
+	UploadArchive    CommandType = "git-upload-archive"
+)
+
+var (
+	whoKeyRegex      = regexp.MustCompile(`\bkey-(?P<keyid>\d+)\b`)
+	whoUsernameRegex = regexp.MustCompile(`\busername-(?P<username>\S+)\b`)
+)
+
+type Shell struct {
+	Arguments      []string
+	GitlabUsername string
+	GitlabKeyId    string
+	SshArgs        []string
+	CommandType    CommandType
+}
+
+func (s *Shell) Parse() error {
+	if err := s.validate(); err != nil {
+		return err
+	}
+
+	s.parseWho()
+	s.defineCommandType()
+
+	return nil
+}
+
+func (s *Shell) GetArguments() []string {
+	return s.Arguments
+}
+
+func (s *Shell) validate() error {
+	if !s.isSshConnection() {
+		return errors.New("Only SSH allowed")
+	}
+
+	if !s.isValidSshCommand() {
+		return errors.New("Invalid SSH command")
+	}
+
+	return nil
+}
+
+func (s *Shell) isSshConnection() bool {
+	ok := os.Getenv("SSH_CONNECTION")
+	return ok != ""
+}
+
+func (s *Shell) isValidSshCommand() bool {
+	err := s.parseCommand(os.Getenv("SSH_ORIGINAL_COMMAND"))
+	return err == nil
+}
+
+func (s *Shell) parseWho() {
+	for _, argument := range s.Arguments {
+		if keyId := tryParseKeyId(argument); keyId != "" {
+			s.GitlabKeyId = keyId
+			break
+		}
+
+		if username := tryParseUsername(argument); username != "" {
+			s.GitlabUsername = username
+			break
+		}
+	}
+}
+
+func tryParseKeyId(argument string) string {
+	matchInfo := whoKeyRegex.FindStringSubmatch(argument)
+	if len(matchInfo) == 2 {
+		// The first element is the full matched string
+		// The second element is the named `keyid`
+		return matchInfo[1]
+	}
+
+	return ""
+}
+
+func tryParseUsername(argument string) string {
+	matchInfo := whoUsernameRegex.FindStringSubmatch(argument)
+	if len(matchInfo) == 2 {
+		// The first element is the full matched string
+		// The second element is the named `username`
+		return matchInfo[1]
+	}
+
+	return ""
+}
+
+func (s *Shell) parseCommand(commandString string) error {
+	args, err := shellwords.Parse(commandString)
+	if err != nil {
+		return err
+	}
+
+	// Handle Git for Windows 2.14 using "git upload-pack" instead of git-upload-pack
+	if len(args) > 1 && args[0] == "git" {
+		command := args[0] + "-" + args[1]
+		commandArgs := args[2:]
+
+		args = append([]string{command}, commandArgs...)
+	}
+
+	s.SshArgs = args
+
+	return nil
+}
+
+func (s *Shell) defineCommandType() {
+	if len(s.SshArgs) == 0 {
+		s.CommandType = Discover
+	} else {
+		s.CommandType = CommandType(s.SshArgs[0])
+	}
+}

go/internal/command/discover/discover.go

@@ -11,7 +11,7 @@ import (
 
 type Command struct {
 	Config     *config.Config
-	Args       *commandargs.CommandArgs
+	Args       *commandargs.Shell
 	ReadWriter *readwriter.ReadWriter
 }
 

go/internal/command/discover/discover_test.go

@@ -49,27 +49,27 @@ func TestExecute(t *testing.T) {
 
 	testCases := []struct {
 		desc           string
-		arguments      *commandargs.CommandArgs
+		arguments      *commandargs.Shell
 		expectedOutput string
 	}{
 		{
 			desc:           "With a known username",
-			arguments:      &commandargs.CommandArgs{GitlabUsername: "alex-doe"},
+			arguments:      &commandargs.Shell{GitlabUsername: "alex-doe"},
 			expectedOutput: "Welcome to GitLab, @alex-doe!\n",
 		},
 		{
 			desc:           "With a known key id",
-			arguments:      &commandargs.CommandArgs{GitlabKeyId: "1"},
+			arguments:      &commandargs.Shell{GitlabKeyId: "1"},
 			expectedOutput: "Welcome to GitLab, @alex-doe!\n",
 		},
 		{
 			desc:           "With an unknown key",
-			arguments:      &commandargs.CommandArgs{GitlabKeyId: "-1"},
+			arguments:      &commandargs.Shell{GitlabKeyId: "-1"},
 			expectedOutput: "Welcome to GitLab, Anonymous!\n",
 		},
 		{
 			desc:           "With an unknown username",
-			arguments:      &commandargs.CommandArgs{GitlabUsername: "unknown"},
+			arguments:      &commandargs.Shell{GitlabUsername: "unknown"},
 			expectedOutput: "Welcome to GitLab, Anonymous!\n",
 		},
 	}
@@ -97,22 +97,22 @@ func TestFailingExecute(t *testing.T) {
 
 	testCases := []struct {
 		desc          string
-		arguments     *commandargs.CommandArgs
+		arguments     *commandargs.Shell
 		expectedError string
 	}{
 		{
 			desc:          "With missing arguments",
-			arguments:     &commandargs.CommandArgs{},
+			arguments:     &commandargs.Shell{},
 			expectedError: "Failed to get username: who='' is invalid",
 		},
 		{
 			desc:          "When the API returns an error",
-			arguments:     &commandargs.CommandArgs{GitlabUsername: "broken_message"},
+			arguments:     &commandargs.Shell{GitlabUsername: "broken_message"},
 			expectedError: "Failed to get username: Forbidden!",
 		},
 		{
 			desc:          "When the API fails",
-			arguments:     &commandargs.CommandArgs{GitlabUsername: "broken"},
+			arguments:     &commandargs.Shell{GitlabUsername: "broken"},
 			expectedError: "Failed to get username: Internal API error (500)",
 		},
 	}

go/internal/command/fallback/fallback.go

 package fallback
 
 import (
+	"errors"
+	"fmt"
 	"os"
 	"path/filepath"
 	"syscall"
+
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/command/commandargs"
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/executable"
 )
 
 type Command struct {
+	Executable *executable.Executable
 	RootDir    string
-	Args    []string
+	Args       commandargs.CommandArgs
 }
 
 var (
 	// execFunc is overridden in tests
 	execFunc  = syscall.Exec
-)
-
-const (
-	RubyProgram = "gitlab-shell-ruby"
+	whitelist = []string{
+		executable.GitlabShell,
+		executable.AuthorizedKeysCheck,
+		executable.AuthorizedPrincipalsCheck,
+	}
 )
 
 func (c *Command) Execute() error {
-	rubyCmd := filepath.Join(c.RootDir, "bin", RubyProgram)
+	if !c.isWhitelisted() {
+		return errors.New("Failed to execute unknown executable")
+	}
+
+	rubyCmd := c.fallbackProgram()
 
 	// Ensure rubyArgs[0] is the full path to gitlab-shell-ruby
-	rubyArgs := append([]string{rubyCmd}, c.Args[1:]...)
+	rubyArgs := append([]string{rubyCmd}, c.Args.GetArguments()...)
 
 	return execFunc(rubyCmd, rubyArgs, os.Environ())
 }
+
+func (c *Command) isWhitelisted() bool {
+	for _, item := range whitelist {
+		if c.Executable.Name == item {
+			return true
+		}
+	}
+
+	return false
+}
+
+func (c *Command) fallbackProgram() string {
+	fileName := fmt.Sprintf("%s-ruby", c.Executable.Name)
+
+	return filepath.Join(c.RootDir, "bin", fileName)
+}

go/internal/command/fallback/fallback_test.go

@@ -6,6 +6,9 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
+
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/command/commandargs"
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/executable"
 )
 
 type fakeExec struct {
@@ -19,7 +22,7 @@ type fakeExec struct {
 }
 
 var (
-	fakeArgs = []string{"./test", "foo", "bar"}
+	fakeArgs = &commandargs.GenericArgs{Arguments: []string{"foo", "bar"}}
 )
 
 func (f *fakeExec) Exec(filename string, args []string, env []string) error {
@@ -42,7 +45,7 @@ func (f *fakeExec) Cleanup() {
 }
 
 func TestExecuteExecsCommandSuccesfully(t *testing.T) {
-	cmd := &Command{RootDir: "/tmp", Args: fakeArgs}
+	cmd := &Command{Executable: &executable.Executable{Name: executable.GitlabShell}, RootDir: "/tmp", Args: fakeArgs}
 
 	// Override the exec func
 	fake := &fakeExec{}
@@ -56,8 +59,14 @@ func TestExecuteExecsCommandSuccesfully(t *testing.T) {
 	require.Equal(t, fake.Env, os.Environ())
 }
 
+func TestExecuteExecsUnknownExecutable(t *testing.T) {
+	cmd := &Command{Executable: &executable.Executable{Name: "unknown"}, RootDir: "/test"}
+
+	require.Error(t, cmd.Execute())
+}
+
 func TestExecuteExecsCommandOnError(t *testing.T) {
-	cmd := &Command{RootDir: "/test", Args: fakeArgs}
+	cmd := &Command{Executable: &executable.Executable{Name: executable.GitlabShell}, RootDir: "/test", Args: fakeArgs}
 
 	// Override the exec func
 	fake := &fakeExec{Error: errors.New("Test error")}
@@ -69,7 +78,7 @@ func TestExecuteExecsCommandOnError(t *testing.T) {
 }
 
 func TestExecuteGivenNonexistentCommand(t *testing.T) {
-	cmd := &Command{RootDir: "/tmp/does/not/exist", Args: fakeArgs}
+	cmd := &Command{Executable: &executable.Executable{Name: executable.GitlabShell}, RootDir: "/tmp/does/not/exist", Args: fakeArgs}
 
 	require.Error(t, cmd.Execute())
 }

go/internal/command/lfsauthenticate/lfsauthenticate.go

@@ -20,7 +20,7 @@ const (
 
 type Command struct {
 	Config     *config.Config
-	Args       *commandargs.CommandArgs
+	Args       *commandargs.Shell
 	ReadWriter *readwriter.ReadWriter
 }
 

go/internal/command/lfsauthenticate/lfsauthenticate_test.go

@@ -25,22 +25,22 @@ func TestFailedRequests(t *testing.T) {
 
 	testCases := []struct {
 		desc           string
-		arguments      *commandargs.CommandArgs
+		arguments      *commandargs.Shell
 		expectedOutput string
 	}{
 		{
 			desc:           "With missing arguments",
-			arguments:      &commandargs.CommandArgs{},
+			arguments:      &commandargs.Shell{},
 			expectedOutput: "> GitLab: Disallowed command",
 		},
 		{
 			desc:           "With disallowed command",
-			arguments:      &commandargs.CommandArgs{GitlabKeyId: "1", SshArgs: []string{"git-lfs-authenticate", "group/repo", "unknown"}},
+			arguments:      &commandargs.Shell{GitlabKeyId: "1", SshArgs: []string{"git-lfs-authenticate", "group/repo", "unknown"}},
 			expectedOutput: "> GitLab: Disallowed command",
 		},
 		{
 			desc:           "With disallowed user",
-			arguments:      &commandargs.CommandArgs{GitlabKeyId: "disallowed", SshArgs: []string{"git-lfs-authenticate", "group/repo", "download"}},
+			arguments:      &commandargs.Shell{GitlabKeyId: "disallowed", SshArgs: []string{"git-lfs-authenticate", "group/repo", "download"}},
 			expectedOutput: "Disallowed by API call",
 		},
 	}
@@ -140,7 +140,7 @@ func TestLfsAuthenticateRequests(t *testing.T) {
 			output := &bytes.Buffer{}
 			cmd := &Command{
 				Config:     &config.Config{GitlabUrl: url},
-				Args:       &commandargs.CommandArgs{GitlabUsername: tc.username, SshArgs: []string{"git-lfs-authenticate", "group/repo", "upload"}},
+				Args:       &commandargs.Shell{GitlabUsername: tc.username, SshArgs: []string{"git-lfs-authenticate", "group/repo", "upload"}},
 				ReadWriter: &readwriter.ReadWriter{ErrOut: output, Out: output},
 			}
 

go/internal/command/receivepack/customaction_test.go

@@ -92,7 +92,7 @@ func TestCustomReceivePack(t *testing.T) {
 
 	cmd := &Command{
 		Config:     &config.Config{GitlabUrl: url},
-		Args:       &commandargs.CommandArgs{GitlabKeyId: keyId, CommandType: commandargs.ReceivePack, SshArgs: []string{"git-receive-pack", repo}},
+		Args:       &commandargs.Shell{GitlabKeyId: keyId, CommandType: commandargs.ReceivePack, SshArgs: []string{"git-receive-pack", repo}},
 		ReadWriter: &readwriter.ReadWriter{ErrOut: errBuf, Out: outBuf, In: input},
 	}
 

go/internal/command/receivepack/gitalycall_test.go

@@ -29,7 +29,7 @@ func TestReceivePack(t *testing.T) {
 
 	cmd := &Command{
 		Config:     &config.Config{GitlabUrl: url},
-		Args:       &commandargs.CommandArgs{GitlabKeyId: userId, CommandType: commandargs.ReceivePack, SshArgs: []string{"git-receive-pack", repo}},
+		Args:       &commandargs.Shell{GitlabKeyId: userId, CommandType: commandargs.ReceivePack, SshArgs: []string{"git-receive-pack", repo}},
 		ReadWriter: &readwriter.ReadWriter{ErrOut: output, Out: output, In: input},
 	}
 

go/internal/command/receivepack/receivepack.go

@@ -10,7 +10,7 @@ import (
 
 type Command struct {
 	Config     *config.Config
-	Args       *commandargs.CommandArgs
+	Args       *commandargs.Shell
 	ReadWriter *readwriter.ReadWriter
 }
 

go/internal/command/receivepack/receivepack_test.go

@@ -23,7 +23,7 @@ func TestForbiddenAccess(t *testing.T) {
 
 	cmd := &Command{
 		Config:     &config.Config{GitlabUrl: url},
-		Args:       &commandargs.CommandArgs{GitlabKeyId: "disallowed", SshArgs: []string{"git-receive-pack", "group/repo"}},
+		Args:       &commandargs.Shell{GitlabKeyId: "disallowed", SshArgs: []string{"git-receive-pack", "group/repo"}},
 		ReadWriter: &readwriter.ReadWriter{ErrOut: output, Out: output, In: input},
 	}
 

go/internal/command/shared/accessverifier/accessverifier.go

@@ -14,7 +14,7 @@ type Response = accessverifier.Response
 
 type Command struct {
 	Config     *config.Config
-	Args       *commandargs.CommandArgs
+	Args       *commandargs.Shell
 	ReadWriter *readwriter.ReadWriter
 }
 

go/internal/command/shared/accessverifier/accessverifier_test.go

@@ -64,7 +64,7 @@ func TestMissingUser(t *testing.T) {
 	cmd, _, _, cleanup := setup(t)
 	defer cleanup()
 
-	cmd.Args = &commandargs.CommandArgs{GitlabKeyId: "2"}
+	cmd.Args = &commandargs.Shell{GitlabKeyId: "2"}
 	_, err := cmd.Verify(action, repo)
 
 	require.Equal(t, "missing user", err.Error())
@@ -74,7 +74,7 @@ func TestConsoleMessages(t *testing.T) {
 	cmd, errBuf, outBuf, cleanup := setup(t)
 	defer cleanup()
 
-	cmd.Args = &commandargs.CommandArgs{GitlabKeyId: "1"}
+	cmd.Args = &commandargs.Shell{GitlabKeyId: "1"}
 	cmd.Verify(action, repo)
 
 	require.Equal(t, "> GitLab: console\n> GitLab: message\n", errBuf.String())

go/internal/command/twofactorrecover/twofactorrecover.go

@@ -12,7 +12,7 @@ import (
 
 type Command struct {
 	Config     *config.Config
-	Args       *commandargs.CommandArgs
+	Args       *commandargs.Shell
 	ReadWriter *readwriter.ReadWriter
 }
 

go/internal/command/twofactorrecover/twofactorrecover_test.go

@@ -69,13 +69,13 @@ func TestExecute(t *testing.T) {
 
 	testCases := []struct {
 		desc           string
-		arguments      *commandargs.CommandArgs
+		arguments      *commandargs.Shell
 		answer         string
 		expectedOutput string
 	}{
 		{
 			desc:      "With a known key id",
-			arguments: &commandargs.CommandArgs{GitlabKeyId: "1"},
+			arguments: &commandargs.Shell{GitlabKeyId: "1"},
 			answer:    "yes\n",
 			expectedOutput: question +
 				"Your two-factor authentication recovery codes are:\n\nrecovery\ncodes\n\n" +
@@ -85,31 +85,31 @@ func TestExecute(t *testing.T) {
 		},
 		{
 			desc:           "With bad response",
-			arguments:      &commandargs.CommandArgs{GitlabKeyId: "-1"},
+			arguments:      &commandargs.Shell{GitlabKeyId: "-1"},
 			answer:         "yes\n",
 			expectedOutput: question + errorHeader + "Parsing failed\n",
 		},
 		{
 			desc:           "With API returns an error",
-			arguments:      &commandargs.CommandArgs{GitlabKeyId: "forbidden"},
+			arguments:      &commandargs.Shell{GitlabKeyId: "forbidden"},
 			answer:         "yes\n",
 			expectedOutput: question + errorHeader + "Forbidden!\n",
 		},
 		{
 			desc:           "With API fails",
-			arguments:      &commandargs.CommandArgs{GitlabKeyId: "broken"},
+			arguments:      &commandargs.Shell{GitlabKeyId: "broken"},
 			answer:         "yes\n",
 			expectedOutput: question + errorHeader + "Internal API error (500)\n",
 		},
 		{
 			desc:           "With missing arguments",
-			arguments:      &commandargs.CommandArgs{},
+			arguments:      &commandargs.Shell{},
 			answer:         "yes\n",
 			expectedOutput: question + errorHeader + "who='' is invalid\n",
 		},
 		{
 			desc:      "With negative answer",
-			arguments: &commandargs.CommandArgs{},
+			arguments: &commandargs.Shell{},
 			answer:    "no\n",
 			expectedOutput: question +
 				"New recovery codes have *not* been generated. Existing codes will remain valid.\n",

go/internal/command/uploadarchive/gitalycall_test.go

@@ -29,7 +29,7 @@ func TestUploadPack(t *testing.T) {
 
 	cmd := &Command{
 		Config:     &config.Config{GitlabUrl: url},
-		Args:       &commandargs.CommandArgs{GitlabKeyId: userId, CommandType: commandargs.UploadArchive, SshArgs: []string{"git-upload-archive", repo}},
+		Args:       &commandargs.Shell{GitlabKeyId: userId, CommandType: commandargs.UploadArchive, SshArgs: []string{"git-upload-archive", repo}},
 		ReadWriter: &readwriter.ReadWriter{ErrOut: output, Out: output, In: input},
 	}
 

go/internal/command/uploadarchive/uploadarchive.go

@@ -10,7 +10,7 @@ import (
 
 type Command struct {
 	Config     *config.Config
-	Args       *commandargs.CommandArgs
+	Args       *commandargs.Shell
 	ReadWriter *readwriter.ReadWriter
 }
 

go/internal/command/uploadarchive/uploadarchive_test.go

@@ -22,7 +22,7 @@ func TestForbiddenAccess(t *testing.T) {
 
 	cmd := &Command{
 		Config:     &config.Config{GitlabUrl: url},
-		Args:       &commandargs.CommandArgs{GitlabKeyId: "disallowed", SshArgs: []string{"git-upload-archive", "group/repo"}},
+		Args:       &commandargs.Shell{GitlabKeyId: "disallowed", SshArgs: []string{"git-upload-archive", "group/repo"}},
 		ReadWriter: &readwriter.ReadWriter{ErrOut: output, Out: output},
 	}
 

go/internal/command/uploadpack/gitalycall_test.go

@@ -29,7 +29,7 @@ func TestUploadPack(t *testing.T) {
 
 	cmd := &Command{
 		Config:     &config.Config{GitlabUrl: url},
-		Args:       &commandargs.CommandArgs{GitlabKeyId: userId, CommandType: commandargs.UploadPack, SshArgs: []string{"git-upload-pack", repo}},
+		Args:       &commandargs.Shell{GitlabKeyId: userId, CommandType: commandargs.UploadPack, SshArgs: []string{"git-upload-pack", repo}},
 		ReadWriter: &readwriter.ReadWriter{ErrOut: output, Out: output, In: input},
 	}
 

go/internal/command/uploadpack/uploadpack.go

@@ -10,7 +10,7 @@ import (
 
 type Command struct {
 	Config     *config.Config
-	Args       *commandargs.CommandArgs
+	Args       *commandargs.Shell
 	ReadWriter *readwriter.ReadWriter
 }
 

go/internal/command/uploadpack/uploadpack_test.go

@@ -22,7 +22,7 @@ func TestForbiddenAccess(t *testing.T) {
 
 	cmd := &Command{
 		Config:     &config.Config{GitlabUrl: url},
-		Args:       &commandargs.CommandArgs{GitlabKeyId: "disallowed", SshArgs: []string{"git-upload-pack", "group/repo"}},
+		Args:       &commandargs.Shell{GitlabKeyId: "disallowed", SshArgs: []string{"git-upload-pack", "group/repo"}},
 		ReadWriter: &readwriter.ReadWriter{ErrOut: output, Out: output},
 	}
 

go/internal/executable/executable.go

+package executable
+
+import (
+	"os"
+	"path/filepath"
+)
+
+const (
+	GitlabShell               = "gitlab-shell"
+	AuthorizedKeysCheck       = "gitlab-shell-authorized-keys-check"
+	AuthorizedPrincipalsCheck = "gitlab-shell-authorized-principals-check"
+)
+
+type Executable struct {
+	Name    string
+	RootDir string
+}
+
+var (
+	// osExecutable is overridden in tests
+	osExecutable = os.Executable
+)
+
+func New() (*Executable, error) {
+	path, err := osExecutable()
+	if err != nil {
+		return nil, err
+	}
+
+	rootDir, err := findRootDir(path)
+	if err != nil {
+		return nil, err
+	}
+
+	executable := &Executable{
+		Name:    filepath.Base(path),
+		RootDir: rootDir,
+	}
+
+	return executable, nil
+}
+
+func findRootDir(path string) (string, error) {
+	// Start: /opt/.../gitlab-shell/bin/gitlab-shell
+	// Ends:  /opt/.../gitlab-shell
+	rootDir := filepath.Dir(filepath.Dir(path))
+	pathFromEnv := os.Getenv("GITLAB_SHELL_DIR")
+
+	if pathFromEnv != "" {
+		if _, err := os.Stat(pathFromEnv); os.IsNotExist(err) {
+			return "", err
+		}
+
+		rootDir = pathFromEnv
+	}
+
+	return rootDir, nil
+}

go/internal/executable/executable_test.go

+package executable
+
+import (
+	"errors"
+	"testing"
+
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/testhelper"
+
+	"github.com/stretchr/testify/require"
+)
+
+type fakeOs struct {
+	OldExecutable func() (string, error)
+	Path          string
+	Error         error
+}
+
+func (f *fakeOs) Executable() (string, error) {
+	return f.Path, f.Error
+}
+
+func (f *fakeOs) Setup() {
+	f.OldExecutable = osExecutable
+	osExecutable = f.Executable
+}
+
+func (f *fakeOs) Cleanup() {
+	osExecutable = f.OldExecutable
+}
+
+func TestNewSuccess(t *testing.T) {
+	testCases := []struct {
+		desc            string
+		fakeOs          *fakeOs
+		environment     map[string]string
+		expectedRootDir string
+	}{
+		{
+			desc:            "GITLAB_SHELL_DIR env var is not defined",
+			fakeOs:          &fakeOs{Path: "/tmp/bin/gitlab-shell"},
+			expectedRootDir: "/tmp",
+		},
+		{
+			desc:   "GITLAB_SHELL_DIR env var is defined",
+			fakeOs: &fakeOs{Path: "/opt/bin/gitlab-shell"},
+			environment: map[string]string{
+				"GITLAB_SHELL_DIR": "/tmp",
+			},
+			expectedRootDir: "/tmp",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			restoreEnv := testhelper.TempEnv(tc.environment)
+			defer restoreEnv()
+
+			fake := tc.fakeOs
+			fake.Setup()
+			defer fake.Cleanup()
+
+			result, err := New()
+
+			require.NoError(t, err)
+			require.Equal(t, result.Name, "gitlab-shell")
+			require.Equal(t, result.RootDir, tc.expectedRootDir)
+		})
+	}
+}
+
+func TestNewFailure(t *testing.T) {
+	testCases := []struct {
+		desc        string
+		fakeOs      *fakeOs
+		environment map[string]string
+	}{
+		{
+			desc:   "failed to determine executable",
+			fakeOs: &fakeOs{Path: "", Error: errors.New("error")},
+		},
+		{
+			desc:   "GITLAB_SHELL_DIR doesn't exist",
+			fakeOs: &fakeOs{Path: "/tmp/bin/gitlab-shell"},
+			environment: map[string]string{
+				"GITLAB_SHELL_DIR": "/tmp/non/existing/directory",
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			restoreEnv := testhelper.TempEnv(tc.environment)
+			defer restoreEnv()
+
+			fake := tc.fakeOs
+			fake.Setup()
+			defer fake.Cleanup()
+
+			_, err := New()
+
+			require.Error(t, err)
+		})
+	}
+}

go/internal/gitlabnet/accessverifier/client.go

@@ -71,7 +71,7 @@ func NewClient(config *config.Config) (*Client, error) {
 	return &Client{client: client}, nil
 }
 
-func (c *Client) Verify(args *commandargs.CommandArgs, action commandargs.CommandType, repo string) (*Response, error) {
+func (c *Client) Verify(args *commandargs.Shell, action commandargs.CommandType, repo string) (*Response, error) {
 	request := &Request{Action: action, Repo: repo, Protocol: protocol, Changes: anyChanges}
 
 	if args.GitlabUsername != "" {
@@ -89,7 +89,7 @@ func (c *Client) Verify(args *commandargs.CommandArgs, action commandargs.Comman
 	return parse(response, args)
 }
 
-func parse(hr *http.Response, args *commandargs.CommandArgs) (*Response, error) {
+func parse(hr *http.Response, args *commandargs.Shell) (*Response, error) {
 	response := &Response{}
 	if err := gitlabnet.ParseJSON(hr, response); err != nil {
 		return nil, err

go/internal/gitlabnet/accessverifier/client_test.go

@@ -57,16 +57,16 @@ func TestSuccessfulResponses(t *testing.T) {
 
 	testCases := []struct {
 		desc string
-		args *commandargs.CommandArgs
+		args *commandargs.Shell
 		who  string
 	}{
 		{
 			desc: "Provide key id within the request",
-			args: &commandargs.CommandArgs{GitlabKeyId: "1"},
+			args: &commandargs.Shell{GitlabKeyId: "1"},
 			who:  "key-1",
 		}, {
 			desc: "Provide username within the request",
-			args: &commandargs.CommandArgs{GitlabUsername: "first"},
+			args: &commandargs.Shell{GitlabUsername: "first"},
 			who:  "user-1",
 		},
 	}
@@ -86,7 +86,7 @@ func TestGetCustomAction(t *testing.T) {
 	client, cleanup := setup(t)
 	defer cleanup()
 
-	args := &commandargs.CommandArgs{GitlabUsername: "custom"}
+	args := &commandargs.Shell{GitlabUsername: "custom"}
 	result, err := client.Verify(args, action, repo)
 	require.NoError(t, err)
 
@@ -134,7 +134,7 @@ func TestErrorResponses(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			args := &commandargs.CommandArgs{GitlabKeyId: tc.fakeId}
+			args := &commandargs.Shell{GitlabKeyId: tc.fakeId}
 			resp, err := client.Verify(args, action, repo)
 
 			require.EqualError(t, err, tc.expectedError)

go/internal/gitlabnet/discover/client.go

@@ -30,7 +30,7 @@ func NewClient(config *config.Config) (*Client, error) {
 	return &Client{config: config, client: client}, nil
 }
 
-func (c *Client) GetByCommandArgs(args *commandargs.CommandArgs) (*Response, error) {
+func (c *Client) GetByCommandArgs(args *commandargs.Shell) (*Response, error) {
 	params := url.Values{}
 	if args.GitlabUsername != "" {
 		params.Add("username", args.GitlabUsername)

go/internal/gitlabnet/lfsauthenticate/client.go

@@ -13,7 +13,7 @@ import (
 type Client struct {
 	config *config.Config
 	client *gitlabnet.GitlabClient
-	args   *commandargs.CommandArgs
+	args   *commandargs.Shell
 }
 
 type Request struct {
@@ -30,7 +30,7 @@ type Response struct {
 	ExpiresIn int    `json:"expires_in"`
 }
 
-func NewClient(config *config.Config, args *commandargs.CommandArgs) (*Client, error) {
+func NewClient(config *config.Config, args *commandargs.Shell) (*Client, error) {
 	client, err := gitlabnet.GetClient(config)
 	if err != nil {
 		return nil, fmt.Errorf("Error creating http client: %v", err)

go/internal/gitlabnet/lfsauthenticate/client_test.go

@@ -59,22 +59,22 @@ func TestFailedRequests(t *testing.T) {
 
 	testCases := []struct {
 		desc           string
-		args           *commandargs.CommandArgs
+		args           *commandargs.Shell
 		expectedOutput string
 	}{
 		{
 			desc:           "With bad response",
-			args:           &commandargs.CommandArgs{GitlabKeyId: "-1", CommandType: commandargs.UploadPack},
+			args:           &commandargs.Shell{GitlabKeyId: "-1", CommandType: commandargs.UploadPack},
 			expectedOutput: "Parsing failed",
 		},
 		{
 			desc:           "With API returns an error",
-			args:           &commandargs.CommandArgs{GitlabKeyId: "forbidden", CommandType: commandargs.UploadPack},
+			args:           &commandargs.Shell{GitlabKeyId: "forbidden", CommandType: commandargs.UploadPack},
 			expectedOutput: "Internal API error (403)",
 		},
 		{
 			desc:           "With API fails",
-			args:           &commandargs.CommandArgs{GitlabKeyId: "broken", CommandType: commandargs.UploadPack},
+			args:           &commandargs.Shell{GitlabKeyId: "broken", CommandType: commandargs.UploadPack},
 			expectedOutput: "Internal API error (500)",
 		},
 	}
@@ -99,7 +99,7 @@ func TestSuccessfulRequests(t *testing.T) {
 	url, cleanup := testserver.StartHttpServer(t, requests)
 	defer cleanup()
 
-	args := &commandargs.CommandArgs{GitlabKeyId: keyId, CommandType: commandargs.LfsAuthenticate}
+	args := &commandargs.Shell{GitlabKeyId: keyId, CommandType: commandargs.LfsAuthenticate}
 	client, err := NewClient(&config.Config{GitlabUrl: url}, args)
 	require.NoError(t, err)
 

go/internal/gitlabnet/twofactorrecover/client.go

@@ -36,7 +36,7 @@ func NewClient(config *config.Config) (*Client, error) {
 	return &Client{config: config, client: client}, nil
 }
 
-func (c *Client) GetRecoveryCodes(args *commandargs.CommandArgs) ([]string, error) {
+func (c *Client) GetRecoveryCodes(args *commandargs.Shell) ([]string, error) {
 	requestBody, err := c.getRequestBody(args)
 
 	if err != nil {
@@ -65,7 +65,7 @@ func parse(hr *http.Response) ([]string, error) {
 	return response.RecoveryCodes, nil
 }
 
-func (c *Client) getRequestBody(args *commandargs.CommandArgs) (*RequestBody, error) {
+func (c *Client) getRequestBody(args *commandargs.Shell) (*RequestBody, error) {
 	client, err := discover.NewClient(c.config)
 
 	if err != nil {

go/internal/gitlabnet/twofactorrecover/client_test.go

@@ -85,7 +85,7 @@ func TestGetRecoveryCodesByKeyId(t *testing.T) {
 	client, cleanup := setup(t)
 	defer cleanup()
 
-	args := &commandargs.CommandArgs{GitlabKeyId: "0"}
+	args := &commandargs.Shell{GitlabKeyId: "0"}
 	result, err := client.GetRecoveryCodes(args)
 	assert.NoError(t, err)
 	assert.Equal(t, []string{"recovery 1", "codes 1"}, result)
@@ -95,7 +95,7 @@ func TestGetRecoveryCodesByUsername(t *testing.T) {
 	client, cleanup := setup(t)
 	defer cleanup()
 
-	args := &commandargs.CommandArgs{GitlabUsername: "jane-doe"}
+	args := &commandargs.Shell{GitlabUsername: "jane-doe"}
 	result, err := client.GetRecoveryCodes(args)
 	assert.NoError(t, err)
 	assert.Equal(t, []string{"recovery 2", "codes 2"}, result)
@@ -105,7 +105,7 @@ func TestMissingUser(t *testing.T) {
 	client, cleanup := setup(t)
 	defer cleanup()
 
-	args := &commandargs.CommandArgs{GitlabKeyId: "1"}
+	args := &commandargs.Shell{GitlabKeyId: "1"}
 	_, err := client.GetRecoveryCodes(args)
 	assert.Equal(t, "missing user", err.Error())
 }
@@ -138,7 +138,7 @@ func TestErrorResponses(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.desc, func(t *testing.T) {
-			args := &commandargs.CommandArgs{GitlabKeyId: tc.fakeId}
+			args := &commandargs.Shell{GitlabKeyId: tc.fakeId}
 			resp, err := client.GetRecoveryCodes(args)
 
 			assert.EqualError(t, err, tc.expectedError)

spec/gitlab_shell_authorized_keys_check_spec.rb

@@ -21,14 +21,9 @@ describe 'bin/gitlab-shell-authorized-keys-check' do
     end
   end
 
-  before(:all) do
-    write_config(
-      "gitlab_url" => "http+unix://#{CGI.escape(tmp_socket_path)}",
-    )
-  end
-
   let(:authorized_keys_check_path) { File.join(tmp_root_path, 'bin', 'gitlab-shell-authorized-keys-check') }
 
+  shared_examples 'authorized keys check' do
     it 'succeeds when a valid key is given' do
       output, status = run!
 
@@ -70,6 +65,41 @@ describe 'bin/gitlab-shell-authorized-keys-check' do
       expect(output).to eq('')
       expect(status).to be_success
     end
+  end
+
+  describe 'without go features' do
+    before(:all) do
+      write_config(
+        "gitlab_url" => "http+unix://#{CGI.escape(tmp_socket_path)}",
+      )
+    end
+
+    it_behaves_like 'authorized keys check'
+  end
+
+  describe 'without go features (via go)', :go do
+    before(:all) do
+      write_config(
+        "gitlab_url" => "http+unix://#{CGI.escape(tmp_socket_path)}",
+      )
+    end
+
+    it_behaves_like 'authorized keys check'
+  end
+
+  pending 'with the go authorized-keys-check feature', :go do
+    before(:all) do
+      write_config(
+        'gitlab_url' => "http+unix://#{CGI.escape(tmp_socket_path)}",
+        'migration' => {
+          'enabled' => true,
+          'features' => ['authorized-keys-check']
+        }
+      )
+    end
+
+    it_behaves_like 'authorized keys check'
+  end
 
   def run!(expected_user: 'git', actual_user: 'git', key: 'known-rsa-key')
     cmd = [

spec/gitlab_shell_authorized_principals_check_spec.rb

+require_relative 'spec_helper'
+
+describe 'bin/gitlab-shell-authorized-principals-check' do
+  include_context 'gitlab shell'
+
+  def mock_server(server)
+    # Do nothing as we're not connecting to a server in this check.
+  end
+
+  let(:authorized_principals_check_path) { File.join(tmp_root_path, 'bin', 'gitlab-shell-authorized-principals-check') }
+
+  shared_examples 'authorized principals check' do
+    it 'succeeds when a valid principal is given' do
+      output, status = run!
+
+      expect(output).to eq("command=\"#{gitlab_shell_path} username-key\",no-port-forwarding,no-X11-forwarding,no-agent-forwarding,no-pty principal\n")
+      expect(status).to be_success
+    end
+
+    it 'fails when not enough arguments are given' do
+      output, status = run!(key_id: nil, principals: [])
+
+      expect(output).to eq('')
+      expect(status).not_to be_success
+    end
+
+    it 'fails when key_id is blank' do
+      output, status = run!(key_id: '')
+
+      expect(output).to eq('')
+      expect(status).not_to be_success
+    end
+
+    it 'fails when principals include an empty item' do
+      output, status = run!(principals: ['principal', ''])
+
+      expect(output).to eq('')
+      expect(status).not_to be_success
+    end
+  end
+
+  describe 'without go features' do
+    before(:all) do
+      write_config({})
+    end
+
+    it_behaves_like 'authorized principals check'
+  end
+
+  describe 'without go features (via go)', :go do
+    before(:all) do
+      write_config({})
+    end
+
+    it_behaves_like 'authorized principals check'
+  end
+
+  pending 'with the go authorized-principals-check feature', :go do
+    before(:all) do
+      write_config(
+        'migration' => {
+          'enabled' => true,
+          'features' => ['authorized-principals-check']
+        }
+      )
+    end
+
+    it_behaves_like 'authorized principals check'
+  end
+
+  def run!(key_id: 'key', principals: ['principal'])
+    cmd = [
+      authorized_principals_check_path,
+      key_id,
+      principals,
+    ].flatten.compact
+
+    output = IO.popen(cmd, &:read)
+
+    [output, $?]
+  end
+end

spec/gitlab_shell_gitlab_shell_spec.rb

@@ -123,10 +123,10 @@ describe 'bin/gitlab-shell' do
 
     it_behaves_like 'results with keys'
 
-    it 'outputs "Only ssh allowed"' do
+    it 'outputs "Only SSH allowed"' do
       _, stderr, status = run!(["-c/usr/share/webapps/gitlab-shell/bin/gitlab-shell", "username-someuser"], env: {})
 
-      expect(stderr).to eq("Only ssh allowed\n")
+      expect(stderr).to eq("Only SSH allowed\n")
       expect(status).not_to be_success
     end