Validate REST search results against filters and selectors

[AmirMS (AmelBawa-msft)]

📖 Description

Reject REST candidates that contradict the search request before package selection, rather than trusting permissive server results.

• Add shared tri-state matching in MatchCriteriaResolver for (Query OR Inclusions...) AND Filters..., covering IDs, names, monikers, tags, commands, and system references.
• Use the same capability-validated request for serialization and filtering. Retrieve and cache missing manifests only when needed to evaluate a match.
• Filter normal and optimized lookup results before result limits. Prevent unrestricted searches when all selectors are unsupported, and fail explicitly on continuation-token cycles.
• Preserve installed-package correlation through paired name/publisher data and correct version metadata.
• Fix Unicode prefix matching in ICUCaseInsensitiveStartsWith, and add regression coverage and release notes.

Only definite mismatches are rejected. Source-defined queries, unsupported criteria, and unavailable metadata remain conservative. This does not add interactive selection or cross-source equivalence heuristics.

🎞️ Demo

🔗 References

Resolves #2966.
Related to #3547 and #6297.

🔍 Validation

• Built x86 Release and the x64 Debug development package. Passed 225 targeted native test cases / 4,754 assertions covering REST, matching, composite sources, source priority, strings, and completion. The package build completed with packaging/resource warnings.
• Passed 14 sequential, read-only wingetdev checks. Verbose logs confirmed ID-filter rejection, manifest retrieval and rejection for show ., and preservation of the Store ID lookup fallback.
• Captured seven live before/after scenarios covering AND/OR semantics, genuine ambiguity, filtered searches, command metadata, and result limits.

✅ Checklist

• Signed the Contributor License Agreement
• Linked to an issue
• Updated Release Notes (if applicable)
• Updated documentation (if applicable)
• Updated Copilot instructions (if build, architecture, or conventions changed)

📋 Issue Type

• Bug fix
• Feature
• Task

[Copilot]

🟢 Approval recommended

The filtering behavior is correctly integrated across REST paths and supported by focused regression coverage.

Pull request overview

Enforces explicit package ID filters on REST results and fixes Unicode-aware prefix matching.

Changes:

• Filters REST search and optimized lookup results by explicit IDs.
• Corrects Unicode case-folded prefix comparisons.
• Adds regression tests and release notes.

File summaries

File	Description
src/AppInstallerSharedLib/AppInstallerStrings.cpp	Fixes Unicode prefix matching.
src/AppInstallerRepositoryCore/Rest/Schema/1_0/RestInterface_1_0.cpp	Enforces ID filters before limits.
src/AppInstallerRepositoryCore/MatchCriteriaResolver.h	Exposes reusable match evaluation.
src/AppInstallerRepositoryCore/MatchCriteriaResolver.cpp	Implements optional local matching.
src/AppInstallerCLITests/Strings.cpp	Tests Unicode prefix behavior.
src/AppInstallerCLITests/RestInterface_1_1.cpp	Tests inherited REST filtering.
src/AppInstallerCLITests/RestInterface_1_0.cpp	Covers filtering, pagination, and fallback behavior.
src/AppInstallerCLITests/MatchCriteriaResolver.cpp	Tests supported and unsupported match types.
doc/ReleaseNotes.md	Documents both fixes.

Review details

• Files reviewed: 9/9 changed files
• Comments generated: 0
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[JohnMcPMS]

The scope is too limited and doesn't meet the criteria for the "hero" case. This function generates that:

winget-cli/src/AppInstallerCLICore/Workflows/WorkflowBase.cpp

Lines 879 to 906 in c17eadb

	void GetSearchRequestForSingle(Execution::Context& context)
	{
	const auto& args = context.Args;
	MatchType matchType = MatchType::CaseInsensitive;
	if (args.Contains(Execution::Args::Type::Exact))
	{
	matchType = MatchType::Exact;
	}
	SearchRequest searchRequest;
	// Note: MultiQuery when we need search for single is handled with one sub-context per query.
	if (args.Contains(Execution::Args::Type::Query))
	{
	std::string_view query = args.GetArg(Execution::Args::Type::Query);
	// Regardless of match type, always use an exact match for the system reference strings.
	searchRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::PackageFamilyName, MatchType::Exact, query));
	searchRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::ProductCode, MatchType::Exact, query));
	searchRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::Id, matchType, query));
	searchRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::Name, matchType, query));
	searchRequest.Inclusions.emplace_back(PackageMatchFilter(PackageMatchField::Moniker, matchType, query));
	}
	SearchSourceApplyFilters(context, searchRequest, matchType);
	context.Add<Execution::Data::SearchRequest>(std::move(searchRequest));
	}

We need a more generalized filtering pass, and it should probably live outside of the REST code since it isn't REST specific.

[ranm-msft]

Read the diff against head (64ae352) and walked FilterSearchResult through SearchInternal's continuation loop. Not re-raising the Inclusions gap or the generalization/placement point - John has those covered.

One thing I'd like your read on, and the new Search_ExplicitIdFilters_Continuation test suggests you already thought about it: the loop's exit condition is results.Matches.size() < request.MaximumResults, and filtering now runs before that count advances. The test encodes exactly that - allFiltered with MaximumResults == 1 still issues all three requests. I think that is the right semantics, since a valid match can live on page three and you cannot stop after N raw candidates. But it does mean MaximumResults no longer bounds round trips at all when a source returns candidates the filter rejects, which is the lax-source case this PR exists for.

Is it worth a separate bound on continuation requests - a page budget, or repeated-token detection - so "limit results" and "limit work" stay distinct concepts? Against a source that pages a broad name match, a bounded request can now walk the whole chain one POST at a time. Happy to be told the practical chain length makes this theoretical.

[Copilot]

Copilot review overview

🔵 Needs a closer look

It changes core REST filtering, pagination, manifest retrieval, and package correlation across several interfaces, requiring final human validation.

Review effort: Balanced
Findings: None

[ranm-msft]

Thanks for the updates - I re-read at the current head and my earlier continuation concern is addressed. The repeated continuation-token detection plus the three-package manifest-retrieval budget, both with regression coverage, cover what I was worried about.

One thing I would like clarified before I sign off: how is NormalizedNameAndPublisher expected to be validated client-side? From the current flow, MatchesPackage appears to delegate that criterion to manifest matching, manifest matching returns no determination for it, and FilterSearchResult does not request a manifest for it. The test around RestInterface_1_0.cpp:1470-1480 looks like it retains Foo.Bar for a non-matching filter value while expecting zero manifest requests.

If that criterion is intentionally server-enforced, or is only a correlation hint rather than a hard selector, could we capture that contract somewhere? Otherwise the installed-correlation path, which is built from exact name+publisher pairs, would keep results the client has not actually validated against the selector.

[AmirMS (AmelBawa-msft)]

ranm-msft, following up on your review:

You’re right: NormalizedNameAndPublisher is not validated locally, and we cannot assume server enforcement because REST requests omit the publisher (microsoft/winget-cli-restsource#59). I’ve documented the limitation without changing behavior. Should we address client-side validation in this PR, or handle it separately?

[ranm-msft]

I went back over the delta since I last looked, and the change I cared about is addressed. Enrichment now only runs when the complete source result set is at or under three, with the request widened just enough to tell three from more than three, rather than taking the first three candidates off an unbounded set. That keeps the contract I was worried about: a locally filtered result is only dropped on a proven mismatch, never because it happened to fall outside an arbitrary window.

The NormalizedNameAndPublisher point is answered as a documented limitation rather than client-side validation, which I think is the right call here - the REST 1.0 schema has no publisher side to match against, so validating it in the client would be inventing a comparison the source cannot support. Recording that retained candidates are unvalidated for that selector is honest and leaves the door open for a schema-side fix.

I am holding off on approving only because of state, not content: the x64 and x86 test legs are still pending at the head, and John's approval predates the last two commits. Once the test legs come back green I am happy for this to go in, and if John re-confirms at the current head I have nothing further.