Skip to content

Commit

Permalink
Merge pull request from GHSA-7v38-w32m-wx4m
Browse files Browse the repository at this point in the history 
* fix(secrets): add substitution field and honor allow_commands for entrypoint

* ditch substring detection in favor of strict byte replace for log masking

* replace all over replace -1
  • Loading branch information
@ecrupper
ecrupper committed Mar 12, 2024
1 parent bab8877 commit 2e046fc
Show file tree
Hide file tree
Showing 6 changed files with 210 additions and 123 deletions.
63 changes: 33 additions & 30 deletions database/secret.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,21 +42,22 @@ var (

// Secret is the database representation of a secret.
type Secret struct {
ID sql.NullInt64 `sql:"id"`
Org sql.NullString `sql:"org"`
Repo sql.NullString `sql:"repo"`
Team sql.NullString `sql:"team"`
Name sql.NullString `sql:"name"`
Value sql.NullString `sql:"value"`
Type sql.NullString `sql:"type"`
Images pq.StringArray `sql:"images" gorm:"type:varchar(1000)"`
Events pq.StringArray `sql:"events" gorm:"type:varchar(1000)"`
AllowEvents sql.NullInt64 `sql:"allow_events"`
AllowCommand sql.NullBool `sql:"allow_command"`
CreatedAt sql.NullInt64 `sql:"created_at"`
CreatedBy sql.NullString `sql:"created_by"`
UpdatedAt sql.NullInt64 `sql:"updated_at"`
UpdatedBy sql.NullString `sql:"updated_by"`
ID sql.NullInt64 `sql:"id"`
Org sql.NullString `sql:"org"`
Repo sql.NullString `sql:"repo"`
Team sql.NullString `sql:"team"`
Name sql.NullString `sql:"name"`
Value sql.NullString `sql:"value"`
Type sql.NullString `sql:"type"`
Images pq.StringArray `sql:"images" gorm:"type:varchar(1000)"`
Events pq.StringArray `sql:"events" gorm:"type:varchar(1000)"`
AllowEvents sql.NullInt64 `sql:"allow_events"`
AllowCommand sql.NullBool `sql:"allow_command"`
AllowSubstitution sql.NullBool `sql:"allow_substitution"`
CreatedAt sql.NullInt64 `sql:"created_at"`
CreatedBy sql.NullString `sql:"created_by"`
UpdatedAt sql.NullInt64 `sql:"updated_at"`
UpdatedBy sql.NullString `sql:"updated_by"`
}

// Decrypt will manipulate the existing secret value by
Expand Down Expand Up @@ -196,6 +197,7 @@ func (s *Secret) ToLibrary() *library.Secret {
secret.SetEvents(s.Events)
secret.SetAllowEvents(library.NewEventsFromMask(s.AllowEvents.Int64))
secret.SetAllowCommand(s.AllowCommand.Bool)
secret.SetAllowSubstitution(s.AllowSubstitution.Bool)
secret.SetCreatedAt(s.CreatedAt.Int64)
secret.SetCreatedBy(s.CreatedBy.String)
secret.SetUpdatedAt(s.UpdatedAt.Int64)
Expand Down Expand Up @@ -272,21 +274,22 @@ func (s *Secret) Validate() error {
// to a database Secret type.
func SecretFromLibrary(s *library.Secret) *Secret {
secret := &Secret{
ID: sql.NullInt64{Int64: s.GetID(), Valid: true},
Org: sql.NullString{String: s.GetOrg(), Valid: true},
Repo: sql.NullString{String: s.GetRepo(), Valid: true},
Team: sql.NullString{String: s.GetTeam(), Valid: true},
Name: sql.NullString{String: s.GetName(), Valid: true},
Value: sql.NullString{String: s.GetValue(), Valid: true},
Type: sql.NullString{String: s.GetType(), Valid: true},
Images: pq.StringArray(s.GetImages()),
Events: pq.StringArray(s.GetEvents()),
AllowEvents: sql.NullInt64{Int64: s.GetAllowEvents().ToDatabase(), Valid: true},
AllowCommand: sql.NullBool{Bool: s.GetAllowCommand(), Valid: true},
CreatedAt: sql.NullInt64{Int64: s.GetCreatedAt(), Valid: true},
CreatedBy: sql.NullString{String: s.GetCreatedBy(), Valid: true},
UpdatedAt: sql.NullInt64{Int64: s.GetUpdatedAt(), Valid: true},
UpdatedBy: sql.NullString{String: s.GetUpdatedBy(), Valid: true},
ID: sql.NullInt64{Int64: s.GetID(), Valid: true},
Org: sql.NullString{String: s.GetOrg(), Valid: true},
Repo: sql.NullString{String: s.GetRepo(), Valid: true},
Team: sql.NullString{String: s.GetTeam(), Valid: true},
Name: sql.NullString{String: s.GetName(), Valid: true},
Value: sql.NullString{String: s.GetValue(), Valid: true},
Type: sql.NullString{String: s.GetType(), Valid: true},
Images: pq.StringArray(s.GetImages()),
Events: pq.StringArray(s.GetEvents()),
AllowEvents: sql.NullInt64{Int64: s.GetAllowEvents().ToDatabase(), Valid: true},
AllowCommand: sql.NullBool{Bool: s.GetAllowCommand(), Valid: true},
AllowSubstitution: sql.NullBool{Bool: s.GetAllowSubstitution(), Valid: true},
CreatedAt: sql.NullInt64{Int64: s.GetCreatedAt(), Valid: true},
CreatedBy: sql.NullString{String: s.GetCreatedBy(), Valid: true},
UpdatedAt: sql.NullInt64{Int64: s.GetUpdatedAt(), Valid: true},
UpdatedBy: sql.NullString{String: s.GetUpdatedBy(), Valid: true},
}

return secret.Nullify()
Expand Down
33 changes: 18 additions & 15 deletions database/secret_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ func TestDatabase_Secret_ToLibrary(t *testing.T) {
want.SetEvents([]string{"push", "tag", "deployment"})
want.SetAllowEvents(library.NewEventsFromMask(1))
want.SetAllowCommand(true)
want.SetAllowSubstitution(true)
want.SetCreatedAt(tsCreate)
want.SetCreatedBy("octocat")
want.SetUpdatedAt(tsUpdate)
Expand Down Expand Up @@ -295,6 +296,7 @@ func TestDatabase_SecretFromLibrary(t *testing.T) {
s.SetEvents([]string{"push", "tag", "deployment"})
s.SetAllowEvents(library.NewEventsFromMask(1))
s.SetAllowCommand(true)
s.SetAllowSubstitution(true)
s.SetCreatedAt(tsCreate)
s.SetCreatedBy("octocat")
s.SetUpdatedAt(tsUpdate)
Expand All @@ -314,20 +316,21 @@ func TestDatabase_SecretFromLibrary(t *testing.T) {
// type with all fields set to a fake value.
func testSecret() *Secret {
return &Secret{
ID: sql.NullInt64{Int64: 1, Valid: true},
Org: sql.NullString{String: "github", Valid: true},
Repo: sql.NullString{String: "octocat", Valid: true},
Team: sql.NullString{String: "octokitties", Valid: true},
Name: sql.NullString{String: "foo", Valid: true},
Value: sql.NullString{String: "bar", Valid: true},
Type: sql.NullString{String: "repo", Valid: true},
Images: []string{"alpine"},
Events: []string{"push", "tag", "deployment"},
AllowEvents: sql.NullInt64{Int64: 1, Valid: true},
AllowCommand: sql.NullBool{Bool: true, Valid: true},
CreatedAt: sql.NullInt64{Int64: tsCreate, Valid: true},
CreatedBy: sql.NullString{String: "octocat", Valid: true},
UpdatedAt: sql.NullInt64{Int64: tsUpdate, Valid: true},
UpdatedBy: sql.NullString{String: "octocat2", Valid: true},
ID: sql.NullInt64{Int64: 1, Valid: true},
Org: sql.NullString{String: "github", Valid: true},
Repo: sql.NullString{String: "octocat", Valid: true},
Team: sql.NullString{String: "octokitties", Valid: true},
Name: sql.NullString{String: "foo", Valid: true},
Value: sql.NullString{String: "bar", Valid: true},
Type: sql.NullString{String: "repo", Valid: true},
Images: []string{"alpine"},
Events: []string{"push", "tag", "deployment"},
AllowEvents: sql.NullInt64{Int64: 1, Valid: true},
AllowCommand: sql.NullBool{Bool: true, Valid: true},
AllowSubstitution: sql.NullBool{Bool: true, Valid: true},
CreatedAt: sql.NullInt64{Int64: tsCreate, Valid: true},
CreatedBy: sql.NullString{String: "octocat", Valid: true},
UpdatedAt: sql.NullInt64{Int64: tsUpdate, Valid: true},
UpdatedBy: sql.NullString{String: "octocat2", Valid: true},
}
}
27 changes: 8 additions & 19 deletions library/log.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
package library

import (
"bytes"
"fmt"
"regexp"

"github.com/go-vela/types/constants"
)
Expand Down Expand Up @@ -45,25 +45,14 @@ func (l *Log) AppendData(data []byte) {
func (l *Log) MaskData(secrets []string) {
data := l.GetData()

// early exit on empty log or secret list
if len(data) == 0 || len(secrets) == 0 {
return
}

// byte replace data with masked logs
for _, secret := range secrets {
// escape regexp meta characters if they exist within value of secret
//
// https://pkg.go.dev/regexp#QuoteMeta
escaped := regexp.QuoteMeta(secret)

// create regexp to match secrets in the log data surrounded by regexp metacharacters
//
// https://pkg.go.dev/regexp#MustCompile
buffer := `(\s|^|=|"|\?|:|'|\.|,|&|$|;|\[|\])`
re := regexp.MustCompile((buffer + escaped + buffer))

// create a mask for the secret
mask := fmt.Sprintf("$1%s$2", constants.SecretLogMask)

// replace all regexp matches of secret with mask
//
// https://pkg.go.dev/regexp#Regexp.ReplaceAll
data = re.ReplaceAll(data, []byte(mask))
data = bytes.ReplaceAll(data, []byte(secret), []byte(constants.SecretLogMask))
}

// update data field to masked logs
Expand Down
82 changes: 53 additions & 29 deletions library/log_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,59 +42,83 @@ func TestLibrary_Log_AppendData(t *testing.T) {

func TestLibrary_Log_MaskData(t *testing.T) {
// set up test secrets
sVals := []string{"secret", "((%.YY245***pP.><@@}}", "littlesecret", "extrasecret"}

// set up test logs
s1 := "$ echo $NO_SECRET\nnosecret\n"
s2 := "((%.YY245***pP.><@@}}"
s2Masked := "***"
s3 := "$ echo $SECRET1\n((%.YY245***pP.><@@}}\n$ echo $SECRET2\nlittlesecret\n"
s3Masked := "$ echo $SECRET1\n***\n$ echo $SECRET2\n***\n"
s4 := "SOME_SECRET=((%.YY245***pP.><@@}}"
s4Masked := "SOME_SECRET=***"
s5 := "www.example.com?username=secret&password=extrasecret"
s5Masked := "www.example.com?username=***&password=***"
s6 := "[token: extrasecret]"
s6Masked := "[token: ***]"
sVals := []string{"gh_abc123def456", "((%.YY245***pP.><@@}}", "quick-bear-fox-squid", "SUPERSECRETVALUE"}

tests := []struct {
want []byte
log []byte
want []byte
secrets []string
}{
{ // no secrets in log
want: []byte(s1),
log: []byte(s1),
log: []byte(
"$ echo hello\nhello\n",
),
want: []byte(
"$ echo hello\nhello\n",
),
secrets: sVals,
},
{ // one secret in log
want: []byte(s2Masked),
log: []byte(s2),
log: []byte(
"((%.YY245***pP.><@@}}",
),
want: []byte(
"***",
),
secrets: sVals,
},
{ // multiple secrets in log
want: []byte(s3Masked),
log: []byte(s3),
log: []byte(
"$ echo $SECRET1\n((%.YY245***pP.><@@}}\n$ echo $SECRET2\nquick-bear-fox-squid\n",
),
want: []byte(
"$ echo $SECRET1\n***\n$ echo $SECRET2\n***\n",
),
secrets: sVals,
},
{ // secret with leading =
want: []byte(s4Masked),
log: []byte(s4),
log: []byte(
"SOME_SECRET=((%.YY245***pP.><@@}}",
),
want: []byte(
"SOME_SECRET=***",
),
secrets: sVals,
},
{ // secret baked in URL query params
want: []byte(s5Masked),
log: []byte(s5),
log: []byte(
"www.example.com?username=quick-bear-fox-squid&password=SUPERSECRETVALUE",
),
want: []byte(
"www.example.com?username=***&password=***",
),
secrets: sVals,
},
{ // secret in verbose brackets
want: []byte(s6Masked),
log: []byte(s6),
log: []byte(
"[token: gh_abc123def456]",
),
want: []byte(
"[token: ***]",
),
secrets: sVals,
},
{ // double secret
log: []byte(
"echo ${GITHUB_TOKEN}${SUPER_SECRET}\ngh_abc123def456SUPERSECRETVALUE\n",
),
want: []byte(
"echo ${GITHUB_TOKEN}${SUPER_SECRET}\n******\n",
),
secrets: sVals,
},
{ // empty secrets slice
want: []byte(s3),
log: []byte(s3),
log: []byte(
"echo hello\nhello\n",
),
want: []byte(
"echo hello\nhello\n",
),
secrets: []string{},
},
}
Expand Down
Loading

0 comments on commit 2e046fc

Please sign in to comment.