Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
scripts/packageTorPluggableTransports.js|57| [semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal
Cc @thypon @bcaller
scripts/packageTorPluggableTransports.js|61| [semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal
Cc @thypon @bcaller
scripts/packageWalletDataFiles.js|14| [semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal
Cc @thypon @bcaller
scripts/packageWalletDataFiles.js|30| [semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal
Cc @thypon @bcaller
scripts/packageWalletDataFiles.js|35| [semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal
Cc @thypon @bcaller
| this.componentId = componentData.id | ||
|
|
||
| util.installErrorHandlers() | ||
| this.stagingDir = path.join(rootBuildDir, 'staging', this.platformRegion) |
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal
Cc @thypon @bcaller
|
|
||
| util.installErrorHandlers() | ||
| this.stagingDir = path.join(rootBuildDir, 'staging', this.platformRegion) | ||
| this.crxFile = path.join(rootBuildDir, 'output', `ntp-sponsored-images-${this.platformRegion}.crx`) |
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal
Cc @thypon @bcaller
| .parse(process.argv) | ||
| privateKeyFromDir (keyDir) { | ||
| // Desktop private key file names do not have the -desktop suffix, but android has -android | ||
| return path.join(keyDir, `ntp-sponsored-images-${this.platformRegion.replace('-desktop', '')}.pem`) |
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal
Cc @thypon @bcaller
| } | ||
|
|
||
| privateKeyFromDir (keyDir) { | ||
| return path.join(keyDir, `${this.crxName}.pem`) |
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal
Cc @thypon @bcaller
| async stageFiles (version, outputDir) { | ||
| const regions = await getRegionalLists() | ||
| const regionalCatalogString = JSON.stringify(regions) | ||
| const regionalCatalogPath = path.join(outputDir, 'regional_catalog.json') |
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal
Cc @thypon @bcaller
| this.componentId = componentId | ||
| this.superReferrerName = superReferrerName | ||
| this.stagingDir = path.join(rootBuildDir, 'staging', this.superReferrerName) | ||
| this.crxFile = path.join(rootBuildDir, 'output', `ntp-super-referrer-${this.superReferrerName}.crx`) |
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal
Cc @thypon @bcaller
| .option('-d, --keys-directory <dir>', 'directory containing private keys for signing crx files', 'abc') | ||
| .option('-f, --key-file <file>', 'private key file for signing crx', 'key.pem')) | ||
| .parse(process.argv) | ||
| this.stagingDir = path.join('build', 'tor-client-updater', this.platform) |
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal
Cc @thypon @bcaller
| .option('-f, --key-file <file>', 'private key file for signing crx', 'key.pem')) | ||
| .parse(process.argv) | ||
| this.stagingDir = path.join('build', 'tor-client-updater', this.platform) | ||
| this.crxFile = path.join('build', 'tor-client-updater', `tor-client-updater-${this.platform}.crx`) |
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal
Cc @thypon @bcaller
|
|
||
| let keyParam = '' | ||
| privateKeyFromDir (keyDir) { | ||
| return path.join(keyDir, `tor-client-updater-${this.platform}.pem`) |
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal
Cc @thypon @bcaller
| this.componentId = util.getIDFromBase64PublicKey(parsedManifest.key) | ||
|
|
||
| util.installErrorHandlers() | ||
| this.stagingDir = path.join('build', TOR_PLUGGABLE_TRANSPORTS_UPDATER, this.platform) |
There was a problem hiding this comment.
reported by reviewdog 🐶
[semgrep] Detected possible user input going into a path.join or path.resolve function. This could possibly lead to a path traversal vulnerability, where the attacker can access arbitrary files stored in the file system. Instead, be sure to sanitize or validate user input first.
Source: https://semgrep.dev/r/javascript.lang.security.audit.path-traversal.path-join-resolve-traversal.path-join-resolve-traversal
Cc @thypon @bcaller
|
Nice, FYI I'm doing another PR here https://github.com/brave/devops/pull/11212 to shrink the codebase and avoid copy-paste for all extensions pipelines also |
4 participants
The purpose of this PR is to trim the copy-pasted boilerplate at the end of each packaging script (
generateCRXFile,util.installErrorHandlers,util.addCommonScriptOptions,keyDir,util.createTableIfNotExists, etc.).Each
package*.jsscript now contains a much more complete, declarative view of what goes into and comes out of each component. Slightly less-used functionality (e.g.--local-run, file content hashing) is now directly available for use by all of the scripts.I took the added step of moving logic from
generateAdBlockRustDataFiles.jsandgenerateManifestForRustAdblock.jsintopackageAdBlock.js. I'd like to do this for other components, but I need to understand them better first.This PR shrinks the codebase by about 300 lines. Note that most of the "added" lines consist of moved/reorganized code.
Scripts tested in staging:
package-brave-playerpackage-ethereum-remote-clientpackage-ad-blockpackage-tor-clientpackage-tor-pluggable-transportspackage-ipfs-daemonpackage-wallet-data-filespackage-local-data-filespackage-ntp-background-imagespackage-ntp-sponsored-imagespackage-ntp-super-referrerpackage-ntp-super-referrer-mapping-tablepackage-playlistpackage-user-model-installer-updates