Skip to content

fix(wrappers): guard against undefined Browser.cookies in getChatGptAccessToken #965

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

Open
octo-patch wants to merge 1 commit into
base: master
Choose a base branch
from
+8 −5
Open

fix(wrappers): guard against undefined Browser.cookies in getChatGptAccessToken #965

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 8 additions & 5 deletions src/services/wrappers.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,14 @@ export async function getChatGptAccessToken() {
if (userConfig.accessToken) {
return userConfig.accessToken
} else {
const cookie = (await Browser.cookies.getAll({ url: 'https://chatgpt.com/' }))
.map((cookie) => {
return `${cookie.name}=${cookie.value}`
})
.join('; ')
let cookie = ''
if (Browser.cookies && Browser.cookies.getAll) {
cookie = (await Browser.cookies.getAll({ url: 'https://chatgpt.com/' }))
.map((cookie) => {
return `${cookie.name}=${cookie.value}`
Comment on lines +21 to +22
})
.join('; ')
}
Comment on lines +18 to +25
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot Apr 25, 2026

Choose a reason for hiding this comment

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

medium

The variable cookie is shadowed by the parameter in the .map() callback, which is a poor practice that can lead to confusion and potential bugs. Additionally, the logic can be simplified by using a const declaration with a ternary operator and optional chaining, making the code more concise and idiomatic while avoiding the let declaration.

    const cookie = (Browser.cookies?.getAll
      ? await Browser.cookies.getAll({ url: 'https://chatgpt.com/' })
      : [])
      .map((c) => `${c.name}=${c.value}`)
      .join('; ')

const resp = await fetch('https://chatgpt.com/api/auth/session', {
Comment on lines +19 to 26
headers: {
Cookie: cookie,
Comment on lines +18 to 28
Copy link
Copy Markdown
Contributor

@qodo-code-review qodo-code-review Bot Apr 25, 2026

Choose a reason for hiding this comment

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

Action required

1. Missing session-cookie fallback 🐞 Bug ≡ Correctness

When Browser.cookies is unavailable, getChatGptAccessToken still sends an always-present (often
empty) Cookie header and does not set credentials: 'include', so the /api/auth/session request may
not carry any session cookies and will keep returning no accessToken (throwing UNAUTHORIZED). This
makes the new guard avoid the crash but still breaks token retrieval in the target environments.
Agent Prompt 
### Issue description
`getChatGptAccessToken()` now avoids a crash when `Browser.cookies` is missing, but the fallback still makes a `fetch('https://chatgpt.com/api/auth/session')` request that may not include any session cookies (it always sets `headers.Cookie`, often to `''`, and does not set `credentials: 'include'`). This can cause the endpoint to return no `accessToken`, resulting in persistent `UNAUTHORIZED` errors in the very environments this PR targets.

### Issue Context
A more robust pattern already exists in `chatgpt-web.mjs`: always use `credentials: 'include'`, and only set a manual `Cookie` header when you actually have a non-empty cookie string.

### Fix Focus Areas
- src/services/wrappers.mjs[18-30]

### Suggested change
- Build `headers` so `Cookie` is only included when `cookie` is non-empty (e.g. `...(cookie && { Cookie: cookie })`).
- Add `credentials: 'include'` to the `fetch` options so environments without `Browser.cookies` still have a chance to send existing session cookies automatically.
- (Optional but recommended) Add/extend a unit test to cover the `Browser.cookies` undefined case and assert that the request either omits `Cookie` when empty and/or sets `credentials: 'include'`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Expand Down