diff --git a/.changeset/fix-env-undefined.md b/.changeset/fix-env-undefined.md new file mode 100644 index 0000000..d205ade --- /dev/null +++ b/.changeset/fix-env-undefined.md @@ -0,0 +1,6 @@ +--- +'@powersync/cli-core': patch +'powersync': patch +--- + +fail `!env` substitution when the named environment variable is missing diff --git a/cli/src/api/parse-local-cloud-service-config.ts b/cli/src/api/parse-local-cloud-service-config.ts index 5dcea5f..dbb4976 100644 --- a/cli/src/api/parse-local-cloud-service-config.ts +++ b/cli/src/api/parse-local-cloud-service-config.ts @@ -21,17 +21,11 @@ export function parseLocalCloudServiceConfig( const servicePath = join(projectDirectory, SERVICE_FILENAME); if (!existsSync(servicePath)) return undefined; - let raw: ServiceCloudConfig | undefined; - try { - const doc = parseYamlFile(servicePath); - raw = doc.contents?.toJSON(); - if (useRawConfig) { - return raw; - } - - return ServiceCloudConfig.decode(raw as ServiceCloudConfig); - } catch (error) { - if (!useRawConfig) throw error; + const doc = parseYamlFile(servicePath); + const raw = doc.contents?.toJSON(); + if (useRawConfig) { return raw; } + + return ServiceCloudConfig.decode(raw as ServiceCloudConfig); } diff --git a/cli/test/commands/link.test.ts b/cli/test/commands/link.test.ts index 16b452d..b2712b4 100644 --- a/cli/test/commands/link.test.ts +++ b/cli/test/commands/link.test.ts @@ -331,6 +331,20 @@ type: self-hosted expect(linkYaml.api_key).toBe('!env PS_ADMIN_TOKEN'); }); + it('links when service.yaml has unresolved !env placeholders', async () => { + const projectDir = join(tmpDir, PROJECT_DIR); + mkdirSync(projectDir, { recursive: true }); + writeFileSync( + join(projectDir, SERVICE_FILENAME), + '_type: self-hosted\nreplication:\n connections:\n - type: postgresql\n uri: !env PS_DATA_SOURCE_URI\n', + 'utf8' + ); + process.env.PS_ADMIN_TOKEN = 'k'; + const { error, stdout } = await runLinkSelfHostedDirect(['--api-url=https://sync.example.com']); + expect(error).toBeUndefined(); + expect(stdout).toContain(`Updated ${PROJECT_DIR}/${CLI_FILENAME} with self-hosted link.`); + }); + it('respects --directory flag', async () => { const customDir = 'my-powersync'; mkdirSync(join(tmpDir, customDir), { recursive: true }); diff --git a/cli/test/commands/validate.test.ts b/cli/test/commands/validate.test.ts index 40284b1..2ae2535 100644 --- a/cli/test/commands/validate.test.ts +++ b/cli/test/commands/validate.test.ts @@ -168,5 +168,65 @@ describe('validate', () => { expect(call.syncConfigContent).toContain('SELECT 1 FROM cloud_validate_override'); expect(call.syncConfigContent).not.toContain('cloud_default_file_only'); }); + + it('names a missing !env variable instead of testing connections with a bogus URI', async () => { + resetManagementClientMocks(); + const origUri = process.env.PS_DATABASE_URI; + delete process.env.PS_DATABASE_URI; + + try { + const { instanceId, orgId, projectId } = MOCK_CLOUD_IDS; + const projectDir = join(tmpRoot, 'powersync'); + mkdirSync(projectDir, { recursive: true }); + + writeFileSync( + join(projectDir, CLI_FILENAME), + `type: cloud\ninstance_id: ${instanceId}\norg_id: ${orgId}\nproject_id: ${projectId}\n`, + 'utf8' + ); + env.PS_ADMIN_TOKEN = 'token'; + env.INSTANCE_ID = undefined; + + managementClientMock.getInstanceConfig.mockResolvedValue({ + config: { region: 'us' }, + id: instanceId, + name: 'test-instance', + sync_rules: '' + }); + + writeFileSync( + join(projectDir, SERVICE_FILENAME), + [ + '_type: cloud', + 'name: test-instance', + 'region: us', + 'replication:', + ' connections:', + ' - type: postgresql', + ' uri: !env PS_DATABASE_URI', + '' + ].join('\n'), + 'utf8' + ); + + const config = await Config.load({ root }); + const cmd = new Validate( + ['--directory', 'powersync', '--validate-only', 'connections', '--output', 'json'], + config + ); + const result = await captureOutput(() => cmd.run()); + + expect(result.error).toBeDefined(); + expect(result.error?.message).toMatch(/PS_DATABASE_URI/); + expect(result.error?.message).toMatch(/undefined/); + expect(managementClientMock.testConnection).not.toHaveBeenCalled(); + } finally { + if (origUri === undefined) { + delete process.env.PS_DATABASE_URI; + } else { + process.env.PS_DATABASE_URI = origUri; + } + } + }); }); }); diff --git a/cli/test/utils/yaml.test.ts b/cli/test/utils/yaml.test.ts new file mode 100644 index 0000000..9ebd7c9 --- /dev/null +++ b/cli/test/utils/yaml.test.ts @@ -0,0 +1,44 @@ +import { parseYamlFile } from '@powersync/cli-core'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +describe('parseYamlFile !env', () => { + let tmpDir: string; + let origUri: string | undefined; + + beforeEach(() => { + origUri = process.env.PS_DATABASE_URI; + delete process.env.PS_DATABASE_URI; + tmpDir = mkdtempSync(join(tmpdir(), 'yaml-env-')); + }); + + afterEach(() => { + if (origUri === undefined) { + delete process.env.PS_DATABASE_URI; + } else { + process.env.PS_DATABASE_URI = origUri; + } + + rmSync(tmpDir, { force: true, recursive: true }); + }); + + it('throws naming the missing variable instead of substituting the name', () => { + const filePath = join(tmpDir, 'service.yaml'); + writeFileSync(filePath, 'uri: !env PS_DATABASE_URI\n', 'utf8'); + + expect(() => parseYamlFile(filePath)).toThrow(/PS_DATABASE_URI/); + expect(() => parseYamlFile(filePath)).toThrow(/undefined/); + }); + + it('substitutes the environment variable when it is set', () => { + process.env.PS_DATABASE_URI = 'postgresql://repro:repro@db.example.invalid:5432/postgres'; + const filePath = join(tmpDir, 'service.yaml'); + writeFileSync(filePath, 'uri: !env PS_DATABASE_URI\n', 'utf8'); + + expect(parseYamlFile(filePath).contents?.toJSON()).toEqual({ + uri: 'postgresql://repro:repro@db.example.invalid:5432/postgres' + }); + }); +}); diff --git a/packages/cli-core/src/utils/ensure-service-type.ts b/packages/cli-core/src/utils/ensure-service-type.ts index 55a0c21..95b9ce6 100644 --- a/packages/cli-core/src/utils/ensure-service-type.ts +++ b/packages/cli-core/src/utils/ensure-service-type.ts @@ -1,10 +1,10 @@ import { ux } from '@oclif/core'; -import { existsSync } from 'node:fs'; +import { existsSync, readFileSync } from 'node:fs'; import { join } from 'node:path'; import { PowerSyncCommand } from '../command-types/PowerSyncCommand.js'; import { SERVICE_FILENAME } from './project-config.js'; -import { parseYamlFile } from './yaml.js'; +import { parseYamlDocumentPreserveTags } from './yaml.js'; export enum ServiceType { CLOUD = 'cloud', @@ -36,7 +36,9 @@ export function ensureServiceTypeMatches(options: EnsureServiceTypeMatchesOption return; } - const service = parseYamlFile(servicePath); + // Only `_type` is required here; skip !env resolution so templates with unset + // placeholders (e.g. `uri: !env PS_DATA_SOURCE_URI`) still type-check. + const service = parseYamlDocumentPreserveTags(readFileSync(servicePath, 'utf8')); const serviceJson = service.contents?.toJSON(); if (serviceJson?._type == null) { diff --git a/packages/cli-core/src/utils/yaml.ts b/packages/cli-core/src/utils/yaml.ts index a11da56..ef8ce0e 100644 --- a/packages/cli-core/src/utils/yaml.ts +++ b/packages/cli-core/src/utils/yaml.ts @@ -67,10 +67,18 @@ const YAML_PARSE_OPTIONS = { customTags: [YamlEnvTag] }; /** * Parses a YAML document, evaluating !env tags. + * Throws when substitution fails (missing or invalid env vars) so callers cannot + * treat the unresolved variable name as a real value. */ export function parseYamlFile(filePath: string): yaml.Document { const content = readFileSync(filePath, 'utf8'); - return yaml.parseDocument(content, YAML_PARSE_OPTIONS); + const doc = yaml.parseDocument(content, YAML_PARSE_OPTIONS); + if (doc.errors.length > 0) { + const details = doc.errors.map((error) => error.message.trim()).join('\n'); + throw new Error(`Failed to parse ${filePath}:\n${details}`); + } + + return doc; } /**