Skip to content

Commit

Permalink
fix(schema): fix schema validation and incorrect default rule code in…
Browse files Browse the repository at this point in the history
… setupEngine
  • Loading branch information
botzai committed Aug 28, 2024
1 parent c74d116 commit 0a3c65c
Show file tree
Hide file tree
Showing 11 changed files with 147 additions and 95 deletions.
11 changes: 6 additions & 5 deletions src/archetypes/node-fullstack.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@
],
"config": {
"minimumDependencyVersions": {
"@types/react": "^17.0.0",
"react": "^17.0.0",
"@yarnpkg/lockfile": "^1.2.0",
"commander": "^2.0.0",
"nodemon": "^3.9.0"
"@types/react": ">=18.0.0",
"react": "~17.0.0",
"@yarnpkg/lockfile": "<1.2.0",
"commander": ">=2.0.0 <13.0.0",
"nodemon": ">4.9.0",
"@colors/colors": "1.7.0 || 1.6.0"
},
"standardStructure": {
"app": {
Expand Down
29 changes: 0 additions & 29 deletions src/core/engine/engineSetup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,35 +83,6 @@ describe('setupEngine', () => {
expect(engine).toBeDefined();
});

it('should handle errors when loading rules and add default rules', async () => {
const mockAddRule = jest.fn();
(Engine as jest.Mock).mockImplementation(() => ({
addOperator: jest.fn(),
addRule: mockAddRule,
addFact: jest.fn(),
on: jest.fn()
}));

(ConfigManager.getConfig as jest.Mock).mockResolvedValue({
archetype: mockArchetypeConfig,
rules: [
undefined,
{ name: undefined },
{ name: 'invalidRule', conditions: null }
],
cliOptions: {}
});

await setupEngine(mockParams);

expect(logger.error).toHaveBeenCalledWith('Invalid rule configuration: rule or rule name is undefined');
expect(logger.error).toHaveBeenCalledWith('Invalid rule configuration: rule or rule name is undefined');
expect(logger.error).toHaveBeenCalledWith('Error loading rule: invalidRule');
expect(logger.info).toHaveBeenCalledWith('No valid rules were added. Adding default rules.');
expect(mockAddRule).toHaveBeenCalledWith(expect.objectContaining({ name: 'default-rule-1' }));
expect(mockAddRule).toHaveBeenCalledWith(expect.objectContaining({ name: 'default-rule-2' }));
});

it('should set up event listeners for success events', async () => {
const mockOn = jest.fn();
(Engine as jest.Mock).mockImplementation(() => ({
Expand Down
16 changes: 1 addition & 15 deletions src/core/engine/engineSetup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export async function setupEngine(params: SetupEngineParams): Promise<Engine> {
logger.info(`=== loading json rules..`);
const config = await ConfigManager.getConfig({ archetype, logPrefix: executionLogPrefix });

logger.debug(config.rules);
logger.debug(`rules loaded: ${config.rules}`);

const addedRules = new Set();
config.rules.forEach((rule) => {
Expand All @@ -51,20 +51,6 @@ export async function setupEngine(params: SetupEngineParams): Promise<Engine> {
}
});

if (addedRules.size === 0) {
logger.info('No valid rules were added. Adding default rules.');
engine.addRule({
name: 'default-rule-1',
conditions: { all: [] },
event: { type: 'warning', params: { message: 'Default rule 1 triggered' } }
});
engine.addRule({
name: 'default-rule-2',
conditions: { all: [] },
event: { type: 'warning', params: { message: 'Default rule 2 triggered' } }
});
}

engine.on('success', async ({ type, params }: Event) => {
if (type === 'warning') {
logger.warn(`warning detected: ${JSON.stringify(params)}`);
Expand Down
87 changes: 69 additions & 18 deletions src/facts/repoDependencyFacts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { execSync } from 'child_process';
import fs from 'fs';
import { Almanac } from 'json-rules-engine';
import { LocalDependencies, MinimumDepVersions } from '../types/typeDefs';
import { semverValid } from './repoDependencyFacts';

jest.mock('child_process');
jest.mock('fs');
Expand Down Expand Up @@ -131,32 +132,82 @@ describe('repoDependencyFacts', () => {

describe('semverValid', () => {
it('should return true for valid version comparisons', () => {
expect(repoDependencyFacts.semverValid('2.0.0', '^1.0.0')).toBe(false);
expect(repoDependencyFacts.semverValid('1.5.0', '1.0.0 - 2.0.0')).toBe(true);
expect(repoDependencyFacts.semverValid('1.0.0', '1.0.0')).toBe(true);
expect(repoDependencyFacts.semverValid('2.0.0', '>=1.0.0')).toBe(true);
expect(semverValid('2.0.0', '^1.0.0')).toBe(false);
expect(semverValid('1.5.0', '1.0.0 - 2.0.0')).toBe(true);
expect(semverValid('1.0.0', '1.0.0')).toBe(true);
expect(semverValid('2.0.0', '>=1.0.0')).toBe(true);
});

it('should return false for invalid version comparisons', () => {
expect(repoDependencyFacts.semverValid('1.0.0', '^2.0.0')).toBe(false);
expect(repoDependencyFacts.semverValid('3.0.0', '1.0.0 - 2.0.0')).toBe(false);
expect(repoDependencyFacts.semverValid('0.9.0', '>=1.0.0')).toBe(false);
expect(semverValid('1.0.0', '^2.0.0')).toBe(false);
expect(semverValid('3.0.0', '1.0.0 - 2.0.0')).toBe(false);
expect(semverValid('0.9.0', '>=1.0.0')).toBe(false);
});

it('should handle complex version ranges', () => {
expect(repoDependencyFacts.semverValid('1.2.3', '1.x || >=2.5.0 || 5.0.0 - 7.2.3')).toBe(true);
expect(repoDependencyFacts.semverValid('2.5.0', '1.x || >=2.5.0 || 5.0.0 - 7.2.3')).toBe(true);
expect(repoDependencyFacts.semverValid('5.5.5', '1.x || >=2.5.0 || 5.0.0 - 7.2.3')).toBe(true);
expect(repoDependencyFacts.semverValid('8.0.0', '1.x || >=9.5.0 || 5.0.0 - 7.2.3')).toBe(false);
expect(semverValid('1.2.3', '1.x || >=2.5.0 || 5.0.0 - 7.2.3')).toBe(true);
expect(semverValid('2.5.0', '1.x || >=2.5.0 || 5.0.0 - 7.2.3')).toBe(true);
expect(semverValid('5.5.5', '1.x || >=2.5.0 || 5.0.0 - 7.2.3')).toBe(true);
expect(semverValid('8.0.0', '1.x || >=9.5.0 || 5.0.0 - 7.2.3')).toBe(false);
});


it('should handle caret ranges', () => {
expect(semverValid('1.2.3', '^1.2.3')).toBe(true);
expect(semverValid('1.3.0', '^1.2.3')).toBe(true);
expect(semverValid('2.0.0', '^1.2.3')).toBe(false);
});

it('should handle tilde ranges', () => {
expect(semverValid('1.2.3', '~1.2.3')).toBe(true);
expect(semverValid('1.2.9', '~1.2.3')).toBe(true);
expect(semverValid('1.3.0', '~1.2.3')).toBe(false);
});

it('should handle x-ranges', () => {
expect(semverValid('1.2.3', '1.2.x')).toBe(true);
expect(semverValid('1.2.9', '1.2.x')).toBe(true);
expect(semverValid('1.3.0', '1.2.x')).toBe(false);
expect(semverValid('1.2.3', '1.x')).toBe(true);
expect(semverValid('1.3.0', '1.x')).toBe(true);
expect(semverValid('2.0.0', '1.x')).toBe(false);
});

it('should handle star ranges', () => {
expect(semverValid('1.2.3', '*')).toBe(true);
expect(semverValid('2.0.0', '*')).toBe(true);
});

it('should handle greater than and less than ranges', () => {
expect(semverValid('2.0.0', '>1.2.3')).toBe(true);
expect(semverValid('1.2.3', '>1.2.3')).toBe(false);
expect(semverValid('1.2.2', '<1.2.3')).toBe(true);
expect(semverValid('1.2.3', '<1.2.3')).toBe(false);
});

it('should handle AND ranges', () => {
expect(semverValid('1.2.3', '>1.2.2 <1.2.4')).toBe(true);
expect(semverValid('1.2.4', '>1.2.2 <1.2.4')).toBe(false);
});

it('should handle OR ranges', () => {
expect(semverValid('1.2.3', '1.2.3 || 1.2.4')).toBe(true);
expect(semverValid('1.2.4', '1.2.3 || 1.2.4')).toBe(true);
expect(semverValid('1.2.5', '1.2.3 || 1.2.4')).toBe(false);
});

it('should handle pre-release versions', () => {
expect(semverValid('1.2.3-alpha', '>=1.2.3-alpha')).toBe(true);
expect(semverValid('1.2.3-beta', '>=1.2.3-alpha')).toBe(true);
expect(semverValid('1.2.2', '>=1.2.3-alpha')).toBe(false);
});

it('should return true for empty strings', () => {
expect(repoDependencyFacts.semverValid('', '')).toBe(true);
expect(semverValid('', '')).toBe(true);
});

it('should return false for invalid input', () => {
expect(repoDependencyFacts.semverValid('not-a-version', '1.0.0')).toBe(false);
expect(repoDependencyFacts.semverValid('1.0.0', 'not-a-range')).toBe(false);
expect(semverValid('not-a-version', '1.0.0')).toBe(false);
expect(semverValid('1.0.0', 'not-a-range')).toBe(false);
});
});
});
25 changes: 18 additions & 7 deletions src/facts/repoDependencyFacts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,35 +186,46 @@ export async function repoDependencyAnalysis(params: any, almanac: Almanac) {
}

export function semverValid(installed: string, required: string): boolean {

// Remove potential @namespace from installed version
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const installedVersion = installed.includes('@') ? installed.split('@').pop()! : installed;
if (!required || !installedVersion) {
installed = installed.includes('@') ? installed.split('@').pop()! : installed;

if (!installed || !required) {
return true;
}

// ensure that both inputs are now valid semver or range strings
if (!semver.valid(installed) && !semver.validRange(installed)) {
logger.error(`semverValid: invalid installed version or range: ${installed}`);
return false;
}
if (!semver.valid(required) && !semver.validRange(required)) {
logger.error(`semverValid: invalid required version or range: ${required}`);
return false;
}

// If 'installed' is a single version and 'required' is a range
if (semver.valid(installed) && semver.validRange(required)) {
logger.debug('range vs version');
logger.info('range vs version');
return semver.satisfies(installed, required);
}

// If 'required' is a single version and 'installed' is a range
if (semver.valid(required) && semver.validRange(installed)) {
logger.debug('version vs range');
logger.info('version vs range');
return semver.satisfies(required, installed);
}

// If both are single versions, simply compare them
if (semver.valid(required) && semver.valid(installed)) {
logger.debug('version vs version');
logger.info('version vs version');
return semver.gt(installed, required);
}

// If both are ranges, check if they intersect
if (semver.validRange(required) && semver.validRange(installed)) {
logger.debug('range vs range');
logger.info('range vs range');
return semver.intersects(required, installed);
}

Expand Down
20 changes: 12 additions & 8 deletions src/facts/repoFilesystemFacts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,25 +60,29 @@ describe('File operations', () => {

describe('isBlacklisted', () => {
it('should return true if file path matches any blacklist pattern', () => {
const filePath = '/node_modules/index.js';
expect(isBlacklisted(filePath, mockArchetypeConfig.config.blacklistPatterns)).toBe(true);
const filePath = '/test/repo/node_modules/index.js';
const repoPath = '/test/repo';
expect(isBlacklisted({ filePath, repoPath, blacklistPatterns: mockArchetypeConfig.config.blacklistPatterns })).toBe(true);
});

it('should return false if file path does not match any blacklist pattern', () => {
const filePath = 'src/index.js';
expect(isBlacklisted(filePath, mockArchetypeConfig.config.blacklistPatterns)).toBe(false);
const filePath = '/test/repo/src/index.js';
const repoPath = '/test/repo';
expect(isBlacklisted({ filePath, repoPath, blacklistPatterns: mockArchetypeConfig.config.blacklistPatterns })).toBe(false);
});
});

describe('isWhitelisted', () => {
it('should return true if file path matches any whitelist pattern', () => {
const filePath = '/src/index.js';
expect(isWhitelisted(filePath, mockArchetypeConfig.config.whitelistPatterns)).toBe(true);
const filePath = '/test/repo/src/index.js';
const repoPath = '/test/repo';
expect(isWhitelisted({ filePath, repoPath, whitelistPatterns: mockArchetypeConfig.config.whitelistPatterns })).toBe(true);
});

it('should return false if file path does not match any whitelist pattern', () => {
const filePath = 'build/index.txt';
expect(isWhitelisted(filePath, mockArchetypeConfig.config.whitelistPatterns)).toBe(false);
const filePath = '/test/repo/build/index.txt';
const repoPath = '/test/repo';
expect(isWhitelisted({ filePath, repoPath, whitelistPatterns: mockArchetypeConfig.config.whitelistPatterns })).toBe(false);
});
});
});
10 changes: 5 additions & 5 deletions src/facts/repoFilesystemFacts.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { logger } from '../utils/logger';
import fs from 'fs';
import path from 'path';
import { ArchetypeConfig } from '../types/typeDefs';
import { ArchetypeConfig, IsBlacklistedParams, isWhitelistedParams } from '../types/typeDefs';

interface FileData {
fileName: string;
Expand Down Expand Up @@ -29,12 +29,12 @@ async function collectRepoFileData(repoPath: string, archetypeConfig: ArchetypeC
logger.debug(`checking file: ${filePath}`);
const stats = await fs.promises.lstat(filePath);
if (stats.isDirectory()) {
if (!isBlacklisted(filePath, repoPath, archetypeConfig.config.blacklistPatterns)) {
if (!isBlacklisted({ filePath, repoPath, blacklistPatterns: archetypeConfig.config.blacklistPatterns })) {
const dirFilesData = await collectRepoFileData(filePath, archetypeConfig);
filesData.push(...dirFilesData);
}
} else {
if (!isBlacklisted(filePath, repoPath, archetypeConfig.config.blacklistPatterns) && isWhitelisted(filePath, repoPath, archetypeConfig.config.whitelistPatterns)) {
if (!isBlacklisted({ filePath, repoPath, blacklistPatterns: archetypeConfig.config.blacklistPatterns }) && isWhitelisted({ filePath, repoPath, whitelistPatterns: archetypeConfig.config.whitelistPatterns })) {
logger.debug(`adding file: ${filePath}`);
const fileData = await parseFile(filePath);
filesData.push(fileData);
Expand All @@ -45,7 +45,7 @@ async function collectRepoFileData(repoPath: string, archetypeConfig: ArchetypeC
return filesData;
}

function isBlacklisted(filePath: string, repoPath: string, blacklistPatterns: string[]): boolean {
function isBlacklisted({ filePath, repoPath, blacklistPatterns }: IsBlacklistedParams): boolean {
logger.debug(`checking blacklist for file: ${filePath}`);
const normalizedPath = path.normalize(filePath);
const normalizedRepoPath = path.normalize(repoPath);
Expand All @@ -64,7 +64,7 @@ function isBlacklisted(filePath: string, repoPath: string, blacklistPatterns: st
return false;
}

function isWhitelisted(filePath: string, repoPath: string, whitelistPatterns: string[]): boolean {
function isWhitelisted({ filePath, repoPath, whitelistPatterns }: isWhitelistedParams): boolean {
logger.debug(`checking whitelist for file: ${filePath}`);
const normalizedPath = path.normalize(filePath);
const normalizedRepoPath = path.normalize(repoPath);
Expand Down
2 changes: 1 addition & 1 deletion src/operators/outdatedFramework.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const outdatedFramework: OperatorDefn = {
let result = false;

try {
logger.debug(`outdatedFramework: processing ${repoDependencyAnalysis}`);
logger.debug(`outdatedFramework: processing ${JSON.stringify(repoDependencyAnalysis)}`);

if (repoDependencyAnalysis?.result?.length > 0) {
return true;
Expand Down
2 changes: 1 addition & 1 deletion src/server/configServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import chokidar from 'chokidar';
import { handleConfigChange } from './cacheManager';

const SHARED_SECRET = process.env.XFI_SHARED_SECRET;
const maskedSecret = SHARED_SECRET ? `${SHARED_SECRET.substring(0, 4)}****${SHARED_SECRET.substring(SHARED_SECRET.length - 4)}` : 'not set';
const maskedSecret = SHARED_SECRET ? `${SHARED_SECRET.substring(0, 1)}****${SHARED_SECRET.substring(SHARED_SECRET.length - 1)}` : 'not set';
logger.info(`Shared secret is ${maskedSecret}`);

const app = express();
Expand Down
3 changes: 2 additions & 1 deletion src/types/typeDefs.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Engine, OperatorEvaluator } from 'json-rules-engine';
import { FileData, isWhitelisted } from '../facts/repoFilesystemFacts';
import { FileData } from '../facts/repoFilesystemFacts';
import { JSONSchemaType } from 'ajv';

export type OperatorDefn = {
Expand Down Expand Up @@ -111,6 +111,7 @@ export interface RuleConfig {
params: Record<string, any>;
};
}
export type RuleConfigSchema = JSONSchemaType<RuleConfig>;

export interface OpenAIAnalysisParams {
prompt: string;
Expand Down
Loading

0 comments on commit 0a3c65c

Please sign in to comment.