Skip to content

Commit 04e2306

Browse files
authored
Vulnerability fixes (#34)
Signed-off-by: Erick Bourgeois <erick@jeb.ca>
1 parent aebed9f commit 04e2306

15 files changed

Lines changed: 236 additions & 5 deletions

File tree

.claude/CHANGELOG.md

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,56 @@ The format is based on the regulated environment requirements:
99

1010
---
1111

12+
## [2026-04-20 00:15] - Grant `update` on `scheduledmachines/finalizers` to unblock Machine creation
13+
14+
**Author:** Erick Bourgeois
15+
16+
### Changed
17+
- `deploy/deployment/rbac/clusterrole.yaml`: New rule granting `update` on `scheduledmachines/finalizers` under the `5spot.finos.org` API group. Added as a separate rule (rather than widening the existing `scheduledmachines` / `scheduledmachines/status` rule) so the verb surface on the finalizers subresource stays exactly at the minimum the API-server admission check requires. Inline comment documents the link back to `src/reconcilers/helpers.rs` where `blockOwnerDeletion: true` is set, and quotes the exact API-server error that the missing rule produces.
18+
19+
### Why
20+
`src/reconcilers/helpers.rs:859` stamps `blockOwnerDeletion: true` on the `ownerReference` it writes from every child CAPI `Machine` back to its parent `ScheduledMachine`. Kubernetes treats `blockOwnerDeletion` as functionally equivalent to holding a finalizer on the owner, so during admission it checks that the creating service account has `update` on `<owner-resource>/finalizers` — a separate subresource in RBAC terms from the main `scheduledmachines` resource and from `scheduledmachines/status`. The controller's ClusterRole has shipped since the initial commit with full CRUD on `scheduledmachines` and `scheduledmachines/status` but without any rule for the `finalizers` subresource, so every Machine creation is rejected by the API server with `machines.cluster.x-k8s.io "<name>" is forbidden: cannot set blockOwnerDeletion if an ownerReference refers to a resource you can't set finalizers on`. This is a latent RBAC bug, not a regression. The fix grants exactly the verb the API server checks (`update`); no broader verb set is needed and granting more would violate least-privilege.
21+
22+
### Impact
23+
- [ ] Breaking change
24+
- [x] Requires cluster rollout (re-apply the ClusterRole; no controller restart required — RBAC changes are picked up on the next API request)
25+
- [ ] Config change only
26+
- [ ] Documentation only
27+
28+
### Follow-up (optional, not in this commit)
29+
- Integration test that creates a `ScheduledMachine` against a kind cluster with the shipped ClusterRole and asserts the child `Machine` is admitted. Would catch any future regression in RBAC vs. `ownerReference` shape without waiting for a prod cluster rollout to expose it.
30+
31+
---
32+
33+
## [2026-04-19 21:50] - Triage GHSA-cq8v-f236-94qc (rand soundness); widen VEX identifier shapes
34+
35+
**Author:** Erick Bourgeois
36+
37+
### Changed
38+
- `osv-scanner.toml` (new, at repo root): Ignore entry for `GHSA-cq8v-f236-94qc` with `ignoreUntil = 2026-10-19` and a reason that mirrors the VEX statement. This is the file shape Scorecard's `Vulnerabilities` check (and osv-scanner itself) expect — per the remediation text in the Scorecard finding.
39+
- `.vex/GHSA-cq8v-f236-94qc.toml` (new): OpenVEX statement with `status = not_affected`, `justification = vulnerable_code_not_in_execute_path`, explicit impact statement. Applies to both the Chainguard and Distroless product identifiers.
40+
- `tools/validate_vex.py`: Widened the accepted identifier set from `CVE-YYYY-NNNN+` only to also accept `GHSA-xxxx-xxxx-xxxx` (case-insensitive) and `RUSTSEC-YYYY-NNNN`. The TOML field is still named `cve` for backward compatibility with the existing 11 statement files. New `_is_accepted_id()` helper composes the three regexes; error message updated to list all three shapes.
41+
- `tools/assemble_openvex.py`: Dropped the `.upper()` on `doc["cve"]` when rendering `vulnerability.name`. GHSA ID segments are canonically lowercase and upper-casing them breaks round-tripping against osv.dev / github.com/advisories. CVE IDs in existing files are already uppercase, so this is a no-op for them.
42+
- `tools/tests/validate-vex-tests.sh`: Added happy-path cases `valid-ghsa` and `valid-rustsec`, plus negative cases `invalid-ghsa-format`, `invalid-rustsec-format`, and `duplicate-ghsa` (uniqueness check across case variants of a GHSA ID).
43+
- `tools/tests/fixtures/{valid-ghsa,valid-rustsec,invalid-ghsa-format,invalid-rustsec-format,duplicate-ghsa}/` (new fixture dirs).
44+
- `tools/tests/assemble-openvex-tests.sh`: Added regression guard asserting `GHSA-cq8v-f236-94qc` is emitted verbatim (not upper-cased) into `vulnerability.name`.
45+
- `.vex/README.md`: Documented the expanded identifier format.
46+
47+
### Why
48+
OpenSSF Scorecard's `Vulnerabilities` rule flagged `GHSA-cq8v-f236-94qc` (rand soundness when a custom `log` logger calls `rand::rng()` mid-reseed) on `rand 0.8.6`, pulled transitively via `warp 0.3.7``tokio-tungstenite 0.21``tungstenite 0.21` where rand is used exclusively for websocket frame masking. The vulnerable path requires the `log` logger itself to call into rand during a reseed, which 5-Spot's tracing-subscriber stack never does. The advisory ships without a CVE ID, which the existing VEX validator (`CVE-YYYY-NNNN+` only) rejected — making it the first advisory the VEX pipeline could not represent. Widening the validator to accept GHSA + RUSTSEC identifiers unblocks this triage and every future non-CVE advisory without forcing a field rename of the existing 11 files. The full fix (rand >= 0.9.3) requires crossing a warp 0.3.x semver boundary (warp 0.3 hard-pins tokio-tungstenite ^0.21) and is tracked as a follow-up, not attempted here.
49+
50+
### Impact
51+
- [ ] Breaking change
52+
- [ ] Requires cluster rollout
53+
- [x] Config change only (CI / supply-chain)
54+
- [ ] Documentation only
55+
56+
### Follow-up (optional, not in this commit)
57+
- Warp 0.3 → 0.4 or axum migration so `tokio-tungstenite` and the transitive `rand` can be bumped past the vulnerable range; remove `osv-scanner.toml` entry and the VEX statement when that lands.
58+
- Consider renaming the TOML field from `cve` to `id` once every existing statement has been regenerated; not worth the churn today.
59+
60+
---
61+
1262
## [2026-04-19 21:25] - Remove bogus `vexctl validate` step from build-vex job
1363

1464
**Author:** Erick Bourgeois

.vex/GHSA-cq8v-f236-94qc.toml

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# rand soundness when a custom `log` logger calls rand::rng() and triggers
2+
# ThreadRng reseeding at warn- or trace-level.
3+
#
4+
# Reachable-only-if ALL of:
5+
# 1. the `log` and `thread_rng` rand features are enabled,
6+
# 2. a custom logger implementation is registered,
7+
# 3. that logger itself calls rand::rng() / a TryRng method on ThreadRng,
8+
# 4. ThreadRng reseeds (every 64 kB) while the logger is on the stack,
9+
# 5. trace-level is on OR warn-level is on with getrandom unavailable.
10+
#
11+
# In 5-Spot's dependency graph, the only vulnerable rand 0.8.6 is pulled
12+
# transitively via warp 0.3.7 → tokio-tungstenite 0.21 → tungstenite 0.21,
13+
# where rand is used exclusively for websocket client frame-masking keys
14+
# (tungstenite::protocol::frame::mask). That code path is not invoked
15+
# from, or reachable by, any `log` / `tracing` logger implementation in
16+
# this project — 5-Spot uses tracing-subscriber's stock fmt + json
17+
# layers with env-filter, none of which touch rand from within emit().
18+
#
19+
# The full fix (rand >= 0.9.3) requires warp → tokio-tungstenite 0.22+,
20+
# which warp 0.3.x does not support. Tracked for removal when the warp
21+
# 0.3 → 0.4 / axum migration lands; until then this statement documents
22+
# the non-exploitability and a Scorecard-side ignore is recorded in
23+
# osv-scanner.toml at the repo root.
24+
cve = "GHSA-cq8v-f236-94qc"
25+
status = "not_affected"
26+
justification = "vulnerable_code_not_in_execute_path"
27+
impact_statement = "Vulnerable rand 0.8.6 is pulled transitively by tungstenite 0.21 for websocket frame masking only. The advisory requires ThreadRng to be invoked from within a custom log::Log implementation during reseed; 5-Spot's tracing-subscriber stack does not call into rand from any logger emit path, so the unsound code is not in the execute path."
28+
products = [
29+
"pkg:oci/5-spot-chainguard",
30+
"pkg:oci/5-spot-distroless",
31+
]
32+
author = "erick.bourgeois@gmail.com"
33+
timestamp = "2026-04-19T00:00:00Z"

.vex/README.md

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,16 @@ statement is explicitly authored and reviewed.
3535

3636
## File format
3737

38-
One TOML file per CVE, named `<CVE-ID>.toml` (case-insensitive match on the
39-
`cve` field; file name is informational).
38+
One TOML file per advisory, named `<identifier>.toml` (case-insensitive match
39+
on the `cve` field; file name is informational). Accepted identifier shapes:
40+
41+
- `CVE-YYYY-NNNN+` — MITRE CVE (the common case).
42+
- `GHSA-xxxx-xxxx-xxxx` — GitHub Security Advisory. Use this when the advisory
43+
has no assigned CVE yet (e.g. `GHSA-cq8v-f236-94qc`).
44+
- `RUSTSEC-YYYY-NNNN` — RustSec advisory DB.
45+
46+
The TOML field is still named `cve` for backward compatibility with the
47+
original file format; the value may be any of the above.
4048

4149
```toml
4250
cve = "CVE-2025-12345"

deploy/deployment/rbac/clusterrole.yaml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,22 @@ rules:
1212
resources: ["scheduledmachines", "scheduledmachines/status"]
1313
verbs: ["get", "list", "watch", "create", "update", "patch", "delete"]
1414

15+
# ScheduledMachine finalizers — required because the controller sets
16+
# `blockOwnerDeletion: true` on the ownerReference it writes from every
17+
# child CAPI Machine back to its parent ScheduledMachine
18+
# (src/reconcilers/helpers.rs). The API server treats blockOwnerDeletion
19+
# as equivalent to holding a finalizer on the owner, so it checks that
20+
# the creating identity has `update` on <owner-resource>/finalizers.
21+
# Without this rule, Machine creation is rejected with:
22+
# cannot set blockOwnerDeletion if an ownerReference refers to a
23+
# resource you can't set finalizers on
24+
# Verb is intentionally just `update` — the minimum the subresource
25+
# check requires; the broader CRUD set on the main resource above is
26+
# not needed here.
27+
- apiGroups: ["5spot.finos.org"]
28+
resources: ["scheduledmachines/finalizers"]
29+
verbs: ["update"]
30+
1531
# CAPI Machine management
1632
- apiGroups: ["cluster.x-k8s.io"]
1733
resources: ["machines", "machines/status"]

osv-scanner.toml

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
# osv-scanner.toml — OSV-Scanner ignore list.
2+
#
3+
# This file is consumed by osv-scanner (and, transitively, by the OpenSSF
4+
# Scorecard `Vulnerabilities` check, which runs osv-scanner against the
5+
# lockfiles it finds in the repository). Entries listed here are reported
6+
# as informational but do not fail the scan, so Scorecard can restore the
7+
# Vulnerabilities score to 10/10 while we carry the underlying transitive
8+
# dependency.
9+
#
10+
# Every ignore MUST include:
11+
# - `id` — the advisory ID (CVE / GHSA / RUSTSEC).
12+
# - `reason` — a short justification mirroring the matching
13+
# statement in `.vex/`. If the reason here and
14+
# the VEX impact_statement drift, the VEX doc is
15+
# the source of truth; keep this file aligned.
16+
# - `ignoreUntil` — a reassessment date. Past that date osv-scanner
17+
# will resume treating the advisory as unignored,
18+
# forcing a re-triage.
19+
#
20+
# File format reference:
21+
# https://google.github.io/osv-scanner/configuration/
22+
23+
[[IgnoredVulns]]
24+
id = "GHSA-cq8v-f236-94qc"
25+
ignoreUntil = "2026-10-19"
26+
reason = """
27+
rand 0.8.6 is pulled transitively via warp 0.3.7 -> tokio-tungstenite 0.21 ->
28+
tungstenite 0.21 and is used exclusively for websocket frame masking. The
29+
advisory requires ThreadRng to be invoked from inside a custom log::Log
30+
implementation during reseed; 5-Spot's tracing-subscriber stack never
31+
calls rand from a logger emit path, so the unsound code is not in the
32+
execute path. Full remediation requires warp 0.3 -> 0.4 / axum migration
33+
to pick up tokio-tungstenite 0.22+ (rand >= 0.9.3). Triage recorded in
34+
.vex/GHSA-cq8v-f236-94qc.toml (OpenVEX: not_affected,
35+
vulnerable_code_not_in_execute_path). Reassess by 2026-10-19.
36+
"""

tools/assemble_openvex.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,13 @@ def _load_statement(path: Path) -> dict:
4343
with path.open("rb") as fh:
4444
doc = tomllib.load(fh)
4545

46+
# Preserve the identifier verbatim — CVE-YYYY-NNNN+ is uppercase by
47+
# MITRE convention, but GHSA-xxxx-xxxx-xxxx segments are lowercase,
48+
# and upper-casing them breaks round-tripping against osv.dev and
49+
# github.com/advisories (which treat GHSA IDs case-insensitively for
50+
# matching but render them in their canonical lowercase form).
4651
statement: dict = {
47-
"vulnerability": {"name": doc["cve"].upper()},
52+
"vulnerability": {"name": doc["cve"]},
4853
"products": [{"@id": product} for product in doc["products"]],
4954
"status": doc["status"],
5055
"timestamp": _ts_to_str(doc["timestamp"]),

tools/tests/assemble-openvex-tests.sh

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,25 @@ else
100100
fail_case "missing --id / --author rejected" "$TMP/err"
101101
fi
102102

103+
# ── GHSA identifier case is preserved (not upper-cased) ────────────────────
104+
# GHSA IDs are conventionally lowercase alphanumeric segments; upper-casing
105+
# them changes the on-wire identifier and breaks a round-trip against
106+
# osv.dev / github.com/advisories. Regression guard for the rand soundness
107+
# advisory (GHSA-cq8v-f236-94qc) which was the first non-CVE ID we shipped.
108+
out_ghsa="$TMP/ghsa.json"
109+
python3 "$ASSEMBLER" \
110+
--vex-dir "$FIXTURES/valid-ghsa" \
111+
--id "urn:vex:ghsa-test" \
112+
--author "ci@example" \
113+
--timestamp "2026-04-19T00:00:00Z" \
114+
--output "$out_ghsa" >/dev/null 2>"$TMP/err"
115+
if grep -q '"GHSA-cq8v-f236-94qc"' "$out_ghsa" &&
116+
! grep -q '"GHSA-CQ8V-F236-94QC"' "$out_ghsa"; then
117+
pass_case "GHSA identifier case is preserved verbatim"
118+
else
119+
fail_case "GHSA identifier case is preserved verbatim" "$out_ghsa"
120+
fi
121+
103122
# ── Default timestamp (no --timestamp) produces RFC-3339 UTC ───────────────
104123
out_dt="$TMP/dt.json"
105124
python3 "$ASSEMBLER" \
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
cve = "GHSA-CQ8V-F236-94QC"
2+
status = "not_affected"
3+
justification = "vulnerable_code_not_in_execute_path"
4+
products = ["pkg:oci/5-spot-chainguard"]
5+
author = "erick.bourgeois@gmail.com"
6+
timestamp = "2026-04-19T00:00:00Z"
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
cve = "GHSA-cq8v-f236-94qc"
2+
status = "not_affected"
3+
justification = "vulnerable_code_not_in_execute_path"
4+
products = ["pkg:oci/5-spot-chainguard"]
5+
author = "erick.bourgeois@gmail.com"
6+
timestamp = "2026-04-19T00:00:00Z"
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
cve = "GHSA-not-right"
2+
status = "fixed"
3+
products = ["pkg:oci/5-spot-chainguard"]
4+
author = "erick.bourgeois@gmail.com"
5+
timestamp = "2026-04-19T00:00:00Z"

0 commit comments

Comments
 (0)