feat: container build context config + full build log preservation (#1110)

myakove · web-flow · commit d5390cc3a853 · 2026-06-10T19:17:47.000+03:00
<h3>PR Summary by Qodo</h3> Add container build context option and preserve full redacted build logs <code>✨ Enhancement</code> <code>🐞 Bug fix</code> <code>🧪 Tests</code> <code>⚙️ Configuration changes</code> <code>📝 Documentation</code> <code>🕐 20-40 Minutes</code> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <h3>Walkthroughs</h3> <details open> <summary>User Description</summary> <br/> ## Summary Two container build improvements: ### Full build log preservation (#1090) - Log full redacted build output before truncation for debugging - Extract `_redact_output()` helper for reusable secret redaction - Truncation threshold uses ANSI-stripped lengths consistently ### Docker build context from non-root directory (#1065) - Add `context` config option under `container` for non-root build context - Path traversal validation prevents context escaping worktree root - Schema pattern constraint rejects obviously bad values Closes #1090 Closes #1065 </details> <details open> <summary>AI Description</summary> <br/> <pre> • Add <b><i>container.context</i></b> config to build images from a repo subdirectory. • Validate context stays within the checked-out worktree to prevent path traversal. • Log full redacted build output before GitHub check-run truncation for debugging. </pre> </details> <details> <summary>Diagram</summary> <br/> ```mermaid graph TD cfg["Container config"] --> loader["Config loader"] --> runner["RunnerHandler"] --> podman["Podman build"] runner --> check["CheckRunHandler"] --> gh["GitHub Checks API"] ``` </details> <details> <summary>High-Level Assessment</summary> <br/> The following are alternative approaches to this PR: <details> <summary><b>1. Use commonpath/is_relative_to for context validation</b></summary> - ➕ Less error-prone than string prefix checks - ➕ More readable intent: “resolved path is within worktree” - ➖ `Path.is_relative_to` requires Python 3.9+; `commonpath` still needs careful normalization/symlink handling </details> <details> <summary><b>2. Persist full build logs as artifacts instead of debug logs</b></summary> - ➕ Avoids flooding debug logs while still preserving full output - ➕ Makes large logs easier to browse/download - ➖ Requires artifact storage/retention and plumbing (upload, linking, cleanup) - ➖ More moving parts than a server-side debug log </details> **Recommendation:** The PR’s approach is reasonable: config-driven context selection with realpath-based containment checks, plus pre-truncation redacted logging gated on truncation. If context validation edge cases become a concern, consider switching to `os.path.commonpath` (or `Path.resolve().is_relative_to()` where supported) to express containment more directly. </details> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <h3>File Changes</h3> <details> <summary><strong>Enhancement</strong> (2)</summary> <blockquote> <details> <summary><strong>github_api.py</strong> <code>Read container context from repo config into webhook runtime</code> <code>+1/-0</code></summary> <br/> >Read container context from repo config into webhook runtime > ><pre> >• Extends repository config ingestion to populate &#x27;container_context&#x27; from &#x27;container.context&#x27;, defaulting to empty string when unset. ></pre> > ><a href='https://github.com/myk-org/github-webhook-server/pull/1110/files#diff-7c5f6dfcadb38e75c2d0f1d418ba1a861cc9f6c0efe72905a250e9f43a6cfdcf'>webhook_server/libs/github_api.py</a> <hr/> </details> </blockquote> <blockquote> <details> <summary><strong>runner_handler.py</strong> <code>Support building containers from a configured subdirectory context</code> <code>+26/-6</code></summary> <br/> >Support building containers from a configured subdirectory context > ><pre> >• Uses &#x27;container_context&#x27; to select the podman build context directory while keeping the Dockerfile path rooted at the worktree. Adds realpath-based validation to reject contexts that resolve outside the checked-out worktree and fails the check run instead of building. ></pre> > ><a href='https://github.com/myk-org/github-webhook-server/pull/1110/files#diff-0cb54c95cafda12d8d169c7b03ac484738f4cf925c22f6e6b8b8c5db0730ce42'>webhook_server/libs/handlers/runner_handler.py</a> <hr/> </details> </blockquote> </details> <details> <summary><strong>Bug fix</strong> (1)</summary> <blockquote> <details> <summary><strong>check_run_handler.py</strong> <code>Preserve full redacted output in logs before check-run truncation</code> <code>+38/-18</code></summary> <br/> >Preserve full redacted output in logs before check-run truncation > ><pre> >• Extracts a reusable &#x27;_redact_output()&#x27; helper and uses it both for returned check-run text and for a new debug log that captures the full redacted output when truncation is required. Truncation detection is based on ANSI-stripped lengths to match what is sent to GitHub. ></pre> > ><a href='https://github.com/myk-org/github-webhook-server/pull/1110/files#diff-6b505edd8695d646f7fec8e71517af3e42df16342d1409a74c8212063ab26f56'>webhook_server/libs/handlers/check_run_handler.py</a> <hr/> </details> </blockquote> </details> <details> <summary><strong>Tests</strong> (3)</summary> <blockquote> <details> <summary><strong>test_check_run_handler.py</strong> <code>Add tests for pre-truncation full-output logging and redaction</code> <code>+53/-0</code></summary> <br/> >Add tests for pre-truncation full-output logging and redaction > ><pre> >• Adds coverage ensuring the full output debug log is emitted only when truncation occurs and that secrets are redacted in the logged content. ></pre> > ><a href='https://github.com/myk-org/github-webhook-server/pull/1110/files#diff-ab2e5c2c544a42d78004d8aa40f768b38426f08f365090a75fdb5e4fad57a7cc'>webhook_server/tests/test_check_run_handler.py</a> <hr/> </details> </blockquote> <blockquote> <details> <summary><strong>test_repo_data_from_config.py</strong> <code>Test container context config parsing and defaults</code> <code>+27/-0</code></summary> <br/> >Test container context config parsing and defaults > ><pre> >• Asserts &#x27;container_context&#x27; defaults to empty string and is correctly read when supplied under the &#x27;container&#x27; config block. ></pre> > ><a href='https://github.com/myk-org/github-webhook-server/pull/1110/files#diff-4e76f4b4427b5be5ab45623aa0f35a24a51b526903e36dbf3ea347757ae1cc62'>webhook_server/tests/test_repo_data_from_config.py</a> <hr/> </details> </blockquote> <blockquote> <details> <summary><strong>test_runner_handler.py</strong> <code>Test build context selection and traversal rejection in container builds</code> <code>+96/-0</code></summary> <br/> >Test build context selection and traversal rejection in container builds > ><pre> >• Adds tests verifying the podman command uses the configured subdirectory as context, rejects traversal attempts that escape the worktree, and defaults to repo root when context is empty. ></pre> > ><a href='https://github.com/myk-org/github-webhook-server/pull/1110/files#diff-beb446b71d75b7a427d17e48e75971c270de4820cb9786a12af9e4f988887269'>webhook_server/tests/test_runner_handler.py</a> <hr/> </details> </blockquote> </details> <details> <summary><strong>Documentation</strong> (1)</summary> <blockquote> <details> <summary><strong>config.yaml</strong> <code>Document optional container build context setting</code> <code>+1/-0</code></summary> <br/> >Document optional container build context setting > ><pre> >• Adds an example/commented &#x27;context&#x27; key under &#x27;container&#x27; to demonstrate building from a subdirectory instead of repo root. ></pre> > ><a href='https://github.com/myk-org/github-webhook-server/pull/1110/files#diff-e3ac27ce128a0ddae53723bc8c3133257530a13c00f97c9edfeef152ebc3b8ce'>examples/config.yaml</a> <hr/> </details> </blockquote> </details> <details> <summary><strong>Other</strong> (1)</summary> <blockquote> <details> <summary><strong>schema.yaml</strong> <code>Add &#x27;container.context&#x27; to configuration schema with input constraints</code> <code>+8/-0</code></summary> <br/> >Add &#x27;container.context&#x27; to configuration schema with input constraints > ><pre> >• Introduces a &#x27;context&#x27; string option under &#x27;container&#x27; with a conservative allowed-character pattern and a default of empty string (repo root). ></pre> > ><a href='https://github.com/myk-org/github-webhook-server/pull/1110/files#diff-0eaf85d7f2a5888c61710a31c06434f63fe254f177f3df114332204780388f67'>webhook_server/config/schema.yaml</a> <hr/> </details> </blockquote> </details> <img src="https://www.qodo.ai/wp-content/uploads/2025/11/light-grey-line.svg" height="10%" alt="Grey Divider"> <a href="https://www.qodo.ai"><img src="https://www.qodo.ai/wp-content/uploads/2025/03/qodo-logo.svg" width="80" alt="Qodo Logo"></a>
diff --git a/examples/config.yaml b/examples/config.yaml
@@ -175,6 +175,7 @@ repositories:
       repository: <registry_repository_full_path>
       tag: <image_tag>
       release: true # Push image to registry on new release with release as the tag
+      # context: src  # Subdirectory to use as Docker build context (default: repo root)
       build-args: # build args to send to podman build command
         - my-build-arg1=1
         - my-build-arg2=2
diff --git a/webhook_server/config/schema.yaml b/webhook_server/config/schema.yaml
@@ -392,6 +392,14 @@ properties:
               items:
                 type: string
               description: Additional arguments for build command
+            context:
+              type: string
+              pattern: "^[a-zA-Z0-9._/-]*$"
+              description: |
+                Subdirectory to use as the Docker build context (relative to repo root).
+                Default is empty string (repo root). Example: "src" builds from <repo>/src/.
+                Only alphanumeric characters, dots, hyphens, underscores, and forward slashes allowed.
+              default: ""
             oci-annotations:
               type: object
               description: OCI image annotations (https://github.com/opencontainers/image-spec/blob/main/annotations.md)
diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py
@@ -818,6 +818,7 @@ def _repo_data_from_config(self, repository_config: dict[str, Any]) -> None:
             self.container_repository_password: str = self.build_and_push_container["password"]
             self.container_repository: str = self.build_and_push_container["repository"]
             self.dockerfile: str = self.build_and_push_container.get("dockerfile", "Dockerfile")
+            self.container_context: str = self.build_and_push_container.get("context", "")
             self.container_tag: str = self.build_and_push_container.get("tag", "latest")
             _build_args = self.build_and_push_container.get("build-args", [])
             _cmd_args = self.build_and_push_container.get("args", [])
diff --git a/webhook_server/libs/handlers/check_run_handler.py b/webhook_server/libs/handlers/check_run_handler.py
@@ -1,4 +1,5 @@
 import asyncio
+import logging
 from typing import TYPE_CHECKING, Any, NotRequired, TypedDict
 
 from github.CheckRun import CheckRun
@@ -202,17 +203,57 @@ async def set_check_run_status(
                 log_prefix=self.log_prefix,
             )
 
+    def _redact_output(self, text: str) -> str:
+        """Replace sensitive tokens, passwords, and credentials with *****."""
+        _hased_str = "*****"
+
+        pypi_config = self.github_webhook.pypi
+        if isinstance(pypi_config, dict):
+            pypi_token = pypi_config.get("token")
+            if pypi_token:
+                text = text.replace(pypi_token, _hased_str)
+
+        if getattr(self.github_webhook, "container_repository_username", None):
+            text = text.replace(self.github_webhook.container_repository_username, _hased_str)
+
+        if getattr(self.github_webhook, "container_repository_password", None):
+            text = text.replace(self.github_webhook.container_repository_password, _hased_str)
+
+        if self.github_webhook.token:
+            text = text.replace(self.github_webhook.token, _hased_str)
+
+        return text
+
     def get_check_run_text(self, err: str, out: str) -> str:
         # Strip ANSI escape codes from output to prevent scrambled characters in GitHub UI
         err_clean = strip_ansi_codes(err)
         out_clean = strip_ansi_codes(out)
 
+        # Redact BEFORE truncation to prevent partial secret leaks at truncation boundaries
+        err_clean = self._redact_output(err_clean)
+        out_clean = self._redact_output(out_clean)
+
         # GitHub limit is 65535 characters, but we use 65534 to be safe
         # We reserve space for the markdown wrapper: ```\n{err}\n\n{out}\n```
         # Wrapper overhead = len("```\n\n\n```") = 10 characters
         MAX_LEN = 65534
         WRAPPER_OVERHEAD = 10
 
+        # Check if truncation will be needed (using ANSI-stripped, redacted lengths)
+        full_clean_len = len(err_clean) + len(out_clean) + WRAPPER_OVERHEAD
+        will_truncate = full_clean_len > MAX_LEN
+
+        # Log the full redacted output BEFORE truncation so server logs preserve
+        # the complete build output for debugging.
+        # err_clean/out_clean are already redacted at this point.
+        if will_truncate and self.logger.isEnabledFor(logging.DEBUG):
+            full_output = f"```\n{err_clean}\n\n{out_clean}\n```"
+            self.logger.debug(
+                f"{self.log_prefix} Full check run output ({full_clean_len} chars, "
+                f"will be truncated to {MAX_LEN} for GitHub). "
+                f"Redacted full output:\n{full_output}"
+            )
+
         # Prepare error part first - we want to preserve it as much as possible
         # If error itself is huge, we might need to truncate it too, but usually it's small
         # If error + wrapper > MAX_LEN, we truncate error
@@ -241,24 +282,9 @@ def get_check_run_text(self, err: str, out: str) -> str:
 
         _output = f"```\n{err_clean}\n\n{out_clean}\n```"
 
-        _hased_str = "*****"
-
-        pypi_config = self.github_webhook.pypi
-        if isinstance(pypi_config, dict):
-            pypi_token = pypi_config.get("token")
-            if pypi_token:
-                _output = _output.replace(pypi_token, _hased_str)
-
-        if getattr(self.github_webhook, "container_repository_username", None):
-            _output = _output.replace(self.github_webhook.container_repository_username, _hased_str)
-
-        if getattr(self.github_webhook, "container_repository_password", None):
-            _output = _output.replace(self.github_webhook.container_repository_password, _hased_str)
-
-        if self.github_webhook.token:
-            _output = _output.replace(self.github_webhook.token, _hased_str)
-
-        return _output
+        # Final redaction pass is idempotent but catches any secrets
+        # introduced by the markdown wrapper itself (defensive)
+        return self._redact_output(_output)
 
     async def is_check_run_in_progress(self, check_run: str) -> bool:
         if self.github_webhook.last_commit:
diff --git a/webhook_server/libs/handlers/runner_handler.py b/webhook_server/libs/handlers/runner_handler.py
@@ -395,11 +395,42 @@ async def run_build_container(
             is_merged=is_merged,
             tag_name=tag,
         ) as (success, worktree_path, out, err):
+            output: CheckRunOutput = {
+                "title": "Build container",
+                "summary": "",
+                "text": None,
+            }
+
+            if not success:
+                output["text"] = self.check_run_handler.get_check_run_text(out=out, err=err)
+                if pull_request and set_check:
+                    await self.check_run_handler.set_check_failure(name=BUILD_CONTAINER_STR, output=output)
+                return
+
             # Build container build command with worktree path
+            # Use configured context subdirectory for build context (default: repo root)
+            _context = self.github_webhook.container_context
+            if _context:
+                resolved_context = os.path.realpath(os.path.join(worktree_path, _context))
+                resolved_worktree = os.path.realpath(worktree_path)
+                is_under_worktree = resolved_context.startswith(resolved_worktree + os.sep)
+                if not is_under_worktree and resolved_context != resolved_worktree:
+                    self.logger.error(
+                        f"{self.log_prefix} Container context '{_context}' resolves outside "
+                        f"worktree ({resolved_context}), rejecting for security"
+                    )
+                    output["text"] = f"Container build context '{_context}' escapes repository root"
+                    if pull_request and set_check:
+                        await self.check_run_handler.set_check_failure(name=BUILD_CONTAINER_STR, output=output)
+                    return
+                build_context: str = resolved_context
+            else:
+                build_context = worktree_path
+
             build_cmd: str = (
                 f"--network=host {no_cache} -f "
                 f"{worktree_path}/{self.github_webhook.dockerfile} "
-                f"{worktree_path} -t {_container_repository_and_tag}"
+                f"{build_context} -t {_container_repository_and_tag}"
             )
 
             oci_annotation_flags = self._build_oci_annotations(pull_request=pull_request, tag=tag)
@@ -419,17 +450,6 @@ async def run_build_container(
             podman_build_cmd: str = f"podman build {build_cmd}"
             self.logger.debug(f"{self.log_prefix} Podman build command to run: {podman_build_cmd}")
 
-            output: CheckRunOutput = {
-                "title": "Build container",
-                "summary": "",
-                "text": None,
-            }
-            if not success:
-                output["text"] = self.check_run_handler.get_check_run_text(out=out, err=err)
-                if pull_request and set_check:
-                    await self.check_run_handler.set_check_failure(name=BUILD_CONTAINER_STR, output=output)
-                return
-
             build_rc, build_out, build_err = await self.run_podman_command(
                 command=podman_build_cmd,
             )
diff --git a/webhook_server/tests/test_check_run_handler.py b/webhook_server/tests/test_check_run_handler.py
@@ -455,6 +455,59 @@ def test_get_check_run_text_long_length(self, check_run_handler: CheckRunHandler
         # Verify the fix: it should end with the code block closer
         assert result.endswith("\n```")
 
+    def test_get_check_run_text_logs_full_redacted_output_before_truncation(
+        self, check_run_handler: CheckRunHandler
+    ) -> None:
+        """Test that full redacted output is logged via debug before truncation.
+
+        The debug log must fire AFTER redaction to avoid leaking secrets to log
+        aggregation systems, and BEFORE truncation so the complete build output
+        is preserved in server logs.
+        """
+        # Create text that exceeds 65534 characters to trigger truncation
+        long_err = "E" * 10000
+        long_out = "O" * 60000
+
+        check_run_handler.get_check_run_text(long_err, long_out)
+
+        # Verify debug was called with the full output
+        debug_calls = [str(call) for call in check_run_handler.logger.debug.call_args_list]
+        full_output_logged = any(
+            "Full check run output" in call and "will be truncated" in call for call in debug_calls
+        )
+        assert full_output_logged, "Expected debug log with full output before truncation"
+
+    def test_get_check_run_text_full_log_has_secrets_redacted(self, check_run_handler: CheckRunHandler) -> None:
+        """Test that the full output debug log redacts secrets before logging."""
+        # Build output that contains secrets and exceeds truncation threshold
+        # Total (err_clean + out_clean + WRAPPER_OVERHEAD=10) must exceed MAX_LEN=65534
+        long_out = "O" * 66000
+        err_with_secret = "Error: auth failed with token test-token and user test-user"  # pragma: allowlist secret
+
+        check_run_handler.get_check_run_text(err_with_secret, long_out)
+
+        # Verify the debug log does NOT contain the raw secrets
+        debug_calls = [str(call) for call in check_run_handler.logger.debug.call_args_list]
+        full_log_calls = [c for c in debug_calls if "Full check run output" in c]
+        assert full_log_calls, "Expected full output debug log"
+        for call in full_log_calls:
+            assert "test-token" not in call, "Secret token leaked in debug log"
+            assert "test-user" not in call, "Username leaked in debug log"
+            assert "*****" in call, "Expected redacted placeholder in debug log"
+
+    def test_get_check_run_text_no_log_when_not_truncated(self, check_run_handler: CheckRunHandler) -> None:
+        """Test that no full output debug log is emitted when output fits within limit."""
+        err = "Short error"
+        out = "Short output"
+
+        check_run_handler.logger.debug.reset_mock()
+        check_run_handler.get_check_run_text(err, out)
+
+        # Debug should NOT contain "Full check run output" for short text
+        debug_calls = [str(call) for call in check_run_handler.logger.debug.call_args_list]
+        full_output_logged = any("Full check run output" in call for call in debug_calls)
+        assert not full_output_logged, "Should not log full output when no truncation needed"
+
     def test_get_check_run_text_token_replacement(self, check_run_handler: CheckRunHandler) -> None:
         """Test that sensitive tokens are replaced in check run text."""
         err = "Error with token: test-token"
diff --git a/webhook_server/tests/test_repo_data_from_config.py b/webhook_server/tests/test_repo_data_from_config.py
@@ -19,6 +19,7 @@ def test_repo_data_from_config_repository_found(process_github_webhook):
     assert process_github_webhook.container_repository_password == "registry_password"  # pragma: allowlist secret
     assert process_github_webhook.container_repository == "registry_repository_full_path"
     assert process_github_webhook.dockerfile == "Dockerfile"
+    assert process_github_webhook.container_context == ""
     assert process_github_webhook.container_tag == "image_tag"
     assert process_github_webhook.container_build_args == ["my-build-arg1=1", "my-build-arg2=2"]
     assert process_github_webhook.container_command_args == ["--format docker"]
@@ -29,6 +30,32 @@ def test_repo_data_from_config_repository_found(process_github_webhook):
     assert process_github_webhook.minimum_lgtm == 0
 
 
+def test_container_context_from_config(process_github_webhook) -> None:
+    """Test that container context is read from repository config."""
+    original_get_value = process_github_webhook.config.get_value
+
+    def patched_get_value(value: str, *args: Any, **kwargs: Any) -> Any:
+        if value == "container":
+            return {
+                "username": "user",
+                "password": "pass",  # pragma: allowlist secret
+                "repository": "registry/repo",
+                "context": "src/app",
+            }
+        return original_get_value(value, *args, **kwargs)
+
+    with patch.object(process_github_webhook.config, "get_value", side_effect=patched_get_value):
+        process_github_webhook._repo_data_from_config(repository_config={})
+
+    assert process_github_webhook.container_context == "src/app"
+
+
+def test_container_context_defaults_to_empty_string(process_github_webhook) -> None:
+    """Test that container context defaults to empty string when not set."""
+    process_github_webhook._repo_data_from_config(repository_config={})
+    assert process_github_webhook.container_context == ""
+
+
 def test_tox_python_version_nested_no_deprecation_warning(process_github_webhook, caplog):
     """When 'python-version' is set under 'tox', no deprecation warning should be logged."""
     with caplog.at_level(python_logging.WARNING):
diff --git a/webhook_server/tests/test_runner_handler.py b/webhook_server/tests/test_runner_handler.py
