feat: default to OS credstore for storing registry creds #1653

ayush-panta · 2025-12-30T19:39:38Z

Description of changes: This PR enables Finch to integrate with native OS credential stores (Keychain on macOS, Credential Manager on Windows) for container registry authentication.

How it works:

finch login/logout commands execute on host using native credential helpers
When running container commands, Finch creates a temporary credential socket between host and VM
The VM's docker-credential-finch helper connects to this socket to retrieve credentials
Socket is automatically cleaned up when the command completes

Dependencies:

Requires finch-daemon PR #382 for credential helper updates
Requires finch-core to be updated to point to finch-daemon commit

Testing done: Tested locally; works alongside changes to finch-daemon for standard workflows. Added E2E tests for macOS, which worked locally (see below). Must add tests for Windows.

% go test -v -ginkgo.focus="Native Credential Helper" -timeout=30m
=== RUN   TestVM
Running Suite: Finch Virtual Machine E2E Tests - ../finch/e2e/vm
==================================================================================================
Random Seed: 1767117182

Will run 4 of 225 specs
SS••••SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS

Ran 4 of 225 Specs in 453.266 seconds
SUCCESS! -- 4 Passed | 0 Failed | 0 Pending | 221 Skipped
--- PASS: TestVM (453.29s)
PASS
ok  	github.com/runfinch/finch/e2e/vm	454.011s

I've reviewed the guidance in CONTRIBUTING.md

License Acceptance

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Signed-off-by: ayush-panta <[email protected]>

pendo324 · 2025-12-30T22:16:11Z

pkg/config/nerdctl_config_applier.go

+		if credHelper == "finch" {
+			cmdArr = append(cmdArr, configureFinchCredHelperTemplate)
+			// Add as default in credStore to work with nerdctl command flow. 
+			// TODO: support multi-registry workflows. Will involve set registries as credStore vs credHelpers in config.json.


How much more work is this? It'd be nice to have this in the initial implementation so we don't have to think about what happens when user's update

I'm giving this some thought right now. I think it is a matter of making ecr-login behave like acr-login:

{ ... "credHelpers": { "dkr.acr.builder-tools.aws.dev": "acr-login", "dkr.ecr.public.aws....": "ecr-login", } ... }

docker-credential-acr-login has a --setup flag that allows it to populate the config file by itself, but docker-credential-ecr-login does not.

I think what I will do is make it so that if ecr-login is set as a credhelper in finch.yaml, we store it as a credHelper for generic ecr links

^ Going to make this a mini-feature after the project.

pendo324 · 2025-12-30T22:18:06Z

cmd/finch/cred_helper.go

+	case "darwin":
+		helperName = "docker-credential-osxkeychain"
+	case "windows":
+		helperName = "docker-credential-wincred.exe"


Are we expecting the user to install the credential helpers specifically in the finch path? Other tools just require the credential helpers to be in the PATH. This might not be expected.

The user does not have to install anything. I just symlinked the binaries that are packaged with finch-core to .finch/cred-helpers, and then depend on these for credhelper operations.

I did this because as it is right now, if someone configures ecr-login in finch.yaml (for example), docker-credential-ecr-login gets installed into ./finch/cred-helpers.

However, I can try to make it just check if helpers are in PATH while also creating the symlink.

pendo324 · 2025-12-30T22:19:41Z

cmd/finch/cred_socket.go

@@ -0,0 +1,113 @@
+//go:build darwin || windows


Can we create a new directory/package for all of the credential handling files? Namely: cred_socket.go and cred_helper.go?

Shubhranshu153 · 2025-12-31T00:54:52Z

cmd/finch/cred_helper.go

+		return "", fmt.Errorf("unsupported OS: %s", runtime.GOOS)
+	}
+
+	helperPath := filepath.Join(homeDir, ".finch", "cred-helpers", helperName)


This path neednot be in the home dir for example

/opt/homebrew/bin/docker-credential-osxkeychain

i would say we neednot handle packaging them at all. Just have the process in the docs.

to find the path depending on OS you can do a which or where

we should think a little more here whether to package it or not. Packaging it will potentially add maintenance so i am not inclined, but makes it easier for customers

@pendo324 what do you think?

Shubhranshu153 · 2025-12-31T00:57:29Z

cmd/finch/cred_helper.go

+	cmd := exec.Command(helperPath, action)
+
+	// Set input based on action
+	if action == "store" {


if else is not usually desirable in function, try to have single functionality per function

Makes it easier to test also.

Shubhranshu153 · 2025-12-31T00:58:34Z

cmd/finch/cred_helper.go

+		}
+		credJSON, err := json.Marshal(cred)
+		if err != nil {
+			return nil, fmt.Errorf("failed to marshal credentials: %w", err)


dont print the err here, it might leak creds.

Shubhranshu153 · 2025-12-31T00:58:58Z

cmd/finch/cred_helper.go

+
+	output, err := cmd.CombinedOutput()
+	if err != nil {
+		return nil, fmt.Errorf("credential helper failed: %w - %s", err, string(output))


same as above.

Shubhranshu153 · 2025-12-31T01:00:53Z

cmd/finch/cred_socket.go

+
+	// Break if already running
+	if cs.listener != nil {
+		return nil


need an error saying concurrent cred request not supported in finch mac and windows version.

Shubhranshu153 · 2025-12-31T01:02:13Z

cmd/finch/cred_socket.go

+		return nil
+	}
+
+	socketPath := filepath.Join(finchRootPath, "lima", "data", "finch", "sock", "creds.sock")


this should be part of makefile, similar to finch,sock

if socket not there we just throw an error.

Shubhranshu153 · 2025-12-31T01:05:22Z

cmd/finch/nerdctl.go

 	"load":      "Load an image from a tar archive or STDIN",
-	"login":     "Log in to a container registry",
-	"logout":    "Log out from a container registry",
+	// "login":     "Log in to a container registry",


nit: commented out code.

Shubhranshu153 · 2025-12-31T01:07:46Z

cmd/finch/nerdctl_remote.go

+	finchRootPath := filepath.Dir(filepath.Dir(execPath))
+
+	// Wrap nerdctl execution with credential socket
+	return withCredSocket(finchRootPath, func() error {


This will prevent any concurrent execution but thats fine.

I think its good to wrap it for functionalities we actually need creds, otherwise it might un-necessary hold it. This part is actually flimsy.

Also it can slow down the run of other commands.

Shubhranshu153 · 2025-12-31T01:15:41Z

cmd/finch/virtual_machine_init.go

+	return nil
+}
+
+func (iva *initVMAction) ensureNativeCredentialHelpers() error {


i would like to avoid having to manage the installation rather find it from the environment.

Shubhranshu153 · 2025-12-31T15:11:40Z

cmd/finch/cred_helper.go

+		return "", fmt.Errorf("unsupported OS: %s", runtime.GOOS)
+	}
+
+	helperPath := filepath.Join(homeDir, ".finch", "cred-helpers", helperName)


https://pkg.go.dev/os/exec -> this probably can be used to get the path.
Not sure it works for windows but we can try.

Shubhranshu153 · 2025-12-31T15:15:35Z

cmd/finch/cred_socket.go

+}
+
+// withCredSocket wraps command execution with credential socket lifecycle.
+func withCredSocket(finchRootPath string, fn func() error) error {


how will this handle detach case?
Wont it keep holding the console, Lets do a check for it

finch run -d alpine sleep 10

something like that.

Also lets verify the behavior if a image exists then what happens will the request still come?

Shubhranshu153 · 2025-12-31T15:30:39Z

finch.yaml.d/mac.yaml

+    script: |
+      # Create user-writable directory in /run for credential socket
+      mkdir -p /run/finch-user-sockets
+      chmod 777 /run/finch-user-sockets


this seems overly permissive.

Shubhranshu153 · 2025-12-31T15:37:22Z

cmd/finch/login_local.go

+	return loginWithNativeCredStore(serverAddress, username, password, cmd.OutOrStdout())
+}
+
+func loginWithNativeCredStore(serverAddress, username, password string, stdout io.Writer) error {


can we not use login package from nerdctl:
https://github.com/containerd/nerdctl/blob/53e7b272af14b075a5e8d7b95a5c2d862a1620f8/pkg/cmd/login/login.go#L44

we just use it.

Shubhranshu153 · 2025-12-31T15:41:32Z

cmd/finch/cred_socket.go

+	if !scanner.Scan() {
+		return
+	}
+	serverURL := strings.TrimSpace(scanner.Text())


serverURL probably needs some sanitization with regex to check its a valid server request, but probably docker cred helper also does it?

Shubhranshu153 · 2025-12-31T15:45:30Z

cmd/finch/cred_socket.go

+	defer cs.mu.Unlock()
+
+	// Break if already running
+	if cs.listener != nil {


not sure what it does, if there are separate request you wont see them across CLI calls. so this is not a valid check for concurrent connection.

You can probably do flock on the socket to check if its in use or not
"github.com/go-frs/flock"

This should handle crashes as it uses system calls.

Shubhranshu153 · 2025-12-31T15:50:25Z

e2e/vm/vm_darwin_test.go

 		testNonDefaultOptions(o, *e2e.Installed)
 		testSupportBundle(o)
 		testCredHelper(o, *e2e.Installed, *e2e.Registry)
+		testNativeCredHelper(o, *e2e.Installed)


windows also will need it?

Shubhranshu153 · 2025-12-31T15:51:42Z

cmd/finch/logout_local.go

@@ -0,0 +1,51 @@
+//go:build darwin || windows


the naming or is for linux i think, for vm we use remote as we are seeing from perspective of from inside the vm.

Shubhranshu153 · 2025-12-31T16:01:08Z

e2e/vm/cred_helper_native_test.go

+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+//go:build darwin || windows


Need test for:

docker login: This is the most direct. It triggers a store action to save new credentials into your helper/keychain.

docker push: Before uploading layers, Docker calls get to retrieve the registry token or password.

docker pull: Even for public images on Docker Hub, Docker often calls get to check for an account to avoid "Pull Rate Limits." For private registries, it is mandatory.

docker logout: Triggers the erase action to remove credentials from the store.

docker run: Implicitly pulls the image if it’s missing. Should check the detached mode (only run has a detached mode which is relevant for our case)

docker create: Same as run; pulls the image metadata and layers.

docker build / docker buildx: If the FROM line in your Dockerfile references a private image or a remote registry, the builder will request credentials to fetch that base image

Shubhranshu153 · 2025-12-31T16:03:10Z

cmd/finch/cred_socket.go

+	socketPath := filepath.Join(finchRootPath, "lima", "data", "finch", "sock", "creds.sock")
+	if err := os.MkdirAll(filepath.Dir(socketPath), 0750); err != nil {
+		return fmt.Errorf("failed to create socket directory: %w", err)
+	}


how will the credential behave if the command crashes. i think it will just release the conncetion the other end will get a 500 internal error but lets confirm its graceful enough and doesnt hold the connection. If it does we cannot run any further commands.

feat: default to OS credstore for storing registry creds

a291907

Signed-off-by: ayush-panta <[email protected]>

ayush-panta force-pushed the native-creds branch from 74be291 to a291907 Compare December 30, 2025 19:46

Merge branch 'main' into native-creds

a396935

pendo324 reviewed Dec 30, 2025

View reviewed changes

Shubhranshu153 reviewed Dec 31, 2025

View reviewed changes

feat: default to OS credstore for storing registry creds #1653

Are you sure you want to change the base?

feat: default to OS credstore for storing registry creds #1653

Uh oh!

Conversation

ayush-panta commented Dec 30, 2025

License Acceptance

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Shubhranshu153 Dec 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Shubhranshu153 Dec 31, 2025 •

edited

Loading