mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-02 02:07:25 +08:00
fix(logging): redact cloud authentication headers (#14413)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The server records HTTP requests to help operators diagnose failures. > - Cloud requests carry tenant credentials and signed assertions in headers. > - The HTTP logger did not redact four of these headers. > - This pull request adds those headers to the existing redaction list. > - Operators retain the route, method, and response status without recording these values. ## Linked Issues or Issue Description **What happened?** HTTP request logs could contain cloud tenant credentials, session identifiers, runtime identity assertions, and cloud control assertions. **Expected behavior** The logger must redact these header values for successful requests and failed requests. **Steps to reproduce** 1. Create an Express app with the production HTTP logger and redaction configuration. 2. Send a request with the four cloud headers and distinct test values. 3. Inspect the serialized request headers for responses with status 200, 403, and 500. **Paperclip version or commit** Reproduced on `0f14d26123` before this fix. **Deployment mode** Server with cloud proxy authentication. The regression test uses an in-process Express server. ## What Changed - Redact `x-paperclip-cloud-tenant-token`, `x-paperclip-cloud-session-id`, `x-paperclip-cloud-runtime-identity`, and `x-paperclip-cloud-control` in HTTP request logs. - Test real logger output for HTTP 200, 403, and 500 with mixed-case request header names. Route 403 and 500 through the production error handler. Check response bodies, log levels, and server error context. - Check that the method, route, and status remain available. ## Verification The `server/src/__tests__/http-log-redaction.test.ts` suite passed, including all three new cloud-header cases. - Rebased onto master at `14795136f5`. - `pnpm exec vitest run server/src/__tests__/http-log-redaction.test.ts` passed: 59 tests, including all three new response-status cases. The suite and server typecheck also passed after the error-handler coverage update. - `pnpm build` and `pnpm -r typecheck` passed. The full local `pnpm test:run` was attempted and stopped after the failures listed below. Greptile is 5/5 with zero unresolved review threads on the latest head. All checks for head `373d29e2f1` passed (53 successful, two intentional skips). - Full local validation did not pass. The attempt reproduced company-skill cache permission failures, the terminal-workspace cleanup assertion, a heartbeat feedback timeout, and one process-conversation timing failure. It was stopped during the general-server stage after these failures. Remaining general-server tests, other workspace groups, and serialized-server stages did not complete locally. Earlier clean-master checks reproduced the cache failures and isolated cleanup retries passed. The latest-head GitHub suite passed all of these groups. - No browser suites ran locally. This change does not affect browser behavior. ## Risks - These four values will no longer be available in HTTP logs. Route, method, and status remain available. - This change applies to new log entries. It does not remove old entries or rotate credentials. - No schema, API, or authentication behavior changes. ## Model Used OpenAI Codex, GPT-6, with repository inspection, code editing, and test execution. The exact deployment model ID and context window are not exposed in this 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 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 (focused suites; full local-run failures and incomplete stages are documented above) - [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 - [x] All Paperclip CI gates are green - [x] 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: Paperclip <noreply@paperclip.ing>
This commit is contained in:
@@ -428,6 +428,58 @@ describe("HTTP logger redaction", () => {
|
||||
expect(log.res.statusCode).toBe(status);
|
||||
});
|
||||
|
||||
it.each([200, 403, 500])("redacts cloud credentials and assertions from HTTP %i logs", async (status) => {
|
||||
const headers = {
|
||||
"X-Paperclip-Cloud-Tenant-Token": "cloud-tenant-token-canary",
|
||||
"X-Paperclip-Cloud-Session-Id": "cloud-session-id-canary",
|
||||
"X-Paperclip-Cloud-Runtime-Identity": "cloud-runtime-identity-canary",
|
||||
"X-Paperclip-Cloud-Control": "cloud-control-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)));
|
||||
app.get("/api/companies", (_req, res, next) => {
|
||||
if (status === 403) {
|
||||
next(new HttpError(403, "Cloud tenant authentication required"));
|
||||
return;
|
||||
}
|
||||
if (status === 500) {
|
||||
next(new Error("Synthetic cloud request failure"));
|
||||
return;
|
||||
}
|
||||
res.status(status).json({ status });
|
||||
});
|
||||
app.use(errorHandler);
|
||||
|
||||
const response = await request(app).get("/api/companies").set(headers).expect(status);
|
||||
if (status === 403) {
|
||||
expect(response.body).toEqual({ error: "Cloud tenant authentication required" });
|
||||
} else if (status === 500) {
|
||||
expect(response.body).toEqual({ error: "Internal server error" });
|
||||
}
|
||||
|
||||
const output = chunks.join("");
|
||||
const log = JSON.parse(output.trim());
|
||||
for (const [header, secret] of Object.entries(headers)) {
|
||||
expect(output).not.toContain(secret);
|
||||
expect(log.req.headers[header.toLowerCase()]).toBe("[Redacted]");
|
||||
}
|
||||
expect(log.req.method).toBe("GET");
|
||||
expect(log.req.url).toBe("/api/companies");
|
||||
expect(log.res.statusCode).toBe(status);
|
||||
expect(log.level).toBe(status === 500 ? 50 : status === 403 ? 40 : 30);
|
||||
if (status === 500) {
|
||||
expect(log.errorContext.message).toBe("Synthetic cloud request failure");
|
||||
expect(log.err.message).toBe("Synthetic cloud request failure");
|
||||
}
|
||||
});
|
||||
|
||||
it("drops OAuth callback query data from the message and structured request", async () => {
|
||||
const chunks: string[] = [];
|
||||
const stream = new Writable({
|
||||
|
||||
@@ -10,6 +10,11 @@ export const HTTP_LOG_REDACT_PATHS = [
|
||||
'req.headers["x-csrf-token"]',
|
||||
'req.headers["x-xsrf-token"]',
|
||||
'req.headers["x-api-key"]',
|
||||
// Cloud proxy credentials and signed assertions authorize tenant access.
|
||||
'req.headers["x-paperclip-cloud-tenant-token"]',
|
||||
'req.headers["x-paperclip-cloud-session-id"]',
|
||||
'req.headers["x-paperclip-cloud-runtime-identity"]',
|
||||
'req.headers["x-paperclip-cloud-control"]',
|
||||
// Runtime GitHub capabilities authorize credential acquisition for a live run.
|
||||
'req.headers["x-paperclip-github-capability"]',
|
||||
// Telegram's optional webhook verification header is a reusable bearer
|
||||
|
||||
Reference in New Issue
Block a user