GLEIF lookups could panic on nil HTTP responses under network/error edge cases, causing datasource instability in org enrichment paths. This change hardens response handling and adds deterministic regression coverage for the nil-response path.#1112
GLEIF lookups could panic on nil HTTP responses under network/error edge cases, causing datasource instability in org enrichment paths. This change hardens response handling and adds deterministic regression coverage for the nil-response path.#1112MichaelMVS wants to merge 4 commits intoowasp-amass:mainfrom
Conversation
Co-authored-by: MichaelMVS <102698294+MichaelMVS@users.noreply.github.com> Agent-Logs-Url: https://github.com/MichaelMVS/amass/sessions/ce85891c-4f57-4f7b-adb5-1c4be1e87132
Co-authored-by: MichaelMVS <102698294+MichaelMVS@users.noreply.github.com> Agent-Logs-Url: https://github.com/MichaelMVS/amass/sessions/ce85891c-4f57-4f7b-adb5-1c4be1e87132
…urces Fix GLEIF nil-response panic in org support helpers
There was a problem hiding this comment.
Pull request overview
Hardens the org enrichment GLEIF helper functions against nil HTTP responses (preventing nil-pointer panics) and adds regression tests for the nil-response edge case.
Changes:
- Introduces an injectable
requestWebPagefunction for deterministic testing of GLEIF HTTP behavior. - Adds explicit nil/invalid response guards in GLEIF fuzzy, LEI, direct-parent, and direct-children retrieval paths.
- Adds regression tests ensuring no panic occurs when the HTTP layer returns
(nil, nil).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| engine/plugins/support/org/gleif.go | Uses an injectable request function and adds nil/invalid response handling to prevent panics. |
| engine/plugins/support/org/gleif_nil_response_test.go | Adds deterministic tests covering nil HTTP responses for several GLEIF retrieval helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if err != nil { | ||
| msg := fmt.Sprintf("Failed to obtain the LEI record for %s: %s", name, err) | ||
| return nil, fmt.Errorf("GLEIFSearchFuzzyCompletions: %s", msg) | ||
| } | ||
| if resp == nil || resp.Body == "" { | ||
| return nil, fmt.Errorf("GLEIFSearchFuzzyCompletions: failed to obtain the LEI record for %s", name) | ||
| } |
There was a problem hiding this comment.
In GLEIFSearchFuzzyCompletions, the error text refers to obtaining an "LEI record", but this function is calling the fuzzy completions endpoint and returns a FuzzyCompletionsResponse. This makes logs/debugging misleading; please update the messages to reference fuzzy completions (and ideally the endpoint/name being queried) rather than an LEI record.
| func TestGLEIFGetLEIRecordNilResponseNoPanic(t *testing.T) { | ||
| t.Cleanup(func() { | ||
| requestWebPage = http.RequestWebPage | ||
| }) | ||
|
|
||
| requestWebPage = func(ctx context.Context, r *http.Request) (*http.Response, error) { | ||
| return nil, nil | ||
| } | ||
|
|
||
| require.NotPanics(t, func() { | ||
| record, err := GLEIFGetLEIRecord("ZXTILKJKG63JELOEG630") | ||
| require.Error(t, err) | ||
| require.Nil(t, record) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Nil-response guards were added to GLEIFSearchFuzzyCompletions as well, but there is no deterministic regression test covering the nil-response path for this function (unlike the LEI/direct-parent/direct-children helpers). Please add a TestGLEIFSearchFuzzyCompletionsNilResponseNoPanic-style test that stubs requestWebPage to return (nil, nil) and asserts an error is returned without panicking.
| t.Cleanup(func() { | ||
| requestWebPage = http.RequestWebPage | ||
| }) | ||
|
|
||
| requestWebPage = func(ctx context.Context, r *http.Request) (*http.Response, error) { | ||
| return nil, nil | ||
| } |
There was a problem hiding this comment.
These tests reset requestWebPage to http.RequestWebPage unconditionally. To avoid order-dependence if future tests also stub requestWebPage, it’s safer to capture the prior value at the start of each test and restore that in t.Cleanup (instead of assuming the default).
Problem:
GLEIF helper methods could panic with a nil-pointer dereference when HTTP requests returned a nil response object and response fields were accessed without nil checks.
Solution:
Added an injectable request function (requestWebPage) for deterministic tests and added explicit nil-response guards in GLEIF fuzzy, LEI, direct-parent, and direct-children retrieval paths. Returned explicit errors when responses are nil/invalid instead of dereferencing.
Testing: