Fix #2681: verify sha256 on kustomize tarball install - #2686
Merged
jwbron merged 1 commit intoMay 12, 2026
Merged
Conversation
Defense-in-depth follow-up to PR #2658. Pins the published linux_amd64 checksum (`KUSTOMIZE_SHA256`) and runs `sha256sum -c` before extracting so a release-mirror compromise or in-flight tampering on the build node fails the build instead of silently shipping a swapped binary. Bumping `KUSTOMIZE_VERSION` now requires updating `KUSTOMIZE_SHA256` in lockstep (called out in the comment above the ARGs). Stacked on egg/2641-deployment-validation-integration-tests (PR #2658) since the install line itself isn't on main yet.
jwbron
merged commit May 12, 2026
0e1befc
into
egg/2641-deployment-validation-integration-tests
10 of 11 checks passed
jwbron
added a commit
that referenced
this pull request
May 13, 2026
…2641) (#2658) * test(integration): cover deployment-validation routes past auth gate Closes #2641. Adds `integration_tests/test_deployment_validation_logic.py` exercising post-auth behaviour of `POST /api/v1/deployment/validate-manifests` and `POST /api/v1/deployment/validate-network-isolation`. The existing sibling `test_k8s_deployment_tools.py` covers `@require_lifecycle_secret` parity (401/503 on missing/wrong bearer) but stops at the auth gate because the lifecycle bearer wasn't surfaced through the shared `EggStack` fixture. `integration_tests/conftest.py` now reads `lifecycle-secret` from `gateway-secrets` alongside `launcher-secret` and exposes a session-scoped `lifecycle_secret` fixture that skips when the cluster Secret has no such key. Surfaced bugs (filed as follow-ups; xfail or current-state assertions referenced from the test module's docstring): - #2646 — orchestrator SA can't list `kube-system` DaemonSets, so `validate-network-isolation` always short-circuits with `network_policy_enforcement_not_detected` in production. - #2647 — orchestrator container has no `kustomize`/`kubectl` on PATH, so `validate-manifests` returns 500 `kustomize_unavailable` for any default-overlay call. - #2648 — orchestrator SA can `get` but not `list` Deployments in `egg-system`, so `get_deployment_context` always reports `images_unavailable: true`. - #2652 — probe field `orchestrator_direct_blocked` is misleading (`allow-agent-to-orchestrator` policy intentionally permits the same path the field "checks"). `validate_config` is deliberately out of scope for the k3s tier: it's an MCP-side Pydantic handler with no HTTP route. Its coverage stays in `orchestrator/tests/test_mcp_tools.py::TestValidateConfig`. * Fix #2641 follow-ups: RBAC + kustomize + probe-field rename Bundles fixes for the four follow-up issues filed against #2658 so the integration suite can assert happy paths instead of locked-in regressions. - #2646: add ClusterRole egg-cluster-topology-reader granting the orchestrator SA get,list on apps/daemonsets and cluster-scoped nodes. _detect_cni / _detect_k3s now resolve against a real cluster. - #2647: install pinned kustomize v5.6.0 in the orchestrator image so _run_kustomize no longer raises kustomize_unavailable. The secondary repo-not-mounted gap remains acknowledged out of scope. - #2648: add list verb to apps/deployments in the egg-service-log-reader Role so _collect_egg_image_tags returns populated tags instead of {}. - #2652: rename probe field orchestrator_direct_blocked -> orchestrator_api_reachable with flipped polarity. The agent->orchestrator heartbeat path is deliberately permitted so the field now reads positively. Updates callers in mcp_tools, unit tests, docs, and the agent-diagnose skill. Removes the xfail and short-circuit assertions in the integration suite and replaces them with positive happy-path assertions. * Fix hardcoded-ports lint: suppress EGG002 in docstring comment * Fix probe double-000: remove redundant || echo 000 in PROBE_COMMAND_TEMPLATE curl -w '%{http_code}' already outputs '000' when no HTTP response is received (connection refused / timeout / egress blocked). The previous || echo 000 fallback ran on curl's non-zero exit, concatenating a second '000' so internet_status became '000000'. The comparison "" == "000" then evaluated False, reporting internet_blocked: False even on a correctly-isolated cluster. Drop the fallback entirely and redirect stderr so the function is silent; || true keeps the function's exit code at 0 regardless. * Address review feedback on PR #2658 Fixes raised by egg-reviewer across the three review rounds on this PR: * PROBE_COMMAND_TEMPLATE backticks (blocking-grade): the unquoted ``<<PY`` heredoc evaluates backticks as command substitution under ``/bin/sh``, so the markdown-style backticks in the comment block produced ``sh: ...: not found`` lines in every probe pod log and were one fragile rename away from corrupting the Python source itself. Strip the backticks from the comments and add a NOTE inside the heredoc explaining why. Add two unit-test guards under ``TestProbeCommandTemplate``: ``test_template_contains_no_backticks`` asserts the cheap shape invariant, and ``test_template_is_shell_syntax_valid`` runs the template through ``/bin/sh -n`` so a future quoting mistake surfaces in CI rather than in the probe pod. * RBAC over-scope (non-blocking): split ``egg-cluster-topology-reader`` so only ``nodes`` (cluster-scoped resource) keeps cluster-wide read; the ``apps/daemonsets`` grant moves to a namespaced ``Role`` + ``RoleBinding`` in ``kube-system`` (the only namespace ``_detect_cni`` / ``_detect_k3s`` actually query). Matches least-privilege. * ``test_validation_routes_reject_invalid_json`` (non-blocking): the prior bare ``"traceback" not in text`` assertion silently passes through the regression its docstring named (non-silent ``get_json()`` raises Flask BadRequest, which renders as a clean 400 with no traceback). Pin the real invariant: malformed JSON must produce the same status + success-flag as an empty body, which is the contract ``get_json(silent=True) or {}`` guarantees. * Orphan probe-job test scope (non-blocking): scope the kubectl selector to this test's ``pipeline_id`` so a concurrent probe from ``TestValidationRouteConcurrency`` can't be misattributed as a leak. * Default-overlay test ambiguity (non-blocking): pin to exactly one expected outcome per environment by probing the same host bind-mount path the orchestrator pod sees. A regression that flips 200↔404 in either local-dev or CI would otherwise pass. * Secret-leak check (non-blocking): check the full 64-hex-char bearer plus four non-overlapping 16-char windows, not just the 16-char prefix. Catches echoes of any contiguous slice of the secret. Filed #2681 for the kustomize-checksum verification (Dockerfile download has no integrity check) since the reviewer explicitly labelled that one "Worth a follow-up". The ``except (X, Y):`` parenthesization suggestion is a no-op under this project's formatter: pyproject targets py314 and ruff format normalises to the PEP 758 unparenthesized form, so the suggested edit is reverted on every save. Left unchanged; rationale captured in the response comment. * Fix #2681: verify sha256 on kustomize tarball install (#2686) Defense-in-depth follow-up to PR #2658. Pins the published linux_amd64 checksum (`KUSTOMIZE_SHA256`) and runs `sha256sum -c` before extracting so a release-mirror compromise or in-flight tampering on the build node fails the build instead of silently shipping a swapped binary. Bumping `KUSTOMIZE_VERSION` now requires updating `KUSTOMIZE_SHA256` in lockstep (called out in the comment above the ARGs). Stacked on egg/2641-deployment-validation-integration-tests (PR #2658) since the install line itself isn't on main yet. * Fix #2658: bypass kubernetes client auto-deserialization for probe log The kubernetes-python client's ApiClient.deserialize() unconditionally runs json.loads() on every response body before coercing to the declared response_type. For pod logs declared as `str`, when the log content happens to be JSON-parseable, the deserializer turns it into a dict and then str()'s the dict — yielding Python repr (single quotes, ``True``) instead of the original JSON. The probe's ``print(json.dumps(...))`` output thus reaches ``_parse_probe_output`` as Python dict repr, which fails JSON parsing, and the route returns ``probe_output_unparseable``. Pass ``_preload_content=False`` to bypass the deserialize path and read the raw bytes off the urllib3 HTTPResponse directly. * Fix #2658: widen _read_probe_log try/except and cover with unit tests Two non-blocking concerns from the latest review on commit 4978666: 1. _preload_content=False defers the actual network read to .data access. The previous try/except only covered the read_namespaced_pod_log call itself, so a mid-stream connection reset or malformed transfer-encoding at .data access would propagate up and 500 the route handler. Extend the try block to cover the .data access too. 2. Add a new TestReadProbeLog unit test class covering: - bytes path (the happy path on the new code) - invalid-utf-8 bytes (errors="replace" works as intended) - str fallback when the response has no .data attribute - None response returns "" rather than crashing - Exception from read_namespaced_pod_log returns "" - Exception from .data access returns "" (the regression-window this commit closes) The integration suite already exercises the end-to-end happy path against a real cluster; these unit tests pin the bytes/str branching and the body-read exception envelope so a future "simplification" of getattr(raw, "data", raw) -> raw.data or the removal of the .data exception coverage surfaces in fast CI rather than only on the integration tier. * Address review nits on _read_probe_log: guard data is None + clarify defensive test - Add explicit if data is None: return '' after the getattr() in _read_probe_log so a hypothetical response with .data = None yields '' rather than the literal string 'None' (which would flow into _parse_probe_output as probe log content). - Add TestReadProbeLog::test_returns_empty_string_on_none_data unit test pinning the new guard. - Clarify test_str_fallback_for_object_without_data's docstring to state it is a defensive-only branch — kubernetes-python with _preload_content=False always returns urllib3.HTTPResponse, which always has .data; the str() fallback exists only so a future client upgrade or mock returning a plain string degrades cleanly. Both items raised non-blocking on the 1e5daf1 re-review. --------- Co-authored-by: james-in-a-box[bot] <246424927+james-in-a-box[bot]@users.noreply.github.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.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
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.
Summary
KUSTOMIZE_SHA256to the published linux_amd64 checksum for v5.6.0 (54e4031d…12a03, from kustomize'schecksums.txt).sha256sum -cbefore extracting the tarball so a compromised release mirror or in-flight tampering on the build node fails the build instead of silently shipping a swapped binary.KUSTOMIZE_VERSIONrequires updatingKUSTOMIZE_SHA256in lockstep.Closes #2681.
Stacked on #2658
The kustomize install line this PR hardens is introduced by PR #2658 and isn't on
mainyet, so the base for this PR isegg/2641-deployment-validation-integration-tests. The actual diff (visible after #2658 merges) is the comment update +ARG KUSTOMIZE_SHA256=…+sha256sum -c -line inorchestrator/Dockerfile. After #2658 merges, please retarget this PR tomain.Test plan
sha256sum -c -fails the build if the published checksum is wrong (manually verifiable by flipping a hex digit and rebuilding locally).