diff --git a/.trivyignore.yaml b/.trivyignore.yaml new file mode 100644 index 0000000..7529562 --- /dev/null +++ b/.trivyignore.yaml @@ -0,0 +1,39 @@ +# Accepted misconfiguration findings, each with a reason and an expiry date. +# +# The expiry is the point. An ignore without one is a permanent hole that nobody revisits; +# trivy stops honouring these entries after the date, the gate goes red, and someone has to +# look again. Re-dating an entry is a decision. Letting it lapse silently is not possible. +# +# Nothing here is suppressed because it was inconvenient. Both entries are inherent to what +# a control plane that installs arbitrary Helm charts *is*, and both are documented in +# ARCHITECTURE.md under "The provisioner is privileged". + +misconfigurations: + - id: KSV-0041 + # "ClusterRole shouldn't have access to manage resource 'secrets'" + # + # This is true and it is the design. Helm stores release state as Secrets in the + # release's namespace, so anything that runs `helm upgrade --install` must be able to + # create and read Secrets there. The worker installs into a namespace per tenant, and + # those namespaces are created at provision time, so the grant cannot be enumerated in + # advance and ends up cluster-scoped. + # + # The consequence, stated plainly: svcforge can read any Secret in the cluster, + # including ServiceAccount tokens, and can therefore impersonate any workload. It is a + # privileged component. Treat compromise of the worker as compromise of the cluster. + # + # The real fix is per-namespace Roles bound at provision time, which needs RBAC write + # permission — itself an escalation path unless carefully constrained. That is a + # larger piece of work than this reference implementation takes on, and pretending + # otherwise by hiding the finding would be worse than recording it. + statement: "helm stores release state in Secrets; namespaces are created per tenant at runtime" + expired_at: 2026-10-01 + + - id: KSV-0056 + # "ClusterRole should not have create/update/delete on services, endpoints, ..." + # + # Same root cause. A chart that installs Elasticsearch creates a Service; the + # provisioner has to be able to create it. The interception risk the rule describes is + # real and follows from the same privileged position as KSV-0041. + statement: "installing a chart necessarily creates the Services that chart defines" + expired_at: 2026-10-01 diff --git a/tests/integration/test_helm_timeout.py b/tests/integration/test_helm_timeout.py index ce3906d..c62e831 100644 --- a/tests/integration/test_helm_timeout.py +++ b/tests/integration/test_helm_timeout.py @@ -1,29 +1,59 @@ """The one test that proves the timeout is real. -`bash -c "sleep 300 & sleep 300"` is a miniature helm: a process that forks a child and -waits on another. Kill the direct child only and the backgrounded `sleep` reparents to init -and keeps running — which, when the process is helm, means a timed-out task retries while -the original helm is still mutating the same release. +`bash -c "sleep N & sleep N"` is a miniature helm: a process that forks a child and waits on +another. Kill the direct child only and the backgrounded `sleep` reparents to init and keeps +running — which, when the process is helm, means a timed-out task retries while the original +helm is still mutating the same release. -This test needs a real process tree, so it lives in integration/. It needs no database: -the `pool` fixture in conftest is not autouse. +This test needs a real process tree, so it lives in integration/. It needs no database: the +`pool` fixture in conftest is not autouse. """ from __future__ import annotations import asyncio +import os import subprocess import pytest from svcforge_core.adapters.helm import _run +# A duration nothing else on the machine will be sleeping for, derived from the pid so two +# concurrent runs cannot collide either. +# +# The obvious version of this test hardcodes `sleep 300` and then asserts +# `pgrep -f "sleep 300"` is empty. That assertion is machine-global: it matches ANY process +# whose command line contains the string, including a leftover from an earlier run of this +# same test, an unrelated `sleep 300` somewhere on the box, and — the subtle one — the shell +# that is running pgrep, whose own command line contains the pattern it is searching for. +# The result is a test that fails for reasons that have nothing to do with the code, which +# is as useless as one that cannot fail at all. Observed: three spurious matches on a +# developer box, and a red run blamed on a Python upgrade that was innocent. +_SLEEP_S = 30000 + (os.getpid() % 1000) +_PATTERN = f"sleep {_SLEEP_S}" + + +def _survivors() -> list[str]: + """PIDs still matching this run's unique sleep. Empty means the group really died.""" + out = subprocess.run( # noqa: S603 + ["pgrep", "-f", _PATTERN], # noqa: S607 - resolved via PATH, fixed argv + capture_output=True, + text=True, + check=False, + ) + return [line for line in out.stdout.split() if line.strip()] + @pytest.mark.asyncio async def test_timeout_kills_the_whole_process_group() -> None: - argv = ["bash", "-c", "sleep 300 & sleep 300"] # child forks a grandchild + # Guard the guard: if the pattern already matches something, the assertion below would + # be meaningless. Fail loudly rather than report a false negative. + assert not _survivors(), f"{_PATTERN!r} matched before the test started; pick another marker" + + argv = ["bash", "-c", f"{_PATTERN} & {_PATTERN}"] # child forks a grandchild with pytest.raises(TimeoutError): await _run(argv, timeout_s=1) await asyncio.sleep(0.5) - out = subprocess.run(["pgrep", "-f", "sleep 300"], capture_output=True, text=True) # noqa: S607 - assert out.stdout.strip() == "", "grandchild survived: you killed the child, not the group" + + assert not _survivors(), "grandchild survived: you killed the child, not the group"