From 481b678d3c9afce6a7d79cf216949c39c85443f0 Mon Sep 17 00:00:00 2001 From: Varun S G Date: Sat, 26 Sep 2026 20:01:22 +0530 Subject: [PATCH] fix: enableLogging no longer breaks requests or logs each one 79 times sanitizeData parsed a truncated JSON string, so with logging on any request body over 1000 chars threw SyntaxError in the request interceptor and was never sent, and a large error response surfaced as SyntaxError instead of HttpError. The interceptor was also installed on the global axios instance once per resource, logging every request 79 times, touching the host application's own axios traffic, and logging OAuthClient's token requests with client_secret, code and refresh_token in plain text. Truncate to a string that never throws, install the logger once per Configuration, and only log requests under the client's /api/v1 prefix. --- examples/README.md | 5 +- src/api/BaseResource.ts | 46 ++++++++-- tests/unit/base-resource-logging.test.ts | 102 +++++++++++++++++++++++ 3 files changed, 143 insertions(+), 10 deletions(-) create mode 100644 tests/unit/base-resource-logging.test.ts diff --git a/examples/README.md b/examples/README.md index da275062..53fc436b 100644 --- a/examples/README.md +++ b/examples/README.md @@ -36,10 +36,11 @@ const client = new PlaneClient({ When enabled, you'll see detailed logs for: - 🚀 Request details (method, URL, headers, data) -- ✅ Response details (status, data, headers) - ❌ Error details (status, error message, response data) -Sensitive information like API keys and authorization tokens are automatically redacted from logs. +Sensitive information like API keys and authorization tokens are automatically redacted from logs, and bodies +longer than 1000 characters are truncated. Only the SDK's own v1 API requests are logged, once each: `OAuthClient` +token requests and your application's other axios traffic are not. `client.v2` does not log. ## Examples diff --git a/src/api/BaseResource.ts b/src/api/BaseResource.ts index e1ef3d8b..c6bb1995 100644 --- a/src/api/BaseResource.ts +++ b/src/api/BaseResource.ts @@ -2,6 +2,18 @@ import axios, { InternalAxiosRequestConfig } from "axios"; import { Configuration } from "../Configuration"; import { HttpError } from "../errors"; +/** Longest serialized body a log line carries before it is cut. */ +const MAX_LOGGED_DATA_LENGTH = 1000; + +/** + * Configurations whose request logger is already installed on the global axios instance. + * + * Every resource and sub-resource a `PlaneClient` builds runs the `BaseResource` + * constructor with the same `Configuration`. Installing once per resource logged every + * request once per resource; keying on the configuration installs it once per client. + */ +const loggingInstalledFor = new WeakSet(); + /** * Base resource class containing HTTP logic and authentication * All API resources should extend this class @@ -12,7 +24,8 @@ export abstract class BaseResource { constructor(config: Configuration) { this.config = config; - if (config.enableLogging) { + if (config.enableLogging && !loggingInstalledFor.has(config)) { + loggingInstalledFor.add(config); this.setupInterceptors(); } } @@ -37,15 +50,25 @@ export abstract class BaseResource { } /** - * Sanitize data to remove sensitive information and limit size + * Sanitize data to remove sensitive information and limit size. + * + * Only ever feeds a log line, so it must never throw: logging describes a request and + * must not change its outcome. */ - private sanitizeData(data: any): any { + private sanitizeData(data: unknown): unknown { if (!data) return data; - // If data is too large, truncate it - const dataStr = JSON.stringify(data); - if (dataStr.length > 1000) { - return JSON.parse(dataStr.substring(0, 1000)) + "... [TRUNCATED]"; + let dataStr: string | undefined; + try { + dataStr = JSON.stringify(data); + } catch { + return "[UNSERIALIZABLE]"; + } + + // If data is too large, truncate it. The cut lands mid-JSON, so keep it a string — + // `JSON.parse` on it throws. + if (dataStr !== undefined && dataStr.length > MAX_LOGGED_DATA_LENGTH) { + return `${dataStr.slice(0, MAX_LOGGED_DATA_LENGTH)}... [TRUNCATED]`; } return data; @@ -185,12 +208,19 @@ export abstract class BaseResource { } /** - * Setup axios interceptors for request and response logging + * Setup the axios request interceptor that logs requests. + * + * It sits on the global axios instance, which the host application and `OAuthClient` + * use too, so it only reports requests under this client's API prefix. Matching + * `baseUrl` alone would log `OAuthClient`'s token exchange, whose body carries the + * client secret. */ private setupInterceptors(): void { + const apiPrefix = `${this.config.baseUrl}${this.apiBasePath}`; // Request interceptor axios.interceptors.request.use( (config: InternalAxiosRequestConfig) => { + if (!config.url?.startsWith(apiPrefix)) return config; console.log("🚀 [REQUEST]", { method: config.method?.toUpperCase(), url: config.url, diff --git a/tests/unit/base-resource-logging.test.ts b/tests/unit/base-resource-logging.test.ts new file mode 100644 index 00000000..93749754 --- /dev/null +++ b/tests/unit/base-resource-logging.test.ts @@ -0,0 +1,102 @@ +import axios from "axios"; +import nock from "nock"; +import { OAuthClient } from "../../src/client/oauth-client"; +import { PlaneClient } from "../../src/client/plane-client"; +import { HttpError } from "../../src/errors"; + +/** + * `enableLogging: true` describes requests; it must never change their outcome. + * Offline, so it runs without a Plane instance. + */ +const BASE = "https://plane.example.com"; +const API_KEY = "logging-test-secret"; +const LABELS = "/api/v1/workspaces/ws/projects/p/labels/"; +const REQUEST_LOG = "🚀 [REQUEST]"; + +let log: jest.SpyInstance; + +const makeClient = () => new PlaneClient({ apiKey: API_KEY, baseUrl: BASE, enableLogging: true }); + +/** The payload of every `🚀 [REQUEST]` line logged so far. */ +const requestLogs = () => log.mock.calls.filter(([label]) => label === REQUEST_LOG).map(([, entry]) => entry); + +beforeEach(() => { + log = jest.spyOn(console, "log").mockImplementation(() => undefined); + jest.spyOn(console, "error").mockImplementation(() => undefined); +}); + +afterEach(() => { + // The logger is installed on the global axios instance; start every test without one. + axios.interceptors.request.clear(); + nock.cleanAll(); + jest.restoreAllMocks(); +}); + +describe("BaseResource request logging", () => { + it("sends a request whose body serializes past the truncation limit", async () => { + const body = { name: "big", description: "x".repeat(1500) }; + const scope = nock(BASE).post(LABELS, body).reply(201, { id: "l1", name: "big" }); + + await expect(makeClient().labels.create("ws", "p", body)).resolves.toMatchObject({ id: "l1" }); + + expect(scope.isDone()).toBe(true); + }); + + it("logs a truncated body as a string, with the api key redacted", async () => { + nock(BASE).post(LABELS).reply(201, { id: "l1" }); + + await makeClient().labels.create("ws", "p", { name: "big", description: "x".repeat(1500) }); + + const [entry] = requestLogs(); + expect(entry.data).toMatch(/\.\.\. \[TRUNCATED\]$/); + expect(JSON.stringify(entry)).not.toContain(API_KEY); + }); + + it("logs each request once, however many resources the client built", async () => { + nock(BASE).get(`${LABELS}l1/`).reply(200, { id: "l1" }); + + await makeClient().labels.retrieve("ws", "p", "l1"); + + expect(requestLogs()).toHaveLength(1); + }); + + it("raises HttpError for a failed request whose response body is large", async () => { + nock(BASE) + .get(`${LABELS}l1/`) + .reply(502, `${"x".repeat(1500)}`); + + const failure = makeClient().labels.retrieve("ws", "p", "l1"); + + await expect(failure).rejects.toBeInstanceOf(HttpError); + await expect(failure).rejects.toMatchObject({ statusCode: 502 }); + }); + + it("leaves the host application's own axios requests alone", async () => { + makeClient(); // installs the logger on the global axios instance + const scope = nock("https://elsewhere.example.com").post("/hook").reply(200, { ok: true }); + + await expect(axios.post("https://elsewhere.example.com/hook", { blob: "z".repeat(1500) })).resolves.toMatchObject({ + status: 200, + }); + + expect(scope.isDone()).toBe(true); + expect(requestLogs()).toHaveLength(0); + }); + + it("does not log the OAuth token exchange, whose body carries the client secret", async () => { + makeClient(); // same baseUrl as the OAuth client, as with the api.plane.so defaults + const scope = nock(BASE).post("/auth/o/token/").reply(200, { access_token: "a" }); + const oauth = new OAuthClient({ + baseUrl: BASE, + clientId: "client-id", + clientSecret: "oauth-client-secret", + redirectUri: "https://app.example.com/callback", + }); + + await oauth.exchangeCodeForToken("auth-code"); + + expect(scope.isDone()).toBe(true); + expect(requestLogs()).toHaveLength(0); + expect(JSON.stringify(log.mock.calls)).not.toContain("oauth-client-secret"); + }); +});