fix: mask secrets on every path they leave a job (#1188)

A secret in a matrix value reached the log in the clear:

```yaml
strategy:
  matrix:
    include: "${{ github.token }}"
```

Chasing that one route is pointless, so this masks every sink a secret leaves a job by: the uploaded log rows and the on-disk `job.log`, both through one choke point in `appendLogRow`; the runner's own log, which is where planning errors like that one land with no job logger in reach; the job logger's stdout under debug logging; job summaries; job outputs; and the job name that becomes a container name.

Values the runner knows but the job never declared, the proxy password and the task token, are hidden the same way. Masks apply longest first, since `strings.Replacer` matches in argument order and one secret prefixing another would otherwise mask the prefix and print the rest.

### What changes for users

An output whose value carries a secret is skipped with a warning instead of sent, matching GitHub. Output that showed a secret now shows `***`. `ACTIONS_STEP_DEBUG` and `ACTIONS_RUNNER_DEBUG` are never masked, also matching GitHub, so an output of `true` still reaches the jobs that need it.

Each fix has a test that fails without it.

Reviewed-on: https://gitea.com/gitea/runner/pulls/1188
Reviewed-by: bircni <bircni@icloud.com>
Co-authored-by: silverwind <me@silverwind.io>

.gitea/workflows/release-nightly.yml

@@ -20,11 +20,10 @@ jobs:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
         with:
           fetch-depth: 0
-      # Custom publishers (the R2 mirror below) run as the very last
+      # Custom publishers (the R2 upload below) run as the very last
       # step of goreleaser's publish pipeline, after the Gitea release
-      # has already been created and every artifact already uploaded
-      # to S3. Fail here instead, before anything is built or
-      # published, if the R2 secrets are missing.
+      # has already been created. Fail here instead, before anything
+      # is built or published, if the R2 secrets are missing.
       - name: check R2 configuration
         run: sh scripts/upload-r2.sh --check-config
         env:
@@ -43,11 +42,6 @@ jobs:
           args: release --nightly
         env:
           GORELEASER_KEY: ${{ secrets.GORELEASER_KEY }}
-          AWS_REGION: ${{ secrets.AWS_REGION }}
-          AWS_ACCESS_KEY_ID: ${{ secrets.AWS_KEY_ID }}
-          AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
-          S3_REGION: ${{ secrets.AWS_REGION }}
-          S3_BUCKET: ${{ secrets.AWS_BUCKET }}
           R2_ENDPOINT: ${{ secrets.R2_ENDPOINT }}
           R2_BUCKET: ${{ secrets.R2_BUCKET }}
           R2_ACCESS_KEY_ID: ${{ secrets.R2_ACCESS_KEY_ID }}
@@ -83,7 +77,7 @@ jobs:
         uses: docker/setup-qemu-action@96fe6ef7f33517b61c61be40b68a1882f3264fb8 # v4
 
       - name: Set up Docker BuildX
-        uses: docker/setup-buildx-action@bb05f3f5519dd87d3ba754cc423b652a5edd6d2c # v4
+        uses: docker/setup-buildx-action@37fe631027851001ddb9b187196cc803df7f5f0e # v4
 
       - name: Login to DockerHub
         uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4

.gitea/workflows/release-tag.yml

@@ -12,11 +12,10 @@ jobs:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
         with:
           fetch-depth: 0 # all history for all branches and tags
-      # Custom publishers (the R2 mirror below) run as the very last
+      # Custom publishers (the R2 upload below) run as the very last
       # step of goreleaser's publish pipeline, after the Gitea release
-      # has already been created and every artifact already uploaded
-      # to S3. Fail here instead, before anything is built or
-      # published, if the R2 secrets are missing.
+      # has already been created. Fail here instead, before anything
+      # is built or published, if the R2 secrets are missing.
       - name: check R2 configuration
         run: sh scripts/upload-r2.sh --check-config
         env:
@@ -42,11 +41,6 @@ jobs:
           args: release
         env:
           GORELEASER_KEY: ${{ secrets.GORELEASER_KEY }}
-          AWS_REGION: ${{ secrets.AWS_REGION }}
-          AWS_ACCESS_KEY_ID: ${{ secrets.AWS_KEY_ID }}
-          AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
-          S3_REGION: ${{ secrets.AWS_REGION }}
-          S3_BUCKET: ${{ secrets.AWS_BUCKET }}
           R2_ENDPOINT: ${{ secrets.R2_ENDPOINT }}
           R2_BUCKET: ${{ secrets.R2_BUCKET }}
           R2_ACCESS_KEY_ID: ${{ secrets.R2_ACCESS_KEY_ID }}
@@ -86,7 +80,7 @@ jobs:
         uses: docker/setup-qemu-action@96fe6ef7f33517b61c61be40b68a1882f3264fb8 # v4
 
       - name: Set up Docker BuildX
-        uses: docker/setup-buildx-action@bb05f3f5519dd87d3ba754cc423b652a5edd6d2c # v4
+        uses: docker/setup-buildx-action@37fe631027851001ddb9b187196cc803df7f5f0e # v4
 
       - name: Login to DockerHub
         uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4

.gitea/workflows/test.yml

@@ -5,17 +5,18 @@ on:
       - main
   pull_request:
 
+env:
+  # The runner image ships a stale docker.io login; point docker at an empty config so
+  # image pulls go straight to anonymous instead of attempting (and failing) that auth
+  # first. The path must be a literal: the `runner` context is unavailable in workflow-level
+  # env, so `${{ runner.temp }}` would resolve to empty and config.Dir() would fall back
+  # to ~/.docker with the stale credentials.
+  DOCKER_CONFIG: /tmp/docker-noauth
+
 jobs:
   lint:
     name: check and test
     runs-on: ubuntu-latest
-    env:
-      # The runner image ships a stale docker.io login; point docker at an empty config so
-      # image pulls go straight to anonymous instead of attempting (and failing) that auth
-      # first. The path must be a literal: the `runner` context is unavailable in job-level
-      # env, so `${{ runner.temp }}` would resolve to empty and config.Dir() would fall back
-      # to ~/.docker with the stale credentials.
-      DOCKER_CONFIG: /tmp/docker-noauth
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
       - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7
@@ -24,13 +25,20 @@ jobs:
           check-latest: true
       - name: prepare anonymous docker config
         run: mkdir -p "$DOCKER_CONFIG" && echo '{}' > "$DOCKER_CONFIG/config.json"
-      # Pre-pull internal/act/runner's two largest base images so a slow pull can't dominate `make test`;
+      # Pre-pull act/runner's two largest base images so a slow pull can't dominate `make test`;
       # the rest (alpine/ubuntu) pull on demand, absorbed by the make-test -timeout. The host
       # daemon retains them between runs, so this is usually a fast manifest re-check.
       - name: pre-pull test images
+        env:
+          TEST_JOB_IMAGE: node:24-bookworm-slim@sha256:3638d9a6fe4030bd716be989438248074489337ba3275657f93595428be4fc03 # renovate: datasource=docker
+          TEST_SERVICE_IMAGE: nginx:alpine@sha256:db35bfc6b2951e7f8a72db5db120288c127ffaeeb4a6d4b95a26fead017d5913 # renovate: datasource=docker
         run: |
-          for img in node:24-bookworm-slim nginx:alpine; do
-            for try in 1 2 3; do docker pull "$img" && break || sleep 5; done
+          for image in "$TEST_JOB_IMAGE" "$TEST_SERVICE_IMAGE"; do
+            for attempt in 1 2 3; do
+              docker pull "$image" && break
+              [ "$attempt" = 3 ] || sleep 5
+            done
+            docker tag "$image" "${image%@*}"
           done
       - name: lint
         run: make lint
@@ -49,3 +57,22 @@ jobs:
         run: |
           make coverage-report
           cat .tmp/coverage.md >> "$GITHUB_STEP_SUMMARY"
+
+  e2e:
+    name: gitea compatibility (${{ matrix.gitea_image }})
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        gitea_image:
+          - gitea/gitea:latest
+          - gitea/gitea:main-nightly
+    steps:
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
+      - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7
+        with:
+          go-version-file: 'go.mod'
+          check-latest: true
+      - name: prepare anonymous docker config
+        run: mkdir -p "$DOCKER_CONFIG" && echo '{}' > "$DOCKER_CONFIG/config.json"
+      - name: e2e compatibility
+        run: make test-e2e E2E_GITEA_IMAGE=${{ matrix.gitea_image }}

.gitignore

@@ -1,6 +1,6 @@
 /gitea-runner
 .env
-!/internal/act/runner/testdata/secrets/.env
+!/act/runner/testdata/secrets/.env
 .runner
 .runner.lock
 coverage.txt

.golangci.yml

@@ -46,8 +46,7 @@ linters:
     gocritic:
       enabled-checks:
         - equalFold
-      disabled-checks:
-        - ifElseChain
+      disabled-checks: []
     revive:
       severity: error
       rules:
@@ -71,10 +70,14 @@ linters:
         - name: unexported-return
         - name: var-declaration
         - name: var-naming
+          arguments:
+            - [] # AllowList - do not remove as args for the rule are positional and won't work without lists first
+            - [] # DenyList
+            - - skip-initialism-name-checks: true
     staticcheck:
       checks:
         - all
-        - -ST1005
+    testifylint: {}
     usetesting:
       os-temp-dir: true
     perfsprint:
@@ -92,8 +95,6 @@ linters:
     generated: lax
     presets:
       - comments
-      - common-false-positives
-      - legacy
       - std-error-handling
     rules:
       - linters:
@@ -118,7 +119,8 @@ formatters:
         - blank
         - default
     gofumpt:
-      extra-rules: true
+      extra:
+        group-params: true
   exclusions:
     generated: lax
 run:

.goreleaser.yaml

@@ -83,24 +83,12 @@ builds:
       - cmd: sh .goreleaser.checksum.sh {{ .Path }}
       - cmd: sh .goreleaser.checksum.sh {{ .Path }}.xz
 
-blobs:
-  -
-    provider: s3
-    bucket: "{{ .Env.S3_BUCKET }}"
-    region: "{{ .Env.S3_REGION }}"
-    directory: "gitea-runner/{{.Version}}"
-    extra_files:
-      - glob: ./**.xz
-      - glob: ./**.sha256
-
-# Mirrors the S3 `blobs:` upload above into Cloudflare R2 during the
-# parallel S3+R2 period (S3 will be removed once migration completes).
-# A second `blobs:` entry is impossible here since the blob pipe
-# authenticates from the global AWS_* env with no per-entry
-# credentials; `publishers:` supports per-entry `env:` instead, so
-# it's used to invoke scripts/upload-r2.sh once per artifact. Custom
-# publishers inherit almost nothing from the environment, hence the
-# explicit R2_* forwarding below.
+# Uploads every release artifact to Cloudflare R2. The `blobs:` pipe
+# isn't usable here since it authenticates from the global AWS_* env
+# with no per-entry credentials; `publishers:` supports per-entry
+# `env:` instead, so it's used to invoke scripts/upload-r2.sh once per
+# artifact. Custom publishers inherit almost nothing from the
+# environment, hence the explicit R2_* forwarding below.
 #
 # This publisher fires 109 times for 73 distinct keys because
 # goreleaser's release pipe already registers `release.extra_files`
@@ -125,7 +113,7 @@ publishers:
       - R2_SECRET_ACCESS_KEY={{ index .Env "R2_SECRET_ACCESS_KEY" }}
 
 archives:
-  - format: binary
+  - formats: [binary]
     name_template: "{{ .Binary }}"
     allow_different_binary_count: true
 

DEVELOPMENT.md

@@ -30,3 +30,12 @@ depending on the prefix:
   encoded forms too.
 - Command *properties* also escape `%3A` and `%2C`, which the UI never decodes, so the reporter
   decodes exactly those two when folding a location into an annotation.
+
+## End-to-end compatibility tests
+
+`make test-e2e` runs the runner against `E2E_GITEA_IMAGE`. It defaults to the nightly image.
+It requires Docker and is excluded from `make test`. CI runs stable and nightly variants in
+parallel.
+
+The suite shares one Gitea and regular runner. Cache and ephemeral scenarios use isolated
+repository runners. A run path is `<workflow>@<ref>` and a log row is `<timestamp>Z <payload>`.

Dockerfile

@@ -1,7 +1,7 @@
 ### BUILDER STAGE
 #
 #
-FROM golang:1.26-alpine3.23 AS builder
+FROM golang:1.27-alpine3.23 AS builder
 
 # Do not remove `git` here, it is required for getting runner version when executing `make build`
 RUN apk add --no-cache make git
@@ -17,7 +17,7 @@ RUN make clean && make build
 ### DIND VARIANT
 #
 #
-FROM docker:29.6.2-dind AS dind
+FROM docker:29.7.2-dind AS dind
 
 ARG VERSION=dev
 
@@ -37,7 +37,7 @@ ENTRYPOINT ["s6-svscan","/etc/s6"]
 ### DIND-ROOTLESS VARIANT
 #
 #
-FROM docker:29.6.2-dind-rootless AS dind-rootless
+FROM docker:29.7.2-dind-rootless AS dind-rootless
 
 ARG VERSION=dev
 

Makefile

@@ -5,7 +5,7 @@ GO ?= go
 SHASUM ?= shasum -a 256
 HAS_GO = $(shell hash $(GO) > /dev/null 2>&1 && echo "GO" || echo "NOGO" )
 XGO_PACKAGE ?= src.techknowlogick.com/xgo@v1.9.0 # renovate: datasource=go
-XGO_VERSION := go-1.26.x
+XGO_VERSION := go-1.27.x
 GXZ_PACKAGE ?= github.com/ulikunitz/xz/cmd/gxz@v0.5.16 # renovate: datasource=go
 
 LINUX_ARCHS ?= linux/amd64,linux/arm64
@@ -18,8 +18,8 @@ DOCKER_TAG ?= nightly
 DOCKER_REF := $(DOCKER_IMAGE):$(DOCKER_TAG)
 DOCKER_ROOTLESS_REF := $(DOCKER_IMAGE):$(DOCKER_TAG)-dind-rootless
 
-GOLANGCI_LINT_PACKAGE ?= github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.2 # renovate: datasource=go
-GOVULNCHECK_PACKAGE ?= golang.org/x/vuln/cmd/govulncheck@v1.6.0 # renovate: datasource=go
+GOLANGCI_LINT_PACKAGE ?= github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.13.1 # renovate: datasource=go
+GOVULNCHECK_PACKAGE ?= golang.org/x/vuln/cmd/govulncheck@v1.7.0 # renovate: datasource=go
 
 GOTEST_FLAGS ?= -race -timeout 20m -parallel 8
 
@@ -137,7 +137,7 @@ lint-pr-title: ## lint PR title against Conventional Commits (set PR_TITLE=...)
 
 .PHONY: security-check
 security-check:
-	GOEXPERIMENT= $(GO) run $(GOVULNCHECK_PACKAGE) -show color ./... || true
+	$(GO) run $(GOVULNCHECK_PACKAGE) -show color ./... || true
 
 .PHONY: tidy
 tidy: ## run go mod tidy
@@ -171,6 +171,15 @@ coverage-report: ## turn coverage.txt from `make test` into .tmp/coverage.md
 test-dind: ## run the daemon-facing tests against the built dind image (TARGET=dind|dind-rootless)
 	@./scripts/test-dind.sh $(TARGET)
 
+E2E_JOB_IMAGE ?= node:24-bookworm@sha256:934240a162082fd8b8a2f90cd5114446443f1eba1c5378f6687167ca405e6584 # renovate: datasource=docker
+SERVICE_IMAGE ?= nginx:1.31.4-alpine@sha256:db35bfc6b2951e7f8a72db5db120288c127ffaeeb4a6d4b95a26fead017d5913 # renovate: datasource=docker
+E2E_GITEA_IMAGE ?= gitea/gitea:main-nightly
+E2E_CONCURRENCY ?= 8
+
+.PHONY: test-e2e
+test-e2e: ## run Gitea compatibility tests against E2E_GITEA_IMAGE
+	@E2E_CONCURRENCY=$(E2E_CONCURRENCY) E2E_GITEA_IMAGE=$(E2E_GITEA_IMAGE) E2E_JOB_IMAGE=$(E2E_JOB_IMAGE) GO=$(GO) SERVICE_IMAGE=$(SERVICE_IMAGE) ./tools/test-e2e.sh
+
 .PHONY: install
 install: $(GOFILES) ## install the runner binary via `go install`
 	$(GO) install -v -tags '$(TAGS)' -ldflags '-s -w $(EXTLDFLAGS) $(LDFLAGS)'

README.md

@@ -158,6 +158,32 @@ An edit keeps the comments and the key order of the file. Indentation becomes tw
 
 `config get`, `set`, `add` and `remove` use `config.yaml` (or `config.yml`) from the working directory, then from the directory of the binary, and print their choice to stderr. `config init` writes `config.yaml` in the working directory, and refuses to overwrite an existing config without `--force`. Pass `-c` for another path.
 
+#### Tool cache
+
+Setup actions like `setup-go` install tools into `RUNNER_TOOL_CACHE`, which is `/opt/hostedtoolcache` inside a job. `runner.tool_cache_mode` selects what backs it:
+
+| Mode | Tool cache | Trade-off |
+| --- | --- | --- |
+| `none` (default) | Per job, provided by the job image | A version the image lacks is downloaded in every job |
+| `shared` | One volume reused by every job | Two jobs writing the same tool version at once corrupt it, so use it only with `runner.capacity: 1` |
+
+With `none`, tools must come from the job image. Install them into `/opt/hostedtoolcache/<tool>/<version>/<arch>`, with an empty `<arch>.complete` file next to the directory:
+
+```dockerfile
+RUN GO=$(curl -fsSL 'https://go.dev/dl/?mode=json' | grep -oP '"version": "\Kgo1\.26\.[0-9]*' | head -1); \
+  DIR="/opt/hostedtoolcache/go/${GO#go}/x64" && \
+  mkdir -p "$(dirname "$DIR")" && \
+  curl -fsSL "https://dl.google.com/go/${GO}.linux-amd64.tar.gz" | tar -xz -C /tmp && \
+  mv /tmp/go "$DIR" && \
+  touch "${DIR}.complete"
+```
+
+A workflow requesting a minor version, `go-version: "1.26"`, resolves to the newest matching version in the cache, so a patch update in the image still hits it.
+
+Of the [runner images](https://gitea.com/gitea/runner-images), the `-full` flavour is the one that ships tools in this layout.
+
+`gitea-runner exec` reads no config file and takes `--tool-cache-mode` instead, defaulting to `none`.
+
 #### Environment variables
 
 Earlier releases let a few environment variables (`GITEA_DEBUG`, `GITEA_TRACE`, `GITEA_RUNNER_CAPACITY`, `GITEA_RUNNER_FILE`, `GITEA_RUNNER_ENVIRON`, `GITEA_RUNNER_ENV_FILE`) override parts of the config. They are gone, use the YAML file for all settings. The Docker images still read their own variables, such as `RUNNER_STATE_FILE`, see [scripts/run.sh](scripts/run.sh) and the container documentation below.
@@ -202,7 +228,7 @@ a workflow with `runs-on: ubuntu-latest` is executed in the `runner-images:ubunt
 
 Names may themselves contain a colon (for example `pool:e57e18d4-10d4-406f-93bf-60f127221bdd`); only `host` and `docker` are treated as schemas.
 
-If a job's `runs-on` matches none of the runner's labels, the job still runs, in the default `docker.gitea.com/runner-images:ubuntu-latest` image. Images maintained for this purpose are listed at [gitea/runner-images](https://gitea.com/gitea/runner-images).
+If a job's `runs-on` matches none of the runner's labels, or sets no `runs-on` at all, it still runs: in `runner.default_image` where docker is available, on the host where it is not. Images maintained for this purpose are listed at [gitea/runner-images](https://gitea.com/gitea/runner-images).
 
 Labels are chosen at registration time (`--labels`, or the interactive prompt) and can be changed afterwards by editing `runner.labels` in the config file, or in the Gitea UI under the runner's settings.
 
@@ -273,6 +299,12 @@ A password in a proxy URL is hidden in job logs. Any step can still read it, bec
 
 Each runner starts its own cache server automatically. Cache entries are local to that runner — runners do not share a cache by default.
 
+**Eviction**
+
+An entry nothing has read or written for `retention` is removed, and a repository past `repo_size_limit` loses its least recently accessed entries until it fits; `size_limit` caps the whole cache the same way. Age alone never retires an entry still in use, and whatever these allow, the cache keeps free space above `health_check.min_free_disk_space_mb` when health checks are enabled.
+
+These apply where the cache server runs, so on a shared server they belong in *its* config, not the runners'. See `retention`, `repo_size_limit`, `size_limit` and `sweep_interval` in [config.example.yaml](internal/pkg/config/config.example.yaml) for units and defaults.
+
 **Cache service v2**
 
 `actions/cache@v4.2` and later can use the *cache service v2* API. The runner serves it from the same store as v1, on by default, and it works with `external_server`. Turn it off with:
@@ -282,7 +314,7 @@ cache:
   v2: false
 ```
 
-Those actions refuse any host they do not take for GitHub. Rather than misreport the server URL, the runner edits that check out of the action's own bundle and keeps the untouched copy beside it; a bundle it does not recognise is left alone and keeps to v1. The same edit lets the stock `actions/upload-artifact` and `actions/download-artifact` work from `v4.4.0` on, without the `gitea-upload-artifact` fork.
+Those actions refuse any host they do not take for GitHub. Rather than misreport the server URL, the runner edits that check out of the action's own bundle on its way into the job, undone whenever the action is downloaded again. A bundle it does not recognise is left alone and keeps to v1. The same edit lets the stock `actions/upload-artifact` and `actions/download-artifact` work from `v4.4.0` on, without the `gitea-upload-artifact` fork, so it is made whatever `v2` says: that setting only governs the API the runner advertises. Set `runner.patch_actions: false` to leave every bundle exactly as shipped, an escape hatch for an action the edit breaks. The artifact actions then refuse again and the cache client keeps to v1.
 
 **Shared cache across multiple runners**
 
@@ -313,6 +345,8 @@ Run one dedicated `gitea-runner cache-server` that all runners point at.
      # external_secret_file: /path/to/secret # secret can also be passed via a file
    ```
 
+Jobs reach the cache server at `external_server`, so when a reverse proxy fronts the server, point `external_server` at the proxy. The cache server itself needs no extra configuration.
+
 Alternatively, mount the same NFS/CIFS share on every runner and point `cache.dir` at it — simpler, but with weaker isolation between repositories.
 
 **S3 / MinIO** — mount object storage as a FUSE filesystem (e.g. [s3fs](https://github.com/s3fs-fuse/s3fs-fuse) or [goofys](https://github.com/kahing/goofys)) and set `cache.dir` to the mount point.
@@ -352,6 +386,16 @@ Both hooks are synchronous and block the job while they run. Either one exiting
 
 See **[docs/job-hooks.md](docs/job-hooks.md)** for the execution order, environment, and platform notes.
 
+#### Local job logs (`log.job.dir`)
+
+Set `log.job.dir` to a path and the runner writes a copy of every task's log there as `<start time>-task-<id>.log`: the rows exactly as Gitea received them, with the same secrets masked and the job's result on the last line. Off by default, and what Gitea shows does not change.
+
+#### Secret masking
+
+A job's secrets and its `::add-mask::` values are hidden from what the runner writes and uploads: the job log, the local copy above, job summaries, and the names of the containers it creates. A job output carrying one is skipped with a warning rather than sent masked, as GitHub does, so a downstream `needs.<job>.outputs.<name>` reading it is empty.
+
+`log.job.retention` (default `168h`) is how long a log is kept, expired ones being deleted as new tasks start, and `log.job.max_size` (default `1GB`) caps one log. Keep `retention` above `runner.timeout` so a long job cannot outlive its own log, and prefer local disk, the file is written while the job runs. Only the runner's own user can read it.
+
 ### Example Deployments
 
 Check out the [examples](examples) directory for sample deployment types.

act/artifactcache/handler.go

@@ -5,12 +5,13 @@
 package artifactcache
 
 import (
+	"cmp"
 	"context"
 	"crypto/hmac"
 	"crypto/rand"
 	"crypto/sha256"
 	"encoding/hex"
-	"encoding/json"
+	"encoding/json/v2"
 	"errors"
 	"fmt"
 	"io"
@@ -20,13 +21,15 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
+	"slices"
 	"strconv"
 	"strings"
 	"sync"
 	"sync/atomic"
 	"time"
 
-	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/internal/pkg/disk"
 
 	"github.com/julienschmidt/httprouter"
 	"github.com/sirupsen/logrus"
@@ -59,6 +62,9 @@ type JobCredential struct {
 	// remote runner registers with.
 	Results     string `json:"results"`
 	InsecureTLS bool   `json:"insecure_tls"`
+
+	// PublicURL is this server as a reverse proxy makes the job reach it, not the listen address.
+	PublicURL string `json:"public_url"`
 }
 
 // credEntry holds a registered job's credential along with an active
@@ -100,19 +106,38 @@ type Handler struct {
 
 	credMu sync.RWMutex
 	creds  map[string]*credEntry
+
+	policy Policy
+
+	// freeDisk is a field so tests can drive evictForFreeSpace without a full volume.
+	freeDisk func(string) (uint64, error)
+}
+
+// Options configures a cache server started by StartHandler; the zero value is usable.
+type Options struct {
+	Dir        string
+	OutboundIP string
+	Port       uint16
+
+	// InternalSecret, when non-empty, enables a control-plane API at
+	// /_internal/{register,revoke} that lets a remote runner pre-register the
+	// per-job ACTIONS_RUNTIME_TOKENs it expects this server to honor. The
+	// embedded in-process handler leaves it empty and registers tokens via the
+	// in-process RegisterJob method directly.
+	InternalSecret string
+
+	Policy Policy
+	Logger logrus.FieldLogger
 }
 
 // StartHandler opens the on-disk cache store and starts the HTTP server.
-//
-// internalSecret, when non-empty, enables a control-plane API at
-// /_internal/{register,revoke} that lets a remote runner pre-register the
-// per-job ACTIONS_RUNTIME_TOKENs it expects this server to honor. The
-// embedded in-process handler leaves it empty and registers tokens via the
-// in-process RegisterJob method directly.
-func StartHandler(dir, outboundIP string, port uint16, internalSecret string, logger logrus.FieldLogger) (*Handler, error) {
+func StartHandler(opts Options) (*Handler, error) {
+	dir, logger := opts.Dir, opts.Logger
 	h := &Handler{
 		creds:          make(map[string]*credEntry),
-		internalSecret: internalSecret,
+		internalSecret: opts.InternalSecret,
+		policy:         opts.Policy.withDefaults(),
+		freeDisk:       disk.FreeBytes,
 	}
 
 	if logger == nil {
@@ -142,8 +167,8 @@ func StartHandler(dir, outboundIP string, port uint16, internalSecret string, lo
 	}
 	h.storage = storage
 
-	if outboundIP != "" {
-		h.outboundIP = outboundIP
+	if opts.OutboundIP != "" {
+		h.outboundIP = opts.OutboundIP
 	} else if ip := common.GetOutboundIP(); ip == nil {
 		return nil, errors.New("unable to determine outbound IP address")
 	} else {
@@ -182,7 +207,7 @@ func StartHandler(dir, outboundIP string, port uint16, internalSecret string, lo
 	// can break Docker Desktop variants where the host's outbound IP is not
 	// routable from inside the container network. Authentication is enforced
 	// by the bearer middleware and per-repo scoping, not by reachability.
-	listener, err := net.Listen("tcp", fmt.Sprintf(":%d", port))
+	listener, err := net.Listen("tcp", fmt.Sprintf(":%d", opts.Port))
 	if err != nil {
 		return nil, err
 	}
@@ -212,6 +237,13 @@ func (h *Handler) ExternalURL() string {
 	return fmt.Sprintf("http://%s:%d", h.outboundIP, h.port)
 }
 
+func (h *Handler) baseURL(cred JobCredential) string {
+	if base := strings.TrimRight(cred.PublicURL, "/"); base != "" {
+		return base
+	}
+	return h.ExternalURL()
+}
+
 // RegisterJob makes token a valid bearer credential for cache requests from
 // the given repository and returns a function that removes it. The runner
 // calls this at job start and defers the returned func so that the credential
@@ -325,8 +357,8 @@ func (h *Handler) Close() error {
 
 func (h *Handler) openDB() (*bolthold.Store, error) {
 	return bolthold.Open(filepath.Join(h.dir, "bolt.db"), 0o644, &bolthold.Options{
-		Encoder: json.Marshal,
-		Decoder: json.Unmarshal,
+		Encoder: func(value any) ([]byte, error) { return json.Marshal(value) },
+		Decoder: func(data []byte, value any) error { return json.Unmarshal(data, value) },
 		Options: &bbolt.Options{
 			Timeout:      5 * time.Second,
 			NoGrowSync:   bbolt.DefaultOptions.NoGrowSync,
@@ -359,7 +391,7 @@ func (h *Handler) find(w http.ResponseWriter, r *http.Request, _ httprouter.Para
 	}
 	h.responseJSON(w, r, 200, map[string]any{
 		"result":          "hit",
-		"archiveLocation": h.signedArtifactURL(cache.ID, time.Now().Add(artifactURLTTL)),
+		"archiveLocation": h.signedArtifactURL(cred, cache.ID, time.Now().Add(artifactURLTTL)),
 		"cacheKey":        cache.Key,
 	})
 }
@@ -380,6 +412,9 @@ func (h *Handler) lookupCache(db *bolthold.Store, repo string, keys []string, ve
 		_ = db.Delete(cache.ID, cache)
 		return nil, nil //nolint:nilnil // absence is not an error here
 	}
+	// Handing out a download URL counts as access, or eviction could drop the entry between
+	// this call and the GET that follows it.
+	h.touch(db, cache)
 	return cache, nil
 }
 
@@ -387,13 +422,15 @@ func (h *Handler) lookupCache(db *bolthold.Store, repo string, keys []string, ve
 func (h *Handler) reserve(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
 	cred := credFromContext(r.Context())
 	api := &Request{}
-	if err := json.NewDecoder(r.Body).Decode(api); err != nil {
+	if err := json.UnmarshalRead(r.Body, api); err != nil {
 		h.responseJSON(w, r, 400, err)
 		return
 	}
 
-	cache := api.ToCache()
-	cache.Repo = cred.Repo
+	cache := &Cache{Repo: cred.Repo, Key: api.Key, Version: api.Version, Size: api.Size}
+	if cache.Size == 0 {
+		cache.Size = -1
+	}
 	db, err := h.openDB()
 	if err != nil {
 		h.responseJSON(w, r, 500, err)
@@ -517,13 +554,22 @@ func (h *Handler) commitCache(cache *Cache) error {
 	// write real size back to cache, it may be different from the current value when the request doesn't specify it.
 	cache.Size = written
 	cache.Complete = true
+	cache.UsedAt = time.Now().Unix() // a just-written entry counts as accessed, so it cannot be its own eviction victim
 
 	db, err := h.openDB()
 	if err != nil {
 		return err
 	}
 	defer db.Close()
-	return db.Update(cache.ID, cache)
+	if err := db.Update(cache.ID, cache); err != nil {
+		return err
+	}
+	// A commit is the only thing that grows the store, so the only thing that can push the
+	// volume under the floor.
+	h.evictRepo(db, cache.Repo)
+	h.evictTotal(db)
+	h.evictForFreeSpace(db)
+	return nil
 }
 
 // GET /_apis/artifactcache/artifacts/:id
@@ -641,12 +687,12 @@ func (h *Handler) ResultsURL(cred JobCredential) string {
 	if h == nil || cred.Results == "" {
 		return ""
 	}
-	return h.ExternalURL()
+	return h.baseURL(cred)
 }
 
 func (h *Handler) internalRegister(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
 	var body internalRegisterBody
-	if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
+	if err := json.UnmarshalRead(r.Body, &body); err != nil {
 		h.responseJSON(w, r, http.StatusBadRequest, err)
 		return
 	}
@@ -662,7 +708,7 @@ func (h *Handler) internalRegister(w http.ResponseWriter, r *http.Request, _ htt
 // POST /_internal/revoke
 func (h *Handler) internalRevoke(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
 	var body internalRevokeBody
-	if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
+	if err := json.UnmarshalRead(r.Body, &body); err != nil {
 		h.responseJSON(w, r, http.StatusBadRequest, err)
 		return
 	}
@@ -700,16 +746,16 @@ func (h *Handler) computeSignature(purpose string, cacheID, exp int64) string {
 }
 
 // signedURL builds a URL under path that signedAuth accepts for the same purpose.
-func (h *Handler) signedURL(path, purpose string, cacheID uint64, exp time.Time) string {
+func (h *Handler) signedURL(cred JobCredential, path, purpose string, cacheID uint64, exp time.Time) string {
 	expUnix := exp.Unix()
 	q := url.Values{}
 	q.Set("exp", strconv.FormatInt(expUnix, 10))
 	q.Set("sig", h.computeSignature(purpose, int64(cacheID), expUnix))
-	return fmt.Sprintf("%s%s/%d?%s", h.ExternalURL(), path, cacheID, q.Encode())
+	return fmt.Sprintf("%s%s/%d?%s", h.baseURL(cred), path, cacheID, q.Encode())
 }
 
-func (h *Handler) signedArtifactURL(cacheID uint64, exp time.Time) string {
-	return h.signedURL(apiPath+"/artifacts", "", cacheID, exp)
+func (h *Handler) signedArtifactURL(cred JobCredential, cacheID uint64, exp time.Time) string {
+	return h.signedURL(cred, apiPath+"/artifacts", "", cacheID, exp)
 }
 
 // if not found, return (nil, nil) instead of an error.
@@ -811,12 +857,43 @@ func (h *Handler) touchCache(id uint64, requireIncomplete bool) error {
 }
 
 const (
-	keepUsed   = 30 * 24 * time.Hour
-	keepUnused = 7 * 24 * time.Hour
-	keepTemp   = 5 * time.Minute
-	keepOld    = 5 * time.Minute
+	miB = 1024 * 1024
+
+	defaultSweepInterval = time.Hour
+
+	// inUseGrace matches artifactURLTTL so an entry outlives every signed URL still usable
+	// for it, and no sweep cuts off a download in progress.
+	inUseGrace = artifactURLTTL
+
+	// uploadStallTimeout is how long a reservation may sit without a chunk before it counts
+	// as abandoned. Widening it also widens the window for findExactCache to hand a finalize
+	// a stale reservation.
+	uploadStallTimeout = 5 * time.Minute
+
+	defaultMinFreeDisk = 1024 * miB
 )
 
+// Policy bounds what the cache server keeps: a retention window counted from last access,
+// and size limits that evict least recently accessed first. A zero limit is no limit.
+type Policy struct {
+	Retention     time.Duration // Retention removes entries nothing has read or written within this window. Zero keeps them regardless of age.
+	RepoSizeLimit int64         // RepoSizeLimit caps one repository's completed entries in bytes, evicting least recently accessed first.
+	SizeLimit     int64         // SizeLimit caps every repository's completed entries together, in bytes.
+	SweepInterval time.Duration // SweepInterval is the minimum time between two eviction sweeps.
+	MinFreeDisk   int64         // MinFreeDisk is volume headroom the cache will not eat into. Tracks the runner's health-check floor rather than taking a key of its own.
+}
+
+func (p Policy) withDefaults() Policy {
+	// The limits default in config.LoadDefault, so a written 0 means off.
+	if p.MinFreeDisk <= 0 {
+		p.MinFreeDisk = defaultMinFreeDisk
+	}
+	if p.SweepInterval <= 0 {
+		p.SweepInterval = defaultSweepInterval
+	}
+	return p
+}
+
 func (h *Handler) gcCache() {
 	if h.gcing.Load() {
 		return
@@ -826,7 +903,7 @@ func (h *Handler) gcCache() {
 	}
 	defer h.gcing.Store(false)
 
-	if time.Since(h.gcAt) < time.Hour {
+	if time.Since(h.gcAt) < h.policy.SweepInterval {
 		h.logger.Debugf("skip gc: %v", h.gcAt.String())
 		return
 	}
@@ -839,95 +916,208 @@ func (h *Handler) gcCache() {
 	}
 	defer db.Close()
 
-	// Remove the caches which are not completed for a while, they are most likely to be broken.
-	var caches []*Cache
-	if err := db.Find(&caches, bolthold.
-		Where("UsedAt").Lt(time.Now().Add(-keepTemp).Unix()).
-		And("Complete").Eq(false),
-	); err != nil {
-		h.logger.Warnf("find caches: %v", err)
-	} else {
-		for _, cache := range caches {
-			h.storage.Remove(cache.ID)
-			if err := db.Delete(cache.ID, cache); err != nil {
-				h.logger.Warnf("delete cache: %v", err)
-				continue
-			}
-			h.logger.Infof("deleted cache: %+v", cache)
-		}
+	h.evictIncomplete(db)
+	h.evictExpired(db)
+	h.evictSuperseded(db)
+	h.evictOversized(db)
+	h.evictForFreeSpace(db)
+}
+
+// evictForFreeSpace bounds the volume itself, so it also covers bytes the cache never
+// accounted for.
+func (h *Handler) evictForFreeSpace(db *bolthold.Store) {
+	free, err := h.freeDisk(h.dir)
+	if err != nil {
+		h.logger.Debugf("free disk check: %v", err) // unsupported platform, treat as unavailable rather than full
+		return
+	}
+	if free >= uint64(h.policy.MinFreeDisk) {
+		return
 	}
 
-	// Remove the old caches which have not been used recently.
-	caches = caches[:0]
-	if err := db.Find(&caches, bolthold.
-		Where("UsedAt").Lt(time.Now().Add(-keepUnused).Unix()),
-	); err != nil {
-		h.logger.Warnf("find caches: %v", err)
-	} else {
-		for _, cache := range caches {
-			h.storage.Remove(cache.ID)
-			if err := db.Delete(cache.ID, cache); err != nil {
-				h.logger.Warnf("delete cache: %v", err)
-				continue
-			}
-			h.logger.Infof("deleted cache: %+v", cache)
-		}
+	caches := h.completedByUse(db)
+	total, shortfall := totalSize(caches), h.policy.MinFreeDisk-int64(free)
+	if total <= shortfall {
+		// Say so, or shedding everything and still being short reads as the backstop working.
+		h.logger.Warnf("cache volume is %d MiB short of the free space floor with only %d MiB of cache on it; something else is filling it", shortfall/miB, total/miB)
 	}
+	h.evictTo(db, caches, total-shortfall, "the cache volume")
+}
 
-	// Remove the old caches which are too old.
-	caches = caches[:0]
-	if err := db.Find(&caches, bolthold.
-		Where("CreatedAt").Lt(time.Now().Add(-keepUsed).Unix()),
-	); err != nil {
-		h.logger.Warnf("find caches: %v", err)
-	} else {
-		for _, cache := range caches {
-			h.storage.Remove(cache.ID)
-			if err := db.Delete(cache.ID, cache); err != nil {
-				h.logger.Warnf("delete cache: %v", err)
-				continue
-			}
-			h.logger.Infof("deleted cache: %+v", cache)
-		}
+// evictIncomplete removes uploads that stopped part way, which are most likely broken.
+func (h *Handler) evictIncomplete(db *bolthold.Store) {
+	h.sweep(db, bolthold.
+		Where("UsedAt").Lt(time.Now().Add(-uploadStallTimeout).Unix()).
+		And("Complete").Eq(false).
+		Index("UsedAt"))
+}
+
+func (h *Handler) evictExpired(db *bolthold.Store) {
+	if h.policy.Retention <= 0 {
+		return
 	}
+	// Never below inUseGrace, or a short retention would outrun a signed URL already issued.
+	window := max(h.policy.Retention, inUseGrace)
+	h.sweep(db, bolthold.Where("UsedAt").Lt(time.Now().Add(-window).Unix()).Index("UsedAt"))
+}
 
-	// Remove the old caches with the same key and version within the same
-	// repository, keep the latest one. Aggregation must include Repo so two
-	// repos that happen to share a (key, version) do not evict each other —
-	// otherwise per-repo scoping holds for reads but one repo can age
-	// another out after keepOld.
-	// Also keep the olds which have been used recently for a while in case of the cache is still in use.
-	if results, err := db.FindAggregate(
-		&Cache{},
-		bolthold.Where("Complete").Eq(true),
-		"Repo", "Key", "Version",
-	); err != nil {
+// evictSuperseded removes entries a newer one with the same key and version replaced. The
+// aggregation includes Repo so two repos sharing a (key, version) do not evict each other.
+func (h *Handler) evictSuperseded(db *bolthold.Store) {
+	results, err := db.FindAggregate(&Cache{}, bolthold.Where("Complete").Eq(true).Index("Complete"), "Repo", "Key", "Version")
+	if err != nil {
 		h.logger.Warnf("find aggregate caches: %v", err)
-	} else {
-		for _, result := range results {
-			if result.Count() <= 1 {
+		return
+	}
+	var caches []*Cache
+	for _, result := range results {
+		if result.Count() <= 1 {
+			continue
+		}
+		result.Sort("CreatedAt")
+		caches = caches[:0]
+		result.Reduction(&caches)
+		for _, cache := range caches[:len(caches)-1] {
+			if inUse(cache) {
 				continue
 			}
-			result.Sort("CreatedAt")
-			caches = caches[:0]
-			result.Reduction(&caches)
-			for _, cache := range caches[:len(caches)-1] {
-				if time.Since(time.Unix(cache.UsedAt, 0)) < keepOld {
-					// Keep it since it has been used recently, even if it's old.
-					// Or it could break downloading in process.
-					continue
-				}
-				h.storage.Remove(cache.ID)
-				if err := db.Delete(cache.ID, cache); err != nil {
-					h.logger.Warnf("delete cache: %v", err)
-					continue
-				}
-				h.logger.Infof("deleted cache: %+v", cache)
-			}
+			h.deleteCache(db, cache)
 		}
 	}
 }
 
+// evictOversized applies the per-repository limit, then the whole-store one. Only completed
+// entries count, since only those carry a size measured at commit rather than claimed.
+func (h *Handler) evictOversized(db *bolthold.Store) {
+	if h.policy.RepoSizeLimit > 0 {
+		byRepo := make(map[string][]*Cache)
+		for _, cache := range h.completedByUse(db) {
+			byRepo[cache.Repo] = append(byRepo[cache.Repo], cache)
+		}
+		for repo, caches := range byRepo {
+			h.evictTo(db, caches, h.policy.RepoSizeLimit, "repository "+repo)
+		}
+	}
+	h.evictTotal(db)
+}
+
+// evictTotal caps the store as a whole. It re-queries because the per-repo pass may have
+// deleted rows an earlier result still holds.
+func (h *Handler) evictTotal(db *bolthold.Store) {
+	if h.policy.SizeLimit <= 0 {
+		return
+	}
+	h.evictTo(db, h.completedByUse(db), h.policy.SizeLimit, "the cache")
+}
+
+// evictRepo reclaims space when a commit pushes a repo over, rather than at the next sweep.
+func (h *Handler) evictRepo(db *bolthold.Store, repo string) {
+	if h.policy.RepoSizeLimit <= 0 {
+		return
+	}
+	h.evictTo(db, h.cachesByUse(db, bolthold.Where("Repo").Eq(repo).And("Complete").Eq(true).Index("Repo")), h.policy.RepoSizeLimit, "repository "+repo)
+}
+
+// evictTo deletes until caches fit limit. caches must be ordered by UsedAt ascending.
+func (h *Handler) evictTo(db *bolthold.Store, caches []*Cache, limit int64, scope string) {
+	// An entry bigger than the limit never fits, so it goes on its own account instead of
+	// dragging every neighbour out first and then following them next sweep.
+	fits := caches[:0]
+	for _, cache := range caches {
+		if cache.Size <= limit {
+			fits = append(fits, cache)
+			continue
+		}
+		if !inUse(cache) {
+			h.logger.Warnf("cache %q is %d MiB on its own, over the limit for %s; dropping it", cache.Key, cache.Size/miB, scope)
+			h.deleteCache(db, cache)
+		}
+	}
+	caches = fits
+
+	total := totalSize(caches)
+	var freed int64
+	for _, cache := range caches {
+		if total <= limit {
+			break
+		}
+		if inUse(cache) || !h.deleteCache(db, cache) {
+			continue
+		}
+		total -= cache.Size
+		freed += cache.Size
+	}
+	if freed > 0 {
+		h.logger.Warnf("evicted %d MiB from %s, least recently used first", freed/miB, scope)
+	}
+}
+
+// inUse reports whether an entry was read or written recently enough that removing it
+// could break a download in progress.
+func inUse(cache *Cache) bool {
+	return time.Since(time.Unix(cache.UsedAt, 0)) < inUseGrace
+}
+
+// touch stamps UsedAt through the caller's store, a bolt write on the read path. It cannot
+// go through touchCache, which opens its own store and would block on the exclusive lock
+// for as long as the caller holds one.
+func (h *Handler) touch(db *bolthold.Store, cache *Cache) {
+	cache.UsedAt = time.Now().Unix()
+	if err := db.Update(cache.ID, cache); err != nil {
+		h.logger.Warnf("touch cache: %v", err)
+	}
+}
+
+func (h *Handler) sweep(db *bolthold.Store, query *bolthold.Query) {
+	for _, cache := range h.caches(db, query) {
+		h.deleteCache(db, cache)
+	}
+}
+
+func (h *Handler) caches(db *bolthold.Store, query *bolthold.Query) []*Cache {
+	var caches []*Cache
+	if err := db.Find(&caches, query); err != nil {
+		h.logger.Warnf("find caches: %v", err)
+	}
+	return caches
+}
+
+// cachesByUse returns matches least recently accessed first, sorting here rather than with
+// bolthold's SortBy, which reflects over every field it compares.
+func (h *Handler) cachesByUse(db *bolthold.Store, query *bolthold.Query) []*Cache {
+	caches := h.caches(db, query)
+	slices.SortFunc(caches, func(a, b *Cache) int { return cmp.Compare(a.UsedAt, b.UsedAt) })
+	return caches
+}
+
+// completedByUse returns every entry the size limits count, least recently accessed first.
+func (h *Handler) completedByUse(db *bolthold.Store) []*Cache {
+	return h.cachesByUse(db, bolthold.Where("Complete").Eq(true).Index("Complete"))
+}
+
+func totalSize(caches []*Cache) int64 {
+	var total int64
+	for _, cache := range caches {
+		total += cache.Size
+	}
+	return total
+}
+
+// deleteCache drops an entry and its bytes, reporting whether it went fully. The blob goes
+// first, so a failed unlink leaves the row for the next sweep instead of orphaning bytes.
+func (h *Handler) deleteCache(db *bolthold.Store, cache *Cache) bool {
+	if err := h.storage.Remove(cache.ID); err != nil {
+		h.logger.Warnf("remove cache blob: %v", err)
+		return false
+	}
+	if err := db.Delete(cache.ID, cache); err != nil {
+		h.logger.Warnf("delete cache: %v", err)
+		return false
+	}
+	h.logger.Infof("deleted cache: %+v", cache)
+	return true
+}
+
 func (h *Handler) responseJSON(w http.ResponseWriter, r *http.Request, code int, v ...any) {
 	w.Header().Set("Content-Type", "application/json; charset=utf-8")
 	var data []byte

act/artifactcache/handler_test.go

@@ -7,7 +7,8 @@ package artifactcache
 import (
 	"bytes"
 	"crypto/rand"
-	"encoding/json"
+	"encoding/json/v2"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -41,16 +42,19 @@ func (b *bearerTransport) RoundTrip(r *http.Request) (*http.Response, error) {
 
 var testClient = &http.Client{Transport: &bearerTransport{token: testToken}}
 
+// testRetention mirrors config.DefaultCacheRetention; Policy has no defaults of its own.
+const testRetention = 7 * 24 * time.Hour
+
 // signArtifactURL builds a signed download URL the same way the server does;
 // tests use it to reach the get handler directly without going through a
 // find/cache-hit round trip.
 func signArtifactURL(h *Handler, id int64) string {
-	return h.signedArtifactURL(uint64(id), time.Now().Add(artifactURLTTL))
+	return h.signedArtifactURL(JobCredential{}, uint64(id), time.Now().Add(artifactURLTTL))
 }
 
 func TestHandler(t *testing.T) {
 	dir := filepath.Join(t.TempDir(), "artifactcache")
-	handler, err := StartHandler(dir, "", 0, "", nil)
+	handler, err := StartHandler(Options{Dir: dir})
 	require.NoError(t, err)
 	handler.RegisterJob(testToken, JobCredential{Repo: testRepo})
 
@@ -132,7 +136,7 @@ func TestHandler(t *testing.T) {
 			defer resp.Body.Close()
 			assert.Equal(t, 200, resp.StatusCode)
 
-			require.NoError(t, json.NewDecoder(resp.Body).Decode(&first))
+			require.NoError(t, json.UnmarshalRead(resp.Body, &first))
 			assert.NotZero(t, first.CacheID)
 		}
 		{
@@ -147,7 +151,7 @@ func TestHandler(t *testing.T) {
 			defer resp.Body.Close()
 			assert.Equal(t, 200, resp.StatusCode)
 
-			require.NoError(t, json.NewDecoder(resp.Body).Decode(&second))
+			require.NoError(t, json.UnmarshalRead(resp.Body, &second))
 			assert.NotZero(t, second.CacheID)
 		}
 
@@ -200,7 +204,7 @@ func TestHandler(t *testing.T) {
 			got := struct {
 				CacheID uint64 `json:"cacheId"`
 			}{}
-			require.NoError(t, json.NewDecoder(resp.Body).Decode(&got))
+			require.NoError(t, json.UnmarshalRead(resp.Body, &got))
 			id = got.CacheID
 		}
 		{
@@ -255,7 +259,7 @@ func TestHandler(t *testing.T) {
 			got := struct {
 				CacheID uint64 `json:"cacheId"`
 			}{}
-			require.NoError(t, json.NewDecoder(resp.Body).Decode(&got))
+			require.NoError(t, json.UnmarshalRead(resp.Body, &got))
 			id = got.CacheID
 		}
 		{
@@ -311,7 +315,7 @@ func TestHandler(t *testing.T) {
 			got := struct {
 				CacheID uint64 `json:"cacheId"`
 			}{}
-			require.NoError(t, json.NewDecoder(resp.Body).Decode(&got))
+			require.NoError(t, json.UnmarshalRead(resp.Body, &got))
 			id = got.CacheID
 		}
 		{
@@ -358,7 +362,7 @@ func TestHandler(t *testing.T) {
 			got := struct {
 				CacheID uint64 `json:"cacheId"`
 			}{}
-			require.NoError(t, json.NewDecoder(resp.Body).Decode(&got))
+			require.NoError(t, json.UnmarshalRead(resp.Body, &got))
 			id = got.CacheID
 		}
 
@@ -409,7 +413,7 @@ func TestHandler(t *testing.T) {
 			got := struct {
 				CacheID uint64 `json:"cacheId"`
 			}{}
-			require.NoError(t, json.NewDecoder(resp.Body).Decode(&got))
+			require.NoError(t, json.UnmarshalRead(resp.Body, &got))
 			id = got.CacheID
 		}
 		{
@@ -489,7 +493,7 @@ func TestHandler(t *testing.T) {
 			ArchiveLocation string `json:"archiveLocation"`
 			CacheKey        string `json:"cacheKey"`
 		}{}
-		require.NoError(t, json.NewDecoder(resp.Body).Decode(&got))
+		require.NoError(t, json.UnmarshalRead(resp.Body, &got))
 		assert.Equal(t, "hit", got.Result)
 		assert.Equal(t, keys[except], got.CacheKey)
 
@@ -524,7 +528,7 @@ func TestHandler(t *testing.T) {
 				ArchiveLocation string `json:"archiveLocation"`
 				CacheKey        string `json:"cacheKey"`
 			}{}
-			require.NoError(t, json.NewDecoder(resp.Body).Decode(&got))
+			require.NoError(t, json.UnmarshalRead(resp.Body, &got))
 			assert.Equal(t, "hit", got.Result)
 			assert.Equal(t, key, got.CacheKey)
 			assert.NotEqual(t, strings.ToLower(key), got.CacheKey)
@@ -573,7 +577,7 @@ func TestHandler(t *testing.T) {
 			ArchiveLocation string `json:"archiveLocation"`
 			CacheKey        string `json:"cacheKey"`
 		}{}
-		require.NoError(t, json.NewDecoder(resp.Body).Decode(&got))
+		require.NoError(t, json.UnmarshalRead(resp.Body, &got))
 		assert.Equal(t, keys[expect], got.CacheKey)
 
 		contentResp, err := testClient.Get(got.ArchiveLocation)
@@ -629,7 +633,7 @@ func TestHandler(t *testing.T) {
 			ArchiveLocation string `json:"archiveLocation"`
 			CacheKey        string `json:"cacheKey"`
 		}{}
-		require.NoError(t, json.NewDecoder(resp.Body).Decode(&got))
+		require.NoError(t, json.UnmarshalRead(resp.Body, &got))
 		assert.Equal(t, keys[expect], got.CacheKey)
 
 		contentResp, err := testClient.Get(got.ArchiveLocation)
@@ -656,7 +660,7 @@ func backdateCache(t *testing.T, handler *Handler, key string, age time.Duration
 	require.NoError(t, db.Update(caches[0].ID, caches[0]))
 }
 
-func uploadCacheNormally(t *testing.T, base, key, version string, content []byte) { //nolint:unparam // pre-existing issue from nektos/act
+func uploadCacheNormally(t *testing.T, base, key, version string, content []byte) {
 	var id uint64
 	{
 		body, err := json.Marshal(&Request{
@@ -673,7 +677,7 @@ func uploadCacheNormally(t *testing.T, base, key, version string, content []byte
 		got := struct {
 			CacheID uint64 `json:"cacheId"`
 		}{}
-		require.NoError(t, json.NewDecoder(resp.Body).Decode(&got))
+		require.NoError(t, json.UnmarshalRead(resp.Body, &got))
 		id = got.CacheID
 	}
 	{
@@ -704,7 +708,7 @@ func uploadCacheNormally(t *testing.T, base, key, version string, content []byte
 			ArchiveLocation string `json:"archiveLocation"`
 			CacheKey        string `json:"cacheKey"`
 		}{}
-		require.NoError(t, json.NewDecoder(resp.Body).Decode(&got))
+		require.NoError(t, json.UnmarshalRead(resp.Body, &got))
 		assert.Equal(t, "hit", got.Result)
 		assert.Equal(t, key, got.CacheKey)
 		archiveLocation = got.ArchiveLocation
@@ -722,7 +726,7 @@ func uploadCacheNormally(t *testing.T, base, key, version string, content []byte
 
 func TestHandler_gcCache(t *testing.T) {
 	dir := filepath.Join(t.TempDir(), "artifactcache")
-	handler, err := StartHandler(dir, "", 0, "", nil)
+	handler, err := StartHandler(Options{Dir: dir, Policy: Policy{Retention: testRetention}})
 	require.NoError(t, err)
 
 	defer func() {
@@ -752,8 +756,8 @@ func TestHandler_gcCache(t *testing.T) {
 				Key:       "test_key_2",
 				Version:   "test_version",
 				Complete:  false,
-				UsedAt:    now.Add(-(keepTemp + time.Second)).Unix(),
-				CreatedAt: now.Add(-(keepTemp + time.Hour)).Unix(),
+				UsedAt:    now.Add(-(inUseGrace + time.Second)).Unix(),
+				CreatedAt: now.Add(-(inUseGrace + time.Hour)).Unix(),
 			},
 			Kept: false,
 		},
@@ -763,21 +767,21 @@ func TestHandler_gcCache(t *testing.T) {
 				Key:       "test_key_3",
 				Version:   "test_version",
 				Complete:  true,
-				UsedAt:    now.Add(-(keepUnused + time.Second)).Unix(),
-				CreatedAt: now.Add(-(keepUnused + time.Hour)).Unix(),
+				UsedAt:    now.Add(-(testRetention + time.Second)).Unix(),
+				CreatedAt: now.Add(-(testRetention + time.Hour)).Unix(),
 			},
 			Kept: false,
 		},
 		{
-			// should be removed, since it's used but too old.
+			// should be kept, since age alone does not retire an entry that is still used.
 			Cache: &Cache{
 				Key:       "test_key_3",
 				Version:   "test_version",
 				Complete:  true,
 				UsedAt:    now.Unix(),
-				CreatedAt: now.Add(-(keepUsed + time.Second)).Unix(),
+				CreatedAt: now.Add(-365 * 24 * time.Hour).Unix(),
 			},
-			Kept: false,
+			Kept: true,
 		},
 		{
 			// should be kept, since it has a newer edition but be used recently.
@@ -785,7 +789,7 @@ func TestHandler_gcCache(t *testing.T) {
 				Key:       "test_key_1",
 				Version:   "test_version",
 				Complete:  true,
-				UsedAt:    now.Add(-(keepOld - time.Minute)).Unix(),
+				UsedAt:    now.Add(-(inUseGrace - time.Minute)).Unix(),
 				CreatedAt: now.Add(-(time.Hour + time.Second)).Unix(),
 			},
 			Kept: true,
@@ -796,7 +800,7 @@ func TestHandler_gcCache(t *testing.T) {
 				Key:       "test_key_1",
 				Version:   "test_version",
 				Complete:  true,
-				UsedAt:    now.Add(-(keepOld + time.Second)).Unix(),
+				UsedAt:    now.Add(-(inUseGrace + time.Second)).Unix(),
 				CreatedAt: now.Add(-(time.Hour + time.Second)).Unix(),
 			},
 			Kept: false,
@@ -829,11 +833,265 @@ func TestHandler_gcCache(t *testing.T) {
 	require.NoError(t, db.Close())
 }
 
+// TestHandler_evictPolicy covers the non-default policies; TestHandler_gcCache covers the
+// defaults across every pass.
+func TestHandler_evictPolicy(t *testing.T) {
+	now := time.Now()
+	stale := func(d time.Duration) int64 { return now.Add(-d).Unix() }
+	mib := func(n int64) int64 { return n * miB }
+
+	for _, tc := range []struct {
+		name    string
+		policy  Policy
+		entries []*Cache
+		kept    []string
+	}{
+		{
+			name:   "a zero retention keeps an entry nothing has touched",
+			policy: Policy{Retention: 0},
+			entries: []*Cache{
+				{Key: "idle", UsedAt: stale(testRetention + time.Hour)},
+			},
+			kept: []string{"idle"},
+		},
+		{
+			name:   "evicts least recently accessed until the repository fits",
+			policy: Policy{RepoSizeLimit: mib(10)},
+			entries: []*Cache{
+				{Repo: "o/a", Key: "oldest", Size: mib(4), UsedAt: stale(3 * time.Hour)},
+				{Repo: "o/a", Key: "middle", Size: mib(4), UsedAt: stale(2 * time.Hour)},
+				{Repo: "o/a", Key: "newest", Size: mib(4), UsedAt: stale(time.Hour)},
+			},
+			kept: []string{"middle", "newest"},
+		},
+		{
+			name:   "spares entries that may still be downloading",
+			policy: Policy{RepoSizeLimit: mib(10)},
+			entries: []*Cache{
+				{Repo: "o/a", Key: "fresh_1", Size: mib(6), UsedAt: stale(time.Minute)},
+				{Repo: "o/a", Key: "fresh_2", Size: mib(6), UsedAt: stale(time.Minute)},
+			},
+			kept: []string{"fresh_1", "fresh_2"},
+		},
+		{
+			name:   "one repository over its limit leaves another alone",
+			policy: Policy{RepoSizeLimit: mib(10)},
+			entries: []*Cache{
+				{Repo: "o/a", Key: "a_old", Size: mib(6), UsedAt: stale(3 * time.Hour)},
+				{Repo: "o/a", Key: "a_new", Size: mib(6), UsedAt: stale(time.Hour)},
+				{Repo: "o/b", Key: "b_old", Size: mib(6), UsedAt: stale(4 * time.Hour)},
+			},
+			kept: []string{"a_new", "b_old"},
+		},
+		{
+			name:   "the total limit evicts across repositories once each fits its own",
+			policy: Policy{RepoSizeLimit: mib(10), SizeLimit: mib(12)},
+			entries: []*Cache{
+				{Repo: "o/a", Key: "a_old", Size: mib(8), UsedAt: stale(3 * time.Hour)},
+				{Repo: "o/b", Key: "b_new", Size: mib(8), UsedAt: stale(time.Hour)},
+			},
+			kept: []string{"b_new"},
+		},
+		{
+			// Retention below inUseGrace would otherwise drop an entry whose signed URL a job
+			// is still holding.
+			name:   "a retention shorter than the grace still spares a just-served entry",
+			policy: Policy{Retention: time.Minute},
+			entries: []*Cache{
+				{Key: "just_served", UsedAt: stale(2 * time.Minute)},
+				{Key: "idle", UsedAt: stale(time.Hour)},
+			},
+			kept: []string{"just_served"},
+		},
+		{
+			name:   "an entry over the limit goes without emptying the repository",
+			policy: Policy{RepoSizeLimit: mib(10)},
+			entries: []*Cache{
+				{Repo: "o/a", Key: "keeps", Size: mib(4), UsedAt: stale(3 * time.Hour)},
+				{Repo: "o/a", Key: "huge", Size: mib(20), UsedAt: stale(2 * time.Hour)},
+			},
+			kept: []string{"keeps"},
+		},
+		{
+			name:   "a zero limit keeps everything",
+			policy: Policy{RepoSizeLimit: 0},
+			entries: []*Cache{
+				{Repo: "o/a", Key: "huge_1", Size: mib(100), UsedAt: stale(3 * time.Hour)},
+				{Repo: "o/a", Key: "huge_2", Size: mib(100), UsedAt: stale(2 * time.Hour)},
+			},
+			kept: []string{"huge_1", "huge_2"},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			for _, e := range tc.entries {
+				e.Complete = true // only completed entries carry a measured size, so only they count
+			}
+			handler := newTestHandler(t, tc.policy, tc.entries...)
+			handler.gcAt = time.Time{} // ensure gcCache will not skip
+			handler.gcCache()
+			assert.ElementsMatch(t, tc.kept, keptKeys(t, handler, tc.entries))
+		})
+	}
+}
+
+// TestHandler_evictForFreeSpace proves the volume backstop sheds only what it must, and only
+// when the disk is actually short.
+func TestHandler_evictForFreeSpace(t *testing.T) {
+	free := func(n int64) func(string) (uint64, error) {
+		return func(string) (uint64, error) { return uint64(n), nil }
+	}
+
+	for _, tc := range []struct {
+		name     string
+		freeDisk func(string) (uint64, error)
+		kept     []string
+	}{
+		{"ample free space evicts nothing", free(defaultMinFreeDisk), []string{"oldest", "middle", "newest"}},
+		{"a small shortfall sheds one entry", free(defaultMinFreeDisk - 4*miB), []string{"middle", "newest"}},
+		{"a shortfall the cache cannot cover sheds all of it", free(0), nil},
+		{
+			"an unreadable volume is treated as unavailable, not as full",
+			func(string) (uint64, error) { return 0, errors.New("unsupported") },
+			[]string{"oldest", "middle", "newest"},
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			now := time.Now()
+			entries := []*Cache{
+				{Key: "oldest", Complete: true, Size: 4 * miB, UsedAt: now.Add(-3 * time.Hour).Unix()},
+				{Key: "middle", Complete: true, Size: 4 * miB, UsedAt: now.Add(-2 * time.Hour).Unix()},
+				{Key: "newest", Complete: true, Size: 4 * miB, UsedAt: now.Add(-time.Hour).Unix()},
+			}
+			handler := newTestHandler(t, Policy{}, entries...)
+			handler.freeDisk = tc.freeDisk
+
+			db, err := handler.openDB()
+			require.NoError(t, err)
+			handler.evictForFreeSpace(db)
+			require.NoError(t, db.Close())
+
+			assert.ElementsMatch(t, tc.kept, keptKeys(t, handler, entries))
+		})
+	}
+}
+
+// TestHandler_SweepKeepsEntryWhenBlobSurvives proves a failed unlink leaves the row in place,
+// so the next sweep retries rather than orphaning bytes no row points at and no limit counts.
+func TestHandler_SweepKeepsEntryWhenBlobSurvives(t *testing.T) {
+	cache := &Cache{Key: "stuck", Complete: true, UsedAt: time.Now().Add(-(testRetention + time.Hour)).Unix()}
+	handler := newTestHandler(t, Policy{Retention: testRetention}, cache)
+
+	// A non-empty directory where the blob belongs makes os.Remove fail on every platform.
+	blob := handler.storage.filename(cache.ID)
+	require.NoError(t, os.MkdirAll(blob, 0o755))
+	require.NoError(t, os.WriteFile(filepath.Join(blob, "held"), []byte("x"), 0o600))
+
+	handler.gcAt = time.Time{}
+	handler.gcCache()
+
+	assert.Equal(t, []string{"stuck"}, keptKeys(t, handler, []*Cache{cache}), "the entry must outlive a blob that could not be removed")
+}
+
+// TestHandler_FindProtectsFromEviction covers the window between a find handing out a signed
+// download URL and the GET that redeems it: the entry promised to a job must not be the next
+// eviction victim just because its last access predates the find.
+func TestHandler_FindProtectsFromEviction(t *testing.T) {
+	// 12 MiB against a 10 MiB limit, so exactly one entry has to go.
+	wanted := &Cache{Repo: testRepo, Key: "wanted", Version: "v", Complete: true, Size: 4 * miB, UsedAt: time.Now().Add(-3 * time.Hour).Unix()}
+	other := &Cache{Repo: testRepo, Key: "other", Version: "v", Complete: true, Size: 4 * miB, UsedAt: time.Now().Add(-2 * time.Hour).Unix()}
+	newest := &Cache{Repo: testRepo, Key: "newest", Version: "v", Complete: true, Size: 4 * miB, UsedAt: time.Now().Add(-time.Hour).Unix()}
+	handler := newTestHandler(t, Policy{RepoSizeLimit: 10 * miB}, wanted, other, newest)
+	writeBlob(t, handler, wanted.ID) // find only reports a hit when the blob is on disk
+
+	resp, err := testClient.Get(fmt.Sprintf("%s%s/cache?keys=wanted&version=v", handler.ExternalURL(), apiPath))
+	require.NoError(t, err)
+	defer resp.Body.Close()
+	require.Equal(t, 200, resp.StatusCode)
+
+	// Evict directly: the request above kicked off an async gcCache, and writing gcAt here
+	// to drive gcCache would race its read.
+	db, err := handler.openDB()
+	require.NoError(t, err)
+	defer func() { require.NoError(t, db.Close()) }()
+	handler.evictOversized(db)
+
+	require.NoError(t, db.Get(wanted.ID, &Cache{}), "the entry just promised to a job must survive")
+	assert.ErrorIs(t, db.Get(other.ID, &Cache{}), bolthold.ErrNotFound, "the next least recently used goes instead")
+}
+
+// TestHandler_evictOnCommit proves a repository that goes over its limit gets space back at
+// once, rather than waiting out the collection interval.
+func TestHandler_evictOnCommit(t *testing.T) {
+	full := &Cache{Repo: testRepo, Key: "full", Version: "v", Complete: true, Size: 4 * miB, UsedAt: time.Now().Add(-time.Hour).Unix()}
+	handler := newTestHandler(t, Policy{RepoSizeLimit: 4 * miB}, full)
+
+	// StartHandler already stamped gcAt, so the periodic sweep stays rate-limited out and
+	// only the commit path can evict.
+	uploadCacheNormally(t, handler.ExternalURL()+apiPath, "new", "v", []byte("some content"))
+
+	assert.Empty(t, keptKeys(t, handler, []*Cache{full}))
+}
+
+func TestHandler_gcCacheInterval(t *testing.T) {
+	cache := &Cache{Key: "temp", UsedAt: time.Now().Add(-time.Hour).Unix()}
+	// Half the default, so a sweep 45m ago is still inside the default but past this one.
+	handler := newTestHandler(t, Policy{SweepInterval: 30 * time.Minute}, cache)
+
+	handler.gcAt = time.Now().Add(-45 * time.Minute) // past the configured interval, still inside the default
+	handler.gcCache()
+	assert.Empty(t, keptKeys(t, handler, []*Cache{cache}))
+}
+
+// newTestHandler starts a handler with testToken registered, seeded with entries.
+func newTestHandler(t *testing.T, policy Policy, entries ...*Cache) *Handler {
+	t.Helper()
+	handler, err := StartHandler(Options{
+		Dir:        filepath.Join(t.TempDir(), "artifactcache"),
+		OutboundIP: "127.0.0.1",
+		Policy:     policy,
+	})
+	require.NoError(t, err)
+	t.Cleanup(func() { require.NoError(t, handler.Close()) })
+	handler.RegisterJob(testToken, JobCredential{Repo: testRepo})
+
+	db, err := handler.openDB()
+	require.NoError(t, err)
+	for _, e := range entries {
+		require.NoError(t, insertCache(db, e))
+	}
+	require.NoError(t, db.Close())
+	return handler
+}
+
+// keptKeys reports which of entries are still in the store.
+func keptKeys(t *testing.T, handler *Handler, entries []*Cache) []string {
+	t.Helper()
+	db, err := handler.openDB()
+	require.NoError(t, err)
+	defer func() { require.NoError(t, db.Close()) }()
+
+	var kept []string
+	for _, e := range entries {
+		if err := db.Get(e.ID, &Cache{}); err == nil {
+			kept = append(kept, e.Key)
+		}
+	}
+	return kept
+}
+
+// writeBlob gives an entry the on-disk bytes that find and get require.
+func writeBlob(t *testing.T, handler *Handler, id uint64) {
+	t.Helper()
+	require.NoError(t, handler.storage.Write(id, 0, strings.NewReader("a")))
+	_, err := handler.storage.Commit(id, 1)
+	require.NoError(t, err)
+}
+
 // TestHandler_RejectsMissingBearer covers the advisory's root cause:
 // unauthenticated access to management endpoints is now refused with 401.
 func TestHandler_RejectsMissingBearer(t *testing.T) {
 	dir := filepath.Join(t.TempDir(), "artifactcache")
-	handler, err := StartHandler(dir, "", 0, "", nil)
+	handler, err := StartHandler(Options{Dir: dir})
 	require.NoError(t, err)
 	defer handler.Close()
 
@@ -866,7 +1124,7 @@ func TestHandler_RejectsMissingBearer(t *testing.T) {
 // accepted after RegisterJob; stale/forged tokens cannot be replayed.
 func TestHandler_RejectsUnknownBearer(t *testing.T) {
 	dir := filepath.Join(t.TempDir(), "artifactcache")
-	handler, err := StartHandler(dir, "", 0, "", nil)
+	handler, err := StartHandler(Options{Dir: dir})
 	require.NoError(t, err)
 	defer handler.Close()
 
@@ -886,7 +1144,7 @@ func TestHandler_RejectsUnknownBearer(t *testing.T) {
 // working the moment the job ends instead of living for the runner's lifetime.
 func TestHandler_UnregisterRevokes(t *testing.T) {
 	dir := filepath.Join(t.TempDir(), "artifactcache")
-	handler, err := StartHandler(dir, "", 0, "", nil)
+	handler, err := StartHandler(Options{Dir: dir})
 	require.NoError(t, err)
 	defer handler.Close()
 
@@ -917,7 +1175,7 @@ func TestHandler_UnregisterRevokes(t *testing.T) {
 // invisible to queries scoped to repoB.
 func TestHandler_CrossRepoIsolation(t *testing.T) {
 	dir := filepath.Join(t.TempDir(), "artifactcache")
-	handler, err := StartHandler(dir, "", 0, "", nil)
+	handler, err := StartHandler(Options{Dir: dir})
 	require.NoError(t, err)
 	defer handler.Close()
 	handler.RegisterJob("token-a", JobCredential{Repo: "owner/repoA"})
@@ -939,7 +1197,7 @@ func TestHandler_CrossRepoIsolation(t *testing.T) {
 	var reserved struct {
 		CacheID uint64 `json:"cacheId"`
 	}
-	require.NoError(t, json.NewDecoder(resp.Body).Decode(&reserved))
+	require.NoError(t, json.UnmarshalRead(resp.Body, &reserved))
 	resp.Body.Close()
 	require.NotZero(t, reserved.CacheID)
 
@@ -983,7 +1241,7 @@ func TestHandler_CrossRepoIsolation(t *testing.T) {
 // working after artifactURLTTL even if the bearer token is still registered.
 func TestHandler_ArtifactSignature(t *testing.T) {
 	dir := filepath.Join(t.TempDir(), "artifactcache")
-	handler, err := StartHandler(dir, "", 0, "", nil)
+	handler, err := StartHandler(Options{Dir: dir})
 	require.NoError(t, err)
 	defer handler.Close()
 	handler.RegisterJob(testToken, JobCredential{Repo: testRepo})
@@ -998,7 +1256,7 @@ func TestHandler_ArtifactSignature(t *testing.T) {
 	})
 
 	t.Run("tampered signature", func(t *testing.T) {
-		good := handler.signedArtifactURL(1, time.Now().Add(artifactURLTTL))
+		good := signArtifactURL(handler, 1)
 		bad := good[:len(good)-4] + "dead"
 		resp, err := testClient.Get(bad)
 		require.NoError(t, err)
@@ -1007,7 +1265,7 @@ func TestHandler_ArtifactSignature(t *testing.T) {
 	})
 
 	t.Run("expired signature", func(t *testing.T) {
-		expired := handler.signedArtifactURL(1, time.Now().Add(-time.Second))
+		expired := handler.signedArtifactURL(JobCredential{}, 1, time.Now().Add(-time.Second))
 		resp, err := testClient.Get(expired)
 		require.NoError(t, err)
 		resp.Body.Close()
@@ -1016,10 +1274,10 @@ func TestHandler_ArtifactSignature(t *testing.T) {
 
 	t.Run("signature from a different server", func(t *testing.T) {
 		dir2 := filepath.Join(t.TempDir(), "artifactcache2")
-		other, err := StartHandler(dir2, "", 0, "", nil)
+		other, err := StartHandler(Options{Dir: dir2})
 		require.NoError(t, err)
 		defer other.Close()
-		otherURL := other.signedArtifactURL(1, time.Now().Add(artifactURLTTL))
+		otherURL := signArtifactURL(other, 1)
 		// Rewrite the host so the request still lands on our handler, but
 		// the signature was computed with a different secret.
 		parts := strings.SplitN(otherURL, apiPath, 2)
@@ -1038,13 +1296,13 @@ func TestHandler_ArtifactSignature(t *testing.T) {
 func TestHandler_SecretPersistsAcrossRestarts(t *testing.T) {
 	dir := filepath.Join(t.TempDir(), "artifactcache")
 
-	first, err := StartHandler(dir, "127.0.0.1", 0, "", nil)
+	first, err := StartHandler(Options{Dir: dir, OutboundIP: "127.0.0.1"})
 	require.NoError(t, err)
 	exp := time.Now().Add(artifactURLTTL).Unix()
 	sig := first.computeSignature("", 42, exp)
 	require.NoError(t, first.Close())
 
-	second, err := StartHandler(dir, "127.0.0.1", 0, "", nil)
+	second, err := StartHandler(Options{Dir: dir, OutboundIP: "127.0.0.1"})
 	require.NoError(t, err)
 	defer second.Close()
 
@@ -1056,7 +1314,7 @@ func TestHandler_SecretPersistsAcrossRestarts(t *testing.T) {
 // the auth refactor.
 func TestHandler_ArtifactSignatureDownload(t *testing.T) {
 	dir := filepath.Join(t.TempDir(), "artifactcache")
-	handler, err := StartHandler(dir, "", 0, "", nil)
+	handler, err := StartHandler(Options{Dir: dir})
 	require.NoError(t, err)
 	defer handler.Close()
 	handler.RegisterJob(testToken, JobCredential{Repo: testRepo})
@@ -1073,7 +1331,7 @@ func TestHandler_ArtifactSignatureDownload(t *testing.T) {
 	var hit struct {
 		ArchiveLocation string `json:"archiveLocation"`
 	}
-	require.NoError(t, json.NewDecoder(resp.Body).Decode(&hit))
+	require.NoError(t, json.UnmarshalRead(resp.Body, &hit))
 	resp.Body.Close()
 
 	require.Contains(t, hit.ArchiveLocation, "sig=")
@@ -1096,7 +1354,7 @@ func TestHandler_ArtifactSignatureDownload(t *testing.T) {
 // (restart mid-task, retry), which must not kill the live job's auth.
 func TestHandler_RegisterJob_RefCounted(t *testing.T) {
 	dir := filepath.Join(t.TempDir(), "artifactcache")
-	handler, err := StartHandler(dir, "", 0, "", nil)
+	handler, err := StartHandler(Options{Dir: dir})
 	require.NoError(t, err)
 	defer handler.Close()
 
@@ -1125,10 +1383,10 @@ func TestHandler_RegisterJob_RefCounted(t *testing.T) {
 
 // TestHandler_GC_PerRepoDedup ensures duplicate-pruning does not evict
 // another repo's entry. Two repos reserve the same (key, version); after the
-// keepOld window, GC must keep the one from each repo.
+// inUseGrace window, GC must keep the one from each repo.
 func TestHandler_GC_PerRepoDedup(t *testing.T) {
 	dir := filepath.Join(t.TempDir(), "artifactcache")
-	handler, err := StartHandler(dir, "", 0, "", nil)
+	handler, err := StartHandler(Options{Dir: dir})
 	require.NoError(t, err)
 	defer handler.Close()
 	handler.RegisterJob("tok-a", JobCredential{Repo: "owner/repoA"})
@@ -1142,7 +1400,7 @@ func TestHandler_GC_PerRepoDedup(t *testing.T) {
 	db, err := handler.openDB()
 	require.NoError(t, err)
 	now := time.Now().Unix()
-	stale := time.Now().Add(-keepOld - time.Minute).Unix()
+	stale := time.Now().Add(-inUseGrace - time.Minute).Unix()
 	a := &Cache{Repo: "owner/repoA", Key: key, Version: version, Complete: true, CreatedAt: stale, UsedAt: stale, Size: 1}
 	b := &Cache{Repo: "owner/repoB", Key: key, Version: version, Complete: true, CreatedAt: now, UsedAt: now, Size: 1}
 	require.NoError(t, insertCache(db, a))
@@ -1179,7 +1437,7 @@ func TestHandler_GC_PerRepoDedup(t *testing.T) {
 // register/revoke when the feature is off.
 func TestHandler_InternalAPI_Disabled(t *testing.T) {
 	dir := filepath.Join(t.TempDir(), "artifactcache")
-	handler, err := StartHandler(dir, "", 0, "", nil)
+	handler, err := StartHandler(Options{Dir: dir})
 	require.NoError(t, err)
 	defer handler.Close()
 
@@ -1197,7 +1455,7 @@ func TestHandler_InternalAPI_Disabled(t *testing.T) {
 func TestHandler_InternalAPI_AuthAndUsage(t *testing.T) {
 	dir := filepath.Join(t.TempDir(), "artifactcache")
 	const secret = "internal-secret"
-	handler, err := StartHandler(dir, "", 0, secret, nil)
+	handler, err := StartHandler(Options{Dir: dir, InternalSecret: secret})
 	require.NoError(t, err)
 	defer handler.Close()
 

act/artifactcache/handler_v2.go

@@ -5,7 +5,8 @@ package artifactcache
 
 import (
 	"cmp"
-	"encoding/json"
+	"encoding/json/jsontext"
+	"encoding/json/v2"
 	"encoding/xml"
 	"errors"
 	"fmt"
@@ -77,6 +78,7 @@ func (h *Handler) v2CreateCacheEntry(w http.ResponseWriter, r *http.Request, _ h
 		h.twirpError(w, r, twirpInternal, err)
 		return
 	} else if existing != nil {
+		h.touch(db, existing) // the client skips the upload, so this is the only sign the entry is still in use
 		h.twirpNotOK(w, r)
 		return
 	}
@@ -97,7 +99,7 @@ func (h *Handler) v2CreateCacheEntry(w http.ResponseWriter, r *http.Request, _ h
 
 	h.responseJSON(w, r, http.StatusOK, map[string]any{
 		"ok":                true,
-		"signed_upload_url": h.signedURL(blobPath, blobUploadPurpose, cache.ID, time.Now().Add(blobUploadURLTTL)),
+		"signed_upload_url": h.signedURL(cred, blobPath, blobUploadPurpose, cache.ID, time.Now().Add(blobUploadURLTTL)),
 	})
 }
 
@@ -127,7 +129,7 @@ func (h *Handler) v2FinalizeCacheEntryUpload(w http.ResponseWriter, r *http.Requ
 	}
 	db.Close() // commitCache needs the store closed
 
-	cache.Size, _ = cmp.Or(req.SizeBytes, req.SizeBytesCamel).Int64()
+	cache.Size = int64(cmp.Or(req.SizeBytes, req.SizeBytesCamel))
 	if err := h.commitCache(cache); err != nil {
 		h.logger.Errorf("finalize cache %d (%s): %v", cache.ID, cache.Key, err)
 		h.twirpNotOK(w, r)
@@ -168,7 +170,7 @@ func (h *Handler) v2GetCacheEntryDownloadURL(w http.ResponseWriter, r *http.Requ
 
 	h.responseJSON(w, r, http.StatusOK, map[string]any{
 		"ok":                  true,
-		"signed_download_url": h.signedArtifactURL(cache.ID, time.Now().Add(artifactURLTTL)),
+		"signed_download_url": h.signedArtifactURL(cred, cache.ID, time.Now().Add(artifactURLTTL)),
 		"matched_key":         cache.Key,
 	})
 }
@@ -244,10 +246,10 @@ type (
 	}
 
 	v2FinalizeRequest struct {
-		Key            string      `json:"key"`
-		Version        string      `json:"version"`
-		SizeBytes      json.Number `json:"size_bytes"`
-		SizeBytesCamel json.Number `json:"sizeBytes"`
+		Key            string     `json:"key"`
+		Version        string     `json:"version"`
+		SizeBytes      twirpInt64 `json:"size_bytes"`
+		SizeBytesCamel twirpInt64 `json:"sizeBytes"`
 	}
 
 	v2DownloadRequest struct {
@@ -258,6 +260,31 @@ type (
 	}
 )
 
+// twirpInt64 accepts its value as the JSON string the mapping prescribes or as a bare number.
+type twirpInt64 int64
+
+func (n *twirpInt64) UnmarshalJSONFrom(dec *jsontext.Decoder) error {
+	val, err := dec.ReadValue()
+	if err != nil {
+		return err
+	}
+	digits := []byte(val)
+	switch val.Kind() {
+	case 'n': // absent, keep the zero value
+		return nil
+	case '"':
+		if digits, err = jsontext.AppendUnquote(nil, val); err != nil {
+			return err
+		}
+	}
+	parsed, err := strconv.ParseInt(string(digits), 10, 64)
+	if err != nil {
+		return err
+	}
+	*n = twirpInt64(parsed)
+	return nil
+}
+
 func (d v2DownloadRequest) keys() []string {
 	restoreKeys := d.RestoreKeys
 	if len(restoreKeys) == 0 {
@@ -268,6 +295,6 @@ func (d v2DownloadRequest) keys() []string {
 
 func decodeTwirpRequest[T any](r *http.Request) (T, error) {
 	var req T
-	err := json.NewDecoder(io.LimitReader(r.Body, 1<<20)).Decode(&req)
+	err := json.UnmarshalRead(io.LimitReader(r.Body, 1<<20), &req)
 	return req, err
 }

act/artifactcache/handler_v2_test.go

@@ -6,12 +6,12 @@ package artifactcache
 import (
 	"bytes"
 	"encoding/base64"
-	"encoding/json"
+	"encoding/json/v2"
 	"fmt"
 	"io"
 	"net/http"
-	"path/filepath"
 	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -32,7 +32,7 @@ func v2Call(t *testing.T, handler *Handler, client *http.Client, method string,
 	require.Equal(t, http.StatusOK, resp.StatusCode)
 
 	got := map[string]any{}
-	require.NoError(t, json.NewDecoder(resp.Body).Decode(&got))
+	require.NoError(t, json.UnmarshalRead(resp.Body, &got))
 	return got
 }
 
@@ -66,16 +66,6 @@ func getURL(t *testing.T, url string) []byte {
 	return body
 }
 
-func startTestHandler(t *testing.T) *Handler {
-	t.Helper()
-
-	handler, err := StartHandler(filepath.Join(t.TempDir(), "artifactcache"), "127.0.0.1", 0, "", nil)
-	require.NoError(t, err)
-	t.Cleanup(func() { _ = handler.Close() })
-	handler.RegisterJob(testToken, JobCredential{Repo: testRepo})
-	return handler
-}
-
 // saveV2 runs the reserve/upload/finalize sequence and returns the finalize response along
 // with the upload URL it used.
 func saveV2(t *testing.T, handler *Handler, key, version string, content []byte) (finalized map[string]any, uploadURL string) {
@@ -97,7 +87,7 @@ func saveV2(t *testing.T, handler *Handler, key, version string, content []byte)
 // URLs it is handed: unsigned requests are refused, an upload URL cannot be replayed to read
 // or to replace a finalized entry.
 func TestCacheServiceV2RoundTrip(t *testing.T) {
-	handler := startTestHandler(t)
+	handler := newTestHandler(t, Policy{})
 	content := []byte("the cached archive")
 
 	unsigned := fmt.Sprintf("%s%s/1", handler.ExternalURL(), blobPath)
@@ -126,7 +116,7 @@ func TestCacheServiceV2RoundTrip(t *testing.T) {
 // A large archive is staged as blocks and only put in order by the final block list, so
 // blocks that arrive out of order must still be assembled the way the client asked.
 func TestCacheServiceV2BlockUpload(t *testing.T) {
-	handler := startTestHandler(t)
+	handler := newTestHandler(t, Policy{})
 
 	created := v2Call(t, handler, testClient, "CreateCacheEntry", map[string]any{"key": "blocks", "version": "v1"})
 	uploadURL, _ := created["signed_upload_url"].(string)
@@ -163,7 +153,7 @@ func TestCacheServiceV2BlockUpload(t *testing.T) {
 }
 
 func TestCacheServiceV2Lookups(t *testing.T) {
-	handler := startTestHandler(t)
+	handler := newTestHandler(t, Policy{})
 	saved, _ := saveV2(t, handler, "deps-abc", "v1", []byte("x"))
 	require.Equal(t, true, saved["ok"])
 
@@ -198,6 +188,18 @@ func TestCacheServiceV2Lookups(t *testing.T) {
 		assert.NotEmpty(t, reserved["signed_upload_url"])
 	})
 
+	t.Run("a proxied job is handed the address its runner registered", func(t *testing.T) {
+		const proxy = "https://cache.example.invalid"
+		handler.RegisterJob("proxied", JobCredential{Repo: testRepo, PublicURL: proxy + "/"})
+		client := &http.Client{Transport: &bearerTransport{token: "proxied"}}
+
+		created := v2Call(t, handler, client, "CreateCacheEntry", map[string]any{"key": "proxied-key", "version": "v1"})
+		assert.True(t, strings.HasPrefix(created["signed_upload_url"].(string), proxy+blobPath+"/"))
+
+		got := v2Call(t, handler, client, "GetCacheEntryDownloadURL", map[string]any{"key": "deps-abc", "version": "v1"})
+		assert.True(t, strings.HasPrefix(got["signed_download_url"].(string), proxy+apiPath+"/artifacts/"))
+	})
+
 	t.Run("finalizing without a reservation is not ok", func(t *testing.T) {
 		got := v2Call(t, handler, testClient, "FinalizeCacheEntryUpload", map[string]any{
 			"key": "never-reserved", "version": "v1", "size_bytes": 1,
@@ -225,7 +227,7 @@ func TestCacheServiceV2Lookups(t *testing.T) {
 		require.Equal(t, http.StatusOK, resp.StatusCode)
 
 		got := map[string]any{}
-		require.NoError(t, json.NewDecoder(resp.Body).Decode(&got))
+		require.NoError(t, json.UnmarshalRead(resp.Body, &got))
 		assert.Equal(t, "deps-abc", got["cacheKey"])
 		assert.NotEmpty(t, got["archiveLocation"])
 	})

act/artifactcache/model.go

@@ -10,23 +10,6 @@ type Request struct {
 	Size    int64  `json:"cacheSize"`
 }
 
-func (c *Request) ToCache() *Cache {
-	if c == nil {
-		return nil
-	}
-	ret := &Cache{
-		Key:     c.Key,
-		Version: c.Version,
-		Size:    c.Size,
-	}
-	if c.Size == 0 {
-		// So the request comes from old versions of actions, like `actions/cache@v2`.
-		// It doesn't send cache size. Set it to -1 to indicate that.
-		ret.Size = -1
-	}
-	return ret
-}
-
 type Cache struct {
 	ID        uint64 `json:"id" boltholdKey:"ID"`
 	Repo      string `json:"repo" boltholdIndex:"Repo"`

act/artifactcache/results_test.go

@@ -24,7 +24,7 @@ func TestFrontResultsService(t *testing.T) {
 	}))
 	defer gitea.Close()
 
-	handler, err := StartHandler(t.TempDir(), "127.0.0.1", 0, "", nil)
+	handler, err := StartHandler(Options{Dir: t.TempDir(), OutboundIP: "127.0.0.1"})
 	require.NoError(t, err)
 	defer handler.Close()
 	const token = "forward-token"

act/artifactcache/storage.go

@@ -143,9 +143,13 @@ func (s *Storage) Serve(w http.ResponseWriter, r *http.Request, id uint64) {
 	http.ServeFile(w, r, name)
 }
 
-func (s *Storage) Remove(id uint64) {
-	_ = os.Remove(s.filename(id))
-	_ = os.RemoveAll(s.tempDir(id))
+// Remove deletes an entry's blob and any staged parts. It reports failure so the caller can
+// keep the entry and retry, rather than dropping the only reference to bytes on disk.
+func (s *Storage) Remove(id uint64) error {
+	if err := os.Remove(s.filename(id)); err != nil && !os.IsNotExist(err) {
+		return err
+	}
+	return os.RemoveAll(s.tempDir(id))
 }
 
 func (s *Storage) filename(id uint64) string {

act/artifacts/server.go

@@ -6,7 +6,7 @@ package artifacts
 
 import (
 	"context"
-	"encoding/json"
+	"encoding/json/v2"
 	"errors"
 	"fmt"
 	"io"
@@ -17,7 +17,7 @@ import (
 	"strings"
 	"time"
 
-	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/act/common"
 
 	"github.com/julienschmidt/httprouter"
 )
@@ -50,65 +50,29 @@ type ResponseMessage struct {
 	Message string `json:"message"`
 }
 
-type WritableFile interface {
-	io.WriteCloser
-}
-
-type WriteFS interface {
-	OpenWritable(name string) (WritableFile, error)
-	OpenAppendable(name string) (WritableFile, error)
-}
-
-type readWriteFSImpl struct{}
-
-func (fwfs readWriteFSImpl) Open(name string) (fs.File, error) {
-	return os.Open(name)
-}
-
-func (fwfs readWriteFSImpl) OpenWritable(name string) (WritableFile, error) {
-	if err := os.MkdirAll(filepath.Dir(name), os.ModePerm); err != nil {
-		return nil, err
-	}
-	return os.OpenFile(name, os.O_CREATE|os.O_RDWR|os.O_TRUNC, 0o644)
-}
-
-func (fwfs readWriteFSImpl) OpenAppendable(name string) (WritableFile, error) {
-	if err := os.MkdirAll(filepath.Dir(name), os.ModePerm); err != nil {
-		return nil, err
-	}
-	file, err := os.OpenFile(name, os.O_CREATE|os.O_RDWR, 0o644)
-	if err != nil {
-		return nil, err
-	}
-
-	_, err = file.Seek(0, io.SeekEnd)
-	if err != nil {
-		return nil, err
-	}
-	return file, nil
-}
-
 var gzipExtension = ".gz__"
 
 func safeResolve(baseDir, relPath string) string {
 	return filepath.Join(baseDir, filepath.Clean(filepath.Join(string(os.PathSeparator), relPath)))
 }
 
-func uploads(router *httprouter.Router, baseDir string, fsys WriteFS) {
+func writeJSON(w http.ResponseWriter, value any) {
+	data, err := json.Marshal(value)
+	if err != nil {
+		panic(err)
+	}
+	if _, err := w.Write(data); err != nil {
+		panic(err)
+	}
+}
+
+func uploads(router *httprouter.Router, baseDir string) {
 	router.POST("/_apis/pipelines/workflows/:runId/artifacts", func(w http.ResponseWriter, req *http.Request, params httprouter.Params) {
 		runID := params.ByName("runId")
 
-		json, err := json.Marshal(FileContainerResourceURL{
+		writeJSON(w, FileContainerResourceURL{
 			FileContainerResourceURL: fmt.Sprintf("http://%s/upload/%s", req.Host, runID),
 		})
-		if err != nil {
-			panic(err)
-		}
-
-		_, err = w.Write(json)
-		if err != nil {
-			panic(err)
-		}
 	})
 
 	router.PUT("/upload/:runId", func(w http.ResponseWriter, req *http.Request, params httprouter.Params) {
@@ -122,67 +86,47 @@ func uploads(router *httprouter.Router, baseDir string, fsys WriteFS) {
 		safeRunPath := safeResolve(baseDir, runID)
 		safePath := safeResolve(safeRunPath, itemPath)
 
-		file, err := func() (WritableFile, error) {
-			contentRange := req.Header.Get("Content-Range")
-			if contentRange != "" && !strings.HasPrefix(contentRange, "bytes 0-") {
-				return fsys.OpenAppendable(safePath)
-			}
-			return fsys.OpenWritable(safePath)
-		}()
+		if err := os.MkdirAll(filepath.Dir(safePath), os.ModePerm); err != nil {
+			panic(err)
+		}
+		flags := os.O_CREATE | os.O_WRONLY | os.O_TRUNC
+		appendUpload := req.Header.Get("Content-Range")
+		if appendUpload != "" && !strings.HasPrefix(appendUpload, "bytes 0-") {
+			flags = os.O_CREATE | os.O_WRONLY | os.O_APPEND
+		}
+		file, err := os.OpenFile(safePath, flags, 0o644)
 		if err != nil {
 			panic(err)
 		}
 		defer file.Close()
-
-		writer, ok := file.(io.Writer)
-		if !ok {
-			panic(errors.New("File is not writable"))
-		}
-
 		if req.Body == nil {
-			panic(errors.New("No body given"))
+			panic(errors.New("no body given"))
 		}
 
-		_, err = io.Copy(writer, req.Body)
+		_, err = io.Copy(file, req.Body)
 		if err != nil {
 			panic(err)
 		}
 
-		json, err := json.Marshal(ResponseMessage{
+		writeJSON(w, ResponseMessage{
 			Message: "success",
 		})
-		if err != nil {
-			panic(err)
-		}
-
-		_, err = w.Write(json)
-		if err != nil {
-			panic(err)
-		}
 	})
 
 	router.PATCH("/_apis/pipelines/workflows/:runId/artifacts", func(w http.ResponseWriter, req *http.Request, params httprouter.Params) {
-		json, err := json.Marshal(ResponseMessage{
+		writeJSON(w, ResponseMessage{
 			Message: "success",
 		})
-		if err != nil {
-			panic(err)
-		}
-
-		_, err = w.Write(json)
-		if err != nil {
-			panic(err)
-		}
 	})
 }
 
-func downloads(router *httprouter.Router, baseDir string, fsys fs.FS) {
+func downloads(router *httprouter.Router, baseDir string) {
 	router.GET("/_apis/pipelines/workflows/:runId/artifacts", func(w http.ResponseWriter, req *http.Request, params httprouter.Params) {
 		runID := params.ByName("runId")
 
 		safePath := safeResolve(baseDir, runID)
 
-		entries, err := fs.ReadDir(fsys, safePath)
+		entries, err := os.ReadDir(safePath)
 		if err != nil {
 			panic(err)
 		}
@@ -195,18 +139,10 @@ func downloads(router *httprouter.Router, baseDir string, fsys fs.FS) {
 			})
 		}
 
-		json, err := json.Marshal(NamedFileContainerResourceURLResponse{
+		writeJSON(w, NamedFileContainerResourceURLResponse{
 			Count: len(list),
 			Value: list,
 		})
-		if err != nil {
-			panic(err)
-		}
-
-		_, err = w.Write(json)
-		if err != nil {
-			panic(err)
-		}
 	})
 
 	router.GET("/download/:container", func(w http.ResponseWriter, req *http.Request, params httprouter.Params) {
@@ -215,7 +151,7 @@ func downloads(router *httprouter.Router, baseDir string, fsys fs.FS) {
 		safePath := safeResolve(baseDir, filepath.Join(container, itemPath))
 
 		var files []ContainerItem
-		err := fs.WalkDir(fsys, safePath, func(path string, entry fs.DirEntry, err error) error {
+		err := filepath.WalkDir(safePath, func(path string, entry fs.DirEntry, err error) error {
 			if !entry.IsDir() {
 				rel, err := filepath.Rel(safePath, path)
 				if err != nil {
@@ -241,17 +177,9 @@ func downloads(router *httprouter.Router, baseDir string, fsys fs.FS) {
 			panic(err)
 		}
 
-		json, err := json.Marshal(ContainerItemResponse{
+		writeJSON(w, ContainerItemResponse{
 			Value: files,
 		})
-		if err != nil {
-			panic(err)
-		}
-
-		_, err = w.Write(json)
-		if err != nil {
-			panic(err)
-		}
 	})
 
 	router.GET("/artifact/*path", func(w http.ResponseWriter, req *http.Request, params httprouter.Params) {
@@ -259,15 +187,16 @@ func downloads(router *httprouter.Router, baseDir string, fsys fs.FS) {
 
 		safePath := safeResolve(baseDir, path)
 
-		file, err := fsys.Open(safePath)
+		file, err := os.Open(safePath)
 		if err != nil {
 			// try gzip file
-			file, err = fsys.Open(safePath + gzipExtension)
+			file, err = os.Open(safePath + gzipExtension)
 			if err != nil {
 				panic(err)
 			}
 			w.Header().Add("Content-Encoding", "gzip")
 		}
+		defer file.Close()
 
 		_, err = io.Copy(w, file)
 		if err != nil {
@@ -287,9 +216,8 @@ func Serve(ctx context.Context, artifactPath, addr, port string) context.CancelF
 	router := httprouter.New()
 
 	logger.Debugf("Artifacts base path '%s'", artifactPath)
-	fsys := readWriteFSImpl{}
-	uploads(router, artifactPath, fsys)
-	downloads(router, artifactPath, fsys)
+	uploads(router, artifactPath)
+	downloads(router, artifactPath)
 
 	server := &http.Server{
 		Addr:              fmt.Sprintf("%s:%s", addr, port),

act/artifacts/server_test.go

@@ -0,0 +1,189 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// Copyright 2021 The nektos/act Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package artifacts
+
+import (
+	"bytes"
+	"compress/gzip"
+	"encoding/json/v2"
+	"io"
+	"maps"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/julienschmidt/httprouter"
+	"github.com/stretchr/testify/require"
+)
+
+func TestArtifactFlow(t *testing.T) {
+	artifactPath := t.TempDir()
+
+	router := httprouter.New()
+	uploads(router, artifactPath)
+	downloads(router, artifactPath)
+	server := httptest.NewServer(router)
+	defer server.Close()
+
+	baseURL := server.URL
+	client := server.Client()
+	client.Timeout = 5 * time.Second
+
+	request := func(t *testing.T, method, rawURL string, body io.Reader, header http.Header) (int, []byte) {
+		t.Helper()
+		req, err := http.NewRequest(method, rawURL, body)
+		require.NoError(t, err)
+		maps.Copy(req.Header, header)
+		resp, err := client.Do(req)
+		require.NoError(t, err)
+		defer resp.Body.Close()
+		data, err := io.ReadAll(resp.Body)
+		require.NoError(t, err)
+		return resp.StatusCode, data
+	}
+
+	t.Run("upload-and-download", func(t *testing.T) {
+		const runID, item, content = "1", "my-artifact/data.txt", "hello artifact\n"
+
+		status, data := request(t, http.MethodPost, baseURL+"/_apis/pipelines/workflows/"+runID+"/artifacts", nil, nil)
+		require.Equal(t, http.StatusOK, status, string(data))
+		var prep FileContainerResourceURL
+		require.NoError(t, json.Unmarshal(data, &prep))
+		require.Equal(t, baseURL+"/upload/"+runID, prep.FileContainerResourceURL)
+
+		status, data = request(t, http.MethodPut, prep.FileContainerResourceURL+"?itemPath="+url.QueryEscape(item), strings.NewReader(content), nil)
+		require.Equal(t, http.StatusOK, status, string(data))
+		var msg ResponseMessage
+		require.NoError(t, json.Unmarshal(data, &msg))
+		require.Equal(t, "success", msg.Message)
+
+		status, data = request(t, http.MethodPatch, baseURL+"/_apis/pipelines/workflows/"+runID+"/artifacts", nil, nil)
+		require.Equal(t, http.StatusOK, status, string(data))
+		require.NoError(t, json.Unmarshal(data, &msg))
+		require.Equal(t, "success", msg.Message)
+
+		status, data = request(t, http.MethodGet, baseURL+"/_apis/pipelines/workflows/"+runID+"/artifacts", nil, nil)
+		require.Equal(t, http.StatusOK, status, string(data))
+		var list NamedFileContainerResourceURLResponse
+		require.NoError(t, json.Unmarshal(data, &list))
+		require.Equal(t, 1, list.Count)
+		require.Equal(t, "my-artifact", list.Value[0].Name)
+
+		status, data = request(t, http.MethodGet, list.Value[0].FileContainerResourceURL+"?itemPath=my-artifact", nil, nil)
+		require.Equal(t, http.StatusOK, status, string(data))
+		var items ContainerItemResponse
+		require.NoError(t, json.Unmarshal(data, &items))
+		require.Len(t, items.Value, 1)
+		require.Equal(t, "file", items.Value[0].ItemType)
+		require.Equal(t, "my-artifact/data.txt", items.Value[0].Path)
+
+		status, data = request(t, http.MethodGet, items.Value[0].ContentLocation, nil, nil)
+		require.Equal(t, http.StatusOK, status)
+		require.Equal(t, content, string(data))
+
+		stored, err := os.ReadFile(filepath.Join(artifactPath, runID, "my-artifact", "data.txt"))
+		require.NoError(t, err)
+		require.Equal(t, content, string(stored))
+	})
+
+	t.Run("content-range", func(t *testing.T) {
+		const rawURL = "/upload/4?itemPath=chunks.txt"
+		status, data := request(t, http.MethodPut, baseURL+rawURL, strings.NewReader("first"),
+			http.Header{"Content-Range": []string{"bytes 0-4/11"}})
+		require.Equal(t, http.StatusOK, status, string(data))
+
+		status, data = request(t, http.MethodPut, baseURL+rawURL, strings.NewReader("-second"),
+			http.Header{"Content-Range": []string{"bytes 5-11/11"}})
+		require.Equal(t, http.StatusOK, status, string(data))
+
+		stored, err := os.ReadFile(filepath.Join(artifactPath, "4", "chunks.txt"))
+		require.NoError(t, err)
+		require.Equal(t, "first-second", string(stored))
+	})
+
+	t.Run("gzip-roundtrip", func(t *testing.T) {
+		const runID, item, content = "2", "logs/app.log", "compressed payload\n"
+
+		var buf bytes.Buffer
+		gz := gzip.NewWriter(&buf)
+		_, err := gz.Write([]byte(content))
+		require.NoError(t, err)
+		require.NoError(t, gz.Close())
+
+		status, data := request(t, http.MethodPut, baseURL+"/upload/"+runID+"?itemPath="+url.QueryEscape(item),
+			&buf, http.Header{"Content-Encoding": []string{"gzip"}})
+		require.Equal(t, http.StatusOK, status, string(data))
+
+		// stored compressed, with the server's gzip marker suffix
+		_, err = os.Stat(filepath.Join(artifactPath, runID, "logs", "app.log.gz__"))
+		require.NoError(t, err)
+
+		status, data = request(t, http.MethodGet, baseURL+"/download/"+runID+"?itemPath=logs", nil, nil)
+		require.Equal(t, http.StatusOK, status, string(data))
+		var items ContainerItemResponse
+		require.NoError(t, json.Unmarshal(data, &items))
+		require.Len(t, items.Value, 1)
+		require.Equal(t, "logs/app.log", items.Value[0].Path)
+
+		status, data = request(t, http.MethodGet, items.Value[0].ContentLocation, nil, nil)
+		require.Equal(t, http.StatusOK, status)
+		require.Equal(t, content, string(data))
+	})
+
+	// GHSL-2023-004: an itemPath that climbs out of the run directory must be neutralised so the
+	// blob cannot be written outside the artifact root.
+	t.Run("GHSL-2023-004", func(t *testing.T) {
+		const runID, content = "3", "contained\n"
+
+		status, data := request(t, http.MethodPut, baseURL+"/upload/"+runID+"?itemPath="+url.QueryEscape("../../escape.txt"),
+			strings.NewReader(content), nil)
+		require.Equal(t, http.StatusOK, status, string(data))
+
+		stored, err := os.ReadFile(filepath.Join(artifactPath, runID, "escape.txt"))
+		require.NoError(t, err)
+		require.Equal(t, content, string(stored))
+
+		_, err = os.Stat(filepath.Join(filepath.Dir(artifactPath), "escape.txt"))
+		require.True(t, os.IsNotExist(err), "upload escaped the artifact root")
+
+		status, data = request(t, http.MethodGet, baseURL+"/artifact/"+runID+"/escape.txt", nil, nil)
+		require.Equal(t, http.StatusOK, status)
+		require.Equal(t, content, string(data))
+	})
+}
+
+func TestSafeResolve(t *testing.T) {
+	baseDir := "/foo/bar"
+
+	tests := map[string]struct {
+		input string
+		want  string
+	}{
+		"simple":         {input: "baz", want: "/foo/bar/baz"},
+		"nested":         {input: "baz/blue", want: "/foo/bar/baz/blue"},
+		"dots in middle": {input: "baz/../../blue", want: "/foo/bar/blue"},
+		"leading dots":   {input: "../../parent", want: "/foo/bar/parent"},
+		"root path":      {input: "/root", want: "/foo/bar/root"},
+		"root":           {input: "/", want: "/foo/bar"},
+		"empty":          {input: "", want: "/foo/bar"},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			require.Equal(t, tc.want, safeResolve(baseDir, tc.input))
+		})
+	}
+}
+
+func TestServeEmptyArtifactPathReturnsCancelableNoop(t *testing.T) {
+	cancel := Serve(t.Context(), "", "127.0.0.1", "0")
+	require.NotNil(t, cancel)
+	cancel()
+}

act/common/executor.go

@@ -54,22 +54,6 @@ func NewPipelineExecutor(executors ...Executor) Executor {
 	return rtn
 }
 
-// NewConditionalExecutor creates a new executor based on conditions
-func NewConditionalExecutor(conditional Conditional, trueExecutor, falseExecutor Executor) Executor {
-	return func(ctx context.Context) error {
-		if conditional(ctx) {
-			if trueExecutor != nil {
-				return trueExecutor(ctx)
-			}
-		} else {
-			if falseExecutor != nil {
-				return falseExecutor(ctx)
-			}
-		}
-		return nil
-	}
-}
-
 // NewErrorExecutor creates a new executor that always errors out
 func NewErrorExecutor(err error) Executor {
 	return func(ctx context.Context) error {
@@ -187,15 +171,8 @@ func (e Executor) Finally(finally Executor) Executor {
 		err := e(ctx)
 		err2 := finally(ctx)
 		if err2 != nil {
-			return fmt.Errorf("Error occurred running finally: %v (original error: %v)", err2, err)
+			return fmt.Errorf("error occurred running finally: %v (original error: %v)", err2, err)
 		}
 		return err
 	}
 }
-
-// Not return an inverted conditional
-func (c Conditional) Not() Conditional {
-	return func(ctx context.Context) bool {
-		return !c(ctx)
-	}
-}

act/common/executor_test.go

@@ -45,43 +45,6 @@ func TestNewWorkflow(t *testing.T) {
 	assert.Equal(2, runcount)
 }
 
-func TestNewConditionalExecutor(t *testing.T) {
-	assert := assert.New(t)
-
-	ctx := context.Background()
-
-	trueCount := 0
-	falseCount := 0
-
-	err := NewConditionalExecutor(func(ctx context.Context) bool {
-		return false
-	}, func(ctx context.Context) error {
-		trueCount++
-		return nil
-	}, func(ctx context.Context) error {
-		falseCount++
-		return nil
-	})(ctx)
-
-	assert.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act
-	assert.Equal(0, trueCount)
-	assert.Equal(1, falseCount)
-
-	err = NewConditionalExecutor(func(ctx context.Context) bool {
-		return true
-	}, func(ctx context.Context) error {
-		trueCount++
-		return nil
-	}, func(ctx context.Context) error {
-		falseCount++
-		return nil
-	})(ctx)
-
-	assert.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act
-	assert.Equal(1, trueCount)
-	assert.Equal(1, falseCount)
-}
-
 // concurrencyProbe returns an executor recording the peak number of concurrent copies. Copies
 // block until wantActive are in flight so the peak is exact without sleeping, and later copies
 // find the gate already open so the last one still finishes with no partner left.
@@ -223,10 +186,3 @@ func TestExecutorFinallyReturnsFinallyErrorWithOriginal(t *testing.T) {
 		t.Fatalf("finally error = %q, want both cleanup and original error", err)
 	}
 }
-
-func TestConditionalNot(t *testing.T) {
-	cond := Conditional(func(context.Context) bool { return false })
-	if !cond.Not()(context.Background()) {
-		t.Fatal("inverted conditional should be true")
-	}
-}

act/common/git/git.go

@@ -15,7 +15,7 @@ import (
 	"strings"
 	"sync"
 
-	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/act/common"
 	"gitea.com/gitea/runner/internal/pkg/lock"
 
 	"github.com/go-git/go-git/v5"
@@ -36,7 +36,6 @@ var (
 	cloneLocks lock.Keyed[string] // key: clone target directory
 
 	ErrShortRef = errors.New("short SHA references are not supported")
-	ErrNoRepo   = errors.New("unable to find git repo")
 )
 
 // AcquireCloneLock returns an unlock function after locking the per-directory mutex for dir.
@@ -187,19 +186,16 @@ func FindGitRef(ctx context.Context, file string) (string, error) {
 }
 
 // FindGithubRepo get the repo
-func FindGithubRepo(ctx context.Context, file, githubInstance, remoteName string) (string, error) {
+func FindGithubRepo(ctx context.Context, file, githubInstance string) (string, error) {
 	goGitMu.Lock()
 	defer goGitMu.Unlock()
-	if remoteName == "" {
-		remoteName = "origin"
-	}
 
-	url, err := findGitRemoteURL(ctx, file, remoteName)
+	url, err := findGitRemoteURL(ctx, file, "origin")
 	if err != nil {
 		return "", err
 	}
-	_, slug, err := findGitSlug(url, githubInstance)
-	return slug, err
+	_, slug := findGitSlug(url, githubInstance)
+	return slug, nil
 }
 
 func findGitRemoteURL(_ context.Context, file, remoteName string) (string, error) {
@@ -226,25 +222,25 @@ func findGitRemoteURL(_ context.Context, file, remoteName string) (string, error
 	return remote.Config().URLs[0], nil
 }
 
-func findGitSlug(url, githubInstance string) (string, string, error) { //nolint:unparam // pre-existing issue from nektos/act
+func findGitSlug(url, githubInstance string) (string, string) {
 	if matches := codeCommitHTTPRegex.FindStringSubmatch(url); matches != nil {
-		return "CodeCommit", matches[2], nil
+		return "CodeCommit", matches[2]
 	} else if matches := codeCommitSSHRegex.FindStringSubmatch(url); matches != nil {
-		return "CodeCommit", matches[2], nil
+		return "CodeCommit", matches[2]
 	} else if matches := githubHTTPRegex.FindStringSubmatch(url); matches != nil {
-		return "GitHub", fmt.Sprintf("%s/%s", matches[1], matches[2]), nil
+		return "GitHub", fmt.Sprintf("%s/%s", matches[1], matches[2])
 	} else if matches := githubSSHRegex.FindStringSubmatch(url); matches != nil {
-		return "GitHub", fmt.Sprintf("%s/%s", matches[1], matches[2]), nil
+		return "GitHub", fmt.Sprintf("%s/%s", matches[1], matches[2])
 	} else if githubInstance != "github.com" {
 		gheHTTPRegex := regexp.MustCompile(fmt.Sprintf(`^https?://%s/(.+)/(.+?)(?:.git)?$`, githubInstance))
 		gheSSHRegex := regexp.MustCompile(githubInstance + "[:/](.+)/(.+?)(?:.git)?$")
 		if matches := gheHTTPRegex.FindStringSubmatch(url); matches != nil {
-			return "GitHubEnterprise", fmt.Sprintf("%s/%s", matches[1], matches[2]), nil
+			return "GitHubEnterprise", fmt.Sprintf("%s/%s", matches[1], matches[2])
 		} else if matches := gheSSHRegex.FindStringSubmatch(url); matches != nil {
-			return "GitHubEnterprise", fmt.Sprintf("%s/%s", matches[1], matches[2]), nil
+			return "GitHubEnterprise", fmt.Sprintf("%s/%s", matches[1], matches[2])
 		}
 	}
-	return "", url, nil
+	return "", url
 }
 
 // NewGitCloneExecutorInput the input for the NewGitCloneExecutor
@@ -278,11 +274,12 @@ func CloneIfRequired(ctx context.Context, refName plumbing.ReferenceName, input
 			return r, true, nil
 		}
 
-		if err != nil {
+		switch {
+		case err != nil:
 			logger.Debugf("Removing cached clone at %s because origin cannot be read: %v", input.Dir, err)
-		} else if len(remote.Config().URLs) == 0 {
+		case len(remote.Config().URLs) == 0:
 			logger.Debugf("Removing cached clone at %s because origin has no URL", input.Dir)
-		} else {
+		default:
 			logger.Debugf("Removing cached clone at %s because origin URL changed from %s to %s", input.Dir, remote.Config().URLs[0], input.URL)
 		}
 		if err := os.RemoveAll(input.Dir); err != nil {
@@ -345,6 +342,16 @@ func gitOptions(token string) (fetchOptions git.FetchOptions, pullOptions git.Pu
 	return fetchOptions, pullOptions
 }
 
+// staleRefreshErr reports why a failed refresh must abort: the resolve and
+// checkout that follow are local and succeed on a cancelled context, which
+// would hand back the cached revision as if it were fresh.
+func staleRefreshErr(ctx context.Context, err error) error {
+	if err == nil || errors.Is(err, git.NoErrAlreadyUpToDate) {
+		return nil
+	}
+	return ctx.Err()
+}
+
 // NewGitCloneExecutor creates an executor to clone git repos
 func NewGitCloneExecutor(input NewGitCloneExecutorInput) common.Executor {
 	return func(ctx context.Context) error {
@@ -385,7 +392,7 @@ func NewGitCloneExecutor(input NewGitCloneExecutorInput) common.Executor {
 		}
 
 		if !isOfflineMode {
-			err = r.Fetch(&fetchOptions)
+			err = r.FetchContext(ctx, &fetchOptions)
 			if err != nil && !errors.Is(err, git.NoErrAlreadyUpToDate) {
 				return err
 			}
@@ -454,18 +461,17 @@ func NewGitCloneExecutor(input NewGitCloneExecutorInput) common.Executor {
 		switch {
 		case !isOfflineMode && !shallow:
 			// In shallow mode the depth-limited fetch above already advanced the ref.
-			if err = w.Pull(&pullOptions); err != nil && err != git.NoErrAlreadyUpToDate {
+			if err = w.PullContext(ctx, &pullOptions); err != nil && !errors.Is(err, git.NoErrAlreadyUpToDate) {
 				logger.Debugf("Unable to pull %s: %v", refName, err)
 			}
+			if err := staleRefreshErr(ctx, err); err != nil {
+				return err
+			}
 		case isOfflineMode && reused:
-			reusedMsg = " (offline mode)"
+			reusedMsg = " (reused in offline mode)"
 		}
 
-		if reused {
-			logger.Debugf("Reused %s at %s%s", input.URL, input.Dir, reusedMsg)
-		} else {
-			logger.Debugf("Cloned %s to %s", input.URL, input.Dir)
-		}
+		logger.Debugf("Cloned %s to %s%s", input.URL, input.Dir, reusedMsg)
 
 		if hash.String() != input.Ref && refType == "branch" {
 			logger.Debugf("Provided ref is not a sha. Updating branch ref after pull")

act/common/git/git_test.go

@@ -6,18 +6,24 @@ package git
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"net/http"
+	"net/http/httptest"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"strconv"
 	"strings"
+	"sync"
 	"syscall"
 	"testing"
 	"time"
 
-	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/act/common"
 
+	gogit "github.com/go-git/go-git/v5"
+	gogitconfig "github.com/go-git/go-git/v5/config"
 	log "github.com/sirupsen/logrus"
 	logrustest "github.com/sirupsen/logrus/hooks/test"
 	"github.com/stretchr/testify/assert"
@@ -45,9 +51,7 @@ func TestFindGitSlug(t *testing.T) {
 	}
 
 	for _, tt := range slugTests {
-		provider, slug, err := findGitSlug(tt.url, "github.com")
-
-		assert.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act
+		provider, slug := findGitSlug(tt.url, "github.com")
 		assert.Equal(tt.provider, provider)
 		assert.Equal(tt.slug, slug)
 	}
@@ -81,45 +85,20 @@ func cleanGitHooks(dir string) error {
 	return nil
 }
 
-func TestFindGitRemoteURL(t *testing.T) {
-	assert := assert.New(t)
-
-	basedir := t.TempDir()
-	err := gitCmd("init", basedir)
-	assert.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act
-	err = cleanGitHooks(basedir)
-	assert.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act
-
-	remoteURL := "https://git-codecommit.us-east-1.amazonaws.com/v1/repos/my-repo-name"
-	err = gitCmd("-C", basedir, "remote", "add", "origin", remoteURL)
-	assert.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act
-
-	u, err := findGitRemoteURL(context.Background(), basedir, "origin")
-	assert.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act
-	assert.Equal(remoteURL, u)
-
-	remoteURL = "git@github.com/AwesomeOwner/MyAwesomeRepo.git"
-	err = gitCmd("-C", basedir, "remote", "add", "upstream", remoteURL)
-	assert.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act
-	u, err = findGitRemoteURL(context.Background(), basedir, "upstream")
-	assert.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act
-	assert.Equal(remoteURL, u)
-}
-
-func TestFindGithubRepoUsesOriginAndCustomRemote(t *testing.T) {
+func TestFindGithubRepoUsesOrigin(t *testing.T) {
 	basedir := t.TempDir()
+	const remoteURL = "https://github.com/owner/repo.git"
 	require.NoError(t, gitCmd("init", basedir))
 	require.NoError(t, cleanGitHooks(basedir))
-	require.NoError(t, gitCmd("-C", basedir, "remote", "add", "origin", "https://github.com/owner/repo.git"))
-	require.NoError(t, gitCmd("-C", basedir, "remote", "add", "ghe", "git@git.example.com:team/project.git"))
+	require.NoError(t, gitCmd("-C", basedir, "remote", "add", "origin", remoteURL))
 
-	slug, err := FindGithubRepo(context.Background(), basedir, "github.com", "")
+	url, err := findGitRemoteURL(context.Background(), basedir, "origin")
+	require.NoError(t, err)
+	require.Equal(t, remoteURL, url)
+
+	slug, err := FindGithubRepo(context.Background(), basedir, "github.com")
 	require.NoError(t, err)
 	require.Equal(t, "owner/repo", slug)
-
-	slug, err = FindGithubRepo(context.Background(), basedir, "git.example.com", "ghe")
-	require.NoError(t, err)
-	require.Equal(t, "team/project", slug)
 }
 
 func TestGitFindRef(t *testing.T) {
@@ -373,28 +352,22 @@ func TestGitCloneExecutorOfflineMode(t *testing.T) {
 
 	// Prime the cache with an online clone of main.
 	cacheDir := t.TempDir()
-	logger, hook := logrustest.NewNullLogger()
-	logger.SetLevel(log.DebugLevel)
-	ctx := common.WithLogger(context.Background(), logger.WithField("job", "j1"))
 	require.NoError(t, NewGitCloneExecutor(NewGitCloneExecutorInput{
 		URL: remoteDir,
 		Ref: "main",
 		Dir: cacheDir,
-	})(ctx))
-	assert.Contains(t, logMessages(hook), "Cloned "+remoteDir+" to "+cacheDir)
+	})(context.Background()))
 
 	t.Run("cached branch resolves without fetching", func(t *testing.T) {
 		// Offline reuse of a cached branch must succeed even though ResolveRevision(input.Ref)
 		// finds no local refs/heads/<ref>.
-		hook.Reset()
 		err := NewGitCloneExecutor(NewGitCloneExecutorInput{
 			URL:         remoteDir,
 			Ref:         "main",
 			Dir:         cacheDir,
 			OfflineMode: true,
-		})(ctx)
+		})(context.Background())
 		require.NoError(t, err)
-		assert.Contains(t, logMessages(hook), "Reused "+remoteDir+" at "+cacheDir+" (offline mode)")
 
 		out, err := exec.Command("git", "-C", cacheDir, "log", "--oneline", "-1", "--format=%s").Output()
 		require.NoError(t, err)
@@ -451,14 +424,6 @@ func TestGitCloneExecutorQuietDemotesCloneLine(t *testing.T) {
 	}
 }
 
-func logMessages(hook *logrustest.Hook) []string {
-	messages := []string{}
-	for _, entry := range hook.AllEntries() {
-		messages = append(messages, entry.Message)
-	}
-	return messages
-}
-
 func TestGitCloneExecutorShallow(t *testing.T) {
 	// Build a local "remote" with several commits on main plus a tag, so a full clone would pull noticeably more history than a shallow one.
 	remoteDir := t.TempDir()
@@ -624,3 +589,55 @@ func TestAcquireCloneLock(t *testing.T) {
 		}
 	})
 }
+
+// An unresponsive remote must not pin a job: the refresh has to be interruptible.
+func TestNewGitCloneExecutorFetchHonoursContext(t *testing.T) {
+	block := make(chan struct{})
+	reached := make(chan struct{})
+	var once sync.Once
+	server := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
+		once.Do(func() { close(reached) })
+		<-block
+	}))
+	t.Cleanup(func() {
+		close(block)
+		server.Close()
+	})
+
+	dir := filepath.Join(t.TempDir(), "cached-action")
+	repo, err := gogit.PlainInit(dir, false)
+	require.NoError(t, err)
+	_, err = repo.CreateRemote(&gogitconfig.RemoteConfig{Name: "origin", URLs: []string{server.URL}})
+	require.NoError(t, err)
+
+	ctx, cancel := context.WithCancel(t.Context())
+	defer cancel()
+
+	done := make(chan error, 1)
+	go func() {
+		done <- NewGitCloneExecutor(NewGitCloneExecutorInput{URL: server.URL, Ref: "main", Dir: dir})(ctx)
+	}()
+
+	select {
+	case <-reached:
+	case <-time.After(10 * time.Second):
+		t.Fatal("the executor never reached the remote")
+	}
+	cancel()
+
+	select {
+	case err := <-done:
+		require.Error(t, err)
+	case <-time.After(10 * time.Second):
+		t.Fatal("fetch ignored context cancellation")
+	}
+}
+
+func TestStaleRefreshErr(t *testing.T) {
+	ctx, cancel := context.WithCancel(t.Context())
+	require.NoError(t, staleRefreshErr(ctx, errors.New("remote hung up")))
+
+	cancel()
+	require.ErrorIs(t, staleRefreshErr(ctx, errors.New("remote hung up")), context.Canceled)
+	require.NoError(t, staleRefreshErr(ctx, gogit.NoErrAlreadyUpToDate))
+}

act/common/line_writer.go

@@ -46,14 +46,14 @@ func (lw *lineWriter) Write(p []byte) (n int, err error) {
 		line, err := pBuf.ReadString('\n')
 		w, _ := lw.buffer.WriteString(line)
 		written += w
-		if err == nil {
-			lw.handleLine(lw.buffer.String())
-			lw.buffer.Reset()
-		} else if err == io.EOF {
-			break
-		} else {
+		if err != nil {
+			if err == io.EOF {
+				break
+			}
 			return written, err
 		}
+		lw.handleLine(lw.buffer.String())
+		lw.buffer.Reset()
 	}
 
 	return written, nil

act/container/container_types.go

@@ -10,7 +10,7 @@ import (
 	"fmt"
 	"io"
 
-	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/act/common"
 
 	"github.com/docker/go-connections/nat"
 	"github.com/moby/moby/api/types/container"
@@ -25,26 +25,27 @@ func (e ExitCodeError) Error() string {
 
 // NewContainerInput the input for the New function
 type NewContainerInput struct {
-	Image          string
-	Username       string
-	Password       string
-	Entrypoint     []string
-	Cmd            []string
-	WorkingDir     string
-	Env            []string
-	Binds          []string
-	Mounts         map[string]string
-	Name           string
-	Stdout         io.Writer
-	Stderr         io.Writer
-	NetworkMode    string
-	Privileged     bool
-	UsernsMode     string
-	Platform       string
-	Options        string
-	NetworkAliases []string
-	ExposedPorts   nat.PortSet
-	PortBindings   nat.PortMap
+	Image           string
+	Username        string
+	Password        string
+	Entrypoint      []string
+	Cmd             []string
+	WorkingDir      string
+	Env             []string
+	Binds           []string
+	Mounts          map[string]string
+	Name            string
+	Stdout          io.Writer
+	Stderr          io.Writer
+	NetworkMode     string
+	Privileged      bool
+	UsernsMode      string
+	Platform        string
+	RunnerOptions   string // container options the runner was configured with, trusted
+	WorkflowOptions string // container options the workflow asked for, untrusted
+	NetworkAliases  []string
+	ExposedPorts    nat.PortSet
+	PortBindings    nat.PortMap
 
 	// Gitea specific
 	AutoRemove   bool
@@ -88,9 +89,7 @@ type Info struct {
 // Container for managing docker run containers
 type Container interface {
 	Create(capAdd, capDrop []string) common.Executor
-	ConnectToNetwork(name string) common.Executor
 	Copy(destPath string, files ...*FileEntry) common.Executor
-	CopyTarStream(ctx context.Context, destPath string, tarStream io.Reader) error
 	CopyDir(destPath, srcPath string, useGitIgnore bool) common.Executor
 	GetContainerArchive(ctx context.Context, srcPath string) (io.ReadCloser, error)
 	Inspect(ctx context.Context) (*Info, error)

act/container/docker_auth.go

@@ -9,7 +9,7 @@ package container
 import (
 	"context"
 
-	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/act/common"
 
 	"github.com/distribution/reference"
 	"github.com/docker/cli/cli/config"

act/container/docker_build.go

@@ -12,7 +12,7 @@ import (
 	"os"
 	"path/filepath"
 
-	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/act/common"
 
 	"github.com/moby/go-archive"
 	"github.com/moby/go-archive/compression"

act/container/docker_cli.go

@@ -17,8 +17,7 @@
 package container
 
 import (
-	"bytes"
-	"encoding/json"
+	"encoding/json/jsontext"
 	"errors"
 	"fmt"
 	"net"
@@ -351,7 +350,7 @@ type containerConfig struct {
 // parse parses the args for the specified command and generates a Config,
 // a HostConfig and returns them with the specified command.
 // If the specified args are not valid, it will return an error.
-func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*containerConfig, error) { //nolint:gocyclo // verbatim copy from docker/cli
+func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*containerConfig, error) { //nolint:gocyclo,unparam // verbatim copy from docker/cli
 	var (
 		attachStdin  = copts.attach.Get("stdin")
 		attachStdout = copts.attach.Get("stdout")
@@ -959,11 +958,11 @@ func parseSecurityOpts(securityOpts []string) ([]string, error) {
 				if err != nil {
 					return securityOpts, fmt.Errorf("opening seccomp profile (%s) failed: %w", v, err)
 				}
-				var b bytes.Buffer
-				if err := json.Compact(&b, f); err != nil {
+				profile := jsontext.Value(f)
+				if err := profile.Compact(); err != nil {
 					return securityOpts, fmt.Errorf("compacting json for seccomp profile (%s) failed: %w", v, err)
 				}
-				securityOpts[key] = "seccomp=" + b.String()
+				securityOpts[key] = "seccomp=" + string(profile)
 			}
 		}
 	}

act/container/docker_create_flags.go

@@ -10,7 +10,9 @@ import (
 	"fmt"
 	"io"
 	"slices"
+	"strings"
 
+	"github.com/docker/cli/opts"
 	"github.com/kballard/go-shellquote"
 	"github.com/spf13/pflag"
 )
@@ -51,15 +53,16 @@ func parseContainerOptions(options string) (*pflag.FlagSet, *containerOptions, *
 	flags := pflag.NewFlagSet("container_flags", pflag.ContinueOnError)
 	flags.SetOutput(io.Discard)
 	copts := addFlags(flags)
+	copts.env = opts.NewListOpts(validateEnv) // addFlags registered this field's address, so the swap takes effect
 	cf := registerCreateFlags(flags)
 
 	args, err := shellquote.Split(options)
 	if err != nil {
-		return flags, copts, cf, fmt.Errorf("Cannot split container options: '%s': '%w'", options, err)
+		return flags, copts, cf, fmt.Errorf("cannot split container options: '%s': '%w'", options, err)
 	}
 
 	if err := flags.Parse(args); err != nil {
-		return flags, copts, cf, fmt.Errorf("Cannot parse container options: '%s': '%w'", options, err)
+		return flags, copts, cf, fmt.Errorf("cannot parse container options: '%s': '%w'", options, err)
 	}
 
 	return flags, copts, cf, nil
@@ -73,6 +76,30 @@ func createFlagsFromOptions(options string) *createFlags {
 	return cf
 }
 
+// validateEnv is opts.ValidateEnv without its lookup of a bare name in the runner's environment.
+func validateEnv(val string) (string, error) {
+	if name, _, _ := strings.Cut(val, "="); name == "" {
+		return "", errors.New("invalid environment variable: " + val)
+	}
+	return val, nil
+}
+
+// rejectHostReadingOptions refuses the flags naming files that are read here, on the
+// runner, rather than in the container.
+func rejectHostReadingOptions(options string) error {
+	flags, _, _, err := parseContainerOptions(options)
+	if err != nil {
+		return err
+	}
+
+	for _, name := range []string{"env-file", "label-file"} {
+		if flags.Changed(name) {
+			return fmt.Errorf("container option --%s reads files from the runner and is not allowed in a workflow", name)
+		}
+	}
+	return nil
+}
+
 func (cf *createFlags) validate() error {
 	if !slices.Contains(pullPolicies, cf.pull) {
 		return fmt.Errorf("invalid --pull option %q: must be one of %q", cf.pull, pullPolicies)

act/container/docker_create_flags_test.go

@@ -50,13 +50,13 @@ func TestCreateFlagsValidate(t *testing.T) {
 }
 
 func TestNewContainerAppliesCreateFlags(t *testing.T) {
-	input := &NewContainerInput{Platform: "linux/amd64", Options: "--platform linux/arm64 --pull never"}
+	input := &NewContainerInput{Platform: "linux/amd64", RunnerOptions: "--pull never", WorkflowOptions: "--platform linux/arm64"}
 	cr, ok := NewContainer(input).(*containerReference)
 	require.True(t, ok)
 	assert.Equal(t, "linux/arm64", input.Platform)
 	assert.Equal(t, pullPolicyNever, cr.pullPolicy)
 
-	kept := &NewContainerInput{Platform: "linux/amd64", Options: "--privileged"}
+	kept := &NewContainerInput{Platform: "linux/amd64", RunnerOptions: "--privileged"}
 	NewContainer(kept)
 	assert.Equal(t, "linux/amd64", kept.Platform)
 }

act/container/docker_logger.go

@@ -8,7 +8,7 @@ package container
 
 import (
 	"bufio"
-	"encoding/json"
+	"encoding/json/v2"
 	"errors"
 	"io"
 
@@ -20,8 +20,8 @@ type dockerMessage struct {
 	Stream      string `json:"stream"`
 	Error       string `json:"error"`
 	ErrorDetail struct {
-		Message string
-	}
+		Message string `json:"message"`
+	} `json:"errorDetail"`
 	Status   string `json:"status"`
 	Progress string `json:"progress"`
 }
@@ -60,15 +60,16 @@ func logDockerResponse(logger logrus.FieldLogger, dockerResponse io.ReadCloser,
 			return errors.New(msg.ErrorDetail.Message)
 		}
 
-		if msg.Status != "" {
+		switch {
+		case msg.Status != "":
 			if msg.Progress != "" {
 				writeLog(logger, isError, "%s :: %s :: %s\n", msg.Status, msg.ID, msg.Progress)
 			} else {
 				writeLog(logger, isError, "%s :: %s\n", msg.Status, msg.ID)
 			}
-		} else if msg.Stream != "" {
+		case msg.Stream != "":
 			writeLog(logger, isError, "%s", msg.Stream)
-		} else {
+		default:
 			writeLog(logger, false, "Unable to handle line: %s", string(line))
 		}
 	}

act/container/docker_network.go

@@ -13,7 +13,7 @@ import (
 	"strings"
 	"time"
 
-	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/act/common"
 
 	"github.com/moby/moby/client"
 )

act/container/docker_network_test.go

@@ -32,9 +32,9 @@ func TestRemoveOrphanNetworks(t *testing.T) {
 	client.On("NetworkList", ctx, mobyclient.NetworkListOptions{
 		Filters: make(mobyclient.Filters).Add("label", runnerUUIDLabel+"=runner-1"),
 	}).Return(mobyclient.NetworkListResult{Items: []network.Summary{
-		{Network: network.Network{ID: "orphan"}},
-		{Network: network.Network{ID: "busy"}},
-		{Network: network.Network{ID: "starting"}},
+		{ID: "orphan"},
+		{ID: "busy"},
+		{ID: "starting"},
 	}}, nil)
 	client.On("NetworkInspect", ctx, "orphan", mobyclient.NetworkInspectOptions{}).
 		Return(mobyclient.NetworkInspectResult{}, nil)

act/container/docker_pull.go

@@ -11,7 +11,7 @@ import (
 	"fmt"
 	"strings"
 
-	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/act/common"
 
 	"github.com/distribution/reference"
 	"github.com/moby/moby/api/pkg/authconfig"

act/container/docker_run.go

@@ -15,6 +15,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"reflect"
 	"regexp"
 	"runtime"
 	"slices"
@@ -22,8 +23,8 @@ import (
 	"strings"
 	"time"
 
-	"gitea.com/gitea/runner/internal/act/common"
-	"gitea.com/gitea/runner/internal/act/filecollector"
+	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/act/filecollector"
 
 	"dario.cat/mergo"
 	cerrdefs "github.com/containerd/errdefs"
@@ -57,7 +58,7 @@ func NewContainer(input *NewContainerInput) ExecutionsEnvironment {
 	cr := new(containerReference)
 	cr.input = input
 	// Resolved up front because the image pull runs before the container is created.
-	cf := createFlagsFromOptions(input.Options)
+	cf := createFlagsFromOptions(input.allOptions())
 	if cf.platform != "" {
 		cr.input.Platform = cf.platform
 	}
@@ -65,37 +66,14 @@ func NewContainer(input *NewContainerInput) ExecutionsEnvironment {
 	return cr
 }
 
-func (cr *containerReference) ConnectToNetwork(name string) common.Executor {
-	return common.
-		NewDebugExecutor("docker network connect %s %s", name, cr.input.Name).
-		Then(
-			common.NewPipelineExecutor(
-				cr.connect(),
-				cr.connectToNetwork(name, cr.input.NetworkAliases),
-			).IfNot(common.Dryrun),
-		)
-}
-
-func (cr *containerReference) connectToNetwork(name string, aliases []string) common.Executor {
-	return func(ctx context.Context) error {
-		_, err := cr.cli.NetworkConnect(ctx, name, client.NetworkConnectOptions{
-			Container: cr.input.Name,
-			EndpointConfig: &network.EndpointSettings{
-				Aliases: aliases,
-			},
-		})
-		return err
-	}
-}
-
-// supportsContainerImagePlatform returns true if the underlying Docker server
-// API version is 1.41 and beyond
-func supportsContainerImagePlatform(ctx context.Context, cli client.APIClient) bool {
+// supportsContainerImagePlatform reports whether the Docker server API version
+// is 1.41 and beyond
+func supportsContainerImagePlatform(ctx context.Context, cli client.APIClient) (bool, error) {
 	ver, err := cli.ServerVersion(ctx, client.ServerVersionOptions{})
 	if err != nil {
-		common.Logger(ctx).Panicf("Failed to get Docker API Version: %s", err)
+		return false, fmt.Errorf("get docker API version: %w", err)
 	}
-	return versions.GreaterThanOrEqualTo(ver.APIVersion, "1.41")
+	return versions.GreaterThanOrEqualTo(ver.APIVersion, "1.41"), nil
 }
 
 func (cr *containerReference) Create(capAdd, capDrop []string) common.Executor {
@@ -547,29 +525,39 @@ func (cr *containerReference) waitForRemoval(ctx context.Context, idOrName strin
 	}
 }
 
+// allOptions puts the runner's options first, so a flag both sources set ends up the workflow's.
+func (input *NewContainerInput) allOptions() string {
+	return strings.TrimSpace(input.RunnerOptions + " " + input.WorkflowOptions)
+}
+
 func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config *container.Config, hostConfig *container.HostConfig) (*container.Config, *container.HostConfig, error) {
 	logger := common.Logger(ctx)
-	input := cr.input
+	options := cr.input.allOptions()
 
-	if input.Options == "" {
+	if options == "" {
 		return config, hostConfig, nil
 	}
 
+	// For Gitea, checked here because the parse below is what would read those files
+	if err := rejectHostReadingOptions(cr.input.WorkflowOptions); err != nil {
+		return nil, nil, err
+	}
+
 	// parse configuration from CLI container.options
-	flags, copts, cf, err := parseContainerOptions(input.Options)
+	flags, copts, cf, err := parseContainerOptions(options)
 	if err != nil {
 		return nil, nil, err
 	}
 
 	if err := cf.validate(); err != nil {
-		return nil, nil, fmt.Errorf("Cannot process container options: '%s': '%w'", input.Options, err)
+		return nil, nil, fmt.Errorf("cannot process container options: '%s': '%w'", options, err)
 	}
 
 	// FIXME: If everything is fine after gitea/act v0.260.0, remove the following comment.
 	// In the old fork version, the code is
 	// if len(copts.netMode.Value()) == 0 {
 	// 	if err = copts.netMode.Set("host"); err != nil {
-	// 		return nil, nil, fmt.Errorf("Cannot parse networkmode=host. This is an internal error and should not happen: '%w'", err)
+	// 		return nil, nil, fmt.Errorf("cannot parse networkmode=host. This is an internal error and should not happen: '%w'", err)
 	// 	}
 	// }
 	// And it has been commented with:
@@ -581,7 +569,7 @@ func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config
 
 	if len(copts.netMode.Value()) == 0 {
 		if err = copts.netMode.Set(cr.input.NetworkMode); err != nil {
-			return nil, nil, fmt.Errorf("Cannot parse networkmode=%s. This is an internal error and should not happen: '%w'", cr.input.NetworkMode, err)
+			return nil, nil, fmt.Errorf("cannot parse networkmode=%s. This is an internal error and should not happen: '%w'", cr.input.NetworkMode, err)
 		}
 	}
 
@@ -593,24 +581,23 @@ func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config
 
 	containerConfig, err := parse(flags, copts, runtime.GOOS)
 	if err != nil {
-		return nil, nil, fmt.Errorf("Cannot process container options: '%s': '%w'", input.Options, err)
+		return nil, nil, fmt.Errorf("cannot process container options: '%s': '%w'", options, err)
 	}
 
-	// For Gitea
-	// When privileged mode is disabled, container.options is workflow-controlled
-	// untrusted input. Strip the HostConfig fields that would let a workflow break
-	// out of the container (host namespaces, capability expansion, security profile
-	// overrides, device and runtime access). Otherwise these survive into the final
-	// HostConfig even though --privileged is forced off.
+	// For Gitea, forcing --privileged off is not enough, other options reach the host too
 	if !hostConfig.Privileged {
-		sanitizeOptionsHostConfig(logger, containerConfig.HostConfig)
+		trusted, err := parseOptionsHostConfig(cr.input.RunnerOptions)
+		if err != nil {
+			return nil, nil, err
+		}
+		sanitizeOptionsHostConfig(logger, containerConfig.HostConfig, trusted)
 	}
 
 	logger.Debugf("Custom container.Config from options ==> %+v", containerConfig.Config)
 
 	err = mergo.Merge(config, containerConfig.Config, mergo.WithOverride, mergo.WithAppendSlice)
 	if err != nil {
-		return nil, nil, fmt.Errorf("Cannot merge container.Config options: '%s': '%w'", input.Options, err)
+		return nil, nil, fmt.Errorf("cannot merge container.Config options: '%s': '%w'", options, err)
 	}
 	logger.Debugf("Merged container.Config ==> %+v", config)
 
@@ -622,14 +609,15 @@ func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config
 	networkMode := hostConfig.NetworkMode
 	err = mergo.Merge(hostConfig, containerConfig.HostConfig, mergo.WithOverride)
 	if err != nil {
-		return nil, nil, fmt.Errorf("Cannot merge container.HostConfig options: '%s': '%w'", input.Options, err)
+		return nil, nil, fmt.Errorf("cannot merge container.HostConfig options: '%s': '%w'", options, err)
 	}
 	hostConfig.Binds = binds
 	hostConfig.Mounts = mounts
 	if cf.name != "" {
 		logger.Warn("--name in the options will be ignored.")
 	}
-	if len(copts.netMode.Value()) > 0 {
+	// the runner's own network mode was put into copts above, so ask the flags instead
+	if flags.Changed("network") || flags.Changed("net") {
 		logger.Warn("--network and --net in the options will be ignored.")
 	}
 	hostConfig.NetworkMode = networkMode
@@ -682,11 +670,17 @@ func (cr *containerReference) create(capAdd, capDrop []string) common.Executor {
 		}
 
 		var platSpecs *specs.Platform
-		if cr.input.Platform != "" && supportsContainerImagePlatform(ctx, cr.cli) {
-			platSpecs, err = parsePlatform(cr.input.Platform)
+		if cr.input.Platform != "" {
+			// Dropping the platform silently would build for the host arch.
+			supported, err := supportsContainerImagePlatform(ctx, cr.cli)
 			if err != nil {
 				return err
 			}
+			if supported {
+				if platSpecs, err = parsePlatform(cr.input.Platform); err != nil {
+					return err
+				}
+			}
 		}
 
 		hostConfig := &container.HostConfig{
@@ -938,42 +932,6 @@ func (cr *containerReference) waitForCommand(ctx context.Context, resp client.Hi
 	}
 }
 
-func (cr *containerReference) CopyTarStream(ctx context.Context, destPath string, tarStream io.Reader) error {
-	if cr.id == "" {
-		return cr.missingContainerError("copy to %s", destPath)
-	}
-	// Mkdir, with a path relative to the DestinationPath ("/") below. Docker 29.5+
-	// rejects absolute tar entry names with "path escapes from parent".
-	buf := &bytes.Buffer{}
-	tw := tar.NewWriter(buf)
-	_ = tw.WriteHeader(&tar.Header{
-		Name:     strings.TrimPrefix(destPath, "/"),
-		Mode:     0o777,
-		Typeflag: tar.TypeDir,
-	})
-	tw.Close()
-	_, err := cr.cli.CopyToContainer(ctx, cr.id, client.CopyToContainerOptions{
-		DestinationPath: "/",
-		Content:         buf,
-	})
-	if err != nil {
-		return fmt.Errorf("failed to mkdir to copy content to container: %w", err)
-	}
-	// Copy Content
-	_, err = cr.cli.CopyToContainer(ctx, cr.id, client.CopyToContainerOptions{
-		DestinationPath: destPath,
-		Content:         tarStream,
-	})
-	if err != nil {
-		return fmt.Errorf("failed to copy content to container: %w", err)
-	}
-	// If this fails, then folders have wrong permissions on non root container
-	if cr.UID != 0 || cr.GID != 0 {
-		_ = cr.Exec([]string{"chown", "-R", fmt.Sprintf("%d:%d", cr.UID, cr.GID), destPath}, nil, "0", "")(ctx)
-	}
-	return nil
-}
-
 func (cr *containerReference) copyDir(dstPath, srcPath string, useGitIgnore bool) common.Executor {
 	return func(ctx context.Context) error {
 		if cr.id == "" {
@@ -1015,7 +973,6 @@ func (cr *containerReference) copyDir(dstPath, srcPath string, useGitIgnore bool
 		}
 
 		fc := &filecollector.FileCollector{
-			Fs:        &filecollector.DefaultFs{},
 			Ignorer:   ignorer,
 			SrcPath:   srcPath,
 			SrcPrefix: srcPrefix,
@@ -1166,74 +1123,64 @@ func (cr *containerReference) wait() common.Executor {
 }
 
 // For Gitea
-// sanitizeOptionsHostConfig clears the HostConfig fields parsed from a
-// workflow-controlled container.options string that could be used to escape the
-// container when privileged mode is disabled. It must only be called when the
-// runner has privileged mode turned off; with privileged mode enabled the
-// administrator has already opted into host access.
-func sanitizeOptionsHostConfig(logger logrus.FieldLogger, hostConfig *container.HostConfig) {
-	warn := func(option string) {
-		logger.Warnf("container option %q is not allowed when privileged mode is disabled and will be ignored", option)
-	}
+// sanitizeOptionsHostConfig takes back everything a workflow could escape the container with,
+// setting each field to trusted, which is what the runner's own options parse to on their own.
+// Only for unprivileged mode, since privileged mode grants host access anyway.
+func sanitizeOptionsHostConfig(logger logrus.FieldLogger, hostConfig, trusted *container.HostConfig) {
+	resetOption(logger, "--pid", &hostConfig.PidMode, trusted.PidMode)
+	resetOption(logger, "--ipc", &hostConfig.IpcMode, trusted.IpcMode)
+	resetOption(logger, "--uts", &hostConfig.UTSMode, trusted.UTSMode)
+	resetOption(logger, "--cgroupns", &hostConfig.CgroupnsMode, trusted.CgroupnsMode)
+	resetOption(logger, "--userns", &hostConfig.UsernsMode, trusted.UsernsMode) // --userns=host would undo the remapping the runner asked for
+	resetOption(logger, "--cap-add", &hostConfig.CapAdd, trusted.CapAdd)
+	resetOption(logger, "--security-opt", &hostConfig.SecurityOpt, trusted.SecurityOpt)
+	resetOption(logger, "--device", &hostConfig.Devices, trusted.Devices)
+	resetOption(logger, "--device-cgroup-rule", &hostConfig.DeviceCgroupRules, trusted.DeviceCgroupRules)
+	resetOption(logger, "--gpus", &hostConfig.DeviceRequests, trusted.DeviceRequests)
+	resetOption(logger, "--volumes-from", &hostConfig.VolumesFrom, trusted.VolumesFrom)
+	resetOption(logger, "--runtime", &hostConfig.Runtime, trusted.Runtime)
+	resetOption(logger, "--cgroup-parent", &hostConfig.CgroupParent, trusted.CgroupParent)
+	resetOption(logger, "--sysctl", &hostConfig.Sysctls, trusted.Sysctls)
+	resetOption(logger, "--isolation", &hostConfig.Isolation, trusted.Isolation) // windows: process isolation drops the hyper-v boundary
+	resetOption(logger, "--volume-driver", &hostConfig.VolumeDriver, trusted.VolumeDriver)
+	// systempaths=unconfined lands in these two rather than in SecurityOpt
+	resetOption(logger, "--security-opt", &hostConfig.MaskedPaths, trusted.MaskedPaths)
+	resetOption(logger, "--security-opt", &hostConfig.ReadonlyPaths, trusted.ReadonlyPaths)
 
-	if hostConfig.PidMode != "" {
-		warn("--pid")
-		hostConfig.PidMode = ""
+	// a driver mounts what it likes, e.g. local with device= binds any host path, which
+	// valid_volumes never gets to see
+	hostConfig.Mounts = slices.DeleteFunc(hostConfig.Mounts, func(mt mount.Mount) bool {
+		if mt.VolumeOptions == nil || mt.VolumeOptions.DriverConfig == nil ||
+			slices.ContainsFunc(trusted.Mounts, func(t mount.Mount) bool { return reflect.DeepEqual(t, mt) }) {
+			return false
+		}
+		logger.Warnf("volume driver of %q in the workflow is not allowed when privileged mode is disabled and will be ignored", mt.Source)
+		return true
+	})
+}
+
+// resetOption puts a field back to the runner's own value. It compares the values rather than
+// the flags, so a field that more than one option feeds cannot slip through.
+func resetOption[T any](logger logrus.FieldLogger, option string, field *T, trusted T) {
+	if reflect.DeepEqual(*field, trusted) {
+		return
 	}
-	if hostConfig.IpcMode != "" {
-		warn("--ipc")
-		hostConfig.IpcMode = ""
+	logger.Warnf("container option %q in the workflow is not allowed when privileged mode is disabled and will be ignored", option)
+	*field = trusted
+}
+
+// parseOptionsHostConfig parses one options string on its own, to see what it alone asks for.
+// Even "" goes through the parser, or its empty slices and maps would differ from a real parse.
+func parseOptionsHostConfig(options string) (*container.HostConfig, error) {
+	flags, copts, _, err := parseContainerOptions(options)
+	if err != nil {
+		return nil, err
 	}
-	if hostConfig.UTSMode != "" {
-		warn("--uts")
-		hostConfig.UTSMode = ""
-	}
-	if hostConfig.CgroupnsMode != "" {
-		warn("--cgroupns")
-		hostConfig.CgroupnsMode = ""
-	}
-	// UsernsMode is set from the runner-controlled input; never let options
-	// override it (e.g. --userns=host disables user namespace remapping).
-	if hostConfig.UsernsMode != "" {
-		warn("--userns")
-		hostConfig.UsernsMode = ""
-	}
-	if len(hostConfig.CapAdd) > 0 {
-		warn("--cap-add")
-		hostConfig.CapAdd = nil
-	}
-	if len(hostConfig.SecurityOpt) > 0 {
-		warn("--security-opt")
-		hostConfig.SecurityOpt = nil
-	}
-	if len(hostConfig.Devices) > 0 {
-		warn("--device")
-		hostConfig.Devices = nil
-	}
-	if len(hostConfig.DeviceCgroupRules) > 0 {
-		warn("--device-cgroup-rule")
-		hostConfig.DeviceCgroupRules = nil
-	}
-	if len(hostConfig.DeviceRequests) > 0 {
-		warn("--gpus")
-		hostConfig.DeviceRequests = nil
-	}
-	if len(hostConfig.VolumesFrom) > 0 {
-		warn("--volumes-from")
-		hostConfig.VolumesFrom = nil
-	}
-	if hostConfig.Runtime != "" {
-		warn("--runtime")
-		hostConfig.Runtime = ""
-	}
-	if hostConfig.CgroupParent != "" {
-		warn("--cgroup-parent")
-		hostConfig.CgroupParent = ""
-	}
-	if len(hostConfig.Sysctls) > 0 {
-		warn("--sysctl")
-		hostConfig.Sysctls = nil
+	containerConfig, err := parse(flags, copts, runtime.GOOS)
+	if err != nil {
+		return nil, fmt.Errorf("cannot process container options: '%s': '%w'", options, err)
 	}
+	return containerConfig.HostConfig, nil
 }
 
 // For Gitea

act/container/docker_run_test.go

@@ -5,7 +5,6 @@
 package container
 
 import (
-	"archive/tar"
 	"bufio"
 	"bytes"
 	"context"
@@ -17,9 +16,8 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
-	"time"
 
-	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/act/common"
 
 	cerrdefs "github.com/containerd/errdefs"
 	"github.com/moby/moby/api/pkg/stdcopy"
@@ -79,6 +77,11 @@ type mockDockerClient struct {
 	mock.Mock
 }
 
+func (m *mockDockerClient) ServerVersion(ctx context.Context, opts mobyclient.ServerVersionOptions) (mobyclient.ServerVersionResult, error) {
+	args := m.Called(ctx, opts)
+	return args.Get(0).(mobyclient.ServerVersionResult), args.Error(1)
+}
+
 func (m *mockDockerClient) ExecCreate(ctx context.Context, id string, opts mobyclient.ExecCreateOptions) (mobyclient.ExecCreateResult, error) {
 	args := m.Called(ctx, id, opts)
 	return args.Get(0).(mobyclient.ExecCreateResult), args.Error(1)
@@ -144,12 +147,17 @@ func (m *mockDockerClient) NetworkRemove(ctx context.Context, id string, opts mo
 	return args.Get(0).(mobyclient.NetworkRemoveResult), args.Error(1)
 }
 
-type endlessReader struct {
-	io.Reader
+type interruptReader struct {
+	started     chan struct{}
+	interrupted chan struct{}
+	stopped     chan struct{}
 }
 
-func (r endlessReader) Read(_ []byte) (n int, err error) {
-	return 1, nil
+func (r *interruptReader) Read(_ []byte) (int, error) {
+	close(r.started)
+	<-r.interrupted
+	close(r.stopped)
+	return 0, io.EOF
 }
 
 type mockConn struct {
@@ -169,16 +177,17 @@ func (m *mockConn) Close() (err error) {
 func TestDockerExecAbort(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 
+	reader := &interruptReader{started: make(chan struct{}), interrupted: make(chan struct{}), stopped: make(chan struct{})}
 	conn := &mockConn{}
-	conn.On("Write", mock.AnythingOfType("[]uint8")).Return(1, nil)
+	conn.On("Write", []byte{3}).
+		Run(func(mock.Arguments) { close(reader.interrupted) }).
+		Return(1, nil)
 
 	client := &mockDockerClient{}
 	client.On("ExecCreate", ctx, "123", mock.AnythingOfType("client.ExecCreateOptions")).Return(mobyclient.ExecCreateResult{ID: "id"}, nil)
 	client.On("ExecAttach", ctx, "id", mock.AnythingOfType("client.ExecAttachOptions")).Return(mobyclient.ExecAttachResult{
-		HijackedResponse: mobyclient.HijackedResponse{
-			Conn:   conn,
-			Reader: bufio.NewReader(endlessReader{}),
-		},
+		Conn:   conn,
+		Reader: bufio.NewReader(reader),
 	}, nil)
 
 	cr := &containerReference{
@@ -195,11 +204,11 @@ func TestDockerExecAbort(t *testing.T) {
 		channel <- cr.exec([]string{""}, map[string]string{}, "user", "workdir")(ctx)
 	}()
 
-	time.Sleep(500 * time.Millisecond)
-
+	<-reader.started
 	cancel()
 
 	err := <-channel
+	<-reader.stopped
 	assert.ErrorIs(t, err, context.Canceled) //nolint:testifylint // pre-existing issue from nektos/act
 
 	conn.AssertExpectations(t)
@@ -214,10 +223,8 @@ func TestDockerExecFailure(t *testing.T) {
 	client := &mockDockerClient{}
 	client.On("ExecCreate", ctx, "123", mock.AnythingOfType("client.ExecCreateOptions")).Return(mobyclient.ExecCreateResult{ID: "id"}, nil)
 	client.On("ExecAttach", ctx, "id", mock.AnythingOfType("client.ExecAttachOptions")).Return(mobyclient.ExecAttachResult{
-		HijackedResponse: mobyclient.HijackedResponse{
-			Conn:   conn,
-			Reader: bufio.NewReader(strings.NewReader("output")),
-		},
+		Conn:   conn,
+		Reader: bufio.NewReader(strings.NewReader("output")),
 	}, nil)
 	client.On("ExecInspect", ctx, "id", mobyclient.ExecInspectOptions{}).Return(mobyclient.ExecInspectResult{
 		ExitCode: 1,
@@ -269,10 +276,8 @@ func TestDockerAttachFlushesTrailingLine(t *testing.T) {
 	client := &mockDockerClient{}
 	client.On("ContainerAttach", ctx, "123", mock.AnythingOfType("client.ContainerAttachOptions")).
 		Return(mobyclient.ContainerAttachResult{
-			HijackedResponse: mobyclient.HijackedResponse{
-				Conn:   &mockConn{},
-				Reader: bufio.NewReader(framed),
-			},
+			Conn:   &mockConn{},
+			Reader: bufio.NewReader(framed),
 		}, nil)
 
 	statusCh := make(chan container.WaitResponse, 1)
@@ -337,116 +342,6 @@ func TestDockerWaitFailure(t *testing.T) {
 	client.AssertExpectations(t)
 }
 
-func TestDockerCopyTarStream(t *testing.T) {
-	ctx := context.Background()
-
-	client := &mockDockerClient{}
-	client.On("CopyToContainer", ctx, "123", mock.MatchedBy(func(opts mobyclient.CopyToContainerOptions) bool {
-		return opts.DestinationPath == "/" && opts.Content != nil
-	})).Return(mobyclient.CopyToContainerResult{}, nil)
-	client.On("CopyToContainer", ctx, "123", mock.MatchedBy(func(opts mobyclient.CopyToContainerOptions) bool {
-		return opts.DestinationPath == "/var/run/act" && opts.Content != nil
-	})).Return(mobyclient.CopyToContainerResult{}, nil)
-	cr := &containerReference{
-		id:  "123",
-		cli: client,
-		input: &NewContainerInput{
-			Image: "image",
-		},
-	}
-
-	_ = cr.CopyTarStream(ctx, "/var/run/act", &bytes.Buffer{})
-
-	client.AssertExpectations(t)
-}
-
-// Docker 29.5+ rejects absolute names in the mkdir tarball with
-// "path escapes from parent", since it is extracted relative to "/".
-func TestDockerCopyTarStreamMkdirEntryIsRelative(t *testing.T) {
-	ctx := context.Background()
-
-	var mkdirNames []string
-	client := &mockDockerClient{}
-	client.On("CopyToContainer", ctx, "123", mock.MatchedBy(func(opts mobyclient.CopyToContainerOptions) bool {
-		if opts.DestinationPath != "/" || opts.Content == nil {
-			return false
-		}
-		tr := tar.NewReader(opts.Content)
-		for {
-			hdr, err := tr.Next()
-			if err != nil {
-				break
-			}
-			mkdirNames = append(mkdirNames, hdr.Name)
-		}
-		return true
-	})).Return(mobyclient.CopyToContainerResult{}, nil)
-	client.On("CopyToContainer", ctx, "123", mock.MatchedBy(func(opts mobyclient.CopyToContainerOptions) bool {
-		return opts.DestinationPath == "/var/run/act" && opts.Content != nil
-	})).Return(mobyclient.CopyToContainerResult{}, nil)
-	cr := &containerReference{
-		id:  "123",
-		cli: client,
-		input: &NewContainerInput{
-			Image: "image",
-		},
-	}
-
-	require.NoError(t, cr.CopyTarStream(ctx, "/var/run/act", &bytes.Buffer{}))
-	assert.Equal(t, []string{"var/run/act"}, mkdirNames)
-
-	client.AssertExpectations(t)
-}
-
-func TestDockerCopyTarStreamErrorInCopyFiles(t *testing.T) {
-	ctx := context.Background()
-
-	merr := errors.New("Failure")
-
-	client := &mockDockerClient{}
-	client.On("CopyToContainer", ctx, "123", mock.MatchedBy(func(opts mobyclient.CopyToContainerOptions) bool {
-		return opts.DestinationPath == "/" && opts.Content != nil
-	})).Return(mobyclient.CopyToContainerResult{}, merr)
-	cr := &containerReference{
-		id:  "123",
-		cli: client,
-		input: &NewContainerInput{
-			Image: "image",
-		},
-	}
-
-	err := cr.CopyTarStream(ctx, "/var/run/act", &bytes.Buffer{})
-	assert.ErrorIs(t, err, merr) //nolint:testifylint // pre-existing issue from nektos/act
-
-	client.AssertExpectations(t)
-}
-
-func TestDockerCopyTarStreamErrorInMkdir(t *testing.T) {
-	ctx := context.Background()
-
-	merr := errors.New("Failure")
-
-	client := &mockDockerClient{}
-	client.On("CopyToContainer", ctx, "123", mock.MatchedBy(func(opts mobyclient.CopyToContainerOptions) bool {
-		return opts.DestinationPath == "/" && opts.Content != nil
-	})).Return(mobyclient.CopyToContainerResult{}, nil)
-	client.On("CopyToContainer", ctx, "123", mock.MatchedBy(func(opts mobyclient.CopyToContainerOptions) bool {
-		return opts.DestinationPath == "/var/run/act" && opts.Content != nil
-	})).Return(mobyclient.CopyToContainerResult{}, merr)
-	cr := &containerReference{
-		id:  "123",
-		cli: client,
-		input: &NewContainerInput{
-			Image: "image",
-		},
-	}
-
-	err := cr.CopyTarStream(ctx, "/var/run/act", &bytes.Buffer{})
-	assert.ErrorIs(t, err, merr) //nolint:testifylint // pre-existing issue from nektos/act
-
-	client.AssertExpectations(t)
-}
-
 // A remove that raced the daemon's AutoRemove teardown is not a failure and must not
 // be logged as one.
 func TestRemoveIgnoresAutoRemoveRace(t *testing.T) {
@@ -577,7 +472,6 @@ func TestRejectsMissingContainer(t *testing.T) {
 	}
 	check("copyContent", cr.copyContent("/var/run/act", &FileEntry{Name: "x", Mode: 0o644})(ctx))
 	check("copyDir", cr.copyDir("/var/run/act", "/src", false)(ctx))
-	check("CopyTarStream", cr.CopyTarStream(ctx, "/var/run/act", &bytes.Buffer{}))
 	check("exec", cr.exec([]string{"echo"}, nil, "", "")(ctx))
 	_, err := cr.GetContainerArchive(ctx, "/var/run/act/x")
 	check("GetContainerArchive", err)
@@ -613,35 +507,6 @@ func TestPublicCopyPipelineHandlesStaleID(t *testing.T) {
 	client.AssertExpectations(t)
 }
 
-// TestDockerCopyToSymlinkPath is a regression test for gitea/runner#981. Most base images
-// symlink /var/run to /run, so copying into /var/run/act traverses that symlink. The broken
-// docker 29.5.1 daemon fails the extraction with "mkdirat var/run: file exists" (fixed in
-// 29.5.2). Running against the daemon shipped in the dind image, this catches a bad bump.
-func TestDockerCopyToSymlinkPath(t *testing.T) {
-	requireDocker(t)
-	ctx := context.Background()
-
-	rc := NewContainer(&NewContainerInput{
-		Image:      "alpine:latest",
-		Entrypoint: []string{"sleep", "30"},
-		Name:       "act-test-symlink-" + time.Now().Format("20060102150405.000000"),
-		AutoRemove: true,
-	})
-	require.NoError(t, rc.Pull(false)(ctx))
-	require.NoError(t, rc.Create(nil, nil)(ctx))
-	require.NoError(t, rc.Start(false)(ctx))
-	t.Cleanup(func() {
-		_ = rc.Remove()(ctx)
-		_ = rc.Close()(ctx)
-	})
-
-	// CopyTarStream first creates the destination directory by extracting a tar at "/",
-	// which makes the daemon mkdir var, then var/run (the symlink), then act — the exact
-	// step that fails on the broken daemon.
-	err := rc.CopyTarStream(ctx, "/var/run/act/actions/", &bytes.Buffer{})
-	require.NoError(t, err)
-}
-
 // Type assert containerReference implements ExecutionsEnvironment
 var _ ExecutionsEnvironment = &containerReference{}
 
@@ -718,108 +583,133 @@ func TestCheckVolumes(t *testing.T) {
 	}
 }
 
+// A volume driver decides for itself what it mounts, e.g. the local driver with device= binds
+// any host path, which valid_volumes never gets to see.
+func TestMergeContainerConfigsDropsVolumeDriversFromWorkflows(t *testing.T) {
+	const escape = "--mount type=volume,src=job-escape,dst=/host,volume-driver=local,volume-opt=type=none,volume-opt=o=bind,volume-opt=device=/"
+
+	hostConfig, _ := mergeOptions(t, "", escape+" --mount type=volume,src=job-plain,dst=/cache", false)
+	require.Len(t, hostConfig.Mounts, 1)
+	assert.Equal(t, "job-plain", hostConfig.Mounts[0].Source)
+
+	// the same mount from the runner's own options is the administrator's to make
+	hostConfig, _ = mergeOptions(t, escape, "", false)
+	require.Len(t, hostConfig.Mounts, 1)
+	assert.Equal(t, "job-escape", hostConfig.Mounts[0].Source)
+}
+
+// Both of these are read here, on the runner, so a workflow could read the runner's files
+// and environment with them.
+func TestMergeContainerConfigsKeepsTheRunnersFilesAndEnvToItself(t *testing.T) {
+	hostFile := filepath.Join(t.TempDir(), "host.env")
+	require.NoError(t, os.WriteFile(hostFile, []byte("STOLEN=from-the-host\n"), 0o600))
+	t.Setenv("RUNNER_SECRET", "s3cr3t")
+
+	for _, option := range []string{"--env-file " + hostFile, "--label-file " + hostFile} {
+		logger, _ := test.NewNullLogger()
+		cr := &containerReference{input: &NewContainerInput{NetworkMode: "bridge", WorkflowOptions: option}}
+
+		_, _, err := cr.mergeContainerConfigs(common.WithLogger(context.Background(), logger), &container.Config{}, &container.HostConfig{})
+		require.ErrorContains(t, err, "not allowed in a workflow")
+
+		// the runner reading its own files is what those options are for
+		cr = &containerReference{input: &NewContainerInput{NetworkMode: "bridge", RunnerOptions: option}}
+		_, _, err = cr.mergeContainerConfigs(common.WithLogger(context.Background(), logger), &container.Config{}, &container.HostConfig{})
+		require.NoError(t, err)
+	}
+
+	// a bare name is no longer resolved from the runner's environment, for either source
+	logger, _ := test.NewNullLogger()
+	cr := &containerReference{input: &NewContainerInput{
+		NetworkMode:     "bridge",
+		RunnerOptions:   "--env RUNNER_SECRET",
+		WorkflowOptions: "--env RUNNER_SECRET --env GIVEN=value",
+	}}
+
+	config, _, err := cr.mergeContainerConfigs(common.WithLogger(context.Background(), logger), &container.Config{}, &container.HostConfig{})
+	require.NoError(t, err)
+	assert.Equal(t, []string{"RUNNER_SECRET", "RUNNER_SECRET", "GIVEN=value"}, config.Env)
+}
+
 func TestSanitizeOptionsHostConfig(t *testing.T) {
 	logger, _ := test.NewNullLogger()
 
-	dangerous := func() *container.HostConfig {
-		return &container.HostConfig{
-			PidMode:      "host",
-			IpcMode:      "host",
-			UTSMode:      "host",
-			CgroupnsMode: "host",
-			UsernsMode:   "host",
-			CapAdd:       []string{"ALL"},
-			SecurityOpt:  []string{"seccomp=unconfined", "apparmor=unconfined"},
-			VolumesFrom:  []string{"other"},
-			Runtime:      "runc",
-			Resources: container.Resources{
-				CgroupParent:      "/custom",
-				Devices:           []container.DeviceMapping{{PathOnHost: "/dev/sda", PathInContainer: "/dev/sda", CgroupPermissions: "rwm"}},
-				DeviceCgroupRules: []string{"a *:* rwm"},
-			},
-			Sysctls: map[string]string{"net.ipv4.ip_forward": "1"},
-		}
+	// every field the sanitizer resets, so a reset dropped in a refactor fails here
+	hostConfig := &container.HostConfig{
+		PidMode:           "host",
+		IpcMode:           "host",
+		UTSMode:           "host",
+		CgroupnsMode:      "host",
+		UsernsMode:        "host",
+		CapAdd:            []string{"ALL"},
+		SecurityOpt:       []string{"seccomp=unconfined", "apparmor=unconfined"},
+		VolumesFrom:       []string{"other"},
+		Runtime:           "runc",
+		Isolation:         "process",
+		VolumeDriver:      "rogue",
+		MaskedPaths:       []string{},
+		ReadonlyPaths:     []string{},
+		CgroupParent:      "/custom",
+		Devices:           []container.DeviceMapping{{PathOnHost: "/dev/sda", PathInContainer: "/dev/sda", CgroupPermissions: "rwm"}},
+		DeviceCgroupRules: []string{"a *:* rwm"},
+		DeviceRequests:    []container.DeviceRequest{{Count: -1, Capabilities: [][]string{{"gpu"}}}},
+		Sysctls:           map[string]string{"net.ipv4.ip_forward": "1"},
 	}
 
-	hostConfig := dangerous()
-	sanitizeOptionsHostConfig(logger, hostConfig)
+	sanitizeOptionsHostConfig(logger, hostConfig, &container.HostConfig{})
 
-	assert.Empty(t, string(hostConfig.PidMode))
-	assert.Empty(t, string(hostConfig.IpcMode))
-	assert.Empty(t, string(hostConfig.UTSMode))
-	assert.Empty(t, string(hostConfig.CgroupnsMode))
-	assert.Empty(t, string(hostConfig.UsernsMode))
-	assert.Empty(t, hostConfig.CapAdd)
-	assert.Empty(t, hostConfig.SecurityOpt)
-	assert.Empty(t, hostConfig.Devices)
-	assert.Empty(t, hostConfig.DeviceCgroupRules)
-	assert.Empty(t, hostConfig.VolumesFrom)
-	assert.Empty(t, hostConfig.Runtime)
-	assert.Empty(t, hostConfig.CgroupParent)
-	assert.Empty(t, hostConfig.Sysctls)
+	assert.Equal(t, &container.HostConfig{}, hostConfig)
+}
+
+// mergeOptions merges both option sources into a bare container, returning the result and its log.
+func mergeOptions(t *testing.T, runnerOptions, workflowOptions string, privileged bool) (*container.HostConfig, *test.Hook) {
+	t.Helper()
+	logger, hook := test.NewNullLogger()
+	cr := &containerReference{input: &NewContainerInput{
+		RunnerOptions:   runnerOptions,
+		WorkflowOptions: workflowOptions,
+		NetworkMode:     "bridge",
+		UsernsMode:      "private",
+	}}
+
+	_, hostConfig, err := cr.mergeContainerConfigs(common.WithLogger(context.Background(), logger), &container.Config{}, &container.HostConfig{
+		Privileged:  privileged,
+		UsernsMode:  container.UsernsMode("private"),
+		NetworkMode: container.NetworkMode("bridge"),
+	})
+	require.NoError(t, err)
+	return hostConfig, hook
 }
 
 func TestMergeContainerConfigsStripsDangerousOptionsWhenUnprivileged(t *testing.T) {
-	// OS-independent options only: --device parsing requires a linux/windows
-	// server OS, which is not guaranteed for the test host.
+	// OS-independent options only, --device and --gpus need a linux/windows server OS
 	const dangerousOptions = "--pid=host --ipc=host --uts=host --cgroupns=host " +
 		"--userns=host --cap-add=ALL --security-opt seccomp=unconfined " +
-		"--security-opt apparmor=unconfined --volumes-from other " +
+		"--security-opt apparmor=unconfined --volumes-from other --isolation process " +
 		"--runtime runc --cgroup-parent /custom --sysctl net.ipv4.ip_forward=1"
 
-	t.Run("unprivileged strips host-escape options", func(t *testing.T) {
-		logger, _ := test.NewNullLogger()
-		ctx := common.WithLogger(context.Background(), logger)
-		cr := &containerReference{
-			input: &NewContainerInput{
-				Options:     dangerousOptions,
-				NetworkMode: "bridge",
-				UsernsMode:  "private",
-			},
-		}
+	// whatever the workflow adds, an unprivileged container comes out exactly as the runner's
+	// own options alone describe it, field for field
+	for _, runnerOptions := range []string{"--shm-size 1g", dangerousOptions, "--cap-add SYS_ADMIN --security-opt seccomp=unconfined"} {
+		runnerOnly, _ := mergeOptions(t, runnerOptions, "", false)
+		withWorkflow, _ := mergeOptions(t, runnerOptions, dangerousOptions, false)
 
-		_, hostConfig, err := cr.mergeContainerConfigs(ctx, &container.Config{}, &container.HostConfig{
-			Privileged:  false,
-			UsernsMode:  container.UsernsMode("private"),
-			NetworkMode: container.NetworkMode("bridge"),
-		})
-		require.NoError(t, err)
+		assert.Equal(t, runnerOnly, withWorkflow, "runner options: %q", runnerOptions)
+	}
 
-		assert.False(t, hostConfig.Privileged)
-		assert.Empty(t, string(hostConfig.PidMode))
-		assert.Empty(t, string(hostConfig.IpcMode))
-		assert.Empty(t, string(hostConfig.UTSMode))
-		assert.Empty(t, string(hostConfig.CgroupnsMode))
-		// UsernsMode must keep the runner-controlled value, not the one from options.
-		assert.Equal(t, "private", string(hostConfig.UsernsMode))
-		assert.Empty(t, hostConfig.CapAdd)
-		assert.Empty(t, hostConfig.SecurityOpt)
-		assert.Empty(t, hostConfig.VolumesFrom)
-		assert.Empty(t, hostConfig.Runtime)
-		assert.Empty(t, hostConfig.CgroupParent)
-		assert.Empty(t, hostConfig.Sysctls)
-	})
+	// the same options from the runner reach the daemon, even --userns, which no workflow may set
+	kept, _ := mergeOptions(t, dangerousOptions, "", false)
+	assert.Equal(t, "host", string(kept.PidMode))
+	assert.Equal(t, []string{"ALL"}, kept.CapAdd)
+	assert.Equal(t, "runc", kept.Runtime)
+	assert.Equal(t, "host", string(kept.UsernsMode))
+	assert.False(t, kept.Privileged)
 
-	t.Run("privileged preserves options", func(t *testing.T) {
-		logger, _ := test.NewNullLogger()
-		ctx := common.WithLogger(context.Background(), logger)
-		cr := &containerReference{
-			input: &NewContainerInput{
-				Options:     "--pid=host --cap-add=ALL --security-opt seccomp=unconfined",
-				NetworkMode: "bridge",
-			},
-		}
-
-		_, hostConfig, err := cr.mergeContainerConfigs(ctx, &container.Config{}, &container.HostConfig{
-			Privileged:  true,
-			NetworkMode: container.NetworkMode("bridge"),
-		})
-		require.NoError(t, err)
-
-		assert.Equal(t, "host", string(hostConfig.PidMode))
-		assert.Equal(t, []string{"ALL"}, hostConfig.CapAdd)
-		assert.Equal(t, []string{"seccomp=unconfined"}, hostConfig.SecurityOpt)
-	})
+	// privileged is the administrator opting in, so the workflow's options are honored
+	privileged, _ := mergeOptions(t, "", dangerousOptions, true)
+	assert.Equal(t, "host", string(privileged.PidMode))
+	assert.Equal(t, []string{"ALL"}, privileged.CapAdd)
+	assert.Equal(t, []string{"seccomp=unconfined", "apparmor=unconfined"}, privileged.SecurityOpt)
 }
 
 func TestCheckVolumesRejectsEscapingHostPaths(t *testing.T) {
@@ -917,8 +807,8 @@ func TestMergeContainerConfigsVolumesReplaceRunnerMounts(t *testing.T) {
 	ctx := common.WithLogger(context.Background(), logger)
 	cr := &containerReference{
 		input: &NewContainerInput{
-			NetworkMode: "bridge",
-			Options:     "--volume /host/tools:/opt/hostedtoolcache",
+			NetworkMode:   "bridge",
+			RunnerOptions: "--volume /host/tools:/opt/hostedtoolcache",
 		},
 	}
 
@@ -930,3 +820,26 @@ func TestMergeContainerConfigsVolumesReplaceRunnerMounts(t *testing.T) {
 	assert.Equal(t, []string{"/var/run/docker.sock:/var/run/docker.sock", "/host/tools:/opt/hostedtoolcache"}, hostConf.Binds)
 	assert.Empty(t, hostConf.Mounts)
 }
+
+func TestMergeContainerConfigsWarnsOnlyAboutOptionsThatWereGiven(t *testing.T) {
+	warnings := func(runnerOptions, workflowOptions string) int {
+		_, hook := mergeOptions(t, runnerOptions, workflowOptions, false)
+		return len(hook.AllEntries())
+	}
+
+	assert.Zero(t, warnings("--volume /host/tools:/opt/hostedtoolcache", ""))
+	assert.Zero(t, warnings("", "--shm-size 1g"))
+	assert.Equal(t, 1, warnings("--network host", ""))
+}
+
+// A dead daemon must fail the job, not panic through logrus and not silently
+// drop the requested platform.
+func TestSupportsContainerImagePlatformDaemonError(t *testing.T) {
+	cli := &mockDockerClient{}
+	cli.On("ServerVersion", mock.Anything, mock.Anything).
+		Return(mobyclient.ServerVersionResult{}, errors.New("cannot connect to the Docker daemon"))
+
+	supported, err := supportsContainerImagePlatform(t.Context(), cli)
+	require.ErrorContains(t, err, "cannot connect to the Docker daemon")
+	assert.False(t, supported)
+}

act/container/docker_stub.go

@@ -12,7 +12,7 @@ import (
 	"runtime"
 	"time"
 
-	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/act/common"
 
 	"github.com/moby/moby/api/types/system"
 )

act/container/docker_volume.go

@@ -9,7 +9,7 @@ package container
 import (
 	"context"
 
-	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/act/common"
 
 	"github.com/moby/moby/client"
 )

act/container/host_environment.go

@@ -21,11 +21,12 @@ import (
 	"sync/atomic"
 	"time"
 
-	"gitea.com/gitea/runner/internal/act/common"
-	"gitea.com/gitea/runner/internal/act/filecollector"
-	"gitea.com/gitea/runner/internal/act/lookpath"
+	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/act/filecollector"
+	"gitea.com/gitea/runner/act/lookpath"
 	"gitea.com/gitea/runner/internal/pkg/process"
 
+	"github.com/creack/pty"
 	"github.com/go-git/go-billy/v5/helper/polyfill"
 	"github.com/go-git/go-billy/v5/osfs"
 	"github.com/go-git/go-git/v5/plumbing/format/gitignore"
@@ -71,12 +72,6 @@ func (e *HostEnvironment) Create(_, _ []string) common.Executor {
 	}
 }
 
-func (e *HostEnvironment) ConnectToNetwork(name string) common.Executor {
-	return func(ctx context.Context) error {
-		return nil
-	}
-}
-
 func (e *HostEnvironment) Close() common.Executor {
 	return func(ctx context.Context) error {
 		return nil
@@ -97,33 +92,6 @@ func (e *HostEnvironment) Copy(destPath string, files ...*FileEntry) common.Exec
 	}
 }
 
-func (e *HostEnvironment) CopyTarStream(ctx context.Context, destPath string, tarStream io.Reader) error {
-	if err := os.RemoveAll(destPath); err != nil {
-		return err
-	}
-	tr := tar.NewReader(tarStream)
-	cp := &filecollector.CopyCollector{
-		DstDir: destPath,
-	}
-	for {
-		ti, err := tr.Next()
-		if errors.Is(err, io.EOF) {
-			return nil
-		} else if err != nil {
-			return err
-		}
-		if ti.FileInfo().IsDir() {
-			continue
-		}
-		if ctx.Err() != nil {
-			return errors.New("CopyTarStream has been cancelled")
-		}
-		if err := cp.WriteFile(ti.Name, ti.FileInfo(), ti.Linkname, tr); err != nil {
-			return err
-		}
-	}
-}
-
 func (e *HostEnvironment) CopyDir(destPath, srcPath string, useGitIgnore bool) common.Executor {
 	return func(ctx context.Context) error {
 		logger := common.Logger(ctx)
@@ -142,7 +110,6 @@ func (e *HostEnvironment) CopyDir(destPath, srcPath string, useGitIgnore bool) c
 			ignorer = gitignore.NewMatcher(ps)
 		}
 		fc := &filecollector.FileCollector{
-			Fs:        &filecollector.DefaultFs{},
 			Ignorer:   ignorer,
 			SrcPath:   srcPath,
 			SrcPrefix: srcPrefix,
@@ -180,7 +147,6 @@ func (e *HostEnvironment) GetContainerArchive(ctx context.Context, srcPath strin
 			srcPrefix += string(filepath.Separator)
 		}
 		fc := &filecollector.FileCollector{
-			Fs:        &filecollector.DefaultFs{},
 			SrcPath:   srcPath,
 			SrcPrefix: srcPrefix,
 			Handler:   tc,
@@ -246,24 +212,8 @@ func (w *ptyWriter) Write(buf []byte) (int, error) {
 	return w.Out.Write(buf)
 }
 
-type localEnv struct {
-	env map[string]string
-}
-
-func (l *localEnv) Getenv(name string) string {
-	if runtime.GOOS == "windows" {
-		for k, v := range l.env {
-			if strings.EqualFold(name, k) {
-				return v
-			}
-		}
-		return ""
-	}
-	return l.env[name]
-}
-
 func lookupPathHost(cmd string, env map[string]string, writer io.Writer) (string, error) {
-	f, err := lookpath.LookPath2(cmd, &localEnv{env: env})
+	f, err := lookpath.LookPath2(cmd, env)
 	if err != nil {
 		err := "Cannot find: " + cmd + " in PATH"
 		if _, _err := writer.Write([]byte(err + "\n")); _err != nil {
@@ -275,7 +225,7 @@ func lookupPathHost(cmd string, env map[string]string, writer io.Writer) (string
 }
 
 func setupPty(cmd *exec.Cmd, cmdline string) (*os.File, *os.File, error) {
-	ppty, tty, err := openPty()
+	ppty, tty, err := pty.Open()
 	if err != nil {
 		return nil, nil, err
 	}
@@ -401,8 +351,7 @@ func (e *HostEnvironment) exec(ctx context.Context, command []string, cmdline st
 	}
 	err = cmd.Wait()
 	if err != nil {
-		var exitErr *exec.ExitError
-		if errors.As(err, &exitErr) {
+		if exitErr, ok := errors.AsType[*exec.ExitError](err); ok {
 			return ExitCodeError(exitErr.ExitCode())
 		}
 		return err

act/container/host_environment_test.go

@@ -17,7 +17,7 @@ import (
 	"testing"
 	"time"
 
-	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/act/common"
 
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"

act/container/parse_env_file.go

@@ -12,7 +12,7 @@ import (
 	"io"
 	"strings"
 
-	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/act/common"
 
 	"golang.org/x/text/encoding/unicode"
 	"golang.org/x/text/transform"
@@ -46,9 +46,10 @@ func parseEnvFile(e Container, srcPath string, env *map[string]string) common.Ex
 			}
 			singleLineEnv := strings.Index(line, "=")
 			multiLineEnv := strings.Index(line, "<<")
-			if singleLineEnv != -1 && (multiLineEnv == -1 || singleLineEnv < multiLineEnv) {
+			switch {
+			case singleLineEnv != -1 && (multiLineEnv == -1 || singleLineEnv < multiLineEnv):
 				localEnv[line[:singleLineEnv]] = line[singleLineEnv+1:]
-			} else if multiLineEnv != -1 {
+			case multiLineEnv != -1:
 				multiLineEnvContent := ""
 				multiLineEnvDelimiter := line[multiLineEnv+2:]
 				delimiterFound := false
@@ -70,7 +71,7 @@ func parseEnvFile(e Container, srcPath string, env *map[string]string) common.Ex
 					return fmt.Errorf("invalid format delimiter '%v' not found before end of file", multiLineEnvDelimiter)
 				}
 				localEnv[line[:multiLineEnv]] = multiLineEnvContent
-			} else {
+			default:
 				return fmt.Errorf("invalid format '%v', expected a line with '=' or '<<'", line)
 			}
 		}

act/filecollector/file_collector.go

@@ -97,55 +97,25 @@ type FileCollector struct {
 	Ignorer   gitignore.Matcher
 	SrcPath   string
 	SrcPrefix string
-	Fs        Fs
 	Handler   Handler
 }
 
-type Fs interface {
-	Walk(root string, fn filepath.WalkFunc) error
-	OpenGitIndex(path string) (*index.Index, error)
-	Open(path string) (io.ReadCloser, error)
-	Readlink(path string) (string, error)
-}
-
-type DefaultFs struct{}
-
-func (*DefaultFs) Walk(root string, fn filepath.WalkFunc) error {
-	return filepath.Walk(root, fn)
-}
-
-func (*DefaultFs) OpenGitIndex(path string) (*index.Index, error) {
-	r, err := git.PlainOpen(path)
+func openGitIndex(path string) (*index.Index, error) {
+	repo, err := git.PlainOpen(path)
 	if err != nil {
 		return nil, err
 	}
-	i, err := r.Storer.Index()
-	if err != nil {
-		return nil, err
-	}
-	return i, nil
-}
-
-func (*DefaultFs) Open(path string) (io.ReadCloser, error) {
-	return os.Open(path)
-}
-
-func (*DefaultFs) Readlink(path string) (string, error) {
-	return os.Readlink(path)
+	return repo.Storer.Index()
 }
 
 func (fc *FileCollector) CollectFiles(ctx context.Context, submodulePath []string) filepath.WalkFunc {
-	i, _ := fc.Fs.OpenGitIndex(path.Join(fc.SrcPath, path.Join(submodulePath...)))
+	i, _ := openGitIndex(path.Join(fc.SrcPath, path.Join(submodulePath...)))
 	return func(file string, fi os.FileInfo, err error) error {
 		if err != nil {
 			return err
 		}
-		if ctx != nil {
-			select {
-			case <-ctx.Done():
-				return errors.New("copy cancelled")
-			default:
-			}
+		if ctx != nil && ctx.Err() != nil {
+			return errors.New("copy cancelled")
 		}
 
 		sansPrefix := strings.TrimPrefix(file, fc.SrcPrefix)
@@ -175,7 +145,7 @@ func (fc *FileCollector) CollectFiles(ctx context.Context, submodulePath []strin
 			}
 		}
 		if err == nil && entry.Mode == filemode.Submodule {
-			err = fc.Fs.Walk(file, fc.CollectFiles(ctx, split))
+			err = filepath.Walk(file, fc.CollectFiles(ctx, split))
 			if err != nil {
 				return err
 			}
@@ -185,7 +155,7 @@ func (fc *FileCollector) CollectFiles(ctx context.Context, submodulePath []strin
 
 		// return on non-regular files (thanks to [kumo](https://medium.com/@komuw/just-like-you-did-fbdd7df829d3) for this suggested update)
 		if fi.Mode()&os.ModeSymlink == os.ModeSymlink {
-			linkName, err := fc.Fs.Readlink(file)
+			linkName, err := os.Readlink(file)
 			if err != nil {
 				return fmt.Errorf("unable to readlink '%s': %w", file, err)
 			}
@@ -195,23 +165,15 @@ func (fc *FileCollector) CollectFiles(ctx context.Context, submodulePath []strin
 		}
 
 		// open file
-		f, err := fc.Fs.Open(file)
+		f, err := os.Open(file)
 		if err != nil {
 			return err
 		}
 		defer f.Close()
 
 		if ctx != nil {
-			// make io.Copy cancellable by closing the file
-			cpctx, cpfinish := context.WithCancel(ctx)
-			defer cpfinish()
-			go func() {
-				select {
-				case <-cpctx.Done():
-				case <-ctx.Done():
-					f.Close()
-				}
-			}()
+			stop := context.AfterFunc(ctx, func() { _ = f.Close() })
+			defer stop()
 		}
 
 		return fc.Handler.WriteFile(path, fi, "", f)

act/filecollector/file_collector_test.go

@@ -0,0 +1,153 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// Copyright 2024 The nektos/act Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package filecollector
+
+import (
+	"archive/tar"
+	"bytes"
+	"context"
+	"io"
+	"os"
+	"path/filepath"
+	"runtime"
+	"strings"
+	"testing"
+
+	git "github.com/go-git/go-git/v5"
+	"github.com/go-git/go-git/v5/plumbing/format/gitignore"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestIgnoredTrackedfile(t *testing.T) {
+	repoDir := filepath.Join(t.TempDir(), "mygitrepo")
+	repo, err := git.PlainInit(repoDir, false)
+	require.NoError(t, err)
+	require.NoError(t, os.WriteFile(filepath.Join(repoDir, ".gitignore"), []byte(".*\n"), 0o644))
+	require.NoError(t, os.WriteFile(filepath.Join(repoDir, ".env"), []byte("test=val1\n"), 0o644))
+	worktree, err := repo.Worktree()
+	require.NoError(t, err)
+	_, err = worktree.Add(".gitignore")
+	require.NoError(t, err)
+
+	var archive bytes.Buffer
+	tw := tar.NewWriter(&archive)
+	patterns, err := gitignore.ReadPatterns(worktree.Filesystem, nil)
+	require.NoError(t, err)
+	ignorer := gitignore.NewMatcher(patterns)
+	fc := &FileCollector{
+		Ignorer:   ignorer,
+		SrcPath:   repoDir,
+		SrcPrefix: repoDir + string(filepath.Separator),
+		Handler: &TarCollector{
+			TarWriter: tw,
+		},
+	}
+	err = filepath.Walk(repoDir, fc.CollectFiles(context.Background(), nil))
+	assert.NoError(t, err, "successfully collect files")
+	require.NoError(t, tw.Close())
+	tr := tar.NewReader(&archive)
+	h, err := tr.Next()
+	assert.NoError(t, err, "tar must not be empty") //nolint:testifylint // pre-existing issue from nektos/act
+	assert.Equal(t, ".gitignore", h.Name)
+	_, err = tr.Next()
+	assert.ErrorIs(t, err, io.EOF, "tar must only contain one element")
+}
+
+func TestSymlinks(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("creating symlinks requires elevated privileges on Windows")
+	}
+	repoDir := filepath.Join(t.TempDir(), "mygitrepo")
+	repo, err := git.PlainInit(repoDir, false)
+	require.NoError(t, err)
+	require.NoError(t, os.WriteFile(filepath.Join(repoDir, ".env"), []byte("test=val1\n"), 0o644))
+	require.NoError(t, os.Symlink(".env", filepath.Join(repoDir, "test.env")))
+	worktree, err := repo.Worktree()
+	require.NoError(t, err)
+	_, err = worktree.Add("test.env")
+	require.NoError(t, err)
+
+	var archive bytes.Buffer
+	tw := tar.NewWriter(&archive)
+	fc := &FileCollector{
+		SrcPath:   repoDir,
+		SrcPrefix: repoDir + string(filepath.Separator),
+		Handler: &TarCollector{
+			TarWriter: tw,
+		},
+	}
+	err = filepath.Walk(repoDir, fc.CollectFiles(context.Background(), nil))
+	assert.NoError(t, err, "successfully collect files")
+	require.NoError(t, tw.Close())
+	tr := tar.NewReader(&archive)
+	h, err := tr.Next()
+	files := map[string]tar.Header{}
+	for err == nil {
+		files[h.Name] = *h
+		h, err = tr.Next()
+	}
+
+	assert.Equal(t, ".env", files[".env"].Name)
+	assert.Equal(t, "test.env", files["test.env"].Name)
+	assert.Equal(t, ".env", files["test.env"].Linkname)
+	assert.ErrorIs(t, err, io.EOF, "tar must be read cleanly to EOF")
+}
+
+// Regression for https://gitea.com/gitea/runner/issues/876 and /941:
+// re-copying an action directory must overwrite a pre-existing read-only
+// file (e.g. a git pack .idx at mode 0444) instead of failing with EACCES
+// on macOS or "Access is denied" on Windows.
+func TestCopyCollectorWriteFileOverwritesReadOnlyFile(t *testing.T) {
+	dst := t.TempDir()
+	target := filepath.Join(dst, "sub", "pack.idx")
+	require.NoError(t, os.MkdirAll(filepath.Dir(target), 0o755))
+	require.NoError(t, os.WriteFile(target, []byte("old"), 0o444))
+
+	src := filepath.Join(t.TempDir(), "pack.idx")
+	require.NoError(t, os.WriteFile(src, []byte("new"), 0o444))
+	fi, err := os.Stat(src)
+	require.NoError(t, err)
+
+	cc := &CopyCollector{DstDir: dst}
+	require.NoError(t, cc.WriteFile("sub/pack.idx", fi, "", strings.NewReader("new")))
+
+	got, err := os.ReadFile(target)
+	require.NoError(t, err)
+	assert.Equal(t, "new", string(got))
+}
+
+// Without the destination removal, os.Symlink fails with EEXIST when the
+// path already holds a regular file from an earlier copy of the action.
+func TestCopyCollectorWriteFileOverwritesFileWithSymlink(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("creating symlinks requires elevated privileges on Windows")
+	}
+	dst := t.TempDir()
+	target := filepath.Join(dst, "link")
+	require.NoError(t, os.WriteFile(target, []byte("stale"), 0o644))
+
+	fi, err := os.Lstat(target)
+	require.NoError(t, err)
+
+	cc := &CopyCollector{DstDir: dst}
+	require.NoError(t, cc.WriteFile("link", fi, "target", nil))
+
+	resolved, err := os.Readlink(target)
+	require.NoError(t, err)
+	assert.Equal(t, "target", resolved)
+}
+
+func TestFileCollectorCancellationAndWalkError(t *testing.T) {
+	ctx, cancel := context.WithCancel(t.Context())
+	cancel()
+	walk := (&FileCollector{}).CollectFiles(ctx, nil)
+
+	err := walk("file", nil, nil)
+	require.EqualError(t, err, "copy cancelled")
+
+	err = walk("file", nil, os.ErrPermission)
+	require.ErrorIs(t, err, os.ErrPermission)
+}

act/ghcontext/github_context.go

@@ -13,8 +13,8 @@ import (
 	"fmt"
 	"strings"
 
-	"gitea.com/gitea/runner/internal/act/common"
-	"gitea.com/gitea/runner/internal/act/common/git"
+	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/act/common/git"
 
 	"gitea.dev/actionslib/pkg/model"
 )
@@ -25,32 +25,9 @@ var (
 	findGithubRepo  = git.FindGithubRepo
 )
 
-func withDefaultBranch(ctx context.Context, b string, event map[string]any) map[string]any {
-	repoI, ok := event["repository"]
-	if !ok {
-		repoI = make(map[string]any)
-	}
-
-	repo, ok := repoI.(map[string]any)
-	if !ok {
-		common.Logger(ctx).Warnf("unable to set default branch to %v", b)
-		return event
-	}
-
-	// if the branch is already there return with no changes
-	if _, ok = repo["default_branch"]; ok {
-		return event
-	}
-
-	repo["default_branch"] = b
-	event["repository"] = repo
-
-	return event
-}
-
 // SetRef resolves the ref of the context from its event payload, falling back
 // to the ref checked out in repoPath.
-func SetRef(ctx context.Context, ghc *model.GithubContext, defaultBranch, repoPath string) {
+func SetRef(ctx context.Context, ghc *model.GithubContext, repoPath string) {
 	logger := common.Logger(ctx)
 
 	// https://docs.github.com/en/actions/learn-github-actions/events-that-trigger-workflows
@@ -82,11 +59,15 @@ func SetRef(ctx context.Context, ghc *model.GithubContext, defaultBranch, repoPa
 			ghc.Ref = ref
 		}
 
-		// set the branch in the event data
-		if defaultBranch != "" {
-			ghc.Event = withDefaultBranch(ctx, defaultBranch, ghc.Event)
-		} else {
-			ghc.Event = withDefaultBranch(ctx, "master", ghc.Event)
+		repository, exists := ghc.Event["repository"]
+		if !exists {
+			repository = map[string]any{}
+		}
+		if repository, ok := repository.(map[string]any); !ok {
+			logger.Warn("unable to set default branch to master")
+		} else if _, exists := repository["default_branch"]; !exists {
+			repository["default_branch"] = "master"
+			ghc.Event["repository"] = repository
 		}
 
 		if ghc.Ref == "" {
@@ -125,11 +106,11 @@ func SetSha(ctx context.Context, ghc *model.GithubContext, repoPath string) {
 
 // SetRepositoryAndOwner resolves the repository of the context from the git
 // remote in repoPath when it is not set yet, and derives its owner.
-func SetRepositoryAndOwner(ctx context.Context, ghc *model.GithubContext, githubInstance, remoteName, repoPath string) {
+func SetRepositoryAndOwner(ctx context.Context, ghc *model.GithubContext, githubInstance, repoPath string) {
 	if ghc.Repository == "" {
-		repo, err := findGithubRepo(ctx, repoPath, githubInstance, remoteName)
+		repo, err := findGithubRepo(ctx, repoPath, githubInstance)
 		if err != nil {
-			common.Logger(ctx).Warningf("unable to get git repo (githubInstance: %v; remoteName: %v, repoPath: %v): %v", githubInstance, remoteName, repoPath, err)
+			common.Logger(ctx).Warningf("unable to get git repo (githubInstance: %v, repoPath: %v): %v", githubInstance, repoPath, err)
 			return
 		}
 		ghc.Repository = repo

act/ghcontext/github_context_test.go

@@ -104,7 +104,7 @@ func TestSetRef(t *testing.T) {
 				Event:     table.event,
 			}
 
-			SetRef(context.Background(), ghc, "main", "/some/dir")
+			SetRef(context.Background(), ghc, "/some/dir")
 			ghc.SetRefTypeAndName()
 
 			assert.Equal(t, table.ref, ghc.Ref)
@@ -122,7 +122,7 @@ func TestSetRef(t *testing.T) {
 			Event:     map[string]any{},
 		}
 
-		SetRef(context.Background(), ghc, "", "/some/dir")
+		SetRef(context.Background(), ghc, "/some/dir")
 
 		assert.Equal(t, "refs/heads/master", ghc.Ref)
 	})

act/lookpath/env.go

@@ -0,0 +1,21 @@
+// Copyright 2022 The Gitea Authors. All rights reserved.
+// Copyright 2022 The nektos/act Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package lookpath
+
+import (
+	"runtime"
+	"strings"
+)
+
+func getenv(env map[string]string, name string) string {
+	if runtime.GOOS == "windows" {
+		for key, value := range env {
+			if strings.EqualFold(name, key) {
+				return value
+			}
+		}
+	}
+	return env[name]
+}

act/lookpath/lp_js.go

@@ -18,7 +18,7 @@ var ErrNotFound = errors.New("executable file not found in $PATH")
 // directories named by the PATH environment variable.
 // If file contains a slash, it is tried directly and the PATH is not consulted.
 // The result may be an absolute path or a path relative to the current directory.
-func LookPath2(file string, lenv Env) (string, error) {
+func LookPath2(file string, _ map[string]string) (string, error) {
 	// Wasm can not execute processes, so act as if there are no executables at all.
 	return "", &Error{file, ErrNotFound}
 }

act/lookpath/lp_plan9.go

@@ -32,7 +32,7 @@ func findExecutable(file string) error {
 // If file begins with "/", "#", "./", or "../", it is tried
 // directly and the path is not consulted.
 // The result may be an absolute path or a path relative to the current directory.
-func LookPath2(file string, lenv Env) (string, error) {
+func LookPath2(file string, env map[string]string) (string, error) {
 	// skip the path lookup for these prefixes
 	skip := []string{"/", "#", "./", "../"}
 
@@ -46,7 +46,7 @@ func LookPath2(file string, lenv Env) (string, error) {
 		}
 	}
 
-	path := lenv.Getenv("path")
+	path := getenv(env, "path")
 	for _, dir := range filepath.SplitList(path) {
 		path := filepath.Join(dir, file)
 		if err := findExecutable(path); err == nil {

act/lookpath/lp_unix.go

@@ -33,7 +33,7 @@ func findExecutable(file string) error {
 // directories named by the PATH environment variable.
 // If file contains a slash, it is tried directly and the PATH is not consulted.
 // The result may be an absolute path or a path relative to the current directory.
-func LookPath2(file string, lenv Env) (string, error) {
+func LookPath2(file string, env map[string]string) (string, error) {
 	// NOTE(rsc): I wish we could use the Plan 9 behavior here
 	// (only bypass the path if file begins with / or ./ or ../)
 	// but that would not match all the Unix shells.
@@ -45,7 +45,7 @@ func LookPath2(file string, lenv Env) (string, error) {
 		}
 		return "", &Error{file, err}
 	}
-	path := lenv.Getenv("PATH")
+	path := getenv(env, "PATH")
 	for _, dir := range filepath.SplitList(path) {
 		if dir == "" {
 			// Unix shell semantics: path element "" means "."

act/lookpath/lp_unix_test.go

@@ -13,12 +13,6 @@ import (
 	"testing"
 )
 
-type testEnv map[string]string
-
-func (e testEnv) Getenv(name string) string {
-	return e[name]
-}
-
 func TestLookPath2SearchesPathAndEmptyElement(t *testing.T) {
 	dir := t.TempDir()
 	exe := filepath.Join(dir, "tool")
@@ -26,7 +20,7 @@ func TestLookPath2SearchesPathAndEmptyElement(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	got, err := LookPath2("tool", testEnv{"PATH": string(filepath.ListSeparator) + dir})
+	got, err := LookPath2("tool", map[string]string{"PATH": string(filepath.ListSeparator) + dir})
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -42,7 +36,7 @@ func TestLookPath2DirectPathDoesNotSearchPath(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	got, err := LookPath2(exe, testEnv{"PATH": ""})
+	got, err := LookPath2(exe, map[string]string{"PATH": ""})
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -58,7 +52,7 @@ func TestLookPath2ReportsPermissionAndNotFound(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	_, err := LookPath2(file, testEnv{"PATH": dir})
+	_, err := LookPath2(file, map[string]string{"PATH": dir})
 	var pathErr *Error
 	if !errors.As(err, &pathErr) || !errors.Is(pathErr.Err, fs.ErrPermission) {
 		t.Fatalf("LookPath2(non-executable) error = %v, want fs.ErrPermission wrapped in *Error", err)
@@ -67,7 +61,7 @@ func TestLookPath2ReportsPermissionAndNotFound(t *testing.T) {
 		t.Fatalf("Error() = %q, want %q", pathErr.Error(), fs.ErrPermission.Error())
 	}
 
-	_, err = LookPath2("missing", testEnv{"PATH": dir})
+	_, err = LookPath2("missing", map[string]string{"PATH": dir})
 	if !errors.As(err, &pathErr) || !errors.Is(pathErr.Err, ErrNotFound) {
 		t.Fatalf("LookPath2(missing) error = %v, want ErrNotFound wrapped in *Error", err)
 	}

act/lookpath/lp_windows.go

@@ -58,9 +58,9 @@ func findExecutable(file string, exts []string) (string, error) {
 // LookPath also uses PATHEXT environment variable to match
 // a suitable candidate.
 // The result may be an absolute path or a path relative to the current directory.
-func LookPath2(file string, lenv Env) (string, error) {
+func LookPath2(file string, env map[string]string) (string, error) {
 	var exts []string
-	x := lenv.Getenv(`PATHEXT`)
+	x := getenv(env, `PATHEXT`)
 	if x != "" {
 		for e := range strings.SplitSeq(strings.ToLower(x), `;`) {
 			if e == "" {
@@ -85,7 +85,7 @@ func LookPath2(file string, lenv Env) (string, error) {
 	if f, err := findExecutable(filepath.Join(".", file), exts); err == nil {
 		return f, nil
 	}
-	path := lenv.Getenv("path")
+	path := getenv(env, "path")
 	for _, dir := range filepath.SplitList(path) {
 		if f, err := findExecutable(filepath.Join(dir, file), exts); err == nil {
 			return f, nil

act/runner/action.go

@@ -20,9 +20,9 @@ import (
 	"runtime"
 	"strings"
 
-	"gitea.com/gitea/runner/internal/act/common"
-	"gitea.com/gitea/runner/internal/act/common/git"
-	"gitea.com/gitea/runner/internal/act/container"
+	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/act/common/git"
+	"gitea.com/gitea/runner/act/container"
 
 	"gitea.dev/actionslib/pkg/model"
 	"github.com/kballard/go-shellquote"
@@ -124,21 +124,9 @@ func readActionImpl(ctx context.Context, step *model.Step, actionDir, actionPath
 	defer closer.Close()
 
 	action, err := model.ReadAction(reader)
-	// For Gitea, reduce log noise
-	// logger.Debugf("Read action %v from '%s'", action, "Unknown")
 	return action, err
 }
 
-// cachedActionTar returns the action's tree from the action cache, which only a remote action
-// has an entry in.
-func cachedActionTar(ctx context.Context, step actionStep, name, includePrefix string) (io.ReadCloser, error) {
-	remote, ok := step.(*stepActionRemote)
-	if !ok {
-		return nil, fmt.Errorf("action %q is a remote action but runs as %T", name, step)
-	}
-	return step.getRunContext().Config.ActionCache.GetTarArchive(ctx, remote.cacheDir, remote.resolvedSha, includePrefix)
-}
-
 func maybeCopyToActionDir(ctx context.Context, step actionStep, actionDir, actionPath, containerActionDir string) error {
 	logger := common.Logger(ctx)
 	rc := step.getRunContext()
@@ -148,25 +136,20 @@ func maybeCopyToActionDir(ctx context.Context, step actionStep, actionDir, actio
 		return nil
 	}
 
-	var containerActionDirCopy string
-	containerActionDirCopy = strings.TrimSuffix(containerActionDir, actionPath)
+	containerActionDirCopy := strings.TrimSuffix(containerActionDir, actionPath)
 	logger.Debug(containerActionDirCopy)
 
 	if !strings.HasSuffix(containerActionDirCopy, `/`) {
 		containerActionDirCopy += `/`
 	}
 
-	if rc.Config != nil && rc.Config.ActionCache != nil {
-		ta, err := cachedActionTar(ctx, step, stepModel.Uses, "")
-		if err != nil {
-			return err
-		}
-		defer ta.Close()
-		return rc.JobContainer.CopyTarStream(ctx, containerActionDirCopy, ta)
-	}
-
 	defer git.AcquireCloneLock(actionDir)()
 
+	if !rc.Config.NoActionPatch {
+		// A concurrent job's prepare resets this directory, so patch under the copy's lock.
+		patchActions(ctx, actionScriptPaths(filepath.Join(actionDir, actionPath), step.getActionModel()))
+	}
+
 	if err := removeGitIgnore(ctx, actionDir); err != nil {
 		return err
 	}
@@ -186,13 +169,10 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction
 		}
 
 		action := step.getActionModel()
-		// For Gitea, reduce log noise
-		// logger.Debugf("About to run action %v", action)
 
-		err := setupActionEnv(ctx, step, remoteAction)
-		if err != nil {
-			return err
-		}
+		rc.withGithubEnv(ctx, step.getGithubContext(ctx), *step.getEnv())
+		populateEnvsFromSavedState(step.getEnv(), step, rc)
+		populateEnvsFromInput(ctx, step.getEnv(), action, rc)
 
 		actionLocation := path.Join(actionDir, actionPath)
 		actionName, containerActionDir := getContainerActionPaths(stepModel, actionLocation, rc)
@@ -210,7 +190,7 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction
 
 			rc.ApplyExtraPath(ctx, step.getEnv())
 
-			return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx)
+			return rc.JobContainer.Exec(containerArgs, *step.getEnv(), "", "")(ctx)
 		case x.IsDocker():
 			location := actionLocation
 			if remoteAction == nil {
@@ -235,11 +215,11 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction
 			execArgs := []string{filepath.Join(containerActionDir, execFileName)}
 
 			return common.NewPipelineExecutor(
-				rc.execJobContainer(buildArgs, *step.getEnv(), "", containerActionDir),
-				rc.execJobContainer(execArgs, *step.getEnv(), "", ""),
+				rc.JobContainer.Exec(buildArgs, *step.getEnv(), "", containerActionDir),
+				rc.JobContainer.Exec(execArgs, *step.getEnv(), "", ""),
 			)(ctx)
 		default:
-			return fmt.Errorf("The runs.using key must be one of: %v, got %s", []string{
+			return fmt.Errorf("the runs.using key must be one of: %v, got %s", []string{
 				model.ActionRunsUsingDocker,
 				model.ActionRunsUsingNode12,
 				model.ActionRunsUsingNode16,
@@ -252,20 +232,6 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction
 	}
 }
 
-func setupActionEnv(ctx context.Context, step actionStep, _ *remoteAction) error {
-	rc := step.getRunContext()
-
-	// A few fields in the environment (e.g. GITHUB_ACTION_REPOSITORY)
-	// are dependent on the action. That means we can complete the
-	// setup only after resolving the whole action model and cloning
-	// the action
-	rc.withGithubEnv(ctx, step.getGithubContext(ctx), *step.getEnv())
-	populateEnvsFromSavedState(step.getEnv(), step, rc)
-	populateEnvsFromInput(ctx, step.getEnv(), step.getActionModel(), rc)
-
-	return nil
-}
-
 // https://github.com/nektos/act/issues/228#issuecomment-629709055
 // files in .gitignore are not copied in a Docker container
 // this causes issues with actions that ignore other important resources
@@ -293,9 +259,7 @@ func removeGitIgnore(ctx context.Context, directory string) error {
 // same `act-dockeraction:latest` image on a shared docker daemon. A subsequent
 // repository would then silently run the image built for an earlier one.
 // Including the repository keeps the tag stable for caching within a repository
-// while preventing cross-repository collisions. A remote action needs the same
-// treatment, because its actionName is the shared checkout of its repository and
-// ref plus the action's path inside it.
+// while preventing cross-repository collisions.
 // See https://gitea.com/gitea/runner/issues/1039.
 func dockerActionImageTag(repository, actionName string, localAction bool) string {
 	name := actionName
@@ -304,10 +268,11 @@ func dockerActionImageTag(repository, actionName string, localAction bool) strin
 	}
 	// The human-readable name is sanitized by collapsing every non-alphanumeric character to "-".
 	sanitized := regexp.MustCompile("[^a-zA-Z0-9]").ReplaceAllString(name, "-")
-	// Sanitizing is lossy, so a short hash of the raw repository and action path is appended, keeping
-	// the tag unique per repository and per action inside it.
-	sum := sha256.Sum256([]byte(repository + "\x00" + actionName))
-	sanitized += "-" + hex.EncodeToString(sum[:])[:12]
+	if localAction {
+		// For local actions a short hash of the raw repository and action path is appended so the tag stays unique per repository.
+		sum := sha256.Sum256([]byte(repository + "\x00" + actionName))
+		sanitized += "-" + hex.EncodeToString(sum[:])[:12]
+	}
 	// "-dockeraction" ensures that "./", "./test " won't get converted to "act-:latest", "act-test-:latest" which are invalid docker image names
 	image := fmt.Sprintf("%s-dockeraction:%s", sanitized, "latest")
 	image = "act-" + strings.TrimLeft(image, "-")
@@ -360,12 +325,6 @@ func execAsDocker(ctx context.Context, step actionStep, actionName, actionDir, b
 					return err
 				}
 				defer buildContext.Close()
-			} else if rc.Config.ActionCache != nil {
-				buildContext, err = cachedActionTar(ctx, step, actionName, contextDir)
-				if err != nil {
-					return err
-				}
-				defer buildContext.Close()
 			}
 			prepImage = ContainerNewDockerBuildExecutor(container.NewDockerBuildExecutorInput{
 				ContextDir:   contextDir,
@@ -400,21 +359,19 @@ func execAsDocker(ctx context.Context, step actionStep, actionName, actionDir, b
 	if err != nil {
 		return err
 	}
-	stepContainer := newStepContainer(ctx, step, image, cmd, entrypoint)
+	stepContainer := newStepContainer(ctx, step, image, cmd, entrypoint, rc.Config.ContainerOptions)
 	return common.NewPipelineExecutor(
 		prepImage,
 		stepContainer.Pull(forcePull),
-		stepContainer.Remove().IfBool(!rc.Config.ReuseContainers),
+		stepContainer.Remove(),
 		stepContainer.Create(rc.Config.ContainerCapAdd, rc.Config.ContainerCapDrop),
 		stepContainer.Start(true),
-	).Finally(
-		stepContainer.Remove().IfBool(!rc.Config.ReuseContainers && !rc.Config.AutoRemove),
 	).Finally(stepContainer.Close())(ctx)
 }
 
 // dockerEntrypoint returns the entrypoint the action's image runs with for the given
 // stage. Only the main stage honours the `entrypoint` input.
-func dockerEntrypoint(ctx context.Context, step actionStep, eval ExpressionEvaluator, stage stepStage) ([]string, error) {
+func dockerEntrypoint(ctx context.Context, step actionStep, eval *expressionEvaluator, stage stepStage) ([]string, error) {
 	runs := step.getActionModel().Runs
 
 	var entrypoint string
@@ -465,18 +422,9 @@ func evalDockerArgs(ctx context.Context, step step, action *model.Action, cmd *[
 	}
 }
 
-func newStepContainer(ctx context.Context, step step, image string, cmd, entrypoint []string) container.Container {
+func newStepContainer(ctx context.Context, step step, image string, cmd, entrypoint []string, runnerOptions string) container.Container {
 	rc := step.getRunContext()
-	stepModel := step.getStepModel()
-	rawLogger := common.Logger(ctx).WithField("raw_output", true)
-	logWriter := common.NewLineWriter(rc.commandHandler(ctx), func(s string) bool {
-		if rc.Config.LogOutput {
-			rawLogger.Infof("%s", s)
-		} else {
-			rawLogger.Debugf("%s", s)
-		}
-		return true
-	})
+	logWriter := rc.commandLogWriter(ctx)
 	envList := make([]string, 0)
 	for k, v := range *step.getEnv() {
 		envList = append(envList, fmt.Sprintf("%s=%s", k, v))
@@ -489,27 +437,26 @@ func newStepContainer(ctx context.Context, step step, image string, cmd, entrypo
 	if rc.IsHostEnv(ctx) {
 		networkMode = "default"
 	}
-	stepContainer := ContainerNewContainer(&container.NewContainerInput{
-		Cmd:          cmd,
-		Entrypoint:   entrypoint,
-		WorkingDir:   rc.JobContainer.ToContainerPath(rc.Config.Workdir),
-		Image:        image,
-		Name:         createContainerName(rc.jobContainerName(), "STEP-"+stepModel.ID),
-		Env:          envList,
-		Mounts:       mounts,
-		NetworkMode:  networkMode,
-		Binds:        binds,
-		Stdout:       logWriter,
-		Stderr:       logWriter,
-		Privileged:   rc.Config.Privileged,
-		UsernsMode:   rc.Config.UsernsMode,
-		Platform:     rc.Config.ContainerArchitecture,
-		Options:      rc.Config.ContainerOptions,
-		AutoRemove:   rc.Config.AutoRemove,
-		ValidVolumes: rc.validVolumes(),
-		AllocatePTY:  rc.Config.AllocatePTY,
+	return ContainerNewContainer(&container.NewContainerInput{
+		Cmd:           cmd,
+		Entrypoint:    entrypoint,
+		WorkingDir:    rc.JobContainer.ToContainerPath(rc.Config.Workdir),
+		Image:         image,
+		Name:          createContainerName(rc.jobContainerName(), "STEP-"+step.getStepModel().ID),
+		Env:           envList,
+		Mounts:        mounts,
+		NetworkMode:   networkMode,
+		Binds:         binds,
+		Stdout:        logWriter,
+		Stderr:        logWriter,
+		Privileged:    rc.Config.Privileged,
+		UsernsMode:    rc.Config.UsernsMode,
+		Platform:      rc.Config.ContainerArchitecture,
+		RunnerOptions: runnerOptions,
+		AutoRemove:    true,
+		ValidVolumes:  rc.validVolumes(),
+		AllocatePTY:   rc.Config.AllocatePTY,
 	})
-	return stepContainer
 }
 
 func populateEnvsFromSavedState(env *map[string]string, step actionStep, rc *RunContext) {
@@ -644,7 +591,7 @@ func runPreStep(step actionStep) common.Executor {
 
 			rc.ApplyExtraPath(ctx, step.getEnv())
 
-			return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx)
+			return rc.JobContainer.Exec(containerArgs, *step.getEnv(), "", "")(ctx)
 
 		case x.IsDocker():
 			// defaults in pre steps were missing, however provided inputs are available
@@ -677,8 +624,8 @@ func runPreStep(step actionStep) common.Executor {
 			execArgs := []string{filepath.Join(containerActionDir, execFileName)}
 
 			return common.NewPipelineExecutor(
-				rc.execJobContainer(buildArgs, *step.getEnv(), "", containerActionDir),
-				rc.execJobContainer(execArgs, *step.getEnv(), "", ""),
+				rc.JobContainer.Exec(buildArgs, *step.getEnv(), "", containerActionDir),
+				rc.JobContainer.Exec(execArgs, *step.getEnv(), "", ""),
 			)(ctx)
 		default:
 			return nil
@@ -745,7 +692,7 @@ func runPostStep(step actionStep) common.Executor {
 
 			rc.ApplyExtraPath(ctx, step.getEnv())
 
-			return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx)
+			return rc.JobContainer.Exec(containerArgs, *step.getEnv(), "", "")(ctx)
 
 		case x.IsDocker():
 			populateEnvsFromSavedState(step.getEnv(), step, rc)
@@ -771,8 +718,8 @@ func runPostStep(step actionStep) common.Executor {
 			execArgs := []string{filepath.Join(containerActionDir, execFileName)}
 
 			return common.NewPipelineExecutor(
-				rc.execJobContainer(buildArgs, *step.getEnv(), "", containerActionDir),
-				rc.execJobContainer(execArgs, *step.getEnv(), "", ""),
+				rc.JobContainer.Exec(buildArgs, *step.getEnv(), "", containerActionDir),
+				rc.JobContainer.Exec(execArgs, *step.getEnv(), "", ""),
 			)(ctx)
 
 		default:

act/runner/action_composite.go

@@ -12,7 +12,7 @@ import (
 	"strconv"
 	"strings"
 
-	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/act/common"
 
 	"gitea.dev/actionslib/pkg/model"
 )
@@ -55,7 +55,7 @@ func newCompositeRunContext(ctx context.Context, parent *RunContext, step action
 	env := evaluateCompositeInputAndEnv(ctx, parent, step)
 
 	// run with the global config but without secrets
-	configCopy := *(parent.Config)
+	configCopy := *parent.Config
 	configCopy.Secrets = nil
 
 	// create a run context for the composite action to run in
@@ -181,20 +181,7 @@ func (rc *RunContext) compositeExecutor(action *model.Action) *compositeSteps {
 		stepPre := rc.newCompositeCommandExecutor(step.pre())
 		preSteps = append(preSteps, newCompositeStepLogExecutor(stepPre, stepID))
 
-		steps = append(steps, func(ctx context.Context) error {
-			ctx = WithCompositeStepLogger(ctx, stepID)
-			logger := common.Logger(ctx)
-			err := rc.newCompositeCommandExecutor(step.main())(ctx)
-
-			if err != nil {
-				logger.Errorf("##[error]%s", EscapeCommandData(err.Error()))
-				common.SetJobError(ctx, err)
-			} else if ctx.Err() != nil {
-				logger.Errorf("##[error]%s", EscapeCommandData(ctx.Err().Error()))
-				common.SetJobError(ctx, ctx.Err())
-			}
-			return nil
-		})
+		steps = append(steps, newCompositeStepLogExecutor(rc.newCompositeCommandExecutor(step.main()), stepID))
 
 		// run the post executor in reverse order
 		if postExecutor != nil {
@@ -222,19 +209,7 @@ func (rc *RunContext) newCompositeCommandExecutor(executor common.Executor) comm
 	return func(ctx context.Context) error {
 		ctx = WithCompositeLogger(ctx, &rc.Masks)
 
-		// We need to inject a composite RunContext related command
-		// handler into the current running job container
-		// We need this, to support scoping commands to the composite action
-		// executing.
-		rawLogger := common.Logger(ctx).WithField("raw_output", true)
-		logWriter := common.NewLineWriter(rc.commandHandler(ctx), func(s string) bool {
-			if rc.Config.LogOutput {
-				rawLogger.Infof("%s", s)
-			} else {
-				rawLogger.Debugf("%s", s)
-			}
-			return true
-		})
+		logWriter := rc.commandLogWriter(ctx)
 
 		oldout, olderr := rc.JobContainer.ReplaceLogWriter(logWriter, logWriter)
 		defer rc.JobContainer.ReplaceLogWriter(oldout, olderr)

act/runner/action_test.go

@@ -13,9 +13,9 @@ import (
 	"testing"
 	"time"
 
-	"gitea.com/gitea/runner/internal/act/common"
-	"gitea.com/gitea/runner/internal/act/common/git"
-	"gitea.com/gitea/runner/internal/act/container"
+	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/act/common/git"
+	"gitea.com/gitea/runner/act/container"
 
 	"gitea.dev/actionslib/pkg/model"
 	"github.com/stretchr/testify/assert"
@@ -23,12 +23,10 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-type closerMock struct {
-	mock.Mock
-}
+type closerFunc func()
 
-func (m *closerMock) Close() error {
-	m.Called()
+func (close closerFunc) Close() error {
+	close()
 	return nil
 }
 
@@ -39,6 +37,15 @@ runs:
 	using: 'node16'
 	main: 'main.js'
 `, "\t", "  ")
+	yamlAction := &model.Action{
+		Name: "name",
+		Runs: model.ActionRuns{
+			Using:  "node16",
+			Main:   "main.js",
+			PreIf:  "always()",
+			PostIf: "always()",
+		},
+	}
 
 	table := []struct {
 		name        string
@@ -52,30 +59,14 @@ runs:
 			step:        &model.Step{},
 			filename:    "action.yml",
 			fileContent: yaml,
-			expected: &model.Action{
-				Name: "name",
-				Runs: model.ActionRuns{
-					Using:  "node16",
-					Main:   "main.js",
-					PreIf:  "always()",
-					PostIf: "always()",
-				},
-			},
+			expected:    yamlAction,
 		},
 		{
 			name:        "readActionYaml",
 			step:        &model.Step{},
 			filename:    "action.yaml",
 			fileContent: yaml,
-			expected: &model.Action{
-				Name: "name",
-				Runs: model.ActionRuns{
-					Using:  "node16",
-					Main:   "main.js",
-					PreIf:  "always()",
-					PostIf: "always()",
-				},
-			},
+			expected:    yamlAction,
 		},
 		{
 			name:        "readDockerfile",
@@ -121,14 +112,14 @@ runs:
 
 	for _, tt := range table {
 		t.Run(tt.name, func(t *testing.T) {
-			closerMock := &closerMock{}
+			closed := false
 
 			readFile := func(filename string) (io.Reader, io.Closer, error) {
 				if tt.filename != filename {
 					return nil, nil, fs.ErrNotExist
 				}
 
-				return strings.NewReader(tt.fileContent), closerMock, nil
+				return strings.NewReader(tt.fileContent), closerFunc(func() { closed = true }), nil
 			}
 
 			writeFile := func(filename string, data []byte, perm fs.FileMode) error {
@@ -137,58 +128,16 @@ runs:
 				return nil
 			}
 
-			if tt.filename != "" {
-				closerMock.On("Close")
-			}
-
 			action, err := readActionImpl(context.Background(), tt.step, "actionDir", "actionPath", readFile, writeFile)
 
 			assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act
 			assert.Equal(t, tt.expected, action)
 
-			closerMock.AssertExpectations(t)
+			assert.Equal(t, tt.filename != "", closed)
 		})
 	}
 }
 
-// With AutoRemove the daemon reaps the container on exit, so act must not remove it afterwards.
-func TestExecAsDockerAutoRemove(t *testing.T) {
-	orig := ContainerNewContainer
-	defer func() { ContainerNewContainer = orig }()
-
-	for _, tc := range []struct {
-		autoRemove bool
-		removes    int
-	}{
-		{false, 2}, // stale + post-run
-		{true, 1},  // post-run skipped
-	} {
-		cm := &containerMock{}
-		ContainerNewContainer = func(*container.NewContainerInput) container.ExecutionsEnvironment { return cm }
-
-		step := &stepActionRemote{
-			Step: &model.Step{ID: "1", Uses: "org/action@v1"},
-			RunContext: &RunContext{
-				Config:       &Config{AutoRemove: tc.autoRemove},
-				Run:          &model.Run{JobID: "1", Workflow: &model.Workflow{Jobs: map[string]*model.Job{"1": {}}}},
-				JobContainer: cm,
-			},
-			action: &model.Action{Runs: model.ActionRuns{Using: "docker", Image: "docker://node:14"}},
-		}
-
-		removes := 0
-		cm.On("Pull", false).Return(func(context.Context) error { return nil })
-		cm.On("Remove").Return(func(context.Context) error { removes++; return nil })
-		cm.On("Create", []string(nil), []string(nil)).Return(func(context.Context) error { return nil })
-		cm.On("Start", true).Return(func(context.Context) error { return nil })
-		cm.On("Close").Return(func(context.Context) error { return nil })
-
-		require.NoError(t, execAsDocker(context.Background(), step, "action", t.TempDir(), t.TempDir(), false, stepStageMain))
-		cm.AssertExpectations(t)
-		assert.Equal(t, tc.removes, removes)
-	}
-}
-
 func TestActionRunner(t *testing.T) {
 	table := []struct {
 		name        string
@@ -337,11 +286,12 @@ func TestNewStepContainerDoesNotUseDockerSecrets(t *testing.T) {
 	step.On("getStepModel").Return(&model.Step{ID: "action"})
 	step.On("getEnv").Return(&env)
 
-	_ = newStepContainer(ctx, step, "registry.example.com/action:tag", nil, nil)
+	_ = 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)
+	assert.True(t, captured.AutoRemove)
 	step.AssertExpectations(t)
 }
 
@@ -496,11 +446,11 @@ func TestExecAsDockerHoldsCloneLockForRemoteUncached(t *testing.T) {
 }
 
 func TestDockerActionImageTag(t *testing.T) {
-	// A remote action's actionName is the checkout of its repository and ref plus its path inside it,
-	// and paths that sanitize alike share the readable prefix, so siblings must stay apart.
-	assert.NotEqual(t,
-		dockerActionImageTag("owner/repo", "abc123/a-b", false),
-		dockerActionImageTag("owner/repo", "abc123/a_b", false),
+	// Remote actions already carry a unique, ref-scoped actionName (the uses
+	// hash), so the tag must be left untouched for backwards compatibility.
+	assert.Equal(t,
+		"act-abc123-dockeraction:latest",
+		dockerActionImageTag("owner/repo", "abc123", false),
 	)
 
 	// Local actions keep a human-readable, repository-namespaced prefix and gain a short hash suffix that makes the tag unique per (repository, actionName).

act/runner/cancellation_test.go

@@ -8,7 +8,7 @@ import (
 	"testing"
 	"time"
 
-	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/act/common"
 
 	"gitea.dev/actionslib/pkg/exprparser"
 	"gitea.dev/actionslib/pkg/model"
@@ -107,17 +107,14 @@ func TestMainStepsExecutorMarksFailedOnTimeoutBetweenSteps(t *testing.T) {
 		"job1": createJob(t, `runs-on: ubuntu-latest`, ""),
 	})
 
-	// A short deadline that we let elapse between steps, so no step records the error itself.
-	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Millisecond)
-	defer cancel()
+	ctx := newControllableDeadlineContext(context.Background())
 
 	var ran []string
 	var laterStepCtxErr error
 	steps := []common.Executor{
-		func(c context.Context) error {
+		func(context.Context) error {
 			ran = append(ran, "step1")
-			// Block until the job deadline elapses, then return cleanly: the interrupt lands in the loop's between-steps check, not inside a step.
-			<-c.Done()
+			ctx.expire()
 			return nil
 		},
 		func(c context.Context) error {

act/runner/command.go

@@ -10,7 +10,7 @@ import (
 	"regexp"
 	"strings"
 
-	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/act/common"
 )
 
 var commandPatternGA *regexp.Regexp
@@ -163,10 +163,6 @@ func (rc *RunContext) setOutput(ctx context.Context, kvPairs map[string]string,
 	logger := common.Logger(ctx)
 	stepID := rc.CurrentStep
 	outputName := kvPairs["name"]
-	if outputMapping, ok := rc.OutputMappings[MappableOutput{StepID: stepID, OutputName: outputName}]; ok {
-		stepID = outputMapping.StepID
-		outputName = outputMapping.OutputName
-	}
 
 	result, ok := rc.StepResults[stepID]
 	if !ok {

act/runner/command_test.go

@@ -11,7 +11,7 @@ import (
 	"os"
 	"testing"
 
-	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/act/common"
 
 	"gitea.dev/actionslib/pkg/model"
 	"github.com/sirupsen/logrus/hooks/test"

act/runner/container_mock_test.go

@@ -8,12 +8,14 @@ import (
 	"context"
 	"io"
 
-	"gitea.com/gitea/runner/internal/act/common"
-	"gitea.com/gitea/runner/internal/act/container"
+	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/act/container"
 
 	"github.com/stretchr/testify/mock"
 )
 
+var noopExecutor = func(context.Context) error { return nil }
+
 type containerMock struct {
 	mock.Mock
 	container.Container
@@ -50,11 +52,6 @@ func (cm *containerMock) UpdateFromEnv(srcPath string, env *map[string]string) c
 	return args.Get(0).(func(context.Context) error)
 }
 
-func (cm *containerMock) UpdateFromImageEnv(env *map[string]string) common.Executor {
-	args := cm.Called(env)
-	return args.Get(0).(func(context.Context) error)
-}
-
 func (cm *containerMock) Copy(destPath string, files ...*container.FileEntry) common.Executor {
 	args := cm.Called(destPath, files)
 	return args.Get(0).(func(context.Context) error)

act/runner/expression.go

@@ -15,8 +15,8 @@ import (
 	"strings"
 	"time"
 
-	"gitea.com/gitea/runner/internal/act/common"
-	"gitea.com/gitea/runner/internal/act/container"
+	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/act/container"
 
 	_ "embed"
 
@@ -26,20 +26,12 @@ import (
 	"go.yaml.in/yaml/v4"
 )
 
-// ExpressionEvaluator is the interface for evaluating expressions
-type ExpressionEvaluator interface {
-	evaluate(context.Context, string, exprparser.DefaultStatusCheck) (any, error)
-	interpolate(context.Context, string) (string, error)
-	EvaluateYamlNode(context.Context, *yaml.Node) error
-	Interpolate(context.Context, string) string
-}
-
 // NewExpressionEvaluator creates a new evaluator
-func (rc *RunContext) NewExpressionEvaluator(ctx context.Context) ExpressionEvaluator {
+func (rc *RunContext) NewExpressionEvaluator(ctx context.Context) *ExpressionEvaluator {
 	return rc.NewExpressionEvaluatorWithEnv(ctx, rc.GetEnv())
 }
 
-func (rc *RunContext) NewExpressionEvaluatorWithEnv(ctx context.Context, env map[string]string) ExpressionEvaluator {
+func (rc *RunContext) NewExpressionEvaluatorWithEnv(ctx context.Context, env map[string]string) *ExpressionEvaluator {
 	var workflowCallResult map[string]*model.WorkflowCallResult
 
 	// todo: cleanup EvaluationEnvironment creation
@@ -98,7 +90,7 @@ func (rc *RunContext) NewExpressionEvaluatorWithEnv(ctx context.Context, env map
 		HashFiles: getHashFilesFunction(ctx, rc),
 	}
 	ee.Runner = rc.getRunnerContext(ctx)
-	return expressionEvaluator{
+	return &expressionEvaluator{
 		interpreter: exprparser.NewInterpeter(ee, exprparser.Config{
 			Run:        rc.Run,
 			WorkingDir: rc.Config.Workdir,
@@ -111,7 +103,7 @@ func (rc *RunContext) NewExpressionEvaluatorWithEnv(ctx context.Context, env map
 var hashfiles string
 
 // NewStepExpressionEvaluator creates a new evaluator
-func (rc *RunContext) NewStepExpressionEvaluator(ctx context.Context, step step) ExpressionEvaluator {
+func (rc *RunContext) NewStepExpressionEvaluator(ctx context.Context, step step) *ExpressionEvaluator {
 	// todo: cleanup EvaluationEnvironment creation
 	job := rc.Run.Job()
 	strategy := make(map[string]any)
@@ -150,7 +142,7 @@ func (rc *RunContext) NewStepExpressionEvaluator(ctx context.Context, step step)
 		HashFiles: getHashFilesFunction(ctx, rc),
 	}
 	ee.Runner = rc.getRunnerContext(ctx)
-	return expressionEvaluator{
+	return &expressionEvaluator{
 		interpreter: exprparser.NewInterpeter(ee, exprparser.Config{
 			Run:        rc.Run,
 			WorkingDir: rc.Config.Workdir,
@@ -178,7 +170,7 @@ func getHashFilesFunction(ctx context.Context, rc *RunContext) func(v []reflect.
 							followSymlink = true
 							continue
 						}
-						return "", fmt.Errorf("Invalid glob option %s, available option: '--follow-symbolic-links'", s)
+						return "", fmt.Errorf("invalid glob option %s, available option: '--follow-symbolic-links'", s)
 					}
 				}
 				patterns = append(patterns, s)
@@ -196,7 +188,7 @@ func getHashFilesFunction(ctx context.Context, rc *RunContext) func(v []reflect.
 				Mode: 0o644,
 				Body: hashfiles,
 			}).
-				Then(rc.execJobContainer([]string{"node", path.Join(rc.JobContainer.GetActPath(), name)},
+				Then(rc.JobContainer.Exec([]string{"node", path.Join(rc.JobContainer.GetActPath(), name)},
 					env, "", "")).
 				Finally(func(context.Context) error {
 					rc.JobContainer.ReplaceLogWriter(stdout, stderr)
@@ -222,6 +214,8 @@ type expressionEvaluator struct {
 	interpreter exprparser.Interpreter
 }
 
+type ExpressionEvaluator = expressionEvaluator
+
 func (ee expressionEvaluator) evaluate(ctx context.Context, in string, defaultStatusCheck exprparser.DefaultStatusCheck) (any, error) {
 	logger := common.Logger(ctx)
 	logger.Debugf("evaluating expression '%s'", in)
@@ -261,7 +255,7 @@ func (ee expressionEvaluator) interpolate(ctx context.Context, in string) (strin
 
 // EvalBool evaluates an expression against given evaluator. An `if:` is an expression even without
 // `${{ }}`, while literal text around one makes the whole value a string.
-func EvalBool(ctx context.Context, evaluator ExpressionEvaluator, expr string, defaultStatusCheck exprparser.DefaultStatusCheck) (bool, error) {
+func EvalBool(ctx context.Context, evaluator *expressionEvaluator, expr string, defaultStatusCheck exprparser.DefaultStatusCheck) (bool, error) {
 	return expreval.New(func(in string, dsc exprparser.DefaultStatusCheck) (any, error) {
 		return evaluator.evaluate(ctx, in, dsc)
 	}).EvalBool(expr, defaultStatusCheck)

act/runner/expression_test.go

@@ -356,3 +356,25 @@ on:
 		}
 	}
 }
+
+func TestJobNameMasksSecrets(t *testing.T) {
+	workflow, err := model.ReadWorkflow(strings.NewReader(`
+jobs:
+  a:
+    name: deploy ${{ secrets.A }}
+  b:
+    name: deploy ${{ secrets.B }}
+`))
+	require.NoError(t, err)
+
+	runner := &runnerImpl{config: &Config{Secrets: map[string]string{"A": "s3cr3t-a", "B": "s3cr3t-b"}}}
+	containerName := func(jobID string) string {
+		rc := runner.newRunContext(t.Context(), &model.Run{JobID: jobID, Workflow: workflow}, nil)
+		assert.NotContains(t, rc.Name, "s3cr3t")
+		return rc.jobContainerName()
+	}
+
+	a, b := containerName("a"), containerName("b")
+	assert.NotContains(t, a, "s3cr3t") // it reaches the container name, which no log masker covers
+	assert.NotEqual(t, a, b)           // masking the name must not collapse two jobs onto one container
+}

act/runner/helpers_test.go

@@ -9,7 +9,7 @@ import (
 	"runtime"
 	"testing"
 
-	"gitea.com/gitea/runner/internal/act/container"
+	"gitea.com/gitea/runner/act/container"
 
 	mobyclient "github.com/moby/moby/client"
 )

act/runner/job_executor.go

@@ -9,7 +9,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/base64"
-	"encoding/json"
+	"encoding/json/v2"
 	"errors"
 	"fmt"
 	"io"
@@ -21,8 +21,8 @@ import (
 	"time"
 	"unicode"
 
-	"gitea.com/gitea/runner/internal/act/common"
-	"gitea.com/gitea/runner/internal/act/container"
+	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/act/container"
 
 	"gitea.dev/actionslib/pkg/exprparser"
 	"gitea.dev/actionslib/pkg/model"
@@ -240,43 +240,15 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo
 	postExecutor = postExecutor.Finally(func(ctx context.Context) error {
 		jobError := common.JobError(ctx)
 		var err error
-		// jobError == nil keeps a failed job's container alive for post-mortem debugging when
-		// AutoRemove is off (the act-CLI --rm behavior; the shipped runner always sets
-		// AutoRemove). A cancelled run is not a failure to inspect, and the cancel-path post
-		// context now carries its own error container so a failing post step makes jobError
-		// non-nil — OR in rc.jobCancelled so cancellation still always tears the container down.
-		if rc.Config.AutoRemove || jobError == nil || rc.jobCancelled {
-			// always allow 1 min for stopping and removing the runner, even if we were cancelled
-			ctx, cancel := context.WithTimeout(common.WithLogger(context.Background(), common.Logger(ctx)), time.Minute)
-			defer cancel()
+		// always allow 1 min for stopping and removing the runner, even if we were cancelled
+		ctx, cancel := context.WithTimeout(common.WithLogger(context.Background(), common.Logger(ctx)), time.Minute)
+		defer cancel()
 
-			logger := common.Logger(ctx)
-			tryUploadJobSummary(ctx, rc)
-			// For Gitea
-			// We don't need to call `stopServiceContainers` here since it will be called by following `info.stopContainer`
-			// logger.Infof("Cleaning up services for job %s", rc.JobName)
-			// if err := rc.stopServiceContainers()(ctx); err != nil {
-			// 	logger.Errorf("Error while cleaning services: %v", err)
-			// }
-
-			logger.Infof("Cleaning up container for job %s", rc.JobName)
-			if err = info.stopContainer()(ctx); err != nil {
-				logger.Errorf("##[error]%s", EscapeCommandData("Error while stop job container: "+err.Error()))
-			}
-
-			// For Gitea
-			// We don't need to call `NewDockerNetworkRemoveExecutor` here since it is called by above `info.stopContainer`
-			// if !rc.IsHostEnv(ctx) && rc.Config.ContainerNetworkMode == "" {
-			// 	// clean network in docker mode only
-			// 	// if the value of `ContainerNetworkMode` is empty string,
-			// 	// it means that the network to which containers are connecting is created by `runner`,
-			// 	// so, we should remove the network at last.
-			// 	networkName, _ := rc.networkName()
-			// 	logger.Infof("Cleaning up network for job %s, and network name is: %s", rc.JobName, networkName)
-			// 	if err := container.NewDockerNetworkRemoveExecutor(networkName)(ctx); err != nil {
-			// 		logger.Errorf("Error while cleaning network: %v", err)
-			// 	}
-			// }
+		logger := common.Logger(ctx)
+		tryUploadJobSummary(ctx, rc)
+		logger.Infof("Cleaning up container for job %s", rc.JobName)
+		if err = info.stopContainer()(ctx); err != nil {
+			logger.Errorf("##[error]%s", EscapeCommandData("Error while stop job container: "+err.Error()))
 		}
 		setJobResult(ctx, info, rc, jobError == nil)
 		setJobOutputs(ctx, rc)
@@ -416,7 +388,7 @@ func setJobResult(ctx context.Context, info jobInfo, rc *RunContext, success boo
 
 	if rc.caller != nil {
 		// set reusable workflow job result
-		rc.caller.setReusedWorkflowJobResult(rc.JobName, jobResult) // For Gitea
+		rc.caller.setReusedWorkflowJobResult(rc.Run.JobID, jobResult) // For Gitea
 		return
 	}
 
@@ -515,7 +487,8 @@ func tryUploadJobSummary(ctx context.Context, rc *RunContext) {
 		if !ok || len(body) == 0 {
 			continue
 		}
-		uploadJobSummary(ctx, client, base+strconv.Itoa(i)+"/summary", runtimeToken, body)
+		// Gitea renders summaries on the run page, so mask before the upload.
+		uploadJobSummary(ctx, client, base+strconv.Itoa(i)+"/summary", runtimeToken, []byte(rc.maskSecrets(string(body))))
 	}
 }
 
@@ -651,15 +624,7 @@ func useStepLogger(rc *RunContext, stepModel *model.Step, stage stepStage, execu
 	return func(ctx context.Context) error {
 		ctx = withStepLogger(ctx, stepModel.Number, stepModel.ID, rc.ExprEval.Interpolate(ctx, stepModel.String()), stage.String())
 
-		rawLogger := common.Logger(ctx).WithField("raw_output", true)
-		logWriter := common.NewLineWriter(rc.commandHandler(ctx), func(s string) bool {
-			if rc.Config.LogOutput {
-				rawLogger.Infof("%s", s)
-			} else {
-				rawLogger.Debugf("%s", s)
-			}
-			return true
-		})
+		logWriter := rc.commandLogWriter(ctx)
 
 		oldout, olderr := rc.JobContainer.ReplaceLogWriter(logWriter, logWriter)
 		defer rc.JobContainer.ReplaceLogWriter(oldout, olderr)

act/runner/job_executor_test.go

@@ -20,8 +20,8 @@ import (
 	"testing"
 	"time"
 
-	"gitea.com/gitea/runner/internal/act/common"
-	"gitea.com/gitea/runner/internal/act/container"
+	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/act/container"
 
 	"gitea.dev/actionslib/pkg/model"
 	log "github.com/sirupsen/logrus"
@@ -146,8 +146,7 @@ func TestPrintPrepareActionsGolden(t *testing.T) {
 		&actionPreparerMock{reference: "actions/checkout@v7", sha: "9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0", ok: true},
 		// A resolved commit is best effort; the ref alone is reported when it is unknown.
 		&actionPreparerMock{reference: "actions/setup-go@v6", ok: true},
-		// A step that downloads nothing, such as the checkout of the workflow's own repository or a
-		// second step on an action the job already downloaded.
+		// A step that downloads nothing, such as the checkout of the workflow's own repository.
 		&actionPreparerMock{ok: false},
 	}
 	require.NoError(t, printPrepareActions(&RunContext{}, preparers)(ctx))
@@ -337,6 +336,7 @@ func TestNewJobExecutor(t *testing.T) {
 			executedSteps: []string{
 				"startContainer",
 				"step1",
+				"stopContainer",
 				"interpolateOutputs",
 				"closeContainer",
 			},
@@ -528,18 +528,39 @@ func TestNewJobExecutor(t *testing.T) {
 	}
 }
 
+type controllableDeadlineContext struct {
+	context.Context
+	done chan struct{}
+}
+
+func newControllableDeadlineContext(parent context.Context) *controllableDeadlineContext {
+	return &controllableDeadlineContext{Context: parent, done: make(chan struct{})}
+}
+
+func (ctx *controllableDeadlineContext) Done() <-chan struct{} {
+	return ctx.done
+}
+
+func (ctx *controllableDeadlineContext) Err() error {
+	select {
+	case <-ctx.done:
+		return context.DeadlineExceeded
+	default:
+		return nil
+	}
+}
+
+func (ctx *controllableDeadlineContext) expire() {
+	close(ctx.done)
+}
+
 // TestNewJobExecutorRunsPostStepsAfterTimeout guards the timeout-minutes cleanup
 // path: when a job exceeds its timeout the job context is DeadlineExceeded, but
 // the post steps (cleanup hooks like actions/checkout post and cache save) must
 // still run against a fresh, non-expired context, and the job must still be
 // reported as failed.
 func TestNewJobExecutorRunsPostStepsAfterTimeout(t *testing.T) {
-	ctx := common.WithJobErrorContainer(context.Background())
-	// The timeout is generous so the main step (which blocks on ctx.Done below) is
-	// always reached before the deadline fires; otherwise the pipeline would
-	// short-circuit before the step runs and the job error would never be set.
-	ctx, cancel := context.WithTimeout(ctx, 200*time.Millisecond)
-	defer cancel()
+	ctx := newControllableDeadlineContext(common.WithJobErrorContainer(context.Background()))
 
 	jim := &jobInfoMock{}
 	sfm := &stepFactoryMock{}
@@ -563,19 +584,16 @@ func TestNewJobExecutorRunsPostStepsAfterTimeout(t *testing.T) {
 	jim.On("startContainer").Return(func(ctx context.Context) error { return nil })
 	jim.On("interpolateOutputs").Return(func(ctx context.Context) error { return nil })
 	jim.On("closeContainer").Return(func(ctx context.Context) error { return nil })
-	// The job timed out, so it must be reported as failed. stopContainer is left
-	// unexpected on purpose: a timed-out (failed) job preserves its error state, so
-	// the graceful stop is skipped exactly like any other failure without AutoRemove.
+	// The job timed out, so it must be reported as failed and still cleaned up.
+	jim.On("stopContainer").Return(func(context.Context) error { return nil })
 	jim.On("result", "failure")
 
 	sm := &stepMock{}
 	sfm.On("newStep", stepModel, rc).Return(sm, nil)
 	sm.On("pre").Return(func(ctx context.Context) error { return nil })
-	// The main step runs past the job timeout: it blocks until the job context is
-	// done, mirroring a step that overruns timeout-minutes.
-	sm.On("main").Return(func(ctx context.Context) error {
-		<-ctx.Done()
-		return ctx.Err()
+	sm.On("main").Return(func(stepCtx context.Context) error {
+		ctx.expire()
+		return stepCtx.Err()
 	})
 
 	var postRan bool
@@ -985,12 +1003,7 @@ func tarArchive(t *testing.T, entries ...tarEntry) []byte {
 
 func newTestRC(wf *model.Workflow, matrix map[string]any) *RunContext {
 	return &RunContext{
-		Config: &Config{
-			Workdir: ".",
-			Platforms: map[string]string{
-				"ubuntu-latest": "ubuntu-latest",
-			},
-		},
+		Config:      &Config{Workdir: ".", PlatformPicker: func([]string) string { return "ubuntu-latest" }},
 		StepResults: map[string]*model.StepResult{},
 		Env:         map[string]string{},
 		Matrix:      matrix,
@@ -1083,3 +1096,37 @@ func TestJobSetContinueOnError(t *testing.T) {
 		assert.True(t, j.ContinueOnError)
 	})
 }
+
+func TestTryUploadJobSummaryMasksSecrets(t *testing.T) {
+	var got string
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		body, err := io.ReadAll(r.Body)
+		assert.NoError(t, err)
+		got = string(body)
+		w.WriteHeader(http.StatusNoContent)
+	}))
+	defer server.Close()
+
+	cm := &containerMock{}
+	cm.On("GetContainerArchive", mock.Anything, "/var/run/act/workflow/step-summary-0.md").Return(
+		io.NopCloser(bytes.NewReader(tarArchive(t, tarEntry{
+			name: "step-summary-0.md", body: "deployed true with s3cr3t and runtime-added via pr0xypw",
+		}))),
+		nil,
+	).Once()
+
+	rc := newJobSummaryRC(map[string]string{
+		"GITEA_ACTIONS_CAPABILITIES": "job-summary",
+		"ACTIONS_RUNTIME_URL":        server.URL,
+		"ACTIONS_RUNTIME_TOKEN":      fakeRuntimeToken(34),
+		"GITEA_RUN_ID":               "12",
+	}, cm, 1)
+	rc.Config.Secrets = map[string]string{"TOK": "s3cr3t", "ACTIONS_STEP_DEBUG": "true"}
+	rc.Config.ExtraMasks = []string{"pr0xypw"}
+	rc.Masks = []string{"runtime-added"}
+
+	tryUploadJobSummary(context.Background(), rc)
+
+	assert.Equal(t, "deployed true with *** and *** via ***", got)
+	cm.AssertExpectations(t)
+}

act/runner/job_hooks.go

@@ -11,8 +11,8 @@ import (
 	"path"
 	"strings"
 
-	"gitea.com/gitea/runner/internal/act/common"
-	"gitea.com/gitea/runner/internal/act/container"
+	"gitea.com/gitea/runner/act/common"
+	"gitea.com/gitea/runner/act/container"
 )
 
 // GitHub's job-hook variables, read as a fallback when the settings are unset.
@@ -64,7 +64,9 @@ func (rc *RunContext) runJobHook(ctx context.Context, hookPath, name string) err
 	}
 	// Processed even on failure, so a hook that exports what it managed to set up before
 	// failing still hands it to the job.
-	err = cmp.Or(err, rc.processHookFileCommands(ctx))
+	if processErr := rc.processHookFileCommands(ctx); err == nil {
+		err = processErr
+	}
 	if err == nil {
 		return nil
 	}

act/runner/job_hooks_test.go

@@ -11,7 +11,7 @@ import (
 	"maps"
 	"testing"
 
-	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/act/common"
 
 	"gitea.dev/actionslib/pkg/model"
 	"github.com/sirupsen/logrus/hooks/test"

act/runner/logger.go

@@ -8,7 +8,8 @@ import (
 	"bytes"
 	"context"
 	"encoding/base64"
-	"encoding/json"
+	"encoding/json/jsontext"
+	"encoding/json/v2"
 	"fmt"
 	"io"
 	"net/url"
@@ -17,7 +18,7 @@ import (
 	"strings"
 	"sync"
 
-	"gitea.com/gitea/runner/internal/act/common"
+	"gitea.com/gitea/runner/act/common"
 
 	"github.com/sirupsen/logrus"
 	"golang.org/x/term"
@@ -78,7 +79,7 @@ type JobLoggerFactory interface {
 
 type jobLoggerFactoryContextKey string
 
-var jobLoggerFactoryContextKeyVal = (jobLoggerFactoryContextKey)("jobloggerkey")
+var jobLoggerFactoryContextKeyVal = jobLoggerFactoryContextKey("jobloggerkey")
 
 func WithJobLoggerFactory(ctx context.Context, factory JobLoggerFactory) context.Context {
 	return context.WithValue(ctx, jobLoggerFactoryContextKeyVal, factory)
@@ -99,10 +100,7 @@ func WithJobLogger(ctx context.Context, jobID, jobName string, config *Config, m
 			mux.Lock()
 			defer mux.Unlock()
 			nextColor++
-			formatter = &jobLogFormatter{
-				color:          colors[nextColor%len(colors)],
-				logPrefixJobID: config.LogPrefixJobID,
-			}
+			formatter = &jobLogFormatter{color: colors[nextColor%len(colors)]}
 		}
 
 		logger = logrus.New()
@@ -124,7 +122,7 @@ func WithJobLogger(ctx context.Context, jobID, jobName string, config *Config, m
 
 	logger.SetFormatter(&maskedFormatter{
 		Formatter: logger.Formatter,
-		masker:    valueMasker(config.InsecureSecrets, config.Secrets),
+		masker:    valueMasker(config.InsecureSecrets, config.maskers()),
 	})
 	rtn := logger.WithFields(logrus.Fields{
 		"job":    jobName,
@@ -218,7 +216,7 @@ func base64ShiftEncoder(shift int) func(string) string {
 // escapes <, >, & (as act's own toJSON does); the non-HTML variant below covers the runtimes
 // that do not. When v has none of those characters both forms are equal and deduplicated.
 func jsonStringEscape(v string) string {
-	encoded, err := json.Marshal(v)
+	encoded, err := json.Marshal(v, jsontext.EscapeForHTML(true))
 	if err != nil {
 		return v
 	}
@@ -229,15 +227,22 @@ func jsonStringEscape(v string) string {
 // JavaScript (JSON.stringify) or .NET action emits, so a secret containing < > or & is
 // masked in that form too.
 func jsonStringEscapeNoHTML(v string) string {
-	var buf bytes.Buffer
-	enc := json.NewEncoder(&buf)
-	enc.SetEscapeHTML(false)
-	if err := enc.Encode(v); err != nil {
+	encoded, err := json.Marshal(v)
+	if err != nil {
 		return v
 	}
-	// Encode appends a newline; drop it along with the surrounding quotes.
-	encoded := strings.TrimRight(buf.String(), "\n")
-	return encoded[1 : len(encoded)-1]
+	return string(encoded[1 : len(encoded)-1])
+}
+
+// AppendSecretMaskers skips the debug settings, as GitHub does: they arrive as secrets, but
+// masking "true" would corrupt unrelated log lines and drop job outputs that say it.
+func AppendSecretMaskers(oldnew []string, secrets map[string]string) []string {
+	for k, v := range secrets {
+		if k != "ACTIONS_STEP_DEBUG" && k != "ACTIONS_RUNNER_DEBUG" {
+			oldnew = AppendSecretMasker(oldnew, v)
+		}
+	}
+	return oldnew
 }
 
 func AppendSecretMasker(oldnew []string, v string) []string {
@@ -275,13 +280,9 @@ func AppendSecretMasker(oldnew []string, v string) []string {
 
 // valueMasker applies secrets and ::add-mask:: patterns to every log entry, including
 // raw_output (command/stream) lines; there is no bypass by field.
-func valueMasker(insecureSecrets bool, secrets map[string]string) entryProcessor {
-	var oldnew []string
-	for _, v := range secrets {
-		oldnew = AppendSecretMasker(oldnew, v)
-	}
+func valueMasker(insecureSecrets bool, oldnew []string) entryProcessor {
 	oldnew = slices.Clip(oldnew)
-	defReplacer := strings.NewReplacer(oldnew...)
+	defReplacer := NewSecretReplacer(oldnew)
 
 	// A ::add-mask:: only ever appends to the job's mask slice, so the replacer built for
 	// it stays valid until the slice grows. Cache it, keyed by the slice itself and its
@@ -317,7 +318,7 @@ func valueMasker(insecureSecrets bool, secrets map[string]string) entryProcessor
 				pairs = AppendSecretMasker(pairs, v)
 			}
 			masked = len(*masks)
-			replacer = strings.NewReplacer(pairs...)
+			replacer = NewSecretReplacer(pairs)
 		}
 		cmasker := replacer
 		mu.Unlock()
@@ -338,8 +339,7 @@ func (f *maskedFormatter) Format(entry *logrus.Entry) ([]byte, error) {
 }
 
 type jobLogFormatter struct {
-	color          int
-	logPrefixJobID bool
+	color int
 }
 
 func (f *jobLogFormatter) Format(entry *logrus.Entry) ([]byte, error) {
@@ -363,27 +363,23 @@ func (f *jobLogFormatter) Format(entry *logrus.Entry) ([]byte, error) {
 func (f *jobLogFormatter) printColored(b *bytes.Buffer, entry *logrus.Entry) {
 	entry.Message = strings.TrimSuffix(entry.Message, "\n")
 
-	var job any
-	if f.logPrefixJobID {
-		job = entry.Data["jobID"]
-	} else {
-		job = entry.Data["job"]
-	}
+	job := entry.Data["job"]
 
 	debugFlag := ""
 	if entry.Level == logrus.DebugLevel {
 		debugFlag = "[DEBUG] "
 	}
 
-	if entry.Data[rawOutputField] == true {
+	switch {
+	case entry.Data[rawOutputField] == true:
 		if entry.Data[scriptLineCyanField] == true {
 			fmt.Fprintf(b, "\x1b[%dm|\x1b[0m \x1b[36;1m%s\x1b[0m", f.color, entry.Message)
 		} else {
 			fmt.Fprintf(b, "\x1b[%dm|\x1b[0m %s", f.color, entry.Message)
 		}
-	} else if entry.Data["dryrun"] == true {
+	case entry.Data["dryrun"] == true:
 		fmt.Fprintf(b, "\x1b[1m\x1b[%dm\x1b[7m*DRYRUN*\x1b[0m \x1b[%dm[%s] \x1b[0m%s%s", gray, f.color, job, debugFlag, entry.Message)
-	} else {
+	default:
 		fmt.Fprintf(b, "\x1b[%dm[%s] \x1b[0m%s%s", f.color, job, debugFlag, entry.Message)
 	}
 }
@@ -391,23 +387,19 @@ func (f *jobLogFormatter) printColored(b *bytes.Buffer, entry *logrus.Entry) {
 func (f *jobLogFormatter) print(b *bytes.Buffer, entry *logrus.Entry) {
 	entry.Message = strings.TrimSuffix(entry.Message, "\n")
 
-	var job any
-	if f.logPrefixJobID {
-		job = entry.Data["jobID"]
-	} else {
-		job = entry.Data["job"]
-	}
+	job := entry.Data["job"]
 
 	debugFlag := ""
 	if entry.Level == logrus.DebugLevel {
 		debugFlag = "[DEBUG] "
 	}
 
-	if entry.Data[rawOutputField] == true {
+	switch {
+	case entry.Data[rawOutputField] == true:
 		fmt.Fprintf(b, "[%s]   | %s", job, entry.Message)
-	} else if entry.Data["dryrun"] == true {
+	case entry.Data["dryrun"] == true:
 		fmt.Fprintf(b, "*DRYRUN* [%s] %s%s", job, debugFlag, entry.Message)
-	} else {
+	default:
 		fmt.Fprintf(b, "[%s] %s%s", job, debugFlag, entry.Message)
 	}
 }
@@ -434,3 +426,39 @@ func checkIfTerminal(w io.Writer) bool {
 		return false
 	}
 }
+
+// maskSecrets hides this job's secrets in a value that reaches somewhere the log maskers cannot,
+// such as a container name or a job summary. Masks added at runtime count, so a summary written
+// after ::add-mask:: is covered too.
+func (rc *RunContext) maskSecrets(value string) string {
+	oldnew := rc.Config.maskers()
+	for _, mask := range rc.Masks {
+		oldnew = AppendSecretMasker(oldnew, mask)
+	}
+	return NewSecretReplacer(oldnew).Replace(value)
+}
+
+// maskers is every value this job's configuration says to hide, whatever the sink.
+func (c *Config) maskers() []string {
+	oldnew := AppendSecretMaskers(nil, c.Secrets)
+	for _, mask := range c.ExtraMasks {
+		oldnew = AppendSecretMasker(oldnew, mask)
+	}
+	return oldnew
+}
+
+// NewSecretReplacer masks the longest secret first. Replacer matches in argument order, so a
+// secret that prefixes another would otherwise mask only that prefix and print the rest.
+func NewSecretReplacer(oldnew []string) *strings.Replacer {
+	pairs := make([][2]string, 0, len(oldnew)/2)
+	for i := 0; i+1 < len(oldnew); i += 2 {
+		pairs = append(pairs, [2]string{oldnew[i], oldnew[i+1]})
+	}
+	slices.SortFunc(pairs, func(a, b [2]string) int { return len(b[0]) - len(a[0]) })
+
+	sorted := make([]string, 0, len(pairs)*2)
+	for _, pair := range pairs {
+		sorted = append(sorted, pair[0], pair[1])
+	}
+	return strings.NewReplacer(sorted...)
+}

act/runner/logger_test.go

@@ -47,7 +47,7 @@ func TestValueMasker(t *testing.T) {
 	for _, entry := range table {
 		t.Run(entry.name, func(t *testing.T) {
 			ctx := WithMasks(t.Context(), &entry.masks)
-			masker := valueMasker(false, entry.secrets)
+			masker := valueMasker(false, AppendSecretMaskers(nil, entry.secrets))
 			for line := range strings.SplitSeq(entry.lines, "\n") {
 				lentry := masker(&logrus.Entry{
 					Context: ctx,
@@ -65,7 +65,7 @@ func TestValueMasker(t *testing.T) {
 // URL — must be masked as well: masking only the verbatim value leaks it.
 func TestValueMaskerEncodedSecrets(t *testing.T) {
 	secret := `p@ss w"rd/1`
-	masker := valueMasker(false, map[string]string{"TOKEN": secret})
+	masker := valueMasker(false, AppendSecretMaskers(nil, map[string]string{"TOKEN": secret}))
 
 	for _, tc := range []struct {
 		name string
@@ -94,7 +94,7 @@ func TestValueMaskerEncodedSecrets(t *testing.T) {
 // form, so a JS-serialized JSON body does not leak it.
 func TestValueMaskerJSONEscapesBothWays(t *testing.T) {
 	secret := `a"<b>&c`
-	masker := valueMasker(false, map[string]string{"TOKEN": secret})
+	masker := valueMasker(false, AppendSecretMaskers(nil, map[string]string{"TOKEN": secret}))
 
 	for _, tc := range []struct {
 		name string
@@ -112,10 +112,20 @@ func TestValueMaskerJSONEscapesBothWays(t *testing.T) {
 	}
 }
 
+// With debug logging on, the job logger writes to stdout, which no reporter masks, so it has
+// to hide the values that are not job secrets too.
+func TestValueMaskerHidesExtraMasks(t *testing.T) {
+	masker := valueMasker(false, (&Config{ExtraMasks: []string{"pr0xypw"}}).maskers())
+
+	entry := masker(&logrus.Entry{Context: t.Context(), Message: "proxy is http://user:pr0xypw@proxy:3128"})
+
+	assert.Equal(t, "proxy is http://user:***@proxy:3128", entry.Message)
+}
+
 // ::add-mask:: values go through the same masker, so they get the same treatment.
 func TestValueMaskerEncodedMasks(t *testing.T) {
 	masks := []string{"s3cr3t value"}
-	masker := valueMasker(false, nil)
+	masker := valueMasker(false, AppendSecretMaskers(nil, nil))
 
 	entry := masker(&logrus.Entry{
 		Context: WithMasks(t.Context(), &masks),
@@ -131,7 +141,7 @@ func TestValueMaskerEncodedMasks(t *testing.T) {
 // the token to anyone who can decode the log.
 func TestValueMaskerBase64Alignments(t *testing.T) {
 	secret := "s3cr3t-token-value"
-	masker := valueMasker(false, map[string]string{"TOKEN": secret})
+	masker := valueMasker(false, AppendSecretMaskers(nil, map[string]string{"TOKEN": secret}))
 
 	// One prefix per alignment: len%3 of 0, 1 and 2.
 	for _, prefix := range []string{"x-access-token:", "user:", "ab:"} {
@@ -155,7 +165,7 @@ func TestValueMaskerBase64Alignments(t *testing.T) {
 // The masker caches its replacer, so it has to notice both a mask appended to the same
 // slice and a composite action logging with a slice of its own.
 func TestValueMaskerCachedReplacerSeesNewMasks(t *testing.T) {
-	masker := valueMasker(false, map[string]string{"TOKEN": "secret-token"})
+	masker := valueMasker(false, AppendSecretMaskers(nil, map[string]string{"TOKEN": "secret-token"}))
 	mask := func(masks *[]string, message string) string {
 		return masker(&logrus.Entry{Context: WithMasks(t.Context(), masks), Message: message}).Message
 	}