Skip to content

fix: /etc/environment keys with underscores being dropped#391

Merged
ethanpailes merged 1 commit into
shell-pool:masterfrom
wadetregaskis:4
Jun 13, 2026
Merged

fix: /etc/environment keys with underscores being dropped#391
ethanpailes merged 1 commit into
shell-pool:masterfrom
wadetregaskis:4

Conversation

@wadetregaskis

Copy link
Copy Markdown
Contributor

AI Policy Ack

AI Policy

This PR was:

✅ completely vibe coded

Found by Claude Fable 5.

Description

The key validation only accepted alphanumeric characters, but pam_env (whose parsing this file explicitly ports) also allows underscores — and almost every real /etc/environment entry (JAVA_HOME, LC_ALL, HTTP_PROXY, XDG_DATA_DIRS, ...) contains one. All such variables were silently missing from shpool sessions while normal ssh/console logins got them. Match pam_env: ascii alphanumerics plus underscore.

This is hypothetical for me - I don't think I've personally run into issues as a result of this - but it does seem like a valid concern; environmental variable keys do often contain underscores and that's explicitly supported in /etc/environment per the pam_env source. I checked on my Linux work box and indeed bash running inside shpool doesn't have any of the underscored environment variables, whereas bash under SSH does.

The key validation only accepted alphanumeric characters, but pam_env
(whose parsing this file explicitly ports) also allows underscores —
and almost every real /etc/environment entry (JAVA_HOME, LC_ALL,
HTTP_PROXY, XDG_DATA_DIRS, ...) contains one. All such variables were
silently missing from shpool sessions while normal ssh/console logins
got them. Match pam_env: ascii alphanumerics plus underscore.

@ethanpailes ethanpailes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, thanks!

@ethanpailes ethanpailes merged commit a9783d5 into shell-pool:master Jun 13, 2026
15 of 16 checks passed
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