mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-15 10:21:41 +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>
274 lines
8.8 KiB
TypeScript
274 lines
8.8 KiB
TypeScript
// Redaction for HTTP log payloads.
|
|
//
|
|
// `customProps` in logger.ts copies `req.body` / `req.params` / `req.query`
|
|
// verbatim into the 4xx/5xx log lines so operators can diagnose. That means
|
|
// Better Auth's `POST /api/auth/sign-in/email` body (which has the user's
|
|
// plaintext password) and similar payloads (sign-up, reset-password, API
|
|
// keys via Authorization header equivalents) end up on disk.
|
|
//
|
|
// This walker returns a shallow copy of the input with values for sensitive
|
|
// keys replaced with the literal string "[REDACTED]". Recurses into nested
|
|
// objects/arrays. Caps depth so a hostile or accidental cycle can't pin
|
|
// the logger.
|
|
|
|
const SENSITIVE_KEYS = new Set<string>([
|
|
// Provider setup payloads deliberately group all durable authentication
|
|
// material under `credentials`. Redact the whole subtree instead of trying
|
|
// to keep an ever-changing allowlist of provider-specific field names in
|
|
// sync with every connector.
|
|
"sealed",
|
|
"handoff",
|
|
"claim",
|
|
"claimid",
|
|
"claim_id",
|
|
"managertoken",
|
|
"manager_token",
|
|
"configurationtoken",
|
|
"configuration_token",
|
|
"credential",
|
|
"credentials",
|
|
// The app-connect/reconnect envelope. Its keys are caller-chosen provider
|
|
// config paths (`credentials.authorization`, `headers.X-Api-Key`), so they
|
|
// never match a leaf name in this set; redact the envelope by name. This is
|
|
// the durable half: the path list in http-log-redaction.ts only ever covers
|
|
// shapes someone has already seen leak.
|
|
"credentialvalues",
|
|
"password",
|
|
"currentpassword",
|
|
"newpassword",
|
|
"passwordconfirmation",
|
|
"password_confirmation",
|
|
"passwordconfirm",
|
|
"password_confirm",
|
|
"confirmpassword",
|
|
"confirm_password",
|
|
// Secret creation/update bodies use a generic `value` field. Failure logs
|
|
// must prefer losing that diagnostic value over persisting credential
|
|
// material. `token` is likewise ambiguous but frequently credential-bearing.
|
|
"value",
|
|
"token",
|
|
"secret",
|
|
"client_secret",
|
|
"clientsecret",
|
|
"access_token",
|
|
"accesstoken",
|
|
"refresh_token",
|
|
"refreshtoken",
|
|
"id_token",
|
|
"idtoken",
|
|
"api_key",
|
|
"apikey",
|
|
// Probe keys are transient, including rejected or future provider names.
|
|
// Redact the whole container instead of maintaining a second key allowlist.
|
|
"testcredentials",
|
|
"authorization",
|
|
"auth_token",
|
|
"authtoken",
|
|
"session_token",
|
|
"sessiontoken",
|
|
"private_key",
|
|
"privatekey",
|
|
// Defense in depth for legacy, malformed, or provider-specific payloads
|
|
// that place a credential outside the standard `credentials` envelope.
|
|
"app_secret",
|
|
"appsecret",
|
|
"application_secret",
|
|
"applicationsecret",
|
|
"bot_token",
|
|
"bottoken",
|
|
"secret_token",
|
|
"secrettoken",
|
|
"signing_secret",
|
|
"signingsecret",
|
|
"webhook_secret",
|
|
"webhooksecret",
|
|
"webhookkey",
|
|
"webhookurl",
|
|
"paperclip_capability",
|
|
// The Claude setup-token login fields. `browserCode` carries the one-time
|
|
// sign-in code and `authorization_code` carries the OAuth code; neither may
|
|
// reach a log line.
|
|
"browsercode",
|
|
"authorization_code",
|
|
"code_verifier",
|
|
"codeverifier",
|
|
"authorizationcode",
|
|
// The workspace login handoff ticket (PAP-17572). It is a signed bearer
|
|
// credential carried as a query parameter, so it must never reach a log line
|
|
// even though the exchange itself answers 302.
|
|
"ticket",
|
|
// Not secrets Paperclip holds, but attacker-authored prose: an OAuth provider
|
|
// controls `error_description` / `error_uri` on the callback query string, and
|
|
// `customProps` copies the whole query into 4xx log lines. Paperclip maps the
|
|
// `error` code to its own copy instead of reflecting these, so they have no
|
|
// debugging value here either (PAP-17108).
|
|
"error_description",
|
|
"errordescription",
|
|
"error_uri",
|
|
"erroruri",
|
|
]);
|
|
|
|
const MAX_DEPTH = 6;
|
|
const REDACTED = "[REDACTED]";
|
|
const URLISH_KEYS = new Set<string>([
|
|
"href",
|
|
"locator",
|
|
"source",
|
|
"source_locator",
|
|
"sourcelocator",
|
|
"source_url",
|
|
"sourceurl",
|
|
"uri",
|
|
"url",
|
|
// The Claude setup-token login URL. A structured `loginUrl` that reaches a log
|
|
// sink keeps its origin and path only; the OAuth query, fragment, and any
|
|
// credentials are stripped (SR-5 backstop).
|
|
"loginurl",
|
|
"login_url",
|
|
]);
|
|
|
|
function isSensitiveKey(key: string): boolean {
|
|
return SENSITIVE_KEYS.has(key.toLowerCase());
|
|
}
|
|
|
|
function isUrlishKey(key: string): boolean {
|
|
return URLISH_KEYS.has(key.toLowerCase());
|
|
}
|
|
|
|
export function stripSecretBearingUrlParts(value: string): string {
|
|
const suffixStart = value.search(/[?#]/);
|
|
const withoutQueryOrFragment =
|
|
(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) return withoutQueryOrFragment;
|
|
url.username = "";
|
|
url.password = "";
|
|
return url.toString();
|
|
} catch {
|
|
// Request URLs are normally origin-form paths rather than absolute URLs.
|
|
// They still need the same query/fragment policy as URL-valued payloads.
|
|
return withoutQueryOrFragment;
|
|
}
|
|
}
|
|
|
|
export function redactSensitive(value: unknown, depth = 0): unknown {
|
|
if (depth > MAX_DEPTH) return undefined;
|
|
if (value === null || typeof value !== "object") return value;
|
|
if (Array.isArray(value)) {
|
|
if (depth + 1 > MAX_DEPTH) return undefined;
|
|
return value.map((entry) => redactSensitive(entry, depth + 1));
|
|
}
|
|
const out: Record<string, unknown> = {};
|
|
for (const [key, entry] of Object.entries(value as Record<string, unknown>)) {
|
|
if (isSensitiveKey(key)) {
|
|
out[key] = REDACTED;
|
|
continue;
|
|
}
|
|
if (typeof entry === "string" && isUrlishKey(key)) {
|
|
out[key] = stripSecretBearingUrlParts(entry);
|
|
continue;
|
|
}
|
|
out[key] = redactSensitive(entry, depth + 1);
|
|
}
|
|
return out;
|
|
}
|
|
|
|
function collectSensitiveStringValuesInto(
|
|
value: unknown,
|
|
values: Set<string>,
|
|
collectAll: boolean,
|
|
depth: number,
|
|
): void {
|
|
if (depth > MAX_DEPTH || value === null || value === undefined) return;
|
|
if (typeof value === "string") {
|
|
if (collectAll && value.length > 0) values.add(value);
|
|
return;
|
|
}
|
|
if (typeof value !== "object") return;
|
|
if (Array.isArray(value)) {
|
|
for (const entry of value) {
|
|
collectSensitiveStringValuesInto(entry, values, collectAll, depth + 1);
|
|
}
|
|
return;
|
|
}
|
|
for (const [key, entry] of Object.entries(value as Record<string, unknown>)) {
|
|
collectSensitiveStringValuesInto(
|
|
entry,
|
|
values,
|
|
collectAll || isSensitiveKey(key),
|
|
depth + 1,
|
|
);
|
|
}
|
|
}
|
|
|
|
/** Collects submitted credential strings without exposing them to callers' logs. */
|
|
export function collectSensitiveStringValues(value: unknown): string[] {
|
|
const values = new Set<string>();
|
|
collectSensitiveStringValuesInto(value, values, false, 0);
|
|
return [...values];
|
|
}
|
|
|
|
function encodedSensitiveVariants(values: readonly string[]): string[] {
|
|
const variants = new Set<string>();
|
|
for (const value of values) {
|
|
const normalizedValues = new Set([value, value.trim()]);
|
|
for (const normalizedValue of normalizedValues) {
|
|
if (normalizedValue.length === 0) continue;
|
|
variants.add(normalizedValue);
|
|
const json = JSON.stringify(normalizedValue);
|
|
if (json.length >= 2) variants.add(json.slice(1, -1));
|
|
try {
|
|
const urlEncoded = encodeURIComponent(normalizedValue);
|
|
variants.add(urlEncoded);
|
|
variants.add(urlEncoded.replaceAll("%20", "+"));
|
|
} catch {
|
|
// A lone UTF-16 surrogate is not URI-encodable. The raw and JSON-escaped
|
|
// forms are still covered, and error handling must never throw again
|
|
// while trying to sanitize malformed input.
|
|
}
|
|
}
|
|
}
|
|
return [...variants]
|
|
.filter(Boolean)
|
|
.sort((left, right) => right.length - left.length);
|
|
}
|
|
|
|
/**
|
|
* Redacts sensitive-shaped fields, then removes submitted credential values
|
|
* wherever an upstream validator/provider echoed them into otherwise-safe
|
|
* response prose.
|
|
*/
|
|
export function redactSensitiveValueOccurrences(
|
|
value: unknown,
|
|
sensitiveValues: readonly string[],
|
|
): unknown {
|
|
const variants = encodedSensitiveVariants(sensitiveValues);
|
|
const redacted = redactSensitive(value);
|
|
|
|
const visit = (entry: unknown, depth: number): unknown => {
|
|
if (depth > MAX_DEPTH) return undefined;
|
|
if (typeof entry === "string") {
|
|
return variants.reduce(
|
|
(text, sensitiveValue) => text.replaceAll(sensitiveValue, REDACTED),
|
|
entry,
|
|
);
|
|
}
|
|
if (entry === null || typeof entry !== "object") return entry;
|
|
if (Array.isArray(entry))
|
|
return entry.map((item) => visit(item, depth + 1));
|
|
return Object.fromEntries(
|
|
Object.entries(entry as Record<string, unknown>).map(([key, item]) => [
|
|
key,
|
|
visit(item, depth + 1),
|
|
]),
|
|
);
|
|
};
|
|
|
|
return visit(redacted, 0);
|
|
}
|