mirror of
https://github.com/fleetdm/fleet
synced 2026-04-21 13:37:30 +00:00
Do not replace EVs in script-only packages (#43606)
<!-- Add the related story/sub-task/bug number, like Resolves #123, or remove if NA --> **Related issue:** Resolves #43311 # Checklist for submitter If some of the following don't apply, delete the relevant line. - [x] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. - [x] Input data is properly validated, `SELECT *` is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters. - [x] Timeouts are implemented and retries are limited to avoid infinite loops ## Testing - [x] Added/updated automated tests - [x] QA'd all new/changed functionality manually <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Made environment-variable expansion conditional by package type: script-only packages no longer expand host env vars during parsing, while YAML packages still have env vars expanded (expansion errors are recorded and parsing continues). * **Tests** * Added a test to confirm script packages do not expand standard shell variables during parsing. * **Chores** * Updated changelog entry describing the script-only package fix. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
parent
01a79b08b8
commit
15b0cf4277
3 changed files with 37 additions and 6 deletions
1
changes/43311-script-only-pkg-gitops
Normal file
1
changes/43311-script-only-pkg-gitops
Normal file
|
|
@ -0,0 +1 @@
|
|||
* Fixed a bug where host environment variables in script-only packages would cause gitops to fail
|
||||
|
|
@ -1865,16 +1865,16 @@ func parseSoftware(top map[string]json.RawMessage, result *GitOps, baseDir strin
|
|||
multiError = multierror.Append(multiError, fmt.Errorf("failed to read software package file %s: %w", *teamLevelPackage.Path, err))
|
||||
continue
|
||||
}
|
||||
// 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: %w", *teamLevelPackage.Path, err))
|
||||
continue
|
||||
}
|
||||
|
||||
ext := strings.ToLower(filepath.Ext(resolvedPath))
|
||||
switch ext {
|
||||
case ".sh", ".ps1":
|
||||
// Script files: only gather FLEET_SECRET_ variables, don't expand
|
||||
// regular env vars (they are shell variables meant for the endpoint).
|
||||
if err := gatherFileSecrets(result, resolvedPath); err != nil {
|
||||
multiError = multierror.Append(multiError, err)
|
||||
continue
|
||||
}
|
||||
// Script file becomes the install script for a script-only package
|
||||
scriptSpec := fleet.SoftwarePackageSpec{
|
||||
InstallScript: fleet.TeamSpecSoftwareAsset{Path: resolvedPath},
|
||||
|
|
@ -1888,6 +1888,12 @@ func parseSoftware(top map[string]json.RawMessage, result *GitOps, baseDir strin
|
|||
softwarePackageSpecs = append(softwarePackageSpecs, &scriptSpec)
|
||||
|
||||
case ".yml", ".yaml":
|
||||
// Replace $var and ${var} with env values in YAML files only.
|
||||
fileBytes, err = ExpandEnvBytes(fileBytes)
|
||||
if err != nil {
|
||||
multiError = multierror.Append(multiError, fmt.Errorf("failed to expand environment in file %s: %w", *teamLevelPackage.Path, err))
|
||||
continue
|
||||
}
|
||||
var singlePackageSpec SoftwarePackage
|
||||
singlePackageSpec.ReferencedYamlPath = resolvedPath
|
||||
if err := YamlUnmarshal(fileBytes, &singlePackageSpec); err == nil {
|
||||
|
|
|
|||
|
|
@ -3267,6 +3267,30 @@ software:
|
|||
assert.Empty(t, result.Software.Packages[0].SHA256)
|
||||
})
|
||||
|
||||
t.Run("sh_script_with_shell_variables", func(t *testing.T) {
|
||||
config := getTeamConfig([]string{"software"})
|
||||
config += `
|
||||
software:
|
||||
packages:
|
||||
- path: software/shell-vars.sh
|
||||
self_service: true
|
||||
`
|
||||
path, basePath := createTempFile(t, "", config)
|
||||
|
||||
// Create a script that uses standard shell variables (not set in CI env).
|
||||
_, expectedVarUnset := os.LookupEnv("SOMETHING_UNSET") // Make sure at least one is not set in the CI environment
|
||||
require.False(t, expectedVarUnset, "SOMETHING_UNSET should not be set in the test environment")
|
||||
scriptContent := []byte("#!/bin/bash\necho \"EUID=$EUID\"\necho \"USER=$USER\"\necho \"HOME=$HOME\"\necho \"SOMETHING_UNSET=$SOMETHING_UNSET\"\n")
|
||||
require.NoError(t, os.MkdirAll(filepath.Join(basePath, "software"), 0o755)) // nolint:gosec
|
||||
require.NoError(t, os.WriteFile(filepath.Join(basePath, "software", "shell-vars.sh"), scriptContent, 0o755)) // nolint:gosec
|
||||
|
||||
// Shell variables must not be expanded by the gitops parser.
|
||||
result, err := GitOpsFromFile(path, basePath, appConfig, nopLogf)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, result.Software.Packages, 1)
|
||||
assert.True(t, strings.HasSuffix(result.Software.Packages[0].InstallScript.Path, "shell-vars.sh"))
|
||||
})
|
||||
|
||||
t.Run("valid_ps1_script_path", func(t *testing.T) {
|
||||
config := getTeamConfig([]string{"software"})
|
||||
config += `
|
||||
|
|
|
|||
Loading…
Reference in a new issue