Skip to content

Monorepo test naren#44

Merged
ArnavBallinCode merged 34 commits into
mainfrom
monorepo-test-naren
Dec 19, 2025
Merged

Monorepo test naren#44
ArnavBallinCode merged 34 commits into
mainfrom
monorepo-test-naren

Conversation

@ArnavBallinCode
Copy link
Copy Markdown
Member

@ArnavBallinCode ArnavBallinCode commented Nov 30, 2025

Adding this as a PR so we can get an overview and reviews by copilot

Summary by Sourcery

Migrate the project from a Flask-based backend to a Netlify Functions–backed monorepo, update the frontend to consume the new serverless APIs, and add comprehensive documentation and CI/CD for Netlify deployment.

New Features:

  • Introduce a Netlify Functions serverless backend providing authentication, game session, leaderboard, achievements, and quantum measurement APIs backed by Supabase.
  • Add a dedicated React web app package under apps/web with its own TypeScript, tooling, and environment typing configuration.
  • Provide extensive project documentation, including architecture, installation, development workflow, migration notes, and game platform research content.

Enhancements:

  • Revamp the README with modern quick-start instructions, tech stack overview, API endpoints, and links to full documentation and live demo.
  • Update the frontend API and auth context logic to target Netlify Function endpoints with improved response handling and session management.
  • Refine root package metadata, scripts, and engine constraints to reflect the new monorepo structure and build flow, including shared install and build commands.
  • Enhance Netlify configuration to publish from the web app build output, register functions, configure SPA routing, and define API redirects.

Build:

  • Restructure package.json into a monorepo-style setup with root, apps/web, and netlify/functions packages, each with explicit Node/npm engine versions and install scripts.

CI:

  • Add a GitHub Actions workflow to build the web app and deploy it, along with serverless functions, to Netlify on pushes and pull requests.

Documentation:

  • Add detailed documentation files covering project architecture, migration from Flask to serverless, installation and troubleshooting, development workflow, and external educational game research, plus a curated quantum games list.

Chores:

  • Remove legacy Flask backend code, old configs, backup files, and redundant workflow definitions that are no longer used in the new architecture.

narenkarthikx and others added 10 commits November 23, 2025 18:36
- Migrated from Flask backend to Netlify Functions serverless architecture
- Restructured project as monorepo with apps/web/ and netlify/functions/
- Added 7 serverless functions for auth, game state, and achievements
- Fixed TypeScript compilation errors and update dependencies
- Created professional documentation suite (README, full docs, installation guide)
- Added version constraints and installation requirements
- Cleaned up duplicate files and organize project structure
- Configured for deployment on Netlify with environment setup
…in local sys -- always read readme whenever stuck and for easy setup
- Fix build command from 'npm run build' to 'cd apps/web && npm run build'
- Add build script to root package.json for consistency
- Resolves Netlify deployment error: Missing script: 'build'
- Update build command to install dependencies before building
- Ensures Netlify has all required packages (vite, react, three.js)
- Resolves 'vite: not found' deployment error
Added list of game from [QGames document](https://docs.google.com/document/d/1LaDsZcypeMMwwvEyprmxOzr9r3jNk4kyQ3qIGtjjoAc/edit?tab=t.0)

Work in progress
- needs brief overview (summary of game + type of game), quantum concepts covered, potential mechanics to implement, effective learning benefits, areas of improvement
@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented Nov 30, 2025

Reviewer's Guide

Migrate QPlay-Core from a Flask-based single repo to a Netlify Functions–backed monorepo: introduce a dedicated React web app under apps/web, replace Flask APIs with Netlify serverless functions using Supabase, tighten validation/CORS/error handling, and add CI + docs to support the new workflow and deployment model.

Sequence diagram for email login via Netlify Function

sequenceDiagram
  actor User
  participant WebApp as Web_App_UI
  participant Api as ApiService
  participant Fn as auth_login_function
  participant SB as Supabase_REST

  User->>WebApp: Submit_email_for_login
  WebApp->>Api: signIn(email, password)
  Api->>Fn: POST /auth-login {email}
  Fn->>Fn: validateSupabaseConfig()
  Fn->>Fn: isValidEmail(email)
  Fn->>SB: GET /rest/v1/users?email=eq.email
  SB-->>Fn: user_row_or_empty
  alt user_found
    Fn->>SB: PATCH /rest/v1/users?id=eq.user_id {last_login}
    Fn-->>Api: {success:true, user}
    Api->>Api: setAuthToken(user.id)
    Api-->>WebApp: SignInResponse{user,success}
    WebApp-->>User: Login_success
  else user_not_found
    Fn-->>Api: error USER_NOT_FOUND
    Api-->>WebApp: throw_error
    WebApp-->>User: Show_login_error
  end
Loading

Class diagram for ApiService and Netlify utilities

classDiagram
  class ApiService {
    -baseUrl : string
    -authToken : string
    +constructor(baseUrl : string)
    +setAuthToken(token : string) void
    +clearAuthToken() void
    +request(endpoint : string, options : RequestInit) Promise~any~
    +signIn(email : string, password : string) Promise~SignInResponse~
    +signUp(data : SignUpData) Promise~SignInResponse~
    +getCurrentUser() Promise~any~
    +getSpeedLeaderboard(limit : number, difficulty : string) Promise~any~
    +getScoreLeaderboard(limit : number, difficulty : string) Promise~any~
    +getMasteryLeaderboard(limit : number) Promise~any~
    +startGameSession(userId : string, difficulty : string) Promise~any~
    +saveGameProgress(sessionId : string, currentRoom : string, roomTimes : any, roomAttempts : any, roomScores : any) Promise~any~
    +completeGameSession(sessionId : string, userId : string, completionTime : number, totalScore : number, roomsCompleted : number, hintsUsed : number, difficulty : string) Promise~any~
    +logQuantumMeasurement(sessionId : string, roomId : string, measurementType : string, measurementData : any) Promise~any~
    +unlockAchievement(achievementId : string, sessionId : string, userId : string) Promise~any~
  }

  class SignInResponse {
    +access_token : string
    +token_type : string
    +expires_in : number
    +user : any
    +success : boolean
  }

  class SignUpData {
    +email : string
    +password : string
    +username : string
    +full_name : string
  }

  class AuthResponse {
    +success : boolean
    +message : string
    +user : any
    +error : string
  }

  class CorsUtils {
    +getAllowedOrigins() string[]
    +getCorsHeaders(requestOrigin : string, methods : string[]) Object
    +handleCorsPreflightRequest(event : Object, methods : string[]) Object
  }

  class SupabaseUtils {
    +validateSupabaseConfig() Object
    +validateServiceKeyConfig() Object
    +hasServiceKey() boolean
    +getSupabaseHeaders(useServiceKey : boolean) Object
    +getSupabaseUrl() string
  }

  class ValidationUtils {
    +isValidEmail(email : string) boolean
    +isValidUserId(userId : string) boolean
    +isValidCredential(credential : string) boolean
    +isValidSessionId(sessionId : string) boolean
    +isValidUsername(username : string) boolean
  }

  class ErrorUtils {
    +createErrorResponse(statusCode : number, userMessage : string, headers : Object, internalError : Error) Object
    +ErrorMessages : Object
  }

  class AuthLoginFunction {
    +handler(event : Object, context : Object) Promise~Object~
  }

  class AuthSignupFunction {
    +handler(event : Object, context : Object) Promise~Object~
  }

  class AuthGoogleFunction {
    +handler(event : Object, context : Object) Promise~Object~
    -generateUsername(email : string) string
  }

  class GameSessionFunction {
    +handler(event : Object, context : Object) Promise~Object~
  }

  class LeaderboardFunction {
    +handler(event : Object, context : Object) Promise~Object~
    -enrichLeaderboardWithUserData(leaderboardData : any[], supabaseUrl : string) Promise~any[]~
  }

  class AchievementsFunction {
    +handler(event : Object, context : Object) Promise~Object~
  }

  class QuantumMeasurementsFunction {
    +handler(event : Object, context : Object) Promise~Object~
  }

  ApiService --> SignInResponse
  ApiService --> SignUpData
  ApiService --> AuthResponse

  AuthLoginFunction ..> SupabaseUtils
  AuthLoginFunction ..> ValidationUtils
  AuthLoginFunction ..> ErrorUtils
  AuthLoginFunction ..> CorsUtils

  AuthSignupFunction ..> SupabaseUtils
  AuthSignupFunction ..> ValidationUtils
  AuthSignupFunction ..> ErrorUtils
  AuthSignupFunction ..> CorsUtils

  AuthGoogleFunction ..> SupabaseUtils
  AuthGoogleFunction ..> ValidationUtils
  AuthGoogleFunction ..> ErrorUtils
  AuthGoogleFunction ..> CorsUtils

  GameSessionFunction ..> SupabaseUtils
  GameSessionFunction ..> ValidationUtils
  GameSessionFunction ..> ErrorUtils
  GameSessionFunction ..> CorsUtils

  LeaderboardFunction ..> SupabaseUtils
  LeaderboardFunction ..> ErrorUtils
  LeaderboardFunction ..> CorsUtils

  AchievementsFunction ..> SupabaseUtils
  AchievementsFunction ..> ValidationUtils
  AchievementsFunction ..> ErrorUtils
  AchievementsFunction ..> CorsUtils

  QuantumMeasurementsFunction ..> SupabaseUtils
  QuantumMeasurementsFunction ..> ValidationUtils
  QuantumMeasurementsFunction ..> ErrorUtils
  QuantumMeasurementsFunction ..> CorsUtils
Loading

File-Level Changes

Change Details Files
Reorganize the project into a monorepo with a dedicated React web app package and updated build scripts.
  • Move frontend source into apps/web and add its own package.json, tsconfig, and vite-env typings.
  • Adjust root package.json to act as monorepo root with Netlify-oriented scripts (dev via netlify dev, monorepo build, shared install:all).
  • Add .nvmrc and engine constraints to standardize Node/npm versions across the repo.
apps/web/package.json
apps/web/tsconfig.json
apps/web/src/vite-env.d.ts
package.json
.nvmrc
Replace the Flask backend with Netlify Functions that implement auth, game session, leaderboard, achievements, and quantum measurement APIs using Supabase.
  • Add individual Netlify Functions for auth (email login, signup, Google OAuth), game-session lifecycle, leaderboard queries, achievements, and quantum-measurements.
  • Introduce shared utility modules for Supabase config, CORS, validation, and standard error responses to centralize cross-cutting concerns.
  • Define a minimal functions-specific package.json with google-auth-library and Node engine constraints.
  • Remove the legacy Python Flask backend, requirements, and deployment configs.
netlify/functions/auth-login.js
netlify/functions/auth-signup.js
netlify/functions/auth-google.js
netlify/functions/game-session.js
netlify/functions/leaderboard.js
netlify/functions/achievements.js
netlify/functions/quantum-measurements.js
netlify/functions/utils/supabase.js
netlify/functions/utils/cors.js
netlify/functions/utils/validation.js
netlify/functions/utils/errors.js
netlify/functions/package.json
backend/production_server.py
backend/requirements.txt
backend/sample_data.sql
backend/README.md
backend/.env.example
Retarget the frontend to use Netlify Function endpoints and Supabase directly instead of the Flask API.
  • Update API service to derive base URL from Netlify dev/production conventions and to call new function routes (auth-*, game-session, leaderboard, achievements, quantum-measurements).
  • Change auth flows so email sign-in/sign-up use simplified backend responses (no JWT), storing user id as a lightweight token; adjust types accordingly.
  • Update Google sign-in to post credentials to the auth-google Netlify Function with the same base URL logic.
  • Keep direct Supabase REST usage for some reads (e.g., game_sessions) but gated on env variables and with error logging.
apps/web/src/services/api.ts
apps/web/src/contexts/AuthContext.tsx
Configure Netlify build/deploy and add CI workflow for automated Netlify deployments.
  • Update netlify.toml to build apps/web, publish its dist folder, register the functions directory, and define redirects for /api/* and SPA routing.
  • Add a GitHub Actions workflow that installs monorepo dependencies, builds the web app, and deploys to Netlify using secrets.
  • Remove the old Railway-specific deployment workflow and Render config.
netlify.toml
.github/workflows/netlify-deploy.yml
.github/workflows/deploy-railway.yml
render.yaml
Modernize and expand documentation around architecture, installation, workflow, and platform strategy.
  • Rewrite README to focus on the Netlify/monorepo setup, quick start, scripts, and high-level architecture.
  • Add detailed docs for project architecture, migration rationale, installation/troubleshooting, development workflow, platform strategy, and quantum game references.
  • Add a changes summary document describing the Flask→serverless migration in depth.
README.md
Project_documentation.md
Installation_guide.md
Development_workflow.md
Game_plaforms_research.md
Changes_made.md
QGames_List.md
Clean up legacy root/front-end config now that apps/web owns the frontend build and tooling.
  • Remove old root-level TypeScript/Vite env config files now superseded by apps/web equivalents.
  • Drop temporary .bak files and test artifacts from src and repo root.
  • Ensure dist/node_modules folders are ignored where appropriate via existing .gitignore updates (implicit from new structure).
tsconfig.json
vite-env.d.ts
src/vite-env.d.ts
src/components/3d/QuantumScene.tsx.bak
src/components/MainMenu.tsx.bak
test.txt

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@netlify
Copy link
Copy Markdown

netlify Bot commented Nov 30, 2025

Deploy Preview for quantumescape ready!

Name Link
🔨 Latest commit 01ff9cc
🔍 Latest deploy log https://app.netlify.com/projects/quantumescape/deploys/6944d139ab63b1000800bab8
😎 Deploy Preview https://deploy-preview-44--quantumescape.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify
Copy link
Copy Markdown

netlify Bot commented Nov 30, 2025

Deploy Preview for quantum-escape ready!

Name Link
🔨 Latest commit 01ff9cc
🔍 Latest deploy log https://app.netlify.com/projects/quantum-escape/deploys/6944d1391d596b0008e4bf1c
😎 Deploy Preview https://deploy-preview-44--quantum-escape.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes and found some issues that need to be addressed.

Blocking issues:

  • Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries. (link)
  • Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries. (link)
  • Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries. (link)
  • Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries. (link)
  • Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries. (link)

General comments:

  • The new game-session Netlify function routes on event.path containing /game/start, /game/complete, and /game/save-progress, but the frontend now always calls /.netlify/functions/game-session without those path fragments, so none of these branches will match—consider switching the routing to use a body field or query parameter (e.g. action: 'start' | 'save-progress' | 'complete') or split these into separate functions.
  • Project_documentation.md contains large duplicated sections (e.g., the Technical Details, Troubleshooting, and Deployment sections appear twice), which will make it harder to maintain—consider deduplicating this file so each section appears only once.
  • In several Netlify functions you fall back to wildcard CORS (Access-Control-Allow-Origin: '*') and proceed even when Supabase calls fail (logging warnings only); if this is not intentional, you may want to tighten CORS and surface backend failures to the client so issues are visible rather than silently ignored.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new `game-session` Netlify function routes on `event.path` containing `/game/start`, `/game/complete`, and `/game/save-progress`, but the frontend now always calls `/.netlify/functions/game-session` without those path fragments, so none of these branches will match—consider switching the routing to use a body field or query parameter (e.g. `action: 'start' | 'save-progress' | 'complete'`) or split these into separate functions.
- `Project_documentation.md` contains large duplicated sections (e.g., the Technical Details, Troubleshooting, and Deployment sections appear twice), which will make it harder to maintain—consider deduplicating this file so each section appears only once.
- In several Netlify functions you fall back to wildcard CORS (`Access-Control-Allow-Origin: '*'`) and proceed even when Supabase calls fail (logging warnings only); if this is not intentional, you may want to tighten CORS and surface backend failures to the client so issues are visible rather than silently ignored.

## Individual Comments

### Comment 1
<location> `apps/web/src/services/api.ts:160-162` </location>
<code_context>
   }

-  // Game Session API calls - Updated to match backend endpoints
+  // Game Session API calls - Updated to match Netlify Functions
   async startGameSession(userId: string, difficulty: string = 'easy') {
-    return this.request('/api/game/start', {
+    return this.request('/game-session', {
       method: 'POST',
       body: JSON.stringify({
</code_context>

<issue_to_address>
**issue (bug_risk):** Game session API now always posts to `/game-session` but the Netlify function routes on `/game/start`, `/game/complete`, `/game/save-progress` paths, so requests won’t hit the intended branches.

These methods all POST to `/game-session`, but `netlify/functions/game-session.js` routes based on `event.path` containing `/game/start`, `/game/complete`, or `/game/save-progress`. When called as `/.netlify/functions/game-session`, none of those conditions match, so the function will return a 404.

To resolve this, either:
- Expose separate function endpoints (`/game/start`, `/game/complete`, `/game/save-progress`) and call those from the frontend, or
- Keep a single `game-session` function and route based on a request field (e.g. `action: 'start' | 'complete' | 'save-progress'`) or a query parameter instead of `path.includes(...)`.
</issue_to_address>

### Comment 2
<location> `netlify/functions/game-session.js:34-43` </location>
<code_context>
+  const path = event.path;
</code_context>

<issue_to_address>
**issue (bug_risk):** Routing inside `game-session` relies on URL substrings that will not appear when called as a Netlify function, causing all valid POSTs to fall through to the 404 case.

Because the function is deployed at `/.netlify/functions/game-session`, `event.path` will be that exact string. The current `includes('/game/start' | '/game/complete' | '/game/save-progress')` checks will therefore never match, and every POST will fall through to the 404 handler.

Please adjust the routing so it keys off something that is actually available at this endpoint (e.g. a field in the request body, a query parameter, or by splitting into separate functions), and keep it consistent with the URLs/contract defined in `ApiService`.
</issue_to_address>

### Comment 3
<location> `netlify/functions/auth-login.js:43-52` </location>
<code_context>
+  }
+
+  try {
+    const { email } = JSON.parse(event.body);
+    
+    if (!email) {
+      return {
+        statusCode: 400,
+        headers: corsHeaders,
+        body: JSON.stringify({
+          success: false,
+          error: 'Email is required'
+        })
+      };
+    }
+
+    // Find user by email
+    const response = await fetch(`${SUPABASE_URL}/rest/v1/users?email=eq.${email}`, {
+      headers: getSupabaseHeaders()
+    });
+
+    if (!response.ok) {
+      return {
+        statusCode: 500,
+        headers: corsHeaders,
+        body: JSON.stringify({
+          success: false,
+          error: 'Database connection failed'
+        })
+      };
+    }
+
+    const users = await response.json();
+    
+    if (users.length === 0) {
</code_context>

<issue_to_address>
**🚨 issue (security):** The login function treats knowledge of an email address as sufficient for authentication, which is a significant security weakness.

The `/auth-login` endpoint reads only `email` from the request body and returns the matching user; the frontend still sends `email` and `password`, but the password is ignored. As a result, anyone who knows a user’s email can authenticate as them.

If this is a temporary stub, clearly mark it as such and ensure it cannot be enabled in production. Otherwise, either implement proper authentication (e.g., Supabase Auth, magic links, or password verification) or restrict this endpoint to non-production and fail in production until real auth is in place.
</issue_to_address>

### Comment 4
<location> `netlify/functions/leaderboard.js:24-33` </location>
<code_context>
+});
+
+// Enrich leaderboard with user data
+const enrichLeaderboardWithUserData = async (leaderboardData) => {
+  const enriched = [];
+  
+  for (let i = 0; i < leaderboardData.length; i++) {
+    const entry = { ...leaderboardData[i], rank: i + 1 };
+    
+    try {
+      const userResponse = await fetch(
+        `${SUPABASE_URL}/rest/v1/users?id=eq.${entry.user_id}&select=username,full_name`,
+        { headers: getSupabaseHeaders() }
+      );
+      
+      if (userResponse.ok) {
+        const userData = await userResponse.json();
+        if (userData.length > 0) {
+          entry.username = userData[0].username;
+          entry.full_name = userData[0].full_name;
+        }
+      }
+    } catch (error) {
+      console.warn('Failed to fetch user data for entry:', entry.user_id);
+    }
+    
+    enriched.push(entry);
+  }
+  
+  return enriched;
+};
+
</code_context>

<issue_to_address>
**suggestion (performance):** User enrichment for leaderboard entries performs sequential requests, which will be slow with many entries.

`enrichLeaderboardWithUserData` awaits each `/rest/v1/users` fetch inside the loop, so N entries cause N sequential HTTP calls. Consider either batching with `Promise.all` or doing a single Supabase query using an `in` filter on `user_id` and then joining the results in memory to cut round-trips and latency.

Suggested implementation:

```javascript
const SUPABASE_URL = process.env.SUPABASE_URL;
const SUPABASE_ANON_KEY = process.env.SUPABASE_ANON_KEY;

// Enrich leaderboard with user data
const enrichLeaderboardWithUserData = async (leaderboardData) => {
  // Preserve existing behavior for empty input
  if (!leaderboardData || leaderboardData.length === 0) {
    return [];
  }

  // Collect unique user IDs from leaderboard entries
  const userIds = [
    ...new Set(
      leaderboardData
        .map((entry) => entry.user_id)
        .filter((id) => id !== null && id !== undefined)
    )
  ];

  // If no user IDs, just return leaderboard with rank added
  if (userIds.length === 0) {
    return leaderboardData.map((entry, index) => ({
      ...entry,
      rank: index + 1
    }));
  }

  // Build a single Supabase query using `in` filter
  // Example: id=in.(id1,id2,...) with proper URL encoding
  const inFilter = userIds.map((id) => encodeURIComponent(id)).join(',');
  const usersUrl = `${SUPABASE_URL}/rest/v1/users?id=in.(${inFilter})&select=id,username,full_name`;

  let usersById = {};
  try {
    const userResponse = await fetch(usersUrl, {
      headers: getSupabaseHeaders()
    });

    if (userResponse.ok) {
      const users = await userResponse.json();
      usersById = users.reduce((acc, user) => {
        acc[user.id] = {
          username: user.username,
          full_name: user.full_name
        };
        return acc;
      }, {});
    } else {
      console.warn('Failed to fetch users for leaderboard enrichment:', userResponse.statusText);
    }
  } catch (error) {
    console.warn('Error fetching users for leaderboard enrichment:', error);
  }

  // Enrich leaderboard entries with user data and rank
  return leaderboardData.map((entry, index) => {
    const user = usersById[entry.user_id];
    return {
      ...entry,
      rank: index + 1,
      ...(user
        ? {
            username: user.username,
            full_name: user.full_name
          }
        : {})
    };
  });
};

```

```javascript

```

1. Ensure that `enrichLeaderboardWithUserData` is still being called where the leaderboard is generated, but no call-site changes are required because the function signature remains the same.
2. If the `users` table uses a different primary key than `id` (for example `user_id`), adjust the `id` field in the `select` and `id=in.(...)` filter, as well as the key used in `usersById` to match the actual column name.
</issue_to_address>

### Comment 5
<location> `Project_documentation.md:164` </location>
<code_context>
+│   ├── EntanglementBridge.tsx    # Quantum entanglement concepts
+│   ├── ProbabilityBay.tsx        # Probability distributions
+│   ├── QuantumArchive.tsx        # Historical quantum experiments
+│   ├── StateChambrer.tsx         # Quantum state manipulation
+│   ├── SuperpositionTower.tsx    # Superposition principles
+│   └── TunnelingVault.tsx        # Quantum tunneling effects
</code_context>

<issue_to_address>
**issue (typo):** Typo in component name: likely should be `StateChamber.tsx`.

`StateChambrer.tsx` looks misspelled. Consider renaming to `StateChamber.tsx` (or similar) to better match the description and improve discoverability.

```suggestion
│   ├── StateChamber.tsx          # Quantum state manipulation
```
</issue_to_address>

### Comment 6
<location> `Project_documentation.md:298-307` </location>
<code_context>
+### **Port Architecture**
</code_context>

<issue_to_address>
**suggestion:** Large sections of the documentation appear duplicated in this file.

From this `### **Port Architecture**` heading onward, the document repeats earlier sections (Technical Details, Troubleshooting, Deployment, metrics, roadmap, contributing). Please remove or consolidate this duplicated block to keep the documentation concise and maintainable.

Suggested implementation:

```
└─────────────────────────────────────────────────┘

◈ Loaded function auth-google
◈ Loaded function auth-login
◈ Loaded function achievements
◈ Loaded function game-session
◈ Loaded function leaderboard
◈ Loaded function quantum-measurements
```

```

If there are other repeated sections after this snippet (e.g., Technical Details, Troubleshooting, Deployment, metrics, roadmap, contributing) that are not visible here, apply the same strategy: remove the duplicated later copy and keep only the earlier, canonical sections. Ensure the document still ends cleanly (no stray backticks or separators).
</issue_to_address>

### Comment 7
<location> `netlify/functions/achievements.js:51` </location>
<code_context>
          `${SUPABASE_URL}/rest/v1/game_sessions?id=eq.${session_id}&select=user_id`,
</code_context>

<issue_to_address>
**security (javascript.aws-lambda.security.tainted-sql-string):** Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries.

*Source: opengrep*
</issue_to_address>

### Comment 8
<location> `netlify/functions/auth-google.js:148` </location>
<code_context>
          error: `Failed to create user. Status: ${createUserResponse.status}`
</code_context>

<issue_to_address>
**security (javascript.aws-lambda.security.tainted-sql-string):** Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries.

*Source: opengrep*
</issue_to_address>

### Comment 9
<location> `netlify/functions/auth-google.js:148` </location>
<code_context>
          error: `Failed to create user. Status: ${createUserResponse.status}`
</code_context>

<issue_to_address>
**security (javascript.express.security.injection.tainted-sql-string):** Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries.

*Source: opengrep*
</issue_to_address>

### Comment 10
<location> `netlify/functions/auth-signup.js:103` </location>
<code_context>
          error: `Failed to create user. Status: ${response.status}`
</code_context>

<issue_to_address>
**security (javascript.aws-lambda.security.tainted-sql-string):** Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries.

*Source: opengrep*
</issue_to_address>

### Comment 11
<location> `netlify/functions/auth-signup.js:103` </location>
<code_context>
          error: `Failed to create user. Status: ${response.status}`
</code_context>

<issue_to_address>
**security (javascript.express.security.injection.tainted-sql-string):** Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries.

*Source: opengrep*
</issue_to_address>

### Comment 12
<location> `netlify/functions/achievements.js:6` </location>
<code_context>
const SUPABASE_URL = process.env.SUPABASE_URL;

</code_context>

<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))

```suggestion
const {SUPABASE_URL} = process.env;
```

<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.

From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>

### Comment 13
<location> `netlify/functions/achievements.js:7` </location>
<code_context>
const SUPABASE_ANON_KEY = process.env.SUPABASE_ANON_KEY;

</code_context>

<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))

```suggestion
const {SUPABASE_ANON_KEY} = process.env;
```

<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.

From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>

### Comment 14
<location> `netlify/functions/auth-google.js:8` </location>
<code_context>
const GOOGLE_CLIENT_ID = process.env.GOOGLE_CLIENT_ID;

</code_context>

<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))

```suggestion
const {GOOGLE_CLIENT_ID} = process.env;
```

<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.

From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>

### Comment 15
<location> `netlify/functions/auth-google.js:9` </location>
<code_context>
const SUPABASE_URL = process.env.SUPABASE_URL;

</code_context>

<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))

```suggestion
const {SUPABASE_URL} = process.env;
```

<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.

From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>

### Comment 16
<location> `netlify/functions/auth-google.js:10` </location>
<code_context>
const SUPABASE_ANON_KEY = process.env.SUPABASE_ANON_KEY;

</code_context>

<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))

```suggestion
const {SUPABASE_ANON_KEY} = process.env;
```

<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.

From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>

### Comment 17
<location> `netlify/functions/auth-google.js:11` </location>
<code_context>
const SUPABASE_SERVICE_KEY = process.env.SUPABASE_SERVICE_KEY;

</code_context>

<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))

```suggestion
const {SUPABASE_SERVICE_KEY} = process.env;
```

<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.

From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>

### Comment 18
<location> `netlify/functions/auth-google.js:76` </location>
<code_context>
    const email = payload.email;

</code_context>

<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))

```suggestion
    const {email} = payload;
```

<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.

From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>

### Comment 19
<location> `netlify/functions/auth-login.js:6` </location>
<code_context>
const SUPABASE_URL = process.env.SUPABASE_URL;

</code_context>

<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))

```suggestion
const {SUPABASE_URL} = process.env;
```

<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.

From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>

### Comment 20
<location> `netlify/functions/auth-login.js:7` </location>
<code_context>
const SUPABASE_ANON_KEY = process.env.SUPABASE_ANON_KEY;

</code_context>

<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))

```suggestion
const {SUPABASE_ANON_KEY} = process.env;
```

<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.

From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>

### Comment 21
<location> `netlify/functions/auth-signup.js:6` </location>
<code_context>
const SUPABASE_URL = process.env.SUPABASE_URL;

</code_context>

<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))

```suggestion
const {SUPABASE_URL} = process.env;
```

<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.

From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>

### Comment 22
<location> `netlify/functions/auth-signup.js:7` </location>
<code_context>
const SUPABASE_ANON_KEY = process.env.SUPABASE_ANON_KEY;

</code_context>

<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))

```suggestion
const {SUPABASE_ANON_KEY} = process.env;
```

<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.

From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>

### Comment 23
<location> `netlify/functions/game-session.js:6` </location>
<code_context>
const SUPABASE_URL = process.env.SUPABASE_URL;

</code_context>

<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))

```suggestion
const {SUPABASE_URL} = process.env;
```

<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.

From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>

### Comment 24
<location> `netlify/functions/game-session.js:7` </location>
<code_context>
const SUPABASE_ANON_KEY = process.env.SUPABASE_ANON_KEY;

</code_context>

<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))

```suggestion
const {SUPABASE_ANON_KEY} = process.env;
```

<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.

From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>

### Comment 25
<location> `netlify/functions/game-session.js:34` </location>
<code_context>
  const path = event.path;

</code_context>

<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))

```suggestion
  const {path} = event;
```

<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.

From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>

### Comment 26
<location> `netlify/functions/leaderboard.js:6` </location>
<code_context>
const SUPABASE_URL = process.env.SUPABASE_URL;

</code_context>

<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))

```suggestion
const {SUPABASE_URL} = process.env;
```

<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.

From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>

### Comment 27
<location> `netlify/functions/leaderboard.js:7` </location>
<code_context>
const SUPABASE_ANON_KEY = process.env.SUPABASE_ANON_KEY;

</code_context>

<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))

```suggestion
const {SUPABASE_ANON_KEY} = process.env;
```

<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.

From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>

### Comment 28
<location> `netlify/functions/leaderboard.js:84` </location>
<code_context>
  const path = event.path;

</code_context>

<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))

```suggestion
  const {path} = event;
```

<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.

From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>

### Comment 29
<location> `netlify/functions/quantum-measurements.js:6` </location>
<code_context>
const SUPABASE_URL = process.env.SUPABASE_URL;

</code_context>

<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))

```suggestion
const {SUPABASE_URL} = process.env;
```

<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.

From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>

### Comment 30
<location> `netlify/functions/quantum-measurements.js:7` </location>
<code_context>
const SUPABASE_ANON_KEY = process.env.SUPABASE_ANON_KEY;

</code_context>

<issue_to_address>
**suggestion (code-quality):** Prefer object destructuring when accessing and using properties. ([`use-object-destructuring`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/use-object-destructuring))

```suggestion
const {SUPABASE_ANON_KEY} = process.env;
```

<br/><details><summary>Explanation</summary>Object destructuring can often remove an unnecessary temporary reference, as well as making your code more succinct.

From the [Airbnb Javascript Style Guide](https://airbnb.io/javascript/#destructuring--object)
</details>
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread apps/web/src/services/api.ts
Comment thread netlify/functions/game-session.js Outdated
Comment thread netlify/functions/auth-login.js Outdated
Comment thread netlify/functions/leaderboard.js Outdated
Comment thread Project_documentation.md Outdated
Comment thread Project_documentation.md
@ArnavBallinCode ArnavBallinCode added the help wanted Extra attention is needed label Nov 30, 2025
@ArnavBallinCode
Copy link
Copy Markdown
Member Author

@copilot review

Copy link
Copy Markdown
Contributor

Copilot AI commented Nov 30, 2025

@ArnavBallinCode I've opened a new pull request, #45, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

New security issues found

Comment thread netlify/functions/achievements.js Outdated
Comment thread netlify/functions/auth-google.js Outdated
Comment thread netlify/functions/auth-google.js Outdated
Comment thread netlify/functions/auth-signup.js Outdated
Comment thread netlify/functions/auth-signup.js Outdated
@ArnavBallinCode
Copy link
Copy Markdown
Member Author

@copilot review

Copy link
Copy Markdown
Contributor

Copilot AI commented Nov 30, 2025

@ArnavBallinCode I've opened a new pull request, #46, to work on those changes. Once the pull request is ready, I'll request review from you.

@ArnavBallinCode
Copy link
Copy Markdown
Member Author

@copilot implement above suggestions of sourcery and code review

Copy link
Copy Markdown
Contributor

Copilot AI commented Nov 30, 2025

@ArnavBallinCode I've opened a new pull request, #50, to work on those changes. Once the pull request is ready, I'll request review from you.

narenkarthikx and others added 4 commits December 6, 2025 18:01
Security hardening for Netlify serverless functions
…time

[WIP] Migrate project to Netlify serverless monorepo
Code review for monorepo migration from Flask to Netlify serverless
- Fix game-session.js: Changed from path-based to mode-based routing
  (frontend sends mode: 'start'|'complete'|'save-progress' in body)
- Fix CORS: Added quantum-escape.netlify.app to allowed origins
- Fix api.ts: Added mode parameter to game session API calls
- Fix netlify.toml: Added CORS headers and esbuild bundler config
- Add netlify-deploy.yml: New GitHub Actions workflow for deployment
- Remove deploy-railway.yml: Empty/unused workflow file
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

New security issues found

Comment thread netlify/functions/achievements.js Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 39 out of 111 changed files in this pull request and generated no new comments.

Files not reviewed (1)
  • netlify/functions/package-lock.json: Language not supported
Comments suppressed due to low confidence (4)

netlify/functions/auth-google.js:1

  • The username generation combines timestamp and random values, but there's a potential race condition if two users sign up at the exact same millisecond. While the random component reduces this risk, consider using a cryptographically secure random generator (crypto.randomBytes) or UUID for better uniqueness guarantees, especially since usernames are checked in a while loop that could theoretically run indefinitely.
    netlify/functions/leaderboard.js:1
  • While user IDs are being encoded individually, the Supabase query uses in.() syntax with comma-separated values. Ensure that Supabase properly handles these encoded values in the in operator to prevent potential injection issues. Consider using Supabase's native array parameter binding if available instead of string concatenation.
    netlify/functions/utils/cors.js:1
  • When the origin is not in the allowed list, the function returns CORS headers without 'Access-Control-Allow-Origin', which will cause browsers to block the request. However, this silently fails without logging or explicit error response. Consider logging unauthorized origin attempts for security monitoring and debugging purposes.
    netlify/functions/game-session.js:1
  • The user_id is validated with isValidUserId before this point, but the validation allows alphanumeric, hyphens, and underscores up to 128 characters. Ensure that encodeURIComponent provides sufficient protection for all valid user_id formats in the Supabase query context. Consider using parameterized queries if Supabase client library supports them instead of URL string interpolation.

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

Security Fixes:
- Add validateServiceKeyConfig() to validate SERVICE_KEY before usage
- Update auth-google.js to use validateServiceKeyConfig (fixes bug_risk issue)
- Add hasServiceKey() helper for optional service key checks

Documentation:
- Update .env.example with placeholder values (remove real credentials)
- Add helpful comments explaining where to get each env variable
- Update README with Netlify Functions API reference table
- Add detailed project structure showing all 7 serverless functions
- Update live demo link to quantum-escape.netlify.app
- Add explicit security comments explaining validation
- Extract validated session_id to separate variable
- Clarify that isValidSessionId only allows [a-zA-Z0-9_-]
- Note: This is a REST API URL param, not SQL injection risk
@ArnavBallinCode
Copy link
Copy Markdown
Member Author

@sourcery-ai review

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes and found some issues that need to be addressed.

Blocking issues:

  • Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries. (link)

General comments:

  • The root install:all script uses cd apps/web && npm install && cd ../../netlify/functions && npm install, which will cd out of the repo; from apps/web it should be something like cd ../netlify/functions instead to correctly reach the functions folder.
  • The frontend ApiService.signIn still sends { email, password } to /auth-login, but the auth-login Netlify function only expects/uses email, so you may want to either remove the password path on the client or update the function to enforce the same credential model to avoid confusing or insecure behavior.
  • Similarly, the signup flow on the frontend appears to use a richer SignUpData shape (likely including password), while auth-signup only consumes email, username, and full_name; aligning the request/response contract between client and function will make the API boundary clearer and prevent unused fields from being sent.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The root `install:all` script uses `cd apps/web && npm install && cd ../../netlify/functions && npm install`, which will cd out of the repo; from `apps/web` it should be something like `cd ../netlify/functions` instead to correctly reach the functions folder.
- The frontend `ApiService.signIn` still sends `{ email, password }` to `/auth-login`, but the `auth-login` Netlify function only expects/uses `email`, so you may want to either remove the password path on the client or update the function to enforce the same credential model to avoid confusing or insecure behavior.
- Similarly, the signup flow on the frontend appears to use a richer `SignUpData` shape (likely including password), while `auth-signup` only consumes `email`, `username`, and `full_name`; aligning the request/response contract between client and function will make the API boundary clearer and prevent unused fields from being sent.

## Individual Comments

### Comment 1
<location> `apps/web/src/services/api.ts:74-81` </location>
<code_context>

-  // Authentication API calls (Supabase-powered)
+  // Authentication API calls (Netlify Functions)
   async signIn(email: string, password: string): Promise<SignInResponse> {
-    const response = await this.request('/api/auth/signin', {
+    const response = await this.request('/auth-login', {
       method: 'POST',
       body: JSON.stringify({ email, password })
     });
-    
+
     // Store token automatically
     if (response.access_token) {
       this.setAuthToken(response.access_token);
     }
</code_context>

<issue_to_address>
**issue (bug_risk):** Login API now calls /auth-login, which doesn’t appear to return an access_token, so the client’s token handling and SignInResponse typing are likely inconsistent.

`signIn` still assumes a `SignInResponse` with `access_token` and calls `setAuthToken`, but `/auth-login` only returns `{ success, user }`. This makes `response.access_token` undefined and will break any flows that depend on a stored token.

Please either update `/auth-login` to return a token, or adjust `SignInResponse`, `signIn`/`setAuthToken`, and downstream consumers to use the new session/user-based model without tokens.
</issue_to_address>

### Comment 2
<location> `apps/web/src/services/api.ts:88-81` </location>
<code_context>
     return response;
   }

   async signUp(data: SignUpData): Promise<SignInResponse> {
-    const response = await this.request('/api/auth/signup', {
+    const response = await this.request('/auth-signup', {
       method: 'POST',
       body: JSON.stringify(data)
     });
-    
+
     // Store token automatically
     if (response.access_token) {
       this.setAuthToken(response.access_token);
     }
</code_context>

<issue_to_address>
**issue (bug_risk):** Signup flow expects a token-bearing SignInResponse, but the auth-signup Netlify function returns a user object without access_token.

`signUp` still uses `SignInResponse` and token handling, but `/auth-signup` returns `{ success, message, user }` with no `access_token`, so the typing is wrong and `setAuthToken` never runs.

Please either:
- Update `/auth-signup` to return an `access_token` so it matches `SignInResponse`, or
- Update `SignInResponse`/`signUp` to the new shape and remove token-specific logic if signup is now tokenless.
</issue_to_address>

### Comment 3
<location> `netlify/functions/leaderboard.js:20-27` </location>
<code_context>
+
+  try {
+    // Batch fetch all user data in a single query
+    const userIds = leaderboardData.map(entry => entry.user_id).filter(Boolean);
+    if (userIds.length === 0) {
+      return leaderboardData.map((entry, index) => ({ ...entry, rank: index + 1 }));
+    }
+
+    // Encode each user ID individually before joining
+    const userIdsParam = userIds.map(id => encodeURIComponent(id)).join(',');
+    const userResponse = await fetch(
+      `${SUPABASE_URL}/rest/v1/users?id=in.(${userIdsParam})&select=id,username,full_name`,
</code_context>

<issue_to_address>
**suggestion (performance):** Consider de-duplicating user IDs before building the Supabase in() filter to avoid unnecessarily large URLs and redundant work.

Because `userIdsParam` includes duplicates from `leaderboardData`, large leaderboards can generate bloated query strings and repeated lookups. Consider de-duplicating first, e.g. `const ids = [...new Set(userIds)]`, and using `ids` when building the `id=in.(...)` filter to reduce URL size and DB/network overhead.

```suggestion
    // Batch fetch all user data in a single query
    const userIds = leaderboardData.map(entry => entry.user_id).filter(Boolean);

    // De-duplicate user IDs to avoid bloated query strings and redundant lookups
    const uniqueUserIds = [...new Set(userIds)];
    if (uniqueUserIds.length === 0) {
      return leaderboardData.map((entry, index) => ({ ...entry, rank: index + 1 }));
    }

    // Encode each unique user ID individually before joining
    const userIdsParam = uniqueUserIds.map(id => encodeURIComponent(id)).join(',');
```
</issue_to_address>

### Comment 4
<location> `package.json:13` </location>
<code_context>
-    "three": "0.151.3"
+  "scripts": {
+    "dev": "netlify dev",
+    "build": "npm run install:all && cd apps/web && npm run build",
+    "preview": "cd apps/web && npm run preview",
+    "install:all": "cd apps/web && npm install && cd ../../netlify/functions && npm install"
</code_context>

<issue_to_address>
**suggestion (performance):** Running npm install from the build script will slow builds and can conflict with CI pipelines that already install dependencies.

Here `build` runs `install:all`, which calls `npm install` in both `apps/web` and `netlify/functions`. In most CI setups, dependencies are already installed once (e.g., via `npm ci`), so this duplicates work and may mutate `package-lock.json` during the build. Prefer keeping installs in CI/setup scripts and having `build` only run the actual build (e.g., `cd apps/web && npm run build`).
</issue_to_address>

### Comment 5
<location> `Project_documentation.md:298` </location>
<code_context>
+◈ Loaded function quantum-measurements
+```
+
+### **Port Architecture**
+- **Port 8888**: Main development server (your access point)
+- **Port 3999**: Internal static file server (background)
</code_context>

<issue_to_address>
**issue:** Large sections from "Port Architecture" onward appear duplicated at the end of the file.

From this second "### **Port Architecture**" onward, the content (Port Architecture, Technical Details, Troubleshooting, Deployment, Project Metrics, Next Steps, Additional Resources, Contributing, and the closing section) duplicates material earlier in the document. Please remove the duplicate block or consolidate so each section appears only once.
</issue_to_address>

### Comment 6
<location> `netlify/functions/achievements.js:52` </location>
<code_context>
          `${SUPABASE_URL}/rest/v1/game_sessions?id=eq.${validatedSessionId}&select=user_id`,
</code_context>

<issue_to_address>
**security (javascript.aws-lambda.security.tainted-sql-string):** Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries.

*Source: opengrep*
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread apps/web/src/services/api.ts Outdated
Comment thread netlify/functions/leaderboard.js Outdated
Comment thread package.json
ArnavBallinCode and others added 2 commits December 19, 2025 09:25
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
- Add AuthResponse interface matching backend response
- Make SignInResponse.access_token optional
- Update signIn/signUp to use user.id as session identifier
- Backend returns { success, user } without JWT tokens

Addresses Sourcery AI bug_risk issue about undefined access_token
@ArnavBallinCode
Copy link
Copy Markdown
Member Author

@sourcery-ai review

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes and found some issues that need to be addressed.

Blocking issues:

  • Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries. (link)

General comments:

  • The new auth-login/auth-signup flow only uses email (ignoring the password the frontend still sends) and never provisions real Supabase auth credentials, which effectively turns login into a passwordless “find user by email” lookup; consider either enforcing proper password-based auth or explicitly implementing a passwordless flow and aligning the frontend UX and payloads accordingly.
  • The base URL resolution logic for Netlify Functions is duplicated between the API service (apps/web/src/services/api.ts) and the Google auth flow in AuthContext; consider centralizing this into a single helper so changes to dev/prod endpoints only need to be made in one place.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new auth-login/auth-signup flow only uses email (ignoring the password the frontend still sends) and never provisions real Supabase auth credentials, which effectively turns login into a passwordless “find user by email” lookup; consider either enforcing proper password-based auth or explicitly implementing a passwordless flow and aligning the frontend UX and payloads accordingly.
- The base URL resolution logic for Netlify Functions is duplicated between the API service (apps/web/src/services/api.ts) and the Google auth flow in AuthContext; consider centralizing this into a single helper so changes to dev/prod endpoints only need to be made in one place.

## Individual Comments

### Comment 1
<location> `netlify/functions/auth-login.js:11-20` </location>
<code_context>
+1. **Created Netlify Functions**
+   ```javascript
+   // Example: auth-login.js
+   exports.handler = async (event, context) => {
+     // Serverless authentication logic
+     // Replaced Flask route handling
</code_context>

<issue_to_address>
**🚨 issue (security):** Login currently authenticates users by email only without any password or token verification.

This endpoint effectively grants access to anyone who knows a valid email, since it only checks format and returns the first matching user with no credential verification. If it’s meant to be called only after a separate, trusted auth step (e.g., magic link), it should be clearly named, documented, and access‑restricted to that context. Otherwise, it must require and validate a credential (password, signed token, etc.) before treating the request as a successful login.
</issue_to_address>

### Comment 2
<location> `netlify/functions/auth-signup.js:69-71` </location>
<code_context>
+    };
+
+    // Create user
+    const response = await fetch(`${SUPABASE_URL}/rest/v1/users`, {
+      method: 'POST',
+      headers: getSupabaseHeaders(),
+      body: JSON.stringify(userData)
+    });
</code_context>

<issue_to_address>
**🚨 issue (security):** User creation uses the anon key instead of the service key, which is risky with typical Supabase RLS settings.

This backend-only call uses `getSupabaseHeaders()` (anon key) to write to `users`. In most Supabase setups, `users` writes should require the service key to avoid clients forging arbitrary rows. Please switch to `getSupabaseHeaders(true)` and ensure `validateServiceKeyConfig()` (as in `auth-google`) is used, or otherwise guarantee via RLS that only trusted, system-level contexts can insert into this table.
</issue_to_address>

### Comment 3
<location> `netlify/functions/game-session.js:64-73` </location>
<code_context>
       });
-      
+
       if (response.ok) {
         return await response.json();
       } else {
</code_context>

<issue_to_address>
**suggestion (bug_risk):** Starting a game session silently falls back to a "demo" session while still returning success on Supabase write errors.

On Supabase insert failure, we still fabricate a `demo-session-*` session and return `success: true`, making it impossible for callers to know the session isn’t persisted and risking failures when resuming/updating later. Consider either returning `success: false` with an explicit error, or adding a clear `source`/`isDemo` flag so consumers can handle non-persisted sessions differently.

Suggested implementation:

```javascript
      const response = await fetch(`${SUPABASE_URL}/rest/v1/game_sessions`, {
        method: 'POST',
        headers: getSupabaseHeaders(),
        body: JSON.stringify(gameSession)
      });

      if (response.ok) {
        const sessions = await response.json();

        // If Supabase didn't actually persist a session row, treat this as a demo session
        const isDemo = !Array.isArray(sessions) || sessions.length === 0;
        const session = isDemo
          ? { ...gameSession, id: `demo-session-${Date.now()}`, isDemo: true }
          : { ...sessions[0], isDemo: false };

        return {
          statusCode: 200,
          headers: corsHeaders,
          body: JSON.stringify({
            success: !isDemo, // only report success when the session is persisted
            session,
            isDemo
          })
        };
      } else {
        // On Supabase write failure, do not silently fall back to a demo session
        const errorText = await response.text().catch(() => null);

        return {
          statusCode: 500,
          headers: corsHeaders,
          body: JSON.stringify({
            success: false,
            error: 'Failed to create game session in Supabase',
            details: errorText || null
          })
        };

```

If there are client-side callers that currently assume `success: true` and always-present `session`, they may need to be updated to:
1. Check `success` before using `session`.
2. Handle the `isDemo` flag when `success` is `false` or when they want to treat non-persisted sessions differently.
If you prefer to always return a session (even on Supabase failure) while still signaling non-persistence, you can instead keep a demo session in the `else` branch but set `success: false` and `isDemo: true`.
</issue_to_address>

### Comment 4
<location> `netlify.toml:13-19` </location>
<code_context>
+  directory = "netlify/functions"
+  node_bundler = "esbuild"
+
+# CORS headers for API functions
+[[headers]]
+  for = "/.netlify/functions/*"
+  [headers.values]
+    Access-Control-Allow-Origin = "*"
+    Access-Control-Allow-Methods = "GET, POST, PUT, PATCH, DELETE, OPTIONS"
+    Access-Control-Allow-Headers = "Content-Type, Authorization"
+
+# API redirects to Netlify Functions
</code_context>

<issue_to_address>
**🚨 issue (security):** Netlify-wide CORS header wildcard may conflict with per-function CORS logic that restricts origins.

Function-level CORS (`getCorsHeaders`) only sets `Access-Control-Allow-Origin` for allowed origins; otherwise it omits the header. This global `[[headers]]` rule then unconditionally adds `Access-Control-Allow-Origin = "*"` for all functions, which can negate the origin restrictions in your function code, depending on how Netlify merges headers. Please either remove or tighten this global rule, or align it with the same allowed-origins policy used in the functions.
</issue_to_address>

### Comment 5
<location> `package.json:10-13` </location>
<code_context>
+    "node": ">=18.0.0",
+    "npm": ">=9.0.0"
+  },
+  "scripts": {
+    "dev": "vite",
+    "build": "vite build",
</code_context>

<issue_to_address>
**suggestion (performance):** Running `npm install` as part of the build pipeline can significantly slow builds and complicate caching.

The `build` script calls `npm run install:all`, which re-installs dependencies in `apps/web` and `netlify/functions` even though CI/local runs `npm install` before `npm run build`. This breaks dependency caching and slows builds. Please move these installs to dedicated setup steps (e.g., CI workflow, onboarding scripts) or adopt a workspace/monorepo setup so a single root `npm install` is sufficient.

Suggested implementation:

```
  "scripts": {
    "dev": "vite",
    "build": "vite build",

```

To fully implement your suggestion, you should:
1. Ensure your CI pipeline and any local onboarding scripts explicitly run dependency installation where needed (e.g., `npm install` at the root and in `apps/web` and `netlify/functions` if you keep them separate).
2. Optionally, if `install:all` is no longer used anywhere, remove the `install:all` script from `package.json` to avoid confusion.
3. Longer term, consider configuring npm/yarn/pnpm workspaces so a single root `npm install` handles all subprojects, simplifying your CI setup and caching.
</issue_to_address>

### Comment 6
<location> `apps/web/src/services/api.ts:85-86` </location>
<code_context>
-  // Authentication API calls (Supabase-powered)
+  // Authentication API calls (Netlify Functions)
+  // Note: Current backend uses user-based sessions without JWT tokens
   async signIn(email: string, password: string): Promise<SignInResponse> {
-    const response = await this.request('/api/auth/signin', {
+    const response = await this.request('/auth-login', {
       method: 'POST',
       body: JSON.stringify({ email, password })
</code_context>

<issue_to_address>
**🚨 question (security):** The frontend still passes a password to `auth-login`, but the backend ignores it entirely.

`signIn` still posts `{ email, password }`, but `/auth-login` only reads `email` and does no password check. This makes the API contract misleading and obscures that login is effectively email-only. Once the intended auth flow (e.g., magic-link vs password-based) is confirmed, please either stop sending `password` or add proper password verification and update the types accordingly.
</issue_to_address>

### Comment 7
<location> `Project_documentation.md:298-307` </location>
<code_context>
+### **Port Architecture**
</code_context>

<issue_to_address>
**issue:** Large section appears duplicated in the documentation.

From this second `### **Port Architecture**` heading down to the footer, this section duplicates an earlier part of the file. Please remove or consolidate it to avoid confusion and keep the docs concise.
</issue_to_address>

### Comment 8
<location> `netlify/functions/achievements.js:52` </location>
<code_context>
          `${SUPABASE_URL}/rest/v1/game_sessions?id=eq.${validatedSessionId}&select=user_id`,
</code_context>

<issue_to_address>
**security (javascript.aws-lambda.security.tainted-sql-string):** Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries.

*Source: opengrep*
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread netlify/functions/auth-login.js
Comment thread netlify/functions/auth-signup.js Outdated
Comment thread netlify.toml Outdated
Comment thread package.json
Comment thread apps/web/src/services/api.ts Outdated
Security Fixes:
- auth-signup.js: Use validateServiceKeyConfig and getSupabaseHeaders(true)
  (required for writing to users table)
- netlify.toml: Remove CORS wildcard headers (functions handle CORS with origin validation)
- auth-login.js: Add security documentation explaining simplified email-only auth
- api.ts: Make password parameter optional, don't send unused data to backend

Note: auth-login uses simplified email-lookup for demo purposes.
Production should use Supabase Auth, OAuth, or custom password hashing.
@ArnavBallinCode
Copy link
Copy Markdown
Member Author

@sourcery-ai review

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes and found some issues that need to be addressed.

Blocking issues:

  • Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries. (link)

General comments:

  • The root install:all script (cd apps/web && npm install && cd ../../netlify/functions && npm install) resolves to a directory above the repo when run from apps/web (ending up at ../netlify/functions from the parent of the repo); consider correcting the path (e.g. cd apps/web && npm install && cd ../.. && cd netlify/functions && npm install) so installs run in the intended functions folder.
  • In AuthContext and the ApiService you now have duplicated logic for computing the Netlify Functions base URL from VITE_BACKEND_URL/import.meta.env.MODE; consider centralizing this URL construction in a single helper to avoid drift between the two call sites.
  • The new auth flow stores user.id as the "auth token" via setAuthToken, which may be misleading and potentially unsafe if used as a bearer token later; consider either renaming this to make it clear it’s just a client-side session identifier or introducing a more robust session/token mechanism.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The root `install:all` script (`cd apps/web && npm install && cd ../../netlify/functions && npm install`) resolves to a directory above the repo when run from `apps/web` (ending up at `../netlify/functions` from the parent of the repo); consider correcting the path (e.g. `cd apps/web && npm install && cd ../.. && cd netlify/functions && npm install`) so installs run in the intended functions folder.
- In `AuthContext` and the `ApiService` you now have duplicated logic for computing the Netlify Functions base URL from `VITE_BACKEND_URL`/`import.meta.env.MODE`; consider centralizing this URL construction in a single helper to avoid drift between the two call sites.
- The new auth flow stores `user.id` as the "auth token" via `setAuthToken`, which may be misleading and potentially unsafe if used as a bearer token later; consider either renaming this to make it clear it’s just a client-side session identifier or introducing a more robust session/token mechanism.

## Individual Comments

### Comment 1
<location> `apps/web/src/services/api.ts:93-94` </location>
<code_context>
+
+    // Backend returns { success, user } without access_token
+    // Store user ID as simple session identifier if needed
+    if (response.success && response.user?.id) {
+      this.setAuthToken(response.user.id);
     }
-    
</code_context>

<issue_to_address>
**🚨 issue (security):** Using the raw user ID as an auth token is weak and conflates identity with authentication.

Previously we stored a backend-issued access token; now we pass `response.user.id` to `setAuthToken`, effectively using a public identifier as a credential and making user impersonation trivial.

If JWTs are being removed, the backend should still issue a session-specific opaque token (e.g., random UUID or Supabase session ID) that can be revoked/rotated, rather than using the user ID. It may also be worth renaming this to something like `setSessionToken` to make clear it’s not an auth credential.
</issue_to_address>

### Comment 2
<location> `netlify/functions/auth-login.js:11-20` </location>
<code_context>
+1. **Created Netlify Functions**
+   ```javascript
+   // Example: auth-login.js
+   exports.handler = async (event, context) => {
+     // Serverless authentication logic
+     // Replaced Flask route handling
</code_context>

<issue_to_address>
**🚨 issue (security):** Login endpoint ignores password and authenticates purely by email, which is a serious security flaw.

The handler currently authenticates users solely by matching `email` from the request body and creating a session, so anyone who knows a valid email can log in as that user.

If this is meant to be passwordless, the flow must be explicit (magic link, one-time code, OAuth, etc.) and the UI should not ask for a password. Otherwise, add proper password (or other secret) verification against a secure hash and reject invalid attempts. In practice, either:
- Restore secure password verification, or
- Redesign and rename this endpoint and UI as a passwordless flow that issues a magic link/one-time token instead of logging in directly.
</issue_to_address>

### Comment 3
<location> `netlify/functions/auth-signup.js:34-43` </location>
<code_context>
+  const SUPABASE_URL = getSupabaseUrl();
+
+  try {
+    const { email, username, full_name } = JSON.parse(event.body);
+    
+    // Validate email format
+    if (!isValidEmail(email)) {
+      return createErrorResponse(400, ErrorMessages.INVALID_EMAIL, corsHeaders);
+    }
+
+    // Check if user already exists
+    const checkResponse = await fetch(`${SUPABASE_URL}/rest/v1/users?email=eq.${encodeURIComponent(email)}`, {
+      headers: getSupabaseHeaders()
+    });
+
+    if (checkResponse.ok) {
+      const existingUsers = await checkResponse.json();
+      if (existingUsers.length > 0) {
+        return createErrorResponse(400, ErrorMessages.USER_EXISTS, corsHeaders);
+      }
+    }
+
+    const userData = {
+      email: email,
+      username: username || email.split('@')[0],
</code_context>

<issue_to_address>
**🚨 issue (security):** Signup endpoint ignores password and creates accounts without any credential, which conflicts with the previous auth model.

The handler only reads `email`, `username`, and `full_name` and never accepts or stores any credential, while `auth-login` authenticates purely by email. Together, this effectively removes real authentication.

If passwordless is intentional, update the API/UI to remove password fields and implement an explicit passwordless flow (e.g., magic links). If not, this endpoint should accept and persist a password hash (or delegate to Supabase Auth) and be aligned with how `auth-login` verifies users.
</issue_to_address>

### Comment 4
<location> `netlify.toml:9-25` </location>
<code_context>
+  node_bundler = "esbuild"
+
+# CORS headers for API functions
+[[headers]]
+  for = "/.netlify/functions/*"
+  [headers.values]
+    Access-Control-Allow-Origin = "*"
+    Access-Control-Allow-Methods = "GET, POST, PUT, PATCH, DELETE, OPTIONS"
+    Access-Control-Allow-Headers = "Content-Type, Authorization"
+
+# API redirects to Netlify Functions
</code_context>

<issue_to_address>
**🚨 suggestion (security):** Global `Access-Control-Allow-Origin: *` header can conflict with per-function CORS logic and credentials settings.

Netlify is setting a wildcard `Access-Control-Allow-Origin` for `/.netlify/functions/*`, while each function also sets its own CORS headers (including `Access-Control-Allow-Credentials: true` and specific origins). Depending on how Netlify merges headers, this can cause duplicate `Access-Control-Allow-Origin` values or `*` combined with `Allow-Credentials`, which browsers will reject.

To avoid this, use a single CORS strategy:
- Preferably remove or narrow the Netlify `[[headers]]` block for functions and let function code handle CORS, or
- Make the function-level CORS logic match the global `*` policy and omit `Allow-Credentials` in that case.

```suggestion
[functions]
  directory = "netlify/functions"
  node_bundler = "esbuild"

# NOTE: CORS for API functions is handled within each Netlify Function.
# Avoid setting global wildcard CORS headers here to prevent conflicts
# with per-function CORS (especially when using credentials and
# function-specific origins).

# API redirects to Netlify Functions
[[redirects]]
  from = "/api/*"
  to = "/.netlify/functions/:splat"
  status = 200
```
</issue_to_address>

### Comment 5
<location> `Project_documentation.md:298` </location>
<code_context>
+◈ Loaded function quantum-measurements
+```
+
+### **Port Architecture**
+- **Port 8888**: Main development server (your access point)
+- **Port 3999**: Internal static file server (background)
</code_context>

<issue_to_address>
**issue:** The entire Port Architecture and subsequent sections appear duplicated at the end of the document.

From `### **Port Architecture**` to the footer, the document repeats sections that already appear earlier (Port Architecture, Technical Details, Troubleshooting, Deployment, Project Metrics, Next Steps, Additional Resources, Contributing). Please remove the duplicated block or consolidate so each section appears only once.
</issue_to_address>

### Comment 6
<location> `Installation_guide.md:53` </location>
<code_context>
+2. Installs backend deps in `netlify/functions/` (~10MB)
+3. Total: ~350MB of dependencies
+
+### **4. Setup Environment**
+```powershell
+# Copy example environment file
</code_context>

<issue_to_address>
**nitpick (typo):** Use "Set Up" (verb) instead of "Setup" (noun) in this instructional heading.

As this is an action step, please update the heading to `### **4. Set Up Environment**` to use the correct verb form.

```suggestion
### **4. Set Up Environment**
```
</issue_to_address>

### Comment 7
<location> `netlify/functions/achievements.js:52` </location>
<code_context>
          `${SUPABASE_URL}/rest/v1/game_sessions?id=eq.${validatedSessionId}&select=user_id`,
</code_context>

<issue_to_address>
**security (javascript.aws-lambda.security.tainted-sql-string):** Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries.

*Source: opengrep*
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread Installation_guide.md Outdated
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
@ArnavBallinCode
Copy link
Copy Markdown
Member Author

@sourcery-ai review

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes and found some issues that need to be addressed.

Blocking issues:

  • Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries. (link)

General comments:

  • The email-only auth-login function is explicitly marked as demo/auth-lite but is always enabled; consider gating this behavior behind an environment flag (e.g. DEMO_AUTH_ENABLED) or restricting it to non-production environments to avoid it being accidentally used as the primary auth mechanism in production.
  • In ApiService.signIn, the code now stores response.user.id via setAuthToken while still presenting a SignInResponse shape that used to carry JWTs; it may be worth renaming this token or separating a proper sessionId/userId from auth tokens to avoid downstream confusion where a JWT is expected.
  • The Google OAuth function (auth-google.js) assumes GOOGLE_CLIENT_ID is present but doesn’t validate it like the Supabase config; adding an explicit check with a clear error path (similar to validateServiceKeyConfig) would make misconfiguration failures easier to diagnose.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The email-only `auth-login` function is explicitly marked as demo/auth-lite but is always enabled; consider gating this behavior behind an environment flag (e.g. `DEMO_AUTH_ENABLED`) or restricting it to non-production environments to avoid it being accidentally used as the primary auth mechanism in production.
- In `ApiService.signIn`, the code now stores `response.user.id` via `setAuthToken` while still presenting a `SignInResponse` shape that used to carry JWTs; it may be worth renaming this token or separating a proper `sessionId`/`userId` from auth tokens to avoid downstream confusion where a JWT is expected.
- The Google OAuth function (`auth-google.js`) assumes `GOOGLE_CLIENT_ID` is present but doesn’t validate it like the Supabase config; adding an explicit check with a clear error path (similar to `validateServiceKeyConfig`) would make misconfiguration failures easier to diagnose.

## Individual Comments

### Comment 1
<location> `netlify/functions/auth-signup.js:34-43` </location>
<code_context>
-      return;
-    }
-
-    try {
-      await signInWithGoogle(credential);
-      toast.success("Welcome to Quantum Quest!");
</code_context>

<issue_to_address>
**issue (bug_risk):** Using the anon key for `leaderboard_entries` writes may conflict with RLS and is inconsistent with other auth functions.

In `auth-signup`, the user is created with `getSupabaseHeaders(true)` (service key), but the initial `leaderboard_entries` insert uses `getSupabaseHeaders()` (anon key), whereas `auth-google` uses the service key for the same insert.

With RLS on `leaderboard_entries`, this mismatch can cause 401/403s or silent failures in the signup flow while Google auth continues to work. Please align this with `auth-google` by using `getSupabaseHeaders(true)` here as well, or clearly define RLS policies that allow anon writes to `leaderboard_entries` and document that expectation.
</issue_to_address>

### Comment 2
<location> `Project_documentation.md:298-307` </location>
<code_context>
+### **Port Architecture**
</code_context>

<issue_to_address>
**issue:** Large sections of the documentation are duplicated at the end of the file.

The block from `### **Port Architecture**` through `*Maintained by: QPlay Team*` is duplicated. Please remove one copy so this content appears only once in the file.
</issue_to_address>

### Comment 3
<location> `netlify/functions/achievements.js:52` </location>
<code_context>
          `${SUPABASE_URL}/rest/v1/game_sessions?id=eq.${validatedSessionId}&select=user_id`,
</code_context>

<issue_to_address>
**security (javascript.aws-lambda.security.tainted-sql-string):** Detected user input used to manually construct a SQL string. This is usually bad practice because manual construction could accidentally result in a SQL injection. An attacker could use a SQL injection to steal or modify contents of the database. Instead, use a parameterized query which is available by default in most database engines. Alternatively, consider using an object-relational mapper (ORM) such as Sequelize which will protect your queries.

*Source: opengrep*
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines +34 to +43
try {
const { email, username, full_name } = JSON.parse(event.body);

// Validate email format
if (!isValidEmail(email)) {
return createErrorResponse(400, ErrorMessages.INVALID_EMAIL, corsHeaders);
}

// Check if user already exists
const checkResponse = await fetch(`${SUPABASE_URL}/rest/v1/users?email=eq.${encodeURIComponent(email)}`, {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

issue (bug_risk): Using the anon key for leaderboard_entries writes may conflict with RLS and is inconsistent with other auth functions.

In auth-signup, the user is created with getSupabaseHeaders(true) (service key), but the initial leaderboard_entries insert uses getSupabaseHeaders() (anon key), whereas auth-google uses the service key for the same insert.

With RLS on leaderboard_entries, this mismatch can cause 401/403s or silent failures in the signup flow while Google auth continues to work. Please align this with auth-google by using getSupabaseHeaders(true) here as well, or clearly define RLS policies that allow anon writes to leaderboard_entries and document that expectation.

@ArnavBallinCode ArnavBallinCode merged commit 139b59f into main Dec 19, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

help wanted Extra attention is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants