mirror of
https://github.com/getpaseo/paseo.git
synced 2026-10-27 19:52:49 +00:00
Guard daemon modules against synchronous Node IO (#7991)
* chore(server): guard against synchronous Node IO in daemon modules * chore(server): retire unused sync IO exceptions automatically * chore(server): remove the attachment IO exception after async materialization
This commit is contained in:
@@ -34,9 +34,130 @@ const noRawPressable = {
|
||||
},
|
||||
};
|
||||
|
||||
const SYNC_IO_MODULES = new Set(["fs", "node:fs", "child_process", "node:child_process"]);
|
||||
|
||||
function propertyName(node) {
|
||||
return node.type === "Identifier" ? node.name : node.value;
|
||||
}
|
||||
|
||||
function findDefinition(node, sourceCode) {
|
||||
let scope = sourceCode.getScope(node);
|
||||
while (scope && !scope.set.has(node.name)) scope = scope.upper;
|
||||
return scope?.set.get(node.name)?.defs[0];
|
||||
}
|
||||
|
||||
function isIoNamespaceImport(definition) {
|
||||
if (definition?.node.type === "TSImportEqualsDeclaration") {
|
||||
return SYNC_IO_MODULES.has(definition.node.moduleReference.expression?.value);
|
||||
}
|
||||
if (definition?.type !== "ImportBinding") return false;
|
||||
return (
|
||||
(definition.node.type !== "ImportSpecifier" ||
|
||||
propertyName(definition.node.imported) === "default") &&
|
||||
SYNC_IO_MODULES.has(definition.parent.source.value)
|
||||
);
|
||||
}
|
||||
|
||||
// Follow only immutable namespace aliases. Resolve bindings rather than spelling so a parameter
|
||||
// named `fs` is not mistaken for the imported namespace.
|
||||
function isIoNamespace(node, sourceCode, seen = new Set()) {
|
||||
if (!node || seen.has(node)) return false;
|
||||
seen.add(node);
|
||||
if (node.type === "AwaitExpression") return isIoNamespace(node.argument, sourceCode, seen);
|
||||
if (node.type === "ImportExpression") return SYNC_IO_MODULES.has(node.source.value);
|
||||
if (node.type === "CallExpression") {
|
||||
return (
|
||||
node.callee.type === "Identifier" &&
|
||||
node.callee.name === "require" &&
|
||||
SYNC_IO_MODULES.has(node.arguments[0]?.value)
|
||||
);
|
||||
}
|
||||
if (node.type !== "Identifier") return false;
|
||||
const definition = findDefinition(node, sourceCode);
|
||||
if (isIoNamespaceImport(definition)) return true;
|
||||
return (
|
||||
definition?.type === "Variable" &&
|
||||
definition.parent.kind === "const" &&
|
||||
definition.node.id.type === "Identifier" &&
|
||||
isIoNamespace(definition.node.init, sourceCode, seen)
|
||||
);
|
||||
}
|
||||
|
||||
const noSyncIo = {
|
||||
meta: {
|
||||
schema: [
|
||||
{
|
||||
type: "object",
|
||||
properties: {
|
||||
allow: {
|
||||
type: "object",
|
||||
additionalProperties: { type: "array", items: { type: "string" } },
|
||||
},
|
||||
},
|
||||
additionalProperties: false,
|
||||
},
|
||||
],
|
||||
},
|
||||
create(context) {
|
||||
const filename = context.filename.replaceAll("\\", "/");
|
||||
const relativeFile = filename.split("/packages/server/src/")[1];
|
||||
const allowed = new Set(context.options[0]?.allow?.[relativeFile] ?? []);
|
||||
const unused = new Set(allowed);
|
||||
function check(node, name) {
|
||||
if (typeof name !== "string" || !name.endsWith("Sync")) return;
|
||||
if (allowed.has(name)) {
|
||||
unused.delete(name);
|
||||
return;
|
||||
}
|
||||
context.report({
|
||||
node,
|
||||
message: `Synchronous Node IO (${name}) blocks the daemon event loop. Use an asynchronous capability at the owning module boundary.`,
|
||||
});
|
||||
}
|
||||
return {
|
||||
"Program:exit"(node) {
|
||||
for (const name of unused) {
|
||||
context.report({ node, message: `Remove the unused sync IO exception for ${name}.` });
|
||||
}
|
||||
},
|
||||
ImportDeclaration(node) {
|
||||
if (node.importKind === "type" || !SYNC_IO_MODULES.has(node.source.value)) return;
|
||||
for (const specifier of node.specifiers) {
|
||||
if (specifier.type === "ImportSpecifier" && specifier.importKind !== "type") {
|
||||
check(specifier, propertyName(specifier.imported));
|
||||
}
|
||||
}
|
||||
},
|
||||
ExportNamedDeclaration(node) {
|
||||
if (node.exportKind === "type" || !SYNC_IO_MODULES.has(node.source?.value)) return;
|
||||
for (const specifier of node.specifiers) {
|
||||
if (specifier.exportKind !== "type") check(specifier, propertyName(specifier.local));
|
||||
}
|
||||
},
|
||||
MemberExpression(node) {
|
||||
if (node.computed && node.property.type !== "Literal") return;
|
||||
const name = propertyName(node.property);
|
||||
if (typeof name !== "string" || !name.endsWith("Sync")) return;
|
||||
if (isIoNamespace(node.object, context.sourceCode)) {
|
||||
check(node.property, name);
|
||||
}
|
||||
},
|
||||
VariableDeclarator(node) {
|
||||
if (node.id.type !== "ObjectPattern" || !isIoNamespace(node.init, context.sourceCode)) {
|
||||
return;
|
||||
}
|
||||
for (const property of node.id.properties) {
|
||||
if (property.type === "Property") check(property, propertyName(property.key));
|
||||
}
|
||||
},
|
||||
};
|
||||
},
|
||||
};
|
||||
|
||||
export default {
|
||||
meta: { name: "paseo" },
|
||||
rules: {
|
||||
"no-raw-pressable": noRawPressable,
|
||||
"no-sync-io": noSyncIo,
|
||||
},
|
||||
};
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { spawnSync } from "node:child_process";
|
||||
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
import { test } from "node:test";
|
||||
@@ -10,15 +10,21 @@ const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "../
|
||||
const oxlintBin = path.join(repoRoot, "node_modules/.bin/oxlint");
|
||||
const pluginPath = path.join(repoRoot, "scripts/oxlint/paseo-plugin.mjs");
|
||||
|
||||
function lint(source) {
|
||||
function lint(source, rule = "paseo/no-raw-pressable", options, filename = "probe.tsx") {
|
||||
const config = JSON.stringify({
|
||||
jsPlugins: [pluginPath],
|
||||
rules: { [rule]: options ? ["error", options] : "error" },
|
||||
});
|
||||
return lintWithConfig(source, config, filename);
|
||||
}
|
||||
|
||||
function lintWithConfig(source, config, filename) {
|
||||
const dir = mkdtempSync(path.join(tmpdir(), "paseo-oxlint-"));
|
||||
try {
|
||||
writeFileSync(
|
||||
path.join(dir, ".oxlintrc.json"),
|
||||
JSON.stringify({ jsPlugins: [pluginPath], rules: { "paseo/no-raw-pressable": "error" } }),
|
||||
);
|
||||
writeFileSync(path.join(dir, "probe.tsx"), source);
|
||||
const result = spawnSync(oxlintBin, ["-c", ".oxlintrc.json", "probe.tsx"], {
|
||||
writeFileSync(path.join(dir, ".oxlintrc.json"), config);
|
||||
mkdirSync(path.dirname(path.join(dir, filename)), { recursive: true });
|
||||
writeFileSync(path.join(dir, filename), source);
|
||||
const result = spawnSync(oxlintBin, ["-c", ".oxlintrc.json", filename], {
|
||||
cwd: dir,
|
||||
encoding: "utf8",
|
||||
});
|
||||
@@ -52,3 +58,95 @@ test("press primitives and pressable types are allowed", () => {
|
||||
);
|
||||
assert.equal(result.failed, false, result.output);
|
||||
});
|
||||
|
||||
for (const source of [
|
||||
'import { readFileSync as read } from "node:fs"; export { read };',
|
||||
'import { execSync } from "child_process"; export { execSync };',
|
||||
'import fs from "node:fs"; export const read = fs.readFileSync;',
|
||||
'import { default as fs } from "node:fs"; export const read = fs.readFileSync;',
|
||||
'import fs = require("node:fs"); export const read = fs.readFileSync;',
|
||||
'import * as fs from "fs"; export const read = fs["readFileSync"];',
|
||||
'import cp from "node:child_process"; export const run = cp.spawnSync;',
|
||||
'const { readFileSync: read } = require("node:fs"); export { read };',
|
||||
'const fs = require("fs"); export const read = fs.readFileSync;',
|
||||
'export const run = require("node:child_process").execFileSync;',
|
||||
'import fs from "node:fs"; const other = fs; export const read = other.statSync;',
|
||||
'export { readFileSync as read } from "node:fs";',
|
||||
'export const read = (await import("node:fs")).readFileSync;',
|
||||
]) {
|
||||
test(`sync IO is reported: ${source}`, () => {
|
||||
const result = lint(source, "paseo/no-sync-io");
|
||||
assert.equal(result.failed, true, result.output);
|
||||
assert.match(result.output, /Synchronous Node IO/);
|
||||
});
|
||||
}
|
||||
|
||||
test("async IO, types, unrelated methods and shadowed namespaces are allowed", () => {
|
||||
const result = lint(
|
||||
'import fs from "node:fs"; import { readFile } from "node:fs/promises";\n' +
|
||||
'import type { readFileSync } from "node:fs";\n' +
|
||||
"export const read = readFile; export const asyncRead = fs.promises.readFile;\n" +
|
||||
"export function unrelated(fs: { readFileSync(): void }) { fs.readFileSync(); }\n" +
|
||||
"export type Reader = typeof readFileSync;\n",
|
||||
"paseo/no-sync-io",
|
||||
);
|
||||
assert.equal(result.failed, false, result.output);
|
||||
});
|
||||
|
||||
test("sync IO exceptions allow only named APIs in the named server module", () => {
|
||||
const options = { allow: { "server/startup.ts": ["readFileSync"] } };
|
||||
const source = 'import { readFileSync } from "node:fs"; export { readFileSync };';
|
||||
const allowed = lint(
|
||||
source,
|
||||
"paseo/no-sync-io",
|
||||
options,
|
||||
"packages/server/src/server/startup.ts",
|
||||
);
|
||||
assert.equal(allowed.failed, false, allowed.output);
|
||||
const otherFile = lint(
|
||||
source,
|
||||
"paseo/no-sync-io",
|
||||
options,
|
||||
"packages/server/src/server/session.ts",
|
||||
);
|
||||
assert.equal(otherFile.failed, true, otherFile.output);
|
||||
assert.match(otherFile.output, /Synchronous Node IO/);
|
||||
const otherApi = lint(
|
||||
'import fs from "node:fs"; export const read = fs.statSync;',
|
||||
"paseo/no-sync-io",
|
||||
options,
|
||||
"packages/server/src/server/startup.ts",
|
||||
);
|
||||
assert.equal(otherApi.failed, true, otherApi.output);
|
||||
assert.match(otherApi.output, /Synchronous Node IO/);
|
||||
});
|
||||
|
||||
test("repository config guards request paths while allowing startup APIs and test fixtures", () => {
|
||||
const config = readFileSync(path.join(repoRoot, ".oxlintrc.json"), "utf8").replace(
|
||||
'"./scripts/oxlint/paseo-plugin.mjs"',
|
||||
JSON.stringify(pluginPath),
|
||||
);
|
||||
const source = 'import { existsSync } from "node:fs"; export const probe = existsSync;';
|
||||
const request = lintWithConfig(source, config, "packages/server/src/server/probe.ts");
|
||||
assert.equal(request.failed, true, request.output);
|
||||
assert.match(request.output, /Synchronous Node IO/);
|
||||
for (const filename of [
|
||||
"packages/server/src/server/config.ts",
|
||||
"packages/server/src/server/probe.test.ts",
|
||||
"packages/app/src/probe.ts",
|
||||
]) {
|
||||
const result = lintWithConfig(source, config, filename);
|
||||
assert.equal(result.failed, false, result.output);
|
||||
}
|
||||
});
|
||||
|
||||
test("sync IO exceptions must be removed when a module stops using the API", () => {
|
||||
const result = lint(
|
||||
'import { readFile } from "node:fs/promises"; export const read = readFile;',
|
||||
"paseo/no-sync-io",
|
||||
{ allow: { "server/startup.ts": ["readFileSync"] } },
|
||||
"packages/server/src/server/startup.ts",
|
||||
);
|
||||
assert.equal(result.failed, true, result.output);
|
||||
assert.match(result.output, /Remove the unused sync IO exception for readFileSync/);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user