Add sharding key to dast_site_tokens token unique index

Gregory Havengarequested to merge
599519-cells-index-dast_site_tokens into master

What does this MR do and why?

Make the dast_site_tokens.token unique index Cells-compliant by including the table's sharding key project_id.

Replaces the globally-unique index index_dast_site_token_on_token (token) with a composite unique index index_dast_site_token_on_token_and_project_id (token, project_id), and de-scopes the DastSiteToken model validation to uniqueness: { scope: :project_id }.

Part of #599519 (Audit and fix gitlab_sec unique indexes for Cells compliance). Follows the pattern established in #592042 (closed) and example MRs !208594 (merged) and !214499 (merged).

Cells unique-constraint rationale

Per the Cells unique constraint requirements, a unique index on a gitlab_sec table must include the table's sharding key so uniqueness can be enforced within a single cell's database partition. Because organizations migrate between cells, we can no longer rely on globally-unique constraints across all cells.

dast_site_tokens.token is a free-form random string (token { SecureRandom.uuid } in the factory; created via SecureRandom in AppSec::Dast::SiteTokens::FindOrCreateService). It is not derived from project_id and is not a globally-unique FK/PK, so, unlike most other tables in #599519, it genuinely requires the sharding key to be added (mirrors the emails.confirmation_token / duo_workflows_events.correlation_id cases). The composite index leads with token and appends the sharding key project_id, matching the structure used in !208594 (merged).

The project_id column already exists on the table (CONSTRAINT ... CHECK (project_id IS NOT NULL)), so no column-add or backfill is required. This is a straightforward index swap.

Why this does not weaken site ownership proof

Dropping global uniqueness on token does not make site validation easier to spoof:

  • Tokens are generated server-side with SecureRandom.uuid (ee/app/services/app_sec/dast/site_tokens/find_or_create_service.rb:38). Users never choose the value.
  • Tokens are only ever looked up scoped to a project: find_or_create_service.rb:34 (find_by(project:, url:)), ee/app/models/dast_site.rb:20 (where(project_id:, url:)), and ee/app/models/dast_site_validation.rb:13 (where(dast_site_tokens: { project_id: })).

No code path resolves a token globally, so a token in another project was never usable as proof for this project. Uniqueness within a project is still enforced by the composite index and the scoped validation.

Database review notes

Weaker constraint: global unique becomes per-project unique

The old index index_dast_site_token_on_token made each token value unique across the whole table. The new index index_dast_site_token_on_token_and_project_id only makes each (token, project_id) pair unique, so the same token value can now exist in two different projects. This is a weaker constraint than before. It is acceptable because tokens are generated server-side with SecureRandom.uuid and are only looked up scoped to a project (see the section above). Inside one project, a duplicate token is still rejected by both the index and the model validation.

Composite index column order

The new composite unique index (token, project_id) leads with token. A B-tree index can serve a lookup on its leading column alone, so token-only lookups still use an index scan on the new index (plan 4 below shows an Index Only Scan on index_dast_site_token_on_token_and_project_id with an Index Cond on token only). No query loses index support when the old (token) index is removed. See composite index column order.

Why a regular migration

  • db/docs/dast_site_tokens.yml classifies the table as table_size: small, and it is not in the large tables list in rubocop/rubocop-migrations.yml.
  • On the gitlab-production-sec Database Lab clone (2026-09-25): 2622 rows (EXPLAIN ANALYZE SELECT count(*), joe command 502264), table size 400 kB (\dt+), old index index_dast_site_token_on_token 248 kB, new index index_dast_site_token_on_token_and_project_id 192 kB after building it on the clone (\di+).
  • On the clone, CREATE UNIQUE INDEX index_dast_site_token_on_token_and_project_id ON dast_site_tokens USING btree (token, project_id); DROP INDEX index_dast_site_token_on_token; ANALYZE dast_site_tokens; ran in 34.353 ms (joe command 502272, non-concurrent on the clone).
  • The index guidelines allow a regular migration when "a regular migration can create the required index very fast (usually because the table is new or very small)", with the application code change in the same MR. The model validation change ships in this MR, so the index change runs as a regular migration before the new code deploys. The migration adds the new index before it removes the old one, so there is no point where token has no unique index.

Query plans for the uniqueness validation query

Source: postgres.ai Database Lab, project gitlab-production-sec, joe explain (EXPLAIN plus EXPLAIN ANALYZE), 2026-09-25. The token literal is a random UUID that is not in the table, which is the normal case when the validation runs for a newly generated token. project_id = 278964 is an arbitrary value. Plans 3 and 4 were taken on the same clone after the index swap above.

1. Before: old validation query (token only), old index (joe command 502268) 
SELECT 1 AS one FROM "dast_site_tokens" WHERE "dast_site_tokens"."token" = 'b3f6c1e2-7a4d-4c1b-9e2f-5d8a0c3e6f14' LIMIT 1
 Limit  (cost=0.28..1.80 rows=1 width=4) (actual time=0.024..0.025 rows=0 loops=1)
   Buffers: shared hit=2
   I/O Timings: read=0.000 write=0.000
   ->  Index Only Scan using index_dast_site_token_on_token on public.dast_site_tokens  (cost=0.28..1.80 rows=1 width=4) (actual time=0.023..0.023 rows=0 loops=1)
         Index Cond: (dast_site_tokens.token = 'b3f6c1e2-7a4d-4c1b-9e2f-5d8a0c3e6f14'::text)
         Heap Fetches: 0
         Index Searches: 1
         Buffers: shared hit=2
         I/O Timings: read=0.000 write=0.000
Settings: random_page_cost = '1.5', seq_page_cost = '4', effective_cache_size = '338688MB', jit = 'off', work_mem = '100MB'
Query ID: -7051517896993517267

Time: 1.066 ms
  - planning: 1.004 ms
  - execution: 0.062 ms
    - I/O read: 0.000 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 2 (~16.00 KiB) from the buffer pool
  - reads: 0 from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0
2. Before: new validation query (token and project_id), old index (joe command 502270) 
SELECT 1 AS one FROM "dast_site_tokens" WHERE "dast_site_tokens"."token" = 'b3f6c1e2-7a4d-4c1b-9e2f-5d8a0c3e6f14' AND "dast_site_tokens"."project_id" = 278964 LIMIT 1
 Limit  (cost=0.28..3.30 rows=1 width=4) (actual time=0.025..0.026 rows=0 loops=1)
   Buffers: shared hit=2
   I/O Timings: read=0.000 write=0.000
   ->  Index Scan using index_dast_site_token_on_token on public.dast_site_tokens  (cost=0.28..3.30 rows=1 width=4) (actual time=0.023..0.023 rows=0 loops=1)
         Index Cond: (dast_site_tokens.token = 'b3f6c1e2-7a4d-4c1b-9e2f-5d8a0c3e6f14'::text)
         Index Searches: 1
         Filter: (dast_site_tokens.project_id = 278964)
         Buffers: shared hit=2
         I/O Timings: read=0.000 write=0.000
Settings: random_page_cost = '1.5', seq_page_cost = '4', effective_cache_size = '338688MB', jit = 'off', work_mem = '100MB'
Query ID: -2855463949215569740

Time: 0.958 ms
  - planning: 0.898 ms
  - execution: 0.060 ms
    - I/O read: 0.000 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 2 (~16.00 KiB) from the buffer pool
  - reads: 0 from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0
3. After: new validation query (token and project_id), new composite index (joe command 502277) 
SELECT 1 AS one FROM "dast_site_tokens" WHERE "dast_site_tokens"."token" = 'b3f6c1e2-7a4d-4c1b-9e2f-5d8a0c3e6f14' AND "dast_site_tokens"."project_id" = 278964 LIMIT 1
 Limit  (cost=0.28..3.30 rows=1 width=4) (actual time=0.029..0.030 rows=0 loops=1)
   Buffers: shared hit=5
   I/O Timings: read=0.000 write=0.000
   ->  Index Only Scan using index_dast_site_token_on_token_and_project_id on public.dast_site_tokens  (cost=0.28..3.30 rows=1 width=4) (actual time=0.028..0.028 rows=0 loops=1)
         Index Cond: ((dast_site_tokens.token = 'b3f6c1e2-7a4d-4c1b-9e2f-5d8a0c3e6f14'::text) AND (dast_site_tokens.project_id = 278964))
         Heap Fetches: 0
         Index Searches: 1
         Buffers: shared hit=5
         I/O Timings: read=0.000 write=0.000
Settings: seq_page_cost = '4', effective_cache_size = '338688MB', jit = 'off', work_mem = '100MB', random_page_cost = '1.5'
Query ID: -2855463949215569740

Time: 1.201 ms
  - planning: 1.136 ms
  - execution: 0.065 ms
    - I/O read: 0.000 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 5 (~40.00 KiB) from the buffer pool
  - reads: 0 from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0
4. After: token-only lookup, new composite index (joe command 502276) 
SELECT 1 AS one FROM "dast_site_tokens" WHERE "dast_site_tokens"."token" = 'b3f6c1e2-7a4d-4c1b-9e2f-5d8a0c3e6f14' LIMIT 1
 Limit  (cost=0.28..3.30 rows=1 width=4) (actual time=0.206..0.207 rows=0 loops=1)
   Buffers: shared read=2
   I/O Timings: read=0.139 write=0.000
   ->  Index Only Scan using index_dast_site_token_on_token_and_project_id on public.dast_site_tokens  (cost=0.28..3.30 rows=1 width=4) (actual time=0.204..0.205 rows=0 loops=1)
         Index Cond: (dast_site_tokens.token = 'b3f6c1e2-7a4d-4c1b-9e2f-5d8a0c3e6f14'::text)
         Heap Fetches: 0
         Index Searches: 1
         Buffers: shared read=2
         I/O Timings: read=0.139 write=0.000
Settings: seq_page_cost = '4', effective_cache_size = '338688MB', jit = 'off', work_mem = '100MB', random_page_cost = '1.5'
Query ID: -7051517896993517267

Time: 1.234 ms
  - planning: 0.986 ms
  - execution: 0.248 ms
    - I/O read: 0.139 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 0 from the buffer pool
  - reads: 2 (~16.00 KiB) from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

Changes

  • db/migrate/20260924125415_update_index_dast_site_token_on_token_on_dast_site_tokens.rb: add composite unique index (token, project_id) via add_concurrent_index, then remove_concurrent_index_by_name for the old (token) index. down restores the old index; a comment notes that rollback fails if the same token exists in more than one project, which is not expected because tokens are server-generated.
  • db/structure.sql + db/schema_migrations/20260924125415: regenerated.
  • ee/app/models/dast_site_token.rb: validation uniqueness: true changed to uniqueness: { scope: :project_id }.
  • ee/spec/models/dast_site_token_spec.rb: matcher updated to validate_uniqueness_of(:token).scoped_to(:project_id), plus examples covering the same token in different projects (valid), the same token in the same project (invalid), and a database-level duplicate raising ActiveRecord::RecordNotUnique.

Testing done locally

  • bundle exec rspec ee/spec/models/dast_site_token_spec.rb: 18 examples, 0 failures.
  • Migration up and down against the sec database:
db:migrate:up:sec 
sec: == 20260924125415 UpdateIndexDastSiteTokenOnTokenOnDastSiteTokens: migrating ==
sec: -- transaction_open?(nil)
sec:    -> 0.0000s
sec: -- view_exists?(:postgres_partitions)
sec:    -> 0.0052s
sec: -- index_exists?(:dast_site_tokens, [:token, :project_id], {:unique=>true, :name=>:index_dast_site_token_on_token_and_project_id, :algorithm=>:concurrently})
sec:    -> 0.1026s
sec: -- execute("SET statement_timeout TO 0")
sec:    -> 0.0005s
sec: -- add_index(:dast_site_tokens, [:token, :project_id], {:unique=>true, :name=>:index_dast_site_token_on_token_and_project_id, :algorithm=>:concurrently})
sec:    -> 0.0055s
sec: -- execute("RESET statement_timeout")
sec:    -> 0.0006s
sec: -- transaction_open?(nil)
sec:    -> 0.0000s
sec: -- view_exists?(:postgres_partitions)
sec:    -> 0.0007s
sec: -- index_name_exists?(:dast_site_tokens, :index_dast_site_token_on_token)
sec:    -> 0.0019s
sec: -- remove_index(:dast_site_tokens, {:algorithm=>:concurrently, :name=>:index_dast_site_token_on_token})
sec:    -> 0.0085s
sec: == 20260924125415 UpdateIndexDastSiteTokenOnTokenOnDastSiteTokens: migrated (0.1862s)
db:migrate:down:sec 
sec: == 20260924125415 UpdateIndexDastSiteTokenOnTokenOnDastSiteTokens: reverting ==
sec: -- transaction_open?(nil)
sec:    -> 0.0000s
sec: -- view_exists?(:postgres_partitions)
sec:    -> 0.0079s
sec: -- index_exists?(:dast_site_tokens, [:token], {:unique=>true, :name=>:index_dast_site_token_on_token, :algorithm=>:concurrently})
sec:    -> 0.0038s
sec: -- execute("SET statement_timeout TO 0")
sec:    -> 0.0011s
sec: -- add_index(:dast_site_tokens, [:token], {:unique=>true, :name=>:index_dast_site_token_on_token, :algorithm=>:concurrently})
sec:    -> 0.0020s
sec: -- execute("RESET statement_timeout")
sec:    -> 0.0006s
sec: -- transaction_open?(nil)
sec:    -> 0.0000s
sec: -- view_exists?(:postgres_partitions)
sec:    -> 0.0008s
sec: -- index_name_exists?(:dast_site_tokens, :index_dast_site_token_on_token_and_project_id)
sec:    -> 0.0010s
sec: -- remove_index(:dast_site_tokens, {:algorithm=>:concurrently, :name=>:index_dast_site_token_on_token_and_project_id})
sec:    -> 0.0021s
sec: == 20260924125415 UpdateIndexDastSiteTokenOnTokenOnDastSiteTokens: reverted (0.1318s)

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist.

Related to #599519

Edited by Gregory Havenga

Merge request reports