ci: reduce pr-artifacts to 8core smoke test

External callers were passing sk-oh- session tokens as ?api_key= query
parameters, which get logged by Traefik and application access logs,
exposing sensitive tokens in Datadog.

This change:
- Adds deprecated fallback in get_api_key_from_header() to accept
  api_key from URL query parameters (lowest priority after all headers)
- Logs a warning when query param auth is used, directing callers to
  migrate to Authorization: Bearer <token> header
- Updates middleware _check_tos to recognize query param api_key so
  requests aren't rejected before the auth flow handles them
- Updates all test mocks to include query_params = {} to prevent
  MagicMock auto-creation from causing false positives

Fixes: OpenHands/evaluation#391

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

.github/workflows/pr-artifacts.yml

@@ -1,136 +1,30 @@
----
 name: PR Artifacts
 
+run-name: PR Artifacts Smoke Test
+
 on:
-    workflow_dispatch: # Manual trigger for testing
-    pull_request:
-        types: [opened, synchronize, reopened]
-        branches: [main]
-    pull_request_review:
-        types: [submitted]
+  workflow_dispatch:
 
 jobs:
-  # Auto-remove .pr/ directory when a reviewer approves
-    cleanup-on-approval:
-        concurrency:
-            group: cleanup-pr-artifacts-${{ github.event.pull_request.number }}
-            cancel-in-progress: false
-        if: github.event_name == 'pull_request_review' && github.event.review.state == 'approved'
-        runs-on: ubuntu-latest
-        permissions:
-            contents: write
-            pull-requests: write
-        steps:
-            - name: Check if fork PR
-              id: check-fork
-              run: |
-                  if [ "${{ github.event.pull_request.head.repo.full_name }}" != "${{ github.event.pull_request.base.repo.full_name }}" ]; then
-                    echo "is_fork=true" >> $GITHUB_OUTPUT
-                    echo "::notice::Fork PR detected - skipping auto-cleanup (manual removal required)"
-                  else
-                    echo "is_fork=false" >> $GITHUB_OUTPUT
-                  fi
+  smoke-test:
+    timeout-minutes: 30
+    runs-on: ubuntu-latest-8core
 
-            - uses: actions/checkout@v5
-              if: steps.check-fork.outputs.is_fork == 'false'
-              with:
-                  ref: ${{ github.event.pull_request.head.ref }}
-                  token: ${{ secrets.ALLHANDS_BOT_GITHUB_PAT }}
+    steps:
+      - name: Show runner details
+        run: |
+          set -euxo pipefail
+          echo "runner_name=${RUNNER_NAME}"
+          echo "runner_os=${RUNNER_OS}"
+          echo "runner_arch=${RUNNER_ARCH}"
+          uname -a
+          nproc
+          free -h || true
+          df -h
 
-            - name: Remove .pr/ directory
-              id: remove
-              if: steps.check-fork.outputs.is_fork == 'false'
-              run: |
-                  if [ -d ".pr" ]; then
-                    git config user.name "allhands-bot"
-                    git config user.email "allhands-bot@users.noreply.github.com"
-                    git rm -rf .pr/
-                    git commit -m "chore: Remove PR-only artifacts [automated]"
-                    git push || {
-                      echo "::error::Failed to push cleanup commit. Check branch protection rules."
-                      exit 1
-                    }
-                    echo "removed=true" >> $GITHUB_OUTPUT
-                    echo "::notice::Removed .pr/ directory"
-                  else
-                    echo "removed=false" >> $GITHUB_OUTPUT
-                    echo "::notice::No .pr/ directory to remove"
-                  fi
-
-            - name: Update PR comment after cleanup
-              if: steps.check-fork.outputs.is_fork == 'false' && steps.remove.outputs.removed == 'true'
-              uses: actions/github-script@v7
-              with:
-                  script: |
-                      const marker = '<!-- pr-artifacts-notice -->';
-                      const body = `${marker}
-                      ✅ **PR Artifacts Cleaned Up**
-
-                      The \`.pr/\` directory has been automatically removed.
-                      `;
-
-                      const { data: comments } = await github.rest.issues.listComments({
-                        owner: context.repo.owner,
-                        repo: context.repo.repo,
-                        issue_number: context.issue.number,
-                      });
-
-                      const existing = comments.find(c => c.body.includes(marker));
-                      if (existing) {
-                        await github.rest.issues.updateComment({
-                          owner: context.repo.owner,
-                          repo: context.repo.repo,
-                          comment_id: existing.id,
-                          body: body,
-                        });
-                      }
-
-  # Warn if .pr/ directory exists (will be auto-removed on approval)
-    check-pr-artifacts:
-        if: github.event_name == 'pull_request'
-        runs-on: ubuntu-latest
-        permissions:
-            contents: read
-            pull-requests: write
-        steps:
-            - uses: actions/checkout@v5
-
-            - name: Check for .pr/ directory
-              id: check
-              run: |
-                  if [ -d ".pr" ]; then
-                    echo "exists=true" >> $GITHUB_OUTPUT
-                    echo "::warning::.pr/ directory exists and will be automatically removed when the PR is approved. For fork PRs, manual removal is required before merging."
-                  else
-                    echo "exists=false" >> $GITHUB_OUTPUT
-                  fi
-
-            - name: Post or update PR comment
-              if: steps.check.outputs.exists == 'true'
-              uses: actions/github-script@v7
-              with:
-                  script: |
-                      const marker = '<!-- pr-artifacts-notice -->';
-                      const body = `${marker}
-                      📁 **PR Artifacts Notice**
-
-                      This PR contains a \`.pr/\` directory with PR-specific documents. This directory will be **automatically removed** when the PR is approved.
-
-                      > For fork PRs: Manual removal is required before merging.
-                      `;
-
-                      const { data: comments } = await github.rest.issues.listComments({
-                        owner: context.repo.owner,
-                        repo: context.repo.repo,
-                        issue_number: context.issue.number,
-                      });
-
-                      const existing = comments.find(c => c.body.includes(marker));
-                      if (!existing) {
-                        await github.rest.issues.createComment({
-                          owner: context.repo.owner,
-                          repo: context.repo.repo,
-                          issue_number: context.issue.number,
-                          body: body,
-                        });
-                      }
+      - name: Simple shell check
+        run: |
+          set -euxo pipefail
+          echo "smoke test start"
+          sleep 10
+          echo "smoke test done"

enterprise/server/auth/saas_user_auth.py

@@ -287,7 +287,28 @@ def get_api_key_from_header(request: Request):
         return session_api_key
 
     # Fallback to X-Access-Token header as an additional option
-    return request.headers.get('X-Access-Token')
+    access_token = request.headers.get('X-Access-Token')
+    if access_token:
+        return access_token
+
+    # DEPRECATED: Accept api_key from query parameters for backward compatibility.
+    # Passing API keys as query parameters is a security risk because they are
+    # logged in proxy access logs (e.g. Traefik) and application logs.
+    # Callers should migrate to using the Authorization header instead:
+    #   Authorization: Bearer sk-oh-...
+    # See: https://github.com/OpenHands/evaluation/issues/391
+    query_api_key = request.query_params.get('api_key')
+    if query_api_key:
+        logger.warning(
+            'DEPRECATED: api_key passed as URL query parameter. '
+            'This is a security risk as tokens are logged in proxy/access logs. '
+            'Use the Authorization header instead: Authorization: Bearer <token>. '
+            'Query parameter support will be removed in a future release.',
+            extra={'path': request.url.path},
+        )
+        return query_api_key
+
+    return None
 
 
 async def saas_user_auth_from_bearer(request: Request) -> SaasUserAuth | None:

enterprise/server/middleware.py

@@ -106,12 +106,17 @@ class SetAuthCookieMiddleware:
         auth_header = request.headers.get('Authorization')
         mcp_auth_header = request.headers.get('X-Session-API-Key')
         api_auth_header = request.headers.get('X-Access-Token')
+        # DEPRECATED: Also check for api_key in query params for backward
+        # compatibility. The actual deprecation warning is logged in
+        # get_api_key_from_header() when the key is extracted.
+        query_api_key = request.query_params.get('api_key')
         accepted_tos: bool | None = False
         if (
             keycloak_auth_cookie is None
             and (auth_header is None or not auth_header.startswith('Bearer '))
             and mcp_auth_header is None
             and api_auth_header is None
+            and query_api_key is None
         ):
             raise NoCredentialsError
 

enterprise/tests/unit/test_auth_middleware.py

@@ -25,6 +25,7 @@ def middleware():
 def mock_request():
     request = MagicMock(spec=Request)
     request.cookies = {}
+    request.query_params = {}
     return request
 
 
@@ -356,6 +357,7 @@ async def test_middleware_does_not_skip_similar_non_webhook_paths(
         mock_request.url.path = path
         mock_request.headers = MagicMock()
         mock_request.headers.get = MagicMock(side_effect=lambda k: None)
+        mock_request.query_params = {}
 
         # Since these paths start with /api, _should_attach returns True
         # Since there's no auth, middleware catches NoCredentialsError and returns 401

enterprise/tests/unit/test_saas_user_auth.py

@@ -31,6 +31,7 @@ def mock_request():
     request = MagicMock(spec=Request)
     request.headers = {}
     request.cookies = {}
+    request.query_params = {}
     return request
 
 
@@ -511,6 +512,7 @@ async def test_saas_user_auth_from_bearer_no_auth_header():
     """Test that saas_user_auth_from_bearer returns None if no auth header."""
     mock_request = MagicMock()
     mock_request.headers = {}
+    mock_request.query_params = {}
 
     result = await saas_user_auth_from_bearer(mock_request)
 
@@ -633,6 +635,7 @@ def test_get_api_key_from_header_with_authorization_header():
     # Create a mock request with Authorization header
     mock_request = MagicMock(spec=Request)
     mock_request.headers = {'Authorization': 'Bearer test_api_key'}
+    mock_request.query_params = {}
 
     # Call the function
     api_key = get_api_key_from_header(mock_request)
@@ -646,6 +649,7 @@ def test_get_api_key_from_header_with_x_session_api_key():
     # Create a mock request with X-Session-API-Key header
     mock_request = MagicMock(spec=Request)
     mock_request.headers = {'X-Session-API-Key': 'session_api_key'}
+    mock_request.query_params = {}
 
     # Call the function
     api_key = get_api_key_from_header(mock_request)
@@ -662,6 +666,7 @@ def test_get_api_key_from_header_with_both_headers():
         'Authorization': 'Bearer auth_api_key',
         'X-Session-API-Key': 'session_api_key',
     }
+    mock_request.query_params = {}
 
     # Call the function
     api_key = get_api_key_from_header(mock_request)
@@ -671,10 +676,11 @@ def test_get_api_key_from_header_with_both_headers():
 
 
 def test_get_api_key_from_header_with_no_headers():
-    """Test that get_api_key_from_header returns None when no relevant headers are present."""
+    """Test that get_api_key_from_header returns None when no relevant headers or query params are present."""
     # Create a mock request with no relevant headers
     mock_request = MagicMock(spec=Request)
     mock_request.headers = {'Other-Header': 'some_value'}
+    mock_request.query_params = {}
 
     # Call the function
     api_key = get_api_key_from_header(mock_request)
@@ -688,6 +694,7 @@ def test_get_api_key_from_header_with_invalid_authorization_format():
     # Create a mock request with incorrectly formatted Authorization header
     mock_request = MagicMock(spec=Request)
     mock_request.headers = {'Authorization': 'InvalidFormat api_key'}
+    mock_request.query_params = {}
 
     # Call the function
     api_key = get_api_key_from_header(mock_request)
@@ -701,6 +708,7 @@ def test_get_api_key_from_header_with_x_access_token():
     # Create a mock request with X-Access-Token header
     mock_request = MagicMock(spec=Request)
     mock_request.headers = {'X-Access-Token': 'access_token_key'}
+    mock_request.query_params = {}
 
     # Call the function
     api_key = get_api_key_from_header(mock_request)
@@ -717,6 +725,7 @@ def test_get_api_key_from_header_priority_authorization_over_x_access_token():
         'Authorization': 'Bearer auth_api_key',
         'X-Access-Token': 'access_token_key',
     }
+    mock_request.query_params = {}
 
     # Call the function
     api_key = get_api_key_from_header(mock_request)
@@ -733,6 +742,7 @@ def test_get_api_key_from_header_priority_x_session_over_x_access_token():
         'X-Session-API-Key': 'session_api_key',
         'X-Access-Token': 'access_token_key',
     }
+    mock_request.query_params = {}
 
     # Call the function
     api_key = get_api_key_from_header(mock_request)
@@ -750,6 +760,7 @@ def test_get_api_key_from_header_all_three_headers():
         'X-Session-API-Key': 'session_api_key',
         'X-Access-Token': 'access_token_key',
     }
+    mock_request.query_params = {}
 
     # Call the function
     api_key = get_api_key_from_header(mock_request)
@@ -766,6 +777,7 @@ def test_get_api_key_from_header_invalid_authorization_fallback_to_x_access_toke
         'Authorization': 'InvalidFormat api_key',
         'X-Access-Token': 'access_token_key',
     }
+    mock_request.query_params = {}
 
     # Call the function
     api_key = get_api_key_from_header(mock_request)
@@ -783,6 +795,7 @@ def test_get_api_key_from_header_empty_headers():
         'X-Session-API-Key': '',
         'X-Access-Token': 'access_token_key',
     }
+    mock_request.query_params = {}
 
     # Call the function
     api_key = get_api_key_from_header(mock_request)
@@ -799,6 +812,7 @@ def test_get_api_key_from_header_bearer_with_empty_token():
         'Authorization': 'Bearer ',
         'X-Access-Token': 'access_token_key',
     }
+    mock_request.query_params = {}
 
     # Call the function
     api_key = get_api_key_from_header(mock_request)
@@ -808,6 +822,68 @@ def test_get_api_key_from_header_bearer_with_empty_token():
     assert api_key == ''
 
 
+def test_get_api_key_from_query_param_fallback():
+    """Test that get_api_key_from_header falls back to api_key query parameter (deprecated)."""
+    mock_request = MagicMock(spec=Request)
+    mock_request.headers = {}
+    mock_request.query_params = {'api_key': 'sk-oh-query-param-key'}
+    mock_request.url = MagicMock()
+    mock_request.url.path = '/mcp'
+
+    api_key = get_api_key_from_header(mock_request)
+
+    assert api_key == 'sk-oh-query-param-key'
+
+
+def test_get_api_key_from_header_takes_priority_over_query_param():
+    """Test that Authorization header takes priority over api_key query parameter."""
+    mock_request = MagicMock(spec=Request)
+    mock_request.headers = {'Authorization': 'Bearer header_api_key'}
+    mock_request.query_params = {'api_key': 'sk-oh-query-param-key'}
+
+    api_key = get_api_key_from_header(mock_request)
+
+    assert api_key == 'header_api_key'
+
+
+def test_get_api_key_x_session_header_takes_priority_over_query_param():
+    """Test that X-Session-API-Key header takes priority over api_key query parameter."""
+    mock_request = MagicMock(spec=Request)
+    mock_request.headers = {'X-Session-API-Key': 'session_key'}
+    mock_request.query_params = {'api_key': 'sk-oh-query-param-key'}
+
+    api_key = get_api_key_from_header(mock_request)
+
+    assert api_key == 'session_key'
+
+
+def test_get_api_key_x_access_token_takes_priority_over_query_param():
+    """Test that X-Access-Token header takes priority over api_key query parameter."""
+    mock_request = MagicMock(spec=Request)
+    mock_request.headers = {'X-Access-Token': 'access_token_key'}
+    mock_request.query_params = {'api_key': 'sk-oh-query-param-key'}
+
+    api_key = get_api_key_from_header(mock_request)
+
+    assert api_key == 'access_token_key'
+
+
+def test_get_api_key_from_query_param_logs_deprecation_warning(caplog):
+    """Test that using api_key query parameter logs a deprecation warning."""
+    import logging
+
+    mock_request = MagicMock(spec=Request)
+    mock_request.headers = {}
+    mock_request.query_params = {'api_key': 'sk-oh-query-param-key'}
+    mock_request.url = MagicMock()
+    mock_request.url.path = '/api/v1/auth/github'
+
+    with caplog.at_level(logging.WARNING):
+        api_key = get_api_key_from_header(mock_request)
+
+    assert api_key == 'sk-oh-query-param-key'
+
+
 @pytest.mark.asyncio
 async def test_saas_user_auth_from_signed_token_blocked_domain(mock_config):
     """Test that saas_user_auth_from_signed_token raises AuthError when email domain is blocked."""