Skip to content

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

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

Zheaoli
Copy link
Contributor

@Zheaoli Zheaoli commented Jun 15, 2025

Zheaoli added 2 commits June 16, 2025 02:31
…c has been called

Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
Zheaoli added 2 commits June 16, 2025 03:09
Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
Zheaoli and others added 7 commits June 17, 2025 03:06
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>
@Zheaoli
Copy link
Contributor Author

Zheaoli commented Jun 16, 2025

@vstinner @sobolevn @pablogsal I have updated the test. PTAL when you got time

Zheaoli added 3 commits June 17, 2025 22:44
Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
@Zheaoli
Copy link
Contributor Author

Zheaoli commented Jun 17, 2025

@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?

@Zheaoli Zheaoli requested a review from vstinner June 17, 2025 15:21
Copy link
Member

@vstinner vstinner left a 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>
Zheaoli and others added 3 commits June 17, 2025 23:40
Co-authored-by: Victor Stinner <vstinner@python.org>
Co-authored-by: Victor Stinner <vstinner@python.org>
Signed-off-by: Manjusaka <me@manjusaka.me>
@Zheaoli Zheaoli requested a review from vstinner June 17, 2025 15:41
@Zheaoli
Copy link
Contributor Author

Zheaoli commented Jun 17, 2025

@vstinner PTAL when you got time (lol

Copy link
Member

@vstinner vstinner left a 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>
@Zheaoli
Copy link
Contributor Author

Zheaoli commented Jun 18, 2025

@vstinner sorry about this, I update a little bit of code. PTAL

Zheaoli added 2 commits June 18, 2025 22:38
Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
Zheaoli added 2 commits June 19, 2025 02:42
Signed-off-by: Manjusaka <me@manjusaka.me>
Signed-off-by: Manjusaka <me@manjusaka.me>
@Zheaoli Zheaoli requested a review from zooba June 18, 2025 18:44
@pablogsal
Copy link
Member

LGTM.

@pablogsal: what do you think?

I am ok with this 👍

Comment on lines +1938 to +1939
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.
Copy link
Member

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.

Comment on lines 1943 to 1946
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.
Copy link
Member

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*.

Comment on lines +654 to +657
nonlocal event_pid
event_pid = args[0]
nonlocal event_script_path
event_script_path = args[1]
Copy link
Member

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add new audit event for sys.remote_exec()
5 participants