From 79bda6fab9d527f93f1c296d9b60cf7557f64640 Mon Sep 17 00:00:00 2001 From: Howard Enos Date: Thu, 25 Jun 2026 15:14:18 -0700 Subject: [PATCH] datto-edr: apply code-review fixes (gating + footgun hardening) - deploy-cmd: require explicit --regkey or --group; never auto-pick an arbitrary cross-client registration key (would enroll into wrong org). - raw: block POST to any */scan endpoint with no non-empty `where` (same tenant-wide footgun the scan command guards against). - main(): catch-all for unexpected exceptions -> [ERROR] + errorlog, plus clean KeyboardInterrupt (130). - isolate: forgiving extension-name match (exact, then substring), excludes the paired "Restore" ext; errors on ambiguous match. - detections: --site -> --target-group; Alert.targetGroupId is a scan-target id, not a Location id (distinct from `agents --site`). - status: relabel "Target groups (sites)" -> "Scan target groups". - SKILL.md + docstrings updated to match. Verified: py_compile clean, selftest green (216 agents), guards fire on no-key/empty-where/no-agent, deploy-cmd --group picks the group's key. --- .claude/skills/datto-edr/SKILL.md | 11 +- .claude/skills/datto-edr/scripts/edr.py | 106 ++++++++++++++---- .../skills/datto-edr/scripts/edr_client.py | 32 ++++-- 3 files changed, 113 insertions(+), 36 deletions(-) diff --git a/.claude/skills/datto-edr/SKILL.md b/.claude/skills/datto-edr/SKILL.md index b8a210be..252c6800 100644 --- a/.claude/skills/datto-edr/SKILL.md +++ b/.claude/skills/datto-edr/SKILL.md @@ -84,7 +84,7 @@ $EDR sweep [--org ] # per-client posture rollup (headli $EDR extensions # list response/collection extensions $EDR tasks [--type "Scan - EDR"] [--limit N] # recent userTasks (scan/analysis jobs) $EDR task # one userTask (scan job) detail/status -$EDR deploy-cmd [--regkey ] # emit the agent install one-liner (see Deployment) +$EDR deploy-cmd --regkey | --group # emit the agent install one-liner (see Deployment) ``` `--json` is a global flag (place before the subcommand): `$EDR --json sweep`. @@ -122,9 +122,12 @@ $EDR raw POST Agents/scan --data '{"where":{"and":[{"id":[""]}]},"options":{ ## Deployment Agent install is **not a REST call** — it's the agent binary run on the endpoint. -`deploy-cmd` emits the official one-liner (pulls a live registration key from -`/agentKeys`). To land an agent in a SPECIFIC group: `create-group` → `mint-key` -(the key `id` is caller-supplied) → install with that `-RegKey`. +`deploy-cmd` emits the official one-liner. It needs an explicit key: either +`--regkey ` (used verbatim) or `--group ` (looks up THAT group's key +from `/agentKeys`). It will **not** auto-pick an arbitrary key — keys are +per-target-group and cross-client, so a wrong key lands the agent in the wrong org. +To land an agent in a SPECIFIC group: `create-group` → `mint-key` (the key `id` is +caller-supplied) → `deploy-cmd --group ` → install with that `-RegKey`. ``` Install-EDR -URL "https://azcomp4587.infocyte.com" -RegKey diff --git a/.claude/skills/datto-edr/scripts/edr.py b/.claude/skills/datto-edr/scripts/edr.py index 7b645e36..c488a2c0 100644 --- a/.claude/skills/datto-edr/scripts/edr.py +++ b/.claude/skills/datto-edr/scripts/edr.py @@ -10,17 +10,20 @@ Output: --json emits raw JSON; otherwise a readable table/summary. Usage examples: python edr.py status python edr.py orgs # clients (Organizations) - python edr.py sites [--org ] # target groups - python edr.py agents [--org ] [--site ] + python edr.py sites [--org ] # sites (Locations) + python edr.py agents [--org ] [--site ] python edr.py agent python edr.py detections [--org ] [--severity 3] [--days 7] python edr.py detection python edr.py sweep [--org ] # per-client posture rollup - python edr.py deploy-cmd [--regkey ] # emit agent install one-liner + python edr.py deploy-cmd --regkey | --group # install one-liner python edr.py extensions # list response/collection extensions - python edr.py scan --site --confirm - python edr.py scan --site --target --confirm - python edr.py isolate --site --target --extension-name "Host Isolation" --confirm + python edr.py tasks [--type "Scan - EDR"] # scan/analysis jobs + python edr.py scan --agent [--agent ...] --confirm + python edr.py isolate --agent --confirm + python edr.py cancel --confirm + python edr.py create-group --name "[TEST] X" --org --confirm + python edr.py mint-key --group --key <10char> --confirm python edr.py raw GET Agents --filter '{"limit":1}' """ from __future__ import annotations @@ -52,9 +55,13 @@ def _log_skill_error(skill, msg, context=""): pass +# Only GENUINELY expected/transient API responses are suppressed. Deliberately +# NARROW: a real auth failure (HTTP 401, e.g. the ~1yr token lapsing) MUST reach +# errorlog.md per CLAUDE.md - so we do NOT match broad words like "required", +# "unauthorized", "invalid value" that would swallow it. _EXPECTED_ERROR_MARKERS = ( - "404", "not found", "no method to handle", "invalid value", - "required", "must be", "429", "too many requests", "unauthorized", + "http 404", "not found", "no method to handle", + "http 429", "too many requests", ) @@ -64,10 +71,10 @@ def _is_expected_error(msg: str) -> bool: def _should_log_error(command: str, msg: str) -> bool: + # raw is exploratory but a genuine failure through it (401/500) is still a + # real API failure worth logging - filter only on expectedness, not command. if os.environ.get("EDR_SUPPRESS_ERRORLOG"): return False - if command == "raw": - return False return not _is_expected_error(msg) @@ -87,7 +94,7 @@ def _trunc(s, n): def _t_status(s): print(f"Datto EDR tenant: {s.get('instance')}") print(f" Organizations (clients): {s.get('organizations')}") - print(f" Target groups (sites) : {s.get('targetGroups')}") + print(f" Scan target groups : {s.get('targetGroups')}") print(f" Agents : {s.get('agents')} " f"({s.get('agentsActive')} active, {s.get('agentsIsolated')} isolated)") print(f" Alerts (all time) : {s.get('alerts')}") @@ -202,7 +209,9 @@ def build_parser() -> argparse.ArgumentParser: sp = sub.add_parser("detections", help="list alerts") sp.add_argument("--org") - sp.add_argument("--site") + sp.add_argument("--target-group", dest="target_group", + help="filter alerts by targetGroupId (Alert.targetGroupId is a " + "scan-target id, NOT a Location id - distinct from `agents --site`)") sp.add_argument("--severity", type=int, help="0 info..4 critical") sp.add_argument("--days", type=int) sp.add_argument("--limit", type=int, default=200) @@ -216,7 +225,9 @@ def build_parser() -> argparse.ArgumentParser: sub.add_parser("extensions", help="list agent extensions") sp = sub.add_parser("deploy-cmd", help="emit the agent install one-liner") - sp.add_argument("--regkey", help="registration key (else pulled from /agentKeys)") + sp.add_argument("--regkey", help="registration key string (used verbatim)") + sp.add_argument("--group", help="target group id; looks up THAT group's key from " + "/agentKeys (never auto-picks an arbitrary cross-client key)") sp = sub.add_parser("scan", help="trigger an EDR scan on specific agent(s) (gated)") sp.add_argument("--agent", action="append", dest="agents", metavar="AGENT_ID", @@ -287,15 +298,20 @@ def main(argv=None) -> int: elif cmd == "scan-targets": _emit(client.list_targets(org_id=args.org), j, _t_scan_targets) elif cmd == "agents": - if args.org and not args.site: + if args.site: + if args.org: + print("[INFO] --site given; --org ignored (a site is already " + "org-scoped).", file=sys.stderr) + rows = client.list_agents(location_id=args.site, limit=args.limit) + elif args.org: rows = _agents_for_org(client, args.org, args.limit) else: - rows = client.list_agents(location_id=args.site, limit=args.limit) + rows = client.list_agents(limit=args.limit) _emit(rows, j, _t_agents) elif cmd == "agent": _emit(client.get_agent(args.id), j) elif cmd == "detections": - _emit(client.list_alerts(org_id=args.org, target_group_id=args.site, + _emit(client.list_alerts(org_id=args.org, target_group_id=args.target_group, severity=args.severity, days=args.days, limit=args.limit), j, _t_detections) elif cmd == "detection": @@ -306,10 +322,22 @@ def main(argv=None) -> int: _emit(client.list_extensions(), j) elif cmd == "deploy-cmd": regkey = args.regkey - if not regkey: + if not regkey and args.group: + # Only the requested group's key - never auto-pick keys[0], which + # is an arbitrary CROSS-CLIENT key (agents would land in the wrong org). keys = client.list_agent_keys() - if isinstance(keys, list) and keys: - regkey = keys[0].get("key") or keys[0].get("id") + match = [k for k in (keys or []) if k.get("targetId") == args.group] + if not match: + print(f"[ERROR] no registration key found for group {args.group}. " + f"Mint one with: mint-key --group {args.group} --key <10char>", + file=sys.stderr) + return 1 + regkey = match[0].get("id") + if not regkey: + print("[ERROR] deploy-cmd needs --regkey or --group " + "(refusing to auto-pick an arbitrary cross-client key).", + file=sys.stderr) + return 2 _emit(_deploy_payload(client, regkey), j, _t_deploy) elif cmd == "scan": ids = args.agents or [] @@ -329,14 +357,30 @@ def main(argv=None) -> int: return 2 ext_id = args.extension_id if not ext_id: # resolve name -> id - exts = client.list_extensions() - match = [e for e in (exts or []) - if (e.get("name") or "").lower() == args.extension_name.lower()] + exts = client.list_extensions() or [] + want = args.extension_name.lower() + # Exact first; then forgiving substring (tolerates console renames + # like "Host Isolation [Windows]"). Exclude the paired "Restore" + # extension so we never isolate when asked to isolate-undo, etc. + exact = [e for e in exts if (e.get("name") or "").lower() == want] + if exact: + match = exact + else: + want_key = want.replace("[win/linux]", "").strip() + match = [e for e in exts + if want_key in (e.get("name") or "").lower() + and "restore" not in (e.get("name") or "").lower()] if not match: print(f"[ERROR] extension '{args.extension_name}' not found. " f"Run `extensions` to list, or pass --extension-id.", file=sys.stderr) return 1 + if len(match) > 1: + names = ", ".join(f"{e.get('name')} ({e.get('id')})" for e in match) + print(f"[ERROR] '{args.extension_name}' matched {len(match)} " + f"extensions: {names}. Pass --extension-id to disambiguate.", + file=sys.stderr) + return 1 ext_id = match[0].get("id") if not _gate(args, f"run extension {ext_id} ('{args.extension_name}') " f"on {len(ids)} agent(s): {', '.join(ids)}"): @@ -365,6 +409,16 @@ def main(argv=None) -> int: return 2 filt = json.loads(args.filter) if args.filter else None body = json.loads(args.data) if args.data else None + # Same tenant-wide footgun guard the scan command has: a POST to any + # */scan endpoint with no non-empty `where` scans the ENTIRE tenant. + if method == "POST" and args.path.rstrip("/").lower().endswith("scan"): + where = (body or {}).get("where") if isinstance(body, dict) else None + if not where: + print("[BLOCKED] POST to a */scan endpoint with no non-empty " + "`where` would scan the ENTIRE tenant. Add a where filter, " + "e.g. {\"where\":{\"and\":[{\"id\":[\"\"]}]}}.", + file=sys.stderr) + return 2 _emit(client.raw(method, args.path, filt=filt, body=body), j) else: print(f"[ERROR] unknown command {cmd}", file=sys.stderr) @@ -375,6 +429,14 @@ def main(argv=None) -> int: if _should_log_error(cmd, msg): _log_skill_error("datto-edr", msg, context=f"cmd={cmd}") return 1 + except KeyboardInterrupt: + print("\n[ERROR] interrupted", file=sys.stderr) + return 130 + except Exception as exc: # unexpected - always surfaced AND logged + msg = f"{type(exc).__name__}: {exc}" + print(f"[ERROR] unexpected: {msg}", file=sys.stderr) + _log_skill_error("datto-edr", msg, context=f"cmd={cmd} unexpected") + return 1 return 0 diff --git a/.claude/skills/datto-edr/scripts/edr_client.py b/.claude/skills/datto-edr/scripts/edr_client.py index c0dcda17..1a55c319 100644 --- a/.claude/skills/datto-edr/scripts/edr_client.py +++ b/.claude/skills/datto-edr/scripts/edr_client.py @@ -308,16 +308,20 @@ class DattoEDRClient: orgs = self.list_organizations() if org_id: orgs = [o for o in orgs if o.get("id") == org_id] + since = (datetime.now(timezone.utc) - timedelta(days=7)).isoformat() out = [] for o in orgs: oid = o.get("id") - recent = self.list_alerts(org_id=oid, days=7, limit=500) + # _count (not list+len) so a >500-alert client isn't silently capped + # and mis-sorted, and we don't transfer 500 full records per org. + recent = self._count("Alerts", {"organizationId": oid, + "createdOn": {"gt": since}}) out.append({ "organizationId": oid, "organization": o.get("name"), "agents": o.get("agentCount", 0), "alertsTotal": o.get("alertCount", 0), - "alertsLast7d": len(recent), + "alertsLast7d": recent, }) out.sort(key=lambda r: (-r["alertsLast7d"], (r["organization"] or "").lower())) return {"clients": out} @@ -334,15 +338,26 @@ class DattoEDRClient: # Response extensions (isolate/kill/quarantine) ride the same call via # options.extensions. (Verified live 2026-06-25.) # ====================================================================== + # Full EDR forensic collection - the default so a bare `scan` is a real sweep + # (an empty options body risks a thin scan that returns a false all-clear on a + # compromised host). Pass options={} explicitly to override with a lean scan. + DEFAULT_SCAN_OPTIONS = { + "process": True, "module": True, "driver": True, "memory": True, + "account": True, "artifact": True, "autostart": True, "application": True, + "installed": True, "events": True, + } + def scan_agents(self, agent_ids: list[str], options: Optional[dict] = None, task_name: str = "Scan - EDR") -> Any: """Trigger an EDR scan on specific agents (POST agents/scan). - Targets by Agent.id via a LoopBack where filter: - {"where":{"id":{"inq":[ids]}}, "options":{}, "taskName":"Scan - EDR"}. - REFUSES an empty agent_ids list - an empty where scans the whole tenant. - `options` may carry EDR forensic toggles + extensions; empty {} is valid. + Targets by Agent.id via the AND-wrapped LoopBack where the console uses: + {"where":{"and":[{"id":[ids]}]}, "options":{...}, "taskName":"Scan - EDR"}. + A bare {"id":{"inq":[...]}} returns HTTP 412 "column reference id is + ambiguous" (the scan query joins tables) - the and-wrap disambiguates. + REFUSES an empty agent_ids list (an absent where scans the whole tenant). + `options` defaults to DEFAULT_SCAN_OPTIONS (full forensic); pass {} for lean. STATE-CHANGING - gate behind --confirm at the call site.""" ids = [i for i in (agent_ids or []) if i] if not ids: @@ -350,12 +365,9 @@ class DattoEDRClient: "scan_agents refused: empty agent list. An absent `where` scans " "the ENTIRE tenant (156-host footgun) - pass explicit agent id(s)." ) - # AND-wrapped form is REQUIRED: a bare {id:{inq:[...]}} returns HTTP 412 - # "column reference id is ambiguous" (the backend joins tables). The - # console builds {and:[{id:[...]}]}; verified live -> "Scanning 1 host". body = { "where": {"and": [{"id": ids}]}, - "options": options or {}, + "options": self.DEFAULT_SCAN_OPTIONS if options is None else options, "taskName": task_name, } return self._request("POST", "Agents/scan", body=body)