fix: validate URL against base URL when retreiving pages of items

.changeset/tangy-walls-return.md

@@ -0,0 +1,5 @@
+---
+"@smartthings/core-sdk": patch
+---
+
+validate URLs in paged data

.vscode/settings.json

@@ -11,5 +11,6 @@
 		"smartthingspi",
 		"sshpk"
 	],
-	"typescript.tsdk": "node_modules/typescript/lib"
+	"typescript.tsdk": "node_modules/typescript/lib",
+	"js/ts.tsdk.path": "node_modules/typescript/lib"
 }

src/endpoint-client.ts

@@ -36,7 +36,7 @@ export const chinaSmartThingsURLProvider: SmartThingsURLProvider = {
 
 export interface EndpointClientConfig {
 	authenticator: Authenticator
-	urlProvider?: SmartThingsURLProvider
+	urlProvider: SmartThingsURLProvider
 	logger?: Logger
 	loggingId?: string
 	version?: string
@@ -129,6 +129,11 @@ export class EndpointClient {
 	private logger: Logger
 
 	constructor(public readonly basePath: string, public readonly config: EndpointClientConfig) {
+		try {
+			new URL(config.urlProvider.baseURL) // throws if invalid URL
+		} catch (error) {
+			throw new Error(`Invalid base URL ${config.urlProvider.baseURL}: ${error}`)
+		}
 		this.logger = config.logger ? config.logger : noLogLogger
 	}
 
@@ -147,16 +152,35 @@ export class EndpointClient {
 		return this
 	}
 
-	private url(path?: string): string {
-		if (path) {
-			if (path.startsWith('/')) {
-				return `${this.config.urlProvider?.baseURL}${path}`
-			} else if (path.startsWith('https://')) {
+	private validateAndCalculateURL(path?: string): string {
+		const calculateURL = (): string => {
+			if (path && path.startsWith('http')) {
 				return path
 			}
-			return `${this.config.urlProvider?.baseURL}/${this.basePath}/${path}`
+
+			const baseURL = this.config.urlProvider.baseURL
+			if (path) {
+				// A path starting with a slash "breaks out of" the base path but not the base URL.
+				// Put another way, it ignores the path specified in the constructor but still uses the base URL.
+				return path.startsWith('/') ? `${baseURL}${path}` : `${baseURL}/${this.basePath}/${path}`
+			}
+			return `${baseURL}/${this.basePath}`
+		}
+
+		const calculatedURL = calculateURL()
+
+		const isSameOrigin = (candidate: string, baseURLString: string): boolean => {
+			const candidateURL = new URL(candidate)
+			const baseURL = new URL(baseURLString)
+
+			return candidateURL.origin === baseURL.origin
 		}
-		return `${this.config.urlProvider?.baseURL}/${this.basePath}`
+
+		if (!isSameOrigin(calculatedURL, this.config.urlProvider.baseURL)) {
+			throw Error(`illegal url ${calculatedURL} does not match base URL ${this.config.urlProvider.baseURL}`)
+		}
+
+		return calculatedURL
 	}
 
 	public async request<T = unknown>(method: HttpClientMethod, path?: string,
@@ -179,7 +203,7 @@ export class EndpointClient {
 		}
 
 		const axiosConfig: AxiosRequestConfig = {
-			url: this.url(path),
+			url: this.validateAndCalculateURL(path),
 			method,
 			headers: options?.headerOverrides ? { ...headers, ...options.headerOverrides } : headers,
 			params,

test/unit/apps.test.ts

@@ -11,7 +11,7 @@ const MOCK_APP_OAUTH_GENERATE = { oauthClientId: 'oauthClientId' } as GenerateAp
 
 describe('AppsEndpoint', () => {
 	const authenticator = new NoOpAuthenticator()
-	const apps = new AppsEndpoint({ authenticator })
+	const apps = new AppsEndpoint({ authenticator, urlProvider: { baseURL: 'https://example.com/baseURL' } })
 
 	const getSpy = jest.spyOn(EndpointClient.prototype, 'get')
 	const getPagedItemsSpy = jest.spyOn(EndpointClient.prototype, 'getPagedItems')

test/unit/channels.test.ts

@@ -16,7 +16,10 @@ describe('ChannelsEndpoint', () => {
 	const getPagedItemsSpy = jest.spyOn(EndpointClient.prototype, 'getPagedItems').mockImplementation()
 
 	const authenticator = new NoOpAuthenticator()
-	const channelsEndpoint = new ChannelsEndpoint({ authenticator })
+	const channelsEndpoint = new ChannelsEndpoint({
+		authenticator,
+		urlProvider: { baseURL: 'https://example.com/baseURL' },
+	})
 
 	test('create', async () => {
 		const createRequest = { name: 'channel-to-create' } as ChannelCreate

test/unit/devicepreferences.test.ts

@@ -12,7 +12,10 @@ const MOCK_LOCALE_LIST = [{ tag: 'tag' }] as LocaleReference[]
 
 describe('DevicePreferencesEndpoint', () => {
 	const authenticator = new NoOpAuthenticator()
-	const devicepreferences = new DevicePreferencesEndpoint({ authenticator })
+	const devicepreferences = new DevicePreferencesEndpoint({
+		authenticator,
+		urlProvider: { baseURL: 'https://example.com/baseURL' },
+	})
 
 	const getSpy = jest.spyOn(EndpointClient.prototype, 'get')
 	const getPagedItemsSpy = jest.spyOn(EndpointClient.prototype, 'getPagedItems')

test/unit/devices.test.ts

@@ -26,7 +26,11 @@ describe('DevicesEndpoint', () => {
 	const installedAppIdMock = jest.fn<string, [string | undefined]>()
 		.mockReturnValue('installed-app-id')
 
-	const devicesEndpoint = new DevicesEndpoint({ authenticator })
+	const baseConfig = {
+		authenticator,
+		urlProvider: { baseURL: 'https://example.com/baseURL' },
+	}
+	const devicesEndpoint = new DevicesEndpoint(baseConfig)
 	devicesEndpoint.locationId = locationIdMock
 	devicesEndpoint.installedAppId = installedAppIdMock
 
@@ -44,7 +48,7 @@ describe('DevicesEndpoint', () => {
 		})
 
 		it('includes configured locationId', async () => {
-			const devices = new DevicesEndpoint({ authenticator, locationId: 'configured-location-id' })
+			const devices = new DevicesEndpoint({ ...baseConfig, locationId: 'configured-location-id' })
 			expect(await devices.list()).toBe(deviceList)
 
 			expect(getPagedItemsSpy).toHaveBeenCalledTimes(1)
@@ -76,7 +80,7 @@ describe('DevicesEndpoint', () => {
 
 	describe('listInLocation', () => {
 		it('works on happy path', async () => {
-			const devices = new DevicesEndpoint({ authenticator, locationId: 'configured-location-id' })
+			const devices = new DevicesEndpoint({ ...baseConfig, locationId: 'configured-location-id' })
 			const listSpy = jest.spyOn(devices, 'list').mockResolvedValue(deviceList)
 
 			expect(await devices.listInLocation()).toBe(deviceList)
@@ -101,7 +105,7 @@ describe('DevicesEndpoint', () => {
 
 	describe('findByCapability', () => {
 		it('works on happy path', async () => {
-			const devices = new DevicesEndpoint({ authenticator, locationId: 'unused-in-test' })
+			const devices = new DevicesEndpoint({ ...baseConfig, locationId: 'unused-in-test' })
 			devices.locationId = locationIdMock
 			const listSpy = jest.spyOn(devices, 'list').mockResolvedValue(deviceList)
 
@@ -363,7 +367,7 @@ describe('DevicesEndpoint', () => {
 
 	test('executeCommand', async () => {
 		// create a new instance of devices so we can spy on it and not affect other tests
-		const devices = new DevicesEndpoint({ authenticator })
+		const devices = new DevicesEndpoint(baseConfig)
 		const executeCommandsSpy = jest.spyOn(devices, 'executeCommands')
 			.mockResolvedValueOnce(commandResponse)
 		const command = { command: 'command-1' } as Command
@@ -495,7 +499,7 @@ describe('DevicesEndpoint', () => {
 			// do nothing
 		})
 
-		const devices = new DevicesEndpoint({ authenticator })
+		const devices = new DevicesEndpoint(baseConfig)
 
 		expect(await devices.createEvents('device-id', events)).toBe(SuccessStatusValue)
 
@@ -506,7 +510,7 @@ describe('DevicesEndpoint', () => {
 	test('sendEvents', async () => {
 		const events = { deviceEvents: [] }
 
-		const devices = new DevicesEndpoint({ authenticator })
+		const devices = new DevicesEndpoint(baseConfig)
 
 		await devices.sendEvents('device-id', events)
 
@@ -518,7 +522,7 @@ describe('DevicesEndpoint', () => {
 		const expected = {} as PresentationDevicePresentation
 		getSpy.mockResolvedValueOnce(expected)
 
-		const devices = new DevicesEndpoint({ authenticator })
+		const devices = new DevicesEndpoint(baseConfig)
 
 		expect(await devices.getPresentation('device-id')).toBe(expected)
 
@@ -530,7 +534,7 @@ describe('DevicesEndpoint', () => {
 		const expected = {} as DevicePreferenceResponse
 		getSpy.mockResolvedValueOnce(expected)
 
-		const devices = new DevicesEndpoint({ authenticator })
+		const devices = new DevicesEndpoint(baseConfig)
 
 		expect(await devices.getPreferences('device-id')).toBe(expected)
 

test/unit/drivers.test.ts

@@ -14,7 +14,10 @@ describe('DriversEndpoint', () => {
 	const requestSpy = jest.spyOn(EndpointClient.prototype, 'request').mockImplementation()
 
 	const authenticator = new NoOpAuthenticator()
-	const driversEndpoint = new DriversEndpoint({ authenticator })
+	const driversEndpoint = new DriversEndpoint({
+		authenticator,
+		urlProvider: { baseURL: 'https://example.com/baseURL' },
+	})
 
 	test('get', async () => {
 		const driver = { driverId: 'driver-id' }