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

fix xpi private cot verification #455

Merged
merged 4 commits into from
May 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions HISTORY.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,18 @@ Change Log
All notable changes to this project will be documented in this file.
This project adheres to `Semantic Versioning <http://semver.org/>`__.

[34.2.0] - 2020-05-26
---------------------

Changed
~~~~~~~
- Github source urls starting with ``ssh://`` are now treated as private repositories.
- ``verify_cot`` now takes ``--verbose`` and ``--no-check-task`` options.

Fixed
~~~~~
- ``test_production`` should no longer leave behind temp ``...`` directories.

[34.1.0] - 2020-05-04
---------------------

Expand Down
18 changes: 10 additions & 8 deletions src/scriptworker/cot/verify.py
Original file line number Diff line number Diff line change
Expand Up @@ -1303,7 +1303,9 @@ async def get_in_tree_template(link):
raise CoTError("{} source url {} doesn't end in .yml or .yaml!".format(link.name, source_url))

auth = None
if (any(vcs_rule.get("require_secret") for vcs_rule in context.config["trusted_vcs_rules"])) and "github.com" in source_url:
if (
source_url.startswith("ssh://") or any(vcs_rule.get("require_secret") for vcs_rule in context.config["trusted_vcs_rules"])
) and "github.com" in source_url:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be ok if you wanted to drop require_secret entirely -- guardian no longer uses it... but its fine to leave.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm. XPI doesn't have the requirement of releasing off these private source repos (we release off the manifest repo, which is public and known). We still have a check in is_pull_request that checks for has_commit_landed_on_repository. That check is essentially skipped if we have require_secret set. I don't know if we still need that behavior. I'll leave it for now, but we may want to revisit.

Copy link
Contributor

Choose a reason for hiding this comment

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

we can't test has_commit_landed_on_repository for a private repository. I don't have a need for this skippage anymore for guardian at all, but its worth knowing that there is no other way to get this information [that I could find] which would let us use a token.

It's essentially github-web-secret-api

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#456 for followup.
I may need actual private repo releases for adhoc, so hopefully I'll have a better understanding in a bit.

newurl = re.sub(
r"^(?:ssh://|https?://)(?:[^@/\:]*(?:\:[^@/\:]*)?@)?github.com(?:\:\d*)?/(?P<repopath>.*)/raw/(?P<sha>[a-zA-Z0-9]*)/(?P<filepath>.*)$",
r"https://raw.githubusercontent.com/\g<repopath>/\g<sha>/\g<filepath>",
Expand Down Expand Up @@ -1969,11 +1971,8 @@ async def _async_verify_cot_cmdln(opts, tmp):
if os.environ.get("SCRIPTWORKER_GITHUB_OAUTH_TOKEN"):
context.config["github_oauth_token"] = os.environ.get("SCRIPTWORKER_GITHUB_OAUTH_TOKEN")
cot = ChainOfTrust(context, opts.task_type, task_id=opts.task_id)
await verify_chain_of_trust(cot, check_task=True)
log.info(format_json(cot.dependent_task_ids()))
log.info("{} : {}".format(cot.name, cot.task_id))
for link in cot.links:
log.info("{} : {}".format(link.name, link.task_id))
check_task = opts.no_check_task is False
await verify_chain_of_trust(cot, check_task=check_task)


def verify_cot_cmdln(args=None, event_loop=None):
Expand Down Expand Up @@ -2008,11 +2007,14 @@ def verify_cot_cmdln(args=None, event_loop=None):
parser.add_argument("--cleanup", help="clean up the temp dir afterwards", dest="cleanup", action="store_true", default=False)
parser.add_argument("--cot-product", help="the product type to test", default="firefox")
parser.add_argument("--verify-sigs", help="enable signature verification", action="store_true", default=False)
parser.add_argument("--verbose", "-v", help="enable debug logging", action="store_true", default=False)
parser.add_argument("--no-check-task", help="skip verifying the taskId's cot status", action="store_true", default=False)
opts = parser.parse_args(args)
tmp = tempfile.mkdtemp()
log = logging.getLogger("scriptworker")
log.setLevel(logging.DEBUG)
logging.basicConfig()
level = logging.DEBUG if opts.verbose else logging.INFO
log.setLevel(level)
logging.basicConfig(level=level)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

event_loop = event_loop or asyncio.get_event_loop()
try:
event_loop.run_until_complete(_async_verify_cot_cmdln(opts, tmp))
Expand Down
2 changes: 1 addition & 1 deletion src/scriptworker/version.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def get_version_string(version: Union[ShortVerType, LongVerType]) -> str:

# 1}}}
# Semantic versioning 2.0.0 http://semver.org/
__version__ = (34, 1, 0)
__version__ = (34, 2, 0)
__version_string__ = get_version_string(__version__)


Expand Down
11 changes: 8 additions & 3 deletions tests/test_production.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,10 @@ def build_config(override, basedir):
config.update(
{
"log_dir": os.path.join(basedir, "log"),
"base_artifact_dir": os.path.join(basedir, "artifact"),
"task_log_dir_template": os.path.join(basedir, "artifact", "public", "logs"),
"base_work_dir": os.path.join(basedir, "work"),
"artifact_dir": os.path.join(basedir, "artifact"),
"task_log_dir": os.path.join(basedir, "artifact", "public", "logs"),
"work_dir": os.path.join(basedir, "work"),
"ed25519_private_key_path": "",
}
)
del config["credentials"]
Expand All @@ -45,6 +46,10 @@ def build_config(override, basedir):
with open(os.path.join(basedir, "config.json"), "w") as fh:
json.dump(config, fh, indent=2, sort_keys=True)
config = apply_product_config(config)
# Avoid creating a `...` directory
for k, v in config.items():
if v == "...":
raise Exception(f"Let's not keep any '...' config values. {k} is {v}!")
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice 👍

return config


Expand Down
4 changes: 2 additions & 2 deletions version.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{
"version":[
34,
1,
2,
0
],
"version_string":"34.1.0"
"version_string":"34.2.0"
}