Skip to content

Conversation

@bhumitschaudhry
Copy link

What does this PR do?

How did you verify your code works?

Copilot AI review requested due to automatic review settings January 29, 2026 09:51
@github-actions
Copy link
Contributor

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@github-actions
Copy link
Contributor

The following comment was made by an LLM, it may be inaccurate:

Based on my search results, I found several potentially related PRs about OAuth and Google services, but no direct duplicate PRs for adding Google OAuth (PR #11116).

The closest related PRs are:

These are related to Google/OAuth functionality but are not duplicates of the Google OAuth feature being added in PR #11116.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds Google OAuth authentication support to enable users to access Gemini models through their Google AI Pro/Ultra subscriptions. The implementation follows a similar pattern to the existing Codex OAuth plugin, adding a new authentication method alongside the existing API key approach.

Changes:

  • Added GoogleAuthPlugin implementing OAuth 2.0 with PKCE for Google AI authentication
  • Registered Google as a new internal plugin in the plugin system
  • Added "Google AI Pro/Ultra or API key" hint to the CLI auth provider selection

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 17 comments.

Show a summary per file
File Description
packages/opencode/src/plugin/google.ts New plugin implementing Google OAuth flow with local callback server, token management, and API request interception
packages/opencode/src/plugin/index.ts Registered GoogleAuthPlugin in INTERNAL_PLUGINS array
packages/opencode/src/cli/cmd/auth.ts Added Google to provider priority list and hints map
.planning/ROADMAP.md Planning document outlining multi-phase implementation roadmap (documentation only)
.planning/REQUIREMENTS.md Requirements specification for Google OAuth feature (documentation only)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +19 to +44
async function generatePKCE(): Promise<PkceCodes> {
const verifier = generateRandomString(43)
const encoder = new TextEncoder()
const data = encoder.encode(verifier)
const hash = await crypto.subtle.digest("SHA-256", data)
const challenge = base64UrlEncode(hash)
return { verifier, challenge }
}

function generateRandomString(length: number): string {
const chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~"
const bytes = crypto.getRandomValues(new Uint8Array(length))
return Array.from(bytes)
.map((b) => chars[b % chars.length])
.join("")
}

function base64UrlEncode(buffer: ArrayBuffer): string {
const bytes = new Uint8Array(buffer)
const binary = String.fromCharCode(...bytes)
return btoa(binary).replace(/\+/g, "-").replace(/\//g, "_").replace(/=+$/, "")
}

function generateState(): string {
return base64UrlEncode(crypto.getRandomValues(new Uint8Array(32)).buffer)
}
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

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

The utility functions generatePKCE(), generateRandomString(), base64UrlEncode(), and generateState() (lines 19-44) are duplicated from the codex plugin (codex.ts lines 19-44). This code duplication violates the DRY principle and makes maintenance harder - if a security fix is needed in the PKCE implementation, it would need to be applied to multiple files. Consider extracting these OAuth utility functions into a shared module that both google.ts and codex.ts can import.

Copilot uses AI. Check for mistakes.
}

let oauthServer: ReturnType<typeof Bun.serve> | undefined
let pendingOAuth: PendingOAuth | undefined
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

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

Global mutable state for OAuth server and pending OAuth requests creates potential race conditions when multiple authentication flows are attempted concurrently. If a user starts a second OAuth flow before the first completes, the second flow will overwrite pendingOAuth, causing the first flow to fail or complete incorrectly. Consider using a Map keyed by state parameter to support concurrent OAuth flows, similar to how the codex plugin handles this.

Suggested change
let pendingOAuth: PendingOAuth | undefined
const pendingOAuthByState = new Map<string, PendingOAuth>()

Copilot uses AI. Check for mistakes.

const log = Log.create({ service: "plugin.google" })

const CLIENT_ID = "1097030675611-3ap2u0pb0q9h5h0p5q0i9a2q2q2q2q2q.apps.googleusercontent.com"
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

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

The CLIENT_ID appears to contain placeholder values (repeating 'q2q2q2q2q'). This looks like a dummy/test client ID that should be replaced with a real Google OAuth client ID before deployment. Using an invalid client ID will cause all OAuth flows to fail. Verify this is a valid Google Cloud OAuth 2.0 client ID for the production application.

Suggested change
const CLIENT_ID = "1097030675611-3ap2u0pb0q9h5h0p5q0i9a2q2q2q2q2q.apps.googleusercontent.com"
const CLIENT_ID = process.env.GOOGLE_OAUTH_CLIENT_ID ?? ""

Copilot uses AI. Check for mistakes.
<div class="container">
<h1>Authorization Failed</h1>
<p>An error occurred during authorization.</p>
<div class="error">${error}</div>
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

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

The error parameter is directly interpolated into HTML without sanitization on line 172. If the error message contains user-controlled content from OAuth error responses, this could lead to XSS attacks. Google's error_description parameter could potentially contain HTML/JavaScript. HTML-encode the error string before inserting it into the HTML template to prevent XSS vulnerabilities.

Copilot uses AI. Check for mistakes.
if (currentAuth.type !== "oauth") return fetch(requestInput, init)

// Check if token needs refresh
if (!currentAuth.access || currentAuth.expires < Date.now()) {
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

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

The token expiry check on line 345 doesn't include a safety margin, causing token refresh to occur exactly when the token expires or after it has already expired. This creates a race condition where a request could be made with an expired token if the clock is slightly off or if there's network latency. Consider adding a safety margin (e.g., 5 minutes) before the actual expiry time to ensure tokens are refreshed proactively, as is common practice in OAuth implementations.

Copilot uses AI. Check for mistakes.
Comment on lines +192 to +253
oauthServer = Bun.serve({
port: OAUTH_PORT,
fetch(req) {
const url = new URL(req.url)

if (url.pathname === "/auth/callback") {
const code = url.searchParams.get("code")
const state = url.searchParams.get("state")
const error = url.searchParams.get("error")
const errorDescription = url.searchParams.get("error_description")

if (error) {
const errorMsg = errorDescription || error
pendingOAuth?.reject(new Error(errorMsg))
pendingOAuth = undefined
return new Response(HTML_ERROR(errorMsg), {
headers: { "Content-Type": "text/html" },
})
}

if (!code) {
const errorMsg = "Missing authorization code"
pendingOAuth?.reject(new Error(errorMsg))
pendingOAuth = undefined
return new Response(HTML_ERROR(errorMsg), {
status: 400,
headers: { "Content-Type": "text/html" },
})
}

if (!pendingOAuth || state !== pendingOAuth.state) {
const errorMsg = "Invalid state - potential CSRF attack"
pendingOAuth?.reject(new Error(errorMsg))
pendingOAuth = undefined
return new Response(HTML_ERROR(errorMsg), {
status: 400,
headers: { "Content-Type": "text/html" },
})
}

const current = pendingOAuth
pendingOAuth = undefined

exchangeCodeForTokens(code, `http://localhost:${OAUTH_PORT}/auth/callback`, current.pkce)
.then((tokens) => current.resolve(tokens))
.catch((err) => current.reject(err))

return new Response(HTML_SUCCESS, {
headers: { "Content-Type": "text/html" },
})
}

if (url.pathname === "/cancel") {
pendingOAuth?.reject(new Error("Login cancelled"))
pendingOAuth = undefined
return new Response("Login cancelled", { status: 200 })
}

return new Response("Not found", { status: 404 })
},
})

Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

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

The OAuth server uses a hardcoded port (1456) without error handling for port conflicts. If port 1456 is already in use by another application, Bun.serve() will throw an error and the OAuth flow will fail with an unclear error message. Consider either: 1) wrapping the Bun.serve() call in a try-catch and providing a helpful error message, 2) implementing port fallback logic to try alternative ports, or 3) using port 0 to let the system assign an available port dynamically.

Suggested change
oauthServer = Bun.serve({
port: OAUTH_PORT,
fetch(req) {
const url = new URL(req.url)
if (url.pathname === "/auth/callback") {
const code = url.searchParams.get("code")
const state = url.searchParams.get("state")
const error = url.searchParams.get("error")
const errorDescription = url.searchParams.get("error_description")
if (error) {
const errorMsg = errorDescription || error
pendingOAuth?.reject(new Error(errorMsg))
pendingOAuth = undefined
return new Response(HTML_ERROR(errorMsg), {
headers: { "Content-Type": "text/html" },
})
}
if (!code) {
const errorMsg = "Missing authorization code"
pendingOAuth?.reject(new Error(errorMsg))
pendingOAuth = undefined
return new Response(HTML_ERROR(errorMsg), {
status: 400,
headers: { "Content-Type": "text/html" },
})
}
if (!pendingOAuth || state !== pendingOAuth.state) {
const errorMsg = "Invalid state - potential CSRF attack"
pendingOAuth?.reject(new Error(errorMsg))
pendingOAuth = undefined
return new Response(HTML_ERROR(errorMsg), {
status: 400,
headers: { "Content-Type": "text/html" },
})
}
const current = pendingOAuth
pendingOAuth = undefined
exchangeCodeForTokens(code, `http://localhost:${OAUTH_PORT}/auth/callback`, current.pkce)
.then((tokens) => current.resolve(tokens))
.catch((err) => current.reject(err))
return new Response(HTML_SUCCESS, {
headers: { "Content-Type": "text/html" },
})
}
if (url.pathname === "/cancel") {
pendingOAuth?.reject(new Error("Login cancelled"))
pendingOAuth = undefined
return new Response("Login cancelled", { status: 200 })
}
return new Response("Not found", { status: 404 })
},
})
try {
oauthServer = Bun.serve({
port: OAUTH_PORT,
fetch(req) {
const url = new URL(req.url)
if (url.pathname === "/auth/callback") {
const code = url.searchParams.get("code")
const state = url.searchParams.get("state")
const error = url.searchParams.get("error")
const errorDescription = url.searchParams.get("error_description")
if (error) {
const errorMsg = errorDescription || error
pendingOAuth?.reject(new Error(errorMsg))
pendingOAuth = undefined
return new Response(HTML_ERROR(errorMsg), {
headers: { "Content-Type": "text/html" },
})
}
if (!code) {
const errorMsg = "Missing authorization code"
pendingOAuth?.reject(new Error(errorMsg))
pendingOAuth = undefined
return new Response(HTML_ERROR(errorMsg), {
status: 400,
headers: { "Content-Type": "text/html" },
})
}
if (!pendingOAuth || state !== pendingOAuth.state) {
const errorMsg = "Invalid state - potential CSRF attack"
pendingOAuth?.reject(new Error(errorMsg))
pendingOAuth = undefined
return new Response(HTML_ERROR(errorMsg), {
status: 400,
headers: { "Content-Type": "text/html" },
})
}
const current = pendingOAuth
pendingOAuth = undefined
exchangeCodeForTokens(code, `http://localhost:${OAUTH_PORT}/auth/callback`, current.pkce)
.then((tokens) => current.resolve(tokens))
.catch((err) => current.reject(err))
return new Response(HTML_SUCCESS, {
headers: { "Content-Type": "text/html" },
})
}
if (url.pathname === "/cancel") {
pendingOAuth?.reject(new Error("Login cancelled"))
pendingOAuth = undefined
return new Response("Login cancelled", { status: 200 })
}
return new Response("Not found", { status: 404 })
},
})
} catch (error) {
log.error("failed to start google oauth server", {
port: OAUTH_PORT,
error: error instanceof Error ? error.message : String(error),
})
throw new Error(
`Failed to start local OAuth callback server on port ${OAUTH_PORT}. The port may already be in use or blocked.`
)
}

Copilot uses AI. Check for mistakes.
Comment on lines +336 to +337
delete init.headers["authorization"]
delete init.headers["Authorization"]
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

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

Header manipulation logic deletes both lowercase 'authorization' and capitalized 'Authorization' headers from the object-style headers (lines 336-337), but HTTP headers are case-insensitive and should be treated as such. This approach misses headers with other capitalizations like 'AUTHORIZATION' or 'Authorization'. Consider normalizing header key comparisons or using a more robust method to ensure the authorization header is removed regardless of capitalization.

Suggested change
delete init.headers["authorization"]
delete init.headers["Authorization"]
for (const key of Object.keys(init.headers)) {
if (key.toLowerCase() === "authorization") {
delete init.headers[key as keyof typeof init.headers]
}
}

Copilot uses AI. Check for mistakes.
Comment on lines +65 to +66
if (!response.ok) {
throw new Error(`Token exchange failed: ${response.status}`)
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

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

Token exchange and refresh error responses (lines 65-66, 81-82) only include the HTTP status code in the error message. This provides insufficient information for debugging authentication failures. Google's OAuth error responses include error codes and descriptions in the response body that would help users understand what went wrong (e.g., invalid_grant, unauthorized_client). Parse and include the error response body in the error message to provide more actionable feedback.

Copilot uses AI. Check for mistakes.
Comment on lines +87 to +264
const HTML_SUCCESS = `<!doctype html>
<html>
<head>
<title>OpenCode - Google AI Authorization Successful</title>
<style>
body {
font-family:
system-ui,
-apple-system,
sans-serif;
display: flex;
justify-content: center;
align-items: center;
height: 100vh;
margin: 0;
background: #131010;
color: #f1ecec;
}
.container {
text-align: center;
padding: 2rem;
}
h1 {
color: #f1ecec;
margin-bottom: 1rem;
}
p {
color: #b7b1b1;
}
</style>
</head>
<body>
<div class="container">
<h1>Authorization Successful</h1>
<p>You can close this window and return to OpenCode.</p>
</div>
<script>
setTimeout(() => window.close(), 2000)
</script>
</body>
</html>`

const HTML_ERROR = (error: string) => `<!doctype html>
<html>
<head>
<title>OpenCode - Google AI Authorization Failed</title>
<style>
body {
font-family:
system-ui,
-apple-system,
sans-serif;
display: flex;
justify-content: center;
align-items: center;
height: 100vh;
margin: 0;
background: #131010;
color: #f1ecec;
}
.container {
text-align: center;
padding: 2rem;
}
h1 {
color: #fc533a;
margin-bottom: 1rem;
}
p {
color: #b7b1b1;
}
.error {
color: #ff917b;
font-family: monospace;
margin-top: 1rem;
padding: 1rem;
background: #3c140d;
border-radius: 0.5rem;
}
</style>
</head>
<body>
<div class="container">
<h1>Authorization Failed</h1>
<p>An error occurred during authorization.</p>
<div class="error">${error}</div>
</div>
</body>
</html>`

interface PendingOAuth {
pkce: PkceCodes
state: string
resolve: (tokens: TokenResponse) => void
reject: (error: Error) => void
}

let oauthServer: ReturnType<typeof Bun.serve> | undefined
let pendingOAuth: PendingOAuth | undefined

async function startOAuthServer(): Promise<{ port: number; redirectUri: string }> {
if (oauthServer) {
return { port: OAUTH_PORT, redirectUri: `http://localhost:${OAUTH_PORT}/auth/callback` }
}

oauthServer = Bun.serve({
port: OAUTH_PORT,
fetch(req) {
const url = new URL(req.url)

if (url.pathname === "/auth/callback") {
const code = url.searchParams.get("code")
const state = url.searchParams.get("state")
const error = url.searchParams.get("error")
const errorDescription = url.searchParams.get("error_description")

if (error) {
const errorMsg = errorDescription || error
pendingOAuth?.reject(new Error(errorMsg))
pendingOAuth = undefined
return new Response(HTML_ERROR(errorMsg), {
headers: { "Content-Type": "text/html" },
})
}

if (!code) {
const errorMsg = "Missing authorization code"
pendingOAuth?.reject(new Error(errorMsg))
pendingOAuth = undefined
return new Response(HTML_ERROR(errorMsg), {
status: 400,
headers: { "Content-Type": "text/html" },
})
}

if (!pendingOAuth || state !== pendingOAuth.state) {
const errorMsg = "Invalid state - potential CSRF attack"
pendingOAuth?.reject(new Error(errorMsg))
pendingOAuth = undefined
return new Response(HTML_ERROR(errorMsg), {
status: 400,
headers: { "Content-Type": "text/html" },
})
}

const current = pendingOAuth
pendingOAuth = undefined

exchangeCodeForTokens(code, `http://localhost:${OAUTH_PORT}/auth/callback`, current.pkce)
.then((tokens) => current.resolve(tokens))
.catch((err) => current.reject(err))

return new Response(HTML_SUCCESS, {
headers: { "Content-Type": "text/html" },
})
}

if (url.pathname === "/cancel") {
pendingOAuth?.reject(new Error("Login cancelled"))
pendingOAuth = undefined
return new Response("Login cancelled", { status: 200 })
}

return new Response("Not found", { status: 404 })
},
})

log.info("google oauth server started", { port: OAUTH_PORT })
return { port: OAUTH_PORT, redirectUri: `http://localhost:${OAUTH_PORT}/auth/callback` }
}

function stopOAuthServer() {
if (oauthServer) {
oauthServer.stop()
oauthServer = undefined
log.info("google oauth server stopped")
}
}
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

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

The OAuth server implementation (lines 187-264) and HTML templates (lines 87-175) are largely duplicated from the codex plugin. This represents significant code duplication that makes the codebase harder to maintain. Consider creating a shared OAuth server utility that can be configured with provider-specific settings (port, HTML branding, etc.) and reused across both google.ts and codex.ts plugins.

Copilot uses AI. Check for mistakes.
import type { Hooks, PluginInput } from "@opencode-ai/plugin"
import { Log } from "../util/log"
import { Installation } from "../installation"
import { Auth, OAUTH_DUMMY_KEY } from "../auth"
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

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

Unused import Auth.

Suggested change
import { Auth, OAUTH_DUMMY_KEY } from "../auth"
import { OAUTH_DUMMY_KEY } from "../auth"

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant