Skip to content

Add reversed() to safe_builtins - #336

Closed
divinity76 wants to merge 1 commit into
zopefoundation:masterfrom
divinity76:support-reversed-builtin
Closed

divinity76 wants to merge 1 commit into
zopefoundation:masterfrom
divinity76:support-reversed-builtin

Conversation

@divinity76

Copy link
Copy Markdown

Fixes #335.

reversed() returns an iterator over an already-visible sequence; it exposes no attributes and mutates nothing, so it is safe to allow alongside sorted, len and friends.

  • adds reversed to _safe_names
  • test + changelog entry

Full suite: 303 passed, 7 skipped.

Copilot AI balanced review requested due to automatic review settings November 1, 2026 14:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are reported for the addition of reversed() to the safe builtins.

No open issues 🎉

What changed in this PR

Adds reversed() to RestrictedPython’s safe builtins, addressing issue #335 so restricted code can request reverse iteration over sequences.

Changes:

  • Includes reversed in _safe_names.
  • Adds a test verifying reverse iteration through restricted evaluation.
  • Documents the addition in the unreleased changelog.
File Description
tests/​test_Guards.py Tests that restricted evaluation supports reversed().
src/​RestrictedPython/​Guards.py Adds reversed to the safe builtin allowlist.
CHANGES.rst Records the new builtin support.

Review effort: Balanced

@icemac

icemac commented Nov 2, 2026

Copy link
Copy Markdown
Member

Thank you for your contribution.

According to the contributing policies of the zopefoundation organization you need to sign a contributor agreement before any non-trivial change can be merged. For details please consult the Contributing guidelines for zopefoundation projects.

@icemac

icemac commented Nov 2, 2026

Copy link
Copy Markdown
Member

I reviewed the PR and came to the following conclusion: I am going to reject this PR, and I am also going to remove sorted and tuple from safe_builtins for the same reason (#337).

reversed() iterates its argument in C. Restricted code never goes through _getiter_ for the elements, so the guard that is supposed to control iteration never runs. This is easy to demonstrate with a _getiter_ that refuses everything:

[x for x in secret]   # RuntimeError: iteration refused by _getiter_
tuple(secret)         # ('s', 'e', 'c')
sorted(secret)        # ['c', 'e', 's']
reversed(secret)      # works the same way

The argument that reversed "exposes no attributes and mutates nothing" is true, but it misses the point: it exposes the elements of any iterable without the application getting a say. In Zope that is the difference between a validated item and an Unauthorized error.

sorted and tuple have been in safe_builtins with the same flaw, so this is not a new class of problem. But it is not a reason to add one more unguarded route. Applications that need these builtins have to provide wrappers that pass the argument through _getiter_, the way AccessControl.ZopeGuards.guarded_zip does.

@icemac icemac closed this Nov 2, 2026
@icemac icemac mentioned this pull request Nov 2, 2026
@devdanzin

Copy link
Copy Markdown

sorted and tuple have been in safe_builtins with the same flaw, so this is not a new class of problem. But it is not a reason to add one more unguarded route. Applications that need these builtins have to provide wrappers that pass the argument through _getiter_, the way AccessControl.ZopeGuards.guarded_zip does.

Sorry if this is a silly question, but doesn't that mean zip should also be excluded from safe_builtins ?

@icemac

icemac commented Nov 3, 2026 •

Copy link
Copy Markdown
Member

Sorry if this is a silly question, but doesn't that mean zip should also be excluded from safe_builtins ?

Not a silly question at all, but a nice catch. zip iterates its arguments in C too, so _getiter_ only ever sees the zip object and never the sequences behind it:

# _getiter_ refuses `secret`
[x for x in secret]        # RuntimeError: iteration of secret refused by _getiter_
[x for x in zip(secret)]   # [('s',), ('e',), ('c',)]

It went unnoticed for the same reason sorted and tuple did: Zope's AccessControl.ZopeGuards replaces it with guarded_zip, which runs every argument through guarded_iter first, so the Zope users who make up most of the audience never saw the raw builtin. Plain RestrictedPython users do.

I will remove zip from safe_builtins in #337 as well. The same reasoning applies to anything else that consumes an iterable on the caller's behalf, for example bytes(iterable). If you need these, provide wrappers that pass the argument through _getiter_ first, the way guarded_zip does.

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.

Please support reversed()

4 participants