Skip to content

fix: fix buildkit www-authenticate challenges with depot registry#439

Merged
WitoDelnat merged 2 commits intomainfrom
fix-www-authenticate-challenge
Feb 25, 2026
Merged

fix: fix buildkit www-authenticate challenges with depot registry#439
WitoDelnat merged 2 commits intomainfrom
fix-www-authenticate-challenge

Conversation

@WitoDelnat
Copy link
Contributor

@WitoDelnat WitoDelnat commented Feb 25, 2026

There is a correct implementation when using the legacy moby driver, but with Buildkit it was incorrectly handled in the now patched files. Previously no such custom auth logic existed. This bug was never caught because for the legacy registry, our proxy never returns a www-authenticate challenge since it hi-jacks authentication itself and mints a token before redirecting to upstream registry.

@WitoDelnat WitoDelnat changed the title fix: fix www-authenticate challenges to use buildkit with depot registry fix: fix buildkit www-authenticate challenges with depot registry Feb 25, 2026
@WitoDelnat WitoDelnat force-pushed the fix-www-authenticate-challenge branch 2 times, most recently from f1bd206 to 7657083 Compare February 25, 2026 13:58
@linear
Copy link

linear bot commented Feb 25, 2026

@cursor

This comment has been minimized.

@WitoDelnat
Copy link
Contributor Author

@cursor push 2fd1823

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is ON. A Cloud Agent has been kicked off to fix the reported issue.

}, nil
}
return nil, nil
}
Copy link

Choose a reason for hiding this comment

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

Credential lookup logic duplicated across two methods

Low Severity

The new findCredentials method duplicates the base64-decoding and colon-splitting credential lookup logic already present in the existing Credentials method on AuthProvider. If the credential format or validation ever changes, both places need to be updated in sync. Credentials could be refactored to call findCredentials internally and convert the result, eliminating the duplicated parsing logic.

Additional Locations (1)

Fix in Cursor Fix in Web

@cursor
Copy link

cursor bot commented Feb 25, 2026

Bugbot Autofix prepared fixes for 1 of the 1 bugs found in the latest run.

  • ✅ Fixed: Credential lookup logic duplicated across two methods
    • Refactored the Credentials method to call findCredentials internally and convert the result, eliminating the duplicated base64-decoding and colon-splitting logic.

Create PR

Or push these changes by commenting:

@cursor push 6d6175bdae
Preview (6d6175bdae)
diff --git a/pkg/registry/auth.go b/pkg/registry/auth.go
--- a/pkg/registry/auth.go
+++ b/pkg/registry/auth.go
@@ -60,24 +60,16 @@
 }
 
 func (a *AuthProvider) Credentials(ctx context.Context, req *auth.CredentialsRequest) (*auth.CredentialsResponse, error) {
-	for _, c := range a.credentials {
-		if c.Host == req.Host {
-			decodedAuth, err := base64.StdEncoding.DecodeString(c.Token)
-			if err != nil {
-				return nil, err
-			}
-
-			usernamePassword := strings.SplitN(string(decodedAuth), ":", 2)
-			if len(usernamePassword) != 2 {
-				return nil, fmt.Errorf("invalid auth string")
-			}
-
-			return &auth.CredentialsResponse{
-				Username: usernamePassword[0],
-				Secret:   usernamePassword[1],
-			}, nil
-		}
+	creds, err := a.findCredentials(req.Host)
+	if err != nil {
+		return nil, err
 	}
+	if creds != nil {
+		return &auth.CredentialsResponse{
+			Username: creds.Username,
+			Secret:   creds.Password,
+		}, nil
+	}
 
 	return a.inner.Credentials(ctx, req)
 }

wito and others added 2 commits February 25, 2026 16:00
…ssing ACR status code

- Extract shared fetchTokenWithFallback helper function to eliminate code
  duplication between AuthProvider.FetchToken and DepotAuthProvider.FetchToken
- Update findCredentials to return error instead of silently skipping malformed
  credentials, making error handling consistent with Credentials method
- Update OAuth POST fallback to handle all ErrUnexpectedStatus codes, not just
  405/404/401, which fixes ACR compatibility (ACR returns 400)

Applied via @cursor push command
@WitoDelnat WitoDelnat force-pushed the fix-www-authenticate-challenge branch from 0631ebd to 337bfa3 Compare February 25, 2026 15:00
@WitoDelnat WitoDelnat enabled auto-merge February 25, 2026 15:02
@WitoDelnat WitoDelnat merged commit 62f38fe into main Feb 25, 2026
8 checks passed
@WitoDelnat WitoDelnat deleted the fix-www-authenticate-challenge branch February 25, 2026 15:02
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