fix(backend): use the session auth token for non-Docker launch and resume (#3007)
Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com> Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
@@ -2,11 +2,13 @@ package lifecycle
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"testing"
|
||||
|
||||
"github.com/kandev/kandev/internal/common/logger"
|
||||
"github.com/kandev/kandev/internal/secrets"
|
||||
"github.com/kandev/kandev/internal/task/models"
|
||||
)
|
||||
|
||||
// inMemorySecretStore implements secrets.SecretStore for testing.
|
||||
@@ -296,3 +298,116 @@ func TestRevealContainerControlAuthToken(t *testing.T) {
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// TestResolveLaunchAuthToken is the regression test for the SSH resume
|
||||
// preflight failing with "missing agentctl auth token": #2843 made every
|
||||
// launch/resume consume the Docker-only container control token, so a
|
||||
// non-Docker executor (SSH) with a real session auth secret got "" instead.
|
||||
func TestResolveLaunchAuthToken(t *testing.T) {
|
||||
log, _ := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json"})
|
||||
|
||||
t.Run("ssh with a session auth secret returns that token", func(t *testing.T) {
|
||||
store := newInMemorySecretStore()
|
||||
store.store["session-auth"] = &secrets.SecretWithValue{Value: "the-ssh-token"}
|
||||
m := &Manager{logger: log, secretStore: store}
|
||||
|
||||
req := &LaunchRequest{ExecutorType: string(models.ExecutorTypeSSH)}
|
||||
metadata := map[string]interface{}{MetadataKeyAuthTokenSecret: "session-auth"}
|
||||
|
||||
got, err := m.resolveLaunchAuthToken(context.Background(), req, metadata)
|
||||
if err != nil {
|
||||
t.Fatalf("resolveLaunchAuthToken() error = %v", err)
|
||||
}
|
||||
if got != "the-ssh-token" {
|
||||
t.Fatalf("resolveLaunchAuthToken() = %q, want %q", got, "the-ssh-token")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("ssh reports a session secret reveal failure", func(t *testing.T) {
|
||||
store := newInMemorySecretStore()
|
||||
revealErr := errors.New("secret backend unavailable")
|
||||
store.revealErr = revealErr
|
||||
store.store["session-auth"] = &secrets.SecretWithValue{Value: "the-ssh-token"}
|
||||
m := &Manager{logger: log, secretStore: store}
|
||||
|
||||
req := &LaunchRequest{ExecutorType: string(models.ExecutorTypeSSH)}
|
||||
metadata := map[string]interface{}{MetadataKeyAuthTokenSecret: "session-auth"}
|
||||
|
||||
got, err := m.resolveLaunchAuthToken(context.Background(), req, metadata)
|
||||
if !errors.Is(err, revealErr) {
|
||||
t.Fatalf("resolveLaunchAuthToken() error = %v, want %v", err, revealErr)
|
||||
}
|
||||
if got != "" {
|
||||
t.Fatalf("resolveLaunchAuthToken() = %q, want empty", got)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("ssh with no secret anywhere returns empty and resume still rejects it", func(t *testing.T) {
|
||||
store := newInMemorySecretStore()
|
||||
m := &Manager{logger: log, secretStore: store}
|
||||
|
||||
req := &LaunchRequest{ExecutorType: string(models.ExecutorTypeSSH)}
|
||||
|
||||
got, err := m.resolveLaunchAuthToken(context.Background(), req, map[string]interface{}{})
|
||||
if err != nil {
|
||||
t.Fatalf("resolveLaunchAuthToken() error = %v", err)
|
||||
}
|
||||
if got != "" {
|
||||
t.Fatalf("resolveLaunchAuthToken() = %q, want empty", got)
|
||||
}
|
||||
if err := requireSSHAgentctlAuthToken(got); err == nil {
|
||||
t.Fatal("requireSSHAgentctlAuthToken() accepted an empty token")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("docker with workspace reuse still prefers the container control token", func(t *testing.T) {
|
||||
store := newInMemorySecretStore()
|
||||
store.store["control-secret"] = &secrets.SecretWithValue{Value: "environment-control-token"}
|
||||
m := &Manager{logger: log, secretStore: store}
|
||||
|
||||
req := &LaunchRequest{ExecutorType: string(models.ExecutorTypeLocalDocker), WorkspaceReuseRequired: true}
|
||||
metadata := map[string]interface{}{MetadataKeyContainerControlAuthSecret: "control-secret"}
|
||||
|
||||
got, err := m.resolveLaunchAuthToken(context.Background(), req, metadata)
|
||||
if err != nil {
|
||||
t.Fatalf("resolveLaunchAuthToken() error = %v", err)
|
||||
}
|
||||
if got != "environment-control-token" {
|
||||
t.Fatalf("resolveLaunchAuthToken() = %q, want %q", got, "environment-control-token")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("docker with workspace reuse falls back to the session token when no control handle exists", func(t *testing.T) {
|
||||
store := newInMemorySecretStore()
|
||||
store.store["session-auth"] = &secrets.SecretWithValue{Value: "legacy-session-token"}
|
||||
m := &Manager{logger: log, secretStore: store}
|
||||
|
||||
req := &LaunchRequest{ExecutorType: string(models.ExecutorTypeRemoteDocker), WorkspaceReuseRequired: true}
|
||||
metadata := map[string]interface{}{MetadataKeyAuthTokenSecret: "session-auth"}
|
||||
|
||||
got, err := m.resolveLaunchAuthToken(context.Background(), req, metadata)
|
||||
if err != nil {
|
||||
t.Fatalf("resolveLaunchAuthToken() error = %v", err)
|
||||
}
|
||||
if got != "legacy-session-token" {
|
||||
t.Fatalf("resolveLaunchAuthToken() = %q, want %q", got, "legacy-session-token")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("docker without workspace reuse returns empty when no control handle exists", func(t *testing.T) {
|
||||
store := newInMemorySecretStore()
|
||||
store.store["session-auth"] = &secrets.SecretWithValue{Value: "legacy-session-token"}
|
||||
m := &Manager{logger: log, secretStore: store}
|
||||
|
||||
req := &LaunchRequest{ExecutorType: string(models.ExecutorTypeLocalDocker), WorkspaceReuseRequired: false}
|
||||
metadata := map[string]interface{}{MetadataKeyAuthTokenSecret: "session-auth"}
|
||||
|
||||
got, err := m.resolveLaunchAuthToken(context.Background(), req, metadata)
|
||||
if err != nil {
|
||||
t.Fatalf("resolveLaunchAuthToken() error = %v", err)
|
||||
}
|
||||
if got != "" {
|
||||
t.Fatalf("resolveLaunchAuthToken() = %q, want empty", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -732,7 +732,7 @@ func (m *Manager) prepareExecutionCreateRequest(
|
||||
autoApproveOverride = boolPtr(profileInfo.AutoApprove)
|
||||
}
|
||||
authToken := m.revealRuntimeSecret(ctx, info.Metadata, MetadataKeyAuthTokenSecret)
|
||||
if info.ExecutorType == string(models.ExecutorTypeLocalDocker) || info.ExecutorType == string(models.ExecutorTypeRemoteDocker) {
|
||||
if isDockerExecutorType(info.ExecutorType) {
|
||||
controlToken, err := m.revealContainerControlAuthToken(ctx, info.Metadata, getMetadataString(info.Metadata, MetadataKeyContainerID) != "")
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("resolve container control token: %w", err)
|
||||
@@ -1022,20 +1022,29 @@ func (m *Manager) persistRuntimeSecret(
|
||||
zap.String("metadata_key", metadataKey))
|
||||
}
|
||||
|
||||
func (m *Manager) revealRuntimeSecret(ctx context.Context, metadata map[string]interface{}, metadataKey string) string {
|
||||
if m.secretStore == nil {
|
||||
return ""
|
||||
}
|
||||
// revealRuntimeSecretValue reveals a configured runtime secret and preserves
|
||||
// storage errors for callers that need to report a failed launch.
|
||||
func (m *Manager) revealRuntimeSecretValue(ctx context.Context, metadata map[string]interface{}, metadataKey string) (string, error) {
|
||||
secretID := getMetadataString(metadata, metadataKey)
|
||||
if secretID == "" {
|
||||
return ""
|
||||
return "", nil
|
||||
}
|
||||
if m.secretStore == nil {
|
||||
return "", errors.New("runtime secret store is unavailable")
|
||||
}
|
||||
value, err := revealGlobalSecret(ctx, m.secretStore, secretID)
|
||||
if err != nil {
|
||||
return "", fmt.Errorf("reveal runtime secret %q: %w", metadataKey, err)
|
||||
}
|
||||
return value, nil
|
||||
}
|
||||
|
||||
func (m *Manager) revealRuntimeSecret(ctx context.Context, metadata map[string]interface{}, metadataKey string) string {
|
||||
value, err := m.revealRuntimeSecretValue(ctx, metadata, metadataKey)
|
||||
if err != nil {
|
||||
m.logger.Warn("failed to reveal runtime secret",
|
||||
zap.String("metadata_key", metadataKey),
|
||||
zap.Error(err))
|
||||
return ""
|
||||
}
|
||||
return value
|
||||
}
|
||||
|
||||
@@ -858,11 +858,9 @@ func (m *Manager) launchBuildExecutorRequest(ctx context.Context, executionID st
|
||||
metadata[MetadataKeyContributionDestinations] = contributionDestinations
|
||||
}
|
||||
|
||||
allowLegacyContainerControlFallback := reqWithWorktree.WorkspaceReuseRequired &&
|
||||
(reqWithWorktree.ExecutorType == string(models.ExecutorTypeLocalDocker) || reqWithWorktree.ExecutorType == string(models.ExecutorTypeRemoteDocker))
|
||||
containerControlAuthToken, err := m.revealContainerControlAuthToken(ctx, metadata, allowLegacyContainerControlFallback)
|
||||
launchAuthToken, err := m.resolveLaunchAuthToken(ctx, reqWithWorktree, metadata)
|
||||
if err != nil {
|
||||
return nil, nil, nil, fmt.Errorf("resolve container control token: %w", err)
|
||||
return nil, nil, nil, fmt.Errorf("resolve launch auth token: %w", err)
|
||||
}
|
||||
|
||||
var autoApproveOverride *bool
|
||||
@@ -893,7 +891,7 @@ func (m *Manager) launchBuildExecutorRequest(ctx context.Context, executionID st
|
||||
McpMode: reqWithWorktree.McpMode,
|
||||
McpProviders: reqWithWorktree.McpProviders,
|
||||
McpProfile: reqWithWorktree.McpProfile,
|
||||
AuthToken: containerControlAuthToken,
|
||||
AuthToken: launchAuthToken,
|
||||
BootstrapNonce: m.revealRuntimeSecret(ctx, metadata, MetadataKeyBootstrapNonceSecret),
|
||||
AgentctlStartupConfig: m.agentctlStartupConfig,
|
||||
OnProgress: onProgress,
|
||||
@@ -915,6 +913,23 @@ func (m *Manager) launchBuildExecutorRequest(ctx context.Context, executionID st
|
||||
return execReq, execInstance, rt, nil
|
||||
}
|
||||
|
||||
func isDockerExecutorType(executorType string) bool {
|
||||
return executorType == string(models.ExecutorTypeLocalDocker) || executorType == string(models.ExecutorTypeRemoteDocker)
|
||||
}
|
||||
|
||||
// resolveLaunchAuthToken returns the agentctl token a launch/resume hands the
|
||||
// backend. Docker uses the environment-scoped container control token (#2843)
|
||||
// so a sibling session can authenticate to a shared container, falling back
|
||||
// to the session token only when workspace reuse is required and no
|
||||
// container-control secret exists yet. Every other executor uses its own
|
||||
// session handshake token directly, which SSH resume requires.
|
||||
func (m *Manager) resolveLaunchAuthToken(ctx context.Context, req *LaunchRequest, metadata map[string]interface{}) (string, error) {
|
||||
if isDockerExecutorType(req.ExecutorType) {
|
||||
return m.revealContainerControlAuthToken(ctx, metadata, req.WorkspaceReuseRequired)
|
||||
}
|
||||
return m.revealRuntimeSecretValue(ctx, metadata, MetadataKeyAuthTokenSecret)
|
||||
}
|
||||
|
||||
func resumeRemoteInstancePreflight(ctx context.Context, rt ExecutorBackend, req *ExecutorCreateRequest) error {
|
||||
resumer, ok := rt.(RemoteSessionResumer)
|
||||
if !ok {
|
||||
|
||||
Reference in New Issue
Block a user