-
Notifications
You must be signed in to change notification settings - Fork 7
feat(confmanager): Added ZMS.input.exec.restrict #431
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@drfho & @OscarMeier As discussed earlier today I introduced a new conf property It preserves handling in |
274134a to
a8f1f87
Compare
Prevent usage of these keywords on standard.dt_exec or standard.get_env
a8f1f87 to
e02b822
Compare
This limits the check to actual code execution.
6b3fb9a to
bafe60e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, according to "Restricted Python" we may choose a similar term, because the approach has a similar goal: preventing code executions by defining restricted function names. Moreover there is a term "input" qualifying confproperties, dealing with user-entries.
Putting it together the terminology might be:
- ZMS.input.exec.restrict
- check_restricted_inputs(context, value, can_ignore)
The param 'can_ignore' is a doubled negation? ignore restriction vs. forcing the restriction? If the confprop ist not available, its restriction shall be forced, right? Why not give a default here, like check_restricted_inputs(context, value, force_restriction=False)
drfho
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@drfho I would have expected the message and traceback from the raise exception - as seen in my tests:
|
558eeca to
36d1acc
Compare


On discussing and reverting #430 (comment) we suggest these further actions - but we are unsure whether it has unintended consequences. What do you think...?! cc/ @OscarMeier