Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates Windows storage path handling to use a shared, service-accessible location and improves developer documentation by adding/clarifying build instructions and linking them from the main README.
Changes:
- Updated Windows app data base directory logic to use the Public profile location for service compatibility.
- Added a “Developers” section to
README.mdlinking to the developer-focused README. - Clarified Windows dev build steps in
README-dev.mdby adding the shared library build step.
Reviewed changes
Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| lib/core/utils/storage_utils.dart | Changes Windows app data base path to a shared directory to align with Windows service access patterns. |
| README.md | Adds a developer entry point linking to the developer documentation. |
| README-dev.md | Expands/reorders Windows build instructions to include the shared library build step. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| //the specific user profile. The Windows service will create a subdirectory | ||
| // called "data" within this directory to store its own data. | ||
| final appDataPath = Platform.environment['PUBLIC']; | ||
| final appDir = Directory("$appDataPath/Lantern"); |
There was a problem hiding this comment.
Platform.environment['PUBLIC'] can be null/empty in some Windows execution contexts. If that happens, this will create/return a relative directory like null/Lantern (or /Lantern), which is not the intended shared app-data location. Add validation and a deterministic fallback (or throw a clear error) before constructing the directory path.
| final appDir = Directory("$appDataPath/Lantern"); | |
| if (appDataPath == null || appDataPath.trim().isEmpty) { | |
| throw StateError( | |
| 'The PUBLIC environment variable is not set or is empty; ' | |
| 'cannot determine the shared Windows app-data directory for Lantern.', | |
| ); | |
| } | |
| final appDir = Directory(p.join(appDataPath, 'Lantern')); |
| final appDir = Directory("$appDataPath/Lantern"); | ||
| if (!appDir.existsSync()) { |
There was a problem hiding this comment.
This constructs a Windows path using string interpolation and a hard-coded "/" separator even though this file already uses package:path elsewhere. Prefer p.join(appDataPath, 'Lantern') (and similar joins) to avoid subtle path formatting issues and keep path handling consistent.
| // called "data" within this directory to store its own data. | ||
| final appDataPath = Platform.environment['PUBLIC']; | ||
| final appDir = Directory("$appDataPath/Lantern"); | ||
| if (!appDir.existsSync()) { |
There was a problem hiding this comment.
This async method uses existsSync() (synchronous filesystem I/O) and then awaits create(). For consistency and to avoid blocking the isolate on slow disk/networked profiles, use the async await appDir.exists() pattern like the rest of this file.
| if (!appDir.existsSync()) { | |
| if (!await appDir.exists()) { |
| return dir; | ||
| // On Windows, we want to store app data in C:\Users\Public\Lantern to | ||
| // ensure that the Windows service can access it without needing to know | ||
| //the specific user profile. The Windows service will create a subdirectory |
There was a problem hiding this comment.
Missing space after the comment marker makes this sentence harder to read; please change //the specific user profile to // the specific user profile.
| //the specific user profile. The Windows service will create a subdirectory | |
| // the specific user profile. The Windows service will create a subdirectory |
| final appDataPath = Platform.environment['PUBLIC']; | ||
| final appDir = Directory("$appDataPath/Lantern"); | ||
| if (!appDir.existsSync()) { | ||
| await appDir.create(recursive: true); | ||
| } | ||
|
|
||
| final appDir = Directory(p.join(appData, "Lantern")); | ||
| if (!await appDir.exists()) await appDir.create(recursive: true); | ||
| return appDir; |
There was a problem hiding this comment.
getWindowsAppDataDirectory now points to C:\Users\Public\Lantern, which on typical Windows installations is readable by all local users, and this path is used by getAppDirectory/LocalStorageService to store the ObjectBox DB containing legacyToken, token, oAuthToken, accessToken, and other sensitive user/account data. Any low-privilege local user on the same machine can read the database files under this public directory and hijack accounts or extract PII. Instead, store this data under a per-user app data directory or ensure the shared directory has restricted ACLs so only the service account and the intended user can read it.
* Added windows build step * Added link to dev README * Fixed windows paths
1 participant
This pull request introduces improvements for developers and updates how application data is stored on Windows. The most significant change is to the Windows app data storage location, ensuring compatibility with the Windows service. Additionally, developer documentation has been clarified and expanded to help new contributors.
Developer Documentation Improvements:
README.mdwith a link to the new developer README for build instructions and contribution guidelines.README-outline.mdtoREADME-dev.mdand updated the Windows build instructions to clarify the shared library build step and re-ordered steps for clarity.Windows App Data Storage Update:
getWindowsAppDataDirectorymethod instorage_utils.dartto store app data inC:\Users\Public\Lanterninstead of user-specific directories, ensuring the Windows service can access shared data without needing user profile information.