Skip to content

Conversation

@themavik
Copy link

@themavik themavik commented Feb 11, 2026

Summary

Fixes #9391

UserCodeService.is_execution_allowed() calls output_policy.is_valid(context) but discards its boolean return value. Only exceptions from is_valid() were caught, so when is_valid() returned False without raising an exception, execution was incorrectly allowed.

Root Cause

# Before (broken)
try:
    output_policy.is_valid(context)  # return value discarded
except Exception:
    return IsExecutionAllowedEnum.INVALID_OUTPUT_POLICY

return IsExecutionAllowedEnum.ALLOWED  # always reached if no exception

This means policies like SingleExecutionExactOutput -- which return False on subsequent calls rather than raising -- were silently bypassed, allowing multiple executions when only one should be permitted.

Fix

try:
    is_valid = output_policy.is_valid(context)
    if not is_valid:
        return IsExecutionAllowedEnum.INVALID_OUTPUT_POLICY
except Exception:
    return IsExecutionAllowedEnum.INVALID_OUTPUT_POLICY

return IsExecutionAllowedEnum.ALLOWED

Now:

  • is_valid() returns False -> INVALID_OUTPUT_POLICY
  • is_valid() raises an exception -> INVALID_OUTPUT_POLICY
  • is_valid() returns True -> ALLOWED

Impact

This is a security-relevant fix. Without it, output policies that signal invalidity via return value (rather than exception) are ineffective, allowing unauthorized repeated execution of user code.

Test Plan

  • SingleExecutionExactOutput policy correctly blocks a second execution attempt
  • Policies that raise exceptions on invalid state continue to work as before
  • Policies that return True on valid state continue to allow execution

…llowed

The method called `output_policy.is_valid(context)` but discarded its
boolean return value.  Only exceptions were caught, so when `is_valid()`
returned `False` without raising, execution was incorrectly allowed.

This violated policies like `SingleExecutionExactOutput` which return
`False` on subsequent calls instead of raising.

Now the return value is captured and checked:
- `False` return -> `INVALID_OUTPUT_POLICY`
- Exception raised -> `INVALID_OUTPUT_POLICY`
- `True` return -> `ALLOWED`

Fixes OpenMined#9391
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.

UserCodeService.is_execution_allowed() allows invalid output policy execution

1 participant