microsoft / microsoft/WinAppVSCE
XAML: constrain document-link targets to allowed roots
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 13
- Forks
- 3
- Avg merge
- 6d 1h
- Merged PRs (30d)
- 11
Description
Follow-up from the PR #50 review (finding M1).
server/src/WinUiXaml.LanguageServer/XamlDocumentLinks.cs Combine() (lines 134-152) resolves a link target with Path.GetFullPath and then probes File.Exists, with no containment check against the workspace/project root. A crafted Source="C:\..." or a ..-heavy relative path in a XAML file therefore produces a clickable document link to an arbitrary local path.
Severity is limited because:
package.jsondeclaresuntrustedWorkspaces: "limited", so the server does not run in untrusted workspaces.- The result is a link the user must click, not an automatic read of file contents.
The reason this was not fixed in PR #50: server/test/WinUiXaml.LanguageServer.Tests/XamlDocumentLinksTests.cs lines 265-272 explicitly asserts that an out-of-root absolute path such as C:\other\Colors.xaml resolves to a link. Tightening Combine() would break a blessed test, so this needs a product decision first.
Suggested work:
- Decide whether absolute out-of-root link targets are intended (they are legitimate for some shared-resource-dictionary layouts).
- If not, constrain resolution to the project/workspace roots and update
XamlDocumentLinksTests.csaccordingly.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with Combine() in server/src/WinUiXaml.LanguageServer/XamlDocumentLinks.cs and the out-of-root case in server/test/WinUiXaml.LanguageServer.Tests/XamlDocumentLinksTests.cs. Resolve the product decision about absolute targets first; done means the chosen containment policy is implemented and the affected test assertions match it.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- security, tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100