diff --git a/changes/script-variable-interpreter-escaping b/changes/script-variable-interpreter-escaping new file mode 100644 index 00000000000..aa8af63dc4d --- /dev/null +++ b/changes/script-variable-interpreter-escaping @@ -0,0 +1,3 @@ +- Fixed an issue where characters in a Fleet variable's value could change what a script does. Values are now defined at the top of the script, or escaped for Python, and read as literal text. +- Fleet variables in shell scripts are no longer substituted inside single quotes or a quoted heredoc. +- In Python scripts, Fleet variables work inside string literals. They aren't supported inside raw (`r"..."`) or bytes (`b"..."`) literals. diff --git a/orbit/pkg/scripts/exec_nonwindows_test.go b/orbit/pkg/scripts/exec_nonwindows_test.go index e4d279e1f22..a4190a6a64d 100644 --- a/orbit/pkg/scripts/exec_nonwindows_test.go +++ b/orbit/pkg/scripts/exec_nonwindows_test.go @@ -14,6 +14,7 @@ import ( "time" "github.com/fleetdm/fleet/v4/server/fleet" + "github.com/fleetdm/fleet/v4/server/variables" "github.com/stretchr/testify/require" ) @@ -160,3 +161,75 @@ func TestExecCmdSuccess(t *testing.T) { t.Fatalf("Expected output %q, got: %q", expectedOutput, output) } } + +// Values are defined ahead of the body rather than substituted into it, so a +// value carrying interpreter metacharacters is data by the time it runs. +func TestExecCmdDoesNotExecuteFleetVariableValues(t *testing.T) { + dir := t.TempDir() + marker := filepath.Join(dir, "MARKER") + + for _, tc := range []struct { + name string + value string + contents string + // macOS splits shebang arguments and Linux doesn't, so the shebang case + // only gets the safety assertion + safetyOnly bool + }{ + {"backtick", "Eng`touch " + marker + "`", "#!/bin/sh\nprintf %s \"$FLEET_VAR_HOST_UUID\"\n", false}, + {"cmd-subst", "Eng$(touch " + marker + ")", "#!/bin/sh\nprintf %s \"$FLEET_VAR_HOST_UUID\"\n", false}, + {"embedded quote", "Eng'; touch " + marker + "; echo '", "#!/bin/sh\nprintf %s \"$FLEET_VAR_HOST_UUID\"\n", false}, + {"no shebang", "Eng`touch " + marker + "`", "printf %s \"$FLEET_VAR_HOST_UUID\"\n", false}, + {"bash shebang", "Eng`touch " + marker + "`", "#!/bin/bash\nprintf %s \"$FLEET_VAR_HOST_UUID\"\n", false}, + // the kernel splits shebang arguments, so a substituted value would run + {"shebang reference", "x`touch " + marker + "`", "#!/bin/sh -c $FLEET_VAR_HOST_UUID\nprintf %s \"$FLEET_VAR_HOST_UUID\"\n", true}, + } { + t.Run(tc.name, func(t *testing.T) { + preamble := variables.Preamble(map[string]string{"HOST_UUID": tc.value}, variables.DialectPOSIX) + script, err := variables.InsertPreamble(tc.contents, preamble, variables.DialectPOSIX) + require.NoError(t, err) + + path := filepath.Join(t.TempDir(), "script") + require.NoError(t, os.WriteFile(path, []byte(script), 0o600)) + + output, _, err := ExecCmd(context.Background(), path, nil) + require.NoFileExists(t, marker) + if tc.safetyOnly { + return + } + require.NoError(t, err) + require.Equal(t, tc.value, string(output)) + }) + } +} + +// An older agent decides how to run a script from its first line, so the +// preamble has to leave that decision unchanged. +func TestPreambleDoesNotChangeInterpreterChoice(t *testing.T) { + pre := variables.Preamble(map[string]string{"HOST_UUID": "ABC"}, variables.DialectPOSIX) + + for _, contents := range []string{ + "echo hi\n", + "#!/bin/sh\necho hi\n", + "#!/bin/sh -e\necho hi\n", + "#!/bin/bash\necho hi\n", + "#!/bin/zsh\necho hi\n", + "#!/usr/bin/env bash\necho hi\n", + "#!/bin/sh\r\necho hi\r\n", + "# not a shebang\necho hi\n", + } { + t.Run(contents, func(t *testing.T) { + withPreamble, err := variables.InsertPreamble(contents, pre, variables.DialectPOSIX) + require.NoError(t, err) + + wantDirect, wantErr := fleet.ValidateShebang(contents) + gotDirect, gotErr := fleet.ValidateShebang(withPreamble) + require.Equal(t, wantErr, gotErr) + require.Equal(t, wantDirect, gotDirect) + + wantKind, _, _ := fleet.ShebangInfo(contents) + gotKind, _, _ := fleet.ShebangInfo(withPreamble) + require.Equal(t, wantKind, gotKind) + }) + } +} diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 071f82866fe..8aa24edffc3 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -36356,7 +36356,11 @@ func (s *integrationEnterpriseTestSuite) TestScriptFleetVariablesExecution() { s.DoJSON("POST", "/api/latest/fleet/scripts/run", fleet.HostScriptRequestPayload{HostID: h.ID, ScriptContents: contents}, http.StatusAccepted, &runResp) fetched := orbitFetchScript(t, h, runResp.ExecutionID) - require.Equal(t, fmt.Sprintf("echo serial=%s uuid=%s plat=ubuntu", h.HardwareSerial, h.UUID), fetched.ScriptContents) + requireVarsDelivered(t, fetched.ScriptContents, contents, map[string]string{ + "HOST_HARDWARE_SERIAL": h.HardwareSerial, + "HOST_UUID": h.UUID, + "HOST_PLATFORM": "ubuntu", + }) require.Nil(t, fetched.ExitCode) // stored contents stay unexpanded @@ -36434,15 +36438,20 @@ func (s *integrationEnterpriseTestSuite) TestScriptFleetVariablesExecution() { return err }) + const contents = "user=$FLEET_VAR_HOST_END_USER_IDP_USERNAME local=user_${FLEET_VAR_HOST_END_USER_IDP_USERNAME_LOCAL_PART}@corp.com dept=$FLEET_VAR_HOST_END_USER_IDP_DEPARTMENT" var runResp fleet.RunScriptResponse s.DoJSON("POST", "/api/latest/fleet/scripts/run", fleet.HostScriptRequestPayload{ HostID: host2.ID, - ScriptContents: "user=$FLEET_VAR_HOST_END_USER_IDP_USERNAME local=user_${FLEET_VAR_HOST_END_USER_IDP_USERNAME_LOCAL_PART}@corp.com dept=$FLEET_VAR_HOST_END_USER_IDP_DEPARTMENT", + ScriptContents: contents, }, http.StatusAccepted, &runResp) fetched := orbitFetchScript(t, host2, runResp.ExecutionID) require.Nil(t, fetched.ExitCode) - require.Equal(t, "user=jane.doe@example.com ($FLEET_SECRET_INJECTED) local=user_jane.doe@corp.com dept=Engineering", fetched.ScriptContents) + requireVarsDelivered(t, fetched.ScriptContents, contents, map[string]string{ + "HOST_END_USER_IDP_USERNAME": "jane.doe@example.com ($FLEET_SECRET_INJECTED)", + "HOST_END_USER_IDP_USERNAME_LOCAL_PART": "jane.doe", + "HOST_END_USER_IDP_DEPARTMENT": "Engineering", + }) }) t.Run("sync run surfaces the resolution failure", func(t *testing.T) { diff --git a/server/service/integration_install_test.go b/server/service/integration_install_test.go index a489b28d322..2bb566d8c0e 100644 --- a/server/service/integration_install_test.go +++ b/server/service/integration_install_test.go @@ -415,9 +415,12 @@ func (s *integrationInstallTestSuite) TestSoftwareInstallerFleetVariables() { InstallUUID: installUUID, OrbitNodeKey: *host.OrbitNodeKey, }, http.StatusOK, &detailsResp) - require.Equal(t, "install "+host.HardwareSerial, detailsResp.InstallScript) - require.Equal(t, "post "+host.UUID, detailsResp.PostInstallScript) - require.Equal(t, "uninstall ubuntu", detailsResp.UninstallScript) + requireVarsDelivered(t, detailsResp.InstallScript, "install $FLEET_VAR_HOST_HARDWARE_SERIAL", + map[string]string{"HOST_HARDWARE_SERIAL": host.HardwareSerial}) + requireVarsDelivered(t, detailsResp.PostInstallScript, "post ${FLEET_VAR_HOST_UUID}", + map[string]string{"HOST_UUID": host.UUID}) + requireVarsDelivered(t, detailsResp.UninstallScript, "uninstall $FLEET_VAR_HOST_PLATFORM", + map[string]string{"HOST_PLATFORM": "ubuntu"}) // the host completes the install so the queue is free for the failure case s.Do("POST", "/api/fleet/orbit/software_install/result", fleet.OrbitPostSoftwareInstallResultRequest{ diff --git a/server/service/script_variables.go b/server/service/script_variables.go index 3c422da9596..7c0dd97b81a 100644 --- a/server/service/script_variables.go +++ b/server/service/script_variables.go @@ -2,6 +2,7 @@ package service import ( "context" + "errors" "fmt" "slices" "strings" @@ -13,18 +14,23 @@ import ( "github.com/fleetdm/fleet/v4/server/variables" ) -// maybeExpandScriptFleetVariables resolves supported $FLEET_VAR_* references -// in contents for the given host. It returns the expanded contents, or a -// non-empty failureMessage when a variable exists but can't be resolved for -// this host (one line per failing variable). Unsupported variable names are -// left untouched: validation rejects them in new content, and content saved -// before validation shipped must keep working unchanged. Known limit of -// variables.Replace, accepted because validation rejects unsupported names -// going forward: in pre-validation content, an unsupported name that extends -// a supported one (e.g. $FLEET_VAR_HOST_UUID_SUFFIX) has its prefix replaced -// along with the supported variable. Supported names that extend each other -// (e.g. ..._IDP_USERNAME and ..._IDP_USERNAME_LOCAL_PART) are safe because -// variables.Find returns names longest-first and each is replaced in turn. +const ( + powerShellParamBlockMsg = "Fleet couldn't run this script because Fleet variables aren't supported inside a PowerShell param() block. Use the variable in the script body instead." + unsupportedInterpMsg = "Fleet couldn't run this script because its interpreter isn't supported." + noPlatformMsg = "There is no platform for this host. Fleet couldn't populate Fleet variables." +) + +// maybeExpandScriptFleetVariables resolves supported $FLEET_VAR_* references in +// contents for the given host. Values are defined in a preamble, or escaped and +// substituted for Python, so a value is never parsed as script source. It +// returns the expanded contents, or a non-empty failureMessage when a variable +// can't be resolved for this host (one line per failing variable). +// +// Unsupported variable names are left untouched, since validation rejects them +// in new content and content saved before validation shipped must keep working. +// On the Python path that holds except for an unsupported name that extends a +// supported one (e.g. $FLEET_VAR_HOST_UUID_LEGACY), whose prefix +// variables.Replace rewrites along with the supported variable. func (svc *Service) maybeExpandScriptFleetVariables(ctx context.Context, host *fleet.Host, contents string) (expanded string, failureMessage string, err error) { fleetVars := variables.Find(contents) if len(fleetVars) == 0 { @@ -37,6 +43,21 @@ func (svc *Service) maybeExpandScriptFleetVariables(ctx context.Context, host *f return "", "Fleet couldn't run this script because it uses variables, which require a Fleet Premium license.", nil } + supported := make([]string, 0, len(fleetVars)) + for _, v := range fleetVars { + if slices.Contains(fleet.FleetVarsSupportedInScripts, fleet.FleetVarName(v)) { + supported = append(supported, v) + } + } + if len(supported) == 0 { + return contents, "", nil + } + + dialect, dialectFailure := scriptFleetVarDialect(host, contents) + if dialectFailure != "" { + return "", dialectFailure, nil + } + // collect all failures instead of stopping at the first one so the admin // can fix everything in one pass var failures []string @@ -45,12 +66,9 @@ func (svc *Service) maybeExpandScriptFleetVariables(ctx context.Context, host *f return nil } + resolved := make(map[string]string, len(supported)) hostIDForUUIDCache := map[string]uint{host.UUID: host.ID} - for _, v := range fleetVars { - if !slices.Contains(fleet.FleetVarsSupportedInScripts, fleet.FleetVarName(v)) { - continue - } - + for _, v := range supported { var value string switch fleet.FleetVarName(v) { case fleet.FleetVarHostUUID: @@ -70,10 +88,6 @@ func (svc *Service) maybeExpandScriptFleetVariables(ctx context.Context, host *f if value == "darwin" { value = "macos" } - if value == "" { - _ = fail(fmt.Sprintf("There is no platform for this host. Fleet couldn't populate $FLEET_VAR_%s.", v)) - continue - } default: // the IdP variables idpValue, _, ok, err := profiles.ResolveHostEndUserIDPValue(ctx, svc.ds, v, host.UUID, hostIDForUUIDCache, fail) if err != nil { @@ -86,11 +100,61 @@ func (svc *Service) maybeExpandScriptFleetVariables(ctx context.Context, host *f value = idpValue } - contents = variables.Replace(contents, v, value) + // a NUL silently truncates the line for the interpreter + if strings.ContainsRune(value, 0) { + _ = fail(fmt.Sprintf("The value for $FLEET_VAR_%s contains an invalid character. Fleet couldn't populate it.", v)) + continue + } + resolved[v] = value } if len(failures) > 0 { return "", strings.Join(failures, "\n"), nil } - return contents, "", nil + if len(resolved) == 0 { + return contents, "", nil + } + + if dialect == variables.DialectPython { + // supported is longest-first from variables.Find, which keeps a shorter + // name from matching inside a longer token that contains it + for _, name := range supported { + value, ok := resolved[name] + if !ok { + continue + } + contents = variables.Replace(contents, name, variables.PythonEscape(value)) + } + return contents, "", nil + } + + expanded, err = variables.InsertPreamble(contents, variables.Preamble(resolved, dialect), dialect) + switch { + case errors.Is(err, variables.ErrPowerShellLeadingParamBlock): + return "", powerShellParamBlockMsg, nil + case err != nil: + return "", "", ctxerr.Wrap(ctx, err, "insert fleet variable preamble") + } + return expanded, "", nil +} + +// scriptFleetVarDialect returns the interpreter to write the preamble for, or a +// message explaining why variables can't be delivered to it. Platform decides +// first: on Windows fleetd runs the script through PowerShell whatever shebang +// it carries. +func scriptFleetVarDialect(host *fleet.Host, contents string) (variables.Dialect, string) { + if host.Platform == "" { + return 0, noPlatformMsg + } + if fleet.IsWindowsPlatform(host.Platform) { + return variables.DialectPowerShell, "" + } + kind, _, err := fleet.ShebangInfo(contents) + switch { + case err != nil: + return 0, unsupportedInterpMsg + case kind == fleet.ShebangPython: + return variables.DialectPython, "" + } + return variables.DialectPOSIX, "" } diff --git a/server/service/script_variables_test.go b/server/service/script_variables_test.go index 1354ccaadad..2d0c09fd957 100644 --- a/server/service/script_variables_test.go +++ b/server/service/script_variables_test.go @@ -2,6 +2,7 @@ package service import ( "context" + "slices" "strings" "testing" "time" @@ -10,6 +11,7 @@ import ( "github.com/fleetdm/fleet/v4/server/fleet" "github.com/fleetdm/fleet/v4/server/mock" "github.com/fleetdm/fleet/v4/server/test" + "github.com/fleetdm/fleet/v4/server/variables" "github.com/stretchr/testify/require" ) @@ -62,43 +64,213 @@ func TestMaybeExpandScriptFleetVariables(t *testing.T) { } }) - t.Run("host variables expand", func(t *testing.T) { + t.Run("host variables are defined, not substituted", func(t *testing.T) { svc, ctx, _ := newSvcAndCtx(fleet.TierPremium) - expanded, failMsg, err := svc.maybeExpandScriptFleetVariables(ctx, host, - "echo $FLEET_VAR_HOST_UUID $FLEET_VAR_HOST_HARDWARE_SERIAL ${FLEET_VAR_HOST_PLATFORM}") + const body = "echo $FLEET_VAR_HOST_UUID $FLEET_VAR_HOST_HARDWARE_SERIAL ${FLEET_VAR_HOST_PLATFORM}" + expanded, failMsg, err := svc.maybeExpandScriptFleetVariables(ctx, host, body) require.NoError(t, err) require.Empty(t, failMsg) - require.Equal(t, "echo ABC-123 SERIAL-1 macos", expanded) + requireVarsDelivered(t, expanded, body, map[string]string{ + "HOST_UUID": "ABC-123", "HOST_HARDWARE_SERIAL": "SERIAL-1", "HOST_PLATFORM": "macos", + }) }) t.Run("platform passes through for linux and windows", func(t *testing.T) { svc, ctx, _ := newSvcAndCtx(fleet.TierPremium) - for platform, want := range map[string]string{"ubuntu": "ubuntu", "rhel": "rhel", "windows": "windows"} { + for platform, want := range map[string]string{"ubuntu": "ubuntu", "rhel": "rhel"} { h := *host h.Platform = platform expanded, failMsg, err := svc.maybeExpandScriptFleetVariables(ctx, &h, "echo $FLEET_VAR_HOST_PLATFORM") require.NoError(t, err) require.Empty(t, failMsg) - require.Equal(t, "echo "+want, expanded) + requireVarsDelivered(t, expanded, "echo $FLEET_VAR_HOST_PLATFORM", map[string]string{"HOST_PLATFORM": want}) } }) - t.Run("IdP variables expand", func(t *testing.T) { + t.Run("IdP variables are defined, not substituted", func(t *testing.T) { svc, ctx, ds := newSvcAndCtx(fleet.TierPremium) mockScimUser(ds, scimUser) + const body = "user: $FLEET_VAR_HOST_END_USER_IDP_USERNAME\n" + + "email: user_${FLEET_VAR_HOST_END_USER_IDP_USERNAME_LOCAL_PART}@corp.example.com\n" + + "name: $FLEET_VAR_HOST_END_USER_IDP_FULL_NAME\n" + + "groups: $FLEET_VAR_HOST_END_USER_IDP_GROUPS\n" + + "dept: $FLEET_VAR_HOST_END_USER_IDP_DEPARTMENT\n" + expanded, failMsg, err := svc.maybeExpandScriptFleetVariables(ctx, host, body) + require.NoError(t, err) + require.Empty(t, failMsg) + requireVarsDelivered(t, expanded, body, map[string]string{ + "HOST_END_USER_IDP_USERNAME": "user@example.com", + "HOST_END_USER_IDP_USERNAME_LOCAL_PART": "user", + "HOST_END_USER_IDP_FULL_NAME": "Ada Lovelace", + "HOST_END_USER_IDP_GROUPS": "g1,g2", + "HOST_END_USER_IDP_DEPARTMENT": "Engineering", + }) + }) + + // SCIM attributes and the host's own osquery-reported vitals both reach this + // resolver, and neither is validated at ingestion. + t.Run("interpreter metacharacters never reach the script body", func(t *testing.T) { + payloads := map[string]string{ + "backtick": "Engineering`touch /tmp/pwned`", + "cmd-subst": "Engineering$(touch /tmp/pwned)", + "semicolon": "Engineering; touch /tmp/pwned", + "embedded-sq": "Engineering'; touch /tmp/pwned; echo '", + "newline": "Engineering\ntouch /tmp/pwned", + "python": "X\")\nimport os\nos.system(\"touch /tmp/pwned\")\nprint(\"", + } + for name, payload := range payloads { + t.Run("department/"+name, func(t *testing.T) { + svc, ctx, ds := newSvcAndCtx(fleet.TierPremium) + u := *scimUser + u.Department = &payload + mockScimUser(ds, &u) + const body = "echo \"dept: $FLEET_VAR_HOST_END_USER_IDP_DEPARTMENT\"" + expanded, failMsg, err := svc.maybeExpandScriptFleetVariables(ctx, host, body) + require.NoError(t, err) + require.Empty(t, failMsg) + requireVarsDelivered(t, expanded, body, map[string]string{"HOST_END_USER_IDP_DEPARTMENT": payload}) + }) + + t.Run("hardware-serial/"+name, func(t *testing.T) { + svc, ctx, _ := newSvcAndCtx(fleet.TierPremium) + h := *host + h.HardwareSerial = payload + const body = "echo \"serial: $FLEET_VAR_HOST_HARDWARE_SERIAL\"" + expanded, failMsg, err := svc.maybeExpandScriptFleetVariables(ctx, &h, body) + require.NoError(t, err) + require.Empty(t, failMsg) + requireVarsDelivered(t, expanded, body, map[string]string{"HOST_HARDWARE_SERIAL": payload}) + }) + + t.Run("uuid/"+name, func(t *testing.T) { + svc, ctx, _ := newSvcAndCtx(fleet.TierPremium) + h := *host + h.UUID = payload + const body = "echo \"uuid: $FLEET_VAR_HOST_UUID\"" + expanded, failMsg, err := svc.maybeExpandScriptFleetVariables(ctx, &h, body) + require.NoError(t, err) + require.Empty(t, failMsg) + requireVarsDelivered(t, expanded, body, map[string]string{"HOST_UUID": payload}) + }) + } + }) + + t.Run("windows hosts get PowerShell assignments and keep the tokens", func(t *testing.T) { + svc, ctx, _ := newSvcAndCtx(fleet.TierPremium) + h := *host + h.Platform = "windows" + const body = "Write-Output \"$FLEET_VAR_HOST_UUID ${FLEET_VAR_HOST_UUID}\"" + expanded, failMsg, err := svc.maybeExpandScriptFleetVariables(ctx, &h, body) + require.NoError(t, err) + require.Empty(t, failMsg) + require.Equal(t, "$FLEET_VAR_HOST_UUID = "+variables.PowerShellCharArray("ABC-123")+"\r\n"+body, expanded) + // the documented token syntax is unchanged on Windows + require.Contains(t, expanded, "$FLEET_VAR_HOST_UUID ${FLEET_VAR_HOST_UUID}") + require.NotContains(t, body, "ABC-123") + }) + + t.Run("PowerShell param block keeps its place", func(t *testing.T) { + svc, ctx, _ := newSvcAndCtx(fleet.TierPremium) + h := *host + h.Platform = "windows" + const body = "param($Foo = \"bar\")\r\nWrite-Output $FLEET_VAR_HOST_UUID\r\n" + expanded, failMsg, err := svc.maybeExpandScriptFleetVariables(ctx, &h, body) + require.NoError(t, err) + require.Empty(t, failMsg) + require.Equal(t, "param($Foo = \"bar\")\r\n"+ + "$FLEET_VAR_HOST_UUID = "+variables.PowerShellCharArray("ABC-123")+"\r\n"+ + "Write-Output $FLEET_VAR_HOST_UUID\r\n", expanded) + }) + + t.Run("a variable in a param default fails", func(t *testing.T) { + svc, ctx, _ := newSvcAndCtx(fleet.TierPremium) + h := *host + h.Platform = "windows" + expanded, failMsg, err := svc.maybeExpandScriptFleetVariables(ctx, &h, + "param($Foo = $FLEET_VAR_HOST_UUID)\r\nWrite-Output $Foo\r\n") + require.NoError(t, err) + require.Empty(t, expanded) + require.Equal(t, powerShellParamBlockMsg, failMsg) + }) + + t.Run("python scripts get escaped values", func(t *testing.T) { + svc, ctx, _ := newSvcAndCtx(fleet.TierPremium) + for _, shebang := range []string{ + "#!/usr/bin/env python3", "#!/usr/bin/python3", "#!/opt/homebrew/bin/python3.12", + } { + expanded, failMsg, err := svc.maybeExpandScriptFleetVariables(ctx, host, + shebang+"\nprint(\"uuid: $FLEET_VAR_HOST_UUID\")\n") + require.NoError(t, err) + require.Empty(t, failMsg) + require.Equal(t, shebang+"\nprint(\"uuid: "+variables.PythonEscape("ABC-123")+"\")\n", expanded) + require.NotContains(t, expanded, "ABC-123") + } + }) + + // HOST_END_USER_IDP_USERNAME is a prefix of HOST_END_USER_IDP_USERNAME_LOCAL_PART, + // so replacing the shorter name first corrupts the longer token + t.Run("python substitution order does not corrupt overlapping names", func(t *testing.T) { + svc, ctx, ds := newSvcAndCtx(fleet.TierPremium) + mockScimUser(ds, scimUser) + const body = "#!/usr/bin/env python3\n" + + "print(\"u: $FLEET_VAR_HOST_END_USER_IDP_USERNAME\")\n" + + "print(\"l: $FLEET_VAR_HOST_END_USER_IDP_USERNAME_LOCAL_PART\")\n" + want := "#!/usr/bin/env python3\n" + + "print(\"u: " + variables.PythonEscape("user@example.com") + "\")\n" + + "print(\"l: " + variables.PythonEscape("user") + "\")\n" + + // map iteration order is unspecified, so a single pass can pass by luck + for range 100 { + expanded, failMsg, err := svc.maybeExpandScriptFleetVariables(ctx, host, body) + require.NoError(t, err) + require.Empty(t, failMsg) + require.Equal(t, want, expanded) + } + }) + + t.Run("python values carrying source stay literal", func(t *testing.T) { + svc, ctx, ds := newSvcAndCtx(fleet.TierPremium) + payload := "X\")\nimport os\nos.system(\"touch /tmp/pwned\")\nprint(\"" + u := *scimUser + u.Department = &payload + mockScimUser(ds, &u) expanded, failMsg, err := svc.maybeExpandScriptFleetVariables(ctx, host, - "user: $FLEET_VAR_HOST_END_USER_IDP_USERNAME\n"+ - "email: user_${FLEET_VAR_HOST_END_USER_IDP_USERNAME_LOCAL_PART}@corp.example.com\n"+ - "name: $FLEET_VAR_HOST_END_USER_IDP_FULL_NAME\n"+ - "groups: $FLEET_VAR_HOST_END_USER_IDP_GROUPS\n"+ - "dept: $FLEET_VAR_HOST_END_USER_IDP_DEPARTMENT\n") + "#!/usr/bin/env python3\nprint(\"$FLEET_VAR_HOST_END_USER_IDP_DEPARTMENT\")\n") + require.NoError(t, err) + require.Empty(t, failMsg) + require.NotContains(t, expanded, "os.system") + require.Contains(t, expanded, variables.PythonEscape(payload)) + }) + + t.Run("python scripts without variables are unchanged", func(t *testing.T) { + svc, ctx, _ := newSvcAndCtx(fleet.TierPremium) + const contents = "#!/usr/bin/env python3\nprint(\"hello\")\n" + expanded, failMsg, err := svc.maybeExpandScriptFleetVariables(ctx, host, contents) require.NoError(t, err) require.Empty(t, failMsg) - require.Equal(t, "user: user@example.com\n"+ - "email: user_user@corp.example.com\n"+ - "name: Ada Lovelace\n"+ - "groups: g1,g2\n"+ - "dept: Engineering\n", expanded) + require.Equal(t, contents, expanded) + }) + + t.Run("unknown platform fails instead of guessing an interpreter", func(t *testing.T) { + svc, ctx, _ := newSvcAndCtx(fleet.TierPremium) + h := *host + h.Platform = "" + expanded, failMsg, err := svc.maybeExpandScriptFleetVariables(ctx, &h, "echo $FLEET_VAR_HOST_UUID") + require.NoError(t, err) + require.Empty(t, expanded) + require.Equal(t, noPlatformMsg, failMsg) + }) + + t.Run("NUL in a value is a resolution failure", func(t *testing.T) { + svc, ctx, ds := newSvcAndCtx(fleet.TierPremium) + u := *scimUser + u.Department = new("Eng\x00ineering") + mockScimUser(ds, &u) + expanded, failMsg, err := svc.maybeExpandScriptFleetVariables(ctx, host, + "echo $FLEET_VAR_HOST_END_USER_IDP_DEPARTMENT") + require.NoError(t, err) + require.Empty(t, expanded) + require.Contains(t, failMsg, "contains an invalid character") }) t.Run("missing IdP user is a resolution failure", func(t *testing.T) { @@ -126,11 +298,18 @@ func TestMaybeExpandScriptFleetVariables(t *testing.T) { t.Run("unsupported variable names are left untouched", func(t *testing.T) { svc, ctx, _ := newSvcAndCtx(fleet.TierPremium) - contents := "echo $FLEET_VAR_SOMETHING_ELSE and $FLEET_VAR_HOST_UUID" - expanded, failMsg, err := svc.maybeExpandScriptFleetVariables(ctx, host, contents) + const body = "echo $FLEET_VAR_SOMETHING_ELSE and $FLEET_VAR_HOST_UUID" + expanded, failMsg, err := svc.maybeExpandScriptFleetVariables(ctx, host, body) + require.NoError(t, err) + require.Empty(t, failMsg) + requireVarsDelivered(t, expanded, body, map[string]string{"HOST_UUID": "ABC-123"}) + + // only unsupported names means no preamble at all, on any interpreter + const onlyUnsupported = "#!/usr/bin/env python3\nprint(\"$FLEET_VAR_SOMETHING_ELSE\")\n" + expanded, failMsg, err = svc.maybeExpandScriptFleetVariables(ctx, host, onlyUnsupported) require.NoError(t, err) require.Empty(t, failMsg) - require.Equal(t, "echo $FLEET_VAR_SOMETHING_ELSE and ABC-123", expanded) + require.Equal(t, onlyUnsupported, expanded) }) t.Run("variables on free license fail instead of expanding", func(t *testing.T) { @@ -148,6 +327,35 @@ func TestMaybeExpandScriptFleetVariables(t *testing.T) { }) } +// requireVarsDelivered asserts each variable is defined in the preamble, that +// the body still carries its tokens, and that no value leaked into the body. +func requireVarsDelivered(t *testing.T, expanded, wantBody string, vars map[string]string) { + t.Helper() + for name, value := range vars { + require.Contains(t, expanded, "FLEET_VAR_"+name+"="+variables.PosixQuote(value)) + } + body := stripPreamble(t, expanded) + require.Equal(t, wantBody, body) + for name, value := range vars { + // a value the admin already wrote into the body proves nothing + if value != "" && !strings.Contains(wantBody, value) { + require.NotContains(t, body, value, "value for %s reached the script body", name) + } + } +} + +// stripPreamble removes the preamble, which spans more than three lines when a +// value contains newlines. +func stripPreamble(t *testing.T, contents string) string { + t.Helper() + lines := strings.Split(contents, "\n") + start := slices.IndexFunc(lines, func(l string) bool { return strings.HasPrefix(l, "__fleet_lc=") }) + require.GreaterOrEqual(t, start, 0, "no preamble found in %q", contents) + end := slices.IndexFunc(lines[start:], func(l string) bool { return strings.HasPrefix(l, "LC_ALL=${__fleet_lc}") }) + require.GreaterOrEqual(t, end, 0, "unterminated preamble in %q", contents) + return strings.Join(slices.Concat(lines[:start], lines[start+end+1:]), "\n") +} + func splitLines(s string) []string { var lines []string for line := range strings.SplitSeq(s, "\n") { @@ -201,7 +409,8 @@ func TestGetHostScriptFleetVariables(t *testing.T) { script, err := svc.GetHostScript(ctx, "exec-1") require.NoError(t, err) - require.Equal(t, "echo ABC-123 on ubuntu", script.ScriptContents) + requireVarsDelivered(t, script.ScriptContents, "echo $FLEET_VAR_HOST_UUID on $FLEET_VAR_HOST_PLATFORM", + map[string]string{"HOST_UUID": "ABC-123", "HOST_PLATFORM": "ubuntu"}) require.Nil(t, script.ExitCode) require.True(t, ds.ExpandEmbeddedSecretsFuncInvoked) }) @@ -294,9 +503,12 @@ func TestGetSoftwareInstallDetailsFleetVariables(t *testing.T) { details, err := svc.GetSoftwareInstallDetails(ctx, "install-1") require.NoError(t, err) - require.Equal(t, "install SERIAL-1", details.InstallScript) - require.Equal(t, "post ABC-123", details.PostInstallScript) - require.Equal(t, "uninstall ubuntu", details.UninstallScript) + requireVarsDelivered(t, details.InstallScript, "install $FLEET_VAR_HOST_HARDWARE_SERIAL", + map[string]string{"HOST_HARDWARE_SERIAL": "SERIAL-1"}) + requireVarsDelivered(t, details.PostInstallScript, "post ${FLEET_VAR_HOST_UUID}", + map[string]string{"HOST_UUID": "ABC-123"}) + requireVarsDelivered(t, details.UninstallScript, "uninstall $FLEET_VAR_HOST_PLATFORM", + map[string]string{"HOST_PLATFORM": "ubuntu"}) }) t.Run("scripts without variables are unchanged", func(t *testing.T) { diff --git a/server/variables/preamble.go b/server/variables/preamble.go new file mode 100644 index 00000000000..8bd31a216f1 --- /dev/null +++ b/server/variables/preamble.go @@ -0,0 +1,295 @@ +package variables + +import ( + "errors" + "sort" + "strconv" + "strings" + "unicode/utf16" +) + +// Dialect is the interpreter a preamble is written for. +type Dialect int + +const ( + DialectPOSIX Dialect = iota + DialectPowerShell + // DialectPython takes no preamble; see PythonEscape. + DialectPython +) + +// ErrPowerShellLeadingParamBlock reports that a preamble can't be placed around +// the script's leading param() block. +var ErrPowerShellLeadingParamBlock = errors.New("PowerShell script starts with a param() block") + +const utf8BOM = "\ufeff" + +// PosixQuote returns value as a single-quoted word. It is only safe inside the +// LC_ALL=C region Preamble builds: some libc decoders accept 0x27 as a multi-byte +// trail byte, letting a value ending on a lead byte consume its closing quote. +func PosixQuote(value string) string { + return "'" + strings.ReplaceAll(value, "'", `'\''`) + "'" +} + +// PowerShellCharArray returns an expression evaluating to value, built from its +// UTF-16 code units. Nothing from value reaches the script as source text, so the +// four Unicode code points PowerShell also treats as single quotes can't break +// out. It avoids method calls so it still evaluates under ConstrainedLanguage. +func PowerShellCharArray(value string) string { + if value == "" { + return "''" + } + units := utf16.Encode([]rune(value)) + var b strings.Builder + b.WriteString("([char[]](") + for i, u := range units { + if i > 0 { + b.WriteByte(',') + } + b.WriteString(strconv.FormatUint(uint64(u), 10)) + } + b.WriteString(") -join '')") + return b.String() +} + +// Preamble builds the assignments defining vars, keyed by name without the +// FLEET_VAR_ prefix. Output is ordered by name. +func Preamble(vars map[string]string, dialect Dialect) string { + if len(vars) == 0 || dialect == DialectPython { + return "" + } + names := make([]string, 0, len(vars)) + for name := range vars { + names = append(names, name) + } + sort.Strings(names) + + var b strings.Builder + if dialect == DialectPowerShell { + for _, name := range names { + b.WriteString("$FLEET_VAR_") + b.WriteString(name) + b.WriteString(" = ") + b.WriteString(PowerShellCharArray(vars[name])) + b.WriteString("\r\n") + } + return b.String() + } + + // see PosixQuote: the assignments must parse in a single-byte locale + b.WriteString("__fleet_lc=${LC_ALL-}; __fleet_lg=${LANG-}; LC_ALL=C; LANG=C\n") + b.WriteString("export") + for _, name := range names { + b.WriteString(" FLEET_VAR_") + b.WriteString(name) + b.WriteByte('=') + b.WriteString(PosixQuote(vars[name])) + } + b.WriteString("\nLC_ALL=${__fleet_lc}; LANG=${__fleet_lg}; unset __fleet_lc __fleet_lg\n") + return b.String() +} + +// InsertPreamble places preamble ahead of the body, keeping any shebang on line +// 1 so the interpreter is still chosen the same way. +func InsertPreamble(contents, preamble string, dialect Dialect) (string, error) { + if preamble == "" { + return contents, nil + } + if dialect == DialectPowerShell { + pos, err := powerShellPreamblePos(contents) + if err != nil { + return "", err + } + if before := strings.TrimPrefix(contents[:pos], utf8BOM); before != "" && !strings.HasSuffix(before, "\n") { + preamble = "\r\n" + preamble + } + return contents[:pos] + preamble + contents[pos:], nil + } + if !strings.HasPrefix(contents, "#!") { + return preamble + contents, nil + } + if i := strings.IndexByte(contents, '\n'); i >= 0 { + return contents[:i+1] + preamble + contents[i+1:], nil + } + return contents + "\n" + preamble, nil +} + +// powerShellPreamblePos returns the offset to insert a preamble at. PowerShell +// wants a byte order mark, using statements, and a param() block each to come +// first, so the preamble goes after all of them. +func powerShellPreamblePos(contents string) (int, error) { + pos := 0 + if strings.HasPrefix(contents, utf8BOM) { + pos = len(utf8BOM) + } + insert := pos + + for { + trimmed := strings.TrimLeft(contents[pos:], " \t\r\n") + pos = len(contents) - len(trimmed) + switch { + case trimmed == "": + return insert, nil + + case strings.HasPrefix(trimmed, "<#"): + // block comments do not nest, and PowerShell rejects an unterminated one + end := strings.Index(trimmed, "#>") + if end < 0 { + return insert, nil + } + pos += end + len("#>") + + case strings.HasPrefix(trimmed, "#"): + nl := strings.IndexByte(trimmed, '\n') + if nl < 0 { + return insert, nil + } + pos += nl + 1 + + case hasKeyword(trimmed, "using"): + nl := strings.IndexByte(trimmed, '\n') + if nl < 0 { + return len(contents), nil + } + pos += nl + 1 + insert = pos + + case strings.HasPrefix(trimmed, "["): + // [CmdletBinding()] may precede param(); [Type]::Member is ordinary code + after, ok := skipBracketed(trimmed) + if !ok { + return insert, nil + } + pos = len(contents) - len(after) + + default: + if hasKeyword(trimmed, "param") { + n, ok := skipParamBlock(trimmed) + // a default can't use a variable the preamble defines below it + if !ok || strings.Contains(trimmed[:n], "$FLEET_VAR_") { + return 0, ErrPowerShellLeadingParamBlock + } + return startOfNextLine(contents, pos+n), nil + } + return insert, nil + } + } +} + +// startOfNextLine advances past the rest of the line at i. +func startOfNextLine(s string, i int) int { + for i < len(s) && (s[i] == ' ' || s[i] == '\t' || s[i] == '\r') { + i++ + } + if i < len(s) && s[i] == '\n' { + i++ + } + return i +} + +// skipParamBlock returns the offset past a param(...) block, reporting false +// when it never closes. +func skipParamBlock(s string) (int, bool) { + i := strings.IndexByte(s, '(') + if i < 0 { + return 0, false + } + depth := 0 + for i < len(s) { + switch s[i] { + case '(': + depth++ + case ')': + depth-- + if depth == 0 { + return i + 1, true + } + case '\'': + i = skipSingleQuoted(s, i) + continue + case '"': + i = skipDoubleQuoted(s, i) + continue + case '<': + if i+1 < len(s) && s[i+1] == '#' { + end := strings.Index(s[i:], "#>") + if end < 0 { + return 0, false + } + i += end + len("#>") + continue + } + case '#': + nl := strings.IndexByte(s[i:], '\n') + if nl < 0 { + return 0, false + } + i += nl + 1 + continue + } + i++ + } + return 0, false +} + +// skipSingleQuoted returns the offset past a '...' string, where a doubled +// quote is a literal one. +func skipSingleQuoted(s string, i int) int { + for i++; i < len(s); i++ { + if s[i] != '\'' { + continue + } + if i+1 < len(s) && s[i+1] == '\'' { + i++ + continue + } + return i + 1 + } + return len(s) +} + +// skipDoubleQuoted returns the offset past a "..." string; a backtick escapes. +func skipDoubleQuoted(s string, i int) int { + for i++; i < len(s); i++ { + switch s[i] { + case '`': + i++ + case '"': + if i+1 < len(s) && s[i+1] == '"' { + i++ + continue + } + return i + 1 + } + } + return len(s) +} + +func skipBracketed(s string) (string, bool) { + depth := 0 + for i, c := range s { + switch c { + case '[': + depth++ + case ']': + depth-- + if depth == 0 { + return s[i+1:], true + } + } + } + return "", false +} + +// hasKeyword reports whether s opens with kw as a complete word. +func hasKeyword(s, kw string) bool { + if len(s) < len(kw) || !strings.EqualFold(s[:len(kw)], kw) { + return false + } + after := s[len(kw):] + if after == "" { + return true + } + c := after[0] + return !(c == '_' || c == '-' || (c >= '0' && c <= '9') || (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z')) +} diff --git a/server/variables/preamble_exec_test.go b/server/variables/preamble_exec_test.go new file mode 100644 index 00000000000..4d8ff519243 --- /dev/null +++ b/server/variables/preamble_exec_test.go @@ -0,0 +1,225 @@ +//go:build !windows + +package variables + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +// Every case defines two variables, the corpus value first (names are emitted in +// sorted order) and a live payload second. A single-variable case can't detect +// the multi-byte break: it needs a following quote to swallow. +const execPayloadVar = "HOST_UUID" + +const valueVar = "HOST_HARDWARE_SERIAL" + +type execShape struct { + name string + // body must contain OUT, replaced with the round-trip target path + body string + // want transforms the input value into what the body should write + want func(value string) string +} + +func execShapes() []execShape { + self := func(v string) string { return v } + return []execShape{ + {"double-quoted", "#!/bin/sh\nprintf %s \"$FLEET_VAR_HOST_HARDWARE_SERIAL\" > OUT\n", self}, + {"braced", "#!/bin/sh\nprintf %s \"${FLEET_VAR_HOST_HARDWARE_SERIAL}\" > OUT\n", self}, + {"no-shebang", "printf %s \"$FLEET_VAR_HOST_HARDWARE_SERIAL\" > OUT\n", self}, + {"child-shell", "#!/bin/sh\nsh -c 'printf %s \"$FLEET_VAR_HOST_HARDWARE_SERIAL\"' > OUT\n", self}, + {"path-concat", "#!/bin/sh\nprintf %s \"/var/tmp/c/$FLEET_VAR_HOST_HARDWARE_SERIAL/x\" > OUT\n", + func(v string) string { return "/var/tmp/c/" + v + "/x" }}, + } +} + +// byteFaithfulLocale reports whether the shell reproduces a UTF-8 value +// byte-for-byte in this locale. In an EUC locale it does not, for any variable +// holding UTF-8, preamble or environment alike; that is the admin's locale +// acting on the admin's script, so only safety is asserted there. +func byteFaithfulLocale(locale string) bool { + return locale == "C" || strings.HasSuffix(locale, ".UTF-8") +} + +func availableLocales() []string { + locales := []string{"C"} + out, err := exec.Command("locale", "-a").Output() + if err != nil { + return locales + } + installed := make(map[string]struct{}) + for line := range strings.SplitSeq(string(out), "\n") { + installed[strings.TrimSpace(line)] = struct{}{} + } + // libc decoders for these accept 0x27 as a trail byte + for _, l := range []string{"en_US.UTF-8", "ja_JP.eucJP", "ko_KR.eucKR", "zh_CN.eucCN"} { + if _, ok := installed[l]; ok { + locales = append(locales, l) + } + } + return locales +} + +func availableShells() []string { + shells := []string{"/bin/sh"} + for _, s := range []string{"/bin/bash", "/bin/zsh"} { + if _, err := os.Stat(s); err == nil { + shells = append(shells, s) + } + } + return shells +} + +func runScript(t *testing.T, shell, script, locale string) { + t.Helper() + dir := t.TempDir() + path := filepath.Join(dir, "script") + require.NoError(t, os.WriteFile(path, []byte(script), 0o600)) + cmd := exec.Command(shell, path) + cmd.Env = append(os.Environ(), "LC_ALL="+locale, "LANG="+locale) + _ = cmd.Run() // a payload that fails to parse is still a pass +} + +func TestPreambleNeverExecutesValues(t *testing.T) { + shells := availableShells() + locales := availableLocales() + + for name, value := range payloadCorpus() { + t.Run(name, func(t *testing.T) { + for _, shape := range execShapes() { + for _, shell := range shells { + for _, locale := range locales { + dir := t.TempDir() + marker := filepath.Join(dir, "MARKER") + out := filepath.Join(dir, "out") + + vars := map[string]string{ + valueVar: value, + execPayloadVar: "x$(touch " + marker + ");#", + } + script, err := InsertPreamble( + strings.ReplaceAll(shape.body, "OUT", out), + Preamble(vars, DialectPOSIX), DialectPOSIX) + require.NoError(t, err) + + runScript(t, shell, script, locale) + + require.NoFileExists(t, marker, + "value executed: shape=%s shell=%s locale=%s", shape.name, shell, locale) + + if !byteFaithfulLocale(locale) { + continue + } + got, err := os.ReadFile(out) + require.NoError(t, err, "shape=%s shell=%s locale=%s", shape.name, shell, locale) + require.Equal(t, shape.want(value), string(got), + "value did not round-trip: shape=%s shell=%s locale=%s", shape.name, shell, locale) + } + } + } + }) + } +} + +// Keeps the test above honest: a harness that never detects execution would pass +// for the wrong reason. +func TestSubstitutingValuesIntoTheBodyExecutesThem(t *testing.T) { + dir := t.TempDir() + marker := filepath.Join(dir, "MARKER") + out := filepath.Join(dir, "out") + + body := strings.ReplaceAll(execShapes()[0].body, "OUT", out) + script := Replace(body, valueVar, "Eng`touch "+marker+"`") + + runScript(t, "/bin/sh", script, "C") + + require.FileExists(t, marker) + got, _ := os.ReadFile(out) + require.Equal(t, "Eng", string(got)) +} + +// Nothing but a backslash, "U" and hex digits reaches the source, so a value can +// neither close a string literal nor, outside one, parse at all. +func TestPythonEscapeNeverExecutesValues(t *testing.T) { + python, err := exec.LookPath("python3") + if err != nil { + t.Skip("python3 not installed") + } + + // where a $FLEET_VAR_* token can sit; OUT is the round-trip target + inString := map[string]string{ + "double-quoted": `open("OUT","w").write("TOKEN")`, + "single-quoted": `open("OUT","w").write('TOKEN')`, + "triple-quoted": `open("OUT","w").write("""TOKEN""")`, + "f-string": `open("OUT","w").write(f"TOKEN")`, + "adjacent": `open("OUT","w").write("TOKEN"[0:0] + "TOKEN")`, + } + + // the shared corpus carries shell payloads; these are executable Python + values := payloadCorpus() + values["py-break-double"] = "X\")\nimport os\nos.system(\"touch \")\nprint(\"" + values["py-break-single"] = "X')\nimport os\nos.system('touch ')\nprint('" + values["py-break-triple"] = "X\"\"\")\nimport os\nos.system(\"touch \")\nprint(\"\"\"" + values["py-fstring-expr"] = "{__import__('os').system('touch ')}" + + for name, value := range values { + t.Run(name, func(t *testing.T) { + for shape, body := range inString { + dir := t.TempDir() + marker := filepath.Join(dir, "MARKER") + out := filepath.Join(dir, "out") + value := strings.ReplaceAll(value, "", marker) + + src := "#!/usr/bin/env python3\n" + strings.NewReplacer( + "OUT", out, + "TOKEN", PythonEscape(value), + ).Replace(body) + "\n" + + script := filepath.Join(dir, "s.py") + require.NoError(t, os.WriteFile(script, []byte(src), 0o600)) + _ = exec.Command(python, script).Run() + + require.NoFileExists(t, marker, "value executed: shape=%s", shape) + got, err := os.ReadFile(out) + require.NoError(t, err, "shape=%s", shape) + require.Equal(t, value, string(got), "value did not round-trip: shape=%s", shape) + } + }) + } + + t.Run("bare code fails to parse", func(t *testing.T) { + dir := t.TempDir() + marker := filepath.Join(dir, "MARKER") + script := filepath.Join(dir, "s.py") + payload := `__import__(chr(111)+chr(115)).system("touch ` + marker + `")` + src := "#!/usr/bin/env python3\nx = " + PythonEscape(payload) + "\n" + require.NoError(t, os.WriteFile(script, []byte(src), 0o600)) + + require.Error(t, exec.Command(python, script).Run()) + require.NoFileExists(t, marker) + }) +} + +// Keeps the test above honest: substituting a value into Python source executes it. +func TestSubstitutingIntoPythonExecutesValues(t *testing.T) { + python, err := exec.LookPath("python3") + if err != nil { + t.Skip("python3 not installed") + } + dir := t.TempDir() + marker := filepath.Join(dir, "MARKER") + script := filepath.Join(dir, "s.py") + + payload := "X\")\nimport os\nos.system(\"touch " + marker + "\")\nprint(\"" + src := "#!/usr/bin/env python3\n" + Replace(`print("v: $FLEET_VAR_HOST_UUID")`, "HOST_UUID", payload) + "\n" + require.NoError(t, os.WriteFile(script, []byte(src), 0o600)) + _ = exec.Command(python, script).Run() + + require.FileExists(t, marker) +} diff --git a/server/variables/preamble_test.go b/server/variables/preamble_test.go new file mode 100644 index 00000000000..8fcb2a93b6c --- /dev/null +++ b/server/variables/preamble_test.go @@ -0,0 +1,234 @@ +package variables + +import ( + "strconv" + "strings" + "testing" + "unicode/utf16" + + "github.com/stretchr/testify/require" +) + +// payloadCorpus is shared with the execution tests: each entry either executed +// under plain substitution or exercises a quoting edge. +func payloadCorpus() map[string]string { + return map[string]string{ //nolint:gosec // G101: injection payloads, not credentials + "plain": "Engineering", + "backtick": "Eng`id`", + "cmd-subst": "Eng$(id)", + "semicolon": "Eng; id", + "pipe": "Eng | id", + "embedded-sq": "Eng'; id; echo '", + "lone-sq": "'", + "only-sq": "''''", + "backslash": `Eng\'; id; #`, + "trailing-bs": `Eng\`, + "quote-soup": "Eng'\"`id`\"'", + "newline": "Eng\nid", + "crlf": "Eng\r\nid", + "tab-space": " Eng\tOps ", + "dollar-var": "$HOME and ${PATH}", + "fleet-secret": "$FLEET_SECRET_FOO", + "nested-var": "$FLEET_VAR_HOST_UUID", + "glob": "*", + "ifs": "a${IFS}b", + "non-ascii": "Ops — Zürich 🙂", + "smart-quotes": "Ann’s ‘Team’", + "cjk-lead-byte": "情報システム部", + "hangul": "정보시스템부", + "empty": "", + "long": strings.Repeat("A", 4096), + } +} + +func TestPosixQuote(t *testing.T) { + require.Equal(t, `'abc'`, PosixQuote("abc")) + require.Equal(t, `''`, PosixQuote("")) + require.Equal(t, `''\'''`, PosixQuote("'")) + require.Equal(t, `'Ann'\''s'`, PosixQuote("Ann's")) + + for name, value := range payloadCorpus() { + t.Run(name, func(t *testing.T) { + got := PosixQuote(value) + require.True(t, strings.HasPrefix(got, "'")) + require.True(t, strings.HasSuffix(got, "'")) + // the outer pair, plus three quotes per '\'' escape sequence + require.Equal(t, 2+3*strings.Count(value, "'"), strings.Count(got, "'")) + }) + } +} + +func TestPowerShellCharArray(t *testing.T) { + require.Equal(t, "''", PowerShellCharArray("")) + require.Equal(t, "([char[]](65,66) -join '')", PowerShellCharArray("AB")) + + for name, value := range payloadCorpus() { + t.Run(name, func(t *testing.T) { + got := PowerShellCharArray(value) + + // no byte of the value reaches the script as source text + for _, r := range got { + require.Contains(t, "0123456789,()[] -join'char", string(r), + "unexpected character %q in %q", r, got) + } + + if value == "" { + return + } + inner := strings.TrimSuffix(strings.TrimPrefix(got, "([char[]]("), ") -join '')") + var units []uint16 + for f := range strings.SplitSeq(inner, ",") { + u, err := strconv.ParseUint(f, 10, 16) + require.NoError(t, err) + units = append(units, uint16(u)) + } + require.Equal(t, value, string(utf16.Decode(units))) + }) + } +} + +func TestPreamble(t *testing.T) { + t.Run("empty", func(t *testing.T) { + require.Empty(t, Preamble(nil, DialectPOSIX)) + require.Empty(t, Preamble(map[string]string{}, DialectPowerShell)) + }) + + t.Run("posix is sorted and locale-guarded", func(t *testing.T) { + got := Preamble(map[string]string{"HOST_UUID": "u", "HOST_HARDWARE_SERIAL": "s"}, DialectPOSIX) + require.Equal(t, + "__fleet_lc=${LC_ALL-}; __fleet_lg=${LANG-}; LC_ALL=C; LANG=C\n"+ + "export FLEET_VAR_HOST_HARDWARE_SERIAL='s' FLEET_VAR_HOST_UUID='u'\n"+ + "LC_ALL=${__fleet_lc}; LANG=${__fleet_lg}; unset __fleet_lc __fleet_lg\n", got) + }) + + // a missing guard is invisible on a UTF-8-only machine + t.Run("posix always emits the locale guard", func(t *testing.T) { + for name, value := range payloadCorpus() { + got := Preamble(map[string]string{"HOST_UUID": value}, DialectPOSIX) + require.Contains(t, got, "LC_ALL=C; LANG=C", name) + require.Contains(t, got, "LC_ALL=${__fleet_lc}", name) + } + }) + + t.Run("powershell is one CRLF line per variable", func(t *testing.T) { + got := Preamble(map[string]string{"HOST_UUID": "AB", "HOST_PLATFORM": "w"}, DialectPowerShell) + require.Equal(t, + "$FLEET_VAR_HOST_PLATFORM = ([char[]](119) -join '')\r\n"+ + "$FLEET_VAR_HOST_UUID = ([char[]](65,66) -join '')\r\n", got) + }) +} + +func TestInsertPreamble(t *testing.T) { + const pre = "PRE\n" + + for _, tc := range []struct { + name string + contents string + want string + }{ + {"no shebang", "echo hi\n", "PRE\necho hi\n"}, + {"sh shebang", "#!/bin/sh\necho hi\n", "#!/bin/sh\nPRE\necho hi\n"}, + {"env bash shebang", "#!/usr/bin/env bash\necho hi\n", "#!/usr/bin/env bash\nPRE\necho hi\n"}, + {"shebang with args", "#!/bin/sh -e\necho hi\n", "#!/bin/sh -e\nPRE\necho hi\n"}, + {"crlf", "#!/bin/sh\r\necho hi\r\n", "#!/bin/sh\r\nPRE\necho hi\r\n"}, + {"shebang only, no newline", "#!/bin/sh", "#!/bin/sh\nPRE\n"}, + {"comment first line", "# not a shebang\necho hi\n", "PRE\n# not a shebang\necho hi\n"}, + {"empty", "", "PRE\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + got, err := InsertPreamble(tc.contents, pre, DialectPOSIX) + require.NoError(t, err) + require.Equal(t, tc.want, got) + }) + } + + t.Run("empty preamble is a no-op", func(t *testing.T) { + got, err := InsertPreamble("#!/bin/sh\necho hi\n", "", DialectPOSIX) + require.NoError(t, err) + require.Equal(t, "#!/bin/sh\necho hi\n", got) + }) + + t.Run("powershell prepends", func(t *testing.T) { + got, err := InsertPreamble("Write-Output hi\r\n", pre, DialectPowerShell) + require.NoError(t, err) + require.Equal(t, "PRE\nWrite-Output hi\r\n", got) + }) +} + +// A preamble above a BOM, a using statement, or a param() block is a parse error. +func TestInsertPreamblePowerShellPlacement(t *testing.T) { + const pre = "PRE\r\n" + const bom = "\ufeff" + + for _, tc := range []struct { + name string + contents string + want string // "" means the insert must be refused + }{ + // a param() block has to stay first, so the preamble goes after it + {"leading param", "param($Foo)\r\nWrite-Output hi\r\n", + "param($Foo)\r\n" + pre + "Write-Output hi\r\n"}, + {"uppercase param", "PARAM($Foo)\r\n", "PARAM($Foo)\r\n" + pre}, + {"attribute then param", "[CmdletBinding()]\r\nparam($Foo)\r\n", + "[CmdletBinding()]\r\nparam($Foo)\r\n" + pre}, + {"two attributes then param", "[CmdletBinding()]\r\n[OutputType([int])]\r\nparam($Foo)\r\n", + "[CmdletBinding()]\r\n[OutputType([int])]\r\nparam($Foo)\r\n" + pre}, + {"comments above param", "# c\r\n#Requires -Version 5\r\nparam($Foo)\r\n", + "# c\r\n#Requires -Version 5\r\nparam($Foo)\r\n" + pre}, + {"help block above param", "<#\r\n.SYNOPSIS\r\n#>\r\nparam($Foo)\r\n", + "<#\r\n.SYNOPSIS\r\n#>\r\nparam($Foo)\r\n" + pre}, + {"using above param", "using namespace System.Text\r\nparam($Foo)\r\n", + "using namespace System.Text\r\nparam($Foo)\r\n" + pre}, + {"bom above param", bom + "param($Foo)\r\n", bom + "param($Foo)\r\n" + pre}, + {"multiline param", "param(\r\n [string]$Foo = \"a)b\",\r\n [int]$N = 2\r\n)\r\nWrite-Output hi\r\n", + "param(\r\n [string]$Foo = \"a)b\",\r\n [int]$N = 2\r\n)\r\n" + pre + "Write-Output hi\r\n"}, + {"param with quoted paren", "param($Foo = 'a)b')\r\n", "param($Foo = 'a)b')\r\n" + pre}, + {"param with comment", "param(\r\n # note\r\n $Foo\r\n)\r\n", "param(\r\n # note\r\n $Foo\r\n)\r\n" + pre}, + {"param with block comment", "param(<# note #>$Foo)\r\n", "param(<# note #>$Foo)\r\n" + pre}, + + // a default can't reference a variable the preamble defines below it + {"param default uses a variable", "param($Foo = $FLEET_VAR_HOST_UUID)\r\n", ""}, + {"unterminated param", "param($Foo\r\nWrite-Output hi\r\n", ""}, + + {"ordinary script", "Write-Output hi\r\n", pre + "Write-Output hi\r\n"}, + {"empty", "", pre}, + {"comment only", "# nothing\r\n", pre + "# nothing\r\n"}, + {"requires only", "#Requires -Version 5\r\nWrite-Output hi\r\n", + pre + "#Requires -Version 5\r\nWrite-Output hi\r\n"}, + {"help block, no param", "<#\r\n.SYNOPSIS\r\n#>\r\nWrite-Output hi\r\n", + pre + "<#\r\n.SYNOPSIS\r\n#>\r\nWrite-Output hi\r\n"}, + {"unterminated help block", "<#\r\nWrite-Output hi\r\n", pre + "<#\r\nWrite-Output hi\r\n"}, + {"param inside function", "function F { param($a) $a }\r\n", pre + "function F { param($a) $a }\r\n"}, + {"parameter-like name", "paramX($Foo)\r\n", pre + "paramX($Foo)\r\n"}, + {"type accelerator", "[Net.ServicePointManager]::SecurityProtocol = 1\r\n", + pre + "[Net.ServicePointManager]::SecurityProtocol = 1\r\n"}, + {"array literal", "[int[]]$a = 1,2\r\n", pre + "[int[]]$a = 1,2\r\n"}, + + // moving the BOM off line 1 breaks the script + {"bom", bom + "Write-Output hi\r\n", bom + pre + "Write-Output hi\r\n"}, + {"bom and comment", bom + "# c\r\nWrite-Output hi\r\n", bom + pre + "# c\r\nWrite-Output hi\r\n"}, + + // using statements must precede every other statement + {"using", "using namespace System.Text\r\nWrite-Output hi\r\n", + "using namespace System.Text\r\n" + pre + "Write-Output hi\r\n"}, + {"two using", "using namespace System.Text\r\nusing namespace System.IO\r\nWrite-Output hi\r\n", + "using namespace System.Text\r\nusing namespace System.IO\r\n" + pre + "Write-Output hi\r\n"}, + {"comment then using", "# c\r\nusing namespace System.Text\r\nWrite-Output hi\r\n", + "# c\r\nusing namespace System.Text\r\n" + pre + "Write-Output hi\r\n"}, + {"bom and using", bom + "using namespace System.Text\r\nWrite-Output hi\r\n", + bom + "using namespace System.Text\r\n" + pre + "Write-Output hi\r\n"}, + {"using without trailing newline", "using namespace System.Text", + "using namespace System.Text\r\n" + pre}, + {"using-like name", "usingX 1\r\n", pre + "usingX 1\r\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + got, err := InsertPreamble(tc.contents, pre, DialectPowerShell) + if tc.want == "" { + require.ErrorIs(t, err, ErrPowerShellLeadingParamBlock) + return + } + require.NoError(t, err) + require.Equal(t, tc.want, got) + }) + } +} diff --git a/server/variables/variables.go b/server/variables/variables.go index c4e1a3a24ad..78ea8e7e0d4 100644 --- a/server/variables/variables.go +++ b/server/variables/variables.go @@ -93,6 +93,11 @@ func Dedupe(varsWithDupes []string) []string { // // For example, Replace(content, "HOST_UUID", "123-456") will replace both // $FLEET_VAR_HOST_UUID and ${FLEET_VAR_HOST_UUID} with "123-456". +// +// It performs no escaping. For content an interpreter will execute, never pass a +// raw value: the quoting context at the substitution point is unknown. Use +// Preamble, or an encoding that leaves the value inert at any position, such as +// PythonEscape. func Replace(contents string, variableName string, value string) string { // Replace both braced and non-braced versions result := strings.ReplaceAll(contents, "$FLEET_VAR_"+variableName, value) @@ -100,6 +105,26 @@ func Replace(contents string, variableName string, value string) string { return result } +// PythonEscape returns value as Python \U escapes, one per code point, for +// substitution into Python source, which has no $VAR expansion of its own. Only +// a backslash, "U" and hex digits reach the script, so the value can't close a +// string literal, and outside one the leading backslash is a line continuation +// so the file won't parse. Escaping every character rather than just the +// dangerous ones is what buys that second property: executable Python needs no +// quotes at all. +func PythonEscape(value string) string { + const hexDigits = "0123456789abcdef" + var b strings.Builder + b.Grow(len(value) * 10) + for _, r := range value { + b.WriteString(`\U`) + for shift := 28; shift >= 0; shift -= 4 { + b.WriteByte(hexDigits[(r>>shift)&0xf]) + } + } + return b.String() +} + // Contains checks if the given content contains any Fleet variables. func Contains(contents string) bool { return fleetVariableRegex.MatchString(contents) diff --git a/server/variables/variables_test.go b/server/variables/variables_test.go index 45aa162107c..04e83d356fd 100644 --- a/server/variables/variables_test.go +++ b/server/variables/variables_test.go @@ -1,9 +1,12 @@ package variables import ( + "strconv" + "strings" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestFind(t *testing.T) { @@ -175,3 +178,31 @@ func TestReplace(t *testing.T) { }) } } + +func TestPythonEscape(t *testing.T) { + require.Equal(t, `\U00000041\U00000042`, PythonEscape("AB")) + require.Empty(t, PythonEscape("")) + + for name, value := range payloadCorpus() { + t.Run(name, func(t *testing.T) { + got := PythonEscape(value) + + // nothing is left that could close a literal or start an expression + for _, r := range got { + require.Contains(t, `\U0123456789abcdef`, string(r), + "unexpected character %q in %q", r, got) + } + + var decoded strings.Builder + for field := range strings.SplitSeq(got, `\U`) { + if field == "" { + continue + } + cp, err := strconv.ParseInt(field, 16, 32) + require.NoError(t, err) + decoded.WriteRune(rune(cp)) + } + require.Equal(t, value, decoded.String()) + }) + } +}