Repository navigation
Harden CHEF_POWERSHELL_BIN/architecture resolution and fix a native assembly-resolve crash - #382
Merged
Merged
Conversation
Signed-off-by: John McCrae <mccrae@progress.com>
Signed-off-by: John McCrae <mccrae@progress.com>
Signed-off-by: John McCrae <mccrae@progress.com>
Signed-off-by: John McCrae <mccrae@progress.com>
tpowell-progress
approved these changes
Sep 14, 2026
tpowell-progress
left a comment
Contributor
There was a problem hiding this comment.
We probably want to extract arch = ENV["PROCESSOR_ARCHITECTURE"].to_s.empty? ? "AMD64" : ENV["PROCESSOR_ARCHITECTURE"] to an arch.rb helper, but this works for now.
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.
What we did
While building a fault-injection test harness (
chef-powershell/chef_gem_break_test.rb) to deliberately try to reproduce error 126 / "missing DLL" failures againstchef-powershell, we found and fixed three real bugs, plus added the harness itself as a permanent regression tool.1.
CHEF_POWERSHELL_BINwas silently clobbered atrequiretimepowershell_exec.rbunconditionally overwroteENV["CHEF_POWERSHELL_BIN"]whenever the loaded gem's own bundled DLL folder existed — even if the caller had already set it intentionally (e.g. to point at a freshly built Habitat package for testing, or any other override). Fixed to only auto-detect when the variable isn't already set, so an explicit override is always respected.2.
ENV["PROCESSOR_ARCHITECTURE"] || "AMD64"doesn't fall back correctlyRuby's
||only falls back onnil/false, not on an empty string. IfPROCESSOR_ARCHITECTUREis set but blank (confirmed reproducible via the fault-injection harness), this pattern silently built a broken path instead of defaulting toAMD64, causing aLoadError. Fixed inpwsh.rb,powershell.rb,powershell_exec.rb, andextconf.rb.3. Native assembly resolution could hard-crash the whole
ruby.exehostChef.PowerShell.Wrapper.cpp'sAssemblyResolvehandler readCHEF_POWERSHELL_BINindependently of Ruby's own DLL resolution logic. If that env var was set to anything that didn't containChef.PowerShell.dll— even while the wrapper DLL itself had been correctly loaded from a completely different, correct location — the resolver only triedCHEF_POWERSHELL_BINand gave up, causing an unhandled managed exception that crashes the entire process (SIGSEGV, not a catchable Ruby exception). Fixed by adding a fallback to the directory the wrapper assembly itself was actually loaded from, so a stale/wrongCHEF_POWERSHELL_BINcan no longer break assembly resolution when the correct DLL is sitting right next to the wrapper.4. New:
chef_gem_break_test.rbfault-injection harnessA permanent, repo-committed script that deliberately sabotages chef-powershell in ways a real user/CI/machine might (accidentally or otherwise) and verifies the result is either full resilience or a clean, understandable failure — never a silent wrong result or an unexplained crash. Covers:
hostfxr.dll, missingshared/Microsoft.NETCore.App, missing wrapper DLLs, missing CRT dependencies)DOTNET_ROOT, bogusCHEF_POWERSHELL_BIN, blankPROCESSOR_ARCHITECTURE)CHEF_HAB_IDENT/CHEF_OMNIBUS_RUBY)Every scenario operates on disposable scratch copies — it never touches a real installed
chef-powershell-shim/chef-infra-clientpackage.5. Regression spec
Added an integration spec locking in fix #1 (
does not clobber an explicit CHEF_POWERSHELL_BIN override when requiring chef-powershell).Why this helps
Error 126 ("missing DLL") has been a recurring, hard-to-diagnose failure mode for this gem. These fixes close three concrete gaps that could each independently reproduce that class of failure in real-world conditions (a stale env var left over from a previous run, a machine with a blank/unusual
PROCESSOR_ARCHITECTURE, or any situation whereCHEF_POWERSHELL_BINdoesn't exactly match where the wrapper DLL was actually loaded from). The fault-injection harness turns "we think this is fixed" into something we can re-verify on demand, and gives us a repeatable way to catch the next regression before it ships.Verification
chef_gem_break_test.rb: confirmed each fix flips its corresponding scenario from crash/failure to the expected clean outcomehab pkg build habitat(0 compile errors) and re-tested against the harness and full spec suiteKnown follow-up
A handful of
chef_gem_break_test.rbscenarios (missing wrapper DLL, missing CRT dependency, stale multi-version shadowing) still report unexpected "success" on this dev machine — traced directly to this machine's own pre-existingchef/chef-powershell-shimHabitat install acting as a fallback, not a code defect. These should be re-verified in a clean, disposable container without that package pre-installed.