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

Add nodejs-file-upload sample #259

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add nodejs-file-upload sample #259

wants to merge 1 commit into from

Conversation

lionello
Copy link
Member

@lionello lionello commented Nov 18, 2024

Samples Checklist

  • add README.md to ./samples/nodejs-file-upload/

@lionello lionello temporarily deployed to deploy-changed-samples November 18, 2024 22:41 — with GitHub Actions Inactive
Comment on lines +11 to +31
app.get('/', (req, res) => {
fs.readdir('uploads', (err, files) => {
if (err) {
return res.status(500).send('Unable to scan files!');
}
let fileList = files.map(file => `<li><a href="/${file}">${file}</a></li>`).join('');
res.send(`
<h1>File Upload</h1>
<form ref='uploadForm'
id='uploadForm'
action='/'
method='post'
encType="multipart/form-data">
<input type="file" name="file" />
<input type='submit' value='Upload!' />
</form>
<h2>Uploaded Files</h2>
<ul>${fileList}</ul>
`);
});
});

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 about 2 months ago

To fix the problem, we need to introduce rate limiting to the Express application. The best way to do this is by using the express-rate-limit package, which allows us to set a maximum number of requests that can be made to the server within a specified time window. This will help prevent denial-of-service attacks by limiting the rate at which requests are accepted.

We will:

  1. Install the express-rate-limit package.
  2. Import the express-rate-limit package in the code.
  3. Set up a rate limiter with appropriate configuration.
  4. Apply the rate limiter to all routes in the application.
Suggested changeset 2
samples/nodejs-file-upload/main.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/samples/nodejs-file-upload/main.js b/samples/nodejs-file-upload/main.js
--- a/samples/nodejs-file-upload/main.js
+++ b/samples/nodejs-file-upload/main.js
@@ -4,2 +4,3 @@
 const path = require('path');
+const RateLimit = require('express-rate-limit');
 
@@ -8,2 +9,8 @@
 
+const limiter = RateLimit({
+    windowMs: 15 * 60 * 1000, // 15 minutes
+    max: 100, // max 100 requests per windowMs
+});
+
+app.use(limiter);
 app.use(express.static('uploads'));
EOF
@@ -4,2 +4,3 @@
const path = require('path');
const RateLimit = require('express-rate-limit');

@@ -8,2 +9,8 @@

const limiter = RateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 100, // max 100 requests per windowMs
});

app.use(limiter);
app.use(express.static('uploads'));
samples/nodejs-file-upload/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/samples/nodejs-file-upload/package.json b/samples/nodejs-file-upload/package.json
--- a/samples/nodejs-file-upload/package.json
+++ b/samples/nodejs-file-upload/package.json
@@ -10,3 +10,4 @@
 	  "express": "^4.17.1",
-	  "multer": "^1.4.4"
+	  "multer": "^1.4.4",
+	  "express-rate-limit": "^7.4.1"
 	}
EOF
@@ -10,3 +10,4 @@
"express": "^4.17.1",
"multer": "^1.4.4"
"multer": "^1.4.4",
"express-rate-limit": "^7.4.1"
}
This fix introduces these dependencies
Package Version Security advisories
express-rate-limit (npm) 7.4.1 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
Comment on lines +17 to +29
res.send(`
<h1>File Upload</h1>
<form ref='uploadForm'
id='uploadForm'
action='/'
method='post'
encType="multipart/form-data">
<input type="file" name="file" />
<input type='submit' value='Upload!' />
</form>
<h2>Uploaded Files</h2>
<ul>${fileList}</ul>
`);

Check failure

Code scanning / CodeQL

Stored cross-site scripting High

Stored cross-site scripting vulnerability due to
stored value
.

Copilot Autofix AI about 2 months ago

To fix the stored cross-site scripting vulnerability, we need to sanitize the filenames before using them to generate HTML content. The best way to do this is to use a library like escape-html to escape any potentially dangerous characters in the filenames. This will ensure that any HTML tags or scripts in the filenames are rendered harmless.

  1. Install the escape-html library.
  2. Import the escape-html library in the main.js file.
  3. Use the escape-html function to escape the filenames before embedding them in the HTML content.
Suggested changeset 2
samples/nodejs-file-upload/main.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/samples/nodejs-file-upload/main.js b/samples/nodejs-file-upload/main.js
--- a/samples/nodejs-file-upload/main.js
+++ b/samples/nodejs-file-upload/main.js
@@ -4,2 +4,3 @@
 const path = require('path');
+const escapeHtml = require('escape-html');
 
@@ -15,3 +16,3 @@
         }
-        let fileList = files.map(file => `<li><a href="/${file}">${file}</a></li>`).join('');
+        let fileList = files.map(file => `<li><a href="/${escapeHtml(file)}">${escapeHtml(file)}</a></li>`).join('');
         res.send(`
EOF
@@ -4,2 +4,3 @@
const path = require('path');
const escapeHtml = require('escape-html');

@@ -15,3 +16,3 @@
}
let fileList = files.map(file => `<li><a href="/${file}">${file}</a></li>`).join('');
let fileList = files.map(file => `<li><a href="/${escapeHtml(file)}">${escapeHtml(file)}</a></li>`).join('');
res.send(`
samples/nodejs-file-upload/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/samples/nodejs-file-upload/package.json b/samples/nodejs-file-upload/package.json
--- a/samples/nodejs-file-upload/package.json
+++ b/samples/nodejs-file-upload/package.json
@@ -10,3 +10,4 @@
 	  "express": "^4.17.1",
-	  "multer": "^1.4.4"
+	  "multer": "^1.4.4",
+	  "escape-html": "^1.0.3"
 	}
EOF
@@ -10,3 +10,4 @@
"express": "^4.17.1",
"multer": "^1.4.4"
"multer": "^1.4.4",
"escape-html": "^1.0.3"
}
This fix introduces these dependencies
Package Version Security advisories
escape-html (npm) 1.0.3 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
@raphaeltm
Copy link
Collaborator

@lionello personally I think our philosophy around samples should shift to be more like Railway's: it should be about demonstrating specific technologies, rather than techniques. Or maybe it's just about tagging them... i.e. some things are "samples" some things are "demos" and some things are "templates" or something?

So maybe "samples" are starting points for using technologies, "demos" demonstrate techniques (i.e. stuff you might find in a tutorial) and "templates" are full-featured starting points for a kind of app or site (blog, crm, etc.)?

@lionello
Copy link
Member Author

@raphaeltm I love those categories! Let's use those going forward.

@raphaeltm
Copy link
Collaborator

@raphaeltm I love those categories! Let's use those going forward.

Okidoke. I'll add a "Type" in the readme metadata which should be one of those three.

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