Fix source initialization in noninteractive logons

[Przemysław Kłys (PrzemyslawKlys)]

Description

Addresses #6334: packaged source-extension deployment can fail in a noninteractive logon, preventing source initialization.

The maintainer's #6584 supersedes the replacement proposed here. It includes the interactive-user check and reads the newest package across deployed and local stores, addressing the shared-update-timestamp problem described in the review discussion.

This PR remains parked as a draft while #6584 completes review. Its published head still contains the earlier fallback implementation, with unresolved findings concerning the deployment lock lifetime and deletion of a valid cache after a transient open failure. That implementation should not be merged; this PR can be retired when the maintainer's replacement lands.

AI assistance: OpenAI Codex assisted with the implementation and revision assessment.

Validation

Earlier Windows Server 2025 validation covers this PR's fallback implementation. The smaller local factory proposal passed four native token-selection cases, but it is not published and does not establish packaged SSH/WinRM source lifecycle behavior. Neither result qualifies #6584; its implementation and validation belong to that PR.

Checklist

• Signed the Contributor License Agreement
• Linked to an issue
• Reassessed the design with the maintainer
• Retire this PR after Refactor packaged index factory #6584 lands

Issue Type

• Bug fix
• Feature
• Task

Microsoft Reviewers: Open in CodeFlow

[ranm-msft]

Two things I would like to see nailed down before this goes in, both about the lifecycle rather than the idea.

1. The local-state fallback outlives the failure that created it. The title frames this as a noninteractive/logged-off recovery, but the precedence is evaluated on every Open: ShouldPreferDesktopContext selects the local-state package whenever its version is strictly greater than the deployed extension's, regardless of how it got there. That is probably intentional, but it is a standing change to which catalog a packaged WinGet opens, and it is not stated anywhere in the PR or in the code comments. Could you document the intended precedence explicitly, including what is supposed to happen once the deployed extension catches up, and when the fallback is expected to be retired rather than refreshed?

2. No tests. ~300 lines here decide which catalog is opened and under what trust conditions, and none of the transitions are covered. Worth pinning at least: deployment blocked by logoff writes the fallback; a newer trusted fallback is selected over the extension; an untrusted or unopenable fallback is rejected and removed; the extension wins again once its version catches up; and the TryAcquireNoWait miss path degrades to the extension rather than failing. The refactor that hoists UpdateDesktopContextPackage and OpenDesktopContextIndex into shared helpers also now has two callers with different expectations, which is worth locking down while it is fresh.

[Przemysław Kłys (PrzemyslawKlys)]

ranm-msft Thanks for the review. I've updated the PR to spell out the cache lifecycle and added tests for the precedence decisions, the lock-miss path, and rejection of a trusted package from the wrong source family.

The intended behavior is that the extension takes over again when it catches up, while the validated local copy stays as a dormant recovery cache. It is refreshed on later successful updates and removed when the source is removed. I also tightened the checks before using it and clear an invalid or unreadable copy.

The focused tests pass here, but I have not yet rerun the full packaged-session transition sequence on this exact revision. I've asked the original reporter to try the PR build in their SSH/WinRM scenario. Does this lifecycle match what you had in mind, particularly keeping the dormant copy for a later deployment failure?

[Sylvester Kaczmarek (sylvesterkaczmarek)]

The SOURCE_DATA_MISSING fallback path calls HasValidDesktopContextPackage()/OpenDesktopContextIndex() without taking the source CrossProcessLock. If another process is updating/removing that cache, this branch can race it even though the earlier fallback path deliberately skips cache reads when the lock is held. Can this branch take the same lock before validating/opening the fallback?

[Przemysław Kłys (PrzemyslawKlys)]

Sylvester Kaczmarek (@sylvesterkaczmarek) Thanks for checking this. On the current head, the SOURCE_DATA_MISSING branch is inside the retry-under-lock block: Open() acquires CrossProcessLock(CreateNameForCPL(m_details)) before retrying the packaged open, and both HasValidDesktopContextPackage() and OpenDesktopContextIndex() run before that lock leaves scope. Source updates and removal use the same lock through LockExclusive(). So I believe this particular cache read is already protected. If you meant another path or interleaving, please point me to it.

I also reran the logged-off Windows Server 2025 case on this head. Deployment returned 0x80073D19, the update wrote the fallback, search succeeded while the extension remained absent, and removing the source deleted the cache. The PR description now has the test details and its local-source limitation.

[Sylvester Kaczmarek (sylvesterkaczmarek)]

You're right. I rechecked the full Open() scope rather than the isolated catch branch: on the retry path the second CrossProcessLock is acquired before the packaged reopen, and it remains in scope through HasValidDesktopContextPackage() and OpenDesktopContextIndex(). Updates/removal use the same source lock as well. My race concern on this path is withdrawn.

[JohnMcPMS]

Could the desired functional change be implemented by properly detecting the situation here? Seems like it would result in a lot less churn.

[Przemysław Kłys (PrzemyslawKlys)]

I tried this at the factory boundary. The reduced candidate removes the fallback implementation and changes the condition to:

if (Runtime::IsRunningInPackagedContext() &&
    wil::test_token_membership(nullptr, SECURITY_NT_AUTHORITY, SECURITY_INTERACTIVE_RID))

That keeps console/RDP on the packaged factory and sends logons without the Interactive SID to the existing desktop factory. A native probe of the actual method passed packaged/unpackaged cases with an interactive token and a restricted impersonation token. It is not an end-to-end packaged SSH/WinRM test.

There is one lifecycle issue to settle before I replace this branch: the factories keep separate catalogs but share SourceDetails::LastUpdateTime. If an interactive run refreshes the extension, a remote run a minute later can skip refreshing its older local catalog. Alternating runs can keep renewing the shared timestamp and leave that catalog stale indefinitely; the reverse ordering has the same problem.

Would separate update-check timestamps for the two backing catalogs fit the intended direction? I can keep that in the existing source metadata rather than reintroducing the fallback cache/version-selection machinery. I am holding the replacement until this is agreed; the current fallback implementation also has two unresolved review findings around deployment locking and cache removal on transient open failures, so it should not be merged as-is.

[JohnMcPMS]

I needed to heavily refactor this code for other reasons, so I included this check in #6584

My resolution to the update time issue is to use the newest of any stored packages, regardless of the two storage locations.

[Przemysław Kłys (PrzemyslawKlys)]

Thanks. I checked #6584 at 229181a: CanUseDeployedPackage() includes the interactive-user check, and the composite store compares both stored versions for reads and update checks. That addresses the stale-catalog scenario I described without separate update timestamps.

This supersedes the factory-only replacement I was preparing. I am keeping this PR parked as a draft while #6584 completes review; it can be retired when that replacement lands.