Implement fifo based shell env collector (#617)

Replace the shell env collector from a file-based to named pipe-based
(fifo). For Windows, it will be a named pipe, but it will be done in a
follow-up PR. 

Also, the env dump regardless of the destination is encrypted. 
The encryption is mostly to avoid leaking secrets by accidents. Our
threat model assumes execution of runme by the same user without any
additional isolation.

---------

Co-authored-by: Sebastian Tiedtke <sebastiantiedtke@gmail.com>

.github/workflows/ci.yml

@@ -28,7 +28,7 @@ jobs:
       # to make sure that `make lint` also works.
       - name: Run lint from Makefile
         run: make lint
-      - uses: pre-commit/action@v3.0.0
+      - uses: pre-commit/action@v3.0.1
       - name: pre-commit
         run: pre-commit run --files $(git diff-tree --no-commit-id --name-only -r HEAD)
 

.pre-commit-config.yaml

@@ -3,23 +3,53 @@ repos:
     rev: v4.5.0
     hooks:
       - id: check-yaml
+        stages: [commit]
+      - id: check-json
+        exclude: "^.vscode/"
+        stages: [commit]
+      - id: check-merge-conflict
+        stages: [commit]
+      - id: check-case-conflict
+        stages: [commit]
+      - id: detect-private-key
+        stages: [commit]
       - id: end-of-file-fixer
+        stages: [commit]
       - id: trailing-whitespace
-  - repo: https://github.com/codespell-project/codespell
-    rev: v2.2.4
+        stages: [commit]
+  - repo: https://github.com/crate-ci/committed
+    rev: v1.0.20
     hooks:
-      - id: codespell
-        args: ["-L=ndoes,nd,clen"] # 2x words that are preceeded with "\n"; cLen
-        exclude: (package.*\.json$|go\.sum|pkg/api/gen)
-  - repo: https://github.com/TekWizely/pre-commit-golang
-    rev: v1.0.0-rc.1
+      - id: committed
+        stages: [commit-msg]
+  - repo: https://github.com/crate-ci/typos
+    rev: v1.22.9
+    hooks:
+      - id: typos
+        args: ["--diff", "--force-exclude"]
+        exclude: "^.vscode/|go.mod"
+  - repo: local
     hooks:
-      - id: go-fumpt
-        exclude: (^pkg/api/gen/.*$|^internal/client/graphql/query/generated.go$)
-      - id: go-build-mod
       - id: go-mod-tidy
-      - id: go-revive-repo-mod
-      - id: go-staticcheck-mod
-        exclude: (^expect/.*$|^internal/client/graphql/query/generated.go$)
-      - id: go-sec-repo-mod
-        args: ["-exclude=G204,G304,G404", "-exclude-generated"]
+        name: go mod tidy
+        entry: go
+        args: ["mod", "tidy"]
+        language: system
+        types: [go]
+        pass_filenames: false
+        stages: [commit]
+      - id: lint
+        name: lint
+        entry: make
+        args: ["lint"]
+        language: system
+        types: [go]
+        pass_filenames: false
+        stages: [commit]
+      - id: build
+        name: build
+        entry: make
+        language: system
+        types: [go]
+        pass_filenames: false
+        stages: [commit]

.typos.toml

@@ -0,0 +1,6 @@
+[default]
+extend-ignore-re = [
+    "\"data\":\"\\b[0-9A-Za-z+/]+(=|==)?\\b\"", # base64 blobs in some structs
+    "\"id\":\"[0-9A-Z]+\"",                     # cell IDs
+    "thi\\.\\.\\.ice",
+]

Makefile

@@ -95,7 +95,14 @@ fmt:
 
 .PHONY: lint
 lint:
-	@revive -config revive.toml -formatter stylish -exclude integration/subject/... ./...
+	@gofumpt -d .
+	@revive \
+		-config revive.toml \
+		-formatter stylish \
+		-exclude integration/subject/... \
+		./...
+	@staticcheck ./...
+	@gosec -quiet -exclude=G204,G304,G404 -exclude-generated ./...
 
 .PHONY: pre-commit
 pre-commit: build wasm test lint

internal/cmd/beta/run_cmd.go

@@ -115,8 +115,12 @@ func runCodeBlock(
 	if err != nil {
 		return err
 	}
-	cmd := factory.Build(cfg, options)
-	if err := cmd.Start(ctx); err != nil {
+	cmd, err := factory.Build(cfg, options)
+	if err != nil {
+		return err
+	}
+	err = cmd.Start(ctx)
+	if err != nil {
 		return err
 	}
 	return cmd.Wait()

internal/cmd/environment.go

@@ -3,25 +3,28 @@ package cmd
 import (
 	"context"
 	"fmt"
+	"io"
 	"os"
 	"strings"
 	"time"
 
 	"github.com/cli/cli/v2/pkg/iostreams"
 	"github.com/cli/go-gh/pkg/tableprinter"
 	"github.com/pkg/errors"
-
 	"github.com/spf13/cobra"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials"
 
+	"github.com/stateful/runme/v3/internal/command"
 	"github.com/stateful/runme/v3/internal/owl"
 	"github.com/stateful/runme/v3/internal/runner/client"
 	runmetls "github.com/stateful/runme/v3/internal/tls"
 	runnerv1 "github.com/stateful/runme/v3/pkg/api/gen/proto/go/runme/runner/v1"
 )
 
-var osEnviron = os.Environ
+var newOSEnvironReader = func() (io.Reader, error) {
+	return command.NewEnvProducerFromEnv()
+}
 
 func environmentCmd() *cobra.Command {
 	cmd := cobra.Command{
@@ -204,9 +207,12 @@ func environmentDumpCmd() *cobra.Command {
 				return errors.New("must be run in insecure mode; enable by running with --insecure flag")
 			}
 
-			dumped := getDumpedEnvironment()
+			producer, err := newOSEnvironReader()
+			if err != nil {
+				return err
+			}
 
-			_, _ = cmd.OutOrStdout().Write([]byte(dumped))
+			_, _ = io.Copy(cmd.OutOrStdout(), producer)
 
 			return nil
 		},
@@ -269,7 +275,3 @@ func printStore(msgData *runnerv1.MonitorEnvStoreResponse_Snapshot, lines int, a
 
 	return table.Render()
 }
-
-func getDumpedEnvironment() string {
-	return strings.Join(osEnviron(), "\x00")
-}

internal/cmd/environment_test.go

@@ -2,48 +2,43 @@ package cmd
 
 import (
 	"bytes"
+	"io"
 	"os/exec"
 	"strings"
 	"testing"
 
+	"github.com/stateful/runme/v3/internal/command"
 	"github.com/stretchr/testify/assert"
 )
 
-func runEnvCommand(...string) *exec.Cmd {
-	return exec.Command("env", "-0")
-}
-
-func Test_cmdEnvironment(t *testing.T) {
-	t.Run("Dump", func(t *testing.T) {
-		t.Parallel()
-
-		cmd := runEnvCommand()
+func Test_cmdEnvironment_Dump(t *testing.T) {
+	t.Parallel()
 
-		stdout, stderr := new(bytes.Buffer), new(bytes.Buffer)
+	env := []string{
+		"A=1",
+		"B=2",
+		"C=3",
+	}
 
-		cmd.Stdout = stdout
-		cmd.Stderr = stderr
+	stdout, stderr := new(bytes.Buffer), new(bytes.Buffer)
 
-		env := []string{
-			"A=1",
-			"B=2",
-			"C=3",
-		}
+	cmd := runEnvCommand()
+	cmd.Stdout = stdout
+	cmd.Stderr = stderr
+	cmd.Env = env
 
-		cmd.Env = env
+	original := newOSEnvironReader
+	defer func() { newOSEnvironReader = original }()
+	newOSEnvironReader = func() (io.Reader, error) {
+		return strings.NewReader(command.EnvSliceToString(env)), nil
+	}
 
-		err := cmd.Run()
-
-		old := osEnviron
-		defer func() { osEnviron = old }()
-
-		osEnviron = func() []string {
-			return env
-		}
+	err := cmd.Run()
+	assert.NoError(t, err)
+	assert.Equal(t, "", stderr.String())
+	assert.Equal(t, true, strings.HasPrefix(stdout.String(), "A=1\x00B=2\x00C=3\x00"))
+}
 
-		assert.NoError(t, err)
-		assert.Equal(t, "", stderr.String())
-		assert.Equal(t, true, strings.HasPrefix(stdout.String(), "A=1\x00B=2\x00C=3\x00"))
-		assert.Equal(t, "A=1\x00B=2\x00C=3", getDumpedEnvironment())
-	})
+func runEnvCommand(...string) *exec.Cmd {
+	return exec.Command("env", "-0")
 }

internal/command/command.go

@@ -36,14 +36,13 @@ type internalCommand interface {
 }
 
 type base struct {
-	cfg         *ProgramConfig
-	project     *project.Project
-	runtime     runtime
-	session     *Session
-	stdin       io.Reader
-	stdinWriter io.Writer
-	stdout      io.Writer
-	stderr      io.Writer
+	cfg     *ProgramConfig
+	project *project.Project
+	runtime runtime
+	session *Session
+	stdin   io.Reader
+	stdout  io.Writer
+	stderr  io.Writer
 }
 
 var _ internalCommand = (*base)(nil)
@@ -174,10 +173,6 @@ func (c *base) Stdin() io.Reader {
 	return c.stdin
 }
 
-func (c *base) StdinWriter() io.Writer {
-	return c.stdinWriter
-}
-
 func (c *base) Stdout() io.Writer {
 	if c.stdout == nil {
 		c.stdout = io.Discard

internal/command/command_docker_test.go

@@ -25,7 +25,7 @@ func TestDockerCommand(t *testing.T) {
 
 	// This test case is treated as a warm up. Do not parallelize.
 	t.Run("NoOutput", func(t *testing.T) {
-		cmd := factory.Build(
+		cmd, err := factory.Build(
 			&ProgramConfig{
 				ProgramName: "echo",
 				Arguments:   []string{"-n", "test"},
@@ -34,14 +34,15 @@ func TestDockerCommand(t *testing.T) {
 			},
 			CommandOptions{},
 		)
+		require.NoError(t, err)
 		require.NoError(t, cmd.Start(context.Background()))
 		require.NoError(t, cmd.Wait())
 	})
 
 	t.Run("Output", func(t *testing.T) {
 		t.Parallel()
 		stdout := bytes.NewBuffer(nil)
-		cmd := factory.Build(
+		cmd, err := factory.Build(
 			&ProgramConfig{
 				ProgramName: "echo",
 				Arguments:   []string{"-n", "test"},
@@ -50,44 +51,42 @@ func TestDockerCommand(t *testing.T) {
 			},
 			CommandOptions{Stdout: stdout},
 		)
+		require.NoError(t, err)
 		require.NoError(t, cmd.Start(context.Background()))
 		require.NoError(t, cmd.Wait())
 		assert.Equal(t, "test", stdout.String())
 	})
 
 	t.Run("Running", func(t *testing.T) {
 		t.Parallel()
-		cmd := factory.Build(
+		cmd, err := factory.Build(
 			&ProgramConfig{
 				ProgramName: "sleep",
 				Arguments:   []string{"1"},
 				Mode:        runnerv2alpha1.CommandMode_COMMAND_MODE_INLINE,
 			},
 			CommandOptions{},
 		)
-		err := cmd.Start(context.Background())
 		require.NoError(t, err)
+		require.NoError(t, cmd.Start(context.Background()))
 		require.True(t, cmd.Running())
 		require.Greater(t, cmd.Pid(), 0)
 		require.NoError(t, cmd.Wait())
 	})
 
 	t.Run("NonZeroExit", func(t *testing.T) {
-		t.Skip("enable when [envCollector] supports [Runtime]")
-
 		t.Parallel()
 
-		cmd := factory.Build(
+		cmd, err := factory.Build(
 			&ProgramConfig{
 				ProgramName: "sh",
 				Arguments:   []string{"-c", "exit 11"},
 				Mode:        runnerv2alpha1.CommandMode_COMMAND_MODE_INLINE,
 			},
 			CommandOptions{},
 		)
+		require.NoError(t, err)
 		require.NoError(t, cmd.Start(context.Background()))
-		// TODO(adamb): wait should return non-nil error due to non-zero exit code.
-		require.NoError(t, cmd.Wait())
-		require.Equal(t, 11, cmd.(*dockerCommand).cmd.ProcessState.ExitCode)
+		require.Error(t, cmd.Wait(), "exit code 11 due to error \"\"")
 	})
 }