Python: Replace eval with safer alternatives

[cwhite911]

Fixes the 14 Bandit B307 (eval) code scanning alerts in non-vendored code. All flagged inputs are local (GRASS-shipped XML, parser tokens, model files), so this is a code quality cleanup rather than a vulnerability fix; eval is simply unnecessary at most of these sites.

• grass.temporal (temporal_algebra.py, 9 alerts): eval_datetime_str() evaluated eval(str(a < b)) for each operator, which round-trips a bool through a string; it now compares directly. The td(A) == 1 and start_day() > 5 style comparisons built Python source by string concatenation and eval'd it; they now call the same comparison helper with float(value). The one remaining eval (boolean expression string assembled from isinstance(x, bool)-checked values and grammar tokens) is documented and marked # nosec B307.
• wxGUI: menutree.py resolved <id> menu XML entries with eval("wx." + text), now getattr(wx, text) (verified: every <id> in the shipped menu XMLs resolves); gui_core/menu.py resolved menu handlers with eval, now getattr (all <handler> entries in the shipped XMLs are plain attribute names), which also removes the one pre-existing # nosec B307. The gmodeler loop-condition eval stays (a model's loop expression is authored by the model creator; evaluating it is equivalent to running the model) and is now documented and suppressed.
• i.tasscap: dispatched calc1bands<N> via eval of a formatted call string, now a globals() lookup with a normal call.

Verification: the three temporal algebra testsuites (unittests_temporal_algebra.py, unittests_temporal_raster_algebra.py, unittests_temporal_vector_algebra.py) pass against nc_spm; i.tasscap runs end-to-end on synthetic bands; bandit 1.9.4 reports no B307 findings in the touched files.

This is part of a larger effort to work through the open code scanning alerts, grouped into small PRs by issue type.

Written with the assistance of Claude Code.

[wenzeslaus]

Do we know the tests actually cover this?

[cwhite911]

The eval_datetime_str (called from eval_global_var and the td()/# handler) is exercised by the existing temporal tests:

• float branch: unittests_temporal_conditionals.py uses if(start_month(A) > 2, …), if(start_doy(A) < 3, …), if(start_day(A) <= 2, …); the #/td() count path appears as A # D == 1 in the raster algebra tests.
• datetime branch: unittests_temporal_algebra*.py compare start_date(A) with <, <=, ==, >, >=.

So <, <=, ==, >, >= are all covered; the one operator no existing test hits is !=. The change is behavior-preserving, eval(str(a < b)) is just a < b (verified identical across all six operators over int/float/date values), so it removes the eval() round-trip without changing semantics.

We can add a small test asserting each operator (including !=) to make the. coverage explicit

[wenzeslaus]

The temporal expressions require further care and I didn't have time to explore it more fully before. The tests are still critical, but there are other issues which surfaced when I co-reviewed that with AI. See the individual comments.

[wenzeslaus]

str.lstrip strips a character set, not a prefix: "self.selectAll".lstrip("self.") returns "ctAll".

It is latent today (every shipped <handler> starts with On, which survives untouched) and it is pre-existing, so this is not a regression. But the line is already being changed here and removeprefix is a one-word swap.

[wenzeslaus]

The justification is slightly overstated: the first element is not checked. leftbool = map_i.condition_value[0] (line 1692) goes into condition_value_list unconditionally, while only the loop-appended values pass isinstance(boolean, bool) (line 1708).

It is safe in practice, since every writer of condition_value stores a bool. But a # nosec is a permanent claim that future auditors will trust without re-deriving it, so it is worth being exact. Either tighten the wording or guard leftbool the same way.

[wenzeslaus]

The name no longer describes the function: it does not eval, does not take strings, and this PR gives it two numeric call sites. Since it is public on TemporalAlgebraParser, worth renaming it to what it does and keeping a deprecated alias:

def compare_values(self, left, comp, right):
    """Compare two values with the given comparison operator."""
    ...

def eval_datetime_str(self, tfuncval, comp, value):
    """Compare two values with the given comparison operator.

    .. deprecated:: 8.6.0
        Use :func:`compare_values` instead.
    """
    return self.compare_values(tfuncval, comp, value)

matching the .. deprecated:: convention already used in grass.script (e.g. python/grass/script/vector.py:387).

Also, there is no else branch, so an unrecognized operator gives UnboundLocalError. It is unreachable via p_compare_op, but cheap to close with an explicit raise now that there are more callers.

[wenzeslaus]

Following up on my question about coverage: != is the one operator no existing test reaches. Counting comparison operators used in expressions across python/grass/temporal/testsuite/: < 12, <= 8, >= 9, > 3, == 2, != 0.

That matters more here than for the other five, because != is exactly where eval(str(a != b)) == a != b could fail: a type defining __eq__ without __ne__. It is the only branch where the equivalence is asserted but not observed. The rewritten except (..., TypeError, ValueError) below and the msgr.fatal() it guards are also unexercised.

A unit test over the six operators against int/float/date values would make this observable rather than asserted, and it is small enough to belong in this PR rather than a follow-up.

[wenzeslaus]

float() is not needed here, and it is new behavior. gvar.value comes from a number token, and t_INT/t_FLOAT already converted it to a real int/float before it reaches this rule.

More to the point, the old code never coerced: eval(str(tfuncval) + comp_op + str(value)) compared ints as ints. float() turns start_year(A) == 2001 into a float comparison, and it hard-codes the "RHS is a numeric literal" assumption that the TODO at 2860-2866 explicitly wants to lift (t_var comp_op t_var would put an int from tfuncdict on the right). Passing value through unchanged is closer to both the old semantics and the documented direction.

[wenzeslaus]

Same as the eval_global_var call: float() is not needed. t[3] is already an int/float from the lexer, so value = str(t[3]) on line 2829 followed by float(value) here converts a number to a string and back.

Dropping both and passing t[3] straight through preserves the exact int semantics the old eval had.

[cwhite911]

Addressed in the latest push:

• Rename + alias: eval_datetime_str -> compare_values(self, left, comp, right), with eval_datetime_str kept as a .. deprecated:: 8.6.0 alias delegating to it. Added an explicit raise ValueError("Unknown comparison operator: ...") so an unrecognized operator no longer falls through to UnboundLocalError.
• Dropped the float() (and the str() round-trip): both call sites now pass the value through unchanged. t_INT/t_FLOAT already yield real int/float, so start_year(A) == 2001 and A # B == 2 stay integer-exact, matching the old eval semantics and leaving room for the t_var comp_op t_var direction in the TODO. In eval_global_var the two branches now converge on a single compare_values(...) call after the date conversion.
• Coverage / !=: added python/grass/temporal/tests/temporal_algebra_compare_values_test.py, asserting all six operators against int, float, and date values (equal and unequal), the int-exactness, and the unknown-operator raise. It builds the parser via __new__ (no DB connection needed since compare_values uses no instance state), so it stays a fast unit test. This makes the != branch observable.
• menu.py lstrip: switched to str.removeprefix("self.").
• # nosec at build_...: guarded the seed leftbool with an isinstance(..., bool) check that raises otherwise, so the "only booleans are evaluated" justification is now enforced rather than asserted, and updated the comment to match.

The except in the td/# handler dropped ValueError (no more float() to raise it) and keeps IndexError, AttributeError, TypeError.