Skip to content

Commit 789fc7f

Browse files
committed
Hold the reentrancy flag while degrading in the fail-safe path
Addresses Copilot review on PR #662. The fail-safe catch inside prompt delegated to the wrapped prompt WITHOUT setting $Global:__ShellInteg_Rendering. The normal path clears that flag in its finally before the catch runs, so an adopted CHAINING prompt calls back into us with the flag false and the guard does not fire. Reproduced with a chaining prompt plus a forced failure in the render path: 'CHAIN|CHAIN|CHAIN| ... (130+ levels) ... PS C:\Users\yeelam> ' That is the exact unbounded mutual-delegation cycle the guard exists to prevent; only the engine's call-depth limit stopped it. After the fix the same repro returns 'CHAIN|BASE> ' in 12ms, and the base prompt text is preserved. Two smaller review points in the same commit: - The test took script.substr(rearmFunc) without first proving rearmFunc is not npos, so a missing marker would surface as std::out_of_range instead of a clear VERIFY failure. Added an explicit VERIFY_ARE_NOT_EQUAL. - check-spelling: 'scriptblocks' is not a recognized word - reworded to 'script blocks'. RendersFailSafe now asserts the flag is set before the degraded delegation and cleared in finally; mutation-tested by removing the assignment, which fails the test as intended. 104/104 ShellIntegrationTests pass. Behavioural suite still green on Windows PowerShell 5.1 and PowerShell 7, and the script still survives Constrained Language Mode on both.
1 parent a78f404 commit 789fc7f

2 files changed

Lines changed: 23 additions & 9 deletions

File tree

src/cascadia/UnitTests_TerminalCore/ShellIntegrationTests.cpp

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -515,9 +515,11 @@ void ShellIntegrationTests::PowerShell_ScriptContent_ReArmsFromReadLineBoundary(
515515
// Detection is by object identity against our own scriptblock — never by
516516
// text matching, which a renderer could accidentally satisfy.
517517
const auto rearmFunc = script.find("function Global:__ShellInteg_Rearm");
518-
// -eq, NOT [object]::ReferenceEquals: re-arm runs at script load and from the
519-
// readline boundary, and a static method call under Constrained Language Mode
520-
// aborts the enclosing script even from inside try/catch.
518+
VERIFY_ARE_NOT_EQUAL(std::string::npos, rearmFunc,
519+
L"__ShellInteg_Rearm must exist");
520+
// -eq, NOT a static reference-equality method: re-arm runs at script load and
521+
// from the readline boundary, and a static method call under Constrained
522+
// Language Mode aborts the enclosing script even from inside try/catch.
521523
const auto identityCheck = script.find("if ($current -eq $Global:__ShellInteg_Wrapper) { return }", rearmFunc);
522524
const auto noStaticCall = script.substr(rearmFunc).find("[object]::ReferenceEquals");
523525
const auto adopt = script.find("$Global:__ShellInteg_OriginalPrompt = $current", identityCheck);
@@ -628,15 +630,19 @@ void ShellIntegrationTests::PowerShell_ScriptContent_RendersFailSafe()
628630
const auto renderTry = script.find("try {", promptStart);
629631
const auto normalReturn = script.find("return \"${prefix}${originalOutput}${suffix}\"", renderTry);
630632
const auto renderCatch = script.find("catch {", normalReturn);
631-
const auto degradeToPrompt = script.find("try { return (& $Global:__ShellInteg_OriginalPrompt) }", renderCatch);
633+
const auto failSafeFlag = script.find("$Global:__ShellInteg_Rendering = $true", renderCatch);
634+
const auto degradeToPrompt = script.find("try { return (& $Global:__ShellInteg_OriginalPrompt) }", failSafeFlag);
632635
const auto lastResort = script.find("catch { return \"PS $($executionContext.SessionState.Path.CurrentLocation)> \" }", degradeToPrompt);
636+
const auto failSafeClear = script.find("finally { $Global:__ShellInteg_Rendering = $false }", lastResort);
633637

634-
VERIFY_ARE_NOT_EQUAL(std::string::npos, lastResort);
638+
VERIFY_ARE_NOT_EQUAL(std::string::npos, failSafeClear);
635639
VERIFY_IS_TRUE(renderTry < normalReturn &&
636640
normalReturn < renderCatch &&
637-
renderCatch < degradeToPrompt &&
638-
degradeToPrompt < lastResort,
639-
L"Rendering must fall back to the wrapped prompt, then to a plain prompt, instead of throwing");
641+
renderCatch < failSafeFlag &&
642+
failSafeFlag < degradeToPrompt &&
643+
degradeToPrompt < lastResort &&
644+
lastResort < failSafeClear,
645+
L"The fail-safe must hold the re-entrancy flag while degrading, or an adopted chaining prompt recurses to the call-depth limit");
640646
}
641647

642648
// ─── Install ──────────────────────────────────────────────────────────────────

src/cascadia/inc/PowerShellShellIntegration.h

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -588,8 +588,16 @@ if (-not $Global:__ShellInteg_Installed) {
588588
# FAIL SAFE. Shell integration is a convenience; a broken prompt is
589589
# not. Degrade to the wrapped prompt without marks rather than
590590
# throwing on every prompt.
591+
#
592+
# The re-entrancy flag must be held here too. The normal path clears
593+
# it in its finally before this catch runs, so an adopted CHAINING
594+
# prompt would call back into us with the flag false and recurse
595+
# until the engine's call-depth limit - the very cycle the guard
596+
# above exists to prevent.
597+
$Global:__ShellInteg_Rendering = $true
591598
try { return (& $Global:__ShellInteg_OriginalPrompt) }
592599
catch { return "PS $($executionContext.SessionState.Path.CurrentLocation)> " }
600+
finally { $Global:__ShellInteg_Rendering = $false }
593601
}
594602
}
595603
@@ -619,7 +627,7 @@ function Global:__ShellInteg_Rearm {
619627
# call raises "Method invocation is supported only on core types", which
620628
# aborts the enclosing script even from inside try/catch - breaking profile
621629
# sourcing or the input path. ScriptBlock does not override Equals, so -eq is
622-
# reference identity here (verified: two scriptblocks with identical text
630+
# reference identity here (verified: two script blocks with identical text
623631
# compare False), and the operator is permitted in Constrained Language Mode.
624632
# That keeps re-arming working there instead of disabling it.
625633
$current = $function:prompt

0 commit comments

Comments
 (0)