Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Contributor
Greptile SummaryThis PR implements defense-in-depth domain validation enforcement for SaaS connectors through API endpoint validation and runtime HTTP client checks. Key Changes:
Security Design:
The implementation is solid with no critical issues identified. The validation logic is sound, edge cases are handled correctly, and the test coverage is thorough. Confidence Score: 5/5
Important Files Changed
Last reviewed commit: de20536 |
Contributor
Author
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
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.
Ticket ENG-2569
Description Of Changes
Third of a 4-part series. This PR adds domain validation enforcement at the SaaS config update API endpoint and the runtime HTTP client.
API endpoint (saas_config_endpoints.py):
typeandallowed_valuesare immutable via the API once defined in the connector template.client_config.hostplaceholders in an incoming config must reference connector params oftype="endpoint"withallowed_valuesdefined.validate_saas_config_patch()insaas_util.pyto keep the endpoint focused on HTTP concerns.Runtime (authenticated_client.py):
allowed_valuesbefore every outbound request.This may be obvious for normal saas configs but not for override use: for request_overrides we don't use client configs but we do hardcode the domains inside the codebase most of the time. This means if
allowed_valuesis present in the config we will be enforcing it at runtime, rejecting whatever requests are made by the override unless they match any of the allowed values.Code Changes
validate_connector_param_constraints_not_modified,validate_host_references_domain_restricted_params, andvalidate_saas_config_patchtosaas_util.pysaas_config_endpoints.py, with template fallback when no existing config is found_validate_request_domaintoAuthenticatedClientinauthenticated_client.pySteps to Confirm
typeandallowed_valuessucceedstypeorallowed_valuesvia the API returns a 422type="endpoint"withallowed_valuesalso failsallowed_valuesare rejected at runtimedisable_domain_validationis truePre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and works