From ff3c80e0a90545fee3890a264ae8429c1c2dc72b Mon Sep 17 00:00:00 2001 From: nbensalm-palo Date: Thu, 13 Aug 2026 10:47:15 +0300 Subject: [PATCH 01/20] First implementation --- .../Scripts/BlockDomain/BlockDomain.py | 655 ++++++++++++++++++ .../Scripts/BlockDomain/BlockDomain.yml | 102 +++ .../Scripts/BlockDomain/BlockDomain_test.py | 226 ++++++ 3 files changed, 983 insertions(+) create mode 100644 Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py create mode 100644 Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.yml create mode 100644 Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py new file mode 100644 index 000000000000..54f54d7c3a97 --- /dev/null +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py @@ -0,0 +1,655 @@ +import hashlib +import re +from typing import Any + +import demistomock as demisto # noqa: F401 +from CommonServerPython import * # noqa: F401 +from CommonServerUserPython import * # noqa: F401 + +""" CONSTANTS """ + +SUPPORTED_BRANDS = ["Panorama"] # v1 supports Panorama only; extended in the multi-brand follow-up. + +OBJECT_NAME_PREFIX = "Cortex-" +# PAN-OS object names are limited to 63 characters. Reserve room for the prefix and a hash suffix on overflow. +MAX_OBJECT_NAME_LENGTH = 63 +HASH_SUFFIX_LENGTH = 8 + +PRE_POST = "pre-rulebase" # Q2: hard-coded for v1 (may become an argument later). + +# Status values. +STATUS_DONE = "Done" +STATUS_PENDING = "Pending" +STATUS_SKIPPED = "Skipped" +STATUS_FAILED = "Failed" + +# Result values. +RESULT_SUCCESS = "Success" +RESULT_FAILED = "Failed" + +# Action values (ordered by significance for aggregation). +ACTION_CREATED = "Created" +ACTION_MODIFIED = "Modified" +ACTION_UNCHANGED = "Unchanged" +ACTION_SIGNIFICANCE = {ACTION_UNCHANGED: 0, ACTION_MODIFIED: 1, ACTION_CREATED: 2} + +# A permissive FQDN matcher: labels of alphanumerics/hyphens separated by dots, at least one dot. +FQDN_REGEX = re.compile(r"^(?=.{1,253}$)(?!-)[A-Za-z0-9-]{1,63}(? bool: + """A domain is treated as a wildcard (and therefore unsupported) if it contains an asterisk.""" + return "*" in domain + + +def is_valid_fqdn(domain: str) -> bool: + """Return True if the value looks like a valid, non-wildcard FQDN.""" + return bool(FQDN_REGEX.match(domain)) + + +def derive_object_name(domain: str) -> str: + """Derive a deterministic address-object name from a domain. + + The name is a pure function of the domain so re-runs are idempotent. On overflow of the PAN-OS + max object-name length, the sanitised body is truncated and a short deterministic hash suffix is + appended to keep the name unique. + """ + sanitised = re.sub(r"[^A-Za-z0-9.\-]", "-", domain).strip("-") + candidate = f"{OBJECT_NAME_PREFIX}{sanitised}" + if len(candidate) <= MAX_OBJECT_NAME_LENGTH: + return candidate + + digest = hashlib.sha256(domain.encode("utf-8")).hexdigest()[:HASH_SUFFIX_LENGTH] + keep = MAX_OBJECT_NAME_LENGTH - len(OBJECT_NAME_PREFIX) - 1 - HASH_SUFFIX_LENGTH # 1 for the '-' separator. + truncated = sanitised[:keep].strip("-") + return f"{OBJECT_NAME_PREFIX}{truncated}-{digest}" + + +def most_significant_action(actions: list) -> str: + """Return the most significant action from a list (Created > Modified > Unchanged).""" + if not actions: + return ACTION_UNCHANGED + return max(actions, key=lambda action: ACTION_SIGNIFICANCE.get(action, 0)) + + +def build_result_row( + domain: str, + brand: str, + status: str, + result: str, + action: str, + message: str, + instance: str = "", + rule_name: str = "", +) -> dict: + """Assemble a single BlockDomainResults row in the canonical field order.""" + return { + "Domain": domain, + "Brand": brand, + "Instance": instance, + "Status": status, + "Result": result, + "Action": action, + "RuleName": rule_name, + "Message": message, + } + + +def validate_domains(domain_list: list) -> tuple[list, list]: + """Split the input into (valid_domains, skipped_rows). + + Wildcard and invalid entries never reach a vendor; they produce a per-row Skipped result while + the rest of the list continues. + """ + valid_domains: list = [] + skipped_rows: list = [] + for domain in domain_list: + if is_wildcard(domain): + skipped_rows.append( + build_result_row( + domain=domain, + brand="", + status=STATUS_SKIPPED, + result=RESULT_FAILED, + action=ACTION_UNCHANGED, + message=f"Wildcard domain '{domain}' is not supported by this script; skipped.", + ) + ) + elif not is_valid_fqdn(domain): + skipped_rows.append( + build_result_row( + domain=domain, + brand="", + status=STATUS_SKIPPED, + result=RESULT_FAILED, + action=ACTION_UNCHANGED, + message=f"Invalid FQDN '{domain}' - skipped.", + ) + ) + else: + valid_domains.append(domain) + return valid_domains, skipped_rows + + +def get_enabled_brands() -> set: + """Return the set of brands that have at least one active instance.""" + modules = demisto.getModules() + enabled_brands = {module.get("brand") for module in modules.values() if module.get("state") == "active"} + demisto.debug(f"BlockDomain: the enabled modules are: {enabled_brands=}") + return enabled_brands + + +""" EXECUTE-COMMAND / CONTEXT HELPERS """ + + +def run_execute_command(command_name: str, args: dict[str, Any]) -> list[dict]: + """Execute a command and return its raw entries.""" + demisto.debug(f"BlockDomain: Executing command: {command_name} with {args=}") + res = demisto.executeCommand(command_name, args) + demisto.debug(f"BlockDomain: The response of {command_name} is {res}") + return res + + +def get_relevant_context(original_context: dict[str, Any], key: str) -> dict | list: + """Get the relevant context object from the execute_command response, tolerating suffixed keys.""" + if not original_context: + return {} + if relevant_context := original_context.get(key, {}): + return relevant_context + for k in original_context: + if k.startswith(key): + return original_context.get(k, {}) + return {} + + +def is_error_entry(entry: dict) -> bool: + """Return True if an execute_command entry is an error entry.""" + return isinstance(entry, dict) and entry.get("Type") == entryTypes["error"] + + +def get_entry_error(entry: dict) -> str: + """Extract a human-readable error message from an error entry.""" + return str(entry.get("Contents", "Unknown error")) + + +def as_list(value: Any) -> list: + """Normalise a context value that may be a dict, list, or None into a list.""" + if value is None: + return [] + return value if isinstance(value, list) else [value] + + +""" POLLING FUNCTIONS (commit / push) """ + + +@polling_function(name="block-domain", interval=60, timeout=1200) +def pan_os_commit(args: dict, responses: list) -> PollResult: + """Execute pan-os-commit and start polling on the returned job.""" + res_commit = run_execute_command("pan-os-commit", {"polling": True}) + polling_args = res_commit[0].get("Metadata", {}).get("pollingArgs", {}) + job_id = polling_args.get("commit_job_id") + if job_id: + context_output = {"JobID": job_id, "Status": "Pending"} + continue_to_poll = True + commit_output: Any = CommandResults( + outputs=context_output, readable_output=tableToMarkdown("Commit Status:", context_output, removeNull=True) + ) + demisto.setContext("commit_job_id", job_id) + else: + commit_output = res_commit[0].get("Contents") or "There are no changes to commit." + continue_to_poll = False + global POLLING + POLLING = continue_to_poll + + args_for_next_run = args | { + "commit_job_id": job_id, + "interval_in_seconds": arg_to_number(args.get("interval_in_seconds", 60)), + "timeout": arg_to_number(args.get("timeout", 1200)), + "polling": True, + } + responses.append(res_commit) + return PollResult( + response=commit_output, + continue_to_poll=continue_to_poll, + args_for_next_run=args_for_next_run, + partial_result=CommandResults(readable_output=f"Waiting for commit job ID {job_id} to finish..."), + ) + + +@polling_function(name="block-domain", interval=60, timeout=1200) +def pan_os_commit_status(args: dict, responses: list) -> PollResult: + """Check the status of the commit job in pan-os.""" + commit_job_id = args["commit_job_id"] + res_commit_status = run_execute_command("pan-os-commit-status", {"job_id": commit_job_id}) + responses.append(res_commit_status) + result_commit_status = res_commit_status[0].get("Contents", {}).get("response", {}).get("result", {}).get("job", {}) + job_result = result_commit_status.get("result") + commit_output = {"JobID": commit_job_id, "Status": "Success" if job_result == "OK" else "Failure"} + continue_to_poll = result_commit_status.get("status") != "FIN" + global POLLING + POLLING = continue_to_poll + return PollResult( + response=CommandResults( + outputs=commit_output, + outputs_key_field="JobID", + readable_output=tableToMarkdown("Commit Status:", commit_output, removeNull=True), + ), + args_for_next_run=args, + continue_to_poll=continue_to_poll, + ) + + +@polling_function(name="block-domain", interval=60, timeout=1200) +def pan_os_push_to_device(args: dict, responses: list) -> PollResult: + """Execute pan-os-push-to-device-group and start polling on the returned job.""" + res_push_to_device = run_execute_command("pan-os-push-to-device-group", {"polling": True}) + responses.append(res_push_to_device) + polling_args = res_push_to_device[0].get("Metadata", {}).get("pollingArgs", {}) + job_id = polling_args.get("push_job_id") + device_group = polling_args.get("device-group") + if job_id: + context_output = {"DeviceGroup": device_group, "JobID": job_id, "Status": "Pending"} + continue_to_poll = True + push_cr = CommandResults( + outputs_key_field="JobID", + outputs=context_output, + readable_output=tableToMarkdown("Push to Device Group:", context_output, removeNull=True), + ) + demisto.setContext("push_job_id", job_id) + else: + push_cr = CommandResults(readable_output=res_push_to_device[0].get("Contents") or "There are no changes to push.") + continue_to_poll = False + global POLLING + POLLING = continue_to_poll + return PollResult( + response=push_cr, + continue_to_poll=continue_to_poll, + partial_result=CommandResults(readable_output=f"Waiting for Job-ID {job_id} to finish pushing the changes..."), + ) + + +@polling_function(name="block-domain", interval=60, timeout=1200) +def pan_os_push_status(args: dict, responses: list) -> PollResult: + """Check the status of the push job in pan-os.""" + push_job_id = args["push_job_id"] + res_push_status = run_execute_command("pan-os-push-status", {"job_id": push_job_id}) + responses.append(res_push_status) + push_status = res_push_status[0].get("Contents", {}).get("response", {}).get("result", {}).get("job", {}).get("status", "") + continue_to_poll = bool(push_status and push_status != "FIN") + context_output = {"Status": push_status, "JobID": push_job_id} + push_cr = CommandResults( + outputs_key_field="JobID", + outputs=context_output, + readable_output=tableToMarkdown("Push to Device Group:", context_output, ["JobID", "Status"], removeNull=True), + ) + global POLLING + POLLING = continue_to_poll + return PollResult( + response=push_cr, + continue_to_poll=continue_to_poll, + partial_result=CommandResults(readable_output=f"Waiting for Job-ID {push_job_id} to finish pushing the changes..."), + ) + + +""" PAN-OS FLOW """ + + +class DynamicGroupError(Exception): + """Raised when the target address-group exists and is dynamic (customer-managed).""" + + +class PanOs: + """Implements the PAN-OS static-address-group domain-blocking flow. + + For each valid domain the flow probes/creates an FQDN address-object, ensures it belongs to the + static address-group, and ensures a single deny rule points at the group. Commit + optional push + happen once after all domains are processed. Every step records its effect (Created / Modified / + Unchanged) so the aggregated per-domain row reflects the most significant change. + """ + + def __init__(self, args: dict): + self.args = args + self.brand = "Panorama" + self.rule_name = args["rule_name"] + self.address_group = args["address_group"] + self.tag = args.get("tag", "") + self.log_forwarding_name = args.get("log_forwarding_name", "") + self.domains: list = args.get("domains", []) + self.responses: list = [] + # Per-domain accumulated actions and messages, keyed by domain. + self.domain_actions: dict = {domain: [] for domain in self.domains} + self.domain_messages: dict = {domain: [] for domain in self.domains} + + # ---- context probes ------------------------------------------------- + + def address_object_exists(self, object_name: str) -> bool: + """Return True if an address-object with this name already exists.""" + res = run_execute_command("pan-os-get-address", {"name": object_name}) + entry = res[0] if res else {} + if is_error_entry(entry): + # get-address raises when the object is absent; treat that as 'does not exist'. + demisto.debug(f"BlockDomain: address '{object_name}' not found ({get_entry_error(entry)}).") + return False + self.responses.append(res) + context = get_relevant_context(entry.get("EntryContext", {}), "Panorama.Addresses") + return any(item.get("Name") == object_name for item in as_list(context)) + + def get_address_group(self) -> dict | None: + """Return the target address-group context dict, or None if it does not exist.""" + res = run_execute_command("pan-os-list-address-groups", {}) + self.responses.append(res) + entry = res[0] if res else {} + if is_error_entry(entry): + raise DemistoException(f"Failed to list address groups: {get_entry_error(entry)}") + context = get_relevant_context(entry.get("EntryContext", {}), "Panorama.AddressGroups") + for item in as_list(context): + if item.get("Name") == self.address_group: + return item + return None + + def group_members(self, group_context: dict) -> list: + """Return the current static-group member names.""" + return as_list(group_context.get("Addresses")) + + def rule_exists(self) -> bool: + """Return True if a rule named self.rule_name already exists in the rulebase.""" + res = run_execute_command("pan-os-list-rules", {"pre_post": PRE_POST}) + self.responses.append(res) + entry = res[0] if res else {} + if is_error_entry(entry): + raise DemistoException(f"Failed to list rules: {get_entry_error(entry)}") + context = get_relevant_context(entry.get("EntryContext", {}), "Panorama.SecurityRule") + return any(item.get("Name") == self.rule_name for item in as_list(context)) + + # ---- writes --------------------------------------------------------- + + def ensure_address_object(self, domain: str, object_name: str) -> None: + """Create the FQDN address-object if it does not already exist.""" + if self.address_object_exists(object_name): + self.domain_actions[domain].append(ACTION_UNCHANGED) + self.domain_messages[domain].append(f"Address-object '{object_name}' already exists.") + return + create_args: dict = {"name": object_name, "fqdn": domain} + if self.tag: + create_args["tag"] = self.tag + res = run_execute_command("pan-os-create-address", create_args) + self.responses.append(res) + entry = res[0] if res else {} + if is_error_entry(entry): + raise DemistoException(f"Failed to create address-object '{object_name}': {get_entry_error(entry)}") + self.domain_actions[domain].append(ACTION_CREATED) + self.domain_messages[domain].append(f"Address-object '{object_name}' created for '{domain}'.") + + def ensure_group_membership(self, domain: str, object_name: str, group_context: dict | None) -> None: + """Create the static group or add the object to it, aborting if the group is dynamic.""" + if group_context is None: + create_args: dict = {"name": self.address_group, "type": "static", "addresses": [object_name]} + if self.tag: + create_args["tags"] = self.tag + res = run_execute_command("pan-os-create-address-group", create_args) + self.responses.append(res) + entry = res[0] if res else {} + if is_error_entry(entry): + raise DemistoException(f"Failed to create address-group '{self.address_group}': {get_entry_error(entry)}") + self.domain_actions[domain].append(ACTION_CREATED) + self.domain_messages[domain].append(f"Static address-group '{self.address_group}' created.") + return + + group_type = (group_context.get("Type") or "").lower() + if group_type == "dynamic": + raise DynamicGroupError( + f"Address-group '{self.address_group}' already exists as dynamic; " + f"will not modify a customer-managed dynamic group." + ) + + if object_name in self.group_members(group_context): + self.domain_actions[domain].append(ACTION_UNCHANGED) + self.domain_messages[domain].append( + f"Address-object '{object_name}' is already a member of '{self.address_group}'." + ) + return + + res = run_execute_command( + "pan-os-edit-address-group", + {"name": self.address_group, "type": "static", "element_to_add": object_name}, + ) + self.responses.append(res) + entry = res[0] if res else {} + if is_error_entry(entry): + raise DemistoException(f"Failed to add object to address-group '{self.address_group}': {get_entry_error(entry)}") + self.domain_actions[domain].append(ACTION_MODIFIED) + self.domain_messages[domain].append(f"Address-object '{object_name}' added to '{self.address_group}'.") + + def ensure_rule(self, rule_present: bool) -> str: + """Create the deny rule if missing, then move it to the top. Returns the rule action.""" + if not rule_present: + create_rule_args: dict = { + "rulename": self.rule_name, + "action": "deny", + "source": "any", + "destination": self.address_group, + "application": "any", + "service": "any", + "pre_post": PRE_POST, + "where": "top", + } + if self.tag: + create_rule_args["tags"] = self.tag + if self.log_forwarding_name: + create_rule_args["log_forwarding"] = self.log_forwarding_name + res = run_execute_command("pan-os-create-rule", create_rule_args) + self.responses.append(res) + entry = res[0] if res else {} + if is_error_entry(entry): + raise DemistoException(f"Failed to create rule '{self.rule_name}': {get_entry_error(entry)}") + rule_action = ACTION_CREATED + else: + rule_action = ACTION_UNCHANGED + + # Always ensure the rule sits at the top of the rulebase. + res_move = run_execute_command("pan-os-move-rule", {"rulename": self.rule_name, "where": "top", "pre_post": PRE_POST}) + self.responses.append(res_move) + move_entry = res_move[0] if res_move else {} + if is_error_entry(move_entry): + raise DemistoException(f"Failed to move rule '{self.rule_name}' to top: {get_entry_error(move_entry)}") + return rule_action + + # ---- orchestration -------------------------------------------------- + + def process_domains(self) -> list: + """Run the per-domain object + group flow, then the single shared rule step. + + Returns the list of BlockDomainResults rows for the processed domains. + """ + rows: list = [] + try: + group_context = self.get_address_group() + for domain in self.domains: + object_name = derive_object_name(domain) + self.ensure_address_object(domain, object_name) + self.ensure_group_membership(domain, object_name, group_context) + # Re-read the group once created so subsequent domains see the new membership. + if group_context is None: + group_context = self.get_address_group() + + rule_present = self.rule_exists() + rule_action = self.ensure_rule(rule_present) + + for domain in self.domains: + actions = self.domain_actions[domain] + [rule_action] + rows.append( + build_result_row( + domain=domain, + brand=self.brand, + status=STATUS_DONE, + result=RESULT_SUCCESS, + action=most_significant_action(actions), + rule_name=self.rule_name, + message=" ".join(self.domain_messages[domain]) + + (f" Rule '{self.rule_name}' created at top." if rule_action == ACTION_CREATED else "") + + (f" Rule '{self.rule_name}' already present; moved to top." if rule_action == ACTION_UNCHANGED else ""), + ) + ) + except DynamicGroupError as dyn_err: + # Abort the whole brand for this run; other brands (future) would continue. + for domain in self.domains: + rows.append( + build_result_row( + domain=domain, + brand=self.brand, + status=STATUS_SKIPPED, + result=RESULT_SUCCESS, + action=ACTION_UNCHANGED, + rule_name="", + message=str(dyn_err), + ) + ) + except Exception as ex: + for domain in self.domains: + rows.append( + build_result_row( + domain=domain, + brand=self.brand, + status=STATUS_FAILED, + result=RESULT_FAILED, + action=ACTION_UNCHANGED, + rule_name=self.rule_name, + message=f"Failed to block '{domain}' on Panorama: {ex!s}", + ) + ) + return rows + + def run(self) -> list: # pragma: no cover + """Execute the full PAN-OS flow: per-domain writes, then commit + optional push.""" + rows = self.process_domains() + + # Only commit/push if at least one domain actually reached Done (i.e. no dynamic-group abort / failure). + made_changes = any(row["Status"] == STATUS_DONE for row in rows) + auto_commit = argToBoolean(self.args.get("auto_commit", True)) + if made_changes and auto_commit: + try: + pan_os_commit(self.args, self.responses) + if self.pan_os_is_panorama(): + pan_os_push_to_device(self.args, self.responses) + except Exception as ex: + rows.append( + build_result_row( + domain="", + brand=self.brand, + status=STATUS_FAILED, + result=RESULT_FAILED, + action=ACTION_UNCHANGED, + message=f"Commit/push failed: {ex!s}. Objects and rule are staged but may not be active.", + ) + ) + return rows + + def pan_os_is_panorama(self) -> bool: + """Return True if the instance is a Panorama (vs a single firewall).""" + res = run_execute_command("pan-os", {"cmd": "", "type": "op"}) + self.responses.append(res) + context = get_relevant_context(res[0].get("EntryContext", {}), "Panorama.Command") + model = context.get("response", {}).get("result", {}).get("system", {}).get("model", "") # type: ignore + return model == "Panorama" + + +""" MAIN FUNCTION """ + + +def main(): # pragma: no cover + try: + args = demisto.args() + demisto.debug(f"The script block-domain was called with the arguments {args=}") + + domain_list = argToList(args.get("domain_list", [])) + rule_name = args.get("rule_name", "Cortex - Block Domain") + log_forwarding_name = args.get("log_forwarding_name", "") + address_group = args.get("address_group", "Blocked Domains - Cortex") + tag = args.get("tag", "cortex-blocked-domains") + auto_commit = argToBoolean(args.get("auto_commit", True)) + verbose = argToBoolean(args.get("verbose", False)) + brands_to_run = argToList(args.get("brands", ",".join(SUPPORTED_BRANDS))) + demisto.debug(f"BlockDomain: {verbose=}, {brands_to_run=}") + + valid_domains, skipped_rows = validate_domains(domain_list) + demisto.debug(f"BlockDomain: {valid_domains=}, skipped {len(skipped_rows)} entries.") + + enabled_brands = get_enabled_brands() + brands_to_run = brands_to_run or list(SUPPORTED_BRANDS) + + runnable_brands = [b for b in brands_to_run if b in SUPPORTED_BRANDS and b in enabled_brands] + if not runnable_brands: + return_error( + f"No integrations were found for the brands {brands_to_run}. " + f"Please verify the brand instances' setup. Supported brands: {SUPPORTED_BRANDS}." + ) + + results: list = list(skipped_rows) + + for brand in brands_to_run: + if brand not in SUPPORTED_BRANDS: + results.append( + build_result_row( + domain="", + brand=brand, + status=STATUS_SKIPPED, + result=RESULT_FAILED, + action=ACTION_UNCHANGED, + message=f"The brand {brand} is not supported by 'block-domain'. Supported: {SUPPORTED_BRANDS}.", + ) + ) + elif brand not in enabled_brands: + results.append( + build_result_row( + domain="", + brand=brand, + status=STATUS_SKIPPED, + result=RESULT_FAILED, + action=ACTION_UNCHANGED, + message=f"The brand {brand} isn't enabled.", + ) + ) + elif brand == "Panorama" and valid_domains: + pan_os = PanOs( + { + "domains": valid_domains, + "rule_name": rule_name, + "log_forwarding_name": log_forwarding_name, + "address_group": address_group, + "tag": tag, + "auto_commit": auto_commit, + "verbose": verbose, + "commit_job_id": args.get("commit_job_id"), + "push_job_id": args.get("push_job_id"), + "polling": True, + } + ) + results.extend(pan_os.run()) + + return_results( + CommandResults( + outputs_prefix="BlockDomainResults", + outputs_key_field=["Domain", "Brand"], + outputs=results, + readable_output=tableToMarkdown( + "Block Domain", + results, + headers=["Domain", "Brand", "Instance", "Status", "Result", "Action", "RuleName", "Message"], + removeNull=False, + ), + ) + ) + + except Exception as ex: + return_error(f"Failed to execute block-domain. Error: {ex!s}") + + +""" ENTRY POINT """ + +if __name__ in ("__main__", "__builtin__", "builtins"): + main() diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.yml b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.yml new file mode 100644 index 000000000000..985950a3f882 --- /dev/null +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.yml @@ -0,0 +1,102 @@ +args: +- description: List of domain FQDNs to block. Wildcard entries (e.g. *.evil.com) are not supported and are skipped. + isArray: true + name: domain_list + required: true +- defaultValue: 'Cortex - Block Domain' + description: The name of the rule which will be created in the relevant products. + isArray: false + name: rule_name + required: false +- description: Panorama log forwarding object name. Indicate what type of Log Forwarding setting will be specified in the PAN-OS custom rules. + isArray: false + name: log_forwarding_name + required: false +- description: This input determines whether PANW Panorama or Firewall Address Groups are used. Specify the Address Group name for FQDN handling. + isArray: false + name: address_group + required: false + defaultValue: 'Blocked Domains - Cortex' +- description: Whether to commit the new rule and push to the device group at the end of the run. + isArray: false + name: auto_commit + required: false + defaultValue: 'true' + auto: PREDEFINED + predefined: + - 'true' + - 'false' +- description: The designated tag name for the domain FQDN object. Applied to every object the script creates. + isArray: false + name: tag + required: false + defaultValue: 'cortex-blocked-domains' +- description: |- + Which integrations brands to run the command for. If not provided, the command will run for all available integrations. + For multi-select provide a comma-separated list. + isArray: true + name: brands + required: false + auto: PREDEFINED + predefined: + - 'Panorama' +- description: Whether to retrieve a human-readable entry for every command or only the final result. True retrieves a human-readable entry for every command. False retrieves a human-readable entry only for the final result. + name: verbose + defaultValue: 'false' + auto: PREDEFINED + predefined: + - 'true' + - 'false' +- description: commit job ID to use in polling commands. (automatically filled by polling). + name: commit_job_id + hidden: true +- description: publish job ID to use in polling commands. (automatically filled by polling). + name: publish_job_id + hidden: true +comment: The script blocks a list of domain FQDNs in supported integrations. +commonfields: + id: block-domain + version: -1 +enabled: false +name: block-domain +outputs: +- contextPath: BlockDomainResults.Domain + description: The domain FQDN that was processed. + type: String +- contextPath: BlockDomainResults.Brand + description: The brand (integration) used to block the domain. + type: String +- contextPath: BlockDomainResults.Instance + description: The integration instance used to block the domain. + type: String +- contextPath: BlockDomainResults.Status + description: The lifecycle status of the action. One of Done, Pending, Skipped, Failed. + type: String +- contextPath: BlockDomainResults.Result + description: The result of the action. Success or Failed. + type: String +- contextPath: BlockDomainResults.Action + description: The effect the run had on the target object. One of Created, Modified, Unchanged. + type: String +- contextPath: BlockDomainResults.RuleName + description: The name of the rule used for this integration. Empty if no rule was used. + type: String +- contextPath: BlockDomainResults.Message + description: A message concerning the result of the action. + type: String +script: '-' +system: false +timeout: 20m0s +type: python +subtype: python3 +compliantpolicies: + - IP Blockage +dockerimage: demisto/python3:3.12.13.10116658 +fromversion: 6.1.0 +marketplaces: +- xsoar +- marketplacev2 +- platform +polling: true +tests: +- No tests (auto formatted) diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py new file mode 100644 index 000000000000..eee4fdcbae79 --- /dev/null +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py @@ -0,0 +1,226 @@ +import pytest +from BlockDomain import ( + ACTION_CREATED, + ACTION_MODIFIED, + ACTION_UNCHANGED, + OBJECT_NAME_PREFIX, + MAX_OBJECT_NAME_LENGTH, + RESULT_FAILED, + RESULT_SUCCESS, + STATUS_DONE, + STATUS_FAILED, + STATUS_SKIPPED, + PanOs, + derive_object_name, + is_valid_fqdn, + is_wildcard, + most_significant_action, + validate_domains, +) + + +def ok_entry(entry_context=None, contents="ok"): + """Build a minimal successful execute_command entry.""" + return {"Type": 1, "Contents": contents, "HumanReadable": "", "EntryContext": entry_context or {}} + + +def err_entry(contents="error"): + """Build a minimal error execute_command entry (Type 4 == entryTypes['error']).""" + return {"Type": 4, "Contents": contents, "HumanReadable": "", "EntryContext": {}} + + +@pytest.mark.parametrize( + "domain, expected", + [ + ("*.evil.com", True), + ("evil.*.com", True), + ("evil.example.com", False), + ("sub.domain.co.uk", False), + ], +) +def test_is_wildcard(domain, expected): + assert is_wildcard(domain) is expected + + +@pytest.mark.parametrize( + "domain, expected", + [ + ("evil.example.com", True), + ("sub.domain.co.uk", True), + ("a.b", True), + ("no-dot", False), + ("*.evil.com", False), + ("-leading.example.com", False), + ("trailing-.example.com", False), + ("space in.example.com", False), + ("", False), + ], +) +def test_is_valid_fqdn(domain, expected): + assert is_valid_fqdn(domain) is expected + + +def test_derive_object_name_simple(): + assert derive_object_name("evil.example.com") == "Cortex-evil.example.com" + + +def test_derive_object_name_is_deterministic(): + assert derive_object_name("evil.example.com") == derive_object_name("evil.example.com") + + +def test_derive_object_name_sanitises_illegal_chars(): + # Underscores are not valid PAN-OS object-name characters; they get normalised to hyphens. + assert derive_object_name("bad_domain.example.com") == "Cortex-bad-domain.example.com" + + +def test_derive_object_name_overflow_truncates_and_hashes(): + long_domain = ("a" * 80) + ".example.com" + name = derive_object_name(long_domain) + assert len(name) <= MAX_OBJECT_NAME_LENGTH + assert name.startswith(OBJECT_NAME_PREFIX) + # Overflow names are still deterministic. + assert name == derive_object_name(long_domain) + + +def test_validate_domains_splits_valid_and_skipped(): + valid, skipped = validate_domains(["evil.example.com", "*.evil.com", "no-dot", "phish.attacker.net"]) + + assert valid == ["evil.example.com", "phish.attacker.net"] + assert len(skipped) == 2 + + wildcard_row = next(row for row in skipped if row["Domain"] == "*.evil.com") + assert wildcard_row["Status"] == STATUS_SKIPPED + assert wildcard_row["Result"] == RESULT_SUCCESS + assert wildcard_row["Action"] == ACTION_UNCHANGED + assert "Wildcard" in wildcard_row["Message"] + + invalid_row = next(row for row in skipped if row["Domain"] == "no-dot") + assert invalid_row["Status"] == STATUS_SKIPPED + assert "Invalid FQDN" in invalid_row["Message"] + + +def test_validate_domains_all_valid(): + valid, skipped = validate_domains(["a.com", "b.org"]) + assert valid == ["a.com", "b.org"] + assert skipped == [] + + +@pytest.mark.parametrize( + "actions, expected", + [ + ([], ACTION_UNCHANGED), + ([ACTION_UNCHANGED, ACTION_UNCHANGED], ACTION_UNCHANGED), + ([ACTION_UNCHANGED, ACTION_MODIFIED], ACTION_MODIFIED), + ([ACTION_MODIFIED, ACTION_CREATED], ACTION_CREATED), + ([ACTION_CREATED, ACTION_UNCHANGED], ACTION_CREATED), + ], +) +def test_most_significant_action(actions, expected): + assert most_significant_action(actions) == expected + + +def _pan_os(domains): + return PanOs( + { + "domains": domains, + "rule_name": "Cortex - Block Domain", + "address_group": "Blocked Domains - Cortex", + "tag": "cortex-blocked-domains", + "log_forwarding_name": "", + "auto_commit": True, + } + ) + + +def _mock_execute(monkeypatch, side_effect): + """Patch BlockDomain.demisto.executeCommand to yield the given responses in order.""" + import BlockDomain + + responses = iter(side_effect) + monkeypatch.setattr(BlockDomain.demisto, "executeCommand", lambda *a, **k: next(responses)) + + +def test_process_domains_create_everything(monkeypatch): + # Group missing -> create group; address missing -> create address; rule missing -> create + move. + _mock_execute( + monkeypatch, + [ + [ok_entry({"Panorama.AddressGroups": []})], # list-address-groups (missing) + [err_entry("not found")], # get-address (missing) + [ok_entry({"Panorama.Addresses": [{"Name": "Cortex-evil.example.com"}]})], # create-address + [ok_entry()], # create-address-group + [ok_entry({"Panorama.AddressGroups": [{"Name": "Blocked Domains - Cortex", "Type": "static"}]})], # re-list group + [ok_entry({"Panorama.SecurityRule": []})], # list-rules (missing) + [ok_entry()], # create-rule + [ok_entry()], # move-rule + ], + ) + rows = _pan_os(["evil.example.com"]).process_domains() + assert len(rows) == 1 + assert rows[0]["Status"] == STATUS_DONE + assert rows[0]["Result"] == RESULT_SUCCESS + assert rows[0]["Action"] == ACTION_CREATED + assert rows[0]["RuleName"] == "Cortex - Block Domain" + + +def test_process_domains_all_unchanged(monkeypatch): + obj = "Cortex-evil.example.com" + _mock_execute( + monkeypatch, + [ + [ok_entry({"Panorama.AddressGroups": [{"Name": "Blocked Domains - Cortex", "Type": "static", "Addresses": [obj]}]})], + [ok_entry({"Panorama.Addresses": [{"Name": obj}]})], # get-address (exists) + [ok_entry({"Panorama.SecurityRule": [{"Name": "Cortex - Block Domain"}]})], # list-rules (exists) + [ok_entry()], # move-rule + ], + ) + rows = _pan_os(["evil.example.com"]).process_domains() + assert rows[0]["Status"] == STATUS_DONE + assert rows[0]["Action"] == ACTION_UNCHANGED + + +def test_process_domains_modified_when_added_to_existing_group(monkeypatch): + _mock_execute( + monkeypatch, + [ + [ok_entry({"Panorama.AddressGroups": [{"Name": "Blocked Domains - Cortex", "Type": "static", "Addresses": []}]})], + [err_entry("not found")], # get-address (missing) + [ok_entry()], # create-address + [ok_entry()], # edit-address-group (add member) + [ok_entry({"Panorama.SecurityRule": [{"Name": "Cortex - Block Domain"}]})], # list-rules (exists) + [ok_entry()], # move-rule + ], + ) + rows = _pan_os(["evil.example.com"]).process_domains() + assert rows[0]["Status"] == STATUS_DONE + assert rows[0]["Action"] == ACTION_CREATED # object was created -> most significant + + +def test_process_domains_dynamic_group_is_skipped(monkeypatch): + _mock_execute( + monkeypatch, + [ + [ok_entry({"Panorama.AddressGroups": [{"Name": "Blocked Domains - Cortex", "Type": "dynamic", "Match": "x"}]})], + [err_entry("not found")], # get-address (missing) + [ok_entry()], # create-address + ], + ) + rows = _pan_os(["evil.example.com"]).process_domains() + assert rows[0]["Status"] == STATUS_SKIPPED + assert rows[0]["Result"] == RESULT_SUCCESS + assert "dynamic" in rows[0]["Message"] + + +def test_process_domains_failure_marks_row_failed(monkeypatch): + _mock_execute( + monkeypatch, + [ + [ok_entry({"Panorama.AddressGroups": []})], # list-address-groups + [err_entry("not found")], # get-address (missing) + [err_entry("permission denied")], # create-address fails + ], + ) + rows = _pan_os(["evil.example.com"]).process_domains() + assert rows[0]["Status"] == STATUS_FAILED + assert rows[0]["Result"] == RESULT_FAILED + assert "permission denied" in rows[0]["Message"] From 99ec42bb6af7b790f215b4b85461bef9297322a7 Mon Sep 17 00:00:00 2001 From: nbensalm-palo Date: Thu, 13 Aug 2026 15:42:52 +0300 Subject: [PATCH 02/20] First implementation fixes --- .../Scripts/BlockDomain/BlockDomain.py | 781 +++++++++++------- .../Scripts/BlockDomain/BlockDomain_test.py | 59 +- 2 files changed, 524 insertions(+), 316 deletions(-) diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py index 54f54d7c3a97..b2043cd60dd6 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py @@ -1,3 +1,4 @@ +import ast import hashlib import re from typing import Any @@ -14,9 +15,14 @@ # PAN-OS object names are limited to 63 characters. Reserve room for the prefix and a hash suffix on overflow. MAX_OBJECT_NAME_LENGTH = 63 HASH_SUFFIX_LENGTH = 8 +# Characters that are not allowed in a PAN-OS object name are normalised to a hyphen. +OBJECT_NAME_SANITIZE_REGEX = re.compile(r"[^A-Za-z0-9.\-]") PRE_POST = "pre-rulebase" # Q2: hard-coded for v1 (may become an argument later). +# A permissive FQDN matcher: labels of alphanumerics/hyphens separated by dots, at least one dot. +FQDN_REGEX = re.compile(r"^(?=.{1,253}$)(?!-)[A-Za-z0-9-]{1,63}(? bool: - """A domain is treated as a wildcard (and therefore unsupported) if it contains an asterisk.""" + """Check whether a domain is a wildcard (unsupported). + + Args: + domain (str): The domain to check. + Returns: + True if the domain contains an asterisk, False otherwise. + """ return "*" in domain def is_valid_fqdn(domain: str) -> bool: - """Return True if the value looks like a valid, non-wildcard FQDN.""" + """Check whether a value is a valid, non-wildcard FQDN. + + Args: + domain (str): The domain to validate. + Returns: + True if the value looks like a valid FQDN, False otherwise. + """ return bool(FQDN_REGEX.match(domain)) def derive_object_name(domain: str) -> str: - """Derive a deterministic address-object name from a domain. + """Derive a deterministic PAN-OS address-object name from a domain. The name is a pure function of the domain so re-runs are idempotent. On overflow of the PAN-OS max object-name length, the sanitised body is truncated and a short deterministic hash suffix is appended to keep the name unique. + + Args: + domain (str): The domain to derive the object name from. + Returns: + The derived object name (for example, 'Cortex-evil.example.com'). """ - sanitised = re.sub(r"[^A-Za-z0-9.\-]", "-", domain).strip("-") + sanitised = OBJECT_NAME_SANITIZE_REGEX.sub("-", domain).strip("-") candidate = f"{OBJECT_NAME_PREFIX}{sanitised}" if len(candidate) <= MAX_OBJECT_NAME_LENGTH: return candidate @@ -71,7 +92,13 @@ def derive_object_name(domain: str) -> str: def most_significant_action(actions: list) -> str: - """Return the most significant action from a list (Created > Modified > Unchanged).""" + """Return the most significant action from a list. + + Args: + actions (list): A list of action strings. + Returns: + The most significant action (Created > Modified > Unchanged). + """ if not actions: return ACTION_UNCHANGED return max(actions, key=lambda action: ACTION_SIGNIFICANCE.get(action, 0)) @@ -87,7 +114,20 @@ def build_result_row( instance: str = "", rule_name: str = "", ) -> dict: - """Assemble a single BlockDomainResults row in the canonical field order.""" + """Assemble a single BlockDomainResults row. + + Args: + domain (str): The processed domain. + brand (str): The brand used. + status (str): The lifecycle status. + result (str): Success or Failed. + action (str): Created, Modified, or Unchanged. + message (str): A human-readable message. + instance (str): The integration instance. + rule_name (str): The rule name used (empty if none). + Returns: + A dict representing a single result row. + """ return { "Domain": domain, "Brand": brand, @@ -101,31 +141,36 @@ def build_result_row( def validate_domains(domain_list: list) -> tuple[list, list]: - """Split the input into (valid_domains, skipped_rows). + """Split the input into valid domains and failed-validation rows. - Wildcard and invalid entries never reach a vendor; they produce a per-row Skipped result while - the rest of the list continues. + Wildcard and invalid entries fail validation and never reach a vendor; they produce a per-row + Failed result while the rest of the list continues. + + Args: + domain_list (list): The list of domains to validate. + Returns: + A tuple of (valid_domains, failed_rows). """ valid_domains: list = [] - skipped_rows: list = [] + failed_rows: list = [] for domain in domain_list: if is_wildcard(domain): - skipped_rows.append( + failed_rows.append( build_result_row( domain=domain, brand="", - status=STATUS_SKIPPED, + status=STATUS_FAILED, result=RESULT_FAILED, action=ACTION_UNCHANGED, message=f"Wildcard domain '{domain}' is not supported by this script; skipped.", ) ) elif not is_valid_fqdn(domain): - skipped_rows.append( + failed_rows.append( build_result_row( domain=domain, brand="", - status=STATUS_SKIPPED, + status=STATUS_FAILED, result=RESULT_FAILED, action=ACTION_UNCHANGED, message=f"Invalid FQDN '{domain}' - skipped.", @@ -133,11 +178,15 @@ def validate_domains(domain_list: list) -> tuple[list, list]: ) else: valid_domains.append(domain) - return valid_domains, skipped_rows + return valid_domains, failed_rows def get_enabled_brands() -> set: - """Return the set of brands that have at least one active instance.""" + """Return the set of brands that have at least one active instance. + + Returns: + A set of enabled brand names. + """ modules = demisto.getModules() enabled_brands = {module.get("brand") for module in modules.values() if module.get("state") == "active"} demisto.debug(f"BlockDomain: the enabled modules are: {enabled_brands=}") @@ -148,7 +197,14 @@ def get_enabled_brands() -> set: def run_execute_command(command_name: str, args: dict[str, Any]) -> list[dict]: - """Execute a command and return its raw entries.""" + """Execute a command and return its raw entries. + + Args: + command_name (str): The command to execute. + args (dict): The command arguments. + Returns: + The raw list of command entries. + """ demisto.debug(f"BlockDomain: Executing command: {command_name} with {args=}") res = demisto.executeCommand(command_name, args) demisto.debug(f"BlockDomain: The response of {command_name} is {res}") @@ -156,7 +212,14 @@ def run_execute_command(command_name: str, args: dict[str, Any]) -> list[dict]: def get_relevant_context(original_context: dict[str, Any], key: str) -> dict | list: - """Get the relevant context object from the execute_command response, tolerating suffixed keys.""" + """Get the relevant context object from the execute_command response, tolerating suffixed keys. + + Args: + original_context (dict): The 'EntryContext' from the command response. + key (str): The key to extract. + Returns: + A dict or list that is the relevant command context. + """ if not original_context: return {} if relevant_context := original_context.get(key, {}): @@ -167,135 +230,6 @@ def get_relevant_context(original_context: dict[str, Any], key: str) -> dict | l return {} -def is_error_entry(entry: dict) -> bool: - """Return True if an execute_command entry is an error entry.""" - return isinstance(entry, dict) and entry.get("Type") == entryTypes["error"] - - -def get_entry_error(entry: dict) -> str: - """Extract a human-readable error message from an error entry.""" - return str(entry.get("Contents", "Unknown error")) - - -def as_list(value: Any) -> list: - """Normalise a context value that may be a dict, list, or None into a list.""" - if value is None: - return [] - return value if isinstance(value, list) else [value] - - -""" POLLING FUNCTIONS (commit / push) """ - - -@polling_function(name="block-domain", interval=60, timeout=1200) -def pan_os_commit(args: dict, responses: list) -> PollResult: - """Execute pan-os-commit and start polling on the returned job.""" - res_commit = run_execute_command("pan-os-commit", {"polling": True}) - polling_args = res_commit[0].get("Metadata", {}).get("pollingArgs", {}) - job_id = polling_args.get("commit_job_id") - if job_id: - context_output = {"JobID": job_id, "Status": "Pending"} - continue_to_poll = True - commit_output: Any = CommandResults( - outputs=context_output, readable_output=tableToMarkdown("Commit Status:", context_output, removeNull=True) - ) - demisto.setContext("commit_job_id", job_id) - else: - commit_output = res_commit[0].get("Contents") or "There are no changes to commit." - continue_to_poll = False - global POLLING - POLLING = continue_to_poll - - args_for_next_run = args | { - "commit_job_id": job_id, - "interval_in_seconds": arg_to_number(args.get("interval_in_seconds", 60)), - "timeout": arg_to_number(args.get("timeout", 1200)), - "polling": True, - } - responses.append(res_commit) - return PollResult( - response=commit_output, - continue_to_poll=continue_to_poll, - args_for_next_run=args_for_next_run, - partial_result=CommandResults(readable_output=f"Waiting for commit job ID {job_id} to finish..."), - ) - - -@polling_function(name="block-domain", interval=60, timeout=1200) -def pan_os_commit_status(args: dict, responses: list) -> PollResult: - """Check the status of the commit job in pan-os.""" - commit_job_id = args["commit_job_id"] - res_commit_status = run_execute_command("pan-os-commit-status", {"job_id": commit_job_id}) - responses.append(res_commit_status) - result_commit_status = res_commit_status[0].get("Contents", {}).get("response", {}).get("result", {}).get("job", {}) - job_result = result_commit_status.get("result") - commit_output = {"JobID": commit_job_id, "Status": "Success" if job_result == "OK" else "Failure"} - continue_to_poll = result_commit_status.get("status") != "FIN" - global POLLING - POLLING = continue_to_poll - return PollResult( - response=CommandResults( - outputs=commit_output, - outputs_key_field="JobID", - readable_output=tableToMarkdown("Commit Status:", commit_output, removeNull=True), - ), - args_for_next_run=args, - continue_to_poll=continue_to_poll, - ) - - -@polling_function(name="block-domain", interval=60, timeout=1200) -def pan_os_push_to_device(args: dict, responses: list) -> PollResult: - """Execute pan-os-push-to-device-group and start polling on the returned job.""" - res_push_to_device = run_execute_command("pan-os-push-to-device-group", {"polling": True}) - responses.append(res_push_to_device) - polling_args = res_push_to_device[0].get("Metadata", {}).get("pollingArgs", {}) - job_id = polling_args.get("push_job_id") - device_group = polling_args.get("device-group") - if job_id: - context_output = {"DeviceGroup": device_group, "JobID": job_id, "Status": "Pending"} - continue_to_poll = True - push_cr = CommandResults( - outputs_key_field="JobID", - outputs=context_output, - readable_output=tableToMarkdown("Push to Device Group:", context_output, removeNull=True), - ) - demisto.setContext("push_job_id", job_id) - else: - push_cr = CommandResults(readable_output=res_push_to_device[0].get("Contents") or "There are no changes to push.") - continue_to_poll = False - global POLLING - POLLING = continue_to_poll - return PollResult( - response=push_cr, - continue_to_poll=continue_to_poll, - partial_result=CommandResults(readable_output=f"Waiting for Job-ID {job_id} to finish pushing the changes..."), - ) - - -@polling_function(name="block-domain", interval=60, timeout=1200) -def pan_os_push_status(args: dict, responses: list) -> PollResult: - """Check the status of the push job in pan-os.""" - push_job_id = args["push_job_id"] - res_push_status = run_execute_command("pan-os-push-status", {"job_id": push_job_id}) - responses.append(res_push_status) - push_status = res_push_status[0].get("Contents", {}).get("response", {}).get("result", {}).get("job", {}).get("status", "") - continue_to_poll = bool(push_status and push_status != "FIN") - context_output = {"Status": push_status, "JobID": push_job_id} - push_cr = CommandResults( - outputs_key_field="JobID", - outputs=context_output, - readable_output=tableToMarkdown("Push to Device Group:", context_output, ["JobID", "Status"], removeNull=True), - ) - global POLLING - POLLING = continue_to_poll - return PollResult( - response=push_cr, - continue_to_poll=continue_to_poll, - partial_result=CommandResults(readable_output=f"Waiting for Job-ID {push_job_id} to finish pushing the changes..."), - ) - - """ PAN-OS FLOW """ @@ -306,13 +240,19 @@ class DynamicGroupError(Exception): class PanOs: """Implements the PAN-OS static-address-group domain-blocking flow. - For each valid domain the flow probes/creates an FQDN address-object, ensures it belongs to the - static address-group, and ensures a single deny rule points at the group. Commit + optional push - happen once after all domains are processed. Every step records its effect (Created / Modified / - Unchanged) so the aggregated per-domain row reflects the most significant change. + The address-group and the deny rule are singletons (their names are constant), so they are + ensured once per run. Each valid domain then gets an FQDN address-object that is added to the + group. Commit + optional push happen once after all domains are processed. Every write records + its effect (Created / Modified / Unchanged) so the aggregated per-domain row reflects the most + significant change. """ def __init__(self, args: dict): + """Initialize the PanOs flow. + + Args: + args (dict): The flow arguments (domains, rule_name, address_group, tag, etc.). + """ self.args = args self.brand = "Panorama" self.rule_name = args["rule_name"] @@ -321,85 +261,91 @@ def __init__(self, args: dict): self.log_forwarding_name = args.get("log_forwarding_name", "") self.domains: list = args.get("domains", []) self.responses: list = [] - # Per-domain accumulated actions and messages, keyed by domain. - self.domain_actions: dict = {domain: [] for domain in self.domains} - self.domain_messages: dict = {domain: [] for domain in self.domains} + + # ---- execution helper ---------------------------------------------- + + def execute_or_raise(self, command_name: str, command_args: dict, error_prefix: str) -> list[dict]: + """Run a command, record its response, and raise on error. + + Args: + command_name (str): The command to execute. + command_args (dict): The command arguments. + error_prefix (str): A prefix for the raised error message. + Returns: + The raw command entries. + """ + res = run_execute_command(command_name, command_args) + self.responses.append(res) + if is_error(res): + raise DemistoException(f"{error_prefix}: {get_error(res)}") + return res # ---- context probes ------------------------------------------------- def address_object_exists(self, object_name: str) -> bool: - """Return True if an address-object with this name already exists.""" + """Check whether an address-object already exists. + + Args: + object_name (str): The address-object name to probe. + Returns: + True if the object exists, False otherwise. + """ res = run_execute_command("pan-os-get-address", {"name": object_name}) - entry = res[0] if res else {} - if is_error_entry(entry): + if is_error(res): # get-address raises when the object is absent; treat that as 'does not exist'. - demisto.debug(f"BlockDomain: address '{object_name}' not found ({get_entry_error(entry)}).") + demisto.debug(f"BlockDomain: address '{object_name}' not found ({get_error(res)}).") return False self.responses.append(res) - context = get_relevant_context(entry.get("EntryContext", {}), "Panorama.Addresses") - return any(item.get("Name") == object_name for item in as_list(context)) + context = get_relevant_context(res[0].get("EntryContext", {}), "Panorama.Addresses") + items = context if isinstance(context, list) else [context] + return any(item.get("Name") == object_name for item in items) def get_address_group(self) -> dict | None: - """Return the target address-group context dict, or None if it does not exist.""" - res = run_execute_command("pan-os-list-address-groups", {}) - self.responses.append(res) - entry = res[0] if res else {} - if is_error_entry(entry): - raise DemistoException(f"Failed to list address groups: {get_entry_error(entry)}") - context = get_relevant_context(entry.get("EntryContext", {}), "Panorama.AddressGroups") - for item in as_list(context): + """Return the target address-group context dict, or None if it does not exist. + + Returns: + The address-group context dict, or None. + """ + res = self.execute_or_raise("pan-os-list-address-groups", {}, "Failed to list address groups") + context = get_relevant_context(res[0].get("EntryContext", {}), "Panorama.AddressGroups") + items = context if isinstance(context, list) else [context] + for item in items: if item.get("Name") == self.address_group: return item return None - def group_members(self, group_context: dict) -> list: - """Return the current static-group member names.""" - return as_list(group_context.get("Addresses")) + def rule_destinations(self) -> tuple[bool, list]: + """Return whether the rule exists and its current destination list. - def rule_exists(self) -> bool: - """Return True if a rule named self.rule_name already exists in the rulebase.""" - res = run_execute_command("pan-os-list-rules", {"pre_post": PRE_POST}) - self.responses.append(res) - entry = res[0] if res else {} - if is_error_entry(entry): - raise DemistoException(f"Failed to list rules: {get_entry_error(entry)}") - context = get_relevant_context(entry.get("EntryContext", {}), "Panorama.SecurityRule") - return any(item.get("Name") == self.rule_name for item in as_list(context)) - - # ---- writes --------------------------------------------------------- - - def ensure_address_object(self, domain: str, object_name: str) -> None: - """Create the FQDN address-object if it does not already exist.""" - if self.address_object_exists(object_name): - self.domain_actions[domain].append(ACTION_UNCHANGED) - self.domain_messages[domain].append(f"Address-object '{object_name}' already exists.") - return - create_args: dict = {"name": object_name, "fqdn": domain} - if self.tag: - create_args["tag"] = self.tag - res = run_execute_command("pan-os-create-address", create_args) - self.responses.append(res) - entry = res[0] if res else {} - if is_error_entry(entry): - raise DemistoException(f"Failed to create address-object '{object_name}': {get_entry_error(entry)}") - self.domain_actions[domain].append(ACTION_CREATED) - self.domain_messages[domain].append(f"Address-object '{object_name}' created for '{domain}'.") - - def ensure_group_membership(self, domain: str, object_name: str, group_context: dict | None) -> None: - """Create the static group or add the object to it, aborting if the group is dynamic.""" + Returns: + A tuple of (rule_exists, destination_list). + """ + res = self.execute_or_raise("pan-os-list-rules", {"pre_post": PRE_POST}, "Failed to list rules") + context = get_relevant_context(res[0].get("EntryContext", {}), "Panorama.SecurityRule") + items = context if isinstance(context, list) else [context] + for item in items: + if item.get("Name") == self.rule_name: + destination = item.get("Destination") + destination_list = destination if isinstance(destination, list) else [destination] if destination else [] + return True, destination_list + return False, [] + + # ---- single-run writes (group + rule are singletons) ---------------- + + def ensure_group(self, group_context: dict | None) -> None: + """Ensure the static address-group exists, aborting if it is dynamic. + + Args: + group_context (dict | None): The existing group context, or None if missing. + """ if group_context is None: - create_args: dict = {"name": self.address_group, "type": "static", "addresses": [object_name]} + create_args: dict = {"name": self.address_group, "type": "static"} if self.tag: create_args["tags"] = self.tag - res = run_execute_command("pan-os-create-address-group", create_args) - self.responses.append(res) - entry = res[0] if res else {} - if is_error_entry(entry): - raise DemistoException(f"Failed to create address-group '{self.address_group}': {get_entry_error(entry)}") - self.domain_actions[domain].append(ACTION_CREATED) - self.domain_messages[domain].append(f"Static address-group '{self.address_group}' created.") + self.execute_or_raise( + "pan-os-create-address-group", create_args, f"Failed to create address-group '{self.address_group}'" + ) return - group_type = (group_context.get("Type") or "").lower() if group_type == "dynamic": raise DynamicGroupError( @@ -407,26 +353,13 @@ def ensure_group_membership(self, domain: str, object_name: str, group_context: f"will not modify a customer-managed dynamic group." ) - if object_name in self.group_members(group_context): - self.domain_actions[domain].append(ACTION_UNCHANGED) - self.domain_messages[domain].append( - f"Address-object '{object_name}' is already a member of '{self.address_group}'." - ) - return + def ensure_rule(self, rule_present: bool, rule_destinations: list) -> None: + """Ensure the deny rule exists, points at the group, and sits at the top. - res = run_execute_command( - "pan-os-edit-address-group", - {"name": self.address_group, "type": "static", "element_to_add": object_name}, - ) - self.responses.append(res) - entry = res[0] if res else {} - if is_error_entry(entry): - raise DemistoException(f"Failed to add object to address-group '{self.address_group}': {get_entry_error(entry)}") - self.domain_actions[domain].append(ACTION_MODIFIED) - self.domain_messages[domain].append(f"Address-object '{object_name}' added to '{self.address_group}'.") - - def ensure_rule(self, rule_present: bool) -> str: - """Create the deny rule if missing, then move it to the top. Returns the rule action.""" + Args: + rule_present (bool): Whether the rule already exists. + rule_destinations (list): The rule's current destination list. + """ if not rule_present: create_rule_args: dict = { "rulename": self.rule_name, @@ -442,57 +375,97 @@ def ensure_rule(self, rule_present: bool) -> str: create_rule_args["tags"] = self.tag if self.log_forwarding_name: create_rule_args["log_forwarding"] = self.log_forwarding_name - res = run_execute_command("pan-os-create-rule", create_rule_args) - self.responses.append(res) - entry = res[0] if res else {} - if is_error_entry(entry): - raise DemistoException(f"Failed to create rule '{self.rule_name}': {get_entry_error(entry)}") - rule_action = ACTION_CREATED + self.execute_or_raise("pan-os-create-rule", create_rule_args, f"Failed to create rule '{self.rule_name}'") + elif self.address_group not in rule_destinations: + # The rule exists but does not yet reference our group - add it without replacing existing destinations. + self.execute_or_raise( + "pan-os-edit-rule", + { + "rulename": self.rule_name, + "element_to_change": "destination", + "element_value": self.address_group, + "behaviour": "add", + "pre_post": PRE_POST, + }, + f"Failed to add group to rule '{self.rule_name}'", + ) + # Always ensure the rule sits at the top of the rulebase. + self.execute_or_raise( + "pan-os-move-rule", + {"rulename": self.rule_name, "where": "top", "pre_post": PRE_POST}, + f"Failed to move rule '{self.rule_name}' to top", + ) + + def ensure_domain(self, domain: str, current_members: list) -> tuple[str, str]: + """Ensure a single domain's address-object exists and belongs to the group. + + Args: + domain (str): The domain to block. + current_members (list): The group's current member names. + Returns: + A tuple of (action, message) describing the effect for this domain. + """ + object_name = derive_object_name(domain) + actions: list = [] + messages: list = [] + + if self.address_object_exists(object_name): + actions.append(ACTION_UNCHANGED) + messages.append(f"Address-object '{object_name}' already exists.") + else: + create_args: dict = {"name": object_name, "fqdn": domain} + if self.tag: + create_args["tag"] = self.tag + self.execute_or_raise("pan-os-create-address", create_args, f"Failed to create address-object '{object_name}'") + actions.append(ACTION_CREATED) + messages.append(f"Address-object '{object_name}' created for '{domain}'.") + + if object_name in current_members: + actions.append(ACTION_UNCHANGED) + messages.append(f"Already a member of '{self.address_group}'.") else: - rule_action = ACTION_UNCHANGED + self.execute_or_raise( + "pan-os-edit-address-group", + {"name": self.address_group, "type": "static", "element_to_add": object_name}, + f"Failed to add object to address-group '{self.address_group}'", + ) + current_members.append(object_name) + actions.append(ACTION_MODIFIED) + messages.append(f"Added to '{self.address_group}'.") - # Always ensure the rule sits at the top of the rulebase. - res_move = run_execute_command("pan-os-move-rule", {"rulename": self.rule_name, "where": "top", "pre_post": PRE_POST}) - self.responses.append(res_move) - move_entry = res_move[0] if res_move else {} - if is_error_entry(move_entry): - raise DemistoException(f"Failed to move rule '{self.rule_name}' to top: {get_entry_error(move_entry)}") - return rule_action + return most_significant_action(actions), " ".join(messages) # ---- orchestration -------------------------------------------------- def process_domains(self) -> list: - """Run the per-domain object + group flow, then the single shared rule step. + """Ensure the group and rule once, then loop over domains adding each object. - Returns the list of BlockDomainResults rows for the processed domains. + Returns: + The list of BlockDomainResults rows for the processed domains. """ rows: list = [] try: group_context = self.get_address_group() - for domain in self.domains: - object_name = derive_object_name(domain) - self.ensure_address_object(domain, object_name) - self.ensure_group_membership(domain, object_name, group_context) - # Re-read the group once created so subsequent domains see the new membership. - if group_context is None: - group_context = self.get_address_group() + self.ensure_group(group_context) + current_members = [] + if group_context is not None: + members = group_context.get("Addresses") + current_members = list(members) if isinstance(members, list) else [members] if members else [] - rule_present = self.rule_exists() - rule_action = self.ensure_rule(rule_present) + rule_present, rule_destinations = self.rule_destinations() + self.ensure_rule(rule_present, rule_destinations) for domain in self.domains: - actions = self.domain_actions[domain] + [rule_action] + action, message = self.ensure_domain(domain, current_members) rows.append( build_result_row( domain=domain, brand=self.brand, status=STATUS_DONE, result=RESULT_SUCCESS, - action=most_significant_action(actions), + action=action, rule_name=self.rule_name, - message=" ".join(self.domain_messages[domain]) - + (f" Rule '{self.rule_name}' created at top." if rule_action == ACTION_CREATED else "") - + (f" Rule '{self.rule_name}' already present; moved to top." if rule_action == ACTION_UNCHANGED else ""), + message=f"{message} Rule '{self.rule_name}' enforced at top.", ) ) except DynamicGroupError as dyn_err: @@ -524,39 +497,246 @@ def process_domains(self) -> list: ) return rows - def run(self) -> list: # pragma: no cover - """Execute the full PAN-OS flow: per-domain writes, then commit + optional push.""" - rows = self.process_domains() - - # Only commit/push if at least one domain actually reached Done (i.e. no dynamic-group abort / failure). - made_changes = any(row["Status"] == STATUS_DONE for row in rows) - auto_commit = argToBoolean(self.args.get("auto_commit", True)) - if made_changes and auto_commit: - try: - pan_os_commit(self.args, self.responses) - if self.pan_os_is_panorama(): - pan_os_push_to_device(self.args, self.responses) - except Exception as ex: - rows.append( - build_result_row( - domain="", - brand=self.brand, - status=STATUS_FAILED, - result=RESULT_FAILED, - action=ACTION_UNCHANGED, - message=f"Commit/push failed: {ex!s}. Objects and rule are staged but may not be active.", - ) - ) - return rows - def pan_os_is_panorama(self) -> bool: - """Return True if the instance is a Panorama (vs a single firewall).""" + """Check whether the instance is a Panorama (vs a single firewall). + + Returns: + True if the instance model is 'Panorama', False otherwise. + """ res = run_execute_command("pan-os", {"cmd": "", "type": "op"}) self.responses.append(res) context = get_relevant_context(res[0].get("EntryContext", {}), "Panorama.Command") model = context.get("response", {}).get("result", {}).get("system", {}).get("model", "") # type: ignore return model == "Panorama" + def reduce_responses(self) -> list: + """Reduce the accumulated responses to the parts needed across polling cycles. + + Returns: + A list of reduced response entries suitable for serialization to context. + """ + reduced = [] + for res in self.responses: + reduced.append( + [ + { + "HumanReadable": entry.get("HumanReadable"), + "Contents": entry.get("Contents"), + "Type": entry.get("Type"), + "Metadata": entry.get("Metadata"), + } + for entry in res + ] + ) + return reduced + + def manage_pan_os_flow(self) -> Any: # pragma: no cover + """Manage the PAN-OS flow across polling cycles. + + On re-entry (a push or commit job is in flight) the flow jumps straight to the relevant + status poller. Otherwise it runs the object/group/rule flow and starts the commit. + + Returns: + A PollResult when a job is in flight, or the list of result rows when finished. + """ + incident_context = demisto.context() + commit_job_id = self.args.get("commit_job_id") or demisto.get(incident_context, "commit_job_id") + + # State: a push job is in flight -> poll its status. + if push_job_id := demisto.get(incident_context, "push_job_id"): + self.responses = ast.literal_eval(incident_context.get("panorama_responses", "[]") or "[]") + self.args["push_job_id"] = push_job_id + res_push_status = pan_os_push_status(self.args, self.responses) + if not POLLING: + return self.finish() + demisto.setContext("panorama_responses", str(self.reduce_responses())) + return res_push_status + + # State: a commit job is in flight -> poll its status, then maybe push. + if commit_job_id: + self.args["commit_job_id"] = commit_job_id + self.responses = ast.literal_eval(incident_context.get("panorama_responses", "[]") or "[]") + poll_commit_status = pan_os_commit_status(self.args, self.responses) + if not POLLING: + if self.pan_os_is_panorama(): + poll_push = pan_os_push_to_device(self.args, self.responses) + if not POLLING: + return self.finish() + demisto.setContext("panorama_responses", str(self.reduce_responses())) + return poll_push + return self.finish() + demisto.setContext("panorama_responses", str(self.reduce_responses())) + return poll_commit_status + + # State: beginning of the flow. + rows = self.process_domains() + demisto.setContext("block_domain_rows", str(rows)) + made_changes = any(row["Status"] == STATUS_DONE for row in rows) + auto_commit = argToBoolean(self.args.get("auto_commit", True)) + if made_changes and auto_commit: + poll_commit = pan_os_commit(self.args, self.responses) + if not POLLING: + return self.finish() + demisto.setContext("panorama_responses", str(self.reduce_responses())) + return poll_commit + return rows + + def finish(self) -> list: # pragma: no cover + """Clean up polling context and return the final result rows. + + Returns: + The list of BlockDomainResults rows accumulated for the run. + """ + rows_raw = demisto.context().get("block_domain_rows", "[]") + demisto.setContext("commit_job_id", "") + demisto.setContext("push_job_id", "") + demisto.setContext("panorama_responses", "") + demisto.setContext("block_domain_rows", "") + try: + return ast.literal_eval(rows_raw) if rows_raw else [] + except (ValueError, SyntaxError): + return [] + + +""" POLLING FUNCTIONS (commit / push) """ + + +@polling_function(name="block-domain", interval=60, timeout=1200) +def pan_os_commit(args: dict, responses: list) -> PollResult: + """Execute pan-os-commit. + + Args: + args (dict): The arguments of the function. + responses (list): The responses of the commands executed so far. + Returns: + The PollResult object. + """ + res_commit = run_execute_command("pan-os-commit", {"polling": True}) + polling_args = res_commit[0].get("Metadata", {}).get("pollingArgs", {}) + job_id = polling_args.get("commit_job_id") + if job_id: + context_output = {"JobID": job_id, "Status": "Pending"} + continue_to_poll = True + commit_output: Any = CommandResults( + outputs=context_output, readable_output=tableToMarkdown("Commit Status:", context_output, removeNull=True) + ) + demisto.setContext("commit_job_id", job_id) + else: + commit_output = res_commit[0].get("Contents") or "There are no changes to commit." + continue_to_poll = False + global POLLING + POLLING = continue_to_poll + + args_for_next_run = args | { + "commit_job_id": job_id, + "interval_in_seconds": arg_to_number(args.get("interval_in_seconds", 60)), + "timeout": arg_to_number(args.get("timeout", 1200)), + "polling": True, + } + responses.append(res_commit) + return PollResult( + response=commit_output, + continue_to_poll=continue_to_poll, + args_for_next_run=args_for_next_run, + partial_result=CommandResults(readable_output=f"Waiting for commit job ID {job_id} to finish..."), + ) + + +@polling_function(name="block-domain", interval=60, timeout=1200) +def pan_os_commit_status(args: dict, responses: list) -> PollResult: + """Check the status of the commit job in pan-os. + + Args: + args (dict): The arguments of the function. + responses (list): The responses of the previous command. + Returns: + The PollResult object. + """ + commit_job_id = args["commit_job_id"] + res_commit_status = run_execute_command("pan-os-commit-status", {"job_id": commit_job_id}) + responses.append(res_commit_status) + result_commit_status = res_commit_status[0].get("Contents", {}).get("response", {}).get("result", {}).get("job", {}) + job_result = result_commit_status.get("result") + commit_output = {"JobID": commit_job_id, "Status": "Success" if job_result == "OK" else "Failure"} + continue_to_poll = result_commit_status.get("status") != "FIN" + global POLLING + POLLING = continue_to_poll + return PollResult( + response=CommandResults( + outputs=commit_output, + outputs_key_field="JobID", + readable_output=tableToMarkdown("Commit Status:", commit_output, removeNull=True), + ), + args_for_next_run=args, + continue_to_poll=continue_to_poll, + ) + + +@polling_function(name="block-domain", interval=60, timeout=1200) +def pan_os_push_to_device(args: dict, responses: list) -> PollResult: + """Execute pan-os-push-to-device-group. + + Args: + args (dict): The arguments of the function. + responses (list): The responses of the previous command. + Returns: + The PollResult object. + """ + res_push_to_device = run_execute_command("pan-os-push-to-device-group", {"polling": True}) + responses.append(res_push_to_device) + polling_args = res_push_to_device[0].get("Metadata", {}).get("pollingArgs", {}) + job_id = polling_args.get("push_job_id") + device_group = polling_args.get("device-group") + if job_id: + context_output = {"DeviceGroup": device_group, "JobID": job_id, "Status": "Pending"} + continue_to_poll = True + push_cr = CommandResults( + outputs_key_field="JobID", + outputs=context_output, + readable_output=tableToMarkdown("Push to Device Group:", context_output, removeNull=True), + ) + demisto.setContext("push_job_id", job_id) + else: + push_cr = CommandResults(readable_output=res_push_to_device[0].get("Contents") or "There are no changes to push.") + continue_to_poll = False + global POLLING + POLLING = continue_to_poll + return PollResult( + response=push_cr, + continue_to_poll=continue_to_poll, + partial_result=CommandResults(readable_output=f"Waiting for Job-ID {job_id} to finish pushing the changes..."), + ) + + +@polling_function(name="block-domain", interval=60, timeout=1200) +def pan_os_push_status(args: dict, responses: list) -> PollResult: + """Check the status of the push job in pan-os. + + Args: + args (dict): The arguments of the function. + responses (list): The responses of the previous command. + Returns: + The PollResult object. + """ + push_job_id = args["push_job_id"] + res_push_status = run_execute_command("pan-os-push-status", {"job_id": push_job_id}) + responses.append(res_push_status) + push_status = res_push_status[0].get("Contents", {}).get("response", {}).get("result", {}).get("job", {}).get("status", "") + continue_to_poll = bool(push_status and push_status != "FIN") + context_output = {"Status": push_status, "JobID": push_job_id} + push_cr = CommandResults( + outputs_key_field="JobID", + outputs=context_output, + readable_output=tableToMarkdown("Push to Device Group:", context_output, ["JobID", "Status"], removeNull=True), + ) + global POLLING + POLLING = continue_to_poll + return PollResult( + response=push_cr, + continue_to_poll=continue_to_poll, + partial_result=CommandResults(readable_output=f"Waiting for Job-ID {push_job_id} to finish pushing the changes..."), + ) + """ MAIN FUNCTION """ @@ -576,8 +756,11 @@ def main(): # pragma: no cover brands_to_run = argToList(args.get("brands", ",".join(SUPPORTED_BRANDS))) demisto.debug(f"BlockDomain: {verbose=}, {brands_to_run=}") - valid_domains, skipped_rows = validate_domains(domain_list) - demisto.debug(f"BlockDomain: {valid_domains=}, skipped {len(skipped_rows)} entries.") + if not domain_list: + return_error("domain_list argument is required.") + + valid_domains, failed_rows = validate_domains(domain_list) + demisto.debug(f"BlockDomain: {valid_domains=}, {len(failed_rows)} entries failed validation.") enabled_brands = get_enabled_brands() brands_to_run = brands_to_run or list(SUPPORTED_BRANDS) @@ -589,7 +772,7 @@ def main(): # pragma: no cover f"Please verify the brand instances' setup. Supported brands: {SUPPORTED_BRANDS}." ) - results: list = list(skipped_rows) + results: list = list(failed_rows) for brand in brands_to_run: if brand not in SUPPORTED_BRANDS: @@ -597,7 +780,7 @@ def main(): # pragma: no cover build_result_row( domain="", brand=brand, - status=STATUS_SKIPPED, + status=STATUS_FAILED, result=RESULT_FAILED, action=ACTION_UNCHANGED, message=f"The brand {brand} is not supported by 'block-domain'. Supported: {SUPPORTED_BRANDS}.", @@ -608,7 +791,7 @@ def main(): # pragma: no cover build_result_row( domain="", brand=brand, - status=STATUS_SKIPPED, + status=STATUS_FAILED, result=RESULT_FAILED, action=ACTION_UNCHANGED, message=f"The brand {brand} isn't enabled.", @@ -629,7 +812,11 @@ def main(): # pragma: no cover "polling": True, } ) - results.extend(pan_os.run()) + pan_os_result = pan_os.manage_pan_os_flow() + if isinstance(pan_os_result, PollResult): + return_results(pan_os_result) + return + results.extend(pan_os_result) return_results( CommandResults( diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py index eee4fdcbae79..2eccc38a7456 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py @@ -82,20 +82,21 @@ def test_derive_object_name_overflow_truncates_and_hashes(): assert name == derive_object_name(long_domain) -def test_validate_domains_splits_valid_and_skipped(): - valid, skipped = validate_domains(["evil.example.com", "*.evil.com", "no-dot", "phish.attacker.net"]) +def test_validate_domains_splits_valid_and_failed(): + valid, failed = validate_domains(["evil.example.com", "*.evil.com", "no-dot", "phish.attacker.net"]) assert valid == ["evil.example.com", "phish.attacker.net"] - assert len(skipped) == 2 + assert len(failed) == 2 - wildcard_row = next(row for row in skipped if row["Domain"] == "*.evil.com") - assert wildcard_row["Status"] == STATUS_SKIPPED - assert wildcard_row["Result"] == RESULT_SUCCESS + wildcard_row = next(row for row in failed if row["Domain"] == "*.evil.com") + assert wildcard_row["Status"] == STATUS_FAILED + assert wildcard_row["Result"] == RESULT_FAILED assert wildcard_row["Action"] == ACTION_UNCHANGED assert "Wildcard" in wildcard_row["Message"] - invalid_row = next(row for row in skipped if row["Domain"] == "no-dot") - assert invalid_row["Status"] == STATUS_SKIPPED + invalid_row = next(row for row in failed if row["Domain"] == "no-dot") + assert invalid_row["Status"] == STATUS_FAILED + assert invalid_row["Result"] == RESULT_FAILED assert "Invalid FQDN" in invalid_row["Message"] @@ -141,18 +142,18 @@ def _mock_execute(monkeypatch, side_effect): def test_process_domains_create_everything(monkeypatch): - # Group missing -> create group; address missing -> create address; rule missing -> create + move. + # Group missing -> create group; rule missing -> create + move; address missing -> create + add. _mock_execute( monkeypatch, [ [ok_entry({"Panorama.AddressGroups": []})], # list-address-groups (missing) - [err_entry("not found")], # get-address (missing) - [ok_entry({"Panorama.Addresses": [{"Name": "Cortex-evil.example.com"}]})], # create-address - [ok_entry()], # create-address-group - [ok_entry({"Panorama.AddressGroups": [{"Name": "Blocked Domains - Cortex", "Type": "static"}]})], # re-list group + [ok_entry()], # create-address-group (static, empty) [ok_entry({"Panorama.SecurityRule": []})], # list-rules (missing) [ok_entry()], # create-rule [ok_entry()], # move-rule + [err_entry("not found")], # get-address (missing) + [ok_entry()], # create-address + [ok_entry()], # edit-address-group (add member) ], ) rows = _pan_os(["evil.example.com"]).process_domains() @@ -169,9 +170,9 @@ def test_process_domains_all_unchanged(monkeypatch): monkeypatch, [ [ok_entry({"Panorama.AddressGroups": [{"Name": "Blocked Domains - Cortex", "Type": "static", "Addresses": [obj]}]})], - [ok_entry({"Panorama.Addresses": [{"Name": obj}]})], # get-address (exists) - [ok_entry({"Panorama.SecurityRule": [{"Name": "Cortex - Block Domain"}]})], # list-rules (exists) + [ok_entry({"Panorama.SecurityRule": [{"Name": "Cortex - Block Domain", "Destination": ["Blocked Domains - Cortex"]}]})], [ok_entry()], # move-rule + [ok_entry({"Panorama.Addresses": [{"Name": obj}]})], # get-address (exists) ], ) rows = _pan_os(["evil.example.com"]).process_domains() @@ -184,11 +185,11 @@ def test_process_domains_modified_when_added_to_existing_group(monkeypatch): monkeypatch, [ [ok_entry({"Panorama.AddressGroups": [{"Name": "Blocked Domains - Cortex", "Type": "static", "Addresses": []}]})], + [ok_entry({"Panorama.SecurityRule": [{"Name": "Cortex - Block Domain", "Destination": ["Blocked Domains - Cortex"]}]})], + [ok_entry()], # move-rule [err_entry("not found")], # get-address (missing) [ok_entry()], # create-address [ok_entry()], # edit-address-group (add member) - [ok_entry({"Panorama.SecurityRule": [{"Name": "Cortex - Block Domain"}]})], # list-rules (exists) - [ok_entry()], # move-rule ], ) rows = _pan_os(["evil.example.com"]).process_domains() @@ -196,13 +197,29 @@ def test_process_domains_modified_when_added_to_existing_group(monkeypatch): assert rows[0]["Action"] == ACTION_CREATED # object was created -> most significant +def test_process_domains_existing_rule_missing_group_is_edited(monkeypatch): + obj = "Cortex-evil.example.com" + _mock_execute( + monkeypatch, + [ + [ok_entry({"Panorama.AddressGroups": [{"Name": "Blocked Domains - Cortex", "Type": "static", "Addresses": [obj]}]})], + # Rule exists but its destination does not reference our group -> triggers edit-rule. + [ok_entry({"Panorama.SecurityRule": [{"Name": "Cortex - Block Domain", "Destination": ["something-else"]}]})], + [ok_entry()], # edit-rule (add destination) + [ok_entry()], # move-rule + [ok_entry({"Panorama.Addresses": [{"Name": obj}]})], # get-address (exists) + ], + ) + rows = _pan_os(["evil.example.com"]).process_domains() + assert rows[0]["Status"] == STATUS_DONE + assert rows[0]["Action"] == ACTION_UNCHANGED # object + membership unchanged; rule edit is a group-level fix + + def test_process_domains_dynamic_group_is_skipped(monkeypatch): _mock_execute( monkeypatch, [ [ok_entry({"Panorama.AddressGroups": [{"Name": "Blocked Domains - Cortex", "Type": "dynamic", "Match": "x"}]})], - [err_entry("not found")], # get-address (missing) - [ok_entry()], # create-address ], ) rows = _pan_os(["evil.example.com"]).process_domains() @@ -216,6 +233,10 @@ def test_process_domains_failure_marks_row_failed(monkeypatch): monkeypatch, [ [ok_entry({"Panorama.AddressGroups": []})], # list-address-groups + [ok_entry()], # create-address-group + [ok_entry({"Panorama.SecurityRule": []})], # list-rules + [ok_entry()], # create-rule + [ok_entry()], # move-rule [err_entry("not found")], # get-address (missing) [err_entry("permission denied")], # create-address fails ], From b5bddf1603b1d44c99dc176154601f57b8e2a4dd Mon Sep 17 00:00:00 2001 From: nbensalm-palo Date: Sun, 16 Aug 2026 16:24:04 +0300 Subject: [PATCH 03/20] re factoring code --- .../Scripts/BlockDomain/BlockDomain.py | 87 +++++++++++++------ 1 file changed, 60 insertions(+), 27 deletions(-) diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py index b2043cd60dd6..01c95005b547 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py @@ -530,45 +530,78 @@ def reduce_responses(self) -> list: ) return reduced + def restore_responses(self) -> None: + """Restore the accumulated responses that were serialized to context in a previous cycle.""" + self.responses = ast.literal_eval(demisto.context().get("panorama_responses", "[]") or "[]") + + def save_responses(self) -> None: + """Serialize the accumulated responses to context for the next polling cycle.""" + demisto.setContext("panorama_responses", str(self.reduce_responses())) + def manage_pan_os_flow(self) -> Any: # pragma: no cover - """Manage the PAN-OS flow across polling cycles. + """Dispatch the PAN-OS flow to the correct state. On re-entry (a push or commit job is in flight) the flow jumps straight to the relevant - status poller. Otherwise it runs the object/group/rule flow and starts the commit. + status poller. Otherwise it starts the object/group/rule flow. Returns: A PollResult when a job is in flight, or the list of result rows when finished. """ incident_context = demisto.context() commit_job_id = self.args.get("commit_job_id") or demisto.get(incident_context, "commit_job_id") + push_job_id = demisto.get(incident_context, "push_job_id") - # State: a push job is in flight -> poll its status. - if push_job_id := demisto.get(incident_context, "push_job_id"): - self.responses = ast.literal_eval(incident_context.get("panorama_responses", "[]") or "[]") - self.args["push_job_id"] = push_job_id - res_push_status = pan_os_push_status(self.args, self.responses) - if not POLLING: - return self.finish() - demisto.setContext("panorama_responses", str(self.reduce_responses())) - return res_push_status - - # State: a commit job is in flight -> poll its status, then maybe push. + if push_job_id: + return self.handle_push_in_flight(push_job_id) if commit_job_id: - self.args["commit_job_id"] = commit_job_id - self.responses = ast.literal_eval(incident_context.get("panorama_responses", "[]") or "[]") - poll_commit_status = pan_os_commit_status(self.args, self.responses) + return self.handle_commit_in_flight(commit_job_id) + return self.start_flow() + + def handle_push_in_flight(self, push_job_id: str) -> Any: # pragma: no cover + """Poll the status of an in-flight push-to-device-group job. + + Args: + push_job_id (str): The push job ID to poll. + Returns: + A PollResult while the push is running, or the final result rows when it finishes. + """ + self.restore_responses() + self.args["push_job_id"] = push_job_id + res_push_status = pan_os_push_status(self.args, self.responses) + if not POLLING: + return self.finish() + self.save_responses() + return res_push_status + + def handle_commit_in_flight(self, commit_job_id: str) -> Any: # pragma: no cover + """Poll the status of an in-flight commit job, then start the push if needed. + + Args: + commit_job_id (str): The commit job ID to poll. + Returns: + A PollResult while commit/push is running, or the final result rows when finished. + """ + self.args["commit_job_id"] = commit_job_id + self.restore_responses() + poll_commit_status = pan_os_commit_status(self.args, self.responses) + if POLLING: + self.save_responses() + return poll_commit_status + # Commit finished - push to the device group if this is a Panorama instance. + if self.pan_os_is_panorama(): + poll_push = pan_os_push_to_device(self.args, self.responses) if not POLLING: - if self.pan_os_is_panorama(): - poll_push = pan_os_push_to_device(self.args, self.responses) - if not POLLING: - return self.finish() - demisto.setContext("panorama_responses", str(self.reduce_responses())) - return poll_push return self.finish() - demisto.setContext("panorama_responses", str(self.reduce_responses())) - return poll_commit_status + self.save_responses() + return poll_push + return self.finish() + + def start_flow(self) -> Any: # pragma: no cover + """Run the object/group/rule flow, then start the commit if there were changes. - # State: beginning of the flow. + Returns: + A PollResult while the commit is running, or the final result rows when finished. + """ rows = self.process_domains() demisto.setContext("block_domain_rows", str(rows)) made_changes = any(row["Status"] == STATUS_DONE for row in rows) @@ -577,7 +610,7 @@ def manage_pan_os_flow(self) -> Any: # pragma: no cover poll_commit = pan_os_commit(self.args, self.responses) if not POLLING: return self.finish() - demisto.setContext("panorama_responses", str(self.reduce_responses())) + self.save_responses() return poll_commit return rows @@ -821,7 +854,7 @@ def main(): # pragma: no cover return_results( CommandResults( outputs_prefix="BlockDomainResults", - outputs_key_field=["Domain", "Brand"], + outputs_key_field=["Domain", "Brand", "Instance"], outputs=results, readable_output=tableToMarkdown( "Block Domain", From c1e626013d779d836fdb98c0d7aa3e932e721a74 Mon Sep 17 00:00:00 2001 From: nbensalm-palo Date: Sun, 16 Aug 2026 17:19:26 +0300 Subject: [PATCH 04/20] updating HR to support verbose --- .../Scripts/BlockDomain/BlockDomain.py | 79 ++++++++++++++++--- .../Scripts/BlockDomain/BlockDomain_test.py | 41 ++++++++++ 2 files changed, 107 insertions(+), 13 deletions(-) diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py index 01c95005b547..924cd21c7682 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py @@ -230,6 +230,58 @@ def get_relevant_context(original_context: dict[str, Any], key: str) -> dict | l return {} +""" HUMAN-READABLE / FINAL-RESULT AGGREGATION """ + + +def build_verbose_human_readable(responses: list) -> str: + """Concatenate the per-command human-readable outputs into a single, blank-line-separated string. + + Args: + responses (list): The accumulated command responses (each a list of entries). + Returns: + A single markdown string with each command's human-readable output separated by a blank line, + or an empty string if no command produced human-readable output. + """ + human_readables: list = [] + for res in responses or []: + for entry in res or []: + command_hr = entry.get("HumanReadable") + if command_hr and command_hr != str(None): + human_readables.append(command_hr) + # A leading "" yields a blank line separating the summary table from the first verbose entry. + return "\n\n".join(["", *human_readables]) if human_readables else "" + + +def build_final_command_results(rows: list, verbose: bool, responses: list) -> CommandResults: + """Build the single final CommandResults for the run. + + The CommandResults carries the aggregated BlockDomainResults context and a markdown summary table. + When verbose is True, the per-command human-readable outputs are appended to the same readable + output (blank-line separated), mirroring the ExpirePassword aggregated script. + + Args: + rows (list): The aggregated BlockDomainResults rows. + verbose (bool): Whether to append per-command human-readable output. + responses (list): The accumulated command responses (used only when verbose). + Returns: + A single CommandResults to return from the script. + """ + readable_output = tableToMarkdown( + "Block Domain", + rows, + headers=["Domain", "Brand", "Instance", "Status", "Result", "Action", "RuleName", "Message"], + removeNull=False, + ) + if verbose: + readable_output += build_verbose_human_readable(responses) + return CommandResults( + outputs_prefix="BlockDomainResults", + outputs_key_field=["Domain", "Brand", "Instance"], + outputs=rows, + readable_output=readable_output, + ) + + """ PAN-OS FLOW """ @@ -617,10 +669,20 @@ def start_flow(self) -> Any: # pragma: no cover def finish(self) -> list: # pragma: no cover """Clean up polling context and return the final result rows. + The accumulated responses (restored from context across polling cycles) are kept on the + instance so the caller can build verbose output before they are cleared from context. + Returns: The list of BlockDomainResults rows accumulated for the run. """ rows_raw = demisto.context().get("block_domain_rows", "[]") + # Preserve responses on the instance for verbose output before clearing context. + stored = demisto.context().get("panorama_responses", "") + if stored: + try: + self.responses = ast.literal_eval(stored) + except (ValueError, SyntaxError): + pass demisto.setContext("commit_job_id", "") demisto.setContext("push_job_id", "") demisto.setContext("panorama_responses", "") @@ -806,6 +868,7 @@ def main(): # pragma: no cover ) results: list = list(failed_rows) + command_responses: list = [] # accumulated per-command responses, used for verbose output. for brand in brands_to_run: if brand not in SUPPORTED_BRANDS: @@ -847,23 +910,13 @@ def main(): # pragma: no cover ) pan_os_result = pan_os.manage_pan_os_flow() if isinstance(pan_os_result, PollResult): + # A commit/push job is in flight; let the platform re-invoke the script. return_results(pan_os_result) return results.extend(pan_os_result) + command_responses.extend(pan_os.responses) - return_results( - CommandResults( - outputs_prefix="BlockDomainResults", - outputs_key_field=["Domain", "Brand", "Instance"], - outputs=results, - readable_output=tableToMarkdown( - "Block Domain", - results, - headers=["Domain", "Brand", "Instance", "Status", "Result", "Action", "RuleName", "Message"], - removeNull=False, - ), - ) - ) + return_results(build_final_command_results(results, verbose, command_responses)) except Exception as ex: return_error(f"Failed to execute block-domain. Error: {ex!s}") diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py index 2eccc38a7456..40180079da2f 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py @@ -11,6 +11,8 @@ STATUS_FAILED, STATUS_SKIPPED, PanOs, + build_final_command_results, + build_verbose_human_readable, derive_object_name, is_valid_fqdn, is_wildcard, @@ -245,3 +247,42 @@ def test_process_domains_failure_marks_row_failed(monkeypatch): assert rows[0]["Status"] == STATUS_FAILED assert rows[0]["Result"] == RESULT_FAILED assert "permission denied" in rows[0]["Message"] + + +def test_build_verbose_human_readable_joins_with_blank_lines(): + responses = [ + [ok_entry(contents="c1")], # no HumanReadable -> skipped + [{"Type": 1, "Contents": "c2", "HumanReadable": "HR-two", "EntryContext": {}}], + [{"Type": 1, "Contents": "c3", "HumanReadable": "HR-three", "EntryContext": {}}], + ] + verbose_hr = build_verbose_human_readable(responses) + # Leading blank line then each HR separated by a blank line. + assert verbose_hr == "\n\nHR-two\n\nHR-three" + + +def test_build_verbose_human_readable_empty_when_no_hr(): + assert build_verbose_human_readable([[ok_entry(contents="c1")]]) == "" + assert build_verbose_human_readable([]) == "" + + +def test_build_final_command_results_non_verbose_is_table_only(): + rows = [{"Domain": "a.com", "Brand": "Panorama", "Instance": "", "Status": STATUS_DONE, + "Result": RESULT_SUCCESS, "Action": ACTION_CREATED, "RuleName": "Cortex - Block Domain", "Message": "ok"}] + responses = [[{"Type": 1, "Contents": "c", "HumanReadable": "HR", "EntryContext": {}}]] + + result = build_final_command_results(rows, verbose=False, responses=responses) + assert result.outputs_prefix == "BlockDomainResults" + assert result.outputs == rows + assert "a.com" in result.readable_output + assert "HR" not in result.readable_output # verbose not appended + + +def test_build_final_command_results_verbose_appends_command_hr(): + rows = [{"Domain": "a.com", "Brand": "Panorama", "Instance": "", "Status": STATUS_DONE, + "Result": RESULT_SUCCESS, "Action": ACTION_CREATED, "RuleName": "Cortex - Block Domain", "Message": "ok"}] + responses = [[{"Type": 1, "Contents": "c", "HumanReadable": "HR-one", "EntryContext": {}}]] + + result = build_final_command_results(rows, verbose=True, responses=responses) + assert result.outputs == rows + assert "a.com" in result.readable_output # summary table present + assert result.readable_output.endswith("HR-one") # verbose appended after the table From 161bfa682a36d5473f7d06814a6b53f572c4684f Mon Sep 17 00:00:00 2001 From: nbensalm-palo Date: Wed, 19 Aug 2026 14:39:19 +0300 Subject: [PATCH 05/20] PAN-OS: expose create_tag argument on pan-os-create-address The pan-os-create-address command already reads the create_tag argument in Panorama.py, but it was missing from the command's YAML arguments, so the platform stripped it before execution. Declaring it (mirroring the deprecated panorama-create-address command) lets callers auto-create tags that do not yet exist instead of failing. --- Packs/PAN-OS/Integrations/Panorama/Panorama.yml | 7 +++++++ Packs/PAN-OS/ReleaseNotes/2_6_48.md | 6 ++++++ Packs/PAN-OS/pack_metadata.json | 2 +- 3 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 Packs/PAN-OS/ReleaseNotes/2_6_48.md diff --git a/Packs/PAN-OS/Integrations/Panorama/Panorama.yml b/Packs/PAN-OS/Integrations/Panorama/Panorama.yml index 33b6f7f693ac..efe6c28c8602 100644 --- a/Packs/PAN-OS/Integrations/Panorama/Panorama.yml +++ b/Packs/PAN-OS/Integrations/Panorama/Panorama.yml @@ -4071,6 +4071,13 @@ script: - description: The tag for the new address. isArray: true name: tag + - auto: PREDEFINED + description: Whether to create the tag if it does not exist. + defaultValue: 'false' + name: create_tag + predefined: + - 'true' + - 'false' description: Creates an address object. name: pan-os-create-address outputs: diff --git a/Packs/PAN-OS/ReleaseNotes/2_6_48.md b/Packs/PAN-OS/ReleaseNotes/2_6_48.md new file mode 100644 index 000000000000..0c509a91e50c --- /dev/null +++ b/Packs/PAN-OS/ReleaseNotes/2_6_48.md @@ -0,0 +1,6 @@ + +#### Integrations + +##### Palo Alto Networks PAN-OS + +- Added the *create_tag* argument to the ***pan-os-create-address*** command. When *create_tag=true* is set, tags that do not already exist are created automatically instead of failing the command. diff --git a/Packs/PAN-OS/pack_metadata.json b/Packs/PAN-OS/pack_metadata.json index d88c4789bc90..274bf22af859 100644 --- a/Packs/PAN-OS/pack_metadata.json +++ b/Packs/PAN-OS/pack_metadata.json @@ -2,7 +2,7 @@ "name": "PAN-OS by Palo Alto Networks", "description": "Manage Palo Alto Networks Firewall and Panorama. Use this pack to manage Prisma Access through Panorama. For more information see Panorama documentation.", "support": "xsoar", - "currentVersion": "2.6.47", + "currentVersion": "2.6.48", "author": "Cortex XSOAR", "url": "https://www.paloaltonetworks.com/cortex", "email": "", From 208cadbc932c242766b48f1013772f58c3c31208 Mon Sep 17 00:00:00 2001 From: nbensalm-palo Date: Wed, 19 Aug 2026 14:43:42 +0300 Subject: [PATCH 06/20] BlockDomain: pass create_tag=true when creating FQDN address-object --- Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py index 924cd21c7682..4f7f58ea5c34 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py @@ -467,7 +467,10 @@ def ensure_domain(self, domain: str, current_members: list) -> tuple[str, str]: else: create_args: dict = {"name": object_name, "fqdn": domain} if self.tag: + # create_tag=true auto-creates the tag on the firewall; pan-os-create-address otherwise + # fails when the tag does not already exist. create_args["tag"] = self.tag + create_args["create_tag"] = "true" self.execute_or_raise("pan-os-create-address", create_args, f"Failed to create address-object '{object_name}'") actions.append(ACTION_CREATED) messages.append(f"Address-object '{object_name}' created for '{domain}'.") From 4921c3bb4ffcfffedfc1506739e05ecfb0e861bf Mon Sep 17 00:00:00 2001 From: nbensalm-palo Date: Sun, 23 Aug 2026 10:51:42 +0300 Subject: [PATCH 07/20] changes before implementation --- .../Scripts/BlockDomain/BlockDomain.py | 168 ++++++++++++++---- .../Scripts/BlockDomain/BlockDomain_test.py | 102 +++++++++-- 2 files changed, 223 insertions(+), 47 deletions(-) diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py index 4f7f58ea5c34..6267135db6dc 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py @@ -9,7 +9,7 @@ """ CONSTANTS """ -SUPPORTED_BRANDS = ["Panorama"] # v1 supports Panorama only; extended in the multi-brand follow-up. +SUPPORTED_BRANDS = ["Panorama"] OBJECT_NAME_PREFIX = "Cortex-" # PAN-OS object names are limited to 63 characters. Reserve room for the prefix and a hash suffix on overflow. @@ -18,7 +18,7 @@ # Characters that are not allowed in a PAN-OS object name are normalised to a hyphen. OBJECT_NAME_SANITIZE_REGEX = re.compile(r"[^A-Za-z0-9.\-]") -PRE_POST = "pre-rulebase" # Q2: hard-coded for v1 (may become an argument later). +PRE_POST = "pre-rulebase" # A permissive FQDN matcher: labels of alphanumerics/hyphens separated by dots, at least one dot. FQDN_REGEX = re.compile(r"^(?=.{1,253}$)(?!-)[A-Za-z0-9-]{1,63}(? set: """ modules = demisto.getModules() enabled_brands = {module.get("brand") for module in modules.values() if module.get("state") == "active"} - demisto.debug(f"BlockDomain: the enabled modules are: {enabled_brands=}") + demisto.debug(f"{LOG_TAG} the enabled modules are: {enabled_brands=}") return enabled_brands @@ -205,10 +209,10 @@ def run_execute_command(command_name: str, args: dict[str, Any]) -> list[dict]: Returns: The raw list of command entries. """ - demisto.debug(f"BlockDomain: Executing command: {command_name} with {args=}") - res = demisto.executeCommand(command_name, args) - demisto.debug(f"BlockDomain: The response of {command_name} is {res}") - return res + # Note: intentionally no debug line here to avoid double-logging with execute_or_raise, which + # already logs entry / success / failure for every command that goes through it. Callers that + # bypass execute_or_raise (e.g. context probes) still get one entry log through demisto.executeCommand. + return demisto.executeCommand(command_name, args) def get_relevant_context(original_context: dict[str, Any], key: str) -> dict | list: @@ -313,6 +317,10 @@ def __init__(self, args: dict): self.log_forwarding_name = args.get("log_forwarding_name", "") self.domains: list = args.get("domains", []) self.responses: list = [] + # Tracks whether the deny rule has been ensured this run. The rule create is deferred to + # after the address-group exists (pan-os-create-rule validates that the destination + # references an existing object), so the write may happen mid-loop from ensure_domain. + self._rule_ensured: bool = False # ---- execution helper ---------------------------------------------- @@ -326,10 +334,13 @@ def execute_or_raise(self, command_name: str, command_args: dict, error_prefix: Returns: The raw command entries. """ + demisto.debug(f"{LOG_TAG} Executing command '{command_name}' with args={command_args}") res = run_execute_command(command_name, command_args) self.responses.append(res) if is_error(res): + demisto.debug(f"{LOG_TAG} Command '{command_name}' failed: {get_error(res)}") raise DemistoException(f"{error_prefix}: {get_error(res)}") + demisto.debug(f"{LOG_TAG} Command '{command_name}' succeeded.") return res # ---- context probes ------------------------------------------------- @@ -345,7 +356,7 @@ def address_object_exists(self, object_name: str) -> bool: res = run_execute_command("pan-os-get-address", {"name": object_name}) if is_error(res): # get-address raises when the object is absent; treat that as 'does not exist'. - demisto.debug(f"BlockDomain: address '{object_name}' not found ({get_error(res)}).") + demisto.debug(f"{LOG_TAG} address '{object_name}' not found ({get_error(res)}).") return False self.responses.append(res) context = get_relevant_context(res[0].get("EntryContext", {}), "Panorama.Addresses") @@ -384,26 +395,47 @@ def rule_destinations(self) -> tuple[bool, list]: # ---- single-run writes (group + rule are singletons) ---------------- - def ensure_group(self, group_context: dict | None) -> None: - """Ensure the static address-group exists, aborting if it is dynamic. + def ensure_group(self, group_context: dict | None) -> bool: + """Validate the address-group state without creating it. + + We do NOT create the group here: pan-os-create-address-group refuses to create a static + group with no members, so creation is deferred until we have the first address-object + (see create_group_with_member). This method only detects whether the group already exists + and aborts the run if it exists but is dynamic (customer-managed). Args: group_context (dict | None): The existing group context, or None if missing. + Returns: + True if the (static) group already exists, False if it needs to be created lazily. """ if group_context is None: - create_args: dict = {"name": self.address_group, "type": "static"} - if self.tag: - create_args["tags"] = self.tag - self.execute_or_raise( - "pan-os-create-address-group", create_args, f"Failed to create address-group '{self.address_group}'" - ) - return + demisto.debug(f"{LOG_TAG} Address-group '{self.address_group}' not found; will create on first object.") + return False group_type = (group_context.get("Type") or "").lower() + demisto.debug(f"{LOG_TAG} Address-group '{self.address_group}' already exists (type={group_type or 'static'}).") if group_type == "dynamic": raise DynamicGroupError( f"Address-group '{self.address_group}' already exists as dynamic; " f"will not modify a customer-managed dynamic group." ) + return True + + def create_group_with_member(self, object_name: str) -> None: + """Create the static address-group seeded with a first member (required by PAN-OS). + + pan-os-create-address-group rejects a static group without at least one address. Callers + must ensure the address-object exists on the firewall before invoking this method. + + Args: + object_name (str): The address-object to include as the initial group member. + """ + demisto.debug(f"{LOG_TAG} Creating address-group '{self.address_group}' seeded with member '{object_name}'.") + create_args: dict = {"name": self.address_group, "type": "static", "addresses": object_name} + if self.tag: + create_args["tags"] = self.tag + self.execute_or_raise( + "pan-os-create-address-group", create_args, f"Failed to create address-group '{self.address_group}'" + ) def ensure_rule(self, rule_present: bool, rule_destinations: list) -> None: """Ensure the deny rule exists, points at the group, and sits at the top. @@ -413,6 +445,7 @@ def ensure_rule(self, rule_present: bool, rule_destinations: list) -> None: rule_destinations (list): The rule's current destination list. """ if not rule_present: + demisto.debug(f"{LOG_TAG} Rule '{self.rule_name}' not found; creating deny rule ({PRE_POST}, where=top).") create_rule_args: dict = { "rulename": self.rule_name, "action": "deny", @@ -430,6 +463,9 @@ def ensure_rule(self, rule_present: bool, rule_destinations: list) -> None: self.execute_or_raise("pan-os-create-rule", create_rule_args, f"Failed to create rule '{self.rule_name}'") elif self.address_group not in rule_destinations: # The rule exists but does not yet reference our group - add it without replacing existing destinations. + demisto.debug( + f"{LOG_TAG} Rule '{self.rule_name}' exists but missing group '{self.address_group}'; adding destination." + ) self.execute_or_raise( "pan-os-edit-rule", { @@ -448,19 +484,38 @@ def ensure_rule(self, rule_present: bool, rule_destinations: list) -> None: f"Failed to move rule '{self.rule_name}' to top", ) - def ensure_domain(self, domain: str, current_members: list) -> tuple[str, str]: + def ensure_domain( + self, + domain: str, + current_members: list, + group_exists: bool, + rule_present: bool, + rule_destinations: list, + ) -> tuple[str, str, bool]: """Ensure a single domain's address-object exists and belongs to the group. + PAN-OS ordering constraints handled here: + * pan-os-create-address-group refuses to create an empty static group, so the group is + created lazily using the first domain's address-object as the initial member. + * pan-os-create-rule validates that ``destination`` references an existing object, so the + rule is also created after the group first appears (see _ensure_rule_once). + Args: domain (str): The domain to block. - current_members (list): The group's current member names. + current_members (list): The group's current member names (mutated in place). + group_exists (bool): Whether the target address-group currently exists on the firewall. + rule_present (bool): Whether the deny rule already existed at the start of the run. + rule_destinations (list): The rule's current destinations, when it already exists. Returns: - A tuple of (action, message) describing the effect for this domain. + A tuple of (action, message, group_exists_after) describing the effect for this domain + and the up-to-date group-existence flag for the next iteration. """ object_name = derive_object_name(domain) + demisto.debug(f"{LOG_TAG} Ensuring domain '{domain}' -> object '{object_name}'.") actions: list = [] messages: list = [] + # 1. Ensure the FQDN address-object exists on the firewall. if self.address_object_exists(object_name): actions.append(ACTION_UNCHANGED) messages.append(f"Address-object '{object_name}' already exists.") @@ -475,7 +530,17 @@ def ensure_domain(self, domain: str, current_members: list) -> tuple[str, str]: actions.append(ACTION_CREATED) messages.append(f"Address-object '{object_name}' created for '{domain}'.") - if object_name in current_members: + # 2. Ensure the object is a member of the target group (creating the group on first use). + if not group_exists: + # First object seeds the group; create-address-group requires at least one member. + self.create_group_with_member(object_name) + current_members.append(object_name) + group_exists = True + actions.append(ACTION_CREATED) + messages.append(f"Address-group '{self.address_group}' created with member '{object_name}'.") + # 3. Now that the group exists, it is safe to create the rule that references it. + self._ensure_rule_once(rule_present, rule_destinations) + elif object_name in current_members: actions.append(ACTION_UNCHANGED) messages.append(f"Already a member of '{self.address_group}'.") else: @@ -488,7 +553,21 @@ def ensure_domain(self, domain: str, current_members: list) -> tuple[str, str]: actions.append(ACTION_MODIFIED) messages.append(f"Added to '{self.address_group}'.") - return most_significant_action(actions), " ".join(messages) + final_action = most_significant_action(actions) + demisto.debug(f"{LOG_TAG} Domain '{domain}' processed with action '{final_action}'.") + return final_action, " ".join(messages), group_exists + + def _ensure_rule_once(self, rule_present: bool, rule_destinations: list) -> None: + """Call ensure_rule at most once per run. Safe to invoke from the per-domain loop. + + Args: + rule_present (bool): Whether the deny rule already existed at run start. + rule_destinations (list): The rule's current destinations, when it already exists. + """ + if self._rule_ensured: + return + self.ensure_rule(rule_present, rule_destinations) + self._rule_ensured = True # ---- orchestration -------------------------------------------------- @@ -499,19 +578,25 @@ def process_domains(self) -> list: The list of BlockDomainResults rows for the processed domains. """ rows: list = [] + demisto.debug(f"{LOG_TAG} process_domains started for {len(self.domains)} domain(s): {self.domains}") try: group_context = self.get_address_group() - self.ensure_group(group_context) + group_exists = self.ensure_group(group_context) current_members = [] if group_context is not None: members = group_context.get("Addresses") current_members = list(members) if isinstance(members, list) else [members] if members else [] rule_present, rule_destinations = self.rule_destinations() - self.ensure_rule(rule_present, rule_destinations) + # If the group already exists, it's safe to ensure the rule up front (its destination + # will resolve). Otherwise, defer to after the first domain seeds the group. + if group_exists: + self._ensure_rule_once(rule_present, rule_destinations) for domain in self.domains: - action, message = self.ensure_domain(domain, current_members) + action, message, group_exists = self.ensure_domain( + domain, current_members, group_exists, rule_present, rule_destinations + ) rows.append( build_result_row( domain=domain, @@ -606,10 +691,14 @@ def manage_pan_os_flow(self) -> Any: # pragma: no cover commit_job_id = self.args.get("commit_job_id") or demisto.get(incident_context, "commit_job_id") push_job_id = demisto.get(incident_context, "push_job_id") + demisto.debug(f"{LOG_TAG} manage_pan_os_flow dispatch: {commit_job_id=}, {push_job_id=}") if push_job_id: + demisto.debug(f"{LOG_TAG} Push job in flight ({push_job_id}); polling push status.") return self.handle_push_in_flight(push_job_id) if commit_job_id: + demisto.debug(f"{LOG_TAG} Commit job in flight ({commit_job_id}); polling commit status.") return self.handle_commit_in_flight(commit_job_id) + demisto.debug(f"{LOG_TAG} No job in flight; starting object/group/rule flow.") return self.start_flow() def handle_push_in_flight(self, push_job_id: str) -> Any: # pragma: no cover @@ -624,7 +713,9 @@ def handle_push_in_flight(self, push_job_id: str) -> Any: # pragma: no cover self.args["push_job_id"] = push_job_id res_push_status = pan_os_push_status(self.args, self.responses) if not POLLING: + demisto.debug(f"{LOG_TAG} Push job {push_job_id} finished; finalizing run.") return self.finish() + demisto.debug(f"{LOG_TAG} Push job {push_job_id} still running; re-scheduling.") self.save_responses() return res_push_status @@ -640,15 +731,20 @@ def handle_commit_in_flight(self, commit_job_id: str) -> Any: # pragma: no cove self.restore_responses() poll_commit_status = pan_os_commit_status(self.args, self.responses) if POLLING: + demisto.debug(f"{LOG_TAG} Commit job {commit_job_id} still running; re-scheduling.") self.save_responses() return poll_commit_status + demisto.debug(f"{LOG_TAG} Commit job {commit_job_id} finished.") # Commit finished - push to the device group if this is a Panorama instance. if self.pan_os_is_panorama(): + demisto.debug(f"{LOG_TAG} Instance is Panorama; starting push-to-device-group.") poll_push = pan_os_push_to_device(self.args, self.responses) if not POLLING: + demisto.debug(f"{LOG_TAG} Push completed synchronously; finalizing run.") return self.finish() self.save_responses() return poll_push + demisto.debug(f"{LOG_TAG} Instance is not Panorama; no push needed. Finalizing run.") return self.finish() def start_flow(self) -> Any: # pragma: no cover @@ -661,12 +757,16 @@ def start_flow(self) -> Any: # pragma: no cover demisto.setContext("block_domain_rows", str(rows)) made_changes = any(row["Status"] == STATUS_DONE for row in rows) auto_commit = argToBoolean(self.args.get("auto_commit", True)) + demisto.debug(f"{LOG_TAG} start_flow: {made_changes=}, {auto_commit=}, {len(rows)} row(s) produced.") if made_changes and auto_commit: + demisto.debug(f"{LOG_TAG} Changes made and auto_commit enabled; starting commit.") poll_commit = pan_os_commit(self.args, self.responses) if not POLLING: + demisto.debug(f"{LOG_TAG} Commit completed synchronously; finalizing run.") return self.finish() self.save_responses() return poll_commit + demisto.debug(f"{LOG_TAG} No commit needed (no changes or auto_commit disabled); returning rows.") return rows def finish(self) -> list: # pragma: no cover @@ -678,6 +778,7 @@ def finish(self) -> list: # pragma: no cover Returns: The list of BlockDomainResults rows accumulated for the run. """ + demisto.debug(f"{LOG_TAG} finish: clearing polling context and returning final rows.") rows_raw = demisto.context().get("block_domain_rows", "[]") # Preserve responses on the instance for verbose output before clearing context. stored = demisto.context().get("panorama_responses", "") @@ -842,7 +943,7 @@ def pan_os_push_status(args: dict, responses: list) -> PollResult: def main(): # pragma: no cover try: args = demisto.args() - demisto.debug(f"The script block-domain was called with the arguments {args=}") + demisto.debug(f"{LOG_TAG} block-domain invoked with arguments {args=}") domain_list = argToList(args.get("domain_list", [])) rule_name = args.get("rule_name", "Cortex - Block Domain") @@ -852,16 +953,17 @@ def main(): # pragma: no cover auto_commit = argToBoolean(args.get("auto_commit", True)) verbose = argToBoolean(args.get("verbose", False)) brands_to_run = argToList(args.get("brands", ",".join(SUPPORTED_BRANDS))) - demisto.debug(f"BlockDomain: {verbose=}, {brands_to_run=}") + demisto.debug(f"{LOG_TAG} {verbose=}, {brands_to_run=}") if not domain_list: return_error("domain_list argument is required.") valid_domains, failed_rows = validate_domains(domain_list) - demisto.debug(f"BlockDomain: {valid_domains=}, {len(failed_rows)} entries failed validation.") + demisto.debug(f"{LOG_TAG} {valid_domains=}, {len(failed_rows)} entries failed validation.") enabled_brands = get_enabled_brands() brands_to_run = brands_to_run or list(SUPPORTED_BRANDS) + demisto.debug(f"{LOG_TAG} {enabled_brands=}, {brands_to_run=}") runnable_brands = [b for b in brands_to_run if b in SUPPORTED_BRANDS and b in enabled_brands] if not runnable_brands: @@ -912,16 +1014,22 @@ def main(): # pragma: no cover } ) pan_os_result = pan_os.manage_pan_os_flow() - if isinstance(pan_os_result, PollResult): - # A commit/push job is in flight; let the platform re-invoke the script. + # manage_pan_os_flow returns a list[dict] (per-domain rows) only when the run is + # complete. Anything else (PollResult, CommandResults from a freshly-started commit + # job) means a job is in flight - hand it straight to return_results so the + # platform re-invokes the script for the next polling cycle. + if not isinstance(pan_os_result, list): + demisto.debug(f"{LOG_TAG} PAN-OS flow returned non-list ({type(pan_os_result).__name__}); polling in flight.") return_results(pan_os_result) return results.extend(pan_os_result) command_responses.extend(pan_os.responses) + demisto.debug(f"{LOG_TAG} Run complete; returning {len(results)} result row(s).") return_results(build_final_command_results(results, verbose, command_responses)) except Exception as ex: + demisto.debug(f"{LOG_TAG} block-domain failed with error: {ex!s}") return_error(f"Failed to execute block-domain. Error: {ex!s}") diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py index 40180079da2f..fa209ec4e71d 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py @@ -144,18 +144,18 @@ def _mock_execute(monkeypatch, side_effect): def test_process_domains_create_everything(monkeypatch): - # Group missing -> create group; rule missing -> create + move; address missing -> create + add. + # Group missing + rule missing: address is created first, then group is seeded with that + # object, then rule is created (destination must resolve to an existing group), then move. _mock_execute( monkeypatch, [ [ok_entry({"Panorama.AddressGroups": []})], # list-address-groups (missing) - [ok_entry()], # create-address-group (static, empty) [ok_entry({"Panorama.SecurityRule": []})], # list-rules (missing) - [ok_entry()], # create-rule - [ok_entry()], # move-rule [err_entry("not found")], # get-address (missing) [ok_entry()], # create-address - [ok_entry()], # edit-address-group (add member) + [ok_entry()], # create-address-group (seeded with first member) + [ok_entry()], # create-rule (destination = the now-existing group) + [ok_entry()], # move-rule ], ) rows = _pan_os(["evil.example.com"]).process_domains() @@ -166,13 +166,60 @@ def test_process_domains_create_everything(monkeypatch): assert rows[0]["RuleName"] == "Cortex - Block Domain" +def test_process_domains_missing_group_created_lazily_with_first_object(monkeypatch): + # Regression for two PAN-OS ordering rules: + # 1. pan-os-create-address-group refuses a static group without members -> must be created + # AFTER pan-os-create-address, and seeded with the first object. + # 2. pan-os-create-rule validates that `destination` references an existing object -> must + # be created AFTER pan-os-create-address-group. + calls: list = [] + + def _capture(name, args): + calls.append((name, args)) + seq = { + "pan-os-list-address-groups": [ok_entry({"Panorama.AddressGroups": []})], + "pan-os-list-rules": [ok_entry({"Panorama.SecurityRule": []})], + "pan-os-create-rule": [ok_entry()], + "pan-os-move-rule": [ok_entry()], + "pan-os-get-address": [err_entry("not found")], + "pan-os-create-address": [ok_entry()], + "pan-os-create-address-group": [ok_entry()], + } + return seq.get(name, [ok_entry()]) + + import BlockDomain + + monkeypatch.setattr(BlockDomain.demisto, "executeCommand", _capture) + + _pan_os(["evil.example.com"]).process_domains() + + names = [c[0] for c in calls] + # Ordering constraint 1: address must be created before the group. + assert names.index("pan-os-create-address") < names.index("pan-os-create-address-group") + # Ordering constraint 2: group must exist before the rule is created (destination resolves). + assert names.index("pan-os-create-address-group") < names.index("pan-os-create-rule") + # Group create carries the seed member (never an empty static group). + group_create_args = next(args for name, args in calls if name == "pan-os-create-address-group") + assert group_create_args["type"] == "static" + assert group_create_args["addresses"] == "Cortex-evil.example.com" + # Rule create destination points at the group. + rule_create_args = next(args for name, args in calls if name == "pan-os-create-rule") + assert rule_create_args["destination"] == "Blocked Domains - Cortex" + # No pan-os-edit-address-group was called for the first (seed) domain. + assert "pan-os-edit-address-group" not in names + + def test_process_domains_all_unchanged(monkeypatch): obj = "Cortex-evil.example.com" _mock_execute( monkeypatch, [ [ok_entry({"Panorama.AddressGroups": [{"Name": "Blocked Domains - Cortex", "Type": "static", "Addresses": [obj]}]})], - [ok_entry({"Panorama.SecurityRule": [{"Name": "Cortex - Block Domain", "Destination": ["Blocked Domains - Cortex"]}]})], + [ + ok_entry( + {"Panorama.SecurityRule": [{"Name": "Cortex - Block Domain", "Destination": ["Blocked Domains - Cortex"]}]} + ) + ], [ok_entry()], # move-rule [ok_entry({"Panorama.Addresses": [{"Name": obj}]})], # get-address (exists) ], @@ -187,7 +234,11 @@ def test_process_domains_modified_when_added_to_existing_group(monkeypatch): monkeypatch, [ [ok_entry({"Panorama.AddressGroups": [{"Name": "Blocked Domains - Cortex", "Type": "static", "Addresses": []}]})], - [ok_entry({"Panorama.SecurityRule": [{"Name": "Cortex - Block Domain", "Destination": ["Blocked Domains - Cortex"]}]})], + [ + ok_entry( + {"Panorama.SecurityRule": [{"Name": "Cortex - Block Domain", "Destination": ["Blocked Domains - Cortex"]}]} + ) + ], [ok_entry()], # move-rule [err_entry("not found")], # get-address (missing) [ok_entry()], # create-address @@ -234,13 +285,10 @@ def test_process_domains_failure_marks_row_failed(monkeypatch): _mock_execute( monkeypatch, [ - [ok_entry({"Panorama.AddressGroups": []})], # list-address-groups - [ok_entry()], # create-address-group - [ok_entry({"Panorama.SecurityRule": []})], # list-rules - [ok_entry()], # create-rule - [ok_entry()], # move-rule + [ok_entry({"Panorama.AddressGroups": []})], # list-address-groups (missing, deferred) + [ok_entry({"Panorama.SecurityRule": []})], # list-rules (missing, deferred) [err_entry("not found")], # get-address (missing) - [err_entry("permission denied")], # create-address fails + [err_entry("permission denied")], # create-address fails before group/rule are touched ], ) rows = _pan_os(["evil.example.com"]).process_domains() @@ -266,8 +314,18 @@ def test_build_verbose_human_readable_empty_when_no_hr(): def test_build_final_command_results_non_verbose_is_table_only(): - rows = [{"Domain": "a.com", "Brand": "Panorama", "Instance": "", "Status": STATUS_DONE, - "Result": RESULT_SUCCESS, "Action": ACTION_CREATED, "RuleName": "Cortex - Block Domain", "Message": "ok"}] + rows = [ + { + "Domain": "a.com", + "Brand": "Panorama", + "Instance": "", + "Status": STATUS_DONE, + "Result": RESULT_SUCCESS, + "Action": ACTION_CREATED, + "RuleName": "Cortex - Block Domain", + "Message": "ok", + } + ] responses = [[{"Type": 1, "Contents": "c", "HumanReadable": "HR", "EntryContext": {}}]] result = build_final_command_results(rows, verbose=False, responses=responses) @@ -278,8 +336,18 @@ def test_build_final_command_results_non_verbose_is_table_only(): def test_build_final_command_results_verbose_appends_command_hr(): - rows = [{"Domain": "a.com", "Brand": "Panorama", "Instance": "", "Status": STATUS_DONE, - "Result": RESULT_SUCCESS, "Action": ACTION_CREATED, "RuleName": "Cortex - Block Domain", "Message": "ok"}] + rows = [ + { + "Domain": "a.com", + "Brand": "Panorama", + "Instance": "", + "Status": STATUS_DONE, + "Result": RESULT_SUCCESS, + "Action": ACTION_CREATED, + "RuleName": "Cortex - Block Domain", + "Message": "ok", + } + ] responses = [[{"Type": 1, "Contents": "c", "HumanReadable": "HR-one", "EntryContext": {}}]] result = build_final_command_results(rows, verbose=True, responses=responses) From a8ae9e4441c7d5ccf701ab80ed10c17c19624748 Mon Sep 17 00:00:00 2001 From: nbensalm-palo Date: Sun, 23 Aug 2026 15:31:05 +0300 Subject: [PATCH 08/20] Working commit + push version - Unchanged items --- .../Scripts/BlockDomain/BlockDomain.py | 55 +++++++++++++++++-- .../Scripts/BlockDomain/BlockDomain_test.py | 2 +- 2 files changed, 50 insertions(+), 7 deletions(-) diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py index 6267135db6dc..2d5d1577bfb4 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py @@ -315,7 +315,11 @@ def __init__(self, args: dict): self.address_group = args["address_group"] self.tag = args.get("tag", "") self.log_forwarding_name = args.get("log_forwarding_name", "") - self.domains: list = args.get("domains", []) + # Named "domain_list" (matching the YAML arg name) so that when we merge these args into + # args_for_next_run in pan_os_commit / pan_os_push_to_device, the platform re-invokes the + # script with the required "domain_list" arg present (otherwise the platform's arg-validation + # step rejects the polling re-invocation with "Missing argument values: domain_list"). + self.domains: list = args.get("domain_list", []) self.responses: list = [] # Tracks whether the deny rule has been ensured this run. The rule create is deferred to # after the address-group exists (pan-os-create-rule validates that the destination @@ -687,9 +691,27 @@ def manage_pan_os_flow(self) -> Any: # pragma: no cover Returns: A PollResult when a job is in flight, or the list of result rows when finished. """ + # A legitimate polling re-entry always carries the job id in self.args (the polling + # machinery injects it via args_for_next_run). Anything sitting only in demisto.context() + # is stale from a previous crashed run and MUST NOT hijack a fresh manual invocation — + # otherwise the user gets an opaque poll on a job they didn't start (and, as observed, + # a crash when the stale job id no longer resolves). + commit_job_id = self.args.get("commit_job_id") + push_job_id = self.args.get("push_job_id") + + # Detect and clean up stale context so subsequent runs start fresh. incident_context = demisto.context() - commit_job_id = self.args.get("commit_job_id") or demisto.get(incident_context, "commit_job_id") - push_job_id = demisto.get(incident_context, "push_job_id") + stale_commit = demisto.get(incident_context, "commit_job_id") + stale_push = demisto.get(incident_context, "push_job_id") + if not commit_job_id and not push_job_id and (stale_commit or stale_push): + demisto.debug( + f"{LOG_TAG} Fresh invocation but stale polling context detected " + f"(stale_commit={stale_commit!r}, stale_push={stale_push!r}); clearing." + ) + demisto.setContext("commit_job_id", "") + demisto.setContext("push_job_id", "") + demisto.setContext("panorama_responses", "") + demisto.setContext("block_domain_rows", "") demisto.debug(f"{LOG_TAG} manage_pan_os_flow dispatch: {commit_job_id=}, {push_job_id=}") if push_job_id: @@ -851,14 +873,32 @@ def pan_os_commit_status(args: dict, responses: list) -> PollResult: Returns: The PollResult object. """ + global POLLING commit_job_id = args["commit_job_id"] res_commit_status = run_execute_command("pan-os-commit-status", {"job_id": commit_job_id}) responses.append(res_commit_status) - result_commit_status = res_commit_status[0].get("Contents", {}).get("response", {}).get("result", {}).get("job", {}) + # Defensive: when pan-os-commit-status errors (job id no longer exists, transport error, + # etc.) Contents is a plain string (the error text) rather than the nested dict shape. + # Treat that as a terminal failure and stop polling, instead of blowing up on `.get()`. + raw_contents = res_commit_status[0].get("Contents", {}) if res_commit_status else {} + if not isinstance(raw_contents, dict): + demisto.debug(f"{LOG_TAG} pan-os-commit-status returned non-dict Contents ({raw_contents!r}); treating as failure.") + commit_output = {"JobID": commit_job_id, "Status": "Failure"} + continue_to_poll = False + POLLING = continue_to_poll + return PollResult( + response=CommandResults( + outputs=commit_output, + outputs_key_field="JobID", + readable_output=tableToMarkdown("Commit Status:", commit_output, removeNull=True), + ), + args_for_next_run=args, + continue_to_poll=continue_to_poll, + ) + result_commit_status = raw_contents.get("response", {}).get("result", {}).get("job", {}) job_result = result_commit_status.get("result") commit_output = {"JobID": commit_job_id, "Status": "Success" if job_result == "OK" else "Failure"} continue_to_poll = result_commit_status.get("status") != "FIN" - global POLLING POLLING = continue_to_poll return PollResult( response=CommandResults( @@ -1001,7 +1041,10 @@ def main(): # pragma: no cover elif brand == "Panorama" and valid_domains: pan_os = PanOs( { - "domains": valid_domains, + # Key MUST be "domain_list" (the YAML arg name), not "domains" - see + # PanOs.__init__ comment. The platform re-invokes the script during polling + # with args_for_next_run, and it validates YAML-required args on every call. + "domain_list": valid_domains, "rule_name": rule_name, "log_forwarding_name": log_forwarding_name, "address_group": address_group, diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py index fa209ec4e71d..b2d34d525699 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py @@ -125,7 +125,7 @@ def test_most_significant_action(actions, expected): def _pan_os(domains): return PanOs( { - "domains": domains, + "domain_list": domains, "rule_name": "Cortex - Block Domain", "address_group": "Blocked Domains - Cortex", "tag": "cortex-blocked-domains", From 999c8a5ba679cf940eb154d5d1c16cb54e46ee8a Mon Sep 17 00:00:00 2001 From: nbensalm-palo Date: Sun, 23 Aug 2026 16:34:59 +0300 Subject: [PATCH 09/20] Working version including polling push --- .../Scripts/BlockDomain/BlockDomain.py | 35 ++++++++++++------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py index 2d5d1577bfb4..97683edbc279 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py @@ -691,27 +691,38 @@ def manage_pan_os_flow(self) -> Any: # pragma: no cover Returns: A PollResult when a job is in flight, or the list of result rows when finished. """ - # A legitimate polling re-entry always carries the job id in self.args (the polling - # machinery injects it via args_for_next_run). Anything sitting only in demisto.context() - # is stale from a previous crashed run and MUST NOT hijack a fresh manual invocation — - # otherwise the user gets an opaque poll on a job they didn't start (and, as observed, - # a crash when the stale job id no longer resolves). + # Polling re-entry mechanics: + # * The @polling_function machinery injects `commit_job_id` back into self.args via + # `args_for_next_run` on each poll cycle. So `commit_job_id in self.args` == "we are + # inside a polling re-invocation". + # * `push_job_id`, however, is not carried in args_for_next_run (see pan_os_push_to_device); + # it is written to demisto.context() by that function and MUST be read back from context + # on subsequent polls. So `push_job_id in demisto.context()` DURING a polling re-entry + # means "we already started the push, now poll its status". + # * A truly-fresh manual invocation has neither in args. Anything hanging around in context + # at that point is leftover state from a previous crashed run and must be scrubbed so it + # can't hijack the fresh run (see bug #4 in the log history). + incident_context = demisto.context() commit_job_id = self.args.get("commit_job_id") - push_job_id = self.args.get("push_job_id") + context_push_job_id = demisto.get(incident_context, "push_job_id") + context_commit_job_id = demisto.get(incident_context, "commit_job_id") - # Detect and clean up stale context so subsequent runs start fresh. - incident_context = demisto.context() - stale_commit = demisto.get(incident_context, "commit_job_id") - stale_push = demisto.get(incident_context, "push_job_id") - if not commit_job_id and not push_job_id and (stale_commit or stale_push): + # Detect a fresh invocation: no polling args injected, so args_for_next_run wasn't used. + is_polling_reentry = bool(commit_job_id) + if not is_polling_reentry and (context_commit_job_id or context_push_job_id): demisto.debug( f"{LOG_TAG} Fresh invocation but stale polling context detected " - f"(stale_commit={stale_commit!r}, stale_push={stale_push!r}); clearing." + f"(stale_commit={context_commit_job_id!r}, stale_push={context_push_job_id!r}); clearing." ) demisto.setContext("commit_job_id", "") demisto.setContext("push_job_id", "") demisto.setContext("panorama_responses", "") demisto.setContext("block_domain_rows", "") + context_push_job_id = None + + # push_job_id is only trusted during an actual polling re-entry (otherwise it's stale and + # was just cleared above). + push_job_id = context_push_job_id if is_polling_reentry else None demisto.debug(f"{LOG_TAG} manage_pan_os_flow dispatch: {commit_job_id=}, {push_job_id=}") if push_job_id: From b43afef1b63bd301d994243c14db5d02e2c16b09 Mon Sep 17 00:00:00 2001 From: nbensalm-palo Date: Sun, 23 Aug 2026 16:59:32 +0300 Subject: [PATCH 10/20] Adding readme --- .../Scripts/BlockDomain/README.md | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 Packs/AggregatedScripts/Scripts/BlockDomain/README.md diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/README.md b/Packs/AggregatedScripts/Scripts/BlockDomain/README.md new file mode 100644 index 000000000000..4706a8db9fae --- /dev/null +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/README.md @@ -0,0 +1,40 @@ +The script blocks a list of domain FQDNs in supported integrations. Safe to re-run: domains that are already blocked are reported as `Unchanged`. + +## Script Data + +--- + +| **Name** | **Description** | +| --- | --- | +| Script Type | python3 | +| Cortex XSOAR Version | 6.10.0 | + +## Inputs + +--- + +| **Argument Name** | **Description** | +| --- | --- | +| domain_list | List of domain FQDNs to block. Wildcard entries \(e.g. \*.evil.com\) are not supported and are skipped. | +| rule_name | The name of the rule which will be created in the relevant products. Default: `Cortex - Block Domain`. | +| log_forwarding_name | Panorama log forwarding object name. Indicate what type of Log Forwarding setting will be specified in the PAN-OS custom rules. | +| address_group | Address Group name used to hold the blocked domain objects. Default: `Blocked Domains - Cortex`. | +| auto_commit | Whether to commit the new rule and push to the device group at the end of the run. Default: `true`. | +| tag | The designated tag name for the domain FQDN object. Applied to every object the script creates. Default: `cortex-blocked-domains`. | +| brands | Which integration brands to run the command for. If not provided, the command will run for all available integrations.
For multi-select provide a comma-separated list. Default: `Panorama`. | +| verbose | Whether to retrieve a human-readable entry for every command or only the final result. True retrieves a human-readable entry for every command. False retrieves a human-readable entry only for the final result. Default: `false`. | + +## Outputs + +--- + +| **Path** | **Description** | **Type** | +| --- | --- | --- | +| BlockDomainResults.Domain | The domain FQDN that was processed. | String | +| BlockDomainResults.Brand | The brand \(integration\) used to block the domain. | String | +| BlockDomainResults.Instance | The integration instance used to block the domain. | String | +| BlockDomainResults.Status | The lifecycle status of the action. One of Done, Pending, Skipped, Failed. | String | +| BlockDomainResults.Result | The result of the action. Success or Failed. | String | +| BlockDomainResults.Action | The effect the run had on the target object. One of Created, Modified, Unchanged. | String | +| BlockDomainResults.RuleName | The name of the rule used for this integration. Empty if no rule was used. | String | +| BlockDomainResults.Message | A message concerning the result of the action. | String | From 6ac20f0f6666a522017c18ddfe0717129495281b Mon Sep 17 00:00:00 2001 From: nbensalm-palo Date: Sun, 23 Aug 2026 16:59:53 +0300 Subject: [PATCH 11/20] release notes and ruff --- Packs/AggregatedScripts/ReleaseNotes/1_4_0.md | 6 ++++++ Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.yml | 4 ++-- Packs/AggregatedScripts/pack_metadata.json | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) create mode 100644 Packs/AggregatedScripts/ReleaseNotes/1_4_0.md diff --git a/Packs/AggregatedScripts/ReleaseNotes/1_4_0.md b/Packs/AggregatedScripts/ReleaseNotes/1_4_0.md new file mode 100644 index 000000000000..c5dd0e714351 --- /dev/null +++ b/Packs/AggregatedScripts/ReleaseNotes/1_4_0.md @@ -0,0 +1,6 @@ + +#### Scripts + +##### New: block-domain + +- Added the **block-domain** script, which blocks one or more domains across your configured security products. diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.yml b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.yml index 985950a3f882..e29033230110 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.yml +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.yml @@ -90,9 +90,9 @@ timeout: 20m0s type: python subtype: python3 compliantpolicies: - - IP Blockage +- IP Blockage dockerimage: demisto/python3:3.12.13.10116658 -fromversion: 6.1.0 +fromversion: 6.10.0 marketplaces: - xsoar - marketplacev2 diff --git a/Packs/AggregatedScripts/pack_metadata.json b/Packs/AggregatedScripts/pack_metadata.json index 63c73ad0dd22..4049fc6bc8bb 100644 --- a/Packs/AggregatedScripts/pack_metadata.json +++ b/Packs/AggregatedScripts/pack_metadata.json @@ -2,7 +2,7 @@ "name": "Aggregated Scripts", "description": "A pack containing all aggregated scripts.", "support": "xsoar", - "currentVersion": "1.3.52", + "currentVersion": "1.4.0", "author": "Cortex XSOAR", "url": "https://www.paloaltonetworks.com/cortex", "email": "", From 41e6f2cc4f558bd7a60e2c0c8fd73f40672bcfe1 Mon Sep 17 00:00:00 2001 From: nbensalm-palo Date: Sun, 23 Aug 2026 17:14:18 +0300 Subject: [PATCH 12/20] adding validation for unchanged to not commit --- .../Scripts/BlockDomain/BlockDomain.py | 45 ++++++- .../Scripts/BlockDomain/BlockDomain_test.py | 122 +++++++++++++++++- 2 files changed, 162 insertions(+), 5 deletions(-) diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py index 97683edbc279..ca3550691b21 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py @@ -200,6 +200,23 @@ def get_enabled_brands() -> set: """ EXECUTE-COMMAND / CONTEXT HELPERS """ +def get_instance_from_result(res: dict) -> str: + """Extract the integration instance name from a demisto.executeCommand response entry. + + Each entry that comes back from ``demisto.executeCommand`` carries the instance that produced + it under ``Metadata.instance`` (also exposed as ``Metadata.brand`` for the brand). Mirrors the + helper used by other aggregated scripts such as ExpirePassword and get-user-data. + + Args: + res (dict): A single entry from the list returned by ``demisto.executeCommand``. + Returns: + The instance name, or an empty string if the entry doesn't carry one (e.g. context probes + that never actually ran a command). + """ + value = dict_safe_get(res, ["Metadata", "instance"]) + return str(value) if value else "" + + def run_execute_command(command_name: str, args: dict[str, Any]) -> list[dict]: """Execute a command and return its raw entries. @@ -325,6 +342,11 @@ def __init__(self, args: dict): # after the address-group exists (pan-os-create-rule validates that the destination # references an existing object), so the write may happen mid-loop from ensure_domain. self._rule_ensured: bool = False + # Instance name of the Panorama integration that actually served the calls this run. + # Captured lazily from the first response that carries Metadata.instance (usually the + # very first pan-os-list-address-groups call). Falls back to "" if nothing responds + # (e.g. an aborted brand run before any command completes). + self.instance_name: str = "" # ---- execution helper ---------------------------------------------- @@ -344,6 +366,14 @@ def execute_or_raise(self, command_name: str, command_args: dict, error_prefix: if is_error(res): demisto.debug(f"{LOG_TAG} Command '{command_name}' failed: {get_error(res)}") raise DemistoException(f"{error_prefix}: {get_error(res)}") + # Capture the serving instance name from the first successful response we see so that + # every row this run produces can be attributed to the right integration instance (matches + # the ExpirePassword pattern). Subsequent responses may come from the same instance so + # only overwrite if we still don't have one. + if not self.instance_name and res: + self.instance_name = get_instance_from_result(res[0]) + if self.instance_name: + demisto.debug(f"{LOG_TAG} Captured instance_name={self.instance_name!r} from '{command_name}'.") demisto.debug(f"{LOG_TAG} Command '{command_name}' succeeded.") return res @@ -608,6 +638,7 @@ def process_domains(self) -> list: status=STATUS_DONE, result=RESULT_SUCCESS, action=action, + instance=self.instance_name, rule_name=self.rule_name, message=f"{message} Rule '{self.rule_name}' enforced at top.", ) @@ -622,6 +653,7 @@ def process_domains(self) -> list: status=STATUS_SKIPPED, result=RESULT_SUCCESS, action=ACTION_UNCHANGED, + instance=self.instance_name, rule_name="", message=str(dyn_err), ) @@ -635,6 +667,7 @@ def process_domains(self) -> list: status=STATUS_FAILED, result=RESULT_FAILED, action=ACTION_UNCHANGED, + instance=self.instance_name, rule_name=self.rule_name, message=f"Failed to block '{domain}' on Panorama: {ex!s}", ) @@ -788,7 +821,12 @@ def start_flow(self) -> Any: # pragma: no cover """ rows = self.process_domains() demisto.setContext("block_domain_rows", str(rows)) - made_changes = any(row["Status"] == STATUS_DONE for row in rows) + # A commit/push is only worth kicking off when at least one row actually mutated + # Panorama state (Created or Modified). A run that returned exclusively `Unchanged` + # rows means every address was already in the group and the rule was already at top, + # so there's nothing in the candidate config to commit or push - skipping saves a + # commit job + a potentially-multi-minute push polling loop for idempotent re-runs. + made_changes = any(row.get("Action") in (ACTION_CREATED, ACTION_MODIFIED) for row in rows) auto_commit = argToBoolean(self.args.get("auto_commit", True)) demisto.debug(f"{LOG_TAG} start_flow: {made_changes=}, {auto_commit=}, {len(rows)} row(s) produced.") if made_changes and auto_commit: @@ -799,7 +837,10 @@ def start_flow(self) -> Any: # pragma: no cover return self.finish() self.save_responses() return poll_commit - demisto.debug(f"{LOG_TAG} No commit needed (no changes or auto_commit disabled); returning rows.") + if not made_changes: + demisto.debug(f"{LOG_TAG} All rows Unchanged; skipping commit and push (idempotent no-op run).") + else: + demisto.debug(f"{LOG_TAG} auto_commit disabled; skipping commit and push - changes remain uncommitted.") return rows def finish(self) -> list: # pragma: no cover diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py index b2d34d525699..b30cdcbbe1e1 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py @@ -21,9 +21,17 @@ ) -def ok_entry(entry_context=None, contents="ok"): - """Build a minimal successful execute_command entry.""" - return {"Type": 1, "Contents": contents, "HumanReadable": "", "EntryContext": entry_context or {}} +def ok_entry(entry_context=None, contents="ok", instance=None, brand="Panorama"): + """Build a minimal successful execute_command entry. + + When ``instance`` is provided, includes a ``Metadata`` block matching what the platform + actually returns (``Metadata.instance`` / ``Metadata.brand``) so tests can assert that the + aggregate script correctly captures the serving-instance name from response entries. + """ + entry: dict = {"Type": 1, "Contents": contents, "HumanReadable": "", "EntryContext": entry_context or {}} + if instance is not None: + entry["Metadata"] = {"instance": instance, "brand": brand} + return entry def err_entry(contents="error"): @@ -166,6 +174,114 @@ def test_process_domains_create_everything(monkeypatch): assert rows[0]["RuleName"] == "Cortex - Block Domain" +def test_start_flow_skips_commit_when_all_actions_unchanged(monkeypatch): + """Idempotent re-runs (every row Unchanged) must skip the commit+push cycle entirely. + + Without this optimisation a re-run that changed nothing still triggers pan-os-commit and + pan-os-push-to-device-group, which on a busy Panorama can add several minutes of polling + for zero benefit (nothing in the candidate config to commit). + """ + import BlockDomain + + calls: list = [] + + def _capture(name, args): + calls.append((name, args)) + seq = { + "pan-os-list-address-groups": [ok_entry({"Panorama.AddressGroups": [ + {"Name": "Blocked Domains - Cortex", "Type": "static", "Addresses": ["Cortex-evil.example.com"]} + ]})], + "pan-os-list-rules": [ok_entry({"Panorama.SecurityRule": [ + {"Name": "Cortex - Block Domain", "Destination": ["Blocked Domains - Cortex"]} + ]})], + "pan-os-get-address": [ok_entry({"Panorama.Addresses": {"Name": "Cortex-evil.example.com"}})], + "pan-os-move-rule": [ok_entry()], # move is idempotent noop but still executed + } + return seq.get(name, [ok_entry()]) + + monkeypatch.setattr(BlockDomain.demisto, "executeCommand", _capture) + # Guard: if the code decides to commit despite all Unchanged, this stub raises loudly. + monkeypatch.setattr(BlockDomain, "pan_os_commit", lambda *a, **k: pytest.fail( + "pan_os_commit must not be called when all rows are Unchanged" + )) + # setContext calls are harmless in tests; stub to avoid touching real state. + monkeypatch.setattr(BlockDomain.demisto, "setContext", lambda *a, **k: None) + + result = _pan_os(["evil.example.com"]).start_flow() + + # start_flow returns the rows directly (no polling), all Unchanged. + assert isinstance(result, list) + assert len(result) == 1 + assert result[0]["Action"] == ACTION_UNCHANGED + # No pan-os-commit was executed. + assert "pan-os-commit" not in [c[0] for c in calls] + + +def test_start_flow_commits_when_at_least_one_row_modified(monkeypatch): + """A run with any Created/Modified action must still trigger the commit flow.""" + import BlockDomain + + # Track whether the commit polling helper was invoked. + commit_called: list = [] + # Signal that pan_os_commit finished synchronously so start_flow returns rows (no polling). + def _fake_commit(args, responses): + commit_called.append(True) + # Mimic "no job started" -> not polling -> finish() path. + BlockDomain.POLLING = False + # Return a plain CommandResults; the caller will fall through to self.finish(). + return BlockDomain.CommandResults(readable_output="fake commit ok") + + def _capture(name, args): + seq = { + "pan-os-list-address-groups": [ok_entry({"Panorama.AddressGroups": [ + {"Name": "Blocked Domains - Cortex", "Type": "static", "Addresses": []} + ]})], + "pan-os-list-rules": [ok_entry({"Panorama.SecurityRule": [ + {"Name": "Cortex - Block Domain", "Destination": ["Blocked Domains - Cortex"]} + ]})], + "pan-os-get-address": [err_entry("not found")], + "pan-os-create-address": [ok_entry()], + "pan-os-edit-address-group": [ok_entry()], + "pan-os-move-rule": [ok_entry()], + } + return seq.get(name, [ok_entry()]) + + monkeypatch.setattr(BlockDomain.demisto, "executeCommand", _capture) + monkeypatch.setattr(BlockDomain.demisto, "setContext", lambda *a, **k: None) + monkeypatch.setattr(BlockDomain, "pan_os_commit", _fake_commit) + # After commit finishes synchronously, start_flow calls self.finish() which touches context. + # Feed it an empty stored rows blob so it returns []. + monkeypatch.setattr(BlockDomain.demisto, "context", lambda: {"block_domain_rows": "[]"}) + + _pan_os(["evil.example.com"]).start_flow() + + assert commit_called, "pan_os_commit must be called when at least one row is Created/Modified" + + +def test_process_domains_captures_instance_name_from_response_metadata(monkeypatch): + # Every row this run produces must be attributed to the integration instance that actually + # served the PAN-OS calls. The platform exposes it in Metadata.instance on every entry. + _mock_execute( + monkeypatch, + [ + # First response carries the Metadata.instance; the class should capture it and + # propagate it into every row. Subsequent responses may or may not carry it. + [ok_entry({"Panorama.AddressGroups": []}, instance="Panorama_QA")], + [ok_entry({"Panorama.SecurityRule": []})], + [err_entry("not found")], # get-address + [ok_entry()], # create-address + [ok_entry()], # create-address-group (seeded) + [ok_entry()], # create-rule + [ok_entry()], # move-rule + ], + ) + pan_os = _pan_os(["evil.example.com"]) + rows = pan_os.process_domains() + assert pan_os.instance_name == "Panorama_QA" + assert len(rows) == 1 + assert rows[0]["Instance"] == "Panorama_QA" + + def test_process_domains_missing_group_created_lazily_with_first_object(monkeypatch): # Regression for two PAN-OS ordering rules: # 1. pan-os-create-address-group refuses a static group without members -> must be created From 1199c56de86195f2fa1209b68fca31435731249d Mon Sep 17 00:00:00 2001 From: nbensalm-palo Date: Sun, 23 Aug 2026 17:32:24 +0300 Subject: [PATCH 13/20] removing comments --- .../Scripts/BlockDomain/BlockDomain.py | 181 +++++------------- 1 file changed, 50 insertions(+), 131 deletions(-) diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py index ca3550691b21..bc129dbbce8d 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py @@ -12,15 +12,13 @@ SUPPORTED_BRANDS = ["Panorama"] OBJECT_NAME_PREFIX = "Cortex-" -# PAN-OS object names are limited to 63 characters. Reserve room for the prefix and a hash suffix on overflow. +# PAN-OS object names are capped at 63 chars; reserve room for the prefix and hash suffix. MAX_OBJECT_NAME_LENGTH = 63 HASH_SUFFIX_LENGTH = 8 -# Characters that are not allowed in a PAN-OS object name are normalised to a hyphen. OBJECT_NAME_SANITIZE_REGEX = re.compile(r"[^A-Za-z0-9.\-]") PRE_POST = "pre-rulebase" -# A permissive FQDN matcher: labels of alphanumerics/hyphens separated by dots, at least one dot. FQDN_REGEX = re.compile(r"^(?=.{1,253}$)(?!-)[A-Za-z0-9-]{1,63}(? set: A set of enabled brand names. """ modules = demisto.getModules() - enabled_brands = {module.get("brand") for module in modules.values() if module.get("state") == "active"} - demisto.debug(f"{LOG_TAG} the enabled modules are: {enabled_brands=}") - return enabled_brands + return {module.get("brand") for module in modules.values() if module.get("state") == "active"} """ EXECUTE-COMMAND / CONTEXT HELPERS """ def get_instance_from_result(res: dict) -> str: - """Extract the integration instance name from a demisto.executeCommand response entry. + """Return the integration instance name from a demisto.executeCommand response entry. - Each entry that comes back from ``demisto.executeCommand`` carries the instance that produced - it under ``Metadata.instance`` (also exposed as ``Metadata.brand`` for the brand). Mirrors the - helper used by other aggregated scripts such as ExpirePassword and get-user-data. + Every entry exposes the serving instance under ``Metadata.instance``. Mirrors the pattern + used by other aggregated scripts such as ExpirePassword. Args: res (dict): A single entry from the list returned by ``demisto.executeCommand``. Returns: - The instance name, or an empty string if the entry doesn't carry one (e.g. context probes - that never actually ran a command). + The instance name, or an empty string if the entry doesn't carry one. """ value = dict_safe_get(res, ["Metadata", "instance"]) return str(value) if value else "" @@ -226,9 +218,6 @@ def run_execute_command(command_name: str, args: dict[str, Any]) -> list[dict]: Returns: The raw list of command entries. """ - # Note: intentionally no debug line here to avoid double-logging with execute_or_raise, which - # already logs entry / success / failure for every command that goes through it. Callers that - # bypass execute_or_raise (e.g. context probes) still get one entry log through demisto.executeCommand. return demisto.executeCommand(command_name, args) @@ -332,20 +321,13 @@ def __init__(self, args: dict): self.address_group = args["address_group"] self.tag = args.get("tag", "") self.log_forwarding_name = args.get("log_forwarding_name", "") - # Named "domain_list" (matching the YAML arg name) so that when we merge these args into - # args_for_next_run in pan_os_commit / pan_os_push_to_device, the platform re-invokes the - # script with the required "domain_list" arg present (otherwise the platform's arg-validation - # step rejects the polling re-invocation with "Missing argument values: domain_list"). + # Key MUST match the YAML arg name; args_for_next_run re-passes it during polling re-entry. self.domains: list = args.get("domain_list", []) self.responses: list = [] - # Tracks whether the deny rule has been ensured this run. The rule create is deferred to - # after the address-group exists (pan-os-create-rule validates that the destination - # references an existing object), so the write may happen mid-loop from ensure_domain. + # Rule create is deferred until the group exists (destination validation); this guard + # prevents ensure_rule from running twice per run. self._rule_ensured: bool = False - # Instance name of the Panorama integration that actually served the calls this run. - # Captured lazily from the first response that carries Metadata.instance (usually the - # very first pan-os-list-address-groups call). Falls back to "" if nothing responds - # (e.g. an aborted brand run before any command completes). + # Captured lazily from the first response's Metadata.instance; stamped on every row. self.instance_name: str = "" # ---- execution helper ---------------------------------------------- @@ -360,21 +342,13 @@ def execute_or_raise(self, command_name: str, command_args: dict, error_prefix: Returns: The raw command entries. """ - demisto.debug(f"{LOG_TAG} Executing command '{command_name}' with args={command_args}") res = run_execute_command(command_name, command_args) self.responses.append(res) if is_error(res): - demisto.debug(f"{LOG_TAG} Command '{command_name}' failed: {get_error(res)}") raise DemistoException(f"{error_prefix}: {get_error(res)}") - # Capture the serving instance name from the first successful response we see so that - # every row this run produces can be attributed to the right integration instance (matches - # the ExpirePassword pattern). Subsequent responses may come from the same instance so - # only overwrite if we still don't have one. + # Capture the serving instance on the first successful response (like ExpirePassword). if not self.instance_name and res: self.instance_name = get_instance_from_result(res[0]) - if self.instance_name: - demisto.debug(f"{LOG_TAG} Captured instance_name={self.instance_name!r} from '{command_name}'.") - demisto.debug(f"{LOG_TAG} Command '{command_name}' succeeded.") return res # ---- context probes ------------------------------------------------- @@ -388,9 +362,8 @@ def address_object_exists(self, object_name: str) -> bool: True if the object exists, False otherwise. """ res = run_execute_command("pan-os-get-address", {"name": object_name}) + # pan-os-get-address raises when the object is absent; treat that as 'does not exist'. if is_error(res): - # get-address raises when the object is absent; treat that as 'does not exist'. - demisto.debug(f"{LOG_TAG} address '{object_name}' not found ({get_error(res)}).") return False self.responses.append(res) context = get_relevant_context(res[0].get("EntryContext", {}), "Panorama.Addresses") @@ -430,23 +403,21 @@ def rule_destinations(self) -> tuple[bool, list]: # ---- single-run writes (group + rule are singletons) ---------------- def ensure_group(self, group_context: dict | None) -> bool: - """Validate the address-group state without creating it. + """Detect the group's state without creating it. - We do NOT create the group here: pan-os-create-address-group refuses to create a static - group with no members, so creation is deferred until we have the first address-object - (see create_group_with_member). This method only detects whether the group already exists - and aborts the run if it exists but is dynamic (customer-managed). + pan-os-create-address-group refuses an empty static group, so creation is deferred until + we have the first address-object (see create_group_with_member). Args: group_context (dict | None): The existing group context, or None if missing. Returns: True if the (static) group already exists, False if it needs to be created lazily. + Raises: + DynamicGroupError: If the group exists but is a customer-managed dynamic group. """ if group_context is None: - demisto.debug(f"{LOG_TAG} Address-group '{self.address_group}' not found; will create on first object.") return False group_type = (group_context.get("Type") or "").lower() - demisto.debug(f"{LOG_TAG} Address-group '{self.address_group}' already exists (type={group_type or 'static'}).") if group_type == "dynamic": raise DynamicGroupError( f"Address-group '{self.address_group}' already exists as dynamic; " @@ -463,7 +434,6 @@ def create_group_with_member(self, object_name: str) -> None: Args: object_name (str): The address-object to include as the initial group member. """ - demisto.debug(f"{LOG_TAG} Creating address-group '{self.address_group}' seeded with member '{object_name}'.") create_args: dict = {"name": self.address_group, "type": "static", "addresses": object_name} if self.tag: create_args["tags"] = self.tag @@ -479,7 +449,6 @@ def ensure_rule(self, rule_present: bool, rule_destinations: list) -> None: rule_destinations (list): The rule's current destination list. """ if not rule_present: - demisto.debug(f"{LOG_TAG} Rule '{self.rule_name}' not found; creating deny rule ({PRE_POST}, where=top).") create_rule_args: dict = { "rulename": self.rule_name, "action": "deny", @@ -496,10 +465,7 @@ def ensure_rule(self, rule_present: bool, rule_destinations: list) -> None: create_rule_args["log_forwarding"] = self.log_forwarding_name self.execute_or_raise("pan-os-create-rule", create_rule_args, f"Failed to create rule '{self.rule_name}'") elif self.address_group not in rule_destinations: - # The rule exists but does not yet reference our group - add it without replacing existing destinations. - demisto.debug( - f"{LOG_TAG} Rule '{self.rule_name}' exists but missing group '{self.address_group}'; adding destination." - ) + # Rule exists but doesn't reference our group - add without replacing existing destinations. self.execute_or_raise( "pan-os-edit-rule", { @@ -511,7 +477,7 @@ def ensure_rule(self, rule_present: bool, rule_destinations: list) -> None: }, f"Failed to add group to rule '{self.rule_name}'", ) - # Always ensure the rule sits at the top of the rulebase. + # Always enforce top placement. self.execute_or_raise( "pan-os-move-rule", {"rulename": self.rule_name, "where": "top", "pre_post": PRE_POST}, @@ -545,34 +511,31 @@ def ensure_domain( and the up-to-date group-existence flag for the next iteration. """ object_name = derive_object_name(domain) - demisto.debug(f"{LOG_TAG} Ensuring domain '{domain}' -> object '{object_name}'.") actions: list = [] messages: list = [] - # 1. Ensure the FQDN address-object exists on the firewall. + # 1. FQDN address-object. if self.address_object_exists(object_name): actions.append(ACTION_UNCHANGED) messages.append(f"Address-object '{object_name}' already exists.") else: create_args: dict = {"name": object_name, "fqdn": domain} if self.tag: - # create_tag=true auto-creates the tag on the firewall; pan-os-create-address otherwise - # fails when the tag does not already exist. + # create_tag=true auto-creates the tag; pan-os-create-address fails otherwise. create_args["tag"] = self.tag create_args["create_tag"] = "true" self.execute_or_raise("pan-os-create-address", create_args, f"Failed to create address-object '{object_name}'") actions.append(ACTION_CREATED) messages.append(f"Address-object '{object_name}' created for '{domain}'.") - # 2. Ensure the object is a member of the target group (creating the group on first use). + # 2. Group membership (create the group lazily on first object). if not group_exists: - # First object seeds the group; create-address-group requires at least one member. self.create_group_with_member(object_name) current_members.append(object_name) group_exists = True actions.append(ACTION_CREATED) messages.append(f"Address-group '{self.address_group}' created with member '{object_name}'.") - # 3. Now that the group exists, it is safe to create the rule that references it. + # Now that the group exists, safe to create the rule that references it. self._ensure_rule_once(rule_present, rule_destinations) elif object_name in current_members: actions.append(ACTION_UNCHANGED) @@ -587,9 +550,7 @@ def ensure_domain( actions.append(ACTION_MODIFIED) messages.append(f"Added to '{self.address_group}'.") - final_action = most_significant_action(actions) - demisto.debug(f"{LOG_TAG} Domain '{domain}' processed with action '{final_action}'.") - return final_action, " ".join(messages), group_exists + return most_significant_action(actions), " ".join(messages), group_exists def _ensure_rule_once(self, rule_present: bool, rule_destinations: list) -> None: """Call ensure_rule at most once per run. Safe to invoke from the per-domain loop. @@ -612,7 +573,6 @@ def process_domains(self) -> list: The list of BlockDomainResults rows for the processed domains. """ rows: list = [] - demisto.debug(f"{LOG_TAG} process_domains started for {len(self.domains)} domain(s): {self.domains}") try: group_context = self.get_address_group() group_exists = self.ensure_group(group_context) @@ -622,8 +582,8 @@ def process_domains(self) -> list: current_members = list(members) if isinstance(members, list) else [members] if members else [] rule_present, rule_destinations = self.rule_destinations() - # If the group already exists, it's safe to ensure the rule up front (its destination - # will resolve). Otherwise, defer to after the first domain seeds the group. + # If the group already exists, ensure the rule up front. Otherwise defer to after + # the first domain seeds the group. if group_exists: self._ensure_rule_once(rule_present, rule_destinations) @@ -644,7 +604,7 @@ def process_domains(self) -> list: ) ) except DynamicGroupError as dyn_err: - # Abort the whole brand for this run; other brands (future) would continue. + # Abort the whole brand for this run. for domain in self.domains: rows.append( build_result_row( @@ -724,28 +684,21 @@ def manage_pan_os_flow(self) -> Any: # pragma: no cover Returns: A PollResult when a job is in flight, or the list of result rows when finished. """ - # Polling re-entry mechanics: - # * The @polling_function machinery injects `commit_job_id` back into self.args via - # `args_for_next_run` on each poll cycle. So `commit_job_id in self.args` == "we are - # inside a polling re-invocation". - # * `push_job_id`, however, is not carried in args_for_next_run (see pan_os_push_to_device); - # it is written to demisto.context() by that function and MUST be read back from context - # on subsequent polls. So `push_job_id in demisto.context()` DURING a polling re-entry - # means "we already started the push, now poll its status". - # * A truly-fresh manual invocation has neither in args. Anything hanging around in context - # at that point is leftover state from a previous crashed run and must be scrubbed so it - # can't hijack the fresh run (see bug #4 in the log history). + # Polling re-entry rules: + # - commit_job_id is carried in self.args by args_for_next_run. + # - push_job_id is only written to demisto.context() by pan_os_push_to_device. + # - A fresh manual invocation has neither in args; any leftover context is stale and + # must be scrubbed so it can't hijack the fresh run. incident_context = demisto.context() commit_job_id = self.args.get("commit_job_id") context_push_job_id = demisto.get(incident_context, "push_job_id") context_commit_job_id = demisto.get(incident_context, "commit_job_id") - # Detect a fresh invocation: no polling args injected, so args_for_next_run wasn't used. is_polling_reentry = bool(commit_job_id) if not is_polling_reentry and (context_commit_job_id or context_push_job_id): demisto.debug( - f"{LOG_TAG} Fresh invocation but stale polling context detected " - f"(stale_commit={context_commit_job_id!r}, stale_push={context_push_job_id!r}); clearing." + f"{LOG_TAG} Stale polling context on fresh invocation " + f"(commit={context_commit_job_id!r}, push={context_push_job_id!r}); clearing." ) demisto.setContext("commit_job_id", "") demisto.setContext("push_job_id", "") @@ -753,18 +706,13 @@ def manage_pan_os_flow(self) -> Any: # pragma: no cover demisto.setContext("block_domain_rows", "") context_push_job_id = None - # push_job_id is only trusted during an actual polling re-entry (otherwise it's stale and - # was just cleared above). push_job_id = context_push_job_id if is_polling_reentry else None - demisto.debug(f"{LOG_TAG} manage_pan_os_flow dispatch: {commit_job_id=}, {push_job_id=}") + demisto.debug(f"{LOG_TAG} dispatch: {commit_job_id=}, {push_job_id=}") if push_job_id: - demisto.debug(f"{LOG_TAG} Push job in flight ({push_job_id}); polling push status.") return self.handle_push_in_flight(push_job_id) if commit_job_id: - demisto.debug(f"{LOG_TAG} Commit job in flight ({commit_job_id}); polling commit status.") return self.handle_commit_in_flight(commit_job_id) - demisto.debug(f"{LOG_TAG} No job in flight; starting object/group/rule flow.") return self.start_flow() def handle_push_in_flight(self, push_job_id: str) -> Any: # pragma: no cover @@ -779,9 +727,8 @@ def handle_push_in_flight(self, push_job_id: str) -> Any: # pragma: no cover self.args["push_job_id"] = push_job_id res_push_status = pan_os_push_status(self.args, self.responses) if not POLLING: - demisto.debug(f"{LOG_TAG} Push job {push_job_id} finished; finalizing run.") + demisto.debug(f"{LOG_TAG} Push job {push_job_id} finished.") return self.finish() - demisto.debug(f"{LOG_TAG} Push job {push_job_id} still running; re-scheduling.") self.save_responses() return res_push_status @@ -797,20 +744,16 @@ def handle_commit_in_flight(self, commit_job_id: str) -> Any: # pragma: no cove self.restore_responses() poll_commit_status = pan_os_commit_status(self.args, self.responses) if POLLING: - demisto.debug(f"{LOG_TAG} Commit job {commit_job_id} still running; re-scheduling.") self.save_responses() return poll_commit_status demisto.debug(f"{LOG_TAG} Commit job {commit_job_id} finished.") - # Commit finished - push to the device group if this is a Panorama instance. + # Commit finished - push to the device group if this is Panorama. if self.pan_os_is_panorama(): - demisto.debug(f"{LOG_TAG} Instance is Panorama; starting push-to-device-group.") poll_push = pan_os_push_to_device(self.args, self.responses) if not POLLING: - demisto.debug(f"{LOG_TAG} Push completed synchronously; finalizing run.") return self.finish() self.save_responses() return poll_push - demisto.debug(f"{LOG_TAG} Instance is not Panorama; no push needed. Finalizing run.") return self.finish() def start_flow(self) -> Any: # pragma: no cover @@ -821,26 +764,17 @@ def start_flow(self) -> Any: # pragma: no cover """ rows = self.process_domains() demisto.setContext("block_domain_rows", str(rows)) - # A commit/push is only worth kicking off when at least one row actually mutated - # Panorama state (Created or Modified). A run that returned exclusively `Unchanged` - # rows means every address was already in the group and the rule was already at top, - # so there's nothing in the candidate config to commit or push - skipping saves a - # commit job + a potentially-multi-minute push polling loop for idempotent re-runs. + # Only commit/push when a row actually mutated Panorama state. Skipping on a pure + # Unchanged run saves a commit job + a potentially multi-minute push polling loop. made_changes = any(row.get("Action") in (ACTION_CREATED, ACTION_MODIFIED) for row in rows) auto_commit = argToBoolean(self.args.get("auto_commit", True)) - demisto.debug(f"{LOG_TAG} start_flow: {made_changes=}, {auto_commit=}, {len(rows)} row(s) produced.") + demisto.debug(f"{LOG_TAG} start_flow: {made_changes=}, {auto_commit=}, {len(rows)} row(s)") if made_changes and auto_commit: - demisto.debug(f"{LOG_TAG} Changes made and auto_commit enabled; starting commit.") poll_commit = pan_os_commit(self.args, self.responses) if not POLLING: - demisto.debug(f"{LOG_TAG} Commit completed synchronously; finalizing run.") return self.finish() self.save_responses() return poll_commit - if not made_changes: - demisto.debug(f"{LOG_TAG} All rows Unchanged; skipping commit and push (idempotent no-op run).") - else: - demisto.debug(f"{LOG_TAG} auto_commit disabled; skipping commit and push - changes remain uncommitted.") return rows def finish(self) -> list: # pragma: no cover @@ -852,7 +786,6 @@ def finish(self) -> list: # pragma: no cover Returns: The list of BlockDomainResults rows accumulated for the run. """ - demisto.debug(f"{LOG_TAG} finish: clearing polling context and returning final rows.") rows_raw = demisto.context().get("block_domain_rows", "[]") # Preserve responses on the instance for verbose output before clearing context. stored = demisto.context().get("panorama_responses", "") @@ -929,15 +862,12 @@ def pan_os_commit_status(args: dict, responses: list) -> PollResult: commit_job_id = args["commit_job_id"] res_commit_status = run_execute_command("pan-os-commit-status", {"job_id": commit_job_id}) responses.append(res_commit_status) - # Defensive: when pan-os-commit-status errors (job id no longer exists, transport error, - # etc.) Contents is a plain string (the error text) rather than the nested dict shape. - # Treat that as a terminal failure and stop polling, instead of blowing up on `.get()`. + # When pan-os-commit-status errors, Contents is a plain string instead of the nested dict. + # Treat as a terminal failure to avoid a `.get()` crash on a string. raw_contents = res_commit_status[0].get("Contents", {}) if res_commit_status else {} if not isinstance(raw_contents, dict): - demisto.debug(f"{LOG_TAG} pan-os-commit-status returned non-dict Contents ({raw_contents!r}); treating as failure.") commit_output = {"JobID": commit_job_id, "Status": "Failure"} - continue_to_poll = False - POLLING = continue_to_poll + POLLING = False return PollResult( response=CommandResults( outputs=commit_output, @@ -945,7 +875,7 @@ def pan_os_commit_status(args: dict, responses: list) -> PollResult: readable_output=tableToMarkdown("Commit Status:", commit_output, removeNull=True), ), args_for_next_run=args, - continue_to_poll=continue_to_poll, + continue_to_poll=False, ) result_commit_status = raw_contents.get("response", {}).get("result", {}).get("job", {}) job_result = result_commit_status.get("result") @@ -1035,7 +965,7 @@ def pan_os_push_status(args: dict, responses: list) -> PollResult: def main(): # pragma: no cover try: args = demisto.args() - demisto.debug(f"{LOG_TAG} block-domain invoked with arguments {args=}") + demisto.debug(f"{LOG_TAG} block-domain invoked with {args=}") domain_list = argToList(args.get("domain_list", [])) rule_name = args.get("rule_name", "Cortex - Block Domain") @@ -1045,17 +975,13 @@ def main(): # pragma: no cover auto_commit = argToBoolean(args.get("auto_commit", True)) verbose = argToBoolean(args.get("verbose", False)) brands_to_run = argToList(args.get("brands", ",".join(SUPPORTED_BRANDS))) - demisto.debug(f"{LOG_TAG} {verbose=}, {brands_to_run=}") if not domain_list: return_error("domain_list argument is required.") valid_domains, failed_rows = validate_domains(domain_list) - demisto.debug(f"{LOG_TAG} {valid_domains=}, {len(failed_rows)} entries failed validation.") - enabled_brands = get_enabled_brands() brands_to_run = brands_to_run or list(SUPPORTED_BRANDS) - demisto.debug(f"{LOG_TAG} {enabled_brands=}, {brands_to_run=}") runnable_brands = [b for b in brands_to_run if b in SUPPORTED_BRANDS and b in enabled_brands] if not runnable_brands: @@ -1093,9 +1019,7 @@ def main(): # pragma: no cover elif brand == "Panorama" and valid_domains: pan_os = PanOs( { - # Key MUST be "domain_list" (the YAML arg name), not "domains" - see - # PanOs.__init__ comment. The platform re-invokes the script during polling - # with args_for_next_run, and it validates YAML-required args on every call. + # Key MUST match the YAML arg name; polling re-invocation validates it. "domain_list": valid_domains, "rule_name": rule_name, "log_forwarding_name": log_forwarding_name, @@ -1109,22 +1033,17 @@ def main(): # pragma: no cover } ) pan_os_result = pan_os.manage_pan_os_flow() - # manage_pan_os_flow returns a list[dict] (per-domain rows) only when the run is - # complete. Anything else (PollResult, CommandResults from a freshly-started commit - # job) means a job is in flight - hand it straight to return_results so the - # platform re-invokes the script for the next polling cycle. + # A list means the run finished. Anything else (PollResult / bare CommandResults + # from a freshly-started poll) means a job is in flight. if not isinstance(pan_os_result, list): - demisto.debug(f"{LOG_TAG} PAN-OS flow returned non-list ({type(pan_os_result).__name__}); polling in flight.") return_results(pan_os_result) return results.extend(pan_os_result) command_responses.extend(pan_os.responses) - demisto.debug(f"{LOG_TAG} Run complete; returning {len(results)} result row(s).") return_results(build_final_command_results(results, verbose, command_responses)) except Exception as ex: - demisto.debug(f"{LOG_TAG} block-domain failed with error: {ex!s}") return_error(f"Failed to execute block-domain. Error: {ex!s}") From b8325c0b8a30fb512943f7acff06e46b33634525 Mon Sep 17 00:00:00 2001 From: nbensalm-palo Date: Sun, 23 Aug 2026 17:39:07 +0300 Subject: [PATCH 14/20] updating tests descriptions --- .../Scripts/BlockDomain/BlockDomain_test.py | 315 ++++++++++++++---- 1 file changed, 247 insertions(+), 68 deletions(-) diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py index b30cdcbbe1e1..fec585e34bb8 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py @@ -49,6 +49,14 @@ def err_entry(contents="error"): ], ) def test_is_wildcard(domain, expected): + """ + Given: + - A domain string that may or may not contain a wildcard character. + When: + - Calling is_wildcard to detect wildcard patterns. + Then: + - Returns True for domains containing '*', False otherwise. + """ assert is_wildcard(domain) is expected @@ -67,32 +75,82 @@ def test_is_wildcard(domain, expected): ], ) def test_is_valid_fqdn(domain, expected): + """ + Given: + - A domain string that may or may not be a syntactically valid FQDN. + When: + - Calling is_valid_fqdn to validate the domain. + Then: + - Returns True for well-formed FQDNs (has dot, valid labels, no wildcard/illegal chars), + and False for anything else including empty strings and wildcards. + """ assert is_valid_fqdn(domain) is expected def test_derive_object_name_simple(): + """ + Given: + - A short, standard FQDN. + When: + - Calling derive_object_name to compute the PAN-OS address-object name. + Then: + - Returns the domain prefixed with "Cortex-". + """ assert derive_object_name("evil.example.com") == "Cortex-evil.example.com" def test_derive_object_name_is_deterministic(): + """ + Given: + - The same FQDN passed to derive_object_name twice. + When: + - Comparing the two returned object names. + Then: + - Both invocations return the exact same string (deterministic mapping). + """ assert derive_object_name("evil.example.com") == derive_object_name("evil.example.com") def test_derive_object_name_sanitises_illegal_chars(): - # Underscores are not valid PAN-OS object-name characters; they get normalised to hyphens. + """ + Given: + - A domain containing an underscore, which is not a legal PAN-OS object-name character. + When: + - Calling derive_object_name. + Then: + - Underscores are normalised to hyphens so the resulting name is accepted by PAN-OS. + """ assert derive_object_name("bad_domain.example.com") == "Cortex-bad-domain.example.com" def test_derive_object_name_overflow_truncates_and_hashes(): + """ + Given: + - A domain long enough that the naive prefixed name would exceed the PAN-OS + MAX_OBJECT_NAME_LENGTH limit. + When: + - Calling derive_object_name. + Then: + - The returned name fits within MAX_OBJECT_NAME_LENGTH, still starts with the + "Cortex-" prefix, and remains deterministic across calls. + """ long_domain = ("a" * 80) + ".example.com" name = derive_object_name(long_domain) assert len(name) <= MAX_OBJECT_NAME_LENGTH assert name.startswith(OBJECT_NAME_PREFIX) - # Overflow names are still deterministic. assert name == derive_object_name(long_domain) def test_validate_domains_splits_valid_and_failed(): + """ + Given: + - A mixed list of domains containing valid FQDNs, a wildcard, and an invalid FQDN. + When: + - Calling validate_domains to partition the input. + Then: + - Valid FQDNs are returned in the first list; the wildcard and invalid entries are + returned as failed rows with STATUS_FAILED / RESULT_FAILED and appropriate messages. + """ valid, failed = validate_domains(["evil.example.com", "*.evil.com", "no-dot", "phish.attacker.net"]) assert valid == ["evil.example.com", "phish.attacker.net"] @@ -111,6 +169,14 @@ def test_validate_domains_splits_valid_and_failed(): def test_validate_domains_all_valid(): + """ + Given: + - A list of domains that are all syntactically valid FQDNs. + When: + - Calling validate_domains. + Then: + - All entries end up in the valid list and no failed rows are produced. + """ valid, skipped = validate_domains(["a.com", "b.org"]) assert valid == ["a.com", "b.org"] assert skipped == [] @@ -127,6 +193,14 @@ def test_validate_domains_all_valid(): ], ) def test_most_significant_action(actions, expected): + """ + Given: + - A list of per-step actions (Unchanged / Modified / Created). + When: + - Calling most_significant_action to summarise the whole flow into a single action. + Then: + - Returns Created > Modified > Unchanged in priority; empty list yields Unchanged. + """ assert most_significant_action(actions) == expected @@ -152,18 +226,27 @@ def _mock_execute(monkeypatch, side_effect): def test_process_domains_create_everything(monkeypatch): - # Group missing + rule missing: address is created first, then group is seeded with that - # object, then rule is created (destination must resolve to an existing group), then move. + """ + Given: + - A tenant where neither the address group nor the security rule exist yet, and the + address object for the domain also does not exist. + When: + - Calling process_domains for a single domain. + Then: + - The address is created first, then the group is seeded with that object, then the + rule is created against the now-existing group, then the rule is moved to the top. + The resulting row is Done / Success / Created and carries the expected rule name. + """ _mock_execute( monkeypatch, [ - [ok_entry({"Panorama.AddressGroups": []})], # list-address-groups (missing) - [ok_entry({"Panorama.SecurityRule": []})], # list-rules (missing) - [err_entry("not found")], # get-address (missing) - [ok_entry()], # create-address - [ok_entry()], # create-address-group (seeded with first member) - [ok_entry()], # create-rule (destination = the now-existing group) - [ok_entry()], # move-rule + [ok_entry({"Panorama.AddressGroups": []})], + [ok_entry({"Panorama.SecurityRule": []})], + [err_entry("not found")], + [ok_entry()], + [ok_entry()], + [ok_entry()], + [ok_entry()], ], ) rows = _pan_os(["evil.example.com"]).process_domains() @@ -175,11 +258,15 @@ def test_process_domains_create_everything(monkeypatch): def test_start_flow_skips_commit_when_all_actions_unchanged(monkeypatch): - """Idempotent re-runs (every row Unchanged) must skip the commit+push cycle entirely. - - Without this optimisation a re-run that changed nothing still triggers pan-os-commit and - pan-os-push-to-device-group, which on a busy Panorama can add several minutes of polling - for zero benefit (nothing in the candidate config to commit). + """ + Given: + - A tenant already fully configured for the requested domain (group exists, rule exists, + address object already a member) so every row is Unchanged. + When: + - Calling start_flow. + Then: + - pan_os_commit is never invoked; start_flow returns the Unchanged rows directly + (skipping the multi-minute commit + push polling cycle on Panorama). """ import BlockDomain @@ -195,40 +282,41 @@ def _capture(name, args): {"Name": "Cortex - Block Domain", "Destination": ["Blocked Domains - Cortex"]} ]})], "pan-os-get-address": [ok_entry({"Panorama.Addresses": {"Name": "Cortex-evil.example.com"}})], - "pan-os-move-rule": [ok_entry()], # move is idempotent noop but still executed + "pan-os-move-rule": [ok_entry()], } return seq.get(name, [ok_entry()]) monkeypatch.setattr(BlockDomain.demisto, "executeCommand", _capture) - # Guard: if the code decides to commit despite all Unchanged, this stub raises loudly. monkeypatch.setattr(BlockDomain, "pan_os_commit", lambda *a, **k: pytest.fail( "pan_os_commit must not be called when all rows are Unchanged" )) - # setContext calls are harmless in tests; stub to avoid touching real state. monkeypatch.setattr(BlockDomain.demisto, "setContext", lambda *a, **k: None) result = _pan_os(["evil.example.com"]).start_flow() - # start_flow returns the rows directly (no polling), all Unchanged. assert isinstance(result, list) assert len(result) == 1 assert result[0]["Action"] == ACTION_UNCHANGED - # No pan-os-commit was executed. assert "pan-os-commit" not in [c[0] for c in calls] def test_start_flow_commits_when_at_least_one_row_modified(monkeypatch): - """A run with any Created/Modified action must still trigger the commit flow.""" + """ + Given: + - A tenant where the group and rule already exist but the address object for the + requested domain is missing, so at least one row will end up Created. + When: + - Calling start_flow. + Then: + - pan_os_commit is invoked (candidate config was modified and must be pushed). + """ import BlockDomain - # Track whether the commit polling helper was invoked. commit_called: list = [] - # Signal that pan_os_commit finished synchronously so start_flow returns rows (no polling). + def _fake_commit(args, responses): commit_called.append(True) - # Mimic "no job started" -> not polling -> finish() path. BlockDomain.POLLING = False - # Return a plain CommandResults; the caller will fall through to self.finish(). return BlockDomain.CommandResults(readable_output="fake commit ok") def _capture(name, args): @@ -249,8 +337,6 @@ def _capture(name, args): monkeypatch.setattr(BlockDomain.demisto, "executeCommand", _capture) monkeypatch.setattr(BlockDomain.demisto, "setContext", lambda *a, **k: None) monkeypatch.setattr(BlockDomain, "pan_os_commit", _fake_commit) - # After commit finishes synchronously, start_flow calls self.finish() which touches context. - # Feed it an empty stored rows blob so it returns []. monkeypatch.setattr(BlockDomain.demisto, "context", lambda: {"block_domain_rows": "[]"}) _pan_os(["evil.example.com"]).start_flow() @@ -259,20 +345,26 @@ def _capture(name, args): def test_process_domains_captures_instance_name_from_response_metadata(monkeypatch): - # Every row this run produces must be attributed to the integration instance that actually - # served the PAN-OS calls. The platform exposes it in Metadata.instance on every entry. + """ + Given: + - A stream of PAN-OS responses where the first entry carries Metadata.instance + (the platform stamps this on every execute_command result). + When: + - Calling process_domains. + Then: + - The class captures the serving-instance name on the first successful response and + propagates it into every resulting row's Instance field. + """ _mock_execute( monkeypatch, [ - # First response carries the Metadata.instance; the class should capture it and - # propagate it into every row. Subsequent responses may or may not carry it. [ok_entry({"Panorama.AddressGroups": []}, instance="Panorama_QA")], [ok_entry({"Panorama.SecurityRule": []})], - [err_entry("not found")], # get-address - [ok_entry()], # create-address - [ok_entry()], # create-address-group (seeded) - [ok_entry()], # create-rule - [ok_entry()], # move-rule + [err_entry("not found")], + [ok_entry()], + [ok_entry()], + [ok_entry()], + [ok_entry()], ], ) pan_os = _pan_os(["evil.example.com"]) @@ -283,11 +375,18 @@ def test_process_domains_captures_instance_name_from_response_metadata(monkeypat def test_process_domains_missing_group_created_lazily_with_first_object(monkeypatch): - # Regression for two PAN-OS ordering rules: - # 1. pan-os-create-address-group refuses a static group without members -> must be created - # AFTER pan-os-create-address, and seeded with the first object. - # 2. pan-os-create-rule validates that `destination` references an existing object -> must - # be created AFTER pan-os-create-address-group. + """ + Given: + - A tenant with neither group nor rule pre-configured, and no address object for the + requested domain. PAN-OS refuses to create a static group with no members, and + refuses to create a rule whose destination does not resolve to an existing object. + When: + - Calling process_domains for a single domain. + Then: + - The address is created first; then the group is created lazily seeded with that + first object (never as an empty static group); then the rule is created against the + now-existing group. No edit-address-group call is emitted for the seed domain. + """ calls: list = [] def _capture(name, args): @@ -310,22 +409,26 @@ def _capture(name, args): _pan_os(["evil.example.com"]).process_domains() names = [c[0] for c in calls] - # Ordering constraint 1: address must be created before the group. assert names.index("pan-os-create-address") < names.index("pan-os-create-address-group") - # Ordering constraint 2: group must exist before the rule is created (destination resolves). assert names.index("pan-os-create-address-group") < names.index("pan-os-create-rule") - # Group create carries the seed member (never an empty static group). group_create_args = next(args for name, args in calls if name == "pan-os-create-address-group") assert group_create_args["type"] == "static" assert group_create_args["addresses"] == "Cortex-evil.example.com" - # Rule create destination points at the group. rule_create_args = next(args for name, args in calls if name == "pan-os-create-rule") assert rule_create_args["destination"] == "Blocked Domains - Cortex" - # No pan-os-edit-address-group was called for the first (seed) domain. assert "pan-os-edit-address-group" not in names def test_process_domains_all_unchanged(monkeypatch): + """ + Given: + - A tenant where the group, rule, and address object all already exist and the + address object is already a member of the group. + When: + - Calling process_domains. + Then: + - The resulting row is Done / Unchanged (no create/modify happened). + """ obj = "Cortex-evil.example.com" _mock_execute( monkeypatch, @@ -336,8 +439,8 @@ def test_process_domains_all_unchanged(monkeypatch): {"Panorama.SecurityRule": [{"Name": "Cortex - Block Domain", "Destination": ["Blocked Domains - Cortex"]}]} ) ], - [ok_entry()], # move-rule - [ok_entry({"Panorama.Addresses": [{"Name": obj}]})], # get-address (exists) + [ok_entry()], + [ok_entry({"Panorama.Addresses": [{"Name": obj}]})], ], ) rows = _pan_os(["evil.example.com"]).process_domains() @@ -346,6 +449,16 @@ def test_process_domains_all_unchanged(monkeypatch): def test_process_domains_modified_when_added_to_existing_group(monkeypatch): + """ + Given: + - A tenant where the group and rule already exist but the group has no members yet + and the address object for the requested domain does not exist. + When: + - Calling process_domains. + Then: + - The address is created and added to the existing group; the resulting row is + Done / Created (most-significant action of the create-address step). + """ _mock_execute( monkeypatch, [ @@ -355,36 +468,55 @@ def test_process_domains_modified_when_added_to_existing_group(monkeypatch): {"Panorama.SecurityRule": [{"Name": "Cortex - Block Domain", "Destination": ["Blocked Domains - Cortex"]}]} ) ], - [ok_entry()], # move-rule - [err_entry("not found")], # get-address (missing) - [ok_entry()], # create-address - [ok_entry()], # edit-address-group (add member) + [ok_entry()], + [err_entry("not found")], + [ok_entry()], + [ok_entry()], ], ) rows = _pan_os(["evil.example.com"]).process_domains() assert rows[0]["Status"] == STATUS_DONE - assert rows[0]["Action"] == ACTION_CREATED # object was created -> most significant + assert rows[0]["Action"] == ACTION_CREATED def test_process_domains_existing_rule_missing_group_is_edited(monkeypatch): + """ + Given: + - A tenant where the rule exists but its destination does not yet reference our + address group; the group and the address object already exist. + When: + - Calling process_domains. + Then: + - The rule is edited to include the group in its destination; the resulting row is + Done / Unchanged because the address object and its group membership were unchanged + (rule-level fix is a group-scope, not per-domain, mutation). + """ obj = "Cortex-evil.example.com" _mock_execute( monkeypatch, [ [ok_entry({"Panorama.AddressGroups": [{"Name": "Blocked Domains - Cortex", "Type": "static", "Addresses": [obj]}]})], - # Rule exists but its destination does not reference our group -> triggers edit-rule. [ok_entry({"Panorama.SecurityRule": [{"Name": "Cortex - Block Domain", "Destination": ["something-else"]}]})], - [ok_entry()], # edit-rule (add destination) - [ok_entry()], # move-rule - [ok_entry({"Panorama.Addresses": [{"Name": obj}]})], # get-address (exists) + [ok_entry()], + [ok_entry()], + [ok_entry({"Panorama.Addresses": [{"Name": obj}]})], ], ) rows = _pan_os(["evil.example.com"]).process_domains() assert rows[0]["Status"] == STATUS_DONE - assert rows[0]["Action"] == ACTION_UNCHANGED # object + membership unchanged; rule edit is a group-level fix + assert rows[0]["Action"] == ACTION_UNCHANGED def test_process_domains_dynamic_group_is_skipped(monkeypatch): + """ + Given: + - A tenant where the target group already exists but as a *dynamic* address group + (which cannot accept manually added static members). + When: + - Calling process_domains. + Then: + - The row is marked Skipped / Success and the message explains the group is dynamic. + """ _mock_execute( monkeypatch, [ @@ -398,13 +530,22 @@ def test_process_domains_dynamic_group_is_skipped(monkeypatch): def test_process_domains_failure_marks_row_failed(monkeypatch): + """ + Given: + - A tenant where the group and rule do not exist yet, and the pan-os-create-address + command fails (e.g. permission denied) before the group and rule are touched. + When: + - Calling process_domains. + Then: + - The resulting row is Failed / Failed and the error message from PAN-OS is surfaced. + """ _mock_execute( monkeypatch, [ - [ok_entry({"Panorama.AddressGroups": []})], # list-address-groups (missing, deferred) - [ok_entry({"Panorama.SecurityRule": []})], # list-rules (missing, deferred) - [err_entry("not found")], # get-address (missing) - [err_entry("permission denied")], # create-address fails before group/rule are touched + [ok_entry({"Panorama.AddressGroups": []})], + [ok_entry({"Panorama.SecurityRule": []})], + [err_entry("not found")], + [err_entry("permission denied")], ], ) rows = _pan_os(["evil.example.com"]).process_domains() @@ -414,22 +555,50 @@ def test_process_domains_failure_marks_row_failed(monkeypatch): def test_build_verbose_human_readable_joins_with_blank_lines(): + """ + Given: + - A list of response entries where some carry a HumanReadable string and some do not. + When: + - Calling build_verbose_human_readable. + Then: + - Entries without HumanReadable are skipped; the rest are joined with a blank line + separator, prefixed by a leading blank line so the block detaches from the summary + table above it. + """ responses = [ - [ok_entry(contents="c1")], # no HumanReadable -> skipped + [ok_entry(contents="c1")], [{"Type": 1, "Contents": "c2", "HumanReadable": "HR-two", "EntryContext": {}}], [{"Type": 1, "Contents": "c3", "HumanReadable": "HR-three", "EntryContext": {}}], ] verbose_hr = build_verbose_human_readable(responses) - # Leading blank line then each HR separated by a blank line. assert verbose_hr == "\n\nHR-two\n\nHR-three" def test_build_verbose_human_readable_empty_when_no_hr(): + """ + Given: + - A response list where no entry has a HumanReadable string, or an empty list. + When: + - Calling build_verbose_human_readable. + Then: + - Returns an empty string (nothing to append to the summary table). + """ assert build_verbose_human_readable([[ok_entry(contents="c1")]]) == "" assert build_verbose_human_readable([]) == "" def test_build_final_command_results_non_verbose_is_table_only(): + """ + Given: + - A rows list and a set of responses that include HumanReadable content, with + verbose=False. + When: + - Calling build_final_command_results. + Then: + - The returned CommandResults uses the BlockDomainResults context prefix, exposes the + rows unchanged as outputs, renders the summary table containing the domain, and does + NOT append any of the per-command verbose HR blocks. + """ rows = [ { "Domain": "a.com", @@ -448,10 +617,20 @@ def test_build_final_command_results_non_verbose_is_table_only(): assert result.outputs_prefix == "BlockDomainResults" assert result.outputs == rows assert "a.com" in result.readable_output - assert "HR" not in result.readable_output # verbose not appended + assert "HR" not in result.readable_output def test_build_final_command_results_verbose_appends_command_hr(): + """ + Given: + - A rows list and responses with a HumanReadable block, with verbose=True. + When: + - Calling build_final_command_results. + Then: + - The returned CommandResults exposes the rows as outputs, renders the summary table + containing the domain, and appends the per-command HR block at the end of the + readable_output (so users can see exactly what each downstream call produced). + """ rows = [ { "Domain": "a.com", @@ -468,5 +647,5 @@ def test_build_final_command_results_verbose_appends_command_hr(): result = build_final_command_results(rows, verbose=True, responses=responses) assert result.outputs == rows - assert "a.com" in result.readable_output # summary table present - assert result.readable_output.endswith("HR-one") # verbose appended after the table + assert "a.com" in result.readable_output + assert result.readable_output.endswith("HR-one") From 8fade4bf6cd571e10c31fc422c01add33e8901a5 Mon Sep 17 00:00:00 2001 From: nbensalm-palo Date: Mon, 24 Aug 2026 10:05:01 +0300 Subject: [PATCH 15/20] ruff format --- .../Scripts/BlockDomain/BlockDomain_test.py | 40 ++++++++++++------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py index fec585e34bb8..5351b2d08b37 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py @@ -275,21 +275,29 @@ def test_start_flow_skips_commit_when_all_actions_unchanged(monkeypatch): def _capture(name, args): calls.append((name, args)) seq = { - "pan-os-list-address-groups": [ok_entry({"Panorama.AddressGroups": [ - {"Name": "Blocked Domains - Cortex", "Type": "static", "Addresses": ["Cortex-evil.example.com"]} - ]})], - "pan-os-list-rules": [ok_entry({"Panorama.SecurityRule": [ - {"Name": "Cortex - Block Domain", "Destination": ["Blocked Domains - Cortex"]} - ]})], + "pan-os-list-address-groups": [ + ok_entry( + { + "Panorama.AddressGroups": [ + {"Name": "Blocked Domains - Cortex", "Type": "static", "Addresses": ["Cortex-evil.example.com"]} + ] + } + ) + ], + "pan-os-list-rules": [ + ok_entry( + {"Panorama.SecurityRule": [{"Name": "Cortex - Block Domain", "Destination": ["Blocked Domains - Cortex"]}]} + ) + ], "pan-os-get-address": [ok_entry({"Panorama.Addresses": {"Name": "Cortex-evil.example.com"}})], "pan-os-move-rule": [ok_entry()], } return seq.get(name, [ok_entry()]) monkeypatch.setattr(BlockDomain.demisto, "executeCommand", _capture) - monkeypatch.setattr(BlockDomain, "pan_os_commit", lambda *a, **k: pytest.fail( - "pan_os_commit must not be called when all rows are Unchanged" - )) + monkeypatch.setattr( + BlockDomain, "pan_os_commit", lambda *a, **k: pytest.fail("pan_os_commit must not be called when all rows are Unchanged") + ) monkeypatch.setattr(BlockDomain.demisto, "setContext", lambda *a, **k: None) result = _pan_os(["evil.example.com"]).start_flow() @@ -321,12 +329,14 @@ def _fake_commit(args, responses): def _capture(name, args): seq = { - "pan-os-list-address-groups": [ok_entry({"Panorama.AddressGroups": [ - {"Name": "Blocked Domains - Cortex", "Type": "static", "Addresses": []} - ]})], - "pan-os-list-rules": [ok_entry({"Panorama.SecurityRule": [ - {"Name": "Cortex - Block Domain", "Destination": ["Blocked Domains - Cortex"]} - ]})], + "pan-os-list-address-groups": [ + ok_entry({"Panorama.AddressGroups": [{"Name": "Blocked Domains - Cortex", "Type": "static", "Addresses": []}]}) + ], + "pan-os-list-rules": [ + ok_entry( + {"Panorama.SecurityRule": [{"Name": "Cortex - Block Domain", "Destination": ["Blocked Domains - Cortex"]}]} + ) + ], "pan-os-get-address": [err_entry("not found")], "pan-os-create-address": [ok_entry()], "pan-os-edit-address-group": [ok_entry()], From dafc0fa42ae8fb1c52c08b813fef37245bd39fc0 Mon Sep 17 00:00:00 2001 From: nbensalm-palo Date: Mon, 24 Aug 2026 11:04:08 +0300 Subject: [PATCH 16/20] Updating Code after review --- .../Scripts/BlockDomain/BlockDomain.py | 39 ++-- .../Scripts/BlockDomain/BlockDomain_test.py | 173 ++++++++++++++++++ 2 files changed, 200 insertions(+), 12 deletions(-) diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py index bc129dbbce8d..95b03497c54c 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py @@ -1,4 +1,3 @@ -import ast import hashlib import re from typing import Any @@ -669,11 +668,12 @@ def reduce_responses(self) -> list: def restore_responses(self) -> None: """Restore the accumulated responses that were serialized to context in a previous cycle.""" - self.responses = ast.literal_eval(demisto.context().get("panorama_responses", "[]") or "[]") + stored = demisto.context().get("panorama_responses", "") or "" + self.responses = json.loads(stored) if stored else [] def save_responses(self) -> None: """Serialize the accumulated responses to context for the next polling cycle.""" - demisto.setContext("panorama_responses", str(self.reduce_responses())) + demisto.setContext("panorama_responses", json.dumps(self.reduce_responses())) def manage_pan_os_flow(self) -> Any: # pragma: no cover """Dispatch the PAN-OS flow to the correct state. @@ -763,7 +763,7 @@ def start_flow(self) -> Any: # pragma: no cover A PollResult while the commit is running, or the final result rows when finished. """ rows = self.process_domains() - demisto.setContext("block_domain_rows", str(rows)) + demisto.setContext("block_domain_rows", json.dumps(rows)) # Only commit/push when a row actually mutated Panorama state. Skipping on a pure # Unchanged run saves a commit job + a potentially multi-minute push polling loop. made_changes = any(row.get("Action") in (ACTION_CREATED, ACTION_MODIFIED) for row in rows) @@ -786,21 +786,21 @@ def finish(self) -> list: # pragma: no cover Returns: The list of BlockDomainResults rows accumulated for the run. """ - rows_raw = demisto.context().get("block_domain_rows", "[]") + rows_raw = demisto.context().get("block_domain_rows", "") or "" # Preserve responses on the instance for verbose output before clearing context. - stored = demisto.context().get("panorama_responses", "") + stored = demisto.context().get("panorama_responses", "") or "" if stored: try: - self.responses = ast.literal_eval(stored) - except (ValueError, SyntaxError): + self.responses = json.loads(stored) + except (ValueError, TypeError): pass demisto.setContext("commit_job_id", "") demisto.setContext("push_job_id", "") demisto.setContext("panorama_responses", "") demisto.setContext("block_domain_rows", "") try: - return ast.literal_eval(rows_raw) if rows_raw else [] - except (ValueError, SyntaxError): + return json.loads(rows_raw) if rows_raw else [] + except (ValueError, TypeError): return [] @@ -939,10 +939,26 @@ def pan_os_push_status(args: dict, responses: list) -> PollResult: Returns: The PollResult object. """ + global POLLING push_job_id = args["push_job_id"] res_push_status = run_execute_command("pan-os-push-status", {"job_id": push_job_id}) responses.append(res_push_status) - push_status = res_push_status[0].get("Contents", {}).get("response", {}).get("result", {}).get("job", {}).get("status", "") + # When pan-os-push-status errors, Contents is a plain string instead of the nested dict. + # Treat as a terminal failure to avoid a `.get()` crash on a string (mirrors pan_os_commit_status). + raw_contents = res_push_status[0].get("Contents", {}) if res_push_status else {} + if is_error(res_push_status) or not isinstance(raw_contents, dict): + push_output = {"JobID": push_job_id, "Status": "Failure"} + POLLING = False + return PollResult( + response=CommandResults( + outputs=push_output, + outputs_key_field="JobID", + readable_output=tableToMarkdown("Push to Device Group:", push_output, ["JobID", "Status"], removeNull=True), + ), + args_for_next_run=args, + continue_to_poll=False, + ) + push_status = raw_contents.get("response", {}).get("result", {}).get("job", {}).get("status", "") continue_to_poll = bool(push_status and push_status != "FIN") context_output = {"Status": push_status, "JobID": push_job_id} push_cr = CommandResults( @@ -950,7 +966,6 @@ def pan_os_push_status(args: dict, responses: list) -> PollResult: outputs=context_output, readable_output=tableToMarkdown("Push to Device Group:", context_output, ["JobID", "Status"], removeNull=True), ) - global POLLING POLLING = continue_to_poll return PollResult( response=push_cr, diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py index 5351b2d08b37..69d0175cba4f 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py @@ -1,3 +1,5 @@ +import json + import pytest from BlockDomain import ( ACTION_CREATED, @@ -17,6 +19,7 @@ is_valid_fqdn, is_wildcard, most_significant_action, + pan_os_push_status, validate_domains, ) @@ -659,3 +662,173 @@ def test_build_final_command_results_verbose_appends_command_hr(): assert result.outputs == rows assert "a.com" in result.readable_output assert result.readable_output.endswith("HR-one") + + +def test_pan_os_push_status_error_contents_is_terminal_failure(monkeypatch): + """ + Given: + - pan-os-push-status returns an error entry whose Contents is a plain string + (PAN-OS surfaces errors as a bare string instead of the nested status dict). + When: + - Calling pan_os_push_status. + Then: + - The function does NOT crash with 'str object has no attribute get'; it stops polling + (POLLING flipped to False) and reports a Failure status for the job. + """ + import BlockDomain + + monkeypatch.setattr( + BlockDomain.demisto, + "executeCommand", + lambda *a, **k: [err_entry("Failed to execute pan-os-push-status. Error: job not found")], + ) + + # The @polling_function decorator unwraps the PollResult and returns its CommandResults response + # at runtime, though the declared return type is still PollResult (hence the type: ignore below). + result = pan_os_push_status({"push_job_id": "123"}, []) + + assert BlockDomain.POLLING is False + assert result.outputs == {"JobID": "123", "Status": "Failure"} # type: ignore[attr-defined] + + +def test_pan_os_push_status_fin_stops_polling(monkeypatch): + """ + Given: + - pan-os-push-status returns a well-formed nested dict whose job status is 'FIN'. + When: + - Calling pan_os_push_status. + Then: + - Polling stops (POLLING flipped to False) and the reported job status is 'FIN'. + """ + import BlockDomain + + fin_entry = { + "Type": 1, + "Contents": {"response": {"result": {"job": {"status": "FIN"}}}}, + "HumanReadable": "", + "EntryContext": {}, + } + monkeypatch.setattr(BlockDomain.demisto, "executeCommand", lambda *a, **k: [fin_entry]) + + result = pan_os_push_status({"push_job_id": "456"}, []) + + assert BlockDomain.POLLING is False + assert result.outputs == {"Status": "FIN", "JobID": "456"} # type: ignore[attr-defined] + + +def _install_fake_context(monkeypatch): + """Back demisto.context()/setContext() with an in-memory dict, mirroring platform behavior. + + Returns the backing store so tests can inspect exactly what was serialized to context. + """ + import BlockDomain + + store: dict = {} + monkeypatch.setattr(BlockDomain.demisto, "context", lambda: dict(store)) + monkeypatch.setattr(BlockDomain.demisto, "setContext", lambda key, value: store.__setitem__(key, value)) + return store + + +def test_save_and_restore_responses_json_round_trip(monkeypatch): + """ + Given: + - A PanOs instance whose accumulated responses contain the realistic PAN-OS entry shape + (nested Contents dict, Metadata, None HumanReadable) written to context as JSON. + When: + - Calling save_responses on one polling cycle and restore_responses on the next. + Then: + - The context value is valid JSON (not a Python repr), and the responses survive the + json.dumps -> json.loads round-trip byte-for-byte equal to the reduced form. + """ + import BlockDomain + + store = _install_fake_context(monkeypatch) + + pan_os = _pan_os(["evil.example.com"]) + pan_os.responses = [ + [ + { + "Type": 1, + "Contents": {"response": {"result": {"job": {"status": "FIN", "id": "42"}}}}, + "HumanReadable": None, + "Metadata": {"instance": "Panorama_QA", "brand": "Panorama"}, + "EntryContext": {"dropped": "not serialized"}, + } + ] + ] + expected_reduced = pan_os.reduce_responses() + + pan_os.save_responses() + + # Stored value must be real JSON that json.loads can parse (would fail on a Python repr). + stored = store["panorama_responses"] + assert json.loads(stored) == expected_reduced + + # A fresh instance restoring from the same context recovers the reduced responses exactly. + fresh = _pan_os(["evil.example.com"]) + fresh.restore_responses() + assert fresh.responses == expected_reduced + + +def test_finish_reads_rows_and_responses_as_json_then_clears_context(monkeypatch): + """ + Given: + - Context holds block_domain_rows and panorama_responses that were written as JSON by a + previous polling cycle. + When: + - Calling finish(). + Then: + - The rows are parsed back from JSON and returned; the accumulated responses are restored + onto the instance for verbose output; and all polling context keys are cleared. + """ + import BlockDomain + + store = _install_fake_context(monkeypatch) + + rows = [ + { + "Domain": "evil.example.com", + "Brand": "Panorama", + "Instance": "Panorama_QA", + "Status": STATUS_DONE, + "Result": RESULT_SUCCESS, + "Action": ACTION_CREATED, + "RuleName": "Cortex - Block Domain", + "Message": "ok", + } + ] + responses = [[{"HumanReadable": "HR", "Contents": "ok", "Type": 1, "Metadata": None}]] + store["block_domain_rows"] = json.dumps(rows) + store["panorama_responses"] = json.dumps(responses) + store["commit_job_id"] = "999" + + pan_os = _pan_os(["evil.example.com"]) + result = pan_os.finish() + + assert result == rows + assert pan_os.responses == responses + # All polling context keys are cleared on finish. + assert store["commit_job_id"] == "" + assert store["push_job_id"] == "" + assert store["panorama_responses"] == "" + assert store["block_domain_rows"] == "" + + +def test_finish_tolerates_corrupt_context_data(monkeypatch): + """ + Given: + - Context holds a non-JSON (corrupt) block_domain_rows value. + When: + - Calling finish(). + Then: + - finish() does not raise; it degrades to an empty list and still clears context. + """ + store = _install_fake_context(monkeypatch) + store["block_domain_rows"] = "{not valid json" + store["panorama_responses"] = "also not json" + + pan_os = _pan_os(["evil.example.com"]) + result = pan_os.finish() + + assert result == [] + assert store["block_domain_rows"] == "" From 7a843a07f15fed4bab89a9727f993d850dec15f5 Mon Sep 17 00:00:00 2001 From: nbensalm-palo Date: Mon, 24 Aug 2026 11:34:29 +0300 Subject: [PATCH 17/20] Git CR changes --- .../Scripts/BlockDomain/BlockDomain.py | 22 +++++++------ .../Scripts/BlockDomain/BlockDomain.yml | 33 +++++++++---------- .../Scripts/BlockDomain/BlockDomain_test.py | 32 +++++++----------- .../Scripts/BlockDomain/README.md | 24 +++++++------- 4 files changed, 51 insertions(+), 60 deletions(-) diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py index 95b03497c54c..20018b08d7ea 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py @@ -113,7 +113,7 @@ def build_result_row( instance: str = "", rule_name: str = "", ) -> dict: - """Assemble a single BlockDomainResults row. + """Assemble a single BlockDomain row. Args: domain (str): The processed domain. @@ -264,12 +264,12 @@ def build_verbose_human_readable(responses: list) -> str: def build_final_command_results(rows: list, verbose: bool, responses: list) -> CommandResults: """Build the single final CommandResults for the run. - The CommandResults carries the aggregated BlockDomainResults context and a markdown summary table. + The CommandResults carries the aggregated BlockDomain context and a markdown summary table. When verbose is True, the per-command human-readable outputs are appended to the same readable output (blank-line separated), mirroring the ExpirePassword aggregated script. Args: - rows (list): The aggregated BlockDomainResults rows. + rows (list): The aggregated BlockDomain rows. verbose (bool): Whether to append per-command human-readable output. responses (list): The accumulated command responses (used only when verbose). Returns: @@ -284,7 +284,7 @@ def build_final_command_results(rows: list, verbose: bool, responses: list) -> C if verbose: readable_output += build_verbose_human_readable(responses) return CommandResults( - outputs_prefix="BlockDomainResults", + outputs_prefix="BlockDomain", outputs_key_field=["Domain", "Brand", "Instance"], outputs=rows, readable_output=readable_output, @@ -308,7 +308,7 @@ class PanOs: significant change. """ - def __init__(self, args: dict): + def __init__(self, args: dict) -> None: """Initialize the PanOs flow. Args: @@ -569,7 +569,7 @@ def process_domains(self) -> list: """Ensure the group and rule once, then loop over domains adding each object. Returns: - The list of BlockDomainResults rows for the processed domains. + The list of BlockDomain rows for the processed domains. """ rows: list = [] try: @@ -618,6 +618,7 @@ def process_domains(self) -> list: ) ) except Exception as ex: + demisto.error(f"{LOG_TAG} process_domains failed: {traceback.format_exc()}") for domain in self.domains: rows.append( build_result_row( @@ -784,7 +785,7 @@ def finish(self) -> list: # pragma: no cover instance so the caller can build verbose output before they are cleared from context. Returns: - The list of BlockDomainResults rows accumulated for the run. + The list of BlockDomain rows accumulated for the run. """ rows_raw = demisto.context().get("block_domain_rows", "") or "" # Preserve responses on the instance for verbose output before clearing context. @@ -792,8 +793,8 @@ def finish(self) -> list: # pragma: no cover if stored: try: self.responses = json.loads(stored) - except (ValueError, TypeError): - pass + except (ValueError, TypeError) as err: + demisto.debug(f"{LOG_TAG} Could not parse stored responses from context; ignoring. Error: {err}") demisto.setContext("commit_job_id", "") demisto.setContext("push_job_id", "") demisto.setContext("panorama_responses", "") @@ -977,7 +978,7 @@ def pan_os_push_status(args: dict, responses: list) -> PollResult: """ MAIN FUNCTION """ -def main(): # pragma: no cover +def main() -> None: # pragma: no cover try: args = demisto.args() demisto.debug(f"{LOG_TAG} block-domain invoked with {args=}") @@ -1059,6 +1060,7 @@ def main(): # pragma: no cover return_results(build_final_command_results(results, verbose, command_responses)) except Exception as ex: + demisto.error(f"{LOG_TAG} block-domain failed: {traceback.format_exc()}") return_error(f"Failed to execute block-domain. Error: {ex!s}") diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.yml b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.yml index e29033230110..42c0cd0792d3 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.yml +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.yml @@ -1,5 +1,5 @@ args: -- description: List of domain FQDNs to block. Wildcard entries (e.g. *.evil.com) are not supported and are skipped. +- description: A comma-separated list of domain FQDNs to block. Wildcard entries (e.g. *.evil.com) are not supported and are skipped. isArray: true name: domain_list required: true @@ -8,11 +8,11 @@ args: isArray: false name: rule_name required: false -- description: Panorama log forwarding object name. Indicate what type of Log Forwarding setting will be specified in the PAN-OS custom rules. +- description: The Panorama log forwarding object name that specifies the Log Forwarding setting to apply to the PAN-OS custom rules. isArray: false name: log_forwarding_name required: false -- description: This input determines whether PANW Panorama or Firewall Address Groups are used. Specify the Address Group name for FQDN handling. +- description: The name of the PAN-OS Panorama or Firewall address group used to hold the blocked domain FQDN objects. isArray: false name: address_group required: false @@ -32,8 +32,7 @@ args: required: false defaultValue: 'cortex-blocked-domains' - description: |- - Which integrations brands to run the command for. If not provided, the command will run for all available integrations. - For multi-select provide a comma-separated list. + A comma-separated list of integration brands to run the command for. If not provided, the command runs for all available integrations. isArray: true name: brands required: false @@ -47,41 +46,41 @@ args: predefined: - 'true' - 'false' -- description: commit job ID to use in polling commands. (automatically filled by polling). +- description: The commit job ID to use in polling commands. Automatically filled by polling. name: commit_job_id hidden: true -- description: publish job ID to use in polling commands. (automatically filled by polling). - name: publish_job_id +- description: The push job ID to use in polling commands. Automatically filled by polling. + name: push_job_id hidden: true -comment: The script blocks a list of domain FQDNs in supported integrations. +comment: Blocks a list of domain FQDNs across the configured security products. commonfields: id: block-domain version: -1 enabled: false name: block-domain outputs: -- contextPath: BlockDomainResults.Domain +- contextPath: BlockDomain.Domain description: The domain FQDN that was processed. type: String -- contextPath: BlockDomainResults.Brand +- contextPath: BlockDomain.Brand description: The brand (integration) used to block the domain. type: String -- contextPath: BlockDomainResults.Instance +- contextPath: BlockDomain.Instance description: The integration instance used to block the domain. type: String -- contextPath: BlockDomainResults.Status +- contextPath: BlockDomain.Status description: The lifecycle status of the action. One of Done, Pending, Skipped, Failed. type: String -- contextPath: BlockDomainResults.Result +- contextPath: BlockDomain.Result description: The result of the action. Success or Failed. type: String -- contextPath: BlockDomainResults.Action +- contextPath: BlockDomain.Action description: The effect the run had on the target object. One of Created, Modified, Unchanged. type: String -- contextPath: BlockDomainResults.RuleName +- contextPath: BlockDomain.RuleName description: The name of the rule used for this integration. Empty if no rule was used. type: String -- contextPath: BlockDomainResults.Message +- contextPath: BlockDomain.Message description: A message concerning the result of the action. type: String script: '-' diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py index 69d0175cba4f..6e2c576513e0 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py @@ -1,6 +1,8 @@ import json import pytest + +import BlockDomain from BlockDomain import ( ACTION_CREATED, ACTION_MODIFIED, @@ -222,8 +224,6 @@ def _pan_os(domains): def _mock_execute(monkeypatch, side_effect): """Patch BlockDomain.demisto.executeCommand to yield the given responses in order.""" - import BlockDomain - responses = iter(side_effect) monkeypatch.setattr(BlockDomain.demisto, "executeCommand", lambda *a, **k: next(responses)) @@ -271,8 +271,6 @@ def test_start_flow_skips_commit_when_all_actions_unchanged(monkeypatch): - pan_os_commit is never invoked; start_flow returns the Unchanged rows directly (skipping the multi-minute commit + push polling cycle on Panorama). """ - import BlockDomain - calls: list = [] def _capture(name, args): @@ -321,8 +319,6 @@ def test_start_flow_commits_when_at_least_one_row_modified(monkeypatch): Then: - pan_os_commit is invoked (candidate config was modified and must be pushed). """ - import BlockDomain - commit_called: list = [] def _fake_commit(args, responses): @@ -415,8 +411,6 @@ def _capture(name, args): } return seq.get(name, [ok_entry()]) - import BlockDomain - monkeypatch.setattr(BlockDomain.demisto, "executeCommand", _capture) _pan_os(["evil.example.com"]).process_domains() @@ -550,7 +544,8 @@ def test_process_domains_failure_marks_row_failed(monkeypatch): When: - Calling process_domains. Then: - - The resulting row is Failed / Failed and the error message from PAN-OS is surfaced. + - The resulting row is Failed / Failed and the error message from PAN-OS is surfaced, + and the full traceback is logged via demisto.error. """ _mock_execute( monkeypatch, @@ -561,10 +556,15 @@ def test_process_domains_failure_marks_row_failed(monkeypatch): [err_entry("permission denied")], ], ) + # Capture the traceback log so it does not leak to stdout (conftest fails on any stdout). + errors: list = [] + monkeypatch.setattr(BlockDomain.demisto, "error", lambda msg: errors.append(msg)) + rows = _pan_os(["evil.example.com"]).process_domains() assert rows[0]["Status"] == STATUS_FAILED assert rows[0]["Result"] == RESULT_FAILED assert "permission denied" in rows[0]["Message"] + assert any("process_domains failed" in msg for msg in errors) def test_build_verbose_human_readable_joins_with_blank_lines(): @@ -608,7 +608,7 @@ def test_build_final_command_results_non_verbose_is_table_only(): When: - Calling build_final_command_results. Then: - - The returned CommandResults uses the BlockDomainResults context prefix, exposes the + - The returned CommandResults uses the BlockDomain context prefix, exposes the rows unchanged as outputs, renders the summary table containing the domain, and does NOT append any of the per-command verbose HR blocks. """ @@ -627,7 +627,7 @@ def test_build_final_command_results_non_verbose_is_table_only(): responses = [[{"Type": 1, "Contents": "c", "HumanReadable": "HR", "EntryContext": {}}]] result = build_final_command_results(rows, verbose=False, responses=responses) - assert result.outputs_prefix == "BlockDomainResults" + assert result.outputs_prefix == "BlockDomain" assert result.outputs == rows assert "a.com" in result.readable_output assert "HR" not in result.readable_output @@ -675,8 +675,6 @@ def test_pan_os_push_status_error_contents_is_terminal_failure(monkeypatch): - The function does NOT crash with 'str object has no attribute get'; it stops polling (POLLING flipped to False) and reports a Failure status for the job. """ - import BlockDomain - monkeypatch.setattr( BlockDomain.demisto, "executeCommand", @@ -700,8 +698,6 @@ def test_pan_os_push_status_fin_stops_polling(monkeypatch): Then: - Polling stops (POLLING flipped to False) and the reported job status is 'FIN'. """ - import BlockDomain - fin_entry = { "Type": 1, "Contents": {"response": {"result": {"job": {"status": "FIN"}}}}, @@ -721,8 +717,6 @@ def _install_fake_context(monkeypatch): Returns the backing store so tests can inspect exactly what was serialized to context. """ - import BlockDomain - store: dict = {} monkeypatch.setattr(BlockDomain.demisto, "context", lambda: dict(store)) monkeypatch.setattr(BlockDomain.demisto, "setContext", lambda key, value: store.__setitem__(key, value)) @@ -740,8 +734,6 @@ def test_save_and_restore_responses_json_round_trip(monkeypatch): - The context value is valid JSON (not a Python repr), and the responses survive the json.dumps -> json.loads round-trip byte-for-byte equal to the reduced form. """ - import BlockDomain - store = _install_fake_context(monkeypatch) pan_os = _pan_os(["evil.example.com"]) @@ -781,8 +773,6 @@ def test_finish_reads_rows_and_responses_as_json_then_clears_context(monkeypatch - The rows are parsed back from JSON and returned; the accumulated responses are restored onto the instance for verbose output; and all polling context keys are cleared. """ - import BlockDomain - store = _install_fake_context(monkeypatch) rows = [ diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/README.md b/Packs/AggregatedScripts/Scripts/BlockDomain/README.md index 4706a8db9fae..5b7e34b09d59 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/README.md +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/README.md @@ -15,13 +15,13 @@ The script blocks a list of domain FQDNs in supported integrations. Safe to re-r | **Argument Name** | **Description** | | --- | --- | -| domain_list | List of domain FQDNs to block. Wildcard entries \(e.g. \*.evil.com\) are not supported and are skipped. | +| domain_list | A comma-separated list of domain FQDNs to block. Wildcard entries \(e.g. \*.evil.com\) are not supported and are skipped. | | rule_name | The name of the rule which will be created in the relevant products. Default: `Cortex - Block Domain`. | -| log_forwarding_name | Panorama log forwarding object name. Indicate what type of Log Forwarding setting will be specified in the PAN-OS custom rules. | -| address_group | Address Group name used to hold the blocked domain objects. Default: `Blocked Domains - Cortex`. | +| log_forwarding_name | The Panorama log forwarding object name that specifies the Log Forwarding setting to apply to the PAN-OS custom rules. | +| address_group | The name of the PAN-OS Panorama or Firewall address group used to hold the blocked domain FQDN objects. Default: `Blocked Domains - Cortex`. | | auto_commit | Whether to commit the new rule and push to the device group at the end of the run. Default: `true`. | | tag | The designated tag name for the domain FQDN object. Applied to every object the script creates. Default: `cortex-blocked-domains`. | -| brands | Which integration brands to run the command for. If not provided, the command will run for all available integrations.
For multi-select provide a comma-separated list. Default: `Panorama`. | +| brands | A comma-separated list of integration brands to run the command for. If not provided, the command runs for all available integrations. | | verbose | Whether to retrieve a human-readable entry for every command or only the final result. True retrieves a human-readable entry for every command. False retrieves a human-readable entry only for the final result. Default: `false`. | ## Outputs @@ -30,11 +30,11 @@ The script blocks a list of domain FQDNs in supported integrations. Safe to re-r | **Path** | **Description** | **Type** | | --- | --- | --- | -| BlockDomainResults.Domain | The domain FQDN that was processed. | String | -| BlockDomainResults.Brand | The brand \(integration\) used to block the domain. | String | -| BlockDomainResults.Instance | The integration instance used to block the domain. | String | -| BlockDomainResults.Status | The lifecycle status of the action. One of Done, Pending, Skipped, Failed. | String | -| BlockDomainResults.Result | The result of the action. Success or Failed. | String | -| BlockDomainResults.Action | The effect the run had on the target object. One of Created, Modified, Unchanged. | String | -| BlockDomainResults.RuleName | The name of the rule used for this integration. Empty if no rule was used. | String | -| BlockDomainResults.Message | A message concerning the result of the action. | String | +| BlockDomain.Domain | The domain FQDN that was processed. | String | +| BlockDomain.Brand | The brand \(integration\) used to block the domain. | String | +| BlockDomain.Instance | The integration instance used to block the domain. | String | +| BlockDomain.Status | The lifecycle status of the action. One of Done, Pending, Skipped, Failed. | String | +| BlockDomain.Result | The result of the action. Success or Failed. | String | +| BlockDomain.Action | The effect the run had on the target object. One of Created, Modified, Unchanged. | String | +| BlockDomain.RuleName | The name of the rule used for this integration. Empty if no rule was used. | String | +| BlockDomain.Message | A message concerning the result of the action. | String | From 38b7d83b94c34dacc9955adcf1bad834903a921d Mon Sep 17 00:00:00 2001 From: nbensalm-palo Date: Mon, 24 Aug 2026 13:21:49 +0300 Subject: [PATCH 18/20] adding pack ignore --- Packs/AggregatedScripts/.pack-ignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Packs/AggregatedScripts/.pack-ignore b/Packs/AggregatedScripts/.pack-ignore index 7b5bf9e92e79..ce952abf623a 100644 --- a/Packs/AggregatedScripts/.pack-ignore +++ b/Packs/AggregatedScripts/.pack-ignore @@ -1,6 +1,9 @@ [file:DisableUser.py] ignore=PA124 +[file:BlockDomain.py] +ignore=PA124 + [file:IsolateEndpoint.yml] ignore=SC101 From ae814d2634574c942fb4900fec152c558fc0e77a Mon Sep 17 00:00:00 2001 From: nbensalm-palo Date: Mon, 24 Aug 2026 14:59:26 +0300 Subject: [PATCH 19/20] update pa124 --- Packs/AggregatedScripts/.pack-ignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Packs/AggregatedScripts/.pack-ignore b/Packs/AggregatedScripts/.pack-ignore index ce952abf623a..813606cf89b7 100644 --- a/Packs/AggregatedScripts/.pack-ignore +++ b/Packs/AggregatedScripts/.pack-ignore @@ -1,7 +1,7 @@ [file:DisableUser.py] ignore=PA124 -[file:BlockDomain.py] +[file:BlockDomain.yml] ignore=PA124 [file:IsolateEndpoint.yml] From 8056285db8d7ac55ca5b2770aa8a85709126069f Mon Sep 17 00:00:00 2001 From: nbensalm-palo Date: Mon, 24 Aug 2026 15:43:52 +0300 Subject: [PATCH 20/20] Fixing edge case of new rules --- .../Scripts/BlockDomain/BlockDomain.py | 8 +- .../Scripts/BlockDomain/BlockDomain_test.py | 79 +++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py index 20018b08d7ea..4c3925dd8288 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain.py @@ -326,6 +326,8 @@ def __init__(self, args: dict) -> None: # Rule create is deferred until the group exists (destination validation); this guard # prevents ensure_rule from running twice per run. self._rule_ensured: bool = False + # True once the rule was created or edited this run + self._rule_changed: bool = False # Captured lazily from the first response's Metadata.instance; stamped on every row. self.instance_name: str = "" @@ -463,6 +465,7 @@ def ensure_rule(self, rule_present: bool, rule_destinations: list) -> None: if self.log_forwarding_name: create_rule_args["log_forwarding"] = self.log_forwarding_name self.execute_or_raise("pan-os-create-rule", create_rule_args, f"Failed to create rule '{self.rule_name}'") + self._rule_changed = True elif self.address_group not in rule_destinations: # Rule exists but doesn't reference our group - add without replacing existing destinations. self.execute_or_raise( @@ -476,6 +479,7 @@ def ensure_rule(self, rule_present: bool, rule_destinations: list) -> None: }, f"Failed to add group to rule '{self.rule_name}'", ) + self._rule_changed = True # Always enforce top placement. self.execute_or_raise( "pan-os-move-rule", @@ -767,7 +771,9 @@ def start_flow(self) -> Any: # pragma: no cover demisto.setContext("block_domain_rows", json.dumps(rows)) # Only commit/push when a row actually mutated Panorama state. Skipping on a pure # Unchanged run saves a commit job + a potentially multi-minute push polling loop. - made_changes = any(row.get("Action") in (ACTION_CREATED, ACTION_MODIFIED) for row in rows) + # _rule_changed covers rule create/edit, which is not reflected in per-domain Action. + object_changes = any(row.get("Action") in (ACTION_CREATED, ACTION_MODIFIED) for row in rows) + made_changes = object_changes or self._rule_changed auto_commit = argToBoolean(self.args.get("auto_commit", True)) demisto.debug(f"{LOG_TAG} start_flow: {made_changes=}, {auto_commit=}, {len(rows)} row(s)") if made_changes and auto_commit: diff --git a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py index 6e2c576513e0..4d3dbabef28b 100644 --- a/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py +++ b/Packs/AggregatedScripts/Scripts/BlockDomain/BlockDomain_test.py @@ -353,6 +353,85 @@ def _capture(name, args): assert commit_called, "pan_os_commit must be called when at least one row is Created/Modified" +def test_process_domains_rule_created_when_object_unchanged_sets_rule_changed(monkeypatch): + """ + Given: + - The address object and group already exist and the object is already a member (so the + per-domain Action is Unchanged), but the requested rule_name does NOT exist yet, so a + new rule must be created. + When: + - Calling process_domains. + Then: + - The row Action is Unchanged (object/membership did not change), but the instance's + _rule_changed flag is set True because pan-os-create-rule ran. This is what lets + start_flow still commit a rule-only change. + """ + obj = "Cortex-evil.example.com" + + def _capture(name, args): + seq = { + # Group exists and already contains the object -> membership Unchanged. + "pan-os-list-address-groups": [ + ok_entry({"Panorama.AddressGroups": [{"Name": "Blocked Domains - Cortex", "Type": "static", "Addresses": [obj]}]}) + ], + # No rule with the requested name exists -> ensure_rule will create it. + "pan-os-list-rules": [ok_entry({"Panorama.SecurityRule": []})], + "pan-os-get-address": [ok_entry({"Panorama.Addresses": [{"Name": obj}]})], # object exists + "pan-os-create-rule": [ok_entry()], + "pan-os-move-rule": [ok_entry()], + } + return seq.get(name, [ok_entry()]) + + monkeypatch.setattr(BlockDomain.demisto, "executeCommand", _capture) + + pan_os = _pan_os(["evil.example.com"]) + rows = pan_os.process_domains() + + assert rows[0]["Action"] == ACTION_UNCHANGED + assert pan_os._rule_changed is True + + +def test_start_flow_commits_when_only_the_rule_changed(monkeypatch): + """ + Given: + - Every address object is already present and a member (all rows Unchanged), but a new + rule had to be created this run (rule-only change). + When: + - Calling start_flow. + Then: + - pan_os_commit is still invoked, so the newly created rule is actually committed/pushed + instead of silently sitting in the candidate config. + """ + obj = "Cortex-evil.example.com" + commit_called: list = [] + + def _fake_commit(args, responses): + commit_called.append(True) + BlockDomain.POLLING = False + return BlockDomain.CommandResults(readable_output="fake commit ok") + + def _capture(name, args): + seq = { + "pan-os-list-address-groups": [ + ok_entry({"Panorama.AddressGroups": [{"Name": "Blocked Domains - Cortex", "Type": "static", "Addresses": [obj]}]}) + ], + "pan-os-list-rules": [ok_entry({"Panorama.SecurityRule": []})], + "pan-os-get-address": [ok_entry({"Panorama.Addresses": [{"Name": obj}]})], + "pan-os-create-rule": [ok_entry()], + "pan-os-move-rule": [ok_entry()], + } + return seq.get(name, [ok_entry()]) + + monkeypatch.setattr(BlockDomain.demisto, "executeCommand", _capture) + monkeypatch.setattr(BlockDomain.demisto, "setContext", lambda *a, **k: None) + monkeypatch.setattr(BlockDomain, "pan_os_commit", _fake_commit) + monkeypatch.setattr(BlockDomain.demisto, "context", lambda: {"block_domain_rows": "[]"}) + + _pan_os(["evil.example.com"]).start_flow() + + assert commit_called, "pan_os_commit must be called when only the rule changed (all objects Unchanged)" + + def test_process_domains_captures_instance_name_from_response_metadata(monkeypatch): """ Given: