Skip to content
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

Proxy #1

Merged
merged 15 commits into from
Jan 4, 2025
Merged

Proxy #1

merged 15 commits into from
Jan 4, 2025

Conversation

mpnowacki-reef
Copy link
Contributor

No description provided.

except Exception as e:
msg = f"Failed to decompress data: {str(e)}"
logger.debug(msg)
return HttpResponse(msg.encode(), status=400)

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
flows to this location and may be exposed to an external user.

Copilot Autofix AI 2 months ago

To fix the problem, we need to ensure that detailed exception messages are not exposed to the end user. Instead, we should log the detailed error message on the server and return a generic error message to the user. This can be achieved by modifying the exception handling blocks to log the detailed error and return a generic message.

Steps to fix:

  1. Modify the exception handling block on lines 103-106 to log the detailed error message and return a generic error message.
  2. Similarly, modify the exception handling block on lines 111-114 to log the detailed error message and return a generic error message.
Suggested changeset 1
app/src/project/core/views.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/app/src/project/core/views.py b/app/src/project/core/views.py
--- a/app/src/project/core/views.py
+++ b/app/src/project/core/views.py
@@ -103,5 +103,4 @@
     except Exception as e:
-        msg = f"Failed to decompress data: {str(e)}"
-        logger.debug(msg)
-        return HttpResponse(msg.encode(), status=400)
+        logger.debug(f"Failed to decompress data: {str(e)}")
+        return HttpResponse("Failed to decompress data.", status=400)
 
@@ -111,5 +110,4 @@
     except Exception as e:
-        msg = f"Failed to decode metrics: {str(e)}"
-        logger.debug(msg)
-        return HttpResponse(msg, status=400)
+        logger.debug(f"Failed to decode metrics: {str(e)}")
+        return HttpResponse("Failed to decode metrics.", status=400)
 
EOF
@@ -103,5 +103,4 @@
except Exception as e:
msg = f"Failed to decompress data: {str(e)}"
logger.debug(msg)
return HttpResponse(msg.encode(), status=400)
logger.debug(f"Failed to decompress data: {str(e)}")
return HttpResponse("Failed to decompress data.", status=400)

@@ -111,5 +110,4 @@
except Exception as e:
msg = f"Failed to decode metrics: {str(e)}"
logger.debug(msg)
return HttpResponse(msg, status=400)
logger.debug(f"Failed to decode metrics: {str(e)}")
return HttpResponse("Failed to decode metrics.", status=400)

Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Copy link
Contributor

Choose a reason for hiding this comment

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

👆

except Exception as e:
msg = f"Failed to decode metrics: {str(e)}"
logger.debug(msg)
return HttpResponse(msg, status=400)

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
flows to this location and may be exposed to an external user.

Copilot Autofix AI 2 months ago

To fix the problem, we need to ensure that the error messages returned to the user do not contain any sensitive information. Instead of including the exception message (str(e)) in the response, we should return a generic error message. The detailed error information should be logged on the server for debugging purposes.

Specifically, we will:

  1. Replace the detailed error messages in the HttpResponse with a generic message.
  2. Ensure that the detailed error information is logged using logger.debug.
Suggested changeset 1
app/src/project/core/views.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/app/src/project/core/views.py b/app/src/project/core/views.py
--- a/app/src/project/core/views.py
+++ b/app/src/project/core/views.py
@@ -103,5 +103,4 @@
     except Exception as e:
-        msg = f"Failed to decompress data: {str(e)}"
-        logger.debug(msg)
-        return HttpResponse(msg.encode(), status=400)
+        logger.debug(f"Failed to decompress data: {str(e)}")
+        return HttpResponse("Failed to decompress data.", status=400)
 
@@ -111,5 +110,4 @@
     except Exception as e:
-        msg = f"Failed to decode metrics: {str(e)}"
-        logger.debug(msg)
-        return HttpResponse(msg, status=400)
+        logger.debug(f"Failed to decode metrics: {str(e)}")
+        return HttpResponse("Failed to decode metrics.", status=400)
 
EOF
@@ -103,5 +103,4 @@
except Exception as e:
msg = f"Failed to decompress data: {str(e)}"
logger.debug(msg)
return HttpResponse(msg.encode(), status=400)
logger.debug(f"Failed to decompress data: {str(e)}")
return HttpResponse("Failed to decompress data.", status=400)

@@ -111,5 +110,4 @@
except Exception as e:
msg = f"Failed to decode metrics: {str(e)}"
logger.debug(msg)
return HttpResponse(msg, status=400)
logger.debug(f"Failed to decode metrics: {str(e)}")
return HttpResponse("Failed to decode metrics.", status=400)

Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Copy link
Contributor

Choose a reason for hiding this comment

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

👆

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sometimes I agree with this, but the code blocks under "try" Are terribly short in these cases

hotkey = label.value
if label.value != ss58_address:
msg = f"Received invalid hotkey. Expected {ss58_address} got {label.value}"
error = HttpResponse(status=403, content=msg.encode())

Check warning

Code scanning / CodeQL

Reflected server-side cross-site scripting Medium

Cross-site scripting vulnerability due to a
user-provided value
.

Copilot Autofix AI 2 months ago

To fix the problem, we need to ensure that any user-provided input included in the HTTP response is properly sanitized or escaped to prevent XSS attacks. In this case, we can use the html.escape() function from the html module to escape the user input before including it in the response. This will convert special characters to their corresponding HTML entities, preventing the browser from interpreting them as executable code.

We will apply this fix to the relevant lines in the prometheus_inbound_proxy function where user input is included in the msg variable.

Suggested changeset 1
app/src/project/core/views.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/app/src/project/core/views.py b/app/src/project/core/views.py
--- a/app/src/project/core/views.py
+++ b/app/src/project/core/views.py
@@ -1,2 +1,3 @@
 from urllib.parse import urljoin
+import html
 
@@ -103,3 +104,3 @@
     except Exception as e:
-        msg = f"Failed to decompress data: {str(e)}"
+        msg = f"Failed to decompress data: {html.escape(str(e))}"
         logger.debug(msg)
@@ -111,3 +112,3 @@
     except Exception as e:
-        msg = f"Failed to decode metrics: {str(e)}"
+        msg = f"Failed to decode metrics: {html.escape(str(e))}"
         logger.debug(msg)
@@ -128,3 +129,3 @@
                 if label.value != ss58_address:
-                    msg = f"Received invalid hotkey. Expected {ss58_address} got {label.value}"
+                    msg = f"Received invalid hotkey. Expected {html.escape(ss58_address)} got {html.escape(label.value)}"
                     error = HttpResponse(status=403, content=msg.encode())
@@ -133,3 +134,3 @@
         if not hotkey:
-            msg = f"Received no hotkey"
+            msg = "Received no hotkey"
             error = HttpResponse(status=403, content=msg.encode())
@@ -137,3 +138,3 @@
         if error is not None:
-            error.content = f"Metric: {name}. ".encode() + error.content
+            error.content = f"Metric: {html.escape(name)}. ".encode() + error.content
             logger.info(error.content.decode())
EOF
@@ -1,2 +1,3 @@
from urllib.parse import urljoin
import html

@@ -103,3 +104,3 @@
except Exception as e:
msg = f"Failed to decompress data: {str(e)}"
msg = f"Failed to decompress data: {html.escape(str(e))}"
logger.debug(msg)
@@ -111,3 +112,3 @@
except Exception as e:
msg = f"Failed to decode metrics: {str(e)}"
msg = f"Failed to decode metrics: {html.escape(str(e))}"
logger.debug(msg)
@@ -128,3 +129,3 @@
if label.value != ss58_address:
msg = f"Received invalid hotkey. Expected {ss58_address} got {label.value}"
msg = f"Received invalid hotkey. Expected {html.escape(ss58_address)} got {html.escape(label.value)}"
error = HttpResponse(status=403, content=msg.encode())
@@ -133,3 +134,3 @@
if not hotkey:
msg = f"Received no hotkey"
msg = "Received no hotkey"
error = HttpResponse(status=403, content=msg.encode())
@@ -137,3 +138,3 @@
if error is not None:
error.content = f"Metric: {name}. ".encode() + error.content
error.content = f"Metric: {html.escape(name)}. ".encode() + error.content
logger.info(error.content.decode())
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
app/bittensor_prometheus/entrypoint.sh Outdated Show resolved Hide resolved
app/envs/prod/Dockerfile Show resolved Hide resolved
app/src/project/core/prometheus_protobuf/README.md Outdated Show resolved Hide resolved
app/src/project/settings.py Outdated Show resolved Hide resolved
logger.debug(msg)
return HttpResponse(status=400, content=msg)

if ss58_address not in Validator.objects.filter(active=True).values_list('public_key', flat=True):
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if ss58_address not in Validator.objects.filter(active=True).values_list('public_key', flat=True):
if not Validator.objects.filter(active=True, public_key=ss58_address).exists():

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's gonna nuke the caches cache with each new request, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I missed the use of cacheops.

except Exception as e:
msg = f"Failed to decompress data: {str(e)}"
logger.debug(msg)
return HttpResponse(msg.encode(), status=400)
Copy link
Contributor

Choose a reason for hiding this comment

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

👆

except Exception as e:
msg = f"Failed to decode metrics: {str(e)}"
logger.debug(msg)
return HttpResponse(msg, status=400)
Copy link
Contributor

Choose a reason for hiding this comment

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

👆

app/src/project/settings.py Outdated Show resolved Hide resolved
Co-authored-by: emnoor-reef <137923473+emnoor-reef@users.noreply.github.com>
@mpnowacki-reef mpnowacki-reef merged commit c10ccbe into master Jan 4, 2025
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants