Description
The UserCodeService.is_execution_allowed() method contains a logic error that causes it to ignore the boolean return value from output_policy.is_valid(). This results in execution being allowed even when the output policy should deny it.
Current Behavior
The method uses a try/except block but only checks for exceptions raised by output_policy.is_valid():
- Exception is raised → returns
IsExecutionAllowedEnum.INVALID_OUTPUT_POLICY
- No exception is raised → always returns
IsExecutionAllowedEnum.ALLOWED
The boolean return value from is_valid() is ignored.
Code Reference:
|
try: |
|
output_policy.is_valid(context) |
|
except Exception: |
|
return IsExecutionAllowedEnum.INVALID_OUTPUT_POLICY |
|
|
|
return IsExecutionAllowedEnum.ALLOWED |
Expected Behavior
Based on the method signatures of OutputPolicy classes, the implementation should:
- Call
output_policy.is_valid() and capture its boolean return value
- Return
IsExecutionAllowedEnum.ALLOWED only if is_valid() returns True
- Return
IsExecutionAllowedEnum.INVALID_OUTPUT_POLICY if is_valid() returns False OR raises an exception (e.g., NotImplementedError)
Impact
This bug allows policy violations, including multiple executions of SingleExecutionExactOutput which run without the server returning cached ExecutionOutput values to the client in violation of the intended policy control that should limit execution to a single run
Suggested Fix
Modify the method to check the boolean return value:
try:
is_valid = output_policy.is_valid()
if not is_valid:
return IsExecutionAllowedEnum.INVALID_OUTPUT_POLICY
return IsExecutionAllowedEnum.ALLOWED
except Exception:
return IsExecutionAllowedEnum.INVALID_OUTPUT_POLICY
Description
The
UserCodeService.is_execution_allowed()method contains a logic error that causes it to ignore the boolean return value fromoutput_policy.is_valid(). This results in execution being allowed even when the output policy should deny it.Current Behavior
The method uses a try/except block but only checks for exceptions raised by
output_policy.is_valid():IsExecutionAllowedEnum.INVALID_OUTPUT_POLICYIsExecutionAllowedEnum.ALLOWEDThe boolean return value from
is_valid()is ignored.Code Reference:
PySyft/packages/syft/src/syft/service/code/user_code_service.py
Lines 359 to 364 in 73901fe
Expected Behavior
Based on the method signatures of
OutputPolicyclasses, the implementation should:output_policy.is_valid()and capture its boolean return valueIsExecutionAllowedEnum.ALLOWEDonly ifis_valid()returnsTrueIsExecutionAllowedEnum.INVALID_OUTPUT_POLICYifis_valid()returnsFalseOR raises an exception (e.g.,NotImplementedError)Impact
This bug allows policy violations, including multiple executions of
SingleExecutionExactOutputwhich run without the server returning cachedExecutionOutputvalues to the client in violation of the intended policy control that should limit execution to a single runSuggested Fix
Modify the method to check the boolean return value: