Don't fall back to ruby for non SSH connections

When SSH_CONNECTION is not set, we don't fall back to ruby, but instead fail directly in go writing the error to stderr.

Author: Reprazent

go/cmd/gitlab-shell/main.go

@@ -41,17 +41,16 @@ func main() {
 
 cmd, err := command.New(os.Args, config)
 if err != nil {
-// Failed to build the command, fall back to ruby.
 // For now this could happen if `SSH_CONNECTION` is not set on
 // the environment
-fmt.Fprintf(os.Stderr, "Failed to build command: %v\n", err)
-execRuby()
+fmt.Fprintf(os.Stderr, "%v\n", err)
+os.Exit(1)
 }
 
 // The command will write to STDOUT on execution or replace the current
 // process in case of the `fallback.Command`
 if err = cmd.Execute(); err != nil {
-fmt.Fprintf(os.Stderr, "%s\n", err)
+fmt.Fprintf(os.Stderr, "%v\n", err)
 os.Exit(1)
 }
 }

go/internal/command/commandargs/command_args.go

@@ -37,15 +37,15 @@ func Parse(arguments []string) (*CommandArgs, error) {
 return info, nil
 }
 
-func (info *CommandArgs) parseWho(arguments []string) {
+func (c *CommandArgs) parseWho(arguments []string) {
 for _, argument := range arguments {
 if keyId := tryParseKeyId(argument); keyId != "" {
-info.GitlabKeyId = keyId
+c.GitlabKeyId = keyId
 break
 }
 
 if username := tryParseUsername(argument); username != "" {
-info.GitlabUsername = username
+c.GitlabUsername = username
 break
 }
 }

spec/gitlab_shell_gitlab_shell_spec.rb

@@ -1,5 +1,7 @@
 require_relative 'spec_helper'
 
+require 'open3'
+
 describe 'bin/gitlab-shell' do
  def original_root_path
  ROOT_PATH
@@ -60,67 +62,66 @@ def tmp_socket_path
  shared_examples 'results with keys' do
  # Basic valid input
  it 'succeeds and prints username when a valid known key id is given' do
- output, status = run!(["key-100"])
+ output, _, status = run!(["key-100"])
 
  expect(output).to eq("Welcome to GitLab, @someuser!\n")
  expect(status).to be_success
  end
 
  it 'succeeds and prints username when a valid known username is given' do
- output, status = run!(["username-someuser"])
+ output, _, status = run!(["username-someuser"])
 
  expect(output).to eq("Welcome to GitLab, @someuser!\n")
  expect(status).to be_success
  end
 
  # Valid but unknown input
  it 'succeeds and prints Anonymous when a valid unknown key id is given' do
- output, status = run!(["key-12345"])
+ output, _, status = run!(["key-12345"])
 
  expect(output).to eq("Welcome to GitLab, Anonymous!\n")
  expect(status).to be_success
  end
 
  it 'succeeds and prints Anonymous when a valid unknown username is given' do
- output, status = run!(["username-unknown"])
+ output, _, status = run!(["username-unknown"])
 
  expect(output).to eq("Welcome to GitLab, Anonymous!\n")
  expect(status).to be_success
  end
 
- # Invalid input. TODO: capture stderr & compare
  it 'gets an ArgumentError on invalid input (empty)' do
- output, status = run!([])
+ _, stderr, status = run!([])
 
- expect(output).to eq("")
+ expect(stderr).to match(/who='' is invalid/)
  expect(status).not_to be_success
  end
 
  it 'gets an ArgumentError on invalid input (unknown)' do
- output, status = run!(["whatever"])
+ _, stderr, status = run!(["whatever"])
 
- expect(output).to eq("")
+ expect(stderr).to match(/who='' is invalid/)
  expect(status).not_to be_success
  end
 
  it 'gets an ArgumentError on invalid input (multiple unknown)' do
- output, status = run!(["this", "is", "all", "invalid"])
+ _, stderr, status = run!(["this", "is", "all", "invalid"])
 
- expect(output).to eq("")
+ expect(stderr).to match(/who='' is invalid/)
  expect(status).not_to be_success
  end
 
  # Not so basic valid input
  # (https://gitlab.com/gitlab-org/gitlab-shell/issues/145)
  it 'succeeds and prints username when a valid known key id is given in the middle of other input' do
- output, status = run!(["-c/usr/share/webapps/gitlab-shell/bin/gitlab-shell", "key-100", "2foo"])
+ output, _, status = run!(["-c/usr/share/webapps/gitlab-shell/bin/gitlab-shell", "key-100", "2foo"])
 
  expect(output).to eq("Welcome to GitLab, @someuser!\n")
  expect(status).to be_success
  end
 
  it 'succeeds and prints username when a valid known username is given in the middle of other input' do
- output, status = run!(["-c/usr/share/webapps/gitlab-shell/bin/gitlab-shell", "username-someuser" ,"foo"])
+ output, _, status = run!(["-c/usr/share/webapps/gitlab-shell/bin/gitlab-shell", "username-someuser" ,"foo"])
 
  expect(output).to eq("Welcome to GitLab, @someuser!\n")
  expect(status).to be_success
@@ -144,24 +145,27 @@ def tmp_socket_path
  )
  end
 
-
-
  it_behaves_like 'results with keys' do
  before do
  pending
  end
  end
+
+ 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(status).not_to be_success
+ end
  end
 
- def run!(args)
+ def run!(args, env: {'SSH_CONNECTION' => 'fake'})
  cmd = [
  gitlab_shell_path,
  args
- ].flatten.compact
-
- output = IO.popen({'SSH_CONNECTION' => 'fake'}, cmd, &:read)
+ ].flatten.compact.join(' ')
 
- [output, $?]
+ Open3.capture3(env, cmd)
  end
 
  def write_config(config)