Skip to content

Probe a private repository, or the control verifies the wrong thing - #24

Merged
imanimanyara merged 1 commit into
mainfrom
fix/control-must-probe-a-private-repo
Nov 8, 2026
Merged

imanimanyara merged 1 commit into
mainfrom
fix/control-must-probe-a-private-repo

Conversation

@imanimanyara

Copy link
Copy Markdown
Member

Summary

The positive control added in #23 can pass while the gate is blind. checks: read is enforced on
private repositories only, so a probe that lands on a public repo reports "readable" even when
every private repository answers 403 — and the estate's Dependabot pull requests sit
overwhelmingly in private repos.

This sorts candidates private-first over a wide enough page, names the visibility that was
verified, and — where only a public repository was available — says that the read does not exercise
checks: read.

#23 misread the evidence, and this is the correction

I claimed "every pull request the gate examined errored" in run 41713055813. That was wrong. The
simtabi job examined one pull request and it passed:

merge (simtabi)  1 open Dependabot pull requests.
| simtabi/cli-kit-js | 24 | hold | update type not recognised (group tooling is not the policy group) |

A real policy decision, so check_runs succeeded — with the same token that answered 403 for four
other orgs. The discriminator is visibility, not the org:

repo visibility result
simtabi/cli-kit-js public read succeeded
simbi-prjs-clt/{staging.freeby.io,staging.legalmizer.com,swamari.com,theomaris.com} private 403
simbi-prjs-int/creamer.simtabi.com private 403
tusente/web-app private 403
tayariai/{tayariai,gpt,v2} private 403

So the run was five vacuous greens (0 pull requests) plus one genuine pass on a public repo,
not six vacuous ones.

Why that breaks the control as merged

Measured against main, the repository checks_readable() picks per org:

org picks visibility
simtabi simtabi/go-notifykit public
ichava ichava/icon-sets-sync public
laranail, tusente, tayariai, simbi-prjs-clt, simbi-prjs-int, brandgulp various private

For simtabi and ichava it would have reported "readable" under exactly the 2026-11-03
conditions. The other six pass only by luck of which repository was pushed most recently, which
changes constantly — so this is a latent false pass, not a fixed one.

Verification

  • Suite 27 → 30, OK.

  • Guard proven to have teeth: removing the private-first sort makes
    test_control_prefers_a_private_repository fail (FAILED (failures=1)); restoring it returns 30
    OK. Its fixture is the real shape — public repo readable, private repo 403 — and asserts the
    control returns False rather than passing on the public read.

  • A public-only account still verifies (test_a_public_only_account_still_verifies): with no
    private repository there is no private blindness to detect.

  • Live, read-only, against the two that previously picked public:

    simtabi   ok=True  verified on simtabi/somesha-ios@main   (private)
    ichava    ok=True  verified on ichava/ichava.com@main     (private)
    laranail  ok=True  verified on laranail/tayari-ui@main    (private)
    brandgulp ok=True  verified on brandgulp/brandgulp.com@main (private)
    tusente   ok=True  verified on tusente/web-app@main       (private)
    

checks: read is enforced on private repositories only, so the positive
control added in #23 can report "readable" against a public repo while the
gate is blind to every repository it actually works on. The estate's
Dependabot pull requests sit overwhelmingly in private repos, so that is
not a corner case.

The 2026-11-03 run is the proof, and it was misread when #23 was written.
The simtabi job examined one pull request and it passed -- on the public
simtabi/cli-kit-js, with the same token that answered 403 for every
private repository across four other orgs. So "every pull request the gate
examined errored" was wrong: the one in a public repo succeeded, and the
discriminator was visibility, not the org.

Measured against the control as merged: for simtabi and ichava it picks a
public repo, so for those two it would have passed while private repos
were unreadable. The other six pass only by luck of which repository was
pushed most recently, which changes constantly.

Sort private-first over a wide enough page, name the visibility that was
verified, and where only a public repository was available say that the
read does not exercise checks: read. An account with no private repository
has no private blindness to detect, so a public probe is complete for it.
Copilot AI balanced review requested due to automatic review settings November 8, 2026 13:34

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

@imanimanyara
imanimanyara merged commit 8d00f0f into main Nov 8, 2026
6 checks passed
imanimanyara added a commit that referenced this pull request Nov 8, 2026
#24's control enumerated the errors that are NOT permission failures --
409 and the word "empty" -- and the enumeration was incomplete. An empty
repository whose default_branch is nominally main answers 422 "No commit
found for SHA" on commits/{ref}/check-runs while /commits answers 409, so
the shape never matched.

Measured live after #24 merged: simsange reported
  ok=False cannot read check runs on simsange/documentation@main:
          gh: No commit found for SHA: main (HTTP 422)
because its newest private repository is the empty simsange/documentation.
That is a false permission failure, and the gate would have gone red for
simsange every quiet week.

Inverted: fail only where GitHub says the permission is missing -- 403, or
"Resource not accessible by integration" -- and treat anything else as
inconclusive, keep looking, and return None if nothing conclusive is
found. One shape to recognise instead of an open-ended list of exemptions.

Two existing tests were asserting against the wrong thing and are fixed
here: they raised FakeGh's generic "no route for ..." error, which is not a
403, so they passed for a reason unrelated to what they claimed to check.
They now raise the real message.
@imanimanyara
imanimanyara deleted the fix/control-must-probe-a-private-repo branch November 14, 2026 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants