diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index 641fa0e3224..4cc61bf6de2 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -37,6 +37,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ libxmlsec1-dev \ make \ neovim \ + nginx \ openssh-client \ pkg-config \ postgresql-client \ diff --git a/.github/workflows/pr-craft-k8s-tests.yml b/.github/workflows/pr-craft-k8s-tests.yml index 546911395d2..d8e9688580c 100644 --- a/.github/workflows/pr-craft-k8s-tests.yml +++ b/.github/workflows/pr-craft-k8s-tests.yml @@ -60,6 +60,8 @@ env: SANDBOX_TURN_TIMEOUT_SECONDS: "120" KIND_REGISTRY_NAME: "kind-registry" KIND_REGISTRY_PORT: "5001" + KIND_VERSION: "v0.31.0" + KUBECTL_VERSION: "v1.35.0" # The pytest process runs on the runner, so it reaches chart-managed # Postgres/Redis through kubectl port-forwards. @@ -159,6 +161,121 @@ jobs: fi echo "test-files=[${entries%,}]" >> "$GITHUB_OUTPUT" + prepare-craft-assets: + name: Prepare Craft test assets + needs: changes + if: needs.changes.outputs.craft_k8s == 'true' + runs-on: + - runs-on + - runner=2cpu-linux-x64 + - spot=false + - ${{ format('run-id={0}-craft-k8s-tools', github.run_id) }} + timeout-minutes: 10 + permissions: + contents: read + steps: + - uses: runs-on/action@4e5f72399b6b17f2e79c511c1b38a315a64d22dc + + - name: Checkout code + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # ratchet:actions/checkout@v6 + with: + persist-credentials: false + + # The run-scoped key prevents pull-request code from poisoning caches used + # by other runs. Test shards restore the completed cache after this job. + - name: Restore kind tools + id: kind-tools-cache + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # zizmor: ignore[cache-poisoning] + with: + path: ${{ runner.tool_cache }}/kind/${{ env.KIND_VERSION }}/amd64 + key: craft-kind-tools-${{ runner.os }}-${{ runner.arch }}-${{ env.KIND_VERSION }}-${{ env.KUBECTL_VERSION }}-${{ github.run_id }} + + - name: Download and verify kind tools + if: steps.kind-tools-cache.outputs.cache-hit != 'true' + run: | + set -euo pipefail + + download() { + curl --fail --location --silent --show-error \ + --retry 5 --retry-delay 2 --retry-all-errors \ + --output "$2" "$1" + } + + cache_dir="${RUNNER_TOOL_CACHE}/kind/${KIND_VERSION}/amd64" + kind_dir="${cache_dir}/kind/bin" + kubectl_dir="${cache_dir}/kubectl/bin" + mkdir -p "${kind_dir}" "${kubectl_dir}" + + kind_filename="kind-linux-amd64" + kind_url="https://github.com/kubernetes-sigs/kind/releases/download/${KIND_VERSION}" + download "${kind_url}/${kind_filename}" "${kind_dir}/${kind_filename}" + download "${kind_url}/${kind_filename}.sha256sum" "${kind_dir}/${kind_filename}.sha256sum" + ( + cd "${kind_dir}" + grep "${kind_filename}" "${kind_filename}.sha256sum" | sha256sum --check - + mv "${kind_filename}" kind + rm "${kind_filename}.sha256sum" + chmod +x kind + ) + + kubectl_url="https://dl.k8s.io/release/${KUBECTL_VERSION}/bin/linux/amd64" + download "${kubectl_url}/kubectl" "${kubectl_dir}/kubectl" + download "${kubectl_url}/kubectl.sha256" "${kubectl_dir}/kubectl.sha256" + ( + cd "${kubectl_dir}" + echo "$(cat kubectl.sha256) kubectl" | sha256sum --check - + rm kubectl.sha256 + chmod +x kubectl + ) + + - name: Restore Helm chart dependencies + id: helm-dependencies-cache + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # zizmor: ignore[cache-poisoning] + with: + path: deployment/helm/charts/onyx/charts + key: craft-helm-dependencies-${{ hashFiles('deployment/helm/charts/onyx/Chart.yaml', 'deployment/helm/charts/onyx/Chart.lock') }}-${{ github.run_id }} + + - name: Download Helm chart dependencies + if: steps.helm-dependencies-cache.outputs.cache-hit != 'true' + run: | + set -euo pipefail + + retry() { + local attempt + for attempt in 1 2 3 4 5; do + if "$@"; then + return 0 + fi + if [ "${attempt}" -eq 5 ]; then + return 1 + fi + echo "Command failed (attempt ${attempt}/5); retrying ..." + sleep $((attempt * 5)) + done + } + + retry helm repo add --force-update ingress-nginx https://kubernetes.github.io/ingress-nginx + retry helm repo add --force-update opensearch https://opensearch-project.github.io/helm-charts + retry helm repo add --force-update cloudnative-pg https://cloudnative-pg.github.io/charts + retry helm repo add --force-update ot-container-kit https://ot-container-kit.github.io/helm-charts + retry helm repo add --force-update minio https://charts.min.io/ + retry helm repo add --force-update code-interpreter https://onyx-dot-app.github.io/python-sandbox/ + retry helm repo update + if ! retry helm dependency build --skip-refresh deployment/helm/charts/onyx; then + echo "helm dependency build failed; pulling disabled code-interpreter dependency directly" + code_interpreter_version=$(awk ' + $1 == "-" && $2 == "name:" && $3 == "code-interpreter" { found = 1 } + found && $1 == "version:" { print $2; exit } + ' deployment/helm/charts/onyx/Chart.yaml) + test -n "${code_interpreter_version}" + retry helm pull code-interpreter/code-interpreter \ + --version "${code_interpreter_version}" \ + --destination deployment/helm/charts/onyx/charts + fi + helm dependency list deployment/helm/charts/onyx + helm dependency list deployment/helm/charts/onyx \ + | awk 'NR > 1 && NF && $4 != "ok" { bad = 1 } END { exit bad }' + build-images: # Build both images in parallel (one matrix leg each) and push to the shared # ECR repo so each test shard pulls prebuilt images instead of cold-building. @@ -229,7 +346,7 @@ jobs: craft-k8s-tests: name: craft-k8s (${{ matrix.test-file.name }}) - needs: [changes, discover-test-files, build-images] + needs: [changes, discover-test-files, prepare-craft-assets, build-images] if: needs.changes.outputs.craft_k8s == 'true' # spot=false: this is a long lane (full kind cluster per shard); on-demand # avoids mid-run spot reclamation. Matches the compose lane. @@ -275,6 +392,20 @@ jobs: backend/requirements/dev.txt backend/requirements/ee.txt + - name: Restore kind tools + uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 + with: + path: ${{ runner.tool_cache }}/kind/${{ env.KIND_VERSION }}/amd64 + key: craft-kind-tools-${{ runner.os }}-${{ runner.arch }}-${{ env.KIND_VERSION }}-${{ env.KUBECTL_VERSION }}-${{ github.run_id }} + fail-on-cache-miss: true + + - name: Restore Helm chart dependencies + uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 + with: + path: deployment/helm/charts/onyx/charts + key: craft-helm-dependencies-${{ hashFiles('deployment/helm/charts/onyx/Chart.yaml', 'deployment/helm/charts/onyx/Chart.lock') }}-${{ github.run_id }} + fail-on-cache-miss: true + - name: Log in to ECR pull-through cache uses: ./.github/actions/login-ecr-pullthrough-cache with: @@ -318,6 +449,8 @@ jobs: - name: Create kind cluster uses: helm/kind-action@ef37e7f390d99f746eb8b610417061a60e82a6cc # ratchet:helm/kind-action@v1.14.0 with: + version: ${{ env.KIND_VERSION }} + kubectl_version: ${{ env.KUBECTL_VERSION }} cluster_name: onyx-craft-ci node_image: kindest/node:v1.33.1 config: ${{ runner.temp }}/kind-config.yaml @@ -337,28 +470,8 @@ jobs: - name: Label kind node for sandbox workload run: kubectl label node onyx-craft-ci-control-plane onyx.app/workload=sandbox - # `helm upgrade --install` validates Chart.yaml dependencies up front. - # Add the repos and build deps before installing the CI release. - - name: Build helm chart dependencies + - name: Validate Helm chart dependencies run: | - helm repo add ingress-nginx https://kubernetes.github.io/ingress-nginx - helm repo add opensearch https://opensearch-project.github.io/helm-charts - helm repo add cloudnative-pg https://cloudnative-pg.github.io/charts - helm repo add ot-container-kit https://ot-container-kit.github.io/helm-charts - helm repo add minio https://charts.min.io/ - helm repo add code-interpreter https://onyx-dot-app.github.io/python-sandbox/ - helm repo update - if ! helm dependency build deployment/helm/charts/onyx; then - echo "helm dependency build failed; pulling disabled code-interpreter dependency directly" - code_interpreter_version=$(awk ' - $1 == "-" && $2 == "name:" && $3 == "code-interpreter" { found = 1 } - found && $1 == "version:" { print $2; exit } - ' deployment/helm/charts/onyx/Chart.yaml) - test -n "${code_interpreter_version}" - helm pull code-interpreter/code-interpreter \ - --version "${code_interpreter_version}" \ - --destination deployment/helm/charts/onyx/charts - fi helm dependency list deployment/helm/charts/onyx helm dependency list deployment/helm/charts/onyx \ | awk 'NR > 1 && NF && $4 != "ok" { bad = 1 } END { exit bad }' diff --git a/backend/alembic/versions/17135ac06582_add_incognito_to_user_usage.py b/backend/alembic/versions/17135ac06582_add_incognito_to_user_usage.py new file mode 100644 index 00000000000..8f96f36cfc1 --- /dev/null +++ b/backend/alembic/versions/17135ac06582_add_incognito_to_user_usage.py @@ -0,0 +1,50 @@ +"""add incognito to user_usage + +Revision ID: 17135ac06582 +Revises: 3260759d6965 +Create Date: 2026-08-10 12:30:00.000000 + +""" + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = "17135ac06582" +down_revision = "3260759d6965" +branch_labels = None +depends_on = None + +UNIQUE_INDEX = "uq_user_usage_dims" +BASE_DIMENSIONS = ["user_id", "window_start", "model", "flow", "provider"] + + +def upgrade() -> None: + op.add_column( + "user_usage", + sa.Column( + "incognito", + sa.Boolean(), + nullable=False, + server_default=sa.false(), + ), + ) + # incognito joins the rollup dimension tuple, so the upsert's unique index + # must include it or two rows that differ only by mode would collide. + op.drop_index(UNIQUE_INDEX, table_name="user_usage") + op.create_index( + UNIQUE_INDEX, + "user_usage", + [*BASE_DIMENSIONS, "incognito"], + unique=True, + ) + + +def downgrade() -> None: + op.drop_index(UNIQUE_INDEX, table_name="user_usage") + # Incognito rows only exist because of this revision, and without the + # column they would collide with their ordinary counterparts on the + # narrower index. + op.execute(sa.text("DELETE FROM user_usage WHERE incognito")) + op.drop_column("user_usage", "incognito") + op.create_index(UNIQUE_INDEX, "user_usage", BASE_DIMENSIONS, unique=True) diff --git a/backend/alembic/versions/3260759d6965_add_incognito_to_user_file.py b/backend/alembic/versions/3260759d6965_add_incognito_to_user_file.py new file mode 100644 index 00000000000..8a0fd7f2a18 --- /dev/null +++ b/backend/alembic/versions/3260759d6965_add_incognito_to_user_file.py @@ -0,0 +1,48 @@ +"""add incognito columns to user_file + +Revision ID: 3260759d6965 +Revises: c7f1a9d4e206 +Create Date: 2026-08-10 13:00:00.000000 + +""" + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = "3260759d6965" +down_revision = "c7f1a9d4e206" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column( + "user_file", + sa.Column( + "incognito", + sa.Boolean(), + nullable=False, + server_default=sa.false(), + ), + ) + # No foreign key: the chat_session row is deleted at teardown and these + # rows must outlive it long enough for the orphan sweep to find them. + op.add_column( + "user_file", + sa.Column("incognito_session_id", sa.UUID(as_uuid=True), nullable=True), + ) + # Partial index for the stale-incognito sweep, tiny since incognito rows + # are short-lived. + op.create_index( + "ix_user_file_incognito_sweep", + "user_file", + ["incognito_session_id", "status", "last_accessed_at"], + postgresql_where=sa.text("incognito"), + ) + + +def downgrade() -> None: + op.drop_index("ix_user_file_incognito_sweep", table_name="user_file") + op.drop_column("user_file", "incognito_session_id") + op.drop_column("user_file", "incognito") diff --git a/backend/ee/onyx/connectors/capability_checks.py b/backend/ee/onyx/connectors/capability_checks.py index 53890fe6cfb..068215c3070 100644 --- a/backend/ee/onyx/connectors/capability_checks.py +++ b/backend/ee/onyx/connectors/capability_checks.py @@ -17,12 +17,20 @@ CapabilityCheckContext, CredentialCapability, ) +from onyx.connectors.slack.capability_checks import ( + build_slack_doc_permission_sync_checks, +) from onyx.connectors.source_operations import get_source_operations_class -# Named perm-sync checks per source. Empty at framework stage: per-connector -# work registers named checks here. -_DOC_PERMISSION_SYNC_CHECKS_BY_SOURCE: dict[DocumentSource, list[CapabilityCheck]] = {} +# Named perm-sync checks per source. Per-connector work registers named checks +# here. +_DOC_PERMISSION_SYNC_CHECKS_BY_SOURCE: dict[DocumentSource, list[CapabilityCheck]] = { + DocumentSource.SLACK: build_slack_doc_permission_sync_checks(), +} +# Slack registers nothing here by design: it has no group sync (channel access +# resolves usergroups to individual users, so there is no usergroup-to-document +# mapping). _EXTERNAL_GROUP_SYNC_CHECKS_BY_SOURCE: dict[DocumentSource, list[CapabilityCheck]] = {} diff --git a/backend/ee/onyx/db/query_history.py b/backend/ee/onyx/db/query_history.py index e882aedd3de..78509a61cbb 100644 --- a/backend/ee/onyx/db/query_history.py +++ b/backend/ee/onyx/db/query_history.py @@ -153,8 +153,9 @@ def fetch_chat_sessions_eagerly_by_time( asc_time_order: UnaryExpression = asc(ChatSession.time_created) message_order: UnaryExpression = asc(ChatMessage.id) + # Unfiltered on record mode: this backs the usage report, which carries + # token counts and no message content, and every mode meters usage. filters: list[ColumnElement | BinaryExpression] = [ - content_persisting_sessions_filter(), ChatSession.time_created.between(start, end), ] diff --git a/backend/ee/onyx/server/reporting/usage_export_generation.py b/backend/ee/onyx/server/reporting/usage_export_generation.py index a874d9e9859..af0bbc45ef2 100644 --- a/backend/ee/onyx/server/reporting/usage_export_generation.py +++ b/backend/ee/onyx/server/reporting/usage_export_generation.py @@ -155,6 +155,7 @@ def generate_usage_breakdown_report( "model", "flow", "provider", + "incognito", "input_tokens", "output_tokens", "cache_read_tokens", @@ -170,6 +171,7 @@ def generate_usage_breakdown_report( sanitize_csv_cell_or_none(row.model), sanitize_csv_cell_or_none(row.flow), sanitize_csv_cell_or_none(row.provider), + row.incognito, row.input_tokens, row.output_tokens, row.cache_read_tokens, diff --git a/backend/onyx/background/celery/tasks/user_file_processing/tasks.py b/backend/onyx/background/celery/tasks/user_file_processing/tasks.py index 7402f413f55..5f03d02e0e5 100644 --- a/backend/onyx/background/celery/tasks/user_file_processing/tasks.py +++ b/backend/onyx/background/celery/tasks/user_file_processing/tasks.py @@ -18,7 +18,10 @@ from onyx.background.celery.tasks.shared.RetryDocumentIndex import RetryDocumentIndex from onyx.cache.factory import get_cache_backend from onyx.chat.chat_processing_checker import is_chat_session_processing -from onyx.chat.incognito import delete_incognito_generated_files +from onyx.chat.incognito import ( + delete_incognito_generated_files, + sweep_stale_incognito_user_files, +) from onyx.chat.incognito_context import incognito_session_ended from onyx.configs.app_configs import ( DISABLE_VECTOR_DB, @@ -577,6 +580,10 @@ def _supply_user_file_to_secondary(user_file_id: str, tenant_id: str) -> bool: user_file = db_session.get(UserFile, _as_uuid(user_file_id)) file_id = user_file.file_id if user_file is not None else None file_name = user_file.name if user_file is not None else None + incognito = user_file is not None and user_file.incognito + # Incognito files never enter any index, so the flag clears with no write. + if incognito: + return True if secondary is None or file_id is None: return False @@ -674,6 +681,7 @@ def process_user_file_impl( file_id = uf.file_id file_name = uf.name + skip_search_index = uf.incognito # DB connection returned to pool here; file I/O and indexing run without it. try: @@ -681,7 +689,9 @@ def process_user_file_impl( user_file_id, file_id, file_name, tenant_id ) try: - if DISABLE_VECTOR_DB: + # Incognito uploads get text extraction for chat use but never + # enter the search index. + if DISABLE_VECTOR_DB or skip_search_index: _process_user_file_without_vector_db( user_file_id=user_file_id, documents=documents, @@ -800,6 +810,15 @@ def check_for_user_file_delete(self: Task, *, tenant_id: str) -> None: return None with get_session_with_current_tenant() as db_session: + # Orphaned incognito uploads (teardown never arrived) join the + # DELETING pool here so the standard machinery below cleans them. + stale_incognito = sweep_stale_incognito_user_files(db_session) + if stale_incognito: + db_session.commit() + task_logger.info( + f"check_for_user_file_delete - Queued {stale_incognito} " + f"stale incognito files for tenant={tenant_id}" + ) user_file_ids = ( db_session.execute( select(UserFile.id).where( @@ -943,17 +962,26 @@ def delete_user_file_impl( ) file_store = get_default_file_store() + blob_deleted = True try: - file_store.delete_file(file_id) + file_store.delete_file(file_id, error_on_missing=False) file_store.delete_file( - user_file_id_to_plaintext_file_name(_as_uuid(user_file_id)) + user_file_id_to_plaintext_file_name(_as_uuid(user_file_id)), + error_on_missing=False, ) except Exception as e: + blob_deleted = False task_logger.exception( f"delete_user_file_impl - Error deleting file id={user_file_id} - {e.__class__.__name__}" ) - # Phase 3: short write session — remove the DB record + # Phase 3: short write session, removing the DB record. The row is the + # only handle a retry has on the blob, so a refused delete keeps it. + if not blob_deleted: + task_logger.warning( + f"delete_user_file_impl - Keeping row id={user_file_id} for retry" + ) + return with get_session_with_current_tenant() as db_session: user_file = db_session.get(UserFile, _as_uuid(user_file_id)) if user_file is not None: diff --git a/backend/onyx/background/task_utils.py b/backend/onyx/background/task_utils.py index e30814fffb7..68e27fd1af5 100644 --- a/backend/onyx/background/task_utils.py +++ b/backend/onyx/background/task_utils.py @@ -163,6 +163,7 @@ def drain_delete_loop(tenant_id: str) -> None: from onyx.background.celery.tasks.user_file_processing.tasks import ( delete_user_file_impl, ) + from onyx.chat.incognito import sweep_stale_incognito_user_files from onyx.db.engine.sql_engine import get_session_with_current_tenant failed: set[UUID] = set() @@ -181,6 +182,16 @@ def drain_delete_loop(tenant_id: str) -> None: logger.exception("Failed to delete user file %s", file_id) failed.add(file_id) + # Last, and never fatal: this loop is the whole delete path on lite + # deployments, so a sweep that cannot reach Redis must not hold up ordinary + # deletion. What it queues is picked up on the next pass. + try: + with get_session_with_current_tenant() as session: + if sweep_stale_incognito_user_files(session): + session.commit() + except Exception: + logger.exception("Stale incognito sweep failed") + def drain_project_sync_loop(tenant_id: str) -> None: """Sync all pending project/persona metadata for user files.""" diff --git a/backend/onyx/chat/chat_utils.py b/backend/onyx/chat/chat_utils.py index 771d8fb7d1d..ee42b65d5b5 100644 --- a/backend/onyx/chat/chat_utils.py +++ b/backend/onyx/chat/chat_utils.py @@ -33,7 +33,11 @@ get_chat_messages_by_session, get_or_create_root_message, ) -from onyx.db.enums import IncognitoRecordMode, UserFileStatus +from onyx.db.enums import ( + IncognitoRecordMode, + UserFileStatus, + record_mode_persists_content, +) from onyx.db.file_record import FileRecordNotFoundError from onyx.db.kg_config import ( get_kg_config_settings, @@ -205,21 +209,33 @@ def create_chat_session_from_request( OnyxErrorCode.DEPLOYMENT_UNSUPPORTED, "Incognito chat is not supported on this deployment.", ) - if not incognito_allowed_for_user(user, db_session): + if not incognito_allowed_for_user(user, db_session, cached=False): raise OnyxError( OnyxErrorCode.UNAUTHORIZED, "Incognito chat is not enabled for this user.", ) incognito_mode = resolve_incognito_record_mode() - return create_chat_session( + # A caller-supplied title is conversation-derived, so a content-free + # session stores none of it. + description = ( + chat_session_request.description or "" + if record_mode_persists_content(incognito_mode) + else "" + ) + + chat_session = create_chat_session( db_session=db_session, - description=chat_session_request.description or "", + description=description, user_id=user.id, persona_id=chat_session_request.persona_id, project_id=chat_session_request.project_id, incognito_record_mode=incognito_mode, + session_id=( + chat_session_request.incognito_session_id if incognito_mode else None + ), ) + return chat_session def create_chat_history_chain( diff --git a/backend/onyx/chat/incognito.py b/backend/onyx/chat/incognito.py index cba7cea7cf7..68c872a0250 100644 --- a/backend/onyx/chat/incognito.py +++ b/backend/onyx/chat/incognito.py @@ -14,17 +14,30 @@ for the length of the session. """ +from typing import Any from uuid import UUID from sqlalchemy.orm import Session -from onyx.chat.incognito_context import incognito_context_available +from onyx.chat.incognito_context import ( + incognito_context_available, + incognito_session_ended, +) from onyx.db.enums import IncognitoRecordMode, record_mode_persists_content from onyx.db.file_record import get_incognito_file_ids -from onyx.db.incognito import user_in_incognito_enabled_group +from onyx.db.incognito import ( + is_content_persisting_session, + mark_incognito_user_files_deleting, + mark_unadopted_incognito_files_deleting, + stale_incognito_session_ids, + user_in_incognito_enabled_group, +) from onyx.db.models import User from onyx.file_store.file_store import get_default_file_store from onyx.file_store.models import FileDescriptor +from onyx.llm.constants import LlmProviderNames +from onyx.llm.interfaces import LlmRequestPolicy +from onyx.llm.well_known_providers.constants import BIFROST_PROVIDER_NAME from onyx.server.security.models import IncognitoAvailability from onyx.server.security.store import get_security_settings, load_effective_uncached from onyx.utils.logger import setup_logger @@ -38,19 +51,26 @@ def current_turn_persists_content() -> bool: return record_mode_persists_content(mode) -def incognito_allowed_for_user(user: User, db_session: Session) -> bool: +def incognito_allowed_for_user( + user: User, db_session: Session, *, cached: bool = True +) -> bool: """Whether this user may start an incognito chat. Availability composes the deployment capability (the ephemeral store must exist) with the admin's security setting, which defaults to off. Anonymous users never qualify: they share an identity, have no memberships, and cannot authenticate against the teardown endpoint. + + ``cached`` must be False wherever this decides an action rather than an + affordance. Cache invalidation is process-local, so a second api_server can + authorize against a revoked setting for the cache TTL. """ if user.is_anonymous: return False if not incognito_context_available(): return False - availability = get_security_settings().incognito_availability + settings = get_security_settings() if cached else load_effective_uncached() + availability = settings.incognito_availability if availability is IncognitoAvailability.EVERYONE: return True if availability is IncognitoAvailability.GROUPS: @@ -58,6 +78,18 @@ def incognito_allowed_for_user(user: User, db_session: Session) -> bool: return False +# Bifrost's per-request switch for keeping content out of its gateway log. +# Honored only when the gateway enables allow_per_request_content_storage_override. +# Ignored otherwise, so this stays best effort from Onyx's side. +BIFROST_DISABLE_CONTENT_LOGGING_HEADER = "x-bf-disable-content-logging" +# Portkey "DO NOT TRACK": request/response content stays out of its logs, +# token/cost stats still record. +PORTKEY_DEBUG_HEADER = "x-portkey-debug" +# LiteLLM proxy per-request redaction: content stripped from its logs while +# spend rows still write, which incognito usage metering requires. +LITELLM_PROXY_REDACTION_HEADER = "x-litellm-enable-message-redaction" + + def resolve_incognito_record_mode() -> IncognitoRecordMode: """The mode a new incognito session must pin: the workspace's admin record-mode setting, usage_only by default. @@ -70,6 +102,92 @@ def resolve_incognito_record_mode() -> IncognitoRecordMode: return load_effective_uncached().incognito_record_mode +def sweep_stale_incognito_user_files(db_session: Session) -> int: + """Queue uploads of dead incognito sessions for deletion. Caller commits. + + Covers both shapes: uploads no session ever adopted, and uploads whose + session is gone. A session whose live context is still present is skipped + however old its files are, so a chat left open past the orphan window keeps + its attachments. Shared by the Celery beat task and the lite deployment + poller, which have no scheduler in common. + """ + marked = mark_unadopted_incognito_files_deleting(db_session) + for session_id in stale_incognito_session_ids(db_session): + # Full-history sessions never create a Redis context, so liveness would + # read them as ended and delete a live chat's attachments. + if is_content_persisting_session(db_session, session_id): + continue + if not incognito_session_ended(session_id): + continue + marked += len(mark_incognito_user_files_deleting(db_session, session_id)) + return marked + + +def incognito_llm_extra_headers( + mode: IncognitoRecordMode | None, + provider: str | None, +) -> dict[str, str]: + """Headers an LLM request must carry under this recording mode. + + Only Bifrost honors a per-request retention switch. FULL_HISTORY sends + nothing: the workspace chose to record content, and the gateway log is the + workspace's own infrastructure. + + Pass the result as ``get_llm``'s ``policy_headers`` so it outranks request, + deployment-env, and provider header sources. Merged anywhere earlier, a + deployment-wide header could silently re-enable gateway content logging. + """ + if record_mode_persists_content(mode): + return {} + if provider == BIFROST_PROVIDER_NAME: + return {BIFROST_DISABLE_CONTENT_LOGGING_HEADER: "true"} + if provider == LlmProviderNames.PORTKEY.value: + return {PORTKEY_DEBUG_HEADER: "false"} + if provider == LlmProviderNames.LITELLM_PROXY.value: + return {LITELLM_PROXY_REDACTION_HEADER: "true"} + return {} + + +def incognito_llm_extra_body( + mode: IncognitoRecordMode | None, + provider: str | None, +) -> dict[str, Any]: + """Request-body params a content-free turn must carry, by provider. + + Merged last into model kwargs for the same reason the headers are: a + deployment-wide param must not re-enable provider-side retention. + """ + if record_mode_persists_content(mode): + return {} + # OpenAI Responses API stores by default for 30 days, and Chat Completions + # accepts the same param. Azure's stored-completions opt-in stays off. + if provider in ( + LlmProviderNames.OPENAI.value, + LlmProviderNames.AZURE.value, + ): + return {"store": False} + # Routes only to OpenRouter endpoints that do not retain user data. + if provider == LlmProviderNames.OPENROUTER.value: + return {"extra_body": {"provider": {"data_collection": "deny"}}} + return {} + + +def incognito_llm_request_policy( + mode: IncognitoRecordMode | None, + provider: str | None, +) -> LlmRequestPolicy: + """The per-provider retention suppression a turn under this mode carries. + + Providers with no per-request option (Anthropic, Google, Vertex, Mistral, + Bedrock, Nebius, local servers) get an empty policy: their retention is an + account or deployment concern, which the incognito disclaimer covers. + """ + return LlmRequestPolicy( + headers=incognito_llm_extra_headers(mode, provider), + model_kwargs=incognito_llm_extra_body(mode, provider), + ) + + def content_free_file_descriptors( file_descriptors: list[FileDescriptor], ) -> list[FileDescriptor]: diff --git a/backend/onyx/chat/process_message.py b/backend/onyx/chat/process_message.py index 37e02dde81d..d00189b841d 100644 --- a/backend/onyx/chat/process_message.py +++ b/backend/onyx/chat/process_message.py @@ -15,6 +15,7 @@ from concurrent.futures import ThreadPoolExecutor from contextvars import Token from enum import Enum +from functools import partial from typing import Final, cast from uuid import UUID @@ -41,6 +42,7 @@ from onyx.chat.emitter import Emitter from onyx.chat.incognito import ( content_free_file_descriptors, + incognito_llm_request_policy, ) from onyx.chat.incognito_context import ( append_incognito_message, @@ -691,12 +693,18 @@ def build_chat_turn( if is_multi else [new_msg_req.llm_override or chat_session.llm_override] ) + # Provider-keyed so the factory can apply it to whichever provider the + # persona resolution lands on, with final precedence over other sources. + incognito_policy_fn = partial( + incognito_llm_request_policy, chat_session.incognito_record_mode + ) for override in selected_overrides: llm = get_llm_for_persona( persona=persona, user=user, llm_override=override, additional_headers=litellm_additional_headers, + policy_fn=incognito_policy_fn, ) check_llm_cost_limit_for_provider( db_session=db_session, diff --git a/backend/onyx/connectors/capability_checks/registry.py b/backend/onyx/connectors/capability_checks/registry.py index 92f4009df19..f54178663d4 100644 --- a/backend/onyx/connectors/capability_checks/registry.py +++ b/backend/onyx/connectors/capability_checks/registry.py @@ -4,13 +4,16 @@ CapabilityCheckContext, CredentialCapability, ) +from onyx.connectors.slack.capability_checks import build_slack_indexing_checks from onyx.connectors.source_operations import get_source_operations_class from onyx.utils.variable_functionality import fetch_ee_implementation_or_noop # INDEXING checks per source. Checks must be enumerable without an instantiated # connector, hence a registry module rather than a ``BaseConnector`` method. -# Empty at framework stage: per-connector work registers named checks here. -_INDEXING_CHECKS_BY_SOURCE: dict[DocumentSource, list[CapabilityCheck]] = {} +# Per-connector work registers named checks here. +_INDEXING_CHECKS_BY_SOURCE: dict[DocumentSource, list[CapabilityCheck]] = { + DocumentSource.SLACK: build_slack_indexing_checks(), +} class _ConnectorSettingsFallbackCheck(CapabilityCheck): diff --git a/backend/onyx/connectors/capability_checks/runner.py b/backend/onyx/connectors/capability_checks/runner.py index 1cfbe4f3fef..fc1ea45d548 100644 --- a/backend/onyx/connectors/capability_checks/runner.py +++ b/backend/onyx/connectors/capability_checks/runner.py @@ -20,6 +20,7 @@ get_applicable_capabilities, get_capability_checks, ) +from onyx.connectors.credentials_provider import build_db_credentials_provider from onyx.connectors.exceptions import ( ConnectorValidationError, UnexpectedValidationError, @@ -27,6 +28,10 @@ from onyx.connectors.factory import identify_connector_class, instantiate_connector from onyx.connectors.interfaces import BaseConnector from onyx.connectors.models import InputType +from onyx.connectors.source_operations import ( + SourceOperations, + get_source_operations_class, +) from onyx.db.models import Credential from onyx.utils.credential_audit import emit_credential_access from onyx.utils.logger import setup_logger @@ -288,6 +293,17 @@ def generate_capability_report( e, ) + # Migrated sources always get their gateway constructed: construction is + # lazy (no client build or decrypt until an operation runs), and checks + # treat a registered-but-missing gateway as a programmer error. + source_operations: SourceOperations | None = None + source_operations_class = get_source_operations_class(source) + if source_operations_class is not None: + source_operations = source_operations_class( + credentials_provider=build_db_credentials_provider(source, credential.id), + connector_specific_config=connector_specific_config, + ) + if credential.credential_json: # Distinct decrypt site from ``instantiate_connector``'s paths (which # may not have decrypted at all when instantiation fails). Audit is @@ -311,6 +327,7 @@ def generate_capability_report( # than probe an empty dict. connector_specific_config=connector_specific_config, instantiation_error=instantiation_error, + source_operations=source_operations, ) results = run_capability_checks(checks, context) return CredentialCapabilityReport( diff --git a/backend/onyx/connectors/slack/capability_checks.py b/backend/onyx/connectors/slack/capability_checks.py new file mode 100644 index 00000000000..fc62a5a827f --- /dev/null +++ b/backend/onyx/connectors/slack/capability_checks.py @@ -0,0 +1,882 @@ +"""Capability checks for the Slack connector. + +Each check probes one permission assumption the connector makes at indexing or +doc-permission-sync time by composing the same gateway operations production +code calls (``limit=1`` where possible). Checks need no connector instance: the +runner constructs the registered ``SlackSourceOperations`` gateway from the +credential and hands it to every check via the context, so checks run +config-less at credential-creation time (except the configured-channels check, +which declares its config requirement and is skipped until one exists). + +Slack registers no EXTERNAL_GROUP_SYNC checks: group sync is unregistered for +Slack by design (``sync_params.py``), so that capability stays NOT_APPLICABLE. + +Scope-to-capability mapping (see also the OAuth scope list in +``ee/onyx/server/oauth/slack.py``): + +- ``channels:read`` -> INDEXING (public channel listing and metadata), + DOC_PERMISSION_SYNC (channel enumeration for ACLs) +- ``groups:read`` -> INDEXING (private channels), DOC_PERMISSION_SYNC + (private member ACLs); both silently fall back to public-only without it +- ``channels:history`` / ``groups:history`` -> INDEXING (message and thread + reads) +- ``channels:join`` -> INDEXING (self-join public channels) +- ``team:read`` -> INDEXING and DOC_PERMISSION_SYNC on Enterprise Grid + (workspace enumeration) +- ``users:read`` -> INDEXING (author names; degrades to raw ids), + DOC_PERMISSION_SYNC (member-to-profile resolution) +- ``users:read.email`` -> DOC_PERMISSION_SYNC (member-to-email resolution) +""" + +from typing import Any, NoReturn + +from onyx.connectors.capability_checks.models import ( + CapabilityCheck, + CapabilityCheckContext, + CredentialCapability, +) +from onyx.connectors.exceptions import ( + ConnectorValidationError, + CredentialExpiredError, + CredentialInvalidError, + InsufficientPermissionsError, + UnexpectedValidationError, +) +from onyx.connectors.slack.connector import ( + get_channels, + get_channels_across_teams, + list_grid_team_ids, +) +from onyx.connectors.slack.source_operations import ( + SlackApiError, + SlackAuthTestResponse, + SlackChannelVariant, + SlackSourceOperations, +) + +_SLACK_DOCS_LINK = ( + "https://docs.onyx.app/admins/connectors/official/slack/slack_indexed" +) + +_ADD_SCOPE_REMEDIATION = ( + "Add the `{scope}` bot scope under OAuth & Permissions in the Slack app " + "settings and reinstall the app to the workspace." +) + +# Page size when scanning for a channel to probe message history against; a +# single page is plenty since any public channel works for the probe. +_HISTORY_PROBE_CHANNEL_PAGE_SIZE = 20 + +# Page size when sampling users for email visibility. Workspaces commonly have +# many bot/deactivated users, so sample enough to nearly always include a human +# member. +_EMAIL_PROBE_PAGE_SIZE = 100 + +# Hang guard for the full-workspace channel enumeration, the one multi-page +# probe: triple the 600s ``CAPABILITY_CHECK_TIMEOUT_SECONDS`` default, since the +# gateway's redis-coordinated client also serves any concurrently running +# indexing job's rate-limit backoff. +_CHANNEL_ENUMERATION_TIMEOUT_SECONDS = 1800.0 + + +def _slack_client(context: CapabilityCheckContext) -> SlackSourceOperations: + """Returns the gateway the runner constructed for this run. + + A registered gateway that was not constructed is a programmer error in the + runner, not a check outcome. + """ + assert isinstance(context.source_operations, SlackSourceOperations), ( + "Bug: The runner constructs the registered gateway for migrated sources." + ) + return context.source_operations + + +def _raise_for_slack_api_error( + e: SlackApiError, missing_scope_message: str +) -> NoReturn: + """Maps a Slack API error onto the validation-exception family. + + ``missing_scope_message`` is the check-specific explanation used when the + error is ``missing_scope``. + """ + error = e.response.get("error", "") if e.response is not None else "" + if error == "missing_scope": + needed = e.response.get("needed", "") + message = missing_scope_message + if needed: + message += f" (Slack reported the missing scope as `{needed}`.)" + raise InsufficientPermissionsError(message) from e + if error in ("invalid_auth", "not_authed"): + raise CredentialExpiredError( + f"Invalid or expired Slack bot token ({error})." + ) from e + if error == "account_inactive": + raise CredentialExpiredError( + f"Slack workspace or bot user is deactivated ({error})." + ) from e + if error == "token_revoked": + raise CredentialExpiredError( + f"Slack bot token has been revoked ({error})." + ) from e + if error == "token_expired": + raise CredentialExpiredError(f"Slack bot token has expired ({error}).") from e + if error == "ratelimited": + raise UnexpectedValidationError( + "Slack rate limited the check; re-run the checks in a minute." + ) from e + raise UnexpectedValidationError(f"Unexpected Slack error `{error}`.") from e + + +def _auth_test(slack_client: SlackSourceOperations) -> SlackAuthTestResponse: + try: + return slack_client.check_auth() + except SlackApiError as e: + _raise_for_slack_api_error(e, "Slack bot token failed `auth.test`.") + + +def _first_grid_team_id(slack_client: SlackSourceOperations) -> str | None: + """Returns the first workspace id of an Enterprise Grid org, None off-Grid. + + On Grid org installs ``users.list`` requires a ``team_id``, so user-facing + checks call this to mirror the production call shape. + """ + if not _auth_test(slack_client).enterprise_id: + return None + try: + teams_page = next(slack_client.list_teams(limit=1)) + except SlackApiError as e: + _raise_for_slack_api_error( + e, + "On Enterprise Grid, listing users requires enumerating workspaces " + "first (`auth.teams.list`), which needs the `team:read` scope.", + ) + teams = teams_page.teams + if not teams: + raise UnexpectedValidationError( + "`auth.teams.list` returned no workspaces for this Enterprise Grid org." + ) + return str(teams[0]["id"]) + + +def _first_private_channel( + slack_client: SlackSourceOperations, +) -> dict[str, Any] | None: + """Returns one private channel the bot is in, None when none are in scope. + + None covers an empty listing and a ``missing_scope`` failure alike: without + ``groups:read`` no private channels are in scope, and the non-required + private-listing checks own that finding. Any other listing error maps + through ``_raise_for_slack_api_error``; a rate-limited listing must not turn + into a pass of a required check. + """ + try: + listing = next( + slack_client.list_channels( + variant=SlackChannelVariant.PRIVATE, + channel_types=["private_channel"], + exclude_archived=True, + limit=1, + ) + ) + except SlackApiError as e: + error = e.response.get("error", "") if e.response is not None else "" + if error == "missing_scope": + return None + _raise_for_slack_api_error(e, "The bot token cannot list private channels.") + channels = listing.channels + return channels[0] if channels else None + + +def _probe_channel_history( + slack_client: SlackSourceOperations, + variant: SlackChannelVariant, + channel_id: str, + history_scope_message: str, + replies_scope_message: str, +) -> None: + """Reads one message page, then one thread, from the channel. + + ``not_in_channel`` passes: membership is not scope, and indexing self-joins + public channels. + """ + try: + history_page = next( + slack_client.fetch_channel_history( + variant=variant, channel_id=channel_id, limit=1 + ) + ) + except SlackApiError as e: + if e.response is not None and e.response.get("error") == "not_in_channel": + return + _raise_for_slack_api_error(e, history_scope_message) + messages = history_page.messages + if not messages: + # An empty channel proves history access; there is no thread to read. + return + thread_ts = messages[0].get("ts") + if not thread_ts: + return + try: + next( + slack_client.fetch_thread_replies( + variant=variant, channel_id=channel_id, thread_ts=thread_ts + ) + ) + except SlackApiError as e: + _raise_for_slack_api_error(e, replies_scope_message) + + +class _TokenAuthCheck(CapabilityCheck): + """Rejects non-bot token shapes, then probes ``auth.test``.""" + + def __init__(self) -> None: + super().__init__( + capability=CredentialCapability.INDEXING, + check_id="slack_token_auth", + display_name="Bot token is valid", + requires_connector_instance=False, + remediation=( + "Create a bot token (`xoxb-...`) for the Slack app and paste " + "it into the credential." + ), + docs_link=_SLACK_DOCS_LINK, + ) + + def run(self, context: CapabilityCheckContext) -> None: + token = context.credential_json.get("slack_bot_token") + if not token or not isinstance(token, str): + raise CredentialInvalidError( + "Slack credential is missing the `slack_bot_token` field." + ) + if not token.startswith("xoxb-"): + raise CredentialInvalidError( + "The Slack credential does not look like a bot token (expected " + "an `xoxb-` prefix). User and app-level tokens cannot join and " + "read channels the way indexing requires." + ) + _auth_test(_slack_client(context)) + + +class _PublicChannelListingCheck(CapabilityCheck): + """Lists one public channel, then reads its metadata. + + The ``conversations.info`` probe mirrors checkpoint resume, which + re-resolves channels by id. + """ + + def __init__(self) -> None: + super().__init__( + capability=CredentialCapability.INDEXING, + check_id="slack_public_channel_listing", + display_name="Public channels can be listed", + requires_connector_instance=False, + remediation=_ADD_SCOPE_REMEDIATION.format(scope="channels:read"), + docs_link=_SLACK_DOCS_LINK, + ) + + def run(self, context: CapabilityCheckContext) -> None: + slack_client = _slack_client(context) + try: + listing = next( + slack_client.list_channels( + variant=SlackChannelVariant.PUBLIC, + channel_types=["public_channel"], + limit=1, + ) + ) + except SlackApiError as e: + _raise_for_slack_api_error( + e, + "The bot token cannot list public channels " + "(`conversations.list`), which indexing requires to enumerate " + "what to index.", + ) + channels = listing.channels + if not channels: + # Listing itself is proven; an empty workspace leaves no channel + # whose metadata could be read. + return + try: + slack_client.fetch_channel_info(channel_id=channels[0]["id"]) + except SlackApiError as e: + _raise_for_slack_api_error( + e, + "The bot token cannot read channel metadata " + "(`conversations.info`), which indexing uses to resolve " + "channels when resuming from a checkpoint.", + ) + + +class _PrivateChannelListingCheck(CapabilityCheck): + """Lists one private channel. + + Not required: indexing does not fail without ``groups:read``, it silently + falls back to public channels only. + """ + + def __init__(self) -> None: + super().__init__( + capability=CredentialCapability.INDEXING, + check_id="slack_private_channel_listing", + display_name="Private channels can be listed", + required=False, + requires_connector_instance=False, + remediation=_ADD_SCOPE_REMEDIATION.format(scope="groups:read"), + docs_link=_SLACK_DOCS_LINK, + ) + + def run(self, context: CapabilityCheckContext) -> None: + try: + next( + _slack_client(context).list_channels( + variant=SlackChannelVariant.PRIVATE, + channel_types=["private_channel"], + limit=1, + ) + ) + except SlackApiError as e: + _raise_for_slack_api_error( + e, + "The bot token cannot list private channels. Indexing does NOT " + "fail on this: it silently falls back to public channels only, " + "so private channels would be skipped without any error.", + ) + + +class _MessageHistoryReadCheck(CapabilityCheck): + """Reads one message page and one thread from a public channel.""" + + def __init__(self) -> None: + super().__init__( + capability=CredentialCapability.INDEXING, + check_id="slack_message_history_read", + display_name="Channel messages can be read", + requires_connector_instance=False, + remediation=_ADD_SCOPE_REMEDIATION.format(scope="channels:history"), + docs_link=_SLACK_DOCS_LINK, + ) + + def run(self, context: CapabilityCheckContext) -> None: + slack_client = _slack_client(context) + try: + listing = next( + slack_client.list_channels( + variant=SlackChannelVariant.PUBLIC, + channel_types=["public_channel"], + exclude_archived=True, + limit=_HISTORY_PROBE_CHANNEL_PAGE_SIZE, + ) + ) + except SlackApiError as e: + _raise_for_slack_api_error( + e, + "The bot token cannot list public channels " + "(`conversations.list`), which indexing requires to enumerate " + "what to index.", + ) + channels = listing.channels + if not channels: + raise UnexpectedValidationError( + "No public channels are visible to the bot, so message-history " + "access could not be probed." + ) + # Prefer a channel the bot is a member of; on a non-member channel the + # probe still proves the scope because ``not_in_channel`` sorts after + # ``missing_scope``. + channel = next( + (channel for channel in channels if channel.get("is_member")), + channels[0], + ) + _probe_channel_history( + slack_client, + SlackChannelVariant.PUBLIC, + channel["id"], + history_scope_message=( + "The bot token cannot read channel messages " + "(`conversations.history`), which is the core indexing " + "operation. Grant `channels:history` (and `groups:history` if " + "private channels should be indexed)." + ), + replies_scope_message=( + "The bot token cannot read thread replies " + "(`conversations.replies`), which indexing needs to capture " + "threads. Grant `channels:history` (and `groups:history` if " + "private channels should be indexed)." + ), + ) + + +class _PrivateMessageHistoryReadCheck(CapabilityCheck): + """Reads one message page and one thread from a private channel. + + A token with ``channels:history`` but not ``groups:history`` passes the + public probe while every private channel the bot is in fails mid-indexing. + Listing scope failures are deliberately not reported here: they belong to + ``slack_private_channel_listing``, and without ``groups:read`` no private + channels are in scope at all. + """ + + def __init__(self) -> None: + super().__init__( + capability=CredentialCapability.INDEXING, + check_id="slack_private_message_history_read", + display_name="Private-channel messages can be read", + requires_connector_instance=False, + remediation=_ADD_SCOPE_REMEDIATION.format(scope="groups:history"), + docs_link=_SLACK_DOCS_LINK, + ) + + def run(self, context: CapabilityCheckContext) -> None: + slack_client = _slack_client(context) + channel = _first_private_channel(slack_client) + if channel is None: + return + _probe_channel_history( + slack_client, + SlackChannelVariant.PRIVATE, + channel["id"], + history_scope_message=( + "The bot token cannot read private-channel messages " + "(`conversations.history`). Private channels ARE visible to " + "the bot, so indexing will attempt them and fail. Grant " + "`groups:history`." + ), + replies_scope_message=( + "The bot token cannot read private-channel thread replies " + "(`conversations.replies`), which indexing needs to capture " + "threads. Grant `groups:history`." + ), + ) + + +class _ChannelJoinScopeCheck(CapabilityCheck): + """Verifies the ``channels:join`` scope via the ``X-OAuth-Scopes`` header. + + Header-based instead of a functional probe: actually calling + ``conversations.join`` would join a channel in the customer's workspace as a + side effect (the operation carries a matching ``untested`` annotation). Not + required: a bot manually invited to every configured channel indexes fine + without the scope. + """ + + def __init__(self) -> None: + super().__init__( + capability=CredentialCapability.INDEXING, + check_id="slack_channel_join_scope", + display_name="Bot can join public channels", + required=False, + requires_connector_instance=False, + remediation=( + _ADD_SCOPE_REMEDIATION.format(scope="channels:join") + + " Alternatively, manually invite the bot (`/invite @`) " + "to every channel that should be indexed. Do not dismiss this " + "warning unless the bot is already a member of every such " + "channel." + ), + docs_link=_SLACK_DOCS_LINK, + ) + + def run(self, context: CapabilityCheckContext) -> None: + auth_response = _auth_test(_slack_client(context)) + granted_scopes = auth_response.granted_scopes + if granted_scopes is None: + raise UnexpectedValidationError( + "Slack did not return the `X-OAuth-Scopes` header, so the " + "`channels:join` scope could not be verified." + ) + if "channels:join" not in granted_scopes: + raise InsufficientPermissionsError( + "The bot token lacks the `channels:join` scope, so Onyx cannot " + "automatically join public channels. This is a warning, but it " + "usually still requires action: every public channel the bot " + "has not been invited to WILL fail to index. Either add the " + "`channels:join` scope and reinstall the app, or manually " + "invite the bot (`/invite @`) to every channel that " + "should be indexed." + ) + + +class _GridWorkspaceListingCheck(CapabilityCheck): + """Lists one Grid workspace when the org is an Enterprise Grid.""" + + def __init__(self) -> None: + super().__init__( + capability=CredentialCapability.INDEXING, + check_id="slack_grid_workspace_listing", + display_name="Enterprise Grid workspaces can be listed", + requires_connector_instance=False, + remediation=_ADD_SCOPE_REMEDIATION.format(scope="team:read"), + docs_link=_SLACK_DOCS_LINK, + ) + + def run(self, context: CapabilityCheckContext) -> None: + slack_client = _slack_client(context) + if not _auth_test(slack_client).enterprise_id: + return + try: + next(slack_client.list_teams(limit=1)) + except SlackApiError as e: + _raise_for_slack_api_error( + e, + "Slack Enterprise Grid org detected, but the bot token cannot " + "list workspaces (`auth.teams.list`). Channel enumeration is " + "per-workspace on Grid, so indexing coverage would be " + "incomplete.", + ) + + +class _UserProfileReadCheck(CapabilityCheck): + """Lists one user, then reads one profile. + + Not required: indexing still works without ``users:read``, but message + authors appear as raw Slack ids instead of names. + """ + + def __init__(self) -> None: + super().__init__( + capability=CredentialCapability.INDEXING, + check_id="slack_user_profile_read", + display_name="User profiles can be read", + required=False, + requires_connector_instance=False, + remediation=_ADD_SCOPE_REMEDIATION.format(scope="users:read"), + docs_link=_SLACK_DOCS_LINK, + ) + + def run(self, context: CapabilityCheckContext) -> None: + slack_client = _slack_client(context) + team_id = _first_grid_team_id(slack_client) + try: + users_page = next(slack_client.list_users(limit=1, team_id=team_id)) + except SlackApiError as e: + _raise_for_slack_api_error( + e, + "The bot token cannot read user profiles (`users.list`). " + "Indexing still works, but message authors appear as raw Slack " + "ids instead of names.", + ) + members = users_page.members + if not members: + return + try: + slack_client.fetch_user_info(members[0]["id"]) + except SlackApiError as e: + _raise_for_slack_api_error( + e, + "The bot token cannot read a single user profile " + "(`users.info`), which indexing uses to resolve message " + "authors. Authors would appear as raw Slack ids instead of " + "names.", + ) + + +class _ConfiguredChannelsVisibleCheck(CapabilityCheck): + """Verifies every configured channel name is visible to the bot. + + Existed only as commented-out code in ``validate_connector_settings`` + (removed as too slow for a synchronous request); resurrected here where slow + runs are acceptable. Composes the same enumeration the connector runs + (``get_channels`` / ``get_channels_across_teams``). + """ + + def __init__(self) -> None: + super().__init__( + capability=CredentialCapability.INDEXING, + check_id="slack_configured_channels_visible", + display_name="Configured channels are visible to the bot", + requires_connector_instance=False, + requires_connector_config=True, + timeout_seconds=_CHANNEL_ENUMERATION_TIMEOUT_SECONDS, + remediation=( + "Fix the channel name, or invite the bot to the private " + "channel (`/invite @` in Slack) and grant `groups:read`." + ), + docs_link=_SLACK_DOCS_LINK, + ) + + def run(self, context: CapabilityCheckContext) -> None: + config = context.connector_specific_config or {} + channels_to_include = config.get("channels") + if not channels_to_include: + # No channel filter configured; whatever is visible gets indexed. + return + if config.get("channel_regex_enabled"): + # Regex includes match dynamically; existence cannot be pre-checked. + return + slack_client = _slack_client(context) + try: + if _auth_test(slack_client).enterprise_id: + team_ids = list_grid_team_ids(slack_client) + all_channels = get_channels_across_teams( + slack_client=slack_client, team_ids=team_ids + ) + else: + all_channels = get_channels(slack_client=slack_client) + except SlackApiError as e: + _raise_for_slack_api_error( + e, + "The bot token cannot enumerate channels " + "(`conversations.list`), so the configured channels could not " + "be verified.", + ) + visible_names = {channel["name"] for channel in all_channels} + configured_names = [str(name).removeprefix("#") for name in channels_to_include] + missing = sorted(set(configured_names) - visible_names) + if missing: + raise ConnectorValidationError( + f"Configured channels are not visible to the bot: {missing}. " + "Each may be a typo, an archived channel, or a private channel " + "the bot has not been invited to (private channels also " + "require the `groups:read` scope to be listed)." + ) + + +class _PermSyncChannelListingCheck(CapabilityCheck): + """Lists one channel under the permission-sync capability. + + Doc sync enumerates every channel to build per-channel access lists and + fails outright when it cannot; this is the same ``channels:read`` scope the + INDEXING listing check probes, verified under the capability whose verdict + depends on it. + """ + + def __init__(self) -> None: + super().__init__( + capability=CredentialCapability.DOC_PERMISSION_SYNC, + check_id="slack_perm_sync_channel_listing", + display_name="Channels can be listed for permission sync", + requires_connector_instance=False, + remediation=_ADD_SCOPE_REMEDIATION.format(scope="channels:read"), + docs_link=_SLACK_DOCS_LINK, + ) + + def run(self, context: CapabilityCheckContext) -> None: + try: + next( + _slack_client(context).list_channels( + variant=SlackChannelVariant.PUBLIC, + channel_types=["public_channel"], + limit=1, + ) + ) + except SlackApiError as e: + _raise_for_slack_api_error( + e, + "The bot token cannot list channels (`conversations.list`), " + "which permission sync requires to enumerate channels and " + "build per-channel access lists.", + ) + + +class _PermSyncPrivateChannelListingCheck(CapabilityCheck): + """Lists one private channel under the permission-sync capability. + + Not required: doc sync has the same silent public-only fallback indexing has + when private channels cannot be listed, so the capability degrades rather + than breaks. + """ + + def __init__(self) -> None: + super().__init__( + capability=CredentialCapability.DOC_PERMISSION_SYNC, + check_id="slack_perm_sync_private_channel_listing", + display_name="Private channels can be listed for permission sync", + required=False, + requires_connector_instance=False, + remediation=_ADD_SCOPE_REMEDIATION.format(scope="groups:read"), + docs_link=_SLACK_DOCS_LINK, + ) + + def run(self, context: CapabilityCheckContext) -> None: + try: + next( + _slack_client(context).list_channels( + variant=SlackChannelVariant.PRIVATE, + channel_types=["private_channel"], + limit=1, + ) + ) + except SlackApiError as e: + _raise_for_slack_api_error( + e, + "The bot token cannot list private channels. Permission sync " + "does NOT fail on this: it silently syncs public channels " + "only, so private-channel access lists are never created or " + "updated. Previously indexed private documents keep stale " + "access lists, so users removed from a private channel keep " + "access in Onyx.", + ) + + +class _UserEmailVisibilityCheck(CapabilityCheck): + """Samples users and requires at least one visible email address.""" + + def __init__(self) -> None: + super().__init__( + capability=CredentialCapability.DOC_PERMISSION_SYNC, + check_id="slack_user_email_visibility", + display_name="User emails are visible", + requires_connector_instance=False, + remediation=( + _ADD_SCOPE_REMEDIATION.format(scope="users:read.email") + + " Slack only returns email fields once the app is " + "reinstalled." + ), + docs_link=_SLACK_DOCS_LINK, + ) + + def run(self, context: CapabilityCheckContext) -> None: + slack_client = _slack_client(context) + team_id = _first_grid_team_id(slack_client) + try: + users_page = next( + slack_client.list_users(limit=_EMAIL_PROBE_PAGE_SIZE, team_id=team_id) + ) + except SlackApiError as e: + _raise_for_slack_api_error( + e, + "The bot token cannot list users (`users.list`), which " + "permission sync requires to map Slack members to Onyx users " + "by email.", + ) + human_members = [ + member + for member in users_page.members + if not member.get("is_bot") + and not member.get("deleted") + and member.get("id") != "USLACKBOT" + ] + if not human_members: + raise UnexpectedValidationError( + "`users.list` returned no active human members to sample, so " + "email visibility could not be verified." + ) + if not any(member.get("profile", {}).get("email") for member in human_members): + raise InsufficientPermissionsError( + "`users.list` succeeded but returned no email addresses, so " + "permission sync cannot map Slack members to Onyx users. This " + "is how a missing `users:read.email` scope manifests -- Slack " + "omits the email field instead of raising a scope error." + ) + + +class _PrivateChannelMemberListingCheck(CapabilityCheck): + """Lists one private channel's members, then resolves one member. + + The ``users.info`` step mirrors doc sync's fallback for members missing from + the workspace user list (external users). Listing scope failures pass here: + doc sync silently degrades to public-only without ``groups:read``, and + ``slack_perm_sync_private_channel_listing`` owns that warning. + """ + + def __init__(self) -> None: + super().__init__( + capability=CredentialCapability.DOC_PERMISSION_SYNC, + check_id="slack_private_channel_member_listing", + display_name="Private-channel members can be listed", + requires_connector_instance=False, + remediation=( + _ADD_SCOPE_REMEDIATION.format(scope="groups:read") + + " Member-to-user resolution also needs `users:read`." + ), + docs_link=_SLACK_DOCS_LINK, + ) + + def run(self, context: CapabilityCheckContext) -> None: + slack_client = _slack_client(context) + channel = _first_private_channel(slack_client) + if channel is None: + return + try: + members_page = next( + slack_client.list_channel_members(channel_id=channel["id"]) + ) + except SlackApiError as e: + _raise_for_slack_api_error( + e, + "The bot token cannot list private-channel members " + "(`conversations.members`), which permission sync requires to " + "build per-channel member ACLs.", + ) + members = members_page.members + if not members: + return + try: + slack_client.fetch_user_info(members[0]) + except SlackApiError as e: + _raise_for_slack_api_error( + e, + "The bot token cannot resolve channel members to user " + "profiles (`users.info`), which permission sync uses for " + "members missing from the workspace user list.", + ) + + +class _GridPublicChannelScopingCheck(CapabilityCheck): + """Verifies per-workspace user listing on Enterprise Grid. + + Not required: without it, permission sync still runs but silently marks + every public channel visible org-wide in Onyx. + """ + + def __init__(self) -> None: + super().__init__( + capability=CredentialCapability.DOC_PERMISSION_SYNC, + check_id="slack_grid_public_channel_scoping", + display_name="Grid public channels can be workspace-scoped", + required=False, + requires_connector_instance=False, + remediation=( + _ADD_SCOPE_REMEDIATION.format(scope="users:read") + + " Also grant `users:read.email` and `team:read` so " + "per-workspace user lists can be built." + ), + docs_link=_SLACK_DOCS_LINK, + ) + + def run(self, context: CapabilityCheckContext) -> None: + slack_client = _slack_client(context) + team_id = _first_grid_team_id(slack_client) + if team_id is None: + # Off-Grid, public channels are workspace-public by design. + return + try: + next(slack_client.list_users(limit=1, team_id=team_id)) + except SlackApiError as e: + _raise_for_slack_api_error( + e, + "On Enterprise Grid, per-workspace user listing (`users.list` " + "with `team_id`) scopes public channels to their workspaces. " + "Without it, permission sync silently marks every public " + "channel visible org-wide in Onyx (over-sharing).", + ) + + +def build_slack_indexing_checks() -> list[CapabilityCheck]: + """Returns the INDEXING capability checks for Slack.""" + return [ + _TokenAuthCheck(), + _PublicChannelListingCheck(), + _PrivateChannelListingCheck(), + _MessageHistoryReadCheck(), + _PrivateMessageHistoryReadCheck(), + _ChannelJoinScopeCheck(), + _GridWorkspaceListingCheck(), + _UserProfileReadCheck(), + _ConfiguredChannelsVisibleCheck(), + ] + + +def build_slack_doc_permission_sync_checks() -> list[CapabilityCheck]: + """Returns the DOC_PERMISSION_SYNC capability checks for Slack. + + Registered via the EE capability-check hook; defined here so the probe + logic and remediation text live with the connector. + """ + return [ + _PermSyncChannelListingCheck(), + _PermSyncPrivateChannelListingCheck(), + _UserEmailVisibilityCheck(), + _PrivateChannelMemberListingCheck(), + _GridPublicChannelScopingCheck(), + ] diff --git a/backend/onyx/connectors/slack/connector.py b/backend/onyx/connectors/slack/connector.py index 1178e57ec5e..d78f20706a6 100644 --- a/backend/onyx/connectors/slack/connector.py +++ b/backend/onyx/connectors/slack/connector.py @@ -1399,6 +1399,11 @@ def validate_connector_settings(self) -> None: Channel existence (for non-regex includes) is validated during indexing via filter_channels, not here. """ + # Config-shape validation, load-bearing at creation time: unlike the + # credential probes below (mirrored as named capability checks in + # ``slack/capability_checks.py``), regex compilation is + # credential-invariant and has NO capability-check counterpart. This is + # the only thing that blocks a malformed regex from being created. if self.channel_regex_enabled: _validate_channel_regexes(self.channels, "channel") if self.exclude_channel_regex_enabled: diff --git a/backend/onyx/connectors/slack/source_operations.py b/backend/onyx/connectors/slack/source_operations.py index 4dd0524473d..8142cceccd6 100644 --- a/backend/onyx/connectors/slack/source_operations.py +++ b/backend/onyx/connectors/slack/source_operations.py @@ -68,8 +68,6 @@ # Timeout for the uncoordinated client behind ``fast=True`` operations. _FAST_TIMEOUT = 1 -_TEMPORARILY_UNTESTED = "Not yet tested: checks land in the Slack capability checks PR." - class SlackChannelVariant(str, Enum): """Permission class of a channel-scoped operation call. @@ -440,6 +438,10 @@ class SlackAuthTestResponse(SlackResponseModel): error: str | None = None url: str | None = None enterprise_id: str | None = None + # Parsed from the ``X-OAuth-Scopes`` response header, not the payload; None + # when the header is absent. Lets checks verify scopes whose functional + # probe would have side effects (``channels:join``). + granted_scopes: list[str] | None = None class SlackOkResponse(SlackResponseModel): @@ -495,6 +497,20 @@ def _validated(response: SlackResponse, model: type[_ResponseT]) -> _ResponseT: return model.model_validate(response.data) +def _parse_granted_scopes(headers: dict[str, Any] | None) -> list[str] | None: + """Parses the ``X-OAuth-Scopes`` response header; None when absent. + + urllib may surface a header value as a list; tolerate both shapes, like the + retry handler does for ``retry-after``. + """ + for key, value in (headers or {}).items(): + if str(key).lower() != "x-oauth-scopes": + continue + raw = value[0] if isinstance(value, list) else value + return [scope.strip() for scope in str(raw).split(",") if scope.strip()] + return None + + class SlackSourceOperationsConfig(BaseModel): """The slice of Slack connector config the gateway consumes. @@ -586,11 +602,16 @@ def _client_for(self, fast: bool) -> WebClient: CredentialCapability.DOC_PERMISSION_SYNC, }, consumes=OperationConsumes.CREDENTIAL, - untested=_TEMPORARILY_UNTESTED, ) def check_auth(self, *, fast: bool = False) -> SlackAuthTestResponse: - """``auth.test``: token validity, workspace url, Grid enterprise id.""" - return _validated(self._client_for(fast).auth_test(), SlackAuthTestResponse) + """ + ``auth.test``: token validity, workspace url, Grid enterprise id, and + the granted bot scopes (from the ``X-OAuth-Scopes`` response header). + """ + response = self._client_for(fast).auth_test() + result = _validated(response, SlackAuthTestResponse) + result.granted_scopes = _parse_granted_scopes(response.headers) + return result @source_operation( capabilities={ @@ -599,7 +620,6 @@ def check_auth(self, *, fast: bool = False) -> SlackAuthTestResponse: }, consumes=OperationConsumes.CREDENTIAL, variants=(SlackChannelVariant.PUBLIC, SlackChannelVariant.PRIVATE), - untested=_TEMPORARILY_UNTESTED, ) def list_channels( self, @@ -636,7 +656,6 @@ def list_channels( CredentialCapability.DOC_PERMISSION_SYNC, }, consumes=OperationConsumes.CREDENTIAL, - untested=_TEMPORARILY_UNTESTED, ) def list_teams( self, *, limit: int | None = None, fast: bool = False @@ -678,7 +697,6 @@ def join_channel(self, *, channel_id: str) -> SlackOkResponse: capabilities={CredentialCapability.INDEXING}, consumes=OperationConsumes.CREDENTIAL, variants=(SlackChannelVariant.PUBLIC, SlackChannelVariant.PRIVATE), - untested=_TEMPORARILY_UNTESTED, ) def fetch_channel_history( self, @@ -708,7 +726,6 @@ def fetch_channel_history( capabilities={CredentialCapability.INDEXING}, consumes=OperationConsumes.CREDENTIAL, variants=(SlackChannelVariant.PUBLIC, SlackChannelVariant.PRIVATE), - untested=_TEMPORARILY_UNTESTED, ) def fetch_thread_replies( self, *, variant: SlackChannelVariant, channel_id: str, thread_ts: str @@ -725,7 +742,6 @@ def fetch_thread_replies( @source_operation( capabilities={CredentialCapability.INDEXING}, consumes=OperationConsumes.CREDENTIAL, - untested=_TEMPORARILY_UNTESTED, ) def fetch_channel_info(self, *, channel_id: str) -> SlackChannelInfoResponse: """ @@ -738,13 +754,15 @@ def fetch_channel_info(self, *, channel_id: str) -> SlackChannelInfoResponse: ) @source_operation( + # No EXTERNAL_GROUP_SYNC tag: the one group-sync caller + # (``group_sync.py``) is dormant, since Slack registers no group sync + # in ``sync_params.py``. Tagging it would make the coverage test + # demand a group-sync check for a path that never runs. capabilities={ CredentialCapability.INDEXING, CredentialCapability.DOC_PERMISSION_SYNC, - CredentialCapability.EXTERNAL_GROUP_SYNC, }, consumes=OperationConsumes.CREDENTIAL, - untested=_TEMPORARILY_UNTESTED, ) def fetch_user_info(self, user_id: str) -> SlackUserInfoResponse: """ @@ -762,7 +780,6 @@ def fetch_user_info(self, user_id: str) -> SlackUserInfoResponse: CredentialCapability.DOC_PERMISSION_SYNC, }, consumes=OperationConsumes.CREDENTIAL, - untested=_TEMPORARILY_UNTESTED, ) def list_users( self, @@ -784,7 +801,6 @@ def list_users( @source_operation( capabilities={CredentialCapability.DOC_PERMISSION_SYNC}, consumes=OperationConsumes.CREDENTIAL, - untested=_TEMPORARILY_UNTESTED, ) def list_channel_members( self, *, channel_id: str diff --git a/backend/onyx/context/search/federated/slack_search.py b/backend/onyx/context/search/federated/slack_search.py index 49071e1d708..c74269cb415 100644 --- a/backend/onyx/context/search/federated/slack_search.py +++ b/backend/onyx/context/search/federated/slack_search.py @@ -40,6 +40,7 @@ from onyx.indexing.embedder import DefaultIndexingEmbedder from onyx.indexing.models import DocAwareChunk from onyx.llm.factory import get_default_llm +from onyx.llm.interfaces import LLM from onyx.onyxbot.slack.models import ChannelType, SlackContext from onyx.redis.redis_pool import get_redis_client from onyx.server.federated.models import FederatedConnectorDetail @@ -1005,6 +1006,7 @@ def slack_retrieval( team_id: str | None = None, # Pre-fetched data — when provided, avoids DB query (no session needed) search_settings: SearchSettings | None = None, + llm: LLM | None = None, ) -> list[InferenceChunk]: """ Main entry point for Slack federated search with entity filtering. @@ -1069,8 +1071,10 @@ def slack_retrieval( entities, channel_metadata_dict ) - # Query slack with entity filtering - llm = get_default_llm() + # Query slack with entity filtering. Inside a chat turn the caller's LLM + # carries request policy (e.g. incognito retention headers) that a freshly + # constructed default would drop. + llm = llm or get_default_llm() query_items = build_slack_queries(query, llm, entities, available_channels) # Partition into direct thread fetches and search query strings diff --git a/backend/onyx/db/chat.py b/backend/onyx/db/chat.py index 46c83651e22..cafd27d5eb5 100644 --- a/backend/onyx/db/chat.py +++ b/backend/onyx/db/chat.py @@ -13,7 +13,7 @@ from onyx.configs.constants import MessageType from onyx.context.search.models import InferenceSection, SavedSearchDoc from onyx.context.search.models import SearchDoc as ServerSearchDoc -from onyx.db.enums import IncognitoRecordMode +from onyx.db.enums import IncognitoRecordMode, record_mode_persists_content from onyx.db.models import ( ChatMessage, ChatMessage__SearchDoc, @@ -251,8 +251,12 @@ def create_chat_session( slack_thread_id: str | None = None, project_id: int | None = None, incognito_record_mode: IncognitoRecordMode | None = None, + session_id: UUID | None = None, ) -> ChatSession: chat_session = ChatSession( + # Caller-supplied only for incognito, where uploads name the session + # before it exists so the server can verify them. + **({"id": session_id} if session_id is not None else {}), user_id=user_id, persona_id=persona_id, description=description, @@ -328,7 +332,12 @@ def update_chat_session( if chat_session.deleted: raise ValueError("Trying to rename a deleted chat session") - if description is not None: + # A title is conversation-derived, so a content-free session never stores + # one. Enforced here rather than at each caller: auto-naming, manual + # rename, and the patch endpoint all write through this. + if description is not None and record_mode_persists_content( + chat_session.incognito_record_mode + ): chat_session.description = description if sharing_status is not None: chat_session.shared_status = sharing_status diff --git a/backend/onyx/db/incognito.py b/backend/onyx/db/incognito.py index 955295f5b28..9ad75266afd 100644 --- a/backend/onyx/db/incognito.py +++ b/backend/onyx/db/incognito.py @@ -1,11 +1,25 @@ -"""Membership query for groups-only incognito availability.""" +"""Membership query and file cleanup for incognito chats. +A file's privacy is decided when it is uploaded. The client mints the session +id when incognito is switched on and sends it with every upload, so a file +names its session before that session exists and the server never has to take +the client's word for whether the upload is private. +""" + +from datetime import datetime, timedelta, timezone from uuid import UUID -from sqlalchemy import exists, select +from sqlalchemy import exists, select, update from sqlalchemy.orm import Session -from onyx.db.models import User__UserGroup, UserGroup +from onyx.db.enums import UserFileStatus, record_mode_persists_content +from onyx.db.models import ChatSession, User__UserGroup, UserFile, UserGroup + +# Only reached once a session's live context is gone, so this bounds how long a +# teardown that never arrived (hard tab close, lost beacon) leaves files behind. +INCOGNITO_FILE_ORPHAN_AGE = timedelta(hours=48) +# Bounds one sweep so a backlog cannot flood the delete queue in a single pass. +INCOGNITO_STALE_SWEEP_LIMIT = 500 def user_in_incognito_enabled_group(db_session: Session, user_id: UUID) -> bool: @@ -18,3 +32,113 @@ def user_in_incognito_enabled_group(db_session: Session, user_id: UUID) -> bool: ) ) return bool(db_session.execute(stmt).scalar()) + + +def is_content_persisting_session(db_session: Session, chat_session_id: UUID) -> bool: + """Whether the session records content, which means it never creates a + Redis context and so cannot be judged by context liveness.""" + row = db_session.execute( + select(ChatSession.incognito_record_mode).where( + ChatSession.id == chat_session_id + ) + ).one_or_none() + if row is None: + # The id was minted client-side and no session ever followed, so there + # is no live chat to protect and the files are abandoned. + return False + return record_mode_persists_content(row[0]) + + +def is_incognito_teardown_target( + db_session: Session, chat_session_id: UUID, user_id: UUID +) -> bool: + """Whether this caller may tear down the session named by this id. + + A missing row is a teardown target, not an error: the id is minted + client-side, so uploads can name a session that no message ever created. + Ownership then rests on the per-user scope of the file marking. + """ + row = db_session.execute( + select(ChatSession.user_id, ChatSession.incognito_record_mode).where( + ChatSession.id == chat_session_id + ) + ).one_or_none() + if row is None: + return True + owner_id, record_mode = row + return owner_id in (user_id, None) and record_mode is not None + + +def mark_incognito_user_files_deleting( + db_session: Session, chat_session_id: UUID, user_id: UUID | None = None +) -> list[UUID]: + """Queue a session's uploads for deletion. Caller commits. + + Keyed on the session rather than a caller-supplied id list, so an upload + that finishes after the user closes the chat is still found. Scoped to the + owner where one is known, since the session id originates on a client. + """ + conditions = [ + UserFile.incognito_session_id == chat_session_id, + UserFile.status != UserFileStatus.DELETING, + ] + if user_id is not None: + conditions.append(UserFile.user_id == user_id) + file_ids = list(db_session.scalars(select(UserFile.id).where(*conditions)).all()) + if not file_ids: + return [] + db_session.execute( + update(UserFile) + .where(UserFile.id.in_(file_ids)) + .values(status=UserFileStatus.DELETING) + ) + return file_ids + + +def stale_incognito_session_ids(db_session: Session) -> list[UUID]: + """Sessions whose uploads are past the orphan window.""" + cutoff = datetime.now(timezone.utc) - INCOGNITO_FILE_ORPHAN_AGE + rows = db_session.scalars( + select(UserFile.incognito_session_id) + .where( + # Matches the partial index predicate so Postgres can use it. + UserFile.incognito.is_(True), + UserFile.incognito_session_id.is_not(None), + UserFile.status != UserFileStatus.DELETING, + UserFile.last_accessed_at < cutoff, + ) + .group_by(UserFile.incognito_session_id) + .limit(INCOGNITO_STALE_SWEEP_LIMIT) + ).all() + # The is_not(None) filter above already excludes NULLs. This narrows the + # column's Optional type for the caller. + return [session_id for session_id in rows if session_id is not None] + + +def mark_unadopted_incognito_files_deleting(db_session: Session) -> int: + """Queue incognito uploads no session ever adopted. Caller commits. + + Left by someone who attached a file in incognito and never sent a message, + so no session exists to tear them down. + """ + cutoff = datetime.now(timezone.utc) - INCOGNITO_FILE_ORPHAN_AGE + file_ids = list( + db_session.scalars( + select(UserFile.id) + .where( + UserFile.incognito.is_(True), + UserFile.incognito_session_id.is_(None), + UserFile.status != UserFileStatus.DELETING, + UserFile.last_accessed_at < cutoff, + ) + .limit(INCOGNITO_STALE_SWEEP_LIMIT) + ).all() + ) + if not file_ids: + return 0 + db_session.execute( + update(UserFile) + .where(UserFile.id.in_(file_ids)) + .values(status=UserFileStatus.DELETING) + ) + return len(file_ids) diff --git a/backend/onyx/db/models.py b/backend/onyx/db/models.py index 7942f659055..fdc1b26ee3f 100644 --- a/backend/onyx/db/models.py +++ b/backend/onyx/db/models.py @@ -5395,7 +5395,6 @@ class UserDocument(str, Enum): class UserFile(Base): __tablename__ = "user_file" - id: Mapped[UUID] = mapped_column(PGUUID(as_uuid=True), primary_key=True) user_id: Mapped[UUID | None] = mapped_column(ForeignKey("user.id"), nullable=False) assistants: Mapped[list["Persona"]] = relationship( @@ -5418,6 +5417,17 @@ class UserFile(Base): nullable=False, default=UserFileStatus.PROCESSING, ) + # Privacy is decided when the file is uploaded, from the toggle state, so + # an attachment made before the session exists is already private. + incognito: Mapped[bool] = mapped_column( + Boolean, nullable=False, default=False, server_default=text("false") + ) + # Which session cleans it up. NULL until the session is created on the + # first message and adopts it. No foreign key: the session row is deleted + # first and these must outlive it to be swept. + incognito_session_id: Mapped[UUID | None] = mapped_column( + PGUUID(as_uuid=True), nullable=True, default=None + ) needs_project_sync: Mapped[bool] = mapped_column( Boolean, nullable=False, default=False ) @@ -5449,6 +5459,15 @@ class UserFile(Base): ) __table_args__ = ( + # Declared here as well as in the migration so autogenerate does not + # read it as a stray index and propose dropping it. + Index( + "ix_user_file_incognito_sweep", + "incognito_session_id", + "status", + "last_accessed_at", + postgresql_where=text("incognito"), + ), Index( "ix_user_file_secondary_reconcile_pending", "id", @@ -5979,7 +5998,8 @@ class UserUsage(Base): """ Daily per-user LLM usage rollup for cost/token attribution and budget checks. - One accumulating row per (user, window, model, flow, provider), not per call. + One accumulating row per (user, window, model, flow, provider, incognito), + not per call. """ __tablename__ = "user_usage" @@ -6001,6 +6021,11 @@ class UserUsage(Base): provider: Mapped[str] = mapped_column( String, nullable=False, default="", server_default="" ) + # Incognito-turn spend accumulates in its own rows so reporting can label + # it. Budget readers sum across both values. + incognito: Mapped[bool] = mapped_column( + Boolean, nullable=False, default=False, server_default=text("false") + ) input_tokens: Mapped[int] = mapped_column(BigInteger, nullable=False) output_tokens: Mapped[int] = mapped_column(BigInteger, nullable=False) @@ -6032,6 +6057,7 @@ class UserUsage(Base): "model", "flow", "provider", + "incognito", unique=True, ), ) diff --git a/backend/onyx/db/projects.py b/backend/onyx/db/projects.py index 92a5e9ce369..5baa572e41d 100644 --- a/backend/onyx/db/projects.py +++ b/backend/onyx/db/projects.py @@ -60,6 +60,7 @@ def create_user_files( db_session: Session, link_url: str | None = None, temp_id_map: dict[str, str] | None = None, + incognito_session_id: UUID | None = None, ) -> CategorizedFilesResult: # Categorize the files categorized_files = categorize_uploaded_files(files, db_session) @@ -92,12 +93,15 @@ def create_user_files( content_type=file.content_type, file_type=file.content_type, status=UserFileStatus.SKIPPED if should_skip else UserFileStatus.PROCESSING, + incognito=incognito_session_id is not None, + incognito_session_id=incognito_session_id, last_accessed_at=datetime.datetime.now(datetime.timezone.utc), ) # Persist the UserFile first to satisfy FK constraints for association table db_session.add(new_file) db_session.flush() - if project_id: + # Incognito uploads may use a project as context but never join it. + if project_id and incognito_session_id is None: project_to_user_file = Project__UserFile( project_id=project_id, user_file_id=new_file.id, @@ -120,6 +124,7 @@ def upload_files_to_user_files_with_indexing( temp_id_map: dict[str, str] | None, db_session: Session, background_tasks: BackgroundTasks | None = None, + incognito_session_id: UUID | None = None, ) -> CategorizedFilesResult: if project_id is not None and user is not None: if not check_project_ownership(project_id, user.id, db_session): @@ -131,6 +136,7 @@ def upload_files_to_user_files_with_indexing( user, db_session, temp_id_map=temp_id_map, + incognito_session_id=incognito_session_id, ) user_files = categorized_files_result.user_files rejected_files = categorized_files_result.rejected_files diff --git a/backend/onyx/db/user_file.py b/backend/onyx/db/user_file.py index 8d5282aa3eb..339f54388b0 100644 --- a/backend/onyx/db/user_file.py +++ b/backend/onyx/db/user_file.py @@ -195,6 +195,7 @@ def get_user_file_ids_for_user_batch( stmt = select(UserFile.id).where( UserFile.user_id == user_id, UserFile.status == UserFileStatus.COMPLETED, + UserFile.incognito.is_(False), ) if after_id is not None: stmt = stmt.where(UserFile.id > UUID(after_id)) diff --git a/backend/onyx/db/user_usage.py b/backend/onyx/db/user_usage.py index 8392faf985d..0409dd60139 100644 --- a/backend/onyx/db/user_usage.py +++ b/backend/onyx/db/user_usage.py @@ -1,7 +1,7 @@ """Daily per-user LLM usage rollup for cost/token attribution. A window rollup: rows accumulate in place per (user, window, -model, flow, provider), not an append-only per-call ledger.""" +model, flow, provider, incognito), not an append-only per-call ledger.""" from collections import defaultdict from collections.abc import Iterator, Sequence @@ -27,7 +27,7 @@ COST_BUDGET_PERIOD_ERROR = "Cost budget periods must be whole UTC days" # Not email-shaped on purpose: it can never collide with a real address. DELETED_USER_EXPORT_EMAIL = "(deleted user)" -_CONFLICT_COLS = ["user_id", "window_start", "model", "flow", "provider"] +_CONFLICT_COLS = ["user_id", "window_start", "model", "flow", "provider", "incognito"] class TokenUsageBucket(BaseModel): @@ -123,6 +123,7 @@ class UsageExportRow(BaseModel): model: str flow: str provider: str + incognito: bool day: str # YYYY-MM-DD input_tokens: int output_tokens: int @@ -141,6 +142,7 @@ def record_user_usage( cache_read_tokens: int, cost_cents: float, window_start: datetime, + incognito: bool = False, ) -> None: """Atomically accumulate into the ledger (Postgres upsert). Caller commits.""" # Store "" rather than NULL for a missing provider so the dedup unique index @@ -152,6 +154,7 @@ def record_user_usage( model=model, flow=flow, provider=provider, + incognito=incognito, input_tokens=input_tokens, output_tokens=output_tokens, cache_read_tokens=cache_read_tokens, @@ -226,6 +229,7 @@ def _get_usage_export_query( UserUsage.model, UserUsage.flow, UserUsage.provider, + UserUsage.incognito, utc_day.label("day"), func.sum(UserUsage.input_tokens), func.sum(UserUsage.output_tokens), @@ -240,7 +244,12 @@ def _get_usage_export_query( UserUsage.window_start < end, ) .group_by( - email_label, UserUsage.model, UserUsage.flow, UserUsage.provider, utc_day + email_label, + UserUsage.model, + UserUsage.flow, + UserUsage.provider, + UserUsage.incognito, + utc_day, ) .order_by( email_label, @@ -248,6 +257,7 @@ def _get_usage_export_query( UserUsage.model, UserUsage.flow, UserUsage.provider, + UserUsage.incognito, ) ) if model is not None: @@ -267,12 +277,24 @@ def iter_usage_export( stream_results=True ) ).yield_per(1000) - for email, mdl, flow, provider, day, in_tok, out_tok, cache_tok, cost in result: + for ( + email, + mdl, + flow, + provider, + incognito, + day, + in_tok, + out_tok, + cache_tok, + cost, + ) in result: yield UsageExportRow( email=str(email), model=mdl, flow=flow, provider=provider, + incognito=bool(incognito), day=str(day), input_tokens=int(in_tok or 0), output_tokens=int(out_tok or 0), diff --git a/backend/onyx/llm/factory.py b/backend/onyx/llm/factory.py index 4e45705bc16..aa5a59827f3 100644 --- a/backend/onyx/llm/factory.py +++ b/backend/onyx/llm/factory.py @@ -20,7 +20,7 @@ from onyx.db.models import LLMProvider as LLMProviderModel from onyx.db.models import Persona, SearchSettings, User from onyx.llm.constants import LlmProviderNames -from onyx.llm.interfaces import LLM +from onyx.llm.interfaces import LLM, LlmRequestPolicy from onyx.llm.multi_llm import LitellmLLM from onyx.llm.override_models import LLMOverride from onyx.llm.utils import ( @@ -155,6 +155,7 @@ def get_llm_for_persona( user: User, llm_override: LLMOverride | None = None, additional_headers: dict[str, str] | None = None, + policy_fn: Callable[[str], LlmRequestPolicy] | None = None, ) -> LLM: """Get the appropriate LLM for a persona, with the following priority: 1. LLM override (model configuration id, else provider + model version) @@ -163,7 +164,7 @@ def get_llm_for_persona( """ if persona is None: logger.warning("No persona provided, using default LLM") - return get_default_llm() + return get_default_llm(policy_fn=policy_fn) mc_id_override = llm_override.model_configuration_id if llm_override else None provider_name_override = llm_override.model_provider if llm_override else None @@ -178,6 +179,7 @@ def get_llm_for_persona( return get_default_llm( temperature=temperature_override or GEN_AI_TEMPERATURE, additional_headers=additional_headers, + policy_fn=policy_fn, ) with get_session_with_current_tenant() as db_session: @@ -196,6 +198,7 @@ def get_llm_for_persona( else GEN_AI_TEMPERATURE ), additional_headers=additional_headers, + policy_fn=policy_fn, ) provider_model, model = resolved @@ -213,6 +216,7 @@ def get_llm_for_persona( return get_default_llm( temperature=temperature_override or GEN_AI_TEMPERATURE, additional_headers=additional_headers, + policy_fn=policy_fn, ) llm_provider = LLMProviderView.from_model(provider_model) @@ -222,6 +226,7 @@ def get_llm_for_persona( llm_provider=llm_provider, temperature=temperature_override, additional_headers=additional_headers, + policy_fn=policy_fn, ) @@ -332,6 +337,7 @@ def llm_from_provider( timeout: int | None = None, temperature: float | None = None, additional_headers: dict[str, str] | None = None, + policy_fn: Callable[[str], LlmRequestPolicy] | None = None, ) -> LLM: configured_max_input_tokens = _get_model_configured_max_input_tokens( llm_provider=llm_provider, model_name=model_name @@ -347,6 +353,9 @@ def llm_from_provider( llm_provider=llm_provider, model_name=model_name ) ) + # Resolved here, not at the call site: the caller hands policy as a + # provider-keyed function because it cannot know which provider wins. + policy = policy_fn(llm_provider.provider) if policy_fn else None return get_llm( provider=llm_provider.provider, model=model_name, @@ -360,6 +369,8 @@ def llm_from_provider( additional_headers=additional_headers, max_input_tokens=max_input_tokens, model_kwargs=model_kwargs, + policy_headers=policy.headers if policy else None, + policy_model_kwargs=policy.model_kwargs if policy else None, ) @@ -395,6 +406,7 @@ def get_default_llm( timeout: int | None = None, temperature: float | None = None, additional_headers: dict[str, str] | None = None, + policy_fn: Callable[[str], LlmRequestPolicy] | None = None, ) -> LLM: with get_session_with_current_tenant() as db_session: model = fetch_default_llm_model(db_session) @@ -408,6 +420,7 @@ def get_default_llm( timeout=timeout, temperature=temperature, additional_headers=additional_headers, + policy_fn=policy_fn, ) @@ -424,6 +437,8 @@ def get_llm( timeout: int | None = None, additional_headers: dict[str, str] | None = None, model_kwargs: dict[str, Any] | None = None, + policy_headers: dict[str, str] | None = None, + policy_model_kwargs: dict[str, Any] | None = None, ) -> LLM: if temperature is None: temperature = GEN_AI_TEMPERATURE @@ -436,6 +451,16 @@ def get_llm( if provider_extra_headers: extra_headers.update(provider_extra_headers) + # Last on purpose: policy headers (e.g. incognito retention suppression) + # must win over request, deployment-env, and provider header sources. + if policy_headers: + extra_headers.update(policy_headers) + + # Same precedence rule for body params (e.g. store=False). + merged_model_kwargs = dict(model_kwargs or {}) + if policy_model_kwargs: + merged_model_kwargs.update(policy_model_kwargs) + return LitellmLLM( model_provider=provider, model_name=model, @@ -447,7 +472,7 @@ def get_llm( temperature=temperature, custom_config=custom_config, extra_headers=extra_headers, - model_kwargs=model_kwargs or {}, + model_kwargs=merged_model_kwargs, max_input_tokens=max_input_tokens, ) diff --git a/backend/onyx/llm/interfaces.py b/backend/onyx/llm/interfaces.py index 7d8288061c9..9f9c41a3d10 100644 --- a/backend/onyx/llm/interfaces.py +++ b/backend/onyx/llm/interfaces.py @@ -22,6 +22,14 @@ class LLMUserIdentity(BaseModel): session_id: str | None = None +class LlmRequestPolicy(BaseModel): + """Per-request policy an LLM call must carry (e.g. incognito retention + suppression). Merged after every other source so nothing overrides it.""" + + headers: dict[str, str] = {} + model_kwargs: dict[str, Any] = {} + + class LLMConfig(BaseModel): model_provider: str model_name: str diff --git a/backend/onyx/llm/multi_llm.py b/backend/onyx/llm/multi_llm.py index aad8fde2da3..96dd4c1a864 100644 --- a/backend/onyx/llm/multi_llm.py +++ b/backend/onyx/llm/multi_llm.py @@ -119,6 +119,20 @@ } +def _merge_under(base: dict[str, Any], override: dict[str, Any]) -> dict[str, Any]: + """Fill *base* in beneath *override*, recursing so an override key that + holds a dict keeps the siblings *base* declared under it. Leaves in + *override* always win.""" + merged = dict(base) + for key, value in override.items(): + existing = merged.get(key) + if isinstance(existing, dict) and isinstance(value, dict): + merged[key] = _merge_under(existing, value) + else: + merged[key] = value + return merged + + def _rejection_names_strippable_kwargs(error: Exception, strippable: set[str]) -> bool: """True when the 400's message names a kwarg a later attempt would drop. Unrelated 400s (context length, malformed input) must not be retried.""" @@ -557,10 +571,17 @@ def __init__( # This is needed for Ollama to do proper function calling if model_provider == LlmProviderNames.OLLAMA_CHAT and api_base is not None: model_kwargs["api_base"] = api_base + # Deployment config merges under anything already in model_kwargs, so a + # policy-supplied value wins. Incognito retention flags live here, and + # replacing the dict would silently re-enable provider logging. if extra_headers: - model_kwargs.update({"extra_headers": extra_headers}) + model_kwargs["extra_headers"] = _merge_under( + extra_headers, model_kwargs.get("extra_headers") or {} + ) if extra_body: - model_kwargs.update({"extra_body": extra_body}) + model_kwargs["extra_body"] = _merge_under( + extra_body, model_kwargs.get("extra_body") or {} + ) self._model_kwargs = model_kwargs diff --git a/backend/onyx/server/documents/connector.py b/backend/onyx/server/documents/connector.py index 1d5c28be8ba..195a0ff9759 100644 --- a/backend/onyx/server/documents/connector.py +++ b/backend/onyx/server/documents/connector.py @@ -1552,6 +1552,8 @@ def update_connector_from_model( except ValueError as e: raise HTTPException(status_code=400, detail=str(e)) + # TODO(andrei, evan): Validate the updated config here like the creation + # flows do (``validate_ccpair_for_user`` / ``validate_connector_settings``). updated_connector = update_connector(connector_id, connector_base, db_session) if updated_connector is None: raise HTTPException( diff --git a/backend/onyx/server/features/build/sandbox/kubernetes/kubernetes_sandbox_manager.py b/backend/onyx/server/features/build/sandbox/kubernetes/kubernetes_sandbox_manager.py index 258516c9630..29ea7a76cdc 100644 --- a/backend/onyx/server/features/build/sandbox/kubernetes/kubernetes_sandbox_manager.py +++ b/backend/onyx/server/features/build/sandbox/kubernetes/kubernetes_sandbox_manager.py @@ -37,6 +37,7 @@ import base64 import binascii import copy +import gzip import hashlib import io import ipaddress @@ -244,16 +245,17 @@ def _build_targz(files: FileSet) -> tuple[bytes, str]: f"Bundle size {total} exceeds {_MAX_BUNDLE_BYTES} byte limit" ) buf = io.BytesIO() - with tarfile.open(fileobj=buf, mode="w:gz", compresslevel=6) as tar: - for name in sorted(files): - data = files[name] - info = tarfile.TarInfo(name=name) - info.size = len(data) - info.mtime = 0 - info.uid = 0 - info.gid = 0 - info.mode = 0o644 - tar.addfile(info, io.BytesIO(data)) + with gzip.GzipFile(fileobj=buf, mode="wb", compresslevel=6, mtime=0) as gzip_file: + with tarfile.open(fileobj=gzip_file, mode="w") as tar: + for name in sorted(files): + data = files[name] + info = tarfile.TarInfo(name=name) + info.size = len(data) + info.mtime = 0 + info.uid = 0 + info.gid = 0 + info.mode = 0o644 + tar.addfile(info, io.BytesIO(data)) raw = buf.getvalue() return raw, hashlib.sha256(raw).hexdigest() diff --git a/backend/onyx/server/features/projects/api.py b/backend/onyx/server/features/projects/api.py index 1a984737866..d2583850083 100644 --- a/backend/onyx/server/features/projects/api.py +++ b/backend/onyx/server/features/projects/api.py @@ -15,6 +15,8 @@ from sqlalchemy.orm import Session from onyx.auth.permissions import require_permission +from onyx.chat.incognito import incognito_allowed_for_user +from onyx.chat.incognito_context import incognito_session_ended from onyx.configs.app_configs import DISABLE_VECTOR_DB from onyx.configs.constants import ( PUBLIC_API_TAGS, @@ -25,6 +27,7 @@ ) from onyx.db.engine.sql_engine import get_session from onyx.db.enums import Permission, UserFileStatus +from onyx.db.incognito import mark_incognito_user_files_deleting from onyx.db.models import ChatSession, Project__UserFile, User, UserFile, UserProject from onyx.db.persona import get_personas_by_ids from onyx.db.projects import ( @@ -148,9 +151,26 @@ def upload_user_files( files: list[UploadFile] = File(...), project_id: int | None = Form(None), temp_id_map: str | None = Form(None), # JSON string mapping hashed key -> temp_id + incognito_session_id: UUID | None = Form(None), user: User = Depends(require_permission(Permission.BASIC_ACCESS)), db_session: Session = Depends(get_session), ) -> CategorizedFilesSnapshot: + # The file names its session before that session exists, so it is private + # from the moment it lands and the id is what teardown finds it by. + if incognito_session_id is not None: + if not incognito_allowed_for_user(user, db_session, cached=False): + raise OnyxError( + OnyxErrorCode.UNAUTHORIZED, + "Incognito chat is not enabled for this user.", + ) + # Teardown marks the rows that exist when it runs, so an upload landing + # after it would otherwise sit until the orphan sweep. The tombstone is + # durable, which makes this the point where that race is settled. + if incognito_session_ended(incognito_session_id): + raise OnyxError( + OnyxErrorCode.INVALID_INPUT, + "This incognito chat has ended.", + ) try: parsed_temp_id_map: dict[str, str] | None = None if temp_id_map: @@ -172,8 +192,20 @@ def upload_user_files( temp_id_map=parsed_temp_id_map, db_session=db_session, background_tasks=bg_tasks if DISABLE_VECTOR_DB else None, + incognito_session_id=incognito_session_id, ) + # Re-check after the rows exist. The tombstone is monotonic, so a + # teardown that landed during the upload is caught here even though the + # pre-check passed, which is what the marking query alone would miss. + if incognito_session_id is not None and incognito_session_ended( + incognito_session_id + ): + mark_incognito_user_files_deleting( + db_session, incognito_session_id, user.id + ) + db_session.commit() + return CategorizedFilesSnapshot.from_result(categorized_files_result) except Exception as e: diff --git a/backend/onyx/server/features/usage/api.py b/backend/onyx/server/features/usage/api.py index 6a94d67bc8e..fd319134051 100644 --- a/backend/onyx/server/features/usage/api.py +++ b/backend/onyx/server/features/usage/api.py @@ -60,8 +60,25 @@ from onyx.utils.datetime import get_window_start from shared_configs.configs import USAGE_LIMIT_WINDOW_SECONDS -# Default trailing range for the export when no start is given. -_DEFAULT_EXPORT_DAYS = 30 +# Default trailing range when no start is given. +_DEFAULT_USAGE_RANGE_INCLUSIVE_DAYS = 30 + + +def _start_for_inclusive_range(end_date: date, inclusive_days: int) -> date: + return end_date - timedelta(days=inclusive_days - 1) + + +def _date_range_to_utc_bounds( + start_date: date, end_date: date +) -> tuple[datetime, datetime]: + if start_date > end_date: + raise OnyxError(OnyxErrorCode.INVALID_INPUT, "start must not be after end") + + start_dt = datetime.combine(start_date, time.min, tzinfo=timezone.utc) + end_dt = datetime.combine(end_date, time.min, tzinfo=timezone.utc) + timedelta( + days=1 + ) + return start_dt, end_dt def _used_from_buckets( @@ -260,15 +277,10 @@ def export_usage( ) -> UsageExportResponse: """Company-wide daily usage export by email.""" end_date = end or datetime.now(timezone.utc).date() - start_date = start or (end_date - timedelta(days=_DEFAULT_EXPORT_DAYS)) - if start_date > end_date: - raise OnyxError(OnyxErrorCode.INVALID_INPUT, "start must not be after end") - - # Half-open over the full end day so windows starting on `end` are included. - start_dt = datetime.combine(start_date, time.min, tzinfo=timezone.utc) - end_dt = datetime.combine(end_date, time.min, tzinfo=timezone.utc) + timedelta( - days=1 + start_date = start or _start_for_inclusive_range( + end_date, _DEFAULT_USAGE_RANGE_INCLUSIVE_DAYS ) + start_dt, end_dt = _date_range_to_utc_bounds(start_date, end_date) # TODO(evan-onyx): this might need to be done in a background task rows = get_usage_export(db_session, start=start_dt, end=end_dt, model=model) diff --git a/backend/onyx/server/features/usage/models.py b/backend/onyx/server/features/usage/models.py index 3118c4a09b9..3feb4424741 100644 --- a/backend/onyx/server/features/usage/models.py +++ b/backend/onyx/server/features/usage/models.py @@ -43,6 +43,7 @@ class UsageExportRecord(BaseModel): model: str flow: str provider: str + incognito: bool day: str # YYYY-MM-DD input_tokens: int output_tokens: int diff --git a/backend/onyx/server/manage/users.py b/backend/onyx/server/manage/users.py index 84a371d4dd5..d461a737987 100644 --- a/backend/onyx/server/manage/users.py +++ b/backend/onyx/server/manage/users.py @@ -1341,6 +1341,8 @@ def get_recent_files( .filter(UserFile.user_id == user_id) .filter(UserFile.status != UserFileStatus.FAILED) .filter(UserFile.status != UserFileStatus.DELETING) + # Incognito uploads live only inside their session, never in recents. + .filter(UserFile.incognito.is_(False)) .order_by(UserFile.last_accessed_at.desc()) .all() ) diff --git a/backend/onyx/server/query_and_chat/chat_backend.py b/backend/onyx/server/query_and_chat/chat_backend.py index 05796e8a53c..2c9e0d40548 100644 --- a/backend/onyx/server/query_and_chat/chat_backend.py +++ b/backend/onyx/server/query_and_chat/chat_backend.py @@ -47,7 +47,15 @@ CHAT_RESUME_POLL_INTERVAL_S, HARD_DELETE_CHATS, ) -from onyx.configs.constants import PUBLIC_API_TAGS, MessageType, MilestoneRecordType +from onyx.configs.constants import ( + CELERY_USER_FILE_DELETE_TASK_EXPIRES, + PUBLIC_API_TAGS, + MessageType, + MilestoneRecordType, + OnyxCeleryPriority, + OnyxCeleryQueues, + OnyxCeleryTask, +) from onyx.configs.model_configs import LITELLM_PASS_THROUGH_HEADERS from onyx.db.chat import ( add_chats_to_session_from_slack_thread, @@ -68,6 +76,10 @@ from onyx.db.engine.sql_engine import get_session, get_session_with_current_tenant from onyx.db.enums import Permission, record_mode_persists_content from onyx.db.feedback import create_chat_message_feedback, remove_chat_message_feedback +from onyx.db.incognito import ( + is_incognito_teardown_target, + mark_incognito_user_files_deleting, +) from onyx.db.llm import fetch_default_chat_naming_model from onyx.db.models import ChatMessage, ChatSessionSharedStatus, Persona, User from onyx.db.persona import get_persona_by_id @@ -541,13 +553,15 @@ def rename_chat_session( if name: with get_session_with_current_tenant() as db_session: - update_chat_session( + chat_session = update_chat_session( db_session=db_session, user_id=user_id, chat_session_id=chat_session_id, description=name, ) - return RenameChatSessionResponse(new_name=name) + # Echo what was stored: a content-free session drops the title, and + # reporting the requested one would show a rename that did not happen. + return RenameChatSessionResponse(new_name=chat_session.description or "") # Close the read session before the LLM's multi-second generation window. with get_session_with_current_tenant() as db_session: @@ -615,9 +629,19 @@ def patch_chat_session( return None -def _teardown_incognito_after_delete(chat_session_id: UUID) -> None: +def _teardown_incognito_after_delete( + chat_session_id: UUID, user_id: UUID, db_session: Session +) -> None: """The rows are already gone, so a failed teardown is logged rather than - failing a delete the caller cannot retry. The context TTL is the backstop.""" + failing a delete the caller cannot retry. The context TTL is the backstop. + + Uploads are queued here too, so deleting a chat cleans up the same things + the dedicated teardown endpoint does.""" + try: + mark_incognito_user_files_deleting(db_session, chat_session_id, user_id) + db_session.commit() + except Exception: + logger.exception("Incognito file cleanup failed for %s", chat_session_id) try: teardown_incognito_session(chat_session_id) except Exception: @@ -645,7 +669,7 @@ def delete_all_chat_sessions( except ValueError as e: raise HTTPException(status_code=400, detail=str(e)) for incognito_id in incognito_session_ids: - _teardown_incognito_after_delete(incognito_id) + _teardown_incognito_after_delete(incognito_id, user.id, db_session) @router.delete("/delete-chat-session/{session_id}", tags=PUBLIC_API_TAGS) @@ -678,7 +702,7 @@ def delete_chat_session_by_id( except ValueError as e: raise HTTPException(status_code=400, detail=str(e)) if is_incognito: - _teardown_incognito_after_delete(session_id) + _teardown_incognito_after_delete(session_id, user.id, db_session) class IncognitoAvailabilityResponse(BaseModel): @@ -706,19 +730,40 @@ def end_incognito_session( """Drop an incognito session's live context the moment the chat closes. The context TTL is only the backstop for when this never arrives, such as - a hard tab close. + a hard tab close. Uploads are found by session id, so one still in flight + when the user leaves, or one attached before any message created the + session, is queued for deletion just the same. """ - chat_session = get_chat_session_by_id( - chat_session_id=session_id, user_id=user.id, db_session=db_session - ) - if chat_session.incognito_record_mode is not None: - teardown_incognito_session(session_id) - if not delete_incognito_generated_files(session_id, db_session): - raise OnyxError( - OnyxErrorCode.SERVICE_UNAVAILABLE, - "Some generated files could not be deleted yet and will be retried.", + if not is_incognito_teardown_target(db_session, session_id, user.id): + return + # Durable file marking runs before the Redis teardown: the beacon is + # one-shot, so a failure after this point still leaves the files queued + # for deletion rather than stored but hidden. + deletable_ids = mark_incognito_user_files_deleting(db_session, session_id, user.id) + db_session.commit() + teardown_incognito_session(session_id) + + if deletable_ids: + from onyx.background.celery.versioned_apps.client import app as client_app + + tenant_id = get_current_tenant_id() + for user_file_id in deletable_ids: + client_app.send_task( + OnyxCeleryTask.DELETE_SINGLE_USER_FILE, + kwargs={"user_file_id": str(user_file_id), "tenant_id": tenant_id}, + queue=OnyxCeleryQueues.USER_FILE_DELETE, + priority=OnyxCeleryPriority.HIGH, + expires=CELERY_USER_FILE_DELETE_TASK_EXPIRES, ) + # Last, so a store that refuses a blob cannot strand the queued uploads: + # this raises to tell the client the sweep will retry. + if not delete_incognito_generated_files(session_id, db_session): + raise OnyxError( + OnyxErrorCode.SERVICE_UNAVAILABLE, + "Some generated files could not be deleted yet and will be retried.", + ) + # NOTE: This endpoint is extremely central to the application, any changes to it should be reviewed and approved by an experienced # team member. It is very important to 1. avoid bloat and 2. that this remains backwards compatible across versions. diff --git a/backend/onyx/server/query_and_chat/models.py b/backend/onyx/server/query_and_chat/models.py index 92340101165..2c3d3b5544d 100644 --- a/backend/onyx/server/query_and_chat/models.py +++ b/backend/onyx/server/query_and_chat/models.py @@ -82,6 +82,9 @@ class ChatSessionCreationRequest(BaseModel): # Start the session incognito. Refused with an error when incognito is # unavailable, never silently downgraded to an ordinary chat. incognito: bool = False + # The id the client already used when uploading, so this session owns those + # files. Ignored unless incognito. + incognito_session_id: UUID | None = None class ChatFeedbackRequest(BaseModel): diff --git a/backend/onyx/tools/tool_implementations/search/search_tool.py b/backend/onyx/tools/tool_implementations/search/search_tool.py index fd3d2d2243c..a2c82890b73 100644 --- a/backend/onyx/tools/tool_implementations/search/search_tool.py +++ b/backend/onyx/tools/tool_implementations/search/search_tool.py @@ -462,6 +462,7 @@ def _run_slack_search( bot_token=bot_token, team_id=None, search_settings=search_settings, + llm=self.llm, ) logger.info("Slack federated search returned %s chunks", len(chunks)) diff --git a/backend/onyx/tracing/processors/user_usage_processor.py b/backend/onyx/tracing/processors/user_usage_processor.py index 44bbdbe8a12..7bbd2af655a 100644 --- a/backend/onyx/tracing/processors/user_usage_processor.py +++ b/backend/onyx/tracing/processors/user_usage_processor.py @@ -13,6 +13,7 @@ from sqlalchemy.orm import Session from onyx.db.engine.sql_engine import get_session_with_tenant +from onyx.db.enums import IncognitoRecordMode from onyx.db.user_usage import USER_USAGE_BUCKET_SECONDS, record_user_usage from onyx.llm.cost import compute_cost_cents from onyx.tracing.flows import IMAGE_FLOWS @@ -24,6 +25,7 @@ from onyx.utils.logger import setup_logger from shared_configs.contextvars import ( CURRENT_TENANT_ID_CONTEXTVAR, + get_current_incognito_record_mode, get_current_tenant_id, get_current_user_id, ) @@ -51,6 +53,7 @@ class _UsageRecord: model: str flow: str provider: str | None + incognito: bool input_tokens: int output_tokens: int cache_read_tokens: int @@ -134,12 +137,18 @@ def _capture(self, span: Span[Any]) -> _UsageRecord | None: datetime.now(timezone.utc), period_seconds=USER_USAGE_BUCKET_SECONDS ) + # Every incognito turn is labelled, including full history. The label + # says the turn was incognito, not whether its content was kept. + incognito_mode = IncognitoRecordMode.from_context_value( + get_current_incognito_record_mode() + ) return _UsageRecord( tenant_id=get_current_tenant_id(), user_id=user_id, model=model, flow=flow, provider=provider, + incognito=incognito_mode is not None, input_tokens=input_tokens, output_tokens=output_tokens, cache_read_tokens=cache_read_tokens, @@ -197,7 +206,7 @@ def _flush_batch(self, batch: list[_UsageRecord]) -> None: @staticmethod def _aggregate_batch(batch: list[_UsageRecord]) -> list[_UsageRecord]: aggregated: dict[ - tuple[str, str, str, str, str | None, datetime], _UsageRecord + tuple[str, str, str, str, str | None, bool, datetime], _UsageRecord ] = {} for record in batch: key = ( @@ -206,6 +215,7 @@ def _aggregate_batch(batch: list[_UsageRecord]) -> list[_UsageRecord]: record.model, record.flow, record.provider, + record.incognito, record.window_start, ) current = aggregated.get(key) @@ -255,6 +265,7 @@ def _write_record(db_session: Session, record: _UsageRecord) -> None: cache_read_tokens=record.cache_read_tokens, cost_cents=input_cost + output_cost, window_start=record.window_start, + incognito=record.incognito, ) # --- TracingProcessor interface (non-generation events are no-ops) --- diff --git a/backend/tests/external_dependency_unit/chat/test_incognito_availability.py b/backend/tests/external_dependency_unit/chat/test_incognito_availability.py index 240090ad512..92c6fcc5e68 100644 --- a/backend/tests/external_dependency_unit/chat/test_incognito_availability.py +++ b/backend/tests/external_dependency_unit/chat/test_incognito_availability.py @@ -51,15 +51,16 @@ def _make_group(db_session: Session, user: User, incognito_enabled: bool) -> Non def _workspace( mode: IncognitoAvailability, store_available: bool = True ) -> Iterator[None]: + """Both settings readers return the same mode, so a test that does not care + which one is used passes either way.""" + settings = MagicMock(incognito_availability=mode) with ( patch( "onyx.chat.incognito.incognito_context_available", return_value=store_available, ), - patch( - "onyx.chat.incognito.get_security_settings", - return_value=MagicMock(incognito_availability=mode), - ), + patch("onyx.chat.incognito.get_security_settings", return_value=settings), + patch("onyx.chat.incognito.load_effective_uncached", return_value=settings), ): yield @@ -102,3 +103,25 @@ def test_anonymous_user_is_refused(db_session: Session) -> None: anonymous = MagicMock(is_anonymous=True) with _workspace(IncognitoAvailability.EVERYONE): assert not incognito_allowed_for_user(anonymous, db_session) + + +def test_enforcement_reads_past_the_settings_cache( + db_session: Session, owner: User +) -> None: + """Cache invalidation is process-local, so a second api_server would keep + authorizing against a revoked setting for the cache TTL.""" + with ( + patch("onyx.chat.incognito.incognito_context_available", return_value=True), + patch( + "onyx.chat.incognito.get_security_settings", + return_value=MagicMock( + incognito_availability=IncognitoAvailability.EVERYONE + ), + ), + patch( + "onyx.chat.incognito.load_effective_uncached", + return_value=MagicMock(incognito_availability=IncognitoAvailability.OFF), + ), + ): + assert incognito_allowed_for_user(owner, db_session) + assert not incognito_allowed_for_user(owner, db_session, cached=False) diff --git a/backend/tests/external_dependency_unit/db/test_incognito_history_exclusion.py b/backend/tests/external_dependency_unit/db/test_incognito_history_exclusion.py index 9b06ac8b9f3..7d6834a7bac 100644 --- a/backend/tests/external_dependency_unit/db/test_incognito_history_exclusion.py +++ b/backend/tests/external_dependency_unit/db/test_incognito_history_exclusion.py @@ -23,6 +23,7 @@ create_new_chat_message, get_chat_sessions_by_user, get_or_create_root_message, + update_chat_session, ) from onyx.db.chat_search import search_chat_sessions from onyx.db.enums import IncognitoRecordMode @@ -153,9 +154,12 @@ def test_admin_query_history_page_hides_content_free_sessions( assert usage_only.id not in page_ids -def test_query_history_export_hides_content_free_sessions( +def test_usage_report_still_meters_content_free_sessions( db_session: Session, owner: User ) -> None: + """The usage report carries token counts and no message content, so every + mode belongs in it. Dropping content-free sessions here would make + incognito a way around token rate limits.""" ordinary = _make_session(db_session, owner.id, "ordinary chat", None) usage_only = _make_session( db_session, owner.id, "usage only chat", IncognitoRecordMode.USAGE_ONLY @@ -163,7 +167,7 @@ def test_query_history_export_hides_content_free_sessions( window = timedelta(minutes=5) now = datetime.now(timezone.utc) - export_ids = { + metered_ids = { session.id for session in fetch_chat_sessions_eagerly_by_time( start=now - window, @@ -172,8 +176,8 @@ def test_query_history_export_hides_content_free_sessions( limit=None, ) } - assert ordinary.id in export_ids - assert usage_only.id not in export_ids + assert ordinary.id in metered_ids + assert usage_only.id in metered_ids def test_query_history_detail_hides_content_free_sessions( @@ -198,6 +202,28 @@ def test_query_history_detail_hides_content_free_sessions( fetch_persisting_chat_session_by_id(usage_only.id, db_session) +def test_rename_cannot_store_a_title_on_a_content_free_session( + db_session: Session, owner: User +) -> None: + """A title is caller-supplied conversation content, so no caller may put + one on a content-free session. Guarded in the db layer because auto-naming, + manual rename, and the patch endpoint all write through it.""" + usage_only = _make_session(db_session, owner.id, "", IncognitoRecordMode.USAGE_ONLY) + full_history = _make_session( + db_session, owner.id, "", IncognitoRecordMode.FULL_HISTORY + ) + + for chat_session, expected in ((usage_only, ""), (full_history, "a real title")): + update_chat_session( + db_session=db_session, + user_id=owner.id, + chat_session_id=chat_session.id, + description="a real title", + ) + db_session.refresh(chat_session) + assert chat_session.description == expected + + def test_every_mode_is_excluded_from_history(db_session: Session, owner: User) -> None: """Every mode must stay out of the owner's history. diff --git a/backend/tests/external_dependency_unit/tools/test_python_tool.py b/backend/tests/external_dependency_unit/tools/test_python_tool.py index f0e6c6b38f3..11795870411 100644 --- a/backend/tests/external_dependency_unit/tools/test_python_tool.py +++ b/backend/tests/external_dependency_unit/tools/test_python_tool.py @@ -1170,6 +1170,9 @@ def test_code_interpreter_receives_chat_files( ], project_id=None, temp_id_map=json.dumps({"0|data.csv": "data.csv"}), + # Explicit: calling the endpoint directly leaves this as the Form + # default object, which is truthy and trips the incognito guard. + incognito_session_id=None, user=user, db_session=db_session, ) diff --git a/backend/tests/integration/common_utils/cimd_oauth.py b/backend/tests/integration/common_utils/cimd_oauth.py new file mode 100644 index 00000000000..5b453e5280d --- /dev/null +++ b/backend/tests/integration/common_utils/cimd_oauth.py @@ -0,0 +1,20 @@ +from pathlib import Path + +from pydantic import BaseModel + + +class CimdHttpsEndpoint(BaseModel): + origin: str + ca_file: Path + + +class CimdOAuthTestServices(BaseModel): + mcp_server_url: str + oidc_issuer: str + client_metadata_url: str + + +class MockOidcStatus(BaseModel): + client_metadata_fetch_count: int + registration_request_count: int + last_client_id: str | None diff --git a/backend/tests/integration/mock_services/mcp_test_server/run_mock_oidc_idp.py b/backend/tests/integration/mock_services/mcp_test_server/run_mock_oidc_idp.py index 4ee389a62b9..1795a4bdd88 100644 --- a/backend/tests/integration/mock_services/mcp_test_server/run_mock_oidc_idp.py +++ b/backend/tests/integration/mock_services/mcp_test_server/run_mock_oidc_idp.py @@ -36,6 +36,11 @@ MOCK_OIDC_AUDIENCE token audience (default: api://mcp) MOCK_OIDC_SCOPE scope granted into the token (default: mcp:use) MOCK_OIDC_SUBJECT sub claim for the fake user (default: mock-user@example.com) + MOCK_OIDC_CIMD_ONLY advertise CIMD and reject DCR when true + MOCK_OIDC_EXPECTED_CLIENT_ID + exact CIMD URL accepted by the authorization endpoint + MOCK_OIDC_CLIENT_METADATA_CA_FILE + CA used to fetch the HTTPS client metadata document """ from __future__ import annotations @@ -44,19 +49,50 @@ import hashlib import json import os +import ssl import sys import time +import urllib.request import uuid -from typing import Any +from typing import Any, Literal import jwt import uvicorn from cryptography.hazmat.primitives import serialization from cryptography.hazmat.primitives.asymmetric import rsa from fastapi import FastAPI, Form, Request -from fastapi.responses import JSONResponse, PlainTextResponse, RedirectResponse +from fastapi.responses import ( + JSONResponse, + PlainTextResponse, + RedirectResponse, + Response, +) +from pydantic import AnyUrl, BaseModel KEY_ID = "mock-oidc-key-1" +CIMD_ONLY_ENV_VAR = "MOCK_OIDC_CIMD_ONLY" +EXPECTED_CLIENT_ID_ENV_VAR = "MOCK_OIDC_EXPECTED_CLIENT_ID" +CLIENT_METADATA_CA_FILE_ENV_VAR = "MOCK_OIDC_CLIENT_METADATA_CA_FILE" +TRUE_VALUES = frozenset({"1", "true", "yes"}) + + +class OAuthClientMetadataDocument(BaseModel): + client_id: AnyUrl + redirect_uris: list[AnyUrl] + grant_types: list[Literal["authorization_code", "refresh_token"]] + response_types: list[Literal["code"]] + token_endpoint_auth_method: Literal["none"] + + +class MockOidcStatus(BaseModel): + client_metadata_fetch_count: int + registration_request_count: int + last_client_id: str | None + + +class NoRedirectHandler(urllib.request.HTTPRedirectHandler): + def redirect_request(self, *_args: Any, **_kwargs: Any) -> None: + return None def _b64url_uint(value: int) -> str: @@ -71,11 +107,27 @@ def _b64url(data: bytes) -> str: class MockOidc: """Holds signing key + in-flight authorization codes.""" - def __init__(self, *, issuer: str, audience: str, scope: str, subject: str) -> None: + def __init__( + self, + *, + issuer: str, + audience: str, + scope: str, + subject: str, + cimd_only: bool = False, + expected_client_id: str | None = None, + client_metadata_ca_file: str | None = None, + ) -> None: self.issuer = issuer.rstrip("/") self.audience = audience self.scope = scope self.subject = subject + self.cimd_only = cimd_only + self.expected_client_id = expected_client_id + self.client_metadata_ca_file = client_metadata_ca_file + self.client_metadata_fetch_count = 0 + self.registration_request_count = 0 + self.last_client_id: str | None = None self._private_key = rsa.generate_private_key( public_exponent=65537, key_size=2048 ) @@ -87,6 +139,29 @@ def __init__(self, *, issuer: str, audience: str, scope: str, subject: str) -> N # code -> {code_challenge, redirect_uri, scope} self._codes: dict[str, dict[str, str]] = {} + def validate_client_metadata(self, client_id: str, redirect_uri: str) -> None: + if not self.cimd_only: + return + if client_id != self.expected_client_id: + raise ValueError("CIMD client_id does not match the expected URL") + + ssl_context = ssl.create_default_context(cafile=self.client_metadata_ca_file) + opener = urllib.request.build_opener( + urllib.request.ProxyHandler({}), + NoRedirectHandler(), + urllib.request.HTTPSHandler(context=ssl_context), + ) + with opener.open(client_id, timeout=10) as response: + metadata = OAuthClientMetadataDocument.model_validate_json(response.read()) + + if str(metadata.client_id) != client_id: + raise ValueError("CIMD client_id does not match the document URL") + if redirect_uri not in {str(uri) for uri in metadata.redirect_uris}: + raise ValueError("redirect_uri is not registered in the CIMD document") + + self.client_metadata_fetch_count += 1 + self.last_client_id = client_id + # -- JWKS ----------------------------------------------------------------- def jwks(self) -> dict[str, Any]: numbers = self._private_key.public_key().public_numbers() @@ -155,12 +230,11 @@ def mint_access_token(self, *, scope: str, client_id: str) -> str: def build_app(oidc: MockOidc) -> FastAPI: app = FastAPI(title="Mock OIDC IdP for MCP tests") - metadata = { + metadata: dict[str, Any] = { "issuer": oidc.issuer, "authorization_endpoint": f"{oidc.issuer}/authorize", "token_endpoint": f"{oidc.issuer}/token", "jwks_uri": f"{oidc.issuer}/jwks", - "registration_endpoint": f"{oidc.issuer}/register", "response_types_supported": ["code"], "grant_types_supported": ["authorization_code", "refresh_token"], "code_challenge_methods_supported": ["S256"], @@ -173,6 +247,10 @@ def build_app(oidc: MockOidc) -> FastAPI: "subject_types_supported": ["public"], "id_token_signing_alg_values_supported": ["RS256"], } + if oidc.cimd_only: + metadata["client_id_metadata_document_supported"] = True + else: + metadata["registration_endpoint"] = f"{oidc.issuer}/register" @app.get("/.well-known/oauth-authorization-server") @app.get("/.well-known/openid-configuration") @@ -187,9 +265,22 @@ def jwks() -> JSONResponse: def healthz() -> PlainTextResponse: return PlainTextResponse("ok") + @app.get("/test/status") + def test_status() -> MockOidcStatus: + return MockOidcStatus( + client_metadata_fetch_count=oidc.client_metadata_fetch_count, + registration_request_count=oidc.registration_request_count, + last_client_id=oidc.last_client_id, + ) + @app.post("/register") async def register(request: Request) -> JSONResponse: - """RFC 7591 dynamic client registration — accept anything, echo back.""" + """Serve DCR unless CIMD-only mode disables it.""" + oidc.registration_request_count += 1 + if oidc.cimd_only: + return JSONResponse( + {"error": "dynamic_client_registration_disabled"}, status_code=400 + ) body = await request.json() client_id = f"mock-client-{uuid.uuid4().hex[:12]}" return JSONResponse( @@ -218,9 +309,14 @@ def authorize( code_challenge: str = "", code_challenge_method: str = "S256", # noqa: ARG001 (accepted; only S256) scope: str = "", - client_id: str = "", # noqa: ARG001 (accepted; mock issues to any client) - ) -> RedirectResponse: + client_id: str = "", + ) -> Response: """No login page: immediately issue a code and redirect back.""" + try: + oidc.validate_client_metadata(client_id, redirect_uri) + except (OSError, ValueError) as exc: + return JSONResponse({"error": str(exc)}, status_code=400) + code = oidc.issue_code( code_challenge=code_challenge, redirect_uri=redirect_uri, scope=scope ) @@ -280,11 +376,25 @@ def main() -> None: audience = os.getenv("MOCK_OIDC_AUDIENCE", "api://mcp") scope = os.getenv("MOCK_OIDC_SCOPE", "mcp:use") subject = os.getenv("MOCK_OIDC_SUBJECT", "mock-user@example.com") - - oidc = MockOidc(issuer=issuer, audience=audience, scope=scope, subject=subject) + cimd_only = os.getenv(CIMD_ONLY_ENV_VAR, "").lower() in TRUE_VALUES + expected_client_id = os.getenv(EXPECTED_CLIENT_ID_ENV_VAR) + client_metadata_ca_file = os.getenv(CLIENT_METADATA_CA_FILE_ENV_VAR) + + oidc = MockOidc( + issuer=issuer, + audience=audience, + scope=scope, + subject=subject, + cimd_only=cimd_only, + expected_client_id=expected_client_id, + client_metadata_ca_file=client_metadata_ca_file, + ) app = build_app(oidc) - print(f"[mock-oidc] issuer={issuer} audience={audience} scope={scope}") + print( + f"[mock-oidc] issuer={issuer} audience={audience} " + f"scope={scope} cimd_only={cimd_only}" + ) print(f"[mock-oidc] discovery: {issuer}/.well-known/oauth-authorization-server") print(f"[mock-oidc] jwks: {issuer}/jwks") print(json.dumps(oidc.jwks())) diff --git a/backend/tests/integration/tests/mcp_oauth/conftest.py b/backend/tests/integration/tests/mcp_oauth/conftest.py new file mode 100644 index 00000000000..bb36d4d8cae --- /dev/null +++ b/backend/tests/integration/tests/mcp_oauth/conftest.py @@ -0,0 +1,303 @@ +"""Test-owned HTTPS proxy and OAuth service fixtures for CIMD.""" + +from __future__ import annotations + +import ipaddress +import os +import socket +import subprocess +import sys +import threading +import time +from collections.abc import Generator +from datetime import UTC, datetime, timedelta +from pathlib import Path + +import httpx +import pytest +import uvicorn +from cryptography import x509 +from cryptography.hazmat.primitives import hashes, serialization +from cryptography.hazmat.primitives.asymmetric import rsa +from cryptography.x509.oid import NameOID +from fastapi.testclient import TestClient + +from onyx.server.features.mcp import client_metadata +from tests.integration.common_utils.cimd_oauth import ( + CimdHttpsEndpoint, + CimdOAuthTestServices, +) + +NGINX_COMMAND = "nginx" +STARTUP_TIMEOUT_SECONDS = 30.0 + +MOCK_SERVER_DIR = ( + Path(__file__).resolve().parents[2] / "mock_services" / "mcp_test_server" +) +MOCK_OIDC_SCRIPT = MOCK_SERVER_DIR / "run_mock_oidc_idp.py" +MCP_OAUTH_SERVER_SCRIPT = MOCK_SERVER_DIR / "run_mcp_server_oauth.py" + + +def _available_port() -> int: + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: + sock.bind(("0.0.0.0", 0)) + return int(sock.getsockname()[1]) + + +def _wait_for_port( + host: str, + port: int, + process: subprocess.Popen[bytes] | None = None, +) -> None: + deadline = time.monotonic() + STARTUP_TIMEOUT_SECONDS + while time.monotonic() < deadline: + if process is not None and process.poll() is not None: + raise RuntimeError( + f"Process exited during startup with code {process.returncode}" + ) + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock: + sock.settimeout(0.5) + try: + sock.connect((host, port)) + return + except OSError: + time.sleep(0.1) + raise TimeoutError(f"Timed out waiting for {host}:{port}") + + +def _stop_process(process: subprocess.Popen[bytes]) -> None: + process.terminate() + try: + process.wait(timeout=5) + except subprocess.TimeoutExpired: + process.kill() + process.wait(timeout=5) + + +def _write_certificate(directory: Path, hostname: str) -> tuple[Path, Path]: + key = rsa.generate_private_key(public_exponent=65537, key_size=2048) + subject = x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, hostname)]) + now = datetime.now(UTC) + certificate = ( + x509.CertificateBuilder() + .subject_name(subject) + .issuer_name(subject) + .public_key(key.public_key()) + .serial_number(x509.random_serial_number()) + .not_valid_before(now - timedelta(minutes=1)) + .not_valid_after(now + timedelta(days=1)) + .add_extension( + x509.SubjectAlternativeName( + [ + x509.IPAddress(ipaddress.ip_address(hostname)), + x509.DNSName("localhost"), + ] + ), + critical=False, + ) + .add_extension( + x509.BasicConstraints(ca=True, path_length=None), + critical=True, + ) + .sign(key, hashes.SHA256()) + ) + + certificate_path = directory / "mcp-cimd.crt" + key_path = directory / "mcp-cimd.key" + certificate_path.write_bytes(certificate.public_bytes(serialization.Encoding.PEM)) + key_path.write_bytes( + key.private_bytes( + serialization.Encoding.PEM, + serialization.PrivateFormat.PKCS8, + serialization.NoEncryption(), + ) + ) + return certificate_path, key_path + + +def _nginx_config( + directory: Path, + https_port: int, + certificate_path: Path, + key_path: Path, + upstream_port: int, +) -> str: + return f""" +pid {directory / "nginx.pid"}; +error_log stderr notice; +events {{}} +http {{ + access_log off; + server {{ + listen 127.0.0.1:{https_port} ssl; + ssl_certificate {certificate_path}; + ssl_certificate_key {key_path}; + + location /api/ {{ + rewrite ^/api/(.*)$ /$1 break; + proxy_pass http://127.0.0.1:{upstream_port}; + proxy_set_header Host $host; + }} + }} +}} +""" + + +@pytest.fixture(scope="session") +def cimd_api_server( + _test_client: TestClient, +) -> Generator[int, None, None]: + port = _available_port() + server = uvicorn.Server( + uvicorn.Config( + _test_client.app, + host="0.0.0.0", + port=port, + log_level="warning", + lifespan="off", + ) + ) + thread = threading.Thread(target=server.run, daemon=True) + thread.start() + _wait_for_port("127.0.0.1", port) + + try: + yield port + finally: + server.should_exit = True + thread.join(timeout=10) + + +@pytest.fixture(scope="session") +def cimd_https_endpoint( + cimd_api_server: int, + tmp_path_factory: pytest.TempPathFactory, +) -> Generator[CimdHttpsEndpoint, None, None]: + public_host = "127.0.0.1" + https_port = _available_port() + directory = tmp_path_factory.mktemp("mcp-cimd-tls") + certificate_path, key_path = _write_certificate(directory, public_host) + config_path = directory / "nginx.conf" + config_path.write_text( + _nginx_config( + directory, + https_port, + certificate_path, + key_path, + cimd_api_server, + ), + encoding="utf-8", + ) + log_path = directory / "nginx.log" + log_file = log_path.open("wb") + try: + process = subprocess.Popen( + [NGINX_COMMAND, "-c", str(config_path), "-g", "daemon off;"], + stdout=log_file, + stderr=subprocess.STDOUT, + ) + except OSError as error: + log_file.close() + raise RuntimeError(f"Failed to start Nginx: {error}") from error + + web_domain_patch = pytest.MonkeyPatch() + try: + try: + _wait_for_port(public_host, https_port, process) + except (RuntimeError, TimeoutError) as error: + log_file.flush() + logs = log_path.read_text(encoding="utf-8", errors="replace") + raise RuntimeError( + f"Nginx failed during startup: {error}\n{logs}" + ) from error + origin = f"https://{public_host}:{https_port}" + web_domain_patch.setattr(client_metadata, "WEB_DOMAIN", origin) + + metadata_url = f"{origin}/api/mcp/oauth/client-metadata" + deadline = time.monotonic() + STARTUP_TIMEOUT_SECONDS + while time.monotonic() < deadline: + try: + response = httpx.get( + metadata_url, + verify=str(certificate_path), + timeout=1, + ) + if response.status_code == 200: + break + except httpx.HTTPError: + time.sleep(0.1) + else: + log_file.flush() + logs = log_path.read_text(encoding="utf-8", errors="replace") + raise RuntimeError(f"CIMD HTTPS endpoint did not start:\n{logs}") + + yield CimdHttpsEndpoint(origin=origin, ca_file=certificate_path) + finally: + web_domain_patch.undo() + _stop_process(process) + log_file.close() + + +@pytest.fixture(scope="module") +def cimd_oauth_services( + cimd_https_endpoint: CimdHttpsEndpoint, + tmp_path_factory: pytest.TempPathFactory, +) -> Generator[CimdOAuthTestServices, None, None]: + oidc_port = _available_port() + mcp_port = _available_port() + oidc_issuer = f"http://127.0.0.1:{oidc_port}" + mcp_server_url = f"http://127.0.0.1:{mcp_port}/mcp" + client_metadata_url = f"{cimd_https_endpoint.origin}/api/mcp/oauth/client-metadata" + log_directory = tmp_path_factory.mktemp("mcp-cimd-services") + oidc_log = (log_directory / "oidc.log").open("wb") + mcp_log = (log_directory / "mcp.log").open("wb") + + oidc_env = { + **os.environ, + "MOCK_OIDC_PORT": str(oidc_port), + "MOCK_OIDC_BIND_HOST": "0.0.0.0", + "MOCK_OIDC_ISSUER": oidc_issuer, + "MOCK_OIDC_CIMD_ONLY": "true", + "MOCK_OIDC_EXPECTED_CLIENT_ID": client_metadata_url, + "MOCK_OIDC_CLIENT_METADATA_CA_FILE": str(cimd_https_endpoint.ca_file), + } + oidc_process = subprocess.Popen( + [sys.executable, str(MOCK_OIDC_SCRIPT), str(oidc_port)], + cwd=MOCK_SERVER_DIR, + env=oidc_env, + stdout=oidc_log, + stderr=subprocess.STDOUT, + ) + + mcp_process: subprocess.Popen[bytes] | None = None + try: + _wait_for_port("127.0.0.1", oidc_port, oidc_process) + mcp_env = { + **os.environ, + "MCP_SERVER_HOST": "0.0.0.0", + "MCP_SERVER_PUBLIC_URL": mcp_server_url, + "MCP_OAUTH_ISSUER": oidc_issuer, + "MCP_OAUTH_JWKS_URI": f"{oidc_issuer}/jwks", + "MCP_OAUTH_AUDIENCE": "api://mcp", + "MCP_OAUTH_REQUIRED_SCOPES": "mcp:use", + } + mcp_process = subprocess.Popen( + [sys.executable, str(MCP_OAUTH_SERVER_SCRIPT), str(mcp_port)], + cwd=MOCK_SERVER_DIR, + env=mcp_env, + stdout=mcp_log, + stderr=subprocess.STDOUT, + ) + _wait_for_port("127.0.0.1", mcp_port, mcp_process) + + yield CimdOAuthTestServices( + mcp_server_url=mcp_server_url, + oidc_issuer=oidc_issuer, + client_metadata_url=client_metadata_url, + ) + finally: + if mcp_process is not None: + _stop_process(mcp_process) + _stop_process(oidc_process) + mcp_log.close() + oidc_log.close() diff --git a/backend/tests/integration/tests/mcp_oauth/test_mcp_oauth_cimd_integration.py b/backend/tests/integration/tests/mcp_oauth/test_mcp_oauth_cimd_integration.py new file mode 100644 index 00000000000..250d1933b3e --- /dev/null +++ b/backend/tests/integration/tests/mcp_oauth/test_mcp_oauth_cimd_integration.py @@ -0,0 +1,186 @@ +"""Verify CIMD-only OAuth through Onyx API endpoints and a protected MCP server.""" + +from urllib.parse import parse_qs, urlparse + +import httpx + +from onyx.db.enums import ( + MCPAuthenticationPerformer, + MCPAuthenticationType, + MCPOAuthProviderMode, + MCPTransport, +) +from tests.integration.common_utils.cimd_oauth import ( + CimdOAuthTestServices, + MockOidcStatus, +) +from tests.integration.common_utils.constants import API_SERVER_URL +from tests.integration.common_utils.http_client import client +from tests.integration.common_utils.managers.chat import ChatSessionManager +from tests.integration.common_utils.managers.persona import PersonaManager +from tests.integration.common_utils.test_models import ( + DATestLLMProvider, + DATestPersona, + DATestUser, +) + +MCP_SERVER_NAME = "integration-mcp-cimd" +RETURN_PATH = "/admin/actions/mcp" +MCP_TOOL_NAME = "tool_0" + + +def _complete_oauth_flow( + server_id: int, + admin_user: DATestUser, + services: CimdOAuthTestServices, + *, + force_reauthentication: bool, +) -> None: + connect_response = client.post( + f"{API_SERVER_URL}/admin/mcp/oauth/connect", + json={ + "server_id": server_id, + "return_path": RETURN_PATH, + "include_resource_param": True, + "force_reauthentication": force_reauthentication, + }, + headers=admin_user.headers, + cookies=admin_user.cookies, + ) + connect_response.raise_for_status() + oauth_url = str(connect_response.json()["oauth_url"]) + assert oauth_url.startswith(f"{services.oidc_issuer}/authorize?") + + authorization_response = httpx.get( + oauth_url, + follow_redirects=False, + timeout=10, + ) + assert authorization_response.status_code == 302 + callback_url = authorization_response.headers["location"] + callback_params = parse_qs(urlparse(callback_url).query) + + callback_response = client.post( + f"{API_SERVER_URL}/mcp/oauth/callback", + params={ + "code": callback_params["code"][0], + "state": callback_params["state"][0], + }, + headers=admin_user.headers, + cookies=admin_user.cookies, + ) + callback_response.raise_for_status() + assert callback_response.json()["success"] is True + + +def test_mcp_oauth_cimd_only_flow( + cimd_oauth_services: CimdOAuthTestServices, + admin_user: DATestUser, + llm_provider: DATestLLMProvider, # noqa: ARG001 +) -> None: + discovery_response = httpx.get( + f"{cimd_oauth_services.oidc_issuer}/.well-known/oauth-authorization-server", + timeout=10, + ) + discovery_response.raise_for_status() + assert discovery_response.json()["client_id_metadata_document_supported"] is True + assert "registration_endpoint" not in discovery_response.json() + + create_response = client.post( + f"{API_SERVER_URL}/admin/mcp/servers/create", + json={ + "name": MCP_SERVER_NAME, + "description": "CIMD-only OAuth integration server", + "server_url": cimd_oauth_services.mcp_server_url, + "transport": MCPTransport.STREAMABLE_HTTP.value, + "auth_type": MCPAuthenticationType.OAUTH.value, + "auth_performer": MCPAuthenticationPerformer.PER_USER.value, + "oauth_provider_mode": MCPOAuthProviderMode.AUTO_DISCOVERY.value, + "is_public": True, + }, + headers=admin_user.headers, + cookies=admin_user.cookies, + ) + create_response.raise_for_status() + server_id = int(create_response.json()["server_id"]) + persona: DATestPersona | None = None + + try: + _complete_oauth_flow( + server_id, + admin_user, + cimd_oauth_services, + force_reauthentication=False, + ) + + tools_response = client.get( + f"{API_SERVER_URL}/admin/mcp/server/{server_id}/tools", + headers=admin_user.headers, + cookies=admin_user.cookies, + ) + tools_response.raise_for_status() + tool_names = {tool["name"] for tool in tools_response.json()["tools"]} + assert MCP_TOOL_NAME in tool_names + + db_tools_response = client.get( + f"{API_SERVER_URL}/admin/mcp/server/{server_id}/db-tools", + headers=admin_user.headers, + cookies=admin_user.cookies, + ) + db_tools_response.raise_for_status() + tool_id = next( + int(tool["id"]) + for tool in db_tools_response.json()["tools"] + if tool["name"] == MCP_TOOL_NAME + ) + persona = PersonaManager.create( + name="integration-mcp-cimd-persona", + tool_ids=[tool_id], + user_performing_action=admin_user, + ) + + _complete_oauth_flow( + server_id, + admin_user, + cimd_oauth_services, + force_reauthentication=True, + ) + + chat_session = ChatSessionManager.create( + persona_id=persona.id, + user_performing_action=admin_user, + ) + chat_response = ChatSessionManager.send_message( + chat_session_id=chat_session.id, + message="Invoke the CIMD MCP tool.", + user_performing_action=admin_user, + forced_tool_ids=[tool_id], + mock_llm_response=( + '{"name":"tool_0","arguments":{"name":"integration-test"}}' + ), + ) + assert chat_response.error is None + assert any( + tool_call.tool_name == MCP_TOOL_NAME + and tool_call.tool_args == {"name": "integration-test"} + for tool_call in chat_response.tool_call_debug + ) + + status_response = httpx.get( + f"{cimd_oauth_services.oidc_issuer}/test/status", + timeout=10, + ) + status_response.raise_for_status() + status = MockOidcStatus.model_validate(status_response.json()) + assert status.client_metadata_fetch_count >= 2 + assert status.registration_request_count == 0 + assert status.last_client_id == cimd_oauth_services.client_metadata_url + finally: + if persona is not None: + assert PersonaManager.delete(persona, admin_user) + delete_response = client.delete( + f"{API_SERVER_URL}/admin/mcp/server/{server_id}", + headers=admin_user.headers, + cookies=admin_user.cookies, + ) + delete_response.raise_for_status() diff --git a/backend/tests/integration/tests/reporting/test_usage_export_api.py b/backend/tests/integration/tests/reporting/test_usage_export_api.py index ca1996b697a..1ebb432e58f 100644 --- a/backend/tests/integration/tests/reporting/test_usage_export_api.py +++ b/backend/tests/integration/tests/reporting/test_usage_export_api.py @@ -309,6 +309,7 @@ def test_read_usage_report( "model", "flow", "provider", + "incognito", "input_tokens", "output_tokens", "cache_read_tokens", diff --git a/backend/tests/unit/ee/onyx/connectors/test_ee_capability_checks.py b/backend/tests/unit/ee/onyx/connectors/test_ee_capability_checks.py index e917eba9fa5..627054ad4d3 100644 --- a/backend/tests/unit/ee/onyx/connectors/test_ee_capability_checks.py +++ b/backend/tests/unit/ee/onyx/connectors/test_ee_capability_checks.py @@ -52,15 +52,31 @@ def test_censoring_only_source_has_no_perm_sync_capabilities() -> None: def test_probeless_sync_source_gets_no_fallback() -> None: """ - Verifies the no-trivial-pass rule: Slack and Gmail are sync-capable, but - their legacy ``validate_perm_sync`` dispatch is a no-op, so no fallback is - synthesized and no verdict can pass on the basis of a no-op probe. + Verifies the no-trivial-pass rule: Gmail is sync-capable, but its legacy + ``validate_perm_sync`` dispatch is a no-op, so no fallback is synthesized + and no verdict can pass on the basis of a no-op probe. """ # Under test and postcondition. - assert get_perm_sync_capability_checks(DocumentSource.SLACK) == [] assert get_perm_sync_capability_checks(DocumentSource.GMAIL) == [] +def test_slack_registers_named_doc_sync_checks_only() -> None: + """ + Verifies Slack's registered perm-sync suite: named DOC_PERMISSION_SYNC + checks (no fallback), and nothing under EXTERNAL_GROUP_SYNC, which is not + applicable for Slack by design. + """ + # Under test. + checks = get_perm_sync_capability_checks(DocumentSource.SLACK) + + # Postcondition. + assert checks + assert {check.capability for check in checks} == { + CredentialCapability.DOC_PERMISSION_SYNC + } + assert not any(check.is_fallback for check in checks) + + def test_fallback_synthesis_respects_applicability( monkeypatch: pytest.MonkeyPatch, ) -> None: diff --git a/backend/tests/unit/ee/onyx/server/reporting/test_usage_report_data.py b/backend/tests/unit/ee/onyx/server/reporting/test_usage_report_data.py index 331b497b1f2..702269966d8 100644 --- a/backend/tests/unit/ee/onyx/server/reporting/test_usage_report_data.py +++ b/backend/tests/unit/ee/onyx/server/reporting/test_usage_report_data.py @@ -34,12 +34,14 @@ def _row( cost: float = 10.0, day: str = "2026-07-01", flow: str = "chat", + incognito: bool = False, ) -> UsageExportRow: return UsageExportRow( email=email, model="gpt-5", flow=flow, provider="openai", + incognito=incognito, day=day, input_tokens=100, output_tokens=50, diff --git a/backend/tests/unit/onyx/background/celery/tasks/test_user_file_processing_no_vectordb.py b/backend/tests/unit/onyx/background/celery/tasks/test_user_file_processing_no_vectordb.py index af29c5b1e17..cb39e46a4e1 100644 --- a/backend/tests/unit/onyx/background/celery/tasks/test_user_file_processing_no_vectordb.py +++ b/backend/tests/unit/onyx/background/celery/tasks/test_user_file_processing_no_vectordb.py @@ -44,6 +44,7 @@ def _make_user_file( status: UserFileStatus = UserFileStatus.PROCESSING, file_id: str = "test-file-id", name: str = "test.txt", + incognito: bool = False, ) -> MagicMock: """Return a MagicMock mimicking a UserFile ORM instance.""" uf = MagicMock() @@ -51,6 +52,9 @@ def _make_user_file( uf.file_id = file_id uf.name = name uf.status = status + # Explicit: an attribute left to MagicMock reads as truthy and would send + # every file down the incognito branch. + uf.incognito = incognito uf.token_count = None uf.chunk_count = None uf.last_project_sync_at = None @@ -276,6 +280,36 @@ def test_calls_with_indexing_when_vector_db_enabled( mock_with_indexing.assert_called_once() mock_without_vdb.assert_not_called() + @patch(f"{TASKS_MODULE}._process_user_file_without_vector_db") + @patch(f"{TASKS_MODULE}._process_user_file_with_indexing") + @patch(f"{TASKS_MODULE}.DISABLE_VECTOR_DB", False) + @patch(f"{TASKS_MODULE}.get_session_with_current_tenant") + def test_incognito_upload_skips_the_search_index( + self, + mock_get_session: MagicMock, + mock_with_indexing: MagicMock, + mock_without_vdb: MagicMock, + ) -> None: + """An incognito upload is extracted for chat but never indexed, even + with the vector DB enabled.""" + uf = _make_user_file(incognito=True) + session = MagicMock() + session.get.return_value = uf + mock_get_session.return_value.__enter__.return_value = session + + connector_mock = MagicMock() + connector_mock.load_from_state.return_value = [_make_documents(["hello"])] + + with patch(f"{TASKS_MODULE}.LocalFileConnector", return_value=connector_mock): + process_user_file_impl( + user_file_id=str(uf.id), + tenant_id="test-tenant", + redis_locking=False, + ) + + mock_without_vdb.assert_called_once() + mock_with_indexing.assert_not_called() + @patch(f"{TASKS_MODULE}.run_indexing_pipeline") @patch(f"{TASKS_MODULE}.store_user_file_plaintext") @patch(f"{TASKS_MODULE}.DISABLE_VECTOR_DB", True) diff --git a/backend/tests/unit/onyx/chat/test_incognito_record_mode.py b/backend/tests/unit/onyx/chat/test_incognito_record_mode.py index 15b97458cc0..aa720475fd4 100644 --- a/backend/tests/unit/onyx/chat/test_incognito_record_mode.py +++ b/backend/tests/unit/onyx/chat/test_incognito_record_mode.py @@ -10,11 +10,17 @@ import pytest from onyx.chat.incognito import ( + BIFROST_DISABLE_CONTENT_LOGGING_HEADER, + LITELLM_PROXY_REDACTION_HEADER, + PORTKEY_DEBUG_HEADER, content_free_file_descriptors, + incognito_llm_extra_body, + incognito_llm_extra_headers, resolve_incognito_record_mode, ) from onyx.db.enums import IncognitoRecordMode from onyx.file_store.models import ChatFileType, FileDescriptor +from onyx.llm.well_known_providers.constants import BIFROST_PROVIDER_NAME class TestModeSinkMatrix: @@ -103,3 +109,71 @@ def test_strips_name_and_keeps_linkage(self) -> None: FileDescriptor(id="file-1", type=ChatFileType.DOC, user_file_id="uf-1") ] assert "name" not in scrubbed[0] + + +class TestBifrostHeaders: + def test_usage_only_on_bifrost_sends_the_header(self) -> None: + assert incognito_llm_extra_headers( + IncognitoRecordMode.USAGE_ONLY, BIFROST_PROVIDER_NAME + ) == {BIFROST_DISABLE_CONTENT_LOGGING_HEADER: "true"} + + def test_header_name_is_the_wire_contract(self) -> None: + """Bifrost matches on this exact string. Renaming it silently re-enables + gateway content logging for every content-free incognito chat.""" + assert BIFROST_DISABLE_CONTENT_LOGGING_HEADER == "x-bf-disable-content-logging" + + def test_full_history_sends_nothing(self) -> None: + """The workspace chose to record, so the gateway log stays consistent.""" + assert ( + incognito_llm_extra_headers( + IncognitoRecordMode.FULL_HISTORY, BIFROST_PROVIDER_NAME + ) + == {} + ) + + def test_ordinary_chat_sends_nothing(self) -> None: + assert incognito_llm_extra_headers(None, BIFROST_PROVIDER_NAME) == {} + + def test_other_providers_get_no_bifrost_header(self) -> None: + for provider in ("openai", "anthropic", "", None): + assert ( + incognito_llm_extra_headers(IncognitoRecordMode.USAGE_ONLY, provider) + == {} + ) + + +class TestProviderPolicyMatrix: + """Pins the per-provider retention suppression each content-free turn + sends. A provider absent here has no per-request option.""" + + def test_gateway_headers(self) -> None: + assert incognito_llm_extra_headers( + IncognitoRecordMode.USAGE_ONLY, "portkey" + ) == {PORTKEY_DEBUG_HEADER: "false"} + assert incognito_llm_extra_headers( + IncognitoRecordMode.USAGE_ONLY, "litellm_proxy" + ) == {LITELLM_PROXY_REDACTION_HEADER: "true"} + + def test_store_false_for_openai_family(self) -> None: + for provider in ("openai", "azure"): + assert incognito_llm_extra_body( + IncognitoRecordMode.USAGE_ONLY, provider + ) == {"store": False} + + def test_openrouter_denies_data_collection(self) -> None: + assert incognito_llm_extra_body( + IncognitoRecordMode.USAGE_ONLY, "openrouter" + ) == {"extra_body": {"provider": {"data_collection": "deny"}}} + + def test_no_option_providers_send_nothing(self) -> None: + for provider in ("anthropic", "google", "vertex_ai", "mistral", "bedrock"): + assert ( + incognito_llm_extra_body(IncognitoRecordMode.USAGE_ONLY, provider) == {} + ) + + def test_full_history_sends_no_body_params(self) -> None: + for provider in ("openai", "azure", "openrouter"): + assert ( + incognito_llm_extra_body(IncognitoRecordMode.FULL_HISTORY, provider) + == {} + ) diff --git a/backend/tests/unit/onyx/connectors/capability_checks/test_generate_capability_report.py b/backend/tests/unit/onyx/connectors/capability_checks/test_generate_capability_report.py index 1d2086f5c32..ebb8e49d660 100644 --- a/backend/tests/unit/onyx/connectors/capability_checks/test_generate_capability_report.py +++ b/backend/tests/unit/onyx/connectors/capability_checks/test_generate_capability_report.py @@ -16,6 +16,7 @@ from onyx.connectors.capability_checks.runner import generate_capability_report from onyx.connectors.exceptions import CredentialInvalidError from onyx.connectors.interfaces import BaseConnector +from onyx.connectors.source_operations import SourceOperations class _CallableCheck(CapabilityCheck): @@ -229,6 +230,72 @@ def test_real_config_unlocks_config_requiring_checks( assert report.check_results[0].status == CapabilityCheckStatus.PASSED +def test_registered_gateway_is_constructed_and_reaches_checks( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """ + Verifies the runner constructs the registered gateway uniformly (provider + plus the supplied config) and hands it to checks via the context. + """ + # Precondition. + seen_contexts: list[CapabilityCheckContext] = [] + check = _CallableCheck( + seen_contexts.append, + check_id="gateway_check", + requires_connector_instance=False, + ) + _patch_runner_environment(monkeypatch, [check]) + provider = MagicMock() + monkeypatch.setattr( + runner_module, + "build_db_credentials_provider", + MagicMock(return_value=provider), + ) + gateway_instance = MagicMock(spec=SourceOperations) + gateway_class = MagicMock(return_value=gateway_instance) + monkeypatch.setattr( + runner_module, + "get_source_operations_class", + MagicMock(return_value=gateway_class), + ) + connector_specific_config = {"channels": ["general"]} + + # Under test. + generate_capability_report( + MagicMock(), + _make_credential(), + connector_specific_config=connector_specific_config, + ) + + # Postcondition. + gateway_class.assert_called_once_with( + credentials_provider=provider, + connector_specific_config=connector_specific_config, + ) + assert seen_contexts[0].source_operations is gateway_instance + + +def test_unregistered_source_gets_no_gateway( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Verifies the context carries no gateway for unmigrated sources.""" + # Precondition. + # Github has no registered gateway, so the real lookup returns None. + seen_contexts: list[CapabilityCheckContext] = [] + check = _CallableCheck( + seen_contexts.append, + check_id="gateway_check", + requires_connector_instance=False, + ) + _patch_runner_environment(monkeypatch, [check]) + + # Under test. + generate_capability_report(MagicMock(), _make_credential()) + + # Postcondition. + assert seen_contexts[0].source_operations is None + + def test_report_decrypt_emits_a_credential_audit_event( monkeypatch: pytest.MonkeyPatch, ) -> None: diff --git a/backend/tests/unit/onyx/connectors/slack/test_slack_capability_checks.py b/backend/tests/unit/onyx/connectors/slack/test_slack_capability_checks.py new file mode 100644 index 00000000000..2d595c34949 --- /dev/null +++ b/backend/tests/unit/onyx/connectors/slack/test_slack_capability_checks.py @@ -0,0 +1,927 @@ +"""Behavior tests for the Slack capability checks. + +Checks compose gateway operations, so every test runs a check against an +autospecced ``SlackSourceOperations`` whose operations return the gateway's +typed page models; probe reach (which operations each check exercises) is +enforced separately by the auto-discovering coverage harness. +""" + +from typing import Any +from unittest.mock import MagicMock, create_autospec + +import pytest + +from onyx.configs.constants import DocumentSource +from onyx.connectors.capability_checks.models import ( + CapabilityCheckContext, + CapabilityCheckStatus, + CapabilityVerdict, + CredentialCapability, + compute_capability_verdicts, +) +from onyx.connectors.capability_checks.registry import get_capability_checks +from onyx.connectors.capability_checks.runner import run_capability_checks +from onyx.connectors.exceptions import ( + ConnectorValidationError, + CredentialExpiredError, + CredentialInvalidError, + InsufficientPermissionsError, + UnexpectedValidationError, +) +from onyx.connectors.slack.capability_checks import ( + build_slack_doc_permission_sync_checks, + build_slack_indexing_checks, +) +from onyx.connectors.slack.source_operations import ( + SlackApiError, + SlackAuthTestResponse, + SlackChannelMembersPage, + SlackChannelsPage, + SlackChannelVariant, + SlackHistoryPage, + SlackSourceOperations, + SlackTeamsPage, + SlackUsersPage, +) + +_ALL_INDEXING_SCOPES = [ + "channels:read", + "channels:history", + "channels:join", + "groups:read", + "groups:history", + "users:read", + "users:read.email", +] + +_CHECKS_BY_ID = { + check.check_id: check + for check in build_slack_indexing_checks() + + build_slack_doc_permission_sync_checks() +} + + +def _slack_error(error: str, needed: str | None = None) -> SlackApiError: + data: dict[str, Any] = {"ok": False, "error": error} + if needed is not None: + data["needed"] = needed + return SlackApiError("slack api error", data) + + +def _auth_response( + enterprise_id: str | None = None, + granted_scopes: list[str] | None = None, +) -> SlackAuthTestResponse: + return SlackAuthTestResponse( + ok=True, + url="https://example.slack.com", + enterprise_id=enterprise_id, + granted_scopes=granted_scopes, + ) + + +def _gateway() -> MagicMock: + """An autospecced gateway; tests set per-operation returns as needed.""" + gateway = create_autospec(SlackSourceOperations, instance=True) + gateway.check_auth.return_value = _auth_response() + return gateway + + +def _context( + gateway: MagicMock, + credential_json: dict[str, Any] | None = None, + connector_specific_config: dict[str, Any] | None = None, +) -> CapabilityCheckContext: + return CapabilityCheckContext( + source=DocumentSource.SLACK, + credential_json=( + {"slack_bot_token": "xoxb-test-token"} + if credential_json is None + else credential_json + ), + connector_specific_config=connector_specific_config, + source_operations=gateway, + ) + + +def _run(check_id: str, context: CapabilityCheckContext) -> None: + _CHECKS_BY_ID[check_id].run(context) + + +def _pages(page: Any) -> Any: + """A ``side_effect`` yielding a fresh one-page generator per call.""" + + def make(**kwargs: Any) -> Any: + del kwargs + return iter([page]) + + return make + + +def test_token_auth_missing_token() -> None: + """ + Verifies that an absent ``slack_bot_token`` key is a credential failure. + """ + # Under test and postcondition. + with pytest.raises(CredentialInvalidError, match="slack_bot_token"): + _run("slack_token_auth", _context(_gateway(), credential_json={})) + + +def test_token_auth_rejects_user_token() -> None: + """Verifies that non-bot tokens (no ``xoxb-`` prefix) are rejected.""" + # Under test and postcondition. + with pytest.raises(CredentialInvalidError, match="xoxb"): + _run( + "slack_token_auth", + _context(_gateway(), credential_json={"slack_bot_token": "xoxp-user"}), + ) + + +def test_token_auth_maps_invalid_auth_to_expired() -> None: + """Verifies that ``invalid_auth`` from ``auth.test`` maps to expiry.""" + # Precondition. + gateway = _gateway() + gateway.check_auth.side_effect = _slack_error("invalid_auth") + + # Under test and postcondition. + with pytest.raises(CredentialExpiredError, match="invalid_auth"): + _run("slack_token_auth", _context(gateway)) + + +def test_token_auth_maps_token_expired_to_expired() -> None: + """Verifies ``token_expired`` (apps with token rotation) maps to expiry.""" + # Precondition. + gateway = _gateway() + gateway.check_auth.side_effect = _slack_error("token_expired") + + # Under test and postcondition. + with pytest.raises(CredentialExpiredError, match="token_expired"): + _run("slack_token_auth", _context(gateway)) + + +def test_token_auth_passes_for_valid_bot_token() -> None: + """Verifies the happy path for token validation.""" + # Under test and postcondition. + _run("slack_token_auth", _context(_gateway())) + + +def test_public_channel_listing_missing_scope() -> None: + """Verifies the ``channels:read`` probe shape and its failure mapping.""" + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _slack_error( + "missing_scope", needed="channels:read" + ) + + # Under test and postcondition. + with pytest.raises(InsufficientPermissionsError, match="channels:read"): + _run("slack_public_channel_listing", _context(gateway)) + gateway.list_channels.assert_called_once_with( + variant=SlackChannelVariant.PUBLIC, + channel_types=["public_channel"], + limit=1, + ) + + +def test_public_channel_listing_probes_channel_info() -> None: + """Verifies the net-new ``conversations.info`` probe on a listed channel.""" + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _pages( + SlackChannelsPage(channels=[{"id": "C1", "name": "general"}]) + ) + gateway.fetch_channel_info.side_effect = _slack_error( + "missing_scope", needed="channels:read" + ) + + # Under test and postcondition. + with pytest.raises(InsufficientPermissionsError, match="conversations.info"): + _run("slack_public_channel_listing", _context(gateway)) + gateway.fetch_channel_info.assert_called_once_with(channel_id="C1") + + +def test_public_channel_listing_empty_workspace_skips_info_probe() -> None: + """Verifies an empty listing still proves the listing scope.""" + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _pages(SlackChannelsPage(channels=[])) + + # Under test. + _run("slack_public_channel_listing", _context(gateway)) + + # Postcondition. + gateway.fetch_channel_info.assert_not_called() + + +def test_private_channel_listing_missing_scope_mentions_silent_skip() -> None: + """ + Verifies the ``groups:read`` probe explains the silent-fallback hazard. + """ + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _slack_error( + "missing_scope", needed="groups:read" + ) + + # Under test and postcondition. + with pytest.raises(InsufficientPermissionsError, match="silently"): + _run("slack_private_channel_listing", _context(gateway)) + gateway.list_channels.assert_called_once_with( + variant=SlackChannelVariant.PRIVATE, + channel_types=["private_channel"], + limit=1, + ) + + +def test_message_history_prefers_member_channel() -> None: + """Verifies the history probe targets a channel the bot is a member of.""" + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _pages( + SlackChannelsPage( + channels=[ + {"id": "C_NOT_MEMBER", "name": "a", "is_member": False}, + {"id": "C_MEMBER", "name": "b", "is_member": True}, + ] + ) + ) + gateway.fetch_channel_history.side_effect = _pages(SlackHistoryPage(messages=[])) + + # Under test. + _run("slack_message_history_read", _context(gateway)) + + # Postcondition. + gateway.fetch_channel_history.assert_called_once_with( + variant=SlackChannelVariant.PUBLIC, channel_id="C_MEMBER", limit=1 + ) + + +def test_message_history_not_in_channel_counts_as_scope_proof() -> None: + """Verifies ``not_in_channel`` passes: membership is fixed by auto-join.""" + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _pages( + SlackChannelsPage(channels=[{"id": "C1", "name": "a", "is_member": False}]) + ) + gateway.fetch_channel_history.side_effect = _slack_error("not_in_channel") + + # Under test and postcondition. + _run("slack_message_history_read", _context(gateway)) + + +def test_message_history_missing_scope() -> None: + """Verifies a missing history scope is a real failure.""" + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _pages( + SlackChannelsPage(channels=[{"id": "C1", "name": "a", "is_member": True}]) + ) + gateway.fetch_channel_history.side_effect = _slack_error( + "missing_scope", needed="channels:history" + ) + + # Under test and postcondition. + with pytest.raises(InsufficientPermissionsError, match="channels:history"): + _run("slack_message_history_read", _context(gateway)) + + +def test_message_history_no_visible_channels_is_indeterminate() -> None: + """Verifies an unprobeable workspace maps to a transient outcome.""" + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _pages(SlackChannelsPage(channels=[])) + + # Under test and postcondition. + with pytest.raises(UnexpectedValidationError, match="[Nn]o public channels"): + _run("slack_message_history_read", _context(gateway)) + + +def test_message_history_probes_a_thread() -> None: + """Verifies the net-new ``conversations.replies`` probe uses a real ts.""" + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _pages( + SlackChannelsPage(channels=[{"id": "C1", "name": "a", "is_member": True}]) + ) + gateway.fetch_channel_history.side_effect = _pages( + SlackHistoryPage(messages=[{"ts": "111.222"}]) + ) + gateway.fetch_thread_replies.side_effect = _slack_error( + "missing_scope", needed="channels:history" + ) + + # Under test and postcondition. + with pytest.raises(InsufficientPermissionsError, match="conversations.replies"): + _run("slack_message_history_read", _context(gateway)) + gateway.fetch_thread_replies.assert_called_once_with( + variant=SlackChannelVariant.PUBLIC, channel_id="C1", thread_ts="111.222" + ) + + +def test_message_history_empty_channel_skips_thread_probe() -> None: + """Verifies an empty channel proves history access without a thread.""" + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _pages( + SlackChannelsPage(channels=[{"id": "C1", "name": "a", "is_member": True}]) + ) + gateway.fetch_channel_history.side_effect = _pages(SlackHistoryPage(messages=[])) + + # Under test. + _run("slack_message_history_read", _context(gateway)) + + # Postcondition. + gateway.fetch_thread_replies.assert_not_called() + + +def test_private_history_passes_when_no_private_channels_in_scope() -> None: + """Verifies a bot in no private channels has no private history to probe.""" + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _pages(SlackChannelsPage(channels=[])) + + # Under test. + _run("slack_private_message_history_read", _context(gateway)) + + # Postcondition. + gateway.fetch_channel_history.assert_not_called() + + +def test_private_history_leaves_listing_scope_failures_to_the_listing_check() -> None: + """ + Verifies a listing scope failure does not double-report here: without + ``groups:read``, no private channels are in scope at all. + """ + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _slack_error( + "missing_scope", needed="groups:read" + ) + + # Under test. + _run("slack_private_message_history_read", _context(gateway)) + + # Postcondition. + gateway.fetch_channel_history.assert_not_called() + + +def test_private_history_rate_limited_listing_is_indeterminate() -> None: + """ + Verifies the listing gate passes only scope failures: a rate-limited listing + must not turn into a false pass of a required check. + """ + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _slack_error("ratelimited") + + # Under test and postcondition. + with pytest.raises(UnexpectedValidationError, match="rate limited"): + _run("slack_private_message_history_read", _context(gateway)) + + +def test_private_history_missing_groups_history_fails() -> None: + """ + Verifies the review P1 regression: a token that lists private channels but + cannot read them (``channels:history`` without ``groups:history``) must not + pass. + """ + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _pages( + SlackChannelsPage(channels=[{"id": "C_PRIV", "name": "secret"}]) + ) + gateway.fetch_channel_history.side_effect = _slack_error( + "missing_scope", needed="groups:history" + ) + + # Under test and postcondition. + with pytest.raises(InsufficientPermissionsError, match="groups:history"): + _run("slack_private_message_history_read", _context(gateway)) + gateway.fetch_channel_history.assert_called_once_with( + variant=SlackChannelVariant.PRIVATE, channel_id="C_PRIV", limit=1 + ) + + +def test_private_history_probes_a_private_thread() -> None: + """Verifies the thread probe carries the private variant.""" + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _pages( + SlackChannelsPage(channels=[{"id": "C_PRIV", "name": "secret"}]) + ) + gateway.fetch_channel_history.side_effect = _pages( + SlackHistoryPage(messages=[{"ts": "333.444"}]) + ) + gateway.fetch_thread_replies.side_effect = _pages(SlackHistoryPage()) + + # Under test. + _run("slack_private_message_history_read", _context(gateway)) + + # Postcondition. + gateway.fetch_thread_replies.assert_called_once_with( + variant=SlackChannelVariant.PRIVATE, channel_id="C_PRIV", thread_ts="333.444" + ) + + +def test_channel_join_scope_present_passes() -> None: + """Verifies the granted-scopes check accepts a granted ``channels:join``.""" + # Precondition. + gateway = _gateway() + gateway.check_auth.return_value = _auth_response( + granted_scopes=_ALL_INDEXING_SCOPES + ) + + # Under test and postcondition. + _run("slack_channel_join_scope", _context(gateway)) + + +def test_channel_join_scope_missing_fails() -> None: + """Verifies a missing ``channels:join`` scope is a real failure.""" + # Precondition. + gateway = _gateway() + gateway.check_auth.return_value = _auth_response( + granted_scopes=["channels:read", "users:read"] + ) + + # Under test and postcondition. + with pytest.raises(InsufficientPermissionsError, match="channels:join"): + _run("slack_channel_join_scope", _context(gateway)) + + +def test_channel_join_scope_absent_header_is_indeterminate() -> None: + """Verifies that a missing scopes header cannot fail the credential.""" + # Precondition. + gateway = _gateway() + gateway.check_auth.return_value = _auth_response(granted_scopes=None) + + # Under test and postcondition. + with pytest.raises(UnexpectedValidationError, match="X-OAuth-Scopes"): + _run("slack_channel_join_scope", _context(gateway)) + + +def test_grid_workspace_listing_noop_off_grid() -> None: + """Verifies non-Grid workspaces skip the ``auth.teams.list`` probe.""" + # Precondition. + gateway = _gateway() + + # Under test. + _run("slack_grid_workspace_listing", _context(gateway)) + + # Postcondition. + gateway.list_teams.assert_not_called() + + +def test_grid_workspace_listing_missing_team_read() -> None: + """Verifies the Grid ``team:read`` requirement is surfaced.""" + # Precondition. + gateway = _gateway() + gateway.check_auth.return_value = _auth_response(enterprise_id="E123") + gateway.list_teams.side_effect = _slack_error("missing_scope", needed="team:read") + + # Under test and postcondition. + with pytest.raises(InsufficientPermissionsError, match="team:read"): + _run("slack_grid_workspace_listing", _context(gateway)) + + +def test_user_profile_read_off_grid_omits_team_id() -> None: + """Verifies the non-Grid ``users.list`` call shape.""" + # Precondition. + gateway = _gateway() + gateway.list_users.side_effect = _pages(SlackUsersPage(members=[])) + + # Under test. + _run("slack_user_profile_read", _context(gateway)) + + # Postcondition. + gateway.list_users.assert_called_once_with(limit=1, team_id=None) + + +def test_user_profile_read_on_grid_passes_team_id() -> None: + """Verifies Grid installs mirror production and pass a ``team_id``.""" + # Precondition. + gateway = _gateway() + gateway.check_auth.return_value = _auth_response(enterprise_id="E123") + gateway.list_teams.side_effect = _pages(SlackTeamsPage(teams=[{"id": "T1"}])) + gateway.list_users.side_effect = _pages(SlackUsersPage(members=[])) + + # Under test. + _run("slack_user_profile_read", _context(gateway)) + + # Postcondition. + gateway.list_users.assert_called_once_with(limit=1, team_id="T1") + + +def test_user_profile_read_missing_scope() -> None: + """Verifies a missing ``users:read`` scope is a real failure.""" + # Precondition. + gateway = _gateway() + gateway.list_users.side_effect = _slack_error("missing_scope", needed="users:read") + + # Under test and postcondition. + with pytest.raises(InsufficientPermissionsError, match="users:read"): + _run("slack_user_profile_read", _context(gateway)) + + +def test_user_profile_read_resolves_a_single_profile() -> None: + """Verifies the net-new ``users.info`` probe mirrors author resolution.""" + # Precondition. + gateway = _gateway() + gateway.list_users.side_effect = _pages(SlackUsersPage(members=[{"id": "U1"}])) + gateway.fetch_user_info.side_effect = _slack_error( + "missing_scope", needed="users:read" + ) + + # Under test and postcondition. + with pytest.raises(InsufficientPermissionsError, match="users.info"): + _run("slack_user_profile_read", _context(gateway)) + gateway.fetch_user_info.assert_called_once_with("U1") + + +def test_email_visibility_passes_when_emails_present() -> None: + """Verifies the happy path: at least one human member exposes an email.""" + # Precondition. + gateway = _gateway() + gateway.list_users.side_effect = _pages( + SlackUsersPage( + members=[ + {"id": "B1", "is_bot": True, "deleted": False, "profile": {}}, + { + "id": "U1", + "is_bot": False, + "deleted": False, + "profile": {"email": "user@example.com"}, + }, + ] + ) + ) + + # Under test and postcondition. + _run("slack_user_email_visibility", _context(gateway)) + + +def test_email_visibility_fails_when_emails_absent() -> None: + """Verifies the content-based detection of a missing ``users:read.email``. + + Slack omits email fields rather than raising ``missing_scope``, so the check + must fail on successful-but-emailless responses. + """ + # Precondition. + gateway = _gateway() + gateway.list_users.side_effect = _pages( + SlackUsersPage( + members=[ + {"id": "U1", "is_bot": False, "deleted": False, "profile": {}}, + {"id": "U2", "is_bot": False, "deleted": False, "profile": {}}, + ] + ) + ) + + # Under test and postcondition. + with pytest.raises(InsufficientPermissionsError, match="users:read.email"): + _run("slack_user_email_visibility", _context(gateway)) + + +def test_email_visibility_indeterminate_without_human_sample() -> None: + """ + Verifies bots, deleted users, and Slackbot are excluded from the sample. + """ + # Precondition. + gateway = _gateway() + gateway.list_users.side_effect = _pages( + SlackUsersPage( + members=[ + {"id": "B1", "is_bot": True, "deleted": False, "profile": {}}, + {"id": "U1", "is_bot": False, "deleted": True, "profile": {}}, + {"id": "USLACKBOT", "is_bot": False, "deleted": False, "profile": {}}, + ] + ) + ) + + # Under test and postcondition. + with pytest.raises(UnexpectedValidationError, match="human members"): + _run("slack_user_email_visibility", _context(gateway)) + + +def test_perm_sync_channel_listing_missing_scope() -> None: + """Verifies the perm-sync enumeration probe and its failure mapping.""" + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _slack_error( + "missing_scope", needed="channels:read" + ) + + # Under test and postcondition. + with pytest.raises(InsufficientPermissionsError, match="permission sync"): + _run("slack_perm_sync_channel_listing", _context(gateway)) + gateway.list_channels.assert_called_once_with( + variant=SlackChannelVariant.PUBLIC, + channel_types=["public_channel"], + limit=1, + ) + + +def test_perm_sync_private_listing_warning_names_stale_access_lists() -> None: + """Verifies the warning states the silent degradation and its hazard.""" + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _slack_error( + "missing_scope", needed="groups:read" + ) + + # Under test and postcondition. + with pytest.raises(InsufficientPermissionsError, match="stale"): + _run("slack_perm_sync_private_channel_listing", _context(gateway)) + + +def test_private_member_listing_missing_listing_scope_passes() -> None: + """ + Verifies a listing scope failure is not reported here: doc sync silently + degrades to public-only, and the perm-sync private-listing warning check + owns that finding. + """ + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _slack_error( + "missing_scope", needed="groups:read" + ) + + # Under test. + _run("slack_private_channel_member_listing", _context(gateway)) + + # Postcondition. + gateway.list_channel_members.assert_not_called() + + +def test_private_member_listing_vacuous_pass_without_private_channels() -> None: + """Verifies a bot in no private channels has nothing to probe.""" + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _pages(SlackChannelsPage(channels=[])) + + # Under test. + _run("slack_private_channel_member_listing", _context(gateway)) + + # Postcondition. + gateway.list_channel_members.assert_not_called() + + +def test_private_member_listing_missing_scope() -> None: + """Verifies ``conversations.members`` failures surface for perm sync.""" + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _pages( + SlackChannelsPage(channels=[{"id": "C_PRIV", "name": "secret"}]) + ) + gateway.list_channel_members.side_effect = _slack_error( + "missing_scope", needed="groups:read" + ) + + # Under test and postcondition. + with pytest.raises(InsufficientPermissionsError, match="groups:read"): + _run("slack_private_channel_member_listing", _context(gateway)) + gateway.list_channel_members.assert_called_once_with(channel_id="C_PRIV") + + +def test_private_member_listing_resolves_one_member() -> None: + """ + Verifies the ``users.info`` step mirrors doc sync's external-user fallback. + """ + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _pages( + SlackChannelsPage(channels=[{"id": "C_PRIV", "name": "secret"}]) + ) + gateway.list_channel_members.side_effect = _pages( + SlackChannelMembersPage(members=["U_EXT"]) + ) + gateway.fetch_user_info.side_effect = _slack_error( + "missing_scope", needed="users:read" + ) + + # Under test and postcondition. + with pytest.raises(InsufficientPermissionsError, match="users.info"): + _run("slack_private_channel_member_listing", _context(gateway)) + gateway.fetch_user_info.assert_called_once_with("U_EXT") + + +def test_grid_public_scoping_noop_off_grid() -> None: + """Verifies the Grid-only check is a no-op off Grid.""" + # Precondition. + gateway = _gateway() + + # Under test. + _run("slack_grid_public_channel_scoping", _context(gateway)) + + # Postcondition. + gateway.list_users.assert_not_called() + + +def test_grid_public_scoping_failure_explains_over_sharing() -> None: + """Verifies the over-sharing degradation is named on failure.""" + # Precondition. + gateway = _gateway() + gateway.check_auth.return_value = _auth_response(enterprise_id="E123") + gateway.list_teams.side_effect = _pages(SlackTeamsPage(teams=[{"id": "T1"}])) + gateway.list_users.side_effect = _slack_error("missing_scope", needed="users:read") + + # Under test and postcondition. + with pytest.raises(InsufficientPermissionsError, match="org-wide"): + _run("slack_grid_public_channel_scoping", _context(gateway)) + gateway.list_users.assert_called_once_with(limit=1, team_id="T1") + + +def test_configured_channels_all_visible_passes() -> None: + """Verifies the config check accepts channels present in the listing.""" + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _pages( + SlackChannelsPage(channels=[{"id": "C1", "name": "general"}]) + ) + # The ``#`` prefix must be stripped, mirroring the connector's ``channels`` + # setter. + context = _context(gateway, connector_specific_config={"channels": ["#general"]}) + + # Under test and postcondition. + _run("slack_configured_channels_visible", context) + + +def test_configured_channels_missing_channel_fails() -> None: + """Verifies invisible configured channels are named in the failure.""" + # Precondition. + gateway = _gateway() + gateway.list_channels.side_effect = _pages( + SlackChannelsPage(channels=[{"id": "C1", "name": "general"}]) + ) + context = _context( + gateway, connector_specific_config={"channels": ["general", "not-a-channel"]} + ) + + # Under test and postcondition. + with pytest.raises(ConnectorValidationError, match="not-a-channel"): + _run("slack_configured_channels_visible", context) + + +def test_configured_channels_regex_mode_skips_probe() -> None: + """Verifies regex-mode configs are not existence-checked.""" + # Precondition. + gateway = _gateway() + context = _context( + gateway, + connector_specific_config={ + "channels": ["gen.*"], + "channel_regex_enabled": True, + }, + ) + + # Under test. + _run("slack_configured_channels_visible", context) + + # Postcondition. + gateway.list_channels.assert_not_called() + + +def test_check_metadata_is_pinned() -> None: + """Verifies builder metadata: ids, requiredness, and execution needs.""" + # Precondition. + checks = build_slack_indexing_checks() + build_slack_doc_permission_sync_checks() + + # Under test and postcondition. + check_ids = [check.check_id for check in checks] + assert len(check_ids) == len(set(check_ids)), "Check ids must be unique." + assert all(not check.requires_connector_instance for check in checks), ( + "Slack checks compose the gateway; none needs a connector instance." + ) + config_requiring = { + check.check_id for check in checks if check.requires_connector_config + } + assert config_requiring == {"slack_configured_channels_visible"} + optional = {check.check_id for check in checks if not check.required} + assert optional == { + "slack_private_channel_listing", + "slack_perm_sync_private_channel_listing", + "slack_user_profile_read", + "slack_grid_public_channel_scoping", + # Not required (a fully invited bot works without ``channels:join``), + # but the warning text demands action in most setups. + "slack_channel_join_scope", + } + assert all(check.remediation for check in checks) + assert all(check.docs_link for check in checks) + # The full-workspace enumeration is the one probe with a raised hang guard. + assert _CHECKS_BY_ID["slack_configured_channels_visible"].timeout_seconds == 1800 + + +def test_slack_registers_named_indexing_checks() -> None: + """ + Verifies the registry serves Slack's named INDEXING checks rather than + synthesizing the ``validate_connector_settings`` fallback. + """ + # Under test. + checks = get_capability_checks(DocumentSource.SLACK) + + # Postcondition. + indexing_ids = { + check.check_id + for check in checks + if check.capability == CredentialCapability.INDEXING + } + assert indexing_ids == {check.check_id for check in build_slack_indexing_checks()} + assert not any(check.is_fallback for check in checks) + + +def _coherent_gateway() -> MagicMock: + """An off-Grid gateway with every probe answering like a healthy token.""" + gateway = _gateway() + gateway.check_auth.return_value = _auth_response( + granted_scopes=_ALL_INDEXING_SCOPES + ) + + def list_channels(**kwargs: Any) -> Any: + # The bot is in no private channels; every public-inclusive listing + # sees one channel. + if "public_channel" in kwargs["channel_types"]: + return iter( + [ + SlackChannelsPage( + channels=[{"id": "C1", "name": "general", "is_member": True}] + ) + ] + ) + return iter([SlackChannelsPage(channels=[])]) + + gateway.list_channels.side_effect = list_channels + gateway.fetch_channel_history.side_effect = _pages(SlackHistoryPage(messages=[])) + gateway.list_users.side_effect = _pages( + SlackUsersPage( + members=[ + { + "id": "U1", + "is_bot": False, + "deleted": False, + "profile": {"email": "user@example.com"}, + } + ] + ) + ) + return gateway + + +def test_full_slack_check_run_happy_path() -> None: + """Verifies an end-to-end run over all Slack checks with a healthy token.""" + # Precondition. + checks = build_slack_indexing_checks() + build_slack_doc_permission_sync_checks() + context = _context( + _coherent_gateway(), connector_specific_config={"channels": ["#general"]} + ) + + # Under test. + results = run_capability_checks(checks, context) + + # Postcondition. + status_by_id = {result.check_id: result.status for result in results} + assert all( + status == CapabilityCheckStatus.PASSED for status in status_by_id.values() + ), f"Expected every check to pass, got {status_by_id}." + verdicts = compute_capability_verdicts( + {CredentialCapability.INDEXING, CredentialCapability.DOC_PERMISSION_SYNC}, + results, + ) + assert verdicts == { + CredentialCapability.INDEXING: CapabilityVerdict.PASSED, + CredentialCapability.DOC_PERMISSION_SYNC: CapabilityVerdict.PASSED, + CredentialCapability.EXTERNAL_GROUP_SYNC: CapabilityVerdict.NOT_APPLICABLE, + } + + +def test_configless_run_gates_indexing_on_the_required_config_check() -> None: + """ + Verifies the credential-time verdict semantics: every runnable check passes, + but ``slack_configured_channels_visible`` is required and needs a config, so + its skip keeps INDEXING at SKIPPED (no pass-ish claim on a partially + verified capability) until connector binding re-runs the checks. + """ + # Precondition. + checks = build_slack_indexing_checks() + build_slack_doc_permission_sync_checks() + context = _context(_coherent_gateway()) + + # Under test. + results = run_capability_checks(checks, context) + + # Postcondition. + status_by_id = {result.check_id: result.status for result in results} + assert ( + status_by_id.pop("slack_configured_channels_visible") + == CapabilityCheckStatus.SKIPPED + ) + assert all( + status == CapabilityCheckStatus.PASSED for status in status_by_id.values() + ), f"Expected all non-config checks to pass, got {status_by_id}." + verdicts = compute_capability_verdicts( + {CredentialCapability.INDEXING, CredentialCapability.DOC_PERMISSION_SYNC}, + results, + ) + assert verdicts == { + CredentialCapability.INDEXING: CapabilityVerdict.SKIPPED, + CredentialCapability.DOC_PERMISSION_SYNC: CapabilityVerdict.PASSED, + CredentialCapability.EXTERNAL_GROUP_SYNC: CapabilityVerdict.NOT_APPLICABLE, + } diff --git a/backend/tests/unit/onyx/connectors/slack/test_slack_source_operations.py b/backend/tests/unit/onyx/connectors/slack/test_slack_source_operations.py index 734ab6386d1..54fdcb7c3cd 100644 --- a/backend/tests/unit/onyx/connectors/slack/test_slack_source_operations.py +++ b/backend/tests/unit/onyx/connectors/slack/test_slack_source_operations.py @@ -70,9 +70,15 @@ def test_operation_inventory_is_pinned() -> None: assert set(specs) == _EXPECTED_OPERATIONS # Every operation runs on the bot token alone. assert all(spec.consumes == OperationConsumes.CREDENTIAL for spec in specs.values()) - # Checks land in the Slack capability checks PR; until then everything is - # untested with a reviewable reason. - assert all(spec.untested for spec in specs.values()) + # Only the permanent exemptions remain untested: a side-effecting probe, a + # gracefully-degrading one, and the dormant group-sync pair. Everything else + # is exercised by the checks in ``slack/capability_checks.py``. + assert {name for name, spec in specs.items() if spec.untested} == { + "join_channel", + "fetch_team_info", + "list_usergroups", + "list_usergroup_members", + } def test_permission_class_variants_are_pinned() -> None: @@ -99,10 +105,11 @@ def test_capability_tags_are_pinned() -> None: specs = SlackSourceOperations.operation_specs() # Postcondition. + # No EXTERNAL_GROUP_SYNC on ``fetch_user_info``: its only group-sync caller + # is the dormant, unregistered ``group_sync.py`` path. assert specs["fetch_user_info"].capabilities == { CredentialCapability.INDEXING, CredentialCapability.DOC_PERMISSION_SYNC, - CredentialCapability.EXTERNAL_GROUP_SYNC, } assert specs["list_channel_members"].capabilities == { CredentialCapability.DOC_PERMISSION_SYNC @@ -179,3 +186,38 @@ def test_clients_are_memoized_and_fast_client_is_separate() -> None: assert gateway._fast_client() is fast assert fast is not client assert fast.timeout == 1 + + +def test_check_auth_parses_granted_scopes_from_the_response_header() -> None: + """ + Verifies scope introspection: ``granted_scopes`` comes from the + ``X-OAuth-Scopes`` response header (case-insensitive, list-tolerant), not + the payload. + """ + # Precondition. + gateway = _gateway() + client = MagicMock() + client.auth_test.return_value = MagicMock( + data={"ok": True, "url": "https://onyx.slack.com"}, + headers={"X-OAuth-Scopes": ["channels:read, channels:join"]}, + ) + gateway._cached_client = client + + # Under test. + response = gateway.check_auth() + + # Postcondition. + assert response.ok is True + assert response.granted_scopes == ["channels:read", "channels:join"] + + +def test_check_auth_scopes_are_none_when_the_header_is_absent() -> None: + """Verifies the absent-header case stays distinguishable from no scopes.""" + # Precondition. + gateway = _gateway() + client = MagicMock() + client.auth_test.return_value = MagicMock(data={"ok": True}, headers={}) + gateway._cached_client = client + + # Under test and postcondition. + assert gateway.check_auth().granted_scopes is None diff --git a/backend/tests/unit/onyx/db/test_user_usage.py b/backend/tests/unit/onyx/db/test_user_usage.py index e7bdec0911d..90053cdf4e4 100644 --- a/backend/tests/unit/onyx/db/test_user_usage.py +++ b/backend/tests/unit/onyx/db/test_user_usage.py @@ -177,6 +177,31 @@ def test_null_provider_stored_as_empty_string(self) -> None: compiled = stmt.compile(dialect=postgresql.dialect()) assert compiled.params["provider"] == "" + def test_incognito_dimension_reaches_the_upsert(self) -> None: + """The flag must land in both the inserted row and the conflict target, + or incognito spend would silently merge into regular rows.""" + mock_session = MagicMock() + window = datetime.datetime(2026, 6, 1, tzinfo=datetime.timezone.utc) + + record_user_usage( + mock_session, + user_id=str(uuid4()), + model="model-a", + flow="CHAT", + provider="openai", + input_tokens=10, + output_tokens=5, + cache_read_tokens=0, + cost_cents=0.1, + window_start=window, + incognito=True, + ) + + stmt = mock_session.execute.call_args[0][0] + compiled = stmt.compile(dialect=postgresql.dialect()) + assert compiled.params["incognito"] is True + assert "incognito" in str(compiled).split("ON CONFLICT")[1] + def test_user_deletion_retains_usage_row_with_null_user_id(self) -> None: engine: Engine = create_engine("sqlite://") diff --git a/backend/tests/unit/onyx/llm/test_factory.py b/backend/tests/unit/onyx/llm/test_factory.py index 551f35e7868..81c681a4e49 100644 --- a/backend/tests/unit/onyx/llm/test_factory.py +++ b/backend/tests/unit/onyx/llm/test_factory.py @@ -1,8 +1,25 @@ -from unittest.mock import patch - +from functools import partial +from unittest.mock import MagicMock, patch + +from onyx.chat.incognito import ( + BIFROST_DISABLE_CONTENT_LOGGING_HEADER, + incognito_llm_extra_headers, + incognito_llm_request_policy, +) +from onyx.db.enums import IncognitoRecordMode from onyx.llm.constants import LlmProviderNames -from onyx.llm.factory import _build_provider_extra_headers, get_llm, llm_from_provider -from onyx.llm.well_known_providers.constants import LM_STUDIO_API_KEY_CONFIG_KEY +from onyx.llm.factory import ( + _build_provider_extra_headers, + get_default_llm, + get_llm, + get_llm_for_persona, + llm_from_provider, +) +from onyx.llm.interfaces import LlmRequestPolicy +from onyx.llm.well_known_providers.constants import ( + BIFROST_PROVIDER_NAME, + LM_STUDIO_API_KEY_CONFIG_KEY, +) from onyx.server.manage.llm.models import LLMProviderView, ModelConfigurationView @@ -154,3 +171,178 @@ def test_llm_from_provider_never_sets_ollama_num_ctx_for_non_ollama_provider() - kwargs = mock_get_llm.call_args.kwargs assert kwargs["max_input_tokens"] == 16384 assert kwargs["model_kwargs"] == {} + + +def test_get_llm_policy_headers_win_over_every_other_source() -> None: + """Policy headers must be the final merge. The request and deployment-env + sources set the same header to false here.""" + policy = incognito_llm_extra_headers( + IncognitoRecordMode.USAGE_ONLY, BIFROST_PROVIDER_NAME + ) + header = BIFROST_DISABLE_CONTENT_LOGGING_HEADER + with ( + patch("onyx.llm.factory.LitellmLLM") as mock_litellm_llm, + patch("onyx.utils.headers.LITELLM_EXTRA_HEADERS", {header: "false"}), + ): + get_llm( + provider=BIFROST_PROVIDER_NAME, + model="gpt-4o", + deployment_name=None, + max_input_tokens=4096, + additional_headers={header: "false"}, + policy_headers=policy, + ) + + kwargs = mock_litellm_llm.call_args.kwargs + assert kwargs["extra_headers"][header] == "true" + + +def test_get_llm_without_policy_headers_keeps_the_existing_merge() -> None: + with patch("onyx.llm.factory.LitellmLLM") as mock_litellm_llm: + get_llm( + provider="openai", + model="gpt-4o", + deployment_name=None, + max_input_tokens=4096, + additional_headers={"x-request-scoped": "a"}, + ) + + kwargs = mock_litellm_llm.call_args.kwargs + assert kwargs["extra_headers"] == {"x-request-scoped": "a"} + + +def test_llm_from_provider_resolves_policy_headers_for_the_winning_provider() -> None: + """The caller hands policy as a provider-keyed function because persona + resolution decides the provider inside the factory.""" + provider = _build_provider_view( + provider=BIFROST_PROVIDER_NAME, + max_input_tokens=4096, + ) + + with patch("onyx.llm.factory.get_llm") as mock_get_llm: + llm_from_provider( + model_name="gpt-4o", + llm_provider=provider, + policy_fn=partial( + incognito_llm_request_policy, IncognitoRecordMode.USAGE_ONLY + ), + ) + + kwargs = mock_get_llm.call_args.kwargs + assert kwargs["policy_headers"] == { + BIFROST_DISABLE_CONTENT_LOGGING_HEADER: "true" + } + + +def test_llm_from_provider_without_policy_fn_passes_none() -> None: + provider = _build_provider_view( + provider=BIFROST_PROVIDER_NAME, + max_input_tokens=4096, + ) + + with patch("onyx.llm.factory.get_llm") as mock_get_llm: + llm_from_provider(model_name="gpt-4o", llm_provider=provider) + + assert mock_get_llm.call_args.kwargs["policy_headers"] is None + + +def _sentinel_policy_fn(_provider: str) -> LlmRequestPolicy: + return LlmRequestPolicy() + + +class TestPolicyFnForwarding: + """Every exit of the persona chain must forward the policy function. + + A dropped forward is a silent policy loss on a fallback path, invisible to + the precedence test, which only guards the final merge inside get_llm. + """ + + def test_no_persona_exit_forwards(self) -> None: + with patch("onyx.llm.factory.get_default_llm") as mock_default: + get_llm_for_persona( + persona=None, + user=MagicMock(), + policy_fn=_sentinel_policy_fn, + ) + assert mock_default.call_args.kwargs["policy_fn"] is _sentinel_policy_fn + + def test_unconfigured_persona_exit_forwards(self) -> None: + persona = MagicMock() + persona.default_model_configuration_id = None + with patch("onyx.llm.factory.get_default_llm") as mock_default: + get_llm_for_persona( + persona=persona, + user=MagicMock(), + policy_fn=_sentinel_policy_fn, + ) + assert mock_default.call_args.kwargs["policy_fn"] is _sentinel_policy_fn + + def test_failed_resolution_exit_forwards(self) -> None: + persona = MagicMock() + persona.default_model_configuration_id = 123 + with ( + patch("onyx.llm.factory.get_session_with_current_tenant"), + patch("onyx.llm.factory._resolve_provider_and_model", return_value=None), + patch("onyx.llm.factory.get_default_llm") as mock_default, + ): + get_llm_for_persona( + persona=persona, + user=MagicMock(), + policy_fn=_sentinel_policy_fn, + ) + assert mock_default.call_args.kwargs["policy_fn"] is _sentinel_policy_fn + + def test_access_denied_exit_forwards(self) -> None: + persona = MagicMock() + persona.default_model_configuration_id = 123 + with ( + patch("onyx.llm.factory.get_session_with_current_tenant"), + patch( + "onyx.llm.factory._resolve_provider_and_model", + return_value=(MagicMock(), "some-model"), + ), + patch("onyx.llm.factory.fetch_user_group_ids", return_value=[]), + patch("onyx.llm.factory.can_user_access_llm_provider", return_value=False), + patch("onyx.llm.factory.get_default_llm") as mock_default, + ): + get_llm_for_persona( + persona=persona, + user=MagicMock(), + policy_fn=_sentinel_policy_fn, + ) + assert mock_default.call_args.kwargs["policy_fn"] is _sentinel_policy_fn + + def test_resolved_provider_exit_forwards(self) -> None: + persona = MagicMock() + persona.default_model_configuration_id = 123 + with ( + patch("onyx.llm.factory.get_session_with_current_tenant"), + patch( + "onyx.llm.factory._resolve_provider_and_model", + return_value=(MagicMock(), "some-model"), + ), + patch("onyx.llm.factory.fetch_user_group_ids", return_value=[]), + patch("onyx.llm.factory.can_user_access_llm_provider", return_value=True), + patch("onyx.llm.factory.LLMProviderView"), + patch("onyx.llm.factory.llm_from_provider") as mock_from_provider, + ): + get_llm_for_persona( + persona=persona, + user=MagicMock(), + policy_fn=_sentinel_policy_fn, + ) + assert ( + mock_from_provider.call_args.kwargs["policy_fn"] is _sentinel_policy_fn + ) + + def test_get_default_llm_forwards(self) -> None: + with ( + patch("onyx.llm.factory.get_session_with_current_tenant"), + patch("onyx.llm.factory.fetch_default_llm_model", return_value=MagicMock()), + patch("onyx.llm.factory.LLMProviderView"), + patch("onyx.llm.factory.llm_from_provider") as mock_from_provider, + ): + get_default_llm(policy_fn=_sentinel_policy_fn) + assert ( + mock_from_provider.call_args.kwargs["policy_fn"] is _sentinel_policy_fn + ) diff --git a/backend/tests/unit/onyx/llm/test_multi_llm.py b/backend/tests/unit/onyx/llm/test_multi_llm.py index f0c692ec6f6..63a77ac3f4c 100644 --- a/backend/tests/unit/onyx/llm/test_multi_llm.py +++ b/backend/tests/unit/onyx/llm/test_multi_llm.py @@ -3099,3 +3099,23 @@ def test_invoke_caps_read_timeout_at_total_budget( total_timeout_override=total_timeout_override, ) assert mock_completion.call_args.kwargs["timeout"] == expected_read_timeout + + +def test_policy_extra_body_keeps_deployment_siblings_under_the_same_key() -> None: + """The OpenRouter retention policy sets one key under `provider`. The + deployment's other keys under `provider` must survive that merge.""" + llm = LitellmLLM( + api_key="or-test-key", + timeout=30, + model_provider=LlmProviderNames.OPENROUTER, + model_name="openai/gpt-5.6", + max_input_tokens=128_000, + model_kwargs={"extra_body": {"provider": {"data_collection": "deny"}}}, + extra_body={"provider": {"order": ["Azure"], "allow_fallbacks": False}}, + ) + + assert llm._model_kwargs["extra_body"]["provider"] == { + "order": ["Azure"], + "allow_fallbacks": False, + "data_collection": "deny", + } diff --git a/backend/tests/unit/onyx/server/features/craft/sandbox/test_tarball.py b/backend/tests/unit/onyx/server/features/craft/sandbox/test_tarball.py index f711e983b59..c7770586c22 100644 --- a/backend/tests/unit/onyx/server/features/craft/sandbox/test_tarball.py +++ b/backend/tests/unit/onyx/server/features/craft/sandbox/test_tarball.py @@ -43,6 +43,7 @@ def test_deterministic() -> None: raw1, sha1 = _build_targz(files) raw2, sha2 = _build_targz(files) + assert raw1[4:8] == b"\0\0\0\0" assert raw1 == raw2 assert sha1 == sha2 diff --git a/backend/tests/unit/onyx/server/features/usage/test_admin_usage_export_api.py b/backend/tests/unit/onyx/server/features/usage/test_admin_usage_export_api.py index 59a304eac76..2bf095f761d 100644 --- a/backend/tests/unit/onyx/server/features/usage/test_admin_usage_export_api.py +++ b/backend/tests/unit/onyx/server/features/usage/test_admin_usage_export_api.py @@ -150,6 +150,7 @@ def test_groups_by_email_model_day_with_join(self, db_session: Session) -> None: model="model-a", flow="CHAT", provider="openai", + incognito=False, day="2026-06-01", input_tokens=100, output_tokens=50, @@ -161,6 +162,7 @@ def test_groups_by_email_model_day_with_join(self, db_session: Session) -> None: model="model-b", flow="CHAT", provider="openai", + incognito=False, day="2026-06-01", input_tokens=200, output_tokens=60, @@ -172,6 +174,7 @@ def test_groups_by_email_model_day_with_join(self, db_session: Session) -> None: model="model-a", flow="CHAT", provider="openai", + incognito=False, day="2026-06-08", input_tokens=300, output_tokens=70, @@ -183,6 +186,7 @@ def test_groups_by_email_model_day_with_join(self, db_session: Session) -> None: model="model-a", flow="CHAT", provider="anthropic", + incognito=False, day="2026-06-08", input_tokens=400, output_tokens=80, @@ -247,6 +251,18 @@ def test_date_range_bounds_half_open(self, db_session: Session) -> None: class TestExportEndpoint: + def test_default_range_covers_thirty_calendar_days( + self, db_session: Session + ) -> None: + client = TestClient(_make_app(db_session, _ADMIN)) + + body = client.get("/admin/usage/export").json() + + start = datetime.date.fromisoformat(body["start"]) + end = datetime.date.fromisoformat(body["end"]) + # Inclusive endpoints differ by 29 days when the range has 30 dates. + assert (end - start).days == 29 + def test_nested_per_user_with_totals(self, db_session: Session) -> None: _seed_two_users(db_session) client = TestClient(_make_app(db_session, _ADMIN)) @@ -285,14 +301,27 @@ def test_model_filter_endpoint(self, db_session: Session) -> None: assert all(r["model"] == "model-b" for r in body["users"][0]["records"]) def test_date_range_end_excludes_later_window(self, db_session: Session) -> None: - _seed_two_users(db_session) + alice, _ = _seed_two_users(db_session) + _seed_usage( + db_session, + alice, + "model-a", + "CHAT", + "openai", + 500, + 90, + 0, + 5.0, + datetime.datetime(2026, 6, 7, tzinfo=datetime.timezone.utc), + ) + db_session.commit() client = TestClient(_make_app(db_session, _ADMIN)) # end=2026-06-07 -> half-open through 06-08 00:00, so W2 (06-08) excluded. body = client.get( "/admin/usage/export", params={"start": "2026-06-01", "end": "2026-06-07"} ).json() all_days = {r["day"] for u in body["users"] for r in u["records"]} - assert all_days == {"2026-06-01"} + assert all_days == {"2026-06-01", "2026-06-07"} assert "bob@example.com" not in {u["email"] for u in body["users"]} def test_non_admin_rejected(self, db_session: Session) -> None: diff --git a/web/.dockerignore b/web/.dockerignore index af785f30156..267aed007d3 100644 --- a/web/.dockerignore +++ b/web/.dockerignore @@ -2,6 +2,12 @@ node_modules .next /tests/ +# Co-located unit tests. They import helpers from /tests/, which is excluded above, +# so keeping them would break the `next build` type check inside the image. +# Pre-commit still type checks them through tsconfig.types.json. +src/**/*.test.ts +src/**/*.test.tsx + # Local build artifacts — opal/shared rebuild these via `prepare` on `bun install`. # Excluding them keeps a stale local dist out of hand-built images (CI clones clean, # so a stale dist only exists when building the image from a dirty working tree). diff --git a/web/lib/opal/src/components/buttons/line-item-button/README.md b/web/lib/opal/src/components/buttons/line-item-button/README.md index 435fbfb6ff0..d722778884c 100644 --- a/web/lib/opal/src/components/buttons/line-item-button/README.md +++ b/web/lib/opal/src/components/buttons/line-item-button/README.md @@ -9,7 +9,7 @@ A composite component that wraps `Interactive.Stateful > Interactive.Container > ``` Interactive.Stateful <- selectVariant, state, interaction, onClick, href, ref └─ Interactive.Container <- width, rounding - └─ ContentAction <- withInteractive, padding="lg" + └─ ContentAction <- withInteractive, padding={2} ├─ Content <- icon, title, description, sizePreset, variant, ... └─ rightChildren ``` @@ -18,7 +18,7 @@ The row renders as a focusable `
` (with Enter/Space activatio native `
diff --git a/web/lib/opal/src/components/buttons/sidebar-tab/README.md b/web/lib/opal/src/components/buttons/sidebar-tab/README.md index ae96983f7a5..8d3981b57fd 100644 --- a/web/lib/opal/src/components/buttons/sidebar-tab/README.md +++ b/web/lib/opal/src/components/buttons/sidebar-tab/README.md @@ -7,18 +7,29 @@ A sidebar navigation tab built on `Interactive.Stateful` > `Interactive.Containe ## Architecture ``` -div.relative +div.opal-sidebar-tab <- folded styling hook (see styles.css) └─ Interactive.Stateful <- variant (sidebar-heavy | sidebar-light), state, disabled └─ Interactive.Container <- rounding, height, width - ├─ Link? (absolute overlay for client-side navigation) - ├─ rightChildren? (absolute, above Link for inline actions) + ├─ Link | button? (absolute overlay — the click target) + ├─ rightChildren? (absolute, above the overlay for inline actions) └─ ContentAction (icon + title + truncation spacer) ``` - **`sidebar-heavy`** (default) — muted when unselected (text-03/text-02), bold when selected (text-04/text-03) - **`sidebar-light`** — uniformly muted across all states (text-02/text-02) - **Disabled** — both variants use text-02 foreground, transparent background, no hover/active states -- **Navigation** uses an absolutely positioned `` overlay rather than `href` on the Interactive element, so `rightChildren` can sit above it with `pointer-events-auto`. +- **The click target** is an absolutely positioned overlay: a `` when `href` is set, a `} * /> * diff --git a/web/lib/opal/src/layouts/content-action/ContentAction.stories.tsx b/web/lib/opal/src/layouts/content-action/ContentAction.stories.tsx index 85c811c73d0..691b8ff3f0f 100644 --- a/web/lib/opal/src/layouts/content-action/ContentAction.stories.tsx +++ b/web/lib/opal/src/layouts/content-action/ContentAction.stories.tsx @@ -55,7 +55,7 @@ export const NoPadding: Story = { variant: "section", title: "Compact Row", description: "No padding around content area.", - padding: "fit", + padding: 0, rightChildren: , }, }; diff --git a/web/lib/opal/src/layouts/content-action/README.md b/web/lib/opal/src/layouts/content-action/README.md index 10fb524d9ba..df54176e81d 100644 --- a/web/lib/opal/src/layouts/content-action/README.md +++ b/web/lib/opal/src/layouts/content-action/README.md @@ -15,21 +15,19 @@ Inherits **all** props from [`Content`](../content/README.md) (same discriminate | Prop | Type | Default | Description | |---|---|---|---| | `rightChildren` | `ReactNode` | `undefined` | Content rendered on the right side. Wrapper stretches to the full height of the row. | -| `padding` | `SizeVariant` | `"lg"` | Padding preset applied around the `Content` area. Uses the shared size scale from `@opal/shared`. | +| `padding` | `0 \| 0.5 \| 1 \| 2` | `2` | Padding around the `Content` area, as a spacing step (`N / 4` rem). Narrowed to the paddings `Interactive.Container` uses. | | `fillRight` | `boolean` | `false` | When `true`, the `rightChildren` column grows to fill the row (capped at `--block-width-form-input-column-max`, 240px) instead of hugging its content. Use for full-width form inputs; leave off for compact controls like toggles/buttons. Ignored in the `responsive` branch. | ### `padding` reference -| Value | Padding class | Effective padding | -|---|---|---| -| `lg` | `p-2` | 0.5rem (8px) | -| `md` | `p-1` | 0.25rem (4px) | -| `sm` | `p-1` | 0.25rem (4px) | -| `xs` | `p-0.5` | 0.125rem (2px) | -| `2xs` | `p-0.5` | 0.125rem (2px) | -| `fit` | `p-0` | 0 | +`padding` is a spacing step: `N` is `N / 4` rem. It is narrowed to `0 | 0.5 | 1 | 2`, +and the default `2` is 0.5rem (8px). -These values are identical to the padding applied by `Interactive.Container` at each size, so `ContentAction` labels naturally align with adjacent buttons of the same size. +`Interactive.Container` still derives its padding from its `size` preset, and matching +those paddings is the point of this prop — it is what makes a `ContentAction` label line +up with an adjacent button of the same size. The equivalents are `lg` → `2`, +`md` and `sm` → `1`, `xs` and `2xs` → `0.5`, `fit` → `0`. Note this is a *different* +scale from `Card`, where `lg` was 24px rather than 8px. ## Layout Structure @@ -61,7 +59,7 @@ import SvgSettings from "@opal/icons/settings"; sizePreset="main-content" variant="section" tag={{ title: "Default", color: "blue" }} - padding="lg" + padding={2} rightChildren={ - {/* Below md the reading-width cap never applies (chat is - always full width), so the toggle has nothing to do. */} - + {incognitoAvailable && !incognitoEnabled && ( + + + } + onOpenChange={(state) => { + setPopoverOpen(state); + if (!state) { + setShowMoveOptions(false); + setShowExportOptions(false); + setSearchTerm(""); + } + }} + side="bottom" + align="end" + > + {popoverItems} + + + )} diff --git a/web/src/lib/projects/svc.ts b/web/src/lib/projects/svc.ts index af2b0e53e95..43e679cc3fc 100644 --- a/web/src/lib/projects/svc.ts +++ b/web/src/lib/projects/svc.ts @@ -34,13 +34,18 @@ export async function createProject(name: string): Promise { export async function uploadFiles( files: File[], projectId?: number | null, - tempIdMap?: Map + tempIdMap?: Map, + incognitoSessionId?: string | null ): Promise { const formData = new FormData(); files.forEach((file) => formData.append("files", file)); if (projectId !== undefined && projectId !== null) { formData.append("project_id", String(projectId)); } + // Names the session before it exists, so the server owns the privacy call. + if (incognitoSessionId) { + formData.append("incognito_session_id", incognitoSessionId); + } if (tempIdMap !== undefined && tempIdMap !== null) { formData.append( "temp_id_map", diff --git a/web/src/lib/usage/hooks.ts b/web/src/lib/usage/hooks.ts index e294a36682e..ba6acda533f 100644 --- a/web/src/lib/usage/hooks.ts +++ b/web/src/lib/usage/hooks.ts @@ -7,7 +7,6 @@ import { buildApiPath } from "@/lib/urlBuilder"; import { convertDateToEndOfDay, convertDateToStartOfDay, - getXDaysAgo, } from "@/lib/dateUtils"; import { OnyxBotAnalytics, @@ -17,14 +16,14 @@ import { UserAnalytics, } from "@/lib/usage/interfaces"; import { - DateRangePickerValue, THIRTY_DAYS, + type DateRangePickerValue, + rangeForInclusiveDays, } from "@/refresh-components/DateRangePicker"; export function useTimeRange() { return useState({ - to: new Date(), - from: getXDaysAgo(30), + ...rangeForInclusiveDays(30), selectValue: THIRTY_DAYS, }); } diff --git a/web/src/providers/AppProvider.tsx b/web/src/providers/AppProvider.tsx index f61e27f0821..48df6eae4e0 100644 --- a/web/src/providers/AppProvider.tsx +++ b/web/src/providers/AppProvider.tsx @@ -35,6 +35,7 @@ import { AppBackgroundProvider } from "@/providers/AppBackgroundProvider"; import { QueryControllerProvider } from "@/providers/QueryControllerProvider"; import { NEXT_PUBLIC_INCLUDE_ERROR_POPUP_SUPPORT_LINK } from "@/lib/constants"; import { FullWidthChatProvider } from "@/providers/FullWidthChatProvider"; +import { IncognitoProvider } from "@/providers/IncognitoProvider"; import { UnsavedChangesNavigationProvider } from "@/providers/UnsavedChangesNavigationProvider"; interface SidebarPersistenceProviderProps { @@ -82,17 +83,19 @@ export default function AppProvider({ children }: AppProviderProps) { - - - {children} - - + + + + {children} + + + diff --git a/web/src/providers/IncognitoProvider.tsx b/web/src/providers/IncognitoProvider.tsx new file mode 100644 index 00000000000..ead894c1534 --- /dev/null +++ b/web/src/providers/IncognitoProvider.tsx @@ -0,0 +1,129 @@ +"use client"; + +import { + createContext, + useCallback, + useContext, + useEffect, + useMemo, + useRef, + useState, +} from "react"; +import { usePathname } from "next/navigation"; +import useSWR from "swr"; +import { errorHandlingFetcher } from "@/lib/fetcher"; +import { SWR_KEYS } from "@/lib/swr-keys"; + +interface IncognitoAvailabilityResponse { + available: boolean; +} + +// Shared incognito state so the toggle, submit path, and warning stay in +// sync. Locks once the chat has a message, since the mode pins at creation. +// Availability only gates the toggle. The server enforces it on creation. +interface IncognitoContextValue { + incognitoAvailable: boolean; + incognitoEnabled: boolean; + // Always-current mirror of incognitoEnabled. Send paths must read this at + // submit time: a stale closure over the boolean submits a persisted chat + // while the UI shows incognito. + incognitoEnabledRef: React.RefObject; + incognitoLocked: boolean; + // Minted when incognito is switched on and sent with every upload, so a file + // names its session before the first message creates it. + incognitoSessionId: string | null; + toggleIncognito: () => void; + setIncognitoSessionId: (sessionId: string | null) => void; + setIncognitoEnabled: (enabled: boolean) => void; + setIncognitoLocked: (locked: boolean) => void; +} + +const IncognitoContext = createContext(null); + +interface IncognitoProviderProps { + children: React.ReactNode; +} + +export function IncognitoProvider({ children }: IncognitoProviderProps) { + const [incognitoEnabled, setIncognitoEnabled] = useState(false); + const [incognitoLocked, setIncognitoLocked] = useState(false); + + const { data: availability, mutate: revalidateAvailability } = + useSWR( + SWR_KEYS.incognitoAvailability, + errorHandlingFetcher, + { + // Hiding the toggle is the safe fallback, but a persistent failure + // otherwise looks identical to the admin turning incognito off. + onError: (error) => + console.error("Failed to load incognito availability:", error), + } + ); + const incognitoAvailable = availability?.available ?? false; + + // The provider mounts once for the whole app, so route changes never + // remount the hook. Revalidating per navigation picks up admin changes to + // the availability setting or group flags without a hard refresh. + const pathname = usePathname(); + useEffect(() => { + void revalidateAvailability(); + }, [pathname, revalidateAvailability]); + + const [incognitoSessionId, setIncognitoSessionId] = useState( + null + ); + const toggleIncognito = useCallback(() => { + if (incognitoLocked) return; + const next = !incognitoEnabled; + setIncognitoSessionId(next ? crypto.randomUUID() : null); + setIncognitoEnabled(next); + }, [incognitoLocked, incognitoEnabled]); + + const incognitoEnabledRef = useRef(false); + useEffect(() => { + incognitoEnabledRef.current = incognitoEnabled; + }, [incognitoEnabled]); + + // Memoized so consumers keying callbacks on the context object do not + // rebuild them on every provider render. + const value = useMemo( + () => ({ + incognitoAvailable, + incognitoEnabled, + incognitoEnabledRef, + incognitoLocked, + incognitoSessionId, + toggleIncognito, + setIncognitoEnabled, + setIncognitoSessionId, + setIncognitoLocked, + }), + [ + incognitoAvailable, + incognitoEnabled, + incognitoLocked, + incognitoSessionId, + toggleIncognito, + ] + ); + + return ( + + {children} + + ); +} + +export function useIncognito(): IncognitoContextValue { + const ctx = useContext(IncognitoContext); + if (!ctx) { + throw new Error("useIncognito must be used within an IncognitoProvider"); + } + return ctx; +} + +// For components that also render outside the provider (e.g. shared chats), +// where incognito can never be active. +export function useIncognitoOptional(): IncognitoContextValue | null { + return useContext(IncognitoContext); +} diff --git a/web/src/providers/ProjectsContext.tsx b/web/src/providers/ProjectsContext.tsx index e94c1331c78..097cac2be3e 100644 --- a/web/src/providers/ProjectsContext.tsx +++ b/web/src/providers/ProjectsContext.tsx @@ -46,6 +46,7 @@ import { ChatFileType } from "@/app/app/interfaces"; import { toast } from "@opal/layouts"; import { useProjects } from "@/lib/projects/hooks"; import { useSettings } from "@/lib/settings/hooks"; +import { useIncognitoOptional } from "@/providers/IncognitoProvider"; export type { Project, ProjectFile } from "@/lib/projects/types"; @@ -134,6 +135,14 @@ interface ProjectsProviderProps { export function ProjectsProvider({ children }: ProjectsProviderProps) { // Use SWR hook for projects list - no more SSR initial data const { projects, refreshProjects } = useProjects(); + // Uploads made in an incognito chat are tied to its session so the server + // deletes them at teardown. Optional: falls back to disabled when no + // IncognitoProvider is mounted above. + const incognitoCtx = useIncognitoOptional(); + const incognitoUploadsEnabled = incognitoCtx?.incognitoEnabled ?? false; + const incognitoSessionId = incognitoUploadsEnabled + ? (incognitoCtx?.incognitoSessionId ?? null) + : null; const [recentFiles, setRecentFiles] = useState([]); const [currentProjectDetails, setCurrentProjectDetails] = useState(null); @@ -379,12 +388,14 @@ export function ProjectsProvider({ children }: ProjectsProviderProps) { createOptimisticFile(f, projectId) ); const tempIdMap = getTempIdMap(validFiles, optimisticFiles); - setAllRecentFiles((prev) => [...optimisticFiles, ...prev]); - if (projectId) { + if (!incognitoUploadsEnabled) { + setAllRecentFiles((prev) => [...optimisticFiles, ...prev]); + } + if (projectId && !incognitoUploadsEnabled) { setAllCurrentProjectFiles((prev) => [...optimisticFiles, ...prev]); projectToUploadFilesMapRef.current.set(projectId, optimisticFiles); } - svcUploadFiles(validFiles, projectId, tempIdMap) + svcUploadFiles(validFiles, projectId, tempIdMap, incognitoSessionId) .then((uploaded) => { const uploadedFiles = uploaded.user_files || []; const tempIdToUploadedFileMap = new Map( @@ -485,6 +496,8 @@ export function ProjectsProvider({ children }: ProjectsProviderProps) { refreshRecentFiles, removeOptimisticFilesByTempIds, userFileMaxUploadSizeMb, + incognitoUploadsEnabled, + incognitoUploadsEnabled, ] ); @@ -493,7 +506,12 @@ export function ProjectsProvider({ children }: ProjectsProviderProps) { files: File[], projectId?: number | null ): Promise => { - const uploaded: CategorizedFiles = await svcUploadFiles(files, projectId); + const uploaded: CategorizedFiles = await svcUploadFiles( + files, + projectId, + undefined, + incognitoSessionId + ); const uploadedFiles = uploaded.user_files || []; // Track these uploaded file IDs for targeted polling if (uploadedFiles.length > 0) { @@ -511,7 +529,13 @@ export function ProjectsProvider({ children }: ProjectsProviderProps) { await refreshRecentFiles(); return uploaded; }, - [currentProjectId, refreshCurrentProjectDetails, refreshRecentFiles] + [ + currentProjectId, + refreshCurrentProjectDetails, + refreshRecentFiles, + incognitoUploadsEnabled, + incognitoUploadsEnabled, + ] ); const getFilesInProject = useCallback( @@ -575,7 +599,21 @@ export function ProjectsProvider({ children }: ProjectsProviderProps) { isPollingRef.current = true; try { const statuses = await svcGetUserFileStatuses(ids); - if (!statuses || statuses.length === 0) return; + if (!statuses) return; + if (statuses.length === 0) { + // The server reports none of the REQUESTED files: they were deleted + // (e.g. incognito teardown). Drop only those, not ids registered + // while this request was in flight. + const requested = new Set(ids); + setTrackedUploadIds((prev) => { + const remaining = new Set(); + prev.forEach((id) => { + if (!requested.has(id)) remaining.add(id); + }); + return remaining; + }); + return; + } // Build maps for quick lookup const statusById = new Map(statuses.map((f) => [f.id, f])); @@ -684,6 +722,13 @@ export function ProjectsProvider({ children }: ProjectsProviderProps) { newlyFailed.push(f); } } + // Requested ids the server no longer reports are deleted files: stop + // tracking them. Ids registered after this request went out stay. + for (const id of ids) { + if (!statusById.has(id)) { + remaining.delete(id); + } + } if (newlyFailed.length > 0) { setLastFailedFiles(newlyFailed); } diff --git a/web/src/refresh-components/DateRangePicker.test.tsx b/web/src/refresh-components/DateRangePicker.test.tsx index e3a8fa63edd..5b3e2ed65f3 100644 --- a/web/src/refresh-components/DateRangePicker.test.tsx +++ b/web/src/refresh-components/DateRangePicker.test.tsx @@ -19,7 +19,7 @@ describe("DateRangePicker", () => { render( { }); }); + it("emits exactly 30 calendar days for 1M", async () => { + const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); + const onValueChange = jest.fn(); + + render( + + ); + + await user.click(screen.getByRole("button", { name: "1M" })); + + expect(onValueChange).toHaveBeenCalledWith({ + from: new Date(2026, 6, 6), + to: new Date(2026, 7, 4, 23, 59, 59, 999), + }); + }); + + it("emits exactly 7 calendar days for 7D", async () => { + const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); + const onValueChange = jest.fn(); + + render( + + ); + + await user.click(screen.getByRole("button", { name: "7D" })); + + expect(onValueChange).toHaveBeenCalledWith({ + from: new Date(2026, 6, 29), + to: new Date(2026, 7, 4, 23, 59, 59, 999), + }); + }); + + it("emits exactly 90 calendar days for 3M", async () => { + const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); + const onValueChange = jest.fn(); + + render( + + ); + + await user.click(screen.getByRole("button", { name: "3M" })); + + expect(onValueChange).toHaveBeenCalledWith({ + from: new Date(2026, 4, 7), + to: new Date(2026, 7, 4, 23, 59, 59, 999), + }); + }); + it("shows the committed custom range when reopened", async () => { const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); diff --git a/web/src/refresh-components/DateRangePicker.tsx b/web/src/refresh-components/DateRangePicker.tsx index 79a13b820cd..581c93c9287 100644 --- a/web/src/refresh-components/DateRangePicker.tsx +++ b/web/src/refresh-components/DateRangePicker.tsx @@ -3,7 +3,7 @@ import { endOfDay, format, isSameDay, startOfDay, subDays } from "date-fns"; import { Calendar, Popover, SelectButton } from "@opal/components"; import { SvgCalendar } from "@opal/icons"; -export const THIRTY_DAYS = "30d"; +export const THIRTY_DAYS = "1M"; export type DateRangePickerValue = DateRange & { selectValue: string; @@ -25,19 +25,25 @@ type DraftDateRange = interface DatePreset { label: string; - daysAgo: number; + inclusiveDays: number; } const PRESETS: DatePreset[] = [ - { label: "1D", daysAgo: 0 }, - { label: "7D", daysAgo: 6 }, - { label: "1M", daysAgo: 30 }, - { label: "3M", daysAgo: 90 }, + { label: "1D", inclusiveDays: 1 }, + { label: "7D", inclusiveDays: 7 }, + { label: "1M", inclusiveDays: 30 }, + { label: "3M", inclusiveDays: 90 }, ]; -function rangeForPreset(preset: DatePreset): Exclude { +export function rangeForInclusiveDays( + inclusiveDays: number +): Exclude { const to = endOfDay(new Date()); - return { from: startOfDay(subDays(to, preset.daysAgo)), to }; + return { from: startOfDay(subDays(to, inclusiveDays - 1)), to }; +} + +function rangeForPreset(preset: DatePreset): Exclude { + return rangeForInclusiveDays(preset.inclusiveDays); } function rangesMatch(left: DateRange, right: DateRange): boolean { diff --git a/web/src/refresh-components/inputs/InputSelect.tsx b/web/src/refresh-components/inputs/InputSelect.tsx index c3c0d694612..f8cc9359c09 100644 --- a/web/src/refresh-components/inputs/InputSelect.tsx +++ b/web/src/refresh-components/inputs/InputSelect.tsx @@ -14,8 +14,8 @@ import { } from "@/refresh-components/inputs/styles"; import Truncated from "@/refresh-components/texts/Truncated"; import { SvgChevronDownSmall } from "@opal/icons"; -import { Divider } from "@opal/components"; -import type { Spacing, WithoutStyles } from "@opal/types"; +import { Divider, type DividerSpacing } from "@opal/components"; +import type { WithoutStyles } from "@opal/types"; // ============================================================================ // Context @@ -439,8 +439,8 @@ function InputSelectLabel({ } interface InputSelectSeparatorProps { - paddingParallel?: Spacing; - paddingPerpendicular?: Spacing; + paddingParallel?: DividerSpacing; + paddingPerpendicular?: DividerSpacing; } function InputSelectSeparator({ diff --git a/web/src/sections/admin/ProviderCard.tsx b/web/src/sections/admin/ProviderCard.tsx index 7bb9672a787..c5ceb8620e0 100644 --- a/web/src/sections/admin/ProviderCard.tsx +++ b/web/src/sections/admin/ProviderCard.tsx @@ -119,7 +119,7 @@ export default function ProviderCard({ icon={icon} title={title} description={description} - padding="lg" + padding={2} rightChildren={ isDisconnected && onConnect ? (