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

Streamline packaging #844

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Streamline packaging #844

wants to merge 5 commits into from

Conversation

antonok-edm
Copy link
Collaborator

@antonok-edm antonok-edm commented Feb 9, 2024

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*.js script 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.js and generateManifestForRustAdblock.js into packageAdBlock.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-player
  • package-ethereum-remote-client
  • package-ad-block
  • package-tor-client
  • package-tor-pluggable-transports
  • package-ipfs-daemon
  • package-wallet-data-files
  • package-local-data-files
  • package-ntp-background-images
  • package-ntp-sponsored-images
  • package-ntp-super-referrer
  • package-ntp-super-referrer-mapping-table
  • package-playlist
  • package-user-model-installer-updates

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

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


util.installErrorHandlers()
this.stagingDir = path.join(rootBuildDir, 'staging', this.platformRegion)
Copy link
Contributor

Choose a reason for hiding this comment

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

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`)
Copy link
Contributor

Choose a reason for hiding this comment

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

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`)
Copy link
Contributor

Choose a reason for hiding this comment

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

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`)
Copy link
Contributor

Choose a reason for hiding this comment

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

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')
Copy link
Contributor

Choose a reason for hiding this comment

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

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`)
Copy link
Contributor

Choose a reason for hiding this comment

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

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)
Copy link
Contributor

Choose a reason for hiding this comment

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

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`)
Copy link
Contributor

Choose a reason for hiding this comment

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

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`)
Copy link
Contributor

Choose a reason for hiding this comment

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

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('build', TOR_PLUGGABLE_TRANSPORTS_UPDATER, this.platform)
Copy link
Contributor

Choose a reason for hiding this comment

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

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

@linhkikuchi
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants