From 66ea1fe7005bcc3a01819116967ec7788544efbe Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Thu, 12 Nov 2026 17:42:17 -0300 Subject: [PATCH] refactor(cli): take --ca-bundle on build and up only run-user-commands took a bundle path, checked it held PEM, and then used it only as a switch: the variables it set point at the copy up bound into the container, never at the file given. up --ca-bundle already puts those variables in the container's environment, which the lifecycle commands inherit through the user-env probe, so the flag added nothing. The flag has not been released, so nothing depends on it. run-user-commands now rejects --ca-bundle as an unknown flag, and the e2e test checks that its postAttachCommand trusts the proxy's CA with no flag at all. Co-Authored-By: Claude Opus 5.5 --- docs/DIVERGENCES.md | 2 +- docs/go-only-features.md | 4 ++-- docs/parity/cli-flags-inventory.yaml | 4 ---- internal/cli/ca_bundle.go | 15 -------------- internal/cli/ca_bundle_test.go | 30 ++++++++++++---------------- internal/cli/e2e_ca_bundle_test.go | 22 ++++++++++---------- internal/cli/run_user_commands.go | 15 +++----------- 7 files changed, 29 insertions(+), 63 deletions(-) diff --git a/docs/DIVERGENCES.md b/docs/DIVERGENCES.md index d512757..e7cbeb9 100644 --- a/docs/DIVERGENCES.md +++ b/docs/DIVERGENCES.md @@ -16,7 +16,7 @@ touches a compared surface, reflected in the parity matrix. prebuilt image, skip build + feature install), `read-configuration --cache-key` (deterministic content hash; additive — default output is byte-identical to TS), `build --secrets-file` (BuildKit build secrets; TS `build` has no such flag), - `--ca-bundle` on `build`, `up` and `run-user-commands` (extra CAs trusted in Feature + `--ca-bundle` on `build` and `up` (extra CAs trusted in Feature installs, opted-in Dockerfile `RUN`s and the container, mounted and never copied), `open` (launch VS Code attached to the workspace's dev container via a vscode-remote:// URI), `stop`/`down` (pause, or stop+remove, a workspace's container — Compose-aware; the diff --git a/docs/go-only-features.md b/docs/go-only-features.md index d22ddc5..17af27a 100644 --- a/docs/go-only-features.md +++ b/docs/go-only-features.md @@ -213,14 +213,14 @@ devcontainer up --workspace-folder . --ca-bundle /run/proxy/ca-bundle.crt The file is a full PEM bundle: the system CAs plus the extra ones, since some tools replace the system pool with the file instead of adding to it. A file with no PEM -certificate is refused. `build`, `up` and `run-user-commands` take the flag. +certificate is refused. `build` and `up` take the flag. | Where | What happens | | --- | --- | | Feature install (`build`, `up`) | Every Feature's install `RUN` mounts the bundle as the BuildKit secret `devcontainer-ca-bundle` at `/run/devcontainer/ca-bundle.crt`, and runs `install.sh` with `SSL_CERT_FILE`, `NODE_EXTRA_CA_CERTS`, `REQUESTS_CA_BUNDLE`, `CURL_CA_BUNDLE` and `GIT_SSL_CAINFO` pointing at it, for that `RUN` only. | | Your Dockerfile (`build`, `up`) | The CLI cannot change your `RUN`s, so it passes the same secret to buildx; a `RUN` opts in by mounting it (below). | | Container (`up`) | The bundle is bind-mounted read-only at `/run/devcontainer/ca-bundle.crt` and the five variables are set in the container's environment, over the configuration's `containerEnv`, so lifecycle commands and every shell trust it. | -| `run-user-commands` | Sets the variables for the lifecycle commands. A running container takes no new mount, so the container must have been created with `up --ca-bundle`; one without the mount is an error. | +| `run-user-commands` | Takes no flag: the lifecycle commands get the variables from the environment `up --ca-bundle` gave the container. | ```dockerfile RUN --mount=type=secret,id=devcontainer-ca-bundle,target=/run/devcontainer/ca-bundle.crt \ diff --git a/docs/parity/cli-flags-inventory.yaml b/docs/parity/cli-flags-inventory.yaml index 57cdacf..8a7cb5f 100644 --- a/docs/parity/cli-flags-inventory.yaml +++ b/docs/parity/cli-flags-inventory.yaml @@ -526,10 +526,6 @@ commands: type: string secrets-file: type: string - # Go-only divergence: TS has no ca-bundle. The bundle's variables for the - # lifecycle commands of a container up created with --ca-bundle. - ca-bundle: - type: string log-file: type: string description: "Log file path." diff --git a/internal/cli/ca_bundle.go b/internal/cli/ca_bundle.go index 853a833..ad3892c 100644 --- a/internal/cli/ca_bundle.go +++ b/internal/cli/ca_bundle.go @@ -6,12 +6,10 @@ import ( "maps" "os" "path/filepath" - "slices" "strings" "github.com/devcontainers/cli/internal/imagemeta" "github.com/devcontainers/cli/internal/log" - "github.com/moby/moby/api/types/container" dockermount "github.com/moby/moby/api/types/mount" ) @@ -66,16 +64,3 @@ func withCABundleEnv(env map[string]string, path string) map[string]string { } return out } - -func caBundleRemoteEnv(path string) []string { - if path == "" { - return nil - } - return imagemeta.CABundleEnv() -} - -func containerHasCABundle(mounts []container.MountPoint) bool { - return slices.ContainsFunc(mounts, func(m container.MountPoint) bool { - return m.Destination == imagemeta.CABundlePath - }) -} diff --git a/internal/cli/ca_bundle_test.go b/internal/cli/ca_bundle_test.go index 3058813..601fe73 100644 --- a/internal/cli/ca_bundle_test.go +++ b/internal/cli/ca_bundle_test.go @@ -20,7 +20,6 @@ import ( "github.com/devcontainers/cli/internal/imagemeta" "github.com/devcontainers/cli/internal/log" - "github.com/moby/moby/api/types/container" dockermount "github.com/moby/moby/api/types/mount" ) @@ -114,7 +113,6 @@ func TestCABundleFlagIsValidated(t *testing.T) { for _, args := range [][]string{ {"build", "--ca-bundle", bad}, {"up", "--ca-bundle", bad}, - {"run-user-commands", "--ca-bundle", bad}, } { var stderr bytes.Buffer root := NewRootCommand() @@ -128,6 +126,19 @@ func TestCABundleFlagIsValidated(t *testing.T) { } } +func TestRunUserCommandsTakesNoCABundle(t *testing.T) { + bundle := writeTestFile(t, filepath.Join(t.TempDir(), "bundle.crt"), newTestCA(t).pem) + var stderr bytes.Buffer + root := NewRootCommand() + root.SetArgs([]string{"run-user-commands", "--ca-bundle", bundle, "--workspace-folder", t.TempDir()}) + root.SetOut(&bytes.Buffer{}) + root.SetErr(&stderr) + root.SilenceErrors = true + if err := root.Execute(); err == nil || !strings.Contains(err.Error()+stderr.String(), "unknown flag: --ca-bundle") { + t.Errorf("run-user-commands --ca-bundle: err %v, stderr %q; want an unknown flag", err, stderr.String()) + } +} + func TestCABundleBuildSecrets(t *testing.T) { var logged bytes.Buffer logger := log.New(log.Options{Level: log.LevelTrace, Format: "text", Writer: &logged}) @@ -175,12 +186,6 @@ func TestCABundleEnvNamesEachTLSClient(t *testing.T) { if got := imagemeta.CABundleEnv(); !slices.Equal(got, want) { t.Errorf("CABundleEnv = %v, want %v", got, want) } - if got := caBundleRemoteEnv("/b.crt"); !slices.Equal(got, want) { - t.Errorf("caBundleRemoteEnv = %v, want %v", got, want) - } - if got := caBundleRemoteEnv(""); got != nil { - t.Errorf(`caBundleRemoteEnv("") = %v, want none`, got) - } } func TestCABundleMountIsReadOnly(t *testing.T) { @@ -189,12 +194,3 @@ func TestCABundleMountIsReadOnly(t *testing.T) { t.Errorf("caBundleMount = %+v, want %+v", got, want) } } - -func TestContainerHasCABundle(t *testing.T) { - if containerHasCABundle([]container.MountPoint{{Destination: "/workspaces/x"}}) { - t.Error("a container without the bundle's mount has it") - } - if !containerHasCABundle([]container.MountPoint{{Destination: "/workspaces/x"}, {Destination: "/run/devcontainer/ca-bundle.crt"}}) { - t.Error("a container with the bundle's mount lacks it") - } -} diff --git a/internal/cli/e2e_ca_bundle_test.go b/internal/cli/e2e_ca_bundle_test.go index 8fc26af..af4a565 100644 --- a/internal/cli/e2e_ca_bundle_test.go +++ b/internal/cli/e2e_ca_bundle_test.go @@ -28,7 +28,7 @@ const e2eBaseImage = "mcr.microsoft.com/devcontainers/base:ubuntu" // builds' RUNs and dev containers reach it by its address, with a certificate // for that address signed by ca. The host's firewall may drop a container's // traffic to the host, so the server is a container too. -func startTLSServer(t *testing.T, ca *testCA) (id, url string) { +func startTLSServer(t *testing.T, ca *testCA) string { t.Helper() certs := t.TempDir() if err := os.Chmod(certs, 0o755); err != nil { @@ -42,7 +42,7 @@ func startTLSServer(t *testing.T, ca *testCA) (id, url string) { if err != nil { t.Fatalf("start TLS server: %v\n%s%s", err, out, runErr.Bytes()) } - id = strings.TrimSpace(string(out)) + id := strings.TrimSpace(string(out)) t.Cleanup(func() { removeByID(id) }) ipOut, err := exec.Command("docker", "inspect", "-f", "{{range .NetworkSettings.Networks}}{{.IPAddress}} {{end}}", id).CombinedOutput() if err != nil { @@ -69,7 +69,7 @@ func startTLSServer(t *testing.T, ca *testCA) (id, url string) { conn, err := tls.DialWithDialer(&net.Dialer{Timeout: time.Second}, "tcp", addr, &tls.Config{RootCAs: pool}) if err == nil { conn.Close() - return id, "https://" + addr + "/" + return "https://" + addr + "/" } if time.Now().After(deadline) { t.Fatalf("TLS server at %s never answered: %v\n%s", addr, err, serverDiagnostics(id)) @@ -174,7 +174,7 @@ func uniqueImageName(t *testing.T, prefix string) string { // that mounts the secret reach an HTTPS server signed by a CA the image does // not trust, and fail without it; the image carries neither the bundle nor its // content; the container trusts the server in curl, node and its lifecycle -// commands; run-user-commands refuses a container created without it. +// commands, run-user-commands' included, which takes no flag of its own. func TestE2E_CABundle(t *testing.T) { if !dockerAvailable() { t.Skip("Docker not available") @@ -184,7 +184,7 @@ func TestE2E_CABundle(t *testing.T) { // A line of the CA's base64 body: in a file, a layer or a config only if // the bundle was copied there. marker := strings.Split(string(ca.pem), "\n")[1] - serverID, url := startTLSServer(t, ca) + url := startTLSServer(t, ca) // The server answers any path; a path of this run's own keeps BuildKit from // reusing a RUN an earlier run built, which would pass without the bundle. nonce := make([]byte, 8) @@ -198,7 +198,8 @@ func TestE2E_CABundle(t *testing.T) { ".devcontainer/devcontainer.json": `{ "image": "` + e2eBaseImage + `", "features": { "./reach": {} }, - "postCreateCommand": "` + reach + ` && touch /tmp/post-create-trusted" + "postCreateCommand": "` + reach + ` && touch /tmp/post-create-trusted", + "postAttachCommand": "` + reach + ` && touch /tmp/post-attach-trusted" }`, ".devcontainer/reach/devcontainer-feature.json": `{"id": "reach", "version": "1.0.0", "name": "reach"}`, ".devcontainer/reach/install.sh": "#!/bin/sh\nset -e\n" + reach + "\necho reached > /feature-reached\n", @@ -247,12 +248,9 @@ func TestE2E_CABundle(t *testing.T) { if out, err := exec.Command("docker", "exec", id, "sh", "-c", "echo x > /run/devcontainer/ca-bundle.crt").CombinedOutput(); err == nil { t.Errorf("the bundle is writable in the container:\n%s", out) } - assertOutcome(t, runCLI(t, "run-user-commands", "--container-id", id, "--ca-bundle", bundle), "success") - refused := runCLI(t, "run-user-commands", "--container-id", serverID, "--ca-bundle", bundle) - assertOutcome(t, refused, "error") - if !strings.Contains(refused, "has no CA bundle at /run/devcontainer/ca-bundle.crt") { - t.Errorf("run-user-commands on a container without the bundle: %s", refused) - } + dockerExec(t, id, "rm /tmp/post-attach-trusted") + assertOutcome(t, runCLI(t, "run-user-commands", "--container-id", id), "success") + dockerExec(t, id, "test -f /tmp/post-attach-trusted") }) t.Run("node", func(t *testing.T) { diff --git a/internal/cli/run_user_commands.go b/internal/cli/run_user_commands.go index 59aa50e..666cfea 100644 --- a/internal/cli/run_user_commands.go +++ b/internal/cli/run_user_commands.go @@ -35,7 +35,6 @@ type runUserCommandsOpts struct { defaultUserEnvProbe string containerSessionDataFolder string secretsFile string - caBundle string terminalColumns givenInt terminalRows givenInt } @@ -79,7 +78,6 @@ func newRunUserCommandsCmd() *cobra.Command { _ = f.MarkHidden("skip-feature-auto-mapping") // hidden testing flag (TS parity) f.StringVar(&opts.containerSessionDataFolder, "container-session-data-folder", "", "") f.StringVar(&opts.secretsFile, "secrets-file", "", "") - f.StringVar(&opts.caBundle, "ca-bundle", "", "Set the CA bundle's variables for the lifecycle commands; the container must have been created with `up --ca-bundle`.") f.String("user-data-folder", "", "") f.Var(&opts.terminalColumns, "terminal-columns", "") f.Var(&opts.terminalRows, "terminal-rows", "") @@ -132,10 +130,6 @@ func prepareRunUserCommandsOpts(opts *runUserCommandsOpts) error { if err := validateRemoteEnvs(opts.remoteEnvs); err != nil { return err } - var err error - if opts.caBundle, err = resolveCABundle(opts.caBundle); err != nil { - return err - } if opts.workspaceFolder == "" && len(opts.idLabels) == 0 && opts.containerID == "" { opts.workspaceFolder, _ = os.Getwd() } @@ -158,8 +152,8 @@ func prepareRunUserCommandsOpts(opts *runUserCommandsOpts) error { // run-user-commands.workspace-secrets-success, // run-user-commands.workspace-subfolder-config-success, // run-user-commands.container-metadata-success, -// run-user-commands.hook-failure-error); TestE2E_CABundle runs it with -// --ca-bundle on a container with the bundle and on one without. +// run-user-commands.hook-failure-error); TestE2E_CABundle runs it on a +// container up created with --ca-bundle. func runUserCommandsInContainer(ctx context.Context, out Output, opts *runUserCommandsOpts, logger log.Logger, loadResult *config.LoadResult) error { engine, err := docker.NewEngineClient(logger) if err != nil { @@ -198,9 +192,6 @@ func runUserCommandsInContainer(ctx context.Context, out Output, opts *runUserCo if err != nil { return writeErrorResult(out, fmt.Sprintf("Failed to inspect container: %v", err)) } - if opts.caBundle != "" && !containerHasCABundle(inspect.Mounts) { - return writeErrorResult(out, fmt.Sprintf("--ca-bundle: container %s has no CA bundle at %s; create it with `up --ca-bundle`.", containerID, imagemeta.CABundlePath)) - } // The merged configuration is the container's metadata (TS // getImageMetadataFromContainer), which records the configuration's entry // for a container the CLI created. @@ -239,7 +230,7 @@ func runUserCommandsInContainer(ctx context.Context, out Output, opts *runUserCo remoteUser := lifecycleRemoteUser(merged, cfg) probed := lifecycle.ProbedRemoteEnv(ctx, engine, logger, containerID, remoteUser, lifecycleUserEnvProbe(opts.defaultUserEnvProbe, cfg), opts.containerSessionDataFolder) - remoteEnv := lifecycleRemoteEnv(probed, append(caBundleRemoteEnv(opts.caBundle), buildSecretsFromFile(opts.secretsFile)...), opts.remoteEnvs, cfg) + remoteEnv := lifecycleRemoteEnv(probed, buildSecretsFromFile(opts.secretsFile), opts.remoteEnvs, cfg) shellServer := lifecycle.NewShellServer(ctx, engine, containerID, remoteUser, remoteEnv...) defer shellServer.Close()