feat(mdm): initial MDM support for Windows and macOS [WPB-23215]#9451
feat(mdm): initial MDM support for Windows and macOS [WPB-23215]#9451adamlow-wire wants to merge 5 commits intowireapp:devfrom
Conversation
Enterprise managed configuration: read from Windows Registry (Group Policy or user) or macOS preferences; override user settings; cache for process lifetime. Redact proxy credentials for renderer; serve config to webview via IPC (no registry/OS access in renderer). Safe download path with traversal check; locale normalization to supported keys. Remove environment from MDM (use webappUrl + build). Unit tests for parsing, validation, redaction, and test-only override seam. Add docs/mdm-managed-configuration.md for MDM/Group Policy admins.
|
|
adamlow-wire
force-pushed
the
feature/initial-mdm-support
branch
from
ed114ed to
dd8e4b3
Compare
- Use full path to reg.exe (SystemRoot\System32\reg.exe) so execution does not depend on PATH; prevents running a substituted binary. - Replace backtracking-vulnerable regex with prefix match + slice for value to avoid ReDoS when parsing reg query output.
adamlow-wire
force-pushed
the
feature/initial-mdm-support
branch
from
dd8e4b3 to
2c2ef09
Compare
JSDoc (valid-jsdoc), import order, and formatting in new/added code: ManagedConfig, ManagedConfig.test.main, getSafeDownloadDirectory (main), normalizeManagedLocale (locale), mdm-managed-configuration.md.
Import order and formatting in pre-existing files touched by MDM feature. Upstream may omit this commit if they do not want changes to core code. - EnvironmentUtil, ConfigurationPersistence, globals.d.ts - macosAutoUpdater, squirrel, Webview (ref cleanup)
adamlow-wire
changed the title
…h -1728 - ManagedConfig: skip empty string for string keys on macOS (getUserDefault returns "" for unset keys) - ManagedConfig: strip failed keys from config after Joi validation; log only valid keys; include Joi reason in warning - EnvironmentUtil: treat empty/whitespace webapp URL as not set (source + getWebappUrl fallback) - system: skip managed auto-launch when config.name is Electron (dev); treat login item not found (-1728) as success when disabling
|
| const trimmed = relativePath?.trim(); | ||
| if (!trimmed) { | ||
| return null; | ||
| } | ||
| const home = app.getPath('home'); | ||
| const resolved = path.resolve(home, trimmed); | ||
| const normalizedHome = path.resolve(home); | ||
| const normalizedResolved = path.resolve(resolved); | ||
| if (!normalizedResolved.startsWith(normalizedHome + path.sep) && normalizedResolved !== normalizedHome) { | ||
| return null; | ||
| } | ||
| return normalizedResolved; |
There was a problem hiding this comment.
I understand (at least I believe) that the user is not able to download in a directory like ../../../../foo when it escapes your home directory. But the implementation is very hard to read and understand. Could this be refactored to make it more readable? Also an isolated test just for this (very important?) functionality would be great 👍
| const useManagedDownloadPath = | ||
| (EnvironmentUtil.platform.IS_WINDOWS || EnvironmentUtil.platform.IS_MAC_OS) && | ||
| typeof customDownloadPath === 'string' && | ||
| customDownloadPath.length > 0; |
There was a problem hiding this comment.
I think we have a bug in here: before customDownloadPath was also set when running on Linux and the condition evaluated to true. But now it evaluates to false when running on Linux because it only checks for Windows and macOS.
If Linux builds previously relied on init.json downloadPath being enforced that no longer happens now.
|
|
||
| if (useManagedDownloadPath) { | ||
| const safeDir = getSafeDownloadDirectory(customDownloadPath); | ||
| if (safeDir) { |
There was a problem hiding this comment.
This would mean when you use Linux and don't have a managed device, you are not allowed to download the application in different directory outside of /home, right? (this was possible before)
| const _setImmediate = setImmediate; | ||
|
|
||
| process.once('loaded', () => { | ||
| process.once('loaded', async () => { |
There was a problem hiding this comment.
process.once('loaded') does not allow to put in functions that returns Promises. All of the code in there must be synchronous. The reason why nothing complains here is because of the lax compiler / linter settings in this repository.
It would also have the side effect that global.desktopAppConfig is set after an async round‑trip (it was synchronous before). If the application expects desktopAppConfig to be available immediately at document start this could break on Linux without MDM.
| } catch (error: unknown) { | ||
| const message = error instanceof Error ? error.message : String(error); | ||
| const isLoginItemNotFound = | ||
| message.includes('-1728') || message.includes("Can't get login item") || message.includes('cannot be read'); |
There was a problem hiding this comment.
What is the meaning of -1728? I think in this context it is a macOS error code. I am right? It would be better to extract this value into a meaningful self describing variable otherwise no one in the future will know what's the meaning of it.
4 participants
Pull Request
Summary
What did I change and why?
Enterprise managed configuration (MDM) for Wire Desktop: admins can enforce settings via Windows Registry (Group Policy or user key) or macOS managed preferences. Managed values override user settings and are cached for the process lifetime. Config is read in the main process only; the renderer receives a redacted config via IPC (no registry/OS access in the webview). Implemented: safe download path with home-dir traversal check on Windows and macOS; locale normalization to supported language keys; registry key sanitization (alphanumeric app name) for Windows paths; proxy credentials redacted before exposing to renderer. Removed
environmentfrom MDM (usewebappUrlplus build type instead). Added unit tests for parsing, validation, redaction, and a test-only override seam. Addeddocs/mdm-managed-configuration.mdfor MDM/Group Policy admins and testing notes.Risks and how to roll out / roll back (e.g. feature flags):
Low risk: MDM is opt-in by deployment (admins deploy registry/preferences). No feature flag. If admins remove policy keys or managed preferences, users restart the app and settings fall back to user/default. Roll back: remove deployed registry keys or managed preferences and restart clients.
Security Checklist (required)
Registry/preference values validated (Joi schema); app name sanitized for Windows registry path (alphanumeric); download path validated and checked for traversal outside user home.
N/A for this PR (config source is OS, not an API). Managed config shape validated via schema; invalid values dropped with log.
No HTML from managed config; no UI change that renders user/OS input as HTML.
Registry key path built from sanitized app name only; download path resolved and checked for traversal; proxy URL not executed as command.
Standards Acknowledgement (required)
Screenshots or demo (if the user interface changed)
No UI changes. Managed settings affect behaviour (e.g. download location, locale, menu bar visibility) but do not add new screens.
Notes for reviewers
ManagedConfig/ IPC) rather than a full “single owner” refactor to limit scope.