Skip to content

fix: strip instance specific env vars from launched processes - #178

Merged
Strobotti merged 3 commits into
Strobotti:mainfrom
tjemke:fix/strip-env-vars
Aug 7, 2026
Merged

fix: strip instance specific env vars from launched processes#178
Strobotti merged 3 commits into
Strobotti:mainfrom
tjemke:fix/strip-env-vars

Conversation

@tjemke

@tjemke tjemke commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

This fixes a very specific and somewhat obscure bug I encountered while opening a link from signal-desktop in tidal-hifi. All of the env variables that signal launched with got inherited by linkquisition which in turn passed them down to tidal-hifi. As both tidal-hifi and signal are electron applications, the CHROME_DESKTOP variable was still set to the value it received for (and from) signal, which somehow caused tidal-hifi to segfault.

At first I just changed my manual entry for tidal-hifi to unset the var, which would suffice as a fix if you are against touching the process environment at all. As linkquistion is always going to play the role of a middleman between two apps though, I'd argue that this in scope for what this app should do, as for an (admittedly very specific) edge case like this, it would not be able to fulfill its role in that chain. I'd be interested in your view though :)

I also added this to all platforms as it should apply equally to all of them. I verified that the env also gets inherited on MacOS in the same way that it does on Unix (see also the open(1) man page. I don't currently have a windows install lying around, so those changes are speculative.

Currently, I have not added any test for these changes as I thought their scope to be small enough. I'd be happy to take a crack at building them though, just let me know if a small unit test suffices or if you'd like a full integration test as well :)

Also thanks for building this thing, it solved a very specific problem I had 😅

full disclosure: the list of variables was largely picked out by claude opus 5 but went through a lot of source code checks which makes me fairly confident that it holds.

Comment thread internal/launchenv/launchenv.go Outdated

@Strobotti Strobotti left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for this! I might add a unit test for the SanitizeEnviron later 👍

@Strobotti
Strobotti merged commit 74b66ff into Strobotti:main Aug 7, 2026
7 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