Feat/windows installer #11
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Implements a complete Windows service wrapper for PinShare with: - Native Windows service using golang.org/x/sys/windows/svc - Auto-start on Windows boot - Process management for IPFS and PinShare backends - Health monitoring with automatic restart (max 3 attempts) - Embedded UI server serving React static files - Reverse proxy for API requests (/api -> backend, /ipfs-api -> IPFS) - Configuration via Windows Registry or JSON file - Windows Event Log integration - Graceful shutdown handling with context cancellation - System tray icon with context menu - Service control (Start/Stop/Restart) - Status monitoring (Running/Stopped/Starting/etc.) - One-click UI access via default browser - Log directory access - Auto-start with Windows (via installer) - Professional MSI package - Installs binaries to Program Files - Creates data directories in ProgramData - Registers Windows service with auto-start - Sets up registry configuration - Adds tray app to startup folder - Creates Start Menu shortcuts - Configures service recovery options - Automated build via build.bat - Cross-compilation support (Linux/macOS -> Windows) - Automated builds for all components - IPFS Kubo binary download - React UI production build - Installer generation - Clean targets - Complete installation guide (docs/windows/README.md) - Detailed build instructions (docs/windows/BUILD.md) - Troubleshooting and configuration - Implementation overview (WINDOWS_SERVICE.md) - Native execution (no Docker required) - Service wraps IPFS daemon and PinShare backend - Health checker monitors both components (30s intervals) - Embedded UI server on localhost:8888 - All state in C:\ProgramData\PinShare - Binaries in C:\Program Files\PinShare - Primary: Windows Registry (HKLM\SOFTWARE\PinShare) - Fallback: JSON file (C:\ProgramData\PinShare\config.json) - Configurable ports, paths, feature flags - Organization and group names Dependencies added: - github.com/getlantern/systray for system tray - golang.org/x/sys/windows for Windows service APIs
…ucture ## Critical Security Fixes 1. **Secure encryption key generation** (config.go) - Replaced hardcoded placeholder key with crypto/rand generated key - Uses 32 cryptographically secure random bytes - Panics on failure as this is critical for security 2. **Fix health checker race condition** (service.go) - Initialize health checker before starting IPFS/PinShare processes - Fixes nil pointer dereference in waitForIPFS() and waitForPinShare() - Health checker now available during startup health checks 3. **Replace WiX installer placeholder GUID** (Product.wxs) - Generated proper UpgradeCode GUID: 24D1704E-0ACD-4370-BCF9-ED664A546060 - Required for Windows installer upgrades to work correctly 4. **Fix debug.New() return value handling** (service.go) - Corrected openEventLog() to match actual API (returns single value) ## Testing Infrastructure Added comprehensive Windows testing automation: - **Test-PinShare.ps1**: PowerShell automation script with modular test suites - Build, Service, Health, API, UI, Integration, All, Cleanup suites - Automated service installation and lifecycle testing - Health check verification for IPFS, PinShare API, and UI - Detailed logging with JSON results export - **TESTING.md**: Complete testing guide and strategy - Quick start and usage instructions - Manual testing procedures and troubleshooting guides - Development workflow patterns for rapid iteration - CI/CD integration examples (GitHub Actions, Azure DevOps) - Performance testing strategies These changes address critical security vulnerabilities identified in code review and provide the tooling needed for rapid Windows development iteration.
Updates the Windows installer to use modern WiX 6 tooling: ## Changes ### New Files - **Package.wxs**: WiX 6 format installer definition - Uses new namespace: http://wixtoolset.org/schemas/v4/wxs - <Package> element instead of <Product> - <StandardDirectory> for system folders - Bitness="always64" on all components - FileRef instead of FileKey for custom actions - **PinShare.wixproj**: MSBuild SDK-style project - Uses WixToolset.Sdk/6.0.2 - Built-in HarvestDirectory for auto-harvesting UI files - No need for manual heat.exe calls - **build-wix6.bat / build-wix6.sh**: Modern build scripts - Checks for .NET SDK and WiX tool - Auto-installs WiX if missing - Uses 'dotnet build' instead of candle/light - **README-WIX6.md**: Comprehensive WiX 6 documentation ## Why WiX 6? WiX 3 is deprecated. WiX 6 is the current version with: - .NET tool installation: dotnet tool install --global wix - MSBuild integration: dotnet build PinShare.wixproj - Simplified syntax and auto-harvesting - Better CI/CD integration ## Building Prerequisites: 1. .NET SDK 6+ (dotnet.microsoft.com) 2. WiX tool: dotnet tool install --global wix Build: ```bash make -f Makefile.windows windows-all cd installer build-wix6.bat # or ./build-wix6.sh on Linux ``` Output: bin/Release/PinShare-Setup.msi ## Migration Notes - Old build.bat (WiX 3) is kept for reference but deprecated - Package.wxs replaces Product.wxs with WiX 6 syntax - All custom actions and components preserved - Service installation/startup logic unchanged
Provides alternative build methods for Windows users: New Files: - build-windows.ps1: PowerShell build script with interactive prompts - build-windows.bat: Batch script for Git Bash/CMD - BUILD-WINDOWS-SIMPLE.md: Simple build guide without make Features: - No make dependency required - Builds all components (backend, service, tray, UI) - Auto-downloads IPFS if missing - Optional installer build with prompt - Works in PowerShell, CMD, and Git Bash - Clear progress output and error handling Usage: PowerShell: .\build-windows.ps1 Batch: build-windows.bat Manual: See BUILD-WINDOWS-SIMPLE.md This is easier for Windows developers who don't have make installed.
…ucture Fix tray application compilation error: - Replace non-existent svc.Start with proper service.Start() call - Create separate startService() function for starting services - Keep controlService() for stop/pause/continue commands The Windows service control API doesn't have a svc.Start command. Starting a service requires calling service.Start() directly on the service object, while other control commands (Stop, Pause, Continue) use service.Control(cmd). This fixes the compilation errors: cmd\pinshare-tray\tray.go:138:31: undefined: svc.Start cmd\pinshare-tray\tray.go:173:31: undefined: svc.Start Verified: Cross-compiled successfully for windows/amd64
Ignore dist/, bin/, and *.msi files as these are build artifacts that should not be committed to version control.
The pinshare-ui is on the infra/refactor branch and will be merged later. For now, disable all UI-related components to allow the installer to build: Changes: - PinShare.wixproj: Comment out HarvestDirectory for UI files - Package.wxs: Comment out UIComponents ComponentGroupRef - Package.wxs: Comment out UIFolder directory definition - Package.wxs: Comment out UIComponents Fragment The installer now builds only the service wrapper, backend, and tray app. UI can be re-enabled once pinshare-ui is merged from infra/refactor.
The UI is temporarily disabled in the installer configuration, so the build script shouldn't check for UI files either. This allows the installer to build with just: - pinsharesvc.exe - pinshare.exe - pinshare-tray.exe - ipfs.exe UI will be re-enabled when pinshare-ui is merged from infra/refactor.
Previously the script would always print 'Build Complete!' even if the installer build failed. Now it properly checks: - If 'cd installer' succeeds - If 'build-wix6.bat' succeeds If either fails, the script exits with an error code instead of falsely claiming success.
WiX 4+ requires conditions to be in a 'Condition' attribute, not as inner text. Changed from: <Custom Action="..." Before="...">NOT Installed</Custom> To: <Custom Action="..." Before="..." Condition="NOT Installed" /> This fixes the WIX0004 errors: - The Custom element contains illegal inner text Fixes 4 compilation errors.
…try sections error
…ntry sections error
…pVT handling - Add install directory to PATH when starting PinShare subprocess so `ipfs` command is found - Add environment variable loading for path configs (PS_UPLOAD_FOLDER, PS_CACHE_FOLDER, PS_REJECT_FOLDER, PS_METADATA_FILE, PS_IDENTITY_KEY_FILE) - Add environment variable loading for feature flags (PS_FF_ARCHIVE_NODE, PS_FF_CACHE) - When FFSkipVT=true, bypass security capability checks and set capability=2 to allow startup without virus scanning infrastructure - Fix uploads.go and downloads.go to check FFSkipVT flag before any security scanning, not just for SecurityCapability==4
- Add /api/health endpoint to API server for service health monitoring - Increase PinShare startup timeout from 30s to 60s to allow for libp2p initialization and peer discovery
- Use SCRIPT_DIR variable with absolute paths throughout - Replace cd with pushd/popd for reliable directory navigation - Fix installer directory path to use absolute path - Now works correctly when run from any directory
- Add comprehensive docs/windows-architecture.md documenting process hierarchy, components, data flow, and configuration - Fix pinshare-tray icon not displaying by generating a valid 16x16 ICO format icon at runtime instead of the malformed 62-byte placeholder
- Create ConfigDialog.wxs with custom TextStyle definitions (PinShare_Font_Bold) to avoid undefined WixUI_Font_Bold errors when WixUI extension was disabled - Enable WixToolset.UI.wixext in PinShare.wixproj for proper installer UI - Add WixUI_Minimal reference and license RTF in Package.wxs - Define configurable properties (ORGNAME, GROUPNAME, SKIPVIRUSTOTAL, ENABLECACHE) that can be customized during installation - Update registry entries to use the configurable properties The ConfigDialog is defined and ready for integration into the install sequence, but currently uses WixUI_Minimal for simplicity.
Reverts to the working state from the 01NLPuCxVe8RG15ksJTz1N3E branch: - Remove ConfigDialog.wxs (UI temporarily disabled until infra/refactor merge) - Disable WixToolset.UI.wixext in PinShare.wixproj - Revert Package.wxs to use hardcoded registry values - Keep WixUI_Minimal commented out for now This fixes the WIX0204 errors about undefined TextStyle WixUI_Font_Bold.
…ssing Changed from exit with error to graceful skip using goto :skip_ui when the pinshare-ui directory doesn't exist. UI will be added later from the infra/refactor branch.
Restore installer configuration:
- Restore ConfigDialog.wxs with full configuration wizard UI
(Network Identity, Ports, Features, Startup Options)
- Restore Package.wxs with WixUI_InstallDir integration and
configurable properties (ORG_NAME, ports, SKIP_VIRUSTOTAL, etc.)
- Enable WixToolset.UI.wixext in PinShare.wixproj
Add dynamic version support:
- build-windows.bat: Detect version from git tags (git describe)
- build-wix6.bat: Accept version parameter, pass to dotnet build
- PinShare.wixproj: Accept ProductVersion via command line
- Package.wxs: Use preprocessor variable with fallback
Add GitHub Actions workflow:
- New windows-build.yml workflow for automated builds
- Triggers on version tags (v*) or manual dispatch
- Builds all Windows binaries and MSI installer
- Uploads versioned MSI to GitHub Releases
The installer now creates PinShare-{version}-Setup.msi with the
version embedded in both the filename and MSI metadata.
When no version tag (v*) exists, fall back to 1.0.0 instead of using the commit hash. MSI versions must be numeric X.Y.Z or X.Y.Z.W format. - Use --match "v[0-9]*" to only match version tags - Fall back to 1.0.0 when no version tags exist - Properly validate commit count is numeric before appending
Define WixUI_Font_Normal, WixUI_Font_Bold, and WixUI_Font_Title in our custom UI fragment since they aren't inherited from the WixUI extension when using a separate UI element.
ConfigDialog.wxs: - Rename TextStyles to PinShare_Font_* to avoid conflicts with WixUI - Add new "Directories" section with Upload folder field - Expand dialog height to accommodate new field - Adjust Y positions of all elements Package.wxs: - Add UPLOAD_DIR property with default path - Update registry to use UPLOAD_DIR property
- Delete installer/build.bat (legacy WiX 3.x) - build-wix6.bat is the current installer build script
- Update docs/windows/README.md: - Clarify localhost binding for API ports - Add security note for identity.key - Update firewall section with Git Bash commands - Add port reachability testing instructions - Update GitHub URLs to Cypherpunk-Labs - Update docs/windows/BUILD.md: - Update to WiX 6 tooling (via .NET tool) - Remove SQLite/CGO references - Prefer Git Bash throughout - Update GitHub URLs to Cypherpunk-Labs - Update Makefile.windows: - Add platform detection - Update installer target for WiX 6 - Create internal/winservice/constants.go: - Consolidate ServiceName constants - Update tray and service code: - Use shared winservice.ServiceName - Extract time constants
- Add shared port defaults to internal/winservice/constants.go - Add service control timeouts and recovery action delays - Update cmd/pinsharesvc/config.go to use winservice constants - Update cmd/pinsharesvc/service_control.go to use winservice constants - Update cmd/pinsharesvc/process.go to use winservice constants - Update cmd/pinshare-tray/config.go to use winservice constants - Add Reload method to TrayConfig - Extract encryption key length to const This addresses PR feedback to consolidate duplicate constants and extract magic numbers.
- Extract updateStatus switch cases into dedicated handler methods: - handleStatusError, handleStatusRunning, handleStatusStopped - handleStatusStartPending, handleStatusStopPending, handleStatusNotInstalled - Replace hardcoded "/" with filepath.Join() in uploads.go for OS-agnostic paths
…ection - Replace inline PowerShell IPFS download with robust script - Add file validation and ZIP magic bytes check - Implement dual extraction methods (.NET ZipFile + Expand-Archive fallback) - Clean up failed download attempts automatically - Auto-detect .NET SDK in common install locations - Check Program Files and user profile directories - Automatically add to PATH when found - Fixes "dotnet command not found" despite SDK being installed - Add build verification and troubleshooting tools - test-build-fixes.bat validates environment setup - build-windows-no-installer.bat for binaries-only builds - BUILD_TROUBLESHOOTING.md documents all fixes All changes maintain Windows 10/11 compatibility.
Code changes: - Extract timing constants to winservice package (IPFSStartTimeout, PinShareStartTimeout, HealthCheckPoll, recovery delays) - Move ServiceState type from tray.go to winservice package - Rename ProcessManager mutex to processMu for clarity - Rename GetIPFSRepoPath to GetIPFSDataPath - Rename repoPath variable to ipfsDataPath throughout - Change "IPFS repository" log messages to "IPFS" - Parallelize file processing in uploads.go with worker pool Documentation consolidation: - Merge windows-architecture.md diagrams into SERVICE.md - Delete windows-architecture.md (content merged) - Rename installer/README-WIX6.md to installer/README.md - Delete old WiX 3.x installer/README.md - Fix repository URLs from Episk-pos to Cypherpunk-Labs - Update QUICKSTART.md references and remove stale info
Resolved conflicts: - docs/windows-architecture.md: kept deletion (merged into SERVICE.md) - docs/windows/README.md: kept fork's documentation URL format - cmd/pinsharesvc/*.go: resolved import formatting - cmd/pinshare-tray/tray.go: kept winservice.ServiceState (shared package) - internal/winservice/constants.go: removed duplicate declarations - cmd/pinsharesvc/process.go: removed duplicate import
- Replace PowerShell MessageBox settings with lxn/walk native Windows dialog - Add tabbed interface: Network Ports, Organization, Features, Security, Info - Add UAC elevation fallback for config saves to ProgramData - Fix service restart after settings save by waiting for service to fully stop - Add orphaned process cleanup on service startup - Add Windows manifest for visual styles and DPI awareness
- Add getAPIPort() to read PORT env var in internal/api/main_api.go - Add getIPFSAPIPort() to read IPFS_API env var in internal/app/app.go - Fixes hardcoded port 9090/5001 issue when config.json specifies different ports
- Fix configureIPFS() to include all transport protocols (TCP, UDP/QUIC, WebRTC, WebTransport) - Add --json flag for setting IPFS Swarm addresses array - Improve lock file removal with retry logic (3 attempts with delays) - Move lock file removal after process termination delay - Add logging for IPFS port configuration - Fix View Logs to open %LOCALAPPDATA%\PinShare\logs instead of ProgramData - Set default upload directory in installer to %LOCALAPPDATA%\PinShare\upload - Update service start type handling for existing installations
- Rename settings_walk.go to settings_dialog.go for clarity
- Extract magic numbers and configuration defaults to constants
- Add context parameter to handleMenuClicks() for graceful shutdown
- Make About version dynamic using shared winservice.Version constant
- Extract duplicate logic in handleRestartService()
- Use new(pinshareService) instead of &pinshareService{}
- Extract environment variable names to constants in config packages
- Factor out duplicate process killing logic in process.go
- Add TODO comments for interface/IP selection support
- Use OS-agnostic path construction with filepath.Join
- Replace string concatenation with fmt.Printf for logging
These changes improve code maintainability, reduce duplication,
and address review feedback from PR #3.
- uploads.go: rename variables (f→filename), add concurrency comments, extract processFileWithLimit(), use filepath.Join, use fmt.Printf - downloads.go: fix SecurityCapability type casting consistency - process.go: use dir constants, fix comment wording, update TODO ref - config.go: promote appName to module-level, remove duplicate fallback - tray.go: refactor handleRestartService to use handler methods - settings_dialog.go: extract magic numbers to constants, use winservice.Default*Port for labels - Update TODO references to GitHub issue #10
- Create internal/types package for shared SecurityCapability type - Update config.go to use types.SecurityCapability instead of int - Remove type casting in p2p/downloads.go and p2p/uploads.go - Consolidate tray.go constants with winservice package - Update SERVICE.md backup/restore examples with concrete paths - Update PR3_REVIEW_COMMENTS.md tracking file with completion status
When user clicks Exit in the tray menu, show a Yes/No/Cancel dialog asking whether to stop the service before exiting: - Yes: Stop service, then exit tray - No: Exit tray (service continues running) - Cancel: Stay in tray Addresses PR #3 review comment about tray exit behavior.
- Use dirLogs constant instead of hardcoded "logs" string in process.go - Fix GitHub URLs from Episk-pos to Cypherpunk-Labs in issue references - Remove stale git checkout line from SERVICE.md clone instructions
- Migrate pinsharesvc from switch-based CLI to Cobra commands - Add proper subcommands: install, uninstall, start, stop, restart, debug - Add --auto-start flag for install command - Improve help text and command descriptions - Remove "walk" naming from settings dialog functions
Extract tab page definitions into separate methods:
- createNetworkPortsTab()
- createOrganizationTab()
- createFeaturesTab()
- createSecurityTab()
- createInfoTab()
- createButtonsBar()
Improves readability by reducing the main Dialog{} declaration
from ~270 lines to ~20 lines.
Create cmd/pinshare-tray/constants.go with: - App identity: appName, appTooltip - Windows MessageBox flags: MB_OK, MB_YESNO, MB_YESNOCANCEL, MB_ICON* - MessageBox return values: IDYES, IDNO, IDCANCEL - Directory names: dirIPFS, dirPinShare, dirUpload, dirCache, dirRejected, dirLogs - File names: fileConfig, fileSession - Environment variables: envLocalAppData, envUserProfile, envProgramData, envUsername - Default paths: defaultLocalAppDataPath, defaultProgramDataPath Remove duplicate constants from main.go and settings.go. Update all files to use centralized constants.
Move duplicate constants to internal/winservice/constants.go: - AppName: application name for directory paths - DirIPFS, DirPinShare, DirUpload, DirCache, DirRejected, DirLogs - FileConfig, FileSession, FileServiceLog - EnvLocalAppData, EnvUserProfile, EnvProgramData, EnvProgramFiles, EnvUsername - DefaultLocalAppDataPath, DefaultProgramDataPath, DefaultProgramFilesPath Update cmd/pinsharesvc to use winservice.* constants directly. Update cmd/pinshare-tray to alias winservice constants for convenience. This eliminates duplication between pinsharesvc and pinshare-tray packages.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
This PR adds comprehensive Windows installer support with a WiX-based MSI installer, Windows service wrapper, and system tray application. The implementation includes proper service lifecycle management, health monitoring, concurrent upload processing improvements, and security capability refactoring.
Key Changes
- Windows Service Infrastructure: Added
pinsharesvc.exeservice wrapper that manages IPFS and PinShare processes with proper startup sequencing, health checks, and graceful shutdown - System Tray Application: Created
pinshare-tray.exefor user interaction with service control, status monitoring, and settings management - WiX Installer: Implemented MSI installer with service installation, startup shortcuts, and PowerShell-based service startup with retry logic for Windows 11 compatibility
- Configuration System: Moved from registry-based to JSON file configuration stored in user's
LOCALAPPDATA, with session markers for service-to-user data directory mapping - Concurrent Upload Processing: Refactored upload handling to process files concurrently (max 4 simultaneous) with proper semaphore limiting and atomic counting
- Security Refactoring: Moved security capability types to shared
internal/typespackage with cleaner abstraction and improved VirusTotal handling
Critical Issues Found
- Placeholder UpgradeCode in
installer/Package.wxsmust be replaced with production GUID before release - Service stops on tray exit which creates poor UX - closing tray shouldn't stop background service
- Broad SYSTEM permissions granted via
icaclson user data directories poses security risk
Architecture Improvements
- Proper Windows service integration with recovery actions and DACL permissions for non-admin service control
- Health checker with automatic process restart on failures
- Orphaned process cleanup on service startup to handle improper shutdowns
- Thread-safe metadata store operations with proper locking
Confidence Score: 3/5
- This PR has significant functionality but requires fixes before production deployment
- The Windows installer implementation is comprehensive and well-structured with proper service management, health checking, and graceful shutdown. However, critical issues prevent immediate production use: (1) placeholder UpgradeCode in WiX installer will break upgrades, (2) stopping service on tray exit creates poor UX, (3) granting SYSTEM full permissions to user directories is overly broad. The concurrent upload refactoring and security capability improvements are solid. Most service code is production-ready with good error handling and logging
- Pay close attention to
installer/Package.wxs(placeholder GUID),cmd/pinshare-tray/main.go(service lifecycle and permissions), and test the installer thoroughly on Windows 10 and 11
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| installer/Package.wxs | 2/5 | WiX installer configuration with placeholder UpgradeCode that must be replaced before production |
| cmd/pinshare-tray/main.go | 3/5 | Tray application with security concerns around SYSTEM permissions and service lifecycle management on exit |
| cmd/pinsharesvc/main.go | 4/5 | Service wrapper with proper Windows service integration and command-line interface |
| cmd/pinsharesvc/process.go | 4/5 | Process manager for IPFS and PinShare with proper cleanup, orphan detection, and graceful shutdown |
| cmd/pinsharesvc/service.go | 4/5 | Windows service handler with health checking and proper state management |
| cmd/pinsharesvc/config.go | 4/5 | Configuration management with secure encryption key generation and session marker support |
| internal/p2p/uploads.go | 4/5 | Refactored upload processing with concurrent file handling and improved security scanning logic |
| cmd/pinshare-tray/tray.go | 4/5 | System tray UI with service control, status monitoring, and health checking |
| cmd/pinsharesvc/service_control.go | 4/5 | Service installation and control with proper security descriptor modification for user access |
Sequence Diagram
sequenceDiagram
participant User
participant MSI as MSI Installer
participant WiX as WiX Package
participant PS as PowerShell Script
participant SvcExe as pinsharesvc.exe
participant WinSvc as Windows Service Manager
participant Tray as pinshare-tray.exe
participant Service as PinShare Service
participant IPFS as IPFS Daemon
participant Backend as PinShare Backend
User->>MSI: Run PinShare-Setup.msi
MSI->>WiX: Extract files to Program Files
WiX->>SvcExe: Execute install command
SvcExe->>WinSvc: Create service registration
WinSvc-->>SvcExe: Service created
SvcExe->>WinSvc: Set service DACL permissions
WinSvc-->>SvcExe: Permissions granted
SvcExe-->>WiX: Installation complete
alt START_SERVICE_NOW=1 OR SERVICE_AUTO_START=1
WiX->>PS: Execute start-service.ps1
PS->>WinSvc: Start-Service PinShareService
WinSvc->>Service: Launch service process
Service->>Service: Load config from session marker
Service->>IPFS: Start IPFS daemon
IPFS-->>Service: IPFS ready
Service->>Backend: Start PinShare backend
Backend-->>Service: Backend ready
Service-->>WinSvc: Service running
PS-->>WiX: Service started
end
alt START_TRAY_NOW=1
WiX->>Tray: Launch tray application
Tray->>Tray: Create user data directories
Tray->>Tray: Grant SYSTEM permissions (icacls)
Tray->>Tray: Write session marker
Tray->>WinSvc: Check service status
WinSvc-->>Tray: Service status
Tray->>Tray: Display tray icon with status
end
WiX-->>User: Installation complete
Note over User,Backend: Runtime: User interacts with tray
User->>Tray: Click tray menu
Tray->>WinSvc: Start/Stop/Restart service
WinSvc->>Service: Control signal
Service->>IPFS: Start/Stop daemon
Service->>Backend: Start/Stop backend
User->>Tray: Exit tray application
Tray->>WinSvc: Stop service
WinSvc->>Service: Stop signal
Service->>Backend: Graceful shutdown
Service->>IPFS: Graceful shutdown
Service-->>WinSvc: Service stopped
| <?define ProductVersion = "1.0.0" ?> | ||
| <?endif ?> | ||
| <?define Manufacturer = "PinShare Contributors" ?> | ||
| <?define UpgradeCode = "12345678-1234-1234-1234-123456789012" ?> |
There was a problem hiding this comment.
Placeholder UpgradeCode must be replaced with a production GUID. Generate using PowerShell or uuidgen
Comment on lines
+188
to
+193
| // Stop the service when tray exits | ||
| if err := stopService(); err != nil { | ||
| log.Printf("Failed to stop service on exit: %v", err) | ||
| } else { | ||
| log.Println("Service stopped") | ||
| } |
There was a problem hiding this comment.
Stopping the service when tray exits creates UX issues - closing the tray would stop the background service. Consider leaving service running or prompting user
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Author
There was a problem hiding this comment.
This was an intentional UX design choice. The rationale is that if the user closes the tray, they have no convenient way to stop the system service, and are likely expressing their intent to do so by closing the tray app as that's all they associate with the PinShare installation. This is a hard overrule.
| func grantSystemAccess(dir string) error { | ||
| // Use icacls to grant SYSTEM full control with inheritance | ||
| // /grant SYSTEM:(OI)(CI)F = Full control, Object Inherit, Container Inherit | ||
| cmd := exec.Command("icacls", dir, "/grant", "SYSTEM:(OI)(CI)F", "/T", "/Q") |
There was a problem hiding this comment.
Granting SYSTEM full control with inheritance on user data creates security risk. Consider using more restrictive permissions or separate data directories
Comment on lines
+301
to
+302
| titlePtr, _ := syscall.UTF16PtrFromString(title) | ||
| messagePtr, _ := syscall.UTF16PtrFromString(message) |
There was a problem hiding this comment.
Error from UTF16PtrFromString is ignored. Handle conversion errors to prevent potential nil pointer dereference
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.