Feature non zipped actions artifacts (action v7 / nodejs/npm v6.2.0)#36786
Feature non zipped actions artifacts (action v7 / nodejs/npm v6.2.0)#36786ChristopherHX wants to merge 34 commits intogo-gitea:mainfrom
Conversation
Signed-off-by: ChristopherHX <christopher.homberger@web.de>
|
GITHUB_SERVER_URL hack + @actions/artifact can be used like const { default: artifact } = await import("@actions/artifact");
var artifactName = "my-single-file";
process.env.GITHUB_SERVER_URL = "https://github.com" // Escape isGHES bomb
var uploadResult = await artifact.uploadArtifact(
artifactName,
['package.json'],
'./',
{skipArchive: true}
) |
| Status: int(actions.ArtifactStatusUploadConfirmed), | ||
| RunID: runID, | ||
| Status: int(actions.ArtifactStatusUploadConfirmed), | ||
| FinalizedArtifactsV4: true, |
There was a problem hiding this comment.
Replaces || artifact.ArtifactName+".zip" != artifact.ArtifactPath || artifact.ContentEncoding != ArtifactV4ContentEncoding below so this filter is no longer needed
…n the code and fixed it later by changing the other end
This reverts commit c6540ef.
Signed-off-by: ChristopherHX <christopher.homberger@web.de>
* browser blocks scripts + accessing localstorage etc.
Signed-off-by: ChristopherHX <christopher.homberger@web.de>
* ServeDirect Content-Type and Content-Disposition are partially broken in minio and not implemented azure
There was a problem hiding this comment.
Pull request overview
This PR updates the Actions artifacts v4 implementation to support actions/upload-artifact@v7 “non-zipped” artifacts by propagating a MIME type through the API and serving artifacts inline with safer headers for browser preview.
Changes:
- Extend the v4 Twirp proto/API to accept
mime_typeand store it inActionArtifact.ContentEncoding(while keeping.zipbehavior for older clients/versions). - Adjust artifact lookup/filtering to treat v4 artifacts as those whose
content_encodinglooks like a MIME type and update listing logic accordingly. - Serve single v4 artifacts inline (plus CSP sandbox) and update integration tests around download behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/integration/api_actions_artifact_v4_test.go | Refactors download test to run across ServeDirect modes and asserts response headers. |
| routers/web/repo/actions/view.go | Serves single v4 artifacts inline and adds CSP sandbox header; keeps zip bundling for v1–v3. |
| routers/api/actions/artifactsv4.go | Adds MIME/type-aware create/list/download behavior and ServeDirect URL response header parameters. |
| routers/api/actions/artifact.proto | Adds mime_type to CreateArtifactRequest and sets go_package. |
| routers/api/actions/artifact.pb.go | Regenerated protobuf bindings for the updated proto. |
| modules/actions/artifacts.go | Changes v4 detection and adjusts v4 download (ServeDirect + fallback) to use stored MIME type/path. |
| models/actions/artifact.go | Updates v4 “finalized artifacts” filtering to match MIME-like content_encoding. |
Comments suppressed due to low confidence (1)
routers/api/actions/artifactsv4.go:277
getArtifactByNamenow selects byrun_id+artifact_name+content_encodingLIKE '/', but the DB uniqueness is(run_id, artifact_name, artifact_path). If the same artifact name exists with differentartifact_path(e.g..zipvs non-zipped v7), this query can return an arbitrary record and may not match theartifactIDthat was signed/verified. Consider looking up by the signedartifactID(and verifyartifact.RunID/ArtifactName), or includeartifact_pathin the lookup to make selection deterministic.
func (r *artifactV4Routes) getArtifactByName(ctx *ArtifactContext, runID int64, name string) (*actions.ActionArtifact, error) {
var art actions.ActionArtifact
has, err := db.GetEngine(ctx).Where(builder.Eq{"run_id": runID, "artifact_name": name}, builder.Like{"content_encoding", "/"}).Get(&art)
if err != nil {
return nil, err
} else if !has {
return nil, util.ErrNotExist
}
return &art, nil
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if len(artifacts) == 1 && actions.IsArtifactV4(artifacts[0]) { | ||
| ctx.Resp.Header().Set("Content-Disposition", fmt.Sprintf("inline; filename=%s; filename*=UTF-8''%s", url.PathEscape(artifacts[0].ArtifactPath), artifacts[0].ArtifactPath)) | ||
| ctx.Resp.Header().Set("Content-Security-Policy", "sandbox; default-src 'none';") |
There was a problem hiding this comment.
The Content-Disposition header uses filename* but the value is not RFC5987-encoded (it uses the raw ArtifactPath). This can produce an invalid header for spaces/non-ASCII and can lead to inconsistent filenames across browsers. Encode the filename* value (percent-encode UTF-8, similar to url.PathEscape/custom RFC5987 encoder) and consider quoting filename= as well.
| u, err := storage.ActionsArtifacts.URL(artifact.StoragePath, artifact.ArtifactPath, ctx.Req.Method, nil) | ||
| reqParams := url.Values{} | ||
| reqParams.Set("response-content-type", artifact.ContentEncoding) | ||
| reqParams.Set("response-content-disposition", fmt.Sprintf("inline; filename=%s; filename*=UTF-8''%s", url.PathEscape(artifact.ArtifactPath), artifact.ArtifactPath)) |
There was a problem hiding this comment.
The signed ServeDirect URL sets response-content-disposition using filename* but the value is not RFC5987-encoded (raw artifact.ArtifactPath). This can break filenames for spaces/non-ASCII when the storage backend returns the header. Encode the filename* value before embedding it in the disposition string.
| reqParams.Set("response-content-disposition", fmt.Sprintf("inline; filename=%s; filename*=UTF-8''%s", url.PathEscape(artifact.ArtifactPath), artifact.ArtifactPath)) | |
| reqParams.Set("response-content-disposition", fmt.Sprintf("inline; filename=%s; filename*=UTF-8''%s", url.PathEscape(artifact.ArtifactPath), url.PathEscape(artifact.ArtifactPath))) |
Closes #36829