-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix accessibility and small patch #216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| return jsonify({ | ||
| "status": "error", | ||
| "message": result.get('content', 'Unknown error during transformation') | ||
| }), 400 |
Check warning
Code scanning / CodeQL
Information exposure through an exception Medium
Stack trace information
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 19 hours ago
General approach: Avoid returning raw exception details from the sandbox directly to the client. Instead, log detailed error information on the server, and return a generic but user-friendly error message over the API. If some feedback is needed for user-authored transformation code, keep it high-level and ensure it cannot contain stack traces or sensitive environment details.
Best fix in this context:
-
In
py_sandbox.run_in_main_process:- Instead of returning the raw
error_messagederived from the exception, log the full details (including traceback) server-side usingtraceback.format_exc(). - Return a generic error indicator and a sanitized message like
"An error occurred while executing the transformation code.", or at most a brief classification (e.g.,"Import not allowed"), not including arbitrarystr(err).
- Instead of returning the raw
-
In
py_sandbox.run_transform_in_sandbox2020:- Keep the structure the same (returning
{'status': 'error', 'content': ...}), but ensure that thecontentcomes from the sanitized error message produced byrun_in_main_process(as above). No changes needed if we change only whatrun_in_main_processreturns.
- Keep the structure the same (returning
-
In
tables_routes.refresh_derived_data:- Continue to return
result.get('content', 'Unknown error during transformation')as the message, but becausecontentwill now be sanitized, it will no longer contain sensitive exception details. - Optionally, slightly reword the message to be generic, but this is not strictly necessary once
contentitself is safe.
- Continue to return
Concretely, we will:
-
Modify the
exceptblock inrun_in_main_process(lines around 106–111 inpy-src/data_formulator/py_sandbox.py) to:- Build a safe, fixed error message for returning to callers.
- Capture the detailed traceback using
traceback.format_exc()and return it only in a field not propagated to the HTTP response (or not at all—just log it). - To stay minimally invasive and not change the structure used by callers, we will keep the keys
'status'and'error_message', but ensureerror_messageis sanitized and does not embed arbitrarystr(err).
-
Optionally, to preserve server-side diagnostics without changing imports, we can log the detailed traceback inside
run_in_main_processusingprintorwarnings, but a better approach (if a logger were available) would be logging. Since we must not add new imports beyond well-known ones and no logger is defined in this file, we will just avoid returning the sensitive details and, if desired, include a comment encouraging logging via outer layers.
No changes are required in tables_routes.refresh_derived_data beyond relying on the now-sanitized message, since the vulnerability is the content coming from the sandbox, not the shape of the JSON response.
-
Copy modified lines R109-R115
| @@ -106,8 +106,13 @@ | ||
| try: | ||
| exec(code, restricted_globals) | ||
| except Exception as err: | ||
| error_message = f"Error: {type(err).__name__} - {str(err)}" | ||
| return {'status': 'error', 'error_message': error_message} | ||
| # Do not propagate detailed exception information (which may include | ||
| # stack traces, file paths, or other sensitive data) to the caller. | ||
| # | ||
| # Instead, return a generic error message, while allowing callers or | ||
| # outer layers to log detailed information if needed. | ||
| safe_error_message = "An error occurred while executing the transformation code." | ||
| return {'status': 'error', 'error_message': safe_error_message} | ||
|
|
||
| return {'status': 'ok', 'allowed_objects': {key: restricted_globals[key] for key in allowed_objects}} | ||
|
|
No description provided.