diff --git a/.changeset/unknown-command-telemetry.md b/.changeset/unknown-command-telemetry.md new file mode 100644 index 000000000..556f08310 --- /dev/null +++ b/.changeset/unknown-command-telemetry.md @@ -0,0 +1,18 @@ +--- +"cf": minor +--- + +Record sanitised detail about unrecognised commands in usage telemetry + +When `cf` cannot match the arguments to a command, telemetry now records the +kind of failure, the recognised part of the command path, whether that part is +a complete command, and up to three unrecognised words. A word is included only +when it is a `cf` command name in a place where a command name was expected; +anything else, including any extra value after a complete command, is replaced +by a category such as ``, ``, or ``. Unrecognised flags are +counted, never named. + +Failures inside a group or command keep the `command` value they were reported +under before. Failures at the top level that were previously dropped – a typo +with a suggestion, several unknown words, or only flags with no command – are +now reported as unknown commands. diff --git a/packages/cli/src/__tests__/lib/telemetry/command-resolution.test.ts b/packages/cli/src/__tests__/lib/telemetry/command-resolution.test.ts new file mode 100644 index 000000000..20a35ca67 --- /dev/null +++ b/packages/cli/src/__tests__/lib/telemetry/command-resolution.test.ts @@ -0,0 +1,244 @@ +import { afterEach, describe, expect, it, vi } from "vite-plus/test"; +import { + __resetCommandVocabularyForTests, + loadCommandVocabulary, + MAX_UNKNOWN_TOKENS, + sanitizeCommandResolution, + sanitizeCommandToken, +} from "../../../lib/telemetry/command-resolution.js"; +import type * as MetadataModule from "../../../lib/metadata.js"; +import type { CommandResolutionFailure } from "../../../lib/telemetry/types.js"; + +const { actualLoadMeta, loadMeta } = vi.hoisted(() => ({ + actualLoadMeta: { + current: undefined as typeof MetadataModule.loadMeta | undefined, + }, + loadMeta: vi.fn(), +})); + +vi.mock("../../../lib/metadata.js", async (importOriginal) => { + const actual = await importOriginal(); + actualLoadMeta.current = actual.loadMeta; + loadMeta.mockImplementation(actual.loadMeta); + return { ...actual, loadMeta }; +}); + +const commands = new Set(["zones", "list", "kv", "namespaces", "workers"]); +const vocabulary = { + commands, + options: new Set(["zone", "zone-id", "dry-run", "help"]), +}; + +function failure( + overrides: Partial = {} +): CommandResolutionFailure { + return { + kind: "unknown-subcommand", + knownCommand: [], + knownCommandIsLeaf: false, + unknownTokens: [], + ...overrides, + }; +} + +describe("sanitizeCommandToken", () => { + it.each([ + { token: "zones", expected: "zones" }, + { token: "Zones", expected: "zones" }, + { token: "kv:namespaces", expected: "kv:namespaces" }, + { token: "kv:namespace", expected: "" }, + { token: "zones list", expected: "" }, + { token: "https://example.com/a", expected: "" }, + { token: "user@example.com", expected: "" }, + { token: "./a/b", expected: "" }, + { token: "a/b", expected: "" }, + { token: "~", expected: "" }, + { token: ".env", expected: "" }, + { token: "C:\\Users\\name", expected: "" }, + { token: "C:", expected: "" }, + { token: "123e4567-e89b-12d3-a456-426614174000", expected: "" }, + { token: "023e105f4ecef8ad9ca31a8372d0c353", expected: "" }, + { token: "12345", expected: "" }, + { token: "example.com", expected: "" }, + { token: "my-worker", expected: "" }, + { token: "sk_live_secret", expected: "" }, + { token: "-", expected: "" }, + { token: "", expected: "" }, + ])("maps $token to $expected", ({ token, expected }) => { + expect(sanitizeCommandToken(token, commands)).toBe(expected); + }); + + it("classifies only the first 64 characters", () => { + expect(sanitizeCommandToken("w".repeat(500), commands)).toBe(""); + expect(sanitizeCommandToken(`${"f".repeat(64)} x`, commands)).toBe(""); + }); + + it("reduces every token to its shape without a vocabulary", () => { + expect(sanitizeCommandToken("zones", new Set())).toBe(""); + }); +}); + +describe("sanitizeCommandResolution", () => { + it("keeps the recognised path and sanitises unknown tokens", () => { + expect( + sanitizeCommandResolution( + failure({ + kind: "surplus-positional", + knownCommand: ["zones", "list"], + knownCommandIsLeaf: true, + unknownTokens: ["023e105f4ecef8ad9ca31a8372d0c353"], + }), + ["zones", "list", "023e105f4ecef8ad9ca31a8372d0c353"], + vocabulary + ) + ).toEqual({ + resolutionFailure: "surplus-positional", + knownCommand: "zones list", + knownCommandIsLeaf: true, + unknownTokens: [""], + unknownTokenCount: 1, + unknownFlagCount: 0, + argsUsed: [], + argsCombination: "", + }); + }); + + it("labels a surplus positional even when it is a cf command word", () => { + const tokens = ["zones", "list"]; + expect( + sanitizeCommandResolution( + failure({ + kind: "surplus-positional", + knownCommand: ["workers", "get"], + knownCommandIsLeaf: true, + unknownTokens: tokens, + }), + ["workers", "get", ...tokens], + vocabulary + ).unknownTokens + ).toEqual(["", ""]); + expect( + sanitizeCommandResolution( + failure({ knownCommand: ["workers"], unknownTokens: tokens }), + ["workers", ...tokens], + vocabulary + ).unknownTokens + ).toEqual(tokens); + }); + + it("caps the tokens sent but reports the real count", () => { + const result = sanitizeCommandResolution( + failure({ unknownTokens: ["a", "b", "c", "d", "e"] }), + ["a", "b", "c", "d", "e"], + vocabulary + ); + expect(result.unknownTokens).toHaveLength(MAX_UNKNOWN_TOKENS); + expect(result.unknownTokenCount).toBe(5); + }); + + it("names known flags and only counts unknown ones", () => { + const rawArgv = [ + "zones", + "--zone-id", + "abc123", + "--zone=example.com", + "--sk_live_secret", + "--other-secret=value", + "--", + "--after-separator", + ]; + const result = sanitizeCommandResolution( + failure({ knownCommand: ["zones"], unknownTokens: [] }), + rawArgv, + vocabulary + ); + expect(result.argsUsed).toEqual(["zone", "zone-id"]); + expect(result.argsCombination).toBe("zone, zone-id"); + expect(result.unknownFlagCount).toBe(2); + const serialised = JSON.stringify(result); + for (const value of [ + "abc123", + "example.com", + "sk_live", + "secret", + "value", + "after-separator", + ]) { + expect(serialised).not.toContain(value); + } + }); + + it("includes a suggestion only for suggested-command failures", () => { + expect( + sanitizeCommandResolution( + failure({ + kind: "suggested-command", + knownCommand: ["r2", "buckets"], + unknownTokens: ["lsit"], + suggestedCommand: "list", + }), + [], + vocabulary + ).suggestedCommand + ).toBe("list"); + expect( + sanitizeCommandResolution( + failure({ suggestedCommand: "list" }), + [], + vocabulary + ) + ).not.toHaveProperty("suggestedCommand"); + }); + + it("drops a suggestion that does not look like a command name", () => { + expect( + sanitizeCommandResolution( + failure({ + kind: "suggested-command", + suggestedCommand: "not a command", + }), + [], + vocabulary + ) + ).not.toHaveProperty("suggestedCommand"); + }); +}); + +describe("loadCommandVocabulary", () => { + afterEach(() => { + __resetCommandVocabularyForTests(); + if (actualLoadMeta.current) { + loadMeta.mockImplementation(actualLoadMeta.current); + } + }); + + it("reads command words and option names from the catalogue", () => { + const loaded = loadCommandVocabulary(["quiet"]); + for (const word of ["zones", "list", "workers", "help", "whoami"]) { + expect(loaded.commands.has(word)).toBe(true); + } + for (const option of ["zone-id", "dry-run", "quiet", "help"]) { + expect(loaded.options.has(option)).toBe(true); + } + expect(loaded.commands.has("my-worker")).toBe(false); + }); + + it("falls back to global options when the catalogue cannot load", () => { + loadMeta.mockImplementation(() => null); + const loaded = loadCommandVocabulary(["quiet", "zone"]); + expect(loaded.commands.size).toBe(0); + expect([...loaded.options].sort()).toEqual([ + "help", + "quiet", + "version", + "zone", + ]); + }); + + it("falls back when reading the catalogue throws", () => { + loadMeta.mockImplementation(() => { + throw new Error("unreadable"); + }); + expect(loadCommandVocabulary([]).commands.size).toBe(0); + }); +}); diff --git a/packages/cli/src/__tests__/telemetry-errors.test.ts b/packages/cli/src/__tests__/telemetry-errors.test.ts index 6d196bf3d..241fd1a10 100644 --- a/packages/cli/src/__tests__/telemetry-errors.test.ts +++ b/packages/cli/src/__tests__/telemetry-errors.test.ts @@ -6,14 +6,18 @@ import { it, vi, } from "vite-plus/test"; +import { __resetCommandVocabularyForTests } from "../lib/telemetry/command-resolution.js"; import { captureOutput } from "./helpers/capture-output.js"; import { runCf } from "./helpers/run-cf.js"; +import type * as MetadataModule from "../lib/metadata.js"; import type * as TelemetryModule from "../lib/telemetry/index.js"; import type { TelemetryDispatcher } from "../lib/telemetry/index.js"; const { + actualLoadMeta, dispatcher, getTelemetryDispatcher, + loadMeta, sendAdhocEvent, sendCommandEvent, updateState, @@ -27,10 +31,14 @@ const { sendAdhocEvent: mockedSendAdhocEvent, } satisfies TelemetryDispatcher; return { + actualLoadMeta: { + current: undefined as typeof MetadataModule.loadMeta | undefined, + }, dispatcher: mockedDispatcher, getTelemetryDispatcher: vi.fn<() => TelemetryDispatcher>( () => mockedDispatcher ), + loadMeta: vi.fn(), sendAdhocEvent: mockedSendAdhocEvent, sendCommandEvent: mockedSendCommandEvent, updateState: vi.fn(), @@ -44,6 +52,13 @@ vi.mock("../lib/telemetry/index.js", async (importOriginal) => ({ vi.mock("../lib/telemetry/dispatcher.js", () => ({ getTelemetryDispatcher })); +vi.mock("../lib/metadata.js", async (importOriginal) => { + const actual = await importOriginal(); + actualLoadMeta.current = actual.loadMeta; + loadMeta.mockImplementation(actual.loadMeta); + return { ...actual, loadMeta }; +}); + vi.mock("../lib/state.js", () => ({ readState: () => ({}), updateState, @@ -62,6 +77,10 @@ describe("telemetry error reporting", () => { sendCommandEvent.mockClear(); sendAdhocEvent.mockClear(); updateState.mockClear(); + __resetCommandVocabularyForTests(); + if (actualLoadMeta.current) { + loadMeta.mockImplementation(actualLoadMeta.current); + } }); afterEach(() => { @@ -80,16 +99,26 @@ describe("telemetry error reporting", () => { "Unknown command: unrecognizable" ); + const resolution = { + command: "unknown command", + resolutionFailure: "unknown-subcommand", + knownCommand: "", + knownCommandIsLeaf: false, + unknownTokens: [""], + unknownTokenCount: 1, + unknownFlagCount: 0, + argsUsed: [], + }; expect(sendCommandEvent).toHaveBeenCalledTimes(2); expect(sendCommandEvent).toHaveBeenNthCalledWith( 1, "cf command started", - expect.objectContaining({ command: "unknown command" }) + expect.objectContaining(resolution) ); expect(sendCommandEvent).toHaveBeenNthCalledWith( 2, "cf command finished", - expect.objectContaining({ command: "unknown command", outcome: "error" }) + expect.objectContaining({ ...resolution, outcome: "error" }) ); }); @@ -102,17 +131,297 @@ describe("telemetry error reporting", () => { expect(sendCommandEvent).toHaveBeenNthCalledWith( 1, "cf command started", - expect.objectContaining({ command: "unknown command" }) + expect.objectContaining({ + command: "unknown command", + unknownTokens: [""], + argsUsed: ["zone"], + }) ); expect(sendCommandEvent).toHaveBeenNthCalledWith( 2, "cf command finished", expect.objectContaining({ command: "unknown command", outcome: "error" }) ); + expect(JSON.stringify(sendCommandEvent.mock.calls)).not.toContain( + "telemetry" + ); + }); + + it.each([ + { + args: ["frobnicate", "foo"], + error: "Unknown commands: frobnicate, foo", + command: "unknown command", + resolution: { + resolutionFailure: "unknown-subcommand", + knownCommand: "", + unknownTokens: ["", ""], + unknownTokenCount: 2, + }, + }, + { + args: ["tunnel", "run"], + error: "Did you mean tunnels?", + command: "unknown command", + resolution: { + resolutionFailure: "suggested-command", + knownCommand: "", + unknownTokens: ["", "run"], + unknownTokenCount: 2, + suggestedCommand: "tunnels", + }, + }, + { + args: ["acount"], + error: "Did you mean accounts?", + command: "unknown command", + resolution: { + resolutionFailure: "suggested-command", + knownCommand: "", + unknownTokens: [""], + suggestedCommand: "accounts", + }, + }, + { + args: ["--bogus"], + error: "You need to specify a command", + command: "unknown command", + resolution: { + resolutionFailure: "missing-command", + knownCommand: "", + unknownTokens: [], + unknownTokenCount: 0, + unknownFlagCount: 1, + argsUsed: [], + }, + }, + { + args: ["workers", "deploy"], + error: "Unknown command: deploy", + command: "unknown command", + resolution: { + resolutionFailure: "unknown-subcommand", + knownCommand: "workers", + knownCommandIsLeaf: false, + unknownTokens: ["deploy"], + }, + }, + { + args: ["r2", "buckets", "lsit"], + error: "Did you mean list?", + command: "r2 buckets", + resolution: { + resolutionFailure: "suggested-command", + knownCommand: "r2 buckets", + unknownTokens: [""], + suggestedCommand: "list", + }, + }, + { + args: ["zones", "get", "023e105f4ecef8ad9ca31a8372d0c353"], + error: "Unknown command: 023e105f4ecef8ad9ca31a8372d0c353", + command: "unknown command", + resolution: { + resolutionFailure: "surplus-positional", + knownCommand: "zones get", + knownCommandIsLeaf: true, + unknownTokens: [""], + }, + }, + { + args: ["dns", "records", "list", "example.com"], + error: "Unknown command: example.com", + command: "unknown command", + resolution: { + resolutionFailure: "surplus-positional", + knownCommand: "dns records list", + unknownTokens: [""], + }, + }, + { + args: ["zones", "list", "./a/b"], + error: "Unknown command: ./a/b", + command: "unknown command", + resolution: { unknownTokens: [""] }, + }, + { + args: ["zones", "list", "C:\\x"], + error: "Unknown command: C:\\x", + command: "unknown command", + resolution: { unknownTokens: [""] }, + }, + { + // The plural message keeps the resolved command's name, as it did + // before resolution detail was recorded. + args: ["zones", "get", "abc", "extra"], + error: "Unknown commands: abc, extra", + command: "zones get", + resolution: { + resolutionFailure: "surplus-positional", + knownCommand: "zones get", + unknownTokenCount: 2, + }, + }, + { + args: ["workers", "deploy", "my-worker"], + error: "Unknown commands: deploy, my-worker", + command: "workers", + resolution: { + resolutionFailure: "unknown-subcommand", + knownCommand: "workers", + knownCommandIsLeaf: false, + unknownTokens: ["deploy", ""], + unknownTokenCount: 2, + }, + }, + { + args: ["abuse-reports", "get", "RPT123", "test"], + error: "Unknown command: test", + command: "unknown command", + resolution: { + resolutionFailure: "surplus-positional", + knownCommand: "abuse-reports get", + unknownTokens: [""], + }, + }, + { + args: ["zones", "list", "user@example.com"], + error: "Unknown command: user@example.com", + command: "unknown command", + resolution: { unknownTokens: [""] }, + }, + { + args: ["zones", "list", "123e4567-e89b-12d3-a456-426614174000"], + error: "Unknown command: 123e4567-e89b-12d3-a456-426614174000", + command: "unknown command", + resolution: { unknownTokens: [""] }, + }, + { + args: ["zones list"], + error: "Unknown command: zones list", + command: "unknown command", + resolution: { unknownTokens: [""] }, + }, + { + args: ["kv:namespace"], + error: "Unknown command: kv:namespace", + command: "unknown command", + resolution: { unknownTokens: ["kv:namespace"] }, + }, + { + args: ["kv:frobnicate"], + error: "Unknown command: kv:frobnicate", + command: "unknown command", + resolution: { unknownTokens: [""] }, + }, + ])( + "reports resolution detail for $args", + async ({ args, error, command, resolution }) => { + await expect(runCf(args)).rejects.toThrow(error); + expect(sendCommandEvent).toHaveBeenCalledTimes(2); + for (const eventName of ["cf command started", "cf command finished"]) { + expect(sendCommandEvent).toHaveBeenCalledWith( + eventName, + expect.objectContaining({ + command, + sanitizedArgs: {}, + ...resolution, + }) + ); + } + } + ); + + it("never sends an unrecognised resource name", async () => { + await expect(runCf(["zones", "list", "my-worker"])).rejects.toThrow( + "Unknown command: my-worker" + ); + expect(sendCommandEvent).toHaveBeenCalledWith( + "cf command finished", + expect.objectContaining({ unknownTokens: [""] }) + ); + expect(JSON.stringify(sendCommandEvent.mock.calls)).not.toContain( + "my-worker" + ); + }); + + it("caps unknown tokens and ignores tokens after --", async () => { + await expect( + runCf(["zones", "list", "t1", "t2", "t3", "t4", "t5", "--", "secret"]) + ).rejects.toThrow("Unknown commands: t1, t2, t3, t4, t5"); + expect(sendCommandEvent).toHaveBeenCalledWith( + "cf command finished", + expect.objectContaining({ + unknownTokens: ["", "", ""], + unknownTokenCount: 5, + }) + ); + expect(JSON.stringify(sendCommandEvent.mock.calls)).not.toContain("secret"); + }); + + it("names cf flags but only counts unknown ones", async () => { + await expect( + runCf([ + "zones", + "list", + "extra", + "--zone-id", + "abc123", + "--sk_live_value", + ]) + ).rejects.toThrow("Unknown command: extra"); + expect(sendCommandEvent).toHaveBeenCalledWith( + "cf command finished", + expect.objectContaining({ + argsUsed: ["zone-id"], + argsCombination: "zone-id", + unknownFlagCount: 1, + sanitizedArgs: {}, + }) + ); + const sent = JSON.stringify(sendCommandEvent.mock.calls); + expect(sent).not.toContain("abc123"); + expect(sent).not.toContain("sk_live"); + }); + + it("sends only shape labels when the catalogue cannot load", async () => { + loadMeta.mockImplementation(() => null); + + await expect(runCf(["workers", "deploy"])).rejects.toThrow( + "Unknown command: deploy" + ); + expect(sendCommandEvent).toHaveBeenCalledWith( + "cf command finished", + expect.objectContaining({ + command: "unknown command", + knownCommand: "workers", + unknownTokens: [""], + }) + ); + }); + + it("does not load the catalogue when telemetry is disabled", async () => { + getTelemetryDispatcher.mockReturnValue({ ...dispatcher, enabled: false }); + loadMeta.mockClear(); + + await expect(runCf(["workers", "deploy"])).rejects.toThrow( + "Unknown command: deploy" + ); + expect(loadMeta).not.toHaveBeenCalledWith( + expect.anything(), + "commands.json", + expect.anything() + ); + expect(sendCommandEvent).not.toHaveBeenCalled(); }); it.each([ { args: ["cli", "telemetry", "typo"], error: "Unknown command: typo" }, + { args: ["clii", "telemetry", "disable"], error: "Did you mean cli?" }, + { + args: ["cli", "telemtry", "disable"], + error: "Did you mean telemetry?", + }, { args: ["cli", "telemetry", "status", "--bogus"], error: "Unknown argument: bogus", @@ -240,6 +549,9 @@ describe("telemetry error reporting", () => { "cf command finished", expect.objectContaining({ command, outcome: "error" }) ); + expect(sendCommandEvent.mock.calls[1]?.[1]).not.toHaveProperty( + "resolutionFailure" + ); } ); diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index 51499a285..e9a6af62d 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -33,6 +33,10 @@ import { renderPromptIntro } from "./lib/ui/banner.js"; import { theme } from "./lib/ui/index.js"; import { VERSION as version } from "./version.js"; import type * as TelemetryModule from "./lib/telemetry/index.js"; +import type { + CommandResolutionFailure, + CommandResolutionFailureKind, +} from "./lib/telemetry/types.js"; import type { UpdateNotice } from "./lib/update-check.js"; import type { Options } from "yargs"; @@ -265,10 +269,80 @@ async function flushTelemetry(): Promise { } } +// yargs words this singularly or plurally depending on the token count. +const UNKNOWN_COMMAND_MESSAGE = /^Unknown commands?: /; +const SUGGESTED_COMMAND_MESSAGE = /^Did you mean (.+)\?$/; +const MISSING_COMMAND_MESSAGE = "You need to specify a command"; + +// Only the singular form has historically been bucketed as an unknown +// command; the plural keeps the resolved command name for continuity. function isUnknownCommandError(error: unknown): boolean { return error instanceof Error && error.message.startsWith("Unknown command:"); } +interface YargsResolutionState { + parsed?: false | { argv?: { _?: unknown } }; + getInternalMethods(): { + getContext(): { commands: unknown }; + getCommandInstance(): { getCommands(): unknown }; + }; +} + +/** + * Capture where command resolution stopped, read while yargs still holds the + * failing level's context (it resets after parsing). Returns the raw tokens; + * telemetry sanitises them before sending. + */ +function commandResolutionFailure( + yargsInstance: unknown, + message: string +): CommandResolutionFailure | undefined { + const suggestedCommand = SUGGESTED_COMMAND_MESSAGE.exec(message)?.[1]; + let kind: CommandResolutionFailureKind; + if (suggestedCommand !== undefined) { + kind = "suggested-command"; + } else if (UNKNOWN_COMMAND_MESSAGE.test(message)) { + kind = "unknown-subcommand"; + } else if (message === MISSING_COMMAND_MESSAGE) { + kind = "missing-command"; + } else { + return undefined; + } + + try { + const state = yargsInstance as YargsResolutionState; + const internals = state.getInternalMethods(); + const knownCommand = internals.getContext().commands; + const subcommands = internals.getCommandInstance().getCommands(); + const positionals = state.parsed ? state.parsed.argv?._ : undefined; + if ( + !Array.isArray(knownCommand) || + !knownCommand.every((command) => typeof command === "string") || + !Array.isArray(subcommands) + ) { + return undefined; + } + // A leaf has no subcommands, so yargs' strict-commands check reports any + // positional the leaf did not consume as an unknown command. + const knownCommandIsLeaf = + knownCommand.length > 0 && subcommands.length === 0; + return { + kind: + kind === "unknown-subcommand" && knownCommandIsLeaf + ? "surplus-positional" + : kind, + knownCommand, + knownCommandIsLeaf, + unknownTokens: Array.isArray(positionals) + ? positionals.slice(knownCommand.length).map(String) + : [], + ...(suggestedCommand === undefined ? {} : { suggestedCommand }), + }; + } catch { + return undefined; + } +} + function resolvedCommandName(yargsInstance: unknown): string | undefined { try { const commands = ( @@ -298,17 +372,46 @@ function excludesTelemetry(command: string): boolean { return root === "complete" || (root === "cli" && subcommand === "telemetry"); } +/** + * The command the user most likely meant: the recognised path, then the + * suggestion in place of the mistyped word. A typo such as + * `cf clii telemetry disable` still expresses telemetry opt-out intent. + */ +function intendedCommand(failure: CommandResolutionFailure): string { + const [first, ...rest] = failure.unknownTokens; + const next = failure.suggestedCommand ?? first; + return [ + ...failure.knownCommand, + ...(next === undefined ? [] : [next]), + ...rest, + ] + .join(" ") + .toLowerCase(); +} + async function reportParseErrorIfUnreported( error: unknown, - resolvedCommand: string | undefined + resolvedCommand: string | undefined, + resolutionFailure?: CommandResolutionFailure ): Promise { if (wasCommandReported() || error instanceof CliExit) { return; } - const command = isUnknownCommandError(error) ? undefined : resolvedCommand; + // Failures below the root keep the bucketing they had before resolution + // detail was added, so existing per-command data stays comparable: only + // yargs' singular "Unknown command:" becomes an unknown command, while a + // suggestion or several unknown words keep the resolved command's name. + // Root failures had no command name and were dropped; they are now + // reported as unknown commands. + const isUnknownCommand = + isUnknownCommandError(error) || + resolutionFailure?.knownCommand.length === 0; + const command = isUnknownCommand ? undefined : resolvedCommand; if ( - (command === undefined && !isUnknownCommandError(error)) || - excludesTelemetry(command ?? commandPath().join(" ")) + (command === undefined && !isUnknownCommand) || + excludesTelemetry(command ?? commandPath().join(" ")) || + (resolutionFailure !== undefined && + excludesTelemetry(intendedCommand(resolutionFailure))) ) { return; } @@ -319,12 +422,17 @@ async function reportParseErrorIfUnreported( const { sanitizeError } = await import("./lib/telemetry/error-sanitization.js"); const dispatcher = getTelemetryDispatcher(); + if (!dispatcher.enabled) { + return; + } + const resolution = resolutionFailure + ? await describeResolutionFailure(resolutionFailure) + : { argsUsed: [], argsCombination: "" }; const telemetryEventName = command ?? UNKNOWN_COMMAND_EVENT; const base = { command: telemetryEventName, sanitizedArgs: {}, - argsUsed: [], - argsCombination: "", + ...resolution, }; dispatcher.sendCommandEvent("cf command started", base); dispatcher.sendCommandEvent("cf command finished", { @@ -338,6 +446,16 @@ async function reportParseErrorIfUnreported( } } +async function describeResolutionFailure(failure: CommandResolutionFailure) { + const { loadCommandVocabulary, sanitizeCommandResolution } = + await import("./lib/telemetry/command-resolution.js"); + return sanitizeCommandResolution( + failure, + hideBin(process.argv), + loadCommandVocabulary(Object.keys(GLOBAL_OPTIONS)) + ); +} + async function reportHelpShown(command: string | undefined): Promise { if (!command || excludesTelemetry(command)) { return; @@ -432,6 +550,11 @@ interface PreparedUpdateCheck { interface BuildCliOptions { onCommandResolved?: (command: string) => void; + /** Receives the error `.fail` is about to throw for a resolution failure. */ + onCommandResolutionFailed?: ( + failure: CommandResolutionFailure, + error: Error + ) => void; onHelpShown?: (command: string) => void; updateCheck?: PreparedUpdateCheck; } @@ -521,7 +644,12 @@ async function prepareUpdateCheck(): Promise { * list in prose form. */ export function buildCli(rawArgs: string[], options: BuildCliOptions = {}) { - const { onCommandResolved, onHelpShown, updateCheck } = options; + const { + onCommandResolved, + onCommandResolutionFailed, + onHelpShown, + updateCheck, + } = options; let restoreCloudflareDotEnv: (() => void) | undefined; const orange = (s: string) => theme.brand(s); const delimiter = rawArgs.indexOf("--"); @@ -661,7 +789,7 @@ export function buildCli(rawArgs: string[], options: BuildCliOptions = {}) { // Its usage list contains only commands visible at the current depth, // so let parsing continue to the normal unknown-command failure when // the recommendation is hidden. - const recommendation = /^Did you mean (.+)\?$/.exec(msg)?.[1]; + const recommendation = SUGGESTED_COMMAND_MESSAGE.exec(msg)?.[1]; if (recommendation) { const visibleCommands = ( yargsInstance as unknown as { @@ -696,8 +824,13 @@ export function buildCli(rawArgs: string[], options: BuildCliOptions = {}) { // command's help first so the user sees valid usage, then // throw so the error block renders below it (closer to the // prompt). + const error = new Error(msg); + const resolutionFailure = commandResolutionFailure(cli, msg); + if (resolutionFailure) { + onCommandResolutionFailed?.(resolutionFailure, error); + } yargsInstance.showHelp("error"); - throw new Error(msg); + throw error; }); // Every depth can replace `.usage()`, so add the notice at the point @@ -779,11 +912,17 @@ export async function main(): Promise { } let resolvedCommand: string | undefined; + let resolutionFailure: + | { failure: CommandResolutionFailure; error: Error } + | undefined; let helpShown: string | undefined; const { cli, dispose } = buildCli(hideBin(process.argv), { onCommandResolved: (command) => { resolvedCommand = command; }, + onCommandResolutionFailed: (failure, error) => { + resolutionFailure = { failure, error }; + }, onHelpShown: (command) => { helpShown = command; }, @@ -834,9 +973,15 @@ export async function main(): Promise { // Completion hints must not affect command behavior. } } + // Only use the captured failure for the error `.fail` threw with it. + const failure = + resolutionFailure !== undefined && resolutionFailure.error === err + ? resolutionFailure.failure + : undefined; await reportParseErrorIfUnreported( err, - resolvedCommand ?? resolvedCommandName(cli) + resolvedCommand ?? resolvedCommandName(cli), + failure ); throw handleError(err); } finally { diff --git a/packages/cli/src/lib/telemetry/command-resolution.ts b/packages/cli/src/lib/telemetry/command-resolution.ts new file mode 100644 index 000000000..028b0505d --- /dev/null +++ b/packages/cli/src/lib/telemetry/command-resolution.ts @@ -0,0 +1,200 @@ +import { dirname, join } from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; +import { isCommandsMetadata, loadMeta } from "../metadata.js"; +import { passedFlags } from "./sanitization.js"; +import type { + CommandResolutionFailure, + CommandResolutionProperties, +} from "./types.js"; + +/** + * Sanitises what yargs could not resolve before it reaches telemetry. + * + * Unrecognised tokens are often user data (IDs, domains, resource names), so + * a token is sent only when it is a word from cf's own command catalogue and + * appears where a command word was expected (never as a surplus positional + * at a leaf). + * Anything else is replaced by a label describing its shape. A pattern-based + * allowlist would pass resource names such as `acme-prod-db`, which is why + * the vocabulary is closed. + */ + +export const MAX_UNKNOWN_TOKENS = 3; +const MAX_TOKEN_LENGTH = 64; + +/** Names yargs registers itself, outside the generated catalogue. */ +const BUILT_IN_COMMANDS = ["help", "version", "completion"]; +const BUILT_IN_OPTIONS = ["help", "version"]; + +export interface CommandVocabulary { + commands: ReadonlySet; + options: ReadonlySet; +} + +let cachedCatalogue: CommandVocabulary | null | undefined; + +function loadCatalogueVocabulary(): CommandVocabulary | null { + // The bundled chunk sits next to `dist/_meta/`; the source module needs an + // anchor inside `src/commands/`, beside `_generated/`. + const sourceAnchor = pathToFileURL( + join(dirname(fileURLToPath(import.meta.url)), "..", "..", "commands", "_") + ).href; + const metadata = + loadMeta(import.meta.url, "commands.json", isCommandsMetadata) ?? + loadMeta(sourceAnchor, "commands.json", isCommandsMetadata); + if (!metadata) { + return null; + } + + const commands = new Set(); + const options = new Set(); + const addCommandWords = (words: string[]) => { + for (const word of words) { + if (word) { + commands.add(word.toLowerCase()); + } + } + }; + for (const command of metadata.commands) { + addCommandWords(command.fullPath); + addCommandWords(command.aliases ?? []); + for (const option of command.options) { + options.add(option.name); + const aliases = option.alias === undefined ? [] : [option.alias].flat(); + for (const alias of aliases) { + // Single-character aliases are handled as short flags. + if (alias.length > 1) { + options.add(alias); + } + } + } + } + for (const group of Object.keys(metadata.descriptions ?? {})) { + addCommandWords(group.split(" ")); + } + return { commands, options }; +} + +/** + * The command words and option names cf recognises. Loading the catalogue + * costs a few milliseconds, so it happens only on this error path. If it + * cannot be loaded every unknown token is reduced to a shape label. + */ +export function loadCommandVocabulary( + globalOptions: readonly string[] +): CommandVocabulary { + if (cachedCatalogue === undefined) { + try { + cachedCatalogue = loadCatalogueVocabulary(); + } catch { + cachedCatalogue = null; + } + } + const options = new Set([...globalOptions, ...BUILT_IN_OPTIONS]); + if (!cachedCatalogue) { + return { commands: new Set(), options }; + } + return { + commands: new Set([...cachedCatalogue.commands, ...BUILT_IN_COMMANDS]), + options: new Set([...cachedCatalogue.options, ...options]), + }; +} + +const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + +/** Return a cf command word, or a label for the token's shape. */ +export function sanitizeCommandToken( + token: string, + commands: ReadonlySet +): string { + const value = token.slice(0, MAX_TOKEN_LENGTH); + const lower = value.toLowerCase(); + if (commands.has(lower)) { + return lower; + } + if (/\s/.test(value)) { + return ""; + } + if (/^[a-z][a-z0-9+.-]*:\/\//i.test(value)) { + return ""; + } + if (value.includes("@")) { + return ""; + } + if (/[/\\]/.test(value) || /^[.~]/.test(value) || /^[a-z]:$/i.test(value)) { + return ""; + } + if (value.includes(":")) { + // Wrangler-style `kv:namespace`, kept only when every part is a cf word. + const parts = lower.split(":"); + return parts.every((part) => commands.has(part)) ? lower : ""; + } + if (UUID.test(value)) { + return ""; + } + if (/^\d+$/.test(value)) { + return ""; + } + if (/^[0-9a-f]{16,}$/i.test(value)) { + return ""; + } + if (/^[a-z0-9-]+(\.[a-z0-9-]+)+$/i.test(value)) { + return ""; + } + if (/^[a-z][\w-]*$/i.test(value)) { + return ""; + } + return ""; +} + +export interface SanitizedCommandResolution extends CommandResolutionProperties { + argsUsed: string[]; + argsCombination: string; +} + +/** + * Build telemetry properties for a resolution failure. Flag names are kept + * only when they are cf options; their values are never read. + */ +export function sanitizeCommandResolution( + failure: CommandResolutionFailure, + rawArgv: string[], + vocabulary: CommandVocabulary +): SanitizedCommandResolution { + const known = new Set(); + const unknown = new Set(); + for (const flag of passedFlags(rawArgv)) { + (vocabulary.options.has(flag) ? known : unknown).add(flag); + } + const argsUsed = [...known].sort(); + // Surplus positionals at a leaf are almost always resource values, so a + // value that happens to be a cf word (`test`, `cache`) is still labelled. + const commandWords = + failure.kind === "surplus-positional" + ? new Set() + : vocabulary.commands; + const suggestedCommand = + failure.kind === "suggested-command" ? failure.suggestedCommand : undefined; + + return { + resolutionFailure: failure.kind, + knownCommand: failure.knownCommand.join(" "), + knownCommandIsLeaf: failure.knownCommandIsLeaf, + unknownTokens: failure.unknownTokens + .slice(0, MAX_UNKNOWN_TOKENS) + .map((token) => sanitizeCommandToken(token, commandWords)), + unknownTokenCount: failure.unknownTokens.length, + // yargs only suggests registered names; the pattern is a backstop. + ...(suggestedCommand !== undefined && + /^[a-z0-9][a-z0-9-]{0,63}$/i.test(suggestedCommand) + ? { suggestedCommand } + : {}), + unknownFlagCount: unknown.size, + argsUsed, + argsCombination: argsUsed.join(", "), + }; +} + +export function __resetCommandVocabularyForTests(): void { + cachedCatalogue = undefined; +} diff --git a/packages/cli/src/lib/telemetry/sanitization.ts b/packages/cli/src/lib/telemetry/sanitization.ts index 3b51e1e57..3c3d24752 100644 --- a/packages/cli/src/lib/telemetry/sanitization.ts +++ b/packages/cli/src/lib/telemetry/sanitization.ts @@ -36,9 +36,10 @@ function toCamel(flag: string): string { ); } -function passedFlags( +/** Flag names passed before `--`, without values; unknown short flags are skipped. */ +export function passedFlags( rawArgv: string[], - shortFlagAliases: Record + shortFlagAliases: Record = GLOBAL_SHORT_FLAG_ALIASES ): string[] { const flags: string[] = []; for (const token of rawArgv) { diff --git a/packages/cli/src/lib/telemetry/types.ts b/packages/cli/src/lib/telemetry/types.ts index b6fd641ad..6c3777856 100644 --- a/packages/cli/src/lib/telemetry/types.ts +++ b/packages/cli/src/lib/telemetry/types.ts @@ -30,13 +30,52 @@ export type CommandOutcome = "success" | "error" | "cancelled"; export const UNKNOWN_COMMAND_EVENT = "unknown command"; +/** + * Why yargs could not resolve the invocation to a runnable command. + * `surplus-positional` is the strict-commands check failing at a leaf, where + * any extra positional counts as an unknown command. + */ +export type CommandResolutionFailureKind = + | "unknown-subcommand" + | "surplus-positional" + | "suggested-command" + | "missing-command"; + +/** + * Raw resolution state captured from yargs when parsing fails. The unknown + * tokens are user input; only their sanitized form may be sent. + */ +export interface CommandResolutionFailure { + kind: CommandResolutionFailureKind; + /** Registered command names yargs resolved before failing. */ + knownCommand: string[]; + knownCommandIsLeaf: boolean; + unknownTokens: string[]; + /** A registered command name taken from yargs' "Did you mean" message. */ + suggestedCommand?: string; +} + +/** Sent only when the command could not be resolved. */ +export interface CommandResolutionProperties { + resolutionFailure: CommandResolutionFailureKind; + knownCommand: string; + knownCommandIsLeaf: boolean; + /** At most three tokens: cf command words or shape labels such as ``. */ + unknownTokens: string[]; + unknownTokenCount: number; + suggestedCommand?: string; + /** Flags that are not cf option names; counted, never named. */ + unknownFlagCount: number; +} + export interface SanitizedError { errorType: string | undefined; httpStatus?: number; errorCodes?: number[]; } -export interface CommandEventProperties extends CommonEventProperties { +export interface CommandEventProperties + extends CommonEventProperties, Partial { command: string; outcome?: CommandOutcome; sanitizedArgs: Record; diff --git a/packages/cli/telemetry.md b/packages/cli/telemetry.md index 42d43e125..2720ce7a8 100644 --- a/packages/cli/telemetry.md +++ b/packages/cli/telemetry.md @@ -19,6 +19,19 @@ Command telemetry uses command-started and command-finished events containing: - The canonical command path (for example, `cf zones list`), without positional values. +- When `cf` cannot match the arguments to a command, the kind of failure + (unknown subcommand, extra positional argument, a typo with a suggested + command, or no command), the part of the command path it recognised, and + whether that part is a complete command. Up to three unrecognised words are + included only when they are `cf` command names where a command name was + expected; any other word, and any extra value after a complete command, is + replaced by a category such as ``, ``, ``, ``, + ``, or ``. The number of unrecognised words, the suggested + command name, and the number of flags that are not `cf` options are also + included. Those flags are counted, never named. The command path recorded + for these events is the recognised group or command when there is one, and + `unknown command` when nothing at the top level was recognised or a single + unrecognised word follows a command. - The search query passed to `cf cli search` or `cf docs search`, to help us make sure relevant results are being returned. Queries are limited to 1,024 characters in telemetry and included only in command-finished events. @@ -60,8 +73,10 @@ requests needed to carry out commands. Apart from the search queries described above, `cf` does not include account or zone identifiers, domain names, positional values, request or file contents, file paths, credentials, raw error messages, stack traces, or API response data -in its command telemetry. Free-form flag values are redacted, and error -reporting uses only the error class, status, and numeric codes described above. +in its command telemetry. Free-form flag values are redacted, unrecognised +command words are reduced to the categories described above unless they are +`cf` command names, and error reporting uses only the error class, status, and +numeric codes described above. ## How can I inspect what is sent?