From 156830006b594cba76ef1b21a9bc0bbfdc3f7ec2 Mon Sep 17 00:00:00 2001 From: NoSoloSoft <32063006+nosolosoft@users.noreply.github.com> Date: Tue, 21 Jul 2026 00:17:08 +0200 Subject: [PATCH] fix: redact HTTP cookies from server logs (#7977) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Operators rely on Paperclip server logs for maintenance, incident triage, and support handoffs. > - The HTTP logger persisted request metadata and only redacted authorization headers. > - Request cookies and set-cookie headers can contain active session material and should not be written to durable logs. > - This pull request keeps the fix intentionally narrow: centralize the HTTP log redaction path list and include cookie-bearing headers. > - The benefit is lower credential/session leakage risk from routine server.log collection or sharing. ## Linked Issues or Issue Description No GitHub issue exists for this exact local finding. Inline bug report: - Type: security/privacy bug. - Affected area: server HTTP logging middleware. - Observed problem: local Paperclip maintenance found raw cookies present in server.log. - Expected behavior: durable HTTP logs redact authorization and cookie-bearing request/response headers. - Impact: anyone with access to copied/exported logs could see session-bearing cookie values. - Related/open PRs found during dedup search: #7242, #7306, #7346. This PR is the minimal local fix branch created from the verified local maintenance patch; those PRs may be better upstream candidates if maintainers prefer their broader coverage. ## What Changed - Added `HTTP_LOG_REDACT_PATHS` for HTTP logger redaction paths. - Kept existing `req.headers.authorization` redaction. - Added redaction for `req.headers.cookie`, request `set-cookie`, and response `set-cookie` paths. - Added focused tests asserting the required redaction paths are present and that pino-http output redacts live request/response header secrets. ## Verification - `pnpm exec vitest run server/src/__tests__/http-log-redaction.test.ts` - `pnpm --filter @paperclipai/server typecheck` - Pre-commit TruffleHog scan: 0 verified/unverified secrets. - PR CI observed passing so far for policy, Typecheck + Release Registry, Build, e2e, Socket, Snyk, security-review, and serialized/workspace suites; remaining jobs may still be running. ## Risks - Low runtime risk: this only expands pino redaction paths. - Possible coverage risk: broader redaction helpers in related PRs may cover more serialized variants beyond the pino-http request/response header pipeline tested here. - No migrations, schema changes, or UI changes. ## Model Used - OpenAI Codex via Hermes Agent, model gpt-5.5, tool-using coding/ops session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Andrew Aymeloglu --- .../src/__tests__/http-log-redaction.test.ts | 86 +++++++++++++++++++ server/src/middleware/http-log-redaction.ts | 13 +++ server/src/middleware/logger.ts | 3 +- 3 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 server/src/__tests__/http-log-redaction.test.ts create mode 100644 server/src/middleware/http-log-redaction.ts diff --git a/server/src/__tests__/http-log-redaction.test.ts b/server/src/__tests__/http-log-redaction.test.ts new file mode 100644 index 0000000000..46531b935d --- /dev/null +++ b/server/src/__tests__/http-log-redaction.test.ts @@ -0,0 +1,86 @@ +import { createServer, request } from "node:http"; +import { Writable } from "node:stream"; +import pino from "pino"; +import { pinoHttp } from "pino-http"; +import { describe, expect, it } from "vitest"; +import { HTTP_LOG_REDACT_PATHS } from "../middleware/http-log-redaction.js"; + +describe("HTTP logger redaction", () => { + it("defines the HTTP auth and cookie header paths that must be redacted", () => { + expect(HTTP_LOG_REDACT_PATHS).toContain("req.headers.authorization"); + expect(HTTP_LOG_REDACT_PATHS).toContain("req.headers.cookie"); + expect(HTTP_LOG_REDACT_PATHS).toContain('req.headers["set-cookie"]'); + expect(HTTP_LOG_REDACT_PATHS).toContain('res.headers["set-cookie"]'); + expect(HTTP_LOG_REDACT_PATHS).toContain('req.headers["proxy-authorization"]'); + expect(HTTP_LOG_REDACT_PATHS).toContain('req.headers["x-csrf-token"]'); + expect(HTTP_LOG_REDACT_PATHS).toContain('req.headers["x-xsrf-token"]'); + expect(HTTP_LOG_REDACT_PATHS).toContain('req.headers["x-api-key"]'); + }); + + it("redacts request and response header secrets from pino-http output", async () => { + const chunks: string[] = []; + const stream = new Writable({ + write(chunk, _encoding, callback) { + chunks.push(chunk.toString()); + callback(); + }, + }); + const logger = pino({ redact: [...HTTP_LOG_REDACT_PATHS] }, stream); + const httpLogger = pinoHttp({ logger }); + const server = createServer((req, res) => { + httpLogger(req, res); + res.setHeader("set-cookie", "sid=response-secret"); + res.end("ok"); + }); + + try { + await new Promise((resolve, reject) => { + server.once("error", reject); + server.listen(0, "127.0.0.1", () => resolve()); + }); + const address = server.address(); + if (!address || typeof address === "string") { + throw new Error("Expected server to listen on an ephemeral TCP port"); + } + + await new Promise((resolve, reject) => { + const client = request( + { + hostname: "127.0.0.1", + port: address.port, + path: "/redaction-check", + headers: { + authorization: "Bearer auth-secret", + cookie: "sid=request-secret", + "set-cookie": "proxy-secret", + }, + }, + (res) => { + res.resume(); + res.on("end", resolve); + }, + ); + client.on("error", reject); + client.end(); + }); + + await new Promise((resolve) => setImmediate(resolve)); + } finally { + await new Promise((resolve, reject) => { + server.close((err) => (err ? reject(err) : resolve())); + }); + } + + const output = chunks.join(""); + expect(output).not.toMatch(/auth-secret|request-secret|proxy-secret|response-secret/); + + const log = JSON.parse(output.trim()) as { + req: { headers: Record }; + res: { headers: Record }; + }; + expect(log.req.headers.authorization).toBe("[Redacted]"); + expect(log.req.headers.cookie).toBe("[Redacted]"); + expect(log.req.headers["set-cookie"]).toBe("[Redacted]"); + expect(log.res.headers["set-cookie"]).toBe("[Redacted]"); + }); +}); diff --git a/server/src/middleware/http-log-redaction.ts b/server/src/middleware/http-log-redaction.ts new file mode 100644 index 0000000000..bc4ba586a2 --- /dev/null +++ b/server/src/middleware/http-log-redaction.ts @@ -0,0 +1,13 @@ +export const HTTP_LOG_REDACT_PATHS = [ + "req.headers.authorization", + 'req.headers["proxy-authorization"]', + "req.headers.cookie", + // "set-cookie" is normally a response header; keep the request-side + // path as defensive coverage in case a proxy forwards it inbound. + 'req.headers["set-cookie"]', + 'res.headers["set-cookie"]', + // Credential- and session-paired headers with no debugging value. + 'req.headers["x-csrf-token"]', + 'req.headers["x-xsrf-token"]', + 'req.headers["x-api-key"]', +] as const; diff --git a/server/src/middleware/logger.ts b/server/src/middleware/logger.ts index e014a34ca6..2ce76d7250 100644 --- a/server/src/middleware/logger.ts +++ b/server/src/middleware/logger.ts @@ -4,6 +4,7 @@ import pino from "pino"; import { pinoHttp } from "pino-http"; import { readConfigFile } from "../config-file.js"; import { resolveDefaultLogsDir, resolveHomeAwarePath } from "../home-paths.js"; +import { HTTP_LOG_REDACT_PATHS } from "./http-log-redaction.js"; import { shouldSilenceHttpSuccessLog } from "./http-log-policy.js"; import { redactSensitive } from "./redact-sensitive.js"; @@ -30,7 +31,7 @@ const sharedOpts = { export const logger = pino({ level: "debug", - redact: ["req.headers.authorization"], + redact: [...HTTP_LOG_REDACT_PATHS], }, pino.transport({ targets: [ {