Skip to content

feat(discord): add Terraform-managed base allowlist and admins#34

Merged
CoderCoco merged 2 commits into
mainfrom
claude/config-allow-admin-lists-IIBQw
Apr 29, 2026
Merged

feat(discord): add Terraform-managed base allowlist and admins#34
CoderCoco merged 2 commits into
mainfrom
claude/config-allow-admin-lists-IIBQw

Conversation

@CoderCoco
Copy link
Copy Markdown
Owner

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

Adds a two-row DynamoDB design for the Discord config: a Terraform-managed
BASE#discord row holds an immutable floor of guild IDs and admin user/role
IDs that the management UI can never remove. The app-managed CONFIG#discord
row continues to hold dynamic entries added via the UI.

- New Terraform vars: base_allowed_guilds, base_admin_user_ids,
  base_admin_role_ids; written to a BASE#discord DynamoDB item on every
  apply (no ignore_changes — re-apply updates the base).
- Shared: BaseDiscordConfig type; getBaseDiscordConfig() and
  getEffectiveDiscordConfig() (merged union used by canRun()).
- Lambda handlers switch from getDiscordConfig to getEffectiveDiscordConfig
  so base entries are always enforced.
- DiscordConfigService: loadBase() with its own cache, getBaseConfig(),
  removeAllowedGuild() returns {ok,reason} and rejects base guilds,
  getRedacted() includes baseAllowedGuilds and baseAdmins.
- Controller: GET /discord/guilds and GET /discord/admins now return base
  lists alongside dynamic ones; DELETE /discord/guilds/:guildId surfaces
  a 400 when the guild is Terraform-managed.

https://claude.ai/code/session_01GxkgH9sABMbHDCQqXehHP1
Copilot AI review requested due to automatic review settings April 29, 2026 04:49
…list

- terraform.tfvars.example: add commented-out base_allowed_guilds,
  base_admin_user_ids, base_admin_role_ids with usage notes.
- docs/components/terraform.md: update discord_store.tf description and
  add the three new variables to the Variables table.
- docs/setup.md: add a paragraph under step 7 explaining how to seed a
  permanent base guild/admin floor via tfvars.
- CLAUDE.md: add a "Checklist for Terraform variable changes" section so
  all four touch-points (variables.tf, tfvars.example, terraform.md,
  setup.md) are updated together in future changes.

https://claude.ai/code/session_01GxkgH9sABMbHDCQqXehHP1
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a Terraform-managed “base” Discord configuration (immutable via UI) that is merged with the app-managed dynamic Discord config, so allowlisted guilds/admins can have an operator-enforced floor while still supporting UI-managed additions.

Changes:

  • Introduces new Terraform variables and a DynamoDB BASE#discord row to persist base allowlist/admins on each terraform apply.
  • Adds shared code to read the base row and produce an “effective” merged config, and updates Lambdas to use it for permission checks.
  • Updates the management server/service/controller + shared types to surface base lists to the UI and prevent deleting base allowlisted guilds.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
terraform/variables.tf Adds Terraform variables for base allowed guilds/admin user IDs/admin role IDs.
terraform/discord_store.tf Writes the BASE#discord DynamoDB item (conditionally) from Terraform.
app/packages/shared/src/types.ts Adds BaseDiscordConfig and extends RedactedDiscordConfig with base fields.
app/packages/shared/src/ddb/configStore.ts Adds getBaseDiscordConfig() + getEffectiveDiscordConfig() to read/merge base + dynamic config.
app/packages/shared/src/ddb/configStore.test.ts Adds coverage for base config parsing and effective merge behavior.
app/packages/server/src/services/DiscordConfigService.ts Adds base config loading/caching, includes base in redacted config, blocks deleting base guilds.
app/packages/server/src/services/DiscordConfigService.test.ts Updates mocks/tests for base config inclusion + base guild deletion refusal.
app/packages/server/src/controllers/discord.controller.ts Extends guild/admin endpoints to return base lists; returns 400 on delete of base guild.
app/packages/lambda/interactions/src/handler.ts Switches permission-read path to getEffectiveDiscordConfig().
app/packages/lambda/interactions/src/handler.test.ts Updates mocks to match getEffectiveDiscordConfig() usage.
app/packages/lambda/followup/src/handler.ts Switches permission-read path to getEffectiveDiscordConfig().
app/packages/lambda/followup/src/handler.test.ts Updates mocks to match getEffectiveDiscordConfig() usage.

Comment thread terraform/variables.tf
Comment on lines +138 to +150
# ── Discord base allowlist / admins (optional) ───────────────────────────────
# These lists are Terraform-managed and written to a separate DynamoDB row
# (BASE#discord) on every `terraform apply`. They form an immutable floor that
# the management UI can never remove — operators can only add/remove entries
# they themselves added via the UI.
#
# Leave all three empty (the default) to manage everything through the UI.
# Edit the lists in terraform.tfvars and re-apply to change the base set.

variable "base_allowed_guilds" {
description = "Guild IDs permanently allowlisted by Terraform. Cannot be removed via the management UI."
type = list(string)
default = []
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

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

PR title doesn't follow the required Conventional Commits format. Per CONTRIBUTING.md, the title becomes the squash-merge commit subject; please rename it to something like "feat(terraform): add Terraform-managed Discord base config" (optionally include a scope) and keep it under ~70 chars.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed — title updated to feat(discord): add Terraform-managed base allowlist and admins.


Generated by Claude Code

Comment on lines +109 to +123
/**
* Read the Terraform-managed BASE#discord row. Empty base returned when the
* row is absent (i.e. no base Terraform variables were set). Result is cached
* until `invalidateCache()` is called, same as the dynamic config cache.
*/
private async loadBase(): Promise<BaseDiscordConfig> {
if (this.baseCache) return this.baseCache;
if (this.baseInflight) return this.baseInflight;
this.baseInflight = (async () => {
try {
const tableName = this.config.getTfOutputs()?.discord_table_name;
if (!tableName) return { allowedGuilds: [], admins: { userIds: [], roleIds: [] } };
const base = await getBaseDiscordConfig(tableName);
this.baseCache = base;
return base;
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

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

DiscordConfigService caches the BASE#discord row indefinitely (until invalidateCache), but the base config can change via terraform apply without any app-side invalidation. This can leave the UI (and removeAllowedGuild's base check) using stale base lists until a server restart; consider not caching the base row, or adding a short TTL/refresh mechanism so it eventually reflects Terraform updates.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Declined — the dynamic CONFIG#discord row has identical caching behaviour (cached until a write invalidates it) and the same stale-after-out-of-band-edit risk. The base config changes only on terraform apply, which is a deliberate infrastructure operation; an operator who just ran apply would restart the app or wait for the next natural invalidation. Adding TTL complexity for an event that happens a handful of times over the lifetime of a deployment isn't warranted.


Generated by Claude Code

@CoderCoco CoderCoco changed the title Add Terraform-managed Discord base config with UI-enforced immutability feat(discord): add Terraform-managed base allowlist and admins Apr 29, 2026
@CoderCoco CoderCoco merged commit f5ec156 into main Apr 29, 2026
7 checks passed
@CoderCoco CoderCoco deleted the claude/config-allow-admin-lists-IIBQw branch April 29, 2026 04:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants