Skip to content

fix(pilotctl): pass admin token via env to avoid /proc/cmdline leak (PILOT-290) #195

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
matthew-pilot wants to merge 1 commit into
base: main
Choose a base branch
from
+33 −15
Open

fix(pilotctl): pass admin token via env to avoid /proc/cmdline leak (PILOT-290) #195

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions cmd/daemon/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,11 @@ func main() {
logLevel := flag.String("log-level", "info", "log level (debug, info, warn, error)")
logFormat := flag.String("log-format", "text", "log format (text, json)")
flag.Parse()
if *adminToken == "" {
if v := os.Getenv("PILOT_ADMIN_TOKEN"); v != "" {
*adminToken = v
}
}

if *showVersion {
fmt.Println(version)
Expand Down
27 changes: 18 additions & 9 deletions cmd/pilotctl/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2076,7 +2076,7 @@ func gatewayBinaryPath() string {
// unset. This keeps existing pilotctl invocations working unchanged —
// the only difference is that the daemon runs in a separate
// `pilot-daemon` process rather than re-execing pilotctl.
func buildDaemonArgs(args []string) (daemonArgs []string, socketPath string) {
func buildDaemonArgs(args []string) (daemonArgs []string, socketPath string, adminToken string) {
flags, _ := parseFlags(args)

cfg := loadConfig()
Expand Down Expand Up @@ -2134,7 +2134,7 @@ func buildDaemonArgs(args []string) (daemonArgs []string, socketPath string) {
webhookURL = w
}
}
adminToken := flagString(flags, "admin-token", "")
adminToken = flagString(flags, "admin-token", "")
if adminToken == "" {
if a, ok := cfg["admin_token"].(string); ok {
adminToken = a
Expand Down Expand Up @@ -2177,16 +2177,15 @@ func buildDaemonArgs(args []string) (daemonArgs []string, socketPath string) {
if webhookURL != "" {
daemonArgs = append(daemonArgs, "--webhook", webhookURL)
}
if adminToken != "" {
daemonArgs = append(daemonArgs, "--admin-token", adminToken)
}
// adminToken is passed via PILOT_ADMIN_TOKEN env var to avoid
// leaking the secret in /proc/<pid>/cmdline (PILOT-290).
if networks != "" {
daemonArgs = append(daemonArgs, "--networks", networks)
}
if trustAutoApprove {
daemonArgs = append(daemonArgs, "--trust-auto-approve")
}
return daemonArgs, socketPath
return daemonArgs, socketPath, adminToken
}

// launchdAgentLabels enumerates known launchd labels for the daemon.
Expand Down Expand Up @@ -2288,7 +2287,7 @@ func cmdDaemonStart(args []string) {
os.Remove(pidFilePath())
}

daemonArgs, socketPath := buildDaemonArgs(args)
daemonArgs, socketPath, adminToken := buildDaemonArgs(args)

// Clean up stale socket
if _, err := os.Stat(socketPath); err == nil {
Expand All @@ -2311,9 +2310,14 @@ func cmdDaemonStart(args []string) {
// or shell wrappers.
if flagBool(flags, "foreground") {
// syscall.Exec needs argv[0] to be the binary name. Pass the
// full env unchanged.
// full env. Inject PILOT_ADMIN_TOKEN so the daemon doesn't
// need the token on its argv (PILOT-290).
execArgs := append([]string{daemonBin}, daemonArgs...)
if err := syscall.Exec(daemonBin, execArgs, os.Environ()); err != nil {
env := os.Environ()
if adminToken != "" {
env = append(env, "PILOT_ADMIN_TOKEN="+adminToken)
}
if err := syscall.Exec(daemonBin, execArgs, env); err != nil {
fatalCode("internal", "exec %s: %v", daemonBin, err)
}
return
Expand All @@ -2338,6 +2342,11 @@ func cmdDaemonStart(args []string) {
proc.Stdout = logFile
proc.Stderr = logFile
proc.SysProcAttr = &syscall.SysProcAttr{Setsid: true}
// Pass admin token via env, not argv, to avoid leaking in
// /proc/<pid>/cmdline (PILOT-290).
if adminToken != "" {
proc.Env = append(os.Environ(), "PILOT_ADMIN_TOKEN="+adminToken)
}

if err := proc.Start(); err != nil {
fatalCode("internal", "start daemon: %v", err)
Expand Down
16 changes: 10 additions & 6 deletions cmd/pilotctl/zz_lifecycle_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ func TestSkillsHomeRel(t *testing.T) {
// sensible default and no admin/email/public bits added.
func TestBuildDaemonArgsDefaults(t *testing.T) {
withTempHomeFull(t)
args, sock := buildDaemonArgs([]string{})
args, sock, _ := buildDaemonArgs([]string{})
if sock == "" {
t.Fatal("socket path should be set")
}
Expand All @@ -275,7 +275,8 @@ func TestBuildDaemonArgsDefaults(t *testing.T) {
}
}
// Optional bits MUST NOT be present without their flags.
for _, k := range []string{"--public", "--admin-token", "--webhook", "--networks", "--trust-auto-approve"} {
// --admin-token is no longer passed via argv (PILOT-290).
for _, k := range []string{"--public", "--webhook", "--networks", "--trust-auto-approve"} {
if strings.Contains(got, k) {
t.Errorf("unexpected default flag %s in %v", k, args)
}
Expand All @@ -284,7 +285,7 @@ func TestBuildDaemonArgsDefaults(t *testing.T) {

func TestBuildDaemonArgsHonorsFlags(t *testing.T) {
withTempHomeFull(t)
args, _ := buildDaemonArgs([]string{
args, _, adminTok := buildDaemonArgs([]string{
"--registry", "r.x:9000",
"--beacon", "b.x:9001",
"--listen", "1.2.3.4:4000",
Expand All @@ -309,7 +310,6 @@ func TestBuildDaemonArgsHonorsFlags(t *testing.T) {
"--hostname my-host": "hostname flag",
"--public": "public flag",
"--webhook https://hook.example": "webhook flag",
"--admin-token tok": "admin-token flag",
"--networks 1,2,3": "networks flag",
"--trust-auto-approve": "trust auto-approve flag",
"--log-level debug": "log-level flag",
Expand All @@ -320,13 +320,17 @@ func TestBuildDaemonArgsHonorsFlags(t *testing.T) {
t.Errorf("%s: expected %q in %v", label, fragment, args)
}
}
// Admin token is now returned separately, not passed via argv (PILOT-290).
if adminTok != "tok" {
t.Errorf("admin-token flag: expected %q, got %q", "tok", adminTok)
}
}

// TestBuildDaemonArgsOwnerAlias verifies the legacy `-owner` flag is
// honored as an alias for `--email` when no email is given explicitly.
func TestBuildDaemonArgsOwnerAlias(t *testing.T) {
withTempHomeFull(t)
args, _ := buildDaemonArgs([]string{"-owner", "legacy@example.com"})
args, _, _ := buildDaemonArgs([]string{"-owner", "legacy@example.com"})
if !strings.Contains(strings.Join(args, " "), "--email legacy@example.com") {
t.Errorf("expected --email passthrough from -owner: %v", args)
}
Expand All @@ -346,7 +350,7 @@ func TestBuildDaemonArgsConfigFallback(t *testing.T) {
if err := saveConfig(cfg); err != nil {
t.Fatalf("saveConfig: %v", err)
}
args, _ := buildDaemonArgs([]string{})
args, _, _ := buildDaemonArgs([]string{})
got := strings.Join(args, " ")
for _, want := range []string{
"--registry cfg.example:9000",
Expand Down
Loading