From 2e7d37e83f2c93ee8a877063183bc91a157c14af Mon Sep 17 00:00:00 2001 From: alexchang Date: Sun, 13 Sep 2026 22:42:15 -0400 Subject: [PATCH 1/2] fix(agent): reject deployment selectors for prebuilt images Prebuilt image deploys return before reading --deployment, allowing a requested staging deploy to upload an image and report success through the default production workflow. Secrets may also be updated first. Validate prebuilt deployment intent in a deploy-specific Before hook, before client setup, secret updates, push-target acquisition, or image loading. Explain that named deployments require deploying from source. Preserve omitted/empty deployment selectors and source-based deployment. Reject every nonempty prebuilt selector, including literal production: the public client uses an empty string for the default deployment and does not establish production as an equivalent selector alias. Add command-boundary regression tests for both image flags, -d, secrets, missing paths, custom/case/whitespace selectors, quiet mode, and rejection before project validation and client creation. Accepted inputs continue through normal client setup without changing their deployment selector. Validation: - New rejection tests fail on the base and pass with this change. - Full go test -race ./... passes (438 test/subtest passes, 9 skips). - CI-pinned golangci-lint v2.11.4 passes with Go 1.26.3 (0 issues). - Local loopback integration verifies zero requests for rejected inputs, successful default tar upload, and source staging propagation. - Binary rejection and unchanged generated fish completion verified. Fixes #971 --- cmd/lk/agent.go | 14 ++- cmd/lk/agent_deploy_test.go | 165 ++++++++++++++++++++++++++++++++++++ 2 files changed, 178 insertions(+), 1 deletion(-) create mode 100644 cmd/lk/agent_deploy_test.go diff --git a/cmd/lk/agent.go b/cmd/lk/agent.go index a6ec5e6f..9ea2292d 100644 --- a/cmd/lk/agent.go +++ b/cmd/lk/agent.go @@ -248,7 +248,7 @@ var ( { Name: "deploy", Usage: "Deploy a new version of the agent", - Before: createAgentClient, + Before: prepareAgentDeploy, Action: deployAgent, Flags: []cli.Flag{ attributesFlag, @@ -435,6 +435,18 @@ func noAgentError() error { "To get started, see: https://docs.livekit.io/agents/quickstart") } +func prepareAgentDeploy(ctx context.Context, cmd *cli.Command) (context.Context, error) { + // The prebuilt upload client has no deployment selector. Check + // before client setup, secret updates, or acquiring an image push target. + if deployment := cmd.String("deployment"); deployment != "" && + (cmd.String("image") != "" || cmd.String("image-tar") != "") { + return ctx, fmt.Errorf("--deployment %q is not supported with --image or --image-tar; "+ + "prebuilt image deployments use the default production deployment. "+ + "Deploy from source to target a named deployment; omit --deployment only if production is intended", deployment) + } + return createAgentClient(ctx, cmd) +} + func createAgentClient(ctx context.Context, cmd *cli.Command) (context.Context, error) { return createAgentClientWithOpts(ctx, cmd) } diff --git a/cmd/lk/agent_deploy_test.go b/cmd/lk/agent_deploy_test.go new file mode 100644 index 00000000..7a6121ad --- /dev/null +++ b/cmd/lk/agent_deploy_test.go @@ -0,0 +1,165 @@ +package main + +import ( + "context" + "errors" + "io" + "net/http" + "path/filepath" + "reflect" + "testing" + + "github.com/livekit/livekit-cli/v2/pkg/config" + "github.com/livekit/livekit-cli/v2/pkg/util" + "github.com/stretchr/testify/require" + "github.com/urfave/cli/v3" +) + +func TestAgentDeployRejectsPrebuiltDeployment(t *testing.T) { + missingFile := filepath.Join(t.TempDir(), "missing") + for _, tt := range []struct { + name string + args []string + deployment string + }{ + {"tar", []string{"--image-tar", missingFile, "--deployment", "staging"}, "staging"}, + {"image", []string{"--image", "local:latest", "--deployment", "staging"}, "staging"}, + {"alias before image", []string{"-d", "staging", "--image", "local:latest"}, "staging"}, + {"alias before tar", []string{"-d", "staging", "--image-tar", missingFile}, "staging"}, + {"inline secrets", []string{"--image-tar", missingFile, "--secrets", "KEY=value", "-d", "staging"}, "staging"}, + {"secrets file", []string{"--image-tar", missingFile, "--secrets-file", missingFile, "-d", "staging"}, "staging"}, + {"secret mount", []string{"--image", "local:latest", "--secret-mount", missingFile, "-d", "staging"}, "staging"}, + {"custom name", []string{"--image", "local:latest", "-d", "qa-canary"}, "qa-canary"}, + {"literal production", []string{"--image", "local:latest", "-d", "production"}, "production"}, + {"case preserved", []string{"--image-tar", missingFile, "-d", "Production"}, "Production"}, + {"whitespace deployment", []string{"--image", "local:latest", "-d", " "}, " "}, + {"whitespace image", []string{"--image", " ", "-d", "staging"}, "staging"}, + {"both images", []string{"--image", "local:latest", "--image-tar", missingFile, "-d", "staging"}, "staging"}, + {"quiet", []string{"--quiet", "--image-tar", missingFile, "-d", "staging"}, "staging"}, + } { + t.Run(tt.name, func(t *testing.T) { + requests := isolateAgentDeploySetup(t) + actionCalled := false + app := agentDeployTestCommand(t, func(context.Context, *cli.Command) error { + actionCalled = true + return nil + }) + + err := app.Run(context.Background(), append([]string{"deploy"}, tt.args...)) + + require.ErrorContains(t, err, "is not supported with --image or --image-tar") + require.ErrorContains(t, err, "prebuilt image deployments use the default production deployment") + require.ErrorContains(t, err, "omit --deployment only if production is intended") + require.Equal(t, tt.deployment, app.String("deployment")) + require.False(t, actionCalled, "must reject before secrets, tar/Docker loading, or upload") + require.Nil(t, agentsClient, "must reject before creating the agent client") + require.Zero(t, *requests, "must not acquire a push target or make any other HTTP request") + }) + } +} + +func TestAgentDeployRejectsPrebuiltDeploymentBeforeProjectValidation(t *testing.T) { + requests := isolateAgentDeploySetup(t) + project.URL = "invalid-project-url" + actionCalled := false + app := agentDeployTestCommand(t, func(context.Context, *cli.Command) error { + actionCalled = true + return nil + }) + + err := app.Run(context.Background(), []string{"deploy", "--image", "local:latest", "-d", "staging"}) + + require.ErrorContains(t, err, "--deployment \"staging\" is not supported") + require.False(t, actionCalled) + require.Nil(t, agentsClient) + require.Zero(t, *requests) +} + +func TestAgentDeploySupportedInputsReachAction(t *testing.T) { + for _, tt := range []struct { + name string + args []string + deployment string + }{ + {"default tar", []string{"--image-tar", "image.tar"}, ""}, + {"default image", []string{"--image", "local:latest"}, ""}, + {"empty deployment tar", []string{"--image-tar", "image.tar", "--deployment="}, ""}, + {"empty deployment image", []string{"--image", "local:latest", "-d", ""}, ""}, + {"source staging", []string{"--deployment", "staging"}, "staging"}, + {"source alias", []string{"-d", "staging"}, "staging"}, + {"source literal production", []string{"--deployment", "production"}, "production"}, + {"empty image values", []string{"--image=", "--image-tar=", "-d", "staging"}, "staging"}, + } { + t.Run(tt.name, func(t *testing.T) { + requests := isolateAgentDeploySetup(t) + actionCalled := false + app := agentDeployTestCommand(t, func(_ context.Context, cmd *cli.Command) error { + actionCalled = true + require.Equal(t, tt.deployment, cmd.String("deployment")) + return nil + }) + + err := app.Run(context.Background(), append([]string{"deploy"}, tt.args...)) + + require.NoError(t, err) + require.True(t, actionCalled) + require.NotNil(t, agentsClient, "accepted input must still run normal client setup") + require.Zero(t, *requests) + }) + } +} + +// Keep the registered command's flags and Before hook. Only replace the action +// so these command-boundary tests cannot update secrets, load images, or deploy. +func agentDeployTestCommand(t *testing.T, action cli.ActionFunc) *cli.Command { + t.Helper() + agent := findCommandByName(AgentCommands, "agent") + require.NotNil(t, agent) + deploy := findCommandByName(agent.Commands, "deploy") + require.NotNil(t, deploy) + cmd := *deploy + cmd.Action = action + cmd.Writer, cmd.ErrWriter = io.Discard, io.Discard + cmd.Flags = nil + // Flags carry parsing state. Copy each definition to keep cases independent + // while exercising the production defaults and aliases. + for _, flag := range append(append([]cli.Flag{}, deploy.Flags...), quietFlag) { + value := reflect.ValueOf(flag) + copy := reflect.New(value.Elem().Type()) + copy.Elem().Set(value.Elem()) + cmd.Flags = append(cmd.Flags, copy.Interface().(cli.Flag)) + } + return &cmd +} + +// The CLI uses package globals; these tests must not run in parallel. +func isolateAgentDeploySetup(t *testing.T) *int { + t.Helper() + oldProject, oldConfig, oldClient, oldDir, oldOut := project, lkConfig, agentsClient, workingDir, out + oldTransport, oldHTTPClient := http.DefaultTransport, http.DefaultClient + t.Cleanup(func() { + project, lkConfig, agentsClient, workingDir, out = oldProject, oldConfig, oldClient, oldDir, oldOut + http.DefaultTransport, http.DefaultClient = oldTransport, oldHTTPClient + }) + project = &config.ProjectConfig{URL: "https://fixture.livekit.cloud", APIKey: "fixture-key", APISecret: "fixture-secret"} + lkConfig = config.NewLiveKitTOML("fixture").WithDefaultAgent() + lkConfig.Agent.ID = "test-agent" + agentsClient = nil + workingDir = t.TempDir() + out = util.NewPrinter(io.Discard, io.Discard, false) + t.Setenv("LK_AGENTS_URL", "http://127.0.0.1:1") + requests := 0 + transport := agentDeployTestTransport(func(*http.Request) (*http.Response, error) { + requests++ + return nil, errors.New("unexpected HTTP request during deploy setup test") + }) + http.DefaultTransport = transport + http.DefaultClient = &http.Client{Transport: transport} + return &requests +} + +type agentDeployTestTransport func(*http.Request) (*http.Response, error) + +func (f agentDeployTestTransport) RoundTrip(req *http.Request) (*http.Response, error) { + return f(req) +} From 821cd0d7981d5d7b0e01d07bb64a58e93f681237 Mon Sep 17 00:00:00 2001 From: Alex Chang Date: Wed, 30 Sep 2026 01:36:24 -0400 Subject: [PATCH 2/2] test(agent): focus prebuilt deployment regression coverage Replace the deployment flag matrix with four explicit tests covering named image and image-tar rejection, default prebuilt deployment, and named source deployment. Keep one assertion that rejection precedes client creation. Use fresh definitions for the three relevant flags and the registered Before hook. Remove reflection, global HTTP interception, and assertions about aliases, secrets, quiet mode, and unrelated project validation. Production behavior is unchanged. Validation: - Four focused tests pass. - Both rejection tests fail when the registered guard is bypassed. - Complete cmd/lk package passes with the race detector on Go 1.26.3. --- cmd/lk/agent_deploy_test.go | 179 +++++++++--------------------------- 1 file changed, 43 insertions(+), 136 deletions(-) diff --git a/cmd/lk/agent_deploy_test.go b/cmd/lk/agent_deploy_test.go index 7a6121ad..7f549785 100644 --- a/cmd/lk/agent_deploy_test.go +++ b/cmd/lk/agent_deploy_test.go @@ -2,164 +2,71 @@ package main import ( "context" - "errors" "io" - "net/http" - "path/filepath" - "reflect" "testing" "github.com/livekit/livekit-cli/v2/pkg/config" - "github.com/livekit/livekit-cli/v2/pkg/util" "github.com/stretchr/testify/require" "github.com/urfave/cli/v3" ) -func TestAgentDeployRejectsPrebuiltDeployment(t *testing.T) { - missingFile := filepath.Join(t.TempDir(), "missing") - for _, tt := range []struct { - name string - args []string - deployment string - }{ - {"tar", []string{"--image-tar", missingFile, "--deployment", "staging"}, "staging"}, - {"image", []string{"--image", "local:latest", "--deployment", "staging"}, "staging"}, - {"alias before image", []string{"-d", "staging", "--image", "local:latest"}, "staging"}, - {"alias before tar", []string{"-d", "staging", "--image-tar", missingFile}, "staging"}, - {"inline secrets", []string{"--image-tar", missingFile, "--secrets", "KEY=value", "-d", "staging"}, "staging"}, - {"secrets file", []string{"--image-tar", missingFile, "--secrets-file", missingFile, "-d", "staging"}, "staging"}, - {"secret mount", []string{"--image", "local:latest", "--secret-mount", missingFile, "-d", "staging"}, "staging"}, - {"custom name", []string{"--image", "local:latest", "-d", "qa-canary"}, "qa-canary"}, - {"literal production", []string{"--image", "local:latest", "-d", "production"}, "production"}, - {"case preserved", []string{"--image-tar", missingFile, "-d", "Production"}, "Production"}, - {"whitespace deployment", []string{"--image", "local:latest", "-d", " "}, " "}, - {"whitespace image", []string{"--image", " ", "-d", "staging"}, "staging"}, - {"both images", []string{"--image", "local:latest", "--image-tar", missingFile, "-d", "staging"}, "staging"}, - {"quiet", []string{"--quiet", "--image-tar", missingFile, "-d", "staging"}, "staging"}, - } { - t.Run(tt.name, func(t *testing.T) { - requests := isolateAgentDeploySetup(t) - actionCalled := false - app := agentDeployTestCommand(t, func(context.Context, *cli.Command) error { - actionCalled = true - return nil - }) - - err := app.Run(context.Background(), append([]string{"deploy"}, tt.args...)) - - require.ErrorContains(t, err, "is not supported with --image or --image-tar") - require.ErrorContains(t, err, "prebuilt image deployments use the default production deployment") - require.ErrorContains(t, err, "omit --deployment only if production is intended") - require.Equal(t, tt.deployment, app.String("deployment")) - require.False(t, actionCalled, "must reject before secrets, tar/Docker loading, or upload") - require.Nil(t, agentsClient, "must reject before creating the agent client") - require.Zero(t, *requests, "must not acquire a push target or make any other HTTP request") - }) - } +func TestAgentDeployRejectsImageDeployment(t *testing.T) { + cmd := agentDeployTestCommand(t) + + err := cmd.Run(context.Background(), []string{"deploy", "--image", "local:latest", "--deployment", "staging"}) + + require.ErrorContains(t, err, "--deployment \"staging\" is not supported with --image or --image-tar") } -func TestAgentDeployRejectsPrebuiltDeploymentBeforeProjectValidation(t *testing.T) { - requests := isolateAgentDeploySetup(t) - project.URL = "invalid-project-url" - actionCalled := false - app := agentDeployTestCommand(t, func(context.Context, *cli.Command) error { - actionCalled = true - return nil - }) +func TestAgentDeployRejectsImageTarDeployment(t *testing.T) { + cmd := agentDeployTestCommand(t) - err := app.Run(context.Background(), []string{"deploy", "--image", "local:latest", "-d", "staging"}) + err := cmd.Run(context.Background(), []string{"deploy", "--image-tar", "image.tar", "--deployment", "staging"}) - require.ErrorContains(t, err, "--deployment \"staging\" is not supported") - require.False(t, actionCalled) - require.Nil(t, agentsClient) - require.Zero(t, *requests) + require.ErrorContains(t, err, "--deployment \"staging\" is not supported with --image or --image-tar") + require.Nil(t, agentsClient, "rejection must happen before client creation") } -func TestAgentDeploySupportedInputsReachAction(t *testing.T) { - for _, tt := range []struct { - name string - args []string - deployment string - }{ - {"default tar", []string{"--image-tar", "image.tar"}, ""}, - {"default image", []string{"--image", "local:latest"}, ""}, - {"empty deployment tar", []string{"--image-tar", "image.tar", "--deployment="}, ""}, - {"empty deployment image", []string{"--image", "local:latest", "-d", ""}, ""}, - {"source staging", []string{"--deployment", "staging"}, "staging"}, - {"source alias", []string{"-d", "staging"}, "staging"}, - {"source literal production", []string{"--deployment", "production"}, "production"}, - {"empty image values", []string{"--image=", "--image-tar=", "-d", "staging"}, "staging"}, - } { - t.Run(tt.name, func(t *testing.T) { - requests := isolateAgentDeploySetup(t) - actionCalled := false - app := agentDeployTestCommand(t, func(_ context.Context, cmd *cli.Command) error { - actionCalled = true - require.Equal(t, tt.deployment, cmd.String("deployment")) - return nil - }) - - err := app.Run(context.Background(), append([]string{"deploy"}, tt.args...)) - - require.NoError(t, err) - require.True(t, actionCalled) - require.NotNil(t, agentsClient, "accepted input must still run normal client setup") - require.Zero(t, *requests) - }) - } +func TestAgentDeployAllowsPrebuiltWithoutDeployment(t *testing.T) { + cmd := agentDeployTestCommand(t) + + err := cmd.Run(context.Background(), []string{"deploy", "--image-tar", "image.tar"}) + + require.NoError(t, err) } -// Keep the registered command's flags and Before hook. Only replace the action -// so these command-boundary tests cannot update secrets, load images, or deploy. -func agentDeployTestCommand(t *testing.T, action cli.ActionFunc) *cli.Command { - t.Helper() - agent := findCommandByName(AgentCommands, "agent") - require.NotNil(t, agent) - deploy := findCommandByName(agent.Commands, "deploy") - require.NotNil(t, deploy) - cmd := *deploy - cmd.Action = action - cmd.Writer, cmd.ErrWriter = io.Discard, io.Discard - cmd.Flags = nil - // Flags carry parsing state. Copy each definition to keep cases independent - // while exercising the production defaults and aliases. - for _, flag := range append(append([]cli.Flag{}, deploy.Flags...), quietFlag) { - value := reflect.ValueOf(flag) - copy := reflect.New(value.Elem().Type()) - copy.Elem().Set(value.Elem()) - cmd.Flags = append(cmd.Flags, copy.Interface().(cli.Flag)) - } - return &cmd +func TestAgentDeployAllowsNamedSourceDeployment(t *testing.T) { + cmd := agentDeployTestCommand(t) + + err := cmd.Run(context.Background(), []string{"deploy", "--deployment", "staging"}) + + require.NoError(t, err) } -// The CLI uses package globals; these tests must not run in parallel. -func isolateAgentDeploySetup(t *testing.T) *int { +func agentDeployTestCommand(t *testing.T) *cli.Command { t.Helper() - oldProject, oldConfig, oldClient, oldDir, oldOut := project, lkConfig, agentsClient, workingDir, out - oldTransport, oldHTTPClient := http.DefaultTransport, http.DefaultClient + // Client setup uses package globals, so these tests must not run in parallel. + oldProject, oldConfig, oldClient, oldDir := project, lkConfig, agentsClient, workingDir t.Cleanup(func() { - project, lkConfig, agentsClient, workingDir, out = oldProject, oldConfig, oldClient, oldDir, oldOut - http.DefaultTransport, http.DefaultClient = oldTransport, oldHTTPClient + project, lkConfig, agentsClient, workingDir = oldProject, oldConfig, oldClient, oldDir }) project = &config.ProjectConfig{URL: "https://fixture.livekit.cloud", APIKey: "fixture-key", APISecret: "fixture-secret"} - lkConfig = config.NewLiveKitTOML("fixture").WithDefaultAgent() - lkConfig.Agent.ID = "test-agent" - agentsClient = nil + lkConfig, agentsClient = nil, nil workingDir = t.TempDir() - out = util.NewPrinter(io.Discard, io.Discard, false) - t.Setenv("LK_AGENTS_URL", "http://127.0.0.1:1") - requests := 0 - transport := agentDeployTestTransport(func(*http.Request) (*http.Response, error) { - requests++ - return nil, errors.New("unexpected HTTP request during deploy setup test") - }) - http.DefaultTransport = transport - http.DefaultClient = &http.Client{Transport: transport} - return &requests -} -type agentDeployTestTransport func(*http.Request) (*http.Response, error) - -func (f agentDeployTestTransport) RoundTrip(req *http.Request) (*http.Response, error) { - return f(req) + agent := findCommandByName(AgentCommands, "agent") + deploy := findCommandByName(agent.Commands, "deploy") + return &cli.Command{ + Name: "deploy", + Before: deploy.Before, + Writer: io.Discard, + ErrWriter: io.Discard, + Flags: []cli.Flag{ + &cli.StringFlag{Name: "image"}, + &cli.StringFlag{Name: "image-tar"}, + &cli.StringFlag{Name: "deployment"}, + }, + // Exercise the registered Before hook without loading or deploying an image. + Action: func(context.Context, *cli.Command) error { return nil }, + } }