diff --git a/apps/api/src/templates/blob-template-store.security.spec.ts b/apps/api/src/templates/blob-template-store.security.spec.ts index b8864b0..c9aa2b8 100644 --- a/apps/api/src/templates/blob-template-store.security.spec.ts +++ b/apps/api/src/templates/blob-template-store.security.spec.ts @@ -160,15 +160,20 @@ describe('BlobTemplateStore — environment isolation', () => { }); }); -describe('BlobTemplateStore — tenant identity is interpolated into the blob name verbatim', () => { - // Documents CURRENT behaviour, not desired behaviour. Azure Blob Storage has a - // flat namespace, so these values are not path traversal — they are literal - // parts of the blob name. The point is that `tenantId` / `environment` are - // interpolated without the publisher's `assertSafeTenantId` / - // `assertSafeEnvironment` guards, so a value containing `/` silently produces a - // blob name outside the documented per-tenant prefix shape. Not reachable from - // a request today (identity comes from the credential map). Tracked in #59. - it('interpolates a slash-bearing tenantId verbatim, producing a blob name outside the documented prefix shape (known gap)', async () => { +describe('BlobTemplateStore — tenant identity path validation', () => { + async function expectInvalidTemplate( + run: () => Promise, + requestedPaths: string[], + ): Promise { + await assert.rejects(run, (err: unknown) => { + assert.ok(err instanceof TemplateStoreError, 'expected TemplateStoreError'); + assert.equal(err.code, PostKitErrorCode.INVALID_TEMPLATE); + return true; + }); + assert.deepEqual(requestedPaths, [], 'no blob access may happen for an unsafe tenant path'); + } + + it('rejects a slash-bearing tenantId before storage access', async () => { const blobs = new Map(); const tenantId = 'tenant-a/production/templates/shared'; const base = `templates/tenants/${tenantId}/production/templates/${TEMPLATE_KEY}`; @@ -177,32 +182,52 @@ describe('BlobTemplateStore — tenant identity is interpolated into the blob na const { client, requestedPaths } = makeRecordingClient(blobs); const unvalidated = { tenantId, environment: 'production' } as unknown as TenantContext; - const loaded = await makeStore(client).load(unvalidated, TEMPLATE_KEY); - - assert.equal(loaded.templateHtml, HTML); - for (const path of requestedPaths) { - assert.ok( - path.startsWith(`tenants/${tenantId}/production/templates/`), - 'current behaviour: the tenantId is used as-is, extra segments included', - ); - } + await expectInvalidTemplate( + () => makeStore(client).load(unvalidated, TEMPLATE_KEY), + requestedPaths, + ); }); - it('interpolates an environment value that is not a TenantEnvironment (known gap)', async () => { + it('rejects an environment value that is not a TenantEnvironment before storage access', async () => { const blobs = new Map(); const base = `templates/tenants/tenant-a/elsewhere/templates/${TEMPLATE_KEY}`; blobs.set(`${base}/template.html`, HTML); blobs.set(`${base}/metadata.json`, JSON.stringify(METADATA)); - const { client } = makeRecordingClient(blobs); + const { client, requestedPaths } = makeRecordingClient(blobs); const unvalidated = { tenantId: 'tenant-a', environment: 'elsewhere', } as unknown as TenantContext; - const loaded = await makeStore(client).load(unvalidated, TEMPLATE_KEY); - assert.equal(loaded.templateHtml, HTML); + await expectInvalidTemplate( + () => makeStore(client).load(unvalidated, TEMPLATE_KEY), + requestedPaths, + ); }); + + const unsafeTenantIds: Array<[label: string, tenantId: string]> = [ + ['empty tenantId', ''], + ['parent traversal', '../tenant-b'], + ['nested slash', 'a/b'], + ['leading hyphen', '-acme'], + ['trailing hyphen', 'acme-'], + ['double dot segment', 'a..b'], + ]; + + for (const [label, tenantId] of unsafeTenantIds) { + it(`rejects ${label} with INVALID_TEMPLATE and makes no blob call`, async () => { + const { client, requestedPaths } = makeRecordingClient(new Map()); + await expectInvalidTemplate( + () => + makeStore(client).load( + { tenantId, environment: 'production' } as unknown as TenantContext, + TEMPLATE_KEY, + ), + requestedPaths, + ); + }); + } }); describe('BlobTemplateStore — unsafe template keys are rejected before storage access', () => { diff --git a/apps/api/src/templates/blob-template-store.ts b/apps/api/src/templates/blob-template-store.ts index 84cf1fd..11ea788 100644 --- a/apps/api/src/templates/blob-template-store.ts +++ b/apps/api/src/templates/blob-template-store.ts @@ -4,6 +4,7 @@ import { DefaultAzureCredential } from '@azure/identity'; import type { CompiledTemplate, TenantContext, + TenantEnvironment, TemplateSourceMetadata, } from '@singleton-sd/post-kit-types'; import { PostKitErrorCode, TEMPLATE_SCHEMA_VERSION } from '@singleton-sd/post-kit-types'; @@ -12,6 +13,9 @@ import type { TemplateStore } from './template-store'; /** Allowlist regex for safe template keys — alphanumeric, dots, hyphens, underscores only. */ const SAFE_TEMPLATE_KEY = /^[a-zA-Z0-9._-]+$/; +/** Matches publisher `assertSafeTenantId` — single path segment, no slashes or `..`. */ +const SAFE_TENANT_SEGMENT = /^[a-zA-Z0-9]([a-zA-Z0-9-]*[a-zA-Z0-9])?$/; +const VALID_ENVIRONMENTS = new Set(['development', 'staging', 'production']); /** * Error thrown by BlobTemplateStore on load failures. @@ -110,6 +114,8 @@ export class BlobTemplateStore implements TemplateStore { */ async load(tenant: TenantContext, templateKey: string): Promise { validateTemplateKey(templateKey); + validateTenantId(tenant.tenantId); + validateEnvironment(tenant.environment); const { tenantId, environment } = tenant; const basePath = `tenants/${tenantId}/${environment}/templates/${templateKey}`; @@ -158,6 +164,24 @@ function validateTemplateKey(templateKey: string): void { } } +function validateTenantId(tenantId: string): void { + if (!tenantId || !SAFE_TENANT_SEGMENT.test(tenantId) || tenantId.includes('..')) { + throw new TemplateStoreError( + `Invalid tenantId: "${tenantId}". Use alphanumeric characters and hyphens only (no path segments).`, + PostKitErrorCode.INVALID_TEMPLATE, + ); + } +} + +function validateEnvironment(environment: string): void { + if (!VALID_ENVIRONMENTS.has(environment as TenantEnvironment)) { + throw new TemplateStoreError( + `Invalid environment: "${environment}". Must be one of: development, staging, production.`, + PostKitErrorCode.INVALID_TEMPLATE, + ); + } +} + async function downloadBlob( containerClient: ReturnType, blobPath: string,