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 flash from file endpoint #93

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Conversation

rcooke-warwick
Copy link
Contributor

allows to flash from a file on the worker without streaming it over http first

Change-type: patch
Signed-off-by: Ryan Cooke <[email protected]>
Change-type: patch
Signed-off-by: Ryan Cooke <[email protected]>
Change-type: patch
Signed-off-by: Ryan Cooke <[email protected]>
Comment on lines +398 to +426
async (req: express.Request, res: express.Response) => {
res.writeHead(202, {
'Content-Type': 'text/event-stream',
Connection: 'keep-alive',
});

const timer = setInterval(() => {
res.write('status: pending');
}, 5000);


try {
let FILENAME = req.body.filename;
if(FILENAME.includes(`.gz`)){
console.log(`Unzipping file`)
console.log(await execSync(`gunzip -f ${FILENAME}`))
FILENAME = FILENAME.replace(/\.gz$/, '');
}
console.log(`Attempting to flash with file: ${FILENAME}...`);
await worker.flash(FILENAME);
clearInterval(timer);
res.end()
} catch (e) {
//TODO: respdond with error instead of just doing nothing
console.log(e);
clearInterval(timer);
res.end()
}
}

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
a system command
, but is not rate-limited.
let FILENAME = req.body.filename;
if(FILENAME.includes(`.gz`)){
console.log(`Unzipping file`)
console.log(await execSync(`gunzip -f ${FILENAME}`))

Check failure

Code scanning / CodeQL

Uncontrolled command line Critical

This command line depends on a
user-provided value
.
Copy link
Member

@vipulgupta2048 vipulgupta2048 left a comment

Choose a reason for hiding this comment

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

Looks great overall, had some comments inline

lib/index.ts Outdated
@@ -356,7 +362,7 @@ async function setup(

const timer = setInterval(() => {
res.write('status: pending');
}, httpServer.keepAliveTimeout);
}, 5000);
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for defining this. So helpful



try {
let FILENAME = req.body.filename;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let FILENAME = req.body.filename;
const FILENAME = req.body.filename;

if(FILENAME.includes(`.gz`)){
console.log(`Unzipping file`)
console.log(await execSync(`gunzip -f ${FILENAME}`))
FILENAME = FILENAME.replace(/\.gz$/, '');
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
FILENAME = FILENAME.replace(/\.gz$/, '');

FILENAME = FILENAME.replace(/\.gz$/, '');
}
console.log(`Attempting to flash with file: ${FILENAME}...`);
await worker.flash(FILENAME);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
await worker.flash(FILENAME);
await worker.flash(FILENAME.replace(/\.gz$/, ''));

Are we deleting the zipped file after the unzipping. Just making sure the clean up happens.

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