This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Vulnerability Scanning on Third Party Deps #36506
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| name: Third party dependency scan | ||
| on: | ||
| # Only the default branch is supported. | ||
| branch_protection_rule: | ||
| branches: [ main ] | ||
| schedule: | ||
| - cron: "0 8 * * *" # runs daily at 08:00 | ||
|
|
||
|
|
||
| # Declare default permissions as read only. | ||
| permissions: read-all | ||
|
|
||
| jobs: | ||
| analysis: | ||
| name: Third party dependency scan | ||
| runs-on: ubuntu-latest | ||
sealesj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| permissions: | ||
| # Needed to upload the results to code-scanning dashboard. | ||
| security-events: write | ||
| actions: read | ||
| contents: read | ||
|
|
||
| steps: | ||
| - name: "Checkout code" | ||
| uses: actions/checkout@2541b1294d2704b0964813337f33b291d3f8596b | ||
| with: | ||
| persist-credentials: false | ||
|
|
||
| - name: setup python | ||
| uses: actions/setup-python@98f2ad02fd48d057ee3b4d4f66525b231c3e52b6 | ||
| with: | ||
| python-version: '3.7.7' # install the python version needed | ||
|
There was a problem hiding this comment. Looks like this is failing https://github.com/flutter/engine/actions/runs/3566233289 |
||
|
|
||
| - name: install dependency | ||
| run: pip install git+https://github.com/psf/requests.git@4d394574f5555a8ddcc38f707e0c9f57f55d9a3b | ||
|
|
||
| - name: execute py script | ||
| run: python ci/deps_parser.py | ||
|
|
||
| - name: parse deps_parser output.txt | ||
| run: python ci/scan_flattened_deps.py | ||
|
|
||
| # Upload the results as artifacts (optional). Commenting out will disable uploads of run results in SARIF | ||
| # format to the repository Actions tab. | ||
| - name: "Upload artifact" | ||
| uses: actions/upload-artifact@6673cd052c4cd6fcf4b4e6e60ea986c889389535 | ||
| with: | ||
| name: SARIF file | ||
| path: osvReport.sarif | ||
| retention-days: 5 | ||
|
|
||
| # Upload the results to GitHub's code scanning dashboard. | ||
| - name: "Upload to code-scanning" | ||
| uses: github/codeql-action/upload-sarif@a3a6c128d771b6b9bdebb1c9d0583ebd2728a108 | ||
| with: | ||
| sarif_file: osvReport.sarif | ||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |
| # Copyright 2013 The Flutter Authors. All rights reserved. | ||
| # Use of this source code is governed by a BSD-style license that can be | ||
| # found in the LICENSE file. | ||
| # | ||
| # Usage: deps_parser.py --deps <DEPS file> --output <flattened deps> | ||
| # | ||
| # This script parses the DEPS file, extracts the fully qualified dependencies | ||
|
|
@@ -12,11 +12,16 @@ | |
|
|
||
| import argparse | ||
| import os | ||
| import re | ||
sealesj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| import sys | ||
|
|
||
| SCRIPT_DIR = os.path.dirname(sys.argv[0]) | ||
| CHECKOUT_ROOT = os.path.realpath(os.path.join(SCRIPT_DIR, '..')) | ||
|
|
||
| CHROMIUM_README_FILE = 'third_party/accessibility/README.md' | ||
| CHROMIUM_README_COMMIT_LINE = 4 # the fifth line will always contain the commit hash | ||
| CHROMIUM = 'https://chromium.googlesource.com/chromium/src' | ||
|
|
||
|
|
||
| # Used in parsing the DEPS file. | ||
| class VarImpl: | ||
|
|
@@ -55,15 +60,33 @@ def parse_deps_file(deps_file): | |
| # Extract the deps and filter. | ||
| deps = local_scope.get('deps', {}) | ||
| filtered_deps = [] | ||
| for _, dep in deps.items(): | ||
| # We currently do not support packages or cipd which are represented | ||
| # as dictionaries. | ||
| if isinstance(dep, str): | ||
| print(dep) | ||
sealesj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| filtered_deps.append(dep) | ||
| return filtered_deps | ||
|
|
||
|
|
||
| def parse_readme(deps): | ||
| """ | ||
| Opens the Flutter Accessibility Library README and uses the commit hash | ||
| found in the README to check for viulnerabilities. | ||
| The commit hash in this README will always be in the same format | ||
| """ | ||
| file_path = os.path.join(CHECKOUT_ROOT, CHROMIUM_README_FILE) | ||
| file = open(file_path) | ||
sealesj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| # read the content of the file opened | ||
| content = file.readlines() | ||
| commit_line = content[CHROMIUM_README_COMMIT_LINE] | ||
| print('commit line: ' + commit_line) | ||
|
|
||
| commit = re.search(r'(?<=\[).*(?=\])', commit_line) | ||
| print(CHROMIUM + '@' + commit.group()) | ||
|
|
||
| deps.append(CHROMIUM + '@' + commit.group()) | ||
| return deps | ||
|
|
||
|
|
||
| def write_manifest(deps, manifest_file): | ||
| print('\n'.join(sorted(deps))) | ||
| with open(manifest_file, 'w') as manifest: | ||
|
|
@@ -97,6 +120,7 @@ def parse_args(args): | |
| def main(argv): | ||
| args = parse_args(argv) | ||
| deps = parse_deps_file(args.deps) | ||
| deps = parse_readme(deps) | ||
| write_manifest(deps, args.output) | ||
| return 0 | ||
|
|
||
|
|
||
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
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,95 @@ | ||||||
| #!/usr/bin/env python3 | ||||||
| # | ||||||
| # Copyright 2013 The Flutter Authors. All rights reserved. | ||||||
| # Use of this source code is governed by a BSD-style license that can be | ||||||
| # found in the LICENSE file. | ||||||
|
|
||||||
| import os | ||||||
sealesj marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| import sys | ||||||
| import unittest | ||||||
| from deps_parser import VarImpl | ||||||
|
|
||||||
| SCRIPT_DIR = os.path.dirname(sys.argv[0]) | ||||||
| CHECKOUT_ROOT = os.path.realpath(os.path.join(SCRIPT_DIR, '..')) | ||||||
| DEPS = os.path.join(CHECKOUT_ROOT, 'DEPS') | ||||||
| UPSTREAM_PREFIX = 'upstream_' | ||||||
|
|
||||||
|
|
||||||
| class TestDepsParserMethods(unittest.TestCase): | ||||||
|
|
||||||
| # extract both mirrored dep names and URLs & | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| # upstream names and URLs from DEPs file | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| def setUp(self): | ||||||
|
There was a problem hiding this comment.
Suggested change
There was a problem hiding this comment. it needs to be setUp for the python unittest framework There was a problem hiding this comment. lower-camel-case is inconsistent with the rest of the file |
||||||
| with open(DEPS) as file: | ||||||
sealesj marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| local_scope_upstream = {} | ||||||
| global_scope_upstream = {'Var': lambda x: x} # dummy lambda | ||||||
| # Read the content. | ||||||
| with open(DEPS, 'r') as file: | ||||||
| deps_content = file.read() | ||||||
|
|
||||||
| # Eval the content. | ||||||
| exec(deps_content, global_scope_upstream, local_scope_upstream) | ||||||
|
|
||||||
| # Extract the upstream URLs | ||||||
| # vars contains more than just upstream URLs | ||||||
| # however the upstream URLs are prefixed with 'upstream_' | ||||||
| upstream = local_scope_upstream.get('vars') | ||||||
| self.upstream_urls = upstream | ||||||
|
|
||||||
| local_scope_mirror = {} | ||||||
| var = VarImpl(local_scope_mirror) | ||||||
| global_scope_mirror = { | ||||||
| 'Var': var.lookup, | ||||||
| 'deps_os': {}, | ||||||
| } | ||||||
|
|
||||||
| # Eval the content. | ||||||
| exec(deps_content, global_scope_mirror, local_scope_mirror) | ||||||
|
There was a problem hiding this comment. I'm not sure why this needs to be done twice. Could the code from lines 36-37 be moved after here (using |
||||||
|
|
||||||
| # Extract the deps and filter. | ||||||
| deps = local_scope_mirror.get('deps', {}) | ||||||
| filtered_deps = [] | ||||||
| for _, dep in deps.items(): | ||||||
| # We currently do not support packages or cipd which are represented | ||||||
| # as dictionaries. | ||||||
| if isinstance(dep, str): | ||||||
| filtered_deps.append(dep) | ||||||
|
|
||||||
| self.deps = filtered_deps | ||||||
|
|
||||||
| def test_each_dep_has_upstream_url(self): | ||||||
| # for each DEP in the deps file, check for an associated upstream URL in deps file | ||||||
sealesj marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| for dep in self.deps: | ||||||
| dep_repo = dep.split('@')[0] | ||||||
| dep_name = dep_repo.split('/')[-1].split('.')[0] | ||||||
| # vulkan-deps and khronos do not have one upstream URL | ||||||
| # all other deps should have an associated upstream URL for vuln scanning purposes | ||||||
| if dep_name not in ('vulkan-deps', 'khronos'): | ||||||
| # add the prefix on the dep name when searching for the upstream entry | ||||||
sealesj marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| self.assertTrue( | ||||||
| UPSTREAM_PREFIX + dep_name in self.upstream_urls, | ||||||
| msg=dep_name + ' not found in upstream URL list' | ||||||
| ) | ||||||
|
|
||||||
| def test_each_upstream_url_has_dep(self): | ||||||
|
|
||||||
sealesj marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| # parse DEPS into dependency names | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| deps_names = [] | ||||||
| for dep in self.deps: | ||||||
| dep_repo = dep.split('@')[0] | ||||||
| dep_name = dep_repo.split('/')[-1].split('.')[0] | ||||||
| deps_names.append(dep_name) | ||||||
|
|
||||||
| # for each upstream URL dep, check it exists as in DEPS | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| for upsream_dep in self.upstream_urls: | ||||||
| # only test on upstream deps in vars section which start with the upstream prefix | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| if upsream_dep.startswith(UPSTREAM_PREFIX): | ||||||
| # strip the prefix to check that it has a corresponding dependency in the DEPS file | ||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||
| self.assertTrue( | ||||||
| upsream_dep[len(UPSTREAM_PREFIX):] in deps_names, | ||||||
| msg=upsream_dep + ' from upstream list not found in DEPS' | ||||||
|
There was a problem hiding this comment. Does this also need to check the other way around? That is, that there is an entry in Also, ideally, the error message would explain how to fix the problem. There was a problem hiding this comment. I test the other way around in the above method |
||||||
| ) | ||||||
|
|
||||||
|
|
||||||
| if __name__ == '__main__': | ||||||
| unittest.main() | ||||||
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if/when this fails?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this workflow is not connected back to the presubmit checks, failure on the workflow will amount to just failure within the actions tab and notify the person who triggered it. If desired, I could connect to a presubmit check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does "Notify the person who triggered it" mean? I'd prefer a notification, and not anything that could block developers, like a presubmit check. The new test added below should be the only new thing that engine team members need to pay attention to unless there's a vulnerability detected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not a presubmit check and does not block developers. The user who opened the PR will receive an email if the vulnerability scanning has failed.