Skip to content

Commit

Permalink
fix: validate config after dereferencing (promptfoo#2129)
Browse files Browse the repository at this point in the history
  • Loading branch information
mldangelo authored Nov 12, 2024
1 parent 7a28078 commit 6cf92ea
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 4 deletions.
6 changes: 3 additions & 3 deletions src/util/config/load.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,14 +152,14 @@ export async function readConfig(configPath: string): Promise<UnifiedConfig> {
const ext = path.parse(configPath).ext;
if (ext === '.json' || ext === '.yaml' || ext === '.yml') {
const rawConfig = yaml.load(fs.readFileSync(configPath, 'utf-8'));

const validationResult = UnifiedConfigSchema.safeParse(rawConfig);
const dereferencedConfig = await dereferenceConfig(rawConfig as UnifiedConfig);
const validationResult = UnifiedConfigSchema.safeParse(dereferencedConfig);
if (!validationResult.success) {
logger.warn(
`Invalid configuration file ${configPath}:\n${fromError(validationResult.error).message}`,
);
}
ret = await dereferenceConfig(rawConfig as UnifiedConfig);
ret = dereferencedConfig;
} else if (isJavascriptFile(configPath)) {
const imported = await importModule(configPath);
const validationResult = UnifiedConfigSchema.safeParse(imported);
Expand Down
57 changes: 56 additions & 1 deletion test/util/config/load.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import $RefParser from '@apidevtools/json-schema-ref-parser';
import * as fs from 'fs';
import { globSync } from 'glob';
import yaml from 'js-yaml';
import * as path from 'path';
import cliState from '../../../src/cliState';
import { importModule } from '../../../src/esm';
import logger from '../../../src/logger';
import { readTests } from '../../../src/testCases';
import type { UnifiedConfig } from '../../../src/types';
import { maybeLoadFromExternalFile } from '../../../src/util';
Expand Down Expand Up @@ -818,7 +820,7 @@ describe('resolveConfigs', () => {

describe('readConfig', () => {
beforeEach(() => {
jest.resetAllMocks();
jest.clearAllMocks();
});

it('should read JSON config file', async () => {
Expand Down Expand Up @@ -936,4 +938,57 @@ describe('readConfig', () => {
prompts: ['{{prompt}}'],
});
});

it('should resolve YAML references before validation', async () => {
const mockConfig = {
description: 'test_config',
prompts: ['test {{text}}'],
providers: [{ $ref: 'defaultParams.yaml#/model' }],
temperature: 1,
tests: [{ vars: { text: 'test text' } }],
};

const dereferencedConfig = {
description: 'test_config',
prompts: ['test {{text}}'],
providers: ['echo'],
temperature: 1,
tests: [{ vars: { text: 'test text' } }],
};

jest.mocked(fs.readFileSync).mockReturnValue(yaml.dump(mockConfig));
jest.spyOn($RefParser.prototype, 'dereference').mockResolvedValue(dereferencedConfig);

const result = await readConfig('config.yaml');
expect(result).toEqual(dereferencedConfig);
expect(fs.readFileSync).toHaveBeenCalledWith('config.yaml', 'utf-8');
});

it('should throw validation error for invalid dereferenced config', async () => {
const mockConfig = {
description: 'invalid_config',
prompts: ['test prompt'],
providers: [{ $ref: 'defaultParams.yaml#/invalidKey' }],
};

const dereferencedConfig = {
description: 'invalid_config',
prompts: ['test prompt'],
providers: [{ invalid: true }], // This will fail validation
};

jest.mocked(fs.readFileSync).mockReturnValue(yaml.dump(mockConfig));
jest.spyOn($RefParser.prototype, 'dereference').mockResolvedValue(dereferencedConfig);
jest.mocked(fs.existsSync).mockReturnValue(true);
const loggerSpy = jest.spyOn(logger, 'warn').mockImplementation();

await readConfig('config.yaml');

expect(loggerSpy).toHaveBeenCalledWith(
'Invalid configuration file config.yaml:\nValidation error: Unrecognized key(s) in object: \'invalid\' at "providers[0]"',
);
expect(loggerSpy.mock.calls[0][0]).toContain('Invalid configuration file');

loggerSpy.mockRestore();
});
});

0 comments on commit 6cf92ea

Please sign in to comment.