feat(package-vulnerability-scanner): scan as the signed-in viewer

.github/workflows/package-vulnerability-scanner.yml

@@ -36,6 +36,14 @@ jobs:
       - run: npm ci
       - run: npm run build
 
+      # Run the Python backend tests.
+      - uses: astral-sh/setup-uv@v9.0.0
+        with:
+          pyproject-file: ./extensions/${{ env.EXTENSION_NAME }}/pyproject.toml
+
+      - name: Run backend tests
+        run: uv run pytest
+
       # Now that the extension is built we need to upload an artifact to pass
       # to the package-extension action that contains the files we want to be
       # included in the extension

extensions/package-vulnerability-scanner/.prettierignore

@@ -5,3 +5,6 @@ coverage
 
 # Ignore virtual environments:
 .venv
+
+# Ignore test caches:
+.pytest_cache

extensions/package-vulnerability-scanner/main.py

@@ -1,15 +1,116 @@
 import asyncio
 import json
+import os
 
 import httpx
-from fastapi import FastAPI, HTTPException
+import requests
+from fastapi import FastAPI, HTTPException, Request
 from fastapi.staticfiles import StaticFiles
 from posit import connect
+from posit.connect.errors import ClientError
 from pydantic import BaseModel
 
 app = FastAPI()
 
-client = connect.Client()
+# How long a single Connect HTTP request may stall. This is requests' read
+# timeout, i.e. the gap between bytes, so a slow but progressing response is not
+# cut off; only a genuinely stuck one is.
+CONNECT_REQUEST_TIMEOUT_SECONDS = 60
+
+
+# The SDK ships its session with no timeout of its own, so a Connect server that
+# accepts a connection and then goes quiet would hang the calling thread forever.
+# The endpoints run these calls through asyncio.to_thread, which can stop waiting
+# on a stuck call but cannot interrupt it, so without a deadline here those
+# threads accumulate and eventually starve the pool. Supplying the default
+# through an adapter uses requests' own extension point rather than reaching into
+# the SDK. CONNECT_API_TIMEOUT_SECONDS is the separate, shorter bound on how long
+# a caller waits; this one is what eventually frees the thread.
+class _TimeoutAdapter(requests.adapters.HTTPAdapter):
+    def send(self, request, **kwargs):
+        if kwargs.get("timeout") is None:
+            kwargs["timeout"] = CONNECT_REQUEST_TIMEOUT_SECONDS
+        return super().send(request, **kwargs)
+
+
+def _with_request_timeout(c: connect.Client) -> connect.Client:
+    for prefix in ("http://", "https://"):
+        c.session.mount(prefix, _TimeoutAdapter())
+    return c
+
+
+client = _with_request_timeout(connect.Client())
+
+
+# Connect sets one of these to "CONNECT" for deployed content. Check both because
+# a missed detection here would fall back to the owner's client for an anonymous
+# viewer (see get_visitor_client), so err toward "on Connect".
+def _running_on_connect() -> bool:
+    return "CONNECT" in (os.getenv("POSIT_PRODUCT"), os.getenv("RSTUDIO_PRODUCT"))
+
+
+# Build a Connect client scoped to the signed-in viewer by exchanging their
+# per-request session token, so API calls return that viewer's own content and
+# identity.
+def get_visitor_client(request: Request) -> connect.Client:
+    token = request.headers.get("posit-connect-user-session-token")
+    if token:
+        # The exchange builds a fresh client with its own session, so the
+        # timeout has to be applied to that one too.
+        return _with_request_timeout(client.with_user_session_token(token))
+    # On Connect, no token means the viewer's session can't be read (they aren't
+    # signed in, or OAuth integrations are disabled on the server). Falling back to
+    # the owner's default client would scan the owner's content as if it were the
+    # viewer's, so require the visitor session instead. Off Connect (local
+    # development), the default client is the intended one.
+    if _running_on_connect():
+        raise HTTPException(
+            status_code=424,
+            detail="Couldn't read your Connect session, so the scan can't run as "
+            "you. Make sure you're signed in to Connect. If you are, your "
+            "administrator may need to enable OAuth integrations on this server.",
+        )
+    return client
+
+
+# Connect raises ClientError code 212 when the Visitor API Key integration that
+# viewer-scoped calls depend on has not been added to this content. Surface it as
+# a distinct 424 so the UI can show setup instructions.
+def _setup_required(exc: ClientError) -> HTTPException:
+    if exc.error_code == 212:
+        return HTTPException(
+            status_code=424,
+            detail="In the content settings, on the Access tab, add a Connect "
+            "Visitor API Key integration under Integrations, to scan your content.",
+        )
+    # A 4xx here (e.g. a 403 permission denial, or a 404 for a bad guid) is a
+    # legitimate client error, not an upstream failure; pass its real status
+    # through instead of folding it into a 502. A 5xx reaching this point has
+    # already exhausted _fetch_with_retry's retries, so 502 is the right call.
+    if exc.http_status is not None and 400 <= exc.http_status < 500:
+        return HTTPException(
+            status_code=exc.http_status,
+            detail=f"Connect API error: {exc.error_message}",
+        )
+    return HTTPException(
+        status_code=502, detail=f"Connect API error: {exc.error_message}"
+    )
+
+
+# Log the underlying error for the publisher's server logs, but show the viewer a
+# short, curated message rather than a raw SDK/httpx string.
+def _upstream_error(message: str, exc: Exception) -> HTTPException:
+    print(f"{message} {exc}")
+    # Connect returning a 4xx with no JSON body (rare, but the SDK's error hook
+    # falls back to a bare requests.HTTPError in that case) is still a
+    # legitimate client error, not an upstream failure; pass its real status
+    # through instead of folding it into a 502, same as _setup_required does
+    # for a ClientError.
+    status = getattr(getattr(exc, "response", None), "status_code", None)
+    if status is not None and 400 <= status < 500:
+        return HTTPException(status_code=status, detail=message)
+    return HTTPException(status_code=502, detail=message)
+
 
 # The public Package Manager is always current. To scan against your own
 # instance instead, point this at "https://your-ppm/__api__/filter/packages".
@@ -19,25 +120,93 @@
 # deployment doesn't produce an unwieldy payload.
 PPM_QUERY_CHUNK = 100
 
+# How many times to try a Connect API call before giving up, so a transient 5xx
+# (e.g. a 504 while Connect is under load) doesn't fail the whole scan.
+CONTENT_MAX_ATTEMPTS = 3
+
+# Stop waiting on a single Connect API call. A timeout is treated the same as a
+# transient 5xx: retried up to CONTENT_MAX_ATTEMPTS before giving up.
+#
+# Deliberately shorter than CONNECT_REQUEST_TIMEOUT_SECONDS, because the two
+# bound different things. This one caps how long a caller waits, so a slow call
+# is retried promptly; asyncio.to_thread can't interrupt the call it abandons, so
+# that thread lives on until the request-level deadline frees it.
+CONNECT_API_TIMEOUT_SECONDS = 30
+
+
+# A 5xx from Connect or its gateway (e.g. a 504 while it is under scan load), or a
+# call that timed out outright, is worth retrying; a 4xx or a missing item is not.
+# A ClientError carries http_status; a lower-level requests error carries a
+# response with status_code.
+def _is_transient(exc: Exception) -> bool:
+    # Either kind of timeout: we stopped waiting on the call, or the request
+    # itself hit the deadline the session adapter sets.
+    if isinstance(exc, (asyncio.TimeoutError, requests.exceptions.Timeout)):
+        return True
+    status = getattr(exc, "http_status", None) or getattr(
+        getattr(exc, "response", None), "status_code", None
+    )
+    return status is not None and status >= 500
+
+
+# Run a synchronous SDK call off the event loop, retrying a transient 5xx (e.g. a
+# 504 while Connect is under load) or an outright timeout with a short backoff
+# before giving up.
+async def _fetch_with_retry(fn):
+    for attempt in range(CONTENT_MAX_ATTEMPTS):
+        try:
+            return await asyncio.wait_for(
+                asyncio.to_thread(fn), CONNECT_API_TIMEOUT_SECONDS
+            )
+        except Exception as e:
+            if _is_transient(e) and attempt < CONTENT_MAX_ATTEMPTS - 1:
+                await asyncio.sleep(0.5 * (attempt + 1))
+                continue
+            raise
+
 
 @app.get("/api/content")
-async def search_content(show_all: bool = False):
-    if show_all:
-        return client.content.find()
-    return client.me.content.find()
+async def search_content(request: Request, show_all: bool = False):
+    try:
+        # A session-token exchange makes a real Connect API call, so it has to
+        # run off the event loop like every other blocking SDK call here.
+        visitor = await _fetch_with_retry(lambda: get_visitor_client(request))
+        if show_all:
+            return await _fetch_with_retry(lambda: visitor.content.find())
+        return await _fetch_with_retry(lambda: visitor.me.content.find())
+    except ClientError as e:
+        raise _setup_required(e)
+    except HTTPException:
+        raise  # e.g. the 424 from get_visitor_client; don't re-wrap as a 502
+    except Exception as e:
+        raise _upstream_error("Couldn't load your content from Connect.", e)
 
 
 @app.get("/api/packages/{guid}")
-async def get_packages(guid: str):
+async def get_packages(guid: str, request: Request):
     try:
-        content = client.content.get(guid)
-        packages = list(content.packages)
-        return packages
-    except Exception as e:
+        # A session-token exchange makes a real Connect API call, so it has to
+        # run off the event loop like every other blocking SDK call here.
+        visitor = await _fetch_with_retry(lambda: get_visitor_client(request))
+        # Iterating .packages makes its own blocking call, so it has to run
+        # inside the same retried thread as the content fetch, not after it.
+        return await _fetch_with_retry(
+            lambda: list(visitor.content.get(guid).packages)
+        )
+    except ClientError as e:
+        if e.error_code == 212:
+            raise _setup_required(e)
+        # Preserve the existing "not found" shape for other Connect errors (e.g.
+        # an invalid guid), rather than folding every ClientError into the
+        # missing-integration case.
         raise HTTPException(
             status_code=404,
-            detail=f"Content not found or error fetching packages: {str(e)}",
+            detail=f"Content not found or error fetching packages: {e.error_message}",
         )
+    except HTTPException:
+        raise  # e.g. the 424 from get_visitor_client; don't re-wrap as a 502
+    except Exception as e:
+        raise _upstream_error("Couldn't fetch packages from Connect.", e)
 
 
 # The installed packages to scan, as "name==version" specifiers grouped by the
@@ -89,8 +258,18 @@ async def fetch_repo_vulns(repo, specifiers):
 
 
 @app.get("/api/user")
-async def get_current_user():
-    return client.me
+async def get_current_user(request: Request):
+    try:
+        # A session-token exchange makes a real Connect API call, so it has to
+        # run off the event loop like every other blocking SDK call here.
+        visitor = await _fetch_with_retry(lambda: get_visitor_client(request))
+        return await _fetch_with_retry(lambda: visitor.me)
+    except ClientError as e:
+        raise _setup_required(e)
+    except HTTPException:
+        raise  # e.g. the 424 from get_visitor_client; don't re-wrap as a 502
+    except Exception as e:
+        raise _upstream_error("Couldn't load your account from Connect.", e)
 
 
 app.mount("/", StaticFiles(directory="dist", html=True), name="static")

extensions/package-vulnerability-scanner/manifest.json

@@ -30,7 +30,7 @@
       "checksum": "07435c1b16a3ab78d62c07501cc2e32d"
     },
     "main.py": {
-      "checksum": "b4d98ffdae1449709c3bc679f49262d6"
+      "checksum": "331f2474111a6280734062de89152bb6"
     },
     "requirements.txt": {
       "checksum": "9fc5cc5fb559eded1f323793b73e82c5"
@@ -43,7 +43,7 @@
     "homepage": "https://github.com/posit-dev/connect-extensions/tree/main/extensions/package-vulnerability-scanner",
     "category": "extension",
     "minimumConnectVersion": "2025.04.0",
-    "requiredFeatures": ["API Publishing"],
+    "requiredFeatures": ["API Publishing", "OAuth Integrations"],
     "version": "3.0.6"
   }
 }

extensions/package-vulnerability-scanner/pyproject.toml

@@ -9,4 +9,11 @@ dependencies = [
     "starlette>=0.47.2",
     "httpx>=0.28.1",
     "posit-sdk>=0.10.0",
+    "requests>=2.31.0",
+]
+
+# Test-only dependencies; not bundled into the extension (requirements.txt is).
+[dependency-groups]
+dev = [
+    "pytest>=8",
 ]

extensions/package-vulnerability-scanner/requirements.txt

@@ -2,3 +2,4 @@ httpx
 fastapi
 starlette>=0.47.2
 posit-sdk
+requests>=2.31.0