Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/deploy-positional-path-telemetry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"wrangler": minor
---

Categorise the positional path argument to `wrangler deploy` and `wrangler versions upload` in command telemetry

Command telemetry now records a coarse category for the entry-point/assets positional (`wrangler deploy <path>`) under `sanitizedArgs.path`, so we can understand whether people pass a file, a directory, or a relational reference such as `.` or `../example`. The possible values are `file`, `directory`, `current-dir`, `parent-relative`, and `not-found`, or `null` when no positional is provided. The raw path is never sent — only the category.
68 changes: 68 additions & 0 deletions packages/wrangler/src/__tests__/deploy/entry-points.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import * as esbuild from "esbuild";
import { http, HttpResponse } from "msw";
import dedent from "ts-dedent";
import { afterEach, assert, beforeEach, describe, it, vi } from "vitest";
import { logger } from "../../logger";
import { clearOutputFilePath } from "../../output";
import { mockAccountId, mockApiToken } from "../helpers/mock-account-id";
import { mockConsoleMethods } from "../helpers/mock-console";
Expand Down Expand Up @@ -1275,4 +1276,71 @@ addEventListener('fetch', event => {});`
});
});
});

describe("positional path telemetry categorisation", () => {
beforeEach(() => {
logger.loggerLevel = "debug";
});
afterEach(() => {
logger.resetLoggerLevel();
});

it("records null when no positional is passed", async ({ expect }) => {
writeWranglerConfig({ main: "index.js" });
fs.writeFileSync("index.js", "export default {};");

await runWrangler("deploy --dry-run --outdir out");

expect(std.debug).toContain('"sanitizedCommand":"deploy"');
expect(std.debug).toContain('"path":null');
});

it("categorises a file entry-point as 'file'", async ({ expect }) => {
writeWranglerConfig();
fs.writeFileSync("index.js", "export default {};");

await runWrangler("deploy index.js --dry-run --outdir out");

expect(std.debug).toContain('"sanitizedCommand":"deploy"');
expect(std.debug).toContain('"path":"file"');
// The raw path is never sent in the categorised property.
expect(std.debug).not.toContain('"path":"index.js"');
});

it("categorises a directory as 'directory'", async ({ expect }) => {
writeWranglerConfig({ name: "test-name" });
fs.mkdirSync("public");
fs.writeFileSync("public/index.html", "<h1>Hello</h1>");

await runWrangler("deploy public --dry-run --outdir out --no-autoconfig");

expect(std.debug).toContain('"path":"directory"');
});

it("categorises the current directory reference as 'current-dir'", async ({
expect,
}) => {
writeWranglerConfig({ name: "test-name" });
fs.writeFileSync("index.html", "<h1>Hello</h1>");

await runWrangler("deploy . --dry-run --outdir out --no-autoconfig");

expect(std.debug).toContain('"path":"current-dir"');
});

it("categorises a parent-relative reference as 'parent-relative'", async ({
expect,
}) => {
writeWranglerConfig();

// The path does not need to resolve — the started event (which fires
// before the handler) already carries the category. The handler then
// fails because the entry-point cannot be found.
await expect(
runWrangler("deploy ../missing-entry.js --dry-run --outdir out")
).rejects.toThrow();

expect(std.debug).toContain('"path":"parent-relative"');
});
});
});
90 changes: 90 additions & 0 deletions packages/wrangler/src/__tests__/metrics/sanitization.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import * as fs from "node:fs";
import { runInTempDir } from "@cloudflare/workers-utils/test-helpers";
import { describe, it } from "vitest";
import {
ALLOW,
categoriseArgs,
categorisePositionalPath,
COMMAND_ARG_ALLOW_LIST,
getAllowedArgs,
REDACT,
Expand Down Expand Up @@ -136,6 +140,92 @@ describe("getAllowedArgs", () => {
});
});

describe("categorisePositionalPath", () => {
runInTempDir();

it("returns null when no value is provided", ({ expect }) => {
expect(categorisePositionalPath(undefined)).toBeNull();
expect(categorisePositionalPath("")).toBeNull();
expect(categorisePositionalPath(123)).toBeNull();
});

it("categorises the current directory reference", ({ expect }) => {
expect(categorisePositionalPath(".")).toBe("current-dir");
expect(categorisePositionalPath("./")).toBe("current-dir");
});

it("categorises parent-relative references", ({ expect }) => {
expect(categorisePositionalPath("..")).toBe("parent-relative");
expect(categorisePositionalPath("../example")).toBe("parent-relative");
expect(categorisePositionalPath("../../dist")).toBe("parent-relative");
});

it("categorises an existing directory", ({ expect }) => {
fs.mkdirSync("./public");
expect(categorisePositionalPath("./public")).toBe("directory");
expect(categorisePositionalPath("public")).toBe("directory");
});

it("categorises an existing file", ({ expect }) => {
fs.writeFileSync("./index.js", "export default {};");
expect(categorisePositionalPath("./index.js")).toBe("file");
expect(categorisePositionalPath("index.js")).toBe("file");
});

it("categorises a path that does not exist as not-found", ({ expect }) => {
expect(categorisePositionalPath("does-not-exist")).toBe("not-found");
expect(categorisePositionalPath("src/missing.ts")).toBe("not-found");
});
});

describe("categoriseArgs", () => {
runInTempDir();

it("only categorises args whose allow-list entry is a categoriser", ({
expect,
}) => {
fs.mkdirSync("./public");
const allowedArgs: AllowedArgs = {
path: categorisePositionalPath,
force: ALLOW,
config: REDACT,
};
const result = categoriseArgs(
{ path: "./public", force: true, config: "wrangler.toml" },
allowedArgs
);
expect(result).toEqual({ path: "directory" });
});

it("reads positional values straight from the full args object", ({
expect,
}) => {
// `path` here mirrors the positional set by yargs, which sanitizeArgKeys
// would otherwise drop because it is not passed as `--path` in argv.
const result = categoriseArgs(
{ path: "../example" },
{ path: categorisePositionalPath }
);
expect(result).toEqual({ path: "parent-relative" });
});

it("records null when a positional is absent", ({ expect }) => {
const result = categoriseArgs(
{ path: undefined },
{ path: categorisePositionalPath }
);
expect(result).toEqual({ path: null });
});

it("omits args when the categoriser returns undefined", ({ expect }) => {
const result = categoriseArgs(
{ path: "anything" },
{ path: () => undefined }
);
expect(result).toEqual({});
});
});

describe("COMMAND_ARG_ALLOW_LIST", () => {
it("should pass boolean flag values through the full sanitisation pipeline for any command", ({
expect,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { http, HttpResponse } from "msw";
* TODO: remove this `expect` import
*/
import { beforeEach, describe, expect, it, test, vi } from "vitest";
import { logger } from "../../logger";
import * as metrics from "../../metrics";
import { dedent } from "../../utils/dedent";
import { makeApiRequestAsserter } from "../helpers/assert-request";
Expand Down Expand Up @@ -1746,6 +1747,23 @@ describe("versions upload", () => {
expect(std.out).toContain("MY_KV");
expect(std.out).toContain("--dry-run: exiting now.");
});

test("categorises the positional path in command telemetry", async ({
expect,
}) => {
logger.loggerLevel = "debug";
writeWranglerConfig({ name: "test-name" });
writeWorkerSource();

try {
await runWrangler("versions upload index.js --dry-run");
} finally {
logger.resetLoggerLevel();
}

expect(std.debug).toContain('"sanitizedCommand":"versions upload"');
expect(std.debug).toContain('"path":"file"');
});
});

// --no-bundle, --var/--define/--alias, annotations, non-versioned fields,
Expand Down
11 changes: 7 additions & 4 deletions packages/wrangler/src/core/register-yargs-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { isNonInteractiveOrCI } from "../is-interactive";
import { logger } from "../logger";
import { getMetricsDispatcher } from "../metrics";
import {
categoriseArgs,
COMMAND_ARG_ALLOW_LIST,
getAllowedArgs,
sanitizeArgKeys,
Expand Down Expand Up @@ -295,10 +296,12 @@ function createHandler(def: InternalCommandDefinition, argv: string[]) {
sanitizedCommand
);
const argsWithSanitizedKeys = sanitizeArgKeys(args, argv);
const sanitizedArgs = sanitizeArgValues(
argsWithSanitizedKeys,
allowedArgs
);
const sanitizedArgs = {
...sanitizeArgValues(argsWithSanitizedKeys, allowedArgs),
// Categorised positional args (e.g. the deploy path) are added
// separately because positionals are excluded by sanitizeArgKeys.
...categoriseArgs(args, allowedArgs),
};
const argsUsed = Object.keys(argsWithSanitizedKeys).sort();

dispatcher.sendCommandEvent(
Expand Down
80 changes: 78 additions & 2 deletions packages/wrangler/src/metrics/sanitization.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { statSync } from "node:fs";
import { UserError } from "@cloudflare/workers-utils";
import { getErrorType } from "../core/handle-errors";

Expand Down Expand Up @@ -30,22 +31,64 @@ export function sanitizeArgKeys(
export const REDACT = Symbol("REDACT");
/** Allow all values for this arg. */
export const ALLOW = Symbol("ALLOW");
/**
* Transform this arg's value into a coarse, non-identifying category label
* instead of logging it raw. Return `null` to record that the arg was absent,
* or `undefined` to omit the arg entirely.
*
* Categorisers are the safe way to gather telemetry about high-cardinality or
* potentially sensitive values (such as file paths) — only the returned label
* is ever sent, never the original value. Unlike the other allow-list values,
* categorisers are also applied to positional args (see `categoriseArgs`).
*/
export type Categoriser = (value: unknown) => string | null | undefined;
export type AllowedArgs = {
[arg: string]: unknown[] | typeof REDACT | typeof ALLOW;
[arg: string]: unknown[] | typeof REDACT | typeof ALLOW | Categoriser;
};
export type AllowList = Record<string, AllowedArgs>;

/**
* Categorises a positional path argument (e.g. the entry-point or assets path
* passed to `wrangler deploy` / `wrangler versions upload`) into a coarse,
* non-identifying label.
*
* The raw value is never returned — paths can leak sensitive information and
* are too high-cardinality to analyse. Relational references (`.`, `..`) are
* surfaced as their own categories; anything else is resolved against the
* filesystem to distinguish files from directories, mirroring how the deploy
* path positional is actually interpreted. Returns `null` when no positional
* was provided so the property is always present in telemetry.
*/
export function categorisePositionalPath(value: unknown): string | null {
if (typeof value !== "string" || value.length === 0) {
return null;
}
if (value === "." || value === "./" || value === ".\\") {
return "current-dir";
}
if (value === ".." || value.startsWith("../") || value.startsWith("..\\")) {
return "parent-relative";
}
try {
return statSync(value).isDirectory() ? "directory" : "file";
} catch {
return "not-found";
}
}

/**
* A list of all the command args that are allowed.
*
* A wildcard "<command> *" applies to all sub-commands of `<command>`.
* The top level "*" applies to all commands.
* Specific commands can override or add to the allow list.
*
* Each arg can have one of three values:
* Each arg can have one of four values:
* - an array of strings: only those specific values are allowed
* - REDACT: the arg value will always be redacted
* - ALLOW: all values for that arg are allowed
* - a Categoriser function: the value is transformed into a coarse category
* label (also applied to positional args)
*/
export const COMMAND_ARG_ALLOW_LIST: AllowList = {
// * applies to all commands.
Expand Down Expand Up @@ -98,6 +141,12 @@ export const COMMAND_ARG_ALLOW_LIST: AllowList = {
},
deploy: {
containersRollout: ["immediate", "gradual"],
// Categorise the entry-point/assets positional without logging the raw path.
path: categorisePositionalPath,
},
"versions upload": {
// `versions upload` shares the same positional path semantics as `deploy`.
path: categorisePositionalPath,
},
};

Expand Down Expand Up @@ -155,6 +204,33 @@ export function sanitizeArgValues(
return result;
}

/**
* Applies categoriser transforms from the allow-list to the raw args.
*
* Unlike {@link sanitizeArgValues}, this reads from the *full* args object
* rather than the argv-filtered set produced by {@link sanitizeArgKeys}. That
* is what allows positional args — which are otherwise deliberately excluded
* from telemetry — to be safely categorised into a fixed label. Only args whose
* allow-list entry is a {@link Categoriser} are considered, and only the label
* it returns (never the raw value) is emitted. A `null` result is recorded as-is
* (the arg was absent); an `undefined` result omits the arg entirely.
*/
export function categoriseArgs(
args: Record<string, unknown>,
allowedArgs: AllowedArgs
): Record<string, string | null> {
const result: Record<string, string | null> = {};
for (const [key, allowed] of Object.entries(allowedArgs)) {
if (typeof allowed === "function") {
const category = allowed(args[key]);
if (category !== undefined) {
result[key] = category;
}
}
}
return result;
}

/**
* Returns the canonical argument name for metrics reporting.
*/
Expand Down
6 changes: 6 additions & 0 deletions packages/wrangler/src/metrics/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@ type CommandEventProperties = CommonCommandEventProperties & {
* the sanitized args would only contain `{ format: "json" }` since that is the only allowed value.
* The `<key>` positional argument and `--namespace-id` flag would be omitted since they are not allowed.
*
* Positional args are normally excluded entirely, but an allow-list entry may
* define a categoriser that maps a positional value to a coarse, non-identifying
* label (never the raw value). For example, `wrangler deploy ./dist` records
* `{ path: "directory" }`, while `wrangler deploy` with no positional records
* `{ path: null }`.
*
* It is named `sanitizedArgs` to distinguish it from the historical `args` field which
* may have contained sensitive data in older Wrangler versions.
*/
Expand Down
Loading