Skip to content

fix(cron): prefer Git Bash for Windows shell scripts - #66769

Open
Bushels wants to merge 2 commits into
NousResearch:mainfrom
Bushels:fix/windows-cron-git-bash
Open

fix(cron): prefer Git Bash for Windows shell scripts#66769
Bushels wants to merge 2 commits into
NousResearch:mainfrom
Bushels:fix/windows-cron-git-bash

Conversation

@Bushels

@Bushels Bushels commented Jul 18, 2026

Copy link
Copy Markdown

Summary

  • use Hermes' Bash resolver in strict native-Windows mode for cron .sh / .bash jobs
  • reject the legacy WSL bash.exe launcher at System32, Sysnative, and SysWOW64, including canonicalized symlink, junction, 8.3, and UNC aliases
  • preserve the default terminal resolver's documented support for user-configured WSL overrides
  • sanitize Git-Bash and Mandatory-ASLR resolver probes before they spawn, and enforce Tier-1 secret stripping across both subprocess environment builders (including _HERMES_FORCE_ inputs)
  • preserve actionable resolver failures such as Mandatory-ASLR remediation instead of replacing them with a generic "install Git" message

Root cause

On native Windows, the gateway PATH can resolve bash to C:\Windows\System32\bash.exe before Git for Windows. That launcher enters WSL and cannot consume the native/MSYS script path passed by cron, producing exit 127 and a mangled path such as C:Users....

The first repair exposed two adjacent hardening gaps during fail-closed review: resolver probes inherited the gateway environment, and resolver RuntimeError details were discarded. A follow-up review also identified alias-based WSL selection and a Tier-1 blocklist drift case. This PR now closes all four paths.

Verification

  • regression tests were observed RED before each repair and GREEN afterward
  • affected Windows-compatible suite: 796 passed, 13 skipped, 26 platform-assumption tests deselected
  • the actual Windows-only failures outside this change reproduce unchanged at base c48d53413 (POSIX/macOS path-separator, HOME-expansion, and mode-bit assumptions)
  • ruff on all five changed files: passed
  • compileall: passed
  • git diff --check: passed
  • Bandit high-severity scan of both changed source files: passed
  • direct runtime checks: strict/default resolver both select C:\Program Files\Git\bin\bash.exe; all three Windows WSL launcher paths classify as rejected in strict mode
  • live gateway verification after the final code: bushel-source-freshness-mwf completed successfully through its .sh script
  • independent fail-closed review: initial findings fixed; final review returned no security concerns or logic errors

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cron Cron scheduler and job management platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Jul 18, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to the open Windows cron Git-Bash cluster (#52204, #46364, #61629). This is a narrow scheduler delegation to the shared resolver; #52204 additionally covers the resolver and MSYS POSIX-path handling, so this is not a duplicate.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed by Hermes Agent

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused Windows cron fix. Static review found no blocking issue.

Current main still uses shutil.which("bash") for cron .sh/.bash scripts at cron/scheduler.py:2182, while the existing resolver at tools/environments/local.py:629-697 prefers Git-for-Windows candidates. The PR delegates only native-Windows cron resolution to that resolver in strict mode, preserving the documented default terminal support for a WSL-backed HERMES_GIT_BASH_PATH override (website/docs/reference/environment-variables.md:107).

The added tests cover scheduler delegation, resolver errors, WSL launcher filtering, canonicalized aliases, and the affected subprocess-environment filtering paths. The affected files were unchanged from this checkout through the PR base, so salvage should be mechanical.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 18, 2026
@Bushels

Bushels commented Jul 20, 2026

Copy link
Copy Markdown
Author

Retested against current origin/main (a41d280f95c69f67380358b305b62345934ecaf3):

  • Local merge of PR head 1cae1105f71fc512e286298ac7ade8bdae1aafc1: conflict-free
  • Affected Windows-compatible suite: 802 passed, 13 skipped, 26 deselected
  • Ruff: passed
  • git diff --check: passed
  • Python compilation: passed
  • Bandit high-severity scan: passed

Ready for maintainer approval and merge.

@alt-glitch alt-glitch added backend/local Local shell execution and removed sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 20, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Seven PRs are associated with this shell-resolution complex: #42219, #42290, #53106, and #53110 address the macOS background-command swallow; #41456 and #66769 address distinct Windows WSL-versus-Git-Bash selection paths; and #53166 bundles broader Windows compatibility work while referencing the same shell area. The macOS issue was resolved by merged #53110, while the two open Windows PRs retain separate focused scopes.

Related pull requests

Duplicates

#42219 and #53106 are substantially the same _find_shell() change, with #53106 being its rebase; both are superseded by the hardened merged implementation in #53110. #42290 targets the same macOS cause through _find_bash() but is not the same implementation; #41456 and #66769 overlap in WSL-launcher filtering but serve native terminal and cron call paths respectively and are not duplicates.

Suggested consolidation

Keep #66769 open with a salvage path: retain the native-Windows cron delegation to _find_bash(reject_wsl=True), WSL-alias rejection, resolver-error propagation, probe-environment sanitization, and their focused tests, as supported by the maintainer-bot keep-open verdict. Keep #41456 open for its distinct focused terminal-resolver gap in line with the contributor review; leave #42219, #42290, #53106, and #53166 closed, with #42219 and #53106 recorded as superseded duplicates of merged reference #53110.

Cross-PR triage: Reviewed 7 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 125 kB of PR diffs, 22 kB of issue/PR text, 10 kB of discussion (19 comments), 6 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend/local Local shell execution comp/cron Cron scheduler and job management P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants