Skip to content

Commit

Permalink
[Heartbeat] Add stringer redaction to synthexec cmd (#39535)
Browse files Browse the repository at this point in the history
Redact synthetics cmd output to prevent logging of user-defined parameters.

---------

Co-authored-by: Vignesh Shanmugam <[email protected]>
  • Loading branch information
emilioalvap and vigneshshanmugam authored May 16, 2024
1 parent fab441b commit 7306bd4
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 27 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ https://github.com/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff]
- Fix setuid root when running under cgroups v2. {pull}37794[37794]
- Adjust State loader to only retry when response code status is 5xx {pull}37981[37981]
- Reset prctl dumpable flag after cap drop. {pull}38269[38269]
- Redact synthexec cmd output. {pull}39535[39535]

*Heartbeat*

Expand Down
44 changes: 44 additions & 0 deletions x-pack/heartbeat/monitors/browser/synthexec/synthcmd.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
// or more contributor license agreements. Licensed under the Elastic License;
// you may not use this file except in compliance with the Elastic License.
//go:build linux || darwin || synthetics

package synthexec

import (
"fmt"
"os/exec"
"strings"
)

// Variant of exec.command with redacted params and playwright options,
// which might contain sensitive information.
type SynthCmd struct {
*exec.Cmd
}

func (cmd *SynthCmd) String() string {
b := new(strings.Builder)
b.WriteString(cmd.Path)
for i := 1; i < len(cmd.Args); i++ {
b.WriteString(" ")
a := cmd.Args[i]
switch a {
case "--params":
fallthrough
case "--playwright-options":
b.WriteString(fmt.Sprintf("%s { REDACTED }", a))
i++
default:
b.WriteString(a)
}
}

return b.String()
}

// Formatter override redacting params
func (cmd SynthCmd) Format(f fmt.State, verb rune) {

f.Write([]byte(cmd.String()))
}
66 changes: 66 additions & 0 deletions x-pack/heartbeat/monitors/browser/synthexec/synthcmd_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
// or more contributor license agreements. Licensed under the Elastic License;
// you may not use this file except in compliance with the Elastic License.

//go:build linux || synthetics

package synthexec

import (
"fmt"
"io"
"os"
"os/exec"
"testing"

"github.com/stretchr/testify/assert"
)

func TestSynthCmdStringOutput(t *testing.T) {
tests := []struct {
name string
stringer func(cmd SynthCmd) string
}{
{
name: "fmt.Sprintf",
stringer: func(cmd SynthCmd) string {
return fmt.Sprintf("%s", cmd)
},
},
{
name: "fmt.Println",
stringer: func(cmd SynthCmd) string {
r, w, err := os.Pipe()
assert.NoError(t, err)
fmt.Fprint(w, cmd)
w.Close()
defer r.Close()

o, err := io.ReadAll(r)
assert.NoError(t, err)

return string(o)
},
},
{
name: "cmd.String()",
stringer: func(cmd SynthCmd) string {
return cmd.String()
},
},
}

redacted := []string{"secret", "mysecrettoken", "auth", "mysecretauth"}
cmd := SynthCmd{
exec.Command("/nil", "--params", "{'secret':'mysecrettoken'}", "--playwright-options", "{'auth':'mysecretauth'}"),
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s := tt.stringer(cmd)
for _, r := range redacted {
assert.NotContains(t, s, r)
}
})
}
}
45 changes: 21 additions & 24 deletions x-pack/heartbeat/monitors/browser/synthexec/synthexec.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ type FilterJourneyConfig struct {
// This is practically just used by synthexec_unix.go to add Sysprocattrs
// It's still nice for devs to be able to test browser monitors on OSX
// where these are unsupported
var platformCmdMutate func(*exec.Cmd) = func(*exec.Cmd) {}
var platformCmdMutate func(*SynthCmd) = func(*SynthCmd) {}

var SynthexecTimeout struct{}

Expand All @@ -57,30 +57,30 @@ func ProjectJob(ctx context.Context, projectPath string, params mapstr.M, filter
return startCmdJob(ctx, cmdFactory, nil, params, filterJourneys, fields), nil
}

func projectCommandFactory(projectPath string, args ...string) (func() *exec.Cmd, error) {
func projectCommandFactory(projectPath string, args ...string) (func() *SynthCmd, error) {
npmRoot, err := getNpmRoot(projectPath)
if err != nil {
return nil, err
}

newCmd := func() *exec.Cmd {
newCmd := func() *SynthCmd {
bin := filepath.Join(npmRoot, "node_modules/.bin/elastic-synthetics")
// Always put the project path first to prevent conflation with variadic args!
// See https://github.com/tj/commander.js/blob/master/docs/options-taking-varying-arguments.md
// Note, we don't use the -- approach because it's cleaner to always know we can add new options
// to the end.
cmd := exec.Command(bin, append([]string{projectPath}, args...)...)
cmd.Dir = npmRoot
return cmd
return &SynthCmd{cmd}
}

return newCmd, nil
}

// InlineJourneyJob returns a job that runs the given source as a single journey.
func InlineJourneyJob(ctx context.Context, script string, params mapstr.M, fields stdfields.StdMonitorFields, extraArgs ...string) jobs.Job {
newCmd := func() *exec.Cmd {
return exec.Command("elastic-synthetics", append(extraArgs, "--inline")...) //nolint:gosec // we are safely building a command here, users can add args at their own risk
newCmd := func() *SynthCmd {
return &SynthCmd{exec.Command("elastic-synthetics", append(extraArgs, "--inline")...)} //nolint:gosec // we are safely building a command here, users can add args at their own risk
}

return startCmdJob(ctx, newCmd, &script, params, FilterJourneyConfig{}, fields)
Expand All @@ -89,7 +89,7 @@ func InlineJourneyJob(ctx context.Context, script string, params mapstr.M, field
// startCmdJob adapts commands into a heartbeat job. This is a little awkward given that the command's output is
// available via a sequence of events in the multiplexer, while heartbeat jobs are tail recursive continuations.
// Here, we adapt one to the other, where each recursive job pulls another item off the chan until none are left.
func startCmdJob(ctx context.Context, newCmd func() *exec.Cmd, stdinStr *string, params mapstr.M, filterJourneys FilterJourneyConfig, sFields stdfields.StdMonitorFields) jobs.Job {
func startCmdJob(ctx context.Context, newCmd func() *SynthCmd, stdinStr *string, params mapstr.M, filterJourneys FilterJourneyConfig, sFields stdfields.StdMonitorFields) jobs.Job {
return func(event *beat.Event) ([]jobs.Job, error) {
senr := newStreamEnricher(sFields)
mpx, err := runCmd(ctx, newCmd(), stdinStr, params, filterJourneys)
Expand Down Expand Up @@ -124,7 +124,7 @@ func readResultsJob(ctx context.Context, synthEvents <-chan *SynthEvent, enrich
// the params var as a CLI argument.
func runCmd(
ctx context.Context,
cmd *exec.Cmd,
cmd *SynthCmd,
stdinStr *string,
params mapstr.M,
filterJourneys FilterJourneyConfig,
Expand All @@ -151,12 +151,9 @@ func runCmd(
cmd.Args = append(cmd.Args, "--match", filterJourneys.Match)
}

// Variant of the command with no params, which could contain sensitive stuff
loggableCmd := exec.Command(cmd.Path, cmd.Args...) //nolint:gosec // we are safely building a command here...
if len(params) > 0 {
paramsBytes, _ := json.Marshal(params)
cmd.Args = append(cmd.Args, "--params", string(paramsBytes))
loggableCmd.Args = append(loggableCmd.Args, "--params", fmt.Sprintf("\"{%d hidden params}\"", len(params)))
}

// We need to pass both files in here otherwise we get a broken pipe, even
Expand All @@ -166,10 +163,10 @@ func runCmd(
// see the docs for ExtraFiles in https://golang.org/pkg/os/exec/#Cmd
cmd.Args = append(cmd.Args, "--outfd", "3")

logp.Info("Running command: %s in directory: '%s'", loggableCmd.String(), cmd.Dir)
logp.L().Info("Running command: %s in directory: '%s'", cmd, cmd.Dir)

if stdinStr != nil {
logp.Debug(debugSelector, "Using stdin str %s", *stdinStr)
logp.L().Debug(debugSelector, "Using stdin str %s", *stdinStr)
cmd.Stdin = strings.NewReader(*stdinStr)
}

Expand All @@ -184,7 +181,7 @@ func runCmd(
go func() {
err := scanToSynthEvents(stdoutPipe, stdoutToSynthEvent, mpx.writeSynthEvent)
if err != nil {
logp.Warn("could not scan stdout events from synthetics: %s", err)
logp.L().Warn("could not scan stdout events from synthetics: %s", err)
}

wg.Done()
Expand All @@ -198,7 +195,7 @@ func runCmd(
go func() {
err := scanToSynthEvents(stderrPipe, stderrToSynthEvent, mpx.writeSynthEvent)
if err != nil {
logp.Warn("could not scan stderr events from synthetics: %s", err)
logp.L().Warn("could not scan stderr events from synthetics: %s", err)
}
wg.Done()
}()
Expand Down Expand Up @@ -247,7 +244,7 @@ func runCmd(

err = <-cmdStarted
if err != nil {
logp.Warn("Could not start command %s: %s", cmd, err)
logp.L().Warn("Could not start command %s: %s", cmd, err)
return nil, err
}

Expand All @@ -264,27 +261,27 @@ func runCmd(

err := cmd.Process.Kill()
if err != nil {
logp.Warn("could not kill synthetics process: %s", err)
logp.L().Warn("could not kill synthetics process: %s", err)
}
}()

// Close mpx after the process is done and all events have been sent / consumed
go func() {
err := <-cmdDone
_ = jsonWriter.Close()
logp.Info("Command has completed(%d): %s", cmd.ProcessState.ExitCode(), loggableCmd.String())
logp.L().Info("Command has completed(%d): %s", cmd.ProcessState.ExitCode(), cmd)

var cmdError *SynthError = nil
if err != nil {
// err could be generic or it could have been killed by context timeout, log and check context
// to decide which error to stream
logp.Warn("Error executing command '%s' (%d): %s", loggableCmd.String(), cmd.ProcessState.ExitCode(), err)
logp.L().Warn("Error executing command '%s' (%d): %s", cmd, cmd.ProcessState.ExitCode(), err)

if errors.Is(ctx.Err(), context.DeadlineExceeded) {
timeout, _ := ctx.Value(SynthexecTimeout).(time.Duration)
cmdError = ECSErrToSynthError(ecserr.NewCmdTimeoutStatusErr(timeout, loggableCmd.String()))
cmdError = ECSErrToSynthError(ecserr.NewCmdTimeoutStatusErr(timeout, cmd.String()))
} else {
cmdError = ECSErrToSynthError(ecserr.NewBadCmdStatusErr(cmd.ProcessState.ExitCode(), loggableCmd.String()))
cmdError = ECSErrToSynthError(ecserr.NewBadCmdStatusErr(cmd.ProcessState.ExitCode(), cmd.String()))
}
}

Expand Down Expand Up @@ -315,7 +312,7 @@ func scanToSynthEvents(rdr io.ReadCloser, transform func(bytes []byte, text stri
for scanner.Scan() {
se, err := transform(scanner.Bytes(), scanner.Text())
if err != nil {
logp.Warn("error parsing line: %s for line: %s", err, scanner.Text())
logp.L().Warn("error parsing line: %s for line: %s", err, scanner.Text())
continue
}
if se != nil {
Expand All @@ -324,7 +321,7 @@ func scanToSynthEvents(rdr io.ReadCloser, transform func(bytes []byte, text stri
}

if scanner.Err() != nil {
logp.Warn("error scanning synthetics runner results %s", scanner.Err())
logp.L().Warn("error scanning synthetics runner results %s", scanner.Err())
return scanner.Err()
}

Expand All @@ -337,7 +334,7 @@ var stderrToSynthEvent = lineToSynthEventFactory(Stderr)
// lineToSynthEventFactory is a factory that can take a line from the scanner and transform it into a *SynthEvent.
func lineToSynthEventFactory(typ string) func(bytes []byte, text string) (res *SynthEvent, err error) {
return func(bytes []byte, text string) (res *SynthEvent, err error) {
logp.Info("%s: %s", typ, text)
logp.L().Info("%s: %s", typ, text)
return &SynthEvent{
Type: typ,
TimestampEpochMicros: float64(time.Now().UnixMicro()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package synthexec

import (
"os"
"os/exec"

"golang.org/x/sys/unix"

Expand All @@ -16,7 +15,7 @@ import (
)

func init() {
platformCmdMutate = func(cmd *exec.Cmd) {
platformCmdMutate = func(cmd *SynthCmd) {
logp.L().Warn("invoking node as:", security.NodeChildProcCred, " from: ", os.Getenv("HOME"))
// Note that while cmd.SysProcAtr takes a syscall.SysProcAttr object
// we are passing in a unix.SysProcAttr object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func runAndCollect(t *testing.T, cmd *exec.Cmd, stdinStr string, cmdTimeout time
cmd.Dir = filepath.Join(cwd, "testcmd")
ctx := context.WithValue(context.TODO(), SynthexecTimeout, cmdTimeout)

mpx, err := runCmd(ctx, cmd, &stdinStr, nil, FilterJourneyConfig{})
mpx, err := runCmd(ctx, &SynthCmd{cmd}, &stdinStr, nil, FilterJourneyConfig{})
require.NoError(t, err)

var synthEvents []*SynthEvent
Expand Down

0 comments on commit 7306bd4

Please sign in to comment.