Skip to content

Commit de6d57b

Browse files
committed
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) as defense in depth.
1 parent 8e594d9 commit de6d57b

7 files changed

Lines changed: 146 additions & 3 deletions

File tree

.claude/skills/ably-new-command/SKILL.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,8 @@ If the new command shouldn't be available in the web CLI, add it to the appropri
429429
- `WEB_CLI_ANONYMOUS_RESTRICTED_COMMANDS`for commands that expose account/app data
430430
- `INTERACTIVE_UNSUITABLE_COMMANDS`for commands that don't work in interactive mode
431431

432+
**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 serverfile-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.
433+
432434
## Step 6: Validate
433435

434436
After creating command and test files, always run:

src/base-command.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ export const WEB_CLI_RESTRICTED_COMMANDS = [
5757

5858
// config only applicable to local env
5959
"config*",
60+
61+
// File-reading commands can expose server filesystem contents in web CLI mode
62+
"push:config:set-apns",
63+
"push:config:set-fcm",
6064
];
6165

6266
/* Additional restricted commands when running in anonymous web CLI mode */

src/commands/push/config/set-apns.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,15 @@ export default class PushConfigSetApns extends ControlBaseCommand {
6464

6565
if (flags.certificate) {
6666
const certPath = path.resolve(flags.certificate);
67+
const certExt = path.extname(certPath).toLowerCase();
68+
if (certExt !== ".p12" && certExt !== ".pfx") {
69+
this.fail(
70+
`Invalid certificate file type: expected a .p12 or .pfx file, got "${certExt || "(no extension)"}".`,
71+
flags,
72+
"pushConfigSetApns",
73+
);
74+
}
75+
6776
if (!fs.existsSync(certPath)) {
6877
this.fail(
6978
`Certificate file not found: ${certPath}`,
@@ -123,6 +132,15 @@ export default class PushConfigSetApns extends ControlBaseCommand {
123132
}
124133

125134
const keyPath = path.resolve(flags["key-file"]);
135+
const keyExt = path.extname(keyPath).toLowerCase();
136+
if (keyExt !== ".p8") {
137+
this.fail(
138+
`Invalid key file type: expected a .p8 file, got "${keyExt || "(no extension)"}".`,
139+
flags,
140+
"pushConfigSetApns",
141+
);
142+
}
143+
126144
if (!fs.existsSync(keyPath)) {
127145
this.fail(
128146
`Key file not found: ${keyPath}`,

src/control-base-command.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { controlApiFlags } from "./flags.js";
33
import { ControlApi, App } from "./services/control-api.js";
44
import { BaseFlags } from "./types/cli.js";
55
import { errorMessage } from "./utils/errors.js";
6+
import isWebCliMode from "./utils/web-mode.js";
67

78
export abstract class ControlBaseCommand extends AblyBaseCommand {
89
// Control API commands get core + hidden control API flags
@@ -35,6 +36,14 @@ export abstract class ControlBaseCommand extends AblyBaseCommand {
3536
);
3637
}
3738

39+
if (isWebCliMode() && flags["control-host"]) {
40+
this.fail(
41+
"The --control-host flag is not available in web CLI mode.",
42+
flags,
43+
"auth",
44+
);
45+
}
46+
3847
return new ControlApi({
3948
accessToken,
4049
controlHost: flags["control-host"],
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
fake-p12-certificate-data

test/unit/commands/push/config/set-apns.test.ts

Lines changed: 82 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { parseJsonOutput } from "../../../../helpers/ndjson.js";
1717
describe("push:config:set-apns command", () => {
1818
let appId: string;
1919
const p8FixturePath = resolve("test/fixtures/push/test-apns-key.p8");
20+
const p12FixturePath = resolve("test/fixtures/push/test-apns-cert.p12");
2021

2122
beforeEach(() => {
2223
const ctx = getControlApiContext();
@@ -67,7 +68,7 @@ describe("push:config:set-apns command", () => {
6768
.reply(200, { id: "cert-123" });
6869

6970
const { stderr } = await runCommand(
70-
["push:config:set-apns", "--certificate", p8FixturePath],
71+
["push:config:set-apns", "--certificate", p12FixturePath],
7172
import.meta.url,
7273
);
7374

@@ -107,7 +108,7 @@ describe("push:config:set-apns command", () => {
107108
.reply(200, { id: "cert-123" });
108109

109110
const { stdout } = await runCommand(
110-
["push:config:set-apns", "--certificate", p8FixturePath, "--json"],
111+
["push:config:set-apns", "--certificate", p12FixturePath, "--json"],
111112
import.meta.url,
112113
);
113114

@@ -206,7 +207,7 @@ describe("push:config:set-apns command", () => {
206207
.reply(200, { id: appId, apnsUseSandboxEndpoint: true });
207208

208209
const { stderr } = await runCommand(
209-
["push:config:set-apns", "--certificate", p8FixturePath, "--sandbox"],
210+
["push:config:set-apns", "--certificate", p12FixturePath, "--sandbox"],
210211
import.meta.url,
211212
);
212213

@@ -223,6 +224,84 @@ describe("push:config:set-apns command", () => {
223224
});
224225
});
225226

227+
describe("file extension validation", () => {
228+
it("should reject certificate files without .p12 or .pfx extension", async () => {
229+
const { error } = await runCommand(
230+
["push:config:set-apns", "--certificate", "/etc/passwd"],
231+
import.meta.url,
232+
);
233+
234+
expect(error).toBeDefined();
235+
expect(error?.message).toContain("Invalid certificate file type");
236+
expect(error?.message).toContain(".p12 or .pfx");
237+
});
238+
239+
it("should reject key files without .p8 extension", async () => {
240+
const { error } = await runCommand(
241+
[
242+
"push:config:set-apns",
243+
"--key-file",
244+
"/some/file.txt",
245+
"--key-id",
246+
"KEY123",
247+
"--team-id",
248+
"TEAM456",
249+
"--topic",
250+
"com.example.app",
251+
],
252+
import.meta.url,
253+
);
254+
255+
expect(error).toBeDefined();
256+
expect(error?.message).toContain("Invalid key file type");
257+
expect(error?.message).toContain(".p8");
258+
});
259+
260+
it("should accept .pfx certificate files", async () => {
261+
nockControl()
262+
.post(`/v1/apps/${appId}/pkcs12`)
263+
.reply(200, { id: "cert-123" });
264+
265+
// The file won't exist, but extension validation should pass
266+
const { error } = await runCommand(
267+
["push:config:set-apns", "--certificate", "/nonexistent/cert.pfx"],
268+
import.meta.url,
269+
);
270+
271+
// Should fail with "not found", not "invalid file type"
272+
expect(error).toBeDefined();
273+
expect(error?.message).toContain("not found");
274+
});
275+
});
276+
277+
describe("web CLI restrictions", () => {
278+
let originalWebCliMode: string | undefined;
279+
280+
beforeEach(() => {
281+
originalWebCliMode = process.env.ABLY_WEB_CLI_MODE;
282+
});
283+
284+
afterEach(() => {
285+
if (originalWebCliMode === undefined) {
286+
delete process.env.ABLY_WEB_CLI_MODE;
287+
} else {
288+
process.env.ABLY_WEB_CLI_MODE = originalWebCliMode;
289+
}
290+
});
291+
292+
it("should be restricted in web CLI mode", async () => {
293+
process.env.ABLY_WEB_CLI_MODE = "true";
294+
295+
const { error } = await runCommand(
296+
["push:config:set-apns", "--certificate", p12FixturePath],
297+
import.meta.url,
298+
);
299+
300+
expect(error).toBeDefined();
301+
expect(error?.message).toContain("not available in the web CLI");
302+
});
303+
});
304+
226305
describe("error handling", () => {
227306
standardControlApiErrorTests({
228307
commandArgs: [

test/unit/commands/push/config/show.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,36 @@ describe("push:config:show command", () => {
177177
});
178178
});
179179

180+
describe("--control-host web CLI restriction", () => {
181+
let originalWebCliMode: string | undefined;
182+
183+
beforeEach(() => {
184+
originalWebCliMode = process.env.ABLY_WEB_CLI_MODE;
185+
});
186+
187+
afterEach(() => {
188+
if (originalWebCliMode === undefined) {
189+
delete process.env.ABLY_WEB_CLI_MODE;
190+
} else {
191+
process.env.ABLY_WEB_CLI_MODE = originalWebCliMode;
192+
}
193+
});
194+
195+
it("should reject --control-host in web CLI mode", async () => {
196+
process.env.ABLY_WEB_CLI_MODE = "true";
197+
198+
const { error } = await runCommand(
199+
["push:config:show", "--control-host", "attacker.com"],
200+
import.meta.url,
201+
);
202+
203+
expect(error).toBeDefined();
204+
expect(error?.message).toContain(
205+
"--control-host flag is not available in web CLI mode",
206+
);
207+
});
208+
});
209+
180210
describe("error handling", () => {
181211
standardControlApiErrorTests({
182212
commandArgs: ["push:config:show"],

0 commit comments

Comments
 (0)