From 311f663523b48e552d8437ef6cc74b661dfd7eea Mon Sep 17 00:00:00 2001 From: Mevy Assistant Date: Fri, 24 Apr 2026 08:38:29 +0200 Subject: [PATCH] Harden tenant lifecycle validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reject empty tenant input, normalize read-path lookups, and treat shared platform resource aliases as reserved so lifecycle validation catches underscore and hyphen collisions consistently. --- Build: pass | Tests: pass — 25 passed (2 files) --- docs/internal/MULTITENANT-HANDOFF.md | 5 ++ scripts/tenant-lifecycle.ts | 96 +++++++++++++++------------- src/platform-layout.test.ts | 5 ++ src/platform-layout.ts | 8 +++ src/tenant-registry.test.ts | 19 ++++++ src/tenant-registry.ts | 92 ++++++++++++++++++++------ 6 files changed, 161 insertions(+), 64 deletions(-) diff --git a/docs/internal/MULTITENANT-HANDOFF.md b/docs/internal/MULTITENANT-HANDOFF.md index 5b52425..3a15ad2 100644 --- a/docs/internal/MULTITENANT-HANDOFF.md +++ b/docs/internal/MULTITENANT-HANDOFF.md @@ -156,6 +156,11 @@ Also updated: - `just tenant-plan ` - `just tenant-add ` - `just tenant-remove ` (dry-run) +- tenant lifecycle validation is now stricter: + - empty tenant names are rejected + - shared/platform resource aliases are reserved, even when underscores and + hyphens differ + - lifecycle lookups normalize operator input on read paths too ## Recommended first code tasks diff --git a/scripts/tenant-lifecycle.ts b/scripts/tenant-lifecycle.ts index 3fa3a0b..c25613b 100644 --- a/scripts/tenant-lifecycle.ts +++ b/scripts/tenant-lifecycle.ts @@ -73,60 +73,66 @@ function printRemovalPlan( } function main(argv: string[]): void { - const [command, ...rest] = argv; - if (!command) usage(); + try { + const [command, ...rest] = argv; + if (!command) usage(); - if (command === 'list') { - for (const tenant of listTenantRecords()) { - console.log(`${tenant.id}\t${tenant.service}`); + if (command === 'list') { + for (const tenant of listTenantRecords()) { + console.log(`${tenant.id}\t${tenant.service}`); + } + return; } - return; - } - const options = parseOptions(rest); - if (!options.tenantId) usage(); + const options = parseOptions(rest); + if (!options.tenantId) usage(); - if (command === 'show') { - const tenant = getTenantRecord(options.tenantId); - if (!tenant) { - console.error(`Unknown tenant: ${options.tenantId}`); - process.exit(1); + if (command === 'show') { + const tenant = getTenantRecord(options.tenantId); + if (!tenant) { + console.error(`Unknown tenant: ${options.tenantId}`); + process.exit(1); + } + printTenantSummary(tenant); + return; } - printTenantSummary(tenant); - return; - } - if (command === 'plan') { - const draft = deriveTenantRecord(options.tenantId, { - displayName: options.displayName, - domain: options.domain, - }); - console.log( - tenantExists(draft.id) - ? `Tenant ${draft.id} already exists in registry. Derived plan shown for comparison.` - : `Derived plan for new tenant ${draft.id}:`, - ); - printTenantSummary(draft); - return; - } + if (command === 'plan') { + const draft = deriveTenantRecord(options.tenantId, { + displayName: options.displayName, + domain: options.domain, + }); + console.log( + tenantExists(draft.id) + ? `Tenant ${draft.id} already exists in registry. Derived plan shown for comparison.` + : `Derived plan for new tenant ${draft.id}:`, + ); + printTenantSummary(draft); + return; + } - if (command === 'add') { - const added = addTenantRecord(options.tenantId, { - displayName: options.displayName, - domain: options.domain, - }); - console.log(`Added tenant ${added.id} to infra/tenants.yaml`); - printTenantSummary(added); - return; - } + if (command === 'add') { + const added = addTenantRecord(options.tenantId, { + displayName: options.displayName, + domain: options.domain, + }); + console.log(`Added tenant ${added.id} to infra/tenants.yaml`); + printTenantSummary(added); + return; + } - if (command === 'remove') { - const plan = planTenantRemoval(options.tenantId); - printRemovalPlan(plan); - return; - } + if (command === 'remove') { + const plan = planTenantRemoval(options.tenantId); + printRemovalPlan(plan); + return; + } - usage(); + usage(); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + console.error(message); + process.exit(1); + } } main(process.argv.slice(2)); diff --git a/src/platform-layout.test.ts b/src/platform-layout.test.ts index 944f656..7c3ed1f 100644 --- a/src/platform-layout.test.ts +++ b/src/platform-layout.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest'; import { + deriveTenantId, defaultTenantDisplayName, normalizePlatformId, normalizeTenantId, @@ -28,6 +29,10 @@ describe('platform-layout', () => { expect(normalizeTenantId('ščćž')).toBe('sccz'); }); + it('rejects empty tenant names when deriving tenant ids', () => { + expect(() => deriveTenantId(' ')).toThrow('Tenant name cannot be empty'); + }); + it('resolves tenant ids and platform service names from explicit and legacy inputs', () => { expect(resolveTenantId('Mevy Agent')).toBe('mevy-agent'); expect(resolveTenantId(undefined)).toBe('clawdie'); diff --git a/src/platform-layout.ts b/src/platform-layout.ts index cf2dfd5..c84ae25 100644 --- a/src/platform-layout.ts +++ b/src/platform-layout.ts @@ -89,6 +89,14 @@ export function normalizeTenantId(value: string): string { return normalizeResourceId(value); } +export function deriveTenantId(value: string): string { + const trimmed = value.trim(); + if (!trimmed) { + throw new Error('Tenant name cannot be empty'); + } + return normalizeTenantId(trimmed); +} + export function resolveTenantId( tenantId?: string | null, fallback = 'clawdie', diff --git a/src/tenant-registry.test.ts b/src/tenant-registry.test.ts index 4bae8bc..a5b58a1 100644 --- a/src/tenant-registry.test.ts +++ b/src/tenant-registry.test.ts @@ -111,6 +111,20 @@ describe('tenant-registry', () => { ).toThrow('Tenant service conflicts with shared service: clawdie_hostd'); }); + it('rejects tenant ids reserved by shared resource aliases', () => { + const registryPath = makeTempRegistry(); + expect(() => addTenantRecord('clawdie-hostd', {}, registryPath)).toThrow( + 'Tenant id is reserved by platform resources: clawdie-hostd', + ); + }); + + it('rejects empty tenant input before deriving a registry record', () => { + const registryPath = makeTempRegistry(); + expect(() => addTenantRecord(' ', {}, registryPath)).toThrow( + 'Tenant name cannot be empty', + ); + }); + it('can write a registry round-trip', () => { const registryPath = makeTempRegistry(); const registry = loadTenantRegistry(registryPath); @@ -138,4 +152,9 @@ describe('tenant-registry', () => { 'Unknown tenant: unknown', ); }); + + it('normalizes tenant lookups on read paths', () => { + expect(getTenantRecord('Mevy Agent')).toBeNull(); + expect(getTenantRecord('MEVY')?.id).toBe('mevy'); + }); }); diff --git a/src/tenant-registry.ts b/src/tenant-registry.ts index fbb931a..10c73d7 100644 --- a/src/tenant-registry.ts +++ b/src/tenant-registry.ts @@ -5,8 +5,11 @@ import { fileURLToPath } from 'url'; import { parse, stringify } from 'yaml'; import { z } from 'zod'; +import { normalizeDbIdentifierBase } from './db-identifiers.js'; import { + deriveTenantId, defaultTenantDisplayName, + normalizeResourceId, tenantBrainDbName, tenantForgejoDbName, tenantInternalDomain, @@ -14,7 +17,6 @@ import { tenantServiceName, tenantSkillsDbName, tenantWorkerJailName, - normalizeTenantId, } from './platform-layout.js'; const __filename = fileURLToPath(import.meta.url); @@ -141,11 +143,32 @@ function deriveDisplayName(rawTenantInput: string, normalizedId: string): string return raw === normalizedId ? defaultTenantDisplayName(normalizedId) : raw; } +function canonicalResourceAlias(value: string): string { + return normalizeResourceId(value); +} + +function canonicalDomain(value: string): string { + return value.trim().toLowerCase().replace(/\.+$/u, ''); +} + +function canonicalDbAlias(value: string): string { + return normalizeDbIdentifierBase(value); +} + +function hasCanonicalOverlap( + left: string[], + right: string[], + canonicalize: (value: string) => string, +): boolean { + const values = new Set(left.map(canonicalize)); + return right.some((item) => values.has(canonicalize(item))); +} + export function deriveTenantRecord( tenantId: string, options: TenantDraftOptions = {}, ): TenantRecord { - const id = normalizeTenantId(tenantId); + const id = deriveTenantId(tenantId); return { id, displayName: options.displayName || deriveDisplayName(tenantId, id), @@ -208,7 +231,7 @@ export function getTenantRecord( tenantId: string, registry = loadTenantRegistry(), ): TenantRecord | null { - return registry.tenants[tenantId] || null; + return registry.tenants[deriveTenantId(tenantId)] || null; } export function listTenantRecords( @@ -223,22 +246,35 @@ export function tenantExists( tenantId: string, registry = loadTenantRegistry(), ): boolean { - return !!getTenantRecord(normalizeTenantId(tenantId), registry); -} - -function hasOverlap(left: string[], right: string[]): boolean { - const values = new Set(left); - return right.some((item) => values.has(item)); + return !!getTenantRecord(deriveTenantId(tenantId), registry); } function validateTenantRecord( candidate: TenantRecord, registry: PlatformRegistry, ): void { - if (registry.shared.services.includes(candidate.service)) { - throw new Error(`Tenant service conflicts with shared service: ${candidate.service}`); + const candidateIdAlias = canonicalResourceAlias(candidate.id); + const sharedServiceAliases = new Set( + registry.shared.services.map(canonicalResourceAlias), + ); + const sharedJailAliases = new Set( + registry.shared.jails.map(canonicalResourceAlias), + ); + const reservedIdAliases = new Set([ + canonicalResourceAlias(registry.platform.id), + ...sharedServiceAliases, + ...sharedJailAliases, + ]); + + if (reservedIdAliases.has(candidateIdAlias)) { + throw new Error(`Tenant id is reserved by platform resources: ${candidate.id}`); } - if (registry.shared.jails.includes(candidate.service)) { + if (sharedServiceAliases.has(canonicalResourceAlias(candidate.service))) { + throw new Error( + `Tenant service conflicts with shared service: ${candidate.service}`, + ); + } + if (sharedJailAliases.has(canonicalResourceAlias(candidate.service))) { throw new Error(`Tenant service conflicts with shared jail: ${candidate.service}`); } @@ -246,23 +282,41 @@ function validateTenantRecord( if (existing.id === candidate.id) { throw new Error(`Tenant already exists: ${candidate.id}`); } - if (existing.service === candidate.service) { - throw new Error(`Tenant service conflicts with existing tenant: ${candidate.service}`); + if ( + canonicalResourceAlias(existing.service) === + canonicalResourceAlias(candidate.service) + ) { + throw new Error( + `Tenant service conflicts with existing tenant: ${candidate.service}`, + ); } - if (existing.internalDomain === candidate.internalDomain) { + if ( + canonicalDomain(existing.internalDomain) === + canonicalDomain(candidate.internalDomain) + ) { throw new Error( `Tenant internal domain conflicts with existing tenant: ${candidate.internalDomain}`, ); } - if (hasOverlap(existing.workerJails, candidate.workerJails)) { + if ( + hasCanonicalOverlap( + existing.workerJails, + candidate.workerJails, + canonicalResourceAlias, + ) + ) { throw new Error(`Tenant worker jails conflict with existing tenant: ${candidate.id}`); } if ( - hasOverlap(Object.values(existing.databases), Object.values(candidate.databases)) + hasCanonicalOverlap( + Object.values(existing.databases), + Object.values(candidate.databases), + canonicalDbAlias, + ) ) { throw new Error(`Tenant databases conflict with existing tenant: ${candidate.id}`); } - if (hasOverlap(existing.datasets, candidate.datasets)) { + if (hasCanonicalOverlap(existing.datasets, candidate.datasets, canonicalDomain)) { throw new Error(`Tenant datasets conflict with existing tenant: ${candidate.id}`); } } @@ -340,7 +394,7 @@ export function planTenantRemoval( registryPath = REGISTRY_PATH, ): TenantRemovalPlan { const registry = loadTenantRegistry(registryPath); - const normalizedId = normalizeTenantId(tenantId); + const normalizedId = deriveTenantId(tenantId); const tenant = getTenantRecord(normalizedId, registry); if (!tenant) {