From 9d1d5ceddb0d9470c2806cd3a70f46c505cc980d Mon Sep 17 00:00:00 2001 From: silverwind Date: Fri, 23 Oct 2026 01:21:37 +0200 Subject: [PATCH] fix(actions): bound requested artifact retention (#39894) (#39946) `actions/upload-artifact` v4 and later send an absolute `expires_at` computed with the runner's clock, which Gitea stored unchecked. A runner clock running behind made artifacts expire at the next cleanup, and workflows could keep artifacts longer than `ARTIFACT_RETENTION_DAYS`. - Requested expiries are bounded by Gitea's clock, at least 1 day and at most `ARTIFACT_RETENTION_DAYS` when it is not `0`. Longer `retention-days` requests now get the instance limit. - `github.retention_days` now carries `ARTIFACT_RETENTION_DAYS`, so upload actions cap `retention-days` themselves and log a warning, like on GitHub. Co-authored-by: bircni --- custom/conf/app.example.ini | 2 +- models/actions/artifact.go | 7 ++++++- models/actions/artifact_test.go | 24 ++++++++++++++++++++++++ services/actions/cleanup_test.go | 3 ++- services/actions/context.go | 7 ++++++- services/actions/context_test.go | 6 ++++++ 6 files changed, 45 insertions(+), 4 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 43c40c0fc0c..dacd66b860b 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -3083,7 +3083,7 @@ LEVEL = Info ;LOG_RETENTION_DAYS = 365 ;; Days to keep artifacts. Old artifacts will be deleted after this period. 0 means keep forever. ;; Changes only apply to newly uploaded artifacts, existing ones keep the expiry stored when they were uploaded. -;; Artifacts could have their own retention periods by setting the `retention-days` option in `actions/upload-artifact` step. +;; Workflows can request a shorter retention period, down to 1 day. ;ARTIFACT_RETENTION_DAYS = 90 ;; Max artifact size in bytes that can be browsed or previewed in the web UI. Set to 0 to disable artifact previews, or -1 for no limit. ;ARTIFACT_PREVIEW_MAX_SIZE = 10485760 diff --git a/models/actions/artifact.go b/models/actions/artifact.go index 476ffcf5277..fd2eafef6e5 100644 --- a/models/actions/artifact.go +++ b/models/actions/artifact.go @@ -93,7 +93,12 @@ const artifactKeepForever timeutil.TimeStamp = 0 func artifactExpiry(requested optional.Option[timeutil.TimeStamp]) timeutil.TimeStamp { if requested.Has() { - return max(requested.Value(), artifactKeepForever+1) + now := timeutil.TimeStampNow() + expiry := max(requested.Value(), now.Add(timeutil.Day)) // v4 clients use the runner's clock + if setting.Actions.ArtifactRetentionDays > 0 { + expiry = min(expiry, now.Add(timeutil.Day*setting.Actions.ArtifactRetentionDays)) + } + return expiry } if setting.Actions.ArtifactRetentionDays <= 0 { return artifactKeepForever diff --git a/models/actions/artifact_test.go b/models/actions/artifact_test.go index f680483d53a..6392d57827a 100644 --- a/models/actions/artifact_test.go +++ b/models/actions/artifact_test.go @@ -5,6 +5,12 @@ package actions import ( "testing" + "time" + + "gitea.dev/modules/optional" + "gitea.dev/modules/setting" + "gitea.dev/modules/test" + "gitea.dev/modules/timeutil" "github.com/stretchr/testify/assert" ) @@ -25,3 +31,21 @@ func TestKeepLatestAttemptArtifacts(t *testing.T) { } assert.Equal(t, []int64{1, 4, 5}, ids) } + +func TestArtifactExpiry(t *testing.T) { + defer timeutil.MockSet(time.Unix(1_000_000_000, 0))() + now := timeutil.TimeStampNow() + days := func(n int64) timeutil.TimeStamp { return now.Add(n * timeutil.Day) } + requested := optional.Some[timeutil.TimeStamp] + + defer test.MockVariableValue(&setting.Actions.ArtifactRetentionDays, 90)() + assert.Equal(t, days(90), artifactExpiry(optional.None[timeutil.TimeStamp]())) + assert.Equal(t, days(5), artifactExpiry(requested(days(5)))) + assert.Equal(t, days(1), artifactExpiry(requested(days(-2))), "runner clock behind must not expire the artifact at once") + assert.Equal(t, days(90), artifactExpiry(requested(days(1000))), "runner must not exceed the instance retention") + + setting.Actions.ArtifactRetentionDays = 0 + assert.Equal(t, artifactKeepForever, artifactExpiry(optional.None[timeutil.TimeStamp]())) + assert.Equal(t, days(1000), artifactExpiry(requested(days(1000)))) + assert.Equal(t, days(1), artifactExpiry(requested(now))) +} diff --git a/services/actions/cleanup_test.go b/services/actions/cleanup_test.go index d8a65db689a..54ab6a30e12 100644 --- a/services/actions/cleanup_test.go +++ b/services/actions/cleanup_test.go @@ -123,7 +123,7 @@ func TestCleanupRetentionZeroKeepsForever(t *testing.T) { require.NoError(t, err) assert.Zero(t, art.ExpiredUnix) - // a workflow-requested expiry is honored, only the instance default may mean never + // a workflow-requested expiry is bounded, only the instance default may mean never asked, err := actions_model.CreateArtifact(t.Context(), task, "client-asked", "b.txt", optional.Some(timeutil.TimeStampNow())) require.NoError(t, err) assert.Positive(t, asked.ExpiredUnix) @@ -141,6 +141,7 @@ func TestCleanupRetentionZeroKeepsForever(t *testing.T) { _, err = db.GetEngine(t.Context()).In("id", art.ID, past.ID).Cols("status"). Update(&actions_model.ActionArtifact{Status: actions_model.ArtifactStatusUploadConfirmed}) require.NoError(t, err) + require.NoError(t, actions_model.UpdateArtifact(t.Context(), &actions_model.ActionArtifact{ID: past.ID, ExpiredUnix: 1}, "expired_unix")) expiring, err := actions_model.ListNeedExpiredArtifacts(t.Context()) require.NoError(t, err) ids := container.FilterSlice(expiring, func(a *actions_model.ActionArtifact) (int64, bool) { return a.ID, true }) diff --git a/services/actions/context.go b/services/actions/context.go index be814b1de81..8dcc8bc447f 100644 --- a/services/actions/context.go +++ b/services/actions/context.go @@ -64,6 +64,11 @@ func GenerateGiteaContext(ctx context.Context, run *actions_model.ActionRun, att refProtected = false } + retentionDays := "" // not "0", v3 artifact uploads would read it as a 0-day limit + if setting.Actions.ArtifactRetentionDays > 0 { + retentionDays = strconv.FormatInt(setting.Actions.ArtifactRetentionDays, 10) + } + gitContext := GiteaContext{ // standard contexts, see https://docs.github.com/en/actions/learn-github-actions/contexts#github-context "action": "", // string, The name of the action currently running, or the id of a step. GitHub removes special characters, and uses the name __run when the current step runs a script without an id. If you use the same action more than once in the same job, the name will include a suffix with the sequence number with underscore before it. For example, the first script you run will have the name __run, and the second script will be named __run_2. Similarly, the second invocation of actions/checkout will be actionscheckout2. @@ -89,7 +94,7 @@ func GenerateGiteaContext(ctx context.Context, run *actions_model.ActionRun, att "repository": run.Repo.OwnerName + "/" + run.Repo.Name, // string, The owner and repository name. For example, Codertocat/Hello-World. "repository_owner": run.Repo.OwnerName, // string, The repository owner's name. For example, Codertocat. "repositoryUrl": run.Repo.HTMLURL(), // string, The Git URL to the repository. For example, git://github.com/codertocat/hello-world.git. - "retention_days": "", // string, The number of days that workflow run logs and artifacts are kept. + "retention_days": retentionDays, // string, The number of days that artifacts are kept, empty when kept forever. "run_id": strconv.FormatInt(run.ID, 10), // string, A unique number for each workflow run within a repository. This number does not change if you re-run the workflow run. "run_number": strconv.FormatInt(run.Index, 10), // string, A unique number for each run of a particular workflow in a repository. This number begins at 1 for the workflow's first run, and increments with each new run. This number does not change if you re-run the workflow run. "run_attempt": "", // string, A unique number for each attempt of a particular workflow run in a repository. This number begins at 1 for the workflow run's first attempt, and increments with each re-run. diff --git a/services/actions/context_test.go b/services/actions/context_test.go index b16f1247b1f..6d466a123b6 100644 --- a/services/actions/context_test.go +++ b/services/actions/context_test.go @@ -16,6 +16,7 @@ import ( user_model "gitea.dev/models/user" actions_module "gitea.dev/modules/actions" "gitea.dev/modules/json" + "gitea.dev/modules/setting" api "gitea.dev/modules/structs" "gitea.dev/modules/test" webhook_module "gitea.dev/modules/webhook" @@ -408,6 +409,11 @@ func TestGenerateGiteaContextPullRequestTarget(t *testing.T) { assert.Equal(t, "refs/heads/main", giteaCtx["ref"]) assert.Equal(t, "main", giteaCtx["ref_name"]) + + defer test.MockVariableValue(&setting.Actions.ArtifactRetentionDays, 7)() + assert.Equal(t, "7", GenerateGiteaContext(t.Context(), run, nil, nil)["retention_days"]) + setting.Actions.ArtifactRetentionDays = 0 + assert.Empty(t, GenerateGiteaContext(t.Context(), run, nil, nil)["retention_days"]) } func TestGenerateGiteaContextRefProtected(t *testing.T) {