Skip to content

feat: implement strict multi-surface access control - #25

Merged
alain-sv merged 9 commits into
developfrom
claude/multi-surface-access-control-kdTqn
Apr 20, 2026
Merged

feat: implement strict multi-surface access control#25
alain-sv merged 9 commits into
developfrom
claude/multi-surface-access-control-kdTqn

Conversation

@alain-sv

Copy link
Copy Markdown
Contributor

Collapses ad-hoc auth into three router-level surfaces:

public / No auth — home, A2A discovery, docs
private /manage Tailscale-only (100.64.0.0/10) — admin UI, workbench
api /api API key + hierarchical scopes (read/write) — M2M

Key changes:

  • src/supervaizer/access/ — new package: client_ip.py (TRUSTED_PROXIES /
    XFF), tailscale.py (require_tailscale dep), api_auth.py (API_KEYS,
    require_api_key, require_scope); SUPERVAIZER_API_KEY preloaded as write
  • src/supervaizer/routers/ — public_router, api_router, private_router
    factories; router-level deps replace 14+ scattered Security() calls
  • admin/routes.py — removed verify_admin_access, console tokens, APIKeyHeader;
    Tailscale is the sole gate
  • routes.py / data_routes.py — removed per-route Security(); write-mutating
    endpoints get Depends(require_scope("write"))
  • server.py — replaced scattered include_router block + AdminIPAllowlistMiddleware
    with the three router factories
  • templates — global /admin → /manage; removed ?key= URL params and
    console-token JS; WebSocket workbench inherits Tailscale dep via private_router
  • tests — updated paths (/api/supervaizer/..., /manage/...); new
    test_access_client_ip.py, test_access_tailscale.py, test_access_api_auth.py;
    deleted test_admin_ip_allowlist.py (replaced)

457 tests pass; pre-existing boto3/docker failures unaffected.

https://claude.ai/code/session_011Ansn4kxHVP8nLmLWESz36

Collapses ad-hoc auth into three router-level surfaces:

  public  /             No auth — home, A2A discovery, docs
  private /manage       Tailscale-only (100.64.0.0/10) — admin UI, workbench
  api     /api          API key + hierarchical scopes (read/write) — M2M

Key changes:
- src/supervaizer/access/ — new package: client_ip.py (TRUSTED_PROXIES /
  XFF), tailscale.py (require_tailscale dep), api_auth.py (API_KEYS,
  require_api_key, require_scope); SUPERVAIZER_API_KEY preloaded as write
- src/supervaizer/routers/ — public_router, api_router, private_router
  factories; router-level deps replace 14+ scattered Security() calls
- admin/routes.py — removed verify_admin_access, console tokens, APIKeyHeader;
  Tailscale is the sole gate
- routes.py / data_routes.py — removed per-route Security(); write-mutating
  endpoints get Depends(require_scope("write"))
- server.py — replaced scattered include_router block + AdminIPAllowlistMiddleware
  with the three router factories
- templates — global /admin → /manage; removed ?key= URL params and
  console-token JS; WebSocket workbench inherits Tailscale dep via private_router
- tests — updated paths (/api/supervaizer/..., /manage/...); new
  test_access_client_ip.py, test_access_tailscale.py, test_access_api_auth.py;
  deleted test_admin_ip_allowlist.py (replaced)

457 tests pass; pre-existing boto3/docker failures unaffected.

https://claude.ai/code/session_011Ansn4kxHVP8nLmLWESz36
@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Implement strict multi-surface access control with Tailscale and API key scopes

✨ Enhancement 🧪 Tests 🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Implements strict multi-surface access control with three router-level authentication surfaces:
  public (no auth), private/manage (Tailscale-only), and api (API key + hierarchical scopes)
• Creates new supervaizer.access package with client IP extraction, Tailscale CGNAT gating
  (100.64.0.0/10), and API key authentication with read/write scope hierarchy
• Creates new supervaizer.routers package with three router factories (create_public_router,
  create_private_router, create_api_router) that consolidate scattered authentication logic
• Removes ad-hoc per-route authentication: eliminates console tokens, verify_admin_access(),
  scattered Security() calls, and AdminIPAllowlistMiddleware
• Replaces API key checks with scope-based enforcement: read-only endpoints require no auth on API
  surface, write-mutating endpoints require require_scope("write")
• Preloads SUPERVAIZER_API_KEY environment variable as write-scope entry for server-to-server
  communication
• Migrates all routes from /admin to /manage prefix for private surface; API routes use
  /api/supervaizer/... prefix
• Removes API key and console token URL parameters from templates; relies on Tailscale for admin UI
  access control
• Adds comprehensive test coverage for new access control modules: client IP extraction with proxy
  trust, Tailscale IP range validation, and API key/scope enforcement
• Updates all existing tests to reflect new URL prefixes and authentication model (457 tests
  passing)
Diagram
flowchart LR
  A["Request"] --> B{"Route Surface"}
  B -->|"/ (public)"| C["create_public_router"]
  B -->|"/api (M2M)"| D["create_api_router"]
  B -->|"/manage (admin)"| E["create_private_router"]
  C --> F["No Auth<br/>Home, A2A Discovery"]
  D --> G["require_api_key<br/>+ require_scope"]
  E --> H["require_tailscale<br/>100.64.0.0/10"]
  G --> I["Read: No scope<br/>Write: write scope"]
  H --> J["Admin UI<br/>Workbench<br/>Console"]
Loading

Grey Divider

File Changes

1. tests/test_admin_routes.py 🧪 Tests +44/-326

Migrate admin route tests to /manage with Tailscale gating

• Removed imports and tests for console token functions (generate_console_token,
 validate_console_token, verify_admin_access)
• Updated test client fixture to mount admin routes at /manage prefix instead of /admin
• Removed ?key= query parameters from all test requests; routes now rely on Tailscale gating at
 router level
• Simplified test expectations by removing API key header mocking and auth-related assertions

tests/test_admin_routes.py


2. src/supervaizer/admin/routes.py Refactoring +17/-149

Remove console tokens and per-route auth; rely on Tailscale

• Removed APIKeyHeader, Security, Depends imports and console token storage (_console_tokens
 dict)
• Deleted verify_admin_access() function and console token management functions
 (generate_console_token, validate_console_token, cleanup_expired_tokens)
• Removed token generation and validation from console page and log-stream endpoints
• Removed verify_admin_access dependency from workbench routes; both HTTP and WebSocket now
 inherit Tailscale protection from private_router

src/supervaizer/admin/routes.py


3. src/supervaizer/routes.py ✨ Enhancement +15/-15

Replace per-route API key checks with scope-based write enforcement

• Added import of require_scope from supervaizer.access
• Removed Security(server.verify_api_key) dependencies from read-only endpoints
• Replaced Security(server.verify_api_key) with Depends(require_scope("write")) on all
 write-mutating endpoints (update, validate, start, stop, etc.)

src/supervaizer/routes.py


View more (34)
4. tests/test_server.py 🧪 Tests +15/-14

Update test URLs and auth assertions for /api surface

• Updated all API endpoint URLs from /supervaizer/... to /api/supervaizer/... to match new
 api_router prefix
• Changed expected error message from "Not authenticated" to "API key" for 401 responses
• Added API key headers to previously unguarded utility endpoints
 (/api/supervaizer/utils/public_key, /api/supervaizer/utils/encrypt)

tests/test_server.py


5. src/supervaizer/server.py Refactoring +24/-68

Consolidate routing into three surfaces; remove middleware

• Removed imports: Path, AdminIPAllowlistMiddleware, create_admin_routes, Jinja2Templates,
 HTMLResponse
• Added imports: create_api_router, create_private_router, create_public_router from
 supervaizer.routers
• Moved app.state.server = self assignment earlier in startup sequence
• Replaced scattered include_router() calls with three router factories; removed
 AdminIPAllowlistMiddleware
• Moved inline home page handler to routers/public.py

src/supervaizer/server.py


6. src/supervaizer/data_routes.py ✨ Enhancement +10/-9

Enforce scope-based write access on data resource routes

• Added import of require_scope from supervaizer.access
• Removed Security import; added Depends import
• Removed server parameter from _add_resource_routes() function signature
• Removed Security(server.verify_api_key) from read-only operations (GET, list)
• Replaced Security(server.verify_api_key) with Depends(require_scope("write")) on write
 operations (POST, PUT, DELETE, import)

src/supervaizer/data_routes.py


7. tests/test_routes_case_update.py 🧪 Tests +10/-10

Update case update test URLs for /api prefix

• Updated all case update endpoint URLs from /supervaizer/... to /api/supervaizer/...
• Changed expected unauthorized error message from "Not authenticated" to "API key"

tests/test_routes_case_update.py


8. tests/test_access_tailscale.py 🧪 Tests +122/-0

Add comprehensive Tailscale access control tests

• New test file for require_tailscale dependency
• Tests IP range validation against Tailscale CGNAT (100.64.0.0/10)
• Covers allowed IPs, denied IPs, boundary cases, and logging behavior
• Uses mocking to control _extract_client_ip return values in test environment

tests/test_access_tailscale.py


9. tests/test_access_api_auth.py 🧪 Tests +124/-0

Add API key and scope enforcement tests

• New test file for require_api_key and require_scope dependencies
• Tests API key validation, missing/unknown keys, and hierarchical scope enforcement
• Verifies that SUPERVAIZER_API_KEY env var is preloaded with write scope
• Tests scope hierarchy: write satisfies read, but read does not satisfy write

tests/test_access_api_auth.py


10. tests/test_access_client_ip.py 🧪 Tests +81/-0

Add client IP extraction and proxy trust tests

• New test file for _extract_client_ip() and TRUSTED_PROXIES handling
• Tests direct peer IP extraction when no trusted proxies configured
• Tests X-Forwarded-For header parsing when peer is in trusted CIDR range
• Covers malformed IPs, missing headers, and fallback behavior

tests/test_access_client_ip.py


11. src/supervaizer/access/api_auth.py ✨ Enhancement +85/-0

Implement API key auth and hierarchical scope model

• New module implementing API key authentication and hierarchical scope enforcement
• Defines API_KEYS registry with test keys (key_123 read, key_456 write)
• Implements require_api_key() dependency that checks header and falls back to live server key
• Implements require_scope() factory for scope-based access control with hierarchy
• Preloads SUPERVAIZER_API_KEY env var as write-scope entry

src/supervaizer/access/api_auth.py


12. src/supervaizer/access/client_ip.py ✨ Enhancement +65/-0

Add client IP extraction with proxy trust support

• New module for client IP extraction with trusted-proxy support
• Parses TRUSTED_PROXIES env var (comma-separated CIDRs) at import time
• Implements _extract_client_ip() that uses X-Forwarded-For only when peer is in trusted range
• Returns empty string on parse failures (fail-closed behavior)

src/supervaizer/access/client_ip.py


13. src/supervaizer/access/tailscale.py ✨ Enhancement +49/-0

Implement Tailscale CGNAT access control dependency

• New module implementing Tailscale CGNAT range gating (100.64.0.0/10)
• Implements require_tailscale() dependency that denies non-Tailscale IPs with HTTP 403
• Handles both HTTP and WebSocket connections with appropriate error responses
• Uses _extract_client_ip() for IP resolution with proxy support

src/supervaizer/access/tailscale.py


14. src/supervaizer/routers/public.py ✨ Enhancement +63/-0

Create public router for home and A2A discovery

• New module defining public router (no authentication required)
• Implements create_public_router() factory that includes home page and A2A discovery routes
• Moved home page handler from server.py to this module
• Conditionally includes A2A routes based on server.a2a_endpoints flag

src/supervaizer/routers/public.py


15. src/supervaizer/routers/api.py ✨ Enhancement +57/-0

Create API router with router-level key enforcement

• New module defining API router (machine-to-machine surface with API key required)
• Implements create_api_router() factory with router-level require_api_key dependency
• Includes supervision routes, agent routes, data resource routes, and custom agent routes
• All sub-routes inherit API key requirement; scope-specific routes add require_scope() themselves

src/supervaizer/routers/api.py


16. src/supervaizer/routers/private.py ✨ Enhancement +35/-0

Create private router with Tailscale-only gating

• New module defining private router (Tailscale-only admin and workbench surface)
• Implements create_private_router() factory with router-level require_tailscale dependency
• Includes admin routes and WebSocket workbench routes; both inherit Tailscale protection
• Mounted at /manage prefix in server.py

src/supervaizer/routers/private.py


17. src/supervaizer/access/__init__.py ✨ Enhancement +20/-0

Create access control package exports

• New package __init__.py exporting access control modules
• Exports require_api_key, require_scope, require_tailscale, API_KEYS, TRUSTED_PROXIES,
 _extract_client_ip
• Provides single import point for all access control dependencies

src/supervaizer/access/init.py


18. src/supervaizer/routers/__init__.py ✨ Enhancement +13/-0

Create routers package exports

• New package __init__.py exporting router factories
• Exports create_api_router, create_private_router, create_public_router
• Provides single import point for all three surface routers

src/supervaizer/routers/init.py


19. src/supervaizer/common.py ✨ Enhancement +15/-0

Add structured access denial logging functions

• Added log_access_denied_tailscale() function for structured Tailscale denial logging
• Added log_access_denied_api() function for structured API key denial logging with key truncation
• Both functions log with structured fields (ip/key, path, reason) for audit trails

src/supervaizer/common.py


20. tests/test_workbench_routes.py 🧪 Tests +3/-3

Update workbench tests for /manage prefix

• Updated admin routes prefix from /admin to /manage in test fixture
• Updated workbench test URLs from /admin/agents/... to /manage/agents/...
• Removed ?key= query parameters from test requests

tests/test_workbench_routes.py


21. tests/test_validation_endpoints.py 🧪 Tests +2/-0

Wire mock server for API key fallback testing

• Added server.api_key = "test-api-key" to mock server fixture
• Added app.state.server = mock_server to test app for require_api_key live-server fallback

tests/test_validation_endpoints.py


22. src/supervaizer/admin/templates/navigation.html ⚙️ Configuration changes +18/-18

Update navigation template URLs to /manage

• Updated all navigation links from /admin to /manage prefix
• Updated active route detection to check /manage paths instead of /admin
• Updated both desktop and mobile menu navigation links

src/supervaizer/admin/templates/navigation.html


23. src/supervaizer/admin/templates/workbench.html ⚙️ Configuration changes +7/-7

Update workbench template URLs to /manage

• Updated all AJAX/HTMX URLs from /admin/agents/... to /manage/agents/...
• Updated static asset URL from /admin/static/js/workbench-form.js to
 /manage/static/js/workbench-form.js
• Updated JavaScript endpoint references for start, stop, and monitor operations

src/supervaizer/admin/templates/workbench.html


24. src/supervaizer/admin/templates/case_detail.html ⚙️ Configuration changes +4/-4

Update case detail template URLs to /manage

• Updated navigation links from /admin/agents to /manage/agents
• Updated navigation links from /admin/jobs to /manage/jobs

src/supervaizer/admin/templates/case_detail.html


25. src/supervaizer/admin/templates/agents.html ⚙️ Configuration changes +6/-6

Update agents template URLs to /manage

• Updated HTMX GET URLs from /admin/api/agents to /manage/api/agents
• Updated JavaScript AJAX calls from /admin/api/agents to /manage/api/agents

src/supervaizer/admin/templates/agents.html


26. src/supervaizer/admin/templates/cases_list.html ⚙️ Configuration changes +6/-6

Update cases list template URLs to /manage

• Updated HTMX GET URLs from /admin/api/cases to /manage/api/cases
• Updated JavaScript AJAX calls from /admin/api/cases to /manage/api/cases

src/supervaizer/admin/templates/cases_list.html


27. src/supervaizer/admin/templates/jobs_list.html ⚙️ Configuration changes +6/-6

Update jobs list template URLs to /manage

• Updated HTMX GET URLs from /admin/api/jobs to /manage/api/jobs
• Updated JavaScript AJAX calls from /admin/api/jobs to /manage/api/jobs

src/supervaizer/admin/templates/jobs_list.html


28. src/supervaizer/admin/templates/server.html ⚙️ Configuration changes +3/-3

Update server template URLs to /manage

• Updated HTMX GET URLs from /admin/api/server/status to /manage/api/server/status
• Updated fetch call from /admin/api/server/register to /manage/api/server/register
• Updated DOM selector to reference /manage/api/server/status

src/supervaizer/admin/templates/server.html


29. src/supervaizer/admin/templates/cases_table.html ⚙️ Configuration changes +3/-3

Update cases table template URLs to /manage

• Updated HTMX DELETE URLs from /admin/api/cases/... to /manage/api/cases/...
• Updated HTMX GET pagination URLs from /admin/api/cases to /manage/api/cases

src/supervaizer/admin/templates/cases_table.html


30. src/supervaizer/admin/templates/jobs_table.html ⚙️ Configuration changes +3/-3

Update jobs table template URLs to /manage

• Updated HTMX DELETE URLs from /admin/api/jobs/... to /manage/api/jobs/...
• Updated HTMX GET pagination URLs from /admin/api/jobs to /manage/api/jobs

src/supervaizer/admin/templates/jobs_table.html


31. src/supervaizer/admin/templates/job_detail.html ⚙️ Configuration changes +3/-3

Update job detail template URLs to /manage

• Updated HTMX POST URLs from /admin/api/jobs/... to /manage/api/jobs/... for status updates

src/supervaizer/admin/templates/job_detail.html


32. src/supervaizer/admin/templates/dashboard.html ⚙️ Configuration changes +3/-3

Update dashboard template URLs to /manage

• Updated navigation links from /admin/jobs to /manage/jobs
• Updated navigation links from /admin/cases to /manage/cases
• Updated HTMX GET URL from /admin/api/recent-activity to /manage/api/recent-activity

src/supervaizer/admin/templates/dashboard.html


33. src/supervaizer/admin/templates/base.html Security +2/-9

Remove API key URL persistence, update admin routes

• Removed JavaScript code that persisted admin_api_key from URL query parameters to session
 storage
• Updated log-stream endpoint path from /admin/log-stream to /manage/log-stream
• Added clarifying comment that Tailscale is now the access control gate instead of URL parameters

src/supervaizer/admin/templates/base.html


34. src/supervaizer/admin/templates/console.html Security +3/-17

Remove console token authentication, rely on Tailscale

• Removed console_token template variable and validation logic from log-stream connection
• Removed token parameter from EventSource URL (/admin/log-stream?token=.../manage/log-stream)
• Removed token validation and error handling for console command execution endpoint
• Updated command execution endpoint from /admin/api/console/execute?token=... to
 /manage/api/console/execute

src/supervaizer/admin/templates/console.html


35. src/supervaizer/admin/templates/agents_grid.html Security +1/-1

Remove API key URL parameter from workbench links

• Removed ?key= URL parameter from workbench link
• Updated workbench route from /admin/agents/... to /manage/agents/...
• Removed conditional logic that appended API key to the URL

src/supervaizer/admin/templates/agents_grid.html


36. src/supervaizer/admin/templates/index.html ⚙️ Configuration changes +1/-1

Update admin route to manage route

• Updated admin link destination from /admin to /manage

src/supervaizer/admin/templates/index.html


37. tests/test_admin_ip_allowlist.py Additional files +0/-139

...

tests/test_admin_ip_allowlist.py


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Apr 18, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Untyped self in pytest tests📘 Rule violation ▣ Testability
Description
New/added test methods define self without a type annotation, violating the requirement that all
parameters (including self) be explicitly typed in changed Python files. This can break strict
type-checking expectations and reduces consistency across the codebase.
Code

tests/test_access_api_auth.py[38]

+    def test_missing_header_returns_401(self) -> None:
Evidence
PR Compliance ID 351628 requires explicit type hints on all function parameters in changed files,
including self. The added pytest method test_missing_header_returns_401(self) leaves self
unannotated.

Rule 351628: Require explicit type hints on all Python function parameters and returns in changed files
tests/test_access_api_auth.py[38-38]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New pytest test methods have untyped `self` parameters, which violates the rule requiring explicit type hints for *all* parameters (including `self`) in changed files.
## Issue Context
This affects newly added tests and may cause failures under strict mypy/typing enforcement.
## Fix Focus Areas
- tests/test_access_api_auth.py[35-124]
- tests/test_access_client_ip.py[28-81]
- tests/test_access_tailscale.py[30-122]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Untyped fixture patch_trusted_proxies📘 Rule violation ▣ Testability
Description
The new pytest fixture patch_trusted_proxies has no type annotations for parameters and no return
type annotation. This violates the requirement for explicit typing on all function parameters and
returns in changed files.
Code

tests/test_access_client_ip.py[R55-61]

+    @pytest.fixture(autouse=True)
+    def patch_trusted_proxies(self):
+        import ipaddress
+
+        trusted = [ipaddress.ip_network("10.0.0.0/8")]
+        with patch("supervaizer.access.client_ip.TRUSTED_PROXIES", trusted):
+            yield
Evidence
PR Compliance ID 351628 requires explicit type hints on all parameters (including self) and
explicit return types. The added fixture function signature def patch_trusted_proxies(self):
provides neither.

Rule 351628: Require explicit type hints on all Python function parameters and returns in changed files
tests/test_access_client_ip.py[55-61]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `patch_trusted_proxies` pytest fixture lacks type hints for `self` and has no return type, violating the strict typing requirement for changed files.
## Issue Context
Pytest fixtures are functions too; they must comply with the same parameter/return annotation rules.
## Fix Focus Areas
- tests/test_access_client_ip.py[55-61]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. type: ignore lacks justification📘 Rule violation ≡ Correctness
Description
A new # type: ignore[union-attr] was added without an inline justification comment. This violates
the requirement that # type: ignore[...] be narrowly scoped and justified to avoid masking real
typing issues.
Code

src/supervaizer/access/tailscale.py[46]

+            if hasattr(ws, "client_state") and ws.client_state == WebSocketState.CONNECTING:  # type: ignore[union-attr]
Evidence
PR Compliance ID 116967 requires # type: ignore[...] comments to include a short justification and
be scoped to a specific error code. The added ignore has the code but no justification text.

Rule 116967: Enforce type hints and mypy-clean Python code
src/supervaizer/access/tailscale.py[46-46]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A `# type: ignore[union-attr]` was introduced without a justification, which can hide genuine type-safety problems and violates the project's typing compliance requirements.
## Issue Context
If `conn` can be both `HTTPConnection` and `WebSocket`, consider narrowing the type or using `isinstance` checks; if an ignore is truly needed, add a short reason inline.
## Fix Focus Areas
- src/supervaizer/access/tailscale.py[42-48]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (1)
4. Hard-coded API keys🐞 Bug ⛨ Security
Description
require_api_key() accepts two static keys (key_123, key_456) baked into API_KEYS, allowing
unauthorized callers who know these defaults to authenticate and obtain read/write access. This is
effectively default credentials on the entire /api surface.
Code

src/supervaizer/access/api_auth.py[R21-24]

+API_KEYS: dict[str, dict[str, str]] = {
+    "key_123": {"scope": "read"},
+    "key_456": {"scope": "write"},
+}
Evidence
API_KEYS is initialized with two fixed keys and require_api_key() returns metadata for any key
present in that dict, so these defaults will work in any deployment that hasn't removed them.

src/supervaizer/access/api_auth.py[19-56]
src/supervaizer/routers/api.py[34-37]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The code ships with hard-coded API keys (`key_123`, `key_456`) that grant read/write access.
## Issue Context
`require_api_key()` treats any entry in `API_KEYS` as valid. Because `API_KEYS` is initialized with static defaults, those credentials are valid in production unless removed.
## Fix Focus Areas
- src/supervaizer/access/api_auth.py[19-64]
### Suggested fix
- Initialize `API_KEYS` as empty by default.
- Load keys from configuration only (e.g., `SUPERVAIZER_API_KEY` for a single write key, or a new env like `SUPERVAIZER_API_KEYS_JSON` / `SUPERVAIZER_API_KEYS` to support multiple keys + scopes).
- Update tests to inject keys via env/app.state rather than relying on baked-in defaults.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

5. Unknown scope fails open 🐞 Bug ≡ Correctness
Description
require_scope() defaults unknown required_scope values to rank 0 (read), so a typo (e.g.
require_scope("wrtie")) silently weakens protection instead of failing closed. This can
unintentionally allow read-scoped keys to access endpoints meant to be write-protected.
Code

src/supervaizer/access/api_auth.py[R76-79]

+        key_scope = meta.get("scope", "")
+        key_rank = _SCOPE_RANK.get(key_scope, -1)
+        req_rank = _SCOPE_RANK.get(required_scope, 0)
+        if key_rank < req_rank:
Evidence
The required scope rank is computed with a default of 0 when the scope name is missing from
_SCOPE_RANK, which is the least restrictive level in the hierarchy.

src/supervaizer/access/api_auth.py[26-83]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`require_scope()` treats unknown scopes as "read" by default, which can silently downgrade authorization requirements.
## Issue Context
The scope hierarchy is defined in `_SCOPE_RANK`, but `required_scope` lookup uses a permissive default.
## Fix Focus Areas
- src/supervaizer/access/api_auth.py[67-85]
### Suggested fix
- Validate `required_scope` at dependency creation time:
- if `required_scope not in _SCOPE_RANK`: raise `ValueError` (or raise `HTTPException(status_code=500, ...)` if you prefer runtime signaling).
- Add a unit test that asserts an unknown scope is rejected (so typos are caught immediately).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. Custom routes docs mismatch 🐞 Bug ⚙ Maintainability
Description
Agent custom_routes are mounted under /api/agents/{slug}/..., but repository documentation
states they are mounted at /agents/{slug}/api/. This inconsistency will mislead integrators and
can cause clients following the documented path to hit 404s.
Code

src/supervaizer/routers/api.py[R49-55]

+    # Agent custom routes
+    for agent in server.agents:
+        if agent.custom_routes:
+            api_router.include_router(
+                agent.custom_routes,
+                prefix=f"/agents/{agent.slug}",
+            )
Evidence
The runtime mount prefix in create_api_router() omits the documented /api/ suffix segment, while
the changelog explicitly describes the older mount location.

src/supervaizer/routers/api.py[49-55]
docs/CHANGELOG.md[123-128]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The documented mount path for `Agent.custom_routes` does not match the actual mount prefix used by the API router.
## Issue Context
Docs say `/agents/{slug}/api/`, code mounts at `/api/agents/{slug}/...`.
## Fix Focus Areas
- src/supervaizer/routers/api.py[49-55]
- docs/CHANGELOG.md[123-128]
### Suggested fix (choose one)
1) **Keep code behavior** and update documentation to the new `/api/agents/{slug}/...` path.
2) **Keep documentation behavior** and change the mount prefix to include the `/api` segment (e.g. `prefix=f"/agents/{agent.slug}/api"`), ensuring the resulting full path is `/api/agents/{slug}/api/...`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

7. Tailscale WS doc mismatch 🐞 Bug ⚙ Maintainability
Description
require_tailscale() docstring claims WebSockets are closed with code 1008, but the
implementation rejects by raising HTTPException(403) and never uses a WS close code. This mismatch
makes the behavior harder to reason about and can mislead future changes/tests.
Code

src/supervaizer/access/tailscale.py[R25-48]

+    """FastAPI dependency that allows only requests from the Tailscale CGNAT range.
+
+    Raises HTTP 403 for plain HTTP connections and closes WebSocket connections
+    with code 1008 when the client IP is outside 100.64.0.0/10.
+    """
+    path = conn.scope.get("path", "")
+    ip = _extract_client_ip(conn.scope)
+
+    allowed = False
+    if ip:
+        try:
+            allowed = ipaddress.ip_address(ip) in _TAILSCALE_CGNAT
+        except ValueError:
+            pass  # stays False — fail closed
+
+    if not allowed:
+        log_access_denied_tailscale(ip, path, "not in tailscale range")
+        if conn.scope.get("type") == "websocket":
+            # For WebSocket connections, close with policy violation code
+            # We need to check if the connection is still in a connectable state
+            ws = conn  # conn IS the WebSocket for ws scope
+            if hasattr(ws, "client_state") and ws.client_state == WebSocketState.CONNECTING:  # type: ignore[union-attr]
+                raise HTTPException(status_code=403, detail="Forbidden: Tailscale network required")
+            raise HTTPException(status_code=403, detail="Forbidden: Tailscale network required")
Evidence
The docstring explicitly states a 1008 close, while the websocket branch only raises
HTTPException(403); there is no ws.close(code=1008) or equivalent WS-specific exception path.

src/supervaizer/access/tailscale.py[24-49]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The `require_tailscale()` docstring describes closing WebSockets with code 1008, but the function actually rejects unauthorized WebSocket upgrades via HTTP 403.
## Issue Context
This is mainly a documentation/clarity issue; the current mechanism (403 during handshake) can be acceptable, but it should be described accurately.
## Fix Focus Areas
- src/supervaizer/access/tailscale.py[24-49]
### Suggested fix
- Update the docstring to state that unauthorized WebSocket **upgrades are rejected with HTTP 403**.
- Optionally remove the redundant `client_state` check since both branches raise the same exception.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Comment thread tests/test_access_api_auth.py Outdated
Comment thread tests/test_access_client_ip.py
Comment thread src/supervaizer/access/tailscale.py Outdated
Comment thread src/supervaizer/access/api_auth.py Outdated
alain-sv and others added 8 commits April 18, 2026 20:01
- Remove hard-coded API keys (key_123/key_456) from api_auth.py — API_KEYS
  is now empty by default; only SUPERVAIZER_API_KEY env var populates it
- Add self type annotations (self: "ClassName") to all test class methods
- Add return type Generator[None, None, None] to patch_trusted_proxies fixture
- Update test_access_api_auth.py to inject test keys via patch.dict instead
  of relying on baked-in defaults

https://claude.ai/code/session_011Ansn4kxHVP8nLmLWESz36
…ELOG.md showing overallstatus counts and runtime. clarifies current CI test results(502,0 skipped,0 failed, ~54) and preserves the abouttest file updates and access-test. The table addedimmediately0.14.2 release heading to make recent testresults visible to readers and maintainers.
- cli.py startup banner now prints /manage/ and "API key for /manage"
- workbench-form.js default URL fallbacks updated to /manage/agents/...
- workbench_routes.py log-filter string updated to /manage/

https://claude.ai/code/session_011Ansn4kxHVP8nLmLWESz36
When SUPERVAIZER_LOCAL_MODE=true, 127.0.0.1 and ::1 bypass the
Tailscale CGNAT check so /manage/ is reachable during local development
without a Tailscale connection.

https://claude.ai/code/session_011Ansn4kxHVP8nLmLWESz36
@alain-sv
alain-sv merged commit 4e6c5f2 into develop Apr 20, 2026
6 checks passed
@alain-sv
alain-sv deleted the claude/multi-surface-access-control-kdTqn branch May 13, 2026 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants