fix: harden JWT secret defaults and replace eval() in examples (GHSA-3qg8-5g3r-79v5, GHSA-vg22-4gmj-prxw)#1793
Conversation
Addresses remaining attack surface from security advisories: GHSA-3qg8-5g3r-79v5 (JWT signing key - CRITICAL): auth_service.py: PLATFORM_ENV no longer defaults to 'dev' when unset. When PLATFORM_JWT_SECRET is not set and PLATFORM_ENV is not explicitly 'dev', an ephemeral random secret is auto-generated with a warning. This prevents production deployments from silently using the well-known 'dev-secret-change-me' key. GHSA-vg22-4gmj-prxw (unauthenticated eval - CRITICAL): Replaced all eval() calls in official examples and benchmarks with an AST-based safe math evaluator that only allows numeric literals and arithmetic operators. Files fixed: - examples/python/a2a/a2a-server.py (the primary advisory target) - examples/mcp/multi_server.py - examples/python/mcp/mcp-tools-server.py - examples/python/mcp/mcp-tools-server-sse.py - examples/mcp_server/custom_tools_server.py - examples/python/mcp_server_example.py - examples/python/custom_tools/tools_with_latency.py - examples/eval/reliability_example.py - src/praisonai-agents/benchmarks/real_benchmark.py The character-filter approach (allowlist '0123456789+-*/.()') was insufficient because () enables attribute access chains. AST-based evaluation only permits numeric literals and binary/unary arithmetic. GHSA-hvhp-v2gc-268q and GHSA-9cr9-25q5-8prj (file read/write): Already fixed in prior commits via _resolve_rule_path() and _resolve_cwd_yaml_path() containment helpers.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughThis PR contains two independent changes: a security hardening refactor that replaces unsafe ChangesSafe Expression Evaluation Refactor
JWT Secret Initialization Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
@copilot Do a thorough review of this PR. Read ALL existing reviewer comments above from Qodo, Coderabbit, and Gemini first — incorporate their findings. Review areas:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces unsafe eval() calls with custom AST-based safe evaluators across several example and benchmark calculator tools, and updates the JWT secret handling in auth_service.py to auto-generate ephemeral keys when the secret is missing in non-dev environments. Feedback highlights a reliability issue with the ephemeral JWT keys in multi-instance deployments, recommending a fail-fast approach instead. Additionally, the AST evaluators are flagged for a potential Denial of Service (DoS) vulnerability due to the inclusion of the exponentiation operator (ast.Pow), which should be removed.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| else: | ||
| # Either production or environment not specified. | ||
| # Auto-generate an ephemeral secret so the service can start, but | ||
| # warn loudly because tokens will not survive a restart. | ||
| import secrets as _secrets | ||
| import warnings as _warnings | ||
|
|
||
| JWT_SECRET = _secrets.token_urlsafe(32) | ||
| _warnings.warn( | ||
| "PLATFORM_JWT_SECRET is not set. Auto-generated an ephemeral " | ||
| "signing key — JWT tokens will be invalidated on restart. " | ||
| "Set PLATFORM_JWT_SECRET to a stable random value for production " | ||
| "use, or set PLATFORM_ENV=dev to use the development default.", | ||
| stacklevel=2, | ||
| ) |
There was a problem hiding this comment.
Auto-generating an ephemeral random secret in production when PLATFORM_JWT_SECRET is missing can lead to severe reliability issues. In multi-instance deployments (e.g., behind a load balancer or in Kubernetes), each instance will generate a different key, causing random authentication failures (HTTP 401) for users. Additionally, any container restart or redeployment will invalidate all active sessions.
It is highly recommended to fail fast by raising a RuntimeError when running in production without a configured secret. This forces administrators to configure the secret correctly and prevents silent, hard-to-debug session issues.
| else: | |
| # Either production or environment not specified. | |
| # Auto-generate an ephemeral secret so the service can start, but | |
| # warn loudly because tokens will not survive a restart. | |
| import secrets as _secrets | |
| import warnings as _warnings | |
| JWT_SECRET = _secrets.token_urlsafe(32) | |
| _warnings.warn( | |
| "PLATFORM_JWT_SECRET is not set. Auto-generated an ephemeral " | |
| "signing key — JWT tokens will be invalidated on restart. " | |
| "Set PLATFORM_JWT_SECRET to a stable random value for production " | |
| "use, or set PLATFORM_ENV=dev to use the development default.", | |
| stacklevel=2, | |
| ) | |
| else: | |
| # Either production or environment not specified. | |
| # Fail fast to prevent silent multi-instance auth failures and session invalidation on restart. | |
| raise RuntimeError( | |
| "PLATFORM_JWT_SECRET must be set to a strong random value in production. " | |
| "Set PLATFORM_ENV=dev to suppress this check during development." | |
| ) |
| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | ||
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | ||
| ast.Mod: operator.mod, ast.Pow: operator.pow, | ||
| ast.USub: operator.neg, ast.UAdd: operator.pos} |
There was a problem hiding this comment.
Allowing the exponentiation operator (ast.Pow / **) in the safe evaluator introduces a Denial of Service (DoS) vulnerability. An attacker can supply an expression with nested exponentiation (e.g., 9**9**9**9 or 10**1000000000) that consumes excessive CPU and memory, potentially hanging or crashing the server.
Since this is a simple calculator tool, it is safest to remove ast.Pow from the allowed operators.
| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | |
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | |
| ast.Mod: operator.mod, ast.Pow: operator.pow, | |
| ast.USub: operator.neg, ast.UAdd: operator.pos} | |
| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | |
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | |
| ast.Mod: operator.mod, | |
| ast.USub: operator.neg, ast.UAdd: operator.pos} |
| _OPS = { | ||
| ast.Add: operator.add, | ||
| ast.Sub: operator.sub, | ||
| ast.Mult: operator.mul, | ||
| ast.Div: operator.truediv, | ||
| ast.FloorDiv: operator.floordiv, | ||
| ast.Mod: operator.mod, | ||
| ast.Pow: operator.pow, | ||
| ast.USub: operator.neg, | ||
| ast.UAdd: operator.pos, | ||
| } |
There was a problem hiding this comment.
Allowing the exponentiation operator (ast.Pow / **) in the safe evaluator introduces a Denial of Service (DoS) vulnerability. An attacker can supply an expression with nested exponentiation (e.g., 9**9**9**9 or 10**1000000000) that consumes excessive CPU and memory, potentially hanging or crashing the server.
Since this is a simple calculator tool, it is safest to remove ast.Pow from the allowed operators.
_OPS = {
ast.Add: operator.add,
ast.Sub: operator.sub,
ast.Mult: operator.mul,
ast.Div: operator.truediv,
ast.FloorDiv: operator.floordiv,
ast.Mod: operator.mod,
ast.USub: operator.neg,
ast.UAdd: operator.pos,
}| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | ||
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | ||
| ast.Mod: operator.mod, ast.Pow: operator.pow, | ||
| ast.USub: operator.neg, ast.UAdd: operator.pos} |
There was a problem hiding this comment.
Allowing the exponentiation operator (ast.Pow / **) in the safe evaluator introduces a Denial of Service (DoS) vulnerability. An attacker can supply an expression with nested exponentiation (e.g., 9**9**9**9 or 10**1000000000) that consumes excessive CPU and memory, potentially hanging or crashing the server.
Since this is a simple calculator tool, it is safest to remove ast.Pow from the allowed operators.
| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | |
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | |
| ast.Mod: operator.mod, ast.Pow: operator.pow, | |
| ast.USub: operator.neg, ast.UAdd: operator.pos} | |
| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | |
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | |
| ast.Mod: operator.mod, | |
| ast.USub: operator.neg, ast.UAdd: operator.pos} |
| _OPS = { | ||
| ast.Add: operator.add, | ||
| ast.Sub: operator.sub, | ||
| ast.Mult: operator.mul, | ||
| ast.Div: operator.truediv, | ||
| ast.FloorDiv: operator.floordiv, | ||
| ast.Mod: operator.mod, | ||
| ast.Pow: operator.pow, | ||
| ast.USub: operator.neg, | ||
| ast.UAdd: operator.pos, | ||
| } |
There was a problem hiding this comment.
Allowing the exponentiation operator (ast.Pow / **) in the safe evaluator introduces a Denial of Service (DoS) vulnerability. An attacker can supply an expression with nested exponentiation (e.g., 9**9**9**9 or 10**1000000000) that consumes excessive CPU and memory, potentially hanging or crashing the server.
Since this is a simple calculator tool, it is safest to remove ast.Pow from the allowed operators.
_OPS = {
ast.Add: operator.add,
ast.Sub: operator.sub,
ast.Mult: operator.mul,
ast.Div: operator.truediv,
ast.FloorDiv: operator.floordiv,
ast.Mod: operator.mod,
ast.USub: operator.neg,
ast.UAdd: operator.pos,
}| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | ||
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | ||
| ast.Mod: operator.mod, ast.Pow: operator.pow, | ||
| ast.USub: operator.neg, ast.UAdd: operator.pos} |
There was a problem hiding this comment.
Allowing the exponentiation operator (ast.Pow / **) in the safe evaluator introduces a Denial of Service (DoS) vulnerability. An attacker can supply an expression with nested exponentiation (e.g., 9**9**9**9 or 10**1000000000) that consumes excessive CPU and memory, potentially hanging or crashing the server.
Since this is a simple calculator tool, it is safest to remove ast.Pow from the allowed operators.
| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | |
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | |
| ast.Mod: operator.mod, ast.Pow: operator.pow, | |
| ast.USub: operator.neg, ast.UAdd: operator.pos} | |
| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | |
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | |
| ast.Mod: operator.mod, | |
| ast.USub: operator.neg, ast.UAdd: operator.pos} |
| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | ||
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | ||
| ast.Mod: operator.mod, ast.Pow: operator.pow, | ||
| ast.USub: operator.neg, ast.UAdd: operator.pos} |
There was a problem hiding this comment.
Allowing the exponentiation operator (ast.Pow / **) in the safe evaluator introduces a Denial of Service (DoS) vulnerability. An attacker can supply an expression with nested exponentiation (e.g., 9**9**9**9 or 10**1000000000) that consumes excessive CPU and memory, potentially hanging or crashing the server.
Since this is a simple calculator tool, it is safest to remove ast.Pow from the allowed operators.
| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | |
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | |
| ast.Mod: operator.mod, ast.Pow: operator.pow, | |
| ast.USub: operator.neg, ast.UAdd: operator.pos} | |
| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | |
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | |
| ast.Mod: operator.mod, | |
| ast.USub: operator.neg, ast.UAdd: operator.pos} |
| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | ||
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | ||
| ast.Mod: operator.mod, ast.Pow: operator.pow, | ||
| ast.USub: operator.neg, ast.UAdd: operator.pos} |
There was a problem hiding this comment.
Allowing the exponentiation operator (ast.Pow / **) in the safe evaluator introduces a Denial of Service (DoS) vulnerability. An attacker can supply an expression with nested exponentiation (e.g., 9**9**9**9 or 10**1000000000) that consumes excessive CPU and memory, potentially hanging or crashing the server.
Since this is a simple calculator tool, it is safest to remove ast.Pow from the allowed operators.
| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | |
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | |
| ast.Mod: operator.mod, ast.Pow: operator.pow, | |
| ast.USub: operator.neg, ast.UAdd: operator.pos} | |
| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | |
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | |
| ast.Mod: operator.mod, | |
| ast.USub: operator.neg, ast.UAdd: operator.pos} |
| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | ||
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | ||
| ast.Mod: operator.mod, ast.Pow: operator.pow, | ||
| ast.USub: operator.neg, ast.UAdd: operator.pos} |
There was a problem hiding this comment.
Allowing the exponentiation operator (ast.Pow / **) in the safe evaluator introduces a Denial of Service (DoS) vulnerability. An attacker can supply an expression with nested exponentiation (e.g., 9**9**9**9 or 10**1000000000) that consumes excessive CPU and memory, potentially hanging or crashing the server.
Since this is a simple calculator tool, it is safest to remove ast.Pow from the allowed operators.
| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | |
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | |
| ast.Mod: operator.mod, ast.Pow: operator.pow, | |
| ast.USub: operator.neg, ast.UAdd: operator.pos} | |
| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | |
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | |
| ast.Mod: operator.mod, | |
| ast.USub: operator.neg, ast.UAdd: operator.pos} |
| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | ||
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | ||
| ast.Mod: operator.mod, ast.Pow: operator.pow, | ||
| ast.USub: operator.neg, ast.UAdd: operator.pos} |
There was a problem hiding this comment.
Allowing the exponentiation operator (ast.Pow / **) in the safe evaluator introduces a Denial of Service (DoS) vulnerability. An attacker can supply an expression with nested exponentiation (e.g., 9**9**9**9 or 10**1000000000) that consumes excessive CPU and memory, potentially hanging or crashing the server.
Since this is a simple calculator tool, it is safest to remove ast.Pow from the allowed operators.
| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | |
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | |
| ast.Mod: operator.mod, ast.Pow: operator.pow, | |
| ast.USub: operator.neg, ast.UAdd: operator.pos} | |
| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | |
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | |
| ast.Mod: operator.mod, | |
| ast.USub: operator.neg, ast.UAdd: operator.pos} |
Greptile SummaryThis PR addresses two CRITICAL security advisories: it replaces
Confidence Score: 3/5This PR fixes real RCE and JWT-forgery attack surfaces, but introduces two new gaps — an empty-string JWT secret that bypasses every guard, and an unbounded exponentiation path in server-facing tools — that need to be closed before merging into production. The JWT hardening in auth_service.py correctly removes the silent dev-default fallback for unset environments, which is the core of the advisory. However, setting PLATFORM_JWT_SECRET to an empty string now walks straight through every guard and signs tokens with an empty key. The AST evaluator is a genuine improvement over eval() for blocking RCE, but whitelisting ast.Pow without magnitude limits means a single expression like 999 sent to any of the six network-facing MCP/A2A servers can exhaust memory and kill the process. auth_service.py (empty-string secret path) and the six network-facing calculator examples: a2a-server.py, mcp-tools-server.py, mcp-tools-server-sse.py, multi_server.py, custom_tools_server.py, mcp_server_example.py (unbounded exponentiation).
|
| Filename | Overview |
|---|---|
| src/praisonai-platform/praisonai_platform/services/auth_service.py | JWT secret hardening: removes silent default fallback, auto-generates ephemeral key with warning — but empty-string PLATFORM_JWT_SECRET bypasses all guards and the warning message is misleading when the default string is explicitly set. |
| examples/python/a2a/a2a-server.py | Replaces eval() with AST-based evaluator — correctly blocks attribute access and function calls, but ast.Pow is whitelisted without magnitude limits, enabling DoS via expressions like 999. |
| examples/mcp/multi_server.py | Same AST evaluator pattern as a2a-server.py; shares the same exponentiation DoS exposure. |
| examples/python/mcp/mcp-tools-server.py | eval() replaced with AST evaluator; same ast.Pow DoS exposure as other server examples. |
| examples/python/mcp/mcp-tools-server-sse.py | eval() replaced with AST evaluator; same ast.Pow DoS exposure. |
| examples/mcp_server/custom_tools_server.py | eval() replaced with AST evaluator inline inside main(); same ast.Pow DoS exposure. |
| examples/python/mcp_server_example.py | eval() replaced with AST evaluator; same ast.Pow DoS exposure. |
| examples/python/custom_tools/tools_with_latency.py | eval() replaced with AST evaluator; DoS exposure exists but this is not a network-facing server example, reducing practical risk. |
| examples/eval/reliability_example.py | eval() replaced with AST evaluator in a non-server eval harness; DoS exposure is lower risk here. |
| src/praisonai-agents/benchmarks/real_benchmark.py | eval() replaced with AST evaluator in benchmark tooling; change is correct and low-risk in this non-server context. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Module import] --> B{PLATFORM_JWT_SECRET set?}
B -- "None or == default" --> C{PLATFORM_ENV == 'dev'?}
B -- "non-empty, non-default string" --> G[✅ Use provided secret]
B -- "empty string ''" --> H[⚠️ Empty key used — guard bypassed]
C -- "yes (explicit dev)" --> D[Use _DEFAULT_SECRET\ndev-secret-change-me]
C -- "no / unset" --> E[Generate ephemeral\ntoken_urlsafe 32 + warn]
G --> F[JWT sign / verify]
D --> F
E --> F
H --> F
Reviews (1): Last reviewed commit: "fix: harden JWT secret handling and repl..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| JWT_SECRET = os.environ.get("PLATFORM_JWT_SECRET") | ||
| JWT_ALGORITHM = "HS256" | ||
| JWT_TTL_SECONDS = int(os.environ.get("PLATFORM_JWT_TTL", str(30 * 24 * 3600))) | ||
|
|
||
| if JWT_SECRET == _DEFAULT_SECRET and os.environ.get("PLATFORM_ENV", "dev") != "dev": | ||
| raise RuntimeError( | ||
| "PLATFORM_JWT_SECRET must be set to a strong random value in production. " | ||
| "Set PLATFORM_ENV=dev to suppress this check during development." | ||
| ) | ||
| if JWT_SECRET is None or JWT_SECRET == _DEFAULT_SECRET: |
There was a problem hiding this comment.
os.environ.get("PLATFORM_JWT_SECRET") returns "" when the variable is set to an empty string. Because "" is neither None nor equal to _DEFAULT_SECRET, the condition on line 30 is false, and the empty string silently becomes the JWT signing key. PyJWT accepts an empty HMAC key without error, so any production deployment that accidentally sets PLATFORM_JWT_SECRET= (empty) will sign tokens with no meaningful secret — effectively the same exposure this PR is hardening against.
| ast.Div: operator.truediv, | ||
| ast.FloorDiv: operator.floordiv, | ||
| ast.Mod: operator.mod, | ||
| ast.Pow: operator.pow, |
There was a problem hiding this comment.
ast.Pow is whitelisted without any guard on operand magnitude. An expression like 9**9**9 evaluates right-associatively as 9**(9**9) = 9**387420489, which Python will attempt to represent as an arbitrary-precision integer with roughly 370 million decimal digits (~1.5 GB of memory). Because operator.pow is called directly, there is no recursion depth limit or size check, so a single crafted request can OOM or stall any of the 9 server processes that include this evaluator (a2a-server.py, mcp-tools-server.py, mcp-tools-server-sse.py, multi_server.py, custom_tools_server.py, mcp_server_example.py, and others).
| JWT_SECRET = _secrets.token_urlsafe(32) | ||
| _warnings.warn( | ||
| "PLATFORM_JWT_SECRET is not set. Auto-generated an ephemeral " | ||
| "signing key — JWT tokens will be invalidated on restart. " | ||
| "Set PLATFORM_JWT_SECRET to a stable random value for production " | ||
| "use, or set PLATFORM_ENV=dev to use the development default.", | ||
| stacklevel=2, | ||
| ) |
There was a problem hiding this comment.
PLATFORM_JWT_SECRET=dev-secret-change-me will see a confusing message claiming the variable is missing, potentially masking what the code actually rejected.
| JWT_SECRET = _secrets.token_urlsafe(32) | |
| _warnings.warn( | |
| "PLATFORM_JWT_SECRET is not set. Auto-generated an ephemeral " | |
| "signing key — JWT tokens will be invalidated on restart. " | |
| "Set PLATFORM_JWT_SECRET to a stable random value for production " | |
| "use, or set PLATFORM_ENV=dev to use the development default.", | |
| stacklevel=2, | |
| ) | |
| JWT_SECRET = _secrets.token_urlsafe(32) | |
| _warnings.warn( | |
| "PLATFORM_JWT_SECRET is not set or is set to the insecure default. " | |
| "Auto-generated an ephemeral signing key — JWT tokens will be " | |
| "invalidated on restart. Set PLATFORM_JWT_SECRET to a stable random " | |
| "value for production use, or set PLATFORM_ENV=dev to use the " | |
| "development default.", | |
| stacklevel=1, | |
| ) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 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 `@examples/eval/reliability_example.py`:
- Around line 25-30: The helper function _ev currently uses multiple single-line
if/return branches which trigger E701; expand each conditional into its own
multi-line block inside _ev (e.g., convert "if isinstance(n, ast.Expression):
return _ev(n.body)" into a standard two-line if with the return on the next
line) for all branches handling ast.Expression, ast.Constant, ast.UnaryOp, and
ast.BinOp (and keep the final raise ValueError as the fallback). Preserve the
same logic and the use of _OPS for operator handling while only changing the
formatting to satisfy the linter.
In `@examples/mcp_server/custom_tools_server.py`:
- Around line 32-38: The nested evaluator function _ev currently uses multiple
single-line if statements causing E701; expand each one-line branch into a
proper multi-line if/elif/else block inside the _ev function (handle
ast.Expression -> return _ev(n.body), ast.Constant numeric -> return n.value,
ast.UnaryOp with operator in _OPS -> return _OPS[type(n.op)](_ev(n.operand)),
ast.BinOp with operator in _OPS -> return _OPS[type(n.op)](_ev(n.left),
_ev(n.right)), and the final raise ValueError) so that every branch is on its
own lines while keeping usage of _OPS and ast.parse(expression, mode='eval')
unchanged.
In `@examples/mcp/multi_server.py`:
- Around line 45-54: In _safe_eval, the numeric-literal check currently allows
bools because isinstance(node.value, (int, float)) accepts True/False; change
the guard in the Constant branch to reject booleans by using an exact type check
or an explicit bool exclusion (e.g., replace isinstance(node.value, (int,
float)) with type(node.value) in (int, float) or add "and not
isinstance(node.value, bool)") so only real ints/floats are evaluated; update
the Constant branch in function _safe_eval accordingly.
In `@examples/python/a2a/a2a-server.py`:
- Around line 28-53: The eval allows unbounded exponentiation via ast.Pow in the
_OPS mapping and _eval, which can be abused; either remove ast.Pow from _OPS
(and any handling in _eval) to disable ** entirely for the server, or keep
ast.Pow but add strict pre-checks inside _eval before computing pow: validate
operand types and enforce numeric bounds (e.g., limit absolute base and exponent
to small integers and/or limit exponent >= 0 and <= a safe max like 10, and fail
if result would exceed a safe size) and catch/raise on violations; update
references in the _OPS dict and the _eval function to implement the chosen
mitigation.
In `@examples/python/custom_tools/tools_with_latency.py`:
- Around line 57-62: The helper function _ev currently treats booleans as
numbers because isinstance(n.value, (int, float)) accepts bool; update _ev to
explicitly reject bool (e.g., check type(n.value) is int or float or add "and
not isinstance(n.value, bool)") and expand the condensed one-line if-return
branches into multi-line blocks for readability and to satisfy Ruff E701; adjust
the branches handling ast.Expression, ast.Constant, ast.UnaryOp, and ast.BinOp
within _ev accordingly.
- Around line 51-63: The AST evaluator in calculate_with_tracking uses _ev and
_OPS but lacks bounds and treats bool as numeric; update _ev to enforce maximum
recursion depth and maximum AST node/operand count (e.g., MAX_DEPTH, MAX_NODES)
and track/abort when exceeded, restrict the numeric check to exact types (use
type(n.value) is int or type(n.value) is float) instead of isinstance, and limit
or sanitize the Pow operator (e.g., disallow ast.Pow or clamp exponents) to
prevent runaway CPU/memory usage.
In `@examples/python/mcp_server_example.py`:
- Around line 51-63: The evaluator exposed by ToolsMCPServer.calculate() is
vulnerable to DoS and accepts bools; update the _ev evaluator to (1) enforce
explicit caps before parsing/evaluating: limit input string length, maximum AST
node count/depth, and restrict numeric magnitude (e.g., max exponent/absolute
value); (2) reject booleans by checking type(n.value) is int/float but not bool
or explicitly isinstance(n.value, bool) and raising; (3) whitelist allowed AST
node types (Expression, Constant, UnaryOp, BinOp, specific operator classes) and
raise on anything else; and (4) fix single-line if returns in _ev to use
multi-line blocks to satisfy E701. Reference symbols: calculate(),
ToolsMCPServer, _ev, ast.parse, and the _OPS mapping when applying these guards.
- Around line 57-62: The helper _ev currently treats ast.Constant values using
isinstance(n.value, (int, float)) which wrongly accepts booleans (since bool is
a subclass of int) and uses single-line if statements flagged by E701; update
_ev to explicitly exclude bool (e.g., check that value is not isinstance(...,
bool) or use type(...) in (int, float)) and rewrite all single-line if
statements into multi-line blocks for readability and lint compliance, keeping
the existing handling for ast.Expression, ast.UnaryOp and ast.BinOp (using _OPS)
and preserving the final raise ValueError for unsupported nodes.
In `@examples/python/mcp/mcp-tools-server-sse.py`:
- Around line 44-49: The one-line branches inside the _ev function violate the
E701 rule; expand each conditional into a multi-line if/elif/else block so each
branch has its own indented body (e.g., replace "if isinstance(n,
ast.Expression): return _ev(n.body)" with a multi-line if block), and do the
same for the ast.Constant, ast.UnaryOp, ast.BinOp checks and the final raise
ValueError; keep the existing logic and use the same symbols (_ev,
ast.Expression, ast.Constant, ast.UnaryOp, ast.BinOp, _OPS) so behavior remains
unchanged.
- Around line 39-50: The SSE tool's evaluator (_ev) currently allows ast.Pow via
_OPS which enables expensive exponentiation (e.g., 9**9999999); either remove
ast.Pow from the _OPS dict or add strict guards in _ev that validate
exponentiation operands and result size before calling operator.pow (e.g.,
require integer small non-negative exponents, cap exponent value and/or result
magnitude), and raise ValueError for disallowed cases; update the _OPS/_ev logic
accordingly so ast.Pow is either absent or only executed after these checks.
In `@examples/python/mcp/mcp-tools-server.py`:
- Around line 55-60: The helper _ev currently accepts bools as numeric literals
and uses single-line if-statements that trigger Ruff E701; update _ev to
explicitly check the concrete type of numeric constants by replacing
"isinstance(n, ast.Constant) and isinstance(n.value, (int, float))" with
"isinstance(n, ast.Constant) and type(n.value) in (int, float)" to reject bools,
and expand all single-line if ...: return ... statements into multi-line if
blocks (e.g., "if ...:\n return ...") for the branches handling
ast.Expression, ast.Constant, ast.UnaryOp, and ast.BinOp that reference _OPS and
ast so the logic remains identical but conforms to style rules.
In `@src/praisonai-agents/benchmarks/real_benchmark.py`:
- Around line 89-94: The helper _ev should only accept real numeric constants
(not booleans) and must not use single-line if/return branches; update the
isinstance checks in _ev to verify numeric types while excluding bool (e.g.,
change "isinstance(n.value, (int, float))" to "isinstance(n.value, (int, float))
and not isinstance(n.value, bool)"), and expand each one-line conditional like
"if ...: return ..." into multi-line form with explicit return statements; keep
references to _ev and the operator map _OPS when modifying the UnaryOp and BinOp
branches.
In `@src/praisonai-platform/praisonai_platform/services/auth_service.py`:
- Around line 30-53: The current branch treats PLATFORM_ENV unset and explicitly
non-"dev" the same and auto-generates an ephemeral JWT_SECRET; instead, change
the logic in the JWT_SECRET initialization (the block guarded by JWT_SECRET is
None or JWT_SECRET == _DEFAULT_SECRET and the check of
os.environ.get("PLATFORM_ENV")) so that: if _explicit_env is None (unset) you
keep the existing behavior of auto-generating and warning, but if _explicit_env
is explicitly set and not "dev" (e.g., "prod") you fail-fast — raise an
exception (or exit) with a clear message telling the operator to set
PLATFORM_JWT_SECRET to a stable value or set PLATFORM_ENV=dev; reference
JWT_SECRET, _DEFAULT_SECRET and PLATFORM_ENV in the error handling so the code
refuses to start in explicit non-dev environments without a provided secret.
🪄 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
Run ID: e26c2a17-8c25-44d1-86a6-ef6901596470
📒 Files selected for processing (10)
examples/eval/reliability_example.pyexamples/mcp/multi_server.pyexamples/mcp_server/custom_tools_server.pyexamples/python/a2a/a2a-server.pyexamples/python/custom_tools/tools_with_latency.pyexamples/python/mcp/mcp-tools-server-sse.pyexamples/python/mcp/mcp-tools-server.pyexamples/python/mcp_server_example.pysrc/praisonai-agents/benchmarks/real_benchmark.pysrc/praisonai-platform/praisonai_platform/services/auth_service.py
| def _ev(n): | ||
| if isinstance(n, ast.Expression): return _ev(n.body) | ||
| if isinstance(n, ast.Constant) and isinstance(n.value, (int, float)): return n.value | ||
| if isinstance(n, ast.UnaryOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.operand)) | ||
| if isinstance(n, ast.BinOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.left), _ev(n.right)) | ||
| raise ValueError(f"Unsupported: {ast.dump(n)}") |
There was a problem hiding this comment.
Split the one-line evaluator branches.
Ruff is already flagging Lines 26-29 with E701, so this copy will fail the configured lint check until each branch is expanded onto its own lines.
Suggested cleanup
def _ev(n):
- if isinstance(n, ast.Expression): return _ev(n.body)
- if isinstance(n, ast.Constant) and isinstance(n.value, (int, float)): return n.value
- if isinstance(n, ast.UnaryOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.operand))
- if isinstance(n, ast.BinOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.left), _ev(n.right))
+ if isinstance(n, ast.Expression):
+ return _ev(n.body)
+ if isinstance(n, ast.Constant) and isinstance(n.value, (int, float)):
+ return n.value
+ if isinstance(n, ast.UnaryOp) and type(n.op) in _OPS:
+ return _OPS[type(n.op)](_ev(n.operand))
+ if isinstance(n, ast.BinOp) and type(n.op) in _OPS:
+ return _OPS[type(n.op)](_ev(n.left), _ev(n.right))
raise ValueError(f"Unsupported: {ast.dump(n)}")📝 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.
| def _ev(n): | |
| if isinstance(n, ast.Expression): return _ev(n.body) | |
| if isinstance(n, ast.Constant) and isinstance(n.value, (int, float)): return n.value | |
| if isinstance(n, ast.UnaryOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.operand)) | |
| if isinstance(n, ast.BinOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.left), _ev(n.right)) | |
| raise ValueError(f"Unsupported: {ast.dump(n)}") | |
| def _ev(n): | |
| if isinstance(n, ast.Expression): | |
| return _ev(n.body) | |
| if isinstance(n, ast.Constant) and isinstance(n.value, (int, float)): | |
| return n.value | |
| if isinstance(n, ast.UnaryOp) and type(n.op) in _OPS: | |
| return _OPS[type(n.op)](_ev(n.operand)) | |
| if isinstance(n, ast.BinOp) and type(n.op) in _OPS: | |
| return _OPS[type(n.op)](_ev(n.left), _ev(n.right)) | |
| raise ValueError(f"Unsupported: {ast.dump(n)}") |
🧰 Tools
🪛 Ruff (0.15.15)
[error] 26-26: Multiple statements on one line (colon)
(E701)
[error] 27-27: Multiple statements on one line (colon)
(E701)
[error] 28-28: Multiple statements on one line (colon)
(E701)
[error] 29-29: Multiple statements on one line (colon)
(E701)
🤖 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 `@examples/eval/reliability_example.py` around lines 25 - 30, The helper
function _ev currently uses multiple single-line if/return branches which
trigger E701; expand each conditional into its own multi-line block inside _ev
(e.g., convert "if isinstance(n, ast.Expression): return _ev(n.body)" into a
standard two-line if with the return on the next line) for all branches handling
ast.Expression, ast.Constant, ast.UnaryOp, and ast.BinOp (and keep the final
raise ValueError as the fallback). Preserve the same logic and the use of _OPS
for operator handling while only changing the formatting to satisfy the linter.
| def _ev(n): | ||
| if isinstance(n, ast.Expression): return _ev(n.body) | ||
| if isinstance(n, ast.Constant) and isinstance(n.value, (int, float)): return n.value | ||
| if isinstance(n, ast.UnaryOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.operand)) | ||
| if isinstance(n, ast.BinOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.left), _ev(n.right)) | ||
| raise ValueError(f"Unsupported: {ast.dump(n)}") | ||
| return f"Result: {_ev(ast.parse(expression, mode='eval'))}" |
There was a problem hiding this comment.
Expand the one-line if branches.
Ruff is already reporting E701 on Lines 33-36, so this block will keep failing lint until each branch is split onto its own lines.
🧰 Tools
🪛 Ruff (0.15.15)
[error] 33-33: Multiple statements on one line (colon)
(E701)
[error] 34-34: Multiple statements on one line (colon)
(E701)
[error] 35-35: Multiple statements on one line (colon)
(E701)
[error] 36-36: Multiple statements on one line (colon)
(E701)
🤖 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 `@examples/mcp_server/custom_tools_server.py` around lines 32 - 38, The nested
evaluator function _ev currently uses multiple single-line if statements causing
E701; expand each one-line branch into a proper multi-line if/elif/else block
inside the _ev function (handle ast.Expression -> return _ev(n.body),
ast.Constant numeric -> return n.value, ast.UnaryOp with operator in _OPS ->
return _OPS[type(n.op)](_ev(n.operand)), ast.BinOp with operator in _OPS ->
return _OPS[type(n.op)](_ev(n.left), _ev(n.right)), and the final raise
ValueError) so that every branch is on its own lines while keeping usage of _OPS
and ast.parse(expression, mode='eval') unchanged.
| def _safe_eval(node): | ||
| if isinstance(node, ast.Expression): | ||
| return _safe_eval(node.body) | ||
| if isinstance(node, ast.Constant) and isinstance(node.value, (int, float)): | ||
| return node.value | ||
| if isinstance(node, ast.UnaryOp) and type(node.op) in _OPS: | ||
| return _OPS[type(node.op)](_safe_eval(node.operand)) | ||
| if isinstance(node, ast.BinOp) and type(node.op) in _OPS: | ||
| return _OPS[type(node.op)](_safe_eval(node.left), _safe_eval(node.right)) | ||
| raise ValueError(f"Unsupported expression: {ast.dump(node)}") |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Python, is boola subclass ofint, and does isinstance(True, (int, float))returnTrue?
💡 Result:
Yes, in Python, bool is a subclass of int [1][2][3][4]. This relationship exists for backward compatibility, as boolean values were historically represented as 0 and 1 before the formal bool type was introduced [4][5]. Because bool is a subclass of int, the function isinstance(True, (int, float)) returns True [2][5]. This is because isinstance checks if the object is an instance of the specified class or any of its subclasses [6]. Since True is an instance of bool, and bool is a subclass of int, the check against int succeeds [2][4][5].
Citations:
- 1: https://docs.python.org/3/c-api/bool.html
- 2: bool variable isinstance of int python/cpython#90049
- 3: https://github.com/python/cpython/blob/main/Objects/boolobject.c
- 4: https://stackoverflow.com/questions/8169001/why-is-bool-a-subclass-of-int
- 5: https://stackoverflow.com/questions/37888620/comparing-boolean-and-int-using-isinstance
- 6: https://realpython.com/ref/builtin-functions/isinstance/
Exclude bool from numeric literal allowlist in _safe_eval
Line 48 uses isinstance(node.value, (int, float)), which accepts True/False because bool is a subclass of int. Use an exact type check (type(node.value) in (int, float)), or add an explicit not isinstance(node.value, bool) guard.
🤖 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 `@examples/mcp/multi_server.py` around lines 45 - 54, In _safe_eval, the
numeric-literal check currently allows bools because isinstance(node.value,
(int, float)) accepts True/False; change the guard in the Constant branch to
reject booleans by using an exact type check or an explicit bool exclusion
(e.g., replace isinstance(node.value, (int, float)) with type(node.value) in
(int, float) or add "and not isinstance(node.value, bool)") so only real
ints/floats are evaluated; update the Constant branch in function _safe_eval
accordingly.
| _OPS = { | ||
| ast.Add: operator.add, | ||
| ast.Sub: operator.sub, | ||
| ast.Mult: operator.mul, | ||
| ast.Div: operator.truediv, | ||
| ast.FloorDiv: operator.floordiv, | ||
| ast.Mod: operator.mod, | ||
| ast.Pow: operator.pow, | ||
| ast.USub: operator.neg, | ||
| ast.UAdd: operator.pos, | ||
| } | ||
|
|
||
| def _eval(node): | ||
| if isinstance(node, ast.Expression): | ||
| return _eval(node.body) | ||
| if isinstance(node, ast.Constant) and isinstance(node.value, (int, float)): | ||
| return node.value | ||
| if isinstance(node, ast.UnaryOp) and type(node.op) in _OPS: | ||
| return _OPS[type(node.op)](_eval(node.operand)) | ||
| if isinstance(node, ast.BinOp) and type(node.op) in _OPS: | ||
| return _OPS[type(node.op)](_eval(node.left), _eval(node.right)) | ||
| raise ValueError(f"Unsupported expression: {ast.dump(node)}") | ||
|
|
||
| try: | ||
| return f"Result: {eval(expression)}" | ||
| tree = ast.parse(expression, mode="eval") | ||
| return f"Result: {_eval(tree)}" |
There was a problem hiding this comment.
Bound expensive arithmetic before exposing this over A2A.
This server is network-reachable, and the unrestricted ** support here still lets a caller burn CPU or memory with inputs like 9**9999999. Please either remove ast.Pow from the allowlist for the server-facing examples or enforce strict operand/result limits before evaluating it.
Minimal hardening if exponentiation is not required
_OPS = {
ast.Add: operator.add,
ast.Sub: operator.sub,
ast.Mult: operator.mul,
ast.Div: operator.truediv,
ast.FloorDiv: operator.floordiv,
ast.Mod: operator.mod,
- ast.Pow: operator.pow,
ast.USub: operator.neg,
ast.UAdd: operator.pos,
}📝 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.
| _OPS = { | |
| ast.Add: operator.add, | |
| ast.Sub: operator.sub, | |
| ast.Mult: operator.mul, | |
| ast.Div: operator.truediv, | |
| ast.FloorDiv: operator.floordiv, | |
| ast.Mod: operator.mod, | |
| ast.Pow: operator.pow, | |
| ast.USub: operator.neg, | |
| ast.UAdd: operator.pos, | |
| } | |
| def _eval(node): | |
| if isinstance(node, ast.Expression): | |
| return _eval(node.body) | |
| if isinstance(node, ast.Constant) and isinstance(node.value, (int, float)): | |
| return node.value | |
| if isinstance(node, ast.UnaryOp) and type(node.op) in _OPS: | |
| return _OPS[type(node.op)](_eval(node.operand)) | |
| if isinstance(node, ast.BinOp) and type(node.op) in _OPS: | |
| return _OPS[type(node.op)](_eval(node.left), _eval(node.right)) | |
| raise ValueError(f"Unsupported expression: {ast.dump(node)}") | |
| try: | |
| return f"Result: {eval(expression)}" | |
| tree = ast.parse(expression, mode="eval") | |
| return f"Result: {_eval(tree)}" | |
| _OPS = { | |
| ast.Add: operator.add, | |
| ast.Sub: operator.sub, | |
| ast.Mult: operator.mul, | |
| ast.Div: operator.truediv, | |
| ast.FloorDiv: operator.floordiv, | |
| ast.Mod: operator.mod, | |
| ast.USub: operator.neg, | |
| ast.UAdd: operator.pos, | |
| } | |
| def _eval(node): | |
| if isinstance(node, ast.Expression): | |
| return _eval(node.body) | |
| if isinstance(node, ast.Constant) and isinstance(node.value, (int, float)): | |
| return node.value | |
| if isinstance(node, ast.UnaryOp) and type(node.op) in _OPS: | |
| return _OPS[type(node.op)](_eval(node.operand)) | |
| if isinstance(node, ast.BinOp) and type(node.op) in _OPS: | |
| return _OPS[type(node.op)](_eval(node.left), _eval(node.right)) | |
| raise ValueError(f"Unsupported expression: {ast.dump(node)}") | |
| try: | |
| tree = ast.parse(expression, mode="eval") | |
| return f"Result: {_eval(tree)}" |
🤖 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 `@examples/python/a2a/a2a-server.py` around lines 28 - 53, The eval allows
unbounded exponentiation via ast.Pow in the _OPS mapping and _eval, which can be
abused; either remove ast.Pow from _OPS (and any handling in _eval) to disable
** entirely for the server, or keep ast.Pow but add strict pre-checks inside
_eval before computing pow: validate operand types and enforce numeric bounds
(e.g., limit absolute base and exponent to small integers and/or limit exponent
>= 0 and <= a safe max like 10, and fail if result would exceed a safe size) and
catch/raise on violations; update references in the _OPS dict and the _eval
function to implement the chosen mitigation.
| try: | ||
| result = eval(expression) | ||
| import ast, operator | ||
| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | ||
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | ||
| ast.Mod: operator.mod, ast.Pow: operator.pow, | ||
| ast.USub: operator.neg, ast.UAdd: operator.pos} | ||
| def _ev(n): | ||
| if isinstance(n, ast.Expression): return _ev(n.body) | ||
| if isinstance(n, ast.Constant) and isinstance(n.value, (int, float)): return n.value | ||
| if isinstance(n, ast.UnaryOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.operand)) | ||
| if isinstance(n, ast.BinOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.left), _ev(n.right)) | ||
| raise ValueError(f"Unsupported: {ast.dump(n)}") | ||
| result = _ev(ast.parse(expression, mode="eval")) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant portion of the file with line numbers
FILE="examples/python/custom_tools/tools_with_latency.py"
nl -ba "$FILE" | sed -n '1,120p' > /tmp/ttls_top.txt
echo "=== $FILE (lines 1-120) ==="
cat /tmp/ttls_top.txt
# Extract calculate_with_tracking and any helper(s) for context
echo
echo "=== Search for calculate_with_tracking definition ==="
rg -n "def calculate_with_tracking|calculate_with_tracking" "$FILE"
# Look for expression validation or any MAX_* constants in the file
echo
echo "=== Search for expression bounds/validation patterns in the file ==="
rg -n "MAX_|len\(expression\)|expression.*(len|depth|pow|exp|bound)" "$FILE" || true
# Find other occurrences of Pow / AST evaluator in repo to see if this helper is reused
echo
echo "=== Search repo for the _ev(ast.parse(... mode=\"eval\")) pattern ==="
rg -n "_ev\\(ast\\.parse\\(expression, mode=\"eval\"\\)\\)|ast\\.Pow|Unsupported: \\{ast\\.dump\\(n\\)\\}" -S .Repository: MervinPraison/PraisonAI
Length of output: 110
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="examples/python/custom_tools/tools_with_latency.py"
echo "=== Showing file with line numbers around the referenced range (40-100) ==="
cat -n "$FILE" | sed -n '40,110p'
echo
echo "=== Find calculate_with_tracking definition and usage in this file ==="
rg -n "def calculate_with_tracking|calculate_with_tracking\\(" "$FILE" || true
echo
echo "=== Find the AST evaluator helper(s) inside the tool ==="
rg -n "_ev\\b|ast\\.parse\\(expression, mode=\"eval\"\\)|Unsupported: \\{ast\\.dump\\(n\\)\\}|_OPS\\b|ast\\.Pow" "$FILE" || true
echo
echo "=== Check for any existing complexity/length bounds in this file ==="
rg -n "MAX_|min\\(|max\\(|len\\(expression\\)|depth|recurs|exponent|pow\\b.*(bound|limit)|Expression too|Exponent too" "$FILE" || true
echo
echo "=== Check for isinstance(n.value, (int, float)) specifically ==="
rg -n "isinstance\\(n\\.value, \\(int, float\\)\\)|isinstance\\(.*n\\.value.*int" "$FILE" || true
echo
echo "=== Repo-wide: see if this evaluator pattern exists elsewhere (optional but helps) ==="
rg -n "_ev\\(ast\\.parse\\(expression, mode=\"eval\"\\)\\)|Unsupported: \\{ast\\.dump\\(n\\)\\}" -S . || trueRepository: MervinPraison/PraisonAI
Length of output: 5643
Add complexity/length limits to the AST expression evaluator in calculate_with_tracking
RCE is mitigated, but the evaluator still has no bounds (no MAX_*/depth/size checks in examples/python/custom_tools/tools_with_latency.py), so attacker-controlled expression can trigger CPU/memory exhaustion via deeply nested expressions and unrestricted ast.Pow.
isinstance(n.value, (int, float))also acceptsbool(sinceboolis a subclass ofint); restrict to exact numeric types.
Suggested guard rails
+ MAX_EXPRESSION_LENGTH = 200
+ MAX_DEPTH = 32
+ MAX_EXPONENT = 12
+
+ if len(expression) > MAX_EXPRESSION_LENGTH:
+ raise ValueError("Expression too long")
+
- def _ev(n):
- if isinstance(n, ast.Expression): return _ev(n.body)
+ def _ev(n, depth=0):
+ if depth > MAX_DEPTH:
+ raise ValueError("Expression too complex")
+ if isinstance(n, ast.Expression):
+ return _ev(n.body, depth + 1)
...
- if isinstance(n, ast.BinOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.left), _ev(n.right))
+ if isinstance(n, ast.BinOp) and type(n.op) in _OPS:
+ left = _ev(n.left, depth + 1)
+ right = _ev(n.right, depth + 1)
+ if isinstance(n.op, ast.Pow) and abs(right) > MAX_EXPONENT:
+ raise ValueError("Exponent too large")
+ return _OPS[type(n.op)](left, right)🧰 Tools
🪛 Ruff (0.15.15)
[error] 58-58: Multiple statements on one line (colon)
(E701)
[error] 59-59: Multiple statements on one line (colon)
(E701)
[error] 60-60: Multiple statements on one line (colon)
(E701)
[error] 61-61: Multiple statements on one line (colon)
(E701)
🤖 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 `@examples/python/custom_tools/tools_with_latency.py` around lines 51 - 63, The
AST evaluator in calculate_with_tracking uses _ev and _OPS but lacks bounds and
treats bool as numeric; update _ev to enforce maximum recursion depth and
maximum AST node/operand count (e.g., MAX_DEPTH, MAX_NODES) and track/abort when
exceeded, restrict the numeric check to exact types (use type(n.value) is int or
type(n.value) is float) instead of isinstance, and limit or sanitize the Pow
operator (e.g., disallow ast.Pow or clamp exponents) to prevent runaway
CPU/memory usage.
| import ast, operator | ||
| _OPS = {ast.Add: operator.add, ast.Sub: operator.sub, ast.Mult: operator.mul, | ||
| ast.Div: operator.truediv, ast.FloorDiv: operator.floordiv, | ||
| ast.Mod: operator.mod, ast.Pow: operator.pow, | ||
| ast.USub: operator.neg, ast.UAdd: operator.pos} | ||
| def _ev(n): | ||
| if isinstance(n, ast.Expression): return _ev(n.body) | ||
| if isinstance(n, ast.Constant) and isinstance(n.value, (int, float)): return n.value | ||
| if isinstance(n, ast.UnaryOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.operand)) | ||
| if isinstance(n, ast.BinOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.left), _ev(n.right)) | ||
| raise ValueError(f"Unsupported: {ast.dump(n)}") | ||
| return {"expression": expression, "result": _ev(ast.parse(expression, mode="eval"))} |
There was a problem hiding this comment.
Reject or cap exponentiation on the public SSE tool.
Because this server is exposed on 0.0.0.0, a client can send expressions such as 9**9999999 and tie up the process in operator.pow. If you need **, add strict literal/exponent/result limits first; otherwise the safer default is to drop ast.Pow from _OPS here.
🧰 Tools
🪛 Ruff (0.15.15)
[error] 45-45: Multiple statements on one line (colon)
(E701)
[error] 46-46: Multiple statements on one line (colon)
(E701)
[error] 47-47: Multiple statements on one line (colon)
(E701)
[error] 48-48: Multiple statements on one line (colon)
(E701)
🤖 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 `@examples/python/mcp/mcp-tools-server-sse.py` around lines 39 - 50, The SSE
tool's evaluator (_ev) currently allows ast.Pow via _OPS which enables expensive
exponentiation (e.g., 9**9999999); either remove ast.Pow from the _OPS dict or
add strict guards in _ev that validate exponentiation operands and result size
before calling operator.pow (e.g., require integer small non-negative exponents,
cap exponent value and/or result magnitude), and raise ValueError for disallowed
cases; update the _OPS/_ev logic accordingly so ast.Pow is either absent or only
executed after these checks.
| def _ev(n): | ||
| if isinstance(n, ast.Expression): return _ev(n.body) | ||
| if isinstance(n, ast.Constant) and isinstance(n.value, (int, float)): return n.value | ||
| if isinstance(n, ast.UnaryOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.operand)) | ||
| if isinstance(n, ast.BinOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.left), _ev(n.right)) | ||
| raise ValueError(f"Unsupported: {ast.dump(n)}") |
There was a problem hiding this comment.
Split the one-line evaluator branches.
Ruff is already flagging Lines 45-48 with E701, so this block will keep failing lint until each branch is expanded.
🧰 Tools
🪛 Ruff (0.15.15)
[error] 45-45: Multiple statements on one line (colon)
(E701)
[error] 46-46: Multiple statements on one line (colon)
(E701)
[error] 47-47: Multiple statements on one line (colon)
(E701)
[error] 48-48: Multiple statements on one line (colon)
(E701)
🤖 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 `@examples/python/mcp/mcp-tools-server-sse.py` around lines 44 - 49, The
one-line branches inside the _ev function violate the E701 rule; expand each
conditional into a multi-line if/elif/else block so each branch has its own
indented body (e.g., replace "if isinstance(n, ast.Expression): return
_ev(n.body)" with a multi-line if block), and do the same for the ast.Constant,
ast.UnaryOp, ast.BinOp checks and the final raise ValueError; keep the existing
logic and use the same symbols (_ev, ast.Expression, ast.Constant, ast.UnaryOp,
ast.BinOp, _OPS) so behavior remains unchanged.
| def _ev(n): | ||
| if isinstance(n, ast.Expression): return _ev(n.body) | ||
| if isinstance(n, ast.Constant) and isinstance(n.value, (int, float)): return n.value | ||
| if isinstance(n, ast.UnaryOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.operand)) | ||
| if isinstance(n, ast.BinOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.left), _ev(n.right)) | ||
| raise ValueError(f"Unsupported: {ast.dump(n)}") |
There was a problem hiding this comment.
Tighten numeric literal validation and address Ruff E701
- Reject
boolliterals explicitly (isinstance(True, int)isTrue), so onlytype(n.value)in(int, float)is allowed. - Split the
if ...: return ...one-liners to avoid RuffE701.
Suggested fix
def _ev(n):
- if isinstance(n, ast.Expression): return _ev(n.body)
- if isinstance(n, ast.Constant) and isinstance(n.value, (int, float)): return n.value
- if isinstance(n, ast.UnaryOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.operand))
- if isinstance(n, ast.BinOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.left), _ev(n.right))
+ if isinstance(n, ast.Expression):
+ return _ev(n.body)
+ if isinstance(n, ast.Constant):
+ if type(n.value) not in (int, float):
+ raise ValueError(f"Unsupported: {ast.dump(n)}")
+ return n.value
+ if isinstance(n, ast.UnaryOp) and type(n.op) in _OPS:
+ return _OPS[type(n.op)](_ev(n.operand))
+ if isinstance(n, ast.BinOp) and type(n.op) in _OPS:
+ return _OPS[type(n.op)](_ev(n.left), _ev(n.right))
raise ValueError(f"Unsupported: {ast.dump(n)}")🧰 Tools
🪛 Ruff (0.15.15)
[error] 56-56: Multiple statements on one line (colon)
(E701)
[error] 57-57: Multiple statements on one line (colon)
(E701)
[error] 58-58: Multiple statements on one line (colon)
(E701)
[error] 59-59: Multiple statements on one line (colon)
(E701)
🤖 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 `@examples/python/mcp/mcp-tools-server.py` around lines 55 - 60, The helper _ev
currently accepts bools as numeric literals and uses single-line if-statements
that trigger Ruff E701; update _ev to explicitly check the concrete type of
numeric constants by replacing "isinstance(n, ast.Constant) and
isinstance(n.value, (int, float))" with "isinstance(n, ast.Constant) and
type(n.value) in (int, float)" to reject bools, and expand all single-line if
...: return ... statements into multi-line if blocks (e.g., "if ...:\n return
...") for the branches handling ast.Expression, ast.Constant, ast.UnaryOp, and
ast.BinOp that reference _OPS and ast so the logic remains identical but
conforms to style rules.
| def _ev(n): | ||
| if isinstance(n, ast.Expression): return _ev(n.body) | ||
| if isinstance(n, ast.Constant) and isinstance(n.value, (int, float)): return n.value | ||
| if isinstance(n, ast.UnaryOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.operand)) | ||
| if isinstance(n, ast.BinOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.left), _ev(n.right)) | ||
| raise ValueError(f"Unsupported: {ast.dump(n)}") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/praisonai-agents/benchmarks/real_benchmark.py"
echo "== File stats =="
wc -l "$FILE" | cat
echo "== Snippet around lines 70-110 =="
nl -ba "$FILE" | sed -n '70,110p'Repository: MervinPraison/PraisonAI
Length of output: 234
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Try to locate any other _ev occurrences and any Ruff/flake8 configs mentioning E701
FILE="src/praisonai-agents/benchmarks/real_benchmark.py"
rg -n "def _ev\\b" -S "$FILE" || true
rg -n "E701" -S . || true
rg -n "ruff" -S pyproject.toml setup.cfg .ruff.toml . || trueRepository: MervinPraison/PraisonAI
Length of output: 10209
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/praisonai-agents/benchmarks/real_benchmark.py"
echo "== _ev() implementation (around line 80-105) =="
awk 'NR>=75 && NR<=105 {printf "%d:%s\n", NR, $0}' "$FILE"
echo
echo "== ruff.toml (top) =="
if [ -f "ruff.toml" ]; then
awk 'NR<=120 {print NR ":" $0}' ruff.toml
else
echo "ruff.toml not found"
fiRepository: MervinPraison/PraisonAI
Length of output: 2901
Keep _ev’s constant allowlist numeric-only and split single-line branches to satisfy Ruff E701.
src/praisonai-agents/benchmarks/real_benchmark.py:90-93 still accepts True/False (since bool subclasses int) and the one-line if ...: return ... statements trigger Ruff E701.
Suggested fix
def _ev(n):
- if isinstance(n, ast.Expression): return _ev(n.body)
- if isinstance(n, ast.Constant) and isinstance(n.value, (int, float)): return n.value
- if isinstance(n, ast.UnaryOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.operand))
- if isinstance(n, ast.BinOp) and type(n.op) in _OPS: return _OPS[type(n.op)](_ev(n.left), _ev(n.right))
+ if isinstance(n, ast.Expression):
+ return _ev(n.body)
+ if isinstance(n, ast.Constant):
+ if type(n.value) not in (int, float):
+ raise ValueError(f"Unsupported: {ast.dump(n)}")
+ return n.value
+ if isinstance(n, ast.UnaryOp) and type(n.op) in _OPS:
+ return _OPS[type(n.op)](_ev(n.operand))
+ if isinstance(n, ast.BinOp) and type(n.op) in _OPS:
+ return _OPS[type(n.op)](_ev(n.left), _ev(n.right))
raise ValueError(f"Unsupported: {ast.dump(n)}")🧰 Tools
🪛 Ruff (0.15.15)
[error] 90-90: Multiple statements on one line (colon)
(E701)
[error] 91-91: Multiple statements on one line (colon)
(E701)
[error] 92-92: Multiple statements on one line (colon)
(E701)
[error] 93-93: Multiple statements on one line (colon)
(E701)
🤖 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 `@src/praisonai-agents/benchmarks/real_benchmark.py` around lines 89 - 94, The
helper _ev should only accept real numeric constants (not booleans) and must not
use single-line if/return branches; update the isinstance checks in _ev to
verify numeric types while excluding bool (e.g., change "isinstance(n.value,
(int, float))" to "isinstance(n.value, (int, float)) and not isinstance(n.value,
bool)"), and expand each one-line conditional like "if ...: return ..." into
multi-line form with explicit return statements; keep references to _ev and the
operator map _OPS when modifying the UnaryOp and BinOp branches.
| if JWT_SECRET is None or JWT_SECRET == _DEFAULT_SECRET: | ||
| # Determine if we are in an explicit dev environment. | ||
| # Only trust PLATFORM_ENV when it is explicitly set — an unset value | ||
| # must NOT silently fall back to "dev" because production deployments | ||
| # that forget the variable would run with a well-known signing key. | ||
| _explicit_env = os.environ.get("PLATFORM_ENV") | ||
| if _explicit_env == "dev": | ||
| # Developer explicitly opted in to insecure defaults — allow it. | ||
| JWT_SECRET = _DEFAULT_SECRET | ||
| else: | ||
| # Either production or environment not specified. | ||
| # Auto-generate an ephemeral secret so the service can start, but | ||
| # warn loudly because tokens will not survive a restart. | ||
| import secrets as _secrets | ||
| import warnings as _warnings | ||
|
|
||
| JWT_SECRET = _secrets.token_urlsafe(32) | ||
| _warnings.warn( | ||
| "PLATFORM_JWT_SECRET is not set. Auto-generated an ephemeral " | ||
| "signing key — JWT tokens will be invalidated on restart. " | ||
| "Set PLATFORM_JWT_SECRET to a stable random value for production " | ||
| "use, or set PLATFORM_ENV=dev to use the development default.", | ||
| stacklevel=2, | ||
| ) |
There was a problem hiding this comment.
Restore fail-fast behavior for explicit non-dev environments.
This else branch currently handles both PLATFORM_ENV unset and explicitly non-dev values, so PLATFORM_ENV=prod without PLATFORM_JWT_SECRET now boots with an ephemeral signing key instead of failing. That misses the PR’s hardening goal and leaves non-dev deployments in a broken/token-unstable state.
Suggested fix
if JWT_SECRET is None or JWT_SECRET == _DEFAULT_SECRET:
_explicit_env = os.environ.get("PLATFORM_ENV")
if _explicit_env == "dev":
JWT_SECRET = _DEFAULT_SECRET
+ elif _explicit_env is not None:
+ raise RuntimeError(
+ "PLATFORM_JWT_SECRET must be set when PLATFORM_ENV is not 'dev'."
+ )
else:
import secrets as _secrets
import warnings as _warnings
JWT_SECRET = _secrets.token_urlsafe(32)
_warnings.warn(🤖 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 `@src/praisonai-platform/praisonai_platform/services/auth_service.py` around
lines 30 - 53, The current branch treats PLATFORM_ENV unset and explicitly
non-"dev" the same and auto-generates an ephemeral JWT_SECRET; instead, change
the logic in the JWT_SECRET initialization (the block guarded by JWT_SECRET is
None or JWT_SECRET == _DEFAULT_SECRET and the check of
os.environ.get("PLATFORM_ENV")) so that: if _explicit_env is None (unset) you
keep the existing behavior of auto-generating and warning, but if _explicit_env
is explicitly set and not "dev" (e.g., "prod") you fail-fast — raise an
exception (or exit) with a clear message telling the operator to set
PLATFORM_JWT_SECRET to a stable value or set PLATFORM_ENV=dev; reference
JWT_SECRET, _DEFAULT_SECRET and PLATFORM_ENV in the error handling so the code
refuses to start in explicit non-dev environments without a provided secret.
|
@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Focus ONLY on Python packages (praisonaiagents, praisonai). Do NOT modify praisonai-rust or praisonai-ts. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding. Phase 1: Review per AGENTS.md
Phase 2: FIX Valid Issues Phase 3: Final Verdict |
|
Claude finished @MervinPraison's task —— View job Security Architecture Review & Fixes ✅I have conducted the final architecture review and implemented fixes for all critical security issues identified by multiple reviewers. Todo List:
Summary of Changes Implemented✅ Architecture Review (Phase 1)
✅ Critical Security Fixes (Phase 2)1. DoS Vulnerability via Exponentiation (FIXED)
2. JWT Secret Bypass (FIXED)
3. Production Environment Handling (FIXED)
4. Boolean Literal Acceptance (FIXED)
5. Linting Issues (FIXED)
6. Warning Message Accuracy (FIXED)
✅ Testing & Validation (Phase 2)
Final Verdict (Phase 3)🟢 APPROVED - All Critical Issues ResolvedThis PR now successfully addresses both original security advisories plus all additional vulnerabilities identified by the reviewing agents: Original Advisories (✅ FIXED):
Additional Vulnerabilities Found & Fixed (✅ FIXED):
Security Posture: The codebase is now significantly more secure with proper input validation, fail-fast error handling, and defense-in-depth security controls. Impact: Zero breaking changes, only security improvements. All fixes are backward compatible. Recommendation: ✅ MERGE - This PR successfully hardens the security posture while maintaining functionality. |
|
@claude validate if this is the correct way of doing it, for examples, are we not making it complicated ? validate |
|
Claude finished @MervinPraison's task —— View job Security Architecture Review & Validation ✅I've completed a comprehensive review of the security hardening approach, examining the JWT secret handling and AST evaluator implementation across all 10 files. Todo List:
🎯 Final Verdict: The Approach is Fundamentally SoundTL;DR: The security fixes are architecturally correct and the complexity is justified given the critical nature of the vulnerabilities. This is not over-engineering — it's defensive security done right. 🏗️ Architecture Validation✅ Follows AGENTS.md PrinciplesThe changes align perfectly with the framework's core principles:
✅ Layer Separation RespectedThe changes maintain proper architectural boundaries:
🔐 Security AssessmentJWT Secret Hardening (GHSA-3qg8-5g3r-79v5)Problem: Production deployments silently fell back to Solution Analysis: # BEFORE: Silent insecure fallback
JWT_SECRET = os.environ.get("PLATFORM_JWT_SECRET", "dev-secret-change-me")
# AFTER: Environment-aware security
if JWT_SECRET is None or JWT_SECRET == _DEFAULT_SECRET:
_explicit_env = os.environ.get("PLATFORM_ENV")
if _explicit_env == "dev":
JWT_SECRET = _DEFAULT_SECRET # Explicit dev consent
else:
JWT_SECRET = _secrets.token_urlsafe(32) # Safe ephemeral fallback
_warnings.warn(...) # Loud warning✅ Verdict: Architecturally excellent. This implements proper defense-in-depth:
eval() → AST Evaluator (GHSA-vg22-4gmj-prxw)Problem: 9 example files used Solution Analysis: # BEFORE: Dangerous eval with weak filtering
allowed_chars = set("0123456789+-*/.() ")
if all(c in allowed_chars for c in expression):
result = eval(expression) # RCE via (1).__class__.__bases__[0]...
# AFTER: Safe AST evaluator
def _eval(node):
if isinstance(node, ast.Expression): return _eval(node.body)
if isinstance(node, ast.Constant) and isinstance(node.value, (int, float)): return node.value
if isinstance(node, ast.BinOp) and type(node.op) in _OPS:
return _OPS[type(node.op)](_eval(node.left), _eval(node.right))
raise ValueError(f"Unsupported: {ast.dump(node)}")✅ Verdict: Industry best practice. The AST approach is the gold standard for safe expression evaluation:
🤔 Complexity Analysis: Is It Too Much?Complexity Metrics
Alternative Approaches Considered
✅ Conclusion: Right-sized ComplexityThe implementation strikes the optimal balance:
🔍 Code Quality AssessmentStrengths
Areas for Improvement(Addressing reviewer feedback):
These are implementation details, not architectural flaws. 📋 RecommendationsImmediate (Critical)
Future Enhancements
🏆 Final Assessment
Overall: This is excellent security engineering. The complexity is not only justified but necessary for proper security. The alternative (removing functionality or using weaker security) would be worse for users. Recommendation: ✅ Proceed with this approach. Address the specific implementation issues identified by reviewers, but keep the core architectural pattern intact. |
2 participants
Summary
Addresses remaining attack surface from two CRITICAL security advisories that are still exploitable in the current codebase.
GHSA-3qg8-5g3r-79v5 — Hardcoded JWT signing key (CRITICAL)
Vulnerability:
auth_service.pyfalls back toJWT_SECRET = "dev-secret-change-me"whenPLATFORM_JWT_SECRETis not set. The existing guard (PLATFORM_ENV != "dev") is bypassed becausePLATFORM_ENVdefaults to"dev"when unset — meaning any production deployment that forgets to set both environment variables silently runs with a well-known signing key. Any attacker can forge JWTs for arbitrary users.Fix:
PLATFORM_ENVno longer silently defaults to"dev". WhenPLATFORM_JWT_SECRETis not set:PLATFORM_ENVis explicitly"dev"→ allow the default (dev mode)PLATFORM_ENVis explicitly something else →RuntimeError(fail-fast)PLATFORM_ENVis not set → auto-generate an ephemeral random secret + warn (safe default)Advisory status: No patched version listed — this fix addresses the gap.
GHSA-vg22-4gmj-prxw — Unauthenticated eval() in A2A example (CRITICAL)
Vulnerability: 9 official example files use
eval(expression)as a calculator tool. The character-filter approach (allowed_chars = set("0123456789+-*/.() ")) is insufficient because()enables arbitrary function call chains like(1).__class__.__bases__[0].__subclasses__(). When wired to an A2A server (or any MCP tool), this gives remote code execution.Fix: All 9 files now use an AST-based safe evaluator that only permits numeric literals and arithmetic operators (
+,-,*,/,//,%,**, unary-/+). Anything else raisesValueError.Already fixed (no changes needed)
rules.create): Fixed by_resolve_rule_path()containment incli_tools.pyworkflow.show): Fixed by_resolve_cwd_yaml_path()containment incli_tools.pyFiles changed
src/praisonai-platform/praisonai_platform/services/auth_service.pyexamples/python/a2a/a2a-server.pyexamples/mcp/multi_server.pyexamples/python/mcp/mcp-tools-server.pyexamples/python/mcp/mcp-tools-server-sse.pyexamples/mcp_server/custom_tools_server.pyexamples/python/mcp_server_example.pyexamples/python/custom_tools/tools_with_latency.pyexamples/eval/reliability_example.pysrc/praisonai-agents/benchmarks/real_benchmark.pyTesting
The AST evaluator handles the same arithmetic expressions (
2 + 2,3.14 * 2,10 / 3,2 ** 10) while rejecting any attribute access, function calls, imports, or string literals.Summary by CodeRabbit