Skip to content

[high] fix: [greynoise] propagate query errors instead of reporting empty success - #820

Open
elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/012-greynoise-error-propagation
Open

elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/012-greynoise-error-propagation

Conversation

@elhoim

@elhoim elhoim commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

BLUF — greynoise swallows query errors and returns an empty result that looks like success.

  • Problem — handler() in misp_modules/modules/expansion/greynoise.py invokes query_greynoise_ip_hover, query_greynoise_ip_expansion and query_greynoise_vulnerability as bare statements and then unconditionally returns get_result(), discarding the misperrors dict those methods return on failure.
  • Fix — Capture each return value and return it immediately when it is not None.
  • Effect — A bad API key, a GreyNoise outage or an unsupported Community-key vulnerability lookup surfaces as an error rather than a successful-looking empty enrichment implying GreyNoise found nothing.

In misp_modules/modules/expansion/greynoise.py, handler() called the query functions as bare statements and discarded their return value:

if "persistent" in request:
    greynoise_parser.query_greynoise_ip_hover(request["config"]["api_key"])
else:
    greynoise_parser.query_greynoise_ip_expansion(request["config"]["api_key"])
...
if attribute["type"] == "vulnerability":
    greynoise_parser.query_greynoise_vulnerability(request["config"]["api_key"])

return greynoise_parser.get_result()

Each of these query methods returns a misperrors dict when the query fails (bad API key, GreyNoise outage, unsupported vulnerability lookup on a Community key, etc.), but that return value was never captured, and handler() unconditionally returned get_result() afterward.

Impact: when GreyNoise enrichment fails for any reason, the analyst sees a successful-looking response with an empty enrichment instead of an error, silently masking outages or credential problems and giving a false sense that "GreyNoise checked this IP and found nothing."

Fix: capture each query call's return value and return it immediately when it is not None, matching the error-return pattern already used inside the parser methods. Success paths are unaffected, since those methods fall off the end (implicit None) when the query succeeds.

No behaviour change beyond correctly surfacing pre-existing error paths that were previously swallowed.

Verification

  • flake8 on the changed file: clean, no output.
  • Full module test suite: 161 passed, 4 skipped, 5 subtests passed in 144.87s (0:02:24), matching the pre-existing baseline.

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

🤖 Generated with Claude Code

…ccess

The handler calls query_greynoise_ip_hover, query_greynoise_ip_expansion and
query_greynoise_vulnerability as bare statements, so the misperrors dict each of
them returns on failure is discarded and the handler unconditionally returns
get_result(). When the API key is invalid or unauthorised, the GreyNoise service
is down, or the request raises, the analyst gets a successful-looking enrichment
with no attributes rather than the error message the module actually produced.
The return values are now captured and returned when the query reports an error,
matching the pattern already used inside the parser methods.

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: [greynoise] propagate query errors instead of reporting empty success [high] fix: [greynoise] propagate query errors instead of reporting empty success 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