Governance: watch the supply chain, and gate what prose and prod share - #9
Merged
Conversation
The repository had one workflow and no view of its own dependencies. That
sounds wrong for a module with an empty require block, and it is: go.mod is
empty and TestNoExternalDependencies keeps it that way, but ci.yml runs
actions/checkout and actions/setup-go with the repository token, against
mutable tags nobody was watching. Those actions are the entire external
supply chain here. They are now pinned to commit SHAs, and dependabot exists
to move the pins — a pin with no way to move is a pin that rots. No gomod
entry: there is nothing for one to watch, and the test is already the judge.
SECURITY.md said private vulnerability reporting was not enabled and sent
reporters to email. The API says {"enabled":true}. A security policy naming
the wrong channel is worse than none, so the button is now the documented
route and email stays as the one that needs no GitHub account.
The two fuzz targets in segment_test.go had never been fuzzed. `go test`
replays the seed corpus and stops; the engine never started. SECURITY.md
names the segment decoder as the first place a hostile file lands, so `make
fuzz` now runs it and CI calls that. 6.6M executions locally, no crash — but
that is a measurement, not a proof, which is why it runs on every push.
golangci-lint found 56 things. Three were real and are fixed here: Commit
created its index directory 0o755 when nothing weft does needs another user
to read the caller's corpus; a local named `text` shadowed the scorer package
of that name; a test helper used exec.Command where the context was available.
The other 53 were all places the code had already written down why it does
what it does — prune and syncDir dropping errors on purpose, and eight uint64
conversions that gosec flags precisely where segReader.intn and str bound them
first. Those became nolint directives and scoped exclusions, each naming the
sentence that justifies it, so the deliberateness is machine-visible instead
of only stated in prose.
markdownlint found 331, of which 179 were in gitignored scratch files the tool
had no business reading. The rest are fixed in the source rather than by
loosening rules: table delimiter rows normalised, code fences given languages,
bullets and emphasis made consistent, two bare emails wrapped. FINDINGS.md
keeps its two top-level headings and says at that line why — it is an
append-only log of milestone reports, and folding them under one title would
file a later milestone under a conclusion it did not reach.
SPDX headers went on every .go file, which broke the 100-line scorer budget by
exactly one line. countGoLines now excludes the header: that metric asks what a
scorer costs to build, and charging each scorer for a repository-wide licensing
decision measures the wrong thing. recency still counts 99.
Release, docs and governance scaffolding, all previously declined and now
reconsidered:
- changie owns the changelog. The old CHANGELOG's rule — only three things
can force work on a caller — survives as changes/header.tpl.md and as the
first three kinds, so entries are written when the change is made instead
of reconstructed from a diff at tag time.
- goreleaser ships cmd/weft with checksums, SBOMs and provenance. It is a
demo, and .goreleaser.yaml and the release footer both say so; `go get`
still needs nothing from that page.
- The Hugo site mounts /docs rather than copying it, so README's relative
links keep working and there is one copy of every document.
- GOVERNANCE.md documents what is decided by a test rather than by the
maintainer. RELEASE.md takes the release procedure out of CONTRIBUTING so
there is one copy of it. SUPPORT.md says where each kind of question goes.
- stale, labeler, CODEOWNERS, editorconfig, gitattributes, pre-commit.
`make all` stays installable-free on purpose: spdx, lint, lint-docs and fuzz
each cost a tool or a minute, so they are separate targets that CI calls, and
a first-time contributor can still run the whole gate with only Go.
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
…hange The Pages job went red on its first run, and not because the site is broken. `configure-pages` asks the API for the configured Pages site, which 404s until Pages is switched on in repository settings. On a pull request that is the wrong thing to gate: the useful question there is whether the site still builds, and `hugo` answers that on its own by exiting non-zero on a template or config error. So configure-pages and the artifact upload are now confined to the deploy path, and the build runs everywhere. Also raises the pinned Hugo from 0.152.0 to 0.164.0, which is a latent failure this run did not get far enough to reach. site/hugo.yaml uses `locale`, which replaced `languageCode` in 0.158, and `renderHooks.link.useEmbedded`, which replaced `enableDefault` in 0.148 — both were written against the 0.164 the site was developed on. Under 0.152 the config would have parsed and the cross-document links between the mounted docs would have quietly 404'd, which is the failure mode worth avoiding: not a red build, a wrong site. Pages still has to be enabled by hand before anything deploys.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b78dc8e45
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two of these would have broken the first release with nothing local saying so. `.goreleaser.yaml` asks for an SBOM per archive, GoReleaser shells out to syft for that, and the workflow installed only Go and GoReleaser. `goreleaser release --snapshot` on a machine without syft fails after all six archives are built and before anything is published, which is the worst place for a release to stop. `release-check` never saw it: `goreleaser build` does not run the SBOM pipe at all, so the target written to catch a broken release pipeline at edit time is blind to this part of it. A pinned syft is now installed before GoReleaser runs. The same job held `attestations: write` and `id-token: write`, with comments describing provenance, and attested nothing. Permissions authorize; they do not act. The step that acts is now there, named against the archives rather than checksums.txt so that `gh attestation verify` works on the file someone actually downloaded. `UNRELEASED_HEADING := ## Unreleased` assigned the empty string, because Make treats the first unescaped `#` as the start of a comment. Both `changelog` and `changelog-check` were therefore passing `-u ''`: every pending fragment was dropped from CHANGELOG.md, and the check agreed with the merge by being wrong in the same way. That is precisely the failure a generated file's check exists to prevent, produced by the check itself. CHANGELOG.md now carries the four entries that were sitting unseen in changes/unreleased/. Escaping the hashes exposed the next one. With a non-empty heading changie ends the file without a trailing newline, which `end-of-file-fixer` puts back on commit and `changelog-check` would then report as a mismatch on every run afterwards. `endOfVersion: 1` settles which of the two is right rather than leaving them to disagree forever. `Commit` created its directories 0o750 and wrote 0o644 files into them, so the comment claiming that nothing weft does needs another user on the machine to read the corpus was contradicted by every other member of the caller's primary group. 0o700, and the changelog entry follows it. The markdownlint sweep that normalized table delimiters to `| --- |` lost the two-space indentation on the one table nested inside a list item. An unindented delimiter ends the list item and cannot form a table with the indented header above it, so FINDINGS rendered its correction history as pipe-delimited text. Last, two sentences describing behaviour that does not exist. stale.yml said a closed issue reopens with a comment; `remove-stale-when-updated` only takes the label off an item that is still open, and nothing in actions/stale reopens anything, so the comment now says what the closing message already said. And the issue chooser still sent anyone picking "Reporting a vulnerability" to the mailbox that SECURITY.md describes as the one route of the two that can silently drop a report. It points at the private report form now, with the mailbox left where it belongs, as the fallback for someone without an account.
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.
Pull Request
Summary
The repository had one workflow and no view of its own dependencies, and its security policy named a channel that is not the one that works.
go.modhas an empty require block andTestNoExternalDependencieskeeps it that way — butci.ymlrunsactions/checkoutandactions/setup-gowith the repository token, against mutable tags nobody was watching. Those actions are the entire external supply chain here, and nothing was looking at them.Separately,
SECURITY.mdsaid GitHub's private vulnerability reporting was not enabled and routed reporters to email.gh api repos/skyoo2003/weft/private-vulnerability-reportingreturns{"enabled":true}. A security policy pointing at the wrong channel is worse than not having one.This PR closes those, then goes on to add the tooling that was previously declined — golangci-lint, markdownlint, SPDX headers, changie, GoReleaser, a Hugo docs site, and the governance documents — after reconsidering each exclusion.
After this PR: every action is SHA-pinned and Dependabot moves the pins; CodeQL and a real fuzz run watch the surfaces
SECURITY.mdnames; the linters are gates rather than aspirations; and a release ischangie batch→ tag → automated.Changes
.github/dependabot.ymlscoped togithub-actionsonly — there is nogomodentry because there is nothing for one to watch.SECURITY.mdcorrected to the private-reporting button, with email kept for reporters without a GitHub account. Added CodeQL, andmake fuzzso the two fuzz targets insegment_test.goare actually fuzzed —go testonly ever replayed their seed corpus.Commitcreated its index directory0o755when nothing weft does needs another user on the machine to read the caller's corpus; a local namedtextshadowed the imported scorer package of that name; a test helper calledexec.Commandwhere the context was in hand..golangci.yamland.markdownlint.yamlas gates. 56 Go findings and 331 Markdown findings to zero. Every remaining exclusion names the thing that makes the finding wrong here..gofile, withcountGoLinestaught to exclude them so the scorer budget still measures what a scorer costs rather than a repository-wide licensing decision.cmd/weftwith checksums, SBOMs and provenance;release.ymlrefuses a tag with no batched notes./docsrather than copying it;GOVERNANCE.md,RELEASE.md,SUPPORT.md; stale, labeler, CODEOWNERS, editorconfig, gitattributes, pre-commit.Validation
Every gate run locally on the final tree:
make all— pass (fmt,build,vet,go test -race ./...)make lint— 56 issues → 0 (golangci-lint v2.12.2, the version CI pins)make lint-docs— 331 issues → 0 across 15 filesmake spdx— all 25.gofiles carry a headermake arch— 8 PASS / 0 FAIL,scorer/recencystill counts 99 against the 100 budgetmake changelog-check—CHANGELOG.mdmatcheschanges/make fuzz— 6.6M executions againstFuzzSegmentDecoding, no crashgoreleaser checkand a cross-compiled snapshot build — passhugo --source site— 7 pages, no warnings, andFORMAT.mdcross-references resolve to/weft/docs/format/Review Focus
pkg/engine/persist.go— the0o755→0o750change. The only behaviour change to shipped code in this PR. No test asserted the old mode and there are no releases to break, but it is a real change to whatCommitwrites.countGoLinesinarchitecture_test.go. Adding an SPDX header movedrecency.gofrom 99 to 100 and failed the budget. Excluding the header is a judgement about what that metric is for — worth disagreeing with if you read it differently..golangci.yaml. Particularly the G115 exclusion onsegment.go: the claim is thatsegReader.intnandstrbound every conversion before making it. If that claim is wrong anywhere, the exclusion hides a real bug on the hostile-input path..markdownlint.yamlrule choices.emphasis-style,strong-styleandul-styleare set toconsistentrather than a fixed character;table-column-styleis the one rule turned off outright, with the reason inline.Risks / Notes
pkg/engine/testdata/are unchanged, so nothing a caller compiles against moved.make alldeliberately still needs nothing but the Go toolchain.spdx,lint,lint-docsandfuzzeach cost a tool to install or a minute of wall clock, so they are separate targets CI calls — a first-time contributor can still run the whole gate having installed nothing.make fuzzcan go red by chance. It is 30s of random search per target; a failure means it found something real, and the recovery is to seedpkg/engine/testdata/fuzz/with the reported input. It is not a flake to retry past.golangci-lint's version is duplicated betweenci.ymland theMakefileand both say so — that is the one place they can drift.changes/header.tpl.mdand as changie's first three kinds.dependencies,github-actions,staleandarea: *that Dependabot and the labeler reference, and the repository topics, which are currently empty. Commands are in the PR thread.v0.1.0is still uncut. Its changie fragments are inchanges/unreleased/;RELEASE.mdis the procedure.