Keep terminal in a sane state when sudo use_pty is used.#923
Merged
brianmay merged 1 commit intosshuttle:masterfrom Jan 5, 2024
Merged
Keep terminal in a sane state when sudo use_pty is used.#923brianmay merged 1 commit intosshuttle:masterfrom
brianmay merged 1 commit intosshuttle:masterfrom
Conversation
This fixes sshuttle#909 and is an alternative to the sshuttle#922 pull request. When sudo's use_pty is used with sshuttle, it causes issues with the terminal. Pull request sshuttle#712 contains some fixes for this problem. However, when sshuttle is run with the --daemon option, it left the user's terminal in a non-sane state. The problem appears to be related to a socketpair that the firewall uses for communication. By setting it up slightly differently (see changes to client.py and firewall.py), the terminal state is no longer disrupted. This commit also changes line endings of the printed messages from \r\n to \n. This undoes a change introduced by pull request sshuttle#712 and is no longer needed.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| # with \r\n endings fixes that problem and does not appear | ||
| # to cause problems elsewhere. | ||
| sys.stderr.write(prefix + line + "\r\n") | ||
| sys.stderr.write(prefix + line + "\n") |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information
Contributor
Author
|
@dancek, does this fix the problem for you? Any thoughts? I very much appreciate your pull request. It helped me more quickly figure out this solution---which seems to get at the root of the problem. |
|
Yes, this seems to work for me. Looks like a good fix for the root cause, good job! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This fixes #909 and is an alternative to the #922 pull request. When sudo's use_pty is used with sshuttle, it causes issues with the terminal. Pull request #712 contains some fixes for this problem. However, when sshuttle is run with the --daemon option, it left the user's terminal in a non-sane state. The problem appears to be related to a socketpair that the firewall uses for communication. By setting it up slightly differently (see changes to client.py and firewall.py), the terminal state is no longer disrupted. This commit also changes line endings of the printed messages from \r\n to \n. This undoes a change introduced by pull request #712 and is no longer needed.