Skip to content

Contain DI software packages beneath their configured root (OPC 10000-100)#4053

Open
marcschier wants to merge 2 commits into
masterfrom
marcschier/review-fix/di-package-containment
Open

Contain DI software packages beneath their configured root (OPC 10000-100)#4053
marcschier wants to merge 2 commits into
masterfrom
marcschier/review-fix/di-package-containment

Conversation

@marcschier

@marcschier marcschier commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR splits the DI software-package containment changes from #4021.

Specification

The provider-backed package cache must therefore keep each package payload and its metadata together beneath the configured store root.

Failure

FileSystemPackageStore rejected path separators in package identifiers but accepted . and ... It also accepted configured roots containing dot segments. Provider path normalization could therefore resolve a package operation outside the configured package-store root.

Fix

  • Canonicalize configured provider-relative roots to forward-slash absolute provider paths.
  • Reject dot segments in both package identifiers and configured roots.
  • Re-canonicalize every constructed package path and require it to be a strict descendant of the configured root before calling the file-system provider.
  • Preserve valid mixed-separator roots by normalizing them deterministically.

Validation

  • Full Opc.Ua.Di.Tests: net10.0 (288 passed), net48 (288 passed).
  • Review follow-up PackageStoreTests: net10.0 (15 passed), net48 (15 passed).
  • git diff --check passed.

Related Issues

Checklist

Only verified claims are checked.

  • I have signed the CLA and read the CONTRIBUTING doc.
  • I have added tests that prove my fix is effective or that my feature works and increased code coverage.
  • I have added all necessary documentation.
  • I have verified that my changes do not introduce (new) build or analyzer warnings.
  • I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed.
  • I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings.
  • I have addressed all PR feedback received.

…-100)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@marcschier
marcschier marked this pull request as ready for review July 19, 2026 11:31
Copilot AI review requested due to automatic review settings July 19, 2026 11:31
@marcschier
marcschier marked this pull request as draft July 19, 2026 11:36

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.

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Hardens FileSystemPackageStore path handling so DI software packages (payload + metadata) are always contained under the configured provider root, preventing dot-segment/path normalization from escaping the store root.

Changes:

  • Add canonicalization + strict descendant checks for provider paths in FileSystemPackageStore.
  • Reject dot segments (. / ..) in package identifiers and configured roots.
  • Add regression tests covering dot-segment rejection and root canonicalization.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
tests/Opc.Ua.Di.Tests/PackageStoreTests.cs Adds tests for dot-segment rejection and deterministic root canonicalization behavior.
src/Opc.Ua.Di.Server/SoftwareUpdate/FileSystemPackageStore.cs Canonicalizes provider paths, rejects dot segments, and blocks paths from escaping configured root.

Comment thread tests/Opc.Ua.Di.Tests/PackageStoreTests.cs Outdated
Comment thread src/Opc.Ua.Di.Server/SoftwareUpdate/FileSystemPackageStore.cs Outdated
Comment thread src/Opc.Ua.Di.Server/SoftwareUpdate/FileSystemPackageStore.cs Outdated
@marcschier
marcschier marked this pull request as ready for review July 19, 2026 11:55
@marcschier
marcschier marked this pull request as draft July 19, 2026 12:45
@marcschier
marcschier marked this pull request as ready for review July 19, 2026 14:10
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@marcschier
marcschier marked this pull request as draft July 19, 2026 18:04
@marcschier
marcschier marked this pull request as ready for review July 19, 2026 18:08
@marcschier
marcschier requested a review from romanett July 19, 2026 18:37
@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.52%. Comparing base (70c74e7) to head (b106271).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4053      +/-   ##
==========================================
- Coverage   73.55%   73.52%   -0.04%     
==========================================
  Files        1345     1345              
  Lines      180036   180038       +2     
  Branches    31677    31678       +1     
==========================================
- Hits       132427   132374      -53     
- Misses      36911    36955      +44     
- Partials    10698    10709      +11     

see 23 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants