Skip to content

Commit

Permalink
feat: adding secrets field and refactoring the logging process. (ovh#667
Browse files Browse the repository at this point in the history
)

* adding secrets field and refactoring the logging process. Adding secrets in the context and creating a function that we can call to hide sensitive data

Signed-off-by: Fokion Sotiropoulos <[email protected]>
Co-authored-by: Yvonnick Esnault <[email protected]>
Signed-off-by: Ivan Velasco <[email protected]>
  • Loading branch information
2 people authored and ivan-velasco committed Sep 20, 2023
1 parent cc70634 commit 0b3e79f
Show file tree
Hide file tree
Showing 13 changed files with 129 additions and 20 deletions.
2 changes: 1 addition & 1 deletion cmd/venom/run/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ var (
v *venom.Venom

variables []string
secrets []string
format string = "xml" // Set the default value for formatFlag
varFiles []string
outputDir string
Expand Down Expand Up @@ -302,7 +303,6 @@ func displayArg(ctx context.Context) {
venom.Debug(ctx, "option outputDir=%v", outputDir)
venom.Debug(ctx, "option stopOnFailure=%v", stopOnFailure)
venom.Debug(ctx, "option htmlReport=%v", htmlReport)
venom.Debug(ctx, "option variables=%v", strings.Join(variables, " "))
venom.Debug(ctx, "option varFiles=%v", strings.Join(varFiles, " "))
venom.Debug(ctx, "option verbose=%v", verbose)
}
Expand Down
4 changes: 2 additions & 2 deletions executors/exec/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func (Executor) Run(ctx context.Context, step venom.TestStep) (interface{}, erro
return
}
result.Systemout += line
venom.Debug(ctx, line)
venom.Debug(ctx, venom.HideSensitive(ctx, line))
}
}()

Expand All @@ -173,7 +173,7 @@ func (Executor) Run(ctx context.Context, step venom.TestStep) (interface{}, erro
return
}
result.Systemerr += line
venom.Debug(ctx, line)
venom.Debug(ctx, venom.HideSensitive(ctx, line))
}
}()

Expand Down
20 changes: 20 additions & 0 deletions log.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ package venom

import (
"context"
"fmt"
"reflect"
"strings"
"testing"

"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -32,6 +35,23 @@ func fieldsFromContext(ctx context.Context, keys ...string) logrus.Fields {
return fields
}

// HideSensitive replace the value with __hidden__
func HideSensitive(ctx context.Context, arg interface{}) string {
s := ctx.Value(ContextKey("secrets"))
cleanVars := fmt.Sprint(arg)
if s != nil && &s != nil {
switch reflect.TypeOf(s).Kind() {
case reflect.Slice:
secrets := reflect.ValueOf(s)
for i := 0; i < secrets.Len(); i++ {
secret := fmt.Sprint(secrets.Index(i).Interface())
cleanVars = strings.ReplaceAll(cleanVars, secret, "__hidden__")
}
}
}
return cleanVars
}

func Debug(ctx context.Context, format string, args ...interface{}) {
fields := fieldsFromContext(ctx, fields...)
logger.WithFields(fields).Debugf(format, args...)
Expand Down
18 changes: 18 additions & 0 deletions log_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package venom

import (
"context"
"github.com/stretchr/testify/assert"
"testing"
)

func TestHideSensitive(t *testing.T) {
ctx := context.Background()
ctx = context.WithValue(ctx, ContextKey("secrets"), []string{"Joe", "Doe"})
assert.Equal(t, "__hidden__", HideSensitive(ctx, "Joe"))
assert.Equal(t, "__hidden__ tests something", HideSensitive(ctx, "Joe tests something"))
assert.Equal(t, "Dave tests something", HideSensitive(ctx, "Dave tests something"))
assert.Equal(t, "1234", HideSensitive(ctx, 1234))
assert.Equal(t, "__hidden__!", HideSensitive(ctx, "Doe!"))
assert.Equal(t, "__hidden__ __hidden__", HideSensitive(ctx, "Joe Doe"))
}
6 changes: 4 additions & 2 deletions process.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,16 +95,18 @@ func (v *Venom) Parse(ctx context.Context, path []string) error {
ts := &v.Tests.TestSuites[i]
ts.Vars.Add("venom.testsuite", ts.Name)

Info(ctx, "Parsing testsuite %s : %+v", ts.Filepath, ts.Vars)
Info(ctx, "Parsing testsuite %s", ts.Filepath)
tvars, textractedVars, err := v.parseTestSuite(ts)
if err != nil {
return err
}

Debug(ctx, "Testsuite (%s) variables: %+v", ts.Filepath, ts.Vars)
for k := range ts.Vars {
textractedVars = append(textractedVars, k)
}

Debug(ctx, "Testsuite (%s) variables: %s", ts.Filepath, strings.Join(textractedVars, ","))

for _, k := range tvars {
var found bool
for i := 0; i < len(missingVars); i++ {
Expand Down
2 changes: 2 additions & 0 deletions process_files.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,14 @@ func (v *Venom) readFiles(ctx context.Context, filesPath []string) (err error) {
Name: testSuiteInput.Name,
TestCases: make([]TestCase, len(testSuiteInput.TestCases)),
Vars: testSuiteInput.Vars,
Secrets: testSuiteInput.Secrets,
}
for i := range testSuiteInput.TestCases {
ts.TestCases[i] = TestCase{
TestCaseInput: testSuiteInput.TestCases[i],
}
}
Info(ctx, "Has %d Secrets", len(ts.Secrets))

// Default workdir is testsuite directory
ts.WorkDir, err = filepath.Abs(filepath.Dir(filePath))
Expand Down
27 changes: 19 additions & 8 deletions process_testcase.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func (v *Venom) parseTestCase(ts *TestSuite, tc *TestCase) ([]string, []string,
vars := []string{}
extractedVars := []string{}

// the value of each var can contains a double-quote -> "
// the value of each var can contain a double-quote -> "
// if the value is not escaped, it will be used as is, and the json sent to unmarshall will be incorrect.
// This also avoids injections into the json structure of a step
for i := range dvars {
Expand Down Expand Up @@ -67,7 +67,7 @@ func (v *Venom) parseTestCase(ts *TestSuite, tc *TestCase) ([]string, []string,
}
extractedVars = append(extractedVars, tc.Name+"."+k)
if strings.HasSuffix(k, "__type__") && dumpE[k] == "Map" {
// go-dump doesnt dump the map name, here is a workaround
// go-dump doesn't dump the map name, here is a workaround
k = strings.TrimSuffix(k, "__type__")
extractedVars = append(extractedVars, tc.Name+"."+k)
}
Expand Down Expand Up @@ -134,23 +134,34 @@ func (v *Venom) parseTestCase(ts *TestSuite, tc *TestCase) ([]string, []string,

func (v *Venom) runTestCase(ctx context.Context, ts *TestSuite, tc *TestCase) {
ctx = context.WithValue(ctx, ContextKey("testcase"), tc.Name)

tc.TestSuiteVars = ts.Vars.Clone()
tc.Vars = ts.Vars.Clone()
tc.Vars.Add("venom.testcase", tc.Name)
tc.Vars.AddAll(ts.ComputedVars)
tc.computedVars = H{}

Info(ctx, "Starting testcase")
ctx = v.processSecrets(ctx, ts, tc)

for k, v := range tc.Vars {
Debug(ctx, "Running testcase with variable %s: %+v", k, v)
}
Info(ctx, "Starting testcase")

defer Info(ctx, "Ending testcase")
// ##### RUN Test Steps Here
v.runTestSteps(ctx, tc, nil)
}

func (v *Venom) processSecrets(ctx context.Context, ts *TestSuite, tc *TestCase) context.Context {
computedSecrets := []string{}
for k, v := range tc.Vars {
for _, s := range ts.Secrets {
if strings.Compare(k, s) == 0 {
computedSecrets = append(computedSecrets, fmt.Sprint(v))
}
}
}
return context.WithValue(ctx, ContextKey("secrets"), computedSecrets)
}

func (v *Venom) runTestSteps(ctx context.Context, tc *TestCase, tsIn *TestStepResult) {
results, err := testConditionalStatement(ctx, tc, tc.Skip, tc.Vars, "skipping testcase %q: %v")
if err != nil {
Expand Down Expand Up @@ -240,9 +251,9 @@ func (v *Venom) runTestSteps(ctx context.Context, tc *TestCase, tsIn *TestStepRe
}

if ranged.Enabled {
Info(ctx, "Step #%d-%d content is: %s", stepNumber, rangedIndex, content)
Info(ctx, "Step #%d-%d content is: %s", stepNumber, rangedIndex, HideSensitive(ctx, content))
} else {
Info(ctx, "Step #%d content is: %s", stepNumber, content)
Info(ctx, "Step #%d content is: %s", stepNumber, HideSensitive(ctx, content))
}

data, err := yaml.Marshal(rawStep)
Expand Down
2 changes: 1 addition & 1 deletion process_teststep.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func (v *Venom) RunTestStep(ctx context.Context, e ExecutorRunner, tc *TestCase,
continue
}

Debug(ctx, "result of runTestStepExecutor: %+v", result)
Debug(ctx, "result of runTestStepExecutor: %+v", HideSensitive(ctx, result))
mapResult := GetExecutorResult(result)
mapResultString, _ := DumpString(result)

Expand Down
7 changes: 5 additions & 2 deletions process_testsuite.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (v *Venom) runTestSuite(ctx context.Context, ts *TestSuite) error {
}
}

// Intialiaze the testsuite variables and compute a first interpolation over them
// Initialize the testsuite variables and compute a first interpolation over them
ts.Vars.AddAll(v.variables.Clone())
vars, _ := DumpStringPreserveCase(ts.Vars)
for k, v := range vars {
Expand Down Expand Up @@ -65,7 +65,10 @@ func (v *Venom) runTestSuite(ctx context.Context, ts *TestSuite) error {
}

ts.Status = StatusRun

Info(ctx, "With secrets in testsuite")
for _, v := range ts.Secrets {
Info(ctx, "secret %+v", v)
}
// ##### RUN Test Cases Here
v.runTestCases(ctx, ts)

Expand Down
11 changes: 11 additions & 0 deletions tests/hide_secrets.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
name: Hide Secret
vars:
name : Tester
secrets:
- name
testcases:
- name: myvar_first
steps:
- type: exec
script: "echo myvar {{.name}}"

2 changes: 2 additions & 0 deletions types.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,14 @@ type TestSuiteInput struct {
Name string `json:"name" yaml:"name"`
TestCases []TestCaseInput `json:"testcases" yaml:"testcases"`
Vars H `json:"vars" yaml:"vars"`
Secrets []string `json:"secrets" yaml:"secrets"`
}

type TestSuite struct {
Name string `json:"name" yaml:"name"`
TestCases []TestCase `json:"testcases" yaml:"testcases"`
Vars H `json:"vars" yaml:"vars"`
Secrets []string `json:"secrets" yaml:"secrets"`

// computed
ShortName string `json:"shortname" yaml:"-"`
Expand Down
8 changes: 8 additions & 0 deletions venom.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ func New() *Venom {
executorsUser: map[string]Executor{},
executorFileCache: map[string][]byte{},
variables: map[string]interface{}{},
secrets: map[string]interface{}{},
OutputFormat: "xml",
}
return v
Expand All @@ -64,6 +65,7 @@ type Venom struct {

Tests Tests
variables H
secrets H

LibDir string
OutputFormat string
Expand Down Expand Up @@ -97,6 +99,12 @@ func (v *Venom) AddVariables(variables map[string]interface{}) {
}
}

func (v *Venom) AddSecrets(secrets map[string]interface{}) {
for k, s := range secrets {
v.secrets[k] = s
}
}

// RegisterExecutorBuiltin register builtin executors
func (v *Venom) RegisterExecutorBuiltin(name string, e Executor) {
v.executorsBuiltin[name] = e
Expand Down
40 changes: 36 additions & 4 deletions venom_output.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package venom

import (
"bytes"
"context"
"encoding/json"
"encoding/xml"
"fmt"
Expand All @@ -23,12 +24,41 @@ func init() {
}
}

// CleanUpSecrets This method tries to hide all the sensitive variables
func (v *Venom) CleanUpSecrets(testSuite TestSuite) TestSuite {
for _, testCase := range testSuite.TestCases {
ctx := v.processSecrets(context.Background(), &testSuite, &testCase)
for _, result := range testCase.TestStepResults {
for k, v := range result.ComputedVars {
if !strings.HasPrefix(k, "venom.") {
result.ComputedVars[k] = HideSensitive(ctx, v)
}
}
for k, v := range result.InputVars {
if !strings.HasPrefix(k, "venom.") {
result.InputVars[k] = HideSensitive(ctx, v)
}
}
for k, v := range testCase.TestCaseInput.Vars {
if !strings.HasPrefix(k, "venom.") {
testCase.TestCaseInput.Vars[k] = HideSensitive(ctx, v)
}
}
result.Raw = HideSensitive(ctx, fmt.Sprint(result.Raw))
result.Interpolated = HideSensitive(ctx, fmt.Sprint(result.Interpolated))
result.Systemout = HideSensitive(ctx, result.Systemout)
result.Systemerr = HideSensitive(ctx, result.Systemerr)
}
}
return testSuite
}

// OutputResult output result to sdtout, files...
func (v *Venom) OutputResult() error {
if v.OutputDir == "" {
return nil
}

cleanedTs := []TestSuite{}
for i := range v.Tests.TestSuites {
tcFiltered := []TestCase{}
for _, tc := range v.Tests.TestSuites[i].TestCases {
Expand All @@ -37,9 +67,11 @@ func (v *Venom) OutputResult() error {
}
}
v.Tests.TestSuites[i].TestCases = tcFiltered
ts := v.CleanUpSecrets(v.Tests.TestSuites[i])
cleanedTs = append(cleanedTs, ts)

testsResult := &Tests{
TestSuites: []TestSuite{v.Tests.TestSuites[i]},
TestSuites: []TestSuite{ts},
Status: v.Tests.Status,
NbTestsuitesFail: v.Tests.NbTestsuitesFail,
NbTestsuitesPass: v.Tests.NbTestsuitesPass,
Expand Down Expand Up @@ -77,7 +109,7 @@ func (v *Venom) OutputResult() error {
return errors.New("Error: you have to use the --html-report flag")
}

fname := strings.TrimSuffix(v.Tests.TestSuites[i].Filepath, filepath.Ext(v.Tests.TestSuites[i].Filepath))
fname := strings.TrimSuffix(ts.Filepath, filepath.Ext(ts.Filepath))
fname = strings.ReplaceAll(fname, "/", "_")
filename := path.Join(v.OutputDir, "test_results_"+fname+"."+v.OutputFormat)
if err := os.WriteFile(filename, data, 0600); err != nil {
Expand All @@ -88,7 +120,7 @@ func (v *Venom) OutputResult() error {

if v.HtmlReport {
testsResult := &Tests{
TestSuites: v.Tests.TestSuites,
TestSuites: cleanedTs,
Status: v.Tests.Status,
NbTestsuitesFail: v.Tests.NbTestsuitesFail,
NbTestsuitesPass: v.Tests.NbTestsuitesPass,
Expand Down

0 comments on commit 0b3e79f

Please sign in to comment.