feat(config): ${env:NAME} placeholders in plugin config; ADR 0003

README.md

@@ -101,6 +101,18 @@ The API auth settings follow the same rule, so `api.auth.client_id` and `api.aut
 `CCF_API_AUTH_CLIENT_ID` and `CCF_API_AUTH_CLIENT_SECRET`. These values must be configured together; setting only one
 will fail agent startup validation. The `client_id` value must be a valid UUID.
 
+Values that come from `CCF_PLUGINS_*` variables are masked in the configuration reports the agent sends to the API,
+as are secret-like keys and values (see [configuration](./docs/configuration.md#envname-placeholders)).
+Plugins never receive `CCF_API_AUTH_*` variables.
+
+### Remote configuration and state
+
+With `api.auth` credentials the agent reports its configuration to the API. With `remote_config.mode` set to
+`apply_safe` or `apply_all` it also applies a configuration overlay stored there, including `${env:NAME}` placeholders
+in plugin config; the default mode, `report`, never fetches or applies one. Each instance keeps a stable ID and a cache in a state directory (`--state-dir` / `CCF_STATE_DIR`;
+`--instance-id` / `CCF_INSTANCE_ID`). See [configuration](./docs/configuration.md#remote-configuration) and
+[ADR 0003](./docs/adr/0003-remote-config-overlay.md).
+
 ## Usage
 
 To run the agent, you must first build the agent, and then run it with the `agent` command. It is recommended,

cmd/config.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"maps"
 	"os"
 	"path/filepath"
 	"regexp"
@@ -307,6 +308,80 @@ func touchedByOverlay(ptr string, touched []string) bool {
 	return false
 }
 
+// resolveEnv resolves ${env:NAME} placeholders in plugins.*.config values (R24) with the R60
+// file-origin leniency: when every unset variable of a value is already referenced by the
+// base's (file) value at the same pointer, the value is passed to the plugin unchanged, as on
+// main, and a warning is returned. An unset variable the overlay introduced still fails with
+// agentconfig.ErrEnvMissing; forbidden names always fail with agentconfig.ErrEnvForbidden.
+func resolveEnv(declared, base agentconfig.Config, lookup func(string) (string, bool)) (agentconfig.Config, []agentconfig.FieldError, error) {
+	type literal struct{ plugin, key, value string }
+	var keep []literal
+	var warnings []agentconfig.FieldError
+	work := declared
+	copied := map[string]bool{} // plugins whose Config was copied into work
+	for _, name := range slices.Sorted(maps.Keys(declared.Plugins)) {
+		p := declared.Plugins[name]
+		if p == nil {
+			continue
+		}
+		for _, key := range slices.Sorted(maps.Keys(p.Config)) {
+			value := p.Config[key]
+			names := agentconfig.EnvRefs(value)
+			if len(names) == 0 || slices.ContainsFunc(names, agentconfig.IsForbiddenEnvName) {
+				continue
+			}
+			var missing []string
+			for _, n := range names {
+				if _, ok := lookup(n); !ok {
+					missing = append(missing, n)
+				}
+			}
+			if len(missing) == 0 {
+				continue
+			}
+			fileRefs := agentconfig.EnvRefs(basePluginConfigValue(base, name, key))
+			if slices.ContainsFunc(missing, func(n string) bool { return !slices.Contains(fileRefs, n) }) {
+				continue // overlay-introduced: ResolveEnv reports env-missing
+			}
+			if !copied[name] {
+				if len(copied) == 0 {
+					work.Plugins = maps.Clone(declared.Plugins)
+				}
+				cp := *p
+				cp.Config = maps.Clone(p.Config)
+				work.Plugins[name] = &cp
+				copied[name] = true
+			}
+			delete(work.Plugins[name].Config, key)
+			keep = append(keep, literal{name, key, value})
+			warnings = append(warnings, agentconfig.FieldError{
+				Path:    agentconfig.Pointer("plugins", name, "config", key),
+				Code:    agentconfig.FieldCodeEnvMissing,
+				Message: fmt.Sprintf("environment variable %s is not set; the value is passed to the plugin unchanged", strings.Join(missing, ", ")),
+			})
+		}
+	}
+	resolved, err := agentconfig.ResolveEnv(work, lookup)
+	if err != nil {
+		return agentconfig.Config{}, nil, err
+	}
+	for _, l := range keep {
+		p := resolved.Plugins[l.plugin]
+		if p.Config == nil {
+			p.Config = map[string]string{}
+		}
+		p.Config[l.key] = l.value
+	}
+	return resolved, warnings, nil
+}
+
+func basePluginConfigValue(base agentconfig.Config, plugin, key string) string {
+	if p := base.Plugins[plugin]; p != nil {
+		return p.Config[key]
+	}
+	return ""
+}
+
 // toRuntime converts a merged, env-resolved declared config into the runtime structs.
 // Disabled plugins and plugins named in skip (R34) are dropped: they get no cron, no download
 // and no run state, but they stay in the declared form and in reports.

cmd/reconciler.go

@@ -9,6 +9,7 @@ import (
 	"errors"
 	"fmt"
 	"math/rand"
+	"os"
 	"slices"
 	"strings"
 	"sync"
@@ -181,6 +182,8 @@ type reconciler struct {
 
 	// newRemote builds the remote client from a base (a test seam).
 	newRemote func(agentconfig.Config) remoteAPI
+	// lookupEnv resolves ${env:NAME} placeholders (a test seam).
+	lookupEnv func(string) (string, bool)
 	// pluginLib reads the agent library version of a prefetched plugin source (R76);
 	// nil leaves the plugins report empty.
 	pluginLib pluginLibFunc
@@ -225,6 +228,7 @@ func newReconciler(cmd *cobra.Command, configPath string, store *agentstate.Stor
 		fileEvents: make(chan struct{}, 1),
 		runFailed:  make(chan *candidate, 1),
 		debounce:   500 * time.Millisecond,
+		lookupEnv:  os.LookupEnv,
 		now:        time.Now,
 		loggedOnce: map[string]bool{},
 		newRemote:  newSDKRemote,
@@ -560,7 +564,22 @@ func (rc *reconciler) prepare(ctx context.Context, base *baseSnapshot, ov *agent
 		return nil, rejected(agentconfig.ReasonInvalidConfig, errs)
 	}
 
-	runtime, err := toRuntime(declared, part.skip)
+	resolved, envWarnings, err := resolveEnv(declared, base.declared, rc.lookupEnv)
+	switch {
+	case errors.Is(err, agentconfig.ErrEnvForbidden):
+		return nil, rejected(agentconfig.ReasonForbiddenChanges, err)
+	case errors.Is(err, agentconfig.ErrEnvMissing):
+		return nil, failed(agentconfig.ReasonEnvMissing, err)
+	case err != nil:
+		return nil, failed(agentconfig.ReasonInternal, err)
+	}
+	for _, w := range envWarnings {
+		if rc.logOnce("env-missing\x00" + w.Path + "\x00" + w.Message) {
+			rc.logWarnings([]agentconfig.FieldError{w})
+		}
+	}
+
+	runtime, err := toRuntime(resolved, part.skip)
 	if err != nil {
 		return nil, failed(agentconfig.ReasonInvalidConfig, err)
 	}
@@ -592,7 +611,7 @@ func (rc *reconciler) prepare(ctx context.Context, base *baseSnapshot, ov *agent
 		runtime:  runtime,
 		digest:   digest,
 		identity: candidateIdentity(declared),
-		warnings: append([]agentconfig.FieldError{}, part.warnings...),
+		warnings: append(append([]agentconfig.FieldError{}, part.warnings...), envWarnings...),
 		plugins:  plugins,
 	}, nil
 }

cmd/remote_test.go

@@ -167,6 +167,7 @@ func (h *remoteHarness) newReconciler() *reconciler {
 	rc := newReconciler(AgentCmd(), h.path, agentstate.Open(filepath.Join(h.dir, "state"), nil), h.pf, nil)
 	rc.newRemote = func(agentconfig.Config) remoteAPI { return h.remote }
 	rc.now = h.clock.Now
+	rc.lookupEnv = func(string) (string, bool) { return "", false }
 	return rc
 }
 
@@ -208,6 +209,7 @@ func TestStartupReport_RedactsAndDescribes(t *testing.T) {
       org: "${env:GITHUB_ORG}"
       endpoint: https://bot:hunter2@git.example
 `)
+	h.rc.lookupEnv = func(n string) (string, bool) { return "acme", n == "GITHUB_ORG" }
 	h.remote.publish(0, `{}`)
 	mustStartup(t, h.rc)
 
@@ -254,6 +256,7 @@ func TestStartupReport_RedactsAndDescribes(t *testing.T) {
 	}
 	t.Setenv("CCF_PLUGINS_GITHUB_CONFIG_TOKEN", "rotated")
 	rotated := h.newReconciler()
+	rotated.lookupEnv = h.rc.lookupEnv
 	if active := mustStartup(t, rotated); active.digest != r.EffectiveDigest {
 		t.Fatalf("digest changed when the env value rotated: %s vs %s", active.digest, r.EffectiveDigest)
 	}
@@ -823,6 +826,108 @@ func TestStartupLadder(t *testing.T) {
 	})
 }
 
+func TestEnvPlaceholders(t *testing.T) {
+	// ${env:} is only resolved in plugins.*.config (R24): in the file's policy_data it is a
+	// literal passed through unchanged with a warning (R34, as on main), and an overlay using
+	// it there is rejected.
+	lenient := newRemoteHarness(t, remoteConfig("apply_all", "")+`
+    policy_data:
+      url: "${env:NOT_RESOLVED}"
+`)
+	started, err := lenient.rc.startup(context.Background())
+	if err != nil {
+		t.Fatalf("a file policy_data placeholder must not be fatal: %v", err)
+	}
+	if got := started.runtime.Plugins["ssh"].PolicyData["url"]; got != "${env:NOT_RESOLVED}" {
+		t.Fatalf("policy_data must be passed through unchanged, got %v", got)
+	}
+	if r := lenient.remote.lastReport(t); len(r.Warnings) != 1 || r.Warnings[0].Code != agentconfig.FieldCodeEnvLocation {
+		t.Fatalf("expected one env-location warning, got %+v", r.Warnings)
+	}
+
+	h := newRemoteHarness(t, remoteConfig("apply_all", ""))
+	env := map[string]string{"HOST": "db.internal", "PORT": "5432"}
+	h.rc.lookupEnv = func(n string) (string, bool) { v, ok := env[n]; return v, ok }
+	h.remote.publish(1, `{"plugins":{"ssh":{"policy_data":{"url":"${env:HOST}"}}}}`)
+	mustStartup(t, h.rc)
+	if r := h.remote.lastReport(t); r.Status != agentconfig.StatusRejected || r.Reason != agentconfig.ReasonInvalidConfig {
+		t.Fatalf("expected an overlay policy_data placeholder to be rejected, got %s/%s", r.Status, r.Reason)
+	}
+
+	h.remote.publish(2, `{"plugins":{"ssh":{"config":{"host":"${env:HOST}","dsn":"pg://${env:HOST}:${env:PORT}/db"}}}}`)
+	active := h.poll(t)
+	cfg := active.runtime.Plugins["ssh"].Config
+	if cfg["host"] != "db.internal" || cfg["dsn"] != "pg://db.internal:5432/db" {
+		t.Fatalf("placeholders not resolved whole/embedded: %#v", cfg)
+	}
+	var eff agentconfig.Config
+	if err := json.Unmarshal(h.remote.lastReport(t).Effective, &eff); err != nil {
+		t.Fatal(err)
+	}
+	reported := eff.Plugins["ssh"].Config
+	if reported["host"] != "${env:HOST}" {
+		t.Fatalf("the report must carry the unresolved placeholder, got %#v", reported)
+	}
+	// Literal text mixed with a placeholder under a secret-like key is masked; the API's
+	// agentconfig redaction is the source of truth.
+	if reported["dsn"] != agentconfig.MaskedValue {
+		t.Fatalf("a dsn mixing literal text and placeholders must be masked, got %#v", reported)
+	}
+	digest := active.digest
+	env["HOST"] = "rotated"
+	restarted := h.newReconciler()
+	restarted.lookupEnv = h.rc.lookupEnv
+	if again := mustStartup(t, restarted); again.digest != digest || again.overlay == nil {
+		t.Fatal("the digest must not change when an env value changes")
+	}
+
+	delete(env, "PORT")
+	h.remote.publish(3, `{"plugins":{"ssh":{"config":{"host":"${env:HOST}","dsn":"pg://${env:PORT}"}}}}`)
+	h.rc.lookupEnv = func(n string) (string, bool) { v, ok := env[n]; return v, ok }
+	h.poll(t)
+	r := h.remote.lastReport(t)
+	if r.Status != agentconfig.StatusFailed || r.Reason != agentconfig.ReasonEnvMissing {
+		t.Fatalf("expected failed/env-missing, got %s/%s", r.Status, r.Reason)
+	}
+	if !strings.Contains(*r.Error, "PORT") || strings.Contains(*r.Error, "rotated") {
+		t.Fatalf("the error must name the variable, never values: %q", *r.Error)
+	}
+}
+
+// TestEnvPlaceholders_FileOriginUnsetIsWarning pins R60: an unset variable the FILE references
+// is a warning and the literal reaches the plugin unchanged (as on main); an unset variable the
+// overlay introduces still fails with failed/env-missing.
+func TestEnvPlaceholders_FileOriginUnsetIsWarning(t *testing.T) {
+	content := strings.Replace(remoteConfig("apply_all", ""), "token: t0ken", "token: \"${env:UNSET_TOKEN}\"\n      dsn: \"pg://${env:DB_HOST}/x\"", 1)
+	h := newRemoteHarness(t, content)
+	env := map[string]string{"DB_HOST": "db.internal"}
+	h.rc.lookupEnv = func(n string) (string, bool) { v, ok := env[n]; return v, ok }
+	h.remote.publish(1, `{}`)
+
+	active := mustStartup(t, h.rc)
+	cfg := active.runtime.Plugins["ssh"].Config
+	if cfg["token"] != "${env:UNSET_TOKEN}" || cfg["dsn"] != "pg://db.internal/x" {
+		t.Fatalf("expected the unset literal unchanged and the set one resolved, got %#v", cfg)
+	}
+	r := h.remote.lastReport(t)
+	if r.Status != agentconfig.StatusApplied || len(r.Warnings) != 1 || r.Warnings[0].Path != "/plugins/ssh/config/token" || r.Warnings[0].Code != agentconfig.FieldCodeEnvMissing {
+		t.Fatalf("expected applied with one env-missing warning, got %s %+v", r.Status, r.Warnings)
+	}
+
+	h.remote.publish(2, `{"plugins":{"ssh":{"config":{"extra":"${env:NEW_UNSET}"}}}}`)
+	h.poll(t)
+	if r := h.remote.lastReport(t); r.Status != agentconfig.StatusFailed || r.Reason != agentconfig.ReasonEnvMissing {
+		t.Fatalf("an overlay-introduced unset variable must fail with env-missing, got %s/%s", r.Status, r.Reason)
+	}
+
+	// Per (pointer, variable): the overlay rewrites the value but the variable is the file's.
+	h.remote.publish(3, `{"plugins":{"ssh":{"config":{"token":"x-${env:UNSET_TOKEN}"}}}}`)
+	next := h.poll(t)
+	if got := next.runtime.Plugins["ssh"].Config["token"]; got != "x-${env:UNSET_TOKEN}" || next.appliedRevision() == nil || *next.appliedRevision() != 3 {
+		t.Fatalf("expected revision 3 applied with the literal unchanged, got %q", got)
+	}
+}
+
 func TestOneShot_FetchApplyReportRun(t *testing.T) {
 	h := newRemoteHarness(t, strings.Replace(remoteConfig("apply_safe", ""), "daemon: true", "daemon: false", 1))
 	h.remote.publish(1, `{"plugins":{"ssh":{"schedule":"*/5 * * * *"}}}`)