Skip to content

ci: fix Docker tag format and add version pinning for transport builds #132

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
akshaydeo merged 1 commit into from
Jun 26, 2025
Merged

ci: fix Docker tag format and add version pinning for transport builds #132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions .github/workflows/transport-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,18 +145,22 @@ jobs:

echo "tag=${TAG}" >> $GITHUB_OUTPUT

# Extract version from tag
VERSION=${TAG#transports/v}
# Extract version from tag (remove transports/ prefix)
VERSION=${TAG#transports/}
echo "version=${VERSION}" >> $GITHUB_OUTPUT

# Extract numeric version for validation
NUMERIC_VERSION=${VERSION#v}

# Validate version format
if ! echo "$VERSION" | grep -qE '^[0-9]+\.[0-9]+\.[0-9]+$'; then
if ! echo "$NUMERIC_VERSION" | grep -qE '^[0-9]+\.[0-9]+\.[0-9]+$'; then
Comment on lines +148 to +156
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jun 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Path-traversal & whitespace hardening for extracted version

VERSION=${TAG#transports/} and NUMERIC_VERSION=${VERSION#v} rely on the incoming tag being perfectly formed.
If a malicious (or simply mistyped) tag like transports/v1.2.3$(printf '\nX') were pushed, the newline would bleed into subsequent echo >> $GITHUB_OUTPUT lines and break the workflow / poison the build args.

Add simple sanitation:

VERSION=${TAG#transports/}
VERSION=${VERSION//$'\n'/}     # strip newlines
VERSION=${VERSION//$'\r'/}     # strip carriage returns

or use printf '%s\n' instead of bare echo when writing to $GITHUB_OUTPUT.

Tiny change – big safety net.

🤖 Prompt for AI Agents 
In .github/workflows/transport-ci.yml around lines 148 to 156, the extraction of
VERSION and NUMERIC_VERSION from TAG does not sanitize input, allowing newlines
or carriage returns to corrupt the output and break the workflow. To fix this,
after extracting VERSION, strip any newline and carriage return characters from
it using parameter expansion to remove $'\n' and $'\r'. Alternatively, replace
the bare echo commands writing to $GITHUB_OUTPUT with printf '%s\n' to safely
handle special characters. This sanitization prevents path traversal and
whitespace injection issues.

echo "Error: Invalid tag format '$TAG'. Expected format: transports/vMAJOR.MINOR.PATCH"
exit 1
fi

# Create image tags (Docker tags cannot contain slashes, so use version only)
echo "tags<<EOF" >> $GITHUB_OUTPUT
echo "${{ env.REGISTRY }}/${{ env.ACCOUNT }}/${{ env.IMAGE_NAME }}:v${VERSION}" >> $GITHUB_OUTPUT
echo "${{ env.REGISTRY }}/${{ env.ACCOUNT }}/${{ env.IMAGE_NAME }}:${VERSION}" >> $GITHUB_OUTPUT
echo "${{ env.REGISTRY }}/${{ env.ACCOUNT }}/${{ env.IMAGE_NAME }}:latest" >> $GITHUB_OUTPUT
echo "EOF" >> $GITHUB_OUTPUT
Comment on lines 162 to 165
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jun 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Use the official multi-line $GITHUB_OUTPUT syntax to avoid missing delimiter errors

Manually echoing the tags<<EOF … EOF sequence is error-prone.
The recommended pattern is:

{
  echo 'tags<<EOF'
  echo "${{ env.REGISTRY }}/${{ env.ACCOUNT }}/${{ env.IMAGE_NAME }}:${VERSION}"
  echo "${{ env.REGISTRY }}/${{ env.ACCOUNT }}/${{ env.IMAGE_NAME }}:latest"
  echo 'EOF'
} >>"$GITHUB_OUTPUT"

Wrapping the block in braces guarantees the heredoc is kept together and reduces chances of accidental redirection mistakes during future edits.

🤖 Prompt for AI Agents 
In .github/workflows/transport-ci.yml around lines 162 to 165, replace the
multiple echo commands appending to $GITHUB_OUTPUT individually with a single
block wrapped in braces that echoes the heredoc lines together. This ensures the
multi-line output is written atomically and prevents missing delimiter errors by
grouping the echo statements inside braces and redirecting their combined output
once to $GITHUB_OUTPUT.


Expand Down Expand Up @@ -192,6 +196,7 @@ jobs:
org.opencontainers.image.revision=${{ github.sha }}
build-args: |
TRANSPORT_TYPE=http
TAG_VERSION=${{ steps.meta.outputs.version }}
Comment on lines 198 to +199
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jun 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Minor: keep build-arg list transport-agnostic

TRANSPORT_TYPE=http is hard-coded here while the rest of the pipeline is tag-driven. When a non-HTTP transport is tagged this step will still label the image “HTTP”, risking confusion.

Consider:

build-args: |
  TRANSPORT_TYPE=${{ env.TRANSPORT_TYPE }}
  TAG_VERSION=${{ steps.meta.outputs.version }}

and source TRANSPORT_TYPE from the tag or job matrix to stay future-proof.

🤖 Prompt for AI Agents 
In .github/workflows/transport-ci.yml at lines 198-199, the TRANSPORT_TYPE build
argument is hard-coded to "http", which conflicts with the tag-driven approach
and can cause confusion for non-HTTP transports. Update the build-args to use
TRANSPORT_TYPE from the environment variable instead, like TRANSPORT_TYPE=${{
env.TRANSPORT_TYPE }}, and ensure TRANSPORT_TYPE is set appropriately from the
tag or job matrix to keep the pipeline transport-agnostic and future-proof.

platforms: linux/amd64,linux/arm64
cache-from: type=gha
cache-to: type=gha,mode=max
Expand Down
7 changes: 4 additions & 3 deletions transports/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@ RUN apk add --no-cache upx
# Set environment for static build
ENV CGO_ENABLED=0 GOOS=linux GOARCH=amd64

# Define build-time variable for transport type
# Define build-time variables
ARG TRANSPORT_TYPE=http
ARG TAG_VERSION=latest

# Initialize go module and get bifrost-http
# Initialize go module and get bifrost with specified tag
RUN go mod init bifrost-build && \
go get github.com/maximhq/bifrost/transports/bifrost-${TRANSPORT_TYPE}@latest
go get github.com/maximhq/bifrost/transports/bifrost-${TRANSPORT_TYPE}@${TAG_VERSION}
Comment on lines +13 to +17
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jun 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Consider switching from go get to go install (or go mod download) for reproducible, forward-compatible builds

go get mutates go.mod/go.sum and has been discouraged for fetching binaries since Go 1.20.
Using go install (for direct binary installation) or a plain go mod download && go build keeps the module graph immutable and avoids the deprecation warning that will soon turn into an error.

-ARG TAG_VERSION=latest
-# Initialize go module and get bifrost with specified tag
-RUN go mod init bifrost-build && \
-    go get github.com/maximhq/bifrost/transports/bifrost-${TRANSPORT_TYPE}@${TAG_VERSION}
+# Explicit module initialisation
+ARG TAG_VERSION=latest
+RUN go mod init bifrost-build && \
+    go mod tidy && \
+    go install github.com/maximhq/bifrost/transports/bifrost-${TRANSPORT_TYPE}@${TAG_VERSION} && \
+    go mod download

This keeps the Docker layer deterministic, avoids side-effects in go.mod, and aligns with the Go tooling roadmap.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ARG TAG_VERSION=latest
# Initialize go module and get bifrost-http
# Initialize go module and get bifrost with specified tag
RUN go mod init bifrost-build && \
go get github.com/maximhq/bifrost/transports/bifrost-${TRANSPORT_TYPE}@latest
go get github.com/maximhq/bifrost/transports/bifrost-${TRANSPORT_TYPE}@${TAG_VERSION}
ARG TAG_VERSION=latest
# Explicit module initialization
RUN go mod init bifrost-build && \
go mod tidy && \
go install github.com/maximhq/bifrost/transports/bifrost-${TRANSPORT_TYPE}@${TAG_VERSION} && \
go mod download
🤖 Prompt for AI Agents 
In transports/Dockerfile around lines 13 to 17, replace the use of 'go get' with
'go install' or 'go mod download' followed by 'go build' to fetch the bifrost
module. This avoids mutating go.mod and go.sum files during the Docker build,
ensuring reproducible and forward-compatible builds. Adjust the commands to
install or build the binary without changing the module graph, maintaining
deterministic Docker layers and preventing future deprecation errors.


# Build the binary locally
RUN go build \
Expand Down