Summarize a command's env vars in the PIN dialog and Slack (v0.1.27)

convox.yml

@@ -30,7 +30,7 @@ services:
     # Pinned to an immutable release tag. scripts/bump-version.sh sets it to the new
     # version (:vX.Y.Z); the release workflow publishes that tag when the v* git tag is pushed.
     # :e327add is the v0.1.1 release (identical to :latest when this was pinned).
-    image: docker.io/docspringcom/rack-gateway:v0.1.26
+    image: docker.io/docspringcom/rack-gateway:v0.1.27
     command: ./scripts/start-gateway.sh
     environment:
       - PORT=8080
@@ -54,7 +54,7 @@ services:
 
   admin:
     # Uses same image as gateway service (kept in sync by scripts/bump-version.sh)
-    image: docker.io/docspringcom/rack-gateway:v0.1.26
+    image: docker.io/docspringcom/rack-gateway:v0.1.27
     command: echo "Use this service to run migrations, database admin tasks, etc."
     environment:
       - PORT=8080

docs/src/content/docs/concepts/audit-logging.mdx

@@ -331,8 +331,9 @@ denial.
 
 The env vars a command got (see [Env vars](/user-guide/cli/commands/#env-vars)) are recorded with it: `env_vars`,
 the names it got (only the ones the release sets reached it), or `all_env_vars: true` for every env var except
-`RACK_URL`; `env_read_only`, each read-only env var it got with the env var it took its value from; and
-`write: true` when it ran with `--write`, with the real values. `command` is the command as it was sent, without the
+`RACK_URL`; `env_vars_added`, the ones it asked for besides the app's `command_env_vars` (with `--env`, or in its
+`approved_deploy_commands` entry); `env_read_only`, each read-only env var it got with the env var it took its
+value from; and `write: true` when it ran with `--write`, with the real values. `command` is the command as it was sent, without the
 `sh` wrapper the gateway adds.
 
 A command run with a script (`--script`, see [Scripts](/user-guide/cli/commands/#scripts)) is recorded readably:

docs/src/content/docs/user-guide/cli/commands.mdx

@@ -380,11 +380,13 @@ rack-gateway run web "rake db:migrate" -a myapp --write \
   --justification "Run the migration for the new invoices table, which the deploy skipped"
 ```
 
-The PIN dialog lists the env vars the command gets, such as `Env vars: DATABASE_URL (from READ_ONLY_DATABASE_URL),
-PATH`, and `--dry-run` prints the list. The dialog warns in red when `--write` lets the command change data, when
-`--all-env-vars` gives it every env var in an app that limits them, and for every `exec` in such an app (see
-below). The touch prompt says when the command can change data. The audit log and the Slack notification record
-the env vars, and whether the command ran with `--write`.
+The PIN dialog says which env vars the command gets: how many of the app's `command_env_vars`, the ones it asked
+for and the read-only ones, such as `Env vars: the 98 in the app's command_env_vars, plus SENTRY_DSN; read-only
+DATABASE_URL (from READ_ONLY_DATABASE_URL)`. `--dry-run` prints the same line, and the Slack notification says the
+same. The dialog warns in red when `--write` lets the command change data, when `--all-env-vars` gives it every env
+var in an app that limits them, and for every `exec` in such an app (see below). The touch prompt says when the
+command can change data. The audit log records every env var name the command got, the ones it asked for, and
+whether it ran with `--write`.
 
 The gateway sends the rack the command inside a short `sh` wrapper, which starts it with `env -i` and only the env
 vars it gets. An env var the release doesn't set stays unset. The image needs `sh` and `env`. A command can't start

internal/cli/AGENTS.md

@@ -244,8 +244,9 @@ can't, and rely on the gateway's refusal).
 `--env`, `--all-env-vars` and `--write` (`convox_process_env.go`) travel as the `cmdenv.Request` headers, checked
 before anything starts (`cmdenv.CheckCommand` refuses an empty command or one starting with `NAME=value`). The CLI
 reads the app's `command_env_vars` and `command_env_read_only` (`readCommandSettings`, with the justification
-settings) to list the command's env vars in the PIN dialog summary (`settings.CommandEnv.Plan`, as the gateway
-decides them) and to warn: `envWarnings` go to `webauthn.Prompt.Warnings`, which the PIN dialog and the script
+settings) to describe the command's env vars in the PIN dialog summary (`settings.CommandEnv.Plan`, as the gateway
+decides them; `Plan.Summary` counts the app's list and names only the added and read-only ones, as Slack does) and
+to warn: `envWarnings` go to `webauthn.Prompt.Warnings`, which the PIN dialog and the script
 review dialog show in red (a JXA NSAlert, `macdialog`), and the terminal and `--dry-run` print as `WARNING:` lines.
 The touch prompt says "can change data" when `--write` gets a read-only env var's real value.
 

internal/cli/convox_process_env.go

@@ -63,7 +63,7 @@ func (p processCommand) envSummary() string {
 	if !ok {
 		return ""
 	}
-	return "Env vars: " + plan.Describe()
+	return "Env vars: " + plan.Summary()
 }
 
 // writtenVars are the app's read-only env vars the command gets the real values of (--write), sorted.

internal/cli/convox_process_env_test.go

@@ -75,12 +75,14 @@ func limitedProcess(action string, req cmdenv.Request) processCommand {
 // environment. The touch says when it can change data.
 func TestCommandPromptDescribesTheCommandsEnvVars(t *testing.T) {
 	prompt := commandPrompt(limitedProcess("run", cmdenv.Request{Env: []string{"STRIPE_KEY"}}))
-	require.Contains(t, prompt.Summary, "Env vars: DATABASE_URL (from READ_ONLY_DATABASE_URL), PATH, STRIPE_KEY")
+	require.Contains(t, prompt.Summary, "Env vars: the 2 in the app's command_env_vars, plus STRIPE_KEY; read-only "+
+		"DATABASE_URL (from READ_ONLY_DATABASE_URL)")
 	require.Empty(t, prompt.Warnings)
 	require.Equal(t, "run a command on app myapp (rack us)", prompt.Touch)
 
 	prompt = commandPrompt(limitedProcess("run", cmdenv.Request{Write: true}))
-	require.Contains(t, prompt.Summary, "Env vars: DATABASE_URL, PATH")
+	require.Contains(t, prompt.Summary, "Env vars: the 2 in the app's command_env_vars")
+	require.NotContains(t, prompt.Summary, "; read-only")
 	require.Equal(t, []string{
 		"This command can change data: --write gives it the real DATABASE_URL, not the read-only one.",
 	}, prompt.Warnings)
@@ -119,7 +121,8 @@ func TestProcessCommandDryRunShowsTheEnvVars(t *testing.T) {
 	var out bytes.Buffer
 	cmd.SetOut(&out)
 	require.NoError(t, runProcessCommand(cmd, []string{"web", "rake", "db:migrate"}, "run", nil))
-	require.Contains(t, out.String(), "Env vars: DATABASE_URL, PATH")
+	require.Contains(t, out.String(), "Env vars: the 2 in the app's command_env_vars\n")
+	require.NotContains(t, out.String(), "; read-only")
 	require.Contains(t, out.String(), "WARNING: This command can change data: --write gives it the real DATABASE_URL")
 	require.Empty(t, gateway.requests())
 }

internal/cmdenv/cmdenv.go

@@ -43,6 +43,9 @@ type Plan struct {
 	// ReadOnly maps an env var the command gets to the env var it takes its value from instead (DATABASE_URL to
 	// READ_ONLY_DATABASE_URL, say). When the substitute isn't set, the command doesn't get the var at all.
 	ReadOnly map[string]string
+	// Added are the Vars the command gets besides the ones every command in its app gets (the ones it asked for),
+	// sorted. They only change how Summary describes the plan.
+	Added []string
 }
 
 // New returns the plan that gives a command every env var (all), or only vars, with the substitutes in readOnly
@@ -269,6 +272,22 @@ func (p Plan) Describe() string {
 	return strings.Join(p.described(p.Vars), ", ")
 }
 
+// Summary describes the plan in a line, for the person approving the command or reading about it: how many of the
+// app's command_env_vars it gets, the ones added for it, and the read-only ones. Describe names every env var.
+func (p Plan) Summary() string {
+	if p.All || len(p.Vars) == 0 {
+		return p.Describe()
+	}
+	summary := fmt.Sprintf("the %d in the app's command_env_vars", len(p.Vars)-len(p.Added))
+	if len(p.Added) > 0 {
+		summary += ", plus " + strings.Join(p.Added, ", ")
+	}
+	if len(p.ReadOnly) > 0 {
+		summary += "; read-only " + strings.Join(p.described(sortedKeys(p.ReadOnly)), ", ")
+	}
+	return summary
+}
+
 // described names each of names, with its read-only substitute: "DATABASE_URL (from READ_ONLY_DATABASE_URL)".
 func (p Plan) described(names []string) []string {
 	described := make([]string, 0, len(names))

internal/cmdenv/cmdenv_test.go

@@ -165,6 +165,23 @@ func TestDescribe(t *testing.T) {
 		mustPlan(t, true, nil, readOnly).Describe())
 }
 
+// Summary counts the app's env vars, and names only the ones added for the command and the read-only ones.
+func TestSummary(t *testing.T) {
+	readOnly := map[string]string{"DATABASE_URL": "READ_ONLY_DATABASE_URL"}
+	plan := mustPlan(t, false, []string{"PATH", "DATABASE_URL", "HOME", "SENTRY_DSN"}, readOnly)
+	require.Equal(t, "the 4 in the app's command_env_vars; read-only DATABASE_URL (from READ_ONLY_DATABASE_URL)",
+		plan.Summary())
+	plan.Added = []string{"SENTRY_DSN"}
+	require.Equal(t, "the 3 in the app's command_env_vars, plus SENTRY_DSN; read-only DATABASE_URL "+
+		"(from READ_ONLY_DATABASE_URL)", plan.Summary())
+	plan.ReadOnly = nil
+	require.Equal(t, "the 3 in the app's command_env_vars, plus SENTRY_DSN", plan.Summary())
+	// Every env var, and none, read as Describe says them.
+	require.Equal(t, "every env var except RACK_URL; read-only DATABASE_URL (from READ_ONLY_DATABASE_URL)",
+		mustPlan(t, true, nil, readOnly).Summary())
+	require.Equal(t, "no env vars", Plan{}.Summary())
+}
+
 // shell is a shell the rack's image might have: how to run it, and the directory holding the env it uses (""
 // for the one on PATH).
 type shell struct {

internal/gateway/audit/command_env.go

@@ -29,15 +29,19 @@ func CommandEnvFromContext(ctx context.Context) (CommandEnv, bool) {
 }
 
 // details are the audit details that record the environment: all_env_vars when the command got every env var
-// (except RACK_URL), or env_vars, the names it got (only those that were set reached it); env_read_only, each
-// read-only var it got with the var it took its value from; and write when it was given their real values.
+// (except RACK_URL), or env_vars, the names it got (only those that were set reached it), and env_vars_added, the
+// ones it asked for besides the app's; env_read_only, each read-only var it got with the var it took its value
+// from; and write when it was given their real values.
 func (e CommandEnv) details() map[string]interface{} {
 	details := map[string]interface{}{}
 	if e.Plan.All {
 		details["all_env_vars"] = true
 	} else {
 		details["env_vars"] = append([]string{}, e.Plan.Vars...)
 	}
+	if len(e.Plan.Added) > 0 {
+		details["env_vars_added"] = e.Plan.Added
+	}
 	if len(e.Plan.ReadOnly) > 0 {
 		details["env_read_only"] = e.Plan.ReadOnly
 	}

internal/gateway/proxy/command_env_test.go

@@ -68,6 +68,7 @@ func TestACommandRunsWithTheAppsEnvVarsAndTheOnesItAsksFor(t *testing.T) {
 	details := f.processStartDetails(t, "success")
 	require.Equal(t, "rake db:migrate", details["command"])
 	require.Equal(t, []interface{}{"DATABASE_URL", "PATH", "STRIPE_KEY"}, details["env_vars"])
+	require.Equal(t, []interface{}{"STRIPE_KEY"}, details["env_vars_added"])
 	require.Equal(t, map[string]interface{}{"DATABASE_URL": "READ_ONLY_DATABASE_URL"}, details["env_read_only"])
 	require.NotContains(t, details, "write")
 	require.NotContains(t, details, "all_env_vars")
@@ -220,9 +221,12 @@ func TestAnApprovedCommandsEntryDecidesItsEnvVars(t *testing.T) {
 	}, nil))
 
 	for command, want := range map[string]audit.CommandEnv{
-		"bin/pre_release_checks migrate": {Plan: cmdenv.Plan{Vars: []string{"DATABASE_URL", "PATH"}}, Write: true},
+		"bin/pre_release_checks migrate": {
+			Plan: cmdenv.Plan{Vars: []string{"DATABASE_URL", "PATH"}, Added: []string{"DATABASE_URL"}}, Write: true,
+		},
 		"rake stripe:prepare": {Plan: cmdenv.Plan{
 			Vars: []string{"DATABASE_URL", "PATH", "STRIPE_KEY"}, ReadOnly: readOnlyDatabase,
+			Added: []string{"DATABASE_URL", "STRIPE_KEY"},
 		}},
 		"bin/pre_release_checks": {Plan: cmdenv.Plan{All: true, ReadOnly: readOnlyDatabase}},
 	} {

internal/gateway/settings/command_env.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"slices"
 	"strings"
 
 	"github.com/DocSpring/rack-gateway/internal/cmdenv"
@@ -50,7 +51,17 @@ func (e CommandEnv) Plan(req cmdenv.Request) (cmdenv.Plan, error) {
 		readOnly = nil
 	}
 	vars := append(append([]string(nil), e.Vars...), req.Env...)
-	return cmdenv.New(req.AllEnvVars || len(e.Vars) == 0, vars, readOnly)
+	plan, err := cmdenv.New(req.AllEnvVars || len(e.Vars) == 0, vars, readOnly)
+	if err != nil || plan.All {
+		return plan, err
+	}
+	for _, name := range req.Env {
+		if !slices.Contains(e.Vars, name) && !slices.Contains(plan.Added, name) {
+			plan.Added = append(plan.Added, name)
+		}
+	}
+	slices.Sort(plan.Added)
+	return plan, nil
 }
 
 // CommandEnvFromValues reads command_env_vars and command_env_read_only from their values, as the settings API

internal/gateway/settings/command_env_test.go

@@ -156,7 +156,16 @@ func TestCommandEnvPlan(t *testing.T) {
 		{env: limited, want: cmdenv.Plan{Vars: []string{"DATABASE_URL", "PATH"}, ReadOnly: readOnly}},
 		{
 			env: limited, names: []string{"STRIPE_KEY"}, write: true,
-			want: cmdenv.Plan{Vars: []string{"DATABASE_URL", "PATH", "STRIPE_KEY"}},
+			want: cmdenv.Plan{Vars: []string{"DATABASE_URL", "PATH", "STRIPE_KEY"}, Added: []string{"STRIPE_KEY"}},
+		},
+		// Added names only the ones the app doesn't give every command, once each, sorted.
+		{
+			env: limited, names: []string{"STRIPE_KEY", "PATH", "SENTRY_DSN", "STRIPE_KEY"},
+			want: cmdenv.Plan{
+				Vars:     []string{"DATABASE_URL", "PATH", "SENTRY_DSN", "STRIPE_KEY"},
+				ReadOnly: readOnly,
+				Added:    []string{"SENTRY_DSN", "STRIPE_KEY"},
+			},
 		},
 		{env: limited, all: true, want: cmdenv.Plan{All: true, ReadOnly: readOnly}},
 	} {

internal/gateway/slack/process_notifications.go

@@ -35,10 +35,11 @@ type processCommand struct {
 	ScriptName    string `json:"script_name"`
 	ScriptSHA256  string `json:"script_sha256"`
 	// The env vars the command got (see audit.CommandEnv).
-	EnvVars     []string          `json:"env_vars"`
-	AllEnvVars  bool              `json:"all_env_vars"`
-	EnvReadOnly map[string]string `json:"env_read_only"`
-	Write       bool              `json:"write"`
+	EnvVars      []string          `json:"env_vars"`
+	EnvVarsAdded []string          `json:"env_vars_added"`
+	AllEnvVars   bool              `json:"all_env_vars"`
+	EnvReadOnly  map[string]string `json:"env_read_only"`
+	Write        bool              `json:"write"`
 }
 
 // parseProcessCommand reads the command of a process action's audit entry. ok is false for any other entry.
@@ -91,7 +92,8 @@ func (p processCommand) env() string {
 	if p.EnvVars == nil && !p.AllEnvVars {
 		return ""
 	}
-	text := escapeMrkdwn(cmdenv.Plan{All: p.AllEnvVars, Vars: p.EnvVars, ReadOnly: p.EnvReadOnly}.Describe())
+	plan := cmdenv.Plan{All: p.AllEnvVars, Vars: p.EnvVars, ReadOnly: p.EnvReadOnly, Added: p.EnvVarsAdded}
+	text := escapeMrkdwn(plan.Summary())
 	if p.Write {
 		text = ":warning: *Can change data:* run with --write, so it got the real values of the read-only env " +
 			"vars\n" + text

internal/gateway/slack/process_notifications_test.go

@@ -119,10 +119,13 @@ func TestOnlyProcessCommandsGetTheProcessFormat(t *testing.T) {
 // An entry from before the gateway recorded them says nothing about them.
 func TestProcessMessageSaysWhichEnvVarsTheCommandGot(t *testing.T) {
 	for want, env := range map[string]map[string]interface{}{
-		"*Env vars:*\\nDATABASE_URL (from READ_ONLY_DATABASE_URL), PATH": {
+		"*Env vars:*\\nthe 2 in the app's command_env_vars; read-only DATABASE_URL (from READ_ONLY": {
 			"env_vars":      []string{"DATABASE_URL", "PATH"},
 			"env_read_only": map[string]string{"DATABASE_URL": "READ_ONLY_DATABASE_URL"},
 		},
+		"*Env vars:*\\nthe 2 in the app's command_env_vars, plus SENTRY_DSN": {
+			"env_vars": []string{"DATABASE_URL", "PATH", "SENTRY_DSN"}, "env_vars_added": []string{"SENTRY_DSN"},
+		},
 		"*Env vars:*\\n:warning: *Can change data:* run with --write": {
 			"env_vars": []string{"DATABASE_URL", "PATH"}, "write": true,
 		},

web/package.json

@@ -1,6 +1,6 @@
 {
   "name": "web",
-  "version": "0.1.26",
+  "version": "0.1.27",
   "private": true,
   "type": "module",
   "scripts": {

web/src/components/audit-logs/command-details.test.tsx

@@ -56,8 +56,21 @@ describe('CommandDetails', () => {
     expect(screen.getByTestId('audit-command-env')).toHaveTextContent(
       'Env vars: DATABASE_URL (from READ_ONLY_DATABASE_URL), PATH'
     )
+    expect(screen.queryByTestId('audit-command-env-added')).toBeNull()
     expect(screen.queryByTestId('audit-command-write')).toBeNull()
 
+    // The ones it asked for besides the app's are named again.
+    rerender(
+      <CommandDetails
+        details={JSON.stringify({
+          command: 'rake stripe:sync',
+          env_vars: ['DATABASE_URL', 'PATH', 'STRIPE_KEY'],
+          env_vars_added: ['STRIPE_KEY'],
+        })}
+      />
+    )
+    expect(screen.getByTestId('audit-command-env-added')).toHaveTextContent('Asked for: STRIPE_KEY')
+
     rerender(
       <CommandDetails
         details={JSON.stringify({ command: 'rake db:migrate', all_env_vars: true, write: true })}

web/src/components/audit-logs/command-details.tsx

@@ -8,6 +8,8 @@ interface CommandAuditDetails {
   scriptName?: string
   scriptSHA256?: string
   env?: string
+  // envAdded are the env vars the command asked for besides the ones every command in the app gets.
+  envAdded: string[]
   write: boolean
 }
 
@@ -68,6 +70,9 @@ function parseCommandDetails(details: string | null | undefined): CommandAuditDe
     scriptName: stringField(record, 'script_name'),
     scriptSHA256: stringField(record, 'script_sha256'),
     env: envDescription(record),
+    envAdded: Array.isArray(record.env_vars_added)
+      ? record.env_vars_added.filter((name): name is string => typeof name === 'string')
+      : [],
     write: record.write === true,
   }
 }
@@ -147,6 +152,11 @@ export function CommandDetails({ details }: { details: string | null | undefined
           <span className="text-muted-foreground">Env vars:</span> {command.env}
         </div>
       )}
+      {command.envAdded.length > 0 && (
+        <div className="break-words" data-testid="audit-command-env-added">
+          <span className="text-muted-foreground">Asked for:</span> {command.envAdded.join(', ')}
+        </div>
+      )}
       {command.write && (
         <div className="font-medium text-destructive" data-testid="audit-command-write">
           Can change data: run with --write, so it got the real values of the read-only env vars.