fix: harden QA runtime triage findings

This commit is contained in:
Gustavo Madeira Santana
2026-04-15 20:04:10 -04:00
parent dccb23e864
commit 5dbc515048
8 changed files with 146 additions and 36 deletions

View File

@@ -9,6 +9,13 @@ const QA_ALWAYS_STAGE_RUNTIME_PLUGIN_IDS = Object.freeze([
"speech-core",
]);
const QA_OPENAI_PLUGIN_ID = "openai";
const QA_BUNDLED_PLUGIN_ID_PATTERN = /^[A-Za-z0-9][A-Za-z0-9._-]*$/;
function assertSafeQaBundledPluginId(pluginId: string) {
if (!QA_BUNDLED_PLUGIN_ID_PATTERN.test(pluginId)) {
throw new Error(`invalid QA bundled plugin id: ${pluginId}`);
}
}
function parseStableSemverFloor(value: string | undefined) {
if (!value) {
@@ -56,6 +63,7 @@ function isQaOpenAiResponsesProviderConfig(config: ModelProviderConfig) {
}
export function resolveQaBundledPluginSourceDir(params: { repoRoot: string; pluginId: string }) {
assertSafeQaBundledPluginId(params.pluginId);
const candidates = [
path.join(params.repoRoot, "dist", "extensions", params.pluginId),
path.join(params.repoRoot, "dist-runtime", "extensions", params.pluginId),
@@ -141,7 +149,12 @@ function collectQaBundledPluginIds(params: {
repoRoot: string;
allowedPluginIds: readonly string[];
}) {
const pluginIds = new Set(params.allowedPluginIds);
const pluginIds = new Set(
params.allowedPluginIds.map((pluginId) => {
assertSafeQaBundledPluginId(pluginId);
return pluginId;
}),
);
for (const pluginId of QA_ALWAYS_STAGE_RUNTIME_PLUGIN_IDS) {
if (
resolveQaBundledPluginSourceDir({

View File

@@ -856,6 +856,30 @@ describe("qa bundled plugin dir", () => {
).resolves.toBeTruthy();
});
it("rejects invalid bundled plugin ids before staging paths are built", async () => {
const repoRoot = await mkdtemp(path.join(os.tmpdir(), "qa-bundled-invalid-id-"));
cleanups.push(async () => {
await rm(repoRoot, { recursive: true, force: true });
});
await writeFile(
path.join(repoRoot, "package.json"),
JSON.stringify({ name: "openclaw", type: "module" }, null, 2),
"utf8",
);
const tempRoot = await mkdtemp(path.join(os.tmpdir(), "qa-bundled-invalid-target-"));
cleanups.push(async () => {
await rm(tempRoot, { recursive: true, force: true });
});
await expect(
__testing.createQaBundledPluginsDir({
repoRoot,
tempRoot,
allowedPluginIds: ["../escape"],
}),
).rejects.toThrow("invalid QA bundled plugin id: ../escape");
});
it("stages source-only bundled plugins into a repo-like runtime root with node_modules", async () => {
const repoRoot = await mkdtemp(path.join(os.tmpdir(), "qa-bundled-source-stage-"));
cleanups.push(async () => {

View File

@@ -0,0 +1,11 @@
import { describe, expect, it } from "vitest";
import { normalizeQaTransportId } from "./qa-transport-registry.js";
describe("qa transport registry", () => {
it("rejects inherited prototype keys as unsupported transport ids", () => {
expect(() => normalizeQaTransportId("toString")).toThrow("unsupported QA transport: toString");
expect(() => normalizeQaTransportId("__proto__")).toThrow(
"unsupported QA transport: __proto__",
);
});
});

View File

@@ -23,9 +23,9 @@ const QA_TRANSPORT_REGISTRY = {
>;
export function normalizeQaTransportId(input?: string | null): QaTransportId {
const transportId = (input?.trim() || DEFAULT_QA_TRANSPORT_ID) as QaTransportId;
if (transportId in QA_TRANSPORT_REGISTRY) {
return transportId;
const transportId = input?.trim() || DEFAULT_QA_TRANSPORT_ID;
if (Object.hasOwn(QA_TRANSPORT_REGISTRY, transportId)) {
return transportId as QaTransportId;
}
throw new Error(`unsupported QA transport: ${transportId}`);
}

View File

@@ -210,8 +210,8 @@ export const resolveBuildRequirement = (deps) => {
}
if (
deps.env.OPENCLAW_BUILD_PRIVATE_QA === "1" &&
deps.privateQaDistEntry &&
statMtime(deps.privateQaDistEntry, deps.fs) == null
((deps.privateQaDistEntry && statMtime(deps.privateQaDistEntry, deps.fs) == null) ||
(deps.privateQaBundledCliEntry && statMtime(deps.privateQaBundledCliEntry, deps.fs) == null))
) {
return { shouldBuild: true, reason: "missing_private_qa_dist" };
}
@@ -398,6 +398,7 @@ export async function runNodeMain(params = {}) {
}));
deps.configFiles = runNodeConfigFiles.map((filePath) => path.join(deps.cwd, filePath));
deps.privateQaDistEntry = path.join(deps.distRoot, "plugin-sdk", "qa-lab.js");
deps.privateQaBundledCliEntry = path.join(deps.distRoot, "extensions", "qa-lab", "cli.js");
if (deps.args[0] === "qa") {
deps.env.OPENCLAW_BUILD_PRIVATE_QA = "1";
deps.env.OPENCLAW_ENABLE_PRIVATE_QA_CLI = "1";

View File

@@ -20,6 +20,7 @@ const GENERATED_A2UI_BUNDLE_HASH = "src/canvas-host/a2ui/.bundle.hash";
const DIST_ENTRY = "dist/entry.js";
const BUILD_STAMP = "dist/.buildstamp";
const QA_LAB_PLUGIN_SDK_ENTRY = "dist/plugin-sdk/qa-lab.js";
const QA_LAB_BUNDLED_CLI_ENTRY = "dist/extensions/qa-lab/cli.js";
const EXTENSION_SRC = bundledPluginFile("demo", "src/index.ts");
const EXTENSION_MANIFEST = bundledPluginFile("demo", "openclaw.plugin.json");
const EXTENSION_PACKAGE = bundledPluginFile("demo", "package.json");
@@ -343,6 +344,46 @@ describe("run-node script", () => {
});
it("skips rebuilding for private QA commands when the QA CLI facade is present", async () => {
await withTempDir({ prefix: "openclaw-run-node-" }, async (tmp) => {
await setupTrackedProject(tmp, {
files: {
[ROOT_SRC]: "export const value = 1;\n",
[QA_LAB_PLUGIN_SDK_ENTRY]: "export const qaLab = true;\n",
[QA_LAB_BUNDLED_CLI_ENTRY]: "export const registerQaLabCli = () => {};\n",
},
oldPaths: [
ROOT_SRC,
ROOT_TSCONFIG,
ROOT_PACKAGE,
QA_LAB_PLUGIN_SDK_ENTRY,
QA_LAB_BUNDLED_CLI_ENTRY,
],
buildPaths: [DIST_ENTRY, BUILD_STAMP],
});
const { spawnCalls, spawn, spawnSync } = createSpawnRecorder({
gitHead: "abc123\n",
gitStatus: "",
});
const exitCode = await runQaCommand({ tmp, spawn, spawnSync });
expect(exitCode).toBe(0);
expect(spawnCalls).toEqual([
[
process.execPath,
"openclaw.mjs",
"qa",
"suite",
"--transport",
"qa-channel",
"--provider-mode",
"mock-openai",
],
]);
});
});
it("rebuilds private QA commands when the QA bundled CLI surface is missing", async () => {
await withTempDir({ prefix: "openclaw-run-node-" }, async (tmp) => {
await setupTrackedProject(tmp, {
files: {
@@ -359,33 +400,19 @@ describe("run-node script", () => {
});
const exitCode = await runQaCommand({ tmp, spawn, spawnSync });
expect(exitCode).toBe(0);
expect(spawnCalls).toEqual([
[process.execPath, "openclaw.mjs", "qa", "suite", "--transport", "qa-channel", "--provider-mode", "mock-openai"],
]);
});
});
it("rebuilds private QA commands when the QA CLI facade is missing", async () => {
await withTempDir({ prefix: "openclaw-run-node-" }, async (tmp) => {
await setupTrackedProject(tmp, {
files: {
[ROOT_SRC]: "export const value = 1;\n",
},
oldPaths: [ROOT_SRC, ROOT_TSCONFIG, ROOT_PACKAGE],
buildPaths: [DIST_ENTRY, BUILD_STAMP],
});
const { spawnCalls, spawn, spawnSync } = createSpawnRecorder({
gitHead: "abc123\n",
gitStatus: "",
});
const exitCode = await runQaCommand({ tmp, spawn, spawnSync });
expect(exitCode).toBe(0);
expect(spawnCalls).toEqual([
expectedBuildSpawn(),
[process.execPath, "openclaw.mjs", "qa", "suite", "--transport", "qa-channel", "--provider-mode", "mock-openai"],
[
process.execPath,
"openclaw.mjs",
"qa",
"suite",
"--transport",
"qa-channel",
"--provider-mode",
"mock-openai",
],
]);
});
});

View File

@@ -141,7 +141,9 @@ function listPrivateLocalOnlyPluginSdkSubpaths() {
if (!Array.isArray(parsed)) {
return [];
}
return parsed.filter((subpath) => typeof subpath === "string");
return parsed.filter(
(subpath) => typeof subpath === "string" && /^[A-Za-z0-9][A-Za-z0-9_-]*$/.test(subpath),
);
} catch {
return [];
}

View File

@@ -29,6 +29,7 @@ function loadRootAliasWithStubs(options?: {
packageExports?: Record<string, unknown>;
platform?: string;
existingPaths?: string[];
privateLocalOnlySubpaths?: unknown;
}) {
let createJitiCalls = 0;
let jitiLoadCalls = 0;
@@ -61,13 +62,21 @@ function loadRootAliasWithStubs(options?: {
}
if (id === "node:fs") {
return {
readFileSync: () =>
JSON.stringify({
readFileSync: (targetPath: string) => {
if (
targetPath.endsWith(
path.join("scripts", "lib", "plugin-sdk-private-local-only-subpaths.json"),
)
) {
return JSON.stringify(options?.privateLocalOnlySubpaths ?? []);
}
return JSON.stringify({
exports: {
"./plugin-sdk/group-access": { default: "./dist/plugin-sdk/group-access.js" },
...options?.packageExports,
},
}),
});
},
existsSync: (targetPath: string) => {
if (targetPath.endsWith(path.join("dist", "infra", "diagnostic-events.js"))) {
return options?.distExists ?? false;
@@ -298,17 +307,40 @@ describe("plugin-sdk root alias", () => {
(lazyModule.createJitiOptions.at(-1)?.alias ?? {}) as Record<string, string>,
);
expect(aliasKeys).toEqual([
"openclaw/plugin-sdk",
"@openclaw/plugin-sdk",
"openclaw/plugin-sdk/alpha",
"@openclaw/plugin-sdk/alpha",
"openclaw/plugin-sdk/group-access",
"@openclaw/plugin-sdk/group-access",
"openclaw/plugin-sdk/zeta",
"@openclaw/plugin-sdk/zeta",
"openclaw/plugin-sdk",
"@openclaw/plugin-sdk",
]);
});
it("ignores unsafe private local-only plugin-sdk subpaths in the CJS root alias", () => {
const packageRoot = path.dirname(path.dirname(path.dirname(rootAliasPath)));
const lazyModule = loadRootAliasWithStubs({
env: { OPENCLAW_ENABLE_PRIVATE_QA_CLI: "1" },
privateLocalOnlySubpaths: ["qa-lab", "../escape", "nested/path"],
existingPaths: [path.join(packageRoot, "src", "plugin-sdk", "qa-lab.ts")],
monolithicExports: {
slowHelper: (): string => "loaded",
},
});
expect((lazyModule.moduleExports.slowHelper as () => string)()).toBe("loaded");
const aliasMap = (lazyModule.createJitiOptions.at(-1)?.alias ?? {}) as Record<string, string>;
expect(aliasMap["openclaw/plugin-sdk/qa-lab"]).toBe(
path.join(packageRoot, "src", "plugin-sdk", "qa-lab.ts"),
);
expect(aliasMap["@openclaw/plugin-sdk/qa-lab"]).toBe(
path.join(packageRoot, "src", "plugin-sdk", "qa-lab.ts"),
);
expect(aliasMap).not.toHaveProperty("openclaw/plugin-sdk/../escape");
expect(aliasMap).not.toHaveProperty("openclaw/plugin-sdk/nested/path");
});
it("builds source plugin-sdk subpath aliases through the wider source extension family", () => {
const packageRoot = path.dirname(path.dirname(path.dirname(rootAliasPath)));
const lazyModule = loadRootAliasWithStubs({