Skip to content

ft/address security review issues - #17

Open
bolorundurowb wants to merge 6 commits into
masterfrom
ft/address-security-review-issues
Open

ft/address security review issues#17
bolorundurowb wants to merge 6 commits into
masterfrom
ft/address-security-review-issues

Conversation

@bolorundurowb

Copy link
Copy Markdown
Owner

No description provided.

… roots

- Introduced `InstallPathPolicy` to prevent unsafe directory usage during install/uninstall operations.
- Added `RelativePathGuard` to enforce strict validation for filenames and relative paths in manifests.
- Developed `RuntimeManifestGuard` for runtime validation of manifest tasks and services to ensure security.
- Implemented `WindowsServiceOwnership` to verify service binaries are confined to the install root.
- Added `InstallPayloadLimits` to enforce resource limits for decompression and zip extraction.
- Introduced comprehensive regression tests covering edge cases related to shell injection, path validation, PID re-validation, payload limits, and runtime manifest safeguards.
- Verified proper behavior of security mechanisms like `PosixPathPal`, `RelativePathGuard`, `InstallPathPolicy`, and more.
- Ensured adherence to stricter handling of invalid inputs in installation and uninstallation processes.
- Improved input validation for `create_shortcut` and `create_desktop_entry` tasks to restrict filenames to simple names without path traversals or separators.
- Replaced string-based validation with `RelativePathGuard`/`RuntimeManifestGuard` for stricter checks across manifests, service management, and file paths.
- Enhanced `InstallProcessGuard` and related methods to revalidate process paths and enforce directory constraints.
- Introduced stricter handling for payload extraction to prevent traversal exploits using `InstallPayloadLimits` and comprehensive directory validation.
- Updated uninstall logic to ensure proper ownership verification for system services, registry keys, and environment variables, avoiding unauthorized modifications.
- Added detailed logging and tests for all key changes to improve maintainability and security coverage.
- Provided XML summaries for public classes, methods, and properties, enhancing clarity and maintainability.
- Updated `ManifestYaml`, `UninstallRunner`, `FeatureDefinition`, and other modules with detailed descriptions.
- Improved tooltips and API references for IDE integrations by including `<summary>`, `<param>`, and `<returns>` tags.
- Ensured alignment with existing security and process management updates.
- Added descriptions to schema elements for improved clarity and developer guidance.
- Updated key properties like `metadata`, `build`, `ui`, `files`, `tasks`, and others with explanatory details.
- Enhanced understanding of configuration options across platforms: Windows, Linux, and macOS.
- Clarified optional fields, default values, and feature gating mechanisms.
@codecov

codecov Bot commented Aug 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 64.33121% with 112 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.24%. Comparing base (dd67075) to head (9a20cae).

Files with missing lines Patch % Lines
...rc/PolyInstall.Core/Pal/WindowsServiceOwnership.cs 26.08% 32 Missing and 2 partials ⚠️
...yInstall.Core/Pal/LinuxSystemdServiceManagerPal.cs 0.00% 13 Missing ⚠️
...yInstall.Core/Pal/MacOsLaunchdServiceManagerPal.cs 0.00% 13 Missing ⚠️
src/PolyInstall.Core/Install/InstallPathPolicy.cs 79.59% 6 Missing and 4 partials ⚠️
.../PolyInstall.Core/Manifest/RuntimeManifestGuard.cs 80.00% 4 Missing and 5 partials ⚠️
...c/PolyInstall.Core/Install/UninstallCoordinator.cs 53.33% 5 Missing and 2 partials ⚠️
...c/PolyInstall.Core/Pal/WindowsServiceManagerPal.cs 0.00% 6 Missing ⚠️
...PolyInstall.Core/Install/WindowsArpRegistration.cs 0.00% 5 Missing ⚠️
src/PolyInstall.Core/Pal/PosixPathPal.cs 76.47% 2 Missing and 2 partials ⚠️
src/PolyInstall.Core/Pal/ProcessManagerPal.cs 42.85% 2 Missing and 2 partials ⚠️
... and 5 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #17      +/-   ##
==========================================
+ Coverage   50.40%   52.24%   +1.83%     
==========================================
  Files          79       84       +5     
  Lines        3718     3968     +250     
  Branches      810      867      +57     
==========================================
+ Hits         1874     2073     +199     
- Misses       1641     1660      +19     
- Partials      203      235      +32     
Flag Coverage Δ
unittests 52.24% <64.33%> (+1.83%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

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

1 participant