gh17 docker mount coverity #22

BenjiMilan · 2026-01-28T14:34:31Z

Closes #17

Mount coverity into dockers instead of codesonar

rdkcmf-jenkins · 2026-01-28T14:35:26Z

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

Protex Server Path: /home/blackduck/github/sc/22/rdkcentral/sc
Commit: db28c1d

Report detail: gist'

Copilot

Pull request overview

Updates sc docker run to support Coverity by mounting the host’s Coverity install into containers (replacing prior CodeSonar-specific mounting) and adjusting the container command invocation accordingly.

Changes:

Add automatic detection of /opt/coverity (symlink) and mount its resolved target into the container.
Modify the generated container command to export Coverity’s bin directory onto PATH.
Remove the previous CodeSonar mount / environment injection.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-28T14:37:53Z

src/sc/docker/docker.py

+        coverity_dir = Path('/opt/coverity').resolve()
+        if Path('/opt/coverity').is_symlink() and Path(coverity_dir).exists():
+            docker_args += ['-v', f"{coverity_dir}:{coverity_dir}"]


The Coverity mount currently mounts the resolved target path back onto the same resolved path inside the container, but _generate_bash_command later hard-codes /opt/coverity/bin into PATH. If /opt/coverity is a symlink (per the issue), the container may not have /opt/coverity pointing at the mounted directory, so /opt/coverity/bin won’t exist. Consider mounting the resolved host directory to /opt/coverity in-container (and optionally mark it read-only), or otherwise ensure /opt/coverity exists in the container and points at the mounted path.

Suggested change

coverity_dir = Path('/opt/coverity').resolve()

if Path('/opt/coverity').is_symlink() and Path(coverity_dir).exists():

docker_args += ['-v', f"{coverity_dir}:{coverity_dir}"]

coverity_path = Path('/opt/coverity')

coverity_dir = coverity_path.resolve()

if coverity_path.exists():

docker_args += ['-v', f"{coverity_dir}:/opt/coverity:ro"]

Copilot · 2026-01-28T14:37:53Z

src/sc/docker/docker.py

+        coverity_dir = Path('/opt/coverity').resolve()
+        if Path('/opt/coverity').is_symlink() and Path(coverity_dir).exists():
+            docker_args += ['-v', f"{coverity_dir}:{coverity_dir}"]


This PR/issue description says Coverity should be mounted “if arg is supplied”, but this code mounts Coverity automatically whenever /opt/coverity is a symlink on the host. There’s no corresponding CLI option in sc/docker_cli.py to enable/disable this behavior. Either add an explicit flag/option and thread it through, or update the PR description/issue expectation to match the implemented behavior.

Copilot · 2026-01-28T14:37:54Z

src/sc/docker/docker.py

+        bash_command = f"bash -c 'source /usr/local/bin/bashext.sh && export PATH=/opt/coverity/bin:$PATH && cd {Path.cwd()}; {' '.join(command)}'"
        if x11 and os.getenv("DISPLAY"):
            bash_command += self._setup_xauth(container_name)
-        return bash_command + " ".join(command)
-    
+        return bash_command


bash_command opens a single-quoted script (bash -c '...'), but when X11 is enabled you append the xauth setup string after that closing quote, which produces an invalid command string and likely breaks --x11 runs. Build the full script (including xauth setup) inside the bash -c payload, or pass bash, -c, and the script as separate docker args so quoting is handled correctly.

Copilot · 2026-01-28T14:37:54Z

src/sc/docker/docker.py

+        bash_command = f"bash -c 'source /usr/local/bin/bashext.sh && export PATH=/opt/coverity/bin:$PATH && cd {Path.cwd()}; {' '.join(command)}'"
        if x11 and os.getenv("DISPLAY"):


Using single quotes around the entire bash -c payload prevents shell expansion inside the user-provided command (e.g., echo $HOME won’t expand) and will break commands containing single quotes. Prefer passing the command as separate argv items (exec form) or construct the bash -c payload with proper escaping (or use double quotes where appropriate) to preserve expected shell behavior.

rdkcmf-jenkins · 2026-01-28T17:11:53Z

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

Protex Server Path: /home/blackduck/github/sc/22/rdkcentral/sc
Commit: 65d5abb

Report detail: gist'

rdkcmf-jenkins · 2026-01-29T10:26:57Z

b'## WARNING: A Blackduck scan failure has been waived

A prior failure has been upvoted

Upvote reason: OK
Commit: 65d5abb
'

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-29T14:26:53Z

src/sc/docker/docker.py

+
    def _execute_docker_run(self, docker_command: list, image: str):
        """Prints and executes the docker run command."""
+        print(docker_command)


print(docker_command) looks like a debug leftover and will emit an unformatted Python list in addition to the existing click.secho output. Consider removing it or using the existing logging/output mechanism so CLI output stays stable.

Suggested change

print(docker_command)

Copilot · 2026-01-29T14:26:54Z

src/sc/docker/docker.py

+        if cmd:
+            parts.append(cmd)
+
+        bash = "bash -c '" + " && ".join(parts) + "'"


_generate_bash_command wraps the full command string in single quotes (bash -c '...') but does not escape embedded single quotes coming from setup_cmds, Path.cwd(), or command. This will break valid inputs (and can change semantics) when any part contains a '. Build the shell command using proper escaping (e.g., quoting each segment/argument) or avoid the nested single-quote construction.

Suggested change

bash = "bash -c '" + " && ".join(parts) + "'"

command_str = " && ".join(parts)

# Escape single quotes so the command is safe inside a single-quoted bash -c argument.

escaped_command_str = command_str.replace("'", "'\"'\"'")

bash = "bash -c '" + escaped_command_str + "'"

Copilot · 2026-01-29T14:26:54Z

src/sc/docker/docker.py

        if xauth_line:
            hostname = os.getenv("HOSTNAME")
            xauth_line = f"{xauth_line}/{hostname}/{container_name}"
-            return f"touch {Path.home()}/.Xauthority; xauth add {xauth_line}; "
-
+            prelude_cmd = [f"touch {Path.home()}/.Xauthority", f"xauth add {xauth_line}"]
+
+        return docker_args, prelude_cmd


_add_x11_support can raise an UnboundLocalError: prelude_cmd is only assigned inside if xauth_line: but is returned unconditionally. Initialize prelude_cmd to an empty list before the try/except and return docker_args, [] when no xauth entry is found.

Copilot · 2026-01-29T14:26:54Z

src/sc/docker/docker.py

+    def _add_coverity_mount(self) -> tuple[list[str], list[str]]:
+        coverity_dir = Path('/opt/coverity').resolve()
+        if Path('/opt/coverity').is_symlink() and Path(coverity_dir).exists():
+            coverity_args = ['-v', f"{coverity_dir}:{coverity_dir}"]


_add_coverity_mount exports PATH=/opt/coverity/bin:$PATH but the volume mount is created at {coverity_dir}:{coverity_dir} (the symlink target), not at /opt/coverity. If /opt/coverity doesn’t exist inside the container, the PATH update points to a non-mounted directory and Coverity won’t be usable. Mount the host Coverity install to /opt/coverity in the container (or update PATH to use the mounted destination path consistently).

Suggested change

coverity_args = ['-v', f"{coverity_dir}:{coverity_dir}"]

coverity_args = ['-v', f"{coverity_dir}:/opt/coverity"]

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-30T11:21:17Z

src/sc/docker/docker.py

+            coverity_args = ['-v', f"{coverity_dir}:{coverity_dir}"]
+            coverity_prelude = [f"export PATH={coverity_dir}:$PATH"]


_add_coverity_mount() resolves /opt/coverity and then bind-mounts the resolved path into the container at the same resolved path. This does not satisfy the requirement to mount /opt/coverity inside the container (tools inside the container will look for /opt/coverity, not the host’s resolved target path). Consider mounting the resolved target to /opt/coverity in the container (or simply mounting /opt/coverity:/opt/coverity if Docker resolves the symlink correctly). Also, the PATH prelude currently adds the install root to PATH; typically the executables are under a bin/ directory, so PATH should likely include the correct bin location under the mounted /opt/coverity.

Suggested change

coverity_args = ['-v', f"{coverity_dir}:{coverity_dir}"]

coverity_prelude = [f"export PATH={coverity_dir}:$PATH"]

# Bind-mount the resolved host directory into the container at /opt/coverity

coverity_args = ['-v', f"{coverity_dir}:/opt/coverity"]

# Add the Coverity bin directory under the mounted /opt/coverity to PATH

coverity_prelude = ["export PATH=/opt/coverity/bin:$PATH"]

Copilot · 2026-01-30T11:21:18Z

src/sc/docker/docker.py

+        coverity_args, coverity_setup_cmds = self._add_coverity_mount()
+        docker_args += coverity_args
+        setup_cmds += coverity_setup_cmds


Coverity is mounted unconditionally whenever /opt/coverity exists as a symlink on the host. Issue #17/PR description indicate the mount should happen only when an argument/flag is supplied. If the CLI is intended to control this behavior, thread a boolean/option down into _generate_docker_run_command() and guard _add_coverity_mount() accordingly.

Copilot · 2026-01-30T11:21:18Z

src/sc/docker/docker.py

+        cmd = " ".join(command)
+        parts = [
+            "source /usr/local/bin/bashext.sh",
+            *setup_cmds,
+            f"cd {Path.cwd()}"


_generate_bash_command() builds cmd via " ".join(command), which loses correct argument quoting/escaping when any argument contains spaces or shell metacharacters. Since this function is explicitly shell-escaping, use a join that preserves argument boundaries (e.g., shlex.join(command)), and also ensure the cd target is shell-quoted so paths with spaces don’t break.

Suggested change

cmd = " ".join(command)

parts = [

"source /usr/local/bin/bashext.sh",

*setup_cmds,

f"cd {Path.cwd()}"

cmd = shlex.join(command)

parts = [

"source /usr/local/bin/bashext.sh",

*setup_cmds,

f"cd {shlex.quote(str(Path.cwd()))}"

Copilot · 2026-01-30T11:21:18Z

src/sc/docker/docker.py

+
        if xauth_line:
            hostname = os.getenv("HOSTNAME")
            xauth_line = f"{xauth_line}/{hostname}/{container_name}"


In _add_x11_support(), xauth_line is taken from xauth list (which already returns a complete xauth add-compatible record) and then has /{hostname}/{container_name} appended. This appears to corrupt the record by modifying the cookie/token portion rather than adjusting only the display name. If the goal is to rewrite the first field (display) for the container, parse the output and replace just that field; otherwise, pass the xauth list output through unchanged.

Suggested change

xauth_line = f"{xauth_line}/{hostname}/{container_name}"

if hostname:

parts = xauth_line.split()

if len(parts) >= 3:

parts[0] = f"{parts[0]}/{hostname}/{container_name}"

xauth_line = " ".join(parts)

TB-1993

Return this as tuples only to reconnect them after is pretty horrible.

I'd suggest either:

Make use of pythons pass by object reference approach and pass the docker_arg list into the functions and make the necessary modifications to it.
Keep the args for docker run and the args to the commands to be run in the container in two separate lists.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 10 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-04T16:13:58Z

src/sc/docker/docker.py

+    def _add_coverity_mount(self, docker_run: DockerRunBuilder):
+        coverity_dir = Path('/opt/coverity').resolve()
+        if Path('/opt/coverity').is_symlink() and Path(coverity_dir).exists():
+            docker_run.add_volume_mount(f"{coverity_dir}:{coverity_dir}")
+            docker_run.add_bash_command(f"export PATH={coverity_dir}:$PATH")


The Coverity mount uses the resolved symlink target as both source and destination ({coverity_dir}:{coverity_dir}). The linked issue describes Coverity being available via /opt/coverity; mounting to the resolved path means the container may not have /opt/coverity at all. Consider mounting to /opt/coverity:/opt/coverity (or ensuring the container path /opt/coverity exists) so callers can rely on the documented location.

src/sc/docker/docker_run_builder.py

Copilot · 2026-02-04T16:13:59Z

src/sc/docker/docker_run_builder.py

+        if value:
+            self.docker_args += ['-e', f"{var_name}={value}"]
+        else:


add_env_var() checks if value: which treats valid values like 0 (e.g., uid/gid when running as root) as falsy and will emit -e VAR instead of -e VAR=0. Use an explicit value is not None check (and stringify non-str values) so zero/empty-string values are handled correctly.

Suggested change

if value:

self.docker_args += ['-e', f"{var_name}={value}"]

else:

if value is not None:

# Ensure all non-None values (including falsy ones like 0 or "") are passed as VAR=<value>

self.docker_args += ['-e', f"{var_name}={value!s}"]

else:

# Preserve behavior for flag-style environment variables with no explicit value

Copilot · 2026-02-04T16:13:59Z

src/sc/docker/docker_run_builder.py

@@ -0,0 +1,48 @@
+import shlex
+from pathlib import Path


Path is imported but not used in this module. Removing it avoids dead imports and keeps the file lint-clean.

Suggested change

from pathlib import Path

Copilot · 2026-02-04T16:14:00Z

src/sc/docker/docker.py

 from netrc import netrc, NetrcParseError
 import os
 from pathlib import Path
+import shlex


shlex is imported but never used in this file. Please remove the unused import to avoid dead code / lint failures.

Suggested change

import shlex

Copilot · 2026-02-04T16:14:00Z

src/sc/docker/docker.py

-            sys.exit(f"You cannot mount in any of the following directories: {', '.join(BANNED_MOUNT_DIRS)}")
+            raise ScDockerException(
+                f"You cannot mount to {dest_dir}! You cannot mount "
+                f"in any of the followind directories {', '.join(BANNED_MOUNT_DIRS)}"


Typo in error message: "followind" → "following" (and consider adding a colon before the directory list for readability).

Suggested change

f"in any of the followind directories {', '.join(BANNED_MOUNT_DIRS)}"

f"in any of the following directories: {', '.join(BANNED_MOUNT_DIRS)}"

Copilot · 2026-02-04T16:14:00Z

src/sc/docker/docker.py


    def _validate_docker_mount_source(self, server_dir: str):
        if not os.path.isdir(server_dir):
            sys.exit(f"{server_dir} does not exist")


_validate_docker_mount_source() still calls sys.exit() when the source directory doesn't exist, while other mount validation errors now raise ScDockerException and are handled by the caller. For consistent error handling (and to avoid exiting from deep helpers), raise ScDockerException here as well and let run() decide how to exit.

Suggested change

sys.exit(f"{server_dir} does not exist")

raise ScDockerException(f"{server_dir} does not exist")

Copilot · 2026-02-04T16:14:00Z

src/sc/docker/docker.py

        if xauth_line:
            hostname = os.getenv("HOSTNAME")
-            xauth_line = f"{xauth_line}/{hostname}/{container_name}"
-            return f"touch {Path.home()}/.Xauthority; xauth add {xauth_line}; "
-
-    def _add_user_env_vars(self) -> list:
-        docker_args = [
-            '-e', f"LOCAL_USER_NAME={getpass.getuser()}",
-            '-e', f"LOCAL_USER_ID={os.getuid()}",
-            '-e', f"LOCAL_GROUP_ID={os.getgid()}",
-            '-e', f"LOCAL_START_DIR={Path.home()}",
-        ]
-        try:
-            docker_group_id = grp.getgrnam('docker').gr_gid
-            docker_args += ['-e', f"LOCAL_DOCKER_GROUP={docker_group_id}"]
-        except:
-            pass
-        return docker_args
-
-    def _add_interactive_flag(self) -> list:
-        # If stdout is connected to a terminal
-        return ['-it'] if os.isatty(sys.stdout.fileno()) else []
+            xauth_line = xauth_line.replace(hostname, container_name, 1)
+            docker_run.add_bash_command(f"touch $HOME/.Xauthority")


In _add_x11_support, hostname = os.getenv("HOSTNAME") can be None. Passing None into xauth_line.replace(hostname, ...) will raise a TypeError and break --x11 runs. Guard for a missing HOSTNAME (e.g., fallback to socket.gethostname() or skip the replacement when unset).

Copilot · 2026-02-04T16:14:01Z

src/sc/docker/docker.py

+        try:
+            docker_group_id = grp.getgrnam('docker').gr_gid
+            docker_run.add_env_var("LOCAL_DOCKER_GROUP", docker_group_id)
+        except:


Except block directly handles BaseException.

Suggested change

except:

except KeyError:

Copilot · 2026-02-04T16:14:01Z

src/sc/docker/docker.py

+        try:
+            docker_group_id = grp.getgrnam('docker').gr_gid
+            docker_run.add_env_var("LOCAL_DOCKER_GROUP", docker_group_id)
+        except:


'except' clause does nothing but pass and there is no explanatory comment.

Suggested change

except:

except KeyError:

# If the 'docker' group does not exist on this system, skip setting LOCAL_DOCKER_GROUP.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 12 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-05T16:26:54Z

src/sc/docker/docker.py

-        return ['-it'] if os.isatty(sys.stdout.fileno()) else []
+            xauth_line = xauth_line.replace(hostname, container_name, 1)
+            docker_run.add_bash_command(f"touch $HOME/.Xauthority")
+            docker_run.add_bash_command(f"xauth add {xauth_line}")


Potential command injection vulnerability. The xauth_line from subprocess.check_output is used directly in a bash command without proper escaping. If the xauth output contains shell metacharacters, they could be executed. Use shlex.quote() to safely escape the xauth_line before using it in the bash command: docker_run.add_bash_command(f"xauth add {shlex.quote(xauth_line)}")

Copilot · 2026-02-05T16:26:55Z

src/sc/docker/docker.py

-        return docker_args
+        self._add_coverity_mount(docker_run)
+
+        docker_run.add_bash_command(f"cd {Path.cwd()}")


Path.cwd() returns a Path object but should be converted to a string. While this will work due to implicit string conversion in the f-string, it's better to be explicit: f"cd {str(Path.cwd())}" or use Path.cwd().as_posix() for a portable path string.

Copilot · 2026-02-05T16:26:55Z

src/sc/docker/docker.py

+        docker_run.add_env_var("LOCAL_USER_ID", os.getuid())
+        docker_run.add_env_var("LOCAL_GROUP_ID", os.getgid())
+        docker_run.add_env_var("LOCAL_START_DIR", Path.home())
+        try:
+            docker_group_id = grp.getgrnam('docker').gr_gid
+            docker_run.add_env_var("LOCAL_DOCKER_GROUP", docker_group_id)


Type annotations don't match actual parameter types being passed. The add_env_var method signature expects value: str | None, but non-string values are being passed:

Line 527: os.getuid() returns int

Line 528: os.getgid() returns int

Line 529: Path.home() returns Path

Line 532: docker_group_id is int

While Python's f-string will convert these to strings at runtime, this violates the type contract and will cause type checker errors. Either update the method signature to accept Union[str, int, Path, None] or explicitly convert values to str before passing them.

Suggested change

docker_run.add_env_var("LOCAL_USER_ID", os.getuid())

docker_run.add_env_var("LOCAL_GROUP_ID", os.getgid())

docker_run.add_env_var("LOCAL_START_DIR", Path.home())

try:

docker_group_id = grp.getgrnam('docker').gr_gid

docker_run.add_env_var("LOCAL_DOCKER_GROUP", docker_group_id)

docker_run.add_env_var("LOCAL_USER_ID", str(os.getuid()))

docker_run.add_env_var("LOCAL_GROUP_ID", str(os.getgid()))

docker_run.add_env_var("LOCAL_START_DIR", str(Path.home()))

try:

docker_group_id = grp.getgrnam('docker').gr_gid

docker_run.add_env_var("LOCAL_DOCKER_GROUP", str(docker_group_id))

Copilot · 2026-02-05T16:26:55Z

src/sc/docker/docker.py

+
+        docker_run.add_bash_command(f"cd {Path.cwd()}")
+        if command:
+            docker_run.add_bash_command(" ".join(command))


Potential command injection vulnerability. The user-supplied command tuple is joined with simple spaces without proper shell escaping. If any command argument contains shell metacharacters (spaces, quotes, semicolons, etc.), they could cause unexpected behavior or command injection. Use shlex.join(command) instead of " ".join(command) to properly escape shell arguments. Note that shlex is already imported in docker_run_builder.py and would need to be imported in docker.py as well.

Copilot · 2026-02-05T16:26:56Z

src/sc/docker/docker.py

+            xauth_line = xauth_line.replace(hostname, container_name, 1)
+            docker_run.add_bash_command(f"touch $HOME/.Xauthority")
+            docker_run.add_bash_command(f"xauth add {xauth_line}")


The xauth line manipulation is incorrect and breaks X11 authentication. The old implementation formatted the xauth line as "{xauth_line}/{hostname}/{container_name}" (visible in removed code context), but this new code uses replace(hostname, container_name, 1) which replaces the first occurrence of hostname within the xauth entry itself. This fundamentally changes the xauth entry format instead of appending the hostname/container information as needed. The original xauth entry format should be preserved and the hostname/container name should be appended or handled differently according to xauth requirements.

Suggested change

xauth_line = xauth_line.replace(hostname, container_name, 1)

docker_run.add_bash_command(f"touch $HOME/.Xauthority")

docker_run.add_bash_command(f"xauth add {xauth_line}")

# Preserve the original xauth entry and append hostname/container info

suffix_parts = []

if hostname:

suffix_parts.append(hostname)

if container_name:

suffix_parts.append(container_name)

if suffix_parts:

xauth_entry = f"{xauth_line}/" + "/".join(suffix_parts)

else:

xauth_entry = xauth_line

docker_run.add_bash_command(f"touch $HOME/.Xauthority")

docker_run.add_bash_command(f"xauth add {xauth_entry}")

Copilot · 2026-02-05T16:26:57Z

src/sc/docker/docker_run_builder.py

@@ -0,0 +1,47 @@
+import shlex


Missing copyright header. All other Python files in the codebase (e.g., docker.py, config_manager.py, docker_cli.py) include the Apache 2.0 copyright header at the top of the file. This file should include the same header for consistency and legal compliance.

Copilot · 2026-02-05T16:26:57Z

src/sc/docker/docker.py

-    def _add_interactive_flag(self) -> list:
-        # If stdout is connected to a terminal
-        return ['-it'] if os.isatty(sys.stdout.fileno()) else []
+            xauth_line = xauth_line.replace(hostname, container_name, 1)


Potential TypeError when hostname is None. If the HOSTNAME environment variable is not set, os.getenv("HOSTNAME") returns None, and calling str.replace(None, ...) will raise a TypeError. Add a check to handle the case when hostname is None, or provide a default value: hostname = os.getenv("HOSTNAME") or "localhost"

Suggested change

xauth_line = xauth_line.replace(hostname, container_name, 1)

if hostname:

xauth_line = xauth_line.replace(hostname, container_name, 1)

Copilot · 2026-02-05T16:26:57Z

src/sc/docker/docker_run_builder.py

@@ -0,0 +1,47 @@
+import shlex
+
+class DockerRunBuilder:


Missing documentation for the DockerRunBuilder class. The class lacks a docstring explaining its purpose, which would help future maintainers understand its role in building Docker run commands. Add a class-level docstring following the pattern used in the SCDocker class.

Suggested change

class DockerRunBuilder:

class DockerRunBuilder:

"""

Helper for constructing `docker run` command invocations.

This builder assembles the arguments for a `docker run` command,

including container name, hostname, volume mounts, environment

variables, and interactive flags, and composes a bash command

sequence to be executed inside the container.

"""

Copilot · 2026-02-05T16:26:57Z

src/sc/docker/docker_run_builder.py

+    def add_volume_mount(self, volume: str):
+        self.docker_args += ['-v', volume]
+
+    def add_env_var(self, var_name: str, value: str = None):


The type hint for value parameter is incorrect. The parameter is typed as str = None, but None is not a string. This should be value: str | None = None (Python 3.10+) or value: Optional[str] = None (with from typing import Optional). The current annotation will cause type checking tools to flag this as an error.

Copilot · 2026-02-05T16:26:58Z

src/sc/docker/exceptions.py

@@ -0,0 +1,4 @@
+
+
+class ScDockerException(Exception):


Missing documentation for the ScDockerException class. The custom exception class lacks a docstring explaining when it should be raised and what it represents. Add a class-level docstring to clarify its purpose, e.g., "Raised when sc-docker operations encounter errors related to Docker configuration or execution."

Suggested change

class ScDockerException(Exception):

class ScDockerException(Exception):

"""Raised when sc-docker operations encounter errors related to Docker configuration or execution."""

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-06T11:39:18Z

src/sc/docker/docker.py

-            self._handle_invalid_tag(image, tag, tags)
+        else:
+            self._check_no_registries()
+            images = self._get_remote_images()


_get_remote_images() is called without the required image_ref argument. As written this will raise a TypeError and prevent remote images from running. Pass image_ref through (or change _get_remote_images to take no args).

Suggested change

images = self._get_remote_images()

images = self._get_remote_images(image_ref)

Copilot · 2026-02-06T11:39:18Z

src/sc/docker/docker.py

+            tags = self._fetch_remote_tags(image, registry_url)
+            if tag not in tags:
+                self._handle_invalid_tag(image, tag, tags)


_fetch_remote_tags expects (image, username, api_token), but here it’s called with (image, registry_url). This will fail at runtime and also won’t authenticate correctly. Use _get_registry_creds_by_url(registry_url) (or the existing _fetch_tags(...) helper) to supply the proper credentials.

Copilot · 2026-02-06T11:39:19Z

src/sc/docker/docker.py

+    def _get_remote_images(self, image_ref: str) -> str:
        images = []
-
-        if local:
-            images = list({tag.split(":")[0] for image in self.docker_client.images.list() for tag in image.tags})
+
+        # If image ref matches a host just get that hosts images. Otherwise get all hosts images.
+        host = False
+        for registry_url in self.docker_config:
+            if image_ref.startswith(registry_url):
+                host = registry_url
+
+        if host:
+            images = [f"{host}/{image}" for image in self._fetch_images_by_registry(host)]
        else:
-            # If image ref matches a host just get that hosts images. Otherwise get all hosts images.
-            host = False
            for registry_url in self.docker_config:
-                if image_ref.startswith(registry_url):
-                    host = registry_url
-
-            if host:
-                images = [f"{host}/{image}" for image in self._fetch_images_by_registry(host)]
-            else:
-                for registry_url in self.docker_config:
-                    registry_images = self._fetch_images_by_registry(registry_url)
-                    images.extend([f"{registry_url}/{image}" for image in registry_images])
-
+                registry_images = self._fetch_images_by_registry(registry_url)
+                images.extend([f"{registry_url}/{image}" for image in registry_images])
+


_get_remote_images builds up an images list but never returns it, and its return type annotation is str even though it’s a list. This will currently return None and break callers. Return images and update the annotation accordingly.

Copilot · 2026-02-06T11:39:19Z

src/sc/docker/docker.py

+            docker_run.add_volume_mount(f"{coverity_dir}:{coverity_dir}")
+            docker_run.add_bash_command(f"export PATH={coverity_dir}:$PATH")


The PR/issue states Coverity is linked at /opt/coverity, but this mount uses the resolved target path for both source and destination (<resolved>:<resolved>). That means /opt/coverity inside the container may not exist/point to the mounted install, so tooling expecting /opt/coverity will fail. Mount the resolved host path into /opt/coverity in the container (and consider setting PATH relative to /opt/coverity).

Suggested change

docker_run.add_volume_mount(f"{coverity_dir}:{coverity_dir}")

docker_run.add_bash_command(f"export PATH={coverity_dir}:$PATH")

docker_run.add_volume_mount(f"{coverity_dir}:/opt/coverity")

docker_run.add_bash_command("export PATH=/opt/coverity:$PATH")

Copilot · 2026-02-06T11:39:19Z

src/sc/docker/docker.py

+
+        self._add_coverity_mount(docker_run)
+


Issue #17/PR description mentions mounting Coverity if an argument is supplied and that CodeSonar is optional, but _add_coverity_mount is called unconditionally and CodeSonar mounting has been removed entirely. If the intent is optional behavior, add a CLI flag and thread it through to _generate_docker_run_command so users can opt in/out (and keep CodeSonar support if still required).

Copilot · 2026-02-06T11:39:19Z

src/sc/docker/docker_run_builder.py

+    def build(self) -> list[str]:
+        """Build the final docker run argument list."""
+        return [
+            *self.docker_args,
+            f"{self.image}:{self.tag}",
+            shlex.join(["bash", "-c", " && ".join(self.bash_commands)])
+        ]


build() uses shlex.join(...) to produce a shell-escaped single string and places it as one argv element. Because the command is ultimately executed via os.execvp, those quotes/escaping become literal characters and Docker won’t split it into bash, -c, and the script. If you need exec-form args, append bash, -c, and the script as separate list elements; if you truly need a single script string, avoid shlex.join here and only use it for logging.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-06T12:08:09Z

src/sc/docker/docker.py

+        coverity_dir = Path('/opt/coverity').resolve()
+        if Path('/opt/coverity').is_symlink() and Path(coverity_dir).exists():
+            docker_args += ["-v", f"{coverity_dir}:{coverity_dir}"]
+            bash_commands += [f"export PATH={coverity_dir}:$PATH"]


The PATH export adds the coverity root directory to PATH, which is unusual. Most tool installations have their executables in a 'bin' subdirectory. Consider whether this should be 'export PATH={coverity_dir}/bin:$PATH' to add the bin directory instead of the root directory. This would be consistent with how CodeSonar was handled (which used CODESONAR_PATH={codesonar_dir}/codesonar/bin).

Suggested change

bash_commands += [f"export PATH={coverity_dir}:$PATH"]

bash_commands += [f"export PATH={coverity_dir}/bin:$PATH"]

Copilot · 2026-02-06T12:08:09Z

src/sc/docker/docker.py

-        Returns:
-            str: The full image name
-        """        
+    def _get_remote_images(self, image_ref: str) -> str:


The return type annotation is incorrect. The method returns a list of image strings, not a single string. Change the return type from 'str' to 'list[str]'.

Suggested change

def _get_remote_images(self, image_ref: str) -> str:

def _get_remote_images(self, image_ref: str) -> list[str]:

Copilot · 2026-02-06T12:08:10Z

src/sc/docker/docker.py

+            '-e',
+            f"LOCAL_START_DIR={Path.home()}"
+        ]



The architecture detection logic was removed in this refactoring. Previously, images with '_i386' or '_amd64' suffixes would automatically get the appropriate '--platform' flag. Without this, containers may fail to run on systems where the image architecture doesn't match the host architecture, or Docker may choose the wrong platform variant. Consider whether this functionality should be preserved.

Suggested change

# Preserve legacy behavior: infer Docker platform from image suffix.

# Images ending with "_i386" or "_amd64" automatically get the

# corresponding "--platform" flag.

platform = None

if image.endswith("_i386"):

platform = "linux/386"

elif image.endswith("_amd64"):

platform = "linux/amd64"

if platform is not None:

docker_args += ['--platform', platform]

Copilot · 2026-02-06T12:08:10Z

src/sc/docker/docker.py

+                    f"xauth add {xauth_line}"
+                ]
+
+            except subprocess.CalledProcessError:


'except' clause does nothing but pass and there is no explanatory comment.

Suggested change

except subprocess.CalledProcessError:

except subprocess.CalledProcessError:

# If xauth is unavailable or fails, skip adding X11 auth; container may run without GUI access.

TB-1993 · 2026-02-06T15:57:54Z

src/sc/docker/docker.py

+            if not display:
+                click.secho("WARNING: No DISPLAY variable set", fg="yellow")
+                click.secho("WARNING: X11 not forwarded into docker", fg="yellow")
+            docker_args += ["-e", "DISPLAY"]


Shouldn't this all be in an else, since there's no point doing it if we didn't get display.

TB-1993 · 2026-02-06T16:25:54Z

src/sc/docker/docker.py

+            except subprocess.CalledProcessError:
+                pass


This should actually do something right? If we don't get the line from their xauthority we no point writing the new one inside the container and we need to tell use x11 won't work,

TB-1993 · 2026-02-06T16:37:30Z

src/sc/docker/docker.py

+        """Prints and executes the docker run command."""
+        click.secho(" ".join(docker_command), fg='green')
+        click.echo()
+        os.execvp('docker', docker_command)


Unless you have deliberate reason to use os.execvp here, I'd stick with a subprocess.run call.

Need to use os.execvp as it gives up the python process for the docker process. Otherwise you have to do lots of funky things to deal with the stream of the subprocess.

TB-1993

Some minor changes, but otherwise looks good

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-11T10:37:41Z

src/sc/docker/docker.py

+    def _get_remote_images(self, image_ref: str) -> str:
        images = []
-
-        if local:
-            images = list({tag.split(":")[0] for image in self.docker_client.images.list() for tag in image.tags})
+
+        # If image ref matches a host just get that hosts images. Otherwise get all hosts images.
+        host = False
+        for registry_url in self.docker_config:
+            if image_ref.startswith(registry_url):
+                host = registry_url
+
+        if host:
+            images = [f"{host}/{image}" for image in self._fetch_images_by_registry(host)]
        else:
-            # If image ref matches a host just get that hosts images. Otherwise get all hosts images.
-            host = False
            for registry_url in self.docker_config:
-                if image_ref.startswith(registry_url):
-                    host = registry_url
+                registry_images = self._fetch_images_by_registry(registry_url)
+                images.extend([f"{registry_url}/{image}" for image in registry_images])

-            if host:
-                images = [f"{host}/{image}" for image in self._fetch_images_by_registry(host)]
-            else:
-                for registry_url in self.docker_config:
-                    registry_images = self._fetch_images_by_registry(registry_url)
-                    images.extend([f"{registry_url}/{image}" for image in registry_images])
-
+        return images


_get_remote_images() is annotated to return str, but it actually returns a list of image strings (images). This breaks type hints and can confuse callers/static analysis; update the return annotation to list[str] (or Sequence[str]) to match the actual value.

Copilot · 2026-02-11T10:37:41Z

src/sc/docker/docker.py

+        if Path('/opt/coverity').is_symlink() and Path(coverity_dir).exists():
+            docker_args += ["-v", f"{coverity_dir}:{coverity_dir}"]
+            bash_commands += [f"export PATH={coverity_dir}:$PATH"]


The Coverity mount uses the resolved target of the /opt/coverity symlink as both the host and container path (e.g. /tools/coverity/...:/tools/coverity/...). This doesn’t ensure /opt/coverity exists inside the container (as described in Issue #17) and can break tooling that expects Coverity at /opt/coverity. Mount the resolved host path to /opt/coverity in the container (and consider adding /opt/coverity/bin to PATH if the intent is to expose executables).

Suggested change

if Path('/opt/coverity').is_symlink() and Path(coverity_dir).exists():

docker_args += ["-v", f"{coverity_dir}:{coverity_dir}"]

bash_commands += [f"export PATH={coverity_dir}:$PATH"]

if Path('/opt/coverity').is_symlink() and coverity_dir.exists():

docker_args += ["-v", f"{coverity_dir}:/opt/coverity"]

bash_commands += ["export PATH=/opt/coverity/bin:$PATH"]

Copilot · 2026-02-11T10:37:41Z

src/sc/docker/docker.py

+            hostname = os.getenv("HOSTNAME") or socket.gethostname()
+            xauth_line = xauth_line.replace(hostname, container_name, 1)
        except subprocess.CalledProcessError:
            xauth_line = None


_get_xauth_line() computes xauth_line but never returns it, so callers will always receive None and X11 forwarding will always fall back to _warn_x11_not_forwarded(...). Add an explicit return xauth_line at the end of the function.

Suggested change

xauth_line = None

xauth_line = None

return xauth_line

Copilot · 2026-02-11T10:37:42Z

src/sc/docker/docker.py

+        coverity_dir = Path('/opt/coverity').resolve()
+        if Path('/opt/coverity').is_symlink() and Path(coverity_dir).exists():
+            docker_args += ["-v", f"{coverity_dir}:{coverity_dir}"]
+            bash_commands += [f"export PATH={coverity_dir}:$PATH"]


This Coverity auto-mount is triggered solely by the presence of the /opt/coverity symlink, but Issue #17 / the PR description mention mounting Coverity "if arg is supplied". There is currently no CLI/run parameter that controls this behavior, so users can’t opt in/out explicitly; either add a sc docker run option to enable the Coverity mount, or adjust the requirement/docs to reflect the auto-detection behavior.

TB-1993 · 2026-02-11T11:22:09Z

src/sc/docker/docker.py

+        if self._stdout_connected_to_terminal():
+            docker_args += ['-it']


Did we have this before? I don't think this is conditional behaviour. I think SC should always run an interactive shell.

# test for TTY input and set the docker option accordingly test -t 1 && docker_options+=( '-it' )

This is from old sc docker which I copied originally. Researching what the -it flags do I see how we potentially do want to always have at least the -i flag, but I think the -t flag can be dangerous if running in CI?

TB-1993 · 2026-02-11T11:23:14Z

src/sc/docker/docker.py

+        if local:
+            tags = self._fetch_local_tags(image)
+        else:
+            username, api_token = self._get_registry_creds_by_url(registry_url)
+            tags = self._fetch_remote_tags(image, username, api_token)
+
        if tag not in tags:
            self._handle_invalid_tag(image, tag, tags)

        if not local:


These three conditional statements test the same condition.

TB-1993 · 2026-02-11T11:23:53Z

src/sc/docker/docker.py

+        if host:
+            images = [f"{host}/{image}" for image in self._fetch_images_by_registry(host)]
        else:
-            # If image ref matches a host just get that hosts images. Otherwise get all hosts images.
-            host = False
            for registry_url in self.docker_config:
-                if image_ref.startswith(registry_url):
-                    host = registry_url
+                registry_images = self._fetch_images_by_registry(registry_url)
+                images.extend([f"{registry_url}/{image}" for image in registry_images])

-            if host:
-                images = [f"{host}/{image}" for image in self._fetch_images_by_registry(host)]
-            else:
-                for registry_url in self.docker_config:
-                    registry_images = self._fetch_images_by_registry(registry_url)
-                    images.extend([f"{registry_url}/{image}" for image in registry_images])
-
+        return images


These differing behaviours for the function make this quite confusing.

BenjiMilan self-assigned this Jan 28, 2026

Copilot AI review requested due to automatic review settings January 28, 2026 14:34

Copilot started reviewing on behalf of BenjiMilan January 28, 2026 14:34 View session

Copilot AI reviewed Jan 28, 2026

View reviewed changes

gh17 docker mount coverity

65d5abb

BenjiMilan force-pushed the feature/gh17_sc_docker_mount_coverity branch from db28c1d to 65d5abb Compare January 28, 2026 17:10

Copilot AI review requested due to automatic review settings January 29, 2026 14:24

Copilot started reviewing on behalf of BenjiMilan January 29, 2026 14:24 View session

Copilot AI reviewed Jan 29, 2026

View reviewed changes

BenjiMilan force-pushed the feature/gh17_sc_docker_mount_coverity branch from ff3a39f to 387a564 Compare January 30, 2026 10:50

BenjiMilan requested a review from TB-1993 January 30, 2026 10:54

BenjiMilan force-pushed the feature/gh17_sc_docker_mount_coverity branch from 387a564 to f8939cc Compare January 30, 2026 11:18

Copilot AI review requested due to automatic review settings January 30, 2026 11:18

Copilot started reviewing on behalf of BenjiMilan January 30, 2026 11:19 View session

Copilot AI reviewed Jan 30, 2026

View reviewed changes

BenjiMilan force-pushed the feature/gh17_sc_docker_mount_coverity branch from f8939cc to 5c14a25 Compare February 2, 2026 14:24

TB-1993 requested changes Feb 3, 2026

View reviewed changes

BenjiMilan force-pushed the feature/gh17_sc_docker_mount_coverity branch from 5c14a25 to 85797d3 Compare February 4, 2026 16:08

Copilot AI review requested due to automatic review settings February 4, 2026 16:08

Copilot started reviewing on behalf of BenjiMilan February 4, 2026 16:08 View session

Copilot AI reviewed Feb 4, 2026

View reviewed changes

BenjiMilan force-pushed the feature/gh17_sc_docker_mount_coverity branch from 85797d3 to 7371a6e Compare February 5, 2026 09:32

Copilot AI review requested due to automatic review settings February 5, 2026 16:19

BenjiMilan force-pushed the feature/gh17_sc_docker_mount_coverity branch from 7371a6e to 88e818e Compare February 5, 2026 16:19

Copilot started reviewing on behalf of BenjiMilan February 5, 2026 16:20 View session

Copilot AI reviewed Feb 5, 2026

View reviewed changes

gh17 sc docker mount coverity

1be4ade

BenjiMilan force-pushed the feature/gh17_sc_docker_mount_coverity branch from 88e818e to 1be4ade Compare February 6, 2026 09:37

gh17 - update run method

466f55c

Copilot AI review requested due to automatic review settings February 6, 2026 11:33

Copilot started reviewing on behalf of BenjiMilan February 6, 2026 11:34 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings February 6, 2026 12:02

BenjiMilan force-pushed the feature/gh17_sc_docker_mount_coverity branch from 42a4ef3 to 3b90bdb Compare February 6, 2026 12:02

Copilot started reviewing on behalf of BenjiMilan February 6, 2026 12:03 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

TB-1993 reviewed Feb 6, 2026

View reviewed changes

TB-1993 requested changes Feb 6, 2026

View reviewed changes

gh17 all one function

1c1c336

BenjiMilan force-pushed the feature/gh17_sc_docker_mount_coverity branch from 3b90bdb to 1c1c336 Compare February 9, 2026 14:07

gh17 - shortened function and reduced duplication

e6affd7

Copilot AI review requested due to automatic review settings February 11, 2026 10:33

Copilot started reviewing on behalf of BenjiMilan February 11, 2026 10:33 View session

Copilot AI reviewed Feb 11, 2026

View reviewed changes

TB-1993 requested changes Feb 11, 2026

View reviewed changes

gh17 - smaller functions again

4b84c6c

-        coverity_dir = Path('/opt/coverity').resolve()
-        if Path('/opt/coverity').is_symlink() and Path(coverity_dir).exists():
-            docker_args += ['-v', f"{coverity_dir}:{coverity_dir}"]
+        coverity_path = Path('/opt/coverity')
+        coverity_dir = coverity_path.resolve()
+        if coverity_path.exists():
+            docker_args += ['-v', f"{coverity_dir}:/opt/coverity:ro"]

		bash_command = f"bash -c 'source /usr/local/bin/bashext.sh && export PATH=/opt/coverity/bin:$PATH && cd {Path.cwd()}; {' '.join(command)}'"
		if x11 and os.getenv("DISPLAY"):

-        bash = "bash -c '" + " && ".join(parts) + "'"
+        command_str = " && ".join(parts)
+        # Escape single quotes so the command is safe inside a single-quoted bash -c argument.
+        escaped_command_str = command_str.replace("'", "'\"'\"'")
+        bash = "bash -c '" + escaped_command_str + "'"

	coverity_args = ['-v', f"{coverity_dir}:{coverity_dir}"]
	coverity_args = ['-v', f"{coverity_dir}:/opt/coverity"]

		coverity_args = ['-v', f"{coverity_dir}:{coverity_dir}"]
		coverity_prelude = [f"export PATH={coverity_dir}:$PATH"]

-            coverity_args = ['-v', f"{coverity_dir}:{coverity_dir}"]
-            coverity_prelude = [f"export PATH={coverity_dir}:$PATH"]
+            # Bind-mount the resolved host directory into the container at /opt/coverity
+            coverity_args = ['-v', f"{coverity_dir}:/opt/coverity"]
+            # Add the Coverity bin directory under the mounted /opt/coverity to PATH
+            coverity_prelude = ["export PATH=/opt/coverity/bin:$PATH"]

-            xauth_line = f"{xauth_line}/{hostname}/{container_name}"
+            if hostname:
+                parts = xauth_line.split()
+                if len(parts) >= 3:
+                    parts[0] = f"{parts[0]}/{hostname}/{container_name}"
+                    xauth_line = " ".join(parts)

-        if value:
-            self.docker_args += ['-e', f"{var_name}={value}"]
-        else:
+        if value is not None:
+            # Ensure all non-None values (including falsy ones like 0 or "") are passed as VAR=<value>
+            self.docker_args += ['-e', f"{var_name}={value!s}"]
+        else:
+            # Preserve behavior for flag-style environment variables with no explicit value

	f"in any of the followind directories {', '.join(BANNED_MOUNT_DIRS)}"
	f"in any of the following directories: {', '.join(BANNED_MOUNT_DIRS)}"

	sys.exit(f"{server_dir} does not exist")
	raise ScDockerException(f"{server_dir} does not exist")

	except:
	except KeyError:
	# If the 'docker' group does not exist on this system, skip setting LOCAL_DOCKER_GROUP.

-            xauth_line = xauth_line.replace(hostname, container_name, 1)
-            docker_run.add_bash_command(f"touch $HOME/.Xauthority")
-            docker_run.add_bash_command(f"xauth add {xauth_line}")
+            # Preserve the original xauth entry and append hostname/container info
+            suffix_parts = []
+            if hostname:
+                suffix_parts.append(hostname)
+            if container_name:
+                suffix_parts.append(container_name)
+            if suffix_parts:
+                xauth_entry = f"{xauth_line}/" + "/".join(suffix_parts)
+            else:
+                xauth_entry = xauth_line
+            docker_run.add_bash_command(f"touch $HOME/.Xauthority")
+            docker_run.add_bash_command(f"xauth add {xauth_entry}")

	xauth_line = xauth_line.replace(hostname, container_name, 1)
	if hostname:
	xauth_line = xauth_line.replace(hostname, container_name, 1)

-class DockerRunBuilder:
+class DockerRunBuilder:
+    """
+    Helper for constructing `docker run` command invocations.
+    This builder assembles the arguments for a `docker run` command,
+    including container name, hostname, volume mounts, environment
+    variables, and interactive flags, and composes a bash command
+    sequence to be executed inside the container.
+    """

	class ScDockerException(Exception):
	class ScDockerException(Exception):
	"""Raised when sc-docker operations encounter errors related to Docker configuration or execution."""

	images = self._get_remote_images()
	images = self._get_remote_images(image_ref)

		docker_run.add_volume_mount(f"{coverity_dir}:{coverity_dir}")
		docker_run.add_bash_command(f"export PATH={coverity_dir}:$PATH")

	bash_commands += [f"export PATH={coverity_dir}:$PATH"]
	bash_commands += [f"export PATH={coverity_dir}/bin:$PATH"]

	def _get_remote_images(self, image_ref: str) -> str:
	def _get_remote_images(self, image_ref: str) -> list[str]:

+        # Preserve legacy behavior: infer Docker platform from image suffix.
+        # Images ending with "_i386" or "_amd64" automatically get the
+        # corresponding "--platform" flag.
+        platform = None
+        if image.endswith("_i386"):
+            platform = "linux/386"
+        elif image.endswith("_amd64"):
+            platform = "linux/amd64"
+        if platform is not None:
+            docker_args += ['--platform', platform]

gh17 docker mount coverity #22

Are you sure you want to change the base?

gh17 docker mount coverity #22

Uh oh!

Conversation

BenjiMilan commented Jan 28, 2026

Uh oh!

rdkcmf-jenkins commented Jan 28, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

rdkcmf-jenkins commented Jan 28, 2026

Uh oh!

rdkcmf-jenkins commented Jan 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

TB-1993 left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

	except subprocess.CalledProcessError:
	except subprocess.CalledProcessError:
	# If xauth is unavailable or fails, skip adding X11 auth; container may run without GUI access.

		if self._stdout_connected_to_terminal():
		docker_args += ['-it']