feat: add another fast path for impostor commit detection (#1429)
Some checks are pending
CI / Lint (push) Waiting to run
CI / Test (push) Waiting to run
CI / Test site build (push) Waiting to run
CI / All tests pass (push) Blocked by required conditions
zizmor wheel builds for PyPI 🐍 / Build Linux wheels (manylinux) (push) Waiting to run
zizmor wheel builds for PyPI 🐍 / Build Linux wheels (musllinux) (push) Waiting to run
zizmor wheel builds for PyPI 🐍 / Build Windows wheels (push) Waiting to run
zizmor wheel builds for PyPI 🐍 / Build macOS wheels (push) Waiting to run
zizmor wheel builds for PyPI 🐍 / Build source distribution (push) Waiting to run
zizmor wheel builds for PyPI 🐍 / Release (push) Blocked by required conditions
Deploy zizmor documentation site 🌐 / Deploy zizmor documentation to GitHub Pages 🌐 (push) Waiting to run
GitHub Actions Security Analysis with zizmor 🌈 / Run zizmor 🌈 (push) Waiting to run

This commit is contained in:
William Woodruff 2025-12-10 01:20:05 -08:00 committed by GitHub
parent 1f71a18100
commit b78376a737
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 70 additions and 7 deletions

View file

@ -297,6 +297,12 @@ self_cell!(
impl {Debug, PartialEq}
);
impl Display for RepositoryUses {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.raw())
}
}
impl RepositoryUses {
/// Parse a `uses: some/repo` clause.
pub fn parse(uses: impl Into<String>) -> Result<Self, UsesError> {

View file

@ -25,8 +25,6 @@ gh-token-tests = []
online-tests = ["gh-token-tests"]
# Test-only: enable tests that require `unbuffer` for TTY behavior.
tty-tests = []
# Test-only: enable tests that are typically very slow.
slow-tests = []
[dependencies]
annotate-snippets.workspace = true

View file

@ -77,7 +77,7 @@ impl ImpostorCommit {
return Ok(false);
};
// Fast path: almost all commit refs will be at the tip of
// Fastest path: almost all commit refs will be at the tip of
// the branch or tag's history, so check those first.
// Check tags before branches, since in practice version tags
// are more commonly pinned.
@ -105,6 +105,21 @@ impl ImpostorCommit {
}
}
// Fast path: attempt to use GitHub's undocumented `branch_commits`
// API to see if the commit is present in any branch/tag.
// There are no stabilitiy guarantees for this API, so we fall back
// to the slow(er) paths if it fails.
match self
.client
.branch_commits(uses.owner(), uses.repo(), head_ref)
.await
{
Ok(branch_commits) => return Ok(branch_commits.is_empty()),
Err(e) => tracing::warn!("fast path impostor check failed for {uses}: {e}"),
}
// Slow path: use GitHub's comparison API to check each branch and tag's
// history for presence of the commit.
for branch in &branches {
if self
.named_ref_contains_commit(uses, &format!("refs/heads/{}", &branch.name), head_ref)

View file

@ -575,6 +575,29 @@ impl Client {
.max_by_key(|t| t.name.len()))
}
#[instrument(skip(self))]
pub(crate) async fn branch_commits(
&self,
owner: &str,
repo: &str,
commit: &str,
) -> Result<BranchCommits, ClientError> {
// NOTE(ww): This API is undocumented.
// See: https://github.com/orgs/community/discussions/78161
let url = format!("https://github.com/{owner}/{repo}/branch_commits/{commit}");
// We ask GitHub for JSON, because it sends HTML by default for this endpoint.
self.base_client
.get(&url)
.header(ACCEPT, "application/json")
.send()
.await?
.error_for_status()?
.json()
.await
.map_err(Into::into)
}
#[instrument(skip(self))]
pub(crate) async fn compare_commits(
&self,
@ -859,6 +882,23 @@ pub(crate) struct Commit {
pub(crate) sha: String,
}
/// The response structure from GitHub's undocumented `branch_commits` API.
///
/// This model is intentionally incomplete.
#[derive(Clone, Deserialize)]
#[serde(rename_all = "lowercase")]
#[non_exhaustive]
pub(crate) struct BranchCommits {
branches: Vec<serde_json::Value>,
tags: Vec<String>,
}
impl BranchCommits {
pub(crate) fn is_empty(&self) -> bool {
self.branches.is_empty() && self.tags.is_empty()
}
}
#[derive(Clone, Deserialize)]
#[serde(rename_all = "lowercase")]
pub(crate) enum ComparisonStatus {

View file

@ -1,9 +1,6 @@
use crate::common::{input_under_test, zizmor};
#[cfg_attr(
any(not(feature = "gh-token-tests"), not(feature = "slow-tests")),
ignore
)]
#[cfg_attr(not(feature = "gh-token-tests"), ignore)]
#[test]
fn test_regular_persona() -> anyhow::Result<()> {
insta::assert_snapshot!(

View file

@ -30,6 +30,13 @@ of `zizmor`.
* zizmor now produces more useful and less ambiguous spans for many findings,
particularly those from the [anonymous-definition] audit (#1416)
### Performance Improvements 🚄
* The [impostor-commit] audit is now significantly faster on true positives,
making true positive detection virtually as fast as true negative detection.
In practice, true positive runs are over 100 times faster than before
(#1429)
### Bug Fixes 🐛