[CmdPal] Harden gallery extension lifecycle

Keep gallery installs aligned with the Phase 4 lifecycle and wire contract while tightening cancellation, cleanup, path safety, and mixed-source state.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Michael Jolley
2026-08-26 22:36:53 -05:00
parent dad11790a6
commit a1af071496
11 changed files with 529 additions and 83 deletions

View File

@@ -57,6 +57,9 @@ public sealed partial class ExtensionGalleryItemViewModel : ObservableObject
private readonly Uri? _homepageHttpUri;
private readonly Uri? _authorPageHttpUri;
private readonly Uri? _installLinkHttpUri;
private bool _isDetectedInstalled;
private bool _isWinGetInstalled;
private bool _isWinGetInstalledStateKnown;
// Backs Cancel while a jsonrpc install or uninstall is running.
private CancellationTokenSource? _jsonRpcActionCts;
@@ -89,11 +92,7 @@ public sealed partial class ExtensionGalleryItemViewModel : ObservableObject
// packages that are installed and blocks reinstall into an active directory.
if (_jsExtensionInstaller is not null && HasJsonRpcSource)
{
if (_jsExtensionInstaller.IsInstalled(Id))
{
IsInstalled = true;
}
IsJsonRpcInstalled = _jsExtensionInstaller.IsInstalled(Id);
IsInstalledStateKnown = true;
}
}
@@ -262,7 +261,7 @@ public sealed partial class ExtensionGalleryItemViewModel : ObservableObject
public bool ShowWinGetUnavailableMessage => !string.IsNullOrWhiteSpace(WinGetUnavailableMessage);
public bool ShowInstallViaWinGetButton => HasWinGetSource && (!IsInstalled || IsUpdateAvailable);
public bool ShowInstallViaWinGetButton => HasWinGetSource && (!_isWinGetInstalled || IsUpdateAvailable);
public bool CanInstallViaWinGet => ShowInstallViaWinGetButton && IsWinGetAvailable && !IsWinGetActionInProgress;
@@ -325,9 +324,9 @@ public sealed partial class ExtensionGalleryItemViewModel : ObservableObject
? string.Empty
: IsUpdateAvailable
? Resources.gallery_item_winget_status_update_available
: IsInstalled
: _isWinGetInstalled
? Resources.gallery_item_winget_status_installed
: IsInstalledStateKnown
: _isWinGetInstalledStateKnown
? Resources.gallery_item_winget_status_not_installed
: Resources.gallery_item_winget_status_unavailable;
@@ -341,11 +340,14 @@ public sealed partial class ExtensionGalleryItemViewModel : ObservableObject
[ObservableProperty]
public partial string? JsonRpcActionMessage { get; set; }
public bool ShowInstallViaNpmButton => HasJsonRpcSource && !IsInstalled;
[ObservableProperty]
public partial bool IsJsonRpcInstalled { get; set; }
public bool ShowInstallViaNpmButton => HasJsonRpcSource && !IsJsonRpcInstalled;
public bool CanInstallViaNpm => ShowInstallViaNpmButton && _jsExtensionInstaller is not null && !IsJsonRpcActionInProgress;
public bool ShowUninstallJsonRpcButton => HasJsonRpcSource && IsInstalled;
public bool ShowUninstallJsonRpcButton => HasJsonRpcSource && IsJsonRpcInstalled;
public bool CanUninstallJsonRpc => ShowUninstallJsonRpcButton && _jsExtensionInstaller is not null && !IsJsonRpcActionInProgress;
@@ -366,10 +368,13 @@ public sealed partial class ExtensionGalleryItemViewModel : ObservableObject
public void ApplyWinGetPackageInfo(WinGetPackageInfo packageInfo)
{
IsInstalled = IsInstalled || packageInfo.Status.IsInstalled;
_isWinGetInstalled = packageInfo.Status.IsInstalled;
_isWinGetInstalledStateKnown = packageInfo.Status.IsInstalledStateKnown;
UpdateAggregateInstalledState();
IsInstalledStateKnown = IsInstalledStateKnown || packageInfo.Status.IsInstalledStateKnown;
IsUpdateAvailable = packageInfo.Status.IsUpdateAvailable;
IsUpdateStateKnown = packageInfo.Status.IsUpdateStateKnown;
NotifyWinGetInstallStateChanged();
if (packageInfo.Details is null)
{
@@ -379,6 +384,13 @@ public sealed partial class ExtensionGalleryItemViewModel : ObservableObject
ApplySourceDetails(SourceTypeWinGet, CreateSourceDetails(packageInfo.Details));
}
public void ApplyDetectedInstallationState(bool isInstalled)
{
_isDetectedInstalled = isInstalled;
UpdateAggregateInstalledState();
IsInstalledStateKnown = true;
}
[RelayCommand(CanExecute = nameof(HasHomepage))]
private void OpenHomepage()
{
@@ -452,12 +464,14 @@ public sealed partial class ExtensionGalleryItemViewModel : ObservableObject
var result = await _jsExtensionInstaller.InstallAsync(Id, JsonRpcPackageId, JsonRpcVersion, JsonRpcIntegrity, JsonRpcRegistry, cts.Token).ConfigureAwait(true);
if (result.Succeeded)
{
IsInstalled = true;
IsJsonRpcInstalled = true;
IsInstalledStateKnown = true;
JsonRpcActionMessage = Resources.gallery_item_jsonrpc_action_installed;
}
else
{
IsJsonRpcInstalled = _jsExtensionInstaller.IsInstalled(Id);
IsInstalledStateKnown = true;
JsonRpcActionMessage = result.ErrorMessage ?? Resources.gallery_item_jsonrpc_action_install_failed;
}
}
@@ -488,12 +502,14 @@ public sealed partial class ExtensionGalleryItemViewModel : ObservableObject
var result = await _jsExtensionInstaller.UninstallAsync(Id, cts.Token).ConfigureAwait(true);
if (result.Succeeded)
{
IsInstalled = false;
IsJsonRpcInstalled = false;
IsInstalledStateKnown = true;
JsonRpcActionMessage = Resources.gallery_item_jsonrpc_action_uninstalled;
}
else
{
IsJsonRpcInstalled = _jsExtensionInstaller.IsInstalled(Id);
IsInstalledStateKnown = true;
JsonRpcActionMessage = result.ErrorMessage ?? Resources.gallery_item_jsonrpc_action_uninstall_failed;
}
}
@@ -1056,18 +1072,24 @@ public sealed partial class ExtensionGalleryItemViewModel : ObservableObject
}
}
IsInstalled = completedOperationKind != WinGetPackageOperationKind.Uninstall;
_isWinGetInstalled = completedOperationKind != WinGetPackageOperationKind.Uninstall;
_isWinGetInstalledStateKnown = true;
UpdateAggregateInstalledState();
IsInstalledStateKnown = true;
IsUpdateAvailable = false;
IsUpdateStateKnown = true;
NotifyWinGetInstallStateChanged();
}
private void ApplyOptimisticTrackedCompletion(WinGetPackageOperationKind completedOperationKind)
{
IsInstalled = completedOperationKind != WinGetPackageOperationKind.Uninstall;
_isWinGetInstalled = completedOperationKind != WinGetPackageOperationKind.Uninstall;
_isWinGetInstalledStateKnown = true;
UpdateAggregateInstalledState();
IsInstalledStateKnown = true;
IsUpdateAvailable = false;
IsUpdateStateKnown = true;
NotifyWinGetInstallStateChanged();
}
private ImageSource CreateImageSource(Uri iconUri)
@@ -1122,11 +1144,32 @@ public sealed partial class ExtensionGalleryItemViewModel : ObservableObject
CancelJsonRpcActionCommand.NotifyCanExecuteChanged();
}
private void NotifyWinGetInstallStateChanged()
{
OnPropertyChanged(nameof(ShowInstallViaWinGetButton));
OnPropertyChanged(nameof(CanInstallViaWinGet));
OnPropertyChanged(nameof(WinGetStatusText));
OnPropertyChanged(nameof(ShowWinGetStatusDetails));
OnPropertyChanged(nameof(ShowWinGetActionControls));
InstallViaWinGetCommand.NotifyCanExecuteChanged();
}
private void UpdateAggregateInstalledState()
{
IsInstalled = _isDetectedInstalled || _isWinGetInstalled || IsJsonRpcInstalled;
}
partial void OnIsJsonRpcActionInProgressChanged(bool value)
{
NotifyJsonRpcActionStateChanged();
}
partial void OnIsJsonRpcInstalledChanged(bool value)
{
UpdateAggregateInstalledState();
NotifyJsonRpcActionStateChanged();
}
partial void OnJsonRpcActionMessageChanged(string? value)
{
OnPropertyChanged(nameof(HasJsonRpcActionMessage));

View File

@@ -391,8 +391,7 @@ public sealed partial class ExtensionGalleryViewModel : ObservableObject, IDispo
{
if (!string.IsNullOrEmpty(entry.PackageFamilyName))
{
entry.IsInstalled = installedPfns.Contains(entry.PackageFamilyName);
entry.IsInstalledStateKnown = true;
entry.ApplyDetectedInstallationState(installedPfns.Contains(entry.PackageFamilyName));
}
}

View File

@@ -213,7 +213,9 @@ public sealed partial class JSExtensionWrapper : IExtensionWrapper, IDisposable
}
}
public Task StartExtensionAsync()
public Task StartExtensionAsync() => StartExtensionAsync(CancellationToken.None);
internal Task StartExtensionAsync(CancellationToken cancellationToken)
{
lock (_lock)
{
@@ -229,16 +231,16 @@ public sealed partial class JSExtensionWrapper : IExtensionWrapper, IDisposable
// instead of spawning a second Node process. The start body runs on the thread
// pool so no process is spawned while this lock is held; the task is cleared
// when it completes so a later restart can start again.
_startInProgress ??= Task.Run(RunStartAsync);
_startInProgress ??= Task.Run(() => RunStartAsync(cancellationToken), CancellationToken.None);
return _startInProgress;
}
}
private async Task RunStartAsync()
private async Task RunStartAsync(CancellationToken cancellationToken)
{
try
{
await StartCoreAsync().ConfigureAwait(false);
await StartCoreAsync(cancellationToken).ConfigureAwait(false);
}
finally
{
@@ -249,8 +251,10 @@ public sealed partial class JSExtensionWrapper : IExtensionWrapper, IDisposable
}
}
private async Task StartCoreAsync()
private async Task StartCoreAsync(CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();
lock (_lock)
{
// The wrapper may have been disposed, or another start may have completed,
@@ -361,7 +365,7 @@ public sealed partial class JSExtensionWrapper : IExtensionWrapper, IDisposable
var initResponse = await connection.SendRequestAsync(
"initialize",
new JsonObject { ["extensionId"] = _manifest.Name },
CancellationToken.None).ConfigureAwait(false);
cancellationToken).ConfigureAwait(false);
if (initResponse.Error is not null)
{
@@ -393,7 +397,11 @@ public sealed partial class JSExtensionWrapper : IExtensionWrapper, IDisposable
}
catch (Exception ex)
{
Logger.LogError($"Failed to start JS extension {_manifest.Name}: {ex.Message}");
var canceled = ex is OperationCanceledException && cancellationToken.IsCancellationRequested;
if (!canceled)
{
Logger.LogError($"Failed to start JS extension {_manifest.Name}: {ex.Message}");
}
try
{
@@ -408,6 +416,10 @@ public sealed partial class JSExtensionWrapper : IExtensionWrapper, IDisposable
}
SignalDispose();
if (canceled)
{
throw;
}
}
}
@@ -442,6 +454,22 @@ public sealed partial class JSExtensionWrapper : IExtensionWrapper, IDisposable
}
public void SignalDispose()
{
var (process, connection, proxy) = DetachForDispose();
TearDown(process, connection, proxy);
}
public Task SignalDisposeAsync()
{
var (process, connection, proxy) = DetachForDispose();
return process is null && connection is null && proxy is null
? Task.CompletedTask
: Task.Run(() => TearDown(process, connection, proxy));
}
public void Dispose() => SignalDispose();
private (Process? Process, JsonRpcConnection? Connection, JSCommandProviderProxy? Proxy) DetachForDispose()
{
Process? process;
JsonRpcConnection? connection;
@@ -459,11 +487,9 @@ public sealed partial class JSExtensionWrapper : IExtensionWrapper, IDisposable
_commandProviderProxy = null;
}
TearDown(process, connection, proxy);
return (process, connection, proxy);
}
public void Dispose() => SignalDispose();
public IExtension? GetExtensionObject()
{
// JS extensions have no WinRT COM object; the wrapper itself is the bridge.

View File

@@ -209,8 +209,8 @@ internal sealed partial class CrashRecoveryTracker : IDisposable
}
/// <summary>
/// Reopens a directory after its uninstall finishes. The caller must first drain recovery,
/// then keep the directory blocked until the extension and its watcher are gone.
/// Reopens a directory after its uninstall finishes or is abandoned. The caller must first
/// drain recovery, then keep the directory blocked until removal completes or is canceled.
/// </summary>
public void CompleteDirectoryRemoval(string directory)
{

View File

@@ -28,7 +28,8 @@ public interface IJsExtensionHost
/// </summary>
/// <param name="extensionDirectory">The extension's directory under the JSExtensions root.</param>
/// <param name="cancellationToken">A token to cancel a bounded wait while the provider is stopped.</param>
void StopExtension(string extensionDirectory, CancellationToken cancellationToken = default);
/// <returns>A task that completes after the extension and provider are stopped.</returns>
Task StopExtensionAsync(string extensionDirectory, CancellationToken cancellationToken = default);
/// <summary>
/// Determines whether <paramref name="extensionDirectory"/> contains a CmdPal manifest the host

View File

@@ -55,6 +55,8 @@ namespace Microsoft.CmdPal.UI.ViewModels.Services;
/// </remarks>
public sealed partial class JsonRpcExtensionService : IExtensionService, IJsExtensionHost, IDisposable
{
internal const string GalleryInstallMarkerFileName = ".cmdpal-gallery-installing";
// Consecutive crashes above this threshold disable an extension instead of restarting it.
private const int MaxRestartAttempts = 3;
@@ -169,7 +171,7 @@ public sealed partial class JsonRpcExtensionService : IExtensionService, IJsExte
public string ExtensionsRootPath => ExtensionsPath;
/// <inheritdoc />
public void StopExtension(string extensionDirectory, CancellationToken cancellationToken = default)
public async Task StopExtensionAsync(string extensionDirectory, CancellationToken cancellationToken = default)
{
if (string.IsNullOrEmpty(extensionDirectory))
{
@@ -178,13 +180,13 @@ public sealed partial class JsonRpcExtensionService : IExtensionService, IJsExte
// Use the lifecycle gate so uninstall waits behind any load, refresh, restart, or hot reload
// for this directory and cleans every owned resource. The gallery calls this before deleting
// files, so wait synchronously. Awaited work on this path uses ConfigureAwait(false), and we
// never enter while holding the same gate, so we avoid a reentrant deadlock. The token lets
// Cancel stop waiting for a busy gate.
var removed = RemoveExtensionByDirectoryGatedAsync(extensionDirectory, cancellationToken).GetAwaiter().GetResult();
// files. Awaited work on this path uses ConfigureAwait(false), and we never enter while holding
// the same gate, so we avoid a reentrant deadlock. The token lets Cancel stop waiting for a
// busy gate.
var removed = await RemoveExtensionByDirectoryGatedAsync(extensionDirectory, cancellationToken).ConfigureAwait(false);
if (removed is not null)
{
OnProviderRemoved?.Invoke(this, [removed]);
RaiseProviderRemoved(removed);
}
}
@@ -250,7 +252,7 @@ public sealed partial class JsonRpcExtensionService : IExtensionService, IJsExte
var added = await AddDiscoveredNotLoadedAsync(timeoutCts.Token).ConfigureAwait(false);
foreach (var wrapper in added)
{
OnProviderAdded?.Invoke(this, [wrapper]);
RaiseProviderAdded(wrapper);
}
}
catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested)
@@ -1022,7 +1024,7 @@ public sealed partial class JsonRpcExtensionService : IExtensionService, IJsExte
{
gate = await _directoryGate.AcquireAsync(directory, ct).ConfigureAwait(false);
}
catch (OperationCanceledException)
catch (OperationCanceledException) when (_disposed || _reload.IsStopRequested)
{
return null;
}
@@ -1094,7 +1096,7 @@ public sealed partial class JsonRpcExtensionService : IExtensionService, IJsExte
{
extensionWrapper = new JSExtensionWrapper(manifest, directory);
await extensionWrapper.StartExtensionAsync().ConfigureAwait(false);
await extensionWrapper.StartExtensionAsync(ct).ConfigureAwait(false);
if (!extensionWrapper.IsRunning())
{
@@ -1122,6 +1124,11 @@ public sealed partial class JsonRpcExtensionService : IExtensionService, IJsExte
var wrapper = CommandProviderWrapper.CreateForJsonRpcExtension(extensionWrapper, provider, _taskScheduler);
return new StartedInstance(extensionWrapper, wrapper);
}
catch (OperationCanceledException) when (ct.IsCancellationRequested)
{
extensionWrapper?.SignalDispose();
throw;
}
catch (Exception ex)
{
Logger.LogError($"Failed to load JS extension from {directory}: {ex.Message}");
@@ -1541,6 +1548,11 @@ public sealed partial class JsonRpcExtensionService : IExtensionService, IJsExte
return;
}
if (File.Exists(Path.Combine(extensionDirectory, GalleryInstallMarkerFileName)))
{
return;
}
var token = _reload.Token;
StartObservedBackgroundTask(
() => HandleDirectoryEntryUpsertAsync(extensionDirectory, token),
@@ -1693,10 +1705,17 @@ public sealed partial class JsonRpcExtensionService : IExtensionService, IJsExte
{
// The gate is being torn down; fall through and remove best-effort.
}
catch (OperationCanceledException)
{
// The uninstall never acquired the lifecycle gate, so this directory is staying put.
// Reopen crash recovery before handing cancellation back to the caller.
_recovery.CompleteDirectoryRemoval(directory);
throw;
}
try
{
return RemoveExtensionByDirectoryCore(directory);
return await RemoveExtensionByDirectoryCoreAsync(directory).ConfigureAwait(false);
}
finally
{
@@ -1708,7 +1727,7 @@ public sealed partial class JsonRpcExtensionService : IExtensionService, IJsExte
}
}
private CommandProviderWrapper? RemoveExtensionByDirectoryCore(string directory)
private async Task<CommandProviderWrapper?> RemoveExtensionByDirectoryCoreAsync(string directory)
{
JSExtensionWrapper? extensionToRemove;
CommandProviderWrapper? wrapperToRemove;
@@ -1743,7 +1762,7 @@ public sealed partial class JsonRpcExtensionService : IExtensionService, IJsExte
if (extensionToRemove is not null)
{
extensionToRemove.ProcessExited -= OnExtensionProcessExited;
extensionToRemove.SignalDispose();
await extensionToRemove.SignalDisposeAsync().ConfigureAwait(false);
}
return wrapperToRemove;

View File

@@ -108,13 +108,20 @@ public sealed class NpmCommandRunner : INpmCommandRunner
return NpmCommandResult.Fail(Resources.npm_runner_extract_failed);
}
// 2) The caller compares this tarball hash to the catalog value before promotion.
// 2) Verify the downloaded tarball before extracting or executing npm against anything it
// contains. A mismatched artifact must not get a chance to choose dependency URLs.
var resolvedIntegrity = ComputeTarballIntegrity(tarballPath);
if (resolvedIntegrity is null)
{
return NpmCommandResult.Fail(Resources.npm_runner_extract_failed);
}
if (!string.Equals(resolvedIntegrity, artifact.Integrity, StringComparison.Ordinal))
{
Logger.LogError($"Integrity mismatch installing '{artifact.InstallSpec}': expected {artifact.Integrity}, npm resolved {resolvedIntegrity}.");
return NpmCommandResult.Fail(Resources.npm_installer_integrity_mismatch);
}
// 3) Make the published package the root project before npm ci runs. npm only honors an
// embedded npm-shrinkwrap.json at the project root, not inside a dependency.
var packageRoot = Path.Combine(stagingDirectory, PackageRootDirectoryName);
@@ -133,7 +140,15 @@ public sealed class NpmCommandRunner : INpmCommandRunner
return NpmCommandResult.Fail(shrinkwrapError);
}
// 5) npm ci installs the exact closure named in the shrinkwrap. It fails when the lockfile
// 5) Validate every dependency URL and integrity hash before npm can fetch the closure.
var lockfileError = VerifyLockfileIntegrity(packageRoot);
if (lockfileError is not null)
{
Logger.LogError($"npm package {artifact.InstallSpec} contains an untrusted lockfile: {lockfileError}");
return NpmCommandResult.Fail(lockfileError);
}
// 6) npm ci installs the exact closure named in the shrinkwrap. It fails when the lockfile
// is missing or out of sync with package.json.
var ciResult = await RunNpmAsync(invocation.Value, BuildCiArguments(artifact), packageRoot, artifact.InstallSpec, cancellationToken).ConfigureAwait(false);
if (!ciResult.Succeeded)
@@ -141,9 +156,8 @@ public sealed class NpmCommandRunner : INpmCommandRunner
return NpmCommandResult.Fail(ciResult.ErrorMessage ?? Resources.npm_runner_lockfile_untrusted);
}
// 6) Check every lockfile entry before promotion. Anything outside an approved HTTPS
// registry, or missing a Subresource Integrity hash, is rejected.
var lockfileError = VerifyLockfileIntegrity(packageRoot);
// 7) Check the lockfile again after npm exits so promotion only uses the closure we approved.
lockfileError = VerifyLockfileIntegrity(packageRoot);
if (lockfileError is not null)
{
Logger.LogError($"npm ci {artifact.InstallSpec} produced an untrusted lockfile: {lockfileError}");
@@ -648,9 +662,12 @@ public sealed class NpmCommandRunner : INpmCommandRunner
/// the environment. Returns null when either piece cannot be found.
/// </summary>
internal static NpmInvocation? ResolveNpmInvocation() =>
ResolveNpmInvocation(GetPathDirectories());
ResolveNpmInvocation(GetPathDirectories(), includeUserPrefix: true);
internal static NpmInvocation? ResolveNpmInvocation(IReadOnlyList<string> pathDirectories)
internal static NpmInvocation? ResolveNpmInvocation(IReadOnlyList<string> pathDirectories) =>
ResolveNpmInvocation(pathDirectories, includeUserPrefix: false);
private static NpmInvocation? ResolveNpmInvocation(IReadOnlyList<string> pathDirectories, bool includeUserPrefix)
{
ArgumentNullException.ThrowIfNull(pathDirectories);
@@ -672,7 +689,7 @@ public sealed class NpmCommandRunner : INpmCommandRunner
continue;
}
var npmCli = FindNpmCli(directory);
var npmCli = FindNpmCli(directory, includeUserPrefix);
if (npmCli is not null)
{
return new NpmInvocation(nodeCandidate, new[] { npmCli });
@@ -682,10 +699,10 @@ public sealed class NpmCommandRunner : INpmCommandRunner
return null;
}
private static string? FindNpmCli(string nodeDirectory)
private static string? FindNpmCli(string nodeDirectory, bool includeUserPrefix)
{
// Standard Windows Node.js layout: npm-cli.js sits under the same directory as node.exe.
foreach (var candidateRoot in EnumerateNpmPrefixCandidates(nodeDirectory))
foreach (var candidateRoot in EnumerateNpmPrefixCandidates(nodeDirectory, includeUserPrefix))
{
string npmCli;
try
@@ -706,11 +723,16 @@ public sealed class NpmCommandRunner : INpmCommandRunner
return null;
}
private static IEnumerable<string> EnumerateNpmPrefixCandidates(string nodeDirectory)
private static IEnumerable<string> EnumerateNpmPrefixCandidates(string nodeDirectory, bool includeUserPrefix)
{
// node.exe's own directory (Program Files\nodejs) is the usual prefix on Windows.
yield return nodeDirectory;
if (!includeUserPrefix)
{
yield break;
}
// A user-level npm prefix (npm config's default on Windows) lives under APPDATA\npm.
var appData = Environment.GetEnvironmentVariable("APPDATA");
if (!string.IsNullOrEmpty(appData))

View File

@@ -25,6 +25,32 @@ public sealed class NpmJsExtensionInstaller : IJsExtensionInstaller
// Upper bound on how long to wait for the host to load and register a freshly promoted extension.
private static readonly TimeSpan RegistrationTimeout = TimeSpan.FromSeconds(30);
private static readonly HashSet<string> ReservedWindowsNames = new(StringComparer.OrdinalIgnoreCase)
{
"CON",
"PRN",
"AUX",
"NUL",
"COM1",
"COM2",
"COM3",
"COM4",
"COM5",
"COM6",
"COM7",
"COM8",
"COM9",
"LPT1",
"LPT2",
"LPT3",
"LPT4",
"LPT5",
"LPT6",
"LPT7",
"LPT8",
"LPT9",
};
private readonly IJsExtensionHost _host;
private readonly INpmCommandRunner _npmCommandRunner;
@@ -100,14 +126,14 @@ public sealed class NpmJsExtensionInstaller : IJsExtensionInstaller
await gate.WaitAsync(cancellationToken).ConfigureAwait(false);
try
{
// Stop the Node.js process before delete so file handles are released. StopExtension
// blocks until the process exits, and the token lets Cancel stop waiting on a busy gate.
_host.StopExtension(targetDirectory, cancellationToken);
// Stop the Node.js process before delete so file handles are released. The token lets
// Cancel stop waiting on a busy lifecycle gate without blocking the UI thread.
await _host.StopExtensionAsync(targetDirectory, cancellationToken).ConfigureAwait(false);
// RemoveDirectory refuses to follow a junction or symbolic link and retries briefly when
// handles are still closing. The token stops the retry loop between attempts.
if (!_npmCommandRunner.RemoveDirectory(targetDirectory, cancellationToken))
// Delete on a worker thread since process handles can take a moment to close.
if (!await RemoveDirectoryAsync(targetDirectory).ConfigureAwait(false))
{
await _host.RefreshAndAwaitProviderAsync(targetDirectory, RegistrationTimeout, CancellationToken.None).ConfigureAwait(false);
Logger.LogError($"Uninstall of JS extension '{extensionName}' failed: could not delete {targetDirectory}.");
return JsExtensionInstallResult.Fail(Resources.npm_installer_remove_failed);
}
@@ -191,6 +217,8 @@ public sealed class NpmJsExtensionInstaller : IJsExtensionInstaller
return JsExtensionInstallResult.Fail(Resources.npm_installer_version_mismatch);
}
File.WriteAllText(Path.Combine(packageDirectory, JsonRpcExtensionService.GalleryInstallMarkerFileName), string.Empty);
// The extracted package root already has the layout discovery expects: package.json at the
// root with the frozen dependency closure under its own node_modules. Promote it directly.
//
@@ -205,7 +233,7 @@ public sealed class NpmJsExtensionInstaller : IJsExtensionInstaller
if (!registered)
{
Logger.LogError($"Promoted '{extensionName}' but the host did not register a provider within {RegistrationTimeout.TotalSeconds:0} seconds.");
if (RollbackPromotedInstall(targetDirectory))
if (await RollbackPromotedInstallAsync(targetDirectory).ConfigureAwait(false))
{
promoted = false;
}
@@ -213,6 +241,7 @@ public sealed class NpmJsExtensionInstaller : IJsExtensionInstaller
return JsExtensionInstallResult.Fail(Resources.npm_installer_not_discoverable);
}
TryRemoveInstallMarker(targetDirectory);
Logger.LogInfo($"Installed JS extension '{extensionName}' from npm package '{artifact.InstallSpec}'.");
return JsExtensionInstallResult.Ok();
}
@@ -222,7 +251,7 @@ public sealed class NpmJsExtensionInstaller : IJsExtensionInstaller
// process and provider first, then remove the promoted tree.
if (promoted)
{
RollbackPromotedInstall(targetDirectory);
await RollbackPromotedInstallAsync(targetDirectory).ConfigureAwait(false);
}
return JsExtensionInstallResult.Fail(Resources.npm_installer_canceled);
@@ -231,7 +260,7 @@ public sealed class NpmJsExtensionInstaller : IJsExtensionInstaller
{
if (promoted)
{
RollbackPromotedInstall(targetDirectory);
await RollbackPromotedInstallAsync(targetDirectory).ConfigureAwait(false);
}
Logger.LogError($"Install of '{extensionName}' failed: {ex.Message}");
@@ -241,7 +270,7 @@ public sealed class NpmJsExtensionInstaller : IJsExtensionInstaller
{
// Clean the staging tree on every path, even after cancel. Do not observe the caller token
// here, because cleanup still needs to run.
if (!_npmCommandRunner.RemoveDirectory(stagingDirectory, CancellationToken.None))
if (!await RemoveDirectoryAsync(stagingDirectory).ConfigureAwait(false))
{
Logger.LogWarning($"Failed to clean up staging directory {stagingDirectory}.");
}
@@ -254,10 +283,32 @@ public sealed class NpmJsExtensionInstaller : IJsExtensionInstaller
/// extension both installed and running. Uses no cancellation token so cleanup can finish.
/// </summary>
/// <returns><see langword="true"/> when the promoted directory was removed; otherwise, <see langword="false"/>.</returns>
private bool RollbackPromotedInstall(string targetDirectory)
private async Task<bool> RollbackPromotedInstallAsync(string targetDirectory)
{
_host.StopExtension(targetDirectory);
return _npmCommandRunner.RemoveDirectory(targetDirectory);
await _host.StopExtensionAsync(targetDirectory, CancellationToken.None).ConfigureAwait(false);
var removed = await RemoveDirectoryAsync(targetDirectory).ConfigureAwait(false);
if (!removed)
{
TryRemoveInstallMarker(targetDirectory);
}
return removed;
}
private Task<bool> RemoveDirectoryAsync(string targetDirectory) =>
Task.Run(() => _npmCommandRunner.RemoveDirectory(targetDirectory, CancellationToken.None));
private static void TryRemoveInstallMarker(string targetDirectory)
{
var markerPath = Path.Combine(targetDirectory, JsonRpcExtensionService.GalleryInstallMarkerFileName);
try
{
File.Delete(markerPath);
}
catch (Exception ex) when (ex is IOException or UnauthorizedAccessException)
{
Logger.LogWarning($"Failed to remove gallery install marker '{markerPath}': {ex.Message}");
}
}
/// <summary>
@@ -346,8 +397,13 @@ public sealed class NpmJsExtensionInstaller : IJsExtensionInstaller
// Guard against path traversal or absolute paths escaping the JSExtensions root.
var trimmed = extensionName.Trim();
if (trimmed.IndexOfAny(Path.GetInvalidFileNameChars()) >= 0
var separatorIndex = trimmed.IndexOf('.');
var deviceName = separatorIndex >= 0 ? trimmed[..separatorIndex] : trimmed;
if (!string.Equals(trimmed, extensionName, StringComparison.Ordinal)
|| trimmed.EndsWith('.')
|| trimmed.IndexOfAny(Path.GetInvalidFileNameChars()) >= 0
|| trimmed is "." or ".."
|| ReservedWindowsNames.Contains(deviceName)
|| Path.IsPathRooted(trimmed))
{
return false;
@@ -365,21 +421,38 @@ public sealed class NpmJsExtensionInstaller : IJsExtensionInstaller
return false;
}
var candidate = Path.GetFullPath(Path.Combine(normalizedRoot, trimmed));
string candidate;
try
{
candidate = Path.GetFullPath(Path.Combine(normalizedRoot, trimmed));
}
catch (Exception ex) when (ex is ArgumentException or NotSupportedException or PathTooLongException)
{
return false;
}
if (!candidate.StartsWith(normalizedRoot + Path.DirectorySeparatorChar, StringComparison.OrdinalIgnoreCase))
{
return false;
}
if (!string.Equals(Path.GetFileName(candidate), trimmed, StringComparison.OrdinalIgnoreCase)
|| (Directory.Exists(candidate) && !DirectoryResolvesToItself(candidate)))
{
return false;
}
targetDirectory = candidate;
return true;
}
private static bool RootResolvesToItself(string normalizedRoot)
private static bool RootResolvesToItself(string normalizedRoot) => DirectoryResolvesToItself(normalizedRoot);
private static bool DirectoryResolvesToItself(string directory)
{
// A root that does not exist yet cannot redirect anywhere. Install creates it as a real
// directory before promoting into it.
if (!Directory.Exists(normalizedRoot))
if (!Directory.Exists(directory))
{
return true;
}
@@ -388,12 +461,12 @@ public sealed class NpmJsExtensionInstaller : IJsExtensionInstaller
{
// ResolveLinkTarget returns null when the path is not a reparse point. Any reparse point on
// the root is treated as unsafe.
return Directory.ResolveLinkTarget(normalizedRoot, returnFinalTarget: true) is null;
return Directory.ResolveLinkTarget(directory, returnFinalTarget: true) is null;
}
catch (Exception ex) when (ex is IOException or UnauthorizedAccessException or System.Security.SecurityException)
{
// If the root cannot be inspected, play it safe and refuse.
Logger.LogError($"Failed to inspect extensions root '{normalizedRoot}' for reparse points: {ex.Message}");
Logger.LogError($"Failed to inspect extension directory '{directory}' for reparse points: {ex.Message}");
return false;
}
}

View File

@@ -789,12 +789,63 @@ public class ExtensionGalleryItemViewModelTests
Assert.AreEqual("npm was not found", viewModel.JsonRpcActionMessage);
}
[TestMethod]
public async Task InstallViaNpmCommand_RefreshesInstalledState_OnFailure()
{
var installer = new Mock<IJsExtensionInstaller>();
installer
.SetupSequence(x => x.IsInstalled("sample-js-extension"))
.Returns(false)
.Returns(true);
installer
.Setup(x => x.InstallAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string?>(), It.IsAny<string?>(), It.IsAny<string?>(), It.IsAny<CancellationToken>()))
.ReturnsAsync(JsExtensionInstallResult.Fail("rollback failed"));
var viewModel = CreateViewModel(CreateJsonRpcEntry(), jsExtensionInstaller: installer.Object);
await viewModel.InstallViaNpmCommand.ExecuteAsync(null);
Assert.IsTrue(viewModel.IsJsonRpcInstalled);
Assert.IsTrue(viewModel.ShowUninstallJsonRpcButton);
}
[TestMethod]
public async Task CancelJsonRpcActionCommand_NotifiesActionStateImmediately()
{
var operationStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
var installer = new Mock<IJsExtensionInstaller>();
installer.Setup(x => x.IsInstalled("sample-js-extension")).Returns(false);
installer
.Setup(x => x.InstallAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string?>(), It.IsAny<string?>(), It.IsAny<string?>(), It.IsAny<CancellationToken>()))
.Returns(async (string _, string _, string? _, string? _, string? _, CancellationToken token) =>
{
operationStarted.SetResult();
try
{
await Task.Delay(Timeout.InfiniteTimeSpan, token);
return JsExtensionInstallResult.Ok();
}
catch (OperationCanceledException)
{
return JsExtensionInstallResult.Fail("canceled");
}
});
var viewModel = CreateViewModel(CreateJsonRpcEntry(), jsExtensionInstaller: installer.Object);
var installTask = viewModel.InstallViaNpmCommand.ExecuteAsync(null);
await operationStarted.Task;
Assert.IsTrue(viewModel.ShowCancelJsonRpcActionButton);
viewModel.CancelJsonRpcActionCommand.Execute(null);
Assert.IsFalse(viewModel.ShowCancelJsonRpcActionButton);
Assert.IsFalse(viewModel.CancelJsonRpcActionCommand.CanExecute(null));
await installTask;
}
[TestMethod]
public async Task UninstallJsonRpcCommand_Uninstalls_AndMarksNotInstalled()
{
var viewModel = CreateJsonRpcViewModel(out var installer);
viewModel.IsInstalled = true;
viewModel.IsInstalledStateKnown = true;
var viewModel = CreateJsonRpcViewModel(out var installer, isInstalled: true);
installer
.Setup(x => x.UninstallAsync("sample-js-extension", It.IsAny<CancellationToken>()))
.ReturnsAsync(JsExtensionInstallResult.Ok());
@@ -808,6 +859,134 @@ public class ExtensionGalleryItemViewModelTests
Assert.IsTrue(viewModel.ShowInstallViaNpmButton);
}
[TestMethod]
public void WinGetInstall_DoesNotHideJsonRpcInstall()
{
var installer = new Mock<IJsExtensionInstaller>();
var entry = CreateJsonRpcEntry();
entry.InstallSources.Add(new GalleryInstallSource { Type = "winget", Id = "Contoso.Sample" });
var viewModel = CreateViewModel(entry, jsExtensionInstaller: installer.Object);
viewModel.ApplyWinGetPackageInfo(
new WinGetPackageInfo(
new WinGetPackageStatus(
IsInstalled: true,
IsInstalledStateKnown: true,
IsUpdateAvailable: false,
IsUpdateStateKnown: true),
Details: null));
Assert.IsTrue(viewModel.IsInstalled);
Assert.IsTrue(viewModel.ShowInstallViaNpmButton);
Assert.IsFalse(viewModel.ShowUninstallJsonRpcButton);
}
[TestMethod]
public void JsonRpcInstall_DoesNotHideWinGetInstall()
{
var installer = new Mock<IJsExtensionInstaller>();
installer.Setup(x => x.IsInstalled("sample-js-extension")).Returns(true);
var entry = CreateJsonRpcEntry();
entry.InstallSources.Add(new GalleryInstallSource { Type = "winget", Id = "Contoso.Sample" });
var viewModel = CreateViewModel(entry, jsExtensionInstaller: installer.Object);
List<string> propertyNames = [];
viewModel.PropertyChanged += (_, args) =>
{
if (args.PropertyName is not null)
{
propertyNames.Add(args.PropertyName);
}
};
viewModel.ApplyWinGetPackageInfo(
new WinGetPackageInfo(
new WinGetPackageStatus(
IsInstalled: false,
IsInstalledStateKnown: true,
IsUpdateAvailable: false,
IsUpdateStateKnown: true),
Details: null));
Assert.IsTrue(viewModel.IsInstalled);
Assert.IsTrue(viewModel.ShowInstallViaWinGetButton);
Assert.IsTrue(viewModel.ShowWinGetStatusDetails);
CollectionAssert.Contains(propertyNames, nameof(viewModel.ShowInstallViaWinGetButton));
CollectionAssert.Contains(propertyNames, nameof(viewModel.WinGetStatusText));
}
[TestMethod]
public void TrackedWinGetUninstall_PreservesJsonRpcInstalledState()
{
var installer = new Mock<IJsExtensionInstaller>();
installer.Setup(x => x.IsInstalled("sample-js-extension")).Returns(true);
var entry = CreateJsonRpcEntry();
entry.InstallSources.Add(new GalleryInstallSource { Type = "winget", Id = "Contoso.Sample" });
var viewModel = CreateViewModel(entry, jsExtensionInstaller: installer.Object);
viewModel.ApplyTrackedOperation(CreateCompletedWinGetOperation(WinGetPackageOperationKind.Uninstall));
Assert.IsTrue(viewModel.IsInstalled);
Assert.IsTrue(viewModel.IsJsonRpcInstalled);
}
[TestMethod]
public async Task RefreshWinGetInstall_PreservesWinGetStateAfterJsonRpcRemoval()
{
var installer = new Mock<IJsExtensionInstaller>();
installer.Setup(x => x.IsInstalled("sample-js-extension")).Returns(true);
var entry = CreateJsonRpcEntry();
entry.InstallSources.Add(new GalleryInstallSource { Type = "winget", Id = "Contoso.Sample" });
var viewModel = CreateViewModel(entry, jsExtensionInstaller: installer.Object);
await viewModel.RefreshWinGetPackageInfoAsync(WinGetPackageOperationKind.Install);
viewModel.IsJsonRpcInstalled = false;
Assert.IsTrue(viewModel.IsInstalled);
}
[TestMethod]
public void DetectedInstall_PersistsAcrossOtherSourceUpdates()
{
var installer = new Mock<IJsExtensionInstaller>();
installer.Setup(x => x.IsInstalled("sample-js-extension")).Returns(true);
var entry = CreateJsonRpcEntry();
entry.InstallSources.Add(new GalleryInstallSource { Type = "winget", Id = "Contoso.Sample" });
var viewModel = CreateViewModel(entry, jsExtensionInstaller: installer.Object);
viewModel.ApplyDetectedInstallationState(true);
viewModel.ApplyWinGetPackageInfo(
new WinGetPackageInfo(
new WinGetPackageStatus(
IsInstalled: false,
IsInstalledStateKnown: true,
IsUpdateAvailable: false,
IsUpdateStateKnown: true),
Details: null));
viewModel.IsJsonRpcInstalled = false;
Assert.IsTrue(viewModel.IsInstalled);
}
private static WinGetPackageOperation CreateCompletedWinGetOperation(WinGetPackageOperationKind kind)
{
var now = DateTimeOffset.UtcNow;
return new WinGetPackageOperation(
OperationId: Guid.NewGuid(),
PackageId: "Contoso.Sample",
PackageName: "Contoso Sample",
Kind: kind,
State: WinGetPackageOperationState.Succeeded,
CanCancel: false,
IsIndeterminate: false,
ProgressPercent: 100,
BytesDownloaded: 0,
BytesRequired: 0,
ErrorMessage: null,
StartedAt: now,
UpdatedAt: now,
CompletedAt: now);
}
private static GalleryExtensionEntry CreateJsonRpcEntry()
{
return new GalleryExtensionEntry
@@ -833,9 +1012,10 @@ public class ExtensionGalleryItemViewModelTests
};
}
private static ExtensionGalleryItemViewModel CreateJsonRpcViewModel(out Mock<IJsExtensionInstaller> installer)
private static ExtensionGalleryItemViewModel CreateJsonRpcViewModel(out Mock<IJsExtensionInstaller> installer, bool isInstalled = false)
{
installer = new Mock<IJsExtensionInstaller>();
installer.Setup(x => x.IsInstalled("sample-js-extension")).Returns(isInstalled);
return CreateViewModel(CreateJsonRpcEntry(), jsExtensionInstaller: installer.Object);
}

View File

@@ -4,6 +4,8 @@
using System;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CmdPal.JsonRpc;
using Microsoft.CmdPal.UI.ViewModels.Models;
using Microsoft.CommandPalette.Extensions;
@@ -185,4 +187,17 @@ public class JSExtensionWrapperTests
},
extensionDirectory);
}
[TestMethod]
public async Task StartExtensionAsync_StopsBeforeLaunch_WhenCanceled()
{
var wrapper = CreateWrapper();
using var cancellation = new CancellationTokenSource();
cancellation.Cancel();
await Assert.ThrowsExactlyAsync<OperationCanceledException>(
() => wrapper.StartExtensionAsync(cancellation.Token));
Assert.IsFalse(wrapper.IsRunning());
}
}

View File

@@ -50,6 +50,7 @@ public class NpmJsExtensionInstallerTests
Assert.IsTrue(Directory.Exists(target));
Assert.IsTrue(File.Exists(Path.Combine(target, "package.json")));
Assert.IsTrue(File.Exists(Path.Combine(target, "index.js")));
Assert.IsFalse(File.Exists(Path.Combine(target, JsonRpcExtensionService.GalleryInstallMarkerFileName)));
Assert.IsTrue(host.IsExtensionInstalled("left-pad-ext"));
AssertStagingEmpty(host);
}
@@ -176,6 +177,44 @@ public class NpmJsExtensionInstallerTests
Assert.AreEqual(0, runner.InstallCallCount);
}
[DataTestMethod]
[DataRow("sample-ext.")]
[DataRow(" sample-ext")]
[DataRow("sample-ext ")]
[DataRow("CON")]
[DataRow("con.txt")]
[DataRow("CON.foo.txt")]
[DataRow("COM1.foo.bar")]
public async Task InstallAsync_Fails_ForWindowsAliasName(string extensionName)
{
var host = CreateHost();
var runner = new FakeRunner();
var installer = new NpmJsExtensionInstaller(host, runner);
var result = await installer.InstallAsync(extensionName, Package, Version, ValidIntegrity, null, CancellationToken.None);
Assert.IsFalse(result.Succeeded);
Assert.AreEqual(0, runner.InstallCallCount);
}
[DataTestMethod]
[DataRow("sample-ext.")]
[DataRow("NUL")]
[DataRow("CON.foo.txt")]
[DataRow("COM1.foo.bar")]
public async Task UninstallAsync_Fails_ForWindowsAliasName(string extensionName)
{
var host = CreateHost();
var runner = new FakeRunner();
var installer = new NpmJsExtensionInstaller(host, runner);
var result = await installer.UninstallAsync(extensionName, CancellationToken.None);
Assert.IsFalse(result.Succeeded);
Assert.AreEqual(0, host.StopCallCount);
Assert.AreEqual(0, runner.RemoveCallCount);
}
[TestMethod]
public async Task InstallAsync_Fails_AndDoesNotPromote_OnIntegrityMismatch()
{
@@ -246,6 +285,22 @@ public class NpmJsExtensionInstallerTests
AssertStagingEmpty(host);
}
[TestMethod]
public async Task InstallAsync_RemovesMarker_WhenRollbackDeleteFails()
{
var host = CreateHost();
host.RegistrationSucceeds = false;
var runner = new FakeRunner { RemoveSucceeds = false };
var installer = new NpmJsExtensionInstaller(host, runner);
var result = await installer.InstallAsync(ExtensionName, Package, Version, ValidIntegrity, null, CancellationToken.None);
var target = Path.Combine(host.ExtensionsRootPath, ExtensionName);
Assert.IsFalse(result.Succeeded);
Assert.IsTrue(Directory.Exists(target));
Assert.IsFalse(File.Exists(Path.Combine(target, JsonRpcExtensionService.GalleryInstallMarkerFileName)));
}
[TestMethod]
public async Task InstallAsync_Fails_AndCleansStaging_OnNpmFailure()
{
@@ -349,6 +404,10 @@ public class NpmJsExtensionInstallerTests
public async Task UninstallAsync_Fails_WhenRemoveFails()
{
var host = CreateHost();
host.MarkInstalled(ExtensionName);
var target = Path.Combine(host.ExtensionsRootPath, ExtensionName);
Directory.CreateDirectory(target);
File.WriteAllText(Path.Combine(target, "package.json"), "{}");
var runner = new FakeRunner { RemoveSucceeds = false };
var installer = new NpmJsExtensionInstaller(host, runner);
@@ -357,6 +416,7 @@ public class NpmJsExtensionInstallerTests
Assert.IsFalse(result.Succeeded);
Assert.IsNotNull(result.ErrorMessage);
Assert.AreEqual(1, host.StopCallCount);
Assert.IsTrue(host.IsExtensionInstalled(ExtensionName), "A failed delete must reload the surviving extension.");
}
[TestMethod]
@@ -368,21 +428,20 @@ public class NpmJsExtensionInstallerTests
File.WriteAllText(Path.Combine(target, "package.json"), "{}");
using var stopStarted = new ManualResetEventSlim(false);
host.StopHook = token =>
host.StopHook = async token =>
{
// Simulate the host blocking while it stops the provider, then observe cancel after the
// operation has already begun. This matches the real host threading the uninstall token
// through stop and delete.
stopStarted.Set();
Assert.IsTrue(token.WaitHandle.WaitOne(TimeSpan.FromSeconds(5)), "Cancellation was not observed during stop.");
token.ThrowIfCancellationRequested();
await Task.Delay(Timeout.InfiniteTimeSpan, token).ConfigureAwait(false);
};
var runner = new FakeRunner();
var installer = new NpmJsExtensionInstaller(host, runner);
using var cts = new CancellationTokenSource();
var task = Task.Run(() => installer.UninstallAsync(ExtensionName, cts.Token));
var task = installer.UninstallAsync(ExtensionName, cts.Token);
Assert.IsTrue(stopStarted.Wait(TimeSpan.FromSeconds(5)), "Uninstall did not reach the stop step.");
cts.Cancel();
@@ -681,7 +740,7 @@ public class NpmJsExtensionInstallerTests
public ConcurrentQueue<string>? OrderLog { get; set; }
public Action<CancellationToken>? StopHook { get; set; }
public Func<CancellationToken, Task>? StopHook { get; set; }
public ManualResetEventSlim? RegistrationStarted { get; set; }
@@ -693,11 +752,20 @@ public class NpmJsExtensionInstallerTests
}
}
public void StopExtension(string extensionDirectory, CancellationToken cancellationToken = default)
public async Task StopExtensionAsync(string extensionDirectory, CancellationToken cancellationToken = default)
{
OrderLog?.Enqueue("stop");
StopCallCount++;
StopHook?.Invoke(cancellationToken);
if (StopHook is not null)
{
await StopHook(cancellationToken).ConfigureAwait(false);
}
var name = Path.GetFileName(Path.TrimEndingDirectorySeparator(extensionDirectory));
lock (_installedGate)
{
_installed.Remove(name);
}
}
public bool IsExtensionDiscoverable(string extensionDirectory) =>