Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions pkg/artifacts/artifacts_v4.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,13 @@ func (r artifactV4Routes) verifySignature(ctx *ArtifactContext, endp string) (in
sig := ctx.Req.URL.Query().Get("sig")
expires := ctx.Req.URL.Query().Get("expires")
artifactName := ctx.Req.URL.Query().Get("artifactName")
dsig, _ := base64.URLEncoding.DecodeString(sig)
// Newer @actions/upload-artifact releases (v7+) upload blob content with
// the Azure storage SDK, which strips the `=` base64 padding from the `sig`
// query value when it re-serializes the signed URL to append comp/blockid.
// v4-era clients keep the padding. RawURLEncoding + TrimRight decodes both
// the padded and unpadded forms, so the signature check works across every
// supported artifact-action version.
dsig, _ := base64.RawURLEncoding.DecodeString(strings.TrimRight(sig, "="))
taskID, _ := strconv.ParseInt(rawTaskID, 10, 64)

expecedsig := r.buildSignature(endp, expires, artifactName, taskID)
Expand All @@ -244,7 +250,12 @@ func (r *artifactV4Routes) parseProtbufBody(ctx *ArtifactContext, req protorefle
ctx.Error(http.StatusInternalServerError, "Error decode request body")
return false
}
err = protojson.Unmarshal(body, req)
// DiscardUnknown keeps the artifact server forward-compatible with newer
// @actions/upload-artifact releases that add optional request fields the
// vendored protobuf doesn't know about yet (e.g. mime_type in v7+). act
// doesn't consume those fields, so silently dropping them is safe and
// avoids hard-failing the upload with `unknown field "..."`.
err = protojson.UnmarshalOptions{DiscardUnknown: true}.Unmarshal(body, req)
if err != nil {
log.Errorf("Error decode request body: %v", err)
ctx.Error(http.StatusInternalServerError, "Error decode request body")
Expand Down
57 changes: 57 additions & 0 deletions pkg/artifacts/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"net/http"
"net/http/httptest"
"net/url"
"os"
"path"
"path/filepath"
Expand Down Expand Up @@ -317,6 +318,62 @@ func runTestJobFile(ctx context.Context, t *testing.T, tjfi TestJobFileInfo) {
})
}

func TestCreateArtifactV4IgnoresUnknownFields(t *testing.T) {
assert := assert.New(t)

var memfs = fstest.MapFS(map[string]*fstest.MapFile{})

router := httprouter.New()
RoutesV4(router, "artifact/server/path", writeMapFS{memfs}, memfs)

// `mime_type` is sent by @actions/upload-artifact v7+ but isn't part of
// act's vendored protobuf. The server must ignore it instead of failing
// the upload with `unknown field "mime_type"`.
body := `{"workflow_run_backend_id":"1","workflow_job_run_backend_id":"1","name":"test","version":4,"mime_type":"application/zip"}`
req, _ := http.NewRequest("POST", "http://localhost"+path.Join(ArtifactV4RouteBase, "CreateArtifact"), strings.NewReader(body))
rr := httptest.NewRecorder()

router.ServeHTTP(rr, req)

assert.Equal(http.StatusOK, rr.Code)

response := CreateArtifactResponse{}
err := json.Unmarshal(rr.Body.Bytes(), &response)
assert.Nil(err)
assert.True(response.Ok)
}

func TestUploadArtifactV4AcceptsUnpaddedSignature(t *testing.T) {
assert := assert.New(t)

var memfs = fstest.MapFS(map[string]*fstest.MapFile{})

router := httprouter.New()
RoutesV4(router, "artifact/server/path", writeMapFS{memfs}, memfs)

// Build a genuine signed upload URL the same way the server does. The HMAC
// secret and inputs match what the router's verifySignature recomputes, so a
// well-formed request authorizes regardless of AppURL.
signer := artifactV4Routes{prefix: ArtifactV4RouteBase, AppURL: "localhost"}
signedURL := signer.buildArtifactURL("UploadArtifact", "test", 1)

// The Azure storage SDK in @actions/upload-artifact v7+ strips the `=`
// base64 padding from the `sig` query value when it re-serializes the URL.
// Reproduce that and confirm the server still authorizes the block upload.
parsed, err := url.Parse(signedURL)
assert.Nil(err)
q := parsed.Query()
q.Set("sig", strings.TrimRight(q.Get("sig"), "="))
q.Set("comp", "block")
parsed.RawQuery = q.Encode()

uploadReq, _ := http.NewRequest("PUT", parsed.String(), strings.NewReader("content"))
uploadRR := httptest.NewRecorder()
router.ServeHTTP(uploadRR, uploadReq)

assert.Equal(http.StatusCreated, uploadRR.Code)
}

func TestMkdirFsImplSafeResolve(t *testing.T) {
baseDir := "/foo/bar"

Expand Down