fix(build): re-include local bin overrides in the generated .dockerignore

forge-cli/build/dockerfile_stage.go

@@ -7,6 +7,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"sort"
 	"text/template"
 
 	"github.com/initializ/forge/forge-cli/templates"
@@ -227,11 +228,10 @@ func (s *DockerfileStage) injectLocalBins(bc *pipeline.BuildContext) {
 	}
 }
 
-// copyLocalBins copies local binary files into .local-bins/ in the build output directory.
-// It collects binaries from both forge.yaml config (BinOverrides with LocalPath) and
-// CLI flags (bc.LocalBins).
-func (s *DockerfileStage) copyLocalBins(bc *pipeline.BuildContext) error {
-	// Collect local bins from config
+// localBins returns the union of local binary overrides from forge.yaml
+// config (BinOverrides with LocalPath) and CLI flags (bc.LocalBins), which
+// may have additional entries not yet in config.
+func localBins(bc *pipeline.BuildContext) map[string]string {
 	bins := make(map[string]string)
 	if bc.Config != nil {
 		for name, override := range bc.Config.Package.BinOverrides {
@@ -240,11 +240,15 @@ func (s *DockerfileStage) copyLocalBins(bc *pipeline.BuildContext) error {
 			}
 		}
 	}
-	// CLI flags (bc.LocalBins) may have additional entries not yet in config
 	for name, path := range bc.LocalBins {
 		bins[name] = path
 	}
+	return bins
+}
 
+// copyLocalBins copies local binary files into .local-bins/ in the build output directory.
+func (s *DockerfileStage) copyLocalBins(bc *pipeline.BuildContext) error {
+	bins := localBins(bc)
 	if len(bins) == 0 {
 		return nil
 	}
@@ -338,10 +342,25 @@ compiled/
 # Recursive — the Dockerfile shouldn't be baked into its own image.
 Dockerfile
 .dockerignore
-# Build-only local binary stash; binaries land in /usr/local/bin via
-# the bin stage already.
+# Build-only local binary stash. Local bin overrides are re-included
+# below (last match wins) — the generated Dockerfile COPYs each one
+# from .local-bins/ directly, so excluding them breaks the build.
 .local-bins/
 `
+	// Re-include local bin overrides: for each one the generated Dockerfile
+	// carries a `COPY .local-bins/<name> /usr/local/bin/<name>` that reads
+	// straight from the build context, which the blanket .local-bins/
+	// exclusion above would otherwise break.
+	if bins := localBins(bc); len(bins) > 0 {
+		names := make([]string, 0, len(bins))
+		for name := range bins {
+			names = append(names, name)
+		}
+		sort.Strings(names)
+		for _, name := range names {
+			dockerignoreContent += fmt.Sprintf("!.local-bins/%s\n", name)
+		}
+	}
 	ignorePath := filepath.Join(bc.Opts.OutputDir, ".dockerignore")
 	if err := os.WriteFile(ignorePath, []byte(dockerignoreContent), 0644); err != nil {
 		return fmt.Errorf("writing .dockerignore: %w", err)

forge-cli/build/dockerfile_stage_issue147_test.go

@@ -156,3 +156,77 @@ func TestDockerfile_NoLongerCopiesDeadCompiledArtifacts(t *testing.T) {
 		}
 	}
 }
+
+// TestDockerignore_ReincludesLocalBinOverrides pins the --local-bin fix:
+// the generated Dockerfile COPYs each local bin override straight from
+// .local-bins/ in the build context, so the blanket .local-bins/ exclusion
+// must be followed by a !.local-bins/<name> negation per override
+// (last match wins) or the docker build fails with "not found".
+func TestDockerignore_ReincludesLocalBinOverrides(t *testing.T) {
+	tmpDir := t.TempDir()
+	outDir := filepath.Join(tmpDir, "output")
+	if err := os.MkdirAll(outDir, 0755); err != nil {
+		t.Fatal(err)
+	}
+
+	bc := pipeline.NewBuildContext(pipeline.PipelineOptions{WorkDir: tmpDir, OutputDir: outDir})
+	bc.Config = &types.ForgeConfig{AgentID: "test", Version: "1.0.0"}
+	bc.Config.Package.BinOverrides = map[string]types.BinOverride{
+		"jq": {LocalPath: "/usr/bin/jq"},
+	}
+	bc.LocalBins = map[string]string{"forge": "/usr/local/bin/forge"}
+	bc.Spec = &agentspec.AgentSpec{AgentID: "test"}
+
+	s := &DockerfileStage{}
+	if err := s.writeDockerignore(bc); err != nil {
+		t.Fatalf("writeDockerignore: %v", err)
+	}
+
+	data, err := os.ReadFile(filepath.Join(outDir, ".dockerignore"))
+	if err != nil {
+		t.Fatalf("reading .dockerignore: %v", err)
+	}
+	got := string(data)
+
+	// The blanket exclusion stays (keeps unrelated stash content out)…
+	if !strings.Contains(got, ".local-bins/\n") {
+		t.Errorf(".dockerignore lost the .local-bins/ exclusion. Full content:\n%s", got)
+	}
+	// …and every override is re-included after it, config and CLI alike.
+	for _, name := range []string{"forge", "jq"} {
+		neg := "!.local-bins/" + name + "\n"
+		if !strings.Contains(got, neg) {
+			t.Errorf(".dockerignore is missing negation %q. Full content:\n%s", neg, got)
+		}
+		if strings.Index(got, neg) < strings.Index(got, ".local-bins/\n") {
+			t.Errorf("negation %q must come after the .local-bins/ exclusion. Full content:\n%s", neg, got)
+		}
+	}
+}
+
+// TestDockerignore_NoLocalBins_KeepsPlainExclusion guards the default path:
+// without overrides no negations are emitted.
+func TestDockerignore_NoLocalBins_KeepsPlainExclusion(t *testing.T) {
+	tmpDir := t.TempDir()
+	outDir := filepath.Join(tmpDir, "output")
+	if err := os.MkdirAll(outDir, 0755); err != nil {
+		t.Fatal(err)
+	}
+
+	bc := pipeline.NewBuildContext(pipeline.PipelineOptions{WorkDir: tmpDir, OutputDir: outDir})
+	bc.Config = &types.ForgeConfig{AgentID: "test", Version: "1.0.0"}
+	bc.Spec = &agentspec.AgentSpec{AgentID: "test"}
+
+	s := &DockerfileStage{}
+	if err := s.writeDockerignore(bc); err != nil {
+		t.Fatalf("writeDockerignore: %v", err)
+	}
+
+	data, err := os.ReadFile(filepath.Join(outDir, ".dockerignore"))
+	if err != nil {
+		t.Fatalf("reading .dockerignore: %v", err)
+	}
+	if strings.Contains(string(data), "!.local-bins/") {
+		t.Errorf(".dockerignore must not contain negations without local bin overrides. Full content:\n%s", string(data))
+	}
+}

forge-cli/cmd/build.go

@@ -153,6 +153,9 @@ func parseLocalBins(args []string) (map[string]string, error) {
 			return nil, fmt.Errorf("invalid --local-bin format %q: expected name=/path/to/file", arg)
 		}
 		name, path := parts[0], parts[1]
+		if !validate.ValidBinOverrideName(name) {
+			return nil, fmt.Errorf("invalid --local-bin name %q: must be a plain binary basename matching ^[a-zA-Z0-9_.-]{1,64}$ (and not \".\" or \"..\")", name)
+		}
 		absPath, err := filepath.Abs(path)
 		if err != nil {
 			return nil, fmt.Errorf("resolving path for --local-bin %s: %w", name, err)

forge-cli/cmd/build_test.go

@@ -107,3 +107,24 @@ entrypoint: ""
 		t.Fatal("expected error for invalid config")
 	}
 }
+
+// TestParseLocalBins_NameValidation pins the --local-bin name check
+// (PR #374 follow-up): the name flows verbatim into the .local-bins/
+// path, the Dockerfile COPY, and the .dockerignore negation.
+func TestParseLocalBins_NameValidation(t *testing.T) {
+	tmp := t.TempDir()
+	binPath := filepath.Join(tmp, "somebin")
+	if err := os.WriteFile(binPath, []byte("#!/bin/sh\n"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err := parseLocalBins([]string{"forge=" + binPath}); err != nil {
+		t.Errorf("valid name rejected: %v", err)
+	}
+
+	for _, name := range []string{"../escape", "..", ".", "a/b", "bad\nname", "*", "!x"} {
+		if _, err := parseLocalBins([]string{name + "=" + binPath}); err == nil {
+			t.Errorf("name %q should be rejected", name)
+		}
+	}
+}

forge-core/validate/forge_config.go

@@ -3,6 +3,7 @@ package validate
 import (
 	"fmt"
 	"regexp"
+	"sort"
 	"strings"
 
 	"github.com/initializ/forge/forge-core/llm"
@@ -35,6 +36,22 @@ func isGatewayScheme(s string) bool {
 	return s == llm.AuthSchemeAPIKeyHeader || s == llm.AuthSchemeAPIKeyHeaderOnly
 }
 
+// binOverrideNamePattern is the accepted charset for a bin-override name
+// (forge.yaml bin_overrides.<name> / --local-bin name=…). The name is a
+// binary basename that reaches three sinks verbatim — the .local-bins/
+// filesystem path, the Dockerfile COPY line, and the .dockerignore
+// re-include negation — so anything beyond a plain basename (path
+// separators, whitespace, glob metacharacters) is an injection vector.
+var binOverrideNamePattern = regexp.MustCompile(`^[a-zA-Z0-9_.-]{1,64}$`)
+
+// ValidBinOverrideName reports whether name is a safe bin-override name.
+// Pure-dot names are rejected explicitly: "." and ".." pass the charset
+// pattern but are path navigation, not basenames — ".." is exactly the
+// traversal the pattern exists to stop.
+func ValidBinOverrideName(name string) bool {
+	return binOverrideNamePattern.MatchString(name) && strings.Trim(name, ".") != ""
+}
+
 var (
 	agentIDPattern = regexp.MustCompile(`^[a-z0-9-]+$`)
 	semverPattern  = regexp.MustCompile(`^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$`)
@@ -137,6 +154,20 @@ func ValidateForgeConfig(cfg *types.ForgeConfig) *ValidationResult {
 		r.Warnings = append(r.Warnings, "egress mode 'dev-open' is not recommended for production")
 	}
 
+	// Validate bin-override names (package.bin_overrides). The name flows
+	// verbatim into the .local-bins/ path, the Dockerfile COPY, and the
+	// .dockerignore negation — reject anything that isn't a plain basename.
+	binNames := make([]string, 0, len(cfg.Package.BinOverrides))
+	for name := range cfg.Package.BinOverrides {
+		binNames = append(binNames, name)
+	}
+	sort.Strings(binNames)
+	for _, name := range binNames {
+		if !ValidBinOverrideName(name) {
+			r.Errors = append(r.Errors, fmt.Sprintf("package.bin_overrides name %q must be a plain binary basename matching ^[a-zA-Z0-9_.-]{1,64}$ (and not %q or %q)", name, ".", ".."))
+		}
+	}
+
 	// Validate secrets config
 	for _, p := range cfg.Secrets.Providers {
 		if !knownSecretProviders[p] {

forge-core/validate/forge_config_test.go

@@ -225,3 +225,37 @@ func TestValidateForgeConfig_OrgIDOnOpenAI(t *testing.T) {
 		}
 	}
 }
+
+// TestValidateForgeConfig_BinOverrideNames pins the bin-override name
+// validation (PR #374 follow-up): the name flows verbatim into the
+// .local-bins/ filesystem path, the Dockerfile COPY line, and the
+// .dockerignore negation, so only plain basenames may pass.
+func TestValidateForgeConfig_BinOverrideNames(t *testing.T) {
+	valid := []string{"forge", "jq", "kubectl", "my-tool_2", "tool.v2", "a"}
+	for _, name := range valid {
+		cfg := validConfig()
+		cfg.Package.BinOverrides = map[string]types.BinOverride{name: {LocalPath: "/x"}}
+		if r := ValidateForgeConfig(cfg); hasSubstr(r.Errors, "bin_overrides") {
+			t.Errorf("name %q should be accepted, got errors: %v", name, r.Errors)
+		}
+	}
+
+	invalid := []string{
+		"../escape",             // path traversal into the fs sink
+		"..",                    // pure-dot traversal — passes the charset alone
+		".",                     // pure-dot
+		"a/b",                   // path separator
+		"bad\nname",             // newline → .dockerignore line injection
+		"!important",            // dockerignore negation metacharacter
+		"*",                     // glob re-includes the whole stash
+		"name with space",       // whitespace
+		strings.Repeat("a", 65), // over length cap
+	}
+	for _, name := range invalid {
+		cfg := validConfig()
+		cfg.Package.BinOverrides = map[string]types.BinOverride{name: {LocalPath: "/x"}}
+		if r := ValidateForgeConfig(cfg); !hasSubstr(r.Errors, "bin_overrides") {
+			t.Errorf("name %q should be rejected, got errors: %v", name, r.Errors)
+		}
+	}
+}