Repository navigation
ci: pass the organization Go workflow across the repository - #5853
Merged
Merged
Conversation
…workflow Betterleaks, Secretlint, HTMLHint and djLint failed the organization's required Go workflow on test fixtures, not on secret material: - build the Slack-webhook fixture in two pieces, as the test already does for its other secret-shaped fixtures - mark the base64 fixture credentials inline with betterleaks:allow - move the S3 stub's refusal-only credential into a variable whose line carries a Secretlint disable for its basic-auth rule - add a repository Betterleaks config with three allowlists, each bound to one rule and one non-secret shape: numeric docker uid:gid, the mail tests' `fixture:` account, and go.sum `h1:` lines - exclude the byte-exact Hubble UI fixture, whose test requires it unaltered, from the two HTML linters only Part of #5852 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The organization Go workflow runs `go test ./...` and `go test -race -coverprofile=... ./...` with Go's defaults, unlike platform CI's `-parallel 8 -timeout 20m`. refresh-flux-ghcr-auth spends much of each case waiting on the helper's shell and fake-command processes, so the default one slot per CPU took 592s on a four-CPU runner and its last cases ran past the command deadlines derived from Go's ten-minute package budget. TestMain now raises only that default to the eight slots CI already uses; an explicit -parallel is kept as given. TestObservationCommandRefusesForeignDispatchBeforeAccess replaced the child's environment, dropping the GOCOVERDIR a coverage run exports, so the instrumented child's exit warning broke the exact-output check. It is now forwarded, which also records the child's coverage. Part of #5852 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The organization Go workflow lints the whole root module with golangci-lint's default configuration, runs `deadcode -test ./...`, and runs revive over every Go file through MegaLinter. golangci-lint (61 findings, 27 shown under its default per-text cap): - errcheck: handle or explicitly discard Close and write errors. A failed verdict write in analyze-cri-logs and observe-crossplane-recovery now refuses instead of reporting success; validate-homepage-bookmarks documents that its buffered writer's Flush decides; tests check json.Unmarshal. - staticcheck: lowercase two error strings, name the shared sentinel errRefused, and apply the De Morgan and tagged-switch rewrites that `--fix` would otherwise leave as uncommitted changes. - unused/deadcode: remove the unreachable containsLine and number. revive: rename unused parameters to _, stop shadowing builtins, put context.Context first, document the exported API of the internal packages, and give each package that had none a package comment. Part of #5852 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ments MegaLinter passes revive a list of files, so revive asks for a package comment in every file of a package that already has one: 166 findings. Keep every default rule and turn off only package-comments, as the maintainer decided on #5852. Every package already carries its own package comment. Part of #5852 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
GitHub's detected language decides whether the organization's required Go workflow applies, and Go and Shell are within a few kilobytes of each other here. Record that the repository keeps passing it either way. Part of #5852 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
✅
|
Code coverage overviewLanguages: Go Go / code-coverage/go
Line coverage data for the main branch is not yet available. View coverage of the 10 most covered files.
ℹ️ Calculations represent line coverage. Results may change as calculations are updated. Updated |
Betterleaks 1.7 attaches global [[allowlists]] with targetRules only in the top-level config. MegaLinter's generated config extends this file, so the targeted allowlists were dropped and their six findings stayed. Rule-level allowlists merge into the default rules on extend. Part of #5852 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MegaLinter's grype, osv-scanner and trivy scans flag Go module versions with published advisories across the repository's Go modules. Bump each to its fixed release in every module that pins it: - golang.org/x/net to v0.62.0 (GO-2026-6603/6610/6611/6612/6617/ 6860/6866), which raises golang.org/x/text to v0.43.0 (GO-2026-6629) through minimal version selection. - golang.org/x/crypto to v0.58.1-0.20261009181718-2f1f834330b2, the fixed version GO-2026-5932 names (the commit that removes the unmaintained openpgp package; no tagged release carries it yet). Nothing here imports openpgp, so every module still builds. - github.com/google/cel-go to v0.30.0 (GO-2026-6094 / GHSA-gcjh-h69q-9w9g) in the three native admission proofs. The audited controller renderer keeps Helm v4.2.4 and Go 1.26.8, so the build guard's SDK pin and build metadata are unchanged. Part of #5852 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…dules osv-scanner reports two sets that are not defects to bump away: - The govulncheck negative-control fixture pins Go 1.24.0 on purpose, so its standard-library advisories are the point of the fixture. Its own osv-scanner.toml ignores only the stdlib package there; a dependency advisory added to that module is still reported. - Kyverno v1.19.2 in the offline admission driver carries the three Go records the module's .govulncheck-allow.txt already adjudicates as database-range errors (platform#5154). The module's osv-scanner.toml ignores exactly those IDs with the upstream ranges, and TestAdvisoryAcceptancePinsReviewedRelease now requires the two lists to name the same advisories and keeps fencing the release. osv-scanner reads each file only for the go.mod beside it, so no other module's findings are affected. Part of #5852 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…kerfiles - Run the govulncheck findings pipelines in the two Kyverno builders under bash with pipefail (hadolint DL4006), so any failed stage fails the build rather than only the last one. - Pin liblz4-dev to bookworm's only build, 1.9.4-1 (DL3008). - Restate each upstream runtime image's own non-root identity with an explicit USER (65532 for the ko-built Kyverno controllers, 65534:65534 for the Coroot server and cluster agent), read from the pinned digests' configs, so runtime behaviour is unchanged (Checkov CKV_DOCKER_3, Trivy DS-0002). - Declare HEALTHCHECK NONE, as the private-mail image already does: Kubernetes never runs an image health check (Trivy DS-0026). Scoped dispositions, each next to what it covers: - MigrationRuntime: `hadolint ignore=DL3059` on its second exec-form check, because the ko-built runtime image has no shell to share one RUN. - CorootNodeAgent: `checkov:skip=CKV_DOCKER_3`; the eBPF node agent keeps the upstream image's root identity by design (the matching Trivy DS-0002 entry is in .trivyignore.yaml). - private-mail-config: `checkov:skip=CKV_DOCKER_4` on the one ADD that fetches a checksum-pinned URL, which COPY cannot do. Part of #5852 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Fixes: - recover-vault-snapshot becomes a JSON patch with its target in the Hetzner kustomization, following keep-vault-snapshot-on-baseline- workers: a partial Job is scanned as a workload with no containers (Checkov CKV_K8S_23/29/31/38/40, Trivy KSV-0118). The rendered Hetzner infrastructure layer is byte-identical. - The coroot-dns-probe admission fixture states a restricted pod baseline (15 Trivy findings); the ndots mutation it exercises reads only labels, namespace and dnsConfig, and still applies. Path- or resource-scoped dispositions, each with its reason: - KSV-0040 (579, LOW): skipped like KSV-0039. Trivy evaluates the namespace ResourceQuota check per file, so every non-ResourceQuota file fails; the repository's one ResourceQuota passes. - private-mail: uid/gid 2000 is the stalwart identity baked into the pinned Stalwart digest (KSV-0020/0021, CKV_K8S_40), NET_BIND_SERVICE is needed for the binary's file capability (KSV-0022, CKV_K8S_25), recovery mode takes its admin from an env var (CKV_K8S_35), and the two overlay fragments get their image digest and pod securityContext from the kustomization (KSV-0013/0118, CKV_K8S_14/15/29/43). Resource-scoped checkov.io/skipN annotations are numbered across the base and both patches so the merged StatefulSet keeps every reason; only that object's metadata changes in the rendered output. - KSV-0036 on the three pods that disable automounting and project a token only into the container that authenticates. - KSV-0037 on kube-system's own LimitRange, now under bootstrap. - KSV-0048/0049/0053 on five namespaced single-rule Roles (Longhorn Kyverno retrofits, the orphan-alert state record, the owner-only World at Ruin trial). - KSV-01010/0109 on the Homepage layout and the generated security-progress ConfigMap. - DS-0002 on the root eBPF Coroot node agent. Guards: a new test-trivyignore-private-mail-identity-boundary.sh asserts the identity per container, the measured digest and the bootstrap USER, and keeps an identical probe reported (wired into CI). The RBAC test asserts the five new roles' shape and the trial role's sole owner binding; the ConfigMap test pins both new files' matched key sets and clears absolute file paths from its opaque-value premise; the reciprocal path lists in the vendored, vault-config and vault-backup tests name the new reviewed pairs. Part of #5852 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…Go workflow The org validate-go-project workflow runs MegaLinter v10.1.0 over the whole repository. This clears its yamllint, v8r, zizmor, checkov (CKV_GHA_*) and shellcheck findings without changing what any workflow, patch or fixture does. - yamllint (293 in CI, 304 locally before prettier's in-place fixes): add the `---` document marker to 223 files and quote the workflow trigger key as `"on":` in 63 workflows. Every changed YAML file except two parses to identical data. Re-indent three comments that sat outside their content, space the flow sequences in one fixture, drop a trailing blank line, and wrap four lines over 500 characters: the deploy-prod readiness error is now one echo with several arguments (same single-line output), and the CEL in scripts/tests/crossplane-egress-policy-rules.yaml is re-wrapped between tokens (identical parsed AST). The one test that read the trigger block as text (`/^on:/` in test-probe-cilium-externalauth-crossnode.sh) now accepts the quoted key and fails on an empty block, where its no-other-trigger check would otherwise have passed on nothing. - talos/factory-schematic.yaml keeps no marker: its sha256 is the Image Factory schematic ID registered before deploy and read back from nodes. A repository .yamllint.yml (MegaLinter's template plus one path-scoped document-start ignore) records that, and .prettierignore stops MegaLinter's APPLY_FIXES from re-indenting it into a different ID. - v8r: rename the CNPG fixture off `cluster.yaml`, which SchemaStore maps to the RKE cluster schema; the test still copies it in as cluster.yaml. - zizmor: MegaLinter's zizmor 1.25.0 flags `owner` on five app-token steps that already set `repositories`, because it looks for a singular `repository` input (zizmorcore/zizmor#2011, fixed in 1.25.1). Each `owner` line carries an inline github-app ignore with that reason; no token scope, permission or input changes. - checkov CKV_GHA_7: three dispatch-only workflows build nothing; their inputs are a typed interlock, an approval binding and a dry-run default. Each gets a file-scoped checkov skip with that reason, as dr-rebuild.yaml already has. - shellcheck SC2153 in bootstrap-tenant-source.sh was a symptom: shfmt 3.13.1 treats a U+0080 character as end of input and truncated generate-publish-workflow-approved-revisions.sh (630 -> 2 lines) before shellcheck read its sourcer. Its comments held double-encoded mojibake; they are restored to the intended em dashes, arrows and rocket emoji. Part of #5852 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
Review note: what was fixed, and every exceptionEverything below was fixed at the cause unless it is listed as an exception. Fixed
Exceptions (each scoped to one file, rule or shape, with the reason beside it)
Known leftovers outside this PR
🤖 Generated with Claude Code |
This was referenced Nov 3, 2026
….sum Grype reads go.sum as well as go.mod, so modules a build never selects still count. render-helm-chart now requires the fixed grpc, klauspost/compress, go-jose, distribution, edwards25519 and OpenTelemetry releases; three test modules require klauspost/compress v1.19.1. go mod tidy keeps the requirements and is idempotent, and helm still builds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Betterleaks reports a finding's line from the preceding newline, so the anchored go.sum allowlist never matched the two quoted lines in the Kyverno background patch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A parallel fork can hold a freshly written mutant open for writing, so executing it directly failed with "text file busy" on Linux. Both mutation tests now let Bash read the file, as the shared test helper already does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The organization Go workflow lints the whole repository with APPLY_FIXES, so a formatter there rewrites files whose bytes are the point: the frozen EKS approval history (sha256-pinned by its test), the jq-written RBAC baselines (compared with diff -u), the upstream flagd schemas (sha256 recorded in the README) and the captured Stalwart store output. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The organization Go workflow's MegaLinter run formats every file, then commits the result with the workflow token. That token cannot change workflow files, so the commit was refused. These are the same fixes: shfmt -i 2 -ci on shell scripts, prettier on JSON and YAML outside k8s/, and markdownlint. The Coroot security-progress ConfigMap is regenerated because it packages the reformatted healthcheck.sh. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A wrapped line that began with an issue reference such as `#5247` read to markdownlint as a heading missing its space. Its fixes made it a real heading, then stripped the sentence's trailing punctuation. The original wording is back, wrapped so no line starts with the reference. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 tasks
devantler
marked this pull request as ready for review
November 4, 2026 04:26
Contributor
Author
|
@coderabbitai review The review note above lists every fix and each scoped exception, with its reason. 🤖 Generated with Claude Code |
Contributor
Action not completed
|
3 tasks
- tenant-webapp-admission: accept only reviewed osv-scanner.toml line shapes, so a PackageOverrides table, a single-quoted id or a trailing comment fails the test instead of widening the ignore unseen. - private-mail identity guard: require every container's image, not only the main one, and refuse an empty container read. - first-party RBAC guard: read the operator Role's bindings from the rendered production layers rather than one directory. - private-mail-config: a Go test refuses any Dockerfile ADD without a pinned sha256 checksum, because Checkov's CKV_DOCKER_4 skip covers the whole file. - vault-backup-recovery: the production recovery patch must keep every base pod-template annotation, because its JSON Patch add replaces the map. - go.mod: say beside the x/crypto pseudo-version why it is pinned and when to move off it. - Comments: state the real reason for the diagnose-email-startup prettier exclusion, and that the trivy layer of the boundary guards does not run in CI yet (#5864). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
devantler
commented
Nov 4, 2026
devantler
left a comment
devantler
left a comment
Contributor
Author
There was a problem hiding this comment.
Local review round: clean at d328b724fe7ea3de0a17811733828a916e6917d2
Why this was a local review. No external review provider could review this pull request when I checked:
| Provider | Evidence | Scope | Recovery |
|---|---|---|---|
| CodeRabbit | "Review skipped: 544 files exceed the limit of 100" on this PR (comment 6622557535, 2026-11-04 04:28 UTC) | this PR, because of its size | none for this PR |
| Codex | "You have reached your Codex usage limits for code reviews" on devantler-tech/ksail#8184 (01:57 UTC) | the whole account | the maintainer adds credits, or the limit resets |
| Cursor Bugbot | "Bugbot couldn't run - usage limit reached" on devantler-tech/ksail#8179 (00:29 UTC) | the whole account | an admin raises the Cursor limit |
Method. The diff against main touches 553 files.
- 398 files were proven formatting-only by machine.
- YAML and JSON parse to identical data before and after.
- Every shell script's new text is exactly
shfmt -i 2 -ciof its old text, and the old and new text are identical once whitespace,;and\are removed. - Every changed script passes
bash -n.
- The other 155 files went to three independent reviewers. Each was given the code and the repository's review bar, not my conclusions:
- the 89 Go files;
- the production manifests, Dockerfiles,
ci.yamland the deploy action, including an offlinekubectl kustomizerender of every layer onmainand on this branch; - the scanner configuration and exceptions, Go modules, tests and docs.
- The reviewers built and vetted every affected package, ran the affected tests, and ran mutations against the guard tests.
Production effect. Every rendered layer is byte-identical to main, except:
- StatefulSet
private-mail/private-mailgains 13 Checkov skip annotations in its own metadata. The pod template is unchanged, so no pod restarts. - ConfigMap
observability/coroot-security-progresschanges only in whitespace inside two embedded scripts. Reloader will restart that Deployment once.
Findings. None critical or major. Ten were minor; seven are fixed in d328b724f:
- The OSV advisory list test accepts only reviewed line shapes. Before, a
[[PackageOverrides]]table, a single-quoted id or a trailing comment could widen what osv-scanner ignores without failing the test. - The private-mail identity guard checks every container's image, not only the main container's.
- The World at Ruin operator binding is read from the rendered production layers instead of one directory.
- A Go test refuses any
ADDin the private-mail image without a pinned SHA-256 checksum. Checkov'sCKV_DOCKER_4skip covers the whole Dockerfile, so it can't do this. - A Go test makes sure the production recovery patch keeps every base pod-template annotation. Its JSON Patch
addreplaces the whole map. - The
x/cryptopseudo-version now says beside it why it is pinned and when to move off it. - The comment on the diagnose-email-startup prettier exclusion now gives the real reason: its tests edit the compact JSON by exact substring.
Accepted or tracked:
- The boundary guards' trivy layer doesn't run in CI, because the
validatejob doesn't install trivy. All six guards were built that way before this PR. This is now tracked in #5864, and the comments say so. - The Coroot builder stage pins
liblz4-dev=1.9.4-1to satisfy hadolint. If Debian replaces that version, the image build fails visibly. It affects the build stage only, and the pin keeps builds reproducible. - The "no Secret API grant" sentence in the World at Ruin disposition is not asserted by a test. The test does assert the binding's single subject and the Role's single rule.
Separate known issue. The coverage job can still run refresh-flux-ghcr-auth out of time on a busy runner (#5862, with measurements). The organization workflow skips its test jobs in the merge queue, so this costs a re-run on pull requests, not a queue eviction.
Verdict: clean at d328b724fe7ea3de0a17811733828a916e6917d2.
🤖 Generated with Claude Code
This was referenced Nov 4, 2026
devantler
added a commit
that referenced
this pull request
Nov 4, 2026
…tire-cri-log-analyzer-4350 Keeps main's updated Validation paragraph and the retirement's package example (validate-eks-ci-role-policy), and keeps scripts/analyze-cri-logs deleted although main lint-fixed it in #5853. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
Why
Since GitHub started counting this repository as a Go project, the organization requires its Go workflow on every pull request, and that workflow fails here. No pull request can merge until it passes. The maintainer chose to adopt the workflow rather than opt out of it.
What
Makes the whole repository pass the organization Go workflow: the repository-wide lint backlog, the Go lint and dead-code findings, vulnerable Go modules, and the tests that only failed on the organization's runner. Findings are fixed at their cause; the few genuine false positives get narrowly scoped, explained exceptions. The review note on this pull request lists every fix and every exception.
Part of #5852. The issue stays open for its second condition: the workflow must keep running here even if GitHub detects the repository as Shell again.
Known risk, tracked separately: the coverage job can still run the GHCR-auth refresh tests out of time on a busy runner (#5862). The organization workflow skips its tests in the merge queue, so the cost is re-running that job on a pull request.
👉 After merge: open pull requests here need to update from
mainto pick up the fixes before their required checks can pass.🤖 Generated with Claude Code