Skip to content

Enforce explicit ID filters on REST search results - #6514

Open
AmirMS (AmelBawa-msft) wants to merge 4 commits into
feature/multi-source-deduplicationfrom
user/amelbawa/source-filter
Open

Enforce explicit ID filters on REST search results#6514
AmirMS (AmelBawa-msft) wants to merge 4 commits into
feature/multi-source-deduplicationfrom
user/amelbawa/source-filter

Conversation

@AmelBawa-msft

@AmelBawa-msft AmirMS (AmelBawa-msft) commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

📖 Description

Reject REST results that fail explicit package ID filters, preventing a matching name or query from admitting a package with a different ID.

FilterSearchResult reuses comparisons through MatchesRequest for Exact, CaseInsensitive, StartsWith, and Substring matching. Filtering runs before result limits and also applies to optimized ID lookups, preserving pagination and the substring-ID fallback.

Also fix ICUCaseInsensitiveStartsWith to fold complete strings before comparing prefixes, preventing valid Unicode matches from being discarded. Add regression coverage and release notes.

Local rejection is limited to explicit ID filters. Other fields and unsupported match types remain server-evaluated.

🎞️ Demo

winget-rest-id-filter-before-after

🔗 References

Resolves #2966.

🔍 Validation

  • Built x64 Debug successfully. Passed 202 native test cases / 2,959 assertions covering strings, REST interfaces, matching, completion, source priority, and composite sources.
  • Deployed wingetdev and passed 10 sequential, read-only live CLI checks covering combined filters, case sensitivity, invalid IDs, and Store lookups. Verbose logs confirmed that the original Teams scenario discards Store package XP8BT8DW290MPQ because its ID does not match Microsoft.Teams.

✅ Checklist

📋 Issue Type

  • Bug fix
  • Feature
  • Task
Microsoft Reviewers: Open in CodeFlow

@github-actions

This comment was marked as resolved.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 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.

The spelling workflow rejects the intentional Unicode test identifier
suffix `EApp` as an unrecognized word.

- **Spelling metadata**
  - Add `EApp` to the project-specific allowlist.
  - Preserve the Unicode case-folding regression test unchanged.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: AmelBawa-msft <104940545+AmelBawa-msft@users.noreply.github.com>

@JohnMcPMS JohnMcPMS left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

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.

case MatchType::Wildcard:
default:
return ValueMatchFunction_AlwaysFalse;
return nullptr;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fix the other caller to handle a possible null now. I get that it uses a fixed list and that list won't ever get a null from this implementation, but I don't like having that silent coupling.


void FilterSearchResult(const SearchRequest& request, IRestClient::SearchResult& result)
{
for (const auto& filter : request.Filters)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This ignores Inclusions. The goal should be to ensure that the results reflect the request fully, not just that it works in the narrow case we are targeting.

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

Labels

Needs-Author-Feedback Issue needs attention from issue or PR author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants