Skip to content

Conversation

@jonpspri
Copy link
Contributor

🐛 Bug-fix PR

Before opening this PR please:

  1. make lint - passes ruff, mypy, pylint
  2. make test - all unit + integration tests green
  3. make coverage - ≥ 90 %
  4. make docker docker-run-ssl or make podman podman-run-ssl
  5. Update relevant documentation.
  6. Tested with sqlite and postgres + redis.
  7. Manual regression no longer fails. Ensure the UI and /version work correctly.

📌 Summary

Go Docker images built from scratch run as root

🔁 Reproduction Steps

Issue #1227

🐞 Root Cause

Images did not set USER

💡 Fix Description

Set USER to 1001

🧪 Verification

Check Command Status
Lint suite make lint Unrelated errors and failures
Unit tests make test 2 Unrelated failures
Coverage ≥ 90 % make coverage 71% - Unrelated to changes
Manual regression no longer fails steps / screenshots Tested go modules per Makefiles

📐 MCP Compliance (if relevant)

  • [-] Matches current MCP spec
  • [-] No breaking change to MCP clients

✅ Checklist

  • [-] Code formatted (make black isort pre-commit)
  • No secrets/credentials committed

Copilot AI review requested due to automatic review settings October 12, 2025 20:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a security issue where Go Docker images built from scratch were running as root by adding a non-root user configuration. The fix addresses issue #1227 by setting the USER directive to 1001:1001 in all Go container Dockerfiles.

Key changes:

  • Added USER 1001:1001 directive to run containers as non-root
  • Standardized multi-platform build comments across Dockerfiles
  • Fixed LABEL syntax for better compliance

Reviewed Changes

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

File Description
mcp-servers/templates/go/Dockerfile.jinja Template updated with non-root user and standardized comments
mcp-servers/go/fast-time-server/Dockerfile Applied non-root user fix and comment standardization
mcp-servers/go/benchmark-server/Dockerfile Applied non-root user fix with minimal changes

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.


FROM --platform=$TARGETPLATFORM golang:{{ go_version }} AS builder
# This build supports multi-platform builds through standard Docker Buildx
# techniques, including the $TARGETPLATFORM environment veriable.
Copy link

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'veriable' to 'variable'.

Suggested change
# techniques, including the $TARGETPLATFORM environment veriable.
# techniques, including the $TARGETPLATFORM environment variable.

Copilot uses AI. Check for mistakes.
# # now visit http://localhost:8080/sse or http://localhost:8080/http
#
# This build supports multi-platform builds through standard Docker Buildx
# techniques, including the $TARGETPLATFORM environment veriable.
Copy link

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'veriable' to 'variable'.

Suggested change
# techniques, including the $TARGETPLATFORM environment veriable.
# techniques, including the $TARGETPLATFORM environment variable.

Copilot uses AI. Check for mistakes.
@@ -1,4 +1,6 @@
FROM --platform=$TARGETPLATFORM golang:1.23 AS builder
# This build supports multi-platform builds through standard Docker Buildx
# techniques, including the $TARGETPLATFORM environment veriable.
Copy link

Copilot AI Oct 12, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'veriable' to 'variable'.

Suggested change
# techniques, including the $TARGETPLATFORM environment veriable.
# techniques, including the $TARGETPLATFORM environment variable.

Copilot uses AI. Check for mistakes.
@crivetimihai
Copy link
Member

Oh, cool! Yeah, the mcp-server images need the same attention as the regular contextforge build, thanks for the PR ;-)

Copy link
Member

@crivetimihai crivetimihai left a comment

Choose a reason for hiding this comment

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

Thanks!

@crivetimihai crivetimihai merged commit 6c077dd into IBM:main Oct 12, 2025
35 checks passed
@jonpspri jonpspri deleted the secure-go-containers branch October 13, 2025 06:53
p4yl04d3r pushed a commit to p4yl04d3r/mcp-context-forge that referenced this pull request Nov 19, 2025
* fix: Add non-root user to scratch go containers

* fix: spelling corrections

Signed-off-by: p4yl04d3r <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants