diff --git a/act/common/git/git.go b/act/common/git/git.go index 2ad07ff9..31793adb 100644 --- a/act/common/git/git.go +++ b/act/common/git/git.go @@ -168,7 +168,8 @@ type NewGitCloneExecutorInput struct { Quiet bool // For Gitea - InsecureSkipTLS bool + InsecureSkipTLS bool + ClientCertFile, ClientKeyFile string // PEM, absolute as git runs in other directories } // CloneIfRequired reports whether an existing local clone was reused. @@ -367,6 +368,9 @@ func runGit(ctx context.Context, dir string, input *NewGitCloneExecutorInput, ar config := []string{"core.autocrlf=false", "core.eol=lf"} // keep action files LF if input != nil { config = append(config, RemoteConfig(input.Token, input.InsecureSkipTLS)...) + if input.ClientCertFile != "" { + config = append(config, "http.sslCert="+input.ClientCertFile, "http.sslKey="+input.ClientKeyFile) + } } if dir != "" { args = append([]string{"-C", dir}, args...) diff --git a/act/runner/job_executor.go b/act/runner/job_executor.go index 9cba245b..b878e272 100644 --- a/act/runner/job_executor.go +++ b/act/runner/job_executor.go @@ -532,7 +532,7 @@ func tryUploadJobSummary(ctx context.Context, rc *RunContext) { "/jobs/" + strconv.FormatInt(jobID, 10) + "/steps/" actPath := rc.JobContainer.GetActPath() // Reuse a single client across all step uploads so connections can be pooled. - client := &http.Client{Timeout: jobSummaryUploadRequestTimeout} + client := &http.Client{Timeout: jobSummaryUploadRequestTimeout, Transport: rc.Config.instanceTransportFor(base)} for i := range rc.Run.Job().Steps { summaryPath := path.Join(actPath, "workflow", "step-summary-"+strconv.Itoa(i)+".md") body, ok := readSingleFileFromContainerArchive(ctx, rc.JobContainer, summaryPath, maxJobSummaryBytes) @@ -544,6 +544,15 @@ func tryUploadJobSummary(ctx context.Context, rc *RunContext) { } } +// For Gitea +// instanceTransportFor returns InstanceTransport only for URLs on the registered address, as steps can rewrite the summary URL through GITHUB_ENV. +func (c Config) instanceTransportFor(rawURL string) http.RoundTripper { + if !strings.HasPrefix(rawURL, c.GitHubInstance+"/") { + return nil + } + return c.InstanceTransport +} + // extractJobIDFromRuntimeToken returns the JobID claim from an ACTIONS_RUNTIME_TOKEN JWT // without verifying its signature. Returns 0 if the token is unparseable or has no JobID. func extractJobIDFromRuntimeToken(token string) int64 { diff --git a/act/runner/job_executor_test.go b/act/runner/job_executor_test.go index 1384f6e6..32ab9d84 100644 --- a/act/runner/job_executor_test.go +++ b/act/runner/job_executor_test.go @@ -858,7 +858,7 @@ func TestTryUploadJobSummaryStopsAtPhaseTimeout(t *testing.T) { cm.AssertExpectations(t) } -func TestTryUploadJobSummaryUploadsEachStepIndependently(t *testing.T) { +func TestTryUploadJobSummaryUploadsEachStepIndependentlyWithInstanceTransport(t *testing.T) { runtimeToken := fakeRuntimeToken(34) type upload struct { @@ -866,7 +866,7 @@ func TestTryUploadJobSummaryUploadsEachStepIndependently(t *testing.T) { body string } var got []upload - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { body, err := io.ReadAll(r.Body) assert.NoError(t, err) got = append(got, upload{r.URL.Path, string(body)}) @@ -896,6 +896,7 @@ func TestTryUploadJobSummaryUploadsEachStepIndependently(t *testing.T) { "ACTIONS_RUNTIME_TOKEN": runtimeToken, "GITEA_RUN_ID": "12", }, cm, 3) + rc.Config.GitHubInstance, rc.Config.InstanceTransport = server.URL, server.Client().Transport tryUploadJobSummary(ctx, rc) diff --git a/act/runner/reusable_workflow.go b/act/runner/reusable_workflow.go index 49d10bc7..a5f9db4c 100644 --- a/act/runner/reusable_workflow.go +++ b/act/runner/reusable_workflow.go @@ -39,11 +39,8 @@ func newLocalReusableWorkflowExecutor(rc *RunContext) common.Executor { workflowDir := fmt.Sprintf("%s/%s", rc.ActionCacheDir(), safeFilename(uses)) - // If the repository is private, we need a token to clone it - token := rc.Config.GetToken() - return common.NewPipelineExecutor( - cloneRemoteReusableWorkflow(rc, remoteReusableWorkflow.CloneURL(), remoteReusableWorkflow.Ref, workflowDir, token), + cloneRemoteReusableWorkflow(rc, remoteReusableWorkflow.CloneURL(), remoteReusableWorkflow.Ref, workflowDir), newReusableWorkflowExecutor(rc, workflowDir, remoteReusableWorkflow.Path), ) } @@ -60,10 +57,8 @@ func newRemoteReusableWorkflowExecutor(rc *RunContext) common.Executor { filename := fmt.Sprintf("%s/%s@%s", remoteReusableWorkflow.Owner, remoteReusableWorkflow.Repo, remoteReusableWorkflow.Ref) workflowDir := fmt.Sprintf("%s/%s", rc.ActionCacheDir(), safeFilename(filename)) - token := getGitCloneToken(rc.Config, remoteReusableWorkflow.CloneURL()) - return common.NewPipelineExecutor( - cloneRemoteReusableWorkflow(rc, remoteReusableWorkflow.CloneURL(), remoteReusableWorkflow.Ref, workflowDir, token), + cloneRemoteReusableWorkflow(rc, remoteReusableWorkflow.CloneURL(), remoteReusableWorkflow.Ref, workflowDir), newReusableWorkflowExecutor(rc, workflowDir, remoteReusableWorkflow.Path), ) } @@ -76,20 +71,19 @@ func newRemoteReusableWorkflowExecutor(rc *RunContext) common.Executor { // 2. Gitea has already full URL with rc.Config.GitHubInstance when calling newRemoteReusableWorkflow // // remoteReusableWorkflow.URL = rc.getGithubContext(ctx).ServerURL -func cloneRemoteReusableWorkflow(rc *RunContext, cloneURL, ref, targetDirectory, token string) common.Executor { +func cloneRemoteReusableWorkflow(rc *RunContext, cloneURL, ref, targetDirectory string) common.Executor { return func(ctx context.Context) error { interpolatedURL, err := rc.NewExpressionEvaluator(ctx).Interpolate(ctx, cloneURL) if err != nil { return fmt.Errorf("unable to interpolate the workflow clone URL: %w", err) } - return git.NewGitCloneExecutor(git.NewGitCloneExecutorInput{ + return git.NewGitCloneExecutor(rc.Config.withInstanceAuth(git.NewGitCloneExecutorInput{ URL: interpolatedURL, Ref: ref, Dir: targetDirectory, - Token: token, OfflineMode: rc.Config.ActionOfflineMode, Depth: rc.Config.ActionCloneDepth, - })(ctx) + }, rc.Config.GetToken()))(ctx) } } @@ -208,13 +202,18 @@ func setReusedWorkflowCallerResult(rc *RunContext, runner *runnerImpl) common.Ex } // For Gitea -// getGitCloneToken returns GITEA_TOKEN when shouldCloneURLUseToken returns true, -// otherwise returns an empty string -func getGitCloneToken(conf *Config, cloneURL string) string { - if !shouldCloneURLUseToken(conf.GitHubInstance, conf.trustedActionInstance(), cloneURL) { - return "" +// withInstanceAuth passes instance credentials only to clones from this Gitea instance, and runner.insecure only to its registered address. +func (c Config) withInstanceAuth(input git.NewGitCloneExecutorInput, token string) git.NewGitCloneExecutorInput { + u, err := url.Parse(input.URL) + if err != nil || !onInstanceHost(u.Host, c.GitHubInstance, c.trustedActionInstance()) { + return input } - return conf.GetToken() + input.ClientCertFile, input.ClientKeyFile = c.ClientCertFile, c.ClientKeyFile + input.InsecureSkipTLS = c.InsecureSkipTLS && onInstanceHost(u.Host, c.GitHubInstance) + if u.User == nil { + input.Token = token + } + return input } // For Gitea @@ -228,26 +227,16 @@ func (c Config) trustedActionInstance() string { } // For Gitea -// shouldCloneURLUseToken returns true when the following conditions are met: -// 1. cloneURL's host matches this Gitea instance: either the registered instance -// (instanceURL) or, for DEFAULT_ACTIONS_URL=self on a different hostname, the -// self-hosted action instance (trustedActionInstance, "" when not trusted) -// 2. the cloneURL does not have basic auth embedded -func shouldCloneURLUseToken(instanceURL, trustedActionInstance, cloneURL string) bool { - u2, err := url.Parse(cloneURL) - if err != nil || u2.User != nil { - return false - } - - for _, candidate := range []string{instanceURL, trustedActionInstance} { +// onInstanceHost reports whether host is that of one of the instance URLs, which may lack a scheme. +func onInstanceHost(host string, instanceURLs ...string) bool { + for _, candidate := range instanceURLs { if candidate == "" { continue } - if !strings.HasPrefix(candidate, "http://") && - !strings.HasPrefix(candidate, "https://") { + if !strings.HasPrefix(candidate, "http://") && !strings.HasPrefix(candidate, "https://") { candidate = "https://" + candidate } - if u1, err := url.Parse(candidate); err == nil && u1.Host == u2.Host { + if u, err := url.Parse(candidate); err == nil && u.Host == host { return true } } diff --git a/act/runner/reusable_workflow_test.go b/act/runner/reusable_workflow_test.go index 6e4665e3..ec276308 100644 --- a/act/runner/reusable_workflow_test.go +++ b/act/runner/reusable_workflow_test.go @@ -5,7 +5,12 @@ package runner import ( "context" + "crypto/tls" + "crypto/x509" + "encoding/pem" "io/fs" + "net/http" + "net/http/httptest" "os" "os/exec" "path/filepath" @@ -59,7 +64,7 @@ func TestReusableWorkflowCachedBranchRefRefreshes(t *testing.T) { } cacheDir := t.TempDir() - require.NoError(t, cloneRemoteReusableWorkflow(rc, remoteDir, "master", cacheDir, "")(context.Background())) + require.NoError(t, cloneRemoteReusableWorkflow(rc, remoteDir, "master", cacheDir)(context.Background())) got, err := os.ReadFile(filepath.Join(cacheDir, workflowPath)) require.NoError(t, err) require.Equal(t, tmpl("v1"), string(got)) @@ -69,7 +74,7 @@ func TestReusableWorkflowCachedBranchRefRefreshes(t *testing.T) { gitMust(t, workDir, "commit", "-am", "v2") gitMust(t, workDir, "push", "origin", "master") - require.NoError(t, cloneRemoteReusableWorkflow(rc, remoteDir, "master", cacheDir, "")(context.Background())) + require.NoError(t, cloneRemoteReusableWorkflow(rc, remoteDir, "master", cacheDir)(context.Background())) got, err = os.ReadFile(filepath.Join(cacheDir, workflowPath)) require.NoError(t, err) require.Equal(t, tmpl("v2"), string(got), "cached workflow file must reflect the updated branch tip") @@ -123,107 +128,79 @@ func TestNewLocalReusableWorkflowExecutorFindsSameRepositoryPaths(t *testing.T) } } -func TestGetGitCloneTokenWithSchemalessGiteaInstance(t *testing.T) { - conf := &Config{ - GitHubInstance: "gitea.example.net", - Secrets: map[string]string{ - "GITEA_TOKEN": "token-value", +func TestCloneRemoteReusableWorkflowSendsInstanceTokenAndClientCert(t *testing.T) { + var gotCert bool + var gotToken string + server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotCert = len(r.TLS.PeerCertificates) > 0 + _, gotToken, _ = r.BasicAuth() + w.WriteHeader(http.StatusNotFound) + })) + server.TLS = &tls.Config{ClientAuth: tls.RequireAnyClientCert} + server.StartTLS() + defer server.Close() + pair := server.TLS.Certificates[0] + keyDER, err := x509.MarshalPKCS8PrivateKey(pair.PrivateKey) + require.NoError(t, err) + dir := t.TempDir() + certFile, keyFile := filepath.Join(dir, "client.crt"), filepath.Join(dir, "client.key") + require.NoError(t, os.WriteFile(certFile, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: pair.Certificate[0]}), 0o600)) + require.NoError(t, os.WriteFile(keyFile, pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: keyDER}), 0o600)) + rc := &RunContext{ + Config: &Config{ + GitHubInstance: server.URL, + Secrets: map[string]string{"GITEA_TOKEN": "token"}, + InsecureSkipTLS: true, + ClientCertFile: certFile, + ClientKeyFile: keyFile, }, + Run: &model.Run{JobID: "j1", Workflow: &model.Workflow{Jobs: map[string]*model.Job{"j1": {}}}}, } - token := getGitCloneToken(conf, "https://gitea.example.net/actions/tools") - - require.Equal(t, "token-value", token) + require.Error(t, cloneRemoteReusableWorkflow(rc, server.URL+"/owner/repo", "main", t.TempDir())(t.Context())) + require.True(t, gotCert) + require.Equal(t, "token", gotToken) } -func TestGetGitCloneTokenSelfHostedActionsDifferentHost(t *testing.T) { - // The runner registered with one hostname while DEFAULT_ACTIONS_URL=self resolves - // actions against AppURL on a different hostname for the same instance. - conf := &Config{ - GitHubInstance: "gitea.local", - DefaultActionInstance: "https://gitea.my-nas.lan", - DefaultActionInstanceIsSelfHosted: true, - Secrets: map[string]string{ - "GITEA_TOKEN": "token-value", - }, - } - - token := getGitCloneToken(conf, "https://gitea.my-nas.lan/owner/action") - - require.Equal(t, "token-value", token) -} - -func TestShouldCloneURLUseToken(t *testing.T) { - tests := []struct { - name string - instanceURL string - trustedActionInstance string - cloneURL string - want bool +func TestInstanceAuthAndTransportOnlyApplyToTheInstance(t *testing.T) { + for _, tt := range []struct { + name, instanceURL, trustedActionInstance, cloneURL string + token, cert, insecure bool }{ - { - name: "same host with schemaless instance", - instanceURL: "gitea.example.net", - cloneURL: "https://gitea.example.net/actions/tools", - want: true, - }, - { - name: "same host with schemaless instance and port", - instanceURL: "gitea.example.net:3000", - cloneURL: "https://gitea.example.net:3000/actions/tools", - want: true, - }, - { - name: "different host", - instanceURL: "gitea.example.net", - cloneURL: "https://github.com/actions/tools", - want: false, - }, - { - name: "embedded basic auth", - instanceURL: "gitea.example.net", - cloneURL: "https://user:pass@gitea.example.net/actions/tools", - want: false, - }, - { - name: "invalid clone URL", - instanceURL: "gitea.example.net", - cloneURL: "://gitea.example.net/actions/tools", - want: false, - }, - { - // self-hosted DEFAULT_ACTIONS_URL on a different hostname than the - // registered instance: the token must still be attached. - name: "self-hosted action instance on different host", - instanceURL: "gitea.local", - trustedActionInstance: "https://gitea.my-nas.lan", - cloneURL: "https://gitea.my-nas.lan/owner/action", - want: true, - }, - { - // embedded basic auth must still be rejected even when the host matches - // the trusted action instance. - name: "self-hosted action instance with embedded basic auth", - instanceURL: "gitea.local", - trustedActionInstance: "https://gitea.my-nas.lan", - cloneURL: "https://user:pass@gitea.my-nas.lan/owner/action", - want: false, - }, - { - // github.com / mirror hosts are never trusted: trustedActionInstance is - // empty in github mode, so an off-instance clone URL gets no token. - name: "github mode does not trust mirror host", - instanceURL: "gitea.local", - cloneURL: "https://mirror.example.com/owner/action", - want: false, - }, - } - - for _, tt := range tests { + {"same host with schemaless instance", "gitea.example.net", "", "https://gitea.example.net/actions/tools", true, true, true}, + {"same host with schemaless instance and port", "gitea.example.net:3000", "", "https://gitea.example.net:3000/actions/tools", true, true, true}, + {"different host", "gitea.example.net", "", "https://github.com/actions/tools", false, false, false}, + {"embedded basic auth keeps its own credentials", "gitea.example.net", "", "https://user:pass@gitea.example.net/actions/tools", false, true, true}, + {"invalid clone URL", "gitea.example.net", "", "://gitea.example.net/actions/tools", false, false, false}, + {"self-hosted action instance on different host skips no verification", "gitea.local", "https://gitea.my-nas.lan", "https://gitea.my-nas.lan/owner/action", true, true, false}, + {"self-hosted action instance with embedded basic auth", "gitea.local", "https://gitea.my-nas.lan", "https://user:pass@gitea.my-nas.lan/owner/action", false, true, false}, + {"github mode does not trust mirror host", "gitea.local", "", "https://mirror.example.com/owner/action", false, false, false}, + } { t.Run(tt.name, func(t *testing.T) { - require.Equal(t, tt.want, shouldCloneURLUseToken(tt.instanceURL, tt.trustedActionInstance, tt.cloneURL)) + conf := Config{ + GitHubInstance: tt.instanceURL, + DefaultActionInstance: tt.trustedActionInstance, + DefaultActionInstanceIsSelfHosted: tt.trustedActionInstance != "", + InsecureSkipTLS: true, + ClientCertFile: "/cert", + ClientKeyFile: "/key", + } + want := git.NewGitCloneExecutorInput{URL: tt.cloneURL, InsecureSkipTLS: tt.insecure} + if tt.token { + want.Token = "token" + } + if tt.cert { + want.ClientCertFile, want.ClientKeyFile = conf.ClientCertFile, conf.ClientKeyFile + } + require.Equal(t, want, conf.withInstanceAuth(git.NewGitCloneExecutorInput{URL: tt.cloneURL}, "token")) }) } + + transport := &http.Transport{} + conf := Config{GitHubInstance: "https://gitea.local", InstanceTransport: transport} + require.Same(t, transport, conf.instanceTransportFor("https://gitea.local/api/actions_pipeline/")) + require.Nil(t, conf.instanceTransportFor("https://gitea.local.example/api/actions_pipeline/")) + require.Nil(t, conf.instanceTransportFor("https://other.example/x#/api/actions_pipeline/")) } func gitMust(t *testing.T, dir string, args ...string) { diff --git a/act/runner/runner.go b/act/runner/runner.go index edeee0eb..2f1b1aaa 100644 --- a/act/runner/runner.go +++ b/act/runner/runner.go @@ -7,6 +7,7 @@ package runner import ( "context" "fmt" + "net/http" "os" "runtime" "slices" @@ -77,10 +78,12 @@ type Config struct { DefaultActionInstanceIsSelfHosted bool PlatformPicker func(labels []string) string KubernetesPicker func(labels []string) container.KubernetesOptions + InstanceTransport http.RoundTripper JobLoggerLevel *log.Level // the level of job logger ValidVolumes []string // only volumes (and bind mounts) in this slice can be mounted on the job container or service containers SharedToolCache bool // one tool cache for all jobs instead of one per job InsecureSkipTLS bool // whether to skip verifying TLS certificate of the Gitea instance + ClientCertFile, ClientKeyFile string // PEM client certificate for the Gitea instance MaxParallel int // max parallel jobs to run across all workflows (0 = no limit, uses CPU count) AllocatePTY bool // allocate a pseudo-TTY for each step's process ServiceReadyTimeout time.Duration // how long a job waits for its service containers to report healthy (0 uses the default) diff --git a/act/runner/step_action_builtin.go b/act/runner/step_action_builtin.go index aeaa19e3..67ed6fca 100644 --- a/act/runner/step_action_builtin.go +++ b/act/runner/step_action_builtin.go @@ -49,6 +49,8 @@ func (sab *stepActionBuiltin) main() common.Executor { NoSkipCheckout: rc.Config.NoSkipCheckout, UseGitIgnore: rc.Config.UseGitIgnore, InsecureSkipTLS: rc.Config.InsecureSkipTLS, + ClientCertFile: rc.Config.ClientCertFile, + ClientKeyFile: rc.Config.ClientKeyFile, Env: sab.env, }) }) diff --git a/act/runner/step_action_remote.go b/act/runner/step_action_remote.go index 04807615..1968d356 100644 --- a/act/runner/step_action_remote.go +++ b/act/runner/step_action_remote.go @@ -67,27 +67,16 @@ func (sar *stepActionRemote) prepareActionExecutor() common.Executor { actionDir := sar.actionDir() defaultActionURL := sar.RunContext.Config.DefaultActionURL() // For Gitea - // A composite RunContext nils Config.Secrets, so getGitCloneToken would yield an - // empty token and clone the action anonymously (401 against the authenticated - // instance). github.Token survives the composite config copy and matches the - // top-level token; keep the shouldCloneURLUseToken host gate to avoid leaking it. - cloneURL := sar.remoteAction.CloneURL(defaultActionURL) - token := "" - if shouldCloneURLUseToken(sar.RunContext.Config.GitHubInstance, sar.RunContext.Config.trustedActionInstance(), cloneURL) { - token = github.Token - } - gitClone := stepActionRemoteNewCloneExecutor(git.NewGitCloneExecutorInput{ - URL: cloneURL, + // github.Token, unlike Config.GetToken, survives the composite config copy, which nils Config.Secrets. + gitClone := stepActionRemoteNewCloneExecutor(sar.RunContext.Config.withInstanceAuth(git.NewGitCloneExecutorInput{ + URL: sar.remoteAction.CloneURL(defaultActionURL), Ref: sar.remoteAction.Ref, Dir: actionDir, - Token: token, OfflineMode: sar.RunContext.Config.ActionOfflineMode, Depth: sar.RunContext.Config.ActionCloneDepth, // printPrepareActions reports the download with its resolved commit. Quiet: true, - - InsecureSkipTLS: sar.cloneSkipTLS(), // For Gitea - }) + }, github.Token)) if err := gitClone(ctx); err != nil { var refErr *git.Error switch { @@ -250,22 +239,6 @@ func (sar *stepActionRemote) getCompositeSteps() *compositeSteps { return sar.compositeSteps } -// For Gitea -// cloneSkipTLS returns true if the runner can clone an action from the Gitea instance -func (sar *stepActionRemote) cloneSkipTLS() bool { - if !sar.RunContext.Config.InsecureSkipTLS { - // Return false if the Gitea instance is not an insecure instance - return false - } - if sar.remoteAction.URL == "" { - // Empty URL means the default action instance should be used - // Return true if the URL of the Gitea instance is the same as the URL of the default action instance - return sar.RunContext.Config.DefaultActionURL() == sar.RunContext.Config.GitHubInstance - } - // Return true if the URL of the remote action is the same as the URL of the Gitea instance - return sar.remoteAction.URL == sar.RunContext.Config.GitHubInstance -} - type remoteAction struct { URL string Org string diff --git a/docs/builtin-actions.md b/docs/builtin-actions.md index feffc8ed..1b74ed4f 100644 --- a/docs/builtin-actions.md +++ b/docs/builtin-actions.md @@ -34,6 +34,6 @@ Works like `actions/checkout` with these inputs. Any other input fails the step. A branch is checked out as a local branch tracking `origin`, a tag or commit as a detached `HEAD`. A triggering tag that moved since the event fails the step. The step outputs `ref` and `commit` like `actions/checkout`. -Persisted credentials end with the job. `token` is only sent to `github.server_url`, while `ssh-key` is offered to any SSH host, as with `actions/checkout`. Without `ssh-key`, submodule URLs of the form `git@:` are fetched from `github.server_url` with `token`. With `ssh-key`, the repository is fetched from the SSH URL in the event's `repository.ssh_url`, or from the server's host on port 22. +Persisted credentials end with the job. `token` is only sent to `github.server_url`, while `ssh-key` is offered to any SSH host, as with `actions/checkout`. The runner's `client_cert_file` is only offered to `github.server_url` and only during the step, so later steps do not get it. Without `ssh-key`, submodule URLs of the form `git@:` are fetched from `github.server_url` with `token`. With `ssh-key`, the repository is fetched from the SSH URL in the event's `repository.ssh_url`, or from the server's host on port 22. With `gitea-runner exec`, checking out the workflow's own repository copies your local working directory, unless `--no-skip-checkout` or `ssh-key` is set. diff --git a/e2e/runner_harness.go b/e2e/runner_harness.go index e76dba52..d8c1a0bc 100644 --- a/e2e/runner_harness.go +++ b/e2e/runner_harness.go @@ -75,7 +75,7 @@ func startRunner(t *testing.T, repo, labelName string, options runnerOptions) *p } labelNames := []string{label.Name} - pingCli := client.New(fixture.baseURL, cfg.Runner.Insecure, "", "", config.RequestTimeout, cfg.Runner.ExtraHeaders) + pingCli := client.New(fixture.baseURL, cfg.Runner.TLSConfig(), "", "", config.RequestTimeout, cfg.Runner.ExtraHeaders) if _, err := pingCli.Ping(ctx, connect.NewRequest(&pingv1.PingRequest{Data: t.Name()})); err != nil { t.Fatalf("ping %s: %v", fixture.baseURL, err) } @@ -105,7 +105,7 @@ func startRunner(t *testing.T, repo, labelName string, options runnerOptions) *p Ephemeral: regResp.Msg.Runner.Ephemeral, } - cli := client.New(fixture.baseURL, cfg.Runner.Insecure, reg.UUID, reg.Token, config.RequestTimeout, cfg.Runner.ExtraHeaders) + cli := client.New(fixture.baseURL, cfg.Runner.TLSConfig(), reg.UUID, reg.Token, config.RequestTimeout, cfg.Runner.ExtraHeaders) runner := run.NewRunner(cfg, reg, cli) declResp, err := runner.Declare(ctx, labelNames) diff --git a/internal/app/cmd/daemon.go b/internal/app/cmd/daemon.go index 2dc812b5..4e0aeebd 100644 --- a/internal/app/cmd/daemon.go +++ b/internal/app/cmd/daemon.go @@ -170,7 +170,7 @@ func runDaemon(ctx context.Context, daemArgs *daemonArgs, configFile *string) fu cli := client.New( reg.Address, - cfg.Runner.Insecure, + cfg.Runner.TLSConfig(), reg.UUID, reg.Token, config.RequestTimeout, diff --git a/internal/app/cmd/register.go b/internal/app/cmd/register.go index a244c14d..249297fb 100644 --- a/internal/app/cmd/register.go +++ b/internal/app/cmd/register.go @@ -363,7 +363,7 @@ func doRegister(ctx context.Context, cfg *config.Config, inputs *registerInputs) // initial http client cli := client.New( inputs.InstanceAddr, - cfg.Runner.Insecure, + cfg.Runner.TLSConfig(), "", "", config.RequestTimeout, diff --git a/internal/app/run/runner.go b/internal/app/run/runner.go index e9efc02d..c8c1d92f 100644 --- a/internal/app/run/runner.go +++ b/internal/app/run/runner.go @@ -74,6 +74,7 @@ type Runner struct { capabilities string isolatedCacheContainer func() string + instanceTransport http.RoundTripper runningTasks sync.Map runningCount atomic.Int64 @@ -140,6 +141,7 @@ func NewRunner(cfg *config.Config, reg *config.Registration, cli client.Client) runHealthCheck: executeHealthCheck, } runner.isolatedCacheContainer = sync.OnceValue(runner.detectIsolatedCacheContainer) + runner.instanceTransport = client.NewTransport(cli.Address(), cfg.Runner.TLSConfig()) return runner } @@ -626,6 +628,9 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. ValidVolumes: r.cfg.Container.ValidVolumes, SharedToolCache: r.cfg.Runner.ToolCacheMode == config.ToolCacheModeShared, InsecureSkipTLS: r.cfg.Runner.Insecure, + ClientCertFile: r.cfg.Runner.ClientCertFile, + ClientKeyFile: r.cfg.Runner.ClientKeyFile, + InstanceTransport: r.instanceTransport, RunnerName: r.name, KubernetesPicker: r.kubernetesOptions, } diff --git a/internal/pkg/action/action.go b/internal/pkg/action/action.go index 51e52099..67d82f87 100644 --- a/internal/pkg/action/action.go +++ b/internal/pkg/action/action.go @@ -20,5 +20,7 @@ type Context struct { NoSkipCheckout bool UseGitIgnore bool InsecureSkipTLS bool + ClientCertFile string + ClientKeyFile string Env map[string]string } diff --git a/internal/pkg/action/checkout/checkout.go b/internal/pkg/action/checkout/checkout.go index 9fed9527..7008266f 100644 --- a/internal/pkg/action/checkout/checkout.go +++ b/internal/pkg/action/checkout/checkout.go @@ -11,6 +11,7 @@ import ( "fmt" "maps" "net/url" + "os" "path" "path/filepath" "runtime" @@ -122,6 +123,26 @@ func Main(ctx context.Context, c *action.Context) (err error) { if boolInput(c.Inputs, "set-safe-directory", true) { remoteConfig = append(remoteConfig, "safe.directory=*") // a bound workdir belongs to the host user } + if c.ClientCertFile != "" { // only for this step, later steps never get the runner's key + var certPEM, keyPEM []byte + if certPEM, err = os.ReadFile(c.ClientCertFile); err != nil { + return err + } + if keyPEM, err = os.ReadFile(c.ClientKeyFile); err != nil { + return err + } + cert, key := fmt.Sprintf("git-client-cert-%x.pem", digest[:8]), fmt.Sprintf("git-client-key-%x.pem", digest[:8]) + defer func() { + cleanupCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), time.Minute) + defer cancel() + err = errors.Join(err, c.Container.Copy(runnerTemp, &container.FileEntry{Name: key, Mode: 0o600})(cleanupCtx)) + }() + if err := c.Container.Copy(runnerTemp, &container.FileEntry{Name: cert, Mode: 0o600, Body: string(certPEM)}, &container.FileEntry{Name: key, Mode: 0o600, Body: string(keyPEM)})(ctx); err != nil { + return err + } + scope := "http." + strings.TrimSuffix(ghc.ServerURL, "/") + "/." + remoteConfig = append(remoteConfig, scope+"sslCert="+path.Join(runnerTemp, cert), scope+"sslKey="+path.Join(runnerTemp, key)) + } env := gitEnv(c, remoteConfig) env["GIT_LFS_SKIP_SMUDGE"] = strconv.FormatBool(!co.lfs || len(co.sparse) == 0) // lfs pull fetches the objects of a full checkout in one go if sshKey != "" { diff --git a/internal/pkg/action/checkout/checkout_test.go b/internal/pkg/action/checkout/checkout_test.go index cc41a7da..f6b8b020 100644 --- a/internal/pkg/action/checkout/checkout_test.go +++ b/internal/pkg/action/checkout/checkout_test.go @@ -6,8 +6,13 @@ package checkout import ( "cmp" "crypto/sha256" + "crypto/tls" + "crypto/x509" + "encoding/pem" "fmt" "io" + "net/http" + "net/http/httptest" "os" "os/exec" "path/filepath" @@ -286,6 +291,41 @@ exec sh -c "$(printf '%s' "$last" | sed "s#git-upload-pack '/\(.*\)\.git'#git up } } +func TestCheckoutOffersClientCertOnlyDuringTheStep(t *testing.T) { + var gotCert bool + server := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotCert = len(r.TLS.PeerCertificates) > 0 + w.WriteHeader(http.StatusNotFound) + })) + server.TLS = &tls.Config{ClientAuth: tls.RequireAnyClientCert} + server.StartTLS() + defer server.Close() + pair := server.TLS.Certificates[0] + keyDER, err := x509.MarshalPKCS8PrivateKey(pair.PrivateKey) + require.NoError(t, err) + dir, workspace, runnerTemp := t.TempDir(), t.TempDir(), t.TempDir() + certFile, keyFile := filepath.Join(dir, "client.crt"), filepath.Join(dir, "client.key") + require.NoError(t, os.WriteFile(certFile, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: pair.Certificate[0]}), 0o600)) + require.NoError(t, os.WriteFile(keyFile, pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: keyDER}), 0o600)) + + require.ErrorContains(t, Main(t.Context(), &action.Context{ + Container: &container.HostEnvironment{Path: workspace, TmpDir: runnerTemp, StdOut: io.Discard}, + Github: &model.GithubContext{Repository: "org/repo", Ref: "refs/heads/main", ServerURL: server.URL}, + Inputs: map[string]string{"repository": "other/repo"}, + Workspace: workspace, + InsecureSkipTLS: true, + ClientCertFile: certFile, + ClientKeyFile: keyFile, + Env: map[string]string{"PATH": os.Getenv("PATH"), "GIT_CONFIG_GLOBAL": os.DevNull, "GIT_CONFIG_NOSYSTEM": "1"}, + }), "checkout: other/repo@") + require.True(t, gotCert) + keys, _ := filepath.Glob(filepath.Join(runnerTemp, "git-client-key-*")) + require.Len(t, keys, 1) + key, err := os.ReadFile(keys[0]) + require.NoError(t, err) + require.Empty(t, key) +} + func publish(t *testing.T, server, name string, files, submodules map[string]string) { work := t.TempDir() runGit(t, "init", "--initial-branch=main", work) diff --git a/internal/pkg/client/http.go b/internal/pkg/client/http.go index 657735a8..fad6c1e9 100644 --- a/internal/pkg/client/http.go +++ b/internal/pkg/client/http.go @@ -19,20 +19,23 @@ import ( "gitea.dev/actionslib/runner/v1/runnerv1connect" ) -func getHTTPClient(endpoint string, insecure bool, timeout time.Duration, extraHeaders map[string]string) *http.Client { +func NewTransport(endpoint string, tlsConfig *tls.Config) *http.Transport { transport := &http.Transport{ Proxy: http.ProxyFromEnvironment, MaxIdleConns: 10, MaxIdleConnsPerHost: 10, // All requests go to one host; default is 2 which causes frequent reconnects. IdleConnTimeout: 90 * time.Second, + ForceAttemptHTTP2: true, } - if strings.HasPrefix(endpoint, "https://") && insecure { - transport.TLSClientConfig = &tls.Config{ - InsecureSkipVerify: true, - } + if strings.HasPrefix(endpoint, "https://") { + transport.TLSClientConfig = tlsConfig } + return transport +} + +func getHTTPClient(endpoint string, tlsConfig *tls.Config, timeout time.Duration, extraHeaders map[string]string) *http.Client { return &http.Client{ - Transport: transport, + Transport: NewTransport(endpoint, tlsConfig), Timeout: timeout, CheckRedirect: func(req *http.Request, via []*http.Request) error { if len(via) >= 10 { // a custom CheckRedirect replaces net/http's own limit @@ -54,7 +57,7 @@ func getHTTPClient(endpoint string, insecure bool, timeout time.Duration, extraH // New returns a new runner client. timeout bounds every RPC: without it a // stalled connection parks the reporter for the whole job context, so logs and // heartbeats stop together and the task is reaped as a zombie. -func New(endpoint string, insecure bool, uuid, token string, timeout time.Duration, extraHeaders map[string]string, opts ...connect.ClientOption) *HTTPClient { +func New(endpoint string, tlsConfig *tls.Config, uuid, token string, timeout time.Duration, extraHeaders map[string]string, opts ...connect.ClientOption) *HTTPClient { baseURL := strings.TrimRight(endpoint, "/") + "/api/actions" opts = append(opts, connect.WithInterceptors(connect.UnaryInterceptorFunc(func(next connect.UnaryFunc) connect.UnaryFunc { @@ -75,7 +78,7 @@ func New(endpoint string, insecure bool, uuid, token string, timeout time.Durati } }))) - httpClient := getHTTPClient(endpoint, insecure, timeout, extraHeaders) + httpClient := getHTTPClient(endpoint, tlsConfig, timeout, extraHeaders) return &HTTPClient{ PingServiceClient: pingv1connect.NewPingServiceClient( httpClient, diff --git a/internal/pkg/client/http_test.go b/internal/pkg/client/http_test.go index 183a58f4..f532b1b7 100644 --- a/internal/pkg/client/http_test.go +++ b/internal/pkg/client/http_test.go @@ -4,6 +4,7 @@ package client import ( + "crypto/tls" "net" "net/http" "net/http/httptest" @@ -20,7 +21,7 @@ import ( func TestGetHTTPClientUsesProxyFromEnvironment(t *testing.T) { t.Setenv("HTTP_PROXY", "http://proxy.example.com:8080") - client := getHTTPClient("http://gitea.example.com", false, time.Minute, nil) + client := getHTTPClient("http://gitea.example.com", nil, time.Minute, nil) require.Equal(t, time.Minute, client.Timeout) transport, ok := client.Transport.(*http.Transport) require.True(t, ok) @@ -34,29 +35,10 @@ func TestGetHTTPClientUsesProxyFromEnvironment(t *testing.T) { require.Equal(t, "http://proxy.example.com:8080", proxyURL.String()) } -func TestGetHTTPClientInsecureTLS(t *testing.T) { - // insecure only takes effect for https endpoints - httpsInsecure := getHTTPClient("https://gitea.example.com", true, time.Minute, nil) - transport, ok := httpsInsecure.Transport.(*http.Transport) - require.True(t, ok) - require.NotNil(t, transport.TLSClientConfig) - require.True(t, transport.TLSClientConfig.InsecureSkipVerify) - - for _, tc := range []struct { - name string - endpoint string - insecure bool - }{ - {"https secure", "https://gitea.example.com", false}, - {"http insecure ignored", "http://gitea.example.com", true}, - } { - t.Run(tc.name, func(t *testing.T) { - c := getHTTPClient(tc.endpoint, tc.insecure, time.Minute, nil) - tr, ok := c.Transport.(*http.Transport) - require.True(t, ok) - require.Nil(t, tr.TLSClientConfig) - }) - } +func TestGetHTTPClientAppliesTLSConfigOnlyToHTTPS(t *testing.T) { + tlsConfig := &tls.Config{} + require.Same(t, tlsConfig, getHTTPClient("https://gitea.example.com", tlsConfig, time.Minute, nil).Transport.(*http.Transport).TLSClientConfig) + require.Nil(t, getHTTPClient("http://gitea.example.com", tlsConfig, time.Minute, nil).Transport.(*http.Transport).TLSClientConfig) } func TestNewSetsBaseURLAndHeaders(t *testing.T) { @@ -70,7 +52,7 @@ func TestNewSetsBaseURLAndHeaders(t *testing.T) { defer server.Close() // trailing slash must be trimmed before "/api/actions" is appended - c := New(server.URL+"/", false, "the-uuid", "the-token", time.Minute, map[string]string{ + c := New(server.URL+"/", nil, "the-uuid", "the-token", time.Minute, map[string]string{ "X-Proxy-Token": "proxy-token", protocol.UUIDHeader: "other-uuid", protocol.TokenHeader: "other-token", @@ -97,7 +79,7 @@ func TestNewOmitsEmptyHeaders(t *testing.T) { })) defer server.Close() - c := New(server.URL, false, "", "", time.Minute, nil) + c := New(server.URL, nil, "", "", time.Minute, nil) _, _ = c.Ping(t.Context(), connect.NewRequest(&pingv1.PingRequest{Data: "hi"})) require.Empty(t, gotHeaders.Get(protocol.UUIDHeader)) @@ -110,7 +92,7 @@ func TestNewStopsRedirectLoop(t *testing.T) { })) defer server.Close() - c := New(server.URL, false, "", "", time.Minute, nil) + c := New(server.URL, nil, "", "", time.Minute, nil) _, err := c.Ping(t.Context(), connect.NewRequest(&pingv1.PingRequest{Data: "hi"})) require.ErrorContains(t, err, "stopped after 10 redirects") } @@ -136,7 +118,7 @@ func TestNewRedirectKeepsExtraHeadersOnlyOnSameHost(t *testing.T) { } { gotHeaders = nil origin := tc.newOrigin(http.RedirectHandler("http://"+net.JoinHostPort(tc.host, port), http.StatusPermanentRedirect)) - c := New(origin.URL, true, "the-uuid", "the-token", time.Minute, map[string]string{"X-Proxy-Token": "proxy-token", "Content-Type": "text/plain"}) + c := New(origin.URL, &tls.Config{InsecureSkipVerify: true}, "the-uuid", "the-token", time.Minute, map[string]string{"X-Proxy-Token": "proxy-token", "Content-Type": "text/plain"}) _, _ = c.Ping(t.Context(), connect.NewRequest(&pingv1.PingRequest{Data: "hi"})) origin.Close() diff --git a/internal/pkg/config/config.example.yaml b/internal/pkg/config/config.example.yaml index dba49655..033af9bf 100644 --- a/internal/pkg/config/config.example.yaml +++ b/internal/pkg/config/config.example.yaml @@ -41,6 +41,9 @@ runner: #shutdown_timeout: 0s # Whether skip verifying the TLS certificate of the Gitea instance. #insecure: false + # PEM client certificate and key for a Gitea instance that requires mutual TLS. + #client_cert_file: /etc/gitea-runner/client.crt + #client_key_file: /etc/gitea-runner/client.key # The timeout for fetching the job from the Gitea instance. # Values above the 60s RPC timeout are capped to it. #fetch_timeout: 5s diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go index 59719891..c5d11625 100644 --- a/internal/pkg/config/config.go +++ b/internal/pkg/config/config.go @@ -4,6 +4,7 @@ package config import ( + "crypto/tls" "errors" "fmt" "maps" @@ -63,6 +64,8 @@ type Runner struct { Timeout time.Duration `yaml:"timeout"` // Timeout specifies the duration for runner timeout. ShutdownTimeout time.Duration `yaml:"shutdown_timeout"` // ShutdownTimeout specifies the duration to wait for running jobs to complete during a shutdown of the runner. Insecure bool `yaml:"insecure"` // Insecure indicates whether the runner operates in an insecure mode. + ClientCertFile string `yaml:"client_cert_file"` // ClientCertFile is the PEM certificate presented to a Gitea instance that requires mutual TLS. + ClientKeyFile string `yaml:"client_key_file"` // ClientKeyFile is the PEM private key paired with ClientCertFile. ExtraHeaders map[string]string `yaml:"extra_headers"` // ExtraHeaders adds HTTP headers to runner API requests to Gitea. FetchTimeout time.Duration `yaml:"fetch_timeout"` // FetchTimeout specifies the timeout duration for fetching resources. FetchInterval time.Duration `yaml:"fetch_interval"` // FetchInterval specifies the interval duration for fetching resources. @@ -272,6 +275,21 @@ func LoadDefault(file string) (*Config, error) { maps.Copy(cfg.Runner.Envs, envs) } } + if (cfg.Runner.ClientCertFile == "") != (cfg.Runner.ClientKeyFile == "") { + return nil, errors.New("runner.client_cert_file and runner.client_key_file must be set together") + } + if cfg.Runner.ClientCertFile != "" { + if _, err := tls.LoadX509KeyPair(cfg.Runner.ClientCertFile, cfg.Runner.ClientKeyFile); err != nil { + return nil, fmt.Errorf("load runner client certificate: %w", err) + } + for _, file := range []*string{&cfg.Runner.ClientCertFile, &cfg.Runner.ClientKeyFile} { + abs, err := filepath.Abs(*file) // git resolves a relative path against the repository it runs in + if err != nil { + return nil, err + } + *file = abs + } + } if cfg.Log.Level == "" { cfg.Log.Level = "info" @@ -422,6 +440,20 @@ func LoadDefault(file string) (*Config, error) { return cfg, nil } +func (r *Runner) TLSConfig() *tls.Config { + tlsConfig := &tls.Config{InsecureSkipVerify: r.Insecure} + if r.ClientCertFile != "" { + tlsConfig.GetClientCertificate = func(info *tls.CertificateRequestInfo) (*tls.Certificate, error) { + pair, err := tls.LoadX509KeyPair(r.ClientCertFile, r.ClientKeyFile) // per handshake, so renewed files apply + if err != nil || info.SupportsCertificate(&pair) != nil { + return &tls.Certificate{}, err + } + return &pair, nil + } + } + return tlsConfig +} + // expandEnvironment replaces ${NAME} in values and retypes them, so ${CAPACITY} can be an int and an empty value is unset. func expandEnvironment(node *yaml.Node) error { var errs []error diff --git a/internal/pkg/config/config_test.go b/internal/pkg/config/config_test.go index 03b244e6..e4786713 100644 --- a/internal/pkg/config/config_test.go +++ b/internal/pkg/config/config_test.go @@ -4,6 +4,13 @@ package config import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "encoding/pem" + "math/big" "os" "path/filepath" "testing" @@ -42,6 +49,49 @@ cache: require.NoError(t, err) } +func TestClientCertIsValidatedAndAbsoluteOnLoadReadOnUseAndOfferedOnlyWhenAccepted(t *testing.T) { + require.True(t, (&Runner{Insecure: true}).TLSConfig().InsecureSkipVerify) + + dir := t.TempDir() + certPath, keyPath := filepath.Join(dir, "client.crt"), filepath.Join(dir, "client.key") + writePair := func() []byte { + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + template := &x509.Certificate{SerialNumber: big.NewInt(1)} + der, err := x509.CreateCertificate(rand.Reader, template, template, &key.PublicKey, key) + require.NoError(t, err) + keyDER, err := x509.MarshalPKCS8PrivateKey(key) + require.NoError(t, err) + require.NoError(t, os.WriteFile(certPath, pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}), 0o600)) + require.NoError(t, os.WriteFile(keyPath, pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: keyDER}), 0o600)) + return der + } + load := func(certFile, keyFile string) (*Config, error) { + return LoadDefault(write(t, "runner:\n client_cert_file: '"+certFile+"'\n client_key_file: '"+keyFile+"'\n")) + } + writePair() + + _, err := load(certPath, "") + require.ErrorContains(t, err, "must be set together") + _, err = load(keyPath, keyPath) + require.ErrorContains(t, err, "load runner client certificate") + + t.Chdir(dir) + cfg, err := load("client.crt", "client.key") + require.NoError(t, err) + require.True(t, filepath.IsAbs(cfg.Runner.ClientCertFile)) + require.True(t, filepath.IsAbs(cfg.Runner.ClientKeyFile)) + renewed := writePair() + info := &tls.CertificateRequestInfo{Version: tls.VersionTLS13, SignatureSchemes: []tls.SignatureScheme{tls.ECDSAWithP256AndSHA256}} + pair, err := cfg.Runner.TLSConfig().GetClientCertificate(info) + require.NoError(t, err) + require.Equal(t, renewed, pair.Certificate[0]) + info.AcceptableCAs = [][]byte{[]byte("another CA")} + pair, err = cfg.Runner.TLSConfig().GetClientCertificate(info) + require.NoError(t, err) + require.Empty(t, pair.Certificate) +} + func TestLoadDefault_ExpandsEnvironmentValues(t *testing.T) { t.Setenv("RUNNER_CAPACITY", "4") t.Setenv("RUNNER_LABEL", "linux:host")