feat(repo): audit GitHub settings, the one layer no in-repo gate can see - #54
Merged
Conversation
Branch protection, required checks and Dependabot state live only in GitHub.
Nothing inside a repository can observe them, so a repo can carry a full CI
matrix, a protection rule that requires none of it, and no gate anywhere will
say so.
Run against the org today it reports 20 findings, and the shape of them is the
argument for the command:
* three repos carry a protection rule requiring ZERO status checks —
landing-zone among them, whose merges become live AWS infrastructure. This
is the most misleading of the states: the repository reads as protected in
every UI while admitting a PR whose entire CI matrix is red.
* ten have no protection at all.
* three private repos cannot be protected on the current plan, two of which
hold live cluster and tenant GitOps state.
─── three absences, not one ───
No rule, a rule requiring nothing, and protection unavailable on the plan are
three different facts with three different remedies, and the reader keeps them
distinct end to end. Collapsing them is how "upgrade your plan" gets filed as
"add a required check", and how an unprotectable repository reads as compliant
because nothing was reported about it.
─── shape of the implementation ───
internal/repo owns a narrow Reader interface, following the IdentityReader
precedent, so the whole audit runs against fixtures with no token and no
network. The gh-backed reader takes its command runner as a field for the same
reason — its argument construction and its three-way protection parse are
asserted rather than assumed.
gh rather than a GitHub SDK because it already holds the operator's credential.
cloudgov reads AWS through the ambient chain and asks for nothing; this keeps
that property instead of introducing the one token the tool would have to be
handed.
The expected shape is committed. Every required check named in it is a job with
NO paths: filter — load-bearing, because a path-filtered job does not run on a
PR that touches nothing it watches, so requiring it wedges that PR forever, and
with enforce_admins on there is no override left to dig out with. nanohype has
11 of 12 jobs path-filtered, which is why it requires none of them.
An unreadable repository is reported, never skipped: a sweep that skips what it
cannot read reports a clean org over repositories it never looked at, which is
the same defect this tool is pointed at everywhere else.
─── a test of mine that was wrong ───
The gh fixture matched invocations by substring over a Go map, and every
protection URL contains the repo URL — so iteration order decided which stub
answered, and the test passed about half the time. The reader was right; the
fixture was not. Keys are now unambiguous and the suite runs clean at -count=5.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
gosec's pinned CI image flags G204 on the gh invocation — subprocess launched with variable arguments. It is right to look: the argv is built from a caller's --org flag and names returned by the API. There is no command injection to have, because exec.Command passes argv directly with no shell. But there is a real hazard the rule is pointing at from the wrong angle: a name beginning with a dash lands in an argument position and gh reads it as a FLAG rather than a path segment, which turns a read into something else. Every value this package interpolates — org, repo, and the default branch the API returns — is now checked against GitHub's own name character set before it reaches an argument. The suppression that remains states what is true rather than asking to be trusted: the command is the constant "gh", and the arguments are literals plus already-validated tokens. Tested from both sides, because a validator that refuses everything passes a rejection test: the flag-shaped, path-shaped, empty and over-long names are all refused and never reach gh at all, and the ordinary names — including nanohype.dev and a_b — are accepted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Branch protection, required checks and Dependabot state live only in GitHub. Nothing inside a repository can observe them — so a repo can carry a full CI matrix, a protection rule that requires none of it, and no gate anywhere will say so.
What it reports on the org today
cloudgov repo audit --org nanohype→ 20 findings, and their shape is the argument for the command:landing-zone,cloudgov,homebrew-tapclusters,tenants,nanohype.devlanding-zone's merges become live AWS infrastructure, and it has a rule that requires nothing — the most misleading of the three states, since the repository reads as protected in every UI while admitting a PR whose entire CI matrix is red.clustersandtenantshold live GitOps state and cannot be protected at all.Three absences, not one
No rule · a rule requiring nothing · unavailable on the plan — three different facts, three different remedies, kept distinct end to end. Collapsing them is how "upgrade your plan" gets filed as "add a required check", and how an unprotectable repository reads as compliant because nothing was said about it.
Shape
internal/repoowns a narrowReaderinterface, following theIdentityReaderprecedent, so the whole audit runs against fixtures with no token and no network. The gh-backed reader takes its command runner as a field for the same reason — argument construction and the three-way protection parse are asserted, not assumed.gh rather than a GitHub SDK because it already holds the operator's credential. cloudgov reads AWS through the ambient chain and asks for nothing; this keeps that property instead of introducing the one token the tool would have to be handed.
The expected shape is committed. Every required check named in it is a job with no
paths:filter — load-bearing, because a path-filtered job does not run on a PR that touches nothing it watches, so requiring it wedges that PR forever, and withenforceAdminson there is no override left to dig out with.nanohypehas 11 of 12 jobs path-filtered, which is why it requires none of them.An unreadable repository is reported, never skipped. A sweep that skips what it cannot read reports a clean org over repositories it never looked at.
A test of mine that was wrong
The gh fixture matched invocations by substring over a Go map, and every protection URL contains the repo URL — so iteration order decided which stub answered, and the test passed about half the time. The reader was right; the fixture was not. Keys are now unambiguous and the suite runs clean at
-count=5.Verification
task build,task lint(0 issues),scripts/check-context.sh, andscripts/coverage.shall pass —internal/repoat 82.4% against its new 80% floor,internal/outputrestored to 72.3% above its 70%. 27 packages green.Reported, never enforced — consistent with the rest of the tool.