Skip to content

fix: add missing error handling in OAuth token exchange and credential decryption #148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
guyb1 merged 2 commits into from
Apr 3, 2026
Merged

fix: add missing error handling in OAuth token exchange and credential decryption #148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7034292
fix: add missing error handling in OAuth token exchange and credentia…
lawrence3699 Apr 2, 2026
b61eb8f
fix
guyb1 Apr 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions apps/web/src/lib/apps/github.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,12 @@ export const github: AppDefinition = {
},
);

if (!tokenRes.ok) {
throw new Error(
`GitHub token exchange failed: ${tokenRes.status} ${tokenRes.statusText}`,
Comment on lines +92 to +93
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On non-OK responses, this throws using only status/statusText and skips reading the response body. GitHub’s token endpoint often returns a JSON body with useful error/error_description even when the status isn’t 2xx; consider including the parsed JSON (when available) or at least await tokenRes.text() in the thrown error to improve diagnostics.

Suggested change
throw new Error(
`GitHub token exchange failed: ${tokenRes.status} ${tokenRes.statusText}`,
let errorDetail = "";
try {
const bodyText = await tokenRes.text();
if (bodyText) {
errorDetail = ` - Response body: ${bodyText}`;
}
} catch {
// Ignore body read errors; fall back to status information only.
}
throw new Error(
`GitHub token exchange failed: ${tokenRes.status} ${tokenRes.statusText}${errorDetail}`,

Copilot uses AI. Check for mistakes.
);
}

const tokenData = (await tokenRes.json()) as {
access_token?: string;
scope?: string;
Expand Down
6 changes: 6 additions & 0 deletions apps/web/src/lib/apps/google-oauth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ export const exchangeGoogleCode = async ({
}),
});

if (!tokenRes.ok) {
throw new Error(
`Google token exchange failed: ${tokenRes.status} ${tokenRes.statusText}`,
Comment on lines +52 to +53
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The early !tokenRes.ok throw prevents reading Google’s JSON error payload (e.g., error_description), and makes the later tokenData.error handling ineffective for non-2xx responses. Consider on non-OK: read Content-Type and attempt tokenRes.json() when it’s JSON (falling back to tokenRes.text() otherwise) so the thrown error includes the provider’s error details while still avoiding Unexpected token '<' on HTML bodies.

Suggested change
throw new Error(
`Google token exchange failed: ${tokenRes.status} ${tokenRes.statusText}`,
const contentType = tokenRes.headers.get("content-type") ?? "";
let errorDetail: string | undefined;
try {
if (contentType.includes("application/json")) {
const errorBody = (await tokenRes.json()) as {
error?: string;
error_description?: string;
};
errorDetail =
errorBody.error_description ||
errorBody.error ||
undefined;
} else {
const text = await tokenRes.text();
errorDetail = text || undefined;
}
} catch {
// Ignore body parsing errors; fall back to generic message below.
}
const baseMessage = `Google token exchange failed: ${tokenRes.status} ${tokenRes.statusText}`;
throw new Error(
errorDetail ? `${baseMessage} - ${errorDetail}` : baseMessage,

Copilot uses AI. Check for mistakes.
);
}

const tokenData = (await tokenRes.json()) as {
access_token?: string;
refresh_token?: string;
Expand Down
16 changes: 13 additions & 3 deletions apps/web/src/lib/services/app-config-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import { db, Prisma } from "@onecli/db";
import { cryptoService } from "@/lib/crypto";
import { logger } from "@/lib/logger";
import { ServiceError } from "@/lib/services/errors";
import type { OAuthConfigField } from "@/lib/apps/types";

Expand Down Expand Up @@ -55,9 +56,18 @@ export const getAppConfigCredentials = async (

if (!config.credentials) return settings;

const decrypted = JSON.parse(
await cryptoService.decrypt(config.credentials),
) as Record<string, string>;
let decrypted: Record<string, string>;
try {
decrypted = JSON.parse(
await cryptoService.decrypt(config.credentials),
) as Record<string, string>;
} catch (err) {
logger.warn(
{ err, accountId, provider },
"failed to decrypt app config credentials",
);
return settings;
}

return { ...settings, ...decrypted };
};
Expand Down
Loading