codeant-ai-for-open-source[bot] commented on code in PR #45508: URL: https://github.com/apache/superset/pull/45508#discussion_r4360918345
########## superset/integrations/teams/store.py: ########## @@ -0,0 +1,159 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Transactional persistence for authenticated Teams installation events.""" + +from datetime import datetime, timezone +from urllib.parse import urlsplit +from uuid import UUID + +from sqlalchemy.exc import IntegrityError +from sqlalchemy.orm import Session + +from superset.integrations.teams.config import validate_team_id +from superset.integrations.teams.models import TeamsInstallation + + +def _identity( + tenant_id: str, application_id: str, team_id: str +) -> tuple[UUID, UUID, str]: + """Validate and normalize the installation identity without echoing input.""" + try: + tenant = UUID(tenant_id) + application = UUID(application_id) + except (ValueError, TypeError, AttributeError): + raise ValueError("Teams tenant and application IDs must be UUIDs") from None + if not tenant.int or not application.int: + raise ValueError("Teams tenant and application IDs must be nonzero UUIDs") + return tenant, application, validate_team_id(team_id) + + +def _service_url(value: str) -> str: + """Check URL shape; the caller must verify authenticity and Microsoft ownership.""" + try: + url = urlsplit(value) + valid = ( + isinstance(value, str) + and len(value) <= 2048 + and not any( + character.isspace() or ord(character) < 32 for character in value + ) + and url.scheme == "https" + and bool(url.hostname) + and url.port in (None, 443) + and url.username is None + and url.password is None + and not url.query + and not url.fragment + ) + except (ValueError, TypeError, AttributeError): + valid = False + if not valid: + raise ValueError("Teams verified service URL must be an HTTPS URL") + return value + + +class TeamsInstallationStore: + """Persist verified events without committing or modifying configured destinations. + + Callers must authenticate events, enforce the deployment tenant/application, + and verify service URLs before invoking this store. This layer stores no + credentials or channels and never makes a team selectable on its own. + Events are applied in processing order; duplicate installs/removals are safe. + """ + + def __init__(self, session: Session) -> None: + """Use the caller's metadata database transaction.""" + self.session = session + + def get_active( + self, tenant_id: str, application_id: str, team_id: str + ) -> TeamsInstallation | None: + """Return the active installation for exactly the supplied identity.""" + tenant, application, team = _identity(tenant_id, application_id, team_id) + return ( + self.session.query(TeamsInstallation) + .populate_existing() + .filter_by( + tenant_id=tenant, application_id=application, team_id=team, active=True + ) + .one_or_none() + ) + + def install( + self, + tenant_id: str, + application_id: str, + team_id: str, + verified_service_url: str, + ) -> TeamsInstallation: + """Create, refresh, or reactivate an installation, including insert races.""" + tenant, application, team = _identity(tenant_id, application_id, team_id) + service_url = _service_url(verified_service_url) + now = datetime.now(timezone.utc).replace(tzinfo=None) + table = TeamsInstallation.__table__ + update = ( + table.update() + .where(table.c.tenant_id == tenant) + .where(table.c.application_id == application) + .where(table.c.team_id == team) + .values(service_url=service_url, active=True, changed_on=now) Review Comment: **Suggestion:** If an earlier add callback is delayed or retried until after a later removal commits, this update reactivates the bot and enables delivery despite it being uninstalled. **Assessment:** ๐ `Major` ยท ๐ `Occurrence: Rarely` ยท ๐ท๏ธ `Race condition` [](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=c4cc72e0669b4546a6b870295e56aba5&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=c4cc72e0669b4546a6b870295e56aba5&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) <details> <summary><b>Prompt for AI Agent ๐ค </b></summary> ```mdx This is a comment left during a code review. **Path:** superset/integrations/teams/store.py **Line:** 113:113 **Comment:** *Race Condition: If an earlier add callback is delayed or retried until after a later removal commits, this update reactivates the bot and enables delivery despite it being uninstalled. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45508&comment_hash=2567471e8eb4fd85bf8e7b37317be6097806b8a8fe5a98671183c70ba87e7eca&reaction=like'>๐</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45508&comment_hash=2567471e8eb4fd85bf8e7b37317be6097806b8a8fe5a98671183c70ba87e7eca&reaction=dislike'>๐</a> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
