Skip to content

Commit f5ec156

Browse files
CoderCococlaude
andauthored
feat(discord): add Terraform-managed base allowlist and admins (#34)
## Summary Introduces a Terraform-managed baseline for Discord allowlists and admin lists that cannot be removed via the management UI. This allows operators to enforce a permanent floor of allowed guilds and admins while still permitting the UI to manage additional entries. ## Key Changes - **New `BaseDiscordConfig` type** in shared types representing the read-only Terraform baseline (allowedGuilds and admins) - **New DynamoDB row `BASE#discord`** written by Terraform on every apply when any base list is non-empty, with three new Terraform variables: - `base_allowed_guilds`: Guild IDs permanently allowlisted - `base_admin_user_ids`: User IDs with permanent admin privileges - `base_admin_role_ids`: Role IDs with permanent admin privileges - **New `getBaseDiscordConfig()` function** reads the BASE#discord row with strongly consistent reads, returning an empty base when absent - **New `getEffectiveDiscordConfig()` function** merges the Terraform base and dynamic CONFIG#discord rows, deduplicating entries. This is now used by both Lambda handlers for permission checks - **Updated `DiscordConfigService`** to: - Cache and load the base config separately with coalesced concurrent reads - Expose `getBaseConfig()` method for retrieving the base config - Prevent removal of guilds/admins that exist in the Terraform base (returns error with explanation) - Include base lists in the redacted config sent to the web client - **Updated REST API endpoints** to return both dynamic and base lists: - `GET /discord/guilds` and `GET /discord/admins` now include `baseGuilds` and `baseAdmins` - `DELETE /discord/guilds/:guildId` returns 400 if the guild is in the base config - All mutation endpoints return the updated base lists alongside dynamic lists - **Updated Lambda handlers** (interactions and followup) to use `getEffectiveDiscordConfig()` instead of `getDiscordConfig()` for permission checks, ensuring base entries are always enforced ## Implementation Details - Base config is cached in `DiscordConfigService` and invalidated alongside the dynamic config - Terraform resource uses `count` to skip creation when all base lists are empty (UI-only deployments) - Defensive parsing sanitizes malformed base data without throwing - Web client can render base entries as locked/non-removable based on the new `baseAllowedGuilds` and `baseAdmins` fields in the redacted config https://claude.ai/code/session_01GxkgH9sABMbHDCQqXehHP1 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 821249b commit f5ec156

16 files changed

Lines changed: 427 additions & 38 deletions

File tree

CLAUDE.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,15 @@ The full deploy IAM policy (`GameServerDeployAll`) lives in **`docs/setup.md`**
133133

134134
All resources inherit `default_tags` from `provider "aws"` (`Project = "game-servers-poc"`, `Environment = "poc"`, `ManagedBy = "terraform"`). For Cost Explorer breakdowns, the `Project` cost allocation tag must be activated manually in AWS Billing — this is a one-time console action, not Terraform-managed.
135135

136+
## Checklist for Terraform variable changes
137+
138+
Any time you add or remove Terraform variables, update **all four** of these in the same commit — failing to touch any one of them is a common oversight:
139+
140+
1. `terraform/variables.tf` — the variable declaration itself.
141+
2. `terraform/terraform.tfvars.example` — a commented-out example entry with a short explanation so operators know how to use it.
142+
3. `docs/docs/components/terraform.md` — the Variables table row.
143+
4. `docs/docs/setup.md` — any relevant step in the setup guide (especially if the variable affects the Discord bot or a core workflow).
144+
136145
## Code & Test Conventions
137146

138147
- **Test names**: every `it(...)` case must read as a natural-language sentence starting with "should" — e.g. `it('should return null when state file is missing')`, not `it('returns null...')`.

app/packages/lambda/followup/src/handler.test.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,13 @@ import {
1616
EC2Client,
1717
} from '@aws-sdk/client-ec2';
1818

19-
const getDiscordConfigMock = vi.fn();
19+
const getEffectiveDiscordConfigMock = vi.fn();
2020
const putPendingMock = vi.fn();
2121
vi.mock('@gsd/shared', async () => {
2222
const actual = await vi.importActual<typeof import('@gsd/shared')>('@gsd/shared');
2323
return {
2424
...actual,
25-
getDiscordConfig: (...args: unknown[]) => getDiscordConfigMock(...args),
25+
getEffectiveDiscordConfig: (...args: unknown[]) => getEffectiveDiscordConfigMock(...args),
2626
putPending: (...args: unknown[]) => putPendingMock(...args),
2727
};
2828
});
@@ -69,9 +69,9 @@ beforeEach(() => {
6969
ecsMock.reset();
7070
ec2Mock.reset();
7171
fetchMock.mockReset();
72-
getDiscordConfigMock.mockReset();
72+
getEffectiveDiscordConfigMock.mockReset();
7373
putPendingMock.mockReset();
74-
getDiscordConfigMock.mockResolvedValue(PERMISSIVE_CONFIG);
74+
getEffectiveDiscordConfigMock.mockResolvedValue(PERMISSIVE_CONFIG);
7575
fetchMock.mockResolvedValue({ ok: true, text: async () => '' });
7676
});
7777

@@ -116,7 +116,7 @@ describe('FollowupLambda: start', () => {
116116
});
117117

118118
it('should deny the start when canRun() rejects the caller', async () => {
119-
getDiscordConfigMock.mockResolvedValue({
119+
getEffectiveDiscordConfigMock.mockResolvedValue({
120120
...PERMISSIVE_CONFIG,
121121
admins: { userIds: [], roleIds: [] },
122122
gamePermissions: { palworld: { userIds: [], roleIds: [], actions: [] } },
@@ -193,7 +193,7 @@ describe('FollowupLambda: status', () => {
193193

194194
describe('FollowupLambda: list', () => {
195195
it('should only fetch status for games the caller can view, and join lines with newlines', async () => {
196-
getDiscordConfigMock.mockResolvedValue({
196+
getEffectiveDiscordConfigMock.mockResolvedValue({
197197
...PERMISSIVE_CONFIG,
198198
admins: { userIds: [], roleIds: [] },
199199
gamePermissions: {
@@ -211,7 +211,7 @@ describe('FollowupLambda: list', () => {
211211
});
212212

213213
it('should return a helpful message when the caller can see nothing', async () => {
214-
getDiscordConfigMock.mockResolvedValue({
214+
getEffectiveDiscordConfigMock.mockResolvedValue({
215215
...PERMISSIVE_CONFIG,
216216
admins: { userIds: [], roleIds: [] },
217217
gamePermissions: {},

app/packages/lambda/followup/src/handler.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import {
2424
EC2Client,
2525
DescribeNetworkInterfacesCommand,
2626
} from '@aws-sdk/client-ec2';
27-
import { canRun, formatGameStatus, getDiscordConfig, putPending } from '@gsd/shared';
27+
import { canRun, formatGameStatus, getEffectiveDiscordConfig, putPending } from '@gsd/shared';
2828
import type { DiscordAction, DiscordConfig, GameStatus } from '@gsd/shared';
2929

3030
interface FollowupEvent {
@@ -252,7 +252,7 @@ function recheck(event: FollowupEvent, cfg: DiscordConfig, action: DiscordAction
252252
*/
253253
export const handler = async (event: FollowupEvent): Promise<void> => {
254254
const tableName = requireEnv('TABLE_NAME');
255-
const cfg = await getDiscordConfig(tableName);
255+
const cfg = await getEffectiveDiscordConfig(tableName);
256256

257257
let content: string;
258258
try {

app/packages/lambda/interactions/src/handler.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,13 @@ vi.mock('@noble/ed25519', () => ({
2020

2121
// Mock the shared config + secrets stores so we never hit AWS.
2222
const getPublicKeyMock = vi.fn();
23-
const getDiscordConfigMock = vi.fn();
23+
const getEffectiveDiscordConfigMock = vi.fn();
2424
vi.mock('@gsd/shared', async () => {
2525
const actual = await vi.importActual<typeof import('@gsd/shared')>('@gsd/shared');
2626
return {
2727
...actual,
2828
getPublicKey: (...args: unknown[]) => getPublicKeyMock(...args),
29-
getDiscordConfig: (...args: unknown[]) => getDiscordConfigMock(...args),
29+
getEffectiveDiscordConfig: (...args: unknown[]) => getEffectiveDiscordConfigMock(...args),
3030
};
3131
});
3232

@@ -69,9 +69,9 @@ beforeEach(() => {
6969
lambdaMock.reset();
7070
verifyAsyncMock.mockReset();
7171
getPublicKeyMock.mockReset();
72-
getDiscordConfigMock.mockReset();
72+
getEffectiveDiscordConfigMock.mockReset();
7373
getPublicKeyMock.mockResolvedValue('0a'.repeat(32));
74-
getDiscordConfigMock.mockResolvedValue({
74+
getEffectiveDiscordConfigMock.mockResolvedValue({
7575
clientId: 'app-id',
7676
allowedGuilds: ['G1'],
7777
admins: { userIds: ['ADMIN'], roleIds: [] },
@@ -261,7 +261,7 @@ describe('InteractionsLambda: APPLICATION_COMMAND_AUTOCOMPLETE', () => {
261261
it('should cap autocomplete choices at 25 to satisfy the Discord limit', async () => {
262262
const manyGames = Array.from({ length: 50 }, (_, i) => `game${i}`);
263263
process.env['GAME_NAMES'] = manyGames.join(',');
264-
getDiscordConfigMock.mockResolvedValueOnce({
264+
getEffectiveDiscordConfigMock.mockResolvedValueOnce({
265265
clientId: 'app-id',
266266
allowedGuilds: ['G1'],
267267
admins: { userIds: ['U1'], roleIds: [] },

app/packages/lambda/interactions/src/handler.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
import { verifyAsync } from '@noble/ed25519';
1818
import { LambdaClient, InvokeCommand } from '@aws-sdk/client-lambda';
1919
import type { APIGatewayProxyEventV2, APIGatewayProxyResultV2 } from 'aws-lambda';
20-
import { canRun, getDiscordConfig, getPublicKey } from '@gsd/shared';
20+
import { canRun, getEffectiveDiscordConfig, getPublicKey } from '@gsd/shared';
2121
import type { DiscordAction, DiscordConfig } from '@gsd/shared';
2222

2323
/** Discord interaction types we care about. Full list in discord-api-types. */
@@ -297,7 +297,7 @@ export const handler = async (event: APIGatewayProxyEventV2): Promise<APIGateway
297297
}
298298

299299
const tableName = requireEnv('TABLE_NAME');
300-
const cfg = await getDiscordConfig(tableName);
300+
const cfg = await getEffectiveDiscordConfig(tableName);
301301

302302
if (interaction.type === INTERACTION_APPLICATION_COMMAND_AUTOCOMPLETE) {
303303
return handleAutocomplete(interaction, cfg);

app/packages/server/src/controllers/discord.controller.ts

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -90,13 +90,17 @@ export class DiscordController {
9090
};
9191
}
9292

93-
/** Returns the list of allowlisted guild IDs. The interactions Lambda rejects any command from a guild not in this list. */
93+
/**
94+
* Returns the dynamic allowlisted guild IDs and the Terraform-managed base guild IDs.
95+
* The UI should render base guilds as locked (non-removable).
96+
*/
9497
@Get('guilds')
9598
async listGuilds() {
96-
return { guilds: (await this.discord.getConfig()).allowedGuilds };
99+
const [cfg, base] = await Promise.all([this.discord.getConfig(), this.discord.getBaseConfig()]);
100+
return { guilds: cfg.allowedGuilds, baseGuilds: base.allowedGuilds };
97101
}
98102

99-
/** Adds a guild ID to the allowlist persisted in DynamoDB. Takes effect on the next interaction (Lambda re-reads per invocation). */
103+
/** Adds a guild ID to the dynamic allowlist persisted in DynamoDB. Takes effect on the next interaction (Lambda re-reads per invocation). */
100104
@Post('guilds')
101105
async addGuild(@Body() body: { guildId?: unknown } = {}) {
102106
if (typeof body.guildId !== 'string') {
@@ -105,16 +109,25 @@ export class DiscordController {
105109
const guildId = body.guildId.trim();
106110
if (!guildId) throw new BadRequestException({ success: false, error: 'guildId required' });
107111
await this.discord.addAllowedGuild(guildId);
108-
return { success: true, guilds: (await this.discord.getConfig()).allowedGuilds };
112+
const [cfg, base] = await Promise.all([this.discord.getConfig(), this.discord.getBaseConfig()]);
113+
return { success: true, guilds: cfg.allowedGuilds, baseGuilds: base.allowedGuilds };
109114
}
110115

111-
/** Removes a guild ID from the allowlist. Already-registered slash commands remain in Discord until manually cleaned up. */
116+
/**
117+
* Removes a guild ID from the dynamic allowlist. Returns 400 if the guild is
118+
* in the Terraform base config — those entries require a tfvars edit + re-apply.
119+
* Already-registered slash commands remain in Discord until manually cleaned up.
120+
*/
112121
@Delete('guilds/:guildId')
113122
async removeGuild(@Param('guildId') guildIdRaw: string) {
114123
const guildId = (guildIdRaw ?? '').trim();
115124
if (!guildId) throw new BadRequestException({ success: false, error: 'guildId required' });
116-
await this.discord.removeAllowedGuild(guildId);
117-
return { success: true, guilds: (await this.discord.getConfig()).allowedGuilds };
125+
const result = await this.discord.removeAllowedGuild(guildId);
126+
if (!result.ok) {
127+
throw new BadRequestException({ success: false, error: result.reason });
128+
}
129+
const [cfg, base] = await Promise.all([this.discord.getConfig(), this.discord.getBaseConfig()]);
130+
return { success: true, guilds: cfg.allowedGuilds, baseGuilds: base.allowedGuilds };
118131
}
119132

120133
/**
@@ -130,19 +143,27 @@ export class DiscordController {
130143
return this.registrar.registerForGuild(guildId);
131144
}
132145

133-
/** Returns the global admin user/role lists. Admins bypass per-game permission gates in `canRun()`. */
146+
/**
147+
* Returns the dynamic admin user/role lists and the Terraform-managed base admin lists.
148+
* The UI should render base admins as locked (non-removable).
149+
*/
134150
@Get('admins')
135151
async getAdmins() {
136-
return (await this.discord.getConfig()).admins;
152+
const [cfg, base] = await Promise.all([this.discord.getConfig(), this.discord.getBaseConfig()]);
153+
return { ...cfg.admins, baseAdmins: base.admins };
137154
}
138155

139-
/** Replaces the admin user/role lists atomically. Omitted fields are treated as empty arrays (not "leave alone"). */
156+
/**
157+
* Replaces the dynamic admin user/role lists atomically. Omitted fields are treated as empty arrays
158+
* (not "leave alone"). Base admins set via Terraform are unaffected by this endpoint.
159+
*/
140160
@Put('admins')
141161
async putAdmins(@Body() body: { userIds?: unknown; roleIds?: unknown } = {}) {
142162
const userIds = requireStringArray('userIds', body.userIds);
143163
const roleIds = requireStringArray('roleIds', body.roleIds);
144164
await this.discord.setAdmins({ userIds, roleIds });
145-
return { success: true, admins: (await this.discord.getConfig()).admins };
165+
const [cfg, base] = await Promise.all([this.discord.getConfig(), this.discord.getBaseConfig()]);
166+
return { success: true, admins: cfg.admins, baseAdmins: base.admins };
146167
}
147168

148169
/** Returns the per-game permission map (user/role IDs allowed to run specific actions on each game). */

app/packages/server/src/services/DiscordConfigService.test.ts

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { DiscordConfigService } from './DiscordConfigService.js';
1313
import { ConfigService, type TfOutputs } from './ConfigService.js';
1414

1515
const getDiscordConfigMock = vi.fn();
16+
const getBaseDiscordConfigMock = vi.fn();
1617
const putDiscordConfigMock = vi.fn();
1718
const getBotTokenMock = vi.fn();
1819
const getPublicKeyMock = vi.fn();
@@ -25,6 +26,7 @@ vi.mock('@gsd/shared', async () => {
2526
return {
2627
...actual,
2728
getDiscordConfig: (...args: unknown[]) => getDiscordConfigMock(...args),
29+
getBaseDiscordConfig: (...args: unknown[]) => getBaseDiscordConfigMock(...args),
2830
putDiscordConfig: (...args: unknown[]) => putDiscordConfigMock(...args),
2931
getBotToken: (...args: unknown[]) => getBotTokenMock(...args),
3032
getPublicKey: (...args: unknown[]) => getPublicKeyMock(...args),
@@ -61,6 +63,7 @@ function makeService(outputs: TfOutputs | null = TF): DiscordConfigService {
6163

6264
beforeEach(() => {
6365
getDiscordConfigMock.mockReset();
66+
getBaseDiscordConfigMock.mockReset();
6467
putDiscordConfigMock.mockReset();
6568
getBotTokenMock.mockReset();
6669
getPublicKeyMock.mockReset();
@@ -73,6 +76,10 @@ beforeEach(() => {
7376
admins: { userIds: [], roleIds: [] },
7477
gamePermissions: {},
7578
});
79+
getBaseDiscordConfigMock.mockResolvedValue({
80+
allowedGuilds: [],
81+
admins: { userIds: [], roleIds: [] },
82+
});
7683
putDiscordConfigMock.mockResolvedValue(undefined);
7784
getBotTokenMock.mockResolvedValue(null);
7885
getPublicKeyMock.mockResolvedValue(null);
@@ -127,6 +134,23 @@ describe('DiscordConfigService.getRedacted', () => {
127134
expect(redacted.botTokenSet).toBe(false);
128135
expect(redacted.publicKeySet).toBe(false);
129136
});
137+
138+
it('should include base guild and admin lists from the BASE#discord row', async () => {
139+
getBaseDiscordConfigMock.mockResolvedValue({
140+
allowedGuilds: ['G-base'],
141+
admins: { userIds: ['U-base'], roleIds: ['R-base'] },
142+
});
143+
const redacted = await makeService().getRedacted();
144+
expect(redacted.baseAllowedGuilds).toEqual(['G-base']);
145+
expect(redacted.baseAdmins).toEqual({ userIds: ['U-base'], roleIds: ['R-base'] });
146+
});
147+
148+
it('should return empty base lists when no BASE#discord row exists', async () => {
149+
getBaseDiscordConfigMock.mockResolvedValue({ allowedGuilds: [], admins: { userIds: [], roleIds: [] } });
150+
const redacted = await makeService().getRedacted();
151+
expect(redacted.baseAllowedGuilds).toEqual([]);
152+
expect(redacted.baseAdmins).toEqual({ userIds: [], roleIds: [] });
153+
});
130154
});
131155

132156
describe('DiscordConfigService.setCredentials', () => {
@@ -188,21 +212,33 @@ describe('DiscordConfigService.allowedGuilds mutations', () => {
188212
);
189213
});
190214

191-
it('should remove a guild from the allowlist', async () => {
215+
it('should remove a guild from the dynamic allowlist and return ok', async () => {
192216
getDiscordConfigMock.mockResolvedValue({
193217
clientId: '',
194218
allowedGuilds: ['G1', 'G2'],
195219
admins: { userIds: [], roleIds: [] },
196220
gamePermissions: {},
197221
});
198222
const svc = makeService();
199-
await svc.removeAllowedGuild('G1');
223+
const result = await svc.removeAllowedGuild('G1');
224+
expect(result).toEqual({ ok: true });
200225
expect(putDiscordConfigMock).toHaveBeenCalledWith(
201226
'test-discord',
202227
expect.objectContaining({ allowedGuilds: ['G2'] }),
203228
);
204229
});
205230

231+
it('should refuse to remove a guild that is in the Terraform base config', async () => {
232+
getBaseDiscordConfigMock.mockResolvedValue({
233+
allowedGuilds: ['G-base'],
234+
admins: { userIds: [], roleIds: [] },
235+
});
236+
const svc = makeService();
237+
const result = await svc.removeAllowedGuild('G-base');
238+
expect(result).toMatchObject({ ok: false });
239+
expect(putDiscordConfigMock).not.toHaveBeenCalled();
240+
});
241+
206242
it('should dedupe and drop empty strings when setAllowedGuilds is called', async () => {
207243
const svc = makeService();
208244
await svc.setAllowedGuilds(['G1', '', 'G1', 'G2']);

0 commit comments

Comments
 (0)