Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/fix-env-undefined.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@powersync/cli-core': patch
'powersync': patch
---

fail `!env` substitution when the named environment variable is missing
16 changes: 5 additions & 11 deletions cli/src/api/parse-local-cloud-service-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
14 changes: 14 additions & 0 deletions cli/test/commands/link.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
60 changes: 60 additions & 0 deletions cli/test/commands/validate.test.ts

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that when running powersync validate and powersync pull instance in the examples/cloud/basic-cloud-pull example, I got two differently formatted errors. This is preexisting and happens because only some calls to parseYamlFile are wrapped in try { ... } catch (error) { this.styledError(...); }. I'll make a followup PR to fix this; just mentioning it here.

Image

Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
});
});
});
44 changes: 44 additions & 0 deletions cli/test/utils/yaml.test.ts
Original file line number Diff line number Diff line change
@@ -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'
});
});
});
8 changes: 5 additions & 3 deletions packages/cli-core/src/utils/ensure-service-type.ts
Original file line number Diff line number Diff line change
@@ -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',
Expand Down Expand Up @@ -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) {
Expand Down
10 changes: 9 additions & 1 deletion packages/cli-core/src/utils/yaml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This description is slightly misleading since the function now throws on any YAML parsing error, not just missing env vars.

* 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;
}

/**
Expand Down