Skip to content

Conversation

@junaway
Copy link
Contributor

@junaway junaway commented Dec 10, 2025

No description provided.

@vercel
Copy link

vercel bot commented Dec 10, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
agenta-documentation Ready Ready Preview, Comment Jan 8, 2026 7:57pm

@junaway junaway changed the base branch from main to chore/offline-agenta December 10, 2025 21:42
# SuperTokens will use our get_dynamic_oidc_provider to fetch config
supertokens_url = f"/auth/signinup?thirdPartyId={third_party_id}&redirectToPath={redirect}"

return RedirectResponse(url=supertokens_url, status_code=302)

Check warning

Code scanning / CodeQL

URL redirection from remote source

Untrusted URL redirection depends on a [user-provided value](1).

Copilot Autofix

AI 29 days ago

To fix this problem, we should ensure that the redirect parameter is validated before being used in the redirection logic. There are two main approaches:

  1. Whitelist: If the set of allowed redirect targets is known and finite, check that redirect matches one of these safe values.
  2. Relative path validation: If arbitrary relative paths are allowed, ensure that redirect does NOT specify a full URL (with scheme/host), and is a local path.

For this code, since the usage is for paths within the frontend, the second approach is practical:

  • Use Python's urllib.parse.urlparse to check that redirect does not contain a hostname or scheme.
  • Remove any backslash characters (\) to prevent bypass.
  • If validation fails, redirect to a safe default (e.g., /).

Specific changes:

  • Add an import for urlparse from urllib.parse.
  • Validate the redirect parameter before building supertokens_url:
    • Strip backslashes.
    • Ensure urlparse(redirect).netloc and urlparse(redirect).scheme are empty.
    • If the validation fails, set redirect to /.

All code changes are to occur within api/oss/src/apis/fastapi/auth/router.py in the block for /authorize/oidc.
No change to existing functionality except improved security for the redirection.


Suggested changeset 1
api/oss/src/apis/fastapi/auth/router.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/api/oss/src/apis/fastapi/auth/router.py b/api/oss/src/apis/fastapi/auth/router.py
--- a/api/oss/src/apis/fastapi/auth/router.py
+++ b/api/oss/src/apis/fastapi/auth/router.py
@@ -62,6 +62,7 @@
         # Get provider to build third_party_id
         from uuid import UUID
         from oss.src.dbs.postgres.organizations.dao import OrganizationProvidersDAO
+        from urllib.parse import urlparse
 
         providers_dao = OrganizationProvidersDAO()
         provider = await providers_dao.get_by_id(UUID(provider_id))
@@ -75,7 +76,12 @@
 
         # Redirect to SuperTokens third-party signin
         # SuperTokens will use our get_dynamic_oidc_provider to fetch config
-        supertokens_url = f"/auth/signinup?thirdPartyId={third_party_id}&redirectToPath={redirect}"
+        # Validate redirect to avoid open redirect vulnerability
+        safe_redirect = redirect.replace("\\", "")
+        parsed = urlparse(safe_redirect)
+        if parsed.netloc or parsed.scheme:
+            safe_redirect = "/"
+        supertokens_url = f"/auth/signinup?thirdPartyId={third_party_id}&redirectToPath={safe_redirect}"
 
         return RedirectResponse(url=supertokens_url, status_code=302)
 
EOF
@@ -62,6 +62,7 @@
# Get provider to build third_party_id
from uuid import UUID
from oss.src.dbs.postgres.organizations.dao import OrganizationProvidersDAO
from urllib.parse import urlparse

providers_dao = OrganizationProvidersDAO()
provider = await providers_dao.get_by_id(UUID(provider_id))
@@ -75,7 +76,12 @@

# Redirect to SuperTokens third-party signin
# SuperTokens will use our get_dynamic_oidc_provider to fetch config
supertokens_url = f"/auth/signinup?thirdPartyId={third_party_id}&redirectToPath={redirect}"
# Validate redirect to avoid open redirect vulnerability
safe_redirect = redirect.replace("\\", "")
parsed = urlparse(safe_redirect)
if parsed.netloc or parsed.scheme:
safe_redirect = "/"
supertokens_url = f"/auth/signinup?thirdPartyId={third_party_id}&redirectToPath={safe_redirect}"

return RedirectResponse(url=supertokens_url, status_code=302)

Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds Single Sign-On (SSO) via OpenID Connect (OIDC) support to Agenta. The implementation introduces a comprehensive authentication discovery system, dynamic OIDC provider configuration, user identity tracking, and organization-level authentication policies.

Key changes include:

  • Frontend authentication flow refactored to support multiple auth methods with dynamic discovery
  • Backend auth service with OIDC provider management, SuperTokens integration with custom overrides for dynamic providers
  • New database tables for user identities and organization authentication policies (EE)

Reviewed changes

Copilot reviewed 32 out of 41 changed files in this pull request and generated 21 comments.

Show a summary per file
File Description
web/oss/src/pages/auth/[[...path]].tsx Refactored auth page to dynamically show/hide auth methods based on discovery endpoint and environment flags
web/oss/src/lib/helpers/dynamicEnv.ts Added environment variables for email/OIDC auth feature flags
web/oss/src/components/pages/auth/SocialAuth/index.tsx Added optional divider prop to conditionally show separator
web/entrypoint.sh Added logic to derive AUTH_EMAIL_ENABLED and AUTH_OIDC_ENABLED from environment variables
api/oss/src/core/auth/supertokens_overrides.py Implemented SuperTokens overrides for dynamic OIDC providers and custom session handling with user identities
api/oss/src/core/auth/service.py Added AuthService with discovery, authentication, and authorization logic
api/oss/src/core/auth/oidc.py Implemented OIDC client helper utilities for authorization and token exchange
api/oss/src/core/auth/middleware.py Added organization policy enforcement middleware (EE)
api/oss/src/apis/fastapi/auth/router.py Added /discover and /authorize/oidc endpoints
api/oss/src/dbs/postgres/users/* Added user identities DAO, DBE, DBA, mappings, and types
api/oss/src/dbs/postgres/organizations/* Added organization policies, domains, providers, invitations DAOs and models
api/oss/databases/postgres/migrations/* Added migration for user_identities and organization tables
api/ee/src/dbs/postgres/organizations/* EE-specific organization DBE/DBA classes
api/test-auth.http Comprehensive HTTP test file documenting all auth endpoints and flows

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

Comment on lines 12 to 98
class OrganizationPolicyDBA(OrganizationScopeDBA, LegacyLifecycleDBA):
__abstract__ = True

id = Column(
UUID(as_uuid=True),
primary_key=True,
default=uuid.uuid7,
unique=True,
nullable=False,
)
allowed_methods = Column(
ARRAY(String),
nullable=False,
server_default="{}",
)
invitation_only = Column(
Boolean,
nullable=False,
server_default="true",
)
domains_only = Column(
Boolean,
nullable=False,
server_default="false",
)
disable_root = Column(
Boolean,
nullable=False,
server_default="false",
)


class OrganizationDomainDBA(OrganizationScopeDBA, LegacyLifecycleDBA):
__abstract__ = True

id = Column(
UUID(as_uuid=True),
primary_key=True,
default=uuid.uuid7,
unique=True,
nullable=False,
)
domain = Column(
String,
nullable=False,
)
verified = Column(
Boolean,
nullable=False,
server_default="false",
)
verification_token = Column(
String,
nullable=True,
)


class OrganizationProviderDBA(OrganizationScopeDBA, HeaderDBA, LegacyLifecycleDBA):
__abstract__ = True

id = Column(
UUID(as_uuid=True),
primary_key=True,
default=uuid.uuid7,
unique=True,
nullable=False,
)
slug = Column(
String,
nullable=False,
)
enabled = Column(
Boolean,
nullable=False,
server_default="true",
)
domain_id = Column(
UUID(as_uuid=True),
nullable=True,
)
config = Column(
JSONB(none_as_null=True),
nullable=False,
)


class OrganizationInvitationDBA(OrganizationScopeDBA, LegacyLifecycleDBA):
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

The DBA classes inherit from LegacyLifecycleDBA but the migration creates columns matching LifecycleDBA (which includes deleted_at, created_by_id, updated_by_id, and deleted_by_id). This mismatch between the migration and the model definition will cause runtime errors. These classes should inherit from LifecycleDBA instead of LegacyLifecycleDBA to match the migration schema.

Copilot uses AI. Check for mistakes.
return False

# Get domain and check if it matches
domain_dto = await self.domains_dao.get_by_id(provider.domain_id)
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

The code calls await self.domains_dao.get_by_id(provider.domain_id) but the OrganizationDomainsDAO class doesn't have a get_by_id method. It only has get_by_domain, list_by_organization, and mark_verified methods. This will cause an AttributeError at runtime.

Suggested change
domain_dto = await self.domains_dao.get_by_id(provider.domain_id)
if hasattr(self.domains_dao, "get_by_id"):
domain_dto = await self.domains_dao.get_by_id(provider.domain_id)
elif hasattr(self.domains_dao, "get_by_domain"):
domain_dto = await self.domains_dao.get_by_domain(domain)
else:
return False

Copilot uses AI. Check for mistakes.
Comment on lines 24 to 35
from ee.src.core.organizations.types import (
OrganizationPolicy,
OrganizationPolicyCreate,
OrganizationPolicyUpdate,
OrganizationDomain,
OrganizationDomainCreate,
OrganizationProvider,
OrganizationProviderCreate,
OrganizationProviderUpdate,
OrganizationInvitation,
OrganizationInvitationCreate,
)
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

The DAO imports types from ee.src.core.organizations.types but this file is in the OSS directory (oss/src/dbs/postgres/organizations/dao.py). This creates a dependency on EE code from OSS code, which breaks the architectural separation. These types should either be defined in OSS or the DAO should only be used in EE. Based on the context, these organization-related features appear to be EE-only, so this DAO file should probably be in the EE directory.

Copilot uses AI. Check for mistakes.
Comment on lines 1 to 12
from ee.src.core.organizations.types import (
OrganizationPolicy,
OrganizationPolicyCreate,
OrganizationPolicyUpdate,
OrganizationDomain,
OrganizationDomainCreate,
OrganizationProvider,
OrganizationProviderCreate,
OrganizationProviderUpdate,
OrganizationInvitation,
OrganizationInvitationCreate,
)
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

This file imports types from ee.src.core.organizations.types but is located in the OSS directory. This creates an improper dependency on EE code from OSS code, violating the architectural separation. Since organization policies, domains, providers, and invitations appear to be EE-only features, this mappings file should be located in the EE directory structure.

Copilot uses AI. Check for mistakes.
Comment on lines 138 to 143
# Format: oidc:{org_id}:{provider_slug} -> sso:{org_slug}:{provider_slug}
parts = third_party_id.split(":")
org_id_str, provider_slug = parts[1], parts[2]
# TODO: Fetch org_slug from organization table
org_slug = "org" # Placeholder
method = f"sso:{org_slug}:{provider_slug}"
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

The code uses a hardcoded placeholder org_slug = "org" instead of fetching the actual organization slug from the database. This means all SSO identities will be stored with the same generic org slug, making it impossible to distinguish between different organizations' SSO providers. The TODO comment indicates this is known incomplete work that needs to be addressed.

Suggested change
# Format: oidc:{org_id}:{provider_slug} -> sso:{org_slug}:{provider_slug}
parts = third_party_id.split(":")
org_id_str, provider_slug = parts[1], parts[2]
# TODO: Fetch org_slug from organization table
org_slug = "org" # Placeholder
method = f"sso:{org_slug}:{provider_slug}"
# Format: oidc:{org_id}:{provider_slug} -> sso:{org_id}:{provider_slug}
parts = third_party_id.split(":")
org_id_str, provider_slug = parts[1], parts[2]
method = f"sso:{org_id_str}:{provider_slug}"

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,241 @@
"""SuperTokens override functions for dynamic OIDC providers and custom session handling."""

from typing import Dict, Any, List, Optional, Union
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

Import of 'List' is not used.
Import of 'Union' is not used.

Suggested change
from typing import Dict, Any, List, Optional, Union
from typing import Dict, Any, Optional

Copilot uses AI. Check for mistakes.
Comment on lines +6 to +11
from supertokens_python.recipe.thirdparty.provider import (
Provider,
ProviderInput,
ProviderConfig,
ProviderClientConfig,
)
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

Import of 'Provider' is not used.

Copilot uses AI. Check for mistakes.
from supertokens_python.recipe.session.interfaces import (
RecipeInterface as SessionRecipeInterface,
)
from supertokens_python.types import User, RecipeUserId
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

Import of 'User' is not used.

Suggested change
from supertokens_python.types import User, RecipeUserId
from supertokens_python.types import RecipeUserId

Copilot uses AI. Check for mistakes.
Comment on lines 82 to 90
# For now, assume they are
is_member = True

if not is_member:
return {
"error": "NOT_A_MEMBER",
"message": "You are not a member of this organization",
}

Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

This statement is unreachable.

Suggested change
# For now, assume they are
is_member = True
if not is_member:
return {
"error": "NOT_A_MEMBER",
"message": "You are not a member of this organization",
}
# For now, assume they are a member and proceed to policy checks

Copilot uses AI. Check for mistakes.
Comment on lines 267 to 275
# For now, assume they are
is_member = True

if not is_member:
return {
"error": "NOT_A_MEMBER",
"message": "You are not a member of this organization",
}

Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

This statement is unreachable.

Suggested change
# For now, assume they are
is_member = True
if not is_member:
return {
"error": "NOT_A_MEMBER",
"message": "You are not a member of this organization",
}
# For now, assume they are (no membership check implemented yet)

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 57 out of 66 changed files in this pull request and generated 18 comments.


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

Comment on lines 1 to 276
from uuid import UUID
from typing import Optional, List
from sqlalchemy import select

from oss.src.dbs.postgres.shared.engine import engine
from oss.src.dbs.postgres.organizations.dbes import (
OrganizationPolicyDBE,
OrganizationDomainDBE,
OrganizationProviderDBE,
OrganizationInvitationDBE,
)
from oss.src.dbs.postgres.organizations.mappings import (
map_policy_dbe_to_dto,
map_create_policy_dto_to_dbe,
map_update_policy_dto_to_dbe,
map_domain_dbe_to_dto,
map_create_domain_dto_to_dbe,
map_provider_dbe_to_dto,
map_create_provider_dto_to_dbe,
map_update_provider_dto_to_dbe,
map_invitation_dbe_to_dto,
map_create_invitation_dto_to_dbe,
)
from ee.src.core.organizations.types import (
OrganizationPolicy,
OrganizationPolicyCreate,
OrganizationPolicyUpdate,
OrganizationDomain,
OrganizationDomainCreate,
OrganizationProvider,
OrganizationProviderCreate,
OrganizationProviderUpdate,
OrganizationInvitation,
OrganizationInvitationCreate,
)


class OrganizationPoliciesDAO:
async def create(self, dto: OrganizationPolicyCreate) -> OrganizationPolicy:
policy_dbe = map_create_policy_dto_to_dbe(dto)

async with engine.core_session() as session:
session.add(policy_dbe)
await session.commit()
await session.refresh(policy_dbe)

return map_policy_dbe_to_dto(policy_dbe)

async def get_by_organization(
self, organization_id: UUID
) -> Optional[OrganizationPolicy]:
async with engine.core_session() as session:
stmt = select(OrganizationPolicyDBE).filter_by(
organization_id=organization_id
)
result = await session.execute(stmt)
policy_dbe = result.scalar()

if policy_dbe is None:
return None

return map_policy_dbe_to_dto(policy_dbe)

async def update(
self,
organization_id: UUID,
dto: OrganizationPolicyUpdate,
) -> Optional[OrganizationPolicy]:
async with engine.core_session() as session:
stmt = select(OrganizationPolicyDBE).filter_by(
organization_id=organization_id
)
result = await session.execute(stmt)
policy_dbe = result.scalar()

if policy_dbe is None:
return None

map_update_policy_dto_to_dbe(policy_dbe, dto)
await session.commit()
await session.refresh(policy_dbe)

return map_policy_dbe_to_dto(policy_dbe)


class OrganizationDomainsDAO:
async def create(
self, dto: OrganizationDomainCreate
) -> OrganizationDomain:
domain_dbe = map_create_domain_dto_to_dbe(dto)

async with engine.core_session() as session:
session.add(domain_dbe)
await session.commit()
await session.refresh(domain_dbe)

return map_domain_dbe_to_dto(domain_dbe)

async def get_by_id(self, domain_id: UUID) -> Optional[OrganizationDomain]:
async with engine.core_session() as session:
stmt = select(OrganizationDomainDBE).filter_by(id=domain_id)
result = await session.execute(stmt)
domain_dbe = result.scalar()

if domain_dbe is None:
return None

return map_domain_dbe_to_dto(domain_dbe)

async def get_by_domain(self, domain: str) -> Optional[OrganizationDomain]:
async with engine.core_session() as session:
stmt = select(OrganizationDomainDBE).filter_by(domain=domain)
result = await session.execute(stmt)
domain_dbe = result.scalar()

if domain_dbe is None:
return None

return map_domain_dbe_to_dto(domain_dbe)

async def list_by_organization(
self, organization_id: UUID, verified_only: bool = False
) -> List[OrganizationDomain]:
async with engine.core_session() as session:
stmt = select(OrganizationDomainDBE).filter_by(
organization_id=organization_id
)
if verified_only:
stmt = stmt.filter_by(verified=True)

result = await session.execute(stmt)
domain_dbes = result.scalars().all()

return [map_domain_dbe_to_dto(dbe) for dbe in domain_dbes]

async def mark_verified(self, domain_id: UUID) -> Optional[OrganizationDomain]:
async with engine.core_session() as session:
stmt = select(OrganizationDomainDBE).filter_by(id=domain_id)
result = await session.execute(stmt)
domain_dbe = result.scalar()

if domain_dbe is None:
return None

domain_dbe.verified = True
await session.commit()
await session.refresh(domain_dbe)

return map_domain_dbe_to_dto(domain_dbe)


class OrganizationProvidersDAO:
async def create(
self, dto: OrganizationProviderCreate
) -> OrganizationProvider:
provider_dbe = map_create_provider_dto_to_dbe(dto)

async with engine.core_session() as session:
session.add(provider_dbe)
await session.commit()
await session.refresh(provider_dbe)

return map_provider_dbe_to_dto(provider_dbe)

async def get_by_id(self, provider_id: UUID) -> Optional[OrganizationProvider]:
async with engine.core_session() as session:
stmt = select(OrganizationProviderDBE).filter_by(id=provider_id)
result = await session.execute(stmt)
provider_dbe = result.scalar()

if provider_dbe is None:
return None

return map_provider_dbe_to_dto(provider_dbe)

async def get_by_slug(
self, organization_id: UUID, slug: str
) -> Optional[OrganizationProvider]:
async with engine.core_session() as session:
stmt = select(OrganizationProviderDBE).filter_by(
organization_id=organization_id, slug=slug
)
result = await session.execute(stmt)
provider_dbe = result.scalar()

if provider_dbe is None:
return None

return map_provider_dbe_to_dto(provider_dbe)

async def list_by_organization(
self, organization_id: UUID, enabled_only: bool = False
) -> List[OrganizationProvider]:
async with engine.core_session() as session:
stmt = select(OrganizationProviderDBE).filter_by(
organization_id=organization_id
)
if enabled_only:
stmt = stmt.filter_by(enabled=True)

result = await session.execute(stmt)
provider_dbes = result.scalars().all()

return [map_provider_dbe_to_dto(dbe) for dbe in provider_dbes]

async def list_by_domain(
self, domain_id: UUID, enabled_only: bool = False
) -> List[OrganizationProvider]:
async with engine.core_session() as session:
stmt = select(OrganizationProviderDBE).filter_by(domain_id=domain_id)
if enabled_only:
stmt = stmt.filter_by(enabled=True)

result = await session.execute(stmt)
provider_dbes = result.scalars().all()

return [map_provider_dbe_to_dto(dbe) for dbe in provider_dbes]

async def update(
self,
provider_id: UUID,
dto: OrganizationProviderUpdate,
) -> Optional[OrganizationProvider]:
async with engine.core_session() as session:
stmt = select(OrganizationProviderDBE).filter_by(id=provider_id)
result = await session.execute(stmt)
provider_dbe = result.scalar()

if provider_dbe is None:
return None

map_update_provider_dto_to_dbe(provider_dbe, dto)
await session.commit()
await session.refresh(provider_dbe)

return map_provider_dbe_to_dto(provider_dbe)


class OrganizationInvitationsDAO:
async def create(
self, dto: OrganizationInvitationCreate
) -> OrganizationInvitation:
invitation_dbe = map_create_invitation_dto_to_dbe(dto)

async with engine.core_session() as session:
session.add(invitation_dbe)
await session.commit()
await session.refresh(invitation_dbe)

return map_invitation_dbe_to_dto(invitation_dbe)

async def get_by_token(self, token: str) -> Optional[OrganizationInvitation]:
async with engine.core_session() as session:
stmt = select(OrganizationInvitationDBE).filter_by(token=token)
result = await session.execute(stmt)
invitation_dbe = result.scalar()

if invitation_dbe is None:
return None

return map_invitation_dbe_to_dto(invitation_dbe)

async def list_by_organization(
self, organization_id: UUID, status: Optional[str] = None
) -> List[OrganizationInvitation]:
async with engine.core_session() as session:
stmt = select(OrganizationInvitationDBE).filter_by(
organization_id=organization_id
)
if status:
stmt = stmt.filter_by(status=status)

result = await session.execute(stmt)
invitation_dbes = result.scalars().all()

return [map_invitation_dbe_to_dto(dbe) for dbe in invitation_dbes]
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

The OSS version defines the same database models and migrations that appear in the EE version. This creates significant code duplication - the entire contents of dbas.py, dbes.py, mappings.py, and dao.py are duplicated. Since these are meant to be EE-only features (organization policies, SSO providers), they should only exist in the EE codebase. The OSS versions should either be removed or contain only stub/fallback implementations.

Copilot uses AI. Check for mistakes.
from alembic import context

from sqlalchemy import Connection, func, insert, select, update
from sqlalchemy.orm import load_only
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

The load_only optimization is imported but only partially applied. While line 65 adds optimization for the organization query in this batch processing migration, consider whether other queries in the migration could benefit from similar optimization to reduce memory usage during large data migrations.

Copilot uses AI. Check for mistakes.
Comment on lines 17 to 18
showDivider = true,
}: SocialAuthProps) => {
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

The showDivider prop defaults to true to maintain backward compatibility, but the calling code in the auth page explicitly passes the value on line 225. The prop could be made required instead of optional with a default, making the API more explicit and reducing ambiguity about when the divider appears.

Copilot uses AI. Check for mistakes.
Comment on lines 373 to 377
sa.Column(
"expires_at",
sa.TIMESTAMP(timezone=True),
nullable=True,
),
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

The expires_at field in the migration is correctly defined as TIMESTAMP, but the DBA model defines it as UUID (lines 125-128 in dbas.py). This mismatch between the migration and the model will cause runtime errors. The DBA model should be updated to use TIMESTAMP type.

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +50
"""In-memory state store for OIDC flows. TODO: Move to Redis for production."""

from typing import Dict, Optional
from datetime import datetime, timedelta
import asyncio


class StateStore:
"""Simple in-memory state store with expiration."""

def __init__(self):
self._store: Dict[str, Dict] = {}
self._expiry: Dict[str, datetime] = {}

async def set(self, key: str, value: Dict, ttl_seconds: int = 600) -> None:
"""Store a value with TTL (default 10 minutes)."""
self._store[key] = value
self._expiry[key] = datetime.utcnow() + timedelta(seconds=ttl_seconds)
await self._cleanup_expired()

async def get(self, key: str) -> Optional[Dict]:
"""Get a value, return None if expired or not found."""
await self._cleanup_expired()

if key not in self._store:
return None

if key in self._expiry and datetime.utcnow() > self._expiry[key]:
del self._store[key]
del self._expiry[key]
return None

return self._store[key]

async def delete(self, key: str) -> None:
"""Delete a value."""
self._store.pop(key, None)
self._expiry.pop(key, None)

async def _cleanup_expired(self) -> None:
"""Remove expired entries."""
now = datetime.utcnow()
expired_keys = [k for k, exp in self._expiry.items() if now > exp]
for key in expired_keys:
self._store.pop(key, None)
self._expiry.pop(key, None)


# Singleton instance
state_store = StateStore()
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

The in-memory StateStore is not suitable for production use, especially in multi-instance deployments where state needs to be shared. The TODO comment acknowledges this, but using this in production would cause OIDC flows to fail when requests hit different instances. Consider implementing Redis-backed storage before deploying to production, or add validation to prevent deployment with in-memory state.

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,147 @@
"""SuperTokens configuration and initialization."""

from typing import Dict, List, Any, Optional
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

Import of 'Optional' is not used.

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,343 @@
"""SuperTokens override functions for dynamic OIDC providers and custom session handling."""

from typing import Dict, Any, List, Optional, Union
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

Import of 'List' is not used.

Copilot uses AI. Check for mistakes.
from supertokens_python.recipe.session.interfaces import (
RecipeInterface as SessionRecipeInterface,
)
from supertokens_python.types import User, RecipeUserId
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

Import of 'User' is not used.

Copilot uses AI. Check for mistakes.
Comment on lines 83 to 90
is_member = True

if not is_member:
return {
"error": "NOT_A_MEMBER",
"message": "You are not a member of this organization",
}

Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

This statement is unreachable.

Suggested change
is_member = True
if not is_member:
return {
"error": "NOT_A_MEMBER",
"message": "You are not a member of this organization",
}

Copilot uses AI. Check for mistakes.
Comment on lines 281 to 284
return {
"error": "NOT_A_MEMBER",
"message": "You are not a member of this organization",
}
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

This statement is unreachable.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 60 out of 69 changed files in this pull request and generated 4 comments.


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

)


class OrganizationPolicyDBA(OrganizationScopeDBA, LegacyLifecycleDBA):
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

Inconsistent base class usage: OSS uses LifecycleDBA while EE uses LegacyLifecycleDBA. This inconsistency may lead to differences in table schemas and lifecycle field behavior between OSS and EE editions for the same tables.

Copilot uses AI. Check for mistakes.
Comment on lines +122 to +125
call_to_action=(
"Click the link below to accept the invitation:</p><br>"
f'<a href="{invite_link}">Accept Invitation</a>'
),
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

The variable invite_link is referenced in the f-string but is not defined in this diff section. This will cause a NameError. Check if invite_link is defined earlier in the function or if it should be constructed here.

Copilot uses AI. Check for mistakes.
Comment on lines 78 to 90
# Get user ID and check membership
user_id = session.get_user_id()

# TODO: Check if user is a member of organization
# For now, assume they are
is_member = True

if not is_member:
return {
"error": "NOT_A_MEMBER",
"message": "You are not a member of this organization",
}

Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

This statement is unreachable.

Suggested change
# Get user ID and check membership
user_id = session.get_user_id()
# TODO: Check if user is a member of organization
# For now, assume they are
is_member = True
if not is_member:
return {
"error": "NOT_A_MEMBER",
"message": "You are not a member of this organization",
}
# Get user ID (membership enforcement not yet implemented)
# TODO: Enforce that user is a member of the organization before applying policy
user_id = session.get_user_id()

Copilot uses AI. Check for mistakes.
Comment on lines 285 to 288
return {
"error": "NOT_A_MEMBER",
"message": "You are not a member of this organization",
}
Copy link

Copilot AI Dec 25, 2025

Choose a reason for hiding this comment

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

This statement is unreachable.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 62 out of 71 changed files in this pull request and generated 2 comments.


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

identities = payload.get("identities", [])

# Get user ID and check membership
user_id = session.get_user_id()
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

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

Variable user_id is not used.

Suggested change
user_id = session.get_user_id()

Copilot uses AI. Check for mistakes.
Comment on lines 342 to 345
return {
"error": "NOT_A_MEMBER",
"message": "You are not a member of this organization",
}
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

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

This statement is unreachable.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 95 out of 104 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (4)

web/entrypoint.sh:1

  • The order of NEXT_PUBLIC_AGENTA_WEB_URL and NEXT_PUBLIC_AGENTA_API_URL has been swapped compared to the original, but the surrounding code context suggests this was intentional. However, this creates an inconsistency with the removed lines that had AGENTA_API_URL first. Verify this order change is intentional and doesn't break existing configurations that depend on the original ordering.
    api/oss/src/utils/env.py:1
  • Hardcoding a default PostHog API key in production code is a security risk. This key should only be used in development/demo environments. Consider making this explicitly conditional on environment (e.g., only default in demo mode) or removing the default entirely to force explicit configuration.
    api/oss/src/utils/env.py:1
  • Defaulting critical security keys to 'replace-me' is dangerous. These should fail loudly if not set in production rather than falling back to insecure defaults. Consider raising an exception when these are not configured in non-development environments.
    api/oss/src/services/organization_service.py:1
  • The error message 'No existing invitation found for the user' is misleading when an existing_role exists but no existing_invitation. The user may have been previously invited (thus having a role) but the invitation record is missing. Consider a more accurate message like 'User has no active invitation or existing role in this organization'.

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

# --> GET ORGANIZATION BATCH
query = (
select(OrganizationDB)
.options(load_only(OrganizationDB.id, OrganizationDB.owner))
Copy link

Copilot AI Dec 26, 2025

Choose a reason for hiding this comment

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

Adding load_only() to limit columns fetched is good, but verify that no other attributes of OrganizationDB are accessed elsewhere in this migration that aren't included in this list. If other attributes are accessed, this could cause additional queries or errors.

Suggested change
.options(load_only(OrganizationDB.id, OrganizationDB.owner))

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings December 26, 2025 19:24
bekossy and others added 10 commits January 8, 2026 15:11
… CSV parsing

- Prevent grouping of flat columns with dots in their names (e.g., "agents.md")
- Only group columns that came from object expansion (have parentKey property)
- Add proper CSV parser to handle quoted fields with embedded newlines and commas
- Fix cell value access to prioritize direct key lookup before nested path parsing
- Add documentation for expanded column detection logic
…e-in-testset-upload-flow

[Frontend/fix] Improve preview table in file upload flow for new testset creation
…luation-result-overview-page

[AGE-3539]: frontend unify evaluation result page
Copilot AI review requested due to automatic review settings January 8, 2026 15:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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

Files not reviewed (1)
  • docs/package-lock.json: Language not supported

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

except (NoResultFound, ValueError):
await update_organization(
organization_id=organization_id, values_to_update={"owner": str(user_db.id)}
organization_id=organization_id, values_to_update={"owner_id": user_db.id}
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

Changed from 'owner' to 'owner_id', but the database model expects a UUID type. Verify that user_db.id is already a UUID and not a string to avoid type mismatches.

Copilot uses AI. Check for mistakes.
# Auth service for domain policy enforcement
auth_service = AuthService()

def _merge_session_identities(session: Optional[Any], method: Optional[str]) -> List[str]:
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

Function name _merge_session_identities uses a leading underscore suggesting it's private/internal, but it's not a method of a class. Consider removing the underscore or organizing it as a class method if appropriate.

Copilot uses AI. Check for mistakes.
Comment on lines +913 to +915
replace_map = {}
if operations.columns.replace:
replace_map = {old: new for old, new in operations.columns.replace}
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The replace_map initialization at line 913 is immediately overwritten at line 915 if operations.columns.replace exists. Consider removing the initial empty dict assignment or combining the logic.

Suggested change
replace_map = {}
if operations.columns.replace:
replace_map = {old: new for old, new in operations.columns.replace}
replace_map = {old: new for old, new in (operations.columns.replace or [])}

Copilot uses AI. Check for mistakes.
Note: Only methods that are available (true) are included in the response.
Missing methods should be assumed false on the client side.
"""
print(f"[DISCOVERY] Starting discovery for email: {email}")
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

Using print() for logging throughout this file instead of the log object imported at the top. Replace print statements with proper logging calls (e.g., log.info(), log.debug()) for consistency with the rest of the codebase.

Copilot uses AI. Check for mistakes.
stmt=stmt,
DBE=self.BlobDBE,
attribute="id", # UUID7 - use id for cursor-based pagination
attribute="created_at", # Blob IDs are content-hashed (UUID5), use timestamp for ordering
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

Comment explains the change but the original code used 'id' for UUID7. Verify that all blob IDs in the system are indeed UUID5 (content-hashed) and not UUID7, as mixing ordering strategies could cause inconsistent pagination.

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings January 8, 2026 18:44
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 152 out of 520 changed files in this pull request and generated 8 comments.

Files not reviewed (1)
  • docs/package-lock.json: Language not supported

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

Comment on lines +256 to +261
invitation = await create_invitation(existing_role, project_id, payload.email)
else:
raise HTTPException(
status_code=404,
detail="No existing invitation found for the user",
)
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The else clause raises a 404 when there's no existing invitation OR role, but the function is called 'resend_user_organization_invite'. If the user was never invited, this should probably return a different error indicating that there's no invitation to resend, rather than proceeding to create a new invitation with an undefined role.

Copilot uses AI. Check for mistakes.
stmt=stmt,
DBE=self.BlobDBE,
attribute="id", # UUID7 - use id for cursor-based pagination
attribute="created_at", # Blob IDs are content-hashed (UUID5), use timestamp for ordering
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The comment explains the change from 'id' to 'created_at' for ordering, but this breaks cursor-based pagination semantics if multiple blobs have the same timestamp. Consider documenting this limitation or using a composite key (created_at, id) for stable ordering.

Suggested change
attribute="created_at", # Blob IDs are content-hashed (UUID5), use timestamp for ordering
# Blob IDs are content-hashed (UUID5), so they are not suitable for
# chronological ordering. We therefore use `created_at` as the primary
# sort key for windowing. Note: using `created_at` alone means that
# cursor-based pagination is not strictly stable if multiple blobs share
# the same timestamp, because there is no secondary tie-breaker in the
# ORDER BY clause (e.g. `(created_at, id)`). Callers should be aware of
# this limitation when relying on pagination semantics.
attribute="created_at",

Copilot uses AI. Check for mistakes.
tags: Optional[Dict[str, Any]] = None
meta: Optional[Dict[str, Any]] = None
#
owner_id: UUID
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The owner_id field is required (no Optional), but in the Organization model it's not marked as required. This could cause serialization issues if owner_id is None in the database. Consider making it Optional[UUID] to match the database schema's nullable constraint pattern, or ensure it's always set during creation.

Suggested change
owner_id: UUID
owner_id: Optional[UUID] = None

Copilot uses AI. Check for mistakes.
# Check for error status and propagate it
if response.status and response.status.code and response.status.code >= 400:
error_message = response.status.message or "Custom code execution failed"
raise RuntimeError(error_message)
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The generic error message 'Custom code execution failed' doesn't provide enough context for debugging. Consider including the status code in the error message, e.g., f'Custom code execution failed with status {response.status.code}: {error_message}'.

Suggested change
raise RuntimeError(error_message)
raise RuntimeError(
f"Custom code execution failed with status {response.status.code}: {error_message}"
)

Copilot uses AI. Check for mistakes.
Comment on lines +97 to +105
"""
print(f"[DISCOVERY] Starting discovery for email: {email}")

# Extract domain from email (if provided)
domain = email.split("@")[1] if email and "@" in email else None
print(f"[DISCOVERY] Extracted domain: {domain}")

# Check if user exists only when email looks valid
user = None
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

This domain extraction doesn't validate that there's exactly one '@' symbol. An email like 'user@@example.com' would extract '@example.com' as the domain. Use email.count('@') == 1 in the condition or validate email format first.

Suggested change
"""
print(f"[DISCOVERY] Starting discovery for email: {email}")
# Extract domain from email (if provided)
domain = email.split("@")[1] if email and "@" in email else None
print(f"[DISCOVERY] Extracted domain: {domain}")
# Check if user exists only when email looks valid
user = None
# Extract domain from email (if provided and structurally valid)
domain = email.split("@")[1] if email and email.count("@") == 1 else None
print(f"[DISCOVERY] Extracted domain: {domain}")
# Check if user exists only when email looks valid
user = None
user_exists = False
user_id = None
if email and email.count("@") == 1:

Copilot uses AI. Check for mistakes.
healthcheck:
test: ["CMD-SHELL", "pg_isready -U postgres"]
test: ["CMD-SHELL", "pg_isready -U username -d agenta_oss_core"]
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The healthcheck uses hardcoded username 'username' instead of the ${POSTGRES_USER} variable. This will fail if POSTGRES_USER is set to a different value. Use 'pg_isready -U ${POSTGRES_USER:-username} -d agenta_oss_core' to respect the environment variable.

Suggested change
test: ["CMD-SHELL", "pg_isready -U username -d agenta_oss_core"]
test: ["CMD-SHELL", "pg_isready -U ${POSTGRES_USER:-username} -d agenta_oss_core"]

Copilot uses AI. Check for mistakes.
Comment on lines +453 to +454
#
#
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

There are 17 consecutive lines of empty comments (lines 453-469) at the end of the file before the networks section. These appear to be placeholder or accidentally committed. Consider removing them to clean up the file.

Copilot uses AI. Check for mistakes.
agenta = ">=0.72.1"

# Core framework dependencies
fastapi = ">=0.127"
Copy link

Copilot AI Jan 8, 2026

Choose a reason for hiding this comment

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

The pydantic email extra is added but there's no comment explaining why. Since this PR adds email validation in multiple places (domain extraction, user identities), consider adding a comment explaining that this enables EmailStr validators.

Suggested change
fastapi = ">=0.127"
fastapi = ">=0.127"
# Enable Pydantic's email validators (e.g. EmailStr) used for email/domain validation.

Copilot uses AI. Check for mistakes.
Copilot AI review requested due to automatic review settings January 8, 2026 19:56
except ValueError as e:
# Slug validation errors (format, immutability, personal org, etc.)
return JSONResponse(
{"detail": str(e)},

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
flows to this location and may be exposed to an external user.

Copilot Autofix

AI about 3 hours ago

To fix the problem, we should stop returning the raw exception message (str(e)) to the client and instead return a generic, user-safe error message while optionally logging the original exception on the server side. This aligns with the recommendation to keep detailed error information only in logs and not in user-facing responses.

The best minimal-impact fix here is:

  • In the except ValueError as e: block, log the exception using the existing logging facility (get_module_logger) and then return a generic, but still user-meaningful 400 response, such as “Invalid organization update payload.” or a similar non-sensitive message.
  • Do not change the semantics of when we return 400 vs 500; maintain the 400 status code for validation-like errors.
  • Avoid introducing new dependencies; use the already-imported logging utility.

Concretely:

  • At the top of api/ee/src/routers/organization_router.py, after router = APIRouter(), define a module-level logger with logger = get_module_logger(__name__).
  • In the except ValueError as e: block, replace the return that uses str(e) with:
    • a call to logger.warning(...) or logger.error(...) including the exception, and
    • a JSONResponse whose "detail" string is generic and not derived from e.

No new external libraries are needed; we only use the already-present get_module_logger.

Suggested changeset 1
api/ee/src/routers/organization_router.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/api/ee/src/routers/organization_router.py b/api/ee/src/routers/organization_router.py
--- a/api/ee/src/routers/organization_router.py
+++ b/api/ee/src/routers/organization_router.py
@@ -43,6 +43,7 @@
 
 
 router = APIRouter()
+logger = get_module_logger(__name__)
 
 log = get_module_logger(__name__)
 
@@ -191,8 +192,9 @@
 
     except ValueError as e:
         # Slug validation errors (format, immutability, personal org, etc.)
+        logger.warning("Organization update validation error", exc_info=e)
         return JSONResponse(
-            {"detail": str(e)},
+            {"detail": "Invalid organization update payload."},
             status_code=400,
         )
     except Exception as e:
EOF
@@ -43,6 +43,7 @@


router = APIRouter()
logger = get_module_logger(__name__)

log = get_module_logger(__name__)

@@ -191,8 +192,9 @@

except ValueError as e:
# Slug validation errors (format, immutability, personal org, etc.)
logger.warning("Organization update validation error", exc_info=e)
return JSONResponse(
{"detail": str(e)},
{"detail": "Invalid organization update payload."},
status_code=400,
)
except Exception as e:
Copilot is powered by AI and may make mistakes. Always verify output.
except ValueError as e:
# New owner not a member or organization not found
return JSONResponse(
{"detail": str(e)},

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
flows to this location and may be exposed to an external user.

Copilot Autofix

AI about 3 hours ago

In general, the fix is to stop returning the raw exception message to the client and instead return a generic, user‑safe error message while logging the exception details (including stack trace) on the server side. This preserves debuggability for developers while preventing unintentionally sensitive information from leaking to API consumers.

For this file, the best targeted fix is:

  1. Use the module logger (which is already imported via get_module_logger) to log exceptions in both except blocks.
  2. Replace {"detail": str(e)} payloads with generic, stable messages, e.g.:
    • For the ValueError in transfer_organization_ownership, something like "Unable to transfer organization ownership" (optionally more specific but not including str(e)).
    • For the generic Exception in both endpoints, a generic "Internal server error" or similar.
  3. Avoid inlining import traceback inside exception handlers; instead, import traceback once at the top and use log.exception(...) or traceback.format_exc() with log.error(...). log.exception already records the stack trace and is a good fit here.

Concretely:

  • Near the top of organization_router.py, create a log = get_module_logger(__name__) if it does not already exist in the omitted lines, and add an import traceback if we choose to keep explicit traceback formatting. However, we’re constrained to only edit shown snippets; the top snippet shows imports but no logger definition, and the later code uses log.info(...) already, which implies log must be defined somewhere in the omitted portion. To keep behavior consistent, we should not redefine log; instead, just call log.exception(...) in the catch blocks (no new imports needed because log.exception doesn’t require traceback).
  • In the except ValueError as e: block at lines 336‑341, stop using str(e) in the response body. Replace it with a fixed message.
  • In the except Exception as e: block for transfer_organization_ownership, remove import traceback, remove traceback.print_exc(), call log.exception("Error transferring organization ownership"), and raise an HTTPException with a generic detail string.
  • In the except Exception as e: block for create_collaborative_organization, similarly remove the inline traceback import/print, call log.exception("Error creating collaborative organization"), and raise an HTTPException with a generic detail string.

This approach keeps HTTP status codes and overall control flow the same, but removes exposure of raw exception messages and prints stack traces only to logs.


Suggested changeset 1
api/ee/src/routers/organization_router.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/api/ee/src/routers/organization_router.py b/api/ee/src/routers/organization_router.py
--- a/api/ee/src/routers/organization_router.py
+++ b/api/ee/src/routers/organization_router.py
@@ -335,17 +335,18 @@
 
     except ValueError as e:
         # New owner not a member or organization not found
+        # Log the specific error details server-side, but return a generic message to the client
+        log.exception("Error transferring organization ownership due to invalid value")
         return JSONResponse(
-            {"detail": str(e)},
+            {"detail": "Unable to transfer organization ownership."},
             status_code=400,
         )
     except Exception as e:
-        import traceback
-
-        traceback.print_exc()
+        # Log unexpected errors with stack trace, return generic error to client
+        log.exception("Unexpected error while transferring organization ownership")
         raise HTTPException(
             status_code=500,
-            detail=str(e),
+            detail="Internal server error.",
         )
 
 
@@ -391,12 +386,11 @@
         )
 
     except Exception as e:
-        import traceback
-
-        traceback.print_exc()
+        # Log unexpected errors with stack trace, return generic error to client
+        log.exception("Unexpected error while creating collaborative organization")
         raise HTTPException(
             status_code=500,
-            detail=str(e),
+            detail="Internal server error.",
         )
 
 
EOF
@@ -335,17 +335,18 @@

except ValueError as e:
# New owner not a member or organization not found
# Log the specific error details server-side, but return a generic message to the client
log.exception("Error transferring organization ownership due to invalid value")
return JSONResponse(
{"detail": str(e)},
{"detail": "Unable to transfer organization ownership."},
status_code=400,
)
except Exception as e:
import traceback

traceback.print_exc()
# Log unexpected errors with stack trace, return generic error to client
log.exception("Unexpected error while transferring organization ownership")
raise HTTPException(
status_code=500,
detail=str(e),
detail="Internal server error.",
)


@@ -391,12 +386,11 @@
)

except Exception as e:
import traceback

traceback.print_exc()
# Log unexpected errors with stack trace, return generic error to client
log.exception("Unexpected error while creating collaborative organization")
raise HTTPException(
status_code=500,
detail=str(e),
detail="Internal server error.",
)


Copilot is powered by AI and may make mistakes. Always verify output.
@jp-agenta jp-agenta closed this Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants