fix(cli): strip WIGOLO_STUDIO_HIDDEN case-insensitively in runStudio - #508
Merged
KnockOutEZ merged 1 commit intoAug 28, 2026
Merged
Conversation
Spreading process.env collapses win32's case-insensitive env proxy into a plain object, so `delete env.WIGOLO_STUDIO_HIDDEN` removed exactly one spelling. A shell exporting `wigolo_studio_hidden=1` survived the delete, the child read it back through its own case-insensitive process.env, and the human who typed `wigolo studio` got no window — the exact outcome the visible-launch contract exists to deny. Filter the spread on an uppercased key match instead. Asserted on the plain object handed to the injected spawn seam, so the pin is platform-independent.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes wigolo-studio-run#179 (PX0 exit-4, security reviewer, CONFIRMED LOW).
The defect
runStudio's docstring claims the hidden flag is "stripped rather than merelyunset, in case the shell already carried it". Spreading
process.envcollapsesNode's win32 case-insensitive env proxy into a plain object, so
delete env.WIGOLO_STUDIO_HIDDENremoved exactly one spelling. A shell thatexported
wigolo_studio_hidden=1left that key sitting in the spread; the childread it back through its OWN case-insensitive
process.envand started hidden.The human typed
wigolo studioand got no window — the precise outcome thevisible-launch contract exists to deny.
Moot on today's darwin-arm64-only substrate, but the code asserted a guarantee
it did not deliver on one platform.
The fix
Filter the spread on an uppercased key match instead of deleting one spelling.
toUpperCase()is locale-independent, so the comparison does not vary by locale.The docstring now states the case rule and why it exists.
The pin
New arm in
tests/unit/cli/studio.test.tsasserting on the plain object handedto the injected spawn seam, so it is platform-independent: it reds on darwin
today if the filter regresses. It plants a lowercase
wigolo_studio_hidden,then asserts that no key whose uppercase spelling is the flag survives — and,
as the anti-vacuity half, that
PATHstill comes through, so the pin cannot besatisfied by handing spawn an empty env.
Mutation-proven, not N clean runs. Reverting the filter to the original bare
delete:
Restored:
85 passed (85). The six pre-existingrunStudioarms are among the84 survivors, so the new arm is the only thing that moves.
Auto-launch, checked
defaultLaunch(src/studio/auto-launch.ts:160) has the mirror-image shape,but SETTING is the weaker case: it writes the canonical spelling, and the worst
outcome is an unexpectedly visible auto-launch rather than a swallowed window.
It is a declared non-goal here (pinned by sd-164/165) and
src/studio/**isoutside lane
core:cli, so it is logged inknown-issues.mdrather than fixed.Non-goals honoured
Windows substrate support; no change to what auto-launch sets.