mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 07:12:00 +01:00
## 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/<token>/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/<token>/...`, `/invites/<token>/...`, and `/invite/<token>` 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/<inviteId>/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/<id>/...` 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 <noreply@paperclip.ing>
255 lines
10 KiB
TypeScript
255 lines
10 KiB
TypeScript
import pino from "pino";
|
|
import type { IncomingMessage, ServerResponse } from "node:http";
|
|
import type { Logger } from "pino";
|
|
import { pinoHttp } from "pino-http";
|
|
import { HTTP_LOG_REDACT_PATHS } from "./http-log-redaction.js";
|
|
import {
|
|
isPrivateWebhookHttpRequest,
|
|
isSecretSensitiveHttpRequest,
|
|
isPrivateAgentCommentaryHttpRequest,
|
|
isPrivateAgentObservabilityHttpRequest,
|
|
isPrivateMuseHttpRequest,
|
|
isPrivateDotPrincipalHttpRequest,
|
|
shouldSilenceHttpSuccessLog,
|
|
} from "./http-log-policy.js";
|
|
import {
|
|
redactSensitive,
|
|
stripSecretBearingUrlParts,
|
|
} from "./redact-sensitive.js";
|
|
|
|
const sharedOpts = {
|
|
translateTime: "SYS:HH:MM:ss",
|
|
ignore: "pid,hostname",
|
|
singleLine: true,
|
|
};
|
|
|
|
const isProduction = process.env.NODE_ENV === "production";
|
|
export const logger = isProduction
|
|
? pino({
|
|
level: process.env.PAPERCLIP_LOG_LEVEL?.trim() || "info",
|
|
redact: [...HTTP_LOG_REDACT_PATHS],
|
|
})
|
|
: pino(
|
|
{
|
|
level: process.env.PAPERCLIP_LOG_LEVEL?.trim() || "debug",
|
|
redact: [...HTTP_LOG_REDACT_PATHS],
|
|
},
|
|
pino.transport({
|
|
target: "pino-pretty",
|
|
options: {
|
|
...sharedOpts,
|
|
ignore: "pid,hostname,req,res,responseTime",
|
|
colorize: true,
|
|
destination: 1,
|
|
},
|
|
}),
|
|
);
|
|
|
|
function requestClassificationUrl(req: {
|
|
originalUrl?: unknown;
|
|
url?: unknown;
|
|
}): string | undefined {
|
|
return typeof req.originalUrl === "string"
|
|
? req.originalUrl
|
|
: typeof req.url === "string"
|
|
? req.url
|
|
: undefined;
|
|
}
|
|
|
|
function isPrivateWebhook(req: {
|
|
method?: string;
|
|
originalUrl?: unknown;
|
|
url?: unknown;
|
|
}) {
|
|
return isPrivateWebhookHttpRequest(
|
|
req.method,
|
|
requestClassificationUrl(req),
|
|
);
|
|
}
|
|
|
|
function privateWebhookLogUrl(url: unknown) {
|
|
return typeof url === "string" && /\/routine-triggers\/public(?:\/|$)/i.test(url)
|
|
? "/api/routine-triggers/public/:publicId/fire"
|
|
: "/api/chat-webhooks/:publicId/:provider";
|
|
}
|
|
|
|
function requestLogUrl(req: {
|
|
method?: string;
|
|
originalUrl?: unknown;
|
|
url?: unknown;
|
|
}) {
|
|
if(isPrivateDotPrincipalHttpRequest(requestClassificationUrl(req)))return "/mcp/runner/oauth/cloud-principal";
|
|
if(isPrivateMuseHttpRequest(requestClassificationUrl(req)))return "/api/muse/:protocol";
|
|
if (isPrivateAgentObservabilityHttpRequest(requestClassificationUrl(req))) return "/api/companies/:companyId/observability";
|
|
return isPrivateWebhook(req)
|
|
? privateWebhookLogUrl(requestClassificationUrl(req))
|
|
: stripSecretBearingUrlParts(typeof req.url === "string" ? req.url : "");
|
|
}
|
|
|
|
function abortedRequestMessage(req: IncomingMessage, res: ServerResponse) {
|
|
if (!req.readableAborted && res.writableFinished) return null;
|
|
return `${req.method} ${requestLogUrl(req)} — request aborted${res.headersSent ? " during response" : " before response"}`;
|
|
}
|
|
|
|
export function createHttpLogger(baseLogger: Logger) {
|
|
return pinoHttp({
|
|
logger: baseLogger,
|
|
serializers: {
|
|
req(req: Record<string, unknown> & { url?: unknown }) {
|
|
if (isPrivateAgentObservabilityHttpRequest(typeof req.url === "string" ? req.url : undefined)) {
|
|
return { id: req.id, method: req.method, url: "/api/companies/:companyId/observability" };
|
|
}
|
|
if(isPrivateDotPrincipalHttpRequest(typeof req.url === "string" ? req.url : undefined))return {id:req.id,method:req.method,url:"/mcp/runner/oauth/cloud-principal"};
|
|
if(isPrivateMuseHttpRequest(typeof req.url === "string" ? req.url : undefined))return {id:req.id,method:req.method,url:"/api/muse/:protocol"};
|
|
if (isPrivateAgentCommentaryHttpRequest(typeof req.url === "string" ? req.url : undefined)) {
|
|
return { id: req.id, method: req.method, url: stripSecretBearingUrlParts(req.url as string) };
|
|
}
|
|
if (
|
|
isPrivateWebhook({
|
|
method: typeof req.method === "string" ? req.method : undefined,
|
|
url: req.url,
|
|
})
|
|
) {
|
|
// pino's standard request serializer has already selected originalUrl.
|
|
// A closed projection also excludes params, arbitrary headers and any
|
|
// parser/SDK-added body copies, including Buffer numeric byte keys.
|
|
return {
|
|
id: req.id,
|
|
method: req.method,
|
|
url: privateWebhookLogUrl(req.url),
|
|
};
|
|
}
|
|
const headers = req.headers as Record<string, unknown> | 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)
|
|
: req.url,
|
|
// The URL policy intentionally drops all query parameters. The default
|
|
// serializer also exposes the parsed query separately, so omit that
|
|
// duplicate path instead of letting credentials bypass the URL scrub.
|
|
query: undefined,
|
|
};
|
|
},
|
|
res(
|
|
res: Record<string, unknown> & {
|
|
raw?: {
|
|
req?: { method?: string; originalUrl?: unknown; url?: unknown };
|
|
};
|
|
},
|
|
) {
|
|
// A provider error may also be reflected in response headers. Keep the
|
|
// same content-free contract on both sides of a webhook request.
|
|
return res.raw?.req && (isPrivateWebhook(res.raw.req) || isPrivateMuseHttpRequest(requestClassificationUrl(res.raw.req)) || isPrivateDotPrincipalHttpRequest(requestClassificationUrl(res.raw.req)) || isPrivateAgentCommentaryHttpRequest(requestClassificationUrl(res.raw.req)) || isPrivateAgentObservabilityHttpRequest(requestClassificationUrl(res.raw.req)))
|
|
? { statusCode: res.statusCode }
|
|
: res;
|
|
},
|
|
},
|
|
customLogLevel(_req, res, err) {
|
|
if (err || (res as ServerResponse & { err?: unknown }).err || res.statusCode >= 500) return "error";
|
|
if (_req.readableAborted || !res.writableFinished) return "warn";
|
|
if (shouldSilenceHttpSuccessLog(_req.method, _req.url, res.statusCode)) {
|
|
return "silent";
|
|
}
|
|
if (res.statusCode >= 400) return "warn";
|
|
return "info";
|
|
},
|
|
customSuccessMessage(req, res) {
|
|
return abortedRequestMessage(req, res) ?? `${req.method} ${requestLogUrl(req)} ${res.statusCode}`;
|
|
},
|
|
customErrorMessage(req, res, err) {
|
|
const abortedMessage = abortedRequestMessage(req, res);
|
|
if (abortedMessage) return abortedMessage;
|
|
if (
|
|
isSecretSensitiveHttpRequest(req.method, requestClassificationUrl(req))
|
|
) {
|
|
return `${req.method} ${requestLogUrl(req)} ${res.statusCode} — request failed`;
|
|
}
|
|
const ctx = (res as any).__errorContext;
|
|
const errMsg =
|
|
ctx?.error?.message ||
|
|
err?.message ||
|
|
(res as any).err?.message ||
|
|
"unknown error";
|
|
return `${req.method} ${stripSecretBearingUrlParts(req.url ?? "")} ${res.statusCode} — ${errMsg}`;
|
|
},
|
|
customErrorObject(req, _res, _err, value) {
|
|
if (isPrivateAgentObservabilityHttpRequest(requestClassificationUrl(req))) {
|
|
return { ...value, err: { type: "Error", message: "Agent observability request failed" } };
|
|
}
|
|
if(isPrivateDotPrincipalHttpRequest(requestClassificationUrl(req)))return {...value,err:{type:"Error",message:"Dot principal request failed"}};
|
|
if (isPrivateMuseHttpRequest(requestClassificationUrl(req)))return {...value,err:{type:"Error",message:"External agent request failed"}};
|
|
if (isPrivateAgentCommentaryHttpRequest(requestClassificationUrl(req))) {
|
|
return { ...value, err: { type: "Error", message: "Feedback request failed" } };
|
|
}
|
|
// pino-http serializes res.err independently of customProps/errorContext.
|
|
// Do not rely on a particular error handler having sanitized an SDK Error.
|
|
return isPrivateWebhook(req)
|
|
? {
|
|
...value,
|
|
err: { type: "Error", message: "Chat webhook request failed" },
|
|
}
|
|
: value;
|
|
},
|
|
customProps(req, res) {
|
|
if (res.statusCode >= 400) {
|
|
const ctx = (res as any).__errorContext;
|
|
if (/^\/mcp\/(?:oauth|paperclip)(?:\/|$)/.test(requestClassificationUrl(req) ?? "")) {
|
|
return { reqBody: "[REDACTED]", ...(ctx ? { errorContext: { name: "Error" } } : {}) };
|
|
}
|
|
if (isPrivateWebhook(req) || isPrivateMuseHttpRequest(requestClassificationUrl(req)) || isPrivateDotPrincipalHttpRequest(requestClassificationUrl(req)) || isPrivateAgentCommentaryHttpRequest(requestClassificationUrl(req)) || isPrivateAgentObservabilityHttpRequest(requestClassificationUrl(req))) {
|
|
// Omit, rather than recursively redact, the entire provider payload.
|
|
// This applies equally before/after parsing and with/without context.
|
|
return {
|
|
reqBody: "[REDACTED]",
|
|
...(ctx || (res as any).err
|
|
? { errorContext: { name: "Error" } }
|
|
: {}),
|
|
};
|
|
}
|
|
if (ctx) {
|
|
const secretSensitiveRoute = isSecretSensitiveHttpRequest(
|
|
req.method,
|
|
requestClassificationUrl(req),
|
|
);
|
|
return {
|
|
// Provider SDK and validation errors sometimes echo the supplied
|
|
// credential in their prose. Keep only a non-sensitive type marker
|
|
// for setup routes; the status, route, and redacted body remain.
|
|
errorContext: secretSensitiveRoute
|
|
? { name: "Error" }
|
|
: redactSensitive(ctx.error),
|
|
reqBody: redactSensitive(ctx.reqBody),
|
|
reqParams: redactSensitive(ctx.reqParams),
|
|
};
|
|
}
|
|
const props: Record<string, unknown> = {};
|
|
const { body, params } = req as any;
|
|
if (body && typeof body === "object" && Object.keys(body).length > 0) {
|
|
props.reqBody = redactSensitive(body);
|
|
}
|
|
if (
|
|
params &&
|
|
typeof params === "object" &&
|
|
Object.keys(params).length > 0
|
|
) {
|
|
props.reqParams = redactSensitive(params);
|
|
}
|
|
if ((req as any).route?.path) {
|
|
props.routePath = (req as any).route.path;
|
|
}
|
|
return props;
|
|
}
|
|
return {};
|
|
},
|
|
});
|
|
}
|
|
|
|
export const httpLogger = createHttpLogger(logger);
|