Skip to content

Instantly share code, notes, and snippets.

@bryanpaget
Created August 6, 2025 17:49
Show Gist options
  • Select an option

  • Save bryanpaget/b698b7423ced85fd09f55d2db4664476 to your computer and use it in GitHub Desktop.

Select an option

Save bryanpaget/b698b7423ced85fd09f55d2db4664476 to your computer and use it in GitHub Desktop.
Issues with our repo

Issue 1: Runtime Dockerfile Modification is Fragile and Non-Transparent

Title: Replace runtime Dockerfile modification with build arguments Labels: enhancement, good-first-issue, docker

Problem: The workflow modifies Dockerfiles at runtime using sed to inject the base image:

- name: Set FROM and as in Dockerfile
   run: |
     sed -i '1i FROM ${{ env.BASE_IMAGE}} as ${{ inputs.image }}' 
./images/${{ inputs.directory }}/Dockerfile ```

**Why it's problematic**:
1. Creates a mismatch between the repository's Dockerfile and what's actually built 2. Makes debugging difficult since the build artifact differs from source 3. Assumes Dockerfiles don't already have a `FROM` statement 4. Fragile against input variations (e.g., special characters in image
names)

**Suggested solution**:
Use Docker build arguments instead:
1. Modify Dockerfiles to accept build args:
    ```Dockerfile
    ARG BASE_IMAGE
    FROM ${BASE_IMAGE} as stage_name
    ```
2. Update the build step:
    ```yaml
    - name: Build image
      run: |
        make build/${{ inputs.image }} \
          REPO=${{ inputs.registry-name }} \
          DIRECTORY=${{ inputs.directory }} \
          BASE_IMAGE=${{ env.BASE_IMAGE }}
    ```
3. Update Makefile to pass `--build-arg BASE_IMAGE` to `docker build`

**Additional context**:
This aligns with Docker best practices for parameterized builds and improves reproducibility.

---

### Issue 2: Remove Unused Local Registry Service
**Title**: Remove unused local registry service from workflow
**Labels**: `cleanup`, `maintenance`

**Problem**:
The workflow defines a local Docker registry service that's never used:
```yaml
services:
   registry:
     image: registry:2
     ports:
       - 5000:5000

Why it's problematic:

  1. Consumes unnecessary resources (CPU/memory) during workflow runs 2. Creates confusion about the build process 3. Adds unnecessary complexity to the workflow definition

Suggested solution: Remove the entire services block from the workflow file.

Additional context: If local registry testing was intended but not implemented, consider creating a separate workflow for that use case.


Issue 3: Improve Hadolint Implementation

Title: Replace manual Hadolint setup with official GitHub Action Labels: enhancement, security, linting

Problem: The workflow manually downloads and runs Hadolint:

- name: Run Hadolint
   run: |
     sudo curl -L ... --output hadolint
     sudo chmod +x hadolint
     ./hadolint ... --no-fail

Why it's problematic:

  1. Reinvents functionality available in maintained GitHub Actions 2. Uses --no-fail which prevents linting errors from failing the build 3. Increases maintenance burden (version updates, script management) 4. Less secure than using a maintained action

Suggested solution: Replace with the official Hadolint action:

- name: Run Hadolint
   uses: hadolint/[email protected]
   with:
     dockerfile: images/${{ inputs.directory }}/Dockerfile
     failure-threshold: warning  # Or "error" to fail builds on issues ```

**Additional context**:
Consider setting `failure-threshold: error` to enforce linting standards. The action also supports configuration files for rule customization.

---

### Issue 4: Add Input Validation for Security
**Title**: Add input validation to prevent injection attacks
**Labels**: `security`, `robustness`

**Problem**:
The workflow uses user-provided inputs directly in commands without
validation:
```yaml
run: sed -i '1i FROM ${{ env.BASE_IMAGE}} as ${{ inputs.image }}' 
./images/${{ inputs.directory }}/Dockerfile ```

**Why it's problematic**:
1. Potential for command injection if inputs contain malicious characters 2. No validation ensures inputs meet expected formats 3. Risk of unexpected behavior from malformed inputs

**Suggested solution**:
Add validation steps before using inputs:
```yaml
- name: Validate inputs
   run: |
     # Validate image name format
     if [[ ! "${{ inputs.image }}" =~ ^[a-z0-9]([a-z0-9_-]*[a-z0-9])?$ ]]; then
       echo "::error::Invalid image name format"
       exit 1
     fi

     # Validate directory exists
     if [ ! -d "images/${{ inputs.directory }}" ]; then
       echo "::error::Directory does not exist"
       exit 1
     fi

Additional context: Consider using GitHub's actions/github-script for more complex validation. This is especially important after fixing Issue 1 to prevent similar vulnerabilities in the build process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment