Repository navigation
Conversation
Blank or whitespace-only user_jwt values triggered a silent fallback from
OBO (per-user) to service (application) credentials due to Python truthy
checks (if self._user_jwt / if _jwt). Empty and whitespace-only strings
are falsy, silently routing to get_token() (service credentials) instead
of exchange_token() (OBO) — an authorization-escalation attack surface.
Fix: add a single enforcement helper _require_non_blank_jwt and apply it
at every public entry point — constructors (conditional on is not None),
with_user_jwt methods (unconditional), and factory functions (conditional).
Correct both truthy-check bug sites to use is not None comparisons.
Behavior contract (before → after):
- create_client()/user_jwt=None → service creds (unchanged)
- create_client(user_jwt='valid') → OBO exchange (unchanged)
- create_client(user_jwt='')/' ' → ValueError
- AdmsHttp(user_jwt=None) → service creds (unchanged)
- AdmsHttp(user_jwt='')/' ' → ValueError
- with_user_jwt('valid') → OBO exchange (unchanged)
- with_user_jwt('')/' '/None → ValueError
BREAKING CHANGE: passing empty or whitespace user_jwt (which previously
silently used service credentials) now raises ValueError. user_jwt=None /
omitted (service mode) is unchanged. Cross-repo scan confirmed zero
external consumers of sap_cloud_sdk.adms.
This branch has not been deployed
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
1 participant
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.
Summary
Blank or whitespace-only
user_jwtvalues previously caused a silent fallbackfrom OBO (per-user) to service (application) credentials, creating an
authorization-escalation attack surface.
Automatic enforcement — no caller changes required. The validation is built
directly into the SDK entry points. Agents using
sap_cloud_sdk.admsreceivethe protection for free.
Root cause
Both OBO decision points used Python truthy checks (
if self._user_jwt/if _jwt).Empty and whitespace-only strings are falsy, silently routing to
get_token()(service credentials) instead of
exchange_token()(OBO).Behavior contract (before → after)
create_client()/user_jwt=Nonecreate_client(user_jwt="valid")create_client(user_jwt="")/" "ValueErrorAdmsHttp(user_jwt=None)AdmsHttp(user_jwt="")/" "ValueErrorwith_user_jwt("valid")with_user_jwt("")/" "/NoneValueErrorBreaking change
Callers that previously passed
""or whitespace and unknowingly received servicecredentials will now get
ValueError. This is the intended security correction.user_jwt=None/ omitted (service mode) is unchanged at the constructor/factory level.Blast radius: SDK-only. Cross-repo scan found zero importers of
sap_cloud_sdk.admsin any downstream agent/service repo. No external code changes required.
Changes
src/sap_cloud_sdk/adms/_http.py_require_non_blank_jwthelper; fix 2 truthy-check bug sites; 2 constructor guards; 2with_user_jwtguardssrc/sap_cloud_sdk/adms/client.pywith_user_jwtguards; 2 factoryuser_jwtguardssrc/sap_cloud_sdk/adms/user-guide.mdtests/adms/unit/test_http.pyTestAdmsHttpOboInvariant(14 tests)tests/adms/unit/test_client.pyVerification
_http.py94%,client.py95% coverageruff+tyclean on all changedsrc/files