fix: improve the actions toolkit patching (#1328)

Patch the scripts beside each node action entrypoint too, so code-split bundles like rust-cache v2.9.2, setup-java v6 and setup-deno v2 reach the v2 cache API. Entrypoints may point anywhere in the action's repository, as `gradle/actions/setup-gradle` does with `../dist`.

The `.LOCALHOST` match is now anchored on the `.GHE.COM` test before it, so another library's uppercase localhost check is never rewritten.

Reviewed-on: https://gitea.com/gitea/runner/pulls/1328
Reviewed-by: Lunny Xiao <xiaolunwen@gmail.com>

act/runner/action.go

@@ -153,7 +153,7 @@ func maybeCopyToActionDir(ctx context.Context, step actionStep, actionDir, actio
 
 	if !rc.Config.NoActionPatch {
 		// Other jobs share the directory, so patch under the copy's lock.
-		defer patchActions(ctx, actionScriptPaths(filepath.Join(actionDir, actionPath), step.getActionModel()))()
+		defer patchActions(ctx, actionScriptPaths(actionDir, filepath.Join(actionDir, actionPath), step.getActionModel()))()
 	}
 
 	return rc.JobContainer.CopyDir(containerActionDirCopy, actionDir+"/", false, true)(ctx) // an exported commit has no .git to apply gitignore with

act/runner/patch_actions.go

@@ -9,7 +9,7 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
-	"strings"
+	"slices"
 
 	"gitea.com/gitea/runner/act/common"
 
@@ -40,8 +40,9 @@ const (
 	cacheURLEnv       = "ACTIONS_CACHE_URL"
 	resultsURLEnv     = "ACTIONS_RESULTS_URL"
 
-	// localhostHost is the suffix isGhes accepts.
+	// localhostHost is the suffix isGhes accepts, and gheHost the one it tests just before.
 	localhostHost = ".LOCALHOST"
+	gheHost       = ".GHE.COM"
 
 	// artifactRefusal is the only thing the gate guards in @actions/artifact, which is what makes
 	// such a bundle safe to open. A bundle carrying neither toolkit uses isGhes for something this
@@ -57,8 +58,10 @@ var (
 	// testing it, while undici, bundled into all of these actions, tests a lowercase ".localhost"
 	// in isURLPotentiallyTrustworthy. Opening that one would tell its HTTP client that every URL
 	// is trustworthy. Uppercase, the literal occurs nowhere but this test, across 118 bundles
-	// covering every major version of sixteen actions.
-	localhostTest = regexp.MustCompile(`endsWith\s*\(\s*` + quoted(regexp.QuoteMeta(localhostHost)) + `\s*\)`)
+	// covering every major version of sixteen actions. Anchoring on the .GHE.COM test, 5 to 35
+	// bytes before it, keeps any other library's uppercase test out.
+	localhostTest = regexp.MustCompile(`endsWith\s*\(\s*` + quoted(regexp.QuoteMeta(gheHost)) + `\s*\)(?s:.{0,128}?)` +
+		`endsWith\s*\(\s*` + quoted(regexp.QuoteMeta(localhostHost)) + `\s*\)`)
 
 	// serviceURLBranches matches both branches of getCacheServiceURL at once: the v1 branch reads
 	// the cache URL and falls back to the results URL, the v2 branch just below reads the results
@@ -83,26 +86,41 @@ func quoted(pattern string) string {
 	return "(?:'" + pattern + "'|\"" + pattern + "\"|`" + pattern + "`)"
 }
 
-// actionScriptPaths returns the entrypoints of a node action, the only kind with a bundle. Only
+// actionScriptPaths returns a node action's entrypoints and the chunks bundled beside them. Only
 // remote actions get here: a local one lives in the user's checkout, which the runner does not
-// rewrite.
-func actionScriptPaths(dir string, action *model.Action) []string {
+// rewrite. Scripts may leave dir for the rest of the checkout, as setup-gradle's ../dist does.
+func actionScriptPaths(checkout, dir string, action *model.Action) []string {
 	if action == nil || !action.Runs.Using.IsNode() {
 		return nil
 	}
+	root, err := filepath.EvalSymlinks(checkout)
+	if err != nil {
+		return nil
+	}
 	var paths []string
 	for _, script := range []string{action.Runs.Pre, action.Runs.Main, action.Runs.Post} {
 		if script == "" {
 			continue
 		}
-		path := filepath.Join(dir, script)
-		// `runs` is the action's own yaml, and a key pointing outside its directory is not ours.
-		if rel, err := filepath.Rel(dir, path); err != nil || strings.HasPrefix(rel, "..") {
+		path, err := filepath.EvalSymlinks(filepath.Join(dir, script))
+		if err != nil {
+			continue
+		}
+		// `runs` is the action's own yaml, and a key resolving outside its checkout is not ours.
+		bundleDir := filepath.Dir(path)
+		if rel, err := filepath.Rel(root, bundleDir); err != nil || !filepath.IsLocal(rel) {
 			continue
 		}
 		paths = append(paths, path)
+		entries, _ := os.ReadDir(bundleDir)
+		for _, entry := range entries {
+			if entry.Type().IsRegular() && slices.Contains([]string{".js", ".cjs", ".mjs"}, filepath.Ext(entry.Name())) {
+				paths = append(paths, filepath.Join(bundleDir, entry.Name()))
+			}
+		}
 	}
-	return paths
+	slices.Sort(paths)
+	return slices.Compact(paths)
 }
 
 // patchActions returns a restore: the tree is shared, so a job with patching off gets no edits.
@@ -164,7 +182,8 @@ func patchedBundle(data []byte) ([]byte, bool) {
 	opened := localhostTest.ReplaceAllFunc(data, func(test []byte) []byte {
 		// Drop the hostname from the test rather than rewriting the call, so the bundle's own
 		// quoting survives and the result stays valid even inside a string literal.
-		return bytes.Replace(test, []byte(localhostHost), nil, 1)
+		before, after, _ := bytes.CutLast(test, []byte(localhostHost))
+		return slices.Concat(before, after)
 	})
 	if cache {
 		opened = serviceURLBranches.ReplaceAll(opened, cacheURLFirst)

act/runner/patch_actions_e2e_test.go

@@ -20,6 +20,7 @@ import (
 
 	"gitea.com/gitea/runner/act/artifactcache"
 
+	"gitea.dev/actionslib/pkg/model"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -28,7 +29,7 @@ import (
 // deliberately: a new release is exactly what can stop the patch matching.
 const actionsCacheRef = "v6.1.0"
 
-// bundleFromGitHub downloads one entrypoint, keeping it in the user cache dir so repeated runs
+// bundleFromGitHub downloads one bundle file, keeping it in the user cache dir so repeated runs
 // cost nothing. The bundles are megabytes, too large to vendor.
 func bundleFromGitHub(t *testing.T, repo, ref, path string) string {
 	t.Helper()
@@ -52,6 +53,7 @@ func bundleFromGitHub(t *testing.T, repo, ref, path string) string {
 		t.Skipf("cannot reach %s: %v", url, err)
 	}
 	defer resp.Body.Close()
+	require.NotEqual(t, http.StatusNotFound, resp.StatusCode, "%s is not in this release", url)
 	if resp.StatusCode != http.StatusOK {
 		t.Skipf("GET %s: %s", url, resp.Status)
 	}
@@ -189,7 +191,7 @@ func TestCacheServiceV2EndToEnd(t *testing.T) {
 
 // The gate and the URL getter are separate functions, and a bundler may put either first: the gap
 // between them runs from 159 to 1179 bytes across these actions, which is why neither edit is
-// anchored on that distance. One entrypoint from each of the families that bundle the cache
+// anchored on that distance. One bundle file from each of the families that bundle the cache
 // toolkit, patched but not run, is what keeps a future release from quietly matching only one of
 // the two shapes and leaving every cache on v1.
 func TestPatchedBundleAcrossActions(t *testing.T) {
@@ -204,11 +206,14 @@ func TestPatchedBundleAcrossActions(t *testing.T) {
 		{"actions/setup-python", "v7.0.0", "dist/setup/index.js", true},
 		{"actions/setup-go", "v7.0.0", "dist/setup/index.js", true},
 		{"actions/setup-java", "v5.7.0", "dist/setup/index.js", true},
+		{"actions/setup-java", "v6.1.0", "dist/setup/295.index.js", true},
 		{"ruby/setup-ruby", "v1.331.0", "dist/index.js", true},
 		{"pnpm/action-setup", "v6.1.0", "dist/index.js", true},
 		{"oven-sh/setup-bun", "v2.2.0", "dist/setup/index.js", true},
 		{"Swatinem/rust-cache", "v2.9.1", "dist/restore/index.js", true},
+		{"Swatinem/rust-cache", "v2.9.2", "dist/cache-CTfQoCTL.js", true},
 		{"docker/build-push-action", "v7.4.0", "dist/index.cjs", true},
+		{"gradle/actions", "v6.4.0", "dist/setup-gradle/main/index.js", true},
 		// The artifact toolkit, where the gate is a refusal and there is nothing to redirect.
 		// v4.4.0 is the first release whose gate carries the localhost test this matches; the
 		// releases before it refuse in a shape the runner leaves alone.
@@ -364,3 +369,47 @@ func TestSetupActionFindsTheCacheService(t *testing.T) {
 	require.Contains(t, out, "Cache service version: v2")
 	require.Contains(t, out, "npm cache is not found", "the lookup did not reach the cache server")
 }
+
+func TestCodeSplitActionFindsTheCacheService(t *testing.T) {
+	requireHostTools(t, "node", "rustc")
+
+	dir := tempDirPath(t)
+	for _, name := range []string{
+		"action.yml", "dist/restore.js", "dist/save.js", "dist/cleanup-BWEbZ6YT.js",
+		"dist/cache-CTfQoCTL.js", "dist/cache-DC26rUaF.js", "dist/state-cjs-CkvFUaNy.js",
+	} {
+		body, err := os.ReadFile(bundleFromGitHub(t, "Swatinem/rust-cache", "v2.9.2", name))
+		require.NoError(t, err)
+		require.NoError(t, os.MkdirAll(filepath.Dir(filepath.Join(dir, name)), 0o755))
+		require.NoError(t, os.WriteFile(filepath.Join(dir, name), body, 0o600))
+	}
+	body, err := os.ReadFile(filepath.Join(dir, "action.yml"))
+	require.NoError(t, err)
+	action, err := model.ReadAction(strings.NewReader(string(body)))
+	require.NoError(t, err)
+	inputs := map[string]string{"lookup-only": "true", "add-rust-environment-hash-key": "false", "cache-bin": "false"}
+	for name, input := range action.Inputs {
+		if _, ok := inputs[name]; !ok {
+			inputs[name] = input.Default
+		}
+	}
+	t.Setenv("CARGO_HOME", tempDirPath(t))
+
+	handler, err := artifactcache.StartHandler(artifactcache.Options{Dir: filepath.Join(t.TempDir(), "cache"), OutboundIP: "127.0.0.1"})
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = handler.Close() })
+	const token = "split-runtime-token"
+	defer handler.RegisterJob(token, artifactcache.JobCredential{Repo: "testuser/testrepo"})()
+
+	defer patchActions(t.Context(), actionScriptPaths(dir, dir, action))()
+	out := runActionEntrypoint(t, filepath.Join(dir, action.Runs.Main), jobEnv{
+		workspace:  tempDirPath(t),
+		runnerTemp: tempDirPath(t),
+		cacheURL:   handler.ExternalURL(),
+		resultsURL: handler.ExternalURL(),
+		token:      token,
+	}, inputs)
+
+	require.Contains(t, out, "Cache service version: v2")
+	require.Contains(t, out, "No cache found.", "the lookup did not reach the cache server")
+}

act/runner/patch_actions_test.go

@@ -8,6 +8,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"slices"
 	"strings"
 	"testing"
 
@@ -84,6 +85,21 @@ func TestPatchedBundleLeavesTrustworthyURLCheckAlone(t *testing.T) {
 	assert.True(t, gateOpened(string(out)))
 }
 
+func TestPatchedBundleOpensOnlyTheIsGhesTest(t *testing.T) {
+	const unrelated = `function isLocalName(h){return h.toUpperCase().endsWith('.LOCALHOST')}`
+
+	for _, bundle := range []string{unrelated + "\n" + gateTSC, gateTSC + "\n" + unrelated} {
+		out, patched := patchedBundle([]byte(bundle))
+		require.True(t, patched)
+		assert.Contains(t, string(out), unrelated)
+		assert.Contains(t, string(out), "!h.endsWith('')")
+	}
+
+	apart := strings.Replace(gateTSC, "&&!h.endsWith('.LOCALHOST')", "&&"+strings.Repeat(" ", 200)+"!h.endsWith('.LOCALHOST')", 1)
+	_, patched := patchedBundle([]byte(apart))
+	assert.False(t, patched)
+}
+
 // The artifact toolkit puts the same gate in front of a plain refusal, with no URL to move, so
 // opening it is what lets the stock upload-artifact work against Gitea instead of aborting.
 func TestPatchedBundleOpensTheArtifactRefusal(t *testing.T) {
@@ -211,16 +227,42 @@ func bundleFile(t *testing.T, body string) string {
 }
 
 func TestActionScriptPaths(t *testing.T) {
-	node := &model.Action{Runs: model.ActionRuns{Using: "node20", Main: "dist/restore/index.js", Post: "dist/save/index.js"}}
-	assert.Equal(t, []string{"/a/dist/restore/index.js", "/a/dist/save/index.js"}, actionScriptPaths("/a", node))
+	dir, err := filepath.EvalSymlinks(t.TempDir())
+	require.NoError(t, err)
+	dist := filepath.Join(dir, "dist")
+	require.NoError(t, os.MkdirAll(filepath.Join(dist, "sub"), 0o755))
+	require.NoError(t, os.Mkdir(filepath.Join(dist, "directory.js"), 0o755))
+	for _, name := range []string{"dist/restore.js", "dist/save.js", "dist/cache-CTfQoCTL.js", "dist/295.index.js", "dist/chunk.cjs", "dist/chunk.mjs", "dist/LICENSE.txt", "dist/sub/deep.js", "pre.js"} {
+		require.NoError(t, os.WriteFile(filepath.Join(dir, name), nil, 0o644))
+	}
+	outside, err := filepath.EvalSymlinks(t.TempDir())
+	require.NoError(t, err)
+	require.NoError(t, os.WriteFile(filepath.Join(outside, "index.js"), nil, 0o644))
+	require.NoError(t, os.Symlink(filepath.Join(outside, "index.js"), filepath.Join(dist, "linked.js")))
+
+	var chunks []string
+	for _, name := range []string{"295.index.js", "cache-CTfQoCTL.js", "chunk.cjs", "chunk.mjs", "restore.js", "save.js"} {
+		chunks = append(chunks, filepath.Join(dist, name))
+	}
+	node := &model.Action{Runs: model.ActionRuns{Using: "node20", Pre: "pre.js", Main: "dist/restore.js", Post: "dist/save.js"}}
+	assert.Equal(t, append(slices.Clone(chunks), filepath.Join(dir, "pre.js")), actionScriptPaths(dir, dir, node))
 
 	// Only a node action has a bundle to patch.
-	assert.Nil(t, actionScriptPaths("/a", &model.Action{Runs: model.ActionRuns{Using: "docker", Image: "alpine"}}))
-	assert.Nil(t, actionScriptPaths("/a", nil))
+	assert.Nil(t, actionScriptPaths(dir, dir, &model.Action{Runs: model.ActionRuns{Using: "docker", Image: "alpine"}}))
+	assert.Nil(t, actionScriptPaths(dir, dir, nil))
 
-	// An action naming a file outside its own directory does not get it rewritten.
-	escaping := &model.Action{Runs: model.ActionRuns{Using: "node20", Main: "../../elsewhere/index.js"}}
-	assert.Nil(t, actionScriptPaths("/a", escaping))
+	// An action naming a file outside its checkout does not get it rewritten.
+	require.NoError(t, os.Symlink(outside, filepath.Join(dir, "linked")))
+	escaping, err := filepath.Rel(dir, filepath.Join(outside, "index.js"))
+	require.NoError(t, err)
+	for _, main := range []string{"../../elsewhere/index.js", escaping, "dist/linked.js", "linked/index.js"} {
+		assert.Nil(t, actionScriptPaths(dir, dir, &model.Action{Runs: model.ActionRuns{Using: "node20", Main: main}}), main)
+	}
+
+	require.NoError(t, os.Symlink("dist", filepath.Join(dir, "build")))
+	assert.Equal(t, chunks, actionScriptPaths(dir, dir, &model.Action{Runs: model.ActionRuns{Using: "node20", Main: "build/restore.js"}}))
+
+	assert.Equal(t, chunks, actionScriptPaths(dir, filepath.Join(dir, "setup"), &model.Action{Runs: model.ActionRuns{Using: "node20", Main: "../dist/restore.js"}}))
 }
 
 func TestPatchActionsAtTheContainerCopy(t *testing.T) {
@@ -231,15 +273,16 @@ func TestPatchActionsAtTheContainerCopy(t *testing.T) {
 		sar := &stepActionRemote{
 			Step:         &model.Step{Uses: "owner/repo/sub@v1"},
 			remoteAction: &remoteAction{Org: "owner", Repo: "repo", Path: "sub", Ref: "v1"},
-			action:       &model.Action{Runs: model.ActionRuns{Using: "node20", Main: "index.js"}},
+			action:       &model.Action{Runs: model.ActionRuns{Using: "node20", Main: "../dist/index.js"}},
 			actionDir:    t.TempDir(),
 			RunContext: &RunContext{
 				Config:       &Config{ActionCacheDir: t.TempDir(), NoActionPatch: noPatch},
 				JobContainer: cm,
 			},
 		}
-		script := filepath.Join(sar.actionDir, "sub", "index.js")
+		script := filepath.Join(sar.actionDir, "dist", "cache.js")
 		require.NoError(t, os.MkdirAll(filepath.Dir(script), 0o755))
+		require.NoError(t, os.WriteFile(filepath.Join(filepath.Dir(script), "index.js"), []byte("require('./cache.js')"), 0o600))
 		require.NoError(t, os.WriteFile(script, []byte(gateTSC), 0o600))
 
 		var copied string