diff --git a/dist/pullpreview-linux-amd64 b/dist/pullpreview-linux-amd64 index 18bfae3..bf08c16 100755 Binary files a/dist/pullpreview-linux-amd64 and b/dist/pullpreview-linux-amd64 differ diff --git a/internal/providers/lightsail/lightsail.go b/internal/providers/lightsail/lightsail.go index 92ea0dd..bcf2cf4 100644 --- a/internal/providers/lightsail/lightsail.go +++ b/internal/providers/lightsail/lightsail.go @@ -306,6 +306,7 @@ func (p *Provider) fetchAccessDetails(name string) (pullpreview.AccessDetails, e IPAddress: aws.ToString(resp.AccessDetails.IpAddress), CertKey: aws.ToString(resp.AccessDetails.CertKey), PrivateKey: aws.ToString(resp.AccessDetails.PrivateKey), + ExpiresAt: aws.ToTime(resp.AccessDetails.ExpiresAt), }, nil } diff --git a/internal/providers/lightsail/lightsail_test.go b/internal/providers/lightsail/lightsail_test.go index fd16032..c053b31 100644 --- a/internal/providers/lightsail/lightsail_test.go +++ b/internal/providers/lightsail/lightsail_test.go @@ -4,6 +4,7 @@ import ( "context" "strings" "testing" + "time" "github.com/aws/aws-sdk-go-v2/aws" ls "github.com/aws/aws-sdk-go-v2/service/lightsail" @@ -281,6 +282,30 @@ func TestLaunchInstanceCreatesFreshInstanceForCompose(t *testing.T) { } } +func TestFetchAccessDetailsIncludesExpiry(t *testing.T) { + expiresAt := time.Now().UTC().Add(10 * time.Minute).Truncate(time.Second) + client := &fakeLightsailClient{ + getInstanceAccessDetailsOutput: &ls.GetInstanceAccessDetailsOutput{ + AccessDetails: &types.InstanceAccessDetails{ + Username: aws.String("ec2-user"), + IpAddress: aws.String("1.2.3.4"), + CertKey: aws.String("CERT"), + PrivateKey: aws.String("PRIVATE"), + ExpiresAt: aws.Time(expiresAt), + }, + }, + } + provider := &Provider{client: client, ctx: context.Background(), region: DefaultRegion} + + access, err := provider.fetchAccessDetails("demo") + if err != nil { + t.Fatalf("fetchAccessDetails() error: %v", err) + } + if !access.ExpiresAt.Equal(expiresAt) { + t.Fatalf("ExpiresAt=%s, want %s", access.ExpiresAt, expiresAt) + } +} + func TestLaunchRecreatesMismatchedDeploymentIdentity(t *testing.T) { name := "gh-1-pr-1" client := &fakeLightsailClient{ diff --git a/internal/pullpreview/instance.go b/internal/pullpreview/instance.go index 0549815..8cddb6e 100644 --- a/internal/pullpreview/instance.go +++ b/internal/pullpreview/instance.go @@ -3,6 +3,9 @@ package pullpreview import ( "bytes" "context" + "crypto/ed25519" + "crypto/rand" + "encoding/pem" "errors" "fmt" "io" @@ -12,12 +15,15 @@ import ( "strconv" "strings" "time" + + "golang.org/x/crypto/ssh" ) const ( remoteAppPath = "/app" instanceSSHReadyInterval = 5 * time.Second instanceSSHReadyWaitWindow = 5 * time.Minute + runSSHAccessTTL = 12 * time.Hour sshReadyDiagnosticCommand = `if test -f /etc/pullpreview/ready; then echo ready-marker-present exit 0 @@ -82,6 +88,7 @@ type Instance struct { Access AccessDetails Logger *Logger Runner Runner + runSSHPublicKey string } func NewInstance(name string, opts CommonOptions, provider Provider, logger *Logger) *Instance { @@ -290,6 +297,9 @@ func (i *Instance) launchAndWait() error { if i.Logger != nil { i.Logger.Infof("Instance ssh access OK") } + if err := i.handoffExpiringSSHAccess(); err != nil { + return fmt.Errorf("unable to establish deployment SSH access: %w", err) + } return nil } @@ -441,6 +451,70 @@ func (i *Instance) SetupPreScript() error { return i.SCP(bytes.NewBufferString(script), "/tmp/pre_script.sh", "0755") } +func (i *Instance) handoffExpiringSSHAccess() error { + if i.Access.ExpiresAt.IsZero() { + return nil + } + + publicKey, privateKey, err := generateRunSSHKeyPair(time.Now().Add(runSSHAccessTTL)) + if err != nil { + return err + } + content := publicKey + "\n" + homeDir := HomeDirForUser(i.Username()) + if err := i.appendRemoteFile(bytes.NewBufferString(content), fmt.Sprintf("%s/.ssh/authorized_keys", homeDir), "0600"); err != nil { + return err + } + + i.Access.PrivateKey = privateKey + i.Access.CertKey = "" + i.Access.ExpiresAt = time.Time{} + i.runSSHPublicKey = publicKey + if i.Logger != nil { + i.Logger.Infof("Established run-scoped SSH access for deployment") + } + return nil +} + +func (i *Instance) cleanupRunSSHAccess() error { + if strings.TrimSpace(i.runSSHPublicKey) == "" { + return nil + } + homeDir := HomeDirForUser(i.Username()) + target := fmt.Sprintf("%s/.ssh/authorized_keys", homeDir) + command := fmt.Sprintf( + "tmp=$(mktemp) && { grep -Fvx -- %s %s > \"$tmp\" || true; } && cat \"$tmp\" > %s && rm -f \"$tmp\" && chmod 0600 %s", + shellQuote(i.runSSHPublicKey), target, target, target, + ) + if err := i.SSH(command, nil); err != nil { + return err + } + i.runSSHPublicKey = "" + return nil +} + +func generateRunSSHKeyPair(expiresAt time.Time) (string, string, error) { + public, private, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + return "", "", err + } + publicKey, err := ssh.NewPublicKey(public) + if err != nil { + return "", "", err + } + privateBlock, err := ssh.MarshalPrivateKey(private, "pullpreview-run") + if err != nil { + return "", "", err + } + privatePEM := pem.EncodeToMemory(privateBlock) + if privatePEM == nil { + return "", "", errors.New("unable to encode run-scoped SSH private key") + } + key := strings.TrimSpace(string(ssh.MarshalAuthorizedKey(publicKey))) + authorizedKey := fmt.Sprintf("expiry-time=\"%s\" %s pullpreview-run", expiresAt.UTC().Format("20060102150405Z"), key) + return authorizedKey, strings.TrimSpace(string(privatePEM)), nil +} + func (i *Instance) SCP(input io.Reader, target, mode string) error { command := fmt.Sprintf("cat - > %s && chmod %s %s", target, mode, target) return i.SSH(command, input) diff --git a/internal/pullpreview/instance_test.go b/internal/pullpreview/instance_test.go index 1cae75c..7c2d36b 100644 --- a/internal/pullpreview/instance_test.go +++ b/internal/pullpreview/instance_test.go @@ -3,11 +3,15 @@ package pullpreview import ( "context" "errors" + "io" "os" "os/exec" "path/filepath" "strings" "testing" + "time" + + "golang.org/x/crypto/ssh" ) type captureRunner struct { @@ -19,6 +23,47 @@ func (r *captureRunner) Run(cmd *exec.Cmd) error { return nil } +type sshCredentialCapture struct { + args []string + input string + privateKey string + certKey string +} + +type sshCredentialCaptureRunner struct { + calls []sshCredentialCapture + err error +} + +func (r *sshCredentialCaptureRunner) Run(cmd *exec.Cmd) error { + call := sshCredentialCapture{args: append([]string{}, cmd.Args...)} + if cmd.Stdin != nil { + input, err := io.ReadAll(cmd.Stdin) + if err != nil { + return err + } + call.input = string(input) + } + for idx, arg := range cmd.Args { + if arg == "-i" && idx+1 < len(cmd.Args) { + content, err := os.ReadFile(cmd.Args[idx+1]) + if err != nil { + return err + } + call.privateKey = strings.TrimSpace(string(content)) + } + if strings.HasPrefix(arg, "CertificateFile=") { + content, err := os.ReadFile(strings.TrimPrefix(arg, "CertificateFile=")) + if err != nil { + return err + } + call.certKey = strings.TrimSpace(string(content)) + } + } + r.calls = append(r.calls, call) + return r.err +} + type launchSpyProvider struct { launchOpts []LaunchOptions terminateCalls int @@ -244,6 +289,118 @@ func TestSetupSSHAccessAppendsAuthorizedKeys(t *testing.T) { } } +func TestHandoffExpiringSSHAccessUsesRunScopedKey(t *testing.T) { + expiresAt := time.Now().Add(10 * time.Minute) + inst := NewInstance("my-app", CommonOptions{}, fakeProvider{}, nil) + inst.Access = AccessDetails{ + IPAddress: "1.2.3.4", + Username: "ec2-user", + PrivateKey: "TEMP PRIVATE", + CertKey: "TEMP CERT", + ExpiresAt: expiresAt, + } + runner := &sshCredentialCaptureRunner{} + inst.Runner = runner + + if err := inst.handoffExpiringSSHAccess(); err != nil { + t.Fatalf("handoffExpiringSSHAccess() error: %v", err) + } + if len(runner.calls) != 1 { + t.Fatalf("expected one bootstrap SSH call, got %d", len(runner.calls)) + } + if runner.calls[0].privateKey != "TEMP PRIVATE" || runner.calls[0].certKey != "TEMP CERT" { + t.Fatalf("bootstrap did not use temporary access details: %#v", runner.calls[0]) + } + if !strings.Contains(runner.calls[0].input, "pullpreview-run") { + t.Fatalf("bootstrap did not append the run-scoped public key: %q", runner.calls[0].input) + } + if !strings.Contains(runner.calls[0].input, `expiry-time="`) { + t.Fatalf("run-scoped public key has no server-enforced expiry: %q", runner.calls[0].input) + } + if inst.Access.CertKey != "" || !inst.Access.ExpiresAt.IsZero() { + t.Fatalf("temporary certificate remained active: %#v", inst.Access) + } + if _, err := ssh.ParsePrivateKey([]byte(inst.Access.PrivateKey)); err != nil { + t.Fatalf("run-scoped private key is invalid: %v", err) + } + if !strings.Contains(inst.runSSHPublicKey, "pullpreview-run") { + t.Fatalf("run-scoped public key was not retained for cleanup: %q", inst.runSSHPublicKey) + } + + runPrivateKey := inst.Access.PrivateKey + if err := inst.cleanupRunSSHAccess(); err != nil { + t.Fatalf("cleanupRunSSHAccess() error: %v", err) + } + if len(runner.calls) != 2 { + t.Fatalf("expected cleanup SSH call, got %d calls", len(runner.calls)) + } + if runner.calls[1].privateKey != runPrivateKey || runner.calls[1].certKey != "" { + t.Fatalf("cleanup did not use the run-scoped key: %#v", runner.calls[1]) + } + if !strings.Contains(strings.Join(runner.calls[1].args, " "), "grep -Fvx") { + t.Fatalf("cleanup did not remove the run-scoped public key: %v", runner.calls[1].args) + } + if inst.runSSHPublicKey != "" { + t.Fatalf("run-scoped public key remained after cleanup: %q", inst.runSSHPublicKey) + } +} + +func TestGenerateRunSSHKeyPairIncludesServerEnforcedExpiry(t *testing.T) { + expiresAt := time.Date(2026, time.August, 10, 14, 30, 45, 0, time.FixedZone("test", 2*60*60)) + publicKey, privateKey, err := generateRunSSHKeyPair(expiresAt) + if err != nil { + t.Fatalf("generateRunSSHKeyPair() error: %v", err) + } + if !strings.HasPrefix(publicKey, `expiry-time="20260810123045Z" ssh-ed25519 `) { + t.Fatalf("unexpected authorized key expiry: %q", publicKey) + } + if !strings.HasSuffix(publicKey, " pullpreview-run") { + t.Fatalf("authorized key has no run marker: %q", publicKey) + } + if _, err := ssh.ParsePrivateKey([]byte(privateKey)); err != nil { + t.Fatalf("run-scoped private key is invalid: %v", err) + } +} + +func TestHandoffExpiringSSHAccessLeavesCredentialsOnFailure(t *testing.T) { + expiresAt := time.Now().Add(10 * time.Minute) + inst := NewInstance("my-app", CommonOptions{}, fakeProvider{}, nil) + inst.Access = AccessDetails{ + PrivateKey: "TEMP PRIVATE", + CertKey: "TEMP CERT", + ExpiresAt: expiresAt, + } + inst.Runner = &sshCredentialCaptureRunner{err: errors.New("append failed")} + + err := inst.handoffExpiringSSHAccess() + if err == nil || !strings.Contains(err.Error(), "append failed") { + t.Fatalf("expected append failure, got %v", err) + } + if inst.Access.PrivateKey != "TEMP PRIVATE" || inst.Access.CertKey != "TEMP CERT" || !inst.Access.ExpiresAt.Equal(expiresAt) { + t.Fatalf("temporary credentials changed after failed handoff: %#v", inst.Access) + } + if inst.runSSHPublicKey != "" { + t.Fatalf("failed handoff retained a cleanup key: %q", inst.runSSHPublicKey) + } +} + +func TestHandoffExpiringSSHAccessSkipsNonExpiringCredentials(t *testing.T) { + inst := NewInstance("my-app", CommonOptions{}, fakeProvider{}, nil) + inst.Access = AccessDetails{PrivateKey: "PRIVATE"} + runner := &sshCredentialCaptureRunner{} + inst.Runner = runner + + if err := inst.handoffExpiringSSHAccess(); err != nil { + t.Fatalf("handoffExpiringSSHAccess() error: %v", err) + } + if len(runner.calls) != 0 { + t.Fatalf("non-expiring credentials triggered SSH handoff: %#v", runner.calls) + } + if inst.Access.PrivateKey != "PRIVATE" { + t.Fatalf("non-expiring credentials changed: %#v", inst.Access) + } +} + func TestSSHReadyDiagnosticIncludesRemoteDetails(t *testing.T) { inst := NewInstance("my-app", CommonOptions{}, fakeProvider{}, nil) inst.Access = AccessDetails{IPAddress: "1.2.3.4", Username: "ec2-user", PrivateKey: "PRIVATE"} diff --git a/internal/pullpreview/types.go b/internal/pullpreview/types.go index 010b8db..1317c81 100644 --- a/internal/pullpreview/types.go +++ b/internal/pullpreview/types.go @@ -42,6 +42,7 @@ type AccessDetails struct { IPAddress string CertKey string PrivateKey string + ExpiresAt time.Time } type UserDataOptions struct { diff --git a/internal/pullpreview/up.go b/internal/pullpreview/up.go index 6db8d7b..d1c13ae 100644 --- a/internal/pullpreview/up.go +++ b/internal/pullpreview/up.go @@ -34,6 +34,11 @@ func RunUp(opts UpOptions, provider Provider, logger *Logger) (*Instance, error) if err := instance.LaunchAndWait(); err != nil { return nil, err } + defer func() { + if err := instance.cleanupRunSSHAccess(); err != nil && logger != nil { + logger.Warnf("Unable to remove run-scoped SSH access: %v", err) + } + }() if logger != nil { logger.Infof("Synchronizing instance name=%s", instance.Name)