From 0a2f28244dcc31a63bedeebdcc3518d4466692ca Mon Sep 17 00:00:00 2001 From: Zettat123 <39446+zettat123@noreply.gitea.com> Date: Tue, 9 Jun 2026 08:10:45 +0000 Subject: [PATCH] fix!: stop implicitly using `DOCKER_USERNAME`/`DOCKER_PASSWORD` secrets for image pulls (#1007) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Background `DOCKER_USERNAME` and `DOCKER_PASSWORD` are commonly used by workflows as ordinary secrets for logging in to a private registry and pushing images. However, the runner also treated these secret names as implicit Docker pull credentials. These credentials carry no registry information, but they were attached to every pull unconditionally. As a result, a user who configured `DOCKER_USERNAME` / `DOCKER_PASSWORD` secrets for their private registry (e.g. to push images) would have those same credentials sent to Docker Hub when pulling a public image, causing the pull to fail with authentication failure. ## Changes - Stop using `DOCKER_USERNAME` and `DOCKER_PASSWORD` as implicit pull credentials for job containers. - Stop injecting `DOCKER_USERNAME` and `DOCKER_PASSWORD` as pull credentials for step containers. ## ⚠️ BREAKING ⚠️ This is a breaking change. Workflows or runner setups that previously relied on `DOCKER_USERNAME` and `DOCKER_PASSWORD` being implicitly used for Docker image pulls must migrate to an explicit authentication mechanism. Migration options: - For private job container images, use `container.credentials`: ```yaml jobs: build: container: image: registry.example.com/image:tag credentials: username: ${{ secrets.REGISTRY_USERNAME }} password: ${{ secrets.REGISTRY_PASSWORD }} ``` - For private service container images, use service `credentials`. - For private `uses: docker://...` or private Docker actions, configure Docker authentication in the runner environment before the job starts. For example, run `docker login` on the runner host. `DOCKER_USERNAME` and `DOCKER_PASSWORD` can still be used as ordinary workflow secrets, for example with `docker/login-action` before pushing images. --- Related: - Fixes #386 --------- Co-authored-by: Nicolas Co-authored-by: Lunny Xiao Reviewed-on: https://gitea.com/gitea/runner/pulls/1007 Reviewed-by: Lunny Xiao Co-authored-by: Zettat123 <39446+zettat123@noreply.gitea.com> Co-committed-by: Zettat123 <39446+zettat123@noreply.gitea.com> --- act/runner/action.go | 4 +-- act/runner/action_test.go | 48 ++++++++++++++++++++++++++++++++++ act/runner/run_context.go | 9 +++---- act/runner/run_context_test.go | 32 +++++++++++++++++++++++ act/runner/step_docker.go | 2 -- act/runner/step_docker_test.go | 11 +++++++- 6 files changed, 94 insertions(+), 12 deletions(-) diff --git a/act/runner/action.go b/act/runner/action.go index 1e144462..61793d9e 100644 --- a/act/runner/action.go +++ b/act/runner/action.go @@ -436,13 +436,11 @@ func newStepContainer(ctx context.Context, step step, image string, cmd, entrypo if rc.IsHostEnv(ctx) { networkMode = "default" } - stepContainer := container.NewContainer(&container.NewContainerInput{ + stepContainer := ContainerNewContainer(&container.NewContainerInput{ Cmd: cmd, Entrypoint: entrypoint, WorkingDir: rc.JobContainer.ToContainerPath(rc.Config.Workdir), Image: image, - Username: rc.Config.Secrets["DOCKER_USERNAME"], - Password: rc.Config.Secrets["DOCKER_PASSWORD"], Name: createContainerName(rc.jobContainerName(), "STEP-"+stepModel.ID), Env: envList, Mounts: mounts, diff --git a/act/runner/action_test.go b/act/runner/action_test.go index 3814c6eb..c5dc2360 100644 --- a/act/runner/action_test.go +++ b/act/runner/action_test.go @@ -258,6 +258,54 @@ func TestActionRunner(t *testing.T) { } } +func TestNewStepContainerDoesNotUseDockerSecrets(t *testing.T) { + cm := &containerMock{} + + var captured *container.NewContainerInput + origContainerNewContainer := ContainerNewContainer + ContainerNewContainer = func(input *container.NewContainerInput) container.ExecutionsEnvironment { + captured = input + return cm + } + defer func() { + ContainerNewContainer = origContainerNewContainer + }() + + ctx := context.Background() + rc := &RunContext{ + Name: "job", + Config: &Config{ + Secrets: map[string]string{ + "DOCKER_USERNAME": "docker-user", + "DOCKER_PASSWORD": "docker-password", + }, + }, + Run: &model.Run{ + JobID: "job", + Workflow: &model.Workflow{ + Name: "test", + Jobs: map[string]*model.Job{ + "job": {}, + }, + }, + }, + JobContainer: cm, + StepResults: map[string]*model.StepResult{}, + } + env := map[string]string{} + step := &stepMock{} + step.On("getRunContext").Return(rc) + step.On("getStepModel").Return(&model.Step{ID: "action"}) + step.On("getEnv").Return(&env) + + _ = newStepContainer(ctx, step, "registry.example.com/action:tag", nil, nil) + + // DOCKER_USERNAME/DOCKER_PASSWORD should not be injected as pull credentials for docker action containers. + assert.Empty(t, captured.Username) + assert.Empty(t, captured.Password) + step.AssertExpectations(t) +} + func TestMaybeCopyToActionDirHoldsCloneLock(t *testing.T) { ctx := context.Background() diff --git a/act/runner/run_context.go b/act/runner/run_context.go index 0ff660bd..de85ebce 100644 --- a/act/runner/run_context.go +++ b/act/runner/run_context.go @@ -1175,21 +1175,18 @@ func setActionRuntimeVars(rc *RunContext, env map[string]string) { } func (rc *RunContext) handleCredentials(ctx context.Context) (string, string, error) { - // TODO: remove below 2 lines when we can release act with breaking changes - username := rc.Config.Secrets["DOCKER_USERNAME"] - password := rc.Config.Secrets["DOCKER_PASSWORD"] - container := rc.Run.Job().Container() if container == nil || container.Credentials == nil { - return username, password, nil + return "", "", nil } - if container.Credentials != nil && len(container.Credentials) != 2 { + if len(container.Credentials) != 2 { err := errors.New("invalid property count for key 'credentials:'") return "", "", err } ee := rc.NewExpressionEvaluator(ctx) + var username, password string if username = ee.Interpolate(ctx, container.Credentials["username"]); username == "" { err := errors.New("failed to interpolate container.credentials.username") return "", "", err diff --git a/act/runner/run_context_test.go b/act/runner/run_context_test.go index 56c14bd3..40f4d73c 100644 --- a/act/runner/run_context_test.go +++ b/act/runner/run_context_test.go @@ -170,6 +170,38 @@ func TestRunContext_EvalBool(t *testing.T) { } } +func TestRunContextHandleCredentialsDoesNotUseDockerSecrets(t *testing.T) { + workflow, err := model.ReadWorkflow(strings.NewReader(` +name: test +on: push +jobs: + job: + runs-on: ubuntu-latest + steps: [] +`)) + require.NoError(t, err) + + rc := &RunContext{ + Config: &Config{ + Secrets: map[string]string{ + "DOCKER_USERNAME": "docker-user", + "DOCKER_PASSWORD": "docker-password", + }, + Env: map[string]string{}, + }, + Run: &model.Run{ + JobID: "job", + Workflow: workflow, + }, + } + + // DOCKER_USERNAME/DOCKER_PASSWORD secrets should not be used as implicit job container pull credentials. + username, password, err := rc.handleCredentials(t.Context()) + require.NoError(t, err) + assert.Empty(t, username) + assert.Empty(t, password) +} + func TestRunContext_GetBindsAndMounts(t *testing.T) { rctemplate := &RunContext{ Name: "TestRCName", diff --git a/act/runner/step_docker.go b/act/runner/step_docker.go index 28c3bc10..9d2a85a1 100644 --- a/act/runner/step_docker.go +++ b/act/runner/step_docker.go @@ -125,8 +125,6 @@ func (sd *stepDocker) newStepContainer(ctx context.Context, image string, cmd, e Entrypoint: entrypoint, WorkingDir: rc.JobContainer.ToContainerPath(rc.Config.Workdir), Image: image, - Username: rc.Config.Secrets["DOCKER_USERNAME"], - Password: rc.Config.Secrets["DOCKER_PASSWORD"], Name: createContainerName(rc.jobContainerName(), "STEP-"+step.ID), Env: envList, Mounts: mounts, diff --git a/act/runner/step_docker_test.go b/act/runner/step_docker_test.go index 230fa6b4..da508d1f 100644 --- a/act/runner/step_docker_test.go +++ b/act/runner/step_docker_test.go @@ -38,7 +38,12 @@ func TestStepDockerMain(t *testing.T) { sd := &stepDocker{ RunContext: &RunContext{ StepResults: map[string]*model.StepResult{}, - Config: &Config{}, + Config: &Config{ + Secrets: map[string]string{ + "DOCKER_USERNAME": "docker-user", + "DOCKER_PASSWORD": "docker-password", + }, + }, Run: &model.Run{ JobID: "1", Workflow: &model.Workflow{ @@ -106,6 +111,10 @@ func TestStepDockerMain(t *testing.T) { assert.Equal(t, "node:14", input.Image) + // DOCKER_USERNAME/DOCKER_PASSWORD secrets should not be used as implicit pull credentials for docker:// action containers. + assert.Empty(t, input.Username) + assert.Empty(t, input.Password) + cm.AssertExpectations(t) }