Harden the Binjovi cluster surface #20
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/binjovi-platform-hardening"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Five findings from the Binjovi adversarial review, cluster side. Each is a separate commit;
bash tests/check.shpasses.1. The image-digest policy only constrained images already named like ours. The
binjovi-image-onlymatchCondition meant a pod in nsbinjovirunningalpine:latestwas never matched, and even in a matched pod an added sidecar on a floating tag passed. The namespace is now the only selector: inbinjoviandbinjovi-stagingevery container, init container and ephemeral container must carry@sha256:. Scope widened to CREATE+UPDATE on pods plus UPDATE onpods/ephemeralcontainers.failurePolicy: Failkept.Verified against the live cluster: server-side dry-run applies cleanly, and all five running pods across both namespaces are digest-pinned, so the stricter policy admits exactly what is there today. Container and EphemeralContainer are different schema types, so the lists are never concatenated — a CEL runtime error under
failurePolicy: Failwould deny every pod operation in those namespaces. Not verified: no live admission test. Post-deploy smoke:kubectl debuga pod in nsbinjoviwith a tag image (must deny) and a digest image (must admit).2. The webhook signing secret was readable from every namespace. The Role was bound to
forgejo-creds-reader, the identity behind ClusterSecretStoreforgejo-credentials— which carries noconditionsblock, and in External Secrets that admits every namespace. A separate Role contains nothing while the identity stays shared. The signing value now has its own reader ServiceAccount and its own store scoped to nsbinjovi, shaped like the other three Binjovi stores.⚠️ Needs the matching binjovi-repo change (opened alongside this):
manifests/credentials.yamlmust name storebinjovi-webhookinstead offorgejo-credentials. They cannot land atomically. Between them that one ExternalSecret reports NotReady — not an outage, because its target hasdeletionPolicy: Retainand the value is static, so webhook verification keeps working. I chose the brief transient over a dual-grant window, because the alternative meant weakening a brand-new guard.forgejo-credentialsitself is untouched, as its blast radius is a separate decision. Its real consumers:flux-system(forgejo-git-auth),shipwright-build(forgejo-buildkit-secret,forgejo-clone-secret),workflowsandworkflows-alice(forgejo-admin-token,forgejo-pipeline-bot-token),pipelines-operator-system(forgejo-pipeline-bot-token).workflows-alicelooks like tenant-collapse residue.3. A comment described its blast radius backwards. The bootstrap Job's comment called the Forgejo database credential one of "two narrow powers" and contrasted it favourably with an administrator token. It is
PGUSER=forgejo— full read/write on the whole Forgejo database, and stronger than a site-administrator API token, since auser-table write can setis_admin, replace a hash, or mint a session. Comment only; the Job is byte-identical.4. Two unused Argo destinations removed. Nothing on either deploy branch targets
shipwright-buildorbinjovi-probes. Because the Argo controller is cluster-admin, RBAC escalation prevention does not apply, so that reach was free privilege into the privileged BuildKit namespace.5. The MinIO lifecycle rule was not idempotent.
mc ilm rule addhas no--idflag — confirmed in the pinned release's help — and three calls left three identical rules with three different IDs. Replaced withmc ilm rule importfrom a fixed-ID JSON. Verified by extracting the rendered Job script withkubectl kustomizeand running it three times against a throwaway MinIO seeded with duplicates: exactly one rule each time. So the first live run also self-heals whatever has already accumulated.Guards assert properties now, not the old strings, and were mutation-tested: 17 deliberate regressions, all 17 go red. That surfaced a real gap —
grep -Fq 'failurePolicy: Fail'passed a weakened policy because the file's own header names the field in prose; it is now anchored with a regex, and two checks strip comments before grepping.The Job comment claimed idempotent lifecycle convergence, but the command was `mc ilm rule add`. That form has no --id flag, so it mints a new random rule ID and APPENDS a duplicate on each call. With backoffLimit 20 and restartPolicy OnFailure, and with the verify steps that can fail after it, one retry loop could write up to 21 copies of the same rule. `mc ilm rule import` replaces the WHOLE lifecycle configuration from STDIN, so the result is one rule with a fixed ID however many times the Job runs. It also removes duplicates that earlier runs already left on the bucket. Measured against the pinned mc release, with a throwaway MinIO: - three `rule add` calls left three identical rules with three IDs; - the rendered Job script, run three times over those three duplicates, left exactly one rule: ID binjovi-logs-v1-expire-30d, prefix v1/, 30 days. `import` was checked in that release's help output before use, and the mc image has no grep, so the script uses none. The guard asserted the `rule add` string. It now asserts convergence.