Skip to content

var improvements for lanes and accounts/ orgs support#16

Open
lucidNTR wants to merge 1 commit intomainfrom
pr16
Open

var improvements for lanes and accounts/ orgs support#16
lucidNTR wants to merge 1 commit intomainfrom
pr16

Conversation

@lucidNTR
Copy link
Copy Markdown
Contributor

@lucidNTR lucidNTR commented Mar 18, 2025

Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 4 potential issues.

View 7 additional findings in Devin Review.

Open in Devin Review

Comment thread app/src/make-pouch.js
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Post-increment count++ causes infinite retry loop on HTTP 429

In rateRetryFetch, the recursive call uses count++ (post-increment) which passes the current value of count before incrementing. Since this is a recursive call (not a loop), the incremented local variable is never used again. Every recursive call receives the same initial value (1), so sleep always waits 800ms and the count < 4 check always passes, creating an infinite retry loop whenever a 429 status is received. Should be ++count or count + 1.

(Refers to line 23)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread edge/handlers/_session.js
Comment on lines +160 to +161
headers['Set-Cookie'] = 'CF_Authorization=deleted; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; Secure; SameSite=Strict; HttpOnly'
headers['Set-Cookie'] = 'AYU_SESSION_ID=deleted; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; Secure; SameSite=Strict; HttpOnly'
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 Set-Cookie header overwrite prevents CF_Authorization cookie deletion on logout

On logout when jwtPayload.email is truthy, headers['Set-Cookie'] is assigned twice on a plain JavaScript object (lines 160-161). The second assignment (AYU_SESSION_ID=deleted) overwrites the first (CF_Authorization=deleted), so the CF_Authorization cookie is never cleared. This is a security issue: the authentication cookie remains in the browser after logout, potentially allowing session reuse. A Headers object or an array should be used to set multiple Set-Cookie headers.

Prompt for agents
In edge/handlers/_session.js lines 146-170, the logout handler uses a plain object for headers, which cannot hold duplicate keys like Set-Cookie. On lines 160-161, the second assignment to headers['Set-Cookie'] overwrites the first, so CF_Authorization is never deleted on logout. Fix this by using the Headers API or an array of arrays for the headers, so that both Set-Cookie values are sent. For example, replace the plain object with new Headers() and use headers.append('Set-Cookie', ...) for each cookie, or use an array-of-arrays format accepted by the Response constructor. This applies to the jwtPayload.email branch specifically (lines 157-161).
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread app/src/make-pouch.js
if (!replicationConf.push || updateReplications) {
replicationConf.push = sync.push.replicationId
}
await pouch.put(sessionDoc)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 sessionDoc is always written to pouch even when replication IDs haven't changed

The init function in make-pouch.js now calls await pouch.put(sessionDoc) unconditionally at line 364 whenever sync.pull.replicationId && sync.push.replicationId are truthy. Previously, the write was gated by if (!sessionDoc.replications || updateReplications). In the new code, even when replicationConf.pull and replicationConf.push already match the sync replication IDs (i.e., updateReplications is false and both fields are already set), the put still executes, causing an unnecessary database write on every init.

Suggested change
await pouch.put(sessionDoc)
if (!replicationConf.pull || !replicationConf.push || updateReplications) {
await pouch.put(sessionDoc)
}
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread edge/handlers/_session.js
},
members: {
names: [], // lanesUserId
names: [lanesUserId],
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 CouchDB _security members.names receives number instead of string

At edge/handlers/_session.js:380, lanesUserId (a number from Date.now() + Math.floor(Math.random() * 10000)) is placed directly in the members.names array. CouchDB's _security document expects names to be an array of strings (matching CouchDB user names). The _users doc on edge/handlers/_session.js:351 uses name: \${lanesUserId}`` (string), but the security doc passes the raw number. This mismatch means the security document won't correctly match the user, potentially denying access. The same issue occurs at line 405 for the org database.

Suggested change
names: [lanesUserId],
names: [`${lanesUserId}`],
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant