Skip to content

fix: set size cap on reads - #180

Merged
AjayThorve merged 2 commits into
NVIDIA-AI-Blueprints:developfrom
cdgamarose-nv:cdgamarose/fix_auth_cve
Apr 14, 2026
Merged

AjayThorve merged 2 commits into
NVIDIA-AI-Blueprints:developfrom
cdgamarose-nv:cdgamarose/fix_auth_cve

Conversation

@cdgamarose-nv

Copy link
Copy Markdown
Collaborator

No description provided.

@greptile-apps

greptile-apps Bot commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a 64 KB read cap (_MAX_FETCH_BYTES) to the HTTP reads in _fetch_oidc_config and _fetch_jwks_keys, protecting against excessively large OIDC/JWKS responses. Both sites now emit a warning log before capping, addressing the prior observability concern.

The one remaining issue is that truncating to _MAX_FETCH_BYTES still produces invalid JSON, so json.loads always raises JSONDecodeError on the overflow path. The effective behaviour is already correct (large responses are rejected), but an explicit raise at the cap-exceeded check would make the failure mode unambiguous and remove the misleading "truncating" language from the warning.

Confidence Score: 5/5

Safe to merge — the cap is enforced correctly and observability is in place; the truncation style issue is minor.

The single remaining finding is a P2 style/clarity concern: the truncation path always produces a JSONDecodeError anyway, so the effective security behaviour is already correct. There are no correctness, data-integrity, or security regressions introduced by this PR.

frontends/aiq_api/src/aiq_api/auth/jwt_validator.py — consider replacing the truncation step with an explicit raise on both overflow sites.

Important Files Changed

Filename Overview
frontends/aiq_api/src/aiq_api/auth/jwt_validator.py Adds 64 KB read cap to OIDC discovery and JWKS fetch calls with warning logging; truncation logic is misleading since the resulting byte slice is always invalid JSON and will raise JSONDecodeError.

Sequence Diagram

sequenceDiagram
    participant Client
    participant JWTValidator
    participant OIDCEndpoint as OIDC Discovery
    participant JWKSEndpoint as JWKS Endpoint

    Client->>JWTValidator: validate(token)
    JWTValidator->>JWTValidator: _get_signing_key(token)

    alt JWKS URI not cached
        JWTValidator->>OIDCEndpoint: GET /.well-known/openid-configuration
        OIDCEndpoint-->>JWTValidator: response bytes
        alt len(raw) > 64 KB
            JWTValidator->>JWTValidator: log WARNING + truncate → JSONDecodeError
            JWTValidator-->>Client: None (auth failure)
        else len(raw) <= 64 KB
            JWTValidator->>JWTValidator: json.loads(raw) → cache jwks_uri
        end
    end

    JWTValidator->>JWKSEndpoint: GET {jwks_uri}
    JWKSEndpoint-->>JWTValidator: response bytes
    alt len(raw) > 64 KB
        JWTValidator->>JWTValidator: log WARNING + truncate → JSONDecodeError
        JWTValidator-->>Client: None (auth failure)
    else len(raw) <= 64 KB
        JWTValidator->>JWTValidator: parse keys, cache
        JWTValidator->>JWTValidator: jwt.decode(token, signing_key)
        JWTValidator-->>Client: claims dict
    end
Loading

Reviews (2): Last reviewed commit: "add failure log" | Re-trigger Greptile

Comment thread frontends/aiq_api/src/aiq_api/auth/jwt_validator.py Outdated

@AjayThorve AjayThorve 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.

LGTM

@AjayThorve
AjayThorve merged commit 287f7d5 into NVIDIA-AI-Blueprints:develop Apr 14, 2026
4 checks passed
@cdgamarose-nv
cdgamarose-nv deleted the cdgamarose/fix_auth_cve branch April 15, 2026 20:22
taylorjordanNC pushed a commit to taylorjordanNC/rh-research that referenced this pull request May 27, 2026
* set size cap on reads

* add failure log
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.

2 participants