Skip to content

Fallback se:// URI generation when secrets engine is unreachable#448

Merged
saucow merged 2 commits intodocker:mainfrom
saucow:fix/windows-msix-secrets-fallback
Mar 19, 2026
Merged

Fallback se:// URI generation when secrets engine is unreachable#448
saucow merged 2 commits intodocker:mainfrom
saucow:fix/windows-msix-secrets-fallback

Conversation

@saucow
Copy link
Copy Markdown
Contributor

@saucow saucow commented Mar 18, 2026

What I did

When GetSecrets() fails (e.g. MSIX-sandboxed Claude Desktop on Windows cannot follow AF_UNIX reparse points to the WSL2 secrets engine socket), generate se:// URIs for all declared secrets instead of silently setting them to . Docker Desktop resolves se:// URIs at container runtime via named pipes, which are unaffected by MSIX restrictions.

Also log the GetSecrets() error instead of silently discarding it.

Related issue
#443 #424

When GetSecrets() fails (e.g. MSIX-sandboxed Claude Desktop on Windows
cannot follow AF_UNIX reparse points to the WSL2 secrets engine socket),
generate se:// URIs for all declared secrets instead of silently setting
them to <UNKNOWN>. Docker Desktop resolves se:// URIs at container runtime
via named pipes, which are unaffected by MSIX restrictions.

Also log the GetSecrets() error instead of silently discarding it.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@saucow saucow marked this pull request as ready for review March 18, 2026 17:53
@saucow saucow requested a review from a team as a code owner March 18, 2026 17:53
tuna-docker
tuna-docker previously approved these changes Mar 18, 2026
Copy link
Copy Markdown
Collaborator

@slimslenderslacks slimslenderslacks left a comment

Choose a reason for hiding this comment

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

What about the case where secrets can't be resolved, but there are also oauth secrets to resolve? I'll bet you're looking forward to refactoring this code more comprehensively.


// availableSecrets maps secret IDs to their values (used to check existence)
allSecrets, _ := secret.GetSecrets(ctx)
allSecrets, err := secret.GetSecrets(ctx)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What about passing availableSecrets into this function (possibly empty), and treating this as another fallback? Would that make this simpler?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's a good point, I opted for an approach which updates: BuildSecretsURIs to have two clear paths:

    1. GetSecrets() succeeds → buildVerifiedURIs() generates se:// URIs only for secrets that actually exist in the store (OAuth token checked first, then direct secret)
    1. GetSecrets() fails → buildFallbackURIs() generates se:// URIs for all declared secrets since we can't check existence (OAuth preferred when configured, matching normal priority

Refactored into separate functions for clarity:
- buildFallbackURIs: generates se:// URIs for all declared secrets when
  the secrets engine is unreachable (OAuth preferred when configured)
- buildVerifiedURIs: generates se:// URIs only for secrets that exist
  in the store (OAuth checked first, then direct secret)
- oauthMapping: shared helper for OAuth provider lookup

When GetSecrets() fails (e.g. MSIX sandbox on Windows), the fallback
generates URIs for everything and lets Docker Desktop resolve at runtime.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@saucow
Copy link
Copy Markdown
Contributor Author

saucow commented Mar 18, 2026

What about the case where secrets can't be resolved, but there are also oauth secrets to resolve?

@slimslenderslacks Good point, updated so: buildFallbackURIs() now checks for OAuth providers on each server. If a secret has an OAuth provider configured (e.g. GitHub), it generates se://docker/mcp/oauth/github instead of se://docker/mcp/github.personal_access_token. This matches the normal priority order so users who authenticated via Docker Desktop's OAuth flow get the right URI even when the secrets engine is unreachable

I'll bet you're looking forward to refactoring this code more comprehensively.

Yes it's definitely needed!

Copy link
Copy Markdown
Collaborator

@slimslenderslacks slimslenderslacks left a comment

Choose a reason for hiding this comment

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

lgtm

@saucow saucow merged commit 0c22732 into docker:main Mar 19, 2026
5 checks passed
saucow added a commit that referenced this pull request Mar 20, 2026
* Fallback se:// URI generation when secrets engine is unreachable

When GetSecrets() fails (e.g. MSIX-sandboxed Claude Desktop on Windows
cannot follow AF_UNIX reparse points to the WSL2 secrets engine socket),
generate se:// URIs for all declared secrets instead of silently setting
them to <UNKNOWN>. Docker Desktop resolves se:// URIs at container runtime
via named pipes, which are unaffected by MSIX restrictions.

Also log the GetSecrets() error instead of silently discarding it.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Address PR feedback: handle OAuth secrets in fallback path

Refactored into separate functions for clarity:
- buildFallbackURIs: generates se:// URIs for all declared secrets when
  the secrets engine is unreachable (OAuth preferred when configured)
- buildVerifiedURIs: generates se:// URIs only for secrets that exist
  in the store (OAuth checked first, then direct secret)
- oauthMapping: shared helper for OAuth provider lookup

When GetSecrets() fails (e.g. MSIX sandbox on Windows), the fallback
generates URIs for everything and lets Docker Desktop resolve at runtime.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.

3 participants