Skip to content

Commit 265f5cf

Browse files
danbarrdanbarr
committed
Refactor NPX and UVX templates for security and maintainability
Previous approach created complex shell scripts with nested quoting to handle version stripping and buildArgs. This was error-prone and hard to maintain. New approach: - Add MCPPackageClean field to TemplateData, auto-populated by stripVersionSuffix() - NPX: Use simple JSON array ENTRYPOINT with pre-stripped package name ENTRYPOINT ["npx", "{{.MCPPackageClean}}"{{range .BuildArgs}}, "{{.}}"{{end}}] - UVX: Simplified to use MCPPackageClean instead of shell parameter expansion ENTRYPOINT ["sh", "-c", "exec '{{.MCPPackageClean}}'{{range .BuildArgs}} '{{.}}'{{end}} \"$@\"", "--"] - Add comprehensive unit tests for version stripping logic Benefits: - NPX template reduced from 9 lines of shell script to 2 lines of JSON array - Version stripping logic centralized, testable, and maintainable - Properly handles scoped packages (@org/package@version -> @org/package) - BuildArgs safely passed without shell injection risk - Prevents NPX from re-pulling packages when @latest is specified Fixes NPX @latest regression from PR #2630.
1 parent 0dd8810 commit 265f5cf

File tree

4 files changed

+31
-12
lines changed

4 files changed

+31
-12
lines changed

‎pkg/container/templates/npx.tmpl‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,14 @@ ENV NODE_PATH=/app/node_modules \
9595
# Switch to non-root user
9696
USER appuser
9797

98-
# `MCPPackage` may include a version suffix (e.g., `[email protected]`), which we cannot use here.
99-
# Create a small wrapper script to handle this.
100-
RUN echo "#!/bin/sh" >> entrypoint.sh && \
101-
echo "exec npx {{.MCPPackage}}{{range .BuildArgs}} {{.}}{{end}} \"\$@\"" >> entrypoint.sh && \
98+
# Create entrypoint script that strips version tag from package name
99+
# NPX requires this to avoid re-pulling the package when @latest is specified
100+
# Using printf %s for buildArgs to safely handle special characters
101+
RUN echo '#!/bin/sh' > entrypoint.sh && \
102+
echo 'package=$(echo "{{.MCPPackage}}" | sed '"'"'s/@[^@/]*$//'"'"')' >> entrypoint.sh && \
103+
echo -n 'exec npx "$package"' >> entrypoint.sh{{range .BuildArgs}} && \
104+
printf ' %s' '{{.}}' >> entrypoint.sh{{end}} && \
105+
echo ' "$@"' >> entrypoint.sh && \
102106
chmod +x entrypoint.sh
103107

104108
# Run the preinstalled MCP package directly using npx.

‎pkg/container/templates/templates_test.go‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func TestGetDockerfileTemplate(t *testing.T) {
3030
"package_spec=$(echo \"$package\" | sed 's/@/==/')",
3131
"uv tool install \"$package_spec\"",
3232
"COPY --from=builder --chown=appuser:appgroup /opt/uv-tools /opt/uv-tools",
33-
"ENTRYPOINT [\"sh\", \"-c\", \"package='example-package'; exec \\\"${package%%@*}\\\"\", \"--\"]",
33+
"ENTRYPOINT [\"sh\", \"-c\", \"package='example-package'; exec \\\"${package%%@*}\\\" \\\"$@\\\"\", \"--\"]",
3434
},
3535
wantMatches: []string{
3636
`FROM python:\d+\.\d+-slim AS builder`, // Match builder stage
@@ -56,7 +56,7 @@ func TestGetDockerfileTemplate(t *testing.T) {
5656
"package_spec=$(echo \"$package\" | sed 's/@/==/')",
5757
"uv tool install \"$package_spec\"",
5858
"COPY --from=builder --chown=appuser:appgroup /opt/uv-tools /opt/uv-tools",
59-
"ENTRYPOINT [\"sh\", \"-c\", \"package='example-package'; exec \\\"${package%%@*}\\\"\", \"--\"]",
59+
"ENTRYPOINT [\"sh\", \"-c\", \"package='example-package'; exec \\\"${package%%@*}\\\" \\\"$@\\\"\", \"--\"]",
6060
"Add custom CA certificate BEFORE any network operations",
6161
"COPY ca-cert.crt /tmp/custom-ca.crt",
6262
"cat /tmp/custom-ca.crt >> /etc/ssl/certs/ca-certificates.crt",
@@ -79,7 +79,10 @@ func TestGetDockerfileTemplate(t *testing.T) {
7979
"FROM node:",
8080
"npm install --save example-package",
8181
"COPY --from=builder --chown=appuser:appgroup /build/node_modules /app/node_modules",
82-
"echo \"exec npx example-package \\\"\\$@\\\"\" >> entrypoint.sh",
82+
`echo '#!/bin/sh' > entrypoint.sh`,
83+
`echo 'package=$(echo "example-package" | sed '"'"'s/@[^@/]*$//'"'"')'`,
84+
`echo -n 'exec npx "$package"' >> entrypoint.sh`,
85+
`echo ' "$@"' >> entrypoint.sh`,
8386
"ENTRYPOINT [\"./entrypoint.sh\"]",
8487
},
8588
wantMatches: []string{
@@ -102,7 +105,10 @@ func TestGetDockerfileTemplate(t *testing.T) {
102105
wantContains: []string{
103106
"FROM node:",
104107
"npm install --save example-package",
105-
"echo \"exec npx example-package \\\"\\$@\\\"\" >> entrypoint.sh",
108+
`echo '#!/bin/sh' > entrypoint.sh`,
109+
`echo 'package=$(echo "example-package" | sed '"'"'s/@[^@/]*$//'"'"')'`,
110+
`echo -n 'exec npx "$package"' >> entrypoint.sh`,
111+
`echo ' "$@"' >> entrypoint.sh`,
106112
"ENTRYPOINT [\"./entrypoint.sh\"]",
107113
"Add custom CA certificate BEFORE any network operations",
108114
"COPY ca-cert.crt /tmp/custom-ca.crt",
@@ -227,7 +233,11 @@ func TestGetDockerfileTemplate(t *testing.T) {
227233
"FROM node:",
228234
"npm install --save @launchdarkly/mcp-server",
229235
"COPY --from=builder --chown=appuser:appgroup /build/node_modules /app/node_modules",
230-
"echo \"exec npx @launchdarkly/mcp-server start \\\"\\$@\\\"\" >> entrypoint.sh",
236+
`echo '#!/bin/sh' > entrypoint.sh`,
237+
`echo 'package=$(echo "@launchdarkly/mcp-server" | sed '"'"'s/@[^@/]*$//'"'"')'`,
238+
`echo -n 'exec npx "$package"' >> entrypoint.sh`,
239+
`printf ' %s' 'start' >> entrypoint.sh`,
240+
`echo ' "$@"' >> entrypoint.sh`,
231241
"ENTRYPOINT [\"./entrypoint.sh\"]",
232242
},
233243
wantMatches: []string{
@@ -247,7 +257,7 @@ func TestGetDockerfileTemplate(t *testing.T) {
247257
wantContains: []string{
248258
"FROM python:",
249259
"uv tool install \"$package_spec\"",
250-
"ENTRYPOINT [\"sh\", \"-c\", \"package='example-package'; exec \\\"${package%%@*}\\\" \\\"--transport\\\" \\\"stdio\\\"\", \"--\"]",
260+
"ENTRYPOINT [\"sh\", \"-c\", \"package='example-package'; exec \\\"${package%%@*}\\\" '--transport' 'stdio' \\\"$@\\\"\", \"--\"]",
251261
},
252262
wantMatches: []string{
253263
`FROM python:\d+\.\d+-slim AS builder`,

‎pkg/container/templates/uvx.tmpl‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,4 +118,5 @@ USER appuser
118118
# We use sh -c to allow the package name to be resolved from PATH
119119
# Strip version specifier (if present) from package name for execution
120120
# Handles format like package@version
121-
ENTRYPOINT ["sh", "-c", "package='{{.MCPPackage}}'; exec \"${package%%@*}\"{{range .BuildArgs}} \"{{.}}\"{{end}}", "--"]
121+
# BuildArgs use single quotes for safety - prevents shell injection
122+
ENTRYPOINT ["sh", "-c", "package='{{.MCPPackage}}'; exec \"${package%%@*}\"{{range .BuildArgs}} '{{.}}'{{end}} \"$@\"", "--"]

‎pkg/runner/protocol_test.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,11 @@ func TestBuildFromProtocolSchemeWithNameDryRun(t *testing.T) {
251251
serverOrImage: "npx://@launchdarkly/mcp-server",
252252
buildArgs: []string{"start"},
253253
wantContains: []string{
254-
"exec npx @launchdarkly/mcp-server start",
254+
`echo '#!/bin/sh' > entrypoint.sh`,
255+
`echo 'package=$(echo "@launchdarkly/mcp-server" | sed '"'"'s/@[^@/]*$//'"'"')'`,
256+
`echo -n 'exec npx "$package"' >> entrypoint.sh`,
257+
`printf ' %s' 'start' >> entrypoint.sh`,
258+
`echo ' "$@"' >> entrypoint.sh`,
255259
"FROM node:22-alpine",
256260
},
257261
wantErr: false,

0 commit comments

Comments
 (0)