feat: artifacts can be downloaded using their id instead of their name (#8957)

The web endpoint

`/{owner}/{repo}/actions/runs/{run_id}/artifacts/{artifact_name_or_id}`

can be used with either the artifact name used when it is uploaded or the instance wide unique number of the artifact, if it is not found. For instance:

`/root/myrepo/actions/run/3/artifacts/my_artifact_name`

or

`/root/myrepo/actions/run/3/artifacts/42`

The `upload-artifact@v4` output value `artifact-url` is built in this way and is now a valid URL to access the artifact.

Refs https://codeberg.org/forgejo/forgejo/issues/6147
Refs https://code.forgejo.org/forgejo/runner/issues/187
Refs https://code.forgejo.org/forgejo/upload-artifact/src/tag/v4#outputs

## Checklist

The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org).

### Tests

- I added test coverage for Go changes...
  - [x] in their respective `*_test.go` for unit tests.
  - [x] in the `tests/integration` directory if it involves interactions with a live Forgejo server.
- I added test coverage for JavaScript changes...
  - [ ] in `web_src/js/*.test.js` if it can be unit tested.
  - [ ] in `tests/e2e/*.test.e2e.js` if it requires interactions with a live Forgejo server (see also the [developer guide for JavaScript testing](https://codeberg.org/forgejo/forgejo/src/branch/forgejo/tests/e2e/README.md#end-to-end-tests)).

### Documentation

- [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change.
- [x] I did not document these changes and I do not expect someone else to do it.

### Release notes

- [ ] I do not want this change to show in the release notes.
- [x] I want the title to show in the release notes with a link to this pull request.
- [ ] I want the content of the `release-notes/<pull request number>.md` to be be used for the release notes instead of the title.

<!--start release-notes-assistant-->

## Release notes
<!--URL:https://codeberg.org/forgejo/forgejo-->
- Bug fixes
  - [PR](https://codeberg.org/forgejo/forgejo/pulls/8957): <!--number 8957 --><!--line 0 --><!--description YXJ0aWZhY3RzIGNhbiBiZSBkb3dubG9hZGVkIHVzaW5nIHRoZWlyIGlkIGluc3RlYWQgb2YgdGhlaXIgbmFtZQ==-->artifacts can be downloaded using their id instead of their name<!--description-->
<!--end release-notes-assistant-->

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8957
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>

models/actions/artifact.go

@@ -108,6 +108,7 @@ func UpdateArtifactByID(ctx context.Context, id int64, art *ActionArtifact) erro
 
 type FindArtifactsOptions struct {
 	db.ListOptions
+	ID           int64
 	RepoID       int64
 	RunID        int64
 	ArtifactName string
@@ -116,6 +117,9 @@ type FindArtifactsOptions struct {
 
 func (opts FindArtifactsOptions) ToConds() builder.Cond {
 	cond := builder.NewCond()
+	if opts.ID > 0 {
+		cond = cond.And(builder.Eq{"id": opts.ID})
+	}
 	if opts.RepoID > 0 {
 		cond = cond.And(builder.Eq{"repo_id": opts.RepoID})
 	}

models/fixtures/action_artifact.yml

@@ -57,7 +57,7 @@
   run_id: 792
   runner_id: 1
   repo_id: 4
-  owner_id: 1
+  owner_id: 5
   commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0
   storage_path: "27/5/1730330775594233150.chunk"
   file_size: 1024

routers/web/repo/actions/view.go

@@ -668,9 +668,51 @@ func ArtifactsDeleteView(ctx *context_module.Context) {
 	ctx.JSON(http.StatusOK, struct{}{})
 }
 
+func artifactsFind(ctx *context_module.Context, opts actions_model.FindArtifactsOptions) []*actions_model.ActionArtifact {
+	artifacts, err := db.Find[actions_model.ActionArtifact](ctx, opts)
+	if err != nil {
+		ctx.Error(http.StatusInternalServerError, err.Error())
+		return nil
+	}
+	if len(artifacts) == 0 {
+		return nil
+	}
+	return artifacts
+}
+
+func artifactsFindByNameOrID(ctx *context_module.Context, runID int64, nameOrID string) []*actions_model.ActionArtifact {
+	artifacts := artifactsFind(ctx, actions_model.FindArtifactsOptions{
+		RunID:        runID,
+		ArtifactName: nameOrID,
+	})
+	if ctx.Written() {
+		return nil
+	}
+	// if lookup by name found nothing, maybe it is an ID
+	if len(artifacts) == 0 {
+		id, err := strconv.ParseInt(nameOrID, 10, 64)
+		if err != nil || id == 0 {
+			ctx.Error(http.StatusNotFound, fmt.Sprintf("runID %d: artifact name not found: %v", runID, nameOrID))
+			return nil
+		}
+		artifacts = artifactsFind(ctx, actions_model.FindArtifactsOptions{
+			RunID: runID,
+			ID:    id,
+		})
+		if ctx.Written() {
+			return nil
+		}
+		if len(artifacts) == 0 {
+			ctx.Error(http.StatusNotFound, fmt.Sprintf("runID %d: artifact ID not found: %v", runID, nameOrID))
+			return nil
+		}
+	}
+	return artifacts
+}
+
 func ArtifactsDownloadView(ctx *context_module.Context) {
 	runIndex := ctx.ParamsInt64("run")
-	artifactName := ctx.Params("artifact_name")
+	artifactNameOrID := ctx.Params("artifact_name_or_id")
 
 	run, err := actions_model.GetRunByIndex(ctx, ctx.Repo.Repository.ID, runIndex)
 	if err != nil {
@@ -682,16 +724,8 @@ func ArtifactsDownloadView(ctx *context_module.Context) {
 		return
 	}
 
-	artifacts, err := db.Find[actions_model.ActionArtifact](ctx, actions_model.FindArtifactsOptions{
+	artifacts := artifactsFindByNameOrID(ctx, run.ID, artifactNameOrID)
-		RunID:        run.ID,
+	if ctx.Written() {
-		ArtifactName: artifactName,
-	})
-	if err != nil {
-		ctx.Error(http.StatusInternalServerError, err.Error())
-		return
-	}
-	if len(artifacts) == 0 {
-		ctx.Error(http.StatusNotFound, "artifact not found")
 		return
 	}
 
@@ -720,12 +754,14 @@ func ArtifactsDownloadView(ctx *context_module.Context) {
 			ctx.Error(http.StatusInternalServerError, err.Error())
 			return
 		}
-		common.ServeContentByReadSeeker(ctx.Base, artifactName, util.ToPointer(art.UpdatedUnix.AsTime()), f)
+		common.ServeContentByReadSeeker(ctx.Base, artifacts[0].ArtifactName+".zip", util.ToPointer(art.UpdatedUnix.AsTime()), f)
 		return
 	}
 
 	// Artifacts using the v1-v3 backend are stored as multiple individual files per artifact on the backend
 	// Those need to be zipped for download
+	artifactName := artifacts[0].ArtifactName
+
 	ctx.Resp.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%s.zip; filename*=UTF-8''%s.zip", url.PathEscape(artifactName), artifactName))
 	writer := zip.NewWriter(ctx.Resp)
 	defer writer.Close()

routers/web/repo/actions/view_test.go

@@ -0,0 +1,95 @@
+// Copyright 2025 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package actions
+
+import (
+	"fmt"
+	"testing"
+
+	actions_model "forgejo.org/models/actions"
+	unittest "forgejo.org/models/unittest"
+	"forgejo.org/services/contexttest"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_artifactsFind(t *testing.T) {
+	unittest.PrepareTestEnv(t)
+
+	for _, testCase := range []struct {
+		name         string
+		artifactName string
+		count        int
+	}{
+		{
+			name:         "Found",
+			artifactName: "artifact-v4-download",
+			count:        1,
+		},
+		{
+			name:         "NotFound",
+			artifactName: "notexist",
+			count:        0,
+		},
+	} {
+		t.Run(testCase.name, func(t *testing.T) {
+			runID := int64(792)
+			ctx, _ := contexttest.MockContext(t, fmt.Sprintf("user5/repo4/actions/runs/%v/artifacts/%v", runID, testCase.artifactName))
+			artifacts := artifactsFind(ctx, actions_model.FindArtifactsOptions{
+				RunID:        runID,
+				ArtifactName: testCase.artifactName,
+			})
+			assert.False(t, ctx.Written())
+			assert.Len(t, artifacts, testCase.count)
+		})
+	}
+}
+
+func Test_artifactsFindByNameOrID(t *testing.T) {
+	unittest.PrepareTestEnv(t)
+
+	for _, testCase := range []struct {
+		name     string
+		nameOrID string
+		err      string
+	}{
+		{
+			name:     "NameFound",
+			nameOrID: "artifact-v4-download",
+		},
+		{
+			name:     "NameNotFound",
+			nameOrID: "notexist",
+			err:      "artifact name not found",
+		},
+		{
+			name:     "IDFound",
+			nameOrID: "22",
+		},
+		{
+			name:     "IDNotFound",
+			nameOrID: "666",
+			err:      "artifact ID not found",
+		},
+		{
+			name:     "IDZeroNotFound",
+			nameOrID: "0",
+			err:      "artifact name not found",
+		},
+	} {
+		t.Run(testCase.name, func(t *testing.T) {
+			runID := int64(792)
+			ctx, resp := contexttest.MockContext(t, fmt.Sprintf("user5/repo4/actions/runs/%v/artifacts/%v", runID, testCase.nameOrID))
+			artifacts := artifactsFindByNameOrID(ctx, runID, testCase.nameOrID)
+			if testCase.err == "" {
+				assert.NotEmpty(t, artifacts)
+				assert.False(t, ctx.Written(), resp.Body.String())
+			} else {
+				assert.Empty(t, artifacts)
+				assert.True(t, ctx.Written())
+				assert.Contains(t, resp.Body.String(), testCase.err)
+			}
+		})
+	}
+}

routers/web/web.go

@@ -1455,7 +1455,7 @@ func registerRoutes(m *web.Route) {
 					m.Post("/cancel", reqRepoActionsWriter, actions.Cancel)
 					m.Post("/approve", reqRepoActionsWriter, actions.Approve)
 					m.Get("/artifacts", actions.ArtifactsView)
-					m.Get("/artifacts/{artifact_name}", actions.ArtifactsDownloadView)
+					m.Get("/artifacts/{artifact_name_or_id}", actions.ArtifactsDownloadView)
 					m.Delete("/artifacts/{artifact_name}", reqRepoActionsWriter, actions.ArtifactsDeleteView)
 					m.Post("/rerun", reqRepoActionsWriter, actions.Rerun)
 				})

tests/integration/api_actions_artifact_test.go

@@ -267,6 +267,11 @@ func TestActionsArtifactDownloadMultiFiles(t *testing.T) {
 		resp = MakeRequest(t, req, http.StatusOK)
 		assert.Equal(t, strings.Repeat(bodyChar, 1024), resp.Body.String())
 	}
+
+	// Download artifact via user-facing URL
+	req = NewRequest(t, "GET", "/user5/repo4/actions/runs/187/artifacts/multi-file-download")
+	resp = MakeRequest(t, req, http.StatusOK)
+	assert.Contains(t, resp.Header().Get("content-disposition"), "multi-file-download.zip")
 }
 
 func TestActionsArtifactUploadWithRetentionDays(t *testing.T) {

tests/integration/api_actions_artifact_v4_test.go

@@ -344,6 +344,7 @@ func TestActionsArtifactV4DownloadSingle(t *testing.T) {
 	req = NewRequest(t, "GET", "/user5/repo4/actions/runs/188/artifacts/artifact-v4-download")
 	resp = MakeRequest(t, req, http.StatusOK)
 	assert.Equal(t, "bytes", resp.Header().Get("accept-ranges"))
+	assert.Contains(t, resp.Header().Get("content-disposition"), "artifact-v4-download.zip")
 	assert.Equal(t, body, resp.Body.String())
 
 	// Partial artifact download