Skip to content

Commit 038ba71

Browse files
committed
fix: load bundled default includes outside project root
1 parent 02e893a commit 038ba71

2 files changed

Lines changed: 156 additions & 27 deletions

File tree

src/utils/config-loader.ts

Lines changed: 111 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -325,35 +325,10 @@ export class ConfigLoader {
325325
if (defaultConfigPath) {
326326
// Always log to stderr to avoid contaminating formatted output
327327
console.error(`📦 Loading bundled default configuration from ${defaultConfigPath}`);
328-
const content = fs.readFileSync(defaultConfigPath, 'utf8');
329-
let config = yaml.load(content) as Partial<VisorConfig>;
330-
331-
if (!config || typeof config !== 'object') {
332-
throw new Error('Invalid default configuration');
333-
}
334-
335-
// Alias: support 'include' as 'extends' in packaged defaults
336-
if ((config as any).include && !(config as any).extends) {
337-
const inc = (config as any).include;
338-
(config as any).extends = Array.isArray(inc) ? inc : [inc];
339-
delete (config as any).include;
340-
}
341-
328+
const defaultConfigDir = path.dirname(defaultConfigPath);
329+
let config = await this.fetchBundledConfigFile(defaultConfigPath, defaultConfigDir);
342330
// Normalize 'checks' and 'steps' for backward compatibility
343331
config = this.normalizeStepsAndChecks(config);
344-
345-
// Default configs shouldn't have extends, but handle it just in case
346-
if (config.extends) {
347-
// Ensure relative paths (e.g., ./code-review.yaml) resolve from the defaults directory
348-
const previousBaseDir = this.options.baseDir;
349-
try {
350-
this.options.baseDir = path.dirname(defaultConfigPath);
351-
return await this.processExtends(config);
352-
} finally {
353-
this.options.baseDir = previousBaseDir;
354-
}
355-
}
356-
357332
return config;
358333
}
359334

@@ -372,6 +347,90 @@ export class ConfigLoader {
372347
};
373348
}
374349

350+
/**
351+
* Load a bundled config and its includes without using project-local path resolution.
352+
*
353+
* `extends: default` is a built-in source. Its sibling includes are part of the Visor
354+
* package/action bundle, so they must resolve inside that bundle rather than inside the
355+
* caller's repository root.
356+
*/
357+
private async fetchBundledConfigFile(
358+
filePath: string,
359+
bundledRoot: string,
360+
currentDepth: number = 0,
361+
seen: Set<string> = new Set()
362+
): Promise<Partial<VisorConfig>> {
363+
if (currentDepth >= (this.options.maxDepth || 10)) {
364+
throw new Error(
365+
`Maximum bundled default include depth (${this.options.maxDepth}) exceeded. Check for circular dependencies.`
366+
);
367+
}
368+
369+
const resolvedPath = path.resolve(filePath);
370+
this.validateBundledPath(resolvedPath, bundledRoot);
371+
372+
if (seen.has(resolvedPath)) {
373+
throw new Error(`Circular dependency detected in bundled defaults: ${resolvedPath}`);
374+
}
375+
376+
seen.add(resolvedPath);
377+
378+
try {
379+
const content = fs.readFileSync(resolvedPath, 'utf8');
380+
const config = yaml.load(content) as Partial<VisorConfig>;
381+
382+
if (!config || typeof config !== 'object') {
383+
throw new Error(`Invalid default configuration: ${resolvedPath}`);
384+
}
385+
386+
const extendsValue = (config as any).extends || (config as any).include;
387+
delete (config as any).extends;
388+
delete (config as any).include;
389+
390+
this.annotateToolsBaseDir(config, path.dirname(resolvedPath));
391+
392+
if (!extendsValue) {
393+
return config;
394+
}
395+
396+
const { ConfigMerger } = await import('./config-merger');
397+
const merger = new ConfigMerger();
398+
const sources = Array.isArray(extendsValue) ? extendsValue : [extendsValue];
399+
let mergedParents: Partial<VisorConfig> = {};
400+
401+
for (const source of sources) {
402+
if (typeof source !== 'string' || this.getSourceType(source) !== ConfigSourceType.LOCAL) {
403+
throw new Error(
404+
`Bundled default configuration can only include local bundled files: ${String(source)}`
405+
);
406+
}
407+
408+
const parentPath = path.isAbsolute(source)
409+
? source
410+
: path.resolve(path.dirname(resolvedPath), source);
411+
const parentConfig = await this.fetchBundledConfigFile(
412+
parentPath,
413+
bundledRoot,
414+
currentDepth + 1,
415+
seen
416+
);
417+
mergedParents = merger.merge(mergedParents, parentConfig);
418+
}
419+
420+
return merger.merge(mergedParents, config);
421+
} catch (error: any) {
422+
if (error && (error.code === 'ENOENT' || error.code === 'ENOTDIR')) {
423+
throw new Error(`Bundled default configuration file not found: ${resolvedPath}`);
424+
}
425+
if (error instanceof Error) {
426+
throw error;
427+
}
428+
throw error;
429+
} finally {
430+
seen.delete(resolvedPath);
431+
}
432+
}
433+
375434
/**
376435
* Process extends directive in a configuration
377436
*/
@@ -492,6 +551,31 @@ export class ConfigLoader {
492551
}
493552
}
494553

554+
/**
555+
* Validate that bundled default includes stay inside the Visor package/action bundle.
556+
*/
557+
private validateBundledPath(resolvedPath: string, bundledRoot: string): void {
558+
const canonicalize = (p: string): string => {
559+
const resolved = path.resolve(p);
560+
try {
561+
return path.normalize(fs.realpathSync.native(resolved));
562+
} catch {
563+
return path.normalize(resolved);
564+
}
565+
};
566+
const normalizedPath = canonicalize(resolvedPath);
567+
const normalizedRoot = canonicalize(bundledRoot);
568+
569+
if (
570+
normalizedPath !== normalizedRoot &&
571+
!normalizedPath.startsWith(`${normalizedRoot}${path.sep}`)
572+
) {
573+
throw new Error(
574+
`Security error: Bundled default include resolves outside bundled defaults directory: ${bundledRoot}`
575+
);
576+
}
577+
}
578+
495579
/**
496580
* Find package root directory
497581
*/
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import * as fs from 'fs';
2+
import * as os from 'os';
3+
import * as path from 'path';
4+
import * as yaml from 'js-yaml';
5+
import { ConfigLoader } from '../../src/utils/config-loader';
6+
7+
describe('ConfigLoader bundled default extends', () => {
8+
let projectRoot: string;
9+
10+
beforeEach(() => {
11+
projectRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'visor-project-'));
12+
});
13+
14+
afterEach(() => {
15+
fs.rmSync(projectRoot, { recursive: true, force: true });
16+
});
17+
18+
it('loads bundled default includes when project config extends default outside the package root', async () => {
19+
fs.writeFileSync(
20+
path.join(projectRoot, 'visor.yaml'),
21+
yaml.dump({
22+
extends: 'default',
23+
checks: {
24+
'custom-project-check': {
25+
type: 'ai',
26+
prompt: 'Project-specific check',
27+
on: ['pr_opened'],
28+
},
29+
},
30+
})
31+
);
32+
33+
const loader = new ConfigLoader({
34+
baseDir: projectRoot,
35+
projectRoot,
36+
});
37+
38+
const config = await loader.fetchConfig('./visor.yaml');
39+
40+
expect(config.steps).toBeDefined();
41+
expect(config.steps?.overview).toBeDefined();
42+
expect(config.checks).toBeDefined();
43+
expect(config.checks?.['custom-project-check']).toBeDefined();
44+
});
45+
});

0 commit comments

Comments
 (0)