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"); + }); +});