Skip to content
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

Spawn connection-checker as an unprivileged user #6656

Merged
merged 1 commit into from
Aug 22, 2024
Merged

Conversation

dlon
Copy link
Member

@dlon dlon commented Aug 21, 2024

test-runner now spawns all processes as an unprivileged user.

Fix DES-1131
Fix DES-1129


This change is Reviewable

@dlon dlon marked this pull request as ready for review August 21, 2024 16:07
@dlon dlon force-pushed the fix-macos-mul-02-leak branch from f603283 to 8869e29 Compare August 21, 2024 16:23
@dlon dlon requested a review from hulthe August 21, 2024 16:23
@dlon dlon force-pushed the fix-macos-mul-02-leak branch 2 times, most recently from ed0dde2 to ac7cdf2 Compare August 21, 2024 18:06
Copy link
Contributor

@hulthe hulthe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 5 of 6 files at r1, all commit messages.
Reviewable status: 5 of 6 files reviewed, 2 unresolved discussions (waiting on @dlon)


-- commits line 9 at r1:
👌


test/test-runner/src/util.rs line 84 at r1 (raw file):

            log::error!("Failed to restore uid: {error}");
        }
    });

Nice work with the drop guards. They should take care of any bugs if func panics too, right?

@dlon dlon force-pushed the fix-macos-mul-02-leak branch 2 times, most recently from 663803c to de57355 Compare August 22, 2024 13:17
Copy link
Member Author

@dlon dlon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 4 of 6 files reviewed, 1 unresolved discussion (waiting on @hulthe)


test/test-runner/src/util.rs line 84 at r1 (raw file):

Previously, hulthe (Joakim Hulthe) wrote…

Nice work with the drop guards. They should take care of any bugs if func panics too, right?

Yep. Probably not too important in reality, since the runner would crash and (hopefully) be restarted anyway.

@dlon dlon force-pushed the fix-macos-mul-02-leak branch from de57355 to b8e3ff3 Compare August 22, 2024 13:21
@dlon dlon requested a review from hulthe August 22, 2024 13:45
Copy link
Contributor

@hulthe hulthe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 6 files at r1, 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion

Fixes an issue where the connection-checker is allowed to leak traffic
on macOS
@dlon dlon force-pushed the fix-macos-mul-02-leak branch from b8e3ff3 to 283d1eb Compare August 22, 2024 18:19
@dlon dlon merged commit 283d1eb into main Aug 22, 2024
37 of 38 checks passed
@dlon dlon deleted the fix-macos-mul-02-leak branch August 22, 2024 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants