Skip to content
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
isUnknownInteractionError,
resolveSearchEngine,
normalizeSoundCloudUrl,
expandSoundCloudShortUrl,
} from '../queryUtils'
import {
resolveQueryWithFallbacks,
Expand Down Expand Up @@ -55,7 +56,10 @@ export async function executePlayHandler({
}

const rawQuery = interaction.options.getString('query', true)
const query = normalizeSoundCloudUrl(rawQuery)
// Expand SoundCloud short links first (on.soundcloud.com → full URL)
const expandedQuery = await expandSoundCloudShortUrl(rawQuery)
Comment thread
LucasSantana-Dev marked this conversation as resolved.
// Then normalize (strip ?in= params)
const query = normalizeSoundCloudUrl(expandedQuery)
const provider = interaction.options.getString('provider')
const collaborativeCheck = collaborativePlaylistService.canAddTracks(
interaction.guildId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,24 @@ jest.mock('@lucky/shared/utils', () => ({
warnLog: (...args: unknown[]) => warnLogMock(...args),
}))

import { normalizeSoundCloudUrl, isUrl, executePlayAtTop } from './queryUtils'
// Global fetch mock before any imports
const fetchMock = jest.fn() as jest.Mock

// Mock global.fetch before importing modules that use it
Object.defineProperty(global, 'fetch', {
writable: true,
value: fetchMock,
})

// Don't mock withTimeout - let it work with the real implementation
// which will properly await and resolve promises

import {
normalizeSoundCloudUrl,
isUrl,
executePlayAtTop,
expandSoundCloudShortUrl,
} from './queryUtils'

describe('normalizeSoundCloudUrl', () => {
it('strips ?in= playlist context from SoundCloud track URLs', () => {
Expand Down Expand Up @@ -114,6 +131,84 @@ describe('isUrl', () => {
})
})

describe('expandSoundCloudShortUrl', () => {
beforeEach(() => {
fetchMock.mockReset()
debugLogMock.mockReset()
})

it('expands on.soundcloud.com short links to full soundcloud.com URLs', async () => {
const shortUrl = 'https://on.soundcloud.com/abc123'
const fullUrl = 'https://soundcloud.com/artist/track'

fetchMock.mockResolvedValueOnce({
url: fullUrl,
})

const result = await expandSoundCloudShortUrl(shortUrl)

expect(result).toBe(fullUrl)
expect(fetchMock).toHaveBeenCalledWith(shortUrl, {
method: 'HEAD',
redirect: 'follow',
})
})

it('returns original URL if not a short link', async () => {
const fullUrl = 'https://soundcloud.com/artist/track'
const result = await expandSoundCloudShortUrl(fullUrl)
expect(result).toBe(fullUrl)
expect(fetchMock).not.toHaveBeenCalled()
})

it('returns original URL if redirect goes to non-soundcloud domain', async () => {
const shortUrl = 'https://on.soundcloud.com/abc123'
const suspiciousUrl = 'https://attacker.com/malware'

fetchMock.mockResolvedValueOnce({
url: suspiciousUrl,
})

const result = await expandSoundCloudShortUrl(shortUrl)

expect(result).toBe(shortUrl)
expect(debugLogMock).toHaveBeenCalledWith(
expect.objectContaining({
message:
'SoundCloud short URL expansion failed, using original URL',
}),
)
})

it('handles malformed on.soundcloud.com URLs gracefully', async () => {
const malformed = 'on.soundcloud.com/abc123' // No protocol

const result = await expandSoundCloudShortUrl(malformed)

expect(result).toBe(malformed)
expect(fetchMock).not.toHaveBeenCalled()
})

it('rejects domain-suffix bypass attempts (soundcloud.com.attacker.com)', async () => {
const shortUrl = 'https://on.soundcloud.com/abc123'
const bypassUrl = 'https://soundcloud.com.attacker.com/x'

fetchMock.mockResolvedValueOnce({
url: bypassUrl,
})

const result = await expandSoundCloudShortUrl(shortUrl)

expect(result).toBe(shortUrl)
expect(debugLogMock).toHaveBeenCalledWith(
expect.objectContaining({
message:
'SoundCloud short URL expansion failed, using original URL',
}),
)
})
})

describe('executePlayAtTop — fallback chain', () => {
const fakeTrack = { id: 'track-1', title: 'Test Song', author: 'Artist' }
const fakeQueue = {
Expand Down
77 changes: 77 additions & 0 deletions packages/bot/src/functions/music/commands/play/queryUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import { interactionReply } from '../../../../utils/general/interactionReply'
import { createUserFriendlyError } from '@lucky/shared/utils/general/errorSanitizer'
import { errorLog, debugLog, warnLog } from '@lucky/shared/utils'
import { withTimeout } from '@lucky/shared/utils/async'

export const DISCORD_UNKNOWN_INTERACTION_CODE = 10062

Expand All @@ -31,6 +32,82 @@
return query.startsWith('http://') || query.startsWith('https://')
}

/**
* Expands SoundCloud short links (on.soundcloud.com) by following the HTTP redirect.
* Returns the expanded URL if successful, or the original URL if expansion fails.
* Uses a 5-second timeout to prevent hanging requests.
*
* Security: only expands on.soundcloud.com hosts and validates the resolved URL
* is a soundcloud.com domain before returning it.
*/
export async function expandSoundCloudShortUrl(url: string): Promise<string> {
// Fast path: not a short link
if (!url.includes('on.soundcloud.com')) {
Comment thread
LucasSantana-Dev marked this conversation as resolved.
return url
}

try {
const parsed = new URL(url)
if (parsed.hostname !== 'on.soundcloud.com') {
return url
}
Comment thread
LucasSantana-Dev marked this conversation as resolved.

// Follow redirects with a 5-second timeout, using HEAD first (no body download)
const expanded = await withTimeout(
(async () => {
try {
const response = await (global.fetch as typeof fetch)(url, {

Check warning on line 59 in packages/bot/src/functions/music/commands/play/queryUtils.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This assertion is unnecessary since it does not change the type of the expression.

See more on https://sonarcloud.io/project/issues?id=LucasSantana-Dev_Lucky&issues=AZ6RA57hTng3e7MHK5J7&open=AZ6RA57hTng3e7MHK5J7&pullRequest=1177

Check warning on line 59 in packages/bot/src/functions/music/commands/play/queryUtils.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Prefer `globalThis` over `global`.

See more on https://sonarcloud.io/project/issues?id=LucasSantana-Dev_Lucky&issues=AZ6RA57hTng3e7MHK5J6&open=AZ6RA57hTng3e7MHK5J6&pullRequest=1177
method: 'HEAD',
redirect: 'follow',
})
Comment thread
LucasSantana-Dev marked this conversation as resolved.
const finalUrl = response.url
Comment thread
LucasSantana-Dev marked this conversation as resolved.

// Security check: ensure resolved URL is a soundcloud domain
const finalParsed = new URL(finalUrl)
if (
finalParsed.hostname !== 'soundcloud.com' &&
!finalParsed.hostname?.endsWith('.soundcloud.com')
) {
// Redirect went somewhere unexpected — reject and fall back
throw new Error(
`Redirect destination is not a soundcloud.com domain: ${finalUrl}`,
)
}

Comment thread
LucasSantana-Dev marked this conversation as resolved.
debugLog({
message: 'SoundCloud short URL expanded',
data: { originalUrl: url, expandedUrl: finalUrl },
})

return finalUrl
} catch (innerError) {
debugLog({
message:
'Failed to expand SoundCloud short URL in fetch block',
data: { url, error: String(innerError) },
})
throw innerError
}
})(),
5000,
'soundcloud-short-url-expansion',
)

return expanded
} catch (error) {
// Network error, timeout, or security validation failed — gracefully fall back
debugLog({
message:
'SoundCloud short URL expansion failed, using original URL',
data: {
originalUrl: url,
error: String(error),
},
})
return url
}
}

/**
* Strips SoundCloud playlist-context query params (`?in=...`) that the
* SoundCloud extractor cannot resolve. The bare track URL resolves correctly.
Expand Down
Loading