Missing error response for fleetctl mdm run-command

xpkoala commented 1 year ago

Fleet version: main

Operating system: (e.g. macOS 11.2.3)

Web browser: (e.g. Chrome 88.0.4324)

🧑‍💻 Expected behavior

When providing valid xml, but a payload that is not expected, an expected error response would generate.

💥 Actual behavior

Fleet responds with a 500 error.

More info

Pulled from https://github.com/fleetdm/fleet/issues/9643

To reproduce point the --payload at a .mobileconfig file. I don't believe this scenario was covered in the error state responses of the original ticket.

reed@reed fleet % sudo ./build/fleetctl mdm run-command --host reed.localdomain --payload ./test.mobileconfig
Error: run command request: POST /api/latest/fleet/mdm/apple/enqueue received status 500 invalid command: invalid command

gillespi314 commented 1 year ago

@xpkoala @noahtalerman, is the expectation for fleetctl mdm run-command that the payload must have the file extension .xml or are any other file extensions ok?

I am wondering if we can simply validate this at the client level and error before the CLI sends the command to the Fleet server.

roperzh commented 1 year ago

@gillespi314 I'm curious if adjusting the copy to be clearer while leaving the rest as-is might be enough? Generally I would rather rely on the server even if it involves a roundtrip for this kind of validation.

xpkoala commented 1 year ago

@gillespi314 When testing this I was using an example from here.

Test 1: Run the example payload with no modifications. "Error: The payload isn't valid. Please provide a valid MDM command in the form of a plist-encoded XML file: run command request: POST /api/latest/fleet/mdm/apple/enqueue received status 415 Validation Failed: unable to decode plist command"
Test 2: Run the example payload after removing the " Acknowledged " line. "Error: The payload isn't valid XML. Please provide a file with valid XML: plist: error parsing XML property list: missing value in dictionary"
Test 3: Run the example payload after removing the \ key.

panic: assignment to entry in nil map

goroutine 1 [running]:
github.com/fleetdm/fleet/v4/server/service.(*Client).EnqueueCommand(0x16d60fbf3?, {0x14000191250, 0x1, 0x1}, {0x140007bca00, 0x132, 0x200})
    /Users/reed/Documents/Code/fleet/server/service/client_apple_mdm.go:21 +0x160
main.mdmRunCommand.func1(0x14000274780)
    /Users/reed/Documents/Code/fleet/cmd/fleetctl/mdm.go:88 +0x210
github.com/urfave/cli/v2.(*Command).Run(0x1400027e3c0, 0x14000274780, {0x1400018cd70, 0x5, 0x5})
    /Users/reed/go/pkg/mod/github.com/urfave/cli/v2@v2.23.5/command.go:271 +0x804
github.com/urfave/cli/v2.(*Command).Run(0x1400027e500, 0x14000274600, {0x140001ab560, 0x6, 0x6})
    /Users/reed/go/pkg/mod/github.com/urfave/cli/v2@v2.23.5/command.go:264 +0xa24
github.com/urfave/cli/v2.(*Command).Run(0x1400027ea00, 0x140002744c0, {0x140001b2000, 0x7, 0x7})
    /Users/reed/go/pkg/mod/github.com/urfave/cli/v2@v2.23.5/command.go:264 +0xa24
github.com/urfave/cli/v2.(*App).RunContext(0x140000fa960, {0x103aab4b8?, 0x140001ae048}, {0x140001b2000, 0x7, 0x7})
    /Users/reed/go/pkg/mod/github.com/urfave/cli/v2@v2.23.5/app.go:329 +0x5dc
github.com/urfave/cli/v2.(*App).Run(...)
    /Users/reed/go/pkg/mod/github.com/urfave/cli/v2@v2.23.5/app.go:306
main.main()
    /Users/reed/Documents/Code/fleet/cmd/fleetctl/fleetctl.go:29 +0x78

I think we want to catch the third case and return something a bit more user friendly.

The example xml used is the following:

PUT /your/url HTTP/1.1
Host: www.yourhostname.com
Content-Length: 1234
Content-Type: application/x-apple-aspen-mdm; charset=UTF-8

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0"> 
<dict>
    <key>UDID</key>
    <string>...</string>
    <key>CommandUUID</key>
    <string>9F09D114-BCFD-42AD-A974-371AA7D6256E</string>
    <key>Status</key>
    <string>Acknowledged</string> 
</dict>
</plist>

gillespi314 commented 1 year ago

@xpkoala, just to be sure we're on the same page, would you mind copy pasting the exact xml that you used for the third case?

xpkoala commented 1 year ago

@gillespi314

PUT /your/url HTTP/1.1
Host: www.yourhostname.com
Content-Length: 1234
Content-Type: application/x-apple-aspen-mdm; charset=UTF-8

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0"> 

</plist>

fleet-release commented 1 year ago

Error response shines, Guides users through the clouds, Fleet's clarity blooms.

fleetdm / fleet