feat(security): add CSRF token validation for state-changing requests#3134
Closed
crivetimihai wants to merge 10 commits intomainfrom
Closed
feat(security): add CSRF token validation for state-changing requests#3134crivetimihai wants to merge 10 commits intomainfrom
crivetimihai wants to merge 10 commits intomainfrom
Conversation
…/PATCH requests Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
10 tasks
…rets, and admin UI defaults (m-batch-1) (#3129) * fix: request logging hardening and behavior consistency Refs: A-04 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: token scoping hardening and behavior consistency Refs: C-06, C-26 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: llm chat config hardening and behavior consistency Refs: C-31 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: permission model hardening and behavior consistency Refs: C-34, L-13 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: server team assignment hardening and behavior consistency Refs: C-36 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: import visibility defaults hardening and behavior consistency Refs: C-37 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: llm provider config hardening and behavior consistency Refs: C-40 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: tool header protection hardening and behavior consistency Refs: C-41 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: sso identity checks hardening and behavior consistency Refs: O-08, O-13 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: oauth state handling hardening and behavior consistency Refs: O-12 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: admin ui hardening and behavior consistency Refs: U-02, U-03, U-04 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * docs: rc2 hardening release notes and behavior consistency Refs: A-04, C-06, C-26, C-31, C-34, C-36, C-37, C-40, C-41, L-13, O-08, O-12, O-13, U-02, U-03, U-04 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * lint Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: harden medium security defaults and UI regression paths A-04 C-34 L-13 O-12 U-02 U-04 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: escape pagination query params inside Alpine attributes U-03 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * chore: resolve migration and oauth lint warnings C-34 O-12 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix tests Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix cdn Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * test: harden playwright admin auth and entity timing stability Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * test: stabilize localhost admin auth flow in playwright Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: align admin auth-form csrf token issuance and submission Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * test: close remaining diff coverage gaps to 100 percent Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
… authors, cleanup potential insecure defaults, etc. (#3218) Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
* fix: low-batch hardening and behavior consistency - L-01: proxy-auth trust now requires explicit dangerous acknowledgement when client auth is disabled - L-02: unauthenticated admin behavior is explicit and warning-backed when AUTH_REQUIRED is disabled - L-03: OAuth state handling and gateway lookup behavior are aligned for consistent responses - L-05: OAuth state validation flow is tightened before downstream processing - L-07: docs/auth token validation paths are consistent with revocation and user-state checks - L-08: SSO admin preservation behavior is aligned with configured policy and role-sync rules - L-09: SSO issuer allowlist enforcement is active and normalized for comparisons - L-10: crypto/security documentation is aligned with runtime implementation behavior - L-11: SSO nonce-protection documentation is aligned with actual behavior - L-12: SSO team mapping is applied during login flows with regression coverage - L-15: tool cache auth-sensitive hydration is type-safe and avoids unsafe payload assumptions - L-16: token usage-limit enforcement path is active with deny-path coverage - L-17: debug console logging cleanup and frontend test alignment for admin UI scripts Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: address code review findings from low-batch hardening - Rate-limit proxy auth trust warning to log once per process - Replace unnecessary db.commit() with db.rollback() in read-only _check_usage_limits path - Use HTTPException directly for SSO preserve-admin enforcement instead of ValueError control flow; re-raise HTTPException before generic handler so 401 is no longer swallowed into 500 - Document TRUST_PROXY_AUTH_DANGEROUSLY and ALLOW_UNAUTHENTICATED_ADMIN in .env.example with DANGER warnings - Warn when blank issuer bypasses SSO_ISSUERS allowlist - Preserve original console.log/debug on window before override - Add plugin_context_table/plugin_global_context to anonymous context Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * Fix locust test auth Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * Disable OBS Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * chore: remove duplicate linting-workflow-commitlint target from Makefile The target was defined twice — an older version without the COMMITLINT_ENFORCED toggle and the newer toggleable version. Remove the old definition and co-locate all commitlint-only variables with the surviving target, following the Makefile's convention of defining variables near the targets that use them. Signed-off-by: Jonathan Springer <jps@s390x.com> * chore: exclude Entra ID e2e tests from default test targets tests/e2e/test_entra_id_integration.py requires external Entra ID configuration and is not suitable for the default `make test` and `make test-altk` runs. Signed-off-by: Jonathan Springer <jps@s390x.com> * MCP_REQUIRE_AUTH now inherits AUTH_REQUIRED when unset Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * MCP_REQUIRE_AUTH now inherits AUTH_REQUIRED when unset - tests Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * MCP_REQUIRE_AUTH now inherits AUTH_REQUIRED when unset - tests Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * MCP_REQUIRE_AUTH now inherits AUTH_REQUIRED when unset - tests Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * MCP_REQUIRE_AUTH now inherits AUTH_REQUIRED when unset - tests Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * MCP_REQUIRE_AUTH now inherits AUTH_REQUIRED when unset - fix ui Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * linter updates Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Signed-off-by: Jonathan Springer <jps@s390x.com> Co-authored-by: Jonathan Springer <jps@s390x.com>
…ty service (#3140) * refactor(plugins/framework): introduce protocol and dependency injection for observability service Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * chore: fix lint issues Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * feat: add observability adapter for plugins Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * tests: coverage gaps Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * fix: address review findings in observability refactor - Eliminate double plugin execution on failure by restructuring _execute_with_timeout to separate observability from plugin execution - Close orphaned spans with status="error" when plugin hooks raise - Fix docstring typo in ObservabilityMiddleware - Move ObservabilityServiceAdapter import inside conditional block - Document ContextVar bridge requirement on both definitions Signed-off-by: Jonathan Springer <jps@s390x.com> * tests: achieve 100% diff coverage for observability refactor Cover error and edge-case paths in plugin executor observability spans and get_plugin_manager initialization with plugins enabled. Signed-off-by: Jonathan Springer <jps@s390x.com> --------- Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> Signed-off-by: Jonathan Springer <jps@s390x.com> Co-authored-by: Frederico Araujo <frederico.araujo@ibm.com> Co-authored-by: Jonathan Springer <jps@s390x.com>
…3141) * refactor(plugins): decouple plugin framework from gateway settings Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * chore: fix lint issues Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * fix: variable expansion in clean target Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * feat: additional cleanups and documentation Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * fix: mypy warning Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * fix: mypy warnings Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * fix: plugin settings env override Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * fix: minor issues Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * tests: update conftest Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * fix: from_env should read from environment Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * feat: expose cache_clear through settings Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * feat: use cached settings in from_env adapters Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * chore: fix lint issues Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> * fix: preserve Helm backward compat and harden plugin settings - Comment out PLUGINS_CONFIG_FILE default in values.yaml so legacy PLUGIN_CONFIG_FILE overrides are not shadowed by coalesce/AliasChoices - Return PluginsSettings from config.py plugins property (not the LazySettingsWrapper proxy) to match the type annotation - Use SecretStr for keyfile_password fields and .get_secret_value() in from_env adapters - Switch runtime servers from PluginsSettings() to get_settings() for cached singleton access - Simplify get_settings to no-arg lru_cache(maxsize=1) - Remove LazySettingsWrapper.enabled os.getenv bypass - Fix AliasChoices priority to prefer PLUGINS_ prefixed names - Add env_file and extra=ignore to model_config - Simplify test_tool_service_coverage to rely on autouse cache_clear Signed-off-by: Jonathan Springer <jps@s390x.com> * fix: pass observability provider to PluginManager and update test The get_plugin_manager() function accepted an observability parameter but silently discarded it after the settings migration. Also updates the observability test to patch the new framework settings instead of the removed gateway settings fields. Signed-off-by: Jonathan Springer <jps@s390x.com> --------- Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com> Signed-off-by: Jonathan Springer <jps@s390x.com> Co-authored-by: Frederico Araujo <frederico.araujo@ibm.com> Co-authored-by: Jonathan Springer <jps@s390x.com>
* docs: record chart upgrade guidance and new helm resource defaults Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(charts): harden postgres upgrade safety and document rules Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix: tune chart probe timings for high-load stability Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * hardening: reduce token exposure and consolidate postgres secrets Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * hardening: disable pgadmin and redis commander by default Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * hardening: enable redis auth with secret-backed credentials Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * feat(charts): harden workloads and enable default network policies Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * feat(charts): harden ingress tls defaults Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(chart): require auth for MCP endpoint by default Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * docs(helm): document SSRF requirements for in-cluster tool registration Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * Update packages Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * Update docs Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
…/PATCH requests Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
Signed-off-by: Vishu Bhatnagar <vishu.bhatnagar@ibm.com>
10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…/PATCH requests
🔗 Related Issue
Closes #
📝 Summary
What does this PR do and why?
🏷️ Type of Change
🧪 Verification
make lintmake testmake coverage✅ Checklist
make black isort pre-commit)📓 Notes (optional)
Screenshots, design decisions, or additional context.