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

Replace grep with getent in entrypoint.sh #18466

Merged
merged 1 commit into from Oct 25, 2023
Merged

Replace grep with getent in entrypoint.sh #18466

merged 1 commit into from Oct 25, 2023

Conversation

nfsec
Copy link
Contributor

@nfsec nfsec commented Oct 18, 2023

Use simpler, native "getent" command to get entries from NSS insead of "grep".

Use native "getent" command to get entries from NSS insead of "grep".
@adfoster-r7
Copy link
Contributor

Thanks for the PR! I'm in favor of the existing grep functionality here as it's a bit more human readable

@nfsec
Copy link
Contributor Author

nfsec commented Oct 19, 2023

Can you please explain how grep is more readable?

@adfoster-r7
Copy link
Contributor

adfoster-r7 commented Oct 19, 2023

My opinion - everyone knows about grep - someone seeing this for the first time is likely to have to read the getent docs 😄

@nfsec
Copy link
Contributor Author

nfsec commented Oct 19, 2023

Thanks for the opinion. My opinion is that learning commands should not end on grep.

@bwatters-r7
Copy link
Contributor

Honestly, I'd never see this command before, so I looked it up, and this situation seems exactly what this command is for? My only concern is that online someone suggested their zshell implemented this manually rather than using a POSIX executable, though they did not site the OS. Forgive my ignorance of this command, but I know grep is well-supported across a long history of unix/linux/BSD, and from some brief reading it appears getent is as well.
I did see varying opinions on whether or not it was supported on OSX; my assumption is that it would work. Also, since this is to let people run Metasploit in a docker container, I'd expect anything to be relatively recent.

TL;DR: I'm for it, but we should make sure the support is universal to the expected use-cases.

@adfoster-r7
Copy link
Contributor

adfoster-r7 commented Oct 25, 2023

For my env on OSX with zsh it was a shell script yeah, which just uses grep:

$ which getent
getent () {
	if [[ $1 = hosts ]]
	then
		sed 's/#.*//' /etc/$1 | grep -w $2
	elif [[ $2 = <-> ]]
	then
		grep ":$2:[^:]*$" /etc/$1
	else
		grep "^$2:" /etc/$1
	fi
}

And it wasn't available in OSX's bash:

bash-3.2$ getent
bash: getent: command not found

Since this is only being used in the docker entrypoint though in an environment we control, and if we ever change the base image the test suite will fail if getent isn't available, I'm happy enough to land this 👍

@adfoster-r7 adfoster-r7 merged commit 216f6fb into rapid7:master Oct 25, 2023
35 checks passed
@adfoster-r7
Copy link
Contributor

Release Notes

Updates the docker entrypoint script to use getent instead of grep when detecting user/group details

@adfoster-r7 adfoster-r7 added the rn-enhancement release notes enhancement label Oct 25, 2023
@nfsec nfsec deleted the patch-1 branch October 25, 2023 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement rn-enhancement release notes enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants