decentraland / decentraland/worlds-content-server
fix: store permission scope explicitly instead of inferring it from an empty parcel list
- Dominant language
- TypeScript
- Stars
- 0
- Forks
- 7
- Avg merge
- 1d 9h
- Merged PRs (30d)
- 5
Description
## Problem
Parcel-scoped permissions encode their scope as an absence rather than as a stored property: a
permission with no parcels recorded against it is treated as having no parcel restrictions. The two
states are indistinguishable in the database, so the API has no way to represent "restricted to no
parcels" — a permission that exists is either unrestricted or restricted to a non-empty set.
The consequence is that `DELETE /world/:world_name/permissions/:permission_name/address/:address/parcels`
can widen the permission it is meant to narrow. Removing the last parcel from an address's permission
does not reduce that address to no access; it returns the permission to its unrestricted state, which
passes every scope check in the service — scene deployment, undeploy, and world settings updates. The
request returns `204` and emits no event, so the owner gets no signal that anything was widened.
This is reachable through ordinary owner actions, not only by deliberately emptying a list. There is
no atomic way to move a collaborator from one parcel set to another (see #443), so a remove-then-add
sequence passes through the unrestricted state, and stays there if the add is deferred or fails. The
same applies to an owner narrowing a collaborator's scope over time until it reaches zero, or
emptying the list intending to withdraw access — withdrawing access actually requires the separate
`DELETE /world/:world_name/permissions/:permission_name/:address`, which removes the record.
## Expected Behaviour
Removing parcels from a permission must never leave the address with more access than it had. Which
direction we take is open:
1. **Revoke on empty** — if a removal would empty the parcel set, delete the permission record and
emit the permission-revoked event, mirroring the existing revoke route. Probably closest to what
an owner emptying a grant intends.
2. **Reject on empty** — return `400` and point the caller at the revoke route, so that neither
outcome is a silent state change.
3. **Store the scope explicitly** — persist the scope on the permission record (a migration adding a
boolean, backfilled from the current parcel counts) so that "no parcels recorded" can no longer
mean "no restrictions". This removes the same inference from the permission reads and the address
queries that share it, and composes with either 1 or 2.
Whichever we choose, the integration coverage for the parcels routes currently asserts the present
behaviour and will need updating, and the OpenAPI description of the parcels routes should state what
an empty parcel set means.
## Related
Found while reviewing, same root cause or same blind spot — worth handling together:
- The parcel-adding path will also create a permission record with no parcels when handed an empty
list, producing the same unrestricted state. Request validation prevents this over HTTP; internal
callers are unguarded.
- No event is emitted when the parcel scope of an *existing* permission changes. The granted event
fires only when the permission record is newly created, and the removal path never notifies at all,
so changes to a collaborator's footprint are unobservable downstream.
- Coordinate normalization is inconsistent between the parcel write paths and the address-query read
path, so non-canonical coordinates can fail to match stored parcels on that query.
Internal notes with the full code-path detail are available on request.
Contributor guide
Assessment
This issue has not been assessed yet.