Skip to content

Commit 5aa1dc3

Browse files
aledbfclaude
andcommitted
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 <noreply@anthropic.com>
1 parent c890877 commit 5aa1dc3

7 files changed

Lines changed: 29 additions & 63 deletions

File tree

‎docs/DIVERGENCES.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ touches a compared surface, reflected in the parity matrix.
1616
prebuilt image, skip build + feature install), `read-configuration --cache-key`
1717
(deterministic content hash; additive — default output is byte-identical to TS),
1818
`build --secrets-file` (BuildKit build secrets; TS `build` has no such flag),
19-
`--ca-bundle` on `build`, `up` and `run-user-commands` (extra CAs trusted in Feature
19+
`--ca-bundle` on `build` and `up` (extra CAs trusted in Feature
2020
installs, opted-in Dockerfile `RUN`s and the container, mounted and never copied), `open`
2121
(launch VS Code attached to the workspace's dev container via a vscode-remote:// URI),
2222
`stop`/`down` (pause, or stop+remove, a workspace's container — Compose-aware; the

‎docs/go-only-features.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,14 +213,14 @@ devcontainer up --workspace-folder . --ca-bundle /run/proxy/ca-bundle.crt
213213

214214
The file is a full PEM bundle: the system CAs plus the extra ones, since some tools
215215
replace the system pool with the file instead of adding to it. A file with no PEM
216-
certificate is refused. `build`, `up` and `run-user-commands` take the flag.
216+
certificate is refused. `build` and `up` take the flag.
217217

218218
| Where | What happens |
219219
| --- | --- |
220220
| 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. |
221221
| 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). |
222222
| 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. |
223-
| `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. |
223+
| `run-user-commands` | Takes no flag: the lifecycle commands get the variables from the environment `up --ca-bundle` gave the container. |
224224

225225
```dockerfile
226226
RUN --mount=type=secret,id=devcontainer-ca-bundle,target=/run/devcontainer/ca-bundle.crt \

‎docs/parity/cli-flags-inventory.yaml‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -526,10 +526,6 @@ commands:
526526
type: string
527527
secrets-file:
528528
type: string
529-
# Go-only divergence: TS has no ca-bundle. The bundle's variables for the
530-
# lifecycle commands of a container up created with --ca-bundle.
531-
ca-bundle:
532-
type: string
533529
log-file:
534530
type: string
535531
description: "Log file path."

‎internal/cli/ca_bundle.go‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,10 @@ import (
66
"maps"
77
"os"
88
"path/filepath"
9-
"slices"
109
"strings"
1110

1211
"github.com/devcontainers/cli/internal/imagemeta"
1312
"github.com/devcontainers/cli/internal/log"
14-
"github.com/moby/moby/api/types/container"
1513
dockermount "github.com/moby/moby/api/types/mount"
1614
)
1715

@@ -66,16 +64,3 @@ func withCABundleEnv(env map[string]string, path string) map[string]string {
6664
}
6765
return out
6866
}
69-
70-
func caBundleRemoteEnv(path string) []string {
71-
if path == "" {
72-
return nil
73-
}
74-
return imagemeta.CABundleEnv()
75-
}
76-
77-
func containerHasCABundle(mounts []container.MountPoint) bool {
78-
return slices.ContainsFunc(mounts, func(m container.MountPoint) bool {
79-
return m.Destination == imagemeta.CABundlePath
80-
})
81-
}

‎internal/cli/ca_bundle_test.go‎

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020

2121
"github.com/devcontainers/cli/internal/imagemeta"
2222
"github.com/devcontainers/cli/internal/log"
23-
"github.com/moby/moby/api/types/container"
2423
dockermount "github.com/moby/moby/api/types/mount"
2524
)
2625

@@ -114,7 +113,6 @@ func TestCABundleFlagIsValidated(t *testing.T) {
114113
for _, args := range [][]string{
115114
{"build", "--ca-bundle", bad},
116115
{"up", "--ca-bundle", bad},
117-
{"run-user-commands", "--ca-bundle", bad},
118116
} {
119117
var stderr bytes.Buffer
120118
root := NewRootCommand()
@@ -128,6 +126,19 @@ func TestCABundleFlagIsValidated(t *testing.T) {
128126
}
129127
}
130128

129+
func TestRunUserCommandsTakesNoCABundle(t *testing.T) {
130+
bundle := writeTestFile(t, filepath.Join(t.TempDir(), "bundle.crt"), newTestCA(t).pem)
131+
var stderr bytes.Buffer
132+
root := NewRootCommand()
133+
root.SetArgs([]string{"run-user-commands", "--ca-bundle", bundle, "--workspace-folder", t.TempDir()})
134+
root.SetOut(&bytes.Buffer{})
135+
root.SetErr(&stderr)
136+
root.SilenceErrors = true
137+
if err := root.Execute(); err == nil || !strings.Contains(err.Error()+stderr.String(), "unknown flag: --ca-bundle") {
138+
t.Errorf("run-user-commands --ca-bundle: err %v, stderr %q; want an unknown flag", err, stderr.String())
139+
}
140+
}
141+
131142
func TestCABundleBuildSecrets(t *testing.T) {
132143
var logged bytes.Buffer
133144
logger := log.New(log.Options{Level: log.LevelTrace, Format: "text", Writer: &logged})
@@ -175,12 +186,6 @@ func TestCABundleEnvNamesEachTLSClient(t *testing.T) {
175186
if got := imagemeta.CABundleEnv(); !slices.Equal(got, want) {
176187
t.Errorf("CABundleEnv = %v, want %v", got, want)
177188
}
178-
if got := caBundleRemoteEnv("/b.crt"); !slices.Equal(got, want) {
179-
t.Errorf("caBundleRemoteEnv = %v, want %v", got, want)
180-
}
181-
if got := caBundleRemoteEnv(""); got != nil {
182-
t.Errorf(`caBundleRemoteEnv("") = %v, want none`, got)
183-
}
184189
}
185190

186191
func TestCABundleMountIsReadOnly(t *testing.T) {
@@ -189,12 +194,3 @@ func TestCABundleMountIsReadOnly(t *testing.T) {
189194
t.Errorf("caBundleMount = %+v, want %+v", got, want)
190195
}
191196
}
192-
193-
func TestContainerHasCABundle(t *testing.T) {
194-
if containerHasCABundle([]container.MountPoint{{Destination: "/workspaces/x"}}) {
195-
t.Error("a container without the bundle's mount has it")
196-
}
197-
if !containerHasCABundle([]container.MountPoint{{Destination: "/workspaces/x"}, {Destination: "/run/devcontainer/ca-bundle.crt"}}) {
198-
t.Error("a container with the bundle's mount lacks it")
199-
}
200-
}

‎internal/cli/e2e_ca_bundle_test.go‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ const e2eBaseImage = "mcr.microsoft.com/devcontainers/base:ubuntu"
2828
// builds' RUNs and dev containers reach it by its address, with a certificate
2929
// for that address signed by ca. The host's firewall may drop a container's
3030
// traffic to the host, so the server is a container too.
31-
func startTLSServer(t *testing.T, ca *testCA) (id, url string) {
31+
func startTLSServer(t *testing.T, ca *testCA) string {
3232
t.Helper()
3333
certs := t.TempDir()
3434
if err := os.Chmod(certs, 0o755); err != nil {
@@ -42,7 +42,7 @@ func startTLSServer(t *testing.T, ca *testCA) (id, url string) {
4242
if err != nil {
4343
t.Fatalf("start TLS server: %v\n%s%s", err, out, runErr.Bytes())
4444
}
45-
id = strings.TrimSpace(string(out))
45+
id := strings.TrimSpace(string(out))
4646
t.Cleanup(func() { removeByID(id) })
4747
ipOut, err := exec.Command("docker", "inspect", "-f", "{{range .NetworkSettings.Networks}}{{.IPAddress}} {{end}}", id).CombinedOutput()
4848
if err != nil {
@@ -69,7 +69,7 @@ func startTLSServer(t *testing.T, ca *testCA) (id, url string) {
6969
conn, err := tls.DialWithDialer(&net.Dialer{Timeout: time.Second}, "tcp", addr, &tls.Config{RootCAs: pool})
7070
if err == nil {
7171
conn.Close()
72-
return id, "https://" + addr + "/"
72+
return "https://" + addr + "/"
7373
}
7474
if time.Now().After(deadline) {
7575
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 {
174174
// that mounts the secret reach an HTTPS server signed by a CA the image does
175175
// not trust, and fail without it; the image carries neither the bundle nor its
176176
// content; the container trusts the server in curl, node and its lifecycle
177-
// commands; run-user-commands refuses a container created without it.
177+
// commands, run-user-commands' included, which takes no flag of its own.
178178
func TestE2E_CABundle(t *testing.T) {
179179
if !dockerAvailable() {
180180
t.Skip("Docker not available")
@@ -184,7 +184,7 @@ func TestE2E_CABundle(t *testing.T) {
184184
// A line of the CA's base64 body: in a file, a layer or a config only if
185185
// the bundle was copied there.
186186
marker := strings.Split(string(ca.pem), "\n")[1]
187-
serverID, url := startTLSServer(t, ca)
187+
url := startTLSServer(t, ca)
188188
// The server answers any path; a path of this run's own keeps BuildKit from
189189
// reusing a RUN an earlier run built, which would pass without the bundle.
190190
nonce := make([]byte, 8)
@@ -198,7 +198,8 @@ func TestE2E_CABundle(t *testing.T) {
198198
".devcontainer/devcontainer.json": `{
199199
"image": "` + e2eBaseImage + `",
200200
"features": { "./reach": {} },
201-
"postCreateCommand": "` + reach + ` && touch /tmp/post-create-trusted"
201+
"postCreateCommand": "` + reach + ` && touch /tmp/post-create-trusted",
202+
"postAttachCommand": "` + reach + ` && touch /tmp/post-attach-trusted"
202203
}`,
203204
".devcontainer/reach/devcontainer-feature.json": `{"id": "reach", "version": "1.0.0", "name": "reach"}`,
204205
".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) {
247248
if out, err := exec.Command("docker", "exec", id, "sh", "-c", "echo x > /run/devcontainer/ca-bundle.crt").CombinedOutput(); err == nil {
248249
t.Errorf("the bundle is writable in the container:\n%s", out)
249250
}
250-
assertOutcome(t, runCLI(t, "run-user-commands", "--container-id", id, "--ca-bundle", bundle), "success")
251-
refused := runCLI(t, "run-user-commands", "--container-id", serverID, "--ca-bundle", bundle)
252-
assertOutcome(t, refused, "error")
253-
if !strings.Contains(refused, "has no CA bundle at /run/devcontainer/ca-bundle.crt") {
254-
t.Errorf("run-user-commands on a container without the bundle: %s", refused)
255-
}
251+
dockerExec(t, id, "rm /tmp/post-attach-trusted")
252+
assertOutcome(t, runCLI(t, "run-user-commands", "--container-id", id), "success")
253+
dockerExec(t, id, "test -f /tmp/post-attach-trusted")
256254
})
257255

258256
t.Run("node", func(t *testing.T) {

‎internal/cli/run_user_commands.go‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ type runUserCommandsOpts struct {
3535
defaultUserEnvProbe string
3636
containerSessionDataFolder string
3737
secretsFile string
38-
caBundle string
3938
terminalColumns givenInt
4039
terminalRows givenInt
4140
}
@@ -79,7 +78,6 @@ func newRunUserCommandsCmd() *cobra.Command {
7978
_ = f.MarkHidden("skip-feature-auto-mapping") // hidden testing flag (TS parity)
8079
f.StringVar(&opts.containerSessionDataFolder, "container-session-data-folder", "", "")
8180
f.StringVar(&opts.secretsFile, "secrets-file", "", "")
82-
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`.")
8381
f.String("user-data-folder", "", "")
8482
f.Var(&opts.terminalColumns, "terminal-columns", "")
8583
f.Var(&opts.terminalRows, "terminal-rows", "")
@@ -132,10 +130,6 @@ func prepareRunUserCommandsOpts(opts *runUserCommandsOpts) error {
132130
if err := validateRemoteEnvs(opts.remoteEnvs); err != nil {
133131
return err
134132
}
135-
var err error
136-
if opts.caBundle, err = resolveCABundle(opts.caBundle); err != nil {
137-
return err
138-
}
139133
if opts.workspaceFolder == "" && len(opts.idLabels) == 0 && opts.containerID == "" {
140134
opts.workspaceFolder, _ = os.Getwd()
141135
}
@@ -158,8 +152,8 @@ func prepareRunUserCommandsOpts(opts *runUserCommandsOpts) error {
158152
// run-user-commands.workspace-secrets-success,
159153
// run-user-commands.workspace-subfolder-config-success,
160154
// run-user-commands.container-metadata-success,
161-
// run-user-commands.hook-failure-error); TestE2E_CABundle runs it with
162-
// --ca-bundle on a container with the bundle and on one without.
155+
// run-user-commands.hook-failure-error); TestE2E_CABundle runs it on a
156+
// container up created with --ca-bundle.
163157
func runUserCommandsInContainer(ctx context.Context, out Output, opts *runUserCommandsOpts, logger log.Logger, loadResult *config.LoadResult) error {
164158
engine, err := docker.NewEngineClient(logger)
165159
if err != nil {
@@ -198,9 +192,6 @@ func runUserCommandsInContainer(ctx context.Context, out Output, opts *runUserCo
198192
if err != nil {
199193
return writeErrorResult(out, fmt.Sprintf("Failed to inspect container: %v", err))
200194
}
201-
if opts.caBundle != "" && !containerHasCABundle(inspect.Mounts) {
202-
return writeErrorResult(out, fmt.Sprintf("--ca-bundle: container %s has no CA bundle at %s; create it with `up --ca-bundle`.", containerID, imagemeta.CABundlePath))
203-
}
204195
// The merged configuration is the container's metadata (TS
205196
// getImageMetadataFromContainer), which records the configuration's entry
206197
// for a container the CLI created.
@@ -239,7 +230,7 @@ func runUserCommandsInContainer(ctx context.Context, out Output, opts *runUserCo
239230

240231
remoteUser := lifecycleRemoteUser(merged, cfg)
241232
probed := lifecycle.ProbedRemoteEnv(ctx, engine, logger, containerID, remoteUser, lifecycleUserEnvProbe(opts.defaultUserEnvProbe, cfg), opts.containerSessionDataFolder)
242-
remoteEnv := lifecycleRemoteEnv(probed, append(caBundleRemoteEnv(opts.caBundle), buildSecretsFromFile(opts.secretsFile)...), opts.remoteEnvs, cfg)
233+
remoteEnv := lifecycleRemoteEnv(probed, buildSecretsFromFile(opts.secretsFile), opts.remoteEnvs, cfg)
243234

244235
shellServer := lifecycle.NewShellServer(ctx, engine, containerID, remoteUser, remoteEnv...)
245236
defer shellServer.Close()

0 commit comments

Comments
 (0)