[WSLC] Add wslc get cli session api#40141
[WSLC] Add wslc get cli session api#40141chemwolf6922 wants to merge 5 commits intofeature/wsl-for-appsfrom
Conversation
chemwolf6922
changed the title
…et-cli-session-api
There was a problem hiding this comment.
Pull request overview
Adds a new WSLC SDK entrypoint to obtain the default “CLI session” and centralizes the default CLI session naming logic so it can be shared by both the SDK and the wslc CLI.
Changes:
- Added
WslcGetCliSessionto the WSLC SDK (header + export + implementation) to open the default CLI session by name. - Introduced
wsl::windows::common::WSLCSessionDefaults(shared defaults forwslc-cli/wslc-cli-admin) and updated CLI/task/model/test code to use it. - Added a Windows test validating
WslcGetCliSessionbehavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/windows/WslcSdkTests.cpp | Adds GetCliSession SDK test and a release-only RAII wrapper for session refs. |
| test/windows/wslc/e2e/WSLCE2EHelpers.cpp | Switches default session name lookup to the new common defaults helper. |
| src/windows/WslcSDK/wslcsdk.h | Declares the new public WslcGetCliSession API. |
| src/windows/WslcSDK/wslcsdk.def | Exports WslcGetCliSession from the SDK DLL. |
| src/windows/WslcSDK/wslcsdk.cpp | Implements WslcGetCliSession by opening the default session by name. |
| src/windows/wslc/tasks/SessionTasks.cpp | Uses the common defaults helper for default session name + default-name checks. |
| src/windows/wslc/services/SessionModel.h | Removes default session naming helpers/constants from SessionOptions. |
| src/windows/wslc/services/SessionModel.cpp | Uses common defaults for display name and storage path selection. |
| src/windows/common/WslSecurity.h | Adds security::IsElevated() declaration. |
| src/windows/common/WslSecurity.cpp | Implements security::IsElevated() using the current access token. |
| src/windows/common/WSLCSessionDefaults.h | New common header encapsulating elevation-aware default session naming. |
| src/windows/common/CMakeLists.txt | Registers the new common header in the common target headers list. |
src/windows/common/WslSecurity.cpp
Outdated
| // IsTokenElevated checks if the integrity level is exactly HIGH. | ||
| // We must also check for local system because it is above HIGH. | ||
| // However, IsTokenLocalSystem() does not work correctly and fails. | ||
| // TODO: Add proper handling for system user callers. |
There was a problem hiding this comment.
I know this is pre-existing and not related to your PR @chemwolf6922, but is the fix in IsTokenElevated() as simple as changing the == to >= for proper handling of system user callers, @benhillis ?
Now that IsElevated is moved to the shared WslSecurity, this bug/limitation could have more exposure
There was a problem hiding this comment.
I modifed the IsElevated function to use >=. And renamed it to IsElevatedOrAbove to aboid confusion with the IsTokenElevated fucntion.
I don't think we should modify the IsTokenElevated function though. As that affects other parts of the codebase.
src/windows/common/WslSecurity.cpp
Outdated
| // We must also check for local system because it is above HIGH. | ||
| // However, IsTokenLocalSystem() does not work correctly and fails. | ||
| // TODO: Add proper handling for system user callers. | ||
| return IsTokenElevated(token.get()); |
There was a problem hiding this comment.
Also, could the issue referenced above mean that a SYSTEM-context caller of WslcGetCliSession would get the wrong session name (wslc-cli instead of wslc-cli-admin), and so it could likely result in a 'not found'?
There was a problem hiding this comment.
If we assume the user runs cli commands in the same context as the get cli session call. They will always get the same session. No matter if it's the correct one. (local system will get wslc-cli).
For the dev loop scenario, this assumption is true.
|
Just curious about the context of the change, what're the scenarios that an app will need to access a cli session? |
Hi @yao-msft , Please refer to the build experience design doc. I'll try to send you internally. In short, this is for the dev loop where the developer builds their image in the cli session and pick it up directly when debugging. |
Summary of the Pull Request
PR Checklist
Detailed Description of the Pull Request / Additional comments
Validation Steps Performed
Add test: WslcSdkTests::GetCliSession