Skip to content

Commit

Permalink
Allow Bors to sign commits
Browse files Browse the repository at this point in the history
This adds a new `committer.signing_key` entry to `bors.toml`. When
specified, Bors will sign commits with `gpg`. The referenced secret
key must be installed in the local gpg key store, making this patch
unsuitable for public deployment.

Future development could include adding a page to Bors's web UI to
upload a private key for signing operations.
  • Loading branch information
cassiemeharry committed Sep 28, 2021
1 parent fcedb99 commit 75c6160
Show file tree
Hide file tree
Showing 7 changed files with 249 additions and 21 deletions.
2 changes: 2 additions & 0 deletions config/test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,6 @@ config :bors, :server, BorsNG.GitHub.ServerMock
config :bors, :oauth2, BorsNG.GitHub.OAuth2Mock
config :bors, :is_test, true

config :bors, :test_gpg_key_id, {:system, "BORS_TEST_GPG_KEY_ID", ""}

config :bors, :celebrate_new_year, false
13 changes: 7 additions & 6 deletions lib/github/github.ex
Original file line number Diff line number Diff line change
Expand Up @@ -198,12 +198,12 @@ defmodule BorsNG.GitHub do
parents: [bitstring],
commit_message: bitstring,
committer: tcommitter | nil
}) :: binary
def synthesize_commit!(repo_conn, info) do
}, bitstring | nil) :: binary
def synthesize_commit!(repo_conn, info, signing_key \\ nil) do
{:ok, sha} =
GenServer.call(
BorsNG.GitHub,
{:synthesize_commit, repo_conn, {info}},
{:synthesize_commit, repo_conn, {info, signing_key}},
Confex.fetch_env!(:bors, :api_github_timeout)
)

Expand All @@ -214,13 +214,14 @@ defmodule BorsNG.GitHub do
tree: bitstring,
parents: [bitstring],
commit_message: bitstring,
author: tcommitter | nil,
committer: tcommitter | nil
}) :: binary
def create_commit!(repo_conn, info) do
}, bitstring | nil) :: binary
def create_commit!(repo_conn, info, signing_key \\ nil) do
{:ok, sha} =
GenServer.call(
BorsNG.GitHub,
{:create_commit, repo_conn, {info}},
{:create_commit, repo_conn, {info, signing_key}},
Confex.fetch_env!(:bors, :api_github_timeout)
)

Expand Down
49 changes: 45 additions & 4 deletions lib/github/github/server.ex
Original file line number Diff line number Diff line change
Expand Up @@ -282,21 +282,46 @@ defmodule BorsNG.GitHub.Server do
tree: tree,
parents: parents,
commit_message: commit_message,
author: author,
committer: committer
}}
}, signing_key}
) do
msg = %{parents: parents, tree: tree, message: commit_message}

msg =
if is_nil(committer) do
msg
else
Map.put(msg, "author", %{
Map.put(msg, :committer, %{
name: committer.name,
email: committer.email
})
end

msg =
if is_nil(author) do
msg
else
Map.put(msg, :author, author)
end

msg =
cond do
is_nil(signing_key) ->
Logger.debug("Not signing commit in create_commit because signing_key is nil")
msg
is_nil(author) ->
Logger.debug("Not signing commit in create_commit because author is nil")
msg
is_nil(committer) ->
Logger.debug("Not signing commit in create_commit because committer is nil")
msg
true ->
msg
|> GitHub.Signature.add_timestamp(DateTime.utc_now())
|> GitHub.Signature.sign!(signing_key)
end

resp =
repo_conn
|> post!("git/commits", Poison.encode!(msg))
Expand Down Expand Up @@ -333,18 +358,34 @@ defmodule BorsNG.GitHub.Server do
parents: parents,
commit_message: commit_message,
committer: committer
}}
}, signing_key}
) do
msg = %{parents: parents, tree: tree, message: commit_message}

msg =
if is_nil(committer) do
msg
else
Map.put(msg, "author", %{
msg
|> Map.put(:author, %{
name: committer.name,
email: committer.email
})
|> Map.put(:committer, committer)
end

msg =
cond do
is_nil(signing_key) ->
Logger.debug("Not signing commit in synthesize_commit because signing_key is nil")
msg
is_nil(committer) ->
Logger.debug("Not signing commit in synthesize_commit because committer is nil")
msg
true ->
msg
|> GitHub.Signature.add_timestamp(DateTime.utc_now())
|> GitHub.Signature.sign!(signing_key)
end

repo_conn
Expand Down
98 changes: 98 additions & 0 deletions lib/github/github/signature.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
require Logger

defmodule BorsNG.GitHub.Signature do
@moduledoc """
Provides the ability to sign commits using gpg2.
"""

@type tcommit_no_ts :: %{
parents: [binary],
tree: binary,
message: binary,
author: %{name: binary, email: binary},
committer: %{name: binary, email: binary}
}

@type tcommit :: %{
parents: [binary],
tree: binary,
message: binary,
author: %{name: binary, email: binary, date: binary},
committer: %{name: binary, email: binary, date: binary}
}

@type tcommit_sig :: %{
parents: [binary],
tree: binary,
message: binary,
author: %{name: binary, email: binary, date: binary},
committer: %{name: binary, email: binary, date: binary},
signature: binary
}

@spec add_timestamp(tcommit_no_ts, DateTime.t) :: tcommit
def add_timestamp(commit, dt) do
ts = DateTime.to_iso8601(dt)
commit
|> Map.put(:author, Map.put(commit[:author], :date, ts))
|> Map.put(:committer, Map.put(commit[:committer], :date, ts))
end

@spec format_commit(tcommit) :: binary
def format_commit(commit) do
{:ok, author_date, _} = DateTime.from_iso8601(commit[:author][:date])
author_ts = DateTime.to_unix(author_date, :second) |> Integer.to_string()
{:ok, committer_date, _} = DateTime.from_iso8601(commit[:committer][:date])
committer_ts = DateTime.to_unix(committer_date, :second) |> Integer.to_string()
gpg_sig =
case Map.fetch(commit, :signature) do
{:ok, sig} ->
lines = String.split(sig, "\n")
|> Enum.intersperse("\n ")
|> Enum.to_list()
["gpgsig ", lines, "\n"]
:error ->
[]
end

IO.iodata_to_binary([
["tree ", commit[:tree], "\n"],
Enum.map(commit[:parents], fn parent -> ["parent ", parent, "\n"] end),
["author ", commit[:author][:name], " <", commit[:author][:email], "> ", author_ts, " +0000\n"],
["committer ", commit[:committer][:name], " <", commit[:committer][:email], "> ", committer_ts, " +0000\n"],
gpg_sig,
"\n",
commit[:message],
])
end

@spec sign!(tcommit, binary) :: tcommit_sig
def sign!(commit, key_id) do
Logger.info("Signing commit #{inspect(commit)} with key #{inspect(key_id)}")

path = System.find_executable("gpg")
if is_nil(path) do
throw :missing_gpg
end

commit_to_sign = format_commit(commit)
Logger.debug("Commit to sign: #{inspect commit_to_sign}")

tmp_dir = System.tmp_dir!()
tmp_filename = Path.join(tmp_dir, "bors_commit_signing.#{commit[:tree]}.txt")
sig_filename = Path.join(tmp_dir, "bors_commit_signing.#{commit[:tree]}.txt.asc")
try do
_ = File.rm(sig_filename)
File.write!(tmp_filename, commit_to_sign, [:write, :binary, :sync])
args = ["--batch", "--with-colons", "--status-fd", "2", "--armor", "--local-user", key_id, "--detach-sign", tmp_filename]
Logger.debug("Calling #{path} #{Enum.join(args, " ")}")
{output, 0} = System.cmd(path, args)
Logger.debug("Output from gpg: #{inspect output}")
sig = File.read!(sig_filename)
Map.put(commit, :signature, sig)
after
_ = File.rm(tmp_filename)
_ = File.rm(sig_filename)
end
end
end
27 changes: 23 additions & 4 deletions lib/worker/batcher.ex
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,17 @@ defmodule BorsNG.Worker.Batcher do
batch.into_branch
)

toml = repo_conn
|> Batcher.GetBorsToml.get(base.commit)
|> case do
{:ok, toml} -> toml
{:error, message} ->
message = Batcher.Message.generate_bors_toml_error(message)
patches = Enum.map(patch_links, & &1.patch)
send_message(repo_conn, patches, {:config, message})
nil
end

tbase = %{
tree: base.tree,
commit:
Expand All @@ -406,7 +417,12 @@ defmodule BorsNG.Worker.Batcher do
parents: [base.commit],
commit_message: "[ci skip][skip ci][skip netlify]",
committer: nil
}
},
if is_nil(toml) do
nil
else
toml.signing_key
end
)
}

Expand Down Expand Up @@ -535,8 +551,10 @@ defmodule BorsNG.Worker.Batcher do
tree: merge_commit.tree,
parents: [prev_head],
commit_message: commit_message,
committer: %{name: user.name || user.login, email: user_email}
}
author: %{name: user.name || user.login, email: user_email},
committer: toml.committer
},
toml.signing_key
)

Logger.info("Commit Sha #{inspect(cpt)}")
Expand Down Expand Up @@ -574,7 +592,8 @@ defmodule BorsNG.Worker.Batcher do
parents: parents,
commit_message: commit_message,
committer: toml.committer
}
},
toml.signing_key
)
end

Expand Down
20 changes: 13 additions & 7 deletions lib/worker/batcher/bors_toml.ex
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ defmodule BorsNG.Worker.Batcher.BorsToml do
use_codeowners: false,
committer: nil,
commit_title: "Merge ${PR_REFS}",
update_base_for_deletes: false
update_base_for_deletes: false,
signing_key: nil

@type tcommitter :: %{
name: binary,
Expand All @@ -51,7 +52,8 @@ defmodule BorsNG.Worker.Batcher.BorsToml do
use_codeowners: boolean,
committer: tcommitter,
commit_title: binary,
update_base_for_deletes: boolean
update_base_for_deletes: boolean,
signing_key: binary | nil
}

@type err ::
Expand Down Expand Up @@ -81,18 +83,18 @@ defmodule BorsNG.Worker.Batcher.BorsToml do

committer = Map.get(toml, "committer", nil)

committer =
{committer, signing_key} =
case committer do
nil ->
nil
{nil, nil}

_ ->
c = to_map(committer)

%{
{%{
name: Map.get(c, "name", nil),
email: Map.get(c, "email", nil)
}
}, Map.get(c, "signing_key", nil)}
end

toml = %BorsNG.Worker.Batcher.BorsToml{
Expand Down Expand Up @@ -124,7 +126,8 @@ defmodule BorsNG.Worker.Batcher.BorsToml do
),
committer: committer,
commit_title: Map.get(toml, "commit_title", "Merge ${PR_REFS}"),
update_base_for_deletes: Map.get(toml, "update_base_for_deletes", false)
update_base_for_deletes: Map.get(toml, "update_base_for_deletes", false),
signing_key: signing_key,
}

case toml do
Expand Down Expand Up @@ -159,6 +162,9 @@ defmodule BorsNG.Worker.Batcher.BorsToml do
%{commit_title: msg} when not is_binary(msg) and not is_nil(msg) ->
{:error, :commit_title}

%{signing_key: k} when not is_binary(k) and not is_nil(k) ->
{:error, :signing_key}

toml ->
status =
toml.status
Expand Down
Loading

0 comments on commit 75c6160

Please sign in to comment.