Skip to content

πŸ›‘οΈ Sentinel: [MEDIUM] Fix JWT crit header validation #976

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
seonghobae wants to merge 3 commits into from
+58 βˆ’27
Closed

πŸ›‘οΈ Sentinel: [MEDIUM] Fix JWT crit header validation #976

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9694269
πŸ›‘οΈ Sentinel: [MEDIUM] Fix JWT crit header validation
seonghobae Aug 23, 2026
a78d2cd
πŸ›‘οΈ Sentinel: [MEDIUM] Fix JWT crit header validation
seonghobae Aug 23, 2026
e1db64f
πŸ›‘οΈ Sentinel: [HIGH] Fix information leakage in JWT validation errors
seonghobae Aug 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .jules/sentinel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,11 @@
**Vulnerability:** User-provided string fields (like project and connection names) lacked strict validation against control characters, only relying on length constraints.
**Learning:** This could potentially lead to Log Injection (CRLF injection), Null Byte Injection, or terminal escape injection if these strings are subsequently logged or rendered directly.
**Prevention:** Use explicit regex validation `pattern=r'^[^\x00-\x1F\x7F]+$'` on Pydantic string fields to strictly reject control characters.
## 2025-02-18 - JWT crit 헀더 검증 κ°•μ œ
**Vulnerability:** JWT `crit` (critical) 헀더가 μ‘΄μž¬ν•  λ•Œ, RFC 7515에 따라 이λ₯Ό λͺ…μ‹œμ μœΌλ‘œ κ²€μ¦ν•˜μ§€ μ•ŠμœΌλ©΄ μΈμ§€ν•˜μ§€ λͺ»ν•˜λŠ” μ€‘μš” ν™•μž₯이 ν¬ν•¨λœ 토큰을 ν—ˆμš©ν•˜κ²Œ λ˜μ–΄ λ³΄μ•ˆ μŠ€μΊ”(STRIX)을 ν†΅κ³Όν•˜μ§€ λͺ»ν•©λ‹ˆλ‹€.
**Learning:** `PyJWT`λ‚˜ `python-jose`λ₯Ό μ‚¬μš©ν•  λ•Œ `crit` ν—€λ”μ˜ νƒ€μž… 및 길이λ₯Ό μ—„κ²©νžˆ 검증(길이 μ œν•œ 리슀트 ν˜•νƒœ)ν•΄μ•Ό ν•˜κ³ , μ•Œ 수 μ—†λŠ” ν™•μž₯에 λŒ€ν•΄μ„œλŠ” λ°˜λ“œμ‹œ 토큰을 κ±°λΆ€ν•΄μ•Ό ν•©λ‹ˆλ‹€.
**Prevention:** `_validate_jwt_header` ν•¨μˆ˜μ—μ„œ `crit` 헀더 쑴재 μ—¬λΆ€λ₯Ό ν™•μΈν•˜κ³ , 길이가 μ œν•œλœ λ¬Έμžμ—΄ λ¦¬μŠ€νŠΈμΈμ§€ ν™•μΈν•˜λ©°, μ§€μ›ν•˜μ§€ μ•ŠλŠ” ν™•μž₯(ν˜„μž¬ 0개)이 있으면 μ¦‰μ‹œ 401을 λ°˜ν™˜ν•˜λ„λ‘ μˆ˜μ •ν•©λ‹ˆλ‹€.
## 2026-08-23 - JWT Token Validation Error Message Leakage Fix
**Vulnerability:** The API returned highly specific error messages (e.g., "unknown signing key", "algorithm/key type mismatch", "unsupported token algorithm") during JWT validation, leaking internal implementation details to potential attackers.
**Learning:** Returning specific token validation errors allows attackers to perform reconnaissance on the auth mechanism, testing different vectors to map out the exact token requirements and libraries in use.
**Prevention:** Standardize all token validation failure responses to return a generic HTTP 401 with `detail="invalid token"` to provide no useful feedback to unauthorized requests.
42 changes: 27 additions & 15 deletions backend/app/auth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ def _jwt_expiry(claims: dict[str, Any]) -> dt.datetime:

exp = claims.get("exp")
if not isinstance(exp, int | float):
raise HTTPException(status_code=401, detail="token missing exp")
raise HTTPException(status_code=401, detail="invalid token")
return dt.datetime.fromtimestamp(float(exp), tz=dt.timezone.utc)


Expand All @@ -179,15 +179,27 @@ def _validate_jwt_header(header: dict[str, Any]) -> str:
not isinstance(token_type, str)
or token_type.strip().lower() not in OIDC_ALLOWED_TOKEN_TYPES
):
raise HTTPException(status_code=401, detail="unsupported token type")
raise HTTPException(status_code=401, detail="invalid token")

content_type = header.get("cty")
if content_type is not None:
raise HTTPException(status_code=401, detail="unsupported token content type")
raise HTTPException(status_code=401, detail="invalid token")

crit = header.get("crit")
if crit is not None:
if not isinstance(crit, list):
raise HTTPException(status_code=401, detail="invalid token")
if len(crit) > 5:
raise HTTPException(status_code=401, detail="invalid token")
for param in crit:
if not isinstance(param, str):
raise HTTPException(status_code=401, detail="invalid token")
# Reject all unrecognized parameters. We currently do not support any critical extensions.
raise HTTPException(status_code=401, detail="invalid token")

header_alg_raw = header.get("alg")
if not isinstance(header_alg_raw, str) or not header_alg_raw:
raise HTTPException(status_code=401, detail="token missing alg")
raise HTTPException(status_code=401, detail="invalid token")
return header_alg_raw.upper()


Expand Down Expand Up @@ -240,13 +252,13 @@ async def _decode_verified_oidc_token(token: str) -> dict[str, Any]:
try:
header = cast(dict[str, Any], jwt.get_unverified_header(token))
except Exception: # noqa: BLE001
raise HTTPException(status_code=401, detail="invalid token header")
raise HTTPException(status_code=401, detail="invalid token")

header_alg = _validate_jwt_header(header)
if header_alg not in OIDC_ALLOWED_ALGORITHMS:
raise HTTPException(
status_code=401,
detail="unsupported token algorithm",
detail="invalid token",
)

jwks = await _get_jwks()
Expand All @@ -255,20 +267,20 @@ async def _decode_verified_oidc_token(token: str) -> dict[str, Any]:
jwks = await _get_jwks(force_refresh=True)
jwk = _pick_jwk(jwks, header.get("kid"))
if jwk is None:
raise HTTPException(status_code=401, detail="unknown signing key")
raise HTTPException(status_code=401, detail="invalid token")

kty = jwk.get("kty")
if not isinstance(kty, str):
raise HTTPException(status_code=401, detail="algorithm/key type mismatch")
raise HTTPException(status_code=401, detail="invalid token")
jwk_kty = kty.upper()
if jwk_kty == "RSA":
if not (header_alg.startswith("RS") or header_alg.startswith("PS")):
raise HTTPException(status_code=401, detail="algorithm/key type mismatch")
raise HTTPException(status_code=401, detail="invalid token")
elif jwk_kty == "EC":
if not header_alg.startswith("ES"):
raise HTTPException(status_code=401, detail="algorithm/key type mismatch")
raise HTTPException(status_code=401, detail="invalid token")
else:
raise HTTPException(status_code=401, detail="algorithm/key type mismatch")
raise HTTPException(status_code=401, detail="invalid token")

try:
claims = jwt.decode(
Expand All @@ -288,7 +300,7 @@ async def _decode_verified_oidc_token(token: str) -> dict[str, Any]:
)
except Exception as err:
raise HTTPException(
status_code=401, detail="token verification failed"
status_code=401, detail="invalid token"
) from err

return cast(dict[str, Any], claims)
Expand All @@ -303,13 +315,13 @@ async def _verified_token_from_claims(
jwt_id = claims.get("jti")
name = claims.get("name") or claims.get("preferred_username")
if not isinstance(sub, str):
raise HTTPException(status_code=401, detail="token missing sub")
raise HTTPException(status_code=401, detail="invalid token")
if not isinstance(jwt_id, str) or not jwt_id.strip():
raise HTTPException(status_code=401, detail="token missing jti")
raise HTTPException(status_code=401, detail="invalid token")

expires_at = _jwt_expiry(claims)
if verify_revocation and await is_token_jti_revoked(jwt_id):
raise HTTPException(status_code=401, detail="token revoked")
raise HTTPException(status_code=401, detail="invalid token")

return VerifiedToken(
subject=sub,
Expand Down
35 changes: 23 additions & 12 deletions backend/tests/test_auth_security.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ def fail_decode(*_: object, **__: object) -> dict:
)

assert exc_info.value.status_code == 401
assert exc_info.value.detail == "unsupported token algorithm"
assert exc_info.value.detail == "invalid token"


@pytest.mark.asyncio
Expand Down Expand Up @@ -244,7 +244,7 @@ def fail_decode(*_: object, **__: object) -> dict:
await auth._decode_verified_oidc_token("ey...fake...")

assert exc_info.value.status_code == 401
assert exc_info.value.detail == "algorithm/key type mismatch"
assert exc_info.value.detail == "invalid token"


@pytest.mark.asyncio
Expand Down Expand Up @@ -303,21 +303,19 @@ async def mock_is_token_revoked(jti):


@pytest.mark.parametrize(
("header", "detail"),
("header",),
[
(
{"kid": "key-1", "alg": "RS256", "typ": "nested+jwt"},
"unsupported token type",
),
(
{"kid": "key-1", "alg": "RS256", "cty": "JWT"},
"unsupported token content type",
),
],
)
@pytest.mark.asyncio
async def test_oidc_rejects_unsupported_header_types(
monkeypatch: pytest.MonkeyPatch, header: dict[str, str], detail: str
monkeypatch: pytest.MonkeyPatch, header: dict[str, str]
) -> None:
monkeypatch.setattr(settings, "oidc_issuer", "https://issuer.example")
monkeypatch.setattr(settings, "oidc_audience", "pg-erd")
Expand All @@ -334,7 +332,7 @@ async def fail_jwks() -> dict:
)

assert exc_info.value.status_code == 401
assert exc_info.value.detail == detail
assert exc_info.value.detail == "invalid token"


@pytest.mark.asyncio
Expand Down Expand Up @@ -417,7 +415,7 @@ async def mock_is_token_revoked2(jti):
)

assert exc_info.value.status_code == 401
assert exc_info.value.detail == "token missing jti"
assert exc_info.value.detail == "invalid token"


@pytest.mark.asyncio
Expand Down Expand Up @@ -464,7 +462,7 @@ async def mock_revoke(jti, ext):
)

assert exc_info.value.status_code == 401
assert exc_info.value.detail == "token revoked"
assert exc_info.value.detail == "invalid token"


@pytest.mark.asyncio
Expand Down Expand Up @@ -560,7 +558,7 @@ def mock_get_unverified_header(token):
await auth._decode_verified_oidc_token("invalid_token")

assert excinfo.value.status_code == 401
assert excinfo.value.detail == "invalid token header"
assert excinfo.value.detail == "invalid token"


@pytest.mark.asyncio
Expand Down Expand Up @@ -592,7 +590,7 @@ async def mock_is_token_revoked2(jti):
await auth._decode_verified_oidc_token("Bearer token")

assert exc_info.value.status_code == 401
assert exc_info.value.detail == "token verification failed"
assert exc_info.value.detail == "invalid token"

@pytest.mark.asyncio
async def test_oidc_rejects_algorithm_key_type_mismatch(
Expand Down Expand Up @@ -625,7 +623,7 @@ def fail_decode(*_: object, **__: object) -> dict:
await auth._decode_verified_oidc_token("ey...")

assert exc_info.value.status_code == 401
assert exc_info.value.detail == "algorithm/key type mismatch"
assert exc_info.value.detail == "invalid token"
@pytest.mark.asyncio
async def test_oidc_jwks_refresh_rate_limiting(
monkeypatch: pytest.MonkeyPatch,
Expand Down Expand Up @@ -733,3 +731,16 @@ async def get(self, url: str) -> _FakeHttpResponse:
{"keys": [{"kid": "new-key", "kty": "RSA"}]},
]
assert request_count == before_concurrent_refresh + 1

def test_crit_header_validation():

@devin-ai-integration devin-ai-integration Bot Aug 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

πŸ“ Info: New test missing the return annotation used elsewhere

test_crit_header_validation() omits the -> None annotation every other test in the file carries. CI runs mypy only on app/, so it will not fail, but it breaks the file's convention.

Open in Devin Review

Was this helpful? React with πŸ‘ or πŸ‘Ž to provide feedback.

with pytest.raises(HTTPException, match="invalid token"):
auth._validate_jwt_header({"alg": "RS256", "crit": "not-a-list"})

with pytest.raises(HTTPException, match="invalid token"):
auth._validate_jwt_header({"alg": "RS256", "crit": ["a", "b", "c", "d", "e", "f"]})

with pytest.raises(HTTPException, match="invalid token"):
auth._validate_jwt_header({"alg": "RS256", "crit": [123]})

with pytest.raises(HTTPException, match="invalid token"):
auth._validate_jwt_header({"alg": "RS256", "crit": ["unknown"]})
Loading