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

next #405

Merged
merged 23 commits into from
Nov 13, 2024
Merged

next #405

merged 23 commits into from
Nov 13, 2024

Conversation

potts99
Copy link
Collaborator

@potts99 potts99 commented Nov 13, 2024

No description provided.

Copy link

vercel bot commented Nov 13, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Updated (UTC)
docs ⬜️ Skipped (Inspect) Nov 13, 2024 11:56pm
peppermint ⬜️ Skipped (Inspect) Nov 13, 2024 11:56pm

@potts99 potts99 merged commit 44256d1 into main Nov 13, 2024
3 of 4 checks passed
Comment on lines +69 to +74
async (request: FastifyRequest, reply: FastifyReply) => {
const logs = await import("fs/promises").then((fs) =>
fs.readFile("logs.log", "utf-8")
);
reply.send({ logs: logs });
}

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
a file system access
, but is not rate-limited.

Copilot Autofix AI 2 months ago

To fix the problem, we need to introduce rate limiting to the route that performs file system access. We can use the fastify-rate-limit plugin to achieve this. This plugin allows us to set a maximum number of requests per time window for specific routes.

  1. Install the fastify-rate-limit plugin.
  2. Register the fastify-rate-limit plugin with the Fastify instance.
  3. Apply rate limiting to the specific route that performs file system access.
Suggested changeset 2
apps/api/src/controllers/data.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/apps/api/src/controllers/data.ts b/apps/api/src/controllers/data.ts
--- a/apps/api/src/controllers/data.ts
+++ b/apps/api/src/controllers/data.ts
@@ -3,4 +3,9 @@
 import { prisma } from "../prisma";
+import fastifyRateLimit from 'fastify-rate-limit';
 
 export function dataRoutes(fastify: FastifyInstance) {
+  fastify.register(fastifyRateLimit, {
+    max: 100, // maximum number of requests
+    timeWindow: '15 minutes' // time window for rate limiting
+  });
   // Get total count of all tickets
@@ -68,2 +73,10 @@
     "/api/v1/data/logs",
+    {
+      config: {
+        rateLimit: {
+          max: 10, // maximum number of requests for this route
+          timeWindow: '1 minute' // time window for rate limiting
+        }
+      }
+    },
     async (request: FastifyRequest, reply: FastifyReply) => {
EOF
@@ -3,4 +3,9 @@
import { prisma } from "../prisma";
import fastifyRateLimit from 'fastify-rate-limit';

export function dataRoutes(fastify: FastifyInstance) {
fastify.register(fastifyRateLimit, {
max: 100, // maximum number of requests
timeWindow: '15 minutes' // time window for rate limiting
});
// Get total count of all tickets
@@ -68,2 +73,10 @@
"/api/v1/data/logs",
{
config: {
rateLimit: {
max: 10, // maximum number of requests for this route
timeWindow: '1 minute' // time window for rate limiting
}
}
},
async (request: FastifyRequest, reply: FastifyReply) => {
apps/api/package.json
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/apps/api/package.json b/apps/api/package.json
--- a/apps/api/package.json
+++ b/apps/api/package.json
@@ -65,3 +65,4 @@
     "simple-oauth2": "^5.1.0",
-    "xml-encryption": "^3.0.2"
+    "xml-encryption": "^3.0.2",
+    "fastify-rate-limit": "^5.9.0"
   },
EOF
@@ -65,3 +65,4 @@
"simple-oauth2": "^5.1.0",
"xml-encryption": "^3.0.2"
"xml-encryption": "^3.0.2",
"fastify-rate-limit": "^5.9.0"
},
This fix introduces these dependencies
Package Version Security advisories
fastify-rate-limit (npm) 5.9.0 None
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
}),
if (webhook[i].active === true) {
const s = status ? "Completed" : "Outstanding";
if (url.includes("discord.com")) {

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High

'
discord.com
' can be anywhere in the URL, and arbitrary hosts may come before or after it.

Copilot Autofix AI 2 months ago

To fix the problem, we need to parse the URL and check its host value against a whitelist of allowed hosts. This ensures that the check is not bypassed by embedding the allowed host in an unexpected location within the URL.

  1. Parse the URL to extract the host value.
  2. Define a whitelist of allowed hosts.
  3. Check if the host value is in the whitelist before proceeding with the redirection or API call.
Suggested changeset 1
apps/api/src/controllers/ticket.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/apps/api/src/controllers/ticket.ts b/apps/api/src/controllers/ticket.ts
--- a/apps/api/src/controllers/ticket.ts
+++ b/apps/api/src/controllers/ticket.ts
@@ -553,2 +553,4 @@
         const url = webhook[i].url;
+        const parsedUrl = new URL(url);
+        const allowedHosts = ["discord.com", "www.discord.com"];
 
@@ -556,3 +558,3 @@
           const s = status ? "Completed" : "Outstanding";
-          if (url.includes("discord.com")) {
+          if (allowedHosts.includes(parsedUrl.host)) {
             const message = {
EOF
@@ -553,2 +553,4 @@
const url = webhook[i].url;
const parsedUrl = new URL(url);
const allowedHosts = ["discord.com", "www.discord.com"];

@@ -556,3 +558,3 @@
const s = status ? "Completed" : "Outstanding";
if (url.includes("discord.com")) {
if (allowedHosts.includes(parsedUrl.host)) {
const message = {
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
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
Comment on lines +25 to +29
const response = await fetch(`/api/v1/role/${id}`, {
headers: {
Authorization: `Bearer ${getCookie("session")}`,
},
});

Check failure

Code scanning / CodeQL

Server-side request forgery Critical

The
URL
of this request depends on a
user-provided value
.

Copilot Autofix AI 2 months ago

To fix the problem, we need to validate and sanitize the id parameter before using it in the URL for the fetch request. One way to do this is to ensure that the id parameter matches a specific pattern or format that is expected for role IDs. This can be done using a regular expression or by checking against a list of allowed IDs.

In this case, we will use a regular expression to validate the id parameter. We will only allow alphanumeric characters and dashes in the id parameter, which is a common pattern for IDs.

Suggested changeset 1
apps/client/pages/admin/roles/[id].tsx

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/apps/client/pages/admin/roles/[id].tsx b/apps/client/pages/admin/roles/[id].tsx
--- a/apps/client/pages/admin/roles/[id].tsx
+++ b/apps/client/pages/admin/roles/[id].tsx
@@ -19,5 +19,8 @@
 
+  // Function to validate the id parameter
+  const isValidId = (id: string) => /^[a-zA-Z0-9-]+$/.test(id);
+
   // New function to fetch role data
   const fetchRoleData = async () => {
-    if (!id) return;
+    if (!id || !isValidId(id as string)) return;
 
EOF
@@ -19,5 +19,8 @@

// Function to validate the id parameter
const isValidId = (id: string) => /^[a-zA-Z0-9-]+$/.test(id);

// New function to fetch role data
const fetchRoleData = async () => {
if (!id) return;
if (!id || !isValidId(id as string)) return;

Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
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
Comment on lines +64 to +75
await fetch(`/api/v1/role/${id}/update`, {
method: "PUT",
headers: {
Authorization: `Bearer ${getCookie("session")}`,
"Content-Type": "application/json",
},
body: JSON.stringify({
name: roleName,
permissions: selectedPermissions,
users: selectedUsers,
}),
})

Check failure

Code scanning / CodeQL

Server-side request forgery Critical

The
URL
of this request depends on a
user-provided value
.

Copilot Autofix AI 2 months ago

To fix the problem, we need to ensure that the id parameter is validated before it is used to construct the URL for the fetch request. One way to do this is to use a whitelist of allowed role IDs or to validate the format of the id to ensure it meets expected criteria (e.g., it is a valid UUID).

In this case, we will implement a simple validation to ensure that the id is a valid UUID. This will prevent attackers from injecting malicious input into the URL.

Suggested changeset 1
apps/client/pages/admin/roles/[id].tsx

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/apps/client/pages/admin/roles/[id].tsx b/apps/client/pages/admin/roles/[id].tsx
--- a/apps/client/pages/admin/roles/[id].tsx
+++ b/apps/client/pages/admin/roles/[id].tsx
@@ -19,5 +19,11 @@
 
+  // Function to validate UUID
+  const isValidUUID = (uuid) => {
+    const regex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
+    return regex.test(uuid);
+  };
+
   // New function to fetch role data
   const fetchRoleData = async () => {
-    if (!id) return;
+    if (!id || !isValidUUID(id)) return;
 
@@ -61,3 +67,3 @@
   const handleUpdateRole = async () => {
-    if (!roleName || !id) return;
+    if (!roleName || !id || !isValidUUID(id)) return;
 
EOF
@@ -19,5 +19,11 @@

// Function to validate UUID
const isValidUUID = (uuid) => {
const regex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
return regex.test(uuid);
};

// New function to fetch role data
const fetchRoleData = async () => {
if (!id) return;
if (!id || !isValidUUID(id)) return;

@@ -61,3 +67,3 @@
const handleUpdateRole = async () => {
if (!roleName || !id) return;
if (!roleName || !id || !isValidUUID(id)) return;

Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
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
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.

1 participant