diff --git a/.github/workflows/contexts.yml b/.github/workflows/contexts.yml new file mode 100644 index 0000000..6b0ccca --- /dev/null +++ b/.github/workflows/contexts.yml @@ -0,0 +1,108 @@ +# The gate refers to check names, and this is what holds the two together. +# +# A required context is a string. A check name is a string in a workflow file. +# Nothing on the platform ties them, so a job renamed in passing removes itself +# from the required set while the tab still looks green, and a context required +# under a name nothing reports blocks every merge without saying why on the pull +# request. Both are failures of the same disagreement and this job refuses both. +# +# WHY THIS IS A WORKFLOW AND NOT A VERB OF THE RUNNER. Half the comparison is a +# live setting on the hosting platform rather than a file in the tree, so it can +# only be answered by asking the API. lab reads a checkout and opens no network +# connection, and that claim is repeated in every document here, so the fetch +# lives in the step below and the judgement lives in a command that is handed the +# answer on standard input. The token is granted on this one job and nowhere +# else, read-only, because reading a ruleset is all it is for. +# +# The check name below is what the required set refers to later, so it is written +# here rather than left to the job id, and changing it is a change to the gate +# rather than tidying. This job is the one place where that has a consequence it +# can see: its own name is in the deliberate-absence list in +# internal/contexts/contexts.go, so renaming it without moving that entry reddens +# this check and the ordinary suite together. +# +# Hardened the way every other workflow in this tree is, because the audit job in +# zizmor.yml refuses a new one that departs from it: permissions denied at the +# top and granted per job, every action pinned to a commit with its version in a +# comment, checkout without persisted credentials, and nothing expanded into a +# run block. +name: Required contexts + +on: + pull_request: + branches: ["**"] + push: + branches: [main] + +# Explicit deny-all at workflow level; the job below grants only what it needs. +permissions: {} + +# Cancel a superseded run on the same ref. Nothing here publishes, so a run that +# has been overtaken is safe to drop. +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + contexts: + name: required contexts + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + # Read the tree, and read the ruleset. Both are reads and there is no + # third thing this job does. + contents: read + steps: + - name: Checkout Repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Set up Go + uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + # The toolchain comes from go.mod, so the version this job uses moves + # when the module says so and never separately. + go-version-file: go.mod + # This module has no dependencies and therefore no go.sum for a cache + # key to be built from. Asking for a cache anyway makes the step + # depend on a file that is not there. + cache: false + + - name: Read the contexts the ruleset requires + # The fetch is its own step so that a ruleset the run could not read is + # a different outcome from a ruleset that disagrees with the tree. A + # failed fetch here stops the job, and a job that had piped a failed + # fetch into the comparison would have compared the tree against an + # empty list and reported whatever that produced. + # + # An empty answer is not a failure. The ruleset on this board requires + # no status check today, and the comparison still has work to do in + # that state, which is why the count is printed rather than the run + # stopping. + # + # Every value reaches the script through the environment. Nothing an + # author controls is part of a command, and no expression is expanded + # inside a run block. + env: + GH_TOKEN: ${{ github.token }} + REPOSITORY: ${{ github.repository }} + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} + run: | + set -euo pipefail + if ! gh api "repos/${REPOSITORY}/rules/branches/${DEFAULT_BRANCH}" \ + --jq '.[] | select(.type=="required_status_checks") + | .parameters.required_status_checks[].context' > required.txt; then + echo "::error::The ruleset on ${DEFAULT_BRANCH} could not be read, so this run could not judge whether the gate and the tree agree. That is not the same as them agreeing." + exit 1 + fi + echo "the ruleset on ${DEFAULT_BRANCH} requires $(wc -l < required.txt) context(s):" + cat required.txt + + - name: Compare them against the check names this tree declares + # Built from this commit rather than downloaded, so a change that breaks + # a refusal is caught by the pull request that made it. The exit codes + # are the contract in docs/decisions/0011-the-exit-codes.md: 0 refused + # nothing, 1 refused something, 2 could not do the job. This step keys on + # all three by letting the command's own code become the job's. + run: go run ./cmd/contexts < required.txt diff --git a/cmd/contexts/main.go b/cmd/contexts/main.go new file mode 100644 index 0000000..fd8725c --- /dev/null +++ b/cmd/contexts/main.go @@ -0,0 +1,135 @@ +// Command contexts compares the contexts the ruleset requires on the default +// branch against the check names the workflows in this repository declare, and +// refuses when the two disagree. +// +// WHY THIS IS NOT A VERB OF THE RUNNER. lab reads a checkout and opens no +// network connection, which is a claim a downloader is asked to take on trust +// and which cmd/lab's own suite holds it to. Half of this comparison is a live +// setting on the hosting platform, so it can only be answered by asking the API, +// and putting that inside lab would widen what an operator has to believe about +// a binary they downloaded in exchange for a verb that is useless outside a +// workflow. Keeping it here costs a third entry point under cmd/, which record +// 0002 does not name, and that is the judgement in this file a reader should +// check rather than accept. It is the same one cmd/pullrequest already made for +// the same reason. +// +// THE API CALL IS NOT IN HERE EITHER. The required set arrives on standard +// input, one context per line, so this command reads a checkout and a list and +// nothing else. The workflow that runs it is where the token is, which keeps the +// credential in one visible place and lets every rule below be proved against a +// list a test wrote out in full. +package main + +import ( + "bufio" + "fmt" + "io" + "os" + "strings" + + "github.com/Flowfin/lab/internal/contexts" +) + +// The exit codes. Decision record 0011 is the contract, and this command returns +// the same three the runner does, for the same reasons, which is why they are +// written here with the record named rather than invented. The exit-code leg in +// internal/invariants reads every exit-code constant in the tree and refuses a +// code declared with two numbers, or a number declared under two codes, so keep +// the convention these three follow. +const ( + // exitClean means the run completed and refused nothing. It does not mean + // the gate is intact, only that the two lists agree, and the report says + // what it compared so the two are not confused. + exitClean = 0 + + // exitRefused means the run completed and refused something. It is the + // only code that carries refusals. + exitRefused = 1 + + // exitCannot means the check could not do its job: no required set on + // standard input, no workflow directory to read, a file in a shape the + // reader was not built for. It is deliberately not the code a refusal + // returns, because a gate that treats a broken invocation like a violation + // reports one as the other and nobody investigates either. + exitCannot = 2 +) + +func main() { + os.Exit(run(os.Stdin, os.Stdout, os.Stderr, edges{ + workflowsDir: contexts.WorkflowsDir, + absences: contexts.Absences, + alsoReported: contexts.ReportedOutsideAWorkflowFile, + })) +} + +// edges is everything this command reaches for that is not its own logic: where +// the workflows are read from, which deliberate-absence list the comparison is +// made against, and which check names arrive from something other than a job. +// All three are parameters so that what the command prints and what it returns +// can be read by a test against a tree written out in full, rather than against +// whichever repository the suite happens to be running inside. +type edges struct { + workflowsDir string + absences []contexts.Absence + alsoReported []contexts.Declared +} + +// run is main with its edges passed in. +func run(in io.Reader, out, errOut io.Writer, e edges) int { + required, err := readRequired(in) + if err != nil { + fmt.Fprintf(errOut, "contexts: %v\n", err) + return exitCannot + } + + declared, err := contexts.ReadWorkflows(e.workflowsDir) + if err != nil { + fmt.Fprintf(errOut, "contexts: %v\n", err) + return exitCannot + } + declared = append(declared, e.alsoReported...) + + verdict := contexts.Judge(declared, required, e.absences) + fmt.Fprint(out, verdict.Report(declared, required, e.absences)) + if len(verdict.Refusals) > 0 { + return exitRefused + } + return exitClean +} + +// readRequired reads the contexts the ruleset requires, one per line. +// +// AN EMPTY LIST IS A LIST AND NOT A FAILURE. The ruleset on this board requires +// no status check today, so a run that read nothing is the ordinary case rather +// than a broken invocation, and the comparison still has work to do: every check +// name the tree declares has to be written down as a deliberate absence, and an +// absence naming nothing is still refused. A command that treated no input as a +// reason to stop would switch the whole check off on exactly the board it was +// written for. +// +// What it will not accept is a line that is not a context name. A required +// context is a check-run name, and a line carrying a tab or a control character +// is a fetch that returned something other than the list this expects, which is +// a broken invocation rather than a gate that disagrees. +func readRequired(in io.Reader) ([]string, error) { + var required []string + scanner := bufio.NewScanner(in) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if line == "" { + continue + } + if strings.ContainsAny(line, "\t") || strings.ContainsFunc(line, isControl) { + return nil, fmt.Errorf("the required set carries the line %q, which is not a check-run name, so what arrived on standard input is not the list this expects", line) + } + required = append(required, line) + } + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("cannot read the required set: %w", err) + } + return required, nil +} + +func isControl(r rune) bool { + return r < 0x20 || r == 0x7f +} diff --git a/cmd/contexts/main_test.go b/cmd/contexts/main_test.go new file mode 100644 index 0000000..e873a5b --- /dev/null +++ b/cmd/contexts/main_test.go @@ -0,0 +1,101 @@ +package main + +import ( + "bytes" + "path/filepath" + "strings" + "testing" +) + +// fixtures is a case directory from the package's own harness, reused here so +// the command is proved against the same trees the rules are. +const fixtures = "../../testdata/contexts" + +// TestTheCommandReturnsTheCodeItsVerdictEarns pins the contract record 0011 +// sets. Anything keyed on one of these codes is reading that record whether or +// not anybody said so, and a gate that returned the same number for a +// disagreement and for a broken fetch would report one as the other. +func TestTheCommandReturnsTheCodeItsVerdictEarns(t *testing.T) { + for _, one := range []struct { + name string + required string + dir string + want int + }{ + { + name: "the two lists agree", + required: "the first check\nthe second check\n", + dir: filepath.Join(fixtures, "everything-agrees", "workflows"), + want: exitClean, + }, + { + name: "a required context nothing reports", + required: "the first check\nthe second check\na check nobody wrote\n", + dir: filepath.Join(fixtures, "a-required-context-nothing-reports", "workflows"), + want: exitRefused, + }, + { + name: "there is no workflow directory to read", + required: "the first check\n", + dir: filepath.Join(fixtures, "there-is-no-such-case", "workflows"), + want: exitCannot, + }, + { + name: "what arrived is not a list of check names", + required: "the first check\tand a column of something else\n", + dir: filepath.Join(fixtures, "everything-agrees", "workflows"), + want: exitCannot, + }, + } { + t.Run(one.name, func(t *testing.T) { + var out, errOut bytes.Buffer + got := run(strings.NewReader(one.required), &out, &errOut, edges{workflowsDir: one.dir}) + if got != one.want { + t.Errorf("returned %d, want %d\nstdout: %s\nstderr: %s", got, one.want, out.String(), errOut.String()) + } + }) + } +} + +// TestTheReportSaysWhatItCompared refuses a run whose whole output is a verdict. +// A comparison of two empty lists and a comparison of two full ones both produce +// no refusals, and the only thing that tells them apart is the count the run +// printed. +func TestTheReportSaysWhatItCompared(t *testing.T) { + var out, errOut bytes.Buffer + code := run( + strings.NewReader("the first check\nthe second check\n"), + &out, &errOut, + edges{workflowsDir: filepath.Join(fixtures, "everything-agrees", "workflows")}, + ) + if code != exitClean { + t.Fatalf("returned %d\n%s", code, errOut.String()) + } + for _, want := range []string{ + "required by the ruleset: 2", + "declared by the workflows: 2", + "written down as deliberately absent:", + } { + if !strings.Contains(out.String(), want) { + t.Errorf("the report does not say %q\n%s", want, out.String()) + } + } +} + +// TestAnEmptyRequiredSetIsCompared refuses a command that treats no required +// contexts as nothing to do. That is the state of this board today, and a check +// that switched itself off there would be a check that has never run. +func TestAnEmptyRequiredSetIsCompared(t *testing.T) { + var out, errOut bytes.Buffer + code := run( + strings.NewReader(""), + &out, &errOut, + edges{workflowsDir: filepath.Join(fixtures, "everything-agrees", "workflows")}, + ) + if code != exitRefused { + t.Fatalf("returned %d, and with an empty required set every declared name is outside it\n%s%s", code, out.String(), errOut.String()) + } + if !strings.Contains(out.String(), "required by the ruleset: 0") { + t.Errorf("the report does not say the required set was empty\n%s", out.String()) + } +} diff --git a/internal/contexts/contexts.go b/internal/contexts/contexts.go new file mode 100644 index 0000000..8ec6353 --- /dev/null +++ b/internal/contexts/contexts.go @@ -0,0 +1,376 @@ +// Package contexts compares the two lists that have to agree for the merge +// gate on the default branch to mean what a reader thinks it means: the +// contexts the ruleset requires, and the check names the workflows in this +// repository declare. +// +// Five issues in the plan carry the same warning in different words. The check +// name is what the required set refers to, and a job renamed in passing removes +// itself from that set while the tab still looks green. Two other pieces of work +// come close and neither closes it. One assembles the required set from the +// names a completed run reported, which settles what the contexts are called on +// the day it runs. One walks the pull requests those contexts have to arrive on, +// which settles that they arrive on the day it walks. Both are true when they +// are done and neither notices the rename afterwards. +// +// THE JUDGEMENT IS A FUNCTION OVER VALUES AND IT READS NOTHING. Judge takes two +// lists and returns a verdict. Nothing here opens a file, runs a command or +// reads an environment variable, so every rule below is proved against a value a +// test writes out in full. Where the declared names come from is workflows.go's +// half, and the command under cmd/contexts is the only place that asks the API +// for the required set. +// +// # What a green verdict here does not say +// +// IT CANNOT JUDGE BEHAVIOUR. A job that reports the right name having verified +// nothing passes this comparison exactly like one that did its work. Nothing in +// a name says what ran under it, and the proof that a check bites is the fixture +// its own suite carries rather than the string it reports under. A green result +// here says the gate refers to checks that exist and that every check the tree +// declares is accounted for, and it says nothing whatever about what any of them +// found. +// +// IT READS THE RULESET AS THE API ANSWERED ON THE DAY IT RAN. The required set +// is a live setting rather than a tracked file, so a context added or removed by +// hand is caught on the next pull request rather than at the moment it changes. +// Between those two moments the gate and this comparison disagree and nothing +// says so. That is the ordinary failure of anything that describes a setting it +// does not own, and it is stated here because a green result is otherwise read +// as the gate being intact right now. +package contexts + +import ( + "fmt" + "sort" + "strings" +) + +// The properties this package can refuse. A property is the rule, named once +// here and named nowhere else, so a fixture declaring what it expects and a +// refusal the judgement produced are the same string or they are not equal. +const ( + // RequiredContextNothingReports refuses a context the ruleset requires + // that no workflow in this tree declares. It blocks every merge and gives + // no reason on the pull request, because a context that never arrives is + // indistinguishable from one that has not finished. The first response is + // to wait and the second is to drop the context from the required set, + // which repairs a blocked board by making the gate smaller. + RequiredContextNothingReports = "required-context-nothing-reports" + + // DeclaredNameOutsideTheRequiredSet refuses a check name the workflows + // declare that is neither in the required set nor written down as a + // deliberate absence. This is the quieter of the two directions: a gate + // somebody believes in and nothing enforces. It survives indefinitely, and + // a green board is what hides it. + DeclaredNameOutsideTheRequiredSet = "declared-name-outside-the-required-set" + + // AbsenceNamesNothingDeclared refuses a deliberate absence naming a check + // name no workflow in this tree declares. It is the direction that catches + // the rename while the required set is still empty: an absence is written + // against a literal string, so a job renamed in passing leaves its absence + // pointing at nothing and this refuses rather than passing in silence. + AbsenceNamesNothingDeclared = "absence-names-nothing-declared" + + // AbsenceIsRequired refuses a deliberate absence naming a context the + // ruleset requires. The two statements contradict each other, and the + // contradiction is what makes the absence list load-bearing rather than + // decorative: an entry that stayed behind after its context became + // required would silently exempt that context from the direction above. + AbsenceIsRequired = "absence-is-required" + + // NameCarriesAnExpression refuses a declared check name still carrying a + // workflow expression the reader could not resolve. A name reaching the + // comparison half-expanded would be compared against a string no gate can + // ever hold, so this refuses rather than reporting a drift that is really + // a reader that did not understand the file. + NameCarriesAnExpression = "name-carries-an-expression" +) + +// Absence is one check name this tree declares and deliberately keeps out of +// the required set, with the reason and with what retires it. +// +// THE LIST IS DATA AND NOT A JUDGEMENT. A comparison that cannot tell an +// intended absence from a drift produces a red board nobody can act on, and one +// that infers the intent produces a green board nobody can check. So every +// absence is written down as a literal string beside the reason for it, and an +// entry that stops matching the tree is a refusal rather than a silent pass. +type Absence struct { + // Name is the check name, exactly as the workflow declares it. + Name string + + // Why is the reason this name is outside the required set. + Why string + + // Until names the issue that retires this entry, or is empty where the + // absence is permanent. The difference is the whole value of the field: a + // permanent absence is a decision and a pending one is a debt, and a list + // that collapsed them would read as settled everywhere. + Until string +} + +// Absences is the deliberate-absence list, and it is here rather than in a +// document because this is where the comparison is defined and a list written +// somewhere else drifts against the thing that reads it. +// +// MOST OF THIS LIST IS ONE FACT REPEATED. The ruleset on the default branch +// requires no status check at all today, so every check name this tree declares +// is outside the required set, and each of them is written down rather than the +// whole comparison being switched off while the set is empty. What that buys is +// the rename: an absence names a literal string, so a job renamed while the set +// is still empty leaves its entry pointing at nothing and reddens this check. +// A comparison that special-cased the empty set would notice none of it. +// +// Issue #26 is where the set is assembled, and each pending entry below is +// retired by moving its name into the ruleset rather than by editing a document. +var Absences = []Absence{ + { + Name: "Scorecard analysis", + Why: "the supply-chain self-audit publishes from the default branch and has no pull-request trigger, so requiring it would require a context that never arrives on the thing being gated", + Until: "", + }, + {Name: "build (linux/amd64)", Why: theSetIsEmpty, Until: "#26"}, + {Name: "build (linux/arm64)", Why: theSetIsEmpty, Until: "#26"}, + {Name: "build (darwin/amd64)", Why: theSetIsEmpty, Until: "#26"}, + {Name: "build (darwin/arm64)", Why: theSetIsEmpty, Until: "#26"}, + {Name: "build (windows/amd64)", Why: theSetIsEmpty, Until: "#26"}, + {Name: "build (windows/arm64)", Why: theSetIsEmpty, Until: "#26"}, + {Name: "test (linux/amd64)", Why: theSetIsEmpty, Until: "#26"}, + {Name: "test (windows/amd64)", Why: theSetIsEmpty, Until: "#26"}, + {Name: "test (darwin/arm64)", Why: theSetIsEmpty, Until: "#26"}, + {Name: "vet", Why: theSetIsEmpty, Until: "#26"}, + {Name: "format", Why: theSetIsEmpty, Until: "#26"}, + {Name: "CodeQL (go)", Why: theSetIsEmpty, Until: "#26"}, + {Name: "CodeQL", Why: theSetIsEmpty, Until: "#26"}, + {Name: "zizmor", Why: theSetIsEmpty, Until: "#26"}, + {Name: "DCO sign-off", Why: theSetIsEmpty, Until: "#26"}, + {Name: "dependency-review", Why: theSetIsEmpty, Until: "#26"}, + {Name: "headless and unelevated", Why: theSetIsEmpty, Until: "#26"}, + {Name: "invariants", Why: theSetIsEmpty, Until: "#26"}, + {Name: "prose format", Why: theSetIsEmpty, Until: "#26"}, + {Name: "pull request", Why: theSetIsEmpty, Until: "#26"}, + {Name: "records", Why: theSetIsEmpty, Until: "#26"}, + {Name: "required contexts", Why: theSetIsEmpty, Until: "#26"}, + {Name: "Reject Trojan Source Unicode", Why: theSetIsEmpty, Until: "#26"}, + {Name: "Audit workflows (zizmor)", Why: theSetIsEmpty, Until: "#26"}, +} + +// theSetIsEmpty is the reason every pending entry above carries, written once so +// two dozen rows cannot drift into two dozen slightly different sentences. +const theSetIsEmpty = "the ruleset on the default branch requires no status check at all today, so no name this tree declares can be in a set that has no members" + +// ReportedOutsideAWorkflowFile is every check name that arrives on a commit here +// and is written in no workflow file, so the reader of those files can never +// find it. +// +// NOT EVERY CHECK RUN COMES FROM A JOB. A code-scanning upload creates a check +// run of its own, named after the analysis rather than after the job that +// uploaded it, and it is a name a ruleset can require exactly like any other. A +// comparison that knew only about jobs would refuse such a context as one +// nothing reports, which is a false red on the change that assembles the +// required set, and a false red is the failure this comparison exists to avoid +// rather than to cause. +// +// Measured rather than assumed. Every check run on one commit, with the +// application that created it: +// +// gh api "repos/Flowfin/lab/commits/$(git rev-parse HEAD)/check-runs" --paginate \ +// --jq '.check_runs[] | "\(.name)\t\(.app.slug)"' | sort -u +// +// On 2026-08-12 that printed twenty-four names, of which twenty-two came from +// github-actions and the two below came from github-advanced-security. Re-run it +// before trusting this list. +// +// WHAT THIS LIST COSTS. These two names are held here rather than read out of a +// file, so the rename this whole check is about is not caught for them: the +// string a code-scanning upload reports under is decided by the analysis rather +// than by a line somebody can change in this tree, and nothing here reads it. +// For these two the comparison is a statement that the name is expected, and for +// every other name it is a statement about what the tree says. The two are +// different claims and the report says which one a reader is holding. +var ReportedOutsideAWorkflowFile = []Declared{ + { + Name: "CodeQL", + Workflow: "codeql.yml", + FromUpload: true, + }, + { + Name: "zizmor", + Workflow: "zizmor.yml", + FromUpload: true, + }, +} + +// Declared is one check name a workflow in this tree declares. +type Declared struct { + // Name is the check name as it will be reported. + Name string + + // Workflow is the file it was read from, so a refusal names the file a + // reader has to open rather than only the string that was wrong. + Workflow string + + // FromJobID says the name was not written in the file and was taken from + // the job id, which is what the platform reports when a job carries no + // name. It is a note rather than a refusal: the fallback is the platform's + // own rule and a job relying on it is legal, but a reader asking where a + // gate's string comes from is entitled to know it came from an identifier + // rather than from a line somebody chose. + FromJobID bool + + // FromUpload says this name was not read out of a workflow file at all and + // comes from ReportedOutsideAWorkflowFile. Workflow then names the file + // whose run causes the check to appear rather than the file the string was + // read from, and a rename of the string is not something this comparison + // can see. It is a note for the same reason as the field above: what a + // reader is holding is different for these names and the report says so. + FromUpload bool +} + +// Refusal is one violation, named by its property so a fixture and a run compare +// the same strings. +type Refusal struct { + Property string + Subject string + Detail string +} + +func (r Refusal) String() string { + return fmt.Sprintf("%s: %s (%s)", r.Subject, r.Detail, r.Property) +} + +// Note is something worth printing that is not a violation. +type Note struct { + Subject string + Detail string +} + +// Verdict is what one comparison produced. +type Verdict struct { + Refusals []Refusal + Notes []Note +} + +// Properties is the set of property ids this verdict refused, sorted and +// deduplicated, which is what a fixture compares against. +func (v Verdict) Properties() []string { + seen := map[string]bool{} + var props []string + for _, refusal := range v.Refusals { + if !seen[refusal.Property] { + seen[refusal.Property] = true + props = append(props, refusal.Property) + } + } + sort.Strings(props) + return props +} + +// Judge compares what the ruleset requires against what the workflows declare, +// with the deliberate absences it is given. +// +// The absences are a parameter rather than a package-level read, so a fixture +// proves the rules against a list it wrote out in full and never against this +// repository's own. The list this repository runs with is Absences above, and +// the command passes it in. +func Judge(declared []Declared, required []string, absences []Absence) Verdict { + var verdict Verdict + + declaredByName := map[string]Declared{} + for _, entry := range declared { + if strings.Contains(entry.Name, "${{") { + verdict.Refusals = append(verdict.Refusals, Refusal{ + Property: NameCarriesAnExpression, + Subject: entry.Workflow, + Detail: fmt.Sprintf("the check name %q still carries a workflow expression, so what this job reports under was not resolved and nothing here can be compared against it", entry.Name), + }) + continue + } + declaredByName[entry.Name] = entry + if entry.FromJobID { + verdict.Notes = append(verdict.Notes, Note{ + Subject: entry.Workflow, + Detail: fmt.Sprintf("the check name %q comes from the job id, because the job carries no name of its own", entry.Name), + }) + } + if entry.FromUpload { + verdict.Notes = append(verdict.Notes, Note{ + Subject: entry.Workflow, + Detail: fmt.Sprintf("the check name %q is written in no workflow file and is expected here rather than read, so a change to the string it reports under is not caught by this comparison", entry.Name), + }) + } + } + + requiredSet := map[string]bool{} + for _, context := range required { + requiredSet[context] = true + } + + absent := map[string]bool{} + for _, absence := range absences { + absent[absence.Name] = true + if _, ok := declaredByName[absence.Name]; !ok { + verdict.Refusals = append(verdict.Refusals, Refusal{ + Property: AbsenceNamesNothingDeclared, + Subject: absence.Name, + Detail: fmt.Sprintf("this name is written down as deliberately outside the required set, and no workflow in this tree declares it. Either a job was renamed and this entry was left behind, or the entry never matched. The reason it carries is %q", absence.Why), + }) + } + if requiredSet[absence.Name] { + verdict.Refusals = append(verdict.Refusals, Refusal{ + Property: AbsenceIsRequired, + Subject: absence.Name, + Detail: fmt.Sprintf("the ruleset requires this context and this list says it is deliberately outside the required set, so the two disagree. The reason the list carries is %q", absence.Why), + }) + } + } + + for _, context := range sortedKeys(requiredSet) { + if _, ok := declaredByName[context]; !ok { + verdict.Refusals = append(verdict.Refusals, Refusal{ + Property: RequiredContextNothingReports, + Subject: context, + Detail: "the ruleset on the default branch requires this context and no workflow in this tree declares a check name matching it, so every merge waits for a tick that never arrives", + }) + } + } + + for _, name := range sortedKeys(declaredByName) { + if requiredSet[name] || absent[name] { + continue + } + verdict.Refusals = append(verdict.Refusals, Refusal{ + Property: DeclaredNameOutsideTheRequiredSet, + Subject: name, + Detail: fmt.Sprintf("%s declares this check name, the ruleset does not require it, and nothing writes it down as a deliberate absence, so it is a check that runs and holds no merge", declaredByName[name].Workflow), + }) + } + + return verdict +} + +// Report is what the run printed. It says what it examined before it says what +// it found, because a run that compared two empty lists and a run that compared +// two full ones both produce no refusals and are otherwise indistinguishable. +func (v Verdict) Report(declared []Declared, required []string, absences []Absence) string { + out := "compared the required contexts against the check names this tree declares\n" + out += fmt.Sprintf(" required by the ruleset: %d\n", len(required)) + out += fmt.Sprintf(" declared by the workflows: %d\n", len(declared)) + out += fmt.Sprintf(" written down as deliberately absent: %d\n", len(absences)) + + for _, note := range v.Notes { + out += fmt.Sprintf("note: %s: %s\n", note.Subject, note.Detail) + } + for _, refusal := range v.Refusals { + out += "refused: " + refusal.String() + "\n" + } + out += fmt.Sprintf("%d refusal(s), %d note(s)\n", len(v.Refusals), len(v.Notes)) + return out +} + +func sortedKeys[V any](m map[string]V) []string { + keys := make([]string, 0, len(m)) + for key := range m { + keys = append(keys, key) + } + sort.Strings(keys) + return keys +} diff --git a/internal/contexts/contexts_test.go b/internal/contexts/contexts_test.go new file mode 100644 index 0000000..aaeb1ec --- /dev/null +++ b/internal/contexts/contexts_test.go @@ -0,0 +1,321 @@ +// The harness these rules are proved with. +// +// A case is a directory under testdata/contexts/. It holds the workflow files +// the reader read, as files in the repository rather than as strings assembled +// at run time, so a reader can look at exactly the input the comparison saw. It +// holds the two lists the comparison is between, and it holds the whole set of +// refusals it should have produced. +// +// The layout of a case: +// +// testdata/contexts//workflows/ what the reader reads +// testdata/contexts//required one context per line, may be +// empty +// testdata/contexts//absences one deliberately absent check +// name per line, may be empty +// testdata/contexts//expected-refusals one property per line, may be +// empty +// testdata/contexts//near-neighbour the case that differs by the +// smallest legal change, required +// of a case that refuses +// +// It is the shape the record checks and the invariants are proved with rather +// than a third one, because a reader who has understood one harness here should +// not have to learn another. +package contexts + +import ( + "os" + "path/filepath" + "reflect" + "sort" + "strings" + "testing" +) + +// casesDir is where the cases live. Record 0002 puts the runner's own fixtures +// at the root of the tree rather than beside the package, so the path climbs out +// of internal/contexts. +const casesDir = "../../testdata/contexts" + +// workflowsInThisRepository is this repository's own workflow directory, read +// from inside the package directory. +const workflowsInThisRepository = "../../" + WorkflowsDir + +// expectation is what a case says the comparison should have produced. +type expectation struct { + required []string + absences []Absence + refusals []string + neighbour string +} + +// loadCases reads every case directory. It fails rather than skipping: a harness +// that quietly runs no cases is a green suite that proves nothing. +func loadCases(t *testing.T) map[string]expectation { + t.Helper() + + entries, err := os.ReadDir(casesDir) + if err != nil { + t.Fatalf("cannot read %s: %v", casesDir, err) + } + cases := map[string]expectation{} + for _, entry := range entries { + if !entry.IsDir() { + continue + } + dir := filepath.Join(casesDir, entry.Name()) + exp := expectation{ + required: lines(t, dir, "required"), + refusals: lines(t, dir, "expected-refusals"), + neighbour: readOptional(t, dir, "near-neighbour"), + } + for _, name := range lines(t, dir, "absences") { + exp.absences = append(exp.absences, Absence{ + Name: name, + Why: "this case says so", + Until: "", + }) + } + sort.Strings(exp.refusals) + cases[entry.Name()] = exp + } + if len(cases) == 0 { + t.Fatalf("no cases under %s", casesDir) + } + return cases +} + +func lines(t *testing.T, dir, name string) []string { + t.Helper() + data, err := os.ReadFile(filepath.Join(dir, name)) + if err != nil { + if os.IsNotExist(err) { + return nil + } + t.Fatalf("cannot read %s: %v", filepath.Join(dir, name), err) + } + var out []string + for _, line := range strings.Split(string(data), "\n") { + line = strings.TrimSpace(line) + if line != "" { + out = append(out, line) + } + } + return out +} + +func readOptional(t *testing.T, dir, name string) string { + t.Helper() + data, err := os.ReadFile(filepath.Join(dir, name)) + if err != nil { + if os.IsNotExist(err) { + return "" + } + t.Fatalf("cannot read %s: %v", filepath.Join(dir, name), err) + } + return strings.TrimSpace(string(data)) +} + +// TestACaseRefusesExactlyWhatItDeclares is the leg that makes a fixture worth +// having. It compares the whole set of property ids, so a rule that refuses its +// own case and something else besides fails here rather than passing on the +// strength of the one line it was written for. +func TestACaseRefusesExactlyWhatItDeclares(t *testing.T) { + for name, exp := range loadCases(t) { + t.Run(name, func(t *testing.T) { + declared, err := ReadWorkflows(filepath.Join(casesDir, name, "workflows")) + if err != nil { + t.Fatalf("reading the workflows of %s: %v", name, err) + } + verdict := Judge(declared, exp.required, exp.absences) + got := verdict.Properties() + if got == nil { + got = []string{} + } + want := exp.refusals + if want == nil { + want = []string{} + } + if !reflect.DeepEqual(got, want) { + t.Errorf("case %s refused %v and declares %v\n%s", name, got, want, verdict.Report(declared, exp.required, exp.absences)) + } + }) + } +} + +// TestEveryPropertyHasACaseThatTripsIt refuses a property nothing proves. A +// property with no case behind it is a rule nobody has watched bite, and the +// whole reason the fixtures exist is that a rule which cannot be shown to refuse +// is indistinguishable from one that never runs. +func TestEveryPropertyHasACaseThatTripsIt(t *testing.T) { + all := []string{ + RequiredContextNothingReports, + DeclaredNameOutsideTheRequiredSet, + AbsenceNamesNothingDeclared, + AbsenceIsRequired, + NameCarriesAnExpression, + } + tripped := map[string]bool{} + for _, exp := range loadCases(t) { + for _, property := range exp.refusals { + tripped[property] = true + } + } + for _, property := range all { + if !tripped[property] { + t.Errorf("no case trips %s", property) + } + } + for property := range tripped { + found := false + for _, known := range all { + if property == known { + found = true + } + } + if !found { + t.Errorf("a case declares %s and this package has no such property, so the case proves nothing", property) + } + } +} + +// TestACaseThatRefusesNamesANearNeighbour is the leg that catches a rule +// refusing everything. A case proving a refusal names a case differing by the +// smallest legal change, and that case has to come out clean, so a rule that +// fires on anything at all fails here. +func TestACaseThatRefusesNamesANearNeighbour(t *testing.T) { + cases := loadCases(t) + for name, exp := range cases { + if len(exp.refusals) == 0 { + if exp.neighbour != "" { + t.Errorf("case %s refuses nothing and declares a near neighbour", name) + } + continue + } + if exp.neighbour == "" { + t.Errorf("case %s refuses %v and declares no near neighbour, so nothing here would notice a rule that refuses everything", name, exp.refusals) + continue + } + neighbour, ok := cases[exp.neighbour] + if !ok { + t.Errorf("case %s names %s as its near neighbour and there is no such case", name, exp.neighbour) + continue + } + if len(neighbour.refusals) != 0 { + t.Errorf("case %s names %s as its near neighbour, and that case expects %v rather than nothing", name, exp.neighbour, neighbour.refusals) + } + } +} + +// declarationsInThisRepository is every check name that arrives on a commit +// here: the ones the workflow files declare, and the ones a code-scanning upload +// reports under, which are written down in ReportedOutsideAWorkflowFile because +// no file in this tree carries them. +func declarationsInThisRepository(t *testing.T) []Declared { + t.Helper() + declared, err := ReadWorkflows(workflowsInThisRepository) + if err != nil { + t.Fatalf("reading %s: %v", workflowsInThisRepository, err) + } + return append(declared, ReportedOutsideAWorkflowFile...) +} + +// TestNoDeliberateAbsenceNamesSomethingThisTreeDoesNotDeclare is the rename +// refusal, run against this repository rather than against a fixture. +// +// It is here as well as in the workflow because it needs no network. Every entry +// in the absence list is a literal check name, so a job renamed in passing +// leaves its entry pointing at nothing, and this fails in an ordinary suite run +// on the machine of whoever made the rename rather than waiting for the pull +// request. What it cannot do is the other direction, which needs the required +// set and therefore the API, and that half is the workflow's. +func TestNoDeliberateAbsenceNamesSomethingThisTreeDoesNotDeclare(t *testing.T) { + declared := declarationsInThisRepository(t) + names := map[string]bool{} + for _, entry := range declared { + names[entry.Name] = true + } + for _, absence := range Absences { + if !names[absence.Name] { + t.Errorf("the absence list names %q and no workflow in this tree declares it, so either a job was renamed and the entry was left behind or the entry never matched", absence.Name) + } + } +} + +// TestEveryCheckNameThisTreeDeclaresIsWrittenDown is the same reading from the +// other side, and it holds only while the ruleset requires no status check. +// +// The command is what judges this against the live ruleset. This test exists +// because today the required set is empty, which makes the answer knowable +// without asking: every name has to be in the absence list, because there is no +// other place for it to be. When issue #26 assembles the set, a name moving into +// the ruleset is a name leaving this list, and this test is what makes that +// removal deliberate rather than silent. +func TestEveryCheckNameThisTreeDeclaresIsWrittenDown(t *testing.T) { + declared := declarationsInThisRepository(t) + written := map[string]bool{} + for _, absence := range Absences { + written[absence.Name] = true + } + for _, entry := range declared { + if !written[entry.Name] { + t.Errorf("%s declares the check name %q and nothing writes it down, so it is a check that runs and holds no merge", entry.Workflow, entry.Name) + } + } +} + +// TestAnAbsenceCarriesAReason refuses an entry that says only that it is absent. +// The list is read by somebody deciding whether an absence is still right, and +// an entry with no reason gives them nothing to decide with. +func TestAnAbsenceCarriesAReason(t *testing.T) { + for _, absence := range Absences { + if strings.TrimSpace(absence.Why) == "" { + t.Errorf("the absence %q carries no reason", absence.Name) + } + if absence.Until != "" && !strings.HasPrefix(absence.Until, "#") { + t.Errorf("the absence %q says it is retired by %q, which is not an issue reference", absence.Name, absence.Until) + } + } +} + +// TestAJobWithNoNameOfItsOwnIsNotedRatherThanRefused pins the one place this +// package answers with a note. The platform reports the job id where a job +// carries no name, so such a job is legal and its name is still a gate string, and +// a reader asking where that string came from is told. +func TestAJobWithNoNameOfItsOwnIsNotedRatherThanRefused(t *testing.T) { + declared := []Declared{{Name: "dependency-review", Workflow: "dependency-review.yml", FromJobID: true}} + verdict := Judge(declared, []string{"dependency-review"}, nil) + if len(verdict.Refusals) != 0 { + t.Fatalf("refused %v", verdict.Refusals) + } + if len(verdict.Notes) != 1 { + t.Fatalf("produced %d note(s), and the fallback to the job id is meant to be said out loud", len(verdict.Notes)) + } +} + +// TestANameReportedOutsideAWorkflowFileIsNotedRatherThanRefused pins the other +// place this package answers with a note. A code-scanning upload creates a check +// run named after the analysis rather than after the job that uploaded it, so +// the name is in no workflow file and a comparison that knew only about jobs +// would refuse a required context matching it as one nothing reports. That would +// be a false red on the change that assembles the required set. +func TestANameReportedOutsideAWorkflowFileIsNotedRatherThanRefused(t *testing.T) { + if len(ReportedOutsideAWorkflowFile) == 0 { + t.Fatal("nothing is written down as reported outside a workflow file, and the measurement in the list's comment says two names are") + } + var required []string + for _, entry := range ReportedOutsideAWorkflowFile { + if !entry.FromUpload { + t.Errorf("%q is in this list and is not marked as coming from an upload, so the report would describe it as read out of a file", entry.Name) + } + required = append(required, entry.Name) + } + verdict := Judge(ReportedOutsideAWorkflowFile, required, nil) + if len(verdict.Refusals) != 0 { + t.Errorf("refused %v, and a required context these names match is reported rather than missing", verdict.Refusals) + } + if len(verdict.Notes) != len(ReportedOutsideAWorkflowFile) { + t.Errorf("produced %d note(s) for %d name(s), and what a reader is holding for these is different from a name read out of a file", len(verdict.Notes), len(ReportedOutsideAWorkflowFile)) + } +} diff --git a/internal/contexts/workflows.go b/internal/contexts/workflows.go new file mode 100644 index 0000000..9fab96c --- /dev/null +++ b/internal/contexts/workflows.go @@ -0,0 +1,239 @@ +package contexts + +import ( + "fmt" + "os" + "path/filepath" + "sort" + "strings" +) + +// WorkflowsDir is where the platform reads workflows from, and therefore the +// only directory a declared check name can come from. +const WorkflowsDir = ".github/workflows" + +// ReadWorkflows reads every workflow file under the directory it is given and +// returns the check names they declare. +// +// WHY THIS READS THE FILES RATHER THAN A COMPLETED RUN. A run reports the names +// of the jobs that ran on it, which is a different set: a job filtered out by a +// condition reports nothing, and a workflow added on a branch reports nothing +// until it runs there. What the gate has to agree with is what the tree +// declares, because the tree is the thing a pull request changes and the rename +// this check exists to catch is a change to a file. +// +// WHAT IT UNDERSTANDS, AND WHERE IT STOPS. This module carries no dependencies +// and adding a YAML parser to read a dozen files of one known shape is a runtime +// cost paid for something narrower than the library. So it reads the shape the +// workflows in this tree are written in, by line and by indentation: a job is a +// key two spaces in under `jobs:`, its name is `name:` four spaces in, and a +// matrix is an `include:` list of flat key-and-value entries. It stops reading a +// job's own keys at `steps:`, because everything below that belongs to a step +// rather than to the job. +// +// Anything it does not understand is a refusal rather than a guess. A name it +// cannot finish expanding keeps its expression and Judge refuses it by name, so +// a workflow written in a shape this reader was not built for reddens the check +// instead of quietly declaring the wrong string. +func ReadWorkflows(dir string) ([]Declared, error) { + entries, err := os.ReadDir(dir) + if err != nil { + return nil, fmt.Errorf("cannot read %s, which is where a declared check name can only come from: %w", dir, err) + } + + var declared []Declared + for _, entry := range entries { + if entry.IsDir() || !isWorkflowFile(entry.Name()) { + continue + } + path := filepath.Join(dir, entry.Name()) + data, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("cannot read %s: %w", path, err) + } + found, err := readWorkflow(entry.Name(), string(data)) + if err != nil { + return nil, err + } + declared = append(declared, found...) + } + + if len(declared) == 0 { + // A reader that returned nothing would make every deliberate absence + // dangle and every required context unreported, which is a wall of + // refusals describing a reader that found no files rather than a tree + // that is wrong. Say the real thing instead. + return nil, fmt.Errorf("%s declared no check name at all, so either there are no workflows there or this reader did not understand any of them", dir) + } + + sort.Slice(declared, func(i, j int) bool { + if declared[i].Workflow != declared[j].Workflow { + return declared[i].Workflow < declared[j].Workflow + } + return declared[i].Name < declared[j].Name + }) + return declared, nil +} + +func isWorkflowFile(name string) bool { + return strings.HasSuffix(name, ".yml") || strings.HasSuffix(name, ".yaml") +} + +// job is one job as this reader understood it. +type job struct { + id string + name string + named bool + matrix []map[string]string + inSteps bool +} + +// readWorkflow returns the check names one workflow file declares. +func readWorkflow(file, text string) ([]Declared, error) { + var jobs []*job + var current *job + + inJobs := false + includeIndent := -1 + + for _, raw := range strings.Split(text, "\n") { + line := strings.TrimRight(raw, "\r") + trimmed := strings.TrimSpace(line) + if trimmed == "" || strings.HasPrefix(trimmed, "#") { + continue + } + indent := len(line) - len(strings.TrimLeft(line, " ")) + + if indent == 0 { + inJobs = trimmed == "jobs:" + current = nil + includeIndent = -1 + continue + } + if !inJobs { + continue + } + + if indent == 2 && strings.HasSuffix(trimmed, ":") { + current = &job{id: strings.TrimSuffix(trimmed, ":")} + jobs = append(jobs, current) + includeIndent = -1 + continue + } + if current == nil { + continue + } + + // Inside an include list: entries are flat key-and-value pairs, a new + // entry starts at a dash, and the list ends at the first line that is + // not deeper than the `include:` key itself. + if includeIndent >= 0 { + if indent > includeIndent { + if err := readMatrixEntry(current, trimmed, file); err != nil { + return nil, err + } + continue + } + includeIndent = -1 + } + + if indent == 4 { + // Everything under steps: belongs to a step. A step carries a + // name of its own and reading one as the job's would declare a + // check name that no gate will ever see. + if trimmed == "steps:" { + current.inSteps = true + continue + } + if !current.inSteps && strings.HasPrefix(trimmed, "name:") { + current.name = unquote(strings.TrimSpace(strings.TrimPrefix(trimmed, "name:"))) + current.named = true + } + continue + } + if !current.inSteps && trimmed == "include:" { + includeIndent = indent + } + } + + var declared []Declared + for _, j := range jobs { + name := j.name + if !j.named { + // The platform reports the job id where a job carries no name of + // its own, so that is the string a required context would have to + // match. + name = j.id + } + expanded, err := expand(name, j.matrix, file) + if err != nil { + return nil, err + } + for _, one := range expanded { + declared = append(declared, Declared{Name: one, Workflow: file, FromJobID: !j.named}) + } + } + return declared, nil +} + +// readMatrixEntry reads one `key: value` line of an include list into the entry +// it belongs to. +func readMatrixEntry(j *job, trimmed, file string) error { + if strings.HasPrefix(trimmed, "- ") { + j.matrix = append(j.matrix, map[string]string{}) + trimmed = strings.TrimSpace(strings.TrimPrefix(trimmed, "- ")) + } + if len(j.matrix) == 0 { + return fmt.Errorf("%s: the job %q has a line in its include list before any entry started, which is a shape this reader was not built for", file, j.id) + } + key, value, found := strings.Cut(trimmed, ":") + if !found { + return fmt.Errorf("%s: the job %q has the include line %q, which carries no key and value, and this reader will not guess at it", file, j.id, trimmed) + } + j.matrix[len(j.matrix)-1][strings.TrimSpace(key)] = unquote(strings.TrimSpace(value)) + return nil +} + +// expand turns a job name carrying matrix expressions into one name per matrix +// entry. A name with no expression is itself, whether or not the job has a +// matrix, because a matrix job whose name mentions no matrix key reports one +// check name per entry under the same string and the gate sees one context. +func expand(name string, matrix []map[string]string, file string) ([]string, error) { + if !strings.Contains(name, "${{") { + return []string{name}, nil + } + if len(matrix) == 0 { + // The name is left as it stands, expression included, and Judge + // refuses it by name. Returning an error here would stop the whole + // run on one file, and a refusal naming the workflow is what a reader + // can act on. + return []string{name}, nil + } + var out []string + seen := map[string]bool{} + for _, entry := range matrix { + expanded := name + for key, value := range entry { + expanded = strings.ReplaceAll(expanded, "${{ matrix."+key+" }}", value) + expanded = strings.ReplaceAll(expanded, "${{matrix."+key+"}}", value) + } + if seen[expanded] { + continue + } + seen[expanded] = true + out = append(out, expanded) + } + if len(out) == 0 { + return nil, fmt.Errorf("%s: the name %q expanded to nothing", file, name) + } + return out, nil +} + +func unquote(value string) string { + if len(value) >= 2 { + if (value[0] == '"' && value[len(value)-1] == '"') || (value[0] == '\'' && value[len(value)-1] == '\'') { + return value[1 : len(value)-1] + } + } + return value +} diff --git a/internal/contexts/workflows_test.go b/internal/contexts/workflows_test.go new file mode 100644 index 0000000..9e23f86 --- /dev/null +++ b/internal/contexts/workflows_test.go @@ -0,0 +1,131 @@ +package contexts + +import ( + "reflect" + "testing" +) + +// TestTheReaderExpandsAMatrixName pins what a matrix job declares. Six entries +// under one name are six contexts a gate has to hold, and a reader that returned +// the unexpanded string would compare against a name no ruleset can ever carry. +func TestTheReaderExpandsAMatrixName(t *testing.T) { + const file = ` +jobs: + build: + name: build (${{ matrix.goos }}/${{ matrix.goarch }}) + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + include: + - goos: linux + goarch: amd64 + - goos: windows + goarch: arm64 + steps: + - name: Build + run: echo build +` + declared, err := readWorkflow("build.yml", file) + if err != nil { + t.Fatalf("%v", err) + } + var got []string + for _, entry := range declared { + got = append(got, entry.Name) + } + want := []string{"build (linux/amd64)", "build (windows/arm64)"} + if !reflect.DeepEqual(got, want) { + t.Errorf("read %v, want %v", got, want) + } +} + +// TestTheReaderDoesNotReadAStepNameAsAJobName is the near-miss worth spending +// the effort on. Both keys are spelled `name:` and both sit under the same job, +// so a reader keyed on the word rather than on where it sits would declare a +// step's name as a check name, and that string would then be compared against a +// gate and found missing for a reason nobody could follow. +func TestTheReaderDoesNotReadAStepNameAsAJobName(t *testing.T) { + const file = ` +jobs: + records: + name: records + runs-on: ubuntu-latest + steps: + - name: Checkout Repository + uses: actions/checkout@0000000000000000000000000000000000000000 + - name: Walk the records + run: go run ./cmd/lab check . +` + declared, err := readWorkflow("records.yml", file) + if err != nil { + t.Fatalf("%v", err) + } + if len(declared) != 1 || declared[0].Name != "records" { + t.Errorf("read %+v, and the only check name this file declares is \"records\"", declared) + } +} + +// TestAJobWithNoNameDeclaresItsJobID pins the platform's own fallback, which is +// the rule a required context has to match against dependency-review.yml. +func TestAJobWithNoNameDeclaresItsJobID(t *testing.T) { + const file = ` +jobs: + dependency-review: + runs-on: ubuntu-latest + steps: + - name: Dependency review + uses: actions/dependency-review-action@0000000000000000000000000000000000000000 +` + declared, err := readWorkflow("dependency-review.yml", file) + if err != nil { + t.Fatalf("%v", err) + } + if len(declared) != 1 || declared[0].Name != "dependency-review" || !declared[0].FromJobID { + t.Errorf("read %+v, want the job id declared and marked as coming from the job id", declared) + } +} + +// TestAWorkflowNameIsNotAJobName pins that the string at the top of the file is +// the workflow's rather than a job's. A gate holds check names, and a reader +// that declared the workflow name would put a string in the comparison that no +// check run is ever called. +func TestAWorkflowNameIsNotAJobName(t *testing.T) { + const file = ` +name: Build and test + +on: + pull_request: + +jobs: + vet: + name: vet + runs-on: ubuntu-latest + steps: + - name: Vet + run: go vet ./... +` + declared, err := readWorkflow("build.yml", file) + if err != nil { + t.Fatalf("%v", err) + } + if len(declared) != 1 || declared[0].Name != "vet" { + t.Errorf("read %+v, want only the job name", declared) + } +} + +// TestThisRepositoryDeclaresTheNamesItsWorkflowsCarry reads the real directory +// rather than a string, so a workflow written in a shape this reader was not +// built for is caught here rather than as a wall of refusals on a pull request. +func TestThisRepositoryDeclaresTheNamesItsWorkflowsCarry(t *testing.T) { + declared, err := ReadWorkflows(workflowsInThisRepository) + if err != nil { + t.Fatalf("%v", err) + } + for _, entry := range declared { + if entry.Name == "" { + t.Errorf("%s declares an empty check name", entry.Workflow) + } + } + t.Logf("this tree declares %d check name(s)", len(declared)) +} diff --git a/testdata/contexts/a-declared-name-outside-the-required-set/absences b/testdata/contexts/a-declared-name-outside-the-required-set/absences new file mode 100644 index 0000000..e69de29 diff --git a/testdata/contexts/a-declared-name-outside-the-required-set/expected-refusals b/testdata/contexts/a-declared-name-outside-the-required-set/expected-refusals new file mode 100644 index 0000000..48576dc --- /dev/null +++ b/testdata/contexts/a-declared-name-outside-the-required-set/expected-refusals @@ -0,0 +1 @@ +declared-name-outside-the-required-set diff --git a/testdata/contexts/a-declared-name-outside-the-required-set/near-neighbour b/testdata/contexts/a-declared-name-outside-the-required-set/near-neighbour new file mode 100644 index 0000000..8255792 --- /dev/null +++ b/testdata/contexts/a-declared-name-outside-the-required-set/near-neighbour @@ -0,0 +1 @@ +everything-agrees diff --git a/testdata/contexts/a-declared-name-outside-the-required-set/required b/testdata/contexts/a-declared-name-outside-the-required-set/required new file mode 100644 index 0000000..34a4e91 --- /dev/null +++ b/testdata/contexts/a-declared-name-outside-the-required-set/required @@ -0,0 +1 @@ +the first check diff --git a/testdata/contexts/a-declared-name-outside-the-required-set/workflows/two-jobs.yml b/testdata/contexts/a-declared-name-outside-the-required-set/workflows/two-jobs.yml new file mode 100644 index 0000000..49fda54 --- /dev/null +++ b/testdata/contexts/a-declared-name-outside-the-required-set/workflows/two-jobs.yml @@ -0,0 +1,20 @@ +name: Two jobs + +on: + pull_request: + +permissions: {} + +jobs: + first: + name: the first check + runs-on: ubuntu-latest + steps: + - name: Do the first thing + run: echo first + second: + name: the second check + runs-on: ubuntu-latest + steps: + - name: Do the second thing + run: echo second diff --git a/testdata/contexts/a-job-renamed-while-the-set-is-empty/absences b/testdata/contexts/a-job-renamed-while-the-set-is-empty/absences new file mode 100644 index 0000000..f8407c5 --- /dev/null +++ b/testdata/contexts/a-job-renamed-while-the-set-is-empty/absences @@ -0,0 +1,2 @@ +the first check +the second check diff --git a/testdata/contexts/a-job-renamed-while-the-set-is-empty/expected-refusals b/testdata/contexts/a-job-renamed-while-the-set-is-empty/expected-refusals new file mode 100644 index 0000000..f32f74c --- /dev/null +++ b/testdata/contexts/a-job-renamed-while-the-set-is-empty/expected-refusals @@ -0,0 +1,2 @@ +absence-names-nothing-declared +declared-name-outside-the-required-set diff --git a/testdata/contexts/a-job-renamed-while-the-set-is-empty/near-neighbour b/testdata/contexts/a-job-renamed-while-the-set-is-empty/near-neighbour new file mode 100644 index 0000000..7c578a9 --- /dev/null +++ b/testdata/contexts/a-job-renamed-while-the-set-is-empty/near-neighbour @@ -0,0 +1 @@ +the-set-is-empty-and-every-name-is-written-down diff --git a/testdata/contexts/a-job-renamed-while-the-set-is-empty/required b/testdata/contexts/a-job-renamed-while-the-set-is-empty/required new file mode 100644 index 0000000..e69de29 diff --git a/testdata/contexts/a-job-renamed-while-the-set-is-empty/workflows/two-jobs.yml b/testdata/contexts/a-job-renamed-while-the-set-is-empty/workflows/two-jobs.yml new file mode 100644 index 0000000..5b60d1f --- /dev/null +++ b/testdata/contexts/a-job-renamed-while-the-set-is-empty/workflows/two-jobs.yml @@ -0,0 +1,20 @@ +name: Two jobs + +on: + pull_request: + +permissions: {} + +jobs: + first: + name: the first check + runs-on: ubuntu-latest + steps: + - name: Do the first thing + run: echo first + second: + name: the second check, tidied + runs-on: ubuntu-latest + steps: + - name: Do the second thing + run: echo second diff --git a/testdata/contexts/a-matrix-name-expands-per-entry/absences b/testdata/contexts/a-matrix-name-expands-per-entry/absences new file mode 100644 index 0000000..e69de29 diff --git a/testdata/contexts/a-matrix-name-expands-per-entry/expected-refusals b/testdata/contexts/a-matrix-name-expands-per-entry/expected-refusals new file mode 100644 index 0000000..e69de29 diff --git a/testdata/contexts/a-matrix-name-expands-per-entry/required b/testdata/contexts/a-matrix-name-expands-per-entry/required new file mode 100644 index 0000000..d519c6c --- /dev/null +++ b/testdata/contexts/a-matrix-name-expands-per-entry/required @@ -0,0 +1,2 @@ +build (linux/amd64) +build (windows/arm64) diff --git a/testdata/contexts/a-matrix-name-expands-per-entry/workflows/matrix.yml b/testdata/contexts/a-matrix-name-expands-per-entry/workflows/matrix.yml new file mode 100644 index 0000000..4950396 --- /dev/null +++ b/testdata/contexts/a-matrix-name-expands-per-entry/workflows/matrix.yml @@ -0,0 +1,22 @@ +name: A matrix + +on: + pull_request: + +permissions: {} + +jobs: + build: + name: build (${{ matrix.goos }}/${{ matrix.goarch }}) + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + include: + - goos: linux + goarch: amd64 + - goos: windows + goarch: arm64 + steps: + - name: Build + run: echo build diff --git a/testdata/contexts/a-name-this-reader-cannot-expand/absences b/testdata/contexts/a-name-this-reader-cannot-expand/absences new file mode 100644 index 0000000..e69de29 diff --git a/testdata/contexts/a-name-this-reader-cannot-expand/expected-refusals b/testdata/contexts/a-name-this-reader-cannot-expand/expected-refusals new file mode 100644 index 0000000..57eb762 --- /dev/null +++ b/testdata/contexts/a-name-this-reader-cannot-expand/expected-refusals @@ -0,0 +1 @@ +name-carries-an-expression diff --git a/testdata/contexts/a-name-this-reader-cannot-expand/near-neighbour b/testdata/contexts/a-name-this-reader-cannot-expand/near-neighbour new file mode 100644 index 0000000..c784b3b --- /dev/null +++ b/testdata/contexts/a-name-this-reader-cannot-expand/near-neighbour @@ -0,0 +1 @@ +a-matrix-name-expands-per-entry diff --git a/testdata/contexts/a-name-this-reader-cannot-expand/required b/testdata/contexts/a-name-this-reader-cannot-expand/required new file mode 100644 index 0000000..e69de29 diff --git a/testdata/contexts/a-name-this-reader-cannot-expand/workflows/no-matrix.yml b/testdata/contexts/a-name-this-reader-cannot-expand/workflows/no-matrix.yml new file mode 100644 index 0000000..1028f9e --- /dev/null +++ b/testdata/contexts/a-name-this-reader-cannot-expand/workflows/no-matrix.yml @@ -0,0 +1,14 @@ +name: A name with nothing behind it + +on: + pull_request: + +permissions: {} + +jobs: + build: + name: build (${{ matrix.goos }}) + runs-on: ubuntu-latest + steps: + - name: Build + run: echo build diff --git a/testdata/contexts/a-required-context-nothing-reports/absences b/testdata/contexts/a-required-context-nothing-reports/absences new file mode 100644 index 0000000..e69de29 diff --git a/testdata/contexts/a-required-context-nothing-reports/expected-refusals b/testdata/contexts/a-required-context-nothing-reports/expected-refusals new file mode 100644 index 0000000..f749eda --- /dev/null +++ b/testdata/contexts/a-required-context-nothing-reports/expected-refusals @@ -0,0 +1 @@ +required-context-nothing-reports diff --git a/testdata/contexts/a-required-context-nothing-reports/near-neighbour b/testdata/contexts/a-required-context-nothing-reports/near-neighbour new file mode 100644 index 0000000..8255792 --- /dev/null +++ b/testdata/contexts/a-required-context-nothing-reports/near-neighbour @@ -0,0 +1 @@ +everything-agrees diff --git a/testdata/contexts/a-required-context-nothing-reports/required b/testdata/contexts/a-required-context-nothing-reports/required new file mode 100644 index 0000000..a502568 --- /dev/null +++ b/testdata/contexts/a-required-context-nothing-reports/required @@ -0,0 +1,3 @@ +the first check +the second check +a check nobody wrote diff --git a/testdata/contexts/a-required-context-nothing-reports/workflows/two-jobs.yml b/testdata/contexts/a-required-context-nothing-reports/workflows/two-jobs.yml new file mode 100644 index 0000000..49fda54 --- /dev/null +++ b/testdata/contexts/a-required-context-nothing-reports/workflows/two-jobs.yml @@ -0,0 +1,20 @@ +name: Two jobs + +on: + pull_request: + +permissions: {} + +jobs: + first: + name: the first check + runs-on: ubuntu-latest + steps: + - name: Do the first thing + run: echo first + second: + name: the second check + runs-on: ubuntu-latest + steps: + - name: Do the second thing + run: echo second diff --git a/testdata/contexts/an-absence-that-is-required/absences b/testdata/contexts/an-absence-that-is-required/absences new file mode 100644 index 0000000..cfabd08 --- /dev/null +++ b/testdata/contexts/an-absence-that-is-required/absences @@ -0,0 +1 @@ +the second check diff --git a/testdata/contexts/an-absence-that-is-required/expected-refusals b/testdata/contexts/an-absence-that-is-required/expected-refusals new file mode 100644 index 0000000..d05a48a --- /dev/null +++ b/testdata/contexts/an-absence-that-is-required/expected-refusals @@ -0,0 +1 @@ +absence-is-required diff --git a/testdata/contexts/an-absence-that-is-required/near-neighbour b/testdata/contexts/an-absence-that-is-required/near-neighbour new file mode 100644 index 0000000..8255792 --- /dev/null +++ b/testdata/contexts/an-absence-that-is-required/near-neighbour @@ -0,0 +1 @@ +everything-agrees diff --git a/testdata/contexts/an-absence-that-is-required/required b/testdata/contexts/an-absence-that-is-required/required new file mode 100644 index 0000000..f8407c5 --- /dev/null +++ b/testdata/contexts/an-absence-that-is-required/required @@ -0,0 +1,2 @@ +the first check +the second check diff --git a/testdata/contexts/an-absence-that-is-required/workflows/two-jobs.yml b/testdata/contexts/an-absence-that-is-required/workflows/two-jobs.yml new file mode 100644 index 0000000..49fda54 --- /dev/null +++ b/testdata/contexts/an-absence-that-is-required/workflows/two-jobs.yml @@ -0,0 +1,20 @@ +name: Two jobs + +on: + pull_request: + +permissions: {} + +jobs: + first: + name: the first check + runs-on: ubuntu-latest + steps: + - name: Do the first thing + run: echo first + second: + name: the second check + runs-on: ubuntu-latest + steps: + - name: Do the second thing + run: echo second diff --git a/testdata/contexts/an-absence-that-names-nothing/absences b/testdata/contexts/an-absence-that-names-nothing/absences new file mode 100644 index 0000000..7b45295 --- /dev/null +++ b/testdata/contexts/an-absence-that-names-nothing/absences @@ -0,0 +1 @@ +a check nobody wrote diff --git a/testdata/contexts/an-absence-that-names-nothing/expected-refusals b/testdata/contexts/an-absence-that-names-nothing/expected-refusals new file mode 100644 index 0000000..b46ddf8 --- /dev/null +++ b/testdata/contexts/an-absence-that-names-nothing/expected-refusals @@ -0,0 +1 @@ +absence-names-nothing-declared diff --git a/testdata/contexts/an-absence-that-names-nothing/near-neighbour b/testdata/contexts/an-absence-that-names-nothing/near-neighbour new file mode 100644 index 0000000..8255792 --- /dev/null +++ b/testdata/contexts/an-absence-that-names-nothing/near-neighbour @@ -0,0 +1 @@ +everything-agrees diff --git a/testdata/contexts/an-absence-that-names-nothing/required b/testdata/contexts/an-absence-that-names-nothing/required new file mode 100644 index 0000000..f8407c5 --- /dev/null +++ b/testdata/contexts/an-absence-that-names-nothing/required @@ -0,0 +1,2 @@ +the first check +the second check diff --git a/testdata/contexts/an-absence-that-names-nothing/workflows/two-jobs.yml b/testdata/contexts/an-absence-that-names-nothing/workflows/two-jobs.yml new file mode 100644 index 0000000..49fda54 --- /dev/null +++ b/testdata/contexts/an-absence-that-names-nothing/workflows/two-jobs.yml @@ -0,0 +1,20 @@ +name: Two jobs + +on: + pull_request: + +permissions: {} + +jobs: + first: + name: the first check + runs-on: ubuntu-latest + steps: + - name: Do the first thing + run: echo first + second: + name: the second check + runs-on: ubuntu-latest + steps: + - name: Do the second thing + run: echo second diff --git a/testdata/contexts/everything-agrees/absences b/testdata/contexts/everything-agrees/absences new file mode 100644 index 0000000..e69de29 diff --git a/testdata/contexts/everything-agrees/expected-refusals b/testdata/contexts/everything-agrees/expected-refusals new file mode 100644 index 0000000..e69de29 diff --git a/testdata/contexts/everything-agrees/required b/testdata/contexts/everything-agrees/required new file mode 100644 index 0000000..f8407c5 --- /dev/null +++ b/testdata/contexts/everything-agrees/required @@ -0,0 +1,2 @@ +the first check +the second check diff --git a/testdata/contexts/everything-agrees/workflows/two-jobs.yml b/testdata/contexts/everything-agrees/workflows/two-jobs.yml new file mode 100644 index 0000000..49fda54 --- /dev/null +++ b/testdata/contexts/everything-agrees/workflows/two-jobs.yml @@ -0,0 +1,20 @@ +name: Two jobs + +on: + pull_request: + +permissions: {} + +jobs: + first: + name: the first check + runs-on: ubuntu-latest + steps: + - name: Do the first thing + run: echo first + second: + name: the second check + runs-on: ubuntu-latest + steps: + - name: Do the second thing + run: echo second diff --git a/testdata/contexts/the-set-is-empty-and-every-name-is-written-down/absences b/testdata/contexts/the-set-is-empty-and-every-name-is-written-down/absences new file mode 100644 index 0000000..f8407c5 --- /dev/null +++ b/testdata/contexts/the-set-is-empty-and-every-name-is-written-down/absences @@ -0,0 +1,2 @@ +the first check +the second check diff --git a/testdata/contexts/the-set-is-empty-and-every-name-is-written-down/expected-refusals b/testdata/contexts/the-set-is-empty-and-every-name-is-written-down/expected-refusals new file mode 100644 index 0000000..e69de29 diff --git a/testdata/contexts/the-set-is-empty-and-every-name-is-written-down/required b/testdata/contexts/the-set-is-empty-and-every-name-is-written-down/required new file mode 100644 index 0000000..e69de29 diff --git a/testdata/contexts/the-set-is-empty-and-every-name-is-written-down/workflows/two-jobs.yml b/testdata/contexts/the-set-is-empty-and-every-name-is-written-down/workflows/two-jobs.yml new file mode 100644 index 0000000..49fda54 --- /dev/null +++ b/testdata/contexts/the-set-is-empty-and-every-name-is-written-down/workflows/two-jobs.yml @@ -0,0 +1,20 @@ +name: Two jobs + +on: + pull_request: + +permissions: {} + +jobs: + first: + name: the first check + runs-on: ubuntu-latest + steps: + - name: Do the first thing + run: echo first + second: + name: the second check + runs-on: ubuntu-latest + steps: + - name: Do the second thing + run: echo second