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

[rossiam]

• require baseURL in URLProvider (seems like a breaking change but would always fail later if this were left out)
• validate baseURL in constructor
• validate all URLs against baseURL
• updated unit tests
  • include tests for new validations
  • update several other tests to include the now-required baseURL

[changeset-bot]

🦋 Changeset detected

Latest commit: 54d8326

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

This PR includes changesets to release 1 package

Name	Type
@smartthings/core-sdk	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

[erodewald]

Also rest-client.ts's RESTClientConfig still lets urlProvider be undefined at runtime. If they pass in undefined, it clobbers the default via spread, and TS won't catch that until RESTClientConfig is tightened.
e.g.,

this.config = { ...defaultConfig, ...config, urlProvider: config?.urlProvider ?? globalSmartThingsURLProvider, headers }

[erodewald]

.startsWith(...) is not enough to prevent an attack, because globalSmartThingsURLProvider.baseURL is 'https://api.smartthings.com' with no trailing slash, so the prefix has no authority boundary.

A malicious href could be https://api.smartthings.com.evil.example/steal and it would pass this check, but it's actually contacting api.smartthings.com.evil.example instead.

The same problem occurs with path-scoped base urls: baseURL: 'https://example.com/baseURL'

Suggest adding a new method:

const isWithinBase = (candidate: string, baseURL: string): boolean => {
	let url: URL, base: URL
	try {
		url = new URL(candidate)
		base = new URL(baseURL)
	} catch {
		return false
	}
	if (url.origin !== base.origin || url.username || url.password) {
		return false
	}
	const basePath = base.pathname.replace(/\/$/, '')
	return url.pathname === basePath || url.pathname.startsWith(`${basePath}/`)
}

origin comparison covers scheme, host, and port. The explicit username/password rejection makes the userinfo case obvious to a future reader rather than an accident of origin semantics.

[rossiam]

Added isSameOrigin to check the origins from created URL instances.

[erodewald]

This doesn't exercise the actual threat model. That's a "same origin, different path" URL. The entire suite passes with the vulnerable prefix check in place, so it gives no signal on the actual attack. Worth adding cases for https://example.com.evil.test/x, https://example.com@evil.test/x, and https://example.com/baseURLevil.

Separately, this test mocks client.get, so it only exercises the redundant outer call in getPagedItems and never reaches the real defense inside request(). There's also no coverage of the new throw Error('baseURL is not configured') branch.

[rossiam]

I've updated the tests now. I moved them to under request instead of in getPagedItems and removed the redundant call to the validateAndCalculateURL.

[erodewald]

Technically a breaking change though. Making urlProvider required on the exported EndpointClientConfig breaks compilation for anyone constructing EndpointClient or that config type directly, which your PR description acknowledges. Should be major under changesets, or drop the interface change and rely on the runtime guard alone. I'd suggest dropping the interface change since a major version for a security hotfix is unlikely to be picked up with haste.

[rossiam]

I've fixed my comment to describe why this isn't actually a breaking change. (Leaving it out would have just failed later.)