Skip to content

Commit 5da2aaa

Browse files
mafredrimafredri
committed
copy config to protect against remount, fix docker perms
1 parent d559c1d commit 5da2aaa

File tree

2 files changed

+136
-71
lines changed

2 files changed

+136
-71
lines changed

‎envbuilder.go

+100-55
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ func run(ctx context.Context, opts options.Options, execArgs *execArgsInfo) erro
154154

155155
opts.Logger(log.LevelInfo, "%s %s - Build development environments from repositories in a container", newColor(color.Bold).Sprintf("envbuilder"), buildinfo.Version())
156156

157-
cleanupDockerConfigOverride, err := initDockerConfigOverride(opts.Logger, workingDir, opts.DockerConfigBase64)
157+
cleanupDockerConfigOverride, err := initDockerConfigOverride(opts.Filesystem, opts.Logger, workingDir, opts.DockerConfigBase64)
158158
if err != nil {
159159
return err
160160
}
@@ -711,6 +711,11 @@ func run(ctx context.Context, opts options.Options, execArgs *execArgsInfo) erro
711711
// Sanitize the environment of any opts!
712712
options.UnsetEnv()
713713

714+
// Remove the Docker config secret file!
715+
if err := cleanupDockerConfigOverride(); err != nil {
716+
return err
717+
}
718+
714719
// Set the environment from /etc/environment first, so it can be
715720
// overridden by the image and devcontainer settings.
716721
err = setEnvFromEtcEnvironment(opts.Logger)
@@ -770,11 +775,6 @@ func run(ctx context.Context, opts options.Options, execArgs *execArgsInfo) erro
770775
exportEnvFile.Close()
771776
}
772777

773-
// Remove the Docker config secret file!
774-
if err := cleanupDockerConfigOverride(); err != nil {
775-
return err
776-
}
777-
778778
if runtimeData.ContainerUser == "" {
779779
opts.Logger(log.LevelWarn, "#%d: no user specified, using root", stageNumber)
780780
}
@@ -978,7 +978,7 @@ func RunCacheProbe(ctx context.Context, opts options.Options) (v1.Image, error)
978978

979979
opts.Logger(log.LevelInfo, "%s %s - Build development environments from repositories in a container", newColor(color.Bold).Sprintf("envbuilder"), buildinfo.Version())
980980

981-
cleanupDockerConfigOverride, err := initDockerConfigOverride(opts.Logger, workingDir, opts.DockerConfigBase64)
981+
cleanupDockerConfigOverride, err := initDockerConfigOverride(opts.Filesystem, opts.Logger, workingDir, opts.DockerConfigBase64)
982982
if err != nil {
983983
return nil, err
984984
}
@@ -1571,6 +1571,20 @@ func fileExists(fs billy.Filesystem, path string) bool {
15711571
return err == nil
15721572
}
15731573

1574+
func readFile(fs billy.Filesystem, name string) ([]byte, error) {
1575+
f, err := fs.Open(name)
1576+
if err != nil {
1577+
return nil, fmt.Errorf("open file: %w", err)
1578+
}
1579+
defer f.Close()
1580+
1581+
b, err := io.ReadAll(f)
1582+
if err != nil {
1583+
return nil, fmt.Errorf("read file: %w", err)
1584+
}
1585+
return b, nil
1586+
}
1587+
15741588
func copyFile(fs billy.Filesystem, src, dst string, mode fs.FileMode) error {
15751589
srcF, err := fs.Open(src)
15761590
if err != nil {
@@ -1595,6 +1609,21 @@ func copyFile(fs billy.Filesystem, src, dst string, mode fs.FileMode) error {
15951609
return nil
15961610
}
15971611

1612+
func writeFile(fs billy.Filesystem, name string, data []byte, perm fs.FileMode) error {
1613+
f, err := fs.OpenFile(name, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, perm)
1614+
if err != nil {
1615+
return fmt.Errorf("create file: %w", err)
1616+
}
1617+
_, err = f.Write(data)
1618+
if err != nil {
1619+
err = fmt.Errorf("write file: %w", err)
1620+
}
1621+
if err2 := f.Close(); err2 != nil && err == nil {
1622+
err = fmt.Errorf("close file: %w", err2)
1623+
}
1624+
return err
1625+
}
1626+
15981627
func writeMagicImageFile(fs billy.Filesystem, path string, v any) error {
15991628
file, err := fs.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o644)
16001629
if err != nil {
@@ -1627,39 +1656,45 @@ func parseMagicImageFile(fs billy.Filesystem, path string, v any) error {
16271656
return nil
16281657
}
16291658

1630-
func initDockerConfigOverride(logf log.Func, workingDir workingdir.WorkingDir, dockerConfigBase64 string) (func() error, error) {
1631-
var (
1632-
oldDockerConfig = os.Getenv("DOCKER_CONFIG")
1633-
newDockerConfig = workingDir.Path()
1634-
cfgPath = workingDir.Join("config.json")
1635-
restoreEnv = func() error { return nil } // noop.
1636-
)
1637-
if dockerConfigBase64 != "" || oldDockerConfig == "" {
1638-
err := os.Setenv("DOCKER_CONFIG", newDockerConfig)
1639-
if err != nil {
1640-
logf(log.LevelError, "Failed to set DOCKER_CONFIG: %s", err)
1641-
return nil, fmt.Errorf("set DOCKER_CONFIG: %w", err)
1642-
}
1643-
logf(log.LevelInfo, "Set DOCKER_CONFIG to %s", newDockerConfig)
1659+
func initDockerConfigOverride(bfs billy.Filesystem, logf log.Func, workingDir workingdir.WorkingDir, dockerConfigBase64 string) (func() error, error) {
1660+
configFile := "config.json"
1661+
oldDockerConfig := os.Getenv("DOCKER_CONFIG")
1662+
newDockerConfig := workingDir.Path()
16441663

1645-
restoreEnv = func() error {
1646-
// Restore the old DOCKER_CONFIG value.
1664+
err := os.Setenv("DOCKER_CONFIG", newDockerConfig)
1665+
if err != nil {
1666+
logf(log.LevelError, "Failed to set DOCKER_CONFIG: %s", err)
1667+
return nil, fmt.Errorf("set DOCKER_CONFIG: %w", err)
1668+
}
1669+
logf(log.LevelInfo, "Set DOCKER_CONFIG to %s", newDockerConfig)
1670+
restoreEnv := onceErrFunc(func() error {
1671+
// Restore the old DOCKER_CONFIG value.
1672+
if err := func() error {
16471673
if oldDockerConfig == "" {
1648-
err := os.Unsetenv("DOCKER_CONFIG")
1649-
if err != nil {
1650-
return fmt.Errorf("unset DOCKER_CONFIG: %w", err)
1651-
}
1652-
return nil
1653-
}
1654-
err := os.Setenv("DOCKER_CONFIG", oldDockerConfig)
1655-
if err != nil {
1656-
return fmt.Errorf("restore DOCKER_CONFIG: %w", err)
1674+
return os.Unsetenv("DOCKER_CONFIG")
16571675
}
1658-
logf(log.LevelInfo, "Restored DOCKER_CONFIG to %s", oldDockerConfig)
1659-
return nil
1676+
return os.Setenv("DOCKER_CONFIG", oldDockerConfig)
1677+
}(); err != nil {
1678+
return fmt.Errorf("restore DOCKER_CONFIG: %w", err)
16601679
}
1661-
} else {
1662-
logf(log.LevelInfo, "Using existing DOCKER_CONFIG set to %s", oldDockerConfig)
1680+
logf(log.LevelInfo, "Restored DOCKER_CONFIG to %s", oldDockerConfig)
1681+
return nil
1682+
})
1683+
1684+
// If the user hasn't set the BASE64 encoded Docker config, we
1685+
// should respect the DOCKER_CONFIG environment variable.
1686+
oldDockerConfigFile := filepath.Join(oldDockerConfig, configFile)
1687+
if dockerConfigBase64 == "" && fileExists(bfs, oldDockerConfigFile) {
1688+
// It's possible that the target file is mounted and needs to
1689+
// be remounted later, so we should copy the file instead of
1690+
// hoping that the file is still there when we build.
1691+
logf(log.LevelInfo, "Using DOCKER_CONFIG at %s", oldDockerConfig)
1692+
b, err := readFile(bfs, oldDockerConfigFile)
1693+
if err != nil {
1694+
return nil, fmt.Errorf("read existing DOCKER_CONFIG: %w", err)
1695+
}
1696+
// Read into dockerConfigBase64 so we can keep the same logic.
1697+
dockerConfigBase64 = base64.StdEncoding.EncodeToString(b)
16631698
}
16641699

16651700
if dockerConfigBase64 == "" {
@@ -1670,40 +1705,50 @@ func initDockerConfigOverride(logf log.Func, workingDir workingdir.WorkingDir, d
16701705
if err != nil {
16711706
return restoreEnv, fmt.Errorf("decode docker config: %w", err)
16721707
}
1673-
var configFile DockerConfig
16741708
decoded, err = hujson.Standardize(decoded)
16751709
if err != nil {
16761710
return restoreEnv, fmt.Errorf("humanize json for docker config: %w", err)
16771711
}
1678-
err = json.Unmarshal(decoded, &configFile)
1712+
var dockerConfig DockerConfig
1713+
err = json.Unmarshal(decoded, &dockerConfig)
16791714
if err != nil {
16801715
return restoreEnv, fmt.Errorf("parse docker config: %w", err)
16811716
}
1682-
for k := range configFile.AuthConfigs {
1717+
for k := range dockerConfig.AuthConfigs {
16831718
logf(log.LevelInfo, "Docker config contains auth for registry %q", k)
16841719
}
1685-
err = os.WriteFile(cfgPath, decoded, 0o644)
1720+
1721+
newDockerConfigFile := filepath.Join(newDockerConfig, configFile)
1722+
err = writeFile(bfs, newDockerConfigFile, decoded, 0o644)
16861723
if err != nil {
16871724
return restoreEnv, fmt.Errorf("write docker config: %w", err)
16881725
}
1689-
logf(log.LevelInfo, "Wrote Docker config JSON to %s", cfgPath)
1726+
logf(log.LevelInfo, "Wrote Docker config JSON to %s", newDockerConfigFile)
16901727

1691-
var cleanupOnce sync.Once
1692-
return func() error {
1693-
var cleanupErr error
1694-
cleanupOnce.Do(func() {
1695-
cleanupErr = restoreEnv()
1696-
// Remove the Docker config secret file!
1697-
if err := os.Remove(cfgPath); err != nil {
1698-
if !errors.Is(err, fs.ErrNotExist) {
1699-
err = errors.Join(err, fmt.Errorf("remove docker config: %w", err))
1700-
}
1701-
logf(log.LevelError, "Failed to remove the Docker config secret file: %s", cleanupErr)
1702-
cleanupErr = errors.Join(cleanupErr, err)
1728+
cleanup := onceErrFunc(func() error {
1729+
err := restoreEnv()
1730+
// Remove the Docker config secret file!
1731+
if err2 := os.Remove(newDockerConfigFile); err2 != nil {
1732+
if !errors.Is(err2, fs.ErrNotExist) {
1733+
err2 = fmt.Errorf("remove docker config: %w", err2)
17031734
}
1735+
logf(log.LevelError, "Failed to remove the Docker config secret file: %s", err2)
1736+
err = errors.Join(err, err2)
1737+
}
1738+
return err
1739+
})
1740+
return cleanup, nil
1741+
}
1742+
1743+
func onceErrFunc(f func() error) func() error {
1744+
var once sync.Once
1745+
return func() error {
1746+
var err error
1747+
once.Do(func() {
1748+
err = f()
17041749
})
1705-
return cleanupErr
1706-
}, nil
1750+
return err
1751+
}
17071752
}
17081753

17091754
// Allows quick testing of layer caching using a local directory!

‎integration/integration_test.go

+36-16
Original file line numberDiff line numberDiff line change
@@ -565,6 +565,7 @@ func TestBuildFromDockerfile(t *testing.T) {
565565
require.NoError(t, err)
566566

567567
require.Contains(t, logbuf.String(), "Set DOCKER_CONFIG to /.envbuilder")
568+
require.NotContains(t, logbuf.String(), "Using DOCKER_CONFIG at ")
568569

569570
output := execContainer(t, ctr, "echo hello")
570571
require.Equal(t, "hello", strings.TrimSpace(output))
@@ -582,8 +583,17 @@ func TestBuildDockerConfigPathFromEnv(t *testing.T) {
582583
"Dockerfile": "FROM " + testImageAlpine,
583584
},
584585
})
586+
config, err := json.Marshal(envbuilder.DockerConfig{
587+
AuthConfigs: map[string]clitypes.AuthConfig{
588+
"mytestimage": {
589+
Username: "user",
590+
Password: "test",
591+
},
592+
},
593+
})
594+
require.NoError(t, err)
585595
dir := t.TempDir()
586-
err := os.WriteFile(filepath.Join(dir, "config.json"), []byte(`{"experimental": "enabled"}`), 0o644)
596+
err = os.WriteFile(filepath.Join(dir, "config.json"), config, 0o644)
587597
require.NoError(t, err)
588598

589599
logbuf := new(bytes.Buffer)
@@ -593,12 +603,16 @@ func TestBuildDockerConfigPathFromEnv(t *testing.T) {
593603
envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"),
594604
"DOCKER_CONFIG=/config",
595605
},
596-
binds: []string{fmt.Sprintf("%s:/config:ro", dir)},
597-
logbuf: logbuf,
606+
privileged: true,
607+
binds: []string{fmt.Sprintf("%s:/config:ro", dir)},
608+
logbuf: logbuf,
598609
})
599610
require.NoError(t, err)
600611

601-
require.Contains(t, logbuf.String(), "Using existing DOCKER_CONFIG set to /config")
612+
// Logs that the DOCKER_CONFIG is used.
613+
require.Contains(t, logbuf.String(), "Using DOCKER_CONFIG at /config")
614+
// Logs registry auth info from existing file.
615+
require.Contains(t, logbuf.String(), "mytestimage")
602616
}
603617

604618
func TestBuildDockerConfigDefaultPath(t *testing.T) {
@@ -619,6 +633,7 @@ func TestBuildDockerConfigDefaultPath(t *testing.T) {
619633
require.NoError(t, err)
620634

621635
require.Contains(t, logbuf.String(), "Set DOCKER_CONFIG to /.envbuilder")
636+
require.NotContains(t, logbuf.String(), "Using DOCKER_CONFIG at ")
622637
}
623638

624639
func TestBuildPrintBuildOutput(t *testing.T) {
@@ -2345,11 +2360,12 @@ func startContainerFromRef(ctx context.Context, t *testing.T, cli *client.Client
23452360
}
23462361

23472362
type runOpts struct {
2348-
image string
2349-
binds []string
2350-
env []string
2351-
volumes map[string]string
2352-
logbuf *bytes.Buffer
2363+
image string
2364+
privileged bool // Required for remounting.
2365+
binds []string
2366+
env []string
2367+
volumes map[string]string
2368+
logbuf *bytes.Buffer
23532369
}
23542370

23552371
// runEnvbuilder starts the envbuilder container with the given environment
@@ -2387,18 +2403,22 @@ func runEnvbuilder(t *testing.T, opts runOpts) (string, error) {
23872403
require.NoError(t, err, "failed to read image pull response")
23882404
img = opts.image
23892405
}
2406+
hostConfig := &container.HostConfig{
2407+
NetworkMode: container.NetworkMode("host"),
2408+
Binds: opts.binds,
2409+
Mounts: mounts,
2410+
}
2411+
if opts.privileged {
2412+
hostConfig.CapAdd = append(hostConfig.CapAdd, "SYS_ADMIN")
2413+
hostConfig.Privileged = true
2414+
}
23902415
ctr, err := cli.ContainerCreate(ctx, &container.Config{
23912416
Image: img,
23922417
Env: opts.env,
23932418
Labels: map[string]string{
23942419
testContainerLabel: "true",
23952420
},
2396-
}, &container.HostConfig{
2397-
CapAdd: []string{"SYS_ADMIN"}, // For remounting.
2398-
NetworkMode: container.NetworkMode("host"),
2399-
Binds: opts.binds,
2400-
Mounts: mounts,
2401-
}, nil, nil, "")
2421+
}, hostConfig, nil, nil, "")
24022422
require.NoError(t, err)
24032423
t.Cleanup(func() {
24042424
_ = cli.ContainerRemove(ctx, ctr.ID, container.RemoveOptions{
@@ -2413,7 +2433,7 @@ func runEnvbuilder(t *testing.T, opts runOpts) (string, error) {
24132433
go func() {
24142434
for log := range logChan {
24152435
if opts.logbuf != nil {
2416-
opts.logbuf.WriteString(log)
2436+
opts.logbuf.WriteString(log + "\n")
24172437
}
24182438
if strings.HasPrefix(log, "=== Running init command") {
24192439
errChan <- nil

0 commit comments

Comments
 (0)