Skip to content

[high] fix: [mcafee_insights_enrich] stop killing the server on API errors - #829

Open
elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/019-mcafee-insights-no-sysexit
Open

elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/019-mcafee-insights-no-sysexit

Conversation

@elhoim

@elhoim elhoim commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

BLUF — An API error in mcafee_insights_enrich no longer tears down the whole misp-modules server.

  • Problem — expansion/mcafee_insights_enrich.py calls sys.exit() on an IAM auth failure and on a non-200 from search_ioc, but modules run inside a long-running Twisted server, so the SystemExit kills the whole process and every other in-flight enrichment; separately an empty data list fell through and returned None into prep_result(), raising TypeError.
  • Fix — both paths now raise a dedicated MVAPIError that handler() converts into a normal misperrors response, the empty branch returns None explicitly, and prep_result() reports that as an error.
  • Effect — a transient McAfee API error or an empty hit gives a clean error response instead of killing the server for every module on it.

The defect

if res.status_code != 200:
    self.logger.error(
        "Could not authenticate to get the IAM token: {0} - {1}".format(res.status_code, res.text)
    )
    sys.exit()
...
else:
    self.logger.error("Error in search_ioc. HTTP {0} - {1}".format(str(res.status_code), res.text))
    sys.exit()

misp_modules runs the expansion modules inside a long-running Twisted server process. Calling sys.exit() from inside a request handler raises SystemExit, which propagates up and kills the whole modules server, not just the one request — every other in-flight enrichment lookup on the server goes down with it. A single McAfee Insights auth failure or a non-200 response from search_ioc was enough to take out the entire server process.

Separately, when search_ioc() got a 200 response with an empty data list it logged a message but fell through with no return, so it returned None implicitly. That None was then passed straight into prep_result(ioc), which does ioc["data"][0] and raises an unhandled TypeError.

Impact

An analyst enriching an attribute with the MVISION Insights module hits a transient auth error, an HTTP error from the McAfee API, or simply a hash with no results, and the misp-modules server crashes or throws an unhandled exception instead of returning a normal "no results" / error response to MISP. Every other module running on that server instance is disrupted until it's restarted.

The fix

Both sys.exit() calls now raise a dedicated MVAPIError, which handler() catches and converts into a normal misperrors response instead of tearing down the process. The empty-result branch of search_ioc() now returns None explicitly, and prep_result() checks for None and reports it as a misperrors message instead of crashing with a TypeError. No behaviour change for the success path — only the error paths now degrade gracefully instead of taking the server down.

Verification

  • flake8 --extend-exclude=misp_modules/lib/,tests/,website/ on the changed file: clean, no output.
  • Full test suite: 161 passed, 4 skipped, 5 subtests passed in 95.18s (0:01:35), run against a local misp-modules server.

Found during a review of the repository; other findings are being submitted as separate PRs.

🤖 Generated with Claude Code

Both failure paths of the McAfee MVISION Insights module called sys.exit(),
which raises SystemExit inside the long-running modules server: any bad IAM
credential or non-2xx /insights/v2/iocs response tears down the request
handling thread instead of reporting the failure, and the analyst gets a
generic "something went wrong" with no explanation. On top of that,
search_ioc() fell through with no return when the API replied with an empty
data set, so a hash simply unknown to MVISION reached prep_result(None) and
crashed with TypeError. Both failure paths now raise a dedicated MVAPIError
that the handler converts into misperrors, and the empty result set is
returned explicitly as None and reported by prep_result as a plain message.

Verified with flake8 on the changed file and the full test suite against a
local modules server: 161 passed, 4 skipped, 5 subtests passed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018dfYpyaSZd1nxSRLr8suj8
@elhoim elhoim changed the title fix: [mcafee_insights_enrich] stop killing the server on API errors [high] fix: [mcafee_insights_enrich] stop killing the server on API errors Sep 3, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant