From ca0fe884ff736572b9a23dee4105318bcc1708b8 Mon Sep 17 00:00:00 2001 From: Zhaocun Sun Date: Thu, 21 May 2026 04:32:01 +0800 Subject: [PATCH] fix(cli): gate exported subcli descriptors (#84519) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: - This PR filters exported sub-CLI descriptors through the private-QA gate, centralizes that filter, adds regr ... ge, and carries small validation repairs in workspace glob and tunnel-timeout tests plus a changelog entry. - Reproducibility: yes. Current-main source shows the raw SUB_CLI_DESCRIPTORS export can include qa while the helper surfaces filter it, and src/cli/argv.ts consumes that export for root command policy. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(cli): gate exported subcli descriptors - PR branch already contained follow-up commit before automerge: fix(clawsweeper): address review for automerge-openclaw-openclaw-8451… Validation: - ClawSweeper review passed for head ba197a6f308b69d74da5f5d872f7c671fd13f512. - Required merge gates passed before the squash merge. Prepared head SHA: ba197a6f308b69d74da5f5d872f7c671fd13f512 Review: https://github.com/openclaw/openclaw/pull/84519#issuecomment-4496549642 Co-authored-by: Zhaocun Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <781889+takhoffman@users.noreply.github.com> --- CHANGELOG.md | 1 + ...rkspace.load-extra-bootstrap-files.test.ts | 36 ++++++ src/agents/workspace.ts | 103 ++++++++++++++++-- src/cli/program/subcli-descriptors.test.ts | 65 +++++++++++ src/cli/program/subcli-descriptors.ts | 46 +++++--- src/infra/net/http-connect-tunnel.test.ts | 19 ++-- 6 files changed, 237 insertions(+), 33 deletions(-) create mode 100644 src/cli/program/subcli-descriptors.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d5dd8fa8c02..7024f6d9b26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ Docs: https://docs.openclaw.ai - Cron: deliver preferred final assistant output for successful scheduled runs when trailing plain tool warnings remain in diagnostics instead of marking the run failed. - fix(mattermost): fail closed on missing channel type [AI]. (#84091) Thanks @pgondhi987. - Recheck rebuilt system.run argv [AI]. (#84090) Thanks @pgondhi987. +- CLI: keep the private QA subcommand out of exported command descriptors unless `OPENCLAW_ENABLE_PRIVATE_QA_CLI=1`, so root help and subcommand markers match runtime registration. (#84519) - CLI/cron: bound `openclaw cron show` job lookup pagination so non-advancing or unbounded `cron.list` responses fail instead of hanging the command. Fixes #83856. (#83989) - Agents/messages: stop message-tool-only turns after a successful source-channel `message` send while keeping transcript mirrors under the session write lock. (#84289) - Agents: filter silent heartbeat response-tool transcript artifacts out of embedded context snapshots so later user turns are not polluted by heartbeat no-op messages. (#83477) Thanks @fuller-stack-dev. diff --git a/src/agents/workspace.load-extra-bootstrap-files.test.ts b/src/agents/workspace.load-extra-bootstrap-files.test.ts index 7f7b3757e67..57e7f0ed9e8 100644 --- a/src/agents/workspace.load-extra-bootstrap-files.test.ts +++ b/src/agents/workspace.load-extra-bootstrap-files.test.ts @@ -43,6 +43,42 @@ describe("loadExtraBootstrapFiles", () => { ]); }); + it("loads glob patterns with explicit current-directory prefixes", async () => { + const workspaceDir = await createWorkspaceDir("glob-current-dir"); + const packageDir = path.join(workspaceDir, "packages", "core"); + await fs.mkdir(packageDir, { recursive: true }); + await fs.writeFile(path.join(packageDir, "AGENTS.md"), "agents", "utf-8"); + + const files = await loadExtraBootstrapFiles(workspaceDir, ["./packages/*/AGENTS.md"]); + + expect(files).toStrictEqual([ + { + name: "AGENTS.md", + path: path.join(packageDir, "AGENTS.md"), + content: "agents", + missing: false, + }, + ]); + }); + + it("loads literal bootstrap paths with square brackets", async () => { + const workspaceDir = await createWorkspaceDir("literal-brackets"); + const packageDir = path.join(workspaceDir, "pkg[1]"); + await fs.mkdir(packageDir, { recursive: true }); + await fs.writeFile(path.join(packageDir, "AGENTS.md"), "literal agents", "utf-8"); + + const files = await loadExtraBootstrapFiles(workspaceDir, ["pkg[1]/AGENTS.md"]); + + expect(files).toStrictEqual([ + { + name: "AGENTS.md", + path: path.join(packageDir, "AGENTS.md"), + content: "literal agents", + missing: false, + }, + ]); + }); + it("keeps path-traversal attempts outside workspace excluded", async () => { const rootDir = await createWorkspaceDir("root"); const workspaceDir = path.join(rootDir, "workspace"); diff --git a/src/agents/workspace.ts b/src/agents/workspace.ts index dc7f0f1c709..2c60706b7bf 100644 --- a/src/agents/workspace.ts +++ b/src/agents/workspace.ts @@ -702,6 +702,96 @@ export function filterBootstrapFilesForSession( return files.filter((file) => MINIMAL_BOOTSTRAP_ALLOWLIST.has(file.name)); } +function hasGlobPattern(pattern: string): boolean { + // Keep square brackets literal here; workspace paths commonly contain them. + return /[?*{}]/u.test(pattern); +} + +function normalizeWorkspacePatternPath(value: string): string { + return value + .replaceAll(path.sep, "/") + .replaceAll("\\", "/") + .replace(/^\.\/+/u, ""); +} + +function resolveGlobWalkRoot(pattern: string): string { + const normalized = normalizeWorkspacePatternPath(pattern); + const globIndex = normalized.search(/[?*{}]/u); + if (globIndex === -1) { + return normalized; + } + const slashIndex = normalized.lastIndexOf("/", globIndex); + return slashIndex === -1 ? "." : normalized.slice(0, slashIndex) || "."; +} + +async function* walkWorkspaceFiles( + workspaceDir: string, + initialRelativeDir: string, +): AsyncGenerator { + const stack = [initialRelativeDir === "." ? "" : initialRelativeDir]; + while (stack.length > 0) { + const currentRelativeDir = stack.pop() ?? ""; + const currentDir = path.resolve(workspaceDir, currentRelativeDir); + const relativeToWorkspace = path.relative(workspaceDir, currentDir); + if (relativeToWorkspace.startsWith("..") || path.isAbsolute(relativeToWorkspace)) { + continue; + } + + let entries: syncFs.Dirent[]; + try { + entries = await fs.readdir(currentDir, { withFileTypes: true }); + } catch { + continue; + } + + for (const entry of entries) { + const childRelativePath = currentRelativeDir + ? path.join(currentRelativeDir, entry.name) + : entry.name; + if (entry.isDirectory()) { + stack.push(childRelativePath); + continue; + } + if (entry.isFile() || entry.isSymbolicLink()) { + yield normalizeWorkspacePatternPath(childRelativePath); + } + } + } +} + +async function resolveExtraBootstrapPatternPaths( + workspaceDir: string, + pattern: string, +): Promise { + if (typeof fs.glob === "function") { + try { + const matches: string[] = []; + for await (const match of fs.glob(pattern, { cwd: workspaceDir })) { + matches.push(match); + } + return matches; + } catch { + // Fall through to the local matcher before treating the pattern as literal. + } + } + + if (typeof path.matchesGlob !== "function") { + return [pattern]; + } + + const normalizedPattern = normalizeWorkspacePatternPath(pattern); + const matches: string[] = []; + for await (const candidate of walkWorkspaceFiles( + workspaceDir, + resolveGlobWalkRoot(normalizedPattern), + )) { + if (path.matchesGlob(candidate, normalizedPattern)) { + matches.push(candidate); + } + } + return matches.length > 0 ? matches : [pattern]; +} + export async function loadExtraBootstrapFiles( dir: string, extraPatterns: string[], @@ -725,15 +815,10 @@ export async function loadExtraBootstrapFilesWithDiagnostics( // Resolve glob patterns into concrete file paths const resolvedPaths = new Set(); for (const pattern of extraPatterns) { - if (pattern.includes("*") || pattern.includes("?") || pattern.includes("{")) { - try { - const matches = fs.glob(pattern, { cwd: resolvedDir }); - for await (const m of matches) { - resolvedPaths.add(m); - } - } catch { - // glob not available or pattern error — fall back to literal - resolvedPaths.add(pattern); + if (hasGlobPattern(pattern)) { + const matches = await resolveExtraBootstrapPatternPaths(resolvedDir, pattern); + for (const match of matches) { + resolvedPaths.add(match); } } else { resolvedPaths.add(pattern); diff --git a/src/cli/program/subcli-descriptors.test.ts b/src/cli/program/subcli-descriptors.test.ts new file mode 100644 index 00000000000..5c562051af0 --- /dev/null +++ b/src/cli/program/subcli-descriptors.test.ts @@ -0,0 +1,65 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; + +async function importSubCliDescriptors() { + vi.resetModules(); + return import("./subcli-descriptors.js"); +} + +function descriptorNames(descriptors: ReadonlyArray<{ name: string }>): string[] { + return descriptors.map((descriptor) => descriptor.name); +} + +describe("sub-cli descriptors", () => { + const originalPrivateQaCli = process.env.OPENCLAW_ENABLE_PRIVATE_QA_CLI; + + afterEach(() => { + if (originalPrivateQaCli === undefined) { + delete process.env.OPENCLAW_ENABLE_PRIVATE_QA_CLI; + } else { + process.env.OPENCLAW_ENABLE_PRIVATE_QA_CLI = originalPrivateQaCli; + } + vi.resetModules(); + }); + + it("keeps the exported descriptor list aligned with private QA visibility when disabled (#83927)", async () => { + delete process.env.OPENCLAW_ENABLE_PRIVATE_QA_CLI; + + const { SUB_CLI_DESCRIPTORS, getSubCliEntries } = await importSubCliDescriptors(); + const exportedNames = descriptorNames(SUB_CLI_DESCRIPTORS); + + expect(exportedNames).toEqual(descriptorNames(getSubCliEntries())); + expect(exportedNames).not.toContain("qa"); + }); + + it("keeps all sub-cli filter surfaces aligned when private QA is disabled (#83926)", async () => { + delete process.env.OPENCLAW_ENABLE_PRIVATE_QA_CLI; + + const { + SUB_CLI_DESCRIPTORS, + getSubCliCommandsWithSubcommands, + getSubCliParentDefaultHelpCommands, + } = await importSubCliDescriptors(); + const exportedNames = descriptorNames(SUB_CLI_DESCRIPTORS); + + expect(exportedNames).not.toContain("qa"); + expect(getSubCliCommandsWithSubcommands()).not.toContain("qa"); + expect(getSubCliParentDefaultHelpCommands()).not.toContain("qa"); + }); + + it("includes qa in the exported descriptor list when private QA is enabled", async () => { + process.env.OPENCLAW_ENABLE_PRIVATE_QA_CLI = "1"; + + const { + SUB_CLI_DESCRIPTORS, + getSubCliCommandsWithSubcommands, + getSubCliEntries, + getSubCliParentDefaultHelpCommands, + } = await importSubCliDescriptors(); + const exportedNames = descriptorNames(SUB_CLI_DESCRIPTORS); + + expect(exportedNames).toEqual(descriptorNames(getSubCliEntries())); + expect(exportedNames).toContain("qa"); + expect(getSubCliCommandsWithSubcommands()).toContain("qa"); + expect(getSubCliParentDefaultHelpCommands()).not.toContain("qa"); + }); +}); diff --git a/src/cli/program/subcli-descriptors.ts b/src/cli/program/subcli-descriptors.ts index 32a737a0401..bb9ee0843eb 100644 --- a/src/cli/program/subcli-descriptors.ts +++ b/src/cli/program/subcli-descriptors.ts @@ -179,28 +179,42 @@ const subCliCommandCatalog = defineCommandDescriptorCatalog([ }, ] as const satisfies ReadonlyArray); -export const SUB_CLI_DESCRIPTORS = subCliCommandCatalog.descriptors; +function filterPrivateQaItems( + items: ReadonlyArray, + getName: (item: T) => string, +): ReadonlyArray { + if (isPrivateQaCliEnabled()) { + return items; + } + return items.filter((item) => getName(item) !== "qa"); +} + +export const SUB_CLI_DESCRIPTORS = filterPrivateQaItems( + subCliCommandCatalog.descriptors, + (descriptor) => descriptor.name, +); export function getSubCliEntries(): ReadonlyArray { - const descriptors = subCliCommandCatalog.getDescriptors(); - if (isPrivateQaCliEnabled()) { - return descriptors; - } - return descriptors.filter((descriptor) => descriptor.name !== "qa"); + return filterPrivateQaItems( + subCliCommandCatalog.getDescriptors(), + (descriptor) => descriptor.name, + ); } export function getSubCliCommandsWithSubcommands(): string[] { - const commands = subCliCommandCatalog.getCommandsWithSubcommands(); - if (isPrivateQaCliEnabled()) { - return commands; - } - return commands.filter((command) => command !== "qa"); + return [ + ...filterPrivateQaItems( + subCliCommandCatalog.getCommandsWithSubcommands(), + (command) => command, + ), + ]; } export function getSubCliParentDefaultHelpCommands(): string[] { - const commands = subCliCommandCatalog.getParentDefaultHelpCommands(); - if (isPrivateQaCliEnabled()) { - return commands; - } - return commands.filter((command) => command !== "qa"); + return [ + ...filterPrivateQaItems( + subCliCommandCatalog.getParentDefaultHelpCommands(), + (command) => command, + ), + ]; } diff --git a/src/infra/net/http-connect-tunnel.test.ts b/src/infra/net/http-connect-tunnel.test.ts index e6d4dfc1f1f..c4b94abad83 100644 --- a/src/infra/net/http-connect-tunnel.test.ts +++ b/src/infra/net/http-connect-tunnel.test.ts @@ -330,20 +330,23 @@ describe("openHttpConnectTunnel", () => { }); it("rejects and destroys the proxy socket when CONNECT times out", async () => { + vi.useFakeTimers(); const proxySocket = new FakeSocket(); setNextNetSocket(proxySocket); const { openHttpConnectTunnel } = await import("./http-connect-tunnel.js"); - await expect( - openHttpConnectTunnel({ - proxyUrl: new URL("http://proxy.example:8080"), - targetHost: "api.push.apple.com", - targetPort: 443, - timeoutMs: 1, - }), - ).rejects.toThrow( + const tunnel = openHttpConnectTunnel({ + proxyUrl: new URL("http://proxy.example:8080"), + targetHost: "api.push.apple.com", + targetPort: 443, + timeoutMs: 1, + }); + const rejected = expect(tunnel).rejects.toThrow( "Proxy CONNECT failed via http://proxy.example:8080: Proxy CONNECT timed out after 1ms", ); + + await vi.advanceTimersByTimeAsync(1); + await rejected; expect(proxySocket.destroyed).toBe(true); }); });