diff --git a/packages/core/sdk/src/health-check.ts b/packages/core/sdk/src/health-check.ts index d553f994f..4648f24d5 100644 --- a/packages/core/sdk/src/health-check.ts +++ b/packages/core/sdk/src/health-check.ts @@ -379,11 +379,40 @@ export const projectResponseFields = ( return fields; }; +/** Placeholder shown instead of a leaf whose key names it as secret-bearing. */ +export const REDACTED_SAMPLE_VALUE = "[redacted]"; + +/** + * Leaf keys whose value is a credential rather than something worth previewing. + * + * The sample exists so a user can pick their identity field, and the keys that + * serve that (`email`, `login`, `username`, `name`, `id`) do not collide with + * any of these — so this can afford to be blunt. + * + * This catches the secrets we do NOT already know. Scrubbing the connection's + * own credential value out of the sample only helps when the body echoes the + * key we authenticated with; a health check pointed at a key-listing endpoint + * returns different secrets entirely, and no scrub of a known value can see + * those. + */ +const SECRET_KEY_PATTERN = + /(^|[^a-z])(secret|token|password|passwd|apikey|api_key|credential|authorization|auth|session|cookie|private|signature|bearer|refresh)([^a-z]|$)/i; + +/** True when the last segment of a dotted path names a credential. */ +const namesASecret = (path: string): boolean => { + const leaf = path.slice(path.lastIndexOf(".") + 1); + return SECRET_KEY_PATTERN.test(leaf); +}; + /** * Walk an actual JSON response body and return its scalar leaves as * `{ path, value }` rows (value stringified + truncated). Drives the live * preview's "show me what this returns" list. Bounded to depth 4, 25 fields, * and ~120-char values. + * + * Leaves whose key names a credential are kept but their value is replaced, + * so the preview still shows the field exists without persisting its value — + * this result is written to `connection.last_health`. */ export const extractResponseFields = (data: unknown): HealthCheckResponseSample[] => { const out: HealthCheckResponseSample[] = []; @@ -418,7 +447,10 @@ export const extractResponseFields = (data: unknown): HealthCheckResponseSample[ path !== "" && (typeof node === "string" || typeof node === "number" || typeof node === "boolean") ) { - out.push({ path, value: render(String(node)) }); + out.push({ + path, + value: namesASecret(path) ? REDACTED_SAMPLE_VALUE : render(String(node)), + }); } }; diff --git a/packages/core/sdk/src/health-response-sample-redaction.test.ts b/packages/core/sdk/src/health-response-sample-redaction.test.ts new file mode 100644 index 000000000..674b954ce --- /dev/null +++ b/packages/core/sdk/src/health-response-sample-redaction.test.ts @@ -0,0 +1,82 @@ +// A health check writes its response sample into `connection.last_health`, so +// whatever the sample carries is persisted. The operation being probed is +// user-chosen from the plugin's catalog, which means it can be a key-listing +// endpoint just as easily as a `/me`. +// +// These use real response bodies of the shape those endpoints return, because +// the property under test is what survives the walk into the database. + +import { describe, expect, it } from "@effect/vitest"; + +import { extractResponseFields, REDACTED_SAMPLE_VALUE } from "./health-check"; + +/** The sample as a path -> value lookup, which is how the assertions read. */ +const byPath = (data: unknown): Record => + Object.fromEntries(extractResponseFields(data).map((f) => [f.path, f.value])); + +describe("health-check response sample redaction", () => { + it("redacts credential-named leaves while keeping the identity fields", () => { + const fields = byPath({ + email: "alex@example.com", + login: "alex", + id: 4711, + api_key: "sk-live-must-not-be-persisted", + refresh_token: "rt-must-not-be-persisted", + session: "sess-must-not-be-persisted", + }); + + // The reason the sample exists still works. + expect(fields.email).toBe("alex@example.com"); + expect(fields.login).toBe("alex"); + expect(fields.id).toBe("4711"); + + expect(fields.api_key).toBe(REDACTED_SAMPLE_VALUE); + expect(fields.refresh_token).toBe(REDACTED_SAMPLE_VALUE); + expect(fields.session).toBe(REDACTED_SAMPLE_VALUE); + }); + + it("redacts nested and array-borne credentials, not just top-level ones", () => { + // What a key-listing endpoint actually returns. This is the case a scrub + // of the connection's own value cannot catch: these are different secrets. + const fields = byPath({ + keys: [ + { name: "prod", token: "sk-prod-must-not-be-persisted" }, + { name: "staging", token: "sk-staging-must-not-be-persisted" }, + ], + account: { billing: { secret: "whsec-must-not-be-persisted" } }, + }); + + expect(fields["keys.0.name"]).toBe("prod"); + expect(fields["keys.0.token"]).toBe(REDACTED_SAMPLE_VALUE); + expect(fields["keys.1.token"]).toBe(REDACTED_SAMPLE_VALUE); + expect(fields["account.billing.secret"]).toBe(REDACTED_SAMPLE_VALUE); + }); + + it("keeps the field visible so the preview still shows the shape", () => { + // Dropping the row would change what the picker displays. Redacting the + // value keeps the response shape legible without persisting the secret. + const sample = extractResponseFields({ api_key: "sk-live-x" }); + + expect(sample).toHaveLength(1); + expect(sample[0]?.path).toBe("api_key"); + }); + + it("does not redact identity keys that merely contain a matching substring", () => { + // `author` contains "auth". Matching it would silently blank a normal + // field, which is how an over-eager redactor makes the feature useless. + const fields = byPath({ author: "alex", authorization: "Bearer x" }); + + expect(fields.author).toBe("alex"); + expect(fields.authorization).toBe(REDACTED_SAMPLE_VALUE); + }); + + it("POSITIVE CONTROL: an unredacted body does come through verbatim", () => { + // Proves these assertions can fail. Without it, an extractor that returned + // nothing, or redacted everything, would satisfy the checks above. + const fields = byPath({ email: "alex@example.com", plan: "pro" }); + + expect(fields.email).toBe("alex@example.com"); + expect(fields.plan).toBe("pro"); + expect(Object.values(fields)).not.toContain(REDACTED_SAMPLE_VALUE); + }); +}); diff --git a/packages/plugins/openapi/src/sdk/backing.ts b/packages/plugins/openapi/src/sdk/backing.ts index 4e6e5ba95..f5b444ae5 100644 --- a/packages/plugins/openapi/src/sdk/backing.ts +++ b/packages/plugins/openapi/src/sdk/backing.ts @@ -978,7 +978,17 @@ export const checkHealthOpenApi = (input: { // pick an identity field, and error bodies (upstream internals, auth error // envelopes) have no business in the preview. Non-healthy runs carry the // classified `detail` instead. - const responseSample = status === "healthy" ? extractResponseFields(probe.result.data) : []; + // Same scrub the `detail` branch below uses, for the same reason: a body + // can echo back the key it was authenticated with. `extractResponseFields` + // already redacts leaves whose KEY names a credential; this covers the + // other direction, a credential value under an innocent-looking key. + const responseSample = + status === "healthy" + ? extractResponseFields(probe.result.data).map((field) => ({ + ...field, + value: scrubSecrets(field.value), + })) + : []; return { status, httpStatus: probe.result.status,