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:5000Why it's problematic:
- 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.
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-failWhy it's problematic:
- Reinvents functionality available in maintained GitHub Actions 2. Uses
--no-failwhich 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
fiAdditional 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.