Support custom tap folder for apps without casks; add three apps#43784
Support custom tap folder for apps without casks; add three apps#43784allenhouchins merged 9 commits intomainfrom
Conversation
Add per-app api_base_url support for the Homebrew ingester and docs so casks from third‑party taps can be ingested. Implemented normalization and override logic in the ingester, and added unit tests to verify routing to an override host. Add two maintained apps (Druva inSync and Fleet Desktop): input manifests, output app entries, per-app darwin JSON manifests (versions, installer URLs, install/uninstall scripts and refs), frontend SVG icons, and website 2x app icons. Also update the maintained-apps README with usage and an example for ingesting from custom taps.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #43784 +/- ##
==========================================
- Coverage 66.79% 66.78% -0.01%
==========================================
Files 2627 2629 +2
Lines 211230 211250 +20
Branches 9547 9548 +1
==========================================
+ Hits 141081 141087 +6
- Misses 57331 57344 +13
- Partials 12818 12819 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Create a CI-only MDM config stub for Fleet Desktop in Darwin workflows and add a per-app Homebrew API override. - .github/workflows/test-fma-darwin-pr-only.yml: add has_fleet_desktop detection and conditionally write /Library/Managed Preferences/com.fleetdm.fleetd.config.plist when fleet-desktop/darwin is part of the PR so the installer (which expects an MDM-managed prefs profile) can run in CI. - .github/workflows/test-fma-darwin.yml: always write the same MDM config stub before validation since this workflow validates all Darwin apps on each run. The stub contains placeholder EnrollSecret and FleetURL entries and sets permissions so the install/validate steps succeed on CI runners. - ee/maintained-apps/inputs/homebrew/schema/input-schema.json: add "api_base_url" string property (uri, must start with http/https) to allow overriding the Brew API base URL for ingesting casks from third-party taps. These changes ensure fleet-desktop's installer can be exercised in CI and add flexibility for ingesting casks from alternate Homebrew API endpoints.
|
@claude review once |
Allow ingesting Homebrew casks committed into the repo by adding a cask_path option and a custom-tap layout. The Homebrew ingester now resolves cask JSON from (1) cask_path (local file), (2) api_base_url (per-app HTTP), or (3) the default formulae.brew.sh. Added fetchCask() and CaskPath in the ingester, a unit test for cask_path, and schema entry for cask_path. Introduced ee/maintained-apps/inputs/homebrew/custom-tap/ with Casks/*.rb, generated api/*.json, regenerate.sh, and README documenting the workflow. Updated two input manifests (fleet-desktop and druva-insync) to use cask_path and expanded the maintained-apps README to explain both ingestion options. Existing api_base_url behavior remains supported.
Remove support for the per-app api_base_url override in the Homebrew ingester. The ingester now resolves casks either from a local cask_path (inputs/homebrew/custom-tap/) or from the default formulae.brew.sh API. Updates: remove APIBaseURL field and related fetch logic/tests, update schema to remove api_base_url, and adjust README and test text/comments to reflect the simplified flow and the custom-tap path naming.
Add Zoom Rooms (token: zoom-rooms) to the maintained apps catalog. Includes a Homebrew Cask (Casks/zoom-rooms.rb) and corresponding API/input JSON, a darwin output JSON with installer/uninstaller scripts and version 7.0.0.12322 (sha256), and updates apps.json to list the app (bundle id us.zoom.ZoomPresence). Also adds a frontend SVG icon component and PNG asset for the Software page.
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for ingesting Homebrew casks from third-party taps by allowing cask metadata to live in-repo (via a new cask_path field), and onboards three new macOS apps (Fleet Desktop, Druva inSync, Zoom Rooms) into Fleet Maintained Apps with CI updates to validate Fleet Desktop successfully.
Changes:
- Introduces
cask_pathin Homebrew input manifests + schema and updates the ingester to read cask JSON locally when provided. - Adds custom-tap infrastructure (Casks sources, generated API JSON, regeneration script, docs) and registers three new apps + icons.
- Updates Darwin FMA workflows to create a managed-preferences stub needed for Fleet Desktop installer validation in CI.
Reviewed changes
Copilot reviewed 24 out of 26 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/pages/SoftwarePage/components/icons/index.ts | Registers new icons and maps display names to icon components. |
| frontend/pages/SoftwarePage/components/icons/ZoomRooms.tsx | Adds Zoom Rooms icon component. |
| frontend/pages/SoftwarePage/components/icons/FleetDesktop.tsx | Adds Fleet Desktop icon component. |
| ee/maintained-apps/outputs/zoom-rooms/darwin.json | Adds generated FMA output manifest for Zoom Rooms on macOS. |
| ee/maintained-apps/outputs/fleet-desktop/darwin.json | Adds generated FMA output manifest for Fleet Desktop on macOS. |
| ee/maintained-apps/outputs/druva-insync/darwin.json | Adds generated FMA output manifest for Druva inSync on macOS. |
| ee/maintained-apps/outputs/apps.json | Registers the three new apps in the catalog. |
| ee/maintained-apps/inputs/homebrew/zoom-rooms.json | Adds Homebrew input manifest pointing at local cask JSON via cask_path. |
| ee/maintained-apps/inputs/homebrew/schema/input-schema.json | Extends schema to allow cask_path. |
| ee/maintained-apps/inputs/homebrew/fleet-desktop.json | Adds Homebrew input manifest pointing at local cask JSON via cask_path. |
| ee/maintained-apps/inputs/homebrew/druva-insync.json | Adds Homebrew input manifest pointing at local cask JSON via cask_path. |
| ee/maintained-apps/inputs/homebrew/custom-tap/regenerate.sh | Adds deterministic cask JSON regeneration script. |
| ee/maintained-apps/inputs/homebrew/custom-tap/api/zoom-rooms.json | Adds generated cask JSON for Zoom Rooms. |
| ee/maintained-apps/inputs/homebrew/custom-tap/api/fleet-desktop.json | Adds generated cask JSON for Fleet Desktop. |
| ee/maintained-apps/inputs/homebrew/custom-tap/api/druva-insync.json | Adds generated cask JSON for Druva inSync. |
| ee/maintained-apps/inputs/homebrew/custom-tap/README.md | Documents custom-tap workflow for contributors. |
| ee/maintained-apps/inputs/homebrew/custom-tap/Casks/zoom-rooms.rb | Adds Zoom Rooms cask DSL source. |
| ee/maintained-apps/inputs/homebrew/custom-tap/Casks/fleet-desktop.rb | Adds Fleet Desktop cask DSL source. |
| ee/maintained-apps/inputs/homebrew/custom-tap/Casks/druva-insync.rb | Adds Druva inSync cask DSL source. |
| ee/maintained-apps/ingesters/homebrew/ingester_test.go | Adds test for cask_path ingestion behavior. |
| ee/maintained-apps/ingesters/homebrew/ingester.go | Refactors ingestion to use fetchCask supporting local cask_path. |
| ee/maintained-apps/README.md | Documents cask_path and custom tap ingestion flow. |
| .github/workflows/test-fma-darwin.yml | Adds CI step to stub Fleet Desktop managed prefs for validation. |
| .github/workflows/test-fma-darwin-pr-only.yml | Conditionally stubs managed prefs only when Fleet Desktop is being validated. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <svg xmlns="http://www.w3.org/2000/svg" width={32} height={32} {...props}> | ||
| <image | ||
| width={32} | ||
| height={32} | ||
| href="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAIAAAACACAYAAADDPmHLAAAAAXNSR0IArs4c6QAAAERlWElmTU0AKgAAAAgAAYdpAAQAAAABAAAAGgAAAAAAA6ABAAMAAAABAAEAAKACAAQAAAABAAAAgKADAAQAAAABAAAAgAAAAABIjgR3AAAQQElEQVR4Ae1dW2wc1Rn+977r9W1tJ7EdG5vcScLFJcQgkpQSaKtyEwUqLhVtn1qpUvtE+8hrJVRVolUrVSoPhUJbHmhKAy0UmkCAuCQUyMUpOHESJ/H9bu99Zvr9x9nibHa9MzuzM17vOZGzuzPn+v/f+c9/OWfGpSGRTBVLAXfFjlwOXFBAAqDCgSABIAFQ4RSo8OFLCSABUOEUqPDhSwkgAVDhFKjw4UsJIAFQ4RSo8OFLCSABUOEUqPDhSwkgAVDhFKjw4UsJIAFQ4RSo8OFLCSABUOEUqPDhSwkgAVDhFKjw4UsJIAFQ4RSo8OFLCVDhAPA6Pf64EqXJ5CjFlHnyu4NU72+kam9dSbuVUjSaiKVpLqmSx+2i+qBH/JW0URy/SE/PUHpmjvgohrc6TN76OnJ5nJ2DjgHg2FQPvTf6dzoz10vTyXFKaynyuDyC+e1V66m7aS/taPwKQBGwjC994wk6eHaOTo3GaRwASAII4D9V+dzUWuOjW9aGaVdHmGoCHsvaTE1M0dThj2ju+ClKjo6TGk+Iut1+P/kaIxTesp7qb9tBgZbVlrVppCKX3SeDxhKD9NLZX9LRiXdJ1RQw3QsmuMmFfxr/w+xQtDS+qbSh5np6vPNH+NxuZExX5Y2mVPrTsUl6+8wsJdKYfZh0bhdaBPP5WBSfjVLwn6oStdb66LEbItTdFr6qHqMXJt/podH9/6TU5DRmumdhtnOjnNCehga1NGhQXUWNe3dT09fvEPkWMtjzv60AODN3kn7z2dM0FL9AAYj7QimlJinkDdN31z1FtzbdXSh7zvvj0TQ9+8EoncSsD3gZZkuntKoJMD6yvZ4e3Fq/dOY8dzVFoaGX/0aTB94nYsa7C4h5Bl8yRbVfup5av/MIeUKFaZOnacOXC/TMcH15C4wlhgTzR+KXdDGfK/K5/ZRQ4vTc6Z9R7/RHeevOdyOeVunXPaPUC+YHdTCf6/FiTeBlgSXGW5AYxaTR/W/RxNuHyOXzFWY+NwCp4A74aebopzT0x31CMhTTbjFlbAGAqqkQ+8+Kmc9MNZJYL0gqCXq+/+c0n54xUpT+emqaPh1emPlGCrKUZuXwxU8m6MJMykhRmj/VR+P/OEC8xhtN7mBA6AtTHxw1WrTo/LYA4OT0Eaz57+ie+dmj8bp9NBA9TQeGX82+lff38FyK3uibIb+nkNDPXQVLgdmESvt6p3JnyHGV1/TR194mTYEykVnrc+Rb6hJbBQwgJRpbKptl92wBwKHR16HwgSgmktflo8NjbxLrBXpSz4UoTcdVIc715M+Vh8Hz8WCMWI/Qk+LnLlLszDmI/uKNK1YWEyNjNN/7uZ4mTecpOQDiSoz6Yep5oe2bSW4sBcPxASwjAwWrYc3+xEgMFkbBrEtm4Ek8m1Cob2LBdFsyM25G+/qFMlcoX8H7UAp5KbEjlRwAk8kRmoKd74KpZyax/s4K4Uj8YsFqkjD1RubSpmZ/phEYBXRhWp8ekLg0XLToz7THn2w1sBQQNuriGyX4bo4rOjrEHj528picjKIl9g1E03MFW02Ca+zkcRW5Dmc3EIXHUE9S5qPWtIl+C4cRJEGpU8kBwJ48dvZYkVgK6PEfwOIT5pwVbXId7D/Qk9iUs2Tagu9umJCWzJoCHS85ACL+VXDv1kIJNI9mL0zIpmBzgSERbH43NVR5LGmThUgz3MR6kn/NKtjw5sfJ1gS7iYu1JPT0NZOn5AAIg/ns21ewDJhJCtzGjYE11BLqLFgNM21TY4DgBzKVmJUhn4vWN+iLR1St77DIlatR1YZOU33XW7jkAOCOsBuX/fxmUhrmX1dkF4U8Vbqq6W4PC++fmVY5ari5KSgCRXoaDQEAgdY1wr+vJ3/OPJj93rpaqr5+S87bVl+0BQA3N+4RgR29Nnz2IHn281JyV/ND2bfy/l4XCdBtAAFbBMUkLsVu4fu3IGSrTwUQ3r/Gu3abcuWqqTQ17O4mH0LFdiRbAMBx/ieu/bEI7DAzjSSWHBw1fKTjB7Qq2GqkKD16Q4OI7vFMNpo4anjP5lratjpkqGhddxfV7byJ1IQ+h9XiytVkksKb1lHj3XsWXy7pd1sAwCNYX70NUb2fIMDjE2ahnlExWNiEfKD9e7R79T16ilyRpyHkoR92r4JC6BVm4RU38/xgXZWZv6ezmr61HYqYwcQ2fMujD1D11o0LINCp/DLzg20tIhrIMQG7kq3hYB7Uyemj9EL/L2hgvg8i1gdnDcKlV9g7PONV4fKNBFbRw9d8n/asvtcUPS4ioPO7o2N0HIEhjsyyaM+W6qy8s6Rgpe8+iP0Hr6sXAaFiG2Y7fviV12ny0IfQCeCUYvdw9loCcPB+AE41XdsBnPvF+l9sm8WUsx0A3EmO6v1reB/1jL0l3Lvs4WMnDxu+HC1kbf+myO1izV8dXFvMuK4qw3H+d7Eb6AD+BqaScCipANqCqc2AqMO2sO1rgvS1DbW6tf6rGslxgV26EwcPU+z0WUrPzS8EipCPJYWnKkTBjrUU2bVT7AXIUbzklxwBQGZUaTVFg/HzcO9eEB4+dvI0BVtg6nVA2ze/IyfTzuJPlshDiBQOzqZEtI93BzViieCdQLUWbgVb3CZ/5/2AiaFRSk/NYDMQNP3aGgrAbyDs/ezMNv52FAA2jlM2lYcCtimBedqXlx2mgASAwwxwunkJAKc54HD7EgAOM8Dp5iUAnOaAw+1LADjMAKeblwBwmgMOt2/NVh2Tg1BT8ANi4y28wuTxZztpTVaer7iC/QlpBGzYPetDwCfbTZuv3Aq77hgA4pMqTfw3STMDaUpMq6QgAOOGS9Zf46LqVi81bPZTdbN1hzQF3+IzpJ7/iLShXtKmB+GHT4DvEILBGnI1dpK7rYtczYjDVxAYbPcE8my/eDhOwx8nKR2F/x/EFhuGeeLDTct/vK3KjaBM4xYfte8OARQmVyr4f9XPD5J64nXS5rDblhsUf5elDZ9ZUBGUcXvJ1XIdeboeIlekfYXN9dzDsRUAyVmV+vbjwMbZlGDwkhMNQFAhFYKNHtrwjSohFXIPocBVzHLl3y+Sevo9rC8ckSsAJgXLQqCGPDufIHfHjgKVl//tAtSwboBKQqPPX52nmXN4DgDW+SWZz81icrIU4KXis33zFJswtpFE9BwzW/kQzO97FzFg7NgtxHwu5EG+ZJSU958jbfCEqGYl/2cbAAYOxWnmPMfFL4tdnVR1Qw1gydH/RkwoijqLiWzMeLUPM99n8Lg1N4olId3zAmkx/WcDjfRtueS1BQDzQwqNfJIgj0HmZ4jkxr58lhzjvQa2WUHhU46/Jtb1TD2GPhkEsyOknnzTULFyy2wLAEaOJYiVv6u24RihFiwErof1NT1JHfgPESt8vAWo2OTxkXb+CFG8uOcEFNusneVMUEdfN9m+n72gwMY3Jvqza2c+xkZVYTJm38v1W7t0Ut+an6tw5hp0Bi06Sdr42cyVFfdZcgAk51SxhhdU+gqRFvhRkpo+ZRBOHm1myDwAuE/QBbSpi4V6V7b3Sw4AJc5ePpPi/zJ5+QFS6RjqKpTg4dPScQDAnNRZaAYPr0oYezJJoe4tp/slB4CwvKzgw2Wq6bHkBON1ZdTJCjiIVmoqOQB8YTd5A0CAjolbiMjsKg7U6ugyzD5XsJZdioWq1HEfx8zDTTrylWcWHdQ0NzBflYtCDTipa5IXvJvXF0Zd8AwWTJj97Ns33Sg35A2gro6CTZZrhpIDgE2/yCYfmGFOBGjQI2o7fOQN6VtP3O1dMAF1gGUpzsGEcUWuIVe9NWcTlmrKqXulBwBG1nSdX0gBg8cCv6AJsMPu4+ab9R+Zcq3eKAI7IuT7RU3GvkHsuLfsLd6ZZKw1R3LbAgCete13cMwdYyxCEHCouKU7SOE1BmY0Zr+n62ER6hWRPqPkRRDJ3bkTAaGbjZYsq/y2AIAp0rDRR223B8ELnIfTCwLkY9t/1TY/rb3VoD8fbboibYjqfXthKeBwr97E+wQgQTy3PGaNL0Fvuw7kszUczOMbRkxg4GAc9jye4cfP3sm1pIPxDBT2HrbsCFD7rpDYLVQsfbRLxxES/gNp8O0jIJGfqey2ZLGPmS+YH6gutsmyKWc7AJgy0TGFBnsSNHUmRSkA4YplAYDg9b6mzUsttwSprsMaG1yLTpHa+wap5+Db5wifkAiZNQmfCBfzJhD35jvJfW132TDQbEcdAUCm04kZleYHFYqNI/QKjyE/RjgU8VAYW8F0mXuZiox8IrDDvn1272qIGIpdQNVNwtQTu4CsdCAZ6ZdDeR0FgENjls0uooBtSuCiNuXXZUQBCYBlxAwnuiIB4ATVl1GbEgDLiBlOdEUCwAmqL6M2JQCWETOc6IoEgBNUX0ZtSgAsI2Y40RVr/KxF9Jz3h5zA8wI/xt9pFSeG8LLIKvLQNZ4Q3eippRvxlPEgHxeWqaQUcAQAPelJeg7PBzyWnqU4NgnwczszMSG8SxPvF3LTOncVPY6HRN7nb/7/vZJSokIrt9UVzC+E/W3sHD2fGCD+jgfF5mUuNnZjE5FGX8VTwn9atZHqLHrrSIXyOe+wbQMAbwF4JtpHLyUuUkg8Hzhvn664EYWEuM0XoWfC2ygsl4QraGPFD9uUwL8kB+nPiUuGmM8DrALTD6cm6VexfivGK+vIooAtABhWE0L0e3FQI7PWZ/VjyZ+sDL4CAB1Jr+yTuksSoUQ3bQHA/uQwMQj4wfDFJC6Vgj7wMiSITNZSoOQAYK3+vfQENPvimJ8Zrh/l2WQc0wwcEc8Ulp95KVByAIzhZU8DeH2sBxq/mcT2whTeHtKvRM1UI8tmUcAcV7Iqy/VzAkxjTd7c/F+oOY1lYFTny6Nz9UVeu5oCJQcALwFsAlqVrK3Nql6Vbz0lB0AEr30PWrTRkp8x0eDCzlGZLKNAyQGwCu8ObsWrYNjzZyZx7KAG3sBOxApkso4CJQcAHr1IO70RvP7N3PHgNF4qtdVTQ80Ak0zWUaDkAOCu3udfQ3V4RRzrA0UnFP1moMUSZbLoPqzAgrYAoBPv+30i0EaJIqVADFbEnf4m+rKvcQWywNkh2QIAHuKTwTbai8gem4RGUhyg2eSppqdCG5aIHRqpUeZdTAHbAOAH+56u2kT3YjlgphZSCnm5YLB0eevomeqt1MTnxmSynAK2hYMzPWct4C+JIfo99gSwh5B/s3nHYSK28Rfe86yB4QF6MNBMTwbaRUQwU15+WksB2wGQ6f4stoB9gJ1BR1JTdF6N0Tx+B+AwbgXjb8Ss3+VroDX4LlNpKeAYALKHhccxFh0tzK5L/tZPgWUDAP1dljmtpIBtSqCVnZZ1WUcBCQDraFmWNUkAlCXbrOu0BIB1tCzLmiQAypJt1nVaAsA6WpZlTRIAZck26zotAWAdLcuyJgmAsmSbdZ2WALCOlmVZkwRAWbLNuk5LAFhHy7KsSQKgLNlmXaclAKyjZVnWJAFQlmyzrtMSANbRsixrkgAoS7ZZ12kJAOtoWZY1SQCUJdus67QEgHW0LMuaJADKkm3WdVoCwDpalmVNEgBlyTbrOv0/xFAbKo54h7kAAAAASUVORK5CYII=" |
There was a problem hiding this comment.
These icon components inline large base64-encoded PNGs inside SVGs. This can significantly increase the frontend bundle size and slow initial page loads (especially if this icon map is pulled into commonly-visited views). Consider switching to a true vector SVG (paths) or importing the PNG as a static asset (so it can be cached independently and potentially optimized by the build pipeline). Also consider adding a viewBox=\"0 0 32 32\" so resizing via props/CSS behaves predictably.
| <svg xmlns="http://www.w3.org/2000/svg" width={32} height={32} {...props}> | |
| <image | |
| width={32} | |
| height={32} | |
| href="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAIAAAACACAYAAADDPmHLAAAAAXNSR0IArs4c6QAAAERlWElmTU0AKgAAAAgAAYdpAAQAAAABAAAAGgAAAAAAA6ABAAMAAAABAAEAAKACAAQAAAABAAAAgKADAAQAAAABAAAAgAAAAABIjgR3AAAQQElEQVR4Ae1dW2wc1Rn+977r9W1tJ7EdG5vcScLFJcQgkpQSaKtyEwUqLhVtn1qpUvtE+8hrJVRVolUrVSoPhUJbHmhKAy0UmkCAuCQUyMUpOHESJ/H9bu99Zvr9x9nibHa9MzuzM17vOZGzuzPn+v/f+c9/OWfGpSGRTBVLAXfFjlwOXFBAAqDCgSABIAFQ4RSo8OFLCSABUOEUqPDhSwkgAVDhFKjw4UsJIAFQ4RSo8OFLCSABUOEUqPDhSwkgAVDhFKjw4UsJIAFQ4RSo8OFLCSABUOEUqPDhSwkgAVDhFKjw4UsJIAFQ4RSo8OFLCVDhAPA6Pf64EqXJ5CjFlHnyu4NU72+kam9dSbuVUjSaiKVpLqmSx+2i+qBH/JW0URy/SE/PUHpmjvgohrc6TN76OnJ5nJ2DjgHg2FQPvTf6dzoz10vTyXFKaynyuDyC+e1V66m7aS/taPwKQBGwjC994wk6eHaOTo3GaRwASAII4D9V+dzUWuOjW9aGaVdHmGoCHsvaTE1M0dThj2ju+ClKjo6TGk+Iut1+P/kaIxTesp7qb9tBgZbVlrVppCKX3SeDxhKD9NLZX9LRiXdJ1RQw3QsmuMmFfxr/w+xQtDS+qbSh5np6vPNH+NxuZExX5Y2mVPrTsUl6+8wsJdKYfZh0bhdaBPP5WBSfjVLwn6oStdb66LEbItTdFr6qHqMXJt/podH9/6TU5DRmumdhtnOjnNCehga1NGhQXUWNe3dT09fvEPkWMtjzv60AODN3kn7z2dM0FL9AAYj7QimlJinkDdN31z1FtzbdXSh7zvvj0TQ9+8EoncSsD3gZZkuntKoJMD6yvZ4e3Fq/dOY8dzVFoaGX/0aTB94nYsa7C4h5Bl8yRbVfup5av/MIeUKFaZOnacOXC/TMcH15C4wlhgTzR+KXdDGfK/K5/ZRQ4vTc6Z9R7/RHeevOdyOeVunXPaPUC+YHdTCf6/FiTeBlgSXGW5AYxaTR/W/RxNuHyOXzFWY+NwCp4A74aebopzT0x31CMhTTbjFlbAGAqqkQ+8+Kmc9MNZJYL0gqCXq+/+c0n54xUpT+emqaPh1emPlGCrKUZuXwxU8m6MJMykhRmj/VR+P/OEC8xhtN7mBA6AtTHxw1WrTo/LYA4OT0Eaz57+ie+dmj8bp9NBA9TQeGX82+lff38FyK3uibIb+nkNDPXQVLgdmESvt6p3JnyHGV1/TR194mTYEykVnrc+Rb6hJbBQwgJRpbKptl92wBwKHR16HwgSgmktflo8NjbxLrBXpSz4UoTcdVIc715M+Vh8Hz8WCMWI/Qk+LnLlLszDmI/uKNK1YWEyNjNN/7uZ4mTecpOQDiSoz6Yep5oe2bSW4sBcPxASwjAwWrYc3+xEgMFkbBrEtm4Ek8m1Cob2LBdFsyM25G+/qFMlcoX8H7UAp5KbEjlRwAk8kRmoKd74KpZyax/s4K4Uj8YsFqkjD1RubSpmZ/phEYBXRhWp8ekLg0XLToz7THn2w1sBQQNuriGyX4bo4rOjrEHj528picjKIl9g1E03MFW02Ca+zkcRW5Dmc3EIXHUE9S5qPWtIl+C4cRJEGpU8kBwJ48dvZYkVgK6PEfwOIT5pwVbXId7D/Qk9iUs2Tagu9umJCWzJoCHS85ACL+VXDv1kIJNI9mL0zIpmBzgSERbH43NVR5LGmThUgz3MR6kn/NKtjw5sfJ1gS7iYu1JPT0NZOn5AAIg/ns21ewDJhJCtzGjYE11BLqLFgNM21TY4DgBzKVmJUhn4vWN+iLR1St77DIlatR1YZOU33XW7jkAOCOsBuX/fxmUhrmX1dkF4U8Vbqq6W4PC++fmVY5ari5KSgCRXoaDQEAgdY1wr+vJ3/OPJj93rpaqr5+S87bVl+0BQA3N+4RgR29Nnz2IHn281JyV/ND2bfy/l4XCdBtAAFbBMUkLsVu4fu3IGSrTwUQ3r/Gu3abcuWqqTQ17O4mH0LFdiRbAMBx/ieu/bEI7DAzjSSWHBw1fKTjB7Qq2GqkKD16Q4OI7vFMNpo4anjP5lratjpkqGhddxfV7byJ1IQ+h9XiytVkksKb1lHj3XsWXy7pd1sAwCNYX70NUb2fIMDjE2ahnlExWNiEfKD9e7R79T16ilyRpyHkoR92r4JC6BVm4RU38/xgXZWZv6ezmr61HYqYwcQ2fMujD1D11o0LINCp/DLzg20tIhrIMQG7kq3hYB7Uyemj9EL/L2hgvg8i1gdnDcKlV9g7PONV4fKNBFbRw9d8n/asvtcUPS4ioPO7o2N0HIEhjsyyaM+W6qy8s6Rgpe8+iP0Hr6sXAaFiG2Y7fviV12ny0IfQCeCUYvdw9loCcPB+AE41XdsBnPvF+l9sm8WUsx0A3EmO6v1reB/1jL0l3Lvs4WMnDxu+HC1kbf+myO1izV8dXFvMuK4qw3H+d7Eb6AD+BqaScCipANqCqc2AqMO2sO1rgvS1DbW6tf6rGslxgV26EwcPU+z0WUrPzS8EipCPJYWnKkTBjrUU2bVT7AXIUbzklxwBQGZUaTVFg/HzcO9eEB4+dvI0BVtg6nVA2ze/IyfTzuJPlshDiBQOzqZEtI93BzViieCdQLUWbgVb3CZ/5/2AiaFRSk/NYDMQNP3aGgrAbyDs/ezMNv52FAA2jlM2lYcCtimBedqXlx2mgASAwwxwunkJAKc54HD7EgAOM8Dp5iUAnOaAw+1LADjMAKeblwBwmgMOt2/NVh2Tg1BT8ANi4y28wuTxZztpTVaer7iC/QlpBGzYPetDwCfbTZuv3Aq77hgA4pMqTfw3STMDaUpMq6QgAOOGS9Zf46LqVi81bPZTdbN1hzQF3+IzpJ7/iLShXtKmB+GHT4DvEILBGnI1dpK7rYtczYjDVxAYbPcE8my/eDhOwx8nKR2F/x/EFhuGeeLDTct/vK3KjaBM4xYfte8OARQmVyr4f9XPD5J64nXS5rDblhsUf5elDZ9ZUBGUcXvJ1XIdeboeIlekfYXN9dzDsRUAyVmV+vbjwMbZlGDwkhMNQFAhFYKNHtrwjSohFXIPocBVzHLl3y+Sevo9rC8ckSsAJgXLQqCGPDufIHfHjgKVl//tAtSwboBKQqPPX52nmXN4DgDW+SWZz81icrIU4KXis33zFJswtpFE9BwzW/kQzO97FzFg7NgtxHwu5EG+ZJSU958jbfCEqGYl/2cbAAYOxWnmPMfFL4tdnVR1Qw1gydH/RkwoijqLiWzMeLUPM99n8Lg1N4olId3zAmkx/WcDjfRtueS1BQDzQwqNfJIgj0HmZ4jkxr58lhzjvQa2WUHhU46/Jtb1TD2GPhkEsyOknnzTULFyy2wLAEaOJYiVv6u24RihFiwErof1NT1JHfgPESt8vAWo2OTxkXb+CFG8uOcEFNusneVMUEdfN9m+n72gwMY3Jvqza2c+xkZVYTJm38v1W7t0Ut+an6tw5hp0Bi06Sdr42cyVFfdZcgAk51SxhhdU+gqRFvhRkpo+ZRBOHm1myDwAuE/QBbSpi4V6V7b3Sw4AJc5ePpPi/zJ5+QFS6RjqKpTg4dPScQDAnNRZaAYPr0oYezJJoe4tp/slB4CwvKzgw2Wq6bHkBON1ZdTJCjiIVmoqOQB8YTd5A0CAjolbiMjsKg7U6ugyzD5XsJZdioWq1HEfx8zDTTrylWcWHdQ0NzBflYtCDTipa5IXvJvXF0Zd8AwWTJj97Ns33Sg35A2gro6CTZZrhpIDgE2/yCYfmGFOBGjQI2o7fOQN6VtP3O1dMAF1gGUpzsGEcUWuIVe9NWcTlmrKqXulBwBG1nSdX0gBg8cCv6AJsMPu4+ab9R+Zcq3eKAI7IuT7RU3GvkHsuLfsLd6ZZKw1R3LbAgCete13cMwdYyxCEHCouKU7SOE1BmY0Zr+n62ER6hWRPqPkRRDJ3bkTAaGbjZYsq/y2AIAp0rDRR223B8ELnIfTCwLkY9t/1TY/rb3VoD8fbboibYjqfXthKeBwr97E+wQgQTy3PGaNL0Fvuw7kszUczOMbRkxg4GAc9jye4cfP3sm1pIPxDBT2HrbsCFD7rpDYLVQsfbRLxxES/gNp8O0jIJGfqey2ZLGPmS+YH6gutsmyKWc7AJgy0TGFBnsSNHUmRSkA4YplAYDg9b6mzUsttwSprsMaG1yLTpHa+wap5+Db5wifkAiZNQmfCBfzJhD35jvJfW132TDQbEcdAUCm04kZleYHFYqNI/QKjyE/RjgU8VAYW8F0mXuZiox8IrDDvn1272qIGIpdQNVNwtQTu4CsdCAZ6ZdDeR0FgENjls0uooBtSuCiNuXXZUQBCYBlxAwnuiIB4ATVl1GbEgDLiBlOdEUCwAmqL6M2JQCWETOc6IoEgBNUX0ZtSgAsI2Y40RVr/KxF9Jz3h5zA8wI/xt9pFSeG8LLIKvLQNZ4Q3eippRvxlPEgHxeWqaQUcAQAPelJeg7PBzyWnqU4NgnwczszMSG8SxPvF3LTOncVPY6HRN7nb/7/vZJSokIrt9UVzC+E/W3sHD2fGCD+jgfF5mUuNnZjE5FGX8VTwn9atZHqLHrrSIXyOe+wbQMAbwF4JtpHLyUuUkg8Hzhvn664EYWEuM0XoWfC2ygsl4QraGPFD9uUwL8kB+nPiUuGmM8DrALTD6cm6VexfivGK+vIooAtABhWE0L0e3FQI7PWZ/VjyZ+sDL4CAB1Jr+yTuksSoUQ3bQHA/uQwMQj4wfDFJC6Vgj7wMiSITNZSoOQAYK3+vfQENPvimJ8Zrh/l2WQc0wwcEc8Ulp95KVByAIzhZU8DeH2sBxq/mcT2whTeHtKvRM1UI8tmUcAcV7Iqy/VzAkxjTd7c/F+oOY1lYFTny6Nz9UVeu5oCJQcALwFsAlqVrK3Nql6Vbz0lB0AEr30PWrTRkp8x0eDCzlGZLKNAyQGwCu8ObsWrYNjzZyZx7KAG3sBOxApkso4CJQcAHr1IO70RvP7N3PHgNF4qtdVTQ80Ak0zWUaDkAOCu3udfQ3V4RRzrA0UnFP1moMUSZbLoPqzAgrYAoBPv+30i0EaJIqVADFbEnf4m+rKvcQWywNkh2QIAHuKTwTbai8gem4RGUhyg2eSppqdCG5aIHRqpUeZdTAHbAOAH+56u2kT3YjlgphZSCnm5YLB0eevomeqt1MTnxmSynAK2hYMzPWct4C+JIfo99gSwh5B/s3nHYSK28Rfe86yB4QF6MNBMTwbaRUQwU15+WksB2wGQ6f4stoB9gJ1BR1JTdF6N0Tx+B+AwbgXjb8Ss3+VroDX4LlNpKeAYALKHhccxFh0tzK5L/tZPgWUDAP1dljmtpIBtSqCVnZZ1WUcBCQDraFmWNUkAlCXbrOu0BIB1tCzLmiQAypJt1nVaAsA6WpZlTRIAZck26zotAWAdLcuyJgmAsmSbdZ2WALCOlmVZkwRAWbLNuk5LAFhHy7KsSQKgLNlmXaclAKyjZVnWJAFQlmyzrtMSANbRsixrkgAoS7ZZ12kJAOtoWZY1SQCUJdus67QEgHW0LMuaJADKkm3WdVoCwDpalmVNEgBlyTbrOv0/xFAbKo54h7kAAAAASUVORK5CYII=" | |
| <svg | |
| xmlns="http://www.w3.org/2000/svg" | |
| width={32} | |
| height={32} | |
| viewBox="0 0 32 32" | |
| fill="none" | |
| {...props} | |
| > | |
| <rect x={5} y={6} width={22} height={15} rx={2} fill="#0A67D8" /> | |
| <rect x={7} y={8} width={18} height={11} rx={1} fill="#EAF3FF" /> | |
| <path d="M12 25h8" stroke="#4B5563" strokeWidth={2} strokeLinecap="round" /> | |
| <path d="M16 21v4" stroke="#4B5563" strokeWidth={2} strokeLinecap="round" /> | |
| <path d="M9 27h14" stroke="#4B5563" strokeWidth={2} strokeLinecap="round" /> | |
| <circle cx={11} cy={11} r={1.5} fill="#0A67D8" /> | |
| <path | |
| d="M14 14h7M14 11h7M11 17h10" | |
| stroke="#8AB4F8" | |
| strokeWidth={1.5} | |
| strokeLinecap="round" |
| TAP_NAME="fma-custom-tap" | ||
| TAP_DIR="$(brew --repository)/Library/Taps/${TAP_USER}/homebrew-${TAP_NAME}" | ||
|
|
||
| cleanup() { | ||
| rm -rf "$TAP_DIR" | ||
| } | ||
| trap cleanup EXIT | ||
|
|
||
| rm -rf "$TAP_DIR" |
There was a problem hiding this comment.
The script uses a fixed tap directory (.../Library/Taps/fleetdm/homebrew-fma-custom-tap) and unconditionally rm -rf's it. This can accidentally delete a user's real tap if they happen to already have one at that path, and it also makes concurrent runs unsafe. Prefer creating a unique tap directory name (e.g., include PID/timestamp) and only removing that generated directory on exit.
| TAP_NAME="fma-custom-tap" | |
| TAP_DIR="$(brew --repository)/Library/Taps/${TAP_USER}/homebrew-${TAP_NAME}" | |
| cleanup() { | |
| rm -rf "$TAP_DIR" | |
| } | |
| trap cleanup EXIT | |
| rm -rf "$TAP_DIR" | |
| TAP_BASENAME="fma-custom-tap" | |
| TAP_NAME="${TAP_BASENAME}-$$-$(date +%s)" | |
| TAP_DIR="$(brew --repository)/Library/Taps/${TAP_USER}/homebrew-${TAP_NAME}" | |
| cleanup() { | |
| if [ -d "$TAP_DIR" ]; then | |
| rm -rf "$TAP_DIR" | |
| fi | |
| } | |
| trap cleanup EXIT |
WalkthroughAdds three macOS maintained apps (Druva inSync, Fleet Desktop, Zoom Rooms) including Homebrew casks, custom-tap generation script, input/output manifests, installer/uninstaller scripts, and updates apps.json. Extends the Homebrew ingester to support a local Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/test-fma-darwin-pr-only.yml (1)
88-92:⚠️ Potential issue | 🟡 MinorSet
has_fleet_desktop=falsein the no-changes branch for consistency.This early-exit sets
has_darwin_appsandhas_google_chromebut nothas_fleet_desktop. It's not a functional bug (the downstreamif:short-circuits onhas_darwin_apps), but leaving it unset is inconsistent with the sibling early-exit at lines 97–101 and could mask issues if a future step keys solely offhas_fleet_desktop.🛠 Proposed fix
if [ "${{ steps.check-changes.outputs.has_changes }}" != "true" ]; then echo "has_darwin_apps=false" >> $GITHUB_OUTPUT echo "has_google_chrome=false" >> $GITHUB_OUTPUT + echo "has_fleet_desktop=false" >> $GITHUB_OUTPUT exit 0 fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test-fma-darwin-pr-only.yml around lines 88 - 92, The early-exit branch currently writes has_darwin_apps and has_google_chrome to GITHUB_OUTPUT but omits has_fleet_desktop; update that branch to also set has_fleet_desktop=false so all three outputs (has_darwin_apps, has_google_chrome, has_fleet_desktop) are consistently defined when steps.check-changes indicates no changes, matching the sibling early-exit behavior.
🧹 Nitpick comments (8)
frontend/pages/SoftwarePage/components/icons/ZoomRooms.tsx (1)
6-12: Consider adding aviewBoxso the icon scales correctly.The
<svg>only declares fixedwidth={32}/height={32}and noviewBox. If a consumer overrideswidth/heightvia spread props or CSS, the inner<image>(also fixed at 32×32) won't scale with the viewport, producing a clipped/blank icon at non-32px sizes. AddingviewBox="0 0 32 32"makes the SVG resolution-independent and consistent with typical icon-component conventions.♻️ Suggested change
- <svg xmlns="http://www.w3.org/2000/svg" width={32} height={32} {...props}> + <svg + xmlns="http://www.w3.org/2000/svg" + width={32} + height={32} + viewBox="0 0 32 32" + {...props} + >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/pages/SoftwarePage/components/icons/ZoomRooms.tsx` around lines 6 - 12, The SVG in the ZoomRooms icon component lacks a viewBox, so it won't scale when consumers override width/height; update the <svg> element in ZoomRooms.tsx (the one with width={32} height={32} {...props}) to include viewBox="0 0 32 32" so the inner <image> (width/height 32) scales correctly with the SVG viewport when props or CSS change.ee/maintained-apps/ingesters/homebrew/ingester_test.go (2)
181-184: Optional: use an atomic counter forhttpHits.
httpHitsis incremented inside the test server's handler (a separate goroutine). In the success path no request is ever made, so there's no actual race in this test, but if anyone later tweaks the test such that a request can fire, the plainint++would race against the assertion. Anatomic.Int32(and.Load()at the assertion) avoids that footgun.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/maintained-apps/ingesters/homebrew/ingester_test.go` around lines 181 - 184, Replace the plain int counter used in the test server handler with an atomic counter to avoid race conditions: change the `httpHits` variable (used in the `httptest.NewServer` handler) to an `atomic.Int32` (or `int32` with functions from `sync/atomic`), increment it atomically inside the handler, and use an atomic load (e.g., `.Load()` or `atomic.LoadInt32`) when asserting its value after the test run; update imports accordingly.
207-216: Optional: add a malformed-JSON case.
fetchCaskhas three failure modes for the local-file path: missing file (covered), unreadable, andjson.Unmarshalfailure (the"unmarshal local cask JSON for %s"branch). A small case writing[]byte("{")to a temp file and asserting the unmarshal-error message would close the coverage gap without much overhead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/maintained-apps/ingesters/homebrew/ingester_test.go` around lines 207 - 216, Add a test case in ingester_test.go that exercises the json.Unmarshal failure path by creating a temp file containing malformed JSON (e.g. "{"), passing its path as CaskPath to i.ingestOne (same pattern as the missing-file case) and asserting the returned error contains the unmarshal-specific message ("unmarshal local cask JSON for %s" or a substring like "unmarshal local cask JSON"). This should use the same test setup and assertions style (require.ErrorContains) and verify httpHits remains 0; reference the ingestOne call in the existing test and the fetchCask/unmarshal error branch to locate where to add the new case..github/workflows/test-fma-darwin-pr-only.yml (1)
154-172: Optional: deduplicate the MDM stub step across workflows.The same heredoc plist is repeated verbatim in
.github/workflows/test-fma-darwin.yml(lines 75–91). If the stub structure ever needs to change (e.g., adding a key the installer starts requiring), both files must be edited in lockstep. Consider extracting into a shared script under.github/scripts/(or a composite action) and invoking it from both workflows.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test-fma-darwin-pr-only.yml around lines 154 - 172, The duplicated heredoc plist in the workflow step "Create Fleet Desktop MDM config stub (CI-only)" should be extracted into a single reusable script (e.g., create_mdm_stub.sh) and invoked from both workflows instead of duplicating the inline heredoc; update the step to call that script (ensuring it creates the plist, sets permissions, and lists the file) and modify the matching step in the other workflow to call the same shared script so future changes only need to be made in one place.ee/maintained-apps/ingesters/homebrew/ingester.go (1)
200-214:CaskPathis resolved against the process CWD, not "the repo root".The doc comment (Line 270–276) states
cask_pathis "relative to the repo root", butos.ReadFile(input.CaskPath)(Line 206) resolves it against the process working directory. If the ingester binary is ever invoked from somewhere other than the repo root (CI matrix steps, cron jobs, devs running fromee/maintained-apps/, etc.), this will fail with a misleading "no such file or directory" error rather than the helpful"reading local cask JSON file"wrap.Compare with how the input files themselves are loaded on Line 50, where
path.Join(inputsPath, f.Name())is used to anchor the read.♻️ Suggested fix: anchor `CaskPath` to `inputsPath` (or its parent) and update the doc
Pass
inputsPath(or a derived base) intofetchCaskand join it, e.g.:-func (i *brewIngester) fetchCask(ctx context.Context, input inputApp) (brewCask, error) { +func (i *brewIngester) fetchCask(ctx context.Context, inputsPath string, input inputApp) (brewCask, error) { var cask brewCask if input.CaskPath != "" { - body, err := os.ReadFile(input.CaskPath) + caskPath := input.CaskPath + if !filepath.IsAbs(caskPath) { + // Resolve relative to the inputs directory so the tool works + // regardless of the caller's CWD. + caskPath = filepath.Join(inputsPath, caskPath) + } + body, err := os.ReadFile(caskPath) if err != nil { - return cask, ctxerr.WrapWithData(ctx, err, "reading local cask JSON file", map[string]any{"cask_path": input.CaskPath}) + return cask, ctxerr.WrapWithData(ctx, err, "reading local cask JSON file", map[string]any{"cask_path": caskPath}) }And tweak the comment on
CaskPathto describe the actual anchoring (e.g., "relative to the inputs directory passed toIngestApps"). Otherwise update the input JSON files to use absolute or repo-root-anchored paths and document the required CWD explicitly.Also applies to: 270-276
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/maintained-apps/ingesters/homebrew/ingester.go` around lines 200 - 214, fetchCask is reading input.CaskPath with os.ReadFile which resolves against the process CWD instead of the repo/inputs base; change fetchCask to accept the base inputsPath (or derive it from the caller IngestApps) and use path.Join(inputsPath, input.CaskPath) (or join with the inputs directory parent if CaskPath can point into subdirs) before calling os.ReadFile, and update the CaskPath doc/comment to state it is relative to that inputsPath/inputs directory; ensure error wraps still include the joined path when reporting failures.ee/maintained-apps/inputs/homebrew/custom-tap/regenerate.sh (1)
48-66: Avoid silently clobbering a pre-existingfleetdm/fma-custom-tapand consider disabling Homebrew auto-update for determinism.Two small robustness improvements:
Line 57 unconditionally
rm -rf's$TAP_DIR. While the tap name is intentionally throwaway-ish, it lives under the realfleetdmnamespace, so if a maintainer ever tapsfleetdm/fma-custom-tapfor any reason (or a previousregenerate.shrun was interrupted before the EXIT trap fired and they manuallybrew tap'd it to recover), this script will silently destroy that tap including any uncommitted local edits. Detecting the pre-existing tap and bailing keeps the destructive path opt-in.
brew infowill trigger a Homebrew auto-update (slow + non-deterministic for committers running this back-to-back). SettingHOMEBREW_NO_AUTO_UPDATE=1makes regeneration faster and reproducible.♻️ Proposed fix
+export HOMEBREW_NO_AUTO_UPDATE=1 + cleanup() { rm -rf "$TAP_DIR" } trap cleanup EXIT -rm -rf "$TAP_DIR" +if [ -e "$TAP_DIR" ]; then + echo "error: $TAP_DIR already exists; refusing to clobber it." >&2 + echo "If this is a leftover from a previous run, remove it manually:" >&2 + echo " rm -rf \"$TAP_DIR\"" >&2 + trap - EXIT + exit 1 +fi mkdir -p "$TAP_DIR/Casks" cp Casks/*.rb "$TAP_DIR/Casks/"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/maintained-apps/inputs/homebrew/custom-tap/regenerate.sh` around lines 48 - 66, The script currently unconditionally removes TAP_DIR and can trigger Homebrew auto-updates; change it to first check for an existing tap and abort (do not run the destructive rm -rf on TAP_DIR) unless an explicit force env var or flag is provided, and set HOMEBREW_NO_AUTO_UPDATE=1 before any brew calls (used when computing TAP_DIR or running brew info) to prevent auto-updates; update references to TAP_DIR, the cleanup() function and the trap so cleanup only removes the directory if we created it in this run (track creation with a CREATED_TAP variable), and replace the unconditional rm -rf "$TAP_DIR" with a guarded create-or-bail sequence that exits with a clear message if the tap already exists.ee/maintained-apps/outputs/druva-insync/darwin.json (1)
19-19: Optional: tidy up DMG mount point and copied pkg after install.After
hdiutil detach, the empty mktemp directory created at/tmp/dmg_mount_XXXXXX(MOUNT_POINT) is left behind, and the copiedInstall inSync.pkgremains inTMPDIR. Fleet's per-install working dir is normally cleaned up, so this is cosmetic, but a couple ofrmcalls would make this script self-contained and safer if the script is ever invoked outside of Fleet's sandbox.♻️ Suggested cleanup (applied to the install ref body)
hdiutil attach -plist -nobrowse -readonly -mountpoint "$MOUNT_POINT" "$INSTALLER_PATH" sudo cp -R "$MOUNT_POINT"/* "$TMPDIR" hdiutil detach "$MOUNT_POINT" +rmdir "$MOUNT_POINT" 2>/dev/null || : # install pkg files quit_and_track_application 'com.druva.inSyncClient' sudo installer -pkg "$TMPDIR/Install inSync.pkg" -target / relaunch_application 'com.druva.inSyncClient' +sudo rm -rf "$TMPDIR/Install inSync.pkg" 2>/dev/null || :🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/maintained-apps/outputs/druva-insync/darwin.json` at line 19, The script leaves the temporary mount dir (MOUNT_POINT) and the copied installer (TMPDIR/Install inSync.pkg) behind; after hdiutil detach (and after installer completes) remove the now-empty mount directory and the copied pkg to tidy up. Update the script around the hdiutil detach/after installer steps to rm -rf "$MOUNT_POINT" (or rmdir "$MOUNT_POINT" if you prefer safer removal) and rm -f "$TMPDIR/Install inSync.pkg" (optionally rm -rf "$TMPDIR" only if TMPDIR is exclusively created by this script), ensuring these cleanup calls occur whether the installer succeeds or fails (e.g., via trap or immediately after detach and after installer), referencing the MOUNT_POINT and TMPDIR variables and leaving the rest of quit_and_track_application and relaunch_application unchanged.ee/maintained-apps/outputs/zoom-rooms/darwin.json (1)
19-19: Install command should target$INSTALLER_PATHdirectly, not a hardcoded basename.The installer URL is a raw
.pkg, so there is no DMG to mount and nothing is copied intoTMPDIR. The script then runs:sudo installer -pkg "$TMPDIR/ZoomRooms.pkg" -target /This works because
TMPDIR=$(dirname "$(realpath $INSTALLER_PATH)")and Fleet materializes the download with the URL's basename (ZoomRooms.pkg). However, this adds unnecessary indirection. The Druva and similar scripts need the$TMPDIR/<name>.pkgform because they extract the pkg from a DMG; that rationale doesn't apply here.Recommend invoking the installer against the downloaded path directly (matching the standard pattern used by other direct-
.pkgFMAs):♻️ Suggested change (inside the install ref body)
-# variables -APPDIR="/Applications/" -TMPDIR=$(dirname "$(realpath $INSTALLER_PATH)") +# variables +APPDIR="/Applications/" # functions ... # install pkg files quit_and_track_application 'us.zoom.ZoomPresence' -sudo installer -pkg "$TMPDIR/ZoomRooms.pkg" -target / +sudo installer -pkg "$INSTALLER_PATH" -target / relaunch_application 'us.zoom.ZoomPresence'(Apply the equivalent edit to the template that generates this
refs."dc069c1a"body so regeneration doesn't reintroduce it.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/maintained-apps/outputs/zoom-rooms/darwin.json` at line 19, The install command uses a hardcoded "$TMPDIR/ZoomRooms.pkg" instead of the actual downloaded path; update the installer invocation in the script (the line calling sudo installer -pkg ...) to pass the downloaded file path "$INSTALLER_PATH" directly (remove the TMPDIR/basename indirection), and ensure any template that generates the refs."dc069c1a" body is updated so regeneration won't revert this change; relevant symbols: INSTALLER_PATH, TMPDIR, and the installer invocation that currently references "$TMPDIR/ZoomRooms.pkg".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ee/maintained-apps/inputs/homebrew/custom-tap/Casks/fleet-desktop.rb`:
- Around line 5-8: The cask's url and homepage point to a personal repo
(symbols: url, homepage, name, version, sha256) which creates a supply-chain
trust gap; confirm and document whether allenhouchins/fleet-desktop is
officially sanctioned by Fleet and then either (a) change url/homepage to an
organization-controlled upstream (fleetdm) if one exists and update
sha256/version accordingly, or (b) keep the current upstream but add a
documented provenance note in the cask (mentioning the maintainer relationship,
release approval process, and any GPG signing/release verification steps) and
include a link to proof of sanction and the process used to validate each
release before updating sha256/version.
---
Outside diff comments:
In @.github/workflows/test-fma-darwin-pr-only.yml:
- Around line 88-92: The early-exit branch currently writes has_darwin_apps and
has_google_chrome to GITHUB_OUTPUT but omits has_fleet_desktop; update that
branch to also set has_fleet_desktop=false so all three outputs
(has_darwin_apps, has_google_chrome, has_fleet_desktop) are consistently defined
when steps.check-changes indicates no changes, matching the sibling early-exit
behavior.
---
Nitpick comments:
In @.github/workflows/test-fma-darwin-pr-only.yml:
- Around line 154-172: The duplicated heredoc plist in the workflow step "Create
Fleet Desktop MDM config stub (CI-only)" should be extracted into a single
reusable script (e.g., create_mdm_stub.sh) and invoked from both workflows
instead of duplicating the inline heredoc; update the step to call that script
(ensuring it creates the plist, sets permissions, and lists the file) and modify
the matching step in the other workflow to call the same shared script so future
changes only need to be made in one place.
In `@ee/maintained-apps/ingesters/homebrew/ingester_test.go`:
- Around line 181-184: Replace the plain int counter used in the test server
handler with an atomic counter to avoid race conditions: change the `httpHits`
variable (used in the `httptest.NewServer` handler) to an `atomic.Int32` (or
`int32` with functions from `sync/atomic`), increment it atomically inside the
handler, and use an atomic load (e.g., `.Load()` or `atomic.LoadInt32`) when
asserting its value after the test run; update imports accordingly.
- Around line 207-216: Add a test case in ingester_test.go that exercises the
json.Unmarshal failure path by creating a temp file containing malformed JSON
(e.g. "{"), passing its path as CaskPath to i.ingestOne (same pattern as the
missing-file case) and asserting the returned error contains the
unmarshal-specific message ("unmarshal local cask JSON for %s" or a substring
like "unmarshal local cask JSON"). This should use the same test setup and
assertions style (require.ErrorContains) and verify httpHits remains 0;
reference the ingestOne call in the existing test and the fetchCask/unmarshal
error branch to locate where to add the new case.
In `@ee/maintained-apps/ingesters/homebrew/ingester.go`:
- Around line 200-214: fetchCask is reading input.CaskPath with os.ReadFile
which resolves against the process CWD instead of the repo/inputs base; change
fetchCask to accept the base inputsPath (or derive it from the caller
IngestApps) and use path.Join(inputsPath, input.CaskPath) (or join with the
inputs directory parent if CaskPath can point into subdirs) before calling
os.ReadFile, and update the CaskPath doc/comment to state it is relative to that
inputsPath/inputs directory; ensure error wraps still include the joined path
when reporting failures.
In `@ee/maintained-apps/inputs/homebrew/custom-tap/regenerate.sh`:
- Around line 48-66: The script currently unconditionally removes TAP_DIR and
can trigger Homebrew auto-updates; change it to first check for an existing tap
and abort (do not run the destructive rm -rf on TAP_DIR) unless an explicit
force env var or flag is provided, and set HOMEBREW_NO_AUTO_UPDATE=1 before any
brew calls (used when computing TAP_DIR or running brew info) to prevent
auto-updates; update references to TAP_DIR, the cleanup() function and the trap
so cleanup only removes the directory if we created it in this run (track
creation with a CREATED_TAP variable), and replace the unconditional rm -rf
"$TAP_DIR" with a guarded create-or-bail sequence that exits with a clear
message if the tap already exists.
In `@ee/maintained-apps/outputs/druva-insync/darwin.json`:
- Line 19: The script leaves the temporary mount dir (MOUNT_POINT) and the
copied installer (TMPDIR/Install inSync.pkg) behind; after hdiutil detach (and
after installer completes) remove the now-empty mount directory and the copied
pkg to tidy up. Update the script around the hdiutil detach/after installer
steps to rm -rf "$MOUNT_POINT" (or rmdir "$MOUNT_POINT" if you prefer safer
removal) and rm -f "$TMPDIR/Install inSync.pkg" (optionally rm -rf "$TMPDIR"
only if TMPDIR is exclusively created by this script), ensuring these cleanup
calls occur whether the installer succeeds or fails (e.g., via trap or
immediately after detach and after installer), referencing the MOUNT_POINT and
TMPDIR variables and leaving the rest of quit_and_track_application and
relaunch_application unchanged.
In `@ee/maintained-apps/outputs/zoom-rooms/darwin.json`:
- Line 19: The install command uses a hardcoded "$TMPDIR/ZoomRooms.pkg" instead
of the actual downloaded path; update the installer invocation in the script
(the line calling sudo installer -pkg ...) to pass the downloaded file path
"$INSTALLER_PATH" directly (remove the TMPDIR/basename indirection), and ensure
any template that generates the refs."dc069c1a" body is updated so regeneration
won't revert this change; relevant symbols: INSTALLER_PATH, TMPDIR, and the
installer invocation that currently references "$TMPDIR/ZoomRooms.pkg".
In `@frontend/pages/SoftwarePage/components/icons/ZoomRooms.tsx`:
- Around line 6-12: The SVG in the ZoomRooms icon component lacks a viewBox, so
it won't scale when consumers override width/height; update the <svg> element in
ZoomRooms.tsx (the one with width={32} height={32} {...props}) to include
viewBox="0 0 32 32" so the inner <image> (width/height 32) scales correctly with
the SVG viewport when props or CSS change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c40d2954-7efe-4181-bb99-55de2076766f
⛔ Files ignored due to path filters (4)
ee/maintained-apps/README.mdis excluded by!**/*.mdee/maintained-apps/inputs/homebrew/custom-tap/README.mdis excluded by!**/*.mdwebsite/assets/images/app-icon-fleet-desktop-60x60@2x.pngis excluded by!**/*.pngwebsite/assets/images/app-icon-zoom-rooms-60x60@2x.pngis excluded by!**/*.png
📒 Files selected for processing (22)
.github/workflows/test-fma-darwin-pr-only.yml.github/workflows/test-fma-darwin.ymlee/maintained-apps/ingesters/homebrew/ingester.goee/maintained-apps/ingesters/homebrew/ingester_test.goee/maintained-apps/inputs/homebrew/custom-tap/Casks/druva-insync.rbee/maintained-apps/inputs/homebrew/custom-tap/Casks/fleet-desktop.rbee/maintained-apps/inputs/homebrew/custom-tap/Casks/zoom-rooms.rbee/maintained-apps/inputs/homebrew/custom-tap/api/druva-insync.jsonee/maintained-apps/inputs/homebrew/custom-tap/api/fleet-desktop.jsonee/maintained-apps/inputs/homebrew/custom-tap/api/zoom-rooms.jsonee/maintained-apps/inputs/homebrew/custom-tap/regenerate.shee/maintained-apps/inputs/homebrew/druva-insync.jsonee/maintained-apps/inputs/homebrew/fleet-desktop.jsonee/maintained-apps/inputs/homebrew/schema/input-schema.jsonee/maintained-apps/inputs/homebrew/zoom-rooms.jsonee/maintained-apps/outputs/apps.jsonee/maintained-apps/outputs/druva-insync/darwin.jsonee/maintained-apps/outputs/fleet-desktop/darwin.jsonee/maintained-apps/outputs/zoom-rooms/darwin.jsonfrontend/pages/SoftwarePage/components/icons/FleetDesktop.tsxfrontend/pages/SoftwarePage/components/icons/ZoomRooms.tsxfrontend/pages/SoftwarePage/components/icons/index.ts
| url "https://github.com/allenhouchins/fleet-desktop/releases/download/v#{version}/fleet_desktop-v#{version}.pkg" | ||
| name "Fleet Desktop" | ||
| desc "End-user client for Fleet device management" | ||
| homepage "https://github.com/allenhouchins/fleet-desktop" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether github.com/fleetdm publishes an official Fleet Desktop release
# and what the canonical asset URL pattern looks like, vs. allenhouchins'.
set -e
echo "== fleetdm orgs/repos that look related to fleet-desktop =="
gh search repos --owner fleetdm fleet-desktop --json name,description,url 2>/dev/null || true
echo
echo "== latest fleetdm/fleet releases (for fleet-desktop assets, if bundled) =="
gh release list --repo fleetdm/fleet --limit 5 2>/dev/null || true
echo
echo "== assets on the v1.1.0 release referenced by the cask =="
gh api repos/allenhouchins/fleet-desktop/releases/tags/v1.1.0 \
--jq '{tag: .tag_name, author: .author.login, assets: [.assets[] | {name: .name, size: .size, downloads: .download_count, url: .browser_download_url}]}' 2>/dev/null || true
echo
echo "== existing references in this repo to fleet-desktop pkg URLs =="
rg -nP -C1 'fleet[_-]desktop.*\.pkg' --iglob '!**/node_modules/**'Repository: fleetdm/fleet
Length of output: 11009
Installer URL and homepage point to a personal GitHub account, not a Fleet-controlled organization.
Both url (Line 5) and homepage (Line 8) point to github.com/allenhouchins/fleet-desktop, a personal repository. For an officially Fleet-Maintained App named "Fleet Desktop," this creates a supply-chain concern: end users trust this cask as Fleet-published, yet the binary and releases are hosted under an individual account that Fleet does not directly control or manage keys for.
The fleetdm organization has no separate fleet-desktop repository, confirming that allenhouchins/fleet-desktop is the canonical source. However, the core issue remains: the trust anchor (account authority to publish releases) is not organization-controlled. The pinned sha256 (Line 3) protects against tampering of that specific build, but does not address whether future versions (v1.1.x, etc.) can be safely published under a personal account.
Please confirm:
- Whether
allenhouchins/fleet-desktopis officially maintained by or sanctioned by Fleet, and document this relationship. - If so, consider whether mitigations are in place (e.g., organization-level commit signing, code review/approval before release).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ee/maintained-apps/inputs/homebrew/custom-tap/Casks/fleet-desktop.rb` around
lines 5 - 8, The cask's url and homepage point to a personal repo (symbols: url,
homepage, name, version, sha256) which creates a supply-chain trust gap; confirm
and document whether allenhouchins/fleet-desktop is officially sanctioned by
Fleet and then either (a) change url/homepage to an organization-controlled
upstream (fleetdm) if one exists and update sha256/version accordingly, or (b)
keep the current upstream but add a documented provenance note in the cask
(mentioning the maintainer relationship, release approval process, and any GPG
signing/release verification steps) and include a link to proof of sanction and
the process used to validate each release before updating sha256/version.
cdcme
left a comment
There was a problem hiding this comment.
Left a comment, but overall looks good.
When loading a local cask JSON, add sanity checks to ensure the cask's Token matches the configured input token and that the cask has a non-empty Name. This prevents silently ingesting the wrong app due to misconfigured cask_path and surfaces errors that include the cask_path for easier debugging. Tests were added to cover token-mismatch and empty-name cases.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ee/maintained-apps/ingesters/homebrew/ingester.go (1)
205-221: Constraincask_pathto deterministic relative paths.Line 206 reads
input.CaskPathdirectly. Since Line 278+ documents this as repo-relative, consider rejecting absolute/parent-traversal paths to avoid environment-specific behavior and accidental host-file reads.Suggested hardening
import ( "context" "encoding/json" "fmt" "io" "log/slog" "net/http" "net/url" "os" "path" + "path/filepath" "strings" "time" @@ if input.CaskPath != "" { - body, err := os.ReadFile(input.CaskPath) + if filepath.IsAbs(input.CaskPath) { + return cask, ctxerr.Errorf(ctx, "cask_path must be relative, got absolute path: %s", input.CaskPath) + } + cleanPath := filepath.Clean(input.CaskPath) + if cleanPath == ".." || strings.HasPrefix(cleanPath, ".."+string(os.PathSeparator)) { + return cask, ctxerr.Errorf(ctx, "cask_path must not traverse parent directories: %s", input.CaskPath) + } + body, err := os.ReadFile(cleanPath) if err != nil { return cask, ctxerr.WrapWithData(ctx, err, "reading local cask JSON file", map[string]any{"cask_path": input.CaskPath}) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ee/maintained-apps/ingesters/homebrew/ingester.go` around lines 205 - 221, The code reads input.CaskPath directly which allows absolute paths or parent-traversal; validate and reject non-deterministic paths before reading: in the block using input.CaskPath, call filepath.Clean(input.CaskPath) and then reject if filepath.IsAbs(input.CaskPath) || strings.HasPrefix(clean, "..") || strings.Contains(clean, string(os.PathSeparator)+".."+string(os.PathSeparator)) || strings.HasPrefix(input.CaskPath, "~"); return a ctxerr.Errorf with a clear message when rejected, otherwise proceed to os.ReadFile and json.Unmarshal into cask as before (refer to input.CaskPath, cask and the existing error returns).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ee/maintained-apps/ingesters/homebrew/ingester.go`:
- Around line 205-221: The code reads input.CaskPath directly which allows
absolute paths or parent-traversal; validate and reject non-deterministic paths
before reading: in the block using input.CaskPath, call
filepath.Clean(input.CaskPath) and then reject if filepath.IsAbs(input.CaskPath)
|| strings.HasPrefix(clean, "..") || strings.Contains(clean,
string(os.PathSeparator)+".."+string(os.PathSeparator)) ||
strings.HasPrefix(input.CaskPath, "~"); return a ctxerr.Errorf with a clear
message when rejected, otherwise proceed to os.ReadFile and json.Unmarshal into
cask as before (refer to input.CaskPath, cask and the existing error returns).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0ccb1c77-367e-4885-b79b-4dbfc433475a
📒 Files selected for processing (3)
ee/maintained-apps/ingesters/homebrew/ingester.goee/maintained-apps/ingesters/homebrew/ingester_test.goee/maintained-apps/outputs/apps.json
✅ Files skipped from review due to trivial changes (1)
- ee/maintained-apps/ingesters/homebrew/ingester_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- ee/maintained-apps/outputs/apps.json
|
@lukeheath @georgekarrv need CODEOWNERs review since this updates a workflow related to FMAs. |
This pull request introduces support for ingesting Homebrew casks from third-party taps (not available in the official
Homebrew/homebrew-cask) into the Fleet Maintained Apps (FMA) system. It does this by allowing cask metadata to be committed directly into the repository and referenced via a newcask_pathfield. The PR also updates CI workflows to better support Fleet Desktop validation and documents the new contributor flow.Support for custom Homebrew casks:
cask_pathfield to app manifests, allowing the FMA ingester to read cask metadata from a local JSON file instead of fetching from the Homebrew API. This enables ingestion of apps from third-party taps or custom casks not present in the official Homebrew repository. [1] [2]brewIngester) to use a newfetchCaskhelper, which reads from the local file ifcask_pathis set, or falls back to the API otherwise. Includes robust error handling. [1] [2]custom-tap/directory with cask DSL sources, generated JSON, and a regeneration script. [1] [2]fleet-desktop,druva-insync, andzoom-roomsunderinputs/homebrew/custom-tap/Casks/. [1] [2] [3]Testing and validation:
TestIngestCaskPath) to ensure the ingester correctly reads fromcask_pathand does not make unnecessary HTTP requests, with error handling for missing files.CI workflow improvements:
These changes make it possible to maintain and test apps from custom Homebrew taps within the Fleet repo, improving flexibility and reliability for Fleet-maintained apps.
Summary by CodeRabbit