OWASP / OWASP/Nettacker

UnboundLocalError in graph.py when ModuleNotFoundError is caught but die_failure() doesn't exit

Open
#1,198 4 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
5.6k
Forks
1.2k
Avg merge
1d 21h
Merged PRs (30d)
16

Description

Bug Description

The functions build_graph() and build_compare_report() in nettacker/core/graph.py have a bug that causes an UnboundLocalError when a ModuleNotFoundError is caught but the die_failure() function doesn't actually terminate the program (e.g., when die_failure is mocked in tests or if sys.exit() is intercepted).

Environment

Affected Code

File: nettacker/core/graph.py

Issue in build_graph() (lines 28-51):
def build_graph(graph_name, events):
    """
    build a graph
    ...
    """
    log.info(_("build_graph"))
    try:
        start = getattr(
            importlib.import_module(
                f"nettacker.lib.graph.{graph_name.rsplit('_graph')[0]}.engine"
            ),
            "start",
        )
    except ModuleNotFoundError:
        die_failure(_("graph_module_unavailable").format(graph_name))

    log.info(_("finish_build_graph"))
    return start(events)  # ← UnboundLocalError: 'start' referenced before assignment
Issue in build_compare_report() (lines 54-72):
def build_compare_report(compare_results):
    """
    build the compare report
    ...
    """
    log.info(_("build_compare_report"))
    try:
        build_report = getattr(
            importlib.import_module("nettacker.lib.compare_report.engine"),
            "build_report",
        )
    except ModuleNotFoundError:
        die_failure(_("graph_module_unavailable").format("compare_report"))

    log.info(_("finish_build_report"))
    return build_report(compare_results)  # ← UnboundLocalError: 'build_report' referenced before assignment

Root Cause

When ModuleNotFoundError is raised inside the try block:

  1. The variable (start or build_report) is never assigned
  2. The exception handler calls die_failure(), which normally calls sys.exit(1) to terminate the program
  3. However, if sys.exit() doesn't actually exit (e.g., when mocked in tests), execution continues to the next line
  4. The code tries to use the variable that was never assigned → UnboundLocalError

Evidence from Tests

The project's own test suite acknowledges this bug with @pytest.mark.xfail markers:

File: tests/core/test_graph.py (lines 35-39):

@patch("nettacker.core.graph.die_failure")
@patch("nettacker.core.graph.importlib.import_module", side_effect=ModuleNotFoundError)
@pytest.mark.xfail(reason="It will hit an UnboundLocalError")
def test_build_graph_module_not_found(mock_import_module, mock_die_failure):
    build_graph("missing_graph", [])
    mock_die_failure.assert_called_once()

File: tests/core/test_graph.py (lines 51-55):

@patch("nettacker.core.graph.die_failure")
@patch("nettacker.core.graph.importlib.import_module", side_effect=ModuleNotFoundError)
@pytest.mark.xfail(reason="It will hit an UnboundLocalError")
def test_build_compare_report_module_not_found(mock_import_module, mock_die_failure):
    build_compare_report({"some": "results"})
    mock_die_failure.assert_called_once()

These tests explicitly mark this as an expected failure with the exact error message: "It will hit an UnboundLocalError".

Expected Behavior

The function should either:

  1. Properly exit via die_failure() without continuing execution, OR
  2. Return immediately after calling die_failure() to prevent accessing unassigned variables

Actual Behavior

If die_failure() doesn't terminate the program (e.g., in test scenarios or if exception handling changes), the code raises:

UnboundLocalError: local variable 'start' referenced before assignment

or

UnboundLocalError: local variable 'build_report' referenced before assignment

Proposed Solution

Add explicit return statements after die_failure() calls to ensure the function doesn't continue execution:

def build_graph(graph_name, events):
    log.info(_("build_graph"))
    try:
        start = getattr(
            importlib.import_module(
                f"nettacker.lib.graph.{graph_name.rsplit('_graph')[0]}.engine"
            ),
            "start",
        )
    except ModuleNotFoundError:
        die_failure(_("graph_module_unavailable").format(graph_name))
        return  # ← Add this to prevent UnboundLocalError

    log.info(_("finish_build_graph"))
    return start(events)

The same fix applies to build_compare_report().

Impact

  • Severity: Medium
  • Test Coverage: Currently 2 tests are marked as xfail specifically because of this bug
  • User Impact: In production, this likely doesn't manifest because sys.exit(1) successfully terminates. However, it:
    • Prevents proper unit testing with mocked die_failure
    • Could cause unexpected behavior if error handling or exit behavior changes
    • Makes the code fragile and harder to test

Steps to Reproduce

  1. Mock die_failure() to prevent actual program exit
  2. Mock importlib.import_module() to raise ModuleNotFoundError
  3. Call build_graph() or build_compare_report()
  4. Observe UnboundLocalError instead of graceful handling

If this issue is valid, I would like to work on a fix and open a PR.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in nettacker/core/graph.py, focusing on build_graph() and build_compare_report(), then run the affected cases in tests/core/test_graph.py. Confirm the mocked ModuleNotFoundError paths no longer reach an unassigned variable, remove the xfail expectations, and verify both tests pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
security, testing-qa
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.