feat(x): login user independent socket path for windows service#469
feat(x): login user independent socket path for windows service#469
Conversation
There was a problem hiding this comment.
Review Summary
This PR introduces platform-specific implementations for DaemonSocketPath(). While the refactoring approach is sound, the Windows implementation has a critical issue with permissions that will prevent non-elevated processes from accessing the daemon socket.
Critical Issues
- Windows socket path requires elevated privileges: The Windows implementation unconditionally uses
C:\ProgramData, which requires administrative privileges for write access. This will cause permission errors for user-level applications.
Additional Issues
- Missing fallback mechanism: The Unix implementation gracefully falls back to temp directory if the cache directory is unavailable, but the Windows version lacks this safety mechanism.
| "path/filepath" | ||
| ) | ||
|
|
||
| func DaemonSocketPath() string { |
There was a problem hiding this comment.
Permission Issue: Non-elevated processes cannot access ProgramData
The Windows implementation unconditionally returns a path under C:\ProgramData, which requires elevated privileges for write access. When a user-level application calls DaemonSocketPath(), it will attempt to connect to a socket that it cannot create or access without elevation.
The Unix implementation uses os.UserCacheDir() with a fallback to os.TempDir(), providing a resilient approach that works in both user and system contexts. The Windows version should implement a similar strategy:
func DaemonSocketPath() string {
if dir, err := os.UserCacheDir(); err == nil {
return filepath.Join(dir, "docker-secrets-engine", "daemon.sock")
}
return filepath.Join(os.TempDir(), "docker-secrets-engine", "daemon.sock")
}Or, if a system-wide path is truly required for Windows service scenarios, consider detecting the execution context and choosing the appropriate location accordingly.
| func DaemonSocketPath() string { | ||
| base := os.Getenv("ProgramData") | ||
| if base == "" { | ||
| base = `C:\ProgramData` |
There was a problem hiding this comment.
Missing fallback mechanism
The Windows implementation hardcodes C:\ProgramData without any fallback or validation. If the ProgramData environment variable is not set and C:\ProgramData is inaccessible (non-standard Windows installations, restricted environments), the function returns a path that may be invalid or unusable.
The Unix version (defaults_unix.go) properly falls back to os.TempDir() if os.UserCacheDir() fails. The Windows implementation should follow the same pattern to ensure robustness across different Windows environments.
Windows services do not run as the login user. We the previous code the default socket path would become
We need an alternative socket location that does not depend on the login user and is accessible for non privileged processes.