diff --git a/changes/18282-signed-windows-exit-code b/changes/18282-signed-windows-exit-code new file mode 100644 index 000000000000..b321bf4a03df --- /dev/null +++ b/changes/18282-signed-windows-exit-code @@ -0,0 +1,2 @@ +* Cast windows exit codes to signed integers to match windows interpreter +* Fix bug where some scripts got stuck in "upcoming" activity permanently diff --git a/orbit/changes/18282-signed-windows-exit-code b/orbit/changes/18282-signed-windows-exit-code new file mode 100644 index 000000000000..a0488663d42d --- /dev/null +++ b/orbit/changes/18282-signed-windows-exit-code @@ -0,0 +1 @@ +* Cast windows exit codes to signed integers to match windows interpreter diff --git a/orbit/pkg/scripts/exec_windows.go b/orbit/pkg/scripts/exec_windows.go index 08bcf358e5a3..a2619a7e6f13 100644 --- a/orbit/pkg/scripts/exec_windows.go +++ b/orbit/pkg/scripts/exec_windows.go @@ -17,7 +17,14 @@ func execCmd(ctx context.Context, scriptPath string) (output []byte, exitCode in cmd.Dir = filepath.Dir(scriptPath) output, err = cmd.CombinedOutput() if cmd.ProcessState != nil { - exitCode = cmd.ProcessState.ExitCode() + // The windows exit code is a 32-bit unsigned integer, but the + // interpreter treats it like a signed integer. When a process + // is killed, it returns 0xFFFFFFFF (interpreted as -1). We + // convert the integer to an signed 32-bit integer to flip it + // to a -1 to match our expectations, and fit in our db column. + // + // https://en.wikipedia.org/wiki/Exit_status#Windows + exitCode = int(int32(cmd.ProcessState.ExitCode())) } return output, exitCode, err } diff --git a/server/datastore/mysql/scripts.go b/server/datastore/mysql/scripts.go index 49bf64232c1a..24d2730fb9b0 100644 --- a/server/datastore/mysql/scripts.go +++ b/server/datastore/mysql/scripts.go @@ -128,7 +128,12 @@ func (ds *Datastore) SetHostScriptExecutionResult(ctx context.Context, result *f res, err := tx.ExecContext(ctx, updStmt, output, result.Runtime, - result.ExitCode, + // Windows error codes are signed 32-bit integers, but are + // returned as unsigned integers by the windows API. The + // software that receives them is responsible for casting + // it to a 32-bit signed integer. + // See /orbit/pkg/scripts/exec_windows.go + int32(result.ExitCode), result.HostID, result.ExecutionID, ) diff --git a/server/datastore/mysql/scripts_test.go b/server/datastore/mysql/scripts_test.go index 3ccf9f15d6bb..13c69f77ae12 100644 --- a/server/datastore/mysql/scripts_test.go +++ b/server/datastore/mysql/scripts_test.go @@ -4,6 +4,7 @@ import ( "context" _ "embed" "fmt" + "math" "strings" "testing" "time" @@ -220,6 +221,26 @@ func testHostScriptResult(t *testing.T, ds *Datastore) { pending, err = ds.ListPendingHostScriptExecutions(ctx, 1) require.NoError(t, err) require.Empty(t, pending, 0) + + // check that scripts with large unsigned error codes get + // converted to signed error codes + createdUnsignedScript, err := ds.NewHostScriptExecutionRequest(ctx, &fleet.HostScriptRequestPayload{ + HostID: 1, + ScriptContents: "echo", + UserID: &u.ID, + SyncRequest: true, + }) + require.NoError(t, err) + + unsignedScriptResult, err := ds.SetHostScriptExecutionResult(ctx, &fleet.HostScriptResultPayload{ + HostID: 1, + ExecutionID: createdUnsignedScript.ExecutionID, + Output: "foo", + Runtime: 1, + ExitCode: math.MaxUint32, + }) + require.NoError(t, err) + require.EqualValues(t, -1, *unsignedScriptResult.ExitCode) } func testScripts(t *testing.T, ds *Datastore) {