From a1f77e01e5b05bdd304cd686db5e9915e8cb9e66 Mon Sep 17 00:00:00 2001 From: Lucas Manuel Rodriguez Date: Thu, 9 May 2024 12:29:16 -0300 Subject: [PATCH] Add support for escaping $ in gitops yamls (#18845) #18467 - [X] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://fleetdm.com/docs/contributing/committing-changes#changes-files) for more information. - ~[ ] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements)~ - ~[ ] Added support on fleet's osquery simulator `cmd/osquery-perf` for new osquery data ingestion features.~ - [X] Added/updated tests - ~[ ] If database migrations are included, checked table schema to confirm autoupdate~ - ~For database migrations:~ - ~[ ] Checked schema for all modified table for columns that will auto-update timestamps during migration.~ - ~[ ] Confirmed that updating the timestamps is acceptable, and will not cause unwanted side effects.~ - ~[ ] Ensured the correct collation is explicitly set for character columns (`COLLATE utf8mb4_unicode_ci`).~ - [X] Manual QA for all new/changed functionality - ~For Orbit and Fleet Desktop changes:~ - ~[ ] Manual QA must be performed in the three main OSs, macOS, Windows and Linux.~ - ~[ ] Auto-update manual QA, from released version of component to new version (see [tools/tuf/test](../tools/tuf/test/README.md)).~ --- .../18467-provide-way-to-escape-env-variables | 1 + cmd/fleetctl/gitops.go | 14 +- pkg/spec/gitops.go | 217 +++++++++++------- pkg/spec/gitops_test.go | 158 +++++++++---- pkg/spec/spec.go | 60 +++++ pkg/spec/spec_test.go | 81 +++++++ 6 files changed, 397 insertions(+), 134 deletions(-) create mode 100644 changes/18467-provide-way-to-escape-env-variables diff --git a/changes/18467-provide-way-to-escape-env-variables b/changes/18467-provide-way-to-escape-env-variables new file mode 100644 index 0000000000..2e009e89ed --- /dev/null +++ b/changes/18467-provide-way-to-escape-env-variables @@ -0,0 +1 @@ +* Add support for escaping `$` (with `\`) in gitops yaml files. diff --git a/cmd/fleetctl/gitops.go b/cmd/fleetctl/gitops.go index dfa6836ea5..d24ea8b1ba 100644 --- a/cmd/fleetctl/gitops.go +++ b/cmd/fleetctl/gitops.go @@ -3,16 +3,16 @@ package main import ( "errors" "fmt" + "path/filepath" + "slices" + "strings" + "github.com/fleetdm/fleet/v4/pkg/spec" "github.com/fleetdm/fleet/v4/server/fleet" "github.com/fleetdm/fleet/v4/server/ptr" "github.com/fleetdm/fleet/v4/server/service" "github.com/urfave/cli/v2" "golang.org/x/text/unicode/norm" - "os" - "path/filepath" - "slices" - "strings" ) func gitopsCommand() *cli.Command { @@ -82,12 +82,8 @@ func gitopsCommand() *cli.Command { firstFileMustBeGlobal = ptr.Bool(true) } for _, flFilename := range flFilenames.Value() { - b, err := os.ReadFile(flFilename) - if err != nil { - return err - } baseDir := filepath.Dir(flFilename) - config, err := spec.GitOpsFromBytes(b, baseDir) + config, err := spec.GitOpsFromFile(flFilename, baseDir) if err != nil { return err } diff --git a/pkg/spec/gitops.go b/pkg/spec/gitops.go index fe5210caef..5e6f040a99 100644 --- a/pkg/spec/gitops.go +++ b/pkg/spec/gitops.go @@ -4,14 +4,15 @@ import ( "encoding/json" "errors" "fmt" - "github.com/fleetdm/fleet/v4/server/fleet" - "github.com/ghodss/yaml" - "github.com/hashicorp/go-multierror" - "golang.org/x/text/unicode/norm" "os" "path/filepath" "slices" "unicode" + + "github.com/fleetdm/fleet/v4/server/fleet" + "github.com/ghodss/yaml" + "github.com/hashicorp/go-multierror" + "golang.org/x/text/unicode/norm" ) type BaseItem struct { @@ -56,9 +57,19 @@ type GitOps struct { } // GitOpsFromBytes parses a GitOps yaml file. -func GitOpsFromBytes(b []byte, baseDir string) (*GitOps, error) { +func GitOpsFromFile(filePath, baseDir string) (*GitOps, error) { + b, err := os.ReadFile(filePath) + if err != nil { + return nil, fmt.Errorf("failed to read file: %s: %w", filePath, err) + } + + // Replace $var and ${var} with env values. + b, err = ExpandEnvBytes(b) + if err != nil { + return nil, fmt.Errorf("failed to expand environment in file %s: %w", filePath, err) + } + var top map[string]json.RawMessage - b = []byte(os.ExpandEnv(string(b))) // replace $var and ${var} with env values if err := yaml.Unmarshal(b, &top); err != nil { return nil, fmt.Errorf("failed to unmarshal file %w: \n", err) } @@ -124,22 +135,30 @@ func parseOrgSettings(raw json.RawMessage, result *GitOps, baseDir string, multi noError = false multiError = multierror.Append(multiError, fmt.Errorf("failed to read org settings file %s: %v", *orgSettingsTop.Path, err)) } else { - fileBytes = []byte(os.ExpandEnv(string(fileBytes))) - var pathOrgSettings BaseItem - if err := yaml.Unmarshal(fileBytes, &pathOrgSettings); err != nil { + // Replace $var and ${var} with env values. + fileBytes, err = ExpandEnvBytes(fileBytes) + if err != nil { noError = false multiError = multierror.Append( - multiError, fmt.Errorf("failed to unmarshal org settings file %s: %v", *orgSettingsTop.Path, err), + multiError, fmt.Errorf("failed to expand environment in file %s: %v", *orgSettingsTop.Path, err), ) } else { - if pathOrgSettings.Path != nil { + var pathOrgSettings BaseItem + if err := yaml.Unmarshal(fileBytes, &pathOrgSettings); err != nil { noError = false multiError = multierror.Append( - multiError, - fmt.Errorf("nested paths are not supported: %s in %s", *pathOrgSettings.Path, *orgSettingsTop.Path), + multiError, fmt.Errorf("failed to unmarshal org settings file %s: %v", *orgSettingsTop.Path, err), ) } else { - raw = fileBytes + if pathOrgSettings.Path != nil { + noError = false + multiError = multierror.Append( + multiError, + fmt.Errorf("nested paths are not supported: %s in %s", *pathOrgSettings.Path, *orgSettingsTop.Path), + ) + } else { + raw = fileBytes + } } } } @@ -168,22 +187,30 @@ func parseTeamSettings(raw json.RawMessage, result *GitOps, baseDir string, mult noError = false multiError = multierror.Append(multiError, fmt.Errorf("failed to read team settings file %s: %v", *teamSettingsTop.Path, err)) } else { - fileBytes = []byte(os.ExpandEnv(string(fileBytes))) - var pathTeamSettings BaseItem - if err := yaml.Unmarshal(fileBytes, &pathTeamSettings); err != nil { + // Replace $var and ${var} with env values. + fileBytes, err = ExpandEnvBytes(fileBytes) + if err != nil { noError = false multiError = multierror.Append( - multiError, fmt.Errorf("failed to unmarshal team settings file %s: %v", *teamSettingsTop.Path, err), + multiError, fmt.Errorf("failed to expand environment in file %s: %v", *teamSettingsTop.Path, err), ) } else { - if pathTeamSettings.Path != nil { + var pathTeamSettings BaseItem + if err := yaml.Unmarshal(fileBytes, &pathTeamSettings); err != nil { noError = false multiError = multierror.Append( - multiError, - fmt.Errorf("nested paths are not supported: %s in %s", *pathTeamSettings.Path, *teamSettingsTop.Path), + multiError, fmt.Errorf("failed to unmarshal team settings file %s: %v", *teamSettingsTop.Path, err), ) } else { - raw = fileBytes + if pathTeamSettings.Path != nil { + noError = false + multiError = multierror.Append( + multiError, + fmt.Errorf("nested paths are not supported: %s in %s", *pathTeamSettings.Path, *teamSettingsTop.Path), + ) + } else { + raw = fileBytes + } } } } @@ -264,27 +291,34 @@ func parseAgentOptions(top map[string]json.RawMessage, result *GitOps, baseDir s if err != nil { return multierror.Append(multiError, fmt.Errorf("failed to read agent options file %s: %v", *agentOptionsTop.Path, err)) } - fileBytes = []byte(os.ExpandEnv(string(fileBytes))) - var pathAgentOptions BaseItem - if err := yaml.Unmarshal(fileBytes, &pathAgentOptions); err != nil { - return multierror.Append( - multiError, fmt.Errorf("failed to unmarshal agent options file %s: %v", *agentOptionsTop.Path, err), + // Replace $var and ${var} with env values. + fileBytes, err = ExpandEnvBytes(fileBytes) + if err != nil { + multiError = multierror.Append( + multiError, fmt.Errorf("failed to expand environment in file %s: %v", *agentOptionsTop.Path, err), ) + } else { + var pathAgentOptions BaseItem + if err := yaml.Unmarshal(fileBytes, &pathAgentOptions); err != nil { + return multierror.Append( + multiError, fmt.Errorf("failed to unmarshal agent options file %s: %v", *agentOptionsTop.Path, err), + ) + } + if pathAgentOptions.Path != nil { + return multierror.Append( + multiError, + fmt.Errorf("nested paths are not supported: %s in %s", *pathAgentOptions.Path, *agentOptionsTop.Path), + ) + } + var raw json.RawMessage + if err := yaml.Unmarshal(fileBytes, &raw); err != nil { + // This error is currently unreachable because we know the file is valid YAML when we checked for nested path + return multierror.Append( + multiError, fmt.Errorf("failed to unmarshal agent options file %s: %v", *agentOptionsTop.Path, err), + ) + } + result.AgentOptions = &raw } - if pathAgentOptions.Path != nil { - return multierror.Append( - multiError, - fmt.Errorf("nested paths are not supported: %s in %s", *pathAgentOptions.Path, *agentOptionsTop.Path), - ) - } - var raw json.RawMessage - if err := yaml.Unmarshal(fileBytes, &raw); err != nil { - // This error is currently unreachable because we know the file is valid YAML when we checked for nested path - return multierror.Append( - multiError, fmt.Errorf("failed to unmarshal agent options file %s: %v", *agentOptionsTop.Path, err), - ) - } - result.AgentOptions = &raw } } return multiError @@ -306,18 +340,25 @@ func parseControls(top map[string]json.RawMessage, result *GitOps, baseDir strin if err != nil { return multierror.Append(multiError, fmt.Errorf("failed to read controls file %s: %v", *controlsTop.Path, err)) } - fileBytes = []byte(os.ExpandEnv(string(fileBytes))) - var pathControls Controls - if err := yaml.Unmarshal(fileBytes, &pathControls); err != nil { - return multierror.Append(multiError, fmt.Errorf("failed to unmarshal controls file %s: %v", *controlsTop.Path, err)) - } - if pathControls.Path != nil { - return multierror.Append( - multiError, - fmt.Errorf("nested paths are not supported: %s in %s", *pathControls.Path, *controlsTop.Path), + // Replace $var and ${var} with env values. + fileBytes, err = ExpandEnvBytes(fileBytes) + if err != nil { + multiError = multierror.Append( + multiError, fmt.Errorf("failed to expand environment in file %s: %v", *controlsTop.Path, err), ) + } else { + var pathControls Controls + if err := yaml.Unmarshal(fileBytes, &pathControls); err != nil { + return multierror.Append(multiError, fmt.Errorf("failed to unmarshal controls file %s: %v", *controlsTop.Path, err)) + } + if pathControls.Path != nil { + return multierror.Append( + multiError, + fmt.Errorf("nested paths are not supported: %s in %s", *pathControls.Path, *controlsTop.Path), + ) + } + result.Controls = pathControls } - result.Controls = pathControls } return multiError } @@ -341,21 +382,28 @@ func parsePolicies(top map[string]json.RawMessage, result *GitOps, baseDir strin multiError = multierror.Append(multiError, fmt.Errorf("failed to read policies file %s: %v", *item.Path, err)) continue } - fileBytes = []byte(os.ExpandEnv(string(fileBytes))) - var pathPolicies []*Policy - if err := yaml.Unmarshal(fileBytes, &pathPolicies); err != nil { - multiError = multierror.Append(multiError, fmt.Errorf("failed to unmarshal policies file %s: %v", *item.Path, err)) - continue - } - for _, pp := range pathPolicies { - pp := pp - if pp != nil { - if pp.Path != nil { - multiError = multierror.Append( - multiError, fmt.Errorf("nested paths are not supported: %s in %s", *pp.Path, *item.Path), - ) - } else { - result.Policies = append(result.Policies, &pp.PolicySpec) + // Replace $var and ${var} with env values. + fileBytes, err = ExpandEnvBytes(fileBytes) + if err != nil { + multiError = multierror.Append( + multiError, fmt.Errorf("failed to expand environment in file %s: %v", *item.Path, err), + ) + } else { + var pathPolicies []*Policy + if err := yaml.Unmarshal(fileBytes, &pathPolicies); err != nil { + multiError = multierror.Append(multiError, fmt.Errorf("failed to unmarshal policies file %s: %v", *item.Path, err)) + continue + } + for _, pp := range pathPolicies { + pp := pp + if pp != nil { + if pp.Path != nil { + multiError = multierror.Append( + multiError, fmt.Errorf("nested paths are not supported: %s in %s", *pp.Path, *item.Path), + ) + } else { + result.Policies = append(result.Policies, &pp.PolicySpec) + } } } } @@ -407,21 +455,28 @@ func parseQueries(top map[string]json.RawMessage, result *GitOps, baseDir string multiError = multierror.Append(multiError, fmt.Errorf("failed to read queries file %s: %v", *item.Path, err)) continue } - fileBytes = []byte(os.ExpandEnv(string(fileBytes))) - var pathQueries []*Query - if err := yaml.Unmarshal(fileBytes, &pathQueries); err != nil { - multiError = multierror.Append(multiError, fmt.Errorf("failed to unmarshal queries file %s: %v", *item.Path, err)) - continue - } - for _, pq := range pathQueries { - pq := pq - if pq != nil { - if pq.Path != nil { - multiError = multierror.Append( - multiError, fmt.Errorf("nested paths are not supported: %s in %s", *pq.Path, *item.Path), - ) - } else { - result.Queries = append(result.Queries, &pq.QuerySpec) + // Replace $var and ${var} with env values. + fileBytes, err = ExpandEnvBytes(fileBytes) + if err != nil { + multiError = multierror.Append( + multiError, fmt.Errorf("failed to expand environment in file %s: %v", *item.Path, err), + ) + } else { + var pathQueries []*Query + if err := yaml.Unmarshal(fileBytes, &pathQueries); err != nil { + multiError = multierror.Append(multiError, fmt.Errorf("failed to unmarshal queries file %s: %v", *item.Path, err)) + continue + } + for _, pq := range pathQueries { + pq := pq + if pq != nil { + if pq.Path != nil { + multiError = multierror.Append( + multiError, fmt.Errorf("nested paths are not supported: %s in %s", *pq.Path, *item.Path), + ) + } else { + result.Queries = append(result.Queries, &pq.QuerySpec) + } } } } diff --git a/pkg/spec/gitops_test.go b/pkg/spec/gitops_test.go index 98a0a36a0c..5d24c2c4ae 100644 --- a/pkg/spec/gitops_test.go +++ b/pkg/spec/gitops_test.go @@ -42,6 +42,20 @@ team_settings: `, } +func createTempFile(t *testing.T, pattern, contents string) (filePath string, baseDir string) { + tmpFile, err := os.CreateTemp(t.TempDir(), pattern) + require.NoError(t, err) + _, err = tmpFile.WriteString(contents) + require.NoError(t, err) + require.NoError(t, tmpFile.Close()) + return tmpFile.Name(), filepath.Dir(tmpFile.Name()) +} + +func gitOpsFromString(t *testing.T, s string) (*GitOps, error) { + path, basePath := createTempFile(t, "", s) + return GitOpsFromFile(path, basePath) +} + func TestValidGitOpsYaml(t *testing.T) { t.Parallel() tests := map[string]struct { @@ -70,9 +84,7 @@ func TestValidGitOpsYaml(t *testing.T) { t.Run( name, func(t *testing.T) { t.Parallel() - dat, err := os.ReadFile(test.filePath) - require.NoError(t, err) - gitops, err := GitOpsFromBytes(dat, "./testdata") + gitops, err := GitOpsFromFile(test.filePath, "./testdata") require.NoError(t, err) if test.isTeam { @@ -171,7 +183,7 @@ policies: platform: windows query: SELECT 1; ` - _, err := GitOpsFromBytes([]byte(config), "") + _, err := gitOpsFromString(t, config) assert.ErrorContains(t, err, "duplicate policy names") } @@ -197,7 +209,7 @@ queries: automations_enabled: true logging: snapshot ` - _, err := GitOpsFromBytes([]byte(config), "") + _, err := gitOpsFromString(t, config) assert.ErrorContains(t, err, "duplicate query names") } @@ -215,7 +227,7 @@ queries: automations_enabled: true logging: snapshot ` - _, err := GitOpsFromBytes([]byte(config), "") + _, err := gitOpsFromString(t, config) assert.ErrorContains(t, err, "query name must be in ASCII") } @@ -223,30 +235,77 @@ func TestUnicodeTeamName(t *testing.T) { t.Parallel() config := getTeamConfig([]string{"name"}) config += `name: 😊 TeamName` - _, err := GitOpsFromBytes([]byte(config), "") + _, err := gitOpsFromString(t, config) assert.NoError(t, err) } +func TestVarExpansion(t *testing.T) { + os.Setenv("MACOS_OS", "darwin") + os.Setenv("LINUX_OS", "linux") + os.Setenv("EMPTY_VAR", "") + t.Cleanup(func() { + os.Unsetenv("MACOS_OS") + os.Unsetenv("LINUX_OS") + os.Unsetenv("EMPTY_VAR") + }) + config := getGlobalConfig([]string{"queries"}) + config += ` +queries: +- name: orbit_info \$NOT_EXPANDED \\\$ALSO_NOT_EXPANDED + query: "SELECT * from orbit_info; -- double quotes are escaped by YAML after Fleet's escaping of backslashes \\\\\$NOT_EXPANDED" + interval: 0 + platform: $MACOS_OS,${LINUX_OS},windows$EMPTY_VAR + min_osquery_version: all + observer_can_run: false + automations_enabled: true + logging: snapshot + description: 'single quotes are not escaped by YAML \\\$NOT_EXPANDED' +` + gitOps, err := gitOpsFromString(t, config) + require.NoError(t, err) + require.Len(t, gitOps.Queries, 1) + require.Equal(t, "darwin,linux,windows", gitOps.Queries[0].Platform) + require.Equal(t, `orbit_info $NOT_EXPANDED \$ALSO_NOT_EXPANDED`, gitOps.Queries[0].Name) + require.Equal(t, `single quotes are not escaped by YAML \$NOT_EXPANDED`, gitOps.Queries[0].Description) + require.Equal(t, `SELECT * from orbit_info; -- double quotes are escaped by YAML after Fleet's escaping of backslashes \$NOT_EXPANDED`, gitOps.Queries[0].Query) + + config = getGlobalConfig([]string{"queries"}) + config += ` +queries: +- name: orbit_info $NOT_DEFINED + query: SELECT * from orbit_info; + interval: 0 + platform: darwin,linux,windows + min_osquery_version: all + observer_can_run: false + automations_enabled: true + logging: snapshot +` + gitOps, err = gitOpsFromString(t, config) + require.Error(t, err) + require.Contains(t, err.Error(), "variable \"NOT_DEFINED\" not set") +} + func TestMixingGlobalAndTeamConfig(t *testing.T) { t.Parallel() // Mixing org_settings and team name config := getGlobalConfig(nil) config += "name: TeamName\n" - _, err := GitOpsFromBytes([]byte(config), "") + _, err := gitOpsFromString(t, config) assert.ErrorContains(t, err, "'org_settings' cannot be used with 'name' or 'team_settings'") // Mixing org_settings and team_settings config = getGlobalConfig(nil) config += "team_settings:\n secrets: []\n" - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "'org_settings' cannot be used with 'name' or 'team_settings'") // Mixing org_settings and team name and team_settings config = getGlobalConfig(nil) config += "name: TeamName\n" config += "team_settings:\n secrets: []\n" - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "'org_settings' cannot be used with 'name' or 'team_settings'") } @@ -254,7 +313,7 @@ func TestInvalidGitOpsYaml(t *testing.T) { t.Parallel() // Bad YAML - _, err := GitOpsFromBytes([]byte("bad:\nbad"), "") + _, err := gitOpsFromString(t, "bad:\nbad") assert.ErrorContains(t, err, "failed to unmarshal") for _, name := range []string{"global", "team"} { @@ -270,25 +329,25 @@ func TestInvalidGitOpsYaml(t *testing.T) { // Invalid top level key config := getConfig(nil) config += "unknown_key:\n" - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "unknown top-level field") // Invalid team name config = getConfig([]string{"name"}) config += "name: [2]\n" - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "failed to unmarshal name") // Missing team name config = getConfig([]string{"name"}) config += "name:\n" - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "'name' is required") // Invalid team_settings config = getConfig([]string{"team_settings"}) config += "team_settings:\n path: [2]\n" - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "failed to unmarshal team_settings") // Invalid team_settings in a separate file @@ -298,31 +357,31 @@ func TestInvalidGitOpsYaml(t *testing.T) { require.NoError(t, err) config = getConfig([]string{"team_settings"}) config += fmt.Sprintf("%s:\n path: %s\n", "team_settings", tmpFile.Name()) - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "failed to unmarshal team settings file") // Invalid secrets 1 config = getConfig([]string{"team_settings"}) config += "team_settings:\n secrets: bad\n" - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "must be a list of secret items") // Invalid secrets 2 config = getConfig([]string{"team_settings"}) config += "team_settings:\n secrets: [2]\n" - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "must have a 'secret' key") // Missing secrets config = getConfig([]string{"team_settings"}) config += "team_settings:\n" - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "'team_settings.secrets' is required") } else { // Invalid org_settings config := getConfig([]string{"org_settings"}) config += "org_settings:\n path: [2]\n" - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "failed to unmarshal org_settings") // Invalid org_settings in a separate file @@ -332,32 +391,32 @@ func TestInvalidGitOpsYaml(t *testing.T) { require.NoError(t, err) config = getConfig([]string{"org_settings"}) config += fmt.Sprintf("%s:\n path: %s\n", "org_settings", tmpFile.Name()) - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "failed to unmarshal org settings file") // Invalid secrets 1 config = getConfig([]string{"org_settings"}) config += "org_settings:\n secrets: bad\n" - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "must be a list of secret items") // Invalid secrets 2 config = getConfig([]string{"org_settings"}) config += "org_settings:\n secrets: [2]\n" - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "must have a 'secret' key") // Missing secrets config = getConfig([]string{"org_settings"}) config += "org_settings:\n" - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "'org_settings.secrets' is required") } // Invalid agent_options config := getConfig([]string{"agent_options"}) config += "agent_options:\n path: [2]\n" - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "failed to unmarshal agent_options") // Invalid agent_options in a separate file @@ -367,13 +426,13 @@ func TestInvalidGitOpsYaml(t *testing.T) { require.NoError(t, err) config = getConfig([]string{"agent_options"}) config += fmt.Sprintf("%s:\n path: %s\n", "agent_options", tmpFile.Name()) - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "failed to unmarshal agent options file") // Invalid controls config = getConfig([]string{"controls"}) config += "controls:\n path: [2]\n" - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "failed to unmarshal controls") // Invalid controls in a separate file @@ -383,13 +442,13 @@ func TestInvalidGitOpsYaml(t *testing.T) { require.NoError(t, err) config = getConfig([]string{"controls"}) config += fmt.Sprintf("%s:\n path: %s\n", "controls", tmpFile.Name()) - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "failed to unmarshal controls file") // Invalid policies config = getConfig([]string{"policies"}) config += "policies:\n path: [2]\n" - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "failed to unmarshal policies") // Invalid policies in a separate file @@ -399,25 +458,25 @@ func TestInvalidGitOpsYaml(t *testing.T) { require.NoError(t, err) config = getConfig([]string{"policies"}) config += fmt.Sprintf("%s:\n - path: %s\n", "policies", tmpFile.Name()) - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "failed to unmarshal policies file") // Policy name missing config = getConfig([]string{"policies"}) config += "policies:\n - query: SELECT 1;\n" - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "name is required") // Policy query missing config = getConfig([]string{"policies"}) config += "policies:\n - name: Test Policy\n" - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "query is required") // Invalid queries config = getConfig([]string{"queries"}) config += "queries:\n path: [2]\n" - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "failed to unmarshal queries") // Invalid policies in a separate file @@ -427,19 +486,19 @@ func TestInvalidGitOpsYaml(t *testing.T) { require.NoError(t, err) config = getConfig([]string{"queries"}) config += fmt.Sprintf("%s:\n - path: %s\n", "queries", tmpFile.Name()) - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "failed to unmarshal queries file") // Query name missing config = getConfig([]string{"queries"}) config += "queries:\n - query: SELECT 1;\n" - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "name is required") // Query SQL query missing config = getConfig([]string{"queries"}) config += "queries:\n - name: Test Query\n" - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "query is required") }, ) @@ -498,7 +557,7 @@ func TestTopLevelGitOpsValidation(t *testing.T) { } else { config = getGlobalConfig(test.optsToExclude) } - _, err := GitOpsFromBytes([]byte(config), "") + _, err := gitOpsFromString(t, config) if test.shouldPass { assert.NoError(t, err) } else { @@ -514,7 +573,7 @@ func TestGitOpsNullArrays(t *testing.T) { config := getGlobalConfig([]string{"queries", "policies"}) config += "queries: null\npolicies: ~\n" - gitops, err := GitOpsFromBytes([]byte(config), "") + gitops, err := gitOpsFromString(t, config) assert.NoError(t, err) assert.Nil(t, gitops.Queries) assert.Nil(t, gitops.Policies) @@ -567,7 +626,8 @@ func TestGitOpsPaths(t *testing.T) { } // Test an absolute top level path - tmpFile, err := os.CreateTemp(t.TempDir(), "*good.yml") + tmpDir := t.TempDir() + tmpFile, err := os.CreateTemp(tmpDir, "*good.yml") require.NoError(t, err) _, err = tmpFile.WriteString(test.goodConfig) require.NoError(t, err) @@ -577,18 +637,23 @@ func TestGitOpsPaths(t *testing.T) { } else { config += fmt.Sprintf("%s:\n path: %s\n", name, tmpFile.Name()) } - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.NoError(t, err) // Test a relative top level path config = getConfig([]string{name}) + mainTmpFile, err := os.CreateTemp(tmpDir, "*main.yml") + require.NoError(t, err) dir, file := filepath.Split(tmpFile.Name()) if test.isArray { config += fmt.Sprintf("%s:\n - path: ./%s\n", name, file) } else { config += fmt.Sprintf("%s:\n path: ./%s\n", name, file) } - _, err = GitOpsFromBytes([]byte(config), dir) + err = os.WriteFile(mainTmpFile.Name(), []byte(config), 0o644) + require.NoError(t, err) + + _, err = GitOpsFromFile(mainTmpFile.Name(), dir) assert.NoError(t, err) // Test a bad path @@ -598,7 +663,10 @@ func TestGitOpsPaths(t *testing.T) { } else { config += fmt.Sprintf("%s:\n path: ./%s\n", name, "doesNotExist.yml") } - _, err = GitOpsFromBytes([]byte(config), dir) + err = os.WriteFile(mainTmpFile.Name(), []byte(config), 0o644) + require.NoError(t, err) + + _, err = GitOpsFromFile(mainTmpFile.Name(), dir) assert.ErrorContains(t, err, "no such file or directory") // Test a bad file -- cannot be unmarshalled @@ -612,7 +680,7 @@ func TestGitOpsPaths(t *testing.T) { } else { config += fmt.Sprintf("%s:\n path: %s\n", name, tmpFileBad.Name()) } - _, err = GitOpsFromBytes([]byte(config), "") + _, err = gitOpsFromString(t, config) assert.ErrorContains(t, err, "failed to unmarshal") // Test a nested path -- bad @@ -631,7 +699,9 @@ func TestGitOpsPaths(t *testing.T) { } else { config += fmt.Sprintf("%s:\n path: ./%s\n", name, file) } - _, err = GitOpsFromBytes([]byte(config), dir) + err = os.WriteFile(mainTmpFile.Name(), []byte(config), 0o644) + require.NoError(t, err) + _, err = GitOpsFromFile(mainTmpFile.Name(), dir) assert.ErrorContains(t, err, "nested paths are not supported") }, ) diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go index 1174b03f48..d760f8b1d3 100644 --- a/pkg/spec/spec.go +++ b/pkg/spec/spec.go @@ -3,14 +3,18 @@ package spec import ( + "crypto/rand" + "encoding/hex" "encoding/json" "errors" "fmt" + "os" "regexp" "strings" "github.com/fleetdm/fleet/v4/server/fleet" "github.com/ghodss/yaml" + "github.com/hashicorp/go-multierror" ) var yamlSeparator = regexp.MustCompile(`(?m:^---[\t ]*)`) @@ -142,3 +146,59 @@ func SplitYaml(in string) []string { } return out } + +func generateRandomString(sizeBytes int) string { + b := make([]byte, sizeBytes) + if _, err := rand.Read(b); err != nil { + panic(err) + } + return hex.EncodeToString(b) +} + +func ExpandEnv(s string) (string, error) { + // Generate a random escaping prefix that doesn't exist in s. + var preventEscapingPrefix string + for { + preventEscapingPrefix = "PREVENT_ESCAPING_" + generateRandomString(8) + if !strings.Contains(s, preventEscapingPrefix) { + break + } + } + + s = escapeString(s, preventEscapingPrefix) + var err *multierror.Error + s = os.Expand(s, func(env string) string { + if strings.HasPrefix(env, preventEscapingPrefix) { + return "$" + strings.TrimPrefix(env, preventEscapingPrefix) + } + v, ok := os.LookupEnv(env) + if !ok { + err = multierror.Append(err, fmt.Errorf("environment variable %q not set", env)) + return "" + } + return v + }) + if err != nil { + return "", err + } + return s, nil +} + +func ExpandEnvBytes(b []byte) ([]byte, error) { + s, err := ExpandEnv(string(b)) + if err != nil { + return nil, err + } + return []byte(s), nil +} + +var escapePattern = regexp.MustCompile(`(\\+\$)`) + +func escapeString(s string, preventEscapingPrefix string) string { + return escapePattern.ReplaceAllStringFunc(s, func(match string) string { + if len(match)%2 != 0 { + return match + } + return strings.Repeat("\\", (len(match)/2)-1) + "$" + preventEscapingPrefix + }) +} diff --git a/pkg/spec/spec_test.go b/pkg/spec/spec_test.go index 46d0cd6e02..e3036baa16 100644 --- a/pkg/spec/spec_test.go +++ b/pkg/spec/spec_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "github.com/hashicorp/go-multierror" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -111,3 +112,83 @@ kind: "" }) } } + +func TestEscapeString(t *testing.T) { + for _, tc := range []struct { + s string + expResult string + }{ + {`$foo`, `$foo`}, // nothing to escape + {`bar$foo`, `bar$foo`}, // nothing to escape + {`bar${foo}`, `bar${foo}`}, // nothing to escape + {`\$foo`, `$PREVENT_ESCAPING_foo`}, // escaping + {`bar\$foo`, `bar$PREVENT_ESCAPING_foo`}, // escaping + {`\\$foo`, `\\$foo`}, // no escaping + {`bar\\$foo`, `bar\\$foo`}, // no escaping + {`\\\$foo`, `\$PREVENT_ESCAPING_foo`}, // escaping + {`bar\\\$foo`, `bar\$PREVENT_ESCAPING_foo`}, // escaping + {`bar\\\${foo}bar`, `bar\$PREVENT_ESCAPING_{foo}bar`}, // escaping + {`\\\\$foo`, `\\\\$foo`}, // no escaping + {`bar\\\\$foo`, `bar\\\\$foo`}, // no escaping + {`bar\\\\${foo}`, `bar\\\\${foo}`}, // no escaping + } { + result := escapeString(tc.s, "PREVENT_ESCAPING_") + require.Equal(t, tc.expResult, result) + } +} + +func TestExpandEnv(t *testing.T) { + checkMultiErrors := func(errs ...string) func(err error) { + return func(err error) { + me, ok := err.(*multierror.Error) + require.True(t, ok) + require.Len(t, me.Errors, len(errs)) + for i, err := range me.Errors { + require.Equal(t, errs[i], err.Error()) + } + } + } + + for _, tc := range []struct { + environment map[string]string + s string + expResult string + checkErr func(error) + }{ + {map[string]string{"foo": "1"}, `$foo`, `1`, nil}, + {map[string]string{"foo": ""}, `$foo`, ``, nil}, + {map[string]string{"foo": "", "bar": "", "zoo": ""}, `$foo${bar}$zoo`, ``, nil}, + {map[string]string{}, `$foo`, ``, checkMultiErrors("environment variable \"foo\" not set")}, + {map[string]string{"foo": "1"}, `$foo$bar`, ``, checkMultiErrors("environment variable \"bar\" not set")}, + {map[string]string{"bar": "1"}, `$foo $bar $zoo`, ``, checkMultiErrors("environment variable \"foo\" not set", "environment variable \"zoo\" not set")}, + {map[string]string{"foo": "4", "bar": "2"}, `$foo$bar`, `42`, nil}, + {map[string]string{"foo": "42", "bar": ""}, `$foo$bar`, `42`, nil}, + {map[string]string{}, `$$`, ``, checkMultiErrors("environment variable \"$\" not set")}, + {map[string]string{"foo": "1"}, `$$foo`, ``, checkMultiErrors("environment variable \"$\" not set")}, + {map[string]string{"foo": "1"}, `\$${foo}`, `$1`, nil}, + {map[string]string{}, `\$foo`, `$foo`, nil}, // escaped + {map[string]string{"foo": "1"}, `\\$foo`, `\\1`, nil}, // not escaped + {map[string]string{}, `\\\$foo`, `\$foo`, nil}, // escaped + {map[string]string{}, `\\\$foo$`, `\$foo$`, nil}, // escaped + {map[string]string{}, `bar\\\$foo$`, `bar\$foo$`, nil}, // escaped + {map[string]string{"foo": "1"}, `$foo var`, `1 var`, nil}, // not escaped + {map[string]string{"foo": "1"}, `${foo}var`, `1var`, nil}, // not escaped + {map[string]string{"foo": "1"}, `\${foo}var`, `${foo}var`, nil}, // escaped + {map[string]string{"foo": ""}, `${foo}var`, `var`, nil}, + {map[string]string{"foo": "", "$": "2"}, `${$}${foo}var`, `2var`, nil}, + {map[string]string{}, `${foo}var`, ``, checkMultiErrors("environment variable \"foo\" not set")}, + {map[string]string{}, `foo PREVENT_ESCAPING_bar`, `foo PREVENT_ESCAPING_bar`, nil}, // nothing to replace + } { + os.Clearenv() + for k, v := range tc.environment { + os.Setenv(k, v) + } + result, err := ExpandEnv(tc.s) + if tc.checkErr == nil { + require.NoError(t, err) + } else { + tc.checkErr(err) + } + require.Equal(t, tc.expResult, result) + } +}