From 321738496d59bb27f3659a96a2c1960507246e96 Mon Sep 17 00:00:00 2001
From: Bastian Wagner <bastian.wagner@softconcis.de>
Date: Fri, 21 Aug 2026 09:30:04 +0200
Subject: [PATCH] fix(reputation): validate the character exists before
 granting reputation

---
 .../src/reputation/reputation.service.spec.ts | 46 ++++++++++++++++++-
 apps/api/src/reputation/reputation.service.ts | 18 +++++++-
 2 files changed, 62 insertions(+), 2 deletions(-)

diff --git a/apps/api/src/reputation/reputation.service.spec.ts b/apps/api/src/reputation/reputation.service.spec.ts
index 3a68e2a..cbb691e 100644
--- a/apps/api/src/reputation/reputation.service.spec.ts
+++ b/apps/api/src/reputation/reputation.service.spec.ts
@@ -1,4 +1,6 @@
+import { HttpException } from '@nestjs/common';
 import { DataSource, EntityManager, EntityTarget } from 'typeorm';
+import { Character } from '../characters/entities/character.entity';
 import { CharacterReputation } from './entities/character-reputation.entity';
 import { ReputationFaction } from './entities/reputation-faction.entity';
 import { ReputationDomainError } from './reputation.errors';
@@ -8,6 +10,7 @@ const FACTION_ID = '80000000-0000-4000-8000-000000000001';
 const CHARACTER_ID = '10000000-0000-4000-8000-000000000001';
 
 interface State {
+  characters: Character[];
   factions: ReputationFaction[];
   characterReputation: CharacterReputation[];
 }
@@ -72,6 +75,8 @@ class FakeDataSource {
   }
 
   private repoFor<T extends { id: string }>(target: EntityTarget<T>, inTransaction: boolean) {
+    if (target === Character)
+      return new FakeRepository(this.state.characters, 'character', inTransaction) as never;
     if (target === ReputationFaction)
       return new FakeRepository(this.state.factions, 'faction', inTransaction) as never;
     if (target === CharacterReputation)
@@ -80,6 +85,10 @@ class FakeDataSource {
   }
 }
 
+function character(overrides: Partial<Character> = {}): Character {
+  return { id: CHARACTER_ID, ...overrides } as Character;
+}
+
 function faction(overrides: Partial<ReputationFaction> = {}): ReputationFaction {
   return {
     id: FACTION_ID,
@@ -93,7 +102,7 @@ function faction(overrides: Partial<ReputationFaction> = {}): ReputationFaction
 }
 
 function createState(overrides: Partial<State> = {}): State {
-  return { factions: [faction()], characterReputation: [], ...overrides };
+  return { characters: [character()], factions: [faction()], characterReputation: [], ...overrides };
 }
 
 function createService(state: State) {
@@ -115,6 +124,31 @@ async function expectReputationDomainError(promise: Promise<unknown>, code: stri
   expect(error.code).toBe(code);
 }
 
+/**
+ * `characterNotFound` is re-exported from `travel.errors` and returns a
+ * `TravelDomainError`, not a `ReputationDomainError` -- so this asserts on
+ * the thrown HttpException's response shape (its `code`/status) rather than
+ * its class, matching this codebase's convention for shared error helpers.
+ */
+async function expectHttpErrorWithCode(
+  promise: Promise<unknown>,
+  code: string,
+  status: number,
+): Promise<void> {
+  let error: unknown;
+  try {
+    await promise;
+  } catch (cause) {
+    error = cause;
+  }
+  expect(error).toBeInstanceOf(HttpException);
+  if (!(error instanceof HttpException)) {
+    throw new Error('Expected HttpException');
+  }
+  expect(error.getStatus()).toBe(status);
+  expect(error.getResponse()).toMatchObject({ code });
+}
+
 describe('ReputationService', () => {
   describe('grantReputation', () => {
     it('creates a reputation row starting from 0 on the first grant', async () => {
@@ -184,6 +218,16 @@ describe('ReputationService', () => {
         'REPUTATION_FACTION_NOT_FOUND',
       );
     });
+
+    it('rejects an unknown character', async () => {
+      const { service } = createService(createState());
+
+      await expectHttpErrorWithCode(
+        service.grantReputation('unknown-character', 'border-guard', 10),
+        'CHARACTER_NOT_FOUND',
+        404,
+      );
+    });
   });
 
   describe('getCharacterReputation', () => {
diff --git a/apps/api/src/reputation/reputation.service.ts b/apps/api/src/reputation/reputation.service.ts
index 6e76c5f..0edbc58 100644
--- a/apps/api/src/reputation/reputation.service.ts
+++ b/apps/api/src/reputation/reputation.service.ts
@@ -1,8 +1,9 @@
 import { Injectable } from '@nestjs/common';
 import { DataSource, EntityManager } from 'typeorm';
+import { Character } from '../characters/entities/character.entity';
 import { CharacterReputation } from './entities/character-reputation.entity';
 import { ReputationFaction } from './entities/reputation-faction.entity';
-import { reputationFactionNotFound } from './reputation.errors';
+import { characterNotFound, reputationFactionNotFound } from './reputation.errors';
 import { resolveReputationRank } from './reputation-rank';
 
 export interface ReputationGrantResult {
@@ -37,9 +38,24 @@ export class ReputationService {
     manager?: EntityManager,
   ): Promise<ReputationGrantResult> {
     const run = async (txManager: EntityManager): Promise<ReputationGrantResult> => {
+      const characters = txManager.getRepository(Character);
       const factions = txManager.getRepository(ReputationFaction);
       const reputations = txManager.getRepository(CharacterReputation);
 
+      // When TurnInService (Task 6) passes its own `manager`, it will already
+      // hold a pessimistic write lock on this same character row from earlier
+      // in that transaction. Re-locking it here is a no-op re-lock in
+      // Postgres, not a deadlock risk -- CombatService.performAction relies on
+      // the same behavior for grantVictoryRewards. Do not remove this check
+      // to "avoid" the re-lock.
+      const character = await characters.findOne({
+        where: { id: characterId },
+        lock: { mode: 'pessimistic_write' },
+      });
+      if (!character) {
+        throw characterNotFound();
+      }
+
       const faction = await factions.findOneBy({ key: factionKey });
       if (!faction) {
         throw reputationFactionNotFound();