diff --git a/changes/42735-fix-inconsistent-error-message b/changes/42735-fix-inconsistent-error-message new file mode 100644 index 00000000000..8d49c2f7a8f --- /dev/null +++ b/changes/42735-fix-inconsistent-error-message @@ -0,0 +1 @@ +- Added a software package maximum size error message in the UI to fix inconsistent errors across different browsers. diff --git a/cmd/fleetctl/fleetctl/testdata/expectedGetConfigIncludeServerConfigJson.json b/cmd/fleetctl/fleetctl/testdata/expectedGetConfigIncludeServerConfigJson.json index b7af3a10971..9361a721606 100644 --- a/cmd/fleetctl/fleetctl/testdata/expectedGetConfigIncludeServerConfigJson.json +++ b/cmd/fleetctl/fleetctl/testdata/expectedGetConfigIncludeServerConfigJson.json @@ -280,6 +280,7 @@ } } }, + "max_software_package_size": 537919488, "gitops": { "gitops_mode_enabled": false, "repository_url": "", diff --git a/cmd/fleetctl/fleetctl/testdata/expectedGetConfigIncludeServerConfigYaml.yml b/cmd/fleetctl/fleetctl/testdata/expectedGetConfigIncludeServerConfigYaml.yml index c5c86417985..cbdcf220595 100644 --- a/cmd/fleetctl/fleetctl/testdata/expectedGetConfigIncludeServerConfigYaml.yml +++ b/cmd/fleetctl/fleetctl/testdata/expectedGetConfigIncludeServerConfigYaml.yml @@ -158,6 +158,7 @@ spec: max_backups: 0 max_size: 500 plugin: filesystem + max_software_package_size: 537919488 org_info: org_logo_url: "" org_logo_url_light_background: "" diff --git a/frontend/__mocks__/configMock.ts b/frontend/__mocks__/configMock.ts index e686161d61a..58b34a16dc5 100644 --- a/frontend/__mocks__/configMock.ts +++ b/frontend/__mocks__/configMock.ts @@ -234,6 +234,7 @@ const DEFAULT_CONFIG_MOCK: IConfig = { secrets: true, }, }, + max_software_package_size: 10 * 1024 * 1024 * 1024, }; export const createMockConfig = (overrides?: Partial): IConfig => { diff --git a/frontend/interfaces/config.ts b/frontend/interfaces/config.ts index 6d57fc343c2..dbefacd8d20 100644 --- a/frontend/interfaces/config.ts +++ b/frontend/interfaces/config.ts @@ -243,6 +243,7 @@ export interface IConfig { mdm: IMdmConfig; gitops: IGitOpsModeConfig; partnerships?: IFleetPartnerships; + max_software_package_size: number; } interface IFleetPartnerships { diff --git a/frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditSoftwareModal/helpers.tsx b/frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditSoftwareModal/helpers.tsx index 8f2107da928..a5edfea162c 100644 --- a/frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditSoftwareModal/helpers.tsx +++ b/frontend/pages/SoftwarePage/SoftwareTitleDetailsPage/EditSoftwareModal/helpers.tsx @@ -10,7 +10,7 @@ import { } from "pages/SoftwarePage/helpers"; import { ensurePeriod } from "pages/SoftwarePage/SoftwareAddPage/helpers"; -const EDIT_SOFTWARE_ERROR_PREFIX = "Couldn't edit software."; +export const EDIT_SOFTWARE_ERROR_PREFIX = "Couldn't edit software."; const DEFAULT_ERROR_MESSAGE = `${EDIT_SOFTWARE_ERROR_PREFIX} Please try again.`; // eslint-disable-next-line import/prefer-default-export diff --git a/frontend/pages/SoftwarePage/components/forms/PackageForm/PackageForm.tests.tsx b/frontend/pages/SoftwarePage/components/forms/PackageForm/PackageForm.tests.tsx index 3907f7ff0b9..cc6cf995e2c 100644 --- a/frontend/pages/SoftwarePage/components/forms/PackageForm/PackageForm.tests.tsx +++ b/frontend/pages/SoftwarePage/components/forms/PackageForm/PackageForm.tests.tsx @@ -1,8 +1,11 @@ import React from "react"; import { screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; import { createCustomRenderer } from "test/test-utils"; import { createMockSoftwarePackage } from "__mocks__/softwareMock"; +import { notify } from "components/ToastNotification"; +import { IConfig } from "interfaces/config"; import PackageForm from "./PackageForm"; @@ -14,7 +17,8 @@ const BASE_PROPS = { }; const renderForm = ( - overrides: Partial> = {} + overrides: Partial> = {}, + config?: Partial ) => { const render = createCustomRenderer({ withBackendMock: true, @@ -22,12 +26,24 @@ const renderForm = ( app: { isPremiumTier: true, isGlobalAdmin: true, + config, }, }, }); return render(); }; +const ONE_GIB = 1024 * 1024 * 1024; + +// The form reads File.size, so fake the size rather than allocating a real +// multi-gigabyte buffer. +const selectFileOfSize = async (container: HTMLElement, size: number) => { + const file = new File(["installer"], "test.pkg"); + Object.defineProperty(file, "size", { value: size }); + const input = container.querySelector("#upload-file") as HTMLInputElement; + await userEvent.upload(input, file); +}; + const TARGET_BANNER_COPY = /If multiple packages of the same software target the same host, Fleet will install the one that was added first\./i; describe("PackageForm", () => { @@ -63,4 +79,53 @@ describe("PackageForm", () => { expect(screen.getByLabelText("Custom")).toBeInTheDocument(); }); }); + + describe("Maximum package size", () => { + afterEach(() => { + jest.restoreAllMocks(); + }); + + it("rejects a package over the configured maximum before uploading", async () => { + const errorSpy = jest.spyOn(notify, "error"); + const { container } = renderForm( + {}, + { max_software_package_size: ONE_GIB } + ); + + await selectFileOfSize(container, ONE_GIB + 1); + + expect(errorSpy).toHaveBeenCalledWith( + "Couldn't add. The maximum file size is 1GiB." + ); + // The rejected file never reaches form state, so the Target section + // stays hidden. + expect(screen.queryByLabelText("All hosts")).not.toBeInTheDocument(); + }); + + it("rejects any package when the limit is zero", async () => { + // A zero limit is a real setting, not a missing one, and the server + // refuses every upload under it. + const errorSpy = jest.spyOn(notify, "error"); + const { container } = renderForm({}, { max_software_package_size: 0 }); + + await selectFileOfSize(container, 1); + + expect(errorSpy).toHaveBeenCalledWith( + "Couldn't add. The maximum file size is 0B." + ); + }); + + it("accepts a package at the configured maximum", async () => { + const errorSpy = jest.spyOn(notify, "error"); + const { container } = renderForm( + {}, + { max_software_package_size: ONE_GIB } + ); + + await selectFileOfSize(container, ONE_GIB); + + expect(errorSpy).not.toHaveBeenCalled(); + expect(screen.getByLabelText("All hosts")).toBeInTheDocument(); + }); + }); }); diff --git a/frontend/pages/SoftwarePage/components/forms/PackageForm/PackageForm.tsx b/frontend/pages/SoftwarePage/components/forms/PackageForm/PackageForm.tsx index 5f5dd1cb446..9756caa9c84 100644 --- a/frontend/pages/SoftwarePage/components/forms/PackageForm/PackageForm.tsx +++ b/frontend/pages/SoftwarePage/components/forms/PackageForm/PackageForm.tsx @@ -1,10 +1,12 @@ // Used in AddPackageModal.tsx and EditSoftwareModal.tsx -import React, { useState, useEffect, useCallback } from "react"; +import React, { useState, useEffect, useCallback, useContext } from "react"; import classnames from "classnames"; +import { AppContext } from "context/app"; import useGitOpsMode from "hooks/useGitOpsMode"; import { LEARN_MORE_ABOUT_BASE_LINK } from "utilities/constants"; import { + formatFileSize, getExtensionFromFileName, getFileDetails, } from "utilities/file/fileUtils"; @@ -33,11 +35,17 @@ import { import { DropdownTargetLabelSelector } from "components/TargetLabelSelector"; import SoftwareOptionsSelector from "pages/SoftwarePage/components/forms/SoftwareOptionsSelector"; import { GitOpsCustomPackageBanner } from "pages/SoftwarePage/SoftwareAddPage/SoftwareCustomPackage/SoftwareCustomPackage"; +import { ADD_SOFTWARE_ERROR_PREFIX } from "pages/SoftwarePage/SoftwareAddPage/helpers"; +import { EDIT_SOFTWARE_ERROR_PREFIX } from "pages/SoftwarePage/SoftwareTitleDetailsPage/EditSoftwareModal/helpers"; import InfoBanner from "components/InfoBanner"; import CustomLink from "components/CustomLink"; import PackageAdvancedOptions from "../PackageAdvancedOptions"; -import { createTooltipContent, generateFormValidation } from "./helpers"; +import { + createTooltipContent, + estimateUploadSize, + generateFormValidation, +} from "./helpers"; import SoftwareDeploySlider from "../SoftwareDeploySelector"; export const baseClass = "package-form"; @@ -181,6 +189,8 @@ const PackageForm = ({ initialTargetType, }: IPackageFormProps) => { const { gitOpsModeEnabled, repoURL } = useGitOpsMode("software"); + const { config } = useContext(AppContext); + const maxSoftwarePackageSize = config?.max_software_package_size; const initialFormData: IPackageFormData = { // `formData.software` is typed as `File | null` (its shape once a user @@ -209,10 +219,30 @@ const PackageForm = ({ software: { isValid: false }, }); + const notifyTooLarge = () => { + const errorPrefix = isEditingSoftware + ? EDIT_SOFTWARE_ERROR_PREFIX + : ADD_SOFTWARE_ERROR_PREFIX; + notify.error( + `${errorPrefix} The maximum file size is ${formatFileSize( + maxSoftwarePackageSize || 0 + )}.` + ); + }; + const onFileSelect = (files: FileList | null) => { if (files && files.length > 0) { const file = files[0]; + // Reject before uploading if file size is too big + if ( + maxSoftwarePackageSize !== undefined && + file.size > maxSoftwarePackageSize + ) { + notifyTooLarge(); + return; + } + // Only populate default install/uninstall scripts when adding (but not editing) software if (isEditingSoftware) { const newData = { ...formData, software: file }; @@ -250,6 +280,16 @@ const PackageForm = ({ const onFormSubmit = (evt: React.FormEvent) => { evt.preventDefault(); + + // The server caps the whole request body, and not just the file. + if ( + maxSoftwarePackageSize !== undefined && + estimateUploadSize(formData) > maxSoftwarePackageSize + ) { + notifyTooLarge(); + return; + } + onSubmit(formData); }; diff --git a/frontend/pages/SoftwarePage/components/forms/PackageForm/helpers.tsx b/frontend/pages/SoftwarePage/components/forms/PackageForm/helpers.tsx index f7386a99f77..1c9d4665ae8 100644 --- a/frontend/pages/SoftwarePage/components/forms/PackageForm/helpers.tsx +++ b/frontend/pages/SoftwarePage/components/forms/PackageForm/helpers.tsx @@ -1,7 +1,9 @@ import React from "react"; import { validateQuery } from "components/forms/validators/validate_query"; +import { listNamesFromSelectedLabels } from "services/entities/labels"; import { getExtensionFromFileName } from "utilities/file/fileUtils"; +import { encodeScriptBase64 } from "utilities/scripts_encoding"; import { getGitOpsModeTipContent } from "utilities/helpers"; import { IPackageFormData, IPackageFormValidation } from "./PackageForm"; @@ -236,4 +238,47 @@ export const createTooltipContent = ( ); }; +/** Calculates the size of the payload, because the server limits the whole + * request and not just the installer file. Not all fields are accounted for + * in this calculation so if the final payload sent is over the size limit, + * the server will reject it. + */ +export const estimateUploadSize = (formData: IPackageFormData) => { + const scripts = [ + formData.installScript, + formData.uninstallScript, + formData.preInstallQuery, + formData.postInstallScript, + ]; + + // The scripts are base64 encoded on the way out, so encode them to get the + // length that actually goes over the wire. + let scriptsSize = 0; + scripts.forEach((script) => { + scriptsSize += encodeScriptBase64(script)?.length || 0; + }); + + // The two flags are sent as "true" or "false". + let fieldsSize = + String(formData.selfService).length + + String(formData.automaticInstall).length; + + // Names are user written, so count bytes. Anything outside ASCII takes more + // than one. + const encoder = new TextEncoder(); + + formData.categories.forEach((category) => { + fieldsSize += encoder.encode(category).length; + }); + + // Labels are only sent when the target is Custom, and only the selected ones. + if (formData.targetType === "Custom") { + listNamesFromSelectedLabels(formData.labelTargets).forEach((label) => { + fieldsSize += encoder.encode(label).length; + }); + } + + return (formData.software?.size || 0) + scriptsSize + fieldsSize; +}; + export default generateFormValidation; diff --git a/frontend/utilities/file/fileUtils.tests.tsx b/frontend/utilities/file/fileUtils.tests.tsx index ee1c492a986..21376ed4843 100644 --- a/frontend/utilities/file/fileUtils.tests.tsx +++ b/frontend/utilities/file/fileUtils.tests.tsx @@ -1,4 +1,5 @@ import { + formatFileSize, getExtensionFromFileName, getFileDetails, getPlatformDisplayName, @@ -120,4 +121,30 @@ describe("fileUtils", () => { description: "macOS", }); }); + + describe("fileUtils - formatFileSize", () => { + // Expectations verified against the server's installersize.Human, which is + // what writes the same limit into its own too-large error + const testCases = [ + { bytes: 0, expectedSize: "0B" }, + { bytes: 999, expectedSize: "999B" }, + { bytes: 1000, expectedSize: "1kB" }, + { bytes: 1024, expectedSize: "1KiB" }, + { bytes: 1000000, expectedSize: "1MB" }, + { bytes: 1048576, expectedSize: "1MiB" }, + { bytes: 536870912, expectedSize: "512MiB" }, + { bytes: 1073741824, expectedSize: "1GiB" }, + { bytes: 1500000000, expectedSize: "1.5GB" }, + { bytes: 10737418240, expectedSize: "10GiB" }, + { bytes: 5497558138880, expectedSize: "5TiB" }, + { bytes: 1000000000000000, expectedSize: "1PB" }, + { bytes: 1125899906842624, expectedSize: "1PiB" }, + ]; + + testCases.forEach(({ bytes, expectedSize }) => { + it(`should return "${expectedSize}" for ${bytes} bytes`, () => { + expect(formatFileSize(bytes)).toEqual(expectedSize); + }); + }); + }); }); diff --git a/frontend/utilities/file/fileUtils.tsx b/frontend/utilities/file/fileUtils.tsx index daf64154539..383cc61d02f 100644 --- a/frontend/utilities/file/fileUtils.tsx +++ b/frontend/utilities/file/fileUtils.tsx @@ -102,3 +102,51 @@ export interface IFileDetails { name: string; description?: React.ReactNode; } + +// Both tables match the ones go-units gives the server, so the two agree at +// every magnitude rather than only up to terabytes. +const DECIMAL_ABBREVIATIONS = [ + "B", + "kB", + "MB", + "GB", + "TB", + "PB", + "EB", + "ZB", + "YB", +]; +const BINARY_ABBREVIATIONS = [ + "B", + "KiB", + "MiB", + "GiB", + "TiB", + "PiB", + "EiB", + "ZiB", + "YiB", +]; + +const formatWithBase = ( + bytes: number, + base: number, + abbreviations: string[] +) => { + let size = bytes; + let abbreviationIndex = 0; + while (size >= base && abbreviationIndex < abbreviations.length - 1) { + size /= base; + abbreviationIndex += 1; + } + // 4 significant digits with trailing zeros dropped, matching Go's "%.4g" + return `${Number(size.toPrecision(4))}${abbreviations[abbreviationIndex]}`; +}; + +// Returns a human readable size, like the server's installersize.Human function +export const formatFileSize = (bytes: number) => { + const decimal = formatWithBase(bytes, 1000, DECIMAL_ABBREVIATIONS); + const binary = formatWithBase(bytes, 1024, BINARY_ABBREVIATIONS); + + return binary.length < decimal.length ? binary : decimal; +}; diff --git a/server/fleet/app.go b/server/fleet/app.go index 5a3067cda06..9490c0e0175 100644 --- a/server/fleet/app.go +++ b/server/fleet/app.go @@ -1054,11 +1054,12 @@ type EnrichedAppConfig struct { // enrichedAppConfigFields are grouped separately to aid with JSON unmarshaling type enrichedAppConfigFields struct { - UpdateInterval *UpdateIntervalConfig `json:"update_interval,omitempty"` - Vulnerabilities *VulnerabilitiesConfig `json:"vulnerabilities,omitempty"` - License *LicenseInfo `json:"license,omitempty"` - Logging *Logging `json:"logging,omitempty"` - Email *EmailConfig `json:"email,omitempty"` + UpdateInterval *UpdateIntervalConfig `json:"update_interval,omitempty"` + Vulnerabilities *VulnerabilitiesConfig `json:"vulnerabilities,omitempty"` + License *LicenseInfo `json:"license,omitempty"` + Logging *Logging `json:"logging,omitempty"` + Email *EmailConfig `json:"email,omitempty"` + MaxSoftwarePackageSize int64 `json:"max_software_package_size"` } // UnmarshalJSON implements the json.Unmarshaler interface to make sure we serialize diff --git a/server/fleet/service.go b/server/fleet/service.go index d59a29d956b..8dfe1b16434 100644 --- a/server/fleet/service.go +++ b/server/fleet/service.go @@ -547,6 +547,8 @@ type Service interface { AppConfigObfuscated(ctx context.Context) (info *AppConfig, err error) ModifyAppConfig(ctx context.Context, p []byte, applyOpts ApplySpecOptions) (info *AppConfig, err error) SandboxEnabled() bool + // MaxInstallerSizeBytes returns the configured maximum size for software installer uploads. + MaxInstallerSizeBytes() int64 AppConfigUrls(ctx context.Context) (urls *AppConfigUrls, err error) // ApplyEnrollSecretSpec adds and updates the enroll secrets specified in the spec. diff --git a/server/mock/service/service_mock.go b/server/mock/service/service_mock.go index 97af458d2cd..e407cd8cab7 100644 --- a/server/mock/service/service_mock.go +++ b/server/mock/service/service_mock.go @@ -316,6 +316,8 @@ type ModifyAppConfigFunc func(ctx context.Context, p []byte, applyOpts fleet.App type SandboxEnabledFunc func() bool +type MaxInstallerSizeBytesFunc func() int64 + type AppConfigUrlsFunc func(ctx context.Context) (urls *fleet.AppConfigUrls, err error) type ApplyEnrollSecretSpecFunc func(ctx context.Context, spec *fleet.EnrollSecretSpec, applyOpts fleet.ApplySpecOptions) error @@ -1431,6 +1433,9 @@ type Service struct { SandboxEnabledFunc SandboxEnabledFunc SandboxEnabledFuncInvoked bool + MaxInstallerSizeBytesFunc MaxInstallerSizeBytesFunc + MaxInstallerSizeBytesFuncInvoked bool + AppConfigUrlsFunc AppConfigUrlsFunc AppConfigUrlsFuncInvoked bool @@ -3475,6 +3480,13 @@ func (s *Service) SandboxEnabled() bool { return s.SandboxEnabledFunc() } +func (s *Service) MaxInstallerSizeBytes() int64 { + s.mu.Lock() + s.MaxInstallerSizeBytesFuncInvoked = true + s.mu.Unlock() + return s.MaxInstallerSizeBytesFunc() +} + func (s *Service) AppConfigUrls(ctx context.Context) (urls *fleet.AppConfigUrls, err error) { s.mu.Lock() s.AppConfigUrlsFuncInvoked = true diff --git a/server/service/appconfig.go b/server/service/appconfig.go index 440def49ec3..a3cbe164ad1 100644 --- a/server/service/appconfig.go +++ b/server/service/appconfig.go @@ -58,6 +58,8 @@ type appConfigResponseFields struct { SandboxEnabled bool `json:"sandbox_enabled,omitempty"` Err error `json:"error,omitempty"` Partnerships *fleet.Partnerships `json:"partnerships,omitempty"` + // Maximum software package size is loaded from the service. + MaxSoftwarePackageSize int64 `json:"max_software_package_size"` } // UnmarshalJSON implements the json.Unmarshaler interface to make sure we serialize @@ -229,13 +231,14 @@ func getAppConfigEndpoint(ctx context.Context, request interface{}, svc fleet.Se ConditionalAccess: appConfig.ConditionalAccess, }, appConfigResponseFields: appConfigResponseFields{ - UpdateInterval: updateIntervalConfig, - Vulnerabilities: vulnConfig, - License: lic, - Logging: loggingConfig, - Email: emailConfig, - SandboxEnabled: svc.SandboxEnabled(), - Partnerships: partnerships, + UpdateInterval: updateIntervalConfig, + Vulnerabilities: vulnConfig, + License: lic, + Logging: loggingConfig, + Email: emailConfig, + SandboxEnabled: svc.SandboxEnabled(), + Partnerships: partnerships, + MaxSoftwarePackageSize: svc.MaxInstallerSizeBytes(), }, } return response, nil @@ -331,8 +334,9 @@ func modifyAppConfigEndpoint(ctx context.Context, request interface{}, svc fleet response := appConfigResponse{ AppConfig: *appConfig, appConfigResponseFields: appConfigResponseFields{ - License: lic, - Logging: loggingConfig, + License: lic, + Logging: loggingConfig, + MaxSoftwarePackageSize: svc.MaxInstallerSizeBytes(), }, } @@ -2854,3 +2858,7 @@ func isValidHostname(h string) bool { return true } + +func (svc *Service) MaxInstallerSizeBytes() int64 { + return svc.config.Server.MaxInstallerSizeBytes +} diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go index e2e824d4a81..e66a43b63f6 100644 --- a/server/service/integration_core_test.go +++ b/server/service/integration_core_test.go @@ -8869,6 +8869,8 @@ func (s *integrationTestSuite) TestAppConfig() { assert.False(t, acResp.ServerSettings.AIFeaturesDisabled) assert.False(t, acResp.GitOpsConfig.GitopsModeEnabled) assert.Zero(t, acResp.GitOpsConfig.RepositoryURL) + expectedMaxPackageSize := config.TestConfig().Server.MaxInstallerSizeBytes + assert.Equal(t, expectedMaxPackageSize, acResp.MaxSoftwarePackageSize) // set the apple BM terms expired flag, and the enabled and configured flags, // we'll check again at the end of this test to make sure they weren't