From e75f8566b583b23265db2d4fe89d6166c777bd1f Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Sun, 19 Jul 2026 15:51:39 -0400 Subject: [PATCH 1/8] fix(api): require auth on model-manager and app-info read endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A number of model-management and app-info routes carried no auth dependency at all. In multi-user mode this left them reachable by a fully unauthenticated network attacker, contradicting the documented model that model management is administrator-only. The highest-impact route was `GET /api/v2/models/scan_folder`, which takes an attacker-controlled filesystem path and recursively enumerates it, returning absolute paths of model-like files. Its distinct 200/400/500 responses also formed an existence/readability oracle for arbitrary paths. While auditing, `GET /api/v1/app/runtime_config` turned out to be worse: it served the raw `InvokeAIAppConfig` — including `remote_api_tokens` and the `external_*_api_key` values — in plaintext, to anyone. Changes: - `scan_folder` now requires `AdminUserOrDefault`, and every failure mode returns one generic 400 (details go to the server log) so the response cannot be used as a filesystem oracle. Arbitrary paths remain scannable by admins, consistent with `install_model`, which already accepts local paths. - Admin-only (`AdminUserOrDefault`): `missing`, `hugging_face`, `starter_models`, `stats`, `hf_login`, `external_providers/config`, `logging` (GET/POST) and the `invocation_cache` routes. All are either operator-only or rendered exclusively behind the admin-gated install panel. - Authenticated-user (`CurrentUserOrDefault`): model record reads, which ordinary users need for generation, plus `runtime_config`, `app_deps`, `patchmatch_status` and `external_providers/status`. - `runtime_config` now masks API keys and download tokens server-side rather than shipping them to the browser; the UI only ever needed the is-configured signal. Single-user mode is unaffected — `*OrDefault` resolves to a system admin when `multiuser` is off. Not changed: the binary-serving routes (image full/thumbnail, model cover image, workflow thumbnail) are consumed via `` and cannot carry a Bearer header, so closing those needs a signed-URL or cookie scheme. Closes #9365 Co-Authored-By: Claude Opus 4.8 (1M context) --- invokeai/app/api/routers/app_info.py | 51 ++++-- invokeai/app/api/routers/model_manager.py | 35 +++-- tests/app/routers/conftest.py | 2 + tests/app/routers/test_app_info.py | 1 + tests/app/routers/test_model_manager.py | 4 + .../test_model_manager_authorization.py | 146 ++++++++++++++++++ 6 files changed, 214 insertions(+), 25 deletions(-) create mode 100644 tests/app/routers/test_model_manager_authorization.py diff --git a/invokeai/app/api/routers/app_info.py b/invokeai/app/api/routers/app_info.py index 832e58f5e24..8f2d13f1622 100644 --- a/invokeai/app/api/routers/app_info.py +++ b/invokeai/app/api/routers/app_info.py @@ -11,7 +11,7 @@ from fastapi.routing import APIRouter from pydantic import BaseModel, Field, model_validator -from invokeai.app.api.auth_dependencies import AdminUserOrDefault +from invokeai.app.api.auth_dependencies import AdminUserOrDefault, CurrentUserOrDefault from invokeai.app.api.dependencies import ApiDependencies from invokeai.app.services.config.config_default import ( EXTERNAL_PROVIDER_CONFIG_FIELDS, @@ -55,7 +55,7 @@ async def get_version() -> AppVersion: @app_router.get("/app_deps", operation_id="get_app_deps", status_code=200, response_model=dict[str, str]) -async def get_app_deps() -> dict[str, str]: +async def get_app_deps(current_user: CurrentUserOrDefault) -> dict[str, str]: deps: dict[str, str] = {dist.metadata["Name"]: dist.version for dist in distributions()} try: cuda = getattr(getattr(torch, "version", None), "cuda", None) or "N/A" # pyright: ignore[reportAttributeAccessIssue] @@ -70,7 +70,7 @@ async def get_app_deps() -> dict[str, str]: @app_router.get("/patchmatch_status", operation_id="get_patchmatch_status", status_code=200, response_model=bool) -async def get_patchmatch_status() -> bool: +async def get_patchmatch_status(current_user: CurrentUserOrDefault) -> bool: return PatchMatch.patchmatch_available() @@ -139,12 +139,38 @@ def validate_explicit_nulls(self) -> "UpdateAppGenerationSettingsRequest": return self +REDACTED_SECRET = "**********" + + +def _redact_config_secrets(config: InvokeAIAppConfig) -> InvokeAIAppConfig: + """Return a copy of the config with credential fields masked. + + The runtime config is readable by every authenticated user, but it carries provider API keys and model-download + bearer tokens. Those are never needed by the client - the UI only cares whether a credential is configured - so + they are masked here rather than shipped to the browser. + """ + updates: dict[str, Any] = {} + + for field_name in EXTERNAL_PROVIDER_CONFIG_FIELDS: + if not field_name.endswith("_api_key"): + continue + if getattr(config, field_name, None): + updates[field_name] = REDACTED_SECRET + + if config.remote_api_tokens: + updates["remote_api_tokens"] = [ + pair.model_copy(update={"token": REDACTED_SECRET}) for pair in config.remote_api_tokens + ] + + return config.model_copy(update=updates) if updates else config + + @app_router.get( "/runtime_config", operation_id="get_runtime_config", status_code=200, response_model=InvokeAIAppConfigWithSetFields ) -async def get_runtime_config() -> InvokeAIAppConfigWithSetFields: +async def get_runtime_config(current_user: CurrentUserOrDefault) -> InvokeAIAppConfigWithSetFields: config = get_config() - return InvokeAIAppConfigWithSetFields(set_fields=config.model_fields_set, config=config) + return InvokeAIAppConfigWithSetFields(set_fields=config.model_fields_set, config=_redact_config_secrets(config)) @app_router.patch( @@ -178,7 +204,7 @@ async def update_runtime_config( status_code=200, response_model=list[ExternalProviderStatusModel], ) -async def get_external_provider_statuses() -> list[ExternalProviderStatusModel]: +async def get_external_provider_statuses(current_user: CurrentUserOrDefault) -> list[ExternalProviderStatusModel]: statuses = ApiDependencies.invoker.services.external_generation.get_provider_statuses() return [status_to_model(status) for status in statuses.values()] @@ -189,7 +215,7 @@ async def get_external_provider_statuses() -> list[ExternalProviderStatusModel]: status_code=200, response_model=list[ExternalProviderConfigModel], ) -async def get_external_provider_configs() -> list[ExternalProviderConfigModel]: +async def get_external_provider_configs(current_admin: AdminUserOrDefault) -> list[ExternalProviderConfigModel]: config = get_config() return [_build_external_provider_config(provider_id, config) for provider_id in EXTERNAL_PROVIDER_FIELDS] @@ -340,7 +366,7 @@ def _remove_external_models_for_provider(provider_id: str) -> None: responses={200: {"description": "The operation was successful"}}, response_model=LogLevel, ) -async def get_log_level() -> LogLevel: +async def get_log_level(current_admin: AdminUserOrDefault) -> LogLevel: """Returns the log level""" return LogLevel(ApiDependencies.invoker.services.logger.level) @@ -352,6 +378,7 @@ async def get_log_level() -> LogLevel: response_model=LogLevel, ) async def set_log_level( + current_admin: AdminUserOrDefault, level: LogLevel = Body(description="New log verbosity level"), ) -> LogLevel: """Sets the log verbosity level""" @@ -364,7 +391,7 @@ async def set_log_level( operation_id="clear_invocation_cache", responses={200: {"description": "The operation was successful"}}, ) -async def clear_invocation_cache() -> None: +async def clear_invocation_cache(current_admin: AdminUserOrDefault) -> None: """Clears the invocation cache""" ApiDependencies.invoker.services.invocation_cache.clear() @@ -374,7 +401,7 @@ async def clear_invocation_cache() -> None: operation_id="enable_invocation_cache", responses={200: {"description": "The operation was successful"}}, ) -async def enable_invocation_cache() -> None: +async def enable_invocation_cache(current_admin: AdminUserOrDefault) -> None: """Clears the invocation cache""" ApiDependencies.invoker.services.invocation_cache.enable() @@ -384,7 +411,7 @@ async def enable_invocation_cache() -> None: operation_id="disable_invocation_cache", responses={200: {"description": "The operation was successful"}}, ) -async def disable_invocation_cache() -> None: +async def disable_invocation_cache(current_admin: AdminUserOrDefault) -> None: """Clears the invocation cache""" ApiDependencies.invoker.services.invocation_cache.disable() @@ -394,6 +421,6 @@ async def disable_invocation_cache() -> None: operation_id="get_invocation_cache_status", responses={200: {"model": InvocationCacheStatus}}, ) -async def get_invocation_cache_status() -> InvocationCacheStatus: +async def get_invocation_cache_status(current_admin: AdminUserOrDefault) -> InvocationCacheStatus: """Clears the invocation cache""" return ApiDependencies.invoker.services.invocation_cache.get_status() diff --git a/invokeai/app/api/routers/model_manager.py b/invokeai/app/api/routers/model_manager.py index 0321a254296..27bce97d253 100644 --- a/invokeai/app/api/routers/model_manager.py +++ b/invokeai/app/api/routers/model_manager.py @@ -19,7 +19,7 @@ from starlette.exceptions import HTTPException from typing_extensions import Annotated -from invokeai.app.api.auth_dependencies import AdminUserOrDefault +from invokeai.app.api.auth_dependencies import AdminUserOrDefault, CurrentUserOrDefault from invokeai.app.api.dependencies import ApiDependencies from invokeai.app.services.model_images.model_images_common import ModelImageFileNotFoundException from invokeai.app.services.model_install.model_install_common import ModelInstallJob @@ -155,6 +155,7 @@ def prepare_model_config_for_response(config: AnyModelConfig, dependencies: Type operation_id="list_model_records", ) async def list_model_records( + current_user: CurrentUserOrDefault, base_models: Optional[List[BaseModelType]] = Query(default=None, description="Base models to include"), model_type: Optional[ModelType] = Query(default=None, description="The type of model to get"), model_name: Optional[str] = Query(default=None, description="Exact match on the name of the model"), @@ -199,7 +200,7 @@ async def list_model_records( operation_id="list_missing_models", responses={200: {"description": "List of models with missing files"}}, ) -async def list_missing_models() -> ModelsList: +async def list_missing_models(current_admin: AdminUserOrDefault) -> ModelsList: """Get models whose files are missing from disk. These are models that have database entries but their corresponding @@ -224,6 +225,7 @@ async def list_missing_models() -> ModelsList: response_model=AnyModelConfig, ) async def get_model_records_by_attrs( + current_user: CurrentUserOrDefault, name: str = Query(description="The name of the model"), type: ModelType = Query(description="The type of the model"), base: BaseModelType = Query(description="The base model of the model"), @@ -245,6 +247,7 @@ async def get_model_records_by_attrs( response_model=AnyModelConfig, ) async def get_model_records_by_hash( + current_user: CurrentUserOrDefault, hash: str = Query(description="The hash of the model"), ) -> AnyModelConfig: """Gets a model by its hash. This is useful for recalling models that were deleted and reinstalled, @@ -269,6 +272,7 @@ async def get_model_records_by_hash( }, ) async def get_model_record( + current_user: CurrentUserOrDefault, key: str = Path(description="Key of the model record to fetch."), ) -> AnyModelConfig: """Get a model record""" @@ -341,14 +345,19 @@ class FoundModel(BaseModel): response_model=List[FoundModel], ) async def scan_for_models( + current_admin: AdminUserOrDefault, scan_path: str = Query(description="Directory path to search for models", default=None), ) -> List[FoundModel]: + # A single generic error is used for every failure mode below, so that the response cannot be used as an + # existence/readability oracle for arbitrary filesystem paths. + scan_failed = HTTPException( + status_code=400, + detail=f"The search path '{scan_path}' could not be scanned", + ) + path = pathlib.Path(scan_path) if not scan_path or not path.is_dir(): - raise HTTPException( - status_code=400, - detail=f"The search path '{scan_path}' does not exist or is not directory", - ) + raise scan_failed search = ModelSearch() try: @@ -372,11 +381,10 @@ async def scan_for_models( found_model = FoundModel(path=path, is_installed=is_installed) scan_results.append(found_model) except Exception as e: - error_type = type(e).__name__ - raise HTTPException( - status_code=500, - detail=f"An error occurred while searching the directory: {error_type}", + ApiDependencies.invoker.services.logger.error( + f"Error scanning '{scan_path}' for models: {type(e).__name__}: {e}" ) + raise scan_failed return scan_results @@ -396,6 +404,7 @@ class HuggingFaceModels(BaseModel): response_model=HuggingFaceModels, ) async def get_hugging_face_models( + current_admin: AdminUserOrDefault, hugging_face_repo: str = Query(description="Hugging face repo to search for models", default=None), ) -> HuggingFaceModels: try: @@ -1258,7 +1267,7 @@ def get_is_installed( @model_manager_router.get("/starter_models", operation_id="get_starter_models", response_model=StarterModelResponse) -async def get_starter_models() -> StarterModelResponse: +async def get_starter_models(current_admin: AdminUserOrDefault) -> StarterModelResponse: installed_models = ApiDependencies.invoker.services.model_manager.store.search_by_attr() starter_models = deepcopy(STARTER_MODELS) starter_bundles = deepcopy(STARTER_BUNDLES) @@ -1291,7 +1300,7 @@ async def get_starter_models() -> StarterModelResponse: response_model=Optional[CacheStats], summary="Get model manager RAM cache performance statistics.", ) -async def get_stats() -> Optional[CacheStats]: +async def get_stats(current_admin: AdminUserOrDefault) -> Optional[CacheStats]: """Return performance statistics on the model manager's RAM cache. Will return null if no models have been loaded.""" return ApiDependencies.invoker.services.model_manager.load.ram_cache.stats @@ -1341,7 +1350,7 @@ def reset_token(cls) -> HFTokenStatus: @model_manager_router.get("/hf_login", operation_id="get_hf_login_status", response_model=HFTokenStatus) -async def get_hf_login_status() -> HFTokenStatus: +async def get_hf_login_status(current_admin: AdminUserOrDefault) -> HFTokenStatus: token_status = HFTokenHelper.get_status() if token_status is HFTokenStatus.UNKNOWN: diff --git a/tests/app/routers/conftest.py b/tests/app/routers/conftest.py index 1355f618be8..fe3e2bba14c 100644 --- a/tests/app/routers/conftest.py +++ b/tests/app/routers/conftest.py @@ -57,6 +57,8 @@ def __init__(self, invoker: Invoker) -> None: "invokeai.app.api.routers.images", "invokeai.app.api.routers.workflows", "invokeai.app.api.routers._access", + "invokeai.app.api.routers.model_manager", + "invokeai.app.api.routers.app_info", ) diff --git a/tests/app/routers/test_app_info.py b/tests/app/routers/test_app_info.py index da493cee457..94b46b2f2ee 100644 --- a/tests/app/routers/test_app_info.py +++ b/tests/app/routers/test_app_info.py @@ -38,6 +38,7 @@ def test_get_external_provider_statuses(monkeypatch: Any, mock_invoker: Invoker, } monkeypatch.setattr("invokeai.app.api.routers.app_info.ApiDependencies", MockApiDependencies(mock_invoker)) + monkeypatch.setattr("invokeai.app.api.auth_dependencies.ApiDependencies", MockApiDependencies(mock_invoker)) monkeypatch.setattr(mock_invoker.services.external_generation, "get_provider_statuses", lambda: statuses) response = client.get("/api/v1/app/external_providers/status") diff --git a/tests/app/routers/test_model_manager.py b/tests/app/routers/test_model_manager.py index 3260b539bb3..95db8fc4b79 100644 --- a/tests/app/routers/test_model_manager.py +++ b/tests/app/routers/test_model_manager.py @@ -57,6 +57,7 @@ def test_model_manager_external_config_round_trip( invoker = DummyInvoker(services) monkeypatch.setattr("invokeai.app.api.routers.model_manager.ApiDependencies", MockApiDependencies(invoker)) + monkeypatch.setattr("invokeai.app.api.auth_dependencies.ApiDependencies", MockApiDependencies(invoker)) response = client.get("/api/v2/models/", params={"model_type": ModelType.ExternalImageGenerator.value}) @@ -99,6 +100,7 @@ def test_model_manager_external_config_preserves_custom_panel_schema( invoker = DummyInvoker(services) monkeypatch.setattr("invokeai.app.api.routers.model_manager.ApiDependencies", MockApiDependencies(invoker)) + monkeypatch.setattr("invokeai.app.api.auth_dependencies.ApiDependencies", MockApiDependencies(invoker)) response = client.get("/api/v2/models/i/external_custom_schema") @@ -130,6 +132,7 @@ def test_model_manager_external_starter_model_applies_panel_schema_overrides( invoker = DummyInvoker(services) monkeypatch.setattr("invokeai.app.api.routers.model_manager.ApiDependencies", MockApiDependencies(invoker)) + monkeypatch.setattr("invokeai.app.api.auth_dependencies.ApiDependencies", MockApiDependencies(invoker)) response = client.get("/api/v2/models/i/external_starter_schema") @@ -160,6 +163,7 @@ def test_model_manager_gemini_starter_model_applies_reference_and_resolution_ove invoker = DummyInvoker(services) monkeypatch.setattr("invokeai.app.api.routers.model_manager.ApiDependencies", MockApiDependencies(invoker)) + monkeypatch.setattr("invokeai.app.api.auth_dependencies.ApiDependencies", MockApiDependencies(invoker)) response = client.get("/api/v2/models/i/external_gemini_schema") diff --git a/tests/app/routers/test_model_manager_authorization.py b/tests/app/routers/test_model_manager_authorization.py new file mode 100644 index 00000000000..0ce2d6aa5ca --- /dev/null +++ b/tests/app/routers/test_model_manager_authorization.py @@ -0,0 +1,146 @@ +"""Tests for API-level authorization on model-manager and app-info read endpoints. + +These cover the security fix for GH #9365: in multi-user mode a number of model-management and app-info read +routes carried no auth dependency at all, so an unauthenticated network attacker could reach them. The highest +impact was `GET /api/v2/models/scan_folder`, which enumerates an attacker-chosen filesystem path. +""" + +from typing import Any + +import pytest +from fastapi import status +from fastapi.testclient import TestClient + +from invokeai.app.services.config.config_default import InvokeAIAppConfig, URLRegexTokenPair +from invokeai.app.services.invoker import Invoker +from tests.app.routers.conftest import _auth + +# (method, path) for routes that must reject unauthenticated callers in multiuser mode. +PROTECTED_ROUTES = [ + ("get", "/api/v2/models/scan_folder?scan_path=/etc"), + ("get", "/api/v2/models/"), + ("get", "/api/v2/models/missing"), + ("get", "/api/v2/models/get_by_attrs?name=x&type=main&base=sd-1"), + ("get", "/api/v2/models/get_by_hash?hash=x"), + ("get", "/api/v2/models/i/some-key"), + ("get", "/api/v2/models/hugging_face?hugging_face_repo=x/y"), + ("get", "/api/v2/models/starter_models"), + ("get", "/api/v2/models/stats"), + ("get", "/api/v2/models/hf_login"), + ("get", "/api/v1/app/runtime_config"), + ("get", "/api/v1/app/app_deps"), + ("get", "/api/v1/app/patchmatch_status"), + ("get", "/api/v1/app/external_providers/status"), + ("get", "/api/v1/app/external_providers/config"), + ("get", "/api/v1/app/logging"), + ("get", "/api/v1/app/invocation_cache/status"), + ("delete", "/api/v1/app/invocation_cache"), + ("put", "/api/v1/app/invocation_cache/enable"), + ("put", "/api/v1/app/invocation_cache/disable"), +] + +# Routes that must additionally reject authenticated non-admin users. +ADMIN_ONLY_ROUTES = [ + ("get", "/api/v2/models/scan_folder?scan_path=/etc"), + ("get", "/api/v2/models/missing"), + ("get", "/api/v2/models/hugging_face?hugging_face_repo=x/y"), + ("get", "/api/v2/models/starter_models"), + ("get", "/api/v2/models/stats"), + ("get", "/api/v2/models/hf_login"), + ("get", "/api/v1/app/external_providers/config"), + ("get", "/api/v1/app/logging"), + ("delete", "/api/v1/app/invocation_cache"), + ("put", "/api/v1/app/invocation_cache/enable"), + ("put", "/api/v1/app/invocation_cache/disable"), +] + + +@pytest.mark.parametrize(("method", "path"), PROTECTED_ROUTES) +def test_route_rejects_unauthenticated( + method: str, path: str, enable_multiuser: Any, setup_jwt_secret: None, client: TestClient +) -> None: + """In multiuser mode, no Authorization header must yield 401 - never a 200 or a data-bearing error.""" + response = getattr(client, method)(path) + assert response.status_code == status.HTTP_401_UNAUTHORIZED + + +@pytest.mark.parametrize(("method", "path"), ADMIN_ONLY_ROUTES) +def test_route_rejects_non_admin( + method: str, path: str, enable_multiuser: Any, client: TestClient, user1_token: str +) -> None: + """Model-management routes are administrator-only, per the documented multi-user model.""" + response = getattr(client, method)(path, headers=_auth(user1_token)) + assert response.status_code == status.HTTP_403_FORBIDDEN + + +def test_scan_folder_allows_admin(enable_multiuser: Any, client: TestClient, admin_token: str) -> None: + """An admin still reaches the handler - 400 here is the scan failing, not the auth layer rejecting.""" + response = client.get( + "/api/v2/models/scan_folder?scan_path=/nonexistent_xyz", + headers=_auth(admin_token), + ) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + +@pytest.mark.parametrize("scan_path", ["/nonexistent_xyz", "/etc/ssl", "/"]) +def test_scan_folder_is_not_an_existence_oracle( + scan_path: str, enable_multiuser: Any, client: TestClient, admin_token: str +) -> None: + """Nonexistent, unreadable and readable-but-unscannable paths must be indistinguishable from the response. + + Previously these returned 400 / 500 / 200 respectively, letting a caller probe arbitrary paths. + """ + response = client.get(f"/api/v2/models/scan_folder?scan_path={scan_path}", headers=_auth(admin_token)) + assert response.status_code in (status.HTTP_200_OK, status.HTTP_400_BAD_REQUEST) + if response.status_code == status.HTTP_400_BAD_REQUEST: + assert response.json()["detail"] == f"The search path '{scan_path}' could not be scanned" + + +def test_regular_user_can_still_list_models( + enable_multiuser: Any, client: TestClient, user1_token: str, mock_invoker: Invoker +) -> None: + """Listing models is needed for ordinary generation, so it must stay available to non-admins.""" + # model_manager is a MagicMock from the enable_multiuser fixture; give the store a real, empty result set. + mock_invoker.services.model_manager.store.search_by_attr.return_value = [] + + response = client.get("/api/v2/models/", headers=_auth(user1_token)) + assert response.status_code == status.HTTP_200_OK + assert response.json() == {"models": []} + + +def test_runtime_config_redacts_secrets() -> None: + """API keys and download bearer tokens must never be serialized to a client.""" + from invokeai.app.api.routers.app_info import REDACTED_SECRET, _redact_config_secrets + + config = InvokeAIAppConfig( + external_openai_api_key="sk-super-secret", + external_gemini_base_url="https://example.invalid", + remote_api_tokens=[URLRegexTokenPair(url_regex="example.com", token="bearer-secret")], + ) + redacted = _redact_config_secrets(config) + + assert redacted.external_openai_api_key == REDACTED_SECRET + assert redacted.remote_api_tokens is not None + assert redacted.remote_api_tokens[0].token == REDACTED_SECRET + # Non-secret fields are untouched, and the original config is not mutated. + assert redacted.remote_api_tokens[0].url_regex == "example.com" + assert redacted.external_gemini_base_url == "https://example.invalid" + assert config.external_openai_api_key == "sk-super-secret" + assert config.remote_api_tokens is not None + assert config.remote_api_tokens[0].token == "bearer-secret" + + +def test_runtime_config_response_has_no_secrets( + enable_multiuser: Any, client: TestClient, user1_token: str, monkeypatch: Any +) -> None: + """End-to-end: an authenticated non-admin gets the config, but with credentials masked.""" + config = InvokeAIAppConfig( + external_openai_api_key="sk-super-secret", + remote_api_tokens=[URLRegexTokenPair(url_regex="example.com", token="bearer-secret")], + ) + monkeypatch.setattr("invokeai.app.api.routers.app_info.get_config", lambda: config) + + response = client.get("/api/v1/app/runtime_config", headers=_auth(user1_token)) + assert response.status_code == status.HTTP_200_OK + assert "sk-super-secret" not in response.text + assert "bearer-secret" not in response.text From 6ad1f5c9cae646a6312b418b74bb0d4f2fa5da93 Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Sun, 19 Jul 2026 19:01:03 -0400 Subject: [PATCH 2/8] chore(api): regenerate openapi.json for new auth dependencies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The added auth dependencies make FastAPI emit a `security: [{HTTPBearer: []}]` requirement on each affected route, so the checked-in schema no longer matched the generated one. Regenerated with the openapi-checks command. The diff is exactly the 21 routes gated in the previous commit gaining a security requirement; no route loses one, and the rest of the document is byte-identical. `schema.ts` is unchanged — openapi-typescript does not encode security requirements in the generated types. Co-Authored-By: Claude Opus 4.8 (1M context) --- invokeai/frontend/web/openapi.json | 135 +++++++++++++++++++++++++---- 1 file changed, 120 insertions(+), 15 deletions(-) diff --git a/invokeai/frontend/web/openapi.json b/invokeai/frontend/web/openapi.json index c877494d23a..cd00baf7473 100644 --- a/invokeai/frontend/web/openapi.json +++ b/invokeai/frontend/web/openapi.json @@ -585,6 +585,11 @@ "summary": "List Model Records", "description": "Get a list of models.", "operationId": "list_model_records", + "security": [ + { + "HTTPBearer": [] + } + ], "parameters": [ { "name": "base_models", @@ -725,7 +730,12 @@ } } } - } + }, + "security": [ + { + "HTTPBearer": [] + } + ] } }, "/api/v2/models/get_by_attrs": { @@ -734,6 +744,11 @@ "summary": "Get Model Records By Attrs", "description": "Gets a model by its attributes. The main use of this route is to provide backwards compatibility with the old\nmodel manager, which identified models by a combination of name, base and type.", "operationId": "get_model_records_by_attrs", + "security": [ + { + "HTTPBearer": [] + } + ], "parameters": [ { "name": "name", @@ -1078,6 +1093,11 @@ "summary": "Get Model Records By Hash", "description": "Gets a model by its hash. This is useful for recalling models that were deleted and reinstalled,\nas the hash remains stable across reinstallations while the key (UUID) changes.", "operationId": "get_model_records_by_hash", + "security": [ + { + "HTTPBearer": [] + } + ], "parameters": [ { "name": "hash", @@ -1402,6 +1422,11 @@ "summary": "Get Model Record", "description": "Get a model record", "operationId": "get_model_record", + "security": [ + { + "HTTPBearer": [] + } + ], "parameters": [ { "name": "key", @@ -2521,6 +2546,11 @@ "tags": ["model_manager"], "summary": "Scan For Models", "operationId": "scan_for_models", + "security": [ + { + "HTTPBearer": [] + } + ], "parameters": [ { "name": "scan_path", @@ -2570,6 +2600,11 @@ "tags": ["model_manager"], "summary": "Get Hugging Face Models", "operationId": "get_hugging_face_models", + "security": [ + { + "HTTPBearer": [] + } + ], "parameters": [ { "name": "hugging_face_repo", @@ -3753,7 +3788,12 @@ } } } - } + }, + "security": [ + { + "HTTPBearer": [] + } + ] } }, "/api/v2/models/stats": { @@ -3781,7 +3821,12 @@ } } } - } + }, + "security": [ + { + "HTTPBearer": [] + } + ] } }, "/api/v2/models/empty_model_cache": { @@ -3823,7 +3868,12 @@ } } } - } + }, + "security": [ + { + "HTTPBearer": [] + } + ] }, "post": { "tags": ["model_manager"], @@ -6516,7 +6566,12 @@ } } } - } + }, + "security": [ + { + "HTTPBearer": [] + } + ] } }, "/api/v1/app/patchmatch_status": { @@ -6536,7 +6591,12 @@ } } } - } + }, + "security": [ + { + "HTTPBearer": [] + } + ] } }, "/api/v1/app/runtime_config": { @@ -6555,7 +6615,12 @@ } } } - } + }, + "security": [ + { + "HTTPBearer": [] + } + ] }, "patch": { "tags": ["app"], @@ -6621,7 +6686,12 @@ } } } - } + }, + "security": [ + { + "HTTPBearer": [] + } + ] } }, "/api/v1/app/external_providers/config": { @@ -6644,7 +6714,12 @@ } } } - } + }, + "security": [ + { + "HTTPBearer": [] + } + ] } }, "/api/v1/app/external_providers/config/{provider_id}": { @@ -6767,7 +6842,12 @@ } } } - } + }, + "security": [ + { + "HTTPBearer": [] + } + ] }, "post": { "tags": ["app"], @@ -6806,7 +6886,12 @@ } } } - } + }, + "security": [ + { + "HTTPBearer": [] + } + ] } }, "/api/v1/app/invocation_cache": { @@ -6824,7 +6909,12 @@ } } } - } + }, + "security": [ + { + "HTTPBearer": [] + } + ] } }, "/api/v1/app/invocation_cache/enable": { @@ -6842,7 +6932,12 @@ } } } - } + }, + "security": [ + { + "HTTPBearer": [] + } + ] } }, "/api/v1/app/invocation_cache/disable": { @@ -6860,7 +6955,12 @@ } } } - } + }, + "security": [ + { + "HTTPBearer": [] + } + ] } }, "/api/v1/app/invocation_cache/status": { @@ -6880,7 +6980,12 @@ } } } - } + }, + "security": [ + { + "HTTPBearer": [] + } + ] } }, "/api/v1/queue/{queue_id}/enqueue_batch": { From 238d0e04d08f4831b03e3615d02d441a3fbf1b20 Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Sun, 19 Jul 2026 19:36:48 -0400 Subject: [PATCH 3/8] fix(api): make runtime_config administrator-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Serving the runtime config to every authenticated user relied on a denylist: `_redact_config_secrets` masks the credential fields we know about, so any future secret added to `InvokeAIAppConfig` would leak by default until someone remembered to update it. Admin-only is an allowlist posture without that failure mode. Nothing outside the admin UI actually needed it. The three non-admin call sites read `config.multiuser` purely to derive "admin or single-user", which is already available from the unauthenticated `/auth/status` route as `multiuser_enabled` — the same source `useIsModelManagerEnabled` uses. The remaining fields (`max_queue_history`, `image_subfolder_strategy`) only feed admin-gated edit controls. - Extract that predicate into `useIsAdmin`, and have `useIsModelManagerEnabled` delegate to it. Semantics are preserved exactly, so this is a pure refactor. - Use it in the three settings components instead of `runtime_config`, and skip the query for non-admins so they generate no failed requests. - AboutModal's debug blob now omits the config section for non-admins. Its client-side redaction is dropped: it only ever masked `remote_api_tokens` and never the `external_*_api_key` fields, so a logged-in non-admin could read provider API keys out of it. The server now masks both. Server-side masking is kept as defense in depth — an admin's browser has no use for the raw values either. Co-Authored-By: Claude Opus 4.8 (1M context) --- invokeai/app/api/routers/app_info.py | 8 ++--- .../web/src/features/auth/hooks/useIsAdmin.ts | 29 +++++++++++++++++++ .../hooks/useIsModelManagerEnabled.ts | 27 ++--------------- .../components/AboutModal/AboutModal.tsx | 28 ++++++++---------- .../SettingsImageStorageMaintenance.tsx | 9 ++---- .../SettingsImageSubfolderStrategySelect.tsx | 20 ++++++------- .../SettingsModal/SettingsModal.tsx | 8 ++--- .../test_model_manager_authorization.py | 7 +++-- 8 files changed, 68 insertions(+), 68 deletions(-) create mode 100644 invokeai/frontend/web/src/features/auth/hooks/useIsAdmin.ts diff --git a/invokeai/app/api/routers/app_info.py b/invokeai/app/api/routers/app_info.py index 8f2d13f1622..8f03221eb1e 100644 --- a/invokeai/app/api/routers/app_info.py +++ b/invokeai/app/api/routers/app_info.py @@ -145,9 +145,9 @@ def validate_explicit_nulls(self) -> "UpdateAppGenerationSettingsRequest": def _redact_config_secrets(config: InvokeAIAppConfig) -> InvokeAIAppConfig: """Return a copy of the config with credential fields masked. - The runtime config is readable by every authenticated user, but it carries provider API keys and model-download - bearer tokens. Those are never needed by the client - the UI only cares whether a credential is configured - so - they are masked here rather than shipped to the browser. + The runtime config carries provider API keys and model-download bearer tokens. The route is admin-only, but no + client - not even an admin's browser - has any use for the raw values; the UI only cares whether a credential is + configured. Masking here means a future secret added to the config is not automatically served to a browser. """ updates: dict[str, Any] = {} @@ -168,7 +168,7 @@ def _redact_config_secrets(config: InvokeAIAppConfig) -> InvokeAIAppConfig: @app_router.get( "/runtime_config", operation_id="get_runtime_config", status_code=200, response_model=InvokeAIAppConfigWithSetFields ) -async def get_runtime_config(current_user: CurrentUserOrDefault) -> InvokeAIAppConfigWithSetFields: +async def get_runtime_config(current_admin: AdminUserOrDefault) -> InvokeAIAppConfigWithSetFields: config = get_config() return InvokeAIAppConfigWithSetFields(set_fields=config.model_fields_set, config=_redact_config_secrets(config)) diff --git a/invokeai/frontend/web/src/features/auth/hooks/useIsAdmin.ts b/invokeai/frontend/web/src/features/auth/hooks/useIsAdmin.ts new file mode 100644 index 00000000000..aa5eeb008ab --- /dev/null +++ b/invokeai/frontend/web/src/features/auth/hooks/useIsAdmin.ts @@ -0,0 +1,29 @@ +import { useAppSelector } from 'app/store/storeHooks'; +import { selectCurrentUser } from 'features/auth/store/authSlice'; +import { useMemo } from 'react'; +import { useGetSetupStatusQuery } from 'services/api/endpoints/auth'; + +/** + * Hook to determine whether the current user holds administrator privileges. + * + * Returns true if: + * - Multiuser mode is disabled (single-user mode = always admin) + * - Multiuser mode is enabled AND the user is an admin + * + * This mirrors the backend's `AdminUserOrDefault` dependency. Prefer it over reading `multiuser` from + * `runtime_config`, which is itself an admin-only route. + */ +export const useIsAdmin = (): boolean => { + const user = useAppSelector(selectCurrentUser); + const { data: setupStatus } = useGetSetupStatusQuery(); + + return useMemo(() => { + // If multiuser is disabled, treat as admin (single-user mode) + if (setupStatus && !setupStatus.multiuser_enabled) { + return true; + } + + // If multiuser is enabled, check if user is admin + return user?.is_admin ?? false; + }, [setupStatus, user]); +}; diff --git a/invokeai/frontend/web/src/features/modelManagerV2/hooks/useIsModelManagerEnabled.ts b/invokeai/frontend/web/src/features/modelManagerV2/hooks/useIsModelManagerEnabled.ts index 81b00eb77e5..7746b5d796e 100644 --- a/invokeai/frontend/web/src/features/modelManagerV2/hooks/useIsModelManagerEnabled.ts +++ b/invokeai/frontend/web/src/features/modelManagerV2/hooks/useIsModelManagerEnabled.ts @@ -1,29 +1,8 @@ -import { useAppSelector } from 'app/store/storeHooks'; -import { selectCurrentUser } from 'features/auth/store/authSlice'; -import { useMemo } from 'react'; -import { useGetSetupStatusQuery } from 'services/api/endpoints/auth'; +import { useIsAdmin } from 'features/auth/hooks/useIsAdmin'; /** * Hook to determine if model manager features should be enabled for the current user. * - * Returns true if: - * - Multiuser mode is disabled (single-user mode = always admin) - * - Multiuser mode is enabled AND user is an admin - * - * Returns false if: - * - Multiuser mode is enabled AND user is not an admin + * Model management is administrator-only, so this is exactly the admin check. */ -export const useIsModelManagerEnabled = (): boolean => { - const user = useAppSelector(selectCurrentUser); - const { data: setupStatus } = useGetSetupStatusQuery(); - - return useMemo(() => { - // If multiuser is disabled, treat as admin (single-user mode) - if (setupStatus && !setupStatus.multiuser_enabled) { - return true; - } - - // If multiuser is enabled, check if user is admin - return user?.is_admin ?? false; - }, [setupStatus, user]); -}; +export const useIsModelManagerEnabled = (): boolean => useIsAdmin(); diff --git a/invokeai/frontend/web/src/features/system/components/AboutModal/AboutModal.tsx b/invokeai/frontend/web/src/features/system/components/AboutModal/AboutModal.tsx index ed1f7cf4d2f..111bddeaa95 100644 --- a/invokeai/frontend/web/src/features/system/components/AboutModal/AboutModal.tsx +++ b/invokeai/frontend/web/src/features/system/components/AboutModal/AboutModal.tsx @@ -15,7 +15,7 @@ import { Text, useDisclosure, } from '@invoke-ai/ui-library'; -import { deepClone } from 'common/util/deepClone'; +import { useIsAdmin } from 'features/auth/hooks/useIsAdmin'; import DataViewer from 'features/gallery/components/ImageMetadataViewer/DataViewer'; import { discordLink, githubLink } from 'features/system/store/constants'; import InvokeLogoYellow from 'public/assets/images/invoke-tag-lrg.svg'; @@ -32,26 +32,22 @@ type AboutModalProps = { const AboutModal = ({ children }: AboutModalProps) => { const { isOpen, onOpen, onClose } = useDisclosure(); const { t } = useTranslation(); - const { data: runtimeConfig } = useGetRuntimeConfigQuery(); + const isAdmin = useIsAdmin(); + // The runtime config is administrator-only, so non-admins get a debug blob without the config section. + const { data: runtimeConfig } = useGetRuntimeConfigQuery(undefined, { skip: !isAdmin }); const { data: dependencies } = useGetAppDepsQuery(); const { data: appVersion } = useGetAppVersionQuery(); - const localData = useMemo(() => { - const clonedRuntimeConfig = deepClone(runtimeConfig); - - if (clonedRuntimeConfig && clonedRuntimeConfig.config.remote_api_tokens) { - clonedRuntimeConfig.config.remote_api_tokens.forEach((remote_api_token) => { - remote_api_token.token = 'REDACTED'; - }); - } - - return { + const localData = useMemo( + () => ({ version: appVersion?.version, dependencies, - config: clonedRuntimeConfig?.config, - set_config_fields: clonedRuntimeConfig?.set_fields, - }; - }, [appVersion, dependencies, runtimeConfig]); + // Credentials in the config are already masked by the server; no client-side redaction needed. + config: runtimeConfig?.config, + set_config_fields: runtimeConfig?.set_fields, + }), + [appVersion, dependencies, runtimeConfig] + ); return ( <> diff --git a/invokeai/frontend/web/src/features/system/components/SettingsModal/SettingsImageStorageMaintenance.tsx b/invokeai/frontend/web/src/features/system/components/SettingsModal/SettingsImageStorageMaintenance.tsx index 49018e657aa..2da533e990a 100644 --- a/invokeai/frontend/web/src/features/system/components/SettingsModal/SettingsImageStorageMaintenance.tsx +++ b/invokeai/frontend/web/src/features/system/components/SettingsModal/SettingsImageStorageMaintenance.tsx @@ -1,11 +1,10 @@ import { Button, Flex, FormControl, FormLabel, Text } from '@invoke-ai/ui-library'; -import { useAppDispatch, useAppSelector } from 'app/store/storeHooks'; -import { selectCurrentUser } from 'features/auth/store/authSlice'; +import { useAppDispatch } from 'app/store/storeHooks'; +import { useIsAdmin } from 'features/auth/hooks/useIsAdmin'; import { toast } from 'features/toast/toast'; import { memo, useCallback, useEffect, useMemo, useState } from 'react'; import { useTranslation } from 'react-i18next'; import { api, LIST_TAG } from 'services/api'; -import { useGetRuntimeConfigQuery } from 'services/api/endpoints/appInfo'; import { useGetImageMoveStatusQuery, useStartImageMoveMutation, @@ -47,9 +46,7 @@ const invalidatedImageMoveJobIds = new Set(); export const SettingsImageStorageMaintenance = memo(() => { const { t } = useTranslation(); const dispatch = useAppDispatch(); - const currentUser = useAppSelector(selectCurrentUser); - const { data: runtimeConfig } = useGetRuntimeConfigQuery(); - const canAccess = runtimeConfig ? !runtimeConfig.config.multiuser || Boolean(currentUser?.is_admin) : false; + const canAccess = useIsAdmin(); const [startImageMove, startImageMoveState] = useStartImageMoveMutation(); const [startImageMoveRecovery, startImageMoveRecoveryState] = useStartImageMoveRecoveryMutation(); const [shouldPollStatus, setShouldPollStatus] = useState(false); diff --git a/invokeai/frontend/web/src/features/system/components/SettingsModal/SettingsImageSubfolderStrategySelect.tsx b/invokeai/frontend/web/src/features/system/components/SettingsModal/SettingsImageSubfolderStrategySelect.tsx index b156a50de27..7f986d8249d 100644 --- a/invokeai/frontend/web/src/features/system/components/SettingsModal/SettingsImageSubfolderStrategySelect.tsx +++ b/invokeai/frontend/web/src/features/system/components/SettingsModal/SettingsImageSubfolderStrategySelect.tsx @@ -1,7 +1,6 @@ import type { ComboboxOnChange } from '@invoke-ai/ui-library'; import { Combobox, FormControl, FormLabel } from '@invoke-ai/ui-library'; -import { useAppSelector } from 'app/store/storeHooks'; -import { selectCurrentUser } from 'features/auth/store/authSlice'; +import { useIsAdmin } from 'features/auth/hooks/useIsAdmin'; import { toast } from 'features/toast/toast'; import { memo, useCallback, useMemo } from 'react'; import { useTranslation } from 'react-i18next'; @@ -38,11 +37,11 @@ export const getImageSubfolderStrategyOption = (strategy: string): ImageSubfolde export const SettingsImageSubfolderStrategySelect = memo(() => { const { t } = useTranslation(); - const currentUser = useAppSelector(selectCurrentUser); - const { data: runtimeConfig } = useGetRuntimeConfigQuery(); + const isAdmin = useIsAdmin(); + // runtime_config is an admin-only route; don't fire it for non-admins just to render a disabled control. + const { data: runtimeConfig } = useGetRuntimeConfigQuery(undefined, { skip: !isAdmin }); const [updateRuntimeConfig, { isLoading }] = useUpdateRuntimeConfigMutation(); const imageSubfolderStrategy: string = runtimeConfig?.config.image_subfolder_strategy ?? 'flat'; - const canEditRuntimeConfig = runtimeConfig ? !runtimeConfig.config.multiuser || currentUser?.is_admin : false; const options = useMemo(() => { const localizedOptions: ImageSubfolderStrategySelectOption[] = imageSubfolderStrategyOptions.map((option) => ({ @@ -84,15 +83,14 @@ export const SettingsImageSubfolderStrategySelect = memo(() => { [imageSubfolderStrategy, t, updateRuntimeConfig] ); + if (!isAdmin) { + return null; + } + return ( {t('settings.imageSubfolderStrategy')} - + ); }); diff --git a/invokeai/frontend/web/src/features/system/components/SettingsModal/SettingsModal.tsx b/invokeai/frontend/web/src/features/system/components/SettingsModal/SettingsModal.tsx index 3bcfb90e46f..78cfda2d36d 100644 --- a/invokeai/frontend/web/src/features/system/components/SettingsModal/SettingsModal.tsx +++ b/invokeai/frontend/web/src/features/system/components/SettingsModal/SettingsModal.tsx @@ -21,7 +21,7 @@ import { useAppDispatch, useAppSelector } from 'app/store/storeHooks'; import { InformationalPopover } from 'common/components/InformationalPopover/InformationalPopover'; import ScrollableContent from 'common/components/OverlayScrollbars/ScrollableContent'; import { buildUseBoolean } from 'common/hooks/useBoolean'; -import { selectCurrentUser } from 'features/auth/store/authSlice'; +import { useIsAdmin } from 'features/auth/hooks/useIsAdmin'; import { selectShouldUseCPUNoise, shouldUseCpuNoiseChanged } from 'features/controlLayers/store/paramsSlice'; import { ExternalProviderStatusList } from 'features/system/components/SettingsModal/ExternalProviderStatusList'; import { useRefreshAfterResetModal } from 'features/system/components/SettingsModal/RefreshAfterResetModal'; @@ -89,8 +89,9 @@ const SettingsModal = (props: { children: ReactElement<{ onClick?: () => void }> const settingsModal = useSettingsModal(); const refreshModal = useRefreshAfterResetModal(); - const currentUser = useAppSelector(selectCurrentUser); - const { data: runtimeConfig } = useGetRuntimeConfigQuery(); + const canEditRuntimeConfig = useIsAdmin(); + // runtime_config is an admin-only route; don't fire it for non-admins just to render disabled controls. + const { data: runtimeConfig } = useGetRuntimeConfigQuery(undefined, { skip: !canEditRuntimeConfig }); const [updateRuntimeConfig, { isLoading: isUpdatingRuntimeConfig }] = useUpdateRuntimeConfigMutation(); const pendingMaxQueueHistoryRef = useRef(undefined); @@ -108,7 +109,6 @@ const SettingsModal = (props: { children: ReactElement<{ onClick?: () => void }> const shouldConfirmOnNewSession = useAppSelector(selectSystemShouldConfirmOnNewSession); const shouldShowInvocationProgressDetail = useAppSelector(selectSystemShouldShowInvocationProgressDetail); const maxQueueHistory = runtimeConfig?.config.max_queue_history ?? null; - const canEditRuntimeConfig = runtimeConfig ? !runtimeConfig.config.multiuser || currentUser?.is_admin : false; const [maxQueueHistoryInputState, setMaxQueueHistoryInputState] = useState(() => ({ source: maxQueueHistory, value: formatOptionalInteger(maxQueueHistory), diff --git a/tests/app/routers/test_model_manager_authorization.py b/tests/app/routers/test_model_manager_authorization.py index 0ce2d6aa5ca..9e54b2fe966 100644 --- a/tests/app/routers/test_model_manager_authorization.py +++ b/tests/app/routers/test_model_manager_authorization.py @@ -48,6 +48,7 @@ ("get", "/api/v2/models/stats"), ("get", "/api/v2/models/hf_login"), ("get", "/api/v1/app/external_providers/config"), + ("get", "/api/v1/app/runtime_config"), ("get", "/api/v1/app/logging"), ("delete", "/api/v1/app/invocation_cache"), ("put", "/api/v1/app/invocation_cache/enable"), @@ -131,16 +132,16 @@ def test_runtime_config_redacts_secrets() -> None: def test_runtime_config_response_has_no_secrets( - enable_multiuser: Any, client: TestClient, user1_token: str, monkeypatch: Any + enable_multiuser: Any, client: TestClient, admin_token: str, monkeypatch: Any ) -> None: - """End-to-end: an authenticated non-admin gets the config, but with credentials masked.""" + """End-to-end: even an admin's browser never receives the raw credentials.""" config = InvokeAIAppConfig( external_openai_api_key="sk-super-secret", remote_api_tokens=[URLRegexTokenPair(url_regex="example.com", token="bearer-secret")], ) monkeypatch.setattr("invokeai.app.api.routers.app_info.get_config", lambda: config) - response = client.get("/api/v1/app/runtime_config", headers=_auth(user1_token)) + response = client.get("/api/v1/app/runtime_config", headers=_auth(admin_token)) assert response.status_code == status.HTTP_200_OK assert "sk-super-secret" not in response.text assert "bearer-secret" not in response.text From f8dad23b31ff8d3e960111f95c32af8ffeab7f52 Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Mon, 20 Jul 2026 19:55:29 -0400 Subject: [PATCH 4/8] =?UTF-8?q?fix(api):=20address=20review=20=E2=80=94=20?= =?UTF-8?q?PATCH=20redaction,=20/missing=20access,=20cache=20UI=20gating?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three issues from JPPhoto's review of #9367: - `PATCH /runtime_config` echoed the updated config back unredacted, so an admin changing an unrelated setting still pulled every external API key and `remote_api_tokens` value into the browser's RTK Query cache, defeating the masking added to the GET route. It now returns `_redact_config_secrets(config)` as GET does. - `GET /models/missing` is `CurrentUserOrDefault` again, not admin-only. The frontend's model hooks (`modelsByType.ts`) subtract this set from the model list so unusable records stay out of the generation dropdowns. Under an admin-only gate a non-admin got 403, the subtraction became a silent no-op, and missing models were offered for selection, failing only at execution. - The invocation-cache panel lives on the universally available Queue tab, but its routes are admin-only. A non-admin saw zeroed statistics and an active Enable button that always 403s. The panel is now gated with `useIsAdmin`, and the enable/disable/clear hooks additionally refuse to offer the mutation to a non-admin, so the guard does not depend on the render site alone. `useIsAdmin`'s predicate is extracted as a pure `getIsAdmin` so it can be unit-tested alongside the new cache-control predicates. Tests: - `test_runtime_config_patch_response_has_no_secrets` patches a writable setting with secrets configured and asserts they are absent from the response. - `test_non_admin_can_filter_out_missing_models` gives a non-admin one present and one missing model record and checks the missing one is not selectable. - `invocationCacheControls.test.ts` asserts a connected non-admin is offered no cache mutation, and that admin semantics are unchanged. openapi.json is unchanged: `/models/missing` keeps the same HTTPBearer security requirement either way. Co-Authored-By: Claude Opus 4.8 (1M context) --- invokeai/app/api/routers/app_info.py | 2 +- invokeai/app/api/routers/model_manager.py | 5 +- .../web/src/features/auth/hooks/useIsAdmin.ts | 30 +++--- .../queue/components/QueueTabContent.tsx | 7 +- .../hooks/invocationCacheControls.test.ts | 65 +++++++++++++ .../queue/hooks/invocationCacheControls.ts | 31 +++++++ .../queue/hooks/useClearInvocationCache.ts | 9 +- .../queue/hooks/useDisableInvocationCache.ts | 5 +- .../queue/hooks/useEnableInvocationCache.ts | 5 +- .../test_model_manager_authorization.py | 91 ++++++++++++++++++- 10 files changed, 232 insertions(+), 18 deletions(-) create mode 100644 invokeai/frontend/web/src/features/queue/hooks/invocationCacheControls.test.ts create mode 100644 invokeai/frontend/web/src/features/queue/hooks/invocationCacheControls.ts diff --git a/invokeai/app/api/routers/app_info.py b/invokeai/app/api/routers/app_info.py index 8f03221eb1e..c17fad7e77d 100644 --- a/invokeai/app/api/routers/app_info.py +++ b/invokeai/app/api/routers/app_info.py @@ -195,7 +195,7 @@ async def update_runtime_config( persisted_config.update_config(update_dict) persisted_config.write_file(config.config_file_path) - return InvokeAIAppConfigWithSetFields(set_fields=config.model_fields_set, config=config) + return InvokeAIAppConfigWithSetFields(set_fields=config.model_fields_set, config=_redact_config_secrets(config)) @app_router.get( diff --git a/invokeai/app/api/routers/model_manager.py b/invokeai/app/api/routers/model_manager.py index 27bce97d253..50be3ac0da3 100644 --- a/invokeai/app/api/routers/model_manager.py +++ b/invokeai/app/api/routers/model_manager.py @@ -200,11 +200,14 @@ async def list_model_records( operation_id="list_missing_models", responses={200: {"description": "List of models with missing files"}}, ) -async def list_missing_models(current_admin: AdminUserOrDefault) -> ModelsList: +async def list_missing_models(current_user: CurrentUserOrDefault) -> ModelsList: """Get models whose files are missing from disk. These are models that have database entries but their corresponding weight files have been deleted externally (not via Model Manager). + + Available to any authenticated user, not just admins: the frontend's model hooks subtract this + set from the model list so unusable models are kept out of the generation dropdowns. """ record_store = ApiDependencies.invoker.services.model_manager.store models_path = ApiDependencies.invoker.services.configuration.models_path diff --git a/invokeai/frontend/web/src/features/auth/hooks/useIsAdmin.ts b/invokeai/frontend/web/src/features/auth/hooks/useIsAdmin.ts index aa5eeb008ab..599403ed809 100644 --- a/invokeai/frontend/web/src/features/auth/hooks/useIsAdmin.ts +++ b/invokeai/frontend/web/src/features/auth/hooks/useIsAdmin.ts @@ -4,12 +4,28 @@ import { useMemo } from 'react'; import { useGetSetupStatusQuery } from 'services/api/endpoints/auth'; /** - * Hook to determine whether the current user holds administrator privileges. - * * Returns true if: * - Multiuser mode is disabled (single-user mode = always admin) * - Multiuser mode is enabled AND the user is an admin * + * Exported separately from the hook so the predicate can be unit-tested directly. + */ +export const getIsAdmin = ( + setupStatus: { multiuser_enabled: boolean } | undefined, + user: { is_admin: boolean } | null | undefined +): boolean => { + // If multiuser is disabled, treat as admin (single-user mode) + if (setupStatus && !setupStatus.multiuser_enabled) { + return true; + } + + // If multiuser is enabled, check if user is admin + return user?.is_admin ?? false; +}; + +/** + * Hook to determine whether the current user holds administrator privileges. + * * This mirrors the backend's `AdminUserOrDefault` dependency. Prefer it over reading `multiuser` from * `runtime_config`, which is itself an admin-only route. */ @@ -17,13 +33,5 @@ export const useIsAdmin = (): boolean => { const user = useAppSelector(selectCurrentUser); const { data: setupStatus } = useGetSetupStatusQuery(); - return useMemo(() => { - // If multiuser is disabled, treat as admin (single-user mode) - if (setupStatus && !setupStatus.multiuser_enabled) { - return true; - } - - // If multiuser is enabled, check if user is admin - return user?.is_admin ?? false; - }, [setupStatus, user]); + return useMemo(() => getIsAdmin(setupStatus, user), [setupStatus, user]); }; diff --git a/invokeai/frontend/web/src/features/queue/components/QueueTabContent.tsx b/invokeai/frontend/web/src/features/queue/components/QueueTabContent.tsx index a2919937ea0..2f888abc34d 100644 --- a/invokeai/frontend/web/src/features/queue/components/QueueTabContent.tsx +++ b/invokeai/frontend/web/src/features/queue/components/QueueTabContent.tsx @@ -1,4 +1,5 @@ import { Box, Flex } from '@invoke-ai/ui-library'; +import { useIsAdmin } from 'features/auth/hooks/useIsAdmin'; import { memo } from 'react'; import InvocationCacheStatus from './InvocationCacheStatus'; @@ -7,12 +8,16 @@ import QueueStatus from './QueueStatus'; import QueueTabQueueControls from './QueueTabQueueControls'; const QueueTabContent = () => { + // The invocation cache status and its enable/disable/clear mutations are administrator-only on the + // backend. Rendering the panel for a non-admin would show zeroed stats and offer buttons that 403. + const isAdmin = useIsAdmin(); + return ( - + {isAdmin && } diff --git a/invokeai/frontend/web/src/features/queue/hooks/invocationCacheControls.test.ts b/invokeai/frontend/web/src/features/queue/hooks/invocationCacheControls.test.ts new file mode 100644 index 00000000000..57d73720e0b --- /dev/null +++ b/invokeai/frontend/web/src/features/queue/hooks/invocationCacheControls.test.ts @@ -0,0 +1,65 @@ +import { getIsAdmin } from 'features/auth/hooks/useIsAdmin'; +import { describe, expect, it } from 'vitest'; + +import { + getIsClearInvocationCacheDisabled, + getIsDisableInvocationCacheDisabled, + getIsEnableInvocationCacheDisabled, +} from './invocationCacheControls'; + +const ENABLED_CACHE = { enabled: true, size: 4, hits: 1, misses: 1, max_size: 20 }; +const DISABLED_CACHE = { enabled: false, size: 0, hits: 0, misses: 0, max_size: 20 }; + +describe('invocation cache controls', () => { + describe('non-admin in multiuser mode', () => { + // The status route is admin-only, so a non-admin's cacheStatus is always undefined. Without the + // admin check that reads as "cache is disabled" and offers an Enable button that always 403s. + const isAdmin = getIsAdmin({ multiuser_enabled: true }, { is_admin: false }); + + it('is not admin', () => { + expect(isAdmin).toBe(false); + }); + + it('offers no cache mutation while connected with no status data', () => { + expect(getIsEnableInvocationCacheDisabled(isAdmin, true, undefined)).toBe(true); + expect(getIsDisableInvocationCacheDisabled(isAdmin, true, undefined)).toBe(true); + expect(getIsClearInvocationCacheDisabled(isAdmin, true, undefined)).toBe(true); + }); + + it('offers no cache mutation even if status data is somehow present', () => { + for (const status of [ENABLED_CACHE, DISABLED_CACHE]) { + expect(getIsEnableInvocationCacheDisabled(isAdmin, true, status)).toBe(true); + expect(getIsDisableInvocationCacheDisabled(isAdmin, true, status)).toBe(true); + expect(getIsClearInvocationCacheDisabled(isAdmin, true, status)).toBe(true); + } + }); + }); + + describe('admin', () => { + it('treats the multiuser admin and the single-user operator alike', () => { + expect(getIsAdmin({ multiuser_enabled: true }, { is_admin: true })).toBe(true); + expect(getIsAdmin({ multiuser_enabled: false }, { is_admin: false })).toBe(true); + }); + + it('preserves the pre-existing enable/disable/clear semantics', () => { + // Enable is offered only when the cache is off and has capacity. + expect(getIsEnableInvocationCacheDisabled(true, true, DISABLED_CACHE)).toBe(false); + expect(getIsEnableInvocationCacheDisabled(true, true, ENABLED_CACHE)).toBe(true); + expect(getIsEnableInvocationCacheDisabled(true, true, { ...DISABLED_CACHE, max_size: 0 })).toBe(true); + + // Disable is offered only when the cache is on. + expect(getIsDisableInvocationCacheDisabled(true, true, ENABLED_CACHE)).toBe(false); + expect(getIsDisableInvocationCacheDisabled(true, true, DISABLED_CACHE)).toBe(true); + + // Clear is offered only when there is something cached. + expect(getIsClearInvocationCacheDisabled(true, true, ENABLED_CACHE)).toBe(false); + expect(getIsClearInvocationCacheDisabled(true, true, DISABLED_CACHE)).toBe(true); + }); + + it('offers nothing while disconnected', () => { + expect(getIsEnableInvocationCacheDisabled(true, false, DISABLED_CACHE)).toBe(true); + expect(getIsDisableInvocationCacheDisabled(true, false, ENABLED_CACHE)).toBe(true); + expect(getIsClearInvocationCacheDisabled(true, false, ENABLED_CACHE)).toBe(true); + }); + }); +}); diff --git a/invokeai/frontend/web/src/features/queue/hooks/invocationCacheControls.ts b/invokeai/frontend/web/src/features/queue/hooks/invocationCacheControls.ts new file mode 100644 index 00000000000..64573876141 --- /dev/null +++ b/invokeai/frontend/web/src/features/queue/hooks/invocationCacheControls.ts @@ -0,0 +1,31 @@ +import type { components } from 'services/api/schema'; + +type InvocationCacheStatus = components['schemas']['InvocationCacheStatus']; + +/** + * Disabled-state predicates for the invocation cache controls. + * + * The invocation cache routes are administrator-only on the backend, so a non-admin must never be + * offered any of these mutations: `cacheStatus` is undefined for them (the status route 403s), which + * on its own reads as "cache disabled" and would present an active Enable button. + * + * These are pure so they can be unit-tested without rendering. + */ + +export const getIsEnableInvocationCacheDisabled = ( + isAdmin: boolean, + isConnected: boolean, + cacheStatus: InvocationCacheStatus | undefined +): boolean => !isAdmin || !isConnected || !!cacheStatus?.enabled || cacheStatus?.max_size === 0; + +export const getIsDisableInvocationCacheDisabled = ( + isAdmin: boolean, + isConnected: boolean, + cacheStatus: InvocationCacheStatus | undefined +): boolean => !isAdmin || !isConnected || !cacheStatus?.enabled || cacheStatus?.max_size === 0; + +export const getIsClearInvocationCacheDisabled = ( + isAdmin: boolean, + isConnected: boolean, + cacheStatus: InvocationCacheStatus | undefined +): boolean => !isAdmin || !isConnected || !cacheStatus?.size; diff --git a/invokeai/frontend/web/src/features/queue/hooks/useClearInvocationCache.ts b/invokeai/frontend/web/src/features/queue/hooks/useClearInvocationCache.ts index 90c5a0dd3af..3a28bf50761 100644 --- a/invokeai/frontend/web/src/features/queue/hooks/useClearInvocationCache.ts +++ b/invokeai/frontend/web/src/features/queue/hooks/useClearInvocationCache.ts @@ -1,4 +1,6 @@ import { useStore } from '@nanostores/react'; +import { useIsAdmin } from 'features/auth/hooks/useIsAdmin'; +import { getIsClearInvocationCacheDisabled } from 'features/queue/hooks/invocationCacheControls'; import { toast } from 'features/toast/toast'; import { useCallback } from 'react'; import { useTranslation } from 'react-i18next'; @@ -9,6 +11,7 @@ export const useClearInvocationCache = () => { const { t } = useTranslation(); const { data: cacheStatus } = useGetInvocationCacheStatusQuery(); const isConnected = useStore($isConnected); + const isAdmin = useIsAdmin(); const [_trigger, { isLoading }] = useClearInvocationCacheMutation({ fixedCacheKey: 'clearInvocationCache', }); @@ -30,5 +33,9 @@ export const useClearInvocationCache = () => { } }, [_trigger, t]); - return { trigger, isLoading, isDisabled: !isConnected || !cacheStatus?.size }; + return { + trigger, + isLoading, + isDisabled: getIsClearInvocationCacheDisabled(isAdmin, isConnected, cacheStatus), + }; }; diff --git a/invokeai/frontend/web/src/features/queue/hooks/useDisableInvocationCache.ts b/invokeai/frontend/web/src/features/queue/hooks/useDisableInvocationCache.ts index 17ac6bec44a..0d31ca7d4a4 100644 --- a/invokeai/frontend/web/src/features/queue/hooks/useDisableInvocationCache.ts +++ b/invokeai/frontend/web/src/features/queue/hooks/useDisableInvocationCache.ts @@ -1,4 +1,6 @@ import { useStore } from '@nanostores/react'; +import { useIsAdmin } from 'features/auth/hooks/useIsAdmin'; +import { getIsDisableInvocationCacheDisabled } from 'features/queue/hooks/invocationCacheControls'; import { toast } from 'features/toast/toast'; import { useCallback } from 'react'; import { useTranslation } from 'react-i18next'; @@ -9,6 +11,7 @@ export const useDisableInvocationCache = () => { const { t } = useTranslation(); const { data: cacheStatus } = useGetInvocationCacheStatusQuery(); const isConnected = useStore($isConnected); + const isAdmin = useIsAdmin(); const [_trigger, { isLoading }] = useDisableInvocationCacheMutation({ fixedCacheKey: 'disableInvocationCache', }); @@ -34,6 +37,6 @@ export const useDisableInvocationCache = () => { trigger, isLoading, cacheStatus, - isDisabled: !cacheStatus?.enabled || !isConnected || cacheStatus?.max_size === 0, + isDisabled: getIsDisableInvocationCacheDisabled(isAdmin, isConnected, cacheStatus), }; }; diff --git a/invokeai/frontend/web/src/features/queue/hooks/useEnableInvocationCache.ts b/invokeai/frontend/web/src/features/queue/hooks/useEnableInvocationCache.ts index 02fcf444fa0..fb80b1ddd28 100644 --- a/invokeai/frontend/web/src/features/queue/hooks/useEnableInvocationCache.ts +++ b/invokeai/frontend/web/src/features/queue/hooks/useEnableInvocationCache.ts @@ -1,4 +1,6 @@ import { useStore } from '@nanostores/react'; +import { useIsAdmin } from 'features/auth/hooks/useIsAdmin'; +import { getIsEnableInvocationCacheDisabled } from 'features/queue/hooks/invocationCacheControls'; import { toast } from 'features/toast/toast'; import { useCallback } from 'react'; import { useTranslation } from 'react-i18next'; @@ -9,6 +11,7 @@ export const useEnableInvocationCache = () => { const { t } = useTranslation(); const { data: cacheStatus } = useGetInvocationCacheStatusQuery(); const isConnected = useStore($isConnected); + const isAdmin = useIsAdmin(); const [_trigger, { isLoading }] = useEnableInvocationCacheMutation({ fixedCacheKey: 'enableInvocationCache', }); @@ -33,6 +36,6 @@ export const useEnableInvocationCache = () => { return { trigger, isLoading, - isDisabled: cacheStatus?.enabled || !isConnected || cacheStatus?.max_size === 0, + isDisabled: getIsEnableInvocationCacheDisabled(isAdmin, isConnected, cacheStatus), }; }; diff --git a/tests/app/routers/test_model_manager_authorization.py b/tests/app/routers/test_model_manager_authorization.py index 9e54b2fe966..3a29a6ed571 100644 --- a/tests/app/routers/test_model_manager_authorization.py +++ b/tests/app/routers/test_model_manager_authorization.py @@ -5,7 +5,9 @@ impact was `GET /api/v2/models/scan_folder`, which enumerates an attacker-chosen filesystem path. """ +from pathlib import Path from typing import Any +from unittest.mock import MagicMock import pytest from fastapi import status @@ -13,6 +15,15 @@ from invokeai.app.services.config.config_default import InvokeAIAppConfig, URLRegexTokenPair from invokeai.app.services.invoker import Invoker +from invokeai.backend.model_manager.configs.main import Main_Checkpoint_SD1_Config +from invokeai.backend.model_manager.taxonomy import ( + BaseModelType, + ModelFormat, + ModelSourceType, + ModelType, + ModelVariantType, + SchedulerPredictionType, +) from tests.app.routers.conftest import _auth # (method, path) for routes that must reject unauthenticated callers in multiuser mode. @@ -42,7 +53,6 @@ # Routes that must additionally reject authenticated non-admin users. ADMIN_ONLY_ROUTES = [ ("get", "/api/v2/models/scan_folder?scan_path=/etc"), - ("get", "/api/v2/models/missing"), ("get", "/api/v2/models/hugging_face?hugging_face_repo=x/y"), ("get", "/api/v2/models/starter_models"), ("get", "/api/v2/models/stats"), @@ -109,6 +119,61 @@ def test_regular_user_can_still_list_models( assert response.json() == {"models": []} +def _sd1_checkpoint(key: str, path: str) -> Main_Checkpoint_SD1_Config: + return Main_Checkpoint_SD1_Config( + key=key, + path=path, + name=key, + format=ModelFormat.Checkpoint, + base=BaseModelType.StableDiffusion1, + type=ModelType.Main, + config_path="/tmp/foo.yaml", + variant=ModelVariantType.Normal, + hash="111222333444", + file_size=8192, + source=path, + source_type=ModelSourceType.Path, + prediction_type=SchedulerPredictionType.Epsilon, + ) + + +def test_non_admin_can_filter_out_missing_models( + enable_multiuser: Any, + client: TestClient, + user1_token: str, + mock_invoker: Invoker, + tmp_path: Path, + monkeypatch: Any, +) -> None: + """A non-admin must be able to read `/missing`, or missing models stay in their generation dropdowns. + + The frontend's model hooks (`modelsByType.ts`) subtract `/missing` from `/`. If `/missing` 403s the + subtraction silently becomes a no-op and every missing model is offered for selection, failing only + when execution tries to load its absent files. + """ + (tmp_path / "present.ckpt").write_text("weights") + present = _sd1_checkpoint("present-key", "present.ckpt") + absent = _sd1_checkpoint("absent-key", "absent.ckpt") + + # models_path is a read-only derived property, so patch it on the class for the duration of the test. + monkeypatch.setattr(InvokeAIAppConfig, "models_path", property(lambda _self: tmp_path)) + mock_invoker.services.model_images = MagicMock() + mock_invoker.services.model_manager.store.all_models.return_value = [present, absent] + mock_invoker.services.model_manager.store.search_by_attr.return_value = [present, absent] + + missing = client.get("/api/v2/models/missing", headers=_auth(user1_token)) + assert missing.status_code == status.HTTP_200_OK + missing_keys = {m["key"] for m in missing.json()["models"]} + assert missing_keys == {"absent-key"} + + all_models = client.get("/api/v2/models/", headers=_auth(user1_token)) + assert all_models.status_code == status.HTTP_200_OK + + # This mirrors what the frontend hooks do with the two responses. + selectable = {m["key"] for m in all_models.json()["models"]} - missing_keys + assert selectable == {"present-key"} + + def test_runtime_config_redacts_secrets() -> None: """API keys and download bearer tokens must never be serialized to a client.""" from invokeai.app.api.routers.app_info import REDACTED_SECRET, _redact_config_secrets @@ -145,3 +210,27 @@ def test_runtime_config_response_has_no_secrets( assert response.status_code == status.HTTP_200_OK assert "sk-super-secret" not in response.text assert "bearer-secret" not in response.text + + +def test_runtime_config_patch_response_has_no_secrets( + enable_multiuser: Any, client: TestClient, admin_token: str, monkeypatch: Any, tmp_path: Path +) -> None: + """PATCH echoes the updated config back, so it must redact exactly as GET does. + + Otherwise an admin changing an unrelated setting pulls the raw credentials into the browser's + RTK Query cache, defeating the server-side masking on GET. + """ + config = InvokeAIAppConfig( + external_openai_api_key="sk-super-secret", + remote_api_tokens=[URLRegexTokenPair(url_regex="example.com", token="bearer-secret")], + ) + config._config_file = tmp_path / "invokeai.yaml" + monkeypatch.setattr("invokeai.app.api.routers.app_info.get_config", lambda: config) + + response = client.patch("/api/v1/app/runtime_config", json={"max_queue_history": 5}, headers=_auth(admin_token)) + assert response.status_code == status.HTTP_200_OK + assert response.json()["config"]["max_queue_history"] == 5 + assert "sk-super-secret" not in response.text + assert "bearer-secret" not in response.text + # The in-memory config itself must keep the real values - masking is response-only. + assert config.external_openai_api_key == "sk-super-secret" From b39f28265d9d765f62dce8209e47a04ea7ebf472 Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Mon, 20 Jul 2026 20:24:21 -0400 Subject: [PATCH 5/8] =?UTF-8?q?fix:=20address=20self-review=20findings=20?= =?UTF-8?q?=E2=80=94=20scan=5Ffolder=20crash,=20stale=20spec,=20UI=20gatin?= =?UTF-8?q?g?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend: - scan_for_models: guard scan_path before pathlib.Path() — the query param defaults to None and Path(None) raised a TypeError (500) when omitted. - scan_for_models: mid-scan failures return a detail-free 500 again instead of the generic 400. By that point is_dir() has confirmed existence, so a distinct status leaks nothing, and it restores the caller-error vs server-fault distinction for admins and retrying clients. Details still go to the log only. - _redact_config_secrets docstring no longer overclaims: coverage is by naming convention, and a differently-named future credential must extend the function. - openapi.json/schema.ts regenerated — the list_missing_models docstring was edited after the last regeneration, which would have failed typegen CI. Tests: - New default-deny meta-test walks app.routes and asserts every route carries an auth dependency or sits on an explicit 12-entry public allowlist, with a staleness check in the other direction. A hand-maintained route list cannot catch the next unauthenticated route — the exact failure mode behind #9365. - The oracle test no longer scans '/' (a real, unbounded ModelSearch walk of the host filesystem); it uses tmp_path fixtures, and the mid-scan-failure path is exercised deterministically by stubbing ModelSearch. - New test for the missing-scan_path 400. Frontend: - SettingsModal hides the Max Queue History field for non-admins instead of rendering it permanently blank-and-disabled (the backing runtime_config query is skipped for them), matching SettingsImageSubfolderStrategySelect. - onModelInstallError only fetches HF token status for admins: the event is broadcast to every client, but /hf_login is admin-only, so every non-admin session fired a doomed request and logged a spurious 403. - Consolidated five inline copies of the admin predicate (UseCacheCheckbox, useStarterModelsToast, NoContentForViewer, ModelPicker, UpscaleWarning) onto useIsAdmin, and getIsCustomNodesEnabled now delegates to getIsAdmin. The toast/viewer/picker copies treated a still-loading setup status as admin=true, disagreeing with the canonical predicate. - getIsAdmin unit tests moved to a colocated useIsAdmin.test.ts per the frontend CLAUDE.md colocation rule; the cache-control tests no longer import it. Test results: tests/app/routers 500 passed; frontend vitest 1354 passed; eslint/prettier/tsc clean; production bundle rebuilt via vite build. Co-Authored-By: Claude Fable 5 --- invokeai/app/api/routers/app_info.py | 6 +- invokeai/app/api/routers/model_manager.py | 15 ++- invokeai/frontend/web/openapi.json | 2 +- .../features/auth/hooks/useIsAdmin.test.ts | 30 +++++ .../customNodes/useIsCustomNodesEnabled.ts | 12 +- .../ImageViewer/NoContentForViewer.tsx | 7 +- .../hooks/useStarterModelsToast.tsx | 10 +- .../nodes/Invocation/UseCacheCheckbox.tsx | 20 +-- .../parameters/components/ModelPicker.tsx | 6 +- .../hooks/invocationCacheControls.test.ts | 13 +- .../UpscaleWarning.tsx | 6 +- .../SettingsModal/SettingsModal.tsx | 35 +++--- .../frontend/web/src/services/api/schema.ts | 3 + .../services/events/onModelInstallError.tsx | 42 ++++--- .../test_model_manager_authorization.py | 119 ++++++++++++++++-- 15 files changed, 231 insertions(+), 95 deletions(-) create mode 100644 invokeai/frontend/web/src/features/auth/hooks/useIsAdmin.test.ts diff --git a/invokeai/app/api/routers/app_info.py b/invokeai/app/api/routers/app_info.py index c17fad7e77d..e893a74e5f5 100644 --- a/invokeai/app/api/routers/app_info.py +++ b/invokeai/app/api/routers/app_info.py @@ -147,7 +147,11 @@ def _redact_config_secrets(config: InvokeAIAppConfig) -> InvokeAIAppConfig: The runtime config carries provider API keys and model-download bearer tokens. The route is admin-only, but no client - not even an admin's browser - has any use for the raw values; the UI only cares whether a credential is - configured. Masking here means a future secret added to the config is not automatically served to a browser. + configured. + + NOTE: coverage is by convention, not automatic. Only `*_api_key` fields listed in + EXTERNAL_PROVIDER_CONFIG_FIELDS plus `remote_api_tokens` are masked. If you add a credential to + InvokeAIAppConfig under any other name, you must extend this function or it will be served verbatim. """ updates: dict[str, Any] = {} diff --git a/invokeai/app/api/routers/model_manager.py b/invokeai/app/api/routers/model_manager.py index 50be3ac0da3..7921e93ff83 100644 --- a/invokeai/app/api/routers/model_manager.py +++ b/invokeai/app/api/routers/model_manager.py @@ -351,15 +351,19 @@ async def scan_for_models( current_admin: AdminUserOrDefault, scan_path: str = Query(description="Directory path to search for models", default=None), ) -> List[FoundModel]: - # A single generic error is used for every failure mode below, so that the response cannot be used as an - # existence/readability oracle for arbitrary filesystem paths. + # A single generic error is used for every path-validation failure below, so that the response cannot be + # used as an existence/readability oracle for arbitrary filesystem paths. scan_failed = HTTPException( status_code=400, detail=f"The search path '{scan_path}' could not be scanned", ) + # Guard before constructing a Path: the query param defaults to None and pathlib.Path(None) raises. + if not scan_path: + raise scan_failed + path = pathlib.Path(scan_path) - if not scan_path or not path.is_dir(): + if not path.is_dir(): raise scan_failed search = ModelSearch() @@ -387,7 +391,10 @@ async def scan_for_models( ApiDependencies.invoker.services.logger.error( f"Error scanning '{scan_path}' for models: {type(e).__name__}: {e}" ) - raise scan_failed + # By this point is_dir() has already confirmed the path exists, so a distinct status leaks nothing + # new. Keeping 500 (with the details only in the server log) preserves the caller-error vs + # server-fault distinction for the admin and for retry logic. + raise HTTPException(status_code=500, detail=f"An error occurred while scanning '{scan_path}'") return scan_results diff --git a/invokeai/frontend/web/openapi.json b/invokeai/frontend/web/openapi.json index cd00baf7473..fa93a2fc0f6 100644 --- a/invokeai/frontend/web/openapi.json +++ b/invokeai/frontend/web/openapi.json @@ -717,7 +717,7 @@ "get": { "tags": ["model_manager"], "summary": "List Missing Models", - "description": "Get models whose files are missing from disk.\n\nThese are models that have database entries but their corresponding\nweight files have been deleted externally (not via Model Manager).", + "description": "Get models whose files are missing from disk.\n\nThese are models that have database entries but their corresponding\nweight files have been deleted externally (not via Model Manager).\n\nAvailable to any authenticated user, not just admins: the frontend's model hooks subtract this\nset from the model list so unusable models are kept out of the generation dropdowns.", "operationId": "list_missing_models", "responses": { "200": { diff --git a/invokeai/frontend/web/src/features/auth/hooks/useIsAdmin.test.ts b/invokeai/frontend/web/src/features/auth/hooks/useIsAdmin.test.ts new file mode 100644 index 00000000000..af674654be7 --- /dev/null +++ b/invokeai/frontend/web/src/features/auth/hooks/useIsAdmin.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from 'vitest'; + +import { getIsAdmin } from './useIsAdmin'; + +describe('getIsAdmin', () => { + it('treats single-user mode as admin regardless of user', () => { + expect(getIsAdmin({ multiuser_enabled: false }, undefined)).toBe(true); + expect(getIsAdmin({ multiuser_enabled: false }, null)).toBe(true); + expect(getIsAdmin({ multiuser_enabled: false }, { is_admin: false })).toBe(true); + }); + + it('treats the multiuser admin and the single-user operator alike', () => { + expect(getIsAdmin({ multiuser_enabled: true }, { is_admin: true })).toBe(true); + expect(getIsAdmin({ multiuser_enabled: false }, { is_admin: false })).toBe(true); + }); + + it('denies multiuser non-admin or missing user', () => { + expect(getIsAdmin({ multiuser_enabled: true }, { is_admin: false })).toBe(false); + expect(getIsAdmin({ multiuser_enabled: true }, null)).toBe(false); + expect(getIsAdmin({ multiuser_enabled: true }, undefined)).toBe(false); + }); + + it('denies while setup status is still loading, unless the user is a known admin', () => { + // Conservative during the query window: admin surfaces stay hidden rather than flashing in. + expect(getIsAdmin(undefined, undefined)).toBe(false); + expect(getIsAdmin(undefined, null)).toBe(false); + expect(getIsAdmin(undefined, { is_admin: false })).toBe(false); + expect(getIsAdmin(undefined, { is_admin: true })).toBe(true); + }); +}); diff --git a/invokeai/frontend/web/src/features/customNodes/useIsCustomNodesEnabled.ts b/invokeai/frontend/web/src/features/customNodes/useIsCustomNodesEnabled.ts index 926e51d5474..44abf6cb209 100644 --- a/invokeai/frontend/web/src/features/customNodes/useIsCustomNodesEnabled.ts +++ b/invokeai/frontend/web/src/features/customNodes/useIsCustomNodesEnabled.ts @@ -1,4 +1,5 @@ import { useAppSelector } from 'app/store/storeHooks'; +import { getIsAdmin } from 'features/auth/hooks/useIsAdmin'; import { selectCurrentUser } from 'features/auth/store/authSlice'; import { useMemo } from 'react'; import { useGetSetupStatusQuery } from 'services/api/endpoints/auth'; @@ -6,6 +7,9 @@ import { useGetSetupStatusQuery } from 'services/api/endpoints/auth'; /** * Pure decision function: determines whether custom node management is enabled. * + * Custom node management is admin-only, so this delegates to the canonical admin predicate + * (`getIsAdmin` in features/auth/hooks/useIsAdmin.ts) rather than carrying its own copy of the rule. + * * Returns true if: * - Multiuser mode is disabled (single-user mode = always admin) * - Multiuser mode is enabled AND user is an admin @@ -13,12 +17,8 @@ import { useGetSetupStatusQuery } from 'services/api/endpoints/auth'; * Returns false if: * - Multiuser mode is enabled AND user is not an admin */ -export const getIsCustomNodesEnabled = (multiuserEnabled: boolean, isAdmin: boolean | undefined): boolean => { - if (!multiuserEnabled) { - return true; - } - return isAdmin ?? false; -}; +export const getIsCustomNodesEnabled = (multiuserEnabled: boolean, isAdmin: boolean | undefined): boolean => + getIsAdmin({ multiuser_enabled: multiuserEnabled }, isAdmin === undefined ? undefined : { is_admin: isAdmin }); type CustomNodesPermission = { /** Whether setup status has loaded and a permission decision can be made. */ diff --git a/invokeai/frontend/web/src/features/gallery/components/ImageViewer/NoContentForViewer.tsx b/invokeai/frontend/web/src/features/gallery/components/ImageViewer/NoContentForViewer.tsx index db27a8fcae0..6ed61af5cbb 100644 --- a/invokeai/frontend/web/src/features/gallery/components/ImageViewer/NoContentForViewer.tsx +++ b/invokeai/frontend/web/src/features/gallery/components/ImageViewer/NoContentForViewer.tsx @@ -1,9 +1,8 @@ import type { ButtonProps } from '@invoke-ai/ui-library'; import { Alert, AlertDescription, AlertIcon, Button, Divider, Flex, Link, Spinner, Text } from '@invoke-ai/ui-library'; -import { useAppSelector } from 'app/store/storeHooks'; import { IAINoContentFallback } from 'common/components/IAIImageFallback'; import { InvokeLogoIcon } from 'common/components/InvokeLogoIcon'; -import { selectCurrentUser } from 'features/auth/store/authSlice'; +import { useIsAdmin } from 'features/auth/hooks/useIsAdmin'; import { LOADING_SYMBOL, useHasImages } from 'features/gallery/hooks/useHasImages'; import { setInstallModelsTabByName } from 'features/modelManagerV2/store/installModelsStore'; import { navigationApi } from 'features/ui/layouts/navigation-api'; @@ -18,11 +17,9 @@ export const NoContentForViewer = memo(() => { const hasImages = useHasImages(); const [mainModels, { data }] = useMainModels(); const { data: setupStatus } = useGetSetupStatusQuery(); - const user = useAppSelector(selectCurrentUser); const { t } = useTranslation(); - const isMultiuser = setupStatus?.multiuser_enabled ?? false; - const isAdmin = !isMultiuser || (user?.is_admin ?? false); + const isAdmin = useIsAdmin(); const adminEmail = setupStatus?.admin_email ?? null; const modelsLoaded = data !== undefined; diff --git a/invokeai/frontend/web/src/features/modelManagerV2/hooks/useStarterModelsToast.tsx b/invokeai/frontend/web/src/features/modelManagerV2/hooks/useStarterModelsToast.tsx index 9b76fbbde67..a7229f1a46f 100644 --- a/invokeai/frontend/web/src/features/modelManagerV2/hooks/useStarterModelsToast.tsx +++ b/invokeai/frontend/web/src/features/modelManagerV2/hooks/useStarterModelsToast.tsx @@ -1,11 +1,11 @@ import { Button, Text, useToast } from '@invoke-ai/ui-library'; import { useAppSelector } from 'app/store/storeHooks'; -import { selectCurrentUser, selectIsAuthenticated } from 'features/auth/store/authSlice'; +import { useIsAdmin } from 'features/auth/hooks/useIsAdmin'; +import { selectIsAuthenticated } from 'features/auth/store/authSlice'; import { setInstallModelsTabByName } from 'features/modelManagerV2/store/installModelsStore'; import { navigationApi } from 'features/ui/layouts/navigation-api'; import { useCallback, useEffect, useState } from 'react'; import { useTranslation } from 'react-i18next'; -import { useGetSetupStatusQuery } from 'services/api/endpoints/auth'; import { useMainModels } from 'services/api/hooks/modelsByType'; const TOAST_ID = 'starterModels'; @@ -16,11 +16,7 @@ export const useStarterModelsToast = () => { const [mainModels, { data }] = useMainModels(); const toast = useToast(); const isAuthenticated = useAppSelector(selectIsAuthenticated); - const { data: setupStatus } = useGetSetupStatusQuery(); - const user = useAppSelector(selectCurrentUser); - - const isMultiuser = setupStatus?.multiuser_enabled ?? false; - const isAdmin = !isMultiuser || (user?.is_admin ?? false); + const isAdmin = useIsAdmin(); useEffect(() => { // Only show the toast if the user is authenticated diff --git a/invokeai/frontend/web/src/features/nodes/components/flow/nodes/Invocation/UseCacheCheckbox.tsx b/invokeai/frontend/web/src/features/nodes/components/flow/nodes/Invocation/UseCacheCheckbox.tsx index 219418f36c4..b0f1fdead32 100644 --- a/invokeai/frontend/web/src/features/nodes/components/flow/nodes/Invocation/UseCacheCheckbox.tsx +++ b/invokeai/frontend/web/src/features/nodes/components/flow/nodes/Invocation/UseCacheCheckbox.tsx @@ -1,28 +1,18 @@ import { Checkbox, FormControl, FormLabel } from '@invoke-ai/ui-library'; -import { useAppDispatch, useAppSelector } from 'app/store/storeHooks'; -import { selectCurrentUser } from 'features/auth/store/authSlice'; +import { useAppDispatch } from 'app/store/storeHooks'; +import { useIsAdmin } from 'features/auth/hooks/useIsAdmin'; import { useUseCache } from 'features/nodes/hooks/useUseCache'; import { nodeUseCacheChanged } from 'features/nodes/store/nodesSlice'; import { NO_FIT_ON_DOUBLE_CLICK_CLASS, NO_PAN_CLASS } from 'features/nodes/types/constants'; import type { ChangeEvent } from 'react'; -import { memo, useCallback, useMemo } from 'react'; +import { memo, useCallback } from 'react'; import { useTranslation } from 'react-i18next'; -import { useGetSetupStatusQuery } from 'services/api/endpoints/auth'; const UseCacheCheckbox = ({ nodeId }: { nodeId: string }) => { const dispatch = useAppDispatch(); const useCache = useUseCache(); - const currentUser = useAppSelector(selectCurrentUser); - const { data: setupStatus } = useGetSetupStatusQuery(); - - const isVisible = useMemo(() => { - // In single-user mode (multiuser disabled), always show the checkbox - if (setupStatus && !setupStatus.multiuser_enabled) { - return true; - } - // In multiuser mode, only show the checkbox to admin users - return currentUser?.is_admin ?? false; - }, [setupStatus, currentUser]); + // Node-cache control is admin-only (single-user mode counts as admin). + const isVisible = useIsAdmin(); const handleChange = useCallback( (e: ChangeEvent) => { diff --git a/invokeai/frontend/web/src/features/parameters/components/ModelPicker.tsx b/invokeai/frontend/web/src/features/parameters/components/ModelPicker.tsx index 6ecbf1c4a64..c32a8a51be7 100644 --- a/invokeai/frontend/web/src/features/parameters/components/ModelPicker.tsx +++ b/invokeai/frontend/web/src/features/parameters/components/ModelPicker.tsx @@ -22,7 +22,7 @@ import { buildGroup, getRegex, isGroup, Picker, usePickerContext } from 'common/ import { useDisclosure } from 'common/hooks/useBoolean'; import { typedMemo } from 'common/util/typedMemo'; import { uniq } from 'es-toolkit/compat'; -import { selectCurrentUser } from 'features/auth/store/authSlice'; +import { useIsAdmin } from 'features/auth/hooks/useIsAdmin'; import { selectLoRAsSlice } from 'features/controlLayers/store/lorasSlice'; import { selectParamsSlice } from 'features/controlLayers/store/paramsSlice'; import { MODEL_BASE_TO_COLOR, MODEL_BASE_TO_LONG_NAME, MODEL_BASE_TO_SHORT_NAME } from 'features/modelManagerV2/models'; @@ -91,10 +91,8 @@ const components = { const NoOptionsFallback = memo(({ noOptionsText }: { noOptionsText?: string }) => { const { t } = useTranslation(); const { data: setupStatus } = useGetSetupStatusQuery(); - const user = useAppSelector(selectCurrentUser); - const isMultiuser = setupStatus?.multiuser_enabled ?? false; - const isAdmin = !isMultiuser || (user?.is_admin ?? false); + const isAdmin = useIsAdmin(); const adminEmail = setupStatus?.admin_email ?? null; if (!isAdmin) { diff --git a/invokeai/frontend/web/src/features/queue/hooks/invocationCacheControls.test.ts b/invokeai/frontend/web/src/features/queue/hooks/invocationCacheControls.test.ts index 57d73720e0b..9568b199984 100644 --- a/invokeai/frontend/web/src/features/queue/hooks/invocationCacheControls.test.ts +++ b/invokeai/frontend/web/src/features/queue/hooks/invocationCacheControls.test.ts @@ -1,4 +1,3 @@ -import { getIsAdmin } from 'features/auth/hooks/useIsAdmin'; import { describe, expect, it } from 'vitest'; import { @@ -14,11 +13,8 @@ describe('invocation cache controls', () => { describe('non-admin in multiuser mode', () => { // The status route is admin-only, so a non-admin's cacheStatus is always undefined. Without the // admin check that reads as "cache is disabled" and offers an Enable button that always 403s. - const isAdmin = getIsAdmin({ multiuser_enabled: true }, { is_admin: false }); - - it('is not admin', () => { - expect(isAdmin).toBe(false); - }); + // (getIsAdmin's own semantics are covered by its colocated test, features/auth/hooks/useIsAdmin.test.ts.) + const isAdmin = false; it('offers no cache mutation while connected with no status data', () => { expect(getIsEnableInvocationCacheDisabled(isAdmin, true, undefined)).toBe(true); @@ -36,11 +32,6 @@ describe('invocation cache controls', () => { }); describe('admin', () => { - it('treats the multiuser admin and the single-user operator alike', () => { - expect(getIsAdmin({ multiuser_enabled: true }, { is_admin: true })).toBe(true); - expect(getIsAdmin({ multiuser_enabled: false }, { is_admin: false })).toBe(true); - }); - it('preserves the pre-existing enable/disable/clear semantics', () => { // Enable is offered only when the cache is off and has capacity. expect(getIsEnableInvocationCacheDisabled(true, true, DISABLED_CACHE)).toBe(false); diff --git a/invokeai/frontend/web/src/features/settingsAccordions/components/UpscaleSettingsAccordion/UpscaleWarning.tsx b/invokeai/frontend/web/src/features/settingsAccordions/components/UpscaleSettingsAccordion/UpscaleWarning.tsx index ff19e7ebb31..5344d329c97 100644 --- a/invokeai/frontend/web/src/features/settingsAccordions/components/UpscaleSettingsAccordion/UpscaleWarning.tsx +++ b/invokeai/frontend/web/src/features/settingsAccordions/components/UpscaleSettingsAccordion/UpscaleWarning.tsx @@ -1,6 +1,6 @@ import { Button, Flex, Link, ListItem, Text, UnorderedList } from '@invoke-ai/ui-library'; import { useAppDispatch, useAppSelector } from 'app/store/storeHooks'; -import { selectCurrentUser } from 'features/auth/store/authSlice'; +import { useIsAdmin } from 'features/auth/hooks/useIsAdmin'; import { selectModel } from 'features/controlLayers/store/paramsSlice'; import { setInstallModelsTabByName } from 'features/modelManagerV2/store/installModelsStore'; import { @@ -22,10 +22,8 @@ export const UpscaleWarning = () => { const dispatch = useAppDispatch(); const [modelConfigs, { isLoading }] = useControlNetModels(); const { data: setupStatus } = useGetSetupStatusQuery(); - const user = useAppSelector(selectCurrentUser); - const isMultiuser = setupStatus?.multiuser_enabled ?? false; - const isAdmin = !isMultiuser || (user?.is_admin ?? false); + const isAdmin = useIsAdmin(); const adminEmail = setupStatus?.admin_email ?? null; useEffect(() => { diff --git a/invokeai/frontend/web/src/features/system/components/SettingsModal/SettingsModal.tsx b/invokeai/frontend/web/src/features/system/components/SettingsModal/SettingsModal.tsx index 78cfda2d36d..29054568430 100644 --- a/invokeai/frontend/web/src/features/system/components/SettingsModal/SettingsModal.tsx +++ b/invokeai/frontend/web/src/features/system/components/SettingsModal/SettingsModal.tsx @@ -312,21 +312,26 @@ const SettingsModal = (props: { children: ReactElement<{ onClick?: () => void }> {t('settings.enableInvisibleWatermark')} - - {t('settings.maxQueueHistory')} - - - - + {/* Admin-only: the backing runtime_config query is skipped for non-admins, so a + rendered-but-disabled input would sit permanently blank. Hide it instead, matching + SettingsImageSubfolderStrategySelect below. */} + {canEditRuntimeConfig && ( + + {t('settings.maxQueueHistory')} + + + + + )} diff --git a/invokeai/frontend/web/src/services/api/schema.ts b/invokeai/frontend/web/src/services/api/schema.ts index 29d9e6f2d76..20241ff0da4 100644 --- a/invokeai/frontend/web/src/services/api/schema.ts +++ b/invokeai/frontend/web/src/services/api/schema.ts @@ -378,6 +378,9 @@ export type paths = { * * These are models that have database entries but their corresponding * weight files have been deleted externally (not via Model Manager). + * + * Available to any authenticated user, not just admins: the frontend's model hooks subtract this + * set from the model list so unusable models are kept out of the generation dropdowns. */ get: operations["list_missing_models"]; put?: never; diff --git a/invokeai/frontend/web/src/services/events/onModelInstallError.tsx b/invokeai/frontend/web/src/services/events/onModelInstallError.tsx index 3496ff6ccd5..2aaa90b749a 100644 --- a/invokeai/frontend/web/src/services/events/onModelInstallError.tsx +++ b/invokeai/frontend/web/src/services/events/onModelInstallError.tsx @@ -1,6 +1,8 @@ import { Button, ExternalLink, Spinner, Text } from '@invoke-ai/ui-library'; import { logger } from 'app/logging/logger'; import type { AppDispatch, AppGetState } from 'app/store/store'; +import { getIsAdmin } from 'features/auth/hooks/useIsAdmin'; +import { selectCurrentUser } from 'features/auth/store/authSlice'; import { getPrefixedId } from 'features/controlLayers/konva/util'; import { discordLink, githubIssuesLink } from 'features/system/store/constants'; import { toast, toastApi } from 'features/toast/toast'; @@ -9,6 +11,7 @@ import { t } from 'i18next'; import { useCallback } from 'react'; import { useTranslation } from 'react-i18next'; import { api } from 'services/api'; +import { authApi } from 'services/api/endpoints/auth'; import { modelsApi, useGetHFTokenStatusQuery } from 'services/api/endpoints/models'; import type { S } from 'services/api/types'; import { assert } from 'tsafe'; @@ -39,6 +42,12 @@ const getHFTokenStatus = async (dispatch: AppDispatch): Promise { + const setupStatus = authApi.endpoints.getSetupStatus.select()(getState()).data; + const user = selectCurrentUser(getState()); + return getIsAdmin(setupStatus, user); +}; + /** * Handles model install error events by logging the error, displaying appropriate toast notifications. */ @@ -50,20 +59,25 @@ export const buildOnModelInstallError = (getState: AppGetState, dispatch: AppDis if (data.error === 'Unauthorized') { if (data.source.type === 'hf') { - const hfTokenStatus = await getHFTokenStatus(dispatch); - if (hfTokenStatus === null) { - // This means there was a problem getting the token status, should never happen... - log.error('Error getting HF token status'); - } else { - const title = getHFTokenStatusToastTitle(hfTokenStatus); - toast({ - id: UNAUTHORIZED_TOAST_ID, - title, - description: , - status: 'error', - isClosable: true, - duration: null, - }); + // Admin-only: model_install_error is broadcast to every connected client, but /hf_login is an + // admin-only route and the toast concerns the server-wide HF token that only an admin can fix. + // Without this guard every non-admin session fires a doomed request and logs a spurious 403. + if (getIsAdminFromState(getState)) { + const hfTokenStatus = await getHFTokenStatus(dispatch); + if (hfTokenStatus === null) { + // This means there was a problem getting the token status, should never happen... + log.error('Error getting HF token status'); + } else { + const title = getHFTokenStatusToastTitle(hfTokenStatus); + toast({ + id: UNAUTHORIZED_TOAST_ID, + title, + description: , + status: 'error', + isClosable: true, + duration: null, + }); + } } } else if (data.source.type === 'url') { toast({ diff --git a/tests/app/routers/test_model_manager_authorization.py b/tests/app/routers/test_model_manager_authorization.py index 3a29a6ed571..5aeb2b369e4 100644 --- a/tests/app/routers/test_model_manager_authorization.py +++ b/tests/app/routers/test_model_manager_authorization.py @@ -93,18 +93,121 @@ def test_scan_folder_allows_admin(enable_multiuser: Any, client: TestClient, adm assert response.status_code == status.HTTP_400_BAD_REQUEST -@pytest.mark.parametrize("scan_path", ["/nonexistent_xyz", "/etc/ssl", "/"]) +def test_scan_folder_missing_param_is_a_clean_400( + enable_multiuser: Any, client: TestClient, admin_token: str +) -> None: + """Omitting scan_path must not crash: the None default is guarded before pathlib.Path() sees it.""" + response = client.get("/api/v2/models/scan_folder", headers=_auth(admin_token)) + assert response.status_code == status.HTTP_400_BAD_REQUEST + + def test_scan_folder_is_not_an_existence_oracle( - scan_path: str, enable_multiuser: Any, client: TestClient, admin_token: str + enable_multiuser: Any, client: TestClient, admin_token: str, tmp_path: Path ) -> None: - """Nonexistent, unreadable and readable-but-unscannable paths must be indistinguishable from the response. + """A nonexistent path and an existing non-directory must be indistinguishable from the response. - Previously these returned 400 / 500 / 200 respectively, letting a caller probe arbitrary paths. + Bounded to tmp_path fixtures - scanning real host paths ('/', '/etc') makes runtime depend on the host + filesystem and can walk enormous trees on permissive CI runners. """ - response = client.get(f"/api/v2/models/scan_folder?scan_path={scan_path}", headers=_auth(admin_token)) - assert response.status_code in (status.HTTP_200_OK, status.HTTP_400_BAD_REQUEST) - if response.status_code == status.HTTP_400_BAD_REQUEST: - assert response.json()["detail"] == f"The search path '{scan_path}' could not be scanned" + not_a_dir = tmp_path / "weights.ckpt" + not_a_dir.write_text("not a directory") + + responses = [ + client.get(f"/api/v2/models/scan_folder?scan_path={p}", headers=_auth(admin_token)) + for p in (tmp_path / "does_not_exist", not_a_dir) + ] + for response, p in zip(responses, (tmp_path / "does_not_exist", not_a_dir)): + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert response.json()["detail"] == f"The search path '{p}' could not be scanned" + + +def test_scan_folder_mid_scan_failure_is_a_generic_500( + enable_multiuser: Any, client: TestClient, admin_token: str, tmp_path: Path, monkeypatch: Any +) -> None: + """A failure after path validation is a server fault: 500, with the details only in the server log. + + By that point is_dir() has already confirmed existence, so a distinct status leaks nothing new - and it + preserves the caller-error vs server-fault distinction for the admin and for retrying clients. + """ + + class ExplodingSearch: + def search(self, path: Path) -> None: + raise PermissionError("permission denied inside the tree") + + monkeypatch.setattr("invokeai.app.api.routers.model_manager.ModelSearch", ExplodingSearch) + + response = client.get(f"/api/v2/models/scan_folder?scan_path={tmp_path}", headers=_auth(admin_token)) + assert response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR + # The response must not echo the exception - details go to the server log only. + assert "PermissionError" not in response.text + assert "permission denied" not in response.text + + +# (method, path) pairs that are deliberately public. Everything else must carry an auth dependency. +# +# - auth bootstrap: reachable pre-login by construction +# - binary /thumbnail routes: browsers cannot attach a Bearer header to requests; closing +# these needs a signed-URL or cookie scheme (tracked separately, see PR #9367) +# - version: intentionally public +# - docs/redoc: API documentation +# - POST /api/v1/images/ is a 501 stub +PUBLIC_ROUTES = { + ("GET", "/api/v1/app/version"), + ("POST", "/api/v1/auth/login"), + ("POST", "/api/v1/auth/setup"), + ("GET", "/api/v1/auth/status"), + ("POST", "/api/v1/images/"), + ("GET", "/api/v1/images/i/{image_name}/full"), + ("HEAD", "/api/v1/images/i/{image_name}/full"), + ("GET", "/api/v1/images/i/{image_name}/thumbnail"), + ("GET", "/api/v1/workflows/i/{workflow_id}/thumbnail"), + ("GET", "/api/v2/models/i/{key}/image"), + ("GET", "/docs"), + ("GET", "/redoc"), +} + + +def _routes_without_auth() -> set[tuple[str, str]]: + """Every (method, path) in the app whose dependency tree contains no auth dependency.""" + from fastapi.routing import APIRoute + + from invokeai.app.api import auth_dependencies + from invokeai.app.api_app import app + + auth_functions = {auth_dependencies.get_current_user, auth_dependencies.get_current_user_or_default} + + def has_auth(dependant: Any) -> bool: + if dependant.call in auth_functions: + return True + return any(has_auth(sub) for sub in dependant.dependencies) + + return { + (method, route.path) + for route in app.routes + if isinstance(route, APIRoute) and not has_auth(route.dependant) + for method in route.methods + } + + +def test_every_route_is_authenticated_or_explicitly_public() -> None: + """Default-deny: a new route without an auth dependency must be a conscious, allowlisted decision. + + GH #9365 happened because auth was opt-in per route and nothing failed when a route lacked it. A + hand-maintained list of protected routes cannot catch the next unauthenticated route; walking the app's + actual dependency trees can. + """ + unauthenticated = _routes_without_auth() + + unlisted = unauthenticated - PUBLIC_ROUTES + assert not unlisted, ( + f"Routes without an auth dependency that are not on the public allowlist: {sorted(unlisted)}. " + "Add an auth dependency (CurrentUserOrDefault / AdminUserOrDefault), or - only for routes that must " + "be public - add them to PUBLIC_ROUTES with a justification." + ) + + # Keep the allowlist honest: an entry that gained auth (or disappeared) must be removed. + stale = PUBLIC_ROUTES - unauthenticated + assert not stale, f"PUBLIC_ROUTES entries that are no longer unauthenticated routes: {sorted(stale)}" def test_regular_user_can_still_list_models( From b11501fb316c66b0920bd693f912bacf87bdbfa2 Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Mon, 20 Jul 2026 20:33:18 -0400 Subject: [PATCH 6/8] =?UTF-8?q?fix(tests):=20satisfy=20CI=20ruff=200.11.2?= =?UTF-8?q?=20=E2=80=94=20drop=20zip()=20flagged=20by=20B905,=20apply=20it?= =?UTF-8?q?s=20formatting?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Local ruff passed but CI pins ruff@0.11.2, which flags zip() without strict= (B905) and formats two signatures differently. Replaced the zip with a plain loop over the paths and ran the pinned version's formatter. Co-Authored-By: Claude Fable 5 --- tests/app/routers/test_model_manager_authorization.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/tests/app/routers/test_model_manager_authorization.py b/tests/app/routers/test_model_manager_authorization.py index 5aeb2b369e4..fa65d6420d4 100644 --- a/tests/app/routers/test_model_manager_authorization.py +++ b/tests/app/routers/test_model_manager_authorization.py @@ -93,9 +93,7 @@ def test_scan_folder_allows_admin(enable_multiuser: Any, client: TestClient, adm assert response.status_code == status.HTTP_400_BAD_REQUEST -def test_scan_folder_missing_param_is_a_clean_400( - enable_multiuser: Any, client: TestClient, admin_token: str -) -> None: +def test_scan_folder_missing_param_is_a_clean_400(enable_multiuser: Any, client: TestClient, admin_token: str) -> None: """Omitting scan_path must not crash: the None default is guarded before pathlib.Path() sees it.""" response = client.get("/api/v2/models/scan_folder", headers=_auth(admin_token)) assert response.status_code == status.HTTP_400_BAD_REQUEST @@ -112,11 +110,8 @@ def test_scan_folder_is_not_an_existence_oracle( not_a_dir = tmp_path / "weights.ckpt" not_a_dir.write_text("not a directory") - responses = [ - client.get(f"/api/v2/models/scan_folder?scan_path={p}", headers=_auth(admin_token)) - for p in (tmp_path / "does_not_exist", not_a_dir) - ] - for response, p in zip(responses, (tmp_path / "does_not_exist", not_a_dir)): + for p in (tmp_path / "does_not_exist", not_a_dir): + response = client.get(f"/api/v2/models/scan_folder?scan_path={p}", headers=_auth(admin_token)) assert response.status_code == status.HTTP_400_BAD_REQUEST assert response.json()["detail"] == f"The search path '{p}' could not be scanned" From 6e01b4b98d5d8263724eb8d39f6963f49ad71b13 Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Tue, 21 Jul 2026 14:27:55 -0400 Subject: [PATCH 7/8] =?UTF-8?q?fix(api):=20address=20review=20round=202=20?= =?UTF-8?q?=E2=80=94=20auth=20on=20upload-entry=20stub,=20complete=20admin?= =?UTF-8?q?=20matrix?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - create_image_upload_entry (POST /api/v1/images/) now requires CurrentUserOrDefault. It was allowlisted as public only because it is a 501 stub, but a later implementation would have shipped unauthenticated without the route-audit test noticing. The allowlist entry is removed; unauthenticated requests get 401 and an authenticated request still reaches the stub (501, covered by a new test). - POST /api/v1/app/logging and GET /api/v1/app/invocation_cache/status added to the authorization matrices (both PROTECTED_ROUTES and ADMIN_ONLY_ROUTES for the former, ADMIN_ONLY_ROUTES for the latter), so demoting either dependency from AdminUserOrDefault would now fail a test. - openapi.json regenerated (HTTPBearer marker on the upload-entry op); schema.ts unchanged by typegen. Co-Authored-By: Claude Fable 5 --- invokeai/app/api/routers/images.py | 1 + invokeai/frontend/web/openapi.json | 5 +++++ .../test_model_manager_authorization.py | 19 +++++++++++++++++-- 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/invokeai/app/api/routers/images.py b/invokeai/app/api/routers/images.py index 8d282ac0bb2..e028a92cef9 100644 --- a/invokeai/app/api/routers/images.py +++ b/invokeai/app/api/routers/images.py @@ -182,6 +182,7 @@ class ImageUploadEntry(BaseModel): @images_router.post("/", operation_id="create_image_upload_entry") async def create_image_upload_entry( + _: CurrentUserOrDefault, width: int = Body(description="The width of the image"), height: int = Body(description="The height of the image"), board_id: Optional[str] = Body(default=None, description="The board to add this image to, if any"), diff --git a/invokeai/frontend/web/openapi.json b/invokeai/frontend/web/openapi.json index fa93a2fc0f6..22c6d43e0e3 100644 --- a/invokeai/frontend/web/openapi.json +++ b/invokeai/frontend/web/openapi.json @@ -4444,6 +4444,11 @@ "summary": "Create Image Upload Entry", "description": "Uploads an image from a URL, not implemented", "operationId": "create_image_upload_entry", + "security": [ + { + "HTTPBearer": [] + } + ], "requestBody": { "required": true, "content": { diff --git a/tests/app/routers/test_model_manager_authorization.py b/tests/app/routers/test_model_manager_authorization.py index fa65d6420d4..5ee03a161fd 100644 --- a/tests/app/routers/test_model_manager_authorization.py +++ b/tests/app/routers/test_model_manager_authorization.py @@ -44,10 +44,12 @@ ("get", "/api/v1/app/external_providers/status"), ("get", "/api/v1/app/external_providers/config"), ("get", "/api/v1/app/logging"), + ("post", "/api/v1/app/logging"), ("get", "/api/v1/app/invocation_cache/status"), ("delete", "/api/v1/app/invocation_cache"), ("put", "/api/v1/app/invocation_cache/enable"), ("put", "/api/v1/app/invocation_cache/disable"), + ("post", "/api/v1/images/"), ] # Routes that must additionally reject authenticated non-admin users. @@ -60,6 +62,8 @@ ("get", "/api/v1/app/external_providers/config"), ("get", "/api/v1/app/runtime_config"), ("get", "/api/v1/app/logging"), + ("post", "/api/v1/app/logging"), + ("get", "/api/v1/app/invocation_cache/status"), ("delete", "/api/v1/app/invocation_cache"), ("put", "/api/v1/app/invocation_cache/enable"), ("put", "/api/v1/app/invocation_cache/disable"), @@ -138,6 +142,19 @@ def search(self, path: Path) -> None: assert "permission denied" not in response.text +def test_create_image_upload_entry_requires_auth_before_the_501_stub( + enable_multiuser: Any, client: TestClient, user1_token: str +) -> None: + """The unimplemented upload-entry endpoint must authenticate first, so that implementing it later + cannot silently ship an unauthenticated state-mutating route (the 401 side is in PROTECTED_ROUTES).""" + response = client.post( + "/api/v1/images/", + headers=_auth(user1_token), + json={"width": 512, "height": 512}, + ) + assert response.status_code == status.HTTP_501_NOT_IMPLEMENTED + + # (method, path) pairs that are deliberately public. Everything else must carry an auth dependency. # # - auth bootstrap: reachable pre-login by construction @@ -145,13 +162,11 @@ def search(self, path: Path) -> None: # these needs a signed-URL or cookie scheme (tracked separately, see PR #9367) # - version: intentionally public # - docs/redoc: API documentation -# - POST /api/v1/images/ is a 501 stub PUBLIC_ROUTES = { ("GET", "/api/v1/app/version"), ("POST", "/api/v1/auth/login"), ("POST", "/api/v1/auth/setup"), ("GET", "/api/v1/auth/status"), - ("POST", "/api/v1/images/"), ("GET", "/api/v1/images/i/{image_name}/full"), ("HEAD", "/api/v1/images/i/{image_name}/full"), ("GET", "/api/v1/images/i/{image_name}/thumbnail"), From e6a062bc8ec0458b907f9fdc73f08f9bd542d9ff Mon Sep 17 00:00:00 2001 From: JPPhoto Date: Tue, 21 Jul 2026 14:56:55 -0500 Subject: [PATCH 8/8] fix(ui): clear API cache on logout --- .../frontend/web/src/app/store/store.test.ts | 27 +++++++++++++++++++ invokeai/frontend/web/src/app/store/store.ts | 19 +++++++++++-- 2 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 invokeai/frontend/web/src/app/store/store.test.ts diff --git a/invokeai/frontend/web/src/app/store/store.test.ts b/invokeai/frontend/web/src/app/store/store.test.ts new file mode 100644 index 00000000000..2ebd62b29ca --- /dev/null +++ b/invokeai/frontend/web/src/app/store/store.test.ts @@ -0,0 +1,27 @@ +import { logout, sessionExpiredLogout } from 'features/auth/store/authSlice'; +import { appInfoApi } from 'services/api/endpoints/appInfo'; +import type { S } from 'services/api/types'; +import { describe, expect, it } from 'vitest'; + +import { createStore } from './store'; + +const runtimeConfig = { + set_fields: ['models_dir'], + config: { models_dir: '/operator-only/models' }, +} as S['InvokeAIAppConfigWithSetFields']; + +describe('auth cache isolation', () => { + it.each([ + ['logout', logout], + ['session expiry', sessionExpiredLogout], + ])('clears API data on %s', async (_label, logOut) => { + const store = createStore(); + + await store.dispatch(appInfoApi.util.upsertQueryData('getRuntimeConfig', undefined, runtimeConfig)); + expect(appInfoApi.endpoints.getRuntimeConfig.select()(store.getState()).data).toEqual(runtimeConfig); + + store.dispatch(logOut()); + + expect(appInfoApi.endpoints.getRuntimeConfig.select()(store.getState()).data).toBeUndefined(); + }); +}); diff --git a/invokeai/frontend/web/src/app/store/store.ts b/invokeai/frontend/web/src/app/store/store.ts index f24d2d0105c..854eaba0f16 100644 --- a/invokeai/frontend/web/src/app/store/store.ts +++ b/invokeai/frontend/web/src/app/store/store.ts @@ -1,5 +1,12 @@ import type { ThunkDispatch, TypedStartListening, UnknownAction } from '@reduxjs/toolkit'; -import { addListener, combineReducers, configureStore, createAction, createListenerMiddleware } from '@reduxjs/toolkit'; +import { + addListener, + combineReducers, + configureStore, + createAction, + createListenerMiddleware, + isAnyOf, +} from '@reduxjs/toolkit'; import { logger } from 'app/logging/logger'; import { errorHandler } from 'app/store/enhancers/reduxRemember/errors'; import { addAdHocPostProcessingRequestedListener } from 'app/store/middleware/listenerMiddleware/listeners/addAdHocPostProcessingRequestedListener'; @@ -19,7 +26,7 @@ import { addSocketConnectedEventListener } from 'app/store/middleware/listenerMi import { deepClone } from 'common/util/deepClone'; import { merge } from 'es-toolkit'; import { omit, pick } from 'es-toolkit/compat'; -import { authSliceConfig } from 'features/auth/store/authSlice'; +import { authSliceConfig, logout, sessionExpiredLogout } from 'features/auth/store/authSlice'; import { changeBoardModalSliceConfig } from 'features/changeBoardModal/store/slice'; import { canvasSettingsSliceConfig } from 'features/controlLayers/store/canvasSettingsSlice'; import { canvasSliceConfig } from 'features/controlLayers/store/canvasSlice'; @@ -256,6 +263,14 @@ export const addAppListener = addListener.withTypes(); // To avoid circular dependencies, all listener middleware listeners are added here in the main store setup file. const startAppListening = listenerMiddleware.startListening as AppStartListening; + +startAppListening({ + matcher: isAnyOf(logout, sessionExpiredLogout), + effect: (_action, { dispatch }) => { + dispatch(api.util.resetApiState()); + }, +}); + addImageUploadedFulfilledListener(startAppListening); // Image deleted