[amshamah419]

Related Issues

Fixes: XSUP-78726

Description

Generic API Event Collector (Beta) 1.1.2. Five fetch bugs found while investigating a customer whose Group-IB instances (set up through the generic collector) ingested nothing. Each of the customer's four GIB instances hit a different one:

1. Pagination crash (GIB Git Leak, GIB Public Leak). fetch_events() built the next request with {pagination_logic.pagination_field_name: next_page_value}, but pagination_field_name is a list (argToList(..., ".")). Every instance with Is pagination needed enabled failed with TypeError: unhashable type: 'list' as soon as a next page existed. The fix uses the last key of the path and sends it as a query param for GET requests. The original code replaced the JSON body, which GET APIs ignore, and dropped any configured request_json. For POST requests it merges the value into the JSON body.
2. Infinite pagination. APIs that always return the cursor field (for example Group-IB seqUpdate) never stop. Pagination now stops on an empty page, on a cursor that doesn't advance, or after MAX_INCIDENTS_TO_FETCH events.
3. Integer ID type ignored (GIB Darkweb Forum). id_type_lower was initialized to None and never assigned, so the integer max() branch could never run. The last fetched ID was always the ID of the last item returned. In the same area: when the ID type is integer, an event whose ID is greater than the stored last ID is now collected even if its timestamp is older than the last fetch time. Sequence-ordered feeds (Group-IB */updated, where the timestamp is the original content date) previously dropped every event. The only trace was a debug-level "This event is to old to pull", so the fetch looked healthy and ingested nothing. The time filter is unchanged for string IDs and when no ID is configured.
4. Unresolved placeholder sent literally (GIB Compromised Breach). recursive_replace() only substitutes placeholders that have a value in the last run. With query params like {"seqUpdate": "@last_fetched_id"} and no stored ID yet (for example right after a last-run reset), the literal text @last_fetched_id was sent. The API returned 400 invalid integer value ... parsing "@last_fetched_id". A failed fetch never saves a last run, so the instance failed the same way on every run. Params whose value still contains a placeholder after substitution are now omitted, with a debug line. The first request goes out without the cursor and the next run uses the stored ID.
5. ID wrapped in a one-item list (GIB Compromised Breach). Group-IB breacheddb returns addInfo.updateTimestampLong as a one-item list ([1579333849000000]). With ID type integer, arg_to_number() raised "[1579333849000000]" is not a valid number on every run, so a last-run reset couldn't help. A one-item list ID is now unwrapped before conversion.

Version: 1.1.1 was published on master in the meantime (demisto-sdk release #46335), so this PR is now 1.1.2. The release notes moved to 1_1_2.md, and master's 1_1_1.md is unchanged.

Evidence (tenant logs, 10-14 to 10-25)

• Git Leak / Public Leak: TypeError: unhashable type: 'list' at request_json = {pagination_logic.pagination_field_name: next_page_value} on every run.
• Compromised Breach: Error in API call [400] ... parsing "@last_fetched_id": invalid syntax on every run from 10-15 16:43. After that fix: ValueError: "[1579333849000000]" is not a valid number from arg_to_number(current_id) on every run.
• Darkweb Forum: ran every minute (~1.5s) with no error and nothing ingested.
• Verified on the customer tenant with a build of this branch. The "not a valid number" errors stopped after the 10-25 19:47Z upload, and the customer confirms all instances now ingest.

AI reviewer comments

• RN prefix: changed to Fixed an issue ... (patch bump).
• Parse previous_fetched_id outside the loop: done. The stored ID is converted once before the loop.
• try/except around arg_to_number: done. With ID type integer, an event whose ID isn't numeric is logged with demisto.error and skipped, so it can't fail the fetch or become the cursor.
• Stop pagination at the user limit: done. Pagination stops once the fetched events reach limit (the yml describes it as "Number of incidents to fetch per fetch"), capped at MAX_INCIDENTS_TO_FETCH.
• VENDOR/PRODUCT constants, keywords in pack_metadata.json: not applicable. This is a generic collector: vendor and product are instance parameters, and there is no single vendor to list.

Validation

• Regression tests for each bug, including test_recursive_replace_omits_unresolved_placeholders and test_fetch_events_integer_id_wrapped_in_one_item_list. The full unit test file passes locally (91 passed, 9 async tests skipped as before).
• demisto-sdk pre-commit on the pack: ruff, ruff-format, pycln, xsoar-lint, validate, secrets, markdownlint all pass. Docker wasn't available locally, so CI runs the docker-backed hooks (pytest and mypy).

Must have

• Tests (regression tests for each bug)
• Release notes

[content-bot]

🤖 AI-Powered Code Review Available

You can leverage AI-powered code review to assist with this PR!

Available Commands:

• @marketplace-ai-reviewer start review - Initiate a full AI code review
• @marketplace-ai-reviewer re-review - Incremental review for new commits

[amshamah419]

@marketplace-ai-reviewer start review

[marketplace-ai-reviewer]

🤖 Analysis started. Please wait for results...

[github-actions]

Coverage Report

File	Stmts	Miss	Cover	Missing
Packs/GenericAPIEventCollector/Integrations/GenericAPIEventCollector
GenericAPIEventCollector.py	444	25	94%	260–261, 265–266, 425, 458–464, 466, 468, 476–479, 647, 849–852, 869, 871
TOTAL	444	25	94%

Tests	Skipped	Failures	Errors	Time
107	2 💤	0 ❌	0 🔥	4.862s ⏱️

[marketplace-ai-reviewer]

🤖 AI Review Disclaimer

This review was generated by an AI-powered tool and may contain inaccuracies. Please be advised, and we extend our sincere apologies for any inconvenience this may cause.

[marketplace-ai-reviewer]

Hi! Thanks for your contribution to the Generic API Event Collector pack.

I've left a few notes to help polish the integration, primarily focusing on adding missing constants, ensuring the pagination loop respects the user-configured fetch limit, and optimizing the fetch loop's error handling. You'll also need to tweak the release notes wording to match the patch version bump and add the vendor name to the metadata keywords.

Thanks again for your hard work!

Additionally, please address the following file-level notes:

• Packs/GenericAPIEventCollector/Integrations/GenericAPIEventCollector/GenericAPIEventCollector.py: Missing mandatory VENDOR and PRODUCT constants at the top of the file.
• Packs/GenericAPIEventCollector/pack_metadata.json: The vendor name must appear in the keywords list.

@JasBeilin, @amshamah419 please review and approve the results generated by the AI Reviewer by responding 👍 on this comment.

[JasBeilin]

great work overall, check my comments.

[content-bot]

Validate summary
If the AG100 validation in the pre-commit GitHub Action fails, the pull request cannot be force-merged.

Verdict: PR can be force merged from validate perspective? ✅

[content-bot]

🔍 AI Triage Report Available

An automated triage report has been generated for this pipeline.

Status: failed
Report ID: 3b75cf57c6a3f6fe

📋 Triage Report
💡 Resolutions are available in the full report.

⚠️ AI-generated triage. Validate before acting.

[JasBeilin]

nice!