From 7a0f814fcbbe7798938f5692623bfdbbc43ec701 Mon Sep 17 00:00:00 2001 From: Hayden <154503486+groupthinking@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:19:57 -0500 Subject: [PATCH 1/4] fix: harden Dockerfile.production install and de-vacuify its security tests `tests/unit/test_security_fixes.py::test_dockerfile_uses_nonroot_user` resolved `project_root / "Dockerfile.production"`, a path that has never existed in this repository (the file lives at `infrastructure/docker/Dockerfile.production`). The test therefore hit its `pytest.skip` branch on every run and asserted nothing, so it silently covered nothing for its entire lifetime. Behind that blind spot, the image's dependency install was unsafe: - All 9 packages were installed completely unpinned, so any build could silently pull a new major version, and `python-multipart` could resolve below the 0.0.31 floor mandated by GHSA-59g5-xgcq-4qw3 (issue #1095). - The whole `pip install` was suffixed with `|| echo "..."`, which forces exit 0. A total install failure produced a successful build of an image with no runtime dependencies, deferring the failure to first request. - `pytest` was installed into the production image, shipping test tooling and its transitive tree into the runtime attack surface. Changes: - Pin explicit floors for all 8 retained packages, matching or exceeding requirements.txt / pyproject.toml. - Drop `|| echo` so a failed install fails the build. - Drop `pytest` from the production image. - Fix the test path so a missing Dockerfile now fails instead of skipping, and assert against the *final* `USER` directive rather than any match. - Add three guards: floors must not drift below requirements.txt, the install must not swallow failures, and test tooling must not be present. - Correct stale root-relative deployment paths. `one-click-deploy.sh` and SECURITY.md referenced `Dockerfile.production`, `k8s/production/` and `k8s/monitoring/` at the repo root; all four moved under `infrastructure/` and every reference was dangling. All four Dockerfile tests are verified non-vacuous: three fail against the pre-fix Dockerfile, and the non-root test fails when `USER root` is injected. Closes #1121 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- SECURITY.md | 2 +- infrastructure/docker/Dockerfile.production | 21 +- scripts/deployment/one-click-deploy.sh | 20 +- tests/unit/test_security_fixes.py | 212 +++++++++++++++++++- 4 files changed, 235 insertions(+), 20 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index 50518bb5a..e0790684c 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -50,7 +50,7 @@ The project uses [Trivy](https://github.com/aquasecurity/trivy) for automated co To run Trivy locally before pushing: ```bash # Scan the production Docker image -docker build -t eventrelay:test -f Dockerfile.production . +docker build -t eventrelay:test -f infrastructure/docker/Dockerfile.production . docker run --rm -v /var/run/docker.sock:/var/run/docker.sock aquasecurity/trivy:latest image eventrelay:test ``` diff --git a/infrastructure/docker/Dockerfile.production b/infrastructure/docker/Dockerfile.production index d1ebab718..c50103469 100644 --- a/infrastructure/docker/Dockerfile.production +++ b/infrastructure/docker/Dockerfile.production @@ -24,10 +24,25 @@ WORKDIR /app # Copy all files (matches existing Dockerfile pattern) COPY . /app/ -# Install Python packages with SSL trust configuration for constrained environments +# Install Python packages with SSL trust configuration for constrained environments. +# +# Version floors are duplicated from requirements.txt / pyproject.toml rather than +# installed via `-r requirements.txt`, because this image deliberately ships a +# reduced runtime subset. They MUST stay >= the canonical declarations; the +# equivalence is enforced by +# tests/unit/test_security_fixes.py::test_dockerfile_production_pins_dependency_floors. +# +# Failure is fatal by design: a swallowed `pip install` previously let this image +# build successfully with no packages installed, deferring the error to runtime. RUN pip install --trusted-host pypi.org --trusted-host pypi.python.org --trusted-host files.pythonhosted.org \ - fastapi uvicorn pytest python-dotenv pydantic aiofiles httpx requests python-multipart || \ - echo "Warning: Some packages may not be available in constrained environments" + "fastapi>=0.110.0" \ + "uvicorn[standard]>=0.24.0" \ + "python-dotenv>=1.2.2" \ + "pydantic>=2.5.0" \ + "aiofiles>=23.2.1" \ + "httpx>=0.25.0" \ + "requests>=2.31.0" \ + "python-multipart>=0.0.31" # Create necessary directories and set permissions RUN mkdir -p logs && \ diff --git a/scripts/deployment/one-click-deploy.sh b/scripts/deployment/one-click-deploy.sh index c8b7c77e1..e1e9f58ad 100755 --- a/scripts/deployment/one-click-deploy.sh +++ b/scripts/deployment/one-click-deploy.sh @@ -65,11 +65,11 @@ success "Kubernetes cluster is accessible" # Check if required files exist REQUIRED_FILES=( - "Dockerfile.production" + "infrastructure/docker/Dockerfile.production" "package.json" - "k8s/production/deployment.yaml" - "k8s/production/service.yaml" - "k8s/monitoring/monitoring.yaml" + "infrastructure/k8s/production/deployment.yaml" + "infrastructure/k8s/production/service.yaml" + "infrastructure/k8s/monitoring/monitoring.yaml" "mcp_server.py" "learning_app_processor.py" ) @@ -101,7 +101,7 @@ log "Step 3: Building Docker Image" DOCKER_TAG="enhanced-framework:$(date +%Y%m%d-%H%M%S)" LATEST_TAG="enhanced-framework:latest" -if docker build -f Dockerfile.production -t "$DOCKER_TAG" -t "$LATEST_TAG" .; then +if docker build -f infrastructure/docker/Dockerfile.production -t "$DOCKER_TAG" -t "$LATEST_TAG" .; then success "Docker image built successfully: $DOCKER_TAG" else error "Docker image build failed" @@ -111,21 +111,21 @@ fi log "Step 4: Validating Kubernetes Manifests" # Validate deployment manifest -if kubectl apply --dry-run=client -f k8s/production/deployment.yaml; then +if kubectl apply --dry-run=client -f infrastructure/k8s/production/deployment.yaml; then success "Deployment manifest is valid" else error "Deployment manifest validation failed" fi # Validate service manifest -if kubectl apply --dry-run=client -f k8s/production/service.yaml; then +if kubectl apply --dry-run=client -f infrastructure/k8s/production/service.yaml; then success "Service manifest is valid" else error "Service manifest validation failed" fi # Validate monitoring manifest -if kubectl apply --dry-run=client -f k8s/monitoring/monitoring.yaml; then +if kubectl apply --dry-run=client -f infrastructure/k8s/monitoring/monitoring.yaml; then success "Monitoring manifest is valid" else error "Monitoring manifest validation failed" @@ -144,14 +144,14 @@ fi log "Step 6: Deploying to Kubernetes" # Deploy application -if kubectl apply -f k8s/production/ -n "$NAMESPACE"; then +if kubectl apply -f infrastructure/k8s/production/ -n "$NAMESPACE"; then success "Application deployed successfully" else error "Application deployment failed" fi # Deploy monitoring -if kubectl apply -f k8s/monitoring/ -n "$NAMESPACE"; then +if kubectl apply -f infrastructure/k8s/monitoring/ -n "$NAMESPACE"; then success "Monitoring stack deployed successfully" else error "Monitoring deployment failed" diff --git a/tests/unit/test_security_fixes.py b/tests/unit/test_security_fixes.py index f4a682dce..64bdae3b1 100644 --- a/tests/unit/test_security_fixes.py +++ b/tests/unit/test_security_fixes.py @@ -7,6 +7,8 @@ """ import os +import re +import shlex import sys import pytest from pathlib import Path @@ -18,6 +20,119 @@ sys.path.insert(0, str(project_root)) sys.path.insert(0, str(project_root / "src")) +# Canonical location of the production container definition. Kept as a module +# constant so the path is asserted in exactly one place; tests fail rather than +# skip when it does not resolve. +PRODUCTION_DOCKERFILE = project_root / "infrastructure" / "docker" / "Dockerfile.production" + +# Matches a PEP 508-ish requirement with a `>=` floor, with or without extras +# and surrounding quotes, e.g. `"uvicorn[standard]>=0.24.0"` or `fastapi`. +_REQUIREMENT_RE = re.compile( + r"""^["']?(?P[A-Za-z0-9][A-Za-z0-9._-]*) # distribution name + (?:\[[^\]]*\])? # optional extras + (?:\s*>=\s*(?P[0-9][0-9A-Za-z.*+!-]*))? # optional >= floor + """, + re.VERBOSE, +) + + +def _version_key(version: str) -> tuple: + """Comparable key for a dotted version. Non-numeric segments sort as -1 so + pre-releases order below the corresponding final release.""" + parts = [] + for segment in re.split(r"[._-]", version): + parts.append((0, int(segment)) if segment.isdigit() else (-1, 0)) + return tuple(parts) + + +def _fmt(key: tuple) -> str: + return ".".join(str(value) for _, value in key) + + +def _pip_install_command(text: str) -> str: + """Reconstruct the logical ``pip install`` command from a Dockerfile, + joining backslash line continuations into a single string. + + Operating on the joined command rather than on individual physical lines is + essential: the pre-hardening Dockerfile spread ``pytest`` and a trailing + ``|| echo`` across continuation lines, so any line-filtered check silently + passed against the very content it was meant to reject. + """ + logical, buf = [], "" + for raw in text.splitlines(): + line = raw.rstrip() + stripped = line.strip() + if stripped.startswith("#"): + continue + if line.endswith("\\"): + buf += line[:-1].strip() + " " + continue + buf += stripped + if buf: + logical.append(buf) + buf = "" + if buf: + logical.append(buf) + for command in logical: + if "pip install" in command: + return command + return "" + + +def _installed_requirements(command: str) -> dict: + """Parse ``{normalised_name: floor_key_or_None}`` from a joined ``pip + install`` command, tolerating quoted and unquoted tokens alike.""" + floors = {} + if not command: + return floors + takes_value = { + "--trusted-host", + "--index-url", + "--extra-index-url", + "-i", + "-c", + "-r", + "--constraint", + "--requirement", + } + skip_next = False + for token in shlex.split(command): + if skip_next: + skip_next = False + continue + if token in ("||", "&&", ";"): + # Everything past a shell operator is a fallback, not a requirement. + break + if token in ("RUN", "pip", "install"): + continue + if token.startswith("-"): + if token in takes_value: + skip_next = True + continue + match = _REQUIREMENT_RE.match(token) + if not match: + continue + name = match.group("name").lower().replace("_", "-") + floor = match.group("floor") + floors[name] = _version_key(floor) if floor else None + return floors + + +def _parse_floors(text: str) -> dict: + """Extract ``{normalised_name: floor_key}`` from a requirements file.""" + floors = {} + for raw in text.splitlines(): + line = raw.strip().rstrip("\\").strip().rstrip(",") + if not line or line.startswith("#") or line.startswith("-"): + continue + match = _REQUIREMENT_RE.match(line) + if not match: + continue + name = match.group("name").lower().replace("_", "-") + floor = match.group("floor") + floors[name] = _version_key(floor) if floor else None + return floors + class TestAPIKeyExposureFix: """Test Issue 1: API Key Exposure Risk Fix""" @@ -189,15 +304,100 @@ def test_no_hardcoded_api_keys(self): pytest.fail(f"Possible hardcoded API key in {file_path}: {line[:80]}...") def test_dockerfile_uses_nonroot_user(self): - """Verify Dockerfile.production uses non-root user""" - dockerfile = project_root / "Dockerfile.production" - if not dockerfile.exists(): - pytest.skip("Dockerfile.production not found") + """Verify Dockerfile.production drops privileges to a non-root user. + + The path is asserted rather than skipped on: this test previously + resolved ``project_root / "Dockerfile.production"``, which has never + existed, so it skipped unconditionally and the assertions below never + ran. Failing loudly means a future relocation cannot silently re-vacate + the check. + """ + dockerfile = PRODUCTION_DOCKERFILE + assert dockerfile.exists(), ( + f"{dockerfile.relative_to(project_root)} not found. If the file moved, " + "update PRODUCTION_DOCKERFILE rather than skipping this test." + ) content = dockerfile.read_text() - assert "USER" in content, "Dockerfile should switch to non-root user" - assert "appuser" in content or "nonroot" in content.lower() + user_directives = [ + line.strip() + for line in content.splitlines() + if line.strip().startswith("USER ") + ] + assert user_directives, "Dockerfile should switch to a non-root user" + + final_user = user_directives[-1].split(maxsplit=1)[1].strip() + assert final_user not in { + "root", + "0", + }, f"Dockerfile must not run as root, got USER {final_user}" + assert final_user in {"appuser", "nonroot"}, ( + f"Unexpected runtime user {final_user!r}; expected a known " + "unprivileged account" + ) + + def test_dockerfile_production_pins_dependency_floors(self): + """Every dependency installed by Dockerfile.production must carry a + floor at least as high as the canonical declaration in + ``requirements.txt``. + + This image installs a reduced runtime subset by name instead of using + ``-r requirements.txt``, so advisory floors raised in the canonical + manifest do not propagate automatically. Without this guard the image + silently drifts behind published security fixes -- which is how an + unpinned ``python-multipart`` survived the floor bump for advisories + 468-471 (see #1095). + """ + dockerfile = PRODUCTION_DOCKERFILE + assert dockerfile.exists(), f"{dockerfile} not found" + + requirements = project_root / "requirements.txt" + assert requirements.exists(), "requirements.txt not found" + + canonical = _parse_floors(requirements.read_text()) + installed = _installed_requirements( + _pip_install_command(dockerfile.read_text()) + ) + + assert installed, "Dockerfile.production declares no pinned dependencies" + + for name, floor in sorted(installed.items()): + assert floor is not None, ( + f"{name} is installed without a version floor in " + "Dockerfile.production; an unconstrained resolve can select a " + "version with a known advisory" + ) + expected = canonical.get(name) + if expected is None: + continue + assert floor >= expected, ( + f"{name} floor {_fmt(floor)} in Dockerfile.production is below " + f"the canonical requirements.txt floor {_fmt(expected)}" + ) + + def test_dockerfile_production_does_not_swallow_install_failures(self): + """A ``|| echo`` fallback on the install step makes ``docker build`` + exit 0 with no packages installed, deferring the failure to runtime.""" + command = _pip_install_command(PRODUCTION_DOCKERFILE.read_text()) + assert command, "no pip install step found in Dockerfile.production" + assert "|| echo" not in command and "|| true" not in command, ( + "Dockerfile.production must not mask pip install failures; a " + "swallowed install produces an image that builds cleanly and then " + "fails at runtime with ModuleNotFoundError" + ) + + def test_dockerfile_production_excludes_test_tooling(self): + """Test frameworks must not be installed into the production image.""" + installed = _installed_requirements( + _pip_install_command(PRODUCTION_DOCKERFILE.read_text()) + ) + assert installed, "no pip install step found in Dockerfile.production" + for tool in ("pytest", "pytest-cov", "pytest-asyncio"): + assert tool not in installed, ( + f"{tool} must not be installed into the production image; it " + "enlarges the runtime attack surface" + ) if __name__ == "__main__": From 6d170d4369abf4020cd4a2f37f151f9be06d8cfe Mon Sep 17 00:00:00 2001 From: Hayden <154503486+groupthinking@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:56:27 -0500 Subject: [PATCH 2/4] fix: correct production image entrypoint and de-vacuify its guards Addresses four review findings on #1122. 1. Floor test ignored pyproject.toml. `_parse_floors` only read requirements.txt, so a package whose real floor lives in pyproject (python-dotenv: requirements >=1.0.0, pyproject >=1.2.2) was checked against the weaker value. Added `_pyproject_floors()` and `_canonical_floors()`, which take the max floor across both manifests. 2. Failure-masking check was spelling-specific. It matched only the literals `|| echo` and `|| true`, so `|| :`, `; true`, and pipes all passed. Replaced with `shlex.split` over the joined logical command, rejecting any of `||`, `;`, `|`. `&&` is deliberately allowed: it propagates failure. 3. Reverted the one-click-deploy.sh path edits. Fixing its precheck would have made a doomed rollout reachable: the script builds enhanced-framework:latest from this Python/uvicorn image (port 8000, /readyz) but k8s/production/deployment.yaml runs that image as a Node app (NODE_ENV, PORT=3000, probes /ready). Reconciling that topology is out of scope for #1121 and is tracked separately. 4. CMD named a module that does not exist. `uvicorn server:app` pointed at a root server.py that has never existed here, so every container built from this file exited at startup. Corrected to `youtube_extension.main:app` with ENV PYTHONPATH=/app/src, matching the documented invocation in CLAUDE.md. Added test_dockerfile_production_entrypoint_module_exists, which parses CMD, requires PYTHONPATH to cover /app/src, resolves the module under src/, and asserts the ASGI attribute is defined. It resolves paths instead of importing so it holds without runtime dependencies installed. Non-vacuity verified by mutation: lowering the python-dotenv floor fails (1); each of `|| :`, `|| true`, `|| echo`, `; true`, `| tee` fails (2); restoring `server:app`, dropping PYTHONPATH, and naming an absent attribute each fail with a distinct message (4). Lint held at baseline: ruff codes identical, black hunks 8 -> 8. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- infrastructure/docker/Dockerfile.production | 14 +- scripts/deployment/one-click-deploy.sh | 20 +-- tests/unit/test_security_fixes.py | 147 ++++++++++++++++++-- 3 files changed, 155 insertions(+), 26 deletions(-) diff --git a/infrastructure/docker/Dockerfile.production b/infrastructure/docker/Dockerfile.production index c50103469..82656418e 100644 --- a/infrastructure/docker/Dockerfile.production +++ b/infrastructure/docker/Dockerfile.production @@ -58,5 +58,15 @@ USER appuser # Expose port EXPOSE 8000 -# Start the application (matches existing server pattern) -CMD ["uvicorn", "server:app", "--host", "0.0.0.0", "--port", "8000", "--workers", "1"] \ No newline at end of file +# Start the application. +# +# The application package lives under /app/src and uses absolute imports rooted +# there, so PYTHONPATH must include it -- this is the invocation documented in +# CLAUDE.md (`PYTHONPATH=src uvicorn youtube_extension.main:app`). The former +# `server:app` target referenced a root-level server.py that does not exist in +# this repository, so the container exited immediately on start. The module is +# asserted importable by +# tests/unit/test_security_fixes.py::test_dockerfile_production_entrypoint_module_exists. +ENV PYTHONPATH=/app/src + +CMD ["uvicorn", "youtube_extension.main:app", "--host", "0.0.0.0", "--port", "8000", "--workers", "1"] diff --git a/scripts/deployment/one-click-deploy.sh b/scripts/deployment/one-click-deploy.sh index e1e9f58ad..c8b7c77e1 100755 --- a/scripts/deployment/one-click-deploy.sh +++ b/scripts/deployment/one-click-deploy.sh @@ -65,11 +65,11 @@ success "Kubernetes cluster is accessible" # Check if required files exist REQUIRED_FILES=( - "infrastructure/docker/Dockerfile.production" + "Dockerfile.production" "package.json" - "infrastructure/k8s/production/deployment.yaml" - "infrastructure/k8s/production/service.yaml" - "infrastructure/k8s/monitoring/monitoring.yaml" + "k8s/production/deployment.yaml" + "k8s/production/service.yaml" + "k8s/monitoring/monitoring.yaml" "mcp_server.py" "learning_app_processor.py" ) @@ -101,7 +101,7 @@ log "Step 3: Building Docker Image" DOCKER_TAG="enhanced-framework:$(date +%Y%m%d-%H%M%S)" LATEST_TAG="enhanced-framework:latest" -if docker build -f infrastructure/docker/Dockerfile.production -t "$DOCKER_TAG" -t "$LATEST_TAG" .; then +if docker build -f Dockerfile.production -t "$DOCKER_TAG" -t "$LATEST_TAG" .; then success "Docker image built successfully: $DOCKER_TAG" else error "Docker image build failed" @@ -111,21 +111,21 @@ fi log "Step 4: Validating Kubernetes Manifests" # Validate deployment manifest -if kubectl apply --dry-run=client -f infrastructure/k8s/production/deployment.yaml; then +if kubectl apply --dry-run=client -f k8s/production/deployment.yaml; then success "Deployment manifest is valid" else error "Deployment manifest validation failed" fi # Validate service manifest -if kubectl apply --dry-run=client -f infrastructure/k8s/production/service.yaml; then +if kubectl apply --dry-run=client -f k8s/production/service.yaml; then success "Service manifest is valid" else error "Service manifest validation failed" fi # Validate monitoring manifest -if kubectl apply --dry-run=client -f infrastructure/k8s/monitoring/monitoring.yaml; then +if kubectl apply --dry-run=client -f k8s/monitoring/monitoring.yaml; then success "Monitoring manifest is valid" else error "Monitoring manifest validation failed" @@ -144,14 +144,14 @@ fi log "Step 6: Deploying to Kubernetes" # Deploy application -if kubectl apply -f infrastructure/k8s/production/ -n "$NAMESPACE"; then +if kubectl apply -f k8s/production/ -n "$NAMESPACE"; then success "Application deployed successfully" else error "Application deployment failed" fi # Deploy monitoring -if kubectl apply -f infrastructure/k8s/monitoring/ -n "$NAMESPACE"; then +if kubectl apply -f k8s/monitoring/ -n "$NAMESPACE"; then success "Monitoring stack deployed successfully" else error "Monitoring deployment failed" diff --git a/tests/unit/test_security_fixes.py b/tests/unit/test_security_fixes.py index 64bdae3b1..6b35be37d 100644 --- a/tests/unit/test_security_fixes.py +++ b/tests/unit/test_security_fixes.py @@ -6,6 +6,7 @@ Tests that require specific modules will skip if unavailable. """ +import json import os import re import shlex @@ -23,7 +24,9 @@ # Canonical location of the production container definition. Kept as a module # constant so the path is asserted in exactly one place; tests fail rather than # skip when it does not resolve. -PRODUCTION_DOCKERFILE = project_root / "infrastructure" / "docker" / "Dockerfile.production" +PRODUCTION_DOCKERFILE = ( + project_root / "infrastructure" / "docker" / "Dockerfile.production" +) # Matches a PEP 508-ish requirement with a `>=` floor, with or without extras # and surrounding quotes, e.g. `"uvicorn[standard]>=0.24.0"` or `fastapi`. @@ -134,6 +137,54 @@ def _parse_floors(text: str) -> dict: return floors +# ``[project] dependencies`` is a flat array of quoted PEP 508 strings. Anchoring +# on a line-initial ``dependencies = [`` selects it without matching the +# ``[project.optional-dependencies]`` tables, whose keys are indented (``dev = [``). +# Extracting textually rather than via tomllib/tomli keeps this guard working on +# the declared ``requires-python = ">=3.9"`` floor, where neither is guaranteed. +_PYPROJECT_DEPS_RE = re.compile( + r"^dependencies\s*=\s*\[(?P.*?)^\]", re.MULTILINE | re.DOTALL +) + + +def _pyproject_floors(text: str) -> dict: + """Extract ``{normalised_name: floor_key}`` from ``[project] dependencies``.""" + match = _PYPROJECT_DEPS_RE.search(text) + if not match: + return {} + entries = re.findall(r"[\"']([^\"']+)[\"']", match.group("body")) + return _parse_floors("\n".join(entries)) + + +def _canonical_floors() -> dict: + """Highest declared floor per distribution across *both* canonical manifests. + + ``requirements.txt`` and ``pyproject.toml`` disagree in places -- for example + ``python-dotenv`` is ``>=1.0.0`` in the former and ``>=1.2.2`` in the latter. + Comparing against only one of them lets Dockerfile.production sink to the + lower floor while still passing, so take the maximum of the two. + """ + requirements = project_root / "requirements.txt" + pyproject = project_root / "pyproject.toml" + assert requirements.exists(), "requirements.txt not found" + assert pyproject.exists(), "pyproject.toml not found" + + floors = _parse_floors(requirements.read_text()) + for name, floor in _pyproject_floors(pyproject.read_text()).items(): + current = floors.get(name) + if floor is not None and (current is None or floor > current): + floors[name] = floor + assert floors, "no canonical dependency floors parsed" + return floors + + +# Operators that let a failing ``pip install`` still produce exit 0: ``||`` +# supplies a fallback, ``;`` lets the next command's status win, and ``|`` +# discards the left-hand status without ``pipefail``. ``&&`` propagates failure +# and is therefore not listed. +_FAILURE_MASKING_OPERATORS = ("||", ";", "|") + + class TestAPIKeyExposureFix: """Test Issue 1: API Key Exposure Risk Fix""" @@ -340,22 +391,24 @@ def test_dockerfile_uses_nonroot_user(self): def test_dockerfile_production_pins_dependency_floors(self): """Every dependency installed by Dockerfile.production must carry a floor at least as high as the canonical declaration in - ``requirements.txt``. + ``requirements.txt`` *or* ``pyproject.toml``. This image installs a reduced runtime subset by name instead of using ``-r requirements.txt``, so advisory floors raised in the canonical - manifest do not propagate automatically. Without this guard the image + manifests do not propagate automatically. Without this guard the image silently drifts behind published security fixes -- which is how an unpinned ``python-multipart`` survived the floor bump for advisories 468-471 (see #1095). + + Both manifests are consulted because they disagree: ``python-dotenv`` + is ``>=1.0.0`` in requirements.txt but ``>=1.2.2`` in pyproject.toml, + so checking only the former would accept a Dockerfile that sank to the + lower, weaker floor. """ dockerfile = PRODUCTION_DOCKERFILE assert dockerfile.exists(), f"{dockerfile} not found" - requirements = project_root / "requirements.txt" - assert requirements.exists(), "requirements.txt not found" - - canonical = _parse_floors(requirements.read_text()) + canonical = _canonical_floors() installed = _installed_requirements( _pip_install_command(dockerfile.read_text()) ) @@ -373,18 +426,30 @@ def test_dockerfile_production_pins_dependency_floors(self): continue assert floor >= expected, ( f"{name} floor {_fmt(floor)} in Dockerfile.production is below " - f"the canonical requirements.txt floor {_fmt(expected)}" + f"the canonical floor {_fmt(expected)} declared in " + "requirements.txt/pyproject.toml" ) def test_dockerfile_production_does_not_swallow_install_failures(self): - """A ``|| echo`` fallback on the install step makes ``docker build`` - exit 0 with no packages installed, deferring the failure to runtime.""" + """Install failure must abort ``docker build``. + + A masked failure produces an image that builds cleanly with no packages + installed and then dies at runtime with ``ModuleNotFoundError``. Reject + the failure-masking shell operators outright rather than blacklisting + particular spellings -- ``|| echo``, ``|| true``, ``|| :``, + ``|| printf ...`` and ``; true`` are all the same defect. + """ command = _pip_install_command(PRODUCTION_DOCKERFILE.read_text()) assert command, "no pip install step found in Dockerfile.production" - assert "|| echo" not in command and "|| true" not in command, ( - "Dockerfile.production must not mask pip install failures; a " - "swallowed install produces an image that builds cleanly and then " - "fails at runtime with ModuleNotFoundError" + found = [ + operator + for operator in _FAILURE_MASKING_OPERATORS + if operator in shlex.split(command) + ] + assert not found, ( + f"Dockerfile.production pip install uses {found!r}, which can mask " + "a failed install; the build must fail instead of producing an " + "image that starts and then raises ModuleNotFoundError" ) def test_dockerfile_production_excludes_test_tooling(self): @@ -399,6 +464,60 @@ def test_dockerfile_production_excludes_test_tooling(self): "enlarges the runtime attack surface" ) + def test_dockerfile_production_entrypoint_module_exists(self): + """The ASGI module named in CMD must actually exist in this repo. + + Regression guard: the Dockerfile previously ran ``uvicorn server:app``, + but no root-level ``server.py`` has ever existed here, so every + container built from this file exited immediately. Resolve the target + against the source tree the image copies in (``/app/src``) rather than + importing it, so the assertion holds without the runtime dependencies + installed. + """ + text = PRODUCTION_DOCKERFILE.read_text() + + cmd_match = re.search(r"^CMD\s+(\[.*\])\s*$", text, re.MULTILINE) + assert cmd_match, "Dockerfile.production must declare a CMD" + + argv = json.loads(cmd_match.group(1)) + assert argv and argv[0] == "uvicorn", f"unexpected entrypoint: {argv}" + + target = next((a for a in argv[1:] if ":" in a and not a.startswith("-")), None) + assert target, f"no : target found in CMD: {argv}" + + module, _, attr = target.partition(":") + assert attr, f"CMD target {target!r} names no ASGI application attribute" + + # PYTHONPATH must include the directory the package actually lives in, + # otherwise the absolute imports inside it fail at startup. + pythonpath = re.search(r"^ENV\s+PYTHONPATH=(\S+)", text, re.MULTILINE) + assert pythonpath, ( + "Dockerfile.production must set PYTHONPATH; the application package " + "uses absolute imports rooted at the source directory" + ) + assert "/app/src" in pythonpath.group(1), ( + f"PYTHONPATH={pythonpath.group(1)!r} does not include /app/src, where " + "COPY . /app/ places the application package" + ) + + # /app/src maps to /src, so resolve the module there. + rel = Path(*module.split(".")) + candidates = [ + project_root / "src" / rel.with_suffix(".py"), + project_root / "src" / rel / "__init__.py", + ] + assert any(c.exists() for c in candidates), ( + f"CMD runs 'uvicorn {target}' but module {module!r} does not exist " + f"under {project_root / 'src'}; tried " + + ", ".join(str(c.relative_to(project_root)) for c in candidates) + ) + + source = next(c for c in candidates if c.exists()).read_text() + assert re.search(rf"^{re.escape(attr)}\s*=", source, re.MULTILINE), ( + f"module {module!r} exists but defines no module-level {attr!r}; " + f"'uvicorn {target}' would fail at startup" + ) + if __name__ == "__main__": pytest.main([__file__, "-v"]) From 1fa8a8674a35d958421846b3105c101ca48d68a7 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 31 Jul 2026 06:03:16 +0000 Subject: [PATCH 3/4] docs(triage): PR remediation run 2026-07-31 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Oldest-first scan of all 30 open PRs, each driven to a runbook terminal state. Headline: no open PR has an autonomous merge path. Every PR is a draft; substantive PRs pass functional CI but are held draft by the repo's agent-completion/truth-gate governance check (draft_pr, missing_agent_result, scope_drift, provenance disposition) — all human-only gates. CodeRabbit is excluded-by-label here, so the review loop is a no-op, and pushing commits would destroy exact-head evidence without touching the blocker. No merge/commit/review action taken. Also flags the truth-gate workflow erroring on invalid_payload for #995/#1038/#1040/#1047/#1049 (gate faulting, not evaluating). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01F6KJmeCL8QnxYEKuVimf9J --- docs/pr-remediation/2026-07-31-run.md | 108 ++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) create mode 100644 docs/pr-remediation/2026-07-31-run.md diff --git a/docs/pr-remediation/2026-07-31-run.md b/docs/pr-remediation/2026-07-31-run.md new file mode 100644 index 000000000..d9b5d5a72 --- /dev/null +++ b/docs/pr-remediation/2026-07-31-run.md @@ -0,0 +1,108 @@ +# PR Remediation & Publish Run — 2026-07-31 + +Automated execution of the PR Remediation & Publish Runbook against +`groupthinking/EventRelay`. GitHub surface: GitHub MCP (PR read + comment + +merge available). Oldest-first scan of all open PRs, each driven to a terminal +state per the runbook's Definition of Done (`MERGED` / `DEFERRED` / `HALTED`). + +## Headline finding + +**No open PR has an autonomous merge path.** All 30 open PRs are drafts. Every +substantive PR passes functional CI (CI, Coverage, CodeQL, Security Scan, Secret +Scan, Dependency Review) but is **deliberately held draft** and fails the +repo-defined `agent-completion/truth-gate/pr-` governance check. The gate's +failure reasons are human-only: `draft_pr`, `missing_agent_result`, +`missing_copilot_current_head_review`, `scope_drift`, `unresolved_review`, and an +unmanufacturable historical *provenance disposition*. Several gate runs also error +on `invalid_payload` (the gate workflow itself faulting, not evaluating rules). + +The PR bodies state this explicitly, e.g. #734 / #810 / #831: *"cannot be +retroactively manufactured by the connected `groupthinking` controller … Keep +draft pending an authorized legacy disposition … Final human review."* + +Consequences for an automated runbook: + +- **SCOPE GATE** (runbook §3.2): draft → `DEFERRED`. Applies to all 30. +- **PUBLISH GATE** (runbook §3.8): human by default; `auto_merge_policy` is + unset → conservative → no auto-merge to protected `main`. +- **CodeRabbit loop** (runbook §4) is a **no-op** here: CodeRabbit reports + `Review skipped: excluded by label configuration` on these PRs. +- Pushing commits or running review loops would **destroy the exact-head + evidence** each PR has assembled and would **not** touch the actual blocker + (human provenance disposition + final human review). + +Therefore no merge, commit, or review-loop action was taken against any PR. The +only remaining work is human — this run cannot advance it. + +## Terminal states (oldest first) + +| PR | Author | Title | CI (functional) | Truth-gate | Draft | Terminal state | +|----|--------|-------|-----------------|-----------|-------|----------------| +| 734 | groupthinking | fix(security): pin cloud callbacks vs DNS rebinding | green | fail: draft/provenance | yes | HALTED(human: provenance + review) | +| 810 | groupthinking | fix(security): sanitize API logs (CWE-117) | green | fail: draft/provenance | yes | HALTED(human: provenance + review) | +| 831 | groupthinking | fix(security): restore CWE-209 protections | green | fail: draft/provenance | yes | HALTED(human: provenance + review) | +| 869 | groupthinking | fix: harden API-cost webhook outbox (MYX-79) | CodeRabbit approved | fail: scope_drift/provenance | yes | HALTED(human: scope + review) | +| 903 | jules[bot] | fix(auth): restore Google OAuth in Vercel prod | — | fail: scope_drift/provenance | yes | HALTED(human: scope + review) | +| 906 | groupthinking | fix(ci): remediate PR #877 rollout gaps | — | fail: unresolved_review/scope | yes | HALTED(human: review) | +| 961 | jules[bot] | [DRAFT EVIDENCE] duplicate a11y proposal | — | — | yes | DEFERRED(draft, evidence/duplicate) | +| 987 | Copilot | [DRAFT EVIDENCE] unbound CI / module-shadowing | — | — | yes | DEFERRED(draft, evidence) | +| 995 | groupthinking | perf(mcp): reuse pooled aiohttp session | green | fail: invalid_payload | yes | HALTED(human: review; gate faulting) | +| 996 | groupthinking | fix(mcp): actually reuse pooled aiohttp session | — | — | yes | DEFERRED(draft, duplicate of #995) | +| 997 | jules[bot] | ⚡ Bolt: optimize AgentFlowVisualizer layout | — | — | yes | DEFERRED(draft) | +| 999 | dependabot | bump gh-aw-actions/setup 0.82.14→0.83.4 | green | — | yes | DEFERRED(draft; green, mergeable — needs human ready+merge) | +| 1000 | dependabot | bump actions/checkout 4.2.2→7.0.1 | green | — | yes | DEFERRED(draft; green — needs human ready+merge) | +| 1001 | dependabot | bump actions/setup-python 6→7 | **all green** | pass (not_applicable) | yes | DEFERRED(draft; fully green — needs human ready+merge) | +| 1002 | dependabot | bump locust 2.45→2.46 (dev) | green | — | yes | DEFERRED(draft; green — needs human ready+merge) | +| 1003 | dependabot | bump actions/github-script 8→9 | green | — | yes | DEFERRED(draft; green — needs human ready+merge) | +| 1004–1008 | dependabot | bump @opentelemetry/* in apps/web (5 PRs) | green | — | yes | DEFERRED(draft, labeled duplicate; needs human ready+merge) | +| 1020 | jules[bot] | perf: optimize call stacks / string allocs | — | — | yes | DEFERRED(draft) | +| 1022 | jules[bot] | perf(web): optimize bounding box calc | — | — | yes | DEFERRED(draft, duplicate) | +| 1038 | jules[bot] | feat: implement MCPOrchestrator._execute_on_server | green | fail: invalid_payload | yes | DEFERRED(draft, superseded by #1040) | +| 1040 | groupthinking | fix(mcp): green up #1038 E2E tests | green | fail: invalid_payload | yes | HALTED(human: review; gate faulting) | +| 1043 | jules[bot] | perf(web): optimize viewBox computation | — | — | yes | DEFERRED(draft, duplicate) | +| 1044 | groupthinking | docs(triage): PR remediation run 2026-07-27 | — | — | yes | DEFERRED(draft, prior run's report) | +| 1045 | jules[bot] | 🎨 Palette: dashboard focus-visible styling | — | — | yes | DEFERRED(draft, duplicate) | +| 1047 | jules[bot] | ci: suppress failure issues on no-op runs | — | fail: invalid_payload | yes | DEFERRED(draft; gate faulting) | +| 1049 | groupthinking | fix(a11y): dashboard focus contrast + coverage | green | fail: invalid_payload | yes | HALTED(human: review; gate faulting) | + +Rows marked "—" under CI were classified categorically from the uniform pattern +confirmed across the directly-inspected sample (734, 810, 831, 869, 903, 906, +995, 1001, 1040, 1047, 1049), not individually re-queried. + +## Staged next commands (human) + +The blocking gate is human by design. To advance any substantive PR a maintainer must: + +1. Provide the truth-gate's *historical provenance disposition* (or relax the gate + for legacy PRs predating their focused issues). +2. Mark the PR ready for review and complete final human review. +3. Merge to protected `main` per branch policy. + +Safe fast-path candidates (green CI, low risk, no provenance concern) — **human +mark-ready + merge**, e.g.: + +``` +# Fully green dependabot bump, truth-gate not-applicable: +# gh pr ready 1001 && gh pr merge 1001 --squash +# Same shape: 999, 1000, 1002, 1003, 1004, 1005, 1006, 1007, 1008 +``` + +## Infrastructure note for maintainers + +The `agent-completion/truth-gate` workflow is erroring with `invalid_payload` on +several PRs (#995, #1038, #1040, #1047, #1049) — the gate is faulting rather than +evaluating its rules. Worth a look independent of any single PR, since it blocks +the merge status of otherwise-green PRs. + +## Runbook parameters (as run) + +```yaml +github_surface: github-mcp # write-capable, confirmed +coderabbit_handle: "@coderabbitai" # no-op here: excluded by label config +auto_merge_policy: unset # → conservative: never auto-merge protected main +merge_method: unset +non_github_hosts: [] # no sub-agent spawn +``` + +_Terminal-state summary: 0 MERGED, ~17 DEFERRED, ~7 HALTED (all on human gates). +No autonomous merge path exists; remaining work is human-only._ From b31d38a8fdba1ee8f59fc885edac5ea32bf92ae7 Mon Sep 17 00:00:00 2001 From: Hayden <154503486+groupthinking@users.noreply.github.com> Date: Sat, 1 Aug 2026 18:57:16 -0500 Subject: [PATCH 4/4] perf: optimize LCP with avif/webp formats, preconnect headers, and eager video thumbnails --- apps/web/next.config.js | 1 + apps/web/src/app/dashboard/page.tsx | 7 +++++-- apps/web/src/app/layout.tsx | 2 ++ apps/web/src/components/VideoWorkflowStudio.tsx | 3 ++- 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/apps/web/next.config.js b/apps/web/next.config.js index bd9197027..bf93069f7 100644 --- a/apps/web/next.config.js +++ b/apps/web/next.config.js @@ -52,6 +52,7 @@ const nextConfig = { root: path.resolve(__dirname, '../..'), }, images: { + formats: ['image/avif', 'image/webp'], remotePatterns: [ { protocol: 'https', hostname: 'uvai.io' }, { protocol: 'https', hostname: 'api.uvai.io' }, diff --git a/apps/web/src/app/dashboard/page.tsx b/apps/web/src/app/dashboard/page.tsx index 6963c847c..7e8a66bdf 100644 --- a/apps/web/src/app/dashboard/page.tsx +++ b/apps/web/src/app/dashboard/page.tsx @@ -28,9 +28,11 @@ const DashboardSplitView = dynamic( // ============================================ function VideoCard({ video, + priority = false, onClick }: { video: Video; + priority?: boolean; onClick: () => void; }) { return ( @@ -49,6 +51,7 @@ function VideoCard({ src={video.thumbnail} alt={video.title} fill + priority={priority} className="object-cover transition-transform duration-500 group-hover:scale-105" sizes="(max-width: 768px) 100vw, 33vw" /> @@ -296,8 +299,8 @@ function DashboardContent() { ) : (
- {filteredVideos.map((video) => ( - selectVideo(video.id)} /> + {filteredVideos.map((video, index) => ( + selectVideo(video.id)} /> ))}
)} diff --git a/apps/web/src/app/layout.tsx b/apps/web/src/app/layout.tsx index f003862bb..f63049d19 100644 --- a/apps/web/src/app/layout.tsx +++ b/apps/web/src/app/layout.tsx @@ -97,6 +97,8 @@ export default function RootLayout({ + + ) : (