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>
290 lines
11 KiB
TypeScript
290 lines
11 KiB
TypeScript
import { describe, expect, it } from "vitest";
|
|
import {
|
|
collectSensitiveStringValues,
|
|
redactSensitive,
|
|
redactSensitiveValueOccurrences,
|
|
stripSecretBearingUrlParts,
|
|
} from "../middleware/redact-sensitive.js";
|
|
|
|
it("redacts Slack manager grants and sealed OAuth handoffs", () => {
|
|
const result = redactSensitive({ managerToken: "manager-canary", refreshToken: "refresh-canary", sealed: { ct: "sealed-canary" }, handoff: { token: "handoff-canary" }, claim_id: "claim-canary" });
|
|
expect(JSON.stringify(result)).not.toContain("canary");
|
|
});
|
|
|
|
describe("redactSensitive", () => {
|
|
it("redacts a plaintext password field on a sign-in body", () => {
|
|
const body = {
|
|
email: "user@example.com",
|
|
password: "founding6gomez6croaking",
|
|
};
|
|
|
|
const out = redactSensitive(body) as Record<string, unknown>;
|
|
|
|
expect(out.email).toBe("user@example.com");
|
|
expect(out.password).toBe("[REDACTED]");
|
|
expect((body as Record<string, unknown>).password).toBe(
|
|
"founding6gomez6croaking",
|
|
);
|
|
});
|
|
|
|
it("redacts password key regardless of casing", () => {
|
|
expect(
|
|
(redactSensitive({ Password: "x" }) as Record<string, unknown>).Password,
|
|
).toBe("[REDACTED]");
|
|
expect(
|
|
(redactSensitive({ PASSWORD: "x" }) as Record<string, unknown>).PASSWORD,
|
|
).toBe("[REDACTED]");
|
|
});
|
|
|
|
it("redacts known credential-shaped keys", () => {
|
|
const out = redactSensitive({
|
|
currentPassword: "a",
|
|
newPassword: "b",
|
|
access_token: "c",
|
|
refresh_token: "d",
|
|
api_key: "e",
|
|
authorization: "Bearer f",
|
|
}) as Record<string, string>;
|
|
|
|
for (const value of Object.values(out)) {
|
|
expect(value).toBe("[REDACTED]");
|
|
}
|
|
});
|
|
|
|
it("drops provider credential envelopes and redacts provider keys outside them", () => {
|
|
const out = redactSensitive({
|
|
credentials: {
|
|
botToken: "nested-bot-canary",
|
|
futureCredential: "nested-future-canary",
|
|
},
|
|
botToken: "bot-canary",
|
|
signing_secret: "signing-canary",
|
|
webhookSecret: "webhook-canary",
|
|
app_secret: "app-canary",
|
|
applicationSecret: "application-canary",
|
|
}) as Record<string, unknown>;
|
|
|
|
expect(out).toEqual({
|
|
credentials: "[REDACTED]",
|
|
botToken: "[REDACTED]",
|
|
signing_secret: "[REDACTED]",
|
|
webhookSecret: "[REDACTED]",
|
|
app_secret: "[REDACTED]",
|
|
applicationSecret: "[REDACTED]",
|
|
});
|
|
expect(JSON.stringify(out)).not.toContain("canary");
|
|
});
|
|
|
|
it("removes raw, JSON-escaped, and URL-encoded submitted credentials from prose", () => {
|
|
const credential = "secret value/with\nnewline";
|
|
const submittedValues = collectSensitiveStringValues({
|
|
action: "configure",
|
|
credentials: { botToken: credential },
|
|
});
|
|
const out = redactSensitiveValueOccurrences(
|
|
{
|
|
raw: `Provider rejected ${credential}`,
|
|
json: `Provider rejected ${JSON.stringify(credential).slice(1, -1)}`,
|
|
url: `Provider rejected ${encodeURIComponent(credential)}`,
|
|
form: `Provider rejected ${encodeURIComponent(credential).replaceAll("%20", "+")}`,
|
|
scopes: ["chat:write", "reactions:write"],
|
|
},
|
|
submittedValues,
|
|
);
|
|
|
|
expect(JSON.stringify(out)).not.toContain("secret value");
|
|
expect(JSON.stringify(out)).not.toContain("secret%20value");
|
|
expect(out).toMatchObject({ scopes: ["chat:write", "reactions:write"] });
|
|
});
|
|
|
|
it("sanitizes malformed UTF-16 credential values without throwing", () => {
|
|
const malformedCredential = "\ud800";
|
|
|
|
expect(() =>
|
|
redactSensitiveValueOccurrences(
|
|
`Provider rejected ${malformedCredential}`,
|
|
[malformedCredential],
|
|
),
|
|
).not.toThrow();
|
|
expect(
|
|
redactSensitiveValueOccurrences(
|
|
`Provider rejected ${malformedCredential}`,
|
|
[malformedCredential],
|
|
),
|
|
).toBe("Provider rejected [REDACTED]");
|
|
});
|
|
|
|
it("removes a normalized provider echo of a whitespace-padded credential", () => {
|
|
expect(
|
|
redactSensitiveValueOccurrences("Provider rejected padded-token-canary", [
|
|
" padded-token-canary \n",
|
|
]),
|
|
).toBe("Provider rejected [REDACTED]");
|
|
});
|
|
|
|
it("redacts an OAuth provider's error_description and error_uri from a callback query", () => {
|
|
const out = redactSensitive({
|
|
state: "paperclip-state",
|
|
error: "access_denied",
|
|
error_description:
|
|
"\u001b[31mPaste your recovery key\u001b[0m sk-live-canary",
|
|
error_uri: "https://attacker.example/explain?leak=sk-live-canary",
|
|
}) as Record<string, unknown>;
|
|
|
|
// The `error` code is Paperclip's one allowlisted label, so it stays legible
|
|
// in logs; the provider's prose does not.
|
|
expect(out.error).toBe("access_denied");
|
|
expect(out.state).toBe("paperclip-state");
|
|
expect(out.error_description).toBe("[REDACTED]");
|
|
expect(out.error_uri).toBe("[REDACTED]");
|
|
expect(JSON.stringify(out)).not.toContain("sk-live-canary");
|
|
expect(JSON.stringify(out)).not.toContain("\\u001b");
|
|
});
|
|
|
|
it("redacts bare value and token fields recursively", () => {
|
|
const out = redactSensitive({
|
|
token: "secret-token",
|
|
nested: { value: "secret-value" },
|
|
entries: [{ value: "array-secret" }],
|
|
limit: 20,
|
|
}) as Record<string, unknown>;
|
|
|
|
expect(out.token).toBe("[REDACTED]");
|
|
expect((out.nested as Record<string, unknown>).value).toBe("[REDACTED]");
|
|
expect((out.entries as Array<Record<string, unknown>>)[0].value).toBe(
|
|
"[REDACTED]",
|
|
);
|
|
expect(out.limit).toBe(20);
|
|
expect(JSON.stringify(out)).not.toMatch(
|
|
/secret-token|secret-value|array-secret/,
|
|
);
|
|
});
|
|
|
|
it("strips secret-bearing query and fragment values from source URLs", () => {
|
|
const out = redactSensitive({
|
|
source: "https://github.com/acme/private-skill?token=secret#token=secret",
|
|
}) as Record<string, unknown>;
|
|
|
|
expect(out.source).toBe("https://github.com/acme/private-skill");
|
|
});
|
|
|
|
it("recurses into nested objects and arrays", () => {
|
|
const out = redactSensitive({
|
|
user: { email: "user@example.com", password: "secret-pass" },
|
|
tokens: [{ access_token: "t1" }, { access_token: "t2" }],
|
|
}) as Record<string, unknown>;
|
|
|
|
expect((out.user as Record<string, unknown>).email).toBe(
|
|
"user@example.com",
|
|
);
|
|
expect((out.user as Record<string, unknown>).password).toBe("[REDACTED]");
|
|
const tokens = out.tokens as Array<Record<string, unknown>>;
|
|
expect(tokens[0].access_token).toBe("[REDACTED]");
|
|
expect(tokens[1].access_token).toBe("[REDACTED]");
|
|
});
|
|
|
|
it("leaves primitives and non-sensitive keys untouched", () => {
|
|
const body = {
|
|
email: "a@b.c",
|
|
name: "Alice",
|
|
count: 7,
|
|
active: true,
|
|
missing: null,
|
|
};
|
|
|
|
expect(redactSensitive(body)).toEqual(body);
|
|
});
|
|
|
|
it("returns primitives unchanged", () => {
|
|
expect(redactSensitive("hello")).toBe("hello");
|
|
expect(redactSensitive(42)).toBe(42);
|
|
expect(redactSensitive(null)).toBe(null);
|
|
expect(redactSensitive(undefined)).toBe(undefined);
|
|
});
|
|
|
|
it("caps recursion depth so cycles do not pin the logger", () => {
|
|
const cycle: Record<string, unknown> = { name: "root" };
|
|
cycle.self = cycle;
|
|
|
|
expect(() => redactSensitive(cycle)).not.toThrow();
|
|
});
|
|
|
|
it("omits deeply-nested arrays at the depth cap instead of leaking null entries to JSON", () => {
|
|
// Build an object whose array field is reached at MAX_DEPTH. Recursing
|
|
// into the array elements would exceed the cap; without the array-level
|
|
// guard, `value.map` would produce `[undefined, ...]` which JSON.stringify
|
|
// renders as `[null, ...]`. Object properties at the same cap are
|
|
// already absent from the JSON output (JSON.stringify skips undefined
|
|
// values on objects), so this test pins the array path to the same
|
|
// contract: silently absent, not visible as nulls.
|
|
let payload: Record<string, unknown> = { values: [1, 2, 3] };
|
|
for (let i = 0; i < 5; i++) payload = { nested: payload };
|
|
|
|
const out = redactSensitive(payload);
|
|
|
|
const json = JSON.stringify(out);
|
|
expect(json).not.toContain("null");
|
|
expect(json).not.toContain("[1,2,3]");
|
|
});
|
|
});
|
|
|
|
describe("stripSecretBearingUrlParts", () => {
|
|
it("redacts the connector credentialValues envelope on an app-connect body", () => {
|
|
const out = redactSensitive({
|
|
galleryKey: "github",
|
|
name: "GitHub",
|
|
credentialValues: {
|
|
"credentials.authorization": "authorization-canary",
|
|
"headers.X-Api-Key": "header-canary",
|
|
},
|
|
}) as Record<string, unknown>;
|
|
|
|
// Diagnostics that make the failed connect debuggable must survive.
|
|
expect(out.galleryKey).toBe("github");
|
|
expect(out.name).toBe("GitHub");
|
|
expect(out.credentialValues).toBe("[REDACTED]");
|
|
expect(JSON.stringify(out)).not.toContain("canary");
|
|
});
|
|
|
|
it("redacts a credentialValues envelope nested below the top level", () => {
|
|
const out = redactSensitive({
|
|
connection: {
|
|
credentialValues: { "credentials.authorization": "nested-canary" },
|
|
},
|
|
});
|
|
|
|
expect(JSON.stringify(out)).not.toContain("nested-canary");
|
|
});
|
|
|
|
it("keeps a request path legible while dropping its complete query and fragment", () => {
|
|
expect(
|
|
stripSecretBearingUrlParts(
|
|
"/api/tools/oauth/callback?code=authorization-code&error_description=provider-prose#fragment",
|
|
),
|
|
).toBe("/api/tools/oauth/callback");
|
|
});
|
|
});
|
|
|
|
it("redacts Grok routine URL and key in setup failure logs", () => {
|
|
const payload = { webhookUrl: "https://bot.example/routine/url-canary", webhookKey: "key-canary" };
|
|
expect(JSON.stringify(redactSensitive(payload))).not.toContain("canary");
|
|
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);
|
|
});
|
|
});
|