Skip to content

Commit

Permalink
Interpret windows exit codes as a signed integer (#18282)
Browse files Browse the repository at this point in the history
#17695

The windows exit code is a 32-bit unsigned integer, but the command
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

FIxed on both the client and server side.
  • Loading branch information
dantecatalfamo authored and sharon-fdm committed Apr 16, 2024
1 parent 1f4bfc0 commit efdb9b1
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 2 deletions.
2 changes: 2 additions & 0 deletions 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
1 change: 1 addition & 0 deletions orbit/changes/18282-signed-windows-exit-code
@@ -0,0 +1 @@
* Cast windows exit codes to signed integers to match windows interpreter
9 changes: 8 additions & 1 deletion orbit/pkg/scripts/exec_windows.go
Expand Up @@ -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
}
7 changes: 6 additions & 1 deletion server/datastore/mysql/scripts.go
Expand Up @@ -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,
)
Expand Down
21 changes: 21 additions & 0 deletions server/datastore/mysql/scripts_test.go
Expand Up @@ -4,6 +4,7 @@ import (
"context"
_ "embed"
"fmt"
"math"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit efdb9b1

Please sign in to comment.