Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
53 changes: 51 additions & 2 deletions executor/linux/step.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"context"
"fmt"
"io/ioutil"
"regexp"
"strings"
"time"

"github.com/go-vela/types/constants"
Expand Down Expand Up @@ -214,6 +216,8 @@ func (c *client) StreamStep(ctx context.Context, ctn *pipeline.Container) error
return err
}

secretValues := getSecretValues(ctn)

defer func() {
// tail the runtime container
rc, err := c.Runtime.TailContainer(ctx, ctn)
Expand All @@ -239,6 +243,9 @@ func (c *client) StreamStep(ctx context.Context, ctn *pipeline.Container) error
return
}

// mask secrets in logs before setting them in the database.
data = maskSecrets(data, secretValues)

// overwrite the existing log with all bytes
//
// https://pkg.go.dev/github.com/go-vela/types/library?tab=doc#Log.SetData
Expand Down Expand Up @@ -305,7 +312,10 @@ func (c *client) StreamStep(ctx context.Context, ctn *pipeline.Container) error
// update the existing log with the new bytes
//
// https://pkg.go.dev/github.com/go-vela/types/library?tab=doc#Log.AppendData
_log.AppendData(logs.Bytes())

data := maskSecrets(logs.Bytes(), secretValues)

_log.AppendData(data)

logger.Debug("appending logs")
// send API call to append the logs for the step
Expand Down Expand Up @@ -362,10 +372,13 @@ func (c *client) StreamStep(ctx context.Context, ctn *pipeline.Container) error
if logs.Len() > 1000 {
logger.Trace(logs.String())

// mask secrets before updating logs
data := maskSecrets(logs.Bytes(), secretValues)

// update the existing log with the new bytes
//
// https://pkg.go.dev/github.com/go-vela/types/library?tab=doc#Log.AppendData
_log.AppendData(logs.Bytes())
_log.AppendData(data)

logger.Debug("appending logs")
// send API call to append the logs for the step
Expand Down Expand Up @@ -438,3 +451,39 @@ func (c *client) DestroyStep(ctx context.Context, ctn *pipeline.Container) error

return nil
}

// getSecretValues is a helper function that creates a slice of
// secret values that will be used to mask secrets in logs before
// updating the database.
func getSecretValues(ctn *pipeline.Container) []string {
secretValues := []string{}
// gather secrets' values from the environment map for masking
for _, secret := range ctn.Secrets {
s := ctn.Environment[strings.ToUpper(secret.Target)]
// handle multi line secrets from files
s = strings.ReplaceAll(s, "\n", " ")

// drop any trailing spaces
if strings.HasSuffix(s, " ") {
s = s[:(len(s) - 1)]
}
secretValues = append(secretValues, s)
}
return secretValues
}

// maskSecrets is a helper function that takes in a byte array
// and a slice of secret values to mask.
func maskSecrets(log []byte, secrets []string) []byte {
strData := string(log)
for _, secret := range secrets {
re := regexp.MustCompile(`\s` + secret + `\s`)
matches := re.FindAllString(strData, -1)
for _, match := range matches {
mask := string(match[0]) + constants.SecretLogMask + string(match[len(match)-1])
strData = strings.Replace(strData, match, mask, 1)
}
strData = re.ReplaceAllString(strData, constants.SecretLogMask)
}
return []byte(strData)
}
Comment on lines +475 to +489
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we think this should be a method on the github.com/go-vela/library/Log{} type?

Thinking the function signature could look like:

func (l *Log) MaskData(secrets []string) {}

Then, to use this function in the the code above:

// update the existing log with the new bytes
//
// https://pkg.go.dev/github.com/go-vela/types/library?tab=doc#Log.AppendData
_log.AppendData(logs.Bytes())

// mask any secrets in the log data
//
// https://pkg.go.dev/github.com/go-vela/types/library?tab=doc#Log.MaskData
_log.MaskData(secretValues)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That might be helpful in the eventual effort to have this masking apply to external secrets

Copy link
Contributor

Choose a reason for hiding this comment

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

You read my mind 👍 I was thinking along those same lines!

Also, if/when we write out other executors then we don't have to duplicate code.

In the interest of not blocking the functionality, I'll approve for now 😄

134 changes: 134 additions & 0 deletions executor/linux/step_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ package linux

import (
"context"
"io/ioutil"
"net/http/httptest"
"reflect"
"testing"

"github.com/gin-gonic/gin"
Expand Down Expand Up @@ -338,6 +340,7 @@ func TestLinux_StreamStep(t *testing.T) {
}

_runtime, err := docker.NewMock()

if err != nil {
t.Errorf("unable to create runtime engine: %v", err)
}
Expand Down Expand Up @@ -522,3 +525,134 @@ func TestLinux_DestroyStep(t *testing.T) {
}
}
}

func TestLinux_getSecretValues(t *testing.T) {
fileSecret, err := ioutil.ReadFile("./testdata/step/secret_text.txt")
if err != nil {
t.Errorf("unable to read from test data file secret. Err: %v", err)
}
tests := []struct {
want []string
container *pipeline.Container
}{
{ // no secrets container
want: []string{},
container: &pipeline.Container{
ID: "step_github_octocat_1_init",
Directory: "/vela/src/github.com/github/octocat",
Environment: map[string]string{"FOO": "bar"},
Image: "#init",
Name: "init",
Number: 1,
Pull: "not_present",
},
},
{ // secrets container
want: []string{"secretUser", "secretPass"},
container: &pipeline.Container{
ID: "step_github_octocat_1_echo",
Directory: "/vela/src/github.com/github/octocat",
Environment: map[string]string{
"FOO": "bar",
"SECRET_USERNAME": "secretUser",
"SECRET_PASSWORD": "secretPass",
},
Image: "alpine:latest",
Name: "echo",
Number: 1,
Pull: "not_present",
Secrets: pipeline.StepSecretSlice{
{
Source: "someSource",
Target: "secret_username",
},
{
Source: "someOtherSource",
Target: "secret_password",
},
},
},
},
{ // secrets container with file as value
want: []string{"secretUser", "this is a secret"},
container: &pipeline.Container{
ID: "step_github_octocat_1_ignorenotfound",
Directory: "/vela/src/github.com/github/octocat",
Environment: map[string]string{
"FOO": "bar",
"SECRET_USERNAME": "secretUser",
"SECRET_PASSWORD": string(fileSecret),
},
Image: "alpine:latest",
Name: "ignorenotfound",
Number: 1,
Pull: "not_present",
Secrets: pipeline.StepSecretSlice{
{
Source: "someSource",
Target: "secret_username",
},
{
Source: "someOtherSource",
Target: "secret_password",
},
},
},
},
}
// run tests
for _, test := range tests {
got := getSecretValues(test.container)

if !reflect.DeepEqual(got, test.want) {
t.Errorf("getSecretValues is %v, want %v", got, test.want)
}
}
}

func TestLinux_maskSecrets(t *testing.T) {
// set up test secrets
sVals := []string{"secret", "bigsecret", "littlesecret", "extrasecret"}

// set up test logs
s1 := "$ echo $NO_SECRET\nnosecret\n"
s2 := "$ echo $SECRET\nbigsecret\n"
s2Masked := "$ echo $SECRET\n***\n"
s3 := "$ echo $SECRET1\nbigsecret\n$ echo $SECRET2\nlittlesecret\n"
s3Masked := "$ echo $SECRET1\n***\n$ echo $SECRET2\n***\n"

tests := []struct {
want []byte
log []byte
secrets []string
}{
{ // no secrets in log
want: []byte(s1),
log: []byte(s1),
secrets: sVals,
},
{ // one secret in log
want: []byte(s2Masked),
log: []byte(s2),
secrets: sVals,
},
{ // multiple secrets in log
want: []byte(s3Masked),
log: []byte(s3),
secrets: sVals,
},
{ // empty secrets slice
want: []byte(s3),
log: []byte(s3),
secrets: []string{},
},
}
// run tests
for _, test := range tests {
got := maskSecrets(test.log, test.secrets)

if !reflect.DeepEqual(got, test.want) {
t.Errorf("maskSecrets is %v, want %v", string(got), string(test.want))
}
}
}
4 changes: 4 additions & 0 deletions executor/linux/testdata/step/secret_text.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
this
is
a
secret
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ require (
github.com/gin-gonic/gin v1.7.7
github.com/go-vela/sdk-go v0.11.0
github.com/go-vela/server v0.11.1-0.20211213155322-eeba06d5ce06
github.com/go-vela/types v0.11.1-0.20211117152001-4dc404f4aabc
github.com/go-vela/types v0.11.1-0.20211221194436-28210cfa70c9
github.com/google/go-cmp v0.5.6
github.com/joho/godotenv v1.4.0
github.com/opencontainers/image-spec v1.0.2
Expand Down
3 changes: 2 additions & 1 deletion go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,9 @@ github.com/go-vela/server v0.11.0/go.mod h1:0phuhEP09iKIiNKpO+cfOa6qU+epgr9Oon1M
github.com/go-vela/server v0.11.1-0.20211213155322-eeba06d5ce06 h1:5a2t2rh2/zD/+NMVrGw9UyILryaF9naG4cGF+WDWuj4=
github.com/go-vela/server v0.11.1-0.20211213155322-eeba06d5ce06/go.mod h1:CG7MFRFVZ4s2ov4B2XRJles4R+vLD+3AMQw7O9qzk1c=
github.com/go-vela/types v0.11.0/go.mod h1:8Oml/G1ATFTJsKdsIsstUuHVLsUv7pl6+EiIyOaUqH0=
github.com/go-vela/types v0.11.1-0.20211117152001-4dc404f4aabc h1:5BJtsCPpi0jlw9XEFafcUo/u+kdEA7N9bp7bBW9/hjA=
github.com/go-vela/types v0.11.1-0.20211117152001-4dc404f4aabc/go.mod h1:W00S1BayYQhCVqI4GuuhGjg173MOfU9UvK3JEDCr1aw=
github.com/go-vela/types v0.11.1-0.20211221194436-28210cfa70c9 h1:wvbQB5W9P5F9etlG3T0bLfJd8Ct/SYWnCKCysqF6n1w=
github.com/go-vela/types v0.11.1-0.20211221194436-28210cfa70c9/go.mod h1:W00S1BayYQhCVqI4GuuhGjg173MOfU9UvK3JEDCr1aw=
github.com/gofrs/uuid v4.0.0+incompatible/go.mod h1:b2aQJv3Z4Fp6yNu3cdSllBxTCLRxnplIgP/c0N/04lM=
github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ=
github.com/gogo/protobuf v1.3.2 h1:Ov1cvc58UF3b5XjBnZv7+opcTcQFZebYjWzi34vdm4Q=
Expand Down