-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
gh-135543: emit sys.remote_exec
audit event when sys.remote_exec
has been called
#135544
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
…c has been called Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
Misc/NEWS.d/next/Core_and_Builtins/2025-06-16-02-31-42.gh-issue-135543.6b0HOF.rst
Outdated
Show resolved
Hide resolved
Co-authored-by: Victor Stinner <vstinner@python.org>
Co-authored-by: Victor Stinner <vstinner@python.org>
…e-135543.6b0HOF.rst Co-authored-by: Victor Stinner <vstinner@python.org>
Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
… manjusaka/fix135543
@vstinner @sobolevn @pablogsal I have updated the test. PTAL when you got time |
Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
@vstinner I have fix the review idea. PTAL when you got time. BTW https://github.com/python/cpython/actions/runs/15710806342/job/44268135397?pr=135544 seems unrelated failed. Would you mind rerunning it for me? |
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.
Thanks for the update, I like the updated code. Another review.
Co-authored-by: Victor Stinner <vstinner@python.org>
Co-authored-by: Victor Stinner <vstinner@python.org>
Co-authored-by: Victor Stinner <vstinner@python.org>
Signed-off-by: Manjusaka <me@manjusaka.me>
@vstinner PTAL when you got time (lol |
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.
LGTM.
@pablogsal: what do you think?
Signed-off-by: Manjusaka <me@manjusaka.me>
@vstinner sorry about this, I update a little bit of code. PTAL |
Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
I am ok with this 👍 |
When the code is executed in the remote process, an :ref:`auditing event <auditing>` | ||
``sys.remote_exec`` is raised with the *pid* and the path to the script file. |
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.
This isn't true, is it? This event gets raised in the calling process when the function is called (the idea is that an audit hook can abort the operation from ever starting, though we don't actively encourage it).
The current text suggests that it's raised asynchronously. Probably the default text here is fine for describing this event - it's the event below that needs better explanation.
When the script is executed in the remote process, an | ||
:ref:`auditing event <auditing>` | ||
``sys.remote_debugger_script`` is raised | ||
``cpython.remote_debugger_script`` is raised | ||
with the path in the remote process. |
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.
This can probably be clearer. It seems the important part is that it's raised in the remote process (the second "in the remote process" is ambiguous - it could refer to the path or the raise).
The default text is no good here, but perhaps this should be:
Before the remote Python process begins executing a script invoked
by this function, an :ref:`auditing event <auditing>`
``cpython.remote_debugger_script`` is raised with the path of the script.
This event is raised in the remote process, not the one that called
*remote_exec*.
nonlocal event_pid | ||
event_pid = args[0] | ||
nonlocal event_script_path | ||
event_script_path = args[1] |
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.
A better way to structure this test would be to print the event info and also print the expected info before calling it, then compare the two lines in the actual test function.
That way if it doesn't match, we've captured the actual values, which are going to be the best hint about what's wrong and how to fix it. Right now, we only get a "pass/fail" reported, and if there's no way to reproduce the issue outside of tests, we'll have to update the test to get enough info to fix it. May as well start by ensuring the info makes it into the test itself, rather than simply failing in the subprocess.
event_pid = -1 | ||
event_script_path = "" | ||
def hook(event, args): | ||
if event != "sys.remote_exec": return |
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.
Since we're calling ourselves, we can also capture cpython.remote_debugger_script
here and validate both events with the same test.
Fix #135543
sys.remote_exec()
#135543📚 Documentation preview 📚: https://cpython-previews--135544.org.readthedocs.build/