damccorm commented on code in PR #38769:
URL: https://github.com/apache/beam/pull/38769#discussion_r3341627633


##########
sdks/python/apache_beam/ml/transforms/base.py:
##########
@@ -591,7 +592,18 @@ def save_attributes(
   def load_attributes(artifact_location):
     with FileSystems.open(os.path.join(artifact_location, 
_ATTRIBUTE_FILE_NAME),
                           'rb') as f:
-      return jsonpickle.decode(f.read())
+      # load_attributes runs eagerly during MLTransform.expand() at pipeline
+      # construction time, so the pipeline's options are available via the
+      # construction-time context.
+      pipeline_options = get_pipeline_options()
+      safe = True
+      if (pipeline_options is not None and
+          pipeline_options.is_compat_version_prior_to("2.75.0")):
+        # Keep the pre-2.75.0 jsonpickle behavior (safe=False permits
+        # eval-based decoding) for backwards compatibility with already-staged
+        # artifacts.
+        safe = False

Review Comment:
   Is there actually a downside to always setting safe=False? IIUC, this is 
protecting against remote code execution exploits that allow escalating 
privileges. But if you are able to compromise this artifact file, I think you 
already functionally have code execution privileges since it allows you to 
define (semi) arbitrary transforms.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to