From 32b6aefcf825ae56c3f9c07810247e3768909eb4 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 5 Feb 2025 16:46:39 +0100 Subject: [PATCH 01/41] Artifacts download api for artifact actions v4 * download endpoint has to use 302 redirect * fake blob download used if direct download not possible * downloading v3 artifacts not possible New apis based on GitHub Rest V3 /runs/{run}/artifacts (Cannot use run index of url due to not beeing unique) /artifacts /artifacts/{artifact_id}/zip --- models/actions/artifact.go | 13 +- modules/structs/repo_actions.go | 30 +++ routers/api/v1/api.go | 4 + routers/api/v1/repo/action.go | 336 ++++++++++++++++++++++++++++++++ services/convert/convert.go | 23 +++ 5 files changed, 402 insertions(+), 4 deletions(-) diff --git a/models/actions/artifact.go b/models/actions/artifact.go index 0bc66ba24e846..4b1b525e0c7e5 100644 --- a/models/actions/artifact.go +++ b/models/actions/artifact.go @@ -108,10 +108,11 @@ func UpdateArtifactByID(ctx context.Context, id int64, art *ActionArtifact) erro type FindArtifactsOptions struct { db.ListOptions - RepoID int64 - RunID int64 - ArtifactName string - Status int + RepoID int64 + RunID int64 + ArtifactName string + Status int + FinalizedArtifactsV2 bool } func (opts FindArtifactsOptions) ToConds() builder.Cond { @@ -128,6 +129,10 @@ func (opts FindArtifactsOptions) ToConds() builder.Cond { if opts.Status > 0 { cond = cond.And(builder.Eq{"status": opts.Status}) } + if opts.FinalizedArtifactsV2 { + cond = cond.And(builder.Eq{"status": ArtifactStatusUploadConfirmed}.Or(builder.Eq{"status": ArtifactStatusExpired})) + cond = cond.And(builder.Eq{"content_encoding": "application/zip"}) + } return cond } diff --git a/modules/structs/repo_actions.go b/modules/structs/repo_actions.go index b13f34473861f..e6c204ddb5d52 100644 --- a/modules/structs/repo_actions.go +++ b/modules/structs/repo_actions.go @@ -32,3 +32,33 @@ type ActionTaskResponse struct { Entries []*ActionTask `json:"workflow_runs"` TotalCount int64 `json:"total_count"` } + +// ActionTask represents a ActionTask +type ActionArtifact struct { + ID int64 `json:"id"` + Name string `json:"name"` + SizeInBytes int64 `json:"size_in_bytes"` + URL string `json:"url"` + ArchiveDownloadURL string `json:"archive_download_url"` + Expired bool `json:"expired"` + WorkflowRun *ActionWorkflowRun `json:"workflow_run"` + + // swagger:strfmt date-time + CreatedAt time.Time `json:"created_at"` + // swagger:strfmt date-time + UpdatedAt time.Time `json:"updated_at"` + // swagger:strfmt date-time + ExpiresAt time.Time `json:"expires_at"` +} + +type ActionWorkflowRun struct { + ID int64 `json:"id"` + RepositoryID int64 `json:"repository_id"` + HeadSha string `json:"head_sha"` +} + +// ActionArtifactsResponse returns a ActionTask +type ActionArtifactsResponse struct { + Entries []*ActionArtifact `json:"artifacts"` + TotalCount int64 `json:"total_count"` +} diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 0aa38b8b6abbb..099dc7fac28a2 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1235,6 +1235,10 @@ func Routes() *web.Router { }, reqToken(), reqAdmin()) m.Group("/actions", func() { m.Get("/tasks", repo.ListActionTasks) + m.Get("/runs/{run}/artifacts", repo.GetArtifactsOfRun) + m.Get("/artifacts", repo.GetArtifacts) + m.Get("/artifacts/{artifact_id}/zip", repo.DownloadArtifact) + m.Get("/artifacts/{artifact_id}/zip/raw", repo.DownloadArtifactRaw) }, reqRepoReader(unit.TypeActions), context.ReferencesGitRepo(true)) m.Group("/keys", func() { m.Combo("").Get(repo.ListDeployKeys). diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index d27e8d2427b39..cce17d3606add 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -5,11 +5,17 @@ package repo import ( "errors" + "fmt" + "io" "net/http" + "net/url" + "strings" actions_model "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/models/db" secret_model "code.gitea.io/gitea/models/secret" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/storage" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" @@ -581,3 +587,333 @@ func ListActionTasks(ctx *context.APIContext) { ctx.JSON(http.StatusOK, &res) } + +// GetArtifacts Lists all artifacts for a repository +func GetArtifactsOfRun(ctx *context.APIContext) { + // swagger:operation GET /repos/{owner}/{repo}/actions/artifacts repository getArtifacts + // --- + // summary: Lists all artifacts for a repository run + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: name of the owner + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repository + // type: string + // required: true + // responses: + // "200": + // description: response when getting the artifacts + // "400": + // "$ref": "#/responses/error" + // "404": + // "$ref": "#/responses/notFound" + + repoID := ctx.Repo.Repository.ID + artifactName := ctx.PathParam("artifact_name") + + runID := ctx.PathParamInt64("run") + + artifacts, total, err := db.FindAndCount[actions_model.ActionArtifact](ctx, actions_model.FindArtifactsOptions{ + RepoID: repoID, + RunID: runID, + ArtifactName: artifactName, + //Status: int(actions_model.ArtifactStatusUploadConfirmed), + FinalizedArtifactsV2: true, + ListOptions: utils.GetListOptions(ctx), + }) + if err != nil { + ctx.Error(http.StatusInternalServerError, err.Error(), err) + return + } + + repoName := ctx.Repo.Repository.FullName() + + res := new(api.ActionArtifactsResponse) + res.TotalCount = total + + res.Entries = make([]*api.ActionArtifact, len(artifacts)) + for i := range artifacts { + convertedArtifact, err := convert.ToActionArtifact(ctx, repoName, artifacts[i]) + if err != nil { + ctx.Error(http.StatusInternalServerError, "ToActionArtifact", err) + return + } + res.Entries[i] = convertedArtifact + } + + ctx.JSON(http.StatusOK, &res) +} + +// GetArtifacts Lists all artifacts for a repository +func GetArtifacts(ctx *context.APIContext) { + // swagger:operation GET /repos/{owner}/{repo}/actions/artifacts repository getArtifacts + // --- + // summary: Lists all artifacts for a repository + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: name of the owner + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repository + // type: string + // required: true + // responses: + // "200": + // description: response when getting the artifacts + // "400": + // "$ref": "#/responses/error" + // "404": + // "$ref": "#/responses/notFound" + + repoID := ctx.Repo.Repository.ID + artifactName := ctx.PathParam("artifact_name") + + artifacts, total, err := db.FindAndCount[actions_model.ActionArtifact](ctx, actions_model.FindArtifactsOptions{ + RepoID: repoID, + ArtifactName: artifactName, + //Status: int(actions_model.ArtifactStatusUploadConfirmed), + FinalizedArtifactsV2: true, + ListOptions: utils.GetListOptions(ctx), + }) + if err != nil { + ctx.Error(http.StatusInternalServerError, err.Error(), err) + return + } + + repoName := ctx.Repo.Repository.FullName() + + res := new(api.ActionArtifactsResponse) + res.TotalCount = total + + res.Entries = make([]*api.ActionArtifact, len(artifacts)) + for i := range artifacts { + convertedArtifact, err := convert.ToActionArtifact(ctx, repoName, artifacts[i]) + if err != nil { + ctx.Error(http.StatusInternalServerError, "ToActionArtifact", err) + return + } + res.Entries[i] = convertedArtifact + } + + ctx.JSON(http.StatusOK, &res) +} + +// DownloadArtifact Gets a specific artifact for a workflow run. +func GetArtifact(ctx *context.APIContext) { + // swagger:operation GET /repos/{owner}/{repo}/actions/artifacts/{artifact_id}/zip repository downloadArtifact + // --- + // summary: Gets a specific artifact for a workflow run. + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: name of the owner + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repository + // type: string + // required: true + // - name: artifact_id + // in: path + // description: id of the artifact + // type: string + // required: true + // responses: + // "200": + // description: response when getting the artifacts + // "400": + // "$ref": "#/responses/error" + // "404": + // "$ref": "#/responses/notFound" + + artifactID := ctx.PathParamInt64("artifact_id") + + art, ok, err := db.GetByID[actions_model.ActionArtifact](ctx, artifactID) + if err != nil { + ctx.Error(http.StatusInternalServerError, err.Error(), err) + return + } + if !ok { + ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) + return + } + + // if artifacts status is not uploaded-confirmed or expired, treat it as not found + if art.Status != int64(actions_model.ArtifactStatusUploadConfirmed) && art.Status != int64(actions_model.ArtifactStatusExpired) { + ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) + return + } + + // Artifacts using the v4 backend are stored as a single combined zip file per artifact on the backend + // The v4 backend enshures ContentEncoding is set to "application/zip", which is not the case for the old backend + if art.ArtifactName+".zip" == art.ArtifactPath && art.ContentEncoding == "application/zip" { + repoName := ctx.Repo.Repository.FullName() + convertedArtifact, err := convert.ToActionArtifact(ctx, repoName, art) + if err != nil { + ctx.Error(http.StatusInternalServerError, "ToActionArtifact", err) + return + } + ctx.JSON(http.StatusOK, convertedArtifact) + return + } + // v3 not supported due to not having one unique id + ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) +} + +// DownloadArtifact Gets a specific artifact for a workflow run. +func DownloadArtifact(ctx *context.APIContext) { + // swagger:operation GET /repos/{owner}/{repo}/actions/artifacts/{artifact_id}/zip repository downloadArtifact + // --- + // summary: Gets a specific artifact for a workflow run redirects to blob url. + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: name of the owner + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repository + // type: string + // required: true + // - name: artifact_id + // in: path + // description: id of the artifact + // type: string + // required: true + // responses: + // "200": + // description: response when getting the artifacts + // "400": + // "$ref": "#/responses/error" + // "404": + // "$ref": "#/responses/notFound" + + artifactID := ctx.PathParamInt64("artifact_id") + + art, ok, err := db.GetByID[actions_model.ActionArtifact](ctx, artifactID) + if err != nil { + ctx.Error(http.StatusInternalServerError, err.Error(), err) + return + } + if !ok { + ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) + return + } + + // if artifacts status is not uploaded-confirmed, treat it as not found + if art.Status != int64(actions_model.ArtifactStatusUploadConfirmed) { + ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) + return + } + ctx.Resp.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%s.zip; filename*=UTF-8''%s.zip", url.PathEscape(art.ArtifactName), art.ArtifactName)) + + // Artifacts using the v4 backend are stored as a single combined zip file per artifact on the backend + // The v4 backend enshures ContentEncoding is set to "application/zip", which is not the case for the old backend + if art.ArtifactName+".zip" == art.ArtifactPath && art.ContentEncoding == "application/zip" { + art := art + if setting.Actions.ArtifactStorage.ServeDirect() { + u, err := storage.ActionsArtifacts.URL(art.StoragePath, art.ArtifactPath, nil) + if u != nil && err == nil { + ctx.Redirect(u.String(), 302) + return + } + } + // ##[error]Unable to download artifact(s): Unable to download artifact. Unexpected status: 200 + repoName := ctx.Repo.Repository.FullName() + url := strings.TrimSuffix(setting.AppURL, "/") + "/api/v1/repos/" + repoName + "/actions/artifacts/" + fmt.Sprintf("%d", art.ID) + "/zip/raw" + ctx.Redirect(url, 302) + return + } + // v3 not supported due to not having one unique id + ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) +} + +// DownloadArtifactRaw Gets a specific artifact for a workflow run. +func DownloadArtifactRaw(ctx *context.APIContext) { + // swagger:operation GET /repos/{owner}/{repo}/actions/artifacts/{artifact_id}/zip/raw repository downloadArtifactRaw + // --- + // summary: Gets a specific artifact for a workflow run direct download. + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: name of the owner + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repository + // type: string + // required: true + // - name: artifact_id + // in: path + // description: id of the artifact + // type: string + // required: true + // responses: + // "200": + // description: response when getting the artifacts + // "400": + // "$ref": "#/responses/error" + // "404": + // "$ref": "#/responses/notFound" + + artifactID := ctx.PathParamInt64("artifact_id") + + art, ok, err := db.GetByID[actions_model.ActionArtifact](ctx, artifactID) + if err != nil { + ctx.Error(http.StatusInternalServerError, err.Error(), err) + return + } + if !ok { + ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) + return + } + + // if artifacts status is not uploaded-confirmed, treat it as not found + if art.Status != int64(actions_model.ArtifactStatusUploadConfirmed) { + ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) + return + } + ctx.Resp.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%s.zip; filename*=UTF-8''%s.zip", url.PathEscape(art.ArtifactName), art.ArtifactName)) + + // Artifacts using the v4 backend are stored as a single combined zip file per artifact on the backend + // The v4 backend enshures ContentEncoding is set to "application/zip", which is not the case for the old backend + if art.ArtifactName+".zip" == art.ArtifactPath && art.ContentEncoding == "application/zip" { + art := art + if setting.Actions.ArtifactStorage.ServeDirect() { + u, err := storage.ActionsArtifacts.URL(art.StoragePath, art.ArtifactPath, nil) + if u != nil && err == nil { + ctx.Redirect(u.String()) + return + } + } + f, err := storage.ActionsArtifacts.Open(art.StoragePath) + if err != nil { + ctx.Error(http.StatusInternalServerError, err.Error(), err) + return + } + _, _ = io.Copy(ctx.Resp, f) + return + } + // v3 not supported due to not having one unique id + ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) +} diff --git a/services/convert/convert.go b/services/convert/convert.go index c8cad2a2ad07e..b63bba238a52d 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -229,6 +229,29 @@ func ToActionTask(ctx context.Context, t *actions_model.ActionTask) (*api.Action }, nil } +// ToActionArtifact convert a actions_model.ActionArtifact to an api.ActionArtifact +func ToActionArtifact(ctx context.Context, repoName string, art *actions_model.ActionArtifact) (*api.ActionArtifact, error) { + + url := strings.TrimSuffix(setting.AppURL, "/") + "/api/v1/repos/" + repoName + "/actions/artifacts/" + fmt.Sprintf("%d", art.ID) + + return &api.ActionArtifact{ + ID: art.ID, + Name: art.ArtifactName, + SizeInBytes: art.FileSize, + Expired: art.Status == int64(actions_model.ArtifactStatusExpired), + URL: url, + ArchiveDownloadURL: url + "/zip", + CreatedAt: art.CreatedUnix.AsLocalTime(), + UpdatedAt: art.UpdatedUnix.AsLocalTime(), + ExpiresAt: art.ExpiredUnix.AsLocalTime(), + WorkflowRun: &api.ActionWorkflowRun{ + ID: art.RunID, + RepositoryID: art.RepoID, + HeadSha: art.CommitSHA, + }, + }, nil +} + // ToVerification convert a git.Commit.Signature to an api.PayloadCommitVerification func ToVerification(ctx context.Context, c *git.Commit) *api.PayloadCommitVerification { verif := asymkey_model.ParseCommitWithSignature(ctx, c) From f7a1dcae6dad78697c8301d4ad75901bbd9798e3 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 5 Feb 2025 17:06:14 +0100 Subject: [PATCH 02/41] Update swagger docs --- modules/structs/repo_actions.go | 5 +- routers/api/v1/api.go | 1 + routers/api/v1/repo/action.go | 26 +-- routers/api/v1/swagger/repo.go | 14 ++ templates/swagger/v1_json.tmpl | 282 +++++++++++++++++++++++++++++++- 5 files changed, 312 insertions(+), 16 deletions(-) diff --git a/modules/structs/repo_actions.go b/modules/structs/repo_actions.go index e6c204ddb5d52..cf560e7517717 100644 --- a/modules/structs/repo_actions.go +++ b/modules/structs/repo_actions.go @@ -33,7 +33,7 @@ type ActionTaskResponse struct { TotalCount int64 `json:"total_count"` } -// ActionTask represents a ActionTask +// ActionArtifact represents a ActionArtifact type ActionArtifact struct { ID int64 `json:"id"` Name string `json:"name"` @@ -51,13 +51,14 @@ type ActionArtifact struct { ExpiresAt time.Time `json:"expires_at"` } +// ActionWorkflowRun represents a WorkflowRun type ActionWorkflowRun struct { ID int64 `json:"id"` RepositoryID int64 `json:"repository_id"` HeadSha string `json:"head_sha"` } -// ActionArtifactsResponse returns a ActionTask +// ActionArtifactsResponse returns ActionArtifacts type ActionArtifactsResponse struct { Entries []*ActionArtifact `json:"artifacts"` TotalCount int64 `json:"total_count"` diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 099dc7fac28a2..709ef255955d9 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1237,6 +1237,7 @@ func Routes() *web.Router { m.Get("/tasks", repo.ListActionTasks) m.Get("/runs/{run}/artifacts", repo.GetArtifactsOfRun) m.Get("/artifacts", repo.GetArtifacts) + m.Get("/artifacts/{artifact_id}", repo.GetArtifact) m.Get("/artifacts/{artifact_id}/zip", repo.DownloadArtifact) m.Get("/artifacts/{artifact_id}/zip/raw", repo.DownloadArtifactRaw) }, reqRepoReader(unit.TypeActions), context.ReferencesGitRepo(true)) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index cce17d3606add..e685d9e545a41 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -590,7 +590,7 @@ func ListActionTasks(ctx *context.APIContext) { // GetArtifacts Lists all artifacts for a repository func GetArtifactsOfRun(ctx *context.APIContext) { - // swagger:operation GET /repos/{owner}/{repo}/actions/artifacts repository getArtifacts + // swagger:operation GET /repos/{owner}/{repo}/actions/artifacts repository getArtifactsOfRun // --- // summary: Lists all artifacts for a repository run // produces: @@ -608,7 +608,7 @@ func GetArtifactsOfRun(ctx *context.APIContext) { // required: true // responses: // "200": - // description: response when getting the artifacts + // "$ref": "#/responses/ArtifactsList" // "400": // "$ref": "#/responses/error" // "404": @@ -670,7 +670,7 @@ func GetArtifacts(ctx *context.APIContext) { // required: true // responses: // "200": - // description: response when getting the artifacts + // "$ref": "#/responses/ArtifactsList" // "400": // "$ref": "#/responses/error" // "404": @@ -709,9 +709,9 @@ func GetArtifacts(ctx *context.APIContext) { ctx.JSON(http.StatusOK, &res) } -// DownloadArtifact Gets a specific artifact for a workflow run. +// GetArtifact Gets a specific artifact for a workflow run. func GetArtifact(ctx *context.APIContext) { - // swagger:operation GET /repos/{owner}/{repo}/actions/artifacts/{artifact_id}/zip repository downloadArtifact + // swagger:operation GET /repos/{owner}/{repo}/actions/artifacts/{artifact_id} repository getArtifact // --- // summary: Gets a specific artifact for a workflow run. // produces: @@ -734,7 +734,7 @@ func GetArtifact(ctx *context.APIContext) { // required: true // responses: // "200": - // description: response when getting the artifacts + // "$ref": "#/responses/Artifact" // "400": // "$ref": "#/responses/error" // "404": @@ -774,11 +774,11 @@ func GetArtifact(ctx *context.APIContext) { ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) } -// DownloadArtifact Gets a specific artifact for a workflow run. +// DownloadArtifact Downloads a specific artifact for a workflow run redirects to blob url. func DownloadArtifact(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/actions/artifacts/{artifact_id}/zip repository downloadArtifact // --- - // summary: Gets a specific artifact for a workflow run redirects to blob url. + // summary: Downloads a specific artifact for a workflow run redirects to blob url. // produces: // - application/json // parameters: @@ -798,8 +798,8 @@ func DownloadArtifact(ctx *context.APIContext) { // type: string // required: true // responses: - // "200": - // description: response when getting the artifacts + // "302": + // description: redirect to the blob download // "400": // "$ref": "#/responses/error" // "404": @@ -845,11 +845,11 @@ func DownloadArtifact(ctx *context.APIContext) { ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) } -// DownloadArtifactRaw Gets a specific artifact for a workflow run. +// DownloadArtifactRaw Downloads a specific artifact for a workflow run directly. func DownloadArtifactRaw(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/actions/artifacts/{artifact_id}/zip/raw repository downloadArtifactRaw // --- - // summary: Gets a specific artifact for a workflow run direct download. + // summary: Downloads a specific artifact for a workflow run directly. // produces: // - application/json // parameters: @@ -870,7 +870,7 @@ func DownloadArtifactRaw(ctx *context.APIContext) { // required: true // responses: // "200": - // description: response when getting the artifacts + // description: the artifact content // "400": // "$ref": "#/responses/error" // "404": diff --git a/routers/api/v1/swagger/repo.go b/routers/api/v1/swagger/repo.go index f754c80a5b3d5..25f137f3bf84b 100644 --- a/routers/api/v1/swagger/repo.go +++ b/routers/api/v1/swagger/repo.go @@ -443,6 +443,20 @@ type swaggerRepoTasksList struct { Body api.ActionTaskResponse `json:"body"` } +// ArtifactsList +// swagger:response ArtifactsList +type swaggerRepoArtifactsList struct { + // in:body + Body api.ActionArtifactsResponse `json:"body"` +} + +// Artifact +// swagger:response Artifact +type swaggerRepoArtifact struct { + // in:body + Body api.ActionArtifact `json:"body"` +} + // swagger:response Compare type swaggerCompare struct { // in:body diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index d22e01c787619..333141d35fa59 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -3919,6 +3919,183 @@ } } }, + "/repos/{owner}/{repo}/actions/artifacts": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Lists all artifacts for a repository", + "operationId": "getArtifacts", + "parameters": [ + { + "type": "string", + "description": "name of the owner", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repository", + "name": "repo", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "$ref": "#/responses/ArtifactsList" + }, + "400": { + "$ref": "#/responses/error" + }, + "404": { + "$ref": "#/responses/notFound" + } + } + } + }, + "/repos/{owner}/{repo}/actions/artifacts/{artifact_id}": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Gets a specific artifact for a workflow run.", + "operationId": "getArtifact", + "parameters": [ + { + "type": "string", + "description": "name of the owner", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repository", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "id of the artifact", + "name": "artifact_id", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "$ref": "#/responses/Artifact" + }, + "400": { + "$ref": "#/responses/error" + }, + "404": { + "$ref": "#/responses/notFound" + } + } + } + }, + "/repos/{owner}/{repo}/actions/artifacts/{artifact_id}/zip": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Downloads a specific artifact for a workflow run redirects to blob url.", + "operationId": "downloadArtifact", + "parameters": [ + { + "type": "string", + "description": "name of the owner", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repository", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "id of the artifact", + "name": "artifact_id", + "in": "path", + "required": true + } + ], + "responses": { + "302": { + "description": "redirect to the blob download" + }, + "400": { + "$ref": "#/responses/error" + }, + "404": { + "$ref": "#/responses/notFound" + } + } + } + }, + "/repos/{owner}/{repo}/actions/artifacts/{artifact_id}/zip/raw": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Downloads a specific artifact for a workflow run directly.", + "operationId": "downloadArtifactRaw", + "parameters": [ + { + "type": "string", + "description": "name of the owner", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repository", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "id of the artifact", + "name": "artifact_id", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "the artifact content" + }, + "400": { + "$ref": "#/responses/error" + }, + "404": { + "$ref": "#/responses/notFound" + } + } + } + }, "/repos/{owner}/{repo}/actions/runners/registration-token": { "get": { "produces": [ @@ -18568,6 +18745,76 @@ }, "x-go-package": "code.gitea.io/gitea/modules/structs" }, + "ActionArtifact": { + "description": "ActionArtifact represents a ActionArtifact", + "type": "object", + "properties": { + "archive_download_url": { + "type": "string", + "x-go-name": "ArchiveDownloadURL" + }, + "created_at": { + "type": "string", + "format": "date-time", + "x-go-name": "CreatedAt" + }, + "expired": { + "type": "boolean", + "x-go-name": "Expired" + }, + "expires_at": { + "type": "string", + "format": "date-time", + "x-go-name": "ExpiresAt" + }, + "id": { + "type": "integer", + "format": "int64", + "x-go-name": "ID" + }, + "name": { + "type": "string", + "x-go-name": "Name" + }, + "size_in_bytes": { + "type": "integer", + "format": "int64", + "x-go-name": "SizeInBytes" + }, + "updated_at": { + "type": "string", + "format": "date-time", + "x-go-name": "UpdatedAt" + }, + "url": { + "type": "string", + "x-go-name": "URL" + }, + "workflow_run": { + "$ref": "#/definitions/ActionWorkflowRun" + } + }, + "x-go-package": "code.gitea.io/gitea/modules/structs" + }, + "ActionArtifactsResponse": { + "description": "ActionArtifactsResponse returns ActionArtifacts", + "type": "object", + "properties": { + "artifacts": { + "type": "array", + "items": { + "$ref": "#/definitions/ActionArtifact" + }, + "x-go-name": "Entries" + }, + "total_count": { + "type": "integer", + "format": "int64", + "x-go-name": "TotalCount" + } + }, + "x-go-package": "code.gitea.io/gitea/modules/structs" + }, "ActionTask": { "description": "ActionTask represents a ActionTask", "type": "object", @@ -18680,6 +18927,27 @@ }, "x-go-package": "code.gitea.io/gitea/modules/structs" }, + "ActionWorkflowRun": { + "description": "ActionWorkflowRun represents a WorkflowRun", + "type": "object", + "properties": { + "head_sha": { + "type": "string", + "x-go-name": "HeadSha" + }, + "id": { + "type": "integer", + "format": "int64", + "x-go-name": "ID" + }, + "repository_id": { + "type": "integer", + "format": "int64", + "x-go-name": "RepositoryID" + } + }, + "x-go-package": "code.gitea.io/gitea/modules/structs" + }, "Activity": { "type": "object", "properties": { @@ -25708,6 +25976,18 @@ "$ref": "#/definitions/AnnotatedTag" } }, + "Artifact": { + "description": "Artifact", + "schema": { + "$ref": "#/definitions/ActionArtifact" + } + }, + "ArtifactsList": { + "description": "ArtifactsList", + "schema": { + "$ref": "#/definitions/ActionArtifactsResponse" + } + }, "Attachment": { "description": "Attachment", "schema": { @@ -26801,4 +27081,4 @@ "TOTPHeader": [] } ] -} +} \ No newline at end of file From 3f0cafd6a3100b8118e7874a2cc687ae43a6c77c Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 5 Feb 2025 17:08:01 +0100 Subject: [PATCH 03/41] fix swagger --- routers/api/v1/repo/action.go | 10 +++++----- templates/swagger/v1_json.tmpl | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index e685d9e545a41..b74eb72eac8bb 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -588,7 +588,7 @@ func ListActionTasks(ctx *context.APIContext) { ctx.JSON(http.StatusOK, &res) } -// GetArtifacts Lists all artifacts for a repository +// GetArtifacts Lists all artifacts for a repository. func GetArtifactsOfRun(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/actions/artifacts repository getArtifactsOfRun // --- @@ -650,7 +650,7 @@ func GetArtifactsOfRun(ctx *context.APIContext) { ctx.JSON(http.StatusOK, &res) } -// GetArtifacts Lists all artifacts for a repository +// GetArtifacts Lists all artifacts for a repository. func GetArtifacts(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/actions/artifacts repository getArtifacts // --- @@ -713,7 +713,7 @@ func GetArtifacts(ctx *context.APIContext) { func GetArtifact(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/actions/artifacts/{artifact_id} repository getArtifact // --- - // summary: Gets a specific artifact for a workflow run. + // summary: Gets a specific artifact for a workflow run // produces: // - application/json // parameters: @@ -778,7 +778,7 @@ func GetArtifact(ctx *context.APIContext) { func DownloadArtifact(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/actions/artifacts/{artifact_id}/zip repository downloadArtifact // --- - // summary: Downloads a specific artifact for a workflow run redirects to blob url. + // summary: Downloads a specific artifact for a workflow run redirects to blob url // produces: // - application/json // parameters: @@ -849,7 +849,7 @@ func DownloadArtifact(ctx *context.APIContext) { func DownloadArtifactRaw(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/actions/artifacts/{artifact_id}/zip/raw repository downloadArtifactRaw // --- - // summary: Downloads a specific artifact for a workflow run directly. + // summary: Downloads a specific artifact for a workflow run directly // produces: // - application/json // parameters: diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 333141d35fa59..3ff569d9db614 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -3966,7 +3966,7 @@ "tags": [ "repository" ], - "summary": "Gets a specific artifact for a workflow run.", + "summary": "Gets a specific artifact for a workflow run", "operationId": "getArtifact", "parameters": [ { @@ -4012,7 +4012,7 @@ "tags": [ "repository" ], - "summary": "Downloads a specific artifact for a workflow run redirects to blob url.", + "summary": "Downloads a specific artifact for a workflow run redirects to blob url", "operationId": "downloadArtifact", "parameters": [ { @@ -4058,7 +4058,7 @@ "tags": [ "repository" ], - "summary": "Downloads a specific artifact for a workflow run directly.", + "summary": "Downloads a specific artifact for a workflow run directly", "operationId": "downloadArtifactRaw", "parameters": [ { From ef5b06917654916c83237d8d77de92a113727489 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 5 Feb 2025 17:19:57 +0100 Subject: [PATCH 04/41] format code --- routers/api/v1/repo/action.go | 4 ++-- services/convert/convert.go | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index b74eb72eac8bb..ede47e2c3b833 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -623,7 +623,7 @@ func GetArtifactsOfRun(ctx *context.APIContext) { RepoID: repoID, RunID: runID, ArtifactName: artifactName, - //Status: int(actions_model.ArtifactStatusUploadConfirmed), + // Status: int(actions_model.ArtifactStatusUploadConfirmed), FinalizedArtifactsV2: true, ListOptions: utils.GetListOptions(ctx), }) @@ -682,7 +682,7 @@ func GetArtifacts(ctx *context.APIContext) { artifacts, total, err := db.FindAndCount[actions_model.ActionArtifact](ctx, actions_model.FindArtifactsOptions{ RepoID: repoID, ArtifactName: artifactName, - //Status: int(actions_model.ArtifactStatusUploadConfirmed), + // Status: int(actions_model.ArtifactStatusUploadConfirmed), FinalizedArtifactsV2: true, ListOptions: utils.GetListOptions(ctx), }) diff --git a/services/convert/convert.go b/services/convert/convert.go index b63bba238a52d..dc8fd37b87e77 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -231,7 +231,6 @@ func ToActionTask(ctx context.Context, t *actions_model.ActionTask) (*api.Action // ToActionArtifact convert a actions_model.ActionArtifact to an api.ActionArtifact func ToActionArtifact(ctx context.Context, repoName string, art *actions_model.ActionArtifact) (*api.ActionArtifact, error) { - url := strings.TrimSuffix(setting.AppURL, "/") + "/api/v1/repos/" + repoName + "/actions/artifacts/" + fmt.Sprintf("%d", art.ID) return &api.ActionArtifact{ From 26514c926c26eaaaf6909725cffea5e4c7fe3975 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 5 Feb 2025 17:44:03 +0100 Subject: [PATCH 05/41] use MakeAbsoluteURL --- services/convert/convert.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/services/convert/convert.go b/services/convert/convert.go index dc8fd37b87e77..9cd529d1ee527 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -7,6 +7,7 @@ package convert import ( "context" "fmt" + "net/url" "strconv" "strings" "time" @@ -24,6 +25,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/httplib" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" @@ -231,7 +233,7 @@ func ToActionTask(ctx context.Context, t *actions_model.ActionTask) (*api.Action // ToActionArtifact convert a actions_model.ActionArtifact to an api.ActionArtifact func ToActionArtifact(ctx context.Context, repoName string, art *actions_model.ActionArtifact) (*api.ActionArtifact, error) { - url := strings.TrimSuffix(setting.AppURL, "/") + "/api/v1/repos/" + repoName + "/actions/artifacts/" + fmt.Sprintf("%d", art.ID) + url := httplib.MakeAbsoluteURL(ctx, setting.AppSubURL+"/api/v1/repos/"+url.PathEscape(repoName)+"/actions/artifacts/"+fmt.Sprintf("%d", art.ID)) return &api.ActionArtifact{ ID: art.ID, From e7308acc5e188b4c94cb10008df895c9504a36a0 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 5 Feb 2025 17:45:47 +0100 Subject: [PATCH 06/41] fix lint? swagger command defect? --- templates/swagger/v1_json.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 3ff569d9db614..afcd1942c03cf 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -27081,4 +27081,4 @@ "TOTPHeader": [] } ] -} \ No newline at end of file +} From e129fe8c2ae258948b69d9bdbaca34cde3607a64 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 5 Feb 2025 17:57:04 +0100 Subject: [PATCH 07/41] revert encoding / --- services/convert/convert.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/services/convert/convert.go b/services/convert/convert.go index 9cd529d1ee527..4b816333d7e9c 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -7,7 +7,6 @@ package convert import ( "context" "fmt" - "net/url" "strconv" "strings" "time" @@ -233,7 +232,7 @@ func ToActionTask(ctx context.Context, t *actions_model.ActionTask) (*api.Action // ToActionArtifact convert a actions_model.ActionArtifact to an api.ActionArtifact func ToActionArtifact(ctx context.Context, repoName string, art *actions_model.ActionArtifact) (*api.ActionArtifact, error) { - url := httplib.MakeAbsoluteURL(ctx, setting.AppSubURL+"/api/v1/repos/"+url.PathEscape(repoName)+"/actions/artifacts/"+fmt.Sprintf("%d", art.ID)) + url := httplib.MakeAbsoluteURL(ctx, setting.AppSubURL+"/api/v1/repos/"+repoName+"/actions/artifacts/"+fmt.Sprintf("%d", art.ID)) return &api.ActionArtifact{ ID: art.ID, From d5520bd26da9a88c754a0d37b6cf028ea5b21fc9 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 5 Feb 2025 20:21:13 +0100 Subject: [PATCH 08/41] 302 => http.StatusFound --- routers/api/v1/repo/action.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index ede47e2c3b833..7baa70fa02a9a 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -831,14 +831,14 @@ func DownloadArtifact(ctx *context.APIContext) { if setting.Actions.ArtifactStorage.ServeDirect() { u, err := storage.ActionsArtifacts.URL(art.StoragePath, art.ArtifactPath, nil) if u != nil && err == nil { - ctx.Redirect(u.String(), 302) + ctx.Redirect(u.String(), http.StatusFound) return } } // ##[error]Unable to download artifact(s): Unable to download artifact. Unexpected status: 200 repoName := ctx.Repo.Repository.FullName() url := strings.TrimSuffix(setting.AppURL, "/") + "/api/v1/repos/" + repoName + "/actions/artifacts/" + fmt.Sprintf("%d", art.ID) + "/zip/raw" - ctx.Redirect(url, 302) + ctx.Redirect(url, http.StatusFound) return } // v3 not supported due to not having one unique id From d3dbfcb474dcc464641de9e92c434703bbd008c6 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 5 Feb 2025 20:25:29 +0100 Subject: [PATCH 09/41] extract getArtifactByID into a helper function --- routers/api/v1/repo/action.go | 58 +++++++++++++++-------------------- 1 file changed, 24 insertions(+), 34 deletions(-) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index 7baa70fa02a9a..93dceef46b28c 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -740,21 +740,8 @@ func GetArtifact(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - artifactID := ctx.PathParamInt64("artifact_id") - - art, ok, err := db.GetByID[actions_model.ActionArtifact](ctx, artifactID) - if err != nil { - ctx.Error(http.StatusInternalServerError, err.Error(), err) - return - } + art, ok := getArtifactByID(ctx) if !ok { - ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) - return - } - - // if artifacts status is not uploaded-confirmed or expired, treat it as not found - if art.Status != int64(actions_model.ArtifactStatusUploadConfirmed) && art.Status != int64(actions_model.ArtifactStatusExpired) { - ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) return } @@ -805,21 +792,14 @@ func DownloadArtifact(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - artifactID := ctx.PathParamInt64("artifact_id") - - art, ok, err := db.GetByID[actions_model.ActionArtifact](ctx, artifactID) - if err != nil { - ctx.Error(http.StatusInternalServerError, err.Error(), err) - return - } + art, ok := getArtifactByID(ctx) if !ok { - ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) return } // if artifacts status is not uploaded-confirmed, treat it as not found - if art.Status != int64(actions_model.ArtifactStatusUploadConfirmed) { - ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) + if art.Status == int64(actions_model.ArtifactStatusExpired) { + ctx.Error(http.StatusNotFound, "artifact has expired", fmt.Errorf("artifact has expired")) return } ctx.Resp.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%s.zip; filename*=UTF-8''%s.zip", url.PathEscape(art.ArtifactName), art.ArtifactName)) @@ -876,21 +856,14 @@ func DownloadArtifactRaw(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - artifactID := ctx.PathParamInt64("artifact_id") - - art, ok, err := db.GetByID[actions_model.ActionArtifact](ctx, artifactID) - if err != nil { - ctx.Error(http.StatusInternalServerError, err.Error(), err) - return - } + art, ok := getArtifactByID(ctx) if !ok { - ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) return } // if artifacts status is not uploaded-confirmed, treat it as not found - if art.Status != int64(actions_model.ArtifactStatusUploadConfirmed) { - ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) + if art.Status == int64(actions_model.ArtifactStatusExpired) { + ctx.Error(http.StatusNotFound, "artifact has expired", fmt.Errorf("artifact has expired")) return } ctx.Resp.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%s.zip; filename*=UTF-8''%s.zip", url.PathEscape(art.ArtifactName), art.ArtifactName)) @@ -917,3 +890,20 @@ func DownloadArtifactRaw(ctx *context.APIContext) { // v3 not supported due to not having one unique id ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) } + +// Try to get the artifact by ID and check access +func getArtifactByID(ctx *context.APIContext) (*actions_model.ActionArtifact, bool) { + artifactID := ctx.PathParamInt64("artifact_id") + + art, ok, err := db.GetByID[actions_model.ActionArtifact](ctx, artifactID) + if err != nil { + ctx.Error(http.StatusInternalServerError, err.Error(), err) + return nil, false + } + // if artifacts status is not uploaded-confirmed, treat it as not found + if !ok || art.RepoID != ctx.Repo.Repository.ID || art.OwnerID != ctx.Repo.Repository.OwnerID || art.Status != int64(actions_model.ArtifactStatusUploadConfirmed) && art.Status != int64(actions_model.ArtifactStatusExpired) { + ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) + return nil, false + } + return art, true +} From d50358f6d721c897f143c873e7e84660834cf25f Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 5 Feb 2025 20:51:50 +0100 Subject: [PATCH 10/41] move old and new logic to modules --- models/actions/artifact.go | 4 +-- modules/actions/artifacts.go | 46 ++++++++++++++++++++++++ routers/api/v1/repo/action.go | 60 +++++++++++--------------------- routers/web/repo/actions/view.go | 16 ++------- 4 files changed, 70 insertions(+), 56 deletions(-) create mode 100644 modules/actions/artifacts.go diff --git a/models/actions/artifact.go b/models/actions/artifact.go index 4b1b525e0c7e5..d786b63a45118 100644 --- a/models/actions/artifact.go +++ b/models/actions/artifact.go @@ -112,7 +112,7 @@ type FindArtifactsOptions struct { RunID int64 ArtifactName string Status int - FinalizedArtifactsV2 bool + FinalizedArtifactsV4 bool } func (opts FindArtifactsOptions) ToConds() builder.Cond { @@ -129,7 +129,7 @@ func (opts FindArtifactsOptions) ToConds() builder.Cond { if opts.Status > 0 { cond = cond.And(builder.Eq{"status": opts.Status}) } - if opts.FinalizedArtifactsV2 { + if opts.FinalizedArtifactsV4 { cond = cond.And(builder.Eq{"status": ArtifactStatusUploadConfirmed}.Or(builder.Eq{"status": ArtifactStatusExpired})) cond = cond.And(builder.Eq{"content_encoding": "application/zip"}) } diff --git a/modules/actions/artifacts.go b/modules/actions/artifacts.go new file mode 100644 index 0000000000000..17d7a9014684c --- /dev/null +++ b/modules/actions/artifacts.go @@ -0,0 +1,46 @@ +package actions + +import ( + "io" + "net/http" + + actions_model "code.gitea.io/gitea/models/actions" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/storage" + "code.gitea.io/gitea/services/context" +) + +// Artifacts using the v4 backend are stored as a single combined zip file per artifact on the backend +// The v4 backend ensures ContentEncoding is set to "application/zip", which is not the case for the old backend +func IsArtifactV4(art *actions_model.ActionArtifact) bool { + return art.ArtifactName+".zip" == art.ArtifactPath && art.ContentEncoding == "application/zip" +} + +func DownloadArtifactV4ServeDirectOnly(ctx *context.Base, art *actions_model.ActionArtifact) (bool, error) { + if setting.Actions.ArtifactStorage.ServeDirect() { + u, err := storage.ActionsArtifacts.URL(art.StoragePath, art.ArtifactPath, nil) + if u != nil && err == nil { + ctx.Redirect(u.String(), http.StatusFound) + return true, nil + } + } + return false, nil +} + +func DownloadArtifactV4Fallback(ctx *context.Base, art *actions_model.ActionArtifact) error { + f, err := storage.ActionsArtifacts.Open(art.StoragePath) + if err != nil { + return err + } + defer f.Close() + _, _ = io.Copy(ctx.Resp, f) + return nil +} + +func DownloadArtifactV4(ctx *context.Base, art *actions_model.ActionArtifact) error { + ok, err := DownloadArtifactV4ServeDirectOnly(ctx, art) + if ok || err != nil { + return err + } + return DownloadArtifactV4Fallback(ctx, art) +} diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index 93dceef46b28c..35d8c3dfb0547 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -6,7 +6,6 @@ package repo import ( "errors" "fmt" - "io" "net/http" "net/url" "strings" @@ -14,8 +13,8 @@ import ( actions_model "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/models/db" secret_model "code.gitea.io/gitea/models/secret" + "code.gitea.io/gitea/modules/actions" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/storage" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" @@ -620,11 +619,10 @@ func GetArtifactsOfRun(ctx *context.APIContext) { runID := ctx.PathParamInt64("run") artifacts, total, err := db.FindAndCount[actions_model.ActionArtifact](ctx, actions_model.FindArtifactsOptions{ - RepoID: repoID, - RunID: runID, - ArtifactName: artifactName, - // Status: int(actions_model.ArtifactStatusUploadConfirmed), - FinalizedArtifactsV2: true, + RepoID: repoID, + RunID: runID, + ArtifactName: artifactName, + FinalizedArtifactsV4: true, ListOptions: utils.GetListOptions(ctx), }) if err != nil { @@ -680,10 +678,9 @@ func GetArtifacts(ctx *context.APIContext) { artifactName := ctx.PathParam("artifact_name") artifacts, total, err := db.FindAndCount[actions_model.ActionArtifact](ctx, actions_model.FindArtifactsOptions{ - RepoID: repoID, - ArtifactName: artifactName, - // Status: int(actions_model.ArtifactStatusUploadConfirmed), - FinalizedArtifactsV2: true, + RepoID: repoID, + ArtifactName: artifactName, + FinalizedArtifactsV4: true, ListOptions: utils.GetListOptions(ctx), }) if err != nil { @@ -745,9 +742,7 @@ func GetArtifact(ctx *context.APIContext) { return } - // Artifacts using the v4 backend are stored as a single combined zip file per artifact on the backend - // The v4 backend enshures ContentEncoding is set to "application/zip", which is not the case for the old backend - if art.ArtifactName+".zip" == art.ArtifactPath && art.ContentEncoding == "application/zip" { + if actions.IsArtifactV4(art) { repoName := ctx.Repo.Repository.FullName() convertedArtifact, err := convert.ToActionArtifact(ctx, repoName, art) if err != nil { @@ -804,18 +799,15 @@ func DownloadArtifact(ctx *context.APIContext) { } ctx.Resp.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%s.zip; filename*=UTF-8''%s.zip", url.PathEscape(art.ArtifactName), art.ArtifactName)) - // Artifacts using the v4 backend are stored as a single combined zip file per artifact on the backend - // The v4 backend enshures ContentEncoding is set to "application/zip", which is not the case for the old backend - if art.ArtifactName+".zip" == art.ArtifactPath && art.ContentEncoding == "application/zip" { - art := art - if setting.Actions.ArtifactStorage.ServeDirect() { - u, err := storage.ActionsArtifacts.URL(art.StoragePath, art.ArtifactPath, nil) - if u != nil && err == nil { - ctx.Redirect(u.String(), http.StatusFound) - return - } + if actions.IsArtifactV4(art) { + ok, err := actions.DownloadArtifactV4ServeDirectOnly(ctx.Base, art) + if ok { + return + } + if err != nil { + ctx.Error(http.StatusInternalServerError, err.Error(), err) + return } - // ##[error]Unable to download artifact(s): Unable to download artifact. Unexpected status: 200 repoName := ctx.Repo.Repository.FullName() url := strings.TrimSuffix(setting.AppURL, "/") + "/api/v1/repos/" + repoName + "/actions/artifacts/" + fmt.Sprintf("%d", art.ID) + "/zip/raw" ctx.Redirect(url, http.StatusFound) @@ -868,24 +860,12 @@ func DownloadArtifactRaw(ctx *context.APIContext) { } ctx.Resp.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%s.zip; filename*=UTF-8''%s.zip", url.PathEscape(art.ArtifactName), art.ArtifactName)) - // Artifacts using the v4 backend are stored as a single combined zip file per artifact on the backend - // The v4 backend enshures ContentEncoding is set to "application/zip", which is not the case for the old backend - if art.ArtifactName+".zip" == art.ArtifactPath && art.ContentEncoding == "application/zip" { - art := art - if setting.Actions.ArtifactStorage.ServeDirect() { - u, err := storage.ActionsArtifacts.URL(art.StoragePath, art.ArtifactPath, nil) - if u != nil && err == nil { - ctx.Redirect(u.String()) - return - } - } - f, err := storage.ActionsArtifacts.Open(art.StoragePath) + if actions.IsArtifactV4(art) { + err := actions.DownloadArtifactV4(ctx.Base, art) if err != nil { - ctx.Error(http.StatusInternalServerError, err.Error(), err) + ctx.Error(http.StatusInternalServerError, "artifact has expired", fmt.Errorf("artifact has expired")) return } - _, _ = io.Copy(ctx.Resp, f) - return } // v3 not supported due to not having one unique id ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index fc346b83b4736..46f4e4baef8c3 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -28,7 +28,6 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/templates" @@ -682,23 +681,12 @@ func ArtifactsDownloadView(ctx *context_module.Context) { ctx.Resp.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%s.zip; filename*=UTF-8''%s.zip", url.PathEscape(artifactName), artifactName)) - // Artifacts using the v4 backend are stored as a single combined zip file per artifact on the backend - // The v4 backend enshures ContentEncoding is set to "application/zip", which is not the case for the old backend - if len(artifacts) == 1 && artifacts[0].ArtifactName+".zip" == artifacts[0].ArtifactPath && artifacts[0].ContentEncoding == "application/zip" { - art := artifacts[0] - if setting.Actions.ArtifactStorage.ServeDirect() { - u, err := storage.ActionsArtifacts.URL(art.StoragePath, art.ArtifactPath, nil) - if u != nil && err == nil { - ctx.Redirect(u.String()) - return - } - } - f, err := storage.ActionsArtifacts.Open(art.StoragePath) + if len(artifacts) == 1 && actions.IsArtifactV4(artifacts[0]) { + err := actions.DownloadArtifactV4(ctx.Base, artifacts[0]) if err != nil { ctx.Error(http.StatusInternalServerError, err.Error()) return } - _, _ = io.Copy(ctx.Resp, f) return } From bbb7017e2f4f3433969d3ce52cd042d9517b7122 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 5 Feb 2025 21:29:08 +0100 Subject: [PATCH 11/41] fixup --- routers/api/v1/repo/action.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index 35d8c3dfb0547..c3d2b1a3b9eb6 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -614,7 +614,7 @@ func GetArtifactsOfRun(ctx *context.APIContext) { // "$ref": "#/responses/notFound" repoID := ctx.Repo.Repository.ID - artifactName := ctx.PathParam("artifact_name") + artifactName := ctx.Req.URL.Query().Get("name") runID := ctx.PathParamInt64("run") @@ -675,7 +675,7 @@ func GetArtifacts(ctx *context.APIContext) { // "$ref": "#/responses/notFound" repoID := ctx.Repo.Repository.ID - artifactName := ctx.PathParam("artifact_name") + artifactName := ctx.Req.URL.Query().Get("name") artifacts, total, err := db.FindAndCount[actions_model.ActionArtifact](ctx, actions_model.FindArtifactsOptions{ RepoID: repoID, @@ -863,7 +863,7 @@ func DownloadArtifactRaw(ctx *context.APIContext) { if actions.IsArtifactV4(art) { err := actions.DownloadArtifactV4(ctx.Base, art) if err != nil { - ctx.Error(http.StatusInternalServerError, "artifact has expired", fmt.Errorf("artifact has expired")) + ctx.Error(http.StatusInternalServerError, err.Error(), err) return } } From fcb8d55e5f8a7bb0fb0fe9418fcddcbecaf312a2 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 5 Feb 2025 21:29:22 +0100 Subject: [PATCH 12/41] intial tests --- .../api_actions_artifact_v4_test.go | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/tests/integration/api_actions_artifact_v4_test.go b/tests/integration/api_actions_artifact_v4_test.go index 8821472801dab..992a8820599ac 100644 --- a/tests/integration/api_actions_artifact_v4_test.go +++ b/tests/integration/api_actions_artifact_v4_test.go @@ -7,14 +7,21 @@ import ( "bytes" "crypto/sha256" "encoding/hex" + "encoding/json" "encoding/xml" + "fmt" "io" "net/http" "strings" "testing" "time" + auth_model "code.gitea.io/gitea/models/auth" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/storage" + api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/routers/api/actions" actions_service "code.gitea.io/gitea/services/actions" @@ -334,6 +341,93 @@ func TestActionsArtifactV4DownloadSingle(t *testing.T) { assert.Equal(t, body, resp.Body.String()) } +func TestActionsArtifactV4RunDownloadSinglePublicApi(t *testing.T) { + defer prepareTestEnvActionsArtifacts(t)() + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + session := loginUser(t, user.Name) + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) + + // confirm artifact upload via rest api + req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/run/792/artifacts?name=artifact-v4-download", repo.FullName()), nil). + AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + var listResp api.ActionArtifactsResponse + err := json.Unmarshal(resp.Body.Bytes(), &listResp) + assert.NoError(t, err) + assert.NotEmpty(t, listResp.Entries[0].ArchiveDownloadURL) + assert.Equal(t, "artifact-v4-download", listResp.Entries[0].Name) + + req = NewRequestWithBody(t, "GET", listResp.Entries[0].ArchiveDownloadURL, nil). + AddTokenAuth(token) + + resp = MakeRequest(t, req, http.StatusOK) + body := strings.Repeat("D", 1024) + assert.Equal(t, body, resp.Body.String()) +} + +func TestActionsArtifactV4DownloadSinglePublicApi(t *testing.T) { + defer prepareTestEnvActionsArtifacts(t)() + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + session := loginUser(t, user.Name) + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) + + // confirm artifact upload via rest api + req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts?name=artifact-v4-download", repo.FullName()), nil). + AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + var listResp api.ActionArtifactsResponse + err := json.Unmarshal(resp.Body.Bytes(), &listResp) + assert.NoError(t, err) + assert.NotEmpty(t, listResp.Entries[0].ArchiveDownloadURL) + assert.Equal(t, "artifact-v4-download", listResp.Entries[0].Name) + + req = NewRequestWithBody(t, "GET", listResp.Entries[0].ArchiveDownloadURL, nil). + AddTokenAuth(token) + + resp = MakeRequest(t, req, http.StatusOK) + body := strings.Repeat("D", 1024) + assert.Equal(t, body, resp.Body.String()) +} + +func TestActionsArtifactV4ListAndGetPublicApi(t *testing.T) { + defer prepareTestEnvActionsArtifacts(t)() + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + session := loginUser(t, user.Name) + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) + + // confirm artifact upload via rest api + req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts", repo.FullName()), nil). + AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + var listResp api.ActionArtifactsResponse + err := json.Unmarshal(resp.Body.Bytes(), &listResp) + assert.NoError(t, err) + + for _, artifact := range listResp.Entries { + assert.Contains(t, fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d", repo.FullName(), artifact.ID), artifact.URL) + assert.Contains(t, fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d/zip", repo.FullName(), artifact.ID), artifact.ArchiveDownloadURL) + req = NewRequestWithBody(t, "GET", listResp.Entries[0].URL, nil). + AddTokenAuth(token) + + resp = MakeRequest(t, req, http.StatusOK) + var artifactResp api.ActionArtifact + err := json.Unmarshal(resp.Body.Bytes(), &artifactResp) + assert.NoError(t, err) + + assert.Equal(t, artifact.ID, artifactResp.ID) + assert.Equal(t, artifact.Name, artifactResp.Name) + assert.Equal(t, artifact.SizeInBytes, artifactResp.SizeInBytes) + assert.Equal(t, artifact.URL, artifactResp.URL) + assert.Equal(t, artifact.ArchiveDownloadURL, artifactResp.ArchiveDownloadURL) + } +} + func TestActionsArtifactV4Delete(t *testing.T) { defer prepareTestEnvActionsArtifacts(t)() From db619c35f0714af11429a5f1bde2a3dca437df80 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 5 Feb 2025 21:37:34 +0100 Subject: [PATCH 13/41] use correct json lib --- tests/integration/api_actions_artifact_v4_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/integration/api_actions_artifact_v4_test.go b/tests/integration/api_actions_artifact_v4_test.go index 992a8820599ac..17a4c966f6b2c 100644 --- a/tests/integration/api_actions_artifact_v4_test.go +++ b/tests/integration/api_actions_artifact_v4_test.go @@ -7,7 +7,6 @@ import ( "bytes" "crypto/sha256" "encoding/hex" - "encoding/json" "encoding/xml" "fmt" "io" @@ -16,6 +15,8 @@ import ( "testing" "time" + "code.gitea.io/gitea/modules/json" + auth_model "code.gitea.io/gitea/models/auth" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" From f4b0811553ae85b67e712b22a517f5baf03e8777 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 5 Feb 2025 21:37:47 +0100 Subject: [PATCH 14/41] use http.ServeContent --- modules/actions/artifacts.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/actions/artifacts.go b/modules/actions/artifacts.go index 17d7a9014684c..7f0847b4ab0f0 100644 --- a/modules/actions/artifacts.go +++ b/modules/actions/artifacts.go @@ -1,7 +1,6 @@ package actions import ( - "io" "net/http" actions_model "code.gitea.io/gitea/models/actions" @@ -33,7 +32,7 @@ func DownloadArtifactV4Fallback(ctx *context.Base, art *actions_model.ActionArti return err } defer f.Close() - _, _ = io.Copy(ctx.Resp, f) + http.ServeContent(ctx.Resp, ctx.Req, art.ArtifactName+".zip", art.CreatedUnix.AsLocalTime(), f) return nil } From c6f1557126633a32c18cd02211d64734cfd64c97 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 5 Feb 2025 22:21:21 +0100 Subject: [PATCH 15/41] fix tests --- models/fixtures/action_artifact.yml | 8 ++++---- models/fixtures/action_run_job.yml | 10 +++++----- routers/api/v1/repo/action.go | 1 + tests/integration/api_actions_artifact_v4_test.go | 15 ++++++++++++--- 4 files changed, 22 insertions(+), 12 deletions(-) diff --git a/models/fixtures/action_artifact.yml b/models/fixtures/action_artifact.yml index 2c51c11ebd079..7ed80fb436d3b 100644 --- a/models/fixtures/action_artifact.yml +++ b/models/fixtures/action_artifact.yml @@ -3,7 +3,7 @@ run_id: 791 runner_id: 1 repo_id: 4 - owner_id: 1 + owner_id: 5 commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 storage_path: "26/1/1712166500347189545.chunk" file_size: 1024 @@ -21,7 +21,7 @@ run_id: 791 runner_id: 1 repo_id: 4 - owner_id: 1 + owner_id: 5 commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 storage_path: "26/19/1712348022422036662.chunk" file_size: 1024 @@ -39,7 +39,7 @@ run_id: 791 runner_id: 1 repo_id: 4 - owner_id: 1 + owner_id: 5 commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 storage_path: "26/20/1712348022423431524.chunk" file_size: 1024 @@ -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 diff --git a/models/fixtures/action_run_job.yml b/models/fixtures/action_run_job.yml index 8837e6ec2d80d..046cc0ba093d7 100644 --- a/models/fixtures/action_run_job.yml +++ b/models/fixtures/action_run_job.yml @@ -2,7 +2,7 @@ id: 192 run_id: 791 repo_id: 4 - owner_id: 1 + owner_id: 5 commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 is_fork_pull_request: 0 name: job_2 @@ -16,7 +16,7 @@ id: 193 run_id: 792 repo_id: 4 - owner_id: 1 + owner_id: 5 commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 is_fork_pull_request: 0 name: job_2 @@ -30,7 +30,7 @@ id: 194 run_id: 793 repo_id: 4 - owner_id: 1 + owner_id: 5 commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 is_fork_pull_request: 0 name: job1 (1) @@ -44,7 +44,7 @@ id: 195 run_id: 793 repo_id: 4 - owner_id: 1 + owner_id: 5 commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 is_fork_pull_request: 0 name: job1 (2) @@ -58,7 +58,7 @@ id: 196 run_id: 793 repo_id: 4 - owner_id: 1 + owner_id: 5 commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 is_fork_pull_request: 0 name: job2 diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index c3d2b1a3b9eb6..119e3aa8761c9 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -866,6 +866,7 @@ func DownloadArtifactRaw(ctx *context.APIContext) { ctx.Error(http.StatusInternalServerError, err.Error(), err) return } + return } // v3 not supported due to not having one unique id ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) diff --git a/tests/integration/api_actions_artifact_v4_test.go b/tests/integration/api_actions_artifact_v4_test.go index 17a4c966f6b2c..2861603d0238b 100644 --- a/tests/integration/api_actions_artifact_v4_test.go +++ b/tests/integration/api_actions_artifact_v4_test.go @@ -351,7 +351,7 @@ func TestActionsArtifactV4RunDownloadSinglePublicApi(t *testing.T) { token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) // confirm artifact upload via rest api - req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/run/792/artifacts?name=artifact-v4-download", repo.FullName()), nil). + req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/runs/792/artifacts?name=artifact-v4-download", repo.FullName()), nil). AddTokenAuth(token) resp := MakeRequest(t, req, http.StatusOK) var listResp api.ActionArtifactsResponse @@ -363,7 +363,12 @@ func TestActionsArtifactV4RunDownloadSinglePublicApi(t *testing.T) { req = NewRequestWithBody(t, "GET", listResp.Entries[0].ArchiveDownloadURL, nil). AddTokenAuth(token) + resp = MakeRequest(t, req, http.StatusFound) + + req = NewRequestWithBody(t, "GET", resp.Header().Get("Location"), nil). + AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusOK) + body := strings.Repeat("D", 1024) assert.Equal(t, body, resp.Body.String()) } @@ -389,6 +394,10 @@ func TestActionsArtifactV4DownloadSinglePublicApi(t *testing.T) { req = NewRequestWithBody(t, "GET", listResp.Entries[0].ArchiveDownloadURL, nil). AddTokenAuth(token) + resp = MakeRequest(t, req, http.StatusFound) + + req = NewRequestWithBody(t, "GET", resp.Header().Get("Location"), nil). + AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusOK) body := strings.Repeat("D", 1024) assert.Equal(t, body, resp.Body.String()) @@ -411,8 +420,8 @@ func TestActionsArtifactV4ListAndGetPublicApi(t *testing.T) { assert.NoError(t, err) for _, artifact := range listResp.Entries { - assert.Contains(t, fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d", repo.FullName(), artifact.ID), artifact.URL) - assert.Contains(t, fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d/zip", repo.FullName(), artifact.ID), artifact.ArchiveDownloadURL) + assert.Contains(t, artifact.URL, fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d", repo.FullName(), artifact.ID)) + assert.Contains(t, artifact.ArchiveDownloadURL, fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d/zip", repo.FullName(), artifact.ID)) req = NewRequestWithBody(t, "GET", listResp.Entries[0].URL, nil). AddTokenAuth(token) From dfae4847566680c95c36859c66c9273e9b92eea3 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 5 Feb 2025 22:23:04 +0100 Subject: [PATCH 16/41] use MakeAbsoluteURL --- routers/api/v1/repo/action.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index 119e3aa8761c9..6ff671e95e44c 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -8,12 +8,12 @@ import ( "fmt" "net/http" "net/url" - "strings" actions_model "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/models/db" secret_model "code.gitea.io/gitea/models/secret" "code.gitea.io/gitea/modules/actions" + "code.gitea.io/gitea/modules/httplib" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" @@ -809,7 +809,7 @@ func DownloadArtifact(ctx *context.APIContext) { return } repoName := ctx.Repo.Repository.FullName() - url := strings.TrimSuffix(setting.AppURL, "/") + "/api/v1/repos/" + repoName + "/actions/artifacts/" + fmt.Sprintf("%d", art.ID) + "/zip/raw" + url := httplib.MakeAbsoluteURL(ctx, setting.AppSubURL+"/api/v1/repos/"+repoName+"/actions/artifacts/"+fmt.Sprintf("%d", art.ID)+"/zip/raw") ctx.Redirect(url, http.StatusFound) return } From ae2202da9e0bd1272cdab8ff56db1d55a010da0b Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 5 Feb 2025 22:26:32 +0100 Subject: [PATCH 17/41] add copyright header --- modules/actions/artifacts.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modules/actions/artifacts.go b/modules/actions/artifacts.go index 7f0847b4ab0f0..4d074435efc8f 100644 --- a/modules/actions/artifacts.go +++ b/modules/actions/artifacts.go @@ -1,3 +1,6 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + package actions import ( From b4c318c3e0f12c59dbc9ec712163880590195eb6 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 5 Feb 2025 22:41:14 +0100 Subject: [PATCH 18/41] more tests fix old type from my side --- routers/api/actions/artifactsv4.go | 2 +- .../api_actions_artifact_v4_test.go | 66 ++++++++++++++++++- 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index 8917a7a8a2320..d29754b6e989c 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -25,7 +25,7 @@ package actions // 1.3. Continue Upload Zip Content to Blobstorage (unauthenticated request), repeat until everything is uploaded // PUT: http://localhost:3000/twirp/github.actions.results.api.v1.ArtifactService/UploadArtifact?sig=mO7y35r4GyjN7fwg0DTv3-Fv1NDXD84KLEgLpoPOtDI=&expires=2024-01-23+21%3A48%3A37.20833956+%2B0100+CET&artifactName=test&taskID=75&comp=appendBlock // 1.4. BlockList xml payload to Blobstorage (unauthenticated request) -// Files of about 800MB are parallel in parallel and / or out of order, this file is needed to enshure the correct order +// Files of about 800MB are parallel in parallel and / or out of order, this file is needed to ensure the correct order // PUT: http://localhost:3000/twirp/github.actions.results.api.v1.ArtifactService/UploadArtifact?sig=mO7y35r4GyjN7fwg0DTv3-Fv1NDXD84KLEgLpoPOtDI=&expires=2024-01-23+21%3A48%3A37.20833956+%2B0100+CET&artifactName=test&taskID=75&comp=blockList // Request // diff --git a/tests/integration/api_actions_artifact_v4_test.go b/tests/integration/api_actions_artifact_v4_test.go index 2861603d0238b..43af0cae7795c 100644 --- a/tests/integration/api_actions_artifact_v4_test.go +++ b/tests/integration/api_actions_artifact_v4_test.go @@ -350,7 +350,7 @@ func TestActionsArtifactV4RunDownloadSinglePublicApi(t *testing.T) { session := loginUser(t, user.Name) token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) - // confirm artifact upload via rest api + // confirm artifact can be listed and found by name req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/runs/792/artifacts?name=artifact-v4-download", repo.FullName()), nil). AddTokenAuth(token) resp := MakeRequest(t, req, http.StatusOK) @@ -360,11 +360,13 @@ func TestActionsArtifactV4RunDownloadSinglePublicApi(t *testing.T) { assert.NotEmpty(t, listResp.Entries[0].ArchiveDownloadURL) assert.Equal(t, "artifact-v4-download", listResp.Entries[0].Name) + // confirm artifact blob storage url can be retrieved req = NewRequestWithBody(t, "GET", listResp.Entries[0].ArchiveDownloadURL, nil). AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusFound) + // confirm artifact can be downloaded and has expected content req = NewRequestWithBody(t, "GET", resp.Header().Get("Location"), nil). AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusOK) @@ -381,7 +383,7 @@ func TestActionsArtifactV4DownloadSinglePublicApi(t *testing.T) { session := loginUser(t, user.Name) token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) - // confirm artifact upload via rest api + // confirm artifact can be listed and found by name req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts?name=artifact-v4-download", repo.FullName()), nil). AddTokenAuth(token) resp := MakeRequest(t, req, http.StatusOK) @@ -391,11 +393,13 @@ func TestActionsArtifactV4DownloadSinglePublicApi(t *testing.T) { assert.NotEmpty(t, listResp.Entries[0].ArchiveDownloadURL) assert.Equal(t, "artifact-v4-download", listResp.Entries[0].Name) + // confirm artifact blob storage url can be retrieved req = NewRequestWithBody(t, "GET", listResp.Entries[0].ArchiveDownloadURL, nil). AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusFound) + // confirm artifact can be downloaded and has expected content req = NewRequestWithBody(t, "GET", resp.Header().Get("Location"), nil). AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusOK) @@ -411,7 +415,7 @@ func TestActionsArtifactV4ListAndGetPublicApi(t *testing.T) { session := loginUser(t, user.Name) token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) - // confirm artifact upload via rest api + // confirm artifact can be listed req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts", repo.FullName()), nil). AddTokenAuth(token) resp := MakeRequest(t, req, http.StatusOK) @@ -438,6 +442,62 @@ func TestActionsArtifactV4ListAndGetPublicApi(t *testing.T) { } } +func TestActionsArtifactV4GetArtifactMismatchedRepoOwnerNotFound(t *testing.T) { + defer prepareTestEnvActionsArtifacts(t)() + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + session := loginUser(t, user.Name) + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) + + // confirm artifacts of wrong owner or repo is not visible + req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d", repo.FullName(), 22), nil). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusNotFound) +} + +func TestActionsArtifactV4DownloadArtifactMismatchedRepoOwnerNotFound(t *testing.T) { + defer prepareTestEnvActionsArtifacts(t)() + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + session := loginUser(t, user.Name) + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) + + // confirm artifacts of wrong owner or repo is not visible + req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d/zip", repo.FullName(), 22), nil). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusNotFound) +} + +func TestActionsArtifactV4DownloadArtifactCorrectRepoOwnerFound(t *testing.T) { + defer prepareTestEnvActionsArtifacts(t)() + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + session := loginUser(t, user.Name) + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) + + // confirm artifacts of wrong owner or repo is not visible + req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d/zip", repo.FullName(), 22), nil). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusFound) +} + +func TestActionsArtifactV4DownloadRawArtifactMismatchedRepoOwnerNotFound(t *testing.T) { + defer prepareTestEnvActionsArtifacts(t)() + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + session := loginUser(t, user.Name) + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) + + // confirm artifacts of wrong owner or repo is not visible + req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d/zip/raw", repo.FullName(), 22), nil). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusNotFound) +} + func TestActionsArtifactV4Delete(t *testing.T) { defer prepareTestEnvActionsArtifacts(t)() From 5e3c79d6a93fdcce9ff11958a5ba0ac21163c8c7 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Wed, 5 Feb 2025 22:45:19 +0100 Subject: [PATCH 19/41] fmt code --- tests/integration/api_actions_artifact_v4_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/integration/api_actions_artifact_v4_test.go b/tests/integration/api_actions_artifact_v4_test.go index 43af0cae7795c..aee75e6354a53 100644 --- a/tests/integration/api_actions_artifact_v4_test.go +++ b/tests/integration/api_actions_artifact_v4_test.go @@ -15,12 +15,11 @@ import ( "testing" "time" - "code.gitea.io/gitea/modules/json" - auth_model "code.gitea.io/gitea/models/auth" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/storage" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/routers/api/actions" From b1b0ef3a49213dca828213a2e9175a5ff5cd3ed2 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Thu, 6 Feb 2025 11:54:58 +0100 Subject: [PATCH 20/41] add DeleteArtifact api --- routers/api/v1/api.go | 5 +- routers/api/v1/repo/action.go | 48 ++++++++++++++++++ templates/swagger/v1_json.tmpl | 44 ++++++++++++++++ .../api_actions_artifact_v4_test.go | 50 ++++++++++++++++++- 4 files changed, 145 insertions(+), 2 deletions(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 709ef255955d9..3517c780d31ec 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1237,7 +1237,10 @@ func Routes() *web.Router { m.Get("/tasks", repo.ListActionTasks) m.Get("/runs/{run}/artifacts", repo.GetArtifactsOfRun) m.Get("/artifacts", repo.GetArtifacts) - m.Get("/artifacts/{artifact_id}", repo.GetArtifact) + m.Group("/artifacts/{artifact_id}", func() { + m.Get("", repo.GetArtifact) + m.Delete("", reqRepoWriter(unit.TypeActions), repo.DeleteArtifact) + }) m.Get("/artifacts/{artifact_id}/zip", repo.DownloadArtifact) m.Get("/artifacts/{artifact_id}/zip/raw", repo.DownloadArtifactRaw) }, reqRepoReader(unit.TypeActions), context.ReferencesGitRepo(true)) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index 6ff671e95e44c..5016a9e66e4cf 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -756,6 +756,54 @@ func GetArtifact(ctx *context.APIContext) { ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) } +// DeleteArtifact Deletes a specific artifact for a workflow run. +func DeleteArtifact(ctx *context.APIContext) { + // swagger:operation DELETE /repos/{owner}/{repo}/actions/artifacts/{artifact_id} repository deleteArtifact + // --- + // summary: Deletes a specific artifact for a workflow run + // produces: + // - application/json + // parameters: + // - name: owner + // in: path + // description: name of the owner + // type: string + // required: true + // - name: repo + // in: path + // description: name of the repository + // type: string + // required: true + // - name: artifact_id + // in: path + // description: id of the artifact + // type: string + // required: true + // responses: + // "204": + // description: "No Content" + // "400": + // "$ref": "#/responses/error" + // "404": + // "$ref": "#/responses/notFound" + + art, ok := getArtifactByID(ctx) + if !ok { + return + } + + if actions.IsArtifactV4(art) { + if err := actions_model.SetArtifactNeedDelete(ctx, art.RunID, art.ArtifactName); err != nil { + ctx.Error(http.StatusInternalServerError, err.Error(), err) + return + } + ctx.Status(http.StatusNoContent) + return + } + // v3 not supported due to not having one unique id + ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) +} + // DownloadArtifact Downloads a specific artifact for a workflow run redirects to blob url. func DownloadArtifact(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/actions/artifacts/{artifact_id}/zip repository downloadArtifact diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index afcd1942c03cf..f107cf064a75d 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -4002,6 +4002,50 @@ "$ref": "#/responses/notFound" } } + }, + "delete": { + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Deletes a specific artifact for a workflow run", + "operationId": "deleteArtifact", + "parameters": [ + { + "type": "string", + "description": "name of the owner", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repository", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "id of the artifact", + "name": "artifact_id", + "in": "path", + "required": true + } + ], + "responses": { + "204": { + "description": "No Content" + }, + "400": { + "$ref": "#/responses/error" + }, + "404": { + "$ref": "#/responses/notFound" + } + } } }, "/repos/{owner}/{repo}/actions/artifacts/{artifact_id}/zip": { diff --git a/tests/integration/api_actions_artifact_v4_test.go b/tests/integration/api_actions_artifact_v4_test.go index aee75e6354a53..268ccb6e08c77 100644 --- a/tests/integration/api_actions_artifact_v4_test.go +++ b/tests/integration/api_actions_artifact_v4_test.go @@ -477,7 +477,7 @@ func TestActionsArtifactV4DownloadArtifactCorrectRepoOwnerFound(t *testing.T) { session := loginUser(t, user.Name) token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) - // confirm artifacts of wrong owner or repo is not visible + // confirm artifacts of correct owner and repo is visible req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d/zip", repo.FullName(), 22), nil). AddTokenAuth(token) MakeRequest(t, req, http.StatusFound) @@ -514,3 +514,51 @@ func TestActionsArtifactV4Delete(t *testing.T) { protojson.Unmarshal(resp.Body.Bytes(), &deleteResp) assert.True(t, deleteResp.Ok) } + +func TestActionsArtifactV4DeletePublicApi(t *testing.T) { + defer prepareTestEnvActionsArtifacts(t)() + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + session := loginUser(t, user.Name) + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) + + // confirm artifacts exists + req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d", repo.FullName(), 22), nil). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusOK) + + // delete artifact by id + req = NewRequestWithBody(t, "DELETE", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d", repo.FullName(), 22), nil). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusNoContent) + + // confirm artifacts has been deleted + req = NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d", repo.FullName(), 22), nil). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusNotFound) +} + +func TestActionsArtifactV4DeletePublicApiNotAllowedReadScope(t *testing.T) { + defer prepareTestEnvActionsArtifacts(t)() + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + session := loginUser(t, user.Name) + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeReadRepository) + + // confirm artifacts exists + req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d", repo.FullName(), 22), nil). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusOK) + + // try delete artifact by id + req = NewRequestWithBody(t, "DELETE", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d", repo.FullName(), 22), nil). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusForbidden) + + // confirm artifacts has not been deleted + req = NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d", repo.FullName(), 22), nil). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusOK) +} From 59cf96407b378c74107cf1158e805c2c462ad1eb Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Mon, 10 Feb 2025 00:41:13 +0100 Subject: [PATCH 21/41] fix some swagger docu issues --- routers/api/v1/repo/action.go | 17 +++++++++- templates/swagger/v1_json.tmpl | 58 ++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index ca3bddb4ce2ec..b9ea9719269ad 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -885,7 +885,7 @@ func (a ActionWorkflow) EnableWorkflow(ctx *context.APIContext) { // GetArtifacts Lists all artifacts for a repository. func GetArtifactsOfRun(ctx *context.APIContext) { - // swagger:operation GET /repos/{owner}/{repo}/actions/artifacts repository getArtifactsOfRun + // swagger:operation GET /repos/{owner}/{repo}/actions/runs/{run}/artifacts repository getArtifactsOfRun // --- // summary: Lists all artifacts for a repository run // produces: @@ -901,6 +901,16 @@ func GetArtifactsOfRun(ctx *context.APIContext) { // description: name of the repository // type: string // required: true + // - name: run + // in: path + // description: runid of the workflow run + // type: integer + // required: true + // - name: name + // in: query + // description: name of the artifact + // type: string + // required: false // responses: // "200": // "$ref": "#/responses/ArtifactsList" @@ -962,6 +972,11 @@ func GetArtifacts(ctx *context.APIContext) { // description: name of the repository // type: string // required: true + // - name: name + // in: query + // description: name of the artifact + // type: string + // required: false // responses: // "200": // "$ref": "#/responses/ArtifactsList" diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 9da4cd9025dbb..7bf73c01b5708 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -3943,6 +3943,12 @@ "name": "repo", "in": "path", "required": true + }, + { + "type": "string", + "description": "name of the artifact", + "name": "name", + "in": "query" } ], "responses": { @@ -4173,6 +4179,58 @@ } } }, + "/repos/{owner}/{repo}/actions/runs/{run}/artifacts": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "repository" + ], + "summary": "Lists all artifacts for a repository run", + "operationId": "getArtifactsOfRun", + "parameters": [ + { + "type": "string", + "description": "name of the owner", + "name": "owner", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the repository", + "name": "repo", + "in": "path", + "required": true + }, + { + "type": "integer", + "description": "runid of the workflow run", + "name": "run", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "name of the artifact", + "name": "name", + "in": "query" + } + ], + "responses": { + "200": { + "$ref": "#/responses/ArtifactsList" + }, + "400": { + "$ref": "#/responses/error" + }, + "404": { + "$ref": "#/responses/notFound" + } + } + } + }, "/repos/{owner}/{repo}/actions/secrets": { "get": { "produces": [ From 4721100ab7f6260c0596c82fea7bc2941d0e944c Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Mon, 10 Feb 2025 00:34:15 +0100 Subject: [PATCH 22/41] use repository apiurl method --- routers/api/v1/repo/action.go | 11 +++-------- services/convert/convert.go | 6 +++--- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index b9ea9719269ad..cb856db4614b8 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -936,14 +936,12 @@ func GetArtifactsOfRun(ctx *context.APIContext) { return } - repoName := ctx.Repo.Repository.FullName() - res := new(api.ActionArtifactsResponse) res.TotalCount = total res.Entries = make([]*api.ActionArtifact, len(artifacts)) for i := range artifacts { - convertedArtifact, err := convert.ToActionArtifact(ctx, repoName, artifacts[i]) + convertedArtifact, err := convert.ToActionArtifact(ctx, ctx.Repo.Repository, artifacts[i]) if err != nil { ctx.Error(http.StatusInternalServerError, "ToActionArtifact", err) return @@ -999,14 +997,12 @@ func GetArtifacts(ctx *context.APIContext) { return } - repoName := ctx.Repo.Repository.FullName() - res := new(api.ActionArtifactsResponse) res.TotalCount = total res.Entries = make([]*api.ActionArtifact, len(artifacts)) for i := range artifacts { - convertedArtifact, err := convert.ToActionArtifact(ctx, repoName, artifacts[i]) + convertedArtifact, err := convert.ToActionArtifact(ctx, ctx.Repo.Repository, artifacts[i]) if err != nil { ctx.Error(http.StatusInternalServerError, "ToActionArtifact", err) return @@ -1054,8 +1050,7 @@ func GetArtifact(ctx *context.APIContext) { } if actions.IsArtifactV4(art) { - repoName := ctx.Repo.Repository.FullName() - convertedArtifact, err := convert.ToActionArtifact(ctx, repoName, art) + convertedArtifact, err := convert.ToActionArtifact(ctx, ctx.Repo.Repository, art) if err != nil { ctx.Error(http.StatusInternalServerError, "ToActionArtifact", err) return diff --git a/services/convert/convert.go b/services/convert/convert.go index 4b816333d7e9c..37372f790fea6 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -19,12 +19,12 @@ import ( "code.gitea.io/gitea/models/organization" "code.gitea.io/gitea/models/perm" access_model "code.gitea.io/gitea/models/perm/access" + "code.gitea.io/gitea/models/repo" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/git" - "code.gitea.io/gitea/modules/httplib" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" @@ -231,8 +231,8 @@ func ToActionTask(ctx context.Context, t *actions_model.ActionTask) (*api.Action } // ToActionArtifact convert a actions_model.ActionArtifact to an api.ActionArtifact -func ToActionArtifact(ctx context.Context, repoName string, art *actions_model.ActionArtifact) (*api.ActionArtifact, error) { - url := httplib.MakeAbsoluteURL(ctx, setting.AppSubURL+"/api/v1/repos/"+repoName+"/actions/artifacts/"+fmt.Sprintf("%d", art.ID)) +func ToActionArtifact(ctx context.Context, repo *repo.Repository, art *actions_model.ActionArtifact) (*api.ActionArtifact, error) { + url := fmt.Sprintf("%s/actions/artifacts/%d", repo.APIURL(), art.ID) return &api.ActionArtifact{ ID: art.ID, From 7a35443a81c4248fd0338e539af034c96cd8beb7 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Mon, 10 Feb 2025 13:20:50 +0100 Subject: [PATCH 23/41] remove unused ctx --- routers/api/v1/repo/action.go | 6 +++--- services/convert/convert.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index cb856db4614b8..64b65323a6a07 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -941,7 +941,7 @@ func GetArtifactsOfRun(ctx *context.APIContext) { res.Entries = make([]*api.ActionArtifact, len(artifacts)) for i := range artifacts { - convertedArtifact, err := convert.ToActionArtifact(ctx, ctx.Repo.Repository, artifacts[i]) + convertedArtifact, err := convert.ToActionArtifact(ctx.Repo.Repository, artifacts[i]) if err != nil { ctx.Error(http.StatusInternalServerError, "ToActionArtifact", err) return @@ -1002,7 +1002,7 @@ func GetArtifacts(ctx *context.APIContext) { res.Entries = make([]*api.ActionArtifact, len(artifacts)) for i := range artifacts { - convertedArtifact, err := convert.ToActionArtifact(ctx, ctx.Repo.Repository, artifacts[i]) + convertedArtifact, err := convert.ToActionArtifact(ctx.Repo.Repository, artifacts[i]) if err != nil { ctx.Error(http.StatusInternalServerError, "ToActionArtifact", err) return @@ -1050,7 +1050,7 @@ func GetArtifact(ctx *context.APIContext) { } if actions.IsArtifactV4(art) { - convertedArtifact, err := convert.ToActionArtifact(ctx, ctx.Repo.Repository, art) + convertedArtifact, err := convert.ToActionArtifact(ctx.Repo.Repository, art) if err != nil { ctx.Error(http.StatusInternalServerError, "ToActionArtifact", err) return diff --git a/services/convert/convert.go b/services/convert/convert.go index 37372f790fea6..41fb02c5f2c48 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -231,7 +231,7 @@ func ToActionTask(ctx context.Context, t *actions_model.ActionTask) (*api.Action } // ToActionArtifact convert a actions_model.ActionArtifact to an api.ActionArtifact -func ToActionArtifact(ctx context.Context, repo *repo.Repository, art *actions_model.ActionArtifact) (*api.ActionArtifact, error) { +func ToActionArtifact(repo *repo.Repository, art *actions_model.ActionArtifact) (*api.ActionArtifact, error) { url := fmt.Sprintf("%s/actions/artifacts/%d", repo.APIURL(), art.ID) return &api.ActionArtifact{ From 44bde87a63b9f39a237b9be25adf65d90cdd7fd1 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Mon, 10 Feb 2025 20:30:29 +0100 Subject: [PATCH 24/41] fix merge --- routers/api/v1/repo/action.go | 1 + 1 file changed, 1 insertion(+) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index a05f180fa98ff..317ea025df1d6 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -5,6 +5,7 @@ package repo import ( "errors" + "fmt" "net/http" "net/url" "strings" From a18c8ddada93e17023925ca357a3452a9feddc0a Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Mon, 10 Feb 2025 21:48:51 +0100 Subject: [PATCH 25/41] fix private repository artifact download redirect --- routers/api/v1/api.go | 4 +- routers/api/v1/repo/action.go | 79 ++++++++++++++++++++-------------- templates/swagger/v1_json.tmpl | 46 -------------------- 3 files changed, 50 insertions(+), 79 deletions(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index de9102fb92874..1f75bae540d0e 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1248,7 +1248,6 @@ func Routes() *web.Router { m.Delete("", reqRepoWriter(unit.TypeActions), repo.DeleteArtifact) }) m.Get("/artifacts/{artifact_id}/zip", repo.DownloadArtifact) - m.Get("/artifacts/{artifact_id}/zip/raw", repo.DownloadArtifactRaw) }, reqRepoReader(unit.TypeActions), context.ReferencesGitRepo(true)) m.Group("/keys", func() { m.Combo("").Get(repo.ListDeployKeys). @@ -1409,6 +1408,9 @@ func Routes() *web.Router { }, repoAssignment(), checkTokenPublicOnly()) }, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryRepository)) + // Artifacts direct download endpoint authenticates via signed url + m.Get("/repos/{username}/{reponame}/actions/artifacts/{artifact_id}/zip/raw", repo.DownloadArtifactRaw) + // Notifications (requires notifications scope) m.Group("/repos", func() { m.Group("/{username}/{reponame}", func() { diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index 317ea025df1d6..afeaa4ca6244e 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -4,17 +4,24 @@ package repo import ( + "crypto/hmac" + "crypto/sha256" + "encoding/base64" "errors" "fmt" "net/http" "net/url" "strings" + "time" + + go_context "context" actions_model "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/models/db" secret_model "code.gitea.io/gitea/models/secret" "code.gitea.io/gitea/modules/actions" "code.gitea.io/gitea/modules/httplib" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" @@ -1084,6 +1091,21 @@ func DeleteArtifact(ctx *context.APIContext) { ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) } +func buildSignature(endp, expires string, artifactID int64) []byte { + mac := hmac.New(sha256.New, setting.GetGeneralTokenSigningSecret()) + mac.Write([]byte(endp)) + mac.Write([]byte(expires)) + mac.Write([]byte(fmt.Sprint(artifactID))) + return mac.Sum(nil) +} + +func buildSigURL(ctx go_context.Context, endp string, artifactID int64) string { + expires := time.Now().Add(60 * time.Minute).Format("2006-01-02 15:04:05.999999999 -0700 MST") + uploadURL := strings.TrimSuffix(httplib.GuessCurrentAppURL(ctx), "/") + + "/" + endp + "?sig=" + base64.URLEncoding.EncodeToString(buildSignature(endp, expires, artifactID)) + "&expires=" + url.QueryEscape(expires) + return uploadURL +} + // DownloadArtifact Downloads a specific artifact for a workflow run redirects to blob url. func DownloadArtifact(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/actions/artifacts/{artifact_id}/zip repository downloadArtifact @@ -1136,9 +1158,9 @@ func DownloadArtifact(ctx *context.APIContext) { ctx.Error(http.StatusInternalServerError, err.Error(), err) return } - repoName := ctx.Repo.Repository.FullName() - url := httplib.MakeAbsoluteURL(ctx, setting.AppSubURL+"/api/v1/repos/"+repoName+"/actions/artifacts/"+fmt.Sprintf("%d", art.ID)+"/zip/raw") - ctx.Redirect(url, http.StatusFound) + + rurl := buildSigURL(ctx, "api/v1/repos/"+url.PathEscape(ctx.Repo.Repository.OwnerName)+"/"+url.PathEscape(ctx.Repo.Repository.Name)+"/actions/artifacts/"+fmt.Sprintf("%d", art.ID)+"/zip/raw", art.ID) + ctx.Redirect(rurl, http.StatusFound) return } // v3 not supported due to not having one unique id @@ -1147,34 +1169,26 @@ func DownloadArtifact(ctx *context.APIContext) { // DownloadArtifactRaw Downloads a specific artifact for a workflow run directly. func DownloadArtifactRaw(ctx *context.APIContext) { - // swagger:operation GET /repos/{owner}/{repo}/actions/artifacts/{artifact_id}/zip/raw repository downloadArtifactRaw - // --- - // summary: Downloads a specific artifact for a workflow run directly - // produces: - // - application/json - // parameters: - // - name: owner - // in: path - // description: name of the owner - // type: string - // required: true - // - name: repo - // in: path - // description: name of the repository - // type: string - // required: true - // - name: artifact_id - // in: path - // description: id of the artifact - // type: string - // required: true - // responses: - // "200": - // description: the artifact content - // "400": - // "$ref": "#/responses/error" - // "404": - // "$ref": "#/responses/notFound" + username := ctx.PathParam("username") + reponame := ctx.PathParam("reponame") + artifactID := ctx.PathParamInt64("artifact_id") + sig := ctx.Req.URL.Query().Get("sig") + expires := ctx.Req.URL.Query().Get("expires") + dsig, _ := base64.URLEncoding.DecodeString(sig) + + endp := "api/v1/repos/" + url.PathEscape(username) + "/" + url.PathEscape(reponame) + "/actions/artifacts/" + fmt.Sprintf("%d", artifactID) + "/zip/raw" + expecedsig := buildSignature(endp, expires, artifactID) + if !hmac.Equal(dsig, expecedsig) { + log.Error("Error unauthorized") + ctx.Error(http.StatusUnauthorized, "Error unauthorized", nil) + return + } + t, err := time.Parse("2006-01-02 15:04:05.999999999 -0700 MST", expires) + if err != nil || t.Before(time.Now()) { + log.Error("Error link expired") + ctx.Error(http.StatusUnauthorized, "Error link expired", nil) + return + } art, ok := getArtifactByID(ctx) if !ok { @@ -1210,7 +1224,8 @@ func getArtifactByID(ctx *context.APIContext) (*actions_model.ActionArtifact, bo return nil, false } // if artifacts status is not uploaded-confirmed, treat it as not found - if !ok || art.RepoID != ctx.Repo.Repository.ID || art.OwnerID != ctx.Repo.Repository.OwnerID || art.Status != int64(actions_model.ArtifactStatusUploadConfirmed) && art.Status != int64(actions_model.ArtifactStatusExpired) { + // ctx.Repo.Repository is nil for the raw download endpoint that checked this already + if !ok || ctx.Repo != nil && ctx.Repo.Repository != nil && (art.RepoID != ctx.Repo.Repository.ID || art.OwnerID != ctx.Repo.Repository.OwnerID) || art.Status != int64(actions_model.ArtifactStatusUploadConfirmed) && art.Status != int64(actions_model.ArtifactStatusExpired) { ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) return nil, false } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index b908d2544bb5a..091ede2ff9a05 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -4100,52 +4100,6 @@ } } }, - "/repos/{owner}/{repo}/actions/artifacts/{artifact_id}/zip/raw": { - "get": { - "produces": [ - "application/json" - ], - "tags": [ - "repository" - ], - "summary": "Downloads a specific artifact for a workflow run directly", - "operationId": "downloadArtifactRaw", - "parameters": [ - { - "type": "string", - "description": "name of the owner", - "name": "owner", - "in": "path", - "required": true - }, - { - "type": "string", - "description": "name of the repository", - "name": "repo", - "in": "path", - "required": true - }, - { - "type": "string", - "description": "id of the artifact", - "name": "artifact_id", - "in": "path", - "required": true - } - ], - "responses": { - "200": { - "description": "the artifact content" - }, - "400": { - "$ref": "#/responses/error" - }, - "404": { - "$ref": "#/responses/notFound" - } - } - } - }, "/repos/{owner}/{repo}/actions/runners/registration-token": { "get": { "produces": [ From f8ce2f6bbf25280fc2918f6c47fe87bb246b9963 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Mon, 10 Feb 2025 21:56:18 +0100 Subject: [PATCH 26/41] fmt --- routers/api/v1/repo/action.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index afeaa4ca6244e..3333270d9aed1 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -4,6 +4,7 @@ package repo import ( + go_context "context" "crypto/hmac" "crypto/sha256" "encoding/base64" @@ -14,8 +15,6 @@ import ( "strings" "time" - go_context "context" - actions_model "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/models/db" secret_model "code.gitea.io/gitea/models/secret" From 5b76dbef328d0d2a14d2b7c28a4fea409a763606 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Mon, 10 Feb 2025 22:14:27 +0100 Subject: [PATCH 27/41] fix import --- services/convert/convert.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/services/convert/convert.go b/services/convert/convert.go index 41fb02c5f2c48..17d1f04adb313 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -19,7 +19,6 @@ import ( "code.gitea.io/gitea/models/organization" "code.gitea.io/gitea/models/perm" access_model "code.gitea.io/gitea/models/perm/access" - "code.gitea.io/gitea/models/repo" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" @@ -231,7 +230,7 @@ func ToActionTask(ctx context.Context, t *actions_model.ActionTask) (*api.Action } // ToActionArtifact convert a actions_model.ActionArtifact to an api.ActionArtifact -func ToActionArtifact(repo *repo.Repository, art *actions_model.ActionArtifact) (*api.ActionArtifact, error) { +func ToActionArtifact(repo *repo_model.Repository, art *actions_model.ActionArtifact) (*api.ActionArtifact, error) { url := fmt.Sprintf("%s/actions/artifacts/%d", repo.APIURL(), art.ID) return &api.ActionArtifact{ From 84850441e3d6a1807aead92c4391748fb2b552aa Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Tue, 11 Feb 2025 16:59:40 +0100 Subject: [PATCH 28/41] add download test for private repo and remove the token to the redirect --- models/fixtures/action_artifact.yml | 18 +++++ .../api_actions_artifact_v4_test.go | 66 +++++++++++++++++-- 2 files changed, 80 insertions(+), 4 deletions(-) diff --git a/models/fixtures/action_artifact.yml b/models/fixtures/action_artifact.yml index 7ed80fb436d3b..665e3268a42dc 100644 --- a/models/fixtures/action_artifact.yml +++ b/models/fixtures/action_artifact.yml @@ -69,3 +69,21 @@ created_unix: 1730330775 updated_unix: 1730330775 expired_unix: 1738106775 + +- + id: 23 + run_id: 793 + runner_id: 1 + repo_id: 2 + owner_id: 2 + commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 + storage_path: "27/5/1730330775594233150.chunk" + file_size: 1024 + file_compressed_size: 1024 + content_encoding: "application/zip" + artifact_path: "artifact-v4-download.zip" + artifact_name: "artifact-v4-download" + status: 2 + created_unix: 1730330775 + updated_unix: 1730330775 + expired_unix: 1738106775 diff --git a/tests/integration/api_actions_artifact_v4_test.go b/tests/integration/api_actions_artifact_v4_test.go index 268ccb6e08c77..6f423a7097d7e 100644 --- a/tests/integration/api_actions_artifact_v4_test.go +++ b/tests/integration/api_actions_artifact_v4_test.go @@ -398,12 +398,56 @@ func TestActionsArtifactV4DownloadSinglePublicApi(t *testing.T) { resp = MakeRequest(t, req, http.StatusFound) - // confirm artifact can be downloaded and has expected content - req = NewRequestWithBody(t, "GET", resp.Header().Get("Location"), nil). + blobLocation := resp.Header().Get("Location") + + // confirm artifact can be downloaded without token and has expected content + req = NewRequestWithBody(t, "GET", blobLocation, nil) + resp = MakeRequest(t, req, http.StatusOK) + body := strings.Repeat("D", 1024) + assert.Equal(t, body, resp.Body.String()) + + // confirm artifact can not be downloaded without query + req = NewRequestWithBody(t, "GET", blobLocation, nil) + req.URL.RawQuery = "" + _ = MakeRequest(t, req, http.StatusUnauthorized) +} + +func TestActionsArtifactV4DownloadSinglePublicApiPrivateRepo(t *testing.T) { + defer prepareTestEnvActionsArtifacts(t)() + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + session := loginUser(t, user.Name) + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) + + // confirm artifact can be listed and found by name + req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts?name=artifact-v4-download", repo.FullName()), nil). + AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusOK) + var listResp api.ActionArtifactsResponse + err := json.Unmarshal(resp.Body.Bytes(), &listResp) + assert.NoError(t, err) + assert.Equal(t, int64(23), listResp.Entries[0].ID) + assert.NotEmpty(t, listResp.Entries[0].ArchiveDownloadURL) + assert.Equal(t, "artifact-v4-download", listResp.Entries[0].Name) + + // confirm artifact blob storage url can be retrieved + req = NewRequestWithBody(t, "GET", listResp.Entries[0].ArchiveDownloadURL, nil). AddTokenAuth(token) + + resp = MakeRequest(t, req, http.StatusFound) + + blobLocation := resp.Header().Get("Location") + // confirm artifact can be downloaded without token and has expected content + req = NewRequestWithBody(t, "GET", blobLocation, nil) resp = MakeRequest(t, req, http.StatusOK) body := strings.Repeat("D", 1024) assert.Equal(t, body, resp.Body.String()) + + // confirm artifact can not be downloaded without query + req = NewRequestWithBody(t, "GET", blobLocation, nil) + req.URL.RawQuery = "" + _ = MakeRequest(t, req, http.StatusUnauthorized) } func TestActionsArtifactV4ListAndGetPublicApi(t *testing.T) { @@ -483,7 +527,7 @@ func TestActionsArtifactV4DownloadArtifactCorrectRepoOwnerFound(t *testing.T) { MakeRequest(t, req, http.StatusFound) } -func TestActionsArtifactV4DownloadRawArtifactMismatchedRepoOwnerNotFound(t *testing.T) { +func TestActionsArtifactV4DownloadRawArtifactMismatchedRepoOwnerMissingSignatureUnauthorized(t *testing.T) { defer prepareTestEnvActionsArtifacts(t)() repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) @@ -494,7 +538,21 @@ func TestActionsArtifactV4DownloadRawArtifactMismatchedRepoOwnerNotFound(t *test // confirm artifacts of wrong owner or repo is not visible req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d/zip/raw", repo.FullName(), 22), nil). AddTokenAuth(token) - MakeRequest(t, req, http.StatusNotFound) + MakeRequest(t, req, http.StatusUnauthorized) +} + +func TestActionsArtifactV4DownloadRawArtifactCorrectRepoOwnerMissingSignatureUnauthorized(t *testing.T) { + defer prepareTestEnvActionsArtifacts(t)() + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) + user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) + session := loginUser(t, user.Name) + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) + + // confirm artifacts of wrong owner or repo is not visible + req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d/zip/raw", repo.FullName(), 22), nil). + AddTokenAuth(token) + MakeRequest(t, req, http.StatusUnauthorized) } func TestActionsArtifactV4Delete(t *testing.T) { From d56cebfeddc0f0d130abe283a0d139f1c8c8b011 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 15 Feb 2025 15:18:27 +0800 Subject: [PATCH 29/41] refactor path parm & error handling --- routers/api/v1/api.go | 2 +- routers/api/v1/repo/action.go | 65 +++++++++++++++++------------------ 2 files changed, 32 insertions(+), 35 deletions(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 1f75bae540d0e..70176f402ed40 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1409,7 +1409,7 @@ func Routes() *web.Router { }, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryRepository)) // Artifacts direct download endpoint authenticates via signed url - m.Get("/repos/{username}/{reponame}/actions/artifacts/{artifact_id}/zip/raw", repo.DownloadArtifactRaw) + m.Get("/repos/{username}/{reponame}/actions/artifacts/{artifact_id}/zip/raw", repoAssignment(), repo.DownloadArtifactRaw) // Notifications (requires notifications scope) m.Group("/repos", func() { diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index 551f5dc1ae31a..a2ac68ca9e0da 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -1028,7 +1028,7 @@ func GetArtifact(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - art, ok := getArtifactByID(ctx) + art, ok := getArtifactByPathParam(ctx) if !ok { return } @@ -1077,8 +1077,8 @@ func DeleteArtifact(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - art, ok := getArtifactByID(ctx) - if !ok { + art := getArtifactByPathParam(ctx) + if ctx.Written() { return } @@ -1102,10 +1102,10 @@ func buildSignature(endp, expires string, artifactID int64) []byte { return mac.Sum(nil) } -func buildSigURL(ctx go_context.Context, endp string, artifactID int64) string { +func buildSigURL(ctx go_context.Context, endPoint string, artifactID int64) string { + // endPoint is a path like "api/v1/repos/owner/repo/actions/artifacts/1/zip/raw" expires := time.Now().Add(60 * time.Minute).Format("2006-01-02 15:04:05.999999999 -0700 MST") - uploadURL := strings.TrimSuffix(httplib.GuessCurrentAppURL(ctx), "/") + - "/" + endp + "?sig=" + base64.URLEncoding.EncodeToString(buildSignature(endp, expires, artifactID)) + "&expires=" + url.QueryEscape(expires) + uploadURL := httplib.GuessCurrentAppURL(ctx) + endPoint + "?sig=" + base64.URLEncoding.EncodeToString(buildSignature(endPoint, expires, artifactID)) + "&expires=" + url.QueryEscape(expires) return uploadURL } @@ -1140,8 +1140,8 @@ func DownloadArtifact(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - art, ok := getArtifactByID(ctx) - if !ok { + art := getArtifactByPathParam(ctx) + if ctx.Written() { return } @@ -1172,35 +1172,30 @@ func DownloadArtifact(ctx *context.APIContext) { // DownloadArtifactRaw Downloads a specific artifact for a workflow run directly. func DownloadArtifactRaw(ctx *context.APIContext) { - username := ctx.PathParam("username") - reponame := ctx.PathParam("reponame") - artifactID := ctx.PathParamInt64("artifact_id") + art := getArtifactByPathParam(ctx) + if ctx.Written() { + return + } + sig := ctx.Req.URL.Query().Get("sig") expires := ctx.Req.URL.Query().Get("expires") dsig, _ := base64.URLEncoding.DecodeString(sig) - endp := "api/v1/repos/" + url.PathEscape(username) + "/" + url.PathEscape(reponame) + "/actions/artifacts/" + fmt.Sprintf("%d", artifactID) + "/zip/raw" - expecedsig := buildSignature(endp, expires, artifactID) - if !hmac.Equal(dsig, expecedsig) { - log.Error("Error unauthorized") - ctx.Error(http.StatusUnauthorized, "Error unauthorized", nil) + endp := "api/v1/repos/" + url.PathEscape(ctx.Repo.Repository.OwnerName) + "/" + url.PathEscape(ctx.Repo.Repository.Name) + "/actions/artifacts/" + fmt.Sprintf("%d", art.ID) + "/zip/raw" + expectedSig := buildSignature(endp, expires, art.ID) + if !hmac.Equal(dsig, expectedSig) { + ctx.Error(http.StatusUnauthorized, "DownloadArtifactRaw", "Error unauthorized") return } t, err := time.Parse("2006-01-02 15:04:05.999999999 -0700 MST", expires) if err != nil || t.Before(time.Now()) { - log.Error("Error link expired") - ctx.Error(http.StatusUnauthorized, "Error link expired", nil) - return - } - - art, ok := getArtifactByID(ctx) - if !ok { + ctx.Error(http.StatusUnauthorized, "DownloadArtifactRaw", "Error link expired") return } // if artifacts status is not uploaded-confirmed, treat it as not found if art.Status == int64(actions_model.ArtifactStatusExpired) { - ctx.Error(http.StatusNotFound, "artifact has expired", fmt.Errorf("artifact has expired")) + ctx.Error(http.StatusNotFound, "DownloadArtifactRaw", "Artifact has expired") return } ctx.Resp.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%s.zip; filename*=UTF-8''%s.zip", url.PathEscape(art.ArtifactName), art.ArtifactName)) @@ -1208,29 +1203,31 @@ func DownloadArtifactRaw(ctx *context.APIContext) { if actions.IsArtifactV4(art) { err := actions.DownloadArtifactV4(ctx.Base, art) if err != nil { - ctx.Error(http.StatusInternalServerError, err.Error(), err) + ctx.Error(http.StatusInternalServerError, "DownloadArtifactV4", err) return } return } // v3 not supported due to not having one unique id - ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) + ctx.Error(http.StatusNotFound, "DownloadArtifactRaw", "artifact not found") } // Try to get the artifact by ID and check access -func getArtifactByID(ctx *context.APIContext) (*actions_model.ActionArtifact, bool) { +func getArtifactByPathParam(ctx *context.APIContext) *actions_model.ActionArtifact { artifactID := ctx.PathParamInt64("artifact_id") art, ok, err := db.GetByID[actions_model.ActionArtifact](ctx, artifactID) if err != nil { - ctx.Error(http.StatusInternalServerError, err.Error(), err) - return nil, false + ctx.Error(http.StatusInternalServerError, "getArtifactByPathParam", err) + return nil } // if artifacts status is not uploaded-confirmed, treat it as not found - // ctx.Repo.Repository is nil for the raw download endpoint that checked this already - if !ok || ctx.Repo != nil && ctx.Repo.Repository != nil && (art.RepoID != ctx.Repo.Repository.ID || art.OwnerID != ctx.Repo.Repository.OwnerID) || art.Status != int64(actions_model.ArtifactStatusUploadConfirmed) && art.Status != int64(actions_model.ArtifactStatusExpired) { - ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) - return nil, false + // FIXME: is the OwnerID check right? What if a repo is transferred to a new owner? + if !ok || + (art.RepoID != ctx.Repo.Repository.ID || art.OwnerID != ctx.Repo.Repository.OwnerID) || + art.Status != int64(actions_model.ArtifactStatusUploadConfirmed) && art.Status != int64(actions_model.ArtifactStatusExpired) { + ctx.Error(http.StatusNotFound, "getArtifactByPathParam", "artifact not found") + return nil } - return art, true + return art } From ee37003dcbe9062b9e11b3e7c9ca0d181cc47287 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 15 Feb 2025 15:22:02 +0800 Subject: [PATCH 30/41] fix type casting --- cmd/migrate_storage.go | 2 +- models/actions/artifact.go | 10 +++++----- routers/api/actions/artifacts_chunks.go | 2 +- routers/api/v1/repo/action.go | 11 +++++------ routers/web/repo/actions/view.go | 2 +- services/convert/convert.go | 2 +- 6 files changed, 14 insertions(+), 15 deletions(-) diff --git a/cmd/migrate_storage.go b/cmd/migrate_storage.go index 6ece4bf661b3b..2e3aba021d2ee 100644 --- a/cmd/migrate_storage.go +++ b/cmd/migrate_storage.go @@ -196,7 +196,7 @@ func migrateActionsLog(ctx context.Context, dstStorage storage.ObjectStorage) er func migrateActionsArtifacts(ctx context.Context, dstStorage storage.ObjectStorage) error { return db.Iterate(ctx, nil, func(ctx context.Context, artifact *actions_model.ActionArtifact) error { - if artifact.Status == int64(actions_model.ArtifactStatusExpired) { + if artifact.Status == actions_model.ArtifactStatusExpired { return nil } diff --git a/models/actions/artifact.go b/models/actions/artifact.go index 342e35fb4e3f3..524224f0701c5 100644 --- a/models/actions/artifact.go +++ b/models/actions/artifact.go @@ -48,7 +48,7 @@ type ActionArtifact struct { ContentEncoding string // The content encoding of the artifact ArtifactPath string `xorm:"index unique(runid_name_path)"` // The path to the artifact when runner uploads it ArtifactName string `xorm:"index unique(runid_name_path)"` // The name of the artifact when runner uploads it - Status int64 `xorm:"index"` // The status of the artifact, uploading, expired or need-delete + Status ArtifactStatus `xorm:"index"` // The status of the artifact, uploading, expired or need-delete CreatedUnix timeutil.TimeStamp `xorm:"created"` UpdatedUnix timeutil.TimeStamp `xorm:"updated index"` ExpiredUnix timeutil.TimeStamp `xorm:"index"` // The time when the artifact will be expired @@ -68,7 +68,7 @@ func CreateArtifact(ctx context.Context, t *ActionTask, artifactName, artifactPa RepoID: t.RepoID, OwnerID: t.OwnerID, CommitSHA: t.CommitSHA, - Status: int64(ArtifactStatusUploadPending), + Status: ArtifactStatusUploadPending, ExpiredUnix: timeutil.TimeStamp(time.Now().Unix() + timeutil.Day*expiredDays), } if _, err := db.GetEngine(ctx).Insert(artifact); err != nil { @@ -177,18 +177,18 @@ func ListPendingDeleteArtifacts(ctx context.Context, limit int) ([]*ActionArtifa // SetArtifactExpired sets an artifact to expired func SetArtifactExpired(ctx context.Context, artifactID int64) error { - _, err := db.GetEngine(ctx).Where("id=? AND status = ?", artifactID, ArtifactStatusUploadConfirmed).Cols("status").Update(&ActionArtifact{Status: int64(ArtifactStatusExpired)}) + _, err := db.GetEngine(ctx).Where("id=? AND status = ?", artifactID, ArtifactStatusUploadConfirmed).Cols("status").Update(&ActionArtifact{Status: ArtifactStatusExpired}) return err } // SetArtifactNeedDelete sets an artifact to need-delete, cron job will delete it func SetArtifactNeedDelete(ctx context.Context, runID int64, name string) error { - _, err := db.GetEngine(ctx).Where("run_id=? AND artifact_name=? AND status = ?", runID, name, ArtifactStatusUploadConfirmed).Cols("status").Update(&ActionArtifact{Status: int64(ArtifactStatusPendingDeletion)}) + _, err := db.GetEngine(ctx).Where("run_id=? AND artifact_name=? AND status = ?", runID, name, ArtifactStatusUploadConfirmed).Cols("status").Update(&ActionArtifact{Status: ArtifactStatusPendingDeletion}) return err } // SetArtifactDeleted sets an artifact to deleted func SetArtifactDeleted(ctx context.Context, artifactID int64) error { - _, err := db.GetEngine(ctx).ID(artifactID).Cols("status").Update(&ActionArtifact{Status: int64(ArtifactStatusDeleted)}) + _, err := db.GetEngine(ctx).ID(artifactID).Cols("status").Update(&ActionArtifact{Status: ArtifactStatusDeleted}) return err } diff --git a/routers/api/actions/artifacts_chunks.go b/routers/api/actions/artifacts_chunks.go index cf48da12aa850..9d2b69820cbf9 100644 --- a/routers/api/actions/artifacts_chunks.go +++ b/routers/api/actions/artifacts_chunks.go @@ -292,7 +292,7 @@ func mergeChunksForArtifact(ctx *ArtifactContext, chunks []*chunkFileItem, st st } artifact.StoragePath = storagePath - artifact.Status = int64(actions.ArtifactStatusUploadConfirmed) + artifact.Status = actions.ArtifactStatusUploadConfirmed if err := actions.UpdateArtifactByID(ctx, artifact.ID, artifact); err != nil { return fmt.Errorf("update artifact error: %v", err) } diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index a2ac68ca9e0da..010ea5f244566 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -20,7 +20,6 @@ import ( secret_model "code.gitea.io/gitea/models/secret" "code.gitea.io/gitea/modules/actions" "code.gitea.io/gitea/modules/httplib" - "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/util" @@ -1028,8 +1027,8 @@ func GetArtifact(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - art, ok := getArtifactByPathParam(ctx) - if !ok { + art := getArtifactByPathParam(ctx) + if ctx.Written() { return } @@ -1146,7 +1145,7 @@ func DownloadArtifact(ctx *context.APIContext) { } // if artifacts status is not uploaded-confirmed, treat it as not found - if art.Status == int64(actions_model.ArtifactStatusExpired) { + if art.Status == actions_model.ArtifactStatusExpired { ctx.Error(http.StatusNotFound, "artifact has expired", fmt.Errorf("artifact has expired")) return } @@ -1194,7 +1193,7 @@ func DownloadArtifactRaw(ctx *context.APIContext) { } // if artifacts status is not uploaded-confirmed, treat it as not found - if art.Status == int64(actions_model.ArtifactStatusExpired) { + if art.Status == actions_model.ArtifactStatusExpired { ctx.Error(http.StatusNotFound, "DownloadArtifactRaw", "Artifact has expired") return } @@ -1225,7 +1224,7 @@ func getArtifactByPathParam(ctx *context.APIContext) *actions_model.ActionArtifa // FIXME: is the OwnerID check right? What if a repo is transferred to a new owner? if !ok || (art.RepoID != ctx.Repo.Repository.ID || art.OwnerID != ctx.Repo.Repository.OwnerID) || - art.Status != int64(actions_model.ArtifactStatusUploadConfirmed) && art.Status != int64(actions_model.ArtifactStatusExpired) { + art.Status != actions_model.ArtifactStatusUploadConfirmed && art.Status != actions_model.ArtifactStatusExpired { ctx.Error(http.StatusNotFound, "getArtifactByPathParam", "artifact not found") return nil } diff --git a/routers/web/repo/actions/view.go b/routers/web/repo/actions/view.go index 787456d23fb76..0e71ce6ff83be 100644 --- a/routers/web/repo/actions/view.go +++ b/routers/web/repo/actions/view.go @@ -668,7 +668,7 @@ func ArtifactsDownloadView(ctx *context_module.Context) { // if artifacts status is not uploaded-confirmed, treat it as not found for _, art := range artifacts { - if art.Status != int64(actions_model.ArtifactStatusUploadConfirmed) { + if art.Status != actions_model.ArtifactStatusUploadConfirmed { ctx.Error(http.StatusNotFound, "artifact not found") return } diff --git a/services/convert/convert.go b/services/convert/convert.go index 17d1f04adb313..fb276499d434e 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -237,7 +237,7 @@ func ToActionArtifact(repo *repo_model.Repository, art *actions_model.ActionArti ID: art.ID, Name: art.ArtifactName, SizeInBytes: art.FileSize, - Expired: art.Status == int64(actions_model.ArtifactStatusExpired), + Expired: art.Status == actions_model.ArtifactStatusExpired, URL: url, ArchiveDownloadURL: url + "/zip", CreatedAt: art.CreatedUnix.AsLocalTime(), From 5d7bf81c03b0589c69616df6aaa117687dd32712 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 15 Feb 2025 15:31:05 +0800 Subject: [PATCH 31/41] refactor getArtifactByPathParam --- routers/api/v1/api.go | 1 + routers/api/v1/repo/action.go | 13 ++++++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 70176f402ed40..a12c087bed63d 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1409,6 +1409,7 @@ func Routes() *web.Router { }, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryRepository)) // Artifacts direct download endpoint authenticates via signed url + // TODO: need to clarify whether to use repoAssignment or not m.Get("/repos/{username}/{reponame}/actions/artifacts/{artifact_id}/zip/raw", repoAssignment(), repo.DownloadArtifactRaw) // Notifications (requires notifications scope) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index 010ea5f244566..5476ff30c4623 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -17,6 +17,7 @@ import ( actions_model "code.gitea.io/gitea/models/actions" "code.gitea.io/gitea/models/db" + repo_model "code.gitea.io/gitea/models/repo" secret_model "code.gitea.io/gitea/models/secret" "code.gitea.io/gitea/modules/actions" "code.gitea.io/gitea/modules/httplib" @@ -1027,7 +1028,7 @@ func GetArtifact(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - art := getArtifactByPathParam(ctx) + art := getArtifactByPathParam(ctx, ctx.Repo.Repository) if ctx.Written() { return } @@ -1076,7 +1077,7 @@ func DeleteArtifact(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - art := getArtifactByPathParam(ctx) + art := getArtifactByPathParam(ctx, ctx.Repo.Repository) if ctx.Written() { return } @@ -1139,7 +1140,7 @@ func DownloadArtifact(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - art := getArtifactByPathParam(ctx) + art := getArtifactByPathParam(ctx, ctx.Repo.Repository) if ctx.Written() { return } @@ -1171,7 +1172,9 @@ func DownloadArtifact(ctx *context.APIContext) { // DownloadArtifactRaw Downloads a specific artifact for a workflow run directly. func DownloadArtifactRaw(ctx *context.APIContext) { - art := getArtifactByPathParam(ctx) + // TODO: if it needs to skip "repoAssignment" middleware, it could query the repo from path params: ctx.PathParam("username"), ctx.PathParam("reponame") + repo := ctx.Repo.Repository + art := getArtifactByPathParam(ctx, repo) if ctx.Written() { return } @@ -1212,7 +1215,7 @@ func DownloadArtifactRaw(ctx *context.APIContext) { } // Try to get the artifact by ID and check access -func getArtifactByPathParam(ctx *context.APIContext) *actions_model.ActionArtifact { +func getArtifactByPathParam(ctx *context.APIContext, repo *repo_model.Repository) *actions_model.ActionArtifact { artifactID := ctx.PathParamInt64("artifact_id") art, ok, err := db.GetByID[actions_model.ActionArtifact](ctx, artifactID) From 3d1b1561790daff553e03731c12a0b0f61edaad5 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 15 Feb 2025 15:46:57 +0800 Subject: [PATCH 32/41] refactor build endpoint --- routers/api/v1/repo/action.go | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index 5476ff30c4623..5c20c7b7af11b 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -1043,7 +1043,7 @@ func GetArtifact(ctx *context.APIContext) { return } // v3 not supported due to not having one unique id - ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) + ctx.Error(http.StatusNotFound, "GetArtifact", "Artifact not found") } // DeleteArtifact Deletes a specific artifact for a workflow run. @@ -1084,14 +1084,14 @@ func DeleteArtifact(ctx *context.APIContext) { if actions.IsArtifactV4(art) { if err := actions_model.SetArtifactNeedDelete(ctx, art.RunID, art.ArtifactName); err != nil { - ctx.Error(http.StatusInternalServerError, err.Error(), err) + ctx.Error(http.StatusInternalServerError, "DeleteArtifact", err) return } ctx.Status(http.StatusNoContent) return } // v3 not supported due to not having one unique id - ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) + ctx.Error(http.StatusNotFound, "DeleteArtifact", "Artifact not found") } func buildSignature(endp, expires string, artifactID int64) []byte { @@ -1102,6 +1102,10 @@ func buildSignature(endp, expires string, artifactID int64) []byte { return mac.Sum(nil) } +func buildDownloadRawEndpoint(repo *repo_model.Repository, artifactID int64) string { + return fmt.Sprintf("api/v1/repos/%s/%s//actions/artifacts/%d/zip/raw", url.PathEscape(repo.OwnerName), url.PathEscape(repo.Name), artifactID) +} + func buildSigURL(ctx go_context.Context, endPoint string, artifactID int64) string { // endPoint is a path like "api/v1/repos/owner/repo/actions/artifacts/1/zip/raw" expires := time.Now().Add(60 * time.Minute).Format("2006-01-02 15:04:05.999999999 -0700 MST") @@ -1147,7 +1151,7 @@ func DownloadArtifact(ctx *context.APIContext) { // if artifacts status is not uploaded-confirmed, treat it as not found if art.Status == actions_model.ArtifactStatusExpired { - ctx.Error(http.StatusNotFound, "artifact has expired", fmt.Errorf("artifact has expired")) + ctx.Error(http.StatusNotFound, "DownloadArtifact", "Artifact has expired") return } ctx.Resp.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%s.zip; filename*=UTF-8''%s.zip", url.PathEscape(art.ArtifactName), art.ArtifactName)) @@ -1158,16 +1162,16 @@ func DownloadArtifact(ctx *context.APIContext) { return } if err != nil { - ctx.Error(http.StatusInternalServerError, err.Error(), err) + ctx.Error(http.StatusInternalServerError, "DownloadArtifactV4ServeDirectOnly", err) return } - rurl := buildSigURL(ctx, "api/v1/repos/"+url.PathEscape(ctx.Repo.Repository.OwnerName)+"/"+url.PathEscape(ctx.Repo.Repository.Name)+"/actions/artifacts/"+fmt.Sprintf("%d", art.ID)+"/zip/raw", art.ID) - ctx.Redirect(rurl, http.StatusFound) + redirectURL := buildSigURL(ctx, buildDownloadRawEndpoint(ctx.Repo.Repository, art.ID), art.ID) + ctx.Redirect(redirectURL, http.StatusFound) return } // v3 not supported due to not having one unique id - ctx.Error(http.StatusNotFound, "artifact not found", fmt.Errorf("artifact not found")) + ctx.Error(http.StatusNotFound, "DownloadArtifact", "Artifact not found") } // DownloadArtifactRaw Downloads a specific artifact for a workflow run directly. @@ -1179,13 +1183,12 @@ func DownloadArtifactRaw(ctx *context.APIContext) { return } - sig := ctx.Req.URL.Query().Get("sig") + sigStr := ctx.Req.URL.Query().Get("sig") expires := ctx.Req.URL.Query().Get("expires") - dsig, _ := base64.URLEncoding.DecodeString(sig) + sigBytes, _ := base64.URLEncoding.DecodeString(sigStr) - endp := "api/v1/repos/" + url.PathEscape(ctx.Repo.Repository.OwnerName) + "/" + url.PathEscape(ctx.Repo.Repository.Name) + "/actions/artifacts/" + fmt.Sprintf("%d", art.ID) + "/zip/raw" - expectedSig := buildSignature(endp, expires, art.ID) - if !hmac.Equal(dsig, expectedSig) { + expectedSig := buildSignature(buildDownloadRawEndpoint(ctx.Repo.Repository, art.ID), expires, art.ID) + if !hmac.Equal(sigBytes, expectedSig) { ctx.Error(http.StatusUnauthorized, "DownloadArtifactRaw", "Error unauthorized") return } @@ -1226,7 +1229,7 @@ func getArtifactByPathParam(ctx *context.APIContext, repo *repo_model.Repository // if artifacts status is not uploaded-confirmed, treat it as not found // FIXME: is the OwnerID check right? What if a repo is transferred to a new owner? if !ok || - (art.RepoID != ctx.Repo.Repository.ID || art.OwnerID != ctx.Repo.Repository.OwnerID) || + (art.RepoID != repo.ID || art.OwnerID != repo.OwnerID) || art.Status != actions_model.ArtifactStatusUploadConfirmed && art.Status != actions_model.ArtifactStatusExpired { ctx.Error(http.StatusNotFound, "getArtifactByPathParam", "artifact not found") return nil From e6629fc8143bf8a2c3d4a8afceadb9eb2e6af4f0 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 15 Feb 2025 15:56:26 +0800 Subject: [PATCH 33/41] remove repoAssignment for raw endpoint --- routers/api/v1/api.go | 2 +- routers/api/v1/repo/action.go | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index a12c087bed63d..f6985bae08c76 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1410,7 +1410,7 @@ func Routes() *web.Router { // Artifacts direct download endpoint authenticates via signed url // TODO: need to clarify whether to use repoAssignment or not - m.Get("/repos/{username}/{reponame}/actions/artifacts/{artifact_id}/zip/raw", repoAssignment(), repo.DownloadArtifactRaw) + m.Get("/repos/{username}/{reponame}/actions/artifacts/{artifact_id}/zip/raw", repo.DownloadArtifactRaw) // Notifications (requires notifications scope) m.Group("/repos", func() { diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index 5c20c7b7af11b..dc4ae04750d73 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -1177,7 +1177,15 @@ func DownloadArtifact(ctx *context.APIContext) { // DownloadArtifactRaw Downloads a specific artifact for a workflow run directly. func DownloadArtifactRaw(ctx *context.APIContext) { // TODO: if it needs to skip "repoAssignment" middleware, it could query the repo from path params: ctx.PathParam("username"), ctx.PathParam("reponame") - repo := ctx.Repo.Repository + repo, err := repo_model.GetRepositoryByOwnerAndName(ctx, ctx.PathParam("username"), ctx.PathParam("reponame")) + if err != nil { + if errors.Is(err, util.ErrNotExist) { + ctx.NotFound() + } else { + ctx.InternalServerError(err) + } + return + } art := getArtifactByPathParam(ctx, repo) if ctx.Written() { return From 7861808761de38535f89fb9e510b4867f70880f6 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 15 Feb 2025 18:23:47 +0800 Subject: [PATCH 34/41] fix test --- routers/api/v1/repo/action.go | 2 +- tests/integration/api_actions_artifact_v4_test.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index dc4ae04750d73..375535f2bd3bb 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -1195,7 +1195,7 @@ func DownloadArtifactRaw(ctx *context.APIContext) { expires := ctx.Req.URL.Query().Get("expires") sigBytes, _ := base64.URLEncoding.DecodeString(sigStr) - expectedSig := buildSignature(buildDownloadRawEndpoint(ctx.Repo.Repository, art.ID), expires, art.ID) + expectedSig := buildSignature(buildDownloadRawEndpoint(repo, art.ID), expires, art.ID) if !hmac.Equal(sigBytes, expectedSig) { ctx.Error(http.StatusUnauthorized, "DownloadArtifactRaw", "Error unauthorized") return diff --git a/tests/integration/api_actions_artifact_v4_test.go b/tests/integration/api_actions_artifact_v4_test.go index 6f423a7097d7e..9a5983ae12681 100644 --- a/tests/integration/api_actions_artifact_v4_test.go +++ b/tests/integration/api_actions_artifact_v4_test.go @@ -536,9 +536,10 @@ func TestActionsArtifactV4DownloadRawArtifactMismatchedRepoOwnerMissingSignature token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) // confirm artifacts of wrong owner or repo is not visible + // TODO: is this test right? `zip/raw` endpoint doesn't use the token? req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d/zip/raw", repo.FullName(), 22), nil). AddTokenAuth(token) - MakeRequest(t, req, http.StatusUnauthorized) + MakeRequest(t, req, http.StatusNotFound) } func TestActionsArtifactV4DownloadRawArtifactCorrectRepoOwnerMissingSignatureUnauthorized(t *testing.T) { From 8e48777eac514c5735bee78e65f02182465d324e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 15 Feb 2025 21:34:51 +0800 Subject: [PATCH 35/41] fix comment --- routers/api/v1/api.go | 2 +- routers/api/v1/repo/action.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index f6985bae08c76..8c39393246a7a 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -1409,7 +1409,7 @@ func Routes() *web.Router { }, tokenRequiresScopes(auth_model.AccessTokenScopeCategoryRepository)) // Artifacts direct download endpoint authenticates via signed url - // TODO: need to clarify whether to use repoAssignment or not + // it is protected by the "sig" parameter (to help to access private repo), so no need to use other middlewares m.Get("/repos/{username}/{reponame}/actions/artifacts/{artifact_id}/zip/raw", repo.DownloadArtifactRaw) // Notifications (requires notifications scope) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index 375535f2bd3bb..4c453065b642f 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -1103,7 +1103,7 @@ func buildSignature(endp, expires string, artifactID int64) []byte { } func buildDownloadRawEndpoint(repo *repo_model.Repository, artifactID int64) string { - return fmt.Sprintf("api/v1/repos/%s/%s//actions/artifacts/%d/zip/raw", url.PathEscape(repo.OwnerName), url.PathEscape(repo.Name), artifactID) + return fmt.Sprintf("api/v1/repos/%s/%s/actions/artifacts/%d/zip/raw", url.PathEscape(repo.OwnerName), url.PathEscape(repo.Name), artifactID) } func buildSigURL(ctx go_context.Context, endPoint string, artifactID int64) string { @@ -1176,7 +1176,7 @@ func DownloadArtifact(ctx *context.APIContext) { // DownloadArtifactRaw Downloads a specific artifact for a workflow run directly. func DownloadArtifactRaw(ctx *context.APIContext) { - // TODO: if it needs to skip "repoAssignment" middleware, it could query the repo from path params: ctx.PathParam("username"), ctx.PathParam("reponame") + // it doesn't use repoAssignment middleware, so it needs to prepare the repo and check permission (sig) by itself repo, err := repo_model.GetRepositoryByOwnerAndName(ctx, ctx.PathParam("username"), ctx.PathParam("reponame")) if err != nil { if errors.Is(err, util.ErrNotExist) { From 421b7b485ef7b5b57b0f8b422aa62d85d456151c Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sat, 15 Feb 2025 15:23:40 +0100 Subject: [PATCH 36/41] remove TestActionsArtifactV4DownloadRawArtifactMismatchedRepoOwnerMissingSignatureUnauthorized --- tests/integration/api_actions_artifact_v4_test.go | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/tests/integration/api_actions_artifact_v4_test.go b/tests/integration/api_actions_artifact_v4_test.go index 9a5983ae12681..55352ca7595c3 100644 --- a/tests/integration/api_actions_artifact_v4_test.go +++ b/tests/integration/api_actions_artifact_v4_test.go @@ -527,21 +527,6 @@ func TestActionsArtifactV4DownloadArtifactCorrectRepoOwnerFound(t *testing.T) { MakeRequest(t, req, http.StatusFound) } -func TestActionsArtifactV4DownloadRawArtifactMismatchedRepoOwnerMissingSignatureUnauthorized(t *testing.T) { - defer prepareTestEnvActionsArtifacts(t)() - - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID}) - session := loginUser(t, user.Name) - token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) - - // confirm artifacts of wrong owner or repo is not visible - // TODO: is this test right? `zip/raw` endpoint doesn't use the token? - req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d/zip/raw", repo.FullName(), 22), nil). - AddTokenAuth(token) - MakeRequest(t, req, http.StatusNotFound) -} - func TestActionsArtifactV4DownloadRawArtifactCorrectRepoOwnerMissingSignatureUnauthorized(t *testing.T) { defer prepareTestEnvActionsArtifacts(t)() From a00f7f8d56212b1ba0040a391c1f86d785b39e7b Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sat, 15 Feb 2025 15:29:56 +0100 Subject: [PATCH 37/41] Update test comments and names to reflect only repo check --- .../integration/api_actions_artifact_v4_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/integration/api_actions_artifact_v4_test.go b/tests/integration/api_actions_artifact_v4_test.go index 55352ca7595c3..b6dfa6e7994d5 100644 --- a/tests/integration/api_actions_artifact_v4_test.go +++ b/tests/integration/api_actions_artifact_v4_test.go @@ -485,7 +485,7 @@ func TestActionsArtifactV4ListAndGetPublicApi(t *testing.T) { } } -func TestActionsArtifactV4GetArtifactMismatchedRepoOwnerNotFound(t *testing.T) { +func TestActionsArtifactV4GetArtifactMismatchedRepoNotFound(t *testing.T) { defer prepareTestEnvActionsArtifacts(t)() repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) @@ -493,13 +493,13 @@ func TestActionsArtifactV4GetArtifactMismatchedRepoOwnerNotFound(t *testing.T) { session := loginUser(t, user.Name) token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) - // confirm artifacts of wrong owner or repo is not visible + // confirm artifacts of wrong repo is not visible req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d", repo.FullName(), 22), nil). AddTokenAuth(token) MakeRequest(t, req, http.StatusNotFound) } -func TestActionsArtifactV4DownloadArtifactMismatchedRepoOwnerNotFound(t *testing.T) { +func TestActionsArtifactV4DownloadArtifactMismatchedRepoNotFound(t *testing.T) { defer prepareTestEnvActionsArtifacts(t)() repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) @@ -507,13 +507,13 @@ func TestActionsArtifactV4DownloadArtifactMismatchedRepoOwnerNotFound(t *testing session := loginUser(t, user.Name) token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) - // confirm artifacts of wrong owner or repo is not visible + // confirm artifacts of wrong repo is not visible req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d/zip", repo.FullName(), 22), nil). AddTokenAuth(token) MakeRequest(t, req, http.StatusNotFound) } -func TestActionsArtifactV4DownloadArtifactCorrectRepoOwnerFound(t *testing.T) { +func TestActionsArtifactV4DownloadArtifactCorrectRepoFound(t *testing.T) { defer prepareTestEnvActionsArtifacts(t)() repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) @@ -521,13 +521,13 @@ func TestActionsArtifactV4DownloadArtifactCorrectRepoOwnerFound(t *testing.T) { session := loginUser(t, user.Name) token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) - // confirm artifacts of correct owner and repo is visible + // confirm artifacts of correct repo is visible req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d/zip", repo.FullName(), 22), nil). AddTokenAuth(token) MakeRequest(t, req, http.StatusFound) } -func TestActionsArtifactV4DownloadRawArtifactCorrectRepoOwnerMissingSignatureUnauthorized(t *testing.T) { +func TestActionsArtifactV4DownloadRawArtifactCorrectRepoMissingSignatureUnauthorized(t *testing.T) { defer prepareTestEnvActionsArtifacts(t)() repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) @@ -535,7 +535,7 @@ func TestActionsArtifactV4DownloadRawArtifactCorrectRepoOwnerMissingSignatureUna session := loginUser(t, user.Name) token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) - // confirm artifacts of wrong owner or repo is not visible + // confirm cannot use the raw artifact endpoint even with a correct access token req := NewRequestWithBody(t, "GET", fmt.Sprintf("/api/v1/repos/%s/actions/artifacts/%d/zip/raw", repo.FullName(), 22), nil). AddTokenAuth(token) MakeRequest(t, req, http.StatusUnauthorized) From 156f0db30875473a01f500e7e771ff494c0d602f Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sat, 15 Feb 2025 15:41:31 +0100 Subject: [PATCH 38/41] use unix timestamp in sig --- routers/api/v1/repo/action.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index 4c453065b642f..d784e92b00647 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -12,6 +12,7 @@ import ( "fmt" "net/http" "net/url" + "strconv" "strings" "time" @@ -1094,10 +1095,10 @@ func DeleteArtifact(ctx *context.APIContext) { ctx.Error(http.StatusNotFound, "DeleteArtifact", "Artifact not found") } -func buildSignature(endp, expires string, artifactID int64) []byte { +func buildSignature(endp string, expires, artifactID int64) []byte { mac := hmac.New(sha256.New, setting.GetGeneralTokenSigningSecret()) mac.Write([]byte(endp)) - mac.Write([]byte(expires)) + mac.Write([]byte(fmt.Sprint(expires))) mac.Write([]byte(fmt.Sprint(artifactID))) return mac.Sum(nil) } @@ -1108,8 +1109,8 @@ func buildDownloadRawEndpoint(repo *repo_model.Repository, artifactID int64) str func buildSigURL(ctx go_context.Context, endPoint string, artifactID int64) string { // endPoint is a path like "api/v1/repos/owner/repo/actions/artifacts/1/zip/raw" - expires := time.Now().Add(60 * time.Minute).Format("2006-01-02 15:04:05.999999999 -0700 MST") - uploadURL := httplib.GuessCurrentAppURL(ctx) + endPoint + "?sig=" + base64.URLEncoding.EncodeToString(buildSignature(endPoint, expires, artifactID)) + "&expires=" + url.QueryEscape(expires) + expires := time.Now().Add(60 * time.Minute).Unix() + uploadURL := httplib.GuessCurrentAppURL(ctx) + endPoint + "?sig=" + base64.URLEncoding.EncodeToString(buildSignature(endPoint, expires, artifactID)) + "&expires=" + strconv.FormatInt(expires, 10) return uploadURL } @@ -1192,15 +1193,16 @@ func DownloadArtifactRaw(ctx *context.APIContext) { } sigStr := ctx.Req.URL.Query().Get("sig") - expires := ctx.Req.URL.Query().Get("expires") + expiresStr := ctx.Req.URL.Query().Get("expires") sigBytes, _ := base64.URLEncoding.DecodeString(sigStr) + expires, _ := strconv.ParseInt(expiresStr, 10, 64) expectedSig := buildSignature(buildDownloadRawEndpoint(repo, art.ID), expires, art.ID) if !hmac.Equal(sigBytes, expectedSig) { ctx.Error(http.StatusUnauthorized, "DownloadArtifactRaw", "Error unauthorized") return } - t, err := time.Parse("2006-01-02 15:04:05.999999999 -0700 MST", expires) + t := time.Unix(expires, 0) if err != nil || t.Before(time.Now()) { ctx.Error(http.StatusUnauthorized, "DownloadArtifactRaw", "Error link expired") return @@ -1235,9 +1237,9 @@ func getArtifactByPathParam(ctx *context.APIContext, repo *repo_model.Repository return nil } // if artifacts status is not uploaded-confirmed, treat it as not found - // FIXME: is the OwnerID check right? What if a repo is transferred to a new owner? + // Only check RepoID here, because the repository owner may change over the time if !ok || - (art.RepoID != repo.ID || art.OwnerID != repo.OwnerID) || + art.RepoID != repo.ID || art.Status != actions_model.ArtifactStatusUploadConfirmed && art.Status != actions_model.ArtifactStatusExpired { ctx.Error(http.StatusNotFound, "getArtifactByPathParam", "artifact not found") return nil From 3df911aa346184663a0e4f846c4c7409a360f8e0 Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sat, 15 Feb 2025 15:43:20 +0100 Subject: [PATCH 39/41] fix comment --- routers/api/v1/repo/action.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index d784e92b00647..c9b436078669b 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -1237,7 +1237,7 @@ func getArtifactByPathParam(ctx *context.APIContext, repo *repo_model.Repository return nil } // if artifacts status is not uploaded-confirmed, treat it as not found - // Only check RepoID here, because the repository owner may change over the time + // only check RepoID here, because the repository owner may change over the time if !ok || art.RepoID != repo.ID || art.Status != actions_model.ArtifactStatusUploadConfirmed && art.Status != actions_model.ArtifactStatusExpired { From 4c1399d3a69d8098ba274dbba8aa34240f09637a Mon Sep 17 00:00:00 2001 From: Christopher Homberger Date: Sat, 15 Feb 2025 15:51:54 +0100 Subject: [PATCH 40/41] revert owner_id changes to reflect only repoid check --- models/fixtures/action_artifact.yml | 8 ++++---- models/fixtures/action_run_job.yml | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/models/fixtures/action_artifact.yml b/models/fixtures/action_artifact.yml index 665e3268a42dc..485474108f7ae 100644 --- a/models/fixtures/action_artifact.yml +++ b/models/fixtures/action_artifact.yml @@ -3,7 +3,7 @@ run_id: 791 runner_id: 1 repo_id: 4 - owner_id: 5 + owner_id: 1 commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 storage_path: "26/1/1712166500347189545.chunk" file_size: 1024 @@ -21,7 +21,7 @@ run_id: 791 runner_id: 1 repo_id: 4 - owner_id: 5 + owner_id: 1 commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 storage_path: "26/19/1712348022422036662.chunk" file_size: 1024 @@ -39,7 +39,7 @@ run_id: 791 runner_id: 1 repo_id: 4 - owner_id: 5 + owner_id: 1 commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 storage_path: "26/20/1712348022423431524.chunk" file_size: 1024 @@ -57,7 +57,7 @@ run_id: 792 runner_id: 1 repo_id: 4 - owner_id: 5 + owner_id: 1 commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 storage_path: "27/5/1730330775594233150.chunk" file_size: 1024 diff --git a/models/fixtures/action_run_job.yml b/models/fixtures/action_run_job.yml index 046cc0ba093d7..8837e6ec2d80d 100644 --- a/models/fixtures/action_run_job.yml +++ b/models/fixtures/action_run_job.yml @@ -2,7 +2,7 @@ id: 192 run_id: 791 repo_id: 4 - owner_id: 5 + owner_id: 1 commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 is_fork_pull_request: 0 name: job_2 @@ -16,7 +16,7 @@ id: 193 run_id: 792 repo_id: 4 - owner_id: 5 + owner_id: 1 commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 is_fork_pull_request: 0 name: job_2 @@ -30,7 +30,7 @@ id: 194 run_id: 793 repo_id: 4 - owner_id: 5 + owner_id: 1 commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 is_fork_pull_request: 0 name: job1 (1) @@ -44,7 +44,7 @@ id: 195 run_id: 793 repo_id: 4 - owner_id: 5 + owner_id: 1 commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 is_fork_pull_request: 0 name: job1 (2) @@ -58,7 +58,7 @@ id: 196 run_id: 793 repo_id: 4 - owner_id: 5 + owner_id: 1 commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 is_fork_pull_request: 0 name: job2 From a0ed53dc5c12f01ccc1bb9cf5624e1aede15cdff Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 15 Feb 2025 23:41:56 +0800 Subject: [PATCH 41/41] Update routers/api/v1/repo/action.go --- routers/api/v1/repo/action.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/api/v1/repo/action.go b/routers/api/v1/repo/action.go index c9b436078669b..480e29cfd334f 100644 --- a/routers/api/v1/repo/action.go +++ b/routers/api/v1/repo/action.go @@ -1203,7 +1203,7 @@ func DownloadArtifactRaw(ctx *context.APIContext) { return } t := time.Unix(expires, 0) - if err != nil || t.Before(time.Now()) { + if t.Before(time.Now()) { ctx.Error(http.StatusUnauthorized, "DownloadArtifactRaw", "Error link expired") return }