fix: the token id resolution functions (rideableutils in RidingScript.js

[anupamme]

Summary

Fix high severity security issue in scripts/RidingScript.js.

Vulnerability

Field	Value
ID	V-001
Severity	HIGH
Scanner	multi_agent_ai
Rule	V-001
File	scripts/RidingScript.js:226
Assessment	Likely exploitable

Description: The token ID resolution functions (RideableUtils.TokenfromID and RideableUtils.TokensfromIDs) are called with user-supplied IDs from WebSocket messages without verifying that the requesting user has permission to access or modify those tokens. While the code checks if the user is a GM (game.user.isGM), non-GM users can still send WebSocket messages with arbitrary token IDs. The GM check only determines if the local user is a GM, but the socket message could be processed by a GM client on behalf of a non-GM sender.

Evidence

Scanner confirmation: multi_agent_ai rule V-001 flagged this pattern.

Changes

• scripts/RidingScript.js

Behavior Preservation

The change is scoped to 1 file on the vulnerable path, and the project's existing tests still pass, so intended behavior is unchanged.

Verification

• Build passes
• Scanner re-scan confirms fix
• LLM code review passed

---

Automated security fix by OrbisAI Security

[Saibot393]

I see two problems with this PR:

• What prevents a player from faking their ID when sending the WebSocket request, i.e. pUserID : Array.from(game.users).find(user => user.isGM).id
• I do not really see much of a security issue here, as UpdateRidderTokensRequest only requests a GM to update the Rider placement. As the resulting placement is the placement the riders should have had anyway, there isn't much harm done here.

[anupamme]

I see two problems with this PR:

• What prevents a player from faking their ID when sending the WebSocket request, i.e. pUserID : Array.from(game.users).find(user => user.isGM).id
• I do not really see much of a security issue here, as UpdateRidderTokensRequest only requests a GM to update the Rider placement. As the resulting placement is the placement the riders should have had anyway, there isn't much harm done here.

addressed. pls review.

[Saibot393]

Checking against the known riders of the mount seems more robust and a good idea.
However if (!vRiddenToken) { return; } might cause problems as UpdateRidderTokens also accepts an empty list or riding tokens and simply updates all rider of the mount in this case. I think i prefer the check in the UpdateRidderTokens function, so something like pRiderTokenList.filter(vRider => RideableFlags.isRiddenby(priddenToken, vRider)) in the planRiderTokens call. I will include that change in the next release, thank you for bringing this to my attention.

I am not quite sure what the purpose of the changes in MountingScript.js is, especially if (game.user.isGM) {pRidingOptions.isGM = false;} seems a bit illogical.

[anupamme]

Makes sense, thanks! Checking inside UpdateRidderTokens against the known riders (via RideableFlags.isRiddenby) is better than my TokenfromID/early-return approach anyway, since it naturally handles the "empty list = update all current riders" case instead of me having to special-case it. Glad to hear you'll fold that into the next release.