You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Multi-user accounts so anyone can sign up, connect their YouTube channel, and use the tool — per the approved plan.
Sign-up/sign-in: Google identity only (/api/auth/google/login, scopes openid email profile). Two-step by design: identity first, then the existing youtube.force-ssl consent at /api/auth/google (URLs unchanged, so no Google console changes).
No auth library (repo constraint stands): DIY sessions in src/lib/server/session.ts — random 32-byte token, sessions table, httpOnly moderaty_session cookie, 30-day sliding expiry. hooks.server.ts populates locals.user.
Guard:(app) layout redirects signed-out visitors to /login; every form action calls requireUser(locals) (401 backstop). New /login page, /logout action, account chip + sign-out in the app header; landing-page CTAs now go to /login.
Ownership scoping: every channel read/write is filtered by channels.userId — dashboard (channels, stats, bans), setToneLevel, rules add/remove, review-queue actions, audit log. Cross-owner always 404 (no existence leaks). Connect callback attaches userId and refuses (409) reattaching a channel owned by another account. This also resolves the earlier CodeRabbit setToneLevel auth finding properly.
Migration 0004 (additive/nullable): users (google_sub unique, plan default 'free' — future Stripe hook), sessions, channels.user_id. Orphaned pre-accounts channels are claimed by the first user ever to sign in — that's how the existing prod DB attaches to its owner.
BYOK: self-hosted uses the same code path with their own Google/OpenAI/Turso env keys — documented in README; hosted plans via Stripe noted as a future feature in the execution plan (§7). Cron unchanged.
Verify
npm run test — 160/160 (new: session lib, login flow incl. find-or-create/orphan-claim/cross-owner, scoping + 401 tests on every action)
After merge: run migration 0004 on prod (additive, no data rewrite). First Google sign-in on the existing DB claims the orphaned channels — sign in with the owner account first. Add http://localhost:5173/api/auth/google/login/callback and the prod equivalent to the Google OAuth client's authorized redirect URIs.
Multi-user accounts so anyone can sign up, connect their YouTube channel, and use the tool — per the approved plan.
Sign-up/sign-in: Google identity only (/api/auth/google/login, scopes openid email profile). Two-step by design: identity first, then the existing youtube.force-ssl consent at /api/auth/google (URLs unchanged, so no Google console changes).
No auth library (repo constraint stands): DIY sessions in src/lib/server/session.ts — random 32-byte token, sessions table, httpOnly moderaty_session cookie, 30-day sliding expiry. hooks.server.ts populates locals.user.
Guard:(app) layout redirects signed-out visitors to /login; every form action calls requireUser(locals) (401 backstop). New /login page, /logout action, account chip + sign-out in the app header; landing-page CTAs now go to /login.
Ownership scoping: every channel read/write is filtered by channels.userId — dashboard (channels, stats, bans), setToneLevel, rules add/remove, review-queue actions, audit log. Cross-owner always 404 (no existence leaks). Connect callback attaches userId and refuses (409) reattaching a channel owned by another account. This also resolves the earlier CodeRabbit setToneLevel auth finding properly.
Migration 0004 (additive/nullable): users (google_sub unique, plan default 'free' — future Stripe hook), sessions, channels.user_id. Orphaned pre-accounts channels are claimed by the first user ever to sign in — that's how the existing prod DB attaches to its owner.
BYOK: self-hosted uses the same code path with their own Google/OpenAI/Turso env keys — documented in README; hosted plans via Stripe noted as a future feature in the execution plan (§7). Cron unchanged.
Verify
npm run test — 160/160 (new: session lib, login flow incl. find-or-create/orphan-claim/cross-owner, scoping + 401 tests on every action)
After merge: run migration 0004 on prod (additive, no data rewrite). First Google sign-in on the existing DB claims the orphaned channels — sign in with the owner account first. Add http://localhost:5173/api/auth/google/login/callback and the prod equivalent to the Google OAuth client's authorized redirect URIs.
CodeAnt-AI Description
Add Google accounts with private, ownership-scoped YouTube moderation
What Changed
Users can sign in with Google, maintain a 30-day session, and sign out from the app
Signed-out visitors are sent to a dedicated sign-in page, while active sessions remain renewed during use
Each dashboard, channel, rule, queue, tone, and audit-log operation is limited to channels owned by the signed-in user
Connecting a YouTube channel requires an authenticated account and cannot take over another account’s channel
Existing ownerless channels are assigned to the first account that signs in
The database now stores users, sessions, and channel ownership; self-hosted installations can use their own Google, OpenAI, and Turso credentials
Impact
✅ Google sign-in for every account ✅ Private channel and moderation data ✅ Fewer unauthorized channel changes
💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
@codeant-ai ask: Your question here
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
@codeant-ai ask: Can you suggest a safer alternative to storing this secret?
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
@codeant-ai: Your feedback here
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
@codeant-ai: Do not flag unused imports.
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
@codeant-ai: review
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
The reason will be displayed to describe this comment to others. Learn more.
This PR implements comprehensive multi-user authentication with proper ownership scoping. The implementation includes DIY session management, Google OAuth integration, and ownership checks across all channel operations.
Critical Issue Found:
One security vulnerability requires immediate attention: missing foreign key constraint on sessions.userId that could allow deleted users to remain authenticated via unexpired sessions.
The ownership scoping implementation is thorough and correctly prevents cross-user access throughout the codebase. Session management follows secure practices with httpOnly cookies, sliding expiry, and proper CSRF protection via state tokens.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
⚠️ This PR contains more than 30 files. Amazon Q is better at reviewing smaller PRs, and may miss issues in larger changesets.
• Add Google identity login and DIY cookie sessions for multi-user accounts
• Gate all (app) routes and actions behind sessions, with 401/404 backstops
• Scope all channel reads/writes by owner and add DB migration + full test coverage
Diagram
graph TD
U(["User / Browser"]) --> R["SvelteKit routes"] --> H["hooks.server.ts"] --> S["session.ts"] --> D[("SQLite (Turso) DB")]
R --> G{{"Google OAuth (OIDC)"}} --> R
R --> Y{{"YouTube OAuth"}} --> R
R --> P["(app) pages & actions"] --> D
subgraph Legend
direction LR
_u(["User"]) ~~~ _svc["App module"] ~~~ _db[("Database")] ~~~ _ext{{"External"}}
end
Loading
High-Level Assessment
The following are alternative approaches to this PR:
1. Adopt an auth library (Auth.js / Lucia / etc.)
➕ Battle-tested session handling, CSRF protections, and provider integration
➕ Reduced custom security surface area
➕ More features (account linking, rotation, adapters) with less code
➖ Conflicts with the repo constraint of no auth libraries
➖ More dependencies and upgrade churn
➖ May impose patterns that don't fit current minimal stack
2. Stateless JWT sessions instead of DB-backed sessions
➕ No sessions table and fewer DB reads per request
➖ Harder to revoke sessions immediately on logout/compromise
➖ More key management complexity (rotation, invalidation)
➖ Riskier to implement correctly without an established library
3. Use a single OAuth consent with both identity + YouTube scopes
➕ Fewer redirects and simpler user flow
➕ One OAuth state machine to maintain
➖ Requests sensitive YouTube scope before it's needed
➖ Would likely require Google Console/OAuth consent adjustments
➖ Less aligned with least-privilege and current URL stability goal
Recommendation: Keep the PR’s approach: DB-backed DIY sessions plus a two-step OAuth (identity first, YouTube consent second). Given the explicit dependency constraint (no auth library) and the desire to keep existing YouTube OAuth URLs unchanged, this is the best fit. The owner-scoping pattern (404 on cross-owner) is consistently applied and appropriately tested; focus review on security edge cases (cookie options, state handling, and ownership filters) rather than swapping approaches.
Files changed (31) +1674 / -59
Enhancement (11) +494 / -5
app.d.tsType locals.user for session-backed authentication+6/-1
Type locals.user for session-backed authentication
• Defines App.Locals.user as SessionUser | null, enabling typed access to the signed-in user across loads and actions.
session.tsImplement DIY DB-backed sessions with requireUser guard+97/-0
Implement DIY DB-backed sessions with requireUser guard
• Introduces session token creation, resolution with sliding expiry renewal, logout deletion, and a requireUser() helper that throws 401 when unauthenticated.
+server.tsAdd Google identity-only login start endpoint+47/-0
Add Google identity-only login start endpoint
• Implements /api/auth/google/login to generate a CSRF state, store it in the oauth_state cookie, and redirect to Google with openid/email/profile scopes.
+server.tsImplement login callback: user creation, orphan claim, session cookie+144/-0
Implement login callback: user creation, orphan claim, session cookie
• Exchanges code for token (no retry), fetches userinfo, find-or-creates a user by google_sub, claims orphaned channels on first-ever login, creates a session row, sets moderaty_session, consumes oauth_state, and redirects to /dashboard.
+page.server.tsScope dashboard data and tone-level updates to owned channels+34/-16
Scope dashboard data and tone-level updates to owned channels
• Requires user in load/actions, filters channel list by owner, constrains stats/bans queries to owned channel IDs, and scopes setToneLevel updates by channels.userId.
+server.tsAttach connected channels to signed-in user and block cross-owner reattach+20/-2
Attach connected channels to signed-in user and block cross-owner reattach
• Requires an authenticated user for YouTube callback, sets channels.userId on upsert, and rejects attempts to attach a channel already owned by another user with 409.
actions.test.tsAdd rules action tests for scoping and authentication failures+30/-4
Add rules action tests for scoping and authentication failures
• Extends tests to include channels ownership, verifies 404 on cross-owner channels and 401 when signed out, and ensures rules remain unchanged on failure.
oauth.test.tsUpdate YouTube OAuth tests for session requirement and channel ownership+44/-3
Update YouTube OAuth tests for session requirement and channel ownership
• Extends mocks to simulate existing channel ownership; adds tests for 401 when signed out, userId attachment on connect, and 409 on cross-owner reconnect attempts.
Concurrent channel connections can bypass the ownership check and reassign a channel
The ownership check and the channel upsert are separate operations. Two users can concurrently observe no conflicting owner, after which the later onConflictDoUpdate overwrites the existing owner and encrypted refresh token. Make the ownership condition part of an atomic conditional update or enforce the conflict inside a transaction so a channel cannot be reassigned after the check.
❌ A channel can be reassigned across user accounts.
❌ One user's refresh token can be overwritten.
⚠️ The original owner can lose channel access.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/routes/api/auth/google/callback/+server.ts
**Line:** 136:158
**Comment:***Race Condition: The ownership check and the channel upsert are separate operations. Two users can concurrently observe no conflicting owner, after which the later `onConflictDoUpdate` overwrites the existing owner and encrypted refresh token. Make the ownership condition part of an atomic conditional update or enforce the conflict inside a transaction so a channel cannot be reassigned after the check.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Critical
Security
Returning the full channel row exposes the encrypted refresh credential in browser load data
The channel query uses an unrestricted select() and the returned row is included in the server load result. SvelteKit serializes this result to the browser, so the encrypted YouTube refresh token in ch.refreshTokenEnc is exposed to the owning user's client. Project only the channel fields needed by the queue page, excluding credential columns.
⚠️ Queue load data exposes encrypted YouTube credentials.
⚠️ Browser extensions can read unnecessary credential ciphertext.
⚠️ Credential data expands client-side exposure unnecessarily.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/routes/(app)/channels/[id]/queue/+page.server.ts
**Line:** 43:49
**Comment:***Security: The channel query uses an unrestricted `select()` and the returned row is included in the server load result. SvelteKit serializes this result to the browser, so the encrypted YouTube refresh token in `ch.refreshTokenEnc` is exposed to the owning user's client. Project only the channel fields needed by the queue page, excluding credential columns.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Major
The rules load exposes the channel's encrypted refresh credential to the browser
The rules page returns the full channel row from its server load, including refreshTokenEnc, which SvelteKit serializes to the browser. This route should use an explicit non-secret projection rather than returning the database row.
⚠️ Rules load data exposes encrypted YouTube credentials.
⚠️ Client receives credential data it never renders.
⚠️ Browser-side exposure increases sensitive-data handling.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/routes/(app)/channels/[id]/rules/+page.server.ts
**Line:** 29:35
**Comment:***Security: The rules page returns the full channel row from its server load, including `refreshTokenEnc`, which SvelteKit serializes to the browser. This route should use an explicit non-secret projection rather than returning the database row.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Major
Every newly created account can claim all remaining orphaned channels
The orphan-claim update runs for every newly created Google account, not only for the first account. Any remaining nullable channel owner is therefore assigned wholesale to whichever unrelated user signs in next, causing ownership and access-control corruption. Gate this migration claim to the first-account condition or perform an explicit one-time migration.
❌ Legacy channels can be assigned to the wrong user.
❌ Ownership scoping can expose another account's channels.
⚠️ Concurrent first logins make migration nondeterministic.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/routes/api/auth/google/login/callback/+server.ts
**Line:** 118:128
**Comment:***Security: The orphan-claim update runs for every newly created Google account, not only for the first account. Any remaining nullable channel owner is therefore assigned wholesale to whichever unrelated user signs in next, causing ownership and access-control corruption. Gate this migration claim to the first-account condition or perform an explicit one-time migration.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Major
Possible bug
Userinfo network failures escape as unhandled errors instead of controlled sign-in failures
The userinfo request and infoRes.text() are outside the error handler used for the token exchange. Network failures and timeout exceptions from either operation escape as unhandled route errors instead of the documented retryable 502 response. Wrap the complete userinfo request and body read in equivalent error handling.
❌ Google sign-in fails with an uncontrolled server error.
⚠️ Userinfo timeouts lack the documented retryable response.
⚠️ Server error handling becomes inconsistent across OAuth steps.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/routes/api/auth/google/login/callback/+server.ts
**Line:** 93:97
**Comment:***Possible Bug: The userinfo request and `infoRes.text()` are outside the error handler used for the token exchange. Network failures and timeout exceptions from either operation escape as unhandled route errors instead of the documented retryable 502 response. Wrap the complete userinfo request and body read in equivalent error handling.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
The PR successfully implements Google identity integration, DIY session management, and strict ownership scoping for channel data. Functional acceptance criteria, including the 404 response for cross-owner access and the 409 conflict for existing channel connections, appear well-handled.
However, the overall code quality is not up to standards. The primary concern is the significant duplication of logic for verifying channel ownership and performing OAuth token exchanges. Because these are security-critical paths, they must be refactored into shared utilities to prevent inconsistent enforcement of access controls in the future. Additionally, there is minor clutter in the TypeScript definitions and duplication in the test utility helpers.
About this PR
The PR exhibits a pattern of duplicating sensitive logic, specifically around channel ownership verification and Google OAuth token exchange. Centralizing these into shared server-side utilities is recommended to ensure consistency and simplify future maintenance of the authentication and authorization layers.
1 comment outside of the diffsrc/app.d.ts
line 18 ⚪ LOW RISK
Nitpick: This empty export is unnecessary. Since this file already imports SessionUser, the export {} is redundant and can be safely removed.
Test suggestions
DIY session lifecycle: creation, resolution, renewal within the 15-day window, and destruction.
Google identity sign-in flow: CSRF state verification, token exchange, and find-or-create user logic.
Ownership scoping: The dashboard must only display channels belonging to the authenticated user.
Cross-owner protection: Form actions (queue, rules, settings) must return 404 when targeting another user's channel.
Authentication guard: Form actions must return 401 when the session is missing or expired.
Conflict prevention: Connecting a YouTube channel already linked to a different account must return 409.
Migration logic: Orphaned channels are correctly claimed by the first user who registers.
The reason will be displayed to describe this comment to others. Learn more.
🟡 MEDIUM RISK
Suggestion: The logic for exchanging an authorization code for tokens and fetching Google user information is duplicated between the identity callback and the YouTube connection callback. Extracting this into a common service (e.g., src/lib/server/google.ts) would ensure consistent error handling for upstream failures.
In the Google login callback, every newly-created user updates all orphan channels
(channels.user_id IS NULL) to themselves, which is not atomic and can assign legacy channels to
the wrong account under concurrent first-time sign-ins. This contradicts the intended invariant that
orphans are claimed by the first user ever to sign in.
+ // Find-or-create the account by Google's stable sub claim.+ let user = await db.select().from(users).where(eq(users.googleSub, info.sub)).get();+ if (!user) {+ user = await db+ .insert(users)+ .values({ id: randomBytes(16).toString('hex'), googleSub: info.sub, email, displayName })+ .returning()+ .get();+ // First login ever on a pre-accounts database claims the orphaned+ // (ownerless) channels. A fresh multi-user deploy has no orphans.+ await db.update(channels).set({ userId: user.id }).where(isNull(channels.userId));+ }
Relevance
●● Moderate
Concurrency/atomic-claim change is non-trivial; similar “atomic claim” hardening was previously
rejected in other context.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The callback unconditionally claims all orphan channels whenever it creates a new user, while
project guidance says this claim should happen only once for the first-ever sign-in.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
`src/routes/api/auth/google/login/callback/+server.ts` claims orphaned channels (`user_id IS NULL`) inside the `if (!user)` block for *any* newly-created user. Because the user insert and orphan-claim update are separate statements with no serialization, two concurrent first-time sign-ins on a pre-accounts DB can race and end up with the last writer owning the legacy channels.
### Issue Context
The repo documentation states: orphan channels are claimed by the *first* user ever to sign in, implying this must be a one-time global initialization step.
### Fix Focus Areas
- src/routes/api/auth/google/login/callback/+server.ts[117-128]
### Suggested fix approach
- Serialize “first-user initialization” so it can only happen once:
- Option A (recommended): Wrap `find-or-create user` + `orphan claim` in a single DB transaction that prevents concurrent writers (on SQLite, ensure the transaction is write-locking/IMMEDIATE so two callbacks can’t interleave).
- Within that transaction, only run the orphan-claim update if the users table was empty *before* this insert (i.e., this request truly created the first user).
- Add a regression test that simulates two concurrent sign-ins on a DB with orphan channels and asserts only one user ends up owning them.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Ownership scoping introduces frequent filters on channels.user_id, but migration 0004 only adds
the column without an index. As the channels table grows, per-user dashboards and ownership checks
will degrade into full table scans.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
The PR adds `channels.user_id` and then relies on it in common queries (e.g., dashboard loads), but the migration does not create an index on `channels.user_id`.
### Issue Context
SQLite will generally need an index on the leading filter column to avoid scanning `channels` for `WHERE user_id = ?`.
### Fix Focus Areas
- drizzle/0004_smiling_nextwave.sql[35-36]
- src/lib/server/db/schema.ts[38-51]
### Suggested fix approach
- Add an index in SQL (either amend 0004 before merge, or add a new migration if 0004 is already deployed anywhere):
- `CREATE INDEX channels_user_id_idx ON channels(user_id);`
- Mirror it in Drizzle schema using `sqliteTable(..., (t) => ({ ... }))` with `index('channels_user_id_idx').on(t.userId)` so drift tools don’t remove it.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The OAuth login flow stores the state value in an HttpOnly cookie, but the cookie is not marked
secure, so it may be sent over non-HTTPS connections. This weakens the CSRF protection
expectations for the OAuth state mechanism.
+ // CSRF guard: bind the login request to this browser session. The new state+ // is appended rather than replacing the cookie so overlapping starts in+ // multiple tabs stay valid.+ const state = randomBytes(16).toString('hex');+ storePendingStates(cookies, [...readPendingStates(cookies), state]);
Relevance
●●● Strong
Repo has history of tightening OAuth state cookie handling; adding secure flag matches existing
session-cookie pattern.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The OAuth state compliance rule’s success criteria calls for an HttpOnly *secure* cookie. The new
login start handler persists state via storePendingStates, but storePendingStates sets the
oauth_state cookie without secure, making the cookie configuration non-compliant with the rule’s
secure-cookie expectation.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
`oauth_state` is stored in an HttpOnly cookie but is missing the `secure` flag, which the compliance rule’s success criteria expects for OAuth `state` cookies.
## Issue Context
`storePendingStates()` currently sets the cookie without `secure`. The new login start handler relies on this helper.
## Fix Focus Areas
- src/lib/server/oauthState.ts[47-57]
- src/routes/api/auth/google/login/+server.ts[29-47]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The newly added src/routes/login/+page.svelte file has no <script lang="ts"> block, which
violates the requirement that new Svelte components use TypeScript. This can lead to inconsistent
component patterns and reduced type safety.
+<svelte:head>+ <title>Moderaty — Sign in</title>+</svelte:head>++<main class="login-main">+ <div class="card login-card">+ <h1>Sign in to Moderaty</h1>+ <p class="muted">+ Sign in with your Google account, then connect your YouTube channel to start moderating+ comments automatically.+ </p>+ <a class="btn" href="/api/auth/google/login">Sign in with Google</a>+ </div>+</main>
Relevance
●●● Strong
Trivial, low-risk consistency fix; repo strongly prefers TypeScript Svelte components and would
likely add empty TS script.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The rule requires each newly added Svelte component to include a <script lang="ts"> (or module
variant). The new login page component contains only markup/styles and lacks any `<script
lang="ts">` block.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
A newly added Svelte component is missing a `<script lang="ts">` block, violating the rule requiring TypeScript for new Svelte components.
## Issue Context
Even if no client-side logic is needed, the rule requires the TypeScript script block for new components.
## Fix Focus Areas
- src/routes/login/+page.svelte[1-51]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
View more (1) 5. Expired sessions never purged 🐞 Bug☼ Reliability
Description
Expired session rows are only deleted when that exact token is looked up, so sessions for users who
stop using the app remain in the sessions table indefinitely. Over time this can bloat the DB and
increase maintenance/backup costs.
+/**+ * Resolves a session cookie token to its user, or null when unknown or+ * expired. Expired rows are deleted lazily on read; sessions in their last 15+ * days are renewed in place (sliding expiry) and reported via `renewed` so+ * the caller can refresh the cookie.+ */+export async function getSessionUser(token: string | undefined): Promise<SessionResolution | null> {+ if (!token) return null;+ const row = await db+ .select({ session: sessions, user: { id: users.id, email: users.email, displayName: users.displayName, plan: users.plan } })+ .from(sessions)+ .innerJoin(users, eq(sessions.userId, users.id))+ .where(eq(sessions.id, token))+ .get();+ if (!row) return null;+ const expiresMs = Date.parse(row.session.expiresAt);+ if (Number.isNaN(expiresMs) || expiresMs <= Date.now()) {+ await db.delete(sessions).where(eq(sessions.id, token));+ return null;+ }+ if (expiresMs - Date.now() < RENEW_BELOW_MS) {+ const expiresAt = new Date(Date.now() + SESSION_TTL_MS).toISOString();+ await db.update(sessions).set({ expiresAt }).where(eq(sessions.id, token));+ return { user: row.user, expiresAt, renewed: true };+ }+ return { user: row.user, expiresAt: row.session.expiresAt, renewed: false };+}++/** Deletes a session (sign-out). Unknown tokens are a no-op. */+export async function destroySession(token: string): Promise<void> {+ await db.delete(sessions).where(eq(sessions.id, token));+}
Relevance
●● Moderate
Requires new cleanup strategy (cron/job) beyond current lazy-deletion design; no clear precedent
for/against purging.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The session library explicitly documents lazy deletion and only deletes by token (on lookup) or
during explicit sign-out, with no other code path that purges expired rows.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
`getSessionUser()` only deletes an expired session row when a request presents that specific token; there is no bulk cleanup path for sessions that expire while the user is inactive.
### Issue Context
This PR introduces a `sessions` table and sliding-expiry cookie sessions.
### Fix Focus Areas
- src/lib/server/session.ts[56-87]
### Suggested fix approach
Implement bounded cleanup of expired sessions, for example:
- Add a helper like `purgeExpiredSessions(nowIso)` that does `DELETE FROM sessions WHERE expires_at <= nowIso`.
- Call it opportunistically (e.g., on `createSession()`), or incorporate it into an existing scheduled pathway (e.g., the existing `/api/cron` handler) so cleanup is guaranteed even for inactive users.
- If you expect many rows, consider limiting frequency (e.g., probabilistic/once-per-hour guard) to avoid doing a full-table delete too often.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The reason will be displayed to describe this comment to others. Learn more.
4. Expired sessions never purged 🐞 Bug☼ Reliability
Expired session rows are only deleted when that exact token is looked up, so sessions for users who
stop using the app remain in the sessions table indefinitely. Over time this can bloat the DB and
increase maintenance/backup costs.
Agent Prompt
### Issue description
`getSessionUser()` only deletes an expired session row when a request presents that specific token; there is no bulk cleanup path for sessions that expire while the user is inactive.
### Issue Context
This PR introduces a `sessions` table and sliding-expiry cookie sessions.
### Fix Focus Areas
- src/lib/server/session.ts[56-87]
### Suggested fix approach
Implement bounded cleanup of expired sessions, for example:
- Add a helper like `purgeExpiredSessions(nowIso)` that does `DELETE FROM sessions WHERE expires_at <= nowIso`.
- Call it opportunistically (e.g., on `createSession()`), or incorporate it into an existing scheduled pathway (e.g., the existing `/api/cron` handler) so cleanup is guaranteed even for inactive users.
- If you expect many rows, consider limiting frequency (e.g., probabilistic/once-per-hour guard) to avoid doing a full-table delete too often.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@drizzle/0004_smiling_nextwave.sql`:
- Around line 19-36: The sessions table in the migration lacks referential
integrity and index support. Update the sessions table definition in schema.ts
to add a FOREIGN KEY constraint on the user_id column referencing users(id),
then add an index on sessions.user_id for query performance. Additionally, add
an index on sessions.expires_at if your expiry cleanup queries filter or sort by
that column. Regenerate the migration file to reflect these schema changes.
In `@EXECUTION_PLAN_YouTube_Comment_Moderator.md`:
- Around line 1887-1888: Update the Google sign-in documentation to use the
complete OAuth callback path `/api/auth/google/login/callback`, alongside
`/api/auth/google/login`, so the documented redirect URI matches the route
implemented by the callback server.
In `@src/hooks.server.ts`:
- Around line 26-40: Wrap the getSessionUser call in handle with server-side
error handling so database failures are caught and logged instead of propagating
through every request. On failure, set event.locals.user to null and skip
session-renewal logic; preserve the existing successful resolution and renewal
behavior.
In `@src/lib/server/db/schema.ts`:
- Around line 31-40: Add foreign key references from sessions.userId and
channels.userId to users.id in the sqliteTable definitions, using onDelete:
'restrict' rather than cascade. Preserve channels.userId as nullable so orphaned
pre-account channels remain reclaimable through the existing null path.
In `@src/lib/server/session.ts`:
- Around line 62-82: Add a periodic session-cleanup job alongside the existing
cron or lease mechanisms that deletes all rows from sessions whose expiresAt is
earlier than the current time, rather than relying only on getSessionUser. Reuse
the existing database access and scheduling patterns, and keep getSessionUser’s
per-token expiry handling unchanged.
In `@src/routes/`(app)/channels/[id]/queue/+page.server.ts:
- Around line 40-50: Extract the duplicated channel-ownership authorization into
one shared ownedChannel helper, retaining the canonical implementation in
src/routes/(app)/channels/[id]/queue/+page.server.ts#L40-L50 or moving it to a
shared module and importing it. Delete the local ownedChannel definition from
src/routes/(app)/channels/[id]/rules/+page.server.ts#L26-L36 and import the
shared helper; replace the inlined ownership check in
src/routes/(app)/channels/[id]/log/+page.server.ts#L21-L33 with a call to
ownedChannel, preserving the existing requireUser, scoped query, and 404
behavior.
In `@src/routes/api/auth/google/callback/`+server.ts:
- Around line 134-158: The upsert in the channels insert block has a race
condition where the separate ownership check can be bypassed by concurrent
requests. Replace the unconditional onConflictDoUpdate with a setWhere clause
that ensures updates only occur when the existing channel is owned by user.id or
is new, add .returning() to detect when no rows are updated, and throw a 409
error if the update was skipped (indicating another account now owns the
channel). Import sql from the drizzle library to construct the predicate if
needed, and remove or simplify the separate SELECT query if it is no longer
necessary for your requirements.
In `@src/routes/api/auth/google/login/callback/`+server.ts:
- Around line 39-113: Extract the shared token-exchange, JSON-parsing, and
redacted-error-logging sequence into a reusable helper function named
exchangeGoogleAuthCode in $lib/server that accepts code and redirectUri as
parameters. In src/routes/api/auth/google/login/callback/+server.ts lines
39-113, replace the entire token-exchange and error-handling block (including
the nested-ternary error-detail construction at line 83) with a call to this new
helper, keeping only the userinfo lookup and identity validation that follow. In
src/routes/api/auth/google/callback/+server.ts lines 42-131, apply the same
helper to the equivalent token-exchange section, then layer the channel-specific
refresh_token requirement and YouTube channel lookup logic on top of the
helper's result. This consolidates the duplicated validation and error handling
while allowing each route to add its own business logic.
- Around line 117-128: Update the find-or-create flow around the users lookup
and insert to catch a unique-constraint failure on users.googleSub
(users_google_sub_unique), then re-select the existing user by info.sub and
continue normally. Preserve the existing insert and orphan-channel claiming
behavior for the callback that successfully creates the user, while surfacing
unrelated database errors unchanged.
In `@src/routes/api/auth/google/login/login.test.ts`:
- Around line 43-67: The test helper functions makeCookies,
makeCookiesWithState, and callbackUrl are duplicated across test files. Create a
new shared test utility module (e.g., a common helpers file for Google auth
tests) and move these three function definitions there. In
src/routes/api/auth/google/login/login.test.ts (lines 43-67), remove the
duplicate function definitions and import them from the shared utility module
instead. In src/routes/api/auth/google/oauth.test.ts (lines 58-83), do the same:
remove the duplicate makeCookies and makeCookiesWithState definitions and import
them from the same shared utility module.
In `@src/routes/login/`+page.svelte:
- Around line 25-51: Update the login page component around the populated login
card to render loading, empty, error, and populated states, using a skeleton for
loading, the shared EmptyState for empty results, and an `.error-box` for
authentication failures. Ensure route authentication errors select the error
state instead of the populated card, and add a route or component test covering
all required states.
In `@src/routes/logout/`+page.server.ts:
- Around line 29-35: Update the default action in actions to accept locals and
call requireUser(locals) before destroySession(token), ensuring unauthenticated
POST requests cannot delete sessions or complete logout. Add an action test
covering an unauthenticated POST and verifying the session deletion call is not
reached.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
Push a commit to this branch (recommended)
Create a new PR with the fixes
ℹ️ Review info⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 39a1dcb8-5ef8-4b5b-8e5d-d342b63033a5
📥 Commits
Reviewing files that changed from the base of the PR and between 5011e7e and e4a0fca.
Is drizzle-kit issue #5782 (SQLite ON DELETE CASCADE table-rebuild data loss) fixed, and in which version?
💡 Result:
Drizzle-kit issue #5782, which concerns silent data loss in SQLite during table-rebuild migrations involving ON DELETE CASCADE foreign keys, was addressed by Pull Request #5784 [1][2]. The PR introduces a fix that prevents this silent data loss by properly managing the PRAGMA foreign_keys setting at the session level, ensuring that foreign key enforcement is reliably disabled during the table-rebuild process [2]. As of July 31, 2026, this issue is considered resolved by PR #5784. Users experiencing this behavior should ensure they are using a version of drizzle-kit that includes this fix [2]. While the exact version number containing the fix was not explicitly identified in the provided sources, the fix was merged to address the specific mechanism of silent deletion described in issue #5782 [1][2]. Users are encouraged to update to the latest available version of drizzle-kit to benefit from this improvement.
# Find package.json and check drizzle-kit version
find . -name "package.json" -type f | head -5 | xargs grep -l "drizzle"2>/dev/null ||true# Check for lock files that might have version infoif [ -f"package.json" ];then
cat package.json | grep -A 2 -B 2 "drizzle"fiif [ -f"package-lock.json" ];then
rg "\"drizzle-kit\"" package-lock.json -A 5 | head -30
fiif [ -f"yarn.lock" ];then
rg "drizzle-kit@" yarn.lock -A 3 | head -30
fiif [ -f"pnpm-lock.yaml" ];then
rg "drizzle-kit:" pnpm-lock.yaml -A 3 | head -30
fi
Repository: Bonobo791/Moderaty
Length of output: 2023
🌐 Web query:
drizzle-kit PR #5784 merged date and first release version containing the fix
💡 Result:
Drizzle-team pull request #5784, which addresses SQLite table-rebuild migrations causing silent data loss for ON DELETE CASCADE foreign keys, was created on May 19, 2026 [1]. As of July 31, 2026, the pull request remains in an open state [1]. Consequently, because the PR has not been merged, it is not included in any release version [1].
Add foreign key references for sessions.userId and channels.userId.
Neither sessions.userId (Line 33) nor channels.userId (Line 40) references users.id. Without a foreign key, the database allows a row to point at a user that no longer exists. When getSessionUser performs an innerJoin on users (src/lib/server/session.ts Line 67), an orphaned session simply stops resolving, but its row is never deleted because the lazy-delete path only runs on the expiry branch, not when a join match fails. An orphaned channels.userId similarly falls outside the userId IS NULL reclaim path documented in the inline comment, leaving a permanently stuck row.
No code in this PR deletes a users row yet, so this is not exploitable today. Add the reference now, before more features depend on the current ownership model.
Do not use onDelete: 'cascade'. Drizzle-kit issue #5782 causes SQLite table-rebuild migrations to silently delete rows in child tables that reference the rebuilt table through ON DELETE CASCADE. Pull Request #5784 addresses this issue but remains unmerged and is not available in any released version of drizzle-kit as of July 2026. Until PR #5784 is merged and released, use onDelete: 'restrict' instead, or implement cleanup in application code. This prevents data loss during unrelated schema changes to the users table.
Proposed schema change
export const sessions = sqliteTable('sessions', {
id: text('id').primaryKey(), // random 32-byte hex token; also the cookie value
- userId: text('user_id').notNull(),+ userId: text('user_id').notNull().references(() => users.id, { onDelete: 'restrict' }),
expiresAt: text('expires_at').notNull(), // ISO timestamp; sliding 30-day expiry
createdAt: text('created_at').notNull().default(sql`(strftime('%Y-%m-%dT%H:%M:%fZ', 'now'))`)
});
export const channels = sqliteTable('channels', {
id: text('id').primaryKey(), // YouTube channel ID (UC...)
- userId: text('user_id'), // owning user; null = pre-accounts orphan, claimed on first login+ userId: text('user_id').references(() => users.id, { onDelete: 'set null' }), // owning user; null = pre-accounts orphan, claimed on first login
📝 Committable suggestion
‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
exportconstsessions=sqliteTable('sessions',{
id: text('id').primaryKey(),// random 32-byte hex token; also the cookie value
userId: text('user_id').notNull(),
expiresAt: text('expires_at').notNull(),// ISO timestamp; sliding 30-day expiry
id: text('id').primaryKey(),// YouTube channel ID (UC...)
userId: text('user_id').references(()=>users.id,{onDelete: 'set null'}),// owning user; null = pre-accounts orphan, claimed on first login
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/server/db/schema.ts` around lines 31 - 40, Add foreign key references
from sessions.userId and channels.userId to users.id in the sqliteTable
definitions, using onDelete: 'restrict' rather than cascade. Preserve
channels.userId as nullable so orphaned pre-account channels remain reclaimable
through the existing null path.
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Consider a periodic sweep for stale sessions.
getSessionUser only deletes an expired row when someone presents that exact token again. A session belonging to a user who never returns after expiry stays in the sessions table indefinitely. Add a scheduled cleanup job (for example, alongside the existing cron/lease mechanisms) that deletes rows where expiresAt is in the past.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/server/session.ts` around lines 62 - 82, Add a periodic
session-cleanup job alongside the existing cron or lease mechanisms that deletes
all rows from sessions whose expiresAt is earlier than the current time, rather
than relying only on getSessionUser. Reuse the existing database access and
scheduling patterns, and keep getSessionUser’s per-token expiry handling
unchanged.
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Render the required login page states.
This page renders only the populated login card. Add a loading skeleton, an EmptyState, and an .error-box. Route authentication failures to the .error-box.
Add a route or component test that fails until each required state renders.
As per coding guidelines, “Every page must implement loading with a skeleton, an EmptyState empty view, an .error-box error view, and a populated view.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/login/`+page.svelte around lines 25 - 51, Update the login page
component around the populated login card to render loading, empty, error, and
populated states, using a skeleton for loading, the shared EmptyState for empty
results, and an `.error-box` for authentication failures. Ensure route
authentication errors select the error state instead of the populated card, and
add a route or component test covering all required states.
amazon-q — missing FK on `sessions.userId`: valid, fixed. `sessions.userId` now `REFERENCES users(id) ON DELETE CASCADE` with a `sessions_user_id_idx` index. Since migration 0004 was already applied to the prod Turso (empty tables, feature unmerged), this ships as a new migration 0005 (SQLite table-rebuild + index) — 0004 stays immutable. 0005 has been applied to prod and verified (FK + index present).
codacy — duplicated token-exchange logic in the two OAuth callbacks: valid, fixed. Extracted `exchangeGoogleCode()` into `src/lib/server/google.ts`: one implementation of the one-time-code exchange with redacted-error logging and response validation; the login and connect callbacks keep only their flow-specific parts (refresh_token requirement, userinfo vs channels lookup, redirect paths). This is also the bulk of the SonarCloud duplication.
codacy — duplicated `ownedChannel` ownership logic: valid, fixed. Now a single `src/lib/server/ownership.ts` used by the queue, rules, and audit-log servers (401 signed-out / 404 cross-owner, no existence leaks).
codacy — duplicated cookie test helpers: valid, fixed. Moved to a shared test-only module `src/lib/server/testcookies.ts` (re-exported from `testdb.ts`), used by both OAuth test suites. Note: they could not live only in `testdb.ts` — that module registers a global `$lib/server/db` mock on import, which would override `oauth.test.ts`'s hand-rolled db mock.
Incidental: `vite.config.ts` now excludes `.worktrees/**` from vitest so local parallel work in git worktrees doesn't pollute the suite.
Snyk check: the failing `security/snyk` check reports no detail accessible from here and no dependency changed in this PR (`code/snyk` and CodeAnt SCA pass clean) — watching it on this push.
Review round 2 (qodo) — all five findings valid; fixed in 54f63e5 (162/162 tests, check 0 errors, build green):
1. Orphan claim race (action required): fixed, and it was worse than a race. Any new user created while orphans existed would claim them — so a signup ahead of the owner would steal the legacy channels. Find-or-create + claim is now a single transaction, and the claim only runs when the users table was empty before the insert — strictly the first-ever user, per the documented invariant. New test proves a second distinct signup does not claim a late orphan. (The transaction also exposed a libsql :memory: gotcha — test db now uses shared-cache memory.)
2. Missing `channels.user_id` index: fixed. Schema index block + migration 0006 (`CREATE INDEX channels_user_id_idx`), applied to prod Turso (0004/0005 immutable as already deployed).
3. `oauth_state` cookie missing `secure`: fixed. `storePendingStates` now sets `secure` whenever `APP_URL` is https (stays off only for local http dev, matching the session cookie's protocol-aware behavior).
5. Expired sessions never purged: fixed. `createSession` now deletes `expires_at <= now` rows opportunistically — logins are infrequent, so this bounds buildup without a new scheduler; the cron path stays untouched. New test proves stale rows are purged.
The condition at Line 89 of src/routes/api/auth/google/callback/+server.ts has three outcomes. Only the rejection path is tested. Add a test for mocks.existingChannel = { userId: OWNER.id }, which must upsert and redirect, and a test for mocks.existingChannel = { userId: null }, which must let the signed-in user claim the orphan channel. Without them, a future change that tightens the check to existing && existing.userId !== user.id would break orphan claiming and reconnects, and the suite would stay green.
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/api/auth/google/oauth.test.ts` around lines 333 - 341, The current
test only covers the rejection case of the three-way conditional at Line 89 in
the callback server. Add two new test cases to the oauth.test.ts file to cover
the allowed reconnect scenarios: one where mocks.existingChannel has userId
equal to OWNER.id (the owner reconnecting their own channel, which should upsert
and redirect), and another where mocks.existingChannel has userId as null
(claiming an orphan channel, which should allow the signed-in user to take
ownership). These additional tests prevent future regressions if the conditional
logic is refactored.
Do not derive the secure cookie attribute from url.protocol.
secure: url.protocol === 'https:' depends on the protocol SvelteKit resolves for the request. Behind a TLS-terminating proxy that forwards plain HTTP without proper ORIGIN configuration, this evaluates to false, and the session token is sent without the Secure attribute. A network attacker can then intercept the token over an unencrypted request.
Derive the flag from deployment configuration. Import dev from $app/environment and use secure: !dev to condition on build mode rather than request protocol.
Note: The same vulnerability exists in src/hooks.server.ts:35, which affects session cookie renewal on all requests.
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/api/auth/google/login/callback/`+server.ts around lines 84 - 90,
Update the session cookie options in the callback handler to import `dev` from
`$app/environment` and set `secure` to `!dev` instead of deriving it from
`url.protocol`. Apply the same change to the session-cookie renewal logic in
`handle` within `src/hooks.server.ts`, preserving the existing cookie behavior
otherwise.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/server/google.ts`:
- Around line 70-104: In the token exchange flow, check tokenRes.ok immediately
after reading the response text and before JSON parsing. For non-OK responses,
use a small helper to safely extract non-secret error and error_description
fields for logging, fall back to a generic detail for invalid bodies, and throw
the existing userError; only parse and validate the success payload afterward,
removing the nested template literal and ternary around the current tokens error
logging.
In `@src/lib/server/testcookies.ts`:
- Around line 23-38: The delete method in makeCookies does not record deletion
calls like the set method does, which can cause logout tests to miss incorrect
cookie-clearing attributes. Add a deleteCalls array alongside setCalls to track
deletions. Update the delete method to accept an opts parameter matching the set
method's signature, then push an entry with the name and opts to deleteCalls
before removing from store. Return the deleteCalls array as a property of the
returned object so tests can verify deletion attributes.
In `@src/lib/server/testdb.ts`:
- Around line 68-69: Remove the makeCookies and makeCookiesWithState re-export
and its duplicate comment from testdb.ts. Update login.test.ts to import those
helpers from $lib/server/testcookies, matching oauth.test.ts, so testcookies.ts
is the sole import path.
- Line 84: The createTestDb function does not enable SQLite foreign-key
enforcement, so the REFERENCES and ON DELETE CASCADE constraints in the schema
are not actually enforced during tests. Add an execution of the PRAGMA
foreign_keys = ON statement in the createTestDb function before the schema
creation steps so that foreign-key constraints are properly enforced during test
execution.
---
Outside diff comments:
In `@src/routes/api/auth/google/login/callback/`+server.ts:
- Around line 84-90: Update the session cookie options in the callback handler
to import `dev` from `$app/environment` and set `secure` to `!dev` instead of
deriving it from `url.protocol`. Apply the same change to the session-cookie
renewal logic in `handle` within `src/hooks.server.ts`, preserving the existing
cookie behavior otherwise.
In `@src/routes/api/auth/google/oauth.test.ts`:
- Around line 333-341: The current test only covers the rejection case of the
three-way conditional at Line 89 in the callback server. Add two new test cases
to the oauth.test.ts file to cover the allowed reconnect scenarios: one where
mocks.existingChannel has userId equal to OWNER.id (the owner reconnecting their
own channel, which should upsert and redirect), and another where
mocks.existingChannel has userId as null (claiming an orphan channel, which
should allow the signed-in user to take ownership). These additional tests
prevent future regressions if the conditional logic is refactored.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
Push a commit to this branch (recommended)
Create a new PR with the fixes
ℹ️ Review info⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5a03a687-84ad-490d-8cac-5108f9590f95
📥 Commits
Reviewing files that changed from the base of the PR and between e4a0fca and c7cbf1a.
Round 3 (CodeRabbit full review) addressed in e2de7b9. 166/166 tests, svelte-check 0, build green.
Fixed this round:
google.ts — token-exchange now checks res.okbefore parsing the body; non-OK responses surface error/error_description detail in logs (best-effort parse, no token leakage).
Login callback — the find-or-create insert is now conflict-tolerant (onConflictDoNothing + re-select by google_sub), so a concurrent same-sub sign-in re-selects the winner instead of throwing a raw unique-violation. Orphan-claim gate (first-user-only, in-transaction) unchanged.
Connect callback — owner check folded into the write as a conditional upsert (onConflictDoUpdate + setWhere: userId IS NULL OR userId = <self>) with an empty returning → 409. Removes the SELECT-then-upsert TOCTOU window.
hooks.server.ts — session lookup wrapped in try/catch; a DB outage degrades to signed-out (public pages survive; gated routes still fail loudly via requireUser). Covered by new hooks.server.test.ts.
Logout action — now calls requireUser(locals) before destroying the session (401 when signed out). Covered by new logout.test.ts, which also asserts cookie deletion via new deleteCalls recording in testcookies.ts.
testdb.ts — PRAGMA foreign_keys = ON so tests match Turso behavior; dropped the cookie-helper re-export (login.test.ts now imports from testcookies.ts directly).
Plan doc — §5b now spells out the full login callback path /api/auth/google/login/callback.
Skipped, with reasons:
sessions.userId FK onDelete: 'cascade' vs drizzle-kit #5782 (restrict): kept cascade — it already shipped in migration 0005 and is applied to prod (migrations are immutable once applied), it matches the earlier review requirement on this PR, and sessions are ephemeral so the worst case is bounded. Same reasoning for intentionally not FK-ing channels.userId (the orphan-claim NULL path is by design).
Login page "render skeleton/EmptyState/error box": skipped — the sign-in page is a deliberately minimal static card; the I12 page-states rule targets the four data-driven app pages, and OAuth failures already fail loudly per repo rules.
Stale (already fixed in earlier rounds, no action needed): 0004 FK, session sweep, queue ownership extraction, login/callback exchange extraction, login-test cookie helpers.
drizzle/0005_*.sql "queries must target RAC_* tables" (×2) — false positive from a Codacy SQL rule configured for an Oracle RAC naming convention this project doesn't use; migration SQL is generated by drizzle-kit, already applied to prod Turso, and immutable. No action.
rules/+page.server.ts:24 unused error import — fixed in 697e3d8 (only fail is used). Verified: 166/166 tests, svelte-check 0, build green.
Note: the CodeRabbit reviews stamped 13:57/14:08 UTC ran against 54f63e5 — every item they raise (google_sub race, logout requireUser, ok-before-parse, cookie deletion recording, testdb re-export + FK pragma) is already fixed in e2de7b9, except the login-page-states item which was intentionally skipped (see previous comment).
qodo suggestions (up to e4a0fca) triaged against 697e3d8; the still-valid ones are fixed in 145f001. 170/170 tests, svelte-check 0, build green.
Stale — already fixed, verified against current code:
Critical — connect-callback ownership race: fixed in e2de7b9. The upsert is now a single atomic conditional write (onConflictDoUpdate + setWhere: userId IS NULL OR userId = <self>); an empty returning yields 409, so the check can't be bypassed by a concurrent connection.
Major — orphan claim for every new account: fixed in 54f63e5. The claim is gated on count(*) === 0 inside the transaction — only the first account ever created claims ownerless channels.
Still valid — fixed in 145f001:
Major — queue load exposed refreshTokenEnc: the load now returns { ch: { id, title } } (the only fields the page renders). Actions keep the full row via ownedChannel — the token never leaves the server in load data.
Major — rules load exposed refreshTokenEnc: same projection applied. The log page had the identical unflagged issue and got the same fix.
Bug — userinfo network failure escaped as 500: the userinfo fetch + body read are now wrapped like the token exchange; network errors/timeouts surface as the controlled, retryable 502.
Tests: queue/rules/log loads each assert the returned ch equals { id, title } with no credential key (new log/load.test.ts); login callback asserts a userinfo network throw → 502 with the state left retryable.
qodo suggestions (up to e4a0fca) triaged against 697e3d8; the still-valid ones are fixed in 145f001. 170/170 tests, svelte-check 0, build green.
Stale — already fixed, verified against current code:
Critical — connect-callback ownership race: fixed in e2de7b9. The upsert is now a single atomic conditional write (onConflictDoUpdate + setWhere: userId IS NULL OR userId = <self>); an empty returning yields 409, so the check can't be bypassed by a concurrent connection.
Major — orphan claim for every new account: fixed in 54f63e5. The claim is gated on count(*) === 0 inside the transaction — only the first account ever created claims ownerless channels.
Still valid — fixed in 145f001:
Major — queue load exposed refreshTokenEnc: the load now returns { ch: { id, title } } (the only fields the page renders). Actions keep the full row via ownedChannel — the token never leaves the server in load data.
Major — rules load exposed refreshTokenEnc: same projection applied. The log page had the identical unflagged issue and got the same fix.
Bug — userinfo network failure escaped as 500: the userinfo fetch + body read are now wrapped like the token exchange; network errors/timeouts surface as the controlled, retryable 502.
Tests: queue/rules/log loads each assert the returned ch equals { id, title } with no credential key (new log/load.test.ts); login callback asserts a userinfo network throw → 502 with the state left retryable.
Thanks for the thorough triage. The listed fixes and validation results look good. The only remaining indexed item is finding 5 (session.ts expired-session cleanup); the current createSession opportunistic purge and its test appear to address it, though the finding is still marked active.
Each handler returns fail(400, ...) before act reaches ownedChannel. A signed-out request with malformed form data therefore bypasses the required authentication path. Call requireUser(locals) before parsing the form, or move parsing after the existing ownership check in a shared handler. Add a signed-out malformed-form test.
As per coding guidelines, every form action must call requireUser(locals).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/`(app)/channels/[id]/queue/+page.server.ts around lines 88 - 109,
Ensure every form action in the actions object, including approve, reject, del,
and ban, calls requireUser(locals) before parsing or validating commentId.
Preserve the existing act calls and responses, and add coverage for a signed-out
request with malformed form data.
encrypt(tokens.refreshToken) runs on Line 91 and again on Line 97. Only one result is ever stored. Hoist the value into a constant.
The conditional upsert itself is correct: SQLite skips the DO UPDATE when setWhere is false, so .returning() yields no row and the handler answers 409.
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/api/auth/google/callback/`+server.ts around lines 85 - 103, In the
channel upsert flow, compute the encrypted refresh token once in a local
constant before the db.insert call, then reuse that constant for both the
values.refreshTokenEnc field and the onConflictDoUpdate set.refreshTokenEnc
field.
Cover the ownership predicate against a real database.
This test proves that the route maps an empty returning() result to 409, and that some setWhere predicate was passed. It cannot prove that the predicate blocks a foreign owner, because the mock decides the outcome from mocks.existingChannel. An incorrect predicate, for example eq(channels.userId, user.id) without the isNull branch, still passes here.
Add one test that uses setupTestDb with real channels rows, as src/routes/api/auth/google/login/login.test.ts does. Assert three cases: a new channel inserts, a channel owned by the caller updates, and a channel owned by another user stays unchanged and yields 409.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/api/auth/google/oauth.test.ts` around lines 333 - 344, Add a new
integration test following the pattern in
src/routes/api/auth/google/login/login.test.ts that uses setupTestDb with real
channels table rows instead of mocks to verify the ownership predicate in the
callback function actually works correctly. The test should assert three
scenarios: inserting a new channel when none exists, updating an existing
channel owned by the current user, and rejecting with 409 status when a channel
is already owned by a different user while keeping that row unchanged in the
database.
Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/hooks.server.ts`:
- Around line 28-44: Update the session lookup catch block in
src/hooks.server.ts lines 28-44 to keep the server-side logging but return the
established generic user-visible server error instead of assigning
event.locals.user = null. Update the session lookup test in
src/hooks.server.test.ts lines 40-50 to replace the successful-resolution
assertion with verification of that generic failure response.
In `@src/lib/server/google.ts`:
- Around line 75-91: In the non-OK response handling of the token request,
update the nested error-description construction inside the detail assignment to
first build the description in a separate statement, then combine it with
body.error. Preserve the existing fallback, type checks, sanitized logging, and
error response behavior in the surrounding token status check.
In `@src/lib/server/oauthState.ts`:
- Around line 58-60: Create a single validated helper function that derives the
cookie Secure attribute from env.APP_URL and throws or logs when APP_URL is
missing, replacing the two separate unvalidated computations. In
src/lib/server/oauthState.ts line 58-60, replace the (env.APP_URL ??
'').startsWith('https://') expression with a call to this helper. In
src/routes/api/auth/google/login/callback/+server.ts line 102-109, replace the
secure: url.protocol === 'https:' expression with the same helper, ensuring both
cookies derive their Secure attribute from the same validated source and
preventing silent fallbacks when APP_URL is missing.
In `@src/lib/server/session.test.ts`:
- Around line 81-91: Update the test creating a session purges already-expired
rows to also seed a non-expired session, then assert that its ID remains after
createSession(userId) while stale-token is removed. Keep the existing
expired-row assertion and use distinct session identifiers for both rows.
In `@src/routes/`(app)/channels/[id]/log/load.test.ts:
- Around line 29-38: Add authorization-regression tests for both protected
paths: in src/routes/(app)/channels/[id]/log/load.test.ts lines 29-38, add
foreign-owner 404 and signed-out 401 cases for load; in
src/routes/(app)/channels/[id]/rules/actions.test.ts lines 87-101, invoke
actions.add as a foreign and signed-out user and assert no rule is inserted. Use
the existing test setup and ensure the assertions fail if ownership or
authentication checks are removed.
In `@src/routes/api/auth/google/login/login.test.ts`:
- Around line 180-199: Remove the redundant test that begins at line 213-223
(which also tests that the first login claims the orphaned channel UC1). The
combined test "only the first-ever user claims orphaned channels" at lines
180-199 already covers this original assertion in its first section and adds the
new distinct assertion that a subsequent signup does not claim a different
orphaned channel. Keep the combined test and delete the now-redundant earlier
test to eliminate the duplication.
- Around line 146-149: Update the log assertion in the test around errorSpy to
use a dedicated matcher that directly verifies the spy’s call count is greater
than zero, so failures report the actual count instead of asserting on a derived
boolean. Leave the oauth_state cookie assertion unchanged.
In `@src/routes/login/`+page.svelte:
- Around line 25-27: The empty script block in the Svelte component contains
only a comment about the static sign-in prompt and guard redirect. Move this
comment from inside the script block into the markup as an HTML comment placed
above the main element, then delete the now-empty script block entirely.
---
Outside diff comments:
In `@src/routes/`(app)/channels/[id]/queue/+page.server.ts:
- Around line 88-109: Ensure every form action in the actions object, including
approve, reject, del, and ban, calls requireUser(locals) before parsing or
validating commentId. Preserve the existing act calls and responses, and add
coverage for a signed-out request with malformed form data.
In `@src/routes/api/auth/google/callback/`+server.ts:
- Around line 85-103: In the channel upsert flow, compute the encrypted refresh
token once in a local constant before the db.insert call, then reuse that
constant for both the values.refreshTokenEnc field and the onConflictDoUpdate
set.refreshTokenEnc field.
In `@src/routes/api/auth/google/oauth.test.ts`:
- Around line 333-344: Add a new integration test following the pattern in
src/routes/api/auth/google/login/login.test.ts that uses setupTestDb with real
channels table rows instead of mocks to verify the ownership predicate in the
callback function actually works correctly. The test should assert three
scenarios: inserting a new channel when none exists, updating an existing
channel owned by the current user, and rejecting with 409 status when a channel
is already owned by a different user while keeping that row unchanged in the
database.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
Push a commit to this branch (recommended)
Create a new PR with the fixes
ℹ️ Review info⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b0d88762-1f47-46fe-9f88-8e6c0a282294
📥 Commits
Reviewing files that changed from the base of the PR and between c7cbf1a and 145f001.
Multi-user accounts so anyone can sign up, connect their YouTube channel, and use the tool — per the approved plan.
Sign-up/sign-in: Google identity only (/api/auth/google/login, scopes openid email profile). Two-step by design: identity first, then the existing youtube.force-ssl consent at /api/auth/google (URLs unchanged, so no Google console changes).
No auth library (repo constraint stands): DIY sessions in src/lib/server/session.ts — random 32-byte token, sessions table, httpOnly moderaty_session cookie, 30-day sliding expiry. hooks.server.ts populates locals.user.
Guard:(app) layout redirects signed-out visitors to /login; every form action calls requireUser(locals) (401 backstop). New /login page, /logout action, account chip + sign-out in the app header; landing-page CTAs now go to /login.
Ownership scoping: every channel read/write is filtered by channels.userId — dashboard (channels, stats, bans), setToneLevel, rules add/remove, review-queue actions, audit log. Cross-owner always 404 (no existence leaks). Connect callback attaches userId and refuses (409) reattaching a channel owned by another account. This also resolves the earlier CodeRabbit setToneLevel auth finding properly.
Migration 0004 (additive/nullable): users (google_sub unique, plan default 'free' — future Stripe hook), sessions, channels.user_id. Orphaned pre-accounts channels are claimed by the first user ever to sign in — that's how the existing prod DB attaches to its owner.
BYOK: self-hosted uses the same code path with their own Google/OpenAI/Turso env keys — documented in README; hosted plans via Stripe noted as a future feature in the execution plan (§7). Cron unchanged.
Verify
npm run test — 160/160 (new: session lib, login flow incl. find-or-create/orphan-claim/cross-owner, scoping + 401 tests on every action)
After merge: run migration 0004 on prod (additive, no data rewrite). First Google sign-in on the existing DB claims the orphaned channels — sign in with the owner account first. Add http://localhost:5173/api/auth/google/login/callback and the prod equivalent to the Google OAuth client's authorized redirect URIs.
CodeAnt-AI Description
Add Google accounts with secure, user-owned YouTube moderation
What Changed
Users can sign in with Google, maintain a 30-day session, and sign out from the app
Signed-out visitors are sent to sign-in, while signed-in users can access only their own channels, rules, queues, audit logs, and dashboard data
Connecting a YouTube channel attaches it to the signed-in account and refuses attempts to take over another account’s channel
Existing channels are assigned to the first account that signs in; later accounts cannot claim them
Sensitive channel credentials are kept out of page data, and failed authentication or session lookups show clear retryable errors
Added database migrations and coverage for account creation, sessions, ownership checks, channel protection, logout, and authentication failures
Impact
✅ Google sign-in for multiple users ✅ No cross-account channel access ✅ Protected YouTube credentials
💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
@codeant-ai ask: Your question here
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
@codeant-ai ask: Can you suggest a safer alternative to storing this secret?
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
@codeant-ai: Your feedback here
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
@codeant-ai: Do not flag unused imports.
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
@codeant-ai: review
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Concurrent OAuth callbacks can restore an already-consumed state and break the one-time state guarantee
The OAuth state cookie is consumed using a stale read-modify-write sequence. If two callbacks run concurrently, both can read the same pending array; one response can remove a state while the other writes its stale filtered array and resurrects the already-consumed state. Because the state is not stored server-side or atomically removed, a successful OAuth transaction is not reliably one-time. Consume states with a server-side one-time record or otherwise serialize/validate the cookie update.
⚠️ Concurrent multi-tab OAuth callbacks can resurrect consumed states.
⚠️ OAuth callback replay protection is weakened for ten minutes.
⚠️ The same stale-cookie pattern also exists in login callback handling.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/routes/api/auth/google/callback/+server.ts
**Line:** 84:84
**Comment:***Security: The OAuth state cookie is consumed using a stale read-modify-write sequence. If two callbacks run concurrently, both can read the same `pending` array; one response can remove a state while the other writes its stale filtered array and resurrects the already-consumed state. Because the state is not stored server-side or atomically removed, a successful OAuth transaction is not reliably one-time. Consume states with a server-side one-time record or otherwise serialize/validate the cookie update.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Major
Race condition
Concurrent requests can perform multiple external moderation actions for the same pending comment
The ownership check and pending-comment lookup are separate reads, and the comment remains pending while the external YouTube request runs. Concurrent submissions or retries can therefore both pass ownedChannel and the pending check, issue multiple moderation actions, and then overwrite the local status and audit records. Atomically claim the pending comment before calling YouTube, or otherwise serialize actions per comment.
❌ Duplicate YouTube moderation actions can affect comments.
⚠️ Local status depends on concurrent request ordering.
⚠️ Audit log records duplicate human decisions.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/routes/(app)/channels/[id]/queue/+page.server.ts
**Line:** 44:44
**Comment:***Race Condition: The ownership check and pending-comment lookup are separate reads, and the comment remains pending while the external YouTube request runs. Concurrent submissions or retries can therefore both pass `ownedChannel` and the pending check, issue multiple moderation actions, and then overwrite the local status and audit records. Atomically claim the pending comment before calling YouTube, or otherwise serialize actions per comment.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Merged updated main (40cc810) into this branch as 5193743.
What came in: PR #26 (night-shift-ink landing redesign — new landing/* components, terminal-ink app.css, Archivo/IBM Plex Mono fonts, rewritten +page.svelte) and PR #24 (shared tonePrompt.js + tone-scoring refinements).
Conflict: exactly one — src/routes/+page.svelte. This branch's 3-line edit there (old landing CTAs → /login) was superseded: the redesigned landing already routes every CTA through LOGIN_URL = '/login' in src/lib/landing/links.ts. Resolved by taking main's rewrite; no other re-application needed.
Verification on the merged tree: 195/195 tests (including the new landing/tone suites), svelte-check 0 errors, production build green. No breakage in the (app) routes from the app.css rewrite.
@Bonobo791, you've reached your PR review limit, so we couldn't start this review.
Next review available in:27 minutes
Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).
How can I continue?
After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.
To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.
How do review limits work?
CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.
For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.
Two latest review findings triaged against 5193743; one fixed in 0961ddc. 197/197 tests, svelte-check 0, build green.
Valid — fixed: queue act() race (duplicate moderation actions). The pending check and the YouTube call were separate steps, so concurrent submissions could both moderate the same comment and write duplicate audit rows. act() now atomically claims the comment first — a conditional UPDATE ... WHERE status = 'pending' with .returning(); the concurrent loser finds zero rows and 404s. The YouTube call runs after the claim; if it fails, the claim is released (status: 'pending', decidedBy: 'none') so the action stays retryable. New tests: second act on a claimed comment 404s with no new audit; a failed YouTube call reverts to pending and the retry succeeds.
Technically correct but skipped: OAuth state cookie read-modify-write. The observation is right — two concurrent successful callbacks from the same browser can resurrect an already-consumed state. But per the execution plan (§5b), the state cookie is deliberately a CSRF binding ("bind the auth request to this browser session"), not a one-time replay token: the one-time artifact in the flow is Google's authorization code, which Google redeems exactly once (a replayed code fails the token exchange with 400 → our 502). A resurrected state can only ever be one this browser legitimately initiated, so no foreign-browser callback injection becomes possible — the CSRF property holds. Moving state server-side would add a table and plumbing to guard against a threat the code redemption already covers, and contradicts the documented cookie-based design.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
codeant-ai
Bot
added
the
size:XXL
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
What
Multi-user accounts so anyone can sign up, connect their YouTube channel, and use the tool — per the approved plan.
/api/auth/google/login, scopesopenid email profile). Two-step by design: identity first, then the existingyoutube.force-sslconsent at/api/auth/google(URLs unchanged, so no Google console changes).src/lib/server/session.ts— random 32-byte token,sessionstable, httpOnlymoderaty_sessioncookie, 30-day sliding expiry.hooks.server.tspopulateslocals.user.(app)layout redirects signed-out visitors to/login; every form action callsrequireUser(locals)(401 backstop). New/loginpage,/logoutaction, account chip + sign-out in the app header; landing-page CTAs now go to/login.channels.userId— dashboard (channels, stats, bans),setToneLevel, rules add/remove, review-queue actions, audit log. Cross-owner always 404 (no existence leaks). Connect callback attachesuserIdand refuses (409) reattaching a channel owned by another account. This also resolves the earlier CodeRabbitsetToneLevelauth finding properly.users(google_subunique,plandefault'free'— future Stripe hook),sessions,channels.user_id. Orphaned pre-accounts channels are claimed by the first user ever to sign in — that's how the existing prod DB attaches to its owner.Verify
npm run test— 160/160 (new: session lib, login flow incl. find-or-create/orphan-claim/cross-owner, scoping + 401 tests on every action)npm run check— 0 errors ·npm run build— greenEXECUTION_PLAN(§5b accounts phase + §7 Stripe future),AGENTS.md(Accounts & Sessions),README.md(accounts/BYOK)Deploy notes
After merge: run migration 0004 on prod (additive, no data rewrite). First Google sign-in on the existing DB claims the orphaned channels — sign in with the owner account first. Add
http://localhost:5173/api/auth/google/login/callbackand the prod equivalent to the Google OAuth client's authorized redirect URIs.