Add script to update q-expand from source XLSX#570
Open
zachmargolis wants to merge 2 commits intomainfrom
Open
Add script to update q-expand from source XLSX#570zachmargolis wants to merge 2 commits intomainfrom
zachmargolis wants to merge 2 commits intomainfrom
Conversation
- Source data can be downloaded from GSA D2D with GSA login (instructions in q-expand-update.js)
| ["stub", "desc"], | ||
| ...outRows.map(({ stub, desc }) => [stub, desc]), | ||
| ]); | ||
| fs.writeFileSync(outCsvPath, output); |
Check failure
Code scanning / CodeQL
Insecure temporary file High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 year ago
To fix the problem, we should use a well-tested library like tmp to create temporary files securely. This library ensures that the file is inaccessible to other users and that the file does not already exist. We need to replace the hardcoded temporary file paths with paths generated by the tmp library.
- Install the
tmplibrary. - Replace the hardcoded temporary file paths in
src/scripts/q-expand-update.test.jswith paths generated by thetmplibrary. - Ensure that the rest of the code uses these secure temporary file paths.
Suggested changeset
2
src/scripts/q-expand-update.test.js
Outside changed files
| @@ -2,2 +2,3 @@ | ||
| const { parse } = require("csv-parse/sync"); // eslint-disable-line import/no-unresolved | ||
| const tmp = require("tmp"); | ||
| const Fs = require("fake-fs"); | ||
| @@ -7,3 +8,3 @@ | ||
| describe("q-expand-update", () => { | ||
| const inXlsxPath = "/tmp/path/to/source.xlsx"; | ||
| const inXlsxPath = tmp.fileSync({ postfix: ".xlsx" }).name; | ||
| const xlsxData = [ | ||
| @@ -23,3 +24,3 @@ | ||
| it("converts xlsx into csv", () => { | ||
| const outCsvPath = "/tmp/path/to/out.csv"; | ||
| const outCsvPath = tmp.fileSync({ postfix: ".csv" }).name; | ||
|
|
package.json
Outside changed files
| @@ -28,3 +28,4 @@ | ||
| "pg": "^8.7.1", | ||
| "plural": "^1.1.0" | ||
| "plural": "^1.1.0", | ||
| "tmp": "^0.2.3" | ||
| }, |
This fix introduces these dependencies
| Package | Version | Security advisories |
| tmp (npm) | 0.2.3 | None |
Copilot is powered by AI and may make mistakes. Always verify output.
Contributor
Author
There was a problem hiding this comment.
This spec already uses fake-fs which I think means this suggestion may not apply
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I had an idea for streamlining the update process, I am happy to update the wiki page on this if folks think this is a viable approach!
I know the current approach allows for some custom renaming, for the time being, I went with direct conversion.
Checklist:
page has been updated if Charlie needs any new OAuth events or scopes
wiki page has been updated if new environment variables were introduced
or existing ones changed