From a67b936885855551ee2e98be32bab250c6ba41fb Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 9 Nov 2026 13:45:00 -0600 Subject: [PATCH] fix(server): redact invitation bearer paths from HTTP logs (#20343) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - People join a company through an invitation link. The link carries a bearer token in its URL path. > - The HTTP logger removes query secrets and URL user information. It keeps the path. > - So invitation bearer tokens reach the HTTP log in the request URL, the log message, and the referrer header. > - PR #17848 fixed this, but it did not merge. Since then, #13742 changed the same redactor file. > - This pull request carries the #17848 commits onto current master. The original author is kept on both commits. > - The benefit is that invitation tokens do not stay in HTTP logs. The chat eval for membership management can then accept a real invitation safely. ## Linked Issues or Issue Description Supersedes #17848 (same commits, rebased onto current master after #13742). **What happened?** The HTTP logger writes invitation bearer tokens to logs. A synthetic POST to `/api/invites//accept` puts the token in `req.url`, in the log message, and in the `Referer`/`Referrer` header record. **Expected behavior** Logs keep the invitation route, the method, and the HTTP status. Logs do not keep the bearer token. **Steps to reproduce** 1. Check out master. 2. Run `server/src/__tests__/logger-invitation-redaction.test.ts` from this PR. 3. The synthetic token appears in the request URL, the message, and the referrer. 12 tests fail on master. **Paperclip version or commit** Master at the merge base of this branch. **Deployment mode** Source server. The tests use a disposable local HTTP listener. They use no database and no provider. ## What Changed - Remove the bearer segment from `/api/invites//...`, `/invites//...`, and `/invite/` URLs, including mounted paths and absolute URLs. - Apply the same URL policy to `referer` and `referrer` request headers. - Keep record IDs on the non-token revoke route `/invites//revoke` and on company-scoped invite routes. - Add regression tests through the real HTTP logger for 202, 400 and 500 responses, with both referrer header spellings. - Original work by @cryppadotta in #17848. This PR only rebases it onto master. ## Verification - From `server`: `node ../node_modules/vitest/vitest.mjs run --config vitest.config.ts src/__tests__/logger-invitation-redaction.test.ts src/__tests__/redact-sensitive.test.ts src/__tests__/http-log-policy.test.ts` — 46 passed. - With the master `logger.ts` and `redact-sensitive.ts`, the same new tests fail: 12 failed. - `http-log-aborted-response`, `cloud-principal-log` and `setup-token-session` tests pass locally. Some other HTTP-log suites could not load locally because workspace package builds were missing in this worktree. CI covers them. ## Risks - Low risk. Referrer query and fragment values are now removed from request logs, as they already are for request URLs. This removes some diagnostic detail to protect secrets. - A route that uses `/invites//...` for a non-bearer record ID, other than `revoke`, now shows `[REDACTED]` in logs for that segment. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), tool use and code execution, used to rebase and verify. The original change was authored in #17848. ## 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 not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Paperclip --- .../logger-invitation-redaction.test.ts | 30 +++++++++++++++++++ server/src/__tests__/redact-sensitive.test.ts | 16 ++++++++++ server/src/middleware/logger.ts | 6 ++++ server/src/middleware/redact-sensitive.ts | 7 +++-- 4 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 server/src/__tests__/logger-invitation-redaction.test.ts diff --git a/server/src/__tests__/logger-invitation-redaction.test.ts b/server/src/__tests__/logger-invitation-redaction.test.ts new file mode 100644 index 00000000000..df025a8d6d0 --- /dev/null +++ b/server/src/__tests__/logger-invitation-redaction.test.ts @@ -0,0 +1,30 @@ +import { Writable } from "node:stream"; +import express from "express"; +import pino from "pino"; +import request from "supertest"; +import { describe, expect, it } from "vitest"; +import { createHttpLogger } from "../middleware/logger.js"; +import { HTTP_LOG_REDACT_PATHS } from "../middleware/http-log-redaction.js"; + +describe("invitation bearer HTTP logs", () => { + it.each([202, 400, 500].flatMap(status => ["Referer", "Referrer"].map(header => ({ status, header }))))("keeps bearer paths private in $status request records with $header", async ({ status, header }) => { + const canary = "synthetic-invitation-bearer-canary"; + const chunks: string[] = []; + const stream = new Writable({ write(chunk, _encoding, callback) { chunks.push(chunk.toString()); callback(); } }); + const app = express(); + app.use(createHttpLogger(pino({ redact: [...HTTP_LOG_REDACT_PATHS] }, stream))); + const router = express.Router(); + router.post("/invites/:token/accept", (_req, res) => res.status(status).end()); + app.use("/api", router); + const response = await request(app).post(`/api/invites/${canary}/accept`).set(header, `https://board.example/invite/${canary}?source=email`); + expect(response.status).toBe(status); + const output = chunks.join(""); + expect(output).not.toContain(canary); + const record = JSON.parse(output.trim()); + expect(record.req.method).toBe("POST"); + expect(record.req.url).toBe("/api/invites/[REDACTED]/accept"); + expect(record.req.headers[header.toLowerCase()]).toBe("https://board.example/invite/[REDACTED]"); + expect(record.res.statusCode).toBe(status); + expect(record.msg).toContain(`POST /invites/[REDACTED]/accept ${status}`); + }); +}); diff --git a/server/src/__tests__/redact-sensitive.test.ts b/server/src/__tests__/redact-sensitive.test.ts index 633d4adb9bd..6ba052041af 100644 --- a/server/src/__tests__/redact-sensitive.test.ts +++ b/server/src/__tests__/redact-sensitive.test.ts @@ -271,3 +271,19 @@ it("redacts Grok routine URL and key in setup failure logs", () => { expect(collectSensitiveStringValues(payload)).toContain(payload.webhookUrl); expect(collectSensitiveStringValues(payload)).toContain(payload.webhookKey); }); + +describe("invitation bearer URL paths", () => { + it.each([ + ["/api/invites/invite-bearer-canary/accept?token=query-canary", "/api/invites/[REDACTED]/accept"], + ["/invites/invite-bearer-canary/onboarding.txt", "/invites/[REDACTED]/onboarding.txt"], + ["/invite/invite-bearer-canary", "/invite/[REDACTED]"], + ["/API/INVITES/encoded%2Fbearer/skills/paperclip", "/API/INVITES/[REDACTED]/skills/paperclip"], + ["https://user:password@board.example/api/invites/invite-bearer-canary/accept#fragment", "https://board.example/api/invites/[REDACTED]/accept"], + ["/api/invites/invite-record-id/revoke", "/api/invites/invite-record-id/revoke"], + ["/invites/invite-record-id/revoke/", "/invites/invite-record-id/revoke/"], + ["/invite/invite-bearer-canary/revoke", "/invite/[REDACTED]/revoke"], + ["/api/companies/company/invites/invite-record-id", "/api/companies/company/invites/invite-record-id"], + ])("sanitizes %s without hiding the route", (input, expected) => { + expect(stripSecretBearingUrlParts(input)).toBe(expected); + }); +}); diff --git a/server/src/middleware/logger.ts b/server/src/middleware/logger.ts index dbbc8f2202b..dcf545bb707 100644 --- a/server/src/middleware/logger.ts +++ b/server/src/middleware/logger.ts @@ -119,8 +119,14 @@ export function createHttpLogger(baseLogger: Logger) { url: privateWebhookLogUrl(req.url), }; } + const headers = req.headers as Record | undefined; return { ...req, + ...(headers ? { headers: { + ...headers, + ...Object.fromEntries(["referer", "referrer"].filter(key => typeof headers[key] === "string") + .map(key => [key, stripSecretBearingUrlParts(headers[key] as string)])), + } } : {}), url: typeof req.url === "string" ? stripSecretBearingUrlParts(req.url) diff --git a/server/src/middleware/redact-sensitive.ts b/server/src/middleware/redact-sensitive.ts index 9401287ba27..72f48c2ca1c 100644 --- a/server/src/middleware/redact-sensitive.ts +++ b/server/src/middleware/redact-sensitive.ts @@ -138,11 +138,14 @@ function isUrlishKey(key: string): boolean { export function stripSecretBearingUrlParts(value: string): string { const suffixStart = value.search(/[?#]/); const withoutQueryOrFragment = - suffixStart === -1 ? value : value.slice(0, suffixStart); + (suffixStart === -1 ? value : value.slice(0, suffixStart)).replace( + /(^|^[a-z][a-z\d+.-]*:\/\/[^/]*)(\/(?:(?:api\/)?invites\/(?![^/]+\/revoke(?:\/|$))|invite\/))[^/]+/i, + "$1$2[REDACTED]", + ); try { const url = new URL(withoutQueryOrFragment); - if (!url.username && !url.password && suffixStart === -1) return value; + if (!url.username && !url.password) return withoutQueryOrFragment; url.username = ""; url.password = ""; return url.toString();