Enforce explicit ID filters on REST search results - #6514
Enforce explicit ID filters on REST search results#6514AmirMS (AmelBawa-msft) wants to merge 4 commits into
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
🟢 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>
…osoft/winget-cli into user/amelbawa/source-filter
JohnMcPMS
left a comment
There was a problem hiding this comment.
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
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; |
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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.
📖 Description
Reject REST results that fail explicit package ID filters, preventing a matching name or query from admitting a package with a different ID.
FilterSearchResultreuses comparisons throughMatchesRequestfor 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
ICUCaseInsensitiveStartsWithto 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
🔗 References
Resolves #2966.
🔍 Validation
wingetdevand 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 packageXP8BT8DW290MPQbecause its ID does not matchMicrosoft.Teams.✅ Checklist
📋 Issue Type
Microsoft Reviewers: Open in CodeFlow