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 <aaymeloglu@gmail.com>
This commit is contained in:
parent
b94907f8af
commit
156830006b
|
|
@ -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<void>((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<void>((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<void>((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<string, string> };
|
||||
res: { headers: Record<string, string> };
|
||||
};
|
||||
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]");
|
||||
});
|
||||
});
|
||||
|
|
@ -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;
|
||||
|
|
@ -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: [
|
||||
{
|
||||
|
|
|
|||
Loading…
Reference in New Issue