fix: [SDK-4510] encode URL path segments and tighten alias validation

[fadi-george]

Description

1 Line Summary

Encode API request path segments at the network boundary and tighten alias label/id validation to harden URL construction.

Details

Internal hardening for SDK-4510.

• OneSignalApiBase.call now percent-encodes each segment of the request action via encodeRFC3986URIComponent before composing the URL, instead of trusting per-callsite sanitization. This is a single chokepoint that covers every endpoint defined in src/core/requests/api.ts.
• Removed the redundant inline sanitizedAlias block from updateUserByAlias since the encoding is centralized.
• validateAliasValue in src/onesignal/User.ts adds a 128-character limit (matching the documented public alias length) plus a small blocklist of characters that have no place in an alias label or id. Both addAlias/addAliases and removeAlias/removeAliases now flow through it for label and id.
• Bumped size-limit thresholds modestly (page.es6.js 42.32 -> 42.4 kB, sw.js 12.35 -> 12.4 kB) to fit the new code. Final sizes are 42.37 kB and 12.35 kB respectively.

Systems Affected

• WebSDK
• Backend
• Dashboard

Validation

Tests

Info

• New __test__/unit/http/urlEncoding.test.ts verifies the centralized encoding behavior at the OneSignalApiBase.call boundary across spaces, reserved chars, sub-delims, control chars, percent signs, and pass-through of URL-safe segments.
• Extended src/onesignal/UserNamespace.test.ts with cases covering the new label/id rules: length cap, traversal substring, dangerous characters, control characters, and a 128-character boundary acceptance test.

Checklist

• All the automated tests pass or I explained why that is not possible
• I have personally tested this on my machine or explained why that is not possible
• I have included test coverage for these changes or explained why they are not needed

Programming Checklist
Interfaces:

• Don't use default export
• New interfaces are in model files

Functions:

• Don't use default export
• All function signatures have return types
• Helpers should not access any data but rather be given the data to operate on.

Typescript:

• No Typescript warnings
• Avoid silencing null/undefined warnings with the exclamation point

Other:

• Iteration: refrain from using `elem of array` syntax. Prefer `forEach` or use `map`
• Avoid using global OneSignal accessor for `context` if possible. Instead, we can pass it to function/constructor so that we don't call `OneSignal.context`

Screenshots

Info

Not applicable; no UI changes.

Checklist

• I have included screenshots/recordings of the intended results or explained why they are not needed

---

Related Tickets

SDK-4510

Made with Cursor

[fadi-george]

@claude review

[claude]

LGTM — the regression I flagged was fixed in 997cf21 and the new tag round-trip test in UserNamespace.test.ts locks in the behavior.

Extended reasoning...

Overview

PR #1463 centralizes URL path-segment encoding in OneSignalApiBase.call and tightens alias label/id validation. Six files touched: the new encoding chokepoint in src/shared/api/base.ts, removal of the now-redundant inline encoder in src/core/requests/api.ts, the validation tightening + parameterized validateArray in src/onesignal/User.ts, a new __test__/unit/http/urlEncoding.test.ts, expanded coverage in src/onesignal/UserNamespace.test.ts, and size-limit threshold bumps.

Security risks

This is a defensive hardening PR, so the security posture improves rather than regresses: percent-encoding at the network boundary closes any per-callsite gaps where caller-supplied data could smuggle path separators, queries, or fragments into URLs, and the alias blocklist (/?#&=\s\x00-\x1F\x7F + ..) plus 128-char cap prevents the most common URL-injection vectors before they ever reach call. The encoding implementation correctly preserves / as a path separator while encoding sub-delims that encodeURIComponent leaves alone, and also encodes % so callers cannot smuggle pre-encoded sequences. No auth/crypto/permissions code is touched.

Level of scrutiny

This sits at the boundary between routine hardening and security-sensitive code. I gave it elevated scrutiny because input validation + URL construction are exactly the surfaces where subtle mistakes have outsized impact, and the validation tightening risked backwards-compatibility issues for existing customer data (which is what I caught in my prior review). With the parameterized validateArray fix and the new round-trip regression test, the tightening is correctly scoped to alias path segments only.

Other factors

The author was responsive: my flagged regression about tag keys being collaterally tightened was acknowledged and fixed within ~12 minutes with the cleaner of the two suggested approaches (parameterizing validateArray rather than duplicating the check in removeTags). The test coverage is solid — urlEncoding.test.ts exercises reserved chars, sub-delims, control bytes, percent signs, and URL-safe pass-through; UserNamespace.test.ts covers the 128-char boundary, traversal substrings, the full character blocklist, and the tag-key round-trip regression case. The bug hunting system found no further issues on the latest revision.