diff options
| -rw-r--r-- | models/actions/artifact.go | 19 | ||||
| -rw-r--r-- | models/fixtures/action_artifact.yml | 18 | ||||
| -rw-r--r-- | routers/api/actions/artifacts.go | 11 | ||||
| -rw-r--r-- | routers/api/actions/artifactsv4.go | 20 | ||||
| -rw-r--r-- | tests/integration/api_actions_artifact_v4_test.go | 20 |
5 files changed, 81 insertions, 7 deletions
diff --git a/models/actions/artifact.go b/models/actions/artifact.go index 524224f070..757bd13acd 100644 --- a/models/actions/artifact.go +++ b/models/actions/artifact.go @@ -30,6 +30,25 @@ const ( ArtifactStatusDeleted // 6, ArtifactStatusDeleted is the status of an artifact that is deleted ) +func (status ArtifactStatus) ToString() string { + switch status { + case ArtifactStatusUploadPending: + return "upload is not yet completed" + case ArtifactStatusUploadConfirmed: + return "upload is completed" + case ArtifactStatusUploadError: + return "upload failed" + case ArtifactStatusExpired: + return "expired" + case ArtifactStatusPendingDeletion: + return "pending deletion" + case ArtifactStatusDeleted: + return "deleted" + default: + return "unknown" + } +} + func init() { db.RegisterModel(new(ActionArtifact)) } diff --git a/models/fixtures/action_artifact.yml b/models/fixtures/action_artifact.yml index 485474108f..1b00daf198 100644 --- a/models/fixtures/action_artifact.yml +++ b/models/fixtures/action_artifact.yml @@ -11,6 +11,24 @@ content_encoding: "" artifact_path: "abc.txt" artifact_name: "artifact-download" + status: 2 + created_unix: 1712338649 + updated_unix: 1712338649 + expired_unix: 1720114649 + +- + id: 2 + run_id: 791 + runner_id: 1 + repo_id: 4 + owner_id: 1 + commit_sha: c2d72f548424103f01ee1dc02889c1e2bff816b0 + storage_path: "" + file_size: 1024 + file_compressed_size: 1024 + content_encoding: "30/20/1712348022422036662.chunk" + artifact_path: "abc.txt" + artifact_name: "artifact-download-incomplete" status: 1 created_unix: 1712338649 updated_unix: 1712338649 diff --git a/routers/api/actions/artifacts.go b/routers/api/actions/artifacts.go index 0832e52f55..6473659e5c 100644 --- a/routers/api/actions/artifacts.go +++ b/routers/api/actions/artifacts.go @@ -337,7 +337,10 @@ func (ar artifactRoutes) listArtifacts(ctx *ArtifactContext) { return } - artifacts, err := db.Find[actions.ActionArtifact](ctx, actions.FindArtifactsOptions{RunID: runID}) + artifacts, err := db.Find[actions.ActionArtifact](ctx, actions.FindArtifactsOptions{ + RunID: runID, + Status: int(actions.ArtifactStatusUploadConfirmed), + }) if err != nil { log.Error("Error getting artifacts: %v", err) ctx.HTTPError(http.StatusInternalServerError, err.Error()) @@ -402,6 +405,7 @@ func (ar artifactRoutes) getDownloadArtifactURL(ctx *ArtifactContext) { artifacts, err := db.Find[actions.ActionArtifact](ctx, actions.FindArtifactsOptions{ RunID: runID, ArtifactName: itemPath, + Status: int(actions.ArtifactStatusUploadConfirmed), }) if err != nil { log.Error("Error getting artifacts: %v", err) @@ -473,6 +477,11 @@ func (ar artifactRoutes) downloadArtifact(ctx *ArtifactContext) { ctx.HTTPError(http.StatusBadRequest) return } + if artifact.Status != actions.ArtifactStatusUploadConfirmed { + log.Error("Error artifact not found: %s", artifact.Status.ToString()) + ctx.HTTPError(http.StatusNotFound, "Error artifact not found") + return + } fd, err := ar.fs.Open(artifact.StoragePath) if err != nil { diff --git a/routers/api/actions/artifactsv4.go b/routers/api/actions/artifactsv4.go index 9fb0a31549..e9e9fc6393 100644 --- a/routers/api/actions/artifactsv4.go +++ b/routers/api/actions/artifactsv4.go @@ -448,17 +448,15 @@ func (r *artifactV4Routes) listArtifacts(ctx *ArtifactContext) { return } - artifacts, err := db.Find[actions.ActionArtifact](ctx, actions.FindArtifactsOptions{RunID: runID}) + artifacts, err := db.Find[actions.ActionArtifact](ctx, actions.FindArtifactsOptions{ + RunID: runID, + Status: int(actions.ArtifactStatusUploadConfirmed), + }) if err != nil { log.Error("Error getting artifacts: %v", err) ctx.HTTPError(http.StatusInternalServerError, err.Error()) return } - if len(artifacts) == 0 { - log.Debug("[artifact] handleListArtifacts, no artifacts") - ctx.HTTPError(http.StatusNotFound) - return - } list := []*ListArtifactsResponse_MonolithArtifact{} @@ -510,6 +508,11 @@ func (r *artifactV4Routes) getSignedArtifactURL(ctx *ArtifactContext) { ctx.HTTPError(http.StatusNotFound, "Error artifact not found") return } + if artifact.Status != actions.ArtifactStatusUploadConfirmed { + log.Error("Error artifact not found: %s", artifact.Status.ToString()) + ctx.HTTPError(http.StatusNotFound, "Error artifact not found") + return + } respData := GetSignedArtifactURLResponse{} @@ -538,6 +541,11 @@ func (r *artifactV4Routes) downloadArtifact(ctx *ArtifactContext) { ctx.HTTPError(http.StatusNotFound, "Error artifact not found") return } + if artifact.Status != actions.ArtifactStatusUploadConfirmed { + log.Error("Error artifact not found: %s", artifact.Status.ToString()) + ctx.HTTPError(http.StatusNotFound, "Error artifact not found") + return + } file, _ := r.fs.Open(artifact.StoragePath) diff --git a/tests/integration/api_actions_artifact_v4_test.go b/tests/integration/api_actions_artifact_v4_test.go index b6dfa6e799..3db8bbb82e 100644 --- a/tests/integration/api_actions_artifact_v4_test.go +++ b/tests/integration/api_actions_artifact_v4_test.go @@ -557,6 +557,26 @@ func TestActionsArtifactV4Delete(t *testing.T) { var deleteResp actions.DeleteArtifactResponse protojson.Unmarshal(resp.Body.Bytes(), &deleteResp) assert.True(t, deleteResp.Ok) + + // confirm artifact is no longer accessible by GetSignedArtifactURL + req = NewRequestWithBody(t, "POST", "/twirp/github.actions.results.api.v1.ArtifactService/GetSignedArtifactURL", toProtoJSON(&actions.GetSignedArtifactURLRequest{ + Name: "artifact-v4-download", + WorkflowRunBackendId: "792", + WorkflowJobRunBackendId: "193", + })). + AddTokenAuth(token) + _ = MakeRequest(t, req, http.StatusNotFound) + + // confirm artifact is no longer enumerateable by ListArtifacts and returns length == 0 without error + req = NewRequestWithBody(t, "POST", "/twirp/github.actions.results.api.v1.ArtifactService/ListArtifacts", toProtoJSON(&actions.ListArtifactsRequest{ + NameFilter: wrapperspb.String("artifact-v4-download"), + WorkflowRunBackendId: "792", + WorkflowJobRunBackendId: "193", + })).AddTokenAuth(token) + resp = MakeRequest(t, req, http.StatusOK) + var listResp actions.ListArtifactsResponse + protojson.Unmarshal(resp.Body.Bytes(), &listResp) + assert.Empty(t, listResp.Artifacts) } func TestActionsArtifactV4DeletePublicApi(t *testing.T) { |