fix: preserve service identity and resume credentials (#2121)
This commit is contained in:
@@ -4,7 +4,6 @@ import (
|
||||
"fmt"
|
||||
"os"
|
||||
"os/exec"
|
||||
"os/user"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"strings"
|
||||
@@ -13,6 +12,7 @@ import (
|
||||
type serviceArgs struct {
|
||||
Action string
|
||||
System bool
|
||||
RunAs string
|
||||
Port int
|
||||
HomeDir string
|
||||
NoBootStart bool
|
||||
@@ -41,7 +41,7 @@ const (
|
||||
const serviceHelp = `kandev service — install kandev as an OS-managed service
|
||||
|
||||
Usage:
|
||||
kandev service install [--system] [--port <port>] [--home-dir <path>] [--no-boot-start]
|
||||
kandev service install [--system] [--run-as <user>] [--port <port>] [--home-dir <path>] [--no-boot-start]
|
||||
kandev service uninstall [--system]
|
||||
kandev service start|stop|restart|status [--system]
|
||||
kandev service logs [-f] [--system]
|
||||
@@ -101,6 +101,9 @@ func parseServiceArgs(argv []string) (serviceArgs, error) {
|
||||
}
|
||||
i = next
|
||||
}
|
||||
if out.RunAs != "" && (action != actionInstall || !out.System) {
|
||||
return out, ParseError{Message: "--run-as is valid only for kandev service install --system"}
|
||||
}
|
||||
if action == actionSelfUpdate && !out.ShowHelp && out.Intent == "" {
|
||||
return out, ParseError{Message: "kandev service self-update requires --intent <path>"}
|
||||
}
|
||||
@@ -124,6 +127,24 @@ func parseServiceFlag(argv []string, i int, action string, out *serviceArgs) (in
|
||||
out.ShowHelp = true
|
||||
case arg == "--system":
|
||||
out.System = true
|
||||
case arg == "--run-as":
|
||||
if action != actionInstall {
|
||||
return i, ParseError{Message: "--run-as is valid only for kandev service install --system"}
|
||||
}
|
||||
value, err := takeValue(argv, i, "--run-as")
|
||||
if err != nil {
|
||||
return i, err
|
||||
}
|
||||
out.RunAs = value
|
||||
return i + 1, nil
|
||||
case strings.HasPrefix(arg, "--run-as="):
|
||||
if action != actionInstall {
|
||||
return i, ParseError{Message: "--run-as is valid only for kandev service install --system"}
|
||||
}
|
||||
out.RunAs = strings.TrimPrefix(arg, "--run-as=")
|
||||
if out.RunAs == "" {
|
||||
return i, ParseError{Message: "--run-as requires a value"}
|
||||
}
|
||||
case arg == "--no-boot-start":
|
||||
out.NoBootStart = true
|
||||
case arg == "-f" || arg == "--follow":
|
||||
@@ -232,7 +253,11 @@ func installSystemd(args serviceArgs, build BuildInfo, unitPath string) int {
|
||||
homeDir := serviceHomeDir(args)
|
||||
logDir := filepath.Join(homeDir, "logs")
|
||||
bundleDir := serviceBundleDir(self)
|
||||
systemUser := serviceUser(args.System)
|
||||
systemUser, err := resolveAndValidateSystemIdentity(args, unitPath, nativeServiceManagerSystemd, homeDir)
|
||||
if err != nil {
|
||||
fmt.Fprintln(os.Stderr, "[kandev] "+err.Error())
|
||||
return 1
|
||||
}
|
||||
input := nativeServiceUnitInput{
|
||||
Executable: self,
|
||||
HomeDir: homeDir,
|
||||
@@ -341,7 +366,11 @@ func installLaunchd(args serviceArgs, build BuildInfo, plistPath, target, domain
|
||||
homeDir := serviceHomeDir(args)
|
||||
logDir := filepath.Join(homeDir, "logs")
|
||||
bundleDir := serviceBundleDir(self)
|
||||
systemUser := serviceUser(args.System)
|
||||
systemUser, err := resolveAndValidateSystemIdentity(args, plistPath, nativeServiceManagerLaunchd, homeDir)
|
||||
if err != nil {
|
||||
fmt.Fprintln(os.Stderr, "[kandev] "+err.Error())
|
||||
return 1
|
||||
}
|
||||
input := nativeServiceUnitInput{
|
||||
Executable: self,
|
||||
HomeDir: homeDir,
|
||||
@@ -721,20 +750,6 @@ func serviceHomeDir(args serviceArgs) string {
|
||||
return resolveHomeDir()
|
||||
}
|
||||
|
||||
func serviceUser(system bool) string {
|
||||
if !system {
|
||||
return ""
|
||||
}
|
||||
if sudo := os.Getenv("SUDO_USER"); sudo != "" && sudo != "root" {
|
||||
return sudo
|
||||
}
|
||||
u, err := user.Current()
|
||||
if err != nil {
|
||||
return ""
|
||||
}
|
||||
return u.Username
|
||||
}
|
||||
|
||||
func printServiceConfig(args serviceArgs) {
|
||||
fmt.Println("manager:", runtime.GOOS)
|
||||
fmt.Println("mode:", map[bool]string{true: "system", false: "user"}[args.System])
|
||||
|
||||
@@ -0,0 +1,201 @@
|
||||
package launcher
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/xml"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"os"
|
||||
"strings"
|
||||
)
|
||||
|
||||
const rootServiceUser = "root"
|
||||
|
||||
func resolveSystemServiceUser(args serviceArgs, servicePath, manager string) (string, error) {
|
||||
if !args.System {
|
||||
return "", nil
|
||||
}
|
||||
if args.RunAs != "" {
|
||||
if err := validateSystemServiceUser(args.RunAs); err != nil {
|
||||
return "", err
|
||||
}
|
||||
return args.RunAs, nil
|
||||
}
|
||||
|
||||
userName, found, err := readManagedSystemServiceUser(servicePath, manager)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
if found {
|
||||
if err := validateSystemServiceUser(userName); err != nil {
|
||||
return "", fmt.Errorf("preserve service account %q from %s: %w", userName, servicePath, err)
|
||||
}
|
||||
return userName, nil
|
||||
}
|
||||
|
||||
if sudoUser := strings.TrimSpace(os.Getenv("SUDO_USER")); sudoUser != "" && sudoUser != rootServiceUser {
|
||||
if err := validateSystemServiceUser(sudoUser); err != nil {
|
||||
return "", err
|
||||
}
|
||||
return sudoUser, nil
|
||||
}
|
||||
return "", fmt.Errorf(
|
||||
"cannot infer the system service account; run `kandev service install --system --run-as <user>` (use --run-as " + rootServiceUser + " to choose root explicitly)",
|
||||
)
|
||||
}
|
||||
|
||||
func resolveAndValidateSystemIdentity(args serviceArgs, servicePath, manager, homeDir string) (string, error) {
|
||||
systemUser, err := resolveSystemServiceUser(args, servicePath, manager)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
if err := validateSystemServiceHomeOwner(homeDir, systemUser); err != nil {
|
||||
return "", err
|
||||
}
|
||||
return systemUser, nil
|
||||
}
|
||||
|
||||
func validateSystemServiceUser(userName string) error {
|
||||
trimmed := strings.TrimSpace(userName)
|
||||
if trimmed == "" || trimmed != userName || strings.ContainsAny(userName, " \t\r\n") {
|
||||
return errors.New("system service account must be a non-empty username")
|
||||
}
|
||||
if _, _, err := lookupNativeServiceOwner(userName); err != nil {
|
||||
return fmt.Errorf("resolve system service user %q: %w", userName, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func readManagedSystemServiceUser(servicePath, manager string) (string, bool, error) {
|
||||
info, err := os.Lstat(servicePath)
|
||||
if errors.Is(err, os.ErrNotExist) {
|
||||
return "", false, nil
|
||||
}
|
||||
if err != nil {
|
||||
return "", false, fmt.Errorf("inspect existing service definition %q: %w", servicePath, err)
|
||||
}
|
||||
if info.Mode()&os.ModeSymlink != 0 {
|
||||
return "", false, fmt.Errorf("managed service definition %q must not be a symlink", servicePath)
|
||||
}
|
||||
if !info.Mode().IsRegular() {
|
||||
return "", false, fmt.Errorf("managed service definition %q is not a regular file", servicePath)
|
||||
}
|
||||
data, err := os.ReadFile(servicePath)
|
||||
if err != nil {
|
||||
return "", false, fmt.Errorf("read existing service definition %q: %w", servicePath, err)
|
||||
}
|
||||
if !bytes.Contains(data, []byte(managedMarker)) {
|
||||
return "", false, nil
|
||||
}
|
||||
|
||||
switch manager {
|
||||
case nativeServiceManagerSystemd:
|
||||
return parseManagedSystemdUser(data, servicePath)
|
||||
case nativeServiceManagerLaunchd:
|
||||
return parseManagedLaunchdUser(data, servicePath)
|
||||
default:
|
||||
return "", false, fmt.Errorf("unsupported service manager %q", manager)
|
||||
}
|
||||
}
|
||||
|
||||
func parseManagedSystemdUser(data []byte, servicePath string) (string, bool, error) {
|
||||
var userName string
|
||||
seen := false
|
||||
for _, line := range strings.Split(string(data), "\n") {
|
||||
line = strings.TrimSpace(line)
|
||||
if line == "" || strings.HasPrefix(line, "#") || !strings.HasPrefix(line, "User=") {
|
||||
continue
|
||||
}
|
||||
if seen {
|
||||
return "", false, fmt.Errorf("managed systemd definition %q contains multiple User= entries", servicePath)
|
||||
}
|
||||
seen = true
|
||||
userName = strings.TrimSpace(strings.TrimPrefix(line, "User="))
|
||||
if userName == "" {
|
||||
return "", false, fmt.Errorf("managed systemd definition %q has an empty User= entry", servicePath)
|
||||
}
|
||||
}
|
||||
if !seen {
|
||||
return rootServiceUser, true, nil
|
||||
}
|
||||
return userName, true, nil
|
||||
}
|
||||
|
||||
func parseManagedLaunchdUser(data []byte, servicePath string) (string, bool, error) {
|
||||
decoder := xml.NewDecoder(bytes.NewReader(data))
|
||||
userKey := false
|
||||
for {
|
||||
token, err := decoder.Token()
|
||||
if errors.Is(err, io.EOF) {
|
||||
break
|
||||
}
|
||||
if err != nil {
|
||||
return "", false, fmt.Errorf("parse managed launchd definition %q: %w", servicePath, err)
|
||||
}
|
||||
start, ok := token.(xml.StartElement)
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
switch start.Name.Local {
|
||||
case "key":
|
||||
var key string
|
||||
if err := decoder.DecodeElement(&key, &start); err != nil {
|
||||
return "", false, fmt.Errorf("parse managed launchd key %q: %w", servicePath, err)
|
||||
}
|
||||
userKey = key == "UserName"
|
||||
case "string":
|
||||
if !userKey {
|
||||
continue
|
||||
}
|
||||
var userName string
|
||||
if err := decoder.DecodeElement(&userName, &start); err != nil {
|
||||
return "", false, fmt.Errorf("parse managed launchd user %q: %w", servicePath, err)
|
||||
}
|
||||
if strings.TrimSpace(userName) == "" {
|
||||
return "", false, fmt.Errorf("managed launchd definition %q has an empty UserName", servicePath)
|
||||
}
|
||||
return strings.TrimSpace(userName), true, nil
|
||||
default:
|
||||
if userKey {
|
||||
return "", false, fmt.Errorf("managed launchd definition %q has a UserName without a string value", servicePath)
|
||||
}
|
||||
userKey = false
|
||||
}
|
||||
}
|
||||
return rootServiceUser, true, nil
|
||||
}
|
||||
|
||||
func validateSystemServiceHomeOwner(homeDir, userName string) error {
|
||||
if userName == "" {
|
||||
return nil
|
||||
}
|
||||
info, err := os.Lstat(homeDir)
|
||||
if errors.Is(err, os.ErrNotExist) {
|
||||
return fmt.Errorf("system service home %q does not exist; pre-create it and chown it to %q before installing", homeDir, userName)
|
||||
}
|
||||
if err != nil {
|
||||
return fmt.Errorf("inspect system service home %q: %w", homeDir, err)
|
||||
}
|
||||
if info.Mode()&os.ModeSymlink != 0 {
|
||||
return fmt.Errorf("system service home %q must not be a symlink", homeDir)
|
||||
}
|
||||
if !info.IsDir() {
|
||||
return fmt.Errorf("system service home %q is not a directory", homeDir)
|
||||
}
|
||||
expectedUID, _, err := lookupNativeServiceOwner(userName)
|
||||
if err != nil {
|
||||
return fmt.Errorf("resolve owner for system service home %q: %w", homeDir, err)
|
||||
}
|
||||
actualUID, err := nativeFileOwnerUID(info)
|
||||
if err != nil {
|
||||
return fmt.Errorf("inspect owner of system service home %q: %w", homeDir, err)
|
||||
}
|
||||
if actualUID != expectedUID {
|
||||
return fmt.Errorf(
|
||||
"system service home %q is owned by uid %d but service account %q uses uid %d; preserve the existing account or reconcile ownership explicitly before reinstalling",
|
||||
homeDir, actualUID, userName, expectedUID,
|
||||
)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
@@ -0,0 +1,12 @@
|
||||
//go:build !linux && !darwin && !windows
|
||||
|
||||
package launcher
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
)
|
||||
|
||||
func nativeFileOwnerUID(os.FileInfo) (int, error) {
|
||||
return 0, fmt.Errorf("system service home ownership is unsupported on this platform")
|
||||
}
|
||||
@@ -0,0 +1,20 @@
|
||||
//go:build linux || darwin
|
||||
|
||||
package launcher
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
"syscall"
|
||||
)
|
||||
|
||||
func nativeFileOwnerUID(info os.FileInfo) (int, error) {
|
||||
if info == nil {
|
||||
return 0, fmt.Errorf("filesystem metadata is nil")
|
||||
}
|
||||
stat, ok := info.Sys().(*syscall.Stat_t)
|
||||
if !ok {
|
||||
return 0, fmt.Errorf("unsupported filesystem metadata type %T", info.Sys())
|
||||
}
|
||||
return int(stat.Uid), nil
|
||||
}
|
||||
@@ -0,0 +1,12 @@
|
||||
//go:build windows
|
||||
|
||||
package launcher
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
)
|
||||
|
||||
func nativeFileOwnerUID(os.FileInfo) (int, error) {
|
||||
return 0, fmt.Errorf("system service home ownership is unsupported on windows")
|
||||
}
|
||||
@@ -0,0 +1,314 @@
|
||||
package launcher
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"os"
|
||||
"os/user"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestParseServiceArgsAcceptsSystemRunAs(t *testing.T) {
|
||||
args, err := parseServiceArgs([]string{"install", "--system", "--run-as", "alice"})
|
||||
if err != nil {
|
||||
t.Fatalf("parseServiceArgs() error = %v, want --run-as to be accepted for system install", err)
|
||||
}
|
||||
if args.Action != actionInstall || !args.System {
|
||||
t.Fatalf("parsed args = %#v, want system install", args)
|
||||
}
|
||||
if args.RunAs != "alice" {
|
||||
t.Fatalf("RunAs = %q, want alice", args.RunAs)
|
||||
}
|
||||
if _, err := parseServiceArgs([]string{"install", "--run-as", "alice"}); err == nil {
|
||||
t.Fatal("parseServiceArgs() accepted --run-as for a user service")
|
||||
}
|
||||
if _, err := parseServiceArgs([]string{"restart", "--system", "--run-as", "alice"}); err == nil {
|
||||
t.Fatal("parseServiceArgs() accepted --run-as for a non-install action")
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveSystemServiceUserRequiresExplicitRootOnFirstInstall(t *testing.T) {
|
||||
t.Setenv("SUDO_USER", "root")
|
||||
_, err := resolveSystemServiceUser(
|
||||
serviceArgs{Action: actionInstall, System: true},
|
||||
filepath.Join(t.TempDir(), "missing.service"),
|
||||
nativeServiceManagerSystemd,
|
||||
)
|
||||
if err == nil || !strings.Contains(err.Error(), "--run-as") {
|
||||
t.Fatalf("resolveSystemServiceUser() error = %v, want explicit --run-as guidance", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestParseManagedServiceUsers(t *testing.T) {
|
||||
systemUser, found, err := parseManagedSystemdUser([]byte("[Service]\nExecStart=/bin/kandev\n"), "kandev.service")
|
||||
if err != nil || !found || systemUser != "root" {
|
||||
t.Fatalf("parseManagedSystemdUser() = %q, %v, %v; want root, true, nil", systemUser, found, err)
|
||||
}
|
||||
launchdUser, found, err := parseManagedLaunchdUser([]byte(`<?xml version="1.0"?><plist><dict><key>UserName</key><string>alice</string></dict></plist>`), "kandev.plist")
|
||||
if err != nil || !found || launchdUser != "alice" {
|
||||
t.Fatalf("parseManagedLaunchdUser() = %q, %v, %v; want alice, true, nil", launchdUser, found, err)
|
||||
}
|
||||
if _, _, err := parseManagedLaunchdUser([]byte(`<?xml version="1.0"?><plist><dict><key>UserName</key><integer>501</integer></dict></plist>`), "kandev.plist"); err == nil {
|
||||
t.Fatal("parseManagedLaunchdUser() accepted a non-string UserName")
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstallSystemdPreservesExistingManagedUser(t *testing.T) {
|
||||
if runtime.GOOS == "windows" {
|
||||
t.Skip("system service identity is unsupported on Windows")
|
||||
}
|
||||
originalExecutablePath := executablePath
|
||||
originalExecuteServiceCommand := executeServiceCommand
|
||||
originalServicePrintln := servicePrintln
|
||||
originalLookup := lookupNativeServiceOwner
|
||||
t.Cleanup(func() {
|
||||
executablePath = originalExecutablePath
|
||||
executeServiceCommand = originalExecuteServiceCommand
|
||||
servicePrintln = originalServicePrintln
|
||||
lookupNativeServiceOwner = originalLookup
|
||||
})
|
||||
|
||||
current, err := user.Current()
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
currentUID, currentGID, err := lookupNativeServiceOwner(current.Username)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
tmp := t.TempDir()
|
||||
homeDir := filepath.Join(tmp, "home")
|
||||
if err := os.Mkdir(homeDir, 0o700); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
unitPath := filepath.Join(tmp, "kandev.service")
|
||||
original := "# managed by kandev — regenerated by `kandev service install`\n[Service]\nUser=alice\n"
|
||||
if err := os.WriteFile(unitPath, []byte(original), 0o640); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
lookupNativeServiceOwner = func(string) (int, int, error) { return currentUID, currentGID, nil }
|
||||
executablePath = func() (string, error) { return "/opt/kandev/bin/kandev", nil }
|
||||
executeServiceCommand = func(string, ...string) error { return nil }
|
||||
servicePrintln = func(string) {}
|
||||
t.Setenv("SUDO_USER", "root")
|
||||
|
||||
if code := installSystemd(
|
||||
serviceArgs{Action: actionInstall, System: true, HomeDir: homeDir, NoBootStart: true},
|
||||
BuildInfo{Version: "test"}, unitPath,
|
||||
); code != 0 {
|
||||
t.Fatalf("installSystemd() = %d, want 0", code)
|
||||
}
|
||||
|
||||
unit, err := os.ReadFile(unitPath)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !strings.Contains(string(unit), "User=alice\n") {
|
||||
t.Fatalf("reinstalled unit lost preserved service user:\n%s", unit)
|
||||
}
|
||||
if !strings.Contains(string(unit), "Environment=KANDEV_SERVICE_MODE=system") {
|
||||
t.Fatalf("reinstalled unit is not a system service:\n%s", unit)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstallSystemdRejectsManagedHomeOwnerMismatchBeforeWrite(t *testing.T) {
|
||||
if runtime.GOOS == "windows" {
|
||||
t.Skip("system service identity is unsupported on Windows")
|
||||
}
|
||||
originalExecutablePath := executablePath
|
||||
originalExecuteServiceCommand := executeServiceCommand
|
||||
originalServicePrintln := servicePrintln
|
||||
originalLookup := lookupNativeServiceOwner
|
||||
t.Cleanup(func() {
|
||||
executablePath = originalExecutablePath
|
||||
executeServiceCommand = originalExecuteServiceCommand
|
||||
servicePrintln = originalServicePrintln
|
||||
lookupNativeServiceOwner = originalLookup
|
||||
})
|
||||
|
||||
tmp := t.TempDir()
|
||||
homeDir := filepath.Join(tmp, "home")
|
||||
if err := os.Mkdir(homeDir, 0o700); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
unitPath := filepath.Join(tmp, "kandev.service")
|
||||
original := "# managed by kandev — regenerated by `kandev service install`\n[Service]\nUser=alice\n"
|
||||
if err := os.WriteFile(unitPath, []byte(original), 0o640); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
lookupNativeServiceOwner = func(string) (int, int, error) { return 1234, 5678, nil }
|
||||
executablePath = func() (string, error) { return "/opt/kandev/bin/kandev", nil }
|
||||
servicePrintln = func(string) {}
|
||||
var managerCalls int
|
||||
executeServiceCommand = func(string, ...string) error {
|
||||
managerCalls++
|
||||
return nil
|
||||
}
|
||||
|
||||
code := installSystemd(
|
||||
serviceArgs{Action: actionInstall, System: true, HomeDir: homeDir, NoBootStart: true},
|
||||
BuildInfo{Version: "test"}, unitPath,
|
||||
)
|
||||
if code == 0 {
|
||||
t.Fatal("installSystemd() = 0, want owner mismatch failure")
|
||||
}
|
||||
if managerCalls != 0 {
|
||||
t.Fatalf("service manager commands = %d, want none after preflight failure", managerCalls)
|
||||
}
|
||||
definition, err := os.ReadFile(unitPath)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if string(definition) != original {
|
||||
t.Fatalf("service definition changed after owner mismatch:\n%s", definition)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstallLaunchdPreservesExistingManagedUser(t *testing.T) {
|
||||
if runtime.GOOS == "windows" {
|
||||
t.Skip("system service identity is unsupported on Windows")
|
||||
}
|
||||
originalExecutablePath := executablePath
|
||||
originalExecuteServiceCommand := executeServiceCommand
|
||||
originalLaunchctlCommand := launchctlCommand
|
||||
originalLaunchctlSleep := launchctlSleep
|
||||
originalServicePrintln := servicePrintln
|
||||
originalLookup := lookupNativeServiceOwner
|
||||
t.Cleanup(func() {
|
||||
executablePath = originalExecutablePath
|
||||
executeServiceCommand = originalExecuteServiceCommand
|
||||
launchctlCommand = originalLaunchctlCommand
|
||||
launchctlSleep = originalLaunchctlSleep
|
||||
servicePrintln = originalServicePrintln
|
||||
lookupNativeServiceOwner = originalLookup
|
||||
})
|
||||
|
||||
current, err := user.Current()
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
currentUID, currentGID, err := lookupNativeServiceOwner(current.Username)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
tmp := t.TempDir()
|
||||
homeDir := filepath.Join(tmp, "home")
|
||||
if err := os.Mkdir(homeDir, 0o700); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
plistPath := filepath.Join(tmp, "com.kdlbs.kandev.plist")
|
||||
original := "<!-- managed by kandev -->\n<plist><dict><key>UserName</key><string>alice</string></dict></plist>\n"
|
||||
if err := os.WriteFile(plistPath, []byte(original), 0o640); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
lookupNativeServiceOwner = func(string) (int, int, error) { return currentUID, currentGID, nil }
|
||||
executablePath = func() (string, error) { return "/opt/kandev/bin/kandev", nil }
|
||||
executeServiceCommand = func(string, ...string) error { return nil }
|
||||
launchctlCommand = func(args ...string) error {
|
||||
if len(args) > 0 && args[0] == "print" {
|
||||
return errors.New("not loaded")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
launchctlSleep = func(time.Duration) {}
|
||||
servicePrintln = func(string) {}
|
||||
|
||||
if code := installLaunchd(
|
||||
serviceArgs{Action: actionInstall, System: true, HomeDir: homeDir, NoBootStart: true},
|
||||
BuildInfo{Version: "test"}, plistPath, "system/com.kdlbs.kandev", "system",
|
||||
); code != 0 {
|
||||
t.Fatalf("installLaunchd() = %d, want 0", code)
|
||||
}
|
||||
|
||||
plist, err := os.ReadFile(plistPath)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !strings.Contains(string(plist), "<key>UserName</key>\n <string>alice</string>") {
|
||||
t.Fatalf("reinstalled plist lost preserved service user:\n%s", plist)
|
||||
}
|
||||
}
|
||||
|
||||
func TestInstallLaunchdRejectsManagedHomeOwnerMismatchBeforeWrite(t *testing.T) {
|
||||
if runtime.GOOS == "windows" {
|
||||
t.Skip("system service identity is unsupported on Windows")
|
||||
}
|
||||
originalExecutablePath := executablePath
|
||||
originalExecuteServiceCommand := executeServiceCommand
|
||||
originalLaunchctlCommand := launchctlCommand
|
||||
originalLaunchctlSleep := launchctlSleep
|
||||
originalServicePrintln := servicePrintln
|
||||
originalLookup := lookupNativeServiceOwner
|
||||
t.Cleanup(func() {
|
||||
executablePath = originalExecutablePath
|
||||
executeServiceCommand = originalExecuteServiceCommand
|
||||
launchctlCommand = originalLaunchctlCommand
|
||||
launchctlSleep = originalLaunchctlSleep
|
||||
servicePrintln = originalServicePrintln
|
||||
lookupNativeServiceOwner = originalLookup
|
||||
})
|
||||
|
||||
current, err := user.Current()
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
currentUID, _, err := lookupNativeServiceOwner(current.Username)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
mismatchUID := currentUID + 1
|
||||
if mismatchUID == currentUID {
|
||||
mismatchUID++
|
||||
}
|
||||
|
||||
tmp := t.TempDir()
|
||||
homeDir := filepath.Join(tmp, "home")
|
||||
if err := os.Mkdir(homeDir, 0o700); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
plistPath := filepath.Join(tmp, "com.kdlbs.kandev.plist")
|
||||
original := "<!-- managed by kandev -->\n<plist><dict><key>UserName</key><string>alice</string></dict></plist>\n"
|
||||
if err := os.WriteFile(plistPath, []byte(original), 0o640); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
lookupNativeServiceOwner = func(string) (int, int, error) { return mismatchUID, 5678, nil }
|
||||
executablePath = func() (string, error) { return "/opt/kandev/bin/kandev", nil }
|
||||
servicePrintln = func(string) {}
|
||||
var managerCalls, launchctlCalls int
|
||||
executeServiceCommand = func(string, ...string) error {
|
||||
managerCalls++
|
||||
return nil
|
||||
}
|
||||
launchctlCommand = func(...string) error {
|
||||
launchctlCalls++
|
||||
return nil
|
||||
}
|
||||
launchctlSleep = func(time.Duration) {}
|
||||
|
||||
code := installLaunchd(
|
||||
serviceArgs{Action: actionInstall, System: true, HomeDir: homeDir, NoBootStart: true},
|
||||
BuildInfo{Version: "test"}, plistPath, "system/com.kdlbs.kandev", "system",
|
||||
)
|
||||
if code == 0 {
|
||||
t.Fatal("installLaunchd() = 0, want owner mismatch failure")
|
||||
}
|
||||
if managerCalls != 0 || launchctlCalls != 0 {
|
||||
t.Fatalf("service commands = manager:%d launchctl:%d, want none after preflight failure", managerCalls, launchctlCalls)
|
||||
}
|
||||
definition, err := os.ReadFile(plistPath)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if string(definition) != original {
|
||||
t.Fatalf("service definition changed after owner mismatch:\n%s", definition)
|
||||
}
|
||||
}
|
||||
@@ -95,6 +95,7 @@ func TestInstallSystemdWritesOwnerOnlyNativeMetadata(t *testing.T) {
|
||||
serviceArgs{
|
||||
Action: actionInstall,
|
||||
System: true,
|
||||
RunAs: currentUser.Username,
|
||||
HomeDir: homeDir,
|
||||
Port: 38429,
|
||||
NoBootStart: true,
|
||||
@@ -463,7 +464,7 @@ func TestInstallSystemdRejectsSymlinkedMetadataPathsBeforeReload(t *testing.T) {
|
||||
}
|
||||
|
||||
code := installSystemd(
|
||||
serviceArgs{Action: actionInstall, System: true, HomeDir: homeDir},
|
||||
serviceArgs{Action: actionInstall, System: true, RunAs: currentUser.Username, HomeDir: homeDir},
|
||||
BuildInfo{Version: "test"},
|
||||
filepath.Join(tmp, "kandev.service"),
|
||||
)
|
||||
|
||||
@@ -286,6 +286,10 @@ type mockRepository struct {
|
||||
// Track calls for verification
|
||||
createTaskSessionCalls []*models.TaskSession
|
||||
updateTaskSessionCalls []*models.TaskSession
|
||||
updateTaskSessionSnapshots []*models.TaskSession
|
||||
updateTaskSessionIfCurrentCalls int
|
||||
updateTaskSessionIfCurrentFailOn int
|
||||
updateTaskSessionIfCurrentFailErr error
|
||||
setSessionMetadataKeyCalls []setSessionMetadataKeyCall
|
||||
setSessionPrimaryCalls []string
|
||||
createTaskEnvironmentCalls []*models.TaskEnvironment
|
||||
@@ -405,6 +409,12 @@ func (m *mockRepository) UpdateTaskSessionIfCurrentState(
|
||||
if current.State != expected {
|
||||
return false, nil
|
||||
}
|
||||
m.updateTaskSessionIfCurrentCalls++
|
||||
if m.updateTaskSessionIfCurrentFailOn > 0 &&
|
||||
m.updateTaskSessionIfCurrentCalls == m.updateTaskSessionIfCurrentFailOn {
|
||||
return false, m.updateTaskSessionIfCurrentFailErr
|
||||
}
|
||||
m.updateTaskSessionSnapshots = append(m.updateTaskSessionSnapshots, cloneMockTaskSession(session))
|
||||
m.updateTaskSessionCalls = append(m.updateTaskSessionCalls, session)
|
||||
m.sessions[session.ID] = session
|
||||
return true, nil
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"fmt"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"reflect"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
@@ -72,6 +73,47 @@ type repoInfo struct {
|
||||
Repository *models.Repository
|
||||
}
|
||||
|
||||
// resumeCredentialSnapshotBackup holds the non-secret routing metadata that
|
||||
// was persisted before a resume attempt. A failed launch must restore this
|
||||
// value so the session never advertises credentials that did not successfully
|
||||
// start an agent.
|
||||
type resumeCredentialSnapshotBackup struct {
|
||||
value interface{}
|
||||
present bool
|
||||
}
|
||||
|
||||
func captureResumeCredentialSnapshot(session *models.TaskSession) resumeCredentialSnapshotBackup {
|
||||
if session == nil || session.Metadata == nil {
|
||||
return resumeCredentialSnapshotBackup{}
|
||||
}
|
||||
value, present := session.Metadata[models.SessionMetaKeyGitCredentialSnapshot]
|
||||
return resumeCredentialSnapshotBackup{value: value, present: present}
|
||||
}
|
||||
|
||||
func resumeCredentialSnapshotChanged(session *models.TaskSession, backup resumeCredentialSnapshotBackup) bool {
|
||||
if session == nil || session.Metadata == nil {
|
||||
return backup.present
|
||||
}
|
||||
value, present := session.Metadata[models.SessionMetaKeyGitCredentialSnapshot]
|
||||
if present != backup.present {
|
||||
return true
|
||||
}
|
||||
if !present {
|
||||
return false
|
||||
}
|
||||
return !reflect.DeepEqual(value, backup.value)
|
||||
}
|
||||
|
||||
func resumeCredentialSnapshotBackupIfPersisted(
|
||||
persisted bool,
|
||||
backup resumeCredentialSnapshotBackup,
|
||||
) *resumeCredentialSnapshotBackup {
|
||||
if !persisted {
|
||||
return nil
|
||||
}
|
||||
return &backup
|
||||
}
|
||||
|
||||
// resolveAllRepoInfo returns the resolved repository info for every repository
|
||||
// linked to the task, ordered by Position. Returns a single-element slice for
|
||||
// single-repo tasks and an empty slice for repo-less tasks (e.g. quick chat).
|
||||
@@ -451,6 +493,7 @@ func (e *Executor) persistWorktreeAssociation(ctx context.Context, taskID string
|
||||
func (e *Executor) ResumeSession(ctx context.Context, session *models.TaskSession, startAgent bool) (*TaskExecution, error) {
|
||||
if session != nil {
|
||||
resumeSnapshot := *session
|
||||
resumeSnapshot.Metadata = cloneMetadata(session.Metadata)
|
||||
session = &resumeSnapshot
|
||||
}
|
||||
task, unlock, err := e.validateAndLockResume(ctx, session)
|
||||
@@ -459,20 +502,8 @@ func (e *Executor) ResumeSession(ctx context.Context, session *models.TaskSessio
|
||||
}
|
||||
defer unlock()
|
||||
|
||||
req, repositoryID, execCfg, existingEnv, _, err := e.buildResumeRequest(ctx, task, session, startAgent)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
e.logger.Debug("resuming agent session",
|
||||
zap.String("task_id", session.TaskID),
|
||||
zap.String("session_id", session.ID),
|
||||
zap.String("agent_profile_id", session.AgentProfileID),
|
||||
zap.String("executor_type", req.ExecutorType),
|
||||
zap.String("resume_token", req.ACPSessionID),
|
||||
zap.Bool("use_worktree", req.UseWorktree))
|
||||
|
||||
resumeInitialState := session.State
|
||||
previousCredentialSnapshot := captureResumeCredentialSnapshot(session)
|
||||
wasTerminalResume := isTerminalSessionState(resumeInitialState)
|
||||
// Force-cleanup any stale in-memory execution / agentctl state for terminal-state
|
||||
// sessions. Their agent process is dead by definition, so "already running" signals
|
||||
@@ -486,16 +517,50 @@ func (e *Executor) ResumeSession(ctx context.Context, session *models.TaskSessio
|
||||
}
|
||||
}
|
||||
|
||||
req.Env = e.applyPreferredShellEnv(ctx, req.ExecutorType, req.Env)
|
||||
|
||||
// Persist STARTING before LaunchAgent requests provider credentials. The
|
||||
// credential broker rejects terminal sessions, so the guarded transition
|
||||
// must be visible before the launch boundary is crossed.
|
||||
resumeStatePersisted := false
|
||||
var beforeCredentialLease func() error
|
||||
if startAgent {
|
||||
if err := e.persistResumeState(ctx, task.ID, session, true); err != nil {
|
||||
return nil, err
|
||||
beforeCredentialLease = func() error {
|
||||
if persistErr := e.persistResumeState(ctx, task.ID, session, true); persistErr != nil {
|
||||
return persistErr
|
||||
}
|
||||
resumeStatePersisted = true
|
||||
return nil
|
||||
}
|
||||
}
|
||||
req, repositoryID, execCfg, existingEnv, _, err := e.buildResumeRequestAtCredentialBoundary(
|
||||
ctx, task, session, startAgent, beforeCredentialLease,
|
||||
)
|
||||
if err != nil {
|
||||
if resumeStatePersisted {
|
||||
e.rollbackResumeStateAfterFailure(ctx, task.ID, session.ID, resumeInitialState, err, nil)
|
||||
}
|
||||
return nil, err
|
||||
}
|
||||
credentialSnapshotPersisted := false
|
||||
if startAgent {
|
||||
// Credential setup records the non-secret routing snapshot after the
|
||||
// lease issuer has observed STARTING. Persist that metadata with the
|
||||
// same expected-state guard before launching, so a concurrent terminal
|
||||
// transition cannot be overwritten by a stale resume.
|
||||
if err := e.persistSessionFullRowIfCurrentState(ctx, session, models.TaskSessionStateStarting); err != nil {
|
||||
if resumeStatePersisted {
|
||||
e.rollbackResumeStateAfterFailure(ctx, task.ID, session.ID, resumeInitialState, err, nil)
|
||||
}
|
||||
return nil, err
|
||||
}
|
||||
credentialSnapshotPersisted = resumeCredentialSnapshotChanged(session, previousCredentialSnapshot)
|
||||
}
|
||||
|
||||
e.logger.Debug("resuming agent session",
|
||||
zap.String("task_id", session.TaskID),
|
||||
zap.String("session_id", session.ID),
|
||||
zap.String("agent_profile_id", session.AgentProfileID),
|
||||
zap.String("executor_type", req.ExecutorType),
|
||||
zap.String("resume_token", req.ACPSessionID),
|
||||
zap.Bool("use_worktree", req.UseWorktree))
|
||||
|
||||
req.Env = e.applyPreferredShellEnv(ctx, req.ExecutorType, req.Env)
|
||||
|
||||
resp, err := e.agentManager.LaunchAgent(ctx, req)
|
||||
if err != nil && isAgentAlreadyRunningError(err) {
|
||||
@@ -511,7 +576,10 @@ func (e *Executor) ResumeSession(ctx context.Context, session *models.TaskSessio
|
||||
zap.String("task_id", task.ID),
|
||||
zap.String("session_id", session.ID))
|
||||
if startAgent {
|
||||
e.rollbackResumeStateAfterLaunchFailure(ctx, task.ID, session.ID, resumeInitialState, err)
|
||||
e.rollbackResumeStateAfterFailure(
|
||||
ctx, task.ID, session.ID, resumeInitialState, err,
|
||||
resumeCredentialSnapshotBackupIfPersisted(credentialSnapshotPersisted, previousCredentialSnapshot),
|
||||
)
|
||||
}
|
||||
return nil, ErrExecutionAlreadyRunning
|
||||
}
|
||||
@@ -527,7 +595,10 @@ func (e *Executor) ResumeSession(ctx context.Context, session *models.TaskSessio
|
||||
}
|
||||
if err != nil {
|
||||
if startAgent {
|
||||
e.rollbackResumeStateAfterLaunchFailure(ctx, task.ID, session.ID, resumeInitialState, err)
|
||||
e.rollbackResumeStateAfterFailure(
|
||||
ctx, task.ID, session.ID, resumeInitialState, err,
|
||||
resumeCredentialSnapshotBackupIfPersisted(credentialSnapshotPersisted, previousCredentialSnapshot),
|
||||
)
|
||||
}
|
||||
e.logger.Error("failed to relaunch agent for session",
|
||||
zap.String("task_id", task.ID),
|
||||
@@ -581,18 +652,68 @@ func (e *Executor) ResumeSession(ctx context.Context, session *models.TaskSessio
|
||||
return execution, nil
|
||||
}
|
||||
|
||||
// rollbackResumeStateAfterLaunchFailure restores the state observed before a
|
||||
// resume attempt only while the session is still STARTING. A concurrent
|
||||
// terminal transition wins and is left untouched by transitionSessionState.
|
||||
func (e *Executor) rollbackResumeStateAfterLaunchFailure(ctx context.Context, taskID, sessionID string, priorState models.TaskSessionState, launchErr error) {
|
||||
// restoreResumeCredentialSnapshotIfStarting restores the prior non-secret Git
|
||||
// credential routing metadata only while the session is still STARTING. The
|
||||
// state guard ensures a concurrent terminal transition cannot be overwritten by
|
||||
// a stale resume rollback.
|
||||
func (e *Executor) restoreResumeCredentialSnapshotIfStarting(
|
||||
ctx context.Context,
|
||||
sessionID string,
|
||||
backup *resumeCredentialSnapshotBackup,
|
||||
) {
|
||||
if backup == nil {
|
||||
return
|
||||
}
|
||||
current, err := e.repo.GetTaskSession(ctx, sessionID)
|
||||
if err != nil || current == nil || current.State != models.TaskSessionStateStarting {
|
||||
return
|
||||
}
|
||||
if !resumeCredentialSnapshotChanged(current, *backup) {
|
||||
return
|
||||
}
|
||||
|
||||
metadata := cloneMetadata(current.Metadata)
|
||||
if backup.present {
|
||||
if metadata == nil {
|
||||
metadata = make(map[string]interface{})
|
||||
}
|
||||
metadata[models.SessionMetaKeyGitCredentialSnapshot] = backup.value
|
||||
} else {
|
||||
delete(metadata, models.SessionMetaKeyGitCredentialSnapshot)
|
||||
}
|
||||
current.Metadata = metadata
|
||||
changed, err := e.repo.UpdateTaskSessionIfCurrentState(ctx, current, models.TaskSessionStateStarting)
|
||||
if err != nil {
|
||||
e.logger.Warn("failed to restore Git credential snapshot after resume failure",
|
||||
zap.String("session_id", sessionID),
|
||||
zap.Error(err))
|
||||
return
|
||||
}
|
||||
if !changed {
|
||||
e.logger.Debug("skipped Git credential snapshot rollback after session state changed",
|
||||
zap.String("session_id", sessionID))
|
||||
}
|
||||
}
|
||||
|
||||
// rollbackResumeStateAfterFailure restores the state observed before a resume
|
||||
// attempt only while the session is still STARTING. A concurrent terminal
|
||||
// transition wins and is left untouched by transitionSessionState.
|
||||
func (e *Executor) rollbackResumeStateAfterFailure(
|
||||
ctx context.Context,
|
||||
taskID, sessionID string,
|
||||
priorState models.TaskSessionState,
|
||||
resumeErr error,
|
||||
credentialSnapshot *resumeCredentialSnapshotBackup,
|
||||
) {
|
||||
e.restoreResumeCredentialSnapshotIfStarting(ctx, sessionID, credentialSnapshot)
|
||||
if e.onSessionStateTransition != nil {
|
||||
current, err := e.repo.GetTaskSession(ctx, sessionID)
|
||||
if err != nil || current == nil || current.State != models.TaskSessionStateStarting {
|
||||
return
|
||||
}
|
||||
_, _, rollbackErr := e.transitionSessionState(ctx, taskID, sessionID, priorState, launchErr.Error())
|
||||
_, _, rollbackErr := e.transitionSessionState(ctx, taskID, sessionID, priorState, resumeErr.Error())
|
||||
if rollbackErr != nil {
|
||||
e.logger.Warn("failed to roll back session state after resume launch failure",
|
||||
e.logger.Warn("failed to roll back session state after resume failure",
|
||||
zap.String("task_id", taskID),
|
||||
zap.String("session_id", sessionID),
|
||||
zap.Error(rollbackErr))
|
||||
@@ -607,9 +728,9 @@ func (e *Executor) rollbackResumeStateAfterLaunchFailure(ctx context.Context, ta
|
||||
sessionID,
|
||||
models.TaskSessionStateStarting,
|
||||
priorState,
|
||||
launchErr.Error(),
|
||||
resumeErr.Error(),
|
||||
); err != nil {
|
||||
e.logger.Warn("failed to roll back session state after resume launch failure",
|
||||
e.logger.Warn("failed to roll back session state after resume failure",
|
||||
zap.String("task_id", taskID),
|
||||
zap.String("session_id", sessionID),
|
||||
zap.Error(err))
|
||||
@@ -620,9 +741,9 @@ func (e *Executor) rollbackResumeStateAfterLaunchFailure(ctx context.Context, ta
|
||||
if err != nil || current == nil || current.State != models.TaskSessionStateStarting {
|
||||
return
|
||||
}
|
||||
_, _, rollbackErr := e.transitionSessionState(ctx, taskID, sessionID, priorState, launchErr.Error())
|
||||
_, _, rollbackErr := e.transitionSessionState(ctx, taskID, sessionID, priorState, resumeErr.Error())
|
||||
if rollbackErr != nil {
|
||||
e.logger.Warn("failed to roll back session state after resume launch failure",
|
||||
e.logger.Warn("failed to roll back session state after resume failure",
|
||||
zap.String("task_id", taskID),
|
||||
zap.String("session_id", sessionID),
|
||||
zap.Error(rollbackErr))
|
||||
@@ -716,6 +837,20 @@ func (e *Executor) validateAndLockResume(ctx context.Context, session *models.Ta
|
||||
// repository details, worktree settings, and ACP resume token.
|
||||
// Returns the request, repository ID, executor config, existing ExecutorRunning record (may be nil), and error.
|
||||
func (e *Executor) buildResumeRequest(ctx context.Context, task *v1.Task, session *models.TaskSession, startAgent bool) (*LaunchAgentRequest, string, executorConfig, *models.TaskEnvironment, *models.ExecutorRunning, error) {
|
||||
return e.buildResumeRequestAtCredentialBoundary(ctx, task, session, startAgent, nil)
|
||||
}
|
||||
|
||||
// buildResumeRequestAtCredentialBoundary prepares a resume request and invokes
|
||||
// beforeCredentialLease after all prior-state-dependent preparation succeeds,
|
||||
// immediately before managed GitHub credential issuance. Production resume
|
||||
// uses this boundary to persist STARTING; focused request-builder tests omit it.
|
||||
func (e *Executor) buildResumeRequestAtCredentialBoundary(
|
||||
ctx context.Context,
|
||||
task *v1.Task,
|
||||
session *models.TaskSession,
|
||||
startAgent bool,
|
||||
beforeCredentialLease func() error,
|
||||
) (*LaunchAgentRequest, string, executorConfig, *models.TaskEnvironment, *models.ExecutorRunning, error) {
|
||||
executionProfileID := session.ExecutionProfileID
|
||||
if executionProfileID == "" {
|
||||
executionProfileID = session.AgentProfileID
|
||||
@@ -759,18 +894,12 @@ func (e *Executor) buildResumeRequest(ctx context.Context, task *v1.Task, sessio
|
||||
req.TaskEnvironmentID = session.TaskEnvironmentID
|
||||
}
|
||||
|
||||
repositoryID, err := e.applyResumeRepoConfig(ctx, task, session, req, existingEnv)
|
||||
if err != nil {
|
||||
return nil, "", execConfig, nil, nil, err
|
||||
}
|
||||
allRepos, err := e.resolveAllRepoInfo(ctx, task.ID)
|
||||
if err != nil {
|
||||
return nil, "", execConfig, nil, nil, err
|
||||
}
|
||||
if err := e.configureGitHubCredentialBrokerForRepositories(ctx, req, allRepos); err != nil {
|
||||
return nil, "", execConfig, nil, nil, err
|
||||
}
|
||||
if err := e.applyGitCredentialSnapshot(ctx, req, session); err != nil {
|
||||
repositoryID, err := e.applyResumeRepoConfig(ctx, task, session, req, existingEnv, allRepos)
|
||||
if err != nil {
|
||||
return nil, "", execConfig, nil, nil, err
|
||||
}
|
||||
|
||||
@@ -782,20 +911,56 @@ func (e *Executor) buildResumeRequest(ctx context.Context, task *v1.Task, sessio
|
||||
}
|
||||
|
||||
existingRunning := e.applyRunningRecordToResumeRequest(ctx, req, task, session, startAgent)
|
||||
e.injectGitLabWorkspaceCredentials(ctx, req)
|
||||
if folders, folderErr := e.repo.ListTaskWorkspaceFolders(ctx, task.ID); folderErr != nil {
|
||||
return nil, "", execConfig, existingEnv, nil, folderErr
|
||||
} else {
|
||||
for _, f := range folders {
|
||||
if f != nil {
|
||||
req.WorkspaceFolders = append(req.WorkspaceFolders, WorkspaceFolderSpec{Name: f.DisplayName, LocalPath: f.LocalPath})
|
||||
}
|
||||
}
|
||||
if err := e.applyResumeWorkspaceFolders(ctx, task.ID, req); err != nil {
|
||||
return nil, "", execConfig, existingEnv, nil, err
|
||||
}
|
||||
if err := e.configureResumeGitHubCredentials(
|
||||
ctx, req, session, allRepos, beforeCredentialLease,
|
||||
); err != nil {
|
||||
return nil, "", execConfig, existingEnv, existingRunning, err
|
||||
}
|
||||
e.injectGitLabWorkspaceCredentials(ctx, req)
|
||||
|
||||
return req, repositoryID, execConfig, existingEnv, existingRunning, nil
|
||||
}
|
||||
|
||||
func (e *Executor) applyResumeWorkspaceFolders(
|
||||
ctx context.Context,
|
||||
taskID string,
|
||||
req *LaunchAgentRequest,
|
||||
) error {
|
||||
folders, err := e.repo.ListTaskWorkspaceFolders(ctx, taskID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
for _, folder := range folders {
|
||||
if folder != nil {
|
||||
req.WorkspaceFolders = append(req.WorkspaceFolders, WorkspaceFolderSpec{
|
||||
Name: folder.DisplayName, LocalPath: folder.LocalPath,
|
||||
})
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (e *Executor) configureResumeGitHubCredentials(
|
||||
ctx context.Context,
|
||||
req *LaunchAgentRequest,
|
||||
session *models.TaskSession,
|
||||
repositories []*repoInfo,
|
||||
beforeCredentialLease func() error,
|
||||
) error {
|
||||
if beforeCredentialLease != nil {
|
||||
if err := beforeCredentialLease(); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
if err := e.configureGitHubCredentialBrokerForRepositories(ctx, req, repositories); err != nil {
|
||||
return err
|
||||
}
|
||||
return e.applyGitCredentialSnapshot(ctx, req, session)
|
||||
}
|
||||
|
||||
func (e *Executor) resolveResumeTaskEnvironment(ctx context.Context, taskID string, session *models.TaskSession) (*models.TaskEnvironment, error) {
|
||||
env, err := e.repo.GetTaskEnvironmentByTaskID(ctx, taskID)
|
||||
if err != nil {
|
||||
@@ -922,7 +1087,21 @@ func (e *Executor) applyRunningRecordToResumeRequest(ctx context.Context, req *L
|
||||
// TaskDirName without an extra DB round-trip and so both the single-repo and
|
||||
// multi-repo branches agree on the same fallback name (the fallback uses a
|
||||
// random suffix, so two independent calls would produce different names).
|
||||
func (e *Executor) applyResumeRepoConfig(ctx context.Context, task *v1.Task, session *models.TaskSession, req *LaunchAgentRequest, existingEnv *models.TaskEnvironment) (string, error) {
|
||||
// resolvedRepos is supplied by buildResumeRequest so primary, multi-repository,
|
||||
// and credential configuration share one materialization pass. Legacy callers
|
||||
// may omit it; those calls resolve the set once locally.
|
||||
func (e *Executor) applyResumeRepoConfig(
|
||||
ctx context.Context,
|
||||
task *v1.Task,
|
||||
session *models.TaskSession,
|
||||
req *LaunchAgentRequest,
|
||||
existingEnv *models.TaskEnvironment,
|
||||
resolved ...[]*repoInfo,
|
||||
) (string, error) {
|
||||
allRepos, err := e.resumeRepoSet(ctx, task.ID, resolved...)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
repositoryID, baseBranch := resolveResumeRepoIDAndBranch(task, session)
|
||||
if baseBranch != "" {
|
||||
req.Branch = baseBranch
|
||||
@@ -931,21 +1110,29 @@ func (e *Executor) applyResumeRepoConfig(ctx context.Context, task *v1.Task, ses
|
||||
return "", nil
|
||||
}
|
||||
|
||||
repository, err := e.repo.GetRepository(ctx, repositoryID)
|
||||
if err != nil {
|
||||
e.logger.Error("failed to load repository for task session resume",
|
||||
zap.String("task_id", task.ID),
|
||||
zap.String("repository_id", repositoryID),
|
||||
zap.Error(err))
|
||||
return "", err
|
||||
var repository *models.Repository
|
||||
for _, info := range allRepos {
|
||||
if info != nil && info.RepositoryID == repositoryID {
|
||||
repository = info.Repository
|
||||
break
|
||||
}
|
||||
}
|
||||
if repository == nil {
|
||||
repository, err = e.repo.GetRepository(ctx, repositoryID)
|
||||
if err != nil {
|
||||
e.logger.Error("failed to load repository for task session resume",
|
||||
zap.String("task_id", task.ID),
|
||||
zap.String("repository_id", repositoryID),
|
||||
zap.Error(err))
|
||||
return "", err
|
||||
}
|
||||
|
||||
// Self-heal a stale/missing provider-backed local path before using it,
|
||||
// mirroring the guard in resolveTaskRepoInfo — otherwise a single-repo
|
||||
// resume with a stale path skips re-cloning and fails downstream in the
|
||||
// worktree preparer.
|
||||
if err := e.ensureRepoLocalPath(ctx, repository); err != nil {
|
||||
return "", err
|
||||
// A persisted session repository may legitimately be absent from the
|
||||
// current task attachment set. Resolve that one exceptional repository,
|
||||
// but never re-resolve the complete task set.
|
||||
if err := e.ensureRepoLocalPath(ctx, repository); err != nil {
|
||||
return "", err
|
||||
}
|
||||
}
|
||||
|
||||
repositoryPath := repository.LocalPath
|
||||
@@ -959,13 +1146,20 @@ func (e *Executor) applyResumeRepoConfig(ctx context.Context, task *v1.Task, ses
|
||||
e.applyResumeWorktreeConfig(ctx, task, req, repository, repositoryID, repositoryPath, baseBranch, existingEnv)
|
||||
}
|
||||
|
||||
if err := e.applyResumeMultiRepoConfig(ctx, task, req, existingEnv); err != nil {
|
||||
if err := e.applyResumeMultiRepoConfig(task, req, existingEnv, allRepos); err != nil {
|
||||
return repositoryID, err
|
||||
}
|
||||
|
||||
return repositoryID, nil
|
||||
}
|
||||
|
||||
func (e *Executor) resumeRepoSet(ctx context.Context, taskID string, resolved ...[]*repoInfo) ([]*repoInfo, error) {
|
||||
if len(resolved) > 0 {
|
||||
return resolved[0], nil
|
||||
}
|
||||
return e.resolveAllRepoInfo(ctx, taskID)
|
||||
}
|
||||
|
||||
// resolveResumeRepoIDAndBranch picks the primary repositoryID and baseBranch
|
||||
// for a resume, preferring the session's persisted values and falling back to
|
||||
// the task's primary repository when the session row was created before those
|
||||
@@ -1025,16 +1219,12 @@ func (e *Executor) applyResumeCloneURL(req *LaunchAgentRequest, repository *mode
|
||||
// legacy top-level fields stay populated from the primary for backwards
|
||||
// compat.
|
||||
//
|
||||
// Gates on resolveAllRepoInfo's DB-backed count, not task.Repositories: task
|
||||
// here is loaded via the raw repository GetTask (validateAndLockResume),
|
||||
// which never attaches the one-to-many task_repositories rows — that field
|
||||
// is always empty on this path. Gating on it silently dropped every repo but
|
||||
// the primary on any resume of a multi-repo task.
|
||||
func (e *Executor) applyResumeMultiRepoConfig(ctx context.Context, task *v1.Task, req *LaunchAgentRequest, existingEnv *models.TaskEnvironment) error {
|
||||
allRepos, err := e.resolveAllRepoInfo(ctx, task.ID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
// Gates on the resolved repository set's DB-backed count, not task.Repositories:
|
||||
// task here is loaded via the raw repository GetTask (validateAndLockResume),
|
||||
// which never attaches the one-to-many task_repositories rows — that field is
|
||||
// always empty on this path. Gating on it silently dropped every repo but the
|
||||
// primary on any resume of a multi-repo task.
|
||||
func (e *Executor) applyResumeMultiRepoConfig(task *v1.Task, req *LaunchAgentRequest, existingEnv *models.TaskEnvironment, allRepos []*repoInfo) error {
|
||||
if len(allRepos) > 1 {
|
||||
req.Repositories = buildRepoSpecs(allRepos)
|
||||
for i := range req.Repositories {
|
||||
|
||||
+49
-3
@@ -9,6 +9,7 @@ import (
|
||||
"testing"
|
||||
|
||||
"github.com/kandev/kandev/internal/task/models"
|
||||
v1 "github.com/kandev/kandev/pkg/api/v1"
|
||||
)
|
||||
|
||||
func TestEnsureRepoLocalPath_ReconcilesGitHubOriginForCredentialPolicy(t *testing.T) {
|
||||
@@ -186,6 +187,49 @@ func TestEnsureRepoLocalPath_PersistsFreshCloneBeforeOriginFailure(t *testing.T)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildResumeRequestPreparesEachAttachedRepositoryOnce(t *testing.T) {
|
||||
if _, err := exec.LookPath("git"); err != nil {
|
||||
t.Skip("git not available")
|
||||
}
|
||||
|
||||
repoPathOne := initGitRepoWithOrigin(t, "https://github.com/acme/one.git")
|
||||
repoPathTwo := initGitRepoWithOrigin(t, "https://github.com/acme/two.git")
|
||||
repo := newMockRepository()
|
||||
repo.tasks["task-1"] = &models.Task{ID: "task-1", WorkspaceID: "workspace-1", Title: "resume"}
|
||||
repo.repositories["repo-1"] = &models.Repository{
|
||||
ID: "repo-1", WorkspaceID: "workspace-1", SourceType: "provider", Provider: "github",
|
||||
ProviderOwner: "acme", ProviderName: "one", LocalPath: repoPathOne,
|
||||
}
|
||||
repo.repositories["repo-2"] = &models.Repository{
|
||||
ID: "repo-2", WorkspaceID: "workspace-1", SourceType: "provider", Provider: "github",
|
||||
ProviderOwner: "acme", ProviderName: "two", LocalPath: repoPathTwo,
|
||||
}
|
||||
repo.taskRepositories["task-repo-1"] = &models.TaskRepository{ID: "task-repo-1", TaskID: "task-1", RepositoryID: "repo-1", Position: 0}
|
||||
repo.taskRepositories["task-repo-2"] = &models.TaskRepository{ID: "task-repo-2", TaskID: "task-1", RepositoryID: "repo-2", Position: 1}
|
||||
executor := newTestExecutor(t, &mockAgentManager{}, repo)
|
||||
cloner := &cloneTransportTestCloner{cloneURL: "git@github.com:acme/widgets.git"}
|
||||
executor.SetRepoCloner(cloner, nil)
|
||||
session := &models.TaskSession{
|
||||
ID: "session-1", TaskID: "task-1", RepositoryID: "repo-1", ExecutorID: models.ExecutorIDLocal,
|
||||
State: models.TaskSessionStateWaitingForInput,
|
||||
}
|
||||
|
||||
if _, _, _, _, _, err := executor.buildResumeRequest(context.Background(), &v1.Task{ID: "task-1", WorkspaceID: "workspace-1", Title: "resume"}, session, true); err != nil {
|
||||
t.Fatalf("buildResumeRequest() error = %v", err)
|
||||
}
|
||||
for _, repositoryPath := range []string{repoPathOne, repoPathTwo} {
|
||||
calls := 0
|
||||
for _, preparedPath := range cloner.setOriginPaths {
|
||||
if preparedPath == repositoryPath {
|
||||
calls++
|
||||
}
|
||||
}
|
||||
if calls != 1 {
|
||||
t.Fatalf("origin reconciliation calls for %q = %d, want one", repositoryPath, calls)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
type localPathRecordingRepoUpdater struct {
|
||||
localPath string
|
||||
}
|
||||
@@ -200,9 +244,10 @@ func (u *localPathRecordingRepoUpdater) UpdateRepositoryDefaultBranch(context.Co
|
||||
}
|
||||
|
||||
type cloneTransportTestCloner struct {
|
||||
cloneURL string
|
||||
returnPath string
|
||||
setOriginErr error
|
||||
cloneURL string
|
||||
returnPath string
|
||||
setOriginErr error
|
||||
setOriginPaths []string
|
||||
}
|
||||
|
||||
func (c *cloneTransportTestCloner) EnsureWorkspaceClonedForProvider(
|
||||
@@ -214,6 +259,7 @@ func (c *cloneTransportTestCloner) EnsureWorkspaceClonedForProvider(
|
||||
func (c *cloneTransportTestCloner) ShouldRecloneForWorkspace(string, string) bool { return false }
|
||||
|
||||
func (c *cloneTransportTestCloner) SetOriginURL(ctx context.Context, repositoryPath, originURL string) error {
|
||||
c.setOriginPaths = append(c.setOriginPaths, repositoryPath)
|
||||
if c.setOriginErr != nil {
|
||||
return c.setOriginErr
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@ package executor
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
@@ -71,6 +72,217 @@ func setupLiveResumeTestFixture(repo *mockRepository) {
|
||||
}
|
||||
}
|
||||
|
||||
type resumeCredentialStateIssuer struct {
|
||||
repo *mockRepository
|
||||
observedState models.TaskSessionState
|
||||
err error
|
||||
afterIssue func()
|
||||
}
|
||||
|
||||
func (i *resumeCredentialStateIssuer) IssueGitHubCredentialLease(
|
||||
ctx context.Context,
|
||||
req GitHubCredentialLeaseRequest,
|
||||
) (GitHubCredentialLease, error) {
|
||||
session, err := i.repo.GetTaskSession(ctx, req.SessionID)
|
||||
if err != nil {
|
||||
return GitHubCredentialLease{}, err
|
||||
}
|
||||
i.observedState = session.State
|
||||
if i.err != nil {
|
||||
return GitHubCredentialLease{}, i.err
|
||||
}
|
||||
if session.State != models.TaskSessionStateStarting {
|
||||
return GitHubCredentialLease{}, fmt.Errorf("GitHub credential scope denied: session is terminal")
|
||||
}
|
||||
if i.afterIssue != nil {
|
||||
i.afterIssue()
|
||||
}
|
||||
return GitHubCredentialLease{Token: "opaque-lease"}, nil
|
||||
}
|
||||
|
||||
func attachManagedGitHubRepositoryForResume(t *testing.T, repo *mockRepository) {
|
||||
t.Helper()
|
||||
repo.sessions["sess-1"].RepositoryID = "repo-1"
|
||||
repo.repositories["repo-1"] = &models.Repository{
|
||||
ID: "repo-1",
|
||||
WorkspaceID: "workspace-1",
|
||||
Name: "widgets",
|
||||
SourceType: sourceTypeLocal,
|
||||
LocalPath: t.TempDir(),
|
||||
Provider: "github",
|
||||
ProviderOwner: "acme",
|
||||
ProviderName: "widgets",
|
||||
DefaultBranch: "main",
|
||||
}
|
||||
repo.taskRepositories["task-repo-1"] = &models.TaskRepository{
|
||||
ID: "task-repo-1", TaskID: "task-1", RepositoryID: "repo-1",
|
||||
}
|
||||
}
|
||||
|
||||
func TestResumeSession_PersistsStartingBeforeCredentialLease(t *testing.T) {
|
||||
for _, initialState := range []models.TaskSessionState{
|
||||
models.TaskSessionStateFailed,
|
||||
models.TaskSessionStateCancelled,
|
||||
} {
|
||||
t.Run(string(initialState), func(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
setupLiveResumeTestFixture(repo)
|
||||
attachManagedGitHubRepositoryForResume(t, repo)
|
||||
repo.sessions["sess-1"].State = initialState
|
||||
|
||||
agentMgr := &mockAgentManager{}
|
||||
issuer := &resumeCredentialStateIssuer{repo: repo}
|
||||
exec := newTestExecutor(t, agentMgr, repo)
|
||||
exec.SetGitHubCredentialBroker(issuer, "http://localhost:8080/api/github/credentials/resolve")
|
||||
|
||||
if _, err := exec.ResumeSession(context.Background(), repo.sessions["sess-1"], true); err != nil {
|
||||
t.Fatalf("ResumeSession: %v", err)
|
||||
}
|
||||
if issuer.observedState != models.TaskSessionStateStarting {
|
||||
t.Fatalf("session state at credential lease = %s, want %s", issuer.observedState, models.TaskSessionStateStarting)
|
||||
}
|
||||
if agentMgr.launchAgentCallCount != 1 {
|
||||
t.Fatalf("LaunchAgent calls = %d, want 1", agentMgr.launchAgentCallCount)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestResumeSession_RollsBackStartingWhenCredentialLeaseFails(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
setupLiveResumeTestFixture(repo)
|
||||
attachManagedGitHubRepositoryForResume(t, repo)
|
||||
repo.sessions["sess-1"].State = models.TaskSessionStateFailed
|
||||
|
||||
leaseErr := errors.New("credential lease unavailable")
|
||||
agentMgr := &mockAgentManager{}
|
||||
issuer := &resumeCredentialStateIssuer{repo: repo, err: leaseErr}
|
||||
exec := newTestExecutor(t, agentMgr, repo)
|
||||
exec.SetGitHubCredentialBroker(issuer, "http://localhost:8080/api/github/credentials/resolve")
|
||||
|
||||
if _, err := exec.ResumeSession(context.Background(), repo.sessions["sess-1"], true); !errors.Is(err, leaseErr) {
|
||||
t.Fatalf("ResumeSession error = %v, want %v", err, leaseErr)
|
||||
}
|
||||
if issuer.observedState != models.TaskSessionStateStarting {
|
||||
t.Fatalf("session state at credential lease = %s, want %s", issuer.observedState, models.TaskSessionStateStarting)
|
||||
}
|
||||
current := repo.sessions["sess-1"]
|
||||
if current.State != models.TaskSessionStateFailed {
|
||||
t.Fatalf("session state after credential failure = %s, want %s", current.State, models.TaskSessionStateFailed)
|
||||
}
|
||||
if !strings.Contains(current.ErrorMessage, leaseErr.Error()) {
|
||||
t.Fatalf("session error = %q, want credential lease error", current.ErrorMessage)
|
||||
}
|
||||
if agentMgr.launchAgentCallCount != 0 {
|
||||
t.Fatalf("LaunchAgent calls = %d, want 0", agentMgr.launchAgentCallCount)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResumeSession_PersistsCredentialSnapshot(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
setupLiveResumeTestFixture(repo)
|
||||
attachManagedGitHubRepositoryForResume(t, repo)
|
||||
repo.sessions["sess-1"].State = models.TaskSessionStateFailed
|
||||
repo.sessions["sess-1"].Metadata = map[string]interface{}{
|
||||
models.SessionMetaKeyGitCredentialSnapshot: models.GitCredentialSnapshot{
|
||||
Version: 1,
|
||||
Source: "executor",
|
||||
Transport: "executor_selected",
|
||||
},
|
||||
}
|
||||
|
||||
issuer := &resumeCredentialStateIssuer{repo: repo}
|
||||
exec := newTestExecutor(t, &mockAgentManager{}, repo)
|
||||
exec.SetGitHubCredentialBroker(issuer, "http://localhost:8080/api/github/credentials/resolve")
|
||||
|
||||
if _, err := exec.ResumeSession(context.Background(), repo.sessions["sess-1"], true); err != nil {
|
||||
t.Fatalf("ResumeSession: %v", err)
|
||||
}
|
||||
|
||||
var persistedSnapshot models.GitCredentialSnapshot
|
||||
for _, persisted := range repo.updateTaskSessionSnapshots {
|
||||
if persisted.State != models.TaskSessionStateStarting || persisted.Metadata == nil {
|
||||
continue
|
||||
}
|
||||
value, ok := persisted.Metadata[models.SessionMetaKeyGitCredentialSnapshot]
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
encoded, err := json.Marshal(value)
|
||||
if err != nil {
|
||||
t.Fatalf("marshal persisted credential snapshot: %v", err)
|
||||
}
|
||||
var candidate models.GitCredentialSnapshot
|
||||
if err := json.Unmarshal(encoded, &candidate); err != nil {
|
||||
t.Fatalf("unmarshal persisted credential snapshot: %v", err)
|
||||
}
|
||||
if candidate.Source == "workspace" {
|
||||
persistedSnapshot = candidate
|
||||
break
|
||||
}
|
||||
}
|
||||
if persistedSnapshot.Source != "workspace" {
|
||||
t.Fatalf("persisted credential snapshot = %#v, want workspace snapshot", persistedSnapshot)
|
||||
}
|
||||
if persistedSnapshot.Transport != "managed_https" {
|
||||
t.Fatalf("persisted credential transport = %q, want managed_https", persistedSnapshot.Transport)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResumeSession_DoesNotLaunchWhenCredentialSnapshotPersistenceLosesTerminalRace(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
setupLiveResumeTestFixture(repo)
|
||||
attachManagedGitHubRepositoryForResume(t, repo)
|
||||
repo.sessions["sess-1"].State = models.TaskSessionStateFailed
|
||||
|
||||
agentMgr := &mockAgentManager{}
|
||||
issuer := &resumeCredentialStateIssuer{
|
||||
repo: repo,
|
||||
afterIssue: func() {
|
||||
repo.sessions["sess-1"].State = models.TaskSessionStateCancelled
|
||||
},
|
||||
}
|
||||
exec := newTestExecutor(t, agentMgr, repo)
|
||||
exec.SetGitHubCredentialBroker(issuer, "http://localhost:8080/api/github/credentials/resolve")
|
||||
|
||||
_, err := exec.ResumeSession(context.Background(), repo.sessions["sess-1"], true)
|
||||
if err == nil || !errors.Is(err, ErrSessionStateSuperseded) {
|
||||
t.Fatalf("ResumeSession error = %v, want ErrSessionStateSuperseded", err)
|
||||
}
|
||||
if got := repo.sessions["sess-1"].State; got != models.TaskSessionStateCancelled {
|
||||
t.Fatalf("session state = %s, want CANCELLED", got)
|
||||
}
|
||||
if agentMgr.launchAgentCallCount != 0 {
|
||||
t.Fatalf("LaunchAgent calls = %d, want 0", agentMgr.launchAgentCallCount)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResumeSession_RollsBackWhenCredentialSnapshotPersistenceFails(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
setupLiveResumeTestFixture(repo)
|
||||
attachManagedGitHubRepositoryForResume(t, repo)
|
||||
repo.sessions["sess-1"].State = models.TaskSessionStateFailed
|
||||
|
||||
persistErr := errors.New("credential snapshot persistence failed")
|
||||
repo.updateTaskSessionIfCurrentFailOn = 2
|
||||
repo.updateTaskSessionIfCurrentFailErr = persistErr
|
||||
issuer := &resumeCredentialStateIssuer{repo: repo}
|
||||
agentMgr := &mockAgentManager{}
|
||||
exec := newTestExecutor(t, agentMgr, repo)
|
||||
exec.SetGitHubCredentialBroker(issuer, "http://localhost:8080/api/github/credentials/resolve")
|
||||
|
||||
_, err := exec.ResumeSession(context.Background(), repo.sessions["sess-1"], true)
|
||||
if !errors.Is(err, persistErr) {
|
||||
t.Fatalf("ResumeSession error = %v, want %v", err, persistErr)
|
||||
}
|
||||
if got := repo.sessions["sess-1"].State; got != models.TaskSessionStateFailed {
|
||||
t.Fatalf("session state = %s, want FAILED", got)
|
||||
}
|
||||
if agentMgr.launchAgentCallCount != 0 {
|
||||
t.Fatalf("LaunchAgent calls = %d, want 0", agentMgr.launchAgentCallCount)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResumeSession_PersistsStartingBeforeLaunch(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
setupLiveResumeTestFixture(repo)
|
||||
@@ -130,7 +342,52 @@ func TestResumeSession_RollsBackStartingWhenLaunchFails(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestRollbackResumeStateAfterLaunchFailure_SkipsTransitionAfterConcurrentStateChange(t *testing.T) {
|
||||
func TestResumeSession_RestoresCredentialSnapshotWhenLaunchFails(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
setupLiveResumeTestFixture(repo)
|
||||
attachManagedGitHubRepositoryForResume(t, repo)
|
||||
repo.sessions["sess-1"].State = models.TaskSessionStateFailed
|
||||
oldSnapshot := models.GitCredentialSnapshot{
|
||||
Version: 1,
|
||||
Source: "executor",
|
||||
Transport: "executor_selected",
|
||||
}
|
||||
repo.sessions["sess-1"].Metadata = map[string]interface{}{
|
||||
models.SessionMetaKeyGitCredentialSnapshot: oldSnapshot,
|
||||
}
|
||||
|
||||
launchErr := errors.New("credential broker rejected launch")
|
||||
agentMgr := &mockAgentManager{
|
||||
launchAgentFunc: func(_ context.Context, _ *LaunchAgentRequest) (*LaunchAgentResponse, error) {
|
||||
return nil, launchErr
|
||||
},
|
||||
}
|
||||
issuer := &resumeCredentialStateIssuer{repo: repo}
|
||||
exec := newTestExecutor(t, agentMgr, repo)
|
||||
exec.SetGitHubCredentialBroker(issuer, "http://localhost:8080/api/github/credentials/resolve")
|
||||
|
||||
if _, err := exec.ResumeSession(context.Background(), repo.sessions["sess-1"], true); !errors.Is(err, launchErr) {
|
||||
t.Fatalf("ResumeSession error = %v, want %v", err, launchErr)
|
||||
}
|
||||
|
||||
current := repo.sessions["sess-1"]
|
||||
if current.State != models.TaskSessionStateFailed {
|
||||
t.Fatalf("session state after failed launch = %s, want FAILED", current.State)
|
||||
}
|
||||
value, ok := current.Metadata[models.SessionMetaKeyGitCredentialSnapshot]
|
||||
if !ok {
|
||||
t.Fatal("credential snapshot missing after failed launch")
|
||||
}
|
||||
got, ok := value.(models.GitCredentialSnapshot)
|
||||
if !ok {
|
||||
t.Fatalf("credential snapshot type = %T, want models.GitCredentialSnapshot", value)
|
||||
}
|
||||
if got.Source != oldSnapshot.Source || got.Transport != oldSnapshot.Transport {
|
||||
t.Fatalf("credential snapshot after failed launch = %#v, want %#v", got, oldSnapshot)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRollbackResumeStateAfterFailure_SkipsTransitionAfterConcurrentStateChange(t *testing.T) {
|
||||
repo := newMockRepository()
|
||||
setupLiveResumeTestFixture(repo)
|
||||
repo.sessions["sess-1"].State = models.TaskSessionStateCancelled
|
||||
@@ -148,12 +405,13 @@ func TestRollbackResumeStateAfterLaunchFailure_SkipsTransitionAfterConcurrentSta
|
||||
return false, models.TaskSessionStateCancelled, nil
|
||||
})
|
||||
|
||||
exec.rollbackResumeStateAfterLaunchFailure(
|
||||
exec.rollbackResumeStateAfterFailure(
|
||||
context.Background(),
|
||||
"task-1",
|
||||
"sess-1",
|
||||
models.TaskSessionStateFailed,
|
||||
errors.New("launch failed"),
|
||||
nil,
|
||||
)
|
||||
if got := repo.sessions["sess-1"].State; got != models.TaskSessionStateCancelled {
|
||||
t.Fatalf("session state = %s, want %s", got, models.TaskSessionStateCancelled)
|
||||
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"strings"
|
||||
"sync"
|
||||
|
||||
@@ -29,9 +30,15 @@ const (
|
||||
gitCredentialHelper = `!f() { if [ "$1" = get ]; then printf '%s\n' "username=$KANDEV_REPOCLONE_GITHUB_USERNAME" "password=$KANDEV_REPOCLONE_GITHUB_TOKEN"; fi; }; f`
|
||||
managedWorkspacesDir = "workspaces"
|
||||
providerCloneDir = "_providers"
|
||||
maxGitDiagnosticBytes = 4096
|
||||
)
|
||||
|
||||
var ErrWorkspaceCredentialUnavailable = errors.New("workspace Git credential is unavailable")
|
||||
var (
|
||||
ErrWorkspaceCredentialUnavailable = errors.New("workspace Git credential is unavailable")
|
||||
ErrRepositoryOwnershipMismatch = errors.New("managed repository ownership mismatch")
|
||||
gitURLUserInfoPattern = regexp.MustCompile(`(?i)(https?://)[^/\s@]+@`)
|
||||
gitCredentialPattern = regexp.MustCompile(`(?i)\b(password|token|secret|authorization)(\s*[:=]\s*)[^\r\n]+`)
|
||||
)
|
||||
|
||||
// Config holds configuration for the repository cloner.
|
||||
type Config struct {
|
||||
@@ -336,14 +343,43 @@ func (c *Cloner) SetOriginURL(ctx context.Context, repositoryPath, originURL str
|
||||
mu.Lock()
|
||||
defer mu.Unlock()
|
||||
|
||||
cmd := exec.CommandContext(ctx, "git", "-C", repositoryPath, "remote", "set-url", "origin", "--", originURL)
|
||||
// `git remote get-url` expands url.*.insteadOf rules before returning the
|
||||
// value. Compare the value stored in the local config instead so a common
|
||||
// HTTPS-to-SSH rewrite does not turn an already-canonical origin into a
|
||||
// needless config write on every launch/resume.
|
||||
cmd := exec.CommandContext(ctx, "git", "-C", repositoryPath, "config", "--local", "--get", "remote.origin.url")
|
||||
configureGitCommand(cmd, nil)
|
||||
if _, err := subproc.RunGitCombinedOutput(ctx, cmd); err != nil {
|
||||
return fmt.Errorf("set repository origin: %w", err)
|
||||
currentOutput, err := subproc.RunGitCombinedOutput(ctx, cmd)
|
||||
if err != nil {
|
||||
return fmt.Errorf("inspect repository origin: %w", formatGitOriginError(repositoryPath, currentOutput, err))
|
||||
}
|
||||
if strings.TrimSpace(string(currentOutput)) == strings.TrimSpace(originURL) {
|
||||
return nil
|
||||
}
|
||||
|
||||
cmd = exec.CommandContext(ctx, "git", "-C", repositoryPath, "remote", "set-url", "origin", "--", originURL)
|
||||
configureGitCommand(cmd, nil)
|
||||
output, err := subproc.RunGitCombinedOutput(ctx, cmd)
|
||||
if err != nil {
|
||||
return fmt.Errorf("set repository origin: %w", formatGitOriginError(repositoryPath, output, err))
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func formatGitOriginError(repositoryPath string, output []byte, err error) error {
|
||||
diagnostic := redactCloneOutput(string(output), "")
|
||||
if strings.Contains(strings.ToLower(diagnostic), "detected dubious ownership") {
|
||||
return fmt.Errorf(
|
||||
"%w: Git rejected managed checkout %q because its filesystem owner differs from the Kandev service account; ensure that account owns the checkout or reinstall Kandev with the intended account (git: %s)",
|
||||
ErrRepositoryOwnershipMismatch, repositoryPath, diagnostic,
|
||||
)
|
||||
}
|
||||
if diagnostic == "" {
|
||||
return err
|
||||
}
|
||||
return fmt.Errorf("git reported %s: %w", diagnostic, err)
|
||||
}
|
||||
|
||||
type cloneAuth struct {
|
||||
origin string
|
||||
username string
|
||||
@@ -625,10 +661,16 @@ func redactCloneURL(raw string) string {
|
||||
}
|
||||
|
||||
func redactCloneOutput(output, token string) string {
|
||||
if token == "" {
|
||||
return output
|
||||
redacted := output
|
||||
if token != "" {
|
||||
redacted = strings.ReplaceAll(redacted, token, "[REDACTED]")
|
||||
}
|
||||
return strings.ReplaceAll(output, token, "[REDACTED]")
|
||||
redacted = gitURLUserInfoPattern.ReplaceAllString(redacted, `${1}[REDACTED]@`)
|
||||
redacted = gitCredentialPattern.ReplaceAllString(redacted, `${1}${2}[REDACTED]`)
|
||||
if len(redacted) > maxGitDiagnosticBytes {
|
||||
return redacted[:maxGitDiagnosticBytes]
|
||||
}
|
||||
return redacted
|
||||
}
|
||||
|
||||
func authToken(auth *cloneAuth) string {
|
||||
|
||||
@@ -79,6 +79,102 @@ func TestSetOriginURL(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestSetOriginURLSkipsWriteWhenOriginAlreadyMatches(t *testing.T) {
|
||||
if _, err := exec.LookPath("git"); err != nil {
|
||||
t.Skip("git not available")
|
||||
}
|
||||
|
||||
repoPath := t.TempDir()
|
||||
runGit(t, repoPath, "init", "--quiet")
|
||||
const origin = "https://github.com/acme/widgets.git"
|
||||
runGit(t, repoPath, "remote", "add", "origin", origin)
|
||||
if err := os.WriteFile(filepath.Join(repoPath, ".git", "config.lock"), nil, 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Cleanup(func() { _ = os.Remove(filepath.Join(repoPath, ".git", "config.lock")) })
|
||||
|
||||
cloner := NewCloner(Config{}, ProtocolHTTPS, t.TempDir(), logger.Default())
|
||||
if err := cloner.SetOriginURL(context.Background(), repoPath, origin); err != nil {
|
||||
t.Fatalf("SetOriginURL() with an already-matching origin = %v, want no-op success", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSetOriginURLComparesStoredOriginBeforeInsteadOfExpansion(t *testing.T) {
|
||||
if _, err := exec.LookPath("git"); err != nil {
|
||||
t.Skip("git not available")
|
||||
}
|
||||
|
||||
repoPath := t.TempDir()
|
||||
runGit(t, repoPath, "init", "--quiet")
|
||||
const origin = "https://github.com/acme/widgets.git"
|
||||
runGit(t, repoPath, "remote", "add", "origin", origin)
|
||||
runGit(t, repoPath, "config", "--local", "url.git@github.com:.insteadOf", "https://github.com/")
|
||||
if got := strings.TrimSpace(runGit(t, repoPath, "remote", "get-url", "origin")); got != "git@github.com:acme/widgets.git" {
|
||||
t.Fatalf("expanded origin = %q, want SSH URL", got)
|
||||
}
|
||||
if err := os.WriteFile(filepath.Join(repoPath, ".git", "config.lock"), nil, 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Cleanup(func() { _ = os.Remove(filepath.Join(repoPath, ".git", "config.lock")) })
|
||||
|
||||
cloner := NewCloner(Config{}, ProtocolHTTPS, t.TempDir(), logger.Default())
|
||||
if err := cloner.SetOriginURL(context.Background(), repoPath, origin); err != nil {
|
||||
t.Fatalf("SetOriginURL() with insteadOf rewrite = %v, want no-op success", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSetOriginURLIncludesBoundedGitDiagnostic(t *testing.T) {
|
||||
if _, err := exec.LookPath("git"); err != nil {
|
||||
t.Skip("git not available")
|
||||
}
|
||||
|
||||
repoPath := t.TempDir()
|
||||
runGit(t, repoPath, "init", "--quiet")
|
||||
runGit(t, repoPath, "remote", "add", "origin", "https://github.com/acme/widgets.git")
|
||||
if err := os.WriteFile(filepath.Join(repoPath, ".git", "config.lock"), nil, 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Cleanup(func() { _ = os.Remove(filepath.Join(repoPath, ".git", "config.lock")) })
|
||||
|
||||
cloner := NewCloner(Config{}, ProtocolSSH, t.TempDir(), logger.Default())
|
||||
err := cloner.SetOriginURL(context.Background(), repoPath, "git@github.com:acme/widgets.git")
|
||||
if err == nil || !strings.Contains(err.Error(), "could not lock config file") {
|
||||
t.Fatalf("SetOriginURL() error = %v, want Git lock diagnostic", err)
|
||||
}
|
||||
if strings.TrimSpace(err.Error()) == "set repository origin: exit status 128" {
|
||||
t.Fatalf("SetOriginURL() discarded Git diagnostic: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSetOriginURLClassifiesDubiousOwnership(t *testing.T) {
|
||||
binDir := t.TempDir()
|
||||
gitPath := filepath.Join(binDir, "git")
|
||||
if err := os.WriteFile(gitPath, []byte("#!/bin/sh\nprintf '%s\\n' 'fatal: detected dubious ownership in repository at /var/lib/kandev/repos/acme/widgets' >&2\nexit 128\n"), 0o755); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH"))
|
||||
|
||||
cloner := NewCloner(Config{}, ProtocolSSH, t.TempDir(), logger.Default())
|
||||
err := cloner.SetOriginURL(context.Background(), "/var/lib/kandev/repos/acme/widgets", "git@github.com:acme/widgets.git")
|
||||
if err == nil || !errors.Is(err, ErrRepositoryOwnershipMismatch) {
|
||||
t.Fatalf("SetOriginURL() error = %v, want ownership mismatch", err)
|
||||
}
|
||||
if !strings.Contains(err.Error(), "service account") || strings.Contains(err.Error(), "safe.directory") {
|
||||
t.Fatalf("ownership diagnostic = %v, want service-account guidance without safe.directory workaround", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRedactCloneOutputRedactsCredentialsAndBoundsOutput(t *testing.T) {
|
||||
output := "fatal: https://alice:secret@example.com/acme/widgets.git password=another-secret Authorization: Bearer synthetic-secret " + strings.Repeat("x", maxGitDiagnosticBytes+100)
|
||||
redacted := redactCloneOutput(output, "another-secret")
|
||||
if strings.Contains(redacted, "secret") || strings.Contains(redacted, "another-secret") {
|
||||
t.Fatalf("redactCloneOutput() leaked credential: %q", redacted)
|
||||
}
|
||||
if len(redacted) > maxGitDiagnosticBytes {
|
||||
t.Fatalf("redactCloneOutput() length = %d, want <= %d", len(redacted), maxGitDiagnosticBytes)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSetOriginURLSerializesConcurrentUpdates(t *testing.T) {
|
||||
if _, err := exec.LookPath("git"); err != nil {
|
||||
t.Skip("git not available")
|
||||
|
||||
@@ -0,0 +1,102 @@
|
||||
# ADR-2026-07-31-system-service-user-continuity: Preserve System Service Identity Across Reinstallation
|
||||
|
||||
**Status:** accepted
|
||||
**Date:** 2026-07-31
|
||||
**Area:** backend, cli, security, operations
|
||||
|
||||
## Context
|
||||
|
||||
`kandev service install --system` currently derives its service account from the process that runs
|
||||
the installer. A command invoked through `sudo` normally sees the non-root `SUDO_USER`, while the
|
||||
same command invoked from a root login resolves to root. Re-running installation after a package
|
||||
upgrade can therefore silently rewrite an existing non-root service as root while leaving
|
||||
`KANDEV_HOME_DIR` and its managed repositories owned by the original account.
|
||||
|
||||
Git correctly rejects a repository owned by another account when the process runs with greater or
|
||||
different authority. The new managed-checkout origin reconciliation made that latent mismatch
|
||||
visible during every launch and resume: `git remote set-url` exits 128 before an agent starts. The
|
||||
same identity drift can affect database access, secrets, generated files, executor credentials, and
|
||||
any future operation, so treating only the Git symptom would leave the underlying operational and
|
||||
security boundary broken.
|
||||
|
||||
Kandev also writes `<KANDEV_HOME_DIR>/service/install.json`. That file is intentionally readable and
|
||||
writable by the service account. A privileged installer cannot trust it as the authority for a
|
||||
future run-as account: a compromised service could edit the file to request a more privileged
|
||||
identity before an administrator reruns installation.
|
||||
|
||||
## Decision
|
||||
|
||||
System-service identity is durable installation state. `kandev service install --system` resolves
|
||||
the target account in this order:
|
||||
|
||||
1. An explicit `--run-as <user>` supplied for this installation.
|
||||
2. The account in an existing root-controlled, Kandev-managed systemd unit or launchd plist.
|
||||
3. A non-root `SUDO_USER` for a first installation.
|
||||
4. Otherwise no implicit account: installation fails and requires `--run-as`, including the
|
||||
explicit `--run-as root` choice.
|
||||
|
||||
Only an existing definition carrying Kandev's managed-service marker participates in preservation.
|
||||
The installer parses the effective account from the root-controlled unit or plist, validates that
|
||||
the account exists, and rejects malformed or contradictory managed definitions before writing.
|
||||
Service-owned install metadata remains useful for status and update compatibility checks, but it
|
||||
may only corroborate the selected account; it cannot select or elevate it.
|
||||
|
||||
Before replacing or restarting a system service, the installer validates the system Kandev home:
|
||||
|
||||
- If the home does not exist, installation fails with pre-create and ownership guidance. Kandev
|
||||
does not create a privileged system data root implicitly.
|
||||
- If it exists, its owner UID must match the selected account's UID.
|
||||
- A mismatch fails before the service definition is changed or restarted and reports the current
|
||||
owner, selected account, and explicit recovery choices.
|
||||
|
||||
Kandev does not recursively chown an existing data tree, add `safe.directory=*`, or suppress Git's
|
||||
ownership check. An operator intentionally migrating service identity must explicitly select the
|
||||
new account and reconcile the home ownership according to local policy before installation can
|
||||
complete. This keeps destructive or privilege-changing filesystem operations outside an otherwise
|
||||
routine package update.
|
||||
|
||||
`--run-as` is valid only with `service install --system`. User services continue to run as the
|
||||
calling user and do not expose an account override.
|
||||
|
||||
## Consequences
|
||||
|
||||
- Reinstalling after a Homebrew, npm, or bundle upgrade cannot silently change an established
|
||||
system service from a non-root account to root merely because the command was run from a root
|
||||
shell.
|
||||
- First-time root-shell installs become explicit. Operators who genuinely want a root service use
|
||||
`--run-as root` and receive a reviewable command history.
|
||||
- Existing installations that already run as root remain root when their managed definition is
|
||||
valid; continuity does not force an unrelated migration.
|
||||
- A service/data ownership mismatch is detected before restart, instead of appearing later as an
|
||||
opaque database, Git, or secret-store failure.
|
||||
- Service-owned metadata cannot be edited to cause privilege escalation on the next privileged
|
||||
install.
|
||||
- Deliberate account migration requires an additional operator-managed ownership step and may need
|
||||
ACL-specific handling on unusual deployments.
|
||||
|
||||
## Alternatives Considered
|
||||
|
||||
### Continue deriving the account from the current shell
|
||||
|
||||
Rejected. The account running a package update is not durable service configuration, and root
|
||||
logins lack the `SUDO_USER` signal that happened to make earlier installs work.
|
||||
|
||||
### Trust `service/install.json` as the preserved identity
|
||||
|
||||
Rejected. The service account owns that file. Letting it choose the account written into a
|
||||
root-controlled service definition would create a privilege-escalation path.
|
||||
|
||||
### Automatically chown the Kandev home when the account changes
|
||||
|
||||
Rejected. Recursive ownership changes are destructive, can cross operator-managed mount or
|
||||
symlink boundaries, discard intended ACL semantics, and turn a typo into a broad data mutation.
|
||||
|
||||
### Add a broad Git `safe.directory` exception
|
||||
|
||||
Rejected. That would mask service identity drift, weaken Git's repository-ownership protection,
|
||||
and leave non-Git files under the wrong operational authority.
|
||||
|
||||
### Always run system services as root
|
||||
|
||||
Rejected. It expands task and integration authority unnecessarily and breaks existing non-root
|
||||
installations whose data and credentials belong to their established service account.
|
||||
@@ -93,3 +93,4 @@ Read individual ADRs for full context. Create new ones via `/record decision` or
|
||||
| 2026-07-30-session-owned-mcp-observability | [Keep MCP Attachment Evidence Session Owned](2026-07-30-session-owned-mcp-observability.md) | accepted | backend, frontend, protocol, security | 2026-07-30 |
|
||||
| 2026-07-30-runtime-task-state-before-running-event | [Publish Task State Before Running Session State](2026-07-30-runtime-task-state-before-running-event.md) | accepted | backend, frontend, protocol, workflow | 2026-07-30 |
|
||||
| 2026-07-31-isolate-manual-pr-review-content | [Isolate Manual PR Review Content](2026-07-31-isolate-manual-pr-review-content.md) | accepted | infra, workflow, security | 2026-07-31 |
|
||||
| 2026-07-31-system-service-user-continuity | [Preserve System Service Identity Across Reinstallation](2026-07-31-system-service-user-continuity.md) | accepted | backend, cli, security, operations | 2026-07-31 |
|
||||
|
||||
@@ -22,10 +22,15 @@ credential, and update surfaces; there is no API or frontend contract change.
|
||||
`executors_running.resume_token` as an unusable saved session and calls
|
||||
`clearResumeToken`, even when ACP `initialize` never returned and
|
||||
`session/load` was never reached.
|
||||
- `Executor.ResumeSession` calls `agentManager.LaunchAgent` before
|
||||
`persistResumeState`. GitHub credential issuance occurs inside that launch,
|
||||
while `githubBrokerScopeAuthorizer` still sees `FAILED` or `CANCELLED` and
|
||||
rejects the lease as terminal.
|
||||
- `Executor.ResumeSession` calls `buildResumeRequest` before
|
||||
`persistResumeState`, and `buildResumeRequest` itself issues the GitHub
|
||||
credential lease. The prior repair moved the transition ahead of
|
||||
`LaunchAgent` but not ahead of this earlier lease boundary, so
|
||||
`githubBrokerScopeAuthorizer` still sees `FAILED` or `CANCELLED` and rejects
|
||||
the lease as terminal.
|
||||
- A successful resume applies `git_credential_snapshot` after the guarded
|
||||
`STARTING` write. Without a second guarded metadata write, SQLite retains
|
||||
the previous credential-routing display even though the new lease is used.
|
||||
- `CacheUpdateCommand` can reuse an already-extracted `_npx` directory. npm's
|
||||
package-content cache can be valid while that execution tree is truncated,
|
||||
so another `npm exec` does not necessarily repair it.
|
||||
@@ -34,7 +39,8 @@ credential, and update surfaces; there is no API or frontend contract change.
|
||||
|
||||
## Backend
|
||||
|
||||
Tasks 01–03 are complete; implementation and focused verification are done.
|
||||
Tasks 01–03 are complete. Tasks 04–05 repair the credential-boundary ordering
|
||||
regression discovered after release and persist its non-secret display metadata.
|
||||
|
||||
### Preserve resume identity on startup failure
|
||||
|
||||
@@ -71,6 +77,33 @@ Tasks 01–03 are complete; implementation and focused verification are done.
|
||||
launch boundary must read the repository and reject any lease attempt unless
|
||||
the session is already `STARTING`.
|
||||
|
||||
### Repair the credential-boundary ordering regression
|
||||
|
||||
- Reorder resume request preparation so state-sensitive request fields and
|
||||
other fallible preparation complete while the in-memory session still
|
||||
reflects its prior state.
|
||||
- Run the existing guarded `persistResumeState` transition immediately before
|
||||
`configureGitHubCredentialBrokerForRepositories`, which is the actual lease
|
||||
boundary, and remove the later duplicate transition before `LaunchAgent`.
|
||||
- Extend rollback to cover credential issuance and any remaining request-build
|
||||
failure after the transition, while retaining the prior-state snapshot for
|
||||
stale-execution cleanup and concurrent-terminal protection.
|
||||
- Add a repository-observing credential issuer in
|
||||
`apps/backend/internal/orchestrator/executor/executor_resume_test.go`; unlike
|
||||
the earlier `LaunchAgent` fake, it asserts the state at the exact broker
|
||||
boundary.
|
||||
|
||||
### Persist the resume credential snapshot
|
||||
|
||||
- After broker configuration records the non-secret credential snapshot, write
|
||||
the changed session metadata with the existing expected-state guard while
|
||||
the session is still `STARTING`.
|
||||
- Abort the launch and use the existing guarded rollback if that metadata write
|
||||
loses a concurrent terminal transition or otherwise fails.
|
||||
- Add a regression test that observes the persisted snapshot after a resumed
|
||||
managed-GitHub launch; the test must distinguish the first `STARTING` write
|
||||
from the later metadata write.
|
||||
|
||||
### Repair one managed npm execution cache
|
||||
|
||||
- Add a deterministic npm execution-cache key helper for
|
||||
@@ -110,6 +143,18 @@ Tasks 01–03 are complete; implementation and focused verification are done.
|
||||
`apps/backend/internal/orchestrator/task_operations_resume_test.go`.
|
||||
**How:** repository-observing fake `LaunchAgent`, table-driven prior states,
|
||||
and a concurrent-terminal-winner case.
|
||||
- **What:** the managed GitHub credential issuer observes `STARTING`, and a
|
||||
lease failure rolls the session back to its prior recoverable state.
|
||||
**File:**
|
||||
`apps/backend/internal/orchestrator/executor/executor_resume_test.go`.
|
||||
**How:** repository-observing fake `GitHubCredentialLeaseIssuer` exercised
|
||||
through the real `ResumeSession -> buildResumeRequest` path.
|
||||
- **What:** a resumed launch persists the current non-secret credential
|
||||
routing snapshot after lease setup without weakening the `STARTING` guard.
|
||||
**File:**
|
||||
`apps/backend/internal/orchestrator/executor/executor_resume_test.go`.
|
||||
**How:** inspect detached full-row snapshots recorded by the repository fake
|
||||
and assert the workspace credential display metadata is present.
|
||||
- **What:** deterministic keys match npm for built-in packages and cache
|
||||
invalidation cannot escape the resolved `_npx` root.
|
||||
**File:**
|
||||
@@ -138,6 +183,14 @@ Wave 2:
|
||||
|
||||
- [x] [Task 02: Transition resume state before credential issuance](task-02-resume-state-before-credentials.md)
|
||||
|
||||
Wave 3:
|
||||
|
||||
- [x] [Task 04: Repair resume lease-boundary ordering](task-04-resume-lease-boundary.md)
|
||||
|
||||
Wave 4:
|
||||
|
||||
- [x] [Task 05: Persist resume credential snapshot](task-05-resume-credential-snapshot.md)
|
||||
|
||||
Execution remains sequential in the primary conversation by default. The
|
||||
parallel-safe label does not authorize delegation.
|
||||
|
||||
@@ -153,5 +206,10 @@ parallel-safe label does not authorize delegation.
|
||||
by the supported npm runtime and a repair failure remains bounded.
|
||||
- Moving `STARTING` earlier changes when observers see resume progress; guarded
|
||||
rollback and state-race tests prevent stale active state.
|
||||
- The transition must remain after request preparation that reads the prior
|
||||
session state, but before the first managed credential lease request.
|
||||
- Credential display metadata is non-secret and must be persisted with an
|
||||
expected `STARTING` state; it must not revive or overwrite a concurrent
|
||||
terminal session.
|
||||
- The plan does not weaken credential authorization, auto-start fresh
|
||||
conversations, clean global npm caches, or add launch-time cache mutation.
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
---
|
||||
id: "04-resume-lease-boundary"
|
||||
title: "Repair resume lease-boundary ordering"
|
||||
status: done
|
||||
wave: 3
|
||||
depends_on:
|
||||
- "02-resume-state-before-credentials"
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/agent-resume-runtime-recovery/spec.md"
|
||||
---
|
||||
|
||||
# Task 04: Repair resume lease-boundary ordering
|
||||
|
||||
## Acceptance
|
||||
|
||||
- A `FAILED` or `CANCELLED` managed-GitHub session is guardedly persisted as
|
||||
`STARTING` before `buildResumeRequest` invokes the credential lease issuer.
|
||||
- Request preparation that depends on the prior session state retains its
|
||||
existing behavior, and `LaunchAgent` does not perform a duplicate state
|
||||
transition.
|
||||
- A credential issuance or later request-build failure rolls a still-`STARTING`
|
||||
session back without overwriting a concurrent terminal transition.
|
||||
|
||||
## Verification
|
||||
|
||||
- `cd apps/backend && go test ./internal/orchestrator/executor -run 'TestResumeSession_(PersistsStartingBeforeCredentialLease|RollsBackStartingWhenCredentialLeaseFails)' -count=1`
|
||||
- `cd apps/backend && go test ./internal/orchestrator/executor -run 'TestResumeSession|TestRollbackResumeState' -count=1`
|
||||
- `cd apps/backend && go test ./internal/orchestrator -run 'TestResumeTaskSession' -count=1`
|
||||
- `cd apps/backend && go test ./internal/backendapp -run 'TestGitHubBrokerScopeAuthorizer' -count=1`
|
||||
- `cd apps/backend && go test -race ./internal/orchestrator/executor -run 'TestResumeSession_(PersistsStartingBeforeCredentialLease|RollsBackStartingWhenCredentialLeaseFails|ConcurrentResume)' -count=1`
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/orchestrator/executor/executor_resume.go`
|
||||
- `apps/backend/internal/orchestrator/executor/executor_resume_test.go`
|
||||
- `docs/specs/agent-resume-runtime-recovery/spec.md`
|
||||
- `docs/plans/agent-resume-runtime-recovery/plan.md`
|
||||
|
||||
## Dependencies
|
||||
|
||||
Task 02. This task corrects the lease-boundary assumption in that completed
|
||||
implementation.
|
||||
|
||||
## Parallelism
|
||||
|
||||
Sequential. Production and regression-test changes share the resume lifecycle
|
||||
path.
|
||||
|
||||
## Inputs
|
||||
|
||||
- Repair spec `Persistence and security constraints` and amended regression
|
||||
scenarios
|
||||
- Plan section `Repair the credential-boundary ordering regression`
|
||||
- `Executor.ResumeSession`, `buildResumeRequest`, `persistResumeState`, and
|
||||
`rollbackResumeStateAfterLaunchFailure`
|
||||
- `githubBrokerScopeAuthorizer` terminal-session rejection, which remains
|
||||
unchanged
|
||||
|
||||
## Output contract
|
||||
|
||||
Report red/green evidence at the credential issuer, rollback and concurrency
|
||||
behavior, exact test results, files changed, blockers, and risk notes. Mark
|
||||
this task `done`, its plan checkbox complete, and the repair spec `shipped` in
|
||||
the same conversation.
|
||||
@@ -0,0 +1,45 @@
|
||||
---
|
||||
id: "05-resume-credential-snapshot"
|
||||
title: "Persist resume credential snapshot"
|
||||
status: done
|
||||
wave: 4
|
||||
depends_on:
|
||||
- "04-resume-lease-boundary"
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/agent-resume-runtime-recovery/spec.md"
|
||||
---
|
||||
|
||||
# Task 05: Persist resume credential snapshot
|
||||
|
||||
## Acceptance
|
||||
|
||||
- A successful managed-GitHub resume persists the current non-secret
|
||||
`git_credential_snapshot` after credential setup and before `LaunchAgent`.
|
||||
- The metadata write is guarded by the session's current `STARTING` state and
|
||||
cannot overwrite a concurrent terminal transition.
|
||||
- A metadata persistence failure aborts launch and uses the existing guarded
|
||||
resume rollback path.
|
||||
|
||||
## Verification
|
||||
|
||||
- `cd apps/backend && go test ./internal/orchestrator/executor -run 'TestResumeSession_(PersistsCredentialSnapshot|RollsBackWhenCredentialSnapshotPersistenceFails|DoesNotLaunchWhenCredentialSnapshotPersistenceLosesTerminalRace)' -count=1`
|
||||
- `cd apps/backend && go test ./internal/orchestrator/executor -run 'TestResumeSession|TestRollbackResumeState' -count=1`
|
||||
- `cd apps/backend && go test -race ./internal/orchestrator/executor -run 'TestResumeSession_(PersistsCredentialSnapshot|RollsBackWhenCredentialSnapshotPersistenceFails|DoesNotLaunchWhenCredentialSnapshotPersistenceLosesTerminalRace|ConcurrentResume)' -count=1`
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/orchestrator/executor/executor_resume.go`
|
||||
- `apps/backend/internal/orchestrator/executor/executor_resume_test.go`
|
||||
- `apps/backend/internal/orchestrator/executor/executor_mocks_test.go`
|
||||
- `docs/specs/agent-resume-runtime-recovery/spec.md`
|
||||
- `docs/plans/agent-resume-runtime-recovery/plan.md`
|
||||
|
||||
## Parallelism
|
||||
|
||||
Sequential. The metadata write shares the resume lifecycle and state guard with
|
||||
Task 04.
|
||||
|
||||
## Output contract
|
||||
|
||||
Report red/green evidence, the expected-state guard behavior, exact targeted
|
||||
test results, and whether any public documentation changed.
|
||||
@@ -0,0 +1,176 @@
|
||||
---
|
||||
spec: docs/specs/native-kandev-cli/spec.md
|
||||
related_spec: docs/specs/integrations/github-authentication.md
|
||||
decision: docs/decisions/2026-07-31-system-service-user-continuity.md
|
||||
created: 2026-07-31
|
||||
status: complete
|
||||
---
|
||||
|
||||
# Fix Plan: Preserve Service Identity and Harden Managed Checkout Recovery
|
||||
|
||||
## Overview
|
||||
|
||||
Release `0.83.0` began reconciling the `origin` URL of every Kandev-managed GitHub checkout during
|
||||
Local and Worktree repository preparation. A reported Homebrew system installation then failed to
|
||||
launch both old and new tasks with `set repository origin: exit status 128`.
|
||||
|
||||
The repair addresses both layers exposed by that incident:
|
||||
|
||||
1. Preserve the established system-service account across reinstall and update commands, reject
|
||||
silent identity drift, and validate the service-home ownership boundary before restart.
|
||||
2. Make managed checkout origin reconciliation idempotent, single-pass on resume, and capable of
|
||||
returning credential-safe Git diagnostics instead of a bare exit code.
|
||||
|
||||
No database migration, frontend state, or executor protocol change is required.
|
||||
|
||||
## Confirmed Root Cause
|
||||
|
||||
The reporter's system service resolved to root, while `/var/lib/kandev`, its managed repository,
|
||||
`.git`, and `.git/config` were all owned by `brewuser`. Git run as root under Kandev's intentionally
|
||||
sanitized configuration rejected the checkout as dubious ownership and returned status 128.
|
||||
|
||||
The update sequence can produce that state because `service install --system` currently derives
|
||||
the account from the installer process: a command invoked with `sudo` can select `SUDO_USER`, while
|
||||
the same command from a root login selects root. Reinstallation rewrites the service definition but
|
||||
does not change the existing data owner.
|
||||
|
||||
Release `0.83.0` exposed the latent mismatch because `repoclone.Cloner.SetOriginURL` now executes
|
||||
`git remote set-url` during repository preparation. The operation discards Git's combined output,
|
||||
so the user sees only `exit status 128`. The resume path also prepares the primary repository in
|
||||
`applyResumeRepoConfig`, resolves all repositories again for multi-repository configuration, and
|
||||
resolves them once more for credential routing, repeating the mutation boundary.
|
||||
|
||||
Smallest reproductions:
|
||||
|
||||
- Run Git with `GIT_CONFIG_NOSYSTEM=1` and `GIT_CONFIG_GLOBAL=/dev/null` as root against a checkout
|
||||
owned by another UID: Git emits `fatal: detected dubious ownership in repository` and exits 128.
|
||||
- Leave `.git/config.lock` in a managed checkout and call `SetOriginURL`: the current application
|
||||
error is the same opaque `set repository origin: exit status 128`.
|
||||
|
||||
## Design
|
||||
|
||||
### 1. Durable system-service identity
|
||||
|
||||
- Add `--run-as <user>` to native `service install --system`; reject it for user services and all
|
||||
non-install actions.
|
||||
- Resolve the target account using ADR-2026-07-31: explicit flag, existing root-controlled managed
|
||||
unit/plist, non-root `SUDO_USER` on first install, otherwise an actionable failure requiring an
|
||||
explicit account.
|
||||
- Parse only a Kandev-managed service definition. A missing systemd `User=` or launchd user key is
|
||||
the explicit root identity of an existing definition.
|
||||
- Treat service-owned `install.json` as corroborating metadata, never as privileged authority.
|
||||
- Resolve the selected account to a UID. If the system home is missing or has another owner UID,
|
||||
fail before writing or restarting the service with pre-create/ownership guidance. Do not chown
|
||||
recursively or add Git trust exceptions.
|
||||
- Persist the effective account in the generated definition and install metadata.
|
||||
|
||||
### 2. Idempotent, diagnosable origin reconciliation
|
||||
|
||||
- Keep the existing per-repository serialization in `repoclone.Cloner`.
|
||||
- Inspect `remote.origin.url` under the same sanitized Git environment. If it exactly matches the
|
||||
desired canonical URL, return without writing `.git/config`.
|
||||
- Never log or return the successful current URL. On failure, bound the diagnostic and redact known
|
||||
authentication material plus HTTP(S) URL userinfo before wrapping it.
|
||||
- Classify Git's dubious-ownership message into a stable ownership-specific error that advises
|
||||
checking the Kandev service account and managed checkout owner. Keep generic sanitized context
|
||||
for failures such as `config.lock`, missing repositories, or malformed configuration.
|
||||
- Continue failing closed. Do not retry with `safe.directory`, a different user, or a stale origin.
|
||||
|
||||
### 3. One repository-resolution pass per resume
|
||||
|
||||
- Resolve all task repositories once in `buildResumeRequest` and pass that result to primary
|
||||
repository configuration, multi-repository request construction, and GitHub credential routing.
|
||||
- Select the persisted session primary repository from that resolved set by repository ID. Preserve
|
||||
the current fallback for older sessions and resolve a persisted repository separately only when
|
||||
it is legitimately absent from the task attachment set.
|
||||
- Ensure each attached managed repository reaches `ensureRepoLocalPath` at most once per resume.
|
||||
Initial launch already uses one resolved set and remains behaviorally unchanged.
|
||||
- Preserve repository ordering, base/checkout branch metadata, repo-less tasks, inherited task
|
||||
environments, user-managed local checkout behavior, and credential snapshot semantics.
|
||||
|
||||
## Tests
|
||||
|
||||
### Native service installer
|
||||
|
||||
- Table-test argument validation and first-install identity precedence, including explicit root.
|
||||
- Render Linux/macOS definitions with `--run-as` and prove reinstall without the flag preserves the
|
||||
existing managed account when the current installer user differs.
|
||||
- Prove untrusted service metadata cannot override the root-controlled definition.
|
||||
- Prove owner-UID mismatch fails before the service file write/restart boundary and reports both
|
||||
identities; a matching pre-created home continues.
|
||||
|
||||
### Repository clone boundary
|
||||
|
||||
- Real-Git test that an already-canonical origin does not rewrite `.git/config` (mtime/content or an
|
||||
injected command boundary), while a changed origin still updates it under the existing lock.
|
||||
- Classifier tests for dubious ownership, stale `config.lock`, bounded output, URL-userinfo
|
||||
redaction, and known-secret redaction.
|
||||
- Preserve concurrent-update serialization and invalid-input coverage.
|
||||
|
||||
### Resume orchestration
|
||||
|
||||
- Instrument repository preparation and assert one call per attached repository during resume,
|
||||
including a multi-repository task.
|
||||
- Cover persisted session primary selection, an older-session fallback, a repo-less task, and the
|
||||
exceptional persisted repository not present in task attachments.
|
||||
- Retain the existing managed/executor transport transitions and user-managed-local no-op tests.
|
||||
|
||||
## Documentation
|
||||
|
||||
- Update `docs/public/run-as-a-service.md` with identity preservation, `--run-as`, first root-shell
|
||||
install behavior, ownership-mismatch recovery, and the explicit no-auto-chown policy.
|
||||
- Update `docs/public/cli.md` with the new flag and valid command combinations.
|
||||
- Update `docs/public/integrations.md` to explain actionable managed-checkout ownership errors
|
||||
without recommending broad Git trust overrides.
|
||||
- Run both public-doc validators.
|
||||
|
||||
## Implementation Tasks
|
||||
|
||||
- [x] [task-01-preserve-system-service-user](task-01-preserve-system-service-user.md) — done
|
||||
- [x] [task-02-harden-origin-reconciliation](task-02-harden-origin-reconciliation.md) — done
|
||||
- [x] [task-03-resolve-resume-repositories-once](task-03-resolve-resume-repositories-once.md) — done
|
||||
- [x] [task-04-document-service-identity-recovery](task-04-document-service-identity-recovery.md) — done
|
||||
|
||||
Tasks 01 and 02 have disjoint production boundaries and no dependency on each other. The primary
|
||||
conversation should execute them sequentially unless the user explicitly authorizes delegation.
|
||||
Task 03 depends on task 02's finalized origin-reconciliation contract. Task 04 follows all behavior
|
||||
tasks so public wording matches the implemented CLI and errors.
|
||||
|
||||
## Validation Strategy
|
||||
|
||||
Run task-focused RED/GREEN commands first, then:
|
||||
|
||||
```bash
|
||||
set -euo pipefail
|
||||
(
|
||||
cd apps/backend
|
||||
go test ./internal/launcher ./internal/repoclone ./internal/orchestrator/executor -count=1
|
||||
make test
|
||||
make lint
|
||||
)
|
||||
node --test scripts/validate-public-docs.test.mjs
|
||||
node scripts/validate-public-docs.mjs
|
||||
```
|
||||
|
||||
## Recorded Results
|
||||
|
||||
- Backend focused tests: `cd apps/backend && go test ./internal/launcher ./internal/repoclone ./internal/orchestrator/executor -count=1` passed.
|
||||
- Backend verification: `cd apps/backend && make test` and `cd apps/backend && make lint` passed.
|
||||
- Public documentation: `node --test scripts/validate-public-docs.test.mjs` passed 58 tests; `node scripts/validate-public-docs.mjs` validated 41 published pages.
|
||||
- Operator migration caveat: an existing system home owned by a different account must be reconciled explicitly before reinstall; Kandev does not recursively chown it or add Git trust exceptions.
|
||||
|
||||
No browser E2E is planned because the change adds no UI state or interaction. Existing backend
|
||||
error propagation displays the improved sanitized message in the current session error surface.
|
||||
|
||||
## Risks and Out of Scope
|
||||
|
||||
- Hand-edited or drop-in-extended service definitions may be ambiguous. Preservation is limited to
|
||||
definitions that Kandev can identify and parse safely; ambiguity fails closed.
|
||||
- UID/name changes managed outside Kandev may require operator reconciliation. Kandev will not
|
||||
infer equivalence from filesystem access or ACLs.
|
||||
- Automatic recursive ownership repair, ACL management, Windows services, global/per-repository
|
||||
`safe.directory` mutation, and privilege dropping for individual Git subprocesses are out of
|
||||
scope.
|
||||
- The fix does not weaken Kandev's isolated Git configuration. Ambient global/system Git config
|
||||
remains intentionally unavailable to repository-clone subprocesses.
|
||||
- User-managed local repository origins remain outside Kandev's mutation boundary.
|
||||
@@ -0,0 +1,79 @@
|
||||
---
|
||||
id: "01-preserve-system-service-user"
|
||||
title: "Preserve system service user"
|
||||
status: done
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/native-kandev-cli/spec.md"
|
||||
---
|
||||
|
||||
# Task 01: Preserve System Service User
|
||||
|
||||
## Acceptance
|
||||
|
||||
- `kandev service install --system --run-as <user>` explicitly selects a valid service account;
|
||||
invalid command/flag combinations fail as usage errors.
|
||||
- Reinstall without `--run-as` preserves the account in an existing root-controlled,
|
||||
Kandev-managed systemd unit or launchd plist, independent of the current login.
|
||||
- A first root-shell install without a non-root `SUDO_USER` requires explicit `--run-as`; root is
|
||||
available only as `--run-as root`.
|
||||
- Service-owned install metadata cannot select a different or more privileged account.
|
||||
- An existing system-home owner mismatch stops installation before service-file replacement or
|
||||
restart. No recursive chown or Git trust mutation occurs.
|
||||
|
||||
## TDD Sequence
|
||||
|
||||
1. Add parser and identity-resolution tests for valid/invalid `--run-as`, preserve-on-reinstall,
|
||||
first-install sudo selection, and explicit root; record RED.
|
||||
2. Add root-controlled managed-definition readers for systemd and launchd plus account validation.
|
||||
3. Add owner-UID preflight tests proving mismatch stops before installer side effects; record RED.
|
||||
4. Wire the resolved identity and preflight into native system installation and metadata writing.
|
||||
5. Run the focused and package commands below; record GREEN.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cd apps/backend && go test ./internal/launcher -run 'Test.*(RunAs|ServiceUser|SystemServiceIdentity|SystemHomeOwner|InstallSystemd|InstallLaunchd)' -count=1
|
||||
cd apps/backend && go test ./internal/launcher -count=1
|
||||
```
|
||||
|
||||
## Files Likely Touched
|
||||
|
||||
- `apps/backend/internal/launcher/service.go`
|
||||
- `apps/backend/internal/launcher/service_test.go`
|
||||
- `apps/backend/internal/launcher/service_metadata.go`
|
||||
- `apps/backend/internal/launcher/service_metadata_root_unix.go`
|
||||
- `apps/backend/internal/launcher/service_native_update_test.go`
|
||||
- a focused OS-specific service identity helper/test file if extraction keeps launcher complexity
|
||||
within lint limits
|
||||
|
||||
## Dependencies
|
||||
|
||||
None.
|
||||
|
||||
## Parallelism
|
||||
|
||||
Production files are disjoint from Task 02, but execution remains in the primary conversation
|
||||
unless the user explicitly authorizes delegation.
|
||||
|
||||
## Recorded Results
|
||||
|
||||
- RED: `TestParseServiceArgsAcceptsSystemRunAs`, `TestInstallSystemdPreservesExistingManagedUser`,
|
||||
and `TestInstallSystemdRejectsManagedHomeOwnerMismatchBeforeWrite` failed before the identity
|
||||
resolver and owner preflight existed.
|
||||
- GREEN: `cd apps/backend && go test ./internal/launcher -run 'Test.*(RunAs|ServiceUser|SystemServiceIdentity|SystemHomeOwner|InstallSystemd|InstallLaunchd)' -count=1` passed 33 cases; `cd apps/backend && go test ./internal/launcher -count=1` passed 116 tests.
|
||||
- Identity precedence is explicit `--run-as`, an existing Kandev-managed systemd/launchd definition,
|
||||
non-root `SUDO_USER` on first install, then an actionable error requiring explicit selection.
|
||||
Root is selected only by `--run-as root` on a first root-shell install.
|
||||
- The service-definition trust check accepts only a non-symlink regular definition containing the
|
||||
Kandev managed marker; service-owned install metadata is not an authority for identity.
|
||||
- Owner preflight reads the system home before any service-definition write or service-manager
|
||||
command and rejects UID mismatches. Windows does not support native system-service ownership
|
||||
validation; identity tests skip there.
|
||||
|
||||
## Output Contract
|
||||
|
||||
Report the RED failures, identity precedence, service-definition trust check, owner preflight
|
||||
behavior, exact GREEN results, and any platform-specific limitation. Update this task and the plan
|
||||
status without changing the ADR's security boundary.
|
||||
@@ -0,0 +1,69 @@
|
||||
---
|
||||
id: "02-harden-origin-reconciliation"
|
||||
title: "Harden origin reconciliation"
|
||||
status: done
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/integrations/github-authentication.md"
|
||||
---
|
||||
|
||||
# Task 02: Harden Origin Reconciliation
|
||||
|
||||
## Acceptance
|
||||
|
||||
- `SetOriginURL` keeps its per-repository lock, inspects the current origin, and performs no
|
||||
configuration write when the desired canonical URL already matches.
|
||||
- Changed origins still update atomically through Git and failures remain fail-closed.
|
||||
- Dubious ownership returns a stable, actionable service/data-ownership diagnostic.
|
||||
- Other Git failures retain bounded useful context while redacting known secrets and HTTP(S) URL
|
||||
userinfo. Successful current origin values are never logged or returned.
|
||||
- Kandev does not add `safe.directory`, retry as another user, or modify filesystem ownership.
|
||||
|
||||
## TDD Sequence
|
||||
|
||||
1. Add real-Git no-op and changed-origin cases; prove the current implementation rewrites or
|
||||
invokes the write boundary in the no-op case and record RED.
|
||||
2. Add pure diagnostic-classifier tests for dubious ownership, `config.lock`, bounds, URL-userinfo,
|
||||
and known-secret redaction; record RED.
|
||||
3. Implement inspection/no-op and credential-safe classification inside `repoclone` while retaining
|
||||
lock coverage across read and conditional write.
|
||||
4. Re-run concurrency and invalid-repository tests to prevent regression.
|
||||
5. Run the commands below and record GREEN.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cd apps/backend && go test ./internal/repoclone -run 'Test.*(SetOriginURL|RepositoryGitError|RedactGitDiagnostic)' -count=1
|
||||
cd apps/backend && go test ./internal/repoclone -count=1
|
||||
```
|
||||
|
||||
## Files Likely Touched
|
||||
|
||||
- `apps/backend/internal/repoclone/clone.go`
|
||||
- `apps/backend/internal/repoclone/clone_test.go`
|
||||
- a focused diagnostic helper/test file if needed
|
||||
|
||||
## Dependencies
|
||||
|
||||
None.
|
||||
|
||||
## Parallelism
|
||||
|
||||
Production files are disjoint from Task 01, but execution remains in the primary conversation
|
||||
unless the user explicitly authorizes delegation.
|
||||
|
||||
## Recorded Results
|
||||
|
||||
- RED: the matching-origin and diagnostic tests failed before the origin inspection/no-op and
|
||||
diagnostic classifier existed; the ownership case could only report a bare exit status.
|
||||
- GREEN: `cd apps/backend && go test ./internal/repoclone -run 'Test.*(SetOriginURL|RepositoryGitError|RedactGitDiagnostic|RedactCloneOutput)' -count=1` passed 7 cases; `cd apps/backend && go test ./internal/repoclone -count=1` passed 48 tests.
|
||||
- `SetOriginURL` now serializes read/compare/write, skips a matching origin, classifies dubious
|
||||
ownership with `errors.Is`, and bounds/redacts Git output, including complete multi-token
|
||||
`Authorization` values.
|
||||
|
||||
## Output Contract
|
||||
|
||||
Report the RED failures, exact no-op boundary, diagnostic/redaction examples using synthetic
|
||||
secrets only, final GREEN results, and remaining Git-platform differences. Update task and plan
|
||||
status.
|
||||
+74
@@ -0,0 +1,74 @@
|
||||
---
|
||||
id: "03-resolve-resume-repositories-once"
|
||||
title: "Resolve resume repositories once"
|
||||
status: done
|
||||
wave: 2
|
||||
depends_on: ["02-harden-origin-reconciliation"]
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/integrations/github-authentication.md"
|
||||
---
|
||||
|
||||
# Task 03: Resolve Resume Repositories Once
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Resume resolves the attached repository set once and reuses it for primary configuration,
|
||||
multi-repository configuration, and GitHub credential routing.
|
||||
- Each attached managed repository reaches preparation/origin reconciliation at most once per
|
||||
resume.
|
||||
- Persisted session primary selection, old-session fallback, repository ordering, branch metadata,
|
||||
repo-less tasks, and inherited task environments retain current behavior.
|
||||
- A legitimate persisted primary absent from current attachments is resolved at most once through
|
||||
an explicit exceptional path rather than triggering another full-set resolution.
|
||||
- Initial launch behavior and user-managed local checkout behavior remain unchanged.
|
||||
|
||||
## TDD Sequence
|
||||
|
||||
1. Add an instrumented multi-repository resume test that counts preparation calls and record the
|
||||
current repeated-primary RED result.
|
||||
2. Add cases for persisted primary selection, old-session fallback, repo-less resume, and a
|
||||
persisted primary absent from the attachment set.
|
||||
3. Refactor `buildResumeRequest` to resolve once and pass the result into primary, multi-repository,
|
||||
and credential consumers.
|
||||
4. Retain existing transport transition, default-branch, task-directory, and environment tests.
|
||||
5. Run the focused and package commands below and record GREEN.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
cd apps/backend && go test ./internal/orchestrator/executor -run 'Test.*(Resume.*Repositor|BuildResumeRequest|RepoLocalPath.*Origin)' -count=1
|
||||
cd apps/backend && go test ./internal/orchestrator/executor -count=1
|
||||
```
|
||||
|
||||
## Files Likely Touched
|
||||
|
||||
- `apps/backend/internal/orchestrator/executor/executor_resume.go`
|
||||
- `apps/backend/internal/orchestrator/executor/executor_environment_test.go`
|
||||
- `apps/backend/internal/orchestrator/executor/executor_multi_repo_test.go`
|
||||
- `apps/backend/internal/orchestrator/executor/executor_resume_clone_transport_test.go`
|
||||
- a focused resume repository-resolution test file if clearer
|
||||
|
||||
## Dependencies
|
||||
|
||||
Task 02 establishes the no-op and error contract used by each preparation call.
|
||||
|
||||
## Parallelism
|
||||
|
||||
Sequential after Task 02 because both define the origin-reconciliation call boundary observed by
|
||||
resume tests.
|
||||
|
||||
## Recorded Results
|
||||
|
||||
- RED: the multi-repository resume regression observed 5 origin-reconciliation calls for 2
|
||||
attached repositories because primary preparation, multi-repository configuration, and the outer
|
||||
credential-resolution pass each materialized the set.
|
||||
- GREEN: the regression now observes 2 calls, one per attached repository; the full executor package
|
||||
passed 326 tests.
|
||||
- The resume path now shares the resolved repository set across primary request fields, multi-repo
|
||||
specs, and GitHub credential routing. A persisted primary absent from current attachments keeps a
|
||||
single explicit fallback lookup.
|
||||
|
||||
## Output Contract
|
||||
|
||||
Report the RED call counts, final resolution flow, preserved fallback behavior, exact GREEN results,
|
||||
and any remaining exceptional additional lookup. Update task and plan status.
|
||||
+72
@@ -0,0 +1,72 @@
|
||||
---
|
||||
id: "04-document-service-identity-recovery"
|
||||
title: "Document service identity recovery"
|
||||
status: done
|
||||
wave: 3
|
||||
depends_on:
|
||||
- "01-preserve-system-service-user"
|
||||
- "02-harden-origin-reconciliation"
|
||||
- "03-resolve-resume-repositories-once"
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/native-kandev-cli/spec.md"
|
||||
---
|
||||
|
||||
# Task 04: Document Service Identity Recovery
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Public service documentation explains preserved identity, `--run-as`, first root-shell install
|
||||
behavior, and safe recovery from a service/home owner mismatch.
|
||||
- CLI reference documents the flag and rejects the implication that it applies to user services or
|
||||
non-install actions.
|
||||
- GitHub integration documentation identifies managed-checkout ownership errors and does not
|
||||
recommend `safe.directory=*` or automatic chown.
|
||||
- Public-doc metadata/links remain valid and both documentation validators pass.
|
||||
|
||||
## TDD Sequence
|
||||
|
||||
1. Update public service, CLI, and integration pages to match the implemented command and error
|
||||
wording.
|
||||
2. Add or update validator assertions if the public CLI contract has structured coverage.
|
||||
3. Run both validators and fix only issues caused by these documentation changes.
|
||||
4. Mark every task and the parent plan complete only after implementation verification is recorded.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
node --test scripts/validate-public-docs.test.mjs
|
||||
node scripts/validate-public-docs.mjs
|
||||
```
|
||||
|
||||
## Files Likely Touched
|
||||
|
||||
- `docs/public/run-as-a-service.md`
|
||||
- `docs/public/cli.md`
|
||||
- `docs/public/integrations.md`
|
||||
- `scripts/validate-public-docs.test.mjs` only if contract coverage requires it
|
||||
- this task file and `plan.md`
|
||||
|
||||
## Dependencies
|
||||
|
||||
Tasks 01-03, so documentation uses the final CLI and diagnostic wording.
|
||||
|
||||
## Parallelism
|
||||
|
||||
Sequential after behavior tasks.
|
||||
|
||||
## Recorded Results
|
||||
|
||||
- Updated `docs/public/run-as-a-service.md`, `docs/public/cli.md`, and
|
||||
`docs/public/integrations.md` with identity preservation, `--run-as`, ownership preflight, and
|
||||
dubious-ownership recovery guidance.
|
||||
- `node --test scripts/validate-public-docs.test.mjs` passed all 58 structural tests.
|
||||
- `node scripts/validate-public-docs.mjs` validated all 41 published pages.
|
||||
- Final backend verification passed: `cd apps/backend && make test` and `cd apps/backend && make lint`.
|
||||
- Operator migration caveat: existing homes with an owner mismatch require explicit operator
|
||||
reconciliation before reinstall; Kandev does not auto-chown or add Git trust exceptions.
|
||||
|
||||
## Output Contract
|
||||
|
||||
Report pages changed, both validator results, final backend verification results from the parent
|
||||
plan, and any operator migration caveat. Mark the plan complete only when all task acceptance
|
||||
criteria are satisfied.
|
||||
+1
-1
@@ -143,7 +143,7 @@ kandev service status
|
||||
kandev service logs --follow
|
||||
```
|
||||
|
||||
Supported actions are `install`, `uninstall`, `start`, `stop`, `restart`, `status`, `logs`, and `config`. Installation accepts `--system`, `--port`, `--home-dir`, and `--no-boot-start`. In the current native installer, `--port` is written as `KANDEV_SERVER_PORT`, but the supervising launcher overwrites that value with its own automatic port selection; the option therefore does not reliably pin a service listener today. The service normally prefers `38429` and falls back when it is busy. Windows service installation is not implemented. Managed user services write owner-only install metadata so **Settings > System > Updates** can offer the guarded Apply action; system services and failed guarded updates use the package manager followed by `kandev service install` and `kandev service restart`. See [Run as a service](./run-as-a-service.md) for privileges, paths, upgrades, and recovery.
|
||||
Supported actions are `install`, `uninstall`, `start`, `stop`, `restart`, `status`, `logs`, and `config`. Installation accepts `--system`, `--run-as <user>` (only with `--system`), `--port`, `--home-dir`, and `--no-boot-start`. Reinstalling an existing Kandev-managed system service preserves its account unless `--run-as` is supplied explicitly. A first system install from a root login requires `--run-as`, including `--run-as root` when root is intentional. In the current native installer, `--port` is written as `KANDEV_SERVER_PORT`, but the supervising launcher overwrites that value with its own automatic port selection; the option therefore does not reliably pin a service listener today. The service normally prefers `38429` and falls back when it is busy. Windows service installation is not implemented. Managed user services write owner-only install metadata so **Settings > System > Updates** can offer the guarded Apply action; system services and failed guarded updates use the package manager followed by `kandev service install` and `kandev service restart`. See [Run as a service](./run-as-a-service.md) for privileges, paths, upgrades, and recovery.
|
||||
|
||||
## Ports and network exposure
|
||||
|
||||
|
||||
@@ -119,6 +119,13 @@ path on desktop hover or focus and in a touch-accessible drawer on mobile.
|
||||
match `remote.*.url` apply; switching back to managed credentials restores the canonical HTTPS
|
||||
origin. Repositories you registered from an existing local checkout are never rewritten.
|
||||
|
||||
If Git rejects a managed checkout with **detected dubious ownership**, the Kandev service account
|
||||
and the checkout owner do not match. Repository preparation stops and the session error identifies
|
||||
the ownership boundary; inspect the service unit and the managed home/repository owner, then
|
||||
preserve or deliberately reconcile the service account. The diagnostic is credential-safe and
|
||||
bounded. Do not add `safe.directory=*` or let Kandev recursively chown the checkout: those actions
|
||||
hide an installation identity problem instead of fixing it.
|
||||
|
||||
Selecting a GitHub CLI workspace connection does not implicitly select host Git credentials; use
|
||||
the explicit inheritance mode when that is the intended boundary. The Changes panel's branch
|
||||
details show the non-secret identity and delivery route captured for a successfully launched
|
||||
|
||||
@@ -20,7 +20,7 @@ Install Kandev first using a persistent [CLI installation](cli.md#install). Do n
|
||||
| Linux unit | `~/.config/systemd/user/kandev.service` | `/etc/systemd/system/kandev.service` |
|
||||
| macOS plist | `~/Library/LaunchAgents/com.kdlbs.kandev.plist` | `/Library/LaunchDaemons/com.kdlbs.kandev.plist` |
|
||||
| Default Kandev home | The installer process's `KANDEV_HOME_DIR` when set; otherwise `~/.kandev` | `/var/lib/kandev` |
|
||||
| Process user | Current user | `$SUDO_USER` when installed through `sudo` from a non-root account; otherwise the current user |
|
||||
| Process user | Current user | Existing Kandev-managed unit/plist account on reinstall; otherwise non-root `$SUDO_USER` when installed through `sudo`. A root login must choose `--run-as` explicitly. |
|
||||
| Best fit | Personal workstation; single-user Linux host with lingering enabled | Boot-time service independent of a login session |
|
||||
|
||||
On Linux, an enabled user service starts at boot only if that user's systemd manager runs at boot. Enable lingering once if that is the desired lifecycle:
|
||||
@@ -51,16 +51,30 @@ curl --fail http://127.0.0.1:43127/health
|
||||
|
||||
### System service
|
||||
|
||||
The native installer does not create or change ownership of the system service home. Create it for the account that will run the service before installing. The following invocation through `sudo` makes that account the invoking user:
|
||||
The native installer does not create or change ownership of the system service home. Create it for the account that will run the service before installing. Select that account explicitly with `--run-as`:
|
||||
|
||||
```bash
|
||||
KANDEV_BIN="$(command -v kandev)"
|
||||
sudo install -d -o "$USER" -g "$(id -gn)" /var/lib/kandev /var/lib/kandev/logs
|
||||
sudo "$KANDEV_BIN" service install --system
|
||||
sudo "$KANDEV_BIN" service install --system --run-as "$USER"
|
||||
sudo "$KANDEV_BIN" service status --system
|
||||
```
|
||||
|
||||
Apply the same ownership rule to a custom `--home-dir`. Root access is also required to write the system unit/plist and control the system service manager. If installation is run from a root login, rather than via `sudo` from the intended account, the service runs as root; prefer an explicit, non-root service account.
|
||||
Apply the same ownership rule to a custom `--home-dir`. Root access is also required to write the system unit/plist and control the system service manager. A root login must pass an explicit non-root account when that is the intended service identity.
|
||||
|
||||
The installer preserves the account from an existing Kandev-managed systemd unit or launchd plist
|
||||
when you reinstall after an upgrade. It does not infer a new account from the shell that happens
|
||||
to run the update. On a first install from a root login, supply the account explicitly:
|
||||
|
||||
```bash
|
||||
sudo kandev service install --system --run-as brewuser --home-dir /var/lib/kandev
|
||||
```
|
||||
|
||||
Use `--run-as root` only when a root service is intentional. `--run-as` is valid only with
|
||||
`service install --system`; it cannot change a user service. Kandev checks that the system home
|
||||
already exists, is not a symlink, and is owned by the selected account before replacing or
|
||||
restarting the service. Missing or mismatched ownership fails with guidance; Kandev does not
|
||||
recursively chown the data tree or add a broad Git `safe.directory` exception.
|
||||
|
||||
## Bind safely
|
||||
|
||||
@@ -84,7 +98,7 @@ Then open `http://127.0.0.1:38429` locally. For shared access, terminate TLS and
|
||||
## Commands and flags
|
||||
|
||||
```text
|
||||
kandev service install [--system] [--port <port>] [--home-dir <path>] [--no-boot-start]
|
||||
kandev service install [--system] [--run-as <user>] [--port <port>] [--home-dir <path>] [--no-boot-start]
|
||||
kandev service uninstall [--system]
|
||||
kandev service start|stop|restart|status [--system]
|
||||
kandev service logs [-f] [--system]
|
||||
@@ -92,6 +106,9 @@ kandev service config [--system]
|
||||
```
|
||||
|
||||
- `--system` selects the system manager and paths shown above.
|
||||
- `--run-as <user>` is valid only for `service install --system`. Reinstalling without it preserves
|
||||
the account in the existing Kandev-managed unit/plist; on a first root-shell install it is
|
||||
required, including `--run-as root` when root is intentional.
|
||||
- `--home-dir <path>` records `KANDEV_HOME_DIR` in the unit. For a user service with no flag, the installer first honors its own `KANDEV_HOME_DIR` environment, then falls back to `~/.kandev`; system mode defaults to `/var/lib/kandev`.
|
||||
- On Linux, `--no-boot-start` starts the service now without enabling the unit, but it does not disable a unit that was already enabled. Disable it explicitly when necessary. On macOS the generated plist has `RunAtLoad=false` but also `KeepAlive=true`; the installer stores it in launchd's normal discovery path, bootstraps and enables it, then kick-starts it. That combination does not provide a dependable “never start at login/boot” guarantee; manage future loading explicitly with launchd if that distinction matters.
|
||||
- `-f` or `--follow` follows logs.
|
||||
@@ -180,6 +197,11 @@ sudo "$KANDEV_BIN" service restart --system
|
||||
sudo "$KANDEV_BIN" service status --system
|
||||
```
|
||||
|
||||
When reinstalling an existing system service, omit `--run-as` to preserve its recorded account, or
|
||||
provide it only for an intentional account migration. Before an intentional migration, reconcile
|
||||
the owner of `/var/lib/kandev` (or the custom home) yourself; the installer refuses to rewrite a
|
||||
service whose data owner does not match the selected account.
|
||||
|
||||
Reinstalling an already active Linux unit performs `enable --now`, which does not guarantee a restart of that running process. The explicit `restart` is required to load the new executable. Back up persistent state before upgrades; see [SQLite backups](operations.md#sqlite-backups) or the PostgreSQL procedure under [Restore and recovery](operations.md#restore-and-recovery).
|
||||
|
||||
## Run a source checkout as a service
|
||||
@@ -223,6 +245,22 @@ sudo namei -l /var/lib/kandev
|
||||
|
||||
Stop the service before correcting ownership, and grant access only to the intended service account.
|
||||
|
||||
### Git reports dubious ownership or origin setup exits with status 128
|
||||
|
||||
This means the Kandev service account and a managed checkout have different filesystem owners. It
|
||||
is common after an upgrade if the service was reinstalled from a root login while the existing
|
||||
checkout remained owned by the previous account. Check both sides before changing anything:
|
||||
|
||||
```bash
|
||||
sudo systemctl cat kandev.service
|
||||
sudo namei -l /var/lib/kandev/repos
|
||||
sudo find /var/lib/kandev/repos -maxdepth 3 -type d -name .git -print
|
||||
```
|
||||
|
||||
Reinstall without `--run-as` to preserve the existing service account, or deliberately reconcile the
|
||||
home and checkout ownership before using an explicit `--run-as`. Do not fix this with
|
||||
`safe.directory=*`; that hides the identity mismatch and weakens Git's ownership protection.
|
||||
|
||||
### Port 38429 is not the logged URL
|
||||
|
||||
The native launcher selects another free port when `38429` is unavailable. Check `kandev service logs` (or its `--system` form), stop the conflicting process if appropriate, and restart. The current installer `--port` value does not reliably control this selection.
|
||||
|
||||
@@ -15,9 +15,11 @@ token. A later retry therefore starts a new provider-native conversation even
|
||||
though the original session remains valid.
|
||||
|
||||
A failed or cancelled GitHub-backed session also requests a new scoped
|
||||
credential lease while its persisted state is still terminal. The credential
|
||||
broker correctly rejects terminal sessions, so the user-visible Resume action
|
||||
cannot reach the agent launch.
|
||||
credential lease while its persisted state is still terminal. Resume assembles
|
||||
the request before persisting `STARTING`, and request assembly itself issues
|
||||
the lease; moving the transition only ahead of `LaunchAgent` therefore leaves
|
||||
the broker boundary too early. The credential broker correctly rejects the
|
||||
terminal session, so the user-visible Resume action cannot reach agent launch.
|
||||
|
||||
Managed npm runtimes can retain a truncated or otherwise corrupt extracted
|
||||
`_npx` execution tree even when npm's content-addressable package cache is
|
||||
@@ -32,11 +34,15 @@ without repairing it.
|
||||
**Start fresh**, or is replaced after the agent successfully creates a new
|
||||
provider-native session.
|
||||
- An authorized resume moves the task session to `STARTING` under the existing
|
||||
per-session resume lock before launching the agent. This makes the session
|
||||
eligible for a scoped GitHub credential lease without weakening the
|
||||
credential broker's terminal-session rejection.
|
||||
- If launch fails after that early transition, Kandev restores the prior
|
||||
recoverable session state unless another terminal transition won the race.
|
||||
per-session resume lock before request assembly reaches scoped GitHub
|
||||
credential issuance. This makes the session eligible for a lease without
|
||||
weakening the credential broker's terminal-session rejection.
|
||||
- A successful resume persists the non-secret Git credential routing snapshot
|
||||
while the session is still guarded `STARTING`, so the task detail view does
|
||||
not retain an earlier workspace/executor credential policy.
|
||||
- If request assembly, credential issuance, or launch fails after that early
|
||||
transition, Kandev restores the prior recoverable session state unless
|
||||
another terminal transition won the race.
|
||||
- The explicit managed-runtime update path may invalidate only the
|
||||
deterministic `_npx` execution directory for the selected built-in package
|
||||
after an initial update failure, then retry once and run the normal ACP
|
||||
@@ -67,11 +73,16 @@ without repairing it.
|
||||
`session/load` completes, **WHEN** the attempt fails, **THEN** Kandev retains
|
||||
the token so a later healthy process can retry it.
|
||||
- **GIVEN** a failed GitHub-backed session, **WHEN** the user selects Resume,
|
||||
**THEN** the session is persisted as `STARTING` before the credential lease
|
||||
is requested and the launch can proceed.
|
||||
- **GIVEN** that launch fails after the early `STARTING` transition, **WHEN**
|
||||
recovery handling completes, **THEN** the session is recoverable and no
|
||||
stale `STARTING` state remains.
|
||||
**THEN** the session is persisted as `STARTING` before
|
||||
`buildResumeRequest` requests the credential lease and the launch can
|
||||
proceed.
|
||||
- **GIVEN** a resume selects a credential policy different from the previous
|
||||
attempt, **WHEN** credential setup succeeds, **THEN** the non-secret
|
||||
`git_credential_snapshot` is persisted before launch and reflects the new
|
||||
policy.
|
||||
- **GIVEN** that request construction, credential issuance, or launch fails
|
||||
after the early `STARTING` transition, **WHEN** recovery handling completes,
|
||||
**THEN** the session is recoverable and no stale `STARTING` state remains.
|
||||
- **GIVEN** an extracted managed npm runtime is corrupt, **WHEN** the first
|
||||
explicit update attempt fails, **THEN** only that package's deterministic
|
||||
execution directory is invalidated, the update runs once more, and success
|
||||
|
||||
@@ -65,6 +65,15 @@ automation under different GitHub Apps without operating separate Kandev deploym
|
||||
an existing managed checkout when the policy changes. This makes Git conditional includes based
|
||||
on `remote.*.url` observe the same transport the task uses. Kandev never rewrites the remote of a
|
||||
repository registered as a user-managed local checkout.
|
||||
- Repository preparation resolves each attached repository once per launch or resume and reuses
|
||||
that result for primary-repository configuration, multi-repository configuration, and credential
|
||||
routing. Origin reconciliation is serialized per managed checkout, compares the current and
|
||||
desired canonical URLs, and performs no write when they already match.
|
||||
- Git failures while inspecting or reconciling a managed checkout preserve a bounded,
|
||||
credential-redacted diagnostic. Git's dubious-ownership failure is classified as a service/data
|
||||
ownership mismatch with guidance to restore the intended Kandev service account or reconcile the
|
||||
managed data owner. Kandev does not bypass Git's ownership protection with a broad
|
||||
`safe.directory` entry.
|
||||
- Under managed routing, App installation tokens are minted for the requested repository and cached
|
||||
only in memory. PAT/CLI tokens retain their provider-granted scope once delivered to a trusted
|
||||
agent subprocess. GitHub HTTPS and the broker-aware `gh` shim fail closed rather than consulting
|
||||
@@ -345,6 +354,9 @@ post-signature processing failures produce `failing`; a later valid successful d
|
||||
- `executor -> executor-visible credentials`, regardless of PAT/CLI/App automation method.
|
||||
- Initial launch and resume run the same resolution. A successful operation replaces the session
|
||||
snapshot; a failed operation leaves the previous snapshot unchanged.
|
||||
- Each attached repository is resolved once for an individual launch or resume. The resolved
|
||||
repository set is shared by task configuration and credential-snapshot construction rather than
|
||||
triggering repeated materialization or origin mutation.
|
||||
- Changing the workspace policy affects new launches and the next resume, not an already-running
|
||||
process.
|
||||
|
||||
@@ -407,6 +419,12 @@ post-signature processing failures produce `failing`; a later valid successful d
|
||||
- If Kandev cannot reconcile a managed checkout's `origin` with the selected task policy, Local and
|
||||
Worktree preparation fails before the agent starts instead of silently using the other policy's
|
||||
transport.
|
||||
- If Git rejects a managed checkout because its filesystem owner differs from the Kandev service
|
||||
account, preparation fails with an ownership-specific, credential-safe diagnostic. Kandev does
|
||||
not retry with a global trust override, suppress the Git check, or mutate filesystem ownership.
|
||||
- Generic origin-inspection and origin-update failures include only bounded, credential-redacted
|
||||
Git output. Credentials embedded in URL userinfo or known authentication material never appear
|
||||
in logs, session errors, or browser payloads.
|
||||
- Deleting a registration with any workspace or personal reference returns
|
||||
`github_app_registration_in_use` with a non-secret binding count.
|
||||
- Changing workspace auth while a flow is open makes the stale callback fail without reverting the
|
||||
@@ -546,6 +564,19 @@ registration and never creates a global default.
|
||||
- **GIVEN** a Kandev-managed GitHub checkout currently has an SSH `origin`, **WHEN** the workspace
|
||||
selects managed credentials and launches a Local or Worktree task, **THEN** Kandev changes that
|
||||
managed checkout's `origin` to canonical HTTPS before task preparation.
|
||||
- **GIVEN** a Kandev-managed checkout already has the canonical origin selected by the task policy,
|
||||
**WHEN** a task launches or resumes, **THEN** Kandev inspects the origin but does not rewrite
|
||||
`.git/config`.
|
||||
- **GIVEN** a task with one or more attached repositories, **WHEN** launch or resume builds the
|
||||
primary, multi-repository, and credential configuration, **THEN** each repository is prepared
|
||||
once and the same resolved result is reused by all three consumers.
|
||||
- **GIVEN** a managed checkout is owned by `brewuser` while the Kandev service runs as root,
|
||||
**WHEN** Git rejects origin inspection or reconciliation as dubious ownership, **THEN** task
|
||||
preparation stops and reports that the service account and managed repository owner disagree,
|
||||
without suggesting `safe.directory=*`.
|
||||
- **GIVEN** Git emits a failure containing credential-bearing URL userinfo, **WHEN** the failure is
|
||||
returned through task preparation, **THEN** the diagnostic retains actionable Git context but
|
||||
redacts the credential and bounds the output length.
|
||||
- **GIVEN** a repository is registered from a user-managed local checkout, **WHEN** either task Git
|
||||
credential policy is selected, **THEN** Kandev leaves its configured `origin` unchanged.
|
||||
- **GIVEN** managed mode and an explicit executor-profile GitHub token, **WHEN** a task launches,
|
||||
@@ -614,10 +645,14 @@ and the
|
||||
[task Git credential policy follow-up plan](../../plans/task-git-credential-policy/plan.md), plus
|
||||
the
|
||||
[executor clone transport repair plan](../../plans/github-executor-clone-transport/plan.md), and
|
||||
the [managed task terminal environment plan](../../plans/task-terminal-git-environment/plan.md).
|
||||
the [managed task terminal environment plan](../../plans/task-terminal-git-environment/plan.md),
|
||||
and the
|
||||
[system-service identity guardrails repair plan](../../plans/system-service-identity-guardrails/plan.md).
|
||||
|
||||
## Decision
|
||||
|
||||
See [ADR-2026-07-21-workspace-selectable-github-app-registrations](../../decisions/2026-07-21-workspace-selectable-github-app-registrations.md)
|
||||
and
|
||||
[ADR-2026-07-27-task-git-credential-policy](../../decisions/2026-07-27-task-git-credential-policy.md).
|
||||
The system-service ownership boundary is defined by
|
||||
[ADR-2026-07-31-system-service-user-continuity](../../decisions/2026-07-31-system-service-user-continuity.md).
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
---
|
||||
status: draft
|
||||
created: 2026-06-15
|
||||
amended: 2026-07-31
|
||||
owner: tbd
|
||||
---
|
||||
|
||||
@@ -22,6 +23,15 @@ Users start Kandev from the terminal through `kandev`, but the current launcher
|
||||
- npm/npx continues to expose the `kandev` command through the `kandev` npm package. The npm package may use a minimal JavaScript shim, but that shim only resolves the platform runtime package and execs its native `bin/kandev`.
|
||||
- `make start` launches through the native `apps/backend/bin/kandev start` path after building local artifacts.
|
||||
- Existing service installs continue to be managed by `kandev service ...`; newly generated service units execute the public `kandev` launcher path.
|
||||
- Reinstalling or updating a system service preserves the service account recorded by the existing
|
||||
root-controlled Kandev unit or plist. The account changes only when the operator supplies
|
||||
`--run-as <user>` explicitly.
|
||||
- A first system install invoked from a root login does not silently select root. It requires an
|
||||
explicit `--run-as <user>`; selecting root is allowed only as the explicit `--run-as root`
|
||||
choice. A first install through `sudo` may continue to select the non-root invoking user.
|
||||
- Before rewriting or restarting a system service, the installer verifies that the configured
|
||||
Kandev home is owned by the selected service account. A mismatch fails with recovery guidance;
|
||||
the installer does not recursively change ownership.
|
||||
- Service units preserve install-time Node/npm/npx bin directories when they are discoverable, so agents can still invoke npm-managed CLIs under service managers that do not source shell profiles.
|
||||
- Startup output continues to show the URL users should open, the MCP URL, database path when applicable, and log level when applicable.
|
||||
- Production `run` and `start` do not execute a Node.js web runtime; the Go backend serves the embedded Vite SPA assets.
|
||||
@@ -34,7 +44,8 @@ Users start Kandev from the terminal through `kandev`, but the current launcher
|
||||
kandev [--port <port>] [--verbose] [--debug] [--headless]
|
||||
kandev run [--port <port>] [--verbose] [--debug] [--headless]
|
||||
kandev start [--port <port>] [--verbose] [--debug] [--headless]
|
||||
kandev service install|uninstall|start|stop|restart|status|logs|config [--system]
|
||||
kandev service install [--system] [--run-as <user>] [--port <port>] [--home-dir <path>] [--no-boot-start]
|
||||
kandev service uninstall|start|stop|restart|status|logs|config [--system]
|
||||
kandev --version
|
||||
kandev --help
|
||||
```
|
||||
@@ -52,6 +63,16 @@ Logging/debug flags:
|
||||
- `--debug` shows debug logs and enables current debug environment behavior.
|
||||
- `--headless` skips browser opening and prints the ready URL.
|
||||
|
||||
System-service identity flags:
|
||||
|
||||
- `--run-as <user>` is valid only for `kandev service install --system`.
|
||||
- On reinstall, omitting `--run-as` preserves the account in the existing Kandev-managed
|
||||
root-controlled unit or plist, even when the update command is run from a different login.
|
||||
- On a first install, omitting `--run-as` selects a non-root `SUDO_USER` when available. A root
|
||||
login without that context must choose explicitly, including when the desired account is root.
|
||||
- The service-owned `service/install.json` metadata records the effective account but is not
|
||||
trusted by a privileged installer as authority for choosing a system account.
|
||||
|
||||
### Hidden backend mode
|
||||
|
||||
The native launcher has a private backend mode:
|
||||
@@ -126,10 +147,24 @@ Transitions:
|
||||
- If the backend does not become healthy before timeout, the launcher exits non-zero and includes captured backend output when running in quiet mode.
|
||||
- If the backend child exits unexpectedly after startup, the launcher either restarts it through the supervisor path when the exit is restart-controlled or shuts down the app with the backend exit code.
|
||||
- If npm/npx optional runtime resolution fails, the JavaScript shim exits non-zero with an actionable message explaining that the platform runtime package is missing.
|
||||
- If a privileged reinstall finds an existing Kandev-managed system unit or plist, it fails before
|
||||
rewriting or restarting the service when that root-controlled definition is malformed or its
|
||||
account cannot be resolved.
|
||||
- If a first system install is run from a root login without `--run-as`, it exits with usage
|
||||
guidance instead of silently creating a root service.
|
||||
- If the configured system Kandev home owner does not match the selected service account, install
|
||||
exits before changing the service definition and explains how to preserve the existing account
|
||||
or deliberately reconcile ownership. It does not add a global Git trust exception or chown the
|
||||
data tree automatically.
|
||||
|
||||
## Persistence guarantees
|
||||
|
||||
- Service installation metadata remains durable across restarts and upgrades.
|
||||
- The effective system-service account remains stable across package upgrades and repeated
|
||||
`service install --system` calls unless an operator explicitly changes it with `--run-as`.
|
||||
- For an existing system service, the root-controlled unit or plist is authoritative for service
|
||||
identity. Service-owned metadata may corroborate that identity but cannot authorize a
|
||||
privileged identity change.
|
||||
- Supervisor manifest/control files remain under the configured Kandev home directory and are overwritten on each launch with the current executable path and backend argv.
|
||||
- The launcher does not persist child process state beyond the existing supervisor manifest. After a launcher restart, it starts a fresh backend child.
|
||||
- User data, database files, task worktrees, and backend-managed persistence remain owned by existing backend persistence behavior; the launcher migration does not change their retention.
|
||||
@@ -143,6 +178,21 @@ Transitions:
|
||||
- **GIVEN** the backend has requested a restart through the restart adapter, **WHEN** the launcher receives the restart request, **THEN** only the `kandev __backend` child process is replaced.
|
||||
- **GIVEN** the user presses Ctrl-C while Kandev is running, **WHEN** the launcher handles the signal, **THEN** it terminates the backend child process before exiting.
|
||||
- **GIVEN** a new service install on Linux or macOS, **WHEN** the user runs `kandev service install`, **THEN** the generated service unit executes the public `kandev` launcher path.
|
||||
- **GIVEN** a system service already runs as `brewuser`, **WHEN** root later runs
|
||||
`kandev service install --system` during an upgrade without `--run-as`, **THEN** the generated
|
||||
definition still runs as `brewuser`.
|
||||
- **GIVEN** an existing system service, **WHEN** an operator intentionally supplies
|
||||
`--run-as another-user`, **THEN** Kandev validates that account and the configured home owner
|
||||
before changing the service definition.
|
||||
- **GIVEN** no existing system service and no non-root `SUDO_USER`, **WHEN** root runs
|
||||
`kandev service install --system` without `--run-as`, **THEN** installation fails and asks for an
|
||||
explicit account rather than defaulting to root.
|
||||
- **GIVEN** a selected service account whose UID differs from the configured system home owner,
|
||||
**WHEN** installation runs, **THEN** it fails before rewriting or restarting the service and does
|
||||
not recursively chown the home.
|
||||
- **GIVEN** service-owned install metadata names a more privileged account than the existing
|
||||
root-controlled service definition, **WHEN** a privileged reinstall runs, **THEN** the metadata
|
||||
cannot change the selected account.
|
||||
- **GIVEN** a native user-service install on Linux or macOS, **WHEN** the service starts, **THEN** its environment and durable install metadata identify it as a kandev-managed service and the guarded System-page updater can launch the native self-update helper.
|
||||
- **GIVEN** Node is managed by nvm/fnm/asdf/volta/mise and `node`, `npm`, or `npx` resolve from that manager at install time, **WHEN** the user runs `kandev service install`, **THEN** the generated service environment includes the detected Node tool bin directory in `PATH`.
|
||||
- **GIVEN** a user runs `npx kandev@latest --help`, **WHEN** npm has installed the platform runtime package, **THEN** the npm shim execs the runtime package's native `bin/kandev` and the user sees the same public help output.
|
||||
@@ -153,6 +203,20 @@ Transitions:
|
||||
- Introducing `kandevctl` as a user-facing command.
|
||||
- Requiring a separate shipped backend binary such as `kandev-backend`.
|
||||
- Changing task, workflow, agent, or integration behavior after the backend has started.
|
||||
- Automatically changing ownership, managing ACLs, or adding broad Git `safe.directory` entries
|
||||
when a system-service identity mismatch is detected.
|
||||
- Managing Windows services or inferring service identity from an untrusted service-owned metadata
|
||||
file.
|
||||
|
||||
## Repair plan
|
||||
|
||||
See the
|
||||
[system-service identity guardrails plan](../../plans/system-service-identity-guardrails/plan.md).
|
||||
|
||||
## Decision
|
||||
|
||||
System-service account continuity and ownership validation follow
|
||||
[ADR-2026-07-31-system-service-user-continuity](../../decisions/2026-07-31-system-service-user-continuity.md).
|
||||
|
||||
## Open questions
|
||||
|
||||
|
||||
Reference in New Issue
Block a user