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

[capistrano] Add ability to use etc.getpwuid instead of etc.getlogin to get the user name #146

Merged
merged 4 commits into from
Jan 30, 2020

Conversation

rkul
Copy link
Contributor

@rkul rkul commented Apr 2, 2018

getlogin() obtains the logged-in username by looking up the current tty in utmp and reporting the login name found in that utmp record. If Capistrano is ran from Jenkins in the host it returns the user logged in to pts/0, which is not the same user as runs the Capistrano. Also Ruby docs recommends avoid using getlogin():

getlogin → String
Returns the short user name of the currently logged in user. Unfortunately, it is often rather easy to fool ::getlogin.

Avoid ::getlogin for security-related purposes.

If ::getlogin fails, try ::getpwuid.

See the unix manpage for getpwuid(3) for more detail.
http://ruby-doc.org/stdlib-2.2.0/libdoc/etc/rdoc/Etc.html#method-c-getlogin

@github-actions
Copy link

github-actions bot commented Jan 4, 2020

This issue has been automatically marked as stale because it has not had activity in the last 30 days.
Note that the issue will not be automatically closed, but this notification will remind us to investigate why there's been inactivity.

@github-actions github-actions bot added the stale Stale - Bot reminder label Jan 4, 2020
@github-actions github-actions bot closed this Jan 11, 2020
@gzussa gzussa removed the stale Stale - Bot reminder label Jan 16, 2020
@gzussa gzussa reopened this Jan 16, 2020
@gzussa gzussa requested a review from a team January 16, 2020 15:55
@gzussa
Copy link
Contributor

gzussa commented Jan 16, 2020

Hi @rkul Sorry for the late reply,
Would you be able to fix conflicts.
Also it seems that this change would potentially introduce backward incompatibility, wouldn't it?

@gzussa gzussa self-assigned this Jan 16, 2020
@gzussa gzussa added backward-incompatible Warns for backward incompatible changes changelog/Changed Changed features results into a major version bump labels Jan 16, 2020
rkul and others added 2 commits January 30, 2020 14:36
`getlogin()` obtains the logged-in username by looking up the current tty in `utmp` and reporting the login name found in that utmp record. If Capistrano is ran from Jenkins in the host it returns the user logged in to `pts/0`, which is not the same user as runs the Capistrano. Also Ruby docs recommends avoid using `getlogin()`:
>getlogin → String
>Returns the short user name of the currently logged in user. Unfortunately, it is often rather easy to fool ::getlogin.
>
>Avoid ::getlogin for security-related purposes.
>
>If ::getlogin fails, try ::getpwuid.
>
>See the unix manpage for getpwuid(3) for more detail.
http://ruby-doc.org/stdlib-2.2.0/libdoc/etc/rdoc/Etc.html#method-c-getlogin
@zippolyte zippolyte added changelog/Added Added features results into a minor version bump and removed backward-incompatible Warns for backward incompatible changes changelog/Changed Changed features results into a major version bump labels Jan 30, 2020
bkabrda
bkabrda previously approved these changes Jan 30, 2020
@zippolyte zippolyte changed the title Do not use getlogin() function to obtain username [capistrano] Add ability to use etc.getpwuid instead of etc.getlogin to get the user name Jan 30, 2020
Copy link
Contributor

@gzussa gzussa left a comment

Choose a reason for hiding this comment

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

Thanks 💯

@zippolyte zippolyte merged commit 0f2fa56 into DataDog:master Jan 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog/Added Added features results into a minor version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants