Skip to content

feat(sdk): sign audio uploads and send the uploading user id - #14550

Closed
rickyrombo wants to merge 1 commit into
mainfrom
feat/signed-audio-uploads
Closed

feat(sdk): sign audio uploads and send the uploading user id#14550
rickyrombo wants to merge 1 commit into
mainfrom
feat/signed-audio-uploads

Conversation

@rickyrombo

@rickyrombo rickyrombo commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Client half of a change to close a content-gating bypass. Server side is OpenAudio/go-openaudio#459 and OpenAudio/go-openaudio#460 — this can merge independently and is inert until those activate.

Why

Storage nodes are moving to attesting on chain which wallet uploaded a file, because that attestation is what will entitle the uploader to name the resulting cids on a track. Today anyone can read a gated track's track_cid off the public API and assert it on their own ungated track, and nothing can tell the difference.

A node can only attest to an uploader it can identify, and right now it cannot: tus metadata carries no wallet, no user id and no signature, so Upload.UserWallet is read from an unsigned metadata key the SDK never sends and sits NULL on every SDK upload.

What changes

uploadTrackFiles accepts userId and Storage accepts an audiusWalletClient. When both are present, audio uploads carry an EIP-712 signature over the user id and a timestamp, with both fields sent alongside so the validator can rebuild the typed data. They are reproduced inside the signed payload, so tampering with either in transit only breaks recovery — it cannot redirect the upload to another user.

Why typed data rather than signed JSON

An earlier revision hashed canonical JSON and used personal_sign. EIP-712 is better on two counts:

  • No canonicalization. The type definition is the encoding, so there is no key ordering or number formatting for client and server to agree on and later drift apart over.
  • Domain separation. A bare hashed payload has none, so a signature produced for any other purpose over the same two fields would be replayable as an upload authorization.

It also matches how this SDK already signs entity-manager writes, so both sides use standard calls rather than a hand-rolled hashing scheme.

The payload deliberately does not name the content: at upload-create time the bytes have not been sent and nobody knows the cid yet. Binding to content happens on the node's side, once transcoding produces cids it can attest to.

What is deliberately unchanged

Image uploads. They are served unauthenticated so there is no claim to protect, and requiring a signature would break signup, which uploads a profile picture before the account has a user id. The ordering nuance is that the Hedgehog wallet does exist by then — what does not is the user id, minted later inside createUserWithEntityManager. That is why the payload is keyed on the wallet.

Unsigned uploads still succeed. Without a wallet client or user id the upload proceeds and simply never earns an attestation, failing later at the point of claiming rather than blocking the upload.

Testing

5 tests covering the returned shape, that the signature verifies against the agreed domain and types, binding to both the user id and the timestamp, and the no-wallet error.

There is a matching fixture test on the validator side pinning a real signature produced by this code, so the two independently-written implementations cannot drift apart silently. I confirmed a viem-signed payload recovers to the expected address in Go before wiring it up.

Both packages/sdk and packages/common typecheck clean.

🤖 Generated with Claude Code

@changeset-bot

changeset-bot Bot commented Aug 6, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 8ec31ef

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
rickyrombo force-pushed the feat/signed-audio-uploads branch 2 times, most recently from 0dee8e0 to ed0ecb3 Compare August 7, 2026 06:35
@rickyrombo
rickyrombo force-pushed the feat/signed-audio-uploads branch from ed0ecb3 to 590911e Compare August 7, 2026 07:29
Storage nodes are moving to attesting on chain which wallet uploaded a file,
because that attestation is what will entitle the uploader to name the
resulting cids on a track. A node can only attest to an uploader it can
identify, and today it cannot: tus metadata carried no wallet, no user id and
no signature, so Upload.UserWallet was read from an unsigned metadata key the
SDK never sent and sat NULL on every SDK upload.

Audio uploads now carry an EIP-712 signature over the user id and a timestamp,
with both fields sent alongside so the validator can rebuild the typed data.
They are reproduced inside the signed payload, so tampering with either in
transit only breaks recovery — it cannot redirect the upload to another user.

Typed data rather than a hashed JSON payload for two reasons. It removes JSON
canonicalization from the protocol entirely: the type definition is the
encoding, so there is no key ordering or number formatting for client and
server to agree on and later drift apart over. And it provides domain
separation, which a bare hashed payload does not — without a domain, a
signature produced for any other purpose over the same two fields would be
replayable as an upload authorization. It also matches how the SDK already
signs entity-manager writes, so both sides use standard calls rather than a
hand-rolled hashing scheme.

The payload deliberately does not name the content. At upload-create time the
bytes have not been sent and nobody knows the cid yet; binding to content
happens on the node's side, once transcoding produces cids it can attest to.

There is a matching fixture test on the validator side pinning a real signature
produced here, so the two implementations cannot drift apart silently.

Image uploads are untouched. They are served unauthenticated so there is no
claim to protect, and requiring a signature would break signup, which uploads a
profile picture before the account has a user id — the wallet exists by then,
which is why the payload is keyed on the wallet rather than the user id.

Signing is best effort: without a wallet client or user id the upload proceeds
unsigned and simply never earns an attestation, failing later at the point of
claiming rather than blocking the upload itself.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@rickyrombo
rickyrombo force-pushed the feat/signed-audio-uploads branch from 590911e to 8ec31ef Compare August 7, 2026 09:13
rickyrombo added a commit that referenced this pull request Aug 7, 2026
Found while adding EIP-712 upload signing in #14550. Independent of that
stack.

`packages/sdk/src/sdk/utils/signatureSchemas.ts` exported `domains`,
`types`, and `generators` alongside `getNonce`, but **only `getNonce`
has ever been imported.** `EntityManagerClient` builds its typed data
from the contract package's `EntityManager.types` instead, and
`sdk/index.ts` re-exports only `parseParams`, `rendezvous`, `errors`,
and `hashId` — not this module. So the rest is unreachable, and removing
it is not a breaking change.

## Why bother

Not the line count. It is an attractive nuisance: it is the first thing
you find when adding EIP-712 signing to the SDK, and what you find is
the wrong definition — contract-shaped with `chainId` and
`verifyingContract`, and typing `userId` as `uint` rather than
`uint256`. That last one is what the `// @ts-ignore Need to update this
type to "uint32" instead of "uint"` in `EntityManagerClient` is working
around.

I reached for it myself before checking whether anything used it.

## Renamed to `nonce.ts`

Two reasons. It matches what is left, and there is a **live**
`packages/libs/src/data-contracts/signatureSchemas.js` — used by
`identity-service/rateLimiter`, `AudiusABIDecoder`, and libs' own
`EntityManagerClient`. That one is untouched here. Two files with the
same name where one is dead invites editing the wrong one; I conflated
them myself at first.

## Verification

`packages/sdk` and `packages/common` typecheck clean, the SDK builds,
and the EntityManager tests pass.

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

---------

Co-authored-by: Marcus Pasell <marcus@audius.co>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
rickyrombo added a commit that referenced this pull request Aug 8, 2026
#14556)

Follow-up bug fix from the content-auth review of #14550/#14552 —
independent of the redesign in #14555, works against today's servers.

## The bug

`useUpdateTrack` never passes `generatePreview` to
`sdk.tracks.updateTrack`, and the SDK's `updateTrack` only calls
`generate_preview` when that flag is set. So editing a gated track's
preview start point updates `preview_start_seconds` in the track
metadata while the track keeps streaming the old `preview_cid`'s clip —
the preview is never re-sliced at the new offset. As far as I can trace,
nothing else regenerates it (`populateTrackMetadataWithUploadResponseV2`
only touches `previewCid` when a new audio file was uploaded).

## The fix

Pass `generatePreview: true` when the preview start is set and differs
from the cached previous track's value:

- The SDK already guards the rest: it skips regeneration when a new
`audioFile` is present (transcoding produces the preview then) and when
`previewStartSeconds` is undefined.
- A cache miss for the previous track errs toward regenerating, which is
idempotent for an unchanged offset.

Once #14555 lands, this call site inherits the attributed
`generate_preview` request automatically — `updateTrack` threads its
`userId` internally.

## Testing

`packages/common` typechecks clean; eslint clean on the touched file.

🤖 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 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 in favor of #14555 — the unsigned-attribution redesign (server stack: OpenAudio/go-openaudio#477OpenAudio/go-openaudio#476). Uploads carry a required userId instead of an EIP-712 signature; the signed approach stays in this branch's history as the v2 contingency. Decision record on OpenAudio/go-openaudio#471.

@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