From e8eb46d20086d5c747543ded43ff388bc8bd6a01 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 20:29:36 +0000 Subject: [PATCH] Send an integer exp for video upload service auth tokens The legacy upload paths built the service auth expiry with `Date.now() / 1000 + 60 * 30`, which leaves the millisecond remainder in place and produces a fractional Unix timestamp. `com.atproto.server.getServiceAuth` types `exp` as an integer and serializes it straight into the query string, so the request fails validation and the upload dies before it starts. The multipart path already floored correctly, which is why only the legacy/fallback transports were affected. Rather than patch the two arithmetic expressions, add a shared `serviceAuthExp()` helper and route all three call sites through it, and floor inside `getServiceAuthToken` itself so a future call site that hand-rolls the arithmetic cannot reintroduce the bug. A non-finite exp now throws with a clear message instead of sending NaN. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_013T6CzrmaiVUkdWwi4FjnB2 --- .../video/__tests__/upload.shared.test.ts | 70 +++++++++++++++++++ src/lib/media/video/multipart/upload.ts | 4 +- src/lib/media/video/upload.shared.ts | 38 +++++++++- src/lib/media/video/upload.ts | 8 ++- src/lib/media/video/upload.web.ts | 8 ++- 5 files changed, 121 insertions(+), 7 deletions(-) create mode 100644 src/lib/media/video/__tests__/upload.shared.test.ts diff --git a/src/lib/media/video/__tests__/upload.shared.test.ts b/src/lib/media/video/__tests__/upload.shared.test.ts new file mode 100644 index 0000000000..7ab8b88ed3 --- /dev/null +++ b/src/lib/media/video/__tests__/upload.shared.test.ts @@ -0,0 +1,70 @@ +import {type Client} from '@atproto/lex' + +import {com} from '#/lexicons' +import { + getServiceAuthToken, + SERVICE_AUTH_TTL_SEC, + serviceAuthExp, +} from '../upload.shared' + +function createClient() { + const call = jest.fn().mockResolvedValue({token: 'token'}) + return {client: {call} as unknown as Client, call} +} + +describe('serviceAuthExp', () => { + it('returns an integer even when the clock has sub-second precision', () => { + jest.spyOn(Date, 'now').mockReturnValue(1_700_000_000_500) + expect(serviceAuthExp()).toBe(1_700_000_000 + SERVICE_AUTH_TTL_SEC) + expect(Number.isInteger(serviceAuthExp())).toBe(true) + }) + + it('accepts a custom ttl and keeps the result integral', () => { + jest.spyOn(Date, 'now').mockReturnValue(1_700_000_000_999) + expect(serviceAuthExp(90.7)).toBe(1_700_000_090) + }) +}) + +describe('getServiceAuthToken', () => { + it('floors a fractional exp before sending it', async () => { + const {client, call} = createClient() + await getServiceAuthToken({ + client, + aud: 'did:web:video.bsky.app', + lxm: 'com.atproto.repo.uploadBlob', + exp: 1_700_001_800.5, + }) + expect(call).toHaveBeenCalledWith(com.atproto.server.getServiceAuth, { + aud: 'did:web:video.bsky.app', + lxm: 'com.atproto.repo.uploadBlob', + exp: 1_700_001_800, + }) + }) + + it('leaves exp undefined when the caller omits it', async () => { + const {client, call} = createClient() + await getServiceAuthToken({ + client, + aud: 'did:web:video.bsky.app', + lxm: 'app.bsky.video.getUploadLimits', + }) + expect(call).toHaveBeenCalledWith(com.atproto.server.getServiceAuth, { + aud: 'did:web:video.bsky.app', + lxm: 'app.bsky.video.getUploadLimits', + exp: undefined, + }) + }) + + it('rejects a non-finite exp rather than sending NaN', async () => { + const {client, call} = createClient() + await expect( + getServiceAuthToken({ + client, + aud: 'did:web:video.bsky.app', + lxm: 'com.atproto.repo.uploadBlob', + exp: NaN, + }), + ).rejects.toThrow('Invalid service auth exp') + expect(call).not.toHaveBeenCalled() + }) +}) diff --git a/src/lib/media/video/multipart/upload.ts b/src/lib/media/video/multipart/upload.ts index d3e5210873..3b0048ecd2 100644 --- a/src/lib/media/video/multipart/upload.ts +++ b/src/lib/media/video/multipart/upload.ts @@ -5,7 +5,7 @@ import {AbortError} from '#/lib/async/cancelable' import {type CompressedVideo} from '#/lib/media/video/types' import {shouldRetryError} from '#/lib/strings/errors' import {type app} from '#/lexicons' -import {getServiceAuthToken} from '../upload.shared' +import {getServiceAuthToken, serviceAuthExp} from '../upload.shared' import {mimeToExt} from '../util' import { abortUpload, @@ -280,7 +280,7 @@ function createTokenProvider( async function get(forceRefresh = false) { if (!forceRefresh && token && Date.now() < expiresAt - 60_000) return token if (!refresh) { - const exp = Math.floor(Date.now() / 1000) + 60 * 30 + const exp = serviceAuthExp() refresh = getServiceAuthTokenWithRetry(client, dispatchUrl, exp, signal) .then(nextToken => { token = nextToken diff --git a/src/lib/media/video/upload.shared.ts b/src/lib/media/video/upload.shared.ts index 9e8d99a185..1ea0c62a9c 100644 --- a/src/lib/media/video/upload.shared.ts +++ b/src/lib/media/video/upload.shared.ts @@ -27,6 +27,11 @@ export async function getServiceAuthToken({ dispatchUrl?: string | URL aud?: string lxm: NsidString + /** + * Unix timestamp in *seconds* at which the token expires. Fractional values + * are floored - see {@link toIntegerExp}. Defaults to the server's own + * short expiry when omitted. + */ exp?: number }) { let resolvedAud = aud @@ -43,11 +48,42 @@ export async function getServiceAuthToken({ const {token} = await client.call(com.atproto.server.getServiceAuth, { aud: resolvedAud as DidString, lxm, - exp, + exp: exp === undefined ? undefined : toIntegerExp(exp), }) return token } +/** + * Default lifetime for the video upload service auth token. Long enough to + * cover a slow upload of a large file, short enough to limit the damage if the + * token leaks. + */ +export const SERVICE_AUTH_TTL_SEC = 60 * 30 + +/** + * Build a service auth `exp` claim `ttlSec` seconds from now. + * + * Always use this instead of hand-rolling the arithmetic: `Date.now()` is in + * milliseconds, and dividing by 1000 without flooring yields a fractional + * timestamp that the endpoint rejects. + */ +export function serviceAuthExp(ttlSec: number = SERVICE_AUTH_TTL_SEC) { + return Math.floor(Date.now() / 1000) + Math.floor(ttlSec) +} + +/** + * The lexicon types `exp` as an integer and it is serialized straight into the + * query string, so a fractional value fails validation and the upload dies + * before it starts. Floor here, at the single chokepoint every caller goes + * through, so a call site that forgets to cannot reintroduce the bug. + */ +function toIntegerExp(exp: number) { + if (!Number.isFinite(exp)) { + throw new Error(`Invalid service auth exp: ${exp}`) + } + return Math.floor(exp) +} + export async function getVideoUploadLimits(client: Client, i18n: I18n) { const token = await getServiceAuthToken({ client, diff --git a/src/lib/media/video/upload.ts b/src/lib/media/video/upload.ts index 287c8f09c6..d5a2af5146 100644 --- a/src/lib/media/video/upload.ts +++ b/src/lib/media/video/upload.ts @@ -13,7 +13,11 @@ import { import {Features, features} from '#/analytics/features' import {type app} from '#/lexicons' import {MultipartFallbackError, uploadVideoMultipart} from './multipart/upload' -import {getServiceAuthToken, getVideoUploadLimits} from './upload.shared' +import { + getServiceAuthToken, + getVideoUploadLimits, + serviceAuthExp, +} from './upload.shared' import {createVideoEndpointUrl, mimeToExt} from './util' export async function uploadVideo({ @@ -72,7 +76,7 @@ export async function uploadVideo({ client, dispatchUrl, lxm: 'com.atproto.repo.uploadBlob', - exp: Date.now() / 1000 + 60 * 30, // 30 minutes + exp: serviceAuthExp(), }) const uploadTask = createUploadTask( uri, diff --git a/src/lib/media/video/upload.web.ts b/src/lib/media/video/upload.web.ts index 90c10a0684..b0fcbaaf09 100644 --- a/src/lib/media/video/upload.web.ts +++ b/src/lib/media/video/upload.web.ts @@ -12,7 +12,11 @@ import { import {Features, features} from '#/analytics/features' import {type app} from '#/lexicons' import {MultipartFallbackError, uploadVideoMultipart} from './multipart/upload' -import {getServiceAuthToken, getVideoUploadLimits} from './upload.shared' +import { + getServiceAuthToken, + getVideoUploadLimits, + serviceAuthExp, +} from './upload.shared' import {createVideoEndpointUrl, mimeToExt} from './util' export async function uploadVideo({ @@ -79,7 +83,7 @@ export async function uploadVideo({ client, dispatchUrl, lxm: 'com.atproto.repo.uploadBlob', - exp: Date.now() / 1000 + 60 * 30, // 30 minutes + exp: serviceAuthExp(), }) if (signal.aborted) {