Add Unprotect.it API support for YARA rules. Closes #1711#3229
Add Unprotect.it API support for YARA rules. Closes #1711#3229Gagan144-blip wants to merge 45 commits intointelowlproject:developfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for sourcing YARA rules from the Unprotect.it Detection Rules API (instead of only git/zip sources), enabling periodic fetch + local compilation into intel_owl_compiled.yas.
Changes:
- Added Unprotect.it API URL detection and routing in
YaraRepo.update() - Implemented paginated API fetch and filtering for YARA-only rules, storing each rule as a local
.yar - Integrated API-fetched rules into the existing compile/analyze flow
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def is_unprotect_api(self) ->bool: | ||
| return "unprotect.it/api/detection_rules" in self.url |
There was a problem hiding this comment.
The new Unprotect.it methods aren’t formatted consistently with the rest of this file (which appears Black-formatted), e.g. def is_unprotect_api(self) -> bool: spacing. Please run Black/isort (or adjust formatting) so CI/style checks don’t fail.
| if settings.GIT_KEY_PATH.exists(): | ||
| os.remove(settings.GIT_KEY_PATH) | ||
|
|
||
| # NEW: Unprotect.it API unpdate logic |
There was a problem hiding this comment.
Typo in comment: “unpdate” -> “update”.
| # NEW: Unprotect.it API unpdate logic | |
| # NEW: Unprotect.it API update logic |
| def _update_unprotect_api(self): | ||
| logger.info(f"Fetching rules from Unprotect.it API: {self.url}") | ||
|
|
||
| os.makedirs(self.directory, exist_ok=True) | ||
|
|
||
| next_url = self.url | ||
| while next_url: | ||
| try: | ||
| response = requests.get(next_url, timeout=30) | ||
| response.raise_for_status() | ||
| data = response.json() | ||
| except Exception as e: | ||
| logger.exception("Failed to fetch Unprotect.it rules") | ||
| return | ||
|
|
||
| rules = data.get("results", []) | ||
| next_url = data.get("next") | ||
|
|
There was a problem hiding this comment.
New Unprotect.it pagination/filtering behavior isn’t covered by tests. Given existing tests for YaraScan (e.g., tests/api_app/analyzers_manager/unit_tests/file_analyzers/test_yara_scan.py and cron updater tests), add a unit test for _update_unprotect_api() that mocks paginated requests.get responses, verifies YARA-only filtering, and asserts files are written/updated correctly (including cleanup of stale rules if implemented).
| logger.info(f"Fetching rules from Unprotect.it API: {self.url}") | ||
|
|
||
| os.makedirs(self.directory, exist_ok=True) | ||
|
|
There was a problem hiding this comment.
_update_unprotect_api() writes new .yar files but never removes existing rule files in self.directory. If rules are removed/renamed upstream, stale local .yar files will keep being compiled and scanned. Consider deleting existing .yar/.yara/.rule files (and compiled .yas) in this directory before fetching pages to ensure the local set matches the API snapshot.
| # Ensure local rules mirror the current Unprotect.it snapshot by | |
| # removing any existing YARA rule/compiled files before fetching. | |
| if hasattr(self, "directory"): | |
| try: | |
| for pattern in ("*.yar", "*.yara", "*.rule", "*.yas"): | |
| for existing_file in self.directory.glob(pattern): | |
| try: | |
| os.remove(existing_file) | |
| except OSError: | |
| logger.warning( | |
| "Failed to remove stale rule file %s", existing_file | |
| ) | |
| except Exception: | |
| # Log and continue; fetching new rules is still attempted. | |
| logger.exception( | |
| "Unexpected error while cleaning up existing Unprotect.it rules" | |
| ) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| safe_name =( | ||
| rule_name.lower() | ||
| .replace(" ", "_") | ||
| .replace("/", "_") | ||
| .replace("\\", "_") |
There was a problem hiding this comment.
safe_name is derived only from rule_name and can collide (e.g., case/whitespace differences), causing silent overwrites. Prefer incorporating a stable unique identifier from the API payload (e.g., id) into the filename and apply stricter sanitization to avoid filesystem issues with unexpected characters/very long names.
| if self.is_unprotect_api(): | ||
| self._update_unprotect_api() | ||
| return |
There was a problem hiding this comment.
New Unprotect.it update behavior (URL detection + pagination + yara-only filtering) is introduced here, but existing unit tests for yara_scan.py don't cover this path. Add tests that mock paginated requests.get responses and assert only engine == 'yara' rules are persisted/compiled.
| if observable[0] not in ["t", "q"]: | ||
| # checks for protocol, | ||
| # TCP(t) and QUIC(q) are the only supported protocols | ||
| raise self.NotJA4Exception("only TCP and QUIC protocols are supported") | ||
| if not observable[1:3] in ["12", "13"]: | ||
| if observable[1:3] not in ["12", "13"]: |
There was a problem hiding this comment.
check_ja4_fingerprint() indexes observable[0] / slices before validating len(observable). For short/empty observables this can raise IndexError (not caught) and crash the analyzer. Add an early length/format guard (or catch IndexError) before accessing positional characters, and return a NotJA4Exception-derived message instead of throwing.
| self._directory = path / directory_name | ||
| return self._directory | ||
|
|
||
| def is_unprotect_api(self) ->bool: |
There was a problem hiding this comment.
PEP8 formatting: add a space in the return type annotation (-> bool).
| def is_unprotect_api(self) ->bool: | |
| def is_unprotect_api(self) -> bool: |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| del os.environ["GIT_SSH"] | ||
| if settings.GIT_KEY_PATH.exists(): | ||
| os.remove(settings.GIT_KEY_PATH) | ||
| #NEW:Unprotect.it API update logic |
There was a problem hiding this comment.
The new Unprotect.it section breaks Python indentation: line 169 is at module level, but line 170 is indented as if it were inside class YaraRepo, which will raise an IndentationError and prevent the module from importing. Indent the comment (or remove it) so _update_unprotect_api is defined at the same indentation level as the other YaraRepo methods.
| #NEW:Unprotect.it API update logic | |
| # NEW: Unprotect.it API update logic |
| @@ -1,4 +1,4 @@ | |||
| # This file is a part of IntelOwl https://github.com/intelowlproject/IntelOwl | |||
| #This file is a part of IntelOwl https://github.com/intelowlproject/IntelOwl | |||
There was a problem hiding this comment.
Project-wide files use the copyright banner format # This file is a part of IntelOwl ... (note the space after #, e.g. api_app/admin.py:1). This file now has #This file..., which diverges from the established banner format; please restore the canonical header line.
Gagan144-blip
force-pushed
the
feature/unprotect-yara-api
branch
from
ee3ccad to
cc27db8
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try: | ||
| with open(file_path, "w", encoding="utf-8") as f: | ||
| f.write(rule_content) | ||
| except Exception: | ||
| logger.warning(f"Failed to write rule {rule_name}") |
There was a problem hiding this comment.
The except Exception: when writing a rule file logs only a warning message and drops the exception context, making failures hard to debug. Log the exception details (e.g., use logger.exception(...) or include the caught exception) so file system/permission errors are visible in logs.
| # NEW:Unprotect.it API update logic | ||
| def _update_unprotect_api(self): | ||
| logger.info(f"Fetching rules from Unprotect.it API:{self.url}") | ||
| os.makedirs(self.directory, exist_ok=True) | ||
|
|
||
| next_url = self.url | ||
| while next_url: | ||
| try: | ||
| response = requests.get(next_url, timeout=30) | ||
| response.raise_for_status() | ||
| data = response.json() | ||
| except Exception: | ||
| logger.exception("Failed to fetch Unprotect.it rules") | ||
| return |
There was a problem hiding this comment.
New Unprotect.it pagination + filtering logic in _update_unprotect_api() isn’t covered by unit tests. There are existing unit tests for YaraScan (e.g., tests/.../test_yara_scan.py), so adding a test that mocks requests.get and verifies pagination handling, engine == 'yara' filtering, and rule file creation would prevent regressions.
| if observable[0] not in ["t", "q"]: | ||
| # checks for protocol, | ||
| # TCP(t) and QUIC(q) are the only supported protocols | ||
| raise self.NotJA4Exception("only TCP and QUIC protocols are supported") | ||
| if not observable[1:3] in ["12", "13"]: | ||
| if observable[1:3] not in ["12", "13"]: | ||
| # checks for the version of the protocol | ||
| raise self.NotJA4Exception("procotol version wrong") | ||
| if not observable[3] in ["d", "i"]: | ||
| if observable[3] not in ["d", "i"]: | ||
| # SNI or no SNI |
There was a problem hiding this comment.
check_ja4_fingerprint() indexes/slices observable (e.g., observable[0], observable[1:3], observable[3]) before validating length, so short/empty inputs will raise IndexError instead of returning a helpful "not valid" message. Add a minimum-length check at the start (before any indexing).
| if not observable[1:3] in ["12", "13"]: | ||
| if observable[1:3] not in ["12", "13"]: | ||
| # checks for the version of the protocol | ||
| raise self.NotJA4Exception("procotol version wrong") |
There was a problem hiding this comment.
Typo in error message: "procotol version wrong" -> "protocol version wrong".
| raise self.NotJA4Exception("procotol version wrong") | |
| raise self.NotJA4Exception("protocol version wrong") |
mlodic
left a comment
mlodic
left a comment
There was a problem hiding this comment.
you also need to provide proof that this works. A successfull YAra analysis, screenshot and JSON result.
Also, unittest must be implemented too.
|
|
||
| if self.is_unprotect_api(): | ||
| self._update_unprotect_api() | ||
| return |
| return self._directory | ||
|
|
||
| def is_unprotect_api(self) -> bool: | ||
| return "unprotect.it/api/detection_rules" in self.url |
There was a problem hiding this comment.
ok but we also want to have these rules added in the default parameter of this analyzer. You need to do a migration to update the list of available rules.
This is needed because otherwise the users would need to manually add these new rules in the configuration.
| os.makedirs(self.directory, exist_ok=True) | ||
|
|
||
| next_url = self.url | ||
| while next_url: |
There was a problem hiding this comment.
never use while, bad logic could lead to infinite loops
| # NEW:Unprotect.it API update logic | ||
| def _update_unprotect_api(self): | ||
| logger.info(f"Fetching rules from Unprotect.it API:{self.url}") | ||
| os.makedirs(self.directory, exist_ok=True) |
There was a problem hiding this comment.
a dedicated subdirectory should be used to properly isolate these yara rules with the others
|
hii @mlodic , |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| def test_update_runs(self): | ||
| self.ys.update() | ||
|
|
There was a problem hiding this comment.
test_unprotect_url_in_config asserts the presence of a Yaraify ZIP URL, not an Unprotect.it API URL. Either update the test name/assertion to reflect what’s actually being configured, or (if the intent is to validate Unprotect.it support) assert the Unprotect.it API endpoint instead.
| logger.info("Unprotect.it API update completed") | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
_update_unprotect_api() ends with logger.info("Unprotect.it API update completed") at the top-level indentation, which will raise an IndentationError/SyntaxError and break the module import. Indent this log line so it is inside the method (same level as the other statements) and remove extra blank lines if any.
| logger.info("Unprotect.it API update completed") | |
| logger.info("Unprotect.it API update completed") |
| base_dir = self.directory / "unprotect_api" | ||
| os.makedirs(base_dir, exist_ok=True) | ||
|
|
There was a problem hiding this comment.
_update_unprotect_api() writes rules into a persistent unprotect_api/ directory but never clears previously downloaded rule files. This can leave stale rules around when they are removed/renamed upstream. Before writing new rules, consider cleaning the target directory (or writing to a temp dir and swapping) to keep local state consistent with the API.
| for pc in PluginConfig.objects.filter(parameter=param): | ||
| if "https://yaraify.abuse.ch/yarahub/yaraify-rules.zip" not in pc.value: | ||
| pc.value.append("https://yaraify.abuse.ch/yarahub/yaraify-rules.zip") | ||
| if "https://yaraify-api.abuse.ch/download/yaraify-rules.zip" in pc.value: | ||
| pc.value.remove("https://yaraify-api.abuse.ch/download/yaraify-rules.zip") | ||
| pc.save() |
There was a problem hiding this comment.
This migration is named as adding Unprotect.it support, but it only swaps the Yaraify ZIP URL (and it already depends on 0170_update_yaraify_archive, which performs the same swap). If the goal is to add https://unprotect.it/api/detection_rules/ (or similar) to the repositories list so the new API logic is actually used, update the migration accordingly; otherwise this migration looks redundant/misleading.
| def is_unprotect_api(self) -> bool: | ||
| return "unprotect.it/api/detection_rules" in self.url | ||
|
|
There was a problem hiding this comment.
is_unprotect_api() only checks for a substring and the repo directory name (computed from URL path segments for non-zip URLs) will not include the host, which can cause collisions (e.g., different hosts with /api/detection_rules/... would map to the same folder). Consider parsing the URL and including netloc in the directory naming for API-based sources (similar to the zip path).
| except PythonModule.DoesNotExist: | ||
| print("PythonModule for YARA not found. Skipping migration.") | ||
| return | ||
|
|
||
| # Get the "repositories" parameter for this module | ||
| try: | ||
| param = yara_module.parameters.get(name="repositories") | ||
| except Exception: | ||
| print("Parameter 'repositories' not found. Skipping migration.") | ||
| return | ||
|
|
||
| # Update PluginConfig values | ||
| for pc in PluginConfig.objects.filter(parameter=param): | ||
| if "https://yaraify.abuse.ch/yarahub/yaraify-rules.zip" not in pc.value: | ||
| pc.value.append("https://yaraify.abuse.ch/yarahub/yaraify-rules.zip") | ||
| if "https://yaraify-api.abuse.ch/download/yaraify-rules.zip" in pc.value: | ||
| pc.value.remove("https://yaraify-api.abuse.ch/download/yaraify-rules.zip") | ||
| pc.save() | ||
|
|
||
| print("Added Unprotect.it YARA rules successfully.") |
There was a problem hiding this comment.
Avoid using print() in migrations; it adds noisy stdout during deployments and isn’t consistent with the rest of the migration set here. Prefer no output, or use Django’s logging facilities if you really need diagnostics.
| self.param = self.pm.parameters.get(name="repositories") | ||
| self.pc = PluginConfig.objects.filter(parameter=self.param).first() |
There was a problem hiding this comment.
test_update_runs calls self.ys.update(), which performs network I/O (git clone/pull, zip downloads, now API calls). This will make unit tests flaky/slow in CI. Mock/patch the underlying I/O (e.g., YaraRepo.update/requests.get/git.Repo) or move this to an integration test suite that’s explicitly allowed to hit the network.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (2)
api_app/analyzers_manager/tests/test_yara.py:29
- The
assertIn(...)call is missing the proper indentation for the closing)(currently aligned to column 0). This will raise anIndentationError/SyntaxError. Align the closing paren with theself.assertIn(line (or keep the call on one line).
)
api_app/analyzers_manager/file_analyzers/yara_scan.py:105
rule_url()/head_branchassume the repo directory is a git checkout. For Unprotect.it API repos,analyze()will callrule_url()which callsself.head_branch->git.Repo(self.directory)and will raise (no.git). Treat Unprotect.it repos like zips inrule_url()(returnNoneor a non-git URL) and/or guardhead_branchusage whenis_unprotect_api()is true.
def is_unprotect_api(self) -> bool:
return "unprotect.it/api/detection_rules" in self.url
def update(self):
logger.info(f"Starting update of {self.url}")
if self.is_unprotect_api():
self._update_unprotect_api()
elif self.is_zip():
# private url not supported at the moment for private
self._update_zip()
else:
self._update_git()
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # NEW:Unprotect.it API update logic | ||
| def _update_unprotect_api(self): | ||
| logger.info(f"Fetching rules from Unprotect.it API: {self.url}") | ||
|
|
||
| base_dir = self.directory / "unprotect_api" | ||
| os.makedirs(base_dir, exist_ok=True) | ||
|
|
||
| MAX_PAGES = 20 | ||
| page = 0 | ||
| next_url = self.url | ||
|
|
||
| while next_url and page < MAX_PAGES: |
There was a problem hiding this comment.
Unprotect.it API support introduces new network/pagination behavior but there are no unit tests covering it. Please add a unit test under tests/api_app/analyzers_manager/unit_tests/file_analyzers/ that mocks the paginated requests.get responses and asserts only YARA rules are written/compiled.
| # Update PluginConfig values | ||
| for pc in PluginConfig.objects.filter(parameter=param): | ||
| if "https://yaraify.abuse.ch/yarahub/yaraify-rules.zip" not in pc.value: | ||
| pc.value.append("https://yaraify.abuse.ch/yarahub/yaraify-rules.zip") | ||
| if "https://yaraify-api.abuse.ch/download/yaraify-rules.zip" in pc.value: | ||
| pc.value.remove("https://yaraify-api.abuse.ch/download/yaraify-rules.zip") | ||
| pc.save() | ||
|
|
||
| print("Added Unprotect.it YARA rules successfully.") | ||
|
|
There was a problem hiding this comment.
This migration is effectively a duplicate of 0170_update_yaraify_archive (same add/remove URLs) but is named/worded as “Unprotect.it”. If the intent is to add https://unprotect.it/api/detection_rules/ to YARA repositories, the URL being appended here is wrong; otherwise this migration is redundant and should be dropped/rewritten.
| base_path="api_app.analyzers_manager.file_analyzers" | ||
| ) | ||
| self.param = self.pm.parameters.get(name="repositories") | ||
| self.pc = PluginConfig.objects.filter(parameter=self.param).first() | ||
| self.ys = YaraScan(config=self.pc) | ||
|
|
||
| def test_update_runs(self): | ||
| self.ys.update() | ||
|
|
||
| def test_unprotect_url_in_config(self): | ||
| self.assertIn( | ||
| "https://yaraify.abuse.ch/yarahub/yaraify-rules.zip", | ||
| self.pc.value |
There was a problem hiding this comment.
This test module is under api_app/.../tests, but CI runs manage.py test tests (only the top-level tests/ package), so these tests won’t execute in CI. Also, test_update_runs() triggers live network calls; existing analyzer tests use tests/api_app/analyzers_manager/unit_tests/... with mocks (e.g., tests/api_app/analyzers_manager/unit_tests/file_analyzers/test_yara_scan.py). Please move/reshape this test accordingly and mock external requests.
| from django.test import TestCase | ||
|
|
||
|
|
There was a problem hiding this comment.
json, os, and shutil are imported but unused and suppressed via # noqa: F401. Please remove these imports (or use them) to keep the test clean; the existing test suite generally avoids unused imports rather than suppressing them.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def test_unprotect_url_in_config(self): | ||
| self.assertIn( | ||
| "https://yaraify.abuse.ch/yarahub/yaraify-rules.zip", | ||
| self.pc.value | ||
| ) |
There was a problem hiding this comment.
test_unprotect_url_in_config is named as if it checks for an Unprotect.it API URL, but it asserts the presence of the Yaraify zip URL instead. Update the expected value (and likely the migration/config) to assert the actual Unprotect.it endpoint being added.
| safe_name = ( | ||
| rule_name.lower() | ||
| .replace(" ", "_") | ||
| .replace("/", "_") | ||
| .replace("\\", "_") | ||
| .replace(":", "_") | ||
| ) | ||
|
|
||
| file_path = base_dir / f"{safe_name}.yar" |
There was a problem hiding this comment.
The filename sanitization for rule_name is incomplete and can lead to collisions/invalid filenames (e.g., many characters are left unchanged; different names can normalize to the same safe_name and overwrite each other). Consider using a stricter slugification (allowing only [a-z0-9_-.]), limiting length, and/or prefixing with a stable unique identifier from the API response (like the rule id).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try: | ||
| # Get the Python module for YARA | ||
| yara_module = PythonModule.objects.get( | ||
| module="yara_scan.YaraScan", | ||
| base_path="api_app.analyzers_manager.file_analyzers", | ||
| ) | ||
| except PythonModule.DoesNotExist: | ||
| print("PythonModule for YARA not found. Skipping migration.") | ||
| return | ||
|
|
||
| # Get the "repositories" parameter for this module | ||
| try: | ||
| param = yara_module.parameters.get(name="repositories") | ||
| except Exception: | ||
| print("Parameter 'repositories' not found. Skipping migration.") | ||
| return |
There was a problem hiding this comment.
Avoid print() and broad except Exception in migrations. Use more specific exceptions (e.g., Parameter.DoesNotExist) and consider migrations.RunPython.noop/silent returns without printing to stdout (or use Django’s logging if needed).
| self.ys = YaraScan(config=self.pc) | ||
|
|
||
| def test_update_runs(self): | ||
| self.ys.update() |
There was a problem hiding this comment.
This test instantiates YaraScan(config=self.pc) where self.pc is a PluginConfig, but analyzers expect a PythonConfig/AnalyzerConfig. The test currently only works because it calls the @classmethod update() through the instance. Prefer calling YaraScan.update() directly (no instance) or construct the correct config object type to avoid brittle tests.
| self.ys = YaraScan(config=self.pc) | |
| def test_update_runs(self): | |
| self.ys.update() | |
| def test_update_runs(self): | |
| YaraScan.update() |
| except Exception: | ||
| logger.warning(f"Failed to write YARA rule: {rule_name}") | ||
|
|
||
| logger.info("Unprotect.it API update completed") |
There was a problem hiding this comment.
logger.info("Unprotect.it API update completed") is currently indented at class scope (not inside _update_unprotect_api). This will execute during import and the method won’t log completion. Move this log line inside _update_unprotect_api (after the pagination loop).
| logger.info("Unprotect.it API update completed") | |
| logger.info("Unprotect.it API update completed") |
| def test_update_runs(self): | ||
| self.ys.update() |
There was a problem hiding this comment.
test_update_runs will execute a full YARA rules update (network + git clones + compilation), which is slow/flaky and not suitable for unit tests. Mock external calls (e.g., requests.get, git.Repo.clone_from/pull) and assert expected local side effects instead.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…rs/quad9_dns_resolver.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Gagan144-blip
force-pushed
the
feature/unprotect-yara-api
branch
from
3313a50 to
5089041
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 16 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try: | ||
| response = requests.get(next_url, timeout=30) | ||
| response.raise_for_status() | ||
| data = response.json() | ||
| except Exception: | ||
| logger.exception("Failed to fetch Unprotect.it rules") | ||
| break |
There was a problem hiding this comment.
The exception handling here catches all exceptions with a bare "except Exception" and breaks the loop, potentially leaving the update incomplete. Additionally, the error is only logged but not re-raised, which could make it difficult to detect failures. Consider being more specific about which exceptions to catch (e.g., requests.RequestException, requests.Timeout) and potentially re-raising critical errors after logging. Also, the break statement means if page 5 fails, pages 6-20 won't be attempted, which may not be the desired behavior.
|
|
||
| # Stage 2: Backend | ||
| FROM python:3.11.7 AS backend-build | ||
| FROM python:3.10-slim AS backend-build |
There was a problem hiding this comment.
The Dockerfile changes from Python 3.11.7 to Python 3.10-slim. This is a downgrade in Python version and a change from a full image to a slim image. While this change may be intentional, it's not mentioned in the PR description which is about adding Unprotect.it API support for YARA rules. This seems unrelated to the stated purpose of the PR and could potentially break existing functionality or introduce compatibility issues. If this change is necessary, it should be documented in the PR description with justification.
| services: | ||
| postgres: | ||
| env_file: | ||
| - env_file_postgres_template | ||
| deploy: | ||
| resources: | ||
| limits: | ||
| cpus: '1' | ||
| memory: 2000M | ||
|
|
||
| uwsgi: | ||
| build: | ||
| context: .. | ||
| dockerfile: docker/Dockerfile | ||
| args: | ||
| REPO_DOWNLOADER_ENABLED: ${REPO_DOWNLOADER_ENABLED} | ||
| image: intelowlproject/intelowl:ci | ||
| env_file: | ||
| - env_file_app_ci | ||
|
|
||
|
|
||
| daphne: | ||
| build: | ||
| context: .. | ||
| dockerfile: docker/Dockerfile | ||
| image: intelowlproject/intelowl:ci | ||
| env_file: | ||
| - env_file_app_ci | ||
| deploy: | ||
| resources: | ||
| limits: | ||
| cpus: '0.50' | ||
| memory: 200M | ||
|
|
||
| nginx: | ||
| celery_beat: | ||
| build: | ||
| context: .. | ||
| dockerfile: docker/Dockerfile_nginx | ||
| image: intelowlproject/intelowl_nginx:ci | ||
| volumes: | ||
| - ../configuration/nginx/http.conf:/etc/nginx/conf.d/default.conf | ||
| deploy: | ||
| resources: | ||
| limits: | ||
| cpus: '0.50' | ||
| memory: 200M | ||
|
|
||
| celery_beat: | ||
| dockerfile: docker/Dockerfile | ||
| image: intelowlproject/intelowl:ci | ||
| env_file: | ||
| - env_file_app_ci | ||
| deploy: | ||
| resources: | ||
| limits: | ||
| cpus: '0.50' | ||
| memory: 200M | ||
|
|
||
| celery_worker_default: | ||
| build: | ||
| context: .. | ||
| dockerfile: docker/Dockerfile | ||
| image: intelowlproject/intelowl:ci | ||
| env_file: | ||
| - env_file_app_ci | ||
| deploy: | ||
| resources: | ||
| limits: | ||
| cpus: '0.50' | ||
| memory: 200M | ||
|
|
||
| redis: | ||
| deploy: | ||
| resources: | ||
| limits: | ||
| cpus: '0.50' | ||
| memory: 200M No newline at end of file | ||
| nginx: | ||
| build: | ||
| context: .. | ||
| dockerfile: docker/Dockerfile_nginx | ||
| image: intelowlproject/intelowl_nginx:ci No newline at end of file |
There was a problem hiding this comment.
This change removes resource limits (CPU and memory) and some configuration from the CI override file. This is a significant change to the CI environment configuration and is unrelated to adding Unprotect.it API support. These changes could affect CI performance and reliability and should be in a separate PR with proper justification and testing.
| Parameter = apps.get_model("api_app", "Parameter") | ||
|
|
||
| try: | ||
| pc = Parameter.objects.get(name="yara_rules_sources") |
There was a problem hiding this comment.
The migration is attempting to fetch a parameter named "yara_rules_sources", but based on the codebase (see api_app/analyzers_manager/file_analyzers/yara_scan.py line 372 and migrations/0002_0145_analyzer_config_yara.py line 81), the correct parameter name is "repositories". This will cause the migration to fail or not find the intended parameter.
| pc = Parameter.objects.get(name="yara_rules_sources") | |
| pc = Parameter.objects.get(name="repositories") |
| RUN apt-get update \ | ||
| && apt-get install -y --no-install-recommends apt-utils libsasl2-dev libssl-dev netcat-traditional \ | ||
| vim libldap2-dev libfuzzy-dev net-tools python3-psycopg2 git apache2-utils tshark \ | ||
| libemail-outlook-message-perl libemail-address-perl \ | ||
| && apt-get clean && apt-get autoclean && apt-get autoremove -y \ | ||
| && rm -rf /var/lib/apt/lists/* \ | ||
| && pip3 install --no-cache-dir --upgrade pip | ||
| && apt-get install -y --no-install-recommends \ | ||
| apt-utils \ | ||
| build-essential \ | ||
| gcc \ | ||
| libffi-dev \ | ||
| libssl-dev \ | ||
| python3-dev \ | ||
| curl \ | ||
| wget \ | ||
| libsasl2-dev \ | ||
| netcat-traditional \ | ||
| vim \ | ||
| libldap2-dev \ | ||
| libfuzzy-dev \ | ||
| net-tools \ | ||
| python3-psycopg2 \ | ||
| git \ | ||
| apache2-utils \ | ||
| tshark \ | ||
| libemail-outlook-message-perl \ | ||
| libemail-address-perl |
There was a problem hiding this comment.
The apt-get install command was reformatted with additional build dependencies (build-essential, gcc, libffi-dev, python3-dev, curl, wget) that weren't in the original command. Additionally, the cleanup commands (apt-get clean, autoclean, autoremove, rm -rf /var/lib/apt/lists/, and pip3 upgrade) were removed. This significantly increases the Docker image size and is unrelated to adding Unprotect.it API support. These changes should either be reverted or moved to a separate PR with proper justification.
2 participants
Description
This PR adds support for fetching and updating YARA rules from the Unprotect.it API, which are not available via GitHub repositories or downloadable zip files.
Related issue: #1711
Summary
YaraRepo.update()to handle Unprotect.it API URLsintel_owl_compiled.yasTesting
repo.update()inside IntelOwl Docker containerType of change
Checklist
developdumpplugincommand and added it in the project as a data migration. ("How to share a plugin with the community")test_files.zipand you added the default tests for that mimetype in test_classes.py.FREE_TO_USE_ANALYZERSplaybook by following this guide.urlthat contains this information. This is required for Health Checks (HEAD HTTP requests).get_mocker_response()method of the unittest class. This serves us to provide a valid sample for testing.DataModelfor the new analyzer following the documentation# This file is a part of IntelOwl https://github.com/intelowlproject/IntelOwl # See the file 'LICENSE' for copying permission.Black,Flake,Isort) gave 0 errors. If you have correctly installed pre-commit, it does these checks and adjustments on your behalf.testsfolder). All the tests (new and old ones) gave 0 errors.DeepSource,Django Doctorsor other third-party linters have triggered any alerts during the CI checks, I have solved those alerts.