fix(agent): minimal env and explicit project dir for compose, resolve every config file's symlinks (#7)
This commit is contained in:
@@ -7,4 +7,3 @@ README.md
|
|||||||
.env
|
.env
|
||||||
.env.*
|
.env.*
|
||||||
gitea-action/
|
gitea-action/
|
||||||
docker-compose.example.yml
|
|
||||||
|
|||||||
+5
-2
@@ -75,8 +75,11 @@ func run() error {
|
|||||||
func logMounts(ctx context.Context, cli updater.SelfInspector, selfID string, log *slog.Logger) {
|
func logMounts(ctx context.Context, cli updater.SelfInspector, selfID string, log *slog.Logger) {
|
||||||
roots, err := updater.MountRoots(ctx, cli, selfID)
|
roots, err := updater.MountRoots(ctx, cli, selfID)
|
||||||
if err != nil || len(roots) == 0 {
|
if err != nil || len(roots) == 0 {
|
||||||
log.Warn("no_stack_mounts", "error", err,
|
attrs := []any{"hint", "Compose updates will fail; mount each stacks directory at the same path, read-write"}
|
||||||
"hint", "Compose updates will fail; mount each stacks directory at the same path, read-write")
|
if err != nil {
|
||||||
|
attrs = append(attrs, "error", err)
|
||||||
|
}
|
||||||
|
log.Warn("no_stack_mounts", attrs...)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
log.Info("stack_mounts", "roots", roots)
|
log.Info("stack_mounts", "roots", roots)
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import (
|
|||||||
"context"
|
"context"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
|
"os"
|
||||||
"os/exec"
|
"os/exec"
|
||||||
"strings"
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
@@ -43,10 +44,29 @@ func ExecDocker(ctx context.Context, dir string, out io.Writer, args ...string)
|
|||||||
cmd.Dir = dir
|
cmd.Dir = dir
|
||||||
cmd.Stdout = out
|
cmd.Stdout = out
|
||||||
cmd.Stderr = out
|
cmd.Stderr = out
|
||||||
|
cmd.Env = dockerEnv()
|
||||||
cmd.WaitDelay = 10 * time.Second
|
cmd.WaitDelay = 10 * time.Second
|
||||||
return cmd.Run()
|
return cmd.Run()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// dockerEnvVars are the only variables from the agent's environment that
|
||||||
|
// reach docker compose. Compose prefers process variables over the stack's
|
||||||
|
// .env file when it interpolates, so passing the agent's own (HOSTNAME,
|
||||||
|
// AGENT_TOKEN, ...) would change stack configuration and expose the token.
|
||||||
|
var dockerEnvVars = []string{"PATH", "HOME", "DOCKER_HOST", "DOCKER_CONFIG", "DOCKER_TLS_VERIFY", "DOCKER_CERT_PATH"}
|
||||||
|
|
||||||
|
// dockerEnv returns the allowed variables that are set in the agent's
|
||||||
|
// environment, as KEY=value pairs.
|
||||||
|
func dockerEnv() []string {
|
||||||
|
var env []string
|
||||||
|
for _, k := range dockerEnvVars {
|
||||||
|
if v := os.Getenv(k); v != "" {
|
||||||
|
env = append(env, k+"="+v)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return env
|
||||||
|
}
|
||||||
|
|
||||||
// ComposeDocker is the subset of the Docker SDK ComposeExecutor needs.
|
// ComposeDocker is the subset of the Docker SDK ComposeExecutor needs.
|
||||||
type ComposeDocker interface {
|
type ComposeDocker interface {
|
||||||
SelfInspector
|
SelfInspector
|
||||||
@@ -115,7 +135,9 @@ func (e *ComposeExecutor) Execute(ctx context.Context, job protocol.JobStart, ou
|
|||||||
return "", err
|
return "", err
|
||||||
}
|
}
|
||||||
|
|
||||||
base := []string{"compose", "-p", project}
|
// --project-directory is explicit because, with -f, compose would take it
|
||||||
|
// from the first file, which differs for stacks started with the flag.
|
||||||
|
base := []string{"compose", "-p", project, "--project-directory", workingDir}
|
||||||
for _, f := range files {
|
for _, f := range files {
|
||||||
base = append(base, "-f", f)
|
base = append(base, "-f", f)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -66,7 +66,7 @@ func TestComposeExecutor(t *testing.T) {
|
|||||||
ro := fakeSelf{mounts: []container.MountPoint{{Type: mount.TypeBind, Source: root, Destination: root, RW: false}}}
|
ro := fakeSelf{mounts: []container.MountPoint{{Type: mount.TypeBind, Source: root, Destination: root, RW: false}}}
|
||||||
wantFiles := []string{"-f", filepath.Join(stack, "compose.yaml"), "-f", filepath.Join(stack, OverrideName)}
|
wantFiles := []string{"-f", filepath.Join(stack, "compose.yaml"), "-f", filepath.Join(stack, OverrideName)}
|
||||||
cmd := func(rest ...string) []string {
|
cmd := func(rest ...string) []string {
|
||||||
return append(append([]string{"compose", "-p", "slusk"}, wantFiles...), rest...)
|
return append(append([]string{"compose", "-p", "slusk", "--project-directory", stack}, wantFiles...), rest...)
|
||||||
}
|
}
|
||||||
overridePath := filepath.Join(stack, OverrideName)
|
overridePath := filepath.Join(stack, OverrideName)
|
||||||
cleanOverride := func() { os.Remove(overridePath) }
|
cleanOverride := func() { os.Remove(overridePath) }
|
||||||
@@ -122,6 +122,18 @@ func TestComposeExecutor(t *testing.T) {
|
|||||||
assert.ErrorContains(t, err, "refused")
|
assert.ErrorContains(t, err, "refused")
|
||||||
assert.Empty(t, rec.cmds)
|
assert.Empty(t, rec.cmds)
|
||||||
})
|
})
|
||||||
|
t.Run("working dir outside the roots", func(t *testing.T) {
|
||||||
|
outside := filepath.Join(t.TempDir(), "elsewhere")
|
||||||
|
bad := container.Summary{Labels: map[string]string{
|
||||||
|
labelWorkingDir: outside,
|
||||||
|
labelConfigFiles: filepath.Join(stack, "compose.yaml"),
|
||||||
|
}}
|
||||||
|
rec := &recorder{}
|
||||||
|
_, err := setup(fakeCompose{rw, []container.Summary{bad}}, &fakeDist{digest: d}, rec).Execute(context.Background(), job, io.Discard)
|
||||||
|
assert.ErrorContains(t, err, "not mounted")
|
||||||
|
assert.Empty(t, rec.cmds)
|
||||||
|
assert.NoFileExists(t, overridePath)
|
||||||
|
})
|
||||||
t.Run("read-only mount", func(t *testing.T) {
|
t.Run("read-only mount", func(t *testing.T) {
|
||||||
rec := &recorder{}
|
rec := &recorder{}
|
||||||
_, err := setup(fakeCompose{ro, []container.Summary{ct}}, &fakeDist{digest: d}, rec).Execute(context.Background(), job, io.Discard)
|
_, err := setup(fakeCompose{ro, []container.Summary{ct}}, &fakeDist{digest: d}, rec).Execute(context.Background(), job, io.Discard)
|
||||||
@@ -130,3 +142,24 @@ func TestComposeExecutor(t *testing.T) {
|
|||||||
assert.NoFileExists(t, overridePath)
|
assert.NoFileExists(t, overridePath)
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestDockerEnv(t *testing.T) {
|
||||||
|
t.Setenv("AGENT_TOKEN", "secret")
|
||||||
|
t.Setenv("HOSTNAME", "abc")
|
||||||
|
t.Setenv("MASTER_URL", "wss://x")
|
||||||
|
t.Setenv("PATH", "/usr/bin")
|
||||||
|
t.Setenv("DOCKER_CONFIG", "/docker-config")
|
||||||
|
t.Setenv("DOCKER_HOST", "")
|
||||||
|
os.Unsetenv("DOCKER_CERT_PATH")
|
||||||
|
|
||||||
|
env := dockerEnv()
|
||||||
|
assert.Contains(t, env, "PATH=/usr/bin")
|
||||||
|
assert.Contains(t, env, "DOCKER_CONFIG=/docker-config")
|
||||||
|
assert.NotContains(t, env, "DOCKER_HOST=", "empty variables are not passed")
|
||||||
|
for _, e := range env {
|
||||||
|
assert.NotContains(t, e, "AGENT_TOKEN")
|
||||||
|
assert.NotContains(t, e, "HOSTNAME")
|
||||||
|
assert.NotContains(t, e, "MASTER_URL")
|
||||||
|
assert.False(t, strings.HasPrefix(e, "DOCKER_CERT_PATH="), "unset variables are not passed")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -133,7 +133,7 @@ func MountRoots(ctx context.Context, cli SelfInspector, selfID string) ([]string
|
|||||||
|
|
||||||
// composeFiles returns the -f arguments for a job: the stack's config files
|
// composeFiles returns the -f arguments for a job: the stack's config files
|
||||||
// from the container label with any copy of the override removed, then the
|
// from the container label with any copy of the override removed, then the
|
||||||
// override last so it wins. Every file, and the real location of the stack
|
// override last so it wins. Every file, and the real location of its
|
||||||
// directory after resolving symlinks, must be inside one of roots.
|
// directory after resolving symlinks, must be inside one of roots.
|
||||||
func composeFiles(roots []string, configFilesLabel string) (files []string, dir string, err error) {
|
func composeFiles(roots []string, configFilesLabel string) (files []string, dir string, err error) {
|
||||||
if len(roots) == 0 {
|
if len(roots) == 0 {
|
||||||
@@ -149,19 +149,7 @@ func composeFiles(roots []string, configFilesLabel string) (files []string, dir
|
|||||||
}
|
}
|
||||||
dir = filepath.Dir(files[0])
|
dir = filepath.Dir(files[0])
|
||||||
override := filepath.Join(dir, OverrideName)
|
override := filepath.Join(dir, OverrideName)
|
||||||
out := files[:0]
|
|
||||||
for _, f := range files {
|
|
||||||
if !filepath.IsAbs(f) || !insideAny(roots, f) {
|
|
||||||
return nil, "", notMounted(filepath.Dir(f))
|
|
||||||
}
|
|
||||||
if filepath.Clean(f) != override {
|
|
||||||
out = append(out, f)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
real, err := filepath.EvalSymlinks(dir)
|
|
||||||
if err != nil {
|
|
||||||
return nil, "", fmt.Errorf("resolve %s: %w", dir, err)
|
|
||||||
}
|
|
||||||
// Only directories count once resolved: this drops a same-path mount of
|
// Only directories count once resolved: this drops a same-path mount of
|
||||||
// a single file such as /var/run/docker.sock.
|
// a single file such as /var/run/docker.sock.
|
||||||
var realRoots []string
|
var realRoots []string
|
||||||
@@ -172,8 +160,21 @@ func composeFiles(roots []string, configFilesLabel string) (files []string, dir
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
out := files[:0]
|
||||||
|
for _, f := range files {
|
||||||
|
if !filepath.IsAbs(f) || !insideAny(roots, f) {
|
||||||
|
return nil, "", notMounted(filepath.Dir(f))
|
||||||
|
}
|
||||||
|
real, err := filepath.EvalSymlinks(filepath.Dir(f))
|
||||||
|
if err != nil {
|
||||||
|
return nil, "", fmt.Errorf("resolve %s: %w", filepath.Dir(f), err)
|
||||||
|
}
|
||||||
if !insideAny(realRoots, real) {
|
if !insideAny(realRoots, real) {
|
||||||
return nil, "", notMounted(dir)
|
return nil, "", notMounted(filepath.Dir(f))
|
||||||
|
}
|
||||||
|
if filepath.Clean(f) != override {
|
||||||
|
out = append(out, f)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
return append(out, override), dir, nil
|
return append(out, override), dir, nil
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -185,6 +185,11 @@ func TestComposeFiles(t *testing.T) {
|
|||||||
_, _, err := composeFiles([]string{root}, filepath.Join(root, "evil", "compose.yaml"))
|
_, _, err := composeFiles([]string{root}, filepath.Join(root, "evil", "compose.yaml"))
|
||||||
assert.EqualError(t, err, "stack directory "+filepath.Join(root, "evil")+" is not mounted into the agent at the same path (read-write)")
|
assert.EqualError(t, err, "stack directory "+filepath.Join(root, "evil")+" is not mounted into the agent at the same path (read-write)")
|
||||||
})
|
})
|
||||||
|
t.Run("second file under a symlink escaping the root", func(t *testing.T) {
|
||||||
|
label := filepath.Join(a, "compose.yaml") + "," + filepath.Join(root, "evil", "extra.yaml")
|
||||||
|
_, _, err := composeFiles([]string{root}, label)
|
||||||
|
assert.ErrorContains(t, err, "not mounted")
|
||||||
|
})
|
||||||
t.Run("prefix trick", func(t *testing.T) {
|
t.Run("prefix trick", func(t *testing.T) {
|
||||||
evil := root + "-evil"
|
evil := root + "-evil"
|
||||||
require.NoError(t, os.MkdirAll(evil, 0o755))
|
require.NoError(t, os.MkdirAll(evil, 0o755))
|
||||||
|
|||||||
@@ -3,7 +3,6 @@ package updater
|
|||||||
import (
|
import (
|
||||||
"encoding/base64"
|
"encoding/base64"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"errors"
|
|
||||||
"fmt"
|
"fmt"
|
||||||
"log/slog"
|
"log/slog"
|
||||||
"os"
|
"os"
|
||||||
@@ -64,7 +63,7 @@ func LoadRegistryAuth(dockerConfigDir string, log *slog.Logger) (*RegistryAuth,
|
|||||||
}
|
}
|
||||||
user, pass, ok := strings.Cut(string(dec), ":")
|
user, pass, ok := strings.Cut(string(dec), ":")
|
||||||
if !ok {
|
if !ok {
|
||||||
return nil, errors.New(path + ": auth for " + key + " is not user:password")
|
return nil, fmt.Errorf("%s: auth for %s is not user:password", path, key)
|
||||||
}
|
}
|
||||||
ac.Username, ac.Password = user, pass
|
ac.Username, ac.Password = user, pass
|
||||||
default:
|
default:
|
||||||
|
|||||||
@@ -2,7 +2,6 @@ package updater
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"errors"
|
|
||||||
"fmt"
|
"fmt"
|
||||||
|
|
||||||
"github.com/docker/docker/api/types/registry"
|
"github.com/docker/docker/api/types/registry"
|
||||||
@@ -43,7 +42,7 @@ func (r *Resolver) Resolve(ctx context.Context, ref string) (string, error) {
|
|||||||
return "", fmt.Errorf("resolve %s: %w", ref, err)
|
return "", fmt.Errorf("resolve %s: %w", ref, err)
|
||||||
}
|
}
|
||||||
if info.Descriptor.Digest == "" {
|
if info.Descriptor.Digest == "" {
|
||||||
return "", errors.New("resolve " + ref + ": registry returned no digest")
|
return "", fmt.Errorf("resolve %s: registry returned no digest", ref)
|
||||||
}
|
}
|
||||||
return ref + "@" + info.Descriptor.Digest.String(), nil
|
return ref + "@" + info.Descriptor.Digest.String(), nil
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user