Skip to content

feat(spaces): replace sequential space ID with UUID#2983

Open
vseehausen wants to merge 4 commits intomainfrom
valentin/wa-1583-replace-sequential-spaceid-with-global-uuid
Open

feat(spaces): replace sequential space ID with UUID#2983
vseehausen wants to merge 4 commits intomainfrom
valentin/wa-1583-replace-sequential-spaceid-with-global-uuid

Conversation

@vseehausen
Copy link
Copy Markdown
Member

WA-1583

Why

Sequential auto-incrementing space IDs leak the total number of spaces and allow enumeration attacks. We replace them with server-generated UUID v4.

We evaluated external ID libraries (nanoid, cuid2, ksuid) but chose native PG gen_random_uuid() — zero dependencies, built-in validation via the existing UuidSchema, and 16-byte binary storage with native indexing. No app-side generation needed.

What changes

  • Migration: add-backfill-swap from SERIAL intuuid DEFAULT gen_random_uuid() on spaces.id and all FK references (members, space_safes, space_address_book_items)
  • Entity & schema: SpaceSchema overrides RowSchema.id with UuidSchema; DB entity uses @PrimaryGeneratedColumn('uuid')
  • Controllers: ParseIntPipe + RowSchema.shape.idUuidSchema for all spaceId params; userId stays as int
  • DTOs: ApiProperty({ type: Number })String for space ID fields
  • Tests: builders and specs updated to use faker.string.uuid()

Scope

Only spaces.id and its FK references change. Child table own PKs, users, and wallets stay as SERIAL int. RowSchema is untouched.

Test plan

  • yarn lint-check passes
  • yarn build compiles
  • yarn test:unit passes
  • yarn test:integration passes (requires test DB)
  • yarn test:e2e passes (requires Redis + RabbitMQ)
  • yarn migration:revert restores integer IDs

🤖 Generated with Claude Code

@vseehausen vseehausen requested a review from a team as a code owner March 18, 2026 16:04
@vseehausen vseehausen force-pushed the valentin/wa-1583-replace-sequential-spaceid-with-global-uuid branch 4 times, most recently from 4509421 to 2172096 Compare March 18, 2026 22:43
@katspaugh
Copy link
Copy Markdown
Member

@vseehausen what happens to existing Spaces, are we going to migrate them?

@katspaugh
Copy link
Copy Markdown
Member

@claude review

@claude
Copy link
Copy Markdown

claude bot commented Mar 19, 2026

Claude finished @katspaugh's task —— View job


  • Read migration file
  • Read entity and schema changes
  • Read controller changes
  • Read test changes
  • Post review

Review

Overall this is a well-structured migration. A few things to address:

Migration (critical)

  1. Single-statement migration is not transactional (migrations/1773836092000-space-id-to-uuid.ts): The entire up() runs as one queryRunner.query() call with ~15 DDL statements. If any statement fails partway through, PG will roll back the implicit transaction, but TypeORM may not mark the migration as failed. Consider wrapping in an explicit BEGIN/COMMIT or splitting into separate queryRunner.query() calls so that TypeORM's migration runner can handle rollback properly. Alternatively, confirm that TypeORM wraps each migration in a transaction by default for your setup.

  2. Constraint name casing change (migrations/1773836092000-space-id-to-uuid.ts:32,72): The original FK_SABI_space_id was created unquoted (folded to fk_sabi_space_id), and the migration correctly drops the lowercase version. However, it recreates it as "FK_SABI_space_id" (now case-preserved due to quoting). The same happens for UQ_SABI_space_id_address"UQ_SABI_space_id_address", uq_ss_chainid_address_space"UQ_SS_chainId_address_space", and idx_sabi_space_id"IDX_SABI_space_id". This is intentional normalization to match entity definitions, but worth calling out — the revert migration must use the new quoted names (which it does correctly).

Code

  1. ParseIntPipe still imported in members.controller.ts:21 — it's still used for userId params, so this is fine. Just noting it's intentional.

  2. UUID string comparison (spaces.service.ts:119): spaces.find((space) => space.id === id) compares UUIDs as strings. This works if both sides are always the same case. Since PG returns lowercase UUIDs and UuidSchema (zod) typically lowercases, this should be fine — but worth verifying that the UuidSchema normalizes to lowercase (e.g. via .toLowerCase() or .transform()), otherwise a client sending uppercase A-F hex chars would miss the match.

  3. GetSpaceResponse is missing safeCount in e2e tests (spaces.controller.e2e-spec.ts): The GET /spaces e2e test asserts full object shape but doesn't check safeCount. This is pre-existing but worth adding since it's part of the DTO.

Minor / nits

  1. Invitation entity (invitation.entity.ts:20): @ApiProperty({ type: String }) for spaceId is correct now (was Number). Good.

  2. space.entity.ts:21: The Omit<z.infer<typeof RowSchema>, 'id'> & { id: UUID; ... } pattern works but is a bit complex. An alternative would be RowSchema.omit({ id: true }).extend(...) to keep it purely in Zod-land, though this is stylistic.

Sequential SERIAL int PKs leak total space count and enable enumeration.
Uses add-backfill-swap pattern since PG can't ALTER int→uuid in-place.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vseehausen vseehausen force-pushed the valentin/wa-1583-replace-sequential-spaceid-with-global-uuid branch from 2172096 to cd152cb Compare March 23, 2026 11:17
vseehausen and others added 3 commits March 23, 2026 12:23
Change DB entity PK from SERIAL int to uuid, override RowSchema's int
id with UuidSchema in domain schema, and update repository signatures
and DTO ApiProperty types from Number to String.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace ParseIntPipe + RowSchema.shape.id with UuidSchema for all
spaceId route params. userId params in MembersController stay as int.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace faker.number.int() with faker.string.uuid() for space IDs
in builders, e2e specs, and integration specs. User/member IDs stay
as integers.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vseehausen vseehausen force-pushed the valentin/wa-1583-replace-sequential-spaceid-with-global-uuid branch from cd152cb to cd721bf Compare March 23, 2026 11:23
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.

2 participants