Skip to content

fix: add subdomain validator - #279

Merged
betinacosta merged 10 commits into
mainfrom
fix/validate-tenant-subdomain
Aug 21, 2026
Merged

fix: add subdomain validator#279
betinacosta merged 10 commits into
mainfrom
fix/validate-tenant-subdomain

Conversation

@betinacosta

Copy link
Copy Markdown
Member

Disclaimer: Do not include SAP-internal or customer-specific information in this PR (e.g. internal system URLs, customer names, tenant IDs, or confidential configurations). This is a public repository.

Description

Adds a shared _validate_tenant_subdomain utility in sap_cloud_sdk.core.url_utils that validates a tenant subdomain against RFC 1123 DNS label rules (ASCII letters, digits, and hyphens only; no leading/trailing hyphens; max 63 characters). The validator is now called before any token URL substitution in:

  • AgentGatewayClient._resolve_tenant_subdomain (agentgateway/agw_client.py)
  • TokenProvider.get_token (destination/_http.py)
  • Auth._resolve_token_url (dms/_auth.py)

A ValueError with a clear message is raised on invalid input instead of silently constructing a malformed URL.

Related Issue

Closes #

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

How to Test

  1. Install the package locally: pip install -e .
  2. Attempt to use AgentGatewayClient with an invalid subdomain:
    from sap_cloud_sdk.agentgateway import AgentGatewayClient
    client = AgentGatewayClient(tenant_subdomain="-invalid")
    client._resolve_tenant_subdomain()  # raises ValueError: Invalid tenant_subdomain
  3. Call TokenProvider.get_token(tenant_subdomain="has.dot") in the destination module — expect ValueError: Invalid tenant_subdomain.
  4. Call Auth._resolve_token_url("trailing-") in the DMS module — expect ValueError: Invalid tenant_subdomain.
  5. Run the unit test suites:
    pytest tests/core/unit/test_url_utils.py
    pytest tests/agentgateway/unit/test_agw_client.py
    pytest tests/destination/unit/test_http.py
    pytest tests/dms/unit/test_auth.py
    
  6. Verify all existing tests still pass.

Checklist

  • I have read the Contributing Guidelines
  • I have verified that my changes solve the issue
  • I have added/updated automated tests to cover my changes
  • All tests pass locally
  • I have verified that my code follows the Code Guidelines
  • I have updated documentation (if applicable)
  • I have added type hints for all public APIs
  • My code does not contain sensitive information (credentials, tokens, etc.)
  • I have followed Conventional Commits for commit messages

Additional Notes

The validator is intentionally kept internal (_validate_tenant_subdomain) and lives in sap_cloud_sdk.core.url_utils for reuse across all modules that accept a tenant_subdomain parameter. Passing None is a no-op to accommodate callers that resolve the subdomain lazily or make it optional.

@betinacosta betinacosta changed the title fix: add subdomain validatorvalidator to core fix: add subdomain validator Aug 20, 2026
Comment thread src/sap_cloud_sdk/agentgateway/user-guide.md
Comment thread src/sap_cloud_sdk/core/telemetry/user-guide.md
Comment thread src/sap_cloud_sdk/outputmanagement/user-guide.md
@betinacosta
betinacosta marked this pull request as ready for review August 20, 2026 17:41
@betinacosta
betinacosta requested a review from a team as a code owner August 20, 2026 17:41
Comment thread src/sap_cloud_sdk/core/_tenant.py

@cassiofariasmachado cassiofariasmachado left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I've just added a minor suggestion

Comment thread src/sap_cloud_sdk/agentgateway/agw_client.py Outdated
Comment thread src/sap_cloud_sdk/destination/_http.py Outdated
@betinacosta
betinacosta merged commit fc67a8f into main Aug 21, 2026
11 checks passed
@betinacosta
betinacosta deleted the fix/validate-tenant-subdomain branch August 21, 2026 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants