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

Improve CI #201

Merged
merged 4 commits into from
Aug 13, 2024
Merged

Improve CI #201

merged 4 commits into from
Aug 13, 2024

Conversation

antonok-edm
Copy link
Collaborator

closes #199


const readResources = (() => {
return metadata.map(item => ({
name: item.name,
aliases: item.aliases,
kind: item.kind,
content: fs.readFileSync(path.join(__dirname, 'resources', item.resourcePath)).toString('base64')
content: fs.readFileSync(path.join(import.meta.dirname, 'resources', item.resourcePath)).toString('base64')

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

@@ -3,6 +3,7 @@
"version": "1.0.0",
"description": "Custom resources and scriptlets used for Brave's adblocker",
"main": "index.js",
"type": "module",
Copy link
Collaborator

Choose a reason for hiding this comment

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

why?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

required for import and stuff

@@ -1,30 +0,0 @@
name: CI
Copy link
Collaborator

Choose a reason for hiding this comment

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

why?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's still there; just moved it to be combined into the other workflow

import test from 'node:test'
import { Engine, FilterFormat, FilterSet } from 'adblock-rs'

const getIDFromBase64PublicKey = (key) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I thought you said we already have this code somewhere? Can we not just export that function?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's deep in brave-core-crx-packager internals. I considered using it directly but it doesn't seem worth it to expose that and pull it in as a new dependency here just for CI purposes

Copy link
Collaborator

@ShivanKaul ShivanKaul left a comment

Choose a reason for hiding this comment

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

LGTM, @bcaller can you take a look

@antonok-edm antonok-edm merged commit b663ee0 into master Aug 13, 2024
7 checks passed
@antonok-edm antonok-edm deleted the improve-ci branch August 13, 2024 20:54
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.

Verify component ids match public keys in CI
4 participants