Skip to content

Commit 6a4c905

Browse files
authored
fix: improve error message when ref listing fails (#1293)
1 parent be294d9 commit 6a4c905

File tree

4 files changed

+71
-3
lines changed

4 files changed

+71
-3
lines changed

crates/zizmor/src/github.rs

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,9 @@ pub(crate) enum ClientError {
148148
/// between listing and fetching it.
149149
#[error("couldn't fetch file {file} from {slug}: is the branch/tag being modified?")]
150150
FileTOCTOU { file: String, slug: String },
151+
/// An accessed repository is missing or private.
152+
#[error("can't access {owner}/{repo}: missing or you have no access")]
153+
RepoMissingOrPrivate { owner: String, repo: String },
151154
/// Any of the errors above, wrapped from concurrent contexts.
152155
#[error(transparent)]
153156
Inner(#[from] Arc<ClientError>),
@@ -372,8 +375,20 @@ impl Client {
372375
.body(req)
373376
.basic_auth("x-access-token", Some(&self.token.0))
374377
.send()
375-
.await?
376-
.error_for_status()?;
378+
.await?;
379+
380+
let resp = match resp.status() {
381+
StatusCode::OK => Ok(resp),
382+
// NOTE: Versions of zizmor prior to 1.16.0 would silently
383+
// skip private or missing repositories, as branch/tag lookups
384+
// were done as a binary present/absent check. This caused
385+
// false negatives.
386+
StatusCode::NOT_FOUND => Err(ClientError::RepoMissingOrPrivate {
387+
owner: owner.to_string(),
388+
repo: repo.to_string(),
389+
}),
390+
_ => Err(resp.error_for_status().unwrap_err().into()),
391+
}?;
377392

378393
let mut remote_refs = vec![];
379394
let content = resp.bytes().await?;

crates/zizmor/tests/integration/e2e.rs

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ fn menagerie() -> Result<()> {
7575
.output(OutputMode::Both)
7676
.args(["--collect=all"])
7777
.input(input_under_test("e2e-menagerie"))
78-
.run()?
78+
.run()?,
7979
);
8080

8181
Ok(())
@@ -367,3 +367,31 @@ fn issue_1207() -> Result<()> {
367367

368368
Ok(())
369369
}
370+
371+
/// Regression test for #1286.
372+
///
373+
/// Ensures that we produce a useful error when a user's input references
374+
/// a private (or missing) repository.
375+
#[cfg_attr(not(feature = "gh-token-tests"), ignore)]
376+
#[test]
377+
fn issue_1286() -> Result<()> {
378+
insta::assert_snapshot!(
379+
zizmor()
380+
.expects_failure(true)
381+
.output(OutputMode::Both)
382+
.offline(false)
383+
.input(input_under_test("issue-1286.yml"))
384+
.run()?,
385+
@r"
386+
🌈 zizmor v@@VERSION@@
387+
fatal: no audit was performed
388+
ref-confusion failed on file://@@INPUT@@
389+
390+
Caused by:
391+
0: couldn't list branches for woodruffw-experiments/this-does-not-exist
392+
1: can't access woodruffw-experiments/this-does-not-exist: missing or you have no access
393+
",
394+
);
395+
396+
Ok(())
397+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
# repro for #1286
2+
3+
name: issue-1286-repro
4+
5+
on: [push, pull_request]
6+
7+
concurrency:
8+
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
9+
cancel-in-progress: true
10+
11+
permissions: {}
12+
13+
jobs:
14+
issue-1286-repro:
15+
name: issue-1286-repro
16+
runs-on: ubuntu-latest
17+
steps:
18+
- name: private
19+
uses: woodruffw-experiments/[email protected]

docs/release-notes.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@ of `zizmor`.
99

1010
## Next (UNRELEASED)
1111

12+
### Enhancements 🌱
13+
14+
* `zizmor` now produces a more useful error message when asked to indirectly
15+
access a nonexistent or private repository via a `uses:` clause (without
16+
a sufficiently privileged GitHub token) (#1293)
17+
1218
## 1.16.0
1319

1420
### New Features 🌈

0 commit comments

Comments
 (0)