amadan

carlos / platform Public

Allowlist the tenant child environment instead of a CARLOS_ denylist

Human

Prompt

Spin up a subagent to do the "allowlist childEnv" work. When you have a fix, push it to amadan with a discussion.

From this commit's own message. The prompt ledger has no entries for it — which is what a squashed, rebased or amended commit looks like, and what every commit looks like in a repo without a ledger.

Summary

childEnv composed each tenant instance's environment by copying the edge agent's own environment minus CARLOS_* (a denylist). Any non-CARLOS_ secret staged in the agent env (an inline AWS key, DATABASE_URL, SMTP cred) therefore flowed into every app's /proc/self/environ. Replace the denylist with an explicit allowlist of benign base vars (PATH, HOME, TMPDIR, TZ, LANG, LC_*, USER, LOGNAME, SHELL, TERM, SSL_CERT_FILE/DIR); everything else, incl. all AWS_* and CARLOS_*, is dropped. fileEnv layering, dedup and ordering are unchanged. Adds a regression test proving planted secrets don't reach a child.
Agent notes
Behavioural change worth release-noting: AWS_REGION and any box-wide var an operator set in host.env for all apps no longer propagate; apps declare their own config in their env file. This is finding #2 of docs/security/2026-10-06-access-assessment.md; it is defence-in-depth — on a FLAT box the tenant can read /etc/carlos/credentials off disk anyway, so the nested-layout migration is the real fix.
claude pushed by james@keymail.dev 7e034465e9940393387dd1503209cbc7d7dda71e parent 7143c7e
2 files changed, +135 −15
  • internal/activator/activator_test.go +61 −5
  • internal/activator/backend_exec.go +74 −10

CI passed — run details

diff --git a/internal/activator/activator_test.go b/internal/activator/activator_test.go
index 9faa04c4f..386ccf47c 100644
--- a/internal/activator/activator_test.go
+++ b/internal/activator/activator_test.go
@@ -4507,13 +4507,16 @@ func stageLiveUnderAccount(t *testing.T, optDir, host, acct, ver, app string) {
}
}
-// childEnv strips every CARLOS_* var from the inherited environment (the
+// childEnv never inherits a CARLOS_* var from the agent's environment (the
// edge's own config must never leak into a child) and then layers the env
-// file on top — including that file's own CARLOS_* entries.
+// file on top — including that file's own CARLOS_* entries. CARLOS_* names
+// fail the allowlist like any other non-base name, so they are dropped from
+// the inherited environment exactly as the old denylist guaranteed.
func TestChildEnvStripsCarlosVars(t *testing.T) {
t.Setenv("CARLOS_HIBERNATE_BUCKET", "leak-me-not")
t.Setenv("CARLOS_TEST_HARMLESS", "also-me")
- t.Setenv("UNRELATED_VAR", "keep")
+ t.Setenv("PATH", "/usr/bin:/bin") // an allowlisted base var — inherited
+ t.Setenv("UNRELATED_VAR", "drop-me") // not allowlisted — must NOT be inherited
env := childEnv(map[string]string{"CARLOS_ADMIN_TOKEN": "tok", "FOO": "bar"})
got := map[string]string{}
for _, kv := range env {
@@ -4527,14 +4530,58 @@ func TestChildEnvStripsCarlosVars(t *testing.T) {
if _, leaked := got["CARLOS_TEST_HARMLESS"]; leaked {
t.Error("inherited CARLOS_TEST_HARMLESS leaked into the child env")
}
- if got["UNRELATED_VAR"] != "keep" {
- t.Error("non-CARLOS inherited var was stripped")
+ if got["PATH"] != "/usr/bin:/bin" {
+ t.Errorf("allowlisted inherited var PATH = %q, want it inherited", got["PATH"])
+ }
+ if _, kept := got["UNRELATED_VAR"]; kept {
+ t.Error("a non-allowlisted inherited var leaked into the child env — childEnv is an allowlist now")
}
if got["CARLOS_ADMIN_TOKEN"] != "tok" || got["FOO"] != "bar" {
t.Errorf("env file vars not layered: %v", got)
}
}
+// TestChildEnvAllowlistDropsPlantedSecrets is the finding #2 regression guard:
+// a NON-CARLOS_ secret planted in the agent's own process environment (the
+// shape of an inline AWS key, a DATABASE_URL, an arbitrary secret) must never
+// reach a tenant child, while a benign allowlisted base var (PATH) does, and a
+// fileEnv entry still overrides a same-named allowlisted parent var.
+func TestChildEnvAllowlistDropsPlantedSecrets(t *testing.T) {
+ t.Setenv("AWS_SECRET_ACCESS_KEY", "AKIA-super-secret")
+ t.Setenv("AWS_ACCESS_KEY_ID", "AKIA-id")
+ t.Setenv("AWS_SHARED_CREDENTIALS_FILE", "/etc/carlos/credentials")
+ t.Setenv("AWS_REGION", "eu-central-1")
+ t.Setenv("DATABASE_URL", "postgres://u:p@h/db")
+ t.Setenv("TOTALLY_SECRET", "do-not-leak")
+ t.Setenv("PATH", "/host/bin")
+
+ // fileEnv is the tenant's own declared config. It carries a PATH that must
+ // win over the inherited one (last-value-wins, fileEnv on top), proving the
+ // app's real config channel is untouched by the allowlist change.
+ env := childEnv(map[string]string{"PATH": "/tenant/bin", "APP_SETTING": "1"})
+ got := map[string]string{}
+ for _, kv := range env {
+ if k, v, ok := splitEnv(kv); ok {
+ got[k] = v
+ }
+ }
+
+ for _, planted := range []string{
+ "AWS_SECRET_ACCESS_KEY", "AWS_ACCESS_KEY_ID", "AWS_SHARED_CREDENTIALS_FILE",
+ "AWS_REGION", "DATABASE_URL", "TOTALLY_SECRET",
+ } {
+ if v, leaked := got[planted]; leaked {
+ t.Errorf("planted non-CARLOS_ secret %s=%q leaked into the child env — the allowlist must drop it", planted, v)
+ }
+ }
+ if got["PATH"] != "/tenant/bin" {
+ t.Errorf("PATH = %q, want the tenant's fileEnv value to win over the inherited base var", got["PATH"])
+ }
+ if got["APP_SETTING"] != "1" {
+ t.Errorf("fileEnv APP_SETTING not layered: %v", got)
+ }
+}
+
func splitEnv(kv string) (k, v string, ok bool) {
for i := 0; i < len(kv); i++ {
if kv[i] == '=' {
@@ -5875,6 +5922,15 @@ func TestMain(m *testing.M) {
fakeAppMain()
return
}
+ // The test binary doubles as the fake instance app (fakeAppMain above), and
+ // Start now hands a spawned child only childEnv's allowlisted environment.
+ // The fake-app activation signals are set in the parent env by
+ // stageServingLive / the unhealthy test and must still reach the re-exec'd
+ // child, so widen the allowlist in THIS (test) process to carry them. A
+ // real app never needs these names; production's childEnvAllow is unchanged.
+ childEnvAllow["ACTIVATOR_FAKE_APP"] = true
+ childEnvAllow["ACTIVATOR_FAKE_APP_UNHEALTHY"] = true
+
// A worker's wake waits out its settle window unless the child reports
// ready, and no fake child does. Shrink it once for the whole package —
// the tests that are ABOUT the window set their own value and restore it
diff --git a/internal/activator/backend_exec.go b/internal/activator/backend_exec.go
index 60fbaa181..eb8a395e7 100644
--- a/internal/activator/backend_exec.go
+++ b/internal/activator/backend_exec.go
@@ -1183,13 +1183,72 @@ func (b *ExecBackend) instanceEnv(rt registry.Route) (map[string]string, error)
return mergeEnv(cfgEnv, ownEnv), nil
}
-// childEnv is the child's complete environment: the agent's own, minus
-// every CARLOS_* variable, with the instance's file values layered on top.
-// One entry per key, last value wins: os/exec deduplicates cmd.Env that
-// way, but the unit path hands this list to syscall.Exec, which passes
-// duplicates through, and a Go child then keeps the FIRST occurrence
-// (design §6.2, codex R1-16). Composing it deduplicated here keeps the
-// two spawn paths identical.
+// childEnvAllow is the exact-key ALLOWLIST of parent-process environment
+// variables an exec-launched tenant instance may inherit from the agent.
+// It replaced a CARLOS_-prefix DENYLIST (2026-10-06 access assessment,
+// finding #2): the agent's own process environment may stage a NON-CARLOS_
+// secret via /etc/carlos/host.env (an inline AWS_SECRET_ACCESS_KEY, a
+// DATABASE_URL, an SMTP credential, a SENTRY_DSN), and a denylist copies
+// every such name verbatim into every tenant's /proc/self/environ — leaked
+// deployment-bucket keys are control-plane takeover, and the inheritance
+// defeats the nested layout's own InaccessiblePaths=/etc/carlos because the
+// secret is already resident in the process. So only the benign, non-secret
+// base variables a process legitimately needs are inherited; every other
+// name — CARLOS_* included (those are injected per-instance elsewhere, e.g.
+// the per-wake sidecar token and the agent-owned control-channel vars), and
+// every unknown name — is dropped. The instance's own fileEnv is still
+// layered on top in childEnv and still wins conflicts, because that is the
+// app's real, declared configuration channel and the one place a tenant may
+// legitimately set anything it needs.
+//
+// Deliberate exclusions worth naming (finding #2): the whole AWS_* namespace,
+// including AWS_REGION (not a secret, but a tenant that needs a region
+// declares it in its own env file rather than inheriting the agent's) and
+// AWS_SHARED_CREDENTIALS_FILE (a mere path pointer, so inheriting it is inert
+// — a nested tenant cannot read /etc/carlos — but it signposts the agent's
+// credentials file to the tenant for no legitimate gain, so it is dropped).
+var childEnvAllow = map[string]bool{
+ "PATH": true, // command resolution for an app (or its subprocesses) that shells out
+ "HOME": true, // config/cache roots many runtimes and tools derive from it
+ "TMPDIR": true, // where the app writes temporary files
+ "TZ": true, // local time
+ "LANG": true, // default locale (the LC_* family is handled by prefix below)
+ "USER": true, // process identity; already knowable, not a secret
+ "LOGNAME": true, // USER's traditional companion — several tools read it instead of USER
+ "SHELL": true, // the shell an app spawns for subprocesses
+ "TERM": true, // terminal type; benign and non-secret
+ "SSL_CERT_FILE": true, // TLS trust store — a public CA bundle, a pointer not a secret
+ "SSL_CERT_DIR": true, // TLS trust store directory, likewise public
+}
+
+// childEnvAllowPrefixes allows whole variable families by prefix, for a
+// family too large to list key by key. LC_* is the locale family (LC_ALL,
+// LC_CTYPE, LC_TIME, LC_NUMERIC, …). Everything not matched here or in
+// childEnvAllow is an exact-key decision.
+var childEnvAllowPrefixes = []string{"LC_"}
+
+// childEnvAllowed reports whether a parent-process variable named k may be
+// inherited by a tenant child: an exact childEnvAllow entry, or a member of
+// an allowed family (childEnvAllowPrefixes).
+func childEnvAllowed(k string) bool {
+ if childEnvAllow[k] {
+ return true
+ }
+ for _, p := range childEnvAllowPrefixes {
+ if strings.HasPrefix(k, p) {
+ return true
+ }
+ }
+ return false
+}
+
+// childEnv is the child's complete environment: the allowlisted subset of the
+// agent's own process environment (childEnvAllow / childEnvAllowPrefixes),
+// with the instance's file values layered on top. One entry per key, last
+// value wins: os/exec deduplicates cmd.Env that way, but the unit path hands
+// this list to syscall.Exec, which passes duplicates through, and a Go child
+// then keeps the FIRST occurrence (design §6.2, codex R1-16). Composing it
+// deduplicated here keeps the two spawn paths identical.
func childEnv(fileEnv map[string]string) []string {
order := make([]string, 0, 32)
values := make(map[string]string, 32)
@@ -1200,13 +1259,18 @@ func childEnv(fileEnv map[string]string) []string {
values[k] = v
}
for _, kv := range os.Environ() {
- if strings.HasPrefix(kv, "CARLOS_") {
- continue
- }
k, v, ok := strings.Cut(kv, "=")
if !ok {
continue
}
+ // An ALLOWLIST, not a CARLOS_-prefix denylist (finding #2): only the
+ // benign base variables above are inherited; a non-CARLOS_ secret
+ // staged in the agent's own environment is dropped rather than handed
+ // to every tenant. CARLOS_* names fail the allowlist too, so they are
+ // still never inherited — exactly as the old denylist guaranteed.
+ if !childEnvAllowed(k) {
+ continue
+ }
set(k, v)
}
keys := make([]string, 0, len(fileEnv))