Skip to content

fix: type hinting fixes and additional code checks #4790

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 8 additions & 9 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
# detection-rules code owners
# POC: Elastic Security Intelligence and Analytics Team

tests/**/*.py @mikaayenson @eric-forte-elastic @terrancedejesus
detection_rules/ @mikaayenson @eric-forte-elastic @terrancedejesus
tests/ @mikaayenson @eric-forte-elastic @terrancedejesus
lib/ @mikaayenson @eric-forte-elastic @terrancedejesus
hunting/ @mikaayenson @eric-forte-elastic @terrancedejesus
tests/**/*.py @mikaayenson @eric-forte-elastic @traut
detection_rules/ @mikaayenson @eric-forte-elastic @traut
tests/ @mikaayenson @eric-forte-elastic @traut
lib/ @mikaayenson @eric-forte-elastic @traut
hunting/ @mikaayenson @eric-forte-elastic @traut

# skip rta-mapping to avoid the spam
detection_rules/etc/packages.yaml @mikaayenson @eric-forte-elastic @terrancedejesus
detection_rules/etc/*.json @mikaayenson @eric-forte-elastic @terrancedejesus
detection_rules/etc/*.json @mikaayenson @eric-forte-elastic @terrancedejesus
detection_rules/etc/*/* @mikaayenson @eric-forte-elastic @terrancedejesus
detection_rules/etc/packages.yaml @mikaayenson @eric-forte-elastic @traut
detection_rules/etc/*.json @mikaayenson @eric-forte-elastic @traut
detection_rules/etc/*/* @mikaayenson @eric-forte-elastic @traut
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
detection_rules/etc/*/* @mikaayenson @eric-forte-elastic @traut
detection_rules/etc/*/* @mikaayenson @eric-forte-elastic @traut
# exclude files from code owners
detection_rules/etc/non-ecs-schema.json

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per our team discussion in the team sync today.

47 changes: 47 additions & 0 deletions .github/workflows/code-checks.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
name: Code checks

on:
push:
branches: [ "main", "7.*", "8.*", "9.*" ]
pull_request:
branches: [ "*" ]
paths:
- 'detection_rules/**/*.py'
- 'hunting/**/*.py'

jobs:
code-checks:

runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v4
with:
fetch-depth: 1

- name: Set up Python 3.13
uses: actions/setup-python@v5
with:
python-version: '3.13'

- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip cache purge
pip install .[dev]
- name: Linting check
run: |
ruff check --exit-non-zero-on-fix
- name: Formatting check
run: |
ruff format --check
- name: Pyright check
run: |
pyright
- name: Python License Check
run: |
python -m detection_rules dev license-check
12 changes: 2 additions & 10 deletions .github/workflows/pythonpackage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,25 +20,17 @@ jobs:
run: |
git fetch origin main:refs/remotes/origin/main
- name: Set up Python 3.12
- name: Set up Python 3.13
uses: actions/setup-python@v5
with:
python-version: '3.12'
python-version: '3.13'

- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip cache purge
pip install .[dev]
- name: Python Lint
run: |
python -m flake8 tests detection_rules --ignore D203,N815 --max-line-length 120
- name: Python License Check
run: |
python -m detection_rules dev license-check
- name: Unit tests
env:
# only run the test test_rule_change_has_updated_date on pull request events to main
Expand Down
30 changes: 15 additions & 15 deletions detection_rules/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,23 +25,23 @@
rule_formatter,
rule_loader,
schemas,
utils
utils,
)

__all__ = (
'custom_rules',
'custom_schemas',
'devtools',
'docs',
'eswrap',
'ghwrap',
'kbwrap',
"custom_rules",
"custom_schemas",
"devtools",
"docs",
"eswrap",
"ghwrap",
"kbwrap",
"main",
'misc',
'ml',
'navigator',
'rule_formatter',
'rule_loader',
'schemas',
'utils'
"misc",
"ml",
"navigator",
"rule_formatter",
"rule_loader",
"schemas",
"utils",
)
1 change: 1 addition & 0 deletions detection_rules/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

# coding=utf-8
"""Shell for detection-rules."""

import sys
from pathlib import Path

Expand Down
25 changes: 16 additions & 9 deletions detection_rules/action.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@
# 2.0.

"""Dataclasses for Action."""

from typing import Any
from dataclasses import dataclass
from pathlib import Path
from typing import List, Optional

from .mixins import MarshmallowDataclassMixin
from .schemas import definitions
Expand All @@ -15,43 +16,49 @@
@dataclass(frozen=True)
class ActionMeta(MarshmallowDataclassMixin):
"""Data stored in an exception's [metadata] section of TOML."""

creation_date: definitions.Date
rule_id: List[definitions.UUIDString]
rule_id: list[definitions.UUIDString]
rule_name: str
updated_date: definitions.Date

# Optional fields
deprecation_date: Optional[definitions.Date]
comments: Optional[str]
maturity: Optional[definitions.Maturity]
deprecation_date: definitions.Date | None
comments: str | None
maturity: definitions.Maturity | None


@dataclass
class Action(MarshmallowDataclassMixin):
"""Data object for rule Action."""

@dataclass
class ActionParams:
"""Data object for rule Action params."""

body: str

action_type_id: definitions.ActionTypeId
group: str
params: ActionParams
id: Optional[str]
frequency: Optional[dict]
alerts_filter: Optional[dict]

id: str | None
frequency: dict[str, Any] | None
alerts_filter: dict[str, Any] | None


@dataclass(frozen=True)
class TOMLActionContents(MarshmallowDataclassMixin):
"""Object for action from TOML file."""

metadata: ActionMeta
actions: List[Action]
actions: list[Action]


@dataclass(frozen=True)
class TOMLAction:
"""Object for action from TOML file."""

contents: TOMLActionContents
path: Path

Expand Down
70 changes: 40 additions & 30 deletions detection_rules/action_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,13 @@
# 2.0.

"""Dataclasses for Action."""

from dataclasses import dataclass
from datetime import datetime
from pathlib import Path
from typing import List, Optional, Tuple
from typing import Any

import pytoml
import pytoml # type: ignore[reportMissingTypeStubs]
from marshmallow import EXCLUDE

from .mixins import MarshmallowDataclassMixin
Expand All @@ -25,40 +26,42 @@ class ActionConnectorMeta(MarshmallowDataclassMixin):

creation_date: definitions.Date
action_connector_name: str
rule_ids: List[definitions.UUIDString]
rule_names: List[str]
rule_ids: list[definitions.UUIDString]
rule_names: list[str]
updated_date: definitions.Date

# Optional fields
deprecation_date: Optional[definitions.Date]
comments: Optional[str]
maturity: Optional[definitions.Maturity]
deprecation_date: definitions.Date | None
comments: str | None
maturity: definitions.Maturity | None


@dataclass
class ActionConnector(MarshmallowDataclassMixin):
"""Data object for rule Action Connector."""

id: str
attributes: dict
frequency: Optional[dict]
managed: Optional[bool]
type: Optional[str]
references: Optional[List]
attributes: dict[str, Any]
frequency: dict[str, Any] | None
managed: bool | None
type: str | None
references: list[Any] | None


@dataclass(frozen=True)
class TOMLActionConnectorContents(MarshmallowDataclassMixin):
"""Object for action connector from TOML file."""

metadata: ActionConnectorMeta
action_connectors: List[ActionConnector]
action_connectors: list[ActionConnector]

@classmethod
def from_action_connector_dict(cls, actions_dict: dict, rule_list: dict) -> "TOMLActionConnectorContents":
def from_action_connector_dict(
cls, actions_dict: dict[str, Any], rule_list: list[dict[str, Any]]
) -> "TOMLActionConnectorContents":
"""Create a TOMLActionContents from a kibana rule resource."""
rule_ids = []
rule_names = []
rule_ids: list[str] = []
rule_names: list[str] = []

for rule in rule_list:
rule_ids.append(rule["id"])
Expand All @@ -77,9 +80,9 @@ def from_action_connector_dict(cls, actions_dict: dict, rule_list: dict) -> "TOM

return cls.from_dict({"metadata": metadata, "action_connectors": [actions_dict]}, unknown=EXCLUDE)

def to_api_format(self) -> List[dict]:
def to_api_format(self) -> list[dict[str, Any]]:
"""Convert the TOML Action Connector to the API format."""
converted = []
converted: list[dict[str, Any]] = []

for action in self.action_connectors:
converted.append(action.to_dict())
Expand Down Expand Up @@ -109,13 +112,15 @@ def save_toml(self):
contents_dict = self.contents.to_dict()
# Sort the dictionary so that 'metadata' is at the top
sorted_dict = dict(sorted(contents_dict.items(), key=lambda item: item[0] != "metadata"))
pytoml.dump(sorted_dict, f)
pytoml.dump(sorted_dict, f) # type: ignore[reportUnknownMemberType]


def parse_action_connector_results_from_api(results: List[dict]) -> tuple[List[dict], List[dict]]:
def parse_action_connector_results_from_api(
results: list[dict[str, Any]],
) -> tuple[list[dict[str, Any]], list[dict[str, Any]]]:
"""Filter Kibana export rule results for action connector dictionaries."""
action_results = []
non_action_results = []
action_results: list[dict[str, Any]] = []
non_action_results: list[dict[str, Any]] = []
for result in results:
if result.get("type") != "action":
non_action_results.append(result)
Expand All @@ -125,17 +130,21 @@ def parse_action_connector_results_from_api(results: List[dict]) -> tuple[List[d
return action_results, non_action_results


def build_action_connector_objects(action_connectors: List[dict], action_connector_rule_table: dict,
action_connectors_directory: Path, save_toml: bool = False,
skip_errors: bool = False, verbose=False,
) -> Tuple[List[TOMLActionConnector], List[str], List[str]]:
def build_action_connector_objects(
action_connectors: list[dict[str, Any]],
action_connector_rule_table: dict[str, Any],
action_connectors_directory: Path | None,
save_toml: bool = False,
skip_errors: bool = False,
verbose: bool = False,
) -> tuple[list[TOMLActionConnector], list[str], list[str]]:
"""Build TOMLActionConnector objects from a list of action connector dictionaries."""
output = []
errors = []
toml_action_connectors = []
output: list[str] = []
errors: list[str] = []
toml_action_connectors: list[TOMLActionConnector] = []
for action_connector_dict in action_connectors:
try:
connector_id = action_connector_dict.get("id")
connector_id = action_connector_dict["id"]
rule_list = action_connector_rule_table.get(connector_id)
if not rule_list:
output.append(f"Warning action connector {connector_id} has no associated rules. Loading skipped.")
Expand All @@ -161,6 +170,7 @@ def build_action_connector_objects(action_connectors: List[dict], action_connect
)
if save_toml:
ac_object.save_toml()

toml_action_connectors.append(ac_object)

except Exception as e:
Expand Down
Loading