Skip to content

fix(sdk): close remaining unsigned audio upload paths from review - #14554

Closed
rickyrombo wants to merge 1 commit into
feat/signed-preview-generationfrom
fix/signed-upload-review-fixes
Closed

fix(sdk): close remaining unsigned audio upload paths from review#14554
rickyrombo wants to merge 1 commit into
feat/signed-preview-generationfrom
fix/signed-upload-review-fixes

Conversation

@rickyrombo

Copy link
Copy Markdown
Contributor

Stacked on #14552 (which is stacked on #14550). Fixes for the issues found reviewing those two — kept separate so the fixes can be reviewed on their own.

Bug: stale userId closures in useUpload

uploadTrackFiles and uploadStemFiles read userId but did not list it in their useCallback deps. On a cold load where the upload page renders before useCurrentAccount resolves, both callbacks memoize over userId = undefined and are never recreated when the account arrives — every upload from that session goes out unsigned. Silent today; once enforcement is on it becomes either a rejected upload or an unclaimable cid. One-line fix in each array.

Gap: web stems saga never signed

stemsUpload/sagas.ts calls uploadTrackFiles({ audioFile }) with no user, even though the decoded userId is already in scope a few lines up. Stems added to an existing track through this flow would publish fine today and fail once the gate opens. Now passes Id.parse(userId) like the useUpload stem path does.

Gap: the uploads API surface could not sign at all

Two halves, both fixed:

  • UploadsApi.createAudioUpload takes an optional encoded userId (the form callers hold — both examples pass their OAuth profile id straight through) and threads the decoded id into the file metadata.
  • The Storage built inside createSdk now receives services.audiusWalletClient, matching what feat(sdk): sign audio uploads and send the uploading user id #14550 did for createSdkWithServices. Without this, a wallet-configured SDK still uploaded unsigned through sdk.uploads.

Both examples updated to pass the id. As before, no wallet or no id means the upload proceeds unsigned and simply never earns an attestation.

Cleanups

  • publishTrack used decodeHashId(params.userId) with the schema-parsed userId already in scope; now passes the parsed value. (The identical expression in updateTrack stays — nothing parsed is in scope there.)
  • generatePreview JSDoc marks userId optional.

Testing

The vitest failure reported in #14552 (Object.defineProperty called on non-object at collection) no longer reproduces — the previously-blocked suites now run and pass. Added Storage.test.ts covering generatePreview: signed requests carry signature/userId/timestamp query params that recover to the signer against the pinned EIP-712 shape, no-wallet and no-userId requests go out with an empty query string, and non-ok responses throw.

  • packages/sdk: 13/13 tests pass (signUpload, Storage, StorageNodeSelector), typecheck clean
  • packages/common, packages/web: typecheck clean
  • eslint clean on all touched files (the mobile example has 16 pre-existing lint errors untouched by this change)

🤖 Generated with Claude Code

Fixes from review of #14550/#14552:

- useUpload: add userId to the two useCallback dep arrays that read it.
  Without it, an account that resolves after the callbacks are memoized
  leaves them closed over userId=undefined forever, silently producing
  unsigned (unclaimable) uploads.
- stemsUpload saga: pass the acting user's id to uploadTrackFiles. This
  flow had no signature at all, so stems added to an existing track
  would fail to publish once content authorization is enforced.
- UploadsApi.createAudioUpload: accept an optional encoded userId, and
  wire audiusWalletClient into the Storage built by createSdk, so the
  uploads API surface can sign at all. Examples updated to pass it.
- publishTrack: use the already-parsed userId instead of re-decoding
  the raw param.
- Storage.generatePreview: mark userId optional in the JSDoc; add tests
  covering signed/unsigned request construction and signer recovery.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Aug 8, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: aa69da2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@audius/sdk Minor
@audius/sdk-legacy Patch
@audius/protocol-dashboard Patch
@audius/sp-actions Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

rickyrombo added a commit that referenced this pull request Aug 8, 2026
… (no signatures) (#14555)

Fresh implementation of upload/preview attribution on the unsigned
design path. **Supersedes #14550, #14552 and #14554** — the client-side
EIP-712 signing approach — which can be closed in favor of this. Server
counterpart: `OpenAudio/go-openaudio` branch
`feat/content-auth-unsigned` (PR opening alongside), stacked on
OpenAudio/go-openaudio#459.

## Design

Audio uploads and `generate_preview` carry the id of the user they are
made for — plain tus metadata / query parameter, **no signature and no
wallet client**. The id is an assertion; what makes the scheme safe is
unchanged from the analysis on the server PR:

- Asserting someone else's id only ever credits *them* — exercising a
claim (naming the cid on a track) happens in a signed, grant-checked
entity-manager write.
- Upload claims derive from bytes the node received; preview claims
require the asserted user to already claim the source cid.
- A minted preview cid is useless to the requester: audio never serves
by bare cid, and discovery only signs track-resolved cidstream URLs.

Dropping the signature is what keeps every SDK flow working — including
OAuth apps with no client-side wallet — with nothing but this parameter.
No proxy, no wallet plumbing, no timestamp windows.

## BREAKING (major changeset included)

- `tracks.uploadTrackFiles` requires `userId` (encoded)
- `uploads.createAudioUpload` requires `userId` (encoded)
- `Storage.generatePreview` requires `userId` (decoded) and sends it as
a query param

High-level methods (`createTrack`, `updateTrack`, `uploadTrack`,
`publishTrack`) already required `userId` and now thread it through —
their callers need no changes. Required rather than optional so an
upgraded integrator cannot silently produce unclaimable uploads that
fail later at publish. Always explicit, never derived from auth state —
a manager or developer-app session can act for more than one user (same
convention as `ChatsApi.currentUserId`).

## Client updates

- `useUpload`: current account's id on
track/stem/cover-art/collection-artwork uploads, `requireUserId()` guard
so a missing account fails loudly at the call site, and the id is in
every dependency array that reads it
- Web stems saga passes the id it already looked up
- All four upload examples (web upload / gated-upload / upload-server,
mobile upload) + READMEs
- Docs site: `sdk/uploads` and `sdk/tracks` pages document the parameter
and why it exists

## Testing

- sdk: vitest 20/20 (`Storage.test.ts` pins the `userId` query param on
`generatePreview`), typecheck clean
- common, web: typecheck clean; eslint clean on touched files (2
pre-existing warnings in examples untouched)
- Example-app tsconfigs resolve the *published* npm SDK, so their
pre-existing typecheck errors are unrelated and unchanged

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Marcus Pasell <marcus@audius.co>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@rickyrombo

Copy link
Copy Markdown
Contributor Author

Closing — these fixes targeted the signed stack (#14550/#14552). Their still-relevant parts (dep arrays, stems saga userId, publishTrack cleanup, example updates) are folded into #14555; the stale-preview-on-edit bug found in the same review is fixed separately in #14556.

@rickyrombo rickyrombo closed this Aug 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant