Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ services:
REGISTRY_STORE: ${REGISTRY_STORE:-postgres}
REGISTRY_DATABASE_URL: ${REGISTRY_DATABASE_URL:-postgresql://${POSTGRES_USER:-fides}:${POSTGRES_PASSWORD:?Set POSTGRES_PASSWORD in .env}@postgres:5432/${POSTGRES_DB:-fides}}
REGISTRY_DB_AUTO_MIGRATE: ${REGISTRY_DB_AUTO_MIGRATE:-true}
REGISTRY_DB_SCHEMA: ${REGISTRY_DB_SCHEMA:-registry}
REGISTRY_DB_POOL_MAX: ${REGISTRY_DB_POOL_MAX:-10}
NODE_ENV: production
CORS_ORIGIN: ${CORS_ORIGIN:-}
Expand Down
1 change: 1 addition & 0 deletions docs/deployment.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ cp .env.example .env
| `REGISTRY_STORE` | `file` | production | `file` for local JSON state, `postgres` for durable hosted registry state |
| `REGISTRY_DATABASE_URL` | _(empty)_ | production when `REGISTRY_STORE=postgres` | Dedicated registry database URL. Falls back to `DATABASE_URL` when unset. |
| `REGISTRY_DB_AUTO_MIGRATE`| `true` | no | Runs idempotent registry migrations on startup and records applied ids plus checksums in `registry_schema_migrations`. Set `false` when migrations are managed externally. |
| `REGISTRY_DB_SCHEMA` | `registry` in Docker Compose, otherwise _(empty)_ | no | Optional Postgres schema name for registry tables when multiple services share one database. Must be a simple identifier. |
| `REGISTRY_DB_POOL_MAX` | `10` | no | Registry store connection pool size. Falls back to `DB_POOL_MAX`. |
| `REGISTRY_STORE_PATH` | _(empty)_ | no | File registry store path. Defaults to `~/.fides/registry/registry.json`. |

Expand Down
9 changes: 2 additions & 7 deletions services/registry/src/migrate.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,12 @@
import postgres from 'postgres'
import { runRegistryMigrations } from './storage.js'
import { createRegistryClient, runRegistryMigrations } from './storage.js'

async function main(): Promise<void> {
const connectionString = process.env.REGISTRY_DATABASE_URL || process.env.DATABASE_URL
if (!connectionString) {
throw new Error('REGISTRY_DATABASE_URL or DATABASE_URL is required')
}

const sql = postgres(connectionString, {
max: 1,
idle_timeout: 5,
connect_timeout: 10,
})
const sql = createRegistryClient(connectionString)

try {
await runRegistryMigrations(sql)
Expand Down
39 changes: 34 additions & 5 deletions services/registry/src/storage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,7 @@ export class PostgresRegistryStore implements RegistryStore {
private initialized: Promise<void>

constructor(connectionString = requiredDatabaseUrl()) {
this.sql = postgres(connectionString, {
max: parseInt(process.env.REGISTRY_DB_POOL_MAX || process.env.DB_POOL_MAX || '10', 10),
idle_timeout: 20,
connect_timeout: 10,
})
this.sql = createRegistryClient(connectionString)
this.initialized = this.init()
}

Expand Down Expand Up @@ -245,6 +241,7 @@ export async function runRegistryMigrations(sql: postgres.Sql): Promise<void> {
await sql`SELECT pg_advisory_lock(hashtext('registry_migrations'))`

try {
await ensureConfiguredRegistrySchema(sql)
await ensureRegistryMigrationLedger(sql)
for (const migration of REGISTRY_MIGRATIONS) {
const checksum = registryMigrationChecksum(migration)
Expand Down Expand Up @@ -280,6 +277,14 @@ export async function runRegistryMigrations(sql: postgres.Sql): Promise<void> {
}
}

async function ensureConfiguredRegistrySchema(sql: postgres.Sql): Promise<void> {
const schemaName = process.env.REGISTRY_DB_SCHEMA
if (!schemaName) return

assertValidRegistrySchemaName(schemaName)
await sql.unsafe(`CREATE SCHEMA IF NOT EXISTS "${schemaName}"`)
}

async function ensureRegistryMigrationLedger(sql: postgres.Sql): Promise<void> {
await sql`
CREATE TABLE IF NOT EXISTS registry_schema_migrations (
Expand Down Expand Up @@ -362,3 +367,27 @@ function requiredDatabaseUrl(): string {
}
return url
}

export function createRegistryClient(connectionString = requiredDatabaseUrl()): postgres.Sql {
return postgres(withRegistrySearchPath(connectionString, process.env.REGISTRY_DB_SCHEMA), {
max: parseInt(process.env.REGISTRY_DB_POOL_MAX || process.env.DB_POOL_MAX || '10', 10),
idle_timeout: 20,
connect_timeout: 10,
})
}

function withRegistrySearchPath(connectionString: string, schemaName?: string): string {
if (!schemaName) return connectionString

assertValidRegistrySchemaName(schemaName)

const url = new URL(connectionString)
url.searchParams.set('options', `-c search_path=${schemaName},public`)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Quote mixed-case schema names in search_path

When REGISTRY_DB_SCHEMA contains uppercase letters (which the validator accepts), this unquoted search_path entry is folded to lowercase by Postgres, while ensureConfiguredRegistrySchema creates the schema as a quoted case-sensitive name. For example REGISTRY_DB_SCHEMA=Registry creates schema "Registry", but the client searches registry,public, so migrations and runtime tables fall back to public instead of the configured schema. Either reject uppercase names or quote/escape the search path entry consistently with schema creation.

Useful? React with 👍 / 👎.

return url.toString()
}

function assertValidRegistrySchemaName(schemaName: string): void {
if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(schemaName)) {
throw new Error('REGISTRY_DB_SCHEMA must be a simple Postgres identifier')
}
}
59 changes: 59 additions & 0 deletions services/registry/test/storage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
InMemoryRegistryStore,
PostgresRegistryStore,
REGISTRY_MIGRATIONS,
createRegistryClient,
runRegistryMigrations,
type RegistryEntry,
} from '../src/storage.js'
Expand Down Expand Up @@ -70,6 +71,23 @@ describe('Registry stores', () => {
expect(await reader.healthCheck()).toMatchObject({ ok: true, kind: 'file' })
})

it('rejects unsafe configured registry schema names', () => {
const previousSchema = process.env.REGISTRY_DB_SCHEMA

try {
process.env.REGISTRY_DB_SCHEMA = 'registry;DROP'
expect(() => createRegistryClient('postgresql://fides:fides@localhost:5432/fides')).toThrow(
'REGISTRY_DB_SCHEMA must be a simple Postgres identifier',
)
} finally {
if (previousSchema === undefined) {
delete process.env.REGISTRY_DB_SCHEMA
} else {
process.env.REGISTRY_DB_SCHEMA = previousSchema
}
}
})

describe.skipIf(!postgresUrl)('postgres registry store', () => {
it('records applied registry migrations once with checksums', async () => {
const sql = postgres(postgresUrl, { max: 1 })
Expand Down Expand Up @@ -117,6 +135,47 @@ describe('Registry stores', () => {
}
}, 30_000)

it('creates and uses the configured registry schema', async () => {
const schema = `registry_configured_${crypto.randomUUID().replaceAll('-', '')}`
const schemaIdentifier = quoteIdentifier(schema)
const adminSql = postgres(postgresUrl, { max: 1 })
const scopedUrl = postgresUrlWithSearchPath(postgresUrl, schema)
const scopedSql = postgres(scopedUrl, { max: 1 })
const previousSchema = process.env.REGISTRY_DB_SCHEMA

try {
process.env.REGISTRY_DB_SCHEMA = schema
await runRegistryMigrations(scopedSql)

const schemaRows = await adminSql`
SELECT schema_name
FROM information_schema.schemata
WHERE schema_name = ${schema}
`
expect(schemaRows).toHaveLength(1)

const tableRows = await adminSql`
SELECT table_name
FROM information_schema.tables
WHERE table_schema = ${schema}
AND table_name IN ('registry_cards', 'registry_schema_migrations')
`
expect(tableRows).toHaveLength(2)

const currentSchema = await scopedSql`SELECT current_schema() AS schema`
expect(currentSchema[0]?.schema).toBe(schema)
} finally {
if (previousSchema === undefined) {
delete process.env.REGISTRY_DB_SCHEMA
} else {
process.env.REGISTRY_DB_SCHEMA = previousSchema
}
await scopedSql.end()
await adminSql.unsafe(`DROP SCHEMA IF EXISTS ${schemaIdentifier} CASCADE`)
await adminSql.end()
}
}, 30_000)

it('fails closed when an applied migration checksum drifts', async () => {
const schema = `registry_migration_checksum_${crypto.randomUUID().replaceAll('-', '')}`
const schemaIdentifier = quoteIdentifier(schema)
Expand Down