diff --git a/.github/workflows/dogfood-gitops.yml b/.github/workflows/dogfood-gitops.yml index 61921b9a7e..3353e27d51 100644 --- a/.github/workflows/dogfood-gitops.yml +++ b/.github/workflows/dogfood-gitops.yml @@ -60,6 +60,7 @@ jobs: DOGFOOD_FAILING_POLICIES_WEBHOOK_URL: ${{ secrets.DOGFOOD_FAILING_POLICIES_WEBHOOK_URL }} DOGFOOD_VULNERABILITIES_WEBHOOK_URL: ${{ secrets.DOGFOOD_VULNERABILITIES_WEBHOOK_URL }} DOGFOOD_WORKSTATIONS_ENROLL_SECRET: ${{ secrets.DOGFOOD_WORKSTATIONS_ENROLL_SECRET }} + DOGFOOD_WORKSTATIONS_CANARY_CALENDAR_WEBHOOK_URL: ${{ secrets.DOGFOOD_WORKSTATIONS_CANARY_CALENDAR_WEBHOOK_URL }} DOGFOOD_WORKSTATIONS_CANARY_ENROLL_SECRET: ${{ secrets.DOGFOOD_WORKSTATIONS_CANARY_ENROLL_SECRET }} DOGFOOD_SERVERS_ENROLL_SECRET: ${{ secrets.DOGFOOD_SERVERS_ENROLL_SECRET }} DOGFOOD_SERVERS_CANARY_ENROLL_SECRET: ${{ secrets.DOGFOOD_SERVERS_CANARY_ENROLL_SECRET }} diff --git a/changes/18060-host-activity-styling-bugs b/changes/18060-host-activity-styling-bugs new file mode 100644 index 0000000000..fb157bbbaf --- /dev/null +++ b/changes/18060-host-activity-styling-bugs @@ -0,0 +1 @@ +- Styling bug fixes of host details page activities (Remove trailing dash line from last activity, Re-instate padding below last activity) diff --git a/changes/18282-signed-windows-exit-code b/changes/18282-signed-windows-exit-code new file mode 100644 index 0000000000..b321bf4a03 --- /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/frontend/pages/DashboardPage/cards/ActivityFeed/ActivityItem/_styles.scss b/frontend/pages/DashboardPage/cards/ActivityFeed/ActivityItem/_styles.scss index 57438f0c7e..d701c91ad5 100644 --- a/frontend/pages/DashboardPage/cards/ActivityFeed/ActivityItem/_styles.scss +++ b/frontend/pages/DashboardPage/cards/ActivityFeed/ActivityItem/_styles.scss @@ -57,7 +57,7 @@ .activity-item__dash { border-right: none; } - .activity-item__details { + .activity-details { padding-bottom: $pad-xxlarge; } } diff --git a/frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/HostActionsDropdown.tests.tsx b/frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/HostActionsDropdown.tests.tsx index cea478dbd7..0c41f4fcda 100644 --- a/frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/HostActionsDropdown.tests.tsx +++ b/frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/HostActionsDropdown.tests.tsx @@ -1,6 +1,6 @@ import React from "react"; import { noop } from "lodash"; -import { screen } from "@testing-library/react"; +import { screen, waitFor } from "@testing-library/react"; import { createCustomRenderer } from "test/test-utils"; import createMockUser from "__mocks__/userMock"; @@ -64,6 +64,126 @@ describe("Host Actions Dropdown", () => { expect(screen.getByText("Transfer")).toBeInTheDocument(); }); }); + describe("Query action", () => { + it("renders the Query action when the user is a global admin and the host is online", async () => { + const render = createCustomRenderer({ + context: { + app: { + isGlobalAdmin: true, + currentUser: createMockUser(), + }, + }, + }); + + const { user } = render( + + ); + + await user.click(screen.getByText("Actions")); + + expect(screen.getByText("Query")).toBeInTheDocument(); + }); + + it("renders the Query action as disabled with a tooltip when a host is offline", async () => { + const render = createCustomRenderer({ + context: { + app: { + isGlobalAdmin: true, + currentUser: createMockUser(), + }, + }, + }); + + const { user } = render( + + ); + + await user.click(screen.getByText("Actions")); + + expect( + screen.getByText("Query").parentElement?.parentElement?.parentElement + ).toHaveClass("is-disabled"); + + await waitFor(() => { + waitFor(() => { + user.hover(screen.getByText("Query")); + }); + + expect( + screen.getByText(/You can't query an offline host./i) + ).toBeInTheDocument(); + }); + }); + + it("renders the Query action as disabled when a host is locked", async () => { + const render = createCustomRenderer({ + context: { + app: { + isGlobalAdmin: true, + currentUser: createMockUser(), + }, + }, + }); + + const { user } = render( + + ); + + await user.click(screen.getByText("Actions")); + expect( + screen.getByText("Query").parentElement?.parentElement?.parentElement + ).toHaveClass("is-disabled"); + }); + + it("renders the Query action as disabled when a host is updating", async () => { + const render = createCustomRenderer({ + context: { + app: { + isGlobalAdmin: true, + currentUser: createMockUser(), + }, + }, + }); + + const { user } = render( + + ); + + await user.click(screen.getByText("Actions")); + + expect(screen.getByText("Query").parentElement).toHaveClass( + "is-disabled" + ); + }); + }); it("renders the Show Disk Encryption Key action when on premium tier and we store the disk encryption key", async () => { const render = createCustomRenderer({ @@ -267,7 +387,7 @@ describe("Host Actions Dropdown", () => { debug(); - expect(screen.getByText("Turn off MDM").parentNode).toHaveClass( + expect(screen.getByText("Turn off MDM").parentElement).toHaveClass( "is-disabled" ); }); @@ -441,6 +561,48 @@ describe("Host Actions Dropdown", () => { expect(screen.getByText("Lock")).toBeInTheDocument(); }); + it("renders as disabled with a tooltip when scripts_enabled is set to false for windows/linux", async () => { + const render = createCustomRenderer({ + context: { + app: { + isPremiumTier: true, + isMacMdmEnabledAndConfigured: true, + isGlobalAdmin: true, + currentUser: createMockUser(), + }, + }, + }); + + const { user } = render( + + ); + + await user.click(screen.getByText("Actions")); + + expect( + screen.getByText("Lock").parentElement?.parentElement?.parentElement + ).toHaveClass("is-disabled"); + + await waitFor(() => { + waitFor(() => { + user.hover(screen.getByText("Lock")); + }); + + expect( + screen.getByText(/fleetd agent with --enable-scripts/i) + ).toBeInTheDocument(); + }); + }); + it("does not render when the host is not enrolled in mdm", async () => { const render = createCustomRenderer({ context: { @@ -653,6 +815,48 @@ describe("Host Actions Dropdown", () => { expect(screen.queryByText("Unlock")).not.toBeInTheDocument(); }); + + it("renders as disabled with a tooltip when scripts_enabled is set to false for windows/linux", async () => { + const render = createCustomRenderer({ + context: { + app: { + isPremiumTier: true, + isMacMdmEnabledAndConfigured: true, + isGlobalAdmin: true, + currentUser: createMockUser(), + }, + }, + }); + + const { user } = render( + + ); + + await user.click(screen.getByText("Actions")); + + expect( + screen.getByText("Unlock").parentElement?.parentElement?.parentElement + ).toHaveClass("is-disabled"); + + await waitFor(() => { + waitFor(() => { + user.hover(screen.getByText("Unlock")); + }); + + expect( + screen.getByText(/fleetd agent with --enable-scripts/i) + ).toBeInTheDocument(); + }); + }); }); describe("Wipe action", () => { @@ -747,5 +951,145 @@ describe("Host Actions Dropdown", () => { expect(screen.queryByText("Wipe")).not.toBeInTheDocument(); }); + + it("renders as disabled with a tooltip when scripts_enabled is set to false for linux", async () => { + const render = createCustomRenderer({ + context: { + app: { + isPremiumTier: true, + isMacMdmEnabledAndConfigured: true, + isGlobalAdmin: true, + currentUser: createMockUser(), + }, + }, + }); + + const { user } = render( + + ); + + await user.click(screen.getByText("Actions")); + + expect( + screen.getByText("Wipe").parentElement?.parentElement?.parentElement + ).toHaveClass("is-disabled"); + + await waitFor(() => { + waitFor(() => { + user.hover(screen.getByText("Wipe")); + }); + + expect( + screen.getByText(/fleetd agent with --enable-scripts/i) + ).toBeInTheDocument(); + }); + }); + }); + + describe("Run script action", () => { + it("renders the Run script action when scripts_enabled is set to true", async () => { + const render = createCustomRenderer({ + context: { + app: { + isGlobalAdmin: true, + currentUser: createMockUser(), + }, + }, + }); + + const { user } = render( + + ); + + await user.click(screen.getByText("Actions")); + + expect(screen.getByText("Run script")).toBeInTheDocument(); + }); + + it("renders the Run script action as disabled with a tooltip when scripts_enabled is set to false", async () => { + const render = createCustomRenderer({ + context: { + app: { + isGlobalAdmin: true, + currentUser: createMockUser(), + }, + }, + }); + + const { user } = render( + + ); + + await user.click(screen.getByText("Actions")); + + expect( + screen.getByText("Run script").parentElement?.parentElement + ?.parentElement + ).toHaveClass("is-disabled"); + + await waitFor(() => { + waitFor(() => { + user.hover(screen.getByText("Run script")); + }); + + expect( + screen.getByText(/fleetd agent with --enable-scripts/i) + ).toBeInTheDocument(); + }); + }); + + it("does not render the Run script action for ChromeOS", async () => { + const render = createCustomRenderer({ + context: { + app: { + isGlobalAdmin: true, + currentUser: createMockUser(), + }, + }, + }); + + const { user } = render( + + ); + + await user.click(screen.getByText("Actions")); + + expect(screen.queryByText("Run script")).not.toBeInTheDocument(); + }); }); }); diff --git a/frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/helpers.tsx b/frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/helpers.tsx index 313e3e8032..2eb35cdf26 100644 --- a/frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/helpers.tsx +++ b/frontend/pages/hosts/details/HostDetailsPage/HostActionsDropdown/helpers.tsx @@ -290,6 +290,7 @@ const setOptionsAsDisabled = ( isSandboxMode, hostMdmDeviceStatus, hostScriptsEnabled, + hostPlatform, }: IHostActionConfigOptions ) => { // Available tooltips for disabled options @@ -339,6 +340,23 @@ const setOptionsAsDisabled = ( optionsToDisable = optionsToDisable.concat( options.filter((option) => option.value === "runScript") ); + if (isLinuxLike(hostPlatform)) { + optionsToDisable = optionsToDisable.concat( + options.filter( + (option) => + option.value === "lock" || + option.value === "unlock" || + option.value === "wipe" + ) + ); + } + if (hostPlatform === "windows") { + optionsToDisable = optionsToDisable.concat( + options.filter( + (option) => option.value === "lock" || option.value === "unlock" + ) + ); + } } if (isSandboxMode) { optionsToDisable = optionsToDisable.concat( diff --git a/frontend/pages/hosts/details/cards/Activity/HostActivityItem/_styles.scss b/frontend/pages/hosts/details/cards/Activity/HostActivityItem/_styles.scss index cfb2fcd282..d6048006a1 100644 --- a/frontend/pages/hosts/details/cards/Activity/HostActivityItem/_styles.scss +++ b/frontend/pages/hosts/details/cards/Activity/HostActivityItem/_styles.scss @@ -54,13 +54,12 @@ } &:last-child { - .past-activity__dash { + .host-activity-item__dash { border-right: none; } - .past-activity__details { + .host-activity-item__details-wrapper { padding-bottom: $pad-xxlarge; } } - } diff --git a/frontend/pages/hosts/details/cards/Activity/UpcomingActivityFeed/UpcomingActivityFeed.tsx b/frontend/pages/hosts/details/cards/Activity/UpcomingActivityFeed/UpcomingActivityFeed.tsx index 94f1d547cd..c73b4b28e2 100644 --- a/frontend/pages/hosts/details/cards/Activity/UpcomingActivityFeed/UpcomingActivityFeed.tsx +++ b/frontend/pages/hosts/details/cards/Activity/UpcomingActivityFeed/UpcomingActivityFeed.tsx @@ -1,6 +1,6 @@ import React from "react"; -import { IActivity, IActivityDetails } from "interfaces/activity"; +import { IActivity } from "interfaces/activity"; import { IActivitiesResponse } from "services/entities/activities"; // @ts-ignore diff --git a/frontend/pages/policies/PolicyPage/components/PolicyForm/PolicyForm.tests.tsx b/frontend/pages/policies/PolicyPage/components/PolicyForm/PolicyForm.tests.tsx index 145908bb0c..1970c20fae 100644 --- a/frontend/pages/policies/PolicyPage/components/PolicyForm/PolicyForm.tests.tsx +++ b/frontend/pages/policies/PolicyPage/components/PolicyForm/PolicyForm.tests.tsx @@ -1,5 +1,5 @@ import React from "react"; -import { screen } from "@testing-library/react"; +import { screen, waitFor } from "@testing-library/react"; import { createCustomRenderer } from "test/test-utils"; import createMockPolicy from "__mocks__/policyMock"; @@ -123,11 +123,15 @@ describe("PolicyForm - component", () => { expect(screen.getByRole("button", { name: "Save" })).toBeDisabled(); expect(screen.getByRole("button", { name: "Run" })).toBeDisabled(); - await user.hover(screen.getByRole("button", { name: "Save" })); + await waitFor(() => { + waitFor(() => { + user.hover(screen.getByRole("button", { name: "Save" })); + }); - expect(container.querySelector("#save-policy-button")).toHaveTextContent( - /to save or run the policy/i - ); + expect(container.querySelector("#save-policy-button")).toHaveTextContent( + /to save or run the policy/i + ); + }); }); it("disables run button with tooltip when live queries are globally disabled", async () => { @@ -190,11 +194,15 @@ describe("PolicyForm - component", () => { expect(screen.getByRole("button", { name: "Run" })).toBeDisabled(); - await user.hover(screen.getByRole("button", { name: "Run" })); + await waitFor(() => { + waitFor(() => { + user.hover(screen.getByRole("button", { name: "Run" })); + }); - expect(container.querySelector("#run-policy-button")).toHaveTextContent( - /live queries are disabled/i - ); + expect( + screen.getByText(/live queries are disabled/i) + ).toBeInTheDocument(); + }); }); // TODO: Consider testing save button is disabled for a sql error diff --git a/orbit/changes/18282-signed-windows-exit-code b/orbit/changes/18282-signed-windows-exit-code new file mode 100644 index 0000000000..a0488663d4 --- /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 08bcf358e5..a2619a7e6f 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 49bf64232c..24d2730fb9 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 3ccf9f15d6..13c69f77ae 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) { diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index 9f8fea2f80..139604ad9b 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -876,8 +876,6 @@ func (s *integrationMDMTestSuite) TestAppleProfileManagement() { errMsg = extractServerErrorText(res.Body) require.Contains(t, errMsg, "Invalid profile UUID prefix") - s.checkMDMProfilesSummaries(t, nil, expectedNoTeamSummary, &expectedNoTeamSummary) // host now verifying global profiles - // set OS updates settings for no-team and team, should not change the // summaries as this profile is ignored. s.Do("PATCH", "/api/latest/fleet/config", json.RawMessage(`{ @@ -888,9 +886,6 @@ func (s *integrationMDMTestSuite) TestAppleProfileManagement() { } } }`), http.StatusOK) - - s.checkMDMProfilesSummaries(t, nil, expectedNoTeamSummary, &expectedNoTeamSummary) // host now verifying global profiles - s.Do("PATCH", fmt.Sprintf("/api/latest/fleet/teams/%d", tm.ID), fleet.TeamPayload{ MDM: &fleet.TeamPayloadMDM{ MacOSUpdates: &fleet.MacOSUpdates{ @@ -899,7 +894,7 @@ func (s *integrationMDMTestSuite) TestAppleProfileManagement() { }, }, }, http.StatusOK) - // s.checkMDMProfilesSummaries(t, nil, expectedNoTeamSummary, &expectedNoTeamSummary) + s.checkMDMProfilesSummaries(t, nil, expectedNoTeamSummary, &expectedNoTeamSummary) s.checkMDMProfilesSummaries(t, &tm.ID, expectedTeamSummary, &expectedTeamSummary) // it should also not show up in the host's profiles list