fix: correct application.yml encoding to resolve SnakeYAML ReaderExce… - #7
fix: correct application.yml encoding to resolve SnakeYAML ReaderExce…#7Gyaan507 wants to merge 13 commits into
Conversation
|
Warning Review limit reached
Next review available in: 51 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe changes add OIDC/JWKS JWT fallback validation, expand Vault synchronization for Redis and service credentials, relocate email extractor Helm environment values, and update Cloudinary secret paths while preserving its effective application settings. ChangesConfiguration and Vault path updates
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
0444d71 to
1882b93
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/sync-vault.ps1`:
- Around line 93-94: Update the production Google write in the sync flow around
Write-VaultData so it does not persist placeholder credentials. Validate that
GOOGLE_CLIENT_ID and GOOGLE_CLIENT_SECRET are present and valid before calling
Write-VaultData for apps/prod/apps/docs/google, and fail fast or skip that write
when either input is missing; leave the preprod write unchanged.
- Around line 83-84: Update the sync flow around the Cloudinary, Google OAUTH,
and JWT_SECRET* Write-VaultData calls to keep preprod and production data
objects separate. Mirror the intended Vault/KV structure, populate production
values only from production-specific inputs, and fail closed when those inputs
are missing instead of reusing preprod process-level objects for apps/prod
paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c92138af-147d-41e2-a709-a8f702fc83f0
📒 Files selected for processing (3)
scripts/sync-vault.ps1services/am-email-extractor/helm/values.preprod.yamlservices/am-email-extractor/helm/values.prod.yaml
…rom am-email-extractor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
services/am-email-extractor/app_api.py (1)
103-104: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReuse one JWKS client for the OIDC fallback.
The
PyJWKClientinstantiated inrequire_jwtgets a per-request JWKS cache that is never reused; authenticated requests withOIDC_JWKS_URLcan fetch JWKS and wait on the default 30s timeout. Move client creation outside the request loop and add a small cache/lifespan.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@services/am-email-extractor/app_api.py` around lines 103 - 104, Update the OIDC fallback in require_jwt to reuse a single PyJWKClient instead of creating one per request. Initialize the client outside the request-handling loop and add a small cache/lifespan configuration so JWKS responses are reused while preserving the existing signing_key validation flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.env.example:
- Around line 43-45: Replace the machine-specific values of BACKUP_PATH and
KUBECONFIG_PATH in the environment example with portable placeholder Windows
paths, such as C:\path\to\backup.json and C:\path\to\kubeconfig, so
sync-vault.ps1 can be copied and used on other machines.
In `@scripts/sync-vault.ps1`:
- Around line 98-99: Update Write-VaultData destinations in
scripts/sync-vault.ps1 to apps/preprod/shared/google and apps/prod/shared/google
so they match the Helm consumers. Retain the existing shared/google paths in
services/am-email-extractor/helm/values.preprod.yaml (lines 34-36) and
services/am-email-extractor/helm/values.prod.yaml (lines 33-35); those sites
require no direct change.
- Around line 106-122: Validate OIDC_JWKS_URL, OIDC_ISSUER,
AM_DOC_INTELLIGENCE_CLIENT_ID, and AM_DOC_INTELLIGENCE_CLIENT_SECRET before
constructing or writing either service object in the sync flow. Stop with an
error when any value is unset or empty, ensuring no Write-VaultData calls for
these services occur with incomplete secrets.
In `@services/am-email-extractor/app_api.py`:
- Around line 107-114: Update the JWT decoding flow around decode_kwargs and
jwt.decode to require the configured issuer, expected audience, and expiration
claim. Remove the verify_aud disablement, always pass issuer and audience to
jwt.decode, and configure required claims to include exp while preserving RS256
signature validation.
---
Nitpick comments:
In `@services/am-email-extractor/app_api.py`:
- Around line 103-104: Update the OIDC fallback in require_jwt to reuse a single
PyJWKClient instead of creating one per request. Initialize the client outside
the request-handling loop and add a small cache/lifespan configuration so JWKS
responses are reused while preserving the existing signing_key validation flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 885a817f-5084-477b-a021-d473c1183564
📒 Files selected for processing (8)
.env.examplescripts/sync-vault.ps1services/am-email-extractor/app_api.pyservices/am-email-extractor/helm/values.dev.yamlservices/am-email-extractor/helm/values.preprod.yamlservices/am-email-extractor/helm/values.prod.yamlservices/am-email-extractor/helm/values.yamlservices/am-email-extractor/helm/vault-mappings.yaml
| VAULT_ROOT_TOKEN=your_vault_root_token | ||
| BACKUP_PATH=C:\Users\drabh\Documents\am-repo\vps\vps_vault_full_backup_20260422_000034.json | ||
| KUBECONFIG_PATH=C:\Users\drabh\Documents\am-repo\vps\kubeconfig.vps |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace machine-specific filesystem paths with placeholders.
These paths expose a local username/layout and cause sync-vault.ps1 to fail when copied elsewhere. Use portable placeholder paths such as C:\path\to\kubeconfig.
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 44-44: [UnorderedKey] The BACKUP_PATH key should go before the VAULT_ROOT_TOKEN key
(UnorderedKey)
[warning] 45-45: [UnorderedKey] The KUBECONFIG_PATH key should go before the VAULT_ROOT_TOKEN key
(UnorderedKey)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.env.example around lines 43 - 45, Replace the machine-specific values of
BACKUP_PATH and KUBECONFIG_PATH in the environment example with portable
placeholder Windows paths, such as C:\path\to\backup.json and
C:\path\to\kubeconfig, so sync-vault.ps1 can be copied and used on other
machines.
| Write-VaultData "apps/preprod/apps/docs/google" $GoogleData | ||
| Write-VaultData "apps/prod/apps/docs/google" $GoogleData |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Align Google Vault destinations with the Helm consumers.
The sync script writes apps/*/apps/docs/google, while the preprod and production releases read apps/data/*/shared/google. Those pods will not receive the credentials written by this script.
scripts/sync-vault.ps1#L98-L99: write toapps/preprod/shared/googleandapps/prod/shared/google.services/am-email-extractor/helm/values.preprod.yaml#L34-L36: retain this path after aligning the producer.services/am-email-extractor/helm/values.prod.yaml#L33-L35: retain this path after aligning the producer.
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] Missing BOM encoding for non-ASCII encoded file 'sync-vault.ps1'
(PSUseBOMForUnicodeEncodedFile)
📍 Affects 3 files
scripts/sync-vault.ps1#L98-L99(this comment)services/am-email-extractor/helm/values.preprod.yaml#L34-L36services/am-email-extractor/helm/values.prod.yaml#L33-L35
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/sync-vault.ps1` around lines 98 - 99, Update Write-VaultData
destinations in scripts/sync-vault.ps1 to apps/preprod/shared/google and
apps/prod/shared/google so they match the Helm consumers. Retain the existing
shared/google paths in services/am-email-extractor/helm/values.preprod.yaml
(lines 34-36) and services/am-email-extractor/helm/values.prod.yaml (lines
33-35); those sites require no direct change.
| # am-identity — provides OIDC config (JWKS URL + Issuer) used for JWT validation | ||
| $IdentityData = [PSCustomObject]@{ | ||
| OIDC_JWKS_URL = $env:OIDC_JWKS_URL | ||
| OIDC_ISSUER = $env:OIDC_ISSUER | ||
| } | ||
| Write-VaultData "apps/dev/services/am-identity" $IdentityData | ||
| Write-VaultData "apps/preprod/services/am-identity" $IdentityData | ||
| Write-VaultData "apps/prod/services/am-identity" $IdentityData | ||
|
|
||
| # am-doc-intelligence — provides OAuth2 client credentials for service-to-service auth | ||
| $DocIntelData = [PSCustomObject]@{ | ||
| AM_DOC_INTELLIGENCE_CLIENT_ID = $env:AM_DOC_INTELLIGENCE_CLIENT_ID | ||
| AM_DOC_INTELLIGENCE_CLIENT_SECRET = $env:AM_DOC_INTELLIGENCE_CLIENT_SECRET | ||
| } | ||
| Write-VaultData "apps/dev/services/am-doc-intelligence" $DocIntelData | ||
| Write-VaultData "apps/preprod/services/am-doc-intelligence" $DocIntelData | ||
| Write-VaultData "apps/prod/services/am-doc-intelligence" $DocIntelData |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Fail before writing incomplete service secrets.
Unset OIDC or doc-intelligence variables become null properties; Write-VaultData then writes partial data (or invokes vault kv put with no values). Validate OIDC_JWKS_URL and both doc-intelligence credentials before any environment write, and stop on missing values.
🧰 Tools
🪛 PSScriptAnalyzer (1.25.0)
[warning] Missing BOM encoding for non-ASCII encoded file 'sync-vault.ps1'
(PSUseBOMForUnicodeEncodedFile)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/sync-vault.ps1` around lines 106 - 122, Validate OIDC_JWKS_URL,
OIDC_ISSUER, AM_DOC_INTELLIGENCE_CLIENT_ID, and
AM_DOC_INTELLIGENCE_CLIENT_SECRET before constructing or writing either service
object in the sync flow. Stop with an error when any value is unset or empty,
ensuring no Write-VaultData calls for these services occur with incomplete
secrets.
| decode_kwargs = { | ||
| "algorithms": ["RS256"], | ||
| "options": {"verify_aud": False} | ||
| } | ||
| if issuer: | ||
| decode_kwargs["issuer"] = issuer | ||
|
|
||
| payload = jwt.decode(token, signing_key.key, **decode_kwargs) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C2 'OIDC_(JWKS_URL|ISSUER|AUDIENCE)|jwt\.decode|PyJWKClient' \
services/am-email-extractor .env.example
fd -HI -t f . | rg '/(requirements.*\.txt|pyproject\.toml|Pipfile(\.lock)?)$' \
| xargs -r rg -n -i 'pyjwt|jwt'Repository: AM-Portfolio/am-doc-intelligence
Length of output: 3565
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== app_api.py relevant section =="
sed -n '1,150p' services/am-email-extractor/app_api.py | nl -ba | sed -n '80,125p'
echo
echo "== env/config references =="
rg -n -C2 'OIDC_|JWT_|requirement|req|auth' services/am-email-extractor .env.example helm 2>/dev/null || true
echo
echo "== PyJWT docs/package docs if installed =="
python3 - <<'PY'
import subprocess, sys
proc = subprocess.run([sys.executable, '-m', 'pip', 'show', 'PyJWT'], stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True)
print("pip show status:", proc.returncode)
print(proc.stdout)
print(proc.stderr)
PY
echo
echo "== JWT decode option semantics from local PyJWT if available =="
python3 - <<'PY'
try:
import jwt
print("PyJWT version:", jwt.__version__)
print(jwt.__file__)
help(jwt.decode)
except Exception as e:
print("PyJWT unavailable/import error:", repr(e))
PYRepository: AM-Portfolio/am-doc-intelligence
Length of output: 244
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== app_api.py relevant section =="
sed -n '1,150p' services/am-email-extractor/app_api.py | awk '{printf "%6d\t%s\n", NR, $0}' | sed -n '80,130p'
echo
echo "== env/config references =="
rg -n -C2 'OIDC_|JWT_|requirement|req|auth' services/am-email-extractor .env.example helm 2>/dev/null || true
echo
echo "== PyJWT package docs if available =="
python3 - <<'PY'
import subprocess, sys
proc = subprocess.run([sys.executable, '-m', 'pip', 'show', 'PyJWT'], stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True)
print("pip show status:", proc.returncode)
print(proc.stdout or "")
print(proc.stderr or "")
PY
echo
echo "== JWT decode option semantics from local PyJWT if available =="
python3 - <<'PY'
try:
import jwt
print("PyJWT version:", jwt.__version__)
print(jwt.__file__)
help(jwt.decode)
except Exception as e:
print("PyJWT unavailable/import error:", repr(e))
PYRepository: AM-Portfolio/am-doc-intelligence
Length of output: 50390
🌐 Web query:
PyJWT 2.8.0 decode PyJWKClient validate issuer aud exp verify options audience
💡 Result:
In PyJWT 2.8.0, validation of claims such as audience (aud), issuer (iss), and expiration (exp) is controlled through the options parameter in the jwt.decode function [1][2]. Validation Logic By default, when you pass specific values for audience or issuer to jwt.decode, the library validates these claims against the token [1][3]. If a claim is present in the token but does not match the provided value, validation fails [1][3]. Key decoding options: - verify_aud: Boolean. If True (default), checks that the aud claim matches the audience parameter [1]. - verify_iss: Boolean. If True (default), checks that the iss claim matches the issuer parameter [1]. - verify_exp: Boolean. If True (default), checks that the exp claim is in the future [1][2]. - strict_aud: Boolean. When enabled, checks that the aud claim is a single value (not a list) and matches the audience exactly [1][3]. - require: A list of claims that must be present in the token (e.g., ["exp", "aud"]) [1][3]. If a required claim is missing, a MissingRequiredClaimError is raised [4][5]. Usage with PyJWKClient When using PyJWKClient to retrieve keys, you typically pass the resolved key into jwt.decode [6]. Validation parameters are applied during the decode step [6]: import jwt from jwt import PyJWKClient url = "https://example.com/.well-known/jwks.json" client = PyJWKClient(url) signing_key = client.get_signing_key_from_jwt(token) data = jwt.decode( token, key=signing_key.key, algorithms=["RS256"], audience="my-audience", issuer="my-issuer", options={"verify_exp": True, "verify_aud": True}) If you need to disable validation for a specific claim—for example, if a token does not always contain the audience claim—you can explicitly set the corresponding verify option to False [2][7]: options = {"verify_aud": False} jwt.decode(token, key=signing_key.key, options=options,...) Note: Claims like exp are only verified if they are present in the token unless they are explicitly added to the require list [1][3]. If you provide an audience or issuer value but the claim is missing from the token, PyJWT will raise a MissingRequiredClaimError [4][5].
Citations:
- 1: https://pyjwt.readthedocs.io/en/2.8.0/api.html
- 2: https://pyjwt.readthedocs.io/en/2.8.0/usage.html
- 3: https://pyjwt.readthedocs.io/en/2.10.0/api.html
- 4: https://pyjwt.readthedocs.io/en/2.8.0/changelog.html
- 5: https://github.com/jpadilla/pyjwt/blob/2.8.0/CHANGELOG.rst
- 6: https://pyjwt.readthedocs.io/en/stable/usage.html
- 7: JWT decode requires audience jpadilla/pyjwt#870
Require issuer, audience, and expiry for OIDC JWT decoding.
The fallback disables aud validation and only optionally checks issuer, so a token signed by the same JWKS for another client/resource can be accepted. Configure the expected audience, pass both issuer and audience to jwt.decode, and require exp validation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/am-email-extractor/app_api.py` around lines 107 - 114, Update the
JWT decoding flow around decode_kwargs and jwt.decode to require the configured
issuer, expected audience, and expiration claim. Remove the verify_aud
disablement, always pass issuer and audience to jwt.decode, and configure
required claims to include exp while preserving RS256 signature validation.
… SSL verification
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@services/am-email-extractor/app_api.py`:
- Around line 132-139: Update the JWT validation exception handler to return a
generic 401 error without including the exception type or message. Replace
traceback printing with concise classified logging for invalid tokens, while
preserving appropriate handling for explicitly structured status_code/detail
exceptions if required.
- Around line 103-108: Update the JWKS client initialization near
jwt.PyJWKClient to remove the custom SSL context that disables certificate and
hostname verification. Omit ssl_context or use the default verified trust chain
so JWKS retrieval validates the server’s TLS certificate and hostname.
In `@services/am-email-extractor/helm/vault-mappings.yaml`:
- Around line 11-14: Update the jwt.mappings entries in vault-mappings.yaml so
SESSION_SECRET references a dedicated Vault secret field rather than JWT_SECRET,
while leaving JWT_SECRET mapped to its existing field and preserving the
environment variable names consumed by app.py and app_api.py.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9d2e0dc1-59ba-494a-8bca-e7d061a51687
📒 Files selected for processing (5)
services/am-cloudinary-manager/helm/values.dev.yamlservices/am-cloudinary-manager/helm/values.yamlservices/am-email-extractor/app_api.pyservices/am-email-extractor/helm/values.yamlservices/am-email-extractor/helm/vault-mappings.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- services/am-email-extractor/helm/values.yaml
| import ssl | ||
| ssl_context = ssl.create_default_context() | ||
| ssl_context.check_hostname = False | ||
| ssl_context.verify_mode = ssl.CERT_NONE | ||
|
|
||
| jwks_client = jwt.PyJWKClient(jwks_url, ssl_context=ssl_context) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file excerpt =="
sed -n '80,130p' services/am-email-extractor/app_api.py
echo "== JWT import and PyJWKClient usages =="
rg -n "jwt\.PyJWKClient|verify_mode|check_hostname|create_default_context|jwt\." services/am-email-extractor/app_api.py || true
echo "== dependencies mentioning PyJWT / aiohttp / requests =="
for f in $(git ls-files | rg '(^|/)(pyproject\.toml|requirements.*\.txt|setup\.py|Poetry\.lock|Pipfile|Pipfile\.lock|uv\.lock)$'); do
echo "--- $f"
rg -n "PyJWT|pyjwt|PyJWKClient|aiohttp|requests|httpx|urllib" "$f" || true
doneRepository: AM-Portfolio/am-doc-intelligence
Length of output: 5581
🌐 Web query:
PyJWT 2.8.0 PyJWKClient ssl_context parameter signature
💡 Result:
In PyJWT 2.8.0, the ssl_context parameter was introduced to the PyJWKClient constructor to allow passing a custom ssl.SSLContext object for JWKS endpoint requests [1][2]. The constructor signature for PyJWKClient in PyJWT 2.8.0 and subsequent versions is [3][4]: PyJWKClient(uri, cache_keys=False, max_cached_keys=16, cache_jwk_set=True, lifespan=300, headers=None, timeout=30, ssl_context=None) Parameters: - uri (str): The URL of the JWKS endpoint [3][4]. - cache_keys (bool): Enable the per-key LRU cache [3][4]. - max_cached_keys (int): Maximum number of signing keys to keep in the LRU cache [3][4]. - cache_jwk_set (bool): Enable the JWK Set response cache [3][4]. - lifespan (float): TTL in seconds for the JWK Set cache [3][4]. - headers (dict or None): Optional HTTP headers to include in requests [3][4]. - timeout (float): HTTP request timeout in seconds [3][4]. - ssl_context (ssl.SSLContext or None): Optional SSL context for the request [3][4]. This parameter allows users to configure SSL/TLS settings, such as custom CA certificates or specific protocol versions, when the client fetches the JSON Web Key Set [5].
Citations:
- 1: https://pyjwt.readthedocs.io/en/2.8.0/changelog.html
- 2: https://github.com/jpadilla/pyjwt/blob/2.8.0/CHANGELOG.rst
- 3: https://pyjwt.readthedocs.io/en/latest/api.html
- 4: https://pyjwt.readthedocs.io/en/stable/api.html
- 5: https://github.com/jpadilla/pyjwt/blob/d7710f7bcff4b195b293af554a32d4a461b07a01/jwt/jwks_client.py
Restore TLS verification for JWKS retrieval.
Disabling certificate and hostname verification lets a network attacker supply an attacker-controlled JWKS and mint tokens accepted by this fallback. Omit the custom context or use a normally verified trust chain.
Proposed fix
- import ssl
- ssl_context = ssl.create_default_context()
- ssl_context.check_hostname = False
- ssl_context.verify_mode = ssl.CERT_NONE
-
- jwks_client = jwt.PyJWKClient(jwks_url, ssl_context=ssl_context)
+ jwks_client = jwt.PyJWKClient(jwks_url)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import ssl | |
| ssl_context = ssl.create_default_context() | |
| ssl_context.check_hostname = False | |
| ssl_context.verify_mode = ssl.CERT_NONE | |
| jwks_client = jwt.PyJWKClient(jwks_url, ssl_context=ssl_context) | |
| jwks_client = jwt.PyJWKClient(jwks_url) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/am-email-extractor/app_api.py` around lines 103 - 108, Update the
JWKS client initialization near jwt.PyJWKClient to remove the custom SSL context
that disables certificate and hostname verification. Omit ssl_context or use the
default verified trust chain so JWKS retrieval validates the server’s TLS
certificate and hostname.
| except Exception as e: | ||
| import traceback | ||
| print(f"JWT Validation Error: {type(e).__name__}: {str(e)}", file=sys.stderr) | ||
| traceback.print_exc(file=sys.stderr) | ||
|
|
||
| if hasattr(e, 'status_code') and hasattr(e, 'detail'): | ||
| return jsonify({'error': getattr(e, 'detail')}), getattr(e, 'status_code') | ||
| return jsonify({'error': f'Invalid token: {str(e)}'}), 401 | ||
| return jsonify({'error': f'Invalid token: {type(e).__name__} - {str(e)}'}), 401 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg '(^|/)app_api\.py$|services/am-email-extractor' || true
echo
echo "Target snippet and surrounding code:"
if [ -f services/am-email-extractor/app_api.py ]; then
nl -ba services/am-email-extractor/app_api.py | sed -n '1,240p'
fi
echo
echo "JWT related imports/usage:"
rg -n "JWT|jwt|Invalid.*token|Token.*invalid|traceback|status_code|detail" services/am-email-extractor/app_api.py || trueRepository: AM-Portfolio/am-doc-intelligence
Length of output: 3820
🌐 Web query:
PyJWT JWTError status_code detail attributes
💡 Result:
The PyJWT library does not provide status_code or detail attributes on its exception classes [1][2]. These attributes are specific to the FastAPI/Starlette HTTPException class [3]. PyJWT exceptions (such as InvalidTokenError, ExpiredSignatureError, and DecodeError) are standard Python Exception objects that typically only contain an error message string [1][2][4]. When integrating PyJWT with frameworks like FastAPI, it is standard practice to catch PyJWT exceptions and manually wrap them in an HTTPException that provides the desired status_code and detail [3][5][6]. Example integration: from fastapi import HTTPException, status import jwt try: payload = jwt.decode(token, SECRET_KEY, algorithms=["HS256"]) except jwt.ExpiredSignatureError: raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Token expired") except jwt.InvalidTokenError as e: raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail=str(e)) In this pattern: 1. You catch specific PyJWT exceptions (or the base PyJWTError) [7][4]. 2. You explicitly construct an HTTPException, defining the status_code (e.g., 401) and the detail message [3][5]. 3. The detail message is often derived from the string representation of the caught PyJWT exception [5].
Citations:
- 1: https://github.com/jpadilla/pyjwt/blob/master/jwt/exceptions.py
- 2: https://pyjwt.readthedocs.io/en/stable/api.html
- 3: https://fastapi.tiangolo.com/tutorial/security/oauth2-jwt/
- 4: https://stackoverflow.com/questions/74747024/what-is-the-base-exception-for-an-invalid-jwt-in-pyjwt
- 5: https://theproductguy.in/blogs/jwt-python/
- 6: https://aldeacode.com/apps/utilities/jwt-decoder/python
- 7: Changing generic
except ExceptiontoPyJWTErrorauth0-blog/auth0-python-fastapi-sample#6
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Line count:"
wc -l services/am-email-extractor/app_api.py
echo
echo "Target file first 240 lines:"
awk '{printf "%6d %s\n", NR, $0}' services/am-email-extractor/app_api.py | sed -n '1,240p'
echo
echo "JWT-related searches:"
rg -n "jwt|JWT|token|traceback|status_code|detail|Invalid token|AuthenticationError|Unauthorized" services/am-email-extractor/app_api.py || true
echo
echo "Imports excerpt:"
awk '{printf "%6d %s\n", NR, $0}' services/am-email-extractor/app_api.py | sed -n '1,80p'Repository: AM-Portfolio/am-doc-intelligence
Length of output: 17619
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Behavioral probe only if jwt package is already available; otherwise report availability without importing repository code.
python3 - <<'PY'
import importlib.util
spec = importlib.util.find_spec("jwt")
print("jwt_available", bool(spec))
if spec:
import jwt as jwt_module
print("jwt_version", getattr(jwt_module, "__version__", "unknown"))
exceptions = [
"Error", "InvalidTokenError", "InvalidSignatureError", "DecodeError", "ExpiredSignatureError",
"InvalidAudienceError", "InvalidIssuedAtError", "ImmatureSignatureError", "InvalidAlgorithmError",
"MissingRequiredClaimError"
]
for name in exceptions:
cls = getattr(jwt_module, name, None)
if cls:
try:
exc = cls("test")
except TypeError:
exc = cls("test", "extra")
print(name, "has_status_code", hasattr(exc, "status_code"), "has_detail", hasattr(exc, "detail"), "attrs=", [a for a in dir(exc) if not a.startswith("_")])
PYRepository: AM-Portfolio/am-doc-intelligence
Length of output: 1169
Avoid leaking JWT validation details for invalid attacker-controlled tokens.
The catch-all handler prints a full traceback for JWT decode/validation errors and returns { 'error': f'Invalid token: <ExceptionType> - <message>' }, so malformed tokens feed log amplification and expose framework/library error details. Return a generic 401 response and log concise classified failures without tracebacks.
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 132-132: Do not catch blind exception: Exception
(BLE001)
[warning] 134-134: Use explicit conversion flag
Replace with conversion flag
(RUF010)
[warning] 138-138: Do not call getattr with a constant attribute value. It is not any safer than normal property access.
Replace getattr with attribute access
(B009)
[warning] 138-138: Do not call getattr with a constant attribute value. It is not any safer than normal property access.
Replace getattr with attribute access
(B009)
[warning] 139-139: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/am-email-extractor/app_api.py` around lines 132 - 139, Update the
JWT validation exception handler to return a generic 401 error without including
the exception type or message. Replace traceback printing with concise
classified logging for invalid tokens, while preserving appropriate handling for
explicitly structured status_code/detail exceptions if required.
Source: Linters/SAST tools
| jwt: | ||
| mappings: | ||
| JWT_SECRET: "JWT_SECRET" | ||
| SESSION_SECRET: "JWT_SECRET" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg 'services/am-email-extractor/(helm/vault-mappings.yaml|app.py|app_api.py)|vault-mappings.yaml|requirements|requirements.txt|poetry.lock|pyproject.toml' || true
echo
echo "Relevant config:"
if [ -f services/am-email-extractor/helm/vault-mappings.yaml ]; then
cat -n services/am-email-extractor/helm/vault-mappings.yaml
fi
echo
echo "Relevant app snippets:"
for f in services/am-email-extractor/app.py services/am-email-extractor/app_api.py; do
if [ -f "$f" ]; then
echo "--- $f"
cat -n "$f" | sed -n '1,120p'
fi
done
echo
echo "Search for SESSION_SECRET/JWT_SECRET references in app:"
rg -n "SESSION_SECRET|JWT_SECRET|SECRET|SECRET_KEY|jwt" services/am-email-extractor || trueRepository: AM-Portfolio/am-doc-intelligence
Length of output: 18549
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Find Vault mappings files mentioning JWT_SECRET / SESSION_SECRET:"
rg -n "JWT_SECRET|SESSION_SECRET|vault-mappings" . || true
echo
echo "Diff/stat if available:"
git diff --stat || true
git diff -- services/am-email-extractor/helm/vault-mappings.yaml services/am-email-extractor/app.py services/am-email-extractor/app_api.py || trueRepository: AM-Portfolio/am-doc-intelligence
Length of output: 2871
Use a dedicated Vault secret for Flask session signing.
SESSION_SECRET is used by app.py:15 as Flask SECRET_KEY for protected sessions, while this mapping assigns it the same Vault field as JWT_SECRET used by app_api.py:59 to validate JWTs. Keep them independent so a compromise of one authentication secret does not also break authentication across the API and session boundaries.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@services/am-email-extractor/helm/vault-mappings.yaml` around lines 11 - 14,
Update the jwt.mappings entries in vault-mappings.yaml so SESSION_SECRET
references a dedicated Vault secret field rather than JWT_SECRET, while leaving
JWT_SECRET mapped to its existing field and preserving the environment variable
names consumed by app.py and app_api.py.
…ption
Summary by CodeRabbit
GMAIL_REDIRECT_URI, plus JWT/Mongo/Kafka config keys) from app config to environment variables; added Vault mappings forJWT_SECRETandSESSION_SECRET..env.example.