From 6affe7cf54561d08e618e84842f647271fc9426f Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 1 Aug 2026 07:49:56 -0700 Subject: [PATCH 1/2] fix: harden plugin import-boundary guards and remove dead scanner (#117386) * fix: harden plugin import-boundary enforcement * fix: exclude generated bundles from plugin source guards * fix: make plugin boundary scanner linear and complete --- scripts/check-architecture-smells.d.mts | 18 -- scripts/check-architecture-smells.mjs | 249 --------------- scripts/check-channel-agnostic-boundaries.mjs | 69 ++-- .../check-extension-plugin-sdk-boundary.mjs | 275 ++++++---------- ...check-plugin-extension-import-boundary.mjs | 109 +++---- scripts/check-plugin-sdk-subpath-exports.mjs | 17 +- .../lib/extension-import-boundary-checker.mjs | 59 +++- scripts/lib/guard-inventory-utils.d.mts | 12 +- scripts/lib/guard-inventory-utils.mjs | 299 ++++++++++-------- scripts/lib/ts-guard-utils.d.mts | 4 + scripts/lib/ts-guard-utils.mjs | 13 +- test/architecture-smells.test.ts | 248 +++++++++++++-- .../check-channel-agnostic-boundaries.test.ts | 36 +++ .../extension-import-boundary-checker.test.ts | 131 +++++++- 14 files changed, 783 insertions(+), 756 deletions(-) delete mode 100644 scripts/check-architecture-smells.d.mts delete mode 100644 scripts/check-architecture-smells.mjs diff --git a/scripts/check-architecture-smells.d.mts b/scripts/check-architecture-smells.d.mts deleted file mode 100644 index 477d5ac5a77e..000000000000 --- a/scripts/check-architecture-smells.d.mts +++ /dev/null @@ -1,18 +0,0 @@ -#!/usr/bin/env node -/** - * Collects architecture smell findings from the configured source roots. - */ -export function collectArchitectureSmells(): Promise< - Array<{ - category: string; - file: string; - line: number; - kind: string; - specifier: string; - reason: string; - }> ->; -/** - * Runs the architecture smell check and writes human/JSON output. - */ -export function main(argv: unknown, io: unknown): Promise; diff --git a/scripts/check-architecture-smells.mjs b/scripts/check-architecture-smells.mjs deleted file mode 100644 index 062ea0c3284d..000000000000 --- a/scripts/check-architecture-smells.mjs +++ /dev/null @@ -1,249 +0,0 @@ -#!/usr/bin/env node - -// Finds core/plugin architecture boundary smells in TypeScript sources. -import { promises as fs } from "node:fs"; -import path from "node:path"; -import { fileURLToPath } from "node:url"; -import pMap from "p-map"; -import { BUNDLED_PLUGIN_PATH_PREFIX } from "./lib/bundled-plugin-paths.mjs"; -import { - collectModuleReferencesFromSource, - normalizeRepoPath, - resolveRepoSpecifier, - writeLine, -} from "./lib/guard-inventory-utils.mjs"; -import { - collectTypeScriptFilesFromRoots, - resolveSourceRoots, - runAsScript, -} from "./lib/ts-guard-utils.mjs"; - -const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); -const scanRoots = resolveSourceRoots(repoRoot, ["src/plugin-sdk", "src/plugins/runtime"]); -let architectureSmellsPromise; - -function compareEntries(left, right) { - return ( - left.category.localeCompare(right.category) || - left.file.localeCompare(right.file) || - left.line - right.line || - left.kind.localeCompare(right.kind) || - left.specifier.localeCompare(right.specifier) || - left.reason.localeCompare(right.reason) - ); -} - -function pushEntry(entries, entry) { - entries.push(entry); -} - -function scanPluginSdkExtensionFacadeSmells(source, filePath) { - const relativeFile = normalizeRepoPath(repoRoot, filePath); - if (!relativeFile.startsWith("src/plugin-sdk/")) { - return []; - } - - const entries = []; - - for (const { kind, line, specifier } of collectModuleReferencesFromSource(source)) { - if (kind !== "export") { - continue; - } - const resolvedPath = resolveRepoSpecifier(repoRoot, specifier, filePath); - if (!resolvedPath?.startsWith(BUNDLED_PLUGIN_PATH_PREFIX)) { - continue; - } - pushEntry(entries, { - category: "plugin-sdk-extension-facade", - file: relativeFile, - line, - kind, - specifier, - resolvedPath, - reason: "plugin-sdk public surface re-exports extension-owned implementation", - }); - } - return entries; -} - -function scanRuntimeTypeImplementationSmells(source, filePath) { - const relativeFile = normalizeRepoPath(repoRoot, filePath); - if (!/^src\/plugins\/runtime\/types(?:-[^/]+)?\.ts$/.test(relativeFile)) { - return []; - } - - const entries = []; - - for (const { kind, line, specifier } of collectModuleReferencesFromSource(source)) { - if (kind !== "dynamic-import") { - continue; - } - const resolvedPath = resolveRepoSpecifier(repoRoot, specifier, filePath); - if ( - resolvedPath && - (/^src\/plugins\/runtime\/runtime-[^/]+\.ts$/.test(resolvedPath) || - /^extensions\/[^/]+\/runtime-api\.[^/]+$/.test(resolvedPath)) - ) { - pushEntry(entries, { - category: "runtime-type-implementation-edge", - file: relativeFile, - line, - kind: "import-type", - specifier, - resolvedPath, - reason: "runtime type file references implementation shim directly", - }); - } - } - - return entries; -} - -function scanRuntimeServiceLocatorSmells(source, filePath) { - const relativeFile = normalizeRepoPath(repoRoot, filePath); - if ( - !relativeFile.startsWith("src/plugin-sdk/") && - !relativeFile.startsWith("src/plugins/runtime/") - ) { - return []; - } - - const entries = []; - const exportedNames = new Set(); - const runtimeStoreCalls = []; - const mutableStateNodes = []; - - const lines = source.split(/\r?\n/); - for (const [index, line] of lines.entries()) { - const lineNumber = index + 1; - const exportedFunction = line.match(/^\s*export\s+function\s+([A-Za-z_$][\w$]*)/); - if (exportedFunction) { - exportedNames.add(exportedFunction[1]); - } - const exportedVariable = line.match(/^\s*export\s+(?:const|let|var)\s+([A-Za-z_$][\w$]*)/); - if (exportedVariable) { - exportedNames.add(exportedVariable[1]); - } - for (const mutableMatch of line.matchAll(/^\s*let\s+([A-Za-z_$][\w$]*)/g)) { - mutableStateNodes.push({ line: lineNumber, text: mutableMatch[1] }); - } - if (line.includes("createPluginRuntimeStore")) { - runtimeStoreCalls.push({ line: lineNumber }); - } - } - - const getterNames = [...exportedNames].filter((name) => /^get[A-Z]/.test(name)); - const setterNames = [...exportedNames].filter((name) => /^set[A-Z]/.test(name)); - - if (runtimeStoreCalls.length > 0 && getterNames.length > 0 && setterNames.length > 0) { - for (const callNode of runtimeStoreCalls) { - pushEntry(entries, { - category: "runtime-service-locator", - file: relativeFile, - line: callNode.line, - kind: "runtime-store", - specifier: "createPluginRuntimeStore", - resolvedPath: relativeFile, - reason: `exports paired runtime accessors (${getterNames.join(", ")} / ${setterNames.join(", ")}) over module-global store state`, - }); - } - } - - if (mutableStateNodes.length > 0 && getterNames.length > 0 && setterNames.length > 0) { - for (const identifier of mutableStateNodes) { - pushEntry(entries, { - category: "runtime-service-locator", - file: relativeFile, - line: identifier.line, - kind: "mutable-state", - specifier: identifier.text, - resolvedPath: relativeFile, - reason: `module-global mutable state backs exported runtime accessors (${getterNames.join(", ")} / ${setterNames.join(", ")})`, - }); - } - } - - return entries; -} - -/** - * Collects architecture smell findings from the configured source roots. - */ -export async function collectArchitectureSmells() { - if (!architectureSmellsPromise) { - architectureSmellsPromise = (async () => { - const files = (await collectTypeScriptFilesFromRoots(scanRoots)).toSorted((left, right) => - normalizeRepoPath(repoRoot, left).localeCompare(normalizeRepoPath(repoRoot, right)), - ); - const entriesByFile = await pMap( - files, - async (filePath) => { - const source = await fs.readFile(filePath, "utf8"); - const entries = scanPluginSdkExtensionFacadeSmells(source, filePath); - entries.push(...scanRuntimeTypeImplementationSmells(source, filePath)); - entries.push(...scanRuntimeServiceLocatorSmells(source, filePath)); - return entries; - }, - { concurrency: 32, stopOnError: true }, - ); - return entriesByFile.flat().toSorted(compareEntries); - })(); - try { - return await architectureSmellsPromise; - } catch (error) { - architectureSmellsPromise = undefined; - throw error; - } - } - return await architectureSmellsPromise; -} - -function formatInventoryHuman(inventory) { - if (inventory.length === 0) { - return "No architecture smells found for the configured checks."; - } - - const lines = ["Architecture smell inventory:"]; - let activeCategory = ""; - let activeFile = ""; - for (const entry of inventory) { - if (entry.category !== activeCategory) { - activeCategory = entry.category; - activeFile = ""; - lines.push(entry.category); - } - if (entry.file !== activeFile) { - activeFile = entry.file; - lines.push(` ${activeFile}`); - } - lines.push(` - line ${entry.line} [${entry.kind}] ${entry.reason}`); - lines.push(` specifier: ${entry.specifier}`); - lines.push(` resolved: ${entry.resolvedPath}`); - } - return lines.join("\n"); -} - -async function runArchitectureSmellsCheck(argv, io) { - const args = argv ?? process.argv.slice(2); - const streams = io ?? { stdout: process.stdout, stderr: process.stderr }; - const json = args.includes("--json"); - const inventory = await collectArchitectureSmells(); - - if (json) { - writeLine(streams.stdout, JSON.stringify(inventory, null, 2)); - return 0; - } - - writeLine(streams.stdout, formatInventoryHuman(inventory)); - writeLine(streams.stdout, `${inventory.length} smell${inventory.length === 1 ? "" : "s"} found.`); - return 0; -} - -/** - * Runs the architecture smell check and writes human/JSON output. - */ -export async function main(argv, io) { - return await runArchitectureSmellsCheck(argv, io); -} - -runAsScript(import.meta.url, main); diff --git a/scripts/check-channel-agnostic-boundaries.mjs b/scripts/check-channel-agnostic-boundaries.mjs index 82d9a87b4878..b3a7be44cf54 100644 --- a/scripts/check-channel-agnostic-boundaries.mjs +++ b/scripts/check-channel-agnostic-boundaries.mjs @@ -4,6 +4,7 @@ import { promises as fs } from "node:fs"; import path from "node:path"; import ts from "typescript"; +import { visitModuleSpecifiers } from "./lib/guard-inventory-utils.mjs"; import { collectTypeScriptFiles, getPropertyNameText, @@ -134,51 +135,33 @@ export function findChannelAgnosticBoundaryViolations( const sourceFile = ts.createSourceFile(fileName, content, ts.ScriptTarget.Latest, true); const violations = []; + const moduleViolations = new Map(); + if (checkModuleSpecifiers) { + visitModuleSpecifiers( + ts, + sourceFile, + ({ kind, node, specifier, specifierNode }) => { + if (moduleSpecifierMatcher(specifier)) { + const verb = + kind === "export" + ? "re-exports" + : kind === "dynamic-import" + ? "dynamically imports" + : "imports"; + moduleViolations.set(node, { + line: toLine(sourceFile, specifierNode), + reason: verb + ' channel module "' + specifier + '"', + }); + } + }, + { includeCommonJs: true, includeImportMetaUrl: true, includeImportTypes: true }, + ); + } const visit = (node) => { - if ( - checkModuleSpecifiers && - ts.isImportDeclaration(node) && - ts.isStringLiteral(node.moduleSpecifier) - ) { - const specifier = node.moduleSpecifier.text; - if (moduleSpecifierMatcher(specifier)) { - violations.push({ - line: toLine(sourceFile, node.moduleSpecifier), - reason: `imports channel module "${specifier}"`, - }); - } - } - - if ( - checkModuleSpecifiers && - ts.isExportDeclaration(node) && - node.moduleSpecifier && - ts.isStringLiteral(node.moduleSpecifier) - ) { - const specifier = node.moduleSpecifier.text; - if (moduleSpecifierMatcher(specifier)) { - violations.push({ - line: toLine(sourceFile, node.moduleSpecifier), - reason: `re-exports channel module "${specifier}"`, - }); - } - } - - if ( - checkModuleSpecifiers && - ts.isCallExpression(node) && - node.expression.kind === ts.SyntaxKind.ImportKeyword && - node.arguments.length > 0 && - ts.isStringLiteral(node.arguments[0]) - ) { - const specifier = node.arguments[0].text; - if (moduleSpecifierMatcher(specifier)) { - violations.push({ - line: toLine(sourceFile, node.arguments[0]), - reason: `dynamically imports channel module "${specifier}"`, - }); - } + const moduleViolation = moduleViolations.get(node); + if (moduleViolation) { + violations.push(moduleViolation); } if ( diff --git a/scripts/check-extension-plugin-sdk-boundary.mjs b/scripts/check-extension-plugin-sdk-boundary.mjs index c4324b246de3..a10d3188575b 100644 --- a/scripts/check-extension-plugin-sdk-boundary.mjs +++ b/scripts/check-extension-plugin-sdk-boundary.mjs @@ -3,22 +3,26 @@ // Inventories extension imports to enforce plugin SDK boundary rules. import { promises as fs } from "node:fs"; import path from "node:path"; -import { fileURLToPath } from "node:url"; import { BUNDLED_PLUGIN_PATH_PREFIX, BUNDLED_PLUGIN_ROOT_DIR, } from "./lib/bundled-plugin-paths.mjs"; +import { createExtensionImportBoundaryChecker } from "./lib/extension-import-boundary-checker.mjs"; import { classifyBundledExtensionSourcePath } from "./lib/extension-source-classifier.mjs"; import { - collectModuleReferencesFromSource, diffInventoryEntries, - normalizeRepoPath, + formatGroupedInventoryHuman, resolveRepoSpecifier, writeLine, } from "./lib/guard-inventory-utils.mjs"; +import { listGeneratedExtensionAssetSources } from "./lib/static-extension-assets.mjs"; +import { resolveRepoRoot, runAsScript } from "./lib/ts-guard-utils.mjs"; -const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); -const extensionsRoot = path.join(repoRoot, BUNDLED_PLUGIN_ROOT_DIR); +const repoRoot = resolveRepoRoot(import.meta.url); +// Generated bundles are validated at their build owner; they are not bounded authored source. +const generatedExtensionAssetSources = new Set( + listGeneratedExtensionAssetSources({ rootDir: repoRoot }), +); const MODES = new Set([ "src-outside-plugin-sdk", @@ -42,8 +46,6 @@ const baselinePathByMode = { }; let allInventoryByModePromise; -let extensionModuleReferencesPromise; - const ruleTextByMode = { "src-outside-plugin-sdk": "Rule: production bundled plugins must not import src/** outside src/plugin-sdk/**", @@ -53,82 +55,6 @@ const ruleTextByMode = { "Rule: production bundled plugins must not use relative imports that escape their own package root", }; -function isCodeFile(fileName) { - return /\.(ts|tsx|mts|cts|js|jsx|mjs|cjs)$/.test(fileName); -} - -function isBoundaryCanaryFile(fileName) { - return fileName.includes("__rootdir_boundary_canary__"); -} - -async function collectExtensionSourceFiles(rootDir) { - const out = []; - async function walk(dir) { - const entries = await fs.readdir(dir, { withFileTypes: true }); - for (const entry of entries) { - if (entry.name === "dist" || entry.name === "node_modules") { - continue; - } - const fullPath = path.join(dir, entry.name); - if (entry.isDirectory()) { - await walk(fullPath); - continue; - } - if (!entry.isFile() || !isCodeFile(entry.name) || isBoundaryCanaryFile(entry.name)) { - continue; - } - const relativePath = normalizeRepoPath(repoRoot, fullPath); - if (classifyBundledExtensionSourcePath(relativePath).isTestLike) { - continue; - } - out.push(fullPath); - } - } - await walk(rootDir); - return out.toSorted((left, right) => - normalizeRepoPath(repoRoot, left).localeCompare(normalizeRepoPath(repoRoot, right)), - ); -} - -async function collectExtensionModuleReferences() { - if (!extensionModuleReferencesPromise) { - extensionModuleReferencesPromise = (async () => { - const files = await collectExtensionSourceFiles(extensionsRoot); - const referenced = await Promise.all( - files.map(async (filePath) => { - const source = await fs.readFile(filePath, "utf8"); - if (!mayContainModuleSpecifier(source)) { - return null; - } - return { - filePath, - references: collectModuleReferencesFromSource(source), - }; - }), - ); - return referenced.filter((entry) => entry && entry.references.length > 0); - })(); - } - return await extensionModuleReferencesPromise; -} - -function mayContainModuleSpecifier(source) { - return ( - /\bfrom\s*["']/.test(source) || - /\bimport\s*(?:\(|["']|type\b|[\w*{])/.test(source) || - /\bexport\s*(?:type\s+)?(?:\*|{)[^;\n]*\bfrom\s*["']/.test(source) - ); -} - -function resolveExtensionRoot(filePath) { - const relativePath = normalizeRepoPath(repoRoot, filePath); - const segments = relativePath.split("/"); - if (segments[0] !== BUNDLED_PLUGIN_ROOT_DIR || !segments[1]) { - return null; - } - return `${segments[0]}/${segments[1]}`; -} - function classifyReason(mode, kind, resolvedPath, specifier) { const verb = kind === "export" @@ -168,93 +94,90 @@ function compareEntries(left, right) { ); } -function shouldReport(mode, resolvedPath) { - if (mode === "relative-outside-package") { - return false; - } - if (!resolvedPath?.startsWith("src/")) { - return false; - } - if (mode === "plugin-sdk-internal") { - return resolvedPath.startsWith("src/plugin-sdk-internal/"); - } - return !resolvedPath.startsWith("src/plugin-sdk/"); -} - -function collectEntriesByModeFromModuleReferences(filePath, references) { - const entriesByMode = { - "src-outside-plugin-sdk": [], - "plugin-sdk-internal": [], - "relative-outside-package": [], - }; - const extensionRoot = resolveExtensionRoot(filePath); - const relativeFile = normalizeRepoPath(repoRoot, filePath); - - function push(kind, line, specifier) { +function collectBoundaryEntries({ filePath, relativeFile, references }) { + const extensionRoot = relativeFile.split("/").slice(0, 2).join("/"); + const entries = []; + for (const { kind, line, specifier } of references) { const resolvedPath = resolveRepoSpecifier(repoRoot, specifier, filePath); - const baseEntry = { - file: relativeFile, - line, - kind, - specifier, - resolvedPath, - }; - - if (specifier.startsWith(".") && resolvedPath && extensionRoot) { - if (!(resolvedPath === extensionRoot || resolvedPath.startsWith(`${extensionRoot}/`))) { - entriesByMode["relative-outside-package"].push({ - ...baseEntry, - reason: classifyReason("relative-outside-package", kind, resolvedPath, specifier), - }); - } + const modes = []; + if ( + specifier.startsWith(".") && + resolvedPath !== extensionRoot && + !resolvedPath.startsWith(extensionRoot + "/") + ) { + modes.push("relative-outside-package"); } - - for (const mode of ["src-outside-plugin-sdk", "plugin-sdk-internal"]) { - if (!shouldReport(mode, resolvedPath)) { - continue; - } - entriesByMode[mode].push({ - ...baseEntry, - reason: classifyReason(mode, kind, resolvedPath, specifier), + if (resolvedPath.startsWith("src/") && !resolvedPath.startsWith("src/plugin-sdk/")) { + modes.push("src-outside-plugin-sdk"); + } + if (resolvedPath.startsWith("src/plugin-sdk-internal/")) { + modes.push("plugin-sdk-internal"); + } + for (const mode of modes) { + entries.push({ + mode, + entry: { + file: relativeFile, + line, + kind, + specifier, + resolvedPath, + reason: classifyReason(mode, kind, resolvedPath, specifier), + }, }); } } - - for (const { kind, line, specifier } of references) { - push(kind, line, specifier); - } - return entriesByMode; + return entries; } -/** - * Collects the current extension plugin SDK boundary inventory. - */ +const extensionBoundaryChecker = createExtensionImportBoundaryChecker({ + roots: [BUNDLED_PLUGIN_ROOT_DIR], + sourceOptions: { + fileExtensions: [".ts", ".tsx", ".mts", ".cts", ".js", ".jsx", ".mjs", ".cjs"], + includeTests: true, + skipDirectories: ["dist"], + }, + shouldSkipFile(relativeFile) { + return ( + generatedExtensionAssetSources.has(relativeFile) || + path.basename(relativeFile).includes("__rootdir_boundary_canary__") || + classifyBundledExtensionSourcePath(relativeFile).isTestLike + ); + }, + acceptSpecifier(specifier, { relativeFile, resolvedPath }) { + if (!resolvedPath) { + return false; + } + const extensionRoot = relativeFile.split("/").slice(0, 2).join("/"); + return ( + resolvedPath.startsWith("src/") || + (specifier.startsWith(".") && + resolvedPath !== extensionRoot && + !resolvedPath.startsWith(extensionRoot + "/")) + ); + }, + collectEntries: collectBoundaryEntries, + compareEntries: (left, right) => compareEntries(left.entry, right.entry), +}); + +/** Collect the current extension plugin SDK boundary inventory. */ async function collectExtensionPluginSdkBoundaryInventory(mode) { if (!MODES.has(mode)) { - throw new Error(`Unknown mode: ${mode}`); + throw new Error("Unknown mode: " + mode); } - if (!allInventoryByModePromise) { - allInventoryByModePromise = (async () => { - const files = await collectExtensionModuleReferences(); - const inventoryByMode = { - "src-outside-plugin-sdk": [], - "plugin-sdk-internal": [], - "relative-outside-package": [], - }; - for (const { filePath, references } of files) { - const entriesByMode = collectEntriesByModeFromModuleReferences(filePath, references); - for (const inventoryMode of MODES) { - inventoryByMode[inventoryMode].push(...entriesByMode[inventoryMode]); - } - } - for (const inventoryMode of MODES) { - inventoryByMode[inventoryMode] = inventoryByMode[inventoryMode].toSorted(compareEntries); - } - return inventoryByMode; - })(); - } - const inventoryByMode = await allInventoryByModePromise; - return inventoryByMode[mode]; + allInventoryByModePromise ??= extensionBoundaryChecker + .collectInventory() + .then((entries) => + Object.fromEntries( + [...MODES].map((inventoryMode) => [ + inventoryMode, + entries + .filter(({ mode: entryMode }) => entryMode === inventoryMode) + .map(({ entry }) => entry), + ]), + ), + ); + return (await allInventoryByModePromise)[mode]; } /** @@ -284,25 +207,15 @@ export function diffInventory(expected, actual) { return diffInventoryEntries(expected, actual, compareEntries); } -function formatInventoryHuman(mode, inventory) { - const lines = [ruleTextByMode[mode]]; - if (inventory.length === 0) { - lines.push("No extension plugin-sdk boundary violations found."); - return lines.join("\n"); - } - lines.push("Extension boundary inventory:"); - let activeFile = ""; - for (const entry of inventory) { - if (entry.file !== activeFile) { - activeFile = entry.file; - lines.push(activeFile); - } - lines.push(` - line ${entry.line} [${entry.kind}] ${entry.reason}`); - lines.push(` specifier: ${entry.specifier}`); - lines.push(` resolved: ${entry.resolvedPath}`); - } - return lines.join("\n"); -} +const formatInventoryHuman = (mode, inventory) => + formatGroupedInventoryHuman( + { + rule: ruleTextByMode[mode], + cleanMessage: "No extension plugin-sdk boundary violations found.", + inventoryTitle: "Extension boundary inventory:", + }, + inventory, + ); /** * Runs the boundary inventory check with CLI-style inputs and outputs. @@ -367,6 +280,4 @@ export async function main(argv, io) { return exitCode; } -if (path.resolve(process.argv[1] ?? "") === fileURLToPath(import.meta.url)) { - await main(); -} +runAsScript(import.meta.url, main); diff --git a/scripts/check-plugin-extension-import-boundary.mjs b/scripts/check-plugin-extension-import-boundary.mjs index fa0200e0fbb4..b824675d854d 100644 --- a/scripts/check-plugin-extension-import-boundary.mjs +++ b/scripts/check-plugin-extension-import-boundary.mjs @@ -3,28 +3,17 @@ // Inventories core plugin imports that cross into bundled extension files. import { promises as fs } from "node:fs"; import path from "node:path"; -import { fileURLToPath } from "node:url"; -import ts from "typescript"; -import { BUNDLED_PLUGIN_PATH_PREFIX } from "./lib/bundled-plugin-paths.mjs"; +import { createExtensionImportBoundaryChecker } from "./lib/extension-import-boundary-checker.mjs"; import { - collectTypeScriptInventory, createCachedAsync, diffInventoryEntries, formatGroupedInventoryHuman, - normalizeRepoPath, runBaselineInventoryCheck, resolveRepoSpecifier, - visitModuleSpecifiers, } from "./lib/guard-inventory-utils.mjs"; -import { - collectTypeScriptFilesFromRoots, - resolveSourceRoots, - runAsScript, - toLine, -} from "./lib/ts-guard-utils.mjs"; +import { resolveRepoRoot, runAsScript } from "./lib/ts-guard-utils.mjs"; -const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); -const scanRoots = resolveSourceRoots(repoRoot, ["src/plugins"]); +const repoRoot = resolveRepoRoot(import.meta.url); const baselinePath = path.join( repoRoot, "test", @@ -75,44 +64,18 @@ function classifyResolvedExtensionReason(kind, resolvedPath) { return `${verb} extension-owned file from src/plugins`; } -function pushEntry(entries, entry) { - entries.push(entry); -} - -function scanImportBoundaryViolations(sourceFile, filePath) { - const entries = []; - const relativeFile = normalizeRepoPath(repoRoot, filePath); - - visitModuleSpecifiers(ts, sourceFile, ({ kind, specifier, specifierNode }) => { - const resolvedPath = resolveRepoSpecifier(repoRoot, specifier, filePath); - if (!resolvedPath?.startsWith(BUNDLED_PLUGIN_PATH_PREFIX)) { - return; - } - pushEntry(entries, { - file: relativeFile, - line: toLine(sourceFile, specifierNode), - kind, - specifier, - resolvedPath, - reason: classifyResolvedExtensionReason(kind, resolvedPath), - }); - }); - return entries; -} - -function scanWebSearchRegistrySmells(sourceFile, filePath) { - const relativeFile = normalizeRepoPath(repoRoot, filePath); +function scanWebSearchRegistrySmells(source, relativeFile) { if (relativeFile !== "src/plugins/web-search-providers.ts") { return []; } const entries = []; - const lines = sourceFile.text.split(/\r?\n/); + const lines = source.split(/\r?\n/); for (const [index, line] of lines.entries()) { const lineNumber = index + 1; if (line.includes("web-search-plugin-factory.js")) { - pushEntry(entries, { + entries.push({ file: relativeFile, line: lineNumber, kind: "registry-smell", @@ -124,7 +87,7 @@ function scanWebSearchRegistrySmells(sourceFile, filePath) { const pluginMatch = line.match(/pluginId:\s*"([^"]+)"/); if (pluginMatch && bundledWebSearchPluginIds.has(pluginMatch[1])) { - pushEntry(entries, { + entries.push({ file: relativeFile, line: lineNumber, kind: "registry-smell", @@ -136,7 +99,7 @@ function scanWebSearchRegistrySmells(sourceFile, filePath) { const providerMatch = line.match(/id:\s*"(brave|firecrawl|gemini|grok|kimi|perplexity)"/); if (providerMatch && bundledWebSearchProviders.has(providerMatch[1])) { - pushEntry(entries, { + entries.push({ file: relativeFile, line: lineNumber, kind: "registry-smell", @@ -150,37 +113,37 @@ function scanWebSearchRegistrySmells(sourceFile, filePath) { return entries; } -function shouldSkipFile(filePath) { - const relativeFile = normalizeRepoPath(repoRoot, filePath); - return ( - relativeFile === "src/plugins/bundled-web-search-registry.ts" || - relativeFile.startsWith("src/plugins/contracts/") || - /^src\/plugins\/runtime\/runtime-[^/]+-contract\.[cm]?[jt]s$/u.test(relativeFile) - ); -} - -/** - * Cached inventory of src/plugins imports that cross into bundled extensions. - */ -export const collectPluginExtensionImportBoundaryInventory = createCachedAsync(async () => { - const files = (await collectTypeScriptFilesFromRoots(scanRoots)) - .filter((filePath) => !shouldSkipFile(filePath)) - .toSorted((left, right) => - normalizeRepoPath(repoRoot, left).localeCompare(normalizeRepoPath(repoRoot, right)), +const boundaryChecker = createExtensionImportBoundaryChecker({ + roots: ["src/plugins"], + shouldSkipFile(relativeFile) { + return ( + relativeFile === "src/plugins/bundled-web-search-registry.ts" || + relativeFile.startsWith("src/plugins/contracts/") || + /^src\/plugins\/runtime\/runtime-[^/]+-contract\.[cm]?[jt]s$/u.test(relativeFile) ); - return await collectTypeScriptInventory({ - ts, - files, - compareEntries, - collectEntries(sourceFile, filePath) { - return [ - ...scanImportBoundaryViolations(sourceFile, filePath), - ...scanWebSearchRegistrySmells(sourceFile, filePath), - ]; - }, - }); + }, + collectEntries({ source, filePath, relativeFile, references }) { + return [ + ...references.map(({ kind, line, specifier }) => { + const resolvedPath = resolveRepoSpecifier(repoRoot, specifier, filePath); + return { + file: relativeFile, + line, + kind, + specifier, + resolvedPath, + reason: classifyResolvedExtensionReason(kind, resolvedPath), + }; + }), + ...scanWebSearchRegistrySmells(source, relativeFile), + ]; + }, + compareEntries, }); +/** Cached inventory of src/plugins imports that cross into bundled extensions. */ +export const collectPluginExtensionImportBoundaryInventory = boundaryChecker.collectInventory; + /** * Cached expected plugin-extension import inventory baseline. */ diff --git a/scripts/check-plugin-sdk-subpath-exports.mjs b/scripts/check-plugin-sdk-subpath-exports.mjs index 1d6c1faec286..1a09eb8e157d 100644 --- a/scripts/check-plugin-sdk-subpath-exports.mjs +++ b/scripts/check-plugin-sdk-subpath-exports.mjs @@ -48,11 +48,7 @@ function readPrivateLocalOnlySubpaths() { } function parsePluginSdkSubpath(specifier) { - if (!specifier.startsWith("openclaw/plugin-sdk/")) { - return null; - } - const subpath = specifier.slice("openclaw/plugin-sdk/".length); - return subpath || null; + return specifier.match(/^@?openclaw\/plugin-sdk\/(.+)$/u)?.[1] ?? null; } function isGeneratedBuildArtifact(filePath) { @@ -153,9 +149,14 @@ async function collectViolations() { }); } - visitModuleSpecifiers(ts, sourceFile, ({ kind, node, specifier, specifierNode }) => { - push(kind, node, specifierNode, specifier); - }); + visitModuleSpecifiers( + ts, + sourceFile, + ({ kind, node, specifier, specifierNode }) => { + push(kind, node, specifierNode, specifier); + }, + { includeCommonJs: true, includeImportTypes: true }, + ); } return violations.toSorted(compareEntries); diff --git a/scripts/lib/extension-import-boundary-checker.mjs b/scripts/lib/extension-import-boundary-checker.mjs index faa33fb50dff..2d42ea90ccef 100644 --- a/scripts/lib/extension-import-boundary-checker.mjs +++ b/scripts/lib/extension-import-boundary-checker.mjs @@ -18,6 +18,26 @@ import { const repoRoot = resolveRepoRoot(import.meta.url); const DEFAULT_BOUNDARY_SOURCE_MAX_BYTES = 2 * 1024 * 1024; +// Escaped plugin paths must reach the scanner without lexing every unrelated escaped source. +const ESCAPED_BUNDLED_PLUGIN_PATH_PREFIX_RE = new RegExp( + Array.from(BUNDLED_PLUGIN_PATH_PREFIX, (character) => { + const hex = character.charCodeAt(0).toString(16).padStart(2, "0"); + return ( + "(?:" + + character + + "|\\\\(?:x" + + hex + + "|u00" + + hex + + "|u\\{0*" + + hex + + "\\}|" + + character + + "))(?:\\\\(?:\\r\\n|[\\r\\n\\u2028\\u2029]))*" + ); + }).join(""), + "iu", +); function compareEntries(left, right) { return ( @@ -39,13 +59,11 @@ function classifyResolvedExtensionReason(kind, boundaryLabel) { return `${verb} bundled plugin file from ${boundaryLabel} boundary`; } -function scanImportBoundaryViolations(source, filePath, boundaryLabel, allowResolvedPath) { +function scanImportBoundaryViolations(references, filePath, boundaryLabel, allowResolvedPath) { const entries = []; const relativeFile = normalizeRepoPath(repoRoot, filePath); - for (const reference of collectModuleReferencesFromSource(source)) { - const kind = reference.kind; - const specifier = reference.specifier; + for (const { kind, line, specifier } of references) { const resolvedPath = resolveRepoSpecifier(repoRoot, specifier, filePath); if (!resolvedPath?.startsWith(BUNDLED_PLUGIN_PATH_PREFIX)) { continue; @@ -55,7 +73,7 @@ function scanImportBoundaryViolations(source, filePath, boundaryLabel, allowReso } entries.push({ file: relativeFile, - line: reference.line, + line, kind, specifier, resolvedPath, @@ -96,7 +114,7 @@ export function createExtensionImportBoundaryChecker(params) { const maxSourceBytes = normalizeMaxSourceBytes(params.maxSourceBytes); const collectInventory = createCachedAsync(async () => { - const files = (await collectTypeScriptFilesFromRoots(scanRoots)) + const files = (await collectTypeScriptFilesFromRoots(scanRoots, params.sourceOptions)) .filter((filePath) => !params.shouldSkipFile?.(normalizeRepoPath(repoRoot, filePath))) .toSorted((left, right) => normalizeRepoPath(repoRoot, left).localeCompare(normalizeRepoPath(repoRoot, right)), @@ -105,23 +123,36 @@ export function createExtensionImportBoundaryChecker(params) { files, async (filePath) => { const source = await readBoundedSourceFile(filePath, maxSourceBytes); + const relativeFile = normalizeRepoPath(repoRoot, filePath); if ( params.skipSourcesWithoutBundledPluginPrefix && - !source.includes(BUNDLED_PLUGIN_PATH_PREFIX) + !source.includes(BUNDLED_PLUGIN_PATH_PREFIX) && + (!source.includes("\\") || !ESCAPED_BUNDLED_PLUGIN_PATH_PREFIX_RE.test(source)) ) { return []; } - return scanImportBoundaryViolations( - source, - filePath, - params.boundaryLabel, - params.allowResolvedPath, - ); + const references = collectModuleReferencesFromSource(source, { + fileName: filePath, + acceptSpecifier(specifier) { + const resolvedPath = resolveRepoSpecifier(repoRoot, specifier, filePath); + return params.acceptSpecifier + ? params.acceptSpecifier(specifier, { filePath, relativeFile, resolvedPath }) + : Boolean(resolvedPath?.startsWith(BUNDLED_PLUGIN_PATH_PREFIX)); + }, + }); + return params.collectEntries + ? params.collectEntries({ source, filePath, relativeFile, references }) + : scanImportBoundaryViolations( + references, + filePath, + params.boundaryLabel, + params.allowResolvedPath, + ); }, { concurrency: 32, stopOnError: true }, ); const inventory = entriesByFile.flat(); - return inventory.toSorted(compareEntries); + return inventory.toSorted(params.compareEntries ?? compareEntries); }); async function main(argv, io) { diff --git a/scripts/lib/guard-inventory-utils.d.mts b/scripts/lib/guard-inventory-utils.d.mts index eabf3858238d..a6b939f23598 100644 --- a/scripts/lib/guard-inventory-utils.d.mts +++ b/scripts/lib/guard-inventory-utils.d.mts @@ -14,6 +14,7 @@ export function visitModuleSpecifiers( options?: { includeCommonJs?: boolean; includeImportMetaUrl?: boolean; + includeImportTypes?: boolean; }, ): void; /** Diff expected and actual inventory entries using JSON identity. */ @@ -27,8 +28,15 @@ export function diffInventoryEntries( }; /** Write one line to a stream without each caller repeating newline handling. */ export function writeLine(stream: unknown, text: unknown): void; -/** Collect import/export/dynamic-import references from source text without full parsing. */ -export function collectModuleReferencesFromSource(source: unknown): unknown[]; +/** Lexically reject clean files before parsing candidate module-boundary violations. */ +export function collectModuleReferencesFromSource( + source: string, + options?: { + acceptSpecifier?: (specifier: string) => boolean; + fileName?: string; + ts?: unknown; + }, +): Array<{ kind: string; line: number; specifier: string }>; /** Memoize an async factory while resetting the cache after failures. */ export function createCachedAsync(factory: unknown): () => Promise; /** Format grouped inventory entries for human-readable guard output. */ diff --git a/scripts/lib/guard-inventory-utils.mjs b/scripts/lib/guard-inventory-utils.mjs index 08acf6628d5d..19fc18a4407d 100644 --- a/scripts/lib/guard-inventory-utils.mjs +++ b/scripts/lib/guard-inventory-utils.mjs @@ -1,9 +1,12 @@ // Shared parsing, diffing, and reporting helpers for inventory guard scripts. import { promises as fs } from "node:fs"; +import { createRequire } from "node:module"; import path from "node:path"; const parsedTypeScriptSourceCache = new Map(); const sourceTextCache = new Map(); +const require = createRequire(import.meta.url); +let cachedTypeScript; /** Convert an absolute file path to a repo-relative POSIX path. */ export function normalizeRepoPath(repoRoot, filePath) { @@ -24,50 +27,53 @@ export function resolveRepoSpecifier(repoRoot, specifier, importerFile) { /** Visit static and dynamic module specifiers in a parsed TypeScript source file. */ export function visitModuleSpecifiers(ts, sourceFile, visit, options = {}) { function walk(node) { + let kind; + let specifierNode; if (ts.isImportDeclaration(node) && ts.isStringLiteral(node.moduleSpecifier)) { - visit({ - kind: "import", - node, - specifier: node.moduleSpecifier.text, - specifierNode: node.moduleSpecifier, - }); + kind = "import"; + specifierNode = node.moduleSpecifier; } else if ( ts.isExportDeclaration(node) && node.moduleSpecifier && ts.isStringLiteral(node.moduleSpecifier) ) { - visit({ - kind: "export", - node, - specifier: node.moduleSpecifier.text, - specifierNode: node.moduleSpecifier, - }); + kind = "export"; + specifierNode = node.moduleSpecifier; } else if ( ts.isCallExpression(node) && node.expression.kind === ts.SyntaxKind.ImportKeyword && - node.arguments.length === 1 && - ts.isStringLiteral(node.arguments[0]) + node.arguments.length >= 1 && + ts.isStringLiteralLike(node.arguments[0]) ) { - visit({ - kind: "dynamic-import", - node, - specifier: node.arguments[0].text, - specifierNode: node.arguments[0], - }); + kind = "dynamic-import"; + specifierNode = node.arguments[0]; + } else if ( + options.includeImportTypes && + ts.isImportTypeNode(node) && + ts.isLiteralTypeNode(node.argument) && + ts.isStringLiteralLike(node.argument.literal) + ) { + kind = "dynamic-import"; + specifierNode = node.argument.literal; } else if ( options.includeCommonJs && ts.isCallExpression(node) && ts.isIdentifier(node.expression) && node.expression.text === "require" && - node.arguments.length === 1 && + node.arguments.length >= 1 && ts.isStringLiteralLike(node.arguments[0]) ) { - visit({ - kind: "commonjs-require", - node, - specifier: node.arguments[0].text, - specifierNode: node.arguments[0], - }); + kind = "commonjs-require"; + specifierNode = node.arguments[0]; + } else if ( + options.includeCommonJs && + ts.isImportEqualsDeclaration(node) && + ts.isExternalModuleReference(node.moduleReference) && + node.moduleReference.expression && + ts.isStringLiteralLike(node.moduleReference.expression) + ) { + kind = "commonjs-require"; + specifierNode = node.moduleReference.expression; } else if ( options.includeImportMetaUrl && ts.isNewExpression(node) && @@ -81,14 +87,13 @@ export function visitModuleSpecifiers(ts, sourceFile, visit, options = {}) { node.arguments[1].expression.keywordToken === ts.SyntaxKind.ImportKeyword && node.arguments[1].expression.name.text === "meta" ) { - visit({ - kind: "import-meta-url", - node, - specifier: node.arguments[0].text, - specifierNode: node.arguments[0], - }); + kind = "import-meta-url"; + specifierNode = node.arguments[0]; } + if (specifierNode) { + visit({ kind, node, specifier: specifierNode.text, specifierNode }); + } ts.forEachChild(node, walk); } @@ -114,44 +119,135 @@ export function writeLine(stream, text) { stream.write(`${text}\n`); } -/** Collect import/export/dynamic-import references from source text without full parsing. */ -export function collectModuleReferencesFromSource(source) { - const lineStarts = computeLineStarts(source); - const isCodePosition = createCodePositionChecker(source); - const references = []; - const push = (kind, specifier, position, syntaxPosition) => { - if (!isCodePosition(syntaxPosition)) { - return; - } - references.push({ - kind, - line: lineFromPosition(lineStarts, position), - specifier, - }); +function hasSupplementalModuleReference(ts, source, acceptSpecifier) { + const checkUrl = source.includes("new") && source.includes("import") && source.includes("meta"); + const interpolationStart = source.indexOf("${"); + const checkRequire = + interpolationStart >= 0 && + (source.includes("require") || source.indexOf("\\u", interpolationStart + 2) >= 0); + const interpolatedSlash = + interpolationStart < 0 ? -1 : source.indexOf("/", interpolationStart + 2); + const checkTemplateImport = + interpolatedSlash >= 0 && source.indexOf("import", interpolatedSlash + 1) >= 0; + const checkNamespaceExport = + source.includes("export") && source.includes("*") && source.includes("as"); + if (!checkUrl && !checkRequire && !checkTemplateImport && !checkNamespaceExport) { + return false; + } + + const scanner = ts.createScanner( + ts.ScriptTarget.Latest, + true, + ts.LanguageVariant.Standard, + source, + ); + const kinds = ts.SyntaxKind; + const templateBraceDepths = []; + const scanAcceptedSpecifier = () => { + const token = scanner.scan(); + return ( + (token === kinds.StringLiteral || token === kinds.NoSubstitutionTemplateLiteral) && + acceptSpecifier(scanner.getTokenValue()) + ); }; - for (const match of source.matchAll(/\bimport\s*\(\s*(["'])([^"']+)\1/g)) { - push("dynamic-import", match[2], match.index + match[0].lastIndexOf(match[1]), match.index); + for (let token = scanner.scan(); token !== kinds.EndOfFileToken; token = scanner.scan()) { + if ( + checkUrl && + token === kinds.NewKeyword && + scanner.lookAhead( + () => + scanner.scan() === kinds.Identifier && + scanner.getTokenValue() === "URL" && + scanner.scan() === kinds.OpenParenToken && + scanAcceptedSpecifier(), + ) + ) { + return true; + } + if ( + checkRequire && + (token === kinds.RequireKeyword || + (token === kinds.Identifier && scanner.getTokenValue() === "require")) && + scanner.lookAhead(() => scanner.scan() === kinds.OpenParenToken && scanAcceptedSpecifier()) + ) { + return true; + } + if ( + checkNamespaceExport && + token === kinds.ExportKeyword && + scanner.lookAhead(() => { + let next = scanner.scan(); + if (next === kinds.TypeKeyword) { + next = scanner.scan(); + } + return next === kinds.AsteriskToken && scanner.scan() === kinds.AsKeyword; + }) + ) { + return true; + } + + // A context-free slash may start a regexp whose `}` would corrupt interpolation tracking. + if ( + templateBraceDepths.length > 0 && + (token === kinds.SlashToken || token === kinds.SlashEqualsToken) + ) { + return true; + } + + // Resume after each nested interpolation; otherwise template tails swallow later imports. + if (token === kinds.TemplateHead) { + templateBraceDepths.push(0); + } else if (token === kinds.OpenBraceToken && templateBraceDepths.length > 0) { + templateBraceDepths[templateBraceDepths.length - 1] += 1; + } else if (token === kinds.CloseBraceToken && templateBraceDepths.length > 0) { + const index = templateBraceDepths.length - 1; + if (templateBraceDepths[index] > 0) { + templateBraceDepths[index] -= 1; + } else if (scanner.reScanTemplateToken(false) === kinds.TemplateTail) { + templateBraceDepths.pop(); + } + } } - for (const match of source.matchAll(/^\s*import\s*(["'])([^"']+)\1/gm)) { - push( - "import", - match[2], - match.index + match[0].lastIndexOf(match[1]), - match.index + match[0].indexOf("import"), - ); - } - for (const match of source.matchAll( - /^\s*(import|export)\s+(?:type\s+)?[^;"']*?\bfrom\s*(["'])([^"']+)\2/gm, - )) { - push( - match[1], - match[3], - match.index + match[0].lastIndexOf(match[2]), - match.index + match[0].indexOf(match[1]), - ); + + return false; +} + +/** Lexically reject clean files before parsing candidate module-boundary violations. */ +export function collectModuleReferencesFromSource(source, options = {}) { + const ts = options.ts ?? (cachedTypeScript ??= require("typescript")); + const acceptSpecifier = options.acceptSpecifier ?? (() => true); + const candidates = ts.preProcessFile(source, true, true).importedFiles; + if ( + !candidates.some(({ fileName }) => acceptSpecifier(fileName)) && + !hasSupplementalModuleReference(ts, source, acceptSpecifier) + ) { + return []; } + const sourceFile = ts.createSourceFile( + options.fileName ?? "source.ts", + source, + ts.ScriptTarget.Latest, + true, + ); + const references = []; + visitModuleSpecifiers( + ts, + sourceFile, + ({ kind, specifier, specifierNode }) => { + if (acceptSpecifier(specifier)) { + references.push({ + kind, + line: + sourceFile.getLineAndCharacterOfPosition(specifierNode.getStart(sourceFile)).line + 1, + specifier, + }); + } + }, + { includeCommonJs: true, includeImportMetaUrl: true, includeImportTypes: true }, + ); + return references.toSorted( (left, right) => left.line - right.line || @@ -160,77 +256,6 @@ export function collectModuleReferencesFromSource(source) { ); } -function createCodePositionChecker(source) { - const codePositions = new Uint8Array(source.length); - - for (let index = 0; index < source.length; index += 1) { - const char = source[index]; - const next = source[index + 1]; - - if (char === "/" && next === "/") { - index += 2; - while (index < source.length && source.charCodeAt(index) !== 10) { - index += 1; - } - index -= 1; - continue; - } - - if (char === "/" && next === "*") { - index += 2; - while (index < source.length && !(source[index] === "*" && source[index + 1] === "/")) { - index += 1; - } - index += 1; - continue; - } - - if (char === "'" || char === '"' || char === "`") { - const quote = char; - index += 1; - while (index < source.length) { - if (source[index] === "\\") { - index += 2; - continue; - } - if (source[index] === quote) { - break; - } - index += 1; - } - continue; - } - - codePositions[index] = 1; - } - - return (position) => codePositions[position] === 1; -} - -function computeLineStarts(source) { - const lineStarts = [0]; - for (let index = 0; index < source.length; index += 1) { - if (source.charCodeAt(index) === 10) { - lineStarts.push(index + 1); - } - } - return lineStarts; -} - -function lineFromPosition(lineStarts, position) { - let low = 0; - let high = lineStarts.length - 1; - while (low <= high) { - const middle = Math.floor((low + high) / 2); - if (lineStarts[middle] <= position) { - low = middle + 1; - } else { - high = middle - 1; - } - } - return high + 1; -} - /** Memoize an async factory while resetting the cache after failures. */ export function createCachedAsync(factory) { let cachedPromise = null; diff --git a/scripts/lib/ts-guard-utils.d.mts b/scripts/lib/ts-guard-utils.d.mts index c6d126efa5df..7e66bbd319a8 100644 --- a/scripts/lib/ts-guard-utils.d.mts +++ b/scripts/lib/ts-guard-utils.d.mts @@ -13,8 +13,10 @@ export function collectTypeScriptFiles( targetPath: string, options?: { extraTestSuffixes?: string[]; + fileExtensions?: string[]; ignoreMissing?: boolean; includeTests?: boolean; + skipDirectories?: string[]; skipNodeModules?: boolean; }, ): Promise; @@ -25,7 +27,9 @@ export function collectTypeScriptFilesFromRoots( sourceRoots: string[], options?: { extraTestSuffixes?: string[]; + fileExtensions?: string[]; includeTests?: boolean; + skipDirectories?: string[]; skipNodeModules?: boolean; }, ): Promise; diff --git a/scripts/lib/ts-guard-utils.mjs b/scripts/lib/ts-guard-utils.mjs index 02ffad269c6a..e355f0d5e160 100644 --- a/scripts/lib/ts-guard-utils.mjs +++ b/scripts/lib/ts-guard-utils.mjs @@ -49,10 +49,14 @@ function isTestLikeTypeScriptFile(filePath, options = {}) { * Recursively collects TypeScript files under a file or directory target. */ export async function collectTypeScriptFiles(targetPath, options = {}) { + const fileExtensions = options.fileExtensions ?? [".ts"]; const includeTests = options.includeTests ?? false; const extraTestSuffixes = options.extraTestSuffixes ?? []; const skipNodeModules = options.skipNodeModules ?? true; + const skipDirectories = options.skipDirectories ?? []; const ignoreMissing = options.ignoreMissing ?? false; + const isSourceFile = (filePath) => + fileExtensions.some((extension) => filePath.endsWith(extension)); let stat; try { @@ -71,7 +75,7 @@ export async function collectTypeScriptFiles(targetPath, options = {}) { } if (stat.isFile()) { - if (!targetPath.endsWith(".ts")) { + if (!isSourceFile(targetPath)) { return []; } if (!includeTests && isTestLikeTypeScriptFile(targetPath, { extraTestSuffixes })) { @@ -85,13 +89,16 @@ export async function collectTypeScriptFiles(targetPath, options = {}) { for (const entry of entries) { const entryPath = path.join(targetPath, entry.name); if (entry.isDirectory()) { - if (skipNodeModules && entry.name === "node_modules") { + if ( + (skipNodeModules && entry.name === "node_modules") || + skipDirectories.includes(entry.name) + ) { continue; } out.push(...(await collectTypeScriptFiles(entryPath, options))); continue; } - if (!entry.isFile() || !entryPath.endsWith(".ts")) { + if (!entry.isFile() || !isSourceFile(entryPath)) { continue; } if (!includeTests && isTestLikeTypeScriptFile(entryPath, { extraTestSuffixes })) { diff --git a/test/architecture-smells.test.ts b/test/architecture-smells.test.ts index 98717fcc451f..46c8ed2c60a4 100644 --- a/test/architecture-smells.test.ts +++ b/test/architecture-smells.test.ts @@ -1,33 +1,229 @@ -// Architecture smell tests flag unwanted dependency and layout patterns. +// Architecture boundary tests cover hostile module-specifier syntax in the canonical scanner. import { describe, expect, it } from "vitest"; -import { collectArchitectureSmells, main } from "../scripts/check-architecture-smells.mjs"; -import { createCapturedIo } from "./helpers/captured-io.js"; +import { collectModuleReferencesFromSource } from "../scripts/lib/guard-inventory-utils.mjs"; -const smellsPromise = collectArchitectureSmells(); +const forbiddenPath = "../../src/private.js"; +const templateQuote = String.fromCharCode(96); -describe("architecture smell inventory", () => { - it("produces stable sorted output", async () => { - const smells = await smellsPromise; +describe("architecture boundary module reference scanner", () => { + it.each([ + { + name: "commented side-effect import", + source: 'import /* gap */ "../../src/private.js"', + kind: "import", + }, + { + name: "commented dynamic import with attributes", + source: 'await import /* gap */ ("../../src/private.js", { with: { type: "json" } })', + kind: "dynamic-import", + }, + { + name: "dynamic import inside template interpolation", + source: + "const message = " + + templateQuote + + "$" + + '{await import("../../src/private.js")}' + + templateQuote, + kind: "dynamic-import", + }, + { + name: "dynamic import after a regexp brace inside template interpolation", + source: 'const message = `${/}/.test("safe") ? import("../../src/private.js") : null}`', + kind: "dynamic-import", + }, + { + name: "commented dynamic import after a regexp character-class brace", + source: + 'const message = `${/[}]/.test("safe") ? import /* gap */ ("../../src/private.js", { with: { type: "json" } }) : null}`', + kind: "dynamic-import", + }, + { + name: "CommonJS require", + source: 'require("../../src/private.js")', + kind: "commonjs-require", + }, + { + name: "CommonJS require inside template interpolation", + source: + "const message = " + + templateQuote + + "$" + + '{require("../../src/private.js")}' + + templateQuote, + kind: "commonjs-require", + }, + { + name: "CommonJS require inside nested template interpolation", + source: 'const message = `outer ${`inner ${require("../../src/private.js")}`} tail`', + kind: "commonjs-require", + }, + { + name: "CommonJS require with a Unicode-escaped first character inside interpolation", + source: 'const message = `${\\u0072equire("../../src/private.js")}`', + kind: "commonjs-require", + }, + { + name: "CommonJS require with a Unicode-escaped middle character inside interpolation", + source: 'const message = `${r\\u0065quire("../../src/private.js")}`', + kind: "commonjs-require", + }, + { + name: "CommonJS require with a Unicode-escaped braced character inside interpolation", + source: 'const message = `${\\u{72}equire("../../src/private.js")}`', + kind: "commonjs-require", + }, + { + name: "CommonJS require with a Unicode-escaped last character inside interpolation", + source: 'const message = `${requir\\u0065("../../src/private.js")}`', + kind: "commonjs-require", + }, + { + name: "CommonJS require after a regexp brace inside template interpolation", + source: 'const message = `${/}/.test("safe") ? require("../../src/private.js") : null}`', + kind: "commonjs-require", + }, + { + name: "CommonJS require after a regexp character-class brace inside interpolation", + source: 'const message = `${/[}]/.test("safe") ? require("../../src/private.js") : null}`', + kind: "commonjs-require", + }, + { + name: "CommonJS require after division inside template interpolation", + source: 'const message = `${(24 / 2) ? require("../../src/private.js") : null}`', + kind: "commonjs-require", + }, + { + name: "TypeScript import-equals require", + source: 'import privateModule = require("../../src/private.js")', + kind: "commonjs-require", + }, + { + name: "import.meta URL with comments", + source: 'new URL("../../src/private.js", import /* gap */ . meta . url)', + kind: "import-meta-url", + }, + { + name: "import.meta URL with escaped constructor", + source: 'new \\u0055RL("../../src/private.js", import.meta.url)', + kind: "import-meta-url", + }, + { + name: "import.meta URL with a template specifier", + source: "new URL(`../../src/private.js`, import.meta.url)", + kind: "import-meta-url", + }, + { + name: "import.meta URL following a completed template interpolation", + source: + 'const message = `prefix ${"allowed"} tail`; new URL("../../src/private.js", import.meta.url)', + kind: "import-meta-url", + }, + { + name: "import.meta URL after a regexp brace inside template interpolation", + source: + 'const message = `${/}/.test("safe") ? new URL("../../src/private.js", import.meta.url) : null}`', + kind: "import-meta-url", + }, + { + name: "runtime namespace re-export", + source: 'export /* gap */ * /* gap */ as privateModule from "../../src/private.js"', + kind: "export", + }, + { + name: "type namespace re-export", + source: 'export type * as privateModule from "../../src/private.js"', + kind: "export", + }, + { + name: "namespace re-export following nested template interpolations", + source: + 'const message = `outer ${`inner ${1}`} tail`; export * as privateModule from "../../src/private.js"', + kind: "export", + }, + { + name: "TypeScript import type", + source: 'type PrivateModule = typeof import("../../src/private.js")', + kind: "dynamic-import", + }, + { + name: "escaped module specifier", + source: 'import "../../src/priv\\u0061te.js"', + kind: "import", + }, + ])("detects $name", ({ source, kind }) => { + expect( + collectModuleReferencesFromSource(source, { + acceptSpecifier: (specifier) => specifier === forbiddenPath, + }), + ).toEqual([{ kind, line: 1, specifier: forbiddenPath }]); + }); + + it("ignores comments, string contents, and allowed module specifiers", () => { + expect( + collectModuleReferencesFromSource( + [ + '// import "../../src/private.js"', + 'const text = "require(\\\"../../src/private.js\\\")";', + 'import "../../src/allowed.js";', + ].join("\n"), + { acceptSpecifier: (specifier) => specifier === forbiddenPath }, + ), + ).toEqual([]); + }); + + it.each([ + 'const message = `${(24 / 2) ? require("../../src/allowed.js") : null}`', + 'const message = `${/}/.test("safe") ? require("../../src/allowed.js") : null}`', + 'const message = `${\\u0072equire("../../src/allowed.js")}`', + 'const message = `${/}/.test("safe") ? import("../../src/allowed.js") : null}`', + ])("keeps ordinary division and regexp fallback free of false positives", (source) => { + expect( + collectModuleReferencesFromSource(source, { + acceptSpecifier: (specifier) => specifier === forbiddenPath, + }), + ).toEqual([]); + }); + + it("preserves the actual source line for multiline guarded imports", () => { + expect( + collectModuleReferencesFromSource('\n\nimport /* comment */ "../../src/private.js";', { + acceptSpecifier: (specifier) => specifier === forbiddenPath, + }), + ).toEqual([{ kind: "import", line: 3, specifier: forbiddenPath }]); + }); + + it.each([ + { + name: "import.meta URL", + createSource: (comments: string) => + "new /*" + comments + '! URL("../../src/allowed.js", import.meta.url)', + }, + { + name: "embedded CommonJS require", + createSource: (comments: string) => + "const message = " + + templateQuote + + "$" + + "{require /*" + + comments + + '! ("../../src/allowed.js")}' + + templateQuote, + }, + { + name: "namespace re-export", + createSource: (comments: string) => + "export /*" + comments + '! * as privateModule from "../../src/allowed.js"', + }, + ])("scans hostile repeated comments in linear time for $name", ({ createSource }) => { + const source = createSource("*//*".repeat(8_192)); + const startedAt = performance.now(); expect( - [...smells].toSorted( - (left, right) => - left.category.localeCompare(right.category) || - left.file.localeCompare(right.file) || - left.line - right.line || - left.kind.localeCompare(right.kind) || - left.specifier.localeCompare(right.specifier) || - left.reason.localeCompare(right.reason), - ), - ).toEqual(smells); - }); - - it("script json output matches the collector", async () => { - const captured = createCapturedIo(); - const exitCode = await main(["--json"], captured.io); - - expect(exitCode).toBe(0); - expect(captured.readStderr()).toBe(""); - expect(JSON.parse(captured.readStdout())).toEqual(await smellsPromise); + collectModuleReferencesFromSource(source, { + acceptSpecifier: (specifier) => specifier === forbiddenPath, + }), + ).toEqual([]); + expect(performance.now() - startedAt).toBeLessThan(1_000); }); }); diff --git a/test/scripts/check-channel-agnostic-boundaries.test.ts b/test/scripts/check-channel-agnostic-boundaries.test.ts index ff9cd8482a21..4e16c08c1673 100644 --- a/test/scripts/check-channel-agnostic-boundaries.test.ts +++ b/test/scripts/check-channel-agnostic-boundaries.test.ts @@ -125,4 +125,40 @@ describe("check-channel-agnostic-boundaries", () => { `; expect(findSystemMarkLiteralViolations(source)).toStrictEqual([]); }); + + it.each([ + { + name: "commented dynamic import with attributes", + source: 'await import /* gap */ ("../discord/private.js", { with: { type: "json" } });', + reason: 'dynamically imports channel module "../discord/private.js"', + }, + { + name: "CommonJS require", + source: 'require("../telegram/private.js");', + reason: 'imports channel module "../telegram/private.js"', + }, + { + name: "import.meta URL", + source: 'new URL("../slack/private.js", import.meta.url);', + reason: 'imports channel module "../slack/private.js"', + }, + { + name: "TypeScript import type", + source: 'type PrivateModule = typeof import("../signal/private.js");', + reason: 'dynamically imports channel module "../signal/private.js"', + }, + ])("flags $name in protected channel-independent sources", ({ source, reason }) => { + expect(findChannelAgnosticBoundaryViolations(source)).toEqual([{ line: 1, reason }]); + }); + + it("preserves source order when imports and config paths share a line", () => { + expect( + findChannelAgnosticBoundaryViolations( + 'import "../telegram/private.js"; const enabled = cfg.channels.discord;', + ), + ).toEqual([ + { line: 1, reason: 'imports channel module "../telegram/private.js"' }, + { line: 1, reason: 'references config path "channels.discord"' }, + ]); + }); }); diff --git a/test/scripts/extension-import-boundary-checker.test.ts b/test/scripts/extension-import-boundary-checker.test.ts index 25468188c82c..779258c3eb43 100644 --- a/test/scripts/extension-import-boundary-checker.test.ts +++ b/test/scripts/extension-import-boundary-checker.test.ts @@ -1,9 +1,10 @@ // Extension import boundary checker tests cover bounded source reads. -import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { createExtensionImportBoundaryChecker } from "../../scripts/lib/extension-import-boundary-checker.mjs"; +import { listGeneratedExtensionAssetSources } from "../../scripts/lib/static-extension-assets.mjs"; const tempDirs: string[] = []; @@ -36,4 +37,132 @@ describe("extension import boundary checker", () => { "extension import boundary source file exceeds 32 byte limit", ); }); + + it("skips declared generated bundles without admitting oversized handwritten JavaScript", async () => { + const root = makeTempRoot(); + const pluginRoot = path.join(root, "extensions", "generated-plugin"); + const assetRoot = path.join(pluginRoot, "assets"); + const generatedPath = path.join(assetRoot, "generated.js"); + const handwrittenPath = path.join(assetRoot, "handwritten.js"); + const oversizedBytes = 2 * 1024 * 1024 + 1; + mkdirSync(assetRoot, { recursive: true }); + writeFileSync( + path.join(pluginRoot, "package.json"), + JSON.stringify({ + openclaw: { + assetScripts: { build: "node generate.mjs" }, + build: { + staticAssets: [{ source: "./assets/generated.js", output: "assets/generated.js" }], + }, + }, + }), + "utf8", + ); + + // Match CI ordering: the build materializes its declared bundle before source guards run. + writeFileSync(generatedPath, "/".repeat(oversizedBytes), "utf8"); + const generatedSources = new Set( + listGeneratedExtensionAssetSources({ rootDir: root }).map((source) => + path.relative(process.cwd(), path.join(root, source)).replaceAll(path.sep, "/"), + ), + ); + const createChecker = () => + createExtensionImportBoundaryChecker({ + boundaryLabel: "test", + cleanMessage: "clean", + inventoryTitle: "inventory", + roots: [path.relative(process.cwd(), path.join(root, "extensions"))], + shouldSkipFile: (relativeFile: string) => generatedSources.has(relativeFile), + sourceOptions: { fileExtensions: [".js"] }, + }); + + await expect(createChecker().collectInventory()).resolves.toEqual([]); + + const targetPath = path.join(process.cwd(), "extensions", "security-proof", "private.js"); + const specifier = path.relative(assetRoot, targetPath).replaceAll(path.sep, "/"); + writeFileSync( + handwrittenPath, + "import " + JSON.stringify(specifier) + ";\n" + "/".repeat(oversizedBytes), + "utf8", + ); + await expect(createChecker().collectInventory()).rejects.toThrow( + "extension import boundary source file exceeds 2097152 byte limit", + ); + }); + + it.each([ + { + name: "comment-obscured side-effect import", + createSource: (specifier: string) => "import /* gap */ " + JSON.stringify(specifier), + kind: "import", + }, + { + name: "dynamic import with attributes", + createSource: (specifier: string) => + "import /* gap */ (" + JSON.stringify(specifier) + ', { with: { type: "json" } })', + kind: "dynamic-import", + }, + { + name: "CommonJS require", + createSource: (specifier: string) => "require(" + JSON.stringify(specifier) + ")", + kind: "commonjs-require", + }, + { + name: "import.meta URL", + createSource: (specifier: string) => + "new URL(" + JSON.stringify(specifier) + ", import.meta.url)", + kind: "import-meta-url", + }, + { + name: "comment-obscured namespace export", + createSource: (specifier: string) => + "export /* gap */ * /* gap */ as privateModule from " + JSON.stringify(specifier), + kind: "export", + }, + { + name: "escaped plugin-path separator", + createSource: (specifier: string) => + "import " + JSON.stringify(specifier).replace("extensions/", "extensions\\/"), + kind: "import", + }, + { + name: "Unicode-escaped plugin-path character", + createSource: (specifier: string) => + "import " + JSON.stringify(specifier).replace("extensions/", "exten\\u0073ions/"), + kind: "import", + }, + { + name: "hex-escaped plugin-path character", + createSource: (specifier: string) => + "import " + JSON.stringify(specifier).replace("extensions/", "\\x65xtensions/"), + kind: "import", + }, + { + name: "plugin-path line continuation", + createSource: (specifier: string) => + "import " + JSON.stringify(specifier).replace("extensions/", "exten\\\nsions/"), + kind: "import", + }, + ])("rejects $name crossing the real plugin boundary", async ({ createSource, kind }) => { + const root = makeTempRoot(); + const sourcePath = path.join(root, "guarded.ts"); + const targetPath = path.join(process.cwd(), "extensions", "security-proof", "private.js"); + const specifier = path.relative(root, targetPath).split(path.sep).join("/"); + writeFileSync(sourcePath, createSource(specifier), "utf8"); + const checker = createExtensionImportBoundaryChecker({ + boundaryLabel: "test", + cleanMessage: "clean", + inventoryTitle: "inventory", + roots: [path.relative(process.cwd(), root)], + skipSourcesWithoutBundledPluginPrefix: true, + }); + + await expect(checker.collectInventory()).resolves.toEqual([ + expect.objectContaining({ + kind, + resolvedPath: "extensions/security-proof/private.js", + specifier, + }), + ]); + }); }); From 5ad7c969b820d8e5e76c0e9fa7005a6e2f82262c Mon Sep 17 00:00:00 2001 From: nocodet888-arch <0668000903@xydigit.com> Date: Sat, 1 Aug 2026 22:50:24 +0800 Subject: [PATCH 2/2] fix(gateway): trim audit.list exact-match filter ids (#110847) --- src/gateway/server-methods/audit.test.ts | 23 +++++ .../server-methods/audit.trim.store.test.ts | 84 +++++++++++++++++++ src/gateway/server-methods/audit.ts | 19 +++-- 3 files changed, 120 insertions(+), 6 deletions(-) create mode 100644 src/gateway/server-methods/audit.trim.store.test.ts diff --git a/src/gateway/server-methods/audit.test.ts b/src/gateway/server-methods/audit.test.ts index 22aca053b8f3..b50866dfd862 100644 --- a/src/gateway/server-methods/audit.test.ts +++ b/src/gateway/server-methods/audit.test.ts @@ -208,4 +208,27 @@ describe("audit gateway methods", () => { expect(listAuditEvents).toHaveBeenCalledWith(expect.objectContaining({ cursor: 11 })); }, ); + + it.each(["audit.list", "audit.activity.list"] as const)( + "trims exact-match filter ids for %s before store lookup", + async (method) => { + const respond = await runAuditHandler(method, { + agentId: " main ", + sessionKey: " agent:main:main ", + runId: " run-1 ", + }); + + expect(respond).toHaveBeenCalledWith(true, expect.anything()); + expect(listAuditEvents).toHaveBeenCalledWith( + expect.objectContaining({ + filters: expect.objectContaining({ + agentId: "main", + sessionKey: "agent:main:main", + runId: "run-1", + ...(method === "audit.activity.list" ? { includeMessages: true } : {}), + }), + }), + ); + }, + ); }); diff --git a/src/gateway/server-methods/audit.trim.store.test.ts b/src/gateway/server-methods/audit.trim.store.test.ts new file mode 100644 index 000000000000..372905b83dae --- /dev/null +++ b/src/gateway/server-methods/audit.trim.store.test.ts @@ -0,0 +1,84 @@ +import { expectDefined } from "@openclaw/normalization-core"; +import { afterAll, afterEach, describe, expect, it, vi } from "vitest"; +import { cleanupTempDirs, makeTempDir } from "../../../test/helpers/temp-dir.js"; +import { listAuditEvents, recordAuditEvent } from "../../audit/audit-event-store.js"; +import { + closeOpenClawStateDatabaseForTest, + type OpenClawStateDatabaseOptions, +} from "../../state/openclaw-state-db.js"; +import { auditHandlers } from "./audit.js"; + +const tempDirs: string[] = []; + +function createDatabaseOptions(): OpenClawStateDatabaseOptions { + return { env: { OPENCLAW_STATE_DIR: makeTempDir(tempDirs, "openclaw-audit-trim-") } }; +} + +afterEach(() => { + closeOpenClawStateDatabaseForTest(); + delete process.env.OPENCLAW_STATE_DIR; +}); + +afterAll(() => { + cleanupTempDirs(tempDirs); +}); + +describe("audit.list padded filters against a real audit store", () => { + it("returns the planted run for padded agentId/runId filters", async () => { + const database = createDatabaseOptions(); + const stateDir = expectDefined(database.env?.OPENCLAW_STATE_DIR, "temp state dir"); + process.env.OPENCLAW_STATE_DIR = stateDir; + + recordAuditEvent( + { + sourceId: "audit-trim-run-source", + sourceSequence: 1, + occurredAt: Date.now(), + kind: "agent_run", + action: "agent.run.finished", + status: "succeeded", + actorType: "agent", + actorId: "main", + agentId: "main", + sessionKey: "agent:main:main", + runId: "run-trim-1", + }, + database, + ); + + // Negative control: untrimmed filter values miss the planted row at the store. + expect( + listAuditEvents({ + limit: 20, + filters: { agentId: " main ", runId: " run-trim-1 " }, + database, + }).events, + ).toEqual([]); + + const respond = vi.fn(); + await expectDefined( + auditHandlers["audit.list"], + "audit.list handler", + )({ + params: { + agentId: " main ", + sessionKey: " agent:main:main ", + runId: " run-trim-1 ", + }, + respond, + } as never); + + expect(respond).toHaveBeenCalledWith( + true, + expect.objectContaining({ + events: [ + expect.objectContaining({ + agentId: "main", + runId: "run-trim-1", + action: "agent.run.finished", + }), + ], + }), + ); + }); +}); diff --git a/src/gateway/server-methods/audit.ts b/src/gateway/server-methods/audit.ts index 325540a16f63..314608a1d484 100644 --- a/src/gateway/server-methods/audit.ts +++ b/src/gateway/server-methods/audit.ts @@ -1,4 +1,5 @@ // Metadata-only operator audit queries over the canonical shared SQLite ledger. +import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce"; import { ErrorCodes, errorShape, @@ -90,13 +91,16 @@ export const auditHandlers: GatewayRequestHandlers = { ); return; } + const agentId = normalizeOptionalString(params.agentId); + const sessionKey = normalizeOptionalString(params.sessionKey); + const runId = normalizeOptionalString(params.runId); const page = listAuditEvents({ limit: Math.min(params.limit ?? DEFAULT_AUDIT_LIST_LIMIT, MAX_AUDIT_LIST_LIMIT), ...(parsed.cursor !== undefined ? { cursor: parsed.cursor } : {}), filters: { - ...(params.agentId ? { agentId: params.agentId } : {}), - ...(params.sessionKey ? { sessionKey: params.sessionKey } : {}), - ...(params.runId ? { runId: params.runId } : {}), + ...(agentId ? { agentId } : {}), + ...(sessionKey ? { sessionKey } : {}), + ...(runId ? { runId } : {}), ...(params.kind ? { kind: params.kind } : {}), ...(params.status ? { status: params.status } : {}), ...(params.after !== undefined ? { after: params.after } : {}), @@ -128,14 +132,17 @@ export const auditHandlers: GatewayRequestHandlers = { ); return; } + const agentId = normalizeOptionalString(params.agentId); + const sessionKey = normalizeOptionalString(params.sessionKey); + const runId = normalizeOptionalString(params.runId); const page = listAuditEvents({ limit: Math.min(params.limit ?? DEFAULT_AUDIT_LIST_LIMIT, MAX_AUDIT_LIST_LIMIT), ...(parsed.cursor !== undefined ? { cursor: parsed.cursor } : {}), filters: { includeMessages: true, - ...(params.agentId ? { agentId: params.agentId } : {}), - ...(params.sessionKey ? { sessionKey: params.sessionKey } : {}), - ...(params.runId ? { runId: params.runId } : {}), + ...(agentId ? { agentId } : {}), + ...(sessionKey ? { sessionKey } : {}), + ...(runId ? { runId } : {}), ...(params.kind ? { kind: params.kind } : {}), ...(params.status ? { status: params.status } : {}), ...(params.direction ? { direction: params.direction } : {}),