Skip to content

Commit

Permalink
Try #348:
Browse files Browse the repository at this point in the history
  • Loading branch information
bors[bot] authored Feb 14, 2021
2 parents e816a67 + 20c7c29 commit 51116de
Show file tree
Hide file tree
Showing 28 changed files with 172 additions and 18 deletions.
2 changes: 1 addition & 1 deletion Project.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name = "RegistryCI"
uuid = "0c95cc5f-2f7e-43fe-82dd-79dbcba86b32"
authors = ["Dilum Aluthge <dilum@aluthge.com>", "Fredrik Ekre <ekrefredrik@gmail.com>", "contributors"]
version = "6.3.1"
version = "6.4.0"

[deps]
Base64 = "2a0f44e3-6c83-55bd-87e4-b1978d98bd5f"
Expand Down
50 changes: 50 additions & 0 deletions src/AutoMerge/dependency_confusion.jl
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# TODO: Add a more thorough explanation of the dependency confusion
# vulnerability and how this guideline mitigates it.

const guideline_dependency_confusion =
Guideline("No UUID conflict with other registries.",
data -> has_no_dependency_confusion(data.pkg,
data.registry_head,
data.public_registries))

# TODO: Needs a strategy to handle connection failures for the public
# registries. Preferably they should also be cloned only once and then
# just updated to mitigate the effect of them being temporarily
# offline. This could be implemented with the help of the Scratch
# package, but requires Julia >= 1.5.
function has_no_dependency_confusion(pkg, registry_head, public_registries)
# We know the name of this package but not its uuid. Look it up in
# the registry that includes the current PR.
packages = Pkg.TOML.parsefile(joinpath(registry_head, "Registry.toml"))["packages"]
filter!(packages) do (key, value)
value["name"] == pkg
end
# For Julia >= 1.4 this can be simplified with the `only` function.
always_assert(length(packages) == 1)
uuid = first(keys(packages))
# Also need to find out the package repository.
repo = Pkg.TOML.parsefile(joinpath(registry, package_path, "Package.toml"))["repo"]
for repo in public_registries
registry = clone_repo(repo)
registry_toml = Pkg.TOML.parsefile(joinpath(registry_head, "Registry.toml"))
packages = registry_toml["packages"]
if haskey(packages, uuid)
message = string("UUID $uuid conflicts with the package ",
packages[uuid]["name"], " in registry ",
registry_toml["name"], " at $repo.\n",
"This could be a dependency confusion attack.")
# Conflict detected. This is benign if the package name
# *and* the package URL matches.
if packages[uuid]["name"] != pkg
return false, message
end
package_path = packages[uuid]["path"]
other_repo = Pkg.TOML.parsefile(joinpath(registry, package_path, "Package.toml"))["repo"]
if repo != other_repo
return false, message
end
end
end

return true, ""
end
8 changes: 7 additions & 1 deletion src/AutoMerge/new-package.jl
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ function pull_request_build(data::GitHubAutoMergeData, ::NewPackage)::Nothing
# 13. Version can be loaded
# - once it's been installed (and built?), can we load the code?
# - i.e. can we run `import Foo`
# 14. Package UUID doesn't conflict with an UUID in the provided
# list of package registries. The exception is if also the
# package name *and* package URL matches those in the other
# registry, in which case this is a valid co-registration.
this_is_jll_package = is_jll_name(data.pkg)
@info("This is a new package pull request",
pkg = data.pkg,
Expand Down Expand Up @@ -89,7 +93,9 @@ function pull_request_build(data::GitHubAutoMergeData, ::NewPackage)::Nothing
(guideline_name_ascii, true), # 11
(:update_status, true),
(guideline_version_can_be_pkg_added, true), # 12
(guideline_version_can_be_imported, true)] # 13
(guideline_version_can_be_imported, true), # 13
(:update_status, true),
(guideline_dependency_confusion, true)] # 14

checked_guidelines = Guideline[]

Expand Down
10 changes: 8 additions & 2 deletions src/AutoMerge/public.jl
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,12 @@ function run(env = ENV,
suggest_onepointzero::Bool = true,
#
registry_deps::Vector{<:AbstractString} = String[],
api_url::String="https://api.github.com")::Nothing
api_url::String="https://api.github.com",
# A list of public Julia registries (repository URLs)
# which will be checked for UUID collisions in order to
# mitigate the dependency confusion vulnerability. See
# the `dependency_confusion.jl` file for details.
public_registries::Vector{<:AbstractString} = String[])::Nothing
all_statuses = deepcopy(additional_statuses)
all_check_runs = deepcopy(additional_check_runs)
push!(all_statuses, "automerge/decision")
Expand Down Expand Up @@ -72,7 +77,8 @@ function run(env = ENV,
master_branch_is_default_branch = master_branch_is_default_branch,
suggest_onepointzero = suggest_onepointzero,
whoami = whoami,
registry_deps = registry_deps)
registry_deps = registry_deps,
public_registries = public_registries)
else
always_assert(run_merge_build)
cron_or_api_build(api,
Expand Down
12 changes: 8 additions & 4 deletions src/AutoMerge/pull-requests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ function pull_request_build(api::GitHub.GitHubAPI,
master_branch::String,
master_branch_is_default_branch::Bool,
suggest_onepointzero::Bool,
registry_deps::Vector{<:AbstractString} = String[])::Nothing
registry_deps::Vector{<:AbstractString} = String[],
public_registries::Vector{<:AbstractString} = String[])::Nothing
pr = my_retry(() -> GitHub.pull_request(api, registry, pr_number; auth=auth))
_github_api_pr_head_commit_sha = pull_request_head_sha(pr)
if current_pr_head_commit_sha != _github_api_pr_head_commit_sha
Expand All @@ -86,7 +87,8 @@ function pull_request_build(api::GitHub.GitHubAPI,
master_branch_is_default_branch=master_branch_is_default_branch,
suggest_onepointzero=suggest_onepointzero,
whoami=whoami,
registry_deps=registry_deps)
registry_deps=registry_deps,
public_registries=public_registries)
return result
end

Expand All @@ -103,7 +105,8 @@ function pull_request_build(api::GitHub.GitHubAPI,
master_branch_is_default_branch::Bool,
suggest_onepointzero::Bool,
whoami::String,
registry_deps::Vector{<:AbstractString} = String[])::Nothing
registry_deps::Vector{<:AbstractString} = String[],
public_registries::Vector{<:AbstractString} = String[])::Nothing
# 1. Check if the PR is open, if not quit.
# 2. Determine if it is a new package or new version of an
# existing package, if neither quit.
Expand Down Expand Up @@ -159,7 +162,8 @@ function pull_request_build(api::GitHub.GitHubAPI,
registry_master = registry_master,
suggest_onepointzero = suggest_onepointzero,
whoami = whoami,
registry_deps = registry_deps)
registry_deps = registry_deps,
public_registries = public_registries)
pull_request_build(data, registration_type)
rm(registry_master; force = true, recursive = true)
return nothing
Expand Down
6 changes: 6 additions & 0 deletions src/AutoMerge/types.jl
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,12 @@ struct GitHubAutoMergeData
# List of dependent registries. Typically this would contain
# "General" when running automerge for a private registry.
registry_deps::Vector{String}

# A list of public Julia registries (repository URLs) which will
# be checked for UUID collisions in order to mitigate the
# dependency confusion vulnerability. See the
# `dependency_confusion.jl` file for details.
public_registries::Vector{String}
end

# Constructor that requires all fields as named arguments.
Expand Down
9 changes: 9 additions & 0 deletions test/automerge-integration-utils.jl
Original file line number Diff line number Diff line change
Expand Up @@ -284,3 +284,12 @@ function with_feature_branch(f::Function,
result = f(b)
return result
end

function generate_public_registry(public_dir::AbstractString, GIT)
public_git_repo = mktempdir()
cp(public_dir, public_git_repo, force = true)
run(`$(GIT) -C $(public_git_repo) init`)
run(`$(GIT) -C $(public_git_repo) add .`)
run(`$(GIT) -C $(public_git_repo) commit -m "create"`)
return public_git_repo
end
30 changes: 20 additions & 10 deletions test/automerge-integration.jl
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,28 @@ delete_old_pull_request_branches(
)

@testset "Integration tests" begin
for (test_number, master_dir, feature_dir, title, pass) in [
(1, "master_1", "feature_1", "New package: Requires v1.0.0", true), # OK: new package
(2, "master_2", "feature_2", "New version: Requires v2.0.0", true), # OK: new version
(3, "master_1", "feature_3", "New package: Req v1.0.0", false), # FAIL: name too short
(4, "master_2", "feature_4", "New version: Requires v2.0.1", false), # FAIL: skips v2.0.0
(5, "master_3", "feature_5", "New version: Requires v2.0.0", false), # FAIL: modifies extra file
(6, "master_1", "feature_6", "New package: HelloWorldC_jll v1.0.6+0", true), # OK: new JLL package
(7, "master_4", "feature_7", "New version: HelloWorldC_jll v1.0.8+0", true), # OK: new JLL version
(8, "master_1", "feature_8", "New package: HelloWorldC_jll v1.0.6+0", false), # FAIL: unallowed dependency
for (test_number, master_dir, feature_dir, public_dir, title, pass) in [
(1, "master_1", "feature_1", "", "New package: Requires v1.0.0", true), # OK: new package
(2, "master_2", "feature_2", "", "New version: Requires v2.0.0", true), # OK: new version
(3, "master_1", "feature_3", "", "New package: Req v1.0.0", false), # FAIL: name too short
(4, "master_2", "feature_4", "", "New version: Requires v2.0.1", false), # FAIL: skips v2.0.0
(5, "master_3", "feature_5", "", "New version: Requires v2.0.0", false), # FAIL: modifies extra file
(6, "master_1", "feature_6", "", "New package: HelloWorldC_jll v1.0.6+0", true), # OK: new JLL package
(7, "master_4", "feature_7", "", "New version: HelloWorldC_jll v1.0.8+0", true), # OK: new JLL version
(8, "master_1", "feature_8", "", "New package: HelloWorldC_jll v1.0.6+0", false), # FAIL: unallowed dependency
(9, "master_1", "feature_1", "public_1", "New package: Requires v1.0.0", true), # OK: no UUID conflict
(10, "master_1", "feature_1", "public_2", "New package: Requires v1.0.0", false), # FAIL: UUID conflict, name differs
(11, "master_1", "feature_1", "public_3", "New package: Requires v1.0.0", false), # FAIL: UUID conflict, repo differs
(12, "master_1", "feature_1", "public_4", "New package: Requires v1.0.0", true), # OK: UUID conflict, name and repo match
]
@info "Performing integration tests with settings" test_number master_dir feature_dir title pass
with_master_branch(templates(master_dir), "master"; GIT = GIT, repo_url = repo_url_with_auth) do master
with_feature_branch(templates(feature_dir), master; GIT = GIT, repo_url = repo_url_with_auth) do feature
public_registries = String[]
if public_dir != ""
public_git_repo = generate_public_registry(public_dir, GIT)
push!(public_registries, "file://$(public_git_repo)/.git")
end
head = feature
base = master
params = Dict("title" => title,
Expand Down Expand Up @@ -75,7 +84,8 @@ delete_old_pull_request_branches(
authorized_authors_special_jll_exceptions = String[whoami],
error_exit_if_automerge_not_applicable = true,
master_branch = master,
master_branch_is_default_branch = false)
master_branch_is_default_branch = false,
public_registries = public_registries)
@info "Running integration test for " test_number master_dir feature_dir title pass
if pass
run_thunk()
Expand Down
2 changes: 2 additions & 0 deletions test/templates/public_1/E/Example/Compat.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
["0.5.3-0"]
julia = "1"
Empty file.
3 changes: 3 additions & 0 deletions test/templates/public_1/E/Example/Package.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
name = "Example"
uuid = "7876af07-990d-54b4-ab0e-23690620f79a"
repo = "https://github.com/JuliaLang/Example.jl.git"
2 changes: 2 additions & 0 deletions test/templates/public_1/E/Example/Versions.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
["0.5.3"]
git-tree-sha1 = "46e44e869b4d90b96bd8ed1fdcf32244fddfb6cc"
8 changes: 8 additions & 0 deletions test/templates/public_1/Registry.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
name = "PublicRegistry"
uuid = "1003c4b9-3c72-409d-ba60-5578a18ea1a7"
repo = ""

description = "This is a test registry for the AutoMerge integration tests."

[packages]
7876af07-990d-54b4-ab0e-23690620f79a = { name = "Example", path = "E/Example" }
2 changes: 2 additions & 0 deletions test/templates/public_2/R/Req/Compat.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[1-2]
julia = "1"
Empty file.
3 changes: 3 additions & 0 deletions test/templates/public_2/R/Req/Package.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
name = "Req"
uuid = "ae029012-a4dd-5104-9daa-d747884805df"
repo = "https://github.com/MikeInnes/Requires.jl.git"
2 changes: 2 additions & 0 deletions test/templates/public_2/R/Req/Versions.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
["1.0.0"]
git-tree-sha1 = "999513b7dea8ac17359ed50ae8ea089e4464e35e"
8 changes: 8 additions & 0 deletions test/templates/public_2/Registry.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
name = "PublicRegistry"
uuid = "1003c4b9-3c72-409d-ba60-5578a18ea1a7"
repo = ""

description = "This is a test registry for the AutoMerge integration tests."

[packages]
ae029012-a4dd-5104-9daa-d747884805df = { name = "Req", path = "R/Req" }
2 changes: 2 additions & 0 deletions test/templates/public_3/R/Requires/Compat.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[1-2]
julia = "1"
Empty file.
3 changes: 3 additions & 0 deletions test/templates/public_3/R/Requires/Package.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
name = "Requires"
uuid = "ae029012-a4dd-5104-9daa-d747884805df"
repo = "https://github.com/JuliaLang/Example.jl.git"
5 changes: 5 additions & 0 deletions test/templates/public_3/R/Requires/Versions.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
["1.0.0"]
git-tree-sha1 = "999513b7dea8ac17359ed50ae8ea089e4464e35e"

["2.0.0"]
git-tree-sha1 = "999513b7dea8ac17359ed50ae8ea089e4464e35e"
8 changes: 8 additions & 0 deletions test/templates/public_3/Registry.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
name = "PublicRegistry"
uuid = "1003c4b9-3c72-409d-ba60-5578a18ea1a7"
repo = ""

description = "This is a test registry for the AutoMerge integration tests."

[packages]
ae029012-a4dd-5104-9daa-d747884805df = { name = "Requires", path = "R/Requires" }
2 changes: 2 additions & 0 deletions test/templates/public_4/R/Requires/Compat.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[0-1]
julia = "1"
Empty file.
3 changes: 3 additions & 0 deletions test/templates/public_4/R/Requires/Package.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
name = "Requires"
uuid = "ae029012-a4dd-5104-9daa-d747884805df"
repo = "https://github.com/MikeInnes/Requires.jl.git"
2 changes: 2 additions & 0 deletions test/templates/public_4/R/Requires/Versions.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
["0.5.2"]
git-tree-sha1 = "f6fbf4ba64d295e146e49e021207993b6b48c7d1"
8 changes: 8 additions & 0 deletions test/templates/public_4/Registry.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
name = "PublicRegistry"
uuid = "1003c4b9-3c72-409d-ba60-5578a18ea1a7"
repo = ""

description = "This is a test registry for the AutoMerge integration tests."

[packages]
ae029012-a4dd-5104-9daa-d747884805df = { name = "Requires", path = "R/Requires" }

0 comments on commit 51116de

Please sign in to comment.