Skip to content

Redact SQLAlchemy bind params, attach SQL context to errors, add RFC8523 JTI replay and tests#4862

Open
cobycloud wants to merge 3 commits into
masterfrom
ccdx/enhance-error-reporting-for-sqlalchemy/2026-02-05-2izwhl
Open

Redact SQLAlchemy bind params, attach SQL context to errors, add RFC8523 JTI replay and tests#4862
cobycloud wants to merge 3 commits into
masterfrom
ccdx/enhance-error-reporting-for-sqlalchemy/2026-02-05-2izwhl

Conversation

@cobycloud

Copy link
Copy Markdown
Contributor

Motivation

  • Prevent sensitive bind values from leaking in REST/JSON-RPC payloads while preserving useful, machine-readable diagnostics when SQLAlchemy errors occur.
  • Validate redaction end-to-end via ASGI (uvicorn) integration tests and ensure unit-level coverage for SQLAlchemy error metadata.
  • Add simple JTI replay detection to exercise RFC8523 replay-protection behavior and fix test DB setup so schema-qualified tables (e.g. authn.tenants) exist during tests.

Description

  • Added _format_sqlalchemy_error_data, _safe_params_metadata, and _looks_like_validation_error helpers to tigrbl/runtime/errors/utils.py and exported them for use by the error conversion layer.
  • Extended tigrbl/runtime/errors/converters.py to recognize StatementError and to include SQL context or safe redaction metadata for StatementError, OperationalError, and DBAPIError when appropriate.
  • Implemented an in-memory _JTI_CACHE with locking and expiry logic in tigrbl_auth/rfc/rfc8523.py and invoked replay detection inside validate_enhanced_jwt_bearer.
  • Added integration and unit tests covering SQLAlchemy redaction (pkgs/standards/tigrbl_tests/tests/i9n/test_sqlalchemy_error_redaction_uvicorn.py and pkgs/standards/tigrbl_tests/tests/unit/runtime/test_error_sqlalchemy_context.py) and extended RFC8523 tests to assert JTI replay behavior (pkgs/standards/tigrbl_auth/tests/unit/test_rfc8523_jwt_client_auth.py).
  • Fixed tigrbl_auth test setup by attaching a temporary SQLite file as the authn schema in pkgs/standards/tigrbl_auth/tests/conftest.py and cleaning it up after the engine is disposed so schema-qualified inserts succeed.

Testing

  • Ran code formatting with uv run --directory pkgs/standards/tigrbl_auth --package tigrbl-auth ruff format . which completed successfully.
  • Ran linter/fix with uv run --directory pkgs/standards/tigrbl_auth --package tigrbl-auth ruff check . --fix which reported All checks passed!.
  • Executed the tigrbl_auth test suite with uv run --package tigrbl-auth --directory standards/tigrbl_auth pytest and observed 388 passed, 5 skipped, 83 deselected (all tests in that run passed).
  • New tigrbl unit and i9n tests for error redaction were added to pkgs/standards/tigrbl_tests; those tests were added but not executed as part of the tigrbl_auth pytest run above.

Codex Task

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b3ba7a96a3

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +183 to +187
_purge_expired_jtis(current_time, max_age_seconds)
if jti in _JTI_CACHE:
return True
if current_time - iat <= max_age_seconds:
_JTI_CACHE[jti] = iat

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Cache JTIs for skew-allowed tokens

Because validate_enhanced_jwt_bearer allows tokens up to max_age_seconds + clock_skew_seconds old, a token with age just over max_age_seconds still passes validation, but is_jwt_replay won’t cache it due to the stricter current_time - iat <= max_age_seconds check. That means reusing the same token within the skew window won’t be detected as a replay. Consider caching JTIs for the full validation window (e.g., include skew or pass the effective max age) so second use of an otherwise valid token is rejected.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant