diff --git a/convox.yml b/convox.yml
index 1cedbb76..003ef335 100644
--- a/convox.yml
+++ b/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
diff --git a/docs/src/content/docs/concepts/audit-logging.mdx b/docs/src/content/docs/concepts/audit-logging.mdx
index 2c888f1f..937cbcf3 100644
--- a/docs/src/content/docs/concepts/audit-logging.mdx
+++ b/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:
diff --git a/docs/src/content/docs/user-guide/cli/commands.mdx b/docs/src/content/docs/user-guide/cli/commands.mdx
index f3ff0ed2..8984cef5 100644
--- a/docs/src/content/docs/user-guide/cli/commands.mdx
+++ b/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
diff --git a/internal/cli/AGENTS.md b/internal/cli/AGENTS.md
index fe71771d..a488f783 100644
--- a/internal/cli/AGENTS.md
+++ b/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.
diff --git a/internal/cli/convox_process_env.go b/internal/cli/convox_process_env.go
index ec56a45e..24784b00 100644
--- a/internal/cli/convox_process_env.go
+++ b/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.
diff --git a/internal/cli/convox_process_env_test.go b/internal/cli/convox_process_env_test.go
index df95fcdd..4779774d 100644
--- a/internal/cli/convox_process_env_test.go
+++ b/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())
}
diff --git a/internal/cmdenv/cmdenv.go b/internal/cmdenv/cmdenv.go
index fb47402d..5f9bcabb 100644
--- a/internal/cmdenv/cmdenv.go
+++ b/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))
diff --git a/internal/cmdenv/cmdenv_test.go b/internal/cmdenv/cmdenv_test.go
index a4367200..8672bdc7 100644
--- a/internal/cmdenv/cmdenv_test.go
+++ b/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 {
diff --git a/internal/gateway/audit/command_env.go b/internal/gateway/audit/command_env.go
index c7721918..bdfcc133 100644
--- a/internal/gateway/audit/command_env.go
+++ b/internal/gateway/audit/command_env.go
@@ -29,8 +29,9 @@ 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 {
@@ -38,6 +39,9 @@ func (e CommandEnv) details() map[string]interface{} {
} 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
}
diff --git a/internal/gateway/proxy/command_env_test.go b/internal/gateway/proxy/command_env_test.go
index b3af9cef..6e9d13e3 100644
--- a/internal/gateway/proxy/command_env_test.go
+++ b/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}},
} {
diff --git a/internal/gateway/settings/command_env.go b/internal/gateway/settings/command_env.go
index 16df104e..550aea9b 100644
--- a/internal/gateway/settings/command_env.go
+++ b/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
diff --git a/internal/gateway/settings/command_env_test.go b/internal/gateway/settings/command_env_test.go
index 04da3719..6691c7f0 100644
--- a/internal/gateway/settings/command_env_test.go
+++ b/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}},
} {
diff --git a/internal/gateway/slack/process_notifications.go b/internal/gateway/slack/process_notifications.go
index 1cfcf0d4..10e3e1a2 100644
--- a/internal/gateway/slack/process_notifications.go
+++ b/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
diff --git a/internal/gateway/slack/process_notifications_test.go b/internal/gateway/slack/process_notifications_test.go
index 1d12edde..c3450d39 100644
--- a/internal/gateway/slack/process_notifications_test.go
+++ b/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,
},
diff --git a/web/package.json b/web/package.json
index 93297821..7f2eeaf6 100644
--- a/web/package.json
+++ b/web/package.json
@@ -1,6 +1,6 @@
{
"name": "web",
- "version": "0.1.26",
+ "version": "0.1.27",
"private": true,
"type": "module",
"scripts": {
diff --git a/web/src/components/audit-logs/command-details.test.tsx b/web/src/components/audit-logs/command-details.test.tsx
index d9557706..52df939f 100644
--- a/web/src/components/audit-logs/command-details.test.tsx
+++ b/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(
+
+ )
+ expect(screen.getByTestId('audit-command-env-added')).toHaveTextContent('Asked for: STRIPE_KEY')
+
rerender(
typeof name === 'string')
+ : [],
write: record.write === true,
}
}
@@ -147,6 +152,11 @@ export function CommandDetails({ details }: { details: string | null | undefined
Env vars: {command.env}
)}
+ {command.envAdded.length > 0 && (
+
+ Asked for: {command.envAdded.join(', ')}
+
+ )}
{command.write && (
Can change data: run with --write, so it got the real values of the read-only env vars.