fix(ui): move pagination data into script tag#3006
fix(ui): move pagination data into script tag#3006gcgoncalves wants to merge 2218 commits intomainfrom
Conversation
* feat: optimize CPU usage in request logging middleware Add configuration options to reduce CPU and database overhead in detailed request logging: - log_detailed_skip_endpoints: List of path prefixes to skip from detailed logging (e.g., high-volume or low-value endpoints) - log_resolve_user_identity: Gate DB fallback for user identity resolution behind opt-in flag (default: false) - log_detailed_sample_rate: Sampling rate (0.0-1.0) to log only a fraction of requests when detailed logging is enabled These optimizations avoid expensive JSON parsing, masking, and identity lookups unless detailed logging is explicitly enabled and required. Closes #1865 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * docs: add documentation for logging CPU optimization options Document the new logging configuration options: - LOG_DETAILED_SKIP_ENDPOINTS: path prefixes to skip from logging - LOG_DETAILED_SAMPLE_RATE: sampling rate for detailed logging - LOG_RESOLVE_USER_IDENTITY: opt-in DB lookup for user identity Updated: - .env.example with new options and descriptions - README.md logging table and examples - Helm chart values.yaml and values.schema.json - charts/mcp-stack/README.md values table - docs/config.schema.json (regenerated from Pydantic model) - docs/docs/config.schema.json (regenerated from Pydantic model) Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: add LOG_DETAILED_SKIP_ENDPOINTS to env list normalizer and add tests - Add LOG_DETAILED_SKIP_ENDPOINTS to _normalize_env_list_vars() to support CSV format and empty string values from environment variables - Add unit tests for skip endpoints, sampling rate, and user identity resolution gating in request logging middleware - Add settings field validation tests for new config options Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * doctest coverage Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * chore: lower doctest coverage threshold to 34% Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
…patibility (#2342) - Use jsonschema.validators.validator_for to detect schema draft automatically - Support multiple JSON Schema drafts (Draft 4, 6, 7, 2019-09, 2020-12) - Log warnings for unsupported drafts or invalid schemas instead of raising errors - Handle None schemas gracefully - Apply consistent validation behavior to both tool and prompt schemas - Add comprehensive tests for different schema drafts - Add fallback validator logic in tool_service.py for runtime validation - Disable MCP SDK's built-in input validation which uses strict Draft 2020-12 Closes #2322 Signed-off-by: Keval Mahajan <mahajankeval23@gmail.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
* fix(db): Guard against inactive transaction during async cleanup When registering MCP servers with long initialization times (like Moody's), a CancelledError can occur during the MCP session teardown (DELETE request). This causes the database transaction to become inactive before get_db() attempts to commit, resulting in: sqlalchemy.exc.InvalidRequestError: This transaction is inactive Add db.is_active checks before commit() and rollback() to handle cases where the transaction becomes inactive during async context manager cleanup. Closes #2341 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: Add upsert logic for resources and prompts to prevent unique constraint violations When re-registering a gateway (e.g., after deletion or crash), orphaned resources and prompts from previous registrations could cause unique constraint violations on `(team_id, owner_email, uri)` for resources and `(team_id, owner_email, name)` for prompts. This fix adds upsert logic that: 1. Queries for existing resources/prompts matching the unique constraint 2. Updates existing records instead of creating duplicates 3. Creates new records only when no match exists This handles scenarios like: - Gateway deletion that didn't properly clean up resources (issue #2341) - Re-registration of the same MCP server under a new gateway name - Race conditions during concurrent operations Closes #2352 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * chore: Add cleanup script for orphaned resources/prompts/tools Adds a utility script to identify and remove database records that were left orphaned due to incomplete gateway deletions (e.g., #2341 crash). Usage: # Dry run (default) - shows what would be deleted python scripts/cleanup_orphaned_resources.py # Actually delete orphaned records python scripts/cleanup_orphaned_resources.py --execute # Filter by team or owner python scripts/cleanup_orphaned_resources.py --team-id <id> --owner-email <email> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: Only upsert orphaned resources/prompts, add tests Addresses code review findings: 1. HIGH: Only update truly orphaned records (gateway_id IS NULL or points to non-existent gateway). Resources belonging to active gateways are no longer at risk of being reassigned. 2. MEDIUM: Use per-resource team/owner overrides when building lookup key, matching exactly what would be inserted to avoid constraint mismatches. 3. LOW: Added tests for orphaned resource upsert logic: - test_register_gateway_updates_orphaned_resources - test_register_gateway_does_not_update_active_gateway_resources Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: Always call rollback() in exception handler, improve tests Addresses remaining code review findings: 1. ROLLBACK GUARD FIX: - REMOVED the `if db.is_active:` check before `db.rollback()` - Empirical testing proved that: * After IntegrityError, is_active becomes False * rollback() when is_active=False SUCCEEDS (doesn't fail!) * rollback() restores is_active to True, cleaning up the session * Skipping rollback when is_active=False leaves session unusable - The is_active guard for commit() is CORRECT (commit fails when False) - The is_active guard for rollback() was WRONG (rollback is always safe) 2. TEST ASSERTIONS: - Rewrote orphaned resource tests with proper assertions - Tests now directly verify: * Orphaned resources are detected and added to map * Resource fields are actually updated during upsert * Resources with deleted gateways are detected as orphaned * Resources with active gateways are NOT touched * Per-resource owner/team overrides are used in lookup key Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * chore: Add file encoding header to cleanup script Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Signed-off-by: RinCodeForge927 <dangnhatrin90@gmail.com> Signed-off-by: RinZ27 <222222878+RinZ27@users.noreply.github.com>
Signed-off-by: RinCodeForge927 <dangnhatrin90@gmail.com> Signed-off-by: RinZ27 <222222878+RinZ27@users.noreply.github.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
…ps (#2359) * fix(perf): resolve lock contention and CPU spin loop under high load (#2355) Phase 1: Replace cascading FOR UPDATE loops with bulk UPDATE statements in gateway_service.set_gateway_state() to eliminate lock contention when activating/deactivating gateways with many tools/servers/prompts/resources. Phase 2: Add nowait=True to get_for_update() calls in set_server_state() and set_tool_state() to fail fast on locked rows instead of blocking. Add ServerLockConflictError and ToolLockConflictError exceptions with 409 Conflict handlers in main.py and admin.py routers. Phase 3: Fix CPU spin loop in SSE transport by properly detecting client disconnection. Add request.is_disconnected() check, consecutive error counting, GeneratorExit handling, and ensure _client_gone is set in all exit paths. Results: - RPS improved from 173-600 to ~2000 under load - Failure rate reduced from 14-22% to 0.03-0.04% - Blocked queries reduced from 33-48 to 0 - CPU after load test: ~1% (was 800%+ spin loop) Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(perf): add database lock timeout configuration (#2355) Add configurable timeout settings for database operations: - db_lock_timeout_ms: Maximum wait for row locks (default 5000ms) - db_statement_timeout_ms: Maximum statement execution (default 30000ms) These settings can be used with get_for_update() to prevent indefinite blocking under high concurrency scenarios. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * test: update tests for bulk UPDATE and SSE transport changes (#2355) Update gateway_service tests to use side_effect for multiple db.execute calls (SELECT + bulk UPDATEs) instead of single return_value. Update row_level_locking test to expect nowait=True parameter in get_for_update calls for set_tool_state. Update SSE transport tests to mock request.is_disconnected() and adjust error handling test to expect consecutive errors causing generator stop instead of error event emission. Add missing exception documentation for ServerLockConflictError and ToolLockConflictError in service docstrings (flake8 DAR401). Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(sse): add send_timeout to EventSourceResponse to prevent spin loops (#2355) When Granian ASGI server fails to send to a disconnected client, it logs "ASGI transport error: SendError" but doesn't raise an exception to our code. This causes rapid iteration of the generator without proper timeout handling. Add send_timeout=5.0 to EventSourceResponse to ensure sends time out if they fail, triggering sse_starlette's built-in error handling. Also enable sse_starlette's built-in ping mechanism when keepalive is enabled, which provides additional disconnect detection. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(sse): add rapid yield detection to prevent CPU spin loops (#2355) When clients disconnect abruptly, Granian may fail sends without raising Python exceptions. This adds rapid yield detection: if 50+ yields occur within 1 second, we assume client is disconnected and stop the generator. New configurable settings: - SSE_SEND_TIMEOUT: ASGI send timeout (default 30s) - SSE_RAPID_YIELD_WINDOW_MS: detection window (default 1000ms) - SSE_RAPID_YIELD_MAX: max yields before disconnect (default 50) Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(sse): log rapid yield detection at ERROR level for visibility Changed from WARNING to ERROR so the detection message is visible even when LOG_LEVEL=ERROR. This is appropriate since rapid yield detection indicates a problem condition (client disconnect not reported by ASGI). Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: address review findings from ChatGPT analysis Finding 1: Fix lock conflict error propagation - Add explicit except handlers for ToolLockConflictError and ServerLockConflictError before the generic Exception handler - This allows 409 responses to propagate correctly instead of being wrapped as generic 400 errors Finding 3: Improve SSE rapid yield detection - Only track message yields, not keepalives - Reset the timestamp deque when timeout occurs (we actually waited) - This prevents false positives on high-throughput legitimate streams Finding 4: Remove unused db timeout settings - Remove db_lock_timeout_ms and db_statement_timeout_ms from config - These settings were defined but never wired into DB operations - Avoids false sense of protection Finding 2 (notifications) is intentional: gateway-level notifications are sent, and bulk UPDATE is used for performance under high load. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(perf): add nowait locks to prompt and resource state changes Extends the lock contention fix to prompt_service and resource_service: - Add PromptLockConflictError and ResourceLockConflictError classes - Use nowait=True in get_for_update to fail fast if row is locked - Add 409 Conflict handlers in main.py for both services - Re-raise specific errors before generic Exception handler This ensures consistent lock handling across all state change endpoints. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(sse): improve rapid yield detection to catch all spin scenarios - Track time since last yield as additional signal (<10ms is suspicious) - Check rapid yield after BOTH message and keepalive yields - Reset timestamps only after successful keepalive wait - Include time interval in error log for debugging Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(sse): add robust spin loop detection and update dependencies (#2355) SSE transport improvements: - Add consecutive rapid yield counter for simpler spin loop detection (triggers after 10 yields < 100ms apart) - Remove deque clearing after keepalives that prevented detection - Add client_close_handler_callable to detect disconnects that ASGI servers like granian may not propagate via request.is_disconnected() Test updates: - Update row-level locking tests to expect nowait=True for prompt and resource state changes Dependency updates: - Containerfile.lite: Update UBI base images to latest - gunicorn 23.0.0 -> 24.1.1 - sqlalchemy 2.0.45 -> 2.0.46 - langgraph 1.0.6 -> 1.0.7 - hypothesis 6.150.2 -> 6.150.3 - schemathesis 4.9.2 -> 4.9.4 - copier 9.11.1 -> 9.11.3 - pytest-html 4.1.1 -> 4.2.0 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * docs: add granian worker lifecycle options for SSE connection leak workaround Document GRANIAN_WORKERS_LIFETIME and GRANIAN_WORKERS_MAX_RSS options as commented-out configuration in docker-compose.yml and run-granian.sh. These options provide a workaround for granian issue #286 where SSE connections are not properly closed after client disconnect, causing CPU spin loops after load tests complete. Refs: #2357, #2358 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * docker-compose updates for GUNICORN Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * docker-compose updates for GUNICORN Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * docker-compose updates for GUNICORN Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * Update pyproject.toml Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * lint Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
* fix: add sso_entra_admin_groups to list field validator - sso_entra_admin_groups now properly parses CSV/JSON from environment - Closes #2265 Signed-off-by: Akshay Shinde <akshayshinde@dhcp-9-162-244-59.mul.ie.ibm.com> * style: fix missing blank lines in test_config.py Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Akshay Shinde <akshayshinde@dhcp-9-162-244-59.mul.ie.ibm.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Akshay Shinde <akshayshinde@dhcp-9-162-244-59.mul.ie.ibm.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
…scanners (#2200) - Convert cache.py to use async redis (redis.asyncio) for non-blocking I/O - Add parallel scanner execution using asyncio.gather in input/output filters - Add asyncio.to_thread for CPU-bound scanner operations - Quiet llm_guard logger to ERROR level to reduce noise - Fix tests to use prompt_id instead of deprecated name parameter - Update test to use environment variables for redis host/port Security: Scanner errors now fail-closed (is_valid=False) instead of being skipped, ensuring policy evaluation denies requests when scanners fail. Closes #1959 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
1. Replace custom CSS classes with native tailwind utility classes. 2. Add Chart.js theming for dark-mode graphs Signed-off-by: Gabriel Costa <gabrielcg@proton.me> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
* fix: plugin template test cases. Signed-off-by: Teryl Taylor <terylt@ibm.com> * fix: context passing in unit test Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> --------- Signed-off-by: Teryl Taylor <terylt@ibm.com> Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> Co-authored-by: Teryl Taylor <terylt@ibm.com> Co-authored-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Precompile all regex patterns at module or configuration initialization time across 14 plugins, eliminating per-request compilation overhead. Closes #1834 Signed-off-by: Shoumi <shoumimukherjee@gmail.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
* update jwt cli with more inputs Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> * fix: prevent non-expiring tokens from invalid expires_in_days - Add ge=1 validation to TokenCreateRequest.expires_in_days schema - Add guard in _generate_token to reject expires_at in the past - Use math.ceil() and max(1, ...) to ensure exp is always set for sub-minute expirations (prevents rounding to 0) - Mark --secret and --algo CLI args as deprecated (always uses config) - Add tests for past expiry rejection and ceiling behavior This fixes a security regression where negative/zero expires_in_days could create permanent tokens instead of expired ones. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: restore --secret and --algo CLI options The --secret and --algo CLI parameters now work as optional overrides: - When provided, they override the configuration values - When not provided, JWT_SECRET_KEY and JWT_ALGORITHM from config are used This preserves backward compatibility while still defaulting to configuration-based signing for consistency. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: require --secret when --algo is specified Prevent invalid token generation by requiring --secret when --algo is provided. Using --algo alone would mix config-based keys with a different algorithm, potentially producing tokens that fail validation. Also fixes stale docstring that still referenced DEFAULT_SECRET/DEFAULT_ALGO instead of the new empty-string defaults with config fallback. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Madhav Kandukuri <madhav165@gmail.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
* Initial commit for filesystem server
- added stdio simple server
- implemented list_directory tool
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Improved main runner
- Added argument handler
- improve tracing
- implemented streamable-http
Signed-off-by: cafalchio <maolivei@tcd.ie>
* added tracing info for each call
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Implemented search files recursively
- use glob patterns
- Added search to tools
- Handle errors
- Improve descriptions
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added files for new functions
- Updated cargo file for file search
Signed-off-by: cafalchio <maolivei@tcd.ie>
* implemented case insensitive search: Lowercase filenames and patterns
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Implemented read_file function and added to a server
- check for Max file size 1Mb
- check if the path is a file
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added tracing for read file, improved error description
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added get_file_info
- get size, created, modified and permissions
- Added to the server
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added Read multiple files
- use read file for each content
- read async
- added to server tools
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added write_file toold func
- Write to a tempfile uuid
- rename file to actual name
- remove tempfile
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added create_directory tool
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added create_directory to server and implemented placeholder for list_allowed_directories
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Changed release config to reduce bin size
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Implemented move file function.
- fails if destination exists
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added move_file to the server
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added edit file to the server
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added edit_file
- support dry_run
- use similar to get diffs
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Adding sandbox for path ccheck
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Apply fix to reduce TOCTOU vulnerability
- atomic write, no checks and write
Signed-off-by: cafalchio <maolivei@tcd.ie>
* improve read to reduce TOCTOU vulnerability
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Stopped to follow symlink on search (security)
Signed-off-by: cafalchio <maolivei@tcd.ie>
* removed unused import
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Improved Sandbox for TOCTOU safety
- initialize sandbox once
- check if new folders are inside root
- resolve path inside root
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added get_roots for server list_allowed_directories
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Using sanbox resolve path before ger file info
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added sandox to write_file and create_directory
- validade parent folder
- clean tempfile after
- check new folder inside root
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added sandbox to write file
- canonicalize and check new folders
- check parent folders
- on create directory, check if exists
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added sandbox check for list_directory
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added sandbox check for edit_file and move_file
- check and canonicalize destination parent
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Apply sandbox checks for read_file and read_multiple_files
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Changed sandbox initialization from global to context
- removed global sandbox
- initialize sandbox in main and pass to each function
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added tests for searc_files and list_directories
- test for symlinks
- test for path outside roots
- > 95% coverage
- formatted using fmt
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Formatted files using cargo fmt
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added tests for get_file_info
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added test coverage for read_file and read_multiple_files
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added unit test coverage for write_file and create_directory
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Improve test coverage for edit_file and move_file
Signed-off-by: cafalchio <maolivei@tcd.ie>
* format edit.rs
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added test coverage for sandbox
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Improve server runner
- addded sever gracefully shutdown
- Declare Config values in main
- Improve server logs
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Improved logs for list_allowed_directories and linted file
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Improve test coverage for server
- iimproved logs in server
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Linted using cargo clippy
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Simplified main.rs and moved server code to lib.rs for integration tests
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added integration tests to simulate workflows
- file and folder manipulation workflow
- permission and metadata workflow
- search and organise workflow
- server tests
Signed-off-by: cafalchio <maolivei@tcd.ie>
* formatted and clippped integration tests
Signed-off-by: cafalchio <maolivei@tcd.ie>
* added result where it was missing in tool call result
- rename all outputs to result
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Normalized write file and create directory output
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Structured search and write outputs
- update tests
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Fixed write result not showing errors correctly
- server will return WriteResult
- create_directory return err or string
- fixed test create_directory tests
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Normalized output result for write_file
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Normalized tool result outpus
- Keep consistency between tools
- Return MPC error or success
- reorganised tools between files
- updated tests
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added server start banner
Signed-off-by: cafalchio <maolivei@tcd.ie>
* fixed main receiving multiple roots
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Clipped and formatted
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Removed justfile and added Makefile
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added correct readme
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Added dockerfile
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Improved tracing logs
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Update test coverage and binary size in readme
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Removed umused dependency
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Updated cargo dependencies
Signed-off-by: cafalchio <maolivei@tcd.ie>
* Updted deprecated InitializedRequestParam
Signed-off-by: cafalchio <maolivei@tcd.ie>
* fix: correct error messages and documentation in filesystem server
- Fix misleading error messages in server.rs that said "Error writing file"
when the actual operation was read, move, or get_file_info
- Update README to accurately reflect that move_file overwrites destination
(previously incorrectly stated it fails)
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
---------
Signed-off-by: cafalchio <maolivei@tcd.ie>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
…ts (#2345) * Fix proxy authentication Signed-off-by: Mohan Lakshmaiah <mohalaks@in.ibm.com> * Fix pylint errors Signed-off-by: Mohan Lakshmaiah <mohalaks@in.ibm.com> * fix: Correct lint issues in proxy auth tests - Add missing blank line between test classes - Remove unused jwt import - Fix excess blank lines Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: Include plugin context in proxy auth for cross-hook sharing Add plugin_context_table and plugin_global_context to proxy authentication paths, matching the JWT authentication path. This ensures HTTP_AUTH_CHECK_PERMISSION hooks can access context set by HTTP_PRE_REQUEST hooks when using proxy authentication. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: Address security concerns in proxy authentication 1. RBAC now checks auth_required when proxy header missing - Returns 401 for API requests, 302 redirect for browsers - Aligns HTTP behavior with WebSocket auth 2. Block anonymous users from token management - Add auth_method=="anonymous" to _require_interactive_session - Prevents token access when proxy header missing 3. Lookup proxy user admin status from database - Check platform_admin_email for admin match - Query EmailUser table for is_admin status - Enables plugin permission hooks to work correctly Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: Align require_auth with RBAC proxy enforcement Update require_auth to check auth_required when proxy header is missing, matching the RBAC/WebSocket behavior. Previously returned "anonymous" even when auth_required=true. - Raise 401 when mcp_client_auth_enabled=false and no proxy header if auth_required=true - Update tests to cover both auth_required=true and false cases Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Mohan Lakshmaiah <mohalaks@in.ibm.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Mohan Lakshmaiah <mohalaks@in.ibm.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
Signed-off-by: Madhav Kandukuri <madhav165@gmail.com>
* fix: FastMCP compatibility
* fix: normalize issuer URL for metadata validation and caching
The original trailing slash fix introduced a bug where the issuer
validation would fail when the server returned an issuer without
trailing slash but the client passed one (or vice versa).
Changes:
- Normalize both the input issuer and metadata issuer for comparison
- Use normalized issuer as cache key for consistent cache lookup
- Add tests for trailing slash normalization scenarios
- Update test to expect refresh_token in grant_types
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
* fix: complete issuer normalization and conditional refresh_token
Address review feedback:
1. Normalize issuer consistently across the entire DCR flow:
- Allowlist validation uses normalized comparison
- Storage uses normalized issuer
- Lookup uses normalized issuer
2. Make refresh_token conditional on AS support:
- Check grant_types_supported in AS metadata
- Only request refresh_token if AS advertises support
3. Fix grant_types fallback:
- Use requested grant_types as fallback when AS response omits them
- Previously hardcoded to ["authorization_code"] which dropped refresh_token
4. Add comprehensive tests:
- Test refresh_token inclusion when AS supports it
- Test grant_types fallback behavior
- Test allowlist trailing slash normalization
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
* fix: handle null grant_types_supported and add issuer normalization migration
Address additional review findings:
1. Fix TypeError when grant_types_supported is explicit null:
- Use `metadata.get("grant_types_supported") or []` instead of
`metadata.get("grant_types_supported", [])`
- The latter returns None when key exists with null value
2. Add configurable permissive refresh_token mode:
- New setting: dcr_request_refresh_token_when_unsupported
- Default: False (strict mode - only request if AS advertises support)
- When True: request refresh_token if AS omits grant_types_supported
3. Add Alembic migration to normalize legacy issuer values:
- Strips trailing slashes from registered_oauth_clients.issuer
- Idempotent and works with SQLite and PostgreSQL
- Prevents duplicate registrations from legacy rows
4. Add comprehensive tests:
- Test explicit null grant_types_supported handling
- Test permissive refresh_token mode
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
* docs: add DCR_REQUEST_REFRESH_TOKEN_WHEN_UNSUPPORTED to documentation
Update documentation for new DCR refresh token configuration option:
- README.md: Add to DCR settings table
- charts/mcp-stack/values.yaml: Add with comment
- charts/mcp-stack/README.md: Regenerated via helm-docs
- docs/docs/manage/dcr.md: Add env var and behavior note
- docs/docs/config.schema.json: Add schema definition
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
---------
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
* feat: add ARM64 load testing support - Add build section for fast-time-server to support ARM64 architecture - Use pre-built ghcr.io image by default for x86_64 performance - ARM64 users can build locally via environment variable override - Fix Dockerfile to use TARGETARCH for proper cross-compilation Signed-off-by: Jonathan Springer <jps@s390x.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * Lint Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Jonathan Springer <jps@s390x.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
…2314) Signed-off-by: Satya <tsp.0713@gmail.com>
…ched Templates (#2333) * Optimize SQLite JSON tag filtering with deterministic binds and cached templates Signed-off-by: Satya <tsp.0713@gmail.com> * feat: add tag filtering support to list resources template in main apis(non-template) Signed-off-by: Satya <tsp.0713@gmail.com> * removed unused fields - page, limit from list resource template from resource services Signed-off-by: Satya <tsp.0713@gmail.com> * fix: remove debug print statements from tool_service.py Remove debugging print statements that were accidentally left in the tag filtering code path. These were outputting query details to stdout which is not appropriate for production code. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * Lint Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: use column-specific bind prefixes to prevent parameter collision When multiple json_contains_tag_expr calls are combined in the same query (e.g., filtering on tags from different columns), the fixed bind names (:p0, :p1) would collide and overwrite parameters. This fix adds column-specific prefixes to bind parameter names (e.g., :tools_tags_p0, :resources_tags_p0) to ensure uniqueness when composing multiple tag filter predicates. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * test: add coverage for json_contains_tag_expr and resource template filters Add comprehensive tests for: - _sanitize_col_prefix helper function - json_contains_tag_expr for SQLite with match_any and match_all - Bind parameter collision prevention when combining multiple tag filters - LRU caching of SQL templates - New list_resource_templates filtering parameters (tags, visibility, include_inactive) Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: use thread-safe counter for fully unique bind prefixes Address edge cases where bind parameters could still collide: 1. Same column filtered multiple times in one query 2. Different column refs that sanitize to identical strings (e.g., "a_b.c" and "a.b_c" both become "a_b_c") Replace static column-based prefix with a thread-safe counter that generates truly unique prefixes per call (e.g., "tools_tags_42_p0"). This removes the LRU caching of templates since each call now has a unique prefix, but ensures correctness in all edge cases. Add test for same-column collision scenario. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Satya <tsp.0713@gmail.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
* optimize response_cache_by_prompt lookup with inverted index Signed-off-by: Shoumi <shoumimukherjee@gmail.com> * fix type hint Signed-off-by: Shoumi <shoumimukherjee@gmail.com> * flake8 fixes Signed-off-by: Shoumi <shoumimukherjee@gmail.com> * test: add unit tests for response_cache_by_prompt inverted index Add comprehensive test coverage for the inverted index optimization: - Tokenization and vectorization functions - Basic cache store and hit functionality - Inverted index population and candidate filtering - Eviction and index rebuild scenarios - Max entries cap with index consistency Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Shoumi <shoumimukherjee@gmail.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
* feat: Add Gateway permission constants Add GATEWAYS_CREATE, GATEWAYS_READ, GATEWAYS_UPDATE, and GATEWAYS_DELETE permission constants to the Permissions class for consistency with other resource types (tools, resources, prompts, servers). Note: The original PR #2186 attempted to fix issue #2185 by modifying the visibility query logic, but that change was incorrect. The team filter should only show resources BELONGING to the filtered team, not all public resources globally. See todo/rbac.md for documentation. Issue #2185 needs further investigation - the reported bug may have a different root cause. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * feat: Add gateway permission patterns to token scoping middleware Add gateway routes to token scoping middleware for consistent permission enforcement: - Add gateway pattern to _RESOURCE_PATTERNS for ID extraction - Add gateway CRUD patterns to _PERMISSION_PATTERNS: - POST /gateways (exact) -> gateways.create - POST /gateways/{id}/... (sub-resources) -> gateways.update - PUT/DELETE -> gateways.update/delete - Add gateway handling in _check_resource_team_ownership: - Public: accessible by all - Team: accessible by team members - Private: owner-only access (per RBAC doc) Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: Enforce owner-only access for private visibility across all resources Per RBAC doc, private visibility means "owner only" - not "team members". Fixed private visibility checks for all resource types to validate owner_email == requester instead of team membership: - Servers - Tools - Resources - Prompts - Gateways (already correct from previous commit) This aligns token scoping middleware with the documented RBAC model. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * test: Add tests for gateway permissions and visibility RBAC Add unit tests covering: - Gateway permission patterns (POST create vs POST update sub-resources) - Private visibility enforces owner-only access - Team visibility allows team members only - Public visibility allows all authenticated users These tests validate the RBAC fixes in token scoping middleware. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
* feat-2187: add additional default roles while bootstrap
Signed-off-by: Nithin Katta <Nithin.Katta@ibm.com>
* feat-2187: fix lint issues
Signed-off-by: Nithin Katta <Nithin.Katta@ibm.com>
* feat-2187: fixing review comments
Signed-off-by: Nithin Katta <Nithin.Katta@ibm.com>
* feat-2187: fixing review comments
Signed-off-by: Nithin Katta <Nithin.Katta@ibm.com>
* feat-2187: test fix
Signed-off-by: Nithin Katta <Nithin.Katta@ibm.com>
* fix: Improve bootstrap roles validation and documentation
Fixes identified by code review:
1. Path resolution: Fixed parent.parent.parent -> parent.parent to correctly
resolve project root from mcpgateway/bootstrap_db.py
2. JSON validation: Added validation that loaded JSON is a list of dicts with
required keys (name, scope, permissions). Invalid entries are skipped with
warnings instead of crashing bootstrap.
3. Improved logging: Log all attempted paths when file not found
Added tests:
- test_bootstrap_roles_with_dict_instead_of_list: Validates error when JSON is
a dict instead of array
- test_bootstrap_roles_with_missing_required_keys: Validates warning when roles
are missing required fields
Added documentation:
- docs/docs/manage/rbac.md: New "Bootstrap Custom Roles" section with
configuration examples for Docker Compose and Kubernetes
- docs/docs/architecture/adr/036-bootstrap-custom-roles.md: ADR documenting
the feature design, error handling, and security considerations
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
* fix: Make description and is_system_role optional for bootstrap roles
ChatGPT review identified that description and is_system_role were accessed
unconditionally via role_def["key"], causing KeyError for minimal roles.
Fix:
- Use role_def.get("description", "") with empty string default
- Use role_def.get("is_system_role", False) with False default
Added test:
- test_bootstrap_roles_with_minimal_valid_role: Verifies a role with only
required fields (name, scope, permissions) is created successfully with
correct defaults for optional fields
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
---------
Signed-off-by: Nithin Katta <Nithin.Katta@ibm.com>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Co-authored-by: Nithin Katta <Nithin.Katta@ibm.com>
Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
…y blockers (#2394) * Remove last 2 security issues from Sonarqube Signed-off-by: Brian Hussey <brian.hussey@ie.ibm.com> * Remove 5 of 8 blocker maintainability issues Signed-off-by: Brian Hussey <brian.hussey@ie.ibm.com> * Correct linting errors Signed-off-by: Brian Hussey <brian.hussey@ie.ibm.com> --------- Signed-off-by: Brian Hussey <brian.hussey@ie.ibm.com>
* update roadmap and changelog Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * update links in docs Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
marekdano
left a comment
There was a problem hiding this comment.
What's Good
- Correct root cause fix — Moving JS out of x-data attributes eliminates the Jinja2/Alpine escaping conflict entirely. This is a clean separation of concerns.
- Single definition, multiple instances — The old approach duplicated the full JS object in every pagination_controls.html include. Now there's one paginationData() function shared by all pagination controls.
- Keyboard listener deduplication — The old <script> block at the bottom of the template registered the keydown listener N times (once per include). Moving it to admin.js inside
alpine:initregisters it once. This is a meaningful bug fix on its own. - query_params handled well — The Jinja2 {% for %} loop for query params is replaced with a data-query-params attribute using | tojson, which is then parsed in init() with defensive error handling.
- Template is dramatically simplified — pagination_controls.html goes from ~147 lines of mixed JS/HTML/Jinja2 down to ~19 lines of clean HTML data attributes.
Issues & Suggestions
- ESLint failures noted in the diff view — The WebFetch summary mentions two ESLint issues:
- Line ~270: formatting issue with object literal line breaks
- Line ~295: missing braces after if condition (if (prevBtn && !prevBtn.disabled) prevBtn.click();)
Confirm these pass make lint. The single-line if without braces is a style choice — if the project enforces curly, this will fail CI.
- Keyboard shortcuts target the wrong pagination control when multiple exist — The keydown handler uses document.querySelector('[\@click="prevPage()"]') which always grabs the first matching button on the page. If there are multiple pagination controls (tools, servers, resources tabs), arrow keys will always paginate the first one regardless of which tab is active. This is a pre-existing bug carried forward, but worth noting for a follow-up.
The approach is sound and addresses the root cause correctly. The PR is still in Draft status, so presumably the ESLint issues will be resolved before marking it ready. The keyboard shortcut scoping is pre-existing and shouldn't block this PR, but it would be good to track as a follow-up.
Recommendation: Approve once ESLint issues are resolved and CI is green. The template simplification and listener deduplication are clear wins beyond just fixing the console errors.
6d1763f to
f3fa212
Compare
@marekdano Where do we have multiple pagination controls on the same page? I want to test this issue. I added a bit to lock the controls to the last interacted with when there are more than one in a page. |
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
f3fa212 to
85eb3f1
Compare
85eb3f1 to
509b6a7
Compare
Signed-off-by: Gabriel Costa <gabrielcg@proton.me>
a609927 to
5fc6275
Compare
This issue is not introduced in this PR. The issue is already in the |
crivetimihai
left a comment
There was a problem hiding this comment.
Review: Move pagination data into script tag
Clean architectural improvement that eliminates duplicated inline JavaScript.
What's good
- Global
paginationData()function: Replaces N inline x-data blocks with a single global definition. Each instance reads its config from data-* attributes viainit(). - Keyboard navigation fix: The old per-include
<script>block registered the ArrowLeft/ArrowRight listener N times (once per pagination control). Now it's registered once globally with_lastActivePaginationRoottracking. - Query params registry:
window._paginationQuerySetterselegantly handles per-section query params without embedding Jinja in the global JS. - Data- attribute pattern*: Clean separation between server-rendered data and client-side behavior.
Minor
- The
rfc9728-compliance.mdlink change (GitHub permalink → relative path) is unrelated to pagination — might be cleaner as a separate commit, but not a blocker.
|
I addressed |
|
Thanks @gcgoncalves — moving pagination data into a script tag is a cleaner approach than embedding it inline. Good separation of concerns. |
|
Reopened as #3155. CI/CD will re-run on the new PR. You are still credited as the author. |
🐛 Bug-fix PR
📌 Summary
paginationData()once as a global function inadmin.js, with all its methods.data-*attributes;Closes #2975
🔁 Reproduction Steps
🐞 Root Cause
Jinja was escaping JS content inside a x-data attribute.
💡 Fix Description
Moved the JS object inside a script tag to prevent escaping, then referred to it from the
x-dataattribute.🧪 Verification
make lintmake testmake coverage✅ Checklist
make black isort pre-commit)