feat(world-builder): stage 2 map objects and structures API (#90) - #106
feat(world-builder): stage 2 map objects and structures API (#90)#106Rodrigoue9 wants to merge 2 commits into
Conversation
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| state: MapObjectState = "placed" | ||
| ): Promise<MapObjectRecord> { | ||
| validateCoordinates(x, y); | ||
| if (objIndex <= 0) { | ||
| throw new Error("objIndex debe ser mayor a 0"); | ||
| } | ||
|
|
||
| const res = await pool.query( | ||
| `INSERT INTO game_map_objects (map_id, x, y, obj_index, amount, state, created_by) | ||
| VALUES ($1, $2, $3, $4, $5, $6, $7) | ||
| RETURNING id, map_id AS "mapId", x, y, obj_index AS "objIndex", amount, state, created_by AS "createdBy", created_at AS "createdAt"`, | ||
| [mapId, x, y, objIndex, amount, state, createdBy] | ||
| ); |
There was a problem hiding this comment.
⚠️ Bug: state is not validated against allowed MapObjectState values
placeObject, setObjectState, and placeStructure write the client-supplied state directly to the DB. MapObjectState is a compile-time-only TypeScript union, so any arbitrary string from the request body is accepted and persisted, corrupting object state semantics (door_open/door_closed/etc.). Validate state against an explicit runtime allow-list (or add a CHECK constraint / enum on the column) and reject unknown values.
Was this helpful? React with 👍 / 👎
| app.post("/internal/map-objects", requireAuth, async (request, response) => { | ||
| try { | ||
| const { mapId, x, y, objIndex, amount, createdBy, state } = request.body; | ||
| const { placeObject } = await import("./repositories/mapObjects"); | ||
| const result = await placeObject(mapId, x, y, objIndex, amount, createdBy, state); | ||
| response.status(201).json(result); | ||
| } catch (error) { | ||
| response.status(400).json({ | ||
| error: error instanceof Error ? error.message : "Unexpected error", | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
⚠️ Security: Editor write endpoints use static-token requireAuth, not admin auth
The new /internal/map-objects write endpoints (place/move/delete/state/structure) mutate map/world data but are guarded only by requireAuth, which just compares the Authorization header to a single static config.tokenAuth. Comparable game-data/map-editing mutations in this server use requireAdminEmailSession + isAuthorizedGameDataAdmin. If these endpoints are reachable by regular clients (not only trusted server-to-server callers), anyone holding the shared token can place/move/delete world objects. Confirm the intended caller; if end users can reach these routes, switch to admin session authorization.
Was this helpful? React with 👍 / 👎
| y INTEGER NOT NULL CHECK (y BETWEEN 1 AND 100), | ||
| obj_index INTEGER NOT NULL CHECK (obj_index > 0), | ||
| amount INTEGER NOT NULL DEFAULT 1 CHECK (amount > 0), | ||
| state VARCHAR(50) NOT NULL DEFAULT 'default' CHECK (state IN ('default', 'door_open', 'door_closed', 'locked', 'destroyed')), |
There was a problem hiding this comment.
🚨 Bug: state CHECK constraint disagrees with allowed app states
The DB CHECK on game_map_objects.state permits ('default','door_open','door_closed','locked','destroyed'), but the application's ALLOWED_MAP_OBJECT_STATES / MapObjectState are ('placed','structure','door_open','door_closed','sign'). validateState() will accept 'placed', 'structure', and 'sign' (including the defaults used by placeObject → 'placed' and placeStructure → 'structure'), so the INSERT will violate the CHECK constraint and throw at the DB layer — placeObject and placeStructure fail for every default call. Align the two lists: update the schema CHECK to IN ('placed','structure','door_open','door_closed','sign') and set the column DEFAULT to 'placed' (or 'structure').
Match the schema CHECK/DEFAULT to the application's MapObjectState values.:
state VARCHAR(50) NOT NULL DEFAULT 'placed' CHECK (state IN ('placed', 'structure', 'door_open', 'door_closed', 'sign')),
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
Code Review 🚫 Blocked 2 resolved / 5 findingsImplements stage 2 map objects and structures API with grid validation and lifecycle endpoints, but the state column allows unvalidated values, the database CHECK constraint disagrees with application states, and write endpoints rely on a weak static-token auth guard instead of admin permissions. 🚨 Bug: state CHECK constraint disagrees with allowed app states📄 api/schema.sql:638 📄 api/src/repositories/mapObjects.ts:27-33 📄 api/src/repositories/mapObjects.ts:54 📄 api/src/repositories/mapObjects.ts:66 📄 api/src/repositories/mapObjects.ts:143-144 The DB CHECK on game_map_objects.state permits ('default','door_open','door_closed','locked','destroyed'), but the application's ALLOWED_MAP_OBJECT_STATES / MapObjectState are ('placed','structure','door_open','door_closed','sign'). validateState() will accept 'placed', 'structure', and 'sign' (including the defaults used by placeObject → 'placed' and placeStructure → 'structure'), so the INSERT will violate the CHECK constraint and throw at the DB layer — placeObject and placeStructure fail for every default call. Align the two lists: update the schema CHECK to IN ('placed','structure','door_open','door_closed','sign') and set the column DEFAULT to 'placed' (or 'structure'). Match the schema CHECK/DEFAULT to the application's MapObjectState values.
|
| Auto-apply | Compact |
|
|
Was this helpful? React with 👍 / 👎 | Gitar
1 participant
Title
feat(world-builder): stage 2 map objects, doors and structures API (#90)
Description
api/src/repositories/mapObjects.tswithplaceObject,moveObject,removeObject,getMapObjects,setObjectState, and atomicplaceStructure./internal/map-objectsfor map editor object lifecycle management.objIndex.Closes #90