From 41e21114741730a82b722ec2da50fcb55f8ac29d Mon Sep 17 00:00:00 2001 From: umair Date: Fri, 10 Apr 2026 11:27:00 +0100 Subject: [PATCH] Fix arbitrary file read vulnerability in web CLI push config commands Block push:config:set-apns and push:config:set-fcm in web CLI mode since they read files from the server filesystem (not the user's local machine). Also block --control-host in web CLI mode to prevent data exfiltration, and add file extension validation (.p12/.pfx/.p8/.json) as defense in depth. --- .claude/skills/ably-new-command/SKILL.md | 2 + src/base-command.ts | 4 + src/commands/push/config/set-apns.ts | 18 ++++ src/commands/push/config/set-fcm.ts | 8 ++ src/control-base-command.ts | 9 ++ test/fixtures/push/test-apns-cert.p12 | 1 + .../commands/push/config/set-apns.test.ts | 85 ++++++++++++++++++- .../unit/commands/push/config/set-fcm.test.ts | 62 ++++++++++++-- test/unit/commands/push/config/show.test.ts | 30 +++++++ 9 files changed, 209 insertions(+), 10 deletions(-) create mode 100644 test/fixtures/push/test-apns-cert.p12 diff --git a/.claude/skills/ably-new-command/SKILL.md b/.claude/skills/ably-new-command/SKILL.md index 65c79874..f543c3ad 100644 --- a/.claude/skills/ably-new-command/SKILL.md +++ b/.claude/skills/ably-new-command/SKILL.md @@ -429,6 +429,8 @@ If the new command shouldn't be available in the web CLI, add it to the appropri - `WEB_CLI_ANONYMOUS_RESTRICTED_COMMANDS` — for commands that expose account/app data - `INTERACTIVE_UNSUITABLE_COMMANDS` — for commands that don't work in interactive mode +**Security rule:** Any command that reads or uploads files from the filesystem (e.g., certificate uploads, key file imports) **must** be added to `WEB_CLI_RESTRICTED_COMMANDS`. The web CLI runs on a server — file-reading commands would read from the server's filesystem, not the user's local machine, which could expose server contents. There is no file upload mechanism in the web CLI to transfer local files. + ## Step 6: Validate After creating command and test files, always run: diff --git a/src/base-command.ts b/src/base-command.ts index e825422c..bb8d9cf1 100644 --- a/src/base-command.ts +++ b/src/base-command.ts @@ -57,6 +57,10 @@ export const WEB_CLI_RESTRICTED_COMMANDS = [ // config only applicable to local env "config*", + + // File-reading commands can expose server filesystem contents in web CLI mode + "push:config:set-apns", + "push:config:set-fcm", ]; /* Additional restricted commands when running in anonymous web CLI mode */ diff --git a/src/commands/push/config/set-apns.ts b/src/commands/push/config/set-apns.ts index 8515390b..df8b95bf 100644 --- a/src/commands/push/config/set-apns.ts +++ b/src/commands/push/config/set-apns.ts @@ -64,6 +64,15 @@ export default class PushConfigSetApns extends ControlBaseCommand { if (flags.certificate) { const certPath = path.resolve(flags.certificate); + const certExt = path.extname(certPath).toLowerCase(); + if (certExt !== ".p12" && certExt !== ".pfx") { + this.fail( + `Invalid certificate file type: expected a .p12 or .pfx file, got "${certExt || "(no extension)"}".`, + flags, + "pushConfigSetApns", + ); + } + if (!fs.existsSync(certPath)) { this.fail( `Certificate file not found: ${certPath}`, @@ -123,6 +132,15 @@ export default class PushConfigSetApns extends ControlBaseCommand { } const keyPath = path.resolve(flags["key-file"]); + const keyExt = path.extname(keyPath).toLowerCase(); + if (keyExt !== ".p8") { + this.fail( + `Invalid key file type: expected a .p8 file, got "${keyExt || "(no extension)"}".`, + flags, + "pushConfigSetApns", + ); + } + if (!fs.existsSync(keyPath)) { this.fail( `Key file not found: ${keyPath}`, diff --git a/src/commands/push/config/set-fcm.ts b/src/commands/push/config/set-fcm.ts index f437e4d8..2ee66007 100644 --- a/src/commands/push/config/set-fcm.ts +++ b/src/commands/push/config/set-fcm.ts @@ -33,6 +33,14 @@ export default class PushConfigSetFcm extends ControlBaseCommand { async (controlApi) => { const appId = await this.requireAppId(flags); const filePath = path.resolve(flags["service-account"]); + const ext = path.extname(filePath).toLowerCase(); + if (ext !== ".json") { + this.fail( + `Invalid service account file type: expected a .json file, got "${ext || "(no extension)"}".`, + flags, + "pushConfigSetFcm", + ); + } if (!fs.existsSync(filePath)) { this.fail( diff --git a/src/control-base-command.ts b/src/control-base-command.ts index a6e1da1d..8b422b14 100644 --- a/src/control-base-command.ts +++ b/src/control-base-command.ts @@ -3,6 +3,7 @@ import { controlApiFlags } from "./flags.js"; import { ControlApi, App } from "./services/control-api.js"; import { BaseFlags } from "./types/cli.js"; import { errorMessage } from "./utils/errors.js"; +import isWebCliMode from "./utils/web-mode.js"; export abstract class ControlBaseCommand extends AblyBaseCommand { // Control API commands get core + hidden control API flags @@ -35,6 +36,14 @@ export abstract class ControlBaseCommand extends AblyBaseCommand { ); } + if (isWebCliMode() && flags["control-host"]) { + this.fail( + "The --control-host flag is not available in web CLI mode.", + flags, + "auth", + ); + } + return new ControlApi({ accessToken, controlHost: flags["control-host"], diff --git a/test/fixtures/push/test-apns-cert.p12 b/test/fixtures/push/test-apns-cert.p12 new file mode 100644 index 00000000..90e0a5cb --- /dev/null +++ b/test/fixtures/push/test-apns-cert.p12 @@ -0,0 +1 @@ +fake-p12-certificate-data \ No newline at end of file diff --git a/test/unit/commands/push/config/set-apns.test.ts b/test/unit/commands/push/config/set-apns.test.ts index a45b97a7..0865fd14 100644 --- a/test/unit/commands/push/config/set-apns.test.ts +++ b/test/unit/commands/push/config/set-apns.test.ts @@ -17,6 +17,7 @@ import { parseJsonOutput } from "../../../../helpers/ndjson.js"; describe("push:config:set-apns command", () => { let appId: string; const p8FixturePath = resolve("test/fixtures/push/test-apns-key.p8"); + const p12FixturePath = resolve("test/fixtures/push/test-apns-cert.p12"); beforeEach(() => { const ctx = getControlApiContext(); @@ -67,7 +68,7 @@ describe("push:config:set-apns command", () => { .reply(200, { id: "cert-123" }); const { stderr } = await runCommand( - ["push:config:set-apns", "--certificate", p8FixturePath], + ["push:config:set-apns", "--certificate", p12FixturePath], import.meta.url, ); @@ -107,7 +108,7 @@ describe("push:config:set-apns command", () => { .reply(200, { id: "cert-123" }); const { stdout } = await runCommand( - ["push:config:set-apns", "--certificate", p8FixturePath, "--json"], + ["push:config:set-apns", "--certificate", p12FixturePath, "--json"], import.meta.url, ); @@ -206,7 +207,7 @@ describe("push:config:set-apns command", () => { .reply(200, { id: appId, apnsUseSandboxEndpoint: true }); const { stderr } = await runCommand( - ["push:config:set-apns", "--certificate", p8FixturePath, "--sandbox"], + ["push:config:set-apns", "--certificate", p12FixturePath, "--sandbox"], import.meta.url, ); @@ -223,6 +224,84 @@ describe("push:config:set-apns command", () => { }); }); + describe("file extension validation", () => { + it("should reject certificate files without .p12 or .pfx extension", async () => { + const { error } = await runCommand( + ["push:config:set-apns", "--certificate", "/etc/passwd"], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toContain("Invalid certificate file type"); + expect(error?.message).toContain(".p12 or .pfx"); + }); + + it("should reject key files without .p8 extension", async () => { + const { error } = await runCommand( + [ + "push:config:set-apns", + "--key-file", + "/some/file.txt", + "--key-id", + "KEY123", + "--team-id", + "TEAM456", + "--topic", + "com.example.app", + ], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toContain("Invalid key file type"); + expect(error?.message).toContain(".p8"); + }); + + it("should accept .pfx certificate files", async () => { + nockControl() + .post(`/v1/apps/${appId}/pkcs12`) + .reply(200, { id: "cert-123" }); + + // The file won't exist, but extension validation should pass + const { error } = await runCommand( + ["push:config:set-apns", "--certificate", "/nonexistent/cert.pfx"], + import.meta.url, + ); + + // Should fail with "not found", not "invalid file type" + expect(error).toBeDefined(); + expect(error?.message).toContain("not found"); + }); + }); + + describe("web CLI restrictions", () => { + let originalWebCliMode: string | undefined; + + beforeEach(() => { + originalWebCliMode = process.env.ABLY_WEB_CLI_MODE; + }); + + afterEach(() => { + if (originalWebCliMode === undefined) { + delete process.env.ABLY_WEB_CLI_MODE; + } else { + process.env.ABLY_WEB_CLI_MODE = originalWebCliMode; + } + }); + + it("should be restricted in web CLI mode", async () => { + process.env.ABLY_WEB_CLI_MODE = "true"; + + const { error } = await runCommand( + ["push:config:set-apns", "--certificate", p12FixturePath], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toContain("not available in the web CLI"); + }); + }); + describe("error handling", () => { standardControlApiErrorTests({ commandArgs: [ diff --git a/test/unit/commands/push/config/set-fcm.test.ts b/test/unit/commands/push/config/set-fcm.test.ts index 9004ad50..92bd30bb 100644 --- a/test/unit/commands/push/config/set-fcm.test.ts +++ b/test/unit/commands/push/config/set-fcm.test.ts @@ -84,13 +84,20 @@ describe("push:config:set-fcm command", () => { }); it("should fail when service account file is not valid JSON", async () => { - const invalidPath = resolve("test/fixtures/push/test-apns-key.p8"); - const { error } = await runCommand( - ["push:config:set-fcm", "--service-account", invalidPath], - import.meta.url, - ); - - expect(error).toBeDefined(); + const tempDir = mkdtempSync(join(tmpdir(), "ably-cli-test-")); + const tempPath = join(tempDir, "invalid.json"); + writeFileSync(tempPath, "not valid json content"); + try { + const { error } = await runCommand( + ["push:config:set-fcm", "--service-account", tempPath], + import.meta.url, + ); + expect(error).toBeDefined(); + expect(error?.message).toContain("not valid JSON"); + } finally { + unlinkSync(tempPath); + rmdirSync(tempDir); + } }); it("should require service-account flag", async () => { @@ -138,6 +145,47 @@ describe("push:config:set-fcm command", () => { }); }); + describe("file extension validation", () => { + it("should reject service account files without .json extension", async () => { + const { error } = await runCommand( + ["push:config:set-fcm", "--service-account", "/etc/passwd"], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toContain("Invalid service account file type"); + expect(error?.message).toContain(".json"); + }); + }); + + describe("web CLI restrictions", () => { + let originalWebCliMode: string | undefined; + + beforeEach(() => { + originalWebCliMode = process.env.ABLY_WEB_CLI_MODE; + }); + + afterEach(() => { + if (originalWebCliMode === undefined) { + delete process.env.ABLY_WEB_CLI_MODE; + } else { + process.env.ABLY_WEB_CLI_MODE = originalWebCliMode; + } + }); + + it("should be restricted in web CLI mode", async () => { + process.env.ABLY_WEB_CLI_MODE = "true"; + + const { error } = await runCommand( + ["push:config:set-fcm", "--service-account", fcmFixturePath], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toContain("not available in the web CLI"); + }); + }); + describe("error handling", () => { standardControlApiErrorTests({ commandArgs: ["push:config:set-fcm", "--service-account", fcmFixturePath], diff --git a/test/unit/commands/push/config/show.test.ts b/test/unit/commands/push/config/show.test.ts index f328bcfd..8c6a883d 100644 --- a/test/unit/commands/push/config/show.test.ts +++ b/test/unit/commands/push/config/show.test.ts @@ -177,6 +177,36 @@ describe("push:config:show command", () => { }); }); + describe("--control-host web CLI restriction", () => { + let originalWebCliMode: string | undefined; + + beforeEach(() => { + originalWebCliMode = process.env.ABLY_WEB_CLI_MODE; + }); + + afterEach(() => { + if (originalWebCliMode === undefined) { + delete process.env.ABLY_WEB_CLI_MODE; + } else { + process.env.ABLY_WEB_CLI_MODE = originalWebCliMode; + } + }); + + it("should reject --control-host in web CLI mode", async () => { + process.env.ABLY_WEB_CLI_MODE = "true"; + + const { error } = await runCommand( + ["push:config:show", "--control-host", "attacker.com"], + import.meta.url, + ); + + expect(error).toBeDefined(); + expect(error?.message).toContain( + "--control-host flag is not available in web CLI mode", + ); + }); + }); + describe("error handling", () => { standardControlApiErrorTests({ commandArgs: ["push:config:show"],