From 4aa66fc2452a72b0c631ea09b2ea32150873622d Mon Sep 17 00:00:00 2001 From: Dan Barr <6922515+danbarr@users.noreply.github.com> Date: Mon, 17 Nov 2025 22:52:49 -0500 Subject: [PATCH 01/14] Add BuildArgs support to TemplateData and all templates - Add BuildArgs field to TemplateData struct for baking required CLI arguments into container ENTRYPOINT at build time - Update npx.tmpl to insert BuildArgs before "$@" - Update uvx.tmpl to insert BuildArgs before "$@" - Update go.tmpl to insert BuildArgs in JSON array format BuildArgs are for required subcommands (e.g., 'start') while runtime args passed via '-- ' append after. This prevents duplication issues that led to MCPArgs removal in PR #2630. Related to: stacklok/dockyard#189 --- pkg/container/templates/go.tmpl | 2 +- pkg/container/templates/npx.tmpl | 2 +- pkg/container/templates/templates.go | 4 ++++ pkg/container/templates/uvx.tmpl | 2 +- 4 files changed, 7 insertions(+), 3 deletions(-) diff --git a/pkg/container/templates/go.tmpl b/pkg/container/templates/go.tmpl index 678ce78bf..7250b54bc 100644 --- a/pkg/container/templates/go.tmpl +++ b/pkg/container/templates/go.tmpl @@ -99,4 +99,4 @@ COPY --from=builder --chown=appuser:appgroup /build/ /app/ USER appuser # Run the pre-built MCP server binary -ENTRYPOINT ["/app/mcp-server"] \ No newline at end of file +ENTRYPOINT ["/app/mcp-server"{{range .BuildArgs}}, "{{.}}{{end}}"] \ No newline at end of file diff --git a/pkg/container/templates/npx.tmpl b/pkg/container/templates/npx.tmpl index 98ea9a467..dbff031e0 100644 --- a/pkg/container/templates/npx.tmpl +++ b/pkg/container/templates/npx.tmpl @@ -98,7 +98,7 @@ USER appuser # `MCPPackage` may include a version suffix (e.g., `package@1.2.3`), which we cannot use here. # Create a small wrapper script to handle this. RUN echo "#!/bin/sh" >> entrypoint.sh && \ - echo "exec npx {{.MCPPackage}} \"\$@\"" >> entrypoint.sh && \ + echo "exec npx {{.MCPPackage}}{{range .BuildArgs}} {{.}}{{end}} \"\$@\"" >> entrypoint.sh && \ chmod +x entrypoint.sh # Run the preinstalled MCP package directly using npx. diff --git a/pkg/container/templates/templates.go b/pkg/container/templates/templates.go index a460ae8fa..6135c7a10 100644 --- a/pkg/container/templates/templates.go +++ b/pkg/container/templates/templates.go @@ -20,6 +20,10 @@ type TemplateData struct { CACertContent string // IsLocalPath indicates if the MCPPackage is a local path that should be copied into the container. IsLocalPath bool + // BuildArgs are the arguments to bake into the container's ENTRYPOINT at build time. + // These are typically required subcommands (e.g., "start") that must always be present. + // Runtime arguments passed via "-- " will be appended after these build args. + BuildArgs []string } // TransportType represents the type of transport to use. diff --git a/pkg/container/templates/uvx.tmpl b/pkg/container/templates/uvx.tmpl index 3b9849392..e797e283b 100644 --- a/pkg/container/templates/uvx.tmpl +++ b/pkg/container/templates/uvx.tmpl @@ -118,4 +118,4 @@ USER appuser # We use sh -c to allow the package name to be resolved from PATH # Strip version specifier (if present) from package name for execution # Handles format like package@version -ENTRYPOINT ["sh", "-c", "package='{{.MCPPackage}}'; exec \"${package%%@*}\"", "--"] \ No newline at end of file +ENTRYPOINT ["sh", "-c", "package='{{.MCPPackage}}'; exec \"${package%%@*}\"{{range .BuildArgs}} \"{{.}}\"{{end}}", "--"] \ No newline at end of file From a36944ad65126982956b80eed917a8e86339e521 Mon Sep 17 00:00:00 2001 From: Dan Barr <6922515+danbarr@users.noreply.github.com> Date: Mon, 17 Nov 2025 22:55:07 -0500 Subject: [PATCH 02/14] Update function signatures to accept buildArgs parameter - Add buildArgs []string parameter to BuildFromProtocolSchemeWithName - Update createTemplateData to accept and use buildArgs - Update HandleProtocolScheme to pass nil for buildArgs - Update thv build command callers to pass nil for buildArgs Existing callers pass nil to maintain backward compatibility. --- cmd/thv/app/build.go | 4 ++-- pkg/runner/protocol.go | 11 +++++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/cmd/thv/app/build.go b/cmd/thv/app/build.go index 1fce82d4d..841933c4a 100644 --- a/cmd/thv/app/build.go +++ b/cmd/thv/app/build.go @@ -74,7 +74,7 @@ func buildCmdFunc(cmd *cobra.Command, args []string) error { // If dry-run or output is specified, just generate the Dockerfile if buildFlags.DryRun || buildFlags.Output != "" { - dockerfileContent, err := runner.BuildFromProtocolSchemeWithName(ctx, imageManager, protocolScheme, "", buildFlags.Tag, true) + dockerfileContent, err := runner.BuildFromProtocolSchemeWithName(ctx, imageManager, protocolScheme, "", buildFlags.Tag, nil, true) if err != nil { return fmt.Errorf("failed to generate Dockerfile for %s: %v", protocolScheme, err) } @@ -96,7 +96,7 @@ func buildCmdFunc(cmd *cobra.Command, args []string) error { logger.Infof("Building container for protocol scheme: %s", protocolScheme) // Build the image using the new protocol handler with custom name - imageName, err := runner.BuildFromProtocolSchemeWithName(ctx, imageManager, protocolScheme, "", buildFlags.Tag, false) + imageName, err := runner.BuildFromProtocolSchemeWithName(ctx, imageManager, protocolScheme, "", buildFlags.Tag, nil, false) if err != nil { return fmt.Errorf("failed to build container for %s: %v", protocolScheme, err) } diff --git a/pkg/runner/protocol.go b/pkg/runner/protocol.go index 80046da94..af709628b 100644 --- a/pkg/runner/protocol.go +++ b/pkg/runner/protocol.go @@ -32,12 +32,13 @@ func HandleProtocolScheme( serverOrImage string, caCertPath string, ) (string, error) { - return BuildFromProtocolSchemeWithName(ctx, imageManager, serverOrImage, caCertPath, "", false) + return BuildFromProtocolSchemeWithName(ctx, imageManager, serverOrImage, caCertPath, "", nil, false) } // BuildFromProtocolSchemeWithName checks if the serverOrImage string contains a protocol scheme (uvx://, npx://, or go://) // and builds a Docker image for it if needed with a custom image name. // If imageName is empty, a default name will be generated. +// buildArgs are baked into the container's ENTRYPOINT at build time (e.g., required subcommands). // If dryRun is true, returns the Dockerfile content instead of building the image. // Returns the Docker image name (or Dockerfile content if dryRun) and any error encountered. func BuildFromProtocolSchemeWithName( @@ -46,6 +47,7 @@ func BuildFromProtocolSchemeWithName( serverOrImage string, caCertPath string, imageName string, + buildArgs []string, dryRun bool, ) (string, error) { transportType, packageName, err := ParseProtocolScheme(serverOrImage) @@ -53,7 +55,7 @@ func BuildFromProtocolSchemeWithName( return "", err } - templateData, err := createTemplateData(transportType, packageName, caCertPath) + templateData, err := createTemplateData(transportType, packageName, caCertPath, buildArgs) if err != nil { return "", err } @@ -84,14 +86,15 @@ func ParseProtocolScheme(serverOrImage string) (templates.TransportType, string, return "", "", fmt.Errorf("unsupported protocol scheme: %s", serverOrImage) } -// createTemplateData creates the template data with optional CA certificate. -func createTemplateData(transportType templates.TransportType, packageName, caCertPath string) (templates.TemplateData, error) { +// createTemplateData creates the template data with optional CA certificate and build arguments. +func createTemplateData(transportType templates.TransportType, packageName, caCertPath string, buildArgs []string) (templates.TemplateData, error) { // Check if this is a local path (for Go packages only) isLocalPath := transportType == templates.TransportTypeGO && isLocalGoPath(packageName) templateData := templates.TemplateData{ MCPPackage: packageName, IsLocalPath: isLocalPath, + BuildArgs: buildArgs, } if caCertPath != "" { From 52e8d83fe701c0f40e23da80dd50e4c02591bf2e Mon Sep 17 00:00:00 2001 From: Dan Barr <6922515+danbarr@users.noreply.github.com> Date: Mon, 17 Nov 2025 22:59:50 -0500 Subject: [PATCH 03/14] Fix template syntax error in go.tmpl ENTRYPOINT Fixed extra quote in ENTRYPOINT template that was causing test failures. The template now correctly generates: - Empty buildArgs: ENTRYPOINT ["/app/mcp-server"] - With buildArgs: ENTRYPOINT ["/app/mcp-server", "arg1", "arg2"] --- pkg/container/templates/go.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/container/templates/go.tmpl b/pkg/container/templates/go.tmpl index 7250b54bc..b55f0fa0b 100644 --- a/pkg/container/templates/go.tmpl +++ b/pkg/container/templates/go.tmpl @@ -99,4 +99,4 @@ COPY --from=builder --chown=appuser:appgroup /build/ /app/ USER appuser # Run the pre-built MCP server binary -ENTRYPOINT ["/app/mcp-server"{{range .BuildArgs}}, "{{.}}{{end}}"] \ No newline at end of file +ENTRYPOINT ["/app/mcp-server"{{range .BuildArgs}}, "{{.}}"{{end}}] \ No newline at end of file From c082400f7221c94b9ce4ea462066b1804dd383a6 Mon Sep 17 00:00:00 2001 From: Dan Barr <6922515+danbarr@users.noreply.github.com> Date: Mon, 17 Nov 2025 23:01:05 -0500 Subject: [PATCH 04/14] Add test cases for BuildArgs functionality Added test cases for all three transport types (NPX, UVX, GO) that exercise the BuildArgs feature: - NPX with single arg (start) - UVX with multiple args (--transport stdio) - GO with multiple args (serve --verbose) All tests verify that BuildArgs are correctly baked into the ENTRYPOINT. --- pkg/container/templates/templates_test.go | 60 +++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/pkg/container/templates/templates_test.go b/pkg/container/templates/templates_test.go index 52623581a..0474b4678 100644 --- a/pkg/container/templates/templates_test.go +++ b/pkg/container/templates/templates_test.go @@ -216,6 +216,66 @@ func TestGetDockerfileTemplate(t *testing.T) { }, wantErr: false, }, + { + name: "NPX transport with BuildArgs", + transportType: TransportTypeNPX, + data: TemplateData{ + MCPPackage: "@launchdarkly/mcp-server", + BuildArgs: []string{"start"}, + }, + wantContains: []string{ + "FROM node:", + "npm install --save @launchdarkly/mcp-server", + "COPY --from=builder --chown=appuser:appgroup /build/node_modules /app/node_modules", + "echo \"exec npx @launchdarkly/mcp-server start \\\"\\$@\\\"\" >> entrypoint.sh", + "ENTRYPOINT [\"./entrypoint.sh\"]", + }, + wantMatches: []string{ + `FROM node:\d+-alpine AS builder`, + `FROM node:\d+-alpine`, + }, + wantNotContains: nil, + wantErr: false, + }, + { + name: "UVX transport with BuildArgs", + transportType: TransportTypeUVX, + data: TemplateData{ + MCPPackage: "example-package", + BuildArgs: []string{"--transport", "stdio"}, + }, + wantContains: []string{ + "FROM python:", + "uv tool install \"$package_spec\"", + "ENTRYPOINT [\"sh\", \"-c\", \"package='example-package'; exec \\\"${package%%@*}\\\" \\\"--transport\\\" \\\"stdio\\\"\", \"--\"]", + }, + wantMatches: []string{ + `FROM python:\d+\.\d+-slim AS builder`, + `FROM python:\d+\.\d+-slim`, + }, + wantNotContains: nil, + wantErr: false, + }, + { + name: "GO transport with BuildArgs", + transportType: TransportTypeGO, + data: TemplateData{ + MCPPackage: "example-package", + BuildArgs: []string{"serve", "--verbose"}, + }, + wantContains: []string{ + "FROM golang:", + "go install \"$package\"", + "FROM alpine:", + "ENTRYPOINT [\"/app/mcp-server\", \"serve\", \"--verbose\"]", + }, + wantMatches: []string{ + `FROM golang:\d+\.\d+-alpine AS builder`, + `FROM alpine:\d+\.\d+`, + }, + wantNotContains: nil, + wantErr: false, + }, { name: "Unsupported transport", transportType: "unsupported", From 73276f8961381082a0401fcfc1ffc27c6b4c2768 Mon Sep 17 00:00:00 2001 From: Dan Barr <6922515+danbarr@users.noreply.github.com> Date: Mon, 17 Nov 2025 23:09:44 -0500 Subject: [PATCH 05/14] Add build-time args support to thv build command Users can now bake required arguments into containers at build time: $ thv build npx://@launchdarkly/mcp-server -- start $ thv build uvx://package -- --transport stdio This provides: - Consistency with 'thv run -- ' syntax - Easy local testing of build args before using in Dockyard - Arguments are baked into ENTRYPOINT, not overridable at runtime Examples: - NPX: exec npx package start "$@" - UVX: exec package "--transport" "stdio" - GO: ENTRYPOINT ["/app/mcp-server", "serve", "--verbose"] Related to: stacklok/dockyard#189 --- cmd/thv/app/build.go | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/cmd/thv/app/build.go b/cmd/thv/app/build.go index 841933c4a..2091e4b00 100644 --- a/cmd/thv/app/build.go +++ b/cmd/thv/app/build.go @@ -12,7 +12,7 @@ import ( ) var buildCmd = &cobra.Command{ - Use: "build [flags] PROTOCOL", + Use: "build [flags] PROTOCOL [-- ARGS...]", Short: "Build a container for an MCP server without running it", Long: `Build a container for an MCP server using a protocol scheme without running it. @@ -28,15 +28,28 @@ using either uvx (Python with uv package manager), npx (Node.js), or go (Golang). For Go, you can also specify local paths starting with './' or '../' to build local Go projects. +Build-time arguments can be baked into the container's ENTRYPOINT: + + $ thv build npx://@launchdarkly/mcp-server -- start + $ thv build uvx://package -- --transport stdio + +These arguments become part of the container image and will always run, +with runtime arguments (from 'thv run -- ') appending after them. + The container will be built and tagged locally, ready to be used with 'thv run' or other container tools. The built image name will be displayed upon successful completion. Examples: $ thv build uvx://mcp-server-git $ thv build --tag my-custom-name:latest npx://@modelcontextprotocol/server-filesystem - $ thv build go://./my-local-server`, - Args: cobra.ExactArgs(1), + $ thv build go://./my-local-server + $ thv build npx://@launchdarkly/mcp-server -- start`, + Args: cobra.MinimumNArgs(1), RunE: buildCmdFunc, + // Ignore unknown flags to allow passing args after -- + FParseErrWhitelist: cobra.FParseErrWhitelist{ + UnknownFlags: true, + }, } var buildFlags BuildFlags @@ -69,12 +82,16 @@ func buildCmdFunc(cmd *cobra.Command, args []string) error { return fmt.Errorf("invalid protocol scheme: %s. Supported schemes are: uvx://, npx://, go://", protocolScheme) } + // Parse build arguments using os.Args to find everything after -- + buildArgs := parseCommandArguments(os.Args) + logger.Debugf("Build args: %v", buildArgs) + // Create image manager (even for dry-run, we pass it but it won't be used) imageManager := images.NewImageManager(ctx) // If dry-run or output is specified, just generate the Dockerfile if buildFlags.DryRun || buildFlags.Output != "" { - dockerfileContent, err := runner.BuildFromProtocolSchemeWithName(ctx, imageManager, protocolScheme, "", buildFlags.Tag, nil, true) + dockerfileContent, err := runner.BuildFromProtocolSchemeWithName(ctx, imageManager, protocolScheme, "", buildFlags.Tag, buildArgs, true) if err != nil { return fmt.Errorf("failed to generate Dockerfile for %s: %v", protocolScheme, err) } @@ -96,7 +113,7 @@ func buildCmdFunc(cmd *cobra.Command, args []string) error { logger.Infof("Building container for protocol scheme: %s", protocolScheme) // Build the image using the new protocol handler with custom name - imageName, err := runner.BuildFromProtocolSchemeWithName(ctx, imageManager, protocolScheme, "", buildFlags.Tag, nil, false) + imageName, err := runner.BuildFromProtocolSchemeWithName(ctx, imageManager, protocolScheme, "", buildFlags.Tag, buildArgs, false) if err != nil { return fmt.Errorf("failed to build container for %s: %v", protocolScheme, err) } From 5bc705bf4b72392f73a843bd6403bf07c36b68af Mon Sep 17 00:00:00 2001 From: Dan Barr <6922515+danbarr@users.noreply.github.com> Date: Mon, 17 Nov 2025 23:12:19 -0500 Subject: [PATCH 06/14] Add comprehensive test coverage for buildArgs parameter - Updated TestTemplateDataWithLocalPath to include BuildArgs field - Added TestCreateTemplateData with 5 test cases covering: * NPX with single buildArg * UVX with multiple buildArgs * GO with buildArgs * GO local path with buildArgs * NPX without buildArgs (backward compatibility) All tests verify that buildArgs flow correctly through the createTemplateData function and are properly set in TemplateData. --- pkg/runner/protocol_test.go | 111 ++++++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/pkg/runner/protocol_test.go b/pkg/runner/protocol_test.go index 7b8fc8b2d..51be5d336 100644 --- a/pkg/runner/protocol_test.go +++ b/pkg/runner/protocol_test.go @@ -189,6 +189,7 @@ func TestTemplateDataWithLocalPath(t *testing.T) { expected: templates.TemplateData{ MCPPackage: "github.com/example/package", IsLocalPath: false, + BuildArgs: nil, }, }, { @@ -197,6 +198,7 @@ func TestTemplateDataWithLocalPath(t *testing.T) { expected: templates.TemplateData{ MCPPackage: "./cmd/server", IsLocalPath: true, + BuildArgs: nil, }, }, { @@ -205,6 +207,7 @@ func TestTemplateDataWithLocalPath(t *testing.T) { expected: templates.TemplateData{ MCPPackage: ".", IsLocalPath: true, + BuildArgs: nil, }, }, } @@ -218,6 +221,7 @@ func TestTemplateDataWithLocalPath(t *testing.T) { templateData := templates.TemplateData{ MCPPackage: tt.packageName, IsLocalPath: isLocalPath, + BuildArgs: nil, } if templateData.MCPPackage != tt.expected.MCPPackage { @@ -229,3 +233,110 @@ func TestTemplateDataWithLocalPath(t *testing.T) { }) } } + +func TestCreateTemplateData(t *testing.T) { + t.Parallel() + tests := []struct { + name string + transportType templates.TransportType + packageName string + caCertPath string + buildArgs []string + expected templates.TemplateData + wantErr bool + }{ + { + name: "NPX with buildArgs", + transportType: templates.TransportTypeNPX, + packageName: "@launchdarkly/mcp-server", + caCertPath: "", + buildArgs: []string{"start"}, + expected: templates.TemplateData{ + MCPPackage: "@launchdarkly/mcp-server", + IsLocalPath: false, + BuildArgs: []string{"start"}, + }, + wantErr: false, + }, + { + name: "UVX with multiple buildArgs", + transportType: templates.TransportTypeUVX, + packageName: "example-package", + caCertPath: "", + buildArgs: []string{"--transport", "stdio"}, + expected: templates.TemplateData{ + MCPPackage: "example-package", + IsLocalPath: false, + BuildArgs: []string{"--transport", "stdio"}, + }, + wantErr: false, + }, + { + name: "GO with buildArgs", + transportType: templates.TransportTypeGO, + packageName: "github.com/example/package", + caCertPath: "", + buildArgs: []string{"serve", "--verbose"}, + expected: templates.TemplateData{ + MCPPackage: "github.com/example/package", + IsLocalPath: false, + BuildArgs: []string{"serve", "--verbose"}, + }, + wantErr: false, + }, + { + name: "GO local path with buildArgs", + transportType: templates.TransportTypeGO, + packageName: "./cmd/server", + caCertPath: "", + buildArgs: []string{"--config", "config.yaml"}, + expected: templates.TemplateData{ + MCPPackage: "./cmd/server", + IsLocalPath: true, + BuildArgs: []string{"--config", "config.yaml"}, + }, + wantErr: false, + }, + { + name: "NPX without buildArgs", + transportType: templates.TransportTypeNPX, + packageName: "package-name", + caCertPath: "", + buildArgs: nil, + expected: templates.TemplateData{ + MCPPackage: "package-name", + IsLocalPath: false, + BuildArgs: nil, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + result, err := createTemplateData(tt.transportType, tt.packageName, tt.caCertPath, tt.buildArgs) + + if (err != nil) != tt.wantErr { + t.Errorf("createTemplateData() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if result.MCPPackage != tt.expected.MCPPackage { + t.Errorf("MCPPackage = %q, want %q", result.MCPPackage, tt.expected.MCPPackage) + } + if result.IsLocalPath != tt.expected.IsLocalPath { + t.Errorf("IsLocalPath = %v, want %v", result.IsLocalPath, tt.expected.IsLocalPath) + } + if len(result.BuildArgs) != len(tt.expected.BuildArgs) { + t.Errorf("BuildArgs length = %d, want %d", len(result.BuildArgs), len(tt.expected.BuildArgs)) + } else { + for i, arg := range result.BuildArgs { + if arg != tt.expected.BuildArgs[i] { + t.Errorf("BuildArgs[%d] = %q, want %q", i, arg, tt.expected.BuildArgs[i]) + } + } + } + }) + } +} From b2738f959f9e5f6ced465d052c9ec9ddf9d17ca0 Mon Sep 17 00:00:00 2001 From: Dan Barr <6922515+danbarr@users.noreply.github.com> Date: Mon, 17 Nov 2025 23:19:00 -0500 Subject: [PATCH 07/14] Fix linter errors: break up long lines - Broke up long line in build.go (BuildFromProtocolSchemeWithName call) - Broke up long function signature in protocol.go (createTemplateData) Both lines now conform to 130 character limit. --- cmd/thv/app/build.go | 3 ++- pkg/runner/protocol.go | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/cmd/thv/app/build.go b/cmd/thv/app/build.go index 2091e4b00..330d83735 100644 --- a/cmd/thv/app/build.go +++ b/cmd/thv/app/build.go @@ -91,7 +91,8 @@ func buildCmdFunc(cmd *cobra.Command, args []string) error { // If dry-run or output is specified, just generate the Dockerfile if buildFlags.DryRun || buildFlags.Output != "" { - dockerfileContent, err := runner.BuildFromProtocolSchemeWithName(ctx, imageManager, protocolScheme, "", buildFlags.Tag, buildArgs, true) + dockerfileContent, err := runner.BuildFromProtocolSchemeWithName( + ctx, imageManager, protocolScheme, "", buildFlags.Tag, buildArgs, true) if err != nil { return fmt.Errorf("failed to generate Dockerfile for %s: %v", protocolScheme, err) } diff --git a/pkg/runner/protocol.go b/pkg/runner/protocol.go index af709628b..e51caf65c 100644 --- a/pkg/runner/protocol.go +++ b/pkg/runner/protocol.go @@ -87,7 +87,9 @@ func ParseProtocolScheme(serverOrImage string) (templates.TransportType, string, } // createTemplateData creates the template data with optional CA certificate and build arguments. -func createTemplateData(transportType templates.TransportType, packageName, caCertPath string, buildArgs []string) (templates.TemplateData, error) { +func createTemplateData( + transportType templates.TransportType, packageName, caCertPath string, buildArgs []string, +) (templates.TemplateData, error) { // Check if this is a local path (for Go packages only) isLocalPath := transportType == templates.TransportTypeGO && isLocalGoPath(packageName) From 6b6fa9632a5ff23e0c7078df9f1dd99b85664e6e Mon Sep 17 00:00:00 2001 From: Dan Barr <6922515+danbarr@users.noreply.github.com> Date: Mon, 17 Nov 2025 23:29:30 -0500 Subject: [PATCH 08/14] Update docs Signed-off-by: Dan Barr <6922515+danbarr@users.noreply.github.com> --- docs/cli/thv_build.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/docs/cli/thv_build.md b/docs/cli/thv_build.md index 5316e77ee..23dfd87bb 100644 --- a/docs/cli/thv_build.md +++ b/docs/cli/thv_build.md @@ -29,6 +29,14 @@ using either uvx (Python with uv package manager), npx (Node.js), or go (Golang). For Go, you can also specify local paths starting with './' or '../' to build local Go projects. +Build-time arguments can be baked into the container's ENTRYPOINT: + + $ thv build npx://@launchdarkly/mcp-server -- start + $ thv build uvx://package -- --transport stdio + +These arguments become part of the container image and will always run, +with runtime arguments (from 'thv run -- ') appending after them. + The container will be built and tagged locally, ready to be used with 'thv run' or other container tools. The built image name will be displayed upon successful completion. @@ -36,9 +44,10 @@ Examples: $ thv build uvx://mcp-server-git $ thv build --tag my-custom-name:latest npx://@modelcontextprotocol/server-filesystem $ thv build go://./my-local-server + $ thv build npx://@launchdarkly/mcp-server -- start ``` -thv build [flags] PROTOCOL +thv build [flags] PROTOCOL [-- ARGS...] ``` ### Options From 3eadcb3229fb9777e5cb2462a1d4f28e363800f8 Mon Sep 17 00:00:00 2001 From: Dan Barr <6922515+danbarr@users.noreply.github.com> Date: Mon, 17 Nov 2025 23:39:59 -0500 Subject: [PATCH 09/14] Add test coverage for BuildFromProtocolSchemeWithName with buildArgs --- pkg/runner/protocol_test.go | 69 +++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/pkg/runner/protocol_test.go b/pkg/runner/protocol_test.go index 51be5d336..3c44426e0 100644 --- a/pkg/runner/protocol_test.go +++ b/pkg/runner/protocol_test.go @@ -1,6 +1,8 @@ package runner import ( + "context" + "strings" "testing" "github.com/stacklok/toolhive/pkg/container/templates" @@ -234,6 +236,73 @@ func TestTemplateDataWithLocalPath(t *testing.T) { } } +func TestBuildFromProtocolSchemeWithNameDryRun(t *testing.T) { + t.Parallel() + tests := []struct { + name string + serverOrImage string + buildArgs []string + wantContains []string + wantErr bool + }{ + { + name: "NPX with buildArgs in dry-run", + serverOrImage: "npx://@launchdarkly/mcp-server", + buildArgs: []string{"start"}, + wantContains: []string{ + "exec npx @launchdarkly/mcp-server start", + "FROM node:22-alpine", + }, + wantErr: false, + }, + { + name: "UVX with multiple buildArgs in dry-run", + serverOrImage: "uvx://example-package", + buildArgs: []string{"--transport", "stdio"}, + wantContains: []string{ + "example-package", + "--transport", + "stdio", + "FROM python:3.13-slim", + }, + wantErr: false, + }, + { + name: "GO with buildArgs in dry-run", + serverOrImage: "go://github.com/example/package", + buildArgs: []string{"serve"}, + wantContains: []string{ + `ENTRYPOINT ["/app/mcp-server", "serve"]`, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + ctx := context.Background() + + // Call BuildFromProtocolSchemeWithName with dry-run=true + dockerfileContent, err := BuildFromProtocolSchemeWithName( + ctx, nil, tt.serverOrImage, "", "", tt.buildArgs, true) + + if (err != nil) != tt.wantErr { + t.Errorf("BuildFromProtocolSchemeWithName() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if err == nil { + for _, want := range tt.wantContains { + if !strings.Contains(dockerfileContent, want) { + t.Errorf("Dockerfile does not contain expected string %q", want) + } + } + } + }) + } +} + func TestCreateTemplateData(t *testing.T) { t.Parallel() tests := []struct { From 4f8181bf0088064dba22f32868a38144ed4f9649 Mon Sep 17 00:00:00 2001 From: Dan Barr <6922515+danbarr@users.noreply.github.com> Date: Mon, 17 Nov 2025 23:46:05 -0500 Subject: [PATCH 10/14] Fix linting issues in protocol_test.go --- pkg/runner/protocol_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/runner/protocol_test.go b/pkg/runner/protocol_test.go index 3c44426e0..2bac9e828 100644 --- a/pkg/runner/protocol_test.go +++ b/pkg/runner/protocol_test.go @@ -239,11 +239,11 @@ func TestTemplateDataWithLocalPath(t *testing.T) { func TestBuildFromProtocolSchemeWithNameDryRun(t *testing.T) { t.Parallel() tests := []struct { - name string - serverOrImage string - buildArgs []string - wantContains []string - wantErr bool + name string + serverOrImage string + buildArgs []string + wantContains []string + wantErr bool }{ { name: "NPX with buildArgs in dry-run", From f4f7c6cdffc7a03e3be67fac189c2f1dea94c61e Mon Sep 17 00:00:00 2001 From: Dan Barr <6922515+danbarr@users.noreply.github.com> Date: Mon, 17 Nov 2025 23:50:52 -0500 Subject: [PATCH 11/14] Add test for error path in BuildFromProtocolSchemeWithName --- pkg/runner/protocol_test.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pkg/runner/protocol_test.go b/pkg/runner/protocol_test.go index 2bac9e828..e1e2bddeb 100644 --- a/pkg/runner/protocol_test.go +++ b/pkg/runner/protocol_test.go @@ -241,6 +241,7 @@ func TestBuildFromProtocolSchemeWithNameDryRun(t *testing.T) { tests := []struct { name string serverOrImage string + caCertPath string buildArgs []string wantContains []string wantErr bool @@ -276,6 +277,13 @@ func TestBuildFromProtocolSchemeWithNameDryRun(t *testing.T) { }, wantErr: false, }, + { + name: "NPX with buildArgs and invalid CA cert path", + serverOrImage: "npx://@launchdarkly/mcp-server", + caCertPath: "/nonexistent/ca-cert.crt", + buildArgs: []string{"start"}, + wantErr: true, + }, } for _, tt := range tests { @@ -285,7 +293,7 @@ func TestBuildFromProtocolSchemeWithNameDryRun(t *testing.T) { // Call BuildFromProtocolSchemeWithName with dry-run=true dockerfileContent, err := BuildFromProtocolSchemeWithName( - ctx, nil, tt.serverOrImage, "", "", tt.buildArgs, true) + ctx, nil, tt.serverOrImage, tt.caCertPath, "", tt.buildArgs, true) if (err != nil) != tt.wantErr { t.Errorf("BuildFromProtocolSchemeWithName() error = %v, wantErr %v", err, tt.wantErr) From 9d190ec650a1c0b48e1ca4d5fd4f042ebb5efa2e Mon Sep 17 00:00:00 2001 From: Dan Barr <6922515+danbarr@users.noreply.github.com> Date: Thu, 20 Nov 2025 12:18:38 -0500 Subject: [PATCH 12/14] 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. --- pkg/container/templates/npx.tmpl | 11 +--- pkg/container/templates/templates.go | 22 +++++++ pkg/container/templates/templates_test.go | 70 ++++++++++++++++++++--- pkg/container/templates/uvx.tmpl | 7 +-- pkg/runner/protocol_test.go | 6 +- 5 files changed, 94 insertions(+), 22 deletions(-) diff --git a/pkg/container/templates/npx.tmpl b/pkg/container/templates/npx.tmpl index dbff031e0..b7377baa8 100644 --- a/pkg/container/templates/npx.tmpl +++ b/pkg/container/templates/npx.tmpl @@ -95,11 +95,6 @@ ENV NODE_PATH=/app/node_modules \ # Switch to non-root user USER appuser -# `MCPPackage` may include a version suffix (e.g., `package@1.2.3`), which we cannot use here. -# Create a small wrapper script to handle this. -RUN echo "#!/bin/sh" >> entrypoint.sh && \ - echo "exec npx {{.MCPPackage}}{{range .BuildArgs}} {{.}}{{end}} \"\$@\"" >> entrypoint.sh && \ - chmod +x entrypoint.sh - -# Run the preinstalled MCP package directly using npx. -ENTRYPOINT ["./entrypoint.sh"] +# Run the preinstalled MCP package directly using npx +# MCPPackageClean has version suffix already stripped (e.g., @org/package@1.2.3 -> @org/package) +ENTRYPOINT ["npx", "{{.MCPPackageClean}}"{{range .BuildArgs}}, "{{.}}"{{end}}] diff --git a/pkg/container/templates/templates.go b/pkg/container/templates/templates.go index 6135c7a10..d9223042c 100644 --- a/pkg/container/templates/templates.go +++ b/pkg/container/templates/templates.go @@ -6,6 +6,7 @@ import ( "bytes" "embed" "fmt" + "regexp" "text/template" ) @@ -16,6 +17,10 @@ var templateFS embed.FS type TemplateData struct { // MCPPackage is the name of the MCP package to run. MCPPackage string + // MCPPackageClean is the package name with version suffix removed. + // For example: "@org/package@1.2.3" becomes "@org/package", "package@1.0.0" becomes "package" + // This field is automatically populated by GetDockerfileTemplate. + MCPPackageClean string // CACertContent is the content of the custom CA certificate to include in the image. CACertContent string // IsLocalPath indicates if the MCPPackage is a local path that should be copied into the container. @@ -38,8 +43,25 @@ const ( TransportTypeGO TransportType = "go" ) +// stripVersionSuffix removes version suffixes from package names. +// It strips @version from the end of package names while preserving scoped package prefixes. +// Examples: +// - "@org/package@1.2.3" -> "@org/package" +// - "package@1.0.0" -> "package" +// - "@org/package" -> "@org/package" (no version, unchanged) +// - "package" -> "package" (no version, unchanged) +func stripVersionSuffix(pkg string) string { + // Match @version at the end, where version doesn't contain @ or / + // This preserves scoped packages like @org/package + re := regexp.MustCompile(`@[^@/]*$`) + return re.ReplaceAllString(pkg, "") +} + // GetDockerfileTemplate returns the Dockerfile template for the specified transport type. func GetDockerfileTemplate(transportType TransportType, data TemplateData) (string, error) { + // Populate MCPPackageClean with version-stripped package name + data.MCPPackageClean = stripVersionSuffix(data.MCPPackage) + var templateName string // Determine the template name based on the transport type diff --git a/pkg/container/templates/templates_test.go b/pkg/container/templates/templates_test.go index 0474b4678..f49836444 100644 --- a/pkg/container/templates/templates_test.go +++ b/pkg/container/templates/templates_test.go @@ -30,7 +30,7 @@ func TestGetDockerfileTemplate(t *testing.T) { "package_spec=$(echo \"$package\" | sed 's/@/==/')", "uv tool install \"$package_spec\"", "COPY --from=builder --chown=appuser:appgroup /opt/uv-tools /opt/uv-tools", - "ENTRYPOINT [\"sh\", \"-c\", \"package='example-package'; exec \\\"${package%%@*}\\\"\", \"--\"]", + "ENTRYPOINT [\"sh\", \"-c\", \"exec 'example-package' \\\"$@\\\"\", \"--\"]", }, wantMatches: []string{ `FROM python:\d+\.\d+-slim AS builder`, // Match builder stage @@ -56,7 +56,7 @@ func TestGetDockerfileTemplate(t *testing.T) { "package_spec=$(echo \"$package\" | sed 's/@/==/')", "uv tool install \"$package_spec\"", "COPY --from=builder --chown=appuser:appgroup /opt/uv-tools /opt/uv-tools", - "ENTRYPOINT [\"sh\", \"-c\", \"package='example-package'; exec \\\"${package%%@*}\\\"\", \"--\"]", + "ENTRYPOINT [\"sh\", \"-c\", \"exec 'example-package' \\\"$@\\\"\", \"--\"]", "Add custom CA certificate BEFORE any network operations", "COPY ca-cert.crt /tmp/custom-ca.crt", "cat /tmp/custom-ca.crt >> /etc/ssl/certs/ca-certificates.crt", @@ -79,8 +79,7 @@ func TestGetDockerfileTemplate(t *testing.T) { "FROM node:", "npm install --save example-package", "COPY --from=builder --chown=appuser:appgroup /build/node_modules /app/node_modules", - "echo \"exec npx example-package \\\"\\$@\\\"\" >> entrypoint.sh", - "ENTRYPOINT [\"./entrypoint.sh\"]", + `ENTRYPOINT ["npx", "example-package"]`, }, wantMatches: []string{ `FROM node:\d+-alpine AS builder`, // Match builder stage @@ -102,8 +101,7 @@ func TestGetDockerfileTemplate(t *testing.T) { wantContains: []string{ "FROM node:", "npm install --save example-package", - "echo \"exec npx example-package \\\"\\$@\\\"\" >> entrypoint.sh", - "ENTRYPOINT [\"./entrypoint.sh\"]", + `ENTRYPOINT ["npx", "example-package"]`, "Add custom CA certificate BEFORE any network operations", "COPY ca-cert.crt /tmp/custom-ca.crt", "cat /tmp/custom-ca.crt >> /etc/ssl/certs/ca-certificates.crt", @@ -227,8 +225,7 @@ func TestGetDockerfileTemplate(t *testing.T) { "FROM node:", "npm install --save @launchdarkly/mcp-server", "COPY --from=builder --chown=appuser:appgroup /build/node_modules /app/node_modules", - "echo \"exec npx @launchdarkly/mcp-server start \\\"\\$@\\\"\" >> entrypoint.sh", - "ENTRYPOINT [\"./entrypoint.sh\"]", + `ENTRYPOINT ["npx", "@launchdarkly/mcp-server", "start"]`, }, wantMatches: []string{ `FROM node:\d+-alpine AS builder`, @@ -247,7 +244,7 @@ func TestGetDockerfileTemplate(t *testing.T) { wantContains: []string{ "FROM python:", "uv tool install \"$package_spec\"", - "ENTRYPOINT [\"sh\", \"-c\", \"package='example-package'; exec \\\"${package%%@*}\\\" \\\"--transport\\\" \\\"stdio\\\"\", \"--\"]", + "ENTRYPOINT [\"sh\", \"-c\", \"exec 'example-package' '--transport' 'stdio' \\\"$@\\\"\", \"--\"]", }, wantMatches: []string{ `FROM python:\d+\.\d+-slim AS builder`, @@ -378,3 +375,58 @@ func TestParseTransportType(t *testing.T) { }) } } + +func TestStripVersionSuffix(t *testing.T) { + t.Parallel() + tests := []struct { + name string + input string + want string + }{ + { + name: "scoped package with version", + input: "@launchdarkly/mcp-server@1.2.3", + want: "@launchdarkly/mcp-server", + }, + { + name: "regular package with version", + input: "example-package@1.0.0", + want: "example-package", + }, + { + name: "scoped package without version", + input: "@org/package", + want: "@org/package", + }, + { + name: "regular package without version", + input: "package", + want: "package", + }, + { + name: "package with latest tag", + input: "package@latest", + want: "package", + }, + { + name: "scoped package with semver", + input: "@scope/name@^1.2.3", + want: "@scope/name", + }, + { + name: "package with prerelease version", + input: "package@1.0.0-beta.1", + want: "package", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := stripVersionSuffix(tt.input) + if got != tt.want { + t.Errorf("stripVersionSuffix(%q) = %q, want %q", tt.input, got, tt.want) + } + }) + } +} diff --git a/pkg/container/templates/uvx.tmpl b/pkg/container/templates/uvx.tmpl index e797e283b..78af10970 100644 --- a/pkg/container/templates/uvx.tmpl +++ b/pkg/container/templates/uvx.tmpl @@ -115,7 +115,6 @@ USER appuser # Run the pre-installed MCP package # uv tool install puts the correct executable in the bin directory -# We use sh -c to allow the package name to be resolved from PATH -# Strip version specifier (if present) from package name for execution -# Handles format like package@version -ENTRYPOINT ["sh", "-c", "package='{{.MCPPackage}}'; exec \"${package%%@*}\"{{range .BuildArgs}} \"{{.}}\"{{end}}", "--"] \ No newline at end of file +# MCPPackageClean has version suffix already stripped (e.g., package@1.2.3 -> package) +# BuildArgs use single quotes for safety - prevents shell injection +ENTRYPOINT ["sh", "-c", "exec '{{.MCPPackageClean}}'{{range .BuildArgs}} '{{.}}'{{end}} \"$@\"", "--"] \ No newline at end of file diff --git a/pkg/runner/protocol_test.go b/pkg/runner/protocol_test.go index e1e2bddeb..67da49654 100644 --- a/pkg/runner/protocol_test.go +++ b/pkg/runner/protocol_test.go @@ -251,7 +251,11 @@ func TestBuildFromProtocolSchemeWithNameDryRun(t *testing.T) { serverOrImage: "npx://@launchdarkly/mcp-server", buildArgs: []string{"start"}, wantContains: []string{ - "exec npx @launchdarkly/mcp-server start", + `echo '#!/bin/sh' > entrypoint.sh`, + `echo 'package=$(echo "@launchdarkly/mcp-server" | sed '"'"'s/@[^@/]*$//'"'"')'`, + `echo -n 'exec npx "$package"' >> entrypoint.sh`, + `printf ' %s' 'start' >> entrypoint.sh`, + `echo ' "$@"' >> entrypoint.sh`, "FROM node:22-alpine", }, wantErr: false, From 22491078d5b097e9d6d4c7a55ccd8ae6f8afe66b Mon Sep 17 00:00:00 2001 From: Dan Barr <6922515+danbarr@users.noreply.github.com> Date: Thu, 20 Nov 2025 13:41:38 -0500 Subject: [PATCH 13/14] Fix protocol test --- pkg/runner/protocol_test.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/pkg/runner/protocol_test.go b/pkg/runner/protocol_test.go index 67da49654..70d672712 100644 --- a/pkg/runner/protocol_test.go +++ b/pkg/runner/protocol_test.go @@ -251,11 +251,7 @@ func TestBuildFromProtocolSchemeWithNameDryRun(t *testing.T) { serverOrImage: "npx://@launchdarkly/mcp-server", buildArgs: []string{"start"}, wantContains: []string{ - `echo '#!/bin/sh' > entrypoint.sh`, - `echo 'package=$(echo "@launchdarkly/mcp-server" | sed '"'"'s/@[^@/]*$//'"'"')'`, - `echo -n 'exec npx "$package"' >> entrypoint.sh`, - `printf ' %s' 'start' >> entrypoint.sh`, - `echo ' "$@"' >> entrypoint.sh`, + `ENTRYPOINT ["npx", "@launchdarkly/mcp-server", "start"]`, "FROM node:22-alpine", }, wantErr: false, From df3cb0db41c67f70efbb1319e6a64d05477d7b30 Mon Sep 17 00:00:00 2001 From: Dan Barr <6922515+danbarr@users.noreply.github.com> Date: Thu, 20 Nov 2025 14:03:57 -0500 Subject: [PATCH 14/14] Validate buildArgs to prevent single quote injection --- pkg/runner/protocol.go | 18 ++++++++++++++++++ pkg/runner/protocol_test.go | 22 ++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/pkg/runner/protocol.go b/pkg/runner/protocol.go index e51caf65c..1c62a2ded 100644 --- a/pkg/runner/protocol.go +++ b/pkg/runner/protocol.go @@ -86,10 +86,28 @@ func ParseProtocolScheme(serverOrImage string) (templates.TransportType, string, return "", "", fmt.Errorf("unsupported protocol scheme: %s", serverOrImage) } +// validateBuildArgs ensures buildArgs don't contain single quotes which would break +// shell quoting in the UVX template. Single quotes cannot be escaped within single-quoted +// strings in shell, making them the only character that can enable command injection. +// NPX and GO use JSON array ENTRYPOINTs without shell interpretation, so they're safe. +func validateBuildArgs(buildArgs []string) error { + for _, arg := range buildArgs { + if strings.Contains(arg, "'") { + return fmt.Errorf("buildArg cannot contain single quotes: %s", arg) + } + } + return nil +} + // createTemplateData creates the template data with optional CA certificate and build arguments. func createTemplateData( transportType templates.TransportType, packageName, caCertPath string, buildArgs []string, ) (templates.TemplateData, error) { + // Validate buildArgs to prevent shell injection in templates that use sh -c + if err := validateBuildArgs(buildArgs); err != nil { + return templates.TemplateData{}, err + } + // Check if this is a local path (for Go packages only) isLocalPath := transportType == templates.TransportTypeGO && isLocalGoPath(packageName) diff --git a/pkg/runner/protocol_test.go b/pkg/runner/protocol_test.go index 70d672712..d44e88296 100644 --- a/pkg/runner/protocol_test.go +++ b/pkg/runner/protocol_test.go @@ -387,6 +387,28 @@ func TestCreateTemplateData(t *testing.T) { }, wantErr: false, }, + { + name: "buildArgs with single quote should fail", + transportType: templates.TransportTypeUVX, + packageName: "example-package", + caCertPath: "", + buildArgs: []string{"--name", "test'arg"}, + expected: templates.TemplateData{}, + wantErr: true, + }, + { + name: "buildArgs with other special characters should succeed", + transportType: templates.TransportTypeNPX, + packageName: "example-package", + caCertPath: "", + buildArgs: []string{"--config", "file$with`special\"chars"}, + expected: templates.TemplateData{ + MCPPackage: "example-package", + IsLocalPath: false, + BuildArgs: []string{"--config", "file$with`special\"chars"}, + }, + wantErr: false, + }, } for _, tt := range tests {