Skip to content

feat: add shebang support to scripts #10134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 9, 2023
Merged

feat: add shebang support to scripts #10134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion agent/agentssh/agentssh.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"time"

"github.com/gliderlabs/ssh"
"github.com/kballard/go-shellquote"
"github.com/pkg/sftp"
"github.com/prometheus/client_golang/prometheus"
"github.com/spf13/afero"
Expand Down Expand Up @@ -515,8 +516,29 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string)
if runtime.GOOS == "windows" {
caller = "/c"
}
name := shell
args := []string{caller, script}

if strings.HasPrefix(strings.TrimSpace(script), "#!") {
// If the script starts with a shebang, we should
// execute it directly. This is useful for running
// scripts that aren't executable.
shebang := strings.SplitN(script, "\n", 2)[0]
shebang = strings.TrimPrefix(shebang, "#!")
shebang = strings.TrimSpace(shebang)
words, err := shellquote.Split(shebang)
if err != nil {
return nil, xerrors.Errorf("split shebang: %w", err)
}
name = words[0]
if len(words) > 1 {
args = words[1:]
} else {
args = []string{}
}
args = append(args, caller, script)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I guess this still assumes that the shebang is for a shell that supports -c or /c.

What if this was the contents:

#!/usr/bin/env node

console.log('hi');

This works on a typical Unix system, but here it would trigger the --check flag for node. I suppose to truly replicate shebang, we should also write script to a file as-is, so that we can then call this via (name, args..., "/path/to/script").

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this gets a bit weird but we totally can. Should we just temporarily write it then defer a cleanup?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, a bit weird, but it'd be awesome to support the use-case fully. I think temp write + cleanup sounds good (and no need to make it executable since we're handling the shebang parsing part)!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Going to fix!

}

// gliderlabs/ssh returns a command slice of zero
// when a shell is requested.
if len(script) == 0 {
Expand All @@ -528,7 +550,7 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string)
}
}

cmd := pty.CommandContext(ctx, shell, args...)
cmd := pty.CommandContext(ctx, name, args...)
cmd.Dir = manifest.Directory

// If the metadata directory doesn't exist, we run the command
Expand Down
37 changes: 37 additions & 0 deletions agent/agentssh/agentssh_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"bytes"
"context"
"net"
"runtime"
"strings"
"sync"
"testing"
Expand Down Expand Up @@ -71,6 +72,42 @@ func TestNewServer_ServeClient(t *testing.T) {
<-done
}

func TestNewServer_ExecuteShebang(t *testing.T) {
t.Parallel()
if runtime.GOOS == "windows" {
t.Skip("bash doesn't exist on Windows")
}

ctx := context.Background()
logger := slogtest.Make(t, nil)
s, err := agentssh.NewServer(ctx, logger, prometheus.NewRegistry(), afero.NewMemMapFs(), 0, "")
require.NoError(t, err)
t.Cleanup(func() {
_ = s.Close()
})
s.AgentToken = func() string { return "" }
s.Manifest = atomic.NewPointer(&agentsdk.Manifest{})

t.Run("Basic", func(t *testing.T) {
t.Parallel()
cmd, err := s.CreateCommand(ctx, `#!/bin/bash
echo test`, nil)
require.NoError(t, err)
output, err := cmd.AsExec().CombinedOutput()
require.NoError(t, err)
require.Equal(t, "test\n", string(output))
})
t.Run("Args", func(t *testing.T) {
t.Parallel()
cmd, err := s.CreateCommand(ctx, `#!/usr/bin/env bash
echo test`, nil)
require.NoError(t, err)
output, err := cmd.AsExec().CombinedOutput()
require.NoError(t, err)
require.Equal(t, "test\n", string(output))
})
}

func TestNewServer_CloseActiveConnections(t *testing.T) {
t.Parallel()

Expand Down
3 changes: 3 additions & 0 deletions dogfood/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ resource "coder_agent" "dev" {
display_name = "Swap Usage (Host)"
key = "4_swap_usage_host"
script = <<EOT
#!/bin/bash
echo "$(free -b | awk '/^Swap/ { printf("%.1f/%.1f", $3/1024.0/1024.0/1024.0, $2/1024.0/1024.0/1024.0) }') GiB"
EOT
interval = 10
Expand All @@ -183,6 +184,7 @@ resource "coder_agent" "dev" {
key = "5_load_host"
# get load avg scaled by number of cores
script = <<EOT
#!/bin/bash
echo "`cat /proc/loadavg | awk '{ print $1 }'` `nproc`" | awk '{ printf "%0.2f", $1/$2 }'
EOT
interval = 60
Expand All @@ -201,6 +203,7 @@ resource "coder_agent" "dev" {
display_name = "Word of the Day"
key = "7_word"
script = <<EOT
#!/bin/bash
curl -o - --silent https://www.merriam-webster.com/word-of-the-day 2>&1 | awk ' $0 ~ "Word of the Day: [A-z]+" { print $5; exit }'
EOT
interval = 86400
Expand Down