Skip to content

fix(runtime_context): verify JWT signature in IASContextProvider - #424

Merged
tiagoek merged 4 commits into
mainfrom
fix/ias-context-provider-verified-claims
Oct 28, 2026
Merged

tiagoek merged 4 commits into
mainfrom
fix/ias-context-provider-verified-claims

Conversation

@tiagoek

@tiagoek tiagoek commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Disclaimer: Do not include SAP-internal or customer-specific information in this PR. This is a public repository.

Description

IASContextProvider.extract() was calling parse_token() — a deliberately unverified JWT decoder (verify_signature: False) — and writing app_tid, sap_gtid, user_uuid directly into the request-scoped RuntimeContext. An attacker could submit a self-signed JWT with arbitrary tenant/user claims and poison the context consumed by RuntimeContextSpanProcessor, IdentityLogFilter, AuditClient, and CBCClient.

Fix: apply the same TokenVerifier / fail-closed pattern used by StarletteIASTelemetryMiddleware — auto-configure an IASVerifier from the environment; return empty context when no binding is found or when verification fails.

AI-generated code: This PR was developed with AI assistance (Claude Code). See the SAP AI contribution guideline.

Related Issue

No GitHub issue tracked — this is a security fix for an internally reported finding.

Type of Change

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

How to Test

  1. Run the runtime context unit tests: uv run pytest tests/core/unit/runtime_context/ -v
  2. Verify 57 tests pass, including the new security-focused cases:
    • test_no_verifier_returns_empty_context — no IAS binding → empty context (fail-closed)
    • test_forged_token_returns_empty_context — verifier raises → no claims leak
    • test_infra_error_logs_warning_not_debug — DNS/TLS failure logs at WARNING level
  3. Run the full unit suite: uv run pytest tests/ --ignore=tests/agent_memory --ignore=tests/destination/integration --ignore=tests/core/integration -q
  4. Confirm IASContextProvider() without an IAS binding starts without crashing and logs a WARNING

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
  • (Security-sensitive changes only) I have run /security-review and addressed all ❌ findings

IASContextProvider was calling parse_token() — a deliberately unverified
JWT decoder — and writing app_tid, sap_gtid, and user_uuid directly into
the request-scoped RuntimeContext. This allowed an attacker to forge a JWT
carrying any tenant/user identity and poison the runtime context consumed
by RuntimeContextSpanProcessor, IdentityLogFilter, AuditClient, and
CBCClient.

Apply the same TokenVerifier / fail-closed pattern already used in
StarletteIASTelemetryMiddleware: auto-configure an IASVerifier from the
environment at construction time; return empty context when no binding is
found or when verification fails.

Security: fixes incomplete CWE-347 remediation from v0.62.0.
…ls in IASContextProvider

IASTokenError (bad token) logged at DEBUG — expected, caller-side issue.
Any other exception (DNS/TLS/JWKS unreachable) logged at WARNING — operator
needs to know identity context is consistently broken.

Addresses G6 review W3.
tiagoek

This comment was marked as outdated.

@tiagoek
tiagoek marked this pull request as ready for review October 26, 2026 14:57
@tiagoek
tiagoek requested a review from a team as a code owner October 26, 2026 14:57
@tiagoek
tiagoek merged commit 393a7a5 into main Oct 28, 2026
15 of 17 checks passed
@tiagoek
tiagoek deleted the fix/ias-context-provider-verified-claims branch October 28, 2026 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants