Skip to content

Commit 108c54f

Browse files
carderneTrigger.dev RepoOps
authored andcommitted
fix(webapp,clickhouse): keep sensitive data out of server logs
Mono-RevId: ea91673acce3eb6a757274025b59b3f1f39e19ef
1 parent d93f848 commit 108c54f

24 files changed

Lines changed: 291 additions & 141 deletions

‎apps/webapp/app/models/user.server.ts‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -243,15 +243,10 @@ async function findOrCreateGoogleUser({
243243
// Check if email user and auth user are the same
244244
if (existingEmailUser.id !== existingUser.id) {
245245
// Different users: email is taken by one user, Google auth belongs to another
246-
logger.warn(
247-
`Google auth conflict: Google ID ${authenticationProfile.id} belongs to user ${existingUser.id} but email ${email} is taken by user ${existingEmailUser.id}`,
248-
{
249-
email,
250-
existingEmailUserId: existingEmailUser.id,
251-
existingAuthUserId: existingUser.id,
252-
authIdentifier,
253-
}
254-
);
246+
logger.warn("Google auth conflict between existing users", {
247+
existingEmailUserId: existingEmailUser.id,
248+
existingAuthUserId: existingUser.id,
249+
});
255250

256251
return {
257252
user: existingUser,

‎apps/webapp/app/routes/api.v1.query.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,14 +94,12 @@ const { action, loader } = createActionApiRoute(
9494
if (queryResult.error instanceof QueryError) {
9595
logger.warn("Query API error", {
9696
error: queryResult.error.message,
97-
query,
9897
});
9998
return json({ error: queryResult.error.message }, { status: 400 });
10099
}
101100

102101
logger.error("Query API error", {
103102
error: queryResult.error,
104-
query,
105103
});
106104

107105
return json(

‎apps/webapp/app/routes/auth.sso.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,9 @@ export async function action({ request }: ActionFunctionArgs) {
5555
);
5656
if (rateError) {
5757
if (rateError instanceof SsoRateLimitError) {
58-
logger.warn("SSO login rate limit exceeded", { clientIp, email });
58+
logger.warn("SSO login rate limit exceeded", { clientIp });
5959
} else {
60-
logger.error("SSO login rate limiter failed", { clientIp, email, error: rateError });
60+
logger.error("SSO login rate limiter failed", { clientIp, error: rateError });
6161
}
6262
return redirect(`/login/sso?email=${encodeURIComponent(email)}&error=rate_limited`);
6363
}
@@ -78,7 +78,7 @@ export async function action({ request }: ActionFunctionArgs) {
7878

7979
const begun = await ssoController.beginAuthorization({ email, redirectTo, flow });
8080
if (begun.isErr()) {
81-
logger.warn("SSO beginAuthorization failed", { reason: begun.error, email, flow });
81+
logger.warn("SSO beginAuthorization failed", { reason: begun.error, flow });
8282
return redirect(`/login/sso?email=${encodeURIComponent(email)}&error=${begun.error}`);
8383
}
8484

‎apps/webapp/app/routes/login.magic/route.tsx‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,13 +185,11 @@ export async function action({ request }: ActionFunctionArgs) {
185185
if (error instanceof MagicLinkRateLimitError) {
186186
logger.warn("Login magic link rate limit exceeded", {
187187
clientIp,
188-
email,
189188
error,
190189
});
191190
} else {
192191
logger.error("Failed sending login magic link", {
193192
clientIp,
194-
email,
195193
error,
196194
});
197195
}

‎apps/webapp/app/services/emailAuth.server.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ const emailStrategy = new EmailLinkStrategy(
2828
form: FormData;
2929
magicLinkVerify: boolean;
3030
}) => {
31-
logger.info("Magic link user authenticated", { email, magicLinkVerify });
31+
logger.info("Magic link user authenticated", { magicLinkVerify });
3232

3333
// Gate the link CLICK, not just the send: a magic link issued before
3434
// SSO enforcement flipped on (or replayed within its validity

‎apps/webapp/app/services/gitHubAuth.server.ts‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,6 @@ export function addGitHubStrategy(
3636
}
3737

3838
try {
39-
logger.debug("GitHub login", {
40-
emails,
41-
profile,
42-
extraParams,
43-
});
44-
4539
const { user, isNewUser } = await findOrCreateUser({
4640
email,
4741
authenticationMethod: "GITHUB",

‎apps/webapp/app/services/googleAuth.server.ts‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,6 @@ export function addGoogleStrategy(
4545
}
4646

4747
try {
48-
logger.debug("Google login", {
49-
emails,
50-
profile,
51-
extraParams,
52-
});
53-
5448
const { user, isNewUser } = await findOrCreateUser({
5549
email,
5650
authenticationMethod: "GOOGLE",

‎apps/webapp/app/services/loops.server.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ class LoopsClient {
1313
email: string;
1414
name: string | null;
1515
}) {
16-
logger.info(`Loops send "sign-up" event`, { userId, email, name });
16+
logger.info(`Loops send "sign-up" event`, { userId });
1717
return this.#sendEvent({
1818
email,
1919
userId,
@@ -53,10 +53,7 @@ class LoopsClient {
5353
if (!response.ok) {
5454
logger.error(`Loops sendEvent ${eventName} bad status`, {
5555
status: response.status,
56-
email,
5756
userId,
58-
firstName,
59-
eventProperties,
6057
eventName,
6158
});
6259
return false;

‎apps/webapp/app/services/routeBuilders/apiBuilder.server.ts‎

Lines changed: 73 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
} from "../personalAccessToken.server";
1919
import { assertTokenOrganizationClaim, assertUserActorScope } from "../userActorEnvironment.server";
2020
import { safeJsonParse } from "~/utils/json";
21+
import { sanitizeHttpUrl } from "~/utils/sanitizeHttpUrl";
2122
import type { AuthenticatedWorkerInstance } from "~/v3/services/worker/workerGroupTokenService.server";
2223
import { WorkerGroupTokenService } from "~/v3/services/worker/workerGroupTokenService.server";
2324
import type { API_VERSIONS } from "~/api/versions";
@@ -31,24 +32,70 @@ import { tenantContext, tenantContextFromAuthEnvironment } from "~/services/tena
3132
// Client aborts and service-level validation errors aren't bugs — they're
3233
// expected at API boundaries. Log them at `warn` so they stay in stdout
3334
// without flowing to Sentry via Logger.onError.
35+
type DrizzleQueryError = Error & {
36+
query: string;
37+
params: unknown[];
38+
cause?: unknown;
39+
};
40+
41+
function isDrizzleQueryError(error: unknown): error is DrizzleQueryError {
42+
if (!(error instanceof Error)) return false;
43+
44+
try {
45+
return (
46+
error.message.startsWith("Failed query:") &&
47+
typeof (error as Partial<DrizzleQueryError>).query === "string" &&
48+
Array.isArray((error as Partial<DrizzleQueryError>).params)
49+
);
50+
} catch {
51+
return false;
52+
}
53+
}
54+
55+
function safeErrorCode(error: unknown): string | number | boolean | undefined {
56+
if ((typeof error !== "object" && typeof error !== "function") || error === null) return;
57+
58+
try {
59+
const code = (error as { code?: unknown }).code;
60+
return typeof code === "string" || typeof code === "number" || typeof code === "boolean"
61+
? code
62+
: undefined;
63+
} catch {
64+
return;
65+
}
66+
}
67+
68+
export function boundaryErrorLogValue(error: unknown) {
69+
if (isDrizzleQueryError(error)) {
70+
const code = safeErrorCode(error.cause);
71+
return {
72+
name: "DrizzleQueryError",
73+
message: "Database query failed",
74+
...(code === undefined ? {} : { causeCode: code }),
75+
};
76+
}
77+
78+
return error instanceof Error
79+
? { name: error.name, message: error.message, stack: error.stack }
80+
: String(error);
81+
}
82+
3483
function logBoundaryError(
3584
message: "Error in loader" | "Error in action" | "Unroutable id",
3685
error: unknown,
3786
url: string
3887
) {
39-
const formatted =
40-
error instanceof Error
41-
? { name: error.name, message: error.message, stack: error.stack }
42-
: String(error);
88+
const formatted = boundaryErrorLogValue(error);
89+
const sanitizedUrl = sanitizeHttpUrl(url);
4390
const isExpected =
4491
error instanceof Error &&
4592
(error.name === "AbortError" ||
4693
error instanceof ServiceValidationError ||
4794
error instanceof EngineServiceValidationError);
4895
if (isExpected) {
49-
logger.warn(message, { error: formatted, url });
96+
logger.warn(message, { error: formatted, url: sanitizedUrl });
5097
} else {
51-
logger.error(message, { error: formatted, url });
98+
logger.error(message, { error: formatted, url: sanitizedUrl });
5299
}
53100
}
54101

@@ -466,7 +513,10 @@ export function createLoaderApiRoute<
466513
corsStrategy !== "none"
467514
);
468515
} catch (innerError) {
469-
logger.error("[apiBuilder] Failed to handle error", { error, innerError });
516+
logger.error("[apiBuilder] Failed to handle error", {
517+
error: boundaryErrorLogValue(error),
518+
innerError: boundaryErrorLogValue(innerError),
519+
});
470520

471521
return json({ error: "Internal Server Error" }, { status: 500 });
472522
}
@@ -769,7 +819,10 @@ export function createLoaderPATApiRoute<
769819
corsStrategy !== "none"
770820
);
771821
} catch (innerError) {
772-
logger.error("[apiBuilder] Failed to handle error", { error, innerError });
822+
logger.error("[apiBuilder] Failed to handle error", {
823+
error: boundaryErrorLogValue(error),
824+
innerError: boundaryErrorLogValue(innerError),
825+
});
773826

774827
return json({ error: "Internal Server Error" }, { status: 500 });
775828
}
@@ -1061,7 +1114,10 @@ export function createActionPATApiRoute<
10611114
corsStrategy !== "none"
10621115
);
10631116
} catch (innerError) {
1064-
logger.error("[apiBuilder] Failed to handle error", { error, innerError });
1117+
logger.error("[apiBuilder] Failed to handle error", {
1118+
error: boundaryErrorLogValue(error),
1119+
innerError: boundaryErrorLogValue(innerError),
1120+
});
10651121

10661122
return json({ error: "Internal Server Error" }, { status: 500 });
10671123
}
@@ -1407,7 +1463,10 @@ export function createActionApiRoute<
14071463
corsStrategy !== "none"
14081464
);
14091465
} catch (innerError) {
1410-
logger.error("[apiBuilder] Failed to handle error", { error, innerError });
1466+
logger.error("[apiBuilder] Failed to handle error", {
1467+
error: boundaryErrorLogValue(error),
1468+
innerError: boundaryErrorLogValue(innerError),
1469+
});
14111470

14121471
return json({ error: "Internal Server Error" }, { status: 500 });
14131472
}
@@ -1679,7 +1738,10 @@ export function createMultiMethodApiRoute<
16791738
corsStrategy !== "none"
16801739
);
16811740
} catch (innerError) {
1682-
logger.error("[apiBuilder] Failed to handle error", { error, innerError });
1741+
logger.error("[apiBuilder] Failed to handle error", {
1742+
error: boundaryErrorLogValue(error),
1743+
innerError: boundaryErrorLogValue(innerError),
1744+
});
16831745
return json({ error: "Internal Server Error" }, { status: 500 });
16841746
}
16851747
}

‎apps/webapp/app/services/ssoAutoDiscovery.server.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,11 @@ export async function ssoRedirectForEmail(
4545
Promise.resolve(ssoController.decideRouteForEmail(normalised))
4646
);
4747
if (error) {
48-
logger.warn("SSO auto-discovery fail-open (threw)", { error, email: normalised });
48+
logger.warn("SSO auto-discovery fail-open (threw)", { error });
4949
return null;
5050
}
5151
if (decision.isErr()) {
52-
logger.warn("SSO auto-discovery fail-open", { reason: decision.error, email: normalised });
52+
logger.warn("SSO auto-discovery fail-open", { reason: decision.error });
5353
return null;
5454
}
5555
if (decision.value.kind !== "sso_required") return null;

0 commit comments

Comments
 (0)