Skip to content
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.

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

Fix security issue between buildkite agent and Bash 5.2 #1781

Merged
merged 10 commits into from
Oct 14, 2022
6 changes: 3 additions & 3 deletions bootstrap/integration/artifact_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func TestArtifactsUploadAfterCommand(t *testing.T) {
})

// Mock out the artifact calls
agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)
agent.
Expect("meta-data", "exists", "buildkite:git:commit").
AndExitWith(0)
Expand Down Expand Up @@ -56,7 +56,7 @@ func TestArtifactsUploadAfterCommandFails(t *testing.T) {
})

// Mock out the artifact calls
agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)
agent.
Expect("meta-data", "exists", "buildkite:git:commit").
AndExitWith(0)
Expand Down Expand Up @@ -91,7 +91,7 @@ func TestArtifactsUploadAfterCommandHookFails(t *testing.T) {
})

// Mock out the artifact calls
agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)
agent.
Expect("meta-data", "exists", "buildkite:git:commit").
AndExitWith(0)
Expand Down
39 changes: 38 additions & 1 deletion bootstrap/integration/bootstrap_tester.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package integration
import (
"bufio"
"bytes"
"encoding/json"
"fmt"
"io"
"io/ioutil"
Expand Down Expand Up @@ -176,6 +177,17 @@ func (b *BootstrapTester) HasMock(name string) bool {
return false
}

// MockAgent creates a mock for the buildkite-agent binary
func (b *BootstrapTester) MockAgent(t *testing.T) *bintest.Mock {
agent := b.MustMock(t, "buildkite-agent")
agent.Expect("env").
Min(0).
Max(bintest.InfiniteTimes).
AndCallFunc(mockEnvAsJSONOnStdout(b))

return agent
}

// writeHookScript generates a buildkite-agent hook script that calls a mock binary
func (b *BootstrapTester) writeHookScript(m *bintest.Mock, name string, dir string, args ...string) (string, error) {
hookScript := filepath.Join(dir, name)
Expand Down Expand Up @@ -234,7 +246,7 @@ func (b *BootstrapTester) ExpectGlobalHook(name string) *bintest.Expectation {
func (b *BootstrapTester) Run(t *testing.T, env ...string) error {
// Mock out the meta-data calls to the agent after checkout
if !b.HasMock("buildkite-agent") {
agent := b.MustMock(t, "buildkite-agent")
agent := b.MockAgent(t)
agent.
Expect("meta-data", "exists", "buildkite:git:commit").
Optionally().
Expand Down Expand Up @@ -348,6 +360,31 @@ func (b *BootstrapTester) Close() error {
return nil
}

func mockEnvAsJSONOnStdout(b *BootstrapTester) func(c *bintest.Call) {
return func(c *bintest.Call) {
envMap := map[string]string{}

for _, e := range b.Env { // The env from the bootstrap tester
k, v, _ := strings.Cut(e, "=")
envMap[k] = v
}

for _, e := range c.Env { // The env from the call
k, v, _ := strings.Cut(e, "=")
envMap[k] = v
}

envJSON, err := json.Marshal(envMap)
if err != nil {
fmt.Println("Failed to marshal env map in mocked agent call:", err)
c.Exit(1)
}

c.Stdout.Write(envJSON)
c.Exit(0)
}
}

type testLogWriter struct {
io.Writer
sync.Mutex
Expand Down
24 changes: 12 additions & 12 deletions bootstrap/integration/checkout_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
// Commit: Example Human <legit@example.com>
// CommitDate: Thu Jan 15 11:05:16 2015 +0800
//
// hello world
// hello world
var commitPattern = bintest.MatchPattern(`(?ms)\Acommit [0-9a-f]+\n.*^Author:`)

// Enable an experiment, returning a function to restore the previous state.
Expand Down Expand Up @@ -72,7 +72,7 @@ func TestCheckingOutGitHubPullRequestsWithGitMirrorsExperiment(t *testing.T) {
})

// Mock out the meta-data calls to the agent after checkout
agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)
agent.Expect("meta-data", "exists", "buildkite:git:commit").AndExitWith(1)
agent.Expect("meta-data", "set", "buildkite:git:commit").WithStdin(commitPattern)

Expand Down Expand Up @@ -126,7 +126,7 @@ func TestWithResolvingCommitExperiment(t *testing.T) {
}

// Mock out the meta-data calls to the agent after checkout
agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)
agent.Expect("meta-data", "exists", "buildkite:git:commit").AndExitWith(1)
agent.Expect("meta-data", "set", "buildkite:git:commit").WithStdin(commitPattern)

Expand Down Expand Up @@ -177,7 +177,7 @@ func TestCheckingOutLocalGitProject(t *testing.T) {
}

// Mock out the meta-data calls to the agent after checkout
agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)
agent.Expect("meta-data", "exists", "buildkite:git:commit").AndExitWith(1)
agent.Expect("meta-data", "set", "buildkite:git:commit").WithStdin(commitPattern)

Expand Down Expand Up @@ -260,7 +260,7 @@ func TestCheckingOutLocalGitProjectWithSubmodules(t *testing.T) {
}

// Mock out the meta-data calls to the agent after checkout
agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)
agent.Expect("meta-data", "exists", "buildkite:git:commit").AndExitWith(1)
agent.Expect("meta-data", "set", "buildkite:git:commit").WithStdin(commitPattern)

Expand Down Expand Up @@ -334,7 +334,7 @@ func TestCheckingOutLocalGitProjectWithSubmodulesDisabled(t *testing.T) {
}

// Mock out the meta-data calls to the agent after checkout
agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)
agent.Expect("meta-data", "exists", "buildkite:git:commit").AndExitWith(1)
agent.Expect("meta-data", "set", "buildkite:git:commit").WithStdin(commitPattern)

Expand Down Expand Up @@ -385,7 +385,7 @@ func TestCheckingOutShallowCloneOfLocalGitProject(t *testing.T) {
}

// Mock out the meta-data calls to the agent after checkout
agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)
agent.Expect("meta-data", "exists", "buildkite:git:commit").AndExitWith(1)
agent.Expect("meta-data", "set", "buildkite:git:commit").WithStdin(commitPattern)

Expand All @@ -401,7 +401,7 @@ func TestCheckingOutSetsCorrectGitMetadataAndSendsItToBuildkite(t *testing.T) {
}
defer tester.Close()

agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)
agent.Expect("meta-data", "exists", "buildkite:git:commit").AndExitWith(1)
agent.Expect("meta-data", "set", "buildkite:git:commit").WithStdin(commitPattern)

Expand Down Expand Up @@ -514,7 +514,7 @@ func TestCleaningAnExistingCheckout(t *testing.T) {
}

// Mock out the meta-data calls to the agent after checkout
agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)
agent.
Expect("meta-data", "exists", "buildkite:git:commit").
AndExitWith(0)
Expand All @@ -535,7 +535,7 @@ func TestForcingACleanCheckout(t *testing.T) {
defer tester.Close()

// Mock out the meta-data calls to the agent after checkout
agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)
agent.
Expect("meta-data", "exists", "buildkite:git:commit").
AndExitWith(0)
Expand Down Expand Up @@ -564,7 +564,7 @@ func TestCheckoutOnAnExistingRepositoryWithoutAGitFolder(t *testing.T) {
t.Fatal(err)
}

agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)
agent.
Expect("meta-data", "exists", "buildkite:git:commit").
AndExitWith(0)
Expand Down Expand Up @@ -735,7 +735,7 @@ func TestGitMirrorEnv(t *testing.T) {
}

// Mock out the meta-data calls to the agent after checkout
agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)
agent.Expect("meta-data", "exists", "buildkite:git:commit").AndExitWith(1)
agent.Expect("meta-data", "set", "buildkite:git:commit").WithStdin(commitPattern)

Expand Down
2 changes: 1 addition & 1 deletion bootstrap/integration/command_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func TestPreExitHooksRunsAfterCommandFails(t *testing.T) {
defer tester.Close()

// Mock out the meta-data calls to the agent after checkout
agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)
agent.
Expect("meta-data", "exists", "buildkite:git:commit").
AndExitWith(0)
Expand Down
14 changes: 7 additions & 7 deletions bootstrap/integration/docker_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func TestRunningCommandWithDocker(t *testing.T) {
defer tester.Close()

// Mock out the meta-data calls to the agent after checkout
agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)
agent.
Expect("meta-data", "exists", "buildkite:git:commit").
AndExitWith(0)
Expand Down Expand Up @@ -57,7 +57,7 @@ func TestRunningCommandWithDockerAndCustomDockerfile(t *testing.T) {
defer tester.Close()

// Mock out the meta-data calls to the agent after checkout
agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)
agent.
Expect("meta-data", "exists", "buildkite:git:commit").
AndExitWith(0)
Expand Down Expand Up @@ -91,7 +91,7 @@ func TestRunningFailingCommandWithDocker(t *testing.T) {
defer tester.Close()

// Mock out the meta-data calls to the agent after checkout
agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)
agent.
Expect("meta-data", "exists", "buildkite:git:commit").
AndExitWith(0)
Expand Down Expand Up @@ -130,7 +130,7 @@ func TestRunningCommandWithDockerCompose(t *testing.T) {
defer tester.Close()

// Mock out the meta-data calls to the agent after checkout
agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)
agent.
Expect("meta-data", "exists", "buildkite:git:commit").
AndExitWith(0)
Expand Down Expand Up @@ -163,7 +163,7 @@ func TestRunningFailingCommandWithDockerCompose(t *testing.T) {
defer tester.Close()

// Mock out the meta-data calls to the agent after checkout
agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)
agent.
Expect("meta-data", "exists", "buildkite:git:commit").
AndExitWith(0)
Expand Down Expand Up @@ -203,7 +203,7 @@ func TestRunningCommandWithDockerComposeAndExtraConfig(t *testing.T) {
defer tester.Close()

// Mock out the meta-data calls to the agent after checkout
agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)
agent.
Expect("meta-data", "exists", "buildkite:git:commit").
AndExitWith(0)
Expand Down Expand Up @@ -237,7 +237,7 @@ func TestRunningCommandWithDockerComposeAndBuildAll(t *testing.T) {
defer tester.Close()

// Mock out the meta-data calls to the agent after checkout
agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)
agent.
Expect("meta-data", "exists", "buildkite:git:commit").
AndExitWith(0)
Expand Down
4 changes: 1 addition & 3 deletions bootstrap/integration/hooks_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,6 @@ func TestDirectoryPassesBetweenHooks(t *testing.T) {
}

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

tester, err := NewBootstrapTester()
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -367,7 +365,7 @@ func TestPreExitHooksFireAfterHookFailures(t *testing.T) {
}
defer tester.Close()

agent := tester.MustMock(t, "buildkite-agent")
agent := tester.MockAgent(t)

tester.ExpectGlobalHook(tc.failingHook).
Once().
Expand Down
1 change: 1 addition & 0 deletions bootstrap/shell/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ func NewTestShell(t *testing.T) *Shell {

if os.Getenv(`DEBUG_SHELL`) == "1" {
sh.Logger = TestingLogger{T: t}
sh.Writer = os.Stdout
}

// Windows requires certain env variables to be present
Expand Down
71 changes: 71 additions & 0 deletions clicommand/env.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package clicommand

import (
"encoding/json"
"fmt"
"os"
"strings"

"github.com/urfave/cli"
)

const envDescription = `Usage:
buildkite-agent env [options]

Description:
Prints out the environment of the current process as a JSON object, easily parsable by other programs. Used when
executing hooks to discover changes that hooks make to the environment.

Example:
$ buildkite-agent env

Prints the environment passed into the process
`

var EnvCommand = cli.Command{
Name: "env",
Usage: "Prints out the environment of the current process as a JSON object",
Description: envDescription,
Flags: []cli.Flag{
cli.BoolFlag{
Name: "pretty",
Usage: "Pretty print the JSON output",
EnvVar: "BUILDKITE_AGENT_ENV_PRETTY",
},
},
Action: func(c *cli.Context) error {
env := os.Environ()
envMap := make(map[string]string, len(env))

for _, e := range env {
k, v, _ := strings.Cut(e, "=")
envMap[k] = v
}

var (
envJSON []byte
err error
)

if c.Bool("pretty") {
envJSON, err = json.MarshalIndent(envMap, "", " ")
} else {
envJSON, err = json.Marshal(envMap)
}

// let's be polite to interactive shells etc.
envJSON = append(envJSON, '\n')

if err != nil {
fmt.Fprintf(os.Stderr, "Error marshalling JSON: %v\n", err)
os.Exit(1)
}

if _, err := os.Stdout.Write(envJSON); err != nil {
fmt.Fprintf(os.Stderr, "Error writing JSON to stdout: %v\n", err)
os.Exit(1)
}

return nil
},
}
Loading