From fe0bd61f472f4dfa85e1c809048b0281c76c8f57 Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Thu, 13 Aug 2026 20:46:03 +0000 Subject: [PATCH 1/4] feat(12factor-env): opt-in dotenv support via dotenv()/parseDotenv --- packages/12factor-env/README.md | 33 ++++++++++ packages/12factor-env/__tests__/env.test.ts | 71 +++++++++++++++++++++ packages/12factor-env/src/index.ts | 59 +++++++++++++++++ 3 files changed, 163 insertions(+) diff --git a/packages/12factor-env/README.md b/packages/12factor-env/README.md index 75504b8624..38b5b90d97 100644 --- a/packages/12factor-env/README.md +++ b/packages/12factor-env/README.md @@ -115,6 +115,39 @@ parseEnvBoolean('YES'); // true parseEnvNumber('42'); // 42 ``` +## Opt-in dotenv support + +The 12-factor rule is **environment first, `.env` as a local-dev convenience**. +`dotenv()` builds an environment record where a local `.env` fills gaps but real +environment variables always win. It never mutates `process.env`, a missing file +is not an error, and nothing changes unless you call it — existing `env()` usage +is unaffected. + +```ts +import { env, dotenv, str, port } from '12factor-env'; + +const config = env( + dotenv(), // process.env, backed by ./.env when present + { DATABASE_URL: str() }, + { PORT: port({ default: 3000 }) } +); +``` + +Options: + +```ts +dotenv(); // /.env merged under process.env +dotenv({ cwd: projectDir }); // look for .env in another directory +dotenv({ file: '.env.local' }); // different file name under cwd +dotenv({ path: '/etc/app/config.env' }); // explicit file path +dotenv({ environment: {} }); // file only, ignore process.env +dotenv({ override: true }); // file values win over the environment +``` + +Parsing uses Node's built-in dotenv parser (`util.parseEnv`), which handles +comments, quoting, and `export` prefixes. The raw parser is exported as +`parseDotenv(source)` if you need it directly. + ## Validators All validators from [envalid](https://github.com/af/envalid) are re-exported: diff --git a/packages/12factor-env/__tests__/env.test.ts b/packages/12factor-env/__tests__/env.test.ts index d649e2cac7..ef1c93d5e6 100644 --- a/packages/12factor-env/__tests__/env.test.ts +++ b/packages/12factor-env/__tests__/env.test.ts @@ -1,7 +1,12 @@ +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + import { bool, boolish, devDefault, + dotenv, env, getNodeEnv, getStrictEnvMode, @@ -9,6 +14,7 @@ import { isDevelopment, isProduction, isTest, + parseDotenv, parseEnvBoolean, parseEnvList, parseEnvNumber, @@ -297,4 +303,69 @@ describe('env', () => { expect(getStrictEnvMode({ STRICT_ENV: 'THROW' })).toBe('throw'); }); }); + + describe('dotenv', () => { + let dir: string; + + beforeEach(() => { + dir = mkdtempSync(path.join(os.tmpdir(), '12factor-env-')); + }); + + afterEach(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + it('parses dotenv source with parseDotenv', () => { + const parsed = parseDotenv('A=1\n# comment\nB="two words"\n'); + expect(parsed).toEqual({ A: '1', B: 'two words' }); + }); + + it('merges .env values under the environment (environment wins)', () => { + writeFileSync(path.join(dir, '.env'), 'FROM_FILE=file\nSHARED=file\n'); + const merged = dotenv({ cwd: dir, environment: { SHARED: 'env', FROM_ENV: 'env' } }); + expect(merged).toEqual({ FROM_FILE: 'file', SHARED: 'env', FROM_ENV: 'env' }); + }); + + it('lets file values win when override is true', () => { + writeFileSync(path.join(dir, '.env'), 'SHARED=file\n'); + const merged = dotenv({ cwd: dir, environment: { SHARED: 'env' }, override: true }); + expect(merged.SHARED).toBe('file'); + }); + + it('returns the environment unchanged when the file is missing', () => { + const merged = dotenv({ cwd: dir, environment: { ONLY: 'env' } }); + expect(merged).toEqual({ ONLY: 'env' }); + }); + + it('resolves an explicit path over cwd/file', () => { + const custom = path.join(dir, 'custom.env'); + writeFileSync(custom, 'CUSTOM=yes\n'); + const merged = dotenv({ path: custom, cwd: '/nonexistent', environment: {} }); + expect(merged).toEqual({ CUSTOM: 'yes' }); + }); + + it('resolves a custom file name under cwd', () => { + writeFileSync(path.join(dir, '.env.local'), 'LOCAL=yes\n'); + const merged = dotenv({ cwd: dir, file: '.env.local', environment: {} }); + expect(merged).toEqual({ LOCAL: 'yes' }); + }); + + it('never mutates process.env or the provided environment', () => { + writeFileSync(path.join(dir, '.env'), 'DOTENV_MUTATION_CHECK=file\n'); + const provided = { KEEP: 'env' }; + dotenv({ cwd: dir, environment: provided }); + dotenv({ cwd: dir }); + expect(provided).toEqual({ KEEP: 'env' }); + expect(process.env.DOTENV_MUTATION_CHECK).toBeUndefined(); + }); + + it('composes with env() for validation', () => { + writeFileSync(path.join(dir, '.env'), 'DATABASE_URL=postgres://localhost/dev\n'); + const config = env( + dotenv({ cwd: dir, environment: {} }), + { DATABASE_URL: str() } + ); + expect(config.DATABASE_URL).toBe('postgres://localhost/dev'); + }); + }); }); diff --git a/packages/12factor-env/src/index.ts b/packages/12factor-env/src/index.ts index dbfaeefe70..1180525666 100644 --- a/packages/12factor-env/src/index.ts +++ b/packages/12factor-env/src/index.ts @@ -1,3 +1,7 @@ +import { readFileSync } from 'node:fs'; +import path from 'node:path'; +import { parseEnv as parseEnvSource } from 'node:util'; + import type { CleanedEnv, CleanOptions, Spec,ValidatorSpec } from 'envalid'; import { bool, @@ -191,6 +195,61 @@ const boolish = makeValidator((value: string) => { return parseEnvBoolean(String(raw)) ?? false; }); +// ── Opt-in dotenv support ──────────────────────────────────────────────────── +// +// The 12-factor rule is "environment first, `.env` as a local-dev convenience": +// a container gets its configuration from the process environment, while a +// developer's project folder may carry a `.env`. `dotenv()` produces an +// environment record that honors that rule — file values fill gaps, real +// environment variables always win (unless `override` is set). It is a pure +// input builder for `env()`/`cleanEnv`; it never mutates `process.env`. + +export type DotenvOptions = { + /** Explicit path to the env file. Takes precedence over `cwd`/`file`. */ + path?: string; + /** Directory to resolve `file` in. Default: `process.cwd()`. */ + cwd?: string; + /** File name resolved under `cwd`. Default: `.env`. */ + file?: string; + /** Base environment the file is merged into. Default: `process.env`. */ + environment?: Record; + /** When true, file values win over the base environment. Default: false. */ + override?: boolean; +}; + +/** Parse dotenv-format source text into a plain record (no interpolation). */ +export const parseDotenv = (source: string): Record => + parseEnvSource(source) as Record; + +/** + * Read an env file and merge it with an environment record. A missing file is + * not an error — the base environment is returned unchanged, so the same code + * path works in a container (no file) and in local dev (file present). + */ +export const dotenv = ( + options: DotenvOptions = {} +): Record => { + const { + environment = process.env, + override = false, + cwd = process.cwd(), + file = '.env' + } = options; + const filePath = options.path ?? path.join(cwd, file); + + let source: string; + try { + source = readFileSync(filePath, 'utf8'); + } catch { + return { ...environment }; + } + + const fileVars = parseDotenv(source); + return override + ? { ...environment, ...fileVars } + : { ...fileVars, ...environment }; +}; + // Type for specs object type Specs = Record>; From 649e74db512ddad1be4470ae51cac7de263dfc27 Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Thu, 13 Aug 2026 20:56:23 +0000 Subject: [PATCH 2/4] refactor(12factor-env): move dotenv to a node-only entry so the main entry stays bundler-safe --- packages/12factor-env/README.md | 22 ++++++- packages/12factor-env/__tests__/env.test.ts | 16 ++++- packages/12factor-env/src/dotenv.ts | 69 +++++++++++++++++++++ packages/12factor-env/src/index.ts | 59 ------------------ 4 files changed, 102 insertions(+), 64 deletions(-) create mode 100644 packages/12factor-env/src/dotenv.ts diff --git a/packages/12factor-env/README.md b/packages/12factor-env/README.md index 38b5b90d97..9f7155904d 100644 --- a/packages/12factor-env/README.md +++ b/packages/12factor-env/README.md @@ -115,7 +115,7 @@ parseEnvBoolean('YES'); // true parseEnvNumber('42'); // 42 ``` -## Opt-in dotenv support +## Opt-in dotenv support (`12factor-env/dotenv`) The 12-factor rule is **environment first, `.env` as a local-dev convenience**. `dotenv()` builds an environment record where a local `.env` fills gaps but real @@ -123,8 +123,13 @@ environment variables always win. It never mutates `process.env`, a missing file is not an error, and nothing changes unless you call it — existing `env()` usage is unaffected. +It lives in its own **node-only** entry point, because it imports `node:fs`. The +main `12factor-env` entry stays free of node builtins so it can be bundled for a +browser, an Electron renderer, or a Next.js client component: + ```ts -import { env, dotenv, str, port } from '12factor-env'; +import { env, str, port } from '12factor-env'; +import { dotenv } from '12factor-env/dotenv'; // node only const config = env( dotenv(), // process.env, backed by ./.env when present @@ -133,6 +138,19 @@ const config = env( ); ``` +In a client/renderer bundle, keep importing only from `12factor-env` and pass +whatever environment record the framework gives you (`process.env` as inlined by +Next/Vite, or values the main process forwarded over IPC) — `env()` validates any +record, so nothing needs to read a file: + +```ts +import { env, str } from '12factor-env'; + +const config = env({ NEXT_PUBLIC_API_URL: process.env.NEXT_PUBLIC_API_URL }, { + NEXT_PUBLIC_API_URL: str() +}); +``` + Options: ```ts diff --git a/packages/12factor-env/__tests__/env.test.ts b/packages/12factor-env/__tests__/env.test.ts index ef1c93d5e6..4c6ea7f81e 100644 --- a/packages/12factor-env/__tests__/env.test.ts +++ b/packages/12factor-env/__tests__/env.test.ts @@ -1,4 +1,4 @@ -import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import os from 'node:os'; import path from 'node:path'; @@ -6,7 +6,6 @@ import { bool, boolish, devDefault, - dotenv, env, getNodeEnv, getStrictEnvMode, @@ -14,7 +13,6 @@ import { isDevelopment, isProduction, isTest, - parseDotenv, parseEnvBoolean, parseEnvList, parseEnvNumber, @@ -23,6 +21,7 @@ import { str, url, withDefault} from '../src'; +import { dotenv, parseDotenv } from '../src/dotenv'; describe('env', () => { const ORIGINAL_ENV = { ...process.env }; @@ -359,6 +358,17 @@ describe('env', () => { expect(process.env.DOTENV_MUTATION_CHECK).toBeUndefined(); }); + it('is not reachable from the main entry, which stays browser-safe', () => { + const main = require('../src') as Record; + expect(main.dotenv).toBeUndefined(); + expect(main.parseDotenv).toBeUndefined(); + + // The main entry is bundled into browsers/Electron renderers/Next client + // components; a node builtin import there breaks those bundles. + const source = readFileSync(path.join(__dirname, '../src/index.ts'), 'utf8'); + expect(source).not.toMatch(/from '(node:|fs|path)/); + }); + it('composes with env() for validation', () => { writeFileSync(path.join(dir, '.env'), 'DATABASE_URL=postgres://localhost/dev\n'); const config = env( diff --git a/packages/12factor-env/src/dotenv.ts b/packages/12factor-env/src/dotenv.ts new file mode 100644 index 0000000000..001afe6193 --- /dev/null +++ b/packages/12factor-env/src/dotenv.ts @@ -0,0 +1,69 @@ +// ── Opt-in dotenv support (node-only entry point) ──────────────────────────── +// +// This module is deliberately NOT re-exported from `12factor-env`'s main entry: +// it imports node builtins (`node:fs`, `node:util`), and the main entry must +// stay dependency-pure so it can be bundled for a browser, an Electron +// renderer, or a Next.js client component. Import it explicitly from code that +// runs in node: +// +// import { dotenv } from '12factor-env/dotenv'; +// import { env, str } from '12factor-env'; +// +// const config = env(dotenv(), { DATABASE_URL: str() }); +// +// The 12-factor rule is "environment first, `.env` as a local-dev convenience": +// a container gets its configuration from the process environment, while a +// developer's project folder may carry a `.env`. `dotenv()` produces an +// environment record honoring that rule — file values fill gaps, real +// environment variables win (unless `override` is set). It is a pure input +// builder for `env()`/`cleanEnv`; it never mutates `process.env`. + +import { readFileSync } from 'node:fs'; +import path from 'node:path'; +import { parseEnv as parseEnvSource } from 'node:util'; + +export type DotenvOptions = { + /** Explicit path to the env file. Takes precedence over `cwd`/`file`. */ + path?: string; + /** Directory to resolve `file` in. Default: `process.cwd()`. */ + cwd?: string; + /** File name resolved under `cwd`. Default: `.env`. */ + file?: string; + /** Base environment the file is merged into. Default: `process.env`. */ + environment?: Record; + /** When true, file values win over the base environment. Default: false. */ + override?: boolean; +}; + +/** Parse dotenv-format source text into a plain record (no interpolation). */ +export const parseDotenv = (source: string): Record => + parseEnvSource(source) as Record; + +/** + * Read an env file and merge it with an environment record. A missing file is + * not an error — the base environment is returned unchanged, so the same code + * path works in a container (no file) and in local dev (file present). + */ +export const dotenv = ( + options: DotenvOptions = {} +): Record => { + const { + environment = process.env, + override = false, + cwd = process.cwd(), + file = '.env' + } = options; + const filePath = options.path ?? path.join(cwd, file); + + let source: string; + try { + source = readFileSync(filePath, 'utf8'); + } catch { + return { ...environment }; + } + + const fileVars = parseDotenv(source); + return override + ? { ...environment, ...fileVars } + : { ...fileVars, ...environment }; +}; diff --git a/packages/12factor-env/src/index.ts b/packages/12factor-env/src/index.ts index 1180525666..dbfaeefe70 100644 --- a/packages/12factor-env/src/index.ts +++ b/packages/12factor-env/src/index.ts @@ -1,7 +1,3 @@ -import { readFileSync } from 'node:fs'; -import path from 'node:path'; -import { parseEnv as parseEnvSource } from 'node:util'; - import type { CleanedEnv, CleanOptions, Spec,ValidatorSpec } from 'envalid'; import { bool, @@ -195,61 +191,6 @@ const boolish = makeValidator((value: string) => { return parseEnvBoolean(String(raw)) ?? false; }); -// ── Opt-in dotenv support ──────────────────────────────────────────────────── -// -// The 12-factor rule is "environment first, `.env` as a local-dev convenience": -// a container gets its configuration from the process environment, while a -// developer's project folder may carry a `.env`. `dotenv()` produces an -// environment record that honors that rule — file values fill gaps, real -// environment variables always win (unless `override` is set). It is a pure -// input builder for `env()`/`cleanEnv`; it never mutates `process.env`. - -export type DotenvOptions = { - /** Explicit path to the env file. Takes precedence over `cwd`/`file`. */ - path?: string; - /** Directory to resolve `file` in. Default: `process.cwd()`. */ - cwd?: string; - /** File name resolved under `cwd`. Default: `.env`. */ - file?: string; - /** Base environment the file is merged into. Default: `process.env`. */ - environment?: Record; - /** When true, file values win over the base environment. Default: false. */ - override?: boolean; -}; - -/** Parse dotenv-format source text into a plain record (no interpolation). */ -export const parseDotenv = (source: string): Record => - parseEnvSource(source) as Record; - -/** - * Read an env file and merge it with an environment record. A missing file is - * not an error — the base environment is returned unchanged, so the same code - * path works in a container (no file) and in local dev (file present). - */ -export const dotenv = ( - options: DotenvOptions = {} -): Record => { - const { - environment = process.env, - override = false, - cwd = process.cwd(), - file = '.env' - } = options; - const filePath = options.path ?? path.join(cwd, file); - - let source: string; - try { - source = readFileSync(filePath, 'utf8'); - } catch { - return { ...environment }; - } - - const fileVars = parseDotenv(source); - return override - ? { ...environment, ...fileVars } - : { ...fileVars, ...environment }; -}; - // Type for specs object type Specs = Record>; From 17c8fab7c552c4faa1a49d5e64aa7c599f36932e Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Thu, 13 Aug 2026 21:13:12 +0000 Subject: [PATCH 3/4] feat(pi): injectable project context source with CONSTRUCTIVE_-prefixed env vars resolveProjectContext takes values (a record, a lookup fn, or process.env via fromEnvironment) instead of only a cwd, so a headless host - container, Job, CI - supplies credentials as environment variables with nothing written into the git clone. Passing a cwd still reads /.env, keeping desktop behavior unchanged. Injected names are prefixed (CONSTRUCTIVE_ACCESS_TOKEN etc.), bare names still resolve for project .env files. Drops pi's hand-rolled parser for 12factor-env/dotenv's parseDotenv. --- agentic/pi/README.md | 30 ++++++ agentic/pi/__tests__/context.test.ts | 147 +++++++++++++++++++++++++++ agentic/pi/package.json | 1 + agentic/pi/src/context.ts | 104 +++++++++++++------ agentic/pi/src/index.ts | 7 ++ pnpm-lock.yaml | 3 + 6 files changed, 261 insertions(+), 31 deletions(-) create mode 100644 agentic/pi/__tests__/context.test.ts diff --git a/agentic/pi/README.md b/agentic/pi/README.md index 0f80167310..f12854ee0f 100644 --- a/agentic/pi/README.md +++ b/agentic/pi/README.md @@ -38,6 +38,36 @@ const dbTools = createDbTools({ pi.use(dbTools); ``` +## Project context + +The tools need a bound database: an access key, a database id, and optionally endpoint/name pins. Where those values come from is the host's choice — `resolveProjectContext` takes values, not a directory: + +```ts +import { fromEnvironment, resolveProjectContext } from '@agentic-kit/pi'; + +// headless: container, Job, CI — nothing on disk, nothing to commit +await resolveProjectContext(fromEnvironment()); + +// local project: read /.env (what Desktop and the CLI do) +await resolveProjectContext(cwd); + +// anything else: a record, or a lookup function into a secret store +await resolveProjectContext((name) => vault.get(name)); +``` + +Injected variables carry a `CONSTRUCTIVE_` prefix; the bare names are also accepted (that is what the scaffolder writes into a project `.env`), with the prefixed spelling winning: + +| Variable | Required | Purpose | +| --- | --- | --- | +| `CONSTRUCTIVE_ACCESS_TOKEN` | yes | project data-plane key | +| `CONSTRUCTIVE_DATABASE_ID` | yes | bound database | +| `CONSTRUCTIVE_DATABASE_NAME` | no | derives the per-db data endpoint | +| `CONSTRUCTIVE_API_ENDPOINT` | no | data-plane api pin | +| `CONSTRUCTIVE_MODULES_ENDPOINT` | no | data-plane modules pin | +| `CONSTRUCTIVE_OWNER_ID` | no | owner fallback when the probe omits it | + +Endpoint pins from the source apply to the **data plane only**. The control plane (binding probe, schema resolution, blueprint/schema tools) always uses the host's `backendConfig()` and the account bearer, so an untrusted cloned project cannot redirect it. + ## Provisioning `provision_database` requests a database through the `requestDatabase` mutation on the api endpoint. When the requested module set matches a cataloged preset, the backend claims a warm pre-baked database in seconds. Otherwise a background job provisions the database cold. The tool polls the provision ticket on the modules endpoint until the database and its owner bootstrap are complete. Then it writes the credentials to the project `.env` and returns. diff --git a/agentic/pi/__tests__/context.test.ts b/agentic/pi/__tests__/context.test.ts new file mode 100644 index 0000000000..97fdd7667e --- /dev/null +++ b/agentic/pi/__tests__/context.test.ts @@ -0,0 +1,147 @@ +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +jest.mock('../src/db-probe', () => ({ probeDatabase: jest.fn() })); +jest.mock('@constructive-io/sdk', () => ({ + api: { createClient: jest.fn() }, + auth: { createClient: jest.fn() }, + modules: { createClient: jest.fn(() => ({ kind: 'modules' })) }, +})); + +import { api } from '@constructive-io/sdk'; + +import { + CONTEXT_ENV_PREFIX, + fromEnvFile, + fromEnvironment, + resolveProjectContext, +} from '../src/context'; +import { probeDatabase } from '../src/db-probe'; +import { configureHost } from '../src/host'; + +const mockProbe = probeDatabase as jest.MockedFunction; +const mockCreateClient = api.createClient as jest.Mock; + +const schemaClient = { + schema: { + findMany: () => ({ + execute: async () => ({ + ok: true, + data: { schemas: { nodes: [{ id: 'schema-1', name: 'app_public' }] } }, + }), + }), + }, +}; + +let dir: string; + +beforeEach(() => { + dir = mkdtempSync(path.join(os.tmpdir(), 'pi-context-')); + mockProbe.mockResolvedValue({ outcome: 'found', name: 'myapp', ownerId: 'u1' }); + mockCreateClient.mockReturnValue(schemaClient); + configureHost({ + account: () => ({ userId: 'u1', accessToken: 'account-bearer' }), + backendConfig: () => ({ + apiEndpoint: 'http://api.localhost:3000/graphql', + modulesEndpoint: 'http://modules.localhost:3000/graphql', + }), + }); +}); + +afterEach(() => { + rmSync(dir, { recursive: true, force: true }); + jest.clearAllMocks(); +}); + +describe('resolveProjectContext sources', () => { + it('resolves from injected environment variables, no file needed', async () => { + const resolved = await resolveProjectContext( + fromEnvironment({ + [`${CONTEXT_ENV_PREFIX}ACCESS_TOKEN`]: 'project-key', + [`${CONTEXT_ENV_PREFIX}DATABASE_ID`]: 'db-1', + [`${CONTEXT_ENV_PREFIX}DATABASE_NAME`]: 'myapp', + }), + ); + + expect(resolved.context).toMatchObject({ + accessToken: 'project-key', + databaseId: 'db-1', + databaseName: 'myapp', + schemaId: 'schema-1', + dataEndpoint: 'http://api-myapp.localhost:3000/graphql', + }); + }); + + it('resolves from a project .env when given a cwd (unchanged host behavior)', async () => { + writeFileSync(path.join(dir, '.env'), 'ACCESS_TOKEN=file-key\nDATABASE_ID=db-file\n'); + + const resolved = await resolveProjectContext(dir); + + expect(resolved.context).toMatchObject({ accessToken: 'file-key', databaseId: 'db-file' }); + }); + + it('prefers the prefixed name over the bare one', async () => { + const resolved = await resolveProjectContext({ + ACCESS_TOKEN: 'bare', + [`${CONTEXT_ENV_PREFIX}ACCESS_TOKEN`]: 'prefixed', + DATABASE_ID: 'db-1', + }); + + expect(resolved.context?.accessToken).toBe('prefixed'); + }); + + it('accepts a lookup function as the source', async () => { + const values: Record = { + [`${CONTEXT_ENV_PREFIX}ACCESS_TOKEN`]: 'from-fn', + [`${CONTEXT_ENV_PREFIX}DATABASE_ID`]: 'db-1', + }; + + const resolved = await resolveProjectContext((name) => values[name]); + + expect(resolved.context?.accessToken).toBe('from-fn'); + }); + + it('pins the data-plane endpoints from the source, never the control plane', async () => { + const resolved = await resolveProjectContext({ + [`${CONTEXT_ENV_PREFIX}ACCESS_TOKEN`]: 'project-key', + [`${CONTEXT_ENV_PREFIX}DATABASE_ID`]: 'db-1', + [`${CONTEXT_ENV_PREFIX}API_ENDPOINT`]: 'http://api.evil.test/graphql', + }); + + expect(resolved.context?.apiEndpoint).toBe('http://api.evil.test/graphql'); + // control-plane clients + probe stay on the host-configured backend + expect(mockCreateClient).toHaveBeenCalledWith( + expect.objectContaining({ endpoint: 'http://api.localhost:3000/graphql' }), + ); + expect(mockProbe).toHaveBeenCalledWith( + expect.objectContaining({ endpoint: 'http://api.localhost:3000/graphql' }), + ); + }); + + it('reports no-env when a cwd has no .env', async () => { + const resolved = await resolveProjectContext(dir); + + expect(resolved.context).toBeNull(); + expect(resolved.code).toBe('no-env'); + }); + + it('reports missing-credentials when the source carries no credentials', async () => { + const resolved = await resolveProjectContext(fromEnvironment({ PATH: '/usr/bin' })); + + expect(resolved.context).toBeNull(); + expect(resolved.code).toBe('missing-credentials'); + }); +}); + +describe('fromEnvFile', () => { + it('parses the project .env, quotes and comments included', async () => { + writeFileSync(path.join(dir, '.env'), '# comment\nACCESS_TOKEN="quoted key"\nDATABASE_ID=db-1\n'); + + expect(await fromEnvFile(dir)).toEqual({ ACCESS_TOKEN: 'quoted key', DATABASE_ID: 'db-1' }); + }); + + it('returns null when the file is absent', async () => { + expect(await fromEnvFile(dir)).toBeNull(); + }); +}); diff --git a/agentic/pi/package.json b/agentic/pi/package.json index 75710317bf..e8b0f8fad9 100644 --- a/agentic/pi/package.json +++ b/agentic/pi/package.json @@ -29,6 +29,7 @@ "test:watch": "jest --watch" }, "dependencies": { + "12factor-env": "workspace:^", "@agentic-kit/harness": "workspace:^", "@constructive-io/graphql-query": "workspace:^", "@constructive-io/sdk": "workspace:^", diff --git a/agentic/pi/src/context.ts b/agentic/pi/src/context.ts index 3cd697f8eb..3d2575f70a 100644 --- a/agentic/pi/src/context.ts +++ b/agentic/pi/src/context.ts @@ -2,6 +2,7 @@ import { readFile } from 'node:fs/promises'; import path from 'node:path'; import { api, auth, modules } from '@constructive-io/sdk'; +import { parseDotenv } from '12factor-env/dotenv'; import { probeDatabase } from './db-probe'; import { DEFAULT_DATA_TOKEN_SKEW_MS, getHost } from './host'; @@ -41,24 +42,58 @@ export type ResolveResult = { code?: ProjectContextFailureCode; }; -function parseEnv(source: string): Record { - const env: Record = {}; - for (const rawLine of source.split('\n')) { - const line = rawLine.trim(); - if (!line || line.startsWith('#')) continue; - const eq = line.indexOf('='); - if (eq === -1) continue; - const key = line.slice(0, eq).trim(); - let value = line.slice(eq + 1).trim(); - if ( - (value.startsWith('"') && value.endsWith('"')) || - (value.startsWith("'") && value.endsWith("'")) - ) { - value = value.slice(1, -1); - } - env[key] = value; +// The project values pi needs, and where they come from. A scaffolded project +// folder carries them in `.env` (desktop); a headless host — a container, a +// Job, CI — injects them as real environment variables, so no credential is +// ever written into a git clone the agent could commit. Both lanes produce the +// same record, which is why the resolver takes VALUES, not a directory. +export const CONTEXT_ENV_KEYS = [ + 'ACCESS_TOKEN', + 'DATABASE_ID', + 'API_ENDPOINT', + 'MODULES_ENDPOINT', + 'DATABASE_NAME', + 'OWNER_ID', +] as const; + +export type ContextEnvKey = (typeof CONTEXT_ENV_KEYS)[number]; + +/** Prefix for injected variables: `ACCESS_TOKEN` is far too generic to claim in + * a shared process environment, `CONSTRUCTIVE_ACCESS_TOKEN` is not. Inside a + * project `.env` the bare names stay readable (and are what the scaffolder + * writes), so both spellings resolve, prefixed winning. */ +export const CONTEXT_ENV_PREFIX = 'CONSTRUCTIVE_'; + +/** Values keyed by name, or a lookup function (a host with a secret store). */ +export type ContextSource = + | Record + | ((name: string) => string | undefined); + +const lookup = (source: ContextSource): ((name: string) => string | undefined) => + typeof source === 'function' ? source : (name) => source[name]; + +const readContextValue = (source: ContextSource, key: ContextEnvKey): string | undefined => { + const get = lookup(source); + const value = get(`${CONTEXT_ENV_PREFIX}${key}`) ?? get(key); + return value ? value : undefined; +}; + +/** Use the process environment (or any injected record) as the source. */ +export function fromEnvironment( + environment: Record = process.env, +): ContextSource { + return environment; +} + +/** Read a project `.env` as the source. The file is authoritative here — the + * key in it belongs to whatever backend wrote it, so it is not merged under + * the ambient environment. Returns null when the file is absent. */ +export async function fromEnvFile(cwd: string): Promise { + try { + return parseDotenv(await readFile(path.join(cwd, '.env'), 'utf8')); + } catch { + return null; } - return env; } export type ResolveOptions = { @@ -71,14 +106,20 @@ export type ResolveOptions = { plane?: 'control' | 'data'; }; +/** + * Resolve the project context from injected values, or from a project folder. + * + * Passing a `cwd` string keeps the original behavior (read `/.env`) so + * existing hosts — Constructive Desktop, the confirm gate — are unchanged. + * Headless hosts pass values instead: `fromEnvironment()` for a container or + * Job, an explicit record for anything else. + */ export async function resolveProjectContext( - cwd: string, + input: string | ContextSource, options: ResolveOptions = {}, ): Promise { - let source: string; - try { - source = await readFile(path.join(cwd, '.env'), 'utf8'); - } catch { + const source = typeof input === 'string' ? await fromEnvFile(input) : input; + if (!source) { return { context: null, reason: @@ -87,20 +128,21 @@ export async function resolveProjectContext( }; } - const env = parseEnv(source); + const env = Object.fromEntries( + CONTEXT_ENV_KEYS.map((key) => [key, readContextValue(source, key)]), + ) as Record; const accessToken = env.ACCESS_TOKEN; const databaseId = env.DATABASE_ID; if (!accessToken || !databaseId) { return { context: null, - reason: - 'Project is not connected to a Constructive database yet (missing ACCESS_TOKEN/DATABASE_ID in .env). Provision the database first, then retry.', + reason: `Project is not connected to a Constructive database yet (missing ${CONTEXT_ENV_PREFIX}ACCESS_TOKEN/${CONTEXT_ENV_PREFIX}DATABASE_ID in the environment, or ACCESS_TOKEN/DATABASE_ID in .env). Provision the database first, then retry.`, code: 'missing-credentials', }; } - // A per-project .env pin wins for the DATA plane (the .env key belongs to + // A source-supplied pin wins for the DATA plane (the access key belongs to // whatever backend wrote it); otherwise fall back to the app's backend-config // store (environment-aware) so app + harness share one endpoint source. const host = getHost(); @@ -111,9 +153,9 @@ export async function resolveProjectContext( // The whole control plane (binding probe, schema resolution, blueprint/schema // tools) authenticates with the ACCOUNT bearer: the platform api rejects // per-database keys, so the project ACCESS_TOKEN can never act on metaschema - // surfaces. The .env key stays in the context for the data plane only. The - // bearer is only ever sent to the app-configured backend — never a - // .env-pinned endpoint, which an untrusted cloned project controls. + // surfaces. The source's key stays in the context for the data plane only. + // The bearer is only ever sent to the app-configured backend — never a + // source-pinned endpoint, which an untrusted cloned project controls. const controlApiEndpoint = backend?.apiEndpoint || DEFAULT_API_ENDPOINT; const controlModulesEndpoint = backend?.modulesEndpoint || DEFAULT_MODULES_ENDPOINT; const account = host.account(); @@ -131,7 +173,7 @@ export async function resolveProjectContext( const apiClient = api.createClient({ endpoint: controlApiEndpoint, headers: controlHeaders }); - // Always probe the bound database — the .env stamp proves the project WAS + // Always probe the bound database — the DATABASE_ID stamp proves it WAS // bound, never that the database still exists (backend refresh, deletion, // revoked key). The probe is the single source of binding health. const probe = await probeDatabase({ @@ -159,7 +201,7 @@ export async function resolveProjectContext( // never shown in the Schemas tab or written to by the agent. The project's // local blueprint survives account changes, so recovery is a reprovision // under the signed-in account. Owner truth comes from the probe (backend), - // not the .env stamp. Data-plane resolution skips this gate — see + // not the source's stamp. Data-plane resolution skips this gate — see // ResolveOptions. const sessionUserId = account?.userId; if ( diff --git a/agentic/pi/src/index.ts b/agentic/pi/src/index.ts index e7eeb83ec2..88dbfe892e 100644 --- a/agentic/pi/src/index.ts +++ b/agentic/pi/src/index.ts @@ -64,9 +64,16 @@ export { createConfirmGate, } from './confirm-gate'; export { + CONTEXT_ENV_KEYS, + CONTEXT_ENV_PREFIX, + type ContextEnvKey, + type ContextSource, deriveSubdomainEndpoint, + fromEnvFile, + fromEnvironment, type ModulesClient, type ProjectContext, + type ProjectContextFailureCode, resolveDataToken, resolveProjectContext, } from './context'; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ce55a54f7e..acb2c9ee50 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -225,6 +225,9 @@ importers: agentic/pi: dependencies: + 12factor-env: + specifier: workspace:^ + version: link:../../packages/12factor-env/dist '@agentic-kit/harness': specifier: workspace:^ version: link:../harness/dist From 5baba02258034e8e62522495704bfef1d6c065a2 Mon Sep 17 00:00:00 2001 From: Dan Lynch Date: Thu, 13 Aug 2026 21:26:18 +0000 Subject: [PATCH 4/4] feat(12factor-env): list/bounded num/duration/enumerated validators, cross-field checks, secret redaction --- packages/12factor-env/README.md | 80 ++++- .../12factor-env/__tests__/validators.test.ts | 326 ++++++++++++++++++ packages/12factor-env/src/checks.ts | 81 +++++ packages/12factor-env/src/index.ts | 126 +++++-- packages/12factor-env/src/redact.ts | 123 +++++++ packages/12factor-env/src/validators.ts | 195 +++++++++++ 6 files changed, 909 insertions(+), 22 deletions(-) create mode 100644 packages/12factor-env/__tests__/validators.test.ts create mode 100644 packages/12factor-env/src/checks.ts create mode 100644 packages/12factor-env/src/redact.ts create mode 100644 packages/12factor-env/src/validators.ts diff --git a/packages/12factor-env/README.md b/packages/12factor-env/README.md index 9f7155904d..d8794d296a 100644 --- a/packages/12factor-env/README.md +++ b/packages/12factor-env/README.md @@ -166,6 +166,20 @@ Parsing uses Node's built-in dotenv parser (`util.parseEnv`), which handles comments, quoting, and `export` prefixes. The raw parser is exported as `parseDotenv(source)` if you need it directly. +## The contract: unset may default, invalid always throws + +Two different things, deliberately: + +- **Unset** — resolves `default`/`devDefault`/`testDefault` if the spec has one, otherwise throws. +- **Set but invalid** — **always throws.** A value that is present and wrong is never + silently replaced by a default or a fallback, because a typo'd `AUTH_KINDS` degrading + to "allow nothing" (or a bad `PORT` degrading to 3000) is a security incident, not a + recovery. This is why `list()` treats "set, but yields no items" as an error rather + than returning `[]`. + +Errors from one `env()` call are reported **together**, including cross-field `checks`, +so an operator fixes one deployment instead of one variable per redeploy. + ## Validators All validators from [envalid](https://github.com/af/envalid) are re-exported: @@ -181,6 +195,67 @@ All validators from [envalid](https://github.com/af/envalid) are re-exported: | `email()` | Valid email address | | `json()` | JSON string (parsed) | +House validators add what consumers were otherwise hand-rolling after `env()`: + +| Validator | Description | +|-----------|-------------| +| `list()` | Separated list of trimmed, non-empty strings. `list({ separator: ':' })`, and `list({ choices: [...] as const })` checks membership **per item** and types the result as the union array | +| `num({ min, max, integer })` | Number with inclusive bounds (superset of envalid's `num`) | +| `int({ min, max })` | `num({ integer: true })` | +| `duration()` | Milliseconds, accepting `250`, `500ms`, `30s`, `5m`, `2h`, `1d` | +| `enumerated([...] as const)` / `oneOf` | `str({ choices })` with a discoverable name and literal-union typing | + +```ts +import { duration, enumerated, env, int, list, str } from '12factor-env'; + +const config = env(process.env, {}, { + AUTH_KINDS: list({ choices: ['api_key', 'jwt'] as const, default: ['api_key'] }), + CACHE_TTL: duration({ default: 30_000 }), + POOL_MAX: int({ min: 1, max: 64, default: 10 }), + CODEGEN_MODE: enumerated(['per-function', 'combined'] as const, { default: 'combined' }) +}); +``` + +## Cross-field checks + +Constraints *between* vars belong in the schema, not in an ad-hoc `if` after it — that +way they run against cleaned values and their failures join the same report: + +```ts +import { distinct, env, mutuallyExclusive, requiredWhen, str } from '12factor-env'; + +const config = env(process.env, { CONTROL_ROLE: str(), DATA_ROLE: str() }, {}, { + checks: [ + distinct(['CONTROL_ROLE', 'DATA_ROLE'], 'the control role needs BYPASSRLS; the data role must not have it'), + mutuallyExclusive(['AUTH_TOKEN', 'AUTH_TOKEN_FILE']), + requiredWhen('TLS_ENABLED', ['TLS_CERT', 'TLS_KEY']), + { vars: ['MIN', 'MAX'], check: (v) => v.MIN <= v.MAX, message: 'MIN must not exceed MAX' } + ] +}); +``` + +A check is **skipped** when any var it names already failed validation, so "both unset" +is not also reported as "both equal". + +## Secrets are not printed + +envalid quotes the offending value into its message (`Invalid url: "postgres://u:pw@h/db"`), +which puts passwords in crash logs. Anything passed as `env()`'s `secrets` argument — or +marked `str({ secret: true })` — is redacted instead: + +```ts +const config = env(process.env, { DATABASE_URL: url() }, { PORT: port({ default: 3000 }) }); + +config.DATABASE_URL // the real value +JSON.stringify(config) // {"DATABASE_URL":"[redacted]","PORT":3000} +console.log(config) // redacted too (util.inspect) + +// a malformed DATABASE_URL reports: 'Invalid url: "[redacted]" (value redacted, 42 chars)' +// a MISSING one still reports its desc, since there is no value to leak +``` + +Use `redactEnvError(err, [knownSecret])` when re-logging an error you built yourself. + ### Validator Options All validators accept options: @@ -196,13 +271,14 @@ const config = env(process.env, { ## API -### `env(inputEnv, secrets, vars)` +### `env(inputEnv, secrets, vars, options?)` Main function to validate environment variables. - `inputEnv` - The environment object (usually `process.env`) -- `secrets` - Required environment variables +- `secrets` - Required environment variables (treated as sensitive: redacted in errors and serialization) - `vars` - Optional environment variables +- `options.checks` - Constraints between vars, evaluated over the cleaned values ## Re-exports from envalid diff --git a/packages/12factor-env/__tests__/validators.test.ts b/packages/12factor-env/__tests__/validators.test.ts new file mode 100644 index 0000000000..3497f77716 --- /dev/null +++ b/packages/12factor-env/__tests__/validators.test.ts @@ -0,0 +1,326 @@ +import { inspect } from 'node:util'; + +import { + cleanEnv, + distinct, + duration, + enumerated, + env, + int, + list, + mutuallyExclusive, + num, + oneOf, + redactEnvError, + requiredWhen, + str, + url, + withDefault} from '../src'; + +describe('list()', () => { + it('splits, trims and drops empty entries', () => { + const config = cleanEnv({ KINDS: ' api_key , jwt ,, ' }, { KINDS: list() }); + expect(config.KINDS).toEqual(['api_key', 'jwt']); + }); + + it('errors when set but empty, instead of yielding []', () => { + // an allowlist that silently becomes [] is an allowlist that allows nothing + expect(() => cleanEnv({ KINDS: ' , ' }, { KINDS: list() })).toThrow(/KINDS: Empty list/); + expect(() => cleanEnv({ KINDS: '' }, { KINDS: list() })).toThrow(/KINDS: Empty list/); + }); + + it('enforces choices per item, not against the whole array', () => { + const spec = { KINDS: list({ choices: ['api_key', 'jwt'] as const }) }; + expect(cleanEnv({ KINDS: 'jwt,api_key' }, spec).KINDS).toEqual(['jwt', 'api_key']); + expect(() => cleanEnv({ KINDS: 'jwt,cookie' }, spec)).toThrow( + /KINDS: Invalid list value\(s\) "cookie" not in choices \[api_key, jwt\]/ + ); + }); + + it('supports a custom separator', () => { + const config = cleanEnv({ DIRS: '/usr/bin:/bin' }, { DIRS: list({ separator: ':' }) }); + expect(config.DIRS).toEqual(['/usr/bin', '/bin']); + }); + + it('accepts a typed default', () => { + const config = cleanEnv({}, { KINDS: withDefault(list, ['api_key']) }); + expect(config.KINDS).toEqual(['api_key']); + }); + + it('is usable through env(), where the second pass sees an array', () => { + const config = env({ KINDS: 'api_key,jwt' }, {}, { KINDS: list() }); + expect(config.KINDS).toEqual(['api_key', 'jwt']); + }); + + it('still throws for a missing required list', () => { + expect(() => cleanEnv({}, { KINDS: list() })).toThrow(/KINDS/); + }); +}); + +describe('num() bounds and int()', () => { + it('accepts plain numbers like envalid num', () => { + expect(cleanEnv({ N: '42' }, { N: num() }).N).toBe(42); + expect(cleanEnv({ N: '3.5' }, { N: num() }).N).toBe(3.5); + expect(() => cleanEnv({ N: 'abc' }, { N: num() })).toThrow(/N: Invalid number input/); + }); + + it('enforces min/max', () => { + expect(cleanEnv({ TTL: '0' }, { TTL: num({ min: 0 }) }).TTL).toBe(0); + expect(() => cleanEnv({ TTL: '-1' }, { TTL: num({ min: 0 }) })).toThrow( + /TTL: Expected a number >= 0/ + ); + expect(() => cleanEnv({ C: '65' }, { C: num({ min: 1, max: 64 }) })).toThrow( + /C: Expected a number <= 64/ + ); + }); + + it('int() rejects a non-integer count', () => { + expect(cleanEnv({ C: '8' }, { C: int({ min: 1 }) }).C).toBe(8); + expect(() => cleanEnv({ C: '1.5' }, { C: int() })).toThrow(/C: Expected an integer/); + }); + + it('resolves a default when unset (envalid returns defaults unvalidated)', () => { + expect(cleanEnv({}, { TTL: num({ min: 0, default: 30_000 }) }).TTL).toBe(30_000); + }); +}); + +describe('duration()', () => { + it('normalizes suffixed values to milliseconds', () => { + const spec = { D: duration() }; + expect(cleanEnv({ D: '250' }, spec).D).toBe(250); + expect(cleanEnv({ D: '500ms' }, spec).D).toBe(500); + expect(cleanEnv({ D: '30s' }, spec).D).toBe(30_000); + expect(cleanEnv({ D: '5m' }, spec).D).toBe(300_000); + expect(cleanEnv({ D: '2h' }, spec).D).toBe(7_200_000); + expect(cleanEnv({ D: '1d' }, spec).D).toBe(86_400_000); + }); + + it('rejects garbage and negatives', () => { + expect(() => cleanEnv({ D: '30 years' }, { D: duration() })).toThrow( + /D: Invalid duration input/ + ); + expect(() => cleanEnv({ D: '-5s' }, { D: duration() })).toThrow(/D: Expected a number >= 0/); + }); + + it('takes a plain millisecond default', () => { + expect(cleanEnv({}, { D: withDefault(duration, 30_000) }).D).toBe(30_000); + }); +}); + +describe('enumerated()', () => { + it('accepts a member and types the union', () => { + const mode = cleanEnv( + { MODE: 'combined' }, + { MODE: enumerated(['per-function', 'combined'] as const) } + ).MODE; + const typed: 'per-function' | 'combined' = mode; + expect(typed).toBe('combined'); + }); + + it('rejects a non-member', () => { + expect(() => + cleanEnv({ MODE: 'sideways' }, { MODE: enumerated(['per-function', 'combined'] as const) }) + ).toThrow(/MODE: Value "sideways" not in choices \[per-function, combined\]/); + }); + + it('oneOf is an alias', () => { + expect(oneOf).toBe(enumerated); + }); +}); + +describe('cross-field checks', () => { + it('reports a failed check in the same consolidated error', () => { + expect(() => + env( + { CONTROL_USER: 'proxy', UPSTREAM_USER: 'proxy' }, + {}, + { CONTROL_USER: str(), UPSTREAM_USER: str() }, + { + checks: [ + distinct( + ['CONTROL_USER', 'UPSTREAM_USER'], + 'must be different roles: the control-plane read needs BYPASSRLS and the data plane must not have it' + ) + ] + } + ) + ).toThrow(/CONTROL_USER, UPSTREAM_USER: must be different roles/); + }); + + it('passes when the constraint holds', () => { + const config = env( + { CONTROL_USER: 'proxy_control', UPSTREAM_USER: 'proxy_data' }, + {}, + { CONTROL_USER: str(), UPSTREAM_USER: str() }, + { checks: [distinct(['CONTROL_USER', 'UPSTREAM_USER'])] } + ); + expect(config.CONTROL_USER).toBe('proxy_control'); + }); + + it('is skipped when a var it names already failed (no "both equal" noise)', () => { + let error: Error | undefined; + try { + env( + {}, + {}, + { CONTROL_USER: str(), UPSTREAM_USER: str() }, + { checks: [distinct(['CONTROL_USER', 'UPSTREAM_USER'], 'must be different roles')] } + ); + } catch (err) { + error = err as Error; + } + expect(error?.message).toMatch(/CONTROL_USER/); + expect(error?.message).not.toMatch(/must be different roles/); + }); + + it('sees cleaned, coerced values', () => { + expect(() => + env( + {}, + {}, + { MIN: num({ default: 10 }), MAX: num({ default: 5 }) }, + { + checks: [ + { + vars: ['MIN', 'MAX'], + check: (v: { MIN: number; MAX: number }) => v.MIN <= v.MAX, + message: 'MIN must not exceed MAX' + } + ] + } + ) + ).toThrow(/MIN must not exceed MAX/); + }); + + it('mutuallyExclusive allows at most one', () => { + const specs = { A: str({ default: undefined }), B: str({ default: undefined }) }; + const options = { checks: [mutuallyExclusive(['A', 'B'])] }; + expect(env({ A: 'x' }, {}, specs, options).A).toBe('x'); + expect(() => env({ A: 'x', B: 'y' }, {}, specs, options)).toThrow( + /only one of A, B may be set/ + ); + }); + + it('requiredWhen turns a flag into requirements', () => { + const specs = { + TLS: str({ default: undefined }), + TLS_CERT: str({ default: undefined }) + }; + const options = { checks: [requiredWhen('TLS', ['TLS_CERT'])] }; + expect(env({}, {}, specs, options).TLS).toBeUndefined(); + expect(() => env({ TLS: '1' }, {}, specs, options)).toThrow( + /TLS_CERT are required when TLS is set/ + ); + }); + + it('a throwing check fails instead of escaping', () => { + expect(() => + env( + { A: 'x' }, + {}, + { A: str() }, + { + checks: [ + { + vars: ['A'], + check: () => { + throw new Error('boom'); + }, + message: 'check blew up' + } + ] + } + ) + ).toThrow(/A: check blew up/); + }); +}); + +describe('secret redaction', () => { + const SECRET_URL = 'not a url, and it has hunter2 in it'; + + it('never prints the value of a var declared in secrets', () => { + let message = ''; + try { + env({ DATABASE_URL: SECRET_URL }, { DATABASE_URL: url() }); + } catch (err) { + message = (err as Error).message; + } + expect(message).toMatch(/DATABASE_URL: Invalid url/); + expect(message).toMatch(/value redacted, \d+ chars/); + expect(message).not.toContain('hunter2'); + }); + + it('honors { secret: true } for a var declared in vars', () => { + let message = ''; + try { + env({ API_KEY: SECRET_URL }, {}, { API_KEY: url({ secret: true }) }); + } catch (err) { + message = (err as Error).message; + } + expect(message).toMatch(/API_KEY: Invalid url/); + expect(message).not.toContain('hunter2'); + }); + + it('keeps the "how to fix" message for a MISSING secret (no value to leak)', () => { + let message = ''; + try { + env({}, { DATABASE_URL: url({ desc: 'the primary connection string' }) }); + } catch (err) { + message = (err as Error).message; + } + expect(message).toMatch(/DATABASE_URL: the primary connection string/); + expect(message).not.toMatch(/value redacted/); + }); + + it('does not redact non-secret vars', () => { + let message = ''; + try { + env({ PUBLIC_URL: 'nope' }, {}, { PUBLIC_URL: url() }); + } catch (err) { + message = (err as Error).message; + } + expect(message).toContain('"nope"'); + }); + + it('redacts secrets from the cleaned env when it is serialized', () => { + const config = env( + { DATABASE_URL: 'postgres://user:hunter2@localhost/db' }, + { DATABASE_URL: url() }, + { PORT: num({ default: 3000 }) } + ); + + // property access still returns the real value + expect(config.DATABASE_URL).toBe('postgres://user:hunter2@localhost/db'); + const serialized = JSON.stringify(config); + expect(serialized).not.toContain('hunter2'); + expect(serialized).toContain('[redacted]'); + expect(JSON.parse(serialized).PORT).toBe(3000); + }); + + it('redacts secrets from util.inspect (console.log of the config)', () => { + const config = env( + { DATABASE_URL: 'postgres://user:hunter2@localhost/db' }, + { DATABASE_URL: url() }, + { PORT: num({ default: 3000 }) } + ); + const inspected = inspect(config); + expect(inspected).not.toContain('hunter2'); + expect(inspected).toContain('[redacted]'); + }); + + it('leaves a non-secret config serializable as before', () => { + const config = env({ PUBLIC_URL: 'https://example.com' }, {}, { PUBLIC_URL: url() }); + expect(JSON.stringify(config)).toBe('{"PUBLIC_URL":"https://example.com"}'); + }); + + it('redactEnvError scrubs quoted values and known secrets', () => { + const redacted = redactEnvError(new Error('Invalid url: "postgres://u:pw@h/db"')); + expect(redacted.message).toBe('Invalid url: "[redacted]"'); + + const custom = redactEnvError(new Error('could not connect with hunter2'), ['hunter2']); + expect(custom.message).toBe('could not connect with [redacted]'); + + const untouched = new Error('nothing to redact'); + expect(redactEnvError(untouched)).toBe(untouched); + }); +}); diff --git a/packages/12factor-env/src/checks.ts b/packages/12factor-env/src/checks.ts new file mode 100644 index 0000000000..1a3070c8a8 --- /dev/null +++ b/packages/12factor-env/src/checks.ts @@ -0,0 +1,81 @@ +// ── Cross-field checks ─────────────────────────────────────────────────────── +// +// Per-var validation cannot express a constraint BETWEEN two vars, so consumers +// wrote those after `env()` returned — each with its own `throw`, so an operator +// fixes one error, redeploys, and hits the next instead of getting envalid's one +// consolidated report. A check runs after per-var validation, over the cleaned +// (coerced) values, and its failure joins that single report. + +export type EnvCheck> = { + /** Vars this check is about: used to attribute the error, and to SKIP the + * check when any of them already failed validation ("both missing" must not + * also report as "both equal"). */ + vars: readonly string[]; + /** Return false (or throw) to fail. Receives the cleaned, coerced values. */ + check: (env: T) => boolean; + /** Why it failed, phrased as an instruction to the operator. */ + message: string; +}; + +/** At most one of `vars` may be set. */ +export const mutuallyExclusive = >( + vars: readonly string[], + message = `only one of ${vars.join(', ')} may be set` +): EnvCheck => ({ + vars, + message, + check: (env) => + vars.filter((name) => (env as Record)[name] !== undefined).length <= 1 + }); + +/** All of `vars` must hold distinct values (e.g. two roles that cannot be the + * same role — one needs BYPASSRLS, the other must be subject to RLS). */ +export const distinct = >( + vars: readonly string[], + message = `${vars.join(', ')} must all be different` +): EnvCheck => ({ + vars, + message, + check: (env) => { + const values = vars + .map((name) => (env as Record)[name]) + .filter((value) => value !== undefined); + return new Set(values).size === values.length; + } + }); + +/** When `flag` is truthy, every var in `vars` must be set — a feature flag that + * turns other vars into requirements. */ +export const requiredWhen = >( + flag: string, + vars: readonly string[], + message = `${vars.join(', ')} are required when ${flag} is set` +): EnvCheck => ({ + vars: [flag, ...vars], + message, + check: (env) => { + const record = env as Record; + if (!record[flag]) return true; + return vars.every((name) => record[name] !== undefined && record[name] !== ''); + } + }); + +/** Run the checks whose vars all validated; return one message per failure. */ +export const runChecks = ( + checks: readonly EnvCheck[], + cleaned: T, + failedVars: ReadonlySet +): { vars: readonly string[]; message: string }[] => { + const failures: { vars: readonly string[]; message: string }[] = []; + for (const check of checks) { + if (check.vars.some((name) => failedVars.has(name))) continue; + let ok: boolean; + try { + ok = check.check(cleaned); + } catch { + ok = false; + } + if (!ok) failures.push({ vars: check.vars, message: check.message }); + } + return failures; +}; diff --git a/packages/12factor-env/src/index.ts b/packages/12factor-env/src/index.ts index dbfaeefe70..cb39dc531a 100644 --- a/packages/12factor-env/src/index.ts +++ b/packages/12factor-env/src/index.ts @@ -1,19 +1,22 @@ import type { CleanedEnv, CleanOptions, Spec,ValidatorSpec } from 'envalid'; import { + applyDefaultMiddleware, bool, - cleanEnv as envalidCleanEnv, + customCleanEnv, email, EnvError, EnvMissingError, host, json, makeValidator, - num, port, str, testOnly, url} from 'envalid'; +import { type EnvCheck, runChecks } from './checks'; +import { isSecretSpec, redactMessage, stripSecret, withRedactedSerialization } from './redact'; + /** * NODE_ENV resolved with "house" semantics. * @@ -86,15 +89,43 @@ const withResolvedNodeEnv = ( }; /** - * Custom reporter that throws an error instead of calling process.exit - * This allows errors to be caught and handled properly in tests and applications + * Reporter that throws instead of calling `process.exit`, so errors can be + * caught and handled in tests and applications. It also owns the two things + * envalid's own reporter cannot do: it REDACTS values for vars marked secret + * (envalid quotes the offending value into its message), and it runs the + * cross-field `checks` so their failures join the same consolidated report + * rather than throwing separately after the call. */ -const throwingReporter = ({ errors }: { errors: Partial> }) => { - const errorKeys = Object.keys(errors) as (keyof T)[]; - if (errorKeys.length > 0) { - const missingVars = errorKeys.map((key) => `${String(key)}: ${errors[key]?.message ?? 'unknown error'}`); - throw new EnvError(`Missing or invalid environment variables:\n ${missingVars.join('\n ')}`); - } +const makeReporter = + ( + environment: Record, + secretNames: ReadonlySet, + checks: readonly EnvCheck[] + ) => + ({ errors, env: cleaned }: { errors: Partial>; env: unknown }) => { + const failedVars = new Set(Object.keys(errors)); + const lines = [...failedVars].map((name) => { + const error = errors[name as keyof T]; + const message = !error + ? 'unknown error' + : secretNames.has(name) + ? redactMessage(error, environment[name]) + : error.message; + return `${name}: ${message}`; + }); + + for (const failure of runChecks(checks, cleaned as never, failedVars)) { + lines.push(`${failure.vars.join(', ')}: ${failure.message}`); + } + + if (lines.length > 0) { + throw new EnvError(`Missing or invalid environment variables:\n ${lines.join('\n ')}`); + } + }; + +export type EnvOptions = CleanOptions & { + /** Constraints between vars, evaluated over the cleaned values. */ + checks?: readonly EnvCheck[]; }; /** @@ -104,12 +135,26 @@ const throwingReporter = ({ errors }: { errors: Partial>>( environment: Record, specs: S, - options?: CleanOptions + options?: EnvOptions ): CleanedEnv => { - return envalidCleanEnv(withResolvedNodeEnv(environment), specs, { - reporter: throwingReporter, - ...options - }); + const { checks = [], ...cleanOptions } = options ?? {}; + const secretNames = new Set( + Object.entries(specs) + .filter(([, spec]) => isSecretSpec(spec)) + .map(([name]) => name) + ); + // customCleanEnv, not cleanEnv: the redacting serializers have to be defined + // on the plain cleaned object, before envalid proxies and freezes it. + return customCleanEnv( + withResolvedNodeEnv(environment), + stripSecret(specs), + (plainCleaned, rawEnvironment) => + applyDefaultMiddleware(withRedactedSerialization(plainCleaned, secretNames), rawEnvironment), + { + reporter: makeReporter(environment, secretNames, checks), + ...cleanOptions + } + ) as CleanedEnv; }; // ── Fallback classes ───────────────────────────────────────────────────────── @@ -126,7 +171,10 @@ const cleanEnv = >>( // `devDefault` relies on the NODE_ENV normalization above, so it is enforced in // production even when NODE_ENV is only implicitly set. -type ValidatorFactory = (spec?: Spec) => ValidatorSpec; +// Loosely typed on purpose: a house validator takes a wider spec than envalid's +// (`list`'s per-item `choices`, `num`'s `min`/`max`), and these wrappers only +// ever add a default to it. +type ValidatorFactory = (spec?: Spec | any) => ValidatorSpec; /** Class 1 — always resolves to `defaultValue` when the var is unset. */ const withDefault = ( @@ -197,9 +245,15 @@ type Specs = Record>; /** * Validate environment variables * + * Everything declared in `secrets` is treated as sensitive: its value is never + * printed in a validation error, and it is redacted from `JSON.stringify`/ + * `util.inspect` output of the returned object. Mark a var in `vars` the same + * way with `str({ secret: true })`. + * * @param inputEnv - The environment object (usually process.env) * @param secrets - Required environment variables (validated with envalid) * @param vars - Optional environment variables (validated with envalid) + * @param options - `checks` for constraints between vars * @returns Validated and cleaned environment object * * @example @@ -213,6 +267,9 @@ type Specs = Record>; * { * PORT: port({ default: 3000 }), * DEBUG: bool({ default: false }) + * }, + * { + * checks: [distinct(['CONTROL_USER', 'UPSTREAM_USER'])] * } * ); * ``` @@ -220,13 +277,19 @@ type Specs = Record>; const env = ( inputEnv: Record, secrets: S = {} as S, - vars: V = {} as V + vars: V = {} as V, + options: EnvOptions = {} ): CleanedEnv => { - // First pass: validate optional vars + const secretSpecs = Object.fromEntries( + Object.entries(secrets).map(([name, spec]) => [name, { ...spec, secret: true }]) + ) as unknown as S; + + // First pass: validate optional vars. Cross-field checks are deferred to the + // second pass, where the secrets are cleaned too and the check can see them. const varEnv = cleanEnv(inputEnv, vars); const mergedEnv = { ...inputEnv, ...varEnv } as unknown as Record; - return cleanEnv(mergedEnv, { ...secrets, ...vars }) as unknown as CleanedEnv; + return cleanEnv(mergedEnv, { ...secretSpecs, ...vars }, options as EnvOptions) as unknown as CleanedEnv; }; export { @@ -242,7 +305,6 @@ export { host, json, makeValidator, - num, // Lenient coercion parseEnvBoolean, parseEnvList, @@ -255,4 +317,28 @@ export { // Fallback-class wrappers withDefault}; +// House validators: csv lists, bounded numbers, durations, string enums. +// `num` shadows envalid's on purpose — same acceptance, plus min/max/integer. +export { + duration, + type DurationSpec, + enumerated, + int, + list, + type ListSpec, + num, + type NumSpec, + oneOf} from './validators'; + +// Cross-field checks. +export { + distinct, + type EnvCheck, + mutuallyExclusive, + requiredWhen, + runChecks} from './checks'; + +// Secret handling. +export { redactEnvError, type SecretSpec } from './redact'; + export type { CleanedEnv, Spec,ValidatorSpec }; diff --git a/packages/12factor-env/src/redact.ts b/packages/12factor-env/src/redact.ts new file mode 100644 index 0000000000..c6c9bcd903 --- /dev/null +++ b/packages/12factor-env/src/redact.ts @@ -0,0 +1,123 @@ +// ── Secret redaction ───────────────────────────────────────────────────────── +// +// envalid embeds the offending value in its error message (`Invalid url: "..."`, +// `Not a string: "..."`, `Invalid port input: "..."`), and a throwing reporter +// concatenates those into the crash log. So a malformed DATABASE_URL prints its +// password, and a bad API key prints the key. `env()` already knows which vars +// are sensitive — its first parameter is literally named `secrets` — so the +// library redacts on the error path instead of trusting the value not to be one. + +import type { Spec, ValidatorSpec } from 'envalid'; +import { EnvError, EnvMissingError } from 'envalid'; + +const REDACTED = '[redacted]'; + +/** A `Spec` may carry `secret: true` to mark the var sensitive on its own, + * independently of whether it was declared in `env()`'s `secrets` argument. */ +export type SecretSpec = { secret?: boolean }; + +// Declared on envalid's own `Spec` so `str({ secret: true })` type-checks with +// the validators consumers already use. The marker is stripped before the spec +// reaches envalid. +declare module 'envalid' { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + interface Spec { + /** Never print this value in a validation error or in serialized output. */ + secret?: boolean; + } +} + +export const isSecretSpec = (spec: ValidatorSpec | Spec): boolean => + (spec as SecretSpec).secret === true; + +/** Strip a spec's `secret` marker before handing it to envalid (which would + * otherwise carry an unknown key through its spec objects). */ +export const stripSecret = >>(specs: S): S => { + const out: Record> = {}; + for (const [name, spec] of Object.entries(specs)) { + if (isSecretSpec(spec)) { + const rest = { ...spec } as ValidatorSpec & SecretSpec; + delete rest.secret; + out[name] = rest as ValidatorSpec; + } else { + out[name] = spec; + } + } + return out as S; +}; + +/** envalid quotes the offending value; replace the quoted payload. */ +const stripQuotedValues = (message: string): string => + message.replace(/"[^"]*"/g, `"${REDACTED}"`); + +/** + * The reported message for a failed secret var: names the var and the failure, + * never the value. A MISSING var has no value to leak, so its message (the + * spec description) is kept verbatim — that is the part that tells an operator + * what to set. + */ +export const redactMessage = (error: Error, rawValue: string | undefined): string => { + if (error instanceof EnvMissingError) return error.message; + const sanitized = stripQuotedValues(error.message); + const length = rawValue?.length ?? 0; + return `${sanitized} (value redacted, ${length} chars)`; +}; + +/** + * Scrub secrets out of an error before re-logging it. Quoted values in envalid + * messages are always replaced; pass known secret values to remove them + * wherever else they appear (a message a consumer built itself). + * + * ```ts + * try { loadConfig(); } catch (err) { logger.error(redactEnvError(err)); } + * ``` + */ +export const redactEnvError = ( + error: unknown, + secretValues: Iterable = [] +): Error => { + if (!(error instanceof Error)) return new Error(String(error)); + let message = stripQuotedValues(error.message); + for (const value of secretValues) { + if (value && value.length > 0) message = message.split(value).join(REDACTED); + } + if (message === error.message) return error; + const redacted = error instanceof EnvError ? new EnvError(message) : new Error(message); + redacted.stack = error.stack; + return redacted; +}; + +/** + * Make the cleaned env safe to log wholesale: services log their config object + * at boot, and `JSON.stringify`/`util.inspect` would print every secret. The + * values stay readable through property access; only serialization redacts. + * + * Must be applied to the PLAIN cleaned object, before envalid's strict proxy and + * `Object.freeze` (whose non-configurable props can no longer be defined on); + * envalid's proxy passes `toJSON` and the inspect symbol through to the target. + */ +export const withRedactedSerialization = ( + cleaned: T, + secretNames: ReadonlySet +): T => { + if (secretNames.size === 0) return cleaned; + + const redactedView = (): Record => { + const view: Record = {}; + for (const name of Object.keys(cleaned)) { + view[name] = secretNames.has(name) + ? REDACTED + : (cleaned as Record)[name]; + } + return view; + }; + + for (const key of ['toJSON', Symbol.for('nodejs.util.inspect.custom')] as const) { + Object.defineProperty(cleaned, key, { + value: redactedView, + enumerable: false, + configurable: true + }); + } + return cleaned; +}; diff --git a/packages/12factor-env/src/validators.ts b/packages/12factor-env/src/validators.ts new file mode 100644 index 0000000000..38ede34539 --- /dev/null +++ b/packages/12factor-env/src/validators.ts @@ -0,0 +1,195 @@ +// ── House validators ───────────────────────────────────────────────────────── +// +// envalid ships str/num/port/bool/url/host/json/email. The validators here fill +// the gaps consumers were provably hand-rolling *after* the schema — a csv var +// could not be declared at all, and nothing expressed a numeric range — so the +// split/trim/bounds check happened outside `env()`, with its own throw and its +// own error message instead of joining the one consolidated report. +// +// Every validator keeps the library's contract: unset resolves the default, +// set-but-invalid THROWS. None of them silently degrade to an empty list or a +// NaN, because that is how "allowlist" becomes "allow nothing" at runtime. + +import type { Spec, ValidatorSpec } from 'envalid'; +import { EnvError, makeValidator } from 'envalid'; + +const asSpec = (spec: Record): Spec => spec as Spec; + +// ── list() ─────────────────────────────────────────────────────────────────── + +export type ListSpec = Omit, 'choices'> & { + /** Item delimiter. Default: `,`. Use `:` for PATH-style vars. */ + separator?: string; + /** Admissable values for EACH item (envalid's `choices` compares the whole + * parsed value, which for a list is the array itself). */ + choices?: ReadonlyArray; +}; + +/** + * Comma-separated list of non-empty, trimmed strings. + * + * ```ts + * list() // 'a, b' -> ['a','b'] + * list({ choices: ['api_key', 'jwt'] as const }) // per-item membership, typed + * list({ separator: ':' }) // PATH-style + * withDefault(list, ['api_key']) // typed default for "unset" + * ``` + * + * A var that is SET but yields no items (`''`, `','`) is an error, not `[]`: + * "set but empty" is a deployment mistake, and an empty allowlist silently + * means "allow nothing". Use a default (or `withDefault`) to express "unset is + * fine". `parseEnvList` keeps the lenient semantics for non-schema callers. + */ +export const list = ( + spec: ListSpec = {} +): ValidatorSpec => { + const { separator = ',', choices, ...rest } = spec; + + const parse = (input: string | T[]): T[] => { + const items = ( + Array.isArray(input) ? input.map(String) : String(input).split(separator) + ) + .map((item) => item.trim()) + .filter(Boolean) as T[]; + + if (items.length === 0) { + throw new EnvError( + `Empty list: set, but lists no ${separator === ',' ? 'comma' : `"${separator}"`}-separated value` + ); + } + if (choices) { + const invalid = items.filter((item) => !choices.includes(item)); + if (invalid.length > 0) { + throw new EnvError( + `Invalid list value(s) ${invalid.map((i) => `"${i}"`).join(', ')} not in choices [${choices.join(', ')}]` + ); + } + } + return items; + }; + + // `choices` is deliberately not forwarded: envalid would test the whole array + // against it. Membership is enforced per item above. + return makeValidator(parse as (input: string) => T[])( + asSpec(rest) + ) as ValidatorSpec; +}; + +// ── num()/int() with bounds ────────────────────────────────────────────────── + +export type NumSpec = Spec & { + /** Inclusive lower bound. `min: 0` for "non-negative milliseconds". */ + min?: number; + /** Inclusive upper bound. */ + max?: number; + /** Reject non-integers. `num()` alone accepts `1.5` for a count. */ + integer?: boolean; +}; + +const parseNumber = (input: string | number, spec: NumSpec): number => { + const value = typeof input === 'number' ? input : Number(input); + if (!Number.isFinite(value)) throw new EnvError(`Invalid number input: "${input}"`); + if (spec.integer && !Number.isInteger(value)) { + throw new EnvError(`Expected an integer, got: "${input}"`); + } + if (spec.min !== undefined && value < spec.min) { + throw new EnvError(`Expected a number >= ${spec.min}, got: "${input}"`); + } + if (spec.max !== undefined && value > spec.max) { + throw new EnvError(`Expected a number <= ${spec.max}, got: "${input}"`); + } + return value; +}; + +/** + * Number, optionally bounded. A superset of envalid's `num`: same acceptance + * for a plain `num()`, plus `min`/`max`/`integer`. + * + * ```ts + * num({ min: 0 }) // non-negative (a duration in ms) + * num({ min: 1, max: 64 }) // a concurrency dial + * ``` + */ +export const num = (spec: NumSpec = {}): ValidatorSpec => { + const rest = { ...spec }; + delete rest.min; + delete rest.max; + delete rest.integer; + return makeValidator(((input: string) => + parseNumber(input, spec)) as (input: string) => number)( + asSpec(rest) + ) as ValidatorSpec; +}; + +/** Integer, optionally bounded — `num({ integer: true })`. */ +export const int = (spec: Omit = {}): ValidatorSpec => + num({ ...spec, integer: true }); + +// ── duration() ────────────────────────────────────────────────────────────── + +const DURATION_UNITS_MS: Record = { + ms: 1, + s: 1_000, + m: 60_000, + h: 3_600_000, + d: 86_400_000 +}; + +export type DurationSpec = Spec & { + /** Inclusive lower bound in milliseconds. Default: 0 (no negative durations). */ + min?: number; + /** Inclusive upper bound in milliseconds. */ + max?: number; +}; + +/** + * Duration normalized to milliseconds, accepting a bare number of ms or a + * suffixed value: `30s`, `5m`, `2h`, `1d`, `500ms`. + * + * ```ts + * duration() // '30s' -> 30000, '250' -> 250 + * withDefault(duration, 30_000) // a default is plain milliseconds + * ``` + */ +export const duration = (spec: DurationSpec = {}): ValidatorSpec => { + const { min = 0, max, ...rest } = spec; + + const parse = (input: string | number): number => { + if (typeof input === 'number') return parseNumber(input, { min, max }); + const match = /^(-?\d+(?:\.\d+)?)\s*(ms|s|m|h|d)?$/i.exec(String(input).trim()); + if (!match) throw new EnvError(`Invalid duration input: "${input}"`); + const unit = (match[2] ?? 'ms').toLowerCase(); + return parseNumber(Number(match[1]) * DURATION_UNITS_MS[unit], { min, max }); + }; + + return makeValidator(parse as (input: string) => number)( + asSpec(rest) + ) as ValidatorSpec; +}; + +// ── enumerated() ──────────────────────────────────────────────────────────── + +/** + * One of a fixed set of strings, typed as the union — `str({ choices })` with a + * name consumers actually find. `oneOf` is an alias. + * + * ```ts + * enumerated(['per-function', 'combined'] as const) + * enumerated(['read', 'write'] as const, { default: 'read' }) + * ``` + */ +export const enumerated = ( + choices: ReadonlyArray, + spec: Omit, 'choices'> = {} +): ValidatorSpec => { + const parse = (input: string): T => { + const value = String(input) as T; + if (!choices.includes(value)) { + throw new EnvError(`Value "${value}" not in choices [${choices.join(', ')}]`); + } + return value; + }; + return makeValidator(parse)(asSpec(spec)) as ValidatorSpec; +}; + +export const oneOf = enumerated;