From 6555118fb7be65fb048190d4ad9f63fa1044ad97 Mon Sep 17 00:00:00 2001 From: kri-agent Date: Wed, 24 Jun 2026 21:35:16 +0530 Subject: [PATCH 1/6] =?UTF-8?q?fix(api):=20unbreak=20master=20=E2=80=94=20?= =?UTF-8?q?rate-limit=20headers,=20import=20validate,=20agent=20reject?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four integration regressions from the #861/#862/#863 merge train left master red. Fixes: - Rate-limit headers (#801): #863 rewrote the limiter without re-emitting X-RateLimit-* headers. headers_enabled on the SlowAPI Limiter can't be used (the decorator then tries to inject into dict-returning endpoints and raises), and SlowAPIMiddleware exempts decorated routes. Add a pure-ASGI RateLimitHeadersMiddleware that injects the headers from scope-state view_rate_limit onto every HTTP response (incl. 401 login) and leaves WebSocket scopes untouched. - Import validate (#862): ImportRow carried a hard field_validator on minion_id, so the dry-run validate endpoint 500'd on intentionally-bad rows instead of reporting them. Move format checking into node_import.validate_row (soft, status="invalid") and re-check defensively on the commit path so a bad minion_id can still never be persisted. - Agent reject (#795): the endpoint correctly commits status="rejected" in its own session; the test's module-scoped db_session (expire_on_commit=False) was asserting against a stale identity-mapped instance. Refresh before asserting. Verified against a Postgres-backed fleet_test: the 4 previously-red tests plus their full files (32) and the related rate-limit/import unit tests (45) pass. --- VERSION | 2 +- fleet_platform/api/limiter.py | 52 +++++++++++++++++++++++++ fleet_platform/api/main.py | 7 +++- fleet_platform/api/routes/fleet.py | 7 +++- fleet_platform/schemas/node_import.py | 14 ++++--- fleet_platform/services/node_import.py | 9 +++++ frontend/package.json | 2 +- tests/integration/test_agent_run_795.py | 18 ++++----- 8 files changed, 92 insertions(+), 19 deletions(-) diff --git a/VERSION b/VERSION index 40a6940b..a9834dc0 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.1.562 +0.1.564 diff --git a/fleet_platform/api/limiter.py b/fleet_platform/api/limiter.py index 96a6a8a6..995f52f2 100644 --- a/fleet_platform/api/limiter.py +++ b/fleet_platform/api/limiter.py @@ -37,7 +37,9 @@ from typing import Callable from slowapi import Limiter +from starlette.datastructures import MutableHeaders from starlette.requests import Request +from starlette.types import ASGIApp, Message, Receive, Scope, Send def make_real_ip_key(proxy_count: int) -> Callable[[Request], str]: @@ -69,3 +71,53 @@ def _default_key(request: Request) -> str: limiter = Limiter(key_func=_default_key) + + +class RateLimitHeadersMiddleware: + """Emit ``X-RateLimit-*`` headers on every HTTP response for rate-limited routes. + + SlowAPI's own header injection is not usable here: + + * ``Limiter(headers_enabled=True)`` makes the ``@limiter.limit`` *decorator* + try to inject headers into the endpoint's return value. Our endpoints + return plain dicts (not ``Response`` objects) and don't declare a + ``response: Response`` parameter, so that path raises + "parameter `response` must be an instance of Response" — it would break + every decorated dict-returning route. + * ``SlowAPIMiddleware`` deliberately *exempts* any route that carries a + ``@limiter.limit`` decorator, so it never adds headers to e.g. ``/auth/login``. + + This is a pure-ASGI middleware (not ``BaseHTTPMiddleware``) so it leaves + WebSocket scopes completely untouched — wrapping WebSocket routes in + ``BaseHTTPMiddleware`` breaks their close-code handshake. For HTTP requests + we read ``view_rate_limit`` from the shared scope state (the decorator sets + it in ``_check_request_limit`` *before* the endpoint runs, so it is present + even when the endpoint raises, e.g. a 401 login) and append the standard + headers as the response starts, mirroring ``Limiter._inject_headers``. + """ + + def __init__(self, app: ASGIApp) -> None: + self.app = app + + async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: + if scope["type"] != "http": + await self.app(scope, receive, send) + return + + async def send_wrapper(message: Message) -> None: + if message["type"] == "http.response.start": + current_limit = (scope.get("state") or {}).get("view_rate_limit") + limiter_obj = getattr(getattr(scope.get("app"), "state", None), "limiter", None) + if current_limit is not None and limiter_obj is not None: + try: + item, args = current_limit + window_stats = limiter_obj.limiter.get_window_stats(item, *args) + headers = MutableHeaders(scope=message) + headers["X-RateLimit-Limit"] = str(item.amount) + headers["X-RateLimit-Remaining"] = str(window_stats[1]) + headers["X-RateLimit-Reset"] = str(1 + window_stats[0]) + except Exception: # never let header decoration break a response + pass + await send(message) + + await self.app(scope, receive, send_wrapper) diff --git a/fleet_platform/api/main.py b/fleet_platform/api/main.py index 89587409..eb80b1eb 100644 --- a/fleet_platform/api/main.py +++ b/fleet_platform/api/main.py @@ -9,7 +9,7 @@ from slowapi.errors import RateLimitExceeded from starlette.responses import Response -from fleet_platform.api.limiter import limiter +from fleet_platform.api.limiter import RateLimitHeadersMiddleware, limiter from fleet_platform.api.metrics_auth import verify_metrics_request from fleet_platform.api.metrics_collectors import refresh_all_gauges from fleet_platform.api.routes import ( @@ -164,6 +164,11 @@ async def structured_validation_exception_handler(request: Request, exc: Request app.add_middleware(SecurityHeaderMiddleware) + # Adds X-RateLimit-* headers to every response on rate-limited routes (see + # RateLimitHeadersMiddleware docstring for why SlowAPI's own injection can't + # be used with our dict-returning decorated endpoints). + app.add_middleware(RateLimitHeadersMiddleware) + app.include_router(health.router, tags=["health"]) app.include_router(auth.router, tags=["auth"]) app.include_router(nodes.router, tags=["nodes"]) diff --git a/fleet_platform/api/routes/fleet.py b/fleet_platform/api/routes/fleet.py index 472ca686..cc2eddfe 100644 --- a/fleet_platform/api/routes/fleet.py +++ b/fleet_platform/api/routes/fleet.py @@ -14,6 +14,7 @@ from fleet_platform.api.deps import get_db, get_redis from fleet_platform.core.audit import audit from fleet_platform.core.auth import get_current_user, hash_password, require_role +from fleet_platform.core.validators import MINION_ID_RE from fleet_platform.models.node import Node from fleet_platform.schemas.fleet import FleetOverviewResponse from fleet_platform.schemas.node_import import ( @@ -194,7 +195,11 @@ async def import_commit( _ssh_key = payload.ssh_key or None _auth_mode = payload.ssh_auth_mode or ("key" if (_ssh_key and not _ssh_pw) else "password") - new_rows = [r for r in payload.rows if r.status == "new"] + # Commit only acts on rows the validate step marked "new". Re-check the + # minion_id format defensively here too: a client could POST status="new" + # with a malformed minion_id directly, bypassing the validate endpoint. + # Such rows are skipped (never persisted) rather than 422-ing the whole batch. + new_rows = [r for r in payload.rows if r.status == "new" and MINION_ID_RE.match(r.minion_id or "")] skipped = len(payload.rows) - len(new_rows) created_ids: list[str] = [] created_nodes: list[Node] = [] diff --git a/fleet_platform/schemas/node_import.py b/fleet_platform/schemas/node_import.py index e7d9a390..1b46e83b 100644 --- a/fleet_platform/schemas/node_import.py +++ b/fleet_platform/schemas/node_import.py @@ -1,12 +1,18 @@ # fleet_platform/schemas/node_import.py """Pydantic schemas for bulk node import endpoints (#360).""" -from pydantic import BaseModel, field_validator - -from fleet_platform.core.validators import validate_minion_id +from pydantic import BaseModel class ImportRow(BaseModel): + # NOTE: minion_id is intentionally NOT hard-validated here. ImportRow is the + # transport for the dry-run *validate* response, which must be able to + # represent rows that FAILED validation (status="invalid") so the UI can + # show the user exactly which rows were rejected and why. Raising at parse + # time would 500 the validate endpoint instead of reporting invalid rows. + # Format enforcement happens in node_import.validate_row (soft, categorising) + # and again defensively on the commit path (see fleet.import_commit), so a + # bad minion_id can never be persisted. minion_id: str hostname: str | None = None ip: str | None = None @@ -15,8 +21,6 @@ class ImportRow(BaseModel): status: str = "new" reason: str = "" - _validate_minion_id = field_validator("minion_id")(validate_minion_id) - class ImportValidateRequest(BaseModel): source: str diff --git a/fleet_platform/services/node_import.py b/fleet_platform/services/node_import.py index 51c8f2ed..6ffbc671 100644 --- a/fleet_platform/services/node_import.py +++ b/fleet_platform/services/node_import.py @@ -5,6 +5,8 @@ import ipaddress import re +from fleet_platform.core.validators import MINION_ID_RE + _HOSTNAME_RE = re.compile(r"^[a-zA-Z0-9]([a-zA-Z0-9\-_.]{0,253}[a-zA-Z0-9])?$") @@ -77,6 +79,13 @@ def validate_row(row: dict, existing_minions: set, existing_ips: set) -> dict: if not mid: return {**out, "status": "invalid", "reason": "missing minion_id"} + if not MINION_ID_RE.match(mid): + return { + **out, + "status": "invalid", + "reason": "minion_id must be 1-128 chars, only [a-zA-Z0-9._-] allowed", + } + if ip: try: ipaddress.ip_address(ip) diff --git a/frontend/package.json b/frontend/package.json index 896bb483..d4c86efc 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "frontend", "private": true, - "version": "0.1.562", + "version": "0.1.564", "packageManager": "pnpm@10.29.3", "type": "module", "scripts": { diff --git a/tests/integration/test_agent_run_795.py b/tests/integration/test_agent_run_795.py index 0b621477..b6791ce2 100644 --- a/tests/integration/test_agent_run_795.py +++ b/tests/integration/test_agent_run_795.py @@ -267,10 +267,6 @@ async def test_list_agent_actions_returns_list(admin_client: AsyncClient): async def test_reject_agent_action_over_http(admin_client: AsyncClient, db_session: AsyncSession): """POST /agent/actions/{id}/reject transitions the action to 'rejected'.""" - from sqlalchemy import select - - from fleet_platform.models.pending_action import PendingAction - action = await _create_pending_action(db_session, requested_by="op@fleet.local") action_id = str(action.id) @@ -278,12 +274,14 @@ async def test_reject_agent_action_over_http(admin_client: AsyncClient, db_sessi assert resp.status_code == 200 assert resp.json().get("status") == "rejected" - # DB must reflect the rejected status - refreshed = ( - await db_session.execute(select(PendingAction).where(PendingAction.id == action.id)) - ).scalar_one_or_none() - assert refreshed is not None - assert refreshed.status == "rejected" + # DB must reflect the rejected status. The API ran the update in its own + # session; this test's db_session (expire_on_commit=False) still holds the + # original instance in its identity map, so refresh it to read the committed + # row rather than asserting against the stale cached object. refresh() is + # awaited, so it is safe in the async session (unlike touching an expired + # attribute, which would trigger a sync lazy-load). + await db_session.refresh(action) + assert action.status == "rejected" async def test_approve_agent_action_over_http(admin_client: AsyncClient, db_session: AsyncSession, admin_user): From 5eefb68d798082727af8faca5c216b3831680aab Mon Sep 17 00:00:00 2001 From: kri-agent Date: Wed, 24 Jun 2026 21:58:09 +0530 Subject: [PATCH 2/6] fix(test): stop integration fixtures rebinding the limiter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The behavioural rate-limit tests failed in the full suite (429 too early / "state shared across IP keys") while passing in isolation. Root cause: four integration app fixtures (conftest.app_with_test_db and the local fixtures in test_auth_endpoints, test_check_minion_id, test_node_import_api) did `limiter_module.limiter = Limiter(...)` before importing the routes. The @limiter.limit decorators bind to whichever limiter object exists at first route import and can't be rebound, so the throwaway test_limiter from the first such module became the decorators' limiter for the whole session — while the behavioural tests reset the *production* limiter and saw stale 429s. Fix: don't swap. Share the real production limiter and just zero its in-memory storage on setup/teardown so counts don't bleed across modules. This keeps app.state.limiter and the decorator limiter as the same object everywhere. Verified: full integration suite (excl. the Redis-only vnc/webssh ws tests that can't run in the sandbox) is 326 passed / 1 xfailed, including all of test_rate_limit_behavioral_801 in full-suite order. --- VERSION | 2 +- frontend/package.json | 2 +- tests/integration/conftest.py | 34 ++++++++++++++++------- tests/integration/test_auth_endpoints.py | 19 +++++++------ tests/integration/test_check_minion_id.py | 19 +++++++------ tests/integration/test_node_import_api.py | 19 +++++++------ 6 files changed, 59 insertions(+), 36 deletions(-) diff --git a/VERSION b/VERSION index a9834dc0..a6936d48 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.1.564 +0.1.565 diff --git a/frontend/package.json b/frontend/package.json index d4c86efc..f7f3a830 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "frontend", "private": true, - "version": "0.1.564", + "version": "0.1.565", "packageManager": "pnpm@10.29.3", "type": "module", "scripts": { diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index d746cf66..1521603b 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -48,20 +48,26 @@ async def db_session(test_engine): async def app_with_test_db(test_engine): from unittest.mock import AsyncMock, MagicMock - from slowapi import Limiter - from slowapi.util import get_remote_address - import fleet_platform.api.limiter as limiter_module - - # Use in-memory rate limiter for tests to avoid 429 false positives - test_limiter = Limiter(key_func=get_remote_address, storage_uri="memory://") - limiter_module.limiter = test_limiter - from fleet_platform.api import deps from fleet_platform.api.main import create_app + # Do NOT swap limiter_module.limiter for a throwaway instance here. The route + # @limiter.limit decorators bind to whichever limiter object the routes + # imported at first load; reassigning the module attribute afterwards cannot + # rebind them and only desynchronises app.state.limiter from the limiter the + # decorators actually use — which is exactly what broke the behavioural + # rate-limit tests (they reset limiter_module.limiter, but the decorators + # were firing on a stale per-module test_limiter). Instead, share the real + # production limiter and just zero its in-memory storage so hit counters from + # other modules don't bleed in and cause spurious 429s. + try: + limiter_module.limiter._storage.reset() + except Exception: + pass + app = create_app() - app.state.limiter = test_limiter + app.state.limiter = limiter_module.limiter TestSession = async_sessionmaker(test_engine, expire_on_commit=False) async def override_get_db(): @@ -86,7 +92,15 @@ async def override_get_redis(): app.dependency_overrides[deps.get_db] = override_get_db app.dependency_overrides[deps.get_redis] = override_get_redis app._test_mock_redis = mock_redis # expose for tests that need to configure it - return app + try: + yield app + finally: + # Clear any hits the route decorators accumulated on the shared limiter + # during this module so later modules start from a clean counter. + try: + limiter_module.limiter._storage.reset() + except Exception: + pass @pytest_asyncio.fixture(scope="module", loop_scope="module") diff --git a/tests/integration/test_auth_endpoints.py b/tests/integration/test_auth_endpoints.py index 1d64f0dd..a8a1ab3c 100644 --- a/tests/integration/test_auth_endpoints.py +++ b/tests/integration/test_auth_endpoints.py @@ -69,19 +69,22 @@ def auth_fake_redis() -> _FakeRedis: @pytest_asyncio.fixture(loop_scope="module") async def auth_client(test_engine, auth_fake_redis: _FakeRedis): - from slowapi import Limiter - from slowapi.util import get_remote_address - import fleet_platform.api.limiter as limiter_module - - test_limiter = Limiter(key_func=get_remote_address, storage_uri="memory://") - limiter_module.limiter = test_limiter - from fleet_platform.api import deps from fleet_platform.api.main import create_app + # Share the real production limiter (do not swap in a throwaway instance — + # the route decorators bind to the limiter present at first import and can't + # be rebound, which desynchronises app.state.limiter from the limiter the + # decorators use and breaks the behavioural rate-limit tests). Just zero its + # in-memory storage so counts from other modules don't cause spurious 429s. + try: + limiter_module.limiter._storage.reset() + except Exception: + pass + app = create_app() - app.state.limiter = test_limiter + app.state.limiter = limiter_module.limiter TestSession = async_sessionmaker(test_engine, expire_on_commit=False) async def override_get_db(): diff --git a/tests/integration/test_check_minion_id.py b/tests/integration/test_check_minion_id.py index 7f145718..9a38049a 100644 --- a/tests/integration/test_check_minion_id.py +++ b/tests/integration/test_check_minion_id.py @@ -37,19 +37,22 @@ async def db_session(test_engine): @pytest_asyncio.fixture(scope="module", loop_scope="module") async def app_with_test_db(test_engine): - from slowapi import Limiter - from slowapi.util import get_remote_address - import fleet_platform.api.limiter as limiter_module - - test_limiter = Limiter(key_func=get_remote_address, storage_uri="memory://") - limiter_module.limiter = test_limiter - from fleet_platform.api import deps from fleet_platform.api.main import create_app + # Share the real production limiter (do not swap in a throwaway instance — + # the route decorators bind to the limiter present at first import and can't + # be rebound, which desynchronises app.state.limiter from the limiter the + # decorators use and breaks the behavioural rate-limit tests). Just zero its + # in-memory storage so counts from other modules don't cause spurious 429s. + try: + limiter_module.limiter._storage.reset() + except Exception: + pass + app = create_app() - app.state.limiter = test_limiter + app.state.limiter = limiter_module.limiter TestSession = async_sessionmaker(test_engine, expire_on_commit=False) async def override_get_db(): diff --git a/tests/integration/test_node_import_api.py b/tests/integration/test_node_import_api.py index 0917ef7f..e39cfc93 100644 --- a/tests/integration/test_node_import_api.py +++ b/tests/integration/test_node_import_api.py @@ -47,19 +47,22 @@ async def db_session(test_engine): async def app_with_test_db(test_engine): from unittest.mock import AsyncMock - from slowapi import Limiter - from slowapi.util import get_remote_address - import fleet_platform.api.limiter as limiter_module - - test_limiter = Limiter(key_func=get_remote_address, storage_uri="memory://") - limiter_module.limiter = test_limiter - from fleet_platform.api import deps from fleet_platform.api.main import create_app + # Share the real production limiter (do not swap in a throwaway instance — + # the route decorators bind to the limiter present at first import and can't + # be rebound, which desynchronises app.state.limiter from the limiter the + # decorators use and breaks the behavioural rate-limit tests). Just zero its + # in-memory storage so counts from other modules don't cause spurious 429s. + try: + limiter_module.limiter._storage.reset() + except Exception: + pass + app = create_app() - app.state.limiter = test_limiter + app.state.limiter = limiter_module.limiter TestSession = async_sessionmaker(test_engine, expire_on_commit=False) async def override_get_db(): From ffd63ddd327622a3ed5f67c49515f52f41c1e718 Mon Sep 17 00:00:00 2001 From: kri-agent Date: Wed, 24 Jun 2026 22:09:02 +0530 Subject: [PATCH 3/6] ci: re-trigger checks Co-authored-by: Cursor From 185034f071b1fc1e6136c7f2a3025a058b0e394c Mon Sep 17 00:00:00 2001 From: kri-agent Date: Wed, 24 Jun 2026 22:29:36 +0530 Subject: [PATCH 4/6] test(unit): align stale assertions with local-tz dates and ImportRow transport Master went red after #877 (local-timezone date formatting) and the ImportRow soft-validation change merged together: - test_ist_timestamps / test_processes_tab_psutil_613 asserted hardcoded "Asia/Kolkata" / " IST" strings; now assert the dynamic getTimezone() + timeZoneName/formatIST contract introduced by #877 (#796). - test_validators_752 TestImportRow expected ValidationError on bad ids, but ImportRow is the validate dry-run transport and must carry invalid rows; assert it accepts the string while MINION_ID_RE rejects it. --- VERSION | 2 +- frontend/package.json | 2 +- tests/unit/test_ist_timestamps.py | 11 +++++++--- tests/unit/test_processes_tab_psutil_613.py | 9 ++++++-- tests/unit/test_validators_752.py | 24 +++++++++++++++------ 5 files changed, 34 insertions(+), 14 deletions(-) diff --git a/VERSION b/VERSION index cf6ee5b2..a6936d48 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.1.563 +0.1.565 diff --git a/frontend/package.json b/frontend/package.json index 680782ef..f7f3a830 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "frontend", "private": true, - "version": "0.1.563", + "version": "0.1.565", "packageManager": "pnpm@10.29.3", "type": "module", "scripts": { diff --git a/tests/unit/test_ist_timestamps.py b/tests/unit/test_ist_timestamps.py index d6a66c6c..6c763e86 100644 --- a/tests/unit/test_ist_timestamps.py +++ b/tests/unit/test_ist_timestamps.py @@ -10,8 +10,11 @@ def test_ist_utils_file_exists(): def test_formatist_is_exported(): content = open("frontend/src/utils/time.ts").read() assert "export function formatIST" in content - assert "Asia/Kolkata" in content - assert "IST" in content + # #796/#877: timestamps now render in the viewer's local timezone via + # getTimezone() (with timeZoneName for the suffix) rather than a hardcoded + # Asia/Kolkata zone. + assert "getTimezone()" in content + assert "timeZoneName" in content def test_node_detail_uses_formatist(): @@ -26,4 +29,6 @@ def test_node_detail_uses_formatist(): def test_ist_suffix_in_utility(): content = open("frontend/src/utils/time.ts").read() - assert "+ ' IST'" in content or "IST'" in content + # The timezone suffix is now derived dynamically from the viewer's locale + # (timeZoneName: 'short') instead of a hardcoded ' IST' string (#796/#877). + assert "timeZoneName" in content diff --git a/tests/unit/test_processes_tab_psutil_613.py b/tests/unit/test_processes_tab_psutil_613.py index 273be41f..1a5b6b65 100644 --- a/tests/unit/test_processes_tab_psutil_613.py +++ b/tests/unit/test_processes_tab_psutil_613.py @@ -60,11 +60,16 @@ def test_nodedetal_uses_is_llm(): def test_nodedetal_uses_ist_timezone(): - assert "Asia/Kolkata" in NODE_DETAIL_TSX, "NodeDetail.tsx must use Asia/Kolkata timezone for IST display" + # #796/#877: NodeDetail no longer hardcodes a timezone. Date rendering (and + # the timezone suffix) is delegated to the shared formatIST/formatISTDate + # helper, which formats in the viewer's local timezone. + assert "formatIST" in NODE_DETAIL_TSX, "NodeDetail.tsx must render dates via the formatIST helper" def test_nodedetal_has_ist_suffix(): - assert " IST" in NODE_DETAIL_TSX, "NodeDetail.tsx must display ' IST' suffix on timestamps" + # The timezone suffix is now produced by the formatIST helper rather than a + # literal ' IST' string embedded in the page (#796/#877). + assert "formatIST" in NODE_DETAIL_TSX, "NodeDetail.tsx must use formatIST so timestamps carry a timezone suffix" # ── NodeDetail.tsx — old data source REMOVED ────────────────────────────── diff --git a/tests/unit/test_validators_752.py b/tests/unit/test_validators_752.py index 5abae7f9..509dec85 100644 --- a/tests/unit/test_validators_752.py +++ b/tests/unit/test_validators_752.py @@ -153,10 +153,20 @@ def test_accepts_valid(self): r = ImportRow(minion_id="build-01.corp") assert r.minion_id == "build-01.corp" - def test_rejects_too_long(self): - with pytest.raises(ValidationError): - ImportRow(minion_id="b" * 129) - - def test_rejects_special_chars(self): - with pytest.raises(ValidationError): - ImportRow(minion_id="foo bar") + # NOTE: ImportRow deliberately does NOT hard-validate minion_id (unlike the + # other request schemas above). It is the transport for the bulk-import + # *validate* dry-run response, which must be able to carry rows that FAILED + # validation (status="invalid") so the UI can show which rows were rejected + # and why. Raising at parse time would 500 the validate endpoint. Format + # enforcement instead happens softly in node_import.validate_row and again + # defensively on the commit path (fleet.import_commit). So a malformed id is + # accepted by the model but categorised/skipped before it can be persisted. + def test_accepts_too_long_as_transport(self): + r = ImportRow(minion_id="b" * 129) + assert r.minion_id == "b" * 129 + assert MINION_ID_RE.match(r.minion_id) is None + + def test_accepts_special_chars_as_transport(self): + r = ImportRow(minion_id="foo bar") + assert r.minion_id == "foo bar" + assert MINION_ID_RE.match(r.minion_id) is None From 3ce14a69d52bfbfa5ca35b2d6b2c477f8cf53113 Mon Sep 17 00:00:00 2001 From: kri-agent Date: Wed, 24 Jun 2026 22:40:16 +0530 Subject: [PATCH 5/6] ci(mutation): pin mutmut<3.6.0 to fix crashing results step mutmut 3.6.0 renamed the [tool.mutmut] config keys and shipped a config-loader regression that crashes `mutmut results` with "can only concatenate list (not 'str') to list". Since the mutation.yml "Print mutation results" step has no continue-on-error, this failed the check on every PR touching validator/redaction tests (e.g. this branch's test_validators_752.py edit). Pin <3.6.0 so the existing string-valued config keeps working. --- VERSION | 2 +- frontend/package.json | 2 +- pyproject.toml | 7 ++++++- uv.lock | 2 +- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/VERSION b/VERSION index a6936d48..c5ce918d 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.1.565 +0.1.566 diff --git a/frontend/package.json b/frontend/package.json index f7f3a830..4e512aaa 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "frontend", "private": true, - "version": "0.1.565", + "version": "0.1.566", "packageManager": "pnpm@10.29.3", "type": "module", "scripts": { diff --git a/pyproject.toml b/pyproject.toml index 032bc747..f90de7a1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -61,7 +61,12 @@ dev = [ "bandit[toml]>=1.9.4", "pre-commit>=4.6.0", "hypothesis>=6.155.2", - "mutmut>=3.1.0", + # Pin below 3.6.0: that release renamed the [tool.mutmut] keys + # (paths_to_mutate->source_paths, tests_dir folded into + # pytest_add_cli_args_test_selection) AND shipped a config-loader bug that + # crashes `mutmut results` with "can only concatenate list (not 'str') to + # list", failing the mutation CI job. 3.5.0 reads our string-valued config. + "mutmut>=3.1.0,<3.6.0", ] # Only package fleet_platform — exclude playbooks/ and other top-level dirs. diff --git a/uv.lock b/uv.lock index 83fc9883..47ca4754 100644 --- a/uv.lock +++ b/uv.lock @@ -788,7 +788,7 @@ requires-dist = [ { name = "httpx", specifier = ">=0.28.1" }, { name = "hypothesis", marker = "extra == 'dev'", specifier = ">=6.155.2" }, { name = "idna", specifier = ">=3.17" }, - { name = "mutmut", marker = "extra == 'dev'", specifier = ">=3.1.0" }, + { name = "mutmut", marker = "extra == 'dev'", specifier = ">=3.1.0,<3.6.0" }, { name = "mypy", marker = "extra == 'dev'", specifier = ">=2.1.0" }, { name = "opentelemetry-exporter-otlp-proto-grpc", specifier = ">=1.27.0" }, { name = "opentelemetry-instrumentation-celery", specifier = ">=0.48b0" }, From b190d85bcabcc19bcacba328712484ffa24d9afe Mon Sep 17 00:00:00 2001 From: kri-agent Date: Wed, 24 Jun 2026 22:53:02 +0530 Subject: [PATCH 6/6] test(unit): ConfirmDialog Escape handling moved to useFocusTrap (#878) #878 extracted Escape-to-close + focus trapping into the shared useFocusTrap hook, so ConfirmDialog.tsx no longer contains the literal "Escape". Assert it delegates to useFocusTrap and that the hook handles Escape, instead of grepping the component for the moved string. --- VERSION | 2 +- frontend/package.json | 2 +- tests/unit/test_ux_confirm_dialogs_b14.py | 8 +++++++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/VERSION b/VERSION index 0cb7abb8..9e85fcc9 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.1.567 +0.1.568 diff --git a/frontend/package.json b/frontend/package.json index 4f9609fb..5ed8e612 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "frontend", "private": true, - "version": "0.1.567", + "version": "0.1.568", "packageManager": "pnpm@10.29.3", "type": "module", "scripts": { diff --git a/tests/unit/test_ux_confirm_dialogs_b14.py b/tests/unit/test_ux_confirm_dialogs_b14.py index 480a2f07..0618aaea 100644 --- a/tests/unit/test_ux_confirm_dialogs_b14.py +++ b/tests/unit/test_ux_confirm_dialogs_b14.py @@ -34,4 +34,10 @@ def test_bulk_salt_state_uses_confirm_dialog(): def test_confirm_dialog_component_exists(): assert 'role="dialog"' in CONFIRM_COMPONENT, "ConfirmDialog must have role=dialog" assert "aria-modal" in CONFIRM_COMPONENT, "ConfirmDialog must have aria-modal" - assert "Escape" in CONFIRM_COMPONENT, "ConfirmDialog must handle Escape key" + # #878 extracted Escape-to-close (and focus trapping) into the shared + # useFocusTrap hook, so the literal "Escape" handler no longer lives in + # ConfirmDialog.tsx. Assert it delegates to that hook, which invokes the + # close/cancel handler on Escape (see frontend/src/hooks/useFocusTrap.ts). + HOOK = Path("frontend/src/hooks/useFocusTrap.ts").read_text() + assert "useFocusTrap" in CONFIRM_COMPONENT, "ConfirmDialog must use the useFocusTrap hook" + assert "Escape" in HOOK, "useFocusTrap must handle the Escape key"