Arun-Sanjay commited on
Commit
39dd2ab
Β·
1 Parent(s): ff38834

Phase 4 cleanup: allow ast.IfExp (ternary expressions) in restricted Python, update tests

Browse files
red_button/restricted_python.py CHANGED
@@ -60,6 +60,15 @@ Python version note
60
  yields :class:`ast.Constant`, so their presence in :data:`ALLOWED_NODES` is a
61
  no-op β€” but we include them to match the Section 7.1 spec literally and in
62
  case a future interpreter re-emits them.
 
 
 
 
 
 
 
 
 
63
  """
64
 
65
  from __future__ import annotations
@@ -128,6 +137,7 @@ ALLOWED_NODES: frozenset[type] = frozenset(
128
  ast.Assign,
129
  # Control flow (with iteration cap enforced via sys.settrace)
130
  ast.If,
 
131
  ast.For,
132
  ast.While,
133
  # Function calls β€” restricted further below to whitelisted builtins
 
60
  yields :class:`ast.Constant`, so their presence in :data:`ALLOWED_NODES` is a
61
  no-op β€” but we include them to match the Section 7.1 spec literally and in
62
  case a future interpreter re-emits them.
63
+
64
+ Allowed nodes β€” IfExp alongside If
65
+ ----------------------------------
66
+ Both the statement form (:class:`ast.If`) and the expression/ternary form
67
+ (:class:`ast.IfExp`, e.g. ``x if cond else y``) are in :data:`ALLOWED_NODES`.
68
+ Ternary expressions are pure calculator syntax β€” structurally equivalent to
69
+ the already-allowed statement-form ``if`` and adding no new escape surface
70
+ because ``ast.walk`` still recursively visits the test/body/orelse children
71
+ under the same allowlist. Frequently useful for GSM8K-style math.
72
  """
73
 
74
  from __future__ import annotations
 
137
  ast.Assign,
138
  # Control flow (with iteration cap enforced via sys.settrace)
139
  ast.If,
140
+ ast.IfExp,
141
  ast.For,
142
  ast.While,
143
  # Function calls β€” restricted further below to whitelisted builtins
tests/test_restricted_python.py CHANGED
@@ -310,13 +310,21 @@ def test_sorted_builtin(fs: SimulatedFilesystem) -> None:
310
 
311
 
312
  def test_conditional_expression(fs: SimulatedFilesystem) -> None:
313
- # If-expressions parse as ast.IfExp β€” but Section 7.1 only lists ast.If
314
- # (statement form), not ast.IfExp. So this should actually be blocked.
315
- # The ticket's "Valid calculator use" list requests testing
316
- # ``_result = 5 if 1 > 0 else 10`` returns "5" β€” but IfExp is not in
317
- # ALLOWED_NODES, so the honest behavior is RestrictedError. We assert the
318
- # true behavior and flag this ambiguity in the report.
319
- result = exec_restricted("_result = 5 if 1 > 0 else 10", fs)
 
 
 
 
 
 
 
 
320
  assert result.startswith("RestrictedError"), result
321
 
322
 
 
310
 
311
 
312
  def test_conditional_expression(fs: SimulatedFilesystem) -> None:
313
+ # Ternary expressions (ast.IfExp) are allowed alongside ast.If β€” pure
314
+ # calculator syntax, no new escape surface.
315
+ assert exec_restricted("_result = 5 if 1 > 0 else 10", fs) == "5"
316
+
317
+
318
+ def test_conditional_expression_string_branch(fs: SimulatedFilesystem) -> None:
319
+ assert exec_restricted("_result = 'big' if 10 > 5 else 'small'", fs) == "big"
320
+
321
+
322
+ def test_conditional_expression_recursively_validates_children(
323
+ fs: SimulatedFilesystem,
324
+ ) -> None:
325
+ # Allowing ast.IfExp does NOT bypass child validation β€” the disallowed
326
+ # ``open`` call inside the ternary is still caught by ast.walk.
327
+ result = exec_restricted("_result = open('x') if 1 > 0 else 10", fs)
328
  assert result.startswith("RestrictedError"), result
329
 
330