feat(ci): adopt cluster-template pre-commit hooks, scope CI tokens - #4381
Closed
Tanguille wants to merge 19 commits into
Closed
feat(ci): adopt cluster-template pre-commit hooks, scope CI tokens#4381Tanguille wants to merge 19 commits into
Tanguille wants to merge 19 commits into
Conversation
…rceSets Thirty objects across 42 files, differing only in name, IP, port and homepage annotations. Adding a service is now one input entry. flux-operator build resourceset renders all 30 objects identically to the previous kustomize output, apart from the resourceset.fluxcd.controlplane.io ownership labels and three targetPort: 443 lines that restate spec.ports.port.
The same four-line dnsConfig sat in five HelmReleases, documented in one. The two reasons it is there (three NXDOMAIN round-trips per in-cluster FQDN, and an external name being hijacked by a same-named local Service) now live once. Merged value trees are identical for all five apps.
HelmChart, Alert, Provider and Receiver emitted gotk_resource_info that no alert and no dashboard reads: flux-instance/prometheusrule.yaml queries only HelmRelease and Kustomization, and the two pinned upstream dashboards query only Kustomization, HelmRelease, GitRepository, HelmRepository, Bucket and OCIRepository. Their RBAC grants go with them. The seven surviving kinds share one name/namespace/ready/suspend shape, so anchors leave each block showing only what differs. 422 lines to 213, rendered output unchanged.
…ches The observer proxy read whole bodies into memory before writing a byte. /observer/shares?limit=10000 measures 9.2MB, fetched every 10s by every open tab, buffered in a 256Mi container. It now streams with copyfileobj and asks for gzip, which the same request serves in 3.0MB. Also: one send_json and one proxy helper replace seven copies of the response boilerplate, ThreadingHTTPServer replaces ThreadingTCPServer plus a manual allow_reuse_address, and the monerod get_info request is built once. Frontend: initialize() refetched /stats_log.json and rebuilt both charts microseconds before updateStats() did the same; chart options and their four closures were rebuilt every tick and thrown away on the update path; pool_info joins the existing Promise.all instead of costing its own round trip.
The pull job refetched a 96.6MiB talosctl on every container PR, roughly 400 a month over the home uplink, to run a tool that then pulls images. The runner image ships yq but no talosctl, so the download itself is needed. An RWX PVC at /tool-cache is shared by every runner pod, keyed by Talos version, so it is one download per version cluster-wide. fsGroup 1001 because the runner is uid/gid 1001 and the CephFS mount is otherwise root-owned.
Eleven sites across ten charts. Each verified by pulling the pinned chart and diffing helm template before and after: rendered output is identical everywhere (crowdsec and vmstack differ only in per-run generated certs). Also collapses five repeated gatus endpoint alert blocks onto alerting.discord.default-alert; enabled: true already restates the gatus default. Parsed alert state is unchanged for all five endpoints.
…ntics Four near-identical strategic-merge patches setting one memory limit each become two regex-targeted ones, matching the idiom already used twice in the same file. The cluster patch carried a substitute-from opt-out label with no users anywhere in the repo. It was a one-word kill switch for sops decryption and secret substitution across a whole app tree, so it is gone; the note about the patch replacing substituteFrom wholesale stays. privileged-movers used a JSON6902 add into /metadata/annotations, which only resolved because common/namespace.yaml happens to emit an annotations map. A strategic merge creates the map when absent and drops the ~1 escaping.
opencode Service and EndpointSlice: type ClusterIP, protocol TCP and a targetPort equal to port. gitleaks: job permissions identical to the workflow-level block. nextcloud: the fromAddressKey workaround cited chart 9.2.0 while the pin is 9.2.5, rechecked and still required. xmrig resourceset: a sub-comment claiming fast scale-up above a 60s window, inside a block its own header calls inert. controller.py: a `not samples` guard unreachable because max() over an empty set already raised, and a timeout kwarg restating the signature default.
The mount does not exist until the runner HelmRelease in this PR rolls out, so the job would fail on its own CI run, and it disappears if CephFS is down. Falls back to a per-run download in both cases.
DeepSource JS-0116: updateWindowLuck lost its only await when the pool_info fetch moved into the batch, so it no longer needs to be async. DeepSource JS-R1004: /observer/pool_info has no interpolation. The talosctl cache is a shared writable mount, so its contents are untrusted: every read is now checked against the release sha256 and copied to job-local storage before going on PATH, a poisoned or truncated entry falls through to a plain download, and the temporary file uses mktemp rather than $$, which is not unique across pods sharing the mount. Superseded versions are pruned, which also bounds growth on the 2Gi share.
Checking the cache entry and then reading it left a window where another job could swap the file in between, so the local copy reached PATH unverified. Copying first and hashing what was actually copied closes it, and drops a verify call.
40 added comment lines down to 25; no comment loses a measured fact.
Two cache-miss jobs pruning the same stale entry make one find call exit non-zero, which under set -e fails the step. Reproduced at 11/12 rounds with two concurrent pruners; 0/12 with -ignore_readdir_race.
…ce PSA baseline cluster-runner mounts a Talos os:admin credential into every pod it schedules, and agent-pr-review feeds untrusted PR text to a tool-calling model on that pod. Split the scale set so only flate's image-pull job keeps the credential. - add review-runner scale set: no talos volume, no talos.dev ServiceAccount - cluster-runner drops to minRunners 0 / maxRunners 2 (flate fires on kubernetes/** only) - opencode: automountServiceAccountToken false, no SA/Role/ClusterRole exists for it - opencode: pin the three npm plugins, they execute in-process in an agent pod - add psa-baseline component, enforce on 9 namespaces verified to have zero violations
…xists Flux creates review-runner only after this merges, so pointing the workflow at it in the same PR leaves the job queued with no runner. Switch follows in a one-line PR.
agent-pr-review still runs on cluster-runner until the follow-up PR moves it, so dropping to 0/2 now would cut review capacity from 5 to 2 and cold-start every review.
- cap review-runner at 0/3 so combined ceiling stays at today's 5 concurrent runners - drop containerMode: no workflow declares `container:` and REQUIRE_JOB_CONTAINER is false, so the k8s hooks never fired; _work now uses emptyDir, not a 25Gi PVC per pod - add opencode customManager so the pinned npm plugin versions do not rot - retarget a topologySpread comment copied verbatim from cluster-runner
… default Completes the runner split and inverts the PodSecurity default so new namespaces land enforced rather than silently unprotected. - agent-pr-review runs on review-runner; cluster-runner keeps os:admin for flate only - cluster-runner 0/2, review-runner 1/3: ceiling stays at 5 concurrent runners - components/common sets enforce: baseline; privileged workloads override to privileged - privileged-movers carries its own override, colocating the exception with its cause - psa-baseline (opt-in) replaced by psa-privileged (opt-out) for the 4 infra namespaces Verified: all 16 namespaces render the intended enforce level, and kubectl apply --dry-run=server reports no PodSecurity violations on the 9 enforced namespaces.
Replaces the proposed shell-lint workflow with the lefthook setup cluster-template ships (oxfmt, mise fmt/lock, zizmor), plus the shellcheck command from home-operations/.github lefthook.common.toml. The config is inlined rather than pulled from the org remote so no unpinned external config runs at commit time. zizmor flagged two live issues, both fixed here: the agent PR review checkout left a usable token in .git/config while checking out untrusted PR code, and the labeler app token inherited every permission its installation holds. ks.yaml moves its comment above the list item because oxfmt rewrites the inline form with a trailing space, which .editorconfig forbids.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| JavaScript |  |
Aug 6, 2026 7:30p.m. | Review ↗ |
| Shell | |
Aug 6, 2026 7:30p.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
This was referenced Aug 6, 2026
Owner
Author
|
Superseded by #4379, which carries a newer variant of the same work committed directly on This branch's actual git base is Nothing is lost. Compared against #4379 the only unique content here is a |
An error occurred while trying to automatically change base from
fix/security-audit-runner-split
to
main
August 8, 2026 14:38
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
1 participant
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.
Third layer of the stack: #4378 (simplify) -> #4379 (security) -> this.
Adopts the lefthook pre-commit setup cluster-template ships, plus the shellcheck command from
home-operations/.githublefthook.common.toml. Config is inlined rather than pulled from the org remote, so no unpinned external config executes at commit time.Hooks
*.just,.justfilejust --fmt.mise.tomlmise fmt*.json,*.json5,*.jsoncoxfmt*.md,*.mdxoxfmt*.yaml,*.ymloxfmt.mise.tomlmise lock, stagesmise.lock*.shshellcheck.github/workflows/*.yamlzizmor --offlineskip = ["merge", "rebase"]keeps the hooks off conflict resolution.misepostinstall = "lefthook install"wires it up on clone.zizmor findings fixed
agent-pr-review.yaml.git/configpersist-credentials: falselabeler.yamlpermission-contents: read,permission-pull-requests: writeagent-pr-reviewis the same workflow #4379 moves off the Talos-credentialed runner. Removing the persisted token closes the other half of that exposure: a prompt injection in PR text can no longer reach a repo-write token from the checkout.Other
kubernetes/flux/cluster/ks.yamlmoves its comment above the list item becauseoxfmtrewrites the inline form with a trailing space, which.editorconfigforbids. No behaviour change.Supersedes
#4377 proposed a
shell-lint.yamlCI workflow for the same goal. Running shellcheck at pre-commit instead means no CI job and no runner minutes, so #4377 should close unmerged if this lands.