From 1eddc8902e57aef6c22b100dfcec72d064691186 Mon Sep 17 00:00:00 2001 From: joel <13933101+lurkshark@users.noreply.github.com> Date: Tue, 10 Nov 2026 18:29:25 -0800 Subject: [PATCH 1/2] CUS-136: Honor --debug in p0 allow and print command debug output `p0 allow` forwards unknown options to P0, so `--debug` was sent to the backend as an allow argument and never produced any debug output. The command now declares `--debug` so the CLI consumes it, and passes it to `authenticate()`. `--help` still reaches P0. `fetchCommand` now prints the argv it sends and the raw response or error it gets back to stderr when `debug` is set. It prints only those values, never the token or headers. Since the output lives in `fetchCommand`, `p0 request`, `p0 grant` and non-admin `p0 ls` print the same lines under `--debug`. Tests cover the flag handling in `allow` and the debug output in `fetchCommand`. --- src/commands/__tests__/allow.test.ts | 67 ++++++++++++++++++++++++++++ src/commands/allow.ts | 6 ++- src/drivers/__tests__/api.test.ts | 66 ++++++++++++++++++++++++++- src/drivers/api.ts | 30 ++++++++----- 4 files changed, 156 insertions(+), 13 deletions(-) create mode 100644 src/commands/__tests__/allow.test.ts diff --git a/src/commands/__tests__/allow.test.ts b/src/commands/__tests__/allow.test.ts new file mode 100644 index 00000000..e3a0a6d1 --- /dev/null +++ b/src/commands/__tests__/allow.test.ts @@ -0,0 +1,67 @@ +/** Copyright © 2024-present P0 Security + +This file is part of @p0security/cli + +@p0security/cli is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, version 3 of the License. + +@p0security/cli is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details. + +You should have received a copy of the GNU General Public License along with @p0security/cli. If not, see . +**/ +import { fetchCommand } from "../../drivers/api"; +import { authenticate } from "../../drivers/auth"; +import { allowCommand } from "../allow"; +import { beforeEach, describe, expect, it, vi, Mock } from "vitest"; +import yargs from "yargs"; + +vi.mock("../../drivers/api"); +vi.mock("../../drivers/auth", () => ({ authenticate: vi.fn() })); +vi.mock("../../drivers/stdio"); + +const mockFetchCommand = fetchCommand as Mock; +const mockAuthenticate = authenticate as Mock; + +describe("allow", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockFetchCommand.mockResolvedValue({ ok: true, message: "a message" }); + }); + + it("consumes --debug instead of forwarding it to P0", async () => { + await allowCommand(yargs()).parse("allow gcloud role viewer --debug"); + expect(mockFetchCommand).toHaveBeenCalledWith( + undefined, + expect.objectContaining({ debug: true }), + ["allow", "gcloud", "role", "viewer"] + ); + expect(mockAuthenticate).toHaveBeenCalledWith({ debug: true }); + }); + + it("consumes --debug when it comes before the arguments", async () => { + await allowCommand(yargs()).parse("allow --debug gcloud role viewer"); + expect(mockFetchCommand).toHaveBeenCalledWith( + undefined, + expect.objectContaining({ debug: true }), + ["allow", "gcloud", "role", "viewer"] + ); + }); + + it("does not set debug without --debug", async () => { + await allowCommand(yargs()).parse("allow gcloud role viewer"); + expect(mockFetchCommand).toHaveBeenCalledWith( + undefined, + expect.not.objectContaining({ debug: true }), + ["allow", "gcloud", "role", "viewer"] + ); + expect(mockAuthenticate).toHaveBeenCalledWith({ debug: undefined }); + }); + + it("forwards --help to P0", async () => { + await allowCommand(yargs()).parse("allow --help"); + expect(mockFetchCommand).toHaveBeenCalledWith( + undefined, + expect.anything(), + ["allow", "--help"] + ); + }); +}); diff --git a/src/commands/allow.ts b/src/commands/allow.ts index f075d53b..1c3e309e 100644 --- a/src/commands/allow.ts +++ b/src/commands/allow.ts @@ -25,6 +25,7 @@ const allowArgs = (yargs: yargs.Argv) => default: false, describe: "Block until the command is completed", }) + .option("debug", { type: "boolean", describe: "Print debug information." }) .option("arguments", { array: true, string: true, @@ -32,7 +33,7 @@ const allowArgs = (yargs: yargs.Argv) => }); export const allowCommand = (yargs: yargs.Argv) => - yargs.command<{ arguments: string[] }>( + yargs.command<{ arguments: string[]; debug?: boolean }>( "allow [arguments..]", "Create standing access for a resource", allowArgs, @@ -45,10 +46,11 @@ export const allow = async ( args: yargs.ArgumentsCamelCase<{ arguments: string[]; wait?: boolean; + debug?: boolean; }>, authn?: Authn ): Promise => { - const resolvedAuthn = authn ?? (await authenticate()); + const resolvedAuthn = authn ?? (await authenticate({ debug: args.debug })); const data = await fetchCommand(resolvedAuthn, args, [ "allow", ...args.arguments, diff --git a/src/drivers/__tests__/api.test.ts b/src/drivers/__tests__/api.test.ts index 888650a6..b368d2cb 100644 --- a/src/drivers/__tests__/api.test.ts +++ b/src/drivers/__tests__/api.test.ts @@ -11,12 +11,14 @@ You should have received a copy of the GNU General Public License along with @p0 import * as config from "../../drivers/config"; import { defaultConfig } from "../../drivers/env"; import { Authn } from "../../types/identity"; -import { fetchWithStreaming, requestStatusUrl } from "../api"; +import { fetchCommand, fetchWithStreaming, requestStatusUrl } from "../api"; +import { print2 } from "../stdio"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; // Mock dependencies vi.mock("../config"); vi.mock("../env"); +vi.mock("../stdio"); vi.mock("../../version", () => ({ getUserAgent: () => "P0 CLI/1.0.0 (npm build; linux; x64; node/v20.0.0)", })); @@ -478,3 +480,65 @@ describe("fetchWithStreaming", () => { ); }); }); + +describe("fetchCommand", () => { + const mockAuthn: Authn = { + getToken: vi.fn().mockResolvedValue("mock-token"), + identity: { + org: { slug: "test-org" }, + }, + } as unknown as Authn; + const debugArgs = { $0: "p0", _: [], debug: true }; + + beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(mockAuthn.getToken).mockResolvedValue("mock-token"); + vi.spyOn(config, "getTenantConfig").mockReturnValue(defaultConfig); + }); + afterEach(() => { + vi.restoreAllMocks(); + }); + + const mockSuccess = () => + vi.spyOn(global, "fetch").mockResolvedValue({ + ok: true, + status: 200, + text: async () => JSON.stringify({ ok: true, message: "done" }), + } as Response); + + it("prints argv and response when debug is set", async () => { + mockSuccess(); + await fetchCommand(mockAuthn, debugArgs, ["allow", "x"]); + expect(print2).toHaveBeenCalledWith('Sending command: ["allow","x"]'); + expect(print2).toHaveBeenCalledWith( + 'Command response: {"ok":true,"message":"done"}' + ); + const printed = JSON.stringify(vi.mocked(print2).mock.calls); + expect(printed).not.toContain("mock-token"); + expect(printed).not.toContain("Bearer"); + }); + + it("prints the error when debug is set", async () => { + vi.spyOn(global, "fetch").mockResolvedValue({ + ok: false, + status: 400, + text: async () => JSON.stringify({ error: "Resource not found" }), + } as Response); + await expect( + fetchCommand(mockAuthn, debugArgs, ["allow", "x"]) + ).rejects.toThrow("Resource not found"); + expect(print2).toHaveBeenCalledWith('Command error: "Resource not found"'); + const printed = JSON.stringify(vi.mocked(print2).mock.calls); + expect(printed).not.toContain("mock-token"); + expect(printed).not.toContain("Bearer"); + }); + + it("prints nothing when debug is not set", async () => { + mockSuccess(); + await fetchCommand(mockAuthn, { $0: "p0", _: [], debug: undefined }, [ + "allow", + "x", + ]); + expect(print2).not.toHaveBeenCalled(); + }); +}); diff --git a/src/drivers/api.ts b/src/drivers/api.ts index 6ad15d56..493f8a9a 100644 --- a/src/drivers/api.ts +++ b/src/drivers/api.ts @@ -78,16 +78,26 @@ export const fetchCommand = async ( authn: Authn, args: yargs.ArgumentsCamelCase<{ debug?: boolean }>, argv: string[] -) => - authFetch(authn, { - url: commandUrl(authn.identity.org.slug), - method: "POST", - body: JSON.stringify({ - argv, - scriptName: path.basename(args.$0), - }), - debug: args.debug, - }); +) => { + const { debug } = args; + if (debug) print2(`Sending command: ${JSON.stringify(argv)}`); + try { + const data = await authFetch(authn, { + url: commandUrl(authn.identity.org.slug), + method: "POST", + body: JSON.stringify({ + argv, + scriptName: path.basename(args.$0), + }), + debug, + }); + if (debug) print2(`Command response: ${JSON.stringify(data)}`); + return data; + } catch (error) { + if (debug) print2(`Command error: ${JSON.stringify(error)}`); + throw error; + } +}; /** Special admin 'ls' command that can retrieve results for all users. Requires 'owner' permission. */ export const fetchAdminLsCommand = async ( From 993f96f2b29bdaf9042a3db3de039f508dea040f Mon Sep 17 00:00:00 2001 From: joel <13933101+lurkshark@users.noreply.github.com> Date: Tue, 10 Nov 2026 18:33:51 -0800 Subject: [PATCH 2/2] Format fetchCommand debug errors safely (CodeRabbit) Error rejections printed as {} and circular values could throw from the debug line. Use the stack or message for Errors and fall back when serialization fails. --- src/drivers/api.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/drivers/api.ts b/src/drivers/api.ts index 493f8a9a..befa9885 100644 --- a/src/drivers/api.ts +++ b/src/drivers/api.ts @@ -74,6 +74,17 @@ export const fetchStreamingStatus = async function* ( ); }; +// JSON.stringify prints Error instances as {} and throws on circular values +const formatDebugError = (error: unknown) => { + try { + return error instanceof Error + ? (error.stack ?? error.message) + : (JSON.stringify(error) ?? String(error)); + } catch { + return "[unserializable error]"; + } +}; + export const fetchCommand = async ( authn: Authn, args: yargs.ArgumentsCamelCase<{ debug?: boolean }>, @@ -94,7 +105,7 @@ export const fetchCommand = async ( if (debug) print2(`Command response: ${JSON.stringify(data)}`); return data; } catch (error) { - if (debug) print2(`Command error: ${JSON.stringify(error)}`); + if (debug) print2(`Command error: ${formatDebugError(error)}`); throw error; } };