diff --git a/.github/workflows/run-tests.yaml b/.github/workflows/run-tests.yaml index b56841a83..3142fb930 100644 --- a/.github/workflows/run-tests.yaml +++ b/.github/workflows/run-tests.yaml @@ -124,6 +124,10 @@ jobs: env: {} - package: packages/csrf env: {} + - package: packages/12factor-env + env: {} + - package: packages/postmaster + env: {} env: PGHOST: localhost diff --git a/packages/12factor-env/__tests__/env.test.ts b/packages/12factor-env/__tests__/env.test.ts new file mode 100644 index 000000000..f4bc471c6 --- /dev/null +++ b/packages/12factor-env/__tests__/env.test.ts @@ -0,0 +1,220 @@ +import { writeFileSync, unlinkSync, mkdirSync, rmSync } from 'fs'; +import { join } from 'path'; +import { tmpdir } from 'os'; + +import { env, str, port, bool, host } from '../src'; + +describe('env', () => { + const ORIGINAL_ENV = { ...process.env }; + const testDir = join(tmpdir(), 'env-test-' + process.pid); + + beforeAll(() => { + mkdirSync(testDir, { recursive: true }); + }); + + afterAll(() => { + try { + rmSync(testDir, { recursive: true }); + } catch { + // ignore cleanup errors + } + }); + + afterEach(() => { + // Remove any env var added during the test + for (const key of Object.keys(process.env)) { + if (!(key in ORIGINAL_ENV)) { + delete process.env[key]; + } + } + // Restore original values + Object.assign(process.env, ORIGINAL_ENV); + }); + + describe('_FILE secret detection', () => { + it('should discover secrets via _FILE suffix pattern in original env', () => { + const secretPath = join(testDir, 'api-secret'); + writeFileSync(secretPath, 'super-secret-value'); + + try { + // API_KEY_FILE is in env but API_KEY is not in vars + // This tests the fix: secretEnv should use inputEnv, not varEnv + const result = env( + { + API_KEY_FILE: secretPath, + PORT: '3000' + }, + { API_KEY: str() }, + { PORT: port() } + ); + + expect(result.API_KEY).toBe('super-secret-value'); + expect(result.PORT).toBe(3000); + } finally { + unlinkSync(secretPath); + } + }); + }); + + describe('access to vars', () => { + it('should allow accessing vars without ReferenceError', () => { + const result = env( + { + MAILGUN_KEY: 'test-key', + MAILGUN_DOMAIN: 'mg.example.com' + }, + { MAILGUN_KEY: str() }, + { MAILGUN_DOMAIN: str() } + ); + + expect(result.MAILGUN_KEY).toBe('test-key'); + expect(result.MAILGUN_DOMAIN).toBe('mg.example.com'); + }); + }); + + describe('precedence', () => { + it('should prefer file secret over plain env when both exist', () => { + const secretPath = join(testDir, 'api-key-secret'); + writeFileSync(secretPath, 'from-file'); + + try { + const result = env( + { + API_KEY: 'from-env', + API_KEY_FILE: secretPath + }, + { API_KEY: str() }, + {} + ); + + expect(result.API_KEY).toBe('from-file'); + } finally { + unlinkSync(secretPath); + } + }); + + it('should use plain env when no file secret exists', () => { + const result = env( + { API_KEY: 'from-env' }, + { API_KEY: str() }, + {} + ); + + expect(result.API_KEY).toBe('from-env'); + }); + }); + + describe('Kubernetes secretKeyRef style', () => { + it('should allow secret set directly in env (no file)', () => { + const result = env( + { + DATABASE_PASSWORD: 'k8s-secret-value', + DATABASE_HOST: 'localhost' + }, + { DATABASE_PASSWORD: str() }, + { DATABASE_HOST: str() } + ); + + expect(result.DATABASE_PASSWORD).toBe('k8s-secret-value'); + expect(result.DATABASE_HOST).toBe('localhost'); + }); + }); + + describe('validation', () => { + it('should throw for missing required secret', () => { + expect(() => { + env( + { PORT: '3000' }, + { API_KEY: str() }, + { PORT: port() } + ); + }).toThrow(/API_KEY/); + }); + + it('should use default values for optional vars', () => { + const result = env( + { API_KEY: 'test-key' }, + { API_KEY: str() }, + { + PORT: port({ default: 8080 }), + DEBUG: bool({ default: false }) + } + ); + + expect(result.API_KEY).toBe('test-key'); + expect(result.PORT).toBe(8080); + expect(result.DEBUG).toBe(false); + }); + }); + + describe('ENV_SECRETS_PATH resolution', () => { + it('A5: should read secrets from ENV_SECRETS_PATH directory', () => { + // Write file named 'API_KEY' (not API_KEY_FILE) to temp dir + const secretPath = join(testDir, 'API_KEY'); + writeFileSync(secretPath, 'secret-from-path'); + + // Set ENV_SECRETS_PATH - now works without module reload thanks to lazy getSecretsPath() + process.env.ENV_SECRETS_PATH = testDir; + + try { + const result = env( + {}, // No API_KEY_FILE, no API_KEY in env + { API_KEY: str() }, + {} + ); + expect(result.API_KEY).toBe('secret-from-path'); + } finally { + unlinkSync(secretPath); + } + }); + }); + + describe('fallback behavior', () => { + it('B1: _FILE present but file missing + env secret present → fallback', () => { + const result = env( + { + API_KEY: 'fallback-value', + API_KEY_FILE: '/nonexistent/path' + }, + { API_KEY: str() }, + {} + ); + expect(result.API_KEY).toBe('fallback-value'); + }); + + it('B2: _FILE present but file missing + env secret missing → throw', () => { + expect(() => { + env( + { API_KEY_FILE: '/nonexistent/path' }, + { API_KEY: str() }, + {} + ); + }).toThrow(/API_KEY/); + }); + }); + + describe('validation errors', () => { + it('B3: Invalid optional var format (host validator) → throw', () => { + expect(() => { + env( + { + API_KEY: 'test-key', + MAILGUN_DOMAIN: 'not a valid host!!!' + }, + { API_KEY: str() }, + { MAILGUN_DOMAIN: host() } + ); + }).toThrow(/MAILGUN_DOMAIN/); + }); + + it('B4: Invalid required secret format (port validator) → throw', () => { + expect(() => { + env( + { DB_PORT: 'not-a-number' }, + { DB_PORT: port() }, + {} + ); + }).toThrow(/DB_PORT/); + }); + }); +}); diff --git a/packages/12factor-env/jest.config.js b/packages/12factor-env/jest.config.js new file mode 100644 index 000000000..e301e43aa --- /dev/null +++ b/packages/12factor-env/jest.config.js @@ -0,0 +1,18 @@ +/** @type {import('ts-jest').JestConfigWithTsJest} */ +module.exports = { + preset: 'ts-jest', + testEnvironment: 'node', + transform: { + '^.+\\.tsx?$': [ + 'ts-jest', + { + babelConfig: false, + tsconfig: 'tsconfig.json' + } + ] + }, + transformIgnorePatterns: ['/node_modules/*'], + testRegex: '(/__tests__/.*|(\\.|/)(test|spec))\\.(jsx?|tsx?)$', + moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json', 'node'], + modulePathIgnorePatterns: ['dist/*'] +}; diff --git a/packages/12factor-env/src/index.ts b/packages/12factor-env/src/index.ts index 6f196e0d5..07226da47 100644 --- a/packages/12factor-env/src/index.ts +++ b/packages/12factor-env/src/index.ts @@ -46,15 +46,16 @@ const cleanEnv = >>( }; /** - * Default path for secret files (Docker/Kubernetes secrets) + * Get the secrets path lazily to allow ENV_SECRETS_PATH changes at runtime */ -const ENV_SECRETS_PATH = process.env.ENV_SECRETS_PATH ?? '/run/secrets/'; +const getSecretsPath = (): string => + process.env.ENV_SECRETS_PATH ?? '/run/secrets/'; /** * Resolve the full path to a secret file */ const secretPath = (name: string): string => - name.startsWith('/') ? name : resolve(join(ENV_SECRETS_PATH, name)); + name.startsWith('/') ? name : resolve(join(getSecretsPath(), name)); /** * Read a secret from a file @@ -140,13 +141,13 @@ const env = ( const varEnv = cleanEnv(inputEnv, vars); // Read secrets from files - const _secrets = secretEnv(varEnv as unknown as Record, secrets); + const _secrets = secretEnv(inputEnv, secrets); // Second pass: validate secrets with file values merged in // Include inputEnv first so env vars (e.g., Kubernetes secretKeyRef) are available, // then varEnv overrides, then file-based secrets have highest priority const mergedEnv = { ...inputEnv, ...varEnv, ..._secrets } as unknown as Record; - return cleanEnv(mergedEnv, secrets) as unknown as CleanedEnv; + return cleanEnv(mergedEnv, { ...secrets, ...vars }) as unknown as CleanedEnv; }; export { @@ -155,6 +156,7 @@ export { secret, getSecret, secretPath, + getSecretsPath, // Re-export from envalid cleanEnv, makeValidator, diff --git a/packages/postmaster/__tests__/send.test.ts b/packages/postmaster/__tests__/send.test.ts new file mode 100644 index 000000000..d4dc78a4b --- /dev/null +++ b/packages/postmaster/__tests__/send.test.ts @@ -0,0 +1,223 @@ +import { writeFileSync, unlinkSync, mkdirSync, rmSync } from 'fs'; +import { join } from 'path'; +import { tmpdir } from 'os'; + +import { send, resetClient } from '../src'; + +// Enhanced mock to capture actual arguments passed to messages.create +const mockCreate = jest.fn().mockResolvedValue({ id: 'mock-message-id' }); + +jest.mock('mailgun.js', () => { + return jest.fn().mockImplementation(() => ({ + client: jest.fn().mockReturnValue({ + messages: { create: mockCreate } + }) + })); +}); + +describe('send', () => { + const ORIGINAL_ENV = { ...process.env }; + const testDir = join(tmpdir(), 'postmaster-test-' + process.pid); + + beforeAll(() => { + mkdirSync(testDir, { recursive: true }); + }); + + afterAll(() => { + try { + rmSync(testDir, { recursive: true }); + } catch { + // ignore cleanup errors + } + }); + + beforeEach(() => { + mockCreate.mockClear(); + }); + + afterEach(() => { + resetClient(); + // Remove any env var added during the test + for (const key of Object.keys(process.env)) { + if (!(key in ORIGINAL_ENV)) { + delete process.env[key]; + } + } + // Restore original values + Object.assign(process.env, ORIGINAL_ENV); + }); + + it('should not throw ReferenceError when MAILGUN_DOMAIN is set in env', async () => { + process.env.MAILGUN_KEY = 'test-api-key'; + process.env.MAILGUN_DOMAIN = 'mg.example.com'; + process.env.MAILGUN_FROM = 'sender@example.com'; + process.env.MAILGUN_REPLY = 'reply@example.com'; + + // This should not throw ReferenceError when accessing vars + // The bug was that secretEnv used varEnv instead of inputEnv + await expect( + send({ + to: 'recipient@example.com', + subject: 'Test Subject', + text: 'Test body' + }) + ).resolves.not.toThrow(); + }); + + it('should throw error when "to" is missing', async () => { + process.env.MAILGUN_KEY = 'test-api-key'; + process.env.MAILGUN_DOMAIN = 'mg.example.com'; + process.env.MAILGUN_FROM = 'sender@example.com'; + process.env.MAILGUN_REPLY = 'reply@example.com'; + + await expect( + send({ + to: '', + subject: 'Test', + text: 'Test' + }) + ).rejects.toThrow('Missing "to"'); + }); + + it('should throw error when "subject" is missing', async () => { + process.env.MAILGUN_KEY = 'test-api-key'; + process.env.MAILGUN_DOMAIN = 'mg.example.com'; + process.env.MAILGUN_FROM = 'sender@example.com'; + process.env.MAILGUN_REPLY = 'reply@example.com'; + + await expect( + send({ + to: 'test@example.com', + subject: '', + text: 'Test' + }) + ).rejects.toThrow('Missing "subject"'); + }); + + it('should throw error when both "html" and "text" are missing', async () => { + process.env.MAILGUN_KEY = 'test-api-key'; + process.env.MAILGUN_DOMAIN = 'mg.example.com'; + process.env.MAILGUN_FROM = 'sender@example.com'; + process.env.MAILGUN_REPLY = 'reply@example.com'; + + await expect( + send({ + to: 'test@example.com', + subject: 'Test' + }) + ).rejects.toThrow('Missing "html" or "text"'); + }); + + it('should send email when MAILGUN_DEV_EMAIL is set', async () => { + process.env.MAILGUN_KEY = 'test-api-key'; + process.env.MAILGUN_DOMAIN = 'mg.example.com'; + process.env.MAILGUN_FROM = 'sender@example.com'; + process.env.MAILGUN_REPLY = 'reply@example.com'; + process.env.MAILGUN_DEV_EMAIL = 'dev@example.com'; + + await expect( + send({ + to: 'recipient@example.com', + subject: 'Test', + text: 'Test body' + }) + ).resolves.not.toThrow(); + }); + + describe('file-based secrets', () => { + it('C1: MAILGUN_KEY_FILE only (no MAILGUN_KEY) → send succeeds', async () => { + const keyPath = join(testDir, 'mailgun-key'); + writeFileSync(keyPath, 'file-api-key'); + + process.env.MAILGUN_KEY_FILE = keyPath; + process.env.MAILGUN_DOMAIN = 'mg.example.com'; + process.env.MAILGUN_FROM = 'sender@example.com'; + process.env.MAILGUN_REPLY = 'reply@example.com'; + // No MAILGUN_KEY set directly + + await expect( + send({ to: 'test@example.com', subject: 'Test', text: 'Body' }) + ).resolves.not.toThrow(); + + unlinkSync(keyPath); + }); + }); + + describe('resetClient behavior', () => { + it('C2: resetClient() forces env re-read', async () => { + process.env.MAILGUN_KEY = 'key1'; + process.env.MAILGUN_DOMAIN = 'domain-a.com'; + process.env.MAILGUN_FROM = 'sender@example.com'; + process.env.MAILGUN_REPLY = 'reply@example.com'; + + await send({ to: 'test@example.com', subject: 'Test', text: 'Body' }); + const firstDomain = mockCreate.mock.calls[0][0]; // Capture domain arg + + // Change env and reset + process.env.MAILGUN_DOMAIN = 'domain-b.com'; + resetClient(); + + await send({ to: 'test@example.com', subject: 'Test', text: 'Body' }); + const secondDomain = mockCreate.mock.calls[1][0]; + + expect(firstDomain).toBe('domain-a.com'); + expect(secondDomain).toBe('domain-b.com'); + }); + }); + + describe('missing configuration errors', () => { + it('D1: Missing MAILGUN_DOMAIN → throw', async () => { + process.env.MAILGUN_KEY = 'test-key'; + delete process.env.MAILGUN_DOMAIN; + process.env.MAILGUN_FROM = 'sender@example.com'; + process.env.MAILGUN_REPLY = 'reply@example.com'; + + await expect( + send({ to: 'test@example.com', subject: 'Test', text: 'Body' }) + ).rejects.toThrow(/MAILGUN_DOMAIN/); + }); + + it('D2: Missing MAILGUN_KEY → throw', async () => { + delete process.env.MAILGUN_KEY; + delete process.env.MAILGUN_KEY_FILE; + process.env.MAILGUN_DOMAIN = 'mg.example.com'; + process.env.MAILGUN_FROM = 'sender@example.com'; + process.env.MAILGUN_REPLY = 'reply@example.com'; + + await expect( + send({ to: 'test@example.com', subject: 'Test', text: 'Body' }) + ).rejects.toThrow(/MAILGUN_KEY/); + }); + }); + + describe('API error propagation', () => { + it('D3: Mailgun client rejects → error propagates', async () => { + mockCreate.mockRejectedValueOnce(new Error('Mailgun API error')); + + process.env.MAILGUN_KEY = 'test-key'; + process.env.MAILGUN_DOMAIN = 'mg.example.com'; + process.env.MAILGUN_FROM = 'sender@example.com'; + process.env.MAILGUN_REPLY = 'reply@example.com'; + + await expect( + send({ to: 'test@example.com', subject: 'Test', text: 'Body' }) + ).rejects.toThrow('Mailgun API error'); + }); + }); + + describe('dev email rewriting', () => { + it('D4: MAILGUN_DEV_EMAIL rewrites recipient deterministically', async () => { + process.env.MAILGUN_KEY = 'test-key'; + process.env.MAILGUN_DOMAIN = 'mg.example.com'; + process.env.MAILGUN_FROM = 'sender@example.com'; + process.env.MAILGUN_REPLY = 'reply@example.com'; + process.env.MAILGUN_DEV_EMAIL = 'dev@example.com'; + + await send({ to: 'user@foo.com', subject: 'Test', text: 'Body' }); + + // Verify the 'to' field passed to messages.create + const createCall = mockCreate.mock.calls[mockCreate.mock.calls.length - 1]; + expect(createCall[1].to).toContain('dev+user_at_foo.com@example.com'); + }); + }); +});