diff --git a/orchestrator/orchestrator/cli.py b/orchestrator/orchestrator/cli.py index 8c4ca18..b7e3f15 100644 --- a/orchestrator/orchestrator/cli.py +++ b/orchestrator/orchestrator/cli.py @@ -255,6 +255,43 @@ def add_to_group( ) +@_app.command() +def pkg_audit( + include_store: bool = typer.Option( + False, "--include-store", help="Also consider apple-store apps (noisy; they reach devices " + "by other means)." + ), +) -> None: + """Which uploaded pkgs is no assignment group carrying? (read-only) + + Uploading a pkg and attaching it are separate steps in SimpleMDM, and an unattached app is + inert with nothing surfacing that fact. Run this after any upload. Also flags the same bundle + id uploaded twice, where which copy a group carries decides what devices get. + """ + workflow.step_pkg_audit(include_store=include_store) + + +@_app.command() +def pkg_attach( + app: str = typer.Argument(..., help="App id, or a unique substring of its name/bundle id."), + group_id: int = typer.Option( + 0, "--group-id", help="Group to attach to (default: settings.bootstrap_group_id)." + ), + push: bool = typer.Option( + False, + "--push", + help="Force delivery now. Re-pushes EVERY app in the group to EVERY member, including " + "postinstalls — never do this to a group carrying the bootstrap pkg while hosts are busy.", + ), +) -> None: + """Attach an uploaded pkg to a group and verify the group really carries it. + + Verifies by re-reading the group, not by trusting the POST. Production groups are refused: + attaching a new app there pushes it to every member mid-task. + """ + workflow.step_pkg_attach(app, group_id=group_id or None, push=push) + + @_app.command() def group_parity( group_id: int = typer.Option( diff --git a/orchestrator/orchestrator/clients/simplemdm.py b/orchestrator/orchestrator/clients/simplemdm.py index ead5a1e..fcfd872 100644 --- a/orchestrator/orchestrator/clients/simplemdm.py +++ b/orchestrator/orchestrator/clients/simplemdm.py @@ -190,6 +190,42 @@ def push_apps(group_id: int) -> None: _request("POST", f"/assignment_groups/{group_id}/push_apps") +def apps() -> list[dict]: + """Every app (pkg) in the account. One paginated sweep.""" + return _paginated("/apps") + + +def assignment_group_app_ids(group_id: int) -> list[int]: + """App IDs attached to the group. + + NB: relationship IDs come back as ints here, as with devices. + """ + rel = get_assignment_group(group_id).get("relationships", {}) + return [int(a["id"]) for a in rel.get("apps", {}).get("data", [])] + + +def add_app_to_assignment_group(group_id: int, app_id: int) -> None: + """Attach an app to an assignment group. This is what makes an upload do anything. + + Uploading a pkg and attaching it are separate operations in SimpleMDM, and an unattached app + is completely inert with nothing surfacing that fact — p_role_tart_worker sat uploaded and + unattached on 2026-08-19 while the hosts it was built for went on with no role file. + + Production groups are refused, using the same guard as the device writes. Attaching a NEW app + to a live production group pushes it to every member: 2017918 had 130+ devices taking work. + That is the "never add the bootstrap pkg to a production group" footgun in its other form, and + a change with that blast radius should be made in the UI with a human looking at it. + """ + if group_id in PROTECTED_GROUP_IDS: + raise ReprovisionError( + f"refusing to attach app {app_id} to assignment group {group_id} " + f"({PROTECTED_GROUP_IDS[group_id]}). Attaching an app there pushes it to every member, " + "mid-task. Attach to the bootstrap/staging group instead, or do it in the UI " + "deliberately." + ) + _request("POST", f"/assignment_groups/{group_id}/apps/{app_id}") + + def wipe(device_id: int, *, obliteration_behavior: str = "DoNotObliterate") -> None: """ Erase the device. Default `DoNotObliterate` = EACS-only: if Erase All Content & Settings diff --git a/orchestrator/orchestrator/workflow.py b/orchestrator/orchestrator/workflow.py index d72cdf8..68117a7 100644 --- a/orchestrator/orchestrator/workflow.py +++ b/orchestrator/orchestrator/workflow.py @@ -584,6 +584,167 @@ def step_group_parity( ) +def _resolve_app(spec: str) -> dict: + """An app by id, or by a unique substring of its name or bundle identifier.""" + catalog = simplemdm.apps() + if spec.isdigit(): + for a in catalog: + if int(a["id"]) == int(spec): + return a + raise ReprovisionError(f"no app with id {spec} in this SimpleMDM account") + + needle = spec.lower() + hits = [ + a for a in catalog + if needle in (a.get("attributes", {}).get("name") or "").lower() + or needle in (a.get("attributes", {}).get("bundle_identifier") or "").lower() + ] + if not hits: + raise ReprovisionError(f"no app matching {spec!r} — check the name or pass the numeric id") + if len(hits) > 1: + listed = "\n ".join( + f"{a['id']} {a['attributes'].get('name')!r} {a['attributes'].get('bundle_identifier')}" + for a in hits[:8] + ) + raise ReprovisionError( + f"{spec!r} matches {len(hits)} apps — narrow it or pass the id:\n {listed}" + ) + return hits[0] + + +def step_pkg_audit(*, include_store: bool = False) -> None: + """Which uploaded pkgs are attached to no assignment group? Read-only, API-only. + + Uploading a pkg and attaching it to a group are separate operations in SimpleMDM, and an + unattached app is completely inert with nothing surfacing that fact. On 2026-08-19 + `p_role_tart_worker` was uploaded and left unattached while the eight hosts it was built for + carried no role file — invisible until someone thought to check the group's app list by hand. + + Only `app_type: custom` apps are considered by default — the pkgs we build and upload. The + account also holds ~20 `apple store` apps (1Password, Duo, Google Drive, ...) for iOS devices, + which legitimately reach devices by other means; including them buried the three real findings + in noise on the first run. `include_store` restores the unfiltered view. + + Also reports duplicate bundle identifiers, attached or not: two uploads of the same pkg is its + own hazard, because which one a group carries decides what devices get. The r8 role pkg is + currently uploaded twice (`-Signed` and `-wrapped`, both + `com.github.munki.pkg.p_role_gecko_t_osx_1400_r8`). + + CAVEAT: this only looks at assignment groups. An app reaching devices solely through a legacy + *device* group would be reported here as an orphan. That is the right default for this fleet — + verified 2026-08-19 that the m4 and tart devices all have `device_group_id: None`, so + assignment groups are the only live delivery path — but check before acting on a surprise. + """ + ui.step("PKG AUDIT", "uploaded pkgs that no assignment group carries (read-only)") + + carried: dict[int, list[str]] = {} + for group in simplemdm.assignment_groups(): + gname = group.get("attributes", {}).get("name", "?") + for app in group.get("relationships", {}).get("apps", {}).get("data", []): + carried.setdefault(int(app["id"]), []).append(f"{group['id']} ({gname})") + + everything = simplemdm.apps() + catalog = everything if include_store else [ + a for a in everything if a.get("attributes", {}).get("app_type") == "custom" + ] + scope = "app(s)" if include_store else "custom pkg(s)" + ui.info( + f"{len(catalog)} {scope} in the account " + f"({len(everything) - len(catalog)} apple-store app(s) excluded), " + f"{len(carried)} app(s) attached to at least one group" + ) + + # Duplicate bundle ids: two uploads of the same pkg, where which one a group carries decides + # what devices actually get. + by_bundle: dict[str, list[dict]] = {} + for a in catalog: + bundle = a.get("attributes", {}).get("bundle_identifier") or "" + if bundle: + by_bundle.setdefault(bundle, []).append(a) + # Only duplicates where at least one copy is attached to nothing. Sharing a bundle id across + # deliberate per-flavour variants is normal here and all-attached sets are not findings: + # com.mozilla.pkg.SignerBootstrap has six copies, one per signer group (vpn/tb/fx/dep/adhoc/ + # ff-ent), and puppet-agent's ARM and Intel builds share com.puppetlabs.puppet-agent. Flagging + # those buried the one real case. A duplicate that includes a stray upload is the smell. + dupes = { + b: v for b, v in by_bundle.items() + if len(v) > 1 and any(int(a["id"]) not in carried for a in v) + } + if dupes: + ui.warn(f"{len(dupes)} bundle id(s) uploaded more than once with a copy attached to nothing:") + for bundle, group in sorted(dupes.items()): + ui.warn(f" {bundle}") + for a in sorted(group, key=lambda a: int(a["id"])): + where = carried.get(int(a["id"])) + ui.warn(f" {a['id']} {a['attributes'].get('name')!r} " + f"{'carried by ' + ', '.join(where) if where else 'ATTACHED TO NOTHING'}") + + orphans = [a for a in catalog if int(a["id"]) not in carried] + if not orphans: + ui.ok(f"every {scope.rstrip('(s)')} is attached to at least one group") + return + + ui.warn(f"{len(orphans)} {scope} attached to NOTHING — uploaded but inert:") + for a in sorted(orphans, key=lambda a: int(a["id"])): + at = a.get("attributes", {}) + ui.warn(f" {a['id']} {at.get('name')!r} bundle={at.get('bundle_identifier')}") + ui.info("attach one with: reprovision pkg-attach --group-id ") + + +def step_pkg_attach(app_spec: str, *, group_id: int | None = None, push: bool = False) -> None: + """Attach an uploaded pkg to an assignment group, then VERIFY the group really carries it. + + Verifies by re-reading the group rather than trusting the POST, for the same reason + step_add_to_group does: with this API a write returning 2xx is not proof the state changed. + + `push` is OFF by default and that is deliberate. `push_apps` re-pushes EVERY app in the group + to EVERY member, so pushing the m4 bootstrap group would re-run the bootstrap pkg's postinstall + on hosts that are mid-task. Without a push the pkg still lands, just on each device's next + check-in — which on these boxes is often boot-only, so it can take a reboot. Choose knowingly: + the count of affected devices is printed either way. + """ + s = get_settings() + gid = group_id or s.bootstrap_group_id + app = _resolve_app(app_spec) + aid = int(app["id"]) + at = app.get("attributes", {}) + + group = simplemdm.get_assignment_group(gid) + gname = group.get("attributes", {}).get("name", "?") + members = len(simplemdm.assignment_group_device_ids(gid)) + + ui.step("PKG ATTACH", "make an uploaded pkg actually reach hosts") + ui.info(f"app {aid} = {at.get('name')!r} bundle={at.get('bundle_identifier')}") + ui.info(f"group {gid} = {gname} ({members} device(s))") + + if aid in simplemdm.assignment_group_app_ids(gid): + ui.ok(f"already attached to {gname} — no change") + else: + ui.wire(f"POST /assignment_groups/{gid}/apps/{aid}") + simplemdm.add_app_to_assignment_group(gid, aid) + if push: + ui.warn( + f"pushing: re-pushes every app in {gname} to all {members} device(s), including " + "any postinstall they run" + ) + ui.wire(f"POST /assignment_groups/{gid}/push_apps") + simplemdm.push_apps(gid) + else: + ui.info( + f"not pushing — the pkg lands on each of the {members} device(s) at its next " + "check-in (often boot-only). Pass --push to force it now." + ) + + # Re-read: the POST returning 2xx is not evidence the group changed. + after = simplemdm.assignment_group_app_ids(gid) + if aid not in after: + raise ReprovisionError( + f"attach reported success but group {gid} ({gname}) still does not list app {aid}. " + "Check the group in SimpleMDM before assuming the pkg will be delivered." + ) + ui.ok(f"verified: {gname} carries app {aid} (group now has {len(after)} app(s))") + + def step_validate(ctx: HostContext, *, expected_refresh_hz: float | None = None) -> None: """Read-only fitness check on a bootstrapped host: is it actually able to run tasks? diff --git a/orchestrator/tests/test_pkg_attach.py b/orchestrator/tests/test_pkg_attach.py new file mode 100644 index 0000000..a22be0d --- /dev/null +++ b/orchestrator/tests/test_pkg_attach.py @@ -0,0 +1,208 @@ +"""`reprovision pkg-audit` / `pkg-attach` — making an uploaded pkg actually reach hosts. + +Uploading a pkg and attaching it to an assignment group are separate operations in SimpleMDM, and +an unattached app is completely inert with nothing surfacing that fact. On 2026-08-19 +`p_role_tart_worker` was signed, uploaded, and left unattached while the eight hosts it was built +for carried no role file — caught only because someone queried the group's app list by hand. + +The interesting cases here are the guards, not the happy path: a production group must be refused +(attaching an app there pushes it to every member mid-task), the push must stay opt-in, and success +must be proven by re-reading the group rather than by trusting the POST. +""" + +from __future__ import annotations + +from unittest.mock import patch + +import pytest + +from orchestrator import workflow +from orchestrator.clients import simplemdm +from orchestrator.errors import ReprovisionError + +BOOTSTRAP_GID = 2417981 +TART_GID = 2105807 +PROD_GID = 2017918 +APP = 690299 + + +def _app(app_id=APP, name="p_role_tart_worker-1.0-signed", + bundle="com.github.munki.pkg.p_role_tart_worker", app_type="custom"): + return {"id": app_id, + "attributes": {"name": name, "bundle_identifier": bundle, "app_type": app_type}} + + +def _group(gid, name, app_ids=(), device_ids=()): + return { + "id": gid, + "attributes": {"name": name}, + "relationships": { + "apps": {"data": [{"id": a} for a in app_ids]}, + "devices": {"data": [{"id": d} for d in device_ids]}, + }, + } + + +# --- the production-group guard --- + + +def test_refuses_to_attach_an_app_to_a_production_group(): + """A new app on a live prod group is pushed to every member, mid-task. 2017918 had 130+.""" + with patch("orchestrator.clients.simplemdm._request") as req, \ + pytest.raises(ReprovisionError, match="refusing to attach"): + simplemdm.add_app_to_assignment_group(PROD_GID, APP) + req.assert_not_called() # the guard fires BEFORE any HTTP call + + +# --- attach --- + + +def _attach(*, group_apps, catalog=None, push=False, gid=TART_GID, spec=str(APP)): + group = _group(gid, "Tart", app_ids=group_apps, device_ids=(1, 2, 3)) + calls = {"add": 0, "push": 0} + + def _add(g, a): + calls["add"] += 1 + group["relationships"]["apps"]["data"].append({"id": a}) + + with patch("orchestrator.workflow.simplemdm.apps", return_value=catalog or [_app()]), \ + patch("orchestrator.workflow.simplemdm.get_assignment_group", return_value=group), \ + patch("orchestrator.workflow.simplemdm.assignment_group_device_ids", return_value=[1, 2, 3]), \ + patch("orchestrator.workflow.simplemdm.assignment_group_app_ids", + side_effect=lambda g: [int(a["id"]) for a in group["relationships"]["apps"]["data"]]), \ + patch("orchestrator.workflow.simplemdm.add_app_to_assignment_group", side_effect=_add), \ + patch("orchestrator.workflow.simplemdm.push_apps", side_effect=lambda g: calls.__setitem__("push", calls["push"] + 1)): + workflow.step_pkg_attach(spec, group_id=gid, push=push) + return calls + + +def test_attaches_when_the_group_does_not_carry_it(): + calls = _attach(group_apps=(600264,)) + assert calls["add"] == 1 + + +def test_already_attached_is_a_no_op(): + calls = _attach(group_apps=(600264, APP)) + assert calls == {"add": 0, "push": 0} + + +def test_push_is_opt_in(): + """push_apps re-pushes EVERY app to EVERY member, including postinstalls. + + Defaulting it on would mean an innocuous-looking attach re-runs the bootstrap pkg on every + member of the bootstrap group, mid-task. + """ + assert _attach(group_apps=(600264,), push=False)["push"] == 0 + assert _attach(group_apps=(600264,), push=True)["push"] == 1 + + +def test_verification_rereads_the_group_and_fails_when_the_attach_did_not_stick(): + """A 2xx from the POST is not evidence the group changed.""" + group = _group(TART_GID, "Tart", app_ids=(600264,), device_ids=(1,)) + with patch("orchestrator.workflow.simplemdm.apps", return_value=[_app()]), \ + patch("orchestrator.workflow.simplemdm.get_assignment_group", return_value=group), \ + patch("orchestrator.workflow.simplemdm.assignment_group_device_ids", return_value=[1]), \ + patch("orchestrator.workflow.simplemdm.assignment_group_app_ids", return_value=[600264]), \ + patch("orchestrator.workflow.simplemdm.add_app_to_assignment_group"), \ + patch("orchestrator.workflow.simplemdm.push_apps"), \ + pytest.raises(ReprovisionError, match="still does not list app"): + workflow.step_pkg_attach(str(APP), group_id=TART_GID) + + +# --- app resolution --- + + +def test_resolves_an_app_by_name_substring(): + calls = _attach(group_apps=(600264,), spec="tart_worker") + assert calls["add"] == 1 + + +def test_resolves_an_app_by_bundle_identifier(): + calls = _attach(group_apps=(600264,), spec="munki.pkg.p_role_tart_worker") + assert calls["add"] == 1 + + +def test_an_ambiguous_name_lists_the_candidates_instead_of_guessing(): + catalog = [_app(1, "p_role_a", "com.x.p_role_a"), _app(2, "p_role_b", "com.x.p_role_b")] + with patch("orchestrator.workflow.simplemdm.apps", return_value=catalog), \ + pytest.raises(ReprovisionError, match="matches 2 apps"): + workflow.step_pkg_attach("p_role", group_id=TART_GID) + + +def test_an_unknown_app_is_refused(): + with patch("orchestrator.workflow.simplemdm.apps", return_value=[_app()]), \ + pytest.raises(ReprovisionError, match="no app matching"): + workflow.step_pkg_attach("nonexistent", group_id=TART_GID) + + +def test_an_unknown_numeric_id_is_refused_by_id_not_treated_as_a_substring(): + with patch("orchestrator.workflow.simplemdm.apps", return_value=[_app()]), \ + pytest.raises(ReprovisionError, match="no app with id 999"): + workflow.step_pkg_attach("999", group_id=TART_GID) + + +# --- the orphan audit --- + + +def _audit(catalog, groups, include_store=False): + warned: list[str] = [] + with patch("orchestrator.workflow.simplemdm.apps", return_value=catalog), \ + patch("orchestrator.workflow.simplemdm.assignment_groups", return_value=groups), \ + patch("orchestrator.workflow.ui.warn", side_effect=warned.append), \ + patch("orchestrator.workflow.ui.ok"), patch("orchestrator.workflow.ui.info"), \ + patch("orchestrator.workflow.ui.step"): + workflow.step_pkg_audit(include_store=include_store) + return "\n".join(warned) + + +def test_audit_names_an_app_no_group_carries(): + """The exact 2026-08-19 failure: uploaded, attached to nothing, silently inert.""" + out = _audit([_app(), _app(600264, "Sudoers", "com.mozilla.pkg.Sudoers")], + [_group(TART_GID, "Tart", app_ids=(600264,))]) + assert "p_role_tart_worker" in out + assert "Sudoers" not in out # attached, so not an orphan + + +def test_audit_is_quiet_when_everything_is_attached(): + out = _audit([_app()], [_group(TART_GID, "Tart", app_ids=(APP,))]) + assert out == "" + + +def test_audit_counts_an_app_attached_to_any_group_as_carried(): + out = _audit([_app()], [_group(BOOTSTRAP_GID, "bootstrap", app_ids=()), + _group(TART_GID, "Tart", app_ids=(APP,))]) + assert out == "" + + +def test_audit_excludes_apple_store_apps_by_default(): + """The account holds ~39 store apps for iOS devices; they reach devices by other means. + + Including them buried the five real findings on the first live run. + """ + store = _app(660813, "Duo Mobile", "com.duosecurity.DuoMobile", app_type="apple store") + out = _audit([store], []) + assert "Duo Mobile" not in out + assert "Duo Mobile" in _audit([store], [], include_store=True) + + +def test_audit_does_not_flag_duplicates_that_are_all_attached(): + """Sharing a bundle id across deliberate per-flavour variants is normal here. + + com.mozilla.pkg.SignerBootstrap has six copies, one per signer group. Flagging fully-attached + duplicate sets buried the one real case (the r8 role pkg, uploaded twice and attached to + nothing). + """ + variants = [_app(624293, "Signer Bootstrap - VPN", "com.mozilla.pkg.SignerBootstrap"), + _app(624294, "Signer Bootstrap - TB", "com.mozilla.pkg.SignerBootstrap")] + groups = [_group(1903167, "Signers - vpn", app_ids=(624293,)), + _group(1903166, "Signers - tb", app_ids=(624294,))] + assert _audit(variants, groups) == "" + + +def test_audit_flags_a_duplicate_when_a_copy_is_attached_to_nothing(): + """The real r8 case: two uploads of the same bundle, neither carried by any group.""" + dupes = [_app(630818, "r8-1.0-Signed", "com.github.munki.pkg.p_role_gecko_t_osx_1400_r8"), + _app(630822, "r8-1.0-wrapped", "com.github.munki.pkg.p_role_gecko_t_osx_1400_r8")] + out = _audit(dupes, []) + assert "uploaded more than once" in out + assert "630818" in out and "630822" in out