Skip to content

Comments

feat(x): login user independent socket path for windows service#469

Merged
joe0BAB merged 1 commit intomainfrom
x/socket-path
Feb 24, 2026
Merged

feat(x): login user independent socket path for windows service#469
joe0BAB merged 1 commit intomainfrom
x/socket-path

Conversation

@joe0BAB
Copy link
Collaborator

@joe0BAB joe0BAB commented Feb 23, 2026

Windows services do not run as the login user. We the previous code the default socket path would become

C:\WINDOWS\system32\config\systemprofile\AppData\Local\docker-secrets-engine\daemon.sock

We need an alternative socket location that does not depend on the login user and is accessible for non privileged processes.

@joe0BAB joe0BAB self-assigned this Feb 23, 2026
@joe0BAB joe0BAB requested a review from Benehiko February 23, 2026 14:55
@joe0BAB joe0BAB marked this pull request as ready for review February 23, 2026 14:56
Copy link

@docker-agent docker-agent bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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`
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@joe0BAB joe0BAB merged commit c25a9c4 into main Feb 24, 2026
22 checks passed
@joe0BAB joe0BAB deleted the x/socket-path branch February 24, 2026 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants