From bfe55997be6b0bc22d3a8c84f3af5f53c54ab10c Mon Sep 17 00:00:00 2001 From: Dylan Jeffers Date: Fri, 4 Sep 2026 12:44:20 -0700 Subject: [PATCH] fix(api): let an artist download their own track with downloads off The download endpoint only ever serves `track.Download`, which dbv1 populates solely for tracks the public may download (`is_downloadable`). About four in five tracks on the network have that off, and for those the artist's own "Download File" button on the edit page - the one paired with "Replace File", which exists precisely so an artist can retrieve what they uploaded - got a 404 and surfaced it as "Something went wrong. Please check your connection and storage and try again." Fall back to signing a link for a requester who proves, with the wallet signature already on the request, that they own the track or manage the account that does. Access for everyone else is unchanged: a stranger, signed or not, still gets the 404 the artist asked for. Ownership is read from the recovered wallet rather than the user_id query param behind myId. user_id is the caller's own claim; it happens to be verified on this route today only because the route sits off authMiddleware's advisory- user_id allowlist, and an artist's original master should not depend on that list continuing to exclude it. Also fix the served filename for the case this now makes reachable: with no orig_file_cid the bytes are the mp3 transcode, so the name must not carry the uploaded file's extension - a .wav name on mp3 bytes is a file most editors refuse to open. Co-Authored-By: Claude Opus 5 --- api/dbv1/media_link.go | 24 +++++++ api/dbv1/tracks.go | 6 +- api/v1_track_download.go | 62 ++++++++++++++--- api/v1_track_download_test.go | 124 ++++++++++++++++++++++++++++++++++ 4 files changed, 203 insertions(+), 13 deletions(-) diff --git a/api/dbv1/media_link.go b/api/dbv1/media_link.go index e782c416..0254441b 100644 --- a/api/dbv1/media_link.go +++ b/api/dbv1/media_link.go @@ -93,3 +93,27 @@ func generateSignature(data map[string]interface{}) (string, error) { return hexutil.Encode(signature), nil } + +// DownloadCid is the blob a download serves: the artist's original upload when +// the row kept one, otherwise the transcode. Empty when the row has neither, +// which happens on uploads whose cid backfill never ran - signing an empty cid +// only produces a content-node URL guaranteed to 404. +func (t *GetTracksRow) DownloadCid() string { + if t.OrigFileCid.String != "" { + return t.OrigFileCid.String + } + return t.TrackCid.String +} + +// SignDownloadLink signs a download URL for this track without consulting +// is_downloadable or the download gates. Nothing here re-checks access, so the +// caller must already have established that the requester may bypass them - +// today only a wallet-verified owner or account manager does, see +// ApiServer.ownerDownloadLink. Returns nil when the row has no cid to sign. +func (t *Track) SignDownloadLink(userId int32) (*MediaLink, error) { + cid := t.DownloadCid() + if cid == "" { + return nil, nil + } + return mediaLink(cid, t.TrackID, userId, nil) +} diff --git a/api/dbv1/tracks.go b/api/dbv1/tracks.go index 90eaf33b..59461144 100644 --- a/api/dbv1/tracks.go +++ b/api/dbv1/tracks.go @@ -224,11 +224,7 @@ func (q *Queries) TracksKeyed(ctx context.Context, arg TracksParams) (map[int32] var download *MediaLink if isStreamable && rawTrack.IsDownloadable && access.Download { - cid := rawTrack.OrigFileCid.String - if cid == "" { - cid = rawTrack.TrackCid.String - } - if cid != "" { + if cid := rawTrack.DownloadCid(); cid != "" { download, err = mediaLink(cid, rawTrack.TrackID, arg.MyID.(int32), nil) if err != nil { return nil, err diff --git a/api/v1_track_download.go b/api/v1_track_download.go index 674b7e77..23e0ce17 100644 --- a/api/v1_track_download.go +++ b/api/v1_track_download.go @@ -1,16 +1,28 @@ package api import ( + "path" + "strings" + "api.audius.co/api/dbv1" "github.com/gofiber/fiber/v2" ) func createFilename(track *dbv1.Track) string { - filename := track.OrigFilename.String - if filename == "" && track.OrigFileCid.String == "" { - filename = track.Title.String + ".mp3" + // The original upload is what a download serves whenever the row kept one, + // so its own name is the right one. + if track.OrigFileCid.String != "" && track.OrigFilename.String != "" { + return track.OrigFilename.String + } + + // Otherwise the bytes are the mp3 transcode, and the name must not promise + // the format the artist uploaded: a .wav name on mp3 bytes is a file most + // editors refuse to open. Only the recorded filename is stripped of its + // extension - a title is free text and "Vol. 2" has no extension to trim. + if name := track.OrigFilename.String; name != "" { + return strings.TrimSuffix(name, path.Ext(name)) + ".mp3" } - return filename + return track.Title.String + ".mp3" } type trackDownloadParams struct { @@ -49,15 +61,27 @@ func (app *ApiServer) v1TrackDownload(c *fiber.Ctx) error { return fiber.NewError(fiber.StatusNotFound, "track not found") } - if !track.Access.Download { - return fiber.NewError(fiber.StatusForbidden, "you are not allowed to download this track") + // track.Download is only populated for tracks the public may download, so + // an artist who left downloads off - about four in five tracks - could not + // get their own file back. The edit page's "Download File" button and the + // replace-file flow both come through here, and both were 404ing for the + // owner of the track, surfacing as a generic "something went wrong". + downloadLink := track.Download + if downloadLink == nil { + downloadLink, err = app.ownerDownloadLink(c, &track) + if err != nil { + return err + } } - if track.Download == nil { + if downloadLink == nil { + if !track.Access.Download { + return fiber.NewError(fiber.StatusForbidden, "you are not allowed to download this track") + } return fiber.NewError(fiber.StatusNotFound, "track is not downloadable") } - downloadUrl := tryFindWorkingUrl(track.Download) + downloadUrl := tryFindWorkingUrl(downloadLink) q := downloadUrl.Query() q.Set("skip_play_count", "true") @@ -70,3 +94,25 @@ func (app *ApiServer) v1TrackDownload(c *fiber.Ctx) error { return c.Redirect(downloadUrl.String(), fiber.StatusFound) } + +// ownerDownloadLink signs a download link for a requester who has proven they +// own the track, or manage the account that does. Ownership comes from the +// wallet recovered from the request signature, not from the user_id query +// param behind myId: user_id is the caller's own claim, and it is only +// trustworthy here because this route sits off authMiddleware's advisory- +// user_id allowlist. Handing out an artist's original master should not rest +// on that list continuing to exclude this route. Returns nil - not an error - +// for everyone else, which leaves the caller's 404 in place. +func (app *ApiServer) ownerDownloadLink(c *fiber.Ctx, track *dbv1.Track) (*dbv1.MediaLink, error) { + wallet := app.tryGetAuthedWallet(c) + if wallet == "" { + return nil, nil + } + + ownerId := track.GetTracksRow.UserID + if !app.isAuthorizedRequest(c.Context(), ownerId, wallet) { + return nil, nil + } + + return track.SignDownloadLink(ownerId) +} diff --git a/api/v1_track_download_test.go b/api/v1_track_download_test.go index 888ce3e5..b7b3daec 100644 --- a/api/v1_track_download_test.go +++ b/api/v1_track_download_test.go @@ -4,6 +4,7 @@ import ( "net/http/httptest" "testing" + "api.audius.co/api/testdata" "api.audius.co/database" "api.audius.co/trashid" "github.com/stretchr/testify/assert" @@ -74,3 +75,126 @@ func TestGetTrackDownload_Original(t *testing.T) { assert.Contains(t, res.Header.Get("Location"), "signature=%7B%22data%22%3A%22%7B%5C%22cid%5C%22%3A%5C%22QmX123%5C%22%2C%5C%22timestamp%5C%22%3") assert.Contains(t, res.Header.Get("Location"), "filename=DharitRocks.wav") } + +const ( + ownerWallet = "0x7d273271690538cf855e5b3002a0dd8c154bb060" + strangerWallet = "0xc3d1d41e6872ffbd15c473d14fc3a9250be5b5e0" + managerWallet = "0x4954d18926ba0ed9378938444731be4e622537b2" +) + +// seedNonDownloadableTrack sets up the shape most tracks on the network have: +// an artist who never turned downloads on, whose original upload is still +// sitting on the content node. +func seedNonDownloadableTrack(t *testing.T) *ApiServer { + app := emptyTestApp(t) + database.Seed(app.pool.Replicas[0], database.FixtureMap{ + "tracks": []map[string]any{ + { + "track_id": 1, + "owner_id": 1, + "title": "Private Master", + "track_cid": "QmTranscode", + "orig_file_cid": "QmOriginal", + "orig_filename": "PrivateMaster.wav", + "is_downloadable": false, + }, + }, + "users": []map[string]any{ + {"user_id": 1, "handle": "artist", "wallet": ownerWallet}, + {"user_id": 2, "handle": "stranger", "wallet": strangerWallet}, + {"user_id": 3, "handle": "manager", "wallet": managerWallet}, + }, + }) + return app +} + +func downloadWithWallet(t *testing.T, app *ApiServer, path string, wallet string) (int, string) { + req := httptest.NewRequest("GET", path, nil) + if wallet != "" { + sigData := testdata.GetSignatureData(wallet) + req.Header.Set("Encoded-Data-Message", sigData.Message) + req.Header.Set("Encoded-Data-Signature", sigData.Signature) + } + res, err := app.Test(req, -1) + assert.NoError(t, err) + return res.StatusCode, res.Header.Get("Location") +} + +// An artist must be able to get their own upload back even with downloads off +// for everyone else - that is what the edit page's "Download File" button does. +func TestGetTrackDownload_OwnerOfNonDownloadableTrack(t *testing.T) { + app := seedNonDownloadableTrack(t) + path := "/v1/tracks/" + trashid.MustEncodeHashID(1) + "/download" + + status, location := downloadWithWallet(t, app, path, ownerWallet) + assert.Equal(t, 302, status) + assert.Contains(t, location, "tracks/cidstream/QmOriginal") + assert.Contains(t, location, "filename=PrivateMaster.wav") +} + +// A manager the artist granted access to acts for them here too. +func TestGetTrackDownload_ManagerOfNonDownloadableTrack(t *testing.T) { + app := seedNonDownloadableTrack(t) + database.SeedTable(app.pool.Replicas[0], "grants", []map[string]any{ + { + "user_id": 1, + "grantee_address": managerWallet, + "is_approved": true, + "is_revoked": false, + }, + }) + + status, location := downloadWithWallet(t, app, + "/v1/tracks/"+trashid.MustEncodeHashID(1)+"/download", managerWallet) + assert.Equal(t, 302, status) + assert.Contains(t, location, "tracks/cidstream/QmOriginal") +} + +// Everyone else still gets the 404 the artist asked for by leaving downloads +// off. Claiming to be the owner through the user_id query param must not be +// enough: the bypass keys on the recovered signature, and the auth middleware +// separately refuses a user_id no signature backs (403 rather than 404). +func TestGetTrackDownload_NonOwnerOfNonDownloadableTrack(t *testing.T) { + app := seedNonDownloadableTrack(t) + trackPath := "/v1/tracks/" + trashid.MustEncodeHashID(1) + "/download" + asOwner := trackPath + "?user_id=" + trashid.MustEncodeHashID(1) + + status, _ := downloadWithWallet(t, app, trackPath, "") + assert.Equal(t, 404, status, "anonymous") + + status, _ = downloadWithWallet(t, app, trackPath, strangerWallet) + assert.Equal(t, 404, status, "signed by another wallet") + + status, _ = downloadWithWallet(t, app, asOwner, "") + assert.Equal(t, 403, status, "user_id claiming to be the owner, unsigned") + + status, _ = downloadWithWallet(t, app, asOwner, strangerWallet) + assert.Equal(t, 403, status, "user_id claiming to be the owner, signed by another wallet") +} + +// With no original kept, the download serves the mp3 transcode, so the name it +// is served under has to say mp3 rather than the format that was uploaded. +func TestGetTrackDownload_FilenameFallsBackToMp3(t *testing.T) { + app := emptyTestApp(t) + database.Seed(app.pool.Replicas[0], database.FixtureMap{ + "tracks": []map[string]any{ + { + "track_id": 1, + "owner_id": 1, + "title": "Vol. 2", + "track_cid": "QmTranscode", + "orig_filename": "Vol. 2.wav", + "is_downloadable": true, + }, + }, + "users": []map[string]any{ + {"user_id": 1, "handle": "artist", "wallet": ownerWallet}, + }, + }) + + status, location := downloadWithWallet(t, app, + "/v1/tracks/"+trashid.MustEncodeHashID(1)+"/download", "") + assert.Equal(t, 302, status) + assert.Contains(t, location, "tracks/cidstream/QmTranscode") + assert.Contains(t, location, "filename=Vol.+2.mp3") +}