Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions api/.env.example
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,8 @@ PORT=3001
DATABASE_URL=postgresql://postgres:postgres@localhost:5432/aoweb
TOKEN_AUTH=changeme
CORS_ORIGIN=http://localhost:3000

# Game Data Administration & Map Builder
GAME_DATA_ADMIN_EMAIL=admin@aoweb.app
GAME_DATA_ADMIN_ACCOUNT_ID=
GAME_DATA_ADMIN_PROXY_TOKEN=
40 changes: 40 additions & 0 deletions api/src/repositories/__tests__/paletteValidation.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { describe, it, expect } from "vitest";
import {
paletteEntrySchema,
UPLOADED_GRAPHIC_INDEX_START,
} from "../worldBuilder";

describe("Palette Entry Schema and Validation (#6)", () => {
it("should accept valid multi-layer palette entries with blocking flag", () => {
const valid = paletteEntrySchema.safeParse({
graphics: [5500, 581],
blocked: true,
});
expect(valid.success).toBe(true);
if (valid.success) {
expect(valid.data.graphics).toEqual([5500, 581]);
expect(valid.data.blocked).toBe(true);
}
});

it("should reject palette entries with empty graphics array", () => {
const invalid = paletteEntrySchema.safeParse({
graphics: [],
blocked: false,
});
expect(invalid.success).toBe(false);
});

it("should reject palette entries exceeding maximum layers (4)", () => {
const invalid = paletteEntrySchema.safeParse({
graphics: [1, 2, 3, 4, 5],
});
expect(invalid.success).toBe(false);
});

it("should enforce non-colliding reserved range for uploaded graphics", () => {
expect(UPLOADED_GRAPHIC_INDEX_START).toBe(1_000_000);
// Original game graphics reach up to 320151, well below 1_000_000
expect(UPLOADED_GRAPHIC_INDEX_START).toBeGreaterThan(320151);
});
});
42 changes: 42 additions & 0 deletions api/src/repositories/__tests__/worldBuilderPermissions.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { describe, it, expect } from "vitest";
import {
PROTECTED_MAPS,
isMapProtected,
canAccountEditMap,
} from "../worldBuilder";

describe("World Builder Map Permissions and Protections (#4)", () => {
it("should identify city maps as protected by default", () => {
expect(isMapProtected(1)).toBe(true); // Ullathorpe
expect(isMapProtected(34)).toBe(true); // Nix
expect(isMapProtected(59)).toBe(true); // Banderbill
expect(isMapProtected(50)).toBe(false); // Regular map
});

it("should reject edits to protected maps when override is false", () => {
const result = canAccountEditMap("admin_123", 1, true, undefined, false);
expect(result.allowed).toBe(false);
expect(result.reason).toContain("protegido contra modificaciones");
});

it("should allow edits to protected maps when override is true", () => {
const result = canAccountEditMap("admin_123", 1, true, undefined, true);
expect(result.allowed).toBe(true);
});

it("should allow superadmin to edit non-protected maps", () => {
const result = canAccountEditMap("admin_123", 50, true, undefined, false);
expect(result.allowed).toBe(true);
});

it("should allow collaborator to edit specifically assigned map", () => {
const result = canAccountEditMap("collab_456", 50, false, [50, 51], false);
expect(result.allowed).toBe(true);
});

it("should reject collaborator attempting to edit unassigned map", () => {
const result = canAccountEditMap("collab_456", 100, false, [50, 51], false);
expect(result.allowed).toBe(false);
expect(result.reason).toContain("no tiene permisos");
});
});
73 changes: 73 additions & 0 deletions api/src/repositories/worldBuilder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,44 @@ export const UPLOADED_GRAPHIC_INDEX_START = 1_000_000;
/** Los mapas del juego son de 100x100. */
export const MAP_SIZE = 100;

/**
* Mapas protegidos contra modificaciones destructivas (ciudades principales).
* Requieren autorizacion explicita con override para ser modificados.
*/
export const PROTECTED_MAPS = new Set<number>([1, 34, 59, 60, 61]);

export function isMapProtected(mapNum: number): boolean {
return PROTECTED_MAPS.has(mapNum);
}

export function canAccountEditMap(
accountId: string,
mapNum: number,
isSuperAdmin: boolean,
allowedMapsForAccount?: number[],
allowProtectedOverride = false,
): { allowed: boolean; reason?: string } {
if (isMapProtected(mapNum) && !allowProtectedOverride) {
return {
allowed: false,
reason: `El mapa ${mapNum} esta protegido contra modificaciones.`,
};
}

if (isSuperAdmin) {
return { allowed: true };
}

if (allowedMapsForAccount && allowedMapsForAccount.includes(mapNum)) {
return { allowed: true };
}

return {
allowed: false,
reason: `La cuenta ${accountId} no tiene permisos para editar el mapa ${mapNum}.`,
};
}

export type UploadedGraphic = {
grhIndex: number;
checksum: string;
Expand Down Expand Up @@ -180,6 +218,41 @@ export async function listGraphics(limit = 100): Promise<UploadedGraphic[]> {
}));
}

export const paletteEntrySchema = z.object({
graphics: z.array(z.number().int().positive()).min(1).max(4),
blocked: z.boolean().optional(),
});

export type PaletteEntry = z.infer<typeof paletteEntrySchema>;

/**
* Valida que los graficos de una entrada de paleta existan (originales o subidos).
*/
export async function validatePaletteEntry(
entry: PaletteEntry,
): Promise<{ valid: boolean; reason?: string }> {
for (const grhIndex of entry.graphics) {
if (grhIndex >= UPLOADED_GRAPHIC_INDEX_START) {
const exists = await pool.query(
`SELECT 1 FROM game_uploaded_graphics WHERE grh_index = $1 LIMIT 1`,
[grhIndex],
);
if (exists.rowCount === 0) {
return {
valid: false,
reason: `El grafico ${grhIndex} no existe en el motor ni en assets subidos.`,
};
}
Comment on lines +231 to +245

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Quality: validatePaletteEntry is never called and untested

The PR's stated goal is to validate multi-layer palette definitions, but validatePaletteEntry is not imported or invoked by any route or caller in the codebase (only its definition exists), and the new test file only exercises paletteEntrySchema, never validatePaletteEntry. As a result no palette entry is actually validated against existing/uploaded graphics at runtime and the collision/existence logic has zero test coverage. Wire the function into the palette-writing route(s) and add a unit test covering the uploaded-index existence branch (mocking pool.query).

Was this helpful? React with 👍 / 👎

} else if (grhIndex <= 0) {
return {
valid: false,
Comment on lines +234 to +248

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Bug: Engine graphic indices are accepted without existence check

The function's docstring claims it verifies graphics exist "originales o subidos", but for any positive index below UPLOADED_GRAPHIC_INDEX_START (1,000,000) it performs no validation at all — including indices between the real engine max (320151) and 1,000,000, which are non-existent yet accepted as valid. This lets invalid engine indices pass validation. Either validate engine indices against the known upper bound (e.g. reject grhIndex > MAX_ENGINE_GRAPHIC_INDEX) or correct the docstring to reflect that only uploaded indices are checked.

Reject engine indices outside the valid original range instead of accepting them blindly.:

} else if (grhIndex <= 0 || grhIndex > 320151) {
    return {
        valid: false,
        reason: `Indice de grafico invalido: ${grhIndex}.`,
    };
}
  • Apply fix

Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎

reason: `Indice de grafico invalido: ${grhIndex}.`,
};
}
}
return { valid: true };
}

export const tilePaintSchema = z.object({
x: z.coerce.number().int().min(1).max(MAP_SIZE),
y: z.coerce.number().int().min(1).max(MAP_SIZE),
Expand Down
72 changes: 72 additions & 0 deletions api/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,12 @@ import {
upsertGameBalance,
} from "./repositories/gameBalance";
import {
canAccountEditMap,
clearTile,
discardDrafts,
getGraphicContent,
getMapStatus,
isMapProtected,
listGraphics,
listMapOverrides,
paintTiles,
Expand Down Expand Up @@ -857,6 +859,20 @@ app.put("/admin/game-data/maps/:mapNum/tiles", async (request, response) => {
return;
}

const allowOverride = request.headers["x-protected-map-override"] === "true";
const permission = canAccountEditMap(
authorized.session.account._id,
mapNum,
true,
undefined,
allowOverride,
);

if (!permission.allowed) {
response.status(403).json({ error: permission.reason });
return;
}

const parsed = paintTilesSchema.safeParse(request.body);

if (!parsed.success) {
Expand Down Expand Up @@ -900,6 +916,20 @@ app.delete(
return;
}

const allowOverride = request.headers["x-protected-map-override"] === "true";
const permission = canAccountEditMap(
authorized.session.account._id,
mapNum,
true,
undefined,
allowOverride,
);

if (!permission.allowed) {
response.status(403).json({ error: permission.reason });
return;
}

response.json({ removed: await clearTile(mapNum, x, y, layer) });
} catch (error) {
const message =
Expand Down Expand Up @@ -963,6 +993,20 @@ app.post("/admin/game-data/maps/:mapNum/publish", async (request, response) => {
return;
}

const allowOverride = request.headers["x-protected-map-override"] === "true";
const permission = canAccountEditMap(
authorized.session.account._id,
mapNum,
true,
undefined,
allowOverride,
);

if (!permission.allowed) {
response.status(403).json({ error: permission.reason });
return;
}

response.json(
await publishMap(mapNum, authorized.session.account._id),
);
Expand All @@ -986,6 +1030,20 @@ app.post("/admin/game-data/maps/:mapNum/discard", async (request, response) => {
return;
}

const allowOverride = request.headers["x-protected-map-override"] === "true";
const permission = canAccountEditMap(
authorized.session.account._id,
mapNum,
true,
undefined,
allowOverride,
);

if (!permission.allowed) {
response.status(403).json({ error: permission.reason });
return;
}

response.json(await discardDrafts(mapNum));
} catch (error) {
const message =
Expand All @@ -1010,6 +1068,20 @@ app.post("/admin/game-data/maps/:mapNum/revert", async (request, response) => {
return;
}

const allowOverride = request.headers["x-protected-map-override"] === "true";
const permission = canAccountEditMap(
authorized.session.account._id,
mapNum,
true,
undefined,
allowOverride,
);

if (!permission.allowed) {
response.status(403).json({ error: permission.reason });
return;
}

response.json(await revertMap(mapNum));
} catch (error) {
const message =
Expand Down
46 changes: 46 additions & 0 deletions api/src/tests/shutdownReset.unit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import assert from "node:assert/strict";
import { test, vi } from "vitest";

const query = vi.hoisted(() => vi.fn());

vi.mock("../db", () => ({
default: { query },
}));

import { resetAllCharactersConnectedStatus } from "../repositories/characters";
import { resetAllArenaRoomMembersConnectedStatus } from "../repositories/arenas";

test("reset helpers clear connected characters and arena members", async () => {
query
.mockReset()
.mockResolvedValueOnce({ rowCount: 4 })
.mockResolvedValueOnce({ rowCount: 2 });

const [updatedCharacters, updatedArenaMembers] = await Promise.all([
resetAllCharactersConnectedStatus(),
resetAllArenaRoomMembersConnectedStatus(),
]);

assert.equal(updatedCharacters, 4);
assert.equal(updatedArenaMembers, 2);
assert.equal(query.mock.calls.length, 2);

const sqlStatements = query.mock.calls.map(([sql]) => String(sql));
assert.equal(
sqlStatements.some(
(sql) =>
sql.includes("UPDATE characters") &&
sql.includes("connected = FALSE") &&
sql.includes("deleted_at IS NULL"),
),
true,
);
assert.equal(
sqlStatements.some(
(sql) =>
sql.includes("UPDATE arena_room_members") &&
sql.includes("connected = FALSE"),
),
true,
);
});
Loading