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

Chrome extensions support #534

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

alessandro-Doyensec
Copy link
Contributor

Extraction of chrome extensions support for scalibr

Resources:

Notes:

I used runtime.GOOS to detect the operating system where scalibr is running. This approach works when the scan target is the host machine, but it doesn't function as expected when scanning a remote image.

Do scalibr plugins have a way to detect if the scan target is running a different operating system than the host?

flatpak.Name: {flatpak.NewDefault},
homebrew.Name: {homebrew.New},
macapps.Name: {macapps.NewDefault},
chromeextensions.Name: {chromeextensions.New},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move into a separate Misc list

// limitations under the License.

// Package extensions extracts chrome extensions.
package extensions
Copy link
Collaborator

Choose a reason for hiding this comment

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

Move into filesystem/misc/

path = filepath.ToSlash(path)

switch runtime.GOOS {
case "windows":
Copy link
Collaborator

Choose a reason for hiding this comment

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

The ScanConfig receives a Capabilities struct that contains which OS SCALIBR is running on.

One thing you could do to have OS-specific behavior is to have 3 separate extractors: One for Linux, Win, and Mac. Each would set Capabilities{OS: plugin.OSLinux}, etc. in their Requirements() function.

When a user enables extractors through the SCALIBR library, they either call list.FromCapabilities or list.FilterByCapabilities to only enable the one for the OS they're running SCALIBR on. Similarly, when running SCALIBR through the CLI, cli.go already calls FilterByCapabilities so the extractors not relevant to the OS will be disabled automatically.

You can avoid code duplication by moving the common parts of Extract, ToPURL, etc. into a helper library that they all call.

return nil, fmt.Errorf("bad format in manifest %s: %w", input.Path, err)
}

// extract the extensions ID from the path
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need for this comment, the function name already says the same thing

//
// /extensionID/version/manifest.json
func extractExtensionsIDFromPath(input *filesystem.ScanInput) (string, error) {
path, err := filepath.Abs(input.Path)
Copy link
Collaborator

Choose a reason for hiding this comment

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

file paths should already be absolute when receiving them through the ScanInput so no need for this.

// extractLocaleInfo extract locale information from the _locales/LOCALE_CODE/messages.json
// following manifest.json v3 specification
func extractLocaleInfo(m *manifest, input *filesystem.ScanInput) error {
messagePath := filepath.Join(filepath.Dir(input.Path), "/_locales/", m.DefaultLocale, "message.json")
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need for the leading and ending slashes in "/_locales/" if you use filepath.Join

return nil
}

// cutPrefixSuffix cuts the specified prefix and suffix
Copy link
Collaborator

Choose a reason for hiding this comment

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

"...if they exist, returns false otherwise"

func (e Extractor) Requirements() *plugin.Capabilities { return &plugin.Capabilities{} }

// FileRequired returns true if the file is chrome manifest extension
func (e Extractor) FileRequired(api filesystem.FileAPI) bool {
Copy link
Member

Choose a reason for hiding this comment

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

This function is called on every single file of the filesystem and hence can be a bottleneck for performances. I would recommend performing a pre-check to return early before using regexp.

For example, just returning early if the filename is not manifest.json before doing the regexp matching. This should significantly improve performances.

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.

3 participants