fix: remove os.Chdir from aspire server InitializeAsync#7362
Open
fix: remove os.Chdir from aspire server InitializeAsync#7362
Conversation
Replace global os.Chdir(rootPath) with explicit path passing to avoid mutating process-wide working directory. This is safer for concurrent operations and eliminates ambient authority. Fixes #3288 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
spboyer
requested review from
JeffreyCA,
hemarina,
rajeshkamal5050,
tg-msft,
vhvb1989,
wbreza and
weikanglim
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Removes process-wide working-directory mutation from the VS RPC server initialization to eliminate ambient authority and reduce concurrency hazards for multi-session scenarios.
Changes:
- Removed
os.Chdir(rootPath)(and related TODO) fromserverService.InitializeAsync. - Removed now-unused
osimport fromserver_service.go.
Comments suppressed due to low confidence (1)
cli/azd/internal/vsrpc/server_service.go:39
- With os.Chdir(rootPath) removed, InitializeAsync no longer validates that rootPath exists/is a directory. That means an empty/invalid rootPath can successfully create a session and only fail later (or cause commands to run under the server process’ current working directory when GlobalCommandOptions.Cwd is set). Consider explicitly validating rootPath here (non-empty + os.Stat + IsDir) and returning a contextual error if it’s invalid.
return nil, err
}
session.rootPath = rootPath
session.rootContainer = s.server.rootContainer
Collaborator
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
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
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.
Summary
Fixes #3288
Removes the
os.Chdir(rootPath)call from aspire server'sInitializeAsyncinserver_service.go. This was an ambient authority pattern that mutated process-wide working directory, which is unsafe for concurrent operations.Changes
server_service.go— Removedos.Chdir(rootPath)and its TODO comment. Removed unusedosimport.Why This Is Safe
rootPathis already passed explicitly everywhere it's needed:CwdinGlobalCommandOptions(deploy/provision)HostProjectPathinRequestContext(debug, aspire, environment services)internal/vsrpc/usesos.Getwd()or relies on process working directoryTesting
go test ./internal/vsrpc/... -short -count=1