From 775c544c8c1e441b3946c2b3c8e18476040e5775 Mon Sep 17 00:00:00 2001 From: Rhuan Barreto Date: Sat, 27 Jun 2026 16:31:43 -0300 Subject: [PATCH] fix: handle parse errors in rule scanner and loader gracefully Bun.Transpiler.transformSync() and import() throw AggregateError when .rules.ts files contain syntax errors. These were unhandled, causing archgate check to crash instead of reporting the file as blocked. Wrap transpiler, parser, and dynamic import calls in try-catch blocks and return structured violation results so the error surfaces as a check failure rather than an unhandled exception. Fixes Sentry issue 130690158. Signed-off-by: Rhuan Barreto --- src/engine/loader.ts | 32 ++++++++++++- src/engine/rule-scanner.ts | 79 +++++++++++++++++++++++++++++-- tests/engine/loader.test.ts | 26 ++++++++++ tests/engine/rule-scanner.test.ts | 26 ++++++++++ 4 files changed, 158 insertions(+), 5 deletions(-) diff --git a/src/engine/loader.ts b/src/engine/loader.ts index 284d22df..ce24893e 100644 --- a/src/engine/loader.ts +++ b/src/engine/loader.ts @@ -309,7 +309,37 @@ export async function loadRuleAdrs( } // Use file:// URL to handle Windows backslash paths in import(). - const mod = await import(pathToFileURL(rulesFile).href); + let mod: Record; + try { + mod = await import(pathToFileURL(rulesFile).href); + } catch (err) { + // Bun throws AggregateError with "Parse error" for files with syntax + // errors that pass the transpiler-based scanner but fail the full + // import parser. Surface the first inner error for a useful message. + const msg = + err instanceof AggregateError && err.errors.length > 0 + ? String(err.errors[0]) + : err instanceof Error + ? err.message + : String(err); + return { + type: "blocked", + value: { + adr, + error: `ADR ${adr.frontmatter.id}: failed to import companion rule file — ${msg}`, + violations: [ + { + message: `Failed to load rule file: ${msg}`, + file: rulesFile, + line: 1, + column: 0, + endLine: 1, + endColumn: 0, + }, + ], + }, + }; + } const parsed = RuleSetSchema.safeParse(mod.default); if (!parsed.success) { diff --git a/src/engine/rule-scanner.ts b/src/engine/rule-scanner.ts index d236c8f0..7183556b 100644 --- a/src/engine/rule-scanner.ts +++ b/src/engine/rule-scanner.ts @@ -46,8 +46,45 @@ import { remapViolations, type RawViolation } from "./source-positions"; */ export function scanRuleSource(source: string): ScanViolation[] { const transpiler = new Bun.Transpiler({ loader: "ts" }); - const js = transpiler.transformSync(source); - const ast = parseModule(js, { next: true, loc: true, module: true }); + let js: string; + try { + js = transpiler.transformSync(source); + } catch (err) { + // Bun.Transpiler throws AggregateError for syntax errors in the source. + // Return a single violation pointing at line 1 so the caller can report + // the file as blocked rather than crashing. + const msg = + err instanceof AggregateError && err.errors.length > 0 + ? String(err.errors[0]) + : err instanceof Error + ? err.message + : String(err); + return [ + { + message: `Parse error: ${msg}`, + line: 1, + column: 0, + endLine: 1, + endColumn: 0, + }, + ]; + } + + let ast: ReturnType; + try { + ast = parseModule(js, { next: true, loc: true, module: true }); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + return [ + { + message: `Parse error: ${msg}`, + line: 1, + column: 0, + endLine: 1, + endColumn: 0, + }, + ]; + } const rawViolations: RawViolation[] = []; /** Track how many times each searchText has been seen, to match by occurrence. */ @@ -204,8 +241,42 @@ const IMPORTED_BLOCKED_GLOBALS = new Set(["require", "WebSocket"]); */ export function scanImportedRuleSource(source: string): ScanViolation[] { const transpiler = new Bun.Transpiler({ loader: "ts" }); - const js = transpiler.transformSync(source); - const ast = parseModule(js, { next: true, loc: true, module: true }); + let js: string; + try { + js = transpiler.transformSync(source); + } catch (err) { + const msg = + err instanceof AggregateError && err.errors.length > 0 + ? String(err.errors[0]) + : err instanceof Error + ? err.message + : String(err); + return [ + { + message: `Parse error: ${msg}`, + line: 1, + column: 0, + endLine: 1, + endColumn: 0, + }, + ]; + } + + let ast: ReturnType; + try { + ast = parseModule(js, { next: true, loc: true, module: true }); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + return [ + { + message: `Parse error: ${msg}`, + line: 1, + column: 0, + endLine: 1, + endColumn: 0, + }, + ]; + } const rawViolations: RawViolation[] = []; const seenCounts = new Map(); diff --git a/tests/engine/loader.test.ts b/tests/engine/loader.test.ts index 8a910332..d49b71d5 100644 --- a/tests/engine/loader.test.ts +++ b/tests/engine/loader.test.ts @@ -205,6 +205,32 @@ export default { expect(blocked.value.violations).toHaveLength(2); }); + test("returns blocked result when .rules.ts has syntax errors", async () => { + const adrsDir = join(tempDir, ".archgate", "adrs"); + copyFileSync( + join(fixturesDir, "TEST-001-sample.md"), + join(adrsDir, "TEST-001-sample.md") + ); + writeFileSync( + join(adrsDir, "TEST-001-sample.rules.ts"), + `/// + +export default { invalid syntax here !!! } satisfies RuleSet; +` + ); + + const results = await loadRuleAdrs(tempDir); + expect(results).toHaveLength(1); + expect(results[0].type).toBe("blocked"); + const blocked = results[0] as Extract< + (typeof results)[0], + { type: "blocked" } + >; + expect(blocked.value.error).toContain("security scanner"); + expect(blocked.value.violations.length).toBeGreaterThanOrEqual(1); + expect(blocked.value.violations[0].message).toContain("Parse error"); + }); + test("loads ADRs from custom directory configured in config.json", async () => { // Create a custom ADR directory outside .archgate/ const customAdrsDir = join(tempDir, "docs", "adrs"); diff --git a/tests/engine/rule-scanner.test.ts b/tests/engine/rule-scanner.test.ts index d72681b9..5682687f 100644 --- a/tests/engine/rule-scanner.test.ts +++ b/tests/engine/rule-scanner.test.ts @@ -254,6 +254,23 @@ describe("scanRuleSource", () => { }); }); + describe("parse error handling", () => { + test("returns violation instead of throwing for unparseable source", () => { + const source = `export default { invalid syntax here !!! }`; + const violations = scanRuleSource(source); + expect(violations).toHaveLength(1); + expect(violations[0].message).toContain("Parse error"); + expect(violations[0].line).toBe(1); + }); + + test("returns violation for completely broken TypeScript", () => { + const source = `<<<<<<< HEAD\nconst x = 1;\n=======\nconst x = 2;\n>>>>>>> branch`; + const violations = scanRuleSource(source); + expect(violations.length).toBeGreaterThanOrEqual(1); + expect(violations[0].message).toContain("Parse error"); + }); + }); + // Position remapping tests are in rule-scanner-positions.test.ts }); @@ -403,6 +420,15 @@ export default { }); }); + describe("parse error handling", () => { + test("returns violation instead of throwing for unparseable source", () => { + const source = `export default { invalid syntax here !!! }`; + const violations = scanImportedRuleSource(source); + expect(violations.length).toBeGreaterThanOrEqual(1); + expect(violations[0].message).toContain("Parse error"); + }); + }); + describe("safe module imports remain allowed", () => { const safeModules = ["node:path", "node:url", "node:util", "node:crypto"];