Skip to content

Commit dfa636f

Browse files
scotwellsclaude
andcommitted
fix(plugin): run install-time manifest probe with a scrubbed environment
Installing a plugin runs the freshly downloaded binary once with --plugin-manifest to read its metadata. That probe now executes with a minimal, built-from-scratch environment (PATH, HOME, and temp-dir hints, plus the Windows equivalents) instead of inheriting the user's full environment, so Datum credentials, helper hooks, and cloud tokens are not exposed to a third-party binary during install. Using an allow-list rather than a deny-list means newly added sensitive variables stay excluded by default. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 83582d0 commit dfa636f

2 files changed

Lines changed: 125 additions & 0 deletions

File tree

internal/cmd/plugin/helpers.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,14 +365,50 @@ func downloadAndVerifyURI(ctx context.Context, uri, sha256hex string) ([]byte, e
365365
return archiveBytes, nil
366366
}
367367

368+
// minimalManifestEnv returns a minimal, scrubbed environment used when executing
369+
// a freshly-downloaded plugin binary to read its manifest. It is built as an
370+
// allow-list from scratch (rather than scrubbing the inherited environment) so
371+
// that sensitive variables — Datum credentials, credential-helper config, cloud
372+
// provider tokens, etc. — are never exposed to third-party code by default, and
373+
// so newly-introduced sensitive vars do not leak as they are added over time.
374+
//
375+
// Only variables a binary plausibly needs to start up and emit a manifest are
376+
// forwarded: a PATH to resolve any runtime/loader, a home directory, and the
377+
// platform temp-dir hints. Each is copied only if present in the host
378+
// environment.
379+
func minimalManifestEnv() []string {
380+
// allow is the set of variable names that may be forwarded. Keep this list
381+
// conservative; anything not named here is intentionally dropped.
382+
allow := []string{"PATH", "HOME", "TMPDIR", "TMP", "TEMP"}
383+
if runtime.GOOS == "windows" {
384+
// Windows resolves the user profile and system DLLs via these.
385+
allow = append(allow, "USERPROFILE", "SYSTEMROOT", "SystemRoot", "windir")
386+
}
387+
388+
env := make([]string, 0, len(allow))
389+
for _, name := range allow {
390+
if v, ok := os.LookupEnv(name); ok {
391+
env = append(env, name+"="+v)
392+
}
393+
}
394+
return env
395+
}
396+
368397
// readPluginManifest writes binaryPath to a temp file (if needed), runs it with
369398
// --plugin-manifest, and parses the JSON output. Returns nil, nil if the binary
370399
// exits non-zero or produces no valid JSON.
400+
//
401+
// SECURITY: this executes the (third-party, freshly-downloaded) plugin binary as
402+
// part of installation. Install is an explicit user action, but to shrink the
403+
// blast radius of running untrusted code we run it with a minimal, scrubbed
404+
// environment (see minimalManifestEnv) rather than the user's full environment,
405+
// so credentials and tokens are not exposed to the binary.
371406
func readPluginManifest(binaryPath string) (*pluginstore.PluginManifest, error) {
372407
ctx, cancel := context.WithTimeout(context.Background(), manifestReadTimeout)
373408
defer cancel()
374409

375410
cmd := exec.CommandContext(ctx, binaryPath, "--plugin-manifest")
411+
cmd.Env = minimalManifestEnv()
376412
out, err := cmd.Output()
377413
if err != nil {
378414
// Non-zero exit — treat as "no manifest".
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
package plugin
2+
3+
import (
4+
"runtime"
5+
"strings"
6+
"testing"
7+
)
8+
9+
// envHas reports whether the KEY=VALUE slice contains an entry for name.
10+
func envHas(env []string, name string) bool {
11+
prefix := name + "="
12+
for _, e := range env {
13+
if strings.HasPrefix(e, prefix) {
14+
return true
15+
}
16+
}
17+
return false
18+
}
19+
20+
// TestMinimalManifestEnv_includesEssentials verifies that the scrubbed
21+
// environment still forwards the variables a plugin binary plausibly needs to
22+
// start up and emit its manifest: PATH, a home directory, and the platform
23+
// temp-dir hint.
24+
func TestMinimalManifestEnv_includesEssentials(t *testing.T) {
25+
t.Setenv("PATH", "/usr/bin")
26+
if runtime.GOOS == "windows" {
27+
t.Setenv("USERPROFILE", `C:\Users\test`)
28+
t.Setenv("TEMP", `C:\Temp`)
29+
} else {
30+
t.Setenv("HOME", "/home/test")
31+
t.Setenv("TMPDIR", "/tmp")
32+
}
33+
34+
env := minimalManifestEnv()
35+
36+
if !envHas(env, "PATH") {
37+
t.Errorf("minimalManifestEnv() missing PATH; got %v", env)
38+
}
39+
40+
if runtime.GOOS == "windows" {
41+
if !envHas(env, "USERPROFILE") {
42+
t.Errorf("minimalManifestEnv() missing USERPROFILE on windows; got %v", env)
43+
}
44+
if !envHas(env, "TEMP") {
45+
t.Errorf("minimalManifestEnv() missing TEMP on windows; got %v", env)
46+
}
47+
} else {
48+
if !envHas(env, "HOME") {
49+
t.Errorf("minimalManifestEnv() missing HOME; got %v", env)
50+
}
51+
if !envHas(env, "TMPDIR") {
52+
t.Errorf("minimalManifestEnv() missing TMPDIR; got %v", env)
53+
}
54+
}
55+
}
56+
57+
// TestMinimalManifestEnv_excludesSensitive verifies that credential and token
58+
// variables present in the host environment are not forwarded to the plugin
59+
// binary.
60+
func TestMinimalManifestEnv_excludesSensitive(t *testing.T) {
61+
t.Setenv("DATUM_CREDENTIALS_HELPER", "secret-helper")
62+
t.Setenv("DATUM_TOKEN", "super-secret-token")
63+
t.Setenv("AWS_SECRET_ACCESS_KEY", "fake-aws-secret")
64+
65+
env := minimalManifestEnv()
66+
67+
for _, name := range []string{"DATUM_CREDENTIALS_HELPER", "DATUM_TOKEN", "AWS_SECRET_ACCESS_KEY"} {
68+
if envHas(env, name) {
69+
t.Errorf("minimalManifestEnv() leaked sensitive var %s; got %v", name, env)
70+
}
71+
}
72+
}
73+
74+
// TestMinimalManifestEnv_isAllowList verifies the environment is built as an
75+
// allow-list: an arbitrary unrelated variable set in the host environment does
76+
// not appear in the result.
77+
func TestMinimalManifestEnv_isAllowList(t *testing.T) {
78+
t.Setenv("FOO", "bar")
79+
t.Setenv("SOME_UNRELATED_VAR", "value")
80+
81+
env := minimalManifestEnv()
82+
83+
if envHas(env, "FOO") {
84+
t.Errorf("minimalManifestEnv() forwarded unrelated var FOO; got %v", env)
85+
}
86+
if envHas(env, "SOME_UNRELATED_VAR") {
87+
t.Errorf("minimalManifestEnv() forwarded unrelated var SOME_UNRELATED_VAR; got %v", env)
88+
}
89+
}

0 commit comments

Comments
 (0)