Skip to content

Conversation

@VietND96
Copy link
Member

@VietND96 VietND96 commented Oct 31, 2025

User description

Thanks for contributing to the Docker-Selenium project!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines, applied for this repository.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

  • Node/Standalone Firefox channel Dev/Beta is now built for multi-arch Linux/amd64, Linux/arm64.
  • Node/Standalone Chrome for Testing (CfT) is now built backward versions (from v113 to latest now v142)

Motivation and Context

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Enhancement


Description

  • Add ChromeForTesting version support to build matrix

  • Enable building multiple CFT versions via CI pipeline

  • Extract CFT_VERSION from browser matrix configuration


Diagram Walkthrough

flowchart LR
  A["Build Matrix Config"] -- "Extract CFT_VERSION" --> B["builder.py"]
  B -- "Write CFT_VERSION env" --> C["Build Environment"]
  C -- "Build CFT Images" --> D["Multiple CFT Versions"]
Loading

File Walkthrough

Relevant files
Enhancement
builder.py
Add ChromeForTesting version extraction to builder             

tests/build-backward-compatible/builder.py

  • Added conditional block to handle chrome-for-testing browser type
  • Extracts CFT_VERSION from the browser matrix configuration
  • Writes CFT_VERSION environment variable to build configuration file
  • Enables parallel building of multiple ChromeForTesting versions
+3/-0     

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Oct 31, 2025

PR Compliance Guide 🔍

(Compliance updated until commit 611633b)

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No audit logging: The added workflow and scripts perform installs and builds without emitting structured
audit logs of critical actions or outcomes, and it is unclear if higher-level logging
covers these operations.

Referred Code
jobs:

  deploy:

    runs-on: ubuntu-24.04
    strategy: 
      fail-fast: false
      matrix:
        include:
          - browser: chrome
            channel: dev
            platforms: linux/amd64
          - browser: chrome
            channel: beta
            platforms: linux/amd64
          - browser: firefox
            channel: dev
            platforms: linux/amd64,linux/arm64
          - browser: firefox
            channel: beta
            platforms: linux/amd64,linux/arm64


 ... (clipped 62 lines)
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Unchecked installs: The else-branch runs apt-get install with a long dependency list but does not check for
failures or handle partial install/edge cases, which may cause silent build errors in CI.

Referred Code
else
  apt-get install -qqy --no-install-recommends ca-certificates fonts-liberation libasound2t64 libatk-bridge2.0-0 libatk1.0-0 libatspi2.0-0 libc6 libcairo2 libcups2 libcurl3-gnutls libdbus-1-3 libdrm2 libexpat1 libgbm1 libglib2.0-0 libgtk-3-0 libnspr4 libnss3 libpango-1.0-0 libvulkan1 libx11-6 libxcb1 libxcomposite1 libxdamage1 libxext6 libxfixes3 libxkbcommon0 libxrandr2 wget xdg-utils
fi
Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Input validation: Environment-derived variables like FIREFOX_VERSION and FIREFOX_DOWNLOAD_URL are used to
construct download/install commands with minimal validation, which could pose security
risks if misconfigured.

Referred Code
ARG FIREFOX_LANG_VERSION=${FIREFOX_VERSION}
RUN apt-get update -qqy && \
  FIREFOX_MAJOR_VERSION=${FIREFOX_VERSION%%.*} && \
  ARCH=$(if [ "$(dpkg --print-architecture)" = "amd64" ]; then echo "x86_64"; else echo "aarch64"; fi) && \
  # Check if FIREFOX_MAJOR_VERSION is numeric before comparison \
  case "$FIREFOX_MAJOR_VERSION" in \
    ''|*[!0-9]*) IS_NUMERIC=false ;; \
    *) IS_NUMERIC=true ;; \
  esac && \
  if [ "$(dpkg --print-architecture)" = "amd64" ] || [ $FIREFOX_VERSION = "latest" ] || [ $FIREFOX_VERSION = "beta-latest" ] || [ $FIREFOX_VERSION = "nightly-latest" ] || [ $FIREFOX_VERSION = "devedition-latest" ] || [ $FIREFOX_VERSION = "esr-latest" ] || { [ "$IS_NUMERIC" = "true" ] && [ "${FIREFOX_MAJOR_VERSION}" -ge 136 ]; }; then \
    if [ $FIREFOX_VERSION = "latest" ] || [ $FIREFOX_VERSION = "beta-latest" ] || [ $FIREFOX_VERSION = "nightly-latest" ] || [ $FIREFOX_VERSION = "devedition-latest" ] || [ $FIREFOX_VERSION = "esr-latest" ]; then \
      /opt/bin/install-firefox-apt.sh \
      && FIREFOX_VERSION=$(echo "-$FIREFOX_VERSION" | sed 's/-latest//') \
      && apt install -y firefox$FIREFOX_VERSION \
      && INSTALL_VIA_APT=true \
      && if [ $FIREFOX_VERSION = "-beta" ] || [ $FIREFOX_VERSION = "-nightly" ] || [ $FIREFOX_VERSION = "-devedition" ] || [ $FIREFOX_VERSION = "-esr" ]; then \
        ln -fs $(which firefox$FIREFOX_VERSION) /usr/bin/firefox ; \
      fi ; \
    else \
      FIREFOX_DOWNLOAD_URL="https://download-installer.cdn.mozilla.net/pub/firefox/releases/$FIREFOX_VERSION/linux-$ARCH/en-US/firefox-$FIREFOX_VERSION.deb" \
      && if [ "404" = "$(curl -s -o /dev/null -w "%{http_code}" $FIREFOX_DOWNLOAD_URL)" ]; then \


 ... (clipped 26 lines)
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Previous compliance checks

Compliance check up to commit 6ef8c54
Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No audit logs: The newly added handling for CFT version writing performs configuration changes without
any logging of the action, user/context, or outcome, making auditability unclear.

Referred Code
if browser_name == "chrome-for-testing" or browser_name == "all":
    CFT_VERSION = matrix["browser"][browser_version]["CFT_VERSION"]
    f.write(f"CFT_VERSION={CFT_VERSION}")
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing key guards: Access to matrix['browser'][browser_version]['CFT_VERSION'] lacks
validation/try-except, which may raise KeyError without contextual error handling.

Referred Code
CFT_VERSION = matrix["browser"][browser_version]["CFT_VERSION"]
f.write(f"CFT_VERSION={CFT_VERSION}")
Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Unvalidated inputs: The code reads CFT_VERSION from an external matrix structure and writes it to an
env/config file without validating or sanitizing the value.

Referred Code
if browser_name == "chrome-for-testing" or browser_name == "all":
    CFT_VERSION = matrix["browser"][browser_version]["CFT_VERSION"]
    f.write(f"CFT_VERSION={CFT_VERSION}")

@qodo-merge-pro
Copy link
Contributor

qodo-merge-pro bot commented Oct 31, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Add a newline to file write

Add a newline character when writing the CFT_VERSION to the build configuration
file to ensure each entry is on its own line and prevent malformed output.

tests/build-backward-compatible/builder.py [67]

-f.write(f"CFT_VERSION={CFT_VERSION}")
+f.write(f"CFT_VERSION={CFT_VERSION}\n")
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a bug where a missing newline would corrupt the configuration file when browser_name is "all", by concatenating CHROME_VERSION and CFT_VERSION on the same line.

High
  • Update

@VietND96 VietND96 force-pushed the cft-versions branch 2 times, most recently from a31a7bd to 91770b7 Compare October 31, 2025 15:21
Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
@VietND96 VietND96 changed the title [ci]: Build multiple versions of ChromeForTesting Docker: Firefox Dev/Beta build multi arch and ChromeForTesting backward versions Oct 31, 2025
@VietND96 VietND96 merged commit a4f170e into trunk Nov 1, 2025
113 of 115 checks passed
@VietND96 VietND96 deleted the cft-versions branch November 1, 2025 01:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants